feat(client): distinct ApplyTransactionAfterSubmitFailed error + one retry

[WiktorStarczewski]

Closes 0xMiden/web-sdk#122

Summary

• Adds a distinct ClientError::ApplyTransactionAfterSubmitFailed { tx_id, submission_height, source } error variant for the case where submit_proven_transaction succeeds but apply_transaction fails
• Retries the apply call once before surfacing the error (covers transient IndexedDB issues)
• The error message explicitly tells callers: the tx is on-chain, don't retry, sync will reconcile

Background

The SDK's transaction pipeline is execute → prove → submit → apply. Between submit succeeding (tx admitted to mempool) and apply completing (local IndexedDB updated), the transaction is live on chain but the client's local state doesn't reflect it.

If apply fails in this window (transient IDB error, service worker killed, browser tab closed), callers previously saw a generic ClientError with no way to distinguish it from pre-submit failures. The natural response is to retry the entire transaction — but the node rejects it because the nullifiers are already consumed. The retry loop fails indefinitely.

This was surfaced by the wallet's stress suite: several transactions hit the submit-succeeded-apply-failed gap during page-reload perturbations, causing the TransactionProcessor to retry indefinitely while showing incorrect balances.

Changes

• crates/rust-client/src/errors.rs:
  • New ApplyTransactionAfterSubmitFailed variant with tx_id, submission_height, and source
  • ErrorHint implementation telling the caller what happened and what to do
  • BlockNumber import added for the submission_height field
• crates/rust-client/src/transaction/mod.rs:
  • submit_new_transaction_with_prover now wraps the apply call in a single retry
  • On second failure, constructs ApplyTransactionAfterSubmitFailed with the submission height from the successful submit

How the wallet uses it

The wallet's TransactionProcessor catches this variant (via errorCode from PR #2060) and marks the transaction as Completed instead of Failed. The user sees "Transaction sent" (correct) rather than "Transaction failed" (incorrect). The next sync reconciles note states via ConsumedExternal automatically.

Test plan

• Existing tests pass
• Simulate apply failure after successful submit → error type is ApplyTransactionAfterSubmitFailed
• Error contains correct tx_id and submission_height
• Transient apply failure (first attempt fails, second succeeds) → no error surfaced

[igamigo]

I think I would overall discard these changes. The suggestion does not really apply, and the user can implement their own recovery mechanisms by using the split transaction APIs (not only for the local store, but for submission or proving as well). A middle ground may be to return an error variant that wraps the store update so that the user can later try to apply it again, but not sure I would do that either.
Alternatively, if there is a quota to stores such as IndexedDB, could it be configured? Could the retry mechanism be applied at the store level? Lastly, would a solution such as this one solve the case where there is a page reload like the description suggests?

[igamigo]

This is not fully correct: the sync may not reconcile state if the account is private, so we should avoid suggesting this.

Additionally, the transaction might have reached the network at this point but this tells us nothing about whether the new state of notes and accounts will actually get finalized, so I wouldn't directly suggest that either.

[igamigo]

Also nit (feel free to disregard): the comment is a bit verbose. I'd go with something like

        // The transaction is already on-chain at this point. If
        // `apply_transaction` fails, local state may diverge from the network's state.
        //
        // Retry once before returning an error that makes clear the transaction
        // was submitted, since some failures may be transient.

[WiktorStarczewski]

Addressed in 8c30fab — same response as on the hint thread below. The comment and hint text now only claim "the node accepted it into the mempool" rather than "it's on-chain and will reconcile." For private accounts specifically, sync cannot restore state from the network at all, so the pending TransactionStoreUpdate attached to the error is now the authoritative recovery path (not sync).

[WiktorStarczewski]

Done in 8c30fab — shortened the comment to 4 lines, in the spirit of your version but also updated to match the new flow (build the update once, retry the write, attach the pending update on failure).

[igamigo]

Is IDB IndexedDB here? If so, what kind of quotas are there? Mostly out of curiosity

[WiktorStarczewski]

Yes, IndexedDB. Per-origin quotas in browsers are split into a soft and hard limit:

• Safari: ~1 GB per origin soft; prompts the user when approaching it. Also evicts aggressively under storage pressure (especially for sites not added to the home screen).
• Chromium: soft cap of 60% of total disk; hard cap at 80%. In practice tabs can fail writes well below that if the profile is large or under memory pressure.
• Firefox: 10% of available disk per group, capped at 10 GB.

The writes we've seen fail weren't about hitting the quota — they were transient failures where the next write in the same session succeeded (service worker killed mid-transaction, the OS paging out IDB under pressure, the wallet's stress suite doing a page reload mid-commit, etc.). Hence the single in-memory retry before we surface the error: cheap, resolves the transient class, and doesn't paper over genuinely full quotas.

Configuring the quota isn't really a lever we have on the web (it's browser policy, not our storage layer). Applying retry at the store level is reasonable for idempotent single-row writes, but the apply step is multi-table (accounts + notes + tags + future notes + tx record) and the right retry boundary is the whole TransactionStoreUpdate, which is what this PR retries.

[igamigo]

Same as above: this is not fully correct

[WiktorStarczewski]

Fixed in 8c30fab — the hint message no longer promises sync reconciliation or "on-chain permanence." It now:

1. Says the tx was accepted by the node (mempool admission, not finality).
2. Points the caller at the attached pending_update field and tells them to re-apply via Client::apply_transaction_update rather than trust sync.
3. Warns that resubmitting will fail because the mempool-accepted copy has already consumed the nullifiers.

This avoids both the finality overstatement and the private-account issue you flagged.

[igamigo]

Same here, the on-chain effect may indeed not be durable, and also the sync may reconcile notes but will make private accounts corrupt because their state cannot be retrieved from the network

[WiktorStarczewski]

You're right on both points. Pushed 8c30fab which:

• drops the "on-chain effect is durable" language — replaced with "accepted by the node at block N" (mempool admission, not finality)
• drops the "sync will reconcile via ConsumedExternal" promise — this is wrong for private accounts, which sync can only detect as mismatched (→ locked), not restore from the network

Instead the variant now attaches the pending TransactionStoreUpdate, so the caller has a concrete recovery path independent of account privacy: persist it, re-apply later via apply_transaction_update. That's the middle ground you suggested in the top-level review.

[WiktorStarczewski]

Thanks for the thorough review. All three of your technical concerns were correct, and I pushed 8c30fab which restructures the PR around your middle-ground suggestion rather than the original "trust sync" framing.

What changed

• Dropped the "on-chain effect is durable" language. submit_proven_transaction returning Ok means mempool admission, not finalization. Error message now says "accepted by the node at block N," which is what we actually know.

• Dropped the "sync will reconcile via ConsumedExternal" promise. You're right that this is wrong for private accounts — sync can only detect commitment mismatch on those (→ AccountLocked), it cannot restore state from the network. The original message could have led callers to rely on a reconciliation path that silently corrupts private account state.

• Attached the pending TransactionStoreUpdate to the error variant. This is the middle-ground path you described. The variant signature is now:

  ApplyTransactionAfterSubmitFailed {
      tx_id: TransactionId,
      submission_height: BlockNumber,
      pending_update: Box<TransactionStoreUpdate>,
      source: Box<ClientError>,
  }

  Callers can persist the update and later retry Client::apply_transaction_update. That works uniformly for public and private accounts, since the update was computed from the local execution and doesn't depend on pulling state back from the network.

• Retry now wraps apply_transaction_update instead of apply_transaction. Previously both attempts also rebuilt the store update from scratch; now we build it once and retry only the write.

On whether to keep this at all

I hear you on "users can do this themselves with the split APIs." That's true — the execute / prove / submit / apply surface is already public. But the wallet's stress suite showed the default-path users (the ones calling submit_new_transaction) can't distinguish "submit failed" from "submit succeeded, apply failed" without this variant, and their natural retry loop resubmits forever against a node that now rejects it. A distinct error type + a single transient-failure retry is the smallest change that closes that trap for default-path callers while still giving advanced callers the split APIs.

Happy to go further (or less far) — e.g. drop the built-in retry and just keep the distinct variant with the attached update; or go the other direction and gate it behind an explicit opt-in. Let me know which direction you'd prefer.

IDB quota sub-thread

Answered inline — short version: the failures we saw weren't quota-related but transient (service worker churn, page reload mid-commit), which is why a single in-memory retry helps. Genuine quota exhaustion would fail both attempts and still surface ApplyTransactionAfterSubmitFailed with the pending update attached, so the caller gets the recovery path regardless.

[igamigo]

I think I still disagree with trying to apply the update again. You mention:

(service worker killed mid-transaction, the OS paging out IDB under pressure, the wallet's stress suite doing a page reload mid-commit, etc.

Retrying once does not help with any of these, does it? I also don't think these are precisely the right problems to solve: Why do we need to solve for a problem related to the wallet's stress suite which decides to do a reload? There isn't much you can do if the process exits at any specific point (power goes out, user closes tab, OS evicts process). It feels a bit like cargo culting. Lastly, does retrying immediately after having failed a write make sense? Or should there be a small delay, etc? Because we'd need to make these general assumptions I'd rather just return the store update and let the user handle it. A wallet or app implementation should already have the tools to do something that makes sense for their environment, even without returning the new error variant (although I think this one is an improvement).

I think a better approach could be to do 2 separate DB writes: one for a "pending insert" and then a "commit" one, but this probably requires a larger refactor

[WiktorStarczewski]

• Removed the in-process retry, the main objection ("retrying once does not help with any of these [failure modes]") — gone. apply_transaction is now called once; no retry loop.
• Attached the pending TransactionStoreUpdate to the error. This is the lighter-weight version of his "just return the store update and let the user handle it" idea — the variant now carries pending_update: Box and the docstring + ErrorHint tell callers to re-apply via apply_transaction_update, not resubmit.

[igamigo]

I think this is a better approach now. Still I would strive to make the hints and comment a bit more correct.

[igamigo]

I think we should avoid giving out too much internal details, but also they should be more correct: really, the nullifiers are not the only problem here. In fact, there may be no nullifiers involved in the transaction at all.

Rather, we should say that if the original transaction was accepted by the mempool and has not expired (and/or was finalized in a block), a duplicate submission would fail because the state of the account (and/or the network's state) was already mutated.

[igamigo]

Same here

[igamigo]

Same here

[WiktorStarczewski]

@igamigo addressed your last-round feedback in 1f1862f0a — replaced the 'nullifiers are already consumed' language in the three places it appeared (the Display impl on the variant, the ErrorHint message, and the CHANGELOG entry) with the more general framing you suggested:

if the original transaction was accepted by the mempool and has not expired (and/or was finalized in a block), a duplicate submission would fail because the state of the account (and/or the network's state) was already mutated.

Also tightened the rejection condition: a resubmit is only rejected if the original is still in the mempool or has been finalized — a tx that expired without making it into a block can be resubmitted, and the new text acknowledges that.