[QuantumExplorer]

Basic explanation

What this does: Android apps can now sync, send and accept DashPay private contact requests through the Kotlin SDK, over the same platform-wallet FFI the Swift SDK uses. The request is signed by the identity signer the app already uses for public requests. The wallet picks how an accepted request is replied to, and result codes 68 to 71 become typed errors. KotlinExampleApp shows a private request still in flight as pending, as the wallet reports it.

Value: parity with iOS for private contact requests.

Risks: low.

• No consensus change.
• New JNI exports and Kotlin wrappers only; the public contact request bridges are unchanged.

Issue being fixed or feature implemented

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

What was done?

JNI (rs-unified-sdk-jni/src/dashpay.rs)

• syncPrivateContactRequests(walletHandle, coreSignerHandle, maxPages) returns the pass's six counters as a long[]. It destroys every returned request handle, since Kotlin reads the requests from Room.
• sendPrivateContactRequest(...) and acceptPrivateContactRequest(...):
  • built with the shielded feature;
  • the signer crosses as the SignerHandle the public sendContactRequest takes;
  • a negative account, or a shielded address that is not 43 bytes, is refused before anything crosses.

Kotlin SDK

• Dashpay.syncPrivateContactRequests(coreSignerHandle, maxPages) returns a PrivateContactSyncResult.
• Dashpay.incomingContactRequest(ourIdentityId, senderId) (Swift's getIncomingContactRequest(senderId:)) gives the private accept its request.
• PlatformWalletManager.sendPrivateContactRequest(...) and acceptPrivateContactRequest(...) sit with the other shielded spends. They use the manager's resolver and identity signer.
• DashSdkError.PlatformWallet.PrivateContactRequestNeedsPrivateReply is code 68.
• Every new wrapper's KDoc cites PrivateContactRequests.swift.

try {
    dashpay.acceptContactRequest(request, signerHandle, coreSignerHandle)
} catch (e: DashSdkError.PlatformWallet.PrivateContactRequestNeedsPrivateReply) {
    // before: no such error; the public accept would have published the relationship
    // after:  the app replies privately
    manager.acceptPrivateContactRequest(walletId, request)
}

KotlinExampleApp

• DashPaySyncHelpers.acceptIncomingContactRequest falls back to the private accept on code 68, as ContactsView.swift does. Requests and Add Contact's collision accept use it.
• Pull-to-refresh on Requests also reads the private store.
• There is no "Send privately" toggle yet. A public send to a private contact shows the wallet's refusal, as the iOS app does with its toggle off.

Parity

• New capability dashpay.private_contact_requests in docs/sdk/sdk-parity-manifest.json, with the new FFI symbols in shared_symbols.
• Both hosts are partial. A send or accept needs a protocol version 15 network with shielded funds, and no gate is recorded yet.
• PARITY_SUMMARY.md regenerated with scripts/check_sdk_parity_manifest.py --write-summary.

Changes from review

• One accept. The JNI bridges platform_wallet_manager_accept_contact_request_replying_in_kind, exported in every build. KotlinExampleApp accepts only through it, so the wallet picks the reply.

• Pending state read from the wallet.

  • Dashpay.privateContact(ourIdentityId, contactId) reads how a contact's requests came, through managed_identity_get_private_contact.
  • Dashpay.privateRepliesPending(ourIdentityId, contactIds) reads which contacts have a private request in flight, through managed_identity_get_private_reply_pending, from a fresh managed-identity snapshot.
  • The Requests rows and Add Contact's Send and collision accept read that flag instead of a remember set, so leaving and reopening a screen keeps a pending request pending.

• New typed codes:

  • 69 ShieldedBuildRequired: a private reply on a build without shielded.
  • 70 ProtocolVersionNotActive: the network is below protocol version 15.
  • 71 PrivateContactRequestPending: a private request is in flight; the app shows it as pending and offers no new submission.

• Other changes. Private refresh failures are reported. The JNI sync bridge releases real request handles. The parity entry tracks the new symbols.

• Faster private sync, host side. The JNI wires the new scan-state persistence slots, and Room goes to version 16 (MIGRATION_15_16, two tables, a new DAO). The handler declares the scan-state capability, stages the writes into the changeset round and purges them with the wallet, so a relaunch resumes the private store read.

• Pending state read failures. A failed pending-state read is kept apart from "nothing pending" (PrivateRepliesPendingSnapshot keeps the last good one).

• Docs. The in-kind accept KDoc is attached to its declaration.

second accept while the first private reply is unconfirmed
  before: a screen-local set kept Accept off until the screen closed
  after:  the wallet returns 71 and privateRepliesPending keeps Accept off until a private sync settles it

How Has This Been Tested?

• cargo clippy -p rs-unified-sdk-jni --all-targets -- -D warnings, with default features and --no-default-features: clean.
• cargo test -p rs-unified-sdk-jni --lib -- dashpay::tests: 5 passed (counter order, saturating position, empty array release, real handle release, the private contact JSON layout).
• Gradle on the JBR runtime: :sdk:compileDebugKotlin, :sdk:compileDebugAndroidTestKotlin, :app:compileDebugKotlin, and :sdk:testDebugUnitTest --tests '*DashSdkErrorTest*' (29 passed, including codes 68 to 71), and ContactRequestOutcomesTest (7 passed, including 71 as pending and the row message).
• python3 scripts/check_sdk_parity_manifest.py: passes.
• Not run locally: FfiSmokeTest (the private contact request symbols, and the private contact and pending reads) needs an emulator; the instrumented CI job runs it.

Breaking Changes

None.

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/kotlin-private-contact-requests
• File: .coderabbit.yaml
• Commit: 2abc9b5
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: a86becfc-3478-4cf0-9117-ef54c98e9ef8

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 — no blockers (commit 2abc9b5) · triage: critical

[thepastaclaw]

Final validation — Phase 1 + Phase 2

Verified the findings against head 655d1e3. The acceptance fallback introduces wallet orchestration in the Kotlin example app contrary to the repository's ownership rules; native-handle cleanup also lacks nonempty regression coverage. This was a static review with no local builds or tests; current CI reports the Kotlin SDK build and emulator tests passing, with only the automated review pending.

🔴 1 blocking | 🟡 1 suggestion(s)

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: ffi-engineer); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: normal by gpt-6.1-sol (effort low) — The change spans JNI, Kotlin wrappers, and example-app flows, but delegates private sends, signing, and acceptance to existing platform-wallet implementations rather than changing intricate funds-movement or cryptographic logic.
• 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)
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — ffi-engineer (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort high); 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/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/dashpay/DashPaySyncHelpers.kt`:
- [BLOCKING] packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/dashpay/DashPaySyncHelpers.kt:73-76: Move public-to-private acceptance orchestration into platform-wallet
  This catch re-reads the incoming request and automatically switches the Accept action from a refused public operation to a shielded-funded private operation. Rust enforces the privacy refusal, but the host still owns the subsequent workflow. The book's "Where code goes" section explicitly assigns mobile DashPay orchestration to rs-platform-wallet, and "FFI and mobile SDKs hold no business logic" requires hosts to call an existing Rust operation rather than make wallet decisions. Copying the Swift example's fallback adds another host-owned implementation; that existing implementation does not exempt the new code from the rule. Add a Rust acceptance entry point with the required spending context, resolve the incoming request and select the public/private route there, then expose it through FFI/JNI so this helper makes one call. Keep the explicitly public API's refusal. Test the unified workflow offline at its Rust owner, including public success, private acceptance, unrelated-error propagation, missing requests, and already-reciprocated requests; the current error-mapping and JNI-binding tests do not exercise this routing.

In `packages/rs-unified-sdk-jni/src/dashpay.rs`:
- [SUGGESTION] packages/rs-unified-sdk-jni/src/dashpay.rs:1520-1525: Exercise cleanup of populated request-handle arrays
  The only cleanup test supplies ContactRequestHandleArray::empty(), so neither the per-handle destruction loop nor the allocated-buffer release executes. The FFI array-free function deliberately frees only the buffer: individual requests remain in CONTACT_REQUEST_STORAGE until contact_request_destroy removes them. Removing the destruction loop would therefore leak requests on every nonempty private sync while the new tests still passed. Add a JVM-independent test that creates at least two real handles through contact_request_create, puts them in a boxed-slice allocation matching the FFI array contract, and invokes release_contact_request_handles. Assert the returned count, null pointer and zero count afterward, and that getters reject both released handles. Use real registered handles rather than fabricated values so the test verifies destruction as well as buffer cleanup.

[thepastaclaw]

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

The two prior findings are fixed: populated request-array cleanup is covered, and public/private reply selection now belongs to platform-wallet. One blocking regression remains: both example-app acceptance flows unconditionally invoke a shielded-only JNI symbol, breaking public acceptance on the documented transparent-only build. This was a static verification at 13f4784; current CI reports the Kotlin build/emulator job passing with shielded enabled, policy checks passing, and the review check pending.

🔴 1 blocking

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: ffi-engineer); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: normal by gpt-6.1-sol (effort low) — The cross-language change is substantial, but it adds JNI/Kotlin adapters to existing private-contact operations rather than changing funds-movement, cryptographic, signature, or consensus logic itself.
• 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)
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — ffi-engineer (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort high); 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/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/dashpay/DashPaySyncHelpers.kt`:
- [BLOCKING] packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/dashpay/DashPaySyncHelpers.kt:66-67: Preserve public acceptance on transparent-only builds
  Requests and Add Contact now route every acceptance through this call, including ordinary public requests. However, `DashpayNative.acceptContactRequestReplyingInKind` has its JNI export behind `#[cfg(feature = "shielded")]` in `rs-unified-sdk-jni/src/dashpay.rs:1497`, and the underlying FFI export is similarly gated. The Kotlin README explicitly supports `./build_android.sh --no-shielded`, which disables that JNI feature. On that build, accepting a public request previously used the available public bridge but now throws `UnsatisfiedLinkError`; neither `mapNativeErrors` nor the screens' `catch (e: Exception)` handlers catches it. Make unified acceptance available without shielded support, retaining route selection in platform-wallet and returning a typed unsupported error only when a private reply requires the missing capability. Add public-acceptance coverage against the transparent-only native library: the passing emulator job builds with shielded enabled, while a Rust no-default-features compile cannot detect this missing-symbol failure at runtime.

