Skip to content

feat(kotlin-sdk): topUpExternalIdentityFromAddresses - #5906

Draft
HashEngineering wants to merge 2 commits into
feat/platform-wallet-external-identity-top-upfrom
feat/kotlin-sdk-external-identity-top-up
Draft

HashEngineering wants to merge 2 commits into
feat/platform-wallet-external-identity-top-upfrom
feat/kotlin-sdk-external-identity-top-up

Conversation

@HashEngineering

@HashEngineering HashEngineering commented Nov 13, 2026 •

Copy link
Copy Markdown
Collaborator

Issue being fixed or feature implemented

Kotlin binding for #5860's external identity top-up: top up an identity the wallet does not manage (someone else's) from the wallet's Platform-address credits. #5860 added the Rust composite, the FFI export and the Swift wrapper, and recorded Kotlin as unsupported in the parity manifest.

Android needs it for dash-wallet's Advanced-mode External Send (dashpay/dash-wallet#1680): its Platform → identity send currently calls the managed topUpFromAddresses, which refuses an identity the wallet does not manage with IdentityNotFound.

Stacked on #5860. The base is #5860's branch; this PR will be retargeted to v5.1-dev once #5860 merges.

What was done?

  • JNI (rs-unified-sdk-jni/src/credits.rs): CreditsNative.topUpExternalIdentityFromAddresses, a thin marshaler over platform_wallet_top_up_external_identity_from_addresses_with_signer. It reads the same inputs blob as topUpFromAddresses. Unlike that export, it does not refuse empty inputs itself: the library owns that rule, and an empty Vec's pointer is non-null, so it passes the FFI's pointer check and reaches the library.
  • Kotlin: the CreditsNative declaration and IdentityCredits.topUpExternalIdentityFromAddresses(walletHandle, identityId, inputs, signerHandle), gated and routed through mapNativeErrors like topUpFromAddresses. An identity that does not exist on Platform surfaces as DashSdkError.NotFound (platform-wallet code 7) before anything is signed. The native call is injectable (AddressTopUpNativeCall) for JVM tests.
  • Parity: identity.external_top_up_from_addresses is now supported for the Kotlin SDK (example app still unsupported: it has no screen that pays someone else's identity, as on Swift). PARITY_SUMMARY.md regenerated.

No Rust library or FFI changes, no new error codes.

How Has This Been Tested?

  • ExternalIdentityTopUpTest (JVM, 3 tests): the handles, id and encoded inputs reach the export and its balance is returned; platform-wallet code 7 maps to NotFound; empty inputs are passed to Rust, not refused in Kotlin.
  • FfiSmokeTest.externalIdentityTopUpSymbolBindsAndRejectsInvalidInput (instrumented): calls the real export with a 31-byte id and expects the JNI argument check's code 1, proving the symbol and descriptor bind. FfiSmokeTest passes 8/8 on an x86_64 Pixel 9 Pro emulator (API 36) against a dev-profile build_android.sh --abi x86_64 library.
  • ./gradlew :sdk:testDebugUnitTest --tests 'org.dashfoundation.dashsdk.credits.*': 5/5 pass.
  • cargo clippy -p rs-unified-sdk-jni --all-targets -- -D warnings and cargo fmt --check: clean.
  • python3 scripts/check_sdk_parity_manifest.py --write-summary: OK.
  • Local pre-review (GPT-6.1 with the thepastaclaw prompts) over this range: 0 blocking; its one suggestion (cover the real JNI boundary) is the smoke test above.

Breaking Changes

None. Adds a method; existing APIs are unchanged.

Checklist:

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

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

🤖 Generated with Claude Code

HashEngineering and others added 2 commits November 12, 2026 21:05
Bridge platform_wallet_top_up_external_identity_from_addresses_with_signer
to Kotlin: a JNI export in rs-unified-sdk-jni, CreditsNative, and
IdentityCredits.topUpExternalIdentityFromAddresses. It tops up an identity
the wallet does not manage from the wallet's Platform-address inputs; Rust
fetches the identity from Platform and never adds it to the wallet.

The JNI layer does not refuse empty inputs: the library owns that rule.
An unknown identity maps to DashSdkError.NotFound.

Parity: identity.external_top_up_from_addresses is now supported for the
Kotlin SDK, verified by ExternalIdentityTopUpTest.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The JVM tests inject the native call, so a wrong symbol name or
descriptor would only fail on device. FfiSmokeTest now calls the real
CreditsNative.topUpExternalIdentityFromAddresses with a 31-byte id and
expects the JNI argument check's code 1.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Nov 13, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

@thepastaclaw

thepastaclaw commented Nov 13, 2026 •

Copy link
Copy Markdown
Collaborator

✅ Final review complete — no blockers (commit 9111d54) · triage: normal · Phase 2 only (queue backlog)

@HashEngineering HashEngineering self-assigned this Nov 13, 2026

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Final validation — Phase 2 only (queue backlog)

The Kotlin/JNI external identity top-up path is correctly layered and preserves Rust ownership of validation, signing, lifecycle handling, and balance reconciliation. One input-marshalling bug remains: invalid Kotlin address types can be silently truncated into valid types before reaching Rust validation.

🟡 1 suggestion(s)

Approved: nothing here blocks the merge. The non-blocking findings are optional: fix them forward, in this PR or a follow-up, without waiting for another review round.

1 finding(s) not shown inline (the lines are not part of this PR's diff)

🟡 Suggestion: Reject invalid address types before encoding funding inputs
packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/credits/IdentityCredits.kt:69

FundingInput.addressType is an unconstrained Int, but DataOutputStream.writeByte retains only the low eight bits. An input with addressType = 256 is therefore encoded as 0 and decoded by JNI as P2PKH; addressType = 257 becomes 1 and is decoded as P2SH. The Rust funding_rows validator cannot recover the original value, so the external top-up can sign and spend against a different address type than the Kotlin caller supplied. Validate that addressType is exactly 0 or 1 before encoding, preferably in FundingInput.init so every use of this wire model is protected.

init {
    require(addressType == 0 || addressType == 1) {
        "FundingInput.addressType must be 0 (P2PKH) or 1 (P2SH), got $addressType"
    }
    require(hash.size == 20) { "FundingInput.hash must be 20 bytes, got ${hash.size}" }
    require(credits > 0) { "FundingInput.credits must be positive, got $credits" }
}

source: gpt-6.1-sol (phase2-reviewer: general, architecture-layering, ffi-engineer, security-auditor)

Review provenance

Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

  • Triage: normal by gpt-6.1-sol (effort low) — This is a contained Kotlin-to-JNI feature addition that crosses an FFI boundary and initiates an external identity top-up, but it does not change the Rust funds-handling logic itself or meet the critical-tier bar for a large intricate change to that critical surface.
  • Phase 1 reviewers: not run (skipped for throughput: 19 PRs queued, above the 10 limit)
  • Second Phase-2 pass: on this first full review every Phase-2 reviewer also ran a second, independent time, and the verifier weighed both sets
  • Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
  • Phase 2 reviewers: gpt-6.1-sol — general (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — ffi-engineer (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — ffi-engineer (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort high); agent phase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review (PastaClaw). How to work through them:
- Verify each finding against the code; fix only what is real, as narrowly as the finding.
- If a finding is wrong, decline it on its thread in one or two sentences; the bot answers once and stops.
- To defer one, link a tracking issue on its thread (or reply asking the bot to file one).
- Suggestions don't block the merge, but most are worth fixing in this PR; judge each on its merits.
- Keep the PR's scope; don't grow it to satisfy hardening suggestions.
- Carried items were raised in an earlier round: read the thread first, and don't re-fix what you already fixed, declined or deferred there. Disputed and deferred items are already handled and are not listed.

In `packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/credits/IdentityCredits.kt`:
- [SUGGESTION] packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/credits/IdentityCredits.kt:69: Reject invalid address types before encoding funding inputs
  `FundingInput.addressType` is an unconstrained `Int`, but `DataOutputStream.writeByte` retains only the low eight bits. An input with `addressType = 256` is therefore encoded as `0` and decoded by JNI as P2PKH; `addressType = 257` becomes `1` and is decoded as P2SH. The Rust `funding_rows` validator cannot recover the original value, so the external top-up can sign and spend against a different address type than the Kotlin caller supplied. Validate that `addressType` is exactly `0` or `1` before encoding, preferably in `FundingInput.init` so every use of this wire model is protected.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants