feat: encrypted account send via ZK proofs

[n13]

Summary

• Add encrypted (wormhole) send flow with ZK proof generation, coin selection, and multi-batch submission
• New EncryptedSendStrategy integrating with the shared send UI via SendStrategy abstraction
• Wormhole coin selection algorithm with batch splitting for large UTXO sets
• Progress screen with step-by-step ZK proof generation feedback
• Account discovery service for gap-limit scanning of encrypted indices
• Refactor wormhole_claim_service → wormhole_send_service to support both claims and sends

Test plan

• Send from encrypted account with sufficient balance completes successfully
• Insufficient encrypted funds shows appropriate blocker
• Batch below minimum exit threshold shows blocker
• Progress steps display correctly during proof generation
• Coin selection unit tests pass (wormhole_coin_selection_test.dart)
• Cancellation mid-proof returns to previous screen cleanly

[n13]

Verdict: Request changes

I found two blocking correctness issues in the encrypted-send lifecycle:

1. Preserve pending-spend state across refreshes (mobile-app/lib/shared/utils/polling_refresh_scope.dart:75, quantus_sdk/lib/src/services/encrypted_account_service.dart:92-100). After pool acceptance, onBatchSubmitted persists nullifiers/change specifically to bridge the period before the indexer reports them. But pull-to-refresh and app-resume call refreshActiveAccountBalance, which calls discardCachedState() and deletes that same state file. If the indexer is still behind, the next load() exposes the just-spent inputs as spendable again and can also roll back/reuse nextIndex. Please clear only the chain-derived transfer/nullifier caches here and retain pending spends plus the monotonic address index until normal reconciliation/expiry.

2. Keep cancellation scoped to the operation that was cancelled (quantus_sdk/lib/src/services/wormhole_send_service.dart:163-168). Future.any returns ClaimCancelled while the proof flow keeps running, and its later _checkCancelled() calls read the mutable service-level _cancelCompleter. If the user closes the cancelled screen and starts another send before the orphaned FFI proof finishes, the second call replaces _cancelCompleter; the first flow now sees not-cancelled and can aggregate/submit after the UI already reported cancellation. Please capture/pass an operation-local cancellation token, or serialize operations, so an abandoned flow remains cancelled permanently.

Validation at head c650e7b88c1c967bebf61833277bbc72153265d2:

• SDK flutter analyze --no-pub: clean
• Mobile flutter analyze: two non-blocking prefer_const_constructors infos
• Focused wormhole coin-selection tests: 6/6 passed
• Rust library tests: 13/13 passed
• Full SDK suite: 97 tests passed; 3 setup failures because the local Flutter runner could not load rust_lib_quantus_wallet.framework
• Focused mobile tests could not build because the isolated worktree lacks the ignored .env asset

[n13]

Code Review — feat: encrypted account send via ZK proofs

Verdict: Request Changes

This is a strong PR with clean architecture — the SendStrategy abstraction, shared WormholeProgressSteps widget, and coin-selection-with-batch-splitting are well designed. I found two blocking correctness issues (concurring with the self-review) plus a few additional observations.

---

Blocking Issues

1. Cancellation token is service-level mutable state — race on overlapping operations

WormholeSendService._cancelCompleter is a single mutable field shared across all operations on the same instance. The pattern is:

Future<ClaimResult> _withCancellation(Future<ClaimResult> Function() flow) async {
  final cancelCompleter = Completer<void>();
  _cancelCompleter = cancelCompleter; // ← service-level
  ...
  return await Future.any([flow(), cancelGuard]);
}

Future.any returns on ClaimCancelled immediately, but the orphaned FFI proof flow keeps running. If the user re-enters the send screen and starts a new send before that flow finishes, _withCancellation replaces _cancelCompleter. The old flow's _checkCancelled() now checks the new completer (which is not cancelled) and can aggregate/submit after the UI already reported cancellation.

Fix: Pass an operation-scoped cancellation token (e.g. a per-call Completer or monotonic sequence number) through the flow, or serialize operations with a lock that prevents a new send until the previous flow is confirmed dead.

