From 70b0852edb329a781940c99632eb6f4a5cebffd5 Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Tue, 21 Jul 2026 06:26:44 +0700 Subject: [PATCH 01/10] docs(kotlin-sdk): spec + independent review for DashPay registration keys Reviewed design (two rounds: feasibility/security, then an independent codex call-graph cross-check) for provisioning the DashPay ENCRYPTION/DECRYPTION key pair at identity-registration time so a freshly created Android identity can use its own Add Contact flow. Co-Authored-By: Claude Opus 4.8 --- docs/sdk/CODEX_REVIEW_NOTES.md | 395 ++++++++++++++++++ ...TLIN_SDK_DASHPAY_REGISTRATION_KEYS_SPEC.md | 384 +++++++++++++++++ 2 files changed, 779 insertions(+) create mode 100644 docs/sdk/CODEX_REVIEW_NOTES.md create mode 100644 docs/sdk/KOTLIN_SDK_DASHPAY_REGISTRATION_KEYS_SPEC.md diff --git a/docs/sdk/CODEX_REVIEW_NOTES.md b/docs/sdk/CODEX_REVIEW_NOTES.md new file mode 100644 index 0000000000..1c0b45c420 --- /dev/null +++ b/docs/sdk/CODEX_REVIEW_NOTES.md @@ -0,0 +1,395 @@ +# Independent review: Kotlin DashPay registration keys + +**Review date:** 2026-07-20 + +**Reviewed branch/worktree:** `feat/kotlin-sdk-dashpay-registration-keys` + +**Reviewed input:** `KOTLIN_SDK_DASHPAY_REGISTRATION_KEYS_SPEC.md` + +## Bottom line + +The underlying defect is real, the proposed PR split is sensible, and the main +design direction (send rich key rows over JNI and provision the DashPay +ENCRYPTION/DECRYPTION pair before identity creation) is sound. This is not a +consensus-protocol change and the existing C `IdentityPubkeyFFI` layout is +already sufficient. + +I would not implement the spec exactly as written yet. The following material +points need reconciliation first: + +1. Promoting only `DecodedPubkeyRow` and `decode_update_pubkeys_blob` to + `pub(crate)` is insufficient: every field on the row is still private. +2. The key-id-0 invariant must not be added unconditionally to the shared + update decoder. Identity updates legitimately add rows without key 0. +3. CREATE lacks the explicit contract-bounds *validation* call used by UPDATE, + but Drive's CREATE insertion path still resolves the contract/document type + and checks its bounded-key requirement. The spec's "any wrong constant + registers successfully" and "exact Add Contact bug" failure description is + therefore inaccurate. +4. Adding two keys changes the creation fee and exactly reaches the protocol's + six-key maximum. Swift intentionally excludes the unused-asset-lock/resume + path, while this spec includes it. Kotlin does not expose the tracked lock's + amount, so a four-key-minimum lock may be insufficient for a six-key create. +5. The C/JNI function signatures remain stable, but the opaque Kotlin-to-JNI + byte format changes incompatibly and the affected Kotlin registration APIs + are public. This is a logical ABI/API concern even though it is not a + protocol or C-struct change. +6. The proposed tests do not yet prove cross-language wire compatibility or + the on-chain contract bounds, and one verification command names a + nonexistent Cargo package. + +## 1. Technical-claim verification + +### 1.1 Bug and current registration paths: verified + +- `role_for_registration_key_id` assigns roles solely from key ID and has no + ENCRYPTION/DECRYPTION case + (`packages/rs-unified-sdk-jni/src/identity.rs:77-131`). +- The legacy decoder carries only `keyId` and public bytes + (`packages/rs-unified-sdk-jni/src/identity.rs:930-1003`). +- All four JNI paths reconstruct the role, set `read_only: false`, and set + `contract_bounds_kind: 0`: + + - resume: `packages/rs-unified-sdk-jni/src/identity.rs:599-623` + - Core/asset-lock funded: `packages/rs-unified-sdk-jni/src/identity.rs:732-767` + - Platform-address funded: `packages/rs-unified-sdk-jni/src/identity.rs:846-882` + - shielded: `packages/rs-unified-sdk-jni/src/funding.rs:703-748` + +- The Add Contact send path calls `select_own_encryption_key` + (`packages/rs-platform-wallet/src/wallet/identity/network/contact_requests.rs:455-465`), + which requires an enabled ECDSA_SECP256K1 ENCRYPTION key and otherwise emits + the quoted failure (`contact_requests.rs:979-1005`). A freshly registered + Kotlin identity therefore cannot pass that flow today. + +### 1.2 Rich decoder reuse: feasible, but the spec omits required changes + +The proposed parser is the correct *wire-format* reuse target: + +- `DecodedPubkeyRow` and `decode_update_pubkeys_blob` are currently private at + `packages/rs-unified-sdk-jni/src/transactions.rs:290-305`. +- The decoder reads key type, purpose, security level, read-only, bounds kind, + public key, optional contract ID, and optional document type + (`transactions.rs:318-444`). +- Update converts those owned rows into `IdentityPubkeyFFI` while retaining the + owners through the synchronous FFI call (`transactions.rs:242-285`). +- Kotlin's encoder matches that layout + (`packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityUpdates.kt:203-240`). + +However, changing the struct and function to `pub(crate)` alone will not let +`identity.rs` or `funding.rs` build FFI rows: all fields remain private at +`transactions.rs:292-300`. Either expose the fields crate-wide or, preferably, +provide a crate-private conversion method/helper that keeps pointer creation +and owner lifetimes in one place. + +The decoder should also be split into a pure `&[u8]` parser plus the thin JNI +exception adapter. The current function requires `JNIEnv`/`JByteArray` +(`transactions.rs:305-306`), which makes the proposed ordinary Rust round-trip +unit test unnecessarily difficult. + +### 1.3 FFI bounds support: verified + +- `IdentityPubkeyFFI` already contains all rich fields; no struct-layout change + is needed + (`packages/rs-platform-wallet-ffi/src/identity_registration_with_signer.rs:110-127`). +- `decode_contract_bounds` is correctly described as the wrong raw-JNI reuse + target: it is `pub(crate)` in another crate and consumes an already-formed + `&IdentityPubkeyFFI` (`identity_registration_with_signer.rs:129-160`). +- It rejects bounds kind 0 for ENCRYPTION/DECRYPTION + (`identity_registration_with_signer.rs:161-173`) and maps kinds 1/2 at + `:175-225`. +- `decode_identity_pubkeys` validates discriminants, public-key pointers, and + contract bounds before constructing DPP keys + (`identity_registration_with_signer.rs:273-323`). All four Android routes + eventually call it (funded/resume in + `identity_registration_funded_with_signer.rs:63-90,180-220`, address-funded + at `identity_registration_with_signer.rs:434-470`, and shielded at + `shielded_send.rs:663-720`). + +One scope detail is missing: `decode_identity_pubkeys` has a fifth caller, the +Swift invitation claim path +(`packages/rs-platform-wallet-ffi/src/invitation.rs:244-300`). Moving a new +key-0 check into this FFI decoder affects that path too. Invitation already has +the same downstream invariant at +`packages/rs-platform-wallet/src/wallet/identity/network/invitation.rs:127-153`, +so the behavioral change should be benign, but it needs an explicit test and +should not be described as touching only four paths. + +### 1.4 Key ID 0 pre-flight gap: verified, with a placement warning + +- The shared funded/resume implementation rejects an empty map and requires + `keys_map[0]` to be MASTER + AUTHENTICATION + (`packages/rs-platform-wallet/src/wallet/identity/network/registration.rs:134-159`). +- Address-funded registration builds an identity directly from the decoded map + and calls `register_from_addresses` without the same check + (`packages/rs-platform-wallet-ffi/src/identity_registration_with_signer.rs:437-470`). +- Shielded registration converts the decoded map to a vector and proceeds + without it (`packages/rs-platform-wallet-ffi/src/shielded_send.rs:703-720`). + +This is specifically a **key ID 0** invariant, not "row 0" or "the first row." +DPP requires exactly one MASTER but does not require that master's ID to be 0. +The ID convention matters to wallet derivation and loading (`MASTER_KEY_INDEX` +is 0 in +`packages/rs-platform-wallet/src/wallet/identity/network/identity_handle.rs:55`). + +Do not put this assertion directly in `decode_update_pubkeys_blob`: that parser +is used by `updateIdentity` (`transactions.rs:193-196`), and a normal add-key +update neither includes nor should include key 0. Safe options are: + +- validate after decoding in a registration-only JNI wrapper used by the four + registration exports; or +- validate in FFI `decode_identity_pubkeys`, acknowledging/testing the + invitation caller as well. + +### 1.5 DPP structure and proof-of-possession claims: mostly verified + +`validate_identity_public_keys_structure_v0` does enforce: + +- ENCRYPTION = MEDIUM only, DECRYPTION = MEDIUM only, TRANSFER = CRITICAL only + (`packages/rs-dpp/src/state_transition/state_transitions/identity/public_key_in_creation/methods/validate_identity_public_keys_structure/v0/mod.rs:21-37,118-153`); +- duplicate key IDs/data (`:70-95`); +- exactly one MASTER for identity creation (`:98-116`); and +- at most six creation keys via the active platform version (`:49-68` and + `packages/rs-platform-version/src/version/dpp_versions/dpp_state_transition_versions/v1.rs:18`, + with the same value in v2/v3). + +Proof of possession is also server-side, but the spec overgeneralizes the +function name. Standard asset-lock create verifies every key in +`identity_create/identity_and_signatures/v0/mod.rs:20-40`; address-funded create +uses its separate validator in +`identity_create_from_addresses/public_key_signatures/v0/mod.rs:20-42`; and +shielded create checks structure plus every PoP in +`packages/rs-drive-abci/src/execution/validation/state_transition/processor/traits/shielded_proof.rs:384-415`. + +The security conclusion is correct: caller-controlled purpose/security bytes +cannot create an elevated ENCRYPTION/DECRYPTION or invalid TRANSFER key. +However, the statement that a malformed duplicate row list necessarily reaches +DPP and fails is false at the FFI boundary: `decode_identity_pubkeys` inserts +into a `BTreeMap` without checking the previous value +(`identity_registration_with_signer.rs:279-320`), so a later duplicate ID +silently overwrites the earlier row. The surviving key is still structurally +validated, so this does not create the claimed privilege escalation, but the +decoder should reject duplicate IDs explicitly. + +### 1.6 CREATE-time contract-bounds semantics: the spec is only partly right + +The narrow call-graph claim is true: the explicit +`validate_identity_public_keys_contract_bounds` state validator is called by +identity UPDATE +(`packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_update/state/v0/mod.rs:97-110`), +not by standard or address identity CREATE. Those CREATE advanced-structure +paths call structural validation and PoP only +(`identity_create/advanced_structure/v0/mod.rs:80-116` and +`identity_create_from_addresses/advanced_structure/v0/mod.rs:34-71`). Shielded +CREATE likewise has only the structure/PoP checks cited above. + +The stronger conclusion in the spec is not true. CREATE execution stores the +new identity through `create_key_tree_with_keys_operations` +(`packages/rs-drive/src/drive/identity/insert/add_new_identity/v0/mod.rs:259-270`). +Each key insertion calls +`add_potential_contract_info_for_contract_bounded_key` +(`packages/rs-drive/src/drive/identity/key/insert/create_key_tree_with_keys/v0/mod.rs:98-123`). +That path: + +- fetches the referenced contract and errors if it does not exist + (`packages/rs-drive/src/drive/identity/contract_info/keys/mod.rs:62-86`); +- resolves the named document type (`keys/mod.rs:94-105`); and +- requires that document type to declare the matching ENCRYPTION/DECRYPTION + bounded-key requirement + (`packages/rs-drive/src/drive/identity/contract_info/keys/add_potential_contract_info_for_contract_bounded_key/v0/mod.rs:430-465`). + +Therefore an arbitrary nonexistent contract ID, missing document type, or +document without the relevant requirement cannot simply register +"successfully." A wrong but existing compatible contract/document could still +be accepted and indexed under the wrong scope, so pinning the constant remains +valuable. + +It also would not recreate the *exact* selector failure described in the spec. +`select_own_encryption_key` tests only enabled/type/purpose, not bounds +(`contact_requests.rs:989-1005`), and contact-request validation intentionally +accepts bound or unbound ENCRYPTION keys +(`packages/rs-platform-wallet/src/wallet/identity/crypto/validation.rs:111-121,164-167`). +The contact document is built from the canonical DashPay contract fetched +separately (`packages/rs-sdk/src/platform/dashpay/contact_request.rs:341-349`). +Consequently, a manual Add Contact success is not proof that the new keys carry +the correct contract ID/document bounds. + +### 1.7 Kotlin and Swift claims: verified with two material qualifications + +- `IdentityUpdates.kt` already has the desired `IdentityPubkey`, enums, and + bounds model (`:12-136`). `encodeAddPubkeys` is private at `:211`, and its + helper `contractBoundsKind` is also private at `:243-248`; both need factoring + if the encoder is shared. +- The Kotlin DashPay entry is private and its 32 bytes match + `packages/dashpay-contract/src/lib.rs:9-16` + (`packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/identity/AddIdentityKeyScreen.kt:381-397`). +- Swift's helper creates consecutive ENCRYPTION/DECRYPTION ECDSA keys at MEDIUM + with DashPay `contactRequest` bounds, validates private/public correspondence, + persists each private key, and returns rich rows + (`packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Services/IdentityRegistrationKeys.swift:15-121`). + +Qualification 1: the duplicate helper in `CreateIdentityView.swift` is a mirror, +not literally byte-for-byte "verbatim." That is editorial, not architectural. + +Qualification 2: Swift deliberately excludes unused-asset-lock/resume from +DashPay provisioning (`CreateIdentityView.swift:152-177,896-905`). It also +raises the minimum funding according to total key count (`:127-149`). The Kotlin +flow proposes six keys on all four routes, including resume, but Kotlin's +`TrackedAssetLock` omits the Rust lock's `amount` +(`packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/TrackedAssetLock.kt:11-19` +versus `packages/rs-platform-wallet/src/wallet/asset_lock/tracked.rs:63-72`). +The two added keys cost 13,000 additional duffs at the current 6,500,000-credit +per-key fee (`packages/rs-platform-version/src/version/fee/state_transition_min_fees/v1.rs:17`). + +The spec must decide whether resume excludes the pair (matching Swift) or gains +an amount-aware sufficiency check/error. A previously built lock funded only +for four keys may fail a six-key create. + +### 1.8 Deletion/caller audit: safe, but clean up related documentation + +A repository-wide search found executable references to +`role_for_registration_key_id`/`decode_pubkeys_blob` only at the four JNI sites +listed above plus the `funding.rs:42` import. No test calls either function, so +deletion after replacement is safe. The six local role constants at +`identity.rs:79-90` then become dead as well. + +The change should also update stale wire/positional-role documentation at: + +- `packages/rs-unified-sdk-jni/src/identity.rs:258-270,682-697,813-815` +- `packages/rs-unified-sdk-jni/src/funding.rs:650-660` +- `packages/rs-platform-wallet-ffi/src/identity_key_preview.rs:77-94,257-280` +- `packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/IdentityNative.kt:148-178` +- `packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityRegistration.kt:191-210` + +## 2. Scope and compatibility assessment + +### What is contained + +- No consensus rule or serialized state-transition format needs changing. +- No C `IdentityPubkeyFFI` field/layout change is needed. +- No Room/database migration is needed; the existing identity-key persistence + model already supports purpose/security/bounds. +- Separating this from Keystore invalidation/documentation follow-ups is a good + rollback boundary: this change spans the registration wire codec and key + provisioning, not storage invalidation. + +### Hidden scope/risk to make explicit + +1. **Opaque wire ABI.** The `byte[]` payload changes from + `count + (id,len,pubkey)` to the rich update layout. JNI method descriptors + remain identical, but an old Kotlin artifact paired with a new `.so` (or the + reverse) will be misparsed/rejected. Update both sides and their KDocs in one + release. A version/magic byte would make skew diagnosable; if that is deemed + too broad, at least reject legacy/trailing input deterministically. +2. **Public Kotlin API.** `IdentityKeyPreview` is public + (`IdentityKeyPreview.kt:17-22`), and public registration methods accept + `List` (`IdentityRegistration.kt:117-146,237-291` and + `PlatformWalletManager.kt:1344-1371`). Replacing that type, or adding fields + to its data-class constructor, has source/binary/behavior compatibility + implications for SDK consumers. Specify the compatibility strategy rather + than treating it as an app-only row refactor. +3. **Six-key ceiling.** Base four plus the pair equals, rather than merely stays + below, `max_public_keys_in_creation = 6`. Add a local assertion/test and + document that any future expansion of the base set requires redesign. +4. **Existing identities.** This fixes only freshly registered identities. + Existing Android-created identities remain without the pair and still need + the Add Identity Key repair flow. State explicitly that no backfill is in + scope, or add product guidance/detection if the intended outcome covers + existing users. +5. **Existing Kotlin reuse.** The app already has derive -> validate -> persist + -> zero -> `IdentityPubkey` plumbing in + `packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/services/IdentityKeyAdditionFlow.kt:84-182`, + used by `AddIdentityKeyScreen.kt:324-349`. Before introducing a separate + `DashpayKeyProvisioning`, decide whether a thin policy wrapper around this + helper is sufficient. Duplicating the lifecycle is another drift surface. +6. **Keypair validation parity.** Swift explicitly verifies the derived private + scalar matches the public key (`IdentityRegistrationKeys.swift:66-82`). The + current Kotlin registration preview path persists without this check + (`CreateIdentityScreen.kt:202-235`). This is existing behavior, not caused by + the pair, but a claimed Swift port should either add the defense or document + the intentional parity gap. + +## 3. Test-plan review and recommended additions + +The device/testnet Add Contact smoke remains useful as an end-to-end product +check, but it is not enough and should not be the only integration evidence. +The following can be automated: + +1. **One cross-language golden blob.** Have a Kotlin unit test encode six + canonical rows to exact bytes, and have a pure Rust `&[u8]` parser test decode + the same checked-in fixture. Independent Kotlin-object and Rust-object tests + do not catch byte-order/field-order skew. +2. **Strict Rust codec tests.** Cover both bounds kinds, truncation at each + variable field, invalid bounds kind, interior NUL, negative key ID, invalid + boolean byte, duplicate ID, trailing bytes, and explicit legacy-format + rejection. The current parser accepts every nonzero `read_only` byte as true + and ignores trailing bytes (`transactions.rs:369-444`); decide and test a + strict policy while this becomes a registration boundary. +3. **Registration-only invariant test.** Assert key ID 0 (regardless of row + order) is MASTER + AUTHENTICATION; missing/wrong ID 0 fails. Add a separate + regression proving identity-update add lists without key 0 remain accepted. +4. **FFI decoder test.** Verify duplicate IDs are rejected rather than + overwritten and ENCRYPTION/DECRYPTION without bounds are rejected. If the + key-0 check lives here, include the invitation caller behavior. +5. **Kotlin provisioning/lifecycle test.** Assert exact IDs 4/5, ECDSA type, + ENCRYPTION/DECRYPTION purposes, MEDIUM level, `readOnly=false`, exact + contract/document bounds, correct derivation indices, storage under the + matching public bytes, and private-array zeroing on success and failure. +6. **Four-way orchestration test.** Extract a small pure preparation/dispatch + seam from `CreateIdentityScreen` and verify all selected funding sources + receive the same rich six-row set. This catches omission at a call site more + cheaply than four JNI/JVM environments. +7. **On-chain bounds assertion.** After a local/testnet create, fetch the + identity and assert IDs 4/5 carry the exact DashPay ID and `contactRequest` + bounds (or query the contract-bound key index). Add Contact success alone + does not test those fields. +8. **Resume funding case.** Test the chosen policy: either four keys for resume, + or a clear early failure/success boundary based on the lock's available + amount for six keys. +9. **Canonical constant test mechanism.** The proposed "Rust test" cannot + compare a private Kotlin constant to `dashpay_contract::ID_BYTES`; Rust + cannot see Kotlin source, and `rs-unified-sdk-jni` does not currently depend + directly on `dashpay-contract`. Use generated Kotlin source/resource, a + shared checked-in cross-language golden fixture, or a Kotlin test that + consumes an independently generated canonical value. Merely copying the + same 32-byte literal into both sides is the tautology the spec wants to + avoid. + +Existing Kotlin tests also need mechanical adjustment if the registration row +type/signature changes: `IdentityAssetLockRecoveryTest.kt:169` constructs the +legacy `IdentityKeyPreview` used by resume tests. + +## 4. Verification-plan corrections + +- `cargo test -p rs-platform-wallet-ffi --lib` is invalid: the package is named + `platform-wallet-ffi` (`packages/rs-platform-wallet-ffi/Cargo.toml:1-2`). The + correct command is `cargo test -p platform-wallet-ffi --lib`. +- Keep targeted tests first, then the proposed workspace clippy/format and + Gradle suites. If production logic is added below the FFI crate, include the + directly affected wallet/DPP/Drive package tests rather than relying only on + JNI and FFI tests. +- Baseline source-head results from this review: + + - `cargo test -p rs-unified-sdk-jni --lib`: 10 passed, 0 failed. + - `cargo test -p platform-wallet-ffi --lib`: 196 passed, 0 failed. + - The spec's uncorrected `cargo test -p rs-platform-wallet-ffi --lib` + invocation fails before testing because no package has that name. + +The source audit did not modify production code or the reviewed spec. + +## 5. Recommended spec edits before implementation + +1. Specify a pure shared rich parser plus a safe FFI-row conversion API; mention + row-field visibility/ownership explicitly. +2. State exactly where registration-only key-ID-0 validation lives and protect + the update path with a regression test. +3. Rewrite the CREATE-bounds risk: no explicit consensus validator, but Drive + insertion resolves and checks bounds; the residual risk is a wrong *existing + compatible* scope and incorrect indexing, not necessarily Add Contact + selector failure. +4. Decide the resume policy and account for the two-key fee plus six-key ceiling. +5. Document the logical JNI blob compatibility and public Kotlin API strategy. +6. Replace the constant-pin and round-trip bullets with an executable + cross-language fixture plan and an on-chain bounds assertion. +7. Add duplicate-ID rejection, strict parser behavior, existing-identity + non-migration, stale-doc cleanup, and the existing `IdentityKeyAdditionFlow` + reuse decision to scope. diff --git a/docs/sdk/KOTLIN_SDK_DASHPAY_REGISTRATION_KEYS_SPEC.md b/docs/sdk/KOTLIN_SDK_DASHPAY_REGISTRATION_KEYS_SPEC.md new file mode 100644 index 0000000000..d499818579 --- /dev/null +++ b/docs/sdk/KOTLIN_SDK_DASHPAY_REGISTRATION_KEYS_SPEC.md @@ -0,0 +1,384 @@ +# Kotlin SDK — DashPay registration-key provisioning spec + +**Status:** REVIEWED (2 rounds) — feasibility/security pass, then an +independent codex cross-check with a much deeper call-graph trace; several +material corrections applied. The resume-path funding question is resolved +by the parity mandate itself (§0.2); one small implementation-detail +decision remains (§0.3) +**Branch:** own PR/worktree, stacked on PR #3999's `feat/kotlin-sdk-and-example-app` +head (split out from the sibling Keystore/docs spec — different subsystem, +own rollback unit, per the repo's own PR-slicing convention at +`KOTLIN_SWIFT_SHARED_PARITY_SPEC.md:508`) +**Scope:** `packages/rs-unified-sdk-jni`, `packages/rs-platform-wallet-ffi`, +`packages/rs-platform-wallet`, `packages/kotlin-sdk`, +`packages/kotlin-sdk/KotlinExampleApp` + +## 0. Spec review findings + +### 0.1 Round 1 (feasibility + security) + +1. **Factual error, corrected: `decode_contract_bounds` is NOT usable from + the JNI decoder** (wrong crate, `pub(crate)`-scoped, operates on an + already-parsed struct not the raw blob). Real target: + `decode_update_pubkeys_blob` + `DecodedPubkeyRow` + (`rs-unified-sdk-jni/src/transactions.rs:291,305`) — **superseded by + §0.2 item 1 below**, which found promoting these alone is still + insufficient. +2. Confirmed: no protocol/FFI-struct change needed — `IdentityPubkeyFFI` + already carries the needed fields. +3. `IdentityUpdates.kt`'s `encodeAddPubkeys` (and its helper + `contractBoundsKind`) are `private` — need hoisting. +4. Security LOW: contract-bounds not validated at CREATE time — + **corrected by §0.2 item 3 below**, which found this claim was + overstated. +5. Security LOW: row-0-MASTER invariant path-dependent — + **corrected by §0.2 item 2 below**: it's a key-ID-0 invariant, not a + row-position invariant, and the fix location matters (must not touch + the shared update decoder). +6. Confirmed safe: security-level/purpose escalation not exploitable + server-side — **reconfirmed by round 2**, still holds. +7. Confirmed: no test references the functions being deleted — safe to + remove. + +### 0.2 Round 2 (independent codex cross-check — supersedes several round-1 items) + +A second, independent review re-derived every claim from source rather than +trusting the first draft, and found the design still has real gaps: + +1. **MAJOR — promoting the decoder alone doesn't make it usable.** + `DecodedPubkeyRow`'s fields are private + (`transactions.rs:291-301`) — making the struct and function + `pub(crate)` doesn't let `identity.rs`/`funding.rs` read the fields to + build `IdentityPubkeyFFI` rows. Need field accessors, a crate-private + `into_ffi`-style conversion helper, or move the row+decoder into a + shared module. Additionally, the current decoder takes `JNIEnv`/ + `JByteArray` directly (`transactions.rs:305-306`) — split it into a + pure `&[u8]` parser plus a thin JNI adapter, or the proposed Rust + round-trip unit tests can't be written as ordinary Rust tests. +2. **MAJOR — the invariant is "key ID 0", not "row 0", and must NOT go in + the shared update decoder.** `decode_update_pubkeys_blob` is also used + by `TransactionsNative.updateIdentity` (`transactions.rs:193-198`), and + an ordinary add-key update legitimately has no key ID 0 at all — adding + an unconditional key-0 check there would break normal key-addition + updates. Put the invariant in a registration-only seam instead: + `rs-platform-wallet-ffi::decode_identity_pubkeys` + (`identity_registration_with_signer.rs:273-323`) or a registration-only + JNI wrapper. **Also: `decode_identity_pubkeys` has a 5th caller the + original spec missed** — Swift's invitation-claim path + (`rs-platform-wallet-ffi/src/invitation.rs:244-300`). Moving the + key-0 check here affects that path too; it already has the same + downstream invariant separately + (`rs-platform-wallet/src/wallet/identity/network/invitation.rs:127-153`) + so the change should be behaviorally benign, but needs its own explicit + test, and the spec must stop saying "four paths." +3. **MAJOR — the CREATE-time-bounds-validation claim was overstated in the + wrong direction.** It's true there's no explicit + `validate_identity_public_keys_contract_bounds` call on the CREATE path + (that validator is UPDATE-only, + `rs-drive-abci/.../identity_update/state/v0/mod.rs:97-110`). But Drive's + actual key-insertion path (`create_key_tree_with_keys_operations` → + `add_potential_contract_info_for_contract_bounded_key`, + `rs-drive/src/drive/identity/key/insert/create_key_tree_with_keys/v0/mod.rs:98-123`) + **does** fetch the referenced contract, error if it doesn't exist, + resolve the document type, and require that type to declare the + matching bounded-key requirement + (`rs-drive/.../add_potential_contract_info_for_contract_bounded_key/v0/mod.rs:430-465`). + So "any wrong constant registers successfully" is **false** — a + nonexistent contract or missing document type fails registration. The + real residual risk is narrower: a wrong-but-*existing-and-compatible* + contract/doc-type would still be silently accepted under the wrong + scope. Test plan corrected accordingly (§4). + **Additional correction**: manual Add Contact success is not proof the + bounds are actually correct — `select_own_encryption_key` only checks + enabled/type/purpose, not bounds, and contact-request validation + intentionally accepts bound *or* unbound ENCRYPTION keys + (`rs-platform-wallet/src/wallet/identity/crypto/validation.rs:111-121, + 164-167`). Add an explicit on-chain bounds assertion to the test plan — + a passing Add Contact smoke does not cover this. +4. **MAJOR — my plan never specified how the BASE 4 keys' roles get + rebuilt after `role_for_registration_key_id` is deleted.** That function + is the *only* current place IDs 0-3 get their roles (MASTER/CRITICAL/ + HIGH/TRANSFER) assigned. The original approach only described stamping + the 2 new DashPay keys — it left the base 4 keys' rich-row construction + completely unspecified. Fixed in §2 approach below: the Kotlin side must + build rich rows for ALL 6 keys (0-5), not just the 2 new ones. +5. **MAJOR — "wire into all 4 funding paths" is not Swift parity and misses + a funding constraint.** Swift deliberately **excludes** the + unused-asset-lock resume path from DashPay provisioning + (`CreateIdentityView.swift:152-177,896-905`) and **raises the minimum + registration funding** based on the added key count (`:127-149`). Adding + 2 keys costs an additional 13,000 duffs at the current per-key fee + (`rs-platform-version/.../fee/state_transition_min_fees/v1.rs:17`) and + reaches exactly `max_public_keys_in_creation = 6` — the protocol + ceiling, not headroom under it. Kotlin's `TrackedAssetLock` doesn't even + expose the underlying lock's `amount` field + (`TrackedAssetLock.kt:11-19` vs. Rust's `tracked.rs:63-72`), so there's + currently no way to check whether a previously-created (four-key-sized) + lock has enough value for a six-key create. **Resolved, not an open + decision (see §0.2 resolution below): this is exactly the problem iOS's + own exclusion already solves — match it, don't re-derive a Kotlin- + specific answer.** +6. **Medium/high — `decode_identity_pubkeys` silently last-wins on + duplicate key IDs**, not fail-closed as claimed: it inserts into a + `BTreeMap` with no check for an existing entry + (`identity_registration_with_signer.rs:273-323`). The surviving key is + still structurally validated (not a privilege escalation), but explicit + duplicate-ID rejection should be added to the decoder. +7. **Medium/high — the proposed contract-ID pin test is impossible as + described.** A Rust test in `rs-unified-sdk-jni` cannot read a private + Kotlin source constant to compare against + (`AddIdentityKeyScreen.kt:382-397` vs. + `packages/dashpay-contract/src/lib.rs:9-16`) — there's no cross-language + visibility. Use a shared generated/checked-in golden fixture instead + (§4). +8. **New scope items the original spec didn't consider at all:** + - **Wire-format skew risk**: the JNI method descriptors stay identical, + but the opaque `byte[]` payload shape changes incompatibly. An old + Kotlin artifact paired with a new native `.so` (or vice versa) would + be silently misparsed, not rejected. At minimum, reject legacy-shaped + or trailing/malformed input deterministically; a version/magic byte + would make skew diagnosable if that's judged worth the extra + complexity. + - **Public Kotlin API compatibility**: `IdentityKeyPreview` is public + (`IdentityKeyPreview.kt:17-22`) and public registration methods accept + `List` (`IdentityRegistration.kt:117-146,237-291`, + `PlatformWalletManager.kt:1344-1371`). Replacing or extending this + type has source/binary compatibility implications for SDK consumers + — needs an explicit compatibility strategy, not treatment as an + app-only refactor. + - **No backfill for existing identities**: this only fixes freshly + registered identities. Existing Android-created identities remain + without the DashPay pair and still need the existing Add Identity Key + repair flow. State this explicitly as non-goal, or add detection/ + guidance if backfill is actually wanted. + - **Existing reusable plumbing**: `KotlinExampleApp` already has a + derive → validate → persist → zero → `IdentityPubkey` pipeline in + `IdentityKeyAdditionFlow.kt:84-182` (used by `AddIdentityKeyScreen.kt`). + Before writing a parallel `DashpayKeyProvisioning` helper, decide + whether a thin policy wrapper around this existing flow is sufficient + — duplicating the lifecycle is another drift surface. + - **Keypair-correspondence validation gap**: Swift explicitly verifies + the derived private scalar matches the public key before persisting + (`IdentityRegistrationKeys.swift:66-82`); the current Kotlin + registration-preview path doesn't do this check at all + (`CreateIdentityScreen.kt:202-235`). Pre-existing, not caused by this + change, but a "ported from Swift" claim should either add the same + defense or explicitly document the parity gap. + - **Secret lifecycle risk**: deriving `base + 2` while also describing + "the existing base list plus an appended pair" risks deriving the + first four private-key byte arrays twice with a discarded, unzeroed + copy. Since proof-of-possession signs every submitted key, losing a + private half after the corresponding pubkey is registered makes that + key permanently, silently unusable. Spec must be explicit: one + six-key preview derivation, not two overlapping ones, with every + preview/private array (including on failure paths) zeroed. +9. **Verification command was wrong.** `cargo test -p rs-platform-wallet-ffi + --lib` fails immediately — the crate is named `platform-wallet-ffi` + (`rs-platform-wallet-ffi/Cargo.toml:1-2`), confirmed by actually running + both commands (196 tests pass under the correct name). Corrected in §5. + +### 0.2 Resolved: resume-path policy + +**Not actually an open decision — resolved by the project's own parity +mandate, no Kotlin-specific design call needed.** iOS resume finishes an +interrupted registration with exactly the key set it originally started +with and never changes that set mid-resume; it does not attempt to +determine or verify whether the pre-existing lock is large enough for a +bigger key set, because it simply never asks the lock to cover more than +what it already committed to. That's iOS's own answer to the exact +fund-safety problem in §0.2 item 5 above (a resumed lock is a fixed, +already-spent-on-chain amount; retroactively growing the transition it +funds risks a resume that fails after the user's DASH is already +irreversibly locked). Since this whole PR exists to port Swift's behavior, +not invent new Kotlin policy, the answer is simply: **leave the resume path +alone.** Do not extend DashPay provisioning to +`resumeIdentityWithExistingAssetLock` at all — it keeps using the base +4-key set exactly as it does today. A user who resumes registration and +wants DashPay capability gets it afterward through the existing Add +Identity Key flow, same as iOS. This requires **zero new FFI surface** +(no `TrackedAssetLock.amount` exposure needed) and removes an entire +branch of scope from this PR rather than adding one. + +### 0.3 Remaining open decision + +- **`DashpayKeyProvisioning` vs. reusing `IdentityKeyAdditionFlow`** + (§0.2 item 8): recommend evaluating the existing flow first; only write + a new helper if it genuinely can't be adapted (e.g. its persistence + timing assumes post-registration, not pre-registration). + +## 1. Problem + +`role_for_registration_key_id` (`rs-unified-sdk-jni/src/identity.rs:122-131`) +reconstructs purpose/security-level *positionally* from a bare `keyId`, and +every registration call site hardcodes `contract_bounds_kind: 0`. The wire +format all 4 sites decode (`decode_pubkeys_blob`, `identity.rs:933`) carries +only `keyId` + pubkey bytes. Consequently, an identity created by the Android +app reaches its own advertised Add Contact flow without an enabled +ECDSA_SECP256K1 encryption key, and `select_own_encryption_key` rejects the +request (confirmed: `contact_requests.rs:455-465,979-1005`). + +Swift's reference flow (`IdentityRegistrationKeys.swift::makeDashpayKeyPair`) +derives 2 extra keys bounded to DashPay's `contactRequest` document type and +appends them before dispatching to registration — for eligible fresh-funding +paths only, explicitly excluding resume (§0.2 item 5). This is Swift +application-level code, not shared Rust. + +## 2. Approach + +1. **Rust — decoder.** Extract a pure `&[u8]` parser from + `decode_update_pubkeys_blob`'s logic (§0.2 item 1), with a + crate-private conversion path from the parsed rows to `IdentityPubkeyFFI` + that exposes what registration needs (not just `pub(crate)` on an + already-private-fielded struct). Keep the existing JNI-facing function as + a thin adapter over the pure parser for the update path; add a + registration-facing entry point that uses the same pure parser plus a + **registration-only** key-ID-0 = MASTER+AUTHENTICATION check and + explicit duplicate-key-ID rejection (§0.2 items 2, 6) — do NOT add + either check to the shared update-path decoder. Reject trailing/ + malformed bytes deterministically in the same pass (§0.2 item 8). +2. **Rust — call sites.** Call from all 4 registration JNI sites + (`identity.rs:609/753/868`, `funding.rs:734`) **and** account for the + 5th caller of `decode_identity_pubkeys`, the invitation-claim path + (`invitation.rs:244-300`) — add explicit test coverage there too rather + than silently changing its behavior (§0.2 item 2). Delete + `role_for_registration_key_id` and `decode_pubkeys_blob` (confirmed + dead — no test references) along with the now-dead local role constants + (`identity.rs:79-90`). +3. **Kotlin — base + DashPay rows, not just DashPay rows.** Build rich rows + for the **complete** 6-key set in one derivation pass — base 4 (IDs 0-3, + replicating today's MASTER/CRITICAL/HIGH/TRANSFER roles explicitly, since + `role_for_registration_key_id` no longer exists to do it implicitly) plus + the 2 DashPay keys (IDs 4-5, ENCRYPTION/DECRYPTION, MEDIUM, DashPay + `SingleContractDocumentType` bounds) — not two separate/overlapping + derivations (§0.2 item 8, secret lifecycle). Hoist `encodeAddPubkeys` + + `contractBoundsKind` out of `IdentityUpdates` into a shared location. + Hoist the DashPay contract-id constant out of `AddIdentityKeyScreen.kt` + into one shared location instead of a third copy. Evaluate reusing + `IdentityKeyAdditionFlow.kt`'s existing derive→validate→persist→zero + pipeline before writing a new parallel helper (§0.3 remaining decision). +4. **Kotlin — funding paths.** Wire the six-key set into fresh Core-funded, + Platform-address-funded, and shielded registration. **Exclude + unused-asset-lock resume** from DashPay provisioning entirely, matching + Swift (§0.2, resolved — not a design choice, just parity) — resume + continues to use the base 4-key set as today; a user recovering via + resume can add DashPay keys afterward through the existing Add Identity + Key flow. Document this explicitly as intentional, not an oversight. +5. **No backfill.** Existing already-registered identities are explicitly + out of scope (§0.2 item 8) — they keep using the existing Add Identity + Key repair flow, unchanged by this PR. +6. **Stale documentation.** Update positional-role wording that becomes + incorrect once `role_for_registration_key_id` is gone: + `identity.rs:258-270,682-697,813-815`, `funding.rs:650-660`, + `identity_key_preview.rs:77-94,257-280`, `IdentityNative.kt:148-178`, + `IdentityRegistration.kt:191-210`. + +## 3. Failure modes (as reviewed, 2 rounds) + +- Wire-format skew between an old Kotlin artifact and a new native library + (or vice versa) would silently misparse rather than fail loud — mitigated + by deterministic rejection of malformed/trailing/legacy-shaped input + (§0.2 item 8); a version/magic byte is a further option if judged + worthwhile. +- A wrong-but-*existing-and-compatible* DashPay contract/doc-type constant + would still register successfully under the wrong scope (narrower than + originally claimed — Drive DOES reject a nonexistent contract/doc-type, + §0.2 item 3) — mitigated by pinning the constant against a genuine + cross-language golden fixture (§4), not a same-literal echo. +- Duplicate key IDs in a caller-supplied row list silently collapse to + last-wins in `decode_identity_pubkeys`'s `BTreeMap` — mitigated by + explicit duplicate-ID rejection (§0.2 item 6). +- Key-ID-0 invariant, if misplaced in the shared update decoder, would + break ordinary key-addition updates that don't include ID 0 — mitigated + by keeping it registration-only (§0.2 item 2). +- A previously-created four-key-sized unused asset lock is excluded from + six-key resume entirely (§0.2, resolved by parity — resume never grows + past what it originally committed to), so no insufficient-funds failure + mode exists there by construction; fresh funding paths need the six-key + minimum reflected in their fee/sufficiency calculation. +- Security-level/purpose escalation via a malformed row list — confirmed + not exploitable; `validate_identity_public_keys_structure_v0` rejects the + whole transition server-side regardless of what Kotlin sends. +- Deriving the base 4 keys' private material twice (once for preview, once + overlapping with DashPay-key derivation) risks an unzeroed discarded + copy — mitigated by one single six-key derivation pass (§2 point 3). + +## 4. Test plan (red → green, per repo TDD discipline) + +- **Rust (red first), pure-parser round trip**: encode a row list including + the full 6-key policy (base 4 + bounded ENCRYPTION/DECRYPTION), decode via + the new pure `&[u8]` parser (§0.2 item 1 — required for this test to be + an ordinary Rust unit test, not JNI-environment-dependent), assert every + field round-trips. Write against the CURRENT decoder first to confirm no + such fields exist today. +- **Rust, cross-language golden fixture** (§0.2 item 7, replacing the + infeasible constant-pin approach): check in one fixture — 6 canonical + rows encoded to exact bytes. A Kotlin unit test asserts its encoder + produces those exact bytes; a Rust unit test asserts its parser decodes + those exact bytes to the expected fields. This catches byte-order/ + field-order skew that two independently-written, independently-passing + tests would miss. +- **Rust, strict codec edge cases**: both bounds kinds, truncation at each + variable-length field, invalid bounds-kind byte, interior NUL, negative + key ID, invalid boolean byte for `readOnly`, duplicate key ID (must + reject, not last-wins), trailing bytes (must reject), and explicit + legacy-format rejection. +- **Rust, registration-only invariant test**: key ID 0 must be + MASTER+AUTHENTICATION regardless of row order; missing/wrong ID 0 fails. + **Separately**, a regression test proving identity-UPDATE add-key lists + *without* key ID 0 remain accepted (proves the invariant didn't leak into + the shared decoder). +- **Rust, FFI decoder test**: duplicate IDs rejected (not silently + overwritten); ENCRYPTION/DECRYPTION without bounds rejected (existing + behavior, confirm still true after the refactor). If the key-0 check + lives in `decode_identity_pubkeys`, include a test for the invitation + caller's behavior too (§0.2 item 2). +- **Kotlin (red first), provisioning/lifecycle test**: assert exact IDs 0-5 + with correct type/purpose/securityLevel/readOnly/bounds for all 6 (not + just the 2 new ones), correct derivation indices, storage under matching + public bytes, and private-array zeroing on both success and failure + paths. Write against the stub/nonexistent helper first to confirm no such + rows are produced today. +- **Kotlin, four-way orchestration test**: extract a small pure + preparation/dispatch seam from `CreateIdentityScreen` and verify every + eligible funding source receives the identical rich 6-row set, and that + resume does NOT (per the resume-exclusion policy) — cheaper than + exercising all 4 JNI/JVM paths directly. +- **On-chain bounds assertion** (§0.2 item 3 — Add Contact success alone + does not prove this): after a local/testnet create, fetch the identity + and assert IDs 4/5 carry the exact DashPay contract ID and `contactRequest` + document-type bounds. +- **Resume-funding case**: test the chosen policy explicitly — resume uses + the base 4-key set only (recommended), or an amount-aware six-key + sufficiency check if policy (b) is chosen instead. +- **Manual/device (environment-bound)**: register a fresh identity via + KotlinExampleApp on testnet, confirm Add Contact now succeeds. This + remains useful as an end-to-end product check but is explicitly NOT + sufficient on its own (§0.2 item 3) — the on-chain bounds assertion above + is the actual correctness proof. +- Existing test needing mechanical update: `IdentityAssetLockRecoveryTest.kt:169` + constructs the legacy `IdentityKeyPreview` shape used by resume tests — + will need updating if the registration row type changes shape (even + though resume itself stays on the base 4-key policy). + +## 5. Verification plan + +```bash +cargo test -p rs-unified-sdk-jni --lib +cargo test -p platform-wallet-ffi --lib # NOT rs-platform-wallet-ffi — that name doesn't exist (§0.2 item 9) +cargo clippy --workspace --all-features +cargo fmt --check --all + +cd packages/kotlin-sdk +JAVA_HOME=/opt/homebrew/opt/openjdk@17 ./gradlew \ + :sdk:assembleDebug :sdk:testDebugUnitTest \ + :app:assembleDebug :app:testDebugUnitTest \ + :sdk:compileDebugAndroidTestKotlin +``` + +Baseline (pre-change) confirmed by the round-2 review: +`cargo test -p rs-unified-sdk-jni --lib` → 10 passed; +`cargo test -p platform-wallet-ffi --lib` → 196 passed. + +Manual/device-bound: testnet identity registration + Add Contact flow smoke, +**plus** the on-chain bounds assertion above (Add Contact success alone is +not sufficient evidence per §0.2 item 3). From 3a64940af804ab18bf210216cb5476ef129a0593 Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Tue, 21 Jul 2026 06:27:01 +0700 Subject: [PATCH 02/10] feat(kotlin-sdk): rich registration key-row wire format (Rust) The four Android identity-registration JNI paths (Core-funded, resume, Platform-address, shielded-from-pool) previously sent a bare (keyId, pubkey) blob and reconstructed each key's DPP role positionally in Rust via role_for_registration_key_id. That layout can't carry the contract bounds a DashPay ENCRYPTION/DECRYPTION key needs, so a freshly registered identity had no encryption key and its own Add Contact flow rejected it (select_own_encryption_key). Switch registration to the same rich key-row wire format the identity add-key update path already uses: - Extract a pure &[u8] parser (parse_pubkey_rows) into a new pubkey_rows module, shared by the update and registration seams and unit-testable without a JVM. It is strict: truncation, trailing bytes, invalid role/bounds/boolean discriminants, interior-NUL doc types, and negative key IDs are all rejected (wire-skew detection). - decode_registration_pubkeys_blob layers the registration-only invariants (>=1 key, no duplicate key IDs, key ID 0 = MASTER+AUTHENTICATION found by id not row position) on top; the shared update decoder gets neither, so ordinary add-key updates without key 0 still parse. - DecodedPubkeyRow::to_ffi keeps FFI pointer creation + the borrow contract in one place; every registration call site keeps the owning Vec alive across the FFI call. - Add explicit duplicate-key-id rejection to the shared FFI decode_identity_pubkeys (its BTreeMap previously last-wins silently), which also covers its fifth caller, the invitation-claim path. - Delete role_for_registration_key_id, decode_pubkeys_blob, and the dead role constants; refresh the now-stale positional-role rustdoc. A checked-in golden fixture (shared byte-for-byte with the Kotlin encoder test) pins the wire format cross-language; the Rust test asserts the DashPay-bound rows carry the real dashpay_contract::ID_BYTES. rs-unified-sdk-jni --lib: 28 passed (was 10). platform-wallet-ffi --lib: 199 passed (was 196). Co-Authored-By: Claude Opus 4.8 --- Cargo.lock | 1 + .../golden/registration_pubkeys_v1.bin | Bin 0 -> 364 bytes .../src/identity_key_preview.rs | 17 +- .../src/identity_registration_with_signer.rs | 83 ++ packages/rs-unified-sdk-jni/Cargo.toml | 3 + packages/rs-unified-sdk-jni/src/funding.rs | 38 +- packages/rs-unified-sdk-jni/src/identity.rs | 268 +------ packages/rs-unified-sdk-jni/src/lib.rs | 1 + .../rs-unified-sdk-jni/src/pubkey_rows.rs | 743 ++++++++++++++++++ .../rs-unified-sdk-jni/src/transactions.rs | 180 +---- 10 files changed, 886 insertions(+), 448 deletions(-) create mode 100644 packages/kotlin-sdk/sdk/src/test/resources/golden/registration_pubkeys_v1.bin create mode 100644 packages/rs-unified-sdk-jni/src/pubkey_rows.rs diff --git a/Cargo.lock b/Cargo.lock index 4d248da745..07f6cde1e1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6448,6 +6448,7 @@ dependencies = [ "android_logger", "dash-network", "dashcore", + "dashpay-contract", "jni 0.21.1", "key-wallet-ffi", "log", diff --git a/packages/kotlin-sdk/sdk/src/test/resources/golden/registration_pubkeys_v1.bin b/packages/kotlin-sdk/sdk/src/test/resources/golden/registration_pubkeys_v1.bin new file mode 100644 index 0000000000000000000000000000000000000000..82bbb3c3bf14eb1406cd67234b39ba4d52677dd5 GIT binary patch literal 364 zcmZQzU|?f_14Sl60H~A^j2IM|2?3y5CNN@9WFZ8AYMB|BfkrbZvJwJ7MJx=A%s|5x z*$9C}3%9Jvf3NgPODf}qkeSQo@b>s@M;!@)NsR~Ho^LAL!@!rEpI4HYToRO8SejZ~ R0yLYIfeCCjJ0U=-*#Q64E-L^4 literal 0 HcmV?d00001 diff --git a/packages/rs-platform-wallet-ffi/src/identity_key_preview.rs b/packages/rs-platform-wallet-ffi/src/identity_key_preview.rs index 6d02c2a454..cecc569c05 100644 --- a/packages/rs-platform-wallet-ffi/src/identity_key_preview.rs +++ b/packages/rs-platform-wallet-ffi/src/identity_key_preview.rs @@ -89,8 +89,10 @@ use crate::handle::*; /// Without the CRITICAL auth key (keyId 1) the identity cannot mint / /// burn / freeze tokens; without the TRANSFER key (keyId 3) credit /// transfers and withdrawals are rejected on-chain with "no transfer -/// public key". The role→keyId mapping lives with the registration -/// path (`role_for_registration_key_id`), not on the derived row. +/// public key". This preview row carries only the derived keypair — the +/// DPP role→keyId mapping lives with each host's registration policy (the +/// Kotlin `RegistrationKeys` table / the Swift create-identity flow), which +/// stamps the role onto the registration wire rows. pub const IDENTITY_REGISTRATION_KEY_SET_COUNT: u32 = 4; use crate::identity_keys_from_mnemonic::{resolve_master_from_resolver, zeroize_and_free_row}; use crate::types::Network; @@ -267,12 +269,11 @@ pub unsafe extern "C" fn platform_wallet_preview_identity_registration_keys( /// keypair a freshly-created identity is built from. /// /// The rows come back in keyId order. The **per-key DPP role** (purpose -/// / security level) is NOT encoded on the row — it is a deterministic, -/// positional function of the keyId that the registration path applies -/// (see `role_for_registration_key_id` in the JNI / the canonical table -/// in `identity_derive_and_persist.rs`). Every row derives an -/// ECDSA_SECP256K1 keypair; the caller stamps the purpose / security -/// level by keyId at registration time. +/// / security level) is NOT encoded on the row — every row derives an +/// ECDSA_SECP256K1 keypair, and the host's registration policy stamps the +/// purpose / security level (and any contract bounds) by keyId when it builds +/// the registration wire rows (the Kotlin `RegistrationKeys` table / the Swift +/// create-identity flow). /// /// `count_or_neg1 < 0` derives the canonical default set /// ([`IDENTITY_REGISTRATION_KEY_SET_COUNT`] = 4: MASTER auth, CRITICAL diff --git a/packages/rs-platform-wallet-ffi/src/identity_registration_with_signer.rs b/packages/rs-platform-wallet-ffi/src/identity_registration_with_signer.rs index 0737400232..fb774b1c5e 100644 --- a/packages/rs-platform-wallet-ffi/src/identity_registration_with_signer.rs +++ b/packages/rs-platform-wallet-ffi/src/identity_registration_with_signer.rs @@ -305,6 +305,22 @@ pub(crate) unsafe fn decode_identity_pubkeys( let pubkey_bytes: Vec = slice::from_raw_parts(row.pubkey_bytes, row.pubkey_len).to_vec(); let contract_bounds = decode_contract_bounds(row, purpose, i, "identity_pubkeys")?; + // Reject duplicate key IDs explicitly. A plain `insert` into the + // `BTreeMap` would silently last-wins, dropping the earlier row — the + // surviving key is still structurally validated, but the caller asked + // to register two keys and only got one, which is a caller bug worth + // surfacing rather than hiding. All callers (the four Android + // registration paths and the invitation-claim path) build a distinct + // key set, so a collision is always a mistake. + if keys_map.contains_key(&row.key_id) { + return Err(PlatformWalletFFIResult::err( + PlatformWalletFFIResultCode::ErrorInvalidParameter, + format!( + "identity_pubkeys[{i}] has a duplicate key id {}", + row.key_id + ), + )); + } keys_map.insert( row.key_id, IdentityPublicKey::V0(IdentityPublicKeyV0 { @@ -814,4 +830,71 @@ mod tests { assert!(out.items.is_null()); assert_eq!(out.count, 0); } + + /// Build an `IdentityPubkeyFFI` borrowing `pubkey` for the caller's + /// lifetime. AUTHENTICATION / MASTER (discriminants 0/0/0) so no contract + /// bounds are required — the raw byte values match the DPP `Purpose` / + /// `SecurityLevel` reprs. + fn ffi_row(key_id: u32, pubkey: &[u8]) -> IdentityPubkeyFFI { + IdentityPubkeyFFI { + key_id, + key_type: 0, // KeyType::ECDSA_SECP256K1 + purpose: 0, // Purpose::AUTHENTICATION + security_level: 0, // SecurityLevel::MASTER + pubkey_bytes: pubkey.as_ptr(), + pubkey_len: pubkey.len(), + read_only: false, + contract_bounds_kind: 0, + contract_bounds_id: ptr::null(), + contract_bounds_document_type: ptr::null(), + } + } + + /// Two rows sharing a key id must be rejected, not silently collapsed to + /// last-wins in the `BTreeMap`. This is the exact decoder the invitation- + /// claim path (`platform_wallet_claim_invitation`) runs, so the guard + /// covers that fifth caller too — not only the four Android registration + /// paths. + #[test] + fn decode_identity_pubkeys_rejects_duplicate_key_ids() { + let pk_a = [0x02u8; 33]; + let pk_b = [0x03u8; 33]; + let rows = [ffi_row(0, &pk_a), ffi_row(0, &pk_b)]; + // SAFETY: `rows` (and the pubkey arrays it borrows) outlive the call. + let mut err = unsafe { decode_identity_pubkeys(rows.as_ptr(), rows.len()) } + .expect_err("duplicate key id must be rejected"); + assert_eq!(err.code, PlatformWalletFFIResultCode::ErrorInvalidParameter); + unsafe { platform_wallet_ffi_result_free(&mut err) }; + } + + /// A distinct key set still decodes cleanly after the duplicate guard — + /// proves the refactor didn't regress the common path. + #[test] + fn decode_identity_pubkeys_accepts_distinct_key_ids() { + let pk_a = [0x02u8; 33]; + let pk_b = [0x03u8; 33]; + let rows = [ffi_row(0, &pk_a), ffi_row(1, &pk_b)]; + // SAFETY: `rows` (and the pubkey arrays it borrows) outlive the call. + let map = unsafe { decode_identity_pubkeys(rows.as_ptr(), rows.len()) } + .expect("distinct key ids must decode"); + assert_eq!(map.len(), 2); + } + + /// An ENCRYPTION key with no contract bounds is still rejected after the + /// refactor (Drive scopes those keys to a contract, so an unbounded one is + /// unusable). Confirms `decode_contract_bounds`' guard survives. + #[test] + fn decode_identity_pubkeys_rejects_unbounded_encryption_key() { + let pk = [0x02u8; 33]; + let mut enc = ffi_row(4, &pk); + enc.purpose = 1; // Purpose::ENCRYPTION + enc.security_level = 3; // SecurityLevel::MEDIUM + // contract_bounds_kind stays 0 → unbounded → rejected. + let rows = [ffi_row(0, &pk), enc]; + // SAFETY: `rows` (and the pubkey array it borrows) outlive the call. + let mut err = unsafe { decode_identity_pubkeys(rows.as_ptr(), rows.len()) } + .expect_err("unbounded encryption key must be rejected"); + assert_eq!(err.code, PlatformWalletFFIResultCode::ErrorInvalidParameter); + unsafe { platform_wallet_ffi_result_free(&mut err) }; + } } diff --git a/packages/rs-unified-sdk-jni/Cargo.toml b/packages/rs-unified-sdk-jni/Cargo.toml index 72e14a5e91..e2604b7ab2 100644 --- a/packages/rs-unified-sdk-jni/Cargo.toml +++ b/packages/rs-unified-sdk-jni/Cargo.toml @@ -25,6 +25,9 @@ android_logger = "0.14" [dev-dependencies] dashcore = { workspace = true } +# Anchors the cross-language golden-fixture test to the canonical DashPay +# contract id, so the mirrored Kotlin constant can't drift undetected. +dashpay-contract = { path = "../dashpay-contract" } [features] default = ["shielded"] diff --git a/packages/rs-unified-sdk-jni/src/funding.rs b/packages/rs-unified-sdk-jni/src/funding.rs index 2a398aeb0a..aeff1d0ce7 100644 --- a/packages/rs-unified-sdk-jni/src/funding.rs +++ b/packages/rs-unified-sdk-jni/src/funding.rs @@ -39,7 +39,7 @@ #![allow(clippy::missing_safety_doc)] #![cfg(feature = "shielded")] -use crate::identity::{decode_pubkeys_blob, role_for_registration_key_id}; +use crate::pubkey_rows::decode_registration_pubkeys_blob; use crate::support::{guard, take_pwffi_error, throw_sdk_exception, JVM}; use jni::objects::{GlobalRef, JByteArray, JClass, JObject, JString}; use jni::sys::{jboolean, jint, jlong, JNI_FALSE, JNI_TRUE}; @@ -654,10 +654,10 @@ pub extern "system" fn Java_org_dashfoundation_dashsdk_ffi_FundingNative_shielde /// spends a note of the fixed exit `denomination` (credits — one of the /// on-chain `shielded_identity_create_denominations`: 0.1 / 0.3 / 0.5 / /// 1.0 DASH) from the wallet's bound Orchard pool (`account`) to fund a new -/// identity at `identity_index`. `pubkeys_blob` is the SAME flat -/// registration-key blob ID-08 uses (`IdentityKeyPreview.encodeForRegistration`), -/// decoded + role-stamped by keyId exactly like -/// [`Java_..._registerIdentityFromAddresses`]. `fallback_address` is the +/// identity at `identity_index`. `pubkeys_blob` is the SAME shared rich +/// registration key-row blob ID-08 uses (built by `IdentityPubkeyCodec`, +/// decoded by `decode_registration_pubkeys_blob` exactly like +/// [`Java_..._registerIdentityFromAddresses`]). `fallback_address` is the /// REQUIRED 21-byte `PlatformAddress` (1 variant tag + 20 hash) that /// receives the value (minus a penalty) if creation fails a stateful check /// — it is bound into the transition sighash. `signer_handle` is the @@ -700,13 +700,9 @@ pub extern "system" fn Java_org_dashfoundation_dashsdk_ffi_FundingNative_shielde return ptr::null_mut(); }; - let Some(decoded) = decode_pubkeys_blob(env, &pubkeys_blob) else { + let Some(decoded) = decode_registration_pubkeys_blob(env, &pubkeys_blob) else { return ptr::null_mut(); }; - if decoded.is_empty() { - throw_sdk_exception(env, 1, "pubkeysBlob contained no keys"); - return ptr::null_mut(); - } // The 21-byte fallback PlatformAddress (1 variant tag + 20 hash), // REQUIRED for Type-20 — validated exactly here. @@ -727,25 +723,9 @@ pub extern "system" fn Java_org_dashfoundation_dashsdk_ffi_FundingNative_shielde return ptr::null_mut(); } - // Same positional keyId → DPP role assignment as ID-01 / ID-08. - let ffi_rows: Vec = decoded - .iter() - .map(|(key_id, bytes)| { - let (key_type, purpose, security_level) = role_for_registration_key_id(*key_id); - IdentityPubkeyFFI { - key_id: *key_id, - key_type, - purpose, - security_level, - pubkey_bytes: bytes.as_ptr(), - pubkey_len: bytes.len(), - read_only: false, - contract_bounds_kind: 0, - contract_bounds_id: ptr::null(), - contract_bounds_document_type: ptr::null(), - } - }) - .collect(); + // Same rich rows as ID-01 / ID-08 — the caller stamps each key's DPP + // role and any contract bounds; this path just marshals them. + let ffi_rows: Vec = decoded.iter().map(|row| row.to_ffi()).collect(); let mut out_id = [0u8; 32]; let result = unsafe { diff --git a/packages/rs-unified-sdk-jni/src/identity.rs b/packages/rs-unified-sdk-jni/src/identity.rs index f5bad4c39d..538f8ff327 100644 --- a/packages/rs-unified-sdk-jni/src/identity.rs +++ b/packages/rs-unified-sdk-jni/src/identity.rs @@ -31,6 +31,7 @@ #![allow(clippy::missing_safety_doc)] +use crate::pubkey_rows::decode_registration_pubkeys_blob; use crate::support::{ generic_asset_lock_recovery_allowed, guard, take_pwffi_error, throw_sdk_exception, }; @@ -74,62 +75,6 @@ impl Drop for ManagedIdentityHandleGuard { } } -// ── Canonical registration key-role table ───────────────────────────── - -/// DPP `KeyType::ECDSA_SECP256K1` discriminant byte. -const KEY_TYPE_ECDSA_SECP256K1: u8 = 0; -/// DPP `Purpose::AUTHENTICATION` discriminant byte. -const PURPOSE_AUTHENTICATION: u8 = 0; -/// DPP `Purpose::TRANSFER` discriminant byte. -const PURPOSE_TRANSFER: u8 = 3; -/// DPP `SecurityLevel::MASTER` discriminant byte. -const SECURITY_LEVEL_MASTER: u8 = 0; -/// DPP `SecurityLevel::CRITICAL` discriminant byte. -const SECURITY_LEVEL_CRITICAL: u8 = 1; -/// DPP `SecurityLevel::HIGH` discriminant byte. -const SECURITY_LEVEL_HIGH: u8 = 2; - -/// Canonical `(key_type, purpose, security_level)` for the registration -/// key at `key_id`. This is the single Android source of truth for the -/// per-slot identity-key role layout, byte-for-byte identical to the iOS -/// reference (`packages/rs-platform-wallet-ffi/src/identity_derive_and_persist.rs`, -/// and `CreateIdentityView.defaultKeyCount` in the SwiftExampleApp): -/// -/// | key_id | key_type | purpose | security_level | -/// |--------|-----------------|----------------|----------------| -/// | 0 | ECDSA_SECP256K1 | AUTHENTICATION | MASTER | -/// | 1 | ECDSA_SECP256K1 | AUTHENTICATION | CRITICAL | -/// | 2 | ECDSA_SECP256K1 | AUTHENTICATION | HIGH | -/// | 3 | ECDSA_SECP256K1 | TRANSFER | CRITICAL | -/// | > 3 | ECDSA_SECP256K1 | AUTHENTICATION | HIGH | -/// -/// - keyId 0 (MASTER/AUTH) signs the IdentityCreate transition. -/// - keyId 1 (CRITICAL/AUTH) signs token state transitions — -/// `combined_security_level_requirement` collapses any batch with a -/// token transition to `[CRITICAL]`, so without it the identity can't -/// mint / burn / freeze tokens. -/// - keyId 2 (HIGH/AUTH) signs general document / DPNS / contract -/// transitions. -/// - keyId 3 (TRANSFER/CRITICAL) signs IdentityCreditTransfer / -/// IdentityCreditWithdrawal — without it those broadcasts are rejected -/// on-chain with "no transfer public key". -/// -/// Previously this JNI hardcoded `purpose = AUTHENTICATION` for every row -/// and `security_level = MASTER if key_id == 0 else HIGH`, so a freshly -/// created identity had no CRITICAL auth key and no TRANSFER key, and all -/// token / credit-transfer / withdrawal writes failed validation right -/// after creation. If DPP renumbers any discriminant, update this table. -pub(crate) fn role_for_registration_key_id(key_id: u32) -> (u8, u8, u8) { - let (purpose, security_level) = match key_id { - 0 => (PURPOSE_AUTHENTICATION, SECURITY_LEVEL_MASTER), - 1 => (PURPOSE_AUTHENTICATION, SECURITY_LEVEL_CRITICAL), - 2 => (PURPOSE_AUTHENTICATION, SECURITY_LEVEL_HIGH), - 3 => (PURPOSE_TRANSFER, SECURITY_LEVEL_CRITICAL), - _ => (PURPOSE_AUTHENTICATION, SECURITY_LEVEL_HIGH), - }; - (KEY_TYPE_ECDSA_SECP256K1, purpose, security_level) -} - /// Read a required 32-byte id from a Java `byte[]`; throws + returns None /// on the wrong length or a JNI error. fn read_id32(env: &mut JNIEnv, arr: &JByteArray, field: &str) -> Option<[u8; 32]> { @@ -264,10 +209,11 @@ pub extern "system" fn Java_org_dashfoundation_dashsdk_ffi_IdentityNative_previe /// /// `count < 0` derives the canonical default set /// (`IDENTITY_REGISTRATION_KEY_SET_COUNT` = 4: MASTER auth, CRITICAL -/// auth, HIGH auth, TRANSFER/CRITICAL). The per-key DPP role is applied -/// positionally by keyId at registration time -/// (`role_for_registration_key_id`), NOT carried on the row — every row -/// is an ECDSA_SECP256K1 keypair. +/// auth, HIGH auth, TRANSFER/CRITICAL). The create-identity flow may request +/// more (e.g. 6, appending the DashPay ENCRYPTION/DECRYPTION pair). Every row +/// is an ECDSA_SECP256K1 keypair; the DPP role for each keyId is stamped by +/// the Kotlin side (`RegistrationKeys`) and shipped on the registration wire, +/// not carried on this derived row. /// /// Returns the same flat `byte[]` BLOB layout as /// [`Java_..._previewRegistrationKeys`], decoded by @@ -596,31 +542,10 @@ pub extern "system" fn Java_org_dashfoundation_dashsdk_ffi_IdentityNative_resume return ptr::null_mut(); } - let Some(decoded) = decode_pubkeys_blob(env, &pubkeys_blob) else { + let Some(decoded) = decode_registration_pubkeys_blob(env, &pubkeys_blob) else { return ptr::null_mut(); }; - if decoded.is_empty() { - throw_sdk_exception(env, 1, "pubkeysBlob contained no keys"); - return ptr::null_mut(); - } - let ffi_rows: Vec = decoded - .iter() - .map(|(key_id, bytes)| { - let (key_type, purpose, security_level) = role_for_registration_key_id(*key_id); - IdentityPubkeyFFI { - key_id: *key_id, - key_type, - purpose, - security_level, - pubkey_bytes: bytes.as_ptr(), - pubkey_len: bytes.len(), - read_only: false, - contract_bounds_kind: 0, - contract_bounds_id: ptr::null(), - contract_bounds_document_type: ptr::null(), - } - }) - .collect(); + let ffi_rows: Vec = decoded.iter().map(|row| row.to_ffi()).collect(); let outpoint = OutPointFFI { txid, vout: outpoint_vout as u32, @@ -681,20 +606,16 @@ pub extern "system" fn Java_org_dashfoundation_dashsdk_ffi_IdentityNative_resume /// /// This is the single FFI entry point the app's `RegistrationCoordinator` /// invokes — no orchestration on the Kotlin side. The caller (Kotlin) has -/// already derived + persisted the identity keys via -/// [`Java_..._previewRegistrationKeys`], so `pubkeysBlob` is the same flat -/// layout `previewRegistrationKeys` produced, trimmed to the keys being -/// registered (row `identity_index` is ignored here — the pubkey rows are -/// read positionally as `keyId = index`). +/// already derived + persisted the identity keys and built the rich +/// registration rows (`RegistrationKeys` + `IdentityPubkeyCodec`), so +/// `pubkeysBlob` is the shared rich key-row layout decoded by +/// `crate::pubkey_rows` — the same format the identity-update add-key path +/// uses, carrying each key's full DPP role and any contract bounds. /// -/// `pubkeysBlob` layout (big-endian): -/// ```text -/// u32 row_count -/// repeat: u32 keyId, u16 pubkey_len, u8[pubkey_len] compressed pubkey -/// ``` -/// Every key is registered as an ECDSA_SECP256K1 / AUTHENTICATION key at -/// the security level implied by its position (row 0 = MASTER). The Rust -/// side validates the security-level layout. +/// The blob is decoded by `decode_registration_pubkeys_blob`, which enforces +/// the registration-only invariants (≥1 key, no duplicate key IDs, key ID 0 +/// = MASTER + AUTHENTICATION). DPP validates the full structural layout +/// server-side. /// /// Returns the 32-byte identity id as a `byte[]`. The `ManagedIdentity` /// handle the FFI produces is destroyed here — Room learns of the new @@ -729,42 +650,18 @@ pub extern "system" fn Java_org_dashfoundation_dashsdk_ffi_IdentityNative_regist return ptr::null_mut(); } - // Decode the pubkey rows into owned buffers that outlive the FFI - // call (the FFI borrows `pubkey_bytes` for the call duration). - let Some(decoded) = decode_pubkeys_blob(env, &pubkeys_blob) else { + // Decode the rich pubkey rows into owned buffers that outlive the FFI + // call (the FFI borrows every `pubkey_bytes` / `contract_bounds_*` + // pointer for the call duration). The per-key DPP role (type / purpose + // / security level) and any contract bounds now ride the wire — the + // Kotlin side stamps them from the canonical registration key policy, + // so keyId 0 arrives MASTER/AUTH, keyId 1 CRITICAL/AUTH, keyId 2 + // HIGH/AUTH, keyId 3 TRANSFER/CRITICAL, plus any DashPay + // ENCRYPTION/DECRYPTION keys with their contract-document bounds. + let Some(decoded) = decode_registration_pubkeys_blob(env, &pubkeys_blob) else { return ptr::null_mut(); }; - if decoded.is_empty() { - throw_sdk_exception(env, 1, "pubkeysBlob contained no keys"); - return ptr::null_mut(); - } - - // Build the FFI rows referencing the owned buffers. `read_only` - // false, no contract bounds (auth / transfer keys never carry - // bounds — only ENCRYPTION / DECRYPTION do). The per-key role - // (key_type / purpose / security_level) is the canonical, - // positional function of `key_id` (see - // `role_for_registration_key_id`), matching iOS exactly — so a - // freshly created identity gets keyId 0 MASTER/AUTH, keyId 1 - // CRITICAL/AUTH, keyId 2 HIGH/AUTH, keyId 3 TRANSFER/CRITICAL. - let ffi_rows: Vec = decoded - .iter() - .map(|(key_id, bytes)| { - let (key_type, purpose, security_level) = role_for_registration_key_id(*key_id); - IdentityPubkeyFFI { - key_id: *key_id, - key_type, - purpose, - security_level, - pubkey_bytes: bytes.as_ptr(), - pubkey_len: bytes.len(), - read_only: false, - contract_bounds_kind: 0, - contract_bounds_id: ptr::null(), - contract_bounds_document_type: ptr::null(), - } - }) - .collect(); + let ffi_rows: Vec = decoded.iter().map(|row| row.to_ffi()).collect(); let mut out_id = [0u8; 32]; let mut out_managed: Handle = 0; @@ -810,9 +707,10 @@ pub extern "system" fn Java_org_dashfoundation_dashsdk_ffi_IdentityNative_regist /// Core asset lock. No Core-chain transaction is broadcast; the inputs are /// existing Platform credits. /// -/// `pubkeysBlob` is the same flat layout `registerIdentityWithFunding` -/// consumes (`u32 rowCount` then per row `u32 keyId, u16 pubkeyLen, -/// pubkey`), each key stamped with its canonical DPP role by `keyId`. +/// `pubkeysBlob` is the same shared rich key-row layout +/// `registerIdentityWithFunding` consumes (decoded by +/// `decode_registration_pubkeys_blob`) — each row carries its DPP role and any +/// contract bounds, stamped Kotlin-side. /// /// `inputsBlob` is the funding-address row shape shared with the top-up / /// transfer-to-addresses exports (`u32 rowCount` then per row @@ -843,13 +741,9 @@ pub extern "system" fn Java_org_dashfoundation_dashsdk_ffi_IdentityNative_regist return ptr::null_mut(); } - let Some(decoded) = decode_pubkeys_blob(env, &pubkeys_blob) else { + let Some(decoded) = decode_registration_pubkeys_blob(env, &pubkeys_blob) else { return ptr::null_mut(); }; - if decoded.is_empty() { - throw_sdk_exception(env, 1, "pubkeysBlob contained no keys"); - return ptr::null_mut(); - } let Some(input_rows) = crate::credits::decode_credit_rows(env, &inputs_blob, "inputsBlob") else { @@ -860,26 +754,9 @@ pub extern "system" fn Java_org_dashfoundation_dashsdk_ffi_IdentityNative_regist return ptr::null_mut(); } - // Same positional keyId → DPP role assignment as ID-01 (keyId 0 - // MASTER/AUTH, 1 CRITICAL/AUTH, 2 HIGH/AUTH, 3 TRANSFER/CRITICAL). - let ffi_rows: Vec = decoded - .iter() - .map(|(key_id, bytes)| { - let (key_type, purpose, security_level) = role_for_registration_key_id(*key_id); - IdentityPubkeyFFI { - key_id: *key_id, - key_type, - purpose, - security_level, - pubkey_bytes: bytes.as_ptr(), - pubkey_len: bytes.len(), - read_only: false, - contract_bounds_kind: 0, - contract_bounds_id: ptr::null(), - contract_bounds_document_type: ptr::null(), - } - }) - .collect(); + // Same rich rows as ID-01 — the caller stamps each key's DPP role and + // any contract bounds; this path just marshals them. + let ffi_rows: Vec = decoded.iter().map(|row| row.to_ffi()).collect(); let input_ffi: Vec = input_rows .into_iter() @@ -927,81 +804,6 @@ pub extern "system" fn Java_org_dashfoundation_dashsdk_ffi_IdentityNative_regist }) } -/// Decode the registration pubkeys BLOB into `(keyId, bytes)` rows whose -/// buffers the caller keeps alive across the FFI call. Throws + returns -/// None on a malformed blob. -pub(crate) fn decode_pubkeys_blob( - env: &mut JNIEnv, - arr: &JByteArray, -) -> Option)>> { - let bytes = match env.convert_byte_array(arr) { - Ok(b) => b, - Err(_) => { - let _ = env.exception_clear(); - throw_sdk_exception(env, 1, "pubkeysBlob was null/invalid"); - return None; - } - }; - let mut cursor = 0usize; - let read = |cursor: &mut usize, n: usize| -> Option<&[u8]> { - if *cursor + n > bytes.len() { - return None; - } - let s = &bytes[*cursor..*cursor + n]; - *cursor += n; - Some(s) - }; - let Some(count_bytes) = read(&mut cursor, 4) else { - throw_sdk_exception(env, 1, "pubkeysBlob truncated (row count)"); - return None; - }; - let count = u32::from_be_bytes(count_bytes.try_into().ok()?) as usize; - // Length-before-allocation guard: each row is at least 6 bytes - // (u32 keyId + u16 len), so a header claiming more rows than the - // remaining payload can hold is malformed — prevents a huge - // `with_capacity` abort from a raw-JNI blob. - if count - .checked_mul(6) - .is_none_or(|need| bytes.len() - cursor < need) - { - throw_sdk_exception( - env, - 1, - &format!("pubkeysBlob claims {count} rows but body is too short"), - ); - return None; - } - let mut rows = Vec::with_capacity(count); - for i in 0..count { - let Some(id_bytes) = read(&mut cursor, 4) else { - throw_sdk_exception(env, 1, &format!("pubkeysBlob truncated at row {i} keyId")); - return None; - }; - let key_id = u32::from_be_bytes(id_bytes.try_into().ok()?); - // The Kotlin encoder writes this field with writeInt (signed); a - // set sign bit means a negative key id crossed the boundary. - if key_id > i32::MAX as u32 { - throw_sdk_exception( - env, - 1, - &format!("pubkeysBlob row {i} keyId must be non-negative"), - ); - return None; - } - let Some(len_bytes) = read(&mut cursor, 2) else { - throw_sdk_exception(env, 1, &format!("pubkeysBlob truncated at row {i} len")); - return None; - }; - let len = u16::from_be_bytes(len_bytes.try_into().ok()?) as usize; - let Some(pubkey) = read(&mut cursor, len) else { - throw_sdk_exception(env, 1, &format!("pubkeysBlob truncated at row {i} pubkey")); - return None; - }; - rows.push((key_id, pubkey.to_vec())); - } - Some(rows) -} - // ── Discovery ───────────────────────────────────────────────────────── /// Scan the wallet's identity-authentication tree for registered diff --git a/packages/rs-unified-sdk-jni/src/lib.rs b/packages/rs-unified-sdk-jni/src/lib.rs index d614fb7132..59560796d4 100644 --- a/packages/rs-unified-sdk-jni/src/lib.rs +++ b/packages/rs-unified-sdk-jni/src/lib.rs @@ -21,6 +21,7 @@ mod funding; mod identity; mod mnemonic; mod persistence; +mod pubkey_rows; mod queries; mod results; mod sdk; diff --git a/packages/rs-unified-sdk-jni/src/pubkey_rows.rs b/packages/rs-unified-sdk-jni/src/pubkey_rows.rs new file mode 100644 index 0000000000..bf41f69608 --- /dev/null +++ b/packages/rs-unified-sdk-jni/src/pubkey_rows.rs @@ -0,0 +1,743 @@ +//! Shared decoder for the identity add/register public-key BLOB. +//! +//! The Kotlin side encodes rich key rows (a `keyId`, the three DPP role bytes, +//! an optional contract-bounds section, and the compressed public key) into an +//! opaque `byte[]`. This module parses that wire format back into owned +//! [`DecodedPubkeyRow`]s whose buffers back the raw pointers of an +//! [`IdentityPubkeyFFI`] for the duration of one synchronous FFI call. +//! +//! ## One pure parser, two JNI seams +//! +//! [`parse_pubkey_rows`] is a pure `&[u8]` → `Vec` function +//! (no `JNIEnv`, no `JByteArray`) so its round-trip and edge-case behaviour is +//! covered by ordinary Rust unit tests rather than JNI-environment-dependent +//! ones. Two thin JNI adapters wrap it: +//! +//! - [`decode_update_pubkeys_blob`] backs the identity-UPDATE add-key path +//! (`TransactionsNative.updateIdentity`). A null / empty blob is a +//! legitimate "no keys to add" — an update may be disable-only. **No** +//! key-ID-0 or single-MASTER invariant applies here: an ordinary add-key +//! update neither includes nor should include key 0. +//! - [`decode_registration_pubkeys_blob`] backs every identity-CREATE path +//! (the four registration JNI exports). It additionally requires ≥1 row, +//! rejects duplicate key IDs, and enforces the wallet's key-ID-0 = +//! MASTER + AUTHENTICATION convention (`MASTER_KEY_INDEX == 0`; see +//! `platform-wallet`'s `identity_handle.rs`). Those extra checks are +//! registration-only and deliberately absent from the update decoder. +//! +//! ## Strictness (wire-skew detection) +//! +//! The parser rejects truncated input, **trailing bytes**, invalid DPP-role / +//! bounds-kind / boolean discriminants, interior-NUL document types, and +//! negative key IDs. The opaque payload shape is not self-describing, so an +//! old Kotlin artifact paired with a newer native library (or the reverse) +//! would otherwise be silently misparsed rather than rejected. Failing loud on +//! any structural surprise turns that skew into a clean error instead of a key +//! registered with the wrong semantics. + +use crate::support::throw_sdk_exception; +use jni::objects::JByteArray; +use jni::JNIEnv; +use platform_wallet_ffi::identity_registration_with_signer::IdentityPubkeyFFI; +use std::ffi::CString; +use std::ptr; + +/// DPP `Purpose::AUTHENTICATION` discriminant byte. +const PURPOSE_AUTHENTICATION: u8 = 0; +/// DPP `SecurityLevel::MASTER` discriminant byte. +const SECURITY_LEVEL_MASTER: u8 = 0; + +/// One decoded add/register key row with owned buffers backing the FFI +/// pointers an [`IdentityPubkeyFFI`] borrows. +/// +/// Fields are crate-visible so the registration invariant checks and the unit +/// tests can read them; call sites should still build their FFI view through +/// [`DecodedPubkeyRow::to_ffi`] so pointer creation and the borrow contract +/// stay in one place. +/// +/// Every field is public-key / role metadata — no private key material — so +/// `Debug` (used by the round-trip tests) leaks nothing sensitive. +#[derive(Debug)] +pub(crate) struct DecodedPubkeyRow { + pub(crate) key_id: u32, + pub(crate) key_type: u8, + pub(crate) purpose: u8, + pub(crate) security_level: u8, + pub(crate) read_only: bool, + pub(crate) contract_bounds_kind: u8, + pub(crate) pubkey_bytes: Vec, + pub(crate) contract_bounds_id: Option<[u8; 32]>, + pub(crate) contract_bounds_document_type: Option, +} + +impl DecodedPubkeyRow { + /// Build the borrowed [`IdentityPubkeyFFI`] view of this row. + /// + /// Every pointer references a buffer owned by `self`, so the returned + /// struct must not outlive it — keep the owning `DecodedPubkeyRow` (or the + /// `Vec` holding it) alive across the whole FFI call that reads the view. + pub(crate) fn to_ffi(&self) -> IdentityPubkeyFFI { + IdentityPubkeyFFI { + key_id: self.key_id, + key_type: self.key_type, + purpose: self.purpose, + security_level: self.security_level, + pubkey_bytes: self.pubkey_bytes.as_ptr(), + pubkey_len: self.pubkey_bytes.len(), + read_only: self.read_only, + contract_bounds_kind: self.contract_bounds_kind, + contract_bounds_id: self + .contract_bounds_id + .as_ref() + .map_or(ptr::null(), |b| b.as_ptr()), + contract_bounds_document_type: self + .contract_bounds_document_type + .as_ref() + .map_or(ptr::null(), |c| c.as_ptr()), + } + } +} + +/// Parse the rich add/register public-key BLOB into owned rows. +/// +/// Layout (all integers big-endian), one row per key: +/// ```text +/// u32 row_count +/// repeat row_count times: +/// u32 key_id +/// u8 key_type (DPP KeyType discriminant, 0 = ECDSA_SECP256K1) +/// u8 purpose (DPP Purpose discriminant, 0 = AUTHENTICATION) +/// u8 security_level (DPP SecurityLevel discriminant, 0 = MASTER) +/// u8 read_only (0 / 1 — any other byte is rejected) +/// u8 contract_bounds_kind (0 none, 1 SingleContract, 2 SingleContractDocumentType) +/// u16 pubkey_len +/// u8[pubkey_len] pubkey_bytes (compressed pubkey, or 20-byte HASH160) +/// if contract_bounds_kind != 0: +/// u8[32] contract_bounds_id +/// if contract_bounds_kind == 2: +/// u16 doc_type_len, u8[doc_type_len] doc_type (UTF-8) +/// ``` +/// +/// Strict: returns `Err` on truncation, trailing bytes, a negative key ID +/// (`writeInt` is signed on the Kotlin side, so a set sign bit is a bug), an +/// invalid `read_only` or `contract_bounds_kind` byte, or an interior NUL in a +/// document type. It does **not** validate the DPP role bytes against DPP's +/// structural rules — that stays server-side — nor does it apply any +/// registration-only invariant; the registration seam layers those on top. +pub(crate) fn parse_pubkey_rows(bytes: &[u8]) -> Result, String> { + let mut cursor = 0usize; + let read = |cursor: &mut usize, n: usize| -> Option<&[u8]> { + let end = cursor.checked_add(n)?; + if end > bytes.len() { + return None; + } + let s = &bytes[*cursor..end]; + *cursor = end; + Some(s) + }; + + let count_bytes = read(&mut cursor, 4).ok_or("pubkey blob truncated (row count)")?; + let count = u32::from_be_bytes([ + count_bytes[0], + count_bytes[1], + count_bytes[2], + count_bytes[3], + ]) as usize; + // Length-before-allocation guard: each row is at least an 11-byte fixed + // header, so a header claiming more rows than the remaining payload can + // possibly hold is malformed — prevents a huge `with_capacity` abort from a + // raw-JNI blob. + if count + .checked_mul(11) + .is_none_or(|need| bytes.len() - cursor < need) + { + return Err(format!( + "pubkey blob claims {count} rows but the body is too short" + )); + } + + let mut rows = Vec::with_capacity(count); + for i in 0..count { + let fixed = read(&mut cursor, 4 + 1 + 1 + 1 + 1 + 1 + 2) + .ok_or_else(|| format!("pubkey blob truncated at row {i} header"))?; + let key_id = u32::from_be_bytes([fixed[0], fixed[1], fixed[2], fixed[3]]); + // The Kotlin encoder writes this field with writeInt (signed); a set + // sign bit means a negative key id crossed the boundary. + if key_id > i32::MAX as u32 { + return Err(format!("pubkey blob row {i} keyId must be non-negative")); + } + let key_type = fixed[4]; + let purpose = fixed[5]; + let security_level = fixed[6]; + let read_only = match fixed[7] { + 0 => false, + 1 => true, + other => { + return Err(format!( + "pubkey blob row {i} readOnly must be 0 or 1, got {other}" + )); + } + }; + let contract_bounds_kind = fixed[8]; + if contract_bounds_kind > 2 { + return Err(format!( + "pubkey blob row {i} contractBoundsKind must be 0, 1 or 2, got {contract_bounds_kind}" + )); + } + let pubkey_len = u16::from_be_bytes([fixed[9], fixed[10]]) as usize; + let pubkey_bytes = read(&mut cursor, pubkey_len) + .ok_or_else(|| format!("pubkey blob truncated at row {i} pubkey"))? + .to_vec(); + + let mut contract_bounds_id: Option<[u8; 32]> = None; + let mut contract_bounds_document_type: Option = None; + if contract_bounds_kind != 0 { + let id_bytes = read(&mut cursor, 32) + .ok_or_else(|| format!("pubkey blob truncated at row {i} contractBoundsId"))?; + let mut id = [0u8; 32]; + id.copy_from_slice(id_bytes); + contract_bounds_id = Some(id); + + if contract_bounds_kind == 2 { + let dt_len_bytes = read(&mut cursor, 2) + .ok_or_else(|| format!("pubkey blob truncated at row {i} docTypeLen"))?; + let dt_len = u16::from_be_bytes([dt_len_bytes[0], dt_len_bytes[1]]) as usize; + let dt_bytes = read(&mut cursor, dt_len) + .ok_or_else(|| format!("pubkey blob truncated at row {i} docType"))?; + contract_bounds_document_type = Some( + CString::new(dt_bytes.to_vec()) + .map_err(|_| format!("pubkey blob row {i} docType had an interior NUL"))?, + ); + } + } + + rows.push(DecodedPubkeyRow { + key_id, + key_type, + purpose, + security_level, + read_only, + contract_bounds_kind, + pubkey_bytes, + contract_bounds_id, + contract_bounds_document_type, + }); + } + + // Reject trailing bytes: the payload is not self-describing, so leftover + // bytes mean the writer and reader disagree on the layout (a wire-format + // skew) — surface it rather than silently ignoring the tail. + if cursor != bytes.len() { + return Err(format!( + "pubkey blob has {} trailing byte(s) after {count} row(s)", + bytes.len() - cursor + )); + } + + Ok(rows) +} + +/// Read a Java `byte[]` into owned bytes, or `None` when the array is null. +fn convert_or_null(env: &mut JNIEnv, arr: &JByteArray) -> Option> { + match env.convert_byte_array(arr) { + Ok(b) => Some(b), + Err(_) => { + let _ = env.exception_clear(); + None + } + } +} + +/// Decode the identity-UPDATE add-keys BLOB (the `updateIdentity` path). +/// +/// A null or empty blob is a legitimate "no keys to add" (an update may be +/// disable-only), so it maps to an empty row list rather than an error. Any +/// non-empty blob must parse strictly. Throws + returns `None` on a malformed +/// blob. No registration-only invariant is applied — see the module docs. +pub(crate) fn decode_update_pubkeys_blob( + env: &mut JNIEnv, + arr: &JByteArray, +) -> Option> { + let Some(bytes) = convert_or_null(env, arr) else { + // A null blob is a legitimate "no keys to add". + return Some(Vec::new()); + }; + if bytes.is_empty() { + return Some(Vec::new()); + } + match parse_pubkey_rows(&bytes) { + Ok(rows) => Some(rows), + Err(msg) => { + throw_sdk_exception(env, 1, &msg); + None + } + } +} + +/// The registration-only invariants layered on top of the shared structural +/// parse (kept pure + separate from the JNI wrapper so they are unit-testable +/// without a JVM): +/// - at least one key row, +/// - no duplicate key IDs — the FFI `decode_identity_pubkeys` inserts into a +/// `BTreeMap` where a later duplicate would otherwise silently overwrite an +/// earlier row, so reject them here (and there) rather than last-wins, +/// - key ID 0 present and MASTER + AUTHENTICATION — the wallet derives and +/// loads the identity's signing key at `MASTER_KEY_INDEX == 0`, and the +/// shared funded path already asserts this, but the address-funded and +/// shielded paths build the identity straight from the decoded map without +/// it. Enforcing it at this shared registration seam closes that gap for +/// every create path uniformly. +/// +/// The order-independence matters: key ID 0 is found by id, not row position, +/// so a caller may submit rows in any order. +pub(crate) fn check_registration_invariants(rows: &[DecodedPubkeyRow]) -> Result<(), String> { + if rows.is_empty() { + return Err("pubkeysBlob contained no keys".to_string()); + } + + let mut seen = std::collections::BTreeSet::new(); + for row in rows { + if !seen.insert(row.key_id) { + return Err(format!("pubkeysBlob has a duplicate keyId {}", row.key_id)); + } + } + + let key0 = rows + .iter() + .find(|r| r.key_id == 0) + .ok_or("pubkeysBlob is missing key ID 0 (the MASTER key)")?; + if key0.security_level != SECURITY_LEVEL_MASTER || key0.purpose != PURPOSE_AUTHENTICATION { + return Err("pubkeysBlob key ID 0 must be MASTER + AUTHENTICATION".to_string()); + } + + Ok(()) +} + +/// Decode an identity-CREATE registration pubkeys BLOB (all four registration +/// JNI exports): the shared structural parse plus +/// [`check_registration_invariants`]. Throws + returns `None` on any violation. +pub(crate) fn decode_registration_pubkeys_blob( + env: &mut JNIEnv, + arr: &JByteArray, +) -> Option> { + let Some(bytes) = convert_or_null(env, arr) else { + throw_sdk_exception(env, 1, "pubkeysBlob was null/invalid"); + return None; + }; + let rows = match parse_pubkey_rows(&bytes) { + Ok(rows) => rows, + Err(msg) => { + throw_sdk_exception(env, 1, &msg); + return None; + } + }; + if let Err(msg) = check_registration_invariants(&rows) { + throw_sdk_exception(env, 1, &msg); + return None; + } + Some(rows) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// DPP discriminants used across the fixtures (see the Kotlin + /// `KeyType` / `KeyPurpose` / `SecurityLevel` enums). + const KEY_TYPE_ECDSA: u8 = 0; + const PURPOSE_AUTH: u8 = 0; + const PURPOSE_ENCRYPTION: u8 = 1; + const PURPOSE_DECRYPTION: u8 = 2; + const PURPOSE_TRANSFER: u8 = 3; + const SEC_MASTER: u8 = 0; + const SEC_CRITICAL: u8 = 1; + const SEC_HIGH: u8 = 2; + const SEC_MEDIUM: u8 = 3; + + /// Test-only encoder — the byte-for-byte mirror of Kotlin's + /// `IdentityPubkeyCodec.encode`, so a round trip through + /// [`parse_pubkey_rows`] proves both directions agree. + struct Row { + key_id: u32, + key_type: u8, + purpose: u8, + security_level: u8, + read_only: u8, + pubkey: Vec, + bounds: Option<(u8, [u8; 32], Option)>, + } + + fn encode(rows: &[Row]) -> Vec { + let mut out = Vec::new(); + out.extend_from_slice(&(rows.len() as u32).to_be_bytes()); + for r in rows { + out.extend_from_slice(&r.key_id.to_be_bytes()); + out.push(r.key_type); + out.push(r.purpose); + out.push(r.security_level); + out.push(r.read_only); + let kind = r.bounds.as_ref().map_or(0, |(k, _, _)| *k); + out.push(kind); + out.extend_from_slice(&(r.pubkey.len() as u16).to_be_bytes()); + out.extend_from_slice(&r.pubkey); + if let Some((_, id, doc)) = &r.bounds { + out.extend_from_slice(id); + if let Some(doc) = doc { + let dt = doc.as_bytes(); + out.extend_from_slice(&(dt.len() as u16).to_be_bytes()); + out.extend_from_slice(dt); + } + } + } + out + } + + fn base_master() -> Row { + Row { + key_id: 0, + key_type: KEY_TYPE_ECDSA, + purpose: PURPOSE_AUTH, + security_level: SEC_MASTER, + read_only: 0, + pubkey: vec![2u8; 33], + bounds: None, + } + } + + /// The full 6-key registration policy: base 4 (auth MASTER/CRITICAL/HIGH, + /// TRANSFER/CRITICAL) + the DashPay ENCRYPTION/DECRYPTION pair bound to the + /// `contactRequest` document type. + fn six_key_policy() -> Vec { + let dashpay_id = [7u8; 32]; + vec![ + base_master(), + Row { + key_id: 1, + key_type: KEY_TYPE_ECDSA, + purpose: PURPOSE_AUTH, + security_level: SEC_CRITICAL, + read_only: 0, + pubkey: vec![3u8; 33], + bounds: None, + }, + Row { + key_id: 2, + key_type: KEY_TYPE_ECDSA, + purpose: PURPOSE_AUTH, + security_level: SEC_HIGH, + read_only: 0, + pubkey: vec![4u8; 33], + bounds: None, + }, + Row { + key_id: 3, + key_type: KEY_TYPE_ECDSA, + purpose: PURPOSE_TRANSFER, + security_level: SEC_CRITICAL, + read_only: 0, + pubkey: vec![5u8; 33], + bounds: None, + }, + Row { + key_id: 4, + key_type: KEY_TYPE_ECDSA, + purpose: PURPOSE_ENCRYPTION, + security_level: SEC_MEDIUM, + read_only: 0, + pubkey: vec![6u8; 33], + bounds: Some((2, dashpay_id, Some("contactRequest".to_string()))), + }, + Row { + key_id: 5, + key_type: KEY_TYPE_ECDSA, + purpose: PURPOSE_DECRYPTION, + security_level: SEC_MEDIUM, + read_only: 0, + pubkey: vec![7u8; 33], + bounds: Some((2, dashpay_id, Some("contactRequest".to_string()))), + }, + ] + } + + #[test] + fn round_trips_the_full_six_key_policy() { + let dashpay_id = [7u8; 32]; + let decoded = parse_pubkey_rows(&encode(&six_key_policy())).expect("parse"); + assert_eq!(decoded.len(), 6); + // Base 4 roles round-trip. + assert_eq!( + ( + decoded[0].purpose, + decoded[0].security_level, + decoded[0].contract_bounds_kind + ), + (PURPOSE_AUTH, SEC_MASTER, 0) + ); + assert_eq!( + (decoded[3].purpose, decoded[3].security_level), + (PURPOSE_TRANSFER, SEC_CRITICAL) + ); + // DashPay ENCRYPTION / DECRYPTION carry the contract-document bounds. + for row in &decoded[4..] { + assert_eq!(row.security_level, SEC_MEDIUM); + assert_eq!(row.contract_bounds_kind, 2); + assert_eq!(row.contract_bounds_id, Some(dashpay_id)); + assert_eq!( + row.contract_bounds_document_type + .as_ref() + .map(|c| c.to_str().unwrap().to_string()), + Some("contactRequest".to_string()) + ); + } + assert_eq!(decoded[4].purpose, PURPOSE_ENCRYPTION); + assert_eq!(decoded[5].purpose, PURPOSE_DECRYPTION); + // Every pubkey survives byte-for-byte. + assert_eq!(decoded[5].pubkey_bytes, vec![7u8; 33]); + } + + #[test] + fn round_trips_single_contract_bounds_kind() { + let id = [9u8; 32]; + let rows = vec![Row { + key_id: 0, + key_type: KEY_TYPE_ECDSA, + purpose: PURPOSE_ENCRYPTION, + security_level: SEC_MEDIUM, + read_only: 1, + pubkey: vec![2u8; 33], + bounds: Some((1, id, None)), + }]; + let decoded = parse_pubkey_rows(&encode(&rows)).expect("parse"); + assert_eq!(decoded[0].contract_bounds_kind, 1); + assert_eq!(decoded[0].contract_bounds_id, Some(id)); + assert!(decoded[0].contract_bounds_document_type.is_none()); + assert!(decoded[0].read_only); + } + + #[test] + fn rejects_trailing_bytes() { + let mut blob = encode(&[base_master()]); + blob.push(0xAB); + let err = parse_pubkey_rows(&blob).unwrap_err(); + assert!(err.contains("trailing"), "{err}"); + } + + #[test] + fn rejects_truncation_at_each_variable_field() { + let full = encode(&six_key_policy()); + // Truncating anywhere past the row count must fail (never silently + // return a short list). + for cut in 4..full.len() { + assert!( + parse_pubkey_rows(&full[..cut]).is_err(), + "expected truncation error at cut {cut}" + ); + } + } + + #[test] + fn rejects_invalid_bounds_kind() { + let mut rows = vec![base_master()]; + rows[0].bounds = Some((3, [1u8; 32], None)); + // encode() writes kind byte from bounds.0 = 3, then a 32-byte id. + let err = parse_pubkey_rows(&encode(&rows)).unwrap_err(); + assert!(err.contains("contractBoundsKind"), "{err}"); + } + + #[test] + fn rejects_invalid_readonly_byte() { + let mut blob = encode(&[base_master()]); + // read_only byte sits at offset 4(count)+4(keyId)+3(role) = 11. + blob[11] = 2; + let err = parse_pubkey_rows(&blob).unwrap_err(); + assert!(err.contains("readOnly"), "{err}"); + } + + #[test] + fn rejects_negative_key_id() { + let mut row = base_master(); + row.key_id = 0x8000_0000; // sign bit set once written as be bytes + let err = parse_pubkey_rows(&encode(&[row])).unwrap_err(); + assert!(err.contains("non-negative"), "{err}"); + } + + #[test] + fn rejects_interior_nul_document_type() { + let rows = vec![Row { + key_id: 0, + key_type: KEY_TYPE_ECDSA, + purpose: PURPOSE_ENCRYPTION, + security_level: SEC_MEDIUM, + read_only: 0, + pubkey: vec![2u8; 33], + bounds: Some((2, [1u8; 32], Some("con\0tact".to_string()))), + }]; + let err = parse_pubkey_rows(&encode(&rows)).unwrap_err(); + assert!(err.contains("interior NUL"), "{err}"); + } + + #[test] + fn rejects_legacy_bare_registration_format() { + // The retired registration blob was: u32 rowCount, then per row + // u32 keyId, u16 pubkeyLen, pubkey — no role/bounds bytes. Feeding + // that shape to the rich parser must fail, not silently misparse. + let mut legacy = Vec::new(); + legacy.extend_from_slice(&1u32.to_be_bytes()); + legacy.extend_from_slice(&0u32.to_be_bytes()); // keyId 0 + legacy.extend_from_slice(&33u16.to_be_bytes()); // pubkey len + legacy.extend_from_slice(&[2u8; 33]); + assert!(parse_pubkey_rows(&legacy).is_err()); + } + + #[test] + fn empty_row_list_round_trips() { + let decoded = parse_pubkey_rows(&0u32.to_be_bytes()).expect("parse"); + assert!(decoded.is_empty()); + } + + // ── Registration-only invariants ────────────────────────────────── + + #[test] + fn registration_accepts_the_full_policy() { + let rows = parse_pubkey_rows(&encode(&six_key_policy())).unwrap(); + assert!(check_registration_invariants(&rows).is_ok()); + } + + #[test] + fn registration_master_check_is_order_independent() { + // Same rows, reversed: key ID 0 is no longer first, but the check + // finds it by id and still passes. + let mut policy = six_key_policy(); + policy.reverse(); + let rows = parse_pubkey_rows(&encode(&policy)).unwrap(); + assert!(check_registration_invariants(&rows).is_ok()); + } + + #[test] + fn registration_rejects_missing_key_zero() { + // A base set that skips key ID 0 (starts at 1). + let rows_no_zero: Vec = six_key_policy() + .into_iter() + .filter(|r| r.key_id != 0) + .collect(); + let rows = parse_pubkey_rows(&encode(&rows_no_zero)).unwrap(); + let err = check_registration_invariants(&rows).unwrap_err(); + assert!(err.contains("missing key ID 0"), "{err}"); + } + + #[test] + fn registration_rejects_non_master_key_zero() { + let mut policy = six_key_policy(); + policy[0].security_level = SEC_HIGH; // key 0 no longer MASTER + let rows = parse_pubkey_rows(&encode(&policy)).unwrap(); + let err = check_registration_invariants(&rows).unwrap_err(); + assert!(err.contains("MASTER + AUTHENTICATION"), "{err}"); + } + + #[test] + fn registration_rejects_duplicate_key_ids() { + let mut policy = six_key_policy(); + policy[2].key_id = 1; // collide with key 1 + let rows = parse_pubkey_rows(&encode(&policy)).unwrap(); + let err = check_registration_invariants(&rows).unwrap_err(); + assert!(err.contains("duplicate keyId 1"), "{err}"); + } + + #[test] + fn registration_rejects_empty_row_list() { + let err = check_registration_invariants(&[]).unwrap_err(); + assert!(err.contains("no keys"), "{err}"); + } + + // ── Cross-language golden fixture ───────────────────────────────── + + /// The checked-in golden blob, shared byte-for-byte with the Kotlin + /// encoder test (`RegistrationKeysTest.encoder output matches the + /// cross-language golden fixture`). Referenced from the single canonical + /// copy in the Kotlin SDK's test resources so the two can never drift. + const GOLDEN: &[u8] = include_bytes!(concat!( + env!("CARGO_MANIFEST_DIR"), + "/../kotlin-sdk/sdk/src/test/resources/golden/registration_pubkeys_v1.bin" + )); + + /// Decode the golden blob and assert the full 6-key DashPay policy — role + /// bytes, ordering, and (critically) that the ENCRYPTION / DECRYPTION rows + /// carry the REAL `dashpay_contract::ID_BYTES`, not a copied literal. Pins + /// the Kotlin-side mirrored contract id to the Rust source of truth. + #[test] + fn golden_fixture_decodes_to_the_dashpay_policy() { + let rows = parse_pubkey_rows(GOLDEN).expect("golden fixture must parse"); + assert_eq!(rows.len(), 6); + // The registration invariants hold on the canonical set. + check_registration_invariants(&rows).expect("golden fixture is a valid registration set"); + + let roles: Vec<(u32, u8, u8, u8, u8)> = rows + .iter() + .map(|r| { + ( + r.key_id, + r.key_type, + r.purpose, + r.security_level, + r.contract_bounds_kind, + ) + }) + .collect(); + assert_eq!( + roles, + vec![ + (0, KEY_TYPE_ECDSA, PURPOSE_AUTH, SEC_MASTER, 0), + (1, KEY_TYPE_ECDSA, PURPOSE_AUTH, SEC_CRITICAL, 0), + (2, KEY_TYPE_ECDSA, PURPOSE_AUTH, SEC_HIGH, 0), + (3, KEY_TYPE_ECDSA, PURPOSE_TRANSFER, SEC_CRITICAL, 0), + (4, KEY_TYPE_ECDSA, PURPOSE_ENCRYPTION, SEC_MEDIUM, 2), + (5, KEY_TYPE_ECDSA, PURPOSE_DECRYPTION, SEC_MEDIUM, 2), + ] + ); + + for row in &rows[4..] { + assert_eq!( + row.contract_bounds_id, + Some(dashpay_contract::ID_BYTES), + "DashPay enc/dec key must be bound to the canonical contract id" + ); + assert_eq!( + row.contract_bounds_document_type + .as_ref() + .map(|c| c.to_str().unwrap().to_string()), + Some("contactRequest".to_string()) + ); + } + } + + #[test] + fn update_add_key_list_without_key_zero_is_structurally_valid() { + // Proves the key-ID-0 invariant did NOT leak into the shared + // structural parser: an ordinary add-key update (keys 4 and 5, no + // key 0) parses fine — only the registration seam layers the + // key-0 invariant on top, and it is not applied here. + let update_rows = vec![ + Row { + key_id: 4, + key_type: KEY_TYPE_ECDSA, + purpose: PURPOSE_AUTH, + security_level: SEC_HIGH, + read_only: 0, + pubkey: vec![2u8; 33], + bounds: None, + }, + Row { + key_id: 5, + key_type: KEY_TYPE_ECDSA, + purpose: PURPOSE_AUTH, + security_level: SEC_HIGH, + read_only: 0, + pubkey: vec![3u8; 33], + bounds: None, + }, + ]; + let rows = parse_pubkey_rows(&encode(&update_rows)).expect("parse"); + assert_eq!(rows.len(), 2); + assert!(rows.iter().all(|r| r.key_id != 0)); + } +} diff --git a/packages/rs-unified-sdk-jni/src/transactions.rs b/packages/rs-unified-sdk-jni/src/transactions.rs index 4eb425b6a2..6daeb50391 100644 --- a/packages/rs-unified-sdk-jni/src/transactions.rs +++ b/packages/rs-unified-sdk-jni/src/transactions.rs @@ -38,6 +38,7 @@ #![allow(clippy::missing_safety_doc)] +use crate::pubkey_rows::decode_update_pubkeys_blob; use crate::support::{guard, take_pwffi_error, throw_sdk_exception}; use jni::objects::{JByteArray, JClass, JString}; use jni::sys::{jint, jlong, jstring}; @@ -240,27 +241,7 @@ pub extern "system" fn Java_org_dashfoundation_dashsdk_ffi_TransactionsNative_up } // Build the FFI rows referencing the owned buffers in `decoded`. - let ffi_rows: Vec = decoded - .iter() - .map(|row| IdentityPubkeyFFI { - key_id: row.key_id, - key_type: row.key_type, - purpose: row.purpose, - security_level: row.security_level, - pubkey_bytes: row.pubkey_bytes.as_ptr(), - pubkey_len: row.pubkey_bytes.len(), - read_only: row.read_only, - contract_bounds_kind: row.contract_bounds_kind, - contract_bounds_id: row - .contract_bounds_id - .as_ref() - .map_or(ptr::null(), |b| b.as_ptr()), - contract_bounds_document_type: row - .contract_bounds_document_type - .as_ref() - .map_or(ptr::null(), |c| c.as_ptr()), - }) - .collect(); + let ffi_rows: Vec = decoded.iter().map(|row| row.to_ffi()).collect(); let result = unsafe { platform_wallet_ffi::platform_wallet_update_identity_with_signer( @@ -287,163 +268,6 @@ pub extern "system" fn Java_org_dashfoundation_dashsdk_ffi_TransactionsNative_up }) } -/// One decoded add-key row with owned buffers backing the FFI pointers. -struct DecodedPubkeyRow { - key_id: u32, - key_type: u8, - purpose: u8, - security_level: u8, - read_only: bool, - contract_bounds_kind: u8, - pubkey_bytes: Vec, - contract_bounds_id: Option<[u8; 32]>, - contract_bounds_document_type: Option, -} - -/// Decode the identity-update add-keys BLOB documented on -/// [`Java_..._updateIdentity`]. Throws + returns None on a malformed blob. -fn decode_update_pubkeys_blob(env: &mut JNIEnv, arr: &JByteArray) -> Option> { - let bytes = match env.convert_byte_array(arr) { - Ok(b) => b, - Err(_) => { - // A null blob is a legitimate "no keys to add". - let _ = env.exception_clear(); - return Some(Vec::new()); - } - }; - if bytes.is_empty() { - return Some(Vec::new()); - } - - let mut cursor = 0usize; - let read = |cursor: &mut usize, n: usize| -> Option> { - if *cursor + n > bytes.len() { - return None; - } - let s = bytes[*cursor..*cursor + n].to_vec(); - *cursor += n; - Some(s) - }; - - let Some(count_bytes) = read(&mut cursor, 4) else { - throw_sdk_exception(env, 1, "addPubkeysBlob truncated (row count)"); - return None; - }; - let count = u32::from_be_bytes(count_bytes.try_into().ok()?) as usize; - // Length-before-allocation guard: each row is at least an 11-byte - // fixed header, so a header claiming more rows than the remaining - // payload can hold is malformed — prevents a huge `with_capacity` - // abort from a raw-JNI blob. - if count - .checked_mul(11) - .is_none_or(|need| bytes.len() - cursor < need) - { - throw_sdk_exception( - env, - 1, - &format!("addPubkeysBlob claims {count} rows but body is too short"), - ); - return None; - } - let mut rows = Vec::with_capacity(count); - for i in 0..count { - let Some(fixed) = read(&mut cursor, 4 + 1 + 1 + 1 + 1 + 1 + 2) else { - throw_sdk_exception( - env, - 1, - &format!("addPubkeysBlob truncated at row {i} header"), - ); - return None; - }; - let key_id = u32::from_be_bytes([fixed[0], fixed[1], fixed[2], fixed[3]]); - // The Kotlin encoder writes this field with writeInt (signed); a - // set sign bit means a negative key id crossed the boundary. - if key_id > i32::MAX as u32 { - throw_sdk_exception( - env, - 1, - &format!("addPubkeysBlob row {i} keyId must be non-negative"), - ); - return None; - } - let key_type = fixed[4]; - let purpose = fixed[5]; - let security_level = fixed[6]; - let read_only = fixed[7] != 0; - let contract_bounds_kind = fixed[8]; - let pubkey_len = u16::from_be_bytes([fixed[9], fixed[10]]) as usize; - - let Some(pubkey_bytes) = read(&mut cursor, pubkey_len) else { - throw_sdk_exception( - env, - 1, - &format!("addPubkeysBlob truncated at row {i} pubkey"), - ); - return None; - }; - - let mut contract_bounds_id: Option<[u8; 32]> = None; - let mut contract_bounds_document_type: Option = None; - if contract_bounds_kind != 0 { - let Some(id_bytes) = read(&mut cursor, 32) else { - throw_sdk_exception( - env, - 1, - &format!("addPubkeysBlob truncated at row {i} contractBoundsId"), - ); - return None; - }; - let mut id = [0u8; 32]; - id.copy_from_slice(&id_bytes); - contract_bounds_id = Some(id); - - if contract_bounds_kind == 2 { - let Some(dt_len_bytes) = read(&mut cursor, 2) else { - throw_sdk_exception( - env, - 1, - &format!("addPubkeysBlob truncated at row {i} docTypeLen"), - ); - return None; - }; - let dt_len = u16::from_be_bytes(dt_len_bytes.try_into().ok()?) as usize; - let Some(dt_bytes) = read(&mut cursor, dt_len) else { - throw_sdk_exception( - env, - 1, - &format!("addPubkeysBlob truncated at row {i} docType"), - ); - return None; - }; - match CString::new(dt_bytes) { - Ok(c) => contract_bounds_document_type = Some(c), - Err(_) => { - throw_sdk_exception( - env, - 1, - &format!("addPubkeysBlob row {i} docType had an interior NUL"), - ); - return None; - } - } - } - } - - rows.push(DecodedPubkeyRow { - key_id, - key_type, - purpose, - security_level, - read_only, - contract_bounds_kind, - pubkey_bytes, - contract_bounds_id, - contract_bounds_document_type, - }); - } - Some(rows) -} - // ── Document purchase / set-price ───────────────────────────────────── /// Shared purchase/set-price marshaling. `purchase == true` routes to From c31ca592d499cac502491165feb5bfdf69cb26d9 Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Tue, 21 Jul 2026 06:27:18 +0700 Subject: [PATCH 03/10] feat(kotlin-sdk): registration key-role policy + shared pubkey codec (SDK) Move the per-keyId DPP role assignment that used to live in Rust (role_for_registration_key_id) to the Kotlin side, now that the role rides the registration wire: - Hoist the rich encoder (encodeAddPubkeys + contractBoundsKind) out of IdentityUpdates into a shared IdentityPubkeyCodec, used by both the add-key update path and every registration path. - Add RegistrationKeys: the canonical role table (base 4 auth/transfer keys + the DashPay ENCRYPTION/DECRYPTION pair at MEDIUM with contactRequest contract-document bounds) and the DashPay contract-id mirror (pinned to the Rust constant by the cross-language golden fixture; handed out as a fresh copy so the process-global id can't be mutated). - The four registration entry points (registerWithWalletFunding, registerFromAddresses, resumeWithExistingAssetLock, shieldedIdentityCreateFromPool) now take List rich rows and encode via IdentityPubkeyCodec; delete the bare IdentityKeyPreview.encodeForRegistration. - Resume keeps carrying the base 4-key set it originally committed to on-chain (the DashPay pair is fresh-funding only); its per-key identity-index guard is dropped (inexpressible on the rich row, and the outpoint/index guard remains). Co-Authored-By: Claude Opus 4.8 --- .../dashsdk/ffi/IdentityNative.kt | 25 +-- .../dashsdk/identity/IdentityKeyPreview.kt | 21 --- .../dashsdk/identity/IdentityPubkeyCodec.kt | 80 +++++++++ .../dashsdk/identity/IdentityRegistration.kt | 47 ++++-- .../dashsdk/identity/IdentityUpdates.kt | 51 +----- .../dashsdk/identity/RegistrationKeys.kt | 156 ++++++++++++++++++ .../dashsdk/wallet/PlatformWalletManager.kt | 17 +- .../identity/IdentityAssetLockRecoveryTest.kt | 32 ++-- .../dashsdk/identity/RegistrationKeysTest.kt | 92 +++++++++++ 9 files changed, 393 insertions(+), 128 deletions(-) create mode 100644 packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityPubkeyCodec.kt create mode 100644 packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/RegistrationKeys.kt create mode 100644 packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/identity/RegistrationKeysTest.kt diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/IdentityNative.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/IdentityNative.kt index 9cfc532faa..696f874437 100644 --- a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/IdentityNative.kt +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/IdentityNative.kt @@ -87,11 +87,15 @@ internal object IdentityNative { * keypair a freshly-created identity is built from. * * [count] < 0 derives the canonical default set (4 keys: MASTER auth, - * CRITICAL auth, HIGH auth, TRANSFER/CRITICAL). Returns the same flat - * BLOB layout as [previewRegistrationKeys], decoded by + * CRITICAL auth, HIGH auth, TRANSFER/CRITICAL); the create-identity flow + * may request more (e.g. 6, appending the DashPay ENCRYPTION/DECRYPTION + * pair). Returns the same flat BLOB layout as [previewRegistrationKeys], + * decoded by * [org.dashfoundation.dashsdk.identity.IdentityKeyPreview.decodeAll]. - * The per-key DPP role is applied positionally by keyId at - * registration time — the row carries only the derived ECDSA keypair. + * The row carries only the derived ECDSA keypair — the per-key DPP role is + * stamped Kotlin-side by + * [org.dashfoundation.dashsdk.identity.RegistrationKeys] when the rich + * registration rows are built. * * @param resolverHandle `MnemonicResolverHandle`, needed for * watch-only / external-signable wallets; ignored for resident-key @@ -149,9 +153,10 @@ internal object IdentityNative { * Register a new identity funded from the wallet's Core balance. The * single FFI entry point the registration coordinator's body invokes. * - * @param pubkeysBlob the keys to register, big-endian: `u32 rowCount` - * then per row `u32 keyId, u16 pubkeyLen, pubkey`. Built by - * [org.dashfoundation.dashsdk.identity.IdentityKeyPreview.encodeForRegistration]. + * @param pubkeysBlob the rich key rows to register, encoded by + * [org.dashfoundation.dashsdk.identity.IdentityPubkeyCodec.encode] — each + * row carries its keyId, DPP role, and any contract bounds (built from + * [org.dashfoundation.dashsdk.identity.RegistrationKeys]). * @param signerHandle identity-key `SignerHandle`. * @param coreSignerHandle `MnemonicResolverHandle` for the asset-lock * credit-spend signature. @@ -173,9 +178,9 @@ internal object IdentityNative { * distinct from [registerIdentityWithFunding] (ID-01) which builds a * new Core asset lock. * - * @param pubkeysBlob the keys to register (same layout as - * [registerIdentityWithFunding]). Built by - * [org.dashfoundation.dashsdk.identity.IdentityKeyPreview.encodeForRegistration]. + * @param pubkeysBlob the rich key rows to register (same layout as + * [registerIdentityWithFunding], encoded by + * [org.dashfoundation.dashsdk.identity.IdentityPubkeyCodec.encode]). * @param signerHandle used for **both** the identity-key and the * platform-address signing roles (the native `VTableSigner` * dispatches by key-type byte). diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityKeyPreview.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityKeyPreview.kt index d18b06ab97..f712bf16cb 100644 --- a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityKeyPreview.kt +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityKeyPreview.kt @@ -75,26 +75,5 @@ data class IdentityKeyPreview( blob.fill(0) return rows } - - /** - * Encode the registration-key rows for - * `IdentityNative.registerIdentityWithFunding`. Only the public - * keys cross (the private material is already persisted to the - * Keystore). Layout (big-endian): `u32 rowCount` then per row - * `u32 keyId, u16 pubkeyLen, pubkey`; [keys] is taken positionally - * so `keyId == index` (row 0 = MASTER). - */ - fun encodeForRegistration(keys: List): ByteArray { - var size = 4 - keys.forEach { size += 4 + 2 + it.publicKey.size } - val buf = ByteBuffer.allocate(size) - buf.putInt(keys.size) - keys.forEachIndexed { index, key -> - buf.putInt(index) - buf.putShort(key.publicKey.size.toShort()) - buf.put(key.publicKey) - } - return buf.array() - } } } diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityPubkeyCodec.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityPubkeyCodec.kt new file mode 100644 index 0000000000..551794c731 --- /dev/null +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityPubkeyCodec.kt @@ -0,0 +1,80 @@ +package org.dashfoundation.dashsdk.identity + +import java.io.ByteArrayOutputStream +import java.io.DataOutputStream + +/** + * Shared big-endian encoder for the identity add/register public-key BLOB. + * + * One wire format is decoded on the Rust side by + * `rs-unified-sdk-jni::pubkey_rows::parse_pubkey_rows`, reused by BOTH: + * - the identity-UPDATE add-key path (`TransactionsNative.updateIdentity`), + * driven by [IdentityUpdates], and + * - every identity-CREATE registration path, driven by + * [IdentityRegistration] and `PlatformWalletManager.shieldedIdentityCreateFromPool`. + * + * Centralising the encoder here keeps the two paths byte-for-byte identical — + * the registration wire format is now the *same* rich layout the update path + * already used, so a key's DPP role (type / purpose / security level) and any + * contract bounds ride the wire rather than being reconstructed positionally + * on the Rust side. + * + * Layout (all integers big-endian), one row per key: + * ```text + * u32 rowCount + * repeat rowCount times: + * u32 keyId + * u8 keyType (DPP KeyType discriminant, 0 = ECDSA_SECP256K1) + * u8 purpose (DPP Purpose discriminant, 0 = AUTHENTICATION) + * u8 securityLevel (DPP SecurityLevel discriminant, 0 = MASTER) + * u8 readOnly (0 / 1) + * u8 contractBoundsKind (0 none, 1 SingleContract, 2 SingleContractDocumentType) + * u16 pubkeyLen + * u8[pubkeyLen] pubkeyBytes (compressed pubkey, or 20-byte HASH160) + * if contractBoundsKind != 0: + * u8[32] contractBoundsId + * if contractBoundsKind == 2: + * u16 docTypeLen, u8[docTypeLen] docType (UTF-8) + * ``` + */ +object IdentityPubkeyCodec { + + /** Encode [keys] into the add/register public-key blob documented above. */ + fun encode(keys: List): ByteArray { + val out = ByteArrayOutputStream() + val dos = DataOutputStream(out) + dos.writeInt(keys.size) + for (k in keys) { + dos.writeInt(k.keyId) + dos.writeByte(k.keyType.ffiValue) + dos.writeByte(k.purpose.ffiValue) + dos.writeByte(k.securityLevel.ffiValue) + dos.writeByte(if (k.readOnly) 1 else 0) + dos.writeByte(contractBoundsKind(k.contractBounds)) + require(k.pubkeyBytes.size <= 0xFFFF) { + "pubkeyBytes too large: ${k.pubkeyBytes.size}" + } + dos.writeShort(k.pubkeyBytes.size) + dos.write(k.pubkeyBytes) + when (val bounds = k.contractBounds) { + null -> Unit + is ContractBounds.SingleContract -> dos.write(bounds.contractId) + is ContractBounds.SingleContractDocumentType -> { + dos.write(bounds.contractId) + val dt = bounds.documentTypeName.toByteArray(Charsets.UTF_8) + require(dt.size <= 0xFFFF) { "documentTypeName too large: ${dt.size}" } + dos.writeShort(dt.size) + dos.write(dt) + } + } + } + return out.toByteArray() + } + + /** Discriminant matching the FFI: 0 none, 1 SingleContract, 2 with doc type. */ + internal fun contractBoundsKind(bounds: ContractBounds?): Int = when (bounds) { + null -> 0 + is ContractBounds.SingleContract -> 1 + is ContractBounds.SingleContractDocumentType -> 2 + } +} diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityRegistration.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityRegistration.kt index 9aac62ad72..b3f69d203c 100644 --- a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityRegistration.kt +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityRegistration.kt @@ -113,12 +113,23 @@ class IdentityRegistration internal constructor( * Rust owns rebroadcast/proof/status decisions; Kotlin never builds a * replacement funding transaction. Generic recovery cannot consume an * invitation voucher. + * + * [keys] are the rich registration rows (built via + * [RegistrationKeys.buildRegistrationRows]). Resume carries the SAME key + * set the interrupted registration originally committed to on-chain — the + * base four auth/transfer keys, **without** the DashPay pair: an + * already-spent asset lock funds a fixed key count, so retroactively + * growing the transition it funds risks a resume that fails after the + * user's DASH is already irreversibly locked (matching iOS, which excludes + * DashPay provisioning from the resume path). A user who resumes and wants + * DashPay capability adds those keys afterward via the Add Identity Key + * flow. */ suspend fun resumeWithExistingAssetLock( walletHandle: Long, lock: TrackedAssetLock, identityIndex: Int, - keys: List, + keys: List, signerHandle: Long, coreSignerHandle: Long, ): ByteArray = gate.op { @@ -130,16 +141,13 @@ class IdentityRegistration internal constructor( "identityIndex $identityIndex does not match tracked lock registrationIndex ${lock.registrationIndex}" } require(keys.isNotEmpty()) { "keys must not be empty" } - require(keys.all { it.identityIndex == lock.registrationIndex }) { - "every registration key must use tracked lock registrationIndex ${lock.registrationIndex}" - } val native = mapNativeErrors { resumeNative.call( walletHandle = walletHandle, outpointTxid = lock.outpointTxid, outpointVout = lock.outpointVout, identityIndex = identityIndex, - pubkeysBlob = IdentityKeyPreview.encodeForRegistration(keys), + pubkeysBlob = IdentityPubkeyCodec.encode(keys), signerHandle = signerHandle, coreSignerHandle = coreSignerHandle, consumeInvitationVoucher = false, @@ -201,9 +209,10 @@ class IdentityRegistration internal constructor( * a HIGH-or-CRITICAL AUTHENTICATION key, and credit transfers / * withdrawals need the TRANSFER key ("no transfer public key"). * - * The per-key DPP role is applied Rust-side by keyId at registration - * time; each row here carries only the derived keypair. The caller - * persists each row's private key to the Keystore before registering. + * Each row here carries only the derived keypair; the per-key DPP role is + * stamped Kotlin-side by [RegistrationKeys] when the rich registration rows + * are built. The caller persists each row's private key to the Keystore + * before registering. * * @param count number of keys to derive; < 0 uses the canonical * default set (4 keys). ([previewRegistrationKeys], by contrast, @@ -229,8 +238,10 @@ class IdentityRegistration internal constructor( /** * Register a new identity funded from the wallet's Core balance. The - * single entry point the registration coordinator's body calls. Keys - * must already be derived + persisted (see [previewRegistrationKeySet]). + * single entry point the registration coordinator's body calls. [keys] are + * the rich registration rows (built via + * [RegistrationKeys.buildRegistrationRows]); each row's private half must + * already be derived + persisted (see [previewRegistrationKeySet]). * * @return the 32-byte identity id. */ @@ -239,7 +250,7 @@ class IdentityRegistration internal constructor( amountDuffs: Long, accountIndex: Int, identityIndex: Int, - keys: List, + keys: List, signerHandle: Long, coreSignerHandle: Long, ): ByteArray = gate.op { @@ -252,7 +263,7 @@ class IdentityRegistration internal constructor( amountDuffs, accountIndex, identityIndex, - IdentityKeyPreview.encodeForRegistration(keys), + IdentityPubkeyCodec.encode(keys), signerHandle, coreSignerHandle, ) @@ -265,16 +276,18 @@ class IdentityRegistration internal constructor( * from [registerWithWalletFunding] (ID-01) which builds a new Core asset * lock. Keys must already be derived + persisted (see * [previewRegistrationKeySet]); [inputs] are the funding addresses (the - * greedily-packed balance-carrying Platform-payment addresses). The same - * [signerHandle] drives both the identity-key and platform-address - * signing roles. Nonces are auto-fetched Rust-side. + * greedily-packed balance-carrying Platform-payment addresses). [keys] are + * the rich registration rows (built via + * [RegistrationKeys.buildRegistrationRows]). The same [signerHandle] drives + * both the identity-key and platform-address signing roles. Nonces are + * auto-fetched Rust-side. * * @return the 32-byte identity id. */ suspend fun registerFromAddresses( walletHandle: Long, identityIndex: Int, - keys: List, + keys: List, signerHandle: Long, inputs: List, ): ByteArray = gate.op { @@ -284,7 +297,7 @@ class IdentityRegistration internal constructor( IdentityNative.registerIdentityFromAddresses( walletHandle, identityIndex, - IdentityKeyPreview.encodeForRegistration(keys), + IdentityPubkeyCodec.encode(keys), signerHandle, FundingInput.encode(inputs), ) diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityUpdates.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityUpdates.kt index 0639911df9..d9435126d5 100644 --- a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityUpdates.kt +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityUpdates.kt @@ -6,8 +6,6 @@ import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext import org.dashfoundation.dashsdk.errors.mapNativeErrors import org.dashfoundation.dashsdk.ffi.TransactionsNative -import java.io.ByteArrayOutputStream -import java.io.DataOutputStream /** * DPP identity public-key type — Kotlin mirror of the Swift `KeyType` @@ -193,57 +191,10 @@ class IdentityUpdates internal constructor( TransactionsNative.updateIdentity( walletHandle, identityId, - encodeAddPubkeys(addPublicKeys), + IdentityPubkeyCodec.encode(addPublicKeys), disablePublicKeyIds.toIntArray(), signerHandle, ) } } - - /** - * Encode [keys] into the add-pubkeys blob the FFI decodes (see - * `TransactionsNative.updateIdentity`): `u32 rowCount` then per row - * `u32 keyId, u8 keyType, u8 purpose, u8 securityLevel, u8 readOnly, - * u8 contractBoundsKind, u16 pubkeyLen, pubkey`, plus the 32-byte contract - * id (kind != 0) and `u16 docTypeLen, docType` (kind == 2). All integers - * big-endian to match the Rust reader. - */ - private fun encodeAddPubkeys(keys: List): ByteArray { - val out = ByteArrayOutputStream() - val dos = DataOutputStream(out) - dos.writeInt(keys.size) - for (k in keys) { - dos.writeInt(k.keyId) - dos.writeByte(k.keyType.ffiValue) - dos.writeByte(k.purpose.ffiValue) - dos.writeByte(k.securityLevel.ffiValue) - dos.writeByte(if (k.readOnly) 1 else 0) - val kind = contractBoundsKind(k.contractBounds) - dos.writeByte(kind) - require(k.pubkeyBytes.size <= 0xFFFF) { - "pubkeyBytes too large: ${k.pubkeyBytes.size}" - } - dos.writeShort(k.pubkeyBytes.size) - dos.write(k.pubkeyBytes) - when (val bounds = k.contractBounds) { - null -> Unit - is ContractBounds.SingleContract -> dos.write(bounds.contractId) - is ContractBounds.SingleContractDocumentType -> { - dos.write(bounds.contractId) - val dt = bounds.documentTypeName.toByteArray(Charsets.UTF_8) - require(dt.size <= 0xFFFF) { "documentTypeName too large: ${dt.size}" } - dos.writeShort(dt.size) - dos.write(dt) - } - } - } - return out.toByteArray() - } - - /** Discriminant matching the FFI: 0 none, 1 SingleContract, 2 with doc type. */ - private fun contractBoundsKind(bounds: ContractBounds?): Int = when (bounds) { - null -> 0 - is ContractBounds.SingleContract -> 1 - is ContractBounds.SingleContractDocumentType -> 2 - } } diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/RegistrationKeys.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/RegistrationKeys.kt new file mode 100644 index 0000000000..1d26b799ab --- /dev/null +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/RegistrationKeys.kt @@ -0,0 +1,156 @@ +package org.dashfoundation.dashsdk.identity + +/** + * The canonical DPP-role layout for a freshly-registered identity's keys — + * the Kotlin source of truth that replaces the retired Rust + * `role_for_registration_key_id`. Since the registration wire format now + * carries each key's full role (see [IdentityPubkeyCodec]) instead of + * reconstructing it positionally on the Rust side, the layout is stamped here + * and shipped over the boundary. + * + * Byte-for-byte identical to the iOS reference (SwiftExampleApp's + * `CreateIdentityView.defaultKeyCount` role table + `makeDashpayKeyPair`): + * + * | keyId | keyType | purpose | securityLevel | bounds | + * |-------|-----------------|----------------|---------------|---------------------------| + * | 0 | ECDSA_SECP256K1 | AUTHENTICATION | MASTER | — | + * | 1 | ECDSA_SECP256K1 | AUTHENTICATION | CRITICAL | — | + * | 2 | ECDSA_SECP256K1 | AUTHENTICATION | HIGH | — | + * | 3 | ECDSA_SECP256K1 | TRANSFER | CRITICAL | — | + * | 4 | ECDSA_SECP256K1 | ENCRYPTION | MEDIUM | DashPay / contactRequest | + * | 5 | ECDSA_SECP256K1 | DECRYPTION | MEDIUM | DashPay / contactRequest | + * + * - keyId 0 (MASTER/AUTH) signs the IdentityCreate / IdentityUpdate transition. + * - keyId 1 (CRITICAL/AUTH) signs token state transitions — without it the + * identity can't mint / burn / freeze tokens. + * - keyId 2 (HIGH/AUTH) signs general document / DPNS / contract transitions. + * - keyId 3 (TRANSFER/CRITICAL) signs IdentityCreditTransfer / …Withdrawal — + * without it those broadcasts are rejected on-chain with "no transfer public + * key". + * - keyId 4 (ENCRYPTION) + keyId 5 (DECRYPTION), bound to DashPay's + * `contactRequest` document type, let the new identity send / receive + * contact requests. Without them the app's own Add Contact flow rejects the + * identity: `select_own_encryption_key` requires an enabled ECDSA + * ENCRYPTION key. These two are appended only on fresh-funding registration, + * not on asset-lock resume (which never grows past the key set it originally + * committed to on-chain). + */ +object RegistrationKeys { + + /** Base auth/transfer key count (keyIds 0..3), every registration path. */ + const val BASE_KEY_COUNT: Int = 4 + + /** Extra DashPay ENCRYPTION + DECRYPTION keys (keyIds 4..5). */ + const val DASHPAY_KEY_COUNT: Int = 2 + + /** Immutable backing bytes for [DASHPAY_CONTRACT_ID] — never handed out directly. */ + private val DASHPAY_CONTRACT_ID_BYTES: ByteArray = byteArrayOf( + 162.toByte(), 161.toByte(), 180.toByte(), 172.toByte(), 111, 239.toByte(), 34, 234.toByte(), + 42, 26, 104, 232.toByte(), 18, 54, 68, 179.toByte(), + 87, 135.toByte(), 95, 107, 65, 44, 24, 16, + 146.toByte(), 129.toByte(), 193.toByte(), 70, 231.toByte(), 178.toByte(), 113, 188.toByte(), + ) + + /** + * DashPay data-contract id (32 bytes) — source of truth + * `packages/dashpay-contract/src/lib.rs::ID_BYTES`. Mirrored here so the + * contract-bounds payload for the ENCRYPTION / DECRYPTION keys can be built + * without a contract fetch; pinned against the Rust constant by the + * cross-language golden fixture in the tests. Network-agnostic. + * + * Returns a fresh copy on every access — the id is process-global, so + * handing out the backing array would let a stray write corrupt every + * DashPay-bound key thereafter. + */ + val DASHPAY_CONTRACT_ID: ByteArray + get() = DASHPAY_CONTRACT_ID_BYTES.copyOf() + + /** + * The DashPay document type the ENCRYPTION / DECRYPTION keys are bound to — + * the only one in the contract that declares + * `requiresIdentityEncryptionBoundedKey`. + */ + const val DASHPAY_CONTACT_REQUEST_DOCUMENT_TYPE: String = "contactRequest" + + /** Total keys a registration derives for the given DashPay choice. */ + fun keyCount(includeDashPayKeys: Boolean): Int = + BASE_KEY_COUNT + if (includeDashPayKeys) DASHPAY_KEY_COUNT else 0 + + /** + * Stamp the canonical DPP roles onto [publicKeys], producing the rich + * [IdentityPubkey] rows the registration wire format carries. [publicKeys] + * must be exactly [keyCount] entries in keyId order (index i → keyId i); + * each is the on-chain public payload for that slot (the derived + * compressed pubkey, already persisted to the Keystore by the caller). No + * private material is touched here. + * + * @throws IllegalArgumentException if the count doesn't match the policy. + */ + fun buildRegistrationRows( + publicKeys: List, + includeDashPayKeys: Boolean, + ): List { + val expected = keyCount(includeDashPayKeys) + require(publicKeys.size == expected) { + "expected $expected registration public keys, got ${publicKeys.size}" + } + return publicKeys.mapIndexed { keyId, pubkey -> rowFor(keyId, pubkey) } + } + + /** The DashPay contact-request contract-document bounds for the enc/dec keys. */ + private fun dashPayBounds(): ContractBounds = + ContractBounds.SingleContractDocumentType( + // The accessor already returns a fresh copy the bounds object owns. + contractId = DASHPAY_CONTRACT_ID, + documentTypeName = DASHPAY_CONTACT_REQUEST_DOCUMENT_TYPE, + ) + + private fun rowFor(keyId: Int, pubkey: ByteArray): IdentityPubkey = when (keyId) { + 0 -> IdentityPubkey( + keyId = 0, + keyType = KeyType.ECDSA_SECP256K1, + purpose = KeyPurpose.AUTHENTICATION, + securityLevel = SecurityLevel.MASTER, + pubkeyBytes = pubkey, + ) + 1 -> IdentityPubkey( + keyId = 1, + keyType = KeyType.ECDSA_SECP256K1, + purpose = KeyPurpose.AUTHENTICATION, + securityLevel = SecurityLevel.CRITICAL, + pubkeyBytes = pubkey, + ) + 2 -> IdentityPubkey( + keyId = 2, + keyType = KeyType.ECDSA_SECP256K1, + purpose = KeyPurpose.AUTHENTICATION, + securityLevel = SecurityLevel.HIGH, + pubkeyBytes = pubkey, + ) + 3 -> IdentityPubkey( + keyId = 3, + keyType = KeyType.ECDSA_SECP256K1, + purpose = KeyPurpose.TRANSFER, + securityLevel = SecurityLevel.CRITICAL, + pubkeyBytes = pubkey, + ) + 4 -> IdentityPubkey( + keyId = 4, + keyType = KeyType.ECDSA_SECP256K1, + purpose = KeyPurpose.ENCRYPTION, + securityLevel = SecurityLevel.MEDIUM, + pubkeyBytes = pubkey, + contractBounds = dashPayBounds(), + ) + 5 -> IdentityPubkey( + keyId = 5, + keyType = KeyType.ECDSA_SECP256K1, + purpose = KeyPurpose.DECRYPTION, + securityLevel = SecurityLevel.MEDIUM, + pubkeyBytes = pubkey, + contractBounds = dashPayBounds(), + ) + // Unreachable: buildRegistrationRows caps the count at keyCount(true) = 6. + else -> error("no registration role defined for keyId $keyId") + } +} diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/PlatformWalletManager.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/PlatformWalletManager.kt index 3ec9f03736..48eca431f6 100644 --- a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/PlatformWalletManager.kt +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/PlatformWalletManager.kt @@ -1332,19 +1332,20 @@ class PlatformWalletManager( * Swift's `shieldedIdentityCreateFromPool`. Spends a note of the fixed * exit [denomination] (credits — one of the on-chain 0.1/0.3/0.5/1.0 DASH * denominations) from the wallet's bound Orchard pool ([account]) to fund - * a new identity at [identityIndex]. [keys] are the derived + persisted - * canonical registration keys, encoded to the same blob ID-08 uses. - * [fallbackAddress] is the REQUIRED 21-byte PlatformAddress that receives - * the value (minus a penalty) if creation fails a stateful check. Signed - * by the Keystore identity signer ([signerHandle]). Blocks for the ~30s - * Halo 2 proof. + * a new identity at [identityIndex]. [keys] are the rich registration rows + * (built via `RegistrationKeys.buildRegistrationRows`), encoded to the same + * blob every registration path uses; each row's private half must already + * be persisted. [fallbackAddress] is the REQUIRED 21-byte PlatformAddress + * that receives the value (minus a penalty) if creation fails a stateful + * check. Signed by the Keystore identity signer ([signerHandle]). Blocks + * for the ~30s Halo 2 proof. * * @return the new 32-byte identity id. */ suspend fun shieldedIdentityCreateFromPool( walletId: ByteArray, identityIndex: Int, - keys: List, + keys: List, denomination: Long, fallbackAddress: ByteArray, account: Int = 0, @@ -1362,7 +1363,7 @@ class PlatformWalletManager( mnemonicResolver.nativeHandle, account, identityIndex, - org.dashfoundation.dashsdk.identity.IdentityKeyPreview.encodeForRegistration(keys), + org.dashfoundation.dashsdk.identity.IdentityPubkeyCodec.encode(keys), denomination, fallbackAddress, signerHandle, diff --git a/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/identity/IdentityAssetLockRecoveryTest.kt b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/identity/IdentityAssetLockRecoveryTest.kt index c6cf3507e1..ab44689b52 100644 --- a/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/identity/IdentityAssetLockRecoveryTest.kt +++ b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/identity/IdentityAssetLockRecoveryTest.kt @@ -111,26 +111,6 @@ class IdentityAssetLockRecoveryTest { assertFalse(called) } - @Test - fun `registration rejects keys for another identity index before JNI`() = runTest { - var called = false - val registration = IdentityRegistration( - resumeNative = ResumeIdentityNativeCall { _, _, _, _, _, _, _, _ -> - called = true - IdentityRegistrationNativeResult(ByteArray(32), 1) - }, - ) - - val failure = runCatching { - registration.resumeWithExistingAssetLock( - 1, lock(TrackedAssetLock.FundingType.IDENTITY_REGISTRATION), - 7, listOf(key(identityIndex = 8)), 2, 3, - ) - } - assertTrue(failure.exceptionOrNull() is IllegalArgumentException) - assertFalse(called) - } - @Test fun `manager teardown waits for registration resolver borrow`() = runBlocking { val gate = TeardownGate() @@ -165,8 +145,16 @@ class IdentityAssetLockRecoveryTest { assertTrue(closed) } - private fun key(identityIndex: Int = 7) = - IdentityKeyPreview(identityIndex, "m/9'/$identityIndex'", ByteArray(33) { 2 }, ByteArray(32)) + // Resume carries the rich base MASTER row; the resume path only checks the + // list is non-empty (the key set is fixed by the tracked lock, not + // re-validated per key here). + private fun key() = IdentityPubkey( + keyId = 0, + keyType = KeyType.ECDSA_SECP256K1, + purpose = KeyPurpose.AUTHENTICATION, + securityLevel = SecurityLevel.MASTER, + pubkeyBytes = ByteArray(33) { 2 }, + ) private fun lock(type: TrackedAssetLock.FundingType) = TrackedAssetLock( outpointTxid = ByteArray(32) { 4 }, diff --git a/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/identity/RegistrationKeysTest.kt b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/identity/RegistrationKeysTest.kt new file mode 100644 index 0000000000..d49eb13c81 --- /dev/null +++ b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/identity/RegistrationKeysTest.kt @@ -0,0 +1,92 @@ +package org.dashfoundation.dashsdk.identity + +import org.junit.Assert.assertArrayEquals +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +class RegistrationKeysTest { + + /** Deterministic fixture pubkey for slot [keyId]: 33 bytes of `keyId + 2`. */ + private fun fixturePubkey(keyId: Int): ByteArray = ByteArray(33) { (keyId + 2).toByte() } + + private fun fixturePubkeys(count: Int): List = (0 until count).map(::fixturePubkey) + + @Test + fun `base four rows carry the canonical auth and transfer roles`() { + val rows = RegistrationKeys.buildRegistrationRows(fixturePubkeys(4), includeDashPayKeys = false) + assertEquals(4, rows.size) + assertEquals( + listOf(0, 1, 2, 3), + rows.map { it.keyId }, + ) + assertEquals( + listOf( + Triple(KeyPurpose.AUTHENTICATION, SecurityLevel.MASTER, null), + Triple(KeyPurpose.AUTHENTICATION, SecurityLevel.CRITICAL, null), + Triple(KeyPurpose.AUTHENTICATION, SecurityLevel.HIGH, null), + Triple(KeyPurpose.TRANSFER, SecurityLevel.CRITICAL, null), + ), + rows.map { Triple(it.purpose, it.securityLevel, it.contractBounds) }, + ) + rows.forEach { assertEquals(KeyType.ECDSA_SECP256K1, it.keyType) } + } + + @Test + fun `six rows append the DashPay encryption and decryption pair`() { + val rows = RegistrationKeys.buildRegistrationRows(fixturePubkeys(6), includeDashPayKeys = true) + assertEquals(6, rows.size) + + val enc = rows[4] + val dec = rows[5] + assertEquals(KeyPurpose.ENCRYPTION, enc.purpose) + assertEquals(KeyPurpose.DECRYPTION, dec.purpose) + for (row in listOf(enc, dec)) { + assertEquals(SecurityLevel.MEDIUM, row.securityLevel) + val bounds = row.contractBounds + assertTrue(bounds is ContractBounds.SingleContractDocumentType) + bounds as ContractBounds.SingleContractDocumentType + assertArrayEquals(RegistrationKeys.DASHPAY_CONTRACT_ID, bounds.contractId) + assertEquals( + RegistrationKeys.DASHPAY_CONTACT_REQUEST_DOCUMENT_TYPE, + bounds.documentTypeName, + ) + } + // The base four keep their auth/transfer roles unbounded. + (0..3).forEach { assertNull(rows[it].contractBounds) } + } + + @Test + fun `wrong key count is rejected`() { + val failure = runCatching { + RegistrationKeys.buildRegistrationRows(fixturePubkeys(5), includeDashPayKeys = true) + } + assertTrue(failure.exceptionOrNull() is IllegalArgumentException) + } + + /** + * Cross-language wire pin: the Kotlin encoder must produce exactly the + * checked-in golden bytes. A byte-identical Rust test + * (`pubkey_rows::tests::golden_fixture_decodes_to_the_dashpay_policy`) + * decodes the SAME file and asserts the DashPay-bound rows carry + * `dashpay_contract::ID_BYTES`, so this catches byte-order / field-order + * skew that two independently-passing tests would miss — and pins the + * mirrored DashPay contract id to the real Rust constant transitively. + */ + @Test + fun `encoder output matches the cross-language golden fixture`() { + val golden = javaClass.getResourceAsStream("/golden/registration_pubkeys_v1.bin") + .use { requireNotNull(it) { "golden fixture resource missing" }.readBytes() } + + val rows = RegistrationKeys.buildRegistrationRows(fixturePubkeys(6), includeDashPayKeys = true) + val encoded = IdentityPubkeyCodec.encode(rows) + + assertArrayEquals( + "Kotlin encoder drifted from the checked-in golden fixture — the Rust " + + "parser will misread the registration blob (wire-format skew)", + golden, + encoded, + ) + } +} From 4a25717471232571975a850262530415e05d5385 Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Tue, 21 Jul 2026 06:27:33 +0700 Subject: [PATCH 04/10] feat(kotlin-sdk): provision DashPay keys on fresh identity registration (app) Wire the DashPay ENCRYPTION/DECRYPTION pair into the create-identity flow so a freshly registered identity can send contact requests immediately, matching iOS. - New DashpayKeyProvisioning: derive the full key set in ONE pass, persist each private scalar to the Keystore, scrub the JVM copy (on both success and every failure path, including the count-mismatch rejection), and build the rich IdentityPubkey rows via RegistrationKeys. A dedicated helper rather than reusing IdentityKeyAdditionFlow, which assigns keyIds as max+1 (never key 0), derives per-slot, and assumes post-registration persistence. - CreateIdentityScreen derives keyCount(includeDashPay) keys (6 for fresh funding, 4 for resume) in the single preview pass and dispatches the rich rows. Fresh Core / Platform-address / shielded paths provision DashPay; asset-lock resume is excluded (CreateIdentityFundingSource. includesDashPayKeys), matching iOS's own resume exclusion. - AddIdentityKeyScreen's DashPay bounds entry now reads the SDK's single RegistrationKeys constant instead of a private copy. app :testDebugUnitTest green (DashpayKeyProvisioningTest: 6-vs-4 selection, scrub-on-success, scrub-on-partial-failure, scrub-on-count-mismatch). Co-Authored-By: Claude Opus 4.8 --- .../services/DashpayKeyProvisioning.kt | 100 ++++++++++++++ .../ui/identity/AddIdentityKeyScreen.kt | 17 ++- .../ui/identity/CreateIdentityScreen.kt | 71 +++++----- .../services/DashpayKeyProvisioningTest.kt | 124 ++++++++++++++++++ 4 files changed, 272 insertions(+), 40 deletions(-) create mode 100644 packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/services/DashpayKeyProvisioning.kt create mode 100644 packages/kotlin-sdk/KotlinExampleApp/app/src/test/java/org/dashfoundation/example/services/DashpayKeyProvisioningTest.kt diff --git a/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/services/DashpayKeyProvisioning.kt b/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/services/DashpayKeyProvisioning.kt new file mode 100644 index 0000000000..1da7aa8386 --- /dev/null +++ b/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/services/DashpayKeyProvisioning.kt @@ -0,0 +1,100 @@ +package org.dashfoundation.example.services + +import org.dashfoundation.dashsdk.identity.IdentityKeyPreview +import org.dashfoundation.dashsdk.identity.IdentityPubkey +import org.dashfoundation.dashsdk.identity.RegistrationKeys + +/** + * Registration key provisioning — the derive-set → persist → zero → + * build-rich-rows sequence the create-identity flow runs before it broadcasts. + * App-level analog of Swift's `CreateIdentityView` submit path + * (`prePersistIdentityKeysForRegistration` + `makeDashpayKeyPair`). + * + * ## Why a dedicated helper rather than reusing `IdentityKeyAdditionFlow` + * + * `IdentityKeyAdditionFlow.prepareKeys` looks similar (it also derives → + * persists → zeroes → builds [IdentityPubkey] rows), but it is built for + * adding keys to an *existing* identity and does not fit registration: + * - it assigns key IDs as `max(existing) + 1`, so it can never produce the + * MASTER slot at key ID 0 that registration requires; + * - it derives one slot at a time through an injected `SlotKeyDeriver`, + * whereas registration derives the whole key set in a **single** + * `previewRegistrationKeySet` pass — the secret-lifecycle mandate is one + * derivation, not N overlapping ones; + * - its lifecycle assumes post-registration persistence (an existing + * identity id), not the pre-broadcast persistence registration needs. + * + * Forcing registration through it would fork its semantics; this helper reuses + * the shared pieces instead — the SDK's [RegistrationKeys] role table for the + * on-chain rows and the same store-then-zero discipline for the secrets. + */ +object DashpayKeyProvisioning { + + /** + * Persists one derived identity private key (public-key hex → scalar), + * owner-scoped so wallet deletion can reach it before the identity's + * `public_keys` row commits. Injected so the provisioning logic is + * unit-testable without an Android Keystore. + */ + fun interface PrivateKeyPersister { + suspend fun persist(publicKeyHex: String, privateKey: ByteArray, ownerWalletId: ByteArray) + } + + /** + * Persist each derived private key, scrub the JVM copy, and return the + * rich [IdentityPubkey] rows the registration wire format carries. + * + * [previews] must be exactly `RegistrationKeys.keyCount(includeDashPayKeys)` + * rows in key-ID order — the single [IdentityKeyPreview] derivation pass + * (`previewRegistrationKeySet`). Every preview's private array is zeroed on + * both the success and every failure path (including the count-mismatch + * rejection); a partial persist failure never leaves a plaintext scalar + * behind. Only the public halves flow into the returned rows. + * + * The derived scalar and its public key come from the same Rust preview + * row, so they are paired by construction — this deliberately does not + * re-run Swift's `validatePrivateKeyForPublicKey` cross-check (there is no + * private-key math on the Kotlin side to check against; a pub/priv mismatch + * would require a Rust derivation bug, which for key 0 fails the create + * signature immediately). + */ + suspend fun provision( + previews: List, + includeDashPayKeys: Boolean, + walletId: ByteArray, + persister: PrivateKeyPersister, + ): List { + val expected = RegistrationKeys.keyCount(includeDashPayKeys) + try { + // Inside the try so the catch below scrubs the derived scalars even + // on a count-mismatch rejection (a wire-skew symptom) — no plaintext + // key may survive any failure once the previews have been derived. + require(previews.size == expected) { + "expected $expected derived registration keys, got ${previews.size}" + } + for (preview in previews) { + try { + // The one allowed Kotlin persist step (kotlin-sdk/CLAUDE.md): + // Rust derived the scalar, we only encrypt it into the + // Keystore keyed by the public bytes the signer looks up. + persister.persist(preview.publicKeyHex, preview.privateKey, walletId) + } finally { + // Keystore is authoritative from here; the JVM copy must not + // outlive the store (IdentityKeyPreview's retention rule). + preview.privateKey.fill(0) + } + } + } catch (e: Throwable) { + // A persist threw after its own key was zeroed by the finally + // above; scrub every remaining preview's private half too so no + // plaintext scalar survives a partial failure. + previews.forEach { it.privateKey.fill(0) } + throw e + } + // Only the public keys cross into the rich rows — the privates are gone. + return RegistrationKeys.buildRegistrationRows( + previews.map { it.publicKey }, + includeDashPayKeys, + ) + } +} diff --git a/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/identity/AddIdentityKeyScreen.kt b/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/identity/AddIdentityKeyScreen.kt index e73f8d71db..5cc95cd7fb 100644 --- a/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/identity/AddIdentityKeyScreen.kt +++ b/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/identity/AddIdentityKeyScreen.kt @@ -381,19 +381,18 @@ private data class BoundsPickerEntry( /** * DashPay — the one system contract whose schema declares * `requiresIdentityEncryptionBoundedKey` (on `contactRequest` only, so - * contract scope is not allowed). ID bytes: - * `packages/dashpay-contract/src/lib.rs::ID_BYTES`; network-agnostic. + * contract scope is not allowed). The contract id + document type come from + * the SDK's single source of truth + * ([org.dashfoundation.dashsdk.identity.RegistrationKeys]), the same policy the + * registration flow provisions its DashPay keys from. */ private fun dashPaySystemEntry(): BoundsPickerEntry = BoundsPickerEntry( - idHex = byteArrayOf( - 162.toByte(), 161.toByte(), 180.toByte(), 172.toByte(), 111, 239.toByte(), 34, 234.toByte(), - 42, 26, 104, 232.toByte(), 18, 54, 68, 179.toByte(), - 87, 135.toByte(), 95, 107, 65, 44, 24, 16, - 146.toByte(), 129.toByte(), 193.toByte(), 70, 231.toByte(), 178.toByte(), 113, 188.toByte(), - ).toHexLocal(), + idHex = org.dashfoundation.dashsdk.identity.RegistrationKeys.DASHPAY_CONTRACT_ID.toHexLocal(), displayName = "DashPay (System)", allowsContractScope = false, - documentTypes = listOf("contactRequest"), + documentTypes = listOf( + org.dashfoundation.dashsdk.identity.RegistrationKeys.DASHPAY_CONTACT_REQUEST_DOCUMENT_TYPE, + ), ) private fun ByteArray.toHexLocal(): String = joinToString("") { "%02x".format(it) } diff --git a/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/identity/CreateIdentityScreen.kt b/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/identity/CreateIdentityScreen.kt index 35be6a7624..4c2138eea0 100644 --- a/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/identity/CreateIdentityScreen.kt +++ b/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/identity/CreateIdentityScreen.kt @@ -37,6 +37,7 @@ import org.dashfoundation.dashsdk.wallet.TrackedAssetLock import org.dashfoundation.example.di.LocalAppContainer import org.dashfoundation.example.di.LocalAppState import org.dashfoundation.example.navigation.RegistrationProgress +import org.dashfoundation.example.services.DashpayKeyProvisioning import org.dashfoundation.example.services.IdentityRegistrationController import org.dashfoundation.example.services.assetlock.IdentityAssetLockRecovery import org.dashfoundation.example.ui.components.AccessiblePicker @@ -196,43 +197,39 @@ fun CreateIdentityScreen(navController: NavHostController) { identityIndexText.toIntOrNull() ?: return@SubmitButton } val mgr = manager ?: return@SubmitButton + val registerDashPayKeys = fundingSource.includesDashPayKeys isSubmitting = true scope.launch { try { - // Step 1 (`.preparingKeys`): derive + persist the full - // canonical identity key SET — the single allowed Kotlin - // persist step. Rust derives keyId 0..3 (MASTER auth, - // CRITICAL auth, HIGH auth, TRANSFER/CRITICAL) at this - // identity index and stamps each key's role by keyId at - // registration; we store each private key under its - // pubkey hex. Deriving only the MASTER key here (the old - // `count = 1` bug) left the identity unable to sign any - // document / token / transfer / withdrawal write. - val keys = mgr.identityRegistration.previewRegistrationKeySet( + // Step 1 (`.preparingKeys`): derive the full registration + // key set in ONE pass, persist each private key to the + // Keystore, and build the rich on-chain rows. Rust derives + // keyId 0..N (base: MASTER auth, CRITICAL auth, HIGH auth, + // TRANSFER/CRITICAL; plus the DashPay ENCRYPTION/DECRYPTION + // pair on fresh funding), and the DPP role for each keyId + // is stamped Kotlin-side by RegistrationKeys and shipped + // over the wire. Fresh funding registers all 6 keys so the + // new identity can send contact requests immediately; + // asset-lock resume keeps the base 4 it originally + // committed to on-chain (§ DashPay resume exclusion). + val previews = mgr.identityRegistration.previewRegistrationKeySet( walletHandle = wallet.handle, mnemonicResolverHandle = mgr.mnemonicResolverHandle, identityIndex = identityIndex, - count = -1, + count = org.dashfoundation.dashsdk.identity.RegistrationKeys + .keyCount(registerDashPayKeys), + ) + val keys = DashpayKeyProvisioning.provision( + previews = previews, + includeDashPayKeys = registerDashPayKeys, + walletId = wallet.walletId, + // Prestored BEFORE the registration broadcasts: only + // the owner index makes it reachable by wallet deletion + // until the public_keys row commits. + persister = { hex, priv, owner -> + container.walletStorage.storePrivateKey(hex, priv, ownerWalletId = owner) + }, ) - keys.forEach { key -> - try { - container.walletStorage.storePrivateKey( - key.publicKeyHex, - key.privateKey, - // Prestored BEFORE the registration - // broadcasts: only the owner index makes - // it reachable by wallet deletion until - // the public_keys row commits. - ownerWalletId = wallet.walletId, - ) - } finally { - // Keystore is authoritative from here; the JVM - // copy must not outlive the store (the - // IdentityKeyPreview retention rule — the - // registration blob reads only publicKey). - key.privateKey.fill(0) - } - } // Step 2: hand the single registration FFI entry point to // the coordinator as the body — no orchestration here. The // funding source picks which registration FFI runs (ID-01 @@ -388,7 +385,19 @@ enum class CreateIdentityFundingSource( CoreBalance("Core balance", amountInCredits = false), PlatformAddress("Platform address", amountInCredits = true), ShieldedBalance("Shielded balance", amountInCredits = true), - AssetLockResume("Resume from asset lock", amountInCredits = false), + AssetLockResume("Resume from asset lock", amountInCredits = false); + + /** + * Whether this funding path provisions the DashPay ENCRYPTION/DECRYPTION + * pair alongside the base four keys. Every fresh-funding path does; + * [AssetLockResume] does NOT — a resumed asset lock funds the fixed key + * count it was originally built for, so growing the transition it funds + * risks a resume that fails after the DASH is already locked. Matches iOS, + * which excludes DashPay provisioning from the resume path. A resumed + * identity gets DashPay capability afterward via the Add Identity Key flow. + */ + val includesDashPayKeys: Boolean + get() = this != AssetLockResume } /** diff --git a/packages/kotlin-sdk/KotlinExampleApp/app/src/test/java/org/dashfoundation/example/services/DashpayKeyProvisioningTest.kt b/packages/kotlin-sdk/KotlinExampleApp/app/src/test/java/org/dashfoundation/example/services/DashpayKeyProvisioningTest.kt new file mode 100644 index 0000000000..98f5344446 --- /dev/null +++ b/packages/kotlin-sdk/KotlinExampleApp/app/src/test/java/org/dashfoundation/example/services/DashpayKeyProvisioningTest.kt @@ -0,0 +1,124 @@ +package org.dashfoundation.example.services + +import kotlinx.coroutines.test.runTest +import org.dashfoundation.dashsdk.identity.ContractBounds +import org.dashfoundation.dashsdk.identity.IdentityKeyPreview +import org.dashfoundation.dashsdk.identity.KeyPurpose +import org.dashfoundation.dashsdk.identity.RegistrationKeys +import org.dashfoundation.dashsdk.identity.SecurityLevel +import org.dashfoundation.example.ui.identity.CreateIdentityFundingSource +import org.junit.Assert.assertArrayEquals +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * Lifecycle + policy coverage for [DashpayKeyProvisioning] and the + * funding-source DashPay-provisioning gate. Verifies the exact 6-key + * (fresh) / 4-key (resume) row set, and that every derived private scalar is + * scrubbed on both the success and the failure path. + */ +class DashpayKeyProvisioningTest { + + /** A derived preview at [keyId] with a non-zero private scalar. */ + private fun preview(keyId: Int): IdentityKeyPreview = IdentityKeyPreview( + identityIndex = 5, + derivationPath = "m/9'/5'/$keyId'", + publicKey = ByteArray(33) { (keyId + 2).toByte() }, + privateKey = ByteArray(32) { (keyId + 1).toByte() }, + ) + + @Test + fun `fresh funding provisions all six keys with DashPay bounds`() = runTest { + val previews = (0 until 6).map(::preview) + val stored = mutableListOf() + val rows = DashpayKeyProvisioning.provision( + previews = previews, + includeDashPayKeys = true, + walletId = ByteArray(32) { 9 }, + persister = { hex, _, _ -> stored += hex }, + ) + + assertEquals(6, rows.size) + assertEquals(listOf(0, 1, 2, 3, 4, 5), rows.map { it.keyId }) + assertEquals(KeyPurpose.ENCRYPTION, rows[4].purpose) + assertEquals(KeyPurpose.DECRYPTION, rows[5].purpose) + assertEquals(SecurityLevel.MEDIUM, rows[4].securityLevel) + val bounds = rows[4].contractBounds + assertTrue(bounds is ContractBounds.SingleContractDocumentType) + bounds as ContractBounds.SingleContractDocumentType + assertArrayEquals(RegistrationKeys.DASHPAY_CONTRACT_ID, bounds.contractId) + + // Every private scalar was persisted then scrubbed. + assertEquals(6, stored.size) + for (p in previews) { + assertArrayEquals(ByteArray(32), p.privateKey) + } + } + + @Test + fun `resume provisions only the base four keys`() = runTest { + val previews = (0 until 4).map(::preview) + val rows = DashpayKeyProvisioning.provision( + previews = previews, + includeDashPayKeys = false, + walletId = ByteArray(32) { 9 }, + persister = { _, _, _ -> }, + ) + assertEquals(4, rows.size) + assertTrue(rows.none { it.contractBounds != null }) + } + + @Test + fun `a persist failure still scrubs every private scalar`() = runTest { + val previews = (0 until 6).map(::preview) + val failure = runCatching { + DashpayKeyProvisioning.provision( + previews = previews, + includeDashPayKeys = true, + walletId = ByteArray(32) { 9 }, + // Fail on the 3rd key — keys 0/1 persisted, 3/4/5 never reached. + persister = { _, _, _ -> + if (persistCount++ == 2) throw IllegalStateException("keystore down") + }, + ) + } + assertTrue(failure.exceptionOrNull() is IllegalStateException) + // No plaintext scalar may survive a partial failure. + for (p in previews) { + assertArrayEquals(ByteArray(32), p.privateKey) + } + } + + private var persistCount = 0 + + @Test + fun `wrong preview count is rejected before any persist and still scrubs scalars`() = runTest { + var persisted = false + val previews = (0 until 5).map(::preview) // neither 4 nor 6 + val failure = runCatching { + DashpayKeyProvisioning.provision( + previews = previews, + includeDashPayKeys = true, + walletId = ByteArray(32) { 9 }, + persister = { _, _, _ -> persisted = true }, + ) + } + assertTrue(failure.exceptionOrNull() is IllegalArgumentException) + assertFalse(persisted) + // The count rejection is a wire-skew symptom fired right after + // derivation — no plaintext scalar may survive it. + for (p in previews) { + assertArrayEquals(ByteArray(32), p.privateKey) + } + } + + @Test + fun `only asset-lock resume excludes DashPay provisioning`() { + assertTrue(CreateIdentityFundingSource.CoreBalance.includesDashPayKeys) + assertTrue(CreateIdentityFundingSource.PlatformAddress.includesDashPayKeys) + assertTrue(CreateIdentityFundingSource.ShieldedBalance.includesDashPayKeys) + assertFalse(CreateIdentityFundingSource.AssetLockResume.includesDashPayKeys) + } +} From 5147d5baa94e922b8bb8a75a04106924112f2d6a Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Tue, 21 Jul 2026 06:37:13 +0700 Subject: [PATCH 05/10] refactor(kotlin-sdk): share the persist-and-scrub secret primitive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The registration-key provisioning and the add-key flow each had their own store-then-zero loop for identity private keys — two places to get the secret-scrubbing discipline right (and where the one review bug lived). Extract that single-scalar step into IdentityKeyPersistence.storeAndScrub (store under the pubkey hex, then zero the JVM copy on both the success and the failure path) and route both DashpayKeyProvisioning and IdentityKeyAdditionFlow through it. The two flows keep their distinct keyId/derivation logic (explicit 0..N + single batch pass vs. max+1 + per-slot) — only the dangerous persist+scrub primitive is unified. Behavior-preserving: app :testDebugUnitTest green (DashpayKeyProvisioning 5, IdentityKeys 6). Co-Authored-By: Claude Opus 4.8 --- .../services/DashpayKeyProvisioning.kt | 37 +++++--------- .../services/IdentityKeyAdditionFlow.kt | 29 +++++------ .../services/IdentityKeyPersistence.kt | 48 +++++++++++++++++++ 3 files changed, 75 insertions(+), 39 deletions(-) create mode 100644 packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/services/IdentityKeyPersistence.kt diff --git a/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/services/DashpayKeyProvisioning.kt b/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/services/DashpayKeyProvisioning.kt index 1da7aa8386..4954b53bc6 100644 --- a/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/services/DashpayKeyProvisioning.kt +++ b/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/services/DashpayKeyProvisioning.kt @@ -26,20 +26,11 @@ import org.dashfoundation.dashsdk.identity.RegistrationKeys * * Forcing registration through it would fork its semantics; this helper reuses * the shared pieces instead — the SDK's [RegistrationKeys] role table for the - * on-chain rows and the same store-then-zero discipline for the secrets. + * on-chain rows and [IdentityKeyPersistence] for the store-then-zero secret + * discipline (shared with [IdentityKeyAdditionFlow]). */ object DashpayKeyProvisioning { - /** - * Persists one derived identity private key (public-key hex → scalar), - * owner-scoped so wallet deletion can reach it before the identity's - * `public_keys` row commits. Injected so the provisioning logic is - * unit-testable without an Android Keystore. - */ - fun interface PrivateKeyPersister { - suspend fun persist(publicKeyHex: String, privateKey: ByteArray, ownerWalletId: ByteArray) - } - /** * Persist each derived private key, scrub the JVM copy, and return the * rich [IdentityPubkey] rows the registration wire format carries. @@ -62,7 +53,7 @@ object DashpayKeyProvisioning { previews: List, includeDashPayKeys: Boolean, walletId: ByteArray, - persister: PrivateKeyPersister, + persister: IdentityKeyPersistence.PrivateKeyPersister, ): List { val expected = RegistrationKeys.keyCount(includeDashPayKeys) try { @@ -73,21 +64,17 @@ object DashpayKeyProvisioning { "expected $expected derived registration keys, got ${previews.size}" } for (preview in previews) { - try { - // The one allowed Kotlin persist step (kotlin-sdk/CLAUDE.md): - // Rust derived the scalar, we only encrypt it into the - // Keystore keyed by the public bytes the signer looks up. - persister.persist(preview.publicKeyHex, preview.privateKey, walletId) - } finally { - // Keystore is authoritative from here; the JVM copy must not - // outlive the store (IdentityKeyPreview's retention rule). - preview.privateKey.fill(0) - } + IdentityKeyPersistence.storeAndScrub( + publicKeyHex = preview.publicKeyHex, + privateKey = preview.privateKey, + walletId = walletId, + persister = persister, + ) } } catch (e: Throwable) { - // A persist threw after its own key was zeroed by the finally - // above; scrub every remaining preview's private half too so no - // plaintext scalar survives a partial failure. + // The whole set was derived up front, so on any failure scrub every + // preview's private half — including keys `storeAndScrub` never + // reached — so no plaintext scalar survives a partial failure. previews.forEach { it.privateKey.fill(0) } throw e } diff --git a/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/services/IdentityKeyAdditionFlow.kt b/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/services/IdentityKeyAdditionFlow.kt index 05c4d024c9..fda5d5c0b1 100644 --- a/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/services/IdentityKeyAdditionFlow.kt +++ b/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/services/IdentityKeyAdditionFlow.kt @@ -132,6 +132,11 @@ object IdentityKeyAdditionFlow { ): List { var freeKeyId = nextKeyId(existingKeyIds) val rows = ArrayList(specs.size) + // Route the one allowed persist step through the shared store-then-scrub + // primitive (same discipline registration provisioning uses). + val persister = IdentityKeyPersistence.PrivateKeyPersister { hex, priv, owner -> + walletStorage.storePrivateKey(hex, priv, ownerWalletId = owner) + } for (spec in specs) { validationError(spec)?.let { throw KeyAdditionException(it) } val keyId = freeKeyId @@ -152,20 +157,16 @@ object IdentityKeyAdditionFlow { // signer, and `KeystoreSigner.storageKeyFor` looks the scalar // up by exactly that hex (Swift's metadata.publicKey choice). val storageKeyHex = pubkeyBytesForChain.joinToString("") { "%02x".format(it) } - try { - // The one allowed Kotlin persist step (kotlin-sdk/CLAUDE.md): - // Rust derived; we only encrypt the scalar into the Keystore. - // ownerWalletId: the key predates its public_keys row (the - // add-key transition hasn't broadcast yet) — the durable - // owner index is what lets wallet deletion find it. - walletStorage.storePrivateKey( - storageKeyHex, - derived.privateKey, - ownerWalletId = walletId, - ) - } finally { - derived.privateKey.fill(0) - } + // Persist then scrub via the shared primitive. ownerWalletId: the + // key predates its public_keys row (the add-key transition hasn't + // broadcast yet) — the durable owner index is what lets wallet + // deletion find it. + IdentityKeyPersistence.storeAndScrub( + publicKeyHex = storageKeyHex, + privateKey = derived.privateKey, + walletId = walletId, + persister = persister, + ) rows.add( IdentityPubkey( diff --git a/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/services/IdentityKeyPersistence.kt b/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/services/IdentityKeyPersistence.kt new file mode 100644 index 0000000000..d97207e076 --- /dev/null +++ b/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/services/IdentityKeyPersistence.kt @@ -0,0 +1,48 @@ +package org.dashfoundation.example.services + +/** + * The single place a derived identity private key is persisted and then + * scrubbed. Both registration-key provisioning ([DashpayKeyProvisioning]) and + * the add-key flow ([IdentityKeyAdditionFlow]) route their one allowed Kotlin + * persist step through here, so the secret-handling discipline — store the + * scalar, then zero the JVM copy on both the success and the failure path — + * lives in exactly one tested place rather than being re-implemented per flow. + */ +object IdentityKeyPersistence { + + /** + * Persists one derived identity private key (public-key hex → scalar), + * owner-scoped so wallet deletion can reach it before the identity's + * `public_keys` row commits. Injected so callers stay unit-testable + * without an Android Keystore. + */ + fun interface PrivateKeyPersister { + suspend fun persist(publicKeyHex: String, privateKey: ByteArray, ownerWalletId: ByteArray) + } + + /** + * Persist [privateKey] under [publicKeyHex] (scoped to [walletId]), then + * scrub the JVM copy — whether the persist succeeds or throws. + * + * The Keystore is authoritative once this returns; the caller must not + * read [privateKey] afterward (it is zeroed). Callers deriving a batch of + * keys up front are still responsible for scrubbing the keys they have + * NOT yet handed to this function if an earlier one fails — this only owns + * the one scalar it was given. + */ + suspend fun storeAndScrub( + publicKeyHex: String, + privateKey: ByteArray, + walletId: ByteArray, + persister: PrivateKeyPersister, + ) { + try { + // The one allowed Kotlin persist step (kotlin-sdk/CLAUDE.md): Rust + // derived the scalar, we only encrypt it into the Keystore keyed by + // the public bytes the signer looks up. + persister.persist(publicKeyHex, privateKey, walletId) + } finally { + privateKey.fill(0) + } + } +} From a71141f28756d51b9efe078e9b9661328800976c Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Tue, 21 Jul 2026 06:57:30 +0700 Subject: [PATCH 06/10] fix(kotlin-sdk): close DashPay registration review gaps --- .../services/DashpayKeyProvisioning.kt | 5 +- .../ui/identity/CreateIdentityScreen.kt | 62 ++++++++++++++-- .../services/DashpayKeyProvisioningTest.kt | 27 +++++-- .../CreateIdentityFundingPolicyTest.kt | 41 +++++++++++ .../dashsdk/identity/IdentityKeyPreview.kt | 71 ++++++++++++------- .../dashsdk/identity/IdentityRegistration.kt | 9 ++- .../dashsdk/wallet/TeardownGate.kt | 30 ++++++++ .../identity/IdentityKeyPreviewTest.kt | 41 +++++++++++ .../dashsdk/identity/RegistrationKeysTest.kt | 7 +- .../dashsdk/wallet/GateCoverageLintTest.kt | 10 ++- .../dashsdk/wallet/TeardownGateTest.kt | 30 ++++++++ .../src/identity_registration_with_signer.rs | 32 +++++++++ .../rs-platform-wallet-ffi/src/invitation.rs | 55 ++++++++++++++ .../rs-unified-sdk-jni/src/pubkey_rows.rs | 16 ++++- 14 files changed, 391 insertions(+), 45 deletions(-) create mode 100644 packages/kotlin-sdk/KotlinExampleApp/app/src/test/java/org/dashfoundation/example/ui/identity/CreateIdentityFundingPolicyTest.kt create mode 100644 packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/identity/IdentityKeyPreviewTest.kt diff --git a/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/services/DashpayKeyProvisioning.kt b/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/services/DashpayKeyProvisioning.kt index 4954b53bc6..334f47af41 100644 --- a/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/services/DashpayKeyProvisioning.kt +++ b/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/services/DashpayKeyProvisioning.kt @@ -21,8 +21,9 @@ import org.dashfoundation.dashsdk.identity.RegistrationKeys * whereas registration derives the whole key set in a **single** * `previewRegistrationKeySet` pass — the secret-lifecycle mandate is one * derivation, not N overlapping ones; - * - its lifecycle assumes post-registration persistence (an existing - * identity id), not the pre-broadcast persistence registration needs. + * - its policy depends on the existing identity's current key IDs and builds + * an update batch, while registration needs a fixed 0..N role table before + * the identity exists. * * Forcing registration through it would fork its semantics; this helper reuses * the shared pieces instead — the SDK's [RegistrationKeys] role table for the diff --git a/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/identity/CreateIdentityScreen.kt b/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/identity/CreateIdentityScreen.kt index 4c2138eea0..564bbbcd79 100644 --- a/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/identity/CreateIdentityScreen.kt +++ b/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/identity/CreateIdentityScreen.kt @@ -32,6 +32,7 @@ import androidx.navigation.NavHostController import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.launch import org.dashfoundation.dashsdk.credits.FundingInput +import org.dashfoundation.dashsdk.identity.RegistrationKeys import org.dashfoundation.dashsdk.wallet.ManagedPlatformWallet import org.dashfoundation.dashsdk.wallet.TrackedAssetLock import org.dashfoundation.example.di.LocalAppContainer @@ -178,8 +179,10 @@ fun CreateIdentityScreen(navController: NavHostController) { text = "Create Identity", isLoading = isSubmitting, enabled = selectedWallet != null && - (fundingSource == CreateIdentityFundingSource.AssetLockResume || - amountText.toLongOrNull() != null) && + isCreateIdentityFundingAmountValid( + fundingSource, + amountText.toLongOrNull(), + ) && (fundingSource != CreateIdentityFundingSource.AssetLockResume || selectedRecoveryLock != null) && identityIndexText.toIntOrNull() != null, @@ -191,6 +194,18 @@ fun CreateIdentityScreen(navController: NavHostController) { } else { amountText.toLongOrNull() ?: return@SubmitButton } + if (!isCreateIdentityFundingAmountValid(fundingSource, amount)) { + error = if (fundingSource == CreateIdentityFundingSource.CoreBalance) { + val minimum = minimumCoreFundingDuffsForKeyCount( + RegistrationKeys.keyCount(fundingSource.includesDashPayKeys), + ) + "Core-funded registration with DashPay keys requires at least " + + "$minimum duffs." + } else { + "Funding amount must be positive." + } + return@SubmitButton + } val identityIndex = if (fundingSource == CreateIdentityFundingSource.AssetLockResume) { selectedRecoveryLock?.registrationIndex ?: return@SubmitButton } else { @@ -216,8 +231,7 @@ fun CreateIdentityScreen(navController: NavHostController) { walletHandle = wallet.handle, mnemonicResolverHandle = mgr.mnemonicResolverHandle, identityIndex = identityIndex, - count = org.dashfoundation.dashsdk.identity.RegistrationKeys - .keyCount(registerDashPayKeys), + count = RegistrationKeys.keyCount(registerDashPayKeys), ) val keys = DashpayKeyProvisioning.provision( previews = previews, @@ -400,6 +414,46 @@ enum class CreateIdentityFundingSource( get() = this != AssetLockResume } +/** Credits charged by the active fee schedule before the per-key surcharge. */ +private const val IDENTITY_CREATE_BASE_COST_CREDITS = 2_000_000L + +/** Credits charged for each public key in an identity-create transition. */ +private const val IDENTITY_KEY_CREATION_COST_CREDITS = 6_500_000L + +/** Core asset-lock floor required before identity-create processing starts. */ +private const val IDENTITY_ASSET_LOCK_BASE_DUFFS = 200_000L + +private const val CREDITS_PER_DUFF = 1_000L + +/** + * Protocol minimum for a Core-funded identity create with [keyCount] keys. + * Mirrors `IdentityCreateTransition::calculate_min_required_fee_v1` and the + * iOS `CreateIdentityView.minFundingDuffs(forKeyCount:)` parity reference. + */ +internal fun minimumCoreFundingDuffsForKeyCount(keyCount: Int): Long { + require(keyCount > 0) { "keyCount must be positive, got $keyCount" } + return IDENTITY_ASSET_LOCK_BASE_DUFFS + + (IDENTITY_CREATE_BASE_COST_CREDITS + + IDENTITY_KEY_CREATION_COST_CREDITS * keyCount) / CREDITS_PER_DUFF +} + +/** Single submit/UI gate for the active funding source's amount semantics. */ +internal fun isCreateIdentityFundingAmountValid( + source: CreateIdentityFundingSource, + amount: Long?, +): Boolean = when (source) { + CreateIdentityFundingSource.CoreBalance -> { + val minimum = minimumCoreFundingDuffsForKeyCount( + RegistrationKeys.keyCount(source.includesDashPayKeys), + ) + amount != null && amount >= minimum + } + CreateIdentityFundingSource.PlatformAddress, + CreateIdentityFundingSource.ShieldedBalance, + -> amount != null && amount > 0 + CreateIdentityFundingSource.AssetLockResume -> true +} + /** * The fixed exit denominations (in CREDITS) a Type-20 shielded identity * create may spend from the pool — 0.1 / 0.3 / 0.5 / 1.0 DASH. Source of diff --git a/packages/kotlin-sdk/KotlinExampleApp/app/src/test/java/org/dashfoundation/example/services/DashpayKeyProvisioningTest.kt b/packages/kotlin-sdk/KotlinExampleApp/app/src/test/java/org/dashfoundation/example/services/DashpayKeyProvisioningTest.kt index 98f5344446..99842f5f8a 100644 --- a/packages/kotlin-sdk/KotlinExampleApp/app/src/test/java/org/dashfoundation/example/services/DashpayKeyProvisioningTest.kt +++ b/packages/kotlin-sdk/KotlinExampleApp/app/src/test/java/org/dashfoundation/example/services/DashpayKeyProvisioningTest.kt @@ -4,6 +4,7 @@ import kotlinx.coroutines.test.runTest import org.dashfoundation.dashsdk.identity.ContractBounds import org.dashfoundation.dashsdk.identity.IdentityKeyPreview import org.dashfoundation.dashsdk.identity.KeyPurpose +import org.dashfoundation.dashsdk.identity.KeyType import org.dashfoundation.dashsdk.identity.RegistrationKeys import org.dashfoundation.dashsdk.identity.SecurityLevel import org.dashfoundation.example.ui.identity.CreateIdentityFundingSource @@ -32,12 +33,16 @@ class DashpayKeyProvisioningTest { @Test fun `fresh funding provisions all six keys with DashPay bounds`() = runTest { val previews = (0 until 6).map(::preview) - val stored = mutableListOf() + val walletId = ByteArray(32) { 9 } + data class StoredKey(val hex: String, val scalar: ByteArray, val owner: ByteArray) + val stored = mutableListOf() val rows = DashpayKeyProvisioning.provision( previews = previews, includeDashPayKeys = true, - walletId = ByteArray(32) { 9 }, - persister = { hex, _, _ -> stored += hex }, + walletId = walletId, + persister = { hex, privateKey, owner -> + stored += StoredKey(hex, privateKey.copyOf(), owner.copyOf()) + }, ) assertEquals(6, rows.size) @@ -45,14 +50,26 @@ class DashpayKeyProvisioningTest { assertEquals(KeyPurpose.ENCRYPTION, rows[4].purpose) assertEquals(KeyPurpose.DECRYPTION, rows[5].purpose) assertEquals(SecurityLevel.MEDIUM, rows[4].securityLevel) + assertEquals(SecurityLevel.MEDIUM, rows[5].securityLevel) + assertEquals(KeyType.ECDSA_SECP256K1, rows[4].keyType) + assertEquals(KeyType.ECDSA_SECP256K1, rows[5].keyType) + assertFalse(rows[4].readOnly) + assertFalse(rows[5].readOnly) val bounds = rows[4].contractBounds assertTrue(bounds is ContractBounds.SingleContractDocumentType) bounds as ContractBounds.SingleContractDocumentType assertArrayEquals(RegistrationKeys.DASHPAY_CONTRACT_ID, bounds.contractId) + assertEquals(RegistrationKeys.DASHPAY_CONTACT_REQUEST_DOCUMENT_TYPE, bounds.documentTypeName) + assertEquals(bounds, rows[5].contractBounds) - // Every private scalar was persisted then scrubbed. + // Every slot is persisted under its matching public bytes while the + // scalar is still non-zero, owner-scoped to this wallet, then scrubbed. assertEquals(6, stored.size) - for (p in previews) { + for ((keyId, p) in previews.withIndex()) { + assertEquals(p.publicKeyHex, stored[keyId].hex) + assertArrayEquals(ByteArray(32) { (keyId + 1).toByte() }, stored[keyId].scalar) + assertArrayEquals(walletId, stored[keyId].owner) + assertArrayEquals(p.publicKey, rows[keyId].pubkeyBytes) assertArrayEquals(ByteArray(32), p.privateKey) } } diff --git a/packages/kotlin-sdk/KotlinExampleApp/app/src/test/java/org/dashfoundation/example/ui/identity/CreateIdentityFundingPolicyTest.kt b/packages/kotlin-sdk/KotlinExampleApp/app/src/test/java/org/dashfoundation/example/ui/identity/CreateIdentityFundingPolicyTest.kt new file mode 100644 index 0000000000..a46360f27d --- /dev/null +++ b/packages/kotlin-sdk/KotlinExampleApp/app/src/test/java/org/dashfoundation/example/ui/identity/CreateIdentityFundingPolicyTest.kt @@ -0,0 +1,41 @@ +package org.dashfoundation.example.ui.identity + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +class CreateIdentityFundingPolicyTest { + + @Test + fun `Core minimum scales with the number of registration keys`() { + assertEquals(228_000L, minimumCoreFundingDuffsForKeyCount(4)) + assertEquals(241_000L, minimumCoreFundingDuffsForKeyCount(6)) + } + + @Test + fun `fresh six-key Core funding rejects amounts below the protocol floor`() { + assertFalse( + isCreateIdentityFundingAmountValid( + CreateIdentityFundingSource.CoreBalance, + 240_999L, + ), + ) + assertTrue( + isCreateIdentityFundingAmountValid( + CreateIdentityFundingSource.CoreBalance, + 241_000L, + ), + ) + } + + @Test + fun `resume has no new funding amount to validate`() { + assertTrue( + isCreateIdentityFundingAmountValid( + CreateIdentityFundingSource.AssetLockResume, + null, + ), + ) + } +} diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityKeyPreview.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityKeyPreview.kt index f712bf16cb..9771e94452 100644 --- a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityKeyPreview.kt +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityKeyPreview.kt @@ -48,32 +48,53 @@ data class IdentityKeyPreview( */ fun decodeAll(blob: ByteArray): List { val buf = ByteBuffer.wrap(blob) // big-endian by default - val count = buf.int - val rows = ArrayList(count) - repeat(count) { - val identityIndex = buf.int - val pathLen = buf.short.toInt() and 0xFFFF - val pathBytes = ByteArray(pathLen) - buf.get(pathBytes) - val pubkey = ByteArray(33) - buf.get(pubkey) - val privkey = ByteArray(32) - buf.get(privkey) - rows.add( - IdentityKeyPreview( - identityIndex = identityIndex, - derivationPath = String(pathBytes, Charsets.UTF_8), - publicKey = pubkey, - privateKey = privkey, - ), - ) + val rows = ArrayList() + try { + val count = buf.int + require(count >= 0) { "preview row count must be non-negative, got $count" } + repeat(count) { + val identityIndex = buf.int + val pathLen = buf.short.toInt() and 0xFFFF + val pathBytes = ByteArray(pathLen) + buf.get(pathBytes) + val pubkey = ByteArray(33) + buf.get(pubkey) + val privkey = ByteArray(32) + var rowOwnsPrivateKey = false + try { + buf.get(privkey) + rows.add( + IdentityKeyPreview( + identityIndex = identityIndex, + derivationPath = String(pathBytes, Charsets.UTF_8), + publicKey = pubkey, + privateKey = privkey, + ), + ) + rowOwnsPrivateKey = true + } finally { + // If construction/list insertion fails after the scalar + // copy, it never reaches the outer rows cleanup. + if (!rowOwnsPrivateKey) privkey.fill(0) + } + } + require(!buf.hasRemaining()) { + "preview blob has ${buf.remaining()} trailing byte(s)" + } + return rows + } catch (e: Throwable) { + // Some rows may already have been copied out of the interleaved + // blob when a later row fails. They will never reach the caller, + // so scrub those discarded scalar arrays here. + rows.forEach { it.privateKey.fill(0) } + throw e + } finally { + // The source blob interleaves every row's raw private scalar. + // Wipe it on success and on every malformed/truncated failure; + // otherwise an exception before the old tail-only fill left all + // native-returned scalars resident in a JVM byte array. + blob.fill(0) } - // The source blob interleaves every row's raw private scalar — - // consume-and-wipe it so the only remaining plaintext copies - // are the per-row arrays the caller owns (and must zero after - // use, per this class's contract). - blob.fill(0) - return rows } } } diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityRegistration.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityRegistration.kt index b3f69d203c..b9e84ed484 100644 --- a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityRegistration.kt +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityRegistration.kt @@ -1,6 +1,7 @@ package org.dashfoundation.dashsdk.identity import org.dashfoundation.dashsdk.wallet.op +import org.dashfoundation.dashsdk.wallet.opWithCleanupOnCancellation import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext @@ -184,7 +185,9 @@ class IdentityRegistration internal constructor( mnemonicResolverHandle: Long, startIndex: Int, count: Int = -1, - ): List = gate.op { + ): List = gate.opWithCleanupOnCancellation( + cleanup = { previews -> previews.forEach { it.privateKey.fill(0) } }, + ) { val blob = mapNativeErrors { IdentityNative.previewRegistrationKeys( walletHandle, @@ -224,7 +227,9 @@ class IdentityRegistration internal constructor( mnemonicResolverHandle: Long, identityIndex: Int, count: Int = -1, - ): List = gate.op { + ): List = gate.opWithCleanupOnCancellation( + cleanup = { previews -> previews.forEach { it.privateKey.fill(0) } }, + ) { val blob = mapNativeErrors { IdentityNative.previewRegistrationKeySet( walletHandle, diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/TeardownGate.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/TeardownGate.kt index d3f0018004..bdcce837a9 100644 --- a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/TeardownGate.kt +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/TeardownGate.kt @@ -91,3 +91,33 @@ class TeardownGate { */ internal suspend fun TeardownGate?.op(block: suspend () -> T): T = if (this != null) withOp(block) else withContext(Dispatchers.IO) { block() } + +/** + * Run [block] like [op], but invoke [cleanup] if a completed result is discarded + * by coroutine cancellation while [withContext] dispatches back to its caller. + * + * This matters for blocking JNI calls that return private material: they cannot + * observe cancellation while native code is running, and `withContext` provides + * prompt cancellation when it resumes the original dispatcher. Without this + * handoff guard, the completed secret-bearing result can be dropped before the + * caller ever receives a reference it can scrub. + */ +internal suspend fun TeardownGate?.opWithCleanupOnCancellation( + cleanup: (T) -> Unit, + block: suspend () -> T, +): T { + class CompletedResult(val value: T) + + var completed: CompletedResult? = null + try { + val value = op { + block().also { completed = CompletedResult(it) } + } + // No suspension point exists between the successful handoff and this + // clear, so ownership has moved to the caller from here onward. + completed = null + return value + } finally { + completed?.let { cleanup(it.value) } + } +} diff --git a/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/identity/IdentityKeyPreviewTest.kt b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/identity/IdentityKeyPreviewTest.kt new file mode 100644 index 0000000000..eac3253dc9 --- /dev/null +++ b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/identity/IdentityKeyPreviewTest.kt @@ -0,0 +1,41 @@ +package org.dashfoundation.dashsdk.identity + +import org.junit.Assert.assertArrayEquals +import org.junit.Assert.assertTrue +import org.junit.Test +import java.io.ByteArrayOutputStream +import java.io.DataOutputStream + +class IdentityKeyPreviewTest { + + @Test + fun `malformed preview blob is scrubbed after a partial decode failure`() { + val complete = previewBlob(rowCount = 2) + val blob = complete.copyOf(complete.size - 1) + + val failure = runCatching { IdentityKeyPreview.decodeAll(blob) } + + assertTrue(failure.isFailure) + assertArrayEquals( + "the JNI preview blob contains private scalars and must be wiped even when parsing fails", + ByteArray(blob.size), + blob, + ) + } + + private fun previewBlob(rowCount: Int): ByteArray { + val out = ByteArrayOutputStream() + DataOutputStream(out).use { dos -> + dos.writeInt(rowCount) + repeat(rowCount) { keyId -> + dos.writeInt(7) + val path = "m/9'/7'/$keyId'".toByteArray() + dos.writeShort(path.size) + dos.write(path) + dos.write(ByteArray(33) { (keyId + 2).toByte() }) + dos.write(ByteArray(32) { (keyId + 1).toByte() }) + } + } + return out.toByteArray() + } +} diff --git a/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/identity/RegistrationKeysTest.kt b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/identity/RegistrationKeysTest.kt index d49eb13c81..8d3ec44436 100644 --- a/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/identity/RegistrationKeysTest.kt +++ b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/identity/RegistrationKeysTest.kt @@ -30,7 +30,10 @@ class RegistrationKeysTest { ), rows.map { Triple(it.purpose, it.securityLevel, it.contractBounds) }, ) - rows.forEach { assertEquals(KeyType.ECDSA_SECP256K1, it.keyType) } + rows.forEach { + assertEquals(KeyType.ECDSA_SECP256K1, it.keyType) + assertEquals(false, it.readOnly) + } } @Test @@ -43,7 +46,9 @@ class RegistrationKeysTest { assertEquals(KeyPurpose.ENCRYPTION, enc.purpose) assertEquals(KeyPurpose.DECRYPTION, dec.purpose) for (row in listOf(enc, dec)) { + assertEquals(KeyType.ECDSA_SECP256K1, row.keyType) assertEquals(SecurityLevel.MEDIUM, row.securityLevel) + assertEquals(false, row.readOnly) val bounds = row.contractBounds assertTrue(bounds is ContractBounds.SingleContractDocumentType) bounds as ContractBounds.SingleContractDocumentType diff --git a/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/wallet/GateCoverageLintTest.kt b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/wallet/GateCoverageLintTest.kt index 45f5a17ec9..4d293cd9fd 100644 --- a/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/wallet/GateCoverageLintTest.kt +++ b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/wallet/GateCoverageLintTest.kt @@ -9,8 +9,9 @@ import org.junit.Test * `suspend fun` in the SDK that borrows a raw native handle by parameter * name (`signerHandle` / `coreSignerHandle` / `resolverHandle` / * `mnemonicResolverHandle` / `sdkHandle`) must open with a gate bracket - * (`gate.op {` / `teardownGate.op {` / `queryGate.op {` / - * `sdk.queryGate.op {`) or visibly delegate to another (gated) method. + * (`gate.op {` / `gate.opWithCleanupOnCancellation(...) {` / + * `teardownGate.op {` / `queryGate.op {` / `sdk.queryGate.op {`) or visibly + * delegate to another (gated) method. * * This exact defect class — a new binding borrowing a handle under plain * `withContext(Dispatchers.IO)` — shipped three separate times during @@ -91,7 +92,10 @@ class GateCoverageLintTest { val HANDLE_PARAM = Regex( """\b(signerHandle|coreSignerHandle|resolverHandle|mnemonicResolverHandle|sdkHandle)\s*:\s*Long\b""", ) - val GATED_OPENER = Regex("""=\s*(\w+\.)?(gate|teardownGate|queryGate)\.op \{""") + val GATED_OPENER = Regex( + """=\s*(\w+\.)?(gate|teardownGate|queryGate)\.""" + + """(?:op \{|opWithCleanupOnCancellation\()""", + ) val DELEGATION_OPENER = Regex("""=\s*\w+\(""") /** `funName@FileName.kt` entries that are intentionally ungated. */ diff --git a/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/wallet/TeardownGateTest.kt b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/wallet/TeardownGateTest.kt index 0667acce4c..7e2e35817d 100644 --- a/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/wallet/TeardownGateTest.kt +++ b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/wallet/TeardownGateTest.kt @@ -8,10 +8,13 @@ import kotlinx.coroutines.cancelAndJoin import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking import kotlinx.coroutines.withTimeout +import org.junit.Assert.assertArrayEquals import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue import org.junit.Test +import java.util.concurrent.CountDownLatch +import java.util.concurrent.TimeUnit /** * Pins the three properties the manager teardown depends on (each was, or @@ -97,4 +100,31 @@ class TeardownGateTest { // NonCancellable) — otherwise this hangs and times out. withTimeout(5_000) { gate.closeAndAwait() } } + + @Test + fun cancelledOpScrubsACompletedSecretResultBeforeDiscardingIt() = runBlocking { + val gate = TeardownGate() + val entered = CountDownLatch(1) + val release = CountDownLatch(1) + val secret = ByteArray(32) { 0x5A } + + val op = launch(Dispatchers.Default) { + gate.opWithCleanupOnCancellation( + cleanup = { discarded: ByteArray -> discarded.fill(0) }, + ) { + entered.countDown() + // A blocking native call does not observe coroutine cancellation. + // Return the secret only after the caller has cancelled, forcing + // withContext's prompt-cancellation result-discard path. + release.await() + secret + } + } + assertTrue("blocking operation never started", entered.await(5, TimeUnit.SECONDS)) + op.cancel() + release.countDown() + withTimeout(5_000) { op.join() } + + assertArrayEquals(ByteArray(32), secret) + } } diff --git a/packages/rs-platform-wallet-ffi/src/identity_registration_with_signer.rs b/packages/rs-platform-wallet-ffi/src/identity_registration_with_signer.rs index fb774b1c5e..83102a1306 100644 --- a/packages/rs-platform-wallet-ffi/src/identity_registration_with_signer.rs +++ b/packages/rs-platform-wallet-ffi/src/identity_registration_with_signer.rs @@ -897,4 +897,36 @@ mod tests { assert_eq!(err.code, PlatformWalletFFIResultCode::ErrorInvalidParameter); unsafe { platform_wallet_ffi_result_free(&mut err) }; } + + #[test] + fn decode_identity_pubkeys_rejects_invalid_role_discriminants() { + let pk = [0x02u8; 33]; + + for (field, row) in [ + ("key_type", { + let mut row = ffi_row(0, &pk); + row.key_type = u8::MAX; + row + }), + ("purpose", { + let mut row = ffi_row(0, &pk); + row.purpose = u8::MAX; + row + }), + ("security_level", { + let mut row = ffi_row(0, &pk); + row.security_level = u8::MAX; + row + }), + ] { + let mut err = unsafe { decode_identity_pubkeys(&row, 1) } + .expect_err("invalid role byte must be rejected"); + assert_eq!(err.code, PlatformWalletFFIResultCode::ErrorInvalidParameter); + let message = unsafe { CStr::from_ptr(err.message) } + .to_str() + .expect("error message is UTF-8"); + assert!(message.contains(field), "{message}"); + unsafe { platform_wallet_ffi_result_free(&mut err) }; + } + } } diff --git a/packages/rs-platform-wallet-ffi/src/invitation.rs b/packages/rs-platform-wallet-ffi/src/invitation.rs index d6632cc500..ed01ce6901 100644 --- a/packages/rs-platform-wallet-ffi/src/invitation.rs +++ b/packages/rs-platform-wallet-ffi/src/invitation.rs @@ -447,6 +447,8 @@ pub unsafe extern "C" fn platform_wallet_parse_invitation( #[cfg(test)] mod tests { use super::*; + use dashcore::secp256k1::SecretKey; + use dashcore::{Network, PrivateKey}; // Marshalling-boundary coverage. The invitation crypto/codec semantics are // pinned library-side in `platform_wallet`'s `crypto::invitation` + @@ -610,6 +612,59 @@ mod tests { assert_ne!(r.code, PlatformWalletFFIResultCode::Success); } + /// The invitation-claim export is the fifth production caller of + /// `decode_identity_pubkeys`. Exercise that call path directly: a valid + /// invitation URI gets past parsing, duplicate rows are rejected before the + /// unknown wallet is looked up, and no signer/network work is attempted. + #[test] + fn claim_invitation_rejects_duplicate_key_ids_before_wallet_lookup() { + let voucher = SecretKey::from_slice(&[0x11u8; 32]).expect("valid scalar"); + let wif = PrivateKey::new(voucher, Network::Testnet).to_wif(); + let uri = + std::ffi::CString::new(format!("dashpay://invite?assetlocktx=aa&pk={wif}")).unwrap(); + + let pk_a = [0x02u8; 33]; + let pk_b = [0x03u8; 33]; + let ffi_row = |pubkey: &[u8]| IdentityPubkeyFFI { + key_id: 0, + key_type: 0, + purpose: 0, + security_level: 0, + pubkey_bytes: pubkey.as_ptr(), + pubkey_len: pubkey.len(), + read_only: false, + contract_bounds_kind: 0, + contract_bounds_id: std::ptr::null(), + contract_bounds_document_type: std::ptr::null(), + }; + let rows = [ffi_row(&pk_a), ffi_row(&pk_b)]; + let dummy_signer = std::ptr::dangling_mut::(); + let mut id = [0xFFu8; 32]; + let mut handle: Handle = 99; + + let mut result = unsafe { + platform_wallet_claim_invitation( + 0xDEAD_BEEF, + uri.as_ptr(), + 0, + rows.as_ptr(), + rows.len(), + dummy_signer, + 0, + &mut id, + &mut handle, + ) + }; + + assert_eq!( + result.code, + PlatformWalletFFIResultCode::ErrorInvalidParameter + ); + assert_eq!(id, [0u8; 32]); + assert_eq!(handle, NULL_HANDLE); + unsafe { platform_wallet_ffi_result_free(&mut result) }; + } + /// A null `uri` is rejected with `ErrorNullPointer` before any parsing. #[test] fn parse_invitation_null_uri_is_null_pointer() { diff --git a/packages/rs-unified-sdk-jni/src/pubkey_rows.rs b/packages/rs-unified-sdk-jni/src/pubkey_rows.rs index bf41f69608..95c3729e6e 100644 --- a/packages/rs-unified-sdk-jni/src/pubkey_rows.rs +++ b/packages/rs-unified-sdk-jni/src/pubkey_rows.rs @@ -27,9 +27,10 @@ //! //! ## Strictness (wire-skew detection) //! -//! The parser rejects truncated input, **trailing bytes**, invalid DPP-role / -//! bounds-kind / boolean discriminants, interior-NUL document types, and -//! negative key IDs. The opaque payload shape is not self-describing, so an +//! The parser rejects truncated input, **trailing bytes**, invalid bounds-kind / +//! boolean discriminants, interior-NUL document types, and negative key IDs. +//! DPP-role discriminants are validated by the downstream FFI conversion. The +//! opaque payload shape is not self-describing, so an //! old Kotlin artifact paired with a newer native library (or the reverse) //! would otherwise be silently misparsed rather than rejected. Failing loud on //! any structural surprise turns that skew into a clean error instead of a key @@ -634,6 +635,15 @@ mod tests { assert!(err.contains("MASTER + AUTHENTICATION"), "{err}"); } + #[test] + fn registration_rejects_non_authentication_key_zero() { + let mut policy = six_key_policy(); + policy[0].purpose = PURPOSE_TRANSFER; + let rows = parse_pubkey_rows(&encode(&policy)).unwrap(); + let err = check_registration_invariants(&rows).unwrap_err(); + assert!(err.contains("MASTER + AUTHENTICATION"), "{err}"); + } + #[test] fn registration_rejects_duplicate_key_ids() { let mut policy = six_key_policy(); From a23bf570c4fa416f9aff49d6568d35412c1867f2 Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Tue, 21 Jul 2026 06:57:58 +0700 Subject: [PATCH 07/10] docs(kotlin-sdk): record final multi-agent review --- docs/sdk/CODE_REVIEW_NOTES.md | 176 ++++++++++++++++++++++++++++++++++ 1 file changed, 176 insertions(+) create mode 100644 docs/sdk/CODE_REVIEW_NOTES.md diff --git a/docs/sdk/CODE_REVIEW_NOTES.md b/docs/sdk/CODE_REVIEW_NOTES.md new file mode 100644 index 0000000000..d5f2cb2142 --- /dev/null +++ b/docs/sdk/CODE_REVIEW_NOTES.md @@ -0,0 +1,176 @@ +# Final multi-agent code review: Kotlin DashPay registration keys + +**Review date:** 2026-07-21 +**Branch:** `feat/kotlin-sdk-dashpay-registration-keys` +**Implementation base:** `6efa83bb53` +**Reviewed implementation commits:** `70b0852edb`, `3a64940af8`, +`c31ca592d4`, `4a25717471`, plus the concurrently-added persistence refactor +`5147d5baa9` + +## Review method + +Three independent agents reviewed the actual diff and call graph under separate +lenses, followed by a primary-agent source audit and full build/test run: + +1. Rust wire format, registration invariants, FFI callers, and consensus-facing + key policy. +2. Kotlin derivation/persistence/zeroization lifecycle, funding policy, resume + exclusion, and the helper-reuse decision. +3. Cross-language fixture and edge-case test adequacy, including the invitation + caller and environment-bound on-chain assertion. + +The agents were instructed to review only and made no edits or commits. While +the review was running, `5147d5baa9` was committed by an external concurrent +session; it was reviewed as part of the resulting branch head and was not +rewritten. + +## Verified correct in the implementation + +- `parse_pubkey_rows` is a pure `&[u8]` parser. JNI conversion is confined to + thin update/registration adapters. +- Empty-list, duplicate-key-ID, and key-ID-0 = MASTER + AUTHENTICATION checks + are registration-only. The shared update parser still accepts ordinary + add-key lists without key ID 0. +- All four Android registration JNI exports use the rich registration decoder: + resume, Core-funded, Platform-address-funded, and shielded-from-pool. +- The fifth `decode_identity_pubkeys` caller, invitation claim, still uses the + shared FFI decoder and therefore receives duplicate-ID rejection without + receiving the JNI-only key-ID-0 policy. +- `role_for_registration_key_id` and `decode_pubkeys_blob` are deleted, not + merely superseded. +- Kotlin explicitly rebuilds the base roles as MASTER/AUTHENTICATION, + CRITICAL/AUTHENTICATION, HIGH/AUTHENTICATION, and CRITICAL/TRANSFER. +- Keys 4/5 are ECDSA secp256k1, ENCRYPTION/DECRYPTION, MEDIUM, writable, and + bounded to the canonical DashPay contract's `contactRequest` document type. +- Fresh registration performs one six-key derivation pass. It does not derive + an overlapping base-four set first. +- Resume derives and submits only the base four keys. The DashPay pair is not + added to an already-funded asset-lock resume. +- The checked-in golden binary is genuinely shared: Kotlin compares its encoder + output to that exact resource, while Rust includes and decodes the same file + and pins its contract ID to `dashpay_contract::ID_BYTES`. +- Strict codec coverage includes truncation, legacy layout, trailing bytes, + invalid bounds/boolean bytes, negative IDs, interior NUL, duplicate IDs, and + update-without-key-0 behavior. FFI tests cover invalid DPP role bytes. + +## Findings fixed during this review + +### 1. Private material survived two failure windows + +`IdentityKeyPreview.decodeAll` wiped the JNI blob only after a complete parse. +A malformed/truncated blob could leave both the source blob and already-copied +private scalars unsanitized. Separately, a blocking JNI preview could finish +after its coroutine was cancelled; `withContext` would then discard the +secret-bearing result before the caller reached provisioning cleanup. + +Fix: + +- Decode under `try/catch/finally`, wiping the source blob on every exit and + wiping all partially decoded private arrays on failure. +- Add `opWithCleanupOnCancellation`, which scrubs a completed result if prompt + cancellation discards it during dispatcher handoff. +- Route both registration preview APIs through that cleanup-aware gate. +- Add malformed-blob and blocking-JNI cancellation regressions. Both tests were + observed failing before the production fix and passing afterward. + +### 2. Six-key Core funding minimum was not enforced + +The app accepted any numeric Core amount even though six creation keys raise +the protocol floor from 228,000 to 241,000 duffs. A below-floor asset lock can +be broadcast before Platform rejects the identity create. + +Fix: + +- Mirror `IdentityCreateTransition::calculate_min_required_fee_v1` for the + current key count. +- Use one amount-policy function for both submit enablement and the click-time + preflight. +- Add boundary tests for the four-key/six-key floors and the resume no-new-funds + case. The new test was observed failing before the implementation and passing + afterward. + +### 3. Invitation claim lacked its required caller-level regression + +The existing duplicate-ID unit test called `decode_identity_pubkeys` directly +and only stated in a comment that invitation claim used it. It did not exercise +the fifth caller itself. + +Fix: + +- Add a valid-invitation/duplicate-row test through + `platform_wallet_claim_invitation`. +- Assert duplicate rejection happens before wallet lookup, signer use, or + network work, and that output sentinels remain zeroed. + +### 4. Coverage and documentation were weaker than the implementation claims + +Fix: + +- Correct the Rust module documentation: the pure structural parser does not + validate DPP role discriminants; the downstream FFI conversion does. +- Add direct invalid key-type/purpose/security-level FFI coverage and a + MASTER-with-wrong-purpose registration invariant test. +- Strengthen Kotlin tests to prove the exact public-key storage key, non-zero + scalar at persistence time, wallet ownership, post-persist scrubbing, + DashPay key type/purpose/security/read-only flags, and exact bounds. +- Correct the helper rationale: the add-key flow persists before broadcast; + the real incompatibilities are its existing-key/max+1 update policy and + per-slot derivation, versus registration's fixed 0..N batch policy. + +## Left for human judgment or environment-bound verification + +1. **On-chain bounds assertion is still manual.** No unit test can prove what a + testnet node persisted. Register a fresh identity, fetch it, and assert keys + 4/5 contain the exact DashPay contract ID plus `contactRequest` bounds. Then + run Add Contact as a separate smoke test; Add Contact success alone is not + proof of the bounds. +2. **Public Kotlin/logical wire compatibility.** Public registration methods now + consume rich `IdentityPubkey` lists, and the opaque `byte[]` layout changed + without a version/magic prefix. The parser fails closed on legacy/malformed + shapes, but an old Kotlin artifact must not be paired with the new native + library. Release coordination or an explicit compatibility layer remains a + product/API decision. +3. **No full Compose four-way dispatch seam test.** Static tracing confirms all + three fresh paths pass the same six-row list and resume passes the four-row + list; policy tests cover the funding-source gate. Extracting a larger pure + screen-dispatch seam solely for orchestration testing is a maintainability + choice, not a discovered production defect. +4. **Kotlin still trusts Rust's preview row keypair correspondence.** Swift + independently revalidates private/public correspondence. Kotlin documents + why it does not duplicate private-key math; whether to add an equivalent + native validation surface remains a parity-hardening decision. +5. **No existing-identity backfill.** Already-created Android identities still + need the Add Identity Key flow. This remains the reviewed non-goal. +6. **Dedicated provisioning helper.** Keeping `DashpayKeyProvisioning` is + justified by the fixed registration IDs and one batch derivation. Commit + `5147d5baa9` shares the dangerous persist-and-scrub primitive with + `IdentityKeyAdditionFlow`, avoiding lifecycle duplication while preserving + the distinct policies. + +## Verification results + +All required local verification passed: + +```text +cargo test -p rs-unified-sdk-jni --lib + 29 passed + +cargo test -p platform-wallet-ffi --lib + 201 passed + +env RUSTC_WRAPPER= cargo clippy --workspace --all-features + passed (the configured sccache was not permitted in the sandbox) + +cargo fmt --check --all + passed + +JAVA_HOME=/opt/homebrew/opt/openjdk@17 \ +ANDROID_HOME=/opt/homebrew/share/android-commandlinetools \ +./gradlew :sdk:assembleDebug :sdk:testDebugUnitTest \ + :app:assembleDebug :app:testDebugUnitTest \ + :sdk:compileDebugAndroidTestKotlin + passed +``` + +Testnet/device registration, the on-chain bounds assertion, and Add Contact +were intentionally not attempted because they are environment-bound. From e1cbe65bec688eeedb9d475cfb81eb5bee63cf62 Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Tue, 21 Jul 2026 19:38:51 +0700 Subject: [PATCH 08/10] fix(kotlin-sdk): address registration review blockers --- docs/sdk/CODE_REVIEW_NOTES.md | 56 +++- .../services/DashpayKeyProvisioning.kt | 20 +- .../ui/identity/CreateIdentityScreen.kt | 252 ++++++++++++++---- .../services/DashpayKeyProvisioningTest.kt | 33 ++- .../CreateIdentityFundingPolicyTest.kt | 116 ++++++++ .../dashsdk/identity/IdentityRegistration.kt | 14 +- .../dashsdk/identity/RegistrationKeys.kt | 19 ++ .../identity/IdentityAssetLockRecoveryTest.kt | 44 ++- 8 files changed, 466 insertions(+), 88 deletions(-) diff --git a/docs/sdk/CODE_REVIEW_NOTES.md b/docs/sdk/CODE_REVIEW_NOTES.md index d5f2cb2142..af6f1b20b0 100644 --- a/docs/sdk/CODE_REVIEW_NOTES.md +++ b/docs/sdk/CODE_REVIEW_NOTES.md @@ -117,6 +117,42 @@ Fix: the real incompatibilities are its existing-key/max+1 update policy and per-slot derivation, versus registration's fixed 0..N batch policy. +## PR #4173 automated-review follow-up + +The first automated PR review found three additional blocking regressions. All +three were independently traced to the current source and fixed: + +1. **Mutable controls could redirect an in-flight registration.** The click + handler captured the key-count choice before suspension but later reread + `fundingSource` and `selectedRecoveryLock`. It now resolves source, amount, + identity index, and a defensive copy of the selected lock into one immutable + submission snapshot before the coroutine starts. Preparation, dispatch, + coordinator tracking, and navigation use only that snapshot. A regression + mutates the backing form values and lock txid after capture and verifies the + submission is unchanged. +2. **Platform-address packing did not reserve the six-key creation fee.** The + default native strategy deducts the fee from the post-spend remainder of + BTreeMap input 0. The packer now mirrors the consensus formula + `2,000,000 + 6,500,000 * keyCount + 500,000 * inputCount`, models Rust's + `(address type, unsigned hash)` ordering, and selects the smallest input set + that contributes the requested identity balance while leaving the full fee + on input 0. This preflight runs before key derivation/persistence. The + reviewer's 70M-balance/30M-spend case was observed failing before the fix; + exact one-input and two-input boundaries are also covered. +3. **Resume lost its HD-slot provenance check.** Rich consensus rows do not + carry a derivation index, so replacing `IdentityKeyPreview` had deleted the + old per-key guard. Provisioning now returns a `RegistrationKeySet` that keeps + the common preview identity index beside the rich rows, rejects mixed-slot + previews before persistence, and resume verifies the set index matches the + tracked lock before JNI. The wrong-slot regression was observed calling JNI + before the fix and passing afterward. + +The full Kotlin SDK/app build, unit-test suites, and instrumented-test-source +compilation passed after these fixes. The PR's initial emulator check hit the +workflow's documented Android Keystore unlock-state race; its failed-job rerun +passed the complete native build and API-35 instrumented suite without a code or +workflow change. + ## Left for human judgment or environment-bound verification 1. **On-chain bounds assertion is still manual.** No unit test can prove what a @@ -124,17 +160,19 @@ Fix: 4/5 contain the exact DashPay contract ID plus `contactRequest` bounds. Then run Add Contact as a separate smoke test; Add Contact success alone is not proof of the bounds. -2. **Public Kotlin/logical wire compatibility.** Public registration methods now - consume rich `IdentityPubkey` lists, and the opaque `byte[]` layout changed - without a version/magic prefix. The parser fails closed on legacy/malformed - shapes, but an old Kotlin artifact must not be paired with the new native - library. Release coordination or an explicit compatibility layer remains a - product/API decision. +2. **Public Kotlin/logical wire compatibility.** Public fresh-registration + methods now consume rich `IdentityPubkey` lists, resume consumes a + provenance-carrying `RegistrationKeySet`, and the opaque `byte[]` layout + changed without a version/magic prefix. The parser fails closed on + legacy/malformed shapes, but an old Kotlin artifact must not be paired with + the new native library. Release coordination or an explicit compatibility + layer remains a product/API decision. 3. **No full Compose four-way dispatch seam test.** Static tracing confirms all three fresh paths pass the same six-row list and resume passes the four-row - list; policy tests cover the funding-source gate. Extracting a larger pure - screen-dispatch seam solely for orchestration testing is a maintainability - choice, not a discovered production defect. + list; policy tests cover the funding-source gate and immutable submission + snapshot. Extracting a larger pure screen-dispatch seam solely for + orchestration testing is a maintainability choice, not a discovered + production defect. 4. **Kotlin still trusts Rust's preview row keypair correspondence.** Swift independently revalidates private/public correspondence. Kotlin documents why it does not duplicate private-key math; whether to add an equivalent diff --git a/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/services/DashpayKeyProvisioning.kt b/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/services/DashpayKeyProvisioning.kt index 334f47af41..198b847e3d 100644 --- a/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/services/DashpayKeyProvisioning.kt +++ b/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/services/DashpayKeyProvisioning.kt @@ -1,7 +1,7 @@ package org.dashfoundation.example.services import org.dashfoundation.dashsdk.identity.IdentityKeyPreview -import org.dashfoundation.dashsdk.identity.IdentityPubkey +import org.dashfoundation.dashsdk.identity.RegistrationKeySet import org.dashfoundation.dashsdk.identity.RegistrationKeys /** @@ -34,7 +34,8 @@ object DashpayKeyProvisioning { /** * Persist each derived private key, scrub the JVM copy, and return the - * rich [IdentityPubkey] rows the registration wire format carries. + * rich rows the registration wire format carries, tied to the identity HD + * slot that produced them. * * [previews] must be exactly `RegistrationKeys.keyCount(includeDashPayKeys)` * rows in key-ID order — the single [IdentityKeyPreview] derivation pass @@ -55,7 +56,7 @@ object DashpayKeyProvisioning { includeDashPayKeys: Boolean, walletId: ByteArray, persister: IdentityKeyPersistence.PrivateKeyPersister, - ): List { + ): RegistrationKeySet { val expected = RegistrationKeys.keyCount(includeDashPayKeys) try { // Inside the try so the catch below scrubs the derived scalars even @@ -64,6 +65,10 @@ object DashpayKeyProvisioning { require(previews.size == expected) { "expected $expected derived registration keys, got ${previews.size}" } + val identityIndex = previews.first().identityIndex + require(previews.all { it.identityIndex == identityIndex }) { + "every registration key must use identityIndex $identityIndex" + } for (preview in previews) { IdentityKeyPersistence.storeAndScrub( publicKeyHex = preview.publicKeyHex, @@ -80,9 +85,12 @@ object DashpayKeyProvisioning { throw e } // Only the public keys cross into the rich rows — the privates are gone. - return RegistrationKeys.buildRegistrationRows( - previews.map { it.publicKey }, - includeDashPayKeys, + return RegistrationKeySet( + identityIndex = previews.first().identityIndex, + rows = RegistrationKeys.buildRegistrationRows( + previews.map { it.publicKey }, + includeDashPayKeys, + ), ) } } diff --git a/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/identity/CreateIdentityScreen.kt b/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/identity/CreateIdentityScreen.kt index 564bbbcd79..e1c2c5467a 100644 --- a/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/identity/CreateIdentityScreen.kt +++ b/packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/identity/CreateIdentityScreen.kt @@ -189,15 +189,17 @@ fun CreateIdentityScreen(navController: NavHostController) { modifier = Modifier.fillMaxWidth().testTag("createIdentity.submit"), ) { val wallet = selectedWallet ?: return@SubmitButton - val amount = if (fundingSource == CreateIdentityFundingSource.AssetLockResume) { - null - } else { - amountText.toLongOrNull() ?: return@SubmitButton - } - if (!isCreateIdentityFundingAmountValid(fundingSource, amount)) { - error = if (fundingSource == CreateIdentityFundingSource.CoreBalance) { + val submission = createIdentitySubmissionSnapshot( + fundingSource = fundingSource, + amountText = amountText, + identityIndexText = identityIndexText, + selectedRecoveryLock = selectedRecoveryLock, + ) ?: return@SubmitButton + val amount = submission.amount + if (!isCreateIdentityFundingAmountValid(submission.fundingSource, amount)) { + error = if (submission.fundingSource == CreateIdentityFundingSource.CoreBalance) { val minimum = minimumCoreFundingDuffsForKeyCount( - RegistrationKeys.keyCount(fundingSource.includesDashPayKeys), + RegistrationKeys.keyCount(submission.includesDashPayKeys), ) "Core-funded registration with DashPay keys requires at least " + "$minimum duffs." @@ -206,16 +208,51 @@ fun CreateIdentityScreen(navController: NavHostController) { } return@SubmitButton } - val identityIndex = if (fundingSource == CreateIdentityFundingSource.AssetLockResume) { - selectedRecoveryLock?.registrationIndex ?: return@SubmitButton - } else { - identityIndexText.toIntOrNull() ?: return@SubmitButton - } + val identityIndex = submission.identityIndex val mgr = manager ?: return@SubmitButton - val registerDashPayKeys = fundingSource.includesDashPayKeys + val registerDashPayKeys = submission.includesDashPayKeys isSubmitting = true scope.launch { try { + // Freeze and validate funding-specific prerequisites + // BEFORE deriving or persisting any registration key. + // The UI remains interactive while this coroutine runs, + // but every operation below uses only `submission`. + val platformInputs = if ( + submission.fundingSource == CreateIdentityFundingSource.PlatformAddress + ) { + val requiredAmount = amount + ?: error("A funding amount is required") + val inputs = packFundingInputs( + candidates = wallet.addressesWithBalances(), + target = requiredAmount, + keyCount = RegistrationKeys.keyCount(registerDashPayKeys), + ) + if (inputs.isEmpty()) { + error = "Not enough Platform-address balance to fund " + + "$requiredAmount credits and reserve the identity-create fee." + return@launch + } + inputs + } else { + null + } + val shieldedFallbackAddress = if ( + submission.fundingSource == CreateIdentityFundingSource.ShieldedBalance + ) { + val fallbackInput = wallet.addressesWithBalances().firstOrNull() + if (fallbackInput == null) { + error = "This wallet has no Platform-payment address for " + + "the required Type-20 creation-failure fallback. Fund " + + "a Platform address first (Wallet → Platform Balance → " + + "Top Up from Core)." + return@launch + } + byteArrayOf(fallbackInput.addressType.toByte()) + fallbackInput.hash + } else { + null + } + // Step 1 (`.preparingKeys`): derive the full registration // key set in ONE pass, persist each private key to the // Keystore, and build the rich on-chain rows. Rust derives @@ -233,7 +270,7 @@ fun CreateIdentityScreen(navController: NavHostController) { identityIndex = identityIndex, count = RegistrationKeys.keyCount(registerDashPayKeys), ) - val keys = DashpayKeyProvisioning.provision( + val keySet = DashpayKeyProvisioning.provision( previews = previews, includeDashPayKeys = registerDashPayKeys, walletId = wallet.walletId, @@ -244,11 +281,12 @@ fun CreateIdentityScreen(navController: NavHostController) { container.walletStorage.storePrivateKey(hex, priv, ownerWalletId = owner) }, ) + val keys = keySet.rows // Step 2: hand the single registration FFI entry point to // the coordinator as the body — no orchestration here. The // funding source picks which registration FFI runs (ID-01 // Core asset lock vs ID-08 Platform addresses). - when (fundingSource) { + when (submission.fundingSource) { CreateIdentityFundingSource.CoreBalance -> { coordinator.startRegistration( walletId = wallet.walletId, @@ -269,21 +307,8 @@ fun CreateIdentityScreen(navController: NavHostController) { ) } CreateIdentityFundingSource.PlatformAddress -> { - // Enumerate balance-carrying Platform-payment - // addresses, then greedily pack up to `amount` - // credits (largest first) — matching the ID-06 - // top-up input assembly. Nonces are auto-fetched - // Rust-side by the register-from-addresses FFI. - val candidates = wallet.addressesWithBalances() - .sortedByDescending { it.credits } - val requiredAmount = amount - ?: error("A funding amount is required") - val inputs = packFundingInputs(candidates, requiredAmount) - if (inputs.isEmpty()) { - error = "Not enough Platform-address balance to fund " + - "$requiredAmount credits. Fund a Platform address first." - return@launch - } + val inputs = platformInputs + ?: error("Platform funding inputs were not prepared") coordinator.startRegistration( walletId = wallet.walletId, identityIndex = identityIndex, @@ -308,17 +333,8 @@ fun CreateIdentityScreen(navController: NavHostController) { // — build it from the wallet's first Platform- // payment address (1 variant tag + 20-byte hash), // the same (addressType, hash) pairing ID-08 feeds. - val fallbackInput = wallet.addressesWithBalances().firstOrNull() - if (fallbackInput == null) { - error = "This wallet has no Platform-payment address for " + - "the required Type-20 creation-failure fallback. Fund " + - "a Platform address first (Wallet → Platform Balance → " + - "Top Up from Core)." - return@launch - } - val fallbackAddress = - byteArrayOf(fallbackInput.addressType.toByte()) + - fallbackInput.hash + val fallbackAddress = shieldedFallbackAddress + ?: error("Shielded fallback address was not prepared") coordinator.startRegistration( walletId = wallet.walletId, identityIndex = identityIndex, @@ -344,7 +360,7 @@ fun CreateIdentityScreen(navController: NavHostController) { ) } CreateIdentityFundingSource.AssetLockResume -> { - val lock = selectedRecoveryLock + val lock = submission.recoveryLock ?: error("No resumable registration asset lock is selected") coordinator.startRegistration( walletId = wallet.walletId, @@ -356,7 +372,7 @@ fun CreateIdentityScreen(navController: NavHostController) { walletHandle = wallet.handle, lock = it, identityIndex = it.registrationIndex, - keys = keys, + keys = keySet, signerHandle = mgr.signerHandle, coreSignerHandle = mgr.mnemonicResolverHandle, ) @@ -414,12 +430,58 @@ enum class CreateIdentityFundingSource( get() = this != AssetLockResume } +/** Immutable control values captured synchronously when Submit is tapped. */ +internal data class CreateIdentitySubmissionSnapshot( + val fundingSource: CreateIdentityFundingSource, + val amount: Long?, + val identityIndex: Int, + val recoveryLock: TrackedAssetLock?, +) { + val includesDashPayKeys: Boolean = fundingSource.includesDashPayKeys +} + +/** + * Resolve every mutable form control into one submission value before the + * first suspension. Resume locks are defensively copied, including the txid + * bytes, so later UI/state updates cannot redirect the in-flight operation. + */ +internal fun createIdentitySubmissionSnapshot( + fundingSource: CreateIdentityFundingSource, + amountText: String, + identityIndexText: String, + selectedRecoveryLock: TrackedAssetLock?, +): CreateIdentitySubmissionSnapshot? { + val lock = if (fundingSource == CreateIdentityFundingSource.AssetLockResume) { + selectedRecoveryLock?.copy(outpointTxid = selectedRecoveryLock.outpointTxid.copyOf()) + ?: return null + } else { + null + } + val amount = if (fundingSource == CreateIdentityFundingSource.AssetLockResume) { + null + } else { + amountText.toLongOrNull() ?: return null + } + val identityIndex = lock?.registrationIndex + ?: identityIndexText.toIntOrNull() + ?: return null + return CreateIdentitySubmissionSnapshot( + fundingSource = fundingSource, + amount = amount, + identityIndex = identityIndex, + recoveryLock = lock, + ) +} + /** Credits charged by the active fee schedule before the per-key surcharge. */ private const val IDENTITY_CREATE_BASE_COST_CREDITS = 2_000_000L /** Credits charged for each public key in an identity-create transition. */ private const val IDENTITY_KEY_CREATION_COST_CREDITS = 6_500_000L +/** Credits charged for every Platform-address funding input. */ +private const val ADDRESS_FUNDING_INPUT_COST_CREDITS = 500_000L + /** Core asset-lock floor required before identity-create processing starts. */ private const val IDENTITY_ASSET_LOCK_BASE_DUFFS = 200_000L @@ -437,6 +499,21 @@ internal fun minimumCoreFundingDuffsForKeyCount(keyCount: Int): Long { IDENTITY_KEY_CREATION_COST_CREDITS * keyCount) / CREDITS_PER_DUFF } +/** + * Protocol fee for an address-funded identity create without a refund output. + * Mirrors `IdentityCreateFromAddressesTransition::calculate_min_required_fee`. + */ +internal fun minimumPlatformAddressFundingFeeCredits( + keyCount: Int, + inputCount: Int, +): Long { + require(keyCount > 0) { "keyCount must be positive, got $keyCount" } + require(inputCount > 0) { "inputCount must be positive, got $inputCount" } + return IDENTITY_CREATE_BASE_COST_CREDITS + + IDENTITY_KEY_CREATION_COST_CREDITS * keyCount + + ADDRESS_FUNDING_INPUT_COST_CREDITS * inputCount +} + /** Single submit/UI gate for the active funding source's amount semantics. */ internal fun isCreateIdentityFundingAmountValid( source: CreateIdentityFundingSource, @@ -564,25 +641,84 @@ private fun FundingSection( } /** - * Greedily select funding inputs from [candidates] (assumed sorted - * largest-balance-first) until their combined credits cover [target], - * spending only what's needed from the final address. Returns an empty list - * when the candidates can't cover [target]. Mirrors the ID-06 top-up - * `packInputs`. + * Select the smallest feasible Platform-address input set that contributes + * exactly [target] credits while leaving the full identity-create fee on the + * address that Rust's default `DeductFromInput(0)` strategy will use. + * + * The native FFI collects inputs into a `BTreeMap`, + * so input 0 is the lowest `(addressType, unsigned hash bytes)` address, not + * the first item in this list. Each attempted fee payer therefore selects only + * lexicographically greater companion inputs. The payer keeps + * [minimumPlatformAddressFundingFeeCredits] after its requested spend; all + * selected inputs contribute at least one credit. Returns an empty list when + * the requested identity balance plus fee cannot be funded. */ -private fun packFundingInputs(candidates: List, target: Long): List { +internal fun packFundingInputs( + candidates: List, + target: Long, + keyCount: Int = RegistrationKeys.keyCount(includeDashPayKeys = true), +): List { if (target <= 0) return emptyList() - var remaining = target - val picked = ArrayList() - for (input in candidates) { - if (remaining <= 0) break - val spend = minOf(input.credits, remaining) - picked.add(input.copy(credits = spend)) - remaining -= spend + + val byAddress = candidates.sortedWith(::comparePlatformAddresses) + for (inputCount in 1..byAddress.size) { + if (target < inputCount) break // every encoded input must spend >= 1 credit + val fee = minimumPlatformAddressFundingFeeCredits(keyCount, inputCount) + + for (payerIndex in byAddress.indices) { + val payer = byAddress[payerIndex] + val payerCapacity = payer.credits - fee + if (payerCapacity < 1) continue + + // Only greater addresses may accompany this payer, otherwise the + // BTreeMap would choose a different input 0 for fee deduction. + val companions = byAddress + .subList(payerIndex + 1, byAddress.size) + .sortedByDescending { it.credits } + .take(inputCount - 1) + if (companions.size != inputCount - 1) continue + + var totalCapacity = payerCapacity + for (companion in companions) { + totalCapacity = saturatingAdd(totalCapacity, companion.credits) + } + if (totalCapacity < target) continue + + val selected = listOf(payer) + companions + val spends = LongArray(inputCount) { 1L } + var remaining = target - inputCount + + // Spend companions first so the BTreeMap-first payer retains as + // much of its fee-bearing balance as possible. + for (index in 1 until selected.size) { + val extra = minOf(selected[index].credits - 1, remaining) + spends[index] += extra + remaining -= extra + } + val payerExtra = minOf(payerCapacity - 1, remaining) + spends[0] += payerExtra + remaining -= payerExtra + check(remaining == 0L) { "feasible funding allocation left $remaining credits" } + + return selected.mapIndexed { index, input -> input.copy(credits = spends[index]) } + } } - return if (remaining <= 0) picked else emptyList() + return emptyList() } +private fun comparePlatformAddresses(left: FundingInput, right: FundingInput): Int { + val typeOrder = left.addressType.compareTo(right.addressType) + if (typeOrder != 0) return typeOrder + for (index in left.hash.indices) { + val byteOrder = left.hash[index].toUByte().compareTo(right.hash[index].toUByte()) + if (byteOrder != 0) return byteOrder + } + return 0 +} + +private fun saturatingAdd(left: Long, right: Long): Long = + if (Long.MAX_VALUE - left < right) Long.MAX_VALUE else left + right + /** Identity-slot section — the Swift `CreateIdentityView` identity-index stepper. */ @Composable private fun KeysSection(identityIndexText: String, onIndexChange: (String) -> Unit) { diff --git a/packages/kotlin-sdk/KotlinExampleApp/app/src/test/java/org/dashfoundation/example/services/DashpayKeyProvisioningTest.kt b/packages/kotlin-sdk/KotlinExampleApp/app/src/test/java/org/dashfoundation/example/services/DashpayKeyProvisioningTest.kt index 99842f5f8a..b6f04b792f 100644 --- a/packages/kotlin-sdk/KotlinExampleApp/app/src/test/java/org/dashfoundation/example/services/DashpayKeyProvisioningTest.kt +++ b/packages/kotlin-sdk/KotlinExampleApp/app/src/test/java/org/dashfoundation/example/services/DashpayKeyProvisioningTest.kt @@ -36,7 +36,7 @@ class DashpayKeyProvisioningTest { val walletId = ByteArray(32) { 9 } data class StoredKey(val hex: String, val scalar: ByteArray, val owner: ByteArray) val stored = mutableListOf() - val rows = DashpayKeyProvisioning.provision( + val keySet = DashpayKeyProvisioning.provision( previews = previews, includeDashPayKeys = true, walletId = walletId, @@ -44,7 +44,9 @@ class DashpayKeyProvisioningTest { stored += StoredKey(hex, privateKey.copyOf(), owner.copyOf()) }, ) + val rows = keySet.rows + assertEquals(5, keySet.identityIndex) assertEquals(6, rows.size) assertEquals(listOf(0, 1, 2, 3, 4, 5), rows.map { it.keyId }) assertEquals(KeyPurpose.ENCRYPTION, rows[4].purpose) @@ -77,12 +79,13 @@ class DashpayKeyProvisioningTest { @Test fun `resume provisions only the base four keys`() = runTest { val previews = (0 until 4).map(::preview) - val rows = DashpayKeyProvisioning.provision( + val keySet = DashpayKeyProvisioning.provision( previews = previews, includeDashPayKeys = false, walletId = ByteArray(32) { 9 }, persister = { _, _, _ -> }, ) + val rows = keySet.rows assertEquals(4, rows.size) assertTrue(rows.none { it.contractBounds != null }) } @@ -131,6 +134,32 @@ class DashpayKeyProvisioningTest { } } + @Test + fun `mixed identity slots are rejected before persist and all scalars are scrubbed`() = runTest { + var persisted = false + val previews = (0 until 4).map(::preview).toMutableList().also { + it[3] = IdentityKeyPreview( + identityIndex = 6, + derivationPath = it[3].derivationPath, + publicKey = it[3].publicKey, + privateKey = it[3].privateKey, + ) + } + + val failure = runCatching { + DashpayKeyProvisioning.provision( + previews = previews, + includeDashPayKeys = false, + walletId = ByteArray(32) { 9 }, + persister = { _, _, _ -> persisted = true }, + ) + } + + assertTrue(failure.exceptionOrNull() is IllegalArgumentException) + assertFalse(persisted) + previews.forEach { assertArrayEquals(ByteArray(32), it.privateKey) } + } + @Test fun `only asset-lock resume excludes DashPay provisioning`() { assertTrue(CreateIdentityFundingSource.CoreBalance.includesDashPayKeys) diff --git a/packages/kotlin-sdk/KotlinExampleApp/app/src/test/java/org/dashfoundation/example/ui/identity/CreateIdentityFundingPolicyTest.kt b/packages/kotlin-sdk/KotlinExampleApp/app/src/test/java/org/dashfoundation/example/ui/identity/CreateIdentityFundingPolicyTest.kt index a46360f27d..8a04ce543f 100644 --- a/packages/kotlin-sdk/KotlinExampleApp/app/src/test/java/org/dashfoundation/example/ui/identity/CreateIdentityFundingPolicyTest.kt +++ b/packages/kotlin-sdk/KotlinExampleApp/app/src/test/java/org/dashfoundation/example/ui/identity/CreateIdentityFundingPolicyTest.kt @@ -1,5 +1,8 @@ package org.dashfoundation.example.ui.identity +import org.dashfoundation.dashsdk.credits.FundingInput +import org.dashfoundation.dashsdk.wallet.TrackedAssetLock +import org.junit.Assert.assertArrayEquals import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue @@ -38,4 +41,117 @@ class CreateIdentityFundingPolicyTest { ), ) } + + @Test + fun `Platform address packing reserves the six-key fee on input zero`() { + val inputs = packFundingInputs( + candidates = listOf( + FundingInput( + addressType = 0, + hash = ByteArray(20) { 1 }, + credits = 70_000_000L, + ), + ), + target = 30_000_000L, + ) + + // One six-key input needs 41.5M credits left after the requested + // 30M spend. This address would leave only 40M, so submitting it + // would persist all keys and then fail on-chain. + assertTrue(inputs.isEmpty()) + } + + @Test + fun `Platform address fee scales with key and input counts`() { + assertEquals(41_500_000L, minimumPlatformAddressFundingFeeCredits(6, 1)) + assertEquals(42_000_000L, minimumPlatformAddressFundingFeeCredits(6, 2)) + } + + @Test + fun `Platform address packing accepts the exact one-input fee boundary`() { + val inputs = packFundingInputs( + candidates = listOf(fundingInput(hashByte = 1, credits = 71_500_000L)), + target = 30_000_000L, + keyCount = 6, + ) + + assertEquals(1, inputs.size) + assertEquals(30_000_000L, inputs.single().credits) + } + + @Test + fun `Platform address packing leaves the two-input fee on BTreeMap input zero`() { + val lowerAddress = fundingInput(hashByte = 1, credits = 42_000_001L) + val higherAddress = fundingInput(hashByte = 2, credits = 29_999_999L) + val inputs = packFundingInputs( + // Deliberately reverse native BTreeMap order. + candidates = listOf(higherAddress, lowerAddress), + target = 30_000_000L, + keyCount = 6, + ) + + assertEquals(2, inputs.size) + assertEquals(30_000_000L, inputs.sumOf { it.credits }) + val inputZero = inputs.minWith(::compareFundingAddressesForTest) + val originalBalance = if (inputZero.hash[0] == 1.toByte()) { + lowerAddress.credits + } else { + higherAddress.credits + } + assertTrue(originalBalance - inputZero.credits >= 42_000_000L) + } + + @Test + fun `submission snapshot is not redirected by later form changes`() { + val originalTxid = ByteArray(32) { 7 } + var source = CreateIdentityFundingSource.AssetLockResume + var selectedLock = registrationLock(registrationIndex = 5, txid = originalTxid) + + val snapshot = createIdentitySubmissionSnapshot( + fundingSource = source, + amountText = "999", + identityIndexText = "42", + selectedRecoveryLock = selectedLock, + )!! + + source = CreateIdentityFundingSource.CoreBalance + selectedLock = registrationLock(registrationIndex = 9, txid = ByteArray(32) { 9 }) + originalTxid.fill(3) + + assertEquals(CreateIdentityFundingSource.AssetLockResume, snapshot.fundingSource) + assertFalse(snapshot.includesDashPayKeys) + assertEquals(5, snapshot.identityIndex) + assertEquals(null, snapshot.amount) + assertEquals(5, snapshot.recoveryLock!!.registrationIndex) + assertArrayEquals(ByteArray(32) { 7 }, snapshot.recoveryLock.outpointTxid) + // Keep assignments observable so the test documents the controls did change. + assertEquals(CreateIdentityFundingSource.CoreBalance, source) + assertEquals(9, selectedLock.registrationIndex) + } + + private fun fundingInput(hashByte: Int, credits: Long) = FundingInput( + addressType = 0, + hash = ByteArray(20).also { it[0] = hashByte.toByte() }, + credits = credits, + ) + + private fun compareFundingAddressesForTest(left: FundingInput, right: FundingInput): Int { + val typeOrder = left.addressType.compareTo(right.addressType) + if (typeOrder != 0) return typeOrder + for (index in left.hash.indices) { + val order = left.hash[index].toUByte().compareTo(right.hash[index].toUByte()) + if (order != 0) return order + } + return 0 + } + + private fun registrationLock(registrationIndex: Int, txid: ByteArray) = TrackedAssetLock( + outpointTxid = txid, + outpointVout = 0, + fundingType = TrackedAssetLock.FundingType.IDENTITY_REGISTRATION, + status = TrackedAssetLock.Status.BUILT, + registrationIndex = registrationIndex, + instantLockPresent = false, + chainLockHeight = 0, + ) } diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityRegistration.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityRegistration.kt index b9e84ed484..0aef05ce72 100644 --- a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityRegistration.kt +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityRegistration.kt @@ -115,8 +115,9 @@ class IdentityRegistration internal constructor( * replacement funding transaction. Generic recovery cannot consume an * invitation voucher. * - * [keys] are the rich registration rows (built via - * [RegistrationKeys.buildRegistrationRows]). Resume carries the SAME key + * [keys] carries the rich registration rows (built via + * [RegistrationKeys.buildRegistrationRows]) and the HD slot that derived + * them. Resume carries the SAME key * set the interrupted registration originally committed to on-chain — the * base four auth/transfer keys, **without** the DashPay pair: an * already-spent asset lock funds a fixed key count, so retroactively @@ -130,7 +131,7 @@ class IdentityRegistration internal constructor( walletHandle: Long, lock: TrackedAssetLock, identityIndex: Int, - keys: List, + keys: RegistrationKeySet, signerHandle: Long, coreSignerHandle: Long, ): ByteArray = gate.op { @@ -141,14 +142,17 @@ class IdentityRegistration internal constructor( require(identityIndex == lock.registrationIndex) { "identityIndex $identityIndex does not match tracked lock registrationIndex ${lock.registrationIndex}" } - require(keys.isNotEmpty()) { "keys must not be empty" } + require(keys.identityIndex == lock.registrationIndex) { + "registration keys use identityIndex ${keys.identityIndex}, expected tracked lock " + + "registrationIndex ${lock.registrationIndex}" + } val native = mapNativeErrors { resumeNative.call( walletHandle = walletHandle, outpointTxid = lock.outpointTxid, outpointVout = lock.outpointVout, identityIndex = identityIndex, - pubkeysBlob = IdentityPubkeyCodec.encode(keys), + pubkeysBlob = IdentityPubkeyCodec.encode(keys.rows), signerHandle = signerHandle, coreSignerHandle = coreSignerHandle, consumeInvitationVoucher = false, diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/RegistrationKeys.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/RegistrationKeys.kt index 1d26b799ab..dcb76ef305 100644 --- a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/RegistrationKeys.kt +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/RegistrationKeys.kt @@ -1,5 +1,24 @@ package org.dashfoundation.dashsdk.identity +/** + * Rich registration rows tied to the identity HD slot that produced them. + * + * The row wire format deliberately contains only consensus key metadata; the + * derivation index is host-side provenance and must travel beside the rows so + * asset-lock resume can verify it still matches the tracked lock. + */ +class RegistrationKeySet( + val identityIndex: Int, + rows: List, +) { + val rows: List = rows.toList() + + init { + require(identityIndex >= 0) { "identityIndex must be non-negative, got $identityIndex" } + require(rows.isNotEmpty()) { "registration key rows must not be empty" } + } +} + /** * The canonical DPP-role layout for a freshly-registered identity's keys — * the Kotlin source of truth that replaces the retired Rust diff --git a/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/identity/IdentityAssetLockRecoveryTest.kt b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/identity/IdentityAssetLockRecoveryTest.kt index ab44689b52..eb44ffb6d0 100644 --- a/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/identity/IdentityAssetLockRecoveryTest.kt +++ b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/identity/IdentityAssetLockRecoveryTest.kt @@ -39,7 +39,7 @@ class IdentityAssetLockRecoveryTest { walletHandle = 1, lock = lock, identityIndex = 7, - keys = listOf(key()), + keys = keySet(), signerHandle = 2, coreSignerHandle = 3, ) @@ -64,7 +64,7 @@ class IdentityAssetLockRecoveryTest { val failure = runCatching { registration.resumeWithExistingAssetLock( 1, lock(TrackedAssetLock.FundingType.IDENTITY_REGISTRATION), - 7, listOf(key()), 2, 3, + 7, keySet(), 2, 3, ) } assertTrue(failure.exceptionOrNull() is IllegalStateException) @@ -84,7 +84,7 @@ class IdentityAssetLockRecoveryTest { val failure = runCatching { registration.resumeWithExistingAssetLock( 1, lock(TrackedAssetLock.FundingType.IDENTITY_TOP_UP), - 7, listOf(key()), 2, 3, + 7, keySet(), 2, 3, ) } assertTrue(failure.exceptionOrNull() is IllegalArgumentException) @@ -104,7 +104,31 @@ class IdentityAssetLockRecoveryTest { val failure = runCatching { registration.resumeWithExistingAssetLock( 1, lock(TrackedAssetLock.FundingType.IDENTITY_REGISTRATION), - 8, listOf(key()), 2, 3, + 8, keySet(), 2, 3, + ) + } + assertTrue(failure.exceptionOrNull() is IllegalArgumentException) + assertFalse(called) + } + + @Test + fun `registration rejects keys derived from a different HD slot before JNI`() = runTest { + var called = false + val registration = IdentityRegistration( + resumeNative = ResumeIdentityNativeCall { _, _, _, _, _, _, _, _ -> + called = true + IdentityRegistrationNativeResult(ByteArray(32), 1) + }, + ) + + val failure = runCatching { + registration.resumeWithExistingAssetLock( + 1, + lock(TrackedAssetLock.FundingType.IDENTITY_REGISTRATION), + 7, + keySet(identityIndex = 8), + 2, + 3, ) } assertTrue(failure.exceptionOrNull() is IllegalArgumentException) @@ -128,7 +152,7 @@ class IdentityAssetLockRecoveryTest { val operation = async(Dispatchers.Default) { registration.resumeWithExistingAssetLock( 1, lock(TrackedAssetLock.FundingType.IDENTITY_REGISTRATION), - 7, listOf(key()), 2, 3, + 7, keySet(), 2, 3, ) } assertTrue(entered.await(5, TimeUnit.SECONDS)) @@ -145,9 +169,13 @@ class IdentityAssetLockRecoveryTest { assertTrue(closed) } - // Resume carries the rich base MASTER row; the resume path only checks the - // list is non-empty (the key set is fixed by the tracked lock, not - // re-validated per key here). + private fun keySet(identityIndex: Int = 7) = RegistrationKeySet( + identityIndex = identityIndex, + rows = listOf(key()), + ) + + // Resume carries the rich base MASTER row; registration-only wire + // invariants are enforced below JNI after the host-side HD-slot check. private fun key() = IdentityPubkey( keyId = 0, keyType = KeyType.ECDSA_SECP256K1, From 2aa999cff225b191e52bb8da35f039441074b71d Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Tue, 21 Jul 2026 19:55:20 +0700 Subject: [PATCH 09/10] docs(kotlin-sdk): document preview blob scrubbing --- .../org/dashfoundation/dashsdk/identity/IdentityKeyPreview.kt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityKeyPreview.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityKeyPreview.kt index 9771e94452..b26fa64e9e 100644 --- a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityKeyPreview.kt +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityKeyPreview.kt @@ -45,6 +45,10 @@ data class IdentityKeyPreview( * `IdentityNative.previewRegistrationKeys`. Layout (big-endian): * `u32 rowCount` then per row `u32 identityIndex, u16 pathLen, * pathUtf8, u8[33] pubkey, u8[32] privkey`. + * + * Destructively wipes [blob] on both successful and failed decoding + * because it contains raw private-key scalars. Callers must not reuse + * the array after passing it here. */ fun decodeAll(blob: ByteArray): List { val buf = ByteBuffer.wrap(blob) // big-endian by default From 0ab30e798c924af5f1e193432235c1b9b25a426a Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Tue, 21 Jul 2026 21:32:57 +0700 Subject: [PATCH 10/10] docs(kotlin-sdk): drop internal registration spec + codex review scratch notes Working docs from the design/review phase; not intended for the repo. Net zero in the PR diff (added earlier in this PR, removed here). Co-Authored-By: Claude Opus 4.8 --- docs/sdk/CODEX_REVIEW_NOTES.md | 395 ------------------ ...TLIN_SDK_DASHPAY_REGISTRATION_KEYS_SPEC.md | 384 ----------------- 2 files changed, 779 deletions(-) delete mode 100644 docs/sdk/CODEX_REVIEW_NOTES.md delete mode 100644 docs/sdk/KOTLIN_SDK_DASHPAY_REGISTRATION_KEYS_SPEC.md diff --git a/docs/sdk/CODEX_REVIEW_NOTES.md b/docs/sdk/CODEX_REVIEW_NOTES.md deleted file mode 100644 index 1c0b45c420..0000000000 --- a/docs/sdk/CODEX_REVIEW_NOTES.md +++ /dev/null @@ -1,395 +0,0 @@ -# Independent review: Kotlin DashPay registration keys - -**Review date:** 2026-07-20 - -**Reviewed branch/worktree:** `feat/kotlin-sdk-dashpay-registration-keys` - -**Reviewed input:** `KOTLIN_SDK_DASHPAY_REGISTRATION_KEYS_SPEC.md` - -## Bottom line - -The underlying defect is real, the proposed PR split is sensible, and the main -design direction (send rich key rows over JNI and provision the DashPay -ENCRYPTION/DECRYPTION pair before identity creation) is sound. This is not a -consensus-protocol change and the existing C `IdentityPubkeyFFI` layout is -already sufficient. - -I would not implement the spec exactly as written yet. The following material -points need reconciliation first: - -1. Promoting only `DecodedPubkeyRow` and `decode_update_pubkeys_blob` to - `pub(crate)` is insufficient: every field on the row is still private. -2. The key-id-0 invariant must not be added unconditionally to the shared - update decoder. Identity updates legitimately add rows without key 0. -3. CREATE lacks the explicit contract-bounds *validation* call used by UPDATE, - but Drive's CREATE insertion path still resolves the contract/document type - and checks its bounded-key requirement. The spec's "any wrong constant - registers successfully" and "exact Add Contact bug" failure description is - therefore inaccurate. -4. Adding two keys changes the creation fee and exactly reaches the protocol's - six-key maximum. Swift intentionally excludes the unused-asset-lock/resume - path, while this spec includes it. Kotlin does not expose the tracked lock's - amount, so a four-key-minimum lock may be insufficient for a six-key create. -5. The C/JNI function signatures remain stable, but the opaque Kotlin-to-JNI - byte format changes incompatibly and the affected Kotlin registration APIs - are public. This is a logical ABI/API concern even though it is not a - protocol or C-struct change. -6. The proposed tests do not yet prove cross-language wire compatibility or - the on-chain contract bounds, and one verification command names a - nonexistent Cargo package. - -## 1. Technical-claim verification - -### 1.1 Bug and current registration paths: verified - -- `role_for_registration_key_id` assigns roles solely from key ID and has no - ENCRYPTION/DECRYPTION case - (`packages/rs-unified-sdk-jni/src/identity.rs:77-131`). -- The legacy decoder carries only `keyId` and public bytes - (`packages/rs-unified-sdk-jni/src/identity.rs:930-1003`). -- All four JNI paths reconstruct the role, set `read_only: false`, and set - `contract_bounds_kind: 0`: - - - resume: `packages/rs-unified-sdk-jni/src/identity.rs:599-623` - - Core/asset-lock funded: `packages/rs-unified-sdk-jni/src/identity.rs:732-767` - - Platform-address funded: `packages/rs-unified-sdk-jni/src/identity.rs:846-882` - - shielded: `packages/rs-unified-sdk-jni/src/funding.rs:703-748` - -- The Add Contact send path calls `select_own_encryption_key` - (`packages/rs-platform-wallet/src/wallet/identity/network/contact_requests.rs:455-465`), - which requires an enabled ECDSA_SECP256K1 ENCRYPTION key and otherwise emits - the quoted failure (`contact_requests.rs:979-1005`). A freshly registered - Kotlin identity therefore cannot pass that flow today. - -### 1.2 Rich decoder reuse: feasible, but the spec omits required changes - -The proposed parser is the correct *wire-format* reuse target: - -- `DecodedPubkeyRow` and `decode_update_pubkeys_blob` are currently private at - `packages/rs-unified-sdk-jni/src/transactions.rs:290-305`. -- The decoder reads key type, purpose, security level, read-only, bounds kind, - public key, optional contract ID, and optional document type - (`transactions.rs:318-444`). -- Update converts those owned rows into `IdentityPubkeyFFI` while retaining the - owners through the synchronous FFI call (`transactions.rs:242-285`). -- Kotlin's encoder matches that layout - (`packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityUpdates.kt:203-240`). - -However, changing the struct and function to `pub(crate)` alone will not let -`identity.rs` or `funding.rs` build FFI rows: all fields remain private at -`transactions.rs:292-300`. Either expose the fields crate-wide or, preferably, -provide a crate-private conversion method/helper that keeps pointer creation -and owner lifetimes in one place. - -The decoder should also be split into a pure `&[u8]` parser plus the thin JNI -exception adapter. The current function requires `JNIEnv`/`JByteArray` -(`transactions.rs:305-306`), which makes the proposed ordinary Rust round-trip -unit test unnecessarily difficult. - -### 1.3 FFI bounds support: verified - -- `IdentityPubkeyFFI` already contains all rich fields; no struct-layout change - is needed - (`packages/rs-platform-wallet-ffi/src/identity_registration_with_signer.rs:110-127`). -- `decode_contract_bounds` is correctly described as the wrong raw-JNI reuse - target: it is `pub(crate)` in another crate and consumes an already-formed - `&IdentityPubkeyFFI` (`identity_registration_with_signer.rs:129-160`). -- It rejects bounds kind 0 for ENCRYPTION/DECRYPTION - (`identity_registration_with_signer.rs:161-173`) and maps kinds 1/2 at - `:175-225`. -- `decode_identity_pubkeys` validates discriminants, public-key pointers, and - contract bounds before constructing DPP keys - (`identity_registration_with_signer.rs:273-323`). All four Android routes - eventually call it (funded/resume in - `identity_registration_funded_with_signer.rs:63-90,180-220`, address-funded - at `identity_registration_with_signer.rs:434-470`, and shielded at - `shielded_send.rs:663-720`). - -One scope detail is missing: `decode_identity_pubkeys` has a fifth caller, the -Swift invitation claim path -(`packages/rs-platform-wallet-ffi/src/invitation.rs:244-300`). Moving a new -key-0 check into this FFI decoder affects that path too. Invitation already has -the same downstream invariant at -`packages/rs-platform-wallet/src/wallet/identity/network/invitation.rs:127-153`, -so the behavioral change should be benign, but it needs an explicit test and -should not be described as touching only four paths. - -### 1.4 Key ID 0 pre-flight gap: verified, with a placement warning - -- The shared funded/resume implementation rejects an empty map and requires - `keys_map[0]` to be MASTER + AUTHENTICATION - (`packages/rs-platform-wallet/src/wallet/identity/network/registration.rs:134-159`). -- Address-funded registration builds an identity directly from the decoded map - and calls `register_from_addresses` without the same check - (`packages/rs-platform-wallet-ffi/src/identity_registration_with_signer.rs:437-470`). -- Shielded registration converts the decoded map to a vector and proceeds - without it (`packages/rs-platform-wallet-ffi/src/shielded_send.rs:703-720`). - -This is specifically a **key ID 0** invariant, not "row 0" or "the first row." -DPP requires exactly one MASTER but does not require that master's ID to be 0. -The ID convention matters to wallet derivation and loading (`MASTER_KEY_INDEX` -is 0 in -`packages/rs-platform-wallet/src/wallet/identity/network/identity_handle.rs:55`). - -Do not put this assertion directly in `decode_update_pubkeys_blob`: that parser -is used by `updateIdentity` (`transactions.rs:193-196`), and a normal add-key -update neither includes nor should include key 0. Safe options are: - -- validate after decoding in a registration-only JNI wrapper used by the four - registration exports; or -- validate in FFI `decode_identity_pubkeys`, acknowledging/testing the - invitation caller as well. - -### 1.5 DPP structure and proof-of-possession claims: mostly verified - -`validate_identity_public_keys_structure_v0` does enforce: - -- ENCRYPTION = MEDIUM only, DECRYPTION = MEDIUM only, TRANSFER = CRITICAL only - (`packages/rs-dpp/src/state_transition/state_transitions/identity/public_key_in_creation/methods/validate_identity_public_keys_structure/v0/mod.rs:21-37,118-153`); -- duplicate key IDs/data (`:70-95`); -- exactly one MASTER for identity creation (`:98-116`); and -- at most six creation keys via the active platform version (`:49-68` and - `packages/rs-platform-version/src/version/dpp_versions/dpp_state_transition_versions/v1.rs:18`, - with the same value in v2/v3). - -Proof of possession is also server-side, but the spec overgeneralizes the -function name. Standard asset-lock create verifies every key in -`identity_create/identity_and_signatures/v0/mod.rs:20-40`; address-funded create -uses its separate validator in -`identity_create_from_addresses/public_key_signatures/v0/mod.rs:20-42`; and -shielded create checks structure plus every PoP in -`packages/rs-drive-abci/src/execution/validation/state_transition/processor/traits/shielded_proof.rs:384-415`. - -The security conclusion is correct: caller-controlled purpose/security bytes -cannot create an elevated ENCRYPTION/DECRYPTION or invalid TRANSFER key. -However, the statement that a malformed duplicate row list necessarily reaches -DPP and fails is false at the FFI boundary: `decode_identity_pubkeys` inserts -into a `BTreeMap` without checking the previous value -(`identity_registration_with_signer.rs:279-320`), so a later duplicate ID -silently overwrites the earlier row. The surviving key is still structurally -validated, so this does not create the claimed privilege escalation, but the -decoder should reject duplicate IDs explicitly. - -### 1.6 CREATE-time contract-bounds semantics: the spec is only partly right - -The narrow call-graph claim is true: the explicit -`validate_identity_public_keys_contract_bounds` state validator is called by -identity UPDATE -(`packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_update/state/v0/mod.rs:97-110`), -not by standard or address identity CREATE. Those CREATE advanced-structure -paths call structural validation and PoP only -(`identity_create/advanced_structure/v0/mod.rs:80-116` and -`identity_create_from_addresses/advanced_structure/v0/mod.rs:34-71`). Shielded -CREATE likewise has only the structure/PoP checks cited above. - -The stronger conclusion in the spec is not true. CREATE execution stores the -new identity through `create_key_tree_with_keys_operations` -(`packages/rs-drive/src/drive/identity/insert/add_new_identity/v0/mod.rs:259-270`). -Each key insertion calls -`add_potential_contract_info_for_contract_bounded_key` -(`packages/rs-drive/src/drive/identity/key/insert/create_key_tree_with_keys/v0/mod.rs:98-123`). -That path: - -- fetches the referenced contract and errors if it does not exist - (`packages/rs-drive/src/drive/identity/contract_info/keys/mod.rs:62-86`); -- resolves the named document type (`keys/mod.rs:94-105`); and -- requires that document type to declare the matching ENCRYPTION/DECRYPTION - bounded-key requirement - (`packages/rs-drive/src/drive/identity/contract_info/keys/add_potential_contract_info_for_contract_bounded_key/v0/mod.rs:430-465`). - -Therefore an arbitrary nonexistent contract ID, missing document type, or -document without the relevant requirement cannot simply register -"successfully." A wrong but existing compatible contract/document could still -be accepted and indexed under the wrong scope, so pinning the constant remains -valuable. - -It also would not recreate the *exact* selector failure described in the spec. -`select_own_encryption_key` tests only enabled/type/purpose, not bounds -(`contact_requests.rs:989-1005`), and contact-request validation intentionally -accepts bound or unbound ENCRYPTION keys -(`packages/rs-platform-wallet/src/wallet/identity/crypto/validation.rs:111-121,164-167`). -The contact document is built from the canonical DashPay contract fetched -separately (`packages/rs-sdk/src/platform/dashpay/contact_request.rs:341-349`). -Consequently, a manual Add Contact success is not proof that the new keys carry -the correct contract ID/document bounds. - -### 1.7 Kotlin and Swift claims: verified with two material qualifications - -- `IdentityUpdates.kt` already has the desired `IdentityPubkey`, enums, and - bounds model (`:12-136`). `encodeAddPubkeys` is private at `:211`, and its - helper `contractBoundsKind` is also private at `:243-248`; both need factoring - if the encoder is shared. -- The Kotlin DashPay entry is private and its 32 bytes match - `packages/dashpay-contract/src/lib.rs:9-16` - (`packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/identity/AddIdentityKeyScreen.kt:381-397`). -- Swift's helper creates consecutive ENCRYPTION/DECRYPTION ECDSA keys at MEDIUM - with DashPay `contactRequest` bounds, validates private/public correspondence, - persists each private key, and returns rich rows - (`packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Services/IdentityRegistrationKeys.swift:15-121`). - -Qualification 1: the duplicate helper in `CreateIdentityView.swift` is a mirror, -not literally byte-for-byte "verbatim." That is editorial, not architectural. - -Qualification 2: Swift deliberately excludes unused-asset-lock/resume from -DashPay provisioning (`CreateIdentityView.swift:152-177,896-905`). It also -raises the minimum funding according to total key count (`:127-149`). The Kotlin -flow proposes six keys on all four routes, including resume, but Kotlin's -`TrackedAssetLock` omits the Rust lock's `amount` -(`packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/TrackedAssetLock.kt:11-19` -versus `packages/rs-platform-wallet/src/wallet/asset_lock/tracked.rs:63-72`). -The two added keys cost 13,000 additional duffs at the current 6,500,000-credit -per-key fee (`packages/rs-platform-version/src/version/fee/state_transition_min_fees/v1.rs:17`). - -The spec must decide whether resume excludes the pair (matching Swift) or gains -an amount-aware sufficiency check/error. A previously built lock funded only -for four keys may fail a six-key create. - -### 1.8 Deletion/caller audit: safe, but clean up related documentation - -A repository-wide search found executable references to -`role_for_registration_key_id`/`decode_pubkeys_blob` only at the four JNI sites -listed above plus the `funding.rs:42` import. No test calls either function, so -deletion after replacement is safe. The six local role constants at -`identity.rs:79-90` then become dead as well. - -The change should also update stale wire/positional-role documentation at: - -- `packages/rs-unified-sdk-jni/src/identity.rs:258-270,682-697,813-815` -- `packages/rs-unified-sdk-jni/src/funding.rs:650-660` -- `packages/rs-platform-wallet-ffi/src/identity_key_preview.rs:77-94,257-280` -- `packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/IdentityNative.kt:148-178` -- `packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityRegistration.kt:191-210` - -## 2. Scope and compatibility assessment - -### What is contained - -- No consensus rule or serialized state-transition format needs changing. -- No C `IdentityPubkeyFFI` field/layout change is needed. -- No Room/database migration is needed; the existing identity-key persistence - model already supports purpose/security/bounds. -- Separating this from Keystore invalidation/documentation follow-ups is a good - rollback boundary: this change spans the registration wire codec and key - provisioning, not storage invalidation. - -### Hidden scope/risk to make explicit - -1. **Opaque wire ABI.** The `byte[]` payload changes from - `count + (id,len,pubkey)` to the rich update layout. JNI method descriptors - remain identical, but an old Kotlin artifact paired with a new `.so` (or the - reverse) will be misparsed/rejected. Update both sides and their KDocs in one - release. A version/magic byte would make skew diagnosable; if that is deemed - too broad, at least reject legacy/trailing input deterministically. -2. **Public Kotlin API.** `IdentityKeyPreview` is public - (`IdentityKeyPreview.kt:17-22`), and public registration methods accept - `List` (`IdentityRegistration.kt:117-146,237-291` and - `PlatformWalletManager.kt:1344-1371`). Replacing that type, or adding fields - to its data-class constructor, has source/binary/behavior compatibility - implications for SDK consumers. Specify the compatibility strategy rather - than treating it as an app-only row refactor. -3. **Six-key ceiling.** Base four plus the pair equals, rather than merely stays - below, `max_public_keys_in_creation = 6`. Add a local assertion/test and - document that any future expansion of the base set requires redesign. -4. **Existing identities.** This fixes only freshly registered identities. - Existing Android-created identities remain without the pair and still need - the Add Identity Key repair flow. State explicitly that no backfill is in - scope, or add product guidance/detection if the intended outcome covers - existing users. -5. **Existing Kotlin reuse.** The app already has derive -> validate -> persist - -> zero -> `IdentityPubkey` plumbing in - `packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/services/IdentityKeyAdditionFlow.kt:84-182`, - used by `AddIdentityKeyScreen.kt:324-349`. Before introducing a separate - `DashpayKeyProvisioning`, decide whether a thin policy wrapper around this - helper is sufficient. Duplicating the lifecycle is another drift surface. -6. **Keypair validation parity.** Swift explicitly verifies the derived private - scalar matches the public key (`IdentityRegistrationKeys.swift:66-82`). The - current Kotlin registration preview path persists without this check - (`CreateIdentityScreen.kt:202-235`). This is existing behavior, not caused by - the pair, but a claimed Swift port should either add the defense or document - the intentional parity gap. - -## 3. Test-plan review and recommended additions - -The device/testnet Add Contact smoke remains useful as an end-to-end product -check, but it is not enough and should not be the only integration evidence. -The following can be automated: - -1. **One cross-language golden blob.** Have a Kotlin unit test encode six - canonical rows to exact bytes, and have a pure Rust `&[u8]` parser test decode - the same checked-in fixture. Independent Kotlin-object and Rust-object tests - do not catch byte-order/field-order skew. -2. **Strict Rust codec tests.** Cover both bounds kinds, truncation at each - variable field, invalid bounds kind, interior NUL, negative key ID, invalid - boolean byte, duplicate ID, trailing bytes, and explicit legacy-format - rejection. The current parser accepts every nonzero `read_only` byte as true - and ignores trailing bytes (`transactions.rs:369-444`); decide and test a - strict policy while this becomes a registration boundary. -3. **Registration-only invariant test.** Assert key ID 0 (regardless of row - order) is MASTER + AUTHENTICATION; missing/wrong ID 0 fails. Add a separate - regression proving identity-update add lists without key 0 remain accepted. -4. **FFI decoder test.** Verify duplicate IDs are rejected rather than - overwritten and ENCRYPTION/DECRYPTION without bounds are rejected. If the - key-0 check lives here, include the invitation caller behavior. -5. **Kotlin provisioning/lifecycle test.** Assert exact IDs 4/5, ECDSA type, - ENCRYPTION/DECRYPTION purposes, MEDIUM level, `readOnly=false`, exact - contract/document bounds, correct derivation indices, storage under the - matching public bytes, and private-array zeroing on success and failure. -6. **Four-way orchestration test.** Extract a small pure preparation/dispatch - seam from `CreateIdentityScreen` and verify all selected funding sources - receive the same rich six-row set. This catches omission at a call site more - cheaply than four JNI/JVM environments. -7. **On-chain bounds assertion.** After a local/testnet create, fetch the - identity and assert IDs 4/5 carry the exact DashPay ID and `contactRequest` - bounds (or query the contract-bound key index). Add Contact success alone - does not test those fields. -8. **Resume funding case.** Test the chosen policy: either four keys for resume, - or a clear early failure/success boundary based on the lock's available - amount for six keys. -9. **Canonical constant test mechanism.** The proposed "Rust test" cannot - compare a private Kotlin constant to `dashpay_contract::ID_BYTES`; Rust - cannot see Kotlin source, and `rs-unified-sdk-jni` does not currently depend - directly on `dashpay-contract`. Use generated Kotlin source/resource, a - shared checked-in cross-language golden fixture, or a Kotlin test that - consumes an independently generated canonical value. Merely copying the - same 32-byte literal into both sides is the tautology the spec wants to - avoid. - -Existing Kotlin tests also need mechanical adjustment if the registration row -type/signature changes: `IdentityAssetLockRecoveryTest.kt:169` constructs the -legacy `IdentityKeyPreview` used by resume tests. - -## 4. Verification-plan corrections - -- `cargo test -p rs-platform-wallet-ffi --lib` is invalid: the package is named - `platform-wallet-ffi` (`packages/rs-platform-wallet-ffi/Cargo.toml:1-2`). The - correct command is `cargo test -p platform-wallet-ffi --lib`. -- Keep targeted tests first, then the proposed workspace clippy/format and - Gradle suites. If production logic is added below the FFI crate, include the - directly affected wallet/DPP/Drive package tests rather than relying only on - JNI and FFI tests. -- Baseline source-head results from this review: - - - `cargo test -p rs-unified-sdk-jni --lib`: 10 passed, 0 failed. - - `cargo test -p platform-wallet-ffi --lib`: 196 passed, 0 failed. - - The spec's uncorrected `cargo test -p rs-platform-wallet-ffi --lib` - invocation fails before testing because no package has that name. - -The source audit did not modify production code or the reviewed spec. - -## 5. Recommended spec edits before implementation - -1. Specify a pure shared rich parser plus a safe FFI-row conversion API; mention - row-field visibility/ownership explicitly. -2. State exactly where registration-only key-ID-0 validation lives and protect - the update path with a regression test. -3. Rewrite the CREATE-bounds risk: no explicit consensus validator, but Drive - insertion resolves and checks bounds; the residual risk is a wrong *existing - compatible* scope and incorrect indexing, not necessarily Add Contact - selector failure. -4. Decide the resume policy and account for the two-key fee plus six-key ceiling. -5. Document the logical JNI blob compatibility and public Kotlin API strategy. -6. Replace the constant-pin and round-trip bullets with an executable - cross-language fixture plan and an on-chain bounds assertion. -7. Add duplicate-ID rejection, strict parser behavior, existing-identity - non-migration, stale-doc cleanup, and the existing `IdentityKeyAdditionFlow` - reuse decision to scope. diff --git a/docs/sdk/KOTLIN_SDK_DASHPAY_REGISTRATION_KEYS_SPEC.md b/docs/sdk/KOTLIN_SDK_DASHPAY_REGISTRATION_KEYS_SPEC.md deleted file mode 100644 index d499818579..0000000000 --- a/docs/sdk/KOTLIN_SDK_DASHPAY_REGISTRATION_KEYS_SPEC.md +++ /dev/null @@ -1,384 +0,0 @@ -# Kotlin SDK — DashPay registration-key provisioning spec - -**Status:** REVIEWED (2 rounds) — feasibility/security pass, then an -independent codex cross-check with a much deeper call-graph trace; several -material corrections applied. The resume-path funding question is resolved -by the parity mandate itself (§0.2); one small implementation-detail -decision remains (§0.3) -**Branch:** own PR/worktree, stacked on PR #3999's `feat/kotlin-sdk-and-example-app` -head (split out from the sibling Keystore/docs spec — different subsystem, -own rollback unit, per the repo's own PR-slicing convention at -`KOTLIN_SWIFT_SHARED_PARITY_SPEC.md:508`) -**Scope:** `packages/rs-unified-sdk-jni`, `packages/rs-platform-wallet-ffi`, -`packages/rs-platform-wallet`, `packages/kotlin-sdk`, -`packages/kotlin-sdk/KotlinExampleApp` - -## 0. Spec review findings - -### 0.1 Round 1 (feasibility + security) - -1. **Factual error, corrected: `decode_contract_bounds` is NOT usable from - the JNI decoder** (wrong crate, `pub(crate)`-scoped, operates on an - already-parsed struct not the raw blob). Real target: - `decode_update_pubkeys_blob` + `DecodedPubkeyRow` - (`rs-unified-sdk-jni/src/transactions.rs:291,305`) — **superseded by - §0.2 item 1 below**, which found promoting these alone is still - insufficient. -2. Confirmed: no protocol/FFI-struct change needed — `IdentityPubkeyFFI` - already carries the needed fields. -3. `IdentityUpdates.kt`'s `encodeAddPubkeys` (and its helper - `contractBoundsKind`) are `private` — need hoisting. -4. Security LOW: contract-bounds not validated at CREATE time — - **corrected by §0.2 item 3 below**, which found this claim was - overstated. -5. Security LOW: row-0-MASTER invariant path-dependent — - **corrected by §0.2 item 2 below**: it's a key-ID-0 invariant, not a - row-position invariant, and the fix location matters (must not touch - the shared update decoder). -6. Confirmed safe: security-level/purpose escalation not exploitable - server-side — **reconfirmed by round 2**, still holds. -7. Confirmed: no test references the functions being deleted — safe to - remove. - -### 0.2 Round 2 (independent codex cross-check — supersedes several round-1 items) - -A second, independent review re-derived every claim from source rather than -trusting the first draft, and found the design still has real gaps: - -1. **MAJOR — promoting the decoder alone doesn't make it usable.** - `DecodedPubkeyRow`'s fields are private - (`transactions.rs:291-301`) — making the struct and function - `pub(crate)` doesn't let `identity.rs`/`funding.rs` read the fields to - build `IdentityPubkeyFFI` rows. Need field accessors, a crate-private - `into_ffi`-style conversion helper, or move the row+decoder into a - shared module. Additionally, the current decoder takes `JNIEnv`/ - `JByteArray` directly (`transactions.rs:305-306`) — split it into a - pure `&[u8]` parser plus a thin JNI adapter, or the proposed Rust - round-trip unit tests can't be written as ordinary Rust tests. -2. **MAJOR — the invariant is "key ID 0", not "row 0", and must NOT go in - the shared update decoder.** `decode_update_pubkeys_blob` is also used - by `TransactionsNative.updateIdentity` (`transactions.rs:193-198`), and - an ordinary add-key update legitimately has no key ID 0 at all — adding - an unconditional key-0 check there would break normal key-addition - updates. Put the invariant in a registration-only seam instead: - `rs-platform-wallet-ffi::decode_identity_pubkeys` - (`identity_registration_with_signer.rs:273-323`) or a registration-only - JNI wrapper. **Also: `decode_identity_pubkeys` has a 5th caller the - original spec missed** — Swift's invitation-claim path - (`rs-platform-wallet-ffi/src/invitation.rs:244-300`). Moving the - key-0 check here affects that path too; it already has the same - downstream invariant separately - (`rs-platform-wallet/src/wallet/identity/network/invitation.rs:127-153`) - so the change should be behaviorally benign, but needs its own explicit - test, and the spec must stop saying "four paths." -3. **MAJOR — the CREATE-time-bounds-validation claim was overstated in the - wrong direction.** It's true there's no explicit - `validate_identity_public_keys_contract_bounds` call on the CREATE path - (that validator is UPDATE-only, - `rs-drive-abci/.../identity_update/state/v0/mod.rs:97-110`). But Drive's - actual key-insertion path (`create_key_tree_with_keys_operations` → - `add_potential_contract_info_for_contract_bounded_key`, - `rs-drive/src/drive/identity/key/insert/create_key_tree_with_keys/v0/mod.rs:98-123`) - **does** fetch the referenced contract, error if it doesn't exist, - resolve the document type, and require that type to declare the - matching bounded-key requirement - (`rs-drive/.../add_potential_contract_info_for_contract_bounded_key/v0/mod.rs:430-465`). - So "any wrong constant registers successfully" is **false** — a - nonexistent contract or missing document type fails registration. The - real residual risk is narrower: a wrong-but-*existing-and-compatible* - contract/doc-type would still be silently accepted under the wrong - scope. Test plan corrected accordingly (§4). - **Additional correction**: manual Add Contact success is not proof the - bounds are actually correct — `select_own_encryption_key` only checks - enabled/type/purpose, not bounds, and contact-request validation - intentionally accepts bound *or* unbound ENCRYPTION keys - (`rs-platform-wallet/src/wallet/identity/crypto/validation.rs:111-121, - 164-167`). Add an explicit on-chain bounds assertion to the test plan — - a passing Add Contact smoke does not cover this. -4. **MAJOR — my plan never specified how the BASE 4 keys' roles get - rebuilt after `role_for_registration_key_id` is deleted.** That function - is the *only* current place IDs 0-3 get their roles (MASTER/CRITICAL/ - HIGH/TRANSFER) assigned. The original approach only described stamping - the 2 new DashPay keys — it left the base 4 keys' rich-row construction - completely unspecified. Fixed in §2 approach below: the Kotlin side must - build rich rows for ALL 6 keys (0-5), not just the 2 new ones. -5. **MAJOR — "wire into all 4 funding paths" is not Swift parity and misses - a funding constraint.** Swift deliberately **excludes** the - unused-asset-lock resume path from DashPay provisioning - (`CreateIdentityView.swift:152-177,896-905`) and **raises the minimum - registration funding** based on the added key count (`:127-149`). Adding - 2 keys costs an additional 13,000 duffs at the current per-key fee - (`rs-platform-version/.../fee/state_transition_min_fees/v1.rs:17`) and - reaches exactly `max_public_keys_in_creation = 6` — the protocol - ceiling, not headroom under it. Kotlin's `TrackedAssetLock` doesn't even - expose the underlying lock's `amount` field - (`TrackedAssetLock.kt:11-19` vs. Rust's `tracked.rs:63-72`), so there's - currently no way to check whether a previously-created (four-key-sized) - lock has enough value for a six-key create. **Resolved, not an open - decision (see §0.2 resolution below): this is exactly the problem iOS's - own exclusion already solves — match it, don't re-derive a Kotlin- - specific answer.** -6. **Medium/high — `decode_identity_pubkeys` silently last-wins on - duplicate key IDs**, not fail-closed as claimed: it inserts into a - `BTreeMap` with no check for an existing entry - (`identity_registration_with_signer.rs:273-323`). The surviving key is - still structurally validated (not a privilege escalation), but explicit - duplicate-ID rejection should be added to the decoder. -7. **Medium/high — the proposed contract-ID pin test is impossible as - described.** A Rust test in `rs-unified-sdk-jni` cannot read a private - Kotlin source constant to compare against - (`AddIdentityKeyScreen.kt:382-397` vs. - `packages/dashpay-contract/src/lib.rs:9-16`) — there's no cross-language - visibility. Use a shared generated/checked-in golden fixture instead - (§4). -8. **New scope items the original spec didn't consider at all:** - - **Wire-format skew risk**: the JNI method descriptors stay identical, - but the opaque `byte[]` payload shape changes incompatibly. An old - Kotlin artifact paired with a new native `.so` (or vice versa) would - be silently misparsed, not rejected. At minimum, reject legacy-shaped - or trailing/malformed input deterministically; a version/magic byte - would make skew diagnosable if that's judged worth the extra - complexity. - - **Public Kotlin API compatibility**: `IdentityKeyPreview` is public - (`IdentityKeyPreview.kt:17-22`) and public registration methods accept - `List` (`IdentityRegistration.kt:117-146,237-291`, - `PlatformWalletManager.kt:1344-1371`). Replacing or extending this - type has source/binary compatibility implications for SDK consumers - — needs an explicit compatibility strategy, not treatment as an - app-only refactor. - - **No backfill for existing identities**: this only fixes freshly - registered identities. Existing Android-created identities remain - without the DashPay pair and still need the existing Add Identity Key - repair flow. State this explicitly as non-goal, or add detection/ - guidance if backfill is actually wanted. - - **Existing reusable plumbing**: `KotlinExampleApp` already has a - derive → validate → persist → zero → `IdentityPubkey` pipeline in - `IdentityKeyAdditionFlow.kt:84-182` (used by `AddIdentityKeyScreen.kt`). - Before writing a parallel `DashpayKeyProvisioning` helper, decide - whether a thin policy wrapper around this existing flow is sufficient - — duplicating the lifecycle is another drift surface. - - **Keypair-correspondence validation gap**: Swift explicitly verifies - the derived private scalar matches the public key before persisting - (`IdentityRegistrationKeys.swift:66-82`); the current Kotlin - registration-preview path doesn't do this check at all - (`CreateIdentityScreen.kt:202-235`). Pre-existing, not caused by this - change, but a "ported from Swift" claim should either add the same - defense or explicitly document the parity gap. - - **Secret lifecycle risk**: deriving `base + 2` while also describing - "the existing base list plus an appended pair" risks deriving the - first four private-key byte arrays twice with a discarded, unzeroed - copy. Since proof-of-possession signs every submitted key, losing a - private half after the corresponding pubkey is registered makes that - key permanently, silently unusable. Spec must be explicit: one - six-key preview derivation, not two overlapping ones, with every - preview/private array (including on failure paths) zeroed. -9. **Verification command was wrong.** `cargo test -p rs-platform-wallet-ffi - --lib` fails immediately — the crate is named `platform-wallet-ffi` - (`rs-platform-wallet-ffi/Cargo.toml:1-2`), confirmed by actually running - both commands (196 tests pass under the correct name). Corrected in §5. - -### 0.2 Resolved: resume-path policy - -**Not actually an open decision — resolved by the project's own parity -mandate, no Kotlin-specific design call needed.** iOS resume finishes an -interrupted registration with exactly the key set it originally started -with and never changes that set mid-resume; it does not attempt to -determine or verify whether the pre-existing lock is large enough for a -bigger key set, because it simply never asks the lock to cover more than -what it already committed to. That's iOS's own answer to the exact -fund-safety problem in §0.2 item 5 above (a resumed lock is a fixed, -already-spent-on-chain amount; retroactively growing the transition it -funds risks a resume that fails after the user's DASH is already -irreversibly locked). Since this whole PR exists to port Swift's behavior, -not invent new Kotlin policy, the answer is simply: **leave the resume path -alone.** Do not extend DashPay provisioning to -`resumeIdentityWithExistingAssetLock` at all — it keeps using the base -4-key set exactly as it does today. A user who resumes registration and -wants DashPay capability gets it afterward through the existing Add -Identity Key flow, same as iOS. This requires **zero new FFI surface** -(no `TrackedAssetLock.amount` exposure needed) and removes an entire -branch of scope from this PR rather than adding one. - -### 0.3 Remaining open decision - -- **`DashpayKeyProvisioning` vs. reusing `IdentityKeyAdditionFlow`** - (§0.2 item 8): recommend evaluating the existing flow first; only write - a new helper if it genuinely can't be adapted (e.g. its persistence - timing assumes post-registration, not pre-registration). - -## 1. Problem - -`role_for_registration_key_id` (`rs-unified-sdk-jni/src/identity.rs:122-131`) -reconstructs purpose/security-level *positionally* from a bare `keyId`, and -every registration call site hardcodes `contract_bounds_kind: 0`. The wire -format all 4 sites decode (`decode_pubkeys_blob`, `identity.rs:933`) carries -only `keyId` + pubkey bytes. Consequently, an identity created by the Android -app reaches its own advertised Add Contact flow without an enabled -ECDSA_SECP256K1 encryption key, and `select_own_encryption_key` rejects the -request (confirmed: `contact_requests.rs:455-465,979-1005`). - -Swift's reference flow (`IdentityRegistrationKeys.swift::makeDashpayKeyPair`) -derives 2 extra keys bounded to DashPay's `contactRequest` document type and -appends them before dispatching to registration — for eligible fresh-funding -paths only, explicitly excluding resume (§0.2 item 5). This is Swift -application-level code, not shared Rust. - -## 2. Approach - -1. **Rust — decoder.** Extract a pure `&[u8]` parser from - `decode_update_pubkeys_blob`'s logic (§0.2 item 1), with a - crate-private conversion path from the parsed rows to `IdentityPubkeyFFI` - that exposes what registration needs (not just `pub(crate)` on an - already-private-fielded struct). Keep the existing JNI-facing function as - a thin adapter over the pure parser for the update path; add a - registration-facing entry point that uses the same pure parser plus a - **registration-only** key-ID-0 = MASTER+AUTHENTICATION check and - explicit duplicate-key-ID rejection (§0.2 items 2, 6) — do NOT add - either check to the shared update-path decoder. Reject trailing/ - malformed bytes deterministically in the same pass (§0.2 item 8). -2. **Rust — call sites.** Call from all 4 registration JNI sites - (`identity.rs:609/753/868`, `funding.rs:734`) **and** account for the - 5th caller of `decode_identity_pubkeys`, the invitation-claim path - (`invitation.rs:244-300`) — add explicit test coverage there too rather - than silently changing its behavior (§0.2 item 2). Delete - `role_for_registration_key_id` and `decode_pubkeys_blob` (confirmed - dead — no test references) along with the now-dead local role constants - (`identity.rs:79-90`). -3. **Kotlin — base + DashPay rows, not just DashPay rows.** Build rich rows - for the **complete** 6-key set in one derivation pass — base 4 (IDs 0-3, - replicating today's MASTER/CRITICAL/HIGH/TRANSFER roles explicitly, since - `role_for_registration_key_id` no longer exists to do it implicitly) plus - the 2 DashPay keys (IDs 4-5, ENCRYPTION/DECRYPTION, MEDIUM, DashPay - `SingleContractDocumentType` bounds) — not two separate/overlapping - derivations (§0.2 item 8, secret lifecycle). Hoist `encodeAddPubkeys` + - `contractBoundsKind` out of `IdentityUpdates` into a shared location. - Hoist the DashPay contract-id constant out of `AddIdentityKeyScreen.kt` - into one shared location instead of a third copy. Evaluate reusing - `IdentityKeyAdditionFlow.kt`'s existing derive→validate→persist→zero - pipeline before writing a new parallel helper (§0.3 remaining decision). -4. **Kotlin — funding paths.** Wire the six-key set into fresh Core-funded, - Platform-address-funded, and shielded registration. **Exclude - unused-asset-lock resume** from DashPay provisioning entirely, matching - Swift (§0.2, resolved — not a design choice, just parity) — resume - continues to use the base 4-key set as today; a user recovering via - resume can add DashPay keys afterward through the existing Add Identity - Key flow. Document this explicitly as intentional, not an oversight. -5. **No backfill.** Existing already-registered identities are explicitly - out of scope (§0.2 item 8) — they keep using the existing Add Identity - Key repair flow, unchanged by this PR. -6. **Stale documentation.** Update positional-role wording that becomes - incorrect once `role_for_registration_key_id` is gone: - `identity.rs:258-270,682-697,813-815`, `funding.rs:650-660`, - `identity_key_preview.rs:77-94,257-280`, `IdentityNative.kt:148-178`, - `IdentityRegistration.kt:191-210`. - -## 3. Failure modes (as reviewed, 2 rounds) - -- Wire-format skew between an old Kotlin artifact and a new native library - (or vice versa) would silently misparse rather than fail loud — mitigated - by deterministic rejection of malformed/trailing/legacy-shaped input - (§0.2 item 8); a version/magic byte is a further option if judged - worthwhile. -- A wrong-but-*existing-and-compatible* DashPay contract/doc-type constant - would still register successfully under the wrong scope (narrower than - originally claimed — Drive DOES reject a nonexistent contract/doc-type, - §0.2 item 3) — mitigated by pinning the constant against a genuine - cross-language golden fixture (§4), not a same-literal echo. -- Duplicate key IDs in a caller-supplied row list silently collapse to - last-wins in `decode_identity_pubkeys`'s `BTreeMap` — mitigated by - explicit duplicate-ID rejection (§0.2 item 6). -- Key-ID-0 invariant, if misplaced in the shared update decoder, would - break ordinary key-addition updates that don't include ID 0 — mitigated - by keeping it registration-only (§0.2 item 2). -- A previously-created four-key-sized unused asset lock is excluded from - six-key resume entirely (§0.2, resolved by parity — resume never grows - past what it originally committed to), so no insufficient-funds failure - mode exists there by construction; fresh funding paths need the six-key - minimum reflected in their fee/sufficiency calculation. -- Security-level/purpose escalation via a malformed row list — confirmed - not exploitable; `validate_identity_public_keys_structure_v0` rejects the - whole transition server-side regardless of what Kotlin sends. -- Deriving the base 4 keys' private material twice (once for preview, once - overlapping with DashPay-key derivation) risks an unzeroed discarded - copy — mitigated by one single six-key derivation pass (§2 point 3). - -## 4. Test plan (red → green, per repo TDD discipline) - -- **Rust (red first), pure-parser round trip**: encode a row list including - the full 6-key policy (base 4 + bounded ENCRYPTION/DECRYPTION), decode via - the new pure `&[u8]` parser (§0.2 item 1 — required for this test to be - an ordinary Rust unit test, not JNI-environment-dependent), assert every - field round-trips. Write against the CURRENT decoder first to confirm no - such fields exist today. -- **Rust, cross-language golden fixture** (§0.2 item 7, replacing the - infeasible constant-pin approach): check in one fixture — 6 canonical - rows encoded to exact bytes. A Kotlin unit test asserts its encoder - produces those exact bytes; a Rust unit test asserts its parser decodes - those exact bytes to the expected fields. This catches byte-order/ - field-order skew that two independently-written, independently-passing - tests would miss. -- **Rust, strict codec edge cases**: both bounds kinds, truncation at each - variable-length field, invalid bounds-kind byte, interior NUL, negative - key ID, invalid boolean byte for `readOnly`, duplicate key ID (must - reject, not last-wins), trailing bytes (must reject), and explicit - legacy-format rejection. -- **Rust, registration-only invariant test**: key ID 0 must be - MASTER+AUTHENTICATION regardless of row order; missing/wrong ID 0 fails. - **Separately**, a regression test proving identity-UPDATE add-key lists - *without* key ID 0 remain accepted (proves the invariant didn't leak into - the shared decoder). -- **Rust, FFI decoder test**: duplicate IDs rejected (not silently - overwritten); ENCRYPTION/DECRYPTION without bounds rejected (existing - behavior, confirm still true after the refactor). If the key-0 check - lives in `decode_identity_pubkeys`, include a test for the invitation - caller's behavior too (§0.2 item 2). -- **Kotlin (red first), provisioning/lifecycle test**: assert exact IDs 0-5 - with correct type/purpose/securityLevel/readOnly/bounds for all 6 (not - just the 2 new ones), correct derivation indices, storage under matching - public bytes, and private-array zeroing on both success and failure - paths. Write against the stub/nonexistent helper first to confirm no such - rows are produced today. -- **Kotlin, four-way orchestration test**: extract a small pure - preparation/dispatch seam from `CreateIdentityScreen` and verify every - eligible funding source receives the identical rich 6-row set, and that - resume does NOT (per the resume-exclusion policy) — cheaper than - exercising all 4 JNI/JVM paths directly. -- **On-chain bounds assertion** (§0.2 item 3 — Add Contact success alone - does not prove this): after a local/testnet create, fetch the identity - and assert IDs 4/5 carry the exact DashPay contract ID and `contactRequest` - document-type bounds. -- **Resume-funding case**: test the chosen policy explicitly — resume uses - the base 4-key set only (recommended), or an amount-aware six-key - sufficiency check if policy (b) is chosen instead. -- **Manual/device (environment-bound)**: register a fresh identity via - KotlinExampleApp on testnet, confirm Add Contact now succeeds. This - remains useful as an end-to-end product check but is explicitly NOT - sufficient on its own (§0.2 item 3) — the on-chain bounds assertion above - is the actual correctness proof. -- Existing test needing mechanical update: `IdentityAssetLockRecoveryTest.kt:169` - constructs the legacy `IdentityKeyPreview` shape used by resume tests — - will need updating if the registration row type changes shape (even - though resume itself stays on the base 4-key policy). - -## 5. Verification plan - -```bash -cargo test -p rs-unified-sdk-jni --lib -cargo test -p platform-wallet-ffi --lib # NOT rs-platform-wallet-ffi — that name doesn't exist (§0.2 item 9) -cargo clippy --workspace --all-features -cargo fmt --check --all - -cd packages/kotlin-sdk -JAVA_HOME=/opt/homebrew/opt/openjdk@17 ./gradlew \ - :sdk:assembleDebug :sdk:testDebugUnitTest \ - :app:assembleDebug :app:testDebugUnitTest \ - :sdk:compileDebugAndroidTestKotlin -``` - -Baseline (pre-change) confirmed by the round-2 review: -`cargo test -p rs-unified-sdk-jni --lib` → 10 passed; -`cargo test -p platform-wallet-ffi --lib` → 196 passed. - -Manual/device-bound: testnet identity registration + Add Contact flow smoke, -**plus** the on-chain bounds assertion above (Add Contact success alone is -not sufficient evidence per §0.2 item 3).