Skip to content

feat(wasm-sdk): seal and scan DashPay private contact requests - #5811

Open
QuantumExplorer wants to merge 23 commits into
feat/sdk-private-documents-send-and-scanfrom
feat/wasm-sdk-private-contact-requests
Open

QuantumExplorer wants to merge 23 commits into
feat/sdk-private-documents-send-and-scanfrom
feat/wasm-sdk-private-contact-requests

Conversation

@QuantumExplorer

Copy link
Copy Markdown
Member

Basic explanation

What this does: JavaScript apps can do three new things:

  • read a page of a private document type's store;
  • seal a DashPay private contact request, signed with the same IdentitySigner they sign transitions with;
  • scan DashPay's private store for the requests a wallet received or sent.

These are thin wasm-sdk bindings over the Rust SDK calls in #5809, exposed in evo-sdk as sdk.privateDocuments.

Value: web wallets can receive private contact requests and check who sent them, and can prepare one to send.

Risks: low. These are new bindings only, with no consensus change. One limitation: appending a sealed entry needs an Orchard proof, which the wasm build cannot make yet (the shielded prover is not in the wasm build). A web app can seal a request, but a wallet that can prove has to append it.

Issue being fixed or feature implemented

Part of anonymous DashPay contact requests for 5.1, on top of the SDK's send, scan and read calls.

What was done?

wasm-sdk (src/private_contact_requests.rs)

  • getPrivateDocumentEntries(dataContractId, documentTypeName, startPosition) returns a proved page: { entries: [{ position, blockHeight, blockTimeMs, data }], totalCount }.

  • sealPrivateContactRequest(options) takes:

    • the sender identity, its encryption key id and that key's private key;
    • a signer: IdentitySigner holding one of the sender's signing keys (an enabled AUTHENTICATION key at HIGH or CRITICAL);
    • the recipient identity and key id;
    • the request fields.

    It returns the 589-byte entry.

  • scanPrivateContactRequests(options) takes a start position, the wallet's recipient identities with their keys, and its sender identities with their encryption keys ({ keyId, privateKey }). It returns one page:

    • received: each checked against its sender's signing keys and the recipient's key;
    • sent: recovered, and checked to be the identity's own;
    • rejected: with a reason;
    • nextPosition, totalCount and isComplete.
  • The private keys read from the options are wiped once used.

  • TypeScript interfaces for the options and results sit in a custom section.

const entry = await sdk.privateDocuments.sealContactRequest({
  senderIdentity: alice,
  senderEncryptionKeyId: 1,
  senderEncryptionPrivateKey: aliceEncryptionKey,
  signer: aliceSigner, // the IdentitySigner Alice signs her transitions with
  recipientIdentity: bob,
  recipientKeyId: 2,
  request: { accountReference, extendedPublicKey },
});
// before: no binding
// after:  a 589-byte Uint8Array

const scan = await sdk.privateDocuments.scanContactRequests({
  startPosition: 0n,
  recipients: [{ identityId: bob, keys: [{ keyId: 2, privateKey: bobDecryptionKey }] }],
  senders: [{ identityId: bob, encryptionKeys: [{ keyId: 1, privateKey: bobEncryptionKey }] }],
});
// before: no binding
// after:  { received: [{ senderId: alice, senderKeyId: 1, request: { accountReference, extendedPublicKey, ... } }],
//           sent: [], rejected: [], nextPosition: 1n, totalCount: 1n, isComplete: true }

evo-sdk

  • PrivateDocumentsFacade (sdk.privateDocuments) has entries, sealContactRequest and scanContactRequests.
  • A unit spec checks each forwards to its wasm method.

How Has This Been Tested?

  • cargo check -p wasm-sdk --target wasm32-unknown-unknown and cargo clippy -p wasm-sdk --target wasm32-unknown-unknown -- -D warnings: clean.
  • The logic is tested in the SDK (22 tests).
  • The evo-sdk facade spec is added. It was not run locally, because this worktree has no node_modules. CI 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

QuantumExplorer and others added 3 commits November 8, 2026 11:09
… contact requests

wasm-sdk binds the SDK's private document and DashPay private contact
request helpers:

- `getPrivateDocumentEntries(dataContractId, documentTypeName,
  startPosition)`: a proved page of a private type's store.
