-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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: add another distance check #3501
Conversation
Codecov Report
... and 8 files with indirect coverage changes
Flags with carried forward coverage won't be shown. Click here to find out more.
|
fn buffered_block_by_hash(&self, block_hash: BlockHash) -> Option<SealedBlock> { | ||
self.tree.read().get_buffered_block(&block_hash).map(|b| b.block.clone()) | ||
} | ||
|
||
fn buffered_header_by_hash(&self, block_hash: BlockHash) -> Option<SealedHeader> { | ||
self.tree.read().get_buffered_block(&block_hash).map(|b| b.header.clone()) | ||
} | ||
|
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.
Nit I'd make this return reference instead of cloning
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.
not possible unfortunately because can't borrow past read lock
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.
ah yikes - good point
while we're downloading via tree, we're still receiving new payloads and FCU, so the sync target and the finalized block and hence the distance from local to head constantly moves, and can eventually exceed the targeted 1 epoch.
this adds another check to also lookup if we have the latest finalized block buffered so we can check the distance here again.
This prevents the scenario where we receive a future finalized block the
NewPayload
and therefore never have to download the finalized block if the FCU is updated.