fix(l1): always use fullsync if db had data #5196
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Lines of code reportTotal lines added: Detailed view |
mpaulucci
reviewed
Nov 5, 2025
crates/networking/p2p/sync.rs
Outdated
| debug!( | ||
| "Sync head is less than {MIN_FULL_BLOCKS} blocks away, switching to FullSync" | ||
| ); | ||
| debug!("Sync head is found switching to FullSync"); |
Collaborator
There was a problem hiding this comment.
this could be an info. Also, comma missing.
mpaulucci
reviewed
Nov 5, 2025
mpaulucci
reviewed
Nov 5, 2025
| current_head_number = last_block_number; | ||
|
|
||
| // If the sync head is less than 64 blocks away from our current head switch to full-sync | ||
| // If the sync head is not 0 we search to fullsync |
Collaborator
There was a problem hiding this comment.
IMO, if the sync head is 0 pero is less than X blocks away, we should also not do snap sync. Snap sync is for 1M blocks distance, not 100.
jrchatruc
approved these changes
Nov 5, 2025
MegaRedHand
reviewed
Nov 5, 2025
| // If the sync head is not 0 we search to fullsync | ||
| let head_found = sync_head_found && store.get_latest_block_number().await? > 0; | ||
| // Or the head is very close to 0 | ||
| let head_close_to_0 = last_block_number < MIN_FULL_BLOCKS; |
Collaborator
There was a problem hiding this comment.
This skips snap-syncing when we have a few blocks in the DB, while the old logic compared the distance between head and sync-head.
Edit: this skips snap-syncing when the sync-head's block number is less than 10000
MegaRedHand
approved these changes
Nov 5, 2025
xqft
pushed a commit
that referenced
this pull request
Nov 11, 2025
**Motivation** When the db had data, we should fullsync, not override the user's data **Description** - Changes the fullsync option instead of snap to be any number instead of `MIN_FULL_BLOCKS`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
When the db had data, we should fullsync, not override the user's data
Description
MIN_FULL_BLOCKS