From d18333eeaacbb387330518f7204bfc0ec07abc76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9gory=20Demay?= Date: Wed, 7 Oct 2026 15:24:20 +0000 Subject: [PATCH 01/10] feat(minter): finalize withdrawals from their nonce accounts Status queries cannot decide older withdrawals, since some providers keep only hours of history and others answer inconsistently. The finalization timer now reads the nonce account of each in-flight withdrawal instead: an unchanged nonce triggers the re-broadcast, a stale one no decision, and a never-seen one fetches the landed transaction with getTransaction to record its outcome, keeping the account bound until it is known. Sweeps are still decided with getSignatureStatuses. Co-Authored-By: Claude Opus 5.5 --- integration_tests/src/fixtures.rs | 65 ++++- integration_tests/tests/tests.rs | 6 +- minter/src/metrics.rs | 6 + minter/src/monitor/mod.rs | 133 +++------- minter/src/monitor/tests.rs | 378 ++++++++++++++++++----------- minter/src/monitor/withdrawals.rs | 212 ++++++++++++++++ minter/src/rpc/mod.rs | 76 +++++- minter/src/state/mod.rs | 2 +- minter/src/state/nonce_pool/mod.rs | 28 +++ minter/src/storage/mod.rs | 2 + minter/src/test_fixtures/mod.rs | 70 ++++++ minter/src/withdraw/nonce/mod.rs | 21 ++ minter/src/withdraw/tests.rs | 14 +- 13 files changed, 746 insertions(+), 267 deletions(-) create mode 100644 minter/src/monitor/withdrawals.rs diff --git a/integration_tests/src/fixtures.rs b/integration_tests/src/fixtures.rs index 8e745858b..90d6dc8e7 100644 --- a/integration_tests/src/fixtures.rs +++ b/integration_tests/src/fixtures.rs @@ -48,6 +48,11 @@ const MOCK_SLOT: u64 = 100_000_000; const SUBMITTED_BLOCKHASH: &str = "4sGjMW1sUnHzSxGspuhpqLDx6wiyjNtZAMdL4VZHirAn"; const SUBMITTED_SIGNATURE: &str = "5VERv8NMvzbJMEkV8xnrLkEaWRtSz9CosKDYjCJjBRnbJLgp8uirBgmQpjKhoR4tjF3ZpRzrFmBV6UjKdiSZkQUW"; +/// Seed of the nonce value the mocked nonce account stores when the withdrawal timer reads it. +const SUBMITTED_NONCE_SEED: [u8; 32] = [0x4E; 32]; +/// Seed of the nonce value the mocked nonce account stores once the submitted withdrawal +/// transaction has advanced it. +const ADVANCED_NONCE_SEED: [u8; 32] = [0x4F; 32]; /// Blockhash the mocks report for a block a timer only reads the height of. const IGNORED_BLOCKHASH: &str = "CzBVNFJkh7WkQDfJUiDjLc7kPrJd8kR2yiCvwBUhSe7Y"; @@ -154,7 +159,7 @@ impl MockBuilder { pub fn get_nonce_account(self) -> Self { self.expect( get_account_info_request(), - get_account_info_nonce_response(), + get_account_info_nonce_response(SUBMITTED_NONCE_SEED), ) } @@ -167,11 +172,19 @@ impl MockBuilder { ) } - /// Mocks for `finalize_transactions` finding only in-flight withdrawal - /// transactions, which carry a durable nonce and need no current block: - /// a single `getSignatureStatuses` reporting them as finalized. - pub fn finalize_withdrawal_transaction(self, signature: &Signature) -> Self { - self.check_signature_statuses(signature, get_signature_statuses_finalized_response()) + /// Mocks for `finalize_transactions` finding only the given in-flight withdrawal + /// transaction, which carries a durable nonce and needs no current block: + /// `getAccountInfo` reporting its nonce account advanced → `getTransaction` + /// reporting it as succeeded. + pub fn finalize_withdrawal_transaction(self, withdrawal: &Transaction) -> Self { + self.expect( + get_account_info_request(), + get_account_info_nonce_response(ADVANCED_NONCE_SEED), + ) + .expect( + get_transaction_request(&withdrawal.signatures[0]), + withdrawal_transaction_response(withdrawal), + ) } /// Mocks for `finalize_transactions` finding the pending transaction with the given @@ -263,17 +276,49 @@ fn sweep_transaction_response(sweep: &Transaction, sweepable_amount: Lamport) -> })) } +/// JSON-RPC `getTransaction` response for the given withdrawal transaction, finalized without +/// error. +fn withdrawal_transaction_response(withdrawal: &Transaction) -> JsonRpcResponse { + let encoded_transaction = STANDARD.encode( + bincode::serialize(withdrawal).expect("serializing the transaction should succeed"), + ); + JsonRpcResponse::from(json!({ + "jsonrpc": "2.0", + "result": { + "blockTime": 1700000000_i64, + "meta": { + "computeUnitsConsumed": 450, + "err": null, + "fee": FEE_PER_SIGNATURE, + "innerInstructions": [], + "loadedAddresses": { "readonly": [], "writable": [] }, + "logMessages": [], + "postBalances": [], + "postTokenBalances": [], + "preBalances": [], + "preTokenBalances": [], + "rewards": [], + "status": { "Ok": null } + }, + "slot": 350_000_000_u64, + "transaction": [encoded_transaction, "base64"], + "version": "legacy" + }, + "id": 1 + })) +} + fn get_account_info_request() -> JsonRpcRequestMatcher { JsonRpcRequestMatcher::with_method("getAccountInfo") } -fn get_account_info_nonce_response() -> JsonRpcResponse { +fn get_account_info_nonce_response(nonce_seed: [u8; 32]) -> JsonRpcResponse { JsonRpcResponse::from(json!({ "jsonrpc": "2.0", "result": { "context": { "apiVersion": "2.0.15", "slot": 341_197_053 }, "value": { - "data": [nonce_account_data(), "base64"], + "data": [nonce_account_data(nonce_seed), "base64"], "executable": false, "lamports": 1_447_680, "owner": "11111111111111111111111111111111", @@ -285,10 +330,10 @@ fn get_account_info_nonce_response() -> JsonRpcResponse { })) } -fn nonce_account_data() -> String { +fn nonce_account_data(nonce_seed: [u8; 32]) -> String { let nonce_account = Versions::new(State::Initialized(Data::new( MINTER_ADDRESS, - DurableNonce::from_blockhash(&Hash::from([0x4E; 32])), + DurableNonce::from_blockhash(&Hash::from(nonce_seed)), FEE_PER_SIGNATURE, ))); STANDARD.encode( diff --git a/integration_tests/tests/tests.rs b/integration_tests/tests/tests.rs index df00a34a4..f51ea73a8 100644 --- a/integration_tests/tests/tests.rs +++ b/integration_tests/tests/tests.rs @@ -786,12 +786,14 @@ mod withdrawal_tests { ); // Only the in-flight withdrawal is monitored, so the finalization timer - // checks the signature statuses without fetching a current block. + // reads its nonce account and fetches the landed transaction without + // fetching a current block. setup.advance_time(FINALIZE_TRANSACTIONS_DELAY).await; + let withdrawal = setup.minter().signed_transaction(&transaction_id).await; setup .execute_http_mocks( MockBuilder::with_start_id(40) - .finalize_withdrawal_transaction(&transaction_id) + .finalize_withdrawal_transaction(&withdrawal) .build(), ) .await; diff --git a/minter/src/metrics.rs b/minter/src/metrics.rs index e8dae4a63..be5f6abfa 100644 --- a/minter/src/metrics.rs +++ b/minter/src/metrics.rs @@ -123,6 +123,12 @@ pub fn encode_metrics(w: &mut MetricsEncoder>, s: &State) -> std::io::Re storage::with_unstable_metrics(|m| m.withdrawal_transaction_rebroadcasts).metric_value(), "Number of re-broadcast attempts of withdrawal transactions since the last upgrade.", )?; + w.encode_gauge( + "withdrawal_transactions_with_unresolved_outcome", + storage::with_unstable_metrics(|m| m.withdrawal_transactions_with_unresolved_outcome) + .metric_value(), + "Number of in-flight withdrawal transactions whose nonce advanced but whose outcome could not be fetched in the last finalization round.", + )?; Ok(()) } diff --git a/minter/src/monitor/mod.rs b/minter/src/monitor/mod.rs index 4976c14c4..b59b53750 100644 --- a/minter/src/monitor/mod.rs +++ b/minter/src/monitor/mod.rs @@ -2,36 +2,32 @@ use crate::{ constants::MAX_CONCURRENT_RPC_CALLS, deposit::sweep::credit_finalized_sweeps, guard::TimerGuard, - rpc::{ - BlockHeight, get_recent_block, get_signature_statuses, - submit_transaction_skipping_preflight, - }, + monitor::withdrawals::check_withdrawal_transactions, + rpc::{BlockHeight, get_recent_block, get_signature_statuses}, runtime::CanisterRuntime, state::{ - MinterTransaction, TaskType, - audit::process_event, - event::{EventType, VersionedMessage}, - mutate_state, read_state, + MinterTransaction, TaskType, audit::process_event, event::EventType, mutate_state, + read_state, }, - storage::with_unstable_metrics_mut, }; use canlog::log; use cksol_types_internal::log::Priority; use itertools::Itertools; use solana_signature::Signature; -use solana_transaction::Transaction; use solana_transaction_status_client_types::TransactionConfirmationStatus; use std::collections::{BTreeMap, BTreeSet}; use std::time::Duration; #[cfg(test)] mod tests; +mod withdrawals; pub const FINALIZE_TRANSACTIONS_DELAY: Duration = Duration::from_mins(2); -/// Minimum time since its submission before a withdrawal transaction without a -/// status is re-broadcast. The minter sends transactions without `maxRetries`, -/// so an Agave RPC node keeps re-sending a durable-nonce transaction every 2 s -/// until it lands, its nonce advances, or 150 blocks (about 60 to 90 s) pass. +/// Minimum time since its submission before a withdrawal transaction whose nonce +/// account still stores its nonce value is re-broadcast. The minter sends +/// transactions without `maxRetries`, so an Agave RPC node keeps re-sending a +/// durable-nonce transaction every 2 s until it lands, its nonce advances, or +/// 150 blocks (about 60 to 90 s) pass. /// Re-broadcasting earlier would only duplicate the work of the RPC node. /// See https://github.com/anza-xyz/agave/blob/master/rpc/src/rpc.rs pub const MIN_REBROADCAST_AGE: Duration = Duration::from_secs(90); @@ -48,8 +44,9 @@ const MAX_BLOCKHASH_AGE_IN_BLOCKS: BlockHeight = BlockHeight::new(150); /// See https://solana.com/docs/rpc/http/getsignaturestatuses const MAX_SIGNATURES_PER_STATUS_CHECK: usize = 256; -/// Check the status of all submitted transactions, finalize succeeded/failed -/// ones, drop expired sweeps, and re-broadcast withdrawals that have no status. +/// Check the status of all submitted sweeps, finalize succeeded/failed ones and drop +/// expired ones, then finalize the withdrawals whose nonce advanced and re-broadcast +/// the withdrawals whose nonce did not. pub async fn finalize_transactions(runtime: R) { let _guard = match TimerGuard::new(TaskType::FinalizeTransactions) { Ok(guard) => guard, @@ -60,7 +57,8 @@ pub async fn finalize_transactions(runtime: R) { runtime.set_timer(Duration::ZERO, finalize_transactions); }); - let more_transactions_to_check = check_submitted_transactions(&runtime).await; + let more_transactions_to_check = check_sweep_transactions(&runtime).await; + check_withdrawal_transactions(&runtime).await; let more_sweeps_to_credit = credit_finalized_sweeps(&runtime).await; if !more_transactions_to_check && !more_sweeps_to_credit { @@ -69,40 +67,31 @@ pub async fn finalize_transactions(runtime: R) { } /// Returns whether the finalization timer must run again immediately. -async fn check_submitted_transactions(runtime: &R) -> bool { - let (signatures, blockhash_transactions): ( - BTreeSet, - BTreeMap, - ) = read_state(|state| { - let submitted = state.submitted_transactions(); - ( - submitted.iter().map(|(sig, _)| *sig).collect(), - submitted - .iter() - .filter_map(|(sig, tx)| match tx { - MinterTransaction::SweepDeposit { block_height, .. } => { - Some((*sig, *block_height)) - } - MinterTransaction::Withdrawal { .. } => None, - }) - .collect(), - ) +async fn check_sweep_transactions(runtime: &R) -> bool { + let blockhash_transactions: BTreeMap = read_state(|state| { + state + .submitted_transactions() + .iter() + .filter_map(|(signature, transaction)| match transaction { + MinterTransaction::SweepDeposit { block_height } => { + Some((*signature, *block_height)) + } + MinterTransaction::Withdrawal { .. } => None, + }) + .collect() }); - if signatures.is_empty() { + if blockhash_transactions.is_empty() { return false; } // Fetch the current block before checking statuses: if a transaction finalizes // after we snapshot the block, the status check will see it as finalized rather // than missing, so it will never be incorrectly marked as expired. - let current_block_height = if blockhash_transactions.is_empty() { - None - } else { - fetch_current_block_height(runtime).await - }; + let current_block_height = fetch_current_block_height(runtime).await; - let num_transactions = signatures.len(); - let statuses = check_transaction_statuses(runtime, signatures.into_iter().collect()).await; + let num_transactions = blockhash_transactions.len(); + let statuses = + check_transaction_statuses(runtime, blockhash_transactions.keys().copied().collect()).await; for (signature, error) in &statuses.errored { log!( @@ -142,8 +131,6 @@ async fn check_submitted_transactions(runtime: &R) -> bool { ); } - rebroadcast_withdrawal_transactions(runtime, &statuses.not_found).await; - num_transactions > MAX_CONCURRENT_RPC_CALLS * MAX_SIGNATURES_PER_STATUS_CHECK } @@ -186,62 +173,6 @@ fn expire_transactions( } } -async fn rebroadcast_withdrawal_transactions( - runtime: &R, - not_found: &BTreeSet, -) { - let now = runtime.time(); - let batches: Vec> = read_state(|state| { - not_found - .iter() - .filter_map( - |signature| match state.submitted_transactions().get(signature)? { - MinterTransaction::Withdrawal { - message: VersionedMessage::Legacy(message), - submitted_at, - .. - } if is_old_enough_to_rebroadcast(*submitted_at, now) => Some(Transaction { - signatures: vec![*signature], - message: message.clone(), - }), - MinterTransaction::Withdrawal { .. } - | MinterTransaction::SweepDeposit { .. } => None, - }, - ) - .chunks(MAX_CONCURRENT_RPC_CALLS) - .into_iter() - .map(Iterator::collect) - .collect() - }); - for batch in batches { - futures::future::join_all( - batch - .into_iter() - .map(|transaction| rebroadcast_transaction(runtime, transaction)), - ) - .await; - } -} - -fn is_old_enough_to_rebroadcast(submitted_at: u64, now: u64) -> bool { - Duration::from_nanos(now.saturating_sub(submitted_at)) >= MIN_REBROADCAST_AGE -} - -async fn rebroadcast_transaction(runtime: &R, transaction: Transaction) { - let signature = transaction.signatures[0]; - with_unstable_metrics_mut(|m| m.withdrawal_transaction_rebroadcasts += 1); - match submit_transaction_skipping_preflight(runtime, transaction).await { - Ok(_) => log!( - Priority::Info, - "Re-broadcast withdrawal transaction {signature}" - ), - Err(e) => log!( - Priority::Info, - "Failed to re-broadcast withdrawal transaction {signature} (will retry next round): {e}" - ), - } -} - fn is_blockhash_expired( transaction_block_height: BlockHeight, current_block_height: BlockHeight, diff --git a/minter/src/monitor/tests.rs b/minter/src/monitor/tests.rs index f696ab4bc..99c9cd86a 100644 --- a/minter/src/monitor/tests.rs +++ b/minter/src/monitor/tests.rs @@ -10,11 +10,13 @@ use crate::{ event::{EventType, VersionedMessage}, mutate_state, read_state, reset_state, }, - storage::reset_events, + storage::{reset_events, with_unstable_metrics}, test_fixtures::{ - EventsAssert, GetTransactionResult, MINIMUM_WITHDRAWAL_AMOUNT, account, - confirmed_block_at_height, events, finalized_status, init_balance, init_schnorr_master_key, - init_state, runtime::TestCanisterRuntime, signature, + EventsAssert, GetTransactionResult, MINIMUM_WITHDRAWAL_AMOUNT, MINTER_ADDRESS, + NONCE_ACCOUNT, account, confirmed_block_at_height, durable_nonce, events, + failed_withdrawal_response, finalized_status, init_balance, init_schnorr_master_key, + init_state, nonce_account_info, runtime::TestCanisterRuntime, signature, + succeeded_withdrawal_response, }, }; use sol_rpc_types::{ @@ -27,6 +29,7 @@ type SlotResult = MultiRpcResult; type BlockResult = MultiRpcResult; type SignatureStatusesResult = MultiRpcResult>>; type SendTransactionResult = MultiRpcResult; +type GetAccountInfoResult = MultiRpcResult>; const CURRENT_SLOT: Slot = 408_807_102; const SUBMISSION_SLOT: Slot = CURRENT_SLOT - 10; @@ -68,7 +71,7 @@ mod finalization { #[tokio::test] async fn should_finalize_but_not_expire_transactions_if_fetching_current_block_fails() { setup(); - let finalized = submit_withdrawal_transaction_with_signature(1); + let finalized = submit_sweep_transaction_with_signature(1, EXPIRED_BLOCK_HEIGHT); let not_found = submit_sweep_transaction_with_signature(2, EXPIRED_BLOCK_HEIGHT); let runtime = TestCanisterRuntime::new() @@ -77,7 +80,8 @@ mod finalization { .add_stub_response(SignatureStatusesResult::Consistent(Ok(vec![ Some(finalized_status()), None, - ]))); + ]))) + .add_stub_response(GetTransactionResult::Consistent(Ok(None))); finalize_transactions(runtime).await; @@ -93,38 +97,35 @@ mod finalization { } #[tokio::test] - async fn should_reschedule_until_all_transactions_finalized() { + async fn should_reschedule_while_more_sweeps_are_in_flight_than_a_round_checks() { setup(); let num = MAX_CONCURRENT_RPC_CALLS * MAX_SIGNATURES_PER_STATUS_CHECK + 1; for i in 0..num { - submit_withdrawal_transaction_with_signature(i); + submit_sweep_transaction_with_signature(i, CURRENT_BLOCK_HEIGHT); } - // Round 1: finalizes MAX_CONCURRENT_RPC_CALLS batches, 1 transaction unchecked → reschedule - let mut runtime = TestCanisterRuntime::new().with_increasing_time(); + let mut runtime = TestCanisterRuntime::new() + .with_increasing_time() + .add_stub_response(SlotResult::Consistent(Ok(CURRENT_SLOT))) + .add_stub_response(BlockResult::Consistent(Ok(current_block()))); for _ in 0..MAX_CONCURRENT_RPC_CALLS { - runtime = runtime.add_stub_response(SignatureStatusesResult::Consistent(Ok( - vec![Some(finalized_status()); MAX_SIGNATURES_PER_STATUS_CHECK], - ))); + runtime = runtime.add_stub_response(SignatureStatusesResult::Consistent(Ok(vec![ + None; + MAX_SIGNATURES_PER_STATUS_CHECK + ]))); } finalize_transactions(runtime.clone()).await; - assert_eq!(read_state(|s| s.submitted_transactions().len()), 1); + assert_eq!( + called_methods(&runtime) + .iter() + .filter(|method| *method == "getSignatureStatuses") + .count(), + MAX_CONCURRENT_RPC_CALLS + ); assert_eq!(runtime.set_timer_call_count(), 1); - - // Round 2: finalizes the remaining 1 transaction → no reschedule - let runtime = TestCanisterRuntime::new() - .with_increasing_time() - .add_stub_response(SignatureStatusesResult::Consistent(Ok(vec![Some( - finalized_status(), - )]))); - - finalize_transactions(runtime.clone()).await; - - assert!(read_state(|s| s.submitted_transactions().is_empty())); - assert_eq!(runtime.set_timer_call_count(), 0); } #[tokio::test] @@ -186,114 +187,6 @@ mod finalization { read_state(|s| assert_eq!(s.submitted_transactions().len(), 1)); } - #[tokio::test] - async fn should_finalize_an_in_flight_withdrawal_without_fetching_a_block() { - setup(); - let signature = submit_withdrawal_transaction(); - - let runtime = TestCanisterRuntime::new() - .with_increasing_time() - .add_stub_response(SignatureStatusesResult::Consistent(Ok(vec![Some( - finalized_status(), - )]))); - - finalize_transactions(runtime).await; - - EventsAssert::from_recorded() - .expect_contains_event_eq(EventType::SucceededTransaction { signature }); - read_state(|s| { - assert!(s.submitted_transactions().is_empty()); - assert!(s.succeeded_transactions().contains(&signature)); - }); - } - - #[tokio::test] - async fn should_rebroadcast_a_missing_withdrawal_unchanged() { - setup(); - let signature = submit_withdrawal_transaction(); - let events_before = EventsAssert::from_recorded(); - - let runtime = TestCanisterRuntime::new() - .with_increasing_time_from(min_rebroadcast_age_nanos()) - .add_stub_response(SignatureStatusesResult::Consistent(Ok(vec![None]))) - .add_stub_response(SendTransactionResult::Consistent(Ok(signature.into()))); - - finalize_transactions(runtime.clone()).await; - - let sent = runtime.sent_transactions(); - assert_eq!(sent.len(), 1); - assert_eq!( - sent[0].get_transaction(), - encoded_submitted_transaction(&signature) - ); - assert_eq!(sent[0].skip_preflight, Some(true)); - assert_eq!(EventsAssert::from_recorded(), events_before); - read_state(|s| assert!(s.submitted_transactions().contains_key(&signature))); - } - - #[tokio::test] - async fn should_rebroadcast_a_missing_withdrawal_only_after_the_minimum_age() { - setup(); - let signature = submit_withdrawal_transaction(); - - let too_early = TestCanisterRuntime::new() - .with_increasing_time() - .add_stub_response(SignatureStatusesResult::Consistent(Ok(vec![None]))); - - finalize_transactions(too_early.clone()).await; - - assert!(too_early.sent_transactions().is_empty()); - - let old_enough = TestCanisterRuntime::new() - .with_increasing_time_from(min_rebroadcast_age_nanos()) - .add_stub_response(SignatureStatusesResult::Consistent(Ok(vec![None]))) - .add_stub_response(SendTransactionResult::Consistent(Ok(signature.into()))); - - finalize_transactions(old_enough.clone()).await; - - assert_eq!(old_enough.sent_transactions().len(), 1); - read_state(|s| assert!(s.submitted_transactions().contains_key(&signature))); - } - - fn min_rebroadcast_age_nanos() -> u64 { - MIN_REBROADCAST_AGE.as_nanos() as u64 - } - - fn encoded_submitted_transaction(signature: &solana_signature::Signature) -> String { - let message = read_state(|s| { - match s - .submitted_transactions() - .get(signature) - .expect("the transaction is submitted") - { - MinterTransaction::Withdrawal { - message: VersionedMessage::Legacy(message), - .. - } => message.clone(), - other => panic!("expected a withdrawal transaction, got {other:?}"), - } - }); - let transaction = Transaction { - signatures: vec![*signature], - message, - }; - SendTransactionParams::try_from(transaction) - .expect("the transaction is serializable") - .get_transaction() - .to_string() - } - - fn submit_withdrawal_transaction() -> solana_signature::Signature { - submit_withdrawal_transaction_with_signature(0x77) - } - - fn submit_withdrawal_transaction_with_signature(i: usize) -> solana_signature::Signature { - let signature = signature(i); - events::accept_withdrawal(account(i), i as u64, MINIMUM_WITHDRAWAL_AMOUNT); - events::submit_withdrawal(signature, vec![i as u64]); - signature - } - #[tokio::test] async fn should_record_failed_transaction_event_on_error() { setup(); @@ -365,16 +258,17 @@ mod finalization { } #[tokio::test] - async fn should_never_expire_nonce_withdrawal_with_missing_status() { + async fn should_never_expire_nonce_withdrawal_with_unchanged_nonce() { setup(); let sweep = submit_sweep_transaction_with_signature(1, EXPIRED_BLOCK_HEIGHT); - let nonce_withdrawal = submit_withdrawal_transaction_with_signature(2); + let nonce_withdrawal = submit_withdrawal_bound_to(2, BOUND_NONCE_SEED); let runtime = TestCanisterRuntime::new() .with_increasing_time() .add_stub_response(SlotResult::Consistent(Ok(CURRENT_SLOT))) .add_stub_response(BlockResult::Consistent(Ok(current_block()))) - .add_stub_response(SignatureStatusesResult::Consistent(Ok(vec![None, None]))); + .add_stub_response(SignatureStatusesResult::Consistent(Ok(vec![None]))) + .add_stub_response(nonce_read(BOUND_NONCE_SEED)); finalize_transactions(runtime).await; @@ -484,6 +378,216 @@ mod finalization { } } +mod withdrawal_finalization { + use super::*; + + #[tokio::test] + async fn should_record_a_withdrawal_as_succeeded_once_its_nonce_advanced() { + setup(); + let signature = submit_withdrawal_bound_to(1, BOUND_NONCE_SEED); + + let runtime = TestCanisterRuntime::new() + .with_increasing_time() + .add_stub_response(nonce_read(UNSEEN_NONCE_SEED)) + .add_stub_response(succeeded_withdrawal_response(&signature)); + + finalize_transactions(runtime.clone()).await; + + EventsAssert::from_recorded() + .expect_contains_event_eq(EventType::SucceededTransaction { signature }); + read_state(|s| { + assert!(s.submitted_transactions().is_empty()); + assert!(s.succeeded_transactions().contains(&signature)); + }); + assert_nonce_account_is_free(); + assert_eq!( + called_methods(&runtime), + vec!["getAccountInfo", "getTransaction"] + ); + } + + #[tokio::test] + async fn should_record_a_withdrawal_as_failed_once_its_nonce_advanced_with_an_error() { + setup(); + let signature = submit_withdrawal_bound_to(1, BOUND_NONCE_SEED); + + let runtime = TestCanisterRuntime::new() + .with_increasing_time() + .add_stub_response(nonce_read(UNSEEN_NONCE_SEED)) + .add_stub_response(failed_withdrawal_response(&signature)); + + finalize_transactions(runtime).await; + + EventsAssert::from_recorded() + .expect_contains_event_eq(EventType::FailedTransaction { signature }); + read_state(|s| { + assert!(s.submitted_transactions().is_empty()); + assert!(s.failed_transactions().contains_key(&signature)); + }); + assert_nonce_account_is_free(); + } + + #[tokio::test] + async fn should_rebroadcast_a_withdrawal_with_an_unchanged_nonce_unchanged() { + setup(); + let signature = submit_withdrawal_bound_to(1, BOUND_NONCE_SEED); + let events_before = EventsAssert::from_recorded(); + + let runtime = TestCanisterRuntime::new() + .with_increasing_time_from(min_rebroadcast_age_nanos()) + .add_stub_response(nonce_read(BOUND_NONCE_SEED)) + .add_stub_response(SendTransactionResult::Consistent(Ok(signature.into()))); + + finalize_transactions(runtime.clone()).await; + + let sent = runtime.sent_transactions(); + assert_eq!(sent.len(), 1); + assert_eq!( + sent[0].get_transaction(), + encoded_submitted_transaction(&signature) + ); + assert_eq!(sent[0].skip_preflight, Some(true)); + assert_eq!(EventsAssert::from_recorded(), events_before); + read_state(|s| assert!(s.submitted_transactions().contains_key(&signature))); + } + + #[tokio::test] + async fn should_rebroadcast_a_withdrawal_with_an_unchanged_nonce_only_after_the_minimum_age() { + setup(); + let signature = submit_withdrawal_bound_to(1, BOUND_NONCE_SEED); + + let too_early = TestCanisterRuntime::new() + .with_increasing_time() + .add_stub_response(nonce_read(BOUND_NONCE_SEED)); + + finalize_transactions(too_early.clone()).await; + + assert!(too_early.sent_transactions().is_empty()); + + let old_enough = TestCanisterRuntime::new() + .with_increasing_time_from(min_rebroadcast_age_nanos()) + .add_stub_response(nonce_read(BOUND_NONCE_SEED)) + .add_stub_response(SendTransactionResult::Consistent(Ok(signature.into()))); + + finalize_transactions(old_enough.clone()).await; + + assert_eq!(old_enough.sent_transactions().len(), 1); + read_state(|s| assert!(s.submitted_transactions().contains_key(&signature))); + } + + #[tokio::test] + async fn should_take_no_decision_on_a_stale_nonce_read() { + setup(); + let landed = submit_withdrawal_bound_to(1, STALE_NONCE_SEED); + events::succeed_transaction(landed); + let signature = submit_withdrawal_bound_to(2, BOUND_NONCE_SEED); + let events_before = EventsAssert::from_recorded(); + + let runtime = TestCanisterRuntime::new() + .with_increasing_time_from(min_rebroadcast_age_nanos()) + .add_stub_response(nonce_read(STALE_NONCE_SEED)); + + finalize_transactions(runtime.clone()).await; + + assert_eq!(EventsAssert::from_recorded(), events_before); + assert!(runtime.sent_transactions().is_empty()); + read_state(|s| assert!(s.submitted_transactions().contains_key(&signature))); + } + + #[tokio::test] + async fn should_keep_the_nonce_account_bound_until_the_outcome_is_known() { + setup(); + let signature = submit_withdrawal_bound_to(1, BOUND_NONCE_SEED); + let events_before = EventsAssert::from_recorded(); + + let runtime = TestCanisterRuntime::new() + .with_increasing_time_from(min_rebroadcast_age_nanos()) + .add_stub_response(nonce_read(UNSEEN_NONCE_SEED)) + .add_stub_response(GetTransactionResult::Consistent(Ok(None))); + + finalize_transactions(runtime.clone()).await; + + assert_eq!(EventsAssert::from_recorded(), events_before); + assert!(runtime.sent_transactions().is_empty()); + read_state(|s| { + assert!(s.submitted_transactions().contains_key(&signature)); + assert_eq!(s.nonce_pool().num_free_accounts(), 0); + }); + assert_eq!( + with_unstable_metrics(|m| m.withdrawal_transactions_with_unresolved_outcome), + 1 + ); + } + + fn assert_nonce_account_is_free() { + read_state(|s| { + assert!( + s.nonce_pool() + .free_accounts() + .any(|account| *account == NONCE_ACCOUNT) + ) + }); + } +} + +const BOUND_NONCE_SEED: usize = 1; +const STALE_NONCE_SEED: usize = 2; +const UNSEEN_NONCE_SEED: usize = 3; + +fn min_rebroadcast_age_nanos() -> u64 { + MIN_REBROADCAST_AGE.as_nanos() as u64 +} + +fn encoded_submitted_transaction(signature: &solana_signature::Signature) -> String { + let message = read_state(|s| { + match s + .submitted_transactions() + .get(signature) + .expect("the transaction is submitted") + { + MinterTransaction::Withdrawal { + message: VersionedMessage::Legacy(message), + .. + } => message.clone(), + other => panic!("expected a withdrawal transaction, got {other:?}"), + } + }); + let transaction = Transaction { + signatures: vec![*signature], + message, + }; + SendTransactionParams::try_from(transaction) + .expect("the transaction is serializable") + .get_transaction() + .to_string() +} + +/// Submits a withdrawal transaction under `signature(i)` bound to [`NONCE_ACCOUNT`] +/// and to the nonce value that [`nonce_read`] returns for `nonce_seed`. +fn submit_withdrawal_bound_to(i: usize, nonce_seed: usize) -> solana_signature::Signature { + let signature = signature(i); + events::accept_withdrawal(account(i), i as u64, MINIMUM_WITHDRAWAL_AMOUNT); + events::create_withdrawal_batch_transaction(durable_nonce(nonce_seed), vec![i as u64]); + events::submit_withdrawal_batch_transaction( + signature, + durable_nonce(nonce_seed), + vec![i as u64], + ); + signature +} + +fn nonce_read(nonce_seed: usize) -> GetAccountInfoResult { + GetAccountInfoResult::Consistent(Ok(Some(nonce_account_info(MINTER_ADDRESS, nonce_seed)))) +} + +fn called_methods(runtime: &TestCanisterRuntime) -> Vec { + runtime + .sent_update_calls() + .into_iter() + .map(|call| call.method) + .collect() +} + fn setup() { init_state(); init_balance(); diff --git a/minter/src/monitor/withdrawals.rs b/minter/src/monitor/withdrawals.rs new file mode 100644 index 000000000..ff2e2e003 --- /dev/null +++ b/minter/src/monitor/withdrawals.rs @@ -0,0 +1,212 @@ +use crate::{ + address::minter_address, + constants::MAX_CONCURRENT_RPC_CALLS, + monitor::MIN_REBROADCAST_AGE, + rpc::{ + TransactionOutcome, get_submitted_transaction_outcome, + submit_transaction_skipping_preflight, + }, + runtime::CanisterRuntime, + state::{ + MinterTransaction, NonceRead, + audit::process_event, + event::{EventType, VersionedMessage}, + mutate_state, read_state, + }, + storage::with_unstable_metrics_mut, + withdraw::nonce::read_verified_nonce, +}; +use canlog::log; +use cksol_types_internal::log::Priority; +use solana_address::Address; +use solana_hash::Hash; +use solana_message::Message; +use solana_signature::Signature; +use solana_transaction::Transaction; +use std::time::Duration; + +/// Decides every in-flight withdrawal transaction from a finalized read of its nonce +/// account: a landed transaction is finalized with the outcome `getTransaction` reports, +/// and a transaction that has not landed is re-broadcast once old enough. +pub(super) async fn check_withdrawal_transactions(runtime: &R) { + let Some(minter_address) = read_state(|state| state.minter_public_key().map(minter_address)) + else { + return; + }; + let withdrawals = in_flight_withdrawals(); + let mut num_unresolved_outcomes = 0; + for batch in withdrawals.chunks(MAX_CONCURRENT_RPC_CALLS) { + num_unresolved_outcomes += futures::future::join_all( + batch + .iter() + .map(|withdrawal| check_withdrawal(runtime, withdrawal, minter_address)), + ) + .await + .into_iter() + .filter(|check| *check == WithdrawalCheck::UnresolvedOutcome) + .count(); + } + with_unstable_metrics_mut(|metrics| { + metrics.withdrawal_transactions_with_unresolved_outcome = num_unresolved_outcomes as u64 + }); +} + +struct InFlightWithdrawal { + signature: Signature, + message: Message, + nonce_account: Address, + nonce_value: Hash, + submitted_at: u64, +} + +impl InFlightWithdrawal { + fn signed_transaction(&self) -> Transaction { + Transaction { + signatures: vec![self.signature], + message: self.message.clone(), + } + } + + fn is_old_enough_to_rebroadcast(&self, now: u64) -> bool { + Duration::from_nanos(now.saturating_sub(self.submitted_at)) >= MIN_REBROADCAST_AGE + } +} + +fn in_flight_withdrawals() -> Vec { + read_state(|state| { + state + .submitted_transactions() + .iter() + .filter_map(|(signature, transaction)| match transaction { + MinterTransaction::Withdrawal { + message: VersionedMessage::Legacy(message), + nonce_account, + nonce_value, + submitted_at, + } => Some(InFlightWithdrawal { + signature: *signature, + message: message.clone(), + nonce_account: *nonce_account, + nonce_value: *nonce_value, + submitted_at: *submitted_at, + }), + MinterTransaction::SweepDeposit { .. } => None, + }) + .collect() + }) +} + +#[derive(PartialEq)] +enum WithdrawalCheck { + NoOutcome, + UnresolvedOutcome, + Finalized, +} + +async fn check_withdrawal( + runtime: &R, + withdrawal: &InFlightWithdrawal, + minter_address: Address, +) -> WithdrawalCheck { + let InFlightWithdrawal { + signature, + nonce_account, + nonce_value, + .. + } = withdrawal; + let read_nonce_value = match read_verified_nonce(runtime, *nonce_account, minter_address).await + { + Ok(read_nonce_value) => read_nonce_value, + Err(e) => { + let priority = if e.is_rpc_failure() { + Priority::Info + } else { + Priority::Error + }; + log!( + priority, + "Failed to read nonce account {nonce_account} of withdrawal transaction {signature}, retrying next round: {e}" + ); + return WithdrawalCheck::NoOutcome; + } + }; + let nonce_read = read_state(|state| { + state + .nonce_pool() + .classify_read(nonce_account, nonce_value, &read_nonce_value) + }); + match nonce_read { + NonceRead::Unchanged => { + if withdrawal.is_old_enough_to_rebroadcast(runtime.time()) { + rebroadcast(runtime, withdrawal).await; + } + WithdrawalCheck::NoOutcome + } + NonceRead::Stale => { + log!( + Priority::Info, + "Stale read of nonce account {nonce_account} of withdrawal transaction {signature}, retrying next round" + ); + WithdrawalCheck::NoOutcome + } + NonceRead::Advanced => finalize_landed_withdrawal(runtime, withdrawal).await, + } +} + +async fn finalize_landed_withdrawal( + runtime: &R, + withdrawal: &InFlightWithdrawal, +) -> WithdrawalCheck { + let signature = withdrawal.signature; + let event = match get_submitted_transaction_outcome(runtime, signature, &withdrawal.message) + .await + { + Ok(Some(TransactionOutcome::Succeeded)) => { + log!(Priority::Info, "Transaction {signature} finalized"); + EventType::SucceededTransaction { signature } + } + Ok(Some(TransactionOutcome::Failed(error))) => { + log!( + Priority::Error, + "Transaction {signature} finalized with on-chain error: {error:?}" + ); + EventType::FailedTransaction { signature } + } + Ok(None) => { + log!( + Priority::Info, + "Withdrawal transaction {signature} landed but was not found, retrying next round" + ); + return WithdrawalCheck::UnresolvedOutcome; + } + Err(e) => { + let priority = if e.is_response_untrustworthy() { + Priority::Error + } else { + Priority::Info + }; + log!( + priority, + "Failed to fetch landed withdrawal transaction {signature}, retrying next round: {e}" + ); + return WithdrawalCheck::UnresolvedOutcome; + } + }; + mutate_state(|state| process_event(state, event, runtime)); + WithdrawalCheck::Finalized +} + +async fn rebroadcast(runtime: &R, withdrawal: &InFlightWithdrawal) { + let signature = withdrawal.signature; + with_unstable_metrics_mut(|m| m.withdrawal_transaction_rebroadcasts += 1); + match submit_transaction_skipping_preflight(runtime, withdrawal.signed_transaction()).await { + Ok(_) => log!( + Priority::Info, + "Re-broadcast withdrawal transaction {signature}" + ), + Err(e) => log!( + Priority::Info, + "Failed to re-broadcast withdrawal transaction {signature} (will retry next round): {e}" + ), + } +} diff --git a/minter/src/rpc/mod.rs b/minter/src/rpc/mod.rs index 09eab56f6..592594536 100644 --- a/minter/src/rpc/mod.rs +++ b/minter/src/rpc/mod.rs @@ -17,12 +17,13 @@ use sol_rpc_types::{ use solana_account_decoder_client_types::UiAccount; use solana_address::Address; use solana_hash::Hash; +use solana_message::Message; use solana_nonce::{state::State as NonceState, versions::Versions as NonceVersions}; use solana_sdk_ids::system_program; use solana_signature::Signature; use solana_transaction::{Transaction, versioned::VersionedTransaction}; use solana_transaction_status_client_types::{ - EncodedConfirmedTransactionWithStatusMeta, UiTransactionStatusMeta, + EncodedConfirmedTransactionWithStatusMeta, UiTransactionError, UiTransactionStatusMeta, }; use thiserror::Error; @@ -42,6 +43,60 @@ pub async fn get_transaction( runtime: &R, signature: Signature, ) -> Result, GetTransactionError> { + let Some(outcome) = get_finalized_transaction(runtime, signature).await? else { + return Ok(None); + }; + let transaction = ensure_signed_with(&outcome, signature)?; + Ok(Some(FetchedTransaction { + transaction, + meta: outcome.transaction.meta, + })) +} + +/// The outcome of a finalized transaction. +#[derive(Debug, PartialEq)] +pub enum TransactionOutcome { + Succeeded, + Failed(UiTransactionError), +} + +/// Fetches the outcome of the finalized transaction the minter submitted with `signature` +/// and `message`. The response is attributed by comparing the returned transaction with the +/// submitted one, which makes verifying its signature redundant. +pub async fn get_submitted_transaction_outcome( + runtime: &R, + signature: Signature, + message: &Message, +) -> Result, GetTransactionError> { + let Some(outcome) = get_finalized_transaction(runtime, signature).await? else { + return Ok(None); + }; + let returned = outcome + .transaction + .transaction + .decode() + .and_then(VersionedTransaction::into_legacy_transaction); + let submitted = Transaction { + signatures: vec![signature], + message: message.clone(), + }; + if returned.as_ref() != Some(&submitted) { + return Err(GetTransactionError::UnexpectedTransaction { queried: signature }); + } + let meta = outcome + .transaction + .meta + .ok_or(GetTransactionError::MissingStatusMeta { queried: signature })?; + Ok(Some(match meta.err { + None => TransactionOutcome::Succeeded, + Some(error) => TransactionOutcome::Failed(error), + })) +} + +async fn get_finalized_transaction( + runtime: &R, + signature: Signature, +) -> Result, GetTransactionError> { let result = read_state(|state| state.sol_rpc_client(runtime.inter_canister_call_runtime())) .get_transaction(signature) .with_encoding(GetTransactionEncoding::Base64) @@ -52,14 +107,7 @@ pub async fn get_transaction( .try_send() .await; match result? { - MultiRpcResult::Consistent(Ok(Some(outcome))) => { - let transaction = ensure_signed_with(&outcome, signature)?; - Ok(Some(FetchedTransaction { - transaction, - meta: outcome.transaction.meta, - })) - } - MultiRpcResult::Consistent(Ok(None)) => Ok(None), + MultiRpcResult::Consistent(Ok(outcome)) => Ok(outcome), MultiRpcResult::Consistent(Err(e)) => Err(GetTransactionError::RpcError(e)), MultiRpcResult::Inconsistent(_) => Err(GetTransactionError::InconsistentRpcResults), } @@ -108,6 +156,12 @@ pub enum GetTransactionError { #[error("Transaction returned for {queried} is not signed by its fee payer")] #[from(ignore)] InvalidSignature { queried: Signature }, + #[error("Transaction returned for {queried} differs from the submitted one")] + #[from(ignore)] + UnexpectedTransaction { queried: Signature }, + #[error("Transaction returned for {queried} has no status metadata")] + #[from(ignore)] + MissingStatusMeta { queried: Signature }, } impl GetTransactionError { @@ -117,7 +171,9 @@ impl GetTransactionError { match self { GetTransactionError::UndecodableTransaction { .. } | GetTransactionError::SignatureMismatch { .. } - | GetTransactionError::InvalidSignature { .. } => true, + | GetTransactionError::InvalidSignature { .. } + | GetTransactionError::UnexpectedTransaction { .. } + | GetTransactionError::MissingStatusMeta { .. } => true, GetTransactionError::IcError(_) | GetTransactionError::RpcError(_) | GetTransactionError::InconsistentRpcResults => false, diff --git a/minter/src/state/mod.rs b/minter/src/state/mod.rs index df6eef830..fee71f6e2 100644 --- a/minter/src/state/mod.rs +++ b/minter/src/state/mod.rs @@ -46,7 +46,7 @@ pub use deposits::{ QueuedDeposit, SettledSweep, Sweep, SweepMismatch, SweepRecoveryError, SweepSettlementError, Sweeps, SweptDeposit, Transfer, UnreadableOutcome, }; -pub use nonce_pool::{DurableNoncePool, NoncePoolError}; +pub use nonce_pool::{DurableNoncePool, NoncePoolError, NonceRead}; thread_local! { static STATE: RefCell> = RefCell::default(); diff --git a/minter/src/state/nonce_pool/mod.rs b/minter/src/state/nonce_pool/mod.rs index 0cd0c1390..2d5654b71 100644 --- a/minter/src/state/nonce_pool/mod.rs +++ b/minter/src/state/nonce_pool/mod.rs @@ -84,6 +84,23 @@ impl DurableNoncePool { .is_some_and(|account| account.seen_nonce_values.contains(nonce_value)) } + /// Classifies a finalized read of `address` while the account is bound to an + /// in-flight transaction carrying `bound_nonce_value`. + pub fn classify_read( + &self, + address: &Address, + bound_nonce_value: &Hash, + read_nonce_value: &Hash, + ) -> NonceRead { + if read_nonce_value == bound_nonce_value { + NonceRead::Unchanged + } else if self.has_seen(address, read_nonce_value) { + NonceRead::Stale + } else { + NonceRead::Advanced + } + } + pub fn addresses(&self) -> impl Iterator { self.accounts.keys() } @@ -130,6 +147,17 @@ impl NonceAccount { } } +/// What a finalized read of a nonce account bound to an in-flight transaction proves. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum NonceRead { + /// The account still stores the bound nonce value: the transaction has not landed. + Unchanged, + /// The account stores a value bound to an earlier transaction: the response lags behind. + Stale, + /// The account stores a value the minter never bound: the transaction has landed. + Advanced, +} + /// The lifecycle state of a durable nonce account in the pool. #[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] enum NonceAccountState { diff --git a/minter/src/storage/mod.rs b/minter/src/storage/mod.rs index 5f07210d5..090d71853 100644 --- a/minter/src/storage/mod.rs +++ b/minter/src/storage/mod.rs @@ -37,6 +37,7 @@ thread_local! { pub(crate) struct Metrics { pub post_upgrade_instructions_consumed: u64, pub withdrawal_transaction_rebroadcasts: u64, + pub withdrawal_transactions_with_unresolved_outcome: u64, } impl Metrics { @@ -44,6 +45,7 @@ impl Metrics { Self { post_upgrade_instructions_consumed: 0, withdrawal_transaction_rebroadcasts: 0, + withdrawal_transactions_with_unresolved_outcome: 0, } } } diff --git a/minter/src/test_fixtures/mod.rs b/minter/src/test_fixtures/mod.rs index 46d16a42d..07d22d9c2 100644 --- a/minter/src/test_fixtures/mod.rs +++ b/minter/src/test_fixtures/mod.rs @@ -346,6 +346,76 @@ pub fn minter_signature_nth(occurrence: usize) -> solana_signature::Signature { signer::derivation_path_signature(&MINTER_DERIVATION_PATH, occurrence) } +/// The `getTransaction` response for the submitted withdrawal transaction with the given +/// signature, finalized without error. +pub fn succeeded_withdrawal_response( + signature: &solana_signature::Signature, +) -> GetTransactionResult { + withdrawal_response(signature, serde_json::Value::Null) +} + +/// The `getTransaction` response for the submitted withdrawal transaction with the given +/// signature, finalized with an on-chain error of its first transfer. +pub fn failed_withdrawal_response(signature: &solana_signature::Signature) -> GetTransactionResult { + withdrawal_response( + signature, + serde_json::json!({"InstructionError": [1, {"Custom": 1}]}), + ) +} + +fn withdrawal_response( + signature: &solana_signature::Signature, + error: serde_json::Value, +) -> GetTransactionResult { + let message = + crate::state::read_state( + |state| match state.submitted_transactions().get(signature) { + Some(crate::state::MinterTransaction::Withdrawal { + message: VersionedMessage::Legacy(message), + .. + }) => message.clone(), + other => panic!("BUG: expected a submitted withdrawal transaction, got {other:?}"), + }, + ); + let transaction = solana_transaction::Transaction { + signatures: vec![*signature], + message, + }; + let encoded = STANDARD.encode( + bincode::serialize(&transaction).expect("BUG: serializing the transaction should succeed"), + ); + let status = if error.is_null() { + serde_json::json!({"Ok": null}) + } else { + serde_json::json!({"Err": error.clone()}) + }; + let outcome: EncodedConfirmedTransactionWithStatusMeta = + serde_json::from_value(serde_json::json!({ + "slot": 350_000_000_u64, + "blockTime": 1_700_000_000_i64, + "meta": { + "err": error, + "status": status, + "fee": FEE_PER_SIGNATURE, + "preBalances": [], + "postBalances": [], + "innerInstructions": [], + "logMessages": [], + "preTokenBalances": [], + "postTokenBalances": [], + "rewards": [], + "loadedAddresses": {"readonly": [], "writable": []}, + "computeUnitsConsumed": 450 + }, + "transaction": [encoded, "base64"], + "version": "legacy" + })) + .expect("BUG: the getTransaction result should deserialize"); + MultiRpcResult::Consistent(Ok(Some(outcome.try_into().expect( + "BUG: the getTransaction result should convert to the SOL RPC canister type", + )))) +} + /// The [`FetchedTransaction`] that [`rpc::get_transaction`] returns for the given /// `getTransaction` output. pub fn fetched(outcome: EncodedConfirmedTransactionWithStatusMeta) -> FetchedTransaction { diff --git a/minter/src/withdraw/nonce/mod.rs b/minter/src/withdraw/nonce/mod.rs index 81a092bf3..3da69ea70 100644 --- a/minter/src/withdraw/nonce/mod.rs +++ b/minter/src/withdraw/nonce/mod.rs @@ -37,3 +37,24 @@ pub enum ReadNonceError { minter_address: Address, }, } + +impl ReadNonceError { + /// Whether the read failed because the RPC call did not produce an agreed response, + /// as opposed to an account that is not a nonce account of the minter. + pub fn is_rpc_failure(&self) -> bool { + match self { + ReadNonceError::GetNonceAccount( + GetNonceAccountError::IcError(_) + | GetNonceAccountError::RpcError(_) + | GetNonceAccountError::InconsistentRpcResults, + ) => true, + ReadNonceError::GetNonceAccount( + GetNonceAccountError::AccountNotFound + | GetNonceAccountError::UnexpectedAccountMetadata { .. } + | GetNonceAccountError::NotAnInitializedNonceAccount(_) + | GetNonceAccountError::LegacyNonceAccount, + ) + | ReadNonceError::ForeignAuthority { .. } => false, + } + } +} diff --git a/minter/src/withdraw/tests.rs b/minter/src/withdraw/tests.rs index 1e42ffd21..70b2a7fc8 100644 --- a/minter/src/withdraw/tests.rs +++ b/minter/src/withdraw/tests.rs @@ -340,13 +340,12 @@ mod process_pending_withdrawals_tests { create_withdrawal_batch_transaction, create_withdrawal_batch_transaction_on, submit_withdrawal_batch_transaction, }, - finalized_status, nonce_account_info, + nonce_account_info, succeeded_withdrawal_response, }, }; type GetAccountInfoResult = MultiRpcResult>; type SendTransactionResult = MultiRpcResult; - type SignatureStatusesResult = MultiRpcResult>>; #[tokio::test] async fn should_do_nothing_if_no_pending_withdrawals() { @@ -571,7 +570,9 @@ mod process_pending_withdrawals_tests { let rebroadcast = TestCanisterRuntime::new() .with_increasing_time_from(submission.time() + MIN_REBROADCAST_AGE.as_nanos() as u64) - .add_stub_response(SignatureStatusesResult::Consistent(Ok(vec![None]))) + .add_stub_response(GetAccountInfoResult::Consistent(Ok(Some( + nonce_account_info(MINTER_ADDRESS, 1), + )))) .add_stub_response(SendTransactionResult::Consistent(Ok( minter_signature().into() ))); @@ -584,9 +585,10 @@ mod process_pending_withdrawals_tests { let finalization = TestCanisterRuntime::new() .with_increasing_time() - .add_stub_response(SignatureStatusesResult::Consistent(Ok(vec![Some( - finalized_status(), - )]))); + .add_stub_response(GetAccountInfoResult::Consistent(Ok(Some( + nonce_account_info(MINTER_ADDRESS, 2), + )))) + .add_stub_response(succeeded_withdrawal_response(&minter_signature())); finalize_transactions(finalization).await; assert_eq!( withdrawal_status(1), From af440e0a3a53092e44c2dbc6c7e953f7971688f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9gory=20Demay?= Date: Wed, 7 Oct 2026 15:24:20 +0000 Subject: [PATCH 02/10] docs: finalize withdrawals from their nonce accounts Co-Authored-By: Claude Opus 5.5 --- docs/design.md | 53 ++++++++++++++++++++++++++++---------------------- 1 file changed, 30 insertions(+), 23 deletions(-) diff --git a/docs/design.md b/docs/design.md index 5e8125d15..50ccb31da 100644 --- a/docs/design.md +++ b/docs/design.md @@ -67,13 +67,13 @@ graph LR The ckSOL minter interacts with the Solana blockchain via the [SOL RPC canister](https://github.com/dfinity/sol-rpc-canister). The ckSOL minter uses the following subset of endpoints: -- [getAccountInfo](https://solana.com/docs/rpc/http/getaccountinfo): Returns the account information for the given address. This function is used to read the state of a durable nonce account before building a withdrawal transaction and to determine whether an in-flight withdrawal transaction has landed (see [Section 3.2.1](#321-durable-nonce-accounts)). +- [getAccountInfo](https://solana.com/docs/rpc/http/getaccountinfo): Returns the account information for the given address. This function is used to read the state of a durable nonce account before building a withdrawal transaction and to determine whether an in-flight withdrawal transaction has landed (see [Section 3.2.1](#321-durable-nonce-accounts) and [Section 3.2.5](#325-finalization-and-re-broadcast)). - [getBalance](https://solana.com/docs/rpc/http/getbalance): Returns the balance of the given address. This function is used by `deposit_sol` to read the balance of a deposit address at the `finalized` commitment level and determine the sweepable amount. - [getBlock](https://solana.com/docs/rpc/http/getblock): Returns the block for the given slot. This function is used to get a recent block hash, which is contained in the response. Note that `transactionDetails` is set to `null` in the request. As a result, signatures and transactions are not returned. - [getSignaturesForAddress](https://solana.com/docs/rpc/http/getsignaturesforaddress): Returns the signatures for a given address. This function is used to learn about new transactions (in Solana, signatures are used to identify transactions, as the first signature in a transaction is considered the transaction ID). -- [getSignatureStatuses](https://solana.com/docs/rpc/http/getsignaturestatuses): Returns the status of each transaction specified through its identifier, i.e., the first signature in the transaction. This function is used to learn whether a transaction has been finalized; how a missing status is handled depends on the kind of transaction, as described in [Section 3.2.5](#325-finalization-and-re-broadcast). +- [getSignatureStatuses](https://solana.com/docs/rpc/http/getsignaturestatuses): Returns the status of each transaction specified through its identifier, i.e., the first signature in the transaction. This function is used to learn whether a sweep transaction has been finalized or has expired, as described in [Section 3.2.5](#325-finalization-and-re-broadcast). Withdrawal transactions are not checked with it. - [getSlot](https://solana.com/docs/rpc/http/getslot): Returns the current slot. Since the slot number changes rapidly, the SOL RPC canister merely obtains a rounded and therefore slightly outdated slot number. The slot number is required to obtain a recent block hash using `getBlock`, whose block height is persisted with the transaction and later compared against the current block height to check if an unconfirmed transaction has expired. -- [getTransaction](https://solana.com/docs/rpc/http/gettransaction): Returns the whole transaction for the given signature. +- [getTransaction](https://solana.com/docs/rpc/http/gettransaction): Returns the whole transaction for the given signature. This function is used to read the outcome of a finalized sweep transaction and of a landed withdrawal transaction. - [sendTransaction](https://solana.com/docs/rpc/http/sendtransaction): Sends out the provided transaction. It requires the execution of the functions `getSlot` and `getBlock` to obtain a recent block hash, which must be part of the transaction, except for withdrawal transactions, which carry a durable nonce instead (see [Section 3.2.1](#321-durable-nonce-accounts)). ### 3.1. Converting SOL to ckSOL @@ -463,7 +463,7 @@ A **durable nonce account** is an account owned by the system program that store The ckSOL minter uses a **pool of durable nonce accounts** exclusively for withdrawal transactions, governed by two invariants: -1. **One in-flight transaction per nonce account**: a nonce account is bound to at most one submitted transaction at a time, and it is reused only once that transaction has been observed to have landed, at which point the account already stores a fresh nonce value. +1. **One in-flight transaction per nonce account**: a nonce account is bound to at most one submitted transaction at a time, and it is reused only once that transaction has been observed to have landed and its outcome is known, at which point the account already stores a fresh nonce value. 2. **One message per nonce value**: the ckSOL minter never signs two different messages for the same nonce account and nonce value. The ckSOL minter's main address is the nonce authority of every account in the pool. Under these invariants, the state of a nonce account, read with `getAccountInfo` at the `finalized` commitment level, is an oracle that no provider's retention policy can distort: if the account still stores the nonce value used by an in-flight withdrawal transaction, the transaction has not landed, and since it can still land, the only safe action is to re-broadcast the identical transaction. If the nonce value has advanced, the transaction has landed, since nobody else can advance the nonce and no other message was signed for that value. @@ -569,7 +569,9 @@ Skipping preflight also means that a transaction the main account cannot pay for #### 3.2.5. Finalization and Re-broadcast -The statuses of (sweep or withdrawal) transactions are checked on a timer by calling the `getSignatureStatuses` endpoint on the SOL RPC canister. The status of any accepted transaction is either `processed`, `confirmed`, or `finalized`. +Submitted transactions are monitored by the finalization timer, which decides sweeps and withdrawals differently: a sweep is decided by its status, a withdrawal by the state of its nonce account. + +**Blockhash transactions (sweeps)** are checked by calling the `getSignatureStatuses` endpoint on the SOL RPC canister, searching the transaction history. The status of any accepted transaction is either `processed`, `confirmed`, or `finalized`. ```mermaid sequenceDiagram @@ -587,17 +589,26 @@ sequenceDiagram deactivate Minter ``` -It is possible that a transaction is not accepted, i.e., it is not found in any of the statuses listed above. Since ckSOL tokens are not reimbursed, the transaction must eventually be confirmed; however, care has to be taken to ensure that there is no double spending. How a missing status is handled depends on the kind of transaction. +It is possible that a sweep is not accepted, i.e., it is not found in any of the statuses listed above. A sweep refers to a recent block hash, which may not be more than 150 blocks in the past, corresponding to roughly 90 seconds. Such a transaction is expired once the current block height exceeds its last valid block height, i.e., the block height persisted with the transaction plus 150. The current block height is read from the `getBlock` response that the finalization timer fetches whenever a sweep is in flight. Expiry is never determined by counting slots, since slots can be skipped and a transaction declared expired too early could still land. An expired sweep is dropped and never resubmitted, as described in [Section 3.1.3](#313-manual-flow). + +Sweeps remain exposed to the limited retention of the providers' transaction history: measurements showed that some providers keep only about 16 hours of history and that others answer inconsistently for older statuses, so the status of a sweep that is checked only long after it landed, e.g., because the ckSOL minter was stopped, may not reach a consensus. This is out of scope here. + +**Durable-nonce transactions (withdrawals)** never expire, and their status is not checked: since a status query is a negative query against storage with unspecified retention (see [Section 3.2.1](#321-durable-nonce-accounts)), a withdrawal whose status was pruned by the providers would remain undecided forever. Instead, on every round of the finalization timer, the ckSOL minter reads the nonce account of each in-flight withdrawal transaction with `getAccountInfo` at the `finalized` commitment level, with at most 10 concurrent calls, and classifies the returned value as described in [Section 3.2.1](#321-durable-nonce-accounts): + +- **Nonce value unchanged**: the transaction has not landed, or has not been finalized yet, and can still land. Once at least **90 seconds** have passed since it was recorded as submitted, the ckSOL minter re-broadcasts the persisted signed transaction unchanged, without preflight, as for the initial send, and without recording an event. No new threshold signature is required, and the transaction keeps its identifier. Re-signing the message would be safe, since at most one of two transactions sharing the same nonce value can land, but it would be pointless: threshold Ed25519 signing is not deterministic, so a fresh signature would merely create a second identifier to track for the same transaction. Re-signing only becomes useful to bump the transaction fee, which is not supported. +- **Nonce value seen before**: the response is stale, coming from a provider lagging behind an already observed state. No decision is taken in this round. +- **Nonce value never seen**: the transaction has landed, but with an unknown outcome, since the nonce advance persists even when a transaction fails during execution: the advanced nonce proves inclusion, not payment. The ckSOL minter resolves the outcome in the same round by fetching the transaction with `getTransaction` at the `finalized` commitment level. The response is attributed to the withdrawal by checking that it carries exactly the signed transaction the ckSOL minter submitted. The transaction transitions to `Succeeded` if the error field of its metadata is `null`, and to `Failed` otherwise, which releases its nonce account. If `getTransaction` returns `null`, fails, or yields inconsistent results, nothing is recorded and the read is repeated in the next round. +- **Read failure**: no decision is taken in this round. A failing RPC call is expected to be transient, while an account that is no longer a nonce account of the ckSOL minter, after it was already bound, is unexpected and reported as an error. -**Blockhash transactions (sweeps)** refer to a recent block hash, which may not be more than 150 blocks in the past, corresponding to roughly 90 seconds. Such a transaction is expired once the current block height exceeds its last valid block height, i.e., the block height persisted with the transaction plus 150. The current block height is read from the `getBlock` response that the finalization timer already fetches, so no additional call is required. Expiry is never determined by counting slots, since slots can be skipped and a transaction declared expired too early could still land. An expired sweep is dropped and never resubmitted, as described in [Section 3.1.3](#313-manual-flow). +The minimum age before a re-broadcast leaves the retries to the RPC node first: since the ckSOL minter sends transactions without `maxRetries`, an Agave RPC node keeps re-sending a durable-nonce transaction every 2 seconds until it lands, its nonce advances, or 150 blocks (about 60 to 90 seconds) pass, so an earlier re-broadcast would only duplicate that work. The submission time is taken from the timestamp of the `SubmittedTransaction` event, so it survives upgrades. Re-broadcasting covers a transaction whose initial send failed as well as one dropped before landing, and it is always safe: all copies carry the same nonce value, so whichever lands first advances the nonce and all others fail the nonce check. A failed re-broadcast is retried in the next round. Re-broadcasting has no backoff and no budget: each round adds one `sendTransaction` call per withdrawal transaction whose nonce is unchanged, i.e., at most one call per transaction every 2 minutes, and the number of such transactions is bounded by the size of the nonce pool. This cost is not covered by the withdrawal fee of [Section 3.3.2](#332-cksol-minter-fees) and is accepted, since a transaction stays unlanded only while it fails to land, which should be rare and is visible through the re-broadcast metric. -**Durable-nonce transactions (withdrawals)** never expire, so the notion of expiry does not apply to them. When the status of a withdrawal transaction is missing, and at least **90 seconds** have passed since it was recorded as submitted, the ckSOL minter re-broadcasts the persisted signed transaction unchanged in the same round of the finalization timer, without preflight, as for the initial send, and without recording an event. The minimum age leaves the retries to the RPC node first: since the ckSOL minter sends transactions without `maxRetries`, an Agave RPC node keeps re-sending a durable-nonce transaction every 2 seconds until it lands, its nonce advances, or 150 blocks (about 60 to 90 seconds) pass, so an earlier re-broadcast would only duplicate that work. The submission time is taken from the timestamp of the `SubmittedTransaction` event, so it survives upgrades. This covers a transaction whose initial send failed as well as one dropped before landing, and it is always safe: all copies carry the same nonce value, so whichever lands first advances the nonce and all others fail the nonce check. A failed re-broadcast is retried in the next round. Re-broadcasting has no backoff and no budget: each round adds one `sendTransaction` call per withdrawal transaction still without status, i.e., at most one call per transaction every 2 minutes, and the number of such transactions is bounded by the size of the nonce pool. This cost is not covered by the withdrawal fee of [Section 3.3.2](#332-cksol-minter-fees) and is accepted, since a transaction stays without status only while it fails to land, which should be rare and is visible through the re-broadcast metric. The ckSOL minter additionally reads the nonce account with `getAccountInfo` at the `finalized` commitment level and classifies the returned value as described in [Section 3.2.1](#321-durable-nonce-accounts): +The nonce account stays bound to a landed withdrawal transaction until `getTransaction` has resolved its outcome; there is no intermediate state in which the account is released while the outcome is still unknown. This keeps the state machine of a withdrawal transaction unchanged, i.e., `Submitted` until it is `Succeeded` or `Failed`, and it adds a second, independent confirmation of the landing: a consistent but bogus nonce read alone, e.g., agreed upon by misbehaving providers, cannot finalize a withdrawal, release its nonce account for reuse, or report it to the user, since the providers must additionally return the exact submitted transaction as finalized. Unlike a status query, `getTransaction` is answered consistently by the default providers for transactions that landed several months ago, and it is only called once the nonce advance has proven that the transaction exists. While the outcome is unresolved, `withdrawal_status` keeps reporting `TxSent`, and the `getTransaction` call is retried on every round without a backoff or retry budget. The number of withdrawal transactions whose nonce advanced but whose outcome could not be fetched in the last round is exposed as a metric. A withdrawal transaction that resolves to an on-chain error follows the regular failure handling described below: the transaction is stored whole and the burned ckSOL is not reimbursed, as there is no reimbursement flow. -- **Nonce value unchanged**: the transaction has not landed and can still land. The transaction keeps being re-broadcast unchanged. No new threshold signature is required, and the transaction keeps its identifier. Re-signing the message would be safe, since at most one of two transactions sharing the same nonce value can land, but it would be pointless: threshold Ed25519 signing is not deterministic, so a fresh signature would merely create a second identifier to track for the same transaction. Re-signing only becomes useful to bump the transaction fee, which is not supported. -- **Nonce value seen before**: the response is stale, coming from a provider lagging behind an already observed state. No decision is taken, and the transaction keeps being re-broadcast. -- **Nonce value never seen**: the transaction has landed, but with an unknown outcome, since the nonce advance persists even when a transaction fails during execution: the advanced nonce proves inclusion, not payment. The ckSOL minter resolves the outcome with `getTransaction`. Should no provider return the transaction anymore because of its retention policy, the withdrawal remains in an unresolved state; no double pay is possible either way, since a transaction whose nonce has advanced is never submitted again. +The following residual risks are accepted: -Entering the `Landed` state releases the transaction's nonce account: the nonce advance is finalized, so the account already stores a fresh value and can serve new withdrawals while the outcome is still being resolved, and unresolved outcomes therefore never reduce the pool capacity. While the outcome of a landed withdrawal transaction is unresolved, `withdrawal_status` keeps reporting `TxSent`, and changes to `TxFinalized` only once the outcome is known. The `getTransaction` call is retried on every round of the finalization timer until it succeeds, without a backoff or retry budget: the cost of retrying indefinitely is accepted, since an outcome stays unresolved only while every provider has pruned the transaction, which should be rare. The number of unresolved transactions, together with the age of the oldest one, is exposed as a metric. A withdrawal transaction that resolves to an on-chain error follows the regular failure handling described below: the transaction is stored whole and the burned ckSOL is not reimbursed, as there is no reimbursement flow. +1. If `getTransaction` cannot reach a consensus, e.g., because the default providers of the SOL RPC canister change to providers with a short history while the ckSOL minter is stopped for a long time, the outcome of a landed withdrawal stays unresolved and its nonce account stays bound. This shows up in the age of the oldest incomplete withdrawal and requires an operator to intervene, e.g., by configuring other providers. +2. A transaction rejected before inclusion never advances its nonce and keeps being re-broadcast, e.g., when the fee payer cannot pay the transaction fee. Holding back the rent exemption threshold from the available balance prevents this case, as described in [Section 3.2.4](#324-submitting-withdrawal-requests). +3. A transaction that is processed or confirmed but not yet finalized still shows its nonce unchanged at the `finalized` commitment level and may be re-broadcast needlessly. This is harmless and rare, since a transaction is only re-broadcast 90 seconds after its submission, while finalization takes about 13 seconds. The different states and their transitions internal to the ckSOL minter are shown in the following figure. @@ -605,27 +616,23 @@ The different states and their transitions internal to the ckSOL minter are show stateDiagram-v2 Withdrawal --> Submitted Sweep --> Submitted - Submitted --> Succeeded: confirmation_status = finalized and err = null - Submitted --> Failed: confirmation_status = finalized and err != null - Submitted --> Submitted: withdrawal not found and nonce unchanged (re-broadcast) - Submitted --> Landed: withdrawal not found and nonce advanced - Landed --> Succeeded: getTransaction with err = null - Landed --> Failed: getTransaction with err != null + Submitted --> Succeeded: sweep finalized with err = null, or withdrawal nonce advanced and getTransaction with err = null + Submitted --> Failed: sweep finalized with err != null, or withdrawal nonce advanced and getTransaction with err != null + Submitted --> Submitted: withdrawal nonce unchanged (re-broadcast) Submitted --> Dropped: sweep expired Succeeded: Succeeded (store transaction_id) Failed: Failed (store whole transaction) - Landed: Landed (outcome unresolved) Dropped: Dropped (deposits marked dropped, no resubmission) ``` -The withdrawal and sweep flows result in the submission of a transaction, which is then in the `Submitted` state. The status of submitted transactions is checked on a timer as outlined above. If a transaction reaches the confirmation status `finalized`, there are two cases: If the transaction was finalized successfully, i.e., without errors, the transaction transitions to the state `Succeeded` and its ID is stored permanently. If there was an error, the transaction transitions to the state `Failed` and is stored in its entirety so that it can be analyzed what happened. Ideally, no transaction ever ends up in this state. However, it is possible for transactions to fail, for example by attempting to withdraw SOL to a program account, which is not allowed. As there is no reimbursement flow, the user's funds would be stuck in this case. Storing the whole failed transaction ensures that the funds are not lost and appropriate actions may be taken when such transactions are encountered. +The withdrawal and sweep flows result in the submission of a transaction, which is then in the `Submitted` state and monitored on a timer as outlined above. Once a transaction is known to be finalized, there are two cases: If the transaction was finalized successfully, i.e., without errors, the transaction transitions to the state `Succeeded` and its ID is stored permanently. If there was an error, the transaction transitions to the state `Failed` and is stored in its entirety so that it can be analyzed what happened. Ideally, no transaction ever ends up in this state. However, it is possible for transactions to fail, for example by attempting to withdraw SOL to a program account, which is not allowed. As there is no reimbursement flow, the user's funds would be stuck in this case. Storing the whole failed transaction ensures that the funds are not lost and appropriate actions may be taken when such transactions are encountered. The states above describe the lifecycle of the transaction itself, not the crediting of the deposits it carries. When a sweep transaction of the manual flow reaches `Succeeded`, the ckSOL minter additionally records its deposits as finalized, fetches the transaction to read the fee that was charged, runs the sanity check on the pre- and post-balances, and enqueues the pending mints, as described in [Section 3.1.3](#313-manual-flow). -An expired sweep is never resubmitted but dropped, as described in [Section 3.1.3](#313-manual-flow), since nothing has been minted for it yet and the user can simply queue a new sweep. A withdrawal transaction with a missing status and an unchanged nonce stays in the `Submitted` state, and the finalization timer re-broadcasts the persisted signed transaction as-is. +An expired sweep is never resubmitted but dropped, as described in [Section 3.1.3](#313-manual-flow), since nothing has been minted for it yet and the user can simply queue a new sweep. A withdrawal transaction with an unchanged nonce stays in the `Submitted` state, and the finalization timer re-broadcasts the persisted signed transaction as-is. -Since withdrawal transactions never expire, a transaction that persistently fails to land occupies its nonce account indefinitely. Re-broadcasting cures the causes that previously led to expiry, such as temporary RPC outages or transactions dropped on the way to a leader, and since preflight is skipped, a transaction whose transfer cannot succeed lands and fails on-chain rather than being silently filtered out by the RPC providers, so under normal conditions every withdrawal transaction eventually lands, successfully or not. The age of the oldest in-flight withdrawal transaction and the number of re-broadcast attempts are exposed as metrics to detect the abnormal case. Deliberately abandoning a stuck transaction would require the ckSOL minter to advance the nonce itself with a separate transaction, which races the stuck transaction; this is left out of scope as a manual, operator-driven procedure. +Since withdrawal transactions never expire, a transaction that persistently fails to land occupies its nonce account indefinitely. Re-broadcasting cures the causes that previously led to expiry, such as temporary RPC outages or transactions dropped on the way to a leader, and since preflight is skipped, a transaction whose transfer cannot succeed lands and fails on-chain rather than being silently filtered out by the RPC providers, so under normal conditions every withdrawal transaction eventually lands, successfully or not. The age of the oldest incomplete withdrawal and the number of re-broadcast attempts are exposed as metrics to detect the abnormal case. Deliberately abandoning a stuck transaction would require the ckSOL minter to advance the nonce itself with a separate transaction, which races the stuck transaction; this is left out of scope as a manual, operator-driven procedure. A prioritization fee is not required under normal load, therefore it is omitted. If the need arises to bump fees, this topic will be revisited. @@ -694,7 +701,7 @@ The **manual deposit fee** is not a parameter but the depositor's share of the s When a deposit address holding x SOL is swept for the first time, the user receives x SOL minus the rent exemption threshold minus the fee share in their account. Later sweeps of the same address only deduct the fee share, since the threshold is already in place. -The **withdrawal fee** can be lower, as it only requires the execution of the withdrawal transaction submission flow, i.e., making one `getAccountInfo` and `sendTransaction` call, followed by a `getSignatureStatuses` call, for a total cost of 6.4B cycles, which corresponds to 0.0064 XDR = 0.009216 USD = 0.00009216 SOL. Adding the threshold signature cost of 0.000377 SOL, the total cost is 0.00046916 SOL. Rounding up, the withdrawal fee could be set to **0.001 SOL**. Re-broadcasts of a withdrawal transaction that has no status after its initial send are not covered by this fee: the ckSOL minter accepts their cost, as described in [Section 3.2.5](#325-finalization-and-re-broadcast). +The **withdrawal fee** can be lower, as it only requires the execution of the withdrawal transaction submission flow and its finalization, i.e., per withdrawal transaction one `getAccountInfo` call to read the nonce account at creation, one `sendTransaction` call, one `getAccountInfo` call per round of the finalization timer until the transaction has landed, typically a single one since a round runs every 2 minutes and a transaction finalizes within seconds, and one `getTransaction` call to read its outcome, for a total cost of 2.1B + 2.2B + 2.1B + 7.5B = 13.9B cycles, which corresponds to 0.0139 XDR = 0.020016 USD = 0.00020016 SOL. Adding the threshold signature cost of 0.000377 SOL, the total cost is 0.00057716 SOL. Rounding up, the withdrawal fee of **0.001 SOL** remains sufficient, even though it is charged per withdrawal while the costs above are incurred per withdrawal transaction, which batches up to 10 withdrawals. Re-broadcasts of a withdrawal transaction that has not landed, together with the additional nonce reads they imply, and retries of the `getTransaction` call are not covered by this fee: the ckSOL minter accepts their cost, as described in [Section 3.2.5](#325-finalization-and-re-broadcast). When the user withdraws x SOL, the user receives x SOL minus the withdrawal fee in the destination account. From b848d1094b07c7175859df6f3495f03e861877d5 Mon Sep 17 00:00:00 2001 From: gregorydemay Date: Thu, 8 Oct 2026 09:50:43 +0200 Subject: [PATCH 03/10] refactor(minter): split withdrawal monitoring into stages Read the nonce accounts of in-flight withdrawals once, then re-broadcast the transactions that have not landed and finalize the ones that did, as independent stages of the finalization timer. Co-Authored-By: Claude Opus 5.5 (1M context) --- minter/src/monitor/mod.rs | 8 +- minter/src/monitor/withdrawals.rs | 122 ++++++++++++++++++++---------- 2 files changed, 88 insertions(+), 42 deletions(-) diff --git a/minter/src/monitor/mod.rs b/minter/src/monitor/mod.rs index b59b53750..cd1d3562a 100644 --- a/minter/src/monitor/mod.rs +++ b/minter/src/monitor/mod.rs @@ -2,7 +2,9 @@ use crate::{ constants::MAX_CONCURRENT_RPC_CALLS, deposit::sweep::credit_finalized_sweeps, guard::TimerGuard, - monitor::withdrawals::check_withdrawal_transactions, + monitor::withdrawals::{ + finalize_transactions_batch, read_in_flight_nonces, resubmit_transactions_batch, + }, rpc::{BlockHeight, get_recent_block, get_signature_statuses}, runtime::CanisterRuntime, state::{ @@ -58,7 +60,9 @@ pub async fn finalize_transactions(runtime: R) { }); let more_transactions_to_check = check_sweep_transactions(&runtime).await; - check_withdrawal_transactions(&runtime).await; + let nonces = read_in_flight_nonces(&runtime).await; + resubmit_transactions_batch(&runtime, &nonces).await; + finalize_transactions_batch(&runtime, &nonces).await; let more_sweeps_to_credit = credit_finalized_sweeps(&runtime).await; if !more_transactions_to_check && !more_sweeps_to_credit { diff --git a/minter/src/monitor/withdrawals.rs b/minter/src/monitor/withdrawals.rs index ff2e2e003..e7968d564 100644 --- a/minter/src/monitor/withdrawals.rs +++ b/minter/src/monitor/withdrawals.rs @@ -18,6 +18,7 @@ use crate::{ }; use canlog::log; use cksol_types_internal::log::Priority; +use itertools::Itertools; use solana_address::Address; use solana_hash::Hash; use solana_message::Message; @@ -25,25 +26,76 @@ use solana_signature::Signature; use solana_transaction::Transaction; use std::time::Duration; -/// Decides every in-flight withdrawal transaction from a finalized read of its nonce -/// account: a landed transaction is finalized with the outcome `getTransaction` reports, -/// and a transaction that has not landed is re-broadcast once old enough. -pub(super) async fn check_withdrawal_transactions(runtime: &R) { +#[derive(Default)] +pub(super) struct InFlightNonces { + unchanged: Vec, + advanced: Vec, +} + +pub(super) async fn read_in_flight_nonces(runtime: &R) -> InFlightNonces { + let mut nonces = InFlightNonces::default(); let Some(minter_address) = read_state(|state| state.minter_public_key().map(minter_address)) else { - return; + return nonces; }; - let withdrawals = in_flight_withdrawals(); + let batches: Vec> = in_flight_withdrawals() + .into_iter() + .chunks(MAX_CONCURRENT_RPC_CALLS) + .into_iter() + .map(Iterator::collect) + .collect(); + for batch in batches { + let reads = futures::future::join_all( + batch + .into_iter() + .map(|withdrawal| read_nonce(runtime, withdrawal, minter_address)), + ) + .await; + for (withdrawal, nonce_read) in reads { + match nonce_read { + Some(NonceRead::Unchanged) => nonces.unchanged.push(withdrawal), + Some(NonceRead::Advanced) => nonces.advanced.push(withdrawal), + Some(NonceRead::Stale) | None => {} + } + } + } + nonces +} + +pub(super) async fn resubmit_transactions_batch( + runtime: &R, + nonces: &InFlightNonces, +) { + let now = runtime.time(); + let to_rebroadcast: Vec<_> = nonces + .unchanged + .iter() + .filter(|withdrawal| withdrawal.is_old_enough_to_rebroadcast(now)) + .collect(); + for batch in to_rebroadcast.chunks(MAX_CONCURRENT_RPC_CALLS) { + futures::future::join_all( + batch + .iter() + .map(|withdrawal| rebroadcast(runtime, withdrawal)), + ) + .await; + } +} + +pub(super) async fn finalize_transactions_batch( + runtime: &R, + nonces: &InFlightNonces, +) { let mut num_unresolved_outcomes = 0; - for batch in withdrawals.chunks(MAX_CONCURRENT_RPC_CALLS) { + for batch in nonces.advanced.chunks(MAX_CONCURRENT_RPC_CALLS) { num_unresolved_outcomes += futures::future::join_all( batch .iter() - .map(|withdrawal| check_withdrawal(runtime, withdrawal, minter_address)), + .map(|withdrawal| finalize_landed_withdrawal(runtime, withdrawal)), ) .await .into_iter() - .filter(|check| *check == WithdrawalCheck::UnresolvedOutcome) + .filter(|finalization| *finalization == Finalization::UnresolvedOutcome) .count(); } with_unstable_metrics_mut(|metrics| { @@ -96,24 +148,17 @@ fn in_flight_withdrawals() -> Vec { }) } -#[derive(PartialEq)] -enum WithdrawalCheck { - NoOutcome, - UnresolvedOutcome, - Finalized, -} - -async fn check_withdrawal( +async fn read_nonce( runtime: &R, - withdrawal: &InFlightWithdrawal, + withdrawal: InFlightWithdrawal, minter_address: Address, -) -> WithdrawalCheck { +) -> (InFlightWithdrawal, Option) { let InFlightWithdrawal { signature, nonce_account, nonce_value, .. - } = withdrawal; + } = &withdrawal; let read_nonce_value = match read_verified_nonce(runtime, *nonce_account, minter_address).await { Ok(read_nonce_value) => read_nonce_value, @@ -127,7 +172,7 @@ async fn check_withdrawal( priority, "Failed to read nonce account {nonce_account} of withdrawal transaction {signature}, retrying next round: {e}" ); - return WithdrawalCheck::NoOutcome; + return (withdrawal, None); } }; let nonce_read = read_state(|state| { @@ -135,28 +180,25 @@ async fn check_withdrawal( .nonce_pool() .classify_read(nonce_account, nonce_value, &read_nonce_value) }); - match nonce_read { - NonceRead::Unchanged => { - if withdrawal.is_old_enough_to_rebroadcast(runtime.time()) { - rebroadcast(runtime, withdrawal).await; - } - WithdrawalCheck::NoOutcome - } - NonceRead::Stale => { - log!( - Priority::Info, - "Stale read of nonce account {nonce_account} of withdrawal transaction {signature}, retrying next round" - ); - WithdrawalCheck::NoOutcome - } - NonceRead::Advanced => finalize_landed_withdrawal(runtime, withdrawal).await, + if nonce_read == NonceRead::Stale { + log!( + Priority::Info, + "Stale read of nonce account {nonce_account} of withdrawal transaction {signature}, retrying next round" + ); } + (withdrawal, Some(nonce_read)) +} + +#[derive(PartialEq)] +enum Finalization { + Finalized, + UnresolvedOutcome, } async fn finalize_landed_withdrawal( runtime: &R, withdrawal: &InFlightWithdrawal, -) -> WithdrawalCheck { +) -> Finalization { let signature = withdrawal.signature; let event = match get_submitted_transaction_outcome(runtime, signature, &withdrawal.message) .await @@ -177,7 +219,7 @@ async fn finalize_landed_withdrawal( Priority::Info, "Withdrawal transaction {signature} landed but was not found, retrying next round" ); - return WithdrawalCheck::UnresolvedOutcome; + return Finalization::UnresolvedOutcome; } Err(e) => { let priority = if e.is_response_untrustworthy() { @@ -189,11 +231,11 @@ async fn finalize_landed_withdrawal( priority, "Failed to fetch landed withdrawal transaction {signature}, retrying next round: {e}" ); - return WithdrawalCheck::UnresolvedOutcome; + return Finalization::UnresolvedOutcome; } }; mutate_state(|state| process_event(state, event, runtime)); - WithdrawalCheck::Finalized + Finalization::Finalized } async fn rebroadcast(runtime: &R, withdrawal: &InFlightWithdrawal) { From 43a1f5569da1329b67edac8e28fec3985f94ba95 Mon Sep 17 00:00:00 2001 From: gregorydemay Date: Thu, 8 Oct 2026 09:58:09 +0200 Subject: [PATCH 04/10] feat(minter): delay the rescheduled finalization round Co-Authored-By: Claude Opus 5.5 (1M context) --- minter/src/monitor/mod.rs | 3 ++- minter/src/monitor/tests.rs | 9 ++++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/minter/src/monitor/mod.rs b/minter/src/monitor/mod.rs index cd1d3562a..c77d1a9d4 100644 --- a/minter/src/monitor/mod.rs +++ b/minter/src/monitor/mod.rs @@ -25,6 +25,7 @@ mod tests; mod withdrawals; pub const FINALIZE_TRANSACTIONS_DELAY: Duration = Duration::from_mins(2); +pub const FINALIZE_TRANSACTIONS_RETRY_DELAY: Duration = Duration::from_secs(10); /// Minimum time since its submission before a withdrawal transaction whose nonce /// account still stores its nonce value is re-broadcast. The minter sends /// transactions without `maxRetries`, so an Agave RPC node keeps re-sending a @@ -56,7 +57,7 @@ pub async fn finalize_transactions(runtime: R) { }; let reschedule = scopeguard::guard(runtime.clone(), |runtime| { - runtime.set_timer(Duration::ZERO, finalize_transactions); + runtime.set_timer(FINALIZE_TRANSACTIONS_RETRY_DELAY, finalize_transactions); }); let more_transactions_to_check = check_sweep_transactions(&runtime).await; diff --git a/minter/src/monitor/tests.rs b/minter/src/monitor/tests.rs index 99c9cd86a..48f42e14b 100644 --- a/minter/src/monitor/tests.rs +++ b/minter/src/monitor/tests.rs @@ -1,6 +1,6 @@ use super::{ - MAX_BLOCKHASH_AGE_IN_BLOCKS, MAX_SIGNATURES_PER_STATUS_CHECK, MIN_REBROADCAST_AGE, - finalize_transactions, + FINALIZE_TRANSACTIONS_RETRY_DELAY, MAX_BLOCKHASH_AGE_IN_BLOCKS, + MAX_SIGNATURES_PER_STATUS_CHECK, MIN_REBROADCAST_AGE, finalize_transactions, }; use crate::{ constants::MAX_CONCURRENT_RPC_CALLS, @@ -125,7 +125,10 @@ mod finalization { .count(), MAX_CONCURRENT_RPC_CALLS ); - assert_eq!(runtime.set_timer_call_count(), 1); + assert_eq!( + runtime.set_timer_delays(), + vec![FINALIZE_TRANSACTIONS_RETRY_DELAY] + ); } #[tokio::test] From 35d513fe0c25e8007c316e048f33ffc162e2dc0f Mon Sep 17 00:00:00 2001 From: gregorydemay Date: Thu, 8 Oct 2026 10:27:08 +0200 Subject: [PATCH 05/10] refactor(minter): finalize withdrawals with getTransaction Fetch the outcome of a landed withdrawal with the same getTransaction call as sweeps, which attributes the response by verifying its signature, instead of a dedicated wrapper comparing it with the submitted message. Tests that finalize withdrawals now sign them with the real test key already used for sweeps. Co-Authored-By: Claude Opus 5.5 (1M context) --- minter/src/monitor/tests.rs | 51 ++++++++++++++--------- minter/src/monitor/withdrawals.rs | 34 +++++++++------- minter/src/rpc/mod.rs | 53 +----------------------- minter/src/test_fixtures/mod.rs | 67 ++++++++++++++++++++++++++++--- minter/src/withdraw/tests.rs | 34 ++++++++++------ 5 files changed, 138 insertions(+), 101 deletions(-) diff --git a/minter/src/monitor/tests.rs b/minter/src/monitor/tests.rs index 48f42e14b..17ef0661f 100644 --- a/minter/src/monitor/tests.rs +++ b/minter/src/monitor/tests.rs @@ -3,6 +3,7 @@ use super::{ MAX_SIGNATURES_PER_STATUS_CHECK, MIN_REBROADCAST_AGE, finalize_transactions, }; use crate::{ + address::minter_address, constants::MAX_CONCURRENT_RPC_CALLS, rpc::BlockHeight, state::{ @@ -12,11 +13,10 @@ use crate::{ }, storage::{reset_events, with_unstable_metrics}, test_fixtures::{ - EventsAssert, GetTransactionResult, MINIMUM_WITHDRAWAL_AMOUNT, MINTER_ADDRESS, - NONCE_ACCOUNT, account, confirmed_block_at_height, durable_nonce, events, - failed_withdrawal_response, finalized_status, init_balance, init_schnorr_master_key, - init_state, nonce_account_info, runtime::TestCanisterRuntime, signature, - succeeded_withdrawal_response, + EventsAssert, GetTransactionResult, MINIMUM_WITHDRAWAL_AMOUNT, NONCE_ACCOUNT, account, + confirmed_block_at_height, devnet_sweep, durable_nonce, events, failed_withdrawal_response, + finalized_status, init_balance, init_schnorr_master_key, init_state, nonce_account_info, + runtime::TestCanisterRuntime, signature, succeeded_withdrawal_response, }, }; use sol_rpc_types::{ @@ -386,8 +386,8 @@ mod withdrawal_finalization { #[tokio::test] async fn should_record_a_withdrawal_as_succeeded_once_its_nonce_advanced() { - setup(); - let signature = submit_withdrawal_bound_to(1, BOUND_NONCE_SEED); + setup_with_signing_key(); + let signature = submit_signed_withdrawal_bound_to(1, BOUND_NONCE_SEED); let runtime = TestCanisterRuntime::new() .with_increasing_time() @@ -411,8 +411,8 @@ mod withdrawal_finalization { #[tokio::test] async fn should_record_a_withdrawal_as_failed_once_its_nonce_advanced_with_an_error() { - setup(); - let signature = submit_withdrawal_bound_to(1, BOUND_NONCE_SEED); + setup_with_signing_key(); + let signature = submit_signed_withdrawal_bound_to(1, BOUND_NONCE_SEED); let runtime = TestCanisterRuntime::new() .with_increasing_time() @@ -432,8 +432,8 @@ mod withdrawal_finalization { #[tokio::test] async fn should_rebroadcast_a_withdrawal_with_an_unchanged_nonce_unchanged() { - setup(); - let signature = submit_withdrawal_bound_to(1, BOUND_NONCE_SEED); + setup_with_signing_key(); + let signature = submit_signed_withdrawal_bound_to(1, BOUND_NONCE_SEED); let events_before = EventsAssert::from_recorded(); let runtime = TestCanisterRuntime::new() @@ -456,8 +456,8 @@ mod withdrawal_finalization { #[tokio::test] async fn should_rebroadcast_a_withdrawal_with_an_unchanged_nonce_only_after_the_minimum_age() { - setup(); - let signature = submit_withdrawal_bound_to(1, BOUND_NONCE_SEED); + setup_with_signing_key(); + let signature = submit_signed_withdrawal_bound_to(1, BOUND_NONCE_SEED); let too_early = TestCanisterRuntime::new() .with_increasing_time() @@ -480,10 +480,10 @@ mod withdrawal_finalization { #[tokio::test] async fn should_take_no_decision_on_a_stale_nonce_read() { - setup(); - let landed = submit_withdrawal_bound_to(1, STALE_NONCE_SEED); + setup_with_signing_key(); + let landed = submit_signed_withdrawal_bound_to(1, STALE_NONCE_SEED); events::succeed_transaction(landed); - let signature = submit_withdrawal_bound_to(2, BOUND_NONCE_SEED); + let signature = submit_signed_withdrawal_bound_to(2, BOUND_NONCE_SEED); let events_before = EventsAssert::from_recorded(); let runtime = TestCanisterRuntime::new() @@ -499,8 +499,8 @@ mod withdrawal_finalization { #[tokio::test] async fn should_keep_the_nonce_account_bound_until_the_outcome_is_known() { - setup(); - let signature = submit_withdrawal_bound_to(1, BOUND_NONCE_SEED); + setup_with_signing_key(); + let signature = submit_signed_withdrawal_bound_to(1, BOUND_NONCE_SEED); let events_before = EventsAssert::from_recorded(); let runtime = TestCanisterRuntime::new() @@ -579,8 +579,16 @@ fn submit_withdrawal_bound_to(i: usize, nonce_seed: usize) -> solana_signature:: signature } +fn submit_signed_withdrawal_bound_to(i: usize, nonce_seed: usize) -> solana_signature::Signature { + events::accept_withdrawal(account(i), i as u64, MINIMUM_WITHDRAWAL_AMOUNT); + events::create_withdrawal_batch_transaction(durable_nonce(nonce_seed), vec![i as u64]); + events::submit_signed_withdrawal_batch_transaction(durable_nonce(nonce_seed), vec![i as u64]) +} + fn nonce_read(nonce_seed: usize) -> GetAccountInfoResult { - GetAccountInfoResult::Consistent(Ok(Some(nonce_account_info(MINTER_ADDRESS, nonce_seed)))) + let minter_address = read_state(|s| s.minter_public_key().map(minter_address)) + .expect("the minter public key is cached"); + GetAccountInfoResult::Consistent(Ok(Some(nonce_account_info(minter_address, nonce_seed)))) } fn called_methods(runtime: &TestCanisterRuntime) -> Vec { @@ -597,6 +605,11 @@ fn setup() { init_schnorr_master_key(); } +fn setup_with_signing_key() { + init_state(); + devnet_sweep::init_balance(); +} + fn current_block() -> ConfirmedBlock { confirmed_block_at_height(CURRENT_BLOCK_HEIGHT) } diff --git a/minter/src/monitor/withdrawals.rs b/minter/src/monitor/withdrawals.rs index e7968d564..e29148b85 100644 --- a/minter/src/monitor/withdrawals.rs +++ b/minter/src/monitor/withdrawals.rs @@ -2,10 +2,7 @@ use crate::{ address::minter_address, constants::MAX_CONCURRENT_RPC_CALLS, monitor::MIN_REBROADCAST_AGE, - rpc::{ - TransactionOutcome, get_submitted_transaction_outcome, - submit_transaction_skipping_preflight, - }, + rpc::{FetchedTransaction, get_transaction, submit_transaction_skipping_preflight}, runtime::CanisterRuntime, state::{ MinterTransaction, NonceRead, @@ -200,19 +197,28 @@ async fn finalize_landed_withdrawal( withdrawal: &InFlightWithdrawal, ) -> Finalization { let signature = withdrawal.signature; - let event = match get_submitted_transaction_outcome(runtime, signature, &withdrawal.message) - .await - { - Ok(Some(TransactionOutcome::Succeeded)) => { - log!(Priority::Info, "Transaction {signature} finalized"); - EventType::SucceededTransaction { signature } - } - Ok(Some(TransactionOutcome::Failed(error))) => { + let event = match get_transaction(runtime, signature).await { + Ok(Some(FetchedTransaction { + meta: Some(meta), .. + })) => match meta.err { + None => { + log!(Priority::Info, "Transaction {signature} finalized"); + EventType::SucceededTransaction { signature } + } + Some(error) => { + log!( + Priority::Error, + "Transaction {signature} finalized with on-chain error: {error:?}" + ); + EventType::FailedTransaction { signature } + } + }, + Ok(Some(FetchedTransaction { meta: None, .. })) => { log!( Priority::Error, - "Transaction {signature} finalized with on-chain error: {error:?}" + "Withdrawal transaction {signature} landed without status metadata, retrying next round" ); - EventType::FailedTransaction { signature } + return Finalization::UnresolvedOutcome; } Ok(None) => { log!( diff --git a/minter/src/rpc/mod.rs b/minter/src/rpc/mod.rs index 592594536..475c8c22d 100644 --- a/minter/src/rpc/mod.rs +++ b/minter/src/rpc/mod.rs @@ -17,13 +17,12 @@ use sol_rpc_types::{ use solana_account_decoder_client_types::UiAccount; use solana_address::Address; use solana_hash::Hash; -use solana_message::Message; use solana_nonce::{state::State as NonceState, versions::Versions as NonceVersions}; use solana_sdk_ids::system_program; use solana_signature::Signature; use solana_transaction::{Transaction, versioned::VersionedTransaction}; use solana_transaction_status_client_types::{ - EncodedConfirmedTransactionWithStatusMeta, UiTransactionError, UiTransactionStatusMeta, + EncodedConfirmedTransactionWithStatusMeta, UiTransactionStatusMeta, }; use thiserror::Error; @@ -53,46 +52,6 @@ pub async fn get_transaction( })) } -/// The outcome of a finalized transaction. -#[derive(Debug, PartialEq)] -pub enum TransactionOutcome { - Succeeded, - Failed(UiTransactionError), -} - -/// Fetches the outcome of the finalized transaction the minter submitted with `signature` -/// and `message`. The response is attributed by comparing the returned transaction with the -/// submitted one, which makes verifying its signature redundant. -pub async fn get_submitted_transaction_outcome( - runtime: &R, - signature: Signature, - message: &Message, -) -> Result, GetTransactionError> { - let Some(outcome) = get_finalized_transaction(runtime, signature).await? else { - return Ok(None); - }; - let returned = outcome - .transaction - .transaction - .decode() - .and_then(VersionedTransaction::into_legacy_transaction); - let submitted = Transaction { - signatures: vec![signature], - message: message.clone(), - }; - if returned.as_ref() != Some(&submitted) { - return Err(GetTransactionError::UnexpectedTransaction { queried: signature }); - } - let meta = outcome - .transaction - .meta - .ok_or(GetTransactionError::MissingStatusMeta { queried: signature })?; - Ok(Some(match meta.err { - None => TransactionOutcome::Succeeded, - Some(error) => TransactionOutcome::Failed(error), - })) -} - async fn get_finalized_transaction( runtime: &R, signature: Signature, @@ -156,12 +115,6 @@ pub enum GetTransactionError { #[error("Transaction returned for {queried} is not signed by its fee payer")] #[from(ignore)] InvalidSignature { queried: Signature }, - #[error("Transaction returned for {queried} differs from the submitted one")] - #[from(ignore)] - UnexpectedTransaction { queried: Signature }, - #[error("Transaction returned for {queried} has no status metadata")] - #[from(ignore)] - MissingStatusMeta { queried: Signature }, } impl GetTransactionError { @@ -171,9 +124,7 @@ impl GetTransactionError { match self { GetTransactionError::UndecodableTransaction { .. } | GetTransactionError::SignatureMismatch { .. } - | GetTransactionError::InvalidSignature { .. } - | GetTransactionError::UnexpectedTransaction { .. } - | GetTransactionError::MissingStatusMeta { .. } => true, + | GetTransactionError::InvalidSignature { .. } => true, GetTransactionError::IcError(_) | GetTransactionError::RpcError(_) | GetTransactionError::InconsistentRpcResults => false, diff --git a/minter/src/test_fixtures/mod.rs b/minter/src/test_fixtures/mod.rs index 07d22d9c2..1dd39d0f3 100644 --- a/minter/src/test_fixtures/mod.rs +++ b/minter/src/test_fixtures/mod.rs @@ -103,11 +103,20 @@ pub fn init_balance() { /// Queueing the funding deposit needs the minter public key, so this records it. pub fn init_balance_to(amount: Lamport) { init_schnorr_master_key(); + fund_minter(queued_deposit_of(funding_account(), amount), MINTER_ADDRESS); +} + +fn funding_account() -> Account { + account(0xFD) +} + +fn fund_minter(deposit: QueuedDeposit, minter_address: Address) { + let amount = deposit.balance.sweepable_amount(); let funding_deposit_id = crate::state::read_state(|state| state.deposits().next_id()); let sweep_signature = signature(0xFF00 + funding_deposit_id as usize); - events::queue_deposit(funding_deposit_id, account(0xFD), amount); - events::submit_sweep(sweep_signature, vec![funding_deposit_id]); + events::queue(funding_deposit_id, deposit); + events::submit_sweep_to(sweep_signature, vec![funding_deposit_id], minter_address); events::succeed_transaction(sweep_signature); events::credit_sweep(sweep_signature, amount); events::mint_swept_deposit(funding_deposit_id, 0xFE00 + funding_deposit_id); @@ -457,6 +466,14 @@ pub mod devnet_sweep { crate::state::mutate_state(|s| s.cache_minter_public_key(master_key())); } + pub fn init_balance() { + init_master_key(); + super::fund_minter( + fresh_deposit(super::funding_account(), u64::MAX / 2), + minter_main_address(), + ); + } + pub fn master_key() -> crate::state::SchnorrPublicKey { crate::state::SchnorrPublicKey { public_key: master_private_key().public_key(), @@ -475,8 +492,19 @@ pub mod devnet_sweep { } fn sign_as(account: &Account, message: &[u8]) -> solana_signature::Signature { + sign_on(crate::address::derivation_path(account), message) + } + + pub fn minter_signature_of(message: &Message) -> solana_signature::Signature { + sign_on(crate::address::MINTER_DERIVATION_PATH, &message.serialize()) + } + + fn sign_on( + derivation_path: crate::address::DerivationPath, + message: &[u8], + ) -> solana_signature::Signature { let path = ic_ed25519::DerivationPath::new( - crate::address::derivation_path(account) + derivation_path .into_iter() .map(ic_ed25519::DerivationIndex) .collect(), @@ -784,13 +812,14 @@ pub mod devnet_sweep { /// All helpers operate on the global thread-local state via [`mutate_state`]. pub mod events { use super::{ - DEFAULT_BLOCK_HEIGHT, MINTER_ADDRESS, NONCE_ACCOUNT, WITHDRAWAL_FEE, queued_deposit, - queued_deposit_of, runtime::TestCanisterRuntime, withdrawal_batch_message, + DEFAULT_BLOCK_HEIGHT, MINTER_ADDRESS, NONCE_ACCOUNT, WITHDRAWAL_FEE, devnet_sweep, + queued_deposit, queued_deposit_of, runtime::TestCanisterRuntime, withdrawal_batch_message, }; use crate::deposit::sweep::deposit_status; use crate::{ numeric::{LedgerBurnIndex, LedgerMintIndex}, rpc::BlockHeight, + sol_transfer::build_batch_withdrawal_message, state::{ QueuedDeposit, Sweep, audit::process_event, @@ -1032,6 +1061,34 @@ pub mod events { nonce_value, &created_withdrawal_transfers(&burn_indices), ); + record_submitted_withdrawal(signature, message, burn_indices); + } + + pub fn submit_signed_withdrawal_batch_transaction( + nonce_value: solana_hash::Hash, + burn_indices: Vec, + ) -> Signature { + let burn_indices: Vec = burn_indices + .into_iter() + .map(LedgerBurnIndex::from) + .collect(); + let message = build_batch_withdrawal_message( + &devnet_sweep::minter_main_address(), + &NONCE_ACCOUNT, + nonce_value, + &created_withdrawal_transfers(&burn_indices), + ) + .expect("BUG: the withdrawal batch message exceeds the transaction size"); + let signature = devnet_sweep::minter_signature_of(&message); + record_submitted_withdrawal(signature, message, burn_indices); + signature + } + + fn record_submitted_withdrawal( + signature: Signature, + message: solana_message::Message, + burn_indices: Vec, + ) { mutate_state(|state| { process_event( state, diff --git a/minter/src/withdraw/tests.rs b/minter/src/withdraw/tests.rs index 70b2a7fc8..652c8da33 100644 --- a/minter/src/withdraw/tests.rs +++ b/minter/src/withdraw/tests.rs @@ -335,7 +335,7 @@ mod process_pending_withdrawals_tests { sol_transfer::build_batch_withdrawal_message, state::event::EventType, test_fixtures::{ - address, durable_nonce, + address, devnet_sweep, durable_nonce, events::{ create_withdrawal_batch_transaction, create_withdrawal_batch_transaction_on, submit_withdrawal_batch_transaction, @@ -552,30 +552,40 @@ mod process_pending_withdrawals_tests { #[tokio::test] async fn should_rebroadcast_a_withdrawal_whose_initial_send_failed_until_it_finalizes() { init_state(); - init_balance(); - init_schnorr_master_key(); + devnet_sweep::init_balance(); events::accept_withdrawal(account(1), 1, MINIMUM_WITHDRAWAL_AMOUNT); + let minter_address = devnet_sweep::minter_main_address(); + let signature = devnet_sweep::minter_signature_of( + &build_batch_withdrawal_message( + &minter_address, + &NONCE_ACCOUNT, + durable_nonce(1), + &[( + solana_address::Address::from([0u8; 32]), + MINIMUM_WITHDRAWAL_AMOUNT - WITHDRAWAL_FEE, + )], + ) + .unwrap(), + ); let submission = TestCanisterRuntime::new() .with_increasing_time() .add_stub_response(GetAccountInfoResult::Consistent(Ok(Some( - nonce_account_info(MINTER_ADDRESS, 1), + nonce_account_info(minter_address, 1), )))) .add_stub_response(SendTransactionResult::Consistent(Err( RpcError::ValidationError("send failed".to_string()), ))) - .add_signer(sign_as_minter()); + .add_signer(sign_as_minter().expect([Ok(signature)])); process_pending_withdrawals(submission.clone()).await; assert_matches!(withdrawal_status(1), WithdrawalStatus::TxSent { .. }); let rebroadcast = TestCanisterRuntime::new() .with_increasing_time_from(submission.time() + MIN_REBROADCAST_AGE.as_nanos() as u64) .add_stub_response(GetAccountInfoResult::Consistent(Ok(Some( - nonce_account_info(MINTER_ADDRESS, 1), + nonce_account_info(minter_address, 1), )))) - .add_stub_response(SendTransactionResult::Consistent(Ok( - minter_signature().into() - ))); + .add_stub_response(SendTransactionResult::Consistent(Ok(signature.into()))); finalize_transactions(rebroadcast.clone()).await; assert_eq!( rebroadcast.sent_transactions(), @@ -586,14 +596,14 @@ mod process_pending_withdrawals_tests { let finalization = TestCanisterRuntime::new() .with_increasing_time() .add_stub_response(GetAccountInfoResult::Consistent(Ok(Some( - nonce_account_info(MINTER_ADDRESS, 2), + nonce_account_info(minter_address, 2), )))) - .add_stub_response(succeeded_withdrawal_response(&minter_signature())); + .add_stub_response(succeeded_withdrawal_response(&signature)); finalize_transactions(finalization).await; assert_eq!( withdrawal_status(1), WithdrawalStatus::TxFinalized(TxFinalizedStatus::Success { - transaction_id: minter_signature().into(), + transaction_id: signature.into(), effective_transaction_fee: None, }) ); From 34b65d0c41bfcc543f42632ad4d2aebdf0958e00 Mon Sep 17 00:00:00 2001 From: gregorydemay Date: Thu, 8 Oct 2026 10:37:05 +0200 Subject: [PATCH 06/10] refactor(minter): restore get_transaction as a single function Co-Authored-By: Claude Opus 5.5 (1M context) --- minter/src/rpc/mod.rs | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/minter/src/rpc/mod.rs b/minter/src/rpc/mod.rs index 475c8c22d..09eab56f6 100644 --- a/minter/src/rpc/mod.rs +++ b/minter/src/rpc/mod.rs @@ -42,20 +42,6 @@ pub async fn get_transaction( runtime: &R, signature: Signature, ) -> Result, GetTransactionError> { - let Some(outcome) = get_finalized_transaction(runtime, signature).await? else { - return Ok(None); - }; - let transaction = ensure_signed_with(&outcome, signature)?; - Ok(Some(FetchedTransaction { - transaction, - meta: outcome.transaction.meta, - })) -} - -async fn get_finalized_transaction( - runtime: &R, - signature: Signature, -) -> Result, GetTransactionError> { let result = read_state(|state| state.sol_rpc_client(runtime.inter_canister_call_runtime())) .get_transaction(signature) .with_encoding(GetTransactionEncoding::Base64) @@ -66,7 +52,14 @@ async fn get_finalized_transaction( .try_send() .await; match result? { - MultiRpcResult::Consistent(Ok(outcome)) => Ok(outcome), + MultiRpcResult::Consistent(Ok(Some(outcome))) => { + let transaction = ensure_signed_with(&outcome, signature)?; + Ok(Some(FetchedTransaction { + transaction, + meta: outcome.transaction.meta, + })) + } + MultiRpcResult::Consistent(Ok(None)) => Ok(None), MultiRpcResult::Consistent(Err(e)) => Err(GetTransactionError::RpcError(e)), MultiRpcResult::Inconsistent(_) => Err(GetTransactionError::InconsistentRpcResults), } From 772d77a53e7b1b0769dc398bbbab1935ba1c2632 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9gory=20Demay?= Date: Thu, 8 Oct 2026 15:27:32 +0000 Subject: [PATCH 07/10] fix(minter): require the fetched withdrawal to carry the submitted message Co-Authored-By: Claude Opus 5.5 --- minter/src/monitor/tests.rs | 32 ++++++++++++++++++- minter/src/monitor/withdrawals.rs | 18 ++++++++++- minter/src/test_fixtures/mod.rs | 51 +++++++++++++++++++++++++++---- 3 files changed, 93 insertions(+), 8 deletions(-) diff --git a/minter/src/monitor/tests.rs b/minter/src/monitor/tests.rs index 17ef0661f..420270745 100644 --- a/minter/src/monitor/tests.rs +++ b/minter/src/monitor/tests.rs @@ -16,7 +16,8 @@ use crate::{ EventsAssert, GetTransactionResult, MINIMUM_WITHDRAWAL_AMOUNT, NONCE_ACCOUNT, account, confirmed_block_at_height, devnet_sweep, durable_nonce, events, failed_withdrawal_response, finalized_status, init_balance, init_schnorr_master_key, init_state, nonce_account_info, - runtime::TestCanisterRuntime, signature, succeeded_withdrawal_response, + runtime::TestCanisterRuntime, signature, succeeded_transaction_response, + succeeded_withdrawal_response, }, }; use sol_rpc_types::{ @@ -522,6 +523,35 @@ mod withdrawal_finalization { ); } + #[tokio::test] + async fn should_not_finalize_a_withdrawal_fetched_with_another_message() { + setup_with_signing_key(); + let other_message = + solana_message::Message::new(&[], Some(&devnet_sweep::minter_main_address())); + let signature = devnet_sweep::minter_signature_of(&other_message); + events::accept_withdrawal(account(1), 1, MINIMUM_WITHDRAWAL_AMOUNT); + events::create_withdrawal_batch_transaction(durable_nonce(BOUND_NONCE_SEED), vec![1]); + events::submit_signed_withdrawal_batch_transaction_under( + signature, + durable_nonce(BOUND_NONCE_SEED), + vec![1], + ); + let events_before = EventsAssert::from_recorded(); + + let runtime = TestCanisterRuntime::new() + .with_increasing_time() + .add_stub_response(nonce_read(UNSEEN_NONCE_SEED)) + .add_stub_response(succeeded_transaction_response(&signature, other_message)); + + finalize_transactions(runtime).await; + + assert_eq!(EventsAssert::from_recorded(), events_before); + read_state(|s| { + assert!(s.submitted_transactions().contains_key(&signature)); + assert_eq!(s.nonce_pool().num_free_accounts(), 0); + }); + } + fn assert_nonce_account_is_free() { read_state(|s| { assert!( diff --git a/minter/src/monitor/withdrawals.rs b/minter/src/monitor/withdrawals.rs index e29148b85..f23738734 100644 --- a/minter/src/monitor/withdrawals.rs +++ b/minter/src/monitor/withdrawals.rs @@ -20,7 +20,7 @@ use solana_address::Address; use solana_hash::Hash; use solana_message::Message; use solana_signature::Signature; -use solana_transaction::Transaction; +use solana_transaction::{Transaction, versioned::VersionedTransaction}; use std::time::Duration; #[derive(Default)] @@ -116,6 +116,13 @@ impl InFlightWithdrawal { } } + fn carries_message_of(&self, transaction: &VersionedTransaction) -> bool { + match &transaction.message { + solana_message::VersionedMessage::Legacy(message) => *message == self.message, + solana_message::VersionedMessage::V0(_) => false, + } + } + fn is_old_enough_to_rebroadcast(&self, now: u64) -> bool { Duration::from_nanos(now.saturating_sub(self.submitted_at)) >= MIN_REBROADCAST_AGE } @@ -198,6 +205,15 @@ async fn finalize_landed_withdrawal( ) -> Finalization { let signature = withdrawal.signature; let event = match get_transaction(runtime, signature).await { + Ok(Some(FetchedTransaction { transaction, .. })) + if !withdrawal.carries_message_of(&transaction) => + { + log!( + Priority::Error, + "Withdrawal transaction {signature} was fetched with another message than submitted, retrying next round" + ); + return Finalization::UnresolvedOutcome; + } Ok(Some(FetchedTransaction { meta: Some(meta), .. })) => match meta.err { diff --git a/minter/src/test_fixtures/mod.rs b/minter/src/test_fixtures/mod.rs index 1dd39d0f3..1c3272442 100644 --- a/minter/src/test_fixtures/mod.rs +++ b/minter/src/test_fixtures/mod.rs @@ -372,6 +372,15 @@ pub fn failed_withdrawal_response(signature: &solana_signature::Signature) -> Ge ) } +/// The `getTransaction` response for a transaction with the given signature and message, +/// finalized without error. +pub fn succeeded_transaction_response( + signature: &solana_signature::Signature, + message: solana_message::Message, +) -> GetTransactionResult { + transaction_response(signature, message, serde_json::Value::Null) +} + fn withdrawal_response( signature: &solana_signature::Signature, error: serde_json::Value, @@ -386,6 +395,14 @@ fn withdrawal_response( other => panic!("BUG: expected a submitted withdrawal transaction, got {other:?}"), }, ); + transaction_response(signature, message, error) +} + +fn transaction_response( + signature: &solana_signature::Signature, + message: solana_message::Message, + error: serde_json::Value, +) -> GetTransactionResult { let transaction = solana_transaction::Transaction { signatures: vec![*signature], message, @@ -1072,16 +1089,38 @@ pub mod events { .into_iter() .map(LedgerBurnIndex::from) .collect(); - let message = build_batch_withdrawal_message( + let message = signed_withdrawal_batch_message(nonce_value, &burn_indices); + let signature = devnet_sweep::minter_signature_of(&message); + record_submitted_withdrawal(signature, message, burn_indices); + signature + } + + /// Records a `SubmittedTransaction` under `signature` for the withdrawal transaction that + /// [`submit_signed_withdrawal_batch_transaction`] would submit, whatever `signature` signs. + pub fn submit_signed_withdrawal_batch_transaction_under( + signature: Signature, + nonce_value: solana_hash::Hash, + burn_indices: Vec, + ) { + let burn_indices: Vec = burn_indices + .into_iter() + .map(LedgerBurnIndex::from) + .collect(); + let message = signed_withdrawal_batch_message(nonce_value, &burn_indices); + record_submitted_withdrawal(signature, message, burn_indices); + } + + fn signed_withdrawal_batch_message( + nonce_value: solana_hash::Hash, + burn_indices: &[LedgerBurnIndex], + ) -> solana_message::Message { + build_batch_withdrawal_message( &devnet_sweep::minter_main_address(), &NONCE_ACCOUNT, nonce_value, - &created_withdrawal_transfers(&burn_indices), + &created_withdrawal_transfers(burn_indices), ) - .expect("BUG: the withdrawal batch message exceeds the transaction size"); - let signature = devnet_sweep::minter_signature_of(&message); - record_submitted_withdrawal(signature, message, burn_indices); - signature + .expect("BUG: the withdrawal batch message exceeds the transaction size") } fn record_submitted_withdrawal( From fc15588c52d83f5296e1abc668dcaf47d13609e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9gory=20Demay?= Date: Thu, 8 Oct 2026 15:28:52 +0000 Subject: [PATCH 08/10] feat(minter): count undecided withdrawal transactions by reason Co-Authored-By: Claude Opus 5.5 --- docs/design.md | 2 +- minter/src/metrics.rs | 16 +++++++----- minter/src/monitor/tests.rs | 8 ++++-- minter/src/monitor/withdrawals.rs | 27 ++++++++++++++----- minter/src/storage/mod.rs | 43 +++++++++++++++++++++++++++++-- 5 files changed, 79 insertions(+), 17 deletions(-) diff --git a/docs/design.md b/docs/design.md index 472412ac1..8d9d3fa69 100644 --- a/docs/design.md +++ b/docs/design.md @@ -602,7 +602,7 @@ Sweeps remain exposed to the limited retention of the providers' transaction his The minimum age before a re-broadcast leaves the retries to the RPC node first: since the ckSOL minter sends transactions without `maxRetries`, an Agave RPC node keeps re-sending a durable-nonce transaction every 2 seconds until it lands, its nonce advances, or 150 blocks (about 60 to 90 seconds) pass, so an earlier re-broadcast would only duplicate that work. The submission time is taken from the timestamp of the `SubmittedTransaction` event, so it survives upgrades. Re-broadcasting covers a transaction whose initial send failed as well as one dropped before landing, and it is always safe: all copies carry the same nonce value, so whichever lands first advances the nonce and all others fail the nonce check. A failed re-broadcast is retried in the next round. Re-broadcasting has no backoff and no budget: each round adds one `sendTransaction` call per withdrawal transaction whose nonce is unchanged, i.e., at most one call per transaction every 2 minutes, and the number of such transactions is bounded by the size of the nonce pool. This cost is not covered by the withdrawal fee of [Section 3.3.2](#332-cksol-minter-fees) and is accepted, since a transaction stays unlanded only while it fails to land, which should be rare and is visible through the re-broadcast metric. -The nonce account stays bound to a landed withdrawal transaction until `getTransaction` has resolved its outcome; there is no intermediate state in which the account is released while the outcome is still unknown. This keeps the state machine of a withdrawal transaction unchanged, i.e., `Submitted` until it is `Succeeded` or `Failed`, and it adds a second, independent confirmation of the landing: a consistent but bogus nonce read alone, e.g., agreed upon by misbehaving providers, cannot finalize a withdrawal, release its nonce account for reuse, or report it to the user, since the providers must additionally return the exact submitted transaction as finalized. Unlike a status query, `getTransaction` is answered consistently by the default providers for transactions that landed several months ago, and it is only called once the nonce advance has proven that the transaction exists. While the outcome is unresolved, `withdrawal_status` keeps reporting `TxSent`, and the `getTransaction` call is retried on every round without a backoff or retry budget. The number of withdrawal transactions whose nonce advanced but whose outcome could not be fetched in the last round is exposed as a metric. A withdrawal transaction that resolves to an on-chain error follows the regular failure handling described below: the transaction is stored whole and the burned ckSOL is not reimbursed, as there is no reimbursement flow. +The nonce account stays bound to a landed withdrawal transaction until `getTransaction` has resolved its outcome; there is no intermediate state in which the account is released while the outcome is still unknown. This keeps the state machine of a withdrawal transaction unchanged, i.e., `Submitted` until it is `Succeeded` or `Failed`, and it adds a second, independent confirmation of the landing: a consistent but bogus nonce read alone, e.g., agreed upon by misbehaving providers, cannot finalize a withdrawal, release its nonce account for reuse, or report it to the user, since the providers must additionally return the exact submitted transaction as finalized. Unlike a status query, `getTransaction` is answered consistently by the default providers for transactions that landed several months ago, and it is only called once the nonce advance has proven that the transaction exists. While the outcome is unresolved, `withdrawal_status` keeps reporting `TxSent`, and the `getTransaction` call is retried on every round without a backoff or retry budget. The number of in-flight withdrawal transactions left undecided by the last round is exposed as a metric, labelled by reason: a stale nonce read, a failed nonce read, or a nonce advance whose outcome could not be fetched. A withdrawal transaction that resolves to an on-chain error follows the regular failure handling described below: the transaction is stored whole and the burned ckSOL is not reimbursed, as there is no reimbursement flow. The following residual risks are accepted: diff --git a/minter/src/metrics.rs b/minter/src/metrics.rs index 056d2f295..6b8d40950 100644 --- a/minter/src/metrics.rs +++ b/minter/src/metrics.rs @@ -1,5 +1,5 @@ use crate::state::{QuarantineCause, State}; -use crate::storage::{self, FailedCreditReason, FailedMintReason}; +use crate::storage::{self, FailedCreditReason, FailedMintReason, UndecidedWithdrawalReason}; use ic_metrics_encoder::MetricsEncoder; const WASM_PAGE_SIZE_IN_BYTES: usize = 65536; @@ -147,12 +147,16 @@ pub fn encode_metrics(w: &mut MetricsEncoder>, s: &State) -> std::io::Re storage::with_unstable_metrics(|m| m.withdrawal_transaction_rebroadcasts).metric_value(), "Number of re-broadcast attempts of withdrawal transactions since the last upgrade.", )?; - w.encode_gauge( - "withdrawal_transactions_with_unresolved_outcome", - storage::with_unstable_metrics(|m| m.withdrawal_transactions_with_unresolved_outcome) - .metric_value(), - "Number of in-flight withdrawal transactions whose nonce advanced but whose outcome could not be fetched in the last finalization round.", + let mut undecided_withdrawal_transactions = w.gauge_vec( + "undecided_withdrawal_transactions", + "Number of in-flight withdrawal transactions left undecided by the last finalization round, by reason: a stale nonce read, a failed nonce read, or a nonce advance whose outcome could not be fetched.", )?; + for reason in UndecidedWithdrawalReason::ALL { + undecided_withdrawal_transactions = undecided_withdrawal_transactions.value( + &[("reason", reason.label())], + storage::undecided_withdrawal_transaction_count(reason).metric_value(), + )?; + } Ok(()) } diff --git a/minter/src/monitor/tests.rs b/minter/src/monitor/tests.rs index 420270745..c32a9db6b 100644 --- a/minter/src/monitor/tests.rs +++ b/minter/src/monitor/tests.rs @@ -11,7 +11,7 @@ use crate::{ event::{EventType, VersionedMessage}, mutate_state, read_state, reset_state, }, - storage::{reset_events, with_unstable_metrics}, + storage::{UndecidedWithdrawalReason, reset_events, undecided_withdrawal_transaction_count}, test_fixtures::{ EventsAssert, GetTransactionResult, MINIMUM_WITHDRAWAL_AMOUNT, NONCE_ACCOUNT, account, confirmed_block_at_height, devnet_sweep, durable_nonce, events, failed_withdrawal_response, @@ -496,6 +496,10 @@ mod withdrawal_finalization { assert_eq!(EventsAssert::from_recorded(), events_before); assert!(runtime.sent_transactions().is_empty()); read_state(|s| assert!(s.submitted_transactions().contains_key(&signature))); + assert_eq!( + undecided_withdrawal_transaction_count(UndecidedWithdrawalReason::StaleNonce), + 1 + ); } #[tokio::test] @@ -518,7 +522,7 @@ mod withdrawal_finalization { assert_eq!(s.nonce_pool().num_free_accounts(), 0); }); assert_eq!( - with_unstable_metrics(|m| m.withdrawal_transactions_with_unresolved_outcome), + undecided_withdrawal_transaction_count(UndecidedWithdrawalReason::UnresolvedOutcome), 1 ); } diff --git a/minter/src/monitor/withdrawals.rs b/minter/src/monitor/withdrawals.rs index f23738734..b329e8989 100644 --- a/minter/src/monitor/withdrawals.rs +++ b/minter/src/monitor/withdrawals.rs @@ -10,7 +10,9 @@ use crate::{ event::{EventType, VersionedMessage}, mutate_state, read_state, }, - storage::with_unstable_metrics_mut, + storage::{ + UndecidedWithdrawalReason, set_undecided_withdrawal_transactions, with_unstable_metrics_mut, + }, withdraw::nonce::read_verified_nonce, }; use canlog::log; @@ -21,12 +23,19 @@ use solana_hash::Hash; use solana_message::Message; use solana_signature::Signature; use solana_transaction::{Transaction, versioned::VersionedTransaction}; -use std::time::Duration; +use std::{collections::BTreeMap, time::Duration}; #[derive(Default)] pub(super) struct InFlightNonces { unchanged: Vec, advanced: Vec, + undecided: BTreeMap, +} + +impl InFlightNonces { + fn leave_undecided(&mut self, reason: UndecidedWithdrawalReason) { + *self.undecided.entry(reason).or_insert(0) += 1; + } } pub(super) async fn read_in_flight_nonces(runtime: &R) -> InFlightNonces { @@ -52,7 +61,10 @@ pub(super) async fn read_in_flight_nonces(runtime: &R) -> In match nonce_read { Some(NonceRead::Unchanged) => nonces.unchanged.push(withdrawal), Some(NonceRead::Advanced) => nonces.advanced.push(withdrawal), - Some(NonceRead::Stale) | None => {} + Some(NonceRead::Stale) => { + nonces.leave_undecided(UndecidedWithdrawalReason::StaleNonce) + } + None => nonces.leave_undecided(UndecidedWithdrawalReason::NonceReadFailed), } } } @@ -95,9 +107,12 @@ pub(super) async fn finalize_transactions_batch( .filter(|finalization| *finalization == Finalization::UnresolvedOutcome) .count(); } - with_unstable_metrics_mut(|metrics| { - metrics.withdrawal_transactions_with_unresolved_outcome = num_unresolved_outcomes as u64 - }); + let mut undecided = nonces.undecided.clone(); + undecided.insert( + UndecidedWithdrawalReason::UnresolvedOutcome, + num_unresolved_outcomes as u64, + ); + set_undecided_withdrawal_transactions(undecided); } struct InFlightWithdrawal { diff --git a/minter/src/storage/mod.rs b/minter/src/storage/mod.rs index 8a07ab28e..1d31b043f 100644 --- a/minter/src/storage/mod.rs +++ b/minter/src/storage/mod.rs @@ -39,7 +39,7 @@ pub(crate) struct Metrics { pub failed_credit_attempts: BTreeMap, pub failed_mint_attempts: BTreeMap, pub withdrawal_transaction_rebroadcasts: u64, - pub withdrawal_transactions_with_unresolved_outcome: u64, + pub undecided_withdrawal_transactions: BTreeMap, } impl Metrics { @@ -49,7 +49,7 @@ impl Metrics { failed_credit_attempts: BTreeMap::new(), failed_mint_attempts: BTreeMap::new(), withdrawal_transaction_rebroadcasts: 0, - withdrawal_transactions_with_unresolved_outcome: 0, + undecided_withdrawal_transactions: BTreeMap::new(), } } } @@ -133,6 +133,45 @@ pub(crate) fn failed_mint_attempt_count(reason: FailedMintReason) -> u64 { with_unstable_metrics(|m| m.failed_mint_attempts.get(&reason).copied().unwrap_or(0)) } +/// Why an in-flight withdrawal transaction was left undecided by a finalization round. +#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)] +pub enum UndecidedWithdrawalReason { + StaleNonce, + NonceReadFailed, + UnresolvedOutcome, +} + +impl UndecidedWithdrawalReason { + pub const ALL: [Self; 3] = [ + Self::StaleNonce, + Self::NonceReadFailed, + Self::UnresolvedOutcome, + ]; + + pub fn label(self) -> &'static str { + match self { + Self::StaleNonce => "stale_nonce", + Self::NonceReadFailed => "nonce_read_failed", + Self::UnresolvedOutcome => "unresolved_outcome", + } + } +} + +pub(crate) fn set_undecided_withdrawal_transactions( + counts: BTreeMap, +) { + with_unstable_metrics_mut(|m| m.undecided_withdrawal_transactions = counts); +} + +pub(crate) fn undecided_withdrawal_transaction_count(reason: UndecidedWithdrawalReason) -> u64 { + with_unstable_metrics(|m| { + m.undecided_withdrawal_transactions + .get(&reason) + .copied() + .unwrap_or(0) + }) +} + /// Appends the event to the event log. pub fn record_event(payload: EventType, runtime: &R) { EVENTS From 57158a152e2ecca2836a03bc6c31758343eb1cd4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9gory=20Demay?= Date: Thu, 8 Oct 2026 15:28:58 +0000 Subject: [PATCH 09/10] fix(minter): log a landed withdrawal not found as an error Co-Authored-By: Claude Opus 5.5 --- minter/src/monitor/withdrawals.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/minter/src/monitor/withdrawals.rs b/minter/src/monitor/withdrawals.rs index b329e8989..43d918677 100644 --- a/minter/src/monitor/withdrawals.rs +++ b/minter/src/monitor/withdrawals.rs @@ -253,7 +253,7 @@ async fn finalize_landed_withdrawal( } Ok(None) => { log!( - Priority::Info, + Priority::Error, "Withdrawal transaction {signature} landed but was not found, retrying next round" ); return Finalization::UnresolvedOutcome; From e90a3d8fb6eb317f05a3ae7c12be1fa56651ce15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9gory=20Demay?= Date: Thu, 8 Oct 2026 15:29:02 +0000 Subject: [PATCH 10/10] docs: name the withdrawal status endpoint after the asset Co-Authored-By: Claude Opus 5.5 --- docs/design.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/design.md b/docs/design.md index 8d9d3fa69..766339def 100644 --- a/docs/design.md +++ b/docs/design.md @@ -602,7 +602,7 @@ Sweeps remain exposed to the limited retention of the providers' transaction his The minimum age before a re-broadcast leaves the retries to the RPC node first: since the ckSOL minter sends transactions without `maxRetries`, an Agave RPC node keeps re-sending a durable-nonce transaction every 2 seconds until it lands, its nonce advances, or 150 blocks (about 60 to 90 seconds) pass, so an earlier re-broadcast would only duplicate that work. The submission time is taken from the timestamp of the `SubmittedTransaction` event, so it survives upgrades. Re-broadcasting covers a transaction whose initial send failed as well as one dropped before landing, and it is always safe: all copies carry the same nonce value, so whichever lands first advances the nonce and all others fail the nonce check. A failed re-broadcast is retried in the next round. Re-broadcasting has no backoff and no budget: each round adds one `sendTransaction` call per withdrawal transaction whose nonce is unchanged, i.e., at most one call per transaction every 2 minutes, and the number of such transactions is bounded by the size of the nonce pool. This cost is not covered by the withdrawal fee of [Section 3.3.2](#332-cksol-minter-fees) and is accepted, since a transaction stays unlanded only while it fails to land, which should be rare and is visible through the re-broadcast metric. -The nonce account stays bound to a landed withdrawal transaction until `getTransaction` has resolved its outcome; there is no intermediate state in which the account is released while the outcome is still unknown. This keeps the state machine of a withdrawal transaction unchanged, i.e., `Submitted` until it is `Succeeded` or `Failed`, and it adds a second, independent confirmation of the landing: a consistent but bogus nonce read alone, e.g., agreed upon by misbehaving providers, cannot finalize a withdrawal, release its nonce account for reuse, or report it to the user, since the providers must additionally return the exact submitted transaction as finalized. Unlike a status query, `getTransaction` is answered consistently by the default providers for transactions that landed several months ago, and it is only called once the nonce advance has proven that the transaction exists. While the outcome is unresolved, `withdrawal_status` keeps reporting `TxSent`, and the `getTransaction` call is retried on every round without a backoff or retry budget. The number of in-flight withdrawal transactions left undecided by the last round is exposed as a metric, labelled by reason: a stale nonce read, a failed nonce read, or a nonce advance whose outcome could not be fetched. A withdrawal transaction that resolves to an on-chain error follows the regular failure handling described below: the transaction is stored whole and the burned ckSOL is not reimbursed, as there is no reimbursement flow. +The nonce account stays bound to a landed withdrawal transaction until `getTransaction` has resolved its outcome; there is no intermediate state in which the account is released while the outcome is still unknown. This keeps the state machine of a withdrawal transaction unchanged, i.e., `Submitted` until it is `Succeeded` or `Failed`, and it adds a second, independent confirmation of the landing: a consistent but bogus nonce read alone, e.g., agreed upon by misbehaving providers, cannot finalize a withdrawal, release its nonce account for reuse, or report it to the user, since the providers must additionally return the exact submitted transaction as finalized. Unlike a status query, `getTransaction` is answered consistently by the default providers for transactions that landed several months ago, and it is only called once the nonce advance has proven that the transaction exists. While the outcome is unresolved, `withdraw_sol_status` keeps reporting `TxSent`, and the `getTransaction` call is retried on every round without a backoff or retry budget. The number of in-flight withdrawal transactions left undecided by the last round is exposed as a metric, labelled by reason: a stale nonce read, a failed nonce read, or a nonce advance whose outcome could not be fetched. A withdrawal transaction that resolves to an on-chain error follows the regular failure handling described below: the transaction is stored whole and the burned ckSOL is not reimbursed, as there is no reimbursement flow. The following residual risks are accepted: