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

Decouple block queue pruning from block announcements #3027

Open
wants to merge 2 commits into
base: albatross
Choose a base branch
from

Conversation

ii-cruz
Copy link
Member

@ii-cruz ii-cruz commented Nov 7, 2024

What's in this pull request?

The live sync has an invariant that all blocks come with no gaps from the gossip sub.
Before #2982 we had a bug where peers (active validators) were being miscategorized as not synced. Thus we would ignore their blocks from the gossip sub, creating gaps during the syncing process. Every gap would generate a missing blocks request from the head of our node to the block announced in the gossip sub, thus creating too many overlapping requests across multiple batches, leading to the issue reported by community #2954.

This PR changes the following:

  • Moves the processing of block announcements to the end of the poll function, prioritizing finishing the ongoing missing block request results.
  • Detaches pruning from the block announcements, so we prune before adding more data to our structs and so we always prune regardless of an announced block arriving.

This PR no longer restricts the request missing blocks to the current batch. The following changes were dropped in favor of tighter rate limiting on the gossipsub #3008:

  • Only allow concurrent missing block requests within the current batch, thus diminishing the DoS surface significantly.
  • Any block on a future batch gets buffered instead, but no request missing blocks is sent over the network.

Pull request checklist

  • All tests pass. The project builds and runs.
  • I have resolved any merge conflicts.
  • I have resolved all clippy and rustfmt warnings.

@ii-cruz ii-cruz added the bug Something isn't working label Nov 7, 2024
@ii-cruz ii-cruz added this to the Nimiq PoS Mainnet milestone Nov 7, 2024
@ii-cruz ii-cruz requested a review from paberr November 7, 2024 00:45
@ii-cruz ii-cruz self-assigned this Nov 7, 2024
@ii-cruz ii-cruz force-pushed the iicruz/remove_live_sync_no_gaps_invariant2 branch 4 times, most recently from e04db33 to 168621a Compare November 8, 2024 21:31
consensus/src/messages/handlers.rs Outdated Show resolved Hide resolved
consensus/src/sync/live/block_queue/queue.rs Outdated Show resolved Hide resolved
consensus/src/sync/live/block_queue/queue.rs Outdated Show resolved Hide resolved
consensus/src/sync/live/block_queue/queue.rs Outdated Show resolved Hide resolved
consensus/src/sync/live/block_queue/queue.rs Outdated Show resolved Hide resolved
consensus/src/sync/live/block_queue/queue.rs Outdated Show resolved Hide resolved
…lock requests.

Buffer blocks if no missing block requests should be induced
@ii-cruz ii-cruz force-pushed the iicruz/remove_live_sync_no_gaps_invariant2 branch from 168621a to 2153ecd Compare November 12, 2024 16:04
@ii-cruz ii-cruz force-pushed the iicruz/remove_live_sync_no_gaps_invariant2 branch from d7731e7 to 09c424d Compare November 12, 2024 23:54
@ii-cruz ii-cruz requested a review from styppo November 13, 2024 00:14
@ii-cruz ii-cruz changed the title Restrict concurrent missing block requests to current batch Decouple block queue pruning from block announcements Nov 13, 2024
@ii-cruz ii-cruz removed this from the Nimiq PoS Mainnet milestone Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants