Avoid mutating commit bitmap during certificate comparison

[Frozen]

Problem

There is a potential order-dependent race between two valid commit certificates for the same block:

1. the node already has an older certificate in the database;
2. a newer COMMITTED message arrives with more signer slots;
3. the node compares the two certificates before deciding which one to keep.

The existing comparison reuses the mutable bitmap decoded from the incoming message. To count signers in the stored certificate, it calls SetMask on that same bitmap object. This can overwrite the active incoming bitmap with the older stored bitmap while the aggregate signature still belongs to the incoming certificate.

For example:

• stored certificate: 7/15 signer slots;
• verified incoming certificate: 11/15 signer slots;
• expected state: signature and bitmap from the 11/15 certificate;
• possible old state: signature from 11/15, but bitmap changed back to 7/15 during comparison.

This is a logical state race caused by update ordering and shared mutable state, rather than a Go race-detector data race.

Fix

This PR makes certificate selection pure and monotonic:

• compare the persisted and incoming payloads without mutating either bitmap;
• accept a replacement only when the verified incoming certificate contains strictly more real committee slots;
• keep the existing certificate when the incoming signer count is equal or lower;
• reject incompatible payload lengths and non-canonical bitmap padding;
• report database write failures instead of ignoring them.

After this change, the comparison itself cannot roll the active bitmap back to the older certificate, and the persisted certificate can only move to a more complete value.

The incoming certificate is still cryptographically and quorum-verified by the existing VerifyHeaderSignature path before this comparison runs.

Scope

This is a focused correctness fix based directly on dev. It does not change consensus quorum rules, BLS verification, wire formats, block timing, leader rotation, retry behavior, or feature activation epochs.

[Test]

• make test
• go test ./consensus -run '^TestIsMoreCompleteCommitPayload$' -count=1
• git diff --check

[greptile-apps]

Greptile Summary

The PR separates commit-certificate comparison from the active consensus mask and only persists a verified replacement when it has more signer slots.

• Adds structural validation and signer counting for commit bitmaps.
• Preserves the active decoded bitmap while comparing persisted and incoming certificates.
• Reports failures when persisting a richer certificate.
• Adds focused tests for signer counts, incompatible lengths, and padding bits.

Confidence Score: 5/5

The PR appears safe to merge within the scope of this follow-up review.

No blocking failure remains in the eligible follow-up findings.

Important Files Changed

Filename	Overview
consensus/commit_bitmap.go	Adds non-mutating structural validation and signer-slot comparison for persisted and incoming commit payloads.
consensus/commit_bitmap_test.go	Covers richer, poorer, equal, incompatible-length, and noncanonical-padding comparisons.
consensus/validator.go	Replaces active-mask mutation with the new comparison helper and surfaces richer-certificate persistence errors.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Receive COMMITTED payload] --> B[Verify signature]
    B --> C[Decode active signature and bitmap]
    C --> D[Read persisted commit certificate]
    D --> E{Both bitmaps canonical?}
    E -- No --> F[Keep persisted certificate]
    E -- Yes --> G{Candidate has more signer slots?}
    G -- No --> F
    G -- Yes --> H[Persist richer certificate]
    H --> I{Write succeeds?}
    I -- No --> J[Log warning]
    I -- Yes --> K[Continue consensus catch-up]

Loading

_{Reviews (2): Last reviewed commit: "consensus: preserve richer commit bitmap" | Re-trigger Greptile}