From 09061d6a5eadb7cca4c7afdf987115e212253e51 Mon Sep 17 00:00:00 2001 From: Louis Pahlavi Date: Fri, 10 Apr 2026 20:29:24 +0200 Subject: [PATCH 1/4] fix: fetch current slot before checking transaction statuses (TOCTOU) Snapshot the current slot before calling getSignatureStatuses so that a transaction which finalizes between the slot snapshot and the status check is seen as finalized (not missing), preventing it from being incorrectly marked as expired. Co-Authored-By: Claude Sonnet 4.6 --- integration_tests/src/fixtures.rs | 12 ++++++--- integration_tests/tests/tests.rs | 8 +++--- minter/src/monitor/mod.rs | 19 ++++++++------ minter/src/monitor/tests.rs | 41 ++++++++++++++----------------- 4 files changed, 43 insertions(+), 37 deletions(-) diff --git a/integration_tests/src/fixtures.rs b/integration_tests/src/fixtures.rs index dc105e3b..62840e2b 100644 --- a/integration_tests/src/fixtures.rs +++ b/integration_tests/src/fixtures.rs @@ -173,12 +173,18 @@ impl MockBuilder { ) } + /// Mocks for `getSlot` → `getBlock`, used by the monitor timer to snapshot the current slot. + pub fn get_current_slot(self, slot: u64, blockhash: &str) -> Self { + self.expect(get_slot_request(), get_slot_response(slot)) + .expect(get_block_request(slot), get_block_response(blockhash)) + } + /// Mocks for resubmitting an expired transaction: - /// `getSignatureStatuses`(not found) → `getSlot` → `getBlock` → `getSlot` → `getBlock` → `sendTransaction`. + /// `getSlot` → `getBlock` → `getSignatureStatuses`(not found) → `getSlot` → `getBlock` → `sendTransaction`. pub fn resubmit_transaction(self, slot: u64, blockhash: &str, tx_signature: &str) -> Self { - self.check_signature_statuses_not_found(1) - .expect(get_slot_request(), get_slot_response(slot)) + self.expect(get_slot_request(), get_slot_response(slot)) .expect(get_block_request(slot), get_block_response(blockhash)) + .check_signature_statuses_not_found(1) .expect(get_slot_request(), get_slot_response(slot)) .expect(get_block_request(slot), get_block_response(blockhash)) .expect( diff --git a/integration_tests/tests/tests.rs b/integration_tests/tests/tests.rs index efe91ff2..59866fa7 100644 --- a/integration_tests/tests/tests.rs +++ b/integration_tests/tests/tests.rs @@ -57,6 +57,7 @@ async fn deposit_and_consolidate_funds(setup: &Setup) { setup .execute_http_mocks( MockBuilder::with_start_id(16) + .get_current_slot(100_000_000, "4sGjMW1sUnHzSxGspuhpqLDx6wiyjNtZAMdL4VZHirAn") .check_signature_statuses_finalized(1) .build(), ) @@ -711,7 +712,7 @@ mod withdrawal_tests { } fn estimate_blockhash_http_mocks(slot: u64) -> MockHttpOutcalls { - MockBuilder::with_start_id(20) + MockBuilder::with_start_id(28) .submit_transaction( slot, "4sGjMW1sUnHzSxGspuhpqLDx6wiyjNtZAMdL4VZHirAn", @@ -722,7 +723,7 @@ mod withdrawal_tests { /// HTTP mocks for resubmitting an expired withdrawal transaction. fn resubmit_withdrawal_http_mocks(current_slot: u64) -> MockHttpOutcalls { - MockBuilder::with_start_id(32) + MockBuilder::with_start_id(40) .resubmit_transaction( current_slot, "9ZNTfG4NyQgxy2SWjSiQoUyBPEvXT2xo7fKc5hPYYJ7b", @@ -733,7 +734,8 @@ mod withdrawal_tests { /// HTTP mocks for finalizing a withdrawal transaction. fn finalize_withdrawal_http_mocks() -> MockHttpOutcalls { - MockBuilder::with_start_id(56) + MockBuilder::with_start_id(64) + .get_current_slot(350_000_200, "9ZNTfG4NyQgxy2SWjSiQoUyBPEvXT2xo7fKc5hPYYJ7b") .check_signature_statuses_finalized(1) .build() } diff --git a/minter/src/monitor/mod.rs b/minter/src/monitor/mod.rs index 19b5392f..bf525bb8 100644 --- a/minter/src/monitor/mod.rs +++ b/minter/src/monitor/mod.rs @@ -54,6 +54,17 @@ pub async fn monitor_submitted_transactions(runtime: R) { return; } + // Fetch the current slot before checking statuses: if a transaction finalizes + // after we snapshot the slot, the status check will see it as finalized rather + // than missing, so it will never be incorrectly marked as expired. + let (current_slot, _) = match get_recent_slot_and_blockhash(&runtime).await { + Ok(result) => result, + Err(e) => { + log!(Priority::Info, "Failed to get current slot: {e}"); + return; + } + }; + let statuses = check_transaction_statuses(&runtime, &all_transactions).await; for (signature, error) in &statuses.errored { @@ -89,14 +100,6 @@ pub async fn monitor_submitted_transactions(runtime: R) { return; } - let (current_slot, _) = match get_recent_slot_and_blockhash(&runtime).await { - Ok(result) => result, - Err(e) => { - log!(Priority::Info, "Failed to get current slot: {e}"); - return; - } - }; - let expired_signatures: BTreeSet = all_transactions .into_iter() .filter(|(sig, slot)| { diff --git a/minter/src/monitor/tests.rs b/minter/src/monitor/tests.rs index b12e9219..d0561e51 100644 --- a/minter/src/monitor/tests.rs +++ b/minter/src/monitor/tests.rs @@ -60,8 +60,6 @@ async fn should_return_early_if_fetching_current_slot_fails() { let events_before = EventsAssert::from_recorded(); let error = SlotResult::Consistent(Err(RpcError::ValidationError("Error".to_string()))); - // We try to fetch the current slot 3 times before giving up, - // and then no more RPC calls are made let runtime = TestCanisterRuntime::new() .add_stub_response(error.clone()) .add_stub_response(error.clone()) @@ -84,6 +82,8 @@ mod finalization { let runtime = TestCanisterRuntime::new() .with_increasing_time() + .add_stub_response(SlotResult::Consistent(Ok(CURRENT_SLOT))) + .add_stub_response(BlockResult::Consistent(Ok(confirmed_block()))) .add_stub_response(SignatureStatusesResult::Consistent(Ok(vec![Some( finalized_status(), )]))); @@ -120,18 +120,15 @@ mod finalization { submit_consolidation_transaction(slot); - let mut runtime = TestCanisterRuntime::new() + let runtime = TestCanisterRuntime::new() .with_increasing_time() + .add_stub_response(SlotResult::Consistent(Ok(CURRENT_SLOT))) + .add_stub_response(BlockResult::Consistent(Ok(confirmed_block()))) .add_stub_response(SignatureStatusesResult::Consistent(Ok(vec![ status.clone(), ]))); - // If the transaction status is null, we also check if the transaction slot is expired - if status.is_none() { - runtime = runtime - .add_stub_response(SlotResult::Consistent(Ok(CURRENT_SLOT))) - .add_stub_response(BlockResult::Consistent(Ok(confirmed_block()))); - } + let _ = status; // suppress unused warning let events_before = EventsAssert::from_recorded(); @@ -151,6 +148,8 @@ mod finalization { let runtime = TestCanisterRuntime::new() .with_increasing_time() + .add_stub_response(SlotResult::Consistent(Ok(CURRENT_SLOT))) + .add_stub_response(BlockResult::Consistent(Ok(confirmed_block()))) .add_stub_response(SignatureStatusesResult::Consistent(Ok(vec![Some( TransactionStatus { slot: RECENT_SLOT, @@ -185,14 +184,14 @@ mod finalization { let runtime = TestCanisterRuntime::new() .with_increasing_time() + .add_stub_response(SlotResult::Consistent(Ok(CURRENT_SLOT))) + .add_stub_response(BlockResult::Consistent(Ok(confirmed_block()))) .add_stub_response(SignatureStatusesResult::Consistent(Ok(vec![ Some(finalized_status()), None, Some(finalized_status()), - ]))) - // get_recent_slot_and_blockhash for the one not_found transaction (getSlot + getBlock) - .add_stub_response(SlotResult::Consistent(Ok(CURRENT_SLOT))) - .add_stub_response(BlockResult::Consistent(Ok(confirmed_block()))); + ]))); + // sig_b is not_found but RECENT_SLOT is not expired, so no resubmission. monitor_submitted_transactions(runtime).await; @@ -251,12 +250,9 @@ mod resubmission { let runtime = TestCanisterRuntime::new() .with_increasing_time() - // getSignatureStatuses: not found - .add_stub_response(SignatureStatusesResult::Consistent(Ok(vec![None]))) - // get_recent_slot_and_blockhash for expiry check (getSlot + getBlock) .add_stub_response(SlotResult::Consistent(Ok(CURRENT_SLOT))) .add_stub_response(BlockResult::Consistent(Ok(confirmed_block()))) - // get_recent_slot_and_blockhash for resubmission (getSlot + getBlock) + .add_stub_response(SignatureStatusesResult::Consistent(Ok(vec![None]))) .add_stub_response(SlotResult::Consistent(Ok(RESUBMISSION_SLOT))) .add_stub_response(BlockResult::Consistent(Ok(confirmed_block()))) .add_stub_response(SendTransactionResult::Consistent(Ok(new_signature.into()))) @@ -286,6 +282,8 @@ mod resubmission { let runtime = TestCanisterRuntime::new() .with_increasing_time() + .add_stub_response(SlotResult::Consistent(Ok(CURRENT_SLOT))) + .add_stub_response(BlockResult::Consistent(Ok(confirmed_block()))) .add_stub_response(SignatureStatusesResult::Consistent(Err( RpcError::ValidationError("Error".to_string()), ))); @@ -308,11 +306,9 @@ mod resubmission { let runtime = TestCanisterRuntime::new() .with_increasing_time() - .add_stub_response(SignatureStatusesResult::Consistent(Ok(vec![None]))) - // get_recent_slot_and_blockhash for expiry check (getSlot + getBlock) .add_stub_response(SlotResult::Consistent(Ok(CURRENT_SLOT))) .add_stub_response(BlockResult::Consistent(Ok(confirmed_block()))) - // get_recent_slot_and_blockhash for resubmission (getSlot + getBlock) + .add_stub_response(SignatureStatusesResult::Consistent(Ok(vec![None]))) .add_stub_response(SlotResult::Consistent(Ok(RESUBMISSION_SLOT))) .add_stub_response(BlockResult::Consistent(Ok(confirmed_block()))) .add_stub_response(SendTransactionResult::Inconsistent(vec![])) @@ -340,13 +336,12 @@ mod resubmission { let mut runtime = TestCanisterRuntime::new() .with_increasing_time() + .add_stub_response(SlotResult::Consistent(Ok(CURRENT_SLOT))) + .add_stub_response(BlockResult::Consistent(Ok(confirmed_block()))) // getSignatureStatuses: all not found .add_stub_response(SignatureStatusesResult::Consistent(Ok( vec![None; num_transactions], ))) - // get_recent_slot_and_blockhash for expiry check (getSlot + getBlock) - .add_stub_response(SlotResult::Consistent(Ok(CURRENT_SLOT))) - .add_stub_response(BlockResult::Consistent(Ok(confirmed_block()))) // Round 1: get_recent_slot_and_blockhash for resubmission (getSlot + getBlock) .add_stub_response(SlotResult::Consistent(Ok(RESUBMISSION_SLOT))) .add_stub_response(BlockResult::Consistent(Ok(confirmed_block()))); From 7d472031b473490e16c0cb01e3d85ec186c2fa77 Mon Sep 17 00:00:00 2001 From: Louis Pahlavi Date: Fri, 10 Apr 2026 20:32:11 +0200 Subject: [PATCH 2/4] feat: add ExpiredTransaction event and resubmission queue Introduces EventType::ExpiredTransaction fired when a submitted transaction's blockhash expires without being finalized. Adds a transactions_to_resubmit BTreeSet to state so that expired transactions are queued for resubmission and survived across upgrades. Co-Authored-By: Claude Sonnet 4.6 --- libs/types-internal/src/event.rs | 7 +++++++ minter/cksol_minter.did | 7 +++++++ minter/src/main.rs | 3 +++ minter/src/monitor/mod.rs | 14 ++++++++++++-- minter/src/monitor/tests.rs | 31 ++++++++++++++++++------------- minter/src/state/audit.rs | 3 +++ minter/src/state/event.rs | 9 +++++++++ minter/src/state/mod.rs | 28 ++++++++++++++++++++++++++++ minter/src/state/tests.rs | 1 + minter/src/test_fixtures/mod.rs | 1 + 10 files changed, 89 insertions(+), 15 deletions(-) diff --git a/libs/types-internal/src/event.rs b/libs/types-internal/src/event.rs index 5fb1eee6..b24c0261 100644 --- a/libs/types-internal/src/event.rs +++ b/libs/types-internal/src/event.rs @@ -105,6 +105,13 @@ pub enum EventType { /// The signature of the failed Solana transaction. signature: Signature, }, + /// A previously submitted Solana transaction has an expired blockhash + /// and a null on-chain status, meaning it will never be executed. + /// The transaction has been marked for resubmission. + ExpiredTransaction { + /// The signature of the expired Solana transaction. + signature: Signature, + }, } /// The purpose of a submitted Solana transaction. diff --git a/minter/cksol_minter.did b/minter/cksol_minter.did index 8a7fb080..1f9c876c 100644 --- a/minter/cksol_minter.did +++ b/minter/cksol_minter.did @@ -347,6 +347,13 @@ type EventType = variant { // The signature of the failed Solana transaction. signature: Signature; }; + // A previously submitted Solana transaction has an expired blockhash + // and a null on-chain status, meaning it will never be executed. + // The transaction has been marked for resubmission. + ExpiredTransaction : record { + // The signature of the expired Solana transaction. + signature: Signature; + }; }; // A single transaction can deposit to multiple accounts, so the signature alone diff --git a/minter/src/main.rs b/minter/src/main.rs index 5019a41a..c91bffcd 100644 --- a/minter/src/main.rs +++ b/minter/src/main.rs @@ -184,6 +184,9 @@ fn get_events( EventType::FailedTransaction { signature } => event::EventType::FailedTransaction { signature: signature.into(), }, + EventType::ExpiredTransaction { signature } => event::EventType::ExpiredTransaction { + signature: signature.into(), + }, } } diff --git a/minter/src/monitor/mod.rs b/minter/src/monitor/mod.rs index bf525bb8..7f988838 100644 --- a/minter/src/monitor/mod.rs +++ b/minter/src/monitor/mod.rs @@ -100,7 +100,7 @@ pub async fn monitor_submitted_transactions(runtime: R) { return; } - let expired_signatures: BTreeSet = all_transactions + let expired_signatures: Vec = all_transactions .into_iter() .filter(|(sig, slot)| { statuses.not_found.contains(sig) && slot + MAX_BLOCKHASH_AGE < current_slot @@ -112,8 +112,18 @@ pub async fn monitor_submitted_transactions(runtime: R) { return; } + for signature in expired_signatures { + mutate_state(|state| { + // Skip if the transaction was finalized concurrently. + if state.submitted_transactions().contains_key(&signature) { + process_event(state, EventType::ExpiredTransaction { signature }, &runtime); + } + }); + } + let to_resubmit: Vec<_> = read_state(|state| { - expired_signatures + state + .transactions_to_resubmit() .iter() .filter_map(|sig| { state diff --git a/minter/src/monitor/tests.rs b/minter/src/monitor/tests.rs index d0561e51..f2d16c02 100644 --- a/minter/src/monitor/tests.rs +++ b/minter/src/monitor/tests.rs @@ -260,11 +260,15 @@ mod resubmission { monitor_submitted_transactions(runtime).await; - EventsAssert::from_recorded().expect_contains_event_eq(EventType::ResubmittedTransaction { - old_signature, - new_signature, - new_slot: RESUBMISSION_SLOT, - }); + EventsAssert::from_recorded() + .expect_contains_event_eq(EventType::ExpiredTransaction { + signature: old_signature, + }) + .expect_contains_event_eq(EventType::ResubmittedTransaction { + old_signature, + new_signature, + new_slot: RESUBMISSION_SLOT, + }); read_state(|s| { assert_eq!(s.submitted_transactions().len(), 1); @@ -316,11 +320,15 @@ mod resubmission { monitor_submitted_transactions(runtime).await; - EventsAssert::from_recorded().expect_contains_event_eq(EventType::ResubmittedTransaction { - old_signature, - new_signature, - new_slot: RESUBMISSION_SLOT, - }); + EventsAssert::from_recorded() + .expect_contains_event_eq(EventType::ExpiredTransaction { + signature: old_signature, + }) + .expect_contains_event_eq(EventType::ResubmittedTransaction { + old_signature, + new_signature, + new_slot: RESUBMISSION_SLOT, + }); } #[tokio::test] @@ -338,11 +346,9 @@ mod resubmission { .with_increasing_time() .add_stub_response(SlotResult::Consistent(Ok(CURRENT_SLOT))) .add_stub_response(BlockResult::Consistent(Ok(confirmed_block()))) - // getSignatureStatuses: all not found .add_stub_response(SignatureStatusesResult::Consistent(Ok( vec![None; num_transactions], ))) - // Round 1: get_recent_slot_and_blockhash for resubmission (getSlot + getBlock) .add_stub_response(SlotResult::Consistent(Ok(RESUBMISSION_SLOT))) .add_stub_response(BlockResult::Consistent(Ok(confirmed_block()))); @@ -355,7 +361,6 @@ mod resubmission { .add_signature([0xA0 + i as u8; 64]); } - // Round 2: get_recent_slot_and_blockhash for resubmission (getSlot + getBlock) runtime = runtime .add_stub_response(SlotResult::Consistent(Ok(RESUBMISSION_SLOT))) .add_stub_response(BlockResult::Consistent(Ok(confirmed_block()))); diff --git a/minter/src/state/audit.rs b/minter/src/state/audit.rs index 6fdeaaec..7e48442e 100644 --- a/minter/src/state/audit.rs +++ b/minter/src/state/audit.rs @@ -63,6 +63,9 @@ fn apply_state_transition(state: &mut State, payload: &EventType, timestamp: u64 EventType::FailedTransaction { signature } => { state.process_transaction_failed(signature); } + EventType::ExpiredTransaction { signature } => { + state.process_transaction_expired(*signature); + } } } diff --git a/minter/src/state/event.rs b/minter/src/state/event.rs index 9a915fff..6de289ee 100644 --- a/minter/src/state/event.rs +++ b/minter/src/state/event.rs @@ -120,6 +120,15 @@ pub enum EventType { #[cbor(n(0), with = "cbor::signature")] signature: Signature, }, + /// A previously submitted Solana transaction has an expired blockhash + /// and a null on-chain status, meaning it will never be executed. + /// The transaction has been marked for resubmission. + #[n(10)] + ExpiredTransaction { + /// The signature of the expired Solana transaction. + #[cbor(n(0), with = "cbor::signature")] + signature: Signature, + }, } /// Payload of the `AcceptedWithdrawalRequest` event. diff --git a/minter/src/state/mod.rs b/minter/src/state/mod.rs index fa00a32f..6bf61e39 100644 --- a/minter/src/state/mod.rs +++ b/minter/src/state/mod.rs @@ -102,6 +102,7 @@ pub struct State { failed_withdrawal_requests: BTreeMap, deposits_to_consolidate: BTreeMap, submitted_transactions: BTreeMap, + transactions_to_resubmit: BTreeSet, succeeded_transactions: BTreeSet, failed_transactions: BTreeMap, consolidation_transactions: BTreeMap, @@ -201,6 +202,29 @@ impl State { &self.submitted_transactions } + pub fn transactions_to_resubmit(&self) -> &BTreeSet { + &self.transactions_to_resubmit + } + + pub fn process_transaction_expired(&mut self, signature: Signature) { + assert!( + !self.succeeded_transactions.contains(&signature), + "BUG: cannot mark already succeeded transaction {signature} for resubmission" + ); + assert!( + !self.failed_transactions.contains_key(&signature), + "BUG: cannot mark already failed transaction {signature} for resubmission" + ); + assert!( + self.submitted_transactions.contains_key(&signature), + "BUG: cannot mark non-submitted transaction {signature} for resubmission" + ); + assert!( + self.transactions_to_resubmit.insert(signature), + "BUG: transaction {signature} is already queued for resubmission" + ); + } + pub fn succeeded_transactions(&self) -> &BTreeSet { &self.succeeded_transactions } @@ -591,6 +615,7 @@ impl State { None, "Attempted to resubmit transaction with signature {new_signature:?} that already exists" ); + self.transactions_to_resubmit.remove(old_signature); if let Some(info) = self.consolidation_transactions.remove(old_signature) { self.consolidation_transactions.insert(*new_signature, info); } @@ -622,6 +647,7 @@ impl State { .checked_sub(tx_fee) .expect("BUG: consolidation amount is less than transaction fee"); } + self.transactions_to_resubmit.remove(signature); assert!( self.succeeded_transactions.insert(*signature), "Attempted to mark transaction {signature:?} as succeeded twice" @@ -644,6 +670,7 @@ impl State { .unwrap_or_else(|| { panic!("Attempted to mark unknown transaction {signature:?} as failed") }); + self.transactions_to_resubmit.remove(signature); assert_eq!( self.failed_transactions.insert(*signature, transaction), None, @@ -711,6 +738,7 @@ impl TryFrom for State { failed_withdrawal_requests: BTreeMap::new(), deposits_to_consolidate: BTreeMap::new(), submitted_transactions: BTreeMap::new(), + transactions_to_resubmit: BTreeSet::new(), succeeded_transactions: BTreeSet::new(), failed_transactions: BTreeMap::new(), consolidation_transactions: BTreeMap::new(), diff --git a/minter/src/state/tests.rs b/minter/src/state/tests.rs index 2752a96b..4142ee64 100644 --- a/minter/src/state/tests.rs +++ b/minter/src/state/tests.rs @@ -64,6 +64,7 @@ mod state_from_init_args { failed_withdrawal_requests: BTreeMap::new(), deposits_to_consolidate: BTreeMap::new(), submitted_transactions: BTreeMap::new(), + transactions_to_resubmit: BTreeSet::new(), succeeded_transactions: BTreeSet::new(), failed_transactions: BTreeMap::new(), consolidation_transactions: BTreeMap::new(), diff --git a/minter/src/test_fixtures/mod.rs b/minter/src/test_fixtures/mod.rs index e679c58b..bc99eba1 100644 --- a/minter/src/test_fixtures/mod.rs +++ b/minter/src/test_fixtures/mod.rs @@ -558,6 +558,7 @@ pub mod arb { ), arb_signature().prop_map(|signature| EventType::SucceededTransaction { signature }), arb_signature().prop_map(|signature| EventType::FailedTransaction { signature }), + arb_signature().prop_map(|signature| EventType::ExpiredTransaction { signature }), ] } From fcc46b60600b5d86636e41bd77aaeaa30a85ed66 Mon Sep 17 00:00:00 2001 From: Louis Pahlavi Date: Mon, 13 Apr 2026 11:13:53 +0200 Subject: [PATCH 3/4] fix: assert transactions_to_resubmit invariants in state transitions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - In `process_transaction_resubmitted`: assert old_signature was in the queue (mirrors the insert assertion in `process_transaction_expired`) - In `process_transaction_succeeded/failed`: replace the defensive remove with an assertion that the signature is NOT queued — a transaction being finalized must still be in `submitted_transactions`, which means it was never fully resubmitted, so it cannot be in the resubmission queue at the same time Co-Authored-By: Claude Sonnet 4.6 --- minter/src/state/mod.rs | 15 ++++++++++++--- minter/src/state/tests.rs | 8 +++++--- minter/src/test_fixtures/mod.rs | 10 ++++++++++ 3 files changed, 27 insertions(+), 6 deletions(-) diff --git a/minter/src/state/mod.rs b/minter/src/state/mod.rs index 6bf61e39..e15c3d19 100644 --- a/minter/src/state/mod.rs +++ b/minter/src/state/mod.rs @@ -615,7 +615,10 @@ impl State { None, "Attempted to resubmit transaction with signature {new_signature:?} that already exists" ); - self.transactions_to_resubmit.remove(old_signature); + assert!( + self.transactions_to_resubmit.remove(old_signature), + "BUG: transaction {old_signature} is not queued for resubmission" + ); if let Some(info) = self.consolidation_transactions.remove(old_signature) { self.consolidation_transactions.insert(*new_signature, info); } @@ -647,7 +650,10 @@ impl State { .checked_sub(tx_fee) .expect("BUG: consolidation amount is less than transaction fee"); } - self.transactions_to_resubmit.remove(signature); + assert!( + !self.transactions_to_resubmit.contains(signature), + "BUG: transaction {signature} is queued for resubmission but is being marked as succeeded" + ); assert!( self.succeeded_transactions.insert(*signature), "Attempted to mark transaction {signature:?} as succeeded twice" @@ -670,7 +676,10 @@ impl State { .unwrap_or_else(|| { panic!("Attempted to mark unknown transaction {signature:?} as failed") }); - self.transactions_to_resubmit.remove(signature); + assert!( + !self.transactions_to_resubmit.contains(signature), + "BUG: transaction {signature} is queued for resubmission but is being marked as failed" + ); assert_eq!( self.failed_transactions.insert(*signature, transaction), None, diff --git a/minter/src/state/tests.rs b/minter/src/state/tests.rs index 4142ee64..aceaa42e 100644 --- a/minter/src/state/tests.rs +++ b/minter/src/state/tests.rs @@ -8,8 +8,9 @@ use crate::{ arb::arb_event, deposit_id, events::{ - accept_deposit, accept_withdrawal, accept_withdrawal_at, fail_transaction, - mint_deposit, resubmit_transaction, submit_withdrawal, succeed_transaction, + accept_deposit, accept_withdrawal, accept_withdrawal_at, expire_transaction, + fail_transaction, mint_deposit, resubmit_transaction, submit_withdrawal, + succeed_transaction, }, init_balance, init_state, ledger_canister_id, runtime::TestCanisterRuntime, @@ -618,7 +619,8 @@ mod oldest_incomplete_withdrawal_created_at { Some(1_000_000_000) ); - // Resubmit the transaction with a new signature + // Expire then resubmit the transaction with a new signature + expire_transaction(signature(0xAA)); resubmit_transaction(signature(0xAA), signature(0xBB), 42); // created_at timestamps should be unchanged diff --git a/minter/src/test_fixtures/mod.rs b/minter/src/test_fixtures/mod.rs index bc99eba1..47072fa2 100644 --- a/minter/src/test_fixtures/mod.rs +++ b/minter/src/test_fixtures/mod.rs @@ -294,6 +294,16 @@ pub mod events { }); } + pub fn expire_transaction(signature: Signature) { + mutate_state(|state| { + process_event( + state, + EventType::ExpiredTransaction { signature }, + &runtime(), + ) + }); + } + pub fn resubmit_transaction( old_signature: Signature, new_signature: Signature, From d110d3cc9611654797987393733dee2f4f98c920 Mon Sep 17 00:00:00 2001 From: Louis Pahlavi Date: Mon, 13 Apr 2026 11:59:01 +0200 Subject: [PATCH 4/4] refactor: process_transaction_expired takes &Signature for consistency Co-Authored-By: Claude Sonnet 4.6 --- minter/src/state/audit.rs | 2 +- minter/src/state/mod.rs | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/minter/src/state/audit.rs b/minter/src/state/audit.rs index 7e48442e..faa6bcd1 100644 --- a/minter/src/state/audit.rs +++ b/minter/src/state/audit.rs @@ -64,7 +64,7 @@ fn apply_state_transition(state: &mut State, payload: &EventType, timestamp: u64 state.process_transaction_failed(signature); } EventType::ExpiredTransaction { signature } => { - state.process_transaction_expired(*signature); + state.process_transaction_expired(signature); } } } diff --git a/minter/src/state/mod.rs b/minter/src/state/mod.rs index e15c3d19..3841bbf5 100644 --- a/minter/src/state/mod.rs +++ b/minter/src/state/mod.rs @@ -206,21 +206,21 @@ impl State { &self.transactions_to_resubmit } - pub fn process_transaction_expired(&mut self, signature: Signature) { + pub fn process_transaction_expired(&mut self, signature: &Signature) { assert!( - !self.succeeded_transactions.contains(&signature), + !self.succeeded_transactions.contains(signature), "BUG: cannot mark already succeeded transaction {signature} for resubmission" ); assert!( - !self.failed_transactions.contains_key(&signature), + !self.failed_transactions.contains_key(signature), "BUG: cannot mark already failed transaction {signature} for resubmission" ); assert!( - self.submitted_transactions.contains_key(&signature), + self.submitted_transactions.contains_key(signature), "BUG: cannot mark non-submitted transaction {signature} for resubmission" ); assert!( - self.transactions_to_resubmit.insert(signature), + self.transactions_to_resubmit.insert(*signature), "BUG: transaction {signature} is already queued for resubmission" ); }