Skip to content

Commit 49fdcbc

Browse files
mswilkisonclaude
andcommitted
fix(tbtc/signer): free finalized non-signing siblings on aggregate (re-review)
Codex re-review (P2, valid): when a multi-seat attempt aggregates with a threshold subset that EXCLUDES a local member which opened/Round1'd the same attempt + root, that member never calls Round2 (it is not a signer), so the Round2 completion gate never runs for it - its interactive_signing entry (nonces, key, message) and its live-member capacity slot stayed resident until the 1h TTL or an explicit abort. interactive_aggregate's success path now frees the LOCAL siblings finalized by the completion: after persisting the marker, remove + zeroize every interactive_signing entry on (attempt_id, taproot root) - the signers' entries were already removed at their Round2, and a sibling on a DIFFERENT root is a distinct signing task and is left untouched. The Round2 completion gate stays as the defense for a sibling that RE-OPENS the finalized attempt and tries to sign. Test interactive_round2_refused_after_aggregate_for_unsigned_sibling now asserts the non-signing sibling is freed at aggregation, then re-opens it and confirms the gate still refuses its Round2. All 296 lib tests pass; cargo fmt clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 73fabaf commit 49fdcbc

2 files changed

Lines changed: 50 additions & 5 deletions

File tree

pkg/tbtc/signer/src/engine/interactive.rs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -915,6 +915,31 @@ pub fn interactive_aggregate(
915915
.remove(&aggregated_marker);
916916
return Err(persist_error);
917917
}
918+
919+
// The attempt is now final for (attempt_id, message, root). A LOCAL sibling seat
920+
// that opened/Round1'd this same attempt + root but is NOT in the signing subset
921+
// never calls Round2, so free those entries now - zeroizing their nonces and
922+
// returning their live-member slots - rather than leaving them resident until the
923+
// TTL sweep. The signers' own entries were already removed at their Round2; a
924+
// sibling on a DIFFERENT root is a distinct signing task and is left untouched.
925+
let session = guard
926+
.sessions
927+
.get_mut(&request.session_id)
928+
.expect("session existed under the held engine lock");
929+
let finalized_members: Vec<u16> = session
930+
.interactive_signing
931+
.iter()
932+
.filter(|(_, entry)| {
933+
entry.attempt_context.attempt_id == attempt_id
934+
&& entry.taproot_merkle_root == taproot_merkle_root
935+
})
936+
.map(|(member, _)| *member)
937+
.collect();
938+
for member in &finalized_members {
939+
if let Some(mut removed) = session.interactive_signing.remove(member) {
940+
zeroize_interactive_round1(&mut removed);
941+
}
942+
}
918943
drop(guard);
919944

920945
record_hardening_telemetry(|telemetry| {

pkg/tbtc/signer/src/engine/tests.rs

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11755,7 +11755,8 @@ fn interactive_round2_refused_after_aggregate_for_unsigned_sibling() {
1175511755
member_identifier: 2,
1175611756
})
1175711757
.expect("member 2 round 1");
11758-
let c3 = interactive_round1(InteractiveRound1Request {
11758+
// Seat 3 opens + Round1s the same attempt but is NOT in the {1,2} signing subset.
11759+
interactive_round1(InteractiveRound1Request {
1175911760
session_id: session_id.to_string(),
1176011761
attempt_id: opened.attempt_id.clone(),
1176111762
member_identifier: 3,
@@ -11829,9 +11830,28 @@ fn interactive_round2_refused_after_aggregate_for_unsigned_sibling() {
1182911830
);
1183011831
}
1183111832

11832-
// Seat 3 (open, round1'd, never signed) tries to release a share for the now
11833-
// completed attempt, in a subset it WOULD be valid for ({1,3}). Without the
11834-
// completion gate this releases a fresh share; with it the attempt is final.
11833+
// Aggregation proactively frees the LOCAL non-signing sibling (seat 3): it never
11834+
// calls Round2, so its entry must not linger to the TTL sweep.
11835+
{
11836+
let guard = state().expect("state").lock().expect("lock");
11837+
let session = guard.sessions.get(session_id).expect("session exists");
11838+
assert!(
11839+
!session.interactive_signing.contains_key(&3),
11840+
"the non-signing sibling is freed when the attempt aggregates"
11841+
);
11842+
}
11843+
11844+
// If seat 3 RE-OPENS the finalized attempt and tries to release a share (in a
11845+
// {1,3} subset it would otherwise be valid for), the completion gate refuses it:
11846+
// the bound marker makes the attempt/message/root final.
11847+
open_interactive_for_test(session_id, key_group, &message, &included, 1, 3, 2)
11848+
.expect("seat 3 re-opens the finalized attempt");
11849+
let c3_reopened = interactive_round1(InteractiveRound1Request {
11850+
session_id: session_id.to_string(),
11851+
attempt_id: opened.attempt_id.clone(),
11852+
member_identifier: 3,
11853+
})
11854+
.expect("seat 3 round 1 after re-open");
1183511855
let package_13 = interactive_package_for_test(
1183611856
&message,
1183711857
vec![
@@ -11841,7 +11861,7 @@ fn interactive_round2_refused_after_aggregate_for_unsigned_sibling() {
1184111861
},
1184211862
NativeFrostCommitment {
1184311863
identifier: key_packages[&3].identifier.clone(),
11844-
data_hex: c3.commitments_hex.clone(),
11864+
data_hex: c3_reopened.commitments_hex.clone(),
1184511865
},
1184611866
],
1184711867
);

0 commit comments

Comments
 (0)