[Claudius-Maginificent]

TL;DR: A node could silently stop sending some validators' votes to one of its peers for the rest of a round. This happened when a late message about the previous block arrived just after both nodes moved to the next block. It can slow a round down or, on poorly connected networks, stall it.

User story

As a validator operator, I want every peer to receive the final-stage votes it still needs, so that rounds finish quickly after network hiccups.

Scenario

Base flow

Each node remembers which votes each peer already has, so it does not send the same vote twice. When both nodes finish one block and move to the next, that memory starts fresh for the new block.

Actual behavior

A message about the block the nodes just finished sometimes arrives after both have moved on. The node then wrongly recorded the peer as already holding the matching votes for the new block. There are two ways in:

• a late vote from the previous block marks one validator's new-block vote as "already sent";
• a late summary of the previous block's votes marks the new block's votes from every validator as "already sent".

Those votes were never sent to that peer. If the peer cannot get them from anyone else, it waits, and the round can stall.

Expected behavior

Messages about the previous block never change what the node thinks a peer has for the current block. Current-block votes are sent as usual.

Detailed discussion

What was done

• Root cause (internal/consensus/peer_state.go, getVoteBitArray):
  • For PRS.Height == height+1 && PRS.LastCommitRound == round && Precommit, it returned ps.PRS.Precommits, which is the peer's current-height array.
  • This dates from refactor(consensus): remove not needed commit timeout and unused LastPrecommits #751, which removed PRS.LastPrecommits and left that branch pointing at the current array.
• Every path into that branch wrote previous-height data into current-height bits:
  • reactor.go VoteChannel handler → SetHasVote for a late height-1 precommit (sets one bit);
  • reactor.go VoteSetBits handler → ApplyVoteSetBitsMessage for an old height. This overwrote the whole array with the peer's previous commit bits, for both ourVotes == nil and non-nil;
  • gossiper.go updatePeerVote → SetHasVote when our own send finishes after the peer's state has advanced.
• Consequence: PickVoteToSend subtracts those bits, so the affected current-height precommits are never offered to that peer. Vote redelivery (fix(consensus): resend same-round votes the peer never acknowledged #1599) cannot repair this, because a vote that was never sent is never tracked as unacknowledged.
• Fix:
  • getVoteBitArray returns nil ("not tracked") for any height other than PRS.Height.
  • Previous-height precommits are never gossiped one by one. pickVoteForGossip works only at rs.Height, and GossipCommit sends the whole Commit, so there is nothing to track.
  • In ApplyNewRoundStepMessage, an if/else with identical branches and a stale "Shift Precommits to LastPrecommits" comment is collapsed into a single assignment. Behavior is unchanged.

Testing

• New tests in internal/consensus/peer_state_lastcommit_test.go:

  Test	Kind
  TestSetHasVoteLastHeightPrecommitDoesNotMarkCurrentHeight	Fails without the fix; also asserts the current-height precommit is still offered by PickVoteToSend.
  TestApplyVoteSetBitsLastHeightPrecommitsDoNotMarkCurrentHeight (ourVotes nil and non-nil)	Both subtests fail without the fix.
  TestApplyNewRoundStepMessageKeepsLastCommitRound	Contract guard for the LastCommitRound carry-over.

• The neighbouring PeerState, VoteSetBits, Maj23, gossiper and reactor tests pass, and make lint reports 0 issues.

• An independent review traced every caller of getVoteBitArray and found none that needed the removed branch.

Breaking changes

None.

Prior work

• refactor(consensus): remove not needed commit timeout and unused LastPrecommits #751: introduced the branch fixed here.
• fix(consensus): resend same-round votes the peer never acknowledged #1599 (open): vote redelivery. The two PRs touch different parts of peer_state.go, so no conflict is expected. This fix also keeps fix(consensus): resend same-round votes the peer never acknowledged #1599's re-offer from resolving a stale height to the current array.

🤖 Generated with Claude Code

https://claude.ai/code/session_01WunYYgcoPDNh3uebhUCwig

_{🤖 Co-authored by Claudius the Magnificent AI Agent}

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration

• Source mode: Auto
• Repository: dashpay/tenderdash
• Branch: fix/consensus-prev-height-precommit-bit
• File: .coderabbit.yaml
• Commit: 47b9439
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: b72067ab-6650-4eb0-b4b9-e919ee018334

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• Trigger review

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

[thepastaclaw]

🕓 Review not started yet because this PR is a draft.

• Request normal review — click when the PR is ready for review.
• Request priority review — click to move this review to the front of the queue.

Commit 47b9439. Normal review starts when eligible; priority review starts as soon as a slot is available.