-
Notifications
You must be signed in to change notification settings - Fork 0
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
Buffer votes for blocks we don't know about #718
Merged
Merged
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
JamesHinshelwood
force-pushed
the
buffer-votes
branch
2 times, most recently
from
February 13, 2024 16:09
52bca93
to
29c17e1
Compare
n-hutton
reviewed
Feb 14, 2024
n-hutton
reviewed
Feb 14, 2024
n-hutton
reviewed
Feb 14, 2024
JamesHinshelwood
force-pushed
the
buffer-votes
branch
from
February 14, 2024 12:52
29c17e1
to
9881e43
Compare
JamesHinshelwood
force-pushed
the
buffer-votes
branch
2 times, most recently
from
February 16, 2024 18:16
1a31548
to
288933f
Compare
JamesHinshelwood
requested review from
bzawisto
and removed request for
n-hutton
February 19, 2024 16:23
theo-zil
requested changes
Feb 22, 2024
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.
Mainly one question about messages sent after processing blocks during sync, and a couple of non-blocking minor nits/questions, otherwise all looks good
JamesHinshelwood
force-pushed
the
buffer-votes
branch
3 times, most recently
from
March 22, 2024 18:01
8c4ee0c
to
28676d5
Compare
JamesHinshelwood
force-pushed
the
buffer-votes
branch
2 times, most recently
from
April 4, 2024 16:19
da04926
to
b1c4e34
Compare
JamesHinshelwood
force-pushed
the
buffer-votes
branch
from
April 8, 2024 07:10
b1c4e34
to
060690f
Compare
theo-zil
reviewed
Apr 8, 2024
JamesHinshelwood
force-pushed
the
buffer-votes
branch
from
April 16, 2024 11:30
060690f
to
09bb5fe
Compare
Votes can arrive for blocks we aren't yet aware of in (at least) two circumstances: * If we disconnect and reconnect, we will download the blocks we missed, but nodes might send us their votes for the next block before we've received the missed blocks. * Due to network latency, a vote for a block proposal could arrive before the block proposal itself. Before this commit, the node ignores these votes and the network eventually recovers via a new view. However, this slows things down so we should recover faster if possible. Instead, we store votes for unknown blocks in memory and replay them if the block later becomes known to us. The implementation is fairly straight forward but there are a few caveats and TODOs: * The return type of `Consensus::proposal` is now more complicated, as it doesn't just return a `Message::Vote` any more. If the proposal results in some buffered votes being replayed and those votes form a supermajority, then the node can immediately propose the next block. * There's nothing which limits the memory usage of buffered votes. A malicious node is perfectly able to send us loads of votes with non-existant block hashes, which we will store forever. I've raised #719 to resolve this. * When applying buffered votes as a result of a proposal, they take priority over our own vote. This means we lose out on the cosigner reward. #720 * The unreliability test could be improved to be more efficient and to assert a stricter condition on the network - #721.
JamesHinshelwood
force-pushed
the
buffer-votes
branch
from
April 16, 2024 13:15
09bb5fe
to
4385fc2
Compare
theo-zil
approved these changes
Apr 16, 2024
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.
Votes can arrive for blocks we aren't yet aware of in (at least) two circumstances:
Before this commit, the node ignores these votes and the network eventually recovers via a new view. However, this slows things down so we should recover faster if possible. Instead, we store votes for unknown blocks in memory and replay them if the block later becomes known to us.
The implementation is fairly straight forward but there are a few caveats and TODOs:
Consensus::proposal
is now more complicated, as it doesn't just return aMessage::Vote
any more. If the proposal results in some buffered votes being replayed and those votes form a supermajority, then the node can immediately propose the next block.unreliable::blocks_are_produced_while_a_node_restarts
test #721.