diff --git a/Cargo.lock b/Cargo.lock index 6fabfe1fe2..0c9545af06 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1229,7 +1229,7 @@ version = "3.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "faf9468729b8cbcea668e36183cb69d317348c2e08e994829fb56ebfdfbaac34" dependencies = [ - "windows-sys 0.59.0", + "windows-sys 0.52.0", ] [[package]] @@ -1662,7 +1662,7 @@ dependencies = [ [[package]] name = "dash-network" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=70d4bf8e36057c58e02d56769a6e9760f701dd06#70d4bf8e36057c58e02d56769a6e9760f701dd06" +source = "git+https://github.com/bfoss765/rust-dashcore?rev=e99959ced0062159d629930f488374e29f63c42b#e99959ced0062159d629930f488374e29f63c42b" dependencies = [ "bincode", "bincode_derive", @@ -1673,7 +1673,7 @@ dependencies = [ [[package]] name = "dash-network-seeds" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=70d4bf8e36057c58e02d56769a6e9760f701dd06#70d4bf8e36057c58e02d56769a6e9760f701dd06" +source = "git+https://github.com/bfoss765/rust-dashcore?rev=e99959ced0062159d629930f488374e29f63c42b#e99959ced0062159d629930f488374e29f63c42b" dependencies = [ "dash-network", ] @@ -1750,7 +1750,7 @@ dependencies = [ [[package]] name = "dash-spv" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=70d4bf8e36057c58e02d56769a6e9760f701dd06#70d4bf8e36057c58e02d56769a6e9760f701dd06" +source = "git+https://github.com/bfoss765/rust-dashcore?rev=e99959ced0062159d629930f488374e29f63c42b#e99959ced0062159d629930f488374e29f63c42b" dependencies = [ "async-trait", "chrono", @@ -1779,7 +1779,7 @@ dependencies = [ [[package]] name = "dashcore" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=70d4bf8e36057c58e02d56769a6e9760f701dd06#70d4bf8e36057c58e02d56769a6e9760f701dd06" +source = "git+https://github.com/bfoss765/rust-dashcore?rev=e99959ced0062159d629930f488374e29f63c42b#e99959ced0062159d629930f488374e29f63c42b" dependencies = [ "anyhow", "base64-compat", @@ -1805,12 +1805,12 @@ dependencies = [ [[package]] name = "dashcore-private" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=70d4bf8e36057c58e02d56769a6e9760f701dd06#70d4bf8e36057c58e02d56769a6e9760f701dd06" +source = "git+https://github.com/bfoss765/rust-dashcore?rev=e99959ced0062159d629930f488374e29f63c42b#e99959ced0062159d629930f488374e29f63c42b" [[package]] name = "dashcore-rpc" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=70d4bf8e36057c58e02d56769a6e9760f701dd06#70d4bf8e36057c58e02d56769a6e9760f701dd06" +source = "git+https://github.com/bfoss765/rust-dashcore?rev=e99959ced0062159d629930f488374e29f63c42b#e99959ced0062159d629930f488374e29f63c42b" dependencies = [ "dashcore-rpc-json", "hex", @@ -1823,7 +1823,7 @@ dependencies = [ [[package]] name = "dashcore-rpc-json" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=70d4bf8e36057c58e02d56769a6e9760f701dd06#70d4bf8e36057c58e02d56769a6e9760f701dd06" +source = "git+https://github.com/bfoss765/rust-dashcore?rev=e99959ced0062159d629930f488374e29f63c42b#e99959ced0062159d629930f488374e29f63c42b" dependencies = [ "bincode", "dashcore", @@ -1838,7 +1838,7 @@ dependencies = [ [[package]] name = "dashcore_hashes" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=70d4bf8e36057c58e02d56769a6e9760f701dd06#70d4bf8e36057c58e02d56769a6e9760f701dd06" +source = "git+https://github.com/bfoss765/rust-dashcore?rev=e99959ced0062159d629930f488374e29f63c42b#e99959ced0062159d629930f488374e29f63c42b" dependencies = [ "bincode", "dashcore-private", @@ -2474,7 +2474,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys 0.59.0", + "windows-sys 0.52.0", ] [[package]] @@ -2535,7 +2535,7 @@ checksum = "0ce92ff622d6dadf7349484f42c93271a0d49b7cc4d466a936405bacbe10aa78" dependencies = [ "cfg-if", "rustix 1.1.4", - "windows-sys 0.59.0", + "windows-sys 0.52.0", ] [[package]] @@ -2904,7 +2904,7 @@ dependencies = [ [[package]] name = "git-state" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=70d4bf8e36057c58e02d56769a6e9760f701dd06#70d4bf8e36057c58e02d56769a6e9760f701dd06" +source = "git+https://github.com/bfoss765/rust-dashcore?rev=e99959ced0062159d629930f488374e29f63c42b#e99959ced0062159d629930f488374e29f63c42b" [[package]] name = "glob" @@ -3839,7 +3839,7 @@ checksum = "3640c1c38b8e4e43584d8df18be5fc6b0aa314ce6ebf51b53313d4306cca8e46" dependencies = [ "hermit-abi", "libc", - "windows-sys 0.59.0", + "windows-sys 0.52.0", ] [[package]] @@ -4095,7 +4095,7 @@ dependencies = [ [[package]] name = "key-wallet" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=70d4bf8e36057c58e02d56769a6e9760f701dd06#70d4bf8e36057c58e02d56769a6e9760f701dd06" +source = "git+https://github.com/bfoss765/rust-dashcore?rev=e99959ced0062159d629930f488374e29f63c42b#e99959ced0062159d629930f488374e29f63c42b" dependencies = [ "aes", "async-trait", @@ -4124,7 +4124,7 @@ dependencies = [ [[package]] name = "key-wallet-ffi" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=70d4bf8e36057c58e02d56769a6e9760f701dd06#70d4bf8e36057c58e02d56769a6e9760f701dd06" +source = "git+https://github.com/bfoss765/rust-dashcore?rev=e99959ced0062159d629930f488374e29f63c42b#e99959ced0062159d629930f488374e29f63c42b" dependencies = [ "cbindgen 0.29.4", "dash-network", @@ -4140,7 +4140,7 @@ dependencies = [ [[package]] name = "key-wallet-manager" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=70d4bf8e36057c58e02d56769a6e9760f701dd06#70d4bf8e36057c58e02d56769a6e9760f701dd06" +source = "git+https://github.com/bfoss765/rust-dashcore?rev=e99959ced0062159d629930f488374e29f63c42b#e99959ced0062159d629930f488374e29f63c42b" dependencies = [ "async-trait", "bincode", @@ -4651,7 +4651,7 @@ version = "0.50.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7957b9740744892f114936ab4a57b3f487491bbeafaf8083688b16841a4240e5" dependencies = [ - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -5749,7 +5749,7 @@ dependencies = [ "once_cell", "socket2 0.5.10", "tracing", - "windows-sys 0.59.0", + "windows-sys 0.52.0", ] [[package]] @@ -6556,7 +6556,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys 0.4.15", - "windows-sys 0.59.0", + "windows-sys 0.52.0", ] [[package]] @@ -6569,7 +6569,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys 0.12.1", - "windows-sys 0.59.0", + "windows-sys 0.52.0", ] [[package]] @@ -6628,7 +6628,7 @@ dependencies = [ "security-framework", "security-framework-sys", "webpki-root-certs", - "windows-sys 0.59.0", + "windows-sys 0.52.0", ] [[package]] @@ -7488,7 +7488,7 @@ dependencies = [ "getrandom 0.4.2", "once_cell", "rustix 1.1.4", - "windows-sys 0.59.0", + "windows-sys 0.52.0", ] [[package]] @@ -8937,7 +8937,7 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" dependencies = [ - "windows-sys 0.59.0", + "windows-sys 0.52.0", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 86e5432b7e..a060afb222 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -52,14 +52,14 @@ members = [ ] [workspace.dependencies] -dashcore = { git = "https://github.com/dashpay/rust-dashcore", rev = "70d4bf8e36057c58e02d56769a6e9760f701dd06" } -dash-network-seeds = { git = "https://github.com/dashpay/rust-dashcore", rev = "70d4bf8e36057c58e02d56769a6e9760f701dd06" } -dash-spv = { git = "https://github.com/dashpay/rust-dashcore", rev = "70d4bf8e36057c58e02d56769a6e9760f701dd06" } -key-wallet = { git = "https://github.com/dashpay/rust-dashcore", rev = "70d4bf8e36057c58e02d56769a6e9760f701dd06" } -key-wallet-ffi = { git = "https://github.com/dashpay/rust-dashcore", rev = "70d4bf8e36057c58e02d56769a6e9760f701dd06" } -key-wallet-manager = { git = "https://github.com/dashpay/rust-dashcore", rev = "70d4bf8e36057c58e02d56769a6e9760f701dd06" } -dash-network = { git = "https://github.com/dashpay/rust-dashcore", rev = "70d4bf8e36057c58e02d56769a6e9760f701dd06" } -dashcore-rpc = { git = "https://github.com/dashpay/rust-dashcore", rev = "70d4bf8e36057c58e02d56769a6e9760f701dd06" } +dashcore = { git = "https://github.com/bfoss765/rust-dashcore", rev = "e99959ced0062159d629930f488374e29f63c42b" } +dash-network-seeds = { git = "https://github.com/bfoss765/rust-dashcore", rev = "e99959ced0062159d629930f488374e29f63c42b" } +dash-spv = { git = "https://github.com/bfoss765/rust-dashcore", rev = "e99959ced0062159d629930f488374e29f63c42b" } +key-wallet = { git = "https://github.com/bfoss765/rust-dashcore", rev = "e99959ced0062159d629930f488374e29f63c42b" } +key-wallet-ffi = { git = "https://github.com/bfoss765/rust-dashcore", rev = "e99959ced0062159d629930f488374e29f63c42b" } +key-wallet-manager = { git = "https://github.com/bfoss765/rust-dashcore", rev = "e99959ced0062159d629930f488374e29f63c42b" } +dash-network = { git = "https://github.com/bfoss765/rust-dashcore", rev = "e99959ced0062159d629930f488374e29f63c42b" } +dashcore-rpc = { git = "https://github.com/bfoss765/rust-dashcore", rev = "e99959ced0062159d629930f488374e29f63c42b" } tokio-metrics = "0.5" 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 de41a05412..7bd06a04c9 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,74 @@ sealed class DashSdkError( cause, ) + /** + * `ErrorTransactionBroadcastRejected` (native code 26). Core + * DEFINITIVELY rejected the core transaction: it is not on the network + * and will not get there. The build's UTXO reservation was released and, + * on the deferred (BIP70/BIP270) path, the token was consumed at the + * same time — so the inputs are spendable again and the token is gone. + * + * The definitive counterpart to [TransactionBroadcastUnconfirmed] (20), + * whose outcome is AMBIGUOUS and which therefore keeps its inputs + * reserved. Because the reservation and token are already gone, this is + * NOT retryable in place: address the rejection reason carried in the + * message, then rebuild with + * [buildSignedPayment][org.dashfoundation.dashsdk.wallet.ManagedPlatformWallet.buildSignedPayment] + * (deferred) or re-issue the send. + */ + class TransactionBroadcastRejected(message: String, cause: Throwable? = null) : + PlatformWallet(message, cause) + + /** + * `ErrorStaleReservationToken` (native code 34). A payment's funding + * reservation has outlived its lifetime: key-wallet's TTL may already + * have swept and re-selected the inputs, so acting on it could touch a + * newer, unrelated reservation. The call did NOT touch the network. + * NOT retryable in place — rebuild the payment. + * + * The code is shared by BOTH deferred-payment surfaces (the messages + * distinguish them): a deferred (BIP70/BIP270) + * [broadcastSigned][org.dashfoundation.dashsdk.wallet.ManagedPlatformWallet.broadcastSigned] + * token, rebuilt with + * [buildSignedPayment][org.dashfoundation.dashsdk.wallet.ManagedPlatformWallet.buildSignedPayment]; + * and a token-less V2 finalized handle whose + * [broadcastTransaction][org.dashfoundation.dashsdk.wallet.ManagedCoreWallet.broadcastTransaction] + * aged past the same reservation bound (abandon still works at any age). + * + * Sibling of the other two deferred-token failures this code used to + * conflate: [ReservationTokenConsumed] (unknown / already broadcast / + * already released) and [ReservationWalletMismatch] (minted against a + * different wallet generation). + */ + class StaleReservationToken(message: String, cause: Throwable? = null) : + PlatformWallet(message, cause) + + /** + * `ErrorReservationTokenConsumed` (native code 35). A deferred + * (BIP70/BIP270) [broadcastSigned][org.dashfoundation.dashsdk.wallet.ManagedPlatformWallet.broadcastSigned] + * token is unknown, already broadcast, or already released — the guard + * that turns a double-broadcast (or a broadcast after release) into a + * typed error instead of a second send. The call did NOT touch the + * network. NOT retryable: rebuild the payment with + * [buildSignedPayment][org.dashfoundation.dashsdk.wallet.ManagedPlatformWallet.buildSignedPayment]. + * (Release is idempotent and never raises this.) + */ + class ReservationTokenConsumed(message: String, cause: Throwable? = null) : + PlatformWallet(message, cause) + + /** + * `ErrorReservationWalletMismatch` (native code 36). A deferred + * (BIP70/BIP270) [broadcastSigned][org.dashfoundation.dashsdk.wallet.ManagedPlatformWallet.broadcastSigned] + * token was minted against a different wallet *generation* than the one + * broadcasting it (e.g. a wallet re-created under the same id); its + * reservation lives in that other generation's reservation set. The call + * did NOT touch the network and did NOT consume the rightful owner's + * token. NOT retryable through this handle: rebuild the payment with + * [buildSignedPayment][org.dashfoundation.dashsdk.wallet.ManagedPlatformWallet.buildSignedPayment]. + */ + class ReservationWalletMismatch(message: String, cause: Throwable? = null) : + PlatformWallet(message, cause) + /** * Any other `PlatformWalletFFIResultCode` without a dedicated type. * Carries the platform-wallet [nativeCode] (already de-offset) and @@ -235,7 +303,20 @@ sealed class DashSdkError( 6 -> PlatformWallet.WalletOperation(message, cause) // ErrorWalletOperation 7, // ErrorIdentityNotFound 8, // ErrorContactNotFound - 98, // NotFound (Option returned as an error) + // NotFound. Handle/Option lookup failures, plus the + // wallet-was-REMOVED case on BOTH deferred-send paths: + // * deferred (BIP70/BIP270) TOKEN path — a signed-payment broadcast + // whose wallet is no longer registered in the manager, or a + // signed-payment finalize whose wallet was removed while it was + // being signed; + // * finalized-transaction HANDLE (V2) path — a tx-builder finalize + // whose wallet was removed or re-created during signing (no handle + // is published), or a V2 broadcast whose generation is gone. + // Every one reconciles the build's UTXO reservation before returning. + // Nothing was broadcast, and unlike ReservationWalletMismatch (36) + // no other live generation holds the payment either — so it is not + // retryable. See dashpay/platform#4185. + 98, -> NotFound(message, cause) 16 -> PlatformWallet.ShieldedBroadcastFailed(message, cause) // ErrorShieldedBroadcastFailed 18 -> PlatformWallet.ShieldedSpendUnconfirmed(message, cause) // ErrorShieldedSpendUnconfirmed @@ -245,6 +326,16 @@ sealed class DashSdkError( 23 -> PlatformWallet.AssetLockNotTracked(message, cause) // ErrorAssetLockNotTracked 24 -> PlatformWallet.AssetLockAlreadyConsumed(message, cause) // ErrorAssetLockAlreadyConsumed 25 -> PlatformWallet.AssetLockFundingMismatch(message, cause) // ErrorAssetLockFundingMismatch + 26 -> PlatformWallet.TransactionBroadcastRejected(message, cause) // ErrorTransactionBroadcastRejected + // The deferred-token trio sits at the contiguous block 34-36 because + // 27-33 are claimed elsewhere: 27 ErrorShutdownIncomplete + // (dashpay/platform#4268, merged), 29 ErrorAssetLockInsufficientFunds + // (#4184), 31 ErrorSigningKeyUnavailable (#4183/#4259), 32 + // ErrorTransactionBuild (#4247/#4256), 33 ErrorTransactionSigning + // (#4256). See packages/rs-platform-wallet-ffi/ERROR_CODE_REGISTRY.md. + 34 -> PlatformWallet.StaleReservationToken(message, cause) // ErrorStaleReservationToken + 35 -> PlatformWallet.ReservationTokenConsumed(message, cause) // ErrorReservationTokenConsumed + 36 -> PlatformWallet.ReservationWalletMismatch(message, cause) // ErrorReservationWalletMismatch 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 205fe225ba..74a40120c2 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 @@ -249,6 +249,45 @@ internal object WalletManagerNative { */ external fun coreTransactionFree(tx: Long) + /** + * `core_wallet_signed_payment_finalize` — atomically fund, reserve, sign, + * AND register a builder for deferred (BIP70/BIP270) submission in one + * native call. 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 a big-endian BLOB 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. + * Rather than double-broadcasting, an unusable token throws one of three + * sibling codes — `ErrorStaleReservationToken` (34, aged out), + * `ErrorReservationTokenConsumed` (35, already consumed/unknown), or + * `ErrorReservationWalletMismatch` (36, different wallet generation). + * [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/CoreTransactionBuilder.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/CoreTransactionBuilder.kt index 9a988601d3..a5c8c5fd7b 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,57 @@ 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, + ) + // Native finalization has ALREADY inserted the payment and committed its + // reservation by the time this blob returns; the token only gains its + // owning NativeCleaner once fromRegisterBlob finishes constructing the + // SignedCoreTransaction. So if construction throws (allocation failure, a + // malformed blob from an ABI mismatch, or Cleaner-registration failure) + // the native token would be registered with no JVM owner able to release + // it, leaking the reservation until key-wallet's TTL. Parse the token + // first (its 8 big-endian bytes lead the blob) and release it defensively + // if ownership construction fails, mirroring the owner-guarded release on + // the rest of the deferred path (dashpay/platform#4185). + var token: Long? = null + return try { + token = java.nio.ByteBuffer.wrap(blob).long + ManagedPlatformWallet.SignedCoreTransaction.fromRegisterBlob(blob) + } catch (error: Throwable) { + token?.let { value -> + runCatching { WalletManagerNative.coreWalletReleaseSignedPayment(value) } + } + throw error + } + } + 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 8a0e661d0e..c8e10199ec 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 @@ -40,14 +40,34 @@ class ManagedCoreWallet internal constructor(handle: Long) : AutoCloseable { tx.accountIndex, ) - /** Consume and broadcast a V2 finalized transaction. */ + /** + * Consume and broadcast a V2 finalized transaction. A handle held past the + * reservation age bound throws the typed + * [StaleReservationToken][org.dashfoundation.dashsdk.errors.DashSdkError.PlatformWallet.StaleReservationToken] + * (native code 34, shared with the deferred-token surface) instead of + * broadcasting against inputs key-wallet's TTL may have re-selected. + * + * On that refusal the handle has **already been consumed** by this call, so + * a follow-up [abandonTransaction] is an invalid-handle error, not a recovery + * path — there is nothing left to release, and the aged reservation is left + * for key-wallet's TTL to reclaim (releasing it by outpoint could free a + * newer build's reservation). Recover by rebuilding the transaction. + */ fun broadcastTransaction(tx: FinalizedCoreTransaction): String = WalletManagerNative.coreWalletBroadcastSignedTransactionV2( handle, tx.takeForBroadcast(), ) - /** Consume without sending and release the selected inputs immediately. */ + /** + * Consume a finalized transaction without sending. Below the reservation age + * bound this releases the selected inputs immediately so a rebuild can + * reselect them. If the handle has aged past the bound the by-outpoint + * release is skipped — key-wallet's TTL may already have swept and + * re-reserved the outpoint, so releasing it could free a newer build's + * reservation — and the aged reservation is left for the TTL to reclaim; the + * handle is torn down either way. + */ fun abandonTransaction(tx: FinalizedCoreTransaction) { WalletManagerNative.coreWalletAbandonSignedTransactionV2( handle, @@ -55,6 +75,19 @@ class ManagedCoreWallet internal constructor(handle: Long) : AutoCloseable { ) } + /** + * Broadcast the deferred payment behind [token] and return its txid. An + * unusable token surfaces as one of the three sibling deferred-token + * errors — aged out + * ([org.dashfoundation.dashsdk.errors.DashSdkError.PlatformWallet.StaleReservationToken]), + * already consumed / unknown + * ([org.dashfoundation.dashsdk.errors.DashSdkError.PlatformWallet.ReservationTokenConsumed]), + * or a different wallet generation + * ([org.dashfoundation.dashsdk.errors.DashSdkError.PlatformWallet.ReservationWalletMismatch]). + */ + 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 ba05a9ff7f..efe8155efa 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,271 @@ 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. + * + * **Owns the reservation token.** The blocking native registration mints the + * token before this object exists, so if the object were then discarded — + * the caller drops it, or a coroutine cancellation is observed after + * [buildSignedPayment]'s native call returned — the token (and its funding + * reservation) would be orphaned until key-wallet's TTL. This type is + * therefore [AutoCloseable] with a [NativeCleaner] GC backstop: [close], or + * GC if you never call it, releases the token exactly once. Release is + * idempotent native-side and tokens are process-unique (never reused), so + * releasing a token already consumed by [broadcastSigned] / + * [releaseReservation] — or releasing twice — is a harmless no-op. A caller + * that broadcasts or releases can still `use`/close this object; a caller + * that abandons it is covered by GC. + * + * @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 (or released by [close] / GC). + */ + class SignedCoreTransaction internal constructor( + val txidHex: String, + val rawTxBytes: ByteArray, + val feeDuffs: Long, + val reservationToken: Long, + ) : AutoCloseable { + + // GC backstop: releases the token if it was neither broadcast nor + // released. The action must not reference this object (it would never + // become phantom-reachable), so it captures the token by value. + private val cleanable = NativeCleaner.register(this, TokenRelease(reservationToken)) + + /** + * Release the funding reservation if this payment was neither broadcast + * nor released, and drop the token. Idempotent — safe to call after a + * [broadcastSigned] / [releaseReservation] (native no-op) and safe to + * call twice. The [NativeCleaner] backstop runs the same release on GC + * if you never call [close]. + */ + override fun close() = cleanable.clean() + + 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)" + + /** Releases the reservation token exactly once, on [close] or GC. */ + private class TokenRelease(private val token: Long) : Runnable { + override fun run() { + WalletManagerNative.coreWalletReleaseSignedPayment(token) + } + } + + internal companion object { + /** + * Decode the big-endian native BLOB the atomic + * finalize-and-register FFI returns: `u64 token, u64 feeDuffs, + * u32 txidLen, txid utf8, u32 txBytesLen, txBytes`. + */ + 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, + ) + } + } + } + + /** + * 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* → finalizeSignedPayment` build runs under the same + * per-wallet teardown gate ([gate]) as [sendToAddresses]. The single atomic + * finalize does select + reserve + sign + register under the wallet-manager + * lock (closing the funding/signing selection race the old setFunding + + * buildSigned split had), so once this returns the reservation holds the + * inputs and [broadcastSigned] / [releaseReservation] operate on the token + * later. + * + * The returned [SignedCoreTransaction] OWNS the token: it is [AutoCloseable] + * with a GC/[NativeCleaner] backstop, so a token that is neither broadcast + * nor released is never orphaned. If a cancellation discards the result + * *after* the blocking native registration already minted the token, this + * call closes it deterministically on the way out (the gate's + * cancellation-cleanup handoff) rather than leaving the reservation to the + * GC backstop or the reservation TTL. Otherwise the backstop releases on GC, + * or the caller releases via an explicit [SignedCoreTransaction.close]; + * consuming the token via [broadcastSigned] / [releaseReservation] makes + * that release a native no-op. + * + * 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 = gate.opWithCleanupOnCancellation( + // Native finalization mints the token and transfers reservation ownership + // to it before the blocking JNI call returns, so the token already exists + // by the time `withContext` dispatches back to the caller. That handoff is + // a prompt-cancellation point: if the caller was cancelled while JNI ran, + // the completed SignedCoreTransaction is discarded before anyone can hold + // it, leaving only the GC/NativeCleaner backstop — the reservation would + // then sit until an unpredictable GC cycle or the reservation TTL. + // Closing the discarded result releases the token deterministically. + cleanup = { payment: SignedCoreTransaction -> payment.close() }, + ) { + 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 + } + mapNativeErrors { + // 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) + } + builder.finalizeSignedPayment( + this@ManagedPlatformWallet, + builderAccountType, + accountIndex, + coreSignerHandle, + ) + } + } + } + + /** + * Broadcast the deferred payment behind [token] (from [buildSignedPayment]) + * and return its broadcast txid — the "merchant server acked" arm. Consumes + * the token. Rather than double-broadcasting, an unusable token throws one + * of three sibling errors: already consumed / unknown + * ([org.dashfoundation.dashsdk.errors.DashSdkError.PlatformWallet.ReservationTokenConsumed], + * e.g. a second [broadcastSigned] with the same token), a different wallet + * generation + * ([org.dashfoundation.dashsdk.errors.DashSdkError.PlatformWallet.ReservationWalletMismatch], + * e.g. a re-created wallet), or aged out + * ([org.dashfoundation.dashsdk.errors.DashSdkError.PlatformWallet.StaleReservationToken]). + * Operates on the token directly (the inputs are already reserved). + * + * Callers holding a [SignedCoreTransaction] should prefer the object + * overload: with the bare token, the source object must stay strongly + * reachable until this call returns, or its GC backstop can release the + * reservation mid-broadcast. + */ + suspend fun broadcastSigned(token: Long): String = withContext(Dispatchers.IO) { + mapNativeErrors { + coreWallet().use { core -> core.broadcastSignedPayment(token) } + } + } + + /** + * Broadcast [payment] and return its txid — the object-owning form of + * [broadcastSigned]. Prefer this over passing the bare + * [SignedCoreTransaction.reservationToken]: the token's lifetime is coupled + * to the object's GC-reachability (the [NativeCleaner] backstop releases the + * reservation when the object is collected), so a caller that extracts the + * `Long` and drops the object races GC and can find the reservation gone. + * This overload keeps the object reachable for the whole native call and + * disarms the backstop once the token is consumed. + */ + suspend fun broadcastSigned(payment: SignedCoreTransaction): String { + try { + val txid = broadcastSigned(payment.reservationToken) + // Token consumed: close() disarms the GC backstop (the underlying + // native release is an idempotent no-op on a consumed token). + payment.close() + return txid + } finally { + // The object must stay reachable across the suspend/native call — + // without this, GC could run the backstop mid-broadcast and release + // the reservation out from under it. + java.lang.ref.Reference.reachabilityFence(payment) + } + } + + /** + * 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) + } + } + } + + /** + * Release [payment]'s funding reservation — the object-owning form of + * [releaseReservation]; see [broadcastSigned] for why it is preferred over + * the bare-token form. + */ + suspend fun releaseReservation(payment: SignedCoreTransaction) { + try { + releaseReservation(payment.reservationToken) + payment.close() + } finally { + java.lang.ref.Reference.reachabilityFence(payment) + } + } + /** * 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 f8e397cade..c1d2da7c69 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,49 @@ class DashSdkErrorTest { ) // The message must warn against retrying (distinct from the anchor case). assertTrue(broadcastUnconfirmed.message!!.contains("do NOT retry")) + + // Definitive broadcast rejection (26) must reach callers as its own type, + // NOT as Generic: it is the definitive counterpart to the ambiguous + // TransactionBroadcastUnconfirmed (20), and on the deferred path the + // reservation was released and the token consumed — so it is not + // retryable in place, it must be rebuilt. + val rejected = DashSdkError.fromNative(DashSDKException(offset + 26, "bad-txns-inputs-spent")) + assertTrue( + "code 26 must not fall through to Generic", + rejected is DashSdkError.PlatformWallet.TransactionBroadcastRejected, + ) + assertFalse( + "TransactionBroadcastRejected must NOT be retryable in place (rebuild the payment)", + rejected.isRetryable, + ) + assertEquals("bad-txns-inputs-spent", rejected.message) + + // Deferred build/broadcast: the three sibling reservation-token failures + // map to three distinct typed errors, none retryable. + val agedOut = DashSdkError.fromNative(DashSDKException(offset + 34, "stale token 7")) + assertTrue(agedOut is DashSdkError.PlatformWallet.StaleReservationToken) + assertFalse( + "StaleReservationToken must NOT be retryable (rebuild the payment)", + agedOut.isRetryable, + ) + assertEquals("stale token 7", agedOut.message) + + val consumed = DashSdkError.fromNative(DashSDKException(offset + 35, "already broadcast")) + assertTrue(consumed is DashSdkError.PlatformWallet.ReservationTokenConsumed) + assertFalse( + "ReservationTokenConsumed must NOT be retryable (rebuild the payment)", + consumed.isRetryable, + ) + assertEquals("already broadcast", consumed.message) + + val walletMismatch = + DashSdkError.fromNative(DashSDKException(offset + 36, "different generation")) + assertTrue(walletMismatch is DashSdkError.PlatformWallet.ReservationWalletMismatch) + assertFalse( + "ReservationWalletMismatch must NOT be retryable (rebuild the payment)", + walletMismatch.isRetryable, + ) + assertEquals("different generation", walletMismatch.message) } @Test diff --git a/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/wallet/SignedCoreTransactionTest.kt b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/wallet/SignedCoreTransactionTest.kt new file mode 100644 index 0000000000..337e3431cf --- /dev/null +++ b/packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/wallet/SignedCoreTransactionTest.kt @@ -0,0 +1,76 @@ +package org.dashfoundation.dashsdk.wallet + +import org.dashfoundation.dashsdk.ffi.NativeCleaner +import org.junit.Assert.assertArrayEquals +import org.junit.Assert.assertEquals +import org.junit.Test +import java.nio.ByteBuffer +import java.util.concurrent.atomic.AtomicInteger + +/** + * Ownership contract for the deferred-payment token (blocker: "Kotlin + * cancellation can orphan a token"). + * + * These are pure-JVM tests — they never call the native release itself (that + * needs the loaded cdylib on the emulator harness). They pin the two properties + * the fix rests on: [ManagedPlatformWallet.SignedCoreTransaction] is an owning + * [AutoCloseable], and the [NativeCleaner] backstop it registers runs its + * release action exactly once (on the first clean / GC and never again), so a + * token abandoned by a dropped object or an observed cancellation is released, + * and a token already consumed by broadcast/release is not double-released. + */ +class SignedCoreTransactionTest { + + private fun registerBlob(token: Long, fee: Long, txid: String, txBytes: ByteArray): ByteArray { + val txidBytes = txid.toByteArray(Charsets.UTF_8) + val buf = ByteBuffer.allocate(8 + 8 + 4 + txidBytes.size + 4 + txBytes.size) + buf.putLong(token) + buf.putLong(fee) + buf.putInt(txidBytes.size) + buf.put(txidBytes) + buf.putInt(txBytes.size) + buf.put(txBytes) + return buf.array() + } + + @Test + fun fromRegisterBlobDecodesFieldsAndIsAnOwningCloseable() { + val txBytes = byteArrayOf(1, 2, 3, 4, 5) + val blob = registerBlob(token = 42L, fee = 7L, txid = "abcd", txBytes = txBytes) + + val signed = ManagedPlatformWallet.SignedCoreTransaction.fromRegisterBlob(blob) + + assertEquals(42L, signed.reservationToken) + assertEquals(7L, signed.feeDuffs) + assertEquals("abcd", signed.txidHex) + assertArrayEquals(txBytes, signed.rawTxBytes) + + // Compile-time proof that the token is owned by a closeable: a dropped + // object can be reclaimed via close() / GC rather than leaking the token. + @Suppress("UNUSED_VARIABLE") + val asCloseable: AutoCloseable = signed + + // Disarm the cleaner backstop before `signed` becomes unreachable: this + // is a pure-JVM test with no cdylib loaded, so the registered native + // release must never fire from the cleaner thread. (NativeCleaner already + // contains the resulting UnsatisfiedLinkError in `runCatching`, so this is + // determinism/hygiene rather than a crash fix.) + signed.close() + } + + @Test + fun cleanerBackstopRunsTheReleaseActionExactlyOnce() { + // The GC/close backstop SignedCoreTransaction relies on: the release + // action runs once on the first clean() and never again — so releasing a + // token that was already broadcast/consumed (or closing twice) cannot + // fire a second native release. + val runs = AtomicInteger(0) + val owner = Any() + val cleanable = NativeCleaner.register(owner) { runs.incrementAndGet() } + + cleanable.clean() + cleanable.clean() + + assertEquals(1, runs.get()) + } +} diff --git a/packages/rs-platform-wallet-ffi/src/core_wallet/broadcast.rs b/packages/rs-platform-wallet-ffi/src/core_wallet/broadcast.rs index 26fa825fa5..b7b2353821 100644 --- a/packages/rs-platform-wallet-ffi/src/core_wallet/broadcast.rs +++ b/packages/rs-platform-wallet-ffi/src/core_wallet/broadcast.rs @@ -27,6 +27,11 @@ fn classify_broadcast_result( /// Success and `MaybeSent` both permanently consume the handle. A definitive /// rejection also consumes it after releasing the reservation. This prevents /// accidental rebroadcast through the same ownership token. +/// +/// A handle whose wallet generation is no longer registered in the manager +/// (removed, or re-created under the same id) is refused with `NotFound` (98) +/// **before** the network is touched; the handle is consumed and its reservation +/// reconciled. This mirrors the deferred-token path's `WalletRemoved` → 98. #[no_mangle] pub unsafe extern "C" fn core_wallet_broadcast_signed_transaction_v2( handle: Handle, @@ -47,14 +52,46 @@ pub unsafe extern "C" fn core_wallet_broadcast_signed_transaction_v2( "invalid core wallet handle".to_string(), ); }; - if wallet.wallet_id() != finalized.wallet.wallet_id() { + // Same generation identity the registry-token path uses: reject a caller + // handle that names a different wallet generation (e.g. a re-created wallet + // under the same id) before acting through the embedded originating wallet. + if !wallet.is_same_generation(&finalized.wallet) { runtime().block_on(finalized.wallet.abandon_transaction(&finalized.transaction)); return PlatformWalletFFIResult::err( PlatformWalletFFIResultCode::ErrorInvalidParameter, - "transaction was finalized by a different wallet".to_string(), + "transaction was finalized by a different wallet generation".to_string(), ); } let local_txid = finalized.transaction.transaction().txid(); + + // Hold this generation's lifecycle gate across BOTH the liveness check and + // the send. The `is_same_generation` check above compares two HANDLES, so it + // passes for a removed generation — both sides name the same removed wallet — + // and nothing further down re-checks: `broadcast_finalized_transaction` goes + // straight to the broadcaster with no manager lookup. Without this, two + // retained handles push a deleted wallet's transaction onto the network, + // where it can conflict with inputs a re-created generation has since + // selected (`dashpay/platform#4185`). + // + // The gate makes this atomic rather than check-then-act: a teardown takes the + // exclusive side, so it cannot interleave between the check and the send. + // Scoped per generation, so this send — up to the broadcaster's timeout — + // blocks only THIS wallet's teardown, never an unrelated wallet's. + let (_lifecycle, wallet_is_live) = runtime().block_on(async { + let gate = wallet.generation_payment_guard().await; + let live = wallet.is_current_generation().await; + (gate, live) + }); + if !wallet_is_live { + runtime().block_on(finalized.wallet.abandon_transaction(&finalized.transaction)); + return PlatformWalletFFIResult::err( + PlatformWalletFFIResultCode::NotFound, + "wallet is no longer registered in the manager (removed or re-created); the \ + transaction was NOT broadcast and its reservation was reconciled" + .to_string(), + ); + } + let result = runtime().block_on( finalized .wallet @@ -90,7 +127,8 @@ pub unsafe extern "C" fn core_wallet_abandon_signed_transaction_v2( "invalid core wallet handle".to_string(), ); }; - if wallet.wallet_id() != transaction.wallet.wallet_id() { + // Same generation identity as the broadcast path / registry-token path. + if !wallet.is_same_generation(&transaction.wallet) { runtime().block_on( transaction .wallet @@ -98,7 +136,7 @@ pub unsafe extern "C" fn core_wallet_abandon_signed_transaction_v2( ); return PlatformWalletFFIResult::err( PlatformWalletFFIResultCode::ErrorInvalidParameter, - "transaction was finalized by a different wallet".to_string(), + "transaction was finalized by a different wallet generation".to_string(), ); } runtime().block_on( @@ -289,6 +327,27 @@ mod tests { runtime().block_on(core.abandon_transaction(&retry)); } + /// Prove the funding reservation is *still held*: a fresh finalize of the + /// same size cannot reselect the single fixture UTXO, so it fails at the + /// build stage. Used to show an aged abandon/free skipped the by-outpoint + /// release (leaving the input reserved for key-wallet's TTL). + fn assert_still_reserved(core: &TestCore, signer: &WalletSigner, tag: u8) { + let rebuild = runtime().block_on(core.finalize_transaction( + TransactionBuilder::new().add_output( + &Address::dummy(Network::Testnet, usize::from(tag)), + 1_000_000, + ), + AccountTypePreference::BIP44, + 0, + signer, + )); + assert!( + rebuild.is_err(), + "aged abandon/free must skip the release, leaving the input reserved; \ + got {rebuild:?}" + ); + } + #[test] fn double_free_is_safe_and_releases_reservation() { let (core, signer) = @@ -328,6 +387,56 @@ mod tests { CORE_WALLET_STORAGE.remove(other_handle); } + /// The deinit/GC backstop (`core_wallet_signed_transaction_v2_free`) is the + /// exact path shumkov flagged: a `FinalizedCoreTransaction` never broadcast + /// or abandoned, freed by the host GC long after finalize. If the reservation + /// has aged past the guard bound the free must **skip** the by-outpoint + /// release — key-wallet's TTL may already have swept and re-reserved the + /// outpoint, and releasing it would free that newer build's reservation. The + /// handle is still torn down (the storage entry is removed) so a re-free is a + /// safe no-op. + #[test] + fn aged_v2_free_skips_reservation_release() { + let (core, signer) = + runtime().block_on(funded_spv_core_wallet(StandardAccountType::BIP44Account)); + let transaction_handle = insert(&core, finalize(&core, &signer, 48)); + + // Age the pinned handle past the guard bound (still below the TTL, so the + // reservation is provably still held — only the software guard trips). + runtime().block_on(platform_wallet::test_support::age_core_past_reservation_guard(&core)); + + core_wallet_signed_transaction_v2_free(transaction_handle); + + // The aged free skipped the release: the input is still reserved. + assert_still_reserved(&core, &signer, 49); + // Handle is gone regardless — a re-free is a harmless no-op. + core_wallet_signed_transaction_v2_free(transaction_handle); + } + + /// The FFI broadcast/abandon *failure* paths (invalid or wrong-generation + /// wallet handle) route their cleanup through `abandon_transaction`, so they + /// inherit the same age guard: when the handle has aged out, the failure-path + /// cleanup must skip the by-outpoint release rather than free a possibly + /// re-reserved outpoint. + #[test] + fn aged_failure_path_abandon_skips_reservation_release() { + let (origin, signer) = + runtime().block_on(funded_spv_core_wallet(StandardAccountType::BIP44Account)); + let transaction_handle = insert(&origin, finalize(&origin, &signer, 50)); + + runtime().block_on(platform_wallet::test_support::age_core_past_reservation_guard(&origin)); + + // Invalid wallet handle → routes through abandon_transaction, then returns + // ErrorInvalidHandle. The embedded aged reservation must be left alone. + let invalid = + unsafe { core_wallet_abandon_signed_transaction_v2(u64::MAX, transaction_handle) }; + assert_eq!( + invalid.code, + PlatformWalletFFIResultCode::ErrorInvalidHandle + ); + assert_still_reserved(&origin, &signer, 51); + } + #[test] fn abandon_then_free_or_broadcast_cannot_reconsume_handle() { let (core, signer) = 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 8e12ebc178..01c0cf4167 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; +pub(crate) 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 0000000000..f352b3329f --- /dev/null +++ b/packages/rs-platform-wallet-ffi/src/core_wallet/signed_payment.rs @@ -0,0 +1,145 @@ +//! 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 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. +pub(crate) static SIGNED_PAYMENT_REGISTRY: Lazy> = + Lazy::new(SignedPaymentRegistry::new); + +/// Serializes tests that reason about the process-global registry's *contents*. +/// +/// [`SIGNED_PAYMENT_REGISTRY`] is one static shared by every test in the binary, +/// and the harness runs tests in parallel threads by default. Any test that +/// captures an `outstanding()` baseline and then asserts a delta against it is +/// therefore racing every other test that mints or consumes a token — the +/// baseline can be captured while a sibling's token is outstanding and compared +/// after that sibling consumed it. +/// +/// Tests take this around their whole body. Poisoning is recovered rather than +/// propagated (mirroring `SignedPaymentRegistry`'s own lock): a panic in one +/// test should fail that test, not cascade into every sibling. +#[cfg(test)] +pub(crate) static REGISTRY_TEST_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + +/// Take [`REGISTRY_TEST_LOCK`], recovering from poisoning. +#[cfg(test)] +pub(crate) fn registry_test_guard() -> std::sync::MutexGuard<'static, ()> { + REGISTRY_TEST_LOCK + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) +} + +/// Broadcast the payment behind `token` (built earlier via +/// [`core_wallet_signed_payment_finalize`](super::transaction_builder::core_wallet_signed_payment_finalize)), +/// 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 `ErrorReservationTokenConsumed` +/// (35) rather than a second send. `core_handle` must resolve to the same wallet +/// *generation* the token was minted against; a wallet re-created under the same +/// id yields `ErrorReservationWalletMismatch` (36). A token whose reservation +/// may already have aged out of key-wallet's TTL yields +/// `ErrorStaleReservationToken` (34). These three deferred-token failures are +/// distinct codes so a host can message each precisely. 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(ReservationToken::from(token), &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() + } + // Split the three deferred-token failures into distinct sibling codes so + // a host can message each precisely. All are non-retryable-in-place and + // none touched the network. + Err(e @ SignedPaymentError::StaleReservationToken(_)) => PlatformWalletFFIResult::err( + PlatformWalletFFIResultCode::ErrorStaleReservationToken, + e.to_string(), + ), + Err(e @ SignedPaymentError::StaleToken(_)) => PlatformWalletFFIResult::err( + PlatformWalletFFIResultCode::ErrorReservationTokenConsumed, + e.to_string(), + ), + Err(e @ SignedPaymentError::WalletMismatch(_)) => PlatformWalletFFIResult::err( + PlatformWalletFFIResultCode::ErrorReservationWalletMismatch, + e.to_string(), + ), + // The wallet was REMOVED from the manager, so there is no live + // generation to broadcast through. Reported as the existing `NotFound` + // (98) rather than a new code: it is exactly the "the thing you named + // does not exist" case 98 already means, and both hosts already map it. + // Distinct from `ErrorReservationWalletMismatch` (36), where a DIFFERENT + // live generation answers to the same id. Did NOT touch the network and + // is NOT retryable — the wallet is gone. + Err(e @ SignedPaymentError::WalletRemoved(_)) => { + PlatformWalletFFIResult::err(PlatformWalletFFIResultCode::NotFound, 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(ReservationToken::from(token))); + 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 8b79efcf2a..19c49986e3 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; @@ -39,6 +40,9 @@ pub struct FFITransactionBuilder { pub struct FFICoreTransaction { tx_bytes: *mut u8, tx_len: usize, + // Part of the C ABI (the Swift host reads `FFICoreTransaction.fee`); the + // Rust side only writes it, so silence the never-read lint. + #[allow(dead_code)] fee: u64, } @@ -90,6 +94,11 @@ impl From for AccountTypePreference { /// On success `out_transaction_handle` receives an opaque V2 handle. Consume /// it with `core_wallet_broadcast_signed_transaction_v2` or /// `core_wallet_abandon_signed_transaction_v2`. +/// +/// If the host removes (or re-creates) this wallet while the external signer is +/// running, no handle is published: the build's reservation is reconciled and +/// this returns `NotFound` (98), the same code the deferred-token sibling +/// `core_wallet_signed_payment_finalize` uses for that case. #[no_mangle] #[allow(clippy::too_many_arguments)] pub unsafe extern "C" fn core_wallet_tx_builder_finalize( @@ -126,6 +135,43 @@ pub unsafe extern "C" fn core_wallet_tx_builder_finalize( &signer, )); let finalized = unwrap_result_or_return!(finalized); + + // Publishing the V2 handle is gated exactly like the deferred-token sibling + // below (`core_wallet_signed_payment_finalize`). `finalize_transaction` drops + // the wallet-manager write lock before awaiting the (external, possibly slow) + // signer, so the host can have removed this wallet while we were signing — + // and that removal's V2-handle sweep has then ALREADY run. Inserting now + // would publish a live handle for a removed generation that no later sweep + // catches, and `core_wallet_broadcast_signed_transaction_v2` would happily + // push it to the network: its `is_same_generation` check compares two + // handles, and a removed generation matches itself (`dashpay/platform#4185`). + // + // Hold THIS generation's lifecycle gate across BOTH the liveness check and + // the insert, so a teardown cannot interleave between them. Acquired AFTER + // the signer await, never around it: holding it across an open signing prompt + // would stall this wallet's teardown for as long as the user takes, and the + // check makes that unnecessary. + let (_lifecycle, wallet_is_live) = runtime().block_on(async { + let gate = wallet.core().generation_payment_guard().await; + let live = wallet.core().is_current_generation().await; + (gate, live) + }); + if !wallet_is_live { + // No handle was published, so nothing would ever release this build's + // reservation. Reconcile it here: the release is generation-bound, so on + // a genuine removal it is a logged no-op (the `ReservationSet` died with + // the generation), and on a re-create it correctly declines to touch the + // new generation's inputs. + runtime().block_on(wallet.core().abandon_transaction(&finalized)); + return PlatformWalletFFIResult::err( + PlatformWalletFFIResultCode::NotFound, + "wallet is no longer registered in the manager (removed or re-created while the \ + transaction was being signed); no transaction handle was published and its \ + reservation was reconciled" + .to_string(), + ); + } + *out_transaction_handle = CORE_SIGNED_TRANSACTION_V2_STORAGE.insert(FFICoreSignedTransactionV2 { wallet: wallet.core().clone(), @@ -134,6 +180,185 @@ 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); + + // `finalize_transaction` drops the wallet-manager write lock before awaiting + // the (external, possibly slow) signer, so the host can have removed this + // wallet while we were signing — and that removal's registry sweep has then + // ALREADY run. Registering now would insert a live token for a removed + // generation, which no later sweep would catch, defeating the teardown + // invariant that dropping tokens makes stale handles inert + // (`dashpay/platform#4185`). + // + // Take THIS wallet generation's lifecycle gate (shared — concurrent payments + // are unaffected) and hold it across BOTH the liveness check and the + // synchronous `register`, so a teardown cannot interleave between them. + // Deliberately acquired AFTER the signer await rather than around it: holding + // it across an open signing prompt would stall this wallet's teardown for as + // long as the user takes, and the check below makes that unnecessary. + let (_lifecycle, wallet_is_live) = runtime().block_on(async { + let gate = wallet.core().generation_payment_guard().await; + let live = wallet.core().is_current_generation().await; + (gate, live) + }); + if !wallet_is_live { + // Nothing was registered, so no token would ever release this build's + // reservation. Reconcile it here: the release is generation-bound, so on + // a genuine removal it is a logged no-op (the `ReservationSet` died with + // the generation), and on a re-create it correctly declines to touch the + // new generation's inputs. + runtime().block_on(wallet.core().abandon_transaction(&finalized)); + return PlatformWalletFFIResult::err( + PlatformWalletFFIResultCode::NotFound, + "wallet is no longer registered in the manager (removed or re-created while the \ + payment was being signed); the payment was not registered and its reservation was \ + reconciled" + .to_string(), + ); + } + + 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` CONSUMES the `SignedCoreTransaction` + // ownership object (deriving its transaction, funding account, reservation + // height, and owner-guard token internally) and binds the token to the wallet + // whose `ReservationSet` holds the inputs. Because the object is consumed + // exactly once, this finalize can yield at most one token — no second token + // can ever name the same reservation (`dashpay/platform#4185`, blocker 1). + // + // `register` is SYNCHRONOUS: its reservation-owning insert runs inline with + // no future that could be dropped before its first poll and silently strand + // the consumed reservation (`dashpay/platform#4185`). It also validates that + // this wallet is the exact generation `finalize` bound the payment to; that + // always holds here (we register through the very wallet that finalized), but + // on the impossible mismatch it hands the finalized payment back so we + // release its reservation (owner-guarded) rather than leaking it. + let token = match crate::core_wallet::signed_payment::SIGNED_PAYMENT_REGISTRY + .register(wallet.core().clone(), finalized) + { + Ok(token) => token, + Err(err) => { + runtime().block_on(wallet.core().abandon_transaction(&err.signed)); + return PlatformWalletFFIResult::err( + PlatformWalletFFIResultCode::ErrorReservationWalletMismatch, + "deferred payment was finalized against a different wallet generation".to_string(), + ); + } + }; + + *out_tx = FFICoreTransaction { + tx_bytes: Box::into_raw(serialized.into_boxed_slice()) as *mut u8, + tx_len: len, + fee, + }; + *out_token = token.as_u64(); + *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-platform-wallet-ffi/src/error.rs b/packages/rs-platform-wallet-ffi/src/error.rs index 54a18c2783..35d4118fb0 100644 --- a/packages/rs-platform-wallet-ffi/src/error.rs +++ b/packages/rs-platform-wallet-ffi/src/error.rs @@ -183,7 +183,81 @@ pub enum PlatformWalletFFIResultCode { /// `PlatformWalletResultCode.errorShutdownIncomplete`. ErrorShutdownIncomplete = 27, - NotFound = 98, // Used exclusively for all the Option that are retuned as errors + // Codes 27-33 are claimed outside this PR and MUST NOT be reused here. + // The deferred-token trio below therefore occupies the contiguous block + // 34-36. Current owners (see ERROR_CODE_REGISTRY.md, dashpay/platform#4261): + // + // 27 ErrorShutdownIncomplete MERGED on v4.2-dev (dashpay/platform#4268) + // 28 (free — vacated by this PR) + // 29 ErrorAssetLockInsufficientFunds dashpay/platform#4184 + // 30 (free — vacated by this PR) + // 31 ErrorSigningKeyUnavailable dashpay/platform#4183, #4259 + // 32 ErrorTransactionBuild dashpay/platform#4247, #4256 + // 33 ErrorTransactionSigning dashpay/platform#4256 + // + // This trio previously sat at 26-28, then 27/28/30. It moved to 34-36 after + // #4268 merged `ErrorShutdownIncomplete = 27` into the v4.2-dev ABI; the + // contiguous block above every current claim ends the renumbering churn. + /// Maps `SignedPaymentError::StaleReservationToken` from the deferred + /// build → broadcast/release core-send lifecycle (`core_wallet_signed_payment_*`): + /// the token has outlived the registry's `RESERVATION_MAX_AGE_BLOCKS` bound + /// and its funding reservation may already have been swept and re-selected by + /// key-wallet's TTL, so acting on it could touch a newer, unrelated + /// reservation. The operation did NOT touch the network. NOT retryable in + /// place — the host must rebuild the payment. + /// + /// Sibling codes split out the other two deferred-token failures that this + /// code used to conflate: [`Self::ErrorReservationTokenConsumed`] (35, + /// unknown / already broadcast / already released) and + /// [`Self::ErrorReservationWalletMismatch`] (36, minted against a different + /// wallet generation). All three are non-retryable-in-place and none touched + /// the network; they are distinct codes so a host can message each precisely. + /// + /// Also maps `PlatformWalletError::StaleReservation` from the atomic V2 + /// finalized-transaction handle path + /// (`core_wallet_broadcast_signed_transaction_v2`): a pinned handle whose + /// funding reservation aged past the SAME `RESERVATION_MAX_AGE_BLOCKS` bound + /// carries the identical "may already have been swept — rebuild" meaning, so + /// the two surfaces intentionally share this one code. The V2 handle carries + /// no numeric reservation token, hence a distinct (token-less) wallet-error + /// variant behind the same FFI code. Abandon/free of a V2 handle never + /// surfaces this — releasing an aged reservation is always allowed. + ErrorStaleReservationToken = 34, + + /// Maps `SignedPaymentError::StaleToken`. The deferred reservation token is + /// unknown, already broadcast, or already released — the guard that turns a + /// double-broadcast (or a broadcast after release) into a typed error + /// instead of a second send. Did NOT touch the network; NOT retryable + /// (rebuild the payment). Release is idempotent and never surfaces this. + ErrorReservationTokenConsumed = 35, + + /// Maps `SignedPaymentError::WalletMismatch`. The deferred reservation token + /// was minted against a different wallet *generation* than the one it is + /// being broadcast through (e.g. a wallet re-created under the same id); its + /// reservation lives in that other generation's `ReservationSet`. Did NOT + /// touch the network and did NOT consume the rightful owner's token; NOT + /// retryable through this handle (rebuild the payment). + /// + ErrorReservationWalletMismatch = 36, + + /// The named thing does not exist. + /// + /// Originally (and still mostly) the code for every `Option` returned as an + /// error — a handle that resolves to nothing, a lookup that came back empty. + /// + /// The deferred build → broadcast/release lifecycle also reports its + /// wallet-was-REMOVED case here rather than minting a fourth + /// deferred-token code, because it *is* that same "does not exist" case: + /// `core_wallet_signed_payment_broadcast` maps + /// `SignedPaymentError::WalletRemoved` (the token's wallet is no longer + /// registered in the manager), and `core_wallet_signed_payment_finalize` + /// refuses to register a payment whose wallet was removed while it was being + /// signed — reconciling that build's reservation before returning. Neither + /// touched the network. Contrast [`Self::ErrorReservationWalletMismatch`] + /// (36), where a DIFFERENT live generation answers to the same wallet id; + /// here there is no live generation at all, so there is nothing to retry + /// against (`dashpay/platform#4185`). + NotFound = 98, ErrorUnknown = 99, } @@ -325,6 +399,14 @@ impl From for PlatformWalletFFIResult { PlatformWalletError::TransactionBroadcast(..) => { PlatformWalletFFIResultCode::ErrorTransactionBroadcastRejected } + // The V2 finalized-transaction handle path's age guard. Shares the + // `ErrorStaleReservationToken` code with the deferred registry-token + // sibling (`SignedPaymentError::StaleReservationToken`): both mean + // "the funding reservation may already have been swept — rebuild", + // and neither touched the network. See the code's doc note. + PlatformWalletError::StaleReservation => { + PlatformWalletFFIResultCode::ErrorStaleReservationToken + } // A definitively-failed address-nonce race (reaches the blanket impl // via identity `top_up_from_addresses` → `?`/`.into()`). Exposing // provided/expected nonce as structured out-fields is INTENTIONALLY @@ -803,6 +885,34 @@ mod tests { assert_eq!(msg, rendered, "Display payload must survive verbatim"); } + /// The V2 finalized-transaction handle age guard + /// (`core_wallet_broadcast_signed_transaction_v2` → `broadcast_finalized_transaction`) + /// surfaces `PlatformWalletError::StaleReservation` through the blanket + /// `From` impl, which must reuse the deferred registry-token path's + /// `ErrorStaleReservationToken` (34) code rather than flattening to + /// `ErrorUnknown` — the two surfaces share the "reservation may have been + /// swept; rebuild" meaning and this one code. The typed Display rendering + /// survives across the boundary as the message. + #[test] + fn stale_reservation_maps_to_shared_stale_reservation_code() { + let err = PlatformWalletError::StaleReservation; + let rendered = err.to_string(); + let result: PlatformWalletFFIResult = err.into(); + assert_eq!( + result.code, + PlatformWalletFFIResultCode::ErrorStaleReservationToken, + "StaleReservation must reuse the registry-token stale code (rendered: {rendered})" + ); + assert!(!result.message.is_null()); + let msg = unsafe { std::ffi::CStr::from_ptr(result.message) } + .to_string_lossy() + .into_owned(); + assert_eq!( + msg, rendered, + "Display payload must survive the FFI boundary verbatim" + ); + } + /// `AddressNonceMismatch` maps to the dedicated `ErrorAddressNonceMismatch` /// FFI code through the blanket `From` impl (the path identity /// `top_up_from_addresses` takes via `?`/`.into()`) rather than flattening diff --git a/packages/rs-platform-wallet-ffi/src/handle.rs b/packages/rs-platform-wallet-ffi/src/handle.rs index f343e4ccc9..2a5692b2a5 100644 --- a/packages/rs-platform-wallet-ffi/src/handle.rs +++ b/packages/rs-platform-wallet-ffi/src/handle.rs @@ -78,6 +78,21 @@ impl HandleStorage { let mut guard = self.items.write(); guard.get_mut(&handle).map(f) } + + /// Remove (and drop) every stored item satisfying `predicate`, returning how + /// many were removed. Used to sweep a wallet generation's handles at + /// teardown (e.g. abandon every finalized-transaction V2 handle whose + /// originating wallet was just removed from its manager — the reservation + /// ceases to exist with the generation, so dropping is the correct action). + pub fn remove_matching(&self, predicate: F) -> usize + where + F: Fn(&T) -> bool, + { + let mut guard = self.items.write(); + let before = guard.len(); + guard.retain(|_, item| !predicate(item)); + before - guard.len() + } } impl Default for HandleStorage { diff --git a/packages/rs-platform-wallet-ffi/src/manager.rs b/packages/rs-platform-wallet-ffi/src/manager.rs index c3aba1491b..23daa81d5c 100644 --- a/packages/rs-platform-wallet-ffi/src/manager.rs +++ b/packages/rs-platform-wallet-ffi/src/manager.rs @@ -506,6 +506,61 @@ pub unsafe extern "C" fn platform_wallet_manager_destroy( PlatformWalletFFIResult::ok() } +/// Remove one wallet from the manager, tearing down its generation's deferred +/// state in the same linearization step. +/// +/// Generic over the persister so tests can drive the exact production sequence +/// with the in-crate test fixture (the FFI handle storage is pinned to +/// [`FFIPersister`](crate::persistence::FFIPersister)). The ordering here is the +/// invariant under test — see the `remove_wallet_lifecycle_tests` module. +pub(crate) async fn remove_wallet_and_tear_down_generation< + P: platform_wallet::changeset::PlatformWalletPersistence + 'static, +>( + manager: &platform_wallet::PlatformWalletManager

, + wallet_id: &[u8; 32], +) -> Result<(), platform_wallet::PlatformWalletError> { + // The removal and the sweep below are ONE linearization point, taken under + // the REMOVED GENERATION'S OWN lifecycle gate. `remove_wallet_with_teardown` + // owns that gate, so the ordering cannot be got wrong here — or by any other + // caller, including a direct Rust embedder that never goes through this FFI. + // + // What the single step buys (`dashpay/platform#4185`): the removal's own + // `.await`s (shielded-coordinator and identity-sync unregistration) used to + // sit in a gap between the removal and the sweep, and a concurrent + // `core_wallet_signed_payment_broadcast` on a retained handle slipped through + // it — its entry was still registered, `is_same_generation` passes (a removed + // generation matches itself), the age guard is skipped (`last_processed_height` + // is `None` once the wallet is gone, which the guard maps to "not expired"), + // and it reached the broadcaster, pushing a removed wallet's payment onto the + // network. + // + // Acquiring the gate waits for this generation's payment operations that have + // entered their liveness-check/publish section. It does NOT wait for one still + // awaiting an external signer: those take the gate only after the signature + // returns (see `core_wallet_signed_payment_finalize` and + // `core_wallet_tx_builder_finalize`), deliberately, so an open signing prompt + // cannot stall teardown. Such a late finalizer instead observes the removed + // generation at its own liveness check and abandons rather than publishing. + manager + .remove_wallet_with_teardown(wallet_id, |removed| { + // The wallet and its accounts' `ReservationSet`s are now gone from + // the manager, so the deferred-payment reservations cease to exist — + // there is nothing to reconcile. DROP (do not release) this + // generation's registry tokens and its finalized-tx V2 handles. This + // is the teardown half of the single generation policy both deferred + // paths share: it makes any stale handle to the removed generation + // inert, so a later destroy/release of a lingering handle can never + // release-by-outpoint against a re-created generation's inputs. + let core = removed.core(); + crate::core_wallet::signed_payment::SIGNED_PAYMENT_REGISTRY + .remove_entries_for_wallet(core); + crate::handle::CORE_SIGNED_TRANSACTION_V2_STORAGE + .remove_matching(|tx| tx.wallet.is_same_generation(core)); + }) + .await?; + Ok(()) +} + /// Remove one wallet from the manager. Idempotent on missing wallets. #[no_mangle] pub unsafe extern "C" fn platform_wallet_manager_remove_wallet( @@ -516,11 +571,14 @@ pub unsafe extern "C" fn platform_wallet_manager_remove_wallet( let wallet_id_value = *wallet_id; let option = PLATFORM_WALLET_MANAGER_STORAGE.with_item(manager_handle, |manager| { - runtime().block_on(manager.remove_wallet(&wallet_id_value)) + runtime().block_on(remove_wallet_and_tear_down_generation( + manager, + &wallet_id_value, + )) }); let result = unwrap_option_or_return!(option); match result { - Ok(_) => PlatformWalletFFIResult::ok(), + Ok(()) => PlatformWalletFFIResult::ok(), // Idempotency: a wallet that's already gone is the success // state callers want. Everything else is a real failure. Err(platform_wallet::PlatformWalletError::WalletNotFound(_)) => { @@ -806,3 +864,571 @@ mod tests { assert_eq!(result.code, PlatformWalletFFIResultCode::Success); } } + +/// Wallet-generation teardown vs. the deferred-payment registry +/// (`dashpay/platform#4185`). +/// +/// The invariant every test here defends is one sentence: **no deferred-payment +/// token for a wallet that is not currently registered in the manager is ever +/// actionable.** Removal and the registry sweep used to be two independent steps +/// with the removal's own `.await`s in the gap, and `register` could land after +/// the sweep, so the invariant held only by timing. +/// +/// These drive [`remove_wallet_and_tear_down_generation`] — the exact sequence +/// `platform_wallet_manager_remove_wallet` runs — rather than the `extern "C"` +/// wrapper, because the FFI handle storage is pinned to `FFIPersister` while the +/// wallet fixture uses the in-crate test persister. The wrapper adds only handle +/// resolution and error-code mapping on top. +#[cfg(test)] +mod remove_wallet_lifecycle_tests { + use super::*; + use crate::core_wallet::signed_payment::SIGNED_PAYMENT_REGISTRY; + use key_wallet::wallet::managed_wallet_info::transaction_building::AccountTypePreference; + use platform_wallet::test_support::test_platform_wallet_manager; + use platform_wallet::{ + CoreWallet, ReservationToken, SignedCoreTransaction, SignedPaymentError, + }; + use std::sync::atomic::{AtomicUsize, Ordering}; + use std::sync::Arc; + use std::time::Duration; + + fn dummy_tx() -> dashcore::Transaction { + dashcore::Transaction { + version: 3, + lock_time: 0, + input: vec![], + output: vec![], + special_transaction_payload: None, + } + } + + /// Mint a token against `core`. The dummy tx reserved nothing (height 0, no + /// funding token), so these tests exercise the lifecycle guards rather than + /// the age or owner guard. + fn register_token( + core: &CoreWallet, + ) -> ReservationToken { + SIGNED_PAYMENT_REGISTRY + .register( + core.clone(), + SignedCoreTransaction::new_for_test( + dummy_tx(), + 0, + AccountTypePreference::BIP44, + 0, + 0, + None, + core.test_generation_marker(), + ), + ) + .expect("register binds to the finalizing generation") + } + + /// A token is dead iff broadcasting it reports it as unknown/consumed. Token- + /// scoped on purpose: the registry is a process-global shared with every + /// other test in the binary, so `outstanding()` deltas are not reliable under + /// the default parallel test harness. + async fn assert_token_is_gone( + token: ReservationToken, + core: &CoreWallet, + ) { + match SIGNED_PAYMENT_REGISTRY.broadcast(token, core).await { + Err(SignedPaymentError::StaleToken(t)) if t == token => {} + other => panic!("token {token} should have been swept, got {other:?}"), + } + } + + /// Requirement: a broadcast must FAIL CLEANLY when the wallet is no longer in + /// the manager, rather than silently proceeding to the network. + /// + /// The setup reproduces the in-flight-finalizer resurrection directly: + /// register AFTER teardown has already swept, which is exactly what + /// `core_wallet_signed_payment_finalize` used to do when the host removed the + /// wallet during the signer await. Before the fix this token was fully + /// actionable — `is_same_generation` passes (a removed generation matches + /// itself), `last_processed_height` is `None` so the age guard is skipped, + /// and `broadcast_payment_releasing_reservation` has no wallet-existence gate + /// — so the payment went to the broadcaster. + #[test] + fn broadcasting_a_token_for_a_removed_wallet_is_refused_before_the_network() { + // Shares the process-global registry with `wallet::destroy_tests`, + // which asserts on `outstanding()` counts — serialize against it. + let _registry = crate::core_wallet::signed_payment::registry_test_guard(); + + runtime().block_on(async { + let (manager, wallet_id) = test_platform_wallet_manager().await; + let wallet = manager + .get_wallet(&wallet_id) + .await + .expect("wallet present"); + let core = wallet.core().clone(); + + remove_wallet_and_tear_down_generation(&manager, &wallet_id) + .await + .expect("remove succeeds"); + assert!( + !core.is_current_generation().await, + "the retained handle must observe its generation as gone" + ); + + let token = register_token(&core); + + match SIGNED_PAYMENT_REGISTRY.broadcast(token, &core).await { + Err(SignedPaymentError::WalletRemoved(t)) if t == token => {} + other => panic!( + "a token whose wallet was removed must be refused without a send, got {other:?}" + ), + } + + // Refusing still CONSUMES the token: the generation is gone, so there + // is nothing to reconcile and nothing to retry. + assert_token_is_gone(token, &core).await; + }); + } + + /// Requirement: removal and the registry sweep are linearized with respect to + /// broadcast. Run repeatedly to shake the interleaving. + /// + /// Both orderings are legal, so the assertion cannot simply be "the broadcast + /// is refused": + /// + /// * teardown first → the entry is swept and the broadcast is refused + /// (`StaleToken`), or the wallet is gone and it is refused + /// (`WalletRemoved`); + /// * broadcast first → it holds the shared gate, the wallet is genuinely + /// still live, the payment legitimately goes to the broadcaster, and the + /// teardown waits. + /// + /// What must be impossible is the combination the pre-fix gap allowed: + /// reaching the broadcaster even though teardown had ALREADY completed. That + /// is what the completion-order tickets pin down. Because the gate serializes + /// the two, a broadcast that reached the broadcaster must have been holding + /// the gate, so the teardown cannot have finished before it — i.e. the + /// sender's ticket must precede the remover's. Without the gate the remover + /// could finish first and the send still go out, which is exactly the + /// `dashpay/platform#4185` finding. + #[test] + fn a_broadcast_never_reaches_the_broadcaster_after_teardown_completed() { + // Shares the process-global registry with `wallet::destroy_tests`, + // which asserts on `outstanding()` counts — serialize against it. + let _registry = crate::core_wallet::signed_payment::registry_test_guard(); + + for iteration in 0..25 { + runtime().block_on(async { + let (manager, wallet_id) = test_platform_wallet_manager().await; + let wallet = manager + .get_wallet(&wallet_id) + .await + .expect("wallet present"); + let core = wallet.core().clone(); + let token = register_token(&core); + + let barrier = Arc::new(tokio::sync::Barrier::new(2)); + // Monotonic tickets stamped the instant each operation returns, + // giving a total order over the two completions. + let ticket = Arc::new(AtomicUsize::new(0)); + + let remover = { + let barrier = Arc::clone(&barrier); + let manager = Arc::clone(&manager); + let ticket = Arc::clone(&ticket); + tokio::spawn(async move { + barrier.wait().await; + let outcome = + remove_wallet_and_tear_down_generation(&manager, &wallet_id).await; + (outcome, ticket.fetch_add(1, Ordering::SeqCst)) + }) + }; + let sender = { + let barrier = Arc::clone(&barrier); + let core = core.clone(); + let ticket = Arc::clone(&ticket); + tokio::spawn(async move { + barrier.wait().await; + let outcome = SIGNED_PAYMENT_REGISTRY.broadcast(token, &core).await; + (outcome, ticket.fetch_add(1, Ordering::SeqCst)) + }) + }; + + let (removed, remover_ticket) = remover.await.expect("remover task"); + removed.expect("remove succeeds"); + let (sent, sender_ticket) = sender.await.expect("sender task"); + + // `Ok` is unreachable in-test (the fixture's SPV client is not + // started, so the broadcaster errors), but it is the same class + // of outcome: the payment was handed to the network layer. + let reached_broadcaster = + matches!(sent, Ok(_) | Err(SignedPaymentError::Broadcast(_))); + if reached_broadcaster { + assert!( + sender_ticket < remover_ticket, + "iteration {iteration}: a payment reached the broadcaster even though \ + wallet teardown had already completed — removal is not linearized with \ + broadcast (got {sent:?})" + ); + } else { + // The only other legal outcomes are the two clean refusals. + assert!( + matches!( + sent, + Err(SignedPaymentError::StaleToken(_)) + | Err(SignedPaymentError::WalletRemoved(_)) + ), + "iteration {iteration}: unexpected outcome {sent:?}" + ); + } + + // Whichever way it went, nothing survives teardown. + assert!(!core.is_current_generation().await); + assert_token_is_gone(token, &core).await; + }); + } + } + + /// Requirement: teardown WAITS for an in-flight finalizer, so a late + /// `register` cannot resurrect a token for a removed generation. + /// + /// Deterministic. The held shared guard stands in for + /// `core_wallet_signed_payment_finalize` sitting between its liveness check + /// and its synchronous `register`. Before the fix nothing connected those two + /// operations: the teardown ran to completion — sweep included — while the + /// finalizer was signing, and the token it then inserted was permanently + /// outside any sweep. + #[test] + fn teardown_waits_for_an_in_flight_finalizer_and_then_sweeps_its_token() { + // Shares the process-global registry with `wallet::destroy_tests`, + // which asserts on `outstanding()` counts — serialize against it. + let _registry = crate::core_wallet::signed_payment::registry_test_guard(); + + runtime().block_on(async { + let (manager, wallet_id) = test_platform_wallet_manager().await; + let wallet = manager + .get_wallet(&wallet_id) + .await + .expect("wallet present"); + let core = wallet.core().clone(); + + // The finalizer enters THIS generation's gate (as the FFI does after + // signing). + let in_flight = core.generation_payment_guard().await; + + let teardown = { + let manager = Arc::clone(&manager); + tokio::spawn(async move { + remove_wallet_and_tear_down_generation(&manager, &wallet_id).await + }) + }; + + // Teardown must block on the exclusive side of the gate. + tokio::time::sleep(Duration::from_millis(200)).await; + assert!( + !teardown.is_finished(), + "teardown must wait for the in-flight finalizer to leave the gate" + ); + + // Because teardown is still waiting, the finalizer's liveness check + // sees a live wallet and its register is legitimate. + assert!( + core.is_current_generation().await, + "the wallet must still be live while a finalizer holds the gate" + ); + let token = register_token(&core); + + drop(in_flight); + teardown + .await + .expect("teardown task") + .expect("remove succeeds"); + + // The teardown that was waiting sweeps the token the finalizer + // inserted — the invariant the gate exists to restore. + assert_token_is_gone(token, &core).await; + }); + } + + // --------------------------------------------------------------------- + // V2 finalized-transaction-handle path (`dashpay/platform#4185` review). + // + // The registry-token path above was gated first; the V2 path + // (`core_wallet_tx_builder_finalize` → `CORE_SIGNED_TRANSACTION_V2_STORAGE` + // → `core_wallet_broadcast_signed_transaction_v2`) reaches the SAME + // broadcaster through a retained handle and was left ungated. Its + // `is_same_generation` check compares two HANDLES, and a removed generation + // matches itself, so two retained handles pushed a deleted wallet's + // transaction onto the network. + // --------------------------------------------------------------------- + + /// Publish a V2 finalized-transaction handle for `core`'s generation, the + /// way `core_wallet_tx_builder_finalize` does. + fn publish_v2_handle( + core: &CoreWallet, + ) -> Handle { + crate::handle::CORE_SIGNED_TRANSACTION_V2_STORAGE.insert( + crate::core_wallet::FFICoreSignedTransactionV2 { + wallet: core.clone(), + transaction: SignedCoreTransaction::new_for_test( + dummy_tx(), + 0, + AccountTypePreference::BIP44, + 0, + 0, + None, + core.test_generation_marker(), + ), + }, + ) + } + + /// Requirement: a V2 handle whose wallet generation was removed must be + /// refused BEFORE the network, exactly as the registry-token path is. + /// + /// Deterministic. The setup reproduces the late-finalizer publication + /// directly: publish the handle AFTER teardown has already swept, which is + /// what an ungated `core_wallet_tx_builder_finalize` does when the host + /// removes the wallet during the signer await. + /// + /// Before the fix this handle was fully actionable — the caller handle and + /// the embedded originating handle name the same removed generation, so + /// `is_same_generation` passes, and `broadcast_finalized_transaction` goes + /// straight to the broadcaster with no manager lookup at all — so the + /// transaction went to the network. + #[test] + fn broadcasting_a_v2_handle_for_a_removed_wallet_is_refused_before_the_network() { + let _registry = crate::core_wallet::signed_payment::registry_test_guard(); + + // The extern "C" entry points call `runtime().block_on` themselves, so + // they must be invoked from OUTSIDE a runtime context — do the async + // setup first, then call across the boundary. + let core = runtime().block_on(async { + let (manager, wallet_id) = test_platform_wallet_manager().await; + let wallet = manager + .get_wallet(&wallet_id) + .await + .expect("wallet present"); + let core = wallet.core().clone(); + + remove_wallet_and_tear_down_generation(&manager, &wallet_id) + .await + .expect("remove succeeds"); + assert!( + !core.is_current_generation().await, + "the retained handle must observe its generation as gone" + ); + core + }); + + let transaction_handle = publish_v2_handle(&core); + let core_handle = crate::handle::CORE_WALLET_STORAGE.insert(core.clone()); + + let mut out_txid: *mut std::os::raw::c_char = std::ptr::null_mut(); + let result = unsafe { + crate::core_wallet::core_wallet_broadcast_signed_transaction_v2( + core_handle, + transaction_handle, + &mut out_txid, + ) + }; + + assert_eq!( + result.code, + PlatformWalletFFIResultCode::NotFound, + "a V2 handle whose wallet was removed must be refused without a send" + ); + assert!( + out_txid.is_null(), + "no txid may be produced for a refused broadcast" + ); + + // Refusing still CONSUMES the handle: the generation is gone, so there is + // nothing to reconcile and nothing to retry. + assert!( + crate::handle::CORE_SIGNED_TRANSACTION_V2_STORAGE + .remove(transaction_handle) + .is_none(), + "the refused V2 handle must have been consumed" + ); + } + + /// Requirement: a teardown WAITS for an in-flight V2 operation on that + /// generation, then sweeps its handle — so a V2 handle can never be published + /// into an already-swept storage and outlive its wallet. + /// + /// Deterministic. The held shared guard stands in for + /// `core_wallet_tx_builder_finalize` sitting between its liveness check and + /// its insert, or `core_wallet_broadcast_signed_transaction_v2` sitting + /// between its liveness check and the send. Before the fix the V2 path took no + /// gate at all: the teardown ran to completion — sweep included — while the + /// finalizer was signing, and the handle it then published was permanently + /// outside any sweep and fully broadcastable. + #[test] + fn teardown_waits_for_an_in_flight_v2_operation_and_then_sweeps_its_handle() { + let _registry = crate::core_wallet::signed_payment::registry_test_guard(); + + let (core, transaction_handle) = runtime().block_on(async { + let (manager, wallet_id) = test_platform_wallet_manager().await; + let wallet = manager + .get_wallet(&wallet_id) + .await + .expect("wallet present"); + let core = wallet.core().clone(); + + // The V2 operation enters this generation's gate (as the FFI does + // after signing). + let in_flight = core.generation_payment_guard().await; + + let teardown = { + let manager = Arc::clone(&manager); + tokio::spawn(async move { + remove_wallet_and_tear_down_generation(&manager, &wallet_id).await + }) + }; + + // Teardown must block on the exclusive side of the gate. + tokio::time::sleep(Duration::from_millis(200)).await; + assert!( + !teardown.is_finished(), + "teardown must wait for the in-flight V2 operation to leave the gate" + ); + + // Because teardown is still waiting, the operation's liveness check + // sees a live wallet and its publish is legitimate. + assert!( + core.is_current_generation().await, + "the wallet must still be live while a V2 operation holds the gate" + ); + let transaction_handle = publish_v2_handle(&core); + + drop(in_flight); + teardown + .await + .expect("teardown task") + .expect("remove succeeds"); + + assert!(!core.is_current_generation().await); + (core, transaction_handle) + }); + + // The teardown that was waiting swept the handle the V2 operation + // published — the invariant the gate exists to restore. + assert!( + crate::handle::CORE_SIGNED_TRANSACTION_V2_STORAGE + .remove(transaction_handle) + .is_none(), + "the in-flight V2 operation's handle must have been swept by teardown" + ); + drop(core); + } + + /// Requirement: the lifecycle gate belongs to shared wallet-generation state, + /// so removal driven through the PUBLIC Rust API is excluded too — not just + /// removal driven through this crate's FFI wrapper. + /// + /// Deterministic. The held shared guard stands in for any payment operation + /// sitting between its liveness check and the action that check authorizes (a + /// register, or a send). `PlatformWalletManager` is public and + /// `SignedPaymentRegistry` is re-exported from `platform-wallet`, so a direct + /// Rust embedder reaches `remove_wallet` without ever touching + /// [`remove_wallet_and_tear_down_generation`]. While the gate lived on the + /// FFI's process-global registry singleton, that path took the write side + /// nowhere at all: removal ran straight through, and the payment then acted on + /// a generation the manager had already dropped. + #[test] + fn public_remove_wallet_waits_for_an_in_flight_payment_on_that_generation() { + let _registry = crate::core_wallet::signed_payment::registry_test_guard(); + + runtime().block_on(async { + let (manager, wallet_id) = test_platform_wallet_manager().await; + let wallet = manager + .get_wallet(&wallet_id) + .await + .expect("wallet present"); + let core = wallet.core().clone(); + + // A payment operation on this generation is in flight. + let in_flight = core.generation_payment_guard().await; + + let remover = { + let manager = Arc::clone(&manager); + tokio::spawn(async move { manager.remove_wallet(&wallet_id).await.map(|_| ()) }) + }; + + tokio::time::sleep(Duration::from_millis(200)).await; + assert!( + !remover.is_finished(), + "PlatformWalletManager::remove_wallet must wait for an in-flight payment on the \ + generation it is removing — the public removal path takes no lifecycle exclusion" + ); + + // Because the removal is still waiting, the in-flight payment's + // liveness check sees a live wallet and its action is legitimate. + assert!( + core.is_current_generation().await, + "the wallet must still be live while a payment holds its generation gate" + ); + + drop(in_flight); + remover + .await + .expect("remover task") + .expect("remove succeeds"); + assert!(!core.is_current_generation().await); + }); + } + + /// Requirement: the gate is scoped to ONE generation, so a slow payment on one + /// wallet cannot stall an unrelated wallet's teardown. + /// + /// Deterministic, and the reason the gate could not stay on the FFI's + /// process-global `SIGNED_PAYMENT_REGISTRY` singleton. A deferred broadcast + /// holds the shared side across an SPV send — up to the broadcaster's timeout + /// — and tokio's `RwLock` is write-preferring, so on one global lock that send + /// blocked teardown, and every payment operation queued behind the waiting + /// writer, for every unrelated wallet in the process. + #[test] + fn an_in_flight_payment_does_not_block_an_unrelated_wallets_teardown() { + let _registry = crate::core_wallet::signed_payment::registry_test_guard(); + + runtime().block_on(async { + let (manager_a, wallet_id_a) = test_platform_wallet_manager().await; + let (manager_b, wallet_id_b) = test_platform_wallet_manager().await; + + let core_a = manager_a + .get_wallet(&wallet_id_a) + .await + .expect("wallet A present") + .core() + .clone(); + let core_b = manager_b + .get_wallet(&wallet_id_b) + .await + .expect("wallet B present") + .core() + .clone(); + assert!( + !core_a.is_same_generation(&core_b), + "the fixture must produce two distinct generations" + ); + + // Wallet A has a payment in flight, holding A's gate. + let in_flight_a = core_a.generation_payment_guard().await; + + // Wallet B's teardown must not care. + let teardown_b = tokio::time::timeout( + Duration::from_secs(5), + remove_wallet_and_tear_down_generation(&manager_b, &wallet_id_b), + ) + .await + .expect( + "an unrelated wallet's teardown must not wait on wallet A's in-flight payment — \ + the lifecycle gate is not scoped to the wallet generation", + ); + teardown_b.expect("remove B succeeds"); + + // A is untouched and still live; B is gone. + assert!(core_a.is_current_generation().await); + assert!(!core_b.is_current_generation().await); + + drop(in_flight_a); + }); + } +} diff --git a/packages/rs-platform-wallet-ffi/src/wallet.rs b/packages/rs-platform-wallet-ffi/src/wallet.rs index 8ffd78a896..df0f6fc9a6 100644 --- a/packages/rs-platform-wallet-ffi/src/wallet.rs +++ b/packages/rs-platform-wallet-ffi/src/wallet.rs @@ -390,6 +390,133 @@ 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 { - PLATFORM_WALLET_STORAGE.remove(handle); + // Destroying a wrapper alias must NOT touch the deferred-payment registry. + // + // `platform_wallet_manager_get_wallet` hands out an independent handle for + // each alias of a wallet *generation*, but none of those wrappers OWN the + // logical wallet — the manager still owns it and can hand out another alias, + // `platform_wallet_get_core` yields independently-owned core handles, and + // each registry entry pins its own `CoreWallet` (keeping the reservation + // live). A registered deferred-payment token is owned by the payment flow + // that minted it, NOT by any wrapper handle, so closing or garbage-collecting + // the last wrapper must leave the token intact: a later merchant ack has to + // remain broadcastable through a retained core handle or a re-acquired alias. + // + // Token cleanup therefore follows the payment owner (an explicit + // broadcast/release) or actual wallet-generation removal + // (`platform_wallet_manager_remove_wallet`, which drops the entries because + // the reservation ceases to exist with the generation) — never a transient + // wrapper-alias count. Dropping this handle just releases its `Arc`s; the + // registry entry's own `CoreWallet` clone keeps the generation alive as long + // as a token references it. (`dashpay/platform#4185`, blocker 2.) + let _ = PLATFORM_WALLET_STORAGE.remove(handle); PlatformWalletFFIResult::ok() } + +#[cfg(test)] +mod destroy_tests { + use super::*; + use crate::core_wallet::signed_payment::SIGNED_PAYMENT_REGISTRY; + use key_wallet::wallet::managed_wallet_info::transaction_building::AccountTypePreference; + use platform_wallet::test_support::test_platform_wallet_manager; + use platform_wallet::SignedCoreTransaction; + + fn dummy_tx() -> dashcore::Transaction { + dashcore::Transaction { + version: 3, + lock_time: 0, + input: vec![], + output: vec![], + special_transaction_payload: None, + } + } + + /// Destroying wrapper alias handles must NEVER invalidate a deferred-payment + /// token — not even when the FINAL alias is destroyed. A wrapper handle does + /// not own the logical wallet or the registered payment; the token is owned + /// by the payment flow that minted it and stays live and actionable until its + /// owner broadcasts/releases it or the generation is actually removed + /// (`platform_wallet_manager_remove_wallet`). Regression for + /// `dashpay/platform#4185` blocker 2: the old final-alias sweep consumed + /// independently-owned payments. + #[test] + fn destroying_wrapper_aliases_never_sweeps_tokens() { + // Asserts `outstanding()` DELTAS against a captured baseline, so it must + // not run while a sibling test mints or consumes tokens in the same + // process-global registry. + let _registry = crate::core_wallet::signed_payment::registry_test_guard(); + + // Async setup only. `platform_wallet_destroy` and the final `release` + // each do their own `runtime().block_on(...)`, exactly as the JNI / + // NativeCleaner threads do (never from inside a tokio runtime). Calling + // them from within an outer `block_on` would nest runtimes and abort, so + // they run on the plain test thread below. + let (manager, handle_a, handle_b, token, baseline) = 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). The + // dummy tx reserved nothing (reservation height 0, no funding token) — + // this test exercises destroy/ownership, not the age or owner guard. + let baseline = SIGNED_PAYMENT_REGISTRY.outstanding(); + let token = SIGNED_PAYMENT_REGISTRY + .register( + core.clone(), + SignedCoreTransaction::new_for_test( + dummy_tx(), + 0, + AccountTypePreference::BIP44, + 0, + 0, + None, + // Bind the finalized payment to this exact wallet + // generation so `register` accepts it (it now validates + // the wallet against the finalizing generation). + core.test_generation_marker(), + ), + ) + .expect("register with the same generation"); + assert_eq!(SIGNED_PAYMENT_REGISTRY.outstanding(), baseline + 1); + (manager, handle_a, handle_b, token, baseline) + }); + + // 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 → the token STILL survives: a wrapper alias + // does not own the payment, so its destruction must not consume the token. + let result = unsafe { platform_wallet_destroy(handle_b) }; + assert_eq!(result.code, PlatformWalletFFIResultCode::Success); + assert_eq!( + SIGNED_PAYMENT_REGISTRY.outstanding(), + baseline + 1, + "destroying the final wrapper alias must NOT sweep an independently-owned token" + ); + + // The token is still fully live: its owner can release it even after both + // wrappers are gone (the registry entry pinned its own `CoreWallet`). + runtime().block_on(SIGNED_PAYMENT_REGISTRY.release(token)); + assert_eq!( + SIGNED_PAYMENT_REGISTRY.outstanding(), + baseline, + "the payment owner can still release the surviving token" + ); + + // Keep the manager alive until the end (owns the wallet + adapter). + drop(manager); + } +} diff --git a/packages/rs-platform-wallet/src/error.rs b/packages/rs-platform-wallet/src/error.rs index 6f34b0cee7..b9596702be 100644 --- a/packages/rs-platform-wallet/src/error.rs +++ b/packages/rs-platform-wallet/src/error.rs @@ -90,6 +90,27 @@ pub enum PlatformWalletError { )] TransactionBroadcastUnconfirmed(String), + /// A finalized V2 transaction handle + /// (`core_wallet_tx_builder_finalize` → `broadcast_finalized_transaction`) + /// was held long enough that its funding reservation may already have been + /// swept and re-selected by key-wallet's TTL: the wallet's + /// `last_processed_height` advanced at least + /// [`RESERVATION_MAX_AGE_BLOCKS`](crate::wallet::reservations::RESERVATION_MAX_AGE_BLOCKS) + /// blocks past the height the reservation was stamped at + /// ([`SignedCoreTransaction::reservation_height`](crate::SignedCoreTransaction::reservation_height)). + /// Broadcasting it could spend against a newer, unrelated reservation, so it + /// is refused **before** touching the network — NOT retryable in place, the + /// caller must rebuild the payment. Abandoning/freeing the handle stays + /// allowed at any age (releasing an old reservation is always safe). + /// + /// This is the V2 handle-path sibling of the deferred registry-token + /// [`SignedPaymentError::StaleReservationToken`](crate::SignedPaymentError::StaleReservationToken); + /// both share the same age bound and the FFI `ErrorStaleReservationToken` + /// code. Carries no token — the handle path is keyed by an opaque handle, + /// not a numeric reservation token. + #[error("finalized transaction reservation has outlived its lifetime; rebuild the payment")] + StaleReservation, + #[error("Transaction building failed: {0}")] TransactionBuild(String), diff --git a/packages/rs-platform-wallet/src/lib.rs b/packages/rs-platform-wallet/src/lib.rs index e91c5ccee0..efde6e58b8 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::{ + RegisterWrongGeneration, 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/manager/load.rs b/packages/rs-platform-wallet/src/manager/load.rs index c746fb802b..65f410f339 100644 --- a/packages/rs-platform-wallet/src/manager/load.rs +++ b/packages/rs-platform-wallet/src/manager/load.rs @@ -5,7 +5,7 @@ use std::sync::Arc; use crate::changeset::{ClientStartState, ClientWalletStartState, PlatformWalletPersistence}; use crate::error::PlatformWalletError; -use crate::wallet::core::WalletBalance; +use crate::wallet::core::WalletGeneration; use crate::wallet::identity::IdentityManager; use crate::wallet::platform_wallet::{PlatformWalletInfo, WalletId}; use crate::wallet::PlatformWallet; @@ -77,7 +77,7 @@ impl PlatformWalletManager

{ tracked_asset_locks.extend(account_locks); } - let balance = Arc::new(WalletBalance::new()); + let generation = Arc::new(WalletGeneration::new()); // Mirror the inner `ManagedWalletInfo.balance` (already // recomputed from the freshly-loaded UTXO set on the FFI // side via `update_balance`) into the lock-free `Arc` the @@ -88,7 +88,7 @@ impl PlatformWalletManager

{ // step has to live inside `platform_wallet` rather than // the FFI loader. let core_balance = &wallet_info.balance; - balance.set( + generation.set( core_balance.confirmed(), core_balance.unconfirmed(), core_balance.immature(), @@ -96,7 +96,7 @@ impl PlatformWalletManager

{ ); let platform_info = PlatformWalletInfo { core_wallet: wallet_info, - balance: Arc::clone(&balance), + generation: Arc::clone(&generation), identity_manager: IdentityManager::from(identity_manager), tracked_asset_locks, }; @@ -156,7 +156,7 @@ impl PlatformWalletManager

{ Arc::clone(&self.sdk), wallet_id, Arc::clone(&self.wallet_manager), - balance, + generation, Arc::clone(&self.lock_notify), Arc::clone(&persister_dyn), broadcaster, diff --git a/packages/rs-platform-wallet/src/manager/wallet_lifecycle.rs b/packages/rs-platform-wallet/src/manager/wallet_lifecycle.rs index 938e0e790e..23086a8e84 100644 --- a/packages/rs-platform-wallet/src/manager/wallet_lifecycle.rs +++ b/packages/rs-platform-wallet/src/manager/wallet_lifecycle.rs @@ -16,7 +16,7 @@ use crate::changeset::{ PlatformWalletPersistence, ProviderKeyAccountEntry, WalletMetadataEntry, }; use crate::error::PlatformWalletError; -use crate::wallet::core::WalletBalance; +use crate::wallet::core::WalletGeneration; use crate::wallet::platform_wallet::{PlatformWalletInfo, WalletId}; use crate::wallet::PlatformWallet; @@ -51,6 +51,33 @@ fn parse_mnemonic_any_language(phrase: &str) -> Result { Err("phrase does not match any supported BIP-39 wordlist") } +/// Test-only rendezvous fired inside [`PlatformWalletManager::remove_wallet_with_teardown`], +/// between the inner-manager removal and the public-map removal. +/// +/// That window is exactly where a concurrent same-id `register_wallet` can +/// publish a NEW generation into both maps — the id is free in the inner +/// manager from the moment the removal above completes, and nothing gates +/// registration. Reproducing it deterministically from outside is not possible: +/// the window is bounded by two *different* locks, and the only lock a test +/// could hold to park the remover inside it (`self.wallets`) is the same lock +/// the registration must acquire to publish, so parking the remover would also +/// block the registration — and `tokio`'s `RwLock` hands the writer queue out +/// in FIFO order, which puts the remover first. A rendezvous is therefore the +/// only way to pin this ordering without a sleep or a completion-order race. +/// +/// Compiled under `cfg(test)` only: neither this static nor its call site +/// exists in a production build, and it is not part of any public API. +#[cfg(test)] +pub(crate) type RemoveWalletMidpointHook = Box< + dyn Fn(&WalletId) -> std::pin::Pin + Send>> + + Send + + Sync, +>; + +#[cfg(test)] +pub(crate) static REMOVE_WALLET_MIDPOINT_HOOK: std::sync::Mutex> = + std::sync::Mutex::new(None); + impl PlatformWalletManager

{ /// Create a PlatformWallet from a BIP39 mnemonic phrase. /// @@ -183,7 +210,7 @@ impl PlatformWalletManager

{ // place below, BEFORE the address-pool snapshot is taken. let mut wallet_info = ManagedWalletInfo::from_wallet(&wallet, birth_height); - let balance = Arc::new(WalletBalance::new()); + let generation = Arc::new(WalletGeneration::new()); // Snapshot per-account xpubs and address-pool entries BEFORE // the wallet / managed-info are moved into insert_wallet. The @@ -333,7 +360,7 @@ impl PlatformWalletManager

{ let platform_info = PlatformWalletInfo { core_wallet: wallet_info, - balance: Arc::clone(&balance), + generation: Arc::clone(&generation), identity_manager: crate::wallet::identity::IdentityManager::new(), tracked_asset_locks: std::collections::BTreeMap::new(), }; @@ -446,7 +473,7 @@ impl PlatformWalletManager

{ Arc::clone(&self.sdk), wallet_id, Arc::clone(&self.wallet_manager), - balance, + generation, Arc::clone(&self.lock_notify), persister_dyn, broadcaster, @@ -567,10 +594,130 @@ impl PlatformWalletManager

{ } /// Remove a wallet from the manager. + /// + /// Runs under the removed generation's lifecycle gate — see + /// [`remove_wallet_with_teardown`](Self::remove_wallet_with_teardown), of + /// which this is the no-extra-teardown case. pub async fn remove_wallet( &self, wallet_id: &WalletId, ) -> Result, PlatformWalletError> { + self.remove_wallet_with_teardown(wallet_id, |_| {}).await + } + + /// Remove a wallet from the manager and run `tear_down` on the removed + /// wallet — both under that generation's exclusive lifecycle gate, as one + /// linearization point. + /// + /// # Why the gate lives here rather than in the caller + /// + /// Removing the generation and tearing down the deferred state that names it + /// (the [`SignedPaymentRegistry`](crate::SignedPaymentRegistry) tokens and + /// the FFI's finalized-transaction handles) must be indivisible. If they are + /// two steps, a retained handle can broadcast in the gap: the removal's own + /// `.await`s (shielded-coordinator and identity-sync unregistration) sit + /// inside it, `CoreWallet::is_same_generation` passes for a removed + /// generation (a removed generation matches itself), and the reservation age + /// guard is disabled once `last_processed_height` returns `None`. So a + /// payment for a wallet the host already deleted reaches the network + /// (`dashpay/platform#4185`). + /// + /// Taking the gate *inside* this method rather than leaving it to the caller + /// is deliberate: `PlatformWalletManager` is public and `SignedPaymentRegistry` + /// is re-exported, so a direct Rust embedder that never goes through the FFI + /// would otherwise remove wallets with no exclusion at all, and could + /// interleave between a payment operation's liveness check and its register + /// or network action. `tear_down` is the hook that lets the FFI layer sweep + /// its own process-global handle storages inside the same critical section + /// without the gate ever being optional. + /// + /// `tear_down` is synchronous by design — it runs while the gate is held, and + /// every sweep it needs (`remove_entries_for_wallet`, + /// `HandleStorage::remove_matching`) is a synchronous map retain. + /// + /// ## Lock ordering + /// + /// The generation gate is always taken BEFORE the manager locks. The lookup + /// that finds the gate takes `wallets` briefly and **drops it before** + /// awaiting the gate, so no manager lock is ever held across a gate + /// acquisition; payment operations likewise take the gate and only then await + /// the manager. The order is total, so the two cannot deadlock. + /// + /// ## Removal is by generation identity, not by key + /// + /// The gate excludes *payment operations on this generation*. It does not + /// exclude a fresh **registration** under the same `wallet_id`: + /// [`register_wallet`](Self::register_wallet) mints its own + /// [`WalletGeneration`] and takes no gate at all, by design — a create must + /// never queue behind an unrelated wallet's teardown. + /// + /// So once this method has removed generation G1 from the inner + /// `wallet_manager`, the id is free and a concurrent registration can publish + /// a *different* generation G2 into both maps before this method reaches its + /// own `self.wallets` removal — the two removals are separately locked, with + /// no happens-before edge between them and the registration. Removing by key + /// there would take G2 out of the public map (leaving it registered in the + /// inner manager, invisible and unremovable) and hand G2 to `tear_down`, + /// which would sweep G2's registry tokens and V2 handles while holding only + /// G1's gate — i.e. with G2's payment operations *not* excluded, which is the + /// exact property this gate exists to provide. + /// + /// The `Arc` validated under the gate is therefore retained, + /// and the public-map entry is removed only while it still names that same + /// generation. Both maps, the returned handle and the `tear_down` argument + /// are then all that one generation (`dashpay/platform#4185`). The one + /// remaining id-keyed step is the shielded coordinator detach below, which + /// has no generation concept at all; a generation that has just been + /// registered has not run `bind_shielded` yet, so it holds no coordinator + /// entry to detach. + /// + /// The inner-manager removal needs no such check: G1 can only leave + /// `wallet_manager` through this method (which requires G1's gate, held here) + /// or through a registration/load rollback for an insert that could not have + /// happened while G1 occupied the id — so while the gate is held and before + /// the removal below, the inner entry is still G1 by construction. + pub async fn remove_wallet_with_teardown( + &self, + wallet_id: &WalletId, + tear_down: F, + ) -> Result, PlatformWalletError> + where + F: FnOnce(&Arc), + { + // Find the generation registered under `wallet_id` and take ITS gate. + // Re-validated after acquisition because the wallet could have been + // removed and re-created under the same id while we waited: in that case + // we hold the OLD generation's gate, which excludes nothing relevant to + // the new one, so retry against the generation that is actually current. + // + // The validated handle is carried out of the loop: it is both what this + // call returns and tears down, and the identity every mutation below is + // matched against. + let (removed, _teardown) = loop { + let candidate = { + let wallets = self.wallets.read().await; + match wallets.get(wallet_id) { + None => { + return Err(PlatformWalletError::WalletNotFound(hex::encode(wallet_id))) + } + Some(wallet) => Arc::clone(wallet), + } + }; + let guard = candidate.generation().teardown_guard().await; + let still_current = { + let wallets = self.wallets.read().await; + wallets + .get(wallet_id) + .is_some_and(|wallet| Arc::ptr_eq(wallet.generation(), candidate.generation())) + }; + if still_current { + break (candidate, guard); + } + // Drop this generation's guard and re-resolve. + drop(guard); + }; + let generation = Arc::clone(removed.generation()); + let owned_identity_ids: Vec = { let mut wm = self.wallet_manager.write().await; let ids = match wm.get_wallet_info(wallet_id) { @@ -598,12 +745,41 @@ impl PlatformWalletManager

{ ids }; - let removed = { + // Test-only rendezvous: the window a concurrent same-id registration can + // publish a new generation into. See `REMOVE_WALLET_MIDPOINT_HOOK`. + #[cfg(test)] + { + let pending = REMOVE_WALLET_MIDPOINT_HOOK + .lock() + .expect("remove-wallet midpoint hook mutex") + .as_ref() + .map(|hook| hook(wallet_id)); + if let Some(rendezvous) = pending { + rendezvous.await; + } + } + + // Remove the public-map entry only while it still names the generation + // validated under the gate. A concurrent same-id registration could have + // published a NEW generation here in the window since the inner removal + // above freed the id (see the "Removal is by generation identity" note on + // this method); removing by key would evict that live wallet and hand it + // to `tear_down` under the wrong gate. + { let mut wallets = self.wallets.write().await; - wallets - .remove(wallet_id) - .ok_or_else(|| PlatformWalletError::WalletNotFound(hex::encode(wallet_id)))? - }; + let entry_is_ours = wallets + .get(wallet_id) + .is_some_and(|wallet| Arc::ptr_eq(wallet.generation(), &generation)); + if entry_is_ours { + wallets.remove(wallet_id); + } else { + tracing::warn!( + wallet_id = %hex::encode(wallet_id), + "remove_wallet: a new generation was registered under this id while the \ + previous one was being removed; leaving the new registration in place" + ); + } + } // Detach the wallet's shielded state from the network // coordinator. After the Phase-2b refactor the coordinator @@ -640,6 +816,12 @@ impl PlatformWalletManager

{ .await; } + // Still under the generation's teardown gate: any deferred state naming + // this generation is dropped in the same critical section as the removal + // itself, so no payment operation can observe the wallet as live and then + // act on it after this returns. + tear_down(&removed); + Ok(removed) } } @@ -858,3 +1040,192 @@ mod register_wallet_duplicate_tests { ); } } + +/// Removal versus a same-id re-registration that lands *during* the removal +/// (`dashpay/platform#4185` review). +/// +/// The invariant: `remove_wallet_with_teardown` removes, returns and tears down +/// exactly the wallet generation it validated under that generation's lifecycle +/// gate — never a different generation that appeared under the same +/// `wallet_id` while the removal was in progress. +#[cfg(test)] +mod remove_versus_recreate_tests { + use std::sync::atomic::{AtomicBool, Ordering}; + use std::sync::{Arc, Mutex}; + + use key_wallet::mnemonic::{Language, Mnemonic}; + use key_wallet::wallet::initialization::WalletAccountCreationOptions; + use key_wallet::Network; + + use super::REMOVE_WALLET_MIDPOINT_HOOK; + use crate::test_support::test_platform_wallet_manager; + use crate::wallet::core::WalletGeneration; + use crate::wallet::PlatformWallet; + + /// The mnemonic `test_platform_wallet_manager` builds its wallet from, so + /// re-registering from the same seed collides on the same network-scoped + /// `wallet_id` — which is the whole point of the scenario. + const TEST_MNEMONIC: &str = "abandon abandon abandon abandon abandon abandon \ + abandon abandon abandon abandon abandon about"; + + /// Clears [`REMOVE_WALLET_MIDPOINT_HOOK`] on drop, including on panic, so a + /// failing assertion can never leave the hook armed for another test in the + /// same binary. + struct MidpointHookGuard; + + impl Drop for MidpointHookGuard { + fn drop(&mut self) { + if let Ok(mut slot) = REMOVE_WALLET_MIDPOINT_HOOK.lock() { + *slot = None; + } + } + } + + /// Requirement: a wallet generation registered while a removal is in flight + /// must survive that removal — in BOTH maps — and the removal must return + /// and tear down the generation it actually validated. + /// + /// Deterministic by construction: the re-registration runs from a rendezvous + /// fired inside the removal, in the exact window between the inner-manager + /// removal and the public-map removal, so there is no completion order to + /// race and no sleep. The registration itself is the real + /// `create_wallet_from_seed_bytes` → `register_wallet` path, publishing into + /// the inner `WalletManager` and then `self.wallets` in the production + /// order. + /// + /// Why that window is reachable in production: the removal frees the id in + /// the inner manager and only then acquires `self.wallets` — two separately + /// locked stages with no happens-before edge to a concurrent registration, + /// which takes no lifecycle gate at all (it mints its own generation). A + /// remover descheduled in that gap resumes into a map that already names the + /// new generation. + /// + /// Before the fix the removal took the public-map entry by KEY: it evicted + /// the freshly registered generation — leaving it registered in the inner + /// manager but invisible and unremovable through `self.wallets` — returned + /// it to the caller, and handed it to `tear_down`, which sweeps that + /// generation's registry tokens and V2 finalized-transaction handles while + /// holding only the OLD generation's gate. The new generation's in-flight + /// payment operations were therefore not excluded, which is the one property + /// the gate exists to provide. + #[tokio::test] + async fn removal_leaves_a_generation_registered_during_it_intact() { + let (manager, wallet_id) = test_platform_wallet_manager().await; + let original = manager + .get_wallet(&wallet_id) + .await + .expect("fixture wallet is registered"); + + // Filled by the rendezvous with the generation the re-registration + // publishes, so the assertions can name it rather than infer it. + let recreated: Arc>>> = Arc::new(Mutex::new(None)); + + let _hook_guard = MidpointHookGuard; + { + let manager_for_hook = Arc::clone(&manager); + let recreated_slot = Arc::clone(&recreated); + // One-shot: the re-registration must not recurse into a later + // removal, and no other test in this binary may see the hook. + let fired = AtomicBool::new(false); + *REMOVE_WALLET_MIDPOINT_HOOK + .lock() + .expect("midpoint hook mutex") = Some(Box::new(move |id| { + let already_fired = fired.swap(true, Ordering::SeqCst); + let manager = Arc::clone(&manager_for_hook); + let recreated_slot = Arc::clone(&recreated_slot); + let id = *id; + Box::pin(async move { + if already_fired { + return; + } + let mnemonic = Mnemonic::from_phrase(TEST_MNEMONIC, Language::English) + .expect("valid test mnemonic"); + let seed_bytes = mnemonic.to_seed(""); + // The real registration path: inner `WalletManager` first, + // then `self.wallets`. `Some(0)` skips the SPV-tip lookup. + let wallet = manager + .create_wallet_from_seed_bytes( + Network::Testnet, + &seed_bytes, + WalletAccountCreationOptions::Default, + Some(0), + ) + .await + .expect( + "the id is free in the inner manager at this point, so a same-seed \ + re-registration must succeed", + ); + assert_eq!(wallet.wallet_id(), id, "the fixture seeds must collide"); + *recreated_slot.lock().expect("recreated slot") = Some(wallet); + }) + })); + } + + // Capture what teardown was actually handed. + let torn_down: Arc>>> = Arc::new(Mutex::new(None)); + let torn_down_slot = Arc::clone(&torn_down); + + let removed = manager + .remove_wallet_with_teardown(&wallet_id, move |wallet| { + *torn_down_slot.lock().expect("torn-down slot") = + Some(Arc::clone(wallet.generation())); + }) + .await + .expect("removal of the validated generation succeeds"); + + let recreated = recreated + .lock() + .expect("recreated slot") + .clone() + .expect("the rendezvous must have re-registered the wallet"); + assert!( + !Arc::ptr_eq(original.generation(), recreated.generation()), + "the fixture must produce two distinct generations under one wallet id" + ); + + // 1. The removal returns the generation it validated under the gate. + assert!( + Arc::ptr_eq(removed.generation(), original.generation()), + "remove_wallet_with_teardown returned a generation it never validated — it took the \ + public-map entry by key and got the generation registered during the removal" + ); + + // 2. …and tears down that same generation. Sweeping the other one here + // would run without holding ITS gate, so its in-flight payment + // operations would not be excluded. + let torn_down = torn_down + .lock() + .expect("torn-down slot") + .clone() + .expect("tear_down must have run"); + assert!( + Arc::ptr_eq(&torn_down, original.generation()), + "tear_down was handed a generation whose lifecycle gate this removal does not hold" + ); + + // 3. The generation registered during the removal is still published. + let still_registered = manager + .get_wallet(&wallet_id) + .await + .expect("a wallet registered during a removal must remain in the public map"); + assert!( + Arc::ptr_eq(still_registered.generation(), recreated.generation()), + "the public map must still name the generation the registration published" + ); + + // 4. …and both maps agree about it: `is_current_generation` compares the + // handle against the inner `WalletManager`, so this fails if the + // removal evicted it from one map only. + assert!( + recreated.core().is_current_generation().await, + "the re-registered generation must be live in both the inner manager and the public \ + map — evicting it from one leaves an invisible, unremovable wallet" + ); + + // 5. The removed generation is gone. + assert!( + !original.core().is_current_generation().await, + "the validated generation must be gone from the inner manager" + ); + } +} diff --git a/packages/rs-platform-wallet/src/test_support.rs b/packages/rs-platform-wallet/src/test_support.rs index 8fd146dc77..45afab3696 100644 --- a/packages/rs-platform-wallet/src/test_support.rs +++ b/packages/rs-platform-wallet/src/test_support.rs @@ -18,6 +18,11 @@ use dashcore::Txid; use dashcore::{Network, Transaction}; use key_wallet::account::account_type::StandardAccountType; use key_wallet::bip32::ExtendedPubKey; +// Only the `#[cfg(test)]` CoinJoin fixture needs the trait (for +// `next_address_with_info` on a non-standard account); gate it to match so a +// `test-utils`-only build does not flag it unused. +#[cfg(test)] +use key_wallet::managed_account::managed_account_trait::ManagedAccountTrait; use key_wallet::signer::{ExtendedPubKeySigner, Signer, SignerMethod}; use key_wallet::test_utils::TestWalletContext; use key_wallet::transaction_checking::{BlockInfo, TransactionContext}; @@ -27,7 +32,7 @@ use tokio::sync::RwLock; #[cfg(test)] use crate::broadcaster::{BroadcastError, TransactionBroadcaster}; -use crate::wallet::core::WalletBalance; +use crate::wallet::core::WalletGeneration; use crate::wallet::identity::IdentityManager; use crate::wallet::platform_wallet::{PlatformWalletInfo, WalletId}; @@ -178,7 +183,7 @@ pub(crate) async fn funded_wallet_manager( ) -> ( Arc>>, WalletId, - Arc, + Arc, WalletSigner, ) { funded_wallet_manager_with_outputs(account_type, &[10_000_000]).await @@ -193,7 +198,7 @@ pub(crate) async fn funded_wallet_manager_with_outputs( ) -> ( Arc>>, WalletId, - Arc, + Arc, WalletSigner, ) { let mut ctx = TestWalletContext::new_random(); @@ -243,10 +248,10 @@ pub(crate) async fn funded_wallet_manager_with_outputs( wallet: ctx.wallet.clone(), }; - let balance = Arc::new(WalletBalance::new()); + let generation = Arc::new(WalletGeneration::new()); let info = PlatformWalletInfo { core_wallet: ctx.managed_wallet, - balance: Arc::clone(&balance), + generation: Arc::clone(&generation), identity_manager: IdentityManager::new(), tracked_asset_locks: BTreeMap::new(), }; @@ -254,7 +259,78 @@ pub(crate) async fn funded_wallet_manager_with_outputs( let mut wm = WalletManager::::new(Network::Testnet); let wallet_id = wm.insert_wallet(ctx.wallet, info).expect("insert wallet"); - (Arc::new(RwLock::new(wm)), wallet_id, balance, signer) + (Arc::new(RwLock::new(wm)), wallet_id, generation, signer) +} + +/// Like [`funded_wallet_manager`] but funds the wallet's CoinJoin account 0 +/// (created by `WalletAccountCreationOptions::Default`) with a single spendable +/// UTXO. Lets the deferred-payment tests exercise a CoinJoin-funded reservation, +/// which has no `StandardAccountType` yet must still be released immediately on +/// rejection/abandon rather than stranded until the TTL backstop. +/// +/// Only the crate's own `#[cfg(test)]` unit tests consume it, so it is gated on +/// `cfg(test)` directly — under the `test-utils` feature alone (the FFI crate's +/// build) it would compile with no user and trip `dead_code`. +#[cfg(test)] +pub(crate) async fn funded_coinjoin_wallet_manager() -> ( + Arc>>, + WalletId, + Arc, + WalletSigner, +) { + let mut ctx = TestWalletContext::new_random(); + + let coinjoin_xpub = ctx + .wallet + .accounts + .coinjoin_accounts + .get(&0) + .expect("default wallet has CoinJoin account 0") + .account_xpub; + // CoinJoin is a non-standard account type: its addresses come from the + // single external pool via `next_address_with_info`, not the standard + // receive/change split that `next_receive_address` serves. + let receive_address = ctx + .managed_wallet + .first_coinjoin_managed_account_mut() + .expect("coinjoin managed account") + .next_address_with_info(Some(&coinjoin_xpub), true) + .expect("coinjoin receive address") + .address; + + let funding_tx = Transaction::dummy(&receive_address, 0..1, &[10_000_000]); + let result = ctx + .check_transaction( + &funding_tx, + TransactionContext::InChainLockedBlock(BlockInfo::new( + 1, + BlockHash::all_zeros(), + 1_700_000_000, + )), + ) + .await; + assert!( + result.is_relevant, + "funding tx should be relevant to the CoinJoin account" + ); + assert!(result.is_new_transaction); + + let signer = WalletSigner { + wallet: ctx.wallet.clone(), + }; + + let generation = Arc::new(WalletGeneration::new()); + let info = PlatformWalletInfo { + core_wallet: ctx.managed_wallet, + generation: Arc::clone(&generation), + identity_manager: IdentityManager::new(), + tracked_asset_locks: BTreeMap::new(), + }; + + let mut wm = WalletManager::::new(Network::Testnet); + let wallet_id = wm.insert_wallet(ctx.wallet, info).expect("insert wallet"); + + (Arc::new(RwLock::new(wm)), wallet_id, generation, signer) } /// Funded SPV-backed Core wallet for downstream FFI lifecycle tests. The SPV @@ -265,7 +341,7 @@ pub async fn funded_spv_core_wallet( crate::CoreWallet, WalletSigner, ) { - let (manager, wallet_id, balance, signer) = funded_wallet_manager(account_type).await; + let (manager, wallet_id, generation, signer) = funded_wallet_manager(account_type).await; let spv = Arc::new(crate::spv::SpvRuntime::new( Arc::clone(&manager), Arc::new(crate::events::PlatformEventManager::new(Vec::new())), @@ -273,7 +349,114 @@ pub async fn funded_spv_core_wallet( let broadcaster = Arc::new(crate::broadcaster::SpvBroadcaster::new(spv)); let sdk = Arc::new(dash_sdk::SdkBuilder::new_mock().build().expect("mock sdk")); ( - crate::CoreWallet::new(sdk, manager, wallet_id, broadcaster, balance), + crate::CoreWallet::new(sdk, manager, wallet_id, broadcaster, generation), signer, ) } + +/// Advance `core`'s `last_processed_height` to just past the reservation age +/// guard bound ([`RESERVATION_MAX_AGE_BLOCKS`](crate::wallet::reservations::RESERVATION_MAX_AGE_BLOCKS)) +/// but below key-wallet's `ReservationSet` TTL, so a handle finalized at the +/// current height ages enough to trip the software guard while its underlying +/// reservation is provably still held (no key-wallet sweep yet). Returns the new +/// height. +/// +/// FFI lifecycle tests use this to exercise the aged abandon/free skip-release +/// path — the deinit/GC backstop and the broadcast/abandon failure paths that +/// route their cleanup through `abandon_transaction`. +pub async fn age_core_past_reservation_guard(core: &crate::CoreWallet) -> u32 +where + B: crate::broadcaster::TransactionBroadcaster + ?Sized, +{ + use key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; + + let stamped = core + .last_processed_height() + .await + .expect("wallet present in manager"); + let target = stamped + crate::wallet::reservations::RESERVATION_MAX_AGE_BLOCKS + 2; + { + 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_last_processed_height(target); + } + target +} + +/// 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` regression asserting that destroying wrapper +/// aliases never sweeps an independently-owned deferred-payment token. +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) +} diff --git a/packages/rs-platform-wallet/src/wallet/apply.rs b/packages/rs-platform-wallet/src/wallet/apply.rs index bbe87893d6..993ccff413 100644 --- a/packages/rs-platform-wallet/src/wallet/apply.rs +++ b/packages/rs-platform-wallet/src/wallet/apply.rs @@ -359,7 +359,7 @@ impl PlatformWalletInfo { // Mirror the recomputed balance into the lock-free Arc that the // UI reads. let core_balance = &self.core_wallet.balance; - self.balance.set( + self.generation.set( core_balance.confirmed(), core_balance.unconfirmed(), core_balance.immature(), @@ -389,7 +389,7 @@ mod tests { ReceivedContactRequestKey, SentContactRequestKey, TokenBalanceChangeSet, }; use crate::wallet::asset_lock::tracked::AssetLockStatus; - use crate::wallet::core::WalletBalance; + use crate::wallet::core::WalletGeneration; use crate::wallet::identity::state::managed_identity::ManagedIdentity; use crate::wallet::identity::IdentityManager; use crate::wallet::identity::{ContactRequest, EstablishedContact}; @@ -410,7 +410,7 @@ mod tests { fn empty_info(wallet: &Wallet) -> PlatformWalletInfo { PlatformWalletInfo { core_wallet: ManagedWalletInfo::from_wallet(wallet, 0), - balance: std::sync::Arc::new(WalletBalance::new()), + generation: std::sync::Arc::new(WalletGeneration::new()), identity_manager: IdentityManager::new(), tracked_asset_locks: BTreeMap::new(), } diff --git a/packages/rs-platform-wallet/src/wallet/asset_lock/sync/recovery.rs b/packages/rs-platform-wallet/src/wallet/asset_lock/sync/recovery.rs index 536f669a86..6083f29822 100644 --- a/packages/rs-platform-wallet/src/wallet/asset_lock/sync/recovery.rs +++ b/packages/rs-platform-wallet/src/wallet/asset_lock/sync/recovery.rs @@ -467,7 +467,7 @@ mod tests { use crate::test_support::{funded_wallet_manager, AlwaysRejectedBroadcaster}; use crate::wallet::asset_lock::manager::AssetLockManager; use crate::wallet::asset_lock::tracked::{AssetLockStatus, TrackedAssetLock}; - use crate::wallet::core::WalletBalance; + use crate::wallet::core::WalletGeneration; use crate::wallet::identity::IdentityManager; use crate::wallet::persister::WalletPersister; use crate::wallet::platform_wallet::PlatformWalletInfo; @@ -740,7 +740,7 @@ mod tests { let restored_wallet = Wallet::new_external_signable(Network::Testnet, wallet_id, accounts); let mut restored_info = PlatformWalletInfo { core_wallet: ManagedWalletInfo::from_wallet(&restored_wallet, 0), - balance: Arc::new(WalletBalance::new()), + generation: Arc::new(WalletGeneration::new()), identity_manager: IdentityManager::new(), tracked_asset_locks: BTreeMap::new(), }; diff --git a/packages/rs-platform-wallet/src/wallet/core/broadcast.rs b/packages/rs-platform-wallet/src/wallet/core/broadcast.rs index 0f3d7fd1f0..233a4e4b2e 100644 --- a/packages/rs-platform-wallet/src/wallet/core/broadcast.rs +++ b/packages/rs-platform-wallet/src/wallet/core/broadcast.rs @@ -1,18 +1,58 @@ use dashcore::Transaction; use key_wallet::account::account_type::StandardAccountType; +use key_wallet::wallet::managed_wallet_info::transaction_building::AccountTypePreference; +use key_wallet::ReservationToken; use super::SignedCoreTransaction; -use crate::broadcaster::TransactionBroadcaster; -use crate::wallet::reservations::broadcast_releasing_on_rejection; +use crate::broadcaster::{BroadcastError, TransactionBroadcaster}; +use crate::wallet::reservations::{broadcast_releasing_on_rejection, reservation_expired}; use crate::{CoreWallet, PlatformWalletError}; impl CoreWallet { /// Broadcast an atomically finalized transaction. A definitive rejection /// releases its reservation; an ambiguous `MaybeSent` outcome retains it. + /// + /// The release is owner-guarded by the finalized transaction's + /// [`reservation_token`](SignedCoreTransaction::reservation_token): the + /// broadcast is `.await`ed, and during that await key-wallet's TTL sweep can + /// reclaim this build's reservation and a concurrent build re-reserve the + /// same inputs under a new token. Releasing by outpoint alone would then + /// free that other build's inputs (the `dashpay/platform#4185` double-spend + /// window); presenting the token frees only inputs this build still owns. + /// + /// # Reservation age guard + /// + /// A V2 finalized-transaction handle can be pinned by the host for an + /// arbitrary time between `finalize` and this broadcast. If the wallet's + /// `last_processed_height` advances at least + /// [`RESERVATION_MAX_AGE_BLOCKS`](crate::wallet::reservations::RESERVATION_MAX_AGE_BLOCKS) + /// blocks past the height the funding reservation was stamped at + /// ([`SignedCoreTransaction::reservation_height`]), key-wallet's own + /// `ReservationSet` TTL could already have swept those inputs and let an + /// unrelated build re-select them. Broadcasting then would spend against a + /// newer, unrelated reservation, so the send is refused with + /// [`PlatformWalletError::StaleReservation`] **before** the broadcaster is + /// touched — mirroring the deferred registry token's + /// [`broadcast`](crate::SignedPaymentRegistry::broadcast) guard, off the + /// same bound and the same `last_processed_height` clock, and running after + /// the FFI layer's generation-identity check just as the registry does. + /// The stale reservation is deliberately left for key-wallet's TTL to + /// reclaim rather than released by outpoint here (which could free a newer + /// build's reservation); the caller must rebuild the payment. Abandon/free + /// ([`abandon_transaction`](Self::abandon_transaction)) honor the same bound: + /// once aged they too skip the by-outpoint release and leave the outpoint for + /// the TTL, because releasing a swept-and-re-reserved outpoint could free a + /// newer build's reservation — only below the bound do they release. pub async fn broadcast_finalized_transaction( &self, transaction: &SignedCoreTransaction, ) -> Result { + if reservation_expired( + transaction.reservation_height(), + self.last_processed_height().await, + ) { + return Err(PlatformWalletError::StaleReservation); + } match self.broadcaster.broadcast(transaction.transaction()).await { Ok(txid) => Ok(txid), Err(error) => { @@ -21,6 +61,7 @@ impl CoreWallet { transaction.funding_account_type(), transaction.funding_account_index(), transaction.transaction(), + transaction.reservation_token(), ) .await; } @@ -86,6 +127,61 @@ impl CoreWallet { .await .map_err(Into::into) } + + /// Broadcast a raw signed `transaction` for the deferred-payment + /// [`SignedPaymentRegistry`](crate::SignedPaymentRegistry), reconciling the + /// funding reservation on failure. + /// + /// Same policy as + /// [`broadcast_finalized_transaction`](Self::broadcast_finalized_transaction): + /// a definitive [`BroadcastError::Rejected`] releases the reservation for an + /// immediate rebuild; an ambiguous `MaybeSent` keeps it. Unlike the + /// `StandardAccountType`-typed + /// [`broadcast_transaction_releasing_reservation`](Self::broadcast_transaction_releasing_reservation) + /// used by the immediate send path, this takes an [`AccountTypePreference`] + /// so it ALSO reconciles a CoinJoin-funded deferred payment — one whose + /// `build_signed`/`finalize` reserved the selected inputs but which has no + /// `StandardAccountType`, and which previously kept its reservation held + /// until the TTL backstop. + /// + /// The release delegates to + /// [`release_transaction_reservation`](Self::release_transaction_reservation), + /// so it acts only on the wallet *generation* this handle names (a wallet + /// re-created under the same id between build and broadcast cannot have its + /// reservation freed by this token) AND — via `token` — only on inputs this + /// build still owns. The deferred registry can hold the reservation across a + /// long build→broadcast gap, so a TTL sweep re-reserving the same inputs + /// under a new token is a real risk; the owner guard closes the + /// `dashpay/platform#4185` release/re-reserve race. + /// + /// `account_type`/`account_index` identify the funding account handed to the + /// builder when the transaction was finalized; `token` is the + /// [`ReservationToken`] that build stamped + /// (`SignedCoreTransaction::reservation_token`), `None` only when the build + /// reserved nothing. + pub(crate) async fn broadcast_payment_releasing_reservation( + &self, + account_type: AccountTypePreference, + account_index: u32, + transaction: &Transaction, + token: Option, + ) -> Result { + match self.broadcaster.broadcast(transaction).await { + Ok(txid) => Ok(txid), + Err(error) => { + if matches!(error, BroadcastError::Rejected { .. }) { + self.release_transaction_reservation( + account_type, + account_index, + transaction, + token, + ) + .await; + } + Err(error.into()) + } + } + } } #[cfg(test)] @@ -98,14 +194,17 @@ mod tests { 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::transaction_building::AccountTypePreference; use key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; use crate::broadcaster::TransactionBroadcaster; use crate::test_support::{ - funded_wallet_manager, AlwaysMaybeSentBroadcaster, RejectFirstBroadcaster, WalletSigner, + funded_wallet_manager, AlwaysMaybeSentBroadcaster, AlwaysOkBroadcaster, + RejectFirstBroadcaster, WalletSigner, }; use crate::wallet::core::CoreWallet; - use crate::PlatformWalletError; + use crate::wallet::reservations::RESERVATION_MAX_AGE_BLOCKS; + use crate::{PlatformWalletError, SignedCoreTransaction}; /// Builds a testnet `CoreWallet` over the shared funded fixture and a /// 1_000_000-duff payment to a dummy recipient. @@ -187,6 +286,218 @@ mod tests { Ok(tx) } + /// Atomically fund + reserve + sign a `SignedCoreTransaction` the way the V2 + /// handle path (`core_wallet_tx_builder_finalize`) does, capturing the + /// reservation's stamp height on the returned handle. + async fn finalize_tx( + core: &CoreWallet, + account_type: AccountTypePreference, + outputs: &[(DashAddress, u64)], + signer: &WalletSigner, + ) -> SignedCoreTransaction { + let mut builder = TransactionBuilder::new(); + for (addr, amount) in outputs { + builder = builder.add_output(addr, *amount); + } + try_finalize_tx(core, account_type, outputs, signer) + .await + .expect("finalize should succeed") + } + + /// Like [`finalize_tx`] but surfaces the build error instead of panicking — + /// used to prove a *rebuild* fails when a still-held reservation keeps its + /// inputs out of the selectable pool. + async fn try_finalize_tx( + core: &CoreWallet, + account_type: AccountTypePreference, + outputs: &[(DashAddress, u64)], + signer: &WalletSigner, + ) -> Result { + let mut builder = TransactionBuilder::new(); + for (addr, amount) in outputs { + builder = builder.add_output(addr, *amount); + } + core.finalize_transaction(builder, account_type, 0, signer) + .await + } + + /// Force the wallet's `last_processed_height` forward, simulating chain + /// progress between `finalize` and a later broadcast of the pinned V2 + /// handle — the window in which key-wallet's `ReservationSet` TTL can sweep + /// the funding reservation. Same clock the 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_last_processed_height(height); + } + + /// A freshly finalized V2 handle — no chain progress since `finalize` — + /// broadcasts normally: the age guard does not trip. + #[tokio::test] + async fn fresh_finalized_handle_broadcasts() { + for account_type in [AccountTypePreference::BIP44, AccountTypePreference::BIP32] { + let (core, signer, outputs) = funded_core_wallet( + account_type_standard(account_type), + Arc::new(AlwaysOkBroadcaster), + ) + .await; + let finalized = finalize_tx(&core, account_type, &outputs, &signer).await; + let sent = core.broadcast_finalized_transaction(&finalized).await; + assert!( + sent.is_ok(), + "a fresh handle must broadcast for {account_type:?}, got {sent:?}" + ); + } + } + + /// A V2 handle pinned while the wallet syncs past `RESERVATION_MAX_AGE_BLOCKS` + /// beyond its reservation stamp must be refused with `StaleReservation` + /// (never a send — the broadcaster is `AlwaysOk`, so a leaked send would + /// surface as `Ok`). An aged abandon/free must then **skip** the by-outpoint + /// release: key-wallet's TTL may already have swept and re-reserved the + /// outpoint, so releasing it could free a newer build's reservation. We prove + /// the skip by showing the input is still reserved after abandon — an + /// immediate rebuild cannot reselect it. + #[tokio::test] + async fn aged_finalized_handle_refuses_broadcast_and_skips_release() { + for account_type in [AccountTypePreference::BIP44, AccountTypePreference::BIP32] { + let (core, signer, outputs) = funded_core_wallet( + account_type_standard(account_type), + Arc::new(AlwaysOkBroadcaster), + ) + .await; + let stamped = core + .last_processed_height() + .await + .expect("last processed height"); + let finalized = finalize_tx(&core, account_type, &outputs, &signer).await; + + // Advance past the guard bound (stay below key-wallet's 24-block TTL, + // so the reservation is provably still held — only our guard tripped). + advance_processed_height(&core, stamped + RESERVATION_MAX_AGE_BLOCKS + 2).await; + + let sent = core.broadcast_finalized_transaction(&finalized).await; + assert!( + matches!(sent, Err(PlatformWalletError::StaleReservation)), + "an aged handle must refuse with StaleReservation for \ + {account_type:?}, got {sent:?}" + ); + + // Aged abandon skips the by-outpoint release. The reservation is left + // for key-wallet's TTL, so the input stays reserved and a rebuild + // cannot reselect it (it would surface as a build/insufficient-funds + // error). + core.abandon_transaction(&finalized).await; + let rebuilt = try_finalize_tx(&core, account_type, &outputs, &signer).await; + assert!( + rebuilt.is_err(), + "aged abandon must skip the release, leaving the input reserved \ + for {account_type:?}, got {rebuilt:?}" + ); + } + } + + /// Below the guard bound the reservation is provably still ours (no sweep + /// possible yet), so abandon/free **do** release by outpoint — returning the + /// inputs so an immediate rebuild reselects them. This is the mirror of the + /// aged skip case. + #[tokio::test] + async fn below_bound_finalized_handle_abandon_releases() { + for account_type in [AccountTypePreference::BIP44, AccountTypePreference::BIP32] { + let (core, signer, outputs) = funded_core_wallet( + account_type_standard(account_type), + Arc::new(AlwaysOkBroadcaster), + ) + .await; + let stamped = core + .last_processed_height() + .await + .expect("last processed height"); + let finalized = finalize_tx(&core, account_type, &outputs, &signer).await; + + // Aged, but one shy of the guard bound: still below both the guard and + // the TTL, so the reservation is unambiguously ours to release. + advance_processed_height(&core, stamped + RESERVATION_MAX_AGE_BLOCKS - 1).await; + + core.abandon_transaction(&finalized).await; + + // The release freed the input: an immediate rebuild reselects it. + let rebuilt = try_finalize_tx(&core, account_type, &outputs, &signer).await; + assert!( + rebuilt.is_ok(), + "below-bound abandon must release the input so a rebuild reselects \ + it for {account_type:?}, got {rebuilt:?}" + ); + core.abandon_transaction(&rebuilt.expect("rebuild")).await; + } + } + + /// The guard boundary is exact: `current - stamped >= RESERVATION_MAX_AGE_BLOCKS` + /// refuses, one block below still broadcasts — for both standard account + /// types, like the fresh/aged tests. + #[tokio::test] + async fn finalized_handle_age_guard_boundary_is_exact() { + for account_type in [AccountTypePreference::BIP44, AccountTypePreference::BIP32] { + // One below the bound: still fresh enough to broadcast. + let (below_core, below_signer, below_outputs) = funded_core_wallet( + account_type_standard(account_type), + Arc::new(AlwaysOkBroadcaster), + ) + .await; + let below_stamped = below_core + .last_processed_height() + .await + .expect("last processed height"); + let below = finalize_tx(&below_core, account_type, &below_outputs, &below_signer).await; + advance_processed_height(&below_core, below_stamped + RESERVATION_MAX_AGE_BLOCKS - 1) + .await; + assert!( + below_core + .broadcast_finalized_transaction(&below) + .await + .is_ok(), + "one block below the bound must still broadcast ({account_type:?})" + ); + + // Exactly at the bound: refused. + let (at_core, at_signer, at_outputs) = funded_core_wallet( + account_type_standard(account_type), + Arc::new(AlwaysOkBroadcaster), + ) + .await; + let at_stamped = at_core + .last_processed_height() + .await + .expect("last processed height"); + let at = finalize_tx(&at_core, account_type, &at_outputs, &at_signer).await; + advance_processed_height(&at_core, at_stamped + RESERVATION_MAX_AGE_BLOCKS).await; + assert!( + matches!( + at_core.broadcast_finalized_transaction(&at).await, + Err(PlatformWalletError::StaleReservation) + ), + "exactly at the bound must refuse with StaleReservation ({account_type:?})" + ); + } + } + + /// Map a builder `AccountTypePreference` (BIP44/BIP32 only in these tests) + /// to the `StandardAccountType` the funded fixture is keyed by. + fn account_type_standard(account_type: AccountTypePreference) -> StandardAccountType { + match account_type { + AccountTypePreference::BIP44 => StandardAccountType::BIP44Account, + AccountTypePreference::BIP32 => StandardAccountType::BIP32Account, + AccountTypePreference::CoinJoin => { + unreachable!("coinjoin funding not exercised by these tests") + } + } + } + /// A pre-send broadcast rejection must release the UTXO reservation taken /// while building the transaction, so an immediate retry can reselect those /// inputs instead of failing with spurious insufficient funds until the TTL diff --git a/packages/rs-platform-wallet/src/wallet/core/generation.rs b/packages/rs-platform-wallet/src/wallet/core/generation.rs new file mode 100644 index 0000000000..5d70488443 --- /dev/null +++ b/packages/rs-platform-wallet/src/wallet/core/generation.rs @@ -0,0 +1,138 @@ +//! Per-wallet-*generation* shared state: the identity marker every handle to +//! one generation shares, and that generation's lifecycle gate. + +use std::ops::Deref; +use std::sync::Arc; + +use tokio::sync::{OwnedRwLockWriteGuard, RwLock, RwLockReadGuard}; + +use super::balance::WalletBalance; + +/// The state one wallet *generation* shares across every handle that names it. +/// +/// A "generation" is one live in-memory instance of a logical wallet. Removing a +/// wallet and re-creating it under the same `wallet_id` produces a *different* +/// generation: same id, same shared multi-wallet `WalletManager` `Arc`, fresh +/// `WalletGeneration`. `PlatformWalletManager` builds exactly one of these per +/// registration and clones the `Arc` into `PlatformWalletInfo`, `PlatformWallet` +/// and `CoreWallet`, so `Arc::ptr_eq` on it is the canonical generation identity +/// (see [`CoreWallet::is_same_generation`](super::CoreWallet::is_same_generation)). +/// +/// # Why the balance and the lifecycle gate live in the *same* object +/// +/// They are one indivisible fact — "which generation is this?" — and splitting +/// them into two `Arc`s threaded separately through ~15 construction sites would +/// let a future site clone the identity marker but mint a *fresh* gate. Two +/// handles would then compare as the same generation while excluding each other +/// through different locks: teardown would take one gate, an in-flight payment +/// would hold the other, and the exclusion would silently vanish with nothing to +/// fail. Keeping them in one `Arc` makes that divergence unrepresentable — +/// same generation is the same gate, by construction (`dashpay/platform#4185`). +/// +/// [`Deref`] to [`WalletBalance`] keeps every existing lock-free balance read +/// (`generation.confirmed()`, `info.balance.locked()`, …) working unchanged. +#[derive(Debug)] +pub struct WalletGeneration { + /// Lock-free balance for UI reads. Updated from `ManagedWalletInfo` after + /// each SPV block/mempool processing and RPC refresh. + balance: WalletBalance, + /// This generation's lifecycle gate — held across whole *operations* rather + /// than around individual state mutations. + /// + /// Shared side ([`payment_guard`](Self::payment_guard)): any operation that + /// will publish an ownership handle for this generation, or push one of its + /// transactions to the network, after observing that the generation is still + /// live. Exclusive side ([`teardown_guard`](Self::teardown_guard)): removing + /// the generation from the manager and sweeping its deferred state. + /// + /// This is deliberately **per generation** rather than one process-global + /// lock. A deferred broadcast holds the shared side across an SPV send + /// (seconds, up to the broadcaster's timeout); with a single global lock that + /// send would block teardown — and, because tokio's `RwLock` is + /// write-preferring, every subsequent payment operation — for *every + /// unrelated wallet* in the process. Scoped here, one wallet's slow send + /// only ever excludes that same wallet's teardown, which is exactly the pair + /// that must not interleave. + /// + /// Held in its own `Arc` so [`teardown_guard`](Self::teardown_guard) can hand + /// back an *owned* guard: the remover resolves which generation is current in + /// a retry loop, and the guard must outlive the loop iteration that produced + /// the `Arc` it came from. + lifecycle: Arc>, +} + +impl Default for WalletGeneration { + fn default() -> Self { + Self::new() + } +} + +impl WalletGeneration { + /// A fresh generation: zeroed balance, uncontended gate. + pub fn new() -> Self { + Self { + balance: WalletBalance::new(), + lifecycle: Arc::new(RwLock::new(())), + } + } + + /// This generation's lock-free balance. + pub fn balance(&self) -> &WalletBalance { + &self.balance + } + + /// Enter this generation's lifecycle gate as a *payment* operation. + /// + /// Shared: any number of payment operations on this generation (and every + /// operation on every *other* generation) run concurrently. What it excludes + /// is this generation's own teardown ([`teardown_guard`](Self::teardown_guard)), + /// which is what makes a liveness observation + /// ([`CoreWallet::is_current_generation`](super::CoreWallet::is_current_generation)) + /// safe to act on: held across both the check and the action it gates, a + /// removal cannot interleave between them. + /// + /// Callers must hold it across the check *and* the publication/network step, + /// and must not already hold it (the `RwLock` is not reentrant, and because + /// tokio's is write-preferring a queued teardown would deadlock the + /// re-entry). + /// + /// # Lock ordering + /// + /// Always taken BEFORE the wallet-manager `RwLock`, never while holding it. + /// Teardown takes it and then awaits the manager write lock; payment + /// operations take it and then await the manager read lock. The order is + /// total, so the two locks cannot deadlock. + pub async fn payment_guard(&self) -> RwLockReadGuard<'_, ()> { + self.lifecycle.read().await + } + + /// Enter this generation's lifecycle gate as a *teardown*. + /// + /// Exclusive against every payment operation on this generation. Removal + /// holds it across BOTH the manager removal and the deferred-state sweep, so + /// the two are one linearization point rather than two steps with a window + /// between them that a retained handle could broadcast through + /// (`dashpay/platform#4185`). + /// + /// Acquiring it waits for payment operations that have already entered their + /// liveness-check/publish section. It does **not** wait for an operation + /// still awaiting an external signer: those acquire the gate only *after* + /// the signature returns, precisely so an open signing prompt cannot stall + /// teardown. Such a late finalizer then observes the removed generation at + /// its liveness check and abandons instead of publishing. + /// + /// Returns an *owned* guard so it can outlive the `Arc` + /// binding it was taken from — the remover resolves the current generation in + /// a retry loop and must carry the guard out of the iteration that found it. + pub async fn teardown_guard(&self) -> OwnedRwLockWriteGuard<()> { + Arc::clone(&self.lifecycle).write_owned().await + } +} + +impl Deref for WalletGeneration { + type Target = WalletBalance; + + fn deref(&self) -> &WalletBalance { + &self.balance + } +} diff --git a/packages/rs-platform-wallet/src/wallet/core/mod.rs b/packages/rs-platform-wallet/src/wallet/core/mod.rs index 5481362ae8..ba84b77f21 100644 --- a/packages/rs-platform-wallet/src/wallet/core/mod.rs +++ b/packages/rs-platform-wallet/src/wallet/core/mod.rs @@ -1,10 +1,12 @@ pub mod balance; pub mod balance_handler; mod broadcast; +pub mod generation; mod transaction; pub mod wallet; pub use balance::WalletBalance; pub use balance_handler::BalanceUpdateHandler; +pub use generation::WalletGeneration; pub use transaction::SignedCoreTransaction; pub use wallet::CoreWallet; diff --git a/packages/rs-platform-wallet/src/wallet/core/transaction.rs b/packages/rs-platform-wallet/src/wallet/core/transaction.rs index 81b2e8a824..c50f015bb2 100644 --- a/packages/rs-platform-wallet/src/wallet/core/transaction.rs +++ b/packages/rs-platform-wallet/src/wallet/core/transaction.rs @@ -6,6 +6,7 @@ //! resolver without pinning wallet state. use std::collections::HashMap; +use std::sync::Arc; use dashcore::{Address, Transaction}; use key_wallet::managed_account::managed_account_trait::ManagedAccountTrait; @@ -16,10 +17,11 @@ use key_wallet::wallet::managed_wallet_info::transaction_builder::{ }; use key_wallet::wallet::managed_wallet_info::transaction_building::AccountTypePreference; use key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; -use key_wallet::{Account, DerivationPath, Utxo}; +use key_wallet::{Account, DerivationPath, ReservationToken, Utxo}; -use super::CoreWallet; +use super::{CoreWallet, WalletGeneration}; use crate::broadcaster::TransactionBroadcaster; +use crate::wallet::reservations::reservation_expired; use crate::PlatformWalletError; fn map_builder_error( @@ -59,6 +61,42 @@ pub struct SignedCoreTransaction { fee: u64, funding_account_type: AccountTypePreference, funding_account_index: u32, + /// The wallet's `last_processed_height` captured **inside** the funding + /// critical section — the exact clock `set_current_height` stamped the + /// selected inputs' reservation with, sampled *before* the (potentially + /// slow, external) signer ran. The deferred-payment registry's age guard + /// must baseline off this, not off a fresh `last_processed_height` sampled + /// after signing: a slow external signer could otherwise let the wallet + /// advance far enough that the token looks fresh while the reservation it + /// covers has already aged toward key-wallet's TTL sweep. + reservation_height: u32, + /// The key-wallet [`ReservationToken`] stamped onto the selected inputs when + /// `build_unsigned_reserved` reserved them, or `None` when the build took no + /// reservation (no reservation set attached — not reached on the funded + /// finalize path). Held so an abandoned or definitively-rejected send + /// releases the reservation *owner-guarded*: after this build's inputs may + /// have been swept by key-wallet's TTL and re-reserved by a concurrent build + /// under a new token, releasing by outpoint alone would free that other + /// build's inputs (the `dashpay/platform#4185` double-spend window). + /// [`ManagedCoreFundsAccount::release_reservation_if_owner`] releases only + /// inputs still owned by this token, closing that window. + reservation_token: Option, + /// The per-generation balance `Arc` of the wallet this payment was + /// **finalized against** — captured from the originating `CoreWallet` inside + /// `finalize_transaction`. It is the same unforgeable generation-identity + /// marker [`CoreWallet::is_same_generation`] compares (a fresh `Arc` per + /// wallet generation; two aliases of one generation share it, a + /// remove-then-recreate under the same id gets a new one). + /// + /// The deferred-payment registry validates the wallet it is asked to bind + /// this payment to against **this** marker before it mints a token + /// ([`SignedPaymentRegistry::register`](crate::SignedPaymentRegistry::register)), + /// so a caller cannot finalize through wallet A and then register/broadcast + /// through an unrelated wallet B — the registry would otherwise treat B as + /// the owner, submit A's transaction through B's broadcaster, and run B's + /// cleanup while A's real reservation leaked until its TTL + /// (`dashpay/platform#4185`). + origin_generation: Arc, } impl SignedCoreTransaction { @@ -77,6 +115,97 @@ impl SignedCoreTransaction { pub fn funding_account_index(&self) -> u32 { self.funding_account_index } + + /// The `last_processed_height` the funding reservation was stamped with, + /// captured in the funding critical section before signing. The deferred + /// registry registers the token with this height so its age guard measures + /// the reservation's true age rather than a post-signing sample. + pub fn reservation_height(&self) -> u32 { + self.reservation_height + } + + /// The key-wallet [`ReservationToken`] the funding inputs were reserved + /// under (`None` if the build reserved nothing). The broadcast/abandon + /// release paths present it to + /// [`ManagedCoreFundsAccount::release_reservation_if_owner`] so a rejected + /// or abandoned send frees only reservations this build still owns. + pub fn reservation_token(&self) -> Option { + self.reservation_token + } + + /// The per-generation balance `Arc` of the wallet this payment was finalized + /// against — the unforgeable generation-identity marker the deferred-payment + /// registry pointer-compares before binding the payment to a wallet (see + /// [`origin_generation`](Self::origin_generation) field docs). Borrowed, not + /// consumed, so the check can run before + /// [`into_registered_parts`](Self::into_registered_parts) takes ownership. + pub(crate) fn origin_generation(&self) -> &Arc { + &self.origin_generation + } + + /// Consume this finalized transaction into the owned parts the deferred + /// [`SignedPaymentRegistry`](crate::SignedPaymentRegistry) stores. + /// + /// Consuming (rather than cloning) is what enforces unique reservation + /// ownership: `SignedCoreTransaction` is deliberately not `Clone`, so a + /// finalize yields exactly one ownership object and the registry can be + /// handed it exactly once — a caller cannot mint two live tokens that name + /// the same held reservation (`dashpay/platform#4185`). The transaction, + /// funding account, and reservation height are derived here, not supplied + /// independently by the caller. + pub(crate) fn into_registered_parts(self) -> RegisteredPaymentParts { + RegisteredPaymentParts { + transaction: self.transaction, + funding_account_type: self.funding_account_type, + funding_account_index: self.funding_account_index, + reservation_height: self.reservation_height, + reservation_token: self.reservation_token, + } + } +} + +/// The owned facts the deferred-payment registry takes over when it registers a +/// finalized transaction. Produced only by +/// [`SignedCoreTransaction::into_registered_parts`], which consumes the +/// non-`Clone` ownership object exactly once. +pub(crate) struct RegisteredPaymentParts { + pub(crate) transaction: Transaction, + pub(crate) funding_account_type: AccountTypePreference, + pub(crate) funding_account_index: u32, + pub(crate) reservation_height: u32, + pub(crate) reservation_token: Option, +} + +#[cfg(any(test, feature = "test-utils"))] +impl SignedCoreTransaction { + /// Build a `SignedCoreTransaction` directly, for tests that need a finalized + /// ownership object without running the full funding + signing pipeline + /// (e.g. the registry and FFI destroy/lifecycle tests). + /// + /// `origin_generation` is the per-generation balance `Arc` the payment is to + /// be treated as finalized against — a test that registers it must hand the + /// registry the SAME generation + /// ([`CoreWallet::test_generation_marker`](crate::CoreWallet::test_generation_marker)), + /// exactly as the production path binds a token to the finalizing wallet. + pub fn new_for_test( + transaction: Transaction, + fee: u64, + funding_account_type: AccountTypePreference, + funding_account_index: u32, + reservation_height: u32, + reservation_token: Option, + origin_generation: Arc, + ) -> Self { + Self { + transaction, + fee, + funding_account_type, + funding_account_index, + reservation_height, + reservation_token, + origin_generation, + } + } } fn account( @@ -125,7 +254,7 @@ impl CoreWallet { account_index: u32, signer: &S, ) -> Result { - let (unsigned, fee, selected, paths) = { + let (unsigned, fee, selected, paths, height, reservation_token) = { let mut manager = self.wallet_manager.write().await; let (wallet, info) = manager .get_wallet_and_info_mut(&self.wallet_id) @@ -147,13 +276,18 @@ impl CoreWallet { )) })?; - // `set_funding` observes ReservationSet and `build_unsigned` - // records its selection. There is no await between them and the - // manager write guard prevents another finalizer interleaving. - let (unsigned, fee) = builder + // `set_funding` observes ReservationSet and `build_unsigned_reserved` + // records its selection AND returns the token stamped onto the + // reserved inputs. There is no await between them and the manager + // write guard prevents another finalizer interleaving. The token + // rides in `SignedCoreTransaction` so a later abandon or rejected + // broadcast releases *only* the inputs this build still owns, even + // if a TTL sweep re-reserved them under a new token meanwhile + // (`dashpay/platform#4185`). + let (unsigned, fee, reservation_token) = builder .set_current_height(height) .set_funding(managed, &account) - .build_unsigned() + .build_unsigned_reserved() .map_err(|error| map_builder_error(error, account_type, account_index))?; let selected: Vec = match unsigned @@ -201,7 +335,7 @@ impl CoreWallet { } }; - (unsigned, fee, selected, paths) + (unsigned, fee, selected, paths, height, reservation_token) }; let signed = match signer @@ -212,8 +346,18 @@ impl CoreWallet { { Ok(signed) => signed, Err(error) => { - self.release_transaction_reservation(account_type, account_index, &unsigned) - .await; + // Signing awaited an (external) signer with the manager lock + // dropped, so key-wallet's TTL sweep could have reclaimed this + // build's reservation and a concurrent build re-taken the same + // inputs under a new token. Release owner-guarded so we free + // only what this build still owns. + self.release_transaction_reservation( + account_type, + account_index, + &unsigned, + reservation_token, + ) + .await; return Err(PlatformWalletError::TransactionBuild(error.to_string())); } }; @@ -223,38 +367,132 @@ impl CoreWallet { fee, funding_account_type: account_type, funding_account_index: account_index, + reservation_height: height, + reservation_token, + // Capture the finalizing wallet's generation identity so the + // deferred registry can refuse to bind this payment to any other + // wallet (`dashpay/platform#4185`). + origin_generation: Arc::clone(self.generation()), }) } /// Release a finalized transaction that the caller has chosen not to send. + /// + /// # Reservation age guard + /// + /// This is the abandon/free arm of the V2 finalized-transaction handle — + /// including the FFI broadcast/abandon *failure* paths (invalid or + /// wrong-generation wallet handle) that route their cleanup here, and the + /// host-language deinit/GC backstop + /// (`core_wallet_signed_transaction_v2_free`). A pinned handle can reach it + /// long after `finalize`, so it honors the **same** age bound as + /// [`broadcast_finalized_transaction`](Self::broadcast_finalized_transaction), + /// off the same shared [`reservation_expired`] predicate and the same + /// `last_processed_height` clock. + /// + /// Once the reservation has aged past + /// [`RESERVATION_MAX_AGE_BLOCKS`](crate::wallet::reservations::RESERVATION_MAX_AGE_BLOCKS), + /// key-wallet's `ReservationSet` TTL may already have swept its outpoint and + /// let an unrelated build re-reserve it; releasing by outpoint now would free + /// that newer build's reservation (`ReservationSet::release` is + /// unconditional). So the aged path **skips** the by-outpoint release and + /// leaves the outpoint for the TTL to reclaim, dropping only the handle — + /// exactly the policy the deferred registry's `reconcile_removed_entry` + /// applies. Below the bound the reservation is provably still ours and is + /// released so an immediate rebuild can reselect the inputs. pub async fn abandon_transaction(&self, transaction: &SignedCoreTransaction) { + if reservation_expired( + transaction.reservation_height, + self.last_processed_height().await, + ) { + // Aged past the shared bound: the outpoint may have been swept and + // re-reserved by an unrelated build. Leave it for key-wallet's TTL; + // releasing it here could free that newer reservation. The handle + // itself is already dropped by the caller (FFI storage removal). + return; + } self.release_transaction_reservation( transaction.funding_account_type, transaction.funding_account_index, &transaction.transaction, + transaction.reservation_token, ) .await; } + /// Release the funding reservation `transaction` holds, bound to this + /// handle's own wallet *generation*. + /// + /// `token` is the [`ReservationToken`] the build stamped onto the inputs + /// (`SignedCoreTransaction::reservation_token`). When present the release is + /// *owner-guarded* — it frees only inputs still owned by that token, so a + /// reservation key-wallet's TTL swept and a concurrent build re-took is left + /// untouched (`dashpay/platform#4185`). When `None` (the build reserved + /// nothing) it falls back to the unconditional by-outpoint release; that + /// path is never reached for a funded finalize, which always reserves. pub(crate) async fn release_transaction_reservation( &self, account_type: AccountTypePreference, account_index: u32, transaction: &Transaction, + token: Option, ) { + // Validate the generation AND mutate the `ReservationSet` under one + // manager-lock hold. `ReservationSet::release` removes an outpoint + // unconditionally, and it is reached via `wallet_id` — an identity that a + // remove-then-recreate under the same id preserves. Between a token's + // generation validation and this cleanup the wallet could therefore have + // been re-created, and an unguarded release-by-outpoint could then free + // the NEW generation's reservation on the same input. + // + // Binding the release to this handle's own generation closes that + // window: the wallet registered under `wallet_id` is the same generation + // as `self` iff their per-generation balance `Arc`s are pointer-equal + // (`wallet_id` + the shared manager `Arc` are both preserved across a + // recreation; only the balance `Arc` is fresh — the same identity + // `is_same_generation` uses). A read lock is enough and makes this atomic + // against recreation: a recreate needs the manager *write* lock, so it + // cannot interleave between the pointer check and the release below. let manager = self.wallet_manager.read().await; - let managed = manager.get_wallet_info(&self.wallet_id).and_then(|info| { - managed_account(&info.core_wallet.accounts, account_type, account_index) - }); - if let Some(managed) = managed { - managed.release_reservation(transaction); - } else { + let Some(info) = manager.get_wallet_info(&self.wallet_id) else { + tracing::warn!( + wallet_id = %hex::encode(self.wallet_id), + ?account_type, + account_index, + "could not release finalized Core transaction reservation: wallet not found" + ); + return; + }; + if !Arc::ptr_eq(&info.generation, self.generation()) { + // The wallet under this id is a different (re-created) generation: + // releasing by outpoint could free ITS reservation. Leave it — the + // original generation's reservation ceased to exist with it. tracing::warn!( wallet_id = %hex::encode(self.wallet_id), ?account_type, account_index, - "could not release finalized Core transaction reservation" + "skipping reservation release: wallet was re-created under the same id \ + (different generation) since the token was minted" ); + return; + } + match managed_account(&info.core_wallet.accounts, account_type, account_index) { + // Owner-guarded when the build stamped a token: even within this + // generation, a TTL sweep between build and release could have + // re-reserved the same outpoints under a new token, and an + // unconditional release would free that newer reservation. With the + // token key-wallet frees only inputs this build still owns. `None` + // (no reservation taken) falls back to the unconditional release. + Some(managed) => match token { + Some(token) => managed.release_reservation_if_owner(transaction, token), + None => managed.release_reservation(transaction), + }, + None => tracing::warn!( + wallet_id = %hex::encode(self.wallet_id), + ?account_type, + account_index, + "could not release finalized Core transaction reservation: account not found" + ), } } } diff --git a/packages/rs-platform-wallet/src/wallet/core/wallet.rs b/packages/rs-platform-wallet/src/wallet/core/wallet.rs index 8cc0488ade..fbf2c7684e 100644 --- a/packages/rs-platform-wallet/src/wallet/core/wallet.rs +++ b/packages/rs-platform-wallet/src/wallet/core/wallet.rs @@ -3,13 +3,15 @@ use std::sync::Arc; use super::balance::WalletBalance; +use super::generation::WalletGeneration; use dashcore::Address as DashAddress; -use tokio::sync::RwLock; +use tokio::sync::{RwLock, RwLockReadGuard}; 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; @@ -32,8 +34,10 @@ pub struct CoreWallet { /// Injected broadcaster — delegates to SPV or DAPI depending on how /// the wallet was constructed by `PlatformWalletManager`. pub(crate) broadcaster: Arc, - /// Lock-free balance for UI reads. - balance: Arc, + /// This handle's wallet *generation*: the lock-free balance the UI reads and + /// that generation's lifecycle gate, in the one `Arc` every handle to the + /// generation shares. + generation: Arc, } impl CoreWallet { @@ -42,20 +46,20 @@ impl CoreWallet { wallet_manager: Arc>>, wallet_id: WalletId, broadcaster: Arc, - balance: Arc, + generation: Arc, ) -> Self { Self { sdk, wallet_manager, wallet_id, broadcaster, - balance, + generation, } } /// Lock-free balance snapshot for UI reads. pub fn balance(&self) -> &WalletBalance { - &self.balance + self.generation.balance() } /// Wallet id this `CoreWallet` operates on. Exposed so FFI @@ -66,6 +70,75 @@ impl CoreWallet { self.wallet_id } + /// Whether `self` and `other` are handles to the same wallet *generation* — + /// the same logical wallet AND the same live in-memory instance. + /// + /// Two aliases of one generation (the `Arc` clones handed + /// out by `PlatformWalletManager::get_wallet`) share the per-generation + /// [`Arc`](WalletGeneration); a wallet removed and + /// re-created under the same `wallet_id` gets a fresh one. `Arc::ptr_eq` on + /// it therefore distinguishes generations that `wallet_id` — and the shared + /// multi-wallet `WalletManager` `Arc` — alone cannot (both are equal across a + /// remove-then-recreate). While either handle is held that `Arc` cannot be + /// freed, so its address can never be reused for a different generation, + /// which makes the pointer comparison sound (the same soundness argument the + /// registry already relies on for `Arc::ptr_eq` on the manager). + /// + /// This is the single generation identity shared by BOTH deferred-payment + /// paths — the registry-token path + /// ([`SignedPaymentRegistry`](crate::SignedPaymentRegistry), `dashpay/platform#4185`) + /// and the V2 finalized-transaction handle path (`dashpay/platform#4196`) — + /// so neither acts on a re-created wallet's `ReservationSet` while an old + /// handle still names the old generation. + pub fn is_same_generation( + &self, + other: &CoreWallet, + ) -> bool { + self.wallet_id == other.wallet_id + && Arc::ptr_eq(&self.wallet_manager, &other.wallet_manager) + && Arc::ptr_eq(&self.generation, &other.generation) + } + + /// This handle's [`WalletGeneration`] `Arc` — the generation-identity marker + /// (see [`is_same_generation`](Self::is_same_generation)). The manager stores + /// the same `Arc` in `PlatformWalletInfo.generation`, so a + /// reservation-cleanup path can, **under the manager lock**, compare this + /// against the wallet currently registered under `wallet_id` and act only if + /// they are the same generation — binding a validate-then-mutate to one lock + /// hold and refusing to touch a generation re-created under the same id. + pub(crate) fn generation(&self) -> &Arc { + &self.generation + } + + /// Enter THIS generation's lifecycle gate as a payment operation — see + /// [`WalletGeneration::payment_guard`]. + /// + /// Every path that publishes an ownership handle for this generation (a + /// registry token, a V2 finalized-transaction handle) or pushes one of its + /// transactions to the network must hold this across both its + /// [`is_current_generation`](Self::is_current_generation) check and the + /// action that check authorizes. Without it the check is a bare + /// point-in-time observation and a teardown can complete in the gap + /// (`dashpay/platform#4185`). + /// + /// Scoped to this generation, so holding it across a slow SPV send blocks + /// only this wallet's teardown — never an unrelated wallet's. + pub async fn generation_payment_guard(&self) -> RwLockReadGuard<'_, ()> { + self.generation.payment_guard().await + } + + /// This handle's per-generation identity marker, cloned — for tests (and + /// downstream FFI-crate tests via `test-utils`) that build a finalized + /// [`SignedCoreTransaction`](crate::SignedCoreTransaction) with + /// [`new_for_test`](crate::SignedCoreTransaction::new_for_test) and must + /// stamp it with the SAME generation they then register it against, exactly + /// as the production `finalize_transaction` path binds a token to the + /// finalizing wallet. + #[cfg(any(test, feature = "test-utils"))] + pub fn test_generation_marker(&self) -> Arc { + Arc::clone(&self.generation) + } + pub async fn set_gap_limit( &self, account_type: AccountTypePreference, @@ -286,6 +359,53 @@ impl CoreWallet { pub fn network(&self) -> key_wallet::Network { self.sdk.network } + + /// Current last-processed block height for this wallet, or `None` if the + /// wallet is no longer present in the manager. + /// + /// 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 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.last_processed_height()) + } + + /// Whether the generation this handle names is STILL the one registered + /// under its `wallet_id` in the manager. + /// + /// [`is_same_generation`](Self::is_same_generation) compares two *handles* + /// and therefore cannot see either way a generation stops being current: + /// + /// * **Removed** (`platform_wallet_manager_remove_wallet`). A retained + /// handle keeps `wallet_id`, the shared manager `Arc`, and its own + /// [`WalletGeneration`] `Arc` alive, so two handles to the removed + /// generation still compare equal to each other. Only a lookup against the + /// manager can tell that nothing is registered under the id any more. + /// * **Re-created** under the same id. `wallet_id` and the manager `Arc` are + /// preserved; only the `WalletGeneration` `Arc` is fresh. + /// + /// Both cases mean the same thing to a deferred payment: the accounts — + /// and therefore the `ReservationSet` holding its funding inputs — that this + /// handle names are no longer the wallet's live state, so acting on them + /// would spend against state the manager no longer owns. Callers that must + /// be atomic against a concurrent teardown take + /// [`generation_payment_guard`](Self::generation_payment_guard) around the + /// check and the action it gates; on its own this is a point-in-time + /// observation (`dashpay/platform#4185`). + pub async fn is_current_generation(&self) -> bool { + let wm = self.wallet_manager.read().await; + wm.get_wallet_info(&self.wallet_id) + .is_some_and(|info| Arc::ptr_eq(&info.generation, self.generation())) + } } impl std::fmt::Debug for CoreWallet { @@ -306,7 +426,72 @@ impl Clone for CoreWallet { wallet_manager: Arc::clone(&self.wallet_manager), wallet_id: self.wallet_id, broadcaster: Arc::clone(&self.broadcaster), - balance: Arc::clone(&self.balance), + generation: Arc::clone(&self.generation), } } } + +#[cfg(test)] +mod tests { + use std::sync::Arc; + + use key_wallet::account::account_type::StandardAccountType; + + use super::WalletGeneration; + use crate::test_support::{funded_wallet_manager, AlwaysOkBroadcaster}; + use crate::wallet::core::CoreWallet; + + /// The single generation identity both deferred-payment paths share: + /// aliases of one generation share the per-generation `WalletGeneration` `Arc` (same + /// generation), while a wallet re-created under the same `wallet_id` and the + /// same multi-wallet `WalletManager` `Arc` but a fresh generation `Arc` is a + /// DIFFERENT generation. Neither `wallet_id` nor the manager `Arc` alone can + /// tell them apart — the generation `Arc` is what distinguishes them, closing + /// the gap where an old handle could act through the old generation while a + /// new generation selected the same inputs. + #[tokio::test] + async fn is_same_generation_distinguishes_recreation_from_aliases() { + let (manager, wallet_id, generation, _signer) = + funded_wallet_manager(StandardAccountType::BIP44Account).await; + let sdk = Arc::new(dash_sdk::SdkBuilder::new_mock().build().expect("mock sdk")); + let broadcaster = Arc::new(AlwaysOkBroadcaster); + + let generation_a = CoreWallet::new( + Arc::clone(&sdk), + Arc::clone(&manager), + wallet_id, + Arc::clone(&broadcaster), + Arc::clone(&generation), + ); + + // A clone is an alias of the SAME generation (shares the generation Arc). + let alias = generation_a.clone(); + assert!( + generation_a.is_same_generation(&alias), + "aliases of one generation must compare equal" + ); + assert!(alias.is_same_generation(&generation_a)); + + // A re-created generation: SAME manager Arc + SAME wallet_id, fresh + // per-generation `WalletGeneration` Arc. + let generation_b = CoreWallet::new( + sdk, + Arc::clone(&manager), + wallet_id, + broadcaster, + Arc::new(WalletGeneration::new()), + ); + assert!( + !generation_a.is_same_generation(&generation_b), + "a re-created generation must NOT match, despite equal wallet_id + manager" + ); + // Sanity: it is ONLY the generation Arc that differs — wallet_id and the + // manager Arc are identical, so those checks alone could not tell the + // two generations apart. + assert_eq!(generation_a.wallet_id(), generation_b.wallet_id()); + assert!(Arc::ptr_eq( + &generation_a.wallet_manager, + &generation_b.wallet_manager + )); + } +} diff --git a/packages/rs-platform-wallet/src/wallet/identity/network/contact_requests.rs b/packages/rs-platform-wallet/src/wallet/identity/network/contact_requests.rs index fc96fd6ead..5df388e7ba 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/network/contact_requests.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/network/contact_requests.rs @@ -3338,7 +3338,7 @@ mod sweep_tests { use super::*; use crate::broadcaster::SpvBroadcaster; use crate::changeset::{ContactChangeSet, PlatformWalletChangeSet, SentContactRequestKey}; - use crate::wallet::core::WalletBalance; + use crate::wallet::core::WalletGeneration; use crate::wallet::identity::IdentityManager; use crate::wallet::persister::{NoPlatformPersistence, WalletPersister}; use crate::wallet::platform_wallet::PlatformWalletInfo; @@ -3362,7 +3362,7 @@ mod sweep_tests { fn empty_info(wallet: &Wallet) -> PlatformWalletInfo { PlatformWalletInfo { core_wallet: ManagedWalletInfo::from_wallet(wallet, 0), - balance: Arc::new(WalletBalance::new()), + generation: Arc::new(WalletGeneration::new()), identity_manager: IdentityManager::new(), tracked_asset_locks: BTreeMap::new(), } diff --git a/packages/rs-platform-wallet/src/wallet/mod.rs b/packages/rs-platform-wallet/src/wallet/mod.rs index 1963422be7..96e11a5ae6 100644 --- a/packages/rs-platform-wallet/src/wallet/mod.rs +++ b/packages/rs-platform-wallet/src/wallet/mod.rs @@ -11,6 +11,7 @@ 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; @@ -25,3 +26,6 @@ pub use platform_wallet::{ PlatformWallet, PlatformWalletInfo, WalletId, WalletStateReadGuard, WalletStateWriteGuard, }; pub use provider_key_at_index::{ProviderDerivedKey, ProviderKeyKind}; +pub use signed_payment_registry::{ + RegisterWrongGeneration, ReservationToken, SignedPaymentError, SignedPaymentRegistry, +}; diff --git a/packages/rs-platform-wallet/src/wallet/platform_wallet.rs b/packages/rs-platform-wallet/src/wallet/platform_wallet.rs index cfb310ea59..bf28a640c2 100644 --- a/packages/rs-platform-wallet/src/wallet/platform_wallet.rs +++ b/packages/rs-platform-wallet/src/wallet/platform_wallet.rs @@ -12,7 +12,7 @@ use tokio::sync::{RwLock, RwLockReadGuard, RwLockWriteGuard}; use super::asset_lock::manager::AssetLockManager; use super::asset_lock::tracked::TrackedAssetLock; -use super::core::{CoreWallet, WalletBalance}; +use super::core::{CoreWallet, WalletBalance, WalletGeneration}; use super::identity::{IdentityManager, IdentityWallet}; use super::persister::WalletPersister; use super::platform_addresses::PlatformAddressWallet; @@ -40,14 +40,28 @@ pub type WalletId = [u8; 32]; /// Lives inside `WalletManager.wallet_infos`. The `Wallet` /// key material is in `WalletManager.wallets` — NOT inside this struct. /// -/// `WalletBalance` is stored as `Arc` for lock-free UI reads. +/// The per-generation state (lock-free balance + lifecycle gate) is stored as +/// `Arc`; `Arc::ptr_eq` on it is this wallet's generation identity. pub struct PlatformWalletInfo { /// Core wallet metadata, accounts, UTXOs, balances. /// Delegates `WalletInfoInterface` methods. pub core_wallet: ManagedWalletInfo, - /// Lock-free balance for UI reads. Updated from `ManagedWalletInfo` after - /// each SPV block/mempool processing and RPC refresh. - pub balance: Arc, + /// This wallet generation's shared state: the lock-free balance for UI reads + /// (updated from `ManagedWalletInfo` after each SPV block/mempool processing + /// and RPC refresh) and the generation's lifecycle gate. + /// + /// Deliberately `pub(crate)`, not `pub`: this `Arc` *is* the generation + /// identity that `Arc::ptr_eq` compares, and `PlatformWalletInfo` is + /// reachable mutably from outside the crate through + /// [`PlatformWallet::state_mut`] / [`PlatformWallet::state_mut_blocking`]. + /// A public field would let safe downstream code drop a fresh `Arc` in here + /// while `PlatformWallet` and `CoreWallet` keep the original, splitting the + /// identity: `is_current_generation()` would then reject the still-live + /// wallet, generation-bound reservation cleanup would become a no-op, and + /// teardown would exclude through a different lifecycle gate than the + /// payment operations it has to fence. Read it through + /// [`PlatformWallet::generation`]; it is assigned only at construction. + pub(crate) generation: Arc, pub identity_manager: IdentityManager, pub tracked_asset_locks: BTreeMap, } @@ -79,8 +93,8 @@ pub struct PlatformWallet { pub(crate) asset_locks: Arc>, /// Per-wallet persistence handle. persister: WalletPersister, - /// Lock-free balance for UI reads, cloned from `PlatformWalletInfo.balance`. - pub(crate) balance: Arc, + /// This generation's shared state, cloned from `PlatformWalletInfo.generation`. + pub(crate) generation: Arc, /// Per-account Orchard keysets, populated by [`bind_shielded`]. /// `None` until bind has run; remains `None` for `WatchOnly` /// / `ExternalSignable` wallets that have never had a @@ -185,8 +199,14 @@ impl PlatformWallet { } /// Get the lock-free balance for UI reads. - pub fn balance(&self) -> &Arc { - &self.balance + pub fn balance(&self) -> &WalletBalance { + self.generation.balance() + } + + /// This wallet's [`WalletGeneration`] `Arc` — its generation identity and + /// lifecycle gate. See [`CoreWallet::is_same_generation`]. + pub fn generation(&self) -> &Arc { + &self.generation } /// Get a reference to the per-wallet persistence handle. @@ -411,7 +431,7 @@ impl PlatformWallet { sdk: Arc, wallet_id: WalletId, wallet_manager: Arc>>, - balance: Arc, + generation: Arc, lock_notify: Arc, persister: Arc, broadcaster: Arc, @@ -426,7 +446,7 @@ impl PlatformWallet { Arc::clone(&wallet_manager), wallet_id, Arc::clone(&broadcaster), - Arc::clone(&balance), + Arc::clone(&generation), ); // Asset-lock broadcaster is pinned to `SpvBroadcaster`; the @@ -475,7 +495,7 @@ impl PlatformWallet { platform, asset_locks, persister: wallet_persister, - balance, + generation, #[cfg(feature = "shielded")] shielded_keys: Arc::new(RwLock::new(None)), #[cfg(feature = "shielded")] @@ -1604,7 +1624,7 @@ impl Clone for PlatformWallet { platform: self.platform.clone(), asset_locks: self.asset_locks.clone(), persister: self.persister.clone(), - balance: self.balance.clone(), + generation: self.generation.clone(), #[cfg(feature = "shielded")] shielded_keys: self.shielded_keys.clone(), #[cfg(feature = "shielded")] diff --git a/packages/rs-platform-wallet/src/wallet/platform_wallet_traits.rs b/packages/rs-platform-wallet/src/wallet/platform_wallet_traits.rs index 9feb25bb04..62b1cef00e 100644 --- a/packages/rs-platform-wallet/src/wallet/platform_wallet_traits.rs +++ b/packages/rs-platform-wallet/src/wallet/platform_wallet_traits.rs @@ -37,7 +37,7 @@ impl WalletInfoInterface for PlatformWalletInfo { let inner = ManagedWalletInfo::from_wallet(wallet, birth_height); Self { core_wallet: inner, - balance: std::sync::Arc::new(super::core::WalletBalance::new()), + generation: std::sync::Arc::new(super::core::WalletGeneration::new()), identity_manager: super::identity::IdentityManager::new(), tracked_asset_locks: std::collections::BTreeMap::new(), } @@ -49,7 +49,7 @@ impl WalletInfoInterface for PlatformWalletInfo { let inner = ManagedWalletInfo::from_wallet_with_name(wallet, name, birth_height); Self { core_wallet: inner, - balance: std::sync::Arc::new(super::core::WalletBalance::new()), + generation: std::sync::Arc::new(super::core::WalletGeneration::new()), identity_manager: super::identity::IdentityManager::new(), tracked_asset_locks: std::collections::BTreeMap::new(), } diff --git a/packages/rs-platform-wallet/src/wallet/reservations.rs b/packages/rs-platform-wallet/src/wallet/reservations.rs index 10b1cbebdf..7c63cae2b5 100644 --- a/packages/rs-platform-wallet/src/wallet/reservations.rs +++ b/packages/rs-platform-wallet/src/wallet/reservations.rs @@ -22,6 +22,71 @@ use tokio::sync::RwLock; use crate::broadcaster::{BroadcastError, TransactionBroadcaster}; use crate::wallet::platform_wallet::{PlatformWalletInfo, WalletId}; +/// Maximum age, in `last_processed_height` blocks, of a held funding +/// reservation before an operation that would *consume* it (broadcast) is +/// refused. Shared by the two deferred/split core-send surfaces so they bound a +/// reservation's lifetime against the same TTL with one number: +/// +/// * the deferred build → broadcast/release registry +/// ([`SignedPaymentRegistry`](crate::SignedPaymentRegistry)), and +/// * the atomic V2 finalized-transaction handle path +/// (`core_wallet_tx_builder_finalize` → +/// `broadcast_finalized_transaction`). +/// +/// Kept strictly below key-wallet's `RESERVATION_TTL_BLOCKS` (24, ~1h at the +/// 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 +/// reservation that was already swept could free (or broadcast against) a newer, +/// unrelated one. Refusing at this lower bound guarantees the guard always trips +/// **before** the underlying reservation could have been swept, leaving a margin +/// for `last_processed_height` to lag a few blocks behind the true tip. +pub(crate) const RESERVATION_MAX_AGE_BLOCKS: u32 = 20; + +/// Whether a reservation stamped at `registered_height` is too old to act on at +/// `current_height` (see [`RESERVATION_MAX_AGE_BLOCKS`]). The registration +/// height is mandatory on both surfaces — it is derived from the finalized +/// [`SignedCoreTransaction::reservation_height`](crate::SignedCoreTransaction) +/// (captured inside the funding critical section, before the potentially-slow +/// external signer ran), never sampled independently. +/// +/// Both *consuming* (broadcasting) and *releasing by outpoint* (abandon/free) a +/// stale reservation are refused. Once the outpoint may already have been swept +/// by key-wallet's TTL and re-reserved by an unrelated build, broadcasting would +/// spend against that newer reservation and releasing would free it — +/// `ReservationSet::release` removes an outpoint unconditionally, with no +/// ownership/generation check. An aged reservation is therefore left for +/// key-wallet's TTL to reclaim: the guarded broadcast +/// ([`broadcast_finalized_transaction`](crate::CoreWallet::broadcast_finalized_transaction)) +/// returns `StaleReservation`, and the guarded abandon/free paths (the registry's +/// `reconcile_removed_entry` and +/// [`abandon_transaction`](crate::CoreWallet::abandon_transaction)) tear the +/// handle/registry entry down without touching the `ReservationSet`. +/// +/// An unknown *current* height means the wallet is gone from the manager, which +/// disables the guard (`None` → not expired). That is safe only because every +/// caller establishes liveness first and so never reaches here with a removed +/// wallet: the registry's +/// [`broadcast`](crate::SignedPaymentRegistry::broadcast) refuses with +/// `SignedPaymentError::WalletRemoved` before sampling the height, its +/// `reconcile_removed_entry` release is itself generation-bound and no-ops on a +/// missing wallet, and the V2 finalized-transaction handle path runs after the +/// FFI layer's generation-identity check. The earlier claim that "the +/// wallet-mismatch / account-lookup paths already reject those cases" was wrong +/// for the registry broadcast path — `is_same_generation` compares handles (a +/// removed generation matches itself) and that path performs no account lookup +/// at all (`dashpay/platform#4185`). +pub(crate) fn reservation_expired(registered_height: u32, current_height: Option) -> bool { + match current_height { + Some(current) => current.saturating_sub(registered_height) >= RESERVATION_MAX_AGE_BLOCKS, + None => false, + } +} + /// Broadcast `tx` and reconcile the funding account's UTXO reservation on /// failure. /// 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 0000000000..9ac78420e6 --- /dev/null +++ b/packages/rs-platform-wallet/src/wallet/signed_payment_registry.rs @@ -0,0 +1,1887 @@ +//! 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) validates the wallet +//! binding **under the lock** and removes **only a matching** entry, so a +//! repeated or concurrent broadcast of the same token can never +//! double-broadcast — the second caller finds nothing and gets +//! [`SignedPaymentError::StaleToken`] — and a wrong-wallet caller cannot +//! consume (and thereby strand) the rightful owner's token. +//! * [`release`](SignedPaymentRegistry::release) is idempotent: releasing an +//! unknown / already-consumed token is a silent no-op. +//! * A token is bound to the exact wallet *generation* it was minted against +//! ([`CoreWallet::is_same_generation`](crate::CoreWallet::is_same_generation) — +//! the same identity the V2 finalized-transaction handle path +//! (`dashpay/platform#4196`) uses). Two +//! wallets sharing one multi-wallet `PlatformWalletManager`, or a re-created +//! wallet under the same id whose in-memory `ReservationSet` no longer holds +//! the inputs, are both told apart: broadcasting through either is a +//! [`SignedPaymentError::WalletMismatch`] rather than a spend against stale +//! state. That check happens at the registry lock, but the reservation +//! cleanup that follows it runs later, off the registry lock — so the +//! check-then-cleanup is *not* one atomic step against a same-id recreation. +//! The cleanup is made safe on its own: every reservation release +//! ([`CoreWallet::release_transaction_reservation`]) re-validates the +//! generation and mutates the `ReservationSet` under a single manager-lock +//! hold, acting only if the wallet still registered under the id is the same +//! generation the token captured (its per-generation balance `Arc`). A +//! recreation needs the manager write lock, so it cannot slip between that +//! check and the release; a stale token can therefore never free a re-created +//! generation's reservation. +//! * A token has a bounded lifetime +//! ([`RESERVATION_MAX_AGE_BLOCKS`](crate::wallet::reservations::RESERVATION_MAX_AGE_BLOCKS)). Once the +//! 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. +//! 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 +//! +//! 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, MutexGuard}; + +use dashcore::{Transaction, Txid}; +use key_wallet::wallet::managed_wallet_info::transaction_building::AccountTypePreference; +// key-wallet's UTXO-reservation token, distinct from this registry's own +// `ReservationToken` (the u64 payment handle below). Aliased so the two never +// blur: the funding token identifies the reserved *inputs* for an owner-guarded +// release, the payment handle identifies the *registered payment*. +use key_wallet::ReservationToken as FundingReservationToken; + +use crate::broadcaster::TransactionBroadcaster; +use crate::wallet::core::{CoreWallet, SignedCoreTransaction}; +// The age bound and its predicate are shared with the atomic V2 finalized- +// transaction handle path (`broadcast_finalized_transaction`), so both surfaces +// measure a reservation's lifetime against key-wallet's TTL with one number. +use crate::wallet::reservations::reservation_expired; +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. +/// +/// A distinct newtype rather than a bare `u64` alias so a payment handle can +/// never be silently confused with any other numeric identifier (the funding +/// [`FundingReservationToken`], an account index, a raw height). It crosses the +/// C ABI as a `u64` — [`from`](ReservationToken::from) / [`as_u64`](ReservationToken::as_u64) +/// are the only conversions, applied at the FFI boundary. +#[repr(transparent)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)] +pub struct ReservationToken(u64); + +impl ReservationToken { + /// The raw wire value handed back across the FFI boundary to the host. + pub const fn as_u64(self) -> u64 { + self.0 + } +} + +impl From for ReservationToken { + fn from(value: u64) -> Self { + Self(value) + } +} + +impl From for u64 { + fn from(token: ReservationToken) -> Self { + token.0 + } +} + +impl std::fmt::Display for ReservationToken { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.0) + } +} + +/// 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 wallet the token was minted against is no longer registered in the + /// manager — it was removed (`platform_wallet_manager_remove_wallet`), so + /// its accounts and their `ReservationSet`s ceased to exist along with it. + /// + /// Distinct from [`WalletMismatch`](Self::WalletMismatch), which means a + /// *different* live generation answers to the same id. Here there is no live + /// generation at all, so there is nothing to broadcast against and nothing + /// to reconcile: the token is dropped WITHOUT releasing (a release by + /// outpoint would have no `ReservationSet` to act on, and the reservation + /// died with the generation). + /// + /// Refusing here is what stops a retained handle from pushing a removed + /// wallet's payment onto the network after the host believed the wallet was + /// gone (`dashpay/platform#4185`). The network was NOT touched. + #[error("reservation token {0} belongs to a wallet that is no longer in the manager")] + WalletRemoved(ReservationToken), + + /// The token has outlived + /// [`RESERVATION_MAX_AGE_BLOCKS`](crate::wallet::reservations::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 + /// on the network" signal). + #[error(transparent)] + Broadcast(#[from] PlatformWalletError), +} + +/// The wallet handed to [`SignedPaymentRegistry::register`] is **not** the +/// generation the payment was finalized against, so registering it would bind +/// the reservation to the wrong wallet. Registration is refused up front rather +/// than minting a token that later broadcasts through — and runs cleanup +/// against — a wallet whose `ReservationSet` never held the inputs +/// (`dashpay/platform#4185`). +/// +/// The rejected [`SignedCoreTransaction`] is returned so its held funding +/// reservation is **never stranded**: the caller still owns it and can release +/// it through the correct wallet ([`CoreWallet::abandon_transaction`]) or drop +/// it. This mirrors the owner-guarded discipline of the rest of the deferred +/// path — an ownership object is never dropped on a failure path without the +/// caller getting a chance to reconcile its reservation. +#[derive(Debug)] +pub struct RegisterWrongGeneration { + /// The finalized payment `register` refused to bind, handed back intact. + pub signed: SignedCoreTransaction, +} + +impl std::fmt::Display for RegisterWrongGeneration { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str("registration wallet is not the generation the payment was finalized against") + } +} + +impl std::error::Error for RegisterWrongGeneration {} + +/// 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 releasable funding-account handle — the account whose reservation + /// `finalize` took and which a rejected broadcast or an explicit release + /// must reconcile. An [`AccountTypePreference`] (not the narrower + /// `StandardAccountType`) so CoinJoin-funded deferred payments retain a + /// releasable handle too: `finalize` reserves the selected inputs for EVERY + /// account variant, so a CoinJoin token must be able to release them + /// immediately on rejection/abandon rather than stranding them until the + /// key-wallet TTL backstop. + account_type: AccountTypePreference, + account_index: u32, + /// Wallet `last_processed_height` captured inside the funding critical + /// section — the exact clock `finalize_transaction` stamps the funding + /// reservation with (`SignedCoreTransaction::reservation_height`). 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`](crate::wallet::reservations::RESERVATION_MAX_AGE_BLOCKS)). + /// Mandatory: it is derived from the consumed ownership object, never + /// sampled independently. + registered_height: u32, + /// The key-wallet [`FundingReservationToken`] stamped onto the funding + /// inputs when `finalize_transaction` reserved them + /// (`SignedCoreTransaction::reservation_token`), or `None` if the build + /// reserved nothing. A deferred payment can sit here across many blocks, so + /// key-wallet's TTL may sweep its reservation and a concurrent build + /// re-reserve the same inputs under a new token before this entry is + /// broadcast or released. Presenting this token to the owner-guarded release + /// frees only inputs still owned by this build, never the other build's + /// (`dashpay/platform#4185`). + funding_reservation_token: Option, +} + +/// 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()), + } + } + + /// 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 finalized [`SignedCoreTransaction`] (whose funding + /// UTXOs `finalize` already reserved) and return an opaque token for a later + /// [`broadcast`](Self::broadcast) or [`release`](Self::release). + /// + /// `signed` is **consumed**, which is what enforces unique reservation + /// ownership: `SignedCoreTransaction` is not `Clone`, so a single finalize + /// can be registered at most once — there is no way to mint two live tokens + /// that name the same held reservation (`dashpay/platform#4185`). The built + /// transaction, the funding account, the mandatory reservation height + /// (`SignedCoreTransaction::reservation_height` — captured inside the + /// funding critical section before the potentially-slow external signer ran, + /// so the age guard measures the reservation's true age rather than a + /// post-signing sample), and the owner-guard token + /// (`SignedCoreTransaction::reservation_token`) are all derived from that + /// object here rather than supplied independently by the caller. + /// + /// `core` is the wallet the token is bound to for its later broadcast / + /// release. It **must** be the same wallet *generation* the payment was + /// finalized against — validated here against the unforgeable + /// `origin_generation` marker `SignedCoreTransaction` captured at finalize. + /// Binding to any other wallet is refused with [`RegisterWrongGeneration`] + /// (the rejected `signed` handed back so its reservation is not stranded): + /// otherwise safe public code could finalize through wallet A and + /// `register(core_b, signed_from_a)`, after which broadcasting through B + /// would pass the generation check and submit A's transaction through B's + /// broadcaster while cleanup ran against B and A's real reservation leaked + /// until its TTL. Deriving/validating the core from the consumed object + /// (rather than trusting a separate argument) upholds the documented + /// guarantee that a token is bound to the generation whose `ReservationSet` + /// owns the inputs. + /// + /// Synchronous **by design**: the body performs the reservation-owning + /// insertion with no `.await`, so there is no future that could be dropped + /// before its first poll and silently drop the consumed `signed` — and its + /// held reservation — without inserting it. An `async fn` here would only + /// move `signed` into a future whose body runs on the first poll; dropping + /// that future before polling would leak the reservation to key-wallet's TTL + /// (`dashpay/platform#4185`). Callers invoke it directly. + /// + /// # Liveness is the caller's obligation + /// + /// The generation check here is `signed`-relative: it proves `core` is the + /// wallet that *finalized* the payment. It says nothing about whether that + /// wallet is still registered in the manager, and being synchronous it + /// cannot ask (the manager lock is `async`). `finalize_transaction` drops + /// the manager write lock before awaiting the signer, so a teardown can run + /// to completion — sweep included — while a finalize is mid-signature; the + /// `register` that follows would then insert a live token for a removed + /// generation, defeating the documented teardown invariant that dropping + /// tokens makes stale handles inert. + /// + /// Callers must therefore hold + /// [`CoreWallet::generation_payment_guard`] — the finalizing generation's own + /// lifecycle gate — across `CoreWallet::is_current_generation` and this call, + /// and abandon the payment (releasing its reservation) when the wallet is + /// gone. The FFI's `core_wallet_signed_payment_finalize` is the production + /// caller and does exactly that. + /// + /// The gate is acquired **after** the external signer returns, not around it: + /// holding a generation's gate across an open signing prompt would stall that + /// wallet's teardown for as long as the user takes, and the liveness check + /// makes it unnecessary. A finalizer whose wallet was torn down mid-signature + /// therefore observes the missing generation at its check and abandons + /// instead of registering. + pub fn register( + &self, + core: CoreWallet, + signed: SignedCoreTransaction, + ) -> Result { + // Bind the payment to the EXACT generation it was finalized against. + // `core.generation()` and `signed.origin_generation()` are the same kind + // of per-generation balance `Arc` `is_same_generation` pointer-compares; + // a mismatch means `core` is a different (switched / stale / unrelated) + // wallet than the one whose `ReservationSet` holds the inputs. Refuse + // BEFORE consuming `signed`, and hand it back so the caller can reconcile + // its reservation. + if !Arc::ptr_eq(core.generation(), signed.origin_generation()) { + return Err(RegisterWrongGeneration { signed }); + } + let parts = signed.into_registered_parts(); + let token = ReservationToken(self.next_token.fetch_add(1, Ordering::SeqCst)); + self.lock().insert( + token, + RegisteredPayment { + core, + tx: parts.transaction, + account_type: parts.funding_account_type, + account_index: parts.funding_account_index, + registered_height: parts.reservation_height, + funding_reservation_token: parts.reservation_token, + }, + ); + Ok(token) + } + + /// Broadcast the payment behind `token`, reconciling its UTXO reservation on + /// failure, then consume the token. + /// + /// The wallet binding is validated **under the registry lock**, and only a + /// *matching* entry is removed. So a wrong-wallet caller can never consume + /// (and thereby destroy) the rightful owner's token: a mismatched token is + /// left in the registry for its owner and this call returns + /// [`SignedPaymentError::WalletMismatch`]. `current` must be the same wallet + /// *generation* the token was minted against + /// (`CoreWallet::is_same_generation`); a re-created wallet under the same id + /// is a mismatch, not a spend against stale state. + /// + /// Because the check-and-consume happen atomically under one lock hold, a + /// repeated or concurrent broadcast of the same token by the rightful owner + /// gets [`SignedPaymentError::StaleToken`] instead of a second send — the + /// first consumer removed it. + /// + /// 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 { + // Validate the wallet binding UNDER the lock and consume ONLY a matching + // entry. Peeking first means a mismatched caller leaves the entry in + // place for its rightful owner rather than removing it (which would + // strand the owner's reservation until the TTL backstop). The + // check-then-remove is one lock hold, so it is atomic against a + // concurrent broadcast; the std::Mutex guard is dropped before any await. + // + // Hold `current`'s OWN generation lifecycle gate for the whole operation. + // That generation's teardown needs the exclusive side, so it cannot + // interleave between the liveness check below and the send: either the + // wallet is gone before we enter (our entry was already swept → + // `StaleToken`), or it stays live until we leave. Shared, so concurrent + // payments — on this generation and on every other — are unaffected, and + // scoped per generation, so holding it across the network send below + // blocks only THIS wallet's teardown rather than every wallet's + // (`dashpay/platform#4185`). + // + // Taking `current`'s gate rather than the entry's is sound because the + // only path that proceeds past the check below is one where + // `entry.core.is_same_generation(current)` held — i.e. they are the same + // generation and therefore the same gate. A mismatched caller returns + // without touching the entry or the network. + let _lifecycle = current.generation_payment_guard().await; + + let entry = { + let mut entries = self.lock(); + match entries.get(&token) { + None => return Err(SignedPaymentError::StaleToken(token)), + Some(entry) => { + // Same wallet generation the token was minted against — the + // single identity the V2 handle path also uses. A re-created + // wallet (same id + manager, new generation) is a mismatch. + if !entry.core.is_same_generation(current) { + // Leave the entry for its rightful owner. + return Err(SignedPaymentError::WalletMismatch(token)); + } + } + } + entries + .remove(&token) + .expect("entry present under the same lock hold") + }; + + // Refuse a token whose wallet is no longer registered in the manager. + // + // `is_same_generation` above compares two HANDLES, so it passes for a + // removed generation: both sides are the same removed wallet. Nothing + // further down re-checks — `broadcast_payment_releasing_reservation` + // goes straight to the broadcaster with no manager lookup, and the age + // guard below is *disabled* for a removed wallet + // (`last_processed_height` is `None`). So without this check a retained + // handle broadcasts a removed wallet's payment onto the network, and the + // teardown sweep cannot stop it: the sweep and the removal are one + // linearization point, but a broadcast that entered the gate first is + // outside it (`dashpay/platform#4185`). + // + // The entry is already removed, so we drop it WITHOUT releasing — the + // reservation ceased to exist with the generation, and a release by + // outpoint has no live `ReservationSet` to act on. Held under the + // lifecycle gate, so this is not a check-then-act: the wallet cannot be + // removed between here and the send below. + if !current.is_current_generation().await { + return Err(SignedPaymentError::WalletRemoved(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.last_processed_height().await, + ) { + return Err(SignedPaymentError::StaleReservationToken(token)); + } + + // One releasing-broadcast path for every funding variant, CoinJoin + // included: a definitive rejection releases the reservation for an + // immediate rebuild, an ambiguous outcome keeps it, and the release is + // bound to the token's own wallet generation. + let txid = entry + .core + .broadcast_payment_releasing_reservation( + entry.account_type, + entry.account_index, + &entry.tx, + entry.funding_reservation_token, + ) + .await?; + Ok(txid) + } + + /// Reconcile one already-removed entry's reservation, honouring the age + /// guard: if the token has outlived its reservation lifetime the funding + /// outpoint may already have been swept and re-selected by an unrelated + /// build, so releasing it by outpoint could free that newer reservation — + /// drop it without touching the `ReservationSet` (key-wallet's TTL reclaims + /// the original). Otherwise release the funding-account reservation (any + /// variant, CoinJoin included), bound to the token's own wallet generation. + async fn reconcile_removed_entry(entry: RegisteredPayment) { + if reservation_expired( + entry.registered_height, + entry.core.last_processed_height().await, + ) { + return; + } + entry + .core + .release_transaction_reservation( + entry.account_type, + entry.account_index, + &entry.tx, + entry.funding_reservation_token, + ) + .await; + } + + /// 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) { + // Same per-generation lifecycle gate as `broadcast`: the reconciliation + // below reads the manager to bind its release to a live generation, so + // that generation's teardown must not interleave between taking the entry + // and acting on it. + // + // No wallet handle is threaded in, so the gate has to come from the entry + // itself. PEEK the entry's generation without consuming it, drop the map + // lock (a `std::sync::Mutex` — it must never be held across an `.await`), + // take that generation's gate, and only then consume. Both ways the peek + // can go stale are already the correct outcome: if a teardown swept the + // entry, or a concurrent release/broadcast consumed it, the `remove` + // below returns `None` and this is the documented idempotent no-op. + let generation = { + let entries = self.lock(); + match entries.get(&token) { + // Unknown / already consumed — idempotent no-op. + None => return, + Some(entry) => Arc::clone(entry.core.generation()), + } + }; + let _lifecycle = generation.payment_guard().await; + + let entry = { self.lock().remove(&token) }; + let Some(entry) = entry else { + // Swept or consumed while we were acquiring the gate — no-op. + return; + }; + Self::reconcile_removed_entry(entry).await; + } + + /// Drop every outstanding token bound to `wallet` (same shared + /// `WalletManager` and `wallet_id`), WITHOUT releasing, returning how many + /// were removed. + /// + /// Called from the FFI at actual wallet-generation *teardown* + /// (`platform_wallet_manager_remove_wallet`): the wallet — and its accounts' + /// `ReservationSet`s — are removed from the manager, so the reservations + /// cease to exist and there is nothing to reconcile. Dropping the tokens here + /// also makes any stale handle to that generation inert, so a later + /// destroy/release of a lingering handle can never release-by-outpoint + /// against a re-created generation's inputs — this is the teardown half of + /// the single generation policy the deferred paths share. + /// + /// # Must be called under the removed generation's [`WalletGeneration::teardown_guard`] + /// + /// Dropping the tokens is only half of teardown; the other half is the + /// manager removal itself, and the two are one atomic step only if the + /// caller holds that generation's exclusive lifecycle gate across BOTH. + /// Sweeping without it leaves two windows a payment operation slips through — + /// a broadcast between the removal and this sweep still finds its entry, and + /// an in-flight finalizer registers a fresh token *after* this sweep has run + /// (`dashpay/platform#4185`). This function cannot take the gate itself: it + /// is synchronous, and the removal it must be atomic with is `async`. + /// + /// [`PlatformWalletManager::remove_wallet_with_teardown`](crate::PlatformWalletManager::remove_wallet_with_teardown) + /// is the supported way to satisfy this: it holds the gate across the removal + /// and runs the sweep as its teardown hook, so the ordering cannot be got + /// wrong by a caller — including a direct Rust embedder that never goes + /// through the FFI. + pub fn remove_entries_for_wallet(&self, wallet: &CoreWallet) -> usize { + let mut entries = self.lock(); + let before = entries.len(); + entries.retain(|_, entry| !entry.core.is_same_generation(wallet)); + before - entries.len() + } + + /// Number of outstanding (registered but not yet broadcast/released) tokens. + /// Exposed under `test-utils` so downstream FFI-layer tests (e.g. the + /// `platform_wallet_destroy` lifecycle tests) can observe registry state. + #[cfg(any(test, feature = "test-utils"))] + pub fn outstanding(&self) -> usize { + self.lock().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::transaction_building::AccountTypePreference; + use key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; + + use super::{ + RegisterWrongGeneration, ReservationToken, SignedPaymentError, SignedPaymentRegistry, + }; + use crate::broadcaster::{BroadcastError, TransactionBroadcaster}; + use crate::test_support::{ + funded_wallet_manager, AlwaysMaybeSentBroadcaster, AlwaysRejectedBroadcaster, WalletSigner, + }; + use crate::wallet::core::{CoreWallet, SignedCoreTransaction}; + use crate::wallet::reservations::RESERVATION_MAX_AGE_BLOCKS; + use crate::PlatformWalletError; + + /// The [`AccountTypePreference`] a `build_signed_tx` funding account maps to + /// — the registry now retains the full account handle (CoinJoin included), + /// so the tests register with the preference rather than the narrower + /// `StandardAccountType`. + fn preference(account_type: StandardAccountType) -> AccountTypePreference { + match account_type { + StandardAccountType::BIP44Account => AccountTypePreference::BIP44, + StandardAccountType::BIP32Account => AccountTypePreference::BIP32, + } + } + + /// 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)]) + } + + /// A testnet `CoreWallet` whose CoinJoin account 0 holds the funded UTXO — + /// the fixture for the CoinJoin-funded deferred-payment reservation tests. + async fn funded_coinjoin_core_wallet( + broadcaster: Arc, + ) -> (CoreWallet, WalletSigner, Vec<(DashAddress, u64)>) { + let (wallet_manager, wallet_id, balance, signer) = + crate::test_support::funded_coinjoin_wallet_manager().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_reserved` reserves the inputs and leaves the reservation + /// held for the later broadcast/release. Returns a finalized + /// [`SignedCoreTransaction`] — the same non-`Clone` ownership object the + /// production `finalize_transaction` path yields — so the test hands it to + /// [`SignedPaymentRegistry::register`] exactly once (it captures the funding + /// account, the reservation height, and the key-wallet owner-guard token). + 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"); + // 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 + .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, reservation_token) = builder + .build_signed_reserved(signer, |addr| { + managed_account.address_derivation_path(&addr) + }) + .await + .map_err(|e| PlatformWalletError::TransactionBuild(e.to_string()))?; + Ok(SignedCoreTransaction::new_for_test( + tx, + fee, + preference(account_type), + account_index, + current_height, + reservation_token, + // Stamp the finalizing generation so registering through this same + // `core` passes the registry's generation binding, exactly as the + // production finalize path does. + core.generation().clone(), + )) + } + + /// 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 signed = build_signed_tx( + &core, + StandardAccountType::BIP44Account, + 0, + &outputs, + &signer, + ) + .await + .expect("build should succeed"); + let expected_bytes = dashcore::consensus::serialize(signed.transaction()); + let expected_txid = signed.transaction().txid(); + + let token = registry + .register(core.clone(), signed) + .expect("test registers with the finalizing generation"); + 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 signed = build_signed_tx(&core, account_type, 0, &outputs, &signer) + .await + .expect("build should succeed"); + let token = registry + .register(core.clone(), signed) + .expect("test registers with the finalizing generation"); + + // 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:?}" + ); + } + } + + /// Regression for the deferred CoinJoin reservation leak: a CoinJoin-funded + /// deferred payment reserves its inputs (finalize reserves for EVERY account + /// variant), so releasing/abandoning it must free that reservation + /// immediately — not strand it until key-wallet's 24-block TTL. Before the + /// fix the registry entry carried only a `StandardAccountType`, so a CoinJoin + /// funding (which has none) reconciled nothing on release. + /// + /// Uses the production `finalize_transaction` path (the atomic + /// select+reserve+sign the FFI runs), which is the only builder that funds a + /// CoinJoin account, then registers/releases through the registry exactly as + /// `core_wallet_signed_payment_finalize` / `_release` do. The CoinJoin + /// funding path is a sweep (`SelectionStrategy::All`): the single output + /// drains the input minus fee, so no change address is derived — the only + /// shape a non-standard CoinJoin account can fund. + #[tokio::test] + async fn coinjoin_funded_release_frees_the_reservation_immediately() { + // A CoinJoin sweep of the funded account to a single recipient. + fn sweep_builder(recipient: &DashAddress) -> TransactionBuilder { + TransactionBuilder::new() + .set_selection_strategy(SelectionStrategy::All) + .add_output(recipient, 1_000_000) + } + + let broadcaster = Arc::new(RecordingBroadcaster::new()); + let (core, signer, outputs) = funded_coinjoin_core_wallet(broadcaster).await; + let recipient = outputs[0].0.clone(); + let registry = SignedPaymentRegistry::new(); + + // finalize: atomic select + reserve + sign against the CoinJoin account. + let finalized = core + .finalize_transaction( + sweep_builder(&recipient), + AccountTypePreference::CoinJoin, + 0, + &signer, + ) + .await + .expect("coinjoin finalize should succeed"); + + let token = registry + .register(core.clone(), finalized) + .expect("test registers with the finalizing generation"); + + // Reservation held: a second CoinJoin finalize finds no unreserved input. + let blocked = core + .finalize_transaction( + sweep_builder(&recipient), + AccountTypePreference::CoinJoin, + 0, + &signer, + ) + .await; + assert!( + matches!( + blocked, + Err(PlatformWalletError::CoreInsufficientFunds { .. }) + ), + "rebuild must fail while the CoinJoin reservation is held, got {blocked:?}" + ); + + // Abandon/nack: the release MUST free the CoinJoin reservation now, not + // strand it until the TTL backstop. + registry.release(token).await; + assert_eq!(registry.outstanding(), 0, "token consumed after release"); + + let rebuilt = core + .finalize_transaction( + sweep_builder(&recipient), + AccountTypePreference::CoinJoin, + 0, + &signer, + ) + .await; + assert!( + rebuilt.is_ok(), + "releasing a CoinJoin-funded token must free its reservation immediately, \ + 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 signed = build_signed_tx( + &core, + StandardAccountType::BIP44Account, + 0, + &outputs, + &signer, + ) + .await + .expect("build should succeed"); + let token = registry + .register(core.clone(), signed) + .expect("test registers with the finalizing generation"); + + 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 signed = build_signed_tx( + &core, + StandardAccountType::BIP44Account, + 0, + &outputs, + &signer, + ) + .await + .expect("build should succeed"); + let token = registry + .register(core.clone(), signed) + .expect("test registers with the finalizing generation"); + + 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 signed = build_signed_tx( + &core, + StandardAccountType::BIP44Account, + 0, + &outputs, + &signer, + ) + .await + .expect("build should succeed"); + let token = registry + .register(core.clone(), signed) + .expect("test registers with the finalizing generation"); + + 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 unknown = ReservationToken::from(9999); + let sent = registry.broadcast(unknown, &core).await; + assert!(matches!(sent, Err(SignedPaymentError::StaleToken(t)) if t == unknown)); + // Releasing an unknown token is a no-op, not a panic. + registry.release(unknown).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 signed = build_signed_tx( + &core_a, + StandardAccountType::BIP44Account, + 0, + &outputs_a, + &signer_a, + ) + .await + .expect("build should succeed"); + let token = registry + .register(core_a.clone(), signed) + .expect("test registers with the finalizing generation"); + + 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(), + 1, + "a mismatched broadcast must NOT consume the rightful owner's token" + ); + } + + /// Regression for `dashpay/platform#4185` blocker: registration must bind the + /// token to the SAME wallet generation the payment was finalized against, not + /// to a separately-supplied wallet. Registering a payment finalized through + /// wallet A through an unrelated wallet B is refused up front with + /// [`RegisterWrongGeneration`], no token is minted (so B can never broadcast + /// A's transaction through B's broadcaster or run cleanup against B), and the + /// rejected `SignedCoreTransaction` is handed back so A's reservation is not + /// stranded — releasing it through A frees the input for an immediate rebuild. + #[tokio::test] + async fn register_rejects_a_different_wallet_generation() { + let (core_a, signer_a, outputs_a) = funded_core_wallet( + StandardAccountType::BIP44Account, + Arc::new(CountingBroadcaster::new()), + ) + .await; + // A separate wallet-manager instance stands in for an unrelated / re-created + // generation: same account shape, different generation-identity `Arc`. + let (core_b, _signer_b, _outputs_b) = funded_core_wallet( + StandardAccountType::BIP44Account, + Arc::new(CountingBroadcaster::new()), + ) + .await; + let registry = SignedPaymentRegistry::new(); + + let signed = build_signed_tx( + &core_a, + StandardAccountType::BIP44Account, + 0, + &outputs_a, + &signer_a, + ) + .await + .expect("build should succeed"); + + // Registering A's finalized payment through wallet B is refused, and no + // token is minted. + let baseline = registry.outstanding(); + let rejected = registry.register(core_b.clone(), signed); + let RegisterWrongGeneration { signed } = match rejected { + Err(err) => err, + Ok(_) => panic!("registering through a different generation must be refused"), + }; + assert_eq!( + registry.outstanding(), + baseline, + "a rejected registration must not mint a token" + ); + + // Registering through the correct generation (an alias of A) is accepted: + // the guard binds to generation identity, not wallet-manager pointer. + let token = registry + .register(core_a.clone(), signed) + .expect("registering through the finalizing generation must be accepted"); + assert_eq!(registry.outstanding(), baseline + 1); + + // The reservation is A's and is reachable: releasing the token frees the + // input, so an immediate rebuild on A succeeds — nothing was stranded. + registry.release(token).await; + assert_eq!(registry.outstanding(), baseline); + let rebuilt = build_signed_tx( + &core_a, + StandardAccountType::BIP44Account, + 0, + &outputs_a, + &signer_a, + ) + .await; + assert!( + rebuilt.is_ok(), + "the reservation must be reachable after a rejected mis-binding, got {rebuilt:?}" + ); + } + + /// 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 signed = build_signed_tx( + &core, + StandardAccountType::BIP44Account, + 0, + &outputs, + &signer, + ) + .await + .expect("build should succeed"); + let token = registry + .register(core.clone(), signed) + .expect("test registers with the finalizing generation"); + + 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 signed = build_signed_tx( + &core, + StandardAccountType::BIP44Account, + 0, + &outputs, + &signer, + ) + .await + .expect("build should succeed"); + let token = registry + .register(core.clone(), signed) + .expect("test registers with the finalizing generation"); + + 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" + ); + } + + // NOTE: the former `concurrent_registers_yield_distinct_tokens` test + // registered sixteen clones of ONE reserved transaction to probe the token + // allocator. That is exactly the duplicate-capability pattern unique + // ownership now forbids: `register` consumes a non-`Clone` + // `SignedCoreTransaction`, so a single reservation can be registered at most + // once (`dashpay/platform#4185`). Token distinctness is guaranteed by + // construction (the `AtomicU64` allocator), and concurrent consumption is + // covered by `concurrent_broadcasts_serialize_to_one_send`. + + /// 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_last_processed_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 + .last_processed_height() + .await + .expect("last processed height"); + let signed = build_signed_tx( + &core, + StandardAccountType::BIP44Account, + 0, + &outputs, + &signer, + ) + .await + .expect("build should succeed"); + let token = registry + .register(core.clone(), signed) + .expect("test registers with the finalizing generation"); + + // 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_processed_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 + .last_processed_height() + .await + .expect("last processed height"); + let signed = build_signed_tx( + &core, + StandardAccountType::BIP44Account, + 0, + &outputs, + &signer, + ) + .await + .expect("build should succeed"); + let token = registry + .register(core.clone(), signed) + .expect("test registers with the finalizing generation"); + + 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"); + + // 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 signed = build_signed_tx( + &core, + StandardAccountType::BIP44Account, + 0, + &outputs, + &signer, + ) + .await + .expect("build should succeed"); + let token = registry + .register(core.clone(), signed) + .expect("test registers with the finalizing generation"); + + // 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(), + 1, + "a mismatched broadcast must NOT consume the rightful owner's token" + ); + } + + /// 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 signed_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(), signed_a) + .expect("test registers with the finalizing generation"); + let signed_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(), signed_b) + .expect("test registers with the finalizing generation"); + 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:?}" + ); + + // Generation teardown drops WITHOUT releasing: A's input stays reserved + // (the account's ReservationSet is conceptually gone with the wallet, so + // there is nothing to reconcile). An immediate rebuild on A still fails. + let blocked = build_signed_tx( + &core_a, + StandardAccountType::BIP44Account, + 0, + &outputs_a, + &signer_a, + ) + .await; + assert!( + matches!(blocked, Err(PlatformWalletError::TransactionBuild(_))), + "remove_entries_for_wallet must NOT release by outpoint, got {blocked:?}" + ); + } + + // NOTE: the former `release_entries_for_wallet_frees_the_reservation` test + // is removed with the `release_entries_for_wallet` method it exercised. + // Destroying wrapper aliases no longer releases deferred-payment tokens: a + // wrapper handle does not own the payment, so its destruction must leave the + // token live and broadcastable (`dashpay/platform#4185`, blocker 2). Token + // reservations are reconciled by the payment owner (explicit + // broadcast/release) or dropped at actual generation teardown + // (`remove_entries_for_wallet`). + + /// Regression for the wrong-wallet-broadcast token theft: a mismatched + /// caller must return `WalletMismatch` WITHOUT consuming the entry, so the + /// rightful owner's token — and its reservation — survive and it can still + /// be broadcast. Previously `broadcast` removed the entry and *then* + /// validated, so a wrong-wallet caller destroyed the owner's token and + /// stranded its reservation until the TTL backstop. + #[tokio::test] + async fn wrong_wallet_broadcast_preserves_the_owners_token() { + 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 is a different generation. + 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 signed = build_signed_tx( + &core_a, + StandardAccountType::BIP44Account, + 0, + &outputs_a, + &signer_a, + ) + .await + .expect("build should succeed"); + let token = registry + .register(core_a.clone(), signed) + .expect("test registers with the finalizing generation"); + + // Wrong wallet: mismatch, and the token MUST survive for its owner. + let mismatched = registry.broadcast(token, &core_b).await; + assert!( + matches!(mismatched, Err(SignedPaymentError::WalletMismatch(t)) if t == token), + "a wrong-wallet broadcast must be WalletMismatch, got {mismatched:?}" + ); + assert_eq!( + registry.outstanding(), + 1, + "the owner's token must survive a wrong-wallet broadcast" + ); + assert_eq!( + broadcaster_a.count.load(Ordering::SeqCst), + 0, + "nothing was sent for the mismatched caller" + ); + + // The rightful owner can still broadcast its own token. + registry + .broadcast(token, &core_a) + .await + .expect("the owner's broadcast should still succeed"); + assert_eq!( + broadcaster_a.count.load(Ordering::SeqCst), + 1, + "the owner's broadcast must reach the network exactly once" + ); + assert_eq!( + registry.outstanding(), + 0, + "the token is consumed by its owner" + ); + } + + /// Regression for the "reservation height captured before signing, token + /// height sampled after" gap: `register` takes the reservation's OWN stamp + /// height, so a slow external signer that let `last_processed_height` + /// advance between stamping and registration cannot make the token look + /// younger than the reservation it covers. + /// + /// The wallet is advanced to `H + (MAX_AGE - 1)` *before* the token is + /// registered — modelling a signer slow enough that a fresh + /// post-signing sample would read that higher height. The token is + /// registered with the reservation's real stamp height `H`. One more block + /// (`H + MAX_AGE`) then trips the guard: exactly `MAX_AGE` past the + /// reservation. Under the old behaviour (sampling `last_processed_height` + /// at register time) the baseline would have been `H + MAX_AGE - 1`, so the + /// same final height would read an age of 1 and the token would broadcast — + /// this test would fail. Baselining on the passed-in reservation height is + /// what keeps the guard tripping before key-wallet's TTL sweep. + #[tokio::test] + async fn register_baselines_on_reservation_height_not_a_post_signing_sample() { + let broadcaster = Arc::new(CountingBroadcaster::new()); + let (core, signer, outputs) = + funded_core_wallet(StandardAccountType::BIP44Account, Arc::clone(&broadcaster)).await; + let registry = SignedPaymentRegistry::new(); + + let reservation_height = core + .last_processed_height() + .await + .expect("last processed height"); + let signed = build_signed_tx( + &core, + StandardAccountType::BIP44Account, + 0, + &outputs, + &signer, + ) + .await + .expect("build should succeed"); + // The finalized object carries the reservation's OWN stamp height, + // captured at build time — not a value the caller samples at register. + assert_eq!(signed.reservation_height(), reservation_height); + + // Slow signer: the wallet advanced to just under the age bound while + // signing. A fresh sample here would read `reservation_height + + // MAX_AGE - 1`. + advance_processed_height(&core, reservation_height + RESERVATION_MAX_AGE_BLOCKS - 1).await; + + // Register: the age baseline is the reservation height the consumed + // object carries, not the advanced `last_processed_height` sampled now. + let token = registry + .register(core.clone(), signed) + .expect("test registers with the finalizing generation"); + + // One block past the reservation height (still below the 24-block TTL) + // trips the guard because the baseline is `reservation_height`. + advance_processed_height(&core, reservation_height + RESERVATION_MAX_AGE_BLOCKS).await; + + let sent = registry.broadcast(token, &core).await; + assert!( + matches!(sent, Err(SignedPaymentError::StaleReservationToken(t)) if t == token), + "a token past MAX_AGE from its reservation height must be StaleReservationToken, \ + got {sent:?}" + ); + assert_eq!( + broadcaster.count.load(Ordering::SeqCst), + 0, + "the network must not have been hit" + ); + } + + /// Replace the wallet's per-generation balance `Arc` under the manager write + /// lock, modelling a same-id remove-then-recreate: `wallet_id`, the manager + /// `Arc`, and the account `ReservationSet` (with the token's input still + /// reserved) are all preserved, only the generation marker is fresh. The + /// still-reserved input now conceptually belongs to the NEW generation. + async fn simulate_same_id_recreation(core: &CoreWallet) { + 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.generation = Arc::new(crate::wallet::core::WalletGeneration::new()); + } + + /// Regression for the non-atomic generation-validation + cleanup: a token's + /// generation is validated at the registry lock, but its reservation cleanup + /// runs later off that lock. If the wallet is removed and re-created under + /// the SAME id in that window, an unguarded release-by-outpoint would free + /// the NEW generation's reservation on the same input. + /// + /// This test recreates the generation (same id, fresh balance `Arc`) between + /// registration and the release, then releases the now-stale token and + /// asserts the reservation SURVIVES — the release, bound to the token's own + /// generation under the manager lock, refuses to touch the re-created + /// generation. An unconditional release-by-outpoint would instead free the + /// new generation's reservation, and the rebuild below would succeed; the + /// generation guard makes it still fail. + #[tokio::test] + async fn recreation_between_validation_and_cleanup_cannot_release_new_generation() { + let broadcaster = Arc::new(RecordingBroadcaster::new()); + let (core, signer, outputs) = + funded_core_wallet(StandardAccountType::BIP44Account, broadcaster).await; + let registry = SignedPaymentRegistry::new(); + + let signed = build_signed_tx( + &core, + StandardAccountType::BIP44Account, + 0, + &outputs, + &signer, + ) + .await + .expect("build should succeed"); + let token = registry + .register(core.clone(), signed) + .expect("test registers with the finalizing generation"); + + // Reservation held: a rebuild fails at input selection. + let blocked = build_signed_tx( + &core, + StandardAccountType::BIP44Account, + 0, + &outputs, + &signer, + ) + .await; + assert!( + matches!(blocked, Err(PlatformWalletError::TransactionBuild(_))), + "rebuild must fail while the reservation is held, got {blocked:?}" + ); + + // Same-id wallet recreation between the token's validation and its + // cleanup: the wallet under this id is now a DIFFERENT generation. + simulate_same_id_recreation(&core).await; + + // Old cleanup runs. The token is dropped, but its release must NOT touch + // the re-created generation's reservation. + registry.release(token).await; + assert_eq!(registry.outstanding(), 0, "the stale token is dropped"); + + // The (new generation's) reservation on the input SURVIVES: a rebuild + // still cannot reselect it. Pre-fix, the unconditional release-by-outpoint + // would have freed it and this rebuild would succeed. + let rebuilt = build_signed_tx( + &core, + StandardAccountType::BIP44Account, + 0, + &outputs, + &signer, + ) + .await; + assert!( + matches!(rebuilt, Err(PlatformWalletError::TransactionBuild(_))), + "a stale token's cleanup must NOT release a re-created generation's \ + reservation, got {rebuilt:?}" + ); + } + + /// A funding builder over the fixture's outputs, selecting largest-first + /// like the production send path. + fn payment_builder(outputs: &[(DashAddress, u64)]) -> TransactionBuilder { + let mut builder = + TransactionBuilder::new().set_selection_strategy(SelectionStrategy::LargestFirst); + for (addr, amount) in outputs { + builder = builder.add_output(addr, *amount); + } + builder + } + + /// Unconditionally release `tx`'s input reservation on the BIP44 account, + /// modelling key-wallet's TTL sweep returning the outpoint to the selectable + /// pool — WITHOUT touching the registry entry, which still holds the token. + async fn force_release_reservation( + core: &CoreWallet, + tx: &Transaction, + ) { + let wm = core.wallet_manager.read().await; + let (_, info) = wm + .get_wallet_and_info(&core.wallet_id()) + .expect("wallet present in manager"); + info.core_wallet + .accounts + .standard_bip44_accounts + .get(&0) + .expect("bip44 managed account") + .release_reservation(tx); + } + + /// Owner-guarded release regression (`dashpay/platform#4185`): a rejected + /// deferred broadcast must free ONLY the inputs its own build still owns. If + /// key-wallet's TTL swept this build's reservation and a concurrent build + /// re-reserved the same outpoint under a new token, the rejection's release + /// must leave that other build's reservation intact — freeing it would let + /// coin selection hand the outpoint to a third build and double-spend it. + /// The registry threads the build's key-wallet `ReservationToken` to the + /// reject path, so the release is owner-guarded rather than by-outpoint. + #[tokio::test] + async fn rejected_broadcast_releases_only_its_own_reservation_not_one_retaken_after_a_sweep() { + let broadcaster = Arc::new(AlwaysRejectedBroadcaster); + let (core, signer, outputs) = + funded_core_wallet(StandardAccountType::BIP44Account, broadcaster).await; + let registry = SignedPaymentRegistry::new(); + + // Build 1 reserves the sole funding UTXO under token T1 and registers it + // for deferred submission. + let finalized = core + .finalize_transaction( + payment_builder(&outputs), + AccountTypePreference::BIP44, + 0, + &signer, + ) + .await + .expect("first finalize should succeed"); + // Capture the built tx before `register` consumes the ownership object; + // the sweep below needs it to release the outpoint by hand. + let finalized_tx = finalized.transaction().clone(); + let token = registry + .register(core.clone(), finalized) + .expect("test registers with the finalizing generation"); + + // Model key-wallet's TTL sweep: the outpoint returns to the selectable + // pool, but the registry still holds T1. + force_release_reservation(&core, &finalized_tx).await; + + // A concurrent build re-selects and re-reserves that same outpoint under + // a NEW token T2. Held alive so its reservation persists to the end. + let retaken = core + .finalize_transaction( + payment_builder(&outputs), + AccountTypePreference::BIP44, + 0, + &signer, + ) + .await + .expect("re-reserving finalize should succeed after the sweep"); + + // Build 1's deferred broadcast is definitively rejected. Its release is + // owner-guarded by T1, so it must NOT free T2's reservation. + let sent = registry.broadcast(token, &core).await; + assert!( + matches!( + sent, + Err(SignedPaymentError::Broadcast( + PlatformWalletError::TransactionBroadcast(_) + )) + ), + "a rejected deferred broadcast must surface the rejection, got {sent:?}" + ); + + // T2 still owns the outpoint: a third build finds no free UTXO. Under the + // pre-fix unconditional release, build 1's rejection would have freed it + // and this build would succeed — double-spending T2's outpoint. + let third = core + .finalize_transaction( + payment_builder(&outputs), + AccountTypePreference::BIP44, + 0, + &signer, + ) + .await; + assert!( + matches!( + third, + Err(PlatformWalletError::CoreInsufficientFunds { .. }) + ), + "the re-taken reservation must survive build 1's rejected broadcast, got {third:?}" + ); + + // Keep T2's build (and thus its reservation) alive until the assertions run. + drop(retaken); + } +} diff --git a/packages/rs-unified-sdk-jni/src/wallet_manager.rs b/packages/rs-unified-sdk-jni/src/wallet_manager.rs index 78c25061fa..070d81357c 100644 --- a/packages/rs-unified-sdk-jni/src/wallet_manager.rs +++ b/packages/rs-unified-sdk-jni/src/wallet_manager.rs @@ -1225,6 +1225,200 @@ 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: +// [coreWalletFinalizeSignedPayment] atomically funds, reserves, signs, and +// registers a builder in one native call, returning the raw bytes to hand to a +// merchant server; the reservation is then broadcast on ack — or released on +// nack/abandonment. Backed by the process-global registry in `platform_wallet_ffi` +// (`core_wallet_signed_payment_*`). See `SignedPaymentRegistry`. + +/// `core_wallet_signed_payment_finalize` — atomically fund, reserve, sign, and +/// register a builder for deferred (BIP70/BIP270) submission in ONE native +/// operation. 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 a big-endian BLOB 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. Rather than double-broadcasting, an unusable token +/// throws one of three sibling codes: `ErrorStaleReservationToken` (34, aged +/// out), `ErrorReservationTokenConsumed` (35, unknown / already broadcast / +/// already released), or `ErrorReservationWalletMismatch` (36, different wallet +/// generation). `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`). diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swift index d22383d5f1..bcccd0e3e0 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swift @@ -69,6 +69,51 @@ public enum PlatformWalletResultCode: Int32, Sendable { /// Core definitively rejected the transaction. Its reserved inputs were /// released and a corrected transaction may be submitted again. case errorTransactionBroadcastRejected = 26 + // Codes 27-33 are claimed outside this PR and must not be reused here: + // 27 errorShutdownIncomplete (dashpay/platform#4268, merged), 29 + // errorAssetLockInsufficientFunds (#4184), 31 errorSigningKeyUnavailable + // (#4183/#4259), 32 errorTransactionBuild (#4247/#4256), 33 + // errorTransactionSigning (#4256); 28 and 30 are free. The deferred-token + // trio therefore occupies the contiguous block 34-36. These raw values + // MUST match `PlatformWalletFFIResultCode` in + // packages/rs-platform-wallet-ffi/src/error.rs — there is no compile-time + // check across the ABI. See ERROR_CODE_REGISTRY.md (#4261). + /// A deferred (BIP70/BIP270) reservation token has outlived its funding + /// reservation's lifetime: key-wallet's TTL may already have swept and + /// re-selected the inputs, so acting on it could touch a newer, unrelated + /// reservation. The call did NOT touch the network. NOT retryable in place — + /// rebuild the payment. + case errorStaleReservationToken = 34 + /// A deferred reservation token is unknown, already broadcast, or already + /// released — the guard that turns a double-broadcast (or a broadcast after + /// release) into a typed error instead of a second send. The call did NOT + /// touch the network. NOT retryable: rebuild the payment. (Release is + /// idempotent and never surfaces this.) + case errorReservationTokenConsumed = 35 + /// A deferred reservation token was minted against a different wallet + /// *generation* than the one broadcasting it (e.g. a wallet re-created under + /// the same id); its reservation lives in that other generation's reservation + /// set. The call did NOT touch the network and did NOT consume the rightful + /// owner's token. NOT retryable through this handle: rebuild the payment. + case errorReservationWalletMismatch = 36 + /// The named thing does not exist. Besides the handle/lookup failures this + /// has always covered, BOTH deferred-send paths report the + /// wallet-was-REMOVED case here. + /// + /// Deferred (BIP70/BIP270) *token* path: a signed-payment broadcast refuses + /// a token whose wallet is no longer registered in the manager, and a + /// signed-payment finalize refuses to register a payment whose wallet was + /// removed while it was being signed. + /// + /// Finalized-transaction *handle* (V2) path: `finalizeAtomic` publishes no + /// handle when the wallet was removed or re-created during signing, and + /// `broadcastTransactionWithOutcome(_: FinalizedCoreTransaction)` refuses a + /// handle whose generation is gone. + /// + /// Every one of these reconciles the build's UTXO reservation before + /// returning. Distinct from `errorReservationWalletMismatch` (36), where a + /// *different* live generation answers to the same id. The call did NOT touch + /// the network and is NOT retryable — the wallet is gone. /// A quiesce/drain barrier did not complete within its budget: an /// in-flight sync pass was still running when a Clear / reset / /// sync-stop needed it provably drained. The operation failed closed — @@ -135,6 +180,12 @@ public enum PlatformWalletResultCode: Int32, Sendable { self = .errorAssetLockFundingMismatch case PLATFORM_WALLET_FFI_RESULT_CODE_ERROR_TRANSACTION_BROADCAST_REJECTED: self = .errorTransactionBroadcastRejected + case PLATFORM_WALLET_FFI_RESULT_CODE_ERROR_STALE_RESERVATION_TOKEN: + self = .errorStaleReservationToken + case PLATFORM_WALLET_FFI_RESULT_CODE_ERROR_RESERVATION_TOKEN_CONSUMED: + self = .errorReservationTokenConsumed + case PLATFORM_WALLET_FFI_RESULT_CODE_ERROR_RESERVATION_WALLET_MISMATCH: + self = .errorReservationWalletMismatch case PLATFORM_WALLET_FFI_RESULT_CODE_ERROR_SHUTDOWN_INCOMPLETE: self = .errorShutdownIncomplete case PLATFORM_WALLET_FFI_RESULT_CODE_NOT_FOUND: @@ -259,6 +310,28 @@ public enum PlatformWalletError: LocalizedError { /// to retry, and the retry re-fetches the address nonce so the mismatch /// self-heals. The submitted/expected nonce values are in the message. case addressNonceMismatch(String) + /// A deferred (BIP70/BIP270) reservation token has outlived its funding + /// reservation's lifetime — key-wallet's TTL may already have swept and + /// re-selected the inputs. Nothing was broadcast. NOT retryable in place; + /// rebuild the payment. Sibling of `reservationTokenConsumed` and + /// `reservationWalletMismatch`, which this code used to conflate. + case staleReservationToken(String) + /// A deferred reservation token is unknown, already broadcast, or already + /// released — the double-broadcast guard. Nothing was broadcast. NOT + /// retryable; rebuild the payment. + case reservationTokenConsumed(String) + /// A deferred reservation token was minted against a different wallet + /// generation than the one broadcasting it (e.g. a wallet re-created under + /// the same id). Nothing was broadcast and the rightful owner's token was + /// not consumed. NOT retryable through this handle; rebuild the payment. + case reservationWalletMismatch(String) + /// The named thing does not exist. For the deferred payment calls this is + /// the wallet-was-REMOVED case: the token's wallet (or the wallet a payment + /// was just signed against) is no longer registered in the manager, so there + /// is no live generation to act through. Nothing was broadcast; the + /// finalize path reconciles the build's reservation before returning. NOT + /// retryable — unlike `reservationWalletMismatch`, no other generation holds + /// this payment either. /// A quiesce/drain barrier (Clear / reset / sync-stop) timed out with a /// sync pass still in flight. The operation failed closed — retry once /// sync is idle. @@ -285,6 +358,8 @@ public enum PlatformWalletError: LocalizedError { .transactionBroadcastUnconfirmed(let m), .transactionBroadcastRejected(let m), .addressNonceMismatch(let m), + .staleReservationToken(let m), .reservationTokenConsumed(let m), + .reservationWalletMismatch(let m), .shutdownIncomplete(let m), .notFound(let m), .unknown(let m): return m @@ -327,6 +402,12 @@ public enum PlatformWalletError: LocalizedError { self = .transactionBroadcastRejected(detail) case .errorAddressNonceMismatch: self = .addressNonceMismatch(detail) + case .errorStaleReservationToken: + self = .staleReservationToken(detail) + case .errorReservationTokenConsumed: + self = .reservationTokenConsumed(detail) + case .errorReservationWalletMismatch: + self = .reservationWalletMismatch(detail) case .errorShutdownIncomplete: self = .shutdownIncomplete(detail) case .notFound: self = .notFound(detail)