Skip to content

feat(swift-sdk): send, receive and accept DashPay private contact requests - #5812

Open
QuantumExplorer wants to merge 52 commits into
feat/platform-wallet-private-contact-requestsfrom
feat/ffi-swift-private-contact-requests
Open

QuantumExplorer wants to merge 52 commits into
feat/platform-wallet-private-contact-requestsfrom
feat/ffi-swift-private-contact-requests

Conversation

@QuantumExplorer

@QuantumExplorer QuantumExplorer commented Nov 8, 2026 •

Copy link
Copy Markdown
Member

Basic explanation

What this does: iOS apps can now send, receive and accept DashPay private contact requests.

  • The Swift SDK wraps the wallet's private request calls: sync the private store, send, accept with a private reply, and read how a contact's requests came.
  • The request is signed by the same KeychainSigner the app passes for public requests.
  • The DashPay encryption key stays in the Keychain-backed resolver signer, which opens the store and seals the entry.
  • SwiftExampleApp gets a "Send privately" toggle. Accepting a privately received request replies privately.

Value: the private request flow from #5810, usable from iOS without the app handling any key.

Risks: low.

  • No consensus change.
  • New C exports only. The existing public contact request exports are unchanged.
  • New result codes 68 to 71 (see below). Only 68 needs routing, and only by a host that doesn't use the in-kind accept.

Issue being fixed or feature implemented

Part of anonymous DashPay contact requests for 5.1, on top of #5810.

What was done?

rs-sdk-ffi (MnemonicResolverCoreSigner)

  • scan_private_document_page sorts a page with the recipients' secret keys and each sender encryption key's outgoing viewing key, all derived inside the signer from one resolve of the mnemonic.
  • seal_private_contact_request derives the sender's encryption key, seals the request with the identity signer's signature, and wipes the key.
  • resolve_and_derive is now a one-path case of the new resolve_master.
  • The SDK's refusals come back as MnemonicResolverSignerError::PrivateContactRequest.

platform-wallet-ffi

  • ResolverContactCryptoProvider implements the wallet's two private methods over that signer.
  • New C exports:
    • platform_wallet_sync_private_contact_requests(wallet_handle, core_signer_handle, max_pages, out_requests, out_report)
    • platform_wallet_manager_send_private_contact_request(handle, wallet_id_bytes, mnemonic_resolver_handle, account, sender_identity_id, recipient_identity_id, account_label, auto_accept_proof, auto_accept_proof_len, shielded_address, signer_handle, out_request_handle) (shielded feature)
    • platform_wallet_manager_accept_private_contact_request(handle, wallet_id_bytes, mnemonic_resolver_handle, account, request_handle, shielded_address, signer_handle, out_established_handle) (shielded feature)
    • managed_identity_get_private_contact(identity_handle, contact_id, out_contact)
  • signer_handle is the SignerHandle platform_wallet_send_contact_request_with_signer takes. The send and accept follow the shielded spend contract (catch_spend_panic, map_spend_result).
  • New result code 68, ErrorPrivateContactRequestNeedsPrivateReply. It is returned when a public send or accept would publish a contact whose requests came or went privately. Recorded in ERROR_CODE_REGISTRY.md.

swift-sdk

  • ManagedPlatformWallet.syncPrivateContactRequests(maxPages:) returns a PrivateContactSyncResult.
  • PlatformWalletManager.sendPrivateContactRequest(...) and acceptPrivateContactRequest(...) take the resolver, as the other shielded calls do, and signer: KeychainSigner, as the public send and accept do.
  • ManagedIdentity.privateContact(contactId:) returns PrivateContactInfo (came privately, went privately, shielded address), or nil.
  • PlatformWalletError.privateContactRequestNeedsPrivateReply carries code 68.
do {
    _ = try await wallet.acceptContactRequest(request, signer: signer)
} catch PlatformWalletError.privateContactRequestNeedsPrivateReply {
    // before: no such error; the public accept would have published the relationship
    // after:  the host replies privately, signed by the same signer
    _ = try await manager.acceptPrivateContactRequest(
        walletId: walletId, resolver: resolver, request: request, signer: signer)
}

SwiftExampleApp

  • Add Contact has a "Send privately" toggle that pays from the wallet's shielded balance.
  • Accepting a request the public accept refuses as private replies privately, in the request list and in Add Contact's collision alert.
  • Pull-to-refresh on the request list also reads the private store.

Changes from review

  • One accept. platform_wallet_manager_accept_contact_request_replying_in_kind(...) (every build) lets the wallet pick the reply: private when the request came privately and nothing was sent back yet, otherwise public or adopted. Its private reply is paid only when it is one. Swift's acceptContactRequestReplyingInKind(...) wraps it, and SwiftExampleApp accepts only through it.
  • Signed by the identity signer. The resolver signer seals each request with a signature by the identity's signing key, from the same KeychainSigner the public send takes. The scan keys come from the SDK's PrivateDocumentScanKeys::derive.
  • Pending state read from the wallet. managed_identity_get_private_reply_pending(identity_handle, contact_id, out_pending) reads whether a private request to a contact is still in flight. Swift wraps it as ManagedIdentity.privateReplyPending(contactId:).
    • SwiftExampleApp's request rows and Add Contact read that flag rather than screen state. A pending row shows no buttons, and Send stays disabled until a private sync settles the request.
    • Private refresh failures are reported.
  • "Send privately" holds through a collision. When the recipient already sent a public request, Add Contact's collision accept replies privately while the toggle is on (acceptPrivateContactRequest). The alert says the reply is private and paid from the shielded balance. With the toggle off it is unchanged.
  • Wallet-side protections, from feat(platform-wallet): send, receive and accept DashPay private contact requests #5810. These reach the new C entry points:
    • A private payment holds the wallet generation's lifecycle guard, so a wallet removed mid-proof pays nothing (NotFound).
    • A pending reservation is restored after a restart from the shielded store's saved unconfirmed spend.
    • An identity's private store is read to its end before its first private send or accept since load, so a reply paid before a restart is adopted rather than paid again.
  • Faster private sync, host side.
    • The resolver signer scans a batch of pages with one mnemonic resolve.
    • The private store's read position, its key fingerprint, and each private contact's shielded address are persisted, so a relaunch resumes instead of rereading the whole store. They go through three size-gated PersistenceCallbacksExtension slots behind the PLATFORM_WALLET_PERSISTENCE_CAPABILITY_PRIVATE_CONTACT_REQUEST_SCAN_STATE capability.
    • Swift stores them in two new SwiftData models in a new live schema V5, with a lightweight V4 to V5 migration, purged with the wallet. Storage Explorer shows them.
    • A host without the capability keeps the old behaviour: it reads from the store's start.
  • Pending state read failures. A failed pending-state read is kept apart from "nothing pending": the screens keep the last good snapshot and leave Accept and Send off until a read succeeds.
  • Removed wallets. A wallet removed before a private payment returns NotFound (98) on the private-payment exports.
  • New result codes:
    • 69 ErrorShieldedBuildRequired: a private reply on a build without shielded.
    • 70 ErrorProtocolVersionNotActive: the network is below protocol version 15.
    • 71 ErrorPrivateContactRequestPending: a private request to that contact is in flight or not read back yet, or the identity's private store could not be read to its end before its first private operation since load. Nothing new was paid, and the host must not submit it again.
    • All three are typed in Swift and recorded in ERROR_CODE_REGISTRY.md. 71 survives map_spend_result.
accept a request that came privately
  before: host calls accept, gets 68, then calls the private accept
  after:  host calls accept_contact_request_replying_in_kind; the wallet replies privately
a second accept while the first private reply is unconfirmed
  before: the screen's own flag, lost when the screen closes
  after:  71 from the wallet, and privateReplyPending(contactId:) stays true

How Has This Been Tested?

  • cargo test -p rs-sdk-ffi --lib -- mnemonic_resolver_core_signer: 20 passed, 6 of them private. The private ones:
    • the sealed entry is byte for byte a resident wallet's;
    • a scanned page reads as the sender's signed request;
    • one mnemonic resolve per scan and per seal;
    • a signature by another key and a wrong encryption key path are refused, each for its own reason;
    • a missing mnemonic is reported.
  • cargo test -p platform-wallet-ffi --lib -- private_contact: 5 passed; with --features shielded: 7 passed, including null signer handles.
  • cargo clippy -p rs-sdk-ffi -p platform-wallet-ffi --all-targets -- -D warnings, and -p platform-wallet-ffi --features shielded: clean.
  • ./build_ios.sh --target sim, then SwiftExampleApp: BUILD SUCCEEDED.
  • ErrorHandlingTests against that framework on an iPhone 17 Pro simulator: 65 passed, including codes 68 to 71 and every field of PrivateContactInfo. ContactRequestOutcomesTests: 9 passed, including code 71 as pending and both states of the collision accept.
  • cargo test -p platform-wallet-ffi --lib --features shielded -- private: 17 passed, including the pending getter (false for a fresh identity, NotFound for an unknown handle, ErrorNullPointer for a null output).
  • Not run: a private send or accept against a network. That needs a protocol version 15 network with shielded funds.

Breaking Changes

None. New exports, plus result codes 68 to 71.

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

QuantumExplorer and others added 9 commits November 8, 2026 12:07
… resolver signer

`MnemonicResolverCoreSigner` gains the three key operations DashPay
private contact requests (protocol version 15) need, in the style of
`contact_info_seal` and `ecdh_shared_secret`: each derives the keys it
needs at Rust-built paths, wipes its copies, and returns only the result.

- `scan_private_document_page` sorts a page of a private document store
  with the recipients' secret keys and the senders' outgoing viewing
  keys.
- `read_private_contact_requests` reads a scan's requests, checking each
  sender with the recipient key that opened it. Only recipient keys are
  derived.
- `seal_private_contact_request` seals a request with the sender's
  encryption key.

A scan or a read derives every key from one resolve of the mnemonic, not
one Keychain read per key: `resolve_and_derive` is now a one-path case of
the new `resolve_master`. The SDK's refusals come back as the new
`MnemonicResolverSignerError::PrivateContactRequest`, keeping the
`dash_sdk::Error`.

Tests pin the signer to a resident wallet of the same seed: the sealed
entry is byte for byte the one the SDK seals with the seed-derived key,
and a page scans and reads exactly as with seed-derived scan keys.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…he FFI

The resolver-backed `ContactCryptoProvider` implements the three private
contact request methods by delegating to `MnemonicResolverCoreSigner`, so
the keys stay in the signer. New C exports:

- `platform_wallet_sync_private_contact_requests(wallet_handle,
  core_signer_handle, max_pages, out_requests, out_report)` runs
  `DashPayView::sync_private_contact_requests` and returns the requests
  it ingested with a `PrivateContactSyncReportFFI`. The wallet is looked
  up under the registry guard and the pass runs outside it.
- `platform_wallet_manager_send_private_contact_request` and
  `platform_wallet_manager_accept_private_contact_request` (`shielded`
  feature) pay from an account's shielded notes, with the shielded spend
  contract: panics and results go through `catch_spend_panic` and
  `map_spend_result`, as the other note-spending exports do.
- `managed_identity_get_private_contact` reads `PrivateContact`: whether
  requests came or went privately, and the contact's shielded address.

`PrivateContactRequestNeedsPrivateReply` maps to the new result code
`ErrorPrivateContactRequestNeedsPrivateReply` (68), so a host routes a
privately received request to the private accept. 61 to 67 are already
held by open branches; the registry records the claim and those holds.

The auto-accept proof copy moved into `read_auto_accept_proof`, shared
by the public and private sends.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Thin wrappers over the new platform-wallet FFI:

- `ManagedPlatformWallet.syncPrivateContactRequests(maxPages:)` returns
  a `PrivateContactSyncResult`.
- `PlatformWalletManager.sendPrivateContactRequest(...)` and
  `acceptPrivateContactRequest(...)` pay from the wallet's shielded
  notes, taking the resolver as the other shielded calls do.
- `ManagedIdentity.privateContact(contactId:)` returns
  `PrivateContactInfo` (came privately, went privately, shielded
  address), or nil.

`PlatformWalletResultCode.errorPrivateContactRequestNeedsPrivateReply`
(68) and `PlatformWalletError.privateContactRequestNeedsPrivateReply`
carry the public accept's refusal, pinned in `ErrorHandlingTests`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…hem privately

- Add Contact has a "Send privately" toggle that sends the request from
  the wallet's shielded balance.
- Accepting a request the public accept refuses as private replies
  privately instead, in the request list and in Add Contact's collision
  alert. Rust decides which; the app only reacts to the refusal.
- Pull-to-refresh on the request list also reads the private store, which
  is what records that a request came privately.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…er's signature

Private contact requests are now signed by the sender's identity signer
with one of its authentication keys, as public requests are. The resolver
signer keeps only the DashPay encryption key:

- `seal_private_contact_request` takes the signing key id and the 65-byte
  signature and passes them to the SDK seal. It signs nothing itself; the
  SDK refuses a signature that is not the signing key's. The encryption
  key, derived for the outgoing viewing key and the key check, is wiped on
  drop as before.
- `read_private_contact_requests` is removed: checking an entry needs no
  secret key, so the wallet reads the scan itself.
- `scan_private_document_page` takes each sender encryption key's id with
  its path, as `PrivateDocumentScanKeys::with_sender` now does.

Tests sign with `SingleKeySigner` through `private_contact_request_signature`,
as an identity signer signs data. The sealed entry is still byte for byte
the one a resident wallet seals; a scanned page reads as the sender's signed
request; a scan and a seal each resolve the mnemonic once; a signature by
another key and a wrong encryption key path are refused, each for its own
reason.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… request FFI

The private send and accept now take the identity signer the public send
and accept take, and pass it to the wallet, which signs the request with
it:

    platform_wallet_manager_send_private_contact_request(handle,
        wallet_id_bytes, mnemonic_resolver_handle, account,
        sender_identity_id, recipient_identity_id, account_label,
        auto_accept_proof, auto_accept_proof_len, shielded_address,
        signer_handle, out_request_handle)
    platform_wallet_manager_accept_private_contact_request(handle,
        wallet_id_bytes, mnemonic_resolver_handle, account, request_handle,
        shielded_address, signer_handle, out_established_handle)

`signer_handle` is a `*mut SignerHandle` (a `VTableSigner`), crossed into
the worker as an address and re-materialized there, as
`platform_wallet_send_contact_request_with_signer` does. A null signer is
`ErrorNullPointer` before the wallet is touched.

`ResolverContactCryptoProvider` follows the new provider trait: no
`read_private_contact_requests`, and the seal passes the signing key id and
signature through. `managed_identity_get_private_contact` maps the owned
`PrivateContact`; its docs say the flags now hold across restarts and only
the shielded address waits for a sync.

Code 68 (`ErrorPrivateContactRequestNeedsPrivateReply`) also covers a
public send to a contact whose requests came or went privately; its docs
and the registry row say so.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`PlatformWalletManager.sendPrivateContactRequest` and
`acceptPrivateContactRequest` take `signer: KeychainSigner`, last as in
`sendContactRequest` and `acceptContactRequest`, and keep it alive across
the FFI call with the resolver. The docs say the private request is signed
by the same identity signer, that `privateContact(contactId:)` holds across
launches (only the shielded address waits for a sync), and that the public
send also throws `privateContactRequestNeedsPrivateReply` for a contact
whose requests came or went privately. The `ErrorHandlingTests` pin of
code 68 carries the wallet's new message.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…in signer

Add Contact's private send and the private-accept fallback pass the
`KeychainSigner` the public send and accept already use.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@QuantumExplorer QuantumExplorer added this to the v5.1.0 milestone Nov 8, 2026
@coderabbitai

coderabbitai Bot commented Nov 8, 2026 •

