[QuantumExplorer]

Basic explanation

What this does: the wallet can now send, receive and accept DashPay contact requests that nobody else can see.

• A private request is sealed to the recipient's DashPay key and paid from the wallet's shielded (Orchard) notes, so nothing on chain names either identity.
• It is signed with the identity signer the wallet signs public requests with.
• The wallet reads DashPay's private store, keeps the requests addressed to its identities after checking each sender, and recovers the ones it sent after a restore from seed.
• Accepting a private request replies privately. A contact whose requests came or went privately is never sent a public request.

Value: private requests make DashPay contacts without publishing who knows whom. Once received, a private request establishes a contact and gets its payment accounts exactly as a public one does, so payments, rotation and restore keep working.

Risks: low to medium.

• Client code only, no consensus change.
• Sending spends shielded notes. It follows the existing shielded spend path: reserve, build, broadcast with redrive, then mark spent only once proven.
• The document send, accept and sync were refactored to share code with the private ones. The existing contact and DashPay tests pass unchanged.
• Deliberate behaviour changes:
  • the public send refuses a contact whose requests came or went privately, which also refuses a public accept that would broadcast a reply;
  • ingest, public and private, passes over a request older than the one tracked with the same contact. Public documents arrive in time order, so this only acts on the private store and on replays.

Issue being fixed or feature implemented

Part of anonymous DashPay contact requests for 5.1. It builds on the SDK calls in #5809 (send, scan and read private contact requests). Sam's decisions applied here:

• accepting a private request replies with a private request;
• the outgoing viewing key comes from the sender identity's DashPay encryption key;
• the request is signed by the sender's signing key, as a public request is.

What was done?

Sending: PlatformWallet::send_private_contact_request(coordinator, seed, account, sender, recipient, label, auto_accept_proof, shielded_address, signer, crypto, prover)

• It picks the keys, our receiving xpub for the pair, the masked account reference and the auto-accept proof exactly as a document send does. Those steps now live in DashPayView::prepare_contact_request, shared by both sends. A later request to the same recipient, either way, rotates past this one.
• signer signs the request with the first of the sender's keys it holds among private_contact_request_signing_keys: enabled AUTHENTICATION keys at a level DashPay's documents are signed at (HIGH or CRITICAL), bound to no other contract or document type.
• The crypto provider seals it. It keeps the encryption key, which the outgoing viewing key comes from.
• The pair's shared key is computed before paying, so nothing that can fail is left after the payment.
• The new shielded operation private_document_create_from_pool_with_metadata appends the entry with a type 29 transition paid from the account's notes:
  • new ShieldedFeeKind::PrivateDocumentCreate { entry_count, entry_size };
  • notes are reserved for the fee alone (amount 0);
  • it records as an own spend with its fee.
• Once the payment is proven, a short private sync reads the entry back and records it, and our receiving account for the contact is registered. An entry not read back yet leaves the request pending (see Changes from review).

send_private_contact_request(Alice -> Bob, label "Alice", shielded address A)
  chain: one 589-byte entry in DashPay's privateContactRequest store; the fee for 2 actions
         is 114,140,000 + (589 + 32) x 27,400 + 10,000,000 = 141,155,400 credits
  Alice's wallet: sent request to Bob (encryption key 1, recipient key 2, signed by key 0),
                  core height 0

Receiving: DashPayView::sync_private_contact_requests(crypto, max_pages)

• It reads the store from the earliest of the identities' cursors.
• The provider opens the page. The wallet fetches the senders, its own identities and the recipients of its sent requests once a pass, and checks each entry against them (private_contact_requests_from_scan). That needs no secret key.
• Received requests and sent ones (recovered with the outgoing viewing keys) are ingested with the same functions the document sync uses: newest per sender, rotation, auto-establish against a matching sent request.
• Account builds are queued as the document sync queues them.
• An identity's cursor advances only when its ingest reached disk. A pass that left one behind does not report itself complete.

How a private request is recorded: as a ContactRequest.

• The entry carries the compact xpub and label in the clear. The wallet encrypts them under the pair's DIP-15 shared key, as a contactRequest document carries them, with IVs taken from the entry's commitment. So:
  • the existing decryption drain registers the contact's account unchanged;
  • a rescan rebuilds the same bytes, so the wallet's comparisons of a contact's encrypted xpub hold;
  • nothing persisted changes shape: no new rows, FFI structs or migrations.
• The record's core height is 0. DashPay requires every contactRequest document to carry its Platform-assigned $createdAtCoreBlockHeight, and no Core chain puts that at 0. So ContactRequest::came_privately() reads the channel from the record itself, and it holds across restarts. Our receiving account for the contact is scanned from the wallet's birth.
• DashPayState::private_contact(contact) derives received_privately and sent_privately from the tracked requests. Only the contact's shielded address stays in memory, noted from the request tracked from that contact and rebuilt by the scan a cold start runs.

Bob syncs; the store holds Alice's entry
  before: no way to read it
  after:  Bob's incoming requests: Alice (account reference r, created at the block's time, core height 0);
          private_contact(Alice) = { received_privately, shielded_address: A }; cursor 1
Bob restarts
  private_contact(Alice) still says received_privately: it comes from the stored record

Accepting: PlatformWallet::accept_private_contact_request(coordinator, seed, account, request, shielded_address, signer, crypto, prover)

• It replies privately, then registers the contact's account as the document accept does.
• The accept body is now accept_contact_request_replying_with, shared by both accepts.

Bob accepts Alice's private request
  accept_contact_request_with_external_signer:
    before: would publish a contactRequest document naming Alice and Bob
    after:  Err(PrivateContactRequestNeedsPrivateReply(Alice))
  accept_private_contact_request: a private reply; Alice established; Bob's account paying her registered
Alice, who sent privately, tries a public request to Bob
  send_contact_request_with_external_signer: Err(PrivateContactRequestNeedsPrivateReply(Bob))

Older requests are no rotation. ingest_received_requests and ingest_sent_requests pass over a request older than the one tracked with the same contact. Older is compared by the account reference's version, which a rotation bumps, then by creation time. A cold start reads the private store from its start, and could otherwise replay a request that a public one already superseded. The same goes for an old request appended to the store again.

tracked from Carol: version 1, created at 100
  Carol's version 0 request, appended again at 200 -> passed over (before: taken as a rotation back)
  Carol's version 2 request, created at 300        -> rotation, as before

Auto-accept. A request that came privately carries no auto-accept proof into the wallet. Accepting it pays from the shielded pool, which the unattended auto-accept drain cannot do, so it is accepted by hand.

Crypto provider. ContactCryptoProvider gains two methods:

• scan_private_document_page
• seal_private_contact_request, which takes the signature from the identity signer

They take Rust-built key paths (PrivateDocumentKeyPaths), so the scalars never leave the signer. Both default to an "unsupported" error, so existing providers compile unchanged. The test SeedCryptoProvider implements them. The Keychain-backed provider comes with the FFI PR.

Unignoring a sender rewinds the private cursor, as it rewinds the document cursor, so the skipped requests are read again.

Refactor. The document sync's teardown of stale external accounts moved into tear_down_stale_external_accounts, which both syncs call. The cursor advance moved ahead of it. That is behaviour-neutral, as all of it runs under one lock.

Changes from review

• One accept for hosts. accept_contact_request_replying_in_kind (on DashPayView, and on PlatformWallet with shielded) replies the way a request came. It replies privately when the request came privately and no reply exists yet, and otherwise with a document or by adopting the reply already sent. Hosts no longer catch code 68 and choose. PlatformWallet's version takes what a private reply is paid with as a closure (PrivateReplyPayment), called only for a private reply, so a public reply needs neither shielded support nor the seed.
• Recorded only once read back. The type 29 proof shows the notes spent, not the entry appended. So after paying, a send reads the store back with a short private sync and returns the request recorded from it. If the entry isn't visible yet, it returns PrivateContactRequestPending.
• One private request in flight per recipient. It is reserved before sealing. A retried or concurrent send or accept gets PrivateContactRequestPending instead of paying again, while the payment is in flight or unconfirmed, and until a sync reads the entry back. A payment that definitely failed releases it.
  • Each reservation belongs to the attempt that made it: an older attempt's cleanup can't release a newer one's.
  • A send dropped before it pays releases its reservation.
  • A reservation settles only once its request is recorded, not when the entry is merely seen.
  • After a restart, reservations are rebuilt from the shielded store's saved unconfirmed spends. Each DashPay private entry is opened with the wallet's outgoing viewing keys. This happens in PlatformWallet::restore_pending_private_contact_requests and right before every private payment, so no second request to that pair is paid for.
  • Not restored: a pending rotation to a contact that already tracks an outgoing request, and a reservation whose spend failed for good. The latter stays until the next restart.
• Public sends wait too. A public request to a recipient with a pending private request is refused with PrivateContactRequestPending, since the private one may already be on chain.
• No paid duplicate at the rotation limit. A request whose account reference would repeat the tracked one, once all 16 rotation versions are used, is refused before anything is signed or paid.
• The sync report lists recorded requests. PrivateContactSyncReport::requests holds what the ingests recorded. Not included: a request already tracked, an ignored sender's, a replay, or one whose store failed.
• Sync.
  • Identities, recipients included, are proved at or after the entries they check.
  • A recipient that can't be read holds back its senders' cursors instead of skipping the request.
  • A cursor rewound mid-pass (unignoring a sender) stays rewound for that pass.
  • Collapsing a sweep orders by the reference's version first, as passing over older requests does.

Alice sends Bob a private request; the proof comes back but a competing spend took the notes
  before: recorded as sent, the contact established on the strength of the spend proof
  after:  the read-back finds no entry: Err(PrivateContactRequestPending(Bob)), nothing recorded;
          a second send to Bob: Err(PrivateContactRequestPending(Bob)) before anything is paid
The app restarts before Bob's entry lands, and Alice accepts again
  before: the reservation was gone, so a second entry was paid for
  after:  the unconfirmed spend is read back, Bob is reserved again, nothing is paid

How Has This Been Tested?

• cargo test -p platform-wallet --features shielded --lib -- private_contact private_document_create contact dashpay older_request: 233 passed.
• New tests:
  • should_send_read_and_accept_a_private_contact_request. Two wallets on mock SDKs:
    • Alice sends, signed with her signing key;
    • Alice's public request to Bob is then refused;
    • Bob reads it (sender checked, shielded address kept, cursor advanced);
    • both sides record the same ciphertext at core height 0, which Bob's shared key opens;
    • the public accept is refused;
    • the private accept establishes the contact and registers Bob's account paying Alice;
    • unignoring rewinds the cursor.
  • should_recover_a_sent_private_contact_request_from_the_store: a wallet restored from seed rebuilds its sent record byte for byte from the store alone.
  • should_record_a_private_request_the_same_way_every_time
  • an_older_request_than_the_tracked_one_is_no_rotation: replays and earlier requests of the same version are passed over on both sides, and a newer rotation is still taken.
  • test_select_for_a_private_document_create_covers_its_fee_alone: the fee kind equals the consensus fee, 141,155,400 credits for one DashPay entry.
• cargo clippy -p platform-wallet --features shielded --all-targets -- -D warnings: clean.
• cargo check -p platform-wallet-ffi, with and without --features shielded.
• Not run: a real Orchard proof. The payment step is a closure, which the tests replace. The operation follows the identity top-up path line for line.

Breaking Changes

None for consensus. Two host-visible behaviour changes, both PlatformWalletError::PrivateContactRequestNeedsPrivateReply: the public accept refuses a request that came privately, and the public send refuses a contact whose requests came or went privately. A host routes those to the private accept and send.

Checklist:

• I have performed a self-review of my own code
• My Rust, FFI, WASM, Swift or Kotlin changes follow the coding conventions
• 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
• If I added or changed a system contract's document schemas, I checked the contract 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

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration

• Source mode: Auto
• Repository: dashpay/platform
• Branch: feat/platform-wallet-private-contact-requests
• File: .coderabbit.yaml
• Commit: 8260a1c
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 97fa4849-bb75-412d-b22a-9f4bab70f9d7

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• Trigger review

• Autofix · 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

[thepastaclaw]

⛔ Final review complete — 2 blocking finding(s) (commit 8260a1c)

[thepastaclaw]

Final validation — Phase 1 + Phase 2

Static verification of head 7bd302a confirms five blocking issues involving publication confirmation, duplicate private-reply payments, rotation selection, cursor rewinds, and stale recipient identities. Additional suggestions address persistence recovery, failure isolation, entropy errors, and the public-send privacy precondition. Current CI shows Kotlin build/tests and policy checks passing, with the automated review pending and no targeted Rust validation check visible; no builds or tests were run.

🔴 5 blocking | 🟡 4 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: rust-quality); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 5: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 6: 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 large, intricate change directly modifies shielded funds movement and note selection in shielded/operations.rs and shielded/note_selection.rs, as well as signing-key selection and encryption-key handling in private_contact_requests.rs.
• 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 quota lookup failed), 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 — 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`:
- [BLOCKING] packages/rs-platform-wallet/src/wallet/shielded/operations.rs:1678-1686: Verify entry inclusion before recording a private request as sent
  This branch accepts an AffectedState result and returns success to send_private_contact_request, which records the outgoing request and can establish the contact. However, the SDK explicitly documents that type 29 proves only spent nullifiers, and Drive verifies only those nullifiers and classifies the result as non-execution-proving. With two real input notes, a competing spend by another device can consume the same nullifiers without appending this entry; an untrusted DAPI can then supply a genuine spent-nullifier snapshot for a request that was never published. Subsequent accepts adopt the nonexistent reciprocal instead of sending one. Keep consumed-note reconciliation separate from publication confirmation, and require a proof-verified private-store read locating the exact sealed entry before recording the request as delivered. If inclusion remains unknown, retain a typed unresolved outcome rather than returning publication success.

In `packages/rs-platform-wallet/src/wallet/identity/network/private_contact_requests.rs`:
- [BLOCKING] packages/rs-platform-wallet/src/wallet/identity/network/private_contact_requests.rs:315-320: Retain and serialize pending private replies before paying
  When pay returns ShieldedSpendUnconfirmed, this function exits without associating an outgoing request with the contact, while the shielded operation retains reserved notes and a byte-identical redrive that may still execute. An accept retry before private sync therefore sees no reciprocal, seals another entry, and can pay again from other available notes. Concurrent accepts have the same gap because accept_contact_request_replying_with releases its state lock before invoking the reply closure. The private store is an opaque append-only log with no cross-transition uniqueness constraint on the contact pair or account reference, so both payments can execute. The existing unstored_sent handling protects only failures after payment has returned success; later sync recovery does not prevent these duplicate fees. Reserve a contact-scoped pending reply before payment, serialize the decision for that pair, and associate the sealed entry with its shielded operation so retries resume or report the existing attempt instead of funding another one.
- [BLOCKING] packages/rs-platform-wallet/src/wallet/identity/network/private_contact_requests.rs:728-740: Apply version-first ordering before collapsing scan candidates
  Both collapse helpers select by (created_at, account_reference), but the new predates guard selects freshness by (account_reference_version, created_at). The guard cannot recover a rotation already discarded during collapse. For example, after an earlier page established version 0, a later page can contain a legitimate version 1 rotation followed by a replay of the version 0 ciphertext. Both survive SDK filtering because their account references differ, and the replay receives the later append timestamp. Collapse keeps version 0, ingest skips its already-tracked reference, and the cursor advances past version 1. This affects both received requests and recovered sent requests, leaving retired payment material or an obsolete outgoing rotation version tracked. Use one version-first ordering for collapse and supersession, and test an earlier-page original followed by a same-page rotation and replay in both directions.
- [BLOCKING] packages/rs-platform-wallet/src/wallet/identity/network/private_contact_requests.rs:747-752: Preserve a concurrent cursor rewind across the entire scan pass
  The compare-and-advance helper correctly refuses to overwrite a concurrent unignore rewind, but the assignment immediately afterward adopts the rewound cursor as this pass's new baseline. If a pass starts at position 100 and unignore_sender resets the live cursor to 0 while the first page is fetched, the first advance is refused and identity.position becomes 0. The next page is still fetched beyond 100 using read.next_position, yet its advancement now compares successfully against 0 and overwrites the rewind. The earlier ignored entries were never reread, so the newly unignored sender can remain missing for the session. Once a cursor mismatch is detected, stop advancing that identity for the rest of the pass or restart its scan at the rewind. Add a two-page regression with unignore occurring during the first fetch.
- [BLOCKING] packages/rs-platform-wallet/src/wallet/identity/network/private_contact_requests.rs:599-602: Height-pin recipient identities used for outgoing recovery
  Unlike proved_identities, this recipient cache discards proof metadata and never refreshes an identity during the pass. A later page can contain a request sealed to a key added after the cached recipient snapshot; even the initial fetch can return a proof older than sent.block_height within the SDK's tolerance. The lookup below then treats the stale snapshot as evidence that the recipient does not hold the key, skips the valid outgoing request, and advances the sender's cursor past it. A restored request or rotation is consequently lost for the running session. Fetch these recipients with the existing height-aware SDK mechanism, requiring proof height at least sent.block_height and refreshing when later pages require it. Failure to obtain a sufficiently recent snapshot must preserve the affected sender's cursor rather than become a permanent skip.
- [SUGGESTION] packages/rs-platform-wallet/src/wallet/identity/network/private_contact_requests.rs:599-602: Isolate recipient-fetch failures without advancing affected senders
  An error fetching any recovered outgoing recipient returns from the whole sync before the current page's unrelated incoming or outgoing requests can be ingested. Earlier pages remain committed, but one failing recovery lookup prevents progress for every identity on this page. The public sweep already supports per-identity degradation. Apply equivalent isolation here: report the failure, hold back cursors for identities whose outgoing records could not be reconstructed, and continue ingesting unaffected identities. Do not simply convert fetch errors to None and advance normally, because that would permanently skip recoverable outgoing requests for the session.
- [SUGGESTION] packages/rs-platform-wallet/src/wallet/identity/network/private_contact_requests.rs:286-287: Return entropy failures through the send result
  StdRng::from_os_rng panics when the operating system cannot supply entropy; rand_core explicitly provides try_from_os_rng for callers that must handle this failure. This introduces a panic into a fallible wallet-library send operation and can terminate an embedding application rather than returning PlatformWalletError. Seed through try_from_os_rng and map the error before filling esk_seed, retaining the existing zeroization after sealing.

In `packages/rs-platform-wallet/src/wallet/identity/network/contact_requests.rs`:
- [SUGGESTION] packages/rs-platform-wallet/src/wallet/identity/network/contact_requests.rs:1305-1311: Let same-reference recovery retry an unstored paid request
  A freshly paid request uses the confirmation proof's metadata.time_ms, whereas scan recovery uses the entry's block_time_ms. GetProofs proves current state, so confirmation can occur in a later block than the append. If initial persistence failed, add_paid_sent_contact_request keeps that later-timestamped request in memory and marks it unstored. This guard then rejects recovery of the identical account reference as older, preventing add_sent_contact_request from reaching its same-reference persistence retry. Sync can advance and report completion while the request remains unstored until a cold rescan. Exempt the same account reference from the freshness rejection so the existing retry persists the tracked record without replacing metadata. Cover a later confirmation timestamp and a failing-then-successful persister.
- [SUGGESTION] packages/rs-platform-wallet/src/wallet/identity/network/contact_requests.rs:583-594: Document or enforce the privacy gate's synchronization precondition
  This gate checks only locally tracked requests. A restored or second device that has not completed private sync has no private records for the pair, so it can publish a public contactRequest even though the relationship was previously private. There is no private-sync readiness check in this send path, and its API documentation does not state that the privacy guarantee depends on synchronization order. Document that limitation and the required completed scan before public sends on restored state, or enforce an explicit readiness policy in Rust. The local gate alone does not establish the PR's unconditional claim that contacts exchanged privately are never sent public requests.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

Verified the exact head d0db59f against the source: eight prior findings are fixed, but pending-operation lifecycle gaps still permit duplicate payments or stranded reservations. The public-send gate also overlooks unresolved private attempts, and exhausted rotation versions can consume fees without producing a usable request. This was a static review with no local builds or tests; current head-specific CI shows the Kotlin build/tests passing and no failed code checks, with the review check still pending.

🔴 3 blocking | 🟡 1 suggestion(s)

1 finding(s) not shown inline (the lines are not part of this PR's diff)

🔴 Blocking: Reject exhausted rotation versions before funding a private send
packages/rs-platform-wallet/src/wallet/identity/network/contact_requests.rs:926-935

This helper now prepares private sends as well as public documents, but saturation still relies on the public document's unique index to reject a repeated reference. With a tracked outgoing version of 15 and unchanged encryption key and receiving xpub, another private send derives the same account reference and pays for a fresh sealed entry. The append-only private store accepts it, while received ingest and add_sent_contact_request treat the reference as already tracked and ignore the new request. The sender's readback check then fails because the fresh commitment produces different encrypted-xpub bytes, so the operation spends its fee but returns pending without applying the request. Sync can clear the reservation, allowing further charged retries. Reject version exhaustion before sealing/payment, or implement a supported supersession scheme that does not depend on the public index.

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

1 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.

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: rust-quality); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 5: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 6: 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 large, intricate change directly modifies shielded funds movement and note selection in shielded/operations.rs and shielded/note_selection.rs, and signing and encryption-key handling in network/private_contact_requests.rs.
• 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 quota lookup failed), 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 — 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/identity/network/contact_requests.rs`:
- [BLOCKING] packages/rs-platform-wallet/src/wallet/identity/network/contact_requests.rs:591-601: Include unresolved private sends in the public-send privacy gate
  This gate checks only recorded incoming/outgoing requests through `private_contact`; it does not check `private_reply_pending`. On a device whose initial private sync is already complete, a first private send can return `ShieldedSpendUnconfirmed` or `PrivateContactRequestPending` while retaining its commitment but creating no outgoing record. A subsequent public send passes this gate and publishes both identities even though the private transition may already have executed or may still be published by redrive. This is separate from the documented restored-device synchronization precondition: scanning cannot recover an entry that has not executed yet, and this wallet already knows about the attempt. Make public-channel eligibility include unresolved private operations for the pair, retaining the refusal until definitive failure or reconciliation.
- [BLOCKING] packages/rs-platform-wallet/src/wallet/identity/network/contact_requests.rs:926-935: Reject exhausted rotation versions before funding a private send
  This helper now prepares private sends as well as public documents, but saturation still relies on the public document's unique index to reject a repeated reference. With a tracked outgoing version of 15 and unchanged encryption key and receiving xpub, another private send derives the same account reference and pays for a fresh sealed entry. The append-only private store accepts it, while received ingest and `add_sent_contact_request` treat the reference as already tracked and ignore the new request. The sender's readback check then fails because the fresh commitment produces different encrypted-xpub bytes, so the operation spends its fee but returns pending without applying the request. Sync can clear the reservation, allowing further charged retries. Reject version exhaustion before sealing/payment, or implement a supported supersession scheme that does not depend on the public index.

In `packages/rs-platform-wallet/src/wallet/identity/network/private_contact_requests.rs`:
- [SUGGESTION] packages/rs-platform-wallet/src/wallet/identity/network/private_contact_requests.rs:888-889: Report committed requests rather than pre-ingest candidates
  `PrivateContactSyncReport::requests` is documented as requests ingested, but these extensions run before collapsing candidates, filtering ignored senders, rejecting older requests, or attempting persistence. An ignored request or a request whose persistence failed is therefore returned as ingested even though it never became wallet state. The received helper already returns successful ingestions through `ingested`, but that vector is discarded here. Populate the report from successful ingestions and add equivalent sent-side tracking; cover ignored senders and a failing persister so callers can distinguish fetched candidates from committed state.