- `sealPrivateContactRequest(options)`: the entry, sealed from the
  sender's encryption key to one of the recipient's DashPay keys.
- `scanPrivateContactRequests(options)`: one page of DashPay's store for
  the wallet's identities: received requests with their senders checked,
  sent requests recovered with the identities' encryption keys, and the
  entries refused, with the cursor to scan from next.

evo-sdk exposes them as `sdk.privateDocuments.entries`,
`sealContactRequest` and `scanContactRequests`. Appending a sealed entry
needs an Orchard proof, which this build cannot make yet.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`sealPrivateContactRequest` takes the `signer: IdentitySigner` the sender
signs its transitions with, which signs the request with the first of the
sender's signing keys it holds. The encryption key's private key is now
`senderEncryptionPrivateKey`.

`scanPrivateContactRequests` takes each sender's encryption keys with their
ids (`encryptionKeys: [{ keyId, privateKey }]`), which a sent entry must
name. Sent results carry `senderKeyId`, and the reasons `notSigned` and
`recipientKeyNotUsable` are mapped. Private keys read from the options are
wiped once used.

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 in progress — actively reviewing now (commit 956e962) · triage: critical
█████░░░░░░░░░░░░░░░ 25% · about 15 min left · running for 5 min
✅ triage → ⏳ Phase 2 (4/5 lanes) → ⏳ Phase 1 (1/2 lanes) → ▫️ verify 2 → ▫️ publish
Estimated from recent reviews of this tier · updated 06:19 UTC · live progress

@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

Verified the findings against head 888068a. Two blocking cold-start defects remain: uncached custom contracts cannot verify private-document pages, and private contact operations can resolve DashPay before discovering protocol version 15. This was a static review; current CI contains successful policy/review checks but no build or test results, and the PastaClaw review is pending.

