From 004bb2296f68e5243f0ef2860e7578dd79dd70cc Mon Sep 17 00:00:00 2001 From: Sofi Azcoaga Date: Mon, 7 Sep 2026 10:24:29 -0300 Subject: [PATCH 1/4] feat(rust-client): retryable batch submissions with an unknown outcome --- crates/rust-client/src/errors.rs | 16 ++ crates/rust-client/src/test_utils/mock.rs | 39 +++- .../src/transaction/batch/error.rs | 20 ++ .../rust-client/src/transaction/batch/mod.rs | 165 ++++++++++++--- crates/rust-client/src/transaction/mod.rs | 20 +- .../miden-client-tests/src/tests/batch.rs | 189 ++++++++++++++---- 6 files changed, 353 insertions(+), 96 deletions(-) diff --git a/crates/rust-client/src/errors.rs b/crates/rust-client/src/errors.rs index bf3fd03644..2e869177e7 100644 --- a/crates/rust-client/src/errors.rs +++ b/crates/rust-client/src/errors.rs @@ -365,6 +365,22 @@ impl From<&ClientError> for Option { docs_url: Some(TROUBLESHOOTING_DOC), }) }, + ClientError::BatchBuilder(BatchBuilderError::BatchSubmissionOutcomeUnknown { + submission, + .. + }) => Some(ErrorHint { + message: format!( + "Do not rebuild the batch: re-executing produces new transaction ids over \ + the same notes, so if the original did land you would be left tracking ids \ + that can never commit. Neither option can apply the batch twice, since both \ + consume the same nullifiers. Either retry with the `submission` attached to \ + this error, which carries the proven batch and each transaction's inputs, or \ + keep syncing and check `get_transactions` for the {} ids in \ + `submission.transaction_ids()` until they commit or expire.", + submission.transaction_count() + ), + docs_url: Some(TROUBLESHOOTING_DOC), + }), _ => None, } } diff --git a/crates/rust-client/src/test_utils/mock.rs b/crates/rust-client/src/test_utils/mock.rs index ca9ae7ebb7..a5748177eb 100644 --- a/crates/rust-client/src/test_utils/mock.rs +++ b/crates/rust-client/src/test_utils/mock.rs @@ -84,6 +84,9 @@ pub struct MockRpcApi { /// [`MockRpcApi::fail_next_call`]. An entry is removed when served, so the call after it /// answers normally and a test can exercise a retry. next_call_failures: Arc>>, + /// Sealed inputs handed to `submit_proven_batch`, one entry per call that reached the mock, so + /// a test can assert that a resubmission sealed again instead of reusing a cached ciphertext. + submitted_batch_sealed_inputs: Arc>>>, } impl Default for MockRpcApi { @@ -108,9 +111,31 @@ impl MockRpcApi { sync_notes_mmr_path_overrides: Arc::new(RwLock::new(BTreeMap::new())), get_notes_by_id_calls: Arc::new(AtomicUsize::new(0)), next_call_failures: Arc::new(RwLock::new(BTreeMap::new())), + submitted_batch_sealed_inputs: Arc::new(RwLock::new(Vec::new())), } } + /// Id of the first account updated in the mock chain's proven blocks, in block then + /// within-block order. Tests use it to get hold of an account the chain already knows. + /// + /// Panics if the chain has no account updates, which for a mock means the test set it up wrong. + pub fn first_account_id(&self) -> AccountId { + self.mock_chain + .read() + .proven_blocks() + .iter() + .flat_map(|block| block.body().updated_accounts()) + .next() + .expect("the mock chain must have at least one account update") + .account_id() + } + + /// Sealed inputs recorded by `submit_proven_batch`, one entry per call that reached the mock. + /// Within an entry the order matches the batch's transaction order. + pub fn submitted_batch_sealed_inputs(&self) -> Vec> { + self.submitted_batch_sealed_inputs.read().clone() + } + /// Makes the next call to `endpoint` fail with `error` instead of answering. The failure is /// consumed, so the call after it answers normally and a test can exercise a retry. /// @@ -551,15 +576,21 @@ impl NodeRpcClient for MockRpcApi { } /// Simulates the submission of a proven batch to the node by adding it to the mock chain's - /// pending batches. The `proposed_batch` and `sealed_transaction_inputs` arguments are accepted - /// to match the trait signature but are unused — the mock relies on the `ProvenBatch` - /// alone, matching how `submit_proven_transaction` ignores its `sealed_transaction_inputs`. + /// pending batches. The `proposed_batch` argument is accepted to match the trait signature but + /// is unused: the mock relies on the `ProvenBatch` alone. The sealed inputs are recorded rather + /// than decrypted, so a test can inspect what each attempt sent. async fn submit_proven_batch( &self, proven_batch: ProvenBatch, _proposed_batch: ProposedBatch, - _sealed_transaction_inputs: Vec, + sealed_transaction_inputs: Vec, ) -> Result { + if let Some(error) = self.take_failure(RpcEndpoint::SubmitProvenBatch) { + return Err(error); + } + + self.submitted_batch_sealed_inputs.write().push(sealed_transaction_inputs); + let mut mock_chain = self.mock_chain.write(); mock_chain.add_pending_batch(proven_batch); drop(mock_chain); diff --git a/crates/rust-client/src/transaction/batch/error.rs b/crates/rust-client/src/transaction/batch/error.rs index 7ee73af5c6..03f335f4e4 100644 --- a/crates/rust-client/src/transaction/batch/error.rs +++ b/crates/rust-client/src/transaction/batch/error.rs @@ -1,6 +1,10 @@ +use alloc::boxed::Box; + use miden_protocol::block::BlockNumber; use miden_protocol::note::NoteId; +use super::ProvenBatchSubmission; +use crate::rpc::RpcError; use crate::store::StoreError; use crate::transaction::TransactionStoreUpdateError; @@ -16,6 +20,22 @@ pub enum BatchBuilderError { #[error("batch is empty — push at least one transaction before submitting")] Empty, + /// The batch submission came back without a definite outcome, so the node may or may not have + /// accepted it. Nothing was recorded locally for any of its transactions. + #[error( + "submission of a batch of {} transactions came back without a definite outcome, so the \ + node may or may not have accepted it; nothing was recorded locally", + submission.transaction_count() + )] + BatchSubmissionOutcomeUnknown { + /// The batch as submitted, to resend with + /// [`Client::submit_proven_batch`](crate::Client::submit_proven_batch) or to read + /// `transaction_ids()` from and track. + submission: Box, + #[source] + source: RpcError, + }, + /// The node accepted the batch (RPC returned `block_num`), but building one of the /// per-tx [`crate::transaction::TransactionStoreUpdate`]s failed. Callers should trigger /// `sync_state` to reconcile. diff --git a/crates/rust-client/src/transaction/batch/mod.rs b/crates/rust-client/src/transaction/batch/mod.rs index 5b67b94b0f..3ac074d274 100644 --- a/crates/rust-client/src/transaction/batch/mod.rs +++ b/crates/rust-client/src/transaction/batch/mod.rs @@ -34,7 +34,12 @@ //! - A failed [`push`](BatchBuilder::push) leaves the batch exactly as it was, so the caller may //! retry with a different request or submit the transactions accumulated so far. //! -//! ## Error semantics after RPC accept +//! ## Error semantics around submission +//! +//! A submission that comes back without a definite outcome raises +//! [`BatchBuilderError::BatchSubmissionOutcomeUnknown`]. The node may or may not have accepted the +//! batch and nothing was recorded locally, so the error carries a [`ProvenBatchSubmission`] to +//! resend with [`Client::submit_proven_batch`] or to track by its transaction ids. //! //! Once the node accepts the batch, the local store still needs to be updated. If that step //! fails, the caller receives one of two errors that both carry the accepted `block_num`: @@ -44,7 +49,7 @@ //! - [`BatchBuilderError::BatchSubmittedButApplyFailed`] — applying the updates atomically to the //! local store failed. //! -//! In both cases the recovery path is to trigger `sync_state` to reconcile. +//! In all three cases `sync_state` reconciles the local store with what the network holds. mod data_store; mod error; @@ -59,15 +64,16 @@ pub(crate) use data_store::InMemoryBatchDataStore; pub use error::BatchBuilderError; use miden_protocol::MIN_PROOF_SECURITY_LEVEL; use miden_protocol::account::AccountId; -use miden_protocol::batch::ProposedBatch; +use miden_protocol::batch::{ProposedBatch, ProvenBatch}; use miden_protocol::block::{BlockHeader, BlockNumber}; use miden_protocol::note::NoteId; -use miden_protocol::transaction::{PartialBlockchain, ProvenTransaction}; +use miden_protocol::transaction::{PartialBlockchain, ProvenTransaction, TransactionId}; use miden_tx::auth::TransactionAuthenticator; use miden_tx_batch::{BatchExecutor, LocalBatchProver}; +use crate::rpc::RpcError; use crate::rpc::encryption::seal_transaction_inputs; -use crate::store::data_store::build_partial_mmr_with_paths; +use crate::store::data_store::{ClientDataStore, build_partial_mmr_with_paths}; use crate::transaction::{ TransactionRequest, TransactionResult, @@ -76,6 +82,36 @@ use crate::transaction::{ }; use crate::{Client, ClientError}; +/// A proven batch together with everything else its submission needs, so a submission whose +/// outcome the node never confirmed can be retried without executing or proving again. +/// +/// Handed back by [`BatchBuilderError::BatchSubmissionOutcomeUnknown`] and accepted by +/// [`Client::submit_proven_batch`]. +#[derive(Debug, Clone)] +pub struct ProvenBatchSubmission { + proven_batch: ProvenBatch, + // Boxed on the protocol type's own advice: it is large, and this struct travels inside an + // error. + proposed_batch: Box, + /// Kept whole rather than as inputs or sealed inputs. Not sealed, because the validator set's + /// key can rotate between attempts and a retry has to seal against the current one. Whole + /// results, because `BatchBuilder::submit` needs them after the RPC to build the store + /// updates. + tx_results: Vec, +} + +impl ProvenBatchSubmission { + /// Number of transactions in the batch. + pub fn transaction_count(&self) -> usize { + self.tx_results.len() + } + + /// Ids of the transactions in the batch, to track until a sync resolves them. + pub fn transaction_ids(&self) -> impl Iterator + '_ { + self.tx_results.iter().map(|tx_result| tx_result.executed_transaction().id()) + } +} + /// A transaction successfully pushed into a [`BatchBuilder`]: the locally-proven transaction /// alongside the [`TransactionResult`] used to build the per-tx [`TransactionStoreUpdate`]. The /// transaction inputs the RPC submission seals are read back from the result. @@ -106,6 +142,74 @@ impl BatchBuilder<'_, AUTH> { } } +impl Client +where + AUTH: TransactionAuthenticator + Sync + 'static, +{ + /// Open a new [`BatchBuilder`] for accumulating transactions across one or more local + /// accounts. + /// + /// See the module-level docs for usage and constraints. + pub fn new_transaction_batch(&mut self) -> BatchBuilder<'_, AUTH> { + let inner_data_store = ClientDataStore::new(self.store.clone(), self.rpc_api.clone()); + BatchBuilder { + client: self, + data_store: InMemoryBatchDataStore::new(inner_data_store), + pushed_txs: Vec::new(), + consumed_input_notes: BTreeSet::new(), + } + } + + /// Resubmits an already-proven batch and returns the node's chain tip upon mempool admission. + /// + /// This is the retry entry point for a submission whose outcome was never confirmed: pass back + /// the [`ProvenBatchSubmission`] carried by + /// [`BatchBuilderError::BatchSubmissionOutcomeUnknown`] and the batch goes out again without + /// being executed or proven a second time. The batch id is fixed, so resending it cannot + /// duplicate its effects, but the node rejects it as a conflict if the original did land. + /// + /// Unlike [`Client::submit_proven_transaction`](crate::Client::submit_proven_transaction) this + /// is not a general-purpose submit: [`ProvenBatchSubmission`] has no public constructor, so the + /// only way to obtain one is from that error. Assembling and proving a batch goes through + /// [`BatchBuilder`]. + /// + /// The local store is not touched. On success, sync to record the transactions. + /// + /// # Errors + /// + /// Returns [`BatchBuilderError::BatchSubmissionOutcomeUnknown`] when the submission comes back + /// without a definite answer. Every other failure is a rejection the node issued deliberately. + pub async fn submit_proven_batch( + &mut self, + submission: &ProvenBatchSubmission, + ) -> Result { + // Each entry is sealed against its own transaction id, with fresh randomness per attempt. + let key = self.transaction_encryption_key().await?; + let sealed_inputs = submission + .tx_results + .iter() + .map(|tx_result| { + let executed = tx_result.executed_transaction(); + seal_transaction_inputs(&mut self.rng, &key, executed.id(), executed.tx_inputs()) + }) + .collect::, _>>()?; + + let result = self + .rpc_api + .submit_proven_batch( + submission.proven_batch.clone(), + (*submission.proposed_batch).clone(), + sealed_inputs, + ) + .await; + if let Err(err) = &result { + self.forget_stale_transaction_encryption_key(err).await; + } + + result.map_err(|err| promote_indeterminate_submission(err, submission)) + } +} + impl BatchBuilder<'_, AUTH> where AUTH: TransactionAuthenticator + Sync + 'static, @@ -192,32 +296,17 @@ where let executed_batch = BatchExecutor::new().execute(proposed_batch.clone())?; let proven_batch = LocalBatchProver::new().prove(executed_batch)?; - // 7. Seal each transaction's inputs, then submit via RPC. Each entry is sealed against its - // own transaction id. - let key = self.client.transaction_encryption_key().await?; - let sealed_inputs = tx_results - .iter() - .map(|tx_result| { - let executed = tx_result.executed_transaction(); - seal_transaction_inputs( - &mut self.client.rng, - &key, - executed.id(), - executed.tx_inputs(), - ) - }) - .collect::, _>>()?; + // 7. Submit via RPC. The proven batch is kept so an unconfirmed submission can be retried + // without executing or proving again. + let submission = ProvenBatchSubmission { + proven_batch, + proposed_batch: Box::new(proposed_batch), + tx_results, + }; + let block_num = self.client.submit_proven_batch(&submission).await?; + let tx_results = submission.tx_results; let mut updates: Vec = Vec::with_capacity(len); - let result = self - .client - .rpc_api - .submit_proven_batch(proven_batch, proposed_batch, sealed_inputs) - .await; - if let Err(err) = &result { - self.client.forget_stale_transaction_encryption_key(err).await; - } - let block_num = result?; // 8. Build per-tx TransactionStoreUpdates. for tx_result in &tx_results { @@ -337,3 +426,21 @@ where validate_executed_transaction(&executed_transaction, &prep.output_recipients)?; TransactionResult::new(executed_transaction, prep.future_notes) } + +/// Promotes a batch submission failure whose outcome is unknown, attaching everything a retry +/// needs. Any other failure is a rejection the node issued deliberately and passes through +/// unchanged. +fn promote_indeterminate_submission( + err: RpcError, + submission: &ProvenBatchSubmission, +) -> ClientError { + if !err.is_indeterminate_submission() { + return ClientError::RpcError(err); + } + + BatchBuilderError::BatchSubmissionOutcomeUnknown { + submission: Box::new(submission.clone()), + source: err, + } + .into() +} diff --git a/crates/rust-client/src/transaction/mod.rs b/crates/rust-client/src/transaction/mod.rs index 7cd1aa0c76..e5c7f81b30 100644 --- a/crates/rust-client/src/transaction/mod.rs +++ b/crates/rust-client/src/transaction/mod.rs @@ -116,10 +116,9 @@ use crate::store::{ TransactionFilter, }; use crate::sync::NoteTagRecord; -use crate::transaction::batch::InMemoryBatchDataStore; pub mod batch; -pub use batch::{BatchBuilder, BatchBuilderError}; +pub use batch::{BatchBuilder, BatchBuilderError, ProvenBatchSubmission}; mod chain_anchor; pub use chain_anchor::{ChainAnchor, ChainAnchorError}; @@ -219,23 +218,6 @@ where self.store.get_transactions(filter).await.map_err(Into::into) } - // TRANSACTION BATCH - // -------------------------------------------------------------------------------------------- - - /// Open a new [`BatchBuilder`] for accumulating transactions across one or more local - /// accounts. - /// - /// See [`crate::transaction::batch`] for usage and constraints. - pub fn new_transaction_batch(&mut self) -> BatchBuilder<'_, AUTH> { - let inner_data_store = ClientDataStore::new(self.store.clone(), self.rpc_api.clone()); - BatchBuilder { - client: self, - data_store: InMemoryBatchDataStore::new(inner_data_store), - pushed_txs: Vec::new(), - consumed_input_notes: BTreeSet::new(), - } - } - // TRANSACTION // -------------------------------------------------------------------------------------------- diff --git a/crates/testing/miden-client-tests/src/tests/batch.rs b/crates/testing/miden-client-tests/src/tests/batch.rs index 4c0d4fdb9b..500cdcda5f 100644 --- a/crates/testing/miden-client-tests/src/tests/batch.rs +++ b/crates/testing/miden-client-tests/src/tests/batch.rs @@ -9,11 +9,12 @@ use miden_client::auth::{AuthSchemeId, AuthSecretKey, AuthSingleSig, RPO_FALCON_ use miden_client::builder::ClientBuilder; use miden_client::keystore::{FilesystemKeyStore, Keystore}; use miden_client::note::{NoteType, NoteUpdateTracker}; -use miden_client::rpc::NodeRpcClient; +use miden_client::rpc::{GrpcError, NodeRpcClient, RpcEndpoint, RpcError}; use miden_client::store::{StoreError, TransactionFilter}; use miden_client::testing::common::{ MINT_AMOUNT, TRANSFER_AMOUNT, + TestClient, create_test_store_path, insert_new_fungible_faucet, insert_new_wallet, @@ -34,6 +35,7 @@ use miden_protocol::account::{ AccountBuilder, AccountComponent, AccountComponentMetadata, + AccountId, StorageMap, StorageMapKey, StorageSlot, @@ -57,16 +59,7 @@ use crate::tests::{create_test_client, seed_mock_transaction_encryption_key}; async fn submit_proven_batch_returns_chain_tip() { let (_client, rpc_api, _keystore) = Box::pin(create_test_client()).await; - // Pick the first account recorded in the prebuilt mock chain. - let account_id = rpc_api - .mock_chain - .read() - .proven_blocks() - .iter() - .flat_map(|block| block.body().updated_accounts()) - .next() - .unwrap() - .account_id(); + let account_id = rpc_api.first_account_id(); // Execute and prove a trivial transaction against that account. let tx_context = rpc_api @@ -105,16 +98,7 @@ async fn submit_proven_batch_returns_chain_tip() { async fn batch_builder_submits_two_txs_on_one_account() { let (mut client, rpc_api, _keystore) = Box::pin(create_test_client()).await; - // Pick the first tracked account in the mock chain (same pattern as the existing test above). - let account_id = rpc_api - .mock_chain - .read() - .proven_blocks() - .iter() - .flat_map(|block| block.body().updated_accounts()) - .next() - .unwrap() - .account_id(); + let account_id = rpc_api.first_account_id(); // Retrieve the committed account state from the mock chain and register it with the client // store so that `new_transaction_batch` can find it. @@ -538,18 +522,7 @@ async fn batch_builder_serves_witnesses_for_state_untouched_by_prior_push() { /// Verify that submitting an empty batch (no pushes) returns `BatchBuilderError::Empty`. #[tokio::test] async fn batch_builder_empty_submit_returns_empty_error() { - let (mut client, rpc_api, _keystore) = Box::pin(create_test_client()).await; - - // Pick the first tracked account in the mock chain. - let _account_id = rpc_api - .mock_chain - .read() - .proven_blocks() - .iter() - .flat_map(|block| block.body().updated_accounts()) - .next() - .unwrap() - .account_id(); + let (mut client, _rpc_api, _keystore) = Box::pin(create_test_client()).await; let batch = client.new_transaction_batch(); assert_eq!(batch.len(), 0); @@ -698,17 +671,9 @@ async fn batch_builder_submits_txs_across_multiple_accounts() { async fn batch_builder_push_for_unknown_account_returns_error() { let (mut client, rpc_api, _keystore) = Box::pin(create_test_client()).await; - // Pick an account that EXISTS on the mock chain but is NOT registered with the client - // store (we never call `client.add_account` for it). - let account_id = rpc_api - .mock_chain - .read() - .proven_blocks() - .iter() - .flat_map(|block| block.body().updated_accounts()) - .next() - .unwrap() - .account_id(); + // An account that EXISTS on the mock chain but is NOT registered with the client store + // (we never call `client.add_account` for it). + let account_id = rpc_api.first_account_id(); // Build a no-op request; we never get to submission — the push itself must fail. let req = TransactionRequestBuilder::new().build().unwrap(); @@ -816,6 +781,142 @@ async fn batch_builder_cross_account_note_flow() { ); } +/// Registers the mock chain's first account with `client` and returns its id, so a batch can be +/// pushed against it. The account uses `IncrNonce` auth, so no signing key is needed. +async fn register_mock_chain_account(client: &mut TestClient, rpc_api: &MockRpcApi) -> AccountId { + let account_id = rpc_api.first_account_id(); + + let account = rpc_api.mock_chain.read().committed_account(account_id).unwrap().clone(); + client.add_account(&account, false).await.unwrap(); + client.sync_state().await.unwrap(); + + account_id +} + +/// A batch submission that comes back without a definite outcome must hand the caller a payload +/// that submits again as-is, with nothing recorded locally in between. +#[tokio::test] +async fn indeterminate_batch_submission_is_retryable_with_the_attached_payload() { + let (mut client, rpc_api, _keystore) = Box::pin(create_test_client()).await; + let account_id = register_mock_chain_account(&mut client, &rpc_api).await; + + // The connection breaks while the response is in flight, so the node may or may not have + // taken the batch. + rpc_api.fail_next_call( + RpcEndpoint::SubmitProvenBatch, + RpcError::RequestError { + endpoint: RpcEndpoint::SubmitProvenBatch, + error_kind: GrpcError::Unknown("transport error".into()), + endpoint_error: None, + source: None, + }, + ); + + let err = Box::pin(async { + let mut batch = client.new_transaction_batch(); + batch + .push(account_id, TransactionRequestBuilder::new().build().unwrap()) + .await?; + batch + .push(account_id, TransactionRequestBuilder::new().build().unwrap()) + .await?; + batch.submit().await + }) + .await + .unwrap_err(); + + let ClientError::BatchBuilder(BatchBuilderError::BatchSubmissionOutcomeUnknown { + submission, + .. + }) = err + else { + panic!("expected BatchSubmissionOutcomeUnknown, got: {err:?}"); + }; + + // The payload describes the whole batch, not just one of its transactions. + assert_eq!(submission.transaction_count(), 2); + let tracked: BTreeSet<_> = submission.transaction_ids().collect(); + assert_eq!( + tracked.len(), + 2, + "each transaction in the batch must be trackable by its own id" + ); + + // Nothing was recorded, so the payload is all the caller has left to work with. + assert!(client.get_transactions(TransactionFilter::All).await.unwrap().is_empty()); + + // The staged failure is consumed before the mock records anything, so these two calls are the + // ones the mock sees. Both go out with the same payload. + Box::pin(client.submit_proven_batch(&submission)) + .await + .expect("the attached payload must be enough to submit again"); + Box::pin(client.submit_proven_batch(&submission)) + .await + .expect("the payload must stay usable across attempts"); + + // The retry entry point does not touch the store: syncing is what records the transactions. + assert!(client.get_transactions(TransactionFilter::All).await.unwrap().is_empty()); + + // Every attempt seals again rather than resending a cached ciphertext, which is what lets a + // retry survive a rotation of the validator set's encryption key. `seal_transaction_inputs` + // draws a fresh ephemeral key per call, so the same inputs seal to different bytes. + let attempts = rpc_api.submitted_batch_sealed_inputs(); + assert_eq!(attempts.len(), 2, "the mock must have seen exactly the two successful attempts"); + assert_eq!(attempts[0].len(), 2, "one sealed entry per transaction in the batch"); + assert_eq!(attempts[1].len(), attempts[0].len()); + for (first, second) in attempts[0].iter().zip(attempts[1].iter()) { + assert_eq!(first.key_id(), second.key_id(), "both attempts seal against the same key"); + assert_ne!( + first.ciphertext(), + second.ciphertext(), + "each attempt must seal again instead of reusing the previous ciphertext" + ); + } +} + +/// A rejection the node issued deliberately is an answer, so it must stay a plain +/// `ClientError::RpcError` and never promote to `BatchSubmissionOutcomeUnknown`. +#[tokio::test] +async fn deliberately_rejected_batch_submission_stays_an_rpc_error() { + let (mut client, rpc_api, _keystore) = Box::pin(create_test_client()).await; + let account_id = register_mock_chain_account(&mut client, &rpc_api).await; + + rpc_api.fail_next_call( + RpcEndpoint::SubmitProvenBatch, + RpcError::RequestError { + endpoint: RpcEndpoint::SubmitProvenBatch, + error_kind: GrpcError::FailedPrecondition, + endpoint_error: None, + source: None, + }, + ); + + let err = Box::pin(async { + let mut batch = client.new_transaction_batch(); + batch + .push(account_id, TransactionRequestBuilder::new().build().unwrap()) + .await?; + batch.submit().await + }) + .await + .unwrap_err(); + + match err { + ClientError::RpcError(RpcError::RequestError { + error_kind: GrpcError::FailedPrecondition, + .. + }) => {}, + other => panic!("expected ClientError::RpcError(FailedPrecondition), got: {other:?}"), + } + + // A `FailedPrecondition` from a submit endpoint evicts the cached encryption key + // unconditionally; this test pins that. + assert!( + client.test_store().get_transaction_encryption_key().await.unwrap().is_none(), + "a rejected submission must evict the stale encryption key" + ); +} + /// The duplicate-input-note check is global to the batch: a note consumed by `tx_a` (account A) /// cannot also appear as an input to `tx_b` (account B). Second push fails with /// `DuplicateInputNote(note_id)`. From 66f89c4b62c62f28eabcb96310215dc385860515 Mon Sep 17 00:00:00 2001 From: Sofi Azcoaga Date: Mon, 7 Sep 2026 14:59:38 -0300 Subject: [PATCH 2/4] refactor(rust-client): rename to retry_proven_batch and fix the retry docs --- crates/rust-client/src/errors.rs | 9 +-- crates/rust-client/src/test_utils/mock.rs | 18 +++--- .../src/transaction/batch/error.rs | 3 +- .../rust-client/src/transaction/batch/mod.rs | 44 +++++++++------ .../miden-client-tests/src/tests/batch.rs | 56 +++++++------------ 5 files changed, 65 insertions(+), 65 deletions(-) diff --git a/crates/rust-client/src/errors.rs b/crates/rust-client/src/errors.rs index 2e869177e7..c50973c02d 100644 --- a/crates/rust-client/src/errors.rs +++ b/crates/rust-client/src/errors.rs @@ -371,12 +371,13 @@ impl From<&ClientError> for Option { }) => Some(ErrorHint { message: format!( "Do not rebuild the batch: re-executing produces new transaction ids over \ - the same notes, so if the original did land you would be left tracking ids \ - that can never commit. Neither option can apply the batch twice, since both \ + the same notes, so if the original did land you would be left with ids that \ + can never commit. Neither option can apply the batch twice, since both \ consume the same nullifiers. Either retry with the `submission` attached to \ this error, which carries the proven batch and each transaction's inputs, or \ - keep syncing and check `get_transactions` for the {} ids in \ - `submission.transaction_ids()` until they commit or expire.", + sync and see whether the accounts moved: nothing was recorded locally, so \ + the {} ids in `submission.transaction_ids()` never show up in \ + `get_transactions`.", submission.transaction_count() ), docs_url: Some(TROUBLESHOOTING_DOC), diff --git a/crates/rust-client/src/test_utils/mock.rs b/crates/rust-client/src/test_utils/mock.rs index a5748177eb..2637534aa0 100644 --- a/crates/rust-client/src/test_utils/mock.rs +++ b/crates/rust-client/src/test_utils/mock.rs @@ -84,8 +84,9 @@ pub struct MockRpcApi { /// [`MockRpcApi::fail_next_call`]. An entry is removed when served, so the call after it /// answers normally and a test can exercise a retry. next_call_failures: Arc>>, - /// Sealed inputs handed to `submit_proven_batch`, one entry per call that reached the mock, so - /// a test can assert that a resubmission sealed again instead of reusing a cached ciphertext. + /// Sealed inputs handed to `submit_proven_batch`, one entry per call and recorded before any + /// staged failure is served, so a test can assert that a resubmission sealed again instead of + /// reusing a cached ciphertext. submitted_batch_sealed_inputs: Arc>>>, } @@ -118,7 +119,7 @@ impl MockRpcApi { /// Id of the first account updated in the mock chain's proven blocks, in block then /// within-block order. Tests use it to get hold of an account the chain already knows. /// - /// Panics if the chain has no account updates, which for a mock means the test set it up wrong. + /// Panics if the chain has no account updates. pub fn first_account_id(&self) -> AccountId { self.mock_chain .read() @@ -130,8 +131,9 @@ impl MockRpcApi { .account_id() } - /// Sealed inputs recorded by `submit_proven_batch`, one entry per call that reached the mock. - /// Within an entry the order matches the batch's transaction order. + /// Sealed inputs recorded by `submit_proven_batch`, one entry per call, including calls that + /// went on to be served a staged failure. Within an entry the order matches the batch's + /// transaction order. pub fn submitted_batch_sealed_inputs(&self) -> Vec> { self.submitted_batch_sealed_inputs.read().clone() } @@ -585,12 +587,14 @@ impl NodeRpcClient for MockRpcApi { _proposed_batch: ProposedBatch, sealed_transaction_inputs: Vec, ) -> Result { + // Recorded before the staged failure is served: a submission whose response is lost still + // reached the node, so a test can compare what that attempt sent against the retry. + self.submitted_batch_sealed_inputs.write().push(sealed_transaction_inputs); + if let Some(error) = self.take_failure(RpcEndpoint::SubmitProvenBatch) { return Err(error); } - self.submitted_batch_sealed_inputs.write().push(sealed_transaction_inputs); - let mut mock_chain = self.mock_chain.write(); mock_chain.add_pending_batch(proven_batch); drop(mock_chain); diff --git a/crates/rust-client/src/transaction/batch/error.rs b/crates/rust-client/src/transaction/batch/error.rs index 03f335f4e4..d17873ad8e 100644 --- a/crates/rust-client/src/transaction/batch/error.rs +++ b/crates/rust-client/src/transaction/batch/error.rs @@ -29,8 +29,7 @@ pub enum BatchBuilderError { )] BatchSubmissionOutcomeUnknown { /// The batch as submitted, to resend with - /// [`Client::submit_proven_batch`](crate::Client::submit_proven_batch) or to read - /// `transaction_ids()` from and track. + /// [`Client::retry_proven_batch`](crate::Client::retry_proven_batch). submission: Box, #[source] source: RpcError, diff --git a/crates/rust-client/src/transaction/batch/mod.rs b/crates/rust-client/src/transaction/batch/mod.rs index 3ac074d274..61e5a4ee1b 100644 --- a/crates/rust-client/src/transaction/batch/mod.rs +++ b/crates/rust-client/src/transaction/batch/mod.rs @@ -39,7 +39,7 @@ //! A submission that comes back without a definite outcome raises //! [`BatchBuilderError::BatchSubmissionOutcomeUnknown`]. The node may or may not have accepted the //! batch and nothing was recorded locally, so the error carries a [`ProvenBatchSubmission`] to -//! resend with [`Client::submit_proven_batch`] or to track by its transaction ids. +//! resend with [`Client::retry_proven_batch`]. //! //! Once the node accepts the batch, the local store still needs to be updated. If that step //! fails, the caller receives one of two errors that both carry the accepted `block_num`: @@ -49,7 +49,9 @@ //! - [`BatchBuilderError::BatchSubmittedButApplyFailed`] — applying the updates atomically to the //! local store failed. //! -//! In all three cases `sync_state` reconciles the local store with what the network holds. +//! In all three cases `sync_state` reconciles the accounts with what the network holds. It does +//! not create transaction records: none were written, so these transactions never appear in +//! `get_transactions`. mod data_store; mod error; @@ -86,17 +88,13 @@ use crate::{Client, ClientError}; /// outcome the node never confirmed can be retried without executing or proving again. /// /// Handed back by [`BatchBuilderError::BatchSubmissionOutcomeUnknown`] and accepted by -/// [`Client::submit_proven_batch`]. +/// [`Client::retry_proven_batch`]. #[derive(Debug, Clone)] pub struct ProvenBatchSubmission { proven_batch: ProvenBatch, - // Boxed on the protocol type's own advice: it is large, and this struct travels inside an - // error. proposed_batch: Box, - /// Kept whole rather than as inputs or sealed inputs. Not sealed, because the validator set's - /// key can rotate between attempts and a retry has to seal against the current one. Whole - /// results, because `BatchBuilder::submit` needs them after the RPC to build the store - /// updates. + /// The validator set's key can rotate between attempts, so a retry has to seal these again, + /// and `BatchBuilder::submit` needs the whole results after the RPC for the store updates. tx_results: Vec, } @@ -106,7 +104,8 @@ impl ProvenBatchSubmission { self.tx_results.len() } - /// Ids of the transactions in the batch, to track until a sync resolves them. + /// Ids the batch was submitted with. The client recorded nothing, so they do not appear in + /// `get_transactions`; they are for the caller's own bookkeeping. pub fn transaction_ids(&self) -> impl Iterator + '_ { self.tx_results.iter().map(|tx_result| tx_result.executed_transaction().id()) } @@ -168,18 +167,29 @@ where /// being executed or proven a second time. The batch id is fixed, so resending it cannot /// duplicate its effects, but the node rejects it as a conflict if the original did land. /// - /// Unlike [`Client::submit_proven_transaction`](crate::Client::submit_proven_transaction) this - /// is not a general-purpose submit: [`ProvenBatchSubmission`] has no public constructor, so the - /// only way to obtain one is from that error. Assembling and proving a batch goes through - /// [`BatchBuilder`]. + /// That error is the only source of a [`ProvenBatchSubmission`]: the type has no public + /// constructor, and assembling and proving a batch goes through [`BatchBuilder`]. /// - /// The local store is not touched. On success, sync to record the transactions. + /// The local store is not touched. These transactions have no local record, so they never + /// appear in `get_transactions`; a sync shows their effect on the accounts instead. /// /// # Errors /// /// Returns [`BatchBuilderError::BatchSubmissionOutcomeUnknown`] when the submission comes back /// without a definite answer. Every other failure is a rejection the node issued deliberately. - pub async fn submit_proven_batch( + pub async fn retry_proven_batch( + &mut self, + submission: &ProvenBatchSubmission, + ) -> Result { + self.send_proven_batch(submission).await + } + + /// Seals the submission's inputs against the current key and sends the batch, mapping an + /// outcome the node never confirmed to the error that carries the submission back. + /// + /// Shared by the first send from [`BatchBuilder::submit`] and by every retry through + /// [`Client::retry_proven_batch`], so the sealing and the error mapping live in one place. + async fn send_proven_batch( &mut self, submission: &ProvenBatchSubmission, ) -> Result { @@ -303,7 +313,7 @@ where proposed_batch: Box::new(proposed_batch), tx_results, }; - let block_num = self.client.submit_proven_batch(&submission).await?; + let block_num = self.client.send_proven_batch(&submission).await?; let tx_results = submission.tx_results; let mut updates: Vec = Vec::with_capacity(len); diff --git a/crates/testing/miden-client-tests/src/tests/batch.rs b/crates/testing/miden-client-tests/src/tests/batch.rs index 500cdcda5f..6ded9ad959 100644 --- a/crates/testing/miden-client-tests/src/tests/batch.rs +++ b/crates/testing/miden-client-tests/src/tests/batch.rs @@ -98,15 +98,7 @@ async fn submit_proven_batch_returns_chain_tip() { async fn batch_builder_submits_two_txs_on_one_account() { let (mut client, rpc_api, _keystore) = Box::pin(create_test_client()).await; - let account_id = rpc_api.first_account_id(); - - // Retrieve the committed account state from the mock chain and register it with the client - // store so that `new_transaction_batch` can find it. - let account = rpc_api.mock_chain.read().committed_account(account_id).unwrap().clone(); - client.add_account(&account, false).await.unwrap(); - - // Sync so the client's store reflects the on-chain state. - client.sync_state().await.unwrap(); + let account_id = register_mock_chain_account(&mut client, &rpc_api).await; // Build two minimal no-op TransactionRequests for the same account. // The mock account uses IncrNonce auth which requires no signing key — a bare @@ -781,8 +773,9 @@ async fn batch_builder_cross_account_note_flow() { ); } -/// Registers the mock chain's first account with `client` and returns its id, so a batch can be -/// pushed against it. The account uses `IncrNonce` auth, so no signing key is needed. +/// Registers the mock chain's first account with `client`, syncing so the store reflects the +/// on-chain state, and returns its id so a batch can be pushed against it. The account uses +/// `IncrNonce` auth, so no signing key is needed. async fn register_mock_chain_account(client: &mut TestClient, rpc_api: &MockRpcApi) -> AccountId { let account_id = rpc_api.first_account_id(); @@ -835,47 +828,40 @@ async fn indeterminate_batch_submission_is_retryable_with_the_attached_payload() // The payload describes the whole batch, not just one of its transactions. assert_eq!(submission.transaction_count(), 2); - let tracked: BTreeSet<_> = submission.transaction_ids().collect(); - assert_eq!( - tracked.len(), - 2, - "each transaction in the batch must be trackable by its own id" - ); + let ids: BTreeSet<_> = submission.transaction_ids().collect(); + assert_eq!(ids.len(), 2, "the payload must expose one distinct id per transaction"); // Nothing was recorded, so the payload is all the caller has left to work with. assert!(client.get_transactions(TransactionFilter::All).await.unwrap().is_empty()); - // The staged failure is consumed before the mock records anything, so these two calls are the - // ones the mock sees. Both go out with the same payload. - Box::pin(client.submit_proven_batch(&submission)) + Box::pin(client.retry_proven_batch(&submission)) .await .expect("the attached payload must be enough to submit again"); - Box::pin(client.submit_proven_batch(&submission)) - .await - .expect("the payload must stay usable across attempts"); - // The retry entry point does not touch the store: syncing is what records the transactions. + // The retry entry point writes no rows, and a sync will not create them: these transactions + // never appear in `get_transactions`. assert!(client.get_transactions(TransactionFilter::All).await.unwrap().is_empty()); - // Every attempt seals again rather than resending a cached ciphertext, which is what lets a - // retry survive a rotation of the validator set's encryption key. `seal_transaction_inputs` - // draws a fresh ephemeral key per call, so the same inputs seal to different bytes. + // The retry seals again rather than resending the ciphertext the first attempt sent, which is + // what lets it survive a rotation of the validator set's encryption key. + // `seal_transaction_inputs` draws a fresh ephemeral key per call, so the same inputs seal to + // different bytes. let attempts = rpc_api.submitted_batch_sealed_inputs(); - assert_eq!(attempts.len(), 2, "the mock must have seen exactly the two successful attempts"); + assert_eq!(attempts.len(), 2, "the mock must have seen the lost attempt and the retry"); assert_eq!(attempts[0].len(), 2, "one sealed entry per transaction in the batch"); assert_eq!(attempts[1].len(), attempts[0].len()); - for (first, second) in attempts[0].iter().zip(attempts[1].iter()) { - assert_eq!(first.key_id(), second.key_id(), "both attempts seal against the same key"); + for (lost, retry) in attempts[0].iter().zip(attempts[1].iter()) { + assert_eq!(lost.key_id(), retry.key_id(), "both attempts seal against the same key"); assert_ne!( - first.ciphertext(), - second.ciphertext(), - "each attempt must seal again instead of reusing the previous ciphertext" + lost.ciphertext(), + retry.ciphertext(), + "the retry must seal again instead of reusing the first attempt's ciphertext" ); } } -/// A rejection the node issued deliberately is an answer, so it must stay a plain -/// `ClientError::RpcError` and never promote to `BatchSubmissionOutcomeUnknown`. +/// A deliberate rejection stays a plain `ClientError::RpcError` instead of promoting to +/// `BatchSubmissionOutcomeUnknown`. `FailedPrecondition` also evicts the cached encryption key. #[tokio::test] async fn deliberately_rejected_batch_submission_stays_an_rpc_error() { let (mut client, rpc_api, _keystore) = Box::pin(create_test_client()).await; From 6ef6ae387b658d7a9e2a268f248b66578b382015 Mon Sep 17 00:00:00 2001 From: Sofi Azcoaga Date: Mon, 7 Sep 2026 14:59:49 -0300 Subject: [PATCH 3/4] docs: add changelog entries --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 07eb7e3356..c56e45cb36 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,10 @@ * [BREAKING][behavior][rust] A transaction submission that comes back without a definite outcome no longer surfaces as `ClientError::RpcError`. `Client::submit_proven_transaction`, and every path through it, returns the new `ClientError::SubmissionOutcomeUnknown`, which carries the `ProvenTransaction` and the `TransactionInputs` it was submitted with, so the caller can hand them straight back to `submit_proven_transaction` without executing or proving again, or track the transaction id until a sync resolves it. Rejections the node issues deliberately are unaffected. Code matching on `ClientError::RpcError` for submission failures still compiles but stops matching these cases. The classification is available as `RpcError::is_indeterminate_submission` ([#2498](https://github.com/0xMiden/rust-sdk/pull/2498)). +* [BREAKING][behavior][rust] `BatchBuilder::submit` returns the new `BatchBuilderError::BatchSubmissionOutcomeUnknown` when a submission comes back without a definite outcome, instead of `ClientError::RpcError`. It carries a `ProvenBatchSubmission` to resend with `Client::retry_proven_batch`. Rejections the node issues deliberately are unaffected, so code matching `ClientError::RpcError` still compiles but stops matching these cases ([#2508](https://github.com/0xMiden/rust-sdk/pull/2508)). + +* [BREAKING][type][rust] Added the `BatchBuilderError::BatchSubmissionOutcomeUnknown` variant, so exhaustive matches on `BatchBuilderError` must handle it ([#2508](https://github.com/0xMiden/rust-sdk/pull/2508)). + ### Fixes * [FIX][test] The integration tests now run against a fee-charging chain. The testing node's genesis charges a fee by default (`MIDEN_VERIFICATION_BASE_FEE`, default `500`), generates the native fee faucet itself so the accounts it deploys can be seeded with that asset, and pre-funds a pool of basic wallets the suite draws from via a new `--funders` argument (`MIDEN_FUNDER_ACCOUNTS_DIR`). Accounts created by the `miden_client::testing::common` helpers are funded and deployed automatically, and `miden_client::testing::fee::deploy_account` does the same for accounts a test builds itself. The AggLayer accounts are consequently always part of genesis (the `AGGLAYER_GENESIS` env var and the `start-node-agglayer` target are gone) and the AggLayer tests load them from `AGGLAYER_ACCOUNTS_DIR` ([#2446](https://github.com/0xMiden/rust-sdk/issues/2446)). @@ -63,6 +67,7 @@ ### Enhancements +* [FEATURE][rust] `Client::retry_proven_batch` resends a batch whose outcome was never confirmed, sealing the transaction inputs again on every attempt so nothing is executed or proven twice. It takes the `ProvenBatchSubmission` from `BatchBuilderError::BatchSubmissionOutcomeUnknown`, which has no public constructor, so that error is the only way to obtain one ([#2508](https://github.com/0xMiden/rust-sdk/pull/2508)). * [FEATURE][rust] `ClientBuilder` accepts any `TransactionAuthenticator + 'static` as its authenticator. The `BuilderAuthenticator` bound no longer requires `Keystore` or `From`, so a signer that holds no secret key, such as a remote signing service, can be plugged into the builder without implementing key management. * [FEATURE][rust] Added `AuthGuardedMultisig`, `AuthGuardedMultisigConfig`, `GuardianConfig` and `ApproverSet` to `miden_client::auth`, which previously exposed only the single- and multisig components. Building a guarded multisig account no longer means reaching past the client into `miden_standards` ([#2465](https://github.com/0xMiden/rust-sdk/pull/2465)). * [FEATURE][rust] `Client::sync_state` now issues its independent gRPC calls concurrently instead of one after another, reducing the total time a sync takes. `NodeRpcClient::sync_notes_with_content` and `NodeRpcClient::sync_transactions` are now called concurrently rather than in sequence, and the per-account `NodeRpcClient::get_account` requests are issued in parallel instead of one at a time ([#2420](https://github.com/0xMiden/rust-sdk/pull/2420)). From d10932d2b0123f66f9aeb567f30f63ef14dda372 Mon Sep 17 00:00:00 2001 From: Sofi Azcoaga Date: Mon, 7 Sep 2026 15:57:34 -0300 Subject: [PATCH 4/4] fix(rust-client): record the batch when a retry is accepted --- crates/rust-client/src/errors.rs | 8 +- .../rust-client/src/transaction/batch/mod.rs | 79 ++++++++++--------- .../miden-client-tests/src/tests/batch.rs | 13 ++- 3 files changed, 55 insertions(+), 45 deletions(-) diff --git a/crates/rust-client/src/errors.rs b/crates/rust-client/src/errors.rs index c50973c02d..e7913c7195 100644 --- a/crates/rust-client/src/errors.rs +++ b/crates/rust-client/src/errors.rs @@ -374,10 +374,10 @@ impl From<&ClientError> for Option { the same notes, so if the original did land you would be left with ids that \ can never commit. Neither option can apply the batch twice, since both \ consume the same nullifiers. Either retry with the `submission` attached to \ - this error, which carries the proven batch and each transaction's inputs, or \ - sync and see whether the accounts moved: nothing was recorded locally, so \ - the {} ids in `submission.transaction_ids()` never show up in \ - `get_transactions`.", + this error, which carries the proven batch and each transaction's inputs and \ + records the batch if the node accepts it, or sync and see whether the \ + accounts moved: until a retry is accepted the {} ids in \ + `submission.transaction_ids()` have no record to look up.", submission.transaction_count() ), docs_url: Some(TROUBLESHOOTING_DOC), diff --git a/crates/rust-client/src/transaction/batch/mod.rs b/crates/rust-client/src/transaction/batch/mod.rs index 61e5a4ee1b..7976655e5e 100644 --- a/crates/rust-client/src/transaction/batch/mod.rs +++ b/crates/rust-client/src/transaction/batch/mod.rs @@ -50,8 +50,9 @@ //! local store failed. //! //! In all three cases `sync_state` reconciles the accounts with what the network holds. It does -//! not create transaction records: none were written, so these transactions never appear in -//! `get_transactions`. +//! not create transaction records, though: syncing updates records the client already holds and +//! never inserts missing ones. For the unknown outcome an accepted retry writes them; for the two +//! post-accept errors nothing will, since neither carries the updates that failed. mod data_store; mod error; @@ -104,8 +105,8 @@ impl ProvenBatchSubmission { self.tx_results.len() } - /// Ids the batch was submitted with. The client recorded nothing, so they do not appear in - /// `get_transactions`; they are for the caller's own bookkeeping. + /// Ids the batch was submitted with. Nothing is recorded for them yet, so they reach + /// `get_transactions` only once a retry is accepted. pub fn transaction_ids(&self) -> impl Iterator + '_ { self.tx_results.iter().map(|tx_result| tx_result.executed_transaction().id()) } @@ -170,8 +171,10 @@ where /// That error is the only source of a [`ProvenBatchSubmission`]: the type has no public /// constructor, and assembling and proving a batch goes through [`BatchBuilder`]. /// - /// The local store is not touched. These transactions have no local record, so they never - /// appear in `get_transactions`; a sync shows their effect on the accounts instead. + /// A retry the node accepts records the batch the way the first send would have, so the + /// transactions reach the store no matter which attempt landed. A retry the node rejects + /// records nothing, and neither will a later sync: syncing updates records the client already + /// holds and never inserts missing ones. /// /// # Errors /// @@ -181,15 +184,16 @@ where &mut self, submission: &ProvenBatchSubmission, ) -> Result { - self.send_proven_batch(submission).await + self.send_and_apply_proven_batch(submission).await } - /// Seals the submission's inputs against the current key and sends the batch, mapping an - /// outcome the node never confirmed to the error that carries the submission back. + /// Seals the submission's inputs against the current key, sends the batch, and on acceptance + /// applies the per-transaction store updates atomically. /// /// Shared by the first send from [`BatchBuilder::submit`] and by every retry through - /// [`Client::retry_proven_batch`], so the sealing and the error mapping live in one place. - async fn send_proven_batch( + /// [`Client::retry_proven_batch`], so both record what the node took and both map an + /// unconfirmed outcome to the error that carries the submission back. + async fn send_and_apply_proven_batch( &mut self, submission: &ProvenBatchSubmission, ) -> Result { @@ -216,7 +220,29 @@ where self.forget_stale_transaction_encryption_key(err).await; } - result.map_err(|err| promote_indeterminate_submission(err, submission)) + let block_num = result.map_err(|err| promote_indeterminate_submission(err, submission))?; + + // The node took the batch. Record it, one update per transaction, applied atomically. + let mut updates: Vec = + Vec::with_capacity(submission.transaction_count()); + for tx_result in &submission.tx_results { + let update = self.get_transaction_store_update(tx_result, block_num).await.map_err( + |source| BatchBuilderError::BatchSubmittedButUpdateBuildFailed { + block_num, + source, + }, + )?; + updates.push(update); + } + + if let Err(source) = self.store.apply_transaction_batch(updates).await { + return Err(ClientError::from(BatchBuilderError::BatchSubmittedButApplyFailed { + block_num, + source, + })); + } + + Ok(block_num) } } @@ -306,37 +332,14 @@ where let executed_batch = BatchExecutor::new().execute(proposed_batch.clone())?; let proven_batch = LocalBatchProver::new().prove(executed_batch)?; - // 7. Submit via RPC. The proven batch is kept so an unconfirmed submission can be retried - // without executing or proving again. + // 7. Submit via RPC and record what the node took. The proven batch is kept so an + // unconfirmed submission can be retried without executing or proving again. let submission = ProvenBatchSubmission { proven_batch, proposed_batch: Box::new(proposed_batch), tx_results, }; - let block_num = self.client.send_proven_batch(&submission).await?; - let tx_results = submission.tx_results; - - let mut updates: Vec = Vec::with_capacity(len); - - // 8. Build per-tx TransactionStoreUpdates. - for tx_result in &tx_results { - let update = - self.client.get_transaction_store_update(tx_result, block_num).await.map_err( - |source| BatchBuilderError::BatchSubmittedButUpdateBuildFailed { - block_num, - source, - }, - )?; - updates.push(update); - } - - // 9. Apply atomically; if it fails, return BatchSubmittedButApplyFailed. - if let Err(source) = self.client.store.apply_transaction_batch(updates).await { - return Err(ClientError::from(BatchBuilderError::BatchSubmittedButApplyFailed { - block_num, - source, - })); - } + let block_num = self.client.send_and_apply_proven_batch(&submission).await?; Ok(block_num) } diff --git a/crates/testing/miden-client-tests/src/tests/batch.rs b/crates/testing/miden-client-tests/src/tests/batch.rs index 6ded9ad959..87d6ac5e7f 100644 --- a/crates/testing/miden-client-tests/src/tests/batch.rs +++ b/crates/testing/miden-client-tests/src/tests/batch.rs @@ -838,9 +838,16 @@ async fn indeterminate_batch_submission_is_retryable_with_the_attached_payload() .await .expect("the attached payload must be enough to submit again"); - // The retry entry point writes no rows, and a sync will not create them: these transactions - // never appear in `get_transactions`. - assert!(client.get_transactions(TransactionFilter::All).await.unwrap().is_empty()); + // A retry the node accepted records the batch, so the ids the payload handed over are the ones + // now in the store. Nothing else can write them: they have no record until this point. + let recorded: BTreeSet<_> = client + .get_transactions(TransactionFilter::All) + .await + .unwrap() + .iter() + .map(|tx| tx.id) + .collect(); + assert_eq!(recorded, ids, "the retry must record exactly the payload's transactions"); // The retry seals again rather than resending the ciphertext the first attempt sent, which is // what lets it survive a rotation of the validator set's encryption key.