Skip to content

feat(kotlin-sdk): transaction decoder JNI binding over key-wallet-ffi transaction_decode#4187

Merged
shumkov merged 1 commit into
dashpay:v4.1-devfrom
bfoss765:port/v4.1/tx-decode-binding
Jul 21, 2026
Merged

feat(kotlin-sdk): transaction decoder JNI binding over key-wallet-ffi transaction_decode#4187
shumkov merged 1 commit into
dashpay:v4.1-devfrom
bfoss765:port/v4.1/tx-decode-binding

Conversation

@bfoss765

Copy link
Copy Markdown
Contributor

Adds the Android/Kotlin counterpart of the Swift TransactionDecoder (#3981): a single JNI export (TxDecodeNative.decodeTransaction) over key-wallet-ffi's transaction_decode — already present in v4.1-dev's rust-dashcore pin via upstream rust-dashcore#870, so no vendored Rust changes are needed. The JNI layer marshals the decode result into a packed big-endian BLOB parsed by the new org.dashfoundation.dashsdk.keywallet.TransactionDecoder into DecodedTransaction (txid + per-input prev outpoints / unauthenticated P2PKH sender hints + per-output address/value/script).

The JNI-free decode_to_blob core is host-tested (7 Rust tests), including a fixture blob pinned byte-for-byte against Kotlin's TransactionDecoderTest (4 tests) as a cross-language layout pin.

Verified: cargo test -p rs-unified-sdk-jni (17 tests) and :sdk:assembleRelease + sdk unit tests all pass against the pin's transaction_decode signature (no API drift).

🤖 Generated with Claude Code

… transaction_decode

Android port of the Swift TransactionDecoder (PR dashpay#3981): one new JNI
export (TxDecodeNative.decodeTransaction) marshaling key-wallet-ffi's
transaction_decode result into a packed big-endian BLOB, parsed by the
new org.dashfoundation.dashsdk.keywallet.TransactionDecoder into
DecodedTransaction (txid + per-input prev outpoints / unauthenticated
P2PKH sender hints + per-output address/value/script).

- rs-unified-sdk-jni/src/tx_decode.rs: thin single-entry-point marshaler
  (CLAUDE.md boundary rule); JNI-free decode_to_blob core is host-tested,
  incl. a fixture blob hex pinned byte-for-byte from Kotlin's
  TransactionDecoderTest (cross-language layout pin).
- Binds the transaction_decode already present in v4.1-dev's
  rust-dashcore pin (upstream rust-dashcore#870) — no vendored Rust
  changes needed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@bfoss765, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 22 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f72b4343-d79b-4388-9dc1-e10df11be763

📥 Commits

Reviewing files that changed from the base of the PR and between 8b466ab and 40aa94f.

📒 Files selected for processing (5)
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/TxDecodeNative.kt
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/keywallet/TransactionDecoder.kt
  • packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/keywallet/TransactionDecoderTest.kt
  • packages/rs-unified-sdk-jni/src/lib.rs
  • packages/rs-unified-sdk-jni/src/tx_decode.rs
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

❤️ Share

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

@github-actions github-actions Bot added this to the v4.1.0 milestone Jul 21, 2026
@thepastaclaw

thepastaclaw commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

✅ Final review complete — no blockers (commit 40aa94f)

@shumkov
shumkov merged commit 1bb7d30 into dashpay:v4.1-dev Jul 21, 2026
17 of 20 checks passed
@shumkov

shumkov commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Reviewed against v4.1-dev with focus on FFI fidelity and Swift parity. This is a faithful counterpart of the Swift TransactionDecoder (#3981): same key-wallet-ffi transaction_decode at the same rust-dashcore pin, exact field parity, and the cross-pinned fixture blob (Rust + Kotlin) is a nice touch. Mergeable as-is. Minor, non-blocking:

  • rs-unified-sdk-jni/src/tx_decode.rs:110-116 — script encoding is half-defensive: length is always written but bytes only if ptr non-null; make the null branch write length 0 so the blob can't shift.
  • net_from_ord is now the third private copy (also transactions.rs, identity.rs) — worth hoisting into support.rs.
  • Follow-up idea (upstream, not this PR): transaction_decode exposes no version/locktime/special-tx payloads, so neither platform can render ProRegTx/AssetLock details — a key-wallet-ffi enhancement both SDKs would pick up together.

@shumkov

shumkov commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Post-merge follow-up only; I found no blocker in the decoder architecture.

Kotlin exposes prevVout as signed Int, so the unsigned coinbase sentinel 0xFFFFFFFF appears as -1, while the Swift wrapper uses UInt32. Consider exposing UInt/Long, or explicitly document that Kotlin preserves the raw u32 bits in a signed value. The current test intentionally pins -1, so this is a host-parity/API clarity issue rather than a decoding bug.

@bfoss765

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Full review finished.


Your plan includes PR reviews subject to rate limits. More reviews will be available in 22 minutes.

@bfoss765

Copy link
Copy Markdown
Contributor Author

Thanks for the review and approval. All three notes are implemented in a small follow-up PR (opening shortly): the script-encoder null branch now writes length 0 (plus the same guard one level up for the input/output arrays, with lying-fixture tests), net_from_ord is hoisted into support.rs (we found and removed a fourth copy in persistence.rs), and prevVout now documents the raw-u32-bits contract with a prevVoutUnsigned convenience accessor — the pinned -1 test is untouched.

Separately: could you hit "Re-run failed jobs" on the x86_64 emulator check on #4182 and #4183? As you noted, the failure predates the diffs — it's the known emulator-boot flake, and we don't have rerun rights.

shumkov pushed a commit that referenced this pull request Jul 22, 2026
… hoist, prevVout docs (#4187 review) (#4192)

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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.

3 participants