🔴 2 blocking | 🟡 4 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 adds substantial WASM marshalling and facade code, including private-key conversion and cleanup in private_contact_requests.rs, but delegates cryptographic and signature decisions to existing SDK calls rather than introducing intricate critical-surface 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/wasm-sdk/src/private_contact_requests.rs`:
- [BLOCKING] packages/wasm-sdk/src/private_contact_requests.rs:452-455: Load the contract before fetching a private document page
  The first query for an uncached custom contract fails even when the server returns a valid proof. PrivateDocumentEntries::FromProof requires provider.get_data_contract() to supply the contract, while the trusted and verified WASM providers consult their known-contract caches and compiled system contracts rather than fetching missing custom contracts. Neither this binding nor the delegated Fetch implementation loads that dependency. Call the existing get_or_fetch_contract helper before fetching the page, as the other contract-dependent bindings do, and cover an initial query for an uncached non-system contract.
- [BLOCKING] packages/wasm-sdk/src/private_contact_requests.rs:506-508: Discover protocol support before resolving the private DashPay type
  With no persisted protocol version or cached DashPay override, an unpinned mainnet/testnet SDK starts at version 13. Sdk::seal_private_contact_request immediately resolves DashPay through the context at that version, obtaining a contract without privateContactRequest; signing-key lookup then fails without any network request that could discover version 15. Sdk::scan_private_contact_requests captures the same old contract before its page fetch discovers protocol support, then passes that stale contract to private_contact_requests_from_scan, so the first scan fails too. Locally held identity wrappers make sealing reachable without an earlier proved read, and scanning needs no earlier identity fetch. Fix the ordering in the owning Rust SDK methods by requiring/discovering protocol version 15 before resolving DashPay, following fetch_decision_proposals_contract, and add cold-start coverage for both operations.
- [SUGGESTION] packages/wasm-sdk/src/private_contact_requests.rs:496-504: Make temporary secret-key cleanup cover every exit
  After encryption_secret_key is constructed, failures parsing signer, recipientIdentity, recipientKeyId, or request return through ? without reaching non_secure_erase(). The recipient parser has the same cleanup gap at lines 395–406: an invalid later element drops previously collected SecretKey values before the erase loop runs. The pinned secp256k1 0.33.1 SecretKey is Copy and has no erasing destructor, so ordinary cleanup leaves these binding-owned scalar temporaries unwiped. This is defense in depth rather than an exposed-key exploit, but it contradicts the stated cleanup guarantee. Use an owning cleanup guard for temporary scalars and partially constructed collections; parsing all other seal options before deriving the scalar also shortens its lifetime. Preserve the caller-owned PrivateKey objects.
- [SUGGESTION] packages/wasm-sdk/src/private_contact_requests.rs:522-525: Expose the scan's identity-lookup privacy warning
  The delegated scanner immediately fetches the identities needed to authenticate entries that opened for the wallet, including their senders and the wallet's recipient/sender identities. A node serving both the page and those lookups can correlate them, revealing information the encrypted store alone does not disclose. The native SDK explicitly documents this tradeoff and offers a lower-level scan/read path with caller-supplied identities, but neither this method's documentation nor the evo-sdk facade carries that warning. Document the correlation risk on both public surfaces and make clear that this convenience API performs those lookups automatically; the native mitigation is not currently exposed by these bindings.
- [SUGGESTION] packages/wasm-sdk/src/private_contact_requests.rs:346-357: Reuse the existing key-option and fixed-byte conversion helpers
  The new helpers duplicate conversions already available in the project: encrypted_for::secret_key_from_options performs the same PrivateKey wrapper extraction and scalar conversion, and wasm_dpp2::utils::try_to_fixed_bytes performs the same Uint8Array-to-array conversion. The local fixed_bytes helper already introduces different length-error wording. Reuse the shared fixed-byte helper and the existing key-option helper at the option-reading call sites, or extract a shared wrapper-to-secret conversion if both forms are needed. Keep PrivateKey wrapper semantics intact rather than replacing extraction with a raw-byte parser. This is a helper-reuse suggestion, not a business-logic ownership violation.

In `packages/js-evo-sdk/tests/unit/facades/private-documents.spec.ts`:
- [SUGGESTION] packages/js-evo-sdk/tests/unit/facades/private-documents.spec.ts:19-28: Test the actual WASM option and result conversions
  These tests stub all three new WASM methods, so they verify facade forwarding but never execute the Rust binding this PR adds. Native SDK tests likewise start after JavaScript options have been converted. Add local, deterministic binding-level tests using real PrivateKey, Identity, and IdentitySigner wrappers, plus representative nonempty page and scan fixtures. Assert optional request fields, rejection mapping, malformed fixed-size inputs, Uint8Array outputs, and exact bigint positions/counts above Number.MAX_SAFE_INTEGER. Keep the existing tests as the separate facade-forwarding contract.

QuantumExplorer and others added 2 commits November 8, 2026 22:05
…ry exit

Review fixes (with the SDK changes of #5809 merged in, which also resolve
DashPay only once protocol version 15 is established):

- `getPrivateDocumentEntries` loads the contract (`get_or_fetch_contract`)
  before fetching a page, as the page's proof is checked against it and
  the context provider holds a custom contract only once loaded.
- Secret keys read from the options are held in a guard that wipes them
  when dropped, so an error parsing a later option wipes them too. The
  seal reads the encryption key after every other option.
- The scan's docs, and the evo-sdk facade's, say it fetches the identities
  the page's requests are checked against right after the page, which a
  node serving both can correlate.
- The shared `secret_key_from_options` and `try_to_fixed_bytes` replace
  the local helpers.

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

Five prior findings are fixed, but a retained pre-upgrade DashPay contract can still prevent the new sealing and scanning APIs from working after protocol discovery. The binding-conversion coverage suggestion also remains open. This was a static review of head 9d347a5; current CI shows no build/test results or failed code checks, and the review check remains pending.

🔴 1 blocking | 🟡 1 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: rust-quality); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 6: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 7: gpt-6-astra (agent: phase2-reviewer, role: architecture-layering); reviewer 8: gpt-6-astra (agent: phase2-reviewer, role: ffi-engineer); reviewer 9: gpt-6-astra (agent: phase2-reviewer, role: rust-quality); reviewer 10: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 11: gpt-6-astra (agent: phase2-reviewer, role: security-auditor); reviewer 12: 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 substantial new WASM binding module packages/wasm-sdk/src/private_contact_requests.rs directly implements private-key ingestion, lifetime and erasure management, and outgoing viewing-key derivation in ReadSecretKeys and scan_keys_from_options, meeting the intricate key-handling surface criterion despite delegating cryptographic primitives to the SDK.
  • 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 — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — general (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — security-auditor (completed, effort xhigh); agent phase2-reviewer
  • Model comparison: every Phase-2 reviewer also ran on gpt-6-astra; the verifier weighed both sets without knowing which model wrote which
🤖 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/wasm-sdk/src/private_contact_requests.rs`:
- [BLOCKING] packages/wasm-sdk/src/private_contact_requests.rs:540-543: Refresh retained DashPay contracts that predate private requests
  A browser wallet that previously fetched DashPay v2 can retain that snapshot across the protocol-15 upgrade. WasmSdkBuilder::build reloads persisted contracts into the context cache, and both TrustedHttpContextProvider::get_data_contract and VerifiedHttpContextProvider::get_data_contract return known contracts before consulting the protocol-selected system contract. The delegated SDK helper now establishes protocol 15, but fetch_dashpay_contract → fetch_system_data_contract still accepts the cached v2 contract, which lacks privateContactRequest. Sealing then fails during signing-key/type lookup, and scanning fails when the page verifier resolves that missing type. Neither protocol refresh nor restarting clears this retained snapshot. This breaks the new APIs for an existing-wallet upgrade state, independently of the fixed empty-cache discovery case. Resolve system-contract freshness in the owning SDK/context-provider path so an older retained contract cannot shadow the protocol's required generation, while preserving newer proved overrides. Add regression coverage for a protocol-15 client hydrated with a protocol-14 DashPay snapshot.

In `packages/js-evo-sdk/tests/unit/facades/private-documents.spec.ts`:
- [SUGGESTION] packages/js-evo-sdk/tests/unit/facades/private-documents.spec.ts:19-27: Test the actual WASM option and result conversions
  (existing thread: https://github.com/dashpay/platform/pull/5811#discussion_r4506329231)
  These stubs bypass all three new bindings, so the tests establish facade forwarding but not request_from_options, scan_keys_from_options, rejection-name mapping, or EntriesPage/ScanResult serialization. The native SDK tests begin with typed Rust inputs and cannot verify those JavaScript-boundary behaviors. Although a WASM runtime is required, a new functional harness is not: wasm-sdk already has wasm-bindgen-test library tests, the reusable tests-rs-wasm.yml workflow runs them under Node, and state_transitions/document.rs demonstrates testing private option-conversion helpers directly. Add deterministic binding tests using real PrivateKey, Identity, and IdentitySigner wrappers and nonempty result fixtures. Cover optional fields, malformed 69/43-byte inputs, rejection reasons, Uint8Array outputs, and exact bigint positions/counts above Number.MAX_SAFE_INTEGER. Keep these forwarding tests separately. If this coverage is deferred to the functional suite, link a tracking issue or follow-up PR; the supplied discussion contains no tracking reference.

@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 2daee2b, six prior findings are fixed, but the generic private-page reader still fails when a retained pre-upgrade contract lacks the requested private type. Binding-level conversion coverage also remains missing. This was a static review; current CI shows passing policy checks and a pending review check, but no build or test results.

🔴 1 blocking | 🟡 1 suggestion(s)

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); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 10: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 11: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 12: 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 sizable addition in packages/wasm-sdk/src/private_contact_requests.rs primarily marshals options, keys and results into existing Rust SDK operations rather than introducing intricate cryptographic, signing or consensus 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 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 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, 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/wasm-sdk/src/private_contact_requests.rs`:
- [BLOCKING] packages/wasm-sdk/src/private_contact_requests.rs:451-456: Refresh obsolete retained contracts on the generic private-page path
  A browser wallet can retain DashPay v2 across the protocol-15 upgrade: WasmSdkBuilder::build reloads persisted contracts, and get_or_fetch_contract immediately returns that cached snapshot. Calling privateDocuments.entries(dashpayId, 'privateContactRequest', 0n) then fails on a valid page because PrivateDocumentEntries::FromProof resolves the contract from the context provider and rejects it for lacking the private type. Both providers prefer known contracts over their protocol-selected system contracts, and this fetch path neither refreshes nor evicts the obsolete snapshot, so retrying or rebuilding the SDK does not repair it. The native fetch_private_contact_request_contract helper fixes sealing and scanning, but the new generic reader bypasses that helper. Resolve the required contract/type freshness in the owning SDK page-read path and delegate to it here, preserving newer proved overrides. Add regression coverage for a direct page read by a protocol-15 client hydrated with DashPay v2.

In `packages/js-evo-sdk/tests/unit/facades/private-documents.spec.ts`:
- [SUGGESTION] packages/js-evo-sdk/tests/unit/facades/private-documents.spec.ts:19-28: Test the actual WASM option and result conversions
  (existing thread: https://github.com/dashpay/platform/pull/5811#discussion_r4506329231)
  All three methods are stubbed here, so these tests establish facade forwarding without executing request_from_options, scan_keys_from_options, rejection mapping, or the result DTO serialization. Native SDK tests begin after JavaScript conversion and cannot establish wrapper extraction or the declared bigint/Uint8Array output contracts. This does not require waiting for a network-dependent functional suite: wasm-sdk already has local wasm-bindgen-test examples in queries/group.rs and queries/document_ranked.rs, and tests-rs-wasm.yml contains a wasm32 library-test job. Add deterministic binding-local fixtures covering real wrappers, optional request fields, malformed fixed-size inputs, nonempty results, rejection reasons, Uint8Array outputs, and exact bigint positions/counts above Number.MAX_SAFE_INTEGER. Keep these forwarding tests separately. If coverage remains deferred, link a tracking issue or follow-up PR; the supplied discussion contains none.

QuantumExplorer and others added 4 commits November 9, 2026 07:11
…declares the type

`getPrivateDocumentEntries` resolved its contract with
`get_or_fetch_contract`, which returns a retained contract as it is, such
as DashPay v2 kept from before protocol version 15. The page's proof is
verified against the contract the context provider holds, so every page of
the private type then failed. The binding now resolves the contract through
the SDK's `fetch_private_document_contract`, which replaces a contract
without the type with the current one in the provider, and stores that one
in the contract cache for the next page load too. The seal and the scan
resolve DashPay the same way first, so the stored copy follows.

Reading a scan's keys from the options sizes each key vector first: growing
it would have moved the secret keys and left copies behind.

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

`wasm_bindgen_test` tests, as the wasm32 CI job runs them:
- a request read from options with and without its optional fields, and a
  fixed-size field of another size refused;
- scan keys read from the wrappers JavaScript passes (an `Identifier`, a
  `Uint8Array`, a base58 string, `PrivateKey`), and malformed entries
  refused;
- a scan handed back with every rejection reason, `Uint8Array` outputs, and
  positions and counts above `Number.MAX_SAFE_INTEGER` as exact bigints.

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 4d30b82, all eight prior findings are addressed, and the new bindings retain the intended Rust SDK ownership boundaries. One non-blocking cache-ordering issue remains: private-contract preparation can overwrite a concurrently promoted contract with an older fetched snapshot. This was a static review; current checks show successful policy/title checks and a pending review check, but no build or test results.

🟡 1 suggestion(s)

Review provenance

Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 6: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 7: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 10: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 11: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 12: 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 substantial new packages/wasm-sdk/src/private_contact_requests.rs binding implements private-key ingestion, secret-key lifetime management, outgoing viewing-key derivation and signer integration, making the diff itself an intricate change to key handling.
  • Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
  • Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity quota lookup failed), glm-5.3-flash (not used above high effort; tier asks max)
  • Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
  • Fresh 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 — 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 — 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/wasm-sdk/src/private_contact_requests.rs`:
- [SUGGESTION] packages/wasm-sdk/src/private_contact_requests.rs:486-490: Preserve newer cached contracts during private-page preparation
  This preparation path bypasses the existing protection against delayed fetches overwriting newer contracts. After awaiting DataContract::fetch, Sdk::fetch_private_document_contract calls provider.register_data_contract, whose trusted and verified implementations unconditionally insert into the shared known-contract map. If another operation promotes v2 while this request is fetching v1, the delayed v1 result replaces v2 before cache_contract runs; its version-aware insertion cannot recover the newer in-memory entry. Both responses can pass the default one-block freshness tolerance. A concurrent page request for a private type introduced in v2 can then fail because its proof verifier resolves v1 instead. Make registration monotonic in the owning SDK/context-provider path using the existing newer-version retention mechanism, and add a regression that promotes a newer contract while the initial fetch is pending.

@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 complete five-file diff at 80094d0 and the relevant SDK, context-provider, proof-verification, and upstream implementations. All nine prior findings are fixed, and no actionable in-scope defects remain. This was static verification only: current CI shows successful policy/title checks and a pending review check, but no build or test results establishing validation of this head.

🔴 0 blocking | 🟡 0 suggestion(s) | 💬 0 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: rust-quality); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 6: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 7: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 10: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 11: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 12: 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 substantial new WASM module packages/wasm-sdk/src/private_contact_requests.rs directly handles private-key parsing, lifetime and erasure, outgoing viewing-key derivation, and signer integration, meeting both the intricacy and key-handling criteria despite delegating cryptographic primitives to Rust SDK calls.
  • 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 — 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 — 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 the current code and confirm that no unresolved issues remain.

No unresolved findings remain from the prior review on this head.

QuantumExplorer and others added 2 commits November 9, 2026 10:28
…w carries

A page literal in the native regression sets `chunk_power: None`, as a page
read without a proof has it.

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 complete diff at 75279c6 and confirmed that all nine prior findings are fixed. One non-blocking performance suggestion remains: warm-cache page reads repeat contract serialization and synchronous browser-cache processing. This was a static review only; current checks show successful policy/title checks and a pending automated review, with no build or test results available.

🟡 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: rust-quality); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 6: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 7: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 10: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 11: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 12: 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 substantial new binding in packages/wasm-sdk/src/private_contact_requests.rs directly handles private encryption keys, outgoing viewing-key derivation, signing integration and secret erasure, meeting both the complexity and critical key-handling criteria.
  • 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 — 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 — 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/wasm-sdk/src/private_contact_requests.rs`:
- [SUGGESTION] packages/wasm-sdk/src/private_contact_requests.rs:486-490: Avoid repeating contract cloning and persistence checks for every page
  Both entries and contact-request scanning call this helper for every page, even when fetch_private_document_contract returns the already-held Arc<DataContract>. This unconditionally deep-clones the contract and calls cache_contract. With an attached context on mainnet or testnet, cache_contract_in_context then calls store_if_newer, which serializes the candidate, synchronously reads and parses the network's entire contract-cache JSON, and decodes the stored contract before recognizing that its version is unchanged. A warm-cache store traversal therefore repeats schema allocation and browser-storage processing on every page. Preserve the native freshness check, but track which contract generation has already been persisted so unchanged pages can skip the clone and persistence path. Cover initial persistence, upgrades, and repeated unchanged pages with a regression test.

QuantumExplorer and others added 2 commits November 9, 2026 11:36
…nce a page

Reading a private document store page fetched the contract declaring
the type and, every page, deep-cloned it and stored it for the next page
load. With a context on mainnet or testnet, storing serializes the
contract, reads and parses the network's whole stored contract map
synchronously and decodes the stored one, only to find its version
unchanged, so a store walk did that for every page of a warm cache.

The SDK now records the version of each contract a private read last
stored, and a page whose contract is at that version neither copies nor
stores it; a new version is stored as before. The record is forgotten
wherever the SDK removes or replaces the stored contract (an explicit
removal, a stale-contract drop, a seeded contract), so the next page
stores it again. The SDK's freshness check of the contract is unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…and-scan' into feat/wasm-sdk-private-contact-requests

@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 699693f, the complete six-file diff preserves SDK ownership of contract preparation, cryptographic operations, and proof verification; all ten prior findings are fixed, and no additional actionable in-scope defects were confirmed. Validation was static only: current checks show successful policy/title checks and a pending review check, but no build or test results.

🔴 0 blocking | 🟡 0 suggestion(s) | 💬 0 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: rust-quality); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 6: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 7: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 10: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 11: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 12: 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 substantial WASM implementation in packages/wasm-sdk/src/private_contact_requests.rs introduces private-key parsing, lifetime and erasure management, outgoing viewing-key derivation, and signer integration, making the diff itself intricate key-handling work despite delegating cryptographic primitives to the Rust SDK.
  • 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 — 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 — 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 the current code and confirm that no unresolved issues remain.

No unresolved findings remain from the prior review on this head.

…m-sdk-private-contact-requests

# Conflicts:
#	packages/wasm-sdk/src/sdk.rs

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