2. discardCachedState() deletes pending-spend records — pull-to-refresh can expose spent inputs

refreshActiveAccountBalance → service.discardCachedState() deletes the entire encrypted_account_w$N.json including pendingSpends and the monotonic nextIndex:

Future<void> discardCachedState() async {
  await WormholeUtxoService.clearAllCaches();
  final file = await _stateFile();
  if (await file.exists()) await file.delete();
}

If the indexer hasn't caught up:

• Spent inputs reappear in the spendable set (their nullifiers are no longer locally excluded).
• nextIndex resets, so a fresh send could derive the same change address as one already in the pool.
• Pending change balance disappears (cosmetic, but confusing).

Fix: Preserve pendingSpends and nextIndex through a refresh. Only clear the wormhole transfer/nullifier GraphQL caches (the "derived-from-chain" part). Reconcile pending spends normally during load() — they expire via _pendingSpendExpiry or when the indexer confirms them.

---

Additional Observations

3. _readState() is not protected by _stateLock

receiveKeyPair() and send() call _readState() outside the _stateLock chain. A concurrent load() (which calls _mutateState) could be mid-write, causing a torn read. Consider routing all reads through the lock, or using _mutateState with an identity transform when you only need to read.

4. Indonesian localization strings left in English

Several app_localizations_id.dart encrypted-send step strings are still English:

String get encryptedSendStepPreparing => 'Preparing';
String get encryptedSendStepGathering => 'Gathering funds';
String get encryptedSendStepSecuring => 'Securing transaction';
...

These should be translated or marked as TODOs for the translation pass.

5. enableEncryptedAccount default flipped to true

remote_config_model.dart changes the fallback from false to true. This enables encrypted accounts for all clients even without a remote config update — intentional for dev, but double-check this is desired for production builds hitting this branch.

6. Coin-selection boundary tests

The test suite is good (6 tests, covers key scenarios). Consider adding a test for exactly 7 inputs (the boundary of a single batch) and for the "send max" case where every input's full net is consumed with zero change.

---

Positives

• SendStrategy / EncryptedSendStrategy plugs cleanly into the existing flow without duplicating UI
• WormholeProgressSteps extracted as a shared widget → DRY with the redeem progress screen
• buildSentTerminalContent lifted from RegularSendStrategy → reusable by both strategies
• AccountDiscoveryService.discoverUsedIndices generalized for encrypted address scanning — nice factoring
• WormholeUtxoService.getTransfersToMany batches queries by cache height → fewer round trips for multi-address discovery
• Proper logout cleanup: both Riverpod invalidation and on-disk file deletion
• Unit tests for coin selection cover the core invariants (fee arithmetic, batch splitting, error cases)

Once the two blocking issues are addressed, this is ready to ship.

[n13]

Verdict: Request changes

The pending-spend refresh issue from the previous review is fixed: refresh now preserves the persisted pending spends and monotonic nextIndex while clearing address-specific chain caches.

One blocking correctness issue remains:

1. Cancellation still is not scoped to the cancelled operation (quantus_sdk/lib/src/services/wormhole_send_service.dart:106-113, quantus_sdk/lib/src/services/wormhole_send_service.dart:163-182, quantus_sdk/lib/src/services/wormhole_send_service.dart:193-195). cancel() now awaits _operationDone, but _operationDone is completed by _withCancellation's finally. Because _withCancellation awaits Future.any, that finally runs immediately when the cancellation guard wins; it does not wait for the orphaned flow() / FFI proof work to finish. The UI can therefore return from cancel(), allow a new send, and replace _cancelCompleter. The old flow's later _checkCancelled() and UTXO isCancelled: () => _cancelled calls then read the new operation's incomplete shared completer and can continue to aggregate or submit. _opId currently only guards cleanup and is never consulted by _checkCancelled, so it does not prevent this. Please give each flow an immutable operation-local token and pass/check that token throughout the flow (including the UTXO callback), or truly serialize operations by retaining/awaiting the underlying flow future before allowing another operation. Add a regression test that cancels during an unresolved proof, starts a second operation, completes the first proof, and verifies the first operation never reaches aggregation/submission.

