Skip to content

Commit d857577

Browse files
mswilkisonclaude
andcommitted
fix(tbtc/signer): bind the aggregate-cleanup filter to the message too (re-review)
Codex re-review (P2, valid): the finalized-sibling cleanup added in the previous commit matched entries on (attempt_id, taproot root) but NOT the message. Because attempt_id is provided by the caller separately from the package, a mismatched aggregate - a valid signing package for message B submitted under a live message-A attempt's id (same root) - would delete the message-A seats' live nonce/commitment state, forcing that unrelated attempt to restart, even though the stored marker (now message-bound) correctly would not match its Round2. Match the FULL finalized identity in the cleanup filter: attempt_id AND hash_hex(entry.message_bytes) == aggregated_message_digest AND taproot root - the same (attempt + message + root) identity the completion marker binds. Test interactive_aggregate_cleanup_is_message_bound: a valid stateless aggregate over message B under message A's attempt id leaves the live message-A seat intact. All 297 lib tests pass; cargo fmt clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 49fdcbc commit d857577

2 files changed

Lines changed: 83 additions & 0 deletions

File tree

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -930,7 +930,13 @@ pub fn interactive_aggregate(
930930
.interactive_signing
931931
.iter()
932932
.filter(|(_, entry)| {
933+
// Match the FULL finalized identity (attempt_id + message + root), not
934+
// just (attempt_id, root): a mismatched aggregate (a valid package for a
935+
// different message submitted under this attempt id) must NOT delete the
936+
// live nonce state of the real, differently-messaged attempt - mirroring
937+
// the message binding the completion marker already enforces.
933938
entry.attempt_context.attempt_id == attempt_id
939+
&& hash_hex(&entry.message_bytes) == aggregated_message_digest
934940
&& entry.taproot_merkle_root == taproot_merkle_root
935941
})
936942
.map(|(member, _)| *member)

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

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12107,6 +12107,83 @@ fn interactive_round2_completion_marker_binds_taproot_root() {
1210712107
);
1210812108
}
1210912109

12110+
#[test]
12111+
fn interactive_aggregate_cleanup_is_message_bound() {
12112+
// The aggregate's finalized-sibling cleanup matches the full identity
12113+
// (attempt_id + message + root). A mismatched aggregate - a VALID package for a
12114+
// DIFFERENT message submitted under a live attempt's id - must NOT delete that
12115+
// attempt's live nonce state (its message differs), mirroring the completion
12116+
// marker's message binding.
12117+
let _guard = lock_test_state();
12118+
reset_for_tests();
12119+
12120+
let key_packages = interactive_test_key_packages();
12121+
let session_id = "interactive-cleanup-message-binding";
12122+
let key_group = "interactive-test-key-group";
12123+
let message_a = [0x88u8; 32];
12124+
let message_b = [0x99u8; 32];
12125+
let included = [1u16, 2];
12126+
12127+
// A live interactive attempt over message A (attempt_id derives from message A).
12128+
let opened = open_interactive_for_test(session_id, key_group, &message_a, &included, 1, 1, 2)
12129+
.expect("member 1 opens message-A attempt");
12130+
interactive_round1(InteractiveRound1Request {
12131+
session_id: session_id.to_string(),
12132+
attempt_id: opened.attempt_id.clone(),
12133+
member_identifier: 1,
12134+
})
12135+
.expect("member 1 round 1 (message A)");
12136+
12137+
// A valid aggregate over a DIFFERENT message B, submitted under message A's
12138+
// attempt id - via stateless shares, so it does not touch the live attempt.
12139+
let m1_b = generate_nonces_and_commitments(GenerateNoncesAndCommitmentsRequest {
12140+
key_package_identifier: key_packages[&1].identifier.clone(),
12141+
key_package_hex: key_packages[&1].data_hex.clone(),
12142+
})
12143+
.expect("stateless nonces 1");
12144+
let m2_b = generate_nonces_and_commitments(GenerateNoncesAndCommitmentsRequest {
12145+
key_package_identifier: key_packages[&2].identifier.clone(),
12146+
key_package_hex: key_packages[&2].data_hex.clone(),
12147+
})
12148+
.expect("stateless nonces 2");
12149+
let package_b = interactive_package_for_test(
12150+
&message_b,
12151+
vec![m1_b.commitment.clone(), m2_b.commitment.clone()],
12152+
);
12153+
let share1_b = sign_share(SignShareRequest {
12154+
signing_package_hex: package_b.clone(),
12155+
nonces_hex: m1_b.nonces_hex,
12156+
key_package_identifier: key_packages[&1].identifier.clone(),
12157+
key_package_hex: key_packages[&1].data_hex.clone(),
12158+
})
12159+
.expect("stateless share 1 over B");
12160+
let share2_b = sign_share(SignShareRequest {
12161+
signing_package_hex: package_b.clone(),
12162+
nonces_hex: m2_b.nonces_hex,
12163+
key_package_identifier: key_packages[&2].identifier.clone(),
12164+
key_package_hex: key_packages[&2].data_hex.clone(),
12165+
})
12166+
.expect("stateless share 2 over B");
12167+
interactive_aggregate(InteractiveAggregateRequest {
12168+
session_id: session_id.to_string(),
12169+
attempt_id: opened.attempt_id.clone(),
12170+
signing_package_hex: package_b,
12171+
signature_shares: vec![share1_b.signature_share, share2_b.signature_share],
12172+
taproot_merkle_root_hex: None,
12173+
})
12174+
.expect("aggregate over message B succeeds under message A's attempt id");
12175+
12176+
// The live message-A seat must survive: the cleanup is message-bound.
12177+
{
12178+
let guard = state().expect("state").lock().expect("lock");
12179+
let session = guard.sessions.get(session_id).expect("session exists");
12180+
assert!(
12181+
session.interactive_signing.contains_key(&1),
12182+
"a mismatched-message aggregate must not delete the live message-A seat"
12183+
);
12184+
}
12185+
}
12186+
1211012187
#[test]
1211112188
fn interactive_round1_is_idempotent_until_consumed() {
1211212189
let _guard = lock_test_state();

0 commit comments

Comments
 (0)