Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)).
Expand All @@ -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<FilesystemKeyStore>`, 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)).
Expand Down
17 changes: 17 additions & 0 deletions crates/rust-client/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,23 @@ impl From<&ClientError> for Option<ErrorHint> {
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 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 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),
}),
_ => None,
}
}
Expand Down
43 changes: 39 additions & 4 deletions crates/rust-client/src/test_utils/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ 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<RwLock<BTreeMap<&'static str, RpcError>>>,
/// 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<RwLock<Vec<Vec<SealedTransactionInputs>>>>,
}

impl Default for MockRpcApi {
Expand All @@ -108,9 +112,32 @@ 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.
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, 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<Vec<SealedTransactionInputs>> {
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.
///
Expand Down Expand Up @@ -551,15 +578,23 @@ 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 unusedthe 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<SealedTransactionInputs>,
sealed_transaction_inputs: Vec<SealedTransactionInputs>,
) -> Result<BlockNumber, RpcError> {
// 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);
}

let mut mock_chain = self.mock_chain.write();
mock_chain.add_pending_batch(proven_batch);
drop(mock_chain);
Expand Down
19 changes: 19 additions & 0 deletions crates/rust-client/src/transaction/batch/error.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -16,6 +20,21 @@ 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::retry_proven_batch`](crate::Client::retry_proven_batch).
submission: Box<ProvenBatchSubmission>,
#[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.
Expand Down
Loading
Loading