Validation at head 46659e348be1767ee287876f7e30fbac6dbb3027:

• SDK flutter analyze --no-pub: clean
• Focused wormhole coin-selection tests: 6/6 passed
• GitHub reports no checks for this branch

[n13]

Review by Grok 4.5

Summary

This PR wires encrypted-account (wormhole) sends end-to-end: coin selection with volume fee / change, dual-exit ZK proofs via FRB, batch submit, Riverpod balance state, and a shared proving progress UI. The core spend math and FFI surface look coherent and are covered by solid coin-selection unit tests. Dominant risks are lifecycle races (logout / dispose while proving), UX promises that are not implemented (notifications, full private activity), and stale plans / nextIndex allocation that are not revalidated at submit time.

Verdict

REQUEST_CHANGES (posted as COMMENT because GitHub disallows requesting changes on your own PR)

Detail

REQUEST_CHANGES
Block on the logout/dispose race that can rewrite encrypted-account disk state after session wipe, and on the progress footer promising notifications that are never scheduled. Other items can follow as follow-ups but should not ship silently.

Issue counts by severity

• bugs: 4
• suggestions: 4
• nits: 2

[n13]

[bug] _startSend captures EncryptedAccountService and runs proving/submit with no dispose cancellation. Logout clears wormhole caches + encrypted_account_w*.json (SubstrateService.logout / EncryptedAccountService.clearAllPersistedState) and only invalidates Riverpod providers (logout_service.dart), without calling cancel(). The in-flight send keeps the old service alive; onBatchSubmitted can recreate encrypted_account_w{N}.json after wipe, leaking pending nullifiers/change into the next session at the same wallet index. Secrets/mnemonic-derived material already held in memory also continue to be used post-logout.

Suggestion: On logout (and preferably in EncryptedSendProgressScreen.dispose), await encryptedAccountService.cancel() for every live wallet (or a process-wide send cancel registry) before clearing disk; make onBatchSubmitted / _mutateState no-op if a session generation token has changed; optionally refuse to persist after logout.

[n13]

users are supposed to either wait for the send to go through or cancel the send

Action items:

• Make sure users are kept on the send page until complete or canceled
• if canceled, make sure we await the successful cancellation

[n13]

[bug] Footer copy (encryptedSendProgressFooter) tells the user they can leave the app and will be notified when the private send completes. This flow never creates a PendingTransactionEvent, never starts pending-tx polling, and never posts a local/remote notification on success/failure. Proving also runs only while the isolate lives—OS kill mid-prove drops unsubmitted work with no recovery UX beyond a later balance refresh. PopScope(canPop: !_running) further prevents leaving the route while running (only backgrounding the app).

Suggestion: Either implement pending-state + notification on pool acceptance / finality (mirroring regular sends), or change the copy to match reality (must keep app open; no notification). Prefer recording submitted batch hashes and notifying when remaining work finishes or fails.

[n13]

change the footer text, they cannot leave the app

[n13]

[bug] Private activity labels (isPrivate: isEncryptedAccount(...)) are applied to whatever GraphQL history is already loaded for active.account.accountId. Encrypted account identity is wormhole index 0 only (AccountsService.createEncryptedAccount), while receive/change rotate across HD indices. Deposits to the receive address and most private spends will not appear under index 0’s transfer history, so “Private Sent/Received” UI is largely cosmetic and activity remains empty or wrong for real encrypted usage.

Suggestion: Build encrypted activity from wormhole transfers / pending spends across discovered HD addresses (or a dedicated indexer view), not transparent accountId history alone. Until then, hide the activity list or show an explicit empty state explaining private activity is not listed.

[n13]

We can show total deposits and total spends, or else just show a private activity label.

Solution: When we have established total balance, in that process we should have all data to show total received and total sent, so lets only show two items, total received, and total sent

Total received is across all wormhole addresses, total spent is the total of all nullifers, we already gather this information when calculating balance, so should be easy to put there, no additional calls needed.

