fix(tbtc/signer): validate incoming attempt context before clearing active round#4111
Merged
mswilkison merged 1 commit intoJun 26, 2026
Conversation
…ctive round start_sign_round cleared the active sign round on an authorized advance *before* validating the incoming attempt context against the deterministic RFC-21 coordinator selection. A malformed advance whose transition evidence was internally consistent but whose coordinator_identifier failed deterministic validation destroyed the in-memory round, then returned an error without persisting. Because the original attempt id stayed in consumed_attempt_ids, that attempt could never be re-signed in-memory, bricking the signing session until the durable (un-cleared) state was reloaded on restart. Run validate_attempt_context on the incoming context before clear_active_sign_round_for_attempt_transition, so a rejected advance leaves the active round intact. Add a regression test that forges the coordinator on an otherwise-valid advance and asserts the original attempt remains signable. This path is gated off in production by enforce_transitional_signing_disabled_in_production, so the impact is limited to the dev/staging transitional-nonce path. Co-Authored-By: Claude Opus 4.8 (1M context) <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 |
921b009
into
extraction/frost-signer-mirror-2026-05-26
20 checks passed
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
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.
Summary
Follow-up to #4005 (FROST/ROAST signer). Fixes a session-bricking bug in the attempt-advance path of
start_sign_round.On an authorized attempt advance,
start_sign_roundcleared the active sign round (clear_active_sign_round_for_attempt_transition) before validating the incoming attempt context against the deterministic RFC-21 coordinator selection. A malformed advance whose transition evidence was internally consistent but whosecoordinator_identifierfailed deterministic validation destroyed the in-memory round, then returned an error without persisting. Because the original attempt id stayed inconsumed_attempt_ids, that attempt could never be re-signed in-memory — the signing session was bricked until the durable (un-cleared) state was reloaded on restart.Fix
Run
validate_attempt_contexton the incoming context before clearing the active round, so a rejected advance leaves the round intact. The same validation already ran later on the fresh-attempt path, so no legitimate advance is newly rejected — the check is simply moved ahead of the destructive clear.Test
Adds
rejected_forged_advance_preserves_active_sign_round: it forges the coordinator on an otherwise-valid advance and asserts the original attempt remains signable. Verified to fail against the unfixed code (ConsumedAttemptReplay) and pass with the fix.Scope
Gated off in production by
enforce_transitional_signing_disabled_in_production, so impact is limited to the dev/staging transitional-nonce path.Found during review of #4005.
🤖 Generated with Claude Code