feat(kotlin-sdk): split build/broadcast with reservation release for BIP70-style deferred submission

[bfoss765]

Splits transaction build from broadcast for BIP70-style deferred submission: a signed-payment registry with reservation release, a deferred-payment token bounded to the reservation's lifetime, native code 27 for stale reservation tokens, token sweeping only when the final wallet write wins, and routing of deferred builds through the atomic finalize-and-register path — across rs-platform-wallet, platform-wallet-ffi, rs-unified-sdk-jni, and the Kotlin SDK surface.

Re-opens #4090 which was auto-closed when the #3999 base branch was deleted; rebased onto v4.1-dev. All seven original commits replayed cleanly — no hunks needed to be dropped as already-absorbed.

Verified: cargo test -p platform-wallet -p platform-wallet-ffi -p rs-unified-sdk-jni all pass (504 / 229 / 10); :sdk:assembleRelease + sdk unit tests pass.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features
  • Added support for deferred signed payments, allowing transactions to be built and signed, then broadcast or released later.
  • Added payment details including transaction ID, raw transaction data, fees, and reservation tokens.
  • Added idempotent reservation release for abandoned payments.
  • Added typed reservation and wallet-mismatch errors across supported SDKs.
• Bug Fixes
  • Improved cleanup when wallets are closed while deferred payments remain active.
  • Prevented reservations from being used with the wrong wallet or after expiration.

---

Why the token registry instead of the V2 handle surface

The deferred BIP70/BIP270 flow uses the reservation-token registry rather than the V2 finalized-transaction handle for two concrete reasons. First, ownership and cleanup: the token is wrapped in an owning, AutoCloseable Kotlin object with a GC/Cleaner backstop, so a payment that is signed but then abandoned — the merchant server never acks, the user backs out, or the coroutine is cancelled after the native registration returned — always releases its funding reservation, for free, without the caller having to remember to abandon a handle. Second, the token path carries a lifetime bound the V2 handle does not: it stamps each token with the reservation's own pre-signing height and refuses to act once that reservation could have aged into key-wallet's TTL sweep, so a slow external signer can never let a stale token spend against an outpoint the wallet already swept and re-selected. A pinned V2 CoreWallet handle has no such age guard and would keep the old wallet actionable indefinitely. Both paths now share one wallet-generation identity and one teardown policy, so the V2 surface stays correct for the immediate send it was built for while the deferred flow gets the GC-safe, age-bounded ownership it needs. A follow-up adds the age guard to the V2 handle path itself (it becomes live the moment iOS does deferred sends).

Review-response summary (2026-07-21)

All five lifetime findings addressed as merge blockers, one commit each, with regression tests:

1. Destroy vs teardown: final-alias platform_wallet_destroy releases the generation's reservations against the still-live wallet; actual generation removal drops tokens and V2 handles — token cleanup is now tied to wallet-generation removal.
2. Height carry: the pre-signing reservation height travels on SignedCoreTransaction and register uses it — no post-signing resample; boundary test pins the TTL margin.
3. One generation identity: CoreWallet::is_same_generation (per-generation identity) is checked by BOTH the V2-handle and registry-token paths, with one teardown policy.
4. Cancellation-safe ownership: SignedCoreTransaction is an owning AutoCloseable with a NativeCleaner backstop; round-2 adds object-owning broadcastSigned/releaseReservation overloads that hold the object reachable across the native call and disarm the backstop on consumption (the bare-token forms remain but document the reachability requirement).
5. Validate-under-lock: broadcast peeks and consumes atomically under one lock hold (network I/O outside the lock); a wrong-wallet caller leaves the owner's token untouched, pinned by test.

Also per review: the dead core_wallet_signed_payment_register four-layer chain is deleted; the single stale-token code is split into typed siblings 27 ErrorStaleReservationToken / 28 ErrorReservationTokenConsumed / 29 ErrorReservationWalletMismatch (code 26 is not used by this PR — upstream now owns it as ErrorTransactionBroadcastRejected; both the Kotlin and Swift enums on this branch map 27/28/29 explicitly); the stale buildSignedPayment KDoc is fixed.

Error-code allocation note. These 27 / 28 / 29 allocations are being reconciled repo-wide in the error-code registry PR #4261. That registry records that ErrorReservationWalletMismatch = 29 on this branch currently collides with ErrorAssetLockInsufficientFunds = 29 on the asset-lock PR #4184, and that code 30 is free after #4184's re-scope (the variant previously reserved at 30 is not defined anywhere). The resolution of record is that #4184 keeps 29 and this PR moves its mismatch code to 30; that renumber has not yet landed on this head.

Local test evidence (fork PRs skip the Rust CI suite): platform-wallet --lib 508 passed, platform-wallet-ffi --lib 197 passed, clippy/fmt clean, Kotlin :sdk:testDebugUnitTest green.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds deferred Core signed-payment flows across the Rust wallet, FFI, JNI, and Kotlin SDK layers. Payments can be built and reserved, then broadcast or released with reservation tokens. The change adds generation validation, lifecycle cleanup, ownership handling, and typed error mappings.

Changes

Deferred signed payment lifecycle

Layer / File(s)	Summary
Wallet generation and reservation registry packages/rs-platform-wallet/src/wallet/core/*, packages/rs-platform-wallet/src/wallet/signed_payment_registry.rs, packages/rs-platform-wallet/src/manager/*	Adds generation-bound reservations, atomic registration, broadcast, release, expiry handling, teardown synchronization, and wallet recreation checks.
Native FFI lifecycle packages/rs-platform-wallet-ffi/src/core_wallet/*, packages/rs-platform-wallet-ffi/src/manager.rs, packages/rs-platform-wallet-ffi/src/error.rs, packages/rs-platform-wallet-ffi/src/wallet.rs	Adds deferred finalization, token broadcast and release, generation validation, error codes, handle cleanup, and lifecycle tests.
JNI and Kotlin API packages/rs-unified-sdk-jni/src/wallet_manager.rs, packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/*, packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/*	Adds signed-payment construction, ownership cleanup, broadcast and release operations, native bindings, typed errors, and JVM tests.
Supporting workspace changes Cargo.toml, packages/rs-platform-wallet/src/test_support.rs, packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swift	Updates Rust dependency sources, adds wallet test fixtures, and maps reservation errors in Swift.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant KotlinSDK
  participant JNI
  participant NativeFFI
  participant SignedPaymentRegistry
  participant CoreWallet
  KotlinSDK->>JNI: finalize signed payment
  JNI->>NativeFFI: fund, reserve, sign, and register
  NativeFFI->>SignedPaymentRegistry: store payment and token
  KotlinSDK->>JNI: broadcast or release token
  JNI->>NativeFFI: execute token operation
  NativeFFI->>SignedPaymentRegistry: broadcast or release reservation
  SignedPaymentRegistry->>CoreWallet: update reservation state

Loading

Possibly related issues

• Unify the two signed-payment concepts (build_signed_payment ↔ reservation-token finalize) #4263: Covers the reservation-token signed-payment lifecycle implemented by this change.
• [kotlin-sdk] Split build/broadcast with reservation release for BIP70-style deferred submission #4089: Covers the deferred build, broadcast, and release API implemented by this change.

Possibly related PRs

• dashpay/platform#4184: Extends related Kotlin platform-wallet native error mappings.
• dashpay/platform#4247: Adds the earlier Kotlin signed-payment build flow extended by this change.
• dashpay/platform#4264: Adds related typed DashSdkError.PlatformWallet mappings and tests.

Suggested reviewers: lklimek, quantumexplorer, shumkov

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main Kotlin SDK change: separate deferred-payment building and broadcasting with reservation release.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[thepastaclaw]

🔍 Review in progress — actively reviewing now (commit 6be6748)

[shumkov]

The core design is right (atomic finalize-and-register closes the double-selection race; reservation lifecycle is leak-free and the test matrix is strong). But two structural asks before merge:

• core_wallet_signed_payment_register → coreWalletRegisterSignedPayment → WalletManagerNative → ManagedCoreWallet.registerSignedPayment is a dead four-layer chain with zero callers after the finalize routing — and it's the unsafe variant (its age guard baselines at registration time, so the TTL protection is structurally defeated). Please delete it (or state explicitly why it stays), especially given refactor(sdk): dedup shared wallet code + remove dead FFI/JNI chains #4106 just removed this class of dead chains.
• The FFI now has two parallel deferred-tx lifecycles: the V2 handle surface Swift uses (core_wallet_tx_builder_finalize/broadcast/abandon_v2) and this token registry. There are real reasons to prefer the token here (V2's GC-backstop free releases the reservation; V2 has no age guard) — but they're stated nowhere, and V2 retains exactly the stale-release hazard this PR defends against. Please add the why-not-V2 rationale to the PR body and file a follow-up for the V2 age guard (it becomes live the moment iOS does deferred sends).

Minor: error code 26 conflates already-consumed (possibly paid!) / wallet-mismatch / aged-out — a payment UX can't tell "maybe paid" from "definitely not"; at minimum fix the doc, ideally split. Stale KDoc on buildSignedPayment still describes the pre-finalize shape. No Swift bindings for the new surface — fine, but track it.

[bfoss765]

@coderabbitai full review

[coderabbitai]

✅ Action performed

Full review finished.

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedCoreWallet.kt (1)

58-75: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value

Deprecate or remove the legacy registerSignedPayment bridge. It has no Kotlin callers in this repo, so keeping it unmarked only leaves a dead ABI surface in place. If it must remain for compatibility, add @Deprecated and point docs to the atomic finalizeSignedPayment flow.

🤖 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/wallet/ManagedCoreWallet.kt`
around lines 58 - 75, Deprecate the internal registerSignedPayment bridge
because it has no Kotlin callers and exposes a legacy ABI surface; if
compatibility requires retaining it, add `@Deprecated` and update its KDoc to
direct callers to the atomic finalizeSignedPayment flow, otherwise remove the
method.

🤖 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.

Inline comments:
In
`@packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedPlatformWallet.kt`:
- Around line 249-257: Update the KDoc paragraph for the method containing
finalizeSignedPayment to describe the atomic finalizeSignedPayment flow instead
of the deprecated new/addOutput*/setFunding/buildSigned sequence. State that
finalizeSignedPayment atomically selects, reserves, signs, and registers the
inputs, while preserving the existing explanation that broadcastSigned and
releaseReservation use the resulting token.

In `@packages/rs-platform-wallet-ffi/src/error.rs`:
- Around line 171-179: Update the documentation for ErrorStaleReservationToken
to explicitly include SignedPaymentError::StaleReservationToken alongside
StaleToken and WalletMismatch, and distinguish the unknown/consumed-token,
wrong-wallet-instance, and aged-out reservation cases with their respective host
semantics. Review the broadcast handler’s mapping and error details so hosts can
determine whether the reservation expired versus was consumed or belongs to
another wallet, without changing the shared error code.

---

Nitpick comments:
In
`@packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedCoreWallet.kt`:
- Around line 58-75: Deprecate the internal registerSignedPayment bridge because
it has no Kotlin callers and exposes a legacy ABI surface; if compatibility
requires retaining it, add `@Deprecated` and update its KDoc to direct callers to
the atomic finalizeSignedPayment flow, otherwise remove the method.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 038310fd-6fae-4081-961e-4fe849c78f63

📥 Commits

Reviewing files that changed from the base of the PR and between 8b466ab and 32cd702.

📒 Files selected for processing (19)

• packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/errors/DashSdkError.kt
• packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/WalletManagerNative.kt
• packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/CoreTransactionBuilder.kt
• packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedCoreWallet.kt
• packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedPlatformWallet.kt
• packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/errors/DashSdkErrorTest.kt
• packages/rs-platform-wallet-ffi/src/core_wallet/mod.rs
• packages/rs-platform-wallet-ffi/src/core_wallet/signed_payment.rs
• packages/rs-platform-wallet-ffi/src/core_wallet/transaction_builder.rs
• packages/rs-platform-wallet-ffi/src/error.rs
• packages/rs-platform-wallet-ffi/src/handle.rs
• packages/rs-platform-wallet-ffi/src/wallet.rs
• packages/rs-platform-wallet/src/lib.rs
• packages/rs-platform-wallet/src/test_support.rs
• packages/rs-platform-wallet/src/wallet/core/broadcast.rs
• packages/rs-platform-wallet/src/wallet/core/wallet.rs
• packages/rs-platform-wallet/src/wallet/mod.rs
• packages/rs-platform-wallet/src/wallet/signed_payment_registry.rs
• packages/rs-unified-sdk-jni/src/wallet_manager.rs

[shumkov]

Additional/strengthened lifetime findings after checking the existing threads:

1. Final-alias destruction consumes registry tokens without releasing live reservations. platform_wallet_destroy calls remove_entries_for_wallet, which only drops entries. Destroying the last wrapper alias does not necessarily remove the logical wallet from its manager, so the same wallet can be handed out again while those inputs remain reserved until TTL. Token cleanup should be tied to actual wallet-generation removal, or release while the original generation is still live.
2. Reservation age and token age start on opposite sides of external signing. The reservation height is captured before sign_tx(...).await, while register samples a fresh height afterward. A slow external signer can let the reservation be swept/reselected while the newly minted token still appears fresh. Carry the original reservation height in SignedCoreTransaction and register with it.
3. The V2 handle and registry-token paths have incompatible wallet-generation rules. V2 storage pins an old CoreWallet and validates only wallet ID, while registry tokens use manager identity plus wallet ID. After wallet recreation, an old V2 handle can act through the old manager while the new manager selects the same inputs. Both paths need one generation identity and one teardown policy.
4. Kotlin cancellation can orphan a token. buildSignedPayment returns a plain value through cancellable withContext(IO). If cancellation is observed after the blocking JNI registration returns, the token is discarded without a Cleaner/release path. Return an owning closeable object or make publication/release cancellation-safe.
5. Wrong-wallet broadcast consumes the token before checking its binding. SignedPaymentRegistry::broadcast removes first and validates second, so a mismatched caller destroys the original wallet's token and leaves its reservation until TTL. Validate under the lock, then atomically consume only a matching entry.

The existing age and dual-lifecycle comments point in the right direction; I would treat them as merge blockers rather than follow-ups because they can produce conflicting spends or stranded reservations.

[bfoss765]

All five lifetime findings are fixed as merge blockers and pushed, one commit each with regression tests: destroy releases the generation's reservations while teardown drops tokens and V2 handles; the pre-signing reservation height travels on SignedCoreTransaction into register; one generation identity across both the V2-handle and token paths; cancellation-safe ownership (an owning AutoCloseable with a Cleaner backstop, plus object-owning broadcastSigned/releaseReservation overloads that keep the payment reachable across the native call); and validate-then-consume under a single lock hold with network I/O outside it. The dead register chain is deleted, code 26 is split into typed 26/27/28 siblings (Kotlin-only host impact; Swift falls through safely), and the why-token-not-V2 rationale is in the PR body.

The V2 age guard you asked to file as a follow-up is implemented as a stacked PR: the V2 broadcast refuses at the same shared threshold off the same pre-signing height stamp (abandon works at any age), with exact-boundary tests on both account types.

[shumkov]

Consolidated re-verification (two independent passes). All five lifetime findings are genuinely fixed with discriminating regression tests, the dead register chain is fully deleted, the why-not-V2 rationale landed, and the error split into 26/27/28 (with txid carried on 27) is right. Two issues remain from the deeper pass:

1. Generation validation and reservation mutation are not atomic. The registry validates is_same_generation and then mutates after releasing its lock (core/wallet.rs:70-98,321-338, core/broadcast.rs:111-124, core/transaction.rs:257-268) — a same-ID wallet recreation between validation and cleanup can make old cleanup release the new generation's reservation. Bind the cleanup to a generation-local handle, or validate-and-mutate under a single manager lock, and cover same-ID recreation in the tests.

2. Deferred CoinJoin finalization can leak reservations until TTL. The FFI finalize path (transaction_builder.rs:170-293) reserves inputs, but registry rejection/abandon/free releases only when an account_type handle is present — CoinJoin-funded deferred payments left without one keep their funds reserved until the 24-block TTL. Retain a releasable account handle in the registry entry.

Minor: signed_payment.rs:40-45 still documents the pre-split error semantics (repeated-broadcast / re-created-wallet now yield 27/28, not 26), and the PR body should cite #4196 by number as the V2-side follow-up.

[shumkov]

Addendum: the missing Swift mappings for the new codes belong to this PR too, not only #4196 — PlatformWalletResult.swift:68-70 jumps from code 25 straight to 98, so 26/27/28 (introduced here) all surface as .errorUnknown on iOS. Fine to fix in either PR, but one of the two must carry it before the pair lands.

[bfoss765]

Both remaining blockers fixed (510 platform-wallet tests):

1. Atomic validate-and-mutate — reservation cleanup now re-validates the wallet generation (Arc::ptr_eq on the per-generation balance Arc) AND mutates the ReservationSet under a single manager read-lock hold. A same-id recreation needs the write lock and so can't interleave — provably atomic, covered by a new regression test that recreates between register and release and asserts the input stays reserved.
2. CoinJoin releasable handle — the registry entry now retains the full AccountTypePreference (incl. CoinJoin), so a rejected/abandoned CoinJoin-funded deferred payment releases immediately instead of waiting out the 24-block TTL (tested).

Swift now maps 26/27/28 to typed StaleReservationToken/ReservationTokenConsumed/ReservationWalletMismatch (exhaustive init(result:), message parity with Kotlin); the stale-broadcast doc is corrected. The V2 handle path is the follow-up in #4196.

One conscious scoping note: the immediate-send reject-release path (reservations.rs) shares the same theoretical generation window but a far narrower one — synchronous build→broadcast, no persisted token surviving a restart, CoinJoin not used for immediate sends — so this PR scopes the guard to the deferred registry + V2 handle paths your finding named. Happy to extend it there too if you'd prefer.

[shumkov]

Round-3 verification (two independent passes, reconciled): both previous blockers are genuinely fixed — the generation-safe release validates and mutates under one manager read-lock hold (and recreation requires the write lock, so no interleave), and CoinJoin entries now retain a releasable AccountTypePreference handle with a production-path regression test.

One new P1 (shared with #4196) — freshness and release are still two separate decisions:

• A reservation stamped at height 100 passes the age guard at 119; the broadcast await can span more than the 4-block margin; sync reaches 124, key-wallet's TTL sweeps the reservation and another build re-reserves the same outpoint; the rejected-broadcast cleanup then releases the new reservation by outpoint.
• Fix: read the current height, validate generation, and mutate as one guarded operation under the release lock; re-check freshness after the broadcast await before the Rejected-release; cover the signing-failure release path too. The existing tests prove endpoint states — add a barrier-controlled interleaving test that forces check → sweep → re-reserve → release.

Also before merge: rebase (branch is CONFLICTING with v4.1-dev), and please cite #4196 by number in the body as the V2-side sibling. Nit: new comments carry fix-round narration — provenance belongs in the PR description.

[thepastaclaw]

Preliminary review — Codex only

Carried forward, prior-1 remains partially valid: consuming SignedCoreTransaction prevents duplicate registration, but the independently supplied CoreWallet can still bind the reservation to the wrong wallet generation; prior-3 remains real but was explicitly deferred to a lifecycle-gate follow-up. The latest delta fixes prior-2, prior-4, and prior-5, but introduces another blocking ownership hole because the now-consuming registration API is still async and can lose the reservation owner before its first poll. The full PR also needs defensive Kotlin cleanup if token-owner construction fails, while the obsolete public alias-scan helper is a minor cleanup.

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 (completed), gpt-5.6-sol — rust-quality (completed), gpt-5.6-sol — ffi-engineer (completed)
• Verifier: gpt-5.6-sol — verifier
• Sonnet: not run (deferred by blocker gate)
  Source: reviewers gpt-5.6-sol; verifier gpt-5.6-sol.

🔴 2 blocking | 🟡 1 suggestion(s) | 💬 1 nitpick(s)

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-platform-wallet/src/wallet/signed_payment_registry.rs`:
- [BLOCKING] packages/rs-platform-wallet/src/wallet/signed_payment_registry.rs:283-300: Registration can bind a reservation to the wrong wallet generation
  `register` consumes the finalized transaction and derives its transaction, account, height, and funding token, but it still accepts an independent `CoreWallet`. `SignedCoreTransaction` carries no originating wallet-generation marker, so safe public Rust code can finalize through wallet A and call `register(core_b, signed_from_a)`. The registry then treats B as the owner: broadcasting through B passes the generation check and submits A's transaction through B's broadcaster, while release or definitive-rejection cleanup runs against B and leaves A's actual reservation held until its TTL. This violates the documented guarantee that a token is bound to the generation whose `ReservationSet` owns the inputs. Store an unforgeable originating-generation marker in `SignedCoreTransaction` and derive or validate the registry's core from it instead of trusting a separate argument.
- [BLOCKING] packages/rs-platform-wallet/src/wallet/signed_payment_registry.rs:283-302: Async registration can drop the reservation owner before insertion
  `register` now consumes the non-`Clone` `SignedCoreTransaction`, but it remains an `async fn` even though its body contains no await. Calling an async function only moves its arguments into the returned future; the body does not execute until that future is polled. If the future is dropped or cancelled before its first poll, the consumed `SignedCoreTransaction` is dropped without being inserted, no payment token is returned, and the type has no `Drop` implementation that releases its funding reservation. The reservation is therefore unreachable to explicit cleanup until key-wallet's TTL. Make registration synchronous and update callers to invoke it directly.

In `packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/CoreTransactionBuilder.kt`:
- [SUGGESTION] packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/CoreTransactionBuilder.kt:184-191: Release the native token if Kotlin owner construction fails
  Native finalization has already inserted the payment and committed its reservation when this byte array returns. The token does not gain its owning `NativeCleaner` until `SignedCoreTransaction.fromRegisterBlob` finishes, so an allocation failure, malformed-data exception from an ABI mismatch, or Cleaner-registration failure leaves the native token registered without any JVM owner capable of releasing it. Parse the token first and release it defensively if construction of the ownership object throws.

[thepastaclaw]

🔴 Blocking: Registration can bind a reservation to the wrong wallet generation

register consumes the finalized transaction and derives its transaction, account, height, and funding token, but it still accepts an independent CoreWallet. SignedCoreTransaction carries no originating wallet-generation marker, so safe public Rust code can finalize through wallet A and call register(core_b, signed_from_a). The registry then treats B as the owner: broadcasting through B passes the generation check and submits A's transaction through B's broadcaster, while release or definitive-rejection cleanup runs against B and leaves A's actual reservation held until its TTL. This violates the documented guarantee that a token is bound to the generation whose ReservationSet owns the inputs. Store an unforgeable originating-generation marker in SignedCoreTransaction and derive or validate the registry's core from it instead of trusting a separate argument.

_{source: ['codex']}

[thepastaclaw]

Resolved in 7d85953 — Registration can bind a reservation to the wrong wallet generation no longer present.

_{Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.}

[bfoss765]

Fixed in 7d85953c2a. SignedCoreTransaction carries origin_generation (transaction.rs:98) — the per-generation balance Arc captured inside finalize_transaction. register pointer-compares it against the supplied core (signed_payment_registry.rs:450-452) and refuses with RegisterWrongGeneration, handing the finalized payment back so its reservation is released rather than stranded (transaction_builder.rs:297-305). Covered by register_rejects_a_different_wallet_generation.

[thepastaclaw]

🔴 Blocking: Async registration can drop the reservation owner before insertion

register now consumes the non-Clone SignedCoreTransaction, but it remains an async fn even though its body contains no await. Calling an async function only moves its arguments into the returned future; the body does not execute until that future is polled. If the future is dropped or cancelled before its first poll, the consumed SignedCoreTransaction is dropped without being inserted, no payment token is returned, and the type has no Drop implementation that releases its funding reservation. The reservation is therefore unreachable to explicit cleanup until key-wallet's TTL. Make registration synchronous and update callers to invoke it directly.

_{source: ['codex']}

[thepastaclaw]

Resolved in 7d85953 — Async registration can drop the reservation owner before insertion no longer present.

_{Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.}

[bfoss765]

Fixed in 7d85953c2a. register (signed_payment_registry.rs:438) is synchronous with no .await in its body, so the reservation-owning insert runs inline and there is no future that could be dropped before first poll. Callers invoke it directly (transaction_builder.rs:294).

[thepastaclaw]

🟡 Suggestion: Release the native token if Kotlin owner construction fails

Native finalization has already inserted the payment and committed its reservation when this byte array returns. The token does not gain its owning NativeCleaner until SignedCoreTransaction.fromRegisterBlob finishes, so an allocation failure, malformed-data exception from an ABI mismatch, or Cleaner-registration failure leaves the native token registered without any JVM owner capable of releasing it. Parse the token first and release it defensively if construction of the ownership object throws.

Suggested change

	val blob = WalletManagerNative.coreWalletFinalizeSignedPayment(
	builderPtr,
	walletHandle,
	accountType.ffiValue,
	accountIndex,
	coreSignerHandle,
	)
	return ManagedPlatformWallet.SignedCoreTransaction.fromRegisterBlob(blob)
	val blob = WalletManagerNative.coreWalletFinalizeSignedPayment(
	builderPtr,
	walletHandle,
	accountType.ffiValue,
	accountIndex,
	coreSignerHandle,
	)
	var token: Long? = null
	return try {
	token = java.nio.ByteBuffer.wrap(blob).long
	ManagedPlatformWallet.SignedCoreTransaction.fromRegisterBlob(blob)
	} catch (error: Throwable) {
	token?.let { value ->
	runCatching { WalletManagerNative.coreWalletReleaseSignedPayment(value) }
	}
	throw error
	}

_{source: ['codex']}

[thepastaclaw]

Resolved in 7d85953 — Release the native token if Kotlin owner construction fails no longer present.

_{Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.}

[bfoss765]

Adopted as suggested — CoreTransactionBuilder.kt:201-210 parses the token from the blob's leading 8 big-endian bytes first, then releases it under runCatching { coreWalletReleaseSignedPayment(value) } if owner construction throws, and rethrows. Offset verified against ManagedPlatformWallet.kt:257-259.

[thepastaclaw]

💬 Nitpick: The removed final-alias policy left a stale public handle API

The latest delta removed the final-alias scan from platform_wallet_destroy, leaving HandleStorage::any with no call sites. Its Rustdoc still says it exists for that removed policy, and handle is a public module, so the helper unnecessarily remains part of the crate's public Rust surface. Remove the method and obsolete explanation.

_{source: ['codex']}

[thepastaclaw]

Resolved in this update — The removed final-alias policy left a stale public handle API no longer present.

_{Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.}

[bfoss765]

Confirmed live and now fixed — the auto-resolve on 2026-07-25 was incorrect; HandleStorage::any was still present at head d854debb33 (packages/rs-platform-wallet-ffi/src/handle.rs:74-83), untouched since the policy removal.

Your read was right on all three counts:

• platform_wallet_destroy no longer scans — wallet.rs:392-414 is just PLATFORM_WALLET_STORAGE.remove(handle) (line 412).
• any had zero call sites workspace-wide, and its Rustdoc still cited "the final-alias check in platform_wallet_destroy".
• It stayed on the public surface silently because handle is a pub mod, so no dead-code lint fires.

It was introduced by ade399913c for that sweep and is not on v4.2-dev, so removing it restores the base surface rather than dropping an API a consumer could have depended on. Method and doc are gone.

Kept remove_matching — still used for the generation sweep at manager.rs:494.

Also corrected one stale cross-reference to the same removed policy: test_support.rs:391-392 described test_platform_wallet_manager as backing "final-alias registry-sweep gating", but its only FFI consumer (destroy_tests::destroying_wrapper_aliases_never_sweeps_tokens) now asserts the opposite.

No behavior change. cargo fmt --check clean; cargo test -p platform-wallet-ffi -p platform-wallet → 762 passed / 0 failed, including the three manager::remove_wallet_lifecycle_tests regressions.

[bfoss765]

@thepastaclaw both fixed in 7d85953c (513 platform-wallet + 6 FFI tests pass):

• Wrong wallet-generation binding: SignedCoreTransaction now carries an unforgeable origin_generation: Arc<WalletBalance> (the same per-generation Arc is_same_generation pointer-compares, captured in finalize_transaction). register validates Arc::ptr_eq(core.generation(), signed.origin_generation()) before consuming signed; a mismatch returns a typed RegisterWrongGeneration that hands signed back (A's reservation never stranded), and the FFI releases it owner-guarded → ErrorReservationWalletMismatch.
• Async register drops the owner: register is now pub fn (its body had no await), so a dropped/cancelled future can no longer drop the consumed SignedCoreTransaction before insertion; callers invoke it directly (removed the block_on).
• Suggestion: CoreTransactionBuilder.finalizeSignedPayment now releases the native token (owner-guarded) if the Kotlin owner construction throws.
  Success path + owner-tagged semantics unchanged; new test register_rejects_a_different_wallet_generation proves the mismatch is refused with no token minted and the reservation still reachable.

[thepastaclaw]

Final validation — Codex + Sonnet

The substantive ownership issues are fixed at the exact reviewed head: registration now validates the originating wallet generation, executes synchronously, and Kotlin releases the native token if owner construction fails. One non-functional cleanup remains: HandleStorage::any is unused public API whose documentation still describes the removed final-alias policy.

Review provenance

• Codex reviewers: gpt-5.6-sol — general (completed), gpt-5.6-sol — rust-quality (completed), gpt-5.6-sol — ffi-engineer (completed)
• Verifier: gpt-5.6-sol — verifier (fallback)
• Sonnet reviewers: claude-sonnet-5 — general (completed), claude-sonnet-5 — rust-quality (failed), claude-sonnet-5 — ffi-engineer (failed), claude-sonnet-5 — rust-quality (completed), claude-sonnet-5 — ffi-engineer (failed), claude-sonnet-5 — ffi-engineer (completed)
  Source: reviewers gpt-5.6-sol + claude-sonnet-5; final verifier gpt-5.6-sol (fallback after unparseable Sonnet verifier).

💬 1 nitpick(s)

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

[bfoss765]

@shumkov — at the current head CI is green and the review gate reads no blockers. You're one of the CODEOWNERS on the rs-platform-wallet packages this PR touches (with @QuantumExplorer, @lklimek, @llbartekll, @ZocoLini), and you've already run several verification passes. Could you confirm who owns the next steps to get this approved? Happy to address anything still outstanding on my end.

[bfoss765]

Friendly re-ping: this PR has been code-complete with CI green and no outstanding findings for several days, but is stalled awaiting a human CODEOWNER approval (my 07-26 question went unanswered). It now gates the receival-spend stack: #4256 stacks directly on it and #4247's final shape follows it. @shumkov — or if ownership sits elsewhere, @QuantumExplorer / @lklimek, could one of you take the approval? Relatedly, #4186 is merge-ready per its final review and only needs its stale PastaClaw check re-triggered. 🤖 Generated with Claude Code

[coderabbitai]

Actionable comments posted: 3

🤖 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.

Inline comments:
In `@Cargo.toml`:
- Around line 55-62: Replace the personal-fork git sources for all eight
rust-dashcore dependencies—dashcore, dash-network-seeds, dash-spv, key-wallet,
key-wallet-ffi, key-wallet-manager, dash-network, and dashcore-rpc—with the
corresponding dashpay/rust-dashcore upstream source, pinned to the required
upstream revision after confirming it exists there. Do not merge the bfoss765
fork revision; if its changes are required, use an upstream revision containing
them.

In
`@packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/errors/DashSdkError.kt`:
- Around line 299-301: Update ErrorReservationWalletMismatch to discriminant 30
everywhere: change the mapping and trailing comment in DashSdkError.kt lines
299-301, revise its class KDoc at line 210 and code-98 comment at line 286,
update the KDoc reference in WalletManagerNative.kt lines 269-279, and change
the offset assertion in DashSdkErrorTest.kt lines 106-132 from 29 to 30. Keep
all Kotlin documentation, mapping, and test expectations consistent with the
native discriminant.

In
`@packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/wallet/SignedCoreTransactionTest.kt`:
- Around line 41-52: Explicitly close the SignedCoreTransaction instance in the
test after the assertions, using signed.close() so its cleaner backstop is
disarmed before the object becomes unreachable. Preserve the existing ownership
compile-time check and transaction assertions.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: aa0a1c5a-6af0-45f8-a802-affd47b0ef72

📥 Commits

Reviewing files that changed from the base of the PR and between 32cd702 and 0b0d5c7.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (8)

• Cargo.toml
• packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/errors/DashSdkError.kt
• packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/WalletManagerNative.kt
• packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/CoreTransactionBuilder.kt
• packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedCoreWallet.kt
• packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedPlatformWallet.kt
• packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/errors/DashSdkErrorTest.kt
• packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/wallet/SignedCoreTransactionTest.kt

🚧 Files skipped from review as they are similar to previous changes (1)

• packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/CoreTransactionBuilder.kt

[coderabbitai]

🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

Do not merge workspace dependencies pinned to a personal fork.

All eight rust-dashcore crates now resolve from https://github.com/bfoss765/rust-dashcore at rev e99959ced0062159d629930f488374e29f63c42b. A personal fork is outside the organization's control: the owner can force-push, rename, or delete the repository, and the build then fails or silently resolves unaudited code. This also makes the whole wallet, SPV, and key-derivation stack unreviewed relative to dashpay/rust-dashcore.

This change is also unrelated to the deferred signed-payment work in this PR. If the fork carries a required upstream change, land that change in dashpay/rust-dashcore first, then pin this workspace to the resulting upstream revision before merge.

🔒 Proposed change once the upstream revision exists

-dashcore = { git = "https://github.com/bfoss765/rust-dashcore", rev = "e99959ced0062159d629930f488374e29f63c42b" }
-dash-network-seeds = { git = "https://github.com/bfoss765/rust-dashcore", rev = "e99959ced0062159d629930f488374e29f63c42b" }
-dash-spv = { git = "https://github.com/bfoss765/rust-dashcore", rev = "e99959ced0062159d629930f488374e29f63c42b" }
-key-wallet = { git = "https://github.com/bfoss765/rust-dashcore", rev = "e99959ced0062159d629930f488374e29f63c42b" }
-key-wallet-ffi = { git = "https://github.com/bfoss765/rust-dashcore", rev = "e99959ced0062159d629930f488374e29f63c42b" }
-key-wallet-manager = { git = "https://github.com/bfoss765/rust-dashcore", rev = "e99959ced0062159d629930f488374e29f63c42b" }
-dash-network = { git = "https://github.com/bfoss765/rust-dashcore", rev = "e99959ced0062159d629930f488374e29f63c42b" }
-dashcore-rpc = { git = "https://github.com/bfoss765/rust-dashcore", rev = "e99959ced0062159d629930f488374e29f63c42b" }
+dashcore = { git = "https://github.com/dashpay/rust-dashcore", rev = "<upstream-rev>" }
+dash-network-seeds = { git = "https://github.com/dashpay/rust-dashcore", rev = "<upstream-rev>" }
+dash-spv = { git = "https://github.com/dashpay/rust-dashcore", rev = "<upstream-rev>" }
+key-wallet = { git = "https://github.com/dashpay/rust-dashcore", rev = "<upstream-rev>" }
+key-wallet-ffi = { git = "https://github.com/dashpay/rust-dashcore", rev = "<upstream-rev>" }
+key-wallet-manager = { git = "https://github.com/dashpay/rust-dashcore", rev = "<upstream-rev>" }
+dash-network = { git = "https://github.com/dashpay/rust-dashcore", rev = "<upstream-rev>" }
+dashcore-rpc = { git = "https://github.com/dashpay/rust-dashcore", rev = "<upstream-rev>" }

Run the following script to check whether the pinned revision exists upstream:

#!/bin/bash
# Check whether the pinned rev is reachable in the upstream org repository.
REV=e99959ced0062159d629930f488374e29f63c42b
echo "--- upstream dashpay/rust-dashcore ---"
gh api "repos/dashpay/rust-dashcore/commits/$REV" --jq '.sha, .commit.message' 2>&1 | head -5
echo "--- fork bfoss765/rust-dashcore ---"
gh api "repos/bfoss765/rust-dashcore" --jq '.full_name, .fork, .private, .parent.full_name' 2>&1 | head -5
echo "--- other fork/personal git deps in the workspace ---"
fd -t f 'Cargo.toml' --exec rg -n 'git = "https://github.com/(?!dashpay/)' -P {} \; 2>/dev/null | head -40

🤖 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 `@Cargo.toml` around lines 55 - 62, Replace the personal-fork git sources for
all eight rust-dashcore dependencies—dashcore, dash-network-seeds, dash-spv,
key-wallet, key-wallet-ffi, key-wallet-manager, dash-network, and
dashcore-rpc—with the corresponding dashpay/rust-dashcore upstream source,
pinned to the required upstream revision after confirming it exists there. Do
not merge the bfoss765 fork revision; if its changes are required, use an
upstream revision containing them.

[bfoss765]

Agreed — this is a genuine merge blocker, and it can't be fixed inside this PR.

The eight crates are pinned to bfoss765/rust-dashcore @ e99959ce because this
PR depends on an unreleased key-wallet change: owner-tagged reservations, which
close the broadcast/release TOCTOU that the deferred build → broadcast lifecycle
in this PR is built on. Without it the reservation set can't tell which owner a
release belongs to, and the split-build feature has no safe primitive to sit on.

That change is upstream as dashpay/rust-dashcore#916
("feat(key-wallet): owner-tagged reservations to close the broadcast-release
TOCTOU (platform#4185)"). It is open and currently conflicting; we're working it now. This PR is blocked on #916 merging and should not go in before it.

On the "does the rev exist upstream" check in your script — it needs one
correction, because the naive form gives a false pass. gh api repos/dashpay/rust-dashcore/commits/e99959ce does return the commit, but only
because forks share object storage with the parent; resolvability is not
reachability. The real check is against a branch:

$ gh api repos/dashpay/rust-dashcore/compare/dev...e99959ce --jq .status
diverged        # ahead_by=1, behind_by=6
$ gh api repos/dashpay/rust-dashcore/commits/e99959ce/branches-where-head
                # empty

So the pin is fork-only, exactly as you say: one commit ahead of a point on
dev and six behind, on no upstream branch.

Plan of record: once #916 lands, this workspace repins all eight crates to
https://github.com/dashpay/rust-dashcore at the resulting upstream revision —
no bfoss765 remotes in the merged tree. I'll leave this thread open as the
merge gate.

[coderabbitai]

@bfoss765, acknowledged. The explanation confirms that the personal-fork pin is a required temporary dependency for owner-tagged reservations in #916.

You are correct about the validation detail. A successful commits/<sha> response only proves object availability. It does not prove that an upstream branch reaches the revision. The branch reachability checks are the correct validation.

I will keep this thread open. Before this PR merges, it must repin all eight workspace dependencies to https://github.com/dashpay/rust-dashcore at an upstream revision that contains #916.

_{You are interacting with an AI system.}

[thepastaclaw]

Preliminary review — Codex only

Carried forward, prior-1 is fixed at the exact reviewed head: commit 6c37e8679e removed the obsolete HandleStorage::any helper and its stale final-alias documentation. In the new lifecycle delta, the registry-token FFI path is substantially improved, but the exclusion remains attached to one process-global registry rather than shared wallet-generation state, so V2 FFI operations and direct Rust manager removal can still publish or broadcast payments after teardown. The lifecycle comments also incorrectly state that finalizers hold the gate while awaiting an external signature.

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 (completed), gpt-5.6-sol — rust-quality (completed), gpt-5.6-sol — ffi-engineer (completed)
• Verifier: gpt-5.6-sol — verifier
• Sonnet: not run (deferred by blocker gate)
  Source: reviewers gpt-5.6-sol; verifier gpt-5.6-sol.

🔴 1 blocking | 💬 1 nitpick(s)

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-platform-wallet-ffi/src/manager.rs`:
- [BLOCKING] packages/rs-platform-wallet-ffi/src/manager.rs:476-494: Move lifecycle exclusion into shared wallet-generation state
  The write guard taken here belongs to the FFI singleton `SIGNED_PAYMENT_REGISTRY`, so it excludes only registry-token operations rather than every operation acting on the generation being removed. The live V2 path bypasses it: `core_wallet_tx_builder_finalize` awaits the external signer and inserts into `CORE_SIGNED_TRANSACTION_V2_STORAGE` without a lifecycle guard or current-generation revalidation (`core_wallet/transaction_builder.rs:126-137`), allowing teardown to sweep while signing is pending and a late finalizer to publish a fresh handle afterward. `core_wallet_broadcast_signed_transaction_v2` likewise consumes a handle and reaches the embedded wallet's broadcaster without a guard or liveness check (`core_wallet/broadcast.rs:41-65`); two retained handles to the removed generation still pass `is_same_generation`, so the transaction can reach the network after removal and conflict with inputs selected by a re-created generation. The same coordination can be bypassed through the newly public Rust surface: `SignedPaymentRegistry` is re-exported from `platform-wallet`, while public `PlatformWalletManager::remove_wallet` never takes the registry's write side, so direct Rust removal can interleave between a token operation's `is_current_generation` check and its register or network action despite the documented read guard. Store the lifecycle gate in shared per-generation or manager state, acquire it from public removal and from both token and V2 publication/network paths, and add deterministic V2 and direct-manager removal race tests. This also prevents a 30-second SPV broadcast for one wallet from blocking lifecycle operations for every unrelated wallet through the current process-global lock. V2 abandon/free cleanup is generation-bound and does not itself require exclusion; the unsafe gaps are late handle publication and network access.

[thepastaclaw]

🔴 Blocking: Move lifecycle exclusion into shared wallet-generation state

The write guard taken here belongs to the FFI singleton SIGNED_PAYMENT_REGISTRY, so it excludes only registry-token operations rather than every operation acting on the generation being removed. The live V2 path bypasses it: core_wallet_tx_builder_finalize awaits the external signer and inserts into CORE_SIGNED_TRANSACTION_V2_STORAGE without a lifecycle guard or current-generation revalidation (core_wallet/transaction_builder.rs:126-137), allowing teardown to sweep while signing is pending and a late finalizer to publish a fresh handle afterward. core_wallet_broadcast_signed_transaction_v2 likewise consumes a handle and reaches the embedded wallet's broadcaster without a guard or liveness check (core_wallet/broadcast.rs:41-65); two retained handles to the removed generation still pass is_same_generation, so the transaction can reach the network after removal and conflict with inputs selected by a re-created generation. The same coordination can be bypassed through the newly public Rust surface: SignedPaymentRegistry is re-exported from platform-wallet, while public PlatformWalletManager::remove_wallet never takes the registry's write side, so direct Rust removal can interleave between a token operation's is_current_generation check and its register or network action despite the documented read guard. Store the lifecycle gate in shared per-generation or manager state, acquire it from public removal and from both token and V2 publication/network paths, and add deterministic V2 and direct-manager removal race tests. This also prevents a 30-second SPV broadcast for one wallet from blocking lifecycle operations for every unrelated wallet through the current process-global lock. V2 abandon/free cleanup is generation-bound and does not itself require exclusion; the unsafe gaps are late handle publication and network access.

_{source: ['codex']}

[thepastaclaw]

Resolved in d1bed4d — Move lifecycle exclusion into shared wallet-generation state no longer present.

_{Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.}

[thepastaclaw]

💬 Nitpick: Correct the finalizer lifecycle-gate comments

A finalizer does not hold the shared lifecycle guard while awaiting the external signer. core_wallet_signed_payment_finalize acquires it only after finalize_transaction and the signer await complete (core_wallet/transaction_builder.rs:220-240), deliberately allowing teardown to finish during an open signing prompt; the subsequent liveness check then rejects registration. This comment and the equivalent Rustdoc at signed_payment_registry.rs:353-356 describe the opposite behavior and could cause future tests or changes to assume that removal waits for signing.

Suggested change

	// * Acquiring it WAITS for in-flight payment operations. A finalize that is
	// mid-signature holds the shared side (`finalize_transaction` drops the
	// manager write lock before awaiting the signer, so nothing else stops
	// it), so it runs to its liveness check and either registers before we
	// start — and is swept below — or observes the removal and abandons.
	// Either way it can no longer insert a token AFTER the sweep has run.
	// * Acquiring it waits for payment operations that have entered their
	// post-signing liveness-check/register section. A finalizer still awaiting
	// the external signer holds no gate; if teardown completes first, its later
	// liveness check observes the missing generation and abandons. If the
	// finalizer reaches the shared gate first, it registers before teardown and
	// the sweep below removes its token.

_{source: ['codex']}

[thepastaclaw]

Resolved in d1bed4d — Correct the finalizer lifecycle-gate comments no longer present.

_{Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.}

[bfoss765]

Verified all three findings at 6c37e8679e before changing anything — all three hold. Fixed in d1bed4dc78.

The V2 path was ungated on both ends: core_wallet_tx_builder_finalize inserted into CORE_SIGNED_TRANSACTION_V2_STORAGE with no gate and no liveness re-check after the signer await, and core_wallet_broadcast_signed_transaction_v2 reached broadcast_finalized_transaction with neither — its is_same_generation check compares two handles, and a removed generation matches itself. Public removal took no exclusion at all; the gate lived only in the FFI wrapper, so a direct Rust embedder bypassed the linearization entirely. Contention was as described: one Lazy static, the shared side held across the SPV send, write-preferring lock — so one wallet's send blocked every other wallet's teardown, and their payments behind the queued writer.

On the nitpick — agreed, and the comments were the thing that was wrong. Finalizers deliberately take the gate after the signature returns, because holding it across an open signing prompt would stall teardown for every wallet. Corrected in manager.rs and in the gate's Rustdoc.

Design. The gate now lives in a new WalletGeneration that is the generation-identity marker — it owns the lock-free WalletBalance and that generation's RwLock, replacing Arc<WalletBalance> wherever identity was compared. One Arc rather than a parallel gate Arc is deliberate: threaded separately through ~15 construction sites, a future site could clone the identity but mint a fresh gate, and two handles would then compare as the same generation while excluding each other through different locks — a silent loss of exclusion with nothing to fail on. Deref<Target = WalletBalance> keeps every existing balance read unchanged.

PlatformWalletManager::remove_wallet_with_teardown holds the generation's exclusive gate across both the removal and a caller-supplied synchronous teardown hook, and remove_wallet routes through it, so the gate cannot be skipped by any caller; the FFI passes its registry + V2-handle sweep as that hook. Lock order stays gate -> manager: the lookup drops wallets before awaiting the gate, then re-validates under it and retries if the generation changed.

Paths gated (swept rather than taken from the finding's list): registry broadcast and release — the latter has no wallet argument, so it peeks the entry's generation under the map mutex, drops it, takes that gate, then consumes — plus core_wallet_signed_payment_finalize, and newly core_wallet_tx_builder_finalize and core_wallet_broadcast_signed_transaction_v2. Both new ones report a dead generation as the existing NotFound (98) after reconciling the reservation, so nothing is stranded.

Also swept: the third CORE_SIGNED_TRANSACTION_V2_STORAGE.insert is inside #[cfg(test)]; V2 abandon/free stay ungated since their release is already generation-bound under a single manager-lock hold. One judgment call flagged rather than skipped silently — the immediate-send path (core_wallet_broadcast_transaction) retains no handle across teardown, so it sits outside the deferred-lifecycle contract and I did not gate it. Say if you'd rather it were covered.

Tests: four new deterministic ones. With the three guards removed, broadcasting_a_v2_handle_for_a_removed_wallet_is_refused_before_the_network, teardown_waits_for_an_in_flight_v2_operation_and_then_sweeps_its_handle and public_remove_wallet_waits_for_an_in_flight_payment_on_that_generation all fail; with the gate re-pointed at a single process-global lock, an_in_flight_payment_does_not_block_an_unrelated_wallets_teardown fails with Elapsed.

Worth flagging: I first wrote two of these in the completion-order style of the existing a_broadcast_never_reaches_the_broadcaster_after_teardown_completed, and they passed against the broken code on some runs. That style is timing-dependent and isn't a reliable gate; they were replaced with the deterministic forms above.

ErrorReservationWalletMismatch stays 30; HandleStorage::any stays removed with remove_matching retained at its live call site. cargo fmt --check clean; cargo test -p platform-wallet-ffi -p platform-wallet green — 513 + 212 lib tests, 0 failures across all 11 result lines. Dependent crates all still compile (the PlatformWalletInfo.balance -> .generation rename is source-breaking).

[thepastaclaw]

Preliminary review — Codex only

Both carried-forward lifecycle findings are fixed at d1bed4d: lifecycle exclusion is now shared per wallet generation across direct removal, registry-token operations, and V2 operations, and the finalizer comments match the post-signing gate acquisition. The verifier ran the lifecycle and registry suites successfully (7/7 and 19/19), but the current head still has two blockers: same-ID registration can race the manager's two-map removal sequence, and all rust-dashcore dependencies remain pinned to an unmerged contributor fork. Three additional in-scope suggestions cover generation-state mutability, deterministic Kotlin cancellation cleanup, and typed Kotlin handling for definitive broadcast rejection.

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 (completed), gpt-5.6-sol — rust-quality (completed), gpt-5.6-sol — ffi-engineer (completed)
• Verifier: gpt-5.6-sol — verifier
• Sonnet: not run (deferred by blocker gate)

🔴 2 blocking | 🟡 3 suggestion(s)

1 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/rs-platform-wallet/src/manager/wallet_lifecycle.rs`:
- [BLOCKING] packages/rs-platform-wallet/src/manager/wallet_lifecycle.rs:655-687: Remove the same wallet generation from both maps atomically
  The retry loop acquires generation G1's teardown gate and verifies that `self.wallets` still names G1, but removal then happens in two independently locked stages. After G1 is removed from the inner `wallet_manager`, that lock is released before `self.wallets` is acquired. A concurrent same-ID registration can now insert generation G2 into the inner manager and publish G2 into `self.wallets`; if that publication wins on a multithreaded executor, this remover takes G2 from the outer map and invokes `tear_down` for G2 while holding only G1's gate. G2 remains registered in the inner manager but disappears from the public map, and its registry tokens or V2 handles can be swept without excluding G2's payment operations. Give registration and removal a single per-ID ordering, or retain the verified G1 `Arc<PlatformWallet>` and remove the outer entry only when it still pointer-matches G1. Add a deterministic remove-versus-recreate regression test.

In `Cargo.toml`:
- [BLOCKING] Cargo.toml:55-62: Do not merge workspace-wide dependencies from a personal fork
  All eight rust-dashcore workspace dependencies are redirected from `dashpay/rust-dashcore` to `bfoss765/rust-dashcore` at e99959ce. That revision is only the first commit of dashpay/rust-dashcore#916, which is currently OPEN with `mergeStateStatus=BLOCKED` and `reviewDecision=REVIEW_REQUIRED`; the upstream PR has since added further fixes, including owner-guarded cleanup for asset-lock downstream failures. Merging this head would therefore make the entire workspace depend on a contributor-controlled repository and a stale, unmerged implementation. Land the reservation API and its follow-up fixes in the Dash-owned repository, then repin these entries and `Cargo.lock` to the corresponding Dash-owned revision.

In `packages/rs-platform-wallet/src/wallet/platform_wallet.rs`:
- [SUGGESTION] packages/rs-platform-wallet/src/wallet/platform_wallet.rs:52: Keep the generation identity immutable outside the crate
  `PlatformWalletInfo` is exposed mutably through `PlatformWallet::state_mut()` and `state_mut_blocking()`, so downstream safe Rust can replace this `Arc<WalletGeneration>`. `PlatformWallet` and `CoreWallet` retain the original Arc, causing the inner manager's generation identity and lifecycle gate to diverge from every existing handle. `is_current_generation()` then rejects the otherwise live wallet, generation-bound reservation cleanup becomes a no-op, and teardown can exclude through a different gate than payment operations. This synchronization identity must not be publicly assignable.

In `packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedPlatformWallet.kt`:
- [SUGGESTION] packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedPlatformWallet.kt:316: Release the registered token when cancellation discards the result
  Native finalization registers the payment and transfers reservation ownership to a token before the blocking JNI call returns. `gate.op` uses `withContext(Dispatchers.IO)`, whose prompt-cancellation handoff can discard the newly constructed `SignedCoreTransaction` when the caller is cancelled while JNI is running. The only remaining cleanup is then the GC Cleaner, so the token and funding reservation can remain held until an unpredictable GC cycle or the reservation TTL. Use the existing `opWithCleanupOnCancellation` helper to close a completed result that cancellation prevents from reaching the caller.

In `packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/errors/DashSdkError.kt`:
- [SUGGESTION] packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/errors/DashSdkError.kt:296-309: Preserve definitive broadcast rejection as a typed Kotlin error
  A definitive deferred-payment broadcast rejection is preserved as native code 26 through the C ABI and JNI, but this switch falls through to `PlatformWallet.Generic`. Callers can inspect `Generic.nativeCode` manually, but the public typed hierarchy otherwise distinguishes ambiguous broadcast outcomes and all deferred-token failures. The new `broadcastSigned` API should likewise expose definitive rejection as a dedicated subtype so callers can distinguish a consumed-and-released rejected payment, which must be rebuilt or corrected, from unrelated generic wallet failures. Add the subtype, map code 26, and pin its non-retry-in-place/rebuild semantics in `DashSdkErrorTest`.

[thepastaclaw]

🔴 Blocking: Do not merge workspace-wide dependencies from a personal fork

All eight rust-dashcore workspace dependencies are redirected from dashpay/rust-dashcore to bfoss765/rust-dashcore at e99959ce. That revision is only the first commit of dashpay/rust-dashcore#916, which is currently OPEN with mergeStateStatus=BLOCKED and reviewDecision=REVIEW_REQUIRED; the upstream PR has since added further fixes, including owner-guarded cleanup for asset-lock downstream failures. Merging this head would therefore make the entire workspace depend on a contributor-controlled repository and a stale, unmerged implementation. Land the reservation API and its follow-up fixes in the Dash-owned repository, then repin these entries and Cargo.lock to the corresponding Dash-owned revision.

_{source: ['codex']}

[thepastaclaw]

🟡 Suggestion: Keep the generation identity immutable outside the crate

PlatformWalletInfo is exposed mutably through PlatformWallet::state_mut() and state_mut_blocking(), so downstream safe Rust can replace this Arc<WalletGeneration>. PlatformWallet and CoreWallet retain the original Arc, causing the inner manager's generation identity and lifecycle gate to diverge from every existing handle. is_current_generation() then rejects the otherwise live wallet, generation-bound reservation cleanup becomes a no-op, and teardown can exclude through a different gate than payment operations. This synchronization identity must not be publicly assignable.

Suggested change

	pub generation: Arc<WalletGeneration>,
	pub(crate) generation: Arc<WalletGeneration>,

_{source: ['codex']}

[thepastaclaw]

🟡 Suggestion: Release the registered token when cancellation discards the result

Native finalization registers the payment and transfers reservation ownership to a token before the blocking JNI call returns. gate.op uses withContext(Dispatchers.IO), whose prompt-cancellation handoff can discard the newly constructed SignedCoreTransaction when the caller is cancelled while JNI is running. The only remaining cleanup is then the GC Cleaner, so the token and funding reservation can remain held until an unpredictable GC cycle or the reservation TTL. Use the existing opWithCleanupOnCancellation helper to close a completed result that cancellation prevents from reaching the caller.

Suggested change

	): SignedCoreTransaction = gate.op {
	): SignedCoreTransaction = gate.opWithCleanupOnCancellation(
	cleanup = { payment: SignedCoreTransaction -> payment.close() },
	) {

_{source: ['codex']}

[thepastaclaw]

🟡 Suggestion: Preserve definitive broadcast rejection as a typed Kotlin error

A definitive deferred-payment broadcast rejection is preserved as native code 26 through the C ABI and JNI, but this switch falls through to PlatformWallet.Generic. Callers can inspect Generic.nativeCode manually, but the public typed hierarchy otherwise distinguishes ambiguous broadcast outcomes and all deferred-token failures. The new broadcastSigned API should likewise expose definitive rejection as a dedicated subtype so callers can distinguish a consumed-and-released rejected payment, which must be rebuilt or corrected, from unrelated generic wallet failures. Add the subtype, map code 26, and pin its non-retry-in-place/rebuild semantics in DashSdkErrorTest.

_{source: ['codex']}

[bfoss765]

Blocker 1 — wallet_lifecycle.rs:655-687, same-ID registration racing the two-map removal: confirmed and fixed in 6be67488.

The finding is correct, and the mechanism is worth naming precisely: registration is deliberately ungated — register_wallet mints its own WalletGeneration (:186) and never takes a teardown gate, so a create never queues behind an unrelated wallet's removal. The consequence is that from the moment wm.remove_wallet (:672) frees the id, nothing orders a concurrent same-id register_wallet against this remover's own self.wallets acquisition (:683) — the two are separated only by a guard drop, with no lock and no await between them.

A remover descheduled there (OS preemption on a multi-threaded executor, or tokio's cooperative budget making RwLock::write() reschedule even on a free permit) resumes into a map that already names G2, removes it by key, returns it, and hands it to tear_down — which sweeps G2's registry tokens and V2 handles while holding only G1's gate. G2 then survives in wallet_manager but is gone from self.wallets: get_wallet returns None and a re-issued remove_wallet returns WalletNotFound, yet is_current_generation() reads the inner manager and still reports live, so G2's retained handles stay broadcastable. load_from_persistor is a second ungated publisher with the same wm-then-wallets shape.

I took the second option from the finding: the retry loop now carries the validated Arc<PlatformWallet> out, and the public-map entry is removed only while it still Arc::ptr_eq-matches that generation (:696-719, :762-780), with a warn! on the mismatch branch. The removed handle, the returned handle and the tear_down argument are now all the one generation this call validated. This reuses the existing generation-identity primitive rather than adding a parallel mechanism, adds no lock, and leaves lock order (gate -> manager) and the "gate not held across a signer await" property untouched.

I did not take the "single per-ID ordering" option by holding self.wallets.write() across the wallet_manager.write() acquisition. It would work, but wallet_manager.write() is SPV-contended and tokio's RwLock is write-preferring, so a remover queued on wm while holding wallets would stall BalanceUpdateHandler and the sync managers on the SPV event path.

I also did not add an identity check to the inner-manager removal. While this call holds G1's gate and G1 occupies the id, G1 can only leave wallet_manager through this same method or through a registration/load rollback for an insert that could not have happened while G1 was present — I checked all five wm.remove_wallet call sites and both insert_wallet sites. That branch would be unreachable and untestable. The reasoning is recorded in the method doc.

One thing left as-is and now documented: the shielded coordinator detach is still id-keyed and has no generation concept. A just-registered generation has not run bind_shielded and so holds no coordinator entry to detach — but that is an argument from timing, not from structure, and generation-scoping the coordinator is a separate change.

Regression test. removal_leaves_a_generation_registered_during_it_intact drives the real create_wallet_from_seed_bytes -> register_wallet path from a #[cfg(test)] rendezvous fired in the exact window.

On why a rendezvous rather than a lock-timing test: the window is bounded by two different locks, and the only lock a test could hold to park the remover inside it is self.wallets — the same lock the registration needs in order to publish. Parking the remover therefore also blocks the registration, and tokio's FIFO writer queue then puts the remover first. No sleep, barrier, or completion-order ticket can pin this ordering. With the rendezvous there is no concurrency in the test at all — one task, one fixed interleaving. That matters here: two earlier tests on this PR were written in a completion-order style and passed against provably broken code on some runs.

Against the previous code it fails deterministically (10/10) on all three load-bearing assertions: the returned generation is G2, the tear_down argument is G2, and the re-registered wallet is gone from the public map. Neither the static nor its call site exists in a non-test build.

cargo fmt --check clean; cargo test -p platform-wallet-ffi -p platform-wallet green — 514 + 212 unit, 9 + 26 + 6 integration, 0 failed. All 7 remove_wallet_lifecycle_tests pass, ErrorReservationWalletMismatch = 30 unchanged, HandleStorage::any stays removed with remove_matching retained.

Blocker 2 — Cargo.toml:55-62 fork pin: acknowledged, not addressed here. It clears when dashpay/rust-dashcore#916 merges and the entries plus Cargo.lock are repinned to the Dash-owned revision. #916 is approved, 35/36 green and mergeable, waiting only on a human CODEOWNER.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

packages/rs-platform-wallet-ffi/src/core_wallet/transaction_builder.rs (1)

154-173: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract the shared generation-gate block into one helper.

Lines 154-173 and lines 276-295 are the same gate: acquire generation_payment_guard, read is_current_generation, abandon on a dead generation, return NotFound. Only the message text differs. Both blocks must stay identical for the teardown invariant to hold on both the V2-handle path and the deferred-token path. A single helper removes the risk that a future change lands on one path only.

♻️ Suggested helper

/// Acquire this generation's payment gate and verify the generation is still
/// current. Returns `Err(result)` after reconciling the build's reservation
/// when the wallet was removed or re-created during signing.
fn enter_live_generation<'a>(
    wallet: &'a platform_wallet::PlatformWallet,
    finalized: &platform_wallet::SignedCoreTransaction,
    not_found_message: &str,
) -> Result<tokio::sync::RwLockReadGuard<'a, ()>, PlatformWalletFFIResult> {
    let (lifecycle, live) = runtime().block_on(async {
        let gate = wallet.core().generation_payment_guard().await;
        let live = wallet.core().is_current_generation().await;
        (gate, live)
    });
    if live {
        return Ok(lifecycle);
    }
    drop(lifecycle);
    runtime().block_on(wallet.core().abandon_transaction(finalized));
    Err(PlatformWalletFFIResult::err(
        PlatformWalletFFIResultCode::NotFound,
        not_found_message.to_string(),
    ))
}

Then each call site becomes:

-    let (_lifecycle, wallet_is_live) = runtime().block_on(async {
-        let gate = wallet.core().generation_payment_guard().await;
-        let live = wallet.core().is_current_generation().await;
-        (gate, live)
-    });
-    if !wallet_is_live {
-        runtime().block_on(wallet.core().abandon_transaction(&finalized));
-        return PlatformWalletFFIResult::err(
-            PlatformWalletFFIResultCode::NotFound,
-            "wallet is no longer registered in the manager (removed or re-created while the \
-             transaction was being signed); no transaction handle was published and its \
-             reservation was reconciled"
-                .to_string(),
-        );
-    }
+    let _lifecycle = match enter_live_generation(
+        &wallet,
+        &finalized,
+        "wallet is no longer registered in the manager (removed or re-created while the \
+         transaction was being signed); no transaction handle was published and its \
+         reservation was reconciled",
+    ) {
+        Ok(guard) => guard,
+        Err(result) => return result,
+    };

🤖 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/rs-platform-wallet-ffi/src/core_wallet/transaction_builder.rs`
around lines 154 - 173, Extract the duplicated generation gate and
dead-generation reconciliation from the V2-handle and deferred-token paths into
a shared enter_live_generation helper. Have it acquire and return the payment
guard for live generations, otherwise release the guard, abandon the finalized
transaction, and return NotFound using the caller-provided message. Replace both
existing blocks with this helper while preserving each path’s distinct error
message and guard lifetime.

🤖 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.

Inline comments:
In `@packages/rs-platform-wallet-ffi/src/error.rs`:
- Around line 205-209: Record the reserved error-code assignments in a durable
registry by creating or updating ERROR_CODE_REGISTRY.md to map
ErrorAssetLockInsufficientFunds to 29 and ErrorReservationWalletMismatch to 30,
or remove the stale registry reference from the ErrorReservationWalletMismatch
documentation.

---

Nitpick comments:
In `@packages/rs-platform-wallet-ffi/src/core_wallet/transaction_builder.rs`:
- Around line 154-173: Extract the duplicated generation gate and
dead-generation reconciliation from the V2-handle and deferred-token paths into
a shared enter_live_generation helper. Have it acquire and return the payment
guard for live generations, otherwise release the guard, abandon the finalized
transaction, and return NotFound using the caller-provided message. Replace both
existing blocks with this helper while preserving each path’s distinct error
message and guard lifetime.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: fcb68778-c02c-41fa-9170-df3e834cd62f

📥 Commits

Reviewing files that changed from the base of the PR and between 0b0d5c7 and 6be6748.

📒 Files selected for processing (25)

• packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/errors/DashSdkError.kt
• packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/WalletManagerNative.kt
• packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/errors/DashSdkErrorTest.kt
• packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/wallet/SignedCoreTransactionTest.kt
• packages/rs-platform-wallet-ffi/src/core_wallet/broadcast.rs
• packages/rs-platform-wallet-ffi/src/core_wallet/signed_payment.rs
• packages/rs-platform-wallet-ffi/src/core_wallet/transaction_builder.rs
• packages/rs-platform-wallet-ffi/src/error.rs
• packages/rs-platform-wallet-ffi/src/handle.rs
• packages/rs-platform-wallet-ffi/src/manager.rs
• packages/rs-platform-wallet/src/manager/load.rs
• packages/rs-platform-wallet/src/manager/wallet_lifecycle.rs
• packages/rs-platform-wallet/src/test_support.rs
• packages/rs-platform-wallet/src/wallet/apply.rs
• packages/rs-platform-wallet/src/wallet/asset_lock/sync/recovery.rs
• packages/rs-platform-wallet/src/wallet/core/generation.rs
• packages/rs-platform-wallet/src/wallet/core/mod.rs
• packages/rs-platform-wallet/src/wallet/core/transaction.rs
• packages/rs-platform-wallet/src/wallet/core/wallet.rs
• packages/rs-platform-wallet/src/wallet/identity/network/contact_requests.rs
• packages/rs-platform-wallet/src/wallet/platform_wallet.rs
• packages/rs-platform-wallet/src/wallet/platform_wallet_traits.rs
• packages/rs-platform-wallet/src/wallet/signed_payment_registry.rs
• packages/rs-unified-sdk-jni/src/wallet_manager.rs
• packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swift

💤 Files with no reviewable changes (1)

• packages/rs-platform-wallet-ffi/src/handle.rs

🚧 Files skipped from review as they are similar to previous changes (2)

• packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/wallet/SignedCoreTransactionTest.kt
• packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/WalletManagerNative.kt

[coderabbitai]

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Confirm the registry file exists and records the 29 / 30 reservation.
fd -t f 'ERROR_CODE_REGISTRY.md'
fd -t f 'ERROR_CODE_REGISTRY.md' --exec rg -n '\b(27|28|29|30)\b' {} \;

# Confirm no other in-tree variant already claims 29 or 30 in this enum.
fd -t f 'error.rs' -p 'rs-platform-wallet-ffi' --exec rg -n '= (27|28|29|30),' {} \;

Repository: dashpay/platform

Length of output: 264

---

🏁 Script executed:

#!/bin/bash
set -e

echo "Files named ERROR_CODE_REGISTRY.md:"
fd -t f 'ERROR_CODE_REGISTRY\.md$' . || true

echo
echo "Error files under packages/rs-platform-wallet-ffi:"
git ls-files 'packages/rs-platform-wallet-ffi/**' | grep -E '(^|/)error\.rs$|ERROR_CODE_REGISTRY\.md$' || true

echo
echo "Relevant enum lines:"
if [ -f packages/rs-platform-wallet-ffi/src/error.rs ]; then
  nl -ba packages/rs-platform-wallet-ffi/src/error.rs | sed -n '180,220p'
fi

echo
echo "All error.rs files under packages/rs-platform-wallet-ffi:"
git ls-files 'packages/rs-platform-wallet-ffi/**/error.rs' | while read -r f; do
  echo "--- $f"
  rg -n '= (27|28|29|30),' "$f" || true
done

echo
echo "Registry mentions of 29/30 if file exists:"
if [ -f packages/rs-platform-wallet-ffi/ERROR_CODE_REGISTRY.md ]; then
  rg -n '\b(27|28|29|30)\b' packages/rs-platform-wallet-ffi/ERROR_CODE_REGISTRY.md || true
else
  echo "packages/rs-platform-wallet-ffi/ERROR_CODE_REGISTRY.md not found"
fi

echo
echo "Repo-wide ERROR_CODE_REGISTRY occurrences:"
rg -n 'ErrorAssetLockInsufficientFunds|ErrorReservationWalletMismatch|code registry|29|30' .github packages/rs-platform-wallet-ffi 2>/dev/null | head -200 || true

Repository: dashpay/platform

Length of output: 8171

---

🏁 Script executed:

#!/bin/bash
set -ed

echo "Files named ERROR_CODE_REGISTRY.md:"
fd -t f 'ERROR_CODE_REGISTRY\.md$' . || true

echo
echo "Error files under packages/rs-platform-wallet-ffi:"
git ls-files 'packages/rs-platform-wallet-ffi/**' | grep -E '(^|/)error\.rs$|ERROR_CODE_REGISTRY\.md$' || true

echo
echo "Relevant enum lines:"
if [ -f packages/rs-platform-wallet-ffi/src/error.rs ]; then
  nl -ba packages/rs-platform-wallet-ffi/src/error.rs | sed -n '180,220p'
fi

echo
echo "All error.rs files under packages/rs-platform-wallet-ffi:"
git ls-files 'packages/rs-platform-wallet-ffi/**/error.rs' | while read -r f; do
  echo "--- $f"
  rg -n '= (27|28|29|30),' "$f" || true
done

echo
echo "Registry mentions of 29/30 if file exists:"
if [ -f packages/rs-platform-wallet-ffi/ERROR_CODE_REGISTRY.md ]; then
  rg -n '\b(27|28|29|30)\b' packages/rs-platform-wallet-ffi/ERROR_CODE_REGISTRY.md || true
else
  echo "packages/rs-platform-wallet-ffi/ERROR_CODE_REGISTRY.md not found"
fi

echo
echo "Repo-wide ERROR_CODE_REGISTRY occurrences:"
rg -n 'ErrorAssetLockInsufficientFunds|ErrorReservationWalletMismatch|code registry|29|30' .github packages/rs-platform-wallet-ffi 2>/dev/null | head -200 || true

Repository: dashpay/platform

Length of output: 8284

---

Document the reserved error codes in a durable source of truth.

packages/rs-platform-wallet-ffi/ERROR_CODE_REGISTRY.md is not present, so the 29/30 split remains unrecorded outside of code comments. Create or link the registry to record ErrorAssetLockInsufficientFunds as 29 and ErrorReservationWalletMismatch as 30, or remove the referenced registry link.

🤖 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/rs-platform-wallet-ffi/src/error.rs` around lines 205 - 209, Record
the reserved error-code assignments in a durable registry by creating or
updating ERROR_CODE_REGISTRY.md to map ErrorAssetLockInsufficientFunds to 29 and
ErrorReservationWalletMismatch to 30, or remove the stale registry reference
from the ErrorReservationWalletMismatch documentation.