-
Notifications
You must be signed in to change notification settings - Fork 219
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: reset broken sync #4955
feat: reset broken sync #4955
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the logic in add_block should then call this method as well, so that we don't have duplicate code
chain_strength_comparer: &dyn ChainStrengthComparer, | ||
) -> Result<(), ChainStorageError> { | ||
let metadata = db.fetch_chain_metadata()?; | ||
// lets clear out all remaining headers that done have a matching block |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// lets clear out all remaining headers that done have a matching block | |
// lets clear out all remaining headers that don't have a matching block |
740166e
to
fad6820
Compare
// height it will only trim the extra headers with no blocks | ||
rewind_to_height(db, metadata.height_of_longest_chain())?; | ||
let all_orphan_tips = db.fetch_all_orphan_chain_tips()?; | ||
if all_orphan_tips.is_empty() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this check should be before we rewind
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I specifically did not do this as I use the rewind code here to remove the "extra" headers.
I want to "main chain" to be "clean" after this run.
The intended case is this.
Header sync downloads 10 headers.
Block sync fails after 5.
That extra 5 headers will stay on top of the main chain till we remove them again. I want to remove them at this point regardless of whether the new tip is higher than our original one.
if all_orphan_tips.is_empty() { | ||
// we have no orphan chain tips, we have trimmed remaining headers, we are on the best tip we have, so lets | ||
// return ok | ||
return Ok(BlockAddResult::OrphanBlock); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably rename this to ChainReorgResult
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And OrphanBlock
to change to NoChange
Description
If sync fails resets chain to the highest pow chain the node locally has the data to.
Motivation and Context
See: #4866
How Has This Been Tested?
Unit tests