-
Notifications
You must be signed in to change notification settings - Fork 220
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
Ensure new blocks are marked complete only when strict finality is reached #3549
Ensure new blocks are marked complete only when strict finality is reached #3549
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.
Lookgs good overall
// We don't gossip immediate switch blocks | ||
if self.is_immediate_switch_block_for_current_protocol_version { | ||
return StateChange::AlreadySet; | ||
} |
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.
Shouldn't this be handled outside the State
logic? To keep it simple and compact, I think we should only do basic stuff here like offering getters and setters, let other parts of the code deal with specific logic.
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'd rather keep it here. The logic isn't complicated and to move it outside would require a getter for is_immediate_switch_block_for_current_protocol_version
. If other reviewers agree with you though, I can do the change.
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 👍 with just a few nits.
Nice job!
bors merge |
Build succeeded: |
This PR introduces a small collection of state flags which are carried around with blocks being executed and accumulated.
The state is held in a type named
HotBlock
- i.e. a new block which is currently being worked upon. The state is primarily used in the reactor'sdispatch_event
via theHotBlockAnnouncement
. This announcement is made by theContractRuntime
after executing aFinalizedBlock
and by theBlockAccumulator
after storing a block which has just reached sufficient finality.The introduction of this state has allowed for the removal of some state from the
BlockAccumulator
and also should ensure that actions such as executing a block, storing a block or marking it complete are all done only once per block.It also has allowed the node to avoid needlessly gossiping new immediate switch blocks (all nodes going through an upgrade create the immediate switch block, not just the validators).
Closes #3520.