[n13]

[suggestion] discardCachedState only clears caches for addresses already present in _keyPairs. After provider recreation (logout/login, process restart, first encryptedAccountServiceProvider read), _keyPairs is empty, so pull-to-refresh / refreshActiveAccountBalance becomes a no-op discard and relies entirely on existing on-disk transfer/nullifier caches. That is usually OK, but it cannot force a full cache drop when the in-memory key cache is cold.

Suggestion: Persist known address indices (or scan nextIndex + pending change addresses from _FileState) and clear caches for those even when _keyPairs is empty; or clear all wormhole caches for the wallet on force reload.

[n13]

force reload should clear all caches, that's the point

[n13]

make sure force reload clears all caches

[n13]

[suggestion] Self-send guard uses account.accountId (wormhole index 0 only). Users can still target their current receive address (nextIndex) or other HD wormhole addresses. That is wasteful (volume fee) and reduces privacy by linking addresses; depending on chain exit semantics it may also be surprising.

Suggestion: Treat all derived wormhole addresses for the wallet (at least 0..nextIndex plus pending change addresses) as self-send destinations, or clearly allow “move to own address” as an explicit consolidation action.

[n13]

This is true. Treat all derived wormhole addresses for the wallet as self address.

[n13]

[suggestion] Test coverage stops at pure coin selection. There are no tests for pending-spend reconciliation, onBatchSubmitted partial-batch persistence, dual-exit WormholeLeafSpend construction, logout/clear races, or encrypted fee blockers in the UI strategy.

Suggestion: Add unit tests for EncryptedAccountService state machine (pending prune/expiry, change index bump) and for WormholeSendService.sendSpends batch callback ordering; widget/strategy tests for quantization / insufficient / below-minimum blockers.

[n13]

ok will add missing tests

[n13]

[nit] Progress UI always lists steps 2–3 (“Gathering funds”, “Securing transaction”), but sendSpends only runs step 1 then jumps to 4–6. Those steps auto-complete when step 4 starts (WormholeProgressSteps via _maxStartedStep), which is slightly misleading during encrypted send (fine for redeem/claim).

Suggestion: Drive the step list from the operation type (claim vs send) or omit discovery steps for sendSpends.

[n13]

make sure our progress UX actually shows what we're doing!

[n13]

Review at head 2827618 — updated

Previous blocking items (pending-spend refresh, footer text, progress steps) are fixed. One remaining correctness issue found and patched in this review.

Fix applied: cancellation state transition race

_startSend() was catching ClaimCancelled and setting _running = false immediately when Future.any picked up the cancel guard — but the underlying FFI flow was still running at that point. Since PopScope(canPop: !_running), this allowed the user to navigate away before the operation had truly stopped.

Fix: _startSend() no longer transitions UI state on ClaimCancelled. Instead, _cancel() owns the transition — it awaits cancel() (which blocks until _operationDone resolves, i.e. the FFI work actually finishes), then sets _running = false / _cancelled = true. The screen stays in the "Canceling…" state with canPop: false until the operation has truly stopped.

This eliminates the theoretical race where a second operation could start while the first was still winding down.

Non-blocking observations (no changes made)

1. Self-send guard only checks index 0 — EncryptedSendStrategy.sourceAccountId returns account.accountId (wormhole index 0 only). The user can enter their current receive address (nextIndex) without triggering the self-send warning. Per PR discussion, all derived wormhole addresses for the wallet should be treated as self.

2. _readState() outside _stateLock — send() reads nextIndex outside the lock to allocate the change address. A concurrent onBatchSubmitted call (from the same send) could bump nextIndex between the read and usage. Low practical risk in single-isolate Dart but would be race-free if routed through _mutateState.

3. Indonesian localization strings — Several encryptedSendStep* entries in app_localizations_id.dart are still English. Not blocking.

Validation

• SDK flutter analyze --no-pub: clean
• Focused wormhole coin-selection tests: 6/6 passed
• Encrypted account service tests: 3/3 passed