feat(kotlin-sdk)!: provision DashPay registration keys#4173
Merged
Conversation
…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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
…(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<IdentityPubkey> 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 <noreply@anthropic.com>
…on (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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
shumkov
requested review from
QuantumExplorer,
ZocoLini,
lklimek and
llbartekll
as code owners
July 21, 2026 01:06
Contributor
📝 WalkthroughWalkthroughAdds a shared rich public-key wire format across Kotlin and Rust, registration-only validation, canonical four- and six-key DashPay provisioning, fee-aware funding logic, private-key cleanup safeguards, and extensive tests and review documentation. ChangesDashPay registration key flow
Estimated code review effort: 4 (Complex) | ~60 minutes Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant App
participant RegistrationKeys
participant IdentityPubkeyCodec
participant JNI
participant Platform
App->>RegistrationKeys: derive and build registration rows
RegistrationKeys->>IdentityPubkeyCodec: encode rich rows
IdentityPubkeyCodec->>JNI: send registration blob
JNI->>JNI: parse and validate rows
JNI->>Platform: submit IdentityPubkeyFFI rows
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Collaborator
|
🕓 Ready for review — 48 ahead in queue (commit 2aa999c) |
thepastaclaw
suggested changes
Jul 21, 2026
thepastaclaw
left a comment
thepastaclaw
left a comment
Collaborator
There was a problem hiding this comment.
Preliminary review — Codex only
All three Codex correctness concerns are confirmed at the exact HEAD, with one clarification: Platform-address fees come from the post-spend remainder of fee-strategy input 0, not from the requested identity amount itself. Mutable submission state can diverge from the prepared keys, resume lost its HD-slot invariant, and six-key address funding is packed without reserving its increased fee. These are in-scope blocking regressions.
Validated blockers were found in the Codex precheck. Sonnet is deferred until a fresh Codex revalidation clears the blocker gate.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (failed),gpt-5.6-sol— general (completed),gpt-5.6-sol— security-auditor (failed),gpt-5.6-sol— security-auditor (completed),gpt-5.6-sol— ffi-engineer (failed),gpt-5.6-sol— ffi-engineer (completed) - Verifier:
gpt-5.6-sol— verifier - Sonnet: not run (deferred by blocker gate)
🔴 3 blocking
2 additional finding(s) omitted (not in diff).
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/identity/CreateIdentityScreen.kt`:
- [BLOCKING] packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/identity/CreateIdentityScreen.kt:215-251: Freeze the registration inputs before preparing keys
The new source-dependent key count is captured in `registerDashPayKeys`, but after the suspending preview and persistence operations the dispatch reads the current mutable `fundingSource` and `selectedRecoveryLock`. The funding, wallet, lock, amount, and identity-index controls remain interactive while only the submit button is disabled. Switching from fresh funding to resume can therefore derive and persist six keys for the original slot, then submit them against the current four-key recovery lock and possibly a different registration index. Changing between fresh funding sources also changes the funding mechanism and amount units after validation, while switching away from resume leaves four persisted keys before the captured null amount fails. Capture one immutable submission snapshot and use it for preparation, dispatch, coordinator tracking, and navigation, or disable every relevant control until completion.
- [BLOCKING] packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/identity/CreateIdentityScreen.kt:277-281: Reserve the six-key fee when packing Platform-address inputs
Fresh registration now creates six keys, raising the one-input minimum fee from 28,500,000 to 41,500,000 credits (`2,000,000 + 6,500,000 × keyCount + 500,000 × inputCount`). Drive pays this fee from the remaining address balances after the requested input amounts are consumed, and the SDK's fixed strategy uses only `DeductFromInput(0)`. `packFundingInputs` currently selects amounts totaling `requiredAmount` without ensuring that the BTreeMap-first included address retains the calculated fee. For example, spending 30,000,000 from a 70,000,000-credit address leaves 40,000,000: enough for the previous four-key fee but not the new six-key fee, so this PR turns a previously valid registration into an on-chain rejection after all keys have been persisted. Before provisioning keys, pack and validate using the selected addresses' post-spend balances, the actual input count, the active key count, and the address that will occupy fee-strategy input 0; simply imposing the fee as a minimum requested identity amount would reject valid funding layouts.
In `packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityRegistration.kt`:
- [BLOCKING] packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityRegistration.kt:129-151: Resume no longer verifies that keys belong to the tracked HD slot
Before this PR, resume accepted `IdentityKeyPreview` rows and rejected any key whose `identityIndex` differed from `lock.registrationIndex`. The replacement `IdentityPubkey` rows contain no derivation index, so that invariant was deleted. JNI and native registration accept the public keys and supplied identity index independently; signing succeeds because the Keystore signer looks up private keys by public-key bytes. Registration then records `ManagedIdentity` and each derivation breadcrumb under the supplied lock index even when the keys came from another slot. Subsequent seed-based loading derives the master key at that recorded index and cannot reproduce the registered identity. Preserve the derivation index alongside the rich rows in a registration-key-set wrapper and restore the equality check before JNI.
shumkov
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityKeyPreview.kt (1)
43-49: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the new destructive
blobside effect.
decodeAllnow always zeroes the inputblobinfinally(line 96), on both success and failure, but the KDoc still only describes the wire layout — it doesn't warn callers that the array they pass in gets wiped and must not be reused afterward.📝 Suggested doc update
/** * Decode the flat BLOB produced by * `IdentityNative.previewRegistrationKeys`. Layout (big-endian): * `u32 rowCount` then per row `u32 identityIndex, u16 pathLen, * pathUtf8, u8[33] pubkey, u8[32] privkey`. + * + * Destructively wipes [blob] to zero in a `finally` block on both + * success and failure, since it carries raw private-key scalars. + * Callers must not retain or reuse the array passed in. */🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityKeyPreview.kt` around lines 43 - 49, Update the KDoc for IdentityKeyPreview.decodeAll to explicitly warn that the input blob is zeroed on both successful and failed decoding and must not be reused afterward, while preserving the existing wire-layout documentation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityKeyPreview.kt`:
- Around line 43-49: Update the KDoc for IdentityKeyPreview.decodeAll to
explicitly warn that the input blob is zeroed on both successful and failed
decoding and must not be reused afterward, while preserving the existing
wire-layout documentation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 733df335-25ef-46e3-81cf-84d982619baa
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockpackages/kotlin-sdk/sdk/src/test/resources/golden/registration_pubkeys_v1.binis excluded by!**/*.bin
📒 Files selected for processing (32)
docs/sdk/CODEX_REVIEW_NOTES.mddocs/sdk/CODE_REVIEW_NOTES.mddocs/sdk/KOTLIN_SDK_DASHPAY_REGISTRATION_KEYS_SPEC.mdpackages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/services/DashpayKeyProvisioning.ktpackages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/services/IdentityKeyAdditionFlow.ktpackages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/services/IdentityKeyPersistence.ktpackages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/identity/AddIdentityKeyScreen.ktpackages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/identity/CreateIdentityScreen.ktpackages/kotlin-sdk/KotlinExampleApp/app/src/test/java/org/dashfoundation/example/services/DashpayKeyProvisioningTest.ktpackages/kotlin-sdk/KotlinExampleApp/app/src/test/java/org/dashfoundation/example/ui/identity/CreateIdentityFundingPolicyTest.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/IdentityNative.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityKeyPreview.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityPubkeyCodec.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityRegistration.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityUpdates.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/RegistrationKeys.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/PlatformWalletManager.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/TeardownGate.ktpackages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/identity/IdentityAssetLockRecoveryTest.ktpackages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/identity/IdentityKeyPreviewTest.ktpackages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/identity/RegistrationKeysTest.ktpackages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/wallet/GateCoverageLintTest.ktpackages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/wallet/TeardownGateTest.ktpackages/rs-platform-wallet-ffi/src/identity_key_preview.rspackages/rs-platform-wallet-ffi/src/identity_registration_with_signer.rspackages/rs-platform-wallet-ffi/src/invitation.rspackages/rs-unified-sdk-jni/Cargo.tomlpackages/rs-unified-sdk-jni/src/funding.rspackages/rs-unified-sdk-jni/src/identity.rspackages/rs-unified-sdk-jni/src/lib.rspackages/rs-unified-sdk-jni/src/pubkey_rows.rspackages/rs-unified-sdk-jni/src/transactions.rs
Collaborator
Author
|
Review follow-up at 2aa999c:
Local validation after the behavioral fixes passed the full SDK/app Gradle gate. The documentation follow-up passed focused SDK Kotlin compilation and git diff --check. CI is running again at the new HEAD. Testnet/on-chain and physical-device Add Contact verification remain explicitly not performed. |
Collaborator
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
Collaborator
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
Collaborator
Author
|
@coderabbitai rate limit |
Contributor
|
Your plan includes PR reviews subject to rate limits. More reviews will be available in 6 minutes. |
Collaborator
Author
|
@coderabbitai rate limit |
Contributor
|
Your plan includes PR reviews subject to rate limits. Reviews are available now. |
Collaborator
Author
|
@coderabbitai full review |
Contributor
✅ Action performedFull review finished. |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityPubkeyCodec.kt (1)
59-69: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winValidate
contractIdlength before writing the fixed-width wire field.The wire format documents
contractBoundsIdas a fixedu8[32]field with no length prefix, and the Rust decoder reads exactly 32 bytes unconditionally. Ifbounds.contractIdis ever not exactly 32 bytes,encode()silently misaligns every byte after it in the blob rather than failing fast — unlikepubkeyBytes/documentTypeName, which are explicitly size-checked here. Please confirm whetherContractBoundsalready enforces this invariant at construction; if not, add a local check.🛡️ Proposed defensive check
when (val bounds = k.contractBounds) { null -> Unit - is ContractBounds.SingleContract -> dos.write(bounds.contractId) + is ContractBounds.SingleContract -> { + require(bounds.contractId.size == 32) { + "contractId must be 32 bytes, got ${bounds.contractId.size}" + } + dos.write(bounds.contractId) + } is ContractBounds.SingleContractDocumentType -> { + require(bounds.contractId.size == 32) { + "contractId must be 32 bytes, got ${bounds.contractId.size}" + } dos.write(bounds.contractId) val dt = bounds.documentTypeName.toByteArray(Charsets.UTF_8)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityPubkeyCodec.kt` around lines 59 - 69, Validate that bounds.contractId is exactly 32 bytes before writing it in both ContractBounds.SingleContract and ContractBounds.SingleContractDocumentType branches of the IdentityPubkeyCodec encoding flow. If ContractBounds does not already enforce this invariant at construction, add a local require that fails before dos.write; preserve the existing documentTypeName validation and encoding behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityPubkeyCodec.kt`:
- Around line 59-69: Validate that bounds.contractId is exactly 32 bytes before
writing it in both ContractBounds.SingleContract and
ContractBounds.SingleContractDocumentType branches of the IdentityPubkeyCodec
encoding flow. If ContractBounds does not already enforce this invariant at
construction, add a local require that fails before dos.write; preserve the
existing documentTypeName validation and encoding behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 07294730-5426-4d41-8d3c-941308b900a2
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockpackages/kotlin-sdk/sdk/src/test/resources/golden/registration_pubkeys_v1.binis excluded by!**/*.bin
📒 Files selected for processing (32)
docs/sdk/CODEX_REVIEW_NOTES.mddocs/sdk/CODE_REVIEW_NOTES.mddocs/sdk/KOTLIN_SDK_DASHPAY_REGISTRATION_KEYS_SPEC.mdpackages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/services/DashpayKeyProvisioning.ktpackages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/services/IdentityKeyAdditionFlow.ktpackages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/services/IdentityKeyPersistence.ktpackages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/identity/AddIdentityKeyScreen.ktpackages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/identity/CreateIdentityScreen.ktpackages/kotlin-sdk/KotlinExampleApp/app/src/test/java/org/dashfoundation/example/services/DashpayKeyProvisioningTest.ktpackages/kotlin-sdk/KotlinExampleApp/app/src/test/java/org/dashfoundation/example/ui/identity/CreateIdentityFundingPolicyTest.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/IdentityNative.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityKeyPreview.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityPubkeyCodec.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityRegistration.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityUpdates.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/RegistrationKeys.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/PlatformWalletManager.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/TeardownGate.ktpackages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/identity/IdentityAssetLockRecoveryTest.ktpackages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/identity/IdentityKeyPreviewTest.ktpackages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/identity/RegistrationKeysTest.ktpackages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/wallet/GateCoverageLintTest.ktpackages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/wallet/TeardownGateTest.ktpackages/rs-platform-wallet-ffi/src/identity_key_preview.rspackages/rs-platform-wallet-ffi/src/identity_registration_with_signer.rspackages/rs-platform-wallet-ffi/src/invitation.rspackages/rs-unified-sdk-jni/Cargo.tomlpackages/rs-unified-sdk-jni/src/funding.rspackages/rs-unified-sdk-jni/src/identity.rspackages/rs-unified-sdk-jni/src/lib.rspackages/rs-unified-sdk-jni/src/pubkey_rows.rspackages/rs-unified-sdk-jni/src/transactions.rs
Collaborator
Author
|
CodeRabbit full-review follow-up: the contractId-width nitpick is already enforced and does not require a codec change. Both ContractBounds.SingleContract and ContractBounds.SingleContractDocumentType have init-time require checks for contractId.size == 32 in IdentityUpdates.kt. A ByteArray can have mutable contents but its length cannot change after construction, so IdentityPubkeyCodec cannot receive either bounds subtype with a non-32-byte field. Keeping the invariant at the shared model boundary also covers every encoder/persistence caller. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v4.1-dev #4173 +/- ##
============================================
+ Coverage 87.43% 87.63% +0.19%
============================================
Files 2646 2624 -22
Lines 333982 331678 -2304
============================================
- Hits 292024 290659 -1365
+ Misses 41958 41019 -939
🚀 New features to boost your workflow:
|
…tch 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 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue being fixed or feature implemented
Fresh identities created by the Kotlin example app did not receive the DashPay
encryption/decryption key pair, so they could not use the Add Contact flow
immediately after registration. The former registration wire format also could
not carry the contract/document bounds required by those keys.
What was done?
rows shared by registration and add-key updates.
duplicate key IDs in all decoded public-key lists, including invitation claims.
keys plus DashPay ENCRYPTION/DECRYPTION keys 4 and 5 at MEDIUM security,
bounded to the DashPay
contactRequestdocument type.Platform-address-funded, and shielded registrations; preserve the four-key
set for asset-lock resume.
set whose slot does not match the tracked asset lock before JNI.
first suspension so interactive form changes cannot redirect an in-flight
registration.
BTreeMap input selected by
DeductFromInput(0), before deriving or persistingany keys.
clean up secret material on success, failure, malformed decode, and coroutine
cancellation.
policy, lifecycle, funding, and invitation-path regressions.
Existing identities are not backfilled by this change; they continue to use the
Add Identity Key flow. Asset-lock resume intentionally remains excluded from
DashPay-key provisioning to match the iOS behavior.
How Has This Been Tested?
The following local verification passed:
cargo test -p rs-unified-sdk-jni --lib(29 passed)cargo test -p platform-wallet-ffi --lib(201 passed)cargo clippy --workspace --all-features(passed with the locally configuredcompiler cache wrapper disabled)
cargo fmt --check --allJAVA_HOME=/opt/homebrew/opt/openjdk@17 ANDROID_HOME=/opt/homebrew/share/android-commandlinetools ./gradlew :sdk:assembleDebug :sdk:testDebugUnitTest :app:assembleDebug :app:testDebugUnitTest :sdk:compileDebugAndroidTestKotlinThe PR-review regressions were observed failing before their fixes and passing
afterward: a 70M-credit address attempting a 30M-credit six-key registration,
a resume key set derived from a different HD slot, and mutation of form/lock
state after the immutable submission snapshot was captured.
Testnet/device registration, fetching the resulting identity to assert the
on-chain key bounds, and the Add Contact smoke test were not performed.
Those remain environment-bound manual verification; Add Contact success alone
would not prove that the expected bounds were persisted.
Breaking Changes
Public Kotlin fresh-registration entry points now consume rich
IdentityPubkeylists instead of
IdentityKeyPreviewlists, asset-lock resume consumes aprovenance-carrying
RegistrationKeySet, and the opaque Kotlin/nativeregistration payload changed shape. Kotlin and native artifacts containing
this change must be released together; legacy or malformed payloads fail
closed.
Checklist:
!to the type of the title to indicate a breaking changeFor repo code owners:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation