[CATCHUP] Init Consensus with 2 views to prevent Double Vote/Propose #3648
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bfish713
changed the title
lukeiannucci
reviewed
Sep 5, 2024
| } | ||
|
|
||
| /// reset last actions to genesis so we can resend events in tests | ||
| pub fn reset_actions(&mut self) { |
There was a problem hiding this comment.
is there a way to gate this to tests only? or do we want to potentially use it elsewhere in the future
There was a problem hiding this comment.
I tried to gate this for test but it causes a whole bunch of issues because lots of the byzantine code isn't gated for test etc. I'll follow up with the gating because we should gate this
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #3626
This PR:
This PR adds a new start parameter to consensus which is the view we last "actioned" in. An action is voting or proposing for DA/Quorum. The PR also enforces that we don't send double actions by checking in the network task if we've already sent something for the view we are about to transmit a vote/proposal to or a higher view. This means if we propose in view 10 we can't then propose for view 9. We should never want to do this.
The PR also adds the consensus shared state to the byzantine behaviors so that they can reset the last actioned view so their byzantine sends are not blocked.
This PR does not:
This PR doesn't apply to DA votes because after view sync we can often legitimately get a proposal for view n and n+1 in either order. If we handle the n+1 view first we don't want to not vote for view n
Key places to review:
-Changes to the
HotshotInitializernetwork.rsConsensus