test(frost): fault-injection coverage for the ROAST blame bridge (7.3 PR2b-2 step 2c)#4090
Merged
mswilkison merged 1 commit intoJun 19, 2026
Conversation
… PR2b-2 step 2c) Closes Step 2's test coverage with two integration fault-injection tests the 2a/2b e2e tests left to direct stash-seeding: - TestDriveInteractiveRoastSigning_StashesCoordinatorProofsOnFailure exercises the 2b extraction seam end-to-end through the REAL drive + runner + collector: a committed interactive attempt that fails at aggregation (after recording the coordinator package) makes driveInteractiveRoastSigningIfEnabled surface collector.CoordinatorPackageProofs and stash them under the attempt key. A 1-of-1 attempt retains exactly its own authoritative package, so one proof is stashed -- proving the seam fires through real code, not a seeded stash. - TestRoastTransitionExchange_CombinedFaultsExcludeBoth is the 2a+2b coexistence capstone: one bundle carrying BOTH f+1 reject evidence and coordinator-equivocation proofs (with snapshots that carry evidence AND a proof at once) excludes BOTH the rejected member and the equivocating coordinator in a single NextAttempt, leaving a feasible threshold-sized included set. Test-only; no production change. Policy (categories/soak/equivocation) and runner capture (RetainsCoordinatorPackageEquivocation etc.) were already covered, so this fills only the capture->stash->bundle->exclusion integration gap. Verified across 5 tag combos + -race + gofmt. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
mswilkison
merged commit 33fee4b
into
feat/frost-schnorr-migration-scaffold
15 of 16 checks passed
mswilkison
added a commit
that referenced
this pull request
Jun 19, 2026
…e (7.3 share-blame) (#4091) ## RFC-21 Phase 7.3 — interactive share-verification blame (the third fault source) Follows PR2b-2 (the blame layer: 2a #4088 coarse f+1 evidence, 2b #4089 coordinator-equivocation proofs, 2c #4090 tests). This closes the **last** blame gap. ### The gap The blame layer excluded members for coarse faults and coordinator equivocation, but an **interactive bad-share submitter escaped blame entirely**. The engine's `InteractiveAggregate` surfaces candidate culprits in a typed `InteractiveAggregateShareVerificationError{CandidateCulprits []uint16}`, yet the runner just wrapped and returned the error, **dropping them**. `EngineRound2ShareVerifier` and `Round2Collector.ClassifyCandidateCulprits` were both complete but constructed/called **nowhere** — so a member that submitted an invalid FROST signature share was never excluded and the retry could loop. ### The wiring On a `runner.Run` share-verification failure, `driveInteractiveRoastSigningIfEnabled` (where 2b already extracts proofs): 1. `errors.As` the typed `*InteractiveAggregateShareVerificationError` (through the runner's `%w` wrap); 2. converts the engine's `uint16` candidates → `MemberIndex`, **dropping `0` / out-of-range** so a malformed candidate can't truncate into an honest seat; 3. type-asserts the engine to `Round2ShareVerifyingEngine` (skip if absent — still stash 2b proofs); 4. builds an `EngineRound2ShareVerifier` bound to the attempt (`SessionID == active.SessionID()`, `AttemptContextHash`, `TaprootMerkleRoot`); 5. `ClassifyCandidateCulprits` re-verifies each candidate's **retained** share (frozen-Q1 boundary: only an **ACCEPTED** retained share that re-verifies **INVALID** is blamed; every not-the-member's-fault condition fails closed); 6. stashes the reject accusations in the **same union pending-evidence entry** as the 2b proofs. `BroadcastForcedSnapshot` carries them; `computeNextAttempt`'s f+1 reject gate excludes an f+1-corroborated bad-share member. ### Why f+1 (not instant) A share is self-incriminating only against the package **this observer accepted** — a byzantine coordinator's targeted split could otherwise make one honest observer instant-exclude an honest peer. f+1 corroboration (honest observers re-verify identical retained bytes deterministically) closes that. Only unforgeable coordinator-equivocation proofs (2b) are instant. ### Safety Best-effort + fail-safe: a non-share error, a verifier-incapable engine, malformed candidates, or an empty classification all stash nothing. `EngineRound2ShareVerifier` already fails closed against false blame (bound-attempt / root / submitter / package-body checks). Compile-time assertion that the cgo engine satisfies `Round2ShareVerifyingEngine`. Stale "evidence/proofs mutually exclusive" comments corrected (one interactive failure now carries both). ### Scope / gating Prod-dormant until the cgo interactive engine is registered (gated on the `frost-secp256k1-tr` audit). ### Verification - build + vet across 5 tag combos (default / `frost_native` / `frost_roast_retry` / `frost_native frost_roast_retry` / `frost_native frost_tbtc_signer` w/ CGO) - tests: blame-fires (engine-verified-invalid share → reject through the real drive+runner+collector+verifier), skip-without-verifier, malformed-candidates-dropped; `-race`; gofmt clean - design locked via Codex + Gemini consult (both PROCEED, no holes) 🤖 Generated with [Claude Code](https://claude.com/claude-code)
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
1 participant
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.
RFC-21 Phase 7.3 PR2b-2 step 2c — fault-injection tests
Closes Step 2's test coverage (after 2a #4088 and 2b #4089). Two integration fault-injection tests that the 2a/2b e2e tests left to direct stash-seeding:
TestDriveInteractiveRoastSigning_StashesCoordinatorProofsOnFailureExercises the 2b extraction seam end-to-end through the real drive + runner + collector: a committed interactive attempt that fails at aggregation (after the runner records the coordinator package) makes
driveInteractiveRoastSigningIfEnabledsurfacecollector.CoordinatorPackageProofsand stash them under the attempt key. A 1-of-1 attempt retains exactly its own authoritative package, so one proof is stashed — proving the seam fires through real code, not a directly-seeded stash.TestRoastTransitionExchange_CombinedFaultsExcludeBothThe 2a+2b coexistence capstone: one transition bundle carrying both f+1 reject evidence (against a bad signer) and coordinator-equivocation proofs (two distinct authentic bodies) — including snapshots that carry evidence and a proof at once — excludes both the rejected member and the equivocating coordinator in a single
NextAttempt, leaving a feasible threshold-sized included set.Scope
Test-only; no production change. The policy layer (categories / soak: overflow-park / silence / reinstatement / infeasibility / equivocation) and runner capture (
RetainsCoordinatorPackageEquivocation,PreservesEvidenceOnAggregateFailure, …) were already covered, so this fills only the remainingcapture → stash → bundle → exclusionintegration gap.Verification
frost_native/frost_roast_retry/frost_native frost_roast_retry/frost_native frost_tbtc_signerw/ CGO) — no prod change, trivially green-raceonpkg/frost/signing; vet + gofmt clean🤖 Generated with Claude Code