From e3df6b21fa1f45b5b160608868784928c134ccab Mon Sep 17 00:00:00 2001 From: bfoss765 <38437574+bfoss765@users.noreply.github.com> Date: Fri, 10 Jul 2026 16:56:35 -0400 Subject: [PATCH 1/7] feat(kotlin-sdk): split build/broadcast with reservation release for BIP70 deferred submission MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BIP70/BIP270 (CTX/DashSpend) sends must sign, POST the raw bytes to a merchant server, and broadcast only on ack — structurally impossible on the one-shot `sendToAddresses`. Expose the existing internal build/broadcast split with an explicit reservation lifecycle, keeping `CoreTransactionBuilder` internal so the manager stays the sole driver of the setFunding/buildSigned race. Rust core (rs-platform-wallet): - New `SignedPaymentRegistry`: a generic, in-memory registry that owns a built+signed tx and its held UTXO reservation between build and submission, keyed by an opaque `ReservationToken`. `broadcast` removes the entry before sending (no double-broadcast — a repeat/concurrent call gets `StaleToken`), binds each token to its originating wallet instance (`Arc::ptr_eq` on the shared `WalletManager`, so a re-created wallet is rejected), and reconciles the reservation on failure via the existing release-on-rejection path. `release` is idempotent. Reservations are memory-only, so a crash between build and broadcast drops both the entry and the reservation on restart — the same property dashj has. - `CoreWallet::release_transaction_reservation` — the explicit "abandoned / nacked" release arm. FFI (platform-wallet-ffi) — additive C ABI: - `core_wallet_transaction_get_bytes`, `core_wallet_signed_payment_register` (token + fee + txid), `core_wallet_signed_payment_broadcast`, `core_wallet_signed_payment_release`, backed by one process-global registry pinned to `SpvBroadcaster`. - New `ErrorStaleReservationToken` (22) result code. JNI (rs-unified-sdk-jni) — additive: `coreTransactionGetBytes`, `coreWalletRegisterSignedPayment` (BLOB), `coreWalletBroadcastSignedPayment`, `coreWalletReleaseSignedPayment`. Kotlin — additive: `ManagedPlatformWallet.SignedCoreTransaction`, `buildSignedPayment` (build under coreSendMutex), `broadcastSigned(token)`, `releaseReservation(token)`; `DashSdkError.PlatformWallet.StaleReservationToken`. No existing signatures change. Refs dashpay/platform#4089, dashpay/dash-wallet#1507 Phase 5c GAP-4. Co-Authored-By: Claude Fable 5 --- .../dashsdk/errors/DashSdkError.kt | 15 + .../dashsdk/ffi/WalletManagerNative.kt | 44 ++ .../dashsdk/wallet/ManagedCoreWallet.kt | 39 + .../dashsdk/wallet/ManagedPlatformWallet.kt | 137 ++++ .../dashsdk/errors/DashSdkErrorTest.kt | 10 + .../src/core_wallet/mod.rs | 2 + .../src/core_wallet/signed_payment.rs | 199 +++++ .../src/core_wallet/transaction_builder.rs | 5 + packages/rs-platform-wallet-ffi/src/error.rs | 9 + packages/rs-platform-wallet/src/lib.rs | 3 + .../src/wallet/core/broadcast.rs | 34 +- packages/rs-platform-wallet/src/wallet/mod.rs | 4 + .../src/wallet/signed_payment_registry.rs | 688 ++++++++++++++++++ .../rs-unified-sdk-jni/src/wallet_manager.rs | 175 +++++ 14 files changed, 1363 insertions(+), 1 deletion(-) create mode 100644 packages/rs-platform-wallet-ffi/src/core_wallet/signed_payment.rs create mode 100644 packages/rs-platform-wallet/src/wallet/signed_payment_registry.rs diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/errors/DashSdkError.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/errors/DashSdkError.kt index de41a05412a..bfa54c7fd66 100644 --- a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/errors/DashSdkError.kt +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/errors/DashSdkError.kt @@ -176,6 +176,20 @@ sealed class DashSdkError( cause, ) + /** + * `ErrorStaleReservationToken` (native code 26). A deferred + * (BIP70/BIP270) [broadcastSigned][org.dashfoundation.dashsdk.wallet.ManagedPlatformWallet.broadcastSigned] + * was given a reservation token that is unknown, already broadcast, + * already released, or was minted against a re-created wallet instance. + * The call did NOT touch the network — there is no double-broadcast — + * but the token can never succeed, so this is NOT retryable: rebuild the + * payment with + * [buildSignedPayment][org.dashfoundation.dashsdk.wallet.ManagedPlatformWallet.buildSignedPayment]. + * (Release is idempotent and never raises this.) + */ + class StaleReservationToken(message: String, cause: Throwable? = null) : + PlatformWallet(message, cause) + /** * Any other `PlatformWalletFFIResultCode` without a dedicated type. * Carries the platform-wallet [nativeCode] (already de-offset) and @@ -245,6 +259,7 @@ sealed class DashSdkError( 23 -> PlatformWallet.AssetLockNotTracked(message, cause) // ErrorAssetLockNotTracked 24 -> PlatformWallet.AssetLockAlreadyConsumed(message, cause) // ErrorAssetLockAlreadyConsumed 25 -> PlatformWallet.AssetLockFundingMismatch(message, cause) // ErrorAssetLockFundingMismatch + 26 -> PlatformWallet.StaleReservationToken(message, cause) // ErrorStaleReservationToken else -> PlatformWallet.Generic(code, message, cause) } } diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/WalletManagerNative.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/WalletManagerNative.kt index 0dfbbedc89d..daee4c76091 100644 --- a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/WalletManagerNative.kt +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/WalletManagerNative.kt @@ -246,6 +246,50 @@ internal object WalletManagerNative { */ external fun coreTransactionFree(tx: Long) + /** + * `core_wallet_transaction_get_bytes` — the consensus-serialized bytes of a + * transaction from [coreTxBuilderBuildSigned], copied into a fresh + * `ByteArray`. The transaction handle must still be live (not yet freed by + * [coreTransactionFree]). + */ + external fun coreTransactionGetBytes(tx: Long): ByteArray + + /** + * `core_wallet_signed_payment_register` — register a built+signed + * transaction (from [coreTxBuilderBuildSigned]) for deferred + * (BIP70/BIP270) submission, holding its UTXO reservation. Does NOT consume + * the transaction — free it separately with [coreTransactionFree]. + * [accountType]/[accountIndex] identify the funding account (0 BIP44, + * 1 BIP32, 2 CoinJoin). + * + * Returns a big-endian BLOB decoded into a `SignedCoreTransaction`: + * `u64 token, u64 feeDuffs, u32 txidLen, txid utf8`. The raw tx bytes come + * from [coreTransactionGetBytes]. + */ + external fun coreWalletRegisterSignedPayment( + coreHandle: Long, + tx: Long, + accountType: Int, + accountIndex: Int, + ): ByteArray + + /** + * `core_wallet_signed_payment_broadcast` — broadcast the payment behind + * [token], reconciling its reservation on failure and consuming the token. + * A repeated/stale/wrong-wallet token throws + * `ErrorStaleReservationToken` (never a double-broadcast). [coreHandle] must + * resolve to the wallet the token was minted against. Returns the txid as a + * lowercase hex string. + */ + external fun coreWalletBroadcastSignedPayment(coreHandle: Long, token: Long): String + + /** + * `core_wallet_signed_payment_release` — release the funding reservation + * behind [token] and drop it. Idempotent: releasing an unknown / + * already-consumed token is a silent no-op. + */ + external fun coreWalletReleaseSignedPayment(token: Long) + /** * Enumerate the wallet's Platform-payment addresses with cached credit * balances, as a big-endian blob: `u32 rowCount` then per row diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedCoreWallet.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedCoreWallet.kt index 8a0e661d0ed..0b9c2bf588a 100644 --- a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedCoreWallet.kt +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedCoreWallet.kt @@ -55,6 +55,45 @@ class ManagedCoreWallet internal constructor(handle: Long) : AutoCloseable { ) } + /** + * Register a built+signed [tx] for deferred (BIP70/BIP270) submission, + * holding its UTXO reservation, and return the resulting + * [ManagedPlatformWallet.SignedCoreTransaction]. Does NOT consume [tx] — the + * caller still closes it. Reads the raw bytes off [tx] and decodes the + * register BLOB (`token, feeDuffs, txid`). + */ + internal fun registerSignedPayment( + tx: CoreTransaction, + ): ManagedPlatformWallet.SignedCoreTransaction { + val rawTxBytes = WalletManagerNative.coreTransactionGetBytes(tx.handle) + val blob = WalletManagerNative.coreWalletRegisterSignedPayment( + handle, + tx.handle, + tx.accountType.ffiValue, + tx.accountIndex, + ) + val buffer = java.nio.ByteBuffer.wrap(blob) // big-endian by default + val token = buffer.long + val feeDuffs = buffer.long + val txidLen = buffer.int + val txidBytes = ByteArray(txidLen) + buffer.get(txidBytes) + return ManagedPlatformWallet.SignedCoreTransaction( + txidHex = String(txidBytes, Charsets.UTF_8), + rawTxBytes = rawTxBytes, + feeDuffs = feeDuffs, + reservationToken = token, + ) + } + + /** + * Broadcast the deferred payment behind [token] and return its txid. A + * stale / already-broadcast / wrong-wallet token surfaces as + * [org.dashfoundation.dashsdk.errors.DashSdkError.PlatformWallet.StaleReservationToken]. + */ + internal fun broadcastSignedPayment(token: Long): String = + WalletManagerNative.coreWalletBroadcastSignedPayment(handle, token) + override fun close() { cleanable.clean() } diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedPlatformWallet.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedPlatformWallet.kt index ba05a9ff7ff..ec692911416 100644 --- a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedPlatformWallet.kt +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedPlatformWallet.kt @@ -172,6 +172,143 @@ class ManagedPlatformWallet internal constructor( } } + /** + * A built, signed Core transaction whose funding UTXOs are reserved, + * awaiting a deferred [broadcastSigned] or [releaseReservation] — the + * split-out result of [buildSignedPayment] for BIP70/BIP270 (CTX/DashSpend) + * flows that must sign now, POST the raw bytes to a merchant server, and + * broadcast only on the server's ack. + * + * @property txidHex the transaction id (lowercase hex) the broadcast will + * return — computed from the signed bytes Rust-side so it matches exactly. + * @property rawTxBytes the consensus-serialized signed transaction, to hand + * to the merchant server. + * @property feeDuffs the fee the build charged, in duffs. + * @property reservationToken the opaque token for [broadcastSigned] / + * [releaseReservation]. Valid only for this wallet instance and only until + * consumed by one of those calls. + */ + class SignedCoreTransaction internal constructor( + val txidHex: String, + val rawTxBytes: ByteArray, + val feeDuffs: Long, + val reservationToken: Long, + ) { + override fun equals(other: Any?): Boolean = + other is SignedCoreTransaction && + txidHex == other.txidHex && + rawTxBytes.contentEquals(other.rawTxBytes) && + feeDuffs == other.feeDuffs && + reservationToken == other.reservationToken + + override fun hashCode(): Int { + var result = txidHex.hashCode() + result = 31 * result + rawTxBytes.contentHashCode() + result = 31 * result + feeDuffs.hashCode() + result = 31 * result + reservationToken.hashCode() + return result + } + + override fun toString(): String = + "SignedCoreTransaction(txidHex=$txidHex, feeDuffs=$feeDuffs, " + + "reservationToken=$reservationToken, rawTxBytes=${rawTxBytes.size} bytes)" + } + + /** + * Build and sign a Core payment to [recipients] WITHOUT broadcasting, + * reserving the funding UTXOs and returning a [SignedCoreTransaction] whose + * [SignedCoreTransaction.reservationToken] later drives [broadcastSigned] + * (server acked) or [releaseReservation] (abandoned / server nacked). + * + * The BIP70/BIP270 counterpart to [sendToAddresses]: those protocols sign, + * POST the raw bytes to a merchant server, and broadcast only on ack, which + * a single build-sign-broadcast call cannot express. The `new → addOutput* → + * setFunding → buildSigned` build runs under the same per-wallet + * [coreSendMutex] as [sendToAddresses] (closing the setFunding/buildSigned + * selection race); [buildSigned] reserves the selected UTXOs, so once this + * returns the reservation holds the inputs and [broadcastSigned] / + * [releaseReservation] operate on the token later WITHOUT the mutex. + * + * Process-death note: the reservation is in-memory. An app crash between + * this call and [broadcastSigned] drops the reservation on restart (the + * UTXOs become spendable again) — the same property dashj has. + * + * @param network the wallet network — see [sendToAddresses]. + * @param coreSignerHandle the manager's `MnemonicResolverHandle` — see + * [sendToAddresses]. No private key crosses the boundary. + */ + suspend fun buildSignedPayment( + recipients: List>, + network: org.dashfoundation.dashsdk.Network, + coreSignerHandle: Long, + accountType: AccountType = AccountType.BIP44, + accountIndex: Int = 0, + ): SignedCoreTransaction = withContext(Dispatchers.IO) { + require(accountIndex >= 0) { "accountIndex must be non-negative, got $accountIndex" } + require(recipients.isNotEmpty()) { "recipients must not be empty" } + require(recipients.all { it.second > 0 }) { + "every recipient amount must be positive" + } + val builderAccountType = when (accountType) { + AccountType.BIP44 -> CoreTransactionBuilder.AccountType.BIP44 + AccountType.BIP32 -> CoreTransactionBuilder.AccountType.BIP32 + } + coreSendMutex.withLock { + mapNativeErrors { + coreWallet().use { core -> + val builder = CoreTransactionBuilder(network) + // `buildSigned` consumes the builder; `use` still safely + // destroys it on the pre-build failure paths. + val signedTx = builder.use { + for ((address, amount) in recipients) { + it.addOutput(address, amount) + } + it.setFunding(this@ManagedPlatformWallet, builderAccountType, accountIndex) + it.buildSigned( + this@ManagedPlatformWallet, + builderAccountType, + accountIndex, + coreSignerHandle, + ) + } + // Register the signed tx (holding its reservation) before the + // native transaction is freed; `use` frees it afterward. + signedTx.use { tx -> core.registerSignedPayment(tx) } + } + } + } + } + + /** + * Broadcast the deferred payment behind [token] (from [buildSignedPayment]) + * and return its broadcast txid — the "merchant server acked" arm. Consumes + * the token: a second [broadcastSigned] with the same token, or one for a + * re-created wallet, throws + * [org.dashfoundation.dashsdk.errors.DashSdkError.PlatformWallet.StaleReservationToken] + * rather than double-broadcasting. Operates on the token WITHOUT the + * [coreSendMutex] (the inputs are already reserved). + */ + suspend fun broadcastSigned(token: Long): String = withContext(Dispatchers.IO) { + mapNativeErrors { + coreWallet().use { core -> core.broadcastSignedPayment(token) } + } + } + + /** + * Release the funding reservation behind [token] (from [buildSignedPayment]) + * — the "payment abandoned / merchant server nacked" arm — returning the + * reserved UTXOs to spendable. Idempotent: releasing an unknown / + * already-broadcast / already-released token is a silent no-op, so it is + * always safe to call defensively. + */ + suspend fun releaseReservation(token: Long) { + withContext(Dispatchers.IO) { + mapNativeErrors { + WalletManagerNative.coreWalletReleaseSignedPayment(token) + } + } + } + /** * The wallet's Platform-payment addresses that currently hold credits, * each as a [FundingInput] whose `credits` is the full cached balance — diff --git a/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/errors/DashSdkErrorTest.kt b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/errors/DashSdkErrorTest.kt index f8e397cade5..502b1d274f8 100644 --- a/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/errors/DashSdkErrorTest.kt +++ b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/errors/DashSdkErrorTest.kt @@ -103,6 +103,16 @@ class DashSdkErrorTest { ) // The message must warn against retrying (distinct from the anchor case). assertTrue(broadcastUnconfirmed.message!!.contains("do NOT retry")) + + // Deferred build/broadcast: a stale/consumed/wrong-wallet reservation + // token → typed StaleReservationToken, not retryable. + val staleToken = DashSdkError.fromNative(DashSDKException(offset + 22, "stale token 7")) + assertTrue(staleToken is DashSdkError.PlatformWallet.StaleReservationToken) + assertFalse( + "StaleReservationToken must NOT be retryable (rebuild the payment)", + staleToken.isRetryable, + ) + assertEquals("stale token 7", staleToken.message) } @Test diff --git a/packages/rs-platform-wallet-ffi/src/core_wallet/mod.rs b/packages/rs-platform-wallet-ffi/src/core_wallet/mod.rs index 8e12ebc1783..5a3dc9d3554 100644 --- a/packages/rs-platform-wallet-ffi/src/core_wallet/mod.rs +++ b/packages/rs-platform-wallet-ffi/src/core_wallet/mod.rs @@ -4,10 +4,12 @@ mod addresses; mod broadcast; +mod signed_payment; mod transaction_builder; mod wallet; pub use addresses::*; pub use broadcast::*; +pub use signed_payment::*; pub use transaction_builder::*; pub use wallet::*; diff --git a/packages/rs-platform-wallet-ffi/src/core_wallet/signed_payment.rs b/packages/rs-platform-wallet-ffi/src/core_wallet/signed_payment.rs new file mode 100644 index 00000000000..e2ffee6c3c7 --- /dev/null +++ b/packages/rs-platform-wallet-ffi/src/core_wallet/signed_payment.rs @@ -0,0 +1,199 @@ +//! FFI bindings for the deferred build → broadcast/release core-send lifecycle +//! (BIP70 / BIP270 "sign now, submit on merchant ack"). +//! +//! The one-shot [`core_wallet_broadcast_transaction`](super::broadcast) sends a +//! just-built transaction immediately. BIP70-style flows must split that: build +//! and sign now (reserving the funding UTXOs), hand the raw bytes to a merchant +//! server, then broadcast only on ack — or release the reservation on a nack / +//! abandonment. These entry points wrap a single process-global +//! [`SignedPaymentRegistry`] pinned to the production `SpvBroadcaster`; the +//! registry owns the built transaction and its held reservation between build +//! and submission and enforces the lifecycle invariants (no double-broadcast, +//! idempotent release, tokens bound to their originating wallet instance). +//! +//! These are ADDITIVE to the existing `core_wallet_tx_builder_*` / +//! `core_wallet_broadcast_transaction` surface — the immediate send path is +//! unchanged. + +use super::transaction_builder::{CoreAccountTypeFFI, FFICoreTransaction}; +use crate::error::*; +use crate::handle::{Handle, CORE_WALLET_STORAGE}; +use crate::runtime::runtime; +use crate::{check_ptr, unwrap_option_or_return}; +use once_cell::sync::Lazy; +use platform_wallet::broadcaster::SpvBroadcaster; +use platform_wallet::{ReservationToken, SignedPaymentError, SignedPaymentRegistry}; +use std::ffi::CString; +use std::os::raw::c_char; + +/// Process-global registry of signed-but-unsent payments, keyed by an opaque +/// [`ReservationToken`]. In-memory only: an app crash between build and +/// broadcast drops the registry entry and the underlying UTXO reservation +/// together, so nothing leaks across a restart. +static SIGNED_PAYMENT_REGISTRY: Lazy> = + Lazy::new(SignedPaymentRegistry::new); + +/// Borrow the consensus-serialized bytes of a transaction built by +/// `core_wallet_tx_builder_build_signed`, for the caller to copy into +/// `SignedCoreTransaction.rawTxBytes`. +/// +/// The written pointer borrows the `FFICoreTransaction`'s own buffer — it is +/// valid only until the transaction is freed with +/// `core_wallet_transaction_free`, so the caller must copy the bytes out +/// immediately and must not retain the pointer. +/// +/// # Safety +/// `tx` must be a valid, non-freed `FFICoreTransaction` pointer; +/// `out_ptr`/`out_len` must be writable. +#[no_mangle] +pub unsafe extern "C" fn core_wallet_transaction_get_bytes( + tx: *const FFICoreTransaction, + out_ptr: *mut *const u8, + out_len: *mut usize, +) -> PlatformWalletFFIResult { + check_ptr!(tx); + check_ptr!(out_ptr); + check_ptr!(out_len); + + let bytes = (*tx).bytes(); + *out_ptr = bytes.as_ptr(); + *out_len = bytes.len(); + PlatformWalletFFIResult::ok() +} + +/// Register a built, signed transaction for deferred submission and return a +/// reservation token. +/// +/// `core_wallet_tx_builder_build_signed` already reserved the funding UTXOs; the +/// registry takes its own copy of the transaction and holds the reservation +/// (via the captured wallet instance behind `core_handle`) until a later +/// [`core_wallet_signed_payment_broadcast`] or +/// [`core_wallet_signed_payment_release`]. The passed `tx` is NOT consumed — the +/// caller still frees it with `core_wallet_transaction_free`. +/// +/// `account_type`/`account_index` identify the funding account handed to +/// `set_funding`, so the reservation can be released on rejection/abandonment. +/// Writes `out_token`, `out_fee` (the build's fee in duffs), and `out_txid` (a +/// heap-allocated lowercase-hex C string the caller frees with +/// `core_wallet_free_address`). +/// +/// # Safety +/// `tx` must be a valid, non-freed `FFICoreTransaction`; `core_handle` a valid +/// core-wallet handle; the three out-pointers must be writable. +#[no_mangle] +pub unsafe extern "C" fn core_wallet_signed_payment_register( + core_handle: Handle, + tx: *const FFICoreTransaction, + account_type: CoreAccountTypeFFI, + account_index: u32, + out_token: *mut u64, + out_fee: *mut u64, + out_txid: *mut *mut c_char, +) -> PlatformWalletFFIResult { + check_ptr!(tx); + check_ptr!(out_token); + check_ptr!(out_fee); + check_ptr!(out_txid); + + let core = unwrap_option_or_return!(CORE_WALLET_STORAGE.with_item(core_handle, |w| w.clone())); + + let transaction: dashcore::Transaction = match dashcore::consensus::deserialize((*tx).bytes()) { + Ok(t) => t, + Err(e) => { + return PlatformWalletFFIResult::err( + PlatformWalletFFIResultCode::ErrorDeserialization, + format!("failed to deserialize signed transaction: {e}"), + ); + } + }; + let txid = transaction.txid(); + let fee = (*tx).fee(); + + let token = SIGNED_PAYMENT_REGISTRY.register( + core, + transaction, + account_type.as_standard_account_type(), + account_index, + ); + + // txid hex never contains a NUL, but handle the impossible case anyway. + let c_txid = match CString::new(txid.to_string()) { + Ok(s) => s, + Err(_) => { + return PlatformWalletFFIResult::err( + PlatformWalletFFIResultCode::ErrorUtf8Conversion, + "txid string contained an interior NUL".to_string(), + ); + } + }; + + *out_token = token; + *out_fee = fee; + *out_txid = c_txid.into_raw(); + PlatformWalletFFIResult::ok() +} + +/// Broadcast the payment behind `token` (built earlier via +/// [`core_wallet_signed_payment_register`]), reconciling its UTXO reservation on +/// failure, and consume the token. +/// +/// The token is consumed atomically before the send, so a repeated or +/// concurrent broadcast of the same token gets `ErrorStaleReservationToken` +/// rather than a second send. `core_handle` must resolve to the same wallet +/// instance the token was minted against; a re-created wallet yields +/// `ErrorStaleReservationToken`. Writes `out_txid` (a heap C string freed with +/// `core_wallet_free_address`) on success. +/// +/// # Safety +/// `core_handle` must be a valid core-wallet handle; `out_txid` must be writable. +#[no_mangle] +pub unsafe extern "C" fn core_wallet_signed_payment_broadcast( + core_handle: Handle, + token: u64, + out_txid: *mut *mut c_char, +) -> PlatformWalletFFIResult { + check_ptr!(out_txid); + + let core = unwrap_option_or_return!(CORE_WALLET_STORAGE.with_item(core_handle, |w| w.clone())); + + let result = runtime().block_on(SIGNED_PAYMENT_REGISTRY.broadcast(token as ReservationToken, &core)); + + match result { + Ok(txid) => { + let c_txid = match CString::new(txid.to_string()) { + Ok(s) => s, + Err(_) => { + return PlatformWalletFFIResult::err( + PlatformWalletFFIResultCode::ErrorUtf8Conversion, + "txid string contained an interior NUL".to_string(), + ); + } + }; + *out_txid = c_txid.into_raw(); + PlatformWalletFFIResult::ok() + } + Err(e @ (SignedPaymentError::StaleToken(_) | SignedPaymentError::WalletMismatch(_))) => { + PlatformWalletFFIResult::err( + PlatformWalletFFIResultCode::ErrorStaleReservationToken, + e.to_string(), + ) + } + // Preserve the typed underlying wallet error (keeps the ambiguous + // "may already be on the network" retry semantics intact). + Err(SignedPaymentError::Broadcast(e)) => PlatformWalletFFIResult::from(e), + } +} + +/// Release the funding reservation behind `token` and drop it — the "payment +/// abandoned / merchant server nacked" arm. Idempotent: releasing an unknown / +/// already-consumed token is a silent success, so it never surfaces +/// `ErrorStaleReservationToken`. Needs no wallet handle: the release acts on the +/// wallet instance the token was minted against. +/// +/// # Safety +/// Always safe to call; `token` is a plain value. +#[no_mangle] +pub unsafe extern "C" fn core_wallet_signed_payment_release(token: u64) -> PlatformWalletFFIResult { + runtime().block_on(SIGNED_PAYMENT_REGISTRY.release(token as ReservationToken)); + PlatformWalletFFIResult::ok() +} diff --git a/packages/rs-platform-wallet-ffi/src/core_wallet/transaction_builder.rs b/packages/rs-platform-wallet-ffi/src/core_wallet/transaction_builder.rs index 8b79efcf2ab..3e30447cf59 100644 --- a/packages/rs-platform-wallet-ffi/src/core_wallet/transaction_builder.rs +++ b/packages/rs-platform-wallet-ffi/src/core_wallet/transaction_builder.rs @@ -59,6 +59,11 @@ impl FFICoreTransaction { unsafe { std::slice::from_raw_parts(self.tx_bytes, self.tx_len) } } } + + /// The fee (duffs) `build_signed` computed for this transaction. + pub(crate) fn fee(&self) -> u64 { + self.fee + } } #[derive(Clone, Copy)] diff --git a/packages/rs-platform-wallet-ffi/src/error.rs b/packages/rs-platform-wallet-ffi/src/error.rs index 7d01177b5b7..b20f6122c6a 100644 --- a/packages/rs-platform-wallet-ffi/src/error.rs +++ b/packages/rs-platform-wallet-ffi/src/error.rs @@ -168,6 +168,15 @@ pub enum PlatformWalletFFIResultCode { /// Existing-lock recovery attempted to use a lock for the wrong funding /// family or bound identity index. ErrorAssetLockFundingMismatch = 25, + /// Maps `SignedPaymentError::StaleToken` / `SignedPaymentError::WalletMismatch` + /// from the deferred build → broadcast/release core-send lifecycle + /// (`core_wallet_signed_payment_*`). The reservation token is unknown, + /// already broadcast, already released, or was minted against a different + /// (re-created) wallet instance. The operation did NOT touch the network — + /// there is no double-broadcast — but the token can never succeed, so this + /// is NOT retryable: the host must rebuild the payment. Release is + /// idempotent and never surfaces this code. + ErrorStaleReservationToken = 26, NotFound = 98, // Used exclusively for all the Option that are retuned as errors ErrorUnknown = 99, diff --git a/packages/rs-platform-wallet/src/lib.rs b/packages/rs-platform-wallet/src/lib.rs index e91c5ccee0a..273ba9e82af 100644 --- a/packages/rs-platform-wallet/src/lib.rs +++ b/packages/rs-platform-wallet/src/lib.rs @@ -58,6 +58,9 @@ pub use wallet::asset_lock::tracked::{AssetLockStatus, TrackedAssetLock}; pub use wallet::asset_lock::AssetLockFunding; pub use wallet::core::WalletBalance; pub use wallet::core::{CoreWallet, SignedCoreTransaction}; +pub use wallet::signed_payment_registry::{ + ReservationToken, SignedPaymentError, SignedPaymentRegistry, +}; // DashPay types + crypto helpers re-exported through the identity // domain (they live under `identity::types::dashpay::*` and // `identity::crypto::*` internally). diff --git a/packages/rs-platform-wallet/src/wallet/core/broadcast.rs b/packages/rs-platform-wallet/src/wallet/core/broadcast.rs index 0f3d7fd1f0d..55466a1dcf7 100644 --- a/packages/rs-platform-wallet/src/wallet/core/broadcast.rs +++ b/packages/rs-platform-wallet/src/wallet/core/broadcast.rs @@ -3,7 +3,9 @@ use key_wallet::account::account_type::StandardAccountType; use super::SignedCoreTransaction; use crate::broadcaster::TransactionBroadcaster; -use crate::wallet::reservations::broadcast_releasing_on_rejection; +use crate::wallet::reservations::{ + broadcast_releasing_on_rejection, release_reservation_after_rejected_broadcast, +}; use crate::{CoreWallet, PlatformWalletError}; impl CoreWallet { @@ -86,6 +88,36 @@ impl CoreWallet { .await .map_err(Into::into) } + + /// Release the funding account's UTXO reservation for `transaction` without + /// broadcasting — the "payment abandoned / merchant server nacked" arm of + /// the deferred build → broadcast/release lifecycle + /// ([`SignedPaymentRegistry`](crate::SignedPaymentRegistry)). + /// + /// `build_signed` reserves the selected inputs and leaves the reservation + /// held; when the caller decides never to broadcast, this returns those + /// inputs to spendable so a later build can reselect them. Idempotent at the + /// account layer (releasing an already-released reservation is a no-op), and + /// best-effort: a missing wallet/account is logged, not surfaced, since + /// there is nothing actionable to reconcile. + /// + /// `account_type`/`account_index` identify the funding account handed to + /// `set_funding` when the transaction was built. + pub async fn release_transaction_reservation( + &self, + account_type: StandardAccountType, + account_index: u32, + transaction: &Transaction, + ) { + release_reservation_after_rejected_broadcast( + &self.wallet_manager, + &self.wallet_id, + account_type, + account_index, + transaction, + ) + .await + } } #[cfg(test)] diff --git a/packages/rs-platform-wallet/src/wallet/mod.rs b/packages/rs-platform-wallet/src/wallet/mod.rs index 1963422be7c..43457733a33 100644 --- a/packages/rs-platform-wallet/src/wallet/mod.rs +++ b/packages/rs-platform-wallet/src/wallet/mod.rs @@ -11,9 +11,13 @@ pub mod provider_key_at_index; pub(crate) mod reservations; #[cfg(feature = "shielded")] pub mod shielded; +pub mod signed_payment_registry; pub mod tokens; pub use self::core::CoreWallet; +pub use signed_payment_registry::{ + ReservationToken, SignedPaymentError, SignedPaymentRegistry, +}; pub use apply::ApplyError; pub use core_address_key::CoreAddressPrivateKey; pub use identity::IdentityWallet; diff --git a/packages/rs-platform-wallet/src/wallet/signed_payment_registry.rs b/packages/rs-platform-wallet/src/wallet/signed_payment_registry.rs new file mode 100644 index 00000000000..ece79d13867 --- /dev/null +++ b/packages/rs-platform-wallet/src/wallet/signed_payment_registry.rs @@ -0,0 +1,688 @@ +//! In-memory registry backing the deferred build → broadcast/release core-send +//! lifecycle (BIP70 / BIP270 "sign now, submit on merchant ack"). +//! +//! The regular send path +//! ([`CoreWallet::broadcast_transaction_releasing_reservation`](crate::CoreWallet::broadcast_transaction_releasing_reservation)) +//! builds, signs, and broadcasts in one uninterrupted step. BIP70-style flows +//! must split that: sign now (reserving the funding UTXOs), hand the raw bytes +//! to a merchant server, and broadcast **only** once the server acks — or +//! release the reservation if it nacks / the user abandons. +//! +//! `TransactionBuilder::build_signed` already reserves the selected UTXOs in the +//! funding account's `ReservationSet` and leaves the reservation held on +//! success (see [`crate::wallet::reservations`]). This registry owns the built +//! transaction and its held reservation between build and submission, keyed by +//! an opaque [`ReservationToken`], and enforces the lifecycle invariants: +//! +//! * [`broadcast`](SignedPaymentRegistry::broadcast) removes the entry **before** +//! sending, so a repeated or concurrent broadcast of the same token can never +//! double-broadcast — the second caller finds nothing and gets +//! [`SignedPaymentError::StaleToken`]. +//! * [`release`](SignedPaymentRegistry::release) is idempotent: releasing an +//! unknown / already-consumed token is a silent no-op. +//! * A token is bound to the exact wallet instance it was minted against +//! (`Arc::ptr_eq` on the shared `WalletManager`). Broadcasting it through a +//! re-created wallet — whose in-memory `ReservationSet` no longer holds the +//! inputs — is a [`SignedPaymentError::WalletMismatch`] rather than a spend +//! against stale state. +//! +//! ## Process-death semantics +//! +//! The registry and the underlying `ReservationSet` are both in-memory. An app +//! crash between build and broadcast drops the registry entry **and** the +//! reservation together, so nothing leaks across a restart — the UTXOs are +//! spendable again on reload. This matches dashj's behaviour (its in-flight +//! reservations are likewise memory-only). No on-disk reservation persistence +//! exists to follow. + +use std::collections::HashMap; +use std::sync::atomic::{AtomicU64, Ordering}; +use std::sync::{Arc, Mutex}; + +use dashcore::{Transaction, Txid}; +use key_wallet::account::account_type::StandardAccountType; + +use crate::broadcaster::TransactionBroadcaster; +use crate::wallet::core::CoreWallet; +use crate::PlatformWalletError; + +/// Opaque handle to a registered, signed-but-unsent payment. Minted by +/// [`SignedPaymentRegistry::register`]; consumed by +/// [`SignedPaymentRegistry::broadcast`] or +/// [`SignedPaymentRegistry::release`]. Values are unique for the process +/// lifetime and never reused, so a stale token can always be recognised. +pub type ReservationToken = u64; + +/// Failure of a deferred broadcast/release token operation. +#[derive(Debug, thiserror::Error)] +pub enum SignedPaymentError { + /// The token is unknown, already broadcast, or already released. The + /// registry never re-broadcasts, so this is the guard that turns a + /// double-broadcast into a typed error instead of a second send. + #[error("reservation token {0} is unknown, already broadcast, or already released")] + StaleToken(ReservationToken), + + /// The token was minted against a different (re-created) wallet instance + /// than the one it is being broadcast through. Its reservation lives in + /// that other instance's `ReservationSet`, so submitting it here would spend + /// against state this wallet never reserved. + #[error("reservation token {0} was minted against a different wallet instance")] + WalletMismatch(ReservationToken), + + /// The underlying broadcast failed. Carries the still-typed wallet error so + /// the FFI boundary can preserve the retry semantics (e.g. the ambiguous + /// [`PlatformWalletError::TransactionBroadcastUnconfirmed`] "may already be + /// on the network" signal). + #[error(transparent)] + Broadcast(#[from] PlatformWalletError), +} + +/// A built, signed transaction whose funding UTXOs are reserved, awaiting a +/// deferred broadcast or an explicit release. +struct RegisteredPayment { + /// The wallet instance the payment was built against — captured so the + /// broadcast/release act on the exact `ReservationSet` that holds the + /// inputs, and so a re-created wallet can be detected via `Arc::ptr_eq`. + core: CoreWallet, + /// The signed transaction to broadcast. + tx: Transaction, + /// The funding account whose reservation must be released on a rejected + /// broadcast or an explicit release. `None` for a CoinJoin funding, which + /// has no standard-account reservation to reconcile (it rides the + /// TTL backstop), mirroring `CoreAccountTypeFFI::as_standard_account_type`. + account_type: Option, + account_index: u32, +} + +/// Registry of signed-but-unsent payments keyed by [`ReservationToken`]. +/// +/// Generic over the broadcaster `B` so it can be unit-tested with mock +/// broadcasters; the FFI layer instantiates a single process-global registry +/// pinned to the production `SpvBroadcaster`. +pub struct SignedPaymentRegistry { + next_token: AtomicU64, + entries: Mutex>>, +} + +impl Default for SignedPaymentRegistry { + fn default() -> Self { + Self::new() + } +} + +impl SignedPaymentRegistry { + /// A fresh, empty registry. + pub fn new() -> Self { + Self { + // Start at 1 so 0 is never a valid token (matches the FFI's + // null-handle convention). + next_token: AtomicU64::new(1), + entries: Mutex::new(HashMap::new()), + } + } + + /// Take ownership of a built, signed `tx` (whose funding UTXOs `build_signed` + /// already reserved) and return an opaque token for a later + /// [`broadcast`](Self::broadcast) or [`release`](Self::release). + /// + /// `core` is the wallet the payment was built against; it is captured so the + /// later operation acts on the exact reservation state that holds the inputs. + pub fn register( + &self, + core: CoreWallet, + tx: Transaction, + account_type: Option, + account_index: u32, + ) -> ReservationToken { + let token = self.next_token.fetch_add(1, Ordering::SeqCst); + self.entries + .lock() + .expect("signed-payment registry mutex poisoned") + .insert( + token, + RegisteredPayment { + core, + tx, + account_type, + account_index, + }, + ); + token + } + + /// Broadcast the payment behind `token`, reconciling its UTXO reservation on + /// failure, then consume the token. + /// + /// The entry is removed **before** the send, so a repeated or concurrent + /// broadcast of the same token gets [`SignedPaymentError::StaleToken`] + /// instead of a second send. `current` must be the same wallet instance the + /// token was minted against (checked by `Arc::ptr_eq` on the shared + /// `WalletManager`); otherwise the call fails with + /// [`SignedPaymentError::WalletMismatch`] and the stale token is dropped. + /// + /// On a definitive rejection the reservation is released for an immediate + /// rebuild; on an ambiguous ("may already be on the network") failure it is + /// kept — the same policy as the non-deferred send path. + pub async fn broadcast( + &self, + token: ReservationToken, + current: &CoreWallet, + ) -> Result { + // Remove under the lock and drop the guard *before* awaiting — a + // std::Mutex guard must never be held across an await point, and the + // atomic take is what makes a double-broadcast impossible. + let entry = { + let mut entries = self + .entries + .lock() + .expect("signed-payment registry mutex poisoned"); + entries.remove(&token) + } + .ok_or(SignedPaymentError::StaleToken(token))?; + + if !Arc::ptr_eq(&entry.core.wallet_manager, ¤t.wallet_manager) { + // The token belongs to another wallet instance; it has been removed, + // so it can never be replayed here. + return Err(SignedPaymentError::WalletMismatch(token)); + } + + let txid = match entry.account_type { + Some(account_type) => { + entry + .core + .broadcast_transaction_releasing_reservation( + account_type, + entry.account_index, + &entry.tx, + ) + .await? + } + None => entry.core.broadcast_transaction(&entry.tx).await?, + }; + Ok(txid) + } + + /// Release the funding reservation behind `token` and drop it. Idempotent: + /// releasing an unknown / already-consumed token is a silent no-op, so a + /// double release (or a release after a broadcast) is harmless. + /// + /// The release acts on the wallet instance the token was minted against — + /// the one whose `ReservationSet` actually holds the inputs — so no wallet + /// handle need be threaded in. + pub async fn release(&self, token: ReservationToken) { + let entry = { + let mut entries = self + .entries + .lock() + .expect("signed-payment registry mutex poisoned"); + entries.remove(&token) + }; + let Some(entry) = entry else { + // Unknown / already consumed — idempotent no-op. + return; + }; + if let Some(account_type) = entry.account_type { + entry + .core + .release_transaction_reservation(account_type, entry.account_index, &entry.tx) + .await; + } + } + + /// Number of outstanding (registered but not yet broadcast/released) tokens. + #[cfg(test)] + pub(crate) fn outstanding(&self) -> usize { + self.entries + .lock() + .expect("signed-payment registry mutex poisoned") + .len() + } +} + +#[cfg(test)] +mod tests { + use std::sync::atomic::{AtomicUsize, Ordering}; + use std::sync::{Arc, Mutex}; + + use async_trait::async_trait; + use dashcore::{Address as DashAddress, Network, Transaction, Txid}; + use key_wallet::account::account_type::StandardAccountType; + use key_wallet::managed_account::managed_account_trait::ManagedAccountTrait; + use key_wallet::signer::Signer; + use key_wallet::wallet::managed_wallet_info::coin_selection::SelectionStrategy; + use key_wallet::wallet::managed_wallet_info::transaction_builder::TransactionBuilder; + use key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; + + use super::{SignedPaymentError, SignedPaymentRegistry}; + use crate::broadcaster::{BroadcastError, TransactionBroadcaster}; + use crate::test_support::{funded_wallet_manager, AlwaysMaybeSentBroadcaster, WalletSigner}; + use crate::wallet::core::CoreWallet; + use crate::PlatformWalletError; + + /// Broadcaster that records the exact bytes handed to it and succeeds, + /// so a test can assert the broadcast tx is byte-identical to the one the + /// caller registered. + struct RecordingBroadcaster { + sent: Mutex>>, + } + + impl RecordingBroadcaster { + fn new() -> Self { + Self { + sent: Mutex::new(Vec::new()), + } + } + + fn last_sent(&self) -> Option> { + self.sent.lock().unwrap().last().cloned() + } + } + + #[async_trait] + impl TransactionBroadcaster for RecordingBroadcaster { + async fn broadcast(&self, transaction: &Transaction) -> Result { + self.sent + .lock() + .unwrap() + .push(dashcore::consensus::serialize(transaction)); + Ok(transaction.txid()) + } + } + + /// Broadcaster that counts how many times it was asked to send. + struct CountingBroadcaster { + count: AtomicUsize, + } + + impl CountingBroadcaster { + fn new() -> Self { + Self { + count: AtomicUsize::new(0), + } + } + } + + #[async_trait] + impl TransactionBroadcaster for CountingBroadcaster { + async fn broadcast(&self, transaction: &Transaction) -> Result { + self.count.fetch_add(1, Ordering::SeqCst); + Ok(transaction.txid()) + } + } + + /// A testnet `CoreWallet` over the shared funded fixture plus a + /// 1_000_000-duff payment to a dummy recipient. + async fn funded_core_wallet( + account_type: StandardAccountType, + broadcaster: Arc, + ) -> (CoreWallet, WalletSigner, Vec<(DashAddress, u64)>) { + let (wallet_manager, wallet_id, balance, signer) = + funded_wallet_manager(account_type).await; + let sdk = Arc::new(dash_sdk::SdkBuilder::new_mock().build().expect("mock sdk")); + let core = CoreWallet::new(sdk, wallet_manager, wallet_id, broadcaster, balance); + let recipient = DashAddress::dummy(Network::Testnet, 42); + (core, signer, vec![(recipient, 1_000_000u64)]) + } + + /// Build + sign a payment exactly as the deferred send path does: + /// `build_signed` reserves the inputs and leaves the reservation held for + /// the later broadcast/release. + async fn build_signed_tx( + core: &CoreWallet, + account_type: StandardAccountType, + account_index: u32, + outputs: &[(DashAddress, u64)], + signer: &S, + ) -> Result { + let mut wm = core.wallet_manager.write().await; + let (wallet, info) = wm + .get_wallet_and_info_mut(&core.wallet_id()) + .expect("wallet present in manager"); + let current_height = info.core_wallet.synced_height(); + let (managed_account, account) = match account_type { + StandardAccountType::BIP44Account => ( + info.core_wallet + .accounts + .standard_bip44_accounts + .get_mut(&account_index) + .expect("bip44 managed account"), + wallet + .accounts + .standard_bip44_accounts + .get(&account_index) + .expect("bip44 account"), + ), + StandardAccountType::BIP32Account => ( + info.core_wallet + .accounts + .standard_bip32_accounts + .get_mut(&account_index) + .expect("bip32 managed account"), + wallet + .accounts + .standard_bip32_accounts + .get(&account_index) + .expect("bip32 account"), + ), + }; + let mut builder = TransactionBuilder::new() + .set_current_height(current_height) + .set_selection_strategy(SelectionStrategy::LargestFirst) + .set_funding(managed_account, account); + for (addr, amount) in outputs { + builder = builder.add_output(addr, *amount); + } + let (tx, _fee) = builder + .build_signed(signer, |addr| managed_account.address_derivation_path(&addr)) + .await + .map_err(|e| PlatformWalletError::TransactionBuild(e.to_string()))?; + Ok(tx) + } + + /// Happy path: a registered token broadcasts the exact bytes it was built + /// with, and the token is consumed afterwards. + #[tokio::test] + async fn build_then_broadcast_sends_registered_bytes() { + let broadcaster = Arc::new(RecordingBroadcaster::new()); + let (core, signer, outputs) = + funded_core_wallet(StandardAccountType::BIP44Account, Arc::clone(&broadcaster)).await; + let registry = SignedPaymentRegistry::new(); + + let tx = build_signed_tx(&core, StandardAccountType::BIP44Account, 0, &outputs, &signer) + .await + .expect("build should succeed"); + let expected_bytes = dashcore::consensus::serialize(&tx); + let expected_txid = tx.txid(); + + let token = registry.register( + core.clone(), + tx, + Some(StandardAccountType::BIP44Account), + 0, + ); + assert_eq!(registry.outstanding(), 1); + + // Broadcast through a *clone* of the same wallet instance — the + // wallet-identity guard must accept it (same `Arc`). + let txid = registry + .broadcast(token, &core.clone()) + .await + .expect("broadcast should succeed"); + + assert_eq!(txid, expected_txid, "returned txid must match the built tx"); + assert_eq!( + broadcaster.last_sent().expect("a tx was sent"), + expected_bytes, + "broadcast bytes must be byte-identical to the registered tx" + ); + assert_eq!(registry.outstanding(), 0, "token consumed after broadcast"); + } + + /// build → release makes the reserved UTXO spendable again: a subsequent + /// build can reselect the released input. + #[tokio::test] + async fn build_then_release_frees_the_reservation() { + for account_type in [ + StandardAccountType::BIP44Account, + StandardAccountType::BIP32Account, + ] { + let broadcaster = Arc::new(RecordingBroadcaster::new()); + let (core, signer, outputs) = funded_core_wallet(account_type, broadcaster).await; + let registry = SignedPaymentRegistry::new(); + + let tx = build_signed_tx(&core, account_type, 0, &outputs, &signer) + .await + .expect("build should succeed"); + let token = registry.register(core.clone(), tx, Some(account_type), 0); + + // With the reservation held, an immediate rebuild finds no + // spendable UTXO and fails. + let blocked = build_signed_tx(&core, account_type, 0, &outputs, &signer).await; + assert!( + matches!(blocked, Err(PlatformWalletError::TransactionBuild(_))), + "rebuild must fail while the reservation is held for {account_type:?}, got {blocked:?}" + ); + + registry.release(token).await; + assert_eq!(registry.outstanding(), 0, "token consumed after release"); + + // The released input is spendable again — the rebuild succeeds. + let rebuilt = build_signed_tx(&core, account_type, 0, &outputs, &signer).await; + assert!( + rebuilt.is_ok(), + "rebuild after release should succeed for {account_type:?}, got {rebuilt:?}" + ); + } + } + + /// A second broadcast of the same token is a typed `StaleToken` error, never + /// a second send. + #[tokio::test] + async fn double_broadcast_is_a_stale_token_error() { + let broadcaster = Arc::new(CountingBroadcaster::new()); + let (core, signer, outputs) = + funded_core_wallet(StandardAccountType::BIP44Account, Arc::clone(&broadcaster)).await; + let registry = SignedPaymentRegistry::new(); + + let tx = build_signed_tx(&core, StandardAccountType::BIP44Account, 0, &outputs, &signer) + .await + .expect("build should succeed"); + let token = registry.register(core.clone(), tx, Some(StandardAccountType::BIP44Account), 0); + + registry + .broadcast(token, &core) + .await + .expect("first broadcast should succeed"); + let second = registry.broadcast(token, &core).await; + assert!( + matches!(second, Err(SignedPaymentError::StaleToken(t)) if t == token), + "second broadcast must be StaleToken, got {second:?}" + ); + assert_eq!( + broadcaster.count.load(Ordering::SeqCst), + 1, + "the network must have been hit exactly once" + ); + } + + /// Releasing twice — or releasing after a broadcast — is a harmless no-op. + #[tokio::test] + async fn double_release_is_idempotent() { + let broadcaster = Arc::new(RecordingBroadcaster::new()); + let (core, signer, outputs) = + funded_core_wallet(StandardAccountType::BIP44Account, broadcaster).await; + let registry = SignedPaymentRegistry::new(); + + let tx = build_signed_tx(&core, StandardAccountType::BIP44Account, 0, &outputs, &signer) + .await + .expect("build should succeed"); + let token = registry.register(core.clone(), tx, Some(StandardAccountType::BIP44Account), 0); + + registry.release(token).await; + // Second release: no panic, no error, still consumed. + registry.release(token).await; + assert_eq!(registry.outstanding(), 0); + } + + /// Broadcasting after a release is a `StaleToken` error (the released token + /// can never reach the network). + #[tokio::test] + async fn broadcast_after_release_is_stale() { + let broadcaster = Arc::new(CountingBroadcaster::new()); + let (core, signer, outputs) = + funded_core_wallet(StandardAccountType::BIP44Account, Arc::clone(&broadcaster)).await; + let registry = SignedPaymentRegistry::new(); + + let tx = build_signed_tx(&core, StandardAccountType::BIP44Account, 0, &outputs, &signer) + .await + .expect("build should succeed"); + let token = registry.register(core.clone(), tx, Some(StandardAccountType::BIP44Account), 0); + + registry.release(token).await; + let sent = registry.broadcast(token, &core).await; + assert!( + matches!(sent, Err(SignedPaymentError::StaleToken(_))), + "broadcast of a released token must be StaleToken, got {sent:?}" + ); + assert_eq!(broadcaster.count.load(Ordering::SeqCst), 0, "nothing was sent"); + } + + /// An unknown token is a `StaleToken` error. + #[tokio::test] + async fn unknown_token_is_stale() { + let broadcaster = Arc::new(CountingBroadcaster::new()); + let (core, _signer, _outputs) = + funded_core_wallet(StandardAccountType::BIP44Account, broadcaster).await; + let registry: SignedPaymentRegistry = SignedPaymentRegistry::new(); + + let sent = registry.broadcast(9999, &core).await; + assert!(matches!(sent, Err(SignedPaymentError::StaleToken(9999)))); + // Releasing an unknown token is a no-op, not a panic. + registry.release(9999).await; + } + + /// A token minted against one wallet instance cannot be broadcast through a + /// different (re-created) instance — its reservation lives elsewhere. + #[tokio::test] + async fn broadcast_rejects_a_different_wallet_instance() { + let broadcaster_a = Arc::new(CountingBroadcaster::new()); + let (core_a, signer_a, outputs_a) = + funded_core_wallet(StandardAccountType::BIP44Account, Arc::clone(&broadcaster_a)).await; + // A separate wallet-manager instance stands in for a re-created wallet. + let broadcaster_b = Arc::new(CountingBroadcaster::new()); + let (core_b, _signer_b, _outputs_b) = + funded_core_wallet(StandardAccountType::BIP44Account, broadcaster_b).await; + let registry = SignedPaymentRegistry::new(); + + let tx = + build_signed_tx(&core_a, StandardAccountType::BIP44Account, 0, &outputs_a, &signer_a) + .await + .expect("build should succeed"); + let token = registry.register( + core_a.clone(), + tx, + Some(StandardAccountType::BIP44Account), + 0, + ); + + let sent = registry.broadcast(token, &core_b).await; + assert!( + matches!(sent, Err(SignedPaymentError::WalletMismatch(t)) if t == token), + "broadcast through a different wallet instance must be WalletMismatch, got {sent:?}" + ); + assert_eq!( + broadcaster_a.count.load(Ordering::SeqCst), + 0, + "nothing was sent on the original wallet" + ); + assert_eq!(registry.outstanding(), 0, "the stale token is dropped"); + } + + /// An ambiguous ("may already be on the network") broadcast failure keeps + /// the reservation and surfaces the typed unconfirmed error; the token is + /// still consumed so it cannot be retried into a double-spend. + #[tokio::test] + async fn ambiguous_broadcast_keeps_reservation_and_consumes_token() { + let broadcaster = Arc::new(AlwaysMaybeSentBroadcaster); + let (core, signer, outputs) = + funded_core_wallet(StandardAccountType::BIP44Account, broadcaster).await; + let registry = SignedPaymentRegistry::new(); + + let tx = build_signed_tx(&core, StandardAccountType::BIP44Account, 0, &outputs, &signer) + .await + .expect("build should succeed"); + let token = registry.register(core.clone(), tx, Some(StandardAccountType::BIP44Account), 0); + + let sent = registry.broadcast(token, &core).await; + assert!( + matches!( + sent, + Err(SignedPaymentError::Broadcast( + PlatformWalletError::TransactionBroadcastUnconfirmed(_) + )) + ), + "ambiguous failure must surface the typed unconfirmed error, got {sent:?}" + ); + assert_eq!(registry.outstanding(), 0, "token consumed even on failure"); + + // Reservation kept: an immediate rebuild fails at input selection. + let rebuilt = build_signed_tx(&core, StandardAccountType::BIP44Account, 0, &outputs, &signer) + .await; + assert!( + matches!(rebuilt, Err(PlatformWalletError::TransactionBuild(_))), + "rebuild must fail with the reservation kept, got {rebuilt:?}" + ); + } + + /// Concurrent broadcasts of the same token serialise on the registry mutex: + /// exactly one wins, every other gets `StaleToken`, and the network is hit + /// once. + #[tokio::test(flavor = "multi_thread", worker_threads = 4)] + async fn concurrent_broadcasts_serialize_to_one_send() { + let broadcaster = Arc::new(CountingBroadcaster::new()); + let (core, signer, outputs) = + funded_core_wallet(StandardAccountType::BIP44Account, Arc::clone(&broadcaster)).await; + let registry = Arc::new(SignedPaymentRegistry::new()); + + let tx = build_signed_tx(&core, StandardAccountType::BIP44Account, 0, &outputs, &signer) + .await + .expect("build should succeed"); + let token = registry.register(core.clone(), tx, Some(StandardAccountType::BIP44Account), 0); + + let mut handles = Vec::new(); + for _ in 0..8 { + let registry = Arc::clone(®istry); + let core = core.clone(); + handles.push(tokio::spawn(async move { + registry.broadcast(token, &core).await + })); + } + let mut successes = 0; + let mut stale = 0; + for handle in handles { + match handle.await.expect("task panicked") { + Ok(_) => successes += 1, + Err(SignedPaymentError::StaleToken(_)) => stale += 1, + Err(other) => panic!("unexpected error: {other:?}"), + } + } + assert_eq!(successes, 1, "exactly one broadcast must win"); + assert_eq!(stale, 7, "every other broadcast must be StaleToken"); + assert_eq!( + broadcaster.count.load(Ordering::SeqCst), + 1, + "the network must have been hit exactly once" + ); + } + + /// Concurrent registrations hand out distinct tokens. + #[tokio::test(flavor = "multi_thread", worker_threads = 4)] + async fn concurrent_registers_yield_distinct_tokens() { + let broadcaster = Arc::new(CountingBroadcaster::new()); + let (core, signer, outputs) = + funded_core_wallet(StandardAccountType::BIP44Account, broadcaster).await; + // One built tx is enough; we register clones of it many times to probe + // the token allocator, not the reservation logic. + let tx = build_signed_tx(&core, StandardAccountType::BIP44Account, 0, &outputs, &signer) + .await + .expect("build should succeed"); + let registry = Arc::new(SignedPaymentRegistry::new()); + + let mut handles = Vec::new(); + for _ in 0..16 { + let registry = Arc::clone(®istry); + let core = core.clone(); + let tx = tx.clone(); + handles.push(tokio::spawn(async move { + registry.register(core, tx, Some(StandardAccountType::BIP44Account), 0) + })); + } + let mut tokens = Vec::new(); + for handle in handles { + tokens.push(handle.await.expect("task panicked")); + } + let unique: std::collections::HashSet<_> = tokens.iter().copied().collect(); + assert_eq!(unique.len(), tokens.len(), "all tokens must be distinct"); + assert_eq!(registry.outstanding(), 16); + } +} diff --git a/packages/rs-unified-sdk-jni/src/wallet_manager.rs b/packages/rs-unified-sdk-jni/src/wallet_manager.rs index 8e0f3e676b5..d15d6a23b41 100644 --- a/packages/rs-unified-sdk-jni/src/wallet_manager.rs +++ b/packages/rs-unified-sdk-jni/src/wallet_manager.rs @@ -1232,6 +1232,181 @@ pub extern "system" fn Java_org_dashfoundation_dashsdk_ffi_WalletManagerNative_c }) } +// ── Deferred build → broadcast/release core-send (BIP70/BIP270) ─────── +// +// ADDITIVE surface over the immediate `coreWalletBroadcastTransaction` path: +// a signed transaction built by [coreTxBuilderBuildSigned] can be registered +// (reserving its UTXOs), its raw bytes handed to a merchant server, and only +// then broadcast on ack — or its reservation released on nack/abandonment. +// Backed by the process-global registry in `platform_wallet_ffi` +// (`core_wallet_signed_payment_*`). See `SignedPaymentRegistry`. + +/// `core_wallet_transaction_get_bytes` — the consensus-serialized bytes of a +/// built transaction from [coreTxBuilderBuildSigned], copied into a fresh +/// Java `byte[]`. The underlying FFI hands back a borrowed pointer valid only +/// while `tx` lives, so we copy it here before returning. +#[no_mangle] +pub extern "system" fn Java_org_dashfoundation_dashsdk_ffi_WalletManagerNative_coreTransactionGetBytes( + mut env: JNIEnv, + _class: JClass, + tx: jlong, +) -> jbyteArray { + guard(&mut env, ptr::null_mut(), |env| { + if tx == 0 { + throw_sdk_exception(env, 1, "transaction handle is 0"); + return ptr::null_mut(); + } + let mut out_ptr: *const u8 = ptr::null(); + let mut out_len: usize = 0; + let result = unsafe { + platform_wallet_ffi::core_wallet_transaction_get_bytes( + tx as *const platform_wallet_ffi::FFICoreTransaction, + &mut out_ptr as *mut *const u8, + &mut out_len as *mut usize, + ) + }; + if take_pwffi_error(env, result) { + return ptr::null_mut(); + } + // Copy immediately: the pointer borrows the transaction's own buffer. + let bytes: &[u8] = if out_ptr.is_null() || out_len == 0 { + &[] + } else { + unsafe { std::slice::from_raw_parts(out_ptr, out_len) } + }; + env.byte_array_from_slice(bytes) + .map(|a| a.into_raw()) + .unwrap_or(ptr::null_mut()) + }) +} + +/// `core_wallet_signed_payment_register` — register a built+signed transaction +/// (from [coreTxBuilderBuildSigned]) for deferred submission, holding its UTXO +/// reservation. `accountType`/`accountIndex` are the funding account (0 BIP44, +/// 1 BIP32, 2 CoinJoin). The passed `tx` is NOT consumed — free it separately +/// with [coreTransactionFree]. +/// +/// Returns a big-endian BLOB the Kotlin side decodes into a +/// `SignedCoreTransaction`: `u64 token, u64 feeDuffs, u32 txidLen, txid utf8`. +/// The raw tx bytes are fetched separately via [coreTransactionGetBytes]. +#[no_mangle] +pub extern "system" fn Java_org_dashfoundation_dashsdk_ffi_WalletManagerNative_coreWalletRegisterSignedPayment( + mut env: JNIEnv, + _class: JClass, + core_handle: jlong, + tx: jlong, + account_type: jni::sys::jint, + account_index: jni::sys::jint, +) -> jbyteArray { + guard(&mut env, ptr::null_mut(), |env| { + if tx == 0 { + throw_sdk_exception(env, 1, "transaction handle is 0"); + return ptr::null_mut(); + } + let Some(account_type) = core_account_type(account_type) else { + throw_sdk_exception(env, 1, "accountType out of range (expected 0..=2)"); + return ptr::null_mut(); + }; + if account_index < 0 { + throw_sdk_exception(env, 1, "accountIndex must be non-negative"); + return ptr::null_mut(); + } + + let mut token: u64 = 0; + let mut fee: u64 = 0; + let mut out_txid: *mut c_char = ptr::null_mut(); + let result = unsafe { + platform_wallet_ffi::core_wallet_signed_payment_register( + core_handle as Handle, + tx as *const platform_wallet_ffi::FFICoreTransaction, + account_type, + account_index as u32, + &mut token as *mut u64, + &mut fee as *mut u64, + &mut out_txid as *mut *mut c_char, + ) + }; + if take_pwffi_error(env, result) { + return ptr::null_mut(); + } + if out_txid.is_null() { + throw_sdk_exception(env, 1, "register returned a NULL txid"); + return ptr::null_mut(); + } + // Copy the txid out, then free the Rust-owned C string. + let txid = unsafe { CStr::from_ptr(out_txid) } + .to_string_lossy() + .into_owned(); + unsafe { platform_wallet_ffi::core_wallet_free_address(out_txid) }; + + // Assemble the big-endian BLOB (matches the Kotlin ByteBuffer decoder). + let txid_bytes = txid.into_bytes(); + let mut blob = Vec::with_capacity(8 + 8 + 4 + txid_bytes.len()); + blob.extend_from_slice(&token.to_be_bytes()); + blob.extend_from_slice(&fee.to_be_bytes()); + blob.extend_from_slice(&(txid_bytes.len() as u32).to_be_bytes()); + blob.extend_from_slice(&txid_bytes); + env.byte_array_from_slice(&blob) + .map(|a| a.into_raw()) + .unwrap_or(ptr::null_mut()) + }) +} + +/// `core_wallet_signed_payment_broadcast` — broadcast the payment behind +/// `token`, releasing/keeping its reservation per the broadcast outcome and +/// consuming the token. A repeated/stale token throws (native +/// `ErrorStaleReservationToken`, code 22) rather than double-broadcasting. +/// `coreHandle` must resolve to the wallet the token was minted against. +/// Returns the txid as a lowercase hex string. +#[no_mangle] +pub extern "system" fn Java_org_dashfoundation_dashsdk_ffi_WalletManagerNative_coreWalletBroadcastSignedPayment( + mut env: JNIEnv, + _class: JClass, + core_handle: jlong, + token: jlong, +) -> jstring { + guard(&mut env, ptr::null_mut(), |env| { + let mut out_txid: *mut c_char = ptr::null_mut(); + let result = unsafe { + platform_wallet_ffi::core_wallet_signed_payment_broadcast( + core_handle as Handle, + token as u64, + &mut out_txid as *mut *mut c_char, + ) + }; + if take_pwffi_error(env, result) { + return ptr::null_mut(); + } + if out_txid.is_null() { + throw_sdk_exception(env, 1, "broadcast returned a NULL txid"); + return ptr::null_mut(); + } + let txid = unsafe { CStr::from_ptr(out_txid) } + .to_string_lossy() + .into_owned(); + unsafe { platform_wallet_ffi::core_wallet_free_address(out_txid) }; + env.new_string(txid) + .map(|s| s.into_raw()) + .unwrap_or(ptr::null_mut()) + }) +} + +/// `core_wallet_signed_payment_release` — release the funding reservation +/// behind `token` and drop it. Idempotent: releasing an unknown / already- +/// consumed token is a silent no-op (never throws the stale-token error). +#[no_mangle] +pub extern "system" fn Java_org_dashfoundation_dashsdk_ffi_WalletManagerNative_coreWalletReleaseSignedPayment( + mut env: JNIEnv, + _class: JClass, + token: jlong, +) { + guard(&mut env, (), |env| { + let result = + unsafe { platform_wallet_ffi::core_wallet_signed_payment_release(token as u64) }; + let _ = take_pwffi_error(env, result); + }) +} + /// Enumerate this wallet's Platform-payment addresses with their cached /// credit balances, returning a flat `byte[]` BLOB for the top-up /// funding-input builder (`TopUpIdentityScreen`). From e09546dd68f9e071edbb723a699396c17eadea4a Mon Sep 17 00:00:00 2001 From: bfoss765 <38437574+bfoss765@users.noreply.github.com> Date: Fri, 10 Jul 2026 21:48:47 -0400 Subject: [PATCH 2/7] fix(kotlin-sdk): bound deferred-payment token lifetime; harden register/release MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review of the SignedPaymentRegistry deferred build→broadcast/release flow. BLOCKING: registry tokens never expired even though the key-wallet UTXO reservation they depend on is swept after RESERVATION_TTL_BLOCKS (24) and released by raw outpoint with no ownership check, so a long-outstanding token's broadcast/release could free or spend against an unrelated newer reservation. Bound the token lifetime: capture the wallet's synced height at register and refuse broadcast/release once the wallet has synced RESERVATION_MAX_AGE_BLOCKS (20, < TTL) past it, returning the typed StaleReservationToken WITHOUT releasing (which could free a newer build's reservation). The pinned key-wallet exposes no per-outpoint generation check, so this client-side bound is the primary guard. Also: - WalletMismatch now compares wallet_id in addition to Arc::ptr_eq on the shared WalletManager, so two wallets in one multi-wallet manager are told apart. - register() returns the raw tx bytes in the same native call and the JNI folds them into the register BLOB; the now-unused core_wallet_transaction_get_bytes / coreTransactionGetBytes is removed (one native round trip per kotlin-sdk rule). - register() does its fallible/pure marshalling before the reservation-holding insert, and the JNI releases the token if it can't hand the BLOB back to Kotlin — no orphaned reservation on a marshalling failure. - PlatformWallet teardown sweeps the registry of that wallet's tokens so a destroyed wallet's WalletManager is no longer pinned alive by a captured CoreWallet clone (hooked at platform_wallet_destroy, not the transient core-handle destroy the deferred flow cycles through). - Registry mutex recovers from poisoning instead of panicking, matching key-wallet's ReservationSet. Adds tests for token expiry (broadcast + release), same-manager different wallet_id mismatch, and the teardown sweep. Co-Authored-By: Claude Fable 5 --- .../dashsdk/ffi/WalletManagerNative.kt | 12 +- .../dashsdk/wallet/ManagedCoreWallet.kt | 8 +- .../src/core_wallet/mod.rs | 2 +- .../src/core_wallet/signed_payment.rs | 90 ++- packages/rs-platform-wallet-ffi/src/wallet.rs | 11 + .../src/wallet/core/wallet.rs | 16 + .../src/wallet/signed_payment_registry.rs | 581 +++++++++++++++--- .../rs-unified-sdk-jni/src/wallet_manager.rs | 73 +-- 8 files changed, 596 insertions(+), 197 deletions(-) diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/WalletManagerNative.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/WalletManagerNative.kt index daee4c76091..54f2cd11688 100644 --- a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/WalletManagerNative.kt +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/WalletManagerNative.kt @@ -246,14 +246,6 @@ internal object WalletManagerNative { */ external fun coreTransactionFree(tx: Long) - /** - * `core_wallet_transaction_get_bytes` — the consensus-serialized bytes of a - * transaction from [coreTxBuilderBuildSigned], copied into a fresh - * `ByteArray`. The transaction handle must still be live (not yet freed by - * [coreTransactionFree]). - */ - external fun coreTransactionGetBytes(tx: Long): ByteArray - /** * `core_wallet_signed_payment_register` — register a built+signed * transaction (from [coreTxBuilderBuildSigned]) for deferred @@ -263,8 +255,8 @@ internal object WalletManagerNative { * 1 BIP32, 2 CoinJoin). * * Returns a big-endian BLOB decoded into a `SignedCoreTransaction`: - * `u64 token, u64 feeDuffs, u32 txidLen, txid utf8`. The raw tx bytes come - * from [coreTransactionGetBytes]. + * `u64 token, u64 feeDuffs, u32 txidLen, txid utf8, u32 txBytesLen, txBytes`. + * The raw tx bytes come back in this same call — no second native round trip. */ external fun coreWalletRegisterSignedPayment( coreHandle: Long, diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedCoreWallet.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedCoreWallet.kt index 0b9c2bf588a..6961c3a093f 100644 --- a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedCoreWallet.kt +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedCoreWallet.kt @@ -59,13 +59,12 @@ class ManagedCoreWallet internal constructor(handle: Long) : AutoCloseable { * Register a built+signed [tx] for deferred (BIP70/BIP270) submission, * holding its UTXO reservation, and return the resulting * [ManagedPlatformWallet.SignedCoreTransaction]. Does NOT consume [tx] — the - * caller still closes it. Reads the raw bytes off [tx] and decodes the - * register BLOB (`token, feeDuffs, txid`). + * caller still closes it. Decodes the single register BLOB + * (`token, feeDuffs, txid, rawTxBytes`) — one native round trip. */ internal fun registerSignedPayment( tx: CoreTransaction, ): ManagedPlatformWallet.SignedCoreTransaction { - val rawTxBytes = WalletManagerNative.coreTransactionGetBytes(tx.handle) val blob = WalletManagerNative.coreWalletRegisterSignedPayment( handle, tx.handle, @@ -78,6 +77,9 @@ class ManagedCoreWallet internal constructor(handle: Long) : AutoCloseable { val txidLen = buffer.int val txidBytes = ByteArray(txidLen) buffer.get(txidBytes) + val txBytesLen = buffer.int + val rawTxBytes = ByteArray(txBytesLen) + buffer.get(rawTxBytes) return ManagedPlatformWallet.SignedCoreTransaction( txidHex = String(txidBytes, Charsets.UTF_8), rawTxBytes = rawTxBytes, diff --git a/packages/rs-platform-wallet-ffi/src/core_wallet/mod.rs b/packages/rs-platform-wallet-ffi/src/core_wallet/mod.rs index 5a3dc9d3554..01c0cf4167a 100644 --- a/packages/rs-platform-wallet-ffi/src/core_wallet/mod.rs +++ b/packages/rs-platform-wallet-ffi/src/core_wallet/mod.rs @@ -4,7 +4,7 @@ mod addresses; mod broadcast; -mod signed_payment; +pub(crate) mod signed_payment; mod transaction_builder; mod wallet; diff --git a/packages/rs-platform-wallet-ffi/src/core_wallet/signed_payment.rs b/packages/rs-platform-wallet-ffi/src/core_wallet/signed_payment.rs index e2ffee6c3c7..9fce7b11c5f 100644 --- a/packages/rs-platform-wallet-ffi/src/core_wallet/signed_payment.rs +++ b/packages/rs-platform-wallet-ffi/src/core_wallet/signed_payment.rs @@ -30,37 +30,9 @@ use std::os::raw::c_char; /// [`ReservationToken`]. In-memory only: an app crash between build and /// broadcast drops the registry entry and the underlying UTXO reservation /// together, so nothing leaks across a restart. -static SIGNED_PAYMENT_REGISTRY: Lazy> = +pub(crate) static SIGNED_PAYMENT_REGISTRY: Lazy> = Lazy::new(SignedPaymentRegistry::new); -/// Borrow the consensus-serialized bytes of a transaction built by -/// `core_wallet_tx_builder_build_signed`, for the caller to copy into -/// `SignedCoreTransaction.rawTxBytes`. -/// -/// The written pointer borrows the `FFICoreTransaction`'s own buffer — it is -/// valid only until the transaction is freed with -/// `core_wallet_transaction_free`, so the caller must copy the bytes out -/// immediately and must not retain the pointer. -/// -/// # Safety -/// `tx` must be a valid, non-freed `FFICoreTransaction` pointer; -/// `out_ptr`/`out_len` must be writable. -#[no_mangle] -pub unsafe extern "C" fn core_wallet_transaction_get_bytes( - tx: *const FFICoreTransaction, - out_ptr: *mut *const u8, - out_len: *mut usize, -) -> PlatformWalletFFIResult { - check_ptr!(tx); - check_ptr!(out_ptr); - check_ptr!(out_len); - - let bytes = (*tx).bytes(); - *out_ptr = bytes.as_ptr(); - *out_len = bytes.len(); - PlatformWalletFFIResult::ok() -} - /// Register a built, signed transaction for deferred submission and return a /// reservation token. /// @@ -73,13 +45,20 @@ pub unsafe extern "C" fn core_wallet_transaction_get_bytes( /// /// `account_type`/`account_index` identify the funding account handed to /// `set_funding`, so the reservation can be released on rejection/abandonment. -/// Writes `out_token`, `out_fee` (the build's fee in duffs), and `out_txid` (a +/// Writes `out_token`, `out_fee` (the build's fee in duffs), `out_txid` (a /// heap-allocated lowercase-hex C string the caller frees with -/// `core_wallet_free_address`). +/// `core_wallet_free_address`), and `out_bytes_ptr`/`out_bytes_len` (the +/// consensus-serialized transaction bytes, returned in the same call so the +/// caller needs no second native round trip). +/// +/// The `out_bytes_ptr` buffer borrows the `FFICoreTransaction`'s own storage — +/// it is valid only until `tx` is freed with `core_wallet_transaction_free`, so +/// the caller must copy the bytes out immediately and must not retain the +/// pointer. /// /// # Safety /// `tx` must be a valid, non-freed `FFICoreTransaction`; `core_handle` a valid -/// core-wallet handle; the three out-pointers must be writable. +/// core-wallet handle; all out-pointers must be writable. #[no_mangle] pub unsafe extern "C" fn core_wallet_signed_payment_register( core_handle: Handle, @@ -89,15 +68,20 @@ pub unsafe extern "C" fn core_wallet_signed_payment_register( out_token: *mut u64, out_fee: *mut u64, out_txid: *mut *mut c_char, + out_bytes_ptr: *mut *const u8, + out_bytes_len: *mut usize, ) -> PlatformWalletFFIResult { check_ptr!(tx); check_ptr!(out_token); check_ptr!(out_fee); check_ptr!(out_txid); + check_ptr!(out_bytes_ptr); + check_ptr!(out_bytes_len); let core = unwrap_option_or_return!(CORE_WALLET_STORAGE.with_item(core_handle, |w| w.clone())); - let transaction: dashcore::Transaction = match dashcore::consensus::deserialize((*tx).bytes()) { + let bytes = (*tx).bytes(); + let transaction: dashcore::Transaction = match dashcore::consensus::deserialize(bytes) { Ok(t) => t, Err(e) => { return PlatformWalletFFIResult::err( @@ -109,14 +93,10 @@ pub unsafe extern "C" fn core_wallet_signed_payment_register( let txid = transaction.txid(); let fee = (*tx).fee(); - let token = SIGNED_PAYMENT_REGISTRY.register( - core, - transaction, - account_type.as_standard_account_type(), - account_index, - ); - - // txid hex never contains a NUL, but handle the impossible case anyway. + // Do all fallible/pure marshalling BEFORE the registry insert — that insert + // mints a token and holds the funding reservation, so a later failure would + // orphan the reservation with no token to release it. txid hex never + // contains a NUL, but handle the impossible case anyway. let c_txid = match CString::new(txid.to_string()) { Ok(s) => s, Err(_) => { @@ -127,9 +107,20 @@ pub unsafe extern "C" fn core_wallet_signed_payment_register( } }; + let token = runtime().block_on(SIGNED_PAYMENT_REGISTRY.register( + core, + transaction, + account_type.as_standard_account_type(), + account_index, + )); + *out_token = token; *out_fee = fee; *out_txid = c_txid.into_raw(); + // Borrowed view into the still-live `tx` buffer; the caller copies it out + // before freeing `tx` (mirrors the retired `core_wallet_transaction_get_bytes`). + *out_bytes_ptr = bytes.as_ptr(); + *out_bytes_len = bytes.len(); PlatformWalletFFIResult::ok() } @@ -156,7 +147,8 @@ pub unsafe extern "C" fn core_wallet_signed_payment_broadcast( let core = unwrap_option_or_return!(CORE_WALLET_STORAGE.with_item(core_handle, |w| w.clone())); - let result = runtime().block_on(SIGNED_PAYMENT_REGISTRY.broadcast(token as ReservationToken, &core)); + let result = + runtime().block_on(SIGNED_PAYMENT_REGISTRY.broadcast(token as ReservationToken, &core)); match result { Ok(txid) => { @@ -172,12 +164,14 @@ pub unsafe extern "C" fn core_wallet_signed_payment_broadcast( *out_txid = c_txid.into_raw(); PlatformWalletFFIResult::ok() } - Err(e @ (SignedPaymentError::StaleToken(_) | SignedPaymentError::WalletMismatch(_))) => { - PlatformWalletFFIResult::err( - PlatformWalletFFIResultCode::ErrorStaleReservationToken, - e.to_string(), - ) - } + Err( + e @ (SignedPaymentError::StaleToken(_) + | SignedPaymentError::WalletMismatch(_) + | SignedPaymentError::StaleReservationToken(_)), + ) => PlatformWalletFFIResult::err( + PlatformWalletFFIResultCode::ErrorStaleReservationToken, + e.to_string(), + ), // Preserve the typed underlying wallet error (keeps the ambiguous // "may already be on the network" retry semantics intact). Err(SignedPaymentError::Broadcast(e)) => PlatformWalletFFIResult::from(e), diff --git a/packages/rs-platform-wallet-ffi/src/wallet.rs b/packages/rs-platform-wallet-ffi/src/wallet.rs index 8ffd78a896f..7b10c6242e4 100644 --- a/packages/rs-platform-wallet-ffi/src/wallet.rs +++ b/packages/rs-platform-wallet-ffi/src/wallet.rs @@ -390,6 +390,17 @@ pub unsafe extern "C" fn platform_wallet_manager_masternode_withdraw( /// Destroy a PlatformWallet handle. #[no_mangle] pub unsafe extern "C" fn platform_wallet_destroy(handle: Handle) -> PlatformWalletFFIResult { + // Sweep any outstanding deferred-payment tokens bound to this wallet first, + // so the registry stops pinning its `WalletManager` (accounts, keys, sync + // state) alive for the rest of the process via the `CoreWallet` clone each + // token captured. Hooked here rather than into `core_wallet_destroy`: the + // deferred flow builds/registers on one short-lived core handle and + // broadcasts on another, so sweeping on core-handle destroy would drop + // tokens between register and broadcast. + PLATFORM_WALLET_STORAGE.with_item(handle, |wallet| { + crate::core_wallet::signed_payment::SIGNED_PAYMENT_REGISTRY + .remove_entries_for_wallet(wallet.core()); + }); PLATFORM_WALLET_STORAGE.remove(handle); PlatformWalletFFIResult::ok() } diff --git a/packages/rs-platform-wallet/src/wallet/core/wallet.rs b/packages/rs-platform-wallet/src/wallet/core/wallet.rs index 8cc0488ade9..8ad384d873d 100644 --- a/packages/rs-platform-wallet/src/wallet/core/wallet.rs +++ b/packages/rs-platform-wallet/src/wallet/core/wallet.rs @@ -10,6 +10,7 @@ use tokio::sync::RwLock; use key_wallet::managed_account::address_pool::KeySource; use key_wallet::managed_account::managed_account_trait::ManagedAccountTrait; use key_wallet::wallet::managed_wallet_info::transaction_building::AccountTypePreference; +use key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; use key_wallet_manager::WalletManager; use crate::broadcaster::TransactionBroadcaster; @@ -286,6 +287,21 @@ impl CoreWallet { pub fn network(&self) -> key_wallet::Network { self.sdk.network } + + /// Current synced block height for this wallet, or `None` if the wallet is no + /// longer present in the manager. + /// + /// Used by the deferred-payment + /// [`SignedPaymentRegistry`](crate::SignedPaymentRegistry) to bound a token's + /// lifetime against key-wallet's UTXO reservation TTL: a `build_signed` + /// reservation is stamped at this height, so the elapsed span since + /// registration tells the registry whether the reservation could have been + /// swept and re-selected out from under the token. + pub(crate) async fn synced_height(&self) -> Option { + let wm = self.wallet_manager.read().await; + wm.get_wallet_and_info(&self.wallet_id) + .map(|(_, info)| info.core_wallet.synced_height()) + } } impl std::fmt::Debug for CoreWallet { diff --git a/packages/rs-platform-wallet/src/wallet/signed_payment_registry.rs b/packages/rs-platform-wallet/src/wallet/signed_payment_registry.rs index ece79d13867..3c89be7e968 100644 --- a/packages/rs-platform-wallet/src/wallet/signed_payment_registry.rs +++ b/packages/rs-platform-wallet/src/wallet/signed_payment_registry.rs @@ -21,10 +21,24 @@ //! * [`release`](SignedPaymentRegistry::release) is idempotent: releasing an //! unknown / already-consumed token is a silent no-op. //! * A token is bound to the exact wallet instance it was minted against -//! (`Arc::ptr_eq` on the shared `WalletManager`). Broadcasting it through a -//! re-created wallet — whose in-memory `ReservationSet` no longer holds the -//! inputs — is a [`SignedPaymentError::WalletMismatch`] rather than a spend -//! against stale state. +//! (`Arc::ptr_eq` on the shared `WalletManager` **and** an equal `wallet_id`, +//! so two wallets sharing one multi-wallet `PlatformWalletManager` are still +//! told apart). Broadcasting it through a re-created wallet — whose in-memory +//! `ReservationSet` no longer holds the inputs — is a +//! [`SignedPaymentError::WalletMismatch`] rather than a spend against stale +//! state. +//! * A token has a bounded lifetime ([`RESERVATION_MAX_AGE_BLOCKS`]). Once the +//! wallet has synced far enough past the height at which `build_signed` +//! stamped the reservation that key-wallet's own `ReservationSet` TTL could +//! have swept and re-selected the funding UTXO for an unrelated build, +//! broadcasting or releasing the token would act on state that may no longer +//! be its own — so both are refused with +//! [`SignedPaymentError::StaleReservationToken`] and the caller must rebuild. +//! This guard is the primary defence: key-wallet exposes no per-outpoint +//! ownership/generation check to make [`release`](SignedPaymentRegistry::release) +//! itself generation-aware without modifying the pinned crate, so an +//! unconditional release-by-outpoint after a sweep is prevented by never +//! reaching it once the token is stale. //! //! ## Process-death semantics //! @@ -37,7 +51,7 @@ use std::collections::HashMap; use std::sync::atomic::{AtomicU64, Ordering}; -use std::sync::{Arc, Mutex}; +use std::sync::{Arc, Mutex, MutexGuard}; use dashcore::{Transaction, Txid}; use key_wallet::account::account_type::StandardAccountType; @@ -53,6 +67,35 @@ use crate::PlatformWalletError; /// lifetime and never reused, so a stale token can always be recognised. pub type ReservationToken = u64; +/// Maximum age, in synced blocks, of a registered token before its broadcast or +/// release is refused. +/// +/// Kept strictly below key-wallet's `RESERVATION_TTL_BLOCKS` (24, ~1h at the +/// mainnet block target): a `build_signed` reservation is stamped at the wallet's +/// synced height and swept by a later `reserve`/`reserved` call once it is +/// `RESERVATION_TTL_BLOCKS` old, silently returning the outpoint to the +/// selectable pool where an unrelated build can re-select and re-reserve it. +/// `ReservationSet::release` removes an outpoint unconditionally, with no +/// ownership/generation check, so acting on a token whose reservation was +/// already swept could free (or broadcast against) a newer, unrelated +/// reservation. Refusing at this lower bound guarantees the guard always trips +/// **before** the underlying reservation could have been swept, leaving a margin +/// for the wallet's synced height to lag a few blocks behind the true tip. +const RESERVATION_MAX_AGE_BLOCKS: u32 = 20; + +/// Whether a token registered at `registered_height` is too old to act on at +/// `current_height` (see [`RESERVATION_MAX_AGE_BLOCKS`]). Unknown heights (the +/// wallet was gone at register or is gone now) disable the guard — the +/// wallet-mismatch / account-lookup paths already reject those cases. +fn reservation_expired(registered_height: Option, current_height: Option) -> bool { + match (registered_height, current_height) { + (Some(registered), Some(current)) => { + current.saturating_sub(registered) >= RESERVATION_MAX_AGE_BLOCKS + } + _ => false, + } +} + /// Failure of a deferred broadcast/release token operation. #[derive(Debug, thiserror::Error)] pub enum SignedPaymentError { @@ -69,6 +112,14 @@ pub enum SignedPaymentError { #[error("reservation token {0} was minted against a different wallet instance")] WalletMismatch(ReservationToken), + /// The token has outlived [`RESERVATION_MAX_AGE_BLOCKS`], so its underlying + /// UTXO reservation may already have been swept by key-wallet's TTL and + /// re-selected by an unrelated build. Acting on it (broadcast or release) + /// could touch a newer reservation, so it is refused and the caller must + /// rebuild the payment. + #[error("reservation token {0} has outlived its reservation lifetime; rebuild the payment")] + StaleReservationToken(ReservationToken), + /// The underlying broadcast failed. Carries the still-typed wallet error so /// the FFI boundary can preserve the retry semantics (e.g. the ambiguous /// [`PlatformWalletError::TransactionBroadcastUnconfirmed`] "may already be @@ -92,6 +143,13 @@ struct RegisteredPayment { /// TTL backstop), mirroring `CoreAccountTypeFFI::as_standard_account_type`. account_type: Option, account_index: u32, + /// Wallet synced height captured at registration — a proxy for the height at + /// which `build_signed` stamped the funding reservation. Compared against the + /// wallet's current synced height to refuse a broadcast/release once the + /// reservation could plausibly have been swept (see + /// [`RESERVATION_MAX_AGE_BLOCKS`]). `None` when the wallet was not resolvable + /// at registration, which disables the age guard for this entry. + registered_height: Option, } /// Registry of signed-but-unsent payments keyed by [`ReservationToken`]. @@ -121,32 +179,46 @@ impl SignedPaymentRegistry { } } + /// Lock the entries map, recovering from a poisoned mutex rather than + /// panicking. The registry is a single process-global, so a panic elsewhere + /// while the lock was held would otherwise permanently disable deferred + /// payments for every wallet; the guarded `HashMap` has no invariant a + /// partial write could break, so recovery is safe (mirrors key-wallet's + /// sibling `ReservationSet::lock`). + fn lock(&self) -> MutexGuard<'_, HashMap>> { + self.entries + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) + } + /// Take ownership of a built, signed `tx` (whose funding UTXOs `build_signed` /// already reserved) and return an opaque token for a later /// [`broadcast`](Self::broadcast) or [`release`](Self::release). /// /// `core` is the wallet the payment was built against; it is captured so the /// later operation acts on the exact reservation state that holds the inputs. - pub fn register( + /// The wallet's current synced height is captured too, to bound the token's + /// lifetime against key-wallet's reservation TTL (see + /// [`RESERVATION_MAX_AGE_BLOCKS`]). + pub async fn register( &self, core: CoreWallet, tx: Transaction, account_type: Option, account_index: u32, ) -> ReservationToken { + let registered_height = core.synced_height().await; let token = self.next_token.fetch_add(1, Ordering::SeqCst); - self.entries - .lock() - .expect("signed-payment registry mutex poisoned") - .insert( - token, - RegisteredPayment { - core, - tx, - account_type, - account_index, - }, - ); + self.lock().insert( + token, + RegisteredPayment { + core, + tx, + account_type, + account_index, + registered_height, + }, + ); token } @@ -171,21 +243,28 @@ impl SignedPaymentRegistry { // Remove under the lock and drop the guard *before* awaiting — a // std::Mutex guard must never be held across an await point, and the // atomic take is what makes a double-broadcast impossible. - let entry = { - let mut entries = self - .entries - .lock() - .expect("signed-payment registry mutex poisoned"); - entries.remove(&token) - } - .ok_or(SignedPaymentError::StaleToken(token))?; + let entry = { self.lock().remove(&token) }.ok_or(SignedPaymentError::StaleToken(token))?; - if !Arc::ptr_eq(&entry.core.wallet_manager, ¤t.wallet_manager) { - // The token belongs to another wallet instance; it has been removed, - // so it can never be replayed here. + // Bound the token to the exact wallet instance: the same shared + // `WalletManager` (`Arc::ptr_eq`) *and* the same `wallet_id`, so two + // wallets sharing one multi-wallet `PlatformWalletManager` are told + // apart (`ptr_eq` alone matches any pair within that manager). The + // entry is already removed, so a mismatched token can never be replayed. + if !Arc::ptr_eq(&entry.core.wallet_manager, ¤t.wallet_manager) + || entry.core.wallet_id() != current.wallet_id() + { return Err(SignedPaymentError::WalletMismatch(token)); } + // Refuse a token whose reservation could already have been swept and + // re-selected by an unrelated build. The entry is already removed, so we + // simply drop it — deliberately WITHOUT releasing, since a release by + // outpoint here could free a newer build's reservation. The stale + // reservation is reclaimed by key-wallet's own TTL sweep. + if reservation_expired(entry.registered_height, current.synced_height().await) { + return Err(SignedPaymentError::StaleReservationToken(token)); + } + let txid = match entry.account_type { Some(account_type) => { entry @@ -210,17 +289,19 @@ impl SignedPaymentRegistry { /// the one whose `ReservationSet` actually holds the inputs — so no wallet /// handle need be threaded in. pub async fn release(&self, token: ReservationToken) { - let entry = { - let mut entries = self - .entries - .lock() - .expect("signed-payment registry mutex poisoned"); - entries.remove(&token) - }; + let entry = { self.lock().remove(&token) }; let Some(entry) = entry else { // Unknown / already consumed — idempotent no-op. return; }; + // If the token has outlived its reservation lifetime, the funding + // outpoint may already have been swept and re-selected by an unrelated + // build; releasing it by outpoint could free that newer reservation. + // Drop the token without touching the `ReservationSet` — the original + // reservation is reclaimed by key-wallet's own TTL sweep. + if reservation_expired(entry.registered_height, entry.core.synced_height().await) { + return; + } if let Some(account_type) = entry.account_type { entry .core @@ -229,13 +310,36 @@ impl SignedPaymentRegistry { } } + /// Drop every outstanding token bound to `wallet` (same shared + /// `WalletManager` and `wallet_id`), returning how many were removed. + /// + /// Called from the FFI when a `PlatformWallet` is destroyed so the registry + /// stops pinning that wallet's `WalletManager` (accounts, keys, sync state) + /// alive for the rest of the process via its captured `CoreWallet` clone. + /// The reservations are intentionally not released: the wallet — and its + /// accounts' `ReservationSet`s — are being torn down with it, so there is + /// nothing to reconcile, and any surviving token would be a + /// [`WalletMismatch`](SignedPaymentError::WalletMismatch) against a + /// re-created instance regardless. + /// + /// This is hooked into `PlatformWallet` teardown rather than the transient + /// `CoreWallet` handle destroy: the deferred flow builds/registers on one + /// short-lived core handle and broadcasts on another, so sweeping on core + /// handle destroy would drop tokens between register and broadcast. + pub fn remove_entries_for_wallet(&self, wallet: &CoreWallet) -> usize { + let mut entries = self.lock(); + let before = entries.len(); + entries.retain(|_, entry| { + !(Arc::ptr_eq(&entry.core.wallet_manager, &wallet.wallet_manager) + && entry.core.wallet_id() == wallet.wallet_id()) + }); + before - entries.len() + } + /// Number of outstanding (registered but not yet broadcast/released) tokens. #[cfg(test)] pub(crate) fn outstanding(&self) -> usize { - self.entries - .lock() - .expect("signed-payment registry mutex poisoned") - .len() + self.lock().len() } } @@ -253,7 +357,7 @@ mod tests { use key_wallet::wallet::managed_wallet_info::transaction_builder::TransactionBuilder; use key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; - use super::{SignedPaymentError, SignedPaymentRegistry}; + use super::{SignedPaymentError, SignedPaymentRegistry, RESERVATION_MAX_AGE_BLOCKS}; use crate::broadcaster::{BroadcastError, TransactionBroadcaster}; use crate::test_support::{funded_wallet_manager, AlwaysMaybeSentBroadcaster, WalletSigner}; use crate::wallet::core::CoreWallet; @@ -373,7 +477,9 @@ mod tests { builder = builder.add_output(addr, *amount); } let (tx, _fee) = builder - .build_signed(signer, |addr| managed_account.address_derivation_path(&addr)) + .build_signed(signer, |addr| { + managed_account.address_derivation_path(&addr) + }) .await .map_err(|e| PlatformWalletError::TransactionBuild(e.to_string()))?; Ok(tx) @@ -388,18 +494,21 @@ mod tests { funded_core_wallet(StandardAccountType::BIP44Account, Arc::clone(&broadcaster)).await; let registry = SignedPaymentRegistry::new(); - let tx = build_signed_tx(&core, StandardAccountType::BIP44Account, 0, &outputs, &signer) - .await - .expect("build should succeed"); + let tx = build_signed_tx( + &core, + StandardAccountType::BIP44Account, + 0, + &outputs, + &signer, + ) + .await + .expect("build should succeed"); let expected_bytes = dashcore::consensus::serialize(&tx); let expected_txid = tx.txid(); - let token = registry.register( - core.clone(), - tx, - Some(StandardAccountType::BIP44Account), - 0, - ); + let token = registry + .register(core.clone(), tx, Some(StandardAccountType::BIP44Account), 0) + .await; assert_eq!(registry.outstanding(), 1); // Broadcast through a *clone* of the same wallet instance — the @@ -433,7 +542,9 @@ mod tests { let tx = build_signed_tx(&core, account_type, 0, &outputs, &signer) .await .expect("build should succeed"); - let token = registry.register(core.clone(), tx, Some(account_type), 0); + let token = registry + .register(core.clone(), tx, Some(account_type), 0) + .await; // With the reservation held, an immediate rebuild finds no // spendable UTXO and fails. @@ -464,10 +575,18 @@ mod tests { funded_core_wallet(StandardAccountType::BIP44Account, Arc::clone(&broadcaster)).await; let registry = SignedPaymentRegistry::new(); - let tx = build_signed_tx(&core, StandardAccountType::BIP44Account, 0, &outputs, &signer) - .await - .expect("build should succeed"); - let token = registry.register(core.clone(), tx, Some(StandardAccountType::BIP44Account), 0); + let tx = build_signed_tx( + &core, + StandardAccountType::BIP44Account, + 0, + &outputs, + &signer, + ) + .await + .expect("build should succeed"); + let token = registry + .register(core.clone(), tx, Some(StandardAccountType::BIP44Account), 0) + .await; registry .broadcast(token, &core) @@ -493,10 +612,18 @@ mod tests { funded_core_wallet(StandardAccountType::BIP44Account, broadcaster).await; let registry = SignedPaymentRegistry::new(); - let tx = build_signed_tx(&core, StandardAccountType::BIP44Account, 0, &outputs, &signer) - .await - .expect("build should succeed"); - let token = registry.register(core.clone(), tx, Some(StandardAccountType::BIP44Account), 0); + let tx = build_signed_tx( + &core, + StandardAccountType::BIP44Account, + 0, + &outputs, + &signer, + ) + .await + .expect("build should succeed"); + let token = registry + .register(core.clone(), tx, Some(StandardAccountType::BIP44Account), 0) + .await; registry.release(token).await; // Second release: no panic, no error, still consumed. @@ -513,10 +640,18 @@ mod tests { funded_core_wallet(StandardAccountType::BIP44Account, Arc::clone(&broadcaster)).await; let registry = SignedPaymentRegistry::new(); - let tx = build_signed_tx(&core, StandardAccountType::BIP44Account, 0, &outputs, &signer) - .await - .expect("build should succeed"); - let token = registry.register(core.clone(), tx, Some(StandardAccountType::BIP44Account), 0); + let tx = build_signed_tx( + &core, + StandardAccountType::BIP44Account, + 0, + &outputs, + &signer, + ) + .await + .expect("build should succeed"); + let token = registry + .register(core.clone(), tx, Some(StandardAccountType::BIP44Account), 0) + .await; registry.release(token).await; let sent = registry.broadcast(token, &core).await; @@ -524,7 +659,11 @@ mod tests { matches!(sent, Err(SignedPaymentError::StaleToken(_))), "broadcast of a released token must be StaleToken, got {sent:?}" ); - assert_eq!(broadcaster.count.load(Ordering::SeqCst), 0, "nothing was sent"); + assert_eq!( + broadcaster.count.load(Ordering::SeqCst), + 0, + "nothing was sent" + ); } /// An unknown token is a `StaleToken` error. @@ -546,24 +685,34 @@ mod tests { #[tokio::test] async fn broadcast_rejects_a_different_wallet_instance() { let broadcaster_a = Arc::new(CountingBroadcaster::new()); - let (core_a, signer_a, outputs_a) = - funded_core_wallet(StandardAccountType::BIP44Account, Arc::clone(&broadcaster_a)).await; + let (core_a, signer_a, outputs_a) = funded_core_wallet( + StandardAccountType::BIP44Account, + Arc::clone(&broadcaster_a), + ) + .await; // A separate wallet-manager instance stands in for a re-created wallet. let broadcaster_b = Arc::new(CountingBroadcaster::new()); let (core_b, _signer_b, _outputs_b) = funded_core_wallet(StandardAccountType::BIP44Account, broadcaster_b).await; let registry = SignedPaymentRegistry::new(); - let tx = - build_signed_tx(&core_a, StandardAccountType::BIP44Account, 0, &outputs_a, &signer_a) - .await - .expect("build should succeed"); - let token = registry.register( - core_a.clone(), - tx, - Some(StandardAccountType::BIP44Account), + let tx = build_signed_tx( + &core_a, + StandardAccountType::BIP44Account, 0, - ); + &outputs_a, + &signer_a, + ) + .await + .expect("build should succeed"); + let token = registry + .register( + core_a.clone(), + tx, + Some(StandardAccountType::BIP44Account), + 0, + ) + .await; let sent = registry.broadcast(token, &core_b).await; assert!( @@ -588,10 +737,18 @@ mod tests { funded_core_wallet(StandardAccountType::BIP44Account, broadcaster).await; let registry = SignedPaymentRegistry::new(); - let tx = build_signed_tx(&core, StandardAccountType::BIP44Account, 0, &outputs, &signer) - .await - .expect("build should succeed"); - let token = registry.register(core.clone(), tx, Some(StandardAccountType::BIP44Account), 0); + let tx = build_signed_tx( + &core, + StandardAccountType::BIP44Account, + 0, + &outputs, + &signer, + ) + .await + .expect("build should succeed"); + let token = registry + .register(core.clone(), tx, Some(StandardAccountType::BIP44Account), 0) + .await; let sent = registry.broadcast(token, &core).await; assert!( @@ -606,8 +763,14 @@ mod tests { assert_eq!(registry.outstanding(), 0, "token consumed even on failure"); // Reservation kept: an immediate rebuild fails at input selection. - let rebuilt = build_signed_tx(&core, StandardAccountType::BIP44Account, 0, &outputs, &signer) - .await; + let rebuilt = build_signed_tx( + &core, + StandardAccountType::BIP44Account, + 0, + &outputs, + &signer, + ) + .await; assert!( matches!(rebuilt, Err(PlatformWalletError::TransactionBuild(_))), "rebuild must fail with the reservation kept, got {rebuilt:?}" @@ -624,10 +787,18 @@ mod tests { funded_core_wallet(StandardAccountType::BIP44Account, Arc::clone(&broadcaster)).await; let registry = Arc::new(SignedPaymentRegistry::new()); - let tx = build_signed_tx(&core, StandardAccountType::BIP44Account, 0, &outputs, &signer) - .await - .expect("build should succeed"); - let token = registry.register(core.clone(), tx, Some(StandardAccountType::BIP44Account), 0); + let tx = build_signed_tx( + &core, + StandardAccountType::BIP44Account, + 0, + &outputs, + &signer, + ) + .await + .expect("build should succeed"); + let token = registry + .register(core.clone(), tx, Some(StandardAccountType::BIP44Account), 0) + .await; let mut handles = Vec::new(); for _ in 0..8 { @@ -663,9 +834,15 @@ mod tests { funded_core_wallet(StandardAccountType::BIP44Account, broadcaster).await; // One built tx is enough; we register clones of it many times to probe // the token allocator, not the reservation logic. - let tx = build_signed_tx(&core, StandardAccountType::BIP44Account, 0, &outputs, &signer) - .await - .expect("build should succeed"); + let tx = build_signed_tx( + &core, + StandardAccountType::BIP44Account, + 0, + &outputs, + &signer, + ) + .await + .expect("build should succeed"); let registry = Arc::new(SignedPaymentRegistry::new()); let mut handles = Vec::new(); @@ -674,7 +851,9 @@ mod tests { let core = core.clone(); let tx = tx.clone(); handles.push(tokio::spawn(async move { - registry.register(core, tx, Some(StandardAccountType::BIP44Account), 0) + registry + .register(core, tx, Some(StandardAccountType::BIP44Account), 0) + .await })); } let mut tokens = Vec::new(); @@ -685,4 +864,226 @@ mod tests { assert_eq!(unique.len(), tokens.len(), "all tokens must be distinct"); assert_eq!(registry.outstanding(), 16); } + + /// Force the wallet's synced height forward, simulating chain progress + /// between build/register and a later broadcast/release — the window in + /// which key-wallet's `ReservationSet` TTL can sweep the funding reservation. + async fn advance_synced_height(core: &CoreWallet, height: u32) { + let mut wm = core.wallet_manager.write().await; + let (_, info) = wm + .get_wallet_and_info_mut(&core.wallet_id()) + .expect("wallet present in manager"); + info.core_wallet.update_synced_height(height); + } + + /// Once the wallet has synced past `RESERVATION_MAX_AGE_BLOCKS` beyond the + /// registration height, the reservation could have been swept and + /// re-selected — so a broadcast must be refused with `StaleReservationToken` + /// (never a send) and must NOT release the reservation by outpoint (which + /// could free a newer, unrelated build's reservation). + #[tokio::test] + async fn expired_token_broadcast_is_stale_and_keeps_reservation() { + let broadcaster = Arc::new(CountingBroadcaster::new()); + let (core, signer, outputs) = + funded_core_wallet(StandardAccountType::BIP44Account, Arc::clone(&broadcaster)).await; + let registry = SignedPaymentRegistry::new(); + + let registered_height = core.synced_height().await.expect("synced height"); + let tx = build_signed_tx( + &core, + StandardAccountType::BIP44Account, + 0, + &outputs, + &signer, + ) + .await + .expect("build should succeed"); + let token = registry + .register(core.clone(), tx, Some(StandardAccountType::BIP44Account), 0) + .await; + + // Advance past the age bound but stay below key-wallet's 24-block TTL, so + // the reservation is provably still held (only our guard has tripped). + advance_synced_height(&core, registered_height + RESERVATION_MAX_AGE_BLOCKS + 2).await; + + let sent = registry.broadcast(token, &core).await; + assert!( + matches!(sent, Err(SignedPaymentError::StaleReservationToken(t)) if t == token), + "an expired token must broadcast as StaleReservationToken, got {sent:?}" + ); + assert_eq!( + broadcaster.count.load(Ordering::SeqCst), + 0, + "an expired token must never hit the network" + ); + assert_eq!(registry.outstanding(), 0, "the expired token is dropped"); + + // The reservation was NOT released: an immediate rebuild still can't + // reselect the input (it is reclaimed only by key-wallet's own TTL). + let rebuilt = build_signed_tx( + &core, + StandardAccountType::BIP44Account, + 0, + &outputs, + &signer, + ) + .await; + assert!( + matches!(rebuilt, Err(PlatformWalletError::TransactionBuild(_))), + "expired broadcast must not release the reservation, got {rebuilt:?}" + ); + } + + /// Releasing an expired token must likewise NOT touch the `ReservationSet`: + /// its outpoint may already belong to a newer build. The token is dropped + /// and the original reservation is left to key-wallet's TTL sweep. + #[tokio::test] + async fn expired_token_release_keeps_reservation() { + let broadcaster = Arc::new(RecordingBroadcaster::new()); + let (core, signer, outputs) = + funded_core_wallet(StandardAccountType::BIP44Account, broadcaster).await; + let registry = SignedPaymentRegistry::new(); + + let registered_height = core.synced_height().await.expect("synced height"); + let tx = build_signed_tx( + &core, + StandardAccountType::BIP44Account, + 0, + &outputs, + &signer, + ) + .await + .expect("build should succeed"); + let token = registry + .register(core.clone(), tx, Some(StandardAccountType::BIP44Account), 0) + .await; + + advance_synced_height(&core, registered_height + RESERVATION_MAX_AGE_BLOCKS + 2).await; + + registry.release(token).await; + assert_eq!(registry.outstanding(), 0, "the expired token is dropped"); + + // Reservation intentionally kept (not released by outpoint): rebuild + // still fails until the TTL backstop reclaims it. + let rebuilt = build_signed_tx( + &core, + StandardAccountType::BIP44Account, + 0, + &outputs, + &signer, + ) + .await; + assert!( + matches!(rebuilt, Err(PlatformWalletError::TransactionBuild(_))), + "expired release must not free the reservation by outpoint, got {rebuilt:?}" + ); + } + + /// Two wallets sharing one multi-wallet `PlatformWalletManager` have the same + /// `wallet_manager` `Arc` (so `Arc::ptr_eq` alone can't tell them apart); the + /// `wallet_id` comparison must reject a token broadcast through the sibling. + #[tokio::test] + async fn broadcast_rejects_same_manager_different_wallet_id() { + let broadcaster = Arc::new(CountingBroadcaster::new()); + let (core, signer, outputs) = + funded_core_wallet(StandardAccountType::BIP44Account, Arc::clone(&broadcaster)).await; + let registry = SignedPaymentRegistry::new(); + + let tx = build_signed_tx( + &core, + StandardAccountType::BIP44Account, + 0, + &outputs, + &signer, + ) + .await + .expect("build should succeed"); + let token = registry + .register(core.clone(), tx, Some(StandardAccountType::BIP44Account), 0) + .await; + + // A sibling handle over the SAME manager Arc but a different wallet_id — + // `Arc::ptr_eq` on `wallet_manager` is true, so only the wallet_id check + // distinguishes it. + let mut sibling = core.clone(); + sibling.wallet_id[0] ^= 0xFF; + assert!(Arc::ptr_eq(&core.wallet_manager, &sibling.wallet_manager)); + + let sent = registry.broadcast(token, &sibling).await; + assert!( + matches!(sent, Err(SignedPaymentError::WalletMismatch(t)) if t == token), + "a sibling wallet in the same manager must be WalletMismatch, got {sent:?}" + ); + assert_eq!( + broadcaster.count.load(Ordering::SeqCst), + 0, + "nothing was sent for the mismatched wallet" + ); + assert_eq!(registry.outstanding(), 0, "the stale token is dropped"); + } + + /// Destroying a wallet sweeps only its own tokens from the registry, so its + /// captured `CoreWallet` clone stops pinning the `WalletManager` alive — + /// other wallets' tokens are untouched. + #[tokio::test] + async fn remove_entries_for_wallet_drops_only_that_wallets_tokens() { + let (core_a, signer_a, outputs_a) = funded_core_wallet( + StandardAccountType::BIP44Account, + Arc::new(CountingBroadcaster::new()), + ) + .await; + let (core_b, signer_b, outputs_b) = funded_core_wallet( + StandardAccountType::BIP44Account, + Arc::new(CountingBroadcaster::new()), + ) + .await; + let registry = SignedPaymentRegistry::new(); + + let tx_a = build_signed_tx( + &core_a, + StandardAccountType::BIP44Account, + 0, + &outputs_a, + &signer_a, + ) + .await + .expect("build A should succeed"); + let token_a = registry + .register( + core_a.clone(), + tx_a, + Some(StandardAccountType::BIP44Account), + 0, + ) + .await; + let tx_b = build_signed_tx( + &core_b, + StandardAccountType::BIP44Account, + 0, + &outputs_b, + &signer_b, + ) + .await + .expect("build B should succeed"); + let _token_b = registry + .register( + core_b.clone(), + tx_b, + Some(StandardAccountType::BIP44Account), + 0, + ) + .await; + assert_eq!(registry.outstanding(), 2); + + let removed = registry.remove_entries_for_wallet(&core_a); + assert_eq!(removed, 1, "exactly wallet A's one token is swept"); + assert_eq!(registry.outstanding(), 1, "wallet B's token survives"); + + // Wallet A's token is gone: broadcasting it is a plain StaleToken. + let sent = registry.broadcast(token_a, &core_a).await; + assert!( + matches!(sent, Err(SignedPaymentError::StaleToken(t)) if t == token_a), + "a swept token must be StaleToken, got {sent:?}" + ); + } } diff --git a/packages/rs-unified-sdk-jni/src/wallet_manager.rs b/packages/rs-unified-sdk-jni/src/wallet_manager.rs index d15d6a23b41..cdc3d6a13d0 100644 --- a/packages/rs-unified-sdk-jni/src/wallet_manager.rs +++ b/packages/rs-unified-sdk-jni/src/wallet_manager.rs @@ -1241,45 +1241,6 @@ pub extern "system" fn Java_org_dashfoundation_dashsdk_ffi_WalletManagerNative_c // Backed by the process-global registry in `platform_wallet_ffi` // (`core_wallet_signed_payment_*`). See `SignedPaymentRegistry`. -/// `core_wallet_transaction_get_bytes` — the consensus-serialized bytes of a -/// built transaction from [coreTxBuilderBuildSigned], copied into a fresh -/// Java `byte[]`. The underlying FFI hands back a borrowed pointer valid only -/// while `tx` lives, so we copy it here before returning. -#[no_mangle] -pub extern "system" fn Java_org_dashfoundation_dashsdk_ffi_WalletManagerNative_coreTransactionGetBytes( - mut env: JNIEnv, - _class: JClass, - tx: jlong, -) -> jbyteArray { - guard(&mut env, ptr::null_mut(), |env| { - if tx == 0 { - throw_sdk_exception(env, 1, "transaction handle is 0"); - return ptr::null_mut(); - } - let mut out_ptr: *const u8 = ptr::null(); - let mut out_len: usize = 0; - let result = unsafe { - platform_wallet_ffi::core_wallet_transaction_get_bytes( - tx as *const platform_wallet_ffi::FFICoreTransaction, - &mut out_ptr as *mut *const u8, - &mut out_len as *mut usize, - ) - }; - if take_pwffi_error(env, result) { - return ptr::null_mut(); - } - // Copy immediately: the pointer borrows the transaction's own buffer. - let bytes: &[u8] = if out_ptr.is_null() || out_len == 0 { - &[] - } else { - unsafe { std::slice::from_raw_parts(out_ptr, out_len) } - }; - env.byte_array_from_slice(bytes) - .map(|a| a.into_raw()) - .unwrap_or(ptr::null_mut()) - }) -} - /// `core_wallet_signed_payment_register` — register a built+signed transaction /// (from [coreTxBuilderBuildSigned]) for deferred submission, holding its UTXO /// reservation. `accountType`/`accountIndex` are the funding account (0 BIP44, @@ -1287,8 +1248,9 @@ pub extern "system" fn Java_org_dashfoundation_dashsdk_ffi_WalletManagerNative_c /// with [coreTransactionFree]. /// /// Returns a big-endian BLOB the Kotlin side decodes into a -/// `SignedCoreTransaction`: `u64 token, u64 feeDuffs, u32 txidLen, txid utf8`. -/// The raw tx bytes are fetched separately via [coreTransactionGetBytes]. +/// `SignedCoreTransaction`: +/// `u64 token, u64 feeDuffs, u32 txidLen, txid utf8, u32 txBytesLen, txBytes`. +/// The raw tx bytes come back in this same call (no second native round trip). #[no_mangle] pub extern "system" fn Java_org_dashfoundation_dashsdk_ffi_WalletManagerNative_coreWalletRegisterSignedPayment( mut env: JNIEnv, @@ -1315,6 +1277,8 @@ pub extern "system" fn Java_org_dashfoundation_dashsdk_ffi_WalletManagerNative_c let mut token: u64 = 0; let mut fee: u64 = 0; let mut out_txid: *mut c_char = ptr::null_mut(); + let mut out_bytes_ptr: *const u8 = ptr::null(); + let mut out_bytes_len: usize = 0; let result = unsafe { platform_wallet_ffi::core_wallet_signed_payment_register( core_handle as Handle, @@ -1324,6 +1288,8 @@ pub extern "system" fn Java_org_dashfoundation_dashsdk_ffi_WalletManagerNative_c &mut token as *mut u64, &mut fee as *mut u64, &mut out_txid as *mut *mut c_char, + &mut out_bytes_ptr as *mut *const u8, + &mut out_bytes_len as *mut usize, ) }; if take_pwffi_error(env, result) { @@ -1339,16 +1305,33 @@ pub extern "system" fn Java_org_dashfoundation_dashsdk_ffi_WalletManagerNative_c .into_owned(); unsafe { platform_wallet_ffi::core_wallet_free_address(out_txid) }; + // Copy the raw tx bytes immediately: the pointer borrows the still-live + // transaction's own buffer. + let tx_bytes: &[u8] = if out_bytes_ptr.is_null() || out_bytes_len == 0 { + &[] + } else { + unsafe { std::slice::from_raw_parts(out_bytes_ptr, out_bytes_len) } + }; + // Assemble the big-endian BLOB (matches the Kotlin ByteBuffer decoder). let txid_bytes = txid.into_bytes(); - let mut blob = Vec::with_capacity(8 + 8 + 4 + txid_bytes.len()); + let mut blob = Vec::with_capacity(8 + 8 + 4 + txid_bytes.len() + 4 + tx_bytes.len()); blob.extend_from_slice(&token.to_be_bytes()); blob.extend_from_slice(&fee.to_be_bytes()); blob.extend_from_slice(&(txid_bytes.len() as u32).to_be_bytes()); blob.extend_from_slice(&txid_bytes); - env.byte_array_from_slice(&blob) - .map(|a| a.into_raw()) - .unwrap_or(ptr::null_mut()) + blob.extend_from_slice(&(tx_bytes.len() as u32).to_be_bytes()); + blob.extend_from_slice(tx_bytes); + match env.byte_array_from_slice(&blob) { + Ok(array) => array.into_raw(), + Err(_) => { + // The registration already committed and is holding the funding + // reservation; release the token so it isn't orphaned to the + // 24-block TTL backstop when Kotlin never receives it. + let _ = unsafe { platform_wallet_ffi::core_wallet_signed_payment_release(token) }; + ptr::null_mut() + } + } }) } From bbcd88e51f4b8a2f9ad00eb127ccfa96e017f8cd Mon Sep 17 00:00:00 2001 From: bfoss765 <38437574+bfoss765@users.noreply.github.com> Date: Thu, 16 Jul 2026 15:48:42 -0400 Subject: [PATCH 3/7] fix(kotlin-sdk): resolve rebase semantic conflicts onto feat/kotlin-sdk-and-example-app Rebasing the split build/broadcast work onto the current base surfaced three semantic collisions the textual merge could not catch: - error code 22 was reassigned on base (ErrorCoreInsufficientFunds and the asset-lock family 22-25); moved ErrorStaleReservationToken to the next free code 26 in platform-wallet-ffi and DashSdkError's native-code mapping. - base added its own CoreWallet::release_transaction_reservation (taking AccountTypePreference, superset incl. CoinJoin) for the finalized-transaction abandon path, colliding with this PR's identically-named StandardAccountType method. Renamed this PR's deferred-payment release to release_payment_reservation (sole caller: SignedPaymentRegistry::release). - base removed the per-wallet coreSendMutex and now serializes/gates core sends through the TeardownGate (gate.op), moving send concurrency safety into the Rust reservation layer. buildSignedPayment now opens with gate.op like its sibling sendToAddresses instead of the removed mutex, which also satisfies the GateCoverageLintTest handle-borrowing fence. Co-Authored-By: Claude Opus 4.8 --- .../dashsdk/wallet/ManagedPlatformWallet.kt | 55 +++++++++---------- .../src/wallet/core/broadcast.rs | 7 ++- .../src/wallet/signed_payment_registry.rs | 2 +- 3 files changed, 34 insertions(+), 30 deletions(-) diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedPlatformWallet.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedPlatformWallet.kt index ec692911416..023f16716fd 100644 --- a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedPlatformWallet.kt +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedPlatformWallet.kt @@ -223,11 +223,12 @@ class ManagedPlatformWallet internal constructor( * The BIP70/BIP270 counterpart to [sendToAddresses]: those protocols sign, * POST the raw bytes to a merchant server, and broadcast only on ack, which * a single build-sign-broadcast call cannot express. The `new → addOutput* → - * setFunding → buildSigned` build runs under the same per-wallet - * [coreSendMutex] as [sendToAddresses] (closing the setFunding/buildSigned - * selection race); [buildSigned] reserves the selected UTXOs, so once this - * returns the reservation holds the inputs and [broadcastSigned] / - * [releaseReservation] operate on the token later WITHOUT the mutex. + * setFunding → buildSigned` build runs under the same per-wallet teardown + * gate ([gate]) as [sendToAddresses]; [buildSigned] atomically reserves the + * selected UTXOs in the Rust reservation layer (which closes the + * setFunding/buildSigned selection race), so once this returns the + * reservation holds the inputs and [broadcastSigned] / [releaseReservation] + * operate on the token later. * * Process-death note: the reservation is in-memory. An app crash between * this call and [broadcastSigned] drops the reservation on restart (the @@ -243,7 +244,7 @@ class ManagedPlatformWallet internal constructor( coreSignerHandle: Long, accountType: AccountType = AccountType.BIP44, accountIndex: Int = 0, - ): SignedCoreTransaction = withContext(Dispatchers.IO) { + ): SignedCoreTransaction = gate.op { require(accountIndex >= 0) { "accountIndex must be non-negative, got $accountIndex" } require(recipients.isNotEmpty()) { "recipients must not be empty" } require(recipients.all { it.second > 0 }) { @@ -253,28 +254,26 @@ class ManagedPlatformWallet internal constructor( AccountType.BIP44 -> CoreTransactionBuilder.AccountType.BIP44 AccountType.BIP32 -> CoreTransactionBuilder.AccountType.BIP32 } - coreSendMutex.withLock { - mapNativeErrors { - coreWallet().use { core -> - val builder = CoreTransactionBuilder(network) - // `buildSigned` consumes the builder; `use` still safely - // destroys it on the pre-build failure paths. - val signedTx = builder.use { - for ((address, amount) in recipients) { - it.addOutput(address, amount) - } - it.setFunding(this@ManagedPlatformWallet, builderAccountType, accountIndex) - it.buildSigned( - this@ManagedPlatformWallet, - builderAccountType, - accountIndex, - coreSignerHandle, - ) + mapNativeErrors { + coreWallet().use { core -> + val builder = CoreTransactionBuilder(network) + // `buildSigned` consumes the builder; `use` still safely + // destroys it on the pre-build failure paths. + val signedTx = builder.use { + for ((address, amount) in recipients) { + it.addOutput(address, amount) } - // Register the signed tx (holding its reservation) before the - // native transaction is freed; `use` frees it afterward. - signedTx.use { tx -> core.registerSignedPayment(tx) } + it.setFunding(this@ManagedPlatformWallet, builderAccountType, accountIndex) + it.buildSigned( + this@ManagedPlatformWallet, + builderAccountType, + accountIndex, + coreSignerHandle, + ) } + // Register the signed tx (holding its reservation) before the + // native transaction is freed; `use` frees it afterward. + signedTx.use { tx -> core.registerSignedPayment(tx) } } } } @@ -285,8 +284,8 @@ class ManagedPlatformWallet internal constructor( * the token: a second [broadcastSigned] with the same token, or one for a * re-created wallet, throws * [org.dashfoundation.dashsdk.errors.DashSdkError.PlatformWallet.StaleReservationToken] - * rather than double-broadcasting. Operates on the token WITHOUT the - * [coreSendMutex] (the inputs are already reserved). + * rather than double-broadcasting. Operates on the token directly (the + * inputs are already reserved). */ suspend fun broadcastSigned(token: Long): String = withContext(Dispatchers.IO) { mapNativeErrors { diff --git a/packages/rs-platform-wallet/src/wallet/core/broadcast.rs b/packages/rs-platform-wallet/src/wallet/core/broadcast.rs index 55466a1dcf7..8386f9a06ce 100644 --- a/packages/rs-platform-wallet/src/wallet/core/broadcast.rs +++ b/packages/rs-platform-wallet/src/wallet/core/broadcast.rs @@ -103,7 +103,12 @@ impl CoreWallet { /// /// `account_type`/`account_index` identify the funding account handed to /// `set_funding` when the transaction was built. - pub async fn release_transaction_reservation( + /// + /// Named distinctly from the `AccountTypePreference`-typed + /// [`release_transaction_reservation`](Self::release_transaction_reservation) + /// (the finalized-transaction abandon path); this `StandardAccountType` + /// form serves the deferred [`SignedPaymentRegistry`](crate::SignedPaymentRegistry). + pub async fn release_payment_reservation( &self, account_type: StandardAccountType, account_index: u32, diff --git a/packages/rs-platform-wallet/src/wallet/signed_payment_registry.rs b/packages/rs-platform-wallet/src/wallet/signed_payment_registry.rs index 3c89be7e968..9f1028d840f 100644 --- a/packages/rs-platform-wallet/src/wallet/signed_payment_registry.rs +++ b/packages/rs-platform-wallet/src/wallet/signed_payment_registry.rs @@ -305,7 +305,7 @@ impl SignedPaymentRegistry { if let Some(account_type) = entry.account_type { entry .core - .release_transaction_reservation(account_type, entry.account_index, &entry.tx) + .release_payment_reservation(account_type, entry.account_index, &entry.tx) .await; } } From b83cdcb48fd05eb0edcfee3a19def55e0a0ec032 Mon Sep 17 00:00:00 2001 From: bfoss765 <38437574+bfoss765@users.noreply.github.com> Date: Fri, 17 Jul 2026 08:55:54 -0400 Subject: [PATCH 4/7] fix(kotlin-sdk): assert native code 26 for the stale-reservation-token mapping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rebase onto feat/kotlin-sdk-and-example-app reassigned native code 22 to ErrorCoreInsufficientFunds and moved ErrorStaleReservationToken to code 26 (on both the Rust enum and DashSdkError's mapping), but DashSdkErrorTest still constructed code 22 and asserted StaleReservationToken. That deterministically resolved to CoreInsufficientFunds, so platformWalletCodesMapToPlatformWalletSubtree failed and :sdk:testDebugUnitTest — the "Kotlin SDK build + tests (x86_64 emulator)" CI job — went red without actually verifying the code-26 mapping. Point the assertion at code 26 so it exercises the real production mapping. Co-Authored-By: Claude Fable 5 --- .../org/dashfoundation/dashsdk/errors/DashSdkErrorTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/errors/DashSdkErrorTest.kt b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/errors/DashSdkErrorTest.kt index 502b1d274f8..b9f3b294fb1 100644 --- a/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/errors/DashSdkErrorTest.kt +++ b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/errors/DashSdkErrorTest.kt @@ -106,7 +106,7 @@ class DashSdkErrorTest { // Deferred build/broadcast: a stale/consumed/wrong-wallet reservation // token → typed StaleReservationToken, not retryable. - val staleToken = DashSdkError.fromNative(DashSDKException(offset + 22, "stale token 7")) + val staleToken = DashSdkError.fromNative(DashSDKException(offset + 26, "stale token 7")) assertTrue(staleToken is DashSdkError.PlatformWallet.StaleReservationToken) assertFalse( "StaleReservationToken must NOT be retryable (rebuild the payment)", From fe3f59a2e95d08b95e025d99ea982c42363c9ae9 Mon Sep 17 00:00:00 2001 From: bfoss765 <38437574+bfoss765@users.noreply.github.com> Date: Fri, 17 Jul 2026 08:56:08 -0400 Subject: [PATCH 5/7] fix(kotlin-sdk): bound the deferred-payment token on the reservation's own height clock The SignedPaymentRegistry age guard stamped registered_height with CoreWallet::synced_height() and compared it against a later synced_height(), while the funding reservation it is meant to stay under is stamped with last_processed_height() (the height finalize_transaction / build_signed pass to set_current_height, and the clock key-wallet's ReservationSet TTL sweeps against). synced_height can regress during a rescan while last_processed_height is monotonic, so measuring the reservation's age against synced_height could let a token outlive its reservation and act on an outpoint key-wallet had already swept and re-selected for an unrelated build. Read last_processed_height() for both the registration stamp and the current comparison so the guard measures the same clock the reservation is stamped with, trips strictly before the underlying TTL, and never regresses. Add CoreWallet::last_processed_height(); drop the now-unused synced_height(). The registry's expiry tests now stamp and advance last_processed_height to match production, and outstanding() is exposed under test-utils for downstream FFI tests. Co-Authored-By: Claude Fable 5 --- .../src/wallet/core/wallet.rs | 22 +++--- .../src/wallet/signed_payment_registry.rs | 77 +++++++++++-------- 2 files changed, 57 insertions(+), 42 deletions(-) diff --git a/packages/rs-platform-wallet/src/wallet/core/wallet.rs b/packages/rs-platform-wallet/src/wallet/core/wallet.rs index 8ad384d873d..9dd2b0e4493 100644 --- a/packages/rs-platform-wallet/src/wallet/core/wallet.rs +++ b/packages/rs-platform-wallet/src/wallet/core/wallet.rs @@ -288,19 +288,23 @@ impl CoreWallet { self.sdk.network } - /// Current synced block height for this wallet, or `None` if the wallet is no - /// longer present in the manager. + /// Current last-processed block height for this wallet, or `None` if the + /// wallet is no longer present in the manager. /// - /// Used by the deferred-payment + /// This is the clock the funding reservation is actually stamped with: + /// `finalize_transaction` / `build_signed` reserve the selected inputs at + /// `set_current_height(last_processed_height())`, and key-wallet's + /// `ReservationSet` TTL sweeps entries relative to a later build's + /// `last_processed_height`. It is therefore the correct — and monotonic — + /// clock for the deferred-payment /// [`SignedPaymentRegistry`](crate::SignedPaymentRegistry) to bound a token's - /// lifetime against key-wallet's UTXO reservation TTL: a `build_signed` - /// reservation is stamped at this height, so the elapsed span since - /// registration tells the registry whether the reservation could have been - /// swept and re-selected out from under the token. - pub(crate) async fn synced_height(&self) -> Option { + /// lifetime against that TTL. `synced_height` is a different clock that can + /// regress during a rescan, so measuring the reservation's age against it + /// could let a token outlive its reservation. + pub(crate) async fn last_processed_height(&self) -> Option { let wm = self.wallet_manager.read().await; wm.get_wallet_and_info(&self.wallet_id) - .map(|(_, info)| info.core_wallet.synced_height()) + .map(|(_, info)| info.core_wallet.last_processed_height()) } } diff --git a/packages/rs-platform-wallet/src/wallet/signed_payment_registry.rs b/packages/rs-platform-wallet/src/wallet/signed_payment_registry.rs index 9f1028d840f..56968725625 100644 --- a/packages/rs-platform-wallet/src/wallet/signed_payment_registry.rs +++ b/packages/rs-platform-wallet/src/wallet/signed_payment_registry.rs @@ -28,9 +28,10 @@ //! [`SignedPaymentError::WalletMismatch`] rather than a spend against stale //! state. //! * A token has a bounded lifetime ([`RESERVATION_MAX_AGE_BLOCKS`]). Once the -//! wallet has synced far enough past the height at which `build_signed` -//! stamped the reservation that key-wallet's own `ReservationSet` TTL could -//! have swept and re-selected the funding UTXO for an unrelated build, +//! wallet's `last_processed_height` has advanced far enough past the height at +//! which `build_signed` / `finalize_transaction` stamped the reservation that +//! key-wallet's own `ReservationSet` TTL could have swept and re-selected the +//! funding UTXO for an unrelated build, //! broadcasting or releasing the token would act on state that may no longer //! be its own — so both are refused with //! [`SignedPaymentError::StaleReservationToken`] and the caller must rebuild. @@ -67,20 +68,22 @@ use crate::PlatformWalletError; /// lifetime and never reused, so a stale token can always be recognised. pub type ReservationToken = u64; -/// Maximum age, in synced blocks, of a registered token before its broadcast or -/// release is refused. +/// Maximum age, in `last_processed_height` blocks, of a registered token before +/// its broadcast or release is refused. /// /// Kept strictly below key-wallet's `RESERVATION_TTL_BLOCKS` (24, ~1h at the -/// mainnet block target): a `build_signed` reservation is stamped at the wallet's -/// synced height and swept by a later `reserve`/`reserved` call once it is -/// `RESERVATION_TTL_BLOCKS` old, silently returning the outpoint to the -/// selectable pool where an unrelated build can re-select and re-reserve it. +/// mainnet block target): a `build_signed` / `finalize_transaction` reservation +/// is stamped at the wallet's `last_processed_height` (via `set_current_height`) +/// and swept by a later `reserve`/`reserved` call — itself stamped with the same +/// `last_processed_height` clock — once it is `RESERVATION_TTL_BLOCKS` old, +/// silently returning the outpoint to the selectable pool where an unrelated +/// build can re-select and re-reserve it. /// `ReservationSet::release` removes an outpoint unconditionally, with no /// ownership/generation check, so acting on a token whose reservation was /// already swept could free (or broadcast against) a newer, unrelated /// reservation. Refusing at this lower bound guarantees the guard always trips /// **before** the underlying reservation could have been swept, leaving a margin -/// for the wallet's synced height to lag a few blocks behind the true tip. +/// for `last_processed_height` to lag a few blocks behind the true tip. const RESERVATION_MAX_AGE_BLOCKS: u32 = 20; /// Whether a token registered at `registered_height` is too old to act on at @@ -143,12 +146,13 @@ struct RegisteredPayment { /// TTL backstop), mirroring `CoreAccountTypeFFI::as_standard_account_type`. account_type: Option, account_index: u32, - /// Wallet synced height captured at registration — a proxy for the height at - /// which `build_signed` stamped the funding reservation. Compared against the - /// wallet's current synced height to refuse a broadcast/release once the - /// reservation could plausibly have been swept (see - /// [`RESERVATION_MAX_AGE_BLOCKS`]). `None` when the wallet was not resolvable - /// at registration, which disables the age guard for this entry. + /// Wallet `last_processed_height` captured at registration — the exact clock + /// `build_signed` / `finalize_transaction` stamps the funding reservation + /// with. Compared against the wallet's current `last_processed_height` to + /// refuse a broadcast/release once the reservation could plausibly have been + /// swept by key-wallet's TTL (see [`RESERVATION_MAX_AGE_BLOCKS`]). `None` when + /// the wallet was not resolvable at registration, which disables the age + /// guard for this entry. registered_height: Option, } @@ -197,8 +201,8 @@ impl SignedPaymentRegistry { /// /// `core` is the wallet the payment was built against; it is captured so the /// later operation acts on the exact reservation state that holds the inputs. - /// The wallet's current synced height is captured too, to bound the token's - /// lifetime against key-wallet's reservation TTL (see + /// The wallet's current `last_processed_height` is captured too, to bound the + /// token's lifetime against key-wallet's reservation TTL (see /// [`RESERVATION_MAX_AGE_BLOCKS`]). pub async fn register( &self, @@ -207,7 +211,7 @@ impl SignedPaymentRegistry { account_type: Option, account_index: u32, ) -> ReservationToken { - let registered_height = core.synced_height().await; + let registered_height = core.last_processed_height().await; let token = self.next_token.fetch_add(1, Ordering::SeqCst); self.lock().insert( token, @@ -261,7 +265,7 @@ impl SignedPaymentRegistry { // simply drop it — deliberately WITHOUT releasing, since a release by // outpoint here could free a newer build's reservation. The stale // reservation is reclaimed by key-wallet's own TTL sweep. - if reservation_expired(entry.registered_height, current.synced_height().await) { + if reservation_expired(entry.registered_height, current.last_processed_height().await) { return Err(SignedPaymentError::StaleReservationToken(token)); } @@ -299,7 +303,7 @@ impl SignedPaymentRegistry { // build; releasing it by outpoint could free that newer reservation. // Drop the token without touching the `ReservationSet` — the original // reservation is reclaimed by key-wallet's own TTL sweep. - if reservation_expired(entry.registered_height, entry.core.synced_height().await) { + if reservation_expired(entry.registered_height, entry.core.last_processed_height().await) { return; } if let Some(account_type) = entry.account_type { @@ -337,8 +341,10 @@ impl SignedPaymentRegistry { } /// Number of outstanding (registered but not yet broadcast/released) tokens. - #[cfg(test)] - pub(crate) fn outstanding(&self) -> usize { + /// Exposed under `test-utils` so downstream FFI-layer tests (e.g. the + /// `platform_wallet_destroy` final-alias sweep) can observe registry state. + #[cfg(any(test, feature = "test-utils"))] + pub fn outstanding(&self) -> usize { self.lock().len() } } @@ -442,7 +448,11 @@ mod tests { let (wallet, info) = wm .get_wallet_and_info_mut(&core.wallet_id()) .expect("wallet present in manager"); - let current_height = info.core_wallet.synced_height(); + // Stamp the reservation with `last_processed_height` exactly as the + // production `build_signed` / `finalize_transaction` paths do, so the + // registry's age guard (which now reads the same clock) is exercised + // against a faithfully-stamped reservation. + let current_height = info.core_wallet.last_processed_height(); let (managed_account, account) = match account_type { StandardAccountType::BIP44Account => ( info.core_wallet @@ -865,15 +875,16 @@ mod tests { assert_eq!(registry.outstanding(), 16); } - /// Force the wallet's synced height forward, simulating chain progress - /// between build/register and a later broadcast/release — the window in - /// which key-wallet's `ReservationSet` TTL can sweep the funding reservation. - async fn advance_synced_height(core: &CoreWallet, height: u32) { + /// Force the wallet's `last_processed_height` forward, simulating chain + /// progress between build/register and a later broadcast/release — the window + /// in which key-wallet's `ReservationSet` TTL can sweep the funding + /// reservation. This is the same clock the registry's age guard reads. + async fn advance_processed_height(core: &CoreWallet, height: u32) { let mut wm = core.wallet_manager.write().await; let (_, info) = wm .get_wallet_and_info_mut(&core.wallet_id()) .expect("wallet present in manager"); - info.core_wallet.update_synced_height(height); + info.core_wallet.update_last_processed_height(height); } /// Once the wallet has synced past `RESERVATION_MAX_AGE_BLOCKS` beyond the @@ -888,7 +899,7 @@ mod tests { funded_core_wallet(StandardAccountType::BIP44Account, Arc::clone(&broadcaster)).await; let registry = SignedPaymentRegistry::new(); - let registered_height = core.synced_height().await.expect("synced height"); + let registered_height = core.last_processed_height().await.expect("last processed height"); let tx = build_signed_tx( &core, StandardAccountType::BIP44Account, @@ -904,7 +915,7 @@ mod tests { // Advance past the age bound but stay below key-wallet's 24-block TTL, so // the reservation is provably still held (only our guard has tripped). - advance_synced_height(&core, registered_height + RESERVATION_MAX_AGE_BLOCKS + 2).await; + advance_processed_height(&core, registered_height + RESERVATION_MAX_AGE_BLOCKS + 2).await; let sent = registry.broadcast(token, &core).await; assert!( @@ -944,7 +955,7 @@ mod tests { funded_core_wallet(StandardAccountType::BIP44Account, broadcaster).await; let registry = SignedPaymentRegistry::new(); - let registered_height = core.synced_height().await.expect("synced height"); + let registered_height = core.last_processed_height().await.expect("last processed height"); let tx = build_signed_tx( &core, StandardAccountType::BIP44Account, @@ -958,7 +969,7 @@ mod tests { .register(core.clone(), tx, Some(StandardAccountType::BIP44Account), 0) .await; - advance_synced_height(&core, registered_height + RESERVATION_MAX_AGE_BLOCKS + 2).await; + advance_processed_height(&core, registered_height + RESERVATION_MAX_AGE_BLOCKS + 2).await; registry.release(token).await; assert_eq!(registry.outstanding(), 0, "the expired token is dropped"); From 70f11b353c0ecadc330090af889f32edc214ef19 Mon Sep 17 00:00:00 2001 From: bfoss765 <38437574+bfoss765@users.noreply.github.com> Date: Fri, 17 Jul 2026 08:56:22 -0400 Subject: [PATCH 6/7] fix(kotlin-sdk): sweep deferred-payment tokens only when the final wallet alias is destroyed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit platform_wallet_destroy unconditionally called remove_entries_for_wallet, which matches every registry entry sharing the destroyed handle's WalletManager pointer + wallet_id. But platform_wallet_manager_get_wallet hands out an independent handle per alias of the same logical wallet (the loadPersistedWallets path can publish a new wrapper while callers still hold an older one). Destroying one alias therefore consumed a sibling alias's still-live deferred-payment token: the sibling's later broadcast failed as stale while the sweep left the UTXO reserved until its TTL. Gate the sweep on final-alias liveness: after removing this handle, scan the remaining PlatformWallet handles for one that shares the same (WalletManager pointer + wallet_id) — exactly the key remove_entries_for_wallet matches. While a sibling is live the destructor only drops this handle; the sweep runs (releasing the registry's WalletManager pin) only once the last alias goes. Adds HandleStorage::any for the scan and a test_support helper that builds real PlatformWallet aliases; a new FFI test proves a sibling alias's token survives one alias's destruction and is swept when the final alias is destroyed. Co-Authored-By: Claude Fable 5 --- packages/rs-platform-wallet-ffi/src/handle.rs | 11 ++ packages/rs-platform-wallet-ffi/src/wallet.rs | 112 ++++++++++++++++-- .../rs-platform-wallet/src/test_support.rs | 67 +++++++++++ 3 files changed, 179 insertions(+), 11 deletions(-) diff --git a/packages/rs-platform-wallet-ffi/src/handle.rs b/packages/rs-platform-wallet-ffi/src/handle.rs index f343e4ccc98..68e5c77dd49 100644 --- a/packages/rs-platform-wallet-ffi/src/handle.rs +++ b/packages/rs-platform-wallet-ffi/src/handle.rs @@ -71,6 +71,17 @@ impl HandleStorage { guard.get(&handle).map(f) } + /// Whether any currently-stored item satisfies `predicate`. Used to detect + /// whether a logical resource still has a live handle after one of its + /// aliases is removed (e.g. the final-alias check in + /// `platform_wallet_destroy`). + pub fn any(&self, predicate: F) -> bool + where + F: Fn(&T) -> bool, + { + self.items.read().values().any(predicate) + } + pub fn with_item_mut(&self, handle: Handle, f: F) -> Option where F: FnOnce(&mut T) -> R, diff --git a/packages/rs-platform-wallet-ffi/src/wallet.rs b/packages/rs-platform-wallet-ffi/src/wallet.rs index 7b10c6242e4..85912e1e99c 100644 --- a/packages/rs-platform-wallet-ffi/src/wallet.rs +++ b/packages/rs-platform-wallet-ffi/src/wallet.rs @@ -390,17 +390,107 @@ pub unsafe extern "C" fn platform_wallet_manager_masternode_withdraw( /// Destroy a PlatformWallet handle. #[no_mangle] pub unsafe extern "C" fn platform_wallet_destroy(handle: Handle) -> PlatformWalletFFIResult { - // Sweep any outstanding deferred-payment tokens bound to this wallet first, - // so the registry stops pinning its `WalletManager` (accounts, keys, sync - // state) alive for the rest of the process via the `CoreWallet` clone each - // token captured. Hooked here rather than into `core_wallet_destroy`: the - // deferred flow builds/registers on one short-lived core handle and - // broadcasts on another, so sweeping on core-handle destroy would drop - // tokens between register and broadcast. - PLATFORM_WALLET_STORAGE.with_item(handle, |wallet| { - crate::core_wallet::signed_payment::SIGNED_PAYMENT_REGISTRY - .remove_entries_for_wallet(wallet.core()); + // Remove this handle first so it is excluded from the final-alias scan + // below (and so a concurrent lookup can no longer resolve it). + let Some(wallet) = PLATFORM_WALLET_STORAGE.remove(handle) else { + return PlatformWalletFFIResult::ok(); + }; + + // `platform_wallet_manager_get_wallet` hands out an independent handle for + // each alias of the same logical wallet (they share the underlying + // `WalletManager` `Arc` and `wallet_id`). A deferred-payment token minted + // through one alias must NOT be invalidated when a *sibling* alias is + // destroyed — the token is still live and broadcastable through the survivor. + // + // So only sweep the registry when THIS is the final live alias: no other + // stored handle shares the same (`WalletManager` pointer + `wallet_id`) — + // exactly the key `remove_entries_for_wallet` matches on. When a sibling is + // still live, the destructor just drops this handle, leaving its tokens + // (and the shared `WalletManager` pin) in place. Once the last alias goes, + // the sweep runs, releasing the registry's pin on the wallet's + // `WalletManager` (accounts, keys, sync state) that each token's captured + // `CoreWallet` clone would otherwise keep alive for the process lifetime. + let core = wallet.core(); + let wallet_id = core.wallet_id(); + let manager = wallet.wallet_manager(); + let sibling_alias_alive = PLATFORM_WALLET_STORAGE.any(|other| { + other.wallet_id() == wallet_id + && std::sync::Arc::ptr_eq(other.wallet_manager(), manager) }); - PLATFORM_WALLET_STORAGE.remove(handle); + if !sibling_alias_alive { + crate::core_wallet::signed_payment::SIGNED_PAYMENT_REGISTRY + .remove_entries_for_wallet(core); + } PlatformWalletFFIResult::ok() } + +#[cfg(test)] +mod destroy_tests { + use super::*; + use crate::core_wallet::signed_payment::SIGNED_PAYMENT_REGISTRY; + use key_wallet::account::account_type::StandardAccountType; + use platform_wallet::test_support::test_platform_wallet_manager; + + fn dummy_tx() -> dashcore::Transaction { + dashcore::Transaction { + version: 3, + lock_time: 0, + input: vec![], + output: vec![], + special_transaction_payload: None, + } + } + + /// Destroying one alias handle of a logical wallet must NOT invalidate a + /// deferred-payment token registered against a sibling alias: the sweep runs + /// only when the FINAL alias is destroyed. Proves the + /// `platform_wallet_destroy` final-alias gating. + #[test] + fn destroying_one_alias_keeps_a_siblings_token() { + runtime().block_on(async { + let (manager, wallet_id) = test_platform_wallet_manager().await; + + // Two independent handles for the SAME logical wallet, exactly as two + // `platform_wallet_manager_get_wallet` calls would hand out. + let alias_a = manager.get_wallet(&wallet_id).await.expect("alias a"); + let alias_b = manager.get_wallet(&wallet_id).await.expect("alias b"); + let core = alias_a.core().clone(); + let handle_a = PLATFORM_WALLET_STORAGE.insert(alias_a); + let handle_b = PLATFORM_WALLET_STORAGE.insert(alias_b); + + // Register a deferred-payment token (the process-global registry is + // shared, so reason about deltas against a captured baseline). + let baseline = SIGNED_PAYMENT_REGISTRY.outstanding(); + let _token = SIGNED_PAYMENT_REGISTRY + .register( + core.clone(), + dummy_tx(), + Some(StandardAccountType::BIP44Account), + 0, + ) + .await; + assert_eq!(SIGNED_PAYMENT_REGISTRY.outstanding(), baseline + 1); + + // Destroy alias A while B is still live → token must survive. + let result = unsafe { platform_wallet_destroy(handle_a) }; + assert_eq!(result.code, PlatformWalletFFIResultCode::Success); + assert_eq!( + SIGNED_PAYMENT_REGISTRY.outstanding(), + baseline + 1, + "a sibling alias's token must survive destroying another alias" + ); + + // Destroy the final alias B → now the token is swept. + let result = unsafe { platform_wallet_destroy(handle_b) }; + assert_eq!(result.code, PlatformWalletFFIResultCode::Success); + assert_eq!( + SIGNED_PAYMENT_REGISTRY.outstanding(), + baseline, + "destroying the final alias must sweep the wallet's tokens" + ); + + // Keep the manager alive until the end (owns the wallet + adapter). + drop(manager); + }); + } +} diff --git a/packages/rs-platform-wallet/src/test_support.rs b/packages/rs-platform-wallet/src/test_support.rs index a4ecddbd2fc..bc0fa4b8829 100644 --- a/packages/rs-platform-wallet/src/test_support.rs +++ b/packages/rs-platform-wallet/src/test_support.rs @@ -259,3 +259,70 @@ pub async fn funded_spv_core_wallet( signer, ) } + +/// No-op persister satisfying [`PlatformWalletManager`] construction for tests +/// that need a full [`PlatformWallet`] but no real persistence pipeline. +pub struct NoopTestPersister; + +impl crate::changeset::PlatformWalletPersistence for NoopTestPersister { + fn store( + &self, + _wallet_id: WalletId, + _changeset: crate::changeset::PlatformWalletChangeSet, + ) -> Result<(), crate::changeset::PersistenceError> { + Ok(()) + } + + fn flush(&self, _wallet_id: WalletId) -> Result<(), crate::changeset::PersistenceError> { + Ok(()) + } + + fn load(&self) -> Result { + Ok(crate::changeset::ClientStartState::default()) + } +} + +struct NoopTestEventHandler; +impl crate::events::EventHandler for NoopTestEventHandler {} +impl crate::events::PlatformEventHandler for NoopTestEventHandler {} + +/// Build a full [`PlatformWallet`] over a mock SDK and a no-op persister, wired +/// through a real [`PlatformWalletManager`] so its `wallet_manager` `Arc` and +/// `wallet_id` are production-shaped. Returns the manager (which the caller must +/// keep alive — it owns the wallet-event adapter task and the registered +/// `Arc`) alongside the wallet id. +/// +/// Used by FFI-layer tests that need genuine `PlatformWallet` aliases, e.g. the +/// `platform_wallet_destroy` final-alias registry-sweep gating. +pub async fn test_platform_wallet_manager( +) -> (Arc>, WalletId) { + use key_wallet::mnemonic::{Language, Mnemonic}; + use key_wallet::wallet::initialization::WalletAccountCreationOptions; + + // Canonical all-`abandon` BIP-39 test vector. + const TEST_MNEMONIC: &str = "abandon abandon abandon abandon abandon abandon \ + abandon abandon abandon abandon abandon about"; + + let sdk = Arc::new(dash_sdk::SdkBuilder::new_mock().build().expect("mock sdk")); + let persister = Arc::new(NoopTestPersister); + let event_handler: Arc = + Arc::new(NoopTestEventHandler); + let manager = Arc::new(crate::PlatformWalletManager::new(sdk, persister, event_handler)); + + let mnemonic = + Mnemonic::from_phrase(TEST_MNEMONIC, Language::English).expect("valid test mnemonic"); + let seed_bytes = mnemonic.to_seed(""); + // `Some(0)` skips the SPV birth-height lookup so the create never hits the + // network. + let wallet = manager + .create_wallet_from_seed_bytes( + Network::Testnet, + &seed_bytes, + WalletAccountCreationOptions::Default, + Some(0), + ) + .await + .expect("create test wallet"); + let wallet_id = wallet.wallet_id(); + (manager, wallet_id) +} From 84ef26789342ea1c9cc3bea1980c29835a673b9c Mon Sep 17 00:00:00 2001 From: bfoss765 <38437574+bfoss765@users.noreply.github.com> Date: Fri, 17 Jul 2026 08:56:43 -0400 Subject: [PATCH 7/7] fix(kotlin-sdk): route deferred builds through the atomic finalize-and-register path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit buildSignedPayment funded, signed, and registered a deferred payment as three separate native round-trips (setFunding + buildSigned + registerSignedPayment). Once the base branch removed the per-wallet coreSendMutex in favour of the TeardownGate — which only counts active ops for safe teardown and does not serialize sends — that split lost its atomic select-and-reserve boundary: two concurrent deferred builds, or a deferred build racing an immediate send, could select the same UTXO before either reserved it and return two signed transactions spending the same input. Restore atomicity in the Rust reservation layer, the correct home now that the Kotlin mutex is gone: add core_wallet_signed_payment_finalize, which runs the same finalize_transaction the immediate V2 path uses — selection and ReservationSet insertion commit as one unit under the wallet-manager lock, signing only after the lock drops — and then registers the built, reserved tx in the same call. buildSignedPayment now issues that single native operation (CoreTransactionBuilder.finalizeSignedPayment + coreWalletFinalizeSignedPayment), so the select+reserve window can no longer interleave. The existing concurrent_same_account_finalizers_cannot_reserve_the_same_input test already covers the atomic boundary the deferred path now shares. The deprecated split wrappers remain but are no longer on the deferred path. Co-Authored-By: Claude Fable 5 --- .../dashsdk/ffi/WalletManagerNative.kt | 23 +++ .../dashsdk/wallet/CoreTransactionBuilder.kt | 32 +++++ .../dashsdk/wallet/ManagedCoreWallet.kt | 16 +-- .../dashsdk/wallet/ManagedPlatformWallet.kt | 61 +++++--- .../src/core_wallet/transaction_builder.rs | 129 +++++++++++++++++ .../rs-unified-sdk-jni/src/wallet_manager.rs | 133 +++++++++++++++++- 6 files changed, 360 insertions(+), 34 deletions(-) diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/WalletManagerNative.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/WalletManagerNative.kt index 54f2cd11688..ff5a2eb647f 100644 --- a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/WalletManagerNative.kt +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/WalletManagerNative.kt @@ -265,6 +265,29 @@ internal object WalletManagerNative { accountIndex: Int, ): ByteArray + /** + * `core_wallet_signed_payment_finalize` — atomically fund, reserve, sign, + * AND register a builder for deferred (BIP70/BIP270) submission in one + * native call. The concurrency-safe replacement for + * [coreTxBuilderSetFunding] + [coreTxBuilderBuildSigned] + + * [coreWalletRegisterSignedPayment]: selection and reservation commit as a + * single unit under the wallet-manager lock, closing the double-selection + * window. CONSUMES [builder]. [accountType]/[accountIndex] identify the + * funding account (0 BIP44, 1 BIP32, 2 CoinJoin); [coreSignerHandle] is a + * `MnemonicResolverHandle`. + * + * Returns the same big-endian BLOB [coreWalletRegisterSignedPayment] + * returns, decoded into a `SignedCoreTransaction`: + * `u64 token, u64 feeDuffs, u32 txidLen, txid utf8, u32 txBytesLen, txBytes`. + */ + external fun coreWalletFinalizeSignedPayment( + builder: Long, + walletHandle: Long, + accountType: Int, + accountIndex: Int, + coreSignerHandle: Long, + ): ByteArray + /** * `core_wallet_signed_payment_broadcast` — broadcast the payment behind * [token], reconciling its reservation on failure and consuming the token. diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/CoreTransactionBuilder.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/CoreTransactionBuilder.kt index 9a988601d30..df72543f231 100644 --- a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/CoreTransactionBuilder.kt +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/CoreTransactionBuilder.kt @@ -159,6 +159,38 @@ class CoreTransactionBuilder internal constructor(network: Network) : AutoClosea return FinalizedCoreTransaction(transaction, fee) } + /** + * Consume this configured builder and, in ONE atomic native operation, + * select + reserve + sign the inputs and register the built transaction for + * deferred (BIP70/BIP270) submission. The concurrency-safe replacement for + * the deprecated [setFunding] + [buildSigned] + register split: selection + * and reservation commit as a single unit under the wallet-manager lock, so + * concurrent deferred builds cannot double-select an input. Returns the + * decoded [ManagedPlatformWallet.SignedCoreTransaction]. + */ + internal fun finalizeSignedPayment( + wallet: ManagedPlatformWallet, + accountType: AccountType, + accountIndex: Int, + coreSignerHandle: Long, + ): ManagedPlatformWallet.SignedCoreTransaction { + require(accountIndex >= 0) { "accountIndex must be non-negative" } + require(coreSignerHandle != 0L) { "coreSignerHandle must be non-zero" } + // Validate every borrowed dependency before transferring builder + // ownership. Once getAndSet(0) runs, JNI consumes the native builder. + val walletHandle = wallet.handle + val builderPtr = handleRef.getAndSet(0) + check(builderPtr != 0L) { "CoreTransactionBuilder has been consumed or closed" } + val blob = WalletManagerNative.coreWalletFinalizeSignedPayment( + builderPtr, + walletHandle, + accountType.ffiValue, + accountIndex, + coreSignerHandle, + ) + return ManagedPlatformWallet.SignedCoreTransaction.fromRegisterBlob(blob) + } + override fun close() { cleanable.clean() } diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedCoreWallet.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedCoreWallet.kt index 6961c3a093f..75f99b8d57e 100644 --- a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedCoreWallet.kt +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedCoreWallet.kt @@ -71,21 +71,7 @@ class ManagedCoreWallet internal constructor(handle: Long) : AutoCloseable { tx.accountType.ffiValue, tx.accountIndex, ) - val buffer = java.nio.ByteBuffer.wrap(blob) // big-endian by default - val token = buffer.long - val feeDuffs = buffer.long - val txidLen = buffer.int - val txidBytes = ByteArray(txidLen) - buffer.get(txidBytes) - val txBytesLen = buffer.int - val rawTxBytes = ByteArray(txBytesLen) - buffer.get(rawTxBytes) - return ManagedPlatformWallet.SignedCoreTransaction( - txidHex = String(txidBytes, Charsets.UTF_8), - rawTxBytes = rawTxBytes, - feeDuffs = feeDuffs, - reservationToken = token, - ) + return ManagedPlatformWallet.SignedCoreTransaction.fromRegisterBlob(blob) } /** diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedPlatformWallet.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedPlatformWallet.kt index 023f16716fd..971635e2ebe 100644 --- a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedPlatformWallet.kt +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedPlatformWallet.kt @@ -212,6 +212,32 @@ class ManagedPlatformWallet internal constructor( override fun toString(): String = "SignedCoreTransaction(txidHex=$txidHex, feeDuffs=$feeDuffs, " + "reservationToken=$reservationToken, rawTxBytes=${rawTxBytes.size} bytes)" + + internal companion object { + /** + * Decode the big-endian native BLOB the deferred build/register FFI + * returns: `u64 token, u64 feeDuffs, u32 txidLen, txid utf8, + * u32 txBytesLen, txBytes`. Shared by the atomic + * finalize-and-register path and the deprecated register path. + */ + internal fun fromRegisterBlob(blob: ByteArray): SignedCoreTransaction { + val buffer = java.nio.ByteBuffer.wrap(blob) // big-endian by default + val token = buffer.long + val feeDuffs = buffer.long + val txidLen = buffer.int + val txidBytes = ByteArray(txidLen) + buffer.get(txidBytes) + val txBytesLen = buffer.int + val rawTxBytes = ByteArray(txBytesLen) + buffer.get(rawTxBytes) + return SignedCoreTransaction( + txidHex = String(txidBytes, Charsets.UTF_8), + rawTxBytes = rawTxBytes, + feeDuffs = feeDuffs, + reservationToken = token, + ) + } + } } /** @@ -255,25 +281,24 @@ class ManagedPlatformWallet internal constructor( AccountType.BIP32 -> CoreTransactionBuilder.AccountType.BIP32 } mapNativeErrors { - coreWallet().use { core -> - val builder = CoreTransactionBuilder(network) - // `buildSigned` consumes the builder; `use` still safely - // destroys it on the pre-build failure paths. - val signedTx = builder.use { - for ((address, amount) in recipients) { - it.addOutput(address, amount) - } - it.setFunding(this@ManagedPlatformWallet, builderAccountType, accountIndex) - it.buildSigned( - this@ManagedPlatformWallet, - builderAccountType, - accountIndex, - coreSignerHandle, - ) + // One atomic native operation: select + reserve + sign + register. + // `finalizeSignedPayment` consumes the builder on every path, so + // `use` only needs to destroy it on the pre-finalize failure paths + // (adding outputs). Selection and reservation commit as a single unit + // under the wallet-manager lock, so a concurrent deferred build — or a + // deferred build racing an immediate send — can no longer double- + // select the same input, restoring the atomicity the removed Kotlin + // per-wallet send mutex used to provide. + CoreTransactionBuilder(network).use { builder -> + for ((address, amount) in recipients) { + builder.addOutput(address, amount) } - // Register the signed tx (holding its reservation) before the - // native transaction is freed; `use` frees it afterward. - signedTx.use { tx -> core.registerSignedPayment(tx) } + builder.finalizeSignedPayment( + this@ManagedPlatformWallet, + builderAccountType, + accountIndex, + coreSignerHandle, + ) } } } diff --git a/packages/rs-platform-wallet-ffi/src/core_wallet/transaction_builder.rs b/packages/rs-platform-wallet-ffi/src/core_wallet/transaction_builder.rs index 3e30447cf59..7bf823aeea7 100644 --- a/packages/rs-platform-wallet-ffi/src/core_wallet/transaction_builder.rs +++ b/packages/rs-platform-wallet-ffi/src/core_wallet/transaction_builder.rs @@ -17,6 +17,7 @@ use key_wallet::wallet::managed_wallet_info::transaction_builder::TransactionBui use key_wallet::wallet::managed_wallet_info::transaction_building::AccountTypePreference; use key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; use rs_sdk_ffi::{MnemonicResolverCoreSigner, MnemonicResolverHandle}; +use std::ffi::CString; use std::os::raw::{c_char, c_void}; use std::str::FromStr; @@ -139,6 +140,134 @@ pub unsafe extern "C" fn core_wallet_tx_builder_finalize( PlatformWalletFFIResult::ok() } +/// Atomically fund, reserve, and sign a configured builder for DEFERRED +/// (BIP70/BIP270) submission, then register the built transaction — holding its +/// UTXO reservation — in one native operation. +/// +/// This is the deferred counterpart to `core_wallet_tx_builder_finalize`: it +/// runs the same atomic `finalize_transaction`, where selection and insertion +/// into the account `ReservationSet` commit as a single unit under the +/// wallet-manager lock (signing happens after the lock is dropped). Routing the +/// deferred build through it closes the double-selection window that the +/// deprecated `set_funding` + `build_signed` + `register` sequence reopened once +/// the Kotlin per-wallet send mutex was removed: two concurrent deferred builds, +/// or a deferred build racing an immediate send, can no longer select the same +/// UTXO. Consumes `builder` on every path after its pointer is accepted. +/// +/// Writes `out_token` (the reservation token for a later +/// `core_wallet_signed_payment_broadcast` / `core_wallet_signed_payment_release`), +/// `out_fee` (the build's fee in duffs), `out_txid` (a heap C string freed with +/// `core_wallet_free_address`), and `out_tx` (an owned `FFICoreTransaction` +/// carrying the consensus-serialized bytes, freed with +/// `core_wallet_transaction_free`). `out_bytes_ptr`/`out_bytes_len` borrow +/// `out_tx`'s buffer — copy them out before freeing `out_tx`. +/// +/// # Safety +/// `builder` must be a valid, non-destroyed pointer; `wallet` a valid +/// platform-wallet handle; `core_signer_handle` a valid resolver handle; every +/// out-pointer must be writable. `out_tx` must point at writable storage for one +/// `FFICoreTransaction` (typically zeroed). +#[no_mangle] +#[allow(clippy::too_many_arguments)] +pub unsafe extern "C" fn core_wallet_signed_payment_finalize( + builder: *mut FFITransactionBuilder, + wallet: Handle, + account_type: CoreAccountTypeFFI, + account_index: u32, + core_signer_handle: *mut MnemonicResolverHandle, + out_token: *mut u64, + out_fee: *mut u64, + out_txid: *mut *mut c_char, + out_tx: *mut FFICoreTransaction, + out_bytes_ptr: *mut *const u8, + out_bytes_len: *mut usize, +) -> PlatformWalletFFIResult { + check_ptr!(builder); + check_ptr!(core_signer_handle); + check_ptr!(out_token); + check_ptr!(out_fee); + check_ptr!(out_txid); + check_ptr!(out_tx); + check_ptr!(out_bytes_ptr); + check_ptr!(out_bytes_len); + *out_token = 0; + + // `finalize_transaction` consumes the builder: reclaim both heap boxes up + // front so they are freed on every return path below. + let ffi = Box::from_raw(builder); + let inner = *Box::from_raw(ffi.inner as *mut TransactionBuilder); + + let wallet = unwrap_option_or_return!(PLATFORM_WALLET_STORAGE.with_item(wallet, |w| w.clone())); + + let builder_network: Network = ffi.network.into(); + if builder_network != wallet.network() { + return PlatformWalletFFIResult::err( + PlatformWalletFFIResultCode::ErrorInvalidParameter, + "builder network does not match wallet network".to_string(), + ); + } + + let signer = + MnemonicResolverCoreSigner::new(core_signer_handle, wallet.wallet_id(), wallet.network()); + + // Atomic select + reserve + sign in one wallet-manager critical section. + let finalized = runtime().block_on(wallet.core().finalize_transaction( + inner, + account_type.into(), + account_index, + &signer, + )); + let finalized = unwrap_result_or_return!(finalized); + + let txid = finalized.transaction().txid(); + let fee = finalized.fee(); + + // Do the one fallible marshalling step BEFORE the registry insert: that + // insert mints a token and keeps the funding reservation held, so a later + // failure would orphan the reservation with no token to release it. txid hex + // never contains a NUL, but handle the impossible case anyway. + let c_txid = match CString::new(txid.to_string()) { + Ok(s) => s, + Err(_) => { + // Nothing registered yet — release the reservation finalize took. + runtime().block_on(wallet.core().abandon_transaction(&finalized)); + return PlatformWalletFFIResult::err( + PlatformWalletFFIResultCode::ErrorUtf8Conversion, + "txid string contained an interior NUL".to_string(), + ); + } + }; + + let serialized = dashcore::consensus::serialize(finalized.transaction()); + let len = serialized.len(); + + // Register the reserved+signed tx for deferred submission. `finalize` already + // committed the reservation; register just takes ownership of the built tx so + // a later broadcast/release can reconcile it, capturing the wallet instance + // whose `ReservationSet` holds the inputs. + let token = + runtime().block_on(crate::core_wallet::signed_payment::SIGNED_PAYMENT_REGISTRY.register( + wallet.core().clone(), + finalized.transaction().clone(), + account_type.as_standard_account_type(), + account_index, + )); + + *out_tx = FFICoreTransaction { + tx_bytes: Box::into_raw(serialized.into_boxed_slice()) as *mut u8, + tx_len: len, + fee, + }; + *out_token = token; + *out_fee = fee; + *out_txid = c_txid.into_raw(); + // Borrowed view into the just-written `out_tx` buffer; the caller copies the + // bytes out before freeing `out_tx` with `core_wallet_transaction_free`. + *out_bytes_ptr = (*out_tx).tx_bytes as *const u8; + *out_bytes_len = len; + PlatformWalletFFIResult::ok() +} + impl CoreAccountTypeFFI { /// The `StandardAccountType` this maps to, or `None` for `CoinJoin`. /// diff --git a/packages/rs-unified-sdk-jni/src/wallet_manager.rs b/packages/rs-unified-sdk-jni/src/wallet_manager.rs index cdc3d6a13d0..82f33253f24 100644 --- a/packages/rs-unified-sdk-jni/src/wallet_manager.rs +++ b/packages/rs-unified-sdk-jni/src/wallet_manager.rs @@ -1335,10 +1335,141 @@ pub extern "system" fn Java_org_dashfoundation_dashsdk_ffi_WalletManagerNative_c }) } +/// `core_wallet_signed_payment_finalize` — atomically fund, reserve, sign, and +/// register a builder for deferred (BIP70/BIP270) submission in ONE native +/// operation. This is the concurrency-safe replacement for the deprecated +/// `coreTxBuilderSetFunding` + `coreTxBuilderBuildSigned` + +/// `coreWalletRegisterSignedPayment` sequence: selection and reservation commit +/// as a single unit under the wallet-manager lock, so concurrent deferred builds +/// (or a deferred build racing an immediate send) can no longer double-select an +/// input. CONSUMES [builder]. `accountType`/`accountIndex` are the funding +/// account (0 BIP44, 1 BIP32, 2 CoinJoin); [coreSignerHandle] is a +/// `MnemonicResolverHandle`. +/// +/// Returns the same big-endian BLOB `coreWalletRegisterSignedPayment` returns, +/// decoded into a `SignedCoreTransaction`: +/// `u64 token, u64 feeDuffs, u32 txidLen, txid utf8, u32 txBytesLen, txBytes`. +#[no_mangle] +#[allow(clippy::too_many_arguments)] +pub extern "system" fn Java_org_dashfoundation_dashsdk_ffi_WalletManagerNative_coreWalletFinalizeSignedPayment( + mut env: JNIEnv, + _class: JClass, + builder: jlong, + wallet_handle: jlong, + account_type: jni::sys::jint, + account_index: jni::sys::jint, + core_signer_handle: jlong, +) -> jbyteArray { + guard(&mut env, ptr::null_mut(), |env| { + if builder == 0 { + throw_sdk_exception(env, 1, "builder handle must be non-zero"); + return ptr::null_mut(); + } + // From here JNI owns the builder. Any pre-call boundary validation must + // destroy it, because Kotlin has already zeroed its owner token. + let destroy_builder = || unsafe { + platform_wallet_ffi::core_wallet_tx_builder_destroy( + builder as *mut platform_wallet_ffi::FFITransactionBuilder, + ) + }; + if wallet_handle == 0 || core_signer_handle == 0 { + destroy_builder(); + throw_sdk_exception(env, 1, "wallet and signer handles must be non-zero"); + return ptr::null_mut(); + } + let Some(account_type) = core_account_type(account_type) else { + destroy_builder(); + throw_sdk_exception(env, 1, "accountType out of range (expected 0..=2)"); + return ptr::null_mut(); + }; + if account_index < 0 { + destroy_builder(); + throw_sdk_exception(env, 1, "accountIndex must be non-negative"); + return ptr::null_mut(); + } + + // Own an out `FFICoreTransaction` on the heap; its fields are private to + // the FFI crate, so allocate it zeroed and let the FFI fill it in place. + let mut boxed: Box> = + Box::new(std::mem::MaybeUninit::zeroed()); + let out_tx = boxed.as_mut_ptr().cast::(); + + let mut token: u64 = 0; + let mut fee: u64 = 0; + let mut out_txid: *mut c_char = ptr::null_mut(); + let mut out_bytes_ptr: *const u8 = ptr::null(); + let mut out_bytes_len: usize = 0; + let result = unsafe { + platform_wallet_ffi::core_wallet_signed_payment_finalize( + builder as *mut platform_wallet_ffi::FFITransactionBuilder, + wallet_handle as Handle, + account_type, + account_index as u32, + core_signer_handle as *mut rs_sdk_ffi::MnemonicResolverHandle, + &mut token as *mut u64, + &mut fee as *mut u64, + &mut out_txid as *mut *mut c_char, + out_tx, + &mut out_bytes_ptr as *mut *const u8, + &mut out_bytes_len as *mut usize, + ) + }; + if take_pwffi_error(env, result) { + // The FFI freed the builder on the error path and left the out struct + // zeroed (null tx_bytes); dropping `boxed` frees only the box. + return ptr::null_mut(); + } + if out_txid.is_null() { + unsafe { platform_wallet_ffi::core_wallet_transaction_free(out_tx) }; + throw_sdk_exception(env, 1, "finalize returned a NULL txid"); + return ptr::null_mut(); + } + + // Copy the txid out, then free the Rust-owned C string. + let txid = unsafe { CStr::from_ptr(out_txid) } + .to_string_lossy() + .into_owned(); + unsafe { platform_wallet_ffi::core_wallet_free_address(out_txid) }; + + // Copy the raw tx bytes (they borrow the still-live `out_tx` buffer). + let tx_bytes: &[u8] = if out_bytes_ptr.is_null() || out_bytes_len == 0 { + &[] + } else { + unsafe { std::slice::from_raw_parts(out_bytes_ptr, out_bytes_len) } + }; + + // Assemble the big-endian BLOB (matches the register decoder). + let txid_bytes = txid.into_bytes(); + let mut blob = Vec::with_capacity(8 + 8 + 4 + txid_bytes.len() + 4 + tx_bytes.len()); + blob.extend_from_slice(&token.to_be_bytes()); + blob.extend_from_slice(&fee.to_be_bytes()); + blob.extend_from_slice(&(txid_bytes.len() as u32).to_be_bytes()); + blob.extend_from_slice(&txid_bytes); + blob.extend_from_slice(&(tx_bytes.len() as u32).to_be_bytes()); + blob.extend_from_slice(tx_bytes); + let out = match env.byte_array_from_slice(&blob) { + Ok(array) => array.into_raw(), + Err(_) => { + // The registration already committed and is holding the funding + // reservation; release the token so it isn't orphaned to the TTL + // backstop when Kotlin never receives it. + let _ = + unsafe { platform_wallet_ffi::core_wallet_signed_payment_release(token) }; + ptr::null_mut() + } + }; + + // Free the tx bytes now that they are copied into the blob; `boxed` frees + // the outer box on scope exit. + unsafe { platform_wallet_ffi::core_wallet_transaction_free(out_tx) }; + out + }) +} + /// `core_wallet_signed_payment_broadcast` — broadcast the payment behind /// `token`, releasing/keeping its reservation per the broadcast outcome and /// consuming the token. A repeated/stale token throws (native -/// `ErrorStaleReservationToken`, code 22) rather than double-broadcasting. +/// `ErrorStaleReservationToken`, code 26) rather than double-broadcasting. /// `coreHandle` must resolve to the wallet the token was minted against. /// Returns the txid as a lowercase hex string. #[no_mangle]