You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The Kotlin/FFI stack currently grows two independent ways to obtain a signed L1 payment, introduced by two PRs in the same stack:
1. build_signed_payment — build-only (#4247). CoreWallet::build_signed_payment(outputs, fee_per_kb, signer, funding_path) selects inputs from one named funds account, signs, and returns the raw bytes plus fee and change. It does not broadcast and does not persist a debit; it reserves the selected inputs in that account's ReservationSet and leaves the reservation held. This is the transition-era primitive the Android wallet already calls: dashj keeps maybeCommitTx (CrowdNode, memos, confidence listeners) and commits/broadcasts the bytes itself.
2. finalize_signed_payment_from_funding_path — token-minting (#4256).
Stacked on #4185 + #4247, this mints a #4185ReservationToken alongside the transaction, so the reservation lifecycle (finalize → broadcast-or-abandon, staleness, single consumption) is carried by a typed handle rather than by convention.
Both produce "a signed payment from a funding path". The difference is who owns the reservation afterwards — an implicit account-ledger entry keyed by the transaction's own outpoints, or an explicit token.
Merge order: this belongs after #4184 (canonical funding_path semantics, owns code 29) and #4185 (owns the reservation lifecycle this lacks), rebased on both. At that point build_signed_payment should mint a #4185ReservationToken alongside the bytes instead of returning a bare byte[] — that closes item 3 and collapses the two "signed payment" concepts into one.
This is the right end state. One concept, one lifecycle, one thing to document and test.
Two reasons, neither of them a disagreement with the goal.
Reshaping build_signed_payment's return type breaks a surface that is already consumed. The bare-bytes shape is what the Android wallet calls today through ManagedPlatformWallet.buildSignedPayment, and the whole point of that primitive during the dashj→SDK transition is that dashj — not the SDK — owns commit and broadcast. Handing back a token that must be consumed or abandoned imposes SDK-side reservation lifecycle on a caller that deliberately does not have one yet. That migration is worth doing, but it is app-side work with its own testing, not a drive-by change inside a review round.
It would hard-couple #4247 to #4185.ReservationToken is #4185's type. Minting one from build_signed_payment makes #4247 unmergeable until #4185 lands, and #4185 has been open since 2026-07-21 without an approving review (REVIEW_REQUIRED). Blocking a reviewed, otherwise-complete PR on an unapproved one costs real time for a refactor that is strictly better done once the token semantics have settled under review.
The safety concern is closed separately, so nothing is left exposed by waiting. Item 3 of the same review flagged the actual hazard — with no release/abandon export, an abandoned build stranded its inputs for RESERVATION_TTL_BLOCKS = 24 (~1h), and indefinitely while last_processed_height() == 0, since ReservationSet::sweep early-returns at height 0. That is fixed in #4247 itself by a standalone export across all four layers (CoreWallet::release_payment_reservation → core_wallet_release_payment_reservation → coreWalletReleasePaymentReservation → ManagedPlatformWallet.releasePaymentReservation), which is idempotent, a no-op after a successful broadcast, and consults no height — so it works pre-sync where the TTL backstop cannot. The unification below is therefore an API-shape cleanup, not an outstanding safety gap.
Have build_signed_payment mint a ReservationToken alongside the signed bytes, and fold finalize_signed_payment_from_funding_path into it so there is a single entry point.
Current state — two "signed payment" surfaces
The Kotlin/FFI stack currently grows two independent ways to obtain a signed L1 payment, introduced by two PRs in the same stack:
1.
build_signed_payment— build-only (#4247).CoreWallet::build_signed_payment(outputs, fee_per_kb, signer, funding_path)selects inputs from one named funds account, signs, and returns the raw bytes plus fee and change. It does not broadcast and does not persist a debit; it reserves the selected inputs in that account'sReservationSetand leaves the reservation held. This is the transition-era primitive the Android wallet already calls: dashj keepsmaybeCommitTx(CrowdNode, memos, confidence listeners) and commits/broadcasts the bytes itself.2.
finalize_signed_payment_from_funding_path— token-minting (#4256).Stacked on #4185 + #4247, this mints a #4185
ReservationTokenalongside the transaction, so the reservation lifecycle (finalize → broadcast-or-abandon, staleness, single consumption) is carried by a typed handle rather than by convention.Both produce "a signed payment from a funding path". The difference is who owns the reservation afterwards — an implicit account-ledger entry keyed by the transaction's own outpoints, or an explicit token.
The suggestion
From @shumkov's review of #4247 (comment):
This is the right end state. One concept, one lifecycle, one thing to document and test.
Why it was deferred rather than done in #4247
Two reasons, neither of them a disagreement with the goal.
Reshaping
build_signed_payment's return type breaks a surface that is already consumed. The bare-bytes shape is what the Android wallet calls today throughManagedPlatformWallet.buildSignedPayment, and the whole point of that primitive during the dashj→SDK transition is that dashj — not the SDK — owns commit and broadcast. Handing back a token that must be consumed or abandoned imposes SDK-side reservation lifecycle on a caller that deliberately does not have one yet. That migration is worth doing, but it is app-side work with its own testing, not a drive-by change inside a review round.It would hard-couple #4247 to #4185.
ReservationTokenis #4185's type. Minting one frombuild_signed_paymentmakes #4247 unmergeable until #4185 lands, and #4185 has been open since 2026-07-21 without an approving review (REVIEW_REQUIRED). Blocking a reviewed, otherwise-complete PR on an unapproved one costs real time for a refactor that is strictly better done once the token semantics have settled under review.The safety concern is closed separately, so nothing is left exposed by waiting. Item 3 of the same review flagged the actual hazard — with no release/abandon export, an abandoned build stranded its inputs for
RESERVATION_TTL_BLOCKS = 24(~1h), and indefinitely whilelast_processed_height() == 0, sinceReservationSet::sweepearly-returns at height 0. That is fixed in #4247 itself by a standalone export across all four layers (CoreWallet::release_payment_reservation→core_wallet_release_payment_reservation→coreWalletReleasePaymentReservation→ManagedPlatformWallet.releasePaymentReservation), which is idempotent, a no-op after a successful broadcast, and consults no height — so it works pre-sync where the TTL backstop cannot. The unification below is therefore an API-shape cleanup, not an outstanding safety gap.Proposed follow-up
Once #4185 has landed:
build_signed_paymentmint aReservationTokenalongside the signed bytes, and foldfinalize_signed_payment_from_funding_pathinto it so there is a single entry point.release_payment_reservationexport once every caller holds a token, or keep it as the token'sabandonimplementation.Sequencing note: this should not start before #4185 is approved, since step 1 depends on the final
ReservationTokenshape.References
build_signed_payment(build-only primitive; contains the standalone release/abandon export)finalize_signed_payment_from_funding_path(token-minting finalize)ReservationTokenand the reservation lifecycle this would unify ontofunding_path/ single-account funding semantics both surfaces rely on🤖 Generated with Claude Code