[thepastaclaw]

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

Verified the findings against head 89d2330. All three prior findings are fixed, but one blocking issue remains: screen-local pending state allows an uncertain private acceptance to initiate another paid reply after navigation or through another screen. This was a static review; no local builds or tests were run, and the current Kotlin emulator build/test check passes.

🔴 1 blocking

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: ffi-engineer); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: normal by gpt-6.1-sol (effort low) — The cross-language feature is substantial but primarily marshals existing private-contact operations and handles application outcomes, without changing the underlying funds movement, cryptography, or signing algorithms.
• 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)
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — ffi-engineer (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort high); 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/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/dashpay/ContactRequestsScreen.kt`:
- [BLOCKING] packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/dashpay/ContactRequestsScreen.kt:95-98: Retain unresolved private acceptances in wallet-owned state
  After `ShieldedSpendUnconfirmed`, this `remember` set prevents another acceptance only while this composition survives. Leaving and reopening Requests clears it, and Add Contact's separate `unconfirmedNotice` is not shared with Requests. The incoming-only row therefore offers Accept again before the first submission is reconciled.

  The Rust path does not prevent the second payment: `send_private_contact_request` returns at `pay(entry).await?` before recording an outgoing request, while `needs_private_reply` and `accept_contact_request_replying_with` recognize only recorded outgoing or established requests. `reserve_notes_covering` excludes the original spend's reserved notes but allows another spend from other available notes. A second acceptance can consequently seal a different entry and pay another fee while the first transition can still land. `kickDashPaySync` runs the ordinary DashPay sweep, which does not read the private request store; the native shielded-sync poke likewise does not establish the missing contact-level pending state.

  Retain the unresolved reply in `rs-platform-wallet`, keyed by wallet and identity pair and associated with its pending spend. Subsequent accepts must return the pending outcome rather than build another reply until reconciliation establishes whether replacement is safe. Expose that Rust-owned status to both screens instead of relying on their lifetimes. Add owner-level coverage where an ambiguous first payment is followed by another acceptance before private-store reconciliation, including reconstruction/re-entry, and assert that the payment callback runs only once. This PR introduces private acceptance into both screens and adds this local guard, so the gap is in scope; wallet-level retry decisions also belong in Rust under the book's mobile-orchestration ownership rule.

[thepastaclaw]

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

Verified head 2890ed8: wallet-owned acceptance routing, transparent-only exports, and populated-handle cleanup coverage address three prior findings. One blocking issue remains: process restart loses the contact-level reservation for an unresolved private reply, allowing another fee-paying acceptance before reconciliation. This was a static review with no local builds or tests; the current Kotlin SDK build and emulator check passed.

🔴 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: ffi-engineer); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: normal by gpt-6.1-sol (effort low) — The substantial cross-language change adds JNI and Kotlin orchestration for existing wallet operations rather than changing funds movement algorithms, cryptography, signing rules, or consensus itself.
• 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)
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — ffi-engineer (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort high); 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/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/dashpay/ContactRequestsScreen.kt`:
- [BLOCKING] packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/dashpay/ContactRequestsScreen.kt:165-169: Retain unresolved private acceptances in wallet-owned state
  (existing thread: https://github.com/dashpay/platform/pull/5813#discussion_r4507144191)
  The fresh wallet snapshot fixes screen re-entry, but it reports no pending reply after process restart. `DashPayState.pending_private_replies` is memory-only, and `Dashpay.privateRepliesPending` explicitly documents the reset at `Dashpay.kt:400–407`. Restoring the persisted incoming request does not restore this reservation, so Requests offers Accept again while Rust's `needs_private_reply` still sees no outgoing counterpart.

  This can duplicate fees even when the first payment was confirmed: `send_reserved_private_contact_request` completes `pay(entry).await?`, fails to read the entry back from the private store, and returns `PrivateContactRequestPending` without recording an outgoing request. If the process then restarts before successful private-store reconciliation, another acceptance reserves the now-empty recipient slot, seals a fresh entry, and pays again. The ambiguous `ShieldedSpendUnconfirmed` path has the same gap while the original signed spend remains recoverable. Shielded note reservations protect only the selected notes; `reserve_notes_covering` can fund the second submission from other available notes. The private store is an append-only log, not contact-level deduplication.

  The #5810 restoration work must be consumed here before this newly exposed acceptance flow is safe across process death. Persist the wallet/identity/recipient reservation and its entry/spend association before broadcast, restore it before pending reads and submissions become available, and retain it until authoritative reconciliation establishes settlement or safe replacement. Keep Kotlin as a consumer of that Rust-owned status. Add owner-level restart coverage for both an ambiguous payment and a successful payment followed by failed read-back, asserting that another acceptance does not invoke payment again.

[thepastaclaw]

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

Verified the findings against head e050ab0. All four prior findings are fixed, but the new pending-state presentation helper still converts failed reads into an apparent absence of pending requests; a separate KDoc placement nitpick also remains. This was static verification only: the current Kotlin SDK build and emulator tests pass, and the review check is pending.

🔴 1 blocking | 💬 1 nitpick(s)

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: ffi-engineer); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: normal by gpt-6.1-sol (effort low) — The cross-language implementation is substantial, but its JNI exports, Kotlin wrappers and UI state handling delegate signing and private transfers to existing wallet functionality rather than changing funds movement, cryptography or key-handling logic themselves.
• 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)
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — ffi-engineer (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort high); 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/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/dashpay/DashPaySyncHelpers.kt`:
- [BLOCKING] packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/dashpay/DashPaySyncHelpers.kt:56-57: Preserve unavailable pending state instead of reporting no pending requests
  This catch makes an operational read failure indistinguishable from a successful snapshot containing no pending replies. The SDK already returns an empty result for an unmanaged identity while propagating other failures, and accessing a closed ManagedPlatformWallet throws. If a pending-state refresh fails, ContactRequestsScreen replaces walletPendingIds with this empty set, removing the pending notice and restoring Accept/Ignore; AddContactScreen similarly sets recipientPending to false and enables Send. No successful reconciliation established that the pending submission cleared. Rust's reservation and recovery guards prevent another private payment, but they do not make this report of wallet state correct. Preserve a distinct unavailable/error result, retain the last successful pending snapshot, and keep pending-sensitive controls unavailable until a successful read. Add coverage for a successful pending snapshot followed by a failed read, separately from a successful empty snapshot.

In `packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/DashpayNative.kt`:
- [NITPICK] packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/DashpayNative.kt:331-338: Attach the in-kind acceptance KDoc to its declaration
  This acceptance documentation is immediately followed by another KDoc block and managedIdentityPrivateContact, while acceptContactRequestReplyingInKind appears below both managed-identity reads without its documentation. Move this block directly above the acceptance declaration's @Suppress annotation so its requestHandle/account links and reply-routing explanation document the intended API.

[thepastaclaw]

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

All six prior findings are addressed at head 31d393e. One blocking persistence-boundary issue remains, along with three non-blocking issues involving code-71 feedback, sync documentation, and callback test coverage. Static verification confirmed these against the source; the current Kotlin SDK build and emulator tests pass, and no local builds or tests were run.

🔴 1 blocking | 🟡 3 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: 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) — The large cross-language change directly modifies storage migrations in DashDatabase.kt through MIGRATION_15_16 and adds native persistence callbacks for private contact scan state and shielded addresses.
• 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/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandler.kt`:
- [BLOCKING] packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandler.kt:4664-4668: Fail scan-state loading instead of silently dropping saved addresses
  This filter converts malformed persisted addresses into a successful partial restore. Room accepts arbitrary blob lengths, so a saved 42-byte address can be omitted while packPrivatePositions returns its identity's valid cursor and key fingerprint. The FFI loader then restores that cursor successfully; with unchanged scanning keys and one scanning identity, subsequent private sync starts at the retained position rather than revisiting the entry containing the address. The contact consequently reports no shielded address instead of reporting unreadable persisted state. Silently selecting partial recovery also exceeds the adapter's marshalling role under the book's "FFI and mobile SDKs hold no business logic" rule. Reject malformed stored rows through the existing loadOrThrow/JNI load-error path. Any automatic cursor invalidation and reconstruction policy belongs in rs-platform-wallet. Add a test combining a valid saved position with a malformed address row and assert that loading fails rather than returning a partial snapshot.

In `packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/dashpay/AddContactScreen.kt`:
- [SUGGESTION] packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/dashpay/AddContactScreen.kt:294-295: Preserve code-71 feedback when no reply reservation exists
  PrivateContactRequestPending does not guarantee that privateRepliesPending contains this contact. Rust's read_private_store_before_sending also returns code 71 when its startup store scan cannot finish, before creating a reservation; should_refuse_a_private_send_until_the_store_reads_to_its_end explicitly verifies that outcome with nothing reserved. Both unified acceptance and public sending pass through this scan gate. Here, the Unit arm discards the refusal, and a subsequent successful empty pending read enables submission without showing a notice or Add Contact's private-sync button. The public Send branch and ContactRequestsScreen make the same assumption. Preserve visible refusal feedback and a private-refresh path independently of whether a contact-level reservation exists. If distinct reasons are needed, expose them from platform-wallet rather than inferring them in Kotlin. Add coverage for code 71 followed by a successful empty pending snapshot.

In `packages/rs-unified-sdk-jni/src/persistence.rs`:
- [SUGGESTION] packages/rs-unified-sdk-jni/src/persistence.rs:1120-1126: Include scan-state callbacks in the JNI descriptor smoke check
  Neither onPersistPrivateContactScanState nor onLoadPrivateContactScanState appears in BRIDGE_METHOD_TABLE, so persistenceBridgeDescriptorsAllResolve does not check these new descriptors against the compiled Kotlin methods. The added Rust unit test compares the constants with manually supplied type names and cannot detect Kotlin signature drift. A mismatch would make bridge_overrides return false and disable the scan-state callbacks and capability, allowing the existing smoke check to pass while relaunches lose cursor persistence. Add both callback names to BRIDGE_METHOD_TABLE using these descriptor constants so the existing emulator test verifies the actual Kotlin signatures.

