Skip to content

fix(bft): make GetLeadingVote deterministic on equal voting power#425

Open
amathxbt wants to merge 1 commit into
canopy-network:mainfrom
amathxbt:fix/bft-leading-vote-nondeterminism
Open

fix(bft): make GetLeadingVote deterministic on equal voting power#425
amathxbt wants to merge 1 commit into
canopy-network:mainfrom
amathxbt:fix/bft-leading-vote-nondeterminism

Conversation

@amathxbt

@amathxbt amathxbt commented Jun 25, 2026

Copy link
Copy Markdown

Bug

bft/vote.go's GetLeadingVote() selects the vote set with the most voting power using >= while iterating over a Go map:

for _, voteSet := range b.Votes[b.View.Round][...] {
    if voteSet.TotalVotedPower >= maxVotes {
        m, maxVotes = voteSet.Vote, voteSet.TotalVotedPower
    }
}

Go map iteration order is randomized. When two payload hashes have equal voting power, GetLeadingVote() can return differentleadingvotes for the
same local vote set.

This function is currently used for consensus status reporting, not for QC construction or consensus advancement, but deterministic behavior is
still preferable for consistent debugging, RPC output, and tests.

## Fix

Iterate with the payload hash key and use a deterministic tie-break. Higher voting power still wins; when voting power is equal, the
lexicographically smallest payload hash wins:

if voteSet.TotalVotedPower > maxVotes ||
    (voteSet.TotalVotedPower == maxVotes && (m == nil || payloadHash < maxPayloadHash)) {

This removes map-iteration randomness from GetLeadingVote() for identical local vote sets.

## Impact

Lowthis stabilizes leading-vote reporting in equal-power ties. It does not appear to affect consensus safety or liveness because consensus
operation uses GetMajorityVote(), not GetLeadingVote().

@andrewnguyen22

andrewnguyen22 commented Jun 27, 2026

Copy link
Copy Markdown
Collaborator

The critical severity of this finding is overstated. This is not used in operation - is decorative for the RPC only.

This is really low-priority cleanup

The PR should be downgraded and reworded. I’ll approve it - but will update the PR description

@amathxbt

amathxbt commented Jun 27, 2026

Copy link
Copy Markdown
Author

Acknowledged updated the PR description to downgrade severity from critical to low and clarify this is RPC/display-only (not used in consensus logic). Appreciate the context.

@amathxbt amathxbt requested a review from andrewnguyen22 June 28, 2026 22:20
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

Successfully merging this pull request may close these issues.

2 participants