perf(platform-wallet): prove shield bundles while fetching nonces

[PastaPastaPasta]

Stacked on #5278 (perf/shielded-prover-prepare). Review that first; GitHub retargets this PR to v5.0-dev when it merges.

Issue being fixed or feature implemented

Shield sends (Platform address → Shielded, and Identity → Shielded) fetched input nonces over the network before starting the Orchard proof. That added a full round trip in series with a multi-second proof on a phone.

The Orchard bundle doesn't depend on those nonces:

• Shield bundles bind only a version tag plus the SHA-256 of the input address keys, in key order (shield_funding_digest_v0, rs-dpp/src/shielded/sighash.rs:791-819). Consensus rebuilds exactly that from the transition (shielded_proof.rs:1239). Nonces and per-input amounts never enter it.
• ShieldFromIdentity binds only the tag and the identity id. Consensus rebuilds it the same way: CheckTx (shielded_proof.rs:1255) and the block path (transform_into_action/v0:104).
• The proof's public inputs (anchor, cv_net, nf, rk, cmx, flags) contain no transparent fields. The value balance is the shield amount, which is known up front.
• Nonces are still signed. The input address witnesses or the identity signature sign the fully assembled transition, after both steps finish.
• drive-abci's own shield tests already prove with (0,0) placeholder nonces and then validate with real ones (shield/tests.rs:136-147).

What was done?

• DPP (shielded-client feature only; no validation or serialization change):
  • prove_shield_bundle / build_shield_transition_from_proved_bundle, and prove_shield_from_identity_bundle / build_shield_from_identity_transition_from_proved_bundle.
  • A proved bundle records the binding it was built with. Assembly recomputes the binding and refuses a mismatch (is_bound_for).
  • ProvedShield*Bundle fields are private, so callers can't skip the check.
  • The existing build_shield_transition / build_shield_from_identity_transition delegate to the new pair, with the same calls in the same order, so their output is unchanged.
• Wallet:
  • Shield sends prove while the nonces are fetched (fetch_while_proving, proof on the blocking pool from perf(platform-wallet)!: build the Orchard proving key once off the async runtime and prove off tokio workers #5278).
  • A fetch error, such as insufficient balance, still wins and returns immediately; the in-flight proof is discarded.
  • The nonce fetch is a proved query and can move the SDK's protocol version (e.g. across v14's credit_pool_bundle_binding). Assembly therefore uses the post-fetch version and re-proves if the binding changed. The DPP assembly check refuses any mismatch.
  • Side effects keep their order: get_identity_nonce(bump = true) still runs to completion, and the unresolved-debit check still runs first.

Saves about one network round trip (identity-nonce or address-nonce fetch with proof verification) per shield send.

How Has This Been Tested?

• cargo test -p dpp --features shielded-client,state-transition-signing --lib -- shielded::builder::shield: 21 pass, including:
  • the binding ignores nonces and amounts at the first and latest protocol versions;
  • the split path end to end;
  • wrong inputs, wrong identity and wrong version are rejected at assembly.
• Wallet tests: fetch_while_proving ordering and error precedence on a paused clock, plus a deterministic overlap test that uses channels and a real blocking proof.
• Full platform-wallet + platform-wallet-ffi suites with shielded pass (1401 + 426 lib tests). Clippy -D warnings (wallet, FFI, dpp with shielded-client) and cargo fmt --check are clean.
• cargo check -p dpp / -p platform-wallet with default (server-like) features is clean; the split code isn't compiled there.
• Independent code review confirmed the binding/commitment analysis against the consensus side and found no blocking issues.
• The re-prove decision (keep a still-bound proof, re-prove a stale one exactly once, return errors before signing) is unit-tested through reprove_if_unbound. Not tested end to end: a version change during a live fetch, because the mock SDK can't drive one. The DPP assembly check that backs it is tested.

Breaking Changes

None. Additive DPP client API; the existing builders keep their signatures and output.

Checklist:

• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have added or updated relevant unit/integration/functional/e2e tests
• I have added "!" to the title and described breaking changes in the corresponding section if my code contains any
• I have made corresponding changes to the documentation if needed
• If I added or changed GroveDB structure, I described it in the area's structure.rs, regenerated grovedb-structure.json, and checked the structure viewer link posted on this pull request

For repository code-owners and collaborators only

• I have assigned this pull request to a milestone

🤖 Generated with Claude Code

[coderabbitai]

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

• Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[thepastaclaw]

🕓 Review not started yet because this PR is a draft.

• Request normal review — click when the PR is ready for review.
• Request priority review — click to move this review to the front of the queue.

Commit b57379b. Normal review starts when eligible; priority review starts as soon as a slot is available.

[thepastaclaw]

Final validation — Phase 1 + Phase 2

The changes preserve the existing funding-binding rules and signing order while overlapping proof generation with nonce fetching; no blocking correctness, security, consensus, or layering issues were found. One non-blocking test gap remains in the newly added post-fetch binding-change retry. Verification was static: the supplied CI snapshot shows successful Kotlin SDK checks but no dedicated DPP or platform-wallet Rust-suite results.

🟡 1 suggestion(s)

Review provenance

Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 5: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 7: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: critical by gpt-6.1-sol (effort low) — The intricate split in shield.rs and shield_from_identity.rs changes cryptographic proof construction and binding checks, while operations.rs changes funds-moving shield sends to prove concurrently with nonce fetching and re-prove when protocol bindings change.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 15% left, 5h 100% left), glm-5.3-flash (not used above high effort; tier asks max)
• Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-platform-wallet/src/wallet/shielded/operations.rs`:
- [SUGGESTION] packages/rs-platform-wallet/src/wallet/shielded/operations.rs:683-688: Exercise the post-fetch binding-change retry
  Neither the new DPP binding tests nor the fetch/prove overlap tests execute this retry or its ShieldFromIdentity counterpart at lines 1062–1066. Those tests would still pass if the retry were removed, even though a nonce fetch that advances the SDK across the binding activation would then cause assembly to fail. Add deterministic coverage for unchanged binding, changed binding with a replacement proof using the post-fetch version, and replacement-proof failure before signing or broadcast. Assert that the retry reuses the fetched nonce and does not fetch it again: get_identity_nonce(..., true, ...) advances the nonce cache. If the SDK mock cannot drive the version change, expose a small test seam for the version getter and proving closure rather than requiring a network-based test.

[thepastaclaw]

🟡 Suggestion: Exercise the post-fetch binding-change retry

Neither the new DPP binding tests nor the fetch/prove overlap tests execute this retry or its ShieldFromIdentity counterpart at lines 1062–1066. Those tests would still pass if the retry were removed, even though a nonce fetch that advances the SDK across the binding activation would then cause assembly to fail. Add deterministic coverage for unchanged binding, changed binding with a replacement proof using the post-fetch version, and replacement-proof failure before signing or broadcast. Assert that the retry reuses the fetched nonce and does not fetch it again: get_identity_nonce(..., true, ...) advances the nonce cache. If the SDK mock cannot drive the version change, expose a small test seam for the version getter and proving closure rather than requiring a network-based test.

_{source: gpt-6.1-sol (phase2-reviewer: rust-quality)}

[PastaPastaPasta]

Partly addressed in a59c644. The re-prove step on both paths (here and the ShieldFromIdentity one) now goes through reprove_if_unbound in prover.rs, which has four unit tests:

• a still-bound proof is kept and not re-proved;
• a stale proof is re-proved exactly once;
• a re-prove error is returned;
• a binding-check error is returned without re-proving.

All of these errors happen before signing or broadcast. The helper takes no fetch, so the retry can only reuse the nonce that was already fetched. It cannot call get_identity_nonce(.., true, ..) a second time.

What is still not covered is an end-to-end run where the SDK's version changes partway through a fetch. The mock SDK can't drive that, and adding an SDK-version seam just for this test isn't worth the extra code in this PR. The safety property doesn't rely on the wallet anyway: DPP assembly recomputes the binding and refuses a mismatch, and DPP tests cover that. So the worst case if this step regressed is a failed send at the version boundary, never a wrongly bound transition.

---

🤖 Posted autonomously by Claude on behalf of pasta.