-
Notifications
You must be signed in to change notification settings - Fork 169
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
Specify 'valid' condition for block SSE event #404
Conversation
I'm unsure of the point of the (The only time we wouldn't have overlap would be if the imported block wasn't made head, but not sure of the use case in that situation, either.) |
Not necessarily, not all newly imported blocks become head and re-orgs cause previously imported blocks to become head. The head event is very relevant on its own and orthogonal to the block event. The rules of this PR align with what Lodestar and Lighthouse are doing right now. I haven't look into other implementations but I suspect it will be similar. |
Prysm does the same |
I'm happy to clarify that wording, I guess my one caveat remains this is still not a great way of measuring anything time sensitive given these messages may be the lowest priority to action on a node, and the latency from that prioritisation etc... Within the scope of whats proposed, I think this is a good clarification. |
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.
LGTM
@michaelsproul feedback? |
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.
LGTM
From discussion in #349, specify to emit the
block
event only for blocks successfully imported on fork-choice. This condition includes: valid state transition (super-set of gossip rules) + valid/syncing execution + DA checks