Copy link
Copy Markdown
Contributor

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

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

@thepastaclaw

thepastaclaw commented Nov 8, 2026 •

Copy link
Copy Markdown
Collaborator

🕓 Review not started yet because the new head is waiting for the 30-minute push debounce.

  • 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 8738c02. Normal review starts when eligible; priority review starts as soon as a slot is available.

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Final validation — Phase 1 + Phase 2

Static verification at 6787f32 confirms four blocking issues involving ambiguous-spend retries, post-payment persistence failures, teardown admission, and acceptance orchestration ownership. The cryptographic delegation and ABI mapping are otherwise sound; the claimed missing Rust pin for code 68 already exists. Current CI reports passing Kotlin build/tests and no failed code checks; no local builds or tests were run.

🔴 4 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: ffi-engineer); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 7: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); 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 cross-language change directly implements cryptographic key derivation, private-document scanning and request sealing in packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs, including scan_private_document_page and seal_private_contact_request.
  • 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/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/DashPay/AddContactView.swift`:
- [BLOCKING] packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/DashPay/AddContactView.swift:471-479: Keep unconfirmed private requests pending instead of enabling a fresh send
  The private send can throw `shieldedSpendUnconfirmed`, but the generic catch treats it as an ordinary failure and the defer clears `isSending`. The native send has not recorded an outgoing contact request at this point, although its transition may already have executed or remain queued for byte-identical re-drive. With other spendable notes available, another tap can build a different transition and pay another fee; reserving the first transition's notes does not prevent this. The newly routed private accepts in this view and ContactRequestsView likewise restore their Accept controls after an ambiguous completion. Handle this outcome as pending, following the distinction already made by SendViewModel, and reconcile native spend recovery plus private-store ingestion before allowing a fresh submission. Add injected-unconfirmed coverage for both send and accept.

In `packages/rs-platform-wallet-ffi/src/private_contact_request.rs`:
- [BLOCKING] packages/rs-platform-wallet-ffi/src/private_contact_request.rs:528-533: Preserve the committed accept outcome after a contact-store failure
  This exported accept can report an ordinary failure after its shielded reply was confirmed. In the native private send, `pay(entry)` succeeds before `add_sent_contact_request`; a failure of that store is logged and swallowed. Because `add_sent_contact_request` persists before updating memory, the failed write leaves neither a sent request nor an established contact. `accept_contact_request_replying_with` then fails its final established-contact lookup with `ContactRequestNotFound`, which this export maps to a generic wallet-operation failure. Retrying before private-store sync recovers the reply can therefore broadcast and pay for another reply. Fix the native private-accept outcome contract so a confirmed payment followed by local persistence failure remains committed/recovery-required, with protection against a fresh reply. Test a failed contact store after successful payment and verify that retry does not invoke payment again.

In `packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PrivateContactRequests.swift`:
- [BLOCKING] packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PrivateContactRequests.swift:156-162: Include private spends in teardown admission
  Both new manager methods detach their native calls without entering `admitNativeOp`/`finishNativeOp`. Consequently, `deleteWallet` can pass its zero-active-operation guard while either spend is running, and shutdown does not drain these calls. The FFI retains wallet and coordinator Arcs, so removal does not cancel an operation already proving. After proof construction, `private_document_create_from_pool_with_metadata` records activity and broadcasts without a wallet-generation liveness gate; `save_activity` uses `entry(id).or_default()`, allowing removed shielded state to be recreated and persisted after the wipe. Count both Swift operations through completion, and make the native private-spend path participate in the existing generation lifecycle guard so direct native/FFI callers are also protected. Test removal and shutdown while proving is paused.

In `packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/DashPay/ContactsView.swift`:
- [BLOCKING] packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/DashPay/ContactsView.swift:249-257: Move automatic acceptance routing into platform-wallet
  This helper attempts public acceptance, interprets the wallet's privacy refusal, and substitutes a shielded-funded private acceptance. That is DashPay orchestration, not argument marshalling or an explicit user-selected channel. The coding conventions assign mobile DashPay orchestration to rs-platform-wallet and require bindings to call an existing Rust operation rather than make these decisions; the motivating example explicitly includes logic in a Swift view. The Rust refusal correctly prevents public disclosure, but every other client must currently reproduce the replacement-operation policy. Add a channel-aware native acceptance operation using the wallet-owned request provenance and reciprocity state, expose it through a thin binding, and have the views supply credentials and acceptance intent. The explicitly public-only API can retain its refusal contract.
- [SUGGESTION] packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/DashPay/ContactsView.swift:233-235: Expose failed or incompletely persisted private refreshes
  This helper suppresses every private-sync error and discards the report. An unsupported pre-v15 network can be an expected case, but network failures, unavailable resolver material, and a nonzero `unpersistedIdentityCount` do not mean the request list was successfully refreshed. This is the example app's only private-store ingestion call, so these outcomes leave stale or empty results without indicating that private requests could not be checked or durably ingested. Handle the expected unsupported-network case separately and expose operational errors and incomplete persistence through refresh error/banner state.

In `packages/rs-platform-wallet-ffi/src/dashpay.rs`:
- [SUGGESTION] packages/rs-platform-wallet-ffi/src/dashpay.rs:1300-1305: Preserve the missing-mnemonic key-availability signal
  When the resolver returns NOT_FOUND during private-store scanning, `MnemonicResolverSignerError::NotFound` carries the reserved key-unavailable marker at position zero. This conversion wraps it in `InvalidIdentityData`, which has no dedicated FFI mapping and therefore becomes `ErrorUnknown` (99). The existing code-31 restoration only recognizes the marker structurally inside `Sdk(Protocol(Generic(...)))`, so it cannot restore the signal from this variant. A caller consequently cannot distinguish missing signing material from an unknown internal failure. Map NotFound explicitly to a key-unavailable outcome and add an FFI mapping test; preserve the structural discriminator rather than introducing substring matching.

In `packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs`:
- [SUGGESTION] packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs:712-726: Share recipient and sender scan-key assembly
  `PrivateDocumentKeyPaths::scan_keys` in platform-wallet already performs the recipient-secret and sender-outgoing-viewing-key transformation through a derivation callback. This adds another implementation in the FFI crate, with independently maintained temporary-key cleanup. The wallet still supplies the roles and paths, so this is not an incorrect key-selection finding, but the two implementations can drift as scan-key construction or erasure changes. Keep mnemonic resolution and derivation inside the resolver adapter, and extract the shared assembly into a lower Rust-library helper, such as a callback-based constructor in rs-sdk's private_document_scan module. Preserve this implementation's wipe-on-error handling when sharing it.
- [SUGGESTION] packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs:181-187: Cover the remaining documented seal-refusal inputs
  The new error contract documents malformed requests and contracts without the private type, but the signer tests exercise SDK refusals only through an unrelated encryption key and another key's signature. Add malformed-request and missing-private-type cases that assert `MnemonicResolverSignerError::PrivateContactRequest` and verify that no sealed entry is returned. Validation should remain in rs-sdk; these tests would pin the adapter's forwarding and typed-error contract rather than add validation to the binding.

QuantumExplorer and others added 4 commits November 8, 2026 20:14
… the FFI

`platform_wallet_manager_accept_contact_request_replying_in_kind(handle,
wallet_id_bytes, mnemonic_resolver_handle, account, request_handle,
shielded_address, signer_handle, out_established_handle)` (`shielded`
feature) exposes `PlatformWallet::accept_contact_request_replying_in_kind`:
the one accept a host calls. The wallet replies privately to a request that
came privately with no reply yet, and with a document (or by adopting the
reply already sent) otherwise, so hosts no longer catch code 68 and call
the private accept themselves.

It is built as `platform_wallet_manager_accept_private_contact_request`
is: same arguments and null checks, `catch_spend_panic` and
`map_spend_result` for the shielded spend contract, and a null-signer test.
The docs of code 68 name it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`PlatformWalletManager.acceptContactRequestReplyingInKind(walletId:resolver:account:request:shieldedAddress:signer:)`
is a thin wrapper over the new FFI export: the wallet decides whether the
reply is private or a document. The explicitly public and private accepts
stay. The file header and the code 68 docs point hosts at it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…quest is replied to

The request list and Add Contact's collision alert call
`acceptContactRequestReplyingInKind` once instead of trying the public
accept and falling back to the private one on code 68. The
`acceptIncomingContactRequest` helper that held that choice is removed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Verified the findings against head 1c07538. Wallet-owned acceptance routing fixes the previous layering issue, but four blocking issues remain: private operations bypass teardown admission, committed and ambiguous replies can enable duplicate fee-bearing requests, and public acceptance unnecessarily requires shielded configuration. This was a static review with no builds or tests run; current CI shows Kotlin build/tests passing and the review pending, with no Rust or iOS validation jobs listed.

🔴 4 blocking | 🟡 3 suggestion(s)

6 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: ffi-engineer); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 7: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); 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 cross-language diff directly changes cryptographic key derivation, private-document scanning, request sealing, and signature handling in packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs, including scan_private_document_page and seal_private_contact_request.
  • 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-ffi/src/private_contact_request.rs`:
