Skip to content

feat(sdk): add dash-sdk-cxx, a C++ binding of dash-sdk for Dash Core - #4633

Draft
PastaPastaPasta wants to merge 2 commits into
build/vendorable-workspace-lockfilefrom
feat/platform-sdk-cxx
Draft

PastaPastaPasta wants to merge 2 commits into
build/vendorable-workspace-lockfilefrom
feat/platform-sdk-cxx

Conversation

@PastaPastaPasta

@PastaPastaPasta PastaPastaPasta commented Sep 8, 2026 •

Copy link
Copy Markdown
Member

feat(sdk): add dash-sdk-cxx, a C++ binding of dash-sdk for Dash Core

Issue being fixed or feature implemented

Dash Core's DashPay GUI (dash-qt) needs Dash Platform from C++, on Core's own terms:

  • quorum keys come from Core's own mined LLMQ commitments, with no trusted HTTP quorum service;
  • evonode endpoints come from Core's deterministic masternode list;
  • traffic follows Core's -proxy/-onion route, usually Tor;
  • signatures come from Core's wallet, and no private key leaves it;
  • wallet unload and node shutdown must not hang on an in-flight request.

rs-sdk-ffi cannot be embedded on these terms:

  • rs-sdk-trusted-context-provider (reqwest) is an unconditional dependency;
  • there is no SOCKS5 option;
  • identity create takes the raw asset-lock private key;
  • calls cannot be cancelled.

This PR adds a separate, thin cxx binding of dash-sdk instead.

This replaces the earlier content of this PR (dash-platform-cxx). Following review, the binding is now minimal:

The crate builds no transitions, keeps no freshness or protocol-version state of its own, and does no paging of its own.

What was done?

