Skip to content

fix(simplex/batcher): handle weighted scheme assembly failure without stall - #4539

Open
Sertug17 wants to merge 1 commit into
commonwarexyz:mainfrom
Sertug17:fix/simplex-batcher-weighted-assembly
Open

fix(simplex/batcher): handle weighted scheme assembly failure without stall#4539
Sertug17 wants to merge 1 commit into
commonwarexyz:mainfrom
Sertug17:fix/simplex-batcher-weighted-assembly

Conversation

@Sertug17

Copy link
Copy Markdown

Closes #4409. Supersedes #4526.

Problem

try_construct_certificate panics and after #4526, stalls when Scheme::assemble returns None despite a participant-count quorum being met. This happens with weighted / threshold schemes (e.g. stake distribution [7, 1, 1, 1] with a share threshold of 8): the three minority participants verify, verified.len() >= quorum triggers try_complete(), which consumes the votes and sets state to Complete, then assembly fails because only 3 shares are available (not 8).

After that:

Fix

Certification<V>

  • Add assembly_failed: bool field
  • Loosen try_complete() guard: allow re-attempts when assembly_failed is set, even if verified.len() < self.quorum
  • Add revert_complete(votes): resets state from Complete back to Incomplete and sets assembly_failed = true
  • Update should_verify(): return true when assembly_failed && !pending.is_empty(), bypassing the participant-count quorum gate

try_construct_certificate

  • Remove all .expect(…) from assembly calls
  • Clone votes before the offload closure so they can be restored on failure
  • Call revert_complete(backup) when assembly returns None

Testing

cargo test -p commonware-consensus simplex::actors::batcher

96 passed; 0 failed

@Sertug17
Sertug17 force-pushed the fix/simplex-batcher-weighted-assembly branch from 705b731 to 1fb0266 Compare August 19, 2026 18:20
Comment thread consensus/src/simplex/actors/batcher/verifier.rs
Comment thread consensus/src/simplex/actors/batcher/verifier.rs Outdated
@Sertug17
Sertug17 force-pushed the fix/simplex-batcher-weighted-assembly branch from 1fb0266 to eb64032 Compare August 19, 2026 18:34

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit eb64032. Configure here.

Comment thread consensus/src/simplex/actors/batcher/verifier.rs
Comment thread consensus/src/simplex/actors/batcher/verifier.rs
… stall

When Scheme::assemble returns None despite a participant-count quorum
(e.g. weighted schemes where minority participants meet the count but
not the share threshold), try_construct_certificate used to panic via
.expect() — and after PR commonwarexyz#4526, silently stall because try_complete()
set state to Complete with no certificate, causing all subsequent votes
to be dropped.

Fix:
- Add assembly_failed: bool to Certification<V>
- Loosen try_complete() guard to allow re-attempts after failed assembly
- Add revert_complete(votes) that resets state from Complete back to
  Incomplete and sets assembly_failed = true
- Update should_verify() to return true when assembly_failed is set and
  pending votes exist, bypassing the participant-count quorum gate
- Clone votes before the offload closure in try_construct_certificate
  so they can be restored on failure; call revert_complete() on None

Closes commonwarexyz#4409. Supersedes commonwarexyz#4526.
@Sertug17
Sertug17 force-pushed the fix/simplex-batcher-weighted-assembly branch from eb64032 to b9fa1e5 Compare August 19, 2026 19:06
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.

simplex/batcher: certificate recovery panics on schemes where assemble legally returns None

1 participant