- [BLOCKING] packages/rs-platform-wallet/src/wallet/identity/network/private_contact_requests.rs:260-264: Retain and serialize pending private replies before paying
  (existing thread: https://github.com/dashpay/platform/pull/5810#discussion_r4506856812)
  The pre-seal reservation closes the straightforward overlapping-call case, but it is not owned and reconciled with the payment throughout its lifecycle. Four verified paths remain:

  - Restart loses `pending_private_replies`, while `FileBackedShieldedStore::rehydrate_pending_spends` restores the original signed transition and its note reservations. If the original request has not landed, even a completed private scan finds no reciprocal. Retrying accept can fund another entry from other notes while the original remains executable or is redriven. GroveDB's private store appends opaque entries without contact-pair/reference uniqueness, so both fees can execute.
  - Lines 912–920 settle every matching item in `read.sent`, including entries omitted from `records` because the recipient fetch failed at lines 799–801. Both ingest helpers can succeed on empty inputs, removing the reservation without storing an outgoing request. The send returns pending, but an accept retry can pay again.
  - After sync settles attempt A, it awaits account building, and A subsequently awaits account registration. Attempt B can reserve the same pair during that interval. A's recipient-only cleanup at lines 288–292 then removes B's reservation, admitting another payment while B is unresolved.
  - Dropping the send future during preparation bypasses cleanup. Cancellation before `note_private_reply_commitment` leaves a `None` reservation that no store scan can settle, blocking further sends and accepts for that pair until the wallet state is recreated.

  Use an attempt-owned lifecycle in the Rust wallet: retain the pair-to-attempt association durably before submission can occur, compare attempt identities when updating or releasing reservations, release abandoned pre-payment preparation safely, and settle only after the corresponding request has been durably reconciled or the payment is proven unable to execute. Add regressions for reopening with an unresolved redrive, recipient-fetch failure after payment, A-settles/B-reserves/A-cleans-up, and cancellation during preparation.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

Verified head 6b54d0d: the store-binding merge is correct, and eleven prior findings are fixed, but restart recovery still allows duplicate payments for an unresolved private rotation. This was a static review; no builds or tests were run. Current CI shows successful Kotlin build/tests and policy checks, with the automated review pending and no Rust test or Clippy result listed.

🔴 1 blocking

1 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.

Review provenance

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

• Triage: critical by gpt-6.1-sol (effort low) — The large, intricate diff directly changes shielded funds spending and note selection in shielded/operations.rs::private_document_create_from_pool_with_metadata and shielded/note_selection.rs, alongside private-request signing and encryption-key handling.
• 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 quota lookup failed), 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 — 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/identity/network/private_contact_requests.rs`:
- [BLOCKING] packages/rs-platform-wallet/src/wallet/identity/network/private_contact_requests.rs:1306-1308: Retain and serialize pending private replies before paying
  (existing thread: https://github.com/dashpay/platform/pull/5810#discussion_r4506856812)
  This guard treats any outgoing request as completion of the saved operation, so the explicitly excluded pending-rotation case still allows duplicate payment. Persist an outgoing v0 request, submit a v1 rotation that returns ShieldedSpendUnconfirmed, and restart before v1 reaches the private store. Both startup restoration and the payment-time hold call this helper and skip the saved v1 PendingRedrive because v0 exists. The initial store scan can recover only v0; retrying then derives v1 again, seals a different entry, and can pay from other available notes while the original signed transition remains executable. Note reservations protect the original inputs, not the contact pair, and neither type-29 validation nor the opaque append-only store enforces account-reference uniqueness across these transitions. Both payments can therefore execute and charge fees. Restore unresolved rotations as well, and discharge a saved operation only after reconciling its particular request, or a superseding request, against recorded state—not merely because an outgoing request exists. Add a restart regression with recorded v0, an unconfirmed v1, and other spendable notes, verifying that retry cannot fund another transition.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

Verified the findings against f98c1bd: six prior findings remain valid and sixteen are fixed. An additional readiness gap after unignoring a sender also defeats the public-send privacy prerequisite, leaving six blocking findings and one cleanup suggestion overall. This was static verification only; current CI shows Kotlin build/tests passing and the review check pending, but provides no wallet-specific Rust validation result.

🔴 6 blocking | 🟡 1 suggestion(s)

1 finding(s) not shown inline (the lines are not part of this PR's diff)

🟡 Suggestion: Evict private-contact payment associations when their redrives are dropped
packages/rs-platform-wallet/src/wallet/shielded/file_store.rs:789-800

Each private payment adds an association to private_contact_payments, but clear_redrive deletes only the SQLite row and SubwalletState record. The mark_spent, clear_pending, and wallet/subwallet purge paths also leave the association map untouched. Joining against live redrives correctly prevents stale restoration results, but does not reclaim the entries: a long-lived store retains historical sender/recipient metadata and allocations after completed payments and wallet removal. Evict associations whenever their owning redrive is removed, and extend cleanup coverage to inspect the backing map rather than only the filtered listing.

_{source: muse-spark-1.3-contributor (phase1-reviewer: rust-quality); gpt-6.1-sol (phase2-reviewer: general, architecture-layering)}

5 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.

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) — This large, intricate change directly modifies shielded funds spending and fee-based note selection in packages/rs-platform-wallet/src/wallet/shielded/operations.rs and note_selection.rs, alongside private contact request signing and key handling.
• 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 quota lookup failed), 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/identity/state/managed_identity/contact_requests.rs`:
- [BLOCKING] packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/contact_requests.rs:389-390: Invalidate private-store readiness when unignoring a sender
  Unignoring rewinds private_contact_requests_position but leaves private_contact_requests_read set. A completed private scan can have skipped this sender's request through ingest_received_requests' ignored-sender branch, leaving no tracked private contact. After unignore, read_private_store_before_sending returns immediately on the stale readiness flag, so a public send can publish the pair before the rewound entries are ingested. There is also a concurrent variant: ScannedIdentity correctly preserves the rewind, but the end-of-store branch at private_contact_requests.rs:1538-1540 marks the identity ready without checking identity.rewound. Clear readiness when unignoring and prevent a rewound pass from re-establishing it; cover both unignore-after-completion and unignore-during-scan before a public send.

In `packages/rs-platform-wallet/src/wallet/shielded/file_store.rs`:
- [SUGGESTION] packages/rs-platform-wallet/src/wallet/shielded/file_store.rs:789-800: Evict private-contact payment associations when their redrives are dropped
  Each private payment adds an association to private_contact_payments, but clear_redrive deletes only the SQLite row and SubwalletState record. The mark_spent, clear_pending, and wallet/subwallet purge paths also leave the association map untouched. Joining against live redrives correctly prevents stale restoration results, but does not reclaim the entries: a long-lived store retains historical sender/recipient metadata and allocations after completed payments and wallet removal. Evict associations whenever their owning redrive is removed, and extend cleanup coverage to inspect the backing map rather than only the filtered listing.