New workspace member packages/rs-sdk-cxx: crate dash-sdk-cxx, C++ namespace dash_sdk, headers dash/sdk/*.h.

Dependencies:

  • dash-sdk: default features off; dpns-contract, dashpay-contract and core_key_wallet on;
  • platform-encryption;
  • tokio, cxx, async-trait, zeroize.

The normal dependency closure contains no rs-sdk-trusted-context-provider, reqwest, openssl-sys or native-tls. A CI step enforces this.

How it works

Trust. context.rs implements the SDK's ContextProvider over a C++ CoreContext callback:

  • A proof must name the configured Platform LLMQ type. The u32 is compared before it is narrowed to u8, so an out-of-range type never reaches C++.
  • The quorum hash is handed over in Core's internal uint256 byte order. The embedder matches type and hash against its own commitments.
  • A key that is not exactly 48 bytes is refused in the header.
  • Anything unknown fails closed. fetch_quorum_public_key keeps its default (no fetch).
  • The DPNS and DashPay contracts are served from dpp's compiled-in copies.

Transport.

  • Endpoints are https with a literal IP host, or an onion host when a proxy is set. Nothing is resolved locally.
  • A malformed proxy is a construction error, never a direct connection.

Writes. Each write is one of the SDK's end-to-end calls:

  • put_to_platform_and_wait_for_response_with_signer;
  • top_up_identity_with_signer;
  • register_dpns_name, including its contest_fund;
  • send_contact_request, using the client-side ECDH branch.

Signing goes through the C++ WalletSigner, which receives the key's id, type and data, plus the signable bytes or the asset-lock digest. The ECDH secret and the compact xpub come from ContactKeys.

Reads. Each is one SDK call (Fetch, the dpns_usernames queries, the contested queries, the contact-request sweep), marshalled into plain structs. Paging budgets come from the caller, not from the binding.

Status table. Every call returns a Status of one of these kinds: Ok, NotFound, Unavailable, Rejected, Consensus (with its code) or Internal.

  • Only Ok and NotFound come from a proof.
  • Transport, proxy and timeout failures are always Unavailable.
  • Core must not fund or pay on anything else.

Runtime.

  • Calls run on the binding's own tokio runtime. The calling thread blocks on the result.
  • C++ callbacks run under block_in_place.
  • shutdown() refuses new calls, aborts running ones, and waits until every call has returned. Then, still inside the serialized shutdown, it drops the SDK and with it the CoreContext, so neither outlives any shutdown() call, concurrent ones included.
  • A panic or C++ exception never crosses the boundary: every exported body runs under catch_unwind, callbacks are noexcept, and the crate refuses to build with panic = "abort". On a panic, is_contested_username answers true, so it fails closed.

C++ API

This is the surface Dash Core #7670 consumes. Both headers are staged under target/<profile>/include/ next to libdash_sdk_cxx.a.

namespace dash_sdk {
// Construction. rust::Error for a config it cannot honour.
rust::Box<PlatformClient> new_platform_client(const Config& config, std::unique_ptr<CoreContext> context);

// PlatformClient: thread-safe; each call blocks its calling thread.
void shutdown() const noexcept;
IdentityResult  get_identity(const std::array<uint8_t, 32>& id) const noexcept;
IdentityResult  get_identity_by_pubkey_hash(const std::array<uint8_t, 20>& pubkey_hash) const noexcept;
IdResult        resolve_name(rust::Str name) const noexcept;
NamesResult     names_of_identity(const std::array<uint8_t, 32>& identity, uint32_t page_limit, uint32_t max_pages) const noexcept;
NamesResult     search_names(rust::Str prefix, uint32_t limit) const noexcept;
ContestsResult  pending_contests_for_identity(const std::array<uint8_t, 32>& identity) const noexcept;
VoteStateResult contested_vote_state(rust::Str label) const noexcept;
ContactRequestsResult contact_requests(const std::array<uint8_t, 32>& identity, ContactDirection direction, uint64_t since_ms) const noexcept;
IdentityResult  register_identity(rust::Slice<const IdentityKey> keys, const AssetLockProof& proof, uint32_t asset_lock_key_ref, std::unique_ptr<WalletSigner> signer) const noexcept;
BalanceResult   top_up_identity(const std::array<uint8_t, 32>& identity, const AssetLockProof& proof, uint32_t asset_lock_key_ref, std::unique_ptr<WalletSigner> signer) const noexcept;
IdResult        register_dpns_name(rust::Str label, const std::array<uint8_t, 32>& identity, uint32_t key_id, uint64_t contest_fund, std::unique_ptr<WalletSigner> signer) const noexcept;
IdResult        send_contact_request(const ContactRequestInput& input, std::unique_ptr<WalletSigner> signer, std::unique_ptr<ContactKeys> keys) const noexcept;

// No network.
bool is_valid_username(rust::Str label) noexcept;      // false on panic
bool is_contested_username(rust::Str label) noexcept;  // true on panic (fails closed)
CompactXpub decrypt_contact_xpub(const std::array<uint8_t, 32>& shared_secret, rust::Slice<const uint8_t> encrypted_public_key); // rust::Error on failure

// Callbacks (dash/sdk/callbacks.h): std::function wrappers whose methods are noexcept.
// A callback that throws, is empty, or returns false is a refusal.
class WalletSigner {
    WalletSigner(SignIdentityKeyFn, SignAssetLockFn);
    // bool(uint32_t key_id, uint8_t key_type, std::span<const uint8_t> key_data,
    //      std::span<const uint8_t> signable, std::array<uint8_t, 65>& sig_out)   -- signs SHA256d(signable)
    // bool(uint32_t key_ref, const std::array<uint8_t, 32>& sighash, std::array<uint8_t, 65>& sig_out)
};
class ContactKeys {
    ContactKeys(SharedSecretFn, CompactXpubFn);
    // bool(uint32_t sender_key_id, const std::array<uint8_t, 33>& recipient_key, std::array<uint8_t, 32>& secret_out)
    // bool(const std::array<uint8_t, 32>& recipient, uint32_t account_reference, std::array<uint8_t, 69>& xpub_out)
};
class CoreContext {
    CoreContext(QuorumPublicKeyFn, PlatformActivationHeightFn);
    // bool(uint8_t llmq_type, const std::array<uint8_t, 32>& quorum_hash, uint32_t core_chain_locked_height,
    //      std::vector<uint8_t>& key_out)   -- exactly 48 bytes or refused
    // bool(uint32_t& height_out)
};
}

Contracts for the embedder:

  • Config is {network, platform_llmq_type, proxy{kind, address, isolate}, endpoints}, fixed for the client's lifetime. To change endpoints or the route, rebuild the client.
  • quorum_hash crosses in Core's internal uint256 byte order; the binding reverses it.
  • resolve_name → NotFound means "does not resolve", not "available". contested_vote_state normalizes the label.
  • names_of_identity reads at most max_pages pages of page_limit names. Both are chosen by the caller and must be ≥ 1. complete comes from the SDK.
  • search_names refuses limit == 0 and always reports complete = false: the SDK drops unparseable documents after the fetch and does not report the raw page length, so not even a short result proves the end.
  • decrypt_contact_xpub returns the 33-byte key unmodified, and checks only its length. Core must run IsFullyValid()/IsCompressed() itself.
  • register_dpns_name: contest_fund == 0 lets the SDK price the contest just before it sends the domain.
  • Callback objects are moved in per call and may be called, and destroyed, on any thread.
  • shutdown() must not be called from a callback, or while holding a lock a callback takes.

File map from the previous dash-platform-cxx

Previous (packages/rs-platform-cxx) Now (packages/rs-sdk-cxx)
Cargo.toml Cargo.toml: renamed. The direct dpp, futures, hex dependencies, the mocks feature and every dev-dependency are dropped
build.rs build.rs: dash/sdk prefix; stages ffi.h, callbacks.h and rust/cxx.h
include/dash/platform/signer.h include/dash/sdk/callbacks.h. WalletSigner now gets the key's id, type and data, and answers into fixed 65-byte buffers. New: ContactKeys and CoreContext
src/runtime.rs src/runtime.rs. Shutdown now joins every call; adds blocking()
src/signer.rs src/signer.rs. Same adapters. The signature re-verification is removed, and the asset-lock key is named by the embedder's key_ref
src/client.rs src/client.rs. Keeps the config, proxy and endpoint policy. Removes set_endpoints, the height watermark and the verified-version state. Holds the entry points and the status table
src/provider.rs (push) src/context.rs (pull over CoreContext). set_quorum_keys, set_chainlock_height and the ChainLock-lag check are removed
src/lib.rs src/lib.rs: new bridge
src/ops.rs Removed: each read is one SDK call; marshalling lives in src/marshal.rs
src/builders.rs Removed: the SDK's end-to-end calls build and sign
src/helpers.rs Removed: username checks are SDK re-exports; the xpub decrypt is in lib.rs; the MAC, account-reference and key-selection helpers stay in the embedder
src/sync.rs Removed
tests/cxx_smoke.cc, scripts/cxx-smoke.sh Rewritten for the new surface
tests/builders.rs, tests/replay.rs, tests/common/mod.rs, test_data/ Removed: behavioural tests belong with the SDK features they test
README.md Removed: the contract is in the bridge and header docs

Also removed: broadcast, the nonce read, normalize_label, credits_per_duff, system_contract_id, get_profile and the profile envelope, the contested vote-fund helper, and the PV13 paging workarounds.

Size: about 10.6k lines before, 2,681 now. Of those, 2,027 are production:

  • 392 lines of bridge declarations, covering 26 shared types, 6 callback methods and 17 exported functions with their contracts;
  • 645 lines of entry points, status classification and lifecycle;
  • the context provider, signers, runtime and marshalling;
  • a 180-line header with the exception barriers;
  • build glue.

The other 654 are tests: 342 in Rust and 312 in the C++ smoke test.

Not included yet

  • DashPay profiles and invitations. These wait for two rs-sdk changes that move the logic out of rs-platform-wallet. The forwards follow in a later PR.
  • A label-only contest-fund quote. The SDK's contest_fund_to_join needs a candidate document, and building a DPNS domain here would duplicate register_dpns_name. A small rs-sdk helper will provide it. Until then register_dpns_name takes the fund, or 0 for the SDK's own price.
  • Masternode voting. Casting a DPNS vote and listing a masternode's votes need Platform logic that should live in the SDK: the vote-poll construction, the voting-key diagnosis, and the votes-by-proTxHash query with its DPNS filter. The existing get_contested_dpns_identity_votes is a stub that returns an empty vector. Voting comes back as a small SDK PR plus a follow-up forward here.

How Has This Been Tested?

Run on macOS (arm64), Rust 1.98, with CARGO_TARGET_DIR outside the tree:

  • cargo test -p dash-sdk-cxx: 10 binding-level tests pass:
    • the status table, including a local-source refusal reading as unavailable;
    • Tor stream isolation mapping to per-connection credentials;
    • the CoreContext checks: the type compared before narrowing, including u32 values that would wrap to the Platform type; hash order; unknown quorums refused; system contracts served locally;
    • panic containment, with the contested-name predicate failing closed;
    • shutdown interrupting a call, and shutdown waiting for a call blocked in a callback;
    • marshalling: contact-request documents, the compact-xpub round trip with the public key unmodified, and asset-lock signature recovery.
  • packages/rs-sdk-cxx/scripts/cxx-smoke.sh compiles tests/cxx_smoke.cc against the staged headers and static archive and prints cxx_smoke: ok. It:
    • refuses every config the binding cannot honour, covering proxy kinds, proxy addresses and endpoint schemes and hosts, and accepts the valid proxied ones;
    • crosses every bridged entry point against a closed port: each returns a typed status, and zero page or limit arguments are refused;
    • reaches the WalletSigner from register_identity and top_up_identity, checking the key and key_ref marshalling;
    • calls every callback method directly for success, refusal and a contained throw, and checks the 48-byte quorum-key bound;
    • checks that a call after shutdown() is Unavailable;
    • checks that two concurrent shutdown() calls both return only after the CoreContext is destroyed. Reintroducing the earlier clone-based shutdown makes this check fail.
  • cargo clippy -p dash-sdk-cxx --all-targets --all-features --locked -- --no-deps -D warnings, cargo fmt --check and cargo machete are clean.
  • The new CI closure step, run locally, passes. It confirms that cargo tree -p dash-sdk-cxx -e normal has no reqwest, trusted provider, OpenSSL or native-tls.

Not tested here: a live testnet round trip. The iOS↔Core end-to-end test (register, top-up, contact request and accept, pay, restore) runs on the Dash Core side, against a pin of this commit.

Breaking Changes

None. This is a new crate.

Checklist:

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

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

🤖 Generated with Claude Code

@coderabbitai

coderabbitai Bot commented Sep 8, 2026 •

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • 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

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

@PastaPastaPasta
PastaPastaPasta force-pushed the feat/shared-dpns-dashpay-builders branch from 0631c07 to 5279114 Compare September 8, 2026 21:16
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this pull request Sep 8, 2026
…crate

dashpay/platform#4633 rebuilds the Platform CXX bindings as a thin bridge over dash-sdk: the SDK owns DAPI transport, retries and proof verification, and Core supplies endpoints, quorum keys, its ChainLock height and wallet signatures. The crate is an ordinary workspace member now, so the package vendors from the workspace root (the lockfile made vendorable by dashpay/platform#4631), builds with -p dash-platform-cxx, and installs the header tree the crate's build.rs stages plus the static archive; the nested standalone manifest and install.sh are gone with the old design.

mbedtls leaves depends: the SDK carries its own TLS stack (rustls with the system trust store), so Core no longer links a TLS library for Platform. The vendoring config gains the workspace's git sources.

Validated on aarch64-apple-darwin: make -C depends PLATFORM_GUI=1 platform_cxx vendors 840 crates (150 MB archive) and builds the crate offline in 3 minutes; the staged prefix carries include/dash/platform/{ffi.h,signer.h}, include/rust/cxx.h and lib/libdash_platform_cxx.a. The knob-off package set is unchanged.
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this pull request Sep 8, 2026
…ble-platform-gui

The Qt-free client library dash-qt drives for DashPay: per-network parameters and system contract ids, the PlatformClient interface, DPP decoding and state-transition adapters, and the wallet record formats. Where the earlier revision (dashpay#7626) carried its own gRPC-Web/TLS transport, hand-written protobuf and CBOR encoders, per-endpoint retry and freshness tracking, and handed request/response byte pairs to a transport-free verifier, this one is a thin consumer of the Dash Platform SDK through dash-platform-cxx (dashpay/platform#4633). The SDK owns query construction, DAPI transport, retries with address banning, proof verification (GroveDB replay plus the Tenderdash quorum signature against the keys this node pushes from its LLMQ store), protocol-version tracking and the chain-id and ChainLock freshness checks; the node supplies evonode endpoints from its deterministic masternode list, the Platform quorum keys, its best ChainLock height and wallet signatures through a digest callback, so private keys never leave the wallet.

The PlatformClient interface the GUI programs against is unchanged apart from gaining an sdk() accessor; the production implementation keeps its single worker thread and callback marshalling and forwards each query to the SDK handle. Absence stays proven, never inferred: an empty result only reaches a callback after the SDK verified a proof of it. The DPP decoders and state-transition builders take the SDK handle so they build under the protocol version the SDK has seen the network run, ratcheted up from the per-network floor in params.cpp. The C++ transport, protobuf, CBOR, retry and freshness code and their unit tests are gone with the design; the DPP byte-exactness suite and the wallet key tests stay, and the fuzz harness keeps the decoder targets (proof verification is fuzzed upstream).

Validated on aarch64-apple-darwin against a depends prefix carrying the SDK-backed archive: configure detects the bindings, libdash_platform.a and test_dash build, platform_dpp_tests and platformkeys_tests pass.
@PastaPastaPasta
PastaPastaPasta force-pushed the feat/shared-dpns-dashpay-builders branch from 5279114 to 47b05e3 Compare September 9, 2026 21:46
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this pull request Sep 9, 2026
…crate

dashpay/platform#4633 rebuilds the Platform CXX bindings as a thin bridge over dash-sdk: the SDK owns DAPI transport, retries and proof verification, and Core supplies endpoints, quorum keys, its ChainLock height and wallet signatures. The crate is an ordinary workspace member now, so the package vendors from the workspace root (the lockfile made vendorable by dashpay/platform#4631), builds with -p dash-platform-cxx, and installs the header tree the crate's build.rs stages plus the static archive; the nested standalone manifest and install.sh are gone with the old design.

mbedtls leaves depends: the SDK carries its own TLS stack (rustls with the system trust store), so Core no longer links a TLS library for Platform. The vendoring config gains the workspace's git sources.

Validated on aarch64-apple-darwin: make -C depends PLATFORM_GUI=1 platform_cxx vendors 840 crates (150 MB archive) and builds the crate offline in 3 minutes; the staged prefix carries include/dash/platform/{ffi.h,signer.h}, include/rust/cxx.h and lib/libdash_platform_cxx.a. The knob-off package set is unchanged.
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this pull request Sep 9, 2026
…ble-platform-gui

The Qt-free client library dash-qt drives for DashPay: per-network parameters and system contract ids, the PlatformClient interface, DPP decoding and state-transition adapters, and the wallet record formats. Where the earlier revision (dashpay#7626) carried its own gRPC-Web/TLS transport, hand-written protobuf and CBOR encoders, per-endpoint retry and freshness tracking, and handed request/response byte pairs to a transport-free verifier, this one is a thin consumer of the Dash Platform SDK through dash-platform-cxx (dashpay/platform#4633). The SDK owns query construction, DAPI transport, retries with address banning, proof verification (GroveDB replay plus the Tenderdash quorum signature against the keys this node pushes from its LLMQ store), protocol-version tracking and the chain-id and ChainLock freshness checks; the node supplies evonode endpoints from its deterministic masternode list, the Platform quorum keys, its best ChainLock height and wallet signatures through a digest callback, so private keys never leave the wallet.

The PlatformClient interface the GUI programs against is unchanged apart from gaining an sdk() accessor; the production implementation keeps its single worker thread and callback marshalling and forwards each query to the SDK handle. Absence stays proven, never inferred: an empty result only reaches a callback after the SDK verified a proof of it. The DPP decoders and state-transition builders take the SDK handle so they build under the protocol version the SDK has seen the network run, ratcheted up from the per-network floor in params.cpp. The C++ transport, protobuf, CBOR, retry and freshness code and their unit tests are gone with the design; the DPP byte-exactness suite and the wallet key tests stay, and the fuzz harness keeps the decoder targets (proof verification is fuzzed upstream).

Validated on aarch64-apple-darwin against a depends prefix carrying the SDK-backed archive: configure detects the bindings, libdash_platform.a and test_dash build, platform_dpp_tests and platformkeys_tests pass.
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this pull request Sep 9, 2026
…crate

dashpay/platform#4633 rebuilds the Platform CXX bindings as a thin bridge over dash-sdk: the SDK owns DAPI transport, retries and proof verification, and Core supplies endpoints, quorum keys, its ChainLock height and wallet signatures. The crate is an ordinary workspace member now, so the package vendors from the workspace root (the lockfile made vendorable by dashpay/platform#4631), builds with -p dash-platform-cxx, and installs the header tree the crate's build.rs stages plus the static archive; the nested standalone manifest and install.sh are gone with the old design.

mbedtls leaves depends: the SDK carries its own TLS stack (rustls with the system trust store), so Core no longer links a TLS library for Platform. The vendoring config gains the workspace's git sources.

Validated on aarch64-apple-darwin: make -C depends PLATFORM_GUI=1 platform_cxx vendors 840 crates (150 MB archive) and builds the crate offline in 3 minutes; the staged prefix carries include/dash/platform/{ffi.h,signer.h}, include/rust/cxx.h and lib/libdash_platform_cxx.a. The knob-off package set is unchanged.
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this pull request Sep 9, 2026
…ble-platform-gui

The Qt-free client library dash-qt drives for DashPay: per-network parameters and system contract ids, the PlatformClient interface, DPP decoding and state-transition adapters, and the wallet record formats. Where the earlier revision (dashpay#7626) carried its own gRPC-Web/TLS transport, hand-written protobuf and CBOR encoders, per-endpoint retry and freshness tracking, and handed request/response byte pairs to a transport-free verifier, this one is a thin consumer of the Dash Platform SDK through dash-platform-cxx (dashpay/platform#4633). The SDK owns query construction, DAPI transport, retries with address banning, proof verification (GroveDB replay plus the Tenderdash quorum signature against the keys this node pushes from its LLMQ store), protocol-version tracking and the chain-id and ChainLock freshness checks; the node supplies evonode endpoints from its deterministic masternode list, the Platform quorum keys, its best ChainLock height and wallet signatures through a digest callback, so private keys never leave the wallet.

The PlatformClient interface the GUI programs against is unchanged apart from gaining an sdk() accessor; the production implementation keeps its single worker thread and callback marshalling and forwards each query to the SDK handle. Absence stays proven, never inferred: an empty result only reaches a callback after the SDK verified a proof of it. The DPP decoders and state-transition builders take the SDK handle so they build under the protocol version the SDK has seen the network run, ratcheted up from the per-network floor in params.cpp. The C++ transport, protobuf, CBOR, retry and freshness code and their unit tests are gone with the design; the DPP byte-exactness suite and the wallet key tests stay, and the fuzz harness keeps the decoder targets (proof verification is fuzzed upstream).

Validated on aarch64-apple-darwin against a depends prefix carrying the SDK-backed archive: configure detects the bindings, libdash_platform.a and test_dash build, platform_dpp_tests and platformkeys_tests pass.
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this pull request Sep 9, 2026
…crate

dashpay/platform#4633 rebuilds the Platform CXX bindings as a thin bridge over dash-sdk: the SDK owns DAPI transport, retries and proof verification, and Core supplies endpoints, quorum keys, its ChainLock height and wallet signatures. The crate is an ordinary workspace member now, so the package vendors from the workspace root (the lockfile made vendorable by dashpay/platform#4631), builds with -p dash-platform-cxx, and installs the header tree the crate's build.rs stages plus the static archive; the nested standalone manifest and install.sh are gone with the old design.

mbedtls leaves depends: the SDK carries its own TLS stack (rustls with the system trust store), so Core no longer links a TLS library for Platform. The vendoring config gains the workspace's git sources.

Validated on aarch64-apple-darwin: make -C depends PLATFORM_GUI=1 platform_cxx vendors 840 crates (150 MB archive) and builds the crate offline in 3 minutes; the staged prefix carries include/dash/platform/{ffi.h,signer.h}, include/rust/cxx.h and lib/libdash_platform_cxx.a. The knob-off package set is unchanged.
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this pull request Sep 9, 2026
…ble-platform-gui

The Qt-free client library dash-qt drives for DashPay: per-network parameters and system contract ids, the PlatformClient interface, DPP decoding and state-transition adapters, and the wallet record formats. Where the earlier revision (dashpay#7626) carried its own gRPC-Web/TLS transport, hand-written protobuf and CBOR encoders, per-endpoint retry and freshness tracking, and handed request/response byte pairs to a transport-free verifier, this one is a thin consumer of the Dash Platform SDK through dash-platform-cxx (dashpay/platform#4633). The SDK owns query construction, DAPI transport, retries with address banning, proof verification (GroveDB replay plus the Tenderdash quorum signature against the keys this node pushes from its LLMQ store), protocol-version tracking and the chain-id and ChainLock freshness checks; the node supplies evonode endpoints from its deterministic masternode list, the Platform quorum keys, its best ChainLock height and wallet signatures through a digest callback, so private keys never leave the wallet.

The PlatformClient interface the GUI programs against is unchanged apart from gaining an sdk() accessor; the production implementation keeps its single worker thread and callback marshalling and forwards each query to the SDK handle. Absence stays proven, never inferred: an empty result only reaches a callback after the SDK verified a proof of it. The DPP decoders and state-transition builders take the SDK handle so they build under the protocol version the SDK has seen the network run, ratcheted up from the per-network floor in params.cpp. The C++ transport, protobuf, CBOR, retry and freshness code and their unit tests are gone with the design; the DPP byte-exactness suite and the wallet key tests stay, and the fuzz harness keeps the decoder targets (proof verification is fuzzed upstream).

Validated on aarch64-apple-darwin against a depends prefix carrying the SDK-backed archive: configure detects the bindings, libdash_platform.a and test_dash build, platform_dpp_tests and platformkeys_tests pass.
@PastaPastaPasta
PastaPastaPasta force-pushed the feat/shared-dpns-dashpay-builders branch 2 times, most recently from cfb93ac to edb833b Compare September 10, 2026 20:18
@thepastaclaw

thepastaclaw commented Sep 15, 2026 •

Copy link
Copy Markdown
Collaborator

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

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

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

@PastaPastaPasta
PastaPastaPasta force-pushed the feat/platform-sdk-cxx branch 2 times, most recently from 25fd33c to b40f44c Compare September 25, 2026 09:20
@PastaPastaPasta
PastaPastaPasta changed the base branch from feat/shared-dpns-dashpay-builders to v4.2-dev September 25, 2026 21:38
@PastaPastaPasta PastaPastaPasta changed the title feat(sdk): add CXX bindings over dash-sdk for C++ embedders feat(sdk): add dash-platform-cxx, a thin CXX shell over dash-sdk for C++ embedders Sep 25, 2026
@github-actions github-actions Bot added this to the v4.2.0 milestone Sep 25, 2026

@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

The CXX shell has four blocking correctness defects: PV13 pagination can silently omit contact requests, contested vote state is truncated at 100 contenders without continuation, rejected foreign-chain responses can poison the SDK's protocol-version state, and document builders omit the state-dependent contest funding required for existing contests. Several additional API, input-validation, and test-quality issues should also be addressed before relying on the new interface.

🔴 4 blocking | 🟡 6 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: architecture-layering); reviewer 3: muse-spark-1.3-contributor (agent: phase1-reviewer, role: ffi-engineer); reviewer 4: muse-spark-1.3-contributor (agent: phase1-reviewer, role: platform-versioning); reviewer 5: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 6: muse-spark-1.3-contributor (agent: phase1-reviewer, role: security-auditor); reviewer 7: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 8: gpt-6-astra (agent: phase2-reviewer, role: architecture-layering); reviewer 9: gpt-6-astra (agent: phase2-reviewer, role: ffi-engineer); reviewer 10: gpt-6-astra (agent: phase2-reviewer, role: platform-versioning); reviewer 11: gpt-6-astra (agent: phase2-reviewer, role: rust-quality); reviewer 12: gpt-6-astra (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6-astra (agent: astra-verifier, role: final-verifier)

  • Triage: critical by gpt-6-astra (effort low) — This large, intricate new FFI layer directly introduces signature and key-handling logic in packages/rs-platform-cxx/src/signer.rs, including wallet signing callbacks and AssetLockSigner::sign_ecdsa compact-signature parsing and public-key recovery, alongside signed state-transition assembly in builders.rs.
  • Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — architecture-layering (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — ffi-engineer (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — platform-versioning (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — security-auditor (completed, effort xhigh); agent phase1-reviewer
  • Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 13% left, 5h 100% left), glm-5.3-flash (not used above high effort; tier asks max)
  • Fresh verifier: gpt-6-astra — final-verifier; agent astra-verifier
  • Phase 2 reviewers: 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 — platform-versioning (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
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-platform-cxx/src/ops.rs`:
- [BLOCKING] packages/rs-platform-cxx/src/ops.rs:644-670: Guard contact-request pagination against PV13 timestamp-boundary skips
  This query orders by `$createdAt` and uses the generic document-ID cursor, but protocol versions below 14 can return a proven empty continuation when a page boundary splits contact requests sharing the same `$createdAt`. The first page can therefore contain 100 requests and report `has_more`, while the next call returns zero items and `has_more = false`, silently omitting requests. Requests created in the same Platform block can share a timestamp, and the behavior affects both incoming and outgoing queries. Use a safe continuation strategy or return `Unavailable` for unsupported PV13 continuations, as `names_of_identity` already does, and add same-timestamp paging coverage.
- [BLOCKING] packages/rs-platform-cxx/src/ops.rs:724-739: Expose continuation for contested states exceeding 100 contenders
  The request always uses `limit: Some(CONTESTED_VOTE_COUNT)` with `CONTESTED_VOTE_COUNT = 100` and `start_at: None`. `VerifiedContested` exposes neither a cursor nor an incomplete-result indicator. Contests can legally contain more than 100 contenders; the current protocol limits permit up to 1,000. The API therefore returns a successful, proved first page as though it were the complete contested state, so an embedder cannot reliably find a later contender or display all tallies. Add pagination/completeness information while preserving the one-request-per-call contract.
- [BLOCKING] packages/rs-platform-cxx/src/ops.rs:169-227: Apply chain acceptance before the SDK mutates verification state
  `fetch` calls `client.run`, which performs proof verification inside the SDK, before calling the shell's `accept` function. Proof verification invokes `Sdk::verify_response_metadata`, which ratchets the shared SDK protocol version before `accept` checks the signed chain ID. A correctly signed response from a foreign chain can therefore advance the SDK to a newer protocol version, be rejected as `ChainIdMismatch`, and leave a later valid legacy response unable to parse under the mutated version. The shell's own watermark remains unchanged, but the shared SDK state is still poisoned. Add an SDK acceptance hook that checks the embedder's chain policy after proof/signature verification and before `verify_response_metadata` commits the version, or otherwise prevent rejected metadata from mutating SDK state.

In `packages/rs-platform-cxx/src/builders.rs`:
- [BLOCKING] packages/rs-platform-cxx/src/builders.rs:213-228: Preserve state-dependent contest-fund preparation in the document builder
  The create path passes `None` for `StateTransitionCreationOptions`, so it bypasses `rs-sdk`'s `with_contest_fund_to_join` preparation. That preparation reads the existing contender count and increases the prefund when joining an existing contest. At the first funding-doubling threshold, the transition built here carries only the base fund while Drive requires the larger amount and rejects it with `DocumentContestNotPaidForError`. The CXX API exposes no contest maximum or creation-options parameter that lets callers supply the required amount. Share the SDK's canonical preparation/build path or expose an explicit, state-aware maximum and test a domain joining an existing contest.
- [SUGGESTION] packages/rs-platform-cxx/src/builders.rs:256-314: Keep protocol-owned document and DIP-15 encoding logic in lower layers
  The CXX shell duplicates protocol semantics that already belong to lower layers: DPNS preorder hashing and document properties, DashPay profile create/replace assembly, and the DIP-15 ASK28/account-reference bit layout in `helpers.rs:71-83`. The implementations match today, but future system-contract or DIP-15 corrections can update the SDK or `platform-encryption` while leaving C++ callers on stale behavior. Add pure, parameterized builders and MAC-based helpers to the owning crates, then have this adapter perform only FFI conversion and signing integration.
- [SUGGESTION] packages/rs-platform-cxx/src/builders.rs:455-466: Reject contradictory recipient IDs before building a contact request
  `ContactRequestInput` carries `to_user_id`, but the builder never validates or uses it. The SDK derives the document's `toUserId` from `recipient.id()` instead. If the two FFI inputs disagree, the builder successfully signs a request addressed to a different identity than the caller supplied, making contradictory embedder state silently actionable. Validate equality before invoking the SDK or remove the redundant field, and add a mismatch test that confirms no signer callback is made.
- [SUGGESTION] packages/rs-platform-cxx/src/builders.rs:418-423: Check the profile revision increment before building a replacement
  `existing.revision + 1` is computed from embedder-supplied data without overflow checking. At `u64::MAX`, debug builds panic and release builds wrap to zero, producing an invalid replacement revision rather than rejecting the input before signing.

In `packages/rs-platform-cxx/tests/cxx_smoke.cc`:
- [SUGGESTION] packages/rs-platform-cxx/tests/cxx_smoke.cc:159-169: Value-initialize the C++ identity key before passing it to Rust
  The generated C++ `ContractBounds` contains a `std::array<uint8_t, 32> contract_id` without a default member initializer. `platform_ffi::IdentityKey key;` therefore leaves that array indeterminate; assigning only `bounds.kind` does not initialize it. Rust unconditionally copies `bounds.contract_id` before matching `NoBounds`, so this smoke test reads uninitialized bytes across the CXX boundary even though the field is unused for this variant.

In `packages/rs-platform-cxx/src/helpers.rs`:
- [SUGGESTION] packages/rs-platform-cxx/src/helpers.rs:71-84: Reject or explicitly constrain DIP-15 versions above four bits
  The public helper accepts `version: u32` but stores it in the top four bits using `(version << 28)`. Values above `0x0f` silently lose their high bits; for example, version 16 produces version bits zero and cannot round-trip through the inverse helper. Validate the range at the FFI boundary or change the API to return an error instead of silently truncating caller input.

In `packages/rs-platform-cxx/src/lib.rs`:
- [SUGGESTION] packages/rs-platform-cxx/src/lib.rs:873-885: Keep the network-free contact builder independent of endpoint readiness
  Unlike the other builders, `build_contact_request` calls `self.0.sdk()?` before invoking the SDK's local contact-document assembly. With no endpoints pushed, this returns `no evonode endpoints` even when a verified protocol version and all contact inputs are available. The API documents builders as requiring no network request, and the other builders work after verification without an endpoint set. Provide an endpoint-independent SDK/context handle for this local operation or otherwise make the readiness precondition consistent across builders.

Comment thread packages/rs-platform-cxx/src/ops.rs Outdated
Comment thread packages/rs-platform-cxx/src/ops.rs Outdated
Comment thread packages/rs-platform-cxx/src/ops.rs Outdated
Comment thread packages/rs-platform-cxx/src/builders.rs Outdated
Comment thread packages/rs-platform-cxx/src/builders.rs Outdated
Comment thread packages/rs-platform-cxx/src/builders.rs Outdated
Comment thread packages/rs-platform-cxx/tests/cxx_smoke.cc Outdated
Comment thread packages/rs-platform-cxx/src/helpers.rs Outdated
Comment thread packages/rs-platform-cxx/src/builders.rs Outdated
Comment thread packages/rs-platform-cxx/src/lib.rs Outdated
@PastaPastaPasta
PastaPastaPasta force-pushed the feat/dapi-client-socks5-proxy branch from 0f8fc29 to 324dbc9 Compare September 29, 2026 05:01
@PastaPastaPasta
PastaPastaPasta force-pushed the feat/platform-sdk-cxx branch 2 times, most recently from c5752ab to 35eac29 Compare September 29, 2026 05:09

@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 — Preliminary review — Phase 1 blocker gate

The CXX shell preserves the intended trust and version-gating boundaries, but several previously identified correctness issues remain. In particular, contested vote results are silently truncated, contact-request pagination can skip results on PV13, and foreign-chain proof verification can mutate SDK protocol-version state before the shell rejects the response.

Validated blockers were found by the Phase-1 review and confirmed by a fresh verifier. Phase 2 is deferred until a fresh same-head revalidation clears the blocker gate.

🔴 4 blocking | 🟡 6 suggestion(s)

10 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: architecture-layering); reviewer 3: muse-spark-1.3-contributor (agent: phase1-reviewer, role: ffi-engineer); reviewer 4: muse-spark-1.3-contributor (agent: phase1-reviewer, role: platform-versioning); reviewer 5: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 6: muse-spark-1.3-contributor (agent: phase1-reviewer, role: security-auditor); final verifier: gpt-6-astra (agent: astra-gate-verifier, role: verifier)

  • Triage: critical by gpt-6-astra (effort low) — This large, intricate addition directly implements signature and key-handling boundaries in packages/rs-platform-cxx/src/signer.rs (including AssetLockSigner::sign_ecdsa signature parsing and public-key recovery) and quorum trust-anchor enforcement in src/provider.rs, meeting both the complexity and critical-surface criteria.
  • Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — architecture-layering (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — ffi-engineer (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — platform-versioning (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — security-auditor (completed, effort xhigh); agent phase1-reviewer
  • Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 13% left, 5h 100% left), glm-5.3-flash (not used above high effort; tier asks max)
  • Fresh verifier: gpt-6-astra — verifier; agent astra-gate-verifier
  • Phase 2 reviewers: not run (deferred by blocker gate)
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-platform-cxx/src/lib.rs`:
- [SUGGESTION] packages/rs-platform-cxx/src/lib.rs:873-885: Keep the network-free contact builder independent of endpoint readiness
  (existing thread: https://github.com/dashpay/platform/pull/4633#discussion_r4129701915)
  The contact builder only assembles and signs a request using the local context provider; it does not perform network I/O. However, this entry point requires `self.0.sdk()`, which returns an error until endpoints have been installed and also fails after endpoints are cleared. This unnecessarily couples offline transition construction to transport readiness. Retain a provider-backed SDK or otherwise separate builder SDK acquisition from the live network SDK.

In `packages/rs-platform-cxx/src/builders.rs`:
- [SUGGESTION] packages/rs-platform-cxx/src/builders.rs:418-423: Check the profile revision increment before building a replacement
  (existing thread: https://github.com/dashpay/platform/pull/4633#discussion_r4129701907)
  The replacement revision is derived from embedder-supplied data with plain `existing.revision + 1`. At `u64::MAX` this panics in debug builds or wraps in release builds, rather than returning a controlled error before signing. Use checked arithmetic at the boundary.
- [SUGGESTION] packages/rs-platform-cxx/src/builders.rs:455-468: Reject contradictory recipient IDs before building a contact request
  (existing thread: https://github.com/dashpay/platform/pull/4633#discussion_r4129701877)
  The bridge accepts both a structured recipient identity and `input.to_user_id`, but the builder never compares them. The SDK request is constructed from the structured recipient, so a caller can provide a different `to_user_id` and receive a successfully built request addressed to a different identity than the input claims. Validate the two IDs before deriving the shared secret and signing.
- [SUGGESTION] packages/rs-platform-cxx/src/builders.rs:256-314: Keep protocol-owned document and DIP-15 encoding logic in lower layers
  (existing thread: https://github.com/dashpay/platform/pull/4633#discussion_r4129701869)
  The shell duplicates protocol-owned DPNS commitment hashing and preorder/domain document construction that already has a canonical SDK/DPP path. The PR describes this crate as a thin shell, but these copies can drift when the SDK or system-contract behavior changes, causing transitions to be rejected only after signing. Move the canonical constructors or encodings into the owning layer and keep this crate focused on FFI translation.
- [BLOCKING] packages/rs-platform-cxx/src/builders.rs:213-228: Preserve state-dependent contest-fund preparation in the document builder
  (existing thread: https://github.com/dashpay/platform/pull/4633#discussion_r4129701856)
  The SDK's normal document-create path calls `with_contest_fund_to_join`, which reads the current contest state and supplies the maximum fund required to join, including the increased amount for crowded contests. This CXX builder calls the lower-level DPP constructor directly with `None` for `StateTransitionCreationOptions` and performs no equivalent state read. DPP only applies a contest fund when one is supplied; it does not perform the SDK's live contest lookup here. Consequently, a contested domain can be built without the required join fund and be rejected after signing and nonce/fee processing. The builder must obtain and pass the state-dependent contest fund, or expose a state-aware SDK helper for this path.

In `packages/rs-platform-cxx/src/helpers.rs`:
- [SUGGESTION] packages/rs-platform-cxx/src/helpers.rs:71-84: Reject or explicitly constrain DIP-15 versions above four bits
  (existing thread: https://github.com/dashpay/platform/pull/4633#discussion_r4129701903)
  The account-reference format reserves only the top four bits for the rotation version, but this function accepts an unrestricted `u32` and shifts it into those bits. Values above 15 silently alias or corrupt the encoded reference. Reject versions greater than 15, or explicitly mask them if wrapping is the intended contract and document that behavior.

In `packages/rs-platform-cxx/tests/cxx_smoke.cc`:
- [SUGGESTION] packages/rs-platform-cxx/tests/cxx_smoke.cc:159-169: Value-initialize the C++ identity key before passing it to Rust
  (existing thread: https://github.com/dashpay/platform/pull/4633#discussion_r4129701882)
  The smoke test default-declares `platform_ffi::IdentityKey key;` and initializes only selected fields. The bounds structure includes an array field that is not explicitly initialized, so this test can pass indeterminate bytes across the CXX boundary. Use value-initialization to make the bridge input deterministic.

In `packages/rs-platform-cxx/src/ops.rs`:
- [BLOCKING] packages/rs-platform-cxx/src/ops.rs:169-227: Apply chain acceptance before the SDK mutates verification state
  (existing thread: https://github.com/dashpay/platform/pull/4633#discussion_r4129701850)
  The shell checks `metadata.chain_id` only after `client.run(op)` has completed SDK proof verification. `Client::platform_version` documents that the SDK ratchets its internal protocol version during verification, before this chain-id check. A validly signed response from another chain can therefore mutate the shared SDK's version before being rejected by the shell. That state is later used by SDK operations such as contact-request creation, so the foreign-chain response can influence subsequent local behavior. Move chain acceptance ahead of the ratchet or isolate and roll back the SDK state when the chain check fails.
- [BLOCKING] packages/rs-platform-cxx/src/ops.rs:724-755: Expose continuation for contested states exceeding 100 contenders
  (existing thread: https://github.com/dashpay/platform/pull/4633#discussion_r4129701841)
  The contested vote query sets `limit: Some(CONTESTED_VOTE_COUNT)` with a limit of 100 and no `start_at`, offset, cursor, or `has_more` result. Contests can contain more than 100 contenders, so the FFI returns an apparently successful but incomplete tally. Either page through all contenders internally or expose a continuation mechanism and make truncation explicit.
- [BLOCKING] packages/rs-platform-cxx/src/ops.rs:644-677: Guard contact-request pagination against PV13 timestamp-boundary skips
  (existing thread: https://github.com/dashpay/platform/pull/4633#discussion_r4129701828)
  Contact requests are ordered by `$createdAt` but continuation uses only a document-id `StartAfter` cursor while the optional filter uses `$createdAt > since_ms`. The neighboring `names_of_identity` implementation explicitly rejects PV13 continuation because this class of index/cursor behavior can return an empty page while entries remain. Contact-request pagination has no equivalent guard, overlap, or compound timestamp-plus-ID cursor, so a multi-page PV13 inbox can silently skip requests at the page boundary. Apply the same PV13 guard or implement a boundary-safe cursor.

@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

The exact head preserves the SDK proof-verification boundary and does not modify shipped consensus behavior. Eight client/API and FFI-fixture issues remain; under the supplied severity policy, these are suggestions rather than consensus blockers. This verification was static only: no builds or tests were run, and the supplied CI snapshot contains no Rust/CXX validation result.

🟡 8 suggestion(s)

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

Review provenance

Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 7: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 8: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); 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 new integration directly implements signature handling and asset-lock public-key recovery in packages/rs-platform-cxx/src/signer.rs and quorum-key trust and proof freshness checks in provider.rs, meeting both the intricacy and critical-surface 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 below 15% reserve: weekly 15% left, 5h 100% left), glm-5.3-flash (not used above high effort; tier asks max)
  • Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
  • Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
  • Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
  • Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-platform-cxx/src/builders.rs`:
- [SUGGESTION] packages/rs-platform-cxx/src/builders.rs:217-228: Preserve state-dependent contest-fund preparation in the document builder
  (existing thread: https://github.com/dashpay/platform/pull/4633#discussion_r4129701856)
  The final None leaves creation options at their default, so DPP attaches only the base contested-document fund. At PV14, required_vote_resolution_fund_to_join requires twice that amount when 250 contenders already exist, with further increases every 50 contenders. Document-create state_v2 rejects an insufficient stated maximum during paid validation. The regular SDK create path prepares this through with_contest_fund_to_join, but build_dpns_domain exposes no funding override, so callers cannot construct a valid join for these populated contests through the bridge. Preserve network-free construction by accepting an explicit contest-fund maximum and forwarding it through StateTransitionCreationOptions.contest_fund, with state-dependent preparation delegated to the SDK. Add a populated-contest regression; comparisons against another DPP construction using None reproduce the omission.
- [SUGGESTION] packages/rs-platform-cxx/src/builders.rs:455-466: Reject contradictory recipient IDs before building a contact request
  (existing thread: https://github.com/dashpay/platform/pull/4633#discussion_r4129701877)
  ContactRequestInput exposes to_user_id, but this conversion never reads or validates it. The SDK constructs toUserId from the separately supplied recipient identity, so contradictory bridge inputs can successfully sign a request addressed to a different identity from the one named in input.to_user_id. The public-key check validates the encryption key selected from that recipient identity; it does not establish agreement between the two destination IDs. Reject a mismatch before minting or signing, or remove the redundant field so the bridge has one authoritative destination. Add a negative case asserting that the signer is not called.
- [SUGGESTION] packages/rs-platform-cxx/src/builders.rs:418-424: Check the profile revision increment before building a replacement
  (existing thread: https://github.com/dashpay/platform/pull/4633#discussion_r4129701907)
  The bridge revision is an unrestricted u64, and the preceding validation rejects only values below INITIAL_REVISION. An input of u64::MAX reaches existing.revision + 1: overflow-checked builds panic, while ordinary release builds wrap to zero. The replacement constructor checks that a revision exists but does not reject zero before signing, so release mode can return a signed invalid replacement. Use checked_add, or construct the document at the existing revision and delegate to DPP's checked increment_revision method. Return a descriptive local error and test that the maximum revision is refused before the signer callback.

In `packages/rs-platform-cxx/src/ops.rs`:
- [SUGGESTION] packages/rs-platform-cxx/src/ops.rs:723-727: Expose continuation for contested states exceeding 100 contenders
  (existing thread: https://github.com/dashpay/platform/pull/4633#discussion_r4129701841)
  This query always requests the first 100 contenders. The bridge accepts no continuation, and VerifiedContested exposes neither page metadata nor a completeness indicator. Valid contests can exceed this limit; PV14 permits 1,000 contenders. A contest containing 101 contenders therefore returns Ok while omitting an identity and its tally, and the caller cannot retrieve that omitted entry through this API. The proof authenticates the bounded query, not a complete contender list. Expose the existing query's start_at capability and page metadata, preserving one SDK request per bridge call, or return an explicit incomplete outcome when completeness cannot be established. Cover a contest exceeding 100 contenders in the replay suite.
- [SUGGESTION] packages/rs-platform-cxx/src/ops.rs:642-664: Guard contact-request pagination against PV13 timestamp-boundary skips
  (existing thread: https://github.com/dashpay/platform/pull/4633#discussion_r4129701828)
  StartAfter is unsafe here at PV13 when a page boundary splits requests sharing $createdAt. With a positive since_ms, the frozen v0 lowering passes the exclusive cursor to WhereClause::to_path_query, which starts strictly after the cursor's timestamp key; the equality-only route also excludes that terminal timestamp key. Remaining document IDs at that timestamp are skipped. Both prover and verifier use this lowering, so the next page can verify successfully and return Ok with has_more=false despite omitted contacts. PV14's cursor-padding path is version-gated and does not protect PV13. Add a fail-closed legacy continuation guard, analogous to names_of_identity, or a demonstrably complete legacy paging strategy. Test a page boundary splitting equal timestamps at PV13 and PV14 without changing shipped Drive v0.

In `packages/rs-platform-cxx/tests/cxx_smoke.cc`:
- [SUGGESTION] packages/rs-platform-cxx/tests/cxx_smoke.cc:157: Value-initialize the C++ identity key before passing it to Rust
  (existing thread: https://github.com/dashpay/platform/pull/4633#discussion_r4129701882)
  The pinned CXX 1.0.198 generator supplies default member initializers for primitive scalar fields, but not std::array members. IdentityKey key therefore leaves bounds.contract_id indeterminate. Assigning NoBounds does not initialize that array, and builders::contract_bounds reads it into an Identifier before matching the bounds kind. The devnet builder path consequently reads uninitialized input across the FFI boundary. Value-initialize the key. Apply the same initialization discipline to the devnet ContactRequestInput at line 204: recipient_pubkey is never assigned, yet build_contact_request reads it through PublicKey::from_slice before producing the expected error. Rust Default and catch_unwind do not make these C++ inputs initialized.

In `packages/rs-platform-cxx/src/lib.rs`:
- [SUGGESTION] packages/rs-platform-cxx/src/lib.rs:876-881: Keep the network-free contact builder independent of endpoint readiness
  (existing thread: https://github.com/dashpay/platform/pull/4633#discussion_r4129701915)
  This local builder requires self.0.sdk(), which fails when the endpoint set is empty. After an accepted read establishes the protocol version, set_endpoints([]) deliberately drops that SDK while preserving the verified version. Other builders remain usable, but contact construction fails with no evonode endpoints even though both identities, encryption inputs, nonce and signing callback are supplied locally and the contract comes from the local provider. Separate local minting capability from transport readiness rather than retaining the connection-owning SDK solely for this builder. Add coverage for accepting a read, clearing endpoints, and then constructing a contact request.

In `packages/rs-platform-cxx/src/helpers.rs`:
- [SUGGESTION] packages/rs-platform-cxx/src/helpers.rs:78-84: Reject or explicitly constrain DIP-15 versions above four bits
  (existing thread: https://github.com/dashpay/platform/pull/4633#discussion_r4129701903)
  The bridge accepts a u32 version but encodes only its low four bits through version << 28. Version 16 therefore encodes identically to version 0, and the advertised inverse cannot recover the supplied version. This matches the existing platform-encryption formula, so it is not an encoding divergence, but the new API does not explicitly state a supported input range or rollover policy. Document and test a 0..=15 precondition, explicitly define modulo-16 behavior, or make the operation fallible for larger versions. This avoids callers treating an unrestricted rotation counter as a lossless input.

@PastaPastaPasta
PastaPastaPasta marked this pull request as draft October 5, 2026 17:41
@PastaPastaPasta
PastaPastaPasta force-pushed the feat/dapi-client-socks5-proxy branch from 2bb7671 to 6109d6e Compare October 6, 2026 18:15
Base automatically changed from feat/dapi-client-socks5-proxy to v5.1-dev October 14, 2026 04:48
@github-actions github-actions Bot modified the milestones: v5.0.0, v5.1.0 Oct 14, 2026
@PastaPastaPasta PastaPastaPasta changed the title feat(sdk): add dash-platform-cxx, a thin CXX shell over dash-sdk for C++ embedders feat(sdk): add dash-sdk-cxx, a C++ binding of dash-sdk for Dash Core Oct 16, 2026
@PastaPastaPasta
PastaPastaPasta force-pushed the feat/platform-sdk-cxx branch 2 times, most recently from fb0a743 to 8d48a01 Compare October 16, 2026 06:36
PastaPastaPasta and others added 2 commits October 16, 2026 01:42
A cxx bridge (C++ namespace dash_sdk, headers dash/sdk/ffi.h and
dash/sdk/callbacks.h) for an embedder that is its own source of truth.
Each entry point forwards to dash-sdk and returns a typed status: Ok and
NotFound only come from proofs.

- Trust: dash-sdk's pull ContextProvider over a C++ CoreContext. Proofs
  must name the configured Platform LLMQ type (checked before narrowing),
  the embedder answers by type and hash, and anything it does not hold
  fails closed. DPNS and DashPay contracts come from dpp.
- Transport: endpoints from the embedder only (https, literal IP, onion
  with a proxy); SOCKS5 through SdkBuilder::with_proxy.
- Writes: the SDK's end-to-end identity create, top-up, DPNS registration
  (with its contest_fund) and contact request, signed through a C++
  WalletSigner and ContactKeys. No private key crosses the FFI.
- Reads: identity by id and key hash, name resolution, names of an
  identity (caller-bounded paging), prefix search, an identity's pending
  contests and a contest's vote state, and the contact request sweep.
- Runtime: calls run on the binding's tokio runtime; shutdown cancels and
  joins every call and destroys the CoreContext before any shutdown
  caller returns; callbacks run under block_in_place; every exported body
  runs under a panic guard and C++ exceptions stay in the callbacks.

Tests are binding-level: a C++ smoke test that crosses every entry point
and every callback method, plus panic and exception containment,
concurrent shutdown, marshalling, the status table and the CoreContext
checks.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Add the crate to the Rust package filters and the workspace nextest run,
and two steps to the workspace job: fail when a trusted HTTP quorum
provider, an HTTP client or OpenSSL enters the crate's normal dependency
closure, or when its sources reach for built-in network seeds; and link
and run the C++ smoke test from the staged headers and static archive.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@PastaPastaPasta
PastaPastaPasta changed the base branch from v5.1-dev to build/vendorable-workspace-lockfile October 16, 2026 06:42

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants