Repository navigation
fix(consensus): deliver and process a polka block for a validator left in its round - #1600
Draft
Claudius-Maginificent wants to merge 3 commits into
Draft
Claudius-Maginificent wants to merge 3 commits into
Claudius-Maginificent wants to merge 3 commits into
Conversation
…t in its round A validator that saw a polka for a block it never received precommits nil and, once its peers move to a later round, can no longer obtain the block: block parts are served only from the proposal part set, which entering a new round clears, while the valid and locked part sets that still hold the block are never gossiped. Even when the block does arrive, one completed in the prevote or precommit step is not processed, so an application that verifies vote extensions against its last processed proposal, like Drive, rejects every precommit for the block. Rejected precommits are not stored and do not count towards the +2/3 of any kind that start the precommit wait, so the validator stays in its round for good and a network without spare voting power stalls with it. Serve a validator peer at this height, in an earlier round, the complete valid or locked part set whose header it is collecting, from the gossip snapshot. Process the round's polka block when it completes too late for the round to vote on it, so that the precommits for it verify; the node casts no vote again, and a failure is not retried. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A polka block the application rejects is processed once and not retried on the precommits for it or on its parts arriving again; a block completing in the prevote wait is processed there and not again in the precommit step; one completing in the propose step with its proposal takes the usual path and is processed once; a block completing without being the round's polka block is not processed; an incomplete held part set is not served. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A block completing while the round's polka is for another block is not processed; a nil polka alone let the identity check go unexercised. The rejected-block test no longer redelivers parts the part set already holds, which completes nothing. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Contributor
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the
|
|
🕓 Review not started yet because this PR is a draft.
Commit 2cc8d96. Normal review starts when eligible; priority review starts as soon as a slot is available. |
This branch has not been deployed
This file contains hidden or 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
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.
TL;DR: A validator that fell behind can now get the block the network already agreed to vote on, and process it, so it can rejoin the round instead of leaving the network stuck.
User story
As a validator operator, I want a validator that missed a block to fetch and process it from its peers, to achieve continued block production after network trouble or validator outages.
Scenario
Base flow
In each round a proposer sends a block. Once more than two-thirds of validators prevote for it (a "polka"), validators precommit it. Precommits for a block carry vote extensions, which the application (Drive) checks before the vote is counted.
Actual behavior
Take a validator that saw a polka for a block but never received the block itself:
If the validators that went ahead cannot reach a two-thirds quorum without the stuck ones, the whole network stalls. Two ways to get there:
Expected behavior
Validators that still hold the agreed block send it to peers that ask for it. A validator that completes the block processes it, accepts the other validators' precommits, starts its timer and rejoins the round. With the round back at a two-thirds quorum, the network commits the block.
Detailed discussion
What was done
gossip_handlers.go,heldBlockPartsFor):rs.ProposalBlockParts, which entering a new round clears. A node then still holds the polka block asValidBlockPartsand/orLockedBlockParts, but those were never served.ValidBlockPartsorLockedBlockPartsis now served to a peer that meets all of these:ProposalBlockPartSetHeaderequals the held one exactly (header equality is the block identity).ProposalBlockParts, andRoundto the peer's round soSetHasProposalBlockPartmarks the parts) and calls the existingGossipProposalBlockParts.state_add_prop_block.go,ProposalCompletedAction.processPolkaBlock):ProcessProposal.ensureProcess(rs, rs.Round).ensureOwnRoundthen finds the round processed, andVerifyVoteExtensionaccepts the peers' precommits.ProposalCompletedActiongets theblockExecdependency (state_controller.go).roundVoteResendGossipHandlerfrom feat(consensus)!: hand a silent proposer's idle round to the next proposer #1572 already resends the earlier round's precommits.Testing
New tests:
TestGossipHandlerTestSuite/TestDataGossipHandlerServesHeldBlockToLaggingPeerTestStatePolkaBlockCompletedInPrecommitIsProcessedTestStatePolkaBlockDeliveryEndsThePrecommitStallTriggeredTimeoutPrecommit.TestStatePolkaBlockRejectedByTheApplicationIsNotRetriedTestStatePolkaBlockCompletedInPrevoteWaitIsProcessedOnceTestStatePolkaBlockCompletedInProposeVotesAndProcessesOnceTestStateCompletedBlockWithoutItsPolkaIsNotProcessedTestStateCompletedBlockOtherThanThePolkaBlockIsNotProcessedThe tests use a Drive-like application that accepts extensions only for a processed (height, round, hash).
The new tests also pass under
-race. A regression subset (lock, full-round, proposal, prevote/precommit, gossip, commit-extension and reactor tests) passes, andmake lintreports 0 issues.An independent review found:
Breaking changes
None.
Prior work
Attribution
🤖 Generated with Claude Code
https://claude.ai/code/session_01WunYYgcoPDNh3uebhUCwig
🤖 Co-authored by Claudius the Magnificent AI Agent