In `packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/tokens/Dashpay.kt`:
- [SUGGESTION] packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/tokens/Dashpay.kt:352-355: Document capability-backed private scan-state persistence
  This KDoc still says that store cursors are memory-only and every launch scans from the beginning. This PR adds Room v16 persistence and JNI load/store callbacks for PRIVATE_CONTACT_REQUEST_SCAN_STATE, and the Rust loader restores the saved cursor when the backend attests that capability. Update the documentation to distinguish capability-backed restoration from the fallback full scan when the capability is unavailable, so SDK consumers receive an accurate description of relaunch and sync behavior.

[thepastaclaw]

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

At head 4763d41, the ten prior findings are addressed, but the new code-71 recovery mechanism incorrectly infers wallet readiness from private-document sync completion, violating the Rust-owned orchestration boundary. Two additional test-coverage suggestions remain. This was a static review without local builds or tests; current Kotlin emulator and policy checks pass, with the review check pending.

🔴 1 blocking | 🟡 2 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 cross-language persistence changes are intricate and directly introduce a storage migration in packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/DashDatabase.kt together with private-contact scan-state persistence and restoration.
• 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/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/dashpay/ContactRequestOutcomes.kt`:
- [BLOCKING] packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/dashpay/ContactRequestOutcomes.kt:95-99: Keep code-71 readiness and recovery decisions in platform-wallet
  The app clears every code-71 refusal after a private-document sync, although that operation cannot resolve every condition represented by code 71. In `rs-platform-wallet-ffi/src/error.rs`, `PrivateContactPaymentsNotRestored` also maps to 71. That condition is a wallet-wide `WalletPersister` hold, not a contact reservation, so `privateRepliesPending` can successfully return an empty set while the hold remains.

  After shielded-store configuration or saved-payment restoration fails, the app can still expose the loaded wallet. A subsequent private-document sync can succeed independently: `sync_private_contact_requests` does not release the restoration hold; `restore_private_contact_payments` does. Both screens nevertheless call `afterPrivateSync()` when the refresh produces no notice, discard the refusal, and enable submission after an empty pending read. The pending notice disappears without authoritative recovery, and the next submission encounters the same 71. Rust still prevents an unsafe send, but the app has reconstructed an unsupported readiness conclusion.

  The book assigns mobile DashPay orchestration to `rs-platform-wallet` and requires hosts to marshal rather than decide. Expose authoritative readiness from that owner, distinguishing contact reservations, incomplete private-store scans, and unrestored payments; marshal it through FFI/JNI and render it in the screens instead of deciding that sync releases every refusal. Add owner-level coverage showing that private-document sync does not release the payment-restoration hold and successful restoration does.

In `packages/kotlin-sdk/sdk/src/androidTest/kotlin/org/dashfoundation/dashsdk/FfiSmokeTest.kt`:
- [SUGGESTION] packages/kotlin-sdk/sdk/src/androidTest/kotlin/org/dashfoundation/dashsdk/FfiSmokeTest.kt:114-124: Gate shielded-only symbol probes without skipping universal acceptance
  The README supports a transparent-only native library built with `./build_android.sh --no-shielded`, but these two JNI exports are deliberately guarded by `#[cfg(feature = "shielded")]`. Against that supported library, the assertions receive `UnsatisfiedLinkError`, not `DashSDKException`, and the test stops before checking the always-exported in-kind acceptance symbol. Gate only the explicit private-send and private-accept probes on `Sdk.hasShielded()`, or move them into a shielded-only test. Keep sync, managed-identity reads, and in-kind acceptance exercised in every build.

In `packages/rs-unified-sdk-jni/src/persistence.rs`:
- [SUGGESTION] packages/rs-unified-sdk-jni/src/persistence.rs:5413-5420: Exercise the scan-state override checks rather than supplying their result
  This test supplies `private_contact_scan_state: wired` directly to `extension_for`, so it checks slot assignment after the decision has already been made. It does not exercise the three `bridge_overrides` checks in `build_extension`: removing the contact-upsert requirement or accepting an inherited scan-state callback would leave it passing. Those checks protect the invariant that a durable cursor must not skip requests an inherited no-op contact writer never stored. The descriptor smoke test checks method resolution, not concrete overrides. Add JVM-backed cases declaring the capability while omitting each required override in turn, and verify that the effective capability is absent; also verify that a fully implemented bridge retains it.

[thepastaclaw]

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

Verified the complete diff at 7bc3a03 and confirmed that all 13 prior findings are addressed. Two documentation-placement nitpicks remain; no blocking defects were confirmed. This was a static review with no local builds or tests; the Kotlin SDK build and x86_64 emulator CI passed on the exact reviewed head.

💬 2 nitpick(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 change is large and cross-language, and DashDatabase.kt directly adds the v15-to-v16 storage migration for persisted private-contact scan positions and shielded addresses.
• 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/kotlin-sdk/sdk/src/androidTest/kotlin/org/dashfoundation/dashsdk/FfiSmokeTest.kt`:
- [NITPICK] packages/kotlin-sdk/sdk/src/androidTest/kotlin/org/dashfoundation/dashsdk/FfiSmokeTest.kt:157-163: Move the descriptor-lockstep KDoc back to its test
  This block describes persistenceBridgeDescriptorsAllResolve, but the new capability test and its own KDoc now separate it from that declaration. Move this block directly above persistenceBridgeDescriptorsAllResolve so the descriptor-resolution explanation documents the intended test rather than preceding an unrelated capability test.

In `packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/tokens/Dashpay.kt`:
- [NITPICK] packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/tokens/Dashpay.kt:434-447: Attach the pending-replies KDoc to privateRepliesPending
  This block describes privateRepliesPending, including its contactIds argument and pending-payment restoration behavior, but privateContactReadiness and its own KDoc now intervene before that method. Move the block directly above privateRepliesPending so consumers see the pending-reply contract on the correct API and its parameter links resolve against the intended declaration.

[thepastaclaw]

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

At head 2abc9b5, all 15 prior findings are fixed, and reply routing, payment reservations, and recovery readiness remain Rust-owned. One non-blocking error-propagation gap remains in the newly exposed explicit private-send and private-accept APIs. Verification was static only; the current Kotlin SDK build and x86_64 emulator check passes.

🟡 1 suggestion(s)

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: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); 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) — This large, cross-language change directly modifies storage migrations in DashDatabase.kt and persistent private-contact scan state handling in PlatformWalletPersistenceHandler.kt and rs-unified-sdk-jni/src/persistence.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 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/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/PlatformWalletManager.kt`:
- [SUGGESTION] packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/PlatformWalletManager.kt:2173-2174: Preserve code 73 through the explicit private request exports
  With shielded support configured and an account bound, a persistence backend lacking the required scan-state/atomic capability causes the Rust private-send path to return PrivateContactRecoveryUnsupported. Both explicit send and private-accept FFI exports pass that error through map_spend_result in rs-platform-wallet-ffi/src/shielded_send.rs, which preserves codes 71 and 72 but lacks an arm for this variant. It therefore returns ErrorWalletOperation (6), so these new Kotlin APIs receive PlatformWallet.WalletOperation instead of PrivateContactRecoveryUnsupported (73). The in-kind accept preserves 73 through its pre-payment blanket conversion, leaving sibling APIs inconsistent. Preserve PrivateContactRecoveryUnsupported in the owning FFI mapper and add a regression assertion against map_spend_result; the Kotlin mapping test constructs code 73 directly and cannot detect its loss before JNI.

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.

• Release discarded request handles in the existing public-sync bridges — The unchanged syncContactRequests and fetchSentContactRequests JNI exports free only their returned array buffers. ContactRequestHandleArray::from_requests registers each request in CONTACT_REQUEST_STORAGE, and the array-free function deliberately leaves those handles alive. Because these bridges return no handles to Kotlin, repeated nonempty calls accumulate unreachable native allocations. This behavior exists unchanged at the base revision; the new private-sync bridge correctly destroys its handles.
  • Follow-up: Address both existing public-sync bridges in a separate follow-up PR by destroying every discarded request handle before freeing the array, with populated-array cleanup coverage.

[thepastaclaw]

🟡 Suggestion: Preserve code 73 through the explicit private request exports

With shielded support configured and an account bound, a persistence backend lacking the required scan-state/atomic capability causes the Rust private-send path to return PrivateContactRecoveryUnsupported. Both explicit send and private-accept FFI exports pass that error through map_spend_result in rs-platform-wallet-ffi/src/shielded_send.rs, which preserves codes 71 and 72 but lacks an arm for this variant. It therefore returns ErrorWalletOperation (6), so these new Kotlin APIs receive PlatformWallet.WalletOperation instead of PrivateContactRecoveryUnsupported (73). The in-kind accept preserves 73 through its pre-payment blanket conversion, leaving sibling APIs inconsistent. Preserve PrivateContactRecoveryUnsupported in the owning FFI mapper and add a regression assertion against map_spend_result; the Kotlin mapping test constructs code 73 directly and cannot detect its loss before JNI.

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