-
-
Notifications
You must be signed in to change notification settings - Fork 281
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: trigger block search for unknown block attestations #5485
Conversation
Performance Report✔️ no performance regression detected Full benchmark results
|
blocked by #5486 |
c3d69c6
to
4214732
Compare
4214732
to
c425a24
Compare
// Trigger unknown block root search here | ||
const rootHex = toHexString(blockRoot); | ||
events.emit(NetworkEvent.unknownBlock, {rootHex}); |
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.
How does this safeguard against spamming unknownBlock events if multiple attestations reference to the same unknown block root
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.
@dapplion I addressed this in the latest commit, there is a cache inside NetworkProcessor to only search for 1 root per slot, and NetworkEvent.unknownBlock
is triggered in a single place
we need to also cover triggering block search for unknown root sync committee and blobsidecar, will do it in another PR |
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.
Looks great! Thank you
🎉 This PR is included in v1.9.0 🎉 |
Motivation
Description
NetworkEvent.unknownBlock
network event, emitted when unknown block attestationsunknownBlockParent
andunknownBlock
eventCloses #3613