Skip to content
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

Limit the amount of memory used by buffered votes #719

Open
JamesHinshelwood opened this issue Feb 14, 2024 · 0 comments
Open

Limit the amount of memory used by buffered votes #719

JamesHinshelwood opened this issue Feb 14, 2024 · 0 comments
Assignees

Comments

@JamesHinshelwood
Copy link
Contributor

When a vote arrives containing a block hash for a block we don't know about, we store it in memory. This is in case the block arrives later and we are the leader of the view, meaning we need to process the vote.

However, we don't know in advance whether we will be the leader of a view (because we don't know the committee). This means we currently store all valid votes that arrive. It is possible for a malicious node to send us valid votes for non-existent blocks and fill up our memory.

We should implement some kind of protection against this. Some ideas include:

  • Limit buffered votes based on how recently they were received. Votes also include a view so once a block with a later view is finalised, we can discard buffered votes for earlier views.
  • Limit the number of votes we buffer from each peer. Realistically, we should only buffer a maximum of one vote for each peer at a given time.
JamesHinshelwood added a commit that referenced this issue Feb 14, 2024
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 added a commit that referenced this issue Feb 16, 2024
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.
@bzawisto bzawisto self-assigned this Mar 14, 2024
JamesHinshelwood added a commit that referenced this issue Mar 18, 2024
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 added a commit that referenced this issue Mar 18, 2024
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 added a commit that referenced this issue Mar 22, 2024
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 added a commit that referenced this issue Apr 3, 2024
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 added a commit that referenced this issue Apr 4, 2024
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 added a commit that referenced this issue Apr 8, 2024
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 added a commit that referenced this issue Apr 16, 2024
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 added a commit that referenced this issue Apr 16, 2024
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.
github-merge-queue bot pushed a commit that referenced this issue Apr 16, 2024
* Buffer votes for blocks we don't know about

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.

* Return a `Proposal` from `receive_block` if we can
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants