diff --git a/docs/design.md b/docs/design.md index 09a74d987..766339def 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, `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. -- **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, `withdraw_sol_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. 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 f72c453f4..8c247fc91 100644 --- a/integration_tests/tests/tests.rs +++ b/integration_tests/tests/tests.rs @@ -795,12 +795,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 93238d0c3..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,6 +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.", )?; + 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/mod.rs b/minter/src/monitor/mod.rs index 4976c14c4..c77d1a9d4 100644 --- a/minter/src/monitor/mod.rs +++ b/minter/src/monitor/mod.rs @@ -2,36 +2,35 @@ 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::{ + finalize_transactions_batch, read_in_flight_nonces, resubmit_transactions_batch, }, + 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. +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 +/// 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 +47,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, @@ -57,10 +57,13 @@ 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_submitted_transactions(&runtime).await; + let more_transactions_to_check = check_sweep_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 { @@ -69,40 +72,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 +136,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 +178,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..c32a9db6b 100644 --- a/minter/src/monitor/tests.rs +++ b/minter/src/monitor/tests.rs @@ -1,8 +1,9 @@ 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::{ + address::minter_address, constants::MAX_CONCURRENT_RPC_CALLS, rpc::BlockHeight, state::{ @@ -10,11 +11,13 @@ use crate::{ event::{EventType, VersionedMessage}, mutate_state, read_state, reset_state, }, - storage::reset_events, + storage::{UndecidedWithdrawalReason, reset_events, undecided_withdrawal_transaction_count}, 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, 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_transaction_response, + succeeded_withdrawal_response, }, }; use sol_rpc_types::{ @@ -27,6 +30,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 +72,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 +81,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 +98,38 @@ 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!(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); + assert_eq!( + called_methods(&runtime) + .iter() + .filter(|method| *method == "getSignatureStatuses") + .count(), + MAX_CONCURRENT_RPC_CALLS + ); + assert_eq!( + runtime.set_timer_delays(), + vec![FINALIZE_TRANSACTIONS_RETRY_DELAY] + ); } #[tokio::test] @@ -186,114 +191,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 +262,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,12 +382,268 @@ mod finalization { } } +mod withdrawal_finalization { + use super::*; + + #[tokio::test] + async fn should_record_a_withdrawal_as_succeeded_once_its_nonce_advanced() { + setup_with_signing_key(); + let signature = submit_signed_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_with_signing_key(); + let signature = submit_signed_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_with_signing_key(); + let signature = submit_signed_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_with_signing_key(); + let signature = submit_signed_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_with_signing_key(); + let landed = submit_signed_withdrawal_bound_to(1, STALE_NONCE_SEED); + events::succeed_transaction(landed); + let signature = submit_signed_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))); + assert_eq!( + undecided_withdrawal_transaction_count(UndecidedWithdrawalReason::StaleNonce), + 1 + ); + } + + #[tokio::test] + async fn should_keep_the_nonce_account_bound_until_the_outcome_is_known() { + 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() + .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!( + undecided_withdrawal_transaction_count(UndecidedWithdrawalReason::UnresolvedOutcome), + 1 + ); + } + + #[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!( + 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 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 { + 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 { + runtime + .sent_update_calls() + .into_iter() + .map(|call| call.method) + .collect() +} + fn setup() { init_state(); init_balance(); 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 new file mode 100644 index 000000000..43d918677 --- /dev/null +++ b/minter/src/monitor/withdrawals.rs @@ -0,0 +1,291 @@ +use crate::{ + address::minter_address, + constants::MAX_CONCURRENT_RPC_CALLS, + monitor::MIN_REBROADCAST_AGE, + rpc::{FetchedTransaction, get_transaction, submit_transaction_skipping_preflight}, + runtime::CanisterRuntime, + state::{ + MinterTransaction, NonceRead, + audit::process_event, + event::{EventType, VersionedMessage}, + mutate_state, read_state, + }, + storage::{ + UndecidedWithdrawalReason, set_undecided_withdrawal_transactions, with_unstable_metrics_mut, + }, + withdraw::nonce::read_verified_nonce, +}; +use canlog::log; +use cksol_types_internal::log::Priority; +use itertools::Itertools; +use solana_address::Address; +use solana_hash::Hash; +use solana_message::Message; +use solana_signature::Signature; +use solana_transaction::{Transaction, versioned::VersionedTransaction}; +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 { + let mut nonces = InFlightNonces::default(); + let Some(minter_address) = read_state(|state| state.minter_public_key().map(minter_address)) + else { + return nonces; + }; + 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) => { + nonces.leave_undecided(UndecidedWithdrawalReason::StaleNonce) + } + None => nonces.leave_undecided(UndecidedWithdrawalReason::NonceReadFailed), + } + } + } + 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 nonces.advanced.chunks(MAX_CONCURRENT_RPC_CALLS) { + num_unresolved_outcomes += futures::future::join_all( + batch + .iter() + .map(|withdrawal| finalize_landed_withdrawal(runtime, withdrawal)), + ) + .await + .into_iter() + .filter(|finalization| *finalization == Finalization::UnresolvedOutcome) + .count(); + } + let mut undecided = nonces.undecided.clone(); + undecided.insert( + UndecidedWithdrawalReason::UnresolvedOutcome, + num_unresolved_outcomes as u64, + ); + set_undecided_withdrawal_transactions(undecided); +} + +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 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 + } +} + +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() + }) +} + +async fn read_nonce( + runtime: &R, + withdrawal: InFlightWithdrawal, + minter_address: Address, +) -> (InFlightWithdrawal, Option) { + 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 (withdrawal, None); + } + }; + let nonce_read = read_state(|state| { + state + .nonce_pool() + .classify_read(nonce_account, nonce_value, &read_nonce_value) + }); + 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, +) -> 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 { + 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, + "Withdrawal transaction {signature} landed without status metadata, retrying next round" + ); + return Finalization::UnresolvedOutcome; + } + Ok(None) => { + log!( + Priority::Error, + "Withdrawal transaction {signature} landed but was not found, retrying next round" + ); + return Finalization::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 Finalization::UnresolvedOutcome; + } + }; + mutate_state(|state| process_event(state, event, runtime)); + Finalization::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/state/mod.rs b/minter/src/state/mod.rs index a86ba1ad7..5db66cd5b 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 084feee33..1d31b043f 100644 --- a/minter/src/storage/mod.rs +++ b/minter/src/storage/mod.rs @@ -39,6 +39,7 @@ pub(crate) struct Metrics { pub failed_credit_attempts: BTreeMap, pub failed_mint_attempts: BTreeMap, pub withdrawal_transaction_rebroadcasts: u64, + pub undecided_withdrawal_transactions: BTreeMap, } impl Metrics { @@ -48,6 +49,7 @@ impl Metrics { failed_credit_attempts: BTreeMap::new(), failed_mint_attempts: BTreeMap::new(), withdrawal_transaction_rebroadcasts: 0, + undecided_withdrawal_transactions: BTreeMap::new(), } } } @@ -131,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 diff --git a/minter/src/test_fixtures/mod.rs b/minter/src/test_fixtures/mod.rs index 46d16a42d..1c3272442 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); @@ -346,6 +355,93 @@ 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}]}), + ) +} + +/// 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, +) -> 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:?}"), + }, + ); + 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, + }; + 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 { @@ -387,6 +483,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(), @@ -405,8 +509,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(), @@ -714,13 +829,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, @@ -962,6 +1078,56 @@ 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 = 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), + ) + .expect("BUG: the withdrawal batch message exceeds the transaction size") + } + + 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/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 8ff273e52..f86670868 100644 --- a/minter/src/withdraw/tests.rs +++ b/minter/src/withdraw/tests.rs @@ -335,18 +335,17 @@ 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, }, - 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() { @@ -553,28 +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), WithdrawSolStatus::TxSent { .. }); 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(SendTransactionResult::Consistent(Ok( - minter_signature().into() - ))); + .add_stub_response(GetAccountInfoResult::Consistent(Ok(Some( + nonce_account_info(minter_address, 1), + )))) + .add_stub_response(SendTransactionResult::Consistent(Ok(signature.into()))); finalize_transactions(rebroadcast.clone()).await; assert_eq!( rebroadcast.sent_transactions(), @@ -584,14 +595,15 @@ 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(&signature)); finalize_transactions(finalization).await; assert_eq!( withdrawal_status(1), WithdrawSolStatus::TxFinalized(TxFinalizedStatus::Success { - transaction_id: minter_signature().into(), + transaction_id: signature.into(), effective_transaction_fee: None, }) );