- [BLOCKING] packages/rs-platform-wallet-ffi/src/private_contact_request.rs:616-623: Acquire shielded resources only when acceptance needs a private reply
  The new in-kind entrypoint calls resolve_wallet_and_coordinator before the wallet chooses public reply, private reply, or adoption. That helper rejects a loaded wallet unless configure_shielded has run, so public acceptance and adoption now fail with 'shielded support not configured' even though neither needs a shielded payment. Both example-app acceptance sites have switched unconditionally to this entrypoint. Resolving the seed here also imposes spend-authority access before the wallet can determine that no payment is needed; delaying derive_spend_keyset inside the wallet does not avoid these earlier failures. Keep routing in rs-platform-wallet, but make shielded-context and spend-authority acquisition lazy behind its private-payment branch. Cover public and adopt-only acceptance without a shielded coordinator.
- [BLOCKING] packages/rs-platform-wallet-ffi/src/private_contact_request.rs:531-536: Preserve the committed accept outcome after a contact-store failure
  (existing thread: https://github.com/dashpay/platform/pull/5812#discussion_r4506333080)
  A confirmed private reply can reach this Err arm as an ordinary operation failure. DashPayView::send_private_contact_request awaits pay successfully, logs a failed add_sent_contact_request, and returns Ok. That identity update persists before inserting the established contact into memory, so the enclosing accept_contact_request_replying_with then fails its final lookup with ContactRequestNotFound. Both accept exports map this to ErrorWalletOperation, and the views offer acceptance again despite the first reply already having committed and consumed its fee. With no recorded outgoing request to adopt, another attempt can pay for another private entry. Preserve a committed-but-local-repair-pending outcome in rs-platform-wallet's acceptance orchestration and marshal it through both bindings; test a persister failure after successful payment without allowing a second paid reply.

In `packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PrivateContactRequests.swift`:
- [BLOCKING] packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PrivateContactRequests.swift:158-164: Include private spends in teardown admission
  (existing thread: https://github.com/dashpay/platform/pull/5812#discussion_r4506333083)
  sendPrivateContactRequest, acceptPrivateContactRequest, and acceptContactRequestReplyingInKind capture the manager handle and suspend into Task.detached without calling admitNativeOp/finishNativeOp. shutdown() drains only activeNativeOpCount, and deleteWallet() uses that same count to exclude concurrent work, so either can overtake these operations. The native exports release the registry guard after cloning wallet/coordinator Arcs; those preserve object lifetime but do not stop a proof already in progress from broadcasting and writing activity after teardown or deletion. Enroll all three wrappers in balanced admission before their first suspension and retain it through native completion and result conversion. Add lifecycle coverage demonstrating that shutdown waits and deletion is refused while a private operation is active.

In `packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/DashPay/AddContactView.swift`:
- [BLOCKING] packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/DashPay/AddContactView.swift:471-479: Keep unconfirmed private requests pending instead of enabling a fresh send
  (existing thread: https://github.com/dashpay/platform/pull/5812#discussion_r4506333077)
  When this call throws shieldedSpendUnconfirmed, the generic catch only sets errorMessage and defer clears isSending, immediately enabling Send Request again. The wallet records the outgoing contact request only after pay returns confirmed metadata, so this ambiguous attempt has no contact record that prevents another submission. Shielded reservations protect the first attempt's notes, not the recipient: another call can select other available notes and append another independently sealed request, while the original may still confirm or be re-driven by sync. Both acceptance actions similarly release their in-flight guards on this outcome. Retain the unresolved contact operation and its reconciliation status in rs-platform-wallet, expose it through the bindings, and render it as pending until resolved, including after reopening or restart.

In `packages/rs-platform-wallet-ffi/src/dashpay.rs`:
- [SUGGESTION] packages/rs-platform-wallet-ffi/src/dashpay.rs:1300-1305: Preserve the missing-mnemonic key-availability signal
  (existing thread: https://github.com/dashpay/platform/pull/5812#discussion_r4506333098)
  MnemonicResolverSignerError::NotFound carries the reserved key-unavailable marker, but this catch-all wraps it in InvalidIdentityData. A missing mnemonic during private scanning therefore becomes ErrorUnknown rather than ErrorSigningKeyUnavailable (31); map_spend_result likewise cannot recover the classification from this wrapper. Keeping the marker somewhere in the rendered message is insufficient because both conversions deliberately recognize it only in the structured SDK ProtocolError::Generic payload. Preserve NotFound through the existing key-unavailable representation while keeping malformed mnemonic and derivation errors distinct. Add a resolver-provider-to-FFI boundary test asserting code 31, not only the signer's intermediate NotFound variant.

In `packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/DashPay/ContactsView.swift`:
- [SUGGESTION] packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/DashPay/ContactsView.swift:233-235: Expose failed or incompletely persisted private refreshes
  (existing thread: https://github.com/dashpay/platform/pull/5812#discussion_r4506333092)
  This helper discards every thrown error and the successful PrivateContactSyncResult. Consequently, a network, proof, or unavailable-mnemonic failure looks like a completed empty refresh, and unpersistedIdentityCount > 0 is ignored even though those requests did not reach durable storage. The wallet retains affected cursors for retry, but the request view receives no indication that recovery is needed. This is the example app's private-store receiving path. Distinguish expected unsupported-network behavior from operational failures, and expose failed or incompletely persisted passes through the refresh/error state with a retry indication.

In `packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs`:
- [SUGGESTION] packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs:712-726: Share recipient and sender scan-key assembly
  (existing thread: https://github.com/dashpay/platform/pull/5812#discussion_r4506333099)
  This helper adds a second implementation of the scan-key assembly already exposed by rs-platform-wallet's PrivateDocumentKeyPaths::scan_keys and used by the resident provider: derive each recipient's keyed secrets and derive an outgoing viewing key for each sender. The implementations already handle temporary cleanup differently, and future construction changes must now be maintained separately for resident and resolver-backed scans. Extract the common assembly into a lower library accessible to both callers, such as an SDK builder accepting a derivation closure. Retain the resolver's single resolve_master scope and wiping guards, and do not introduce a dependency from rs-sdk-ffi back to platform-wallet. This is a shared-helper maintainability suggestion, not a claim that the current code selects incorrect keys.
Out-of-scope follow-up suggestions (1)

These are valid observations, but they are outside this PR's scope and should be handled in separate issues or author/maintainer-requested PRs rather than blocking this review.

  • Apply lifecycle admission to the existing shieldedTransfer wrapper — PlatformWalletManagerShieldedSync.swift's pre-existing shieldedTransfer also captures the manager handle and runs detached native work without admission. Correcting the new private-contact wrappers alone leaves this older spend able to outlive the manager's admission drain.
    • Follow-up: A separate follow-up PR should enroll shieldedTransfer in balanced native-operation admission and test shutdown and deletion during an in-flight transfer.

QuantumExplorer and others added 9 commits November 8, 2026 22:11
… derive

- `scan_private_document_page` builds its keys with
  `PrivateDocumentScanKeys::derive`, which wipes the derived keys on an
  error too. The signer keeps only the mnemonic resolve and the per-path
  derivation; its own scan-keys helper and `WipingSecretKeys` are gone.
- `seal_private_contact_request` takes `platform_version` last and passes
  it to the SDK seal, which validates the payload with dpp.
- Tests follow the SDK: signing data and seals take the platform version,
  and the read takes `ProvedIdentity` entries proved at the heights the
  scan needs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… it is one, in every build

`platform_wallet_manager_accept_contact_request_replying_in_kind` no
longer looks up the shielded coordinator or reads the mnemonic up front.
It hands the wallet a payment that, only for a private reply, looks the
coordinator up and resolves the seed on a blocking thread. A public or
adopted reply therefore works on a manager without
`platform_wallet_manager_configure_shielded` and reads no mnemonic for the
payment (an FFI test pins both). A failure of that lookup returns the code
the lookup reported. A reply for which no payment was asked keeps the
codes the public accept returns; once one was asked, errors keep the
shielded spend contract.

The export and its JNI-facing contract exist in every build. Without the
`shielded` feature it calls `DashPayView::accept_contact_request_replying_in_kind`
with a payment that refuses: a request that needs a private reply fails
with the new `ErrorShieldedBuildRequired` (69), and a public or adopted
reply works as before. No existing code meant "shielded unsupported".

Also:
- `ErrorProtocolVersionNotActive` (70): the SDK's lasting
  `ProtocolVersionNotActive` refusal, such as the private store before
  protocol version 15, was `ErrorUnknown`; hosts can now stay quiet about
  it. A refusal that may lift soon keeps the catch-all.
- A missing mnemonic while scanning or sealing the private store maps to
  the key-unavailable outcome (31), through the signer's position-0
  marker in a `ProtocolError::Generic`, matched by variant; a test drives
  it through the resolver-backed provider.
- The seal passes `platform_version`.
- The private store sync runs on `run_on_big_stack_thread`: the SDK's
  identity fetch stream trips rust-lang/rust#100013 against
  `block_on_worker`'s `Send + 'static` bounds.

Both codes are in `ERROR_CODE_REGISTRY.md`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…type codes 69 and 70

- `sendPrivateContactRequest`, `acceptPrivateContactRequest` and
  `acceptContactRequestReplyingInKind` are admitted with `admitNativeOp`
  and finished with `finishNativeOp`, so a teardown waits for them as it
  does for the other counted manager calls.
- `PlatformWalletError.shieldedBuildRequired` (69) and
  `.protocolVersionNotActive` (70), with `ErrorHandlingTests` pins.
- The in-kind accept's docs say a public or adopted reply needs neither
  shielded support nor a payment mnemonic read.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…and report private refresh failures

- A private send or an in-kind accept whose result could not be confirmed
  (`shieldedSpendUnconfirmed`) stays pending, as `SendViewModel` treats an
  unconfirmed spend: Add Contact shows a Done-only notice and no longer
  enables Send, and the request row keeps its pending state without its
  buttons until the next sync settles it.
- The request list's private refresh stays quiet when the network does not
  serve the private store yet (code 70), and shows any other failure, and
  requests read but not saved, in a notice above the list.
- `ContactRequestSubmissionOutcome` and `PrivateContactRefreshNotice` hold
  those two readings, pinned by `ContactRequestOutcomesTests`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…code and read it per contact

- `PlatformWalletError::PrivateContactRequestPending` maps to the new
  `ErrorPrivateContactRequestPending` (71), both in the blanket conversion
  and in the shielded spend contract the private send and accepts use,
  which would otherwise flatten it to `ErrorWalletOperation`. A private
  request to that contact is in flight or paid and not yet read back;
  nothing new was paid, and a later private sync settles it. Recorded in
  `ERROR_CODE_REGISTRY.md`.
- `managed_identity_get_private_reply_pending(identity_handle, contact_id,
  out_pending)` is a pure read of `DashPayState::private_reply_pending`, so
  hosts disable a second send or accept from wallet-owned state. It is a
  getter rather than a `PrivateContactFFI` field because the latter
  answers NotFound until a request has come or gone privately, while a
  first private send is already pending.
- The private store sync runs on `block_on_worker` again: the SDK's
  identity fetch future is `Send` now, so the big-stack thread is no
  longer needed (`block_on_worker` already polls on the 8 MB worker stack).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…er contact

- `PlatformWalletError.privateContactRequestPending` (71), with an
  `ErrorHandlingTests` pin. The private send, private accept and in-kind
  accept docs name it.
- `ManagedIdentity.privateReplyPending(contactId:)` wraps
  `managed_identity_get_private_reply_pending`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The request rows and Add Contact's Send read whether the wallet has a
private request to the contact in flight or not yet read back
(`privateRepliesPending`, over `ManagedIdentity.privateReplyPending`),
instead of keeping an unconfirmed accept in screen-local state. While it is
set, the row shows pending without its buttons and Send is off; it clears
when a private sync settles it, and it holds when the screen is left and
reopened. The views re-read it on appear, after an accept or send, and
after each sync pass.

`privateContactRequestPending` (71) is the same outcome as an unconfirmed
spend: pending, no fresh submission. `ContactRequestOutcomesTests` covers
it and the row message.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
QuantumExplorer and others added 5 commits November 9, 2026 09:16
…tact-requests' into feat/ffi-swift-private-contact-requests

Takes the wallet's batch scan of the private contact request store and
its PaymentStarted pay token: the in-kind accept's non-shielded refusal
takes the token and never marks it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ent at NotFound

`map_spend_result` sent `WalletRemovedBeforeSend` through its catch-all
to `ErrorWalletOperation` (6), while every other export gives it the
lifecycle refusal code `NotFound` (98). It now passes through the blanket
conversion beside `PrivateContactRequestPending`, so a host can tell a
removed wallet from an ordinary failure without reading the message.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…est scan state

The wallet keeps where each identity reads DashPay's private contact
request store from next, and its private contacts' shielded addresses,
behind `PRIVATE_CONTACT_REQUEST_SCAN_STATE` (bit 14). This carries them
across the FFI and stores them in SwiftData, so a relaunch resumes the
private sync instead of reading the store from its start.

FFI: three size-gated `PersistenceCallbacksExtension` slots, appended
after the asset-lock operation id pair. The persist slot receives a
round's read positions, addresses, address removals and the identities
the round removed (whose scan state the host clears), after the contacts
callback. The load slot returns a wallet's positions and addresses during
the wallet-list load; they are applied to the identities that load
restored, and a host error or a missing buffer fails the load. The
capability is attested only with all three slots, the wallet-list
restore callbacks and the declaration, and nothing is read from a host
that does not attest it.

Swift: two new models, `PersistentPrivateContactRequestsPosition` and
`PersistentPrivateShieldedAddress`, in a new live schema V5; V4 is bound
to its published snapshot with a lightweight V4 to V5 stage. The handler
writes them inside the changeset round, purges them with the wallet, and
declares the capability.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… pending

`privateRepliesPending` read a failed wallet read as "nothing pending",
so the request list brought back Accept and Ignore, and Add Contact
enabled Send, with nothing showing the pending request had cleared. The
read now returns nil on a failure, and `PrivateRepliesPendingSnapshot`
keeps the last successful snapshot and leaves those controls off for
every contact until a read covering it succeeds. An unmanaged identity
still reads as nothing pending.

A pending private request (code 71), which the first public send or
accept since load can now meet, gets its own outcome: nothing was sent,
so the view shows it as pending and Add Contact offers "Sync private
requests", after which Send comes back for the user to try again.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…in Storage Explorer

`PersistentPrivateContactRequestsPosition` and
`PersistentPrivateShieldedAddress` get a row, a count, a list view and a
detail view, as `PersistentIdentityBalanceMetadata` does, so the explorer
covers every model `DashModelContainer` registers again.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
QuantumExplorer added a commit that referenced this pull request Nov 9, 2026
…g pending

`privateRepliesPending` caught a failed wallet read and returned an empty
set, so Requests brought back Accept and Ignore, and Add Contact enabled
Send, with nothing showing the pending request had cleared. The read now
returns null on a failure, and `PrivateRepliesPendingSnapshot` keeps the
last successful snapshot and leaves those controls off for every contact
until a read covering it succeeds. An unmanaged identity still reads as
nothing pending.

A pending private request (code 71), which the first public send or
accept since load can now meet, gets its own outcome: nothing was sent,
so Add Contact shows it as pending and offers "Sync private requests",
after which Send comes back for the user to try again. Port of the Swift
change on #5812.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Verified the findings against head 342162b: three blockers remain involving restart-time payment recovery, unchecked scan-provider binding, and incomplete persistence capability negotiation. Eleven prior findings are fixed, one remains valid, and one remains withdrawn; three additional suggestions are retained. This was static verification only: current code checks pass, while the review check remains pending.

🔴 3 blocking | 🟡 3 suggestion(s)

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: ffi-engineer); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 7: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); 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 cross-language change directly implements private request signing and shielded payment flows in packages/rs-platform-wallet-ffi/src/private_contact_request.rs and changes persisted scan-state handling and SwiftData schema migration.
  • 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-ffi/src/private_contact_request.rs`:
- [BLOCKING] packages/rs-platform-wallet-ffi/src/private_contact_request.rs:156-162: Verify the scan provider before advancing durable cursors
  This export constructs a resolver-backed provider and immediately invokes the state-mutating scan without verifying that the resolver supplies this wallet's seed. The native scan also performs no binding check. A different valid mnemonic derives valid but unrelated keys: `PrivateDocumentScanKeys::scan_page` finds no matching requests but still advances `next_position`. The new persistence extension then saves that progress. Correcting the resolver does not trigger a rewind because `private_scan_key_fingerprint` hashes the managed identity's expected index and public keys, not the keys actually derived by the provider. Requests skipped by the wrong-seed pass can consequently remain missing across launches. Establish the checked boundary in rs-platform-wallet before scan-state mutation, reusing the existing `verify_seed_binds` mechanism rather than duplicating validation in FFI. Add a wrong-mnemonic scan followed by a correct-mnemonic scan, asserting that refusal leaves the cursor unchanged and the correct scan recovers the request.
- [BLOCKING] packages/rs-platform-wallet-ffi/src/private_contact_request.rs:776-781: Keep unconfirmed private requests pending instead of enabling a fresh send
  (existing thread: https://github.com/dashpay/platform/pull/5812#discussion_r4507400051)
  The bind-time restore and read-before-decision gate leave public acceptance available before pending-payment recovery. `load_from_persistor` discards the shielded snapshot, and `should_not_reply_publicly_after_a_restart_with_a_private_reply_unconfirmed` explicitly observes no pending reservation before binding. After restarting with an unresolved private reply to a public incoming request, the committed-store scan can reach its end without finding that unexecuted reply. This in-kind operation can then choose a public reciprocal, while the original private transition remains capable of executing. The getter also exports a successful false during this interval. Additionally, `install_shielded_views` logs reservation-restoration failures and returns success, leaving the same public branch unprotected after a failed restore. The hooks described in your latest reply therefore cover successful binding and already-appended entries, but not recovery readiness. Include the remaining rs-platform-wallet fix from #5810: restore or rule out durable pending associations before exposing definitive pending state or admitting public submissions, or refuse those operations until recovery succeeds. Cover acceptance before binding and after restoration failure without requiring spend authority for genuinely public-only wallets.

In `packages/rs-platform-wallet-ffi/src/persistence.rs`:
- [BLOCKING] packages/rs-platform-wallet-ffi/src/persistence.rs:1797-1803: Require contact persistence before enabling resumable private scans
  The structural capability mask accepts the scan-state callback trio and wallet-list restore even when `on_persist_contacts_fn` is absent; the new `scan_state_persister` fixture explicitly constructs and accepts this combination. In that configuration, `FFIPersister::store` silently skips contact writes and returns success. Native ingestion therefore treats the requests as persisted and saves an advanced cursor through the scan-state extension. On restart, the cursor and matching fingerprint are restored without the corresponding requests, so later scans skip the entries needed to reconstruct them. This changes an ephemeral-contact host from rereading the store to durably losing requests. Require the contact-persistence callback as part of this capability's structural dependencies, and cover a declared scan-state capability with the trio present but the contact callback missing.

In `packages/rs-platform-wallet-ffi/src/shielded_send.rs`:
- [SUGGESTION] packages/rs-platform-wallet-ffi/src/shielded_send.rs:628-629: Preserve protocol-version refusals through the private spend mapper
  The explicit private send and accept use this mapper, but their preliminary private-store read can return `PlatformWalletError::Sdk(ProtocolVersionNotActive)` before any payment. That error reaches the catch-all and becomes `ErrorWalletOperation` (6), bypassing the blanket conversion that maps a lasting refusal to code 70 and leaves a transient refusal as `ErrorUnknown`. The same refusal therefore has a typed outcome through private sync or pre-payment in-kind acceptance but a generic operation failure through the explicit private APIs. Pass this structured SDK variant through the blanket conversion and add mapping regressions for both lasting and transient refusals.

In `packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift`:
- [SUGGESTION] packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift:12133-12136: Fail scan-state loads when a stored row cannot cross the ABI
  These filters turn malformed persisted rows into successful absence. For example, an address row with an invalid byte length is discarded while its identity's valid cursor is returned successfully. Rust then resumes beyond the entry that supplied the address, so an ordinary private refresh cannot reconstruct the omitted state. The load boundary already propagates storage errors; unrepresentable rows should preserve that same distinction between unreadable state and an empty store. Validate all rows before allocating either output and return a load error if any row cannot be marshalled.

In `packages/rs-platform-wallet-ffi/src/contact_persistence.rs`:
- [SUGGESTION] packages/rs-platform-wallet-ffi/src/contact_persistence.rs:229-236: Extend ABI layout guards to the new FFI structs
  The existing contact structs have compile-time size and alignment assertions, but the new scan-position and shielded-address structs have none. `PrivateContactSyncReportFFI` and `PrivateContactFFI` in `private_contact_request.rs` likewise lack these guards. Extend the existing regression protection to the new ABI shapes: on the supported 64-bit targets their size/alignment pairs are 72/8, 107/1, 24/8, and 46/1 respectively. These assertions protect against accidental size or alignment changes; field-offset assertions would be needed to detect same-size field reordering.

QuantumExplorer and others added 9 commits November 9, 2026 11:47
…tact-requests' into feat/ffi-swift-private-contact-requests

Takes the wallet's copy of codes 68 to 71 and `map_spend_result`'s 98,
with code 71 now also mapping `PrivateContactPaymentsNotRestored`, its
doc and registry row extended. One more test page literal gains
`chunk_power: None`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…uest FFI structs

Compile-time size, alignment and field-offset guards, as the contact
structs have: the scan-position row (72/8), the shielded-address row
(107/1), `PrivateContactSyncReportFFI` (24/8) and `PrivateContactFFI`
(46/1), on 64-bit targets.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… scan-state capability

A read position vouches for the contact requests read before it. Without
`on_persist_contacts_fn`, `FFIPersister::store` skips those requests and
returns success, so the advanced position was stored without them and a
restart skipped the entries that carried them. The capability now also
needs the contacts callback; a host with the scan-state trio and the
declaration but no contacts callback is not attested and has nothing read
at load.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…h map_spend_result

The store read a private send or accept makes before paying can return
`Sdk(ProtocolVersionNotActive)`, which the spend mapper flattened to
`ErrorWalletOperation` (6). It now passes through the blanket conversion,
like `PrivateContactPaymentsNotRestored`: a lasting refusal is 70, a
transient one `ErrorUnknown`, as on every other path.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…re by its DashPay code

Each private sync pass, and the store read a send or accept makes first,
now checks that the resolver's seed is the wallet's. A mismatch reached
the host as `ErrorUnknown` (or `ErrorWalletOperation` through the spend
mapper); it now takes `ErrorInvalidParameter`, the code every DashPay
export and the seed check give it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…oss the ABI

The load shim filtered out stored rows of the wrong length and handed
back the rest, so an address row of 42 bytes vanished while its
identity's read position was restored, and the sync resumed past the
entry that carried the address. Every row is now checked before either
array is allocated, and one that cannot be marshalled fails the load as
an unreadable store does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… replies

The private sync and `PrivateContactInfo` docs said the store cursors and
shielded addresses live in memory only; they are restored where the store
attests `privateContactRequestScanState`, with the full read as the
fallback. `privateReplyPending` is restored too once the shielded store is
configured after the load or the wallet binds.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…load

A wallet that bound a shielded account in an earlier process has its
public contact requests refused (code 71) until Rust restores its
unconfirmed private payments, which configuring the shielded store does
with no keys. The app configured it only when binding, which needs the
mnemonic; it now configures the same per-network path right after
`loadFromPersistor`, so the bind's own call is a no-op.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e sync

Code 71 does not mean the wallet holds a reservation for the contact: it
is also returned, with nothing reserved, when the private store could not
be read to its end before the first send or accept since load. The views
dropped the refusal and read the pending state, which then found nothing
and brought Send and Accept back with no notice. `PrivateRepliesPendingSnapshot`
now keeps the refused contact pending, with the private sync offered,
until a private sync completes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
QuantumExplorer added a commit that referenced this pull request Nov 9, 2026
… load

A wallet that bound a shielded account in an earlier process has its
public contact requests refused (code 71) until Rust restores its
unconfirmed private payments, which configuring the shielded store does
with no keys. The app configured it only when binding, which needs the
mnemonic; it now configures the same per-network path right after
`loadPersistedWallets`. Port of the Swift change on #5812.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
QuantumExplorer added a commit that referenced this pull request Nov 9, 2026
…te sync

Code 71 does not mean the wallet holds a reservation for the contact: it
is also returned, with nothing reserved, when the private store could not
be read to its end before the first send or accept since load. Add
Contact's public Send, its collision accept and the Requests rows dropped
the refusal and read the pending state, which then found nothing and
brought the controls back with no notice. `PrivateRepliesPendingSnapshot`
now keeps the refused contact pending, with the private sync offered,
until a private sync completes. Port of the Swift change on #5812.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

At edc9dd1, most prior issues are fixed, but reopening a previously shielded wallet without shielded support still loses the recovery hold and can permit a public reply while an earlier private reply is unresolved. The complete private-sync path also still loses the missing-mnemonic error code. This was static verification only; current CI reports successful Kotlin build/tests and SwiftData explorer checks, with the review check pending and no failed code checks reported.

🔴 1 blocking | 🟡 1 suggestion(s)

2 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: ffi-engineer); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 7: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); 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 cross-language change adds shielded-payment send and accept execution in packages/rs-platform-wallet-ffi/src/private_contact_request.rs and changes SwiftData schema migration in DashModelContainer.swift, directly touching funds movement and storage migrations.
  • 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-ffi/src/private_contact_request.rs`:
- [BLOCKING] packages/rs-platform-wallet-ffi/src/private_contact_request.rs:867-876: Keep unconfirmed private requests pending instead of enabling a fresh send
  (existing thread: https://github.com/dashpay/platform/pull/5812#discussion_r4507400051)
  The load-time recovery hold fixes the before-bind window only in shielded builds, but this new in-kind accept is exported in every build. Without `shielded`, `FFIPersister::load` omits shielded hydration, `ClientStartState` has no shielded field, and manager load never calls `hold_until_private_payments_restored`; the readiness flag therefore remains false and no pending reservation is restored.

  Reopen a persisted wallet containing a public incoming request and an ambiguous private reply whose payment is saved as unconfirmed but has not executed. The preliminary private-store read cannot discover that unexecuted reply, so this branch chooses and broadcasts a public reply. Its code-69 payment closure is never reached because the chosen reply is public. The original private transition can subsequently execute or be redriven, producing both another paid request and the public reciprocal relationship the private operation avoided.

  Preserve recovery-readiness and the necessary non-spending payment metadata across feature configurations in rs-platform-wallet's persistence contract. A build unable to recover an earlier private payment must fail closed rather than treat unavailable recovery state as no pending operation. Add a persisted-unconfirmed-reply fixture exercised without `shielded` and verify that public send and in-kind accept cannot broadcast.

In `packages/rs-platform-wallet-ffi/src/dashpay.rs`:
- [SUGGESTION] packages/rs-platform-wallet-ffi/src/dashpay.rs:1135: Preserve the missing-mnemonic key-availability signal
  (existing thread: https://github.com/dashpay/platform/pull/5812#discussion_r4506333098)
  The variant-aware conversion fixes direct page scans, but an actual private sync with a scan-eligible managed identity first calls `verify_seed_binds`, which invokes `receiving_xpub`. When the resolver returns NOT_FOUND, `extended_public_key` returns `MnemonicResolverSignerError::NotFound`, and this conversion changes it to `InvalidIdentityData`. The exported sync consequently returns ErrorUnknown (99), not ErrorSigningKeyUnavailable (31), before reaching the repaired page-scan conversion. Swift therefore cannot route this failure to key repair.

  Use the shared variant-preserving mapper here. The existing regression invokes `provider.scan_private_document_page` directly and bypasses seed verification; add coverage through wallet sync or the exported sync with a populated wallet and a NOT_FOUND resolver, asserting code 31.

QuantumExplorer and others added 6 commits November 9, 2026 13:53
…tact-requests' into feat/ffi-swift-private-contact-requests

The wallet's `PrivateContactRequestsPosition` gains
`private_payments_unresolved`. Until the scan-state rows carry it, the
load restores it set, which holds the identity until its private payments
are restored from the shielded store; the store-side test literals clear
it. `PrivateContactPaymentsNotRestored` is code 72 now, which the spend
mapper test expects.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…r on the scan-state rows

`PrivateContactRequestsPositionFFI` gains `private_payments_unresolved`
(80 bytes, alignment 8): the store callback writes the wallet's marker
and the load hands it back to `apply_private_contact_requests_position`,
replacing the merge's set-on-load placeholder. Loaded set, the wallet
holds the identity's public contact requests until its private payments
are restored from the shielded store.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… read position

`PersistentPrivateContactRequestsPosition` (live schema V5, unreleased, so
edited in place) gains `privatePaymentsUnresolved`, false by default. The
handler stores the wallet's marker with the identity's read position and
hands it back at load, so a relaunch holds the identity until its private
payments are restored; Storage Explorer shows it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…eadiness

`PlatformWalletError.privateContactPaymentsNotRestored` binds
`ErrorPrivateContactPaymentsNotRestored` (72): the wallet's private
payments from an earlier process are not restored yet, apart from a
contact's pending request (71). `ManagedPlatformWallet.privateContactReadiness(identityId:)`
binds `platform_wallet_get_private_contact_readiness`, so a host reads
whether the payments are restored, whether the private store was read to
its end, and whether a private payment may be unresolved.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…et's readiness

The views cleared every code 71 refusal once a private sync finished,
though a sync releases neither a contact's reservation nor the wallet's
unrestored payments. They now render the wallet's own state:

- code 72 shows "restoring private payments" for every contact, apart
  from a pending request (71), and only a readiness read that says
  restored lifts it; a private sync is not offered for it;
- a code 71 refusal stays pending until the readiness says the private
  store was read to its end, after which the pending read decides;
- the readiness is read with the pending state and again when the
  shielded account binds.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…abilities without a manager

`effective_persistence_capabilities` intersects a declaration with what a
vtable and its extension structurally carry, as manager creation does,
and releases the context. It lets a host adapter test which callbacks
keep a capability; the JNI layer uses it to check its bridge overrides.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
QuantumExplorer added a commit that referenced this pull request Nov 9, 2026
…private contact readiness

`DashSdkError.PlatformWallet.PrivateContactPaymentsNotRestored` binds
`ErrorPrivateContactPaymentsNotRestored` (72), apart from a contact's
pending request (71). `Dashpay.privateContactReadiness` binds
`platform_wallet_get_private_contact_readiness` through a new JNI export
returning its three flags. Port of the Swift bindings on #5812.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
QuantumExplorer added a commit that referenced this pull request Nov 9, 2026
…let's readiness

The screens cleared every code 71 refusal once a private sync finished,
though a sync releases neither a contact's reservation nor the wallet's
unrestored payments. They now render the wallet's own state:

- code 72 shows "restoring private payments" for every contact, apart
  from a pending request (71), and only a readiness read that says
  restored lifts it; a private sync is not offered for it;
- a code 71 refusal stays pending until the readiness says the private
  store was read to its end, after which the pending read decides;
- the readiness is read with the pending state and again when the
  shielded account binds.

Port of the Swift change on #5812.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

At head 27f1a69, two blocking persistence-contract issues remain: non-atomic hosts can lose an unignore rewind, and Swift can acknowledge discarded contacts before saving a cursor beyond them. The 17 actionable prior findings are fixed, the withdrawn finding remains withdrawn, and one new regression-test gap is retained. This was static verification only; current CI shows passing Kotlin build/tests and schema checks, with the review check pending and no Rust or Swift build/test results listed.

🔴 2 blocking | 🟡 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: ffi-engineer); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 7: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 8: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 10: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 11: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 12: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 13: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 14: 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, cross-language diff changes shielded funds movement in private_contact_request.rs and persisted-state migration in DashModelContainer.swift, meeting both the intricacy and critical-surface requirements.
  • 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 final gate: an independent Phase-2 review ran after iterative findings were reconciled
  • 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, 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-ffi/src/persistence.rs`:
- [BLOCKING] packages/rs-platform-wallet-ffi/src/persistence.rs:1804-1809: Require atomic changesets before attesting resumable private scans
  This condition enables PRIVATE_CONTACT_REQUEST_SCAN_STATE without requiring declared ATOMIC_CHANGESETS and the complete begin/end callback pair. Native unignore_contact_sender puts the ignored-sender removal and private-position rewind into one changeset, but FFIPersister::store delivers the contact callback first and the scan-state callback second. A non-atomic host can commit the removal and then crash or fail before storing the rewind. After restart, the sender is unignored while the matching-fingerprint cursor still resumes beyond requests skipped during the ignored period, so ordinary refreshes cannot recover them. Require both the atomic declaration and complete changeset callbacks before attesting resumable scans, and document that dependency. Cover a host declaring scan-state support without atomic rounds and an interrupted unignore that must not commit the sender removal independently of its rewind.

In `packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift`:
- [BLOCKING] packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift:3260: Fail contact persistence before acknowledging resumable scan progress
  Advertising this capability makes the contact callback's successful acknowledgment authoritative for durable scan progress. However, persistContacts uses try? for its owner lookup and logs then continues when the fetch fails or finds no owner; persistContactsCallback subsequently returns 0, and endChangeset can commit an empty round successfully. Native ingestion therefore records the request in memory and persists a cursor beyond an entry whose contact was never saved. After restart, the new durable cursor prevents the previous start-from-zero recovery. Propagate owner-fetch failures, unresolved owners, and contact-staging failures through the callback as a failed round so native ingestion leaves the cursor unchanged. Add a regression that fails the owner lookup, verifies failed ingestion without cursor advancement, and confirms a subsequent successful scan persists the request.

In `packages/rs-platform-wallet-ffi/src/private_contact_request.rs`:
- [SUGGESTION] packages/rs-platform-wallet-ffi/src/private_contact_request.rs:1187-1198: Exercise managed public and adopted replies in the lazy-payment regression
  The fixture addresses an identity the wallet does not manage and asserts IdentityNotFound. It therefore exits in read_private_store_before_deciding before native routing selects public acceptance or adoption. This catches the former up-front FFI coordinator lookup, but would still pass if payment-resource acquisition became eager after identity validation. Add managed-recipient cases through this export: an already-replied request should be adopted successfully without shielded configuration, and a public reply should reach mocked signing/submission rather than fail on shielded-resource acquisition. Exercise both feature configurations and distinguish payment-resource lookups from legitimate mnemonic access for DashPay cryptography.

Comment on lines +1804 to +1809
if wallet_restore
&& self.callbacks.on_persist_contacts_fn.is_some()
&& self
.tracked_masternodes_callbacks
.persist_private_contact_scan_state
.is_some()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Blocking: Require atomic changesets before attesting resumable private scans

This condition enables PRIVATE_CONTACT_REQUEST_SCAN_STATE without requiring declared ATOMIC_CHANGESETS and the complete begin/end callback pair. Native unignore_contact_sender puts the ignored-sender removal and private-position rewind into one changeset, but FFIPersister::store delivers the contact callback first and the scan-state callback second. A non-atomic host can commit the removal and then crash or fail before storing the rewind. After restart, the sender is unignored while the matching-fingerprint cursor still resumes beyond requests skipped during the ignored period, so ordinary refreshes cannot recover them. Require both the atomic declaration and complete changeset callbacks before attesting resumable scans, and document that dependency. Cover a host declaring scan-state support without atomic rounds and an interrupted unignore that must not commit the sender removal independently of its rewind.

source: gpt-6.1-sol (phase2-reviewer: general)

| PlatformWalletPersistenceCapabilities.coreSweepRemoval
| PlatformWalletPersistenceCapabilities.dashpayPayments
| PlatformWalletPersistenceCapabilities.assetLockOperationIds
| PlatformWalletPersistenceCapabilities.privateContactRequestScanState

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Blocking: Fail contact persistence before acknowledging resumable scan progress

Advertising this capability makes the contact callback's successful acknowledgment authoritative for durable scan progress. However, persistContacts uses try? for its owner lookup and logs then continues when the fetch fails or finds no owner; persistContactsCallback subsequently returns 0, and endChangeset can commit an empty round successfully. Native ingestion therefore records the request in memory and persists a cursor beyond an entry whose contact was never saved. After restart, the new durable cursor prevents the previous start-from-zero recovery. Propagate owner-fetch failures, unresolved owners, and contact-staging failures through the callback as a failed round so native ingestion leaves the cursor unchanged. Add a regression that fails the owner lookup, verifies failed ingestion without cursor advancement, and confirms a subsequent successful scan persists the request.

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

Comment on lines +1187 to +1198
// A request to an identity the wallet does not manage.
let request = ContactRequest::new(
[0xA0; 32].into(),
[0xB0; 32].into(),
1,
2,
7,
vec![0x33; 96],
100,
1_700_000_000_000,
);
let request_handle = CONTACT_REQUEST_STORAGE.insert(request);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: Exercise managed public and adopted replies in the lazy-payment regression

The fixture addresses an identity the wallet does not manage and asserts IdentityNotFound. It therefore exits in read_private_store_before_deciding before native routing selects public acceptance or adoption. This catches the former up-front FFI coordinator lookup, but would still pass if payment-resource acquisition became eager after identity validation. Add managed-recipient cases through this export: an already-replied request should be adopted successfully without shielded configuration, and a public reply should reach mocked signing/submission rather than fail on shielded-resource acquisition. Exercise both feature configurations and distinguish payment-resource lookups from legitimate mnemonic access for DashPay cryptography.

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

…tact-requests' into feat/ffi-swift-private-contact-requests

Takes the wallet's code 73 (`ErrorPrivateContactRecoveryUnsupported`) and
its registry row, keeping this branch's code 72 row with the Swift
typing.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants