Repository navigation
fix(consensus): resend same-round votes the peer never acknowledged - #1599
Draft
Claudius-Maginificent wants to merge 9 commits into
Draft
Claudius-Maginificent wants to merge 9 commits into
Claudius-Maginificent wants to merge 9 commits into
Conversation
Vote gossip marks a vote as held by the peer the moment it is handed to the transport, and nothing inside a round puts it back on offer: a copy dropped after enqueue (router queue overflow, receiver over its allowance) is never sent again. The only in-round repair, the VoteSetMaj23 exchange, needs one side to already hold a +2/3 majority, so when the lost votes are exactly what both sides lack to reach one, the round stalls with no timeout to fall back on (prevote-wait starts only on HasTwoThirdsAny). PeerState now tracks current-round prevotes and precommits that were sent but not confirmed by the peer (HasVote, VoteSetBits, or the peer's own copy), in two redelivery windows rotated once per interval. A vote still unconfirmed after a full window is unmarked so regular gossip offers it again; the window scheme means nothing is resent while its acknowledgement is merely in flight, and bookkeeping of a round the peer has left is dropped. The interval follows PeerQueryMaj23SleepDuration, the cadence of the existing periodic repair. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
An acknowledgement applied between picking a vote and recording it as sent was discarded: SetVoteSent tracked the vote regardless, so two rotations later it was resent — and a resend can never be acknowledged, since the receiver drops the duplicate before any HasVote is published. The vote then cycled for the rest of the round. The same loop followed a HasVote lost in transit. SetVoteSent now tracks a vote only when the peer's bit was still clear at the time of the call; the per-peer gossip goroutine picks votes only while their bit is clear, so a set bit means an ack landed in that window. Marking the bit and acknowledging are split (setVoteBit / setHasVote) so the sent path cannot ack itself. Tracking is per vote, with the send time and a re-offer count: each re-offer doubles the wait before the next, capped at voteResendMaxBackoff doublings, so a vote whose ack was lost is resent at a falling rate rather than every interval, and is never given up on. Tests: cover the ack-before-sent interleaving, the backoff schedule, the VoteSetBits acknowledgement path, and make the moved-on test reach the stale-round reset (round-0 bookkeeping must not clear the peer's POL bits that now stand for round 0). 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 9856707. Normal review starts when eligible; priority review starts as soon as a slot is available. |
…elivery # Conflicts: # internal/consensus/peer_state.go
A precommit the application refuses, or a vote conflicting with one already held, is never stored, so no HasVote acknowledges it and its sender re-offers it for the rest of the round. Each copy logged at Error and, for a conflict, was appended to the evidence pool's buffer again. BlockExecutor.VerifyVoteExtension now returns sm.ErrVoteExtensionRejected, classified as peer-floodable so copies log at Debug; the first rejection of a vote at its height is logged at Warn with whether the voted block is processed. A conflicting vote is reported to the evidence pool and logged at Error once per height; copies log at Debug. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…round Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e resend The round vote resend handler's comments said a vote lost after the channel took it is never sent again; it is re-offered after a redelivery interval, with backoff. peer-query-maj23-sleep-duration also paces that redelivery, which its config comment, TOML template and docs now say, and the WithVoteResendInterval comment names where the interval comes from. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A precommit the application refuses for a block this node has not processed is expected and now logs at Debug only; the first refusal of a processed block logs at Warn, counted from when the block was processed, so an early refusal cannot hide a later disagreement over the extensions. The rejected-vote memory is capped: nothing stored bounds a rejected vote and a validator key signs one for any round and block, so past the cap a vote is reported on every copy rather than remembered. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…arning A refused precommit for another block of the processed round, or for the processed block in another round, must not warn: each case fails if its guard is dropped. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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: Validators now resend a consensus vote that a peer never confirmed receiving, so a vote lost in transit can no longer stall a round.
User story
As a validator operator, I want the network to recover by itself when consensus votes get lost during congestion, to achieve continued block production without manual restarts.
Scenario
Base flow
In each round, every validator votes and passes the votes it knows about on to its peers. A round moves on once a validator has seen votes from more than two-thirds of the voting power.
Actual behavior
A node treats a vote as delivered to a peer the moment it hands the vote to the network layer, and never sends that vote to that peer again. If the vote is dropped later on (for example, by an overloaded outgoing queue or by the receiver's rate limit), the peer never gets it. When only honest validators are voting and some of their votes are lost this way, no validator sees enough votes to start the timer that would move the round on. The round stalls until a node reconnects or restarts.
Expected behavior
A vote that the peer has not confirmed is offered again after a short delay, so lost votes are eventually delivered and the round completes.
Detailed discussion
What was done
optimistic: trueingossip_peer_worker.go).GossipVotemarks the vote in the peer's bit array on enqueue, andPeerState.PickVoteToSendnever offers a marked vote again.internal/p2p/rqueue.godrops envelopes abovemaxSize;VoteSetMaj23/VoteSetBitsreconciliation starts only from a localTwoThirdsMajority(). The case of honest validators only, with fewer than 67 votes visible on each side, therefore had no recovery path.PeerStatenow distinguishes a vote that was sent from one that was acknowledged:SetVoteSent, used by the gossiper, sets the peer bit as before and records the vote for the peer's current (height, round). It records the vote only if the bit was still clear; a set bit means an acknowledgement already landed during the unlocked send.HasVote, from the peer's own vote, and fromVoteSetBits, and they drop the entry.ReofferUnackedVotes, called at the start ofGossipVote, clears the peer bit of unacknowledged votes so ordinary gossip offers them again.PeerQueryMaj23SleepDurationof 2 s.PeerGossipSleepDuration, and receivers reject duplicates cheaply before signature verification.NewPeerStatevia the newWithVoteResendIntervaloption inreactor.peerUp.peer-query-maj23-sleep-durationis documented as also pacing vote redelivery (config comment, TOML template,docs/nodes/configuration.md).BlockExecutor.VerifyVoteExtensionreturns the new sentinelsm.ErrVoteExtensionRejected, which is classified as peer-floodable, so each copy logs at Debug.Testing
devor on the first commit before its fix:TestGossiper/TestGossipVoteRedeliversUnacknowledged/{prevote,precommit}TestGossiper/TestGossipVoteRedeliveryStopsWhenPeerMovesOn; a mutation check confirmed it fails when the stale-round reset is removedTestVoteRedeliveryWindowsTestVoteRedeliveryIgnoresAckBeforeSentTestVoteRedeliveryBacksOffTestVoteRedeliveryVoteSetBitsAckTestVoteRedeliveryDropsBookkeepingOnHeightChange(mutation-checked)TestRejectedVoteExtensionLogsOnceThenDebug(unprocessed → processed block, other block, other round; mutation-checked)TestAddVoteErrorMwReportsEachConflictOnce,TestRejectedVotesBounded, an addedTestIsPeerFloodableErrorcaseTestSetHasVote,TestApplyHasVoteMessage,TestPeerState,TestReactorBasic,TestReactorWithEvidence,TestReactorVotingPowerChange,TestReactorValidatorSetChanges, and the Maj23/HasVote/VoteSetBits tests.-raceruns of the gossiper and redelivery tests pass.make lintreports 0 issues.Breaking changes
None.
Prior work
roundVoteResendGossipHandler, which resends a validator peer's own-round votes (and later-round votes) on consensus v2 when the peer is inRoundStepNewRoundor in an earlier round, without acknowledgement tracking. It partly overlaps with this PR on v2. It sends without touching the peer vote bits, so the two do not interfere.🤖 Generated with Claude Code
https://claude.ai/code/session_01WunYYgcoPDNh3uebhUCwig
🤖 Co-authored by Claudius the Magnificent AI Agent