In `packages/rs-platform-wallet/src/wallet/identity/network/private_contact_requests.rs`:
- [BLOCKING] packages/rs-platform-wallet/src/wallet/identity/network/private_contact_requests.rs:205-207: Require network evidence before skipping the private-store scan
  (existing thread: https://github.com/dashpay/platform/pull/5810#discussion_r4508317364)
  This predicate treats every non-transient ProtocolVersionNotActive refusal as evidence that no private store exists. The SDK also returns that refusal when its own version is pinned below activation, or its configuration prevents proof verification; neither establishes the network's version. For example, an SDK pinned to 14 against a protocol-15 network skips private recovery through read_private_store_before_deciding while ordinary public-document operations remain available. A restored device can consequently publish a public contactRequest naming a relationship already exchanged privately. The reviewed SDK exposes network_proven_below specifically to distinguish verified network evidence from client limitations. Require affirmative evidence below the private-document activation version before bypassing the scan; otherwise preserve the refusal.
- [BLOCKING] packages/rs-platform-wallet/src/wallet/identity/network/private_contact_requests.rs:775-782: Release private reservations after terminal shielded failures
  (existing thread: https://github.com/dashpay/platform/pull/5810#discussion_r4508317355)
  take_rejected_private_entries uses mem::take to destructively drain the terminal-rejection notifications before this function awaits wallet_manager.write(). If a send or accept is cancelled while waiting for that lock, the local commitments are dropped without releasing pending_private_replies. Once the coordinator has removed the failed redrive, it has no record from which to emit the notification again, and private sync cannot find an entry that never executed. Subsequent public and private operations for the pair therefore remain blocked until restart. Acquire and validate the managed wallet before draining the notifications, then consume and apply them without another suspension point, or retain ownership until application is acknowledged.

In `packages/rs-platform-wallet/src/wallet/identity/network/contact_requests.rs`:
- [BLOCKING] packages/rs-platform-wallet/src/wallet/identity/network/contact_requests.rs:755-756: Supply production scanning support before requiring it for public requests
  (existing thread: https://github.com/dashpay/platform/pull/5810#discussion_r4508317358)
  The existing public send path now requires a private-store scan before submission. On a network serving private documents, an ordinary managed HD identity with DashPay keys reaches scan_private_document_pages even for an empty store. However, ResolverContactCryptoProvider in packages/rs-platform-wallet-ffi/src/dashpay.rs implements neither scanning method, so it inherits the new unsupported error and public sends, accepts that require a reply, and QR sends fail before submission. The SeedCryptoProvider implementation used by the tests does not cover this production adapter. Default trait methods preserve compilation, not runtime compatibility. Supply production signer-backed scanning with this prerequisite, keeping key operations in their owning signer layer; silently treating unsupported scanning as an empty store would defeat the privacy check.

In `packages/rs-platform-wallet/src/wallet/shielded/operations.rs`:
- [BLOCKING] packages/rs-platform-wallet/src/wallet/shielded/operations.rs:1605-1621: Guard reserved shielded notes against pre-broadcast cancellation
  (existing thread: https://github.com/dashpay/platform/pull/5810#discussion_r4508317357)
  reserve_notes_covering marks the selected nullifiers pending, but this operation holds the reservation only through an ordinary Vec and result-based cleanup. extract_spends_and_anchor then awaits a ShieldedAnchors network fetch before an anchor, activity link, or durable redrive is attached. Dropping or timing out the send at that await skips the final match and cancel_pending. The notes remain excluded from selection and spendable balance, and stale_pending_spends deliberately excludes these unanchored reservations, so normal sync cannot release them. The contact-pair Drop guard does not own the notes. Introduce cancellation-safe note ownership immediately upon reservation, releasing pre-handoff notes on drop and transferring responsibility only once recoverable operation state is installed.
- [BLOCKING] packages/rs-platform-wallet/src/wallet/shielded/operations.rs:1665-1683: Keep cancellation cleanup armed during lazy payment preparation
  (existing thread: https://github.com/dashpay/platform/pull/5810#discussion_r4508317351)
  PaymentStarted is marked before arm_redrive_before_broadcast acquires store.write().await. If another store user holds the lock and the host cancels this future while it waits, no transition has been saved or broadcast, but PrivateReplyReservation::drop sees the marked token and retains the contact reservation. No private entry can arrive to settle it. The later anchor-prune pass cannot release the contact pair either: it obtains the entry commitments from the redrive that was never installed. The lazy host resolver is protected now, but this later pre-submission await still strands both channels until restart. Persist the durable redrive successfully before marking the token, with no intervening await between successful installation and the mark.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

Static verification at a35fe84 confirms that 20 prior findings are fixed, but three blocking lifecycle gaps remain: cancellation during note rollback, public submission before pending-operation recovery, and restoration resurrecting a terminally rejected request. Three additional suggestions address typed FFI outcomes, cancellation-safe secret cleanup, and Rust API migration documentation. No builds or tests were run; current CI shows Kotlin build/tests passing, no failed code checks, and the review check pending, but provides no Rust validation results.

🔴 3 blocking | 🟡 3 suggestion(s)

2 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 8: 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) — This large, intricate diff directly changes shielded funds spending and fee-only note selection in wallet/shielded/operations.rs and note_selection.rs, and signature and encryption-key handling in private_contact_requests.rs and mnemonic_resolver_core_signer.rs.
• 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 quota lookup failed), 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 — ffi-engineer (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/platform_wallet.rs`:
- [BLOCKING] packages/rs-platform-wallet/src/wallet/platform_wallet.rs:1039-1047: Include unresolved private sends in the public-send privacy gate
  The in-process pending check is present, but automatic restart recovery runs only during shielded binding. PlatformWalletManager::load_from_persistor explicitly leaves shielded restoration for later, while public sends and the production platform_wallet_send_contact_request_with_signer export require neither binding nor recovery readiness. After restarting with a saved, unconfirmed private payment, a public send before binding can therefore scan an empty private store, find no in-memory reservation, and publish the pair even though the saved type-29 transition remains executable. The restart regression confirms that the pair is not pending before bind_shielded and checks refusal only afterward. Recover durable pending-channel metadata before public submissions become eligible, or enforce a Rust-owned recovery barrier in public send and accept paths. Extend the regression to attempt a public submission before binding and require it to fail closed.

In `packages/rs-platform-wallet/src/wallet/identity/network/contact_requests.rs`:
- [SUGGESTION] packages/rs-platform-wallet/src/wallet/identity/network/contact_requests.rs:812-820: Preserve contact-request outcomes across the C error boundary
  These new outcomes reach the existing production send and accept exports, but PlatformWalletFFIResult::from handles neither PrivateContactRequestPending nor PrivateContactRequestNeedsPrivateReply. Both fall through to ErrorUnknown (99), which the Swift result mapping exposes as .unknown(detail). Hosts consequently cannot distinguish a request awaiting reconciliation, a public-channel privacy refusal, and an internal failure without parsing human-readable text. This also affects public sends whose mandatory initial private-store scan exhausts its budget. Add stable FFI discriminants and corresponding host mappings, keeping channel selection and reconciliation decisions in Rust.

In `packages/rs-platform-wallet/src/wallet/identity/network/private_contact_requests.rs`:
- [SUGGESTION] packages/rs-platform-wallet/src/wallet/identity/network/private_contact_requests.rs:923-930: Make the sealing seed zeroize on future cancellation
  esk_seed is a plain array retained across the awaited seal_private_contact_request call. The explicit zeroize afterward covers normal completion and returned errors, but cancellation while the provider is suspended skips it and drops the future without wiping the seed. This is sensitive ephemeral encryption material supplied to the envelope sealer. Wrap it in Zeroizing before filling it so cancellation also runs cleanup.
- [BLOCKING] packages/rs-platform-wallet/src/wallet/identity/network/private_contact_requests.rs:1653-1660: Release private reservations after terminal shielded failures
  (existing thread: https://github.com/dashpay/platform/pull/5810#discussion_r4508317355)
  The rejection notification handles sequential rejection and retry, but a stale restoration can recreate the rejected reservation after that notification is consumed. restore_private_contact_payments snapshots redrives before acquiring the wallet lock, and the crypto-based restoration additionally awaits scanning after taking its snapshot. A crypto restore can pause there, let the coordinator definitively reject and remove commitment C, let another send reconcile and consume C's rejection notice, then resume and call restore_private_reply for C. No outgoing request exists for the rejected operation, so the recorded-request check does not stop reinsertion. With neither a redrive nor a retained rejection notice, later sends and accepts remain pending until restart. Make terminal outcomes dominate both restoration paths, using retained terminal metadata or coordinated validation and insertion; do not infer failure merely from a missing redrive. Cover a restore paused during scanning and resumed after rejection reconciliation.

In `packages/rs-platform-wallet/src/wallet/shielded/store.rs`:
- [SUGGESTION] packages/rs-platform-wallet/src/wallet/shielded/store.rs:262-264: Document the new ShieldedStore lifetime requirement in migration guidance
  Adding 'static to this public trait rejects custom backends that borrow storage or configuration from their embedding application, although those implementations satisfied the previous contract. The trait comment explains why detached cleanup needs the bound, but the PR's compatibility discussion and the wallet README's existing API migration section do not describe this additional source incompatibility. Document migration to owned or Arc-owned resources and account for the constraint in release compatibility decisions. This is a Rust API change, not a consensus change, and needs no PlatformVersion slot.

In `packages/rs-platform-wallet/src/wallet/shielded/operations.rs`:
- [BLOCKING] packages/rs-platform-wallet/src/wallet/shielded/operations.rs:1708-1710: Guard reserved shielded notes against pre-broadcast cancellation
  (existing thread: https://github.com/dashpay/platform/pull/5810#discussion_r4508317357)
  The new guard protects cancellation during the anchor fetch, but this unconditional handoff disarms it before definite-failure cleanup finishes. If extract_spends_and_anchor returns an error while another task holds the store write lock, pending_entry is still None and execution reaches cancel_pending, which awaits that lock. Cancelling the send there drops the rollback after the guard has been disarmed. Nothing was broadcast, yet the notes remain reserved with anchor: None; stale_pending_spends excludes them, so anchor-pruning cleanup cannot release them. The same unconditional handoff exists in the other shielded spend paths. Keep rollback ownership until definite-failure cleanup actually clears the notes, while retaining recoverable ownership for potentially executed spends. Add a regression that cancels during lock-contended error rollback and verifies that the notes become spendable.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

At head 2b5e86f, the SDK merge delta changes only documentation and tests, but cumulative verification confirms one remaining restart-privacy blocker: reopening persisted private-payment state without the shielded feature bypasses the recovery hold. Of the other prior findings, 25 are fixed and the remaining mobile error mappings are reasonably deferred to tracked PRs #5812 and #5813. Validation was static only; current CI reports Kotlin build/tests passing, the review check pending, and no Rust validation result.

🔴 1 blocking

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: ffi-engineer); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 5: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 8: 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) — This large, intricate change directly extends shielded funds spending and fee-based note selection in packages/rs-platform-wallet/src/wallet/shielded/operations.rs and note_selection.rs, alongside private-request signing and key handling.
• 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 quota lookup failed), 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 — ffi-engineer (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/manager/load.rs`:
- [BLOCKING] packages/rs-platform-wallet/src/manager/load.rs:571-575: Include unresolved private sends in the public-send privacy gate
  The persisted-viewing-key barrier fixes the pre-bind window only in shielded-enabled builds. Both crates omit `shielded` from their default features; without it, `ClientStartState.shielded`, the FFI shielded loaders, and this hold installation are compiled out, while `private_payments_unrestored` initializes to false and public sends remain available. A wallet that previously broadcast and saved an unconfirmed private payment can therefore reopen through a non-shielded build, scan a private store that does not yet contain that entry, and publish the identity pair publicly while the signed private transition remains executable. Persisting the viewing key during binding does not protect this case because the reopening build cannot expose that marker to the gate. Preserve a feature-independent recovery marker or equivalent persistence contract and refuse public sends and accepts when pending-channel recovery cannot be performed. Add coverage for reopening shielded-created pending state without shielded support.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

Verified c7502c4 against the source and review evidence. Most prior fixes are present, but four blocking privacy/recovery issues remain: optional or non-durable recovery marking, ambiguous broadcast failures releasing protection, malformed recovery associations becoming absence, and compressed-byte matching losing valid recipient keys. This was a static review only; current CI reports passing Kotlin build/tests and policy checks, with the review pending and no separate Rust wallet test result shown.

🔴 4 blocking

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: ffi-engineer); reviewer 4: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 8: 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 large, intricate diff directly changes shielded funds spending and fee-aware note selection in packages/rs-platform-wallet/src/wallet/shielded/operations.rs and note_selection.rs, alongside private-request signing and key handling.
• 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 quota lookup failed), 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 — ffi-engineer (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/identity/network/private_contact_requests.rs`:
- [BLOCKING] packages/rs-platform-wallet/src/wallet/identity/network/private_contact_requests.rs:858-870: Include unresolved private sends in the public-send privacy gate
  Private payment proceeds without feature-independent recovery protection when PRIVATE_CONTACT_REQUEST_SCAN_STATE is absent: this helper returns success without writing a marker, although shielded binding requires only SHIELDED_FVK_RESTART. Reopening that wallet without `shielded` installs no viewing-key barrier, and load_from_persistor additionally discards a restored identity hold through forget_private_contact_scan_state. An empty private-store scan can then permit a public request while the saved private transition remains executable. The example-app assertion does not close this on the reviewed head: neither host declaration includes bit 14, and FFIPersister::callback_capabilities cannot expose it through its capability intersection.

  Even an attesting backend is not protected if it buffers writes. PlatformWalletPersistence explicitly permits store to defer durability until flush, but this helper neither requires store_commits_inline nor flushes before payment. A process exit can therefore retain the shielded transition while losing the true marker. Require durable, feature-independent recovery marking before payment, preserve known unresolved holds independently of optional scan checkpoints, and refuse payment when that contract cannot be satisfied. Cover both a non-attesting backend reopening without shielded support and a buffered backend losing unflushed changes.
- [BLOCKING] packages/rs-platform-wallet/src/wallet/identity/network/private_contact_requests.rs:694-703: Retain private-channel protection after potentially delivered broadcast failures
  This releases the pair and updates its unresolved-payment marker for every payment error except Pending and ShieldedSpendUnconfirmed. However, broadcast_shielded_spend maps errors accepted by broadcast_definitely_failed into ShieldedBroadcastFailed, and that shared policy explicitly acknowledges that Unavailable can occur after delivery or hide an earlier delivered retry. Applying its fund-safe note-release tradeoff to channel privacy is unsafe: the private transition can still land after this reservation is released. Because pre-send sync has already marked the store read, a subsequent public send can publish the identity pair without another scan. Keep the privacy reservation and durable marker for outcomes that permit prior delivery, independently of whether notes are released under the existing spend policy. Preserve enough outcome information to distinguish non-delivery from ambiguity, and cover an admitted submission whose response is Unavailable.
- [BLOCKING] packages/rs-platform-wallet/src/wallet/identity/network/private_contact_requests.rs:1602-1611: Match recovered recipient keys as curve points rather than encoded bytes
  sent.recipient_public_key.serialize() always produces a 33-byte compressed encoding, but valid ECDSA_SECP256K1 identity keys can contain the 65-byte uncompressed encoding of the same point. DPP permits that representation, and the SDK sealer parses it successfully; the SDK also has an explicit uncompressed-key alias regression. This byte comparison nevertheless fails, so outgoing recovery skips the valid request and subsequently advances the cursor and readiness flag. A fresh send pays but cannot complete read-back, while a seed-restored sender can lose the private-channel evidence and later publish a public request to that recipient. Parse ECDSA identity keys and compare normalized curve points, reusing the SDK's key eligibility rules where applicable. Cover recovery with an uncompressed recipient key and verify that a subsequent public send remains refused.

In `packages/rs-platform-wallet/src/wallet/shielded/file_store.rs`:
- [BLOCKING] packages/rs-platform-wallet/src/wallet/shielded/file_store.rs:329-334: Fail closed when a saved private payment association is malformed
  Both a SQLite decoding error and a non-null association blob with an invalid length become None here. The valid signed transition and its redrive are still restored, but private_contact_redrives_for_wallet omits that payment because the association is missing. restore_private_contact_payments then succeeds and clears the wallet and identity recovery holds without reserving the affected pair. If the transition has not executed, a public send can scan an empty private store and publish the relationship while the saved private payment remains executable. The association is now safety-critical recovery state, not optional display metadata. Preserve SQL NULL as the explicit legacy case, but propagate malformed non-null data as a typed recovery error so restoration cannot release the holds. Add a reopen regression containing a valid saved transition and a malformed association.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

Static verification of head 8260a1c confirms two blocking recovery gaps: ambiguous-broadcast cleanup deletes durable private-payment evidence, and restoration treats damaged associated transition bytes as absence. Of the 32 prior findings, 30 are fixed, one remains valid, and one remains intentionally deferred to tracked host-mapping follow-ups. Current CI shows Kotlin build/tests and policy checks passing, with this review pending; no local builds or tests were run.

🔴 2 blocking

Review provenance

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

• Triage: critical by gpt-6.1-sol (effort low) — The large, intricate diff directly changes shielded funds movement and note reservation in packages/rs-platform-wallet/src/wallet/shielded/operations.rs and note_selection.rs, alongside private-request signing and key handling.
• 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 quota lookup failed), 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 — ffi-engineer (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`:
- [BLOCKING] packages/rs-platform-wallet/src/wallet/shielded/operations.rs:1762-1766: Retain private-channel protection after potentially delivered broadcast failures
  ShieldedBroadcastMaybeDelivered preserves the live contact reservation, but the shielded operation still routes it through this generic failure arm. reservation.cancel() calls clear_pending, which deletes the saved redrive row and evicts its PrivateContactPayment association in FileBackedShieldedStore. After an admitted submission returns Unavailable and the application restarts, the durable marker initially holds the identity, but restore_private_contact_payments finds no associated payment and clears both recovery holds. A private-store scan before the admitted transition executes can legitimately return empty, allowing a public request to disclose the identity pair or another private request to pay again. The new marker and fresh scan therefore do not establish the restart guarantee described in your reply: the cleanup has removed the evidence that the earlier transition remains executable. Preserve durable pair and commitment recovery evidence independently of the deliberate note-release policy, and discharge it only after publication is reconciled or non-execution is established. Extend the regression through the actual shielded rollback, file-store reopen, restoration, and a public-send attempt before entry inclusion; the current callback-injected test exercises only the live-session hold.

In `packages/rs-platform-wallet/src/wallet/platform_wallet.rs`:
- [BLOCKING] packages/rs-platform-wallet/src/wallet/platform_wallet.rs:1837-1841: Fail restoration closed when an associated private payment's transition is unreadable
  Every record in saved already has an explicit PrivateContactPayment association, but private_contact_request_redrive_entries silently skips transitions that cannot deserialize or do not match the expected contract/type, and this filter_map also drops entries shorter than their commitment. FileBackedShieldedStore::rehydrate_pending_spends accepts a row with valid metadata and a readable association even when its st_bytes blob cannot deserialize: the otherwise-valid metadata path does not require a successful transition decode. Damaging only those bytes after an unconfirmed broadcast therefore lets the store reopen and list the associated record, produces no payments here, and reaches note_private_payments_restored/private_payments_restored, releasing the known unresolved holds. The original network copy can still execute, so an empty private-store scan can subsequently permit public disclosure. Make explicitly associated payment recovery all-or-error, validating the transition family, target and entries before releasing any hold; preserve associated recovery evidence when redrive decoding fails as well. Add a reopen regression with an intact association and corrupted signed-transition bytes, asserting that restoration fails and public submissions remain refused.

[thepastaclaw]

🔴 Blocking: Retain private-channel protection after potentially delivered broadcast failures

ShieldedBroadcastMaybeDelivered preserves the live contact reservation, but the shielded operation still routes it through this generic failure arm. reservation.cancel() calls clear_pending, which deletes the saved redrive row and evicts its PrivateContactPayment association in FileBackedShieldedStore. After an admitted submission returns Unavailable and the application restarts, the durable marker initially holds the identity, but restore_private_contact_payments finds no associated payment and clears both recovery holds. A private-store scan before the admitted transition executes can legitimately return empty, allowing a public request to disclose the identity pair or another private request to pay again. The new marker and fresh scan therefore do not establish the restart guarantee described in your reply: the cleanup has removed the evidence that the earlier transition remains executable. Preserve durable pair and commitment recovery evidence independently of the deliberate note-release policy, and discharge it only after publication is reconciled or non-execution is established. Extend the regression through the actual shielded rollback, file-store reopen, restoration, and a public-send attempt before entry inclusion; the current callback-injected test exercises only the live-session hold.

_{source: gpt-6.1-sol (phase2-reviewer: general, architecture-layering, platform-versioning, security-auditor)}

[thepastaclaw]

🔴 Blocking: Fail restoration closed when an associated private payment's transition is unreadable

Every record in saved already has an explicit PrivateContactPayment association, but private_contact_request_redrive_entries silently skips transitions that cannot deserialize or do not match the expected contract/type, and this filter_map also drops entries shorter than their commitment. FileBackedShieldedStore::rehydrate_pending_spends accepts a row with valid metadata and a readable association even when its st_bytes blob cannot deserialize: the otherwise-valid metadata path does not require a successful transition decode. Damaging only those bytes after an unconfirmed broadcast therefore lets the store reopen and list the associated record, produces no payments here, and reaches note_private_payments_restored/private_payments_restored, releasing the known unresolved holds. The original network copy can still execute, so an empty private-store scan can subsequently permit public disclosure. Make explicitly associated payment recovery all-or-error, validating the transition family, target and entries before releasing any hold; preserve associated recovery evidence when redrive decoding fails as well. Add a reopen regression with an intact association and corrupted signed-transition bytes, asserting that restoration fails and public submissions remain refused.

_{source: gpt-6.1-sol (phase2-reviewer: general, architecture-layering, ffi-engineer, platform-versioning, rust-quality, security-auditor)}