From ef7cdf2d1840e644f7fc69ae77f306422f480f44 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Wed, 1 Jul 2026 10:32:38 +0000 Subject: [PATCH 01/21] chore(deps): bump rust-dashcore to dev head 78d10022 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bump all 8 rust-dashcore workspace dependencies (dashcore, dash-spv, key-wallet, key-wallet-ffi, key-wallet-manager, dash-network, dash-network-seeds, dashcore-rpc) from 5c0113e7 to 78d10022 (rust-dashcore `dev` head, 15 commits, 0.43.0 -> 0.45.0). key-wallet's `Signer` trait gained a required `extended_public_key` method. Implement it for MnemonicResolverCoreSigner (real BIP-32 xpub derivation with private-scalar zeroization on every return path) and add stubs to the two rs-dpp test signers. Resolver + derive boilerplate extracted into a shared `resolve_and_derive` helper (DRY); the master scalar is wiped on both the success and derive-error paths. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01MoY6vhmqZuHzNsMfJ8wakQ πŸ€– Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent --- Cargo.lock | 70 +++++------ Cargo.toml | 16 +-- packages/rs-dpp/src/state_transition/mod.rs | 11 +- .../signing_tests.rs | 11 +- .../src/mnemonic_resolver_core_signer.rs | 115 ++++++++++++++---- 5 files changed, 151 insertions(+), 72 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8de05bacd92..512dffc0d2d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1206,7 +1206,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]] @@ -1636,8 +1636,8 @@ dependencies = [ [[package]] name = "dash-network" -version = "0.43.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=5c0113e7901551450f6063023eec4be95beeb6b9#5c0113e7901551450f6063023eec4be95beeb6b9" +version = "0.45.0" +source = "git+https://github.com/dashpay/rust-dashcore?rev=78d10022c92384affe9d71be3266ac7bb4710291#78d10022c92384affe9d71be3266ac7bb4710291" dependencies = [ "bincode", "bincode_derive", @@ -1647,8 +1647,8 @@ dependencies = [ [[package]] name = "dash-network-seeds" -version = "0.43.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=5c0113e7901551450f6063023eec4be95beeb6b9#5c0113e7901551450f6063023eec4be95beeb6b9" +version = "0.45.0" +source = "git+https://github.com/dashpay/rust-dashcore?rev=78d10022c92384affe9d71be3266ac7bb4710291#78d10022c92384affe9d71be3266ac7bb4710291" dependencies = [ "dash-network", ] @@ -1724,8 +1724,8 @@ dependencies = [ [[package]] name = "dash-spv" -version = "0.43.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=5c0113e7901551450f6063023eec4be95beeb6b9#5c0113e7901551450f6063023eec4be95beeb6b9" +version = "0.45.0" +source = "git+https://github.com/dashpay/rust-dashcore?rev=78d10022c92384affe9d71be3266ac7bb4710291#78d10022c92384affe9d71be3266ac7bb4710291" dependencies = [ "async-trait", "chrono", @@ -1753,8 +1753,8 @@ dependencies = [ [[package]] name = "dashcore" -version = "0.43.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=5c0113e7901551450f6063023eec4be95beeb6b9#5c0113e7901551450f6063023eec4be95beeb6b9" +version = "0.45.0" +source = "git+https://github.com/dashpay/rust-dashcore?rev=78d10022c92384affe9d71be3266ac7bb4710291#78d10022c92384affe9d71be3266ac7bb4710291" dependencies = [ "anyhow", "base64-compat", @@ -1779,13 +1779,13 @@ dependencies = [ [[package]] name = "dashcore-private" -version = "0.43.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=5c0113e7901551450f6063023eec4be95beeb6b9#5c0113e7901551450f6063023eec4be95beeb6b9" +version = "0.45.0" +source = "git+https://github.com/dashpay/rust-dashcore?rev=78d10022c92384affe9d71be3266ac7bb4710291#78d10022c92384affe9d71be3266ac7bb4710291" [[package]] name = "dashcore-rpc" -version = "0.43.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=5c0113e7901551450f6063023eec4be95beeb6b9#5c0113e7901551450f6063023eec4be95beeb6b9" +version = "0.45.0" +source = "git+https://github.com/dashpay/rust-dashcore?rev=78d10022c92384affe9d71be3266ac7bb4710291#78d10022c92384affe9d71be3266ac7bb4710291" dependencies = [ "dashcore-rpc-json", "hex", @@ -1797,8 +1797,8 @@ dependencies = [ [[package]] name = "dashcore-rpc-json" -version = "0.43.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=5c0113e7901551450f6063023eec4be95beeb6b9#5c0113e7901551450f6063023eec4be95beeb6b9" +version = "0.45.0" +source = "git+https://github.com/dashpay/rust-dashcore?rev=78d10022c92384affe9d71be3266ac7bb4710291#78d10022c92384affe9d71be3266ac7bb4710291" dependencies = [ "bincode", "dashcore", @@ -1812,8 +1812,8 @@ dependencies = [ [[package]] name = "dashcore_hashes" -version = "0.43.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=5c0113e7901551450f6063023eec4be95beeb6b9#5c0113e7901551450f6063023eec4be95beeb6b9" +version = "0.45.0" +source = "git+https://github.com/dashpay/rust-dashcore?rev=78d10022c92384affe9d71be3266ac7bb4710291#78d10022c92384affe9d71be3266ac7bb4710291" dependencies = [ "bincode", "dashcore-private", @@ -2428,7 +2428,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]] @@ -2489,7 +2489,7 @@ checksum = "0ce92ff622d6dadf7349484f42c93271a0d49b7cc4d466a936405bacbe10aa78" dependencies = [ "cfg-if", "rustix 1.1.4", - "windows-sys 0.59.0", + "windows-sys 0.52.0", ] [[package]] @@ -2866,8 +2866,8 @@ dependencies = [ [[package]] name = "git-state" -version = "0.43.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=5c0113e7901551450f6063023eec4be95beeb6b9#5c0113e7901551450f6063023eec4be95beeb6b9" +version = "0.45.0" +source = "git+https://github.com/dashpay/rust-dashcore?rev=78d10022c92384affe9d71be3266ac7bb4710291#78d10022c92384affe9d71be3266ac7bb4710291" [[package]] name = "glob" @@ -3803,7 +3803,7 @@ checksum = "3640c1c38b8e4e43584d8df18be5fc6b0aa314ce6ebf51b53313d4306cca8e46" dependencies = [ "hermit-abi", "libc", - "windows-sys 0.59.0", + "windows-sys 0.52.0", ] [[package]] @@ -4033,8 +4033,8 @@ dependencies = [ [[package]] name = "key-wallet" -version = "0.43.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=5c0113e7901551450f6063023eec4be95beeb6b9#5c0113e7901551450f6063023eec4be95beeb6b9" +version = "0.45.0" +source = "git+https://github.com/dashpay/rust-dashcore?rev=78d10022c92384affe9d71be3266ac7bb4710291#78d10022c92384affe9d71be3266ac7bb4710291" dependencies = [ "aes", "async-trait", @@ -4062,8 +4062,8 @@ dependencies = [ [[package]] name = "key-wallet-ffi" -version = "0.43.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=5c0113e7901551450f6063023eec4be95beeb6b9#5c0113e7901551450f6063023eec4be95beeb6b9" +version = "0.45.0" +source = "git+https://github.com/dashpay/rust-dashcore?rev=78d10022c92384affe9d71be3266ac7bb4710291#78d10022c92384affe9d71be3266ac7bb4710291" dependencies = [ "cbindgen 0.29.4", "dash-network", @@ -4078,8 +4078,8 @@ dependencies = [ [[package]] name = "key-wallet-manager" -version = "0.43.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=5c0113e7901551450f6063023eec4be95beeb6b9#5c0113e7901551450f6063023eec4be95beeb6b9" +version = "0.45.0" +source = "git+https://github.com/dashpay/rust-dashcore?rev=78d10022c92384affe9d71be3266ac7bb4710291#78d10022c92384affe9d71be3266ac7bb4710291" dependencies = [ "async-trait", "bincode", @@ -4596,7 +4596,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]] @@ -5696,7 +5696,7 @@ dependencies = [ "once_cell", "socket2 0.5.10", "tracing", - "windows-sys 0.59.0", + "windows-sys 0.52.0", ] [[package]] @@ -6486,7 +6486,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys 0.4.15", - "windows-sys 0.59.0", + "windows-sys 0.52.0", ] [[package]] @@ -6499,7 +6499,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys 0.12.1", - "windows-sys 0.59.0", + "windows-sys 0.52.0", ] [[package]] @@ -6558,7 +6558,7 @@ dependencies = [ "security-framework", "security-framework-sys", "webpki-root-certs", - "windows-sys 0.59.0", + "windows-sys 0.52.0", ] [[package]] @@ -7418,7 +7418,7 @@ dependencies = [ "getrandom 0.4.2", "once_cell", "rustix 1.1.4", - "windows-sys 0.59.0", + "windows-sys 0.52.0", ] [[package]] @@ -8867,7 +8867,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 5d5f2960f1d..c36a7a9c874 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -50,14 +50,14 @@ members = [ ] [workspace.dependencies] -dashcore = { git = "https://github.com/dashpay/rust-dashcore", rev = "5c0113e7901551450f6063023eec4be95beeb6b9" } -dash-network-seeds = { git = "https://github.com/dashpay/rust-dashcore", rev = "5c0113e7901551450f6063023eec4be95beeb6b9" } -dash-spv = { git = "https://github.com/dashpay/rust-dashcore", rev = "5c0113e7901551450f6063023eec4be95beeb6b9" } -key-wallet = { git = "https://github.com/dashpay/rust-dashcore", rev = "5c0113e7901551450f6063023eec4be95beeb6b9" } -key-wallet-ffi = { git = "https://github.com/dashpay/rust-dashcore", rev = "5c0113e7901551450f6063023eec4be95beeb6b9" } -key-wallet-manager = { git = "https://github.com/dashpay/rust-dashcore", rev = "5c0113e7901551450f6063023eec4be95beeb6b9" } -dash-network = { git = "https://github.com/dashpay/rust-dashcore", rev = "5c0113e7901551450f6063023eec4be95beeb6b9" } -dashcore-rpc = { git = "https://github.com/dashpay/rust-dashcore", rev = "5c0113e7901551450f6063023eec4be95beeb6b9" } +dashcore = { git = "https://github.com/dashpay/rust-dashcore", rev = "78d10022c92384affe9d71be3266ac7bb4710291" } +dash-network-seeds = { git = "https://github.com/dashpay/rust-dashcore", rev = "78d10022c92384affe9d71be3266ac7bb4710291" } +dash-spv = { git = "https://github.com/dashpay/rust-dashcore", rev = "78d10022c92384affe9d71be3266ac7bb4710291" } +key-wallet = { git = "https://github.com/dashpay/rust-dashcore", rev = "78d10022c92384affe9d71be3266ac7bb4710291" } +key-wallet-ffi = { git = "https://github.com/dashpay/rust-dashcore", rev = "78d10022c92384affe9d71be3266ac7bb4710291" } +key-wallet-manager = { git = "https://github.com/dashpay/rust-dashcore", rev = "78d10022c92384affe9d71be3266ac7bb4710291" } +dash-network = { git = "https://github.com/dashpay/rust-dashcore", rev = "78d10022c92384affe9d71be3266ac7bb4710291" } +dashcore-rpc = { git = "https://github.com/dashpay/rust-dashcore", rev = "78d10022c92384affe9d71be3266ac7bb4710291" } tokio-metrics = "0.5" diff --git a/packages/rs-dpp/src/state_transition/mod.rs b/packages/rs-dpp/src/state_transition/mod.rs index 92bf4a4e583..74ebc13c977 100644 --- a/packages/rs-dpp/src/state_transition/mod.rs +++ b/packages/rs-dpp/src/state_transition/mod.rs @@ -3335,7 +3335,7 @@ mod tests { use dashcore::secp256k1::{ ecdsa, rand::rngs::OsRng, Message, PublicKey, Secp256k1, SecretKey, }; - use key_wallet::bip32::DerivationPath; + use key_wallet::bip32::{DerivationPath, ExtendedPubKey}; use key_wallet::signer::{Signer as KwSigner, SignerMethod}; /// Fixed-key in-memory signer used only by this test. Mirrors how a @@ -3370,6 +3370,15 @@ mod tests { async fn public_key(&self, _path: &DerivationPath) -> Result { Ok(self.public) } + + async fn extended_public_key( + &self, + _path: &DerivationPath, + ) -> Result { + // Test stub holds a single raw key with no chain code; extended + // public key derivation is not meaningful here. + Err("FixedKeySigner: no chain code β€” extended_public_key not supported".to_string()) + } } // Generate a single random key. Using the same key on both sides is diff --git a/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_funding_from_asset_lock_transition/signing_tests.rs b/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_funding_from_asset_lock_transition/signing_tests.rs index 67f95088409..b2c4fb67f83 100644 --- a/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_funding_from_asset_lock_transition/signing_tests.rs +++ b/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_funding_from_asset_lock_transition/signing_tests.rs @@ -204,7 +204,7 @@ async fn try_from_asset_lock_with_signer_and_private_key_signs_multiple_inputs() async fn try_from_asset_lock_with_signers_produces_matching_signature() { use async_trait::async_trait; use dashcore::secp256k1::{ecdsa, Message}; - use key_wallet::bip32::DerivationPath; + use key_wallet::bip32::{DerivationPath, ExtendedPubKey}; use key_wallet::signer::{Signer as KwSigner, SignerMethod}; /// Fixed-key in-memory `key_wallet::signer::Signer`. Mirrors how the @@ -237,6 +237,15 @@ async fn try_from_asset_lock_with_signers_produces_matching_signature() { async fn public_key(&self, _path: &DerivationPath) -> Result { Ok(self.public) } + + async fn extended_public_key( + &self, + _path: &DerivationPath, + ) -> Result { + // Test stub holds a single raw key with no chain code; extended + // public key derivation is not meaningful here. + Err("FixedKeySigner: no chain code β€” extended_public_key not supported".to_string()) + } } let secp = Secp256k1::new(); diff --git a/packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs b/packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs index ade11828594..bda1d3aa00f 100644 --- a/packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs +++ b/packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs @@ -64,7 +64,7 @@ use std::ffi::c_void; use std::os::raw::c_char; use async_trait::async_trait; -use key_wallet::bip32::{DerivationPath, ExtendedPrivKey}; +use key_wallet::bip32::{DerivationPath, ExtendedPrivKey, ExtendedPubKey}; use key_wallet::dashcore::secp256k1::{self, Secp256k1}; use key_wallet::signer::{Signer, SignerMethod}; use key_wallet::Network; @@ -222,18 +222,32 @@ impl MnemonicResolverCoreSigner { } } - /// Resolve the mnemonic from the Swift-side callback, then - /// derive the secp256k1 private key at `path`. Returns the raw - /// 32-byte scalar in a `Zeroizing` wrapper so the caller's last - /// drop point zeros it. + /// Resolve the mnemonic from the Swift-side callback, then derive the + /// BIP-32 extended private key at `path`. /// - /// All other intermediate buffers (mnemonic, seed) are dropped - /// (and zeroed) before this method returns β€” only the final - /// derived scalar leaks out, and even that is `Zeroizing`-wrapped. - fn derive_priv( + /// This is the single entry-point for all private-key material in this + /// signer. It handles the full stack: resolver FFI call β†’ result-code + /// mapping β†’ UTF-8 + word-list validation β†’ BIP-39 seed β†’ master + /// `ExtendedPrivKey` β†’ child `ExtendedPrivKey` at `path`. + /// + /// # Zeroization contract + /// + /// - The `master` scalar is wiped inside this helper before returning. + /// - The *caller* is responsible for wiping `derived.private_key` after + /// extracting whatever it needs β€” `ExtendedPrivKey` carries no + /// `Drop`-based zeroization (see the `TODO(upstream)` below). + /// - All other intermediate buffers (mnemonic, seed) are `Zeroizing`- + /// wrapped and wiped on drop before this method returns. + /// + /// # Errors + /// + /// Propagates [`MnemonicResolverSignerError`] for every failure mode: + /// null handle, resolver FFI errors, encoding/parse failures, and BIP-32 + /// derivation errors. + fn resolve_and_derive( &self, path: &DerivationPath, - ) -> Result, MnemonicResolverSignerError> { + ) -> Result { if self.resolver_addr == 0 { return Err(MnemonicResolverSignerError::NullHandle); } @@ -293,27 +307,53 @@ impl MnemonicResolverCoreSigner { let secp = Secp256k1::new(); let mut master = ExtendedPrivKey::new_master(self.network, seed.as_ref()) .map_err(|e| MnemonicResolverSignerError::DerivationFailed(format!("master: {e}")))?; - let mut derived = master - .derive_priv(&secp, path) - .map_err(|e| MnemonicResolverSignerError::DerivationFailed(format!("path: {e}")))?; - - // `secret_bytes()` returns a plain `[u8; 32]`; wrap in - // `Zeroizing` so the caller (and any panic-unwind path) - // wipes it on drop. - let bytes = Zeroizing::new(derived.private_key.secret_bytes()); - // TODO(upstream): `key_wallet::bip32::ExtendedPrivKey` has no // `Drop` / `Zeroize` impl β€” the inner `secp256k1::SecretKey` - // scalars on `master` and `derived` would otherwise drop - // un-wiped. Mirrors the SecretKey-copy hole CodeRabbit R7 - // flagged at the sign-site. Proper fix is a `Zeroize` / - // `ZeroizeOnDrop` impl in `dashpay/rust-dashcore`'s - // `key-wallet/src/bip32.rs`; until that lands, wipe the two - // SecretKey fields explicitly here. Mirrored in the sibling - // FFI at `rs-platform-wallet-ffi/src/sign_with_mnemonic_resolver.rs`. + // scalar on `master` would otherwise drop un-wiped. The `derived` + // scalar is the caller's responsibility (see zeroization contract + // above). Proper fix is a `Zeroize` / `ZeroizeOnDrop` impl in + // `dashpay/rust-dashcore`'s `key-wallet/src/bip32.rs`; until + // that lands, wipe the master scalar explicitly on BOTH paths: + // the error arm below (early return) and the success path after + // the match. Mirrored in the sibling FFI at + // `rs-platform-wallet-ffi/src/sign_with_mnemonic_resolver.rs`. + let derived = match master.derive_priv(&secp, path) { + Ok(d) => d, + Err(e) => { + // Wipe master before returning β€” the success path below + // never runs, so line 323's erase would be skipped without + // this explicit call. + master.private_key.non_secure_erase(); + return Err(MnemonicResolverSignerError::DerivationFailed(format!( + "path: {e}" + ))); + } + }; + + // Success path: master scalar wiped here; derived is the caller's + // responsibility (documented in the zeroization contract above). master.private_key.non_secure_erase(); - derived.private_key.non_secure_erase(); + Ok(derived) + } + + /// Resolve the mnemonic and derive the raw 32-byte scalar at `path`. + /// + /// Returns the scalar in a `Zeroizing` wrapper so the caller's last + /// drop point wipes it. All other intermediate key material is zeroed + /// before this method returns (see [`Self::resolve_and_derive`]). + fn derive_priv( + &self, + path: &DerivationPath, + ) -> Result, MnemonicResolverSignerError> { + let mut derived = self.resolve_and_derive(path)?; + // `secret_bytes()` copies the 32-byte scalar; wrap in `Zeroizing` + // so the caller's drop point wipes it. + let bytes = Zeroizing::new(derived.private_key.secret_bytes()); + // Wipe the derived extended key's scalar β€” the extracted `bytes` + // carry it from here. Mirrors the SecretKey-copy hole noted in + // `resolve_and_derive`'s TODO(upstream). + derived.private_key.non_secure_erase(); Ok(bytes) } } @@ -362,6 +402,27 @@ impl Signer for MnemonicResolverCoreSigner { secret.non_secure_erase(); Ok(pubkey) } + + /// Derive the BIP-32 extended public key at `path`. + /// + /// Returns the full [`ExtendedPubKey`] (public point + chain code) so + /// callers can perform non-hardened child derivation locally without + /// additional round-trips to the resolver. All intermediate private-key + /// material is zeroized before this method returns (see + /// [`Self::resolve_and_derive`]); `ExtendedPubKey` carries only public + /// information and requires no further wiping. + async fn extended_public_key( + &self, + path: &DerivationPath, + ) -> Result { + let mut derived = self.resolve_and_derive(path)?; + let secp = Secp256k1::new(); + // Capture the extended public key *before* wiping the private scalar. + // `ExtendedPubKey` carries only public material (chain code + point). + let xpub = ExtendedPubKey::from_priv(&secp, &derived); + derived.private_key.non_secure_erase(); + Ok(xpub) + } } #[cfg(test)] From 27d235fe437a99331c2b9b2812d5e5efbbfb18ec Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Wed, 1 Jul 2026 11:24:24 +0000 Subject: [PATCH 02/21] fix(dpp): implement extended_public_key on shielded FixedKeySigner test stub MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rust-dashcore bump added `Signer::extended_public_key` as a required trait method. The shielded `shield_from_asset_lock_transition` signing test's `FixedKeySigner` stub was missed: it is feature-gated behind `all(test, state-transition-signing, core_key_wallet, shielded-client)` and is not compiled under default features, so the local workspace check passed while CI (which enables those features) failed with E0046. Add the stub matching the two sibling test signers (returns Err β€” the fixed-key stub carries no chain code). Verified with `cargo check -p dpp --tests --features state-transition-signing,core_key_wallet,shielded-client`. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01MoY6vhmqZuHzNsMfJ8wakQ πŸ€– Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent --- .../signing_tests.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/rs-dpp/src/state_transition/state_transitions/shielded/shield_from_asset_lock_transition/signing_tests.rs b/packages/rs-dpp/src/state_transition/state_transitions/shielded/shield_from_asset_lock_transition/signing_tests.rs index c31e3b1fc1e..5ea8e633b3a 100644 --- a/packages/rs-dpp/src/state_transition/state_transitions/shielded/shield_from_asset_lock_transition/signing_tests.rs +++ b/packages/rs-dpp/src/state_transition/state_transitions/shielded/shield_from_asset_lock_transition/signing_tests.rs @@ -33,7 +33,7 @@ use platform_version::version::PlatformVersion; use async_trait::async_trait; use dashcore::secp256k1::{ecdsa, Message, PublicKey, Secp256k1, SecretKey}; -use key_wallet::bip32::DerivationPath; +use key_wallet::bip32::{DerivationPath, ExtendedPubKey}; use key_wallet::signer::{Signer as KwSigner, SignerMethod}; /// Fixed-key in-memory `key_wallet::signer::Signer`. Mirrors how a @@ -77,6 +77,15 @@ impl KwSigner for FixedKeySigner { async fn public_key(&self, _path: &DerivationPath) -> Result { Ok(self.public) } + + async fn extended_public_key( + &self, + _path: &DerivationPath, + ) -> Result { + // Test stub holds a single raw key with no chain code; extended + // public key derivation is not meaningful here. + Err("FixedKeySigner: no chain code β€” extended_public_key not supported".to_string()) + } } fn make_chain_asset_lock_proof() -> AssetLockProof { From 36e2fed2289c19195a8ccc517f433321ea12be10 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Wed, 1 Jul 2026 16:15:26 +0000 Subject: [PATCH 03/21] chore(deps): bump rust-dashcore to PR #833 (Zeroize for ExtendedPrivKey) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move the 8 rust-dashcore workspace deps (dashcore, dash-spv, key-wallet, key-wallet-ffi, key-wallet-manager, dash-network, dash-network-seeds, dashcore-rpc) from rev 78d10022 to f42498e0 β€” the head of rust-dashcore PR #833, which adds `impl zeroize::Zeroize for ExtendedPrivKey` in key-wallet/src/bip32.rs. Package versions stay at 0.45.0; Cargo.lock change is rev-only. Co-Authored-By: Claude Opus 4.8 --- Cargo.lock | 24 ++++++++++++------------ Cargo.toml | 16 ++++++++-------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 512dffc0d2d..1a0a3f2d44c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1637,7 +1637,7 @@ dependencies = [ [[package]] name = "dash-network" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=78d10022c92384affe9d71be3266ac7bb4710291#78d10022c92384affe9d71be3266ac7bb4710291" +source = "git+https://github.com/dashpay/rust-dashcore?rev=f42498e0d04257e28b4e457c16629904a872ab61#f42498e0d04257e28b4e457c16629904a872ab61" dependencies = [ "bincode", "bincode_derive", @@ -1648,7 +1648,7 @@ dependencies = [ [[package]] name = "dash-network-seeds" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=78d10022c92384affe9d71be3266ac7bb4710291#78d10022c92384affe9d71be3266ac7bb4710291" +source = "git+https://github.com/dashpay/rust-dashcore?rev=f42498e0d04257e28b4e457c16629904a872ab61#f42498e0d04257e28b4e457c16629904a872ab61" dependencies = [ "dash-network", ] @@ -1725,7 +1725,7 @@ dependencies = [ [[package]] name = "dash-spv" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=78d10022c92384affe9d71be3266ac7bb4710291#78d10022c92384affe9d71be3266ac7bb4710291" +source = "git+https://github.com/dashpay/rust-dashcore?rev=f42498e0d04257e28b4e457c16629904a872ab61#f42498e0d04257e28b4e457c16629904a872ab61" dependencies = [ "async-trait", "chrono", @@ -1754,7 +1754,7 @@ dependencies = [ [[package]] name = "dashcore" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=78d10022c92384affe9d71be3266ac7bb4710291#78d10022c92384affe9d71be3266ac7bb4710291" +source = "git+https://github.com/dashpay/rust-dashcore?rev=f42498e0d04257e28b4e457c16629904a872ab61#f42498e0d04257e28b4e457c16629904a872ab61" dependencies = [ "anyhow", "base64-compat", @@ -1780,12 +1780,12 @@ dependencies = [ [[package]] name = "dashcore-private" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=78d10022c92384affe9d71be3266ac7bb4710291#78d10022c92384affe9d71be3266ac7bb4710291" +source = "git+https://github.com/dashpay/rust-dashcore?rev=f42498e0d04257e28b4e457c16629904a872ab61#f42498e0d04257e28b4e457c16629904a872ab61" [[package]] name = "dashcore-rpc" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=78d10022c92384affe9d71be3266ac7bb4710291#78d10022c92384affe9d71be3266ac7bb4710291" +source = "git+https://github.com/dashpay/rust-dashcore?rev=f42498e0d04257e28b4e457c16629904a872ab61#f42498e0d04257e28b4e457c16629904a872ab61" dependencies = [ "dashcore-rpc-json", "hex", @@ -1798,7 +1798,7 @@ dependencies = [ [[package]] name = "dashcore-rpc-json" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=78d10022c92384affe9d71be3266ac7bb4710291#78d10022c92384affe9d71be3266ac7bb4710291" +source = "git+https://github.com/dashpay/rust-dashcore?rev=f42498e0d04257e28b4e457c16629904a872ab61#f42498e0d04257e28b4e457c16629904a872ab61" dependencies = [ "bincode", "dashcore", @@ -1813,7 +1813,7 @@ dependencies = [ [[package]] name = "dashcore_hashes" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=78d10022c92384affe9d71be3266ac7bb4710291#78d10022c92384affe9d71be3266ac7bb4710291" +source = "git+https://github.com/dashpay/rust-dashcore?rev=f42498e0d04257e28b4e457c16629904a872ab61#f42498e0d04257e28b4e457c16629904a872ab61" dependencies = [ "bincode", "dashcore-private", @@ -2867,7 +2867,7 @@ dependencies = [ [[package]] name = "git-state" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=78d10022c92384affe9d71be3266ac7bb4710291#78d10022c92384affe9d71be3266ac7bb4710291" +source = "git+https://github.com/dashpay/rust-dashcore?rev=f42498e0d04257e28b4e457c16629904a872ab61#f42498e0d04257e28b4e457c16629904a872ab61" [[package]] name = "glob" @@ -4034,7 +4034,7 @@ dependencies = [ [[package]] name = "key-wallet" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=78d10022c92384affe9d71be3266ac7bb4710291#78d10022c92384affe9d71be3266ac7bb4710291" +source = "git+https://github.com/dashpay/rust-dashcore?rev=f42498e0d04257e28b4e457c16629904a872ab61#f42498e0d04257e28b4e457c16629904a872ab61" dependencies = [ "aes", "async-trait", @@ -4063,7 +4063,7 @@ dependencies = [ [[package]] name = "key-wallet-ffi" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=78d10022c92384affe9d71be3266ac7bb4710291#78d10022c92384affe9d71be3266ac7bb4710291" +source = "git+https://github.com/dashpay/rust-dashcore?rev=f42498e0d04257e28b4e457c16629904a872ab61#f42498e0d04257e28b4e457c16629904a872ab61" dependencies = [ "cbindgen 0.29.4", "dash-network", @@ -4079,7 +4079,7 @@ dependencies = [ [[package]] name = "key-wallet-manager" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=78d10022c92384affe9d71be3266ac7bb4710291#78d10022c92384affe9d71be3266ac7bb4710291" +source = "git+https://github.com/dashpay/rust-dashcore?rev=f42498e0d04257e28b4e457c16629904a872ab61#f42498e0d04257e28b4e457c16629904a872ab61" dependencies = [ "async-trait", "bincode", diff --git a/Cargo.toml b/Cargo.toml index c36a7a9c874..3561c08beb9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -50,14 +50,14 @@ members = [ ] [workspace.dependencies] -dashcore = { git = "https://github.com/dashpay/rust-dashcore", rev = "78d10022c92384affe9d71be3266ac7bb4710291" } -dash-network-seeds = { git = "https://github.com/dashpay/rust-dashcore", rev = "78d10022c92384affe9d71be3266ac7bb4710291" } -dash-spv = { git = "https://github.com/dashpay/rust-dashcore", rev = "78d10022c92384affe9d71be3266ac7bb4710291" } -key-wallet = { git = "https://github.com/dashpay/rust-dashcore", rev = "78d10022c92384affe9d71be3266ac7bb4710291" } -key-wallet-ffi = { git = "https://github.com/dashpay/rust-dashcore", rev = "78d10022c92384affe9d71be3266ac7bb4710291" } -key-wallet-manager = { git = "https://github.com/dashpay/rust-dashcore", rev = "78d10022c92384affe9d71be3266ac7bb4710291" } -dash-network = { git = "https://github.com/dashpay/rust-dashcore", rev = "78d10022c92384affe9d71be3266ac7bb4710291" } -dashcore-rpc = { git = "https://github.com/dashpay/rust-dashcore", rev = "78d10022c92384affe9d71be3266ac7bb4710291" } +dashcore = { git = "https://github.com/dashpay/rust-dashcore", rev = "f42498e0d04257e28b4e457c16629904a872ab61" } +dash-network-seeds = { git = "https://github.com/dashpay/rust-dashcore", rev = "f42498e0d04257e28b4e457c16629904a872ab61" } +dash-spv = { git = "https://github.com/dashpay/rust-dashcore", rev = "f42498e0d04257e28b4e457c16629904a872ab61" } +key-wallet = { git = "https://github.com/dashpay/rust-dashcore", rev = "f42498e0d04257e28b4e457c16629904a872ab61" } +key-wallet-ffi = { git = "https://github.com/dashpay/rust-dashcore", rev = "f42498e0d04257e28b4e457c16629904a872ab61" } +key-wallet-manager = { git = "https://github.com/dashpay/rust-dashcore", rev = "f42498e0d04257e28b4e457c16629904a872ab61" } +dash-network = { git = "https://github.com/dashpay/rust-dashcore", rev = "f42498e0d04257e28b4e457c16629904a872ab61" } +dashcore-rpc = { git = "https://github.com/dashpay/rust-dashcore", rev = "f42498e0d04257e28b4e457c16629904a872ab61" } tokio-metrics = "0.5" From 3be6885df8ed67a73e24ae8da7aeb63fb6f18b8c Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Wed, 1 Jul 2026 16:15:37 +0000 Subject: [PATCH 04/21] refactor(rs-sdk-ffi): wrap ExtendedPrivKey in Zeroizing, drop manual erase MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that key-wallet's ExtendedPrivKey implements Zeroize (rust-dashcore PR #833), hold the master and derived extended keys in Zeroizing<_> and let RAII wipe them on every exit path β€” success, ?-early-return, and panic-unwind. This removes the 4 manual `.private_key.non_secure_erase()` calls in resolve_and_derive / derive_priv / extended_public_key that only covered the paths they were hand-placed on. resolve_and_derive now returns Zeroizing; its two callers are updated. The two `secret.non_secure_erase()` calls in sign_ecdsa / public_key stay: secp256k1::SecretKey has no Zeroize impl, so those raw SecretKey copies still need an explicit wipe. Module and per-fn zeroization docs rewritten to describe the current Zeroizing approach. Co-Authored-By: Claude Opus 4.8 --- .../src/mnemonic_resolver_core_signer.rs | 104 +++++++----------- 1 file changed, 40 insertions(+), 64 deletions(-) diff --git a/packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs b/packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs index bda1d3aa00f..6c4f73ab924 100644 --- a/packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs +++ b/packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs @@ -42,20 +42,20 @@ //! method returns. Two mechanisms cover the different ownership //! shapes: //! -//! - **`Zeroizing` wrappers** scrub on `Drop` for the byte-buffer -//! intermediates: the resolver mnemonic buffer, the BIP-39 seed, -//! and the final derived 32-byte scalar. -//! - **Explicit `non_secure_erase` calls** scrub the -//! [`secp256k1::SecretKey`] scalars inside the two intermediate +//! - **`Zeroizing` wrappers** scrub on `Drop`. This covers the +//! byte-buffer intermediates (resolver mnemonic buffer, BIP-39 seed, +//! final derived 32-byte scalar) and the two intermediate //! [`ExtendedPrivKey`] values (master + derived). `ExtendedPrivKey` -//! has no `Drop` / `Zeroize` impl in `key-wallet`, so falling out -//! of scope alone would leave those scalars resident; the explicit -//! wipe at the bottom of `derive_priv` closes the gap. Same -//! defense is applied at the sign-site for the `SecretKey` copy -//! `from_slice` creates. A proper fix is a `Zeroize` / -//! `ZeroizeOnDrop` impl in `dashpay/rust-dashcore`'s -//! `key-wallet/src/bip32.rs`; until that ships, the local wipes -//! keep the no-residue invariant true. +//! is `Copy` with no `Drop`, so the wipe fires through its +//! `Zeroizing` wrapper on every exit path β€” success, `?`-early-return, +//! and panic-unwind. `ExtendedPrivKey: Zeroize` comes from +//! rust-dashcore PR #833 (rev +//! `f42498e0d04257e28b4e457c16629904a872ab61`). +//! - **Explicit `non_secure_erase` calls** scrub the raw +//! [`secp256k1::SecretKey`] copies at the two sign sites, where the +//! scalar comes back out of `SecretKey::from_slice`. `SecretKey` has +//! no `Zeroize` impl (only `non_secure_erase()`), so it can't ride a +//! `Zeroizing` wrapper. //! //! Combined, no private key bytes survive past the trait-method //! boundary. @@ -232,12 +232,12 @@ impl MnemonicResolverCoreSigner { /// /// # Zeroization contract /// - /// - The `master` scalar is wiped inside this helper before returning. - /// - The *caller* is responsible for wiping `derived.private_key` after - /// extracting whatever it needs β€” `ExtendedPrivKey` carries no - /// `Drop`-based zeroization (see the `TODO(upstream)` below). - /// - All other intermediate buffers (mnemonic, seed) are `Zeroizing`- - /// wrapped and wiped on drop before this method returns. + /// Both the `master` and returned `derived` extended keys are held in + /// [`Zeroizing`], so every `ExtendedPrivKey` scalar is wiped on drop: + /// `master` when this helper returns, `derived` when the caller's + /// binding drops. The mnemonic and seed buffers are likewise + /// `Zeroizing`-wrapped. `ExtendedPrivKey: Zeroize` comes from + /// rust-dashcore PR #833 (rev `f42498e0d04257e28b4e457c16629904a872ab61`). /// /// # Errors /// @@ -247,7 +247,7 @@ impl MnemonicResolverCoreSigner { fn resolve_and_derive( &self, path: &DerivationPath, - ) -> Result { + ) -> Result, MnemonicResolverSignerError> { if self.resolver_addr == 0 { return Err(MnemonicResolverSignerError::NullHandle); } @@ -305,56 +305,33 @@ impl MnemonicResolverCoreSigner { drop(mnemonic); let secp = Secp256k1::new(); - let mut master = ExtendedPrivKey::new_master(self.network, seed.as_ref()) - .map_err(|e| MnemonicResolverSignerError::DerivationFailed(format!("master: {e}")))?; - // TODO(upstream): `key_wallet::bip32::ExtendedPrivKey` has no - // `Drop` / `Zeroize` impl β€” the inner `secp256k1::SecretKey` - // scalar on `master` would otherwise drop un-wiped. The `derived` - // scalar is the caller's responsibility (see zeroization contract - // above). Proper fix is a `Zeroize` / `ZeroizeOnDrop` impl in - // `dashpay/rust-dashcore`'s `key-wallet/src/bip32.rs`; until - // that lands, wipe the master scalar explicitly on BOTH paths: - // the error arm below (early return) and the success path after - // the match. Mirrored in the sibling FFI at - // `rs-platform-wallet-ffi/src/sign_with_mnemonic_resolver.rs`. - let derived = match master.derive_priv(&secp, path) { - Ok(d) => d, - Err(e) => { - // Wipe master before returning β€” the success path below - // never runs, so line 323's erase would be skipped without - // this explicit call. - master.private_key.non_secure_erase(); - return Err(MnemonicResolverSignerError::DerivationFailed(format!( - "path: {e}" - ))); - } - }; - - // Success path: master scalar wiped here; derived is the caller's - // responsibility (documented in the zeroization contract above). - master.private_key.non_secure_erase(); + let master = Zeroizing::new( + ExtendedPrivKey::new_master(self.network, seed.as_ref()).map_err(|e| { + MnemonicResolverSignerError::DerivationFailed(format!("master: {e}")) + })?, + ); + let derived = master + .derive_priv(&secp, path) + .map_err(|e| MnemonicResolverSignerError::DerivationFailed(format!("path: {e}")))?; - Ok(derived) + Ok(Zeroizing::new(derived)) } /// Resolve the mnemonic and derive the raw 32-byte scalar at `path`. /// /// Returns the scalar in a `Zeroizing` wrapper so the caller's last - /// drop point wipes it. All other intermediate key material is zeroed - /// before this method returns (see [`Self::resolve_and_derive`]). + /// drop point wipes it. All other intermediate key material β€” the two + /// [`ExtendedPrivKey`] values, mnemonic, and seed β€” is `Zeroizing`- + /// wrapped and wiped before this method returns (see + /// [`Self::resolve_and_derive`]). fn derive_priv( &self, path: &DerivationPath, ) -> Result, MnemonicResolverSignerError> { - let mut derived = self.resolve_and_derive(path)?; - // `secret_bytes()` copies the 32-byte scalar; wrap in `Zeroizing` - // so the caller's drop point wipes it. - let bytes = Zeroizing::new(derived.private_key.secret_bytes()); - // Wipe the derived extended key's scalar β€” the extracted `bytes` - // carry it from here. Mirrors the SecretKey-copy hole noted in - // `resolve_and_derive`'s TODO(upstream). - derived.private_key.non_secure_erase(); - Ok(bytes) + let derived = self.resolve_and_derive(path)?; + // `secret_bytes()` copies the 32-byte scalar out of the `Zeroizing`- + // wrapped `derived`, which wipes on drop at end of scope. + Ok(Zeroizing::new(derived.private_key.secret_bytes())) } } @@ -415,12 +392,11 @@ impl Signer for MnemonicResolverCoreSigner { &self, path: &DerivationPath, ) -> Result { - let mut derived = self.resolve_and_derive(path)?; + let derived = self.resolve_and_derive(path)?; let secp = Secp256k1::new(); - // Capture the extended public key *before* wiping the private scalar. - // `ExtendedPubKey` carries only public material (chain code + point). + // `ExtendedPubKey` carries only public material (chain code + point); + // `derived` wipes its private scalar on drop at end of scope. let xpub = ExtendedPubKey::from_priv(&secp, &derived); - derived.private_key.non_secure_erase(); Ok(xpub) } } From b175d5083233f80a6d6e1be63f05857e5074aa44 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Wed, 1 Jul 2026 16:40:03 +0000 Subject: [PATCH 05/21] refactor(rs-sdk-ffi): confine ExtendedPrivKey to resolve_and_derive via extract closure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resolve_and_derive now takes `extract: impl FnOnce(&ExtendedPrivKey) -> T` and returns T, instead of handing back Zeroizing. The derived key is wrapped in Zeroizing the instant derive_priv returns and is only ever borrowed by the closure, so no raw or wrapped ExtendedPrivKey crosses the function boundary β€” closing the Copy stack-residue gap flagged in review. The one remaining by-value copy is derive_priv's own return slot, which is upstream's (key-wallet) API to own. derive_priv and extended_public_key become thin closure callers. Also add a #[tokio::test] driving extended_public_key against an independently derived ExtendedPubKey from the same BIP-39 vector / network / path, asserting full-struct equality plus explicit chain_code / depth / network spot-checks so metadata loss is caught, not just the public point (fixes codecov/patch gap). Co-Authored-By: Claude Opus 4.8 --- .../src/mnemonic_resolver_core_signer.rs | 91 +++++++++++++------ 1 file changed, 65 insertions(+), 26 deletions(-) diff --git a/packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs b/packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs index 6c4f73ab924..1078802c248 100644 --- a/packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs +++ b/packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs @@ -222,8 +222,9 @@ impl MnemonicResolverCoreSigner { } } - /// Resolve the mnemonic from the Swift-side callback, then derive the - /// BIP-32 extended private key at `path`. + /// Resolve the mnemonic from the Swift-side callback, derive the BIP-32 + /// extended private key at `path`, and hand it *by reference* to + /// `extract`, returning whatever `extract` produces. /// /// This is the single entry-point for all private-key material in this /// signer. It handles the full stack: resolver FFI call β†’ result-code @@ -232,22 +233,25 @@ impl MnemonicResolverCoreSigner { /// /// # Zeroization contract /// - /// Both the `master` and returned `derived` extended keys are held in - /// [`Zeroizing`], so every `ExtendedPrivKey` scalar is wiped on drop: - /// `master` when this helper returns, `derived` when the caller's - /// binding drops. The mnemonic and seed buffers are likewise - /// `Zeroizing`-wrapped. `ExtendedPrivKey: Zeroize` comes from - /// rust-dashcore PR #833 (rev `f42498e0d04257e28b4e457c16629904a872ab61`). + /// Both the `master` and `derived` extended keys are held in [`Zeroizing`] + /// and wiped when this method returns. The `ExtendedPrivKey` never crosses + /// the call boundary β€” `extract` only borrows it β€” so no raw or wrapped + /// key can outlive the derivation. `extract` returns public material + /// (`ExtendedPubKey`) or a `Zeroizing` scalar copy; the caller wipes the + /// latter on its own drop. The mnemonic and seed buffers are likewise + /// `Zeroizing`-wrapped. `ExtendedPrivKey: Zeroize` comes from rust-dashcore + /// PR #833 (rev `f42498e0d04257e28b4e457c16629904a872ab61`). /// /// # Errors /// /// Propagates [`MnemonicResolverSignerError`] for every failure mode: /// null handle, resolver FFI errors, encoding/parse failures, and BIP-32 /// derivation errors. - fn resolve_and_derive( + fn resolve_and_derive( &self, path: &DerivationPath, - ) -> Result, MnemonicResolverSignerError> { + extract: impl FnOnce(&ExtendedPrivKey) -> T, + ) -> Result { if self.resolver_addr == 0 { return Err(MnemonicResolverSignerError::NullHandle); } @@ -310,28 +314,29 @@ impl MnemonicResolverCoreSigner { MnemonicResolverSignerError::DerivationFailed(format!("master: {e}")) })?, ); - let derived = master - .derive_priv(&secp, path) - .map_err(|e| MnemonicResolverSignerError::DerivationFailed(format!("path: {e}")))?; + let derived = + Zeroizing::new(master.derive_priv(&secp, path).map_err(|e| { + MnemonicResolverSignerError::DerivationFailed(format!("path: {e}")) + })?); - Ok(Zeroizing::new(derived)) + Ok(extract(&derived)) } /// Resolve the mnemonic and derive the raw 32-byte scalar at `path`. /// /// Returns the scalar in a `Zeroizing` wrapper so the caller's last - /// drop point wipes it. All other intermediate key material β€” the two - /// [`ExtendedPrivKey`] values, mnemonic, and seed β€” is `Zeroizing`- - /// wrapped and wiped before this method returns (see - /// [`Self::resolve_and_derive`]). + /// drop point wipes it. The intermediate `ExtendedPrivKey` values, + /// mnemonic, and seed are wiped inside [`Self::resolve_and_derive`] + /// before this returns. fn derive_priv( &self, path: &DerivationPath, ) -> Result, MnemonicResolverSignerError> { - let derived = self.resolve_and_derive(path)?; - // `secret_bytes()` copies the 32-byte scalar out of the `Zeroizing`- - // wrapped `derived`, which wipes on drop at end of scope. - Ok(Zeroizing::new(derived.private_key.secret_bytes())) + // `secret_bytes()` copies the scalar out of the borrowed key; the + // `ExtendedPrivKey` itself never leaves `resolve_and_derive`. + self.resolve_and_derive(path, |derived| { + Zeroizing::new(derived.private_key.secret_bytes()) + }) } } @@ -392,12 +397,10 @@ impl Signer for MnemonicResolverCoreSigner { &self, path: &DerivationPath, ) -> Result { - let derived = self.resolve_and_derive(path)?; let secp = Secp256k1::new(); // `ExtendedPubKey` carries only public material (chain code + point); - // `derived` wipes its private scalar on drop at end of scope. - let xpub = ExtendedPubKey::from_priv(&secp, &derived); - Ok(xpub) + // the borrowed private key never leaves `resolve_and_derive`. + self.resolve_and_derive(path, |derived| ExtendedPubKey::from_priv(&secp, derived)) } } @@ -493,6 +496,42 @@ mod tests { unsafe { dash_sdk_mnemonic_resolver_destroy(resolver) }; } + #[tokio::test] + async fn extended_public_key_matches_independent_derivation() { + let resolver = make_resolver(english_resolve); + let signer = + unsafe { MnemonicResolverCoreSigner::new(resolver, [0u8; 32], Network::Testnet) }; + + let path = test_path(); + let xpub = signer + .extended_public_key(&path) + .await + .expect("extended_public_key succeeds"); + + // Independently derive the expected xpub straight from the known + // BIP-39 vector β€” same network + path, no resolver in the loop. + let secp = Secp256k1::new(); + let mnemonic = parse_mnemonic_any_language(ENGLISH_PHRASE).expect("valid phrase"); + let master = ExtendedPrivKey::new_master(Network::Testnet, &mnemonic.to_seed("")) + .expect("master derivation"); + let derived = master.derive_priv(&secp, &path).expect("path derivation"); + let expected = ExtendedPubKey::from_priv(&secp, &derived); + + // Full-struct equality catches any field regression; the explicit + // metadata asserts keep the guarantee even if `ExtendedPubKey`'s + // `PartialEq` ever narrows, and document that chain code / depth / + // network are deliberately covered β€” not just the public point. + assert_eq!(xpub, expected, "xpub must match independent derivation"); + assert_eq!( + xpub.chain_code, expected.chain_code, + "chain code must match" + ); + assert_eq!(xpub.depth, expected.depth, "depth must match"); + assert_eq!(xpub.network, expected.network, "network must match"); + + unsafe { dash_sdk_mnemonic_resolver_destroy(resolver) }; + } + #[tokio::test] async fn missing_resolver_surfaces_not_found_error() { let resolver = make_resolver(missing_resolve); From 5a2b2b5019ba67a8156b3c9de0aafd258196dfc8 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Wed, 1 Jul 2026 16:47:29 +0000 Subject: [PATCH 06/21] test(rs-sdk-ffi): make extended_public_key metadata asserts load-bearing; doc irreducible xpriv transient MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reorder the extended_public_key test so each field-level assert (public_key, chain_code, depth, network) runs before the full-struct assert_eq!, which otherwise short-circuits and leaves the per-field metadata checks unreachable on a regression (i.e. vacuous). Proven non-vacuous: temporarily corrupting only `depth` in the impl fails the test at "depth must match", where a pubkey-only check would have passed. Also document the one irreducible transient in resolve_and_derive: key-wallet's ExtendedPrivKey::derive_priv returns by value (Copy), so its return slot holds an un-wiped copy until the same-line Zeroizing::new takes ownership β€” noted so the zeroization contract doesn't over-promise "zero copies ever". Co-Authored-By: Claude Opus 4.8 --- .../src/mnemonic_resolver_core_signer.rs | 26 +++++++++++++++---- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs b/packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs index 1078802c248..80bc8b3b891 100644 --- a/packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs +++ b/packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs @@ -242,6 +242,13 @@ impl MnemonicResolverCoreSigner { /// `Zeroizing`-wrapped. `ExtendedPrivKey: Zeroize` comes from rust-dashcore /// PR #833 (rev `f42498e0d04257e28b4e457c16629904a872ab61`). /// + /// One transient is irreducible: `ExtendedPrivKey::derive_priv` returns the + /// child key *by value* (`ExtendedPrivKey` is `Copy`), so its return slot + /// briefly holds an un-wiped copy until the same-line `Zeroizing::new` + /// takes ownership. Closing that would require an upstream signature change + /// in key-wallet; it is not "zero copies ever", just the tightest this + /// layer can guarantee. + /// /// # Errors /// /// Propagates [`MnemonicResolverSignerError`] for every failure mode: @@ -517,17 +524,26 @@ mod tests { let derived = master.derive_priv(&secp, &path).expect("path derivation"); let expected = ExtendedPubKey::from_priv(&secp, &derived); - // Full-struct equality catches any field regression; the explicit - // metadata asserts keep the guarantee even if `ExtendedPubKey`'s - // `PartialEq` ever narrows, and document that chain code / depth / - // network are deliberately covered β€” not just the public point. - assert_eq!(xpub, expected, "xpub must match independent derivation"); + // Field-level checks run first so a silently-dropped BIP-32 metadatum + // fails here with a precise message β€” not just the public point. The + // final full-struct assert then catches the remaining fields + // (parent_fingerprint, child_number). Ordering matters: a leading + // full-struct `assert_eq!` would short-circuit and make these + // per-field asserts unreachable (i.e. vacuous) on a metadata regression. + assert_eq!( + xpub.public_key, expected.public_key, + "public key must match" + ); assert_eq!( xpub.chain_code, expected.chain_code, "chain code must match" ); assert_eq!(xpub.depth, expected.depth, "depth must match"); assert_eq!(xpub.network, expected.network, "network must match"); + assert_eq!( + xpub, expected, + "full xpub must match independent derivation" + ); unsafe { dash_sdk_mnemonic_resolver_destroy(resolver) }; } From 56612fd529d40268d4f290232f62bab312c2d950 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Wed, 1 Jul 2026 20:48:28 +0000 Subject: [PATCH 07/21] chore(deps): bump rust-dashcore to a8c57fe (drop Copy, zeroize ExtendedPrivKey on Drop) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Advances the 8 rust-dashcore workspace deps from rev f42498e0d04257e28b4e457c16629904a872ab61 to a8c57fe863c96ac9c7e33833549e7a4f75ac9b5e (PR #833 head). The new commit drops `Copy` from `ExtendedPrivKey` and gives it a `Drop` impl that zeroizes its secret material on scope exit β€” so the key wipes itself automatically instead of relying on caller-side wrappers, and non-`Copy` moves leave no stray bitwise duplicate behind. No workspace call sites relied on `ExtendedPrivKey: Copy`: the Copy-removal impact is absorbed upstream in `bip32::derive_priv` (clones `self` instead of `*self`), so `cargo check --workspace --all-targets` is clean with no source changes. Co-Authored-By: Claude Opus 4.8 --- Cargo.lock | 24 ++++++++++++------------ Cargo.toml | 16 ++++++++-------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1a0a3f2d44c..6a71df9fafd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1637,7 +1637,7 @@ dependencies = [ [[package]] name = "dash-network" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=f42498e0d04257e28b4e457c16629904a872ab61#f42498e0d04257e28b4e457c16629904a872ab61" +source = "git+https://github.com/dashpay/rust-dashcore?rev=a8c57fe863c96ac9c7e33833549e7a4f75ac9b5e#a8c57fe863c96ac9c7e33833549e7a4f75ac9b5e" dependencies = [ "bincode", "bincode_derive", @@ -1648,7 +1648,7 @@ dependencies = [ [[package]] name = "dash-network-seeds" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=f42498e0d04257e28b4e457c16629904a872ab61#f42498e0d04257e28b4e457c16629904a872ab61" +source = "git+https://github.com/dashpay/rust-dashcore?rev=a8c57fe863c96ac9c7e33833549e7a4f75ac9b5e#a8c57fe863c96ac9c7e33833549e7a4f75ac9b5e" dependencies = [ "dash-network", ] @@ -1725,7 +1725,7 @@ dependencies = [ [[package]] name = "dash-spv" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=f42498e0d04257e28b4e457c16629904a872ab61#f42498e0d04257e28b4e457c16629904a872ab61" +source = "git+https://github.com/dashpay/rust-dashcore?rev=a8c57fe863c96ac9c7e33833549e7a4f75ac9b5e#a8c57fe863c96ac9c7e33833549e7a4f75ac9b5e" dependencies = [ "async-trait", "chrono", @@ -1754,7 +1754,7 @@ dependencies = [ [[package]] name = "dashcore" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=f42498e0d04257e28b4e457c16629904a872ab61#f42498e0d04257e28b4e457c16629904a872ab61" +source = "git+https://github.com/dashpay/rust-dashcore?rev=a8c57fe863c96ac9c7e33833549e7a4f75ac9b5e#a8c57fe863c96ac9c7e33833549e7a4f75ac9b5e" dependencies = [ "anyhow", "base64-compat", @@ -1780,12 +1780,12 @@ dependencies = [ [[package]] name = "dashcore-private" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=f42498e0d04257e28b4e457c16629904a872ab61#f42498e0d04257e28b4e457c16629904a872ab61" +source = "git+https://github.com/dashpay/rust-dashcore?rev=a8c57fe863c96ac9c7e33833549e7a4f75ac9b5e#a8c57fe863c96ac9c7e33833549e7a4f75ac9b5e" [[package]] name = "dashcore-rpc" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=f42498e0d04257e28b4e457c16629904a872ab61#f42498e0d04257e28b4e457c16629904a872ab61" +source = "git+https://github.com/dashpay/rust-dashcore?rev=a8c57fe863c96ac9c7e33833549e7a4f75ac9b5e#a8c57fe863c96ac9c7e33833549e7a4f75ac9b5e" dependencies = [ "dashcore-rpc-json", "hex", @@ -1798,7 +1798,7 @@ dependencies = [ [[package]] name = "dashcore-rpc-json" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=f42498e0d04257e28b4e457c16629904a872ab61#f42498e0d04257e28b4e457c16629904a872ab61" +source = "git+https://github.com/dashpay/rust-dashcore?rev=a8c57fe863c96ac9c7e33833549e7a4f75ac9b5e#a8c57fe863c96ac9c7e33833549e7a4f75ac9b5e" dependencies = [ "bincode", "dashcore", @@ -1813,7 +1813,7 @@ dependencies = [ [[package]] name = "dashcore_hashes" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=f42498e0d04257e28b4e457c16629904a872ab61#f42498e0d04257e28b4e457c16629904a872ab61" +source = "git+https://github.com/dashpay/rust-dashcore?rev=a8c57fe863c96ac9c7e33833549e7a4f75ac9b5e#a8c57fe863c96ac9c7e33833549e7a4f75ac9b5e" dependencies = [ "bincode", "dashcore-private", @@ -2867,7 +2867,7 @@ dependencies = [ [[package]] name = "git-state" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=f42498e0d04257e28b4e457c16629904a872ab61#f42498e0d04257e28b4e457c16629904a872ab61" +source = "git+https://github.com/dashpay/rust-dashcore?rev=a8c57fe863c96ac9c7e33833549e7a4f75ac9b5e#a8c57fe863c96ac9c7e33833549e7a4f75ac9b5e" [[package]] name = "glob" @@ -4034,7 +4034,7 @@ dependencies = [ [[package]] name = "key-wallet" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=f42498e0d04257e28b4e457c16629904a872ab61#f42498e0d04257e28b4e457c16629904a872ab61" +source = "git+https://github.com/dashpay/rust-dashcore?rev=a8c57fe863c96ac9c7e33833549e7a4f75ac9b5e#a8c57fe863c96ac9c7e33833549e7a4f75ac9b5e" dependencies = [ "aes", "async-trait", @@ -4063,7 +4063,7 @@ dependencies = [ [[package]] name = "key-wallet-ffi" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=f42498e0d04257e28b4e457c16629904a872ab61#f42498e0d04257e28b4e457c16629904a872ab61" +source = "git+https://github.com/dashpay/rust-dashcore?rev=a8c57fe863c96ac9c7e33833549e7a4f75ac9b5e#a8c57fe863c96ac9c7e33833549e7a4f75ac9b5e" dependencies = [ "cbindgen 0.29.4", "dash-network", @@ -4079,7 +4079,7 @@ dependencies = [ [[package]] name = "key-wallet-manager" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=f42498e0d04257e28b4e457c16629904a872ab61#f42498e0d04257e28b4e457c16629904a872ab61" +source = "git+https://github.com/dashpay/rust-dashcore?rev=a8c57fe863c96ac9c7e33833549e7a4f75ac9b5e#a8c57fe863c96ac9c7e33833549e7a4f75ac9b5e" dependencies = [ "async-trait", "bincode", diff --git a/Cargo.toml b/Cargo.toml index 3561c08beb9..746d23ddeb2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -50,14 +50,14 @@ members = [ ] [workspace.dependencies] -dashcore = { git = "https://github.com/dashpay/rust-dashcore", rev = "f42498e0d04257e28b4e457c16629904a872ab61" } -dash-network-seeds = { git = "https://github.com/dashpay/rust-dashcore", rev = "f42498e0d04257e28b4e457c16629904a872ab61" } -dash-spv = { git = "https://github.com/dashpay/rust-dashcore", rev = "f42498e0d04257e28b4e457c16629904a872ab61" } -key-wallet = { git = "https://github.com/dashpay/rust-dashcore", rev = "f42498e0d04257e28b4e457c16629904a872ab61" } -key-wallet-ffi = { git = "https://github.com/dashpay/rust-dashcore", rev = "f42498e0d04257e28b4e457c16629904a872ab61" } -key-wallet-manager = { git = "https://github.com/dashpay/rust-dashcore", rev = "f42498e0d04257e28b4e457c16629904a872ab61" } -dash-network = { git = "https://github.com/dashpay/rust-dashcore", rev = "f42498e0d04257e28b4e457c16629904a872ab61" } -dashcore-rpc = { git = "https://github.com/dashpay/rust-dashcore", rev = "f42498e0d04257e28b4e457c16629904a872ab61" } +dashcore = { git = "https://github.com/dashpay/rust-dashcore", rev = "a8c57fe863c96ac9c7e33833549e7a4f75ac9b5e" } +dash-network-seeds = { git = "https://github.com/dashpay/rust-dashcore", rev = "a8c57fe863c96ac9c7e33833549e7a4f75ac9b5e" } +dash-spv = { git = "https://github.com/dashpay/rust-dashcore", rev = "a8c57fe863c96ac9c7e33833549e7a4f75ac9b5e" } +key-wallet = { git = "https://github.com/dashpay/rust-dashcore", rev = "a8c57fe863c96ac9c7e33833549e7a4f75ac9b5e" } +key-wallet-ffi = { git = "https://github.com/dashpay/rust-dashcore", rev = "a8c57fe863c96ac9c7e33833549e7a4f75ac9b5e" } +key-wallet-manager = { git = "https://github.com/dashpay/rust-dashcore", rev = "a8c57fe863c96ac9c7e33833549e7a4f75ac9b5e" } +dash-network = { git = "https://github.com/dashpay/rust-dashcore", rev = "a8c57fe863c96ac9c7e33833549e7a4f75ac9b5e" } +dashcore-rpc = { git = "https://github.com/dashpay/rust-dashcore", rev = "a8c57fe863c96ac9c7e33833549e7a4f75ac9b5e" } tokio-metrics = "0.5" From 374df98771712ec3858dde230762fbe689f22308 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Wed, 1 Jul 2026 20:48:43 +0000 Subject: [PATCH 08/21] refactor(rs-sdk-ffi): rely on ExtendedPrivKey Drop, drop redundant Zeroizing wrap `ExtendedPrivKey` now zeroizes on `Drop` (rust-dashcore rev a8c57fe863c96ac9c7e33833549e7a4f75ac9b5e), so wrapping the master and derived keys in `Zeroizing` inside `resolve_and_derive` only bought a harmless double-wipe. Unwrap both and let the type's own `Drop` do the work; `Zeroizing` stays on the plain byte buffers that have no `Drop` of their own (mnemonic buffer, BIP-39 seed, final 32-byte scalar). Rewrites the module `# Zeroization` block and the `resolve_and_derive` contract to the present three-mechanism reality (self-wiping key + Zeroizing buffers + non_secure_erase on SecretKey). The previously documented "irreducible transient" caveat is fully closed: a non-`Copy` move leaves no bitwise duplicate, so there is no un-wiped return slot. Co-Authored-By: Claude Opus 4.8 --- .../src/mnemonic_resolver_core_signer.rs | 57 ++++++++----------- 1 file changed, 24 insertions(+), 33 deletions(-) diff --git a/packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs b/packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs index 80bc8b3b891..97ce91cd0a3 100644 --- a/packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs +++ b/packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs @@ -39,18 +39,18 @@ //! # Zeroization //! //! Every intermediate that carries key material is wiped before the -//! method returns. Two mechanisms cover the different ownership +//! method returns. Three mechanisms cover the different ownership //! shapes: //! -//! - **`Zeroizing` wrappers** scrub on `Drop`. This covers the -//! byte-buffer intermediates (resolver mnemonic buffer, BIP-39 seed, -//! final derived 32-byte scalar) and the two intermediate -//! [`ExtendedPrivKey`] values (master + derived). `ExtendedPrivKey` -//! is `Copy` with no `Drop`, so the wipe fires through its -//! `Zeroizing` wrapper on every exit path β€” success, `?`-early-return, -//! and panic-unwind. `ExtendedPrivKey: Zeroize` comes from -//! rust-dashcore PR #833 (rev -//! `f42498e0d04257e28b4e457c16629904a872ab61`). +//! - **[`ExtendedPrivKey`] self-wipes on `Drop`.** The master and +//! derived extended keys zero their secret material when they leave +//! scope, on every exit path β€” success, `?`-early-return, and +//! panic-unwind. The type is no longer `Copy` as of rust-dashcore rev +//! `a8c57fe863c96ac9c7e33833549e7a4f75ac9b5e`, so each move is a real +//! move that leaves no stray bitwise duplicate behind. +//! - **`Zeroizing` wrappers** scrub the plain byte buffers that carry +//! no `Drop` of their own: the resolver mnemonic buffer, the BIP-39 +//! seed, and the final derived 32-byte scalar. //! - **Explicit `non_secure_erase` calls** scrub the raw //! [`secp256k1::SecretKey`] copies at the two sign sites, where the //! scalar comes back out of `SecretKey::from_slice`. `SecretKey` has @@ -233,21 +233,16 @@ impl MnemonicResolverCoreSigner { /// /// # Zeroization contract /// - /// Both the `master` and `derived` extended keys are held in [`Zeroizing`] - /// and wiped when this method returns. The `ExtendedPrivKey` never crosses - /// the call boundary β€” `extract` only borrows it β€” so no raw or wrapped - /// key can outlive the derivation. `extract` returns public material + /// Both the `master` and `derived` extended keys wipe their secret + /// material when they leave this scope β€” [`ExtendedPrivKey`] zeroizes on + /// `Drop` as of rust-dashcore rev + /// `a8c57fe863c96ac9c7e33833549e7a4f75ac9b5e`, and is no longer `Copy`, so + /// each move is a real move that leaves no bitwise duplicate behind. The + /// key never crosses the call boundary β€” `extract` only borrows it β€” so it + /// cannot outlive the derivation. `extract` returns public material /// (`ExtendedPubKey`) or a `Zeroizing` scalar copy; the caller wipes the - /// latter on its own drop. The mnemonic and seed buffers are likewise - /// `Zeroizing`-wrapped. `ExtendedPrivKey: Zeroize` comes from rust-dashcore - /// PR #833 (rev `f42498e0d04257e28b4e457c16629904a872ab61`). - /// - /// One transient is irreducible: `ExtendedPrivKey::derive_priv` returns the - /// child key *by value* (`ExtendedPrivKey` is `Copy`), so its return slot - /// briefly holds an un-wiped copy until the same-line `Zeroizing::new` - /// takes ownership. Closing that would require an upstream signature change - /// in key-wallet; it is not "zero copies ever", just the tightest this - /// layer can guarantee. + /// latter on its own drop. The mnemonic and seed buffers are plain arrays + /// and ride [`Zeroizing`] wrappers for the same guarantee. /// /// # Errors /// @@ -316,15 +311,11 @@ impl MnemonicResolverCoreSigner { drop(mnemonic); let secp = Secp256k1::new(); - let master = Zeroizing::new( - ExtendedPrivKey::new_master(self.network, seed.as_ref()).map_err(|e| { - MnemonicResolverSignerError::DerivationFailed(format!("master: {e}")) - })?, - ); - let derived = - Zeroizing::new(master.derive_priv(&secp, path).map_err(|e| { - MnemonicResolverSignerError::DerivationFailed(format!("path: {e}")) - })?); + let master = ExtendedPrivKey::new_master(self.network, seed.as_ref()) + .map_err(|e| MnemonicResolverSignerError::DerivationFailed(format!("master: {e}")))?; + let derived = master + .derive_priv(&secp, path) + .map_err(|e| MnemonicResolverSignerError::DerivationFailed(format!("path: {e}")))?; Ok(extract(&derived)) } From 1aac44a0abfb2ca93872218d09b25640e4a38939 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Thu, 2 Jul 2026 08:29:01 +0000 Subject: [PATCH 09/21] fix(deps): stage regenerated Cargo.lock for rust-dashcore a8a0968 bump The prior merge commit updated Cargo.toml's rust-dashcore rev pins to the current dev HEAD but left Cargo.lock staged at the pre-cargo-update state (still pinned to v0.44.0/991c6ebe), an oversight caught during PR comment verification. Stage the already-regenerated lockfile so both files agree on the a8a096838b829cf5bec3c2374a23511640a0c35c pin. Co-Authored-By: Claude Opus 4.6 --- Cargo.lock | 70 +++++++++++++++++++++++++++--------------------------- 1 file changed, 35 insertions(+), 35 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5148cee60c1..c7ae962cba6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1206,7 +1206,7 @@ version = "3.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "faf9468729b8cbcea668e36183cb69d317348c2e08e994829fb56ebfdfbaac34" dependencies = [ - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] @@ -1636,8 +1636,8 @@ dependencies = [ [[package]] name = "dash-network" -version = "0.44.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=991c6ebe24d7ea8ba7d900a052b25be8c5498409#991c6ebe24d7ea8ba7d900a052b25be8c5498409" +version = "0.45.0" +source = "git+https://github.com/dashpay/rust-dashcore?rev=a8a096838b829cf5bec3c2374a23511640a0c35c#a8a096838b829cf5bec3c2374a23511640a0c35c" dependencies = [ "bincode", "bincode_derive", @@ -1647,8 +1647,8 @@ dependencies = [ [[package]] name = "dash-network-seeds" -version = "0.44.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=991c6ebe24d7ea8ba7d900a052b25be8c5498409#991c6ebe24d7ea8ba7d900a052b25be8c5498409" +version = "0.45.0" +source = "git+https://github.com/dashpay/rust-dashcore?rev=a8a096838b829cf5bec3c2374a23511640a0c35c#a8a096838b829cf5bec3c2374a23511640a0c35c" dependencies = [ "dash-network", ] @@ -1724,8 +1724,8 @@ dependencies = [ [[package]] name = "dash-spv" -version = "0.44.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=991c6ebe24d7ea8ba7d900a052b25be8c5498409#991c6ebe24d7ea8ba7d900a052b25be8c5498409" +version = "0.45.0" +source = "git+https://github.com/dashpay/rust-dashcore?rev=a8a096838b829cf5bec3c2374a23511640a0c35c#a8a096838b829cf5bec3c2374a23511640a0c35c" dependencies = [ "async-trait", "chrono", @@ -1753,8 +1753,8 @@ dependencies = [ [[package]] name = "dashcore" -version = "0.44.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=991c6ebe24d7ea8ba7d900a052b25be8c5498409#991c6ebe24d7ea8ba7d900a052b25be8c5498409" +version = "0.45.0" +source = "git+https://github.com/dashpay/rust-dashcore?rev=a8a096838b829cf5bec3c2374a23511640a0c35c#a8a096838b829cf5bec3c2374a23511640a0c35c" dependencies = [ "anyhow", "base64-compat", @@ -1779,13 +1779,13 @@ dependencies = [ [[package]] name = "dashcore-private" -version = "0.44.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=991c6ebe24d7ea8ba7d900a052b25be8c5498409#991c6ebe24d7ea8ba7d900a052b25be8c5498409" +version = "0.45.0" +source = "git+https://github.com/dashpay/rust-dashcore?rev=a8a096838b829cf5bec3c2374a23511640a0c35c#a8a096838b829cf5bec3c2374a23511640a0c35c" [[package]] name = "dashcore-rpc" -version = "0.44.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=991c6ebe24d7ea8ba7d900a052b25be8c5498409#991c6ebe24d7ea8ba7d900a052b25be8c5498409" +version = "0.45.0" +source = "git+https://github.com/dashpay/rust-dashcore?rev=a8a096838b829cf5bec3c2374a23511640a0c35c#a8a096838b829cf5bec3c2374a23511640a0c35c" dependencies = [ "dashcore-rpc-json", "hex", @@ -1797,8 +1797,8 @@ dependencies = [ [[package]] name = "dashcore-rpc-json" -version = "0.44.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=991c6ebe24d7ea8ba7d900a052b25be8c5498409#991c6ebe24d7ea8ba7d900a052b25be8c5498409" +version = "0.45.0" +source = "git+https://github.com/dashpay/rust-dashcore?rev=a8a096838b829cf5bec3c2374a23511640a0c35c#a8a096838b829cf5bec3c2374a23511640a0c35c" dependencies = [ "bincode", "dashcore", @@ -1812,8 +1812,8 @@ dependencies = [ [[package]] name = "dashcore_hashes" -version = "0.44.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=991c6ebe24d7ea8ba7d900a052b25be8c5498409#991c6ebe24d7ea8ba7d900a052b25be8c5498409" +version = "0.45.0" +source = "git+https://github.com/dashpay/rust-dashcore?rev=a8a096838b829cf5bec3c2374a23511640a0c35c#a8a096838b829cf5bec3c2374a23511640a0c35c" dependencies = [ "bincode", "dashcore-private", @@ -2428,7 +2428,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] @@ -2489,7 +2489,7 @@ checksum = "0ce92ff622d6dadf7349484f42c93271a0d49b7cc4d466a936405bacbe10aa78" dependencies = [ "cfg-if", "rustix 1.1.4", - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] @@ -2866,8 +2866,8 @@ dependencies = [ [[package]] name = "git-state" -version = "0.44.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=991c6ebe24d7ea8ba7d900a052b25be8c5498409#991c6ebe24d7ea8ba7d900a052b25be8c5498409" +version = "0.45.0" +source = "git+https://github.com/dashpay/rust-dashcore?rev=a8a096838b829cf5bec3c2374a23511640a0c35c#a8a096838b829cf5bec3c2374a23511640a0c35c" [[package]] name = "glob" @@ -3803,7 +3803,7 @@ checksum = "3640c1c38b8e4e43584d8df18be5fc6b0aa314ce6ebf51b53313d4306cca8e46" dependencies = [ "hermit-abi", "libc", - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] @@ -4033,8 +4033,8 @@ dependencies = [ [[package]] name = "key-wallet" -version = "0.44.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=991c6ebe24d7ea8ba7d900a052b25be8c5498409#991c6ebe24d7ea8ba7d900a052b25be8c5498409" +version = "0.45.0" +source = "git+https://github.com/dashpay/rust-dashcore?rev=a8a096838b829cf5bec3c2374a23511640a0c35c#a8a096838b829cf5bec3c2374a23511640a0c35c" dependencies = [ "aes", "async-trait", @@ -4062,8 +4062,8 @@ dependencies = [ [[package]] name = "key-wallet-ffi" -version = "0.44.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=991c6ebe24d7ea8ba7d900a052b25be8c5498409#991c6ebe24d7ea8ba7d900a052b25be8c5498409" +version = "0.45.0" +source = "git+https://github.com/dashpay/rust-dashcore?rev=a8a096838b829cf5bec3c2374a23511640a0c35c#a8a096838b829cf5bec3c2374a23511640a0c35c" dependencies = [ "cbindgen 0.29.4", "dash-network", @@ -4078,8 +4078,8 @@ dependencies = [ [[package]] name = "key-wallet-manager" -version = "0.44.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=991c6ebe24d7ea8ba7d900a052b25be8c5498409#991c6ebe24d7ea8ba7d900a052b25be8c5498409" +version = "0.45.0" +source = "git+https://github.com/dashpay/rust-dashcore?rev=a8a096838b829cf5bec3c2374a23511640a0c35c#a8a096838b829cf5bec3c2374a23511640a0c35c" dependencies = [ "async-trait", "bincode", @@ -4596,7 +4596,7 @@ version = "0.50.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7957b9740744892f114936ab4a57b3f487491bbeafaf8083688b16841a4240e5" dependencies = [ - "windows-sys 0.61.2", + "windows-sys 0.59.0", ] [[package]] @@ -5696,7 +5696,7 @@ dependencies = [ "once_cell", "socket2 0.5.10", "tracing", - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] @@ -6486,7 +6486,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys 0.4.15", - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] @@ -6499,7 +6499,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys 0.12.1", - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] @@ -6558,7 +6558,7 @@ dependencies = [ "security-framework", "security-framework-sys", "webpki-root-certs", - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] @@ -7418,7 +7418,7 @@ dependencies = [ "getrandom 0.4.2", "once_cell", "rustix 1.1.4", - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] @@ -8867,7 +8867,7 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" dependencies = [ - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] From defa2491495a5499c0d97c9dfe6f327d8fb4f6b5 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Thu, 2 Jul 2026 08:31:25 +0000 Subject: [PATCH 10/21] build: sync Cargo.lock to rust-dashcore a8a0968 (0.45.0) The committed lockfile still pinned all 12 rust-dashcore workspace crates at the stale rev 991c6eb (v0.44.0), even though root Cargo.toml was already re-pinned to a8a096838b829cf5bec3c2374a23511640a0c35c (v0.45.0). Cargo silently re-resolves the lock on the next build, so --locked builds (CI) would fail against the mismatch. Regenerate the lock to match: dashcore, dashcore-private, dashcore_hashes, dashcore-rpc, dashcore-rpc-json, dash-network, dash-network-seeds, dash-spv, git-state, key-wallet, key-wallet-ffi, key-wallet-manager all move 991c6eb -> a8a0968. No other crate churn. Co-Authored-By: Claude Opus 4.8 --- Cargo.lock | 48 ++++++++++++++++++++++++------------------------ 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5148cee60c1..c177bf2960a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1636,8 +1636,8 @@ dependencies = [ [[package]] name = "dash-network" -version = "0.44.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=991c6ebe24d7ea8ba7d900a052b25be8c5498409#991c6ebe24d7ea8ba7d900a052b25be8c5498409" +version = "0.45.0" +source = "git+https://github.com/dashpay/rust-dashcore?rev=a8a096838b829cf5bec3c2374a23511640a0c35c#a8a096838b829cf5bec3c2374a23511640a0c35c" dependencies = [ "bincode", "bincode_derive", @@ -1647,8 +1647,8 @@ dependencies = [ [[package]] name = "dash-network-seeds" -version = "0.44.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=991c6ebe24d7ea8ba7d900a052b25be8c5498409#991c6ebe24d7ea8ba7d900a052b25be8c5498409" +version = "0.45.0" +source = "git+https://github.com/dashpay/rust-dashcore?rev=a8a096838b829cf5bec3c2374a23511640a0c35c#a8a096838b829cf5bec3c2374a23511640a0c35c" dependencies = [ "dash-network", ] @@ -1724,8 +1724,8 @@ dependencies = [ [[package]] name = "dash-spv" -version = "0.44.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=991c6ebe24d7ea8ba7d900a052b25be8c5498409#991c6ebe24d7ea8ba7d900a052b25be8c5498409" +version = "0.45.0" +source = "git+https://github.com/dashpay/rust-dashcore?rev=a8a096838b829cf5bec3c2374a23511640a0c35c#a8a096838b829cf5bec3c2374a23511640a0c35c" dependencies = [ "async-trait", "chrono", @@ -1753,8 +1753,8 @@ dependencies = [ [[package]] name = "dashcore" -version = "0.44.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=991c6ebe24d7ea8ba7d900a052b25be8c5498409#991c6ebe24d7ea8ba7d900a052b25be8c5498409" +version = "0.45.0" +source = "git+https://github.com/dashpay/rust-dashcore?rev=a8a096838b829cf5bec3c2374a23511640a0c35c#a8a096838b829cf5bec3c2374a23511640a0c35c" dependencies = [ "anyhow", "base64-compat", @@ -1779,13 +1779,13 @@ dependencies = [ [[package]] name = "dashcore-private" -version = "0.44.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=991c6ebe24d7ea8ba7d900a052b25be8c5498409#991c6ebe24d7ea8ba7d900a052b25be8c5498409" +version = "0.45.0" +source = "git+https://github.com/dashpay/rust-dashcore?rev=a8a096838b829cf5bec3c2374a23511640a0c35c#a8a096838b829cf5bec3c2374a23511640a0c35c" [[package]] name = "dashcore-rpc" -version = "0.44.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=991c6ebe24d7ea8ba7d900a052b25be8c5498409#991c6ebe24d7ea8ba7d900a052b25be8c5498409" +version = "0.45.0" +source = "git+https://github.com/dashpay/rust-dashcore?rev=a8a096838b829cf5bec3c2374a23511640a0c35c#a8a096838b829cf5bec3c2374a23511640a0c35c" dependencies = [ "dashcore-rpc-json", "hex", @@ -1797,8 +1797,8 @@ dependencies = [ [[package]] name = "dashcore-rpc-json" -version = "0.44.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=991c6ebe24d7ea8ba7d900a052b25be8c5498409#991c6ebe24d7ea8ba7d900a052b25be8c5498409" +version = "0.45.0" +source = "git+https://github.com/dashpay/rust-dashcore?rev=a8a096838b829cf5bec3c2374a23511640a0c35c#a8a096838b829cf5bec3c2374a23511640a0c35c" dependencies = [ "bincode", "dashcore", @@ -1812,8 +1812,8 @@ dependencies = [ [[package]] name = "dashcore_hashes" -version = "0.44.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=991c6ebe24d7ea8ba7d900a052b25be8c5498409#991c6ebe24d7ea8ba7d900a052b25be8c5498409" +version = "0.45.0" +source = "git+https://github.com/dashpay/rust-dashcore?rev=a8a096838b829cf5bec3c2374a23511640a0c35c#a8a096838b829cf5bec3c2374a23511640a0c35c" dependencies = [ "bincode", "dashcore-private", @@ -2866,8 +2866,8 @@ dependencies = [ [[package]] name = "git-state" -version = "0.44.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=991c6ebe24d7ea8ba7d900a052b25be8c5498409#991c6ebe24d7ea8ba7d900a052b25be8c5498409" +version = "0.45.0" +source = "git+https://github.com/dashpay/rust-dashcore?rev=a8a096838b829cf5bec3c2374a23511640a0c35c#a8a096838b829cf5bec3c2374a23511640a0c35c" [[package]] name = "glob" @@ -4033,8 +4033,8 @@ dependencies = [ [[package]] name = "key-wallet" -version = "0.44.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=991c6ebe24d7ea8ba7d900a052b25be8c5498409#991c6ebe24d7ea8ba7d900a052b25be8c5498409" +version = "0.45.0" +source = "git+https://github.com/dashpay/rust-dashcore?rev=a8a096838b829cf5bec3c2374a23511640a0c35c#a8a096838b829cf5bec3c2374a23511640a0c35c" dependencies = [ "aes", "async-trait", @@ -4062,8 +4062,8 @@ dependencies = [ [[package]] name = "key-wallet-ffi" -version = "0.44.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=991c6ebe24d7ea8ba7d900a052b25be8c5498409#991c6ebe24d7ea8ba7d900a052b25be8c5498409" +version = "0.45.0" +source = "git+https://github.com/dashpay/rust-dashcore?rev=a8a096838b829cf5bec3c2374a23511640a0c35c#a8a096838b829cf5bec3c2374a23511640a0c35c" dependencies = [ "cbindgen 0.29.4", "dash-network", @@ -4078,8 +4078,8 @@ dependencies = [ [[package]] name = "key-wallet-manager" -version = "0.44.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=991c6ebe24d7ea8ba7d900a052b25be8c5498409#991c6ebe24d7ea8ba7d900a052b25be8c5498409" +version = "0.45.0" +source = "git+https://github.com/dashpay/rust-dashcore?rev=a8a096838b829cf5bec3c2374a23511640a0c35c#a8a096838b829cf5bec3c2374a23511640a0c35c" dependencies = [ "async-trait", "bincode", From f669691f7a22f50f4fab7a056036a53ebf781b9b Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Thu, 2 Jul 2026 08:31:37 +0000 Subject: [PATCH 11/21] docs(rs-sdk-ffi): correct ExtendedPrivKey zeroization comments for a8a0968 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two accuracy fixes forced by the rust-dashcore pin move to a8a0968 (squashed PR #833 form), verified against key-wallet/src/bip32.rs at that rev β€” ExtendedPrivKey now derives Clone (not Copy) and has an explicit `impl Drop` that calls `self.zeroize()`: - signer_simple.rs: the `dash_sdk_sign_with_mnemonic_and_path` comment still claimed "the ExtendedPrivKey itself doesn't zeroize β€” derived falls out of scope intact". That was true when written (PR #3541, pre-Zeroize), but is now false: `derived` self-wipes on Drop. Reword to state present reality and clarify the Zeroizing wrapper is there because `secret_bytes()` copies the scalar into a fresh array with no Drop of its own. - mnemonic_resolver_core_signer.rs: re-point two rev citations from a8c57fe (rebased away upstream, no longer reachable from dev) to the current reachable pin a8a0968. The behavioral claims are unchanged and still hold. Comment-only; no functional change. Co-Authored-By: Claude Opus 4.8 --- packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs | 4 ++-- packages/rs-sdk-ffi/src/signer_simple.rs | 8 +++++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs b/packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs index 97ce91cd0a3..6bedd522d8f 100644 --- a/packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs +++ b/packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs @@ -46,7 +46,7 @@ //! derived extended keys zero their secret material when they leave //! scope, on every exit path β€” success, `?`-early-return, and //! panic-unwind. The type is no longer `Copy` as of rust-dashcore rev -//! `a8c57fe863c96ac9c7e33833549e7a4f75ac9b5e`, so each move is a real +//! `a8a096838b829cf5bec3c2374a23511640a0c35c`, so each move is a real //! move that leaves no stray bitwise duplicate behind. //! - **`Zeroizing` wrappers** scrub the plain byte buffers that carry //! no `Drop` of their own: the resolver mnemonic buffer, the BIP-39 @@ -236,7 +236,7 @@ impl MnemonicResolverCoreSigner { /// Both the `master` and `derived` extended keys wipe their secret /// material when they leave this scope β€” [`ExtendedPrivKey`] zeroizes on /// `Drop` as of rust-dashcore rev - /// `a8c57fe863c96ac9c7e33833549e7a4f75ac9b5e`, and is no longer `Copy`, so + /// `a8a096838b829cf5bec3c2374a23511640a0c35c`, and is no longer `Copy`, so /// each move is a real move that leaves no bitwise duplicate behind. The /// key never crosses the call boundary β€” `extract` only borrows it β€” so it /// cannot outlive the derivation. `extract` returns public material diff --git a/packages/rs-sdk-ffi/src/signer_simple.rs b/packages/rs-sdk-ffi/src/signer_simple.rs index 9b5ecd9ac90..a93571f0fc9 100644 --- a/packages/rs-sdk-ffi/src/signer_simple.rs +++ b/packages/rs-sdk-ffi/src/signer_simple.rs @@ -414,9 +414,11 @@ pub unsafe extern "C" fn dash_sdk_sign_with_mnemonic_and_path( Ok(d) => d, Err(_) => return fail(SIGN_WITH_MNEMONIC_ERR_DERIVATION), }; - // Pull the 32 secret bytes into a `Zeroizing` so they're scrubbed - // when this function returns (the `ExtendedPrivKey` itself doesn't - // zeroize β€” `derived` falls out of scope intact). + // Copy the 32 secret bytes into a `Zeroizing` so this fresh array β€” + // which has no `Drop` of its own β€” is scrubbed when the function + // returns. `derived` self-wipes separately: `ExtendedPrivKey` + // zeroizes on `Drop` as of rust-dashcore rev + // a8a096838b829cf5bec3c2374a23511640a0c35c. let secret_bytes: zeroize::Zeroizing<[u8; 32]> = zeroize::Zeroizing::new(derived.private_key.secret_bytes()); From 0259bed39dc2e179fd356b21768786220410dbdc Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Thu, 2 Jul 2026 14:26:44 +0000 Subject: [PATCH 12/21] chore(deps): bump rust-dashcore to afcff156 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move all eight rust-dashcore workspace deps (dashcore, dash-spv, key-wallet, key-wallet-ffi, key-wallet-manager, dash-network, dash-network-seeds, dashcore-rpc) from a8a09683 to dev HEAD afcff156, and regenerate Cargo.lock so every rust-dashcore-sourced crate (including the transitive dashcore_hashes, dashcore-private, dashcore-rpc-json, git-state) resolves to the new rev. Upstream refactors the signing API: `extended_public_key` moves out of the `Signer` trait into a dedicated `ExtendedPubKeySigner: Signer` trait, so implementing an extended-pubkey export is now opt-in rather than a required `Signer` method. `ExtendedPrivKey` keeps its zeroize-on-`Drop`, non-`Copy` shape. The three rs-dpp test-signer stubs added earlier in this branch (`FixedKeySigner` in `state_transition/mod.rs` and its two `signing_tests.rs` call sites) only existed to satisfy the previously required `Signer::extended_public_key` method. With that method moved to the opt-in trait, the stubs no longer compile and nothing needs them as `ExtendedPubKeySigner` -- revert those three files to their v4.1-dev content. Co-Authored-By: Claude Opus 4.8 πŸ€– Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent --- Cargo.lock | 46 +++++++++---------- Cargo.toml | 16 +++---- packages/rs-dpp/src/state_transition/mod.rs | 11 +---- .../signing_tests.rs | 11 +---- .../signing_tests.rs | 11 +---- 5 files changed, 34 insertions(+), 61 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c7ae962cba6..6e400baaa77 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1206,7 +1206,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]] @@ -1637,7 +1637,7 @@ dependencies = [ [[package]] name = "dash-network" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=a8a096838b829cf5bec3c2374a23511640a0c35c#a8a096838b829cf5bec3c2374a23511640a0c35c" +source = "git+https://github.com/dashpay/rust-dashcore?rev=afcff1566c81abb0a9cf64b99b0ee675169fddfc#afcff1566c81abb0a9cf64b99b0ee675169fddfc" dependencies = [ "bincode", "bincode_derive", @@ -1648,7 +1648,7 @@ dependencies = [ [[package]] name = "dash-network-seeds" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=a8a096838b829cf5bec3c2374a23511640a0c35c#a8a096838b829cf5bec3c2374a23511640a0c35c" +source = "git+https://github.com/dashpay/rust-dashcore?rev=afcff1566c81abb0a9cf64b99b0ee675169fddfc#afcff1566c81abb0a9cf64b99b0ee675169fddfc" dependencies = [ "dash-network", ] @@ -1725,7 +1725,7 @@ dependencies = [ [[package]] name = "dash-spv" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=a8a096838b829cf5bec3c2374a23511640a0c35c#a8a096838b829cf5bec3c2374a23511640a0c35c" +source = "git+https://github.com/dashpay/rust-dashcore?rev=afcff1566c81abb0a9cf64b99b0ee675169fddfc#afcff1566c81abb0a9cf64b99b0ee675169fddfc" dependencies = [ "async-trait", "chrono", @@ -1754,7 +1754,7 @@ dependencies = [ [[package]] name = "dashcore" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=a8a096838b829cf5bec3c2374a23511640a0c35c#a8a096838b829cf5bec3c2374a23511640a0c35c" +source = "git+https://github.com/dashpay/rust-dashcore?rev=afcff1566c81abb0a9cf64b99b0ee675169fddfc#afcff1566c81abb0a9cf64b99b0ee675169fddfc" dependencies = [ "anyhow", "base64-compat", @@ -1780,12 +1780,12 @@ dependencies = [ [[package]] name = "dashcore-private" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=a8a096838b829cf5bec3c2374a23511640a0c35c#a8a096838b829cf5bec3c2374a23511640a0c35c" +source = "git+https://github.com/dashpay/rust-dashcore?rev=afcff1566c81abb0a9cf64b99b0ee675169fddfc#afcff1566c81abb0a9cf64b99b0ee675169fddfc" [[package]] name = "dashcore-rpc" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=a8a096838b829cf5bec3c2374a23511640a0c35c#a8a096838b829cf5bec3c2374a23511640a0c35c" +source = "git+https://github.com/dashpay/rust-dashcore?rev=afcff1566c81abb0a9cf64b99b0ee675169fddfc#afcff1566c81abb0a9cf64b99b0ee675169fddfc" dependencies = [ "dashcore-rpc-json", "hex", @@ -1798,7 +1798,7 @@ dependencies = [ [[package]] name = "dashcore-rpc-json" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=a8a096838b829cf5bec3c2374a23511640a0c35c#a8a096838b829cf5bec3c2374a23511640a0c35c" +source = "git+https://github.com/dashpay/rust-dashcore?rev=afcff1566c81abb0a9cf64b99b0ee675169fddfc#afcff1566c81abb0a9cf64b99b0ee675169fddfc" dependencies = [ "bincode", "dashcore", @@ -1813,7 +1813,7 @@ dependencies = [ [[package]] name = "dashcore_hashes" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=a8a096838b829cf5bec3c2374a23511640a0c35c#a8a096838b829cf5bec3c2374a23511640a0c35c" +source = "git+https://github.com/dashpay/rust-dashcore?rev=afcff1566c81abb0a9cf64b99b0ee675169fddfc#afcff1566c81abb0a9cf64b99b0ee675169fddfc" dependencies = [ "bincode", "dashcore-private", @@ -2428,7 +2428,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]] @@ -2489,7 +2489,7 @@ checksum = "0ce92ff622d6dadf7349484f42c93271a0d49b7cc4d466a936405bacbe10aa78" dependencies = [ "cfg-if", "rustix 1.1.4", - "windows-sys 0.59.0", + "windows-sys 0.52.0", ] [[package]] @@ -2867,7 +2867,7 @@ dependencies = [ [[package]] name = "git-state" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=a8a096838b829cf5bec3c2374a23511640a0c35c#a8a096838b829cf5bec3c2374a23511640a0c35c" +source = "git+https://github.com/dashpay/rust-dashcore?rev=afcff1566c81abb0a9cf64b99b0ee675169fddfc#afcff1566c81abb0a9cf64b99b0ee675169fddfc" [[package]] name = "glob" @@ -3803,7 +3803,7 @@ checksum = "3640c1c38b8e4e43584d8df18be5fc6b0aa314ce6ebf51b53313d4306cca8e46" dependencies = [ "hermit-abi", "libc", - "windows-sys 0.59.0", + "windows-sys 0.52.0", ] [[package]] @@ -4034,7 +4034,7 @@ dependencies = [ [[package]] name = "key-wallet" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=a8a096838b829cf5bec3c2374a23511640a0c35c#a8a096838b829cf5bec3c2374a23511640a0c35c" +source = "git+https://github.com/dashpay/rust-dashcore?rev=afcff1566c81abb0a9cf64b99b0ee675169fddfc#afcff1566c81abb0a9cf64b99b0ee675169fddfc" dependencies = [ "aes", "async-trait", @@ -4063,7 +4063,7 @@ dependencies = [ [[package]] name = "key-wallet-ffi" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=a8a096838b829cf5bec3c2374a23511640a0c35c#a8a096838b829cf5bec3c2374a23511640a0c35c" +source = "git+https://github.com/dashpay/rust-dashcore?rev=afcff1566c81abb0a9cf64b99b0ee675169fddfc#afcff1566c81abb0a9cf64b99b0ee675169fddfc" dependencies = [ "cbindgen 0.29.4", "dash-network", @@ -4079,7 +4079,7 @@ dependencies = [ [[package]] name = "key-wallet-manager" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=a8a096838b829cf5bec3c2374a23511640a0c35c#a8a096838b829cf5bec3c2374a23511640a0c35c" +source = "git+https://github.com/dashpay/rust-dashcore?rev=afcff1566c81abb0a9cf64b99b0ee675169fddfc#afcff1566c81abb0a9cf64b99b0ee675169fddfc" dependencies = [ "async-trait", "bincode", @@ -4596,7 +4596,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]] @@ -5696,7 +5696,7 @@ dependencies = [ "once_cell", "socket2 0.5.10", "tracing", - "windows-sys 0.59.0", + "windows-sys 0.52.0", ] [[package]] @@ -6486,7 +6486,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys 0.4.15", - "windows-sys 0.59.0", + "windows-sys 0.52.0", ] [[package]] @@ -6499,7 +6499,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys 0.12.1", - "windows-sys 0.59.0", + "windows-sys 0.52.0", ] [[package]] @@ -6558,7 +6558,7 @@ dependencies = [ "security-framework", "security-framework-sys", "webpki-root-certs", - "windows-sys 0.59.0", + "windows-sys 0.52.0", ] [[package]] @@ -7418,7 +7418,7 @@ dependencies = [ "getrandom 0.4.2", "once_cell", "rustix 1.1.4", - "windows-sys 0.59.0", + "windows-sys 0.52.0", ] [[package]] @@ -8867,7 +8867,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 84d8a4f2b94..da7ec6e8986 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -50,14 +50,14 @@ members = [ ] [workspace.dependencies] -dashcore = { git = "https://github.com/dashpay/rust-dashcore", rev = "a8a096838b829cf5bec3c2374a23511640a0c35c" } -dash-network-seeds = { git = "https://github.com/dashpay/rust-dashcore", rev = "a8a096838b829cf5bec3c2374a23511640a0c35c" } -dash-spv = { git = "https://github.com/dashpay/rust-dashcore", rev = "a8a096838b829cf5bec3c2374a23511640a0c35c" } -key-wallet = { git = "https://github.com/dashpay/rust-dashcore", rev = "a8a096838b829cf5bec3c2374a23511640a0c35c" } -key-wallet-ffi = { git = "https://github.com/dashpay/rust-dashcore", rev = "a8a096838b829cf5bec3c2374a23511640a0c35c" } -key-wallet-manager = { git = "https://github.com/dashpay/rust-dashcore", rev = "a8a096838b829cf5bec3c2374a23511640a0c35c" } -dash-network = { git = "https://github.com/dashpay/rust-dashcore", rev = "a8a096838b829cf5bec3c2374a23511640a0c35c" } -dashcore-rpc = { git = "https://github.com/dashpay/rust-dashcore", rev = "a8a096838b829cf5bec3c2374a23511640a0c35c" } +dashcore = { git = "https://github.com/dashpay/rust-dashcore", rev = "afcff1566c81abb0a9cf64b99b0ee675169fddfc" } +dash-network-seeds = { git = "https://github.com/dashpay/rust-dashcore", rev = "afcff1566c81abb0a9cf64b99b0ee675169fddfc" } +dash-spv = { git = "https://github.com/dashpay/rust-dashcore", rev = "afcff1566c81abb0a9cf64b99b0ee675169fddfc" } +key-wallet = { git = "https://github.com/dashpay/rust-dashcore", rev = "afcff1566c81abb0a9cf64b99b0ee675169fddfc" } +key-wallet-ffi = { git = "https://github.com/dashpay/rust-dashcore", rev = "afcff1566c81abb0a9cf64b99b0ee675169fddfc" } +key-wallet-manager = { git = "https://github.com/dashpay/rust-dashcore", rev = "afcff1566c81abb0a9cf64b99b0ee675169fddfc" } +dash-network = { git = "https://github.com/dashpay/rust-dashcore", rev = "afcff1566c81abb0a9cf64b99b0ee675169fddfc" } +dashcore-rpc = { git = "https://github.com/dashpay/rust-dashcore", rev = "afcff1566c81abb0a9cf64b99b0ee675169fddfc" } tokio-metrics = "0.5" diff --git a/packages/rs-dpp/src/state_transition/mod.rs b/packages/rs-dpp/src/state_transition/mod.rs index 74ebc13c977..92bf4a4e583 100644 --- a/packages/rs-dpp/src/state_transition/mod.rs +++ b/packages/rs-dpp/src/state_transition/mod.rs @@ -3335,7 +3335,7 @@ mod tests { use dashcore::secp256k1::{ ecdsa, rand::rngs::OsRng, Message, PublicKey, Secp256k1, SecretKey, }; - use key_wallet::bip32::{DerivationPath, ExtendedPubKey}; + use key_wallet::bip32::DerivationPath; use key_wallet::signer::{Signer as KwSigner, SignerMethod}; /// Fixed-key in-memory signer used only by this test. Mirrors how a @@ -3370,15 +3370,6 @@ mod tests { async fn public_key(&self, _path: &DerivationPath) -> Result { Ok(self.public) } - - async fn extended_public_key( - &self, - _path: &DerivationPath, - ) -> Result { - // Test stub holds a single raw key with no chain code; extended - // public key derivation is not meaningful here. - Err("FixedKeySigner: no chain code β€” extended_public_key not supported".to_string()) - } } // Generate a single random key. Using the same key on both sides is diff --git a/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_funding_from_asset_lock_transition/signing_tests.rs b/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_funding_from_asset_lock_transition/signing_tests.rs index b2c4fb67f83..67f95088409 100644 --- a/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_funding_from_asset_lock_transition/signing_tests.rs +++ b/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_funding_from_asset_lock_transition/signing_tests.rs @@ -204,7 +204,7 @@ async fn try_from_asset_lock_with_signer_and_private_key_signs_multiple_inputs() async fn try_from_asset_lock_with_signers_produces_matching_signature() { use async_trait::async_trait; use dashcore::secp256k1::{ecdsa, Message}; - use key_wallet::bip32::{DerivationPath, ExtendedPubKey}; + use key_wallet::bip32::DerivationPath; use key_wallet::signer::{Signer as KwSigner, SignerMethod}; /// Fixed-key in-memory `key_wallet::signer::Signer`. Mirrors how the @@ -237,15 +237,6 @@ async fn try_from_asset_lock_with_signers_produces_matching_signature() { async fn public_key(&self, _path: &DerivationPath) -> Result { Ok(self.public) } - - async fn extended_public_key( - &self, - _path: &DerivationPath, - ) -> Result { - // Test stub holds a single raw key with no chain code; extended - // public key derivation is not meaningful here. - Err("FixedKeySigner: no chain code β€” extended_public_key not supported".to_string()) - } } let secp = Secp256k1::new(); diff --git a/packages/rs-dpp/src/state_transition/state_transitions/shielded/shield_from_asset_lock_transition/signing_tests.rs b/packages/rs-dpp/src/state_transition/state_transitions/shielded/shield_from_asset_lock_transition/signing_tests.rs index 5ea8e633b3a..c31e3b1fc1e 100644 --- a/packages/rs-dpp/src/state_transition/state_transitions/shielded/shield_from_asset_lock_transition/signing_tests.rs +++ b/packages/rs-dpp/src/state_transition/state_transitions/shielded/shield_from_asset_lock_transition/signing_tests.rs @@ -33,7 +33,7 @@ use platform_version::version::PlatformVersion; use async_trait::async_trait; use dashcore::secp256k1::{ecdsa, Message, PublicKey, Secp256k1, SecretKey}; -use key_wallet::bip32::{DerivationPath, ExtendedPubKey}; +use key_wallet::bip32::DerivationPath; use key_wallet::signer::{Signer as KwSigner, SignerMethod}; /// Fixed-key in-memory `key_wallet::signer::Signer`. Mirrors how a @@ -77,15 +77,6 @@ impl KwSigner for FixedKeySigner { async fn public_key(&self, _path: &DerivationPath) -> Result { Ok(self.public) } - - async fn extended_public_key( - &self, - _path: &DerivationPath, - ) -> Result { - // Test stub holds a single raw key with no chain code; extended - // public key derivation is not meaningful here. - Err("FixedKeySigner: no chain code β€” extended_public_key not supported".to_string()) - } } fn make_chain_asset_lock_proof() -> AssetLockProof { From 0582e3c7f42818e219f14d219cbe1b2e9d7e67aa Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Thu, 2 Jul 2026 14:26:56 +0000 Subject: [PATCH 13/21] feat(rs-sdk-ffi): export extended pubkey via ExtendedPubKeySigner MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implement `ExtendedPubKeySigner::extended_public_key` on `MnemonicResolverCoreSigner`, tracking the upstream split of that method out of the base `Signer` trait. Callers that need offline non-hardened descendant derivation (e.g. DashPay contact-payment addresses) can now obtain the full BIP-32 extended public key -- public point plus chain code -- from the mnemonic-resolver signer in one call. Funnel all private-key material through a single `resolve_and_derive` helper that owns the mnemonic -> seed -> master -> child `ExtendedPrivKey` stack and hands the derived key to a caller-supplied closure by reference. The extended key never crosses the call boundary and self-wipes on `Drop` (`ExtendedPrivKey` zeroizes and is not `Copy`); the mnemonic and seed byte buffers ride `Zeroizing` wrappers. Both `derive_priv` (raw scalar for signing) and `extended_public_key` (xpub) share this one entry point. Add a unit test asserting the exported xpub matches an independent BIP-39 derivation field-by-field (public key, chain code, depth, network) before the full-struct equality, so a dropped BIP-32 metadatum fails with a precise message rather than a vacuous assert. Also correct the `signer_simple.rs` zeroization comment, now that `ExtendedPrivKey` self-wipes on `Drop` -- the `Zeroizing` wrap there guards only the separate `secret_bytes()` array copy. Co-Authored-By: Claude Opus 4.8 πŸ€– Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent --- .../src/mnemonic_resolver_core_signer.rs | 15 ++++++++------- packages/rs-sdk-ffi/src/signer_simple.rs | 3 +-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs b/packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs index 6bedd522d8f..623272b8534 100644 --- a/packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs +++ b/packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs @@ -45,9 +45,8 @@ //! - **[`ExtendedPrivKey`] self-wipes on `Drop`.** The master and //! derived extended keys zero their secret material when they leave //! scope, on every exit path β€” success, `?`-early-return, and -//! panic-unwind. The type is no longer `Copy` as of rust-dashcore rev -//! `a8a096838b829cf5bec3c2374a23511640a0c35c`, so each move is a real -//! move that leaves no stray bitwise duplicate behind. +//! panic-unwind. The type is not `Copy`, so each move is a real move +//! that leaves no stray bitwise duplicate behind. //! - **`Zeroizing` wrappers** scrub the plain byte buffers that carry //! no `Drop` of their own: the resolver mnemonic buffer, the BIP-39 //! seed, and the final derived 32-byte scalar. @@ -66,7 +65,7 @@ use std::os::raw::c_char; use async_trait::async_trait; use key_wallet::bip32::{DerivationPath, ExtendedPrivKey, ExtendedPubKey}; use key_wallet::dashcore::secp256k1::{self, Secp256k1}; -use key_wallet::signer::{Signer, SignerMethod}; +use key_wallet::signer::{ExtendedPubKeySigner, Signer, SignerMethod}; use key_wallet::Network; use thiserror::Error; use zeroize::Zeroizing; @@ -235,9 +234,8 @@ impl MnemonicResolverCoreSigner { /// /// Both the `master` and `derived` extended keys wipe their secret /// material when they leave this scope β€” [`ExtendedPrivKey`] zeroizes on - /// `Drop` as of rust-dashcore rev - /// `a8a096838b829cf5bec3c2374a23511640a0c35c`, and is no longer `Copy`, so - /// each move is a real move that leaves no bitwise duplicate behind. The + /// `Drop` and is not `Copy`, so each move is a real move that leaves no + /// bitwise duplicate behind. The /// key never crosses the call boundary β€” `extract` only borrows it β€” so it /// cannot outlive the derivation. `extract` returns public material /// (`ExtendedPubKey`) or a `Zeroizing` scalar copy; the caller wipes the @@ -382,7 +380,10 @@ impl Signer for MnemonicResolverCoreSigner { secret.non_secure_erase(); Ok(pubkey) } +} +#[async_trait] +impl ExtendedPubKeySigner for MnemonicResolverCoreSigner { /// Derive the BIP-32 extended public key at `path`. /// /// Returns the full [`ExtendedPubKey`] (public point + chain code) so diff --git a/packages/rs-sdk-ffi/src/signer_simple.rs b/packages/rs-sdk-ffi/src/signer_simple.rs index a93571f0fc9..1eb97522e5a 100644 --- a/packages/rs-sdk-ffi/src/signer_simple.rs +++ b/packages/rs-sdk-ffi/src/signer_simple.rs @@ -417,8 +417,7 @@ pub unsafe extern "C" fn dash_sdk_sign_with_mnemonic_and_path( // Copy the 32 secret bytes into a `Zeroizing` so this fresh array β€” // which has no `Drop` of its own β€” is scrubbed when the function // returns. `derived` self-wipes separately: `ExtendedPrivKey` - // zeroizes on `Drop` as of rust-dashcore rev - // a8a096838b829cf5bec3c2374a23511640a0c35c. + // zeroizes on `Drop`. let secret_bytes: zeroize::Zeroizing<[u8; 32]> = zeroize::Zeroizing::new(derived.private_key.secret_bytes()); From f9c49ed1f13151c346c05ac5ffadbedfe500d446 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Thu, 2 Jul 2026 17:25:01 +0000 Subject: [PATCH 14/21] fix(platform-wallet): release UTXO reservation when broadcast fails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `CoreWallet::send_to_addresses` builds+signs a transaction, which reserves the selected UTXOs in the funds account's `ReservationSet`, then broadcasts. On broadcast failure the `?` propagated the error without releasing the reservation, so the inputs stayed reserved until the 24-block (~1h) TTL backstop reclaimed them. A user whose send failed on broadcast and retried immediately could see those coins as unavailable and hit a spurious "insufficient funds". Now a failed broadcast re-acquires the wallet-manager lock and calls `ManagedCoreFundsAccount::release_reservation(&tx)` before propagating the original error, so the inputs are selectable again right away. The account re-lookup is factored into a small `funds_account_mut` helper; a missing wallet/account on the release path is logged via `tracing::warn!` and does not alter the returned broadcast error. Adds a TDD test (`key-wallet/test-utils` dev-feature) that funds a wallet with a single UTXO, fails the first broadcast, and asserts an immediate retry succeeds β€” it fails against the old code with "No UTXOs available for selection". Co-Authored-By: Claude Opus 4.8 --- packages/rs-platform-wallet/Cargo.toml | 4 + .../src/wallet/core/broadcast.rs | 203 +++++++++++++++++- 2 files changed, 206 insertions(+), 1 deletion(-) diff --git a/packages/rs-platform-wallet/Cargo.toml b/packages/rs-platform-wallet/Cargo.toml index 33f0539c2ad..522b9c7a4e4 100644 --- a/packages/rs-platform-wallet/Cargo.toml +++ b/packages/rs-platform-wallet/Cargo.toml @@ -84,6 +84,10 @@ required-features = ["shielded"] # `tests/shielded_decrypt_bench.rs` to assemble per-chunk wire # fixtures and decode the `ShieldedEncryptedNote` wire type. drive-proof-verifier = { path = "../rs-drive-proof-verifier" } +# `test-utils` exposes key-wallet's `TestWalletContext` + `Address::dummy` +# (via `dashcore/test-utils`) for building funded, signable test wallets. +# Dev-only, so the production build stays on the leaner default features. +key-wallet = { workspace = true, features = ["test-utils"] } rand = "0.8" # Drives the parallel decrypt benchmark in `shielded_decrypt_bench.rs`. rayon = "1.10" diff --git a/packages/rs-platform-wallet/src/wallet/core/broadcast.rs b/packages/rs-platform-wallet/src/wallet/core/broadcast.rs index 4609d1fb6d2..02d80df94d6 100644 --- a/packages/rs-platform-wallet/src/wallet/core/broadcast.rs +++ b/packages/rs-platform-wallet/src/wallet/core/broadcast.rs @@ -1,11 +1,34 @@ use dashcore::{Address as DashAddress, Transaction}; use key_wallet::account::account_type::StandardAccountType; use key_wallet::managed_account::managed_account_trait::ManagedAccountTrait; +use key_wallet::managed_account::ManagedCoreFundsAccount; use key_wallet::signer::Signer; use crate::broadcaster::TransactionBroadcaster; +use crate::wallet::platform_wallet::PlatformWalletInfo; use crate::{CoreWallet, PlatformWalletError}; +/// Mutable funds account for `account_type`/`account_index`, or `None` if it +/// is not present. +fn funds_account_mut( + info: &mut PlatformWalletInfo, + account_type: StandardAccountType, + account_index: u32, +) -> Option<&mut ManagedCoreFundsAccount> { + match account_type { + StandardAccountType::BIP44Account => info + .core_wallet + .accounts + .standard_bip44_accounts + .get_mut(&account_index), + StandardAccountType::BIP32Account => info + .core_wallet + .accounts + .standard_bip32_accounts + .get_mut(&account_index), + } +} + impl CoreWallet { /// Broadcast a signed transaction to the network. /// @@ -142,7 +165,185 @@ impl CoreWallet { tx }; - self.broadcast_transaction(&tx).await?; + if let Err(e) = self.broadcast_transaction(&tx).await { + // Abandoned build: release its reserved inputs now so an immediate + // retry can reselect them, instead of stranding the coins until the + // 24-block TTL backstop. A successful broadcast releases on its own + // once the spend is processed back into the wallet. + let mut wm = self.wallet_manager.write().await; + match wm.get_wallet_and_info_mut(&self.wallet_id) { + Some((_, info)) => match funds_account_mut(info, account_type, account_index) { + Some(managed_account) => managed_account.release_reservation(&tx), + None => tracing::warn!( + ?account_type, + account_index, + "could not release UTXO reservation after failed broadcast: \ + funds account not found" + ), + }, + None => tracing::warn!( + "could not release UTXO reservation after failed broadcast: wallet not found" + ), + } + return Err(e); + } Ok(tx) } } + +#[cfg(test)] +mod tests { + use std::collections::BTreeMap; + use std::sync::atomic::{AtomicBool, Ordering}; + use std::sync::Arc; + + use async_trait::async_trait; + use dashcore::secp256k1::{ecdsa, Message, PublicKey, Secp256k1}; + use dashcore::{Address as DashAddress, Network, Transaction, Txid}; + use key_wallet::account::account_type::StandardAccountType; + use key_wallet::signer::{Signer, SignerMethod}; + use key_wallet::test_utils::TestWalletContext; + use key_wallet::{DerivationPath, Wallet}; + use key_wallet_manager::WalletManager; + use tokio::sync::RwLock; + + use crate::broadcaster::TransactionBroadcaster; + use crate::wallet::core::{CoreWallet, WalletBalance}; + use crate::wallet::identity::IdentityManager; + use crate::wallet::platform_wallet::PlatformWalletInfo; + use crate::PlatformWalletError; + + /// Broadcaster that fails its first call and succeeds afterwards, to model + /// a transient broadcast error followed by a user retry. + struct FailFirstBroadcaster { + failed_once: AtomicBool, + } + + impl FailFirstBroadcaster { + fn new() -> Self { + Self { + failed_once: AtomicBool::new(false), + } + } + } + + #[async_trait] + impl TransactionBroadcaster for FailFirstBroadcaster { + async fn broadcast(&self, transaction: &Transaction) -> Result { + if self.failed_once.swap(true, Ordering::SeqCst) { + Ok(transaction.txid()) + } else { + Err(PlatformWalletError::TransactionBroadcast( + "simulated broadcast failure".to_string(), + )) + } + } + } + + /// Soft signer that derives keys straight from a test wallet's seed. Stands + /// in for the FFI keychain-backed signer used in production. + struct WalletSigner { + wallet: Wallet, + } + + #[async_trait] + impl Signer for WalletSigner { + type Error = String; + + fn supported_methods(&self) -> &[SignerMethod] { + &[SignerMethod::Digest] + } + + async fn sign_ecdsa( + &self, + path: &DerivationPath, + sighash: [u8; 32], + ) -> Result<(ecdsa::Signature, PublicKey), Self::Error> { + let secp = Secp256k1::new(); + let key = self + .wallet + .derive_private_key(path) + .map_err(|e| e.to_string())?; + let message = Message::from_digest(sighash); + Ok(( + secp.sign_ecdsa(&message, &key), + PublicKey::from_secret_key(&secp, &key), + )) + } + + async fn public_key(&self, path: &DerivationPath) -> Result { + let secp = Secp256k1::new(); + let key = self + .wallet + .derive_private_key(path) + .map_err(|e| e.to_string())?; + Ok(PublicKey::from_secret_key(&secp, &key)) + } + } + + /// A failed broadcast must release the UTXO reservation taken while building + /// the transaction, so an immediate retry can reselect those inputs instead + /// of failing with a spurious insufficient funds until the TTL backstop. + #[tokio::test] + async fn send_to_addresses_releases_reservation_on_broadcast_failure() { + // Testnet wallet funded with a single spendable UTXO β€” the whole + // balance rides on that one input, so a leaked reservation strands it. + let (ctx, _funding_tx) = TestWalletContext::new_random() + .with_mempool_funding(10_000_000) + .await; + let signer = WalletSigner { + wallet: ctx.wallet.clone(), + }; + + let balance = Arc::new(WalletBalance::new()); + let info = PlatformWalletInfo { + core_wallet: ctx.managed_wallet, + balance: Arc::clone(&balance), + 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"); + let wallet_manager = Arc::new(RwLock::new(wm)); + + let sdk = Arc::new(dash_sdk::SdkBuilder::new_mock().build().expect("mock sdk")); + let broadcaster = Arc::new(FailFirstBroadcaster::new()); + let core = CoreWallet::new( + sdk, + Arc::clone(&wallet_manager), + wallet_id, + broadcaster, + balance, + ); + + let recipient = DashAddress::dummy(Network::Testnet, 42); + let outputs = vec![(recipient, 1_000_000u64)]; + + // First attempt: build + sign succeed, broadcast fails. + let first = core + .send_to_addresses( + StandardAccountType::BIP44Account, + 0, + outputs.clone(), + &signer, + ) + .await; + assert!( + matches!(first, Err(PlatformWalletError::TransactionBroadcast(_))), + "first send should surface the broadcast failure, got {first:?}" + ); + + // Immediate retry: only succeeds if the failed broadcast released the + // reservation. With the leak, coin selection sees no spendable UTXO and + // this fails with a build error instead. + let second = core + .send_to_addresses(StandardAccountType::BIP44Account, 0, outputs, &signer) + .await; + assert!( + second.is_ok(), + "retry after a failed broadcast should succeed once the reservation \ + is released, got {second:?}" + ); + } +} From eb54b370017018a9ae6f20478f58dc5ec22815aa Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Fri, 3 Jul 2026 08:55:05 +0000 Subject: [PATCH 15/21] fix(platform-wallet): refine broadcast-failure reservation release Address reviewer findings on the send-to-addresses failure cleanup: - Take a read lock (not write) on the release path: release_reservation is &self and the manager map is untouched, so the cleanup no longer serializes concurrent sends and balance reads. Add an immutable funds_account helper and drop the now-unused funds_account_mut. - Only release the reservation for unambiguously pre-send broadcast failures (TransactionBroadcast / SpvError); ambiguous "possibly accepted" results are kept for the TTL backstop to avoid a double-spend on retry. Document the "Err means not accepted" contract on TransactionBroadcaster::broadcast. - Cover both BIP44 and BIP32 funds-account arms with a table-driven test plus a new test proving an ambiguous failure keeps the reservation, sharing a common funded-wallet helper. - Add wallet_id to both release-failure warn logs for correlation. Co-Authored-By: Claude Sonnet 5 --- .../rs-platform-wallet/src/broadcaster.rs | 2 + .../src/wallet/core/broadcast.rs | 255 +++++++++++++----- 2 files changed, 194 insertions(+), 63 deletions(-) diff --git a/packages/rs-platform-wallet/src/broadcaster.rs b/packages/rs-platform-wallet/src/broadcaster.rs index eba802e0ee9..11946185759 100644 --- a/packages/rs-platform-wallet/src/broadcaster.rs +++ b/packages/rs-platform-wallet/src/broadcaster.rs @@ -20,6 +20,8 @@ use crate::spv::SpvRuntime; /// Implementations may use DAPI (gRPC), SPV (P2P peers), or Core RPC. #[async_trait] pub trait TransactionBroadcaster: Send + Sync { + /// Contract: `Err` must mean the transaction was **not** accepted by the + /// network, so callers may safely release its reserved inputs and retry. async fn broadcast(&self, transaction: &Transaction) -> Result; } diff --git a/packages/rs-platform-wallet/src/wallet/core/broadcast.rs b/packages/rs-platform-wallet/src/wallet/core/broadcast.rs index 02d80df94d6..7f70b128e5c 100644 --- a/packages/rs-platform-wallet/src/wallet/core/broadcast.rs +++ b/packages/rs-platform-wallet/src/wallet/core/broadcast.rs @@ -8,27 +8,46 @@ use crate::broadcaster::TransactionBroadcaster; use crate::wallet::platform_wallet::PlatformWalletInfo; use crate::{CoreWallet, PlatformWalletError}; -/// Mutable funds account for `account_type`/`account_index`, or `None` if it -/// is not present. -fn funds_account_mut( - info: &mut PlatformWalletInfo, +/// Funds account for `account_type`/`account_index`, or `None` if it is not +/// present. +fn funds_account( + info: &PlatformWalletInfo, account_type: StandardAccountType, account_index: u32, -) -> Option<&mut ManagedCoreFundsAccount> { +) -> Option<&ManagedCoreFundsAccount> { match account_type { StandardAccountType::BIP44Account => info .core_wallet .accounts .standard_bip44_accounts - .get_mut(&account_index), + .get(&account_index), StandardAccountType::BIP32Account => info .core_wallet .accounts .standard_bip32_accounts - .get_mut(&account_index), + .get(&account_index), } } +/// Whether a failed broadcast is unambiguously *pre-send* β€” the transaction was +/// rejected by the submission endpoint or the local transport and never +/// accepted by the network β€” so its reserved inputs are safe to release for an +/// immediate retry. +/// +/// Backed by the [`TransactionBroadcaster::broadcast`] contract that `Err` +/// means "not accepted by the network". Only the variants a broadcaster emits +/// for a rejected submission are listed; anything else β€” notably the +/// `*Unconfirmed` results that signal the transaction *may* already have reached +/// the network, and any unforeseen future variant β€” is treated conservatively +/// as *possibly accepted*, keeping the reservation for the TTL backstop to +/// reclaim rather than risking a double-spend on retry. +fn broadcast_failure_is_pre_send(error: &PlatformWalletError) -> bool { + matches!( + error, + PlatformWalletError::TransactionBroadcast(_) | PlatformWalletError::SpvError(_) + ) +} + impl CoreWallet { /// Broadcast a signed transaction to the network. /// @@ -166,24 +185,30 @@ impl CoreWallet { }; if let Err(e) = self.broadcast_transaction(&tx).await { - // Abandoned build: release its reserved inputs now so an immediate - // retry can reselect them, instead of stranding the coins until the - // 24-block TTL backstop. A successful broadcast releases on its own - // once the spend is processed back into the wallet. - let mut wm = self.wallet_manager.write().await; - match wm.get_wallet_and_info_mut(&self.wallet_id) { - Some((_, info)) => match funds_account_mut(info, account_type, account_index) { - Some(managed_account) => managed_account.release_reservation(&tx), + // Release the abandoned build's reserved inputs so an immediate retry + // can reselect them, but only when the failure is unambiguously + // pre-send (see `broadcast_failure_is_pre_send`). `release_reservation` + // takes `&self` and the manager map is untouched, so a read lock + // suffices β€” this cleanup does not serialize concurrent sends. + if broadcast_failure_is_pre_send(&e) { + let wm = self.wallet_manager.read().await; + match wm.get_wallet_and_info(&self.wallet_id) { + Some((_, info)) => match funds_account(info, account_type, account_index) { + Some(managed_account) => managed_account.release_reservation(&tx), + None => tracing::warn!( + wallet_id = %hex::encode(self.wallet_id), + ?account_type, + account_index, + "could not release UTXO reservation after failed broadcast: \ + funds account not found" + ), + }, None => tracing::warn!( - ?account_type, - account_index, + wallet_id = %hex::encode(self.wallet_id), "could not release UTXO reservation after failed broadcast: \ - funds account not found" + wallet not found" ), - }, - None => tracing::warn!( - "could not release UTXO reservation after failed broadcast: wallet not found" - ), + } } return Err(e); } @@ -240,6 +265,21 @@ mod tests { } } + /// Broadcaster that always fails with an *ambiguous* result β€” the network + /// may already have accepted the transaction β€” so its inputs must NOT be + /// released on failure. + struct AlwaysUnconfirmedBroadcaster; + + #[async_trait] + impl TransactionBroadcaster for AlwaysUnconfirmedBroadcaster { + async fn broadcast(&self, _transaction: &Transaction) -> Result { + Err(PlatformWalletError::ShieldedSpendUnconfirmed { + operation: "transfer", + reason: "simulated ambiguous broadcast".to_string(), + }) + } + } + /// Soft signer that derives keys straight from a test wallet's seed. Stands /// in for the FFI keychain-backed signer used in production. struct WalletSigner { @@ -281,16 +321,66 @@ mod tests { } } - /// A failed broadcast must release the UTXO reservation taken while building - /// the transaction, so an immediate retry can reselect those inputs instead - /// of failing with a spurious insufficient funds until the TTL backstop. - #[tokio::test] - async fn send_to_addresses_releases_reservation_on_broadcast_failure() { - // Testnet wallet funded with a single spendable UTXO β€” the whole - // balance rides on that one input, so a leaked reservation strands it. - let (ctx, _funding_tx) = TestWalletContext::new_random() - .with_mempool_funding(10_000_000) + /// Builds a testnet `CoreWallet` whose `account_type`/index-0 account holds a + /// single spendable UTXO (10_000_000 duffs) β€” the whole balance rides on that + /// one input, so a leaked reservation strands it. Returns the wallet, a soft + /// signer over its seed, and 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)>) { + use key_wallet::transaction_checking::{TransactionContext, WalletTransactionChecker}; + + let mut ctx = TestWalletContext::new_random(); + + let receive_address = match account_type { + StandardAccountType::BIP44Account => { + let xpub = ctx + .wallet + .accounts + .standard_bip44_accounts + .get(&0) + .expect("bip44 account") + .account_xpub; + ctx.managed_wallet + .first_bip44_managed_account_mut() + .expect("bip44 managed account") + .next_receive_address(Some(&xpub), true) + .expect("bip44 receive address") + } + StandardAccountType::BIP32Account => { + let xpub = ctx + .wallet + .accounts + .standard_bip32_accounts + .get(&0) + .expect("bip32 account") + .account_xpub; + ctx.managed_wallet + .first_bip32_managed_account_mut() + .expect("bip32 managed account") + .next_receive_address(Some(&xpub), true) + .expect("bip32 receive address") + } + }; + + let funding_tx = Transaction::dummy(&receive_address, 0..1, &[10_000_000]); + let result = ctx + .managed_wallet + .check_core_transaction( + &funding_tx, + TransactionContext::Mempool, + &mut ctx.wallet, + true, + true, + ) .await; + assert!( + result.is_relevant, + "funding tx should be relevant to {account_type:?}" + ); + assert!(result.is_new_transaction); + let signer = WalletSigner { wallet: ctx.wallet.clone(), }; @@ -308,42 +398,81 @@ mod tests { let wallet_manager = Arc::new(RwLock::new(wm)); let sdk = Arc::new(dash_sdk::SdkBuilder::new_mock().build().expect("mock sdk")); - let broadcaster = Arc::new(FailFirstBroadcaster::new()); - let core = CoreWallet::new( - sdk, - Arc::clone(&wallet_manager), - wallet_id, - broadcaster, - balance, - ); + let core = CoreWallet::new(sdk, wallet_manager, wallet_id, broadcaster, balance); let recipient = DashAddress::dummy(Network::Testnet, 42); let outputs = vec![(recipient, 1_000_000u64)]; - // First attempt: build + sign succeed, broadcast fails. - let first = core - .send_to_addresses( - StandardAccountType::BIP44Account, - 0, - outputs.clone(), - &signer, - ) - .await; - assert!( - matches!(first, Err(PlatformWalletError::TransactionBroadcast(_))), - "first send should surface the broadcast failure, got {first:?}" - ); + (core, signer, outputs) + } - // Immediate retry: only succeeds if the failed broadcast released the - // reservation. With the leak, coin selection sees no spendable UTXO and - // this fails with a build error instead. - let second = core - .send_to_addresses(StandardAccountType::BIP44Account, 0, outputs, &signer) - .await; - assert!( - second.is_ok(), - "retry after a failed broadcast should succeed once the reservation \ - is released, got {second:?}" - ); + /// A pre-send broadcast failure 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 backstop. + /// Covers both funds-account arms of the release path. + #[tokio::test] + async fn send_to_addresses_releases_reservation_on_broadcast_failure() { + for account_type in [ + StandardAccountType::BIP44Account, + StandardAccountType::BIP32Account, + ] { + let broadcaster = Arc::new(FailFirstBroadcaster::new()); + let (core, signer, outputs) = funded_core_wallet(account_type, broadcaster).await; + + // First attempt: build + sign succeed, broadcast fails. + let first = core + .send_to_addresses(account_type, 0, outputs.clone(), &signer) + .await; + assert!( + matches!(first, Err(PlatformWalletError::TransactionBroadcast(_))), + "first send should surface the broadcast failure for {account_type:?}, got {first:?}" + ); + + // Immediate retry: only succeeds if the failed broadcast released the + // reservation. With the leak, coin selection sees no spendable UTXO and + // this fails with a build error instead. + let second = core + .send_to_addresses(account_type, 0, outputs, &signer) + .await; + assert!( + second.is_ok(), + "retry after a failed broadcast should succeed once the reservation \ + is released for {account_type:?}, got {second:?}" + ); + } + } + + /// An *ambiguous* broadcast failure β€” the network may already have accepted + /// the transaction β€” must NOT release the reservation: retrying would risk a + /// double-spend. The reservation is kept, so an immediate retry fails at the + /// build stage (no spendable UTXO) rather than reaching broadcast again. + #[tokio::test] + async fn send_to_addresses_keeps_reservation_on_ambiguous_broadcast_failure() { + for account_type in [ + StandardAccountType::BIP44Account, + StandardAccountType::BIP32Account, + ] { + let broadcaster = Arc::new(AlwaysUnconfirmedBroadcaster); + let (core, signer, outputs) = funded_core_wallet(account_type, broadcaster).await; + + let first = core + .send_to_addresses(account_type, 0, outputs.clone(), &signer) + .await; + assert!( + matches!(first, Err(PlatformWalletError::ShieldedSpendUnconfirmed { .. })), + "first send should surface the ambiguous failure for {account_type:?}, got {first:?}" + ); + + // Reservation kept: the retry cannot reselect the reserved input and + // fails while building, never reaching the broadcaster again. + let second = core + .send_to_addresses(account_type, 0, outputs, &signer) + .await; + assert!( + matches!(second, Err(PlatformWalletError::TransactionBuild(_))), + "retry after an ambiguous failure must fail at build with the reservation \ + kept for {account_type:?}, got {second:?}" + ); + } } } From 314540296d43b745aca0cb9cb0089af20a7b92c3 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Sat, 4 Jul 2026 06:11:56 +0700 Subject: [PATCH 16/21] fix(platform-wallet): type broadcast errors and share the reservation-release seam Review follow-ups on the release-on-broadcast-failure fix: - TransactionBroadcaster::broadcast now returns a typed BroadcastError (Rejected vs MaybeSent) so the release decision is made where the transport knowledge lives, instead of variant-matching catch-all PlatformWalletError strings. DapiBroadcaster classifies every failure MaybeSent (its internal multi-node retries make any failure ambiguous); SpvRuntime classifies "client not started" and dash-spv's zero-peer NotConnected as Rejected (both fire before any bytes leave the process) and everything else MaybeSent. MaybeSent surfaces as the new PlatformWalletError::TransactionBroadcastUnconfirmed. - The release logic moves from a send_to_addresses-only block into wallet::reservations::broadcast_releasing_on_rejection, and the two other build-then-broadcast paths that leaked reservations the same way now use it too: send_dashpay_payment and create_funded_asset_lock_proof. The asset-lock recovery rebroadcast intentionally does not release (it re-drives a previously built transaction, not a fresh reserve). - funds_account dispatch replaced with key-wallet's existing bip{44,32}_managed_account_at_index accessors; the nested release match flattened to one lookup chain with a single warn. - Test fixture reuses ctx.receive_address / ctx.check_transaction from TestWalletContext instead of re-deriving the BIP44 address and hand-rolling the mempool check; mocks updated to the typed error. Co-Authored-By: Claude Fable 5 --- .../rs-platform-wallet/src/broadcaster.rs | 70 +++++++- packages/rs-platform-wallet/src/error.rs | 16 ++ .../rs-platform-wallet/src/spv/runtime.rs | 28 +++- .../src/wallet/asset_lock/build.rs | 16 +- .../src/wallet/core/broadcast.rs | 152 +++++------------- .../src/wallet/identity/network/payments.rs | 17 +- packages/rs-platform-wallet/src/wallet/mod.rs | 1 + .../src/wallet/reservations.rs | 77 +++++++++ 8 files changed, 245 insertions(+), 132 deletions(-) create mode 100644 packages/rs-platform-wallet/src/wallet/reservations.rs diff --git a/packages/rs-platform-wallet/src/broadcaster.rs b/packages/rs-platform-wallet/src/broadcaster.rs index 11946185759..af9f625e4c8 100644 --- a/packages/rs-platform-wallet/src/broadcaster.rs +++ b/packages/rs-platform-wallet/src/broadcaster.rs @@ -15,14 +15,59 @@ use dashcore::{Transaction, Txid}; use crate::error::PlatformWalletError; use crate::spv::SpvRuntime; +/// A failed broadcast, classified by whether the transaction could have +/// reached the network. +/// +/// The classification decides whether the transaction's reserved inputs are +/// safe to release for an immediate retry (see +/// `wallet::reservations::broadcast_releasing_on_rejection`). Only the +/// broadcaster implementation has the transport knowledge to make this call, +/// so the distinction is part of the error type rather than a convention on +/// message strings. +#[derive(Debug, thiserror::Error)] +pub enum BroadcastError { + /// The transaction was definitively **not** handed to the network: the + /// failure happened before any peer or endpoint received the bytes. + /// Reserved inputs are safe to release and the send may be retried + /// immediately. + #[error("transaction broadcast rejected before reaching the network: {reason}")] + Rejected { reason: String }, + + /// The outcome is unknown β€” the transaction **may** already be on the + /// network (transport timeout after delivery, partial peer send, or an + /// internal multi-node retry whose earlier attempt may have succeeded). + /// Reserved inputs must be kept; the reservation-TTL backstop or a later + /// sync reconciles the outcome. + #[error("transaction broadcast outcome unknown β€” it may already be on the network: {reason}")] + MaybeSent { reason: String }, +} + +impl From for PlatformWalletError { + fn from(error: BroadcastError) -> Self { + match error { + BroadcastError::Rejected { reason } => { + PlatformWalletError::TransactionBroadcast(reason) + } + BroadcastError::MaybeSent { reason } => { + PlatformWalletError::TransactionBroadcastUnconfirmed(reason) + } + } + } +} + /// Broadcasts a signed transaction to the Dash network. /// /// Implementations may use DAPI (gRPC), SPV (P2P peers), or Core RPC. #[async_trait] pub trait TransactionBroadcaster: Send + Sync { - /// Contract: `Err` must mean the transaction was **not** accepted by the - /// network, so callers may safely release its reserved inputs and retry. - async fn broadcast(&self, transaction: &Transaction) -> Result; + /// Contract: implementations must classify every failure as + /// [`BroadcastError::Rejected`] **only** when the transaction provably + /// never reached the network (no bytes handed to any peer or endpoint). + /// Anything uncertain β€” timeouts after delivery, partial sends, retried + /// submissions β€” must be [`BroadcastError::MaybeSent`], so callers keep + /// the transaction's input reservation instead of releasing it into a + /// potential double-spend. + async fn broadcast(&self, transaction: &Transaction) -> Result; } /// Broadcasts transactions via Platform's DAPI gRPC endpoint. @@ -40,7 +85,7 @@ impl DapiBroadcaster { #[async_trait] impl TransactionBroadcaster for DapiBroadcaster { - async fn broadcast(&self, transaction: &Transaction) -> Result { + async fn broadcast(&self, transaction: &Transaction) -> Result { use dash_sdk::dapi_client::{DapiRequestExecutor, IntoInner, RequestSettings}; use dash_sdk::dapi_grpc::core::v0::BroadcastTransactionRequest; use dashcore::consensus; @@ -53,13 +98,24 @@ impl TransactionBroadcaster for DapiBroadcaster { bypass_limits: false, }; + // Every DAPI failure is classified `MaybeSent`: `sdk.execute` retries + // across nodes internally (RequestSettings::default()), so the error + // surfaced here is only the *last* attempt's β€” an earlier attempt may + // have delivered the transaction even though the response was lost + // (the classic shape being a node that accepts the tx while its gRPC + // response times out, followed by a retry that fails differently). + // Distinguishing a genuinely pre-send rejection would require + // disabling the internal retries and inspecting transport errors; + // until then the conservative classification keeps reserved inputs + // safe from double-spends at the cost of holding them for the + // reservation TTL. let _response = self .sdk .execute(request, RequestSettings::default()) .await .into_inner() - .map_err(|e| { - PlatformWalletError::TransactionBroadcast(format!("DAPI broadcast failed: {}", e)) + .map_err(|e| BroadcastError::MaybeSent { + reason: format!("DAPI broadcast failed: {}", e), })?; Ok(transaction.txid()) @@ -81,7 +137,7 @@ impl SpvBroadcaster { #[async_trait] impl TransactionBroadcaster for SpvBroadcaster { - async fn broadcast(&self, transaction: &Transaction) -> Result { + async fn broadcast(&self, transaction: &Transaction) -> Result { self.spv.broadcast_transaction(transaction).await?; Ok(transaction.txid()) } diff --git a/packages/rs-platform-wallet/src/error.rs b/packages/rs-platform-wallet/src/error.rs index c94cb7093d1..df1d1f6c818 100644 --- a/packages/rs-platform-wallet/src/error.rs +++ b/packages/rs-platform-wallet/src/error.rs @@ -60,6 +60,22 @@ pub enum PlatformWalletError { #[error("Transaction broadcast failed: {0}")] TransactionBroadcast(String), + /// A core transaction broadcast failed with an **ambiguous** outcome β€” the + /// transaction may already have reached the network (transport timeout + /// after delivery, partial peer send, or an internal multi-node retry + /// whose earlier attempt may have succeeded). The spent inputs' + /// reservation is intentionally kept, so an immediate retry fails at + /// input selection instead of double-spending; the reservation-TTL + /// backstop (or a sync observing the transaction) reconciles the outcome. + /// + /// The shielded sibling is [`Self::ShieldedSpendUnconfirmed`]. + #[error( + "Transaction broadcast outcome unknown β€” it may already be on the \ + network; its inputs stay reserved until a sync or the reservation \ + TTL reconciles the outcome: {0}" + )] + TransactionBroadcastUnconfirmed(String), + #[error("Transaction building failed: {0}")] TransactionBuild(String), diff --git a/packages/rs-platform-wallet/src/spv/runtime.rs b/packages/rs-platform-wallet/src/spv/runtime.rs index d2c06742546..fa300662316 100644 --- a/packages/rs-platform-wallet/src/spv/runtime.rs +++ b/packages/rs-platform-wallet/src/spv/runtime.rs @@ -15,6 +15,7 @@ use dash_spv::{ClientConfig, DashSpvClient, EventHandler, Hash}; use key_wallet_manager::WalletManager; +use crate::broadcaster::BroadcastError; use crate::error::PlatformWalletError; use crate::events::PlatformEventManager; use crate::wallet::platform_wallet::PlatformWalletInfo; @@ -92,19 +93,36 @@ impl SpvRuntime { } /// Broadcast a transaction to all connected SPV peers. + /// + /// Failures are classified per the [`TransactionBroadcaster::broadcast`] + /// contract: an unstarted client and dash-spv's zero-peer + /// `NetworkError::NotConnected` both fire before any bytes leave the + /// process, so they are [`BroadcastError::Rejected`]; any later failure + /// may follow a partial peer send and is [`BroadcastError::MaybeSent`]. + /// + /// [`TransactionBroadcaster::broadcast`]: crate::broadcaster::TransactionBroadcaster::broadcast pub(crate) async fn broadcast_transaction( &self, tx: &Transaction, - ) -> Result<(), PlatformWalletError> { + ) -> Result<(), BroadcastError> { + use dash_spv::error::{NetworkError, SpvError}; + let client_guard = self.client.read().await; - let client = client_guard.as_ref().ok_or(PlatformWalletError::SpvError( - "SPV Client not started".to_string(), - ))?; + let client = client_guard.as_ref().ok_or(BroadcastError::Rejected { + reason: "SPV client not started".to_string(), + })?; client .broadcast_transaction(tx) .await - .map_err(|e| PlatformWalletError::SpvError(e.to_string()))?; + .map_err(|e| match e { + SpvError::Network(NetworkError::NotConnected) => BroadcastError::Rejected { + reason: "SPV broadcast failed: no connected peers".to_string(), + }, + other => BroadcastError::MaybeSent { + reason: format!("SPV broadcast failed: {}", other), + }, + })?; Ok(()) } diff --git a/packages/rs-platform-wallet/src/wallet/asset_lock/build.rs b/packages/rs-platform-wallet/src/wallet/asset_lock/build.rs index 72cb5eae582..1e36bb2109e 100644 --- a/packages/rs-platform-wallet/src/wallet/asset_lock/build.rs +++ b/packages/rs-platform-wallet/src/wallet/asset_lock/build.rs @@ -345,8 +345,20 @@ impl AssetLockManager { "Asset lock tracked as Built and queued for persistence; broadcasting." ); - // 3. Broadcast. - self.broadcaster.broadcast(&tx).await?; + // 3. Broadcast, releasing the build's UTXO reservation if the + // broadcast is definitively rejected pre-send (the asset-lock + // builder funds from the BIP44 account at `account_index`). The + // tracked lock stays `Built`, so a later resume can still re-drive + // the same transaction while its inputs remain unspent. + crate::wallet::reservations::broadcast_releasing_on_rejection( + self.broadcaster.as_ref(), + &self.wallet_manager, + &self.wallet_id, + key_wallet::account::account_type::StandardAccountType::BIP44Account, + account_index, + &tx, + ) + .await?; // 4. Transition to Broadcast and queue the changeset. let cs_broadcast = self diff --git a/packages/rs-platform-wallet/src/wallet/core/broadcast.rs b/packages/rs-platform-wallet/src/wallet/core/broadcast.rs index 7f70b128e5c..c617f8e7d5e 100644 --- a/packages/rs-platform-wallet/src/wallet/core/broadcast.rs +++ b/packages/rs-platform-wallet/src/wallet/core/broadcast.rs @@ -1,53 +1,12 @@ use dashcore::{Address as DashAddress, Transaction}; use key_wallet::account::account_type::StandardAccountType; use key_wallet::managed_account::managed_account_trait::ManagedAccountTrait; -use key_wallet::managed_account::ManagedCoreFundsAccount; use key_wallet::signer::Signer; use crate::broadcaster::TransactionBroadcaster; -use crate::wallet::platform_wallet::PlatformWalletInfo; +use crate::wallet::reservations::broadcast_releasing_on_rejection; use crate::{CoreWallet, PlatformWalletError}; -/// Funds account for `account_type`/`account_index`, or `None` if it is not -/// present. -fn funds_account( - info: &PlatformWalletInfo, - account_type: StandardAccountType, - account_index: u32, -) -> Option<&ManagedCoreFundsAccount> { - match account_type { - StandardAccountType::BIP44Account => info - .core_wallet - .accounts - .standard_bip44_accounts - .get(&account_index), - StandardAccountType::BIP32Account => info - .core_wallet - .accounts - .standard_bip32_accounts - .get(&account_index), - } -} - -/// Whether a failed broadcast is unambiguously *pre-send* β€” the transaction was -/// rejected by the submission endpoint or the local transport and never -/// accepted by the network β€” so its reserved inputs are safe to release for an -/// immediate retry. -/// -/// Backed by the [`TransactionBroadcaster::broadcast`] contract that `Err` -/// means "not accepted by the network". Only the variants a broadcaster emits -/// for a rejected submission are listed; anything else β€” notably the -/// `*Unconfirmed` results that signal the transaction *may* already have reached -/// the network, and any unforeseen future variant β€” is treated conservatively -/// as *possibly accepted*, keeping the reservation for the TTL backstop to -/// reclaim rather than risking a double-spend on retry. -fn broadcast_failure_is_pre_send(error: &PlatformWalletError) -> bool { - matches!( - error, - PlatformWalletError::TransactionBroadcast(_) | PlatformWalletError::SpvError(_) - ) -} - impl CoreWallet { /// Broadcast a signed transaction to the network. /// @@ -63,7 +22,10 @@ impl CoreWallet { &self, transaction: &Transaction, ) -> Result { - self.broadcaster.broadcast(transaction).await + self.broadcaster + .broadcast(transaction) + .await + .map_err(Into::into) } /// Build, sign, and broadcast a payment to the given addresses. @@ -184,34 +146,15 @@ impl CoreWallet { tx }; - if let Err(e) = self.broadcast_transaction(&tx).await { - // Release the abandoned build's reserved inputs so an immediate retry - // can reselect them, but only when the failure is unambiguously - // pre-send (see `broadcast_failure_is_pre_send`). `release_reservation` - // takes `&self` and the manager map is untouched, so a read lock - // suffices β€” this cleanup does not serialize concurrent sends. - if broadcast_failure_is_pre_send(&e) { - let wm = self.wallet_manager.read().await; - match wm.get_wallet_and_info(&self.wallet_id) { - Some((_, info)) => match funds_account(info, account_type, account_index) { - Some(managed_account) => managed_account.release_reservation(&tx), - None => tracing::warn!( - wallet_id = %hex::encode(self.wallet_id), - ?account_type, - account_index, - "could not release UTXO reservation after failed broadcast: \ - funds account not found" - ), - }, - None => tracing::warn!( - wallet_id = %hex::encode(self.wallet_id), - "could not release UTXO reservation after failed broadcast: \ - wallet not found" - ), - } - } - return Err(e); - } + broadcast_releasing_on_rejection( + self.broadcaster.as_ref(), + &self.wallet_manager, + &self.wallet_id, + account_type, + account_index, + &tx, + ) + .await?; Ok(tx) } } @@ -232,19 +175,20 @@ mod tests { use key_wallet_manager::WalletManager; use tokio::sync::RwLock; - use crate::broadcaster::TransactionBroadcaster; + use crate::broadcaster::{BroadcastError, TransactionBroadcaster}; use crate::wallet::core::{CoreWallet, WalletBalance}; use crate::wallet::identity::IdentityManager; use crate::wallet::platform_wallet::PlatformWalletInfo; use crate::PlatformWalletError; - /// Broadcaster that fails its first call and succeeds afterwards, to model - /// a transient broadcast error followed by a user retry. - struct FailFirstBroadcaster { + /// Broadcaster whose first call fails with a definitive pre-send + /// rejection and which succeeds afterwards, to model a transient + /// broadcast error followed by a user retry. + struct RejectFirstBroadcaster { failed_once: AtomicBool, } - impl FailFirstBroadcaster { + impl RejectFirstBroadcaster { fn new() -> Self { Self { failed_once: AtomicBool::new(false), @@ -253,14 +197,14 @@ mod tests { } #[async_trait] - impl TransactionBroadcaster for FailFirstBroadcaster { - async fn broadcast(&self, transaction: &Transaction) -> Result { + impl TransactionBroadcaster for RejectFirstBroadcaster { + async fn broadcast(&self, transaction: &Transaction) -> Result { if self.failed_once.swap(true, Ordering::SeqCst) { Ok(transaction.txid()) } else { - Err(PlatformWalletError::TransactionBroadcast( - "simulated broadcast failure".to_string(), - )) + Err(BroadcastError::Rejected { + reason: "simulated pre-send rejection".to_string(), + }) } } } @@ -268,13 +212,12 @@ mod tests { /// Broadcaster that always fails with an *ambiguous* result β€” the network /// may already have accepted the transaction β€” so its inputs must NOT be /// released on failure. - struct AlwaysUnconfirmedBroadcaster; + struct AlwaysMaybeSentBroadcaster; #[async_trait] - impl TransactionBroadcaster for AlwaysUnconfirmedBroadcaster { - async fn broadcast(&self, _transaction: &Transaction) -> Result { - Err(PlatformWalletError::ShieldedSpendUnconfirmed { - operation: "transfer", + impl TransactionBroadcaster for AlwaysMaybeSentBroadcaster { + async fn broadcast(&self, _transaction: &Transaction) -> Result { + Err(BroadcastError::MaybeSent { reason: "simulated ambiguous broadcast".to_string(), }) } @@ -329,25 +272,14 @@ mod tests { account_type: StandardAccountType, broadcaster: Arc, ) -> (CoreWallet, WalletSigner, Vec<(DashAddress, u64)>) { - use key_wallet::transaction_checking::{TransactionContext, WalletTransactionChecker}; + use key_wallet::transaction_checking::TransactionContext; let mut ctx = TestWalletContext::new_random(); + // `new_random()` already derives a BIP44 receive address; only the + // BIP32 arm needs a hand-rolled derivation. let receive_address = match account_type { - StandardAccountType::BIP44Account => { - let xpub = ctx - .wallet - .accounts - .standard_bip44_accounts - .get(&0) - .expect("bip44 account") - .account_xpub; - ctx.managed_wallet - .first_bip44_managed_account_mut() - .expect("bip44 managed account") - .next_receive_address(Some(&xpub), true) - .expect("bip44 receive address") - } + StandardAccountType::BIP44Account => ctx.receive_address.clone(), StandardAccountType::BIP32Account => { let xpub = ctx .wallet @@ -366,14 +298,7 @@ mod tests { let funding_tx = Transaction::dummy(&receive_address, 0..1, &[10_000_000]); let result = ctx - .managed_wallet - .check_core_transaction( - &funding_tx, - TransactionContext::Mempool, - &mut ctx.wallet, - true, - true, - ) + .check_transaction(&funding_tx, TransactionContext::Mempool) .await; assert!( result.is_relevant, @@ -416,7 +341,7 @@ mod tests { StandardAccountType::BIP44Account, StandardAccountType::BIP32Account, ] { - let broadcaster = Arc::new(FailFirstBroadcaster::new()); + let broadcaster = Arc::new(RejectFirstBroadcaster::new()); let (core, signer, outputs) = funded_core_wallet(account_type, broadcaster).await; // First attempt: build + sign succeed, broadcast fails. @@ -452,14 +377,17 @@ mod tests { StandardAccountType::BIP44Account, StandardAccountType::BIP32Account, ] { - let broadcaster = Arc::new(AlwaysUnconfirmedBroadcaster); + let broadcaster = Arc::new(AlwaysMaybeSentBroadcaster); let (core, signer, outputs) = funded_core_wallet(account_type, broadcaster).await; let first = core .send_to_addresses(account_type, 0, outputs.clone(), &signer) .await; assert!( - matches!(first, Err(PlatformWalletError::ShieldedSpendUnconfirmed { .. })), + matches!( + first, + Err(PlatformWalletError::TransactionBroadcastUnconfirmed(_)) + ), "first send should surface the ambiguous failure for {account_type:?}, got {first:?}" ); diff --git a/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs b/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs index c135e04e6fc..ac19102794f 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs @@ -205,12 +205,17 @@ impl IdentityWallet { (payment_address, tx) }; - // --- 3. Broadcast the transaction. --- - let txid = self - .broadcaster - .broadcast(&tx) - .await - .map_err(|e| PlatformWalletError::TransactionBroadcast(e.to_string()))?; + // --- 3. Broadcast the transaction, releasing the build's UTXO + // reservation if the broadcast is definitively rejected pre-send. --- + let txid = crate::wallet::reservations::broadcast_releasing_on_rejection( + self.broadcaster.as_ref(), + &self.wallet_manager, + &self.wallet_id, + key_wallet::account::account_type::StandardAccountType::BIP44Account, + 0, + &tx, + ) + .await?; tracing::info!( from_identity = %from_identity_id, diff --git a/packages/rs-platform-wallet/src/wallet/mod.rs b/packages/rs-platform-wallet/src/wallet/mod.rs index a6d10726fc1..edf707d8c83 100644 --- a/packages/rs-platform-wallet/src/wallet/mod.rs +++ b/packages/rs-platform-wallet/src/wallet/mod.rs @@ -6,6 +6,7 @@ pub mod persister; pub mod platform_addresses; pub mod platform_wallet; mod platform_wallet_traits; +pub(crate) mod reservations; #[cfg(feature = "shielded")] pub mod shielded; pub mod tokens; diff --git a/packages/rs-platform-wallet/src/wallet/reservations.rs b/packages/rs-platform-wallet/src/wallet/reservations.rs new file mode 100644 index 00000000000..fa41ad858da --- /dev/null +++ b/packages/rs-platform-wallet/src/wallet/reservations.rs @@ -0,0 +1,77 @@ +//! Broadcast-side UTXO reservation cleanup. +//! +//! `TransactionBuilder::build_signed` reserves the selected UTXOs in the +//! funding account's `ReservationSet` and leaves the reservation held on +//! success, expecting the transaction to be broadcast. When the broadcast +//! *fails* the reservation must be reconciled here: released for an immediate +//! retry when the transaction provably never reached the network, kept (for +//! the reservation-TTL backstop or a later sync) when it may have. +//! +//! Every build-then-broadcast path must go through +//! [`broadcast_releasing_on_rejection`] so the cleanup exists once instead of +//! per call site. + +use dashcore::{Transaction, Txid}; +use key_wallet::account::account_type::StandardAccountType; +use key_wallet_manager::WalletManager; +use tokio::sync::RwLock; + +use crate::broadcaster::{BroadcastError, TransactionBroadcaster}; +use crate::wallet::platform_wallet::{PlatformWalletInfo, WalletId}; +use crate::PlatformWalletError; + +/// Broadcast `tx` and reconcile the funding account's UTXO reservation on +/// failure. +/// +/// On [`BroadcastError::Rejected`] β€” the transaction provably never reached +/// the network β€” the inputs reserved by the preceding `build_signed` are +/// released so an immediate retry can reselect them instead of failing with +/// spurious insufficient funds until the reservation-TTL backstop. On +/// [`BroadcastError::MaybeSent`] the reservation is intentionally kept: +/// releasing inputs of a transaction that may already be propagating invites +/// a double-spend on retry. +/// +/// `account_type`/`account_index` identify the funding account whose +/// `ReservationSet` holds the inputs β€” the same account handed to +/// `set_funding` when the transaction was built. +pub(crate) async fn broadcast_releasing_on_rejection( + broadcaster: &B, + wallet_manager: &RwLock>, + wallet_id: &WalletId, + account_type: StandardAccountType, + account_index: u32, + tx: &Transaction, +) -> Result { + match broadcaster.broadcast(tx).await { + Ok(txid) => Ok(txid), + Err(e) => { + if matches!(e, BroadcastError::Rejected { .. }) { + // `release_reservation` takes `&self` and the manager map is + // untouched, so a read lock suffices β€” this cleanup does not + // serialize concurrent sends. + let wm = wallet_manager.read().await; + let account = + wm.get_wallet_and_info(wallet_id) + .and_then(|(_, info)| match account_type { + StandardAccountType::BIP44Account => info + .core_wallet + .bip44_managed_account_at_index(account_index), + StandardAccountType::BIP32Account => info + .core_wallet + .bip32_managed_account_at_index(account_index), + }); + match account { + Some(account) => account.release_reservation(tx), + None => tracing::warn!( + wallet_id = %hex::encode(wallet_id), + ?account_type, + account_index, + "could not release UTXO reservation after rejected broadcast: \ + wallet or funds account not found" + ), + } + } + Err(e.into()) + } + } +} From 12ab17f3736b96bceba7a65a2694007ba6fd135e Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Sat, 4 Jul 2026 07:15:22 +0700 Subject: [PATCH 17/21] fix(platform-wallet): untrack rejected asset locks and carry the unconfirmed-broadcast code across FFI Review follow-ups: - On BroadcastError::Rejected, create_funded_asset_lock_proof now untracks the Built lock (in-memory + persisted row via the changeset's removed set). With the funding reservation released for re-selection, leaving the row resumable would let a later resume_asset_lock re-broadcast a transaction whose inputs may already be re-spent. MaybeSent failures keep both the reservation and the resumable row. broadcast_releasing_on_rejection returns the typed BroadcastError so call sites with rejection-specific cleanup can branch on it; `?` still converts via From. - PlatformWalletError::TransactionBroadcastUnconfirmed now crosses the FFI boundary as a dedicated ErrorTransactionBroadcastUnconfirmed code (19) instead of flattening to ErrorUnknown, mirroring the shielded unconfirmed siblings, with the matching Swift PlatformWalletResultCode / PlatformWalletError cases so hosts can special-case the do-not-retry outcome. Co-Authored-By: Claude Fable 5 --- packages/rs-platform-wallet-ffi/src/error.rs | 38 +++++++++++++++++++ .../src/wallet/asset_lock/build.rs | 20 +++++++--- .../src/wallet/asset_lock/sync/tracking.rs | 25 ++++++++++++ .../src/wallet/reservations.rs | 11 ++++-- .../PlatformWallet/PlatformWalletResult.swift | 18 +++++++++ 5 files changed, 104 insertions(+), 8 deletions(-) diff --git a/packages/rs-platform-wallet-ffi/src/error.rs b/packages/rs-platform-wallet-ffi/src/error.rs index de1a6cb9441..60a8c38750f 100644 --- a/packages/rs-platform-wallet-ffi/src/error.rs +++ b/packages/rs-platform-wallet-ffi/src/error.rs @@ -124,6 +124,17 @@ pub enum PlatformWalletFFIResultCode { /// must NOT auto-retry β€” a retry would select different unreserved notes /// and could double-send if the original spend landed. ErrorShieldedSpendUnconfirmed = 18, + /// Maps `PlatformWalletError::TransactionBroadcastUnconfirmed`. A core + /// transaction broadcast (send-to-addresses, DashPay payment, or + /// asset-lock funding) failed with an AMBIGUOUS outcome β€” the transaction + /// may already be on the network (transport timeout after delivery, + /// partial peer send, or an internal multi-node retry whose earlier + /// attempt may have delivered). The wallet intentionally KEEPS the spent + /// inputs' UTXO reservation, so an immediate retry fails at input + /// selection instead of double-spending; the reservation TTL or a sync + /// observing the transaction reconciles the outcome. The host must NOT + /// auto-retry. Shielded sibling: [`Self::ErrorShieldedSpendUnconfirmed`]. + ErrorTransactionBroadcastUnconfirmed = 19, NotFound = 98, // Used exclusively for all the Option that are retuned as errors ErrorUnknown = 99, @@ -237,6 +248,12 @@ impl From for PlatformWalletFFIResult { PlatformWalletError::ShieldedSpendUnconfirmed { .. } => { PlatformWalletFFIResultCode::ErrorShieldedSpendUnconfirmed } + // The core-transaction sibling of the shielded pair above: the + // do-not-retry signal must survive the boundary as a typed code + // so hosts can distinguish it from a definitive rejection. + PlatformWalletError::TransactionBroadcastUnconfirmed(..) => { + PlatformWalletFFIResultCode::ErrorTransactionBroadcastUnconfirmed + } _ => PlatformWalletFFIResultCode::ErrorUnknown, }; PlatformWalletFFIResult::err(code, error.to_string()) @@ -595,6 +612,27 @@ mod tests { assert_eq!(msg, rendered, "Display payload must survive verbatim"); } + /// The ambiguous core-broadcast outcome keeps its typed code across the + /// boundary β€” flattening it to `ErrorUnknown` would erase the + /// do-not-retry signal the variant exists to carry. + #[test] + fn transaction_broadcast_unconfirmed_maps_to_dedicated_code() { + let unconfirmed = PlatformWalletError::TransactionBroadcastUnconfirmed( + "gRPC deadline exceeded after delivery".to_string(), + ); + let rendered = unconfirmed.to_string(); + let result: PlatformWalletFFIResult = unconfirmed.into(); + assert_eq!( + result.code, + PlatformWalletFFIResultCode::ErrorTransactionBroadcastUnconfirmed, + "TransactionBroadcastUnconfirmed should map to its dedicated code (rendered: {rendered})" + ); + let msg = unsafe { std::ffi::CStr::from_ptr(result.message) } + .to_string_lossy() + .into_owned(); + assert_eq!(msg, rendered, "Display payload must survive verbatim"); + } + /// Other wallet-error variants without a dedicated FFI arm still /// fall through to `ErrorUnknown` while carrying the typed /// Display rendering as the message. Pin this so the catch-all diff --git a/packages/rs-platform-wallet/src/wallet/asset_lock/build.rs b/packages/rs-platform-wallet/src/wallet/asset_lock/build.rs index 1e36bb2109e..d182c3bc680 100644 --- a/packages/rs-platform-wallet/src/wallet/asset_lock/build.rs +++ b/packages/rs-platform-wallet/src/wallet/asset_lock/build.rs @@ -347,10 +347,13 @@ impl AssetLockManager { // 3. Broadcast, releasing the build's UTXO reservation if the // broadcast is definitively rejected pre-send (the asset-lock - // builder funds from the BIP44 account at `account_index`). The - // tracked lock stays `Built`, so a later resume can still re-drive - // the same transaction while its inputs remain unspent. - crate::wallet::reservations::broadcast_releasing_on_rejection( + // builder funds from the BIP44 account at `account_index`). On + // rejection the `Built` row is untracked too: with the inputs + // released for re-selection, leaving it resumable would let a + // later `resume_asset_lock` re-broadcast a transaction whose + // inputs may already be re-spent. A `MaybeSent` failure keeps + // both the reservation and the resumable row. + if let Err(e) = crate::wallet::reservations::broadcast_releasing_on_rejection( self.broadcaster.as_ref(), &self.wallet_manager, &self.wallet_id, @@ -358,7 +361,14 @@ impl AssetLockManager { account_index, &tx, ) - .await?; + .await + { + if matches!(e, crate::broadcaster::BroadcastError::Rejected { .. }) { + let cs_untrack = self.untrack_asset_lock(&out_point).await; + self.queue_asset_lock_changeset(cs_untrack); + } + return Err(e.into()); + } // 4. Transition to Broadcast and queue the changeset. let cs_broadcast = self diff --git a/packages/rs-platform-wallet/src/wallet/asset_lock/sync/tracking.rs b/packages/rs-platform-wallet/src/wallet/asset_lock/sync/tracking.rs index a3611e32ccb..d3f619c67e2 100644 --- a/packages/rs-platform-wallet/src/wallet/asset_lock/sync/tracking.rs +++ b/packages/rs-platform-wallet/src/wallet/asset_lock/sync/tracking.rs @@ -25,6 +25,31 @@ impl AssetLockManager { cs } + /// Remove a tracked asset lock whose funding transaction was + /// definitively rejected at broadcast (never reached the network). + /// + /// Unlike [`consume_asset_lock`](Self::consume_asset_lock), the + /// persisted row is deleted too (via the changeset's `removed` set): + /// a rejected lock's funding transaction never existed on the + /// network, so the row has no historical value β€” and because the + /// rejection released the funding UTXO reservation, leaving the + /// `Built` row resumable would let a later `resume_asset_lock` + /// re-broadcast a transaction whose inputs may have been re-spent. + /// + /// Idempotent: returns an empty changeset if the outpoint is not + /// tracked. The caller queues the changeset (call sites live in + /// `asset_lock/build.rs`, inside the module). + pub(crate) async fn untrack_asset_lock(&self, out_point: &OutPoint) -> AssetLockChangeSet { + let mut wm = self.wallet_manager.write().await; + let mut cs = AssetLockChangeSet::default(); + if let Some(info) = wm.get_wallet_info_mut(&self.wallet_id) { + if info.tracked_asset_locks.remove(out_point).is_some() { + cs.removed.insert(*out_point); + } + } + cs + } + /// Mark a tracked asset lock as /// [`Consumed`](AssetLockStatus::Consumed) after a successful /// identity registration or top-up. diff --git a/packages/rs-platform-wallet/src/wallet/reservations.rs b/packages/rs-platform-wallet/src/wallet/reservations.rs index fa41ad858da..c2cb4e6ed85 100644 --- a/packages/rs-platform-wallet/src/wallet/reservations.rs +++ b/packages/rs-platform-wallet/src/wallet/reservations.rs @@ -18,7 +18,6 @@ use tokio::sync::RwLock; use crate::broadcaster::{BroadcastError, TransactionBroadcaster}; use crate::wallet::platform_wallet::{PlatformWalletInfo, WalletId}; -use crate::PlatformWalletError; /// Broadcast `tx` and reconcile the funding account's UTXO reservation on /// failure. @@ -34,6 +33,12 @@ use crate::PlatformWalletError; /// `account_type`/`account_index` identify the funding account whose /// `ReservationSet` holds the inputs β€” the same account handed to /// `set_funding` when the transaction was built. +/// +/// Returns the still-typed [`BroadcastError`] so callers with +/// rejection-specific cleanup of their own (e.g. the asset-lock flow +/// untracking its `Built` row) can branch on the variant; `?` converts it +/// into [`PlatformWalletError`](crate::PlatformWalletError) for everyone +/// else. pub(crate) async fn broadcast_releasing_on_rejection( broadcaster: &B, wallet_manager: &RwLock>, @@ -41,7 +46,7 @@ pub(crate) async fn broadcast_releasing_on_rejection Result { +) -> Result { match broadcaster.broadcast(tx).await { Ok(txid) => Ok(txid), Err(e) => { @@ -71,7 +76,7 @@ pub(crate) async fn broadcast_releasing_on_rejection Date: Sat, 4 Jul 2026 07:40:15 +0700 Subject: [PATCH 18/21] fix(platform-wallet): close the rejected-asset-lock race and pin both branches with tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups: - Untrack-before-release ordering: on a Rejected asset-lock broadcast the Built row is now removed BEFORE the funding reservation is released. While the reservation is held the inputs cannot be re-selected by a new build, and once the row is gone resume_asset_lock cannot re-drive the rejected transaction β€” so at no point is the row resumable while its inputs are re-spendable. untrack_asset_lock is additionally guarded on the row still being Built, so a concurrent resume that advanced it to Broadcast is kept rather than clobbered. The release block moved into release_reservation_after_rejected_broadcast, which broadcast_releasing_on_rejection now delegates to. - Regression tests for the asset-lock branch: a Rejected broadcast empties tracked_asset_locks, queues the outpoint in the changeset's removed set, and a fresh build reselects the released inputs; a MaybeSent failure keeps the Built row, queues no deletion, and a fresh build fails at input selection while the reservation is held. The mocks/signer/funded-wallet fixture moved to a shared cfg(test) test_support module used by both the core-send and asset-lock test suites. Co-Authored-By: Claude Fable 5 --- packages/rs-platform-wallet/src/lib.rs | 2 + .../rs-platform-wallet/src/test_support.rs | 183 +++++++++++++ .../src/wallet/asset_lock/build.rs | 249 ++++++++++++++++-- .../src/wallet/asset_lock/sync/tracking.rs | 21 +- .../src/wallet/core/broadcast.rs | 160 +---------- .../src/wallet/reservations.rs | 85 +++--- 6 files changed, 498 insertions(+), 202 deletions(-) create mode 100644 packages/rs-platform-wallet/src/test_support.rs diff --git a/packages/rs-platform-wallet/src/lib.rs b/packages/rs-platform-wallet/src/lib.rs index 289a71378fd..04d3ef3a2d2 100644 --- a/packages/rs-platform-wallet/src/lib.rs +++ b/packages/rs-platform-wallet/src/lib.rs @@ -19,6 +19,8 @@ pub mod error; pub mod events; pub mod manager; pub mod spv; +#[cfg(test)] +pub(crate) mod test_support; pub mod wallet; pub use error::PlatformWalletError; diff --git a/packages/rs-platform-wallet/src/test_support.rs b/packages/rs-platform-wallet/src/test_support.rs new file mode 100644 index 00000000000..4b7f525cd24 --- /dev/null +++ b/packages/rs-platform-wallet/src/test_support.rs @@ -0,0 +1,183 @@ +//! Shared unit-test scaffolding: mock broadcasters, a seed-backed signer, +//! and a funded in-memory wallet manager. +//! +//! Used by the broadcast-failure regression tests in `wallet::core::broadcast` +//! and `wallet::asset_lock::build`. + +use std::collections::BTreeMap; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::Arc; + +use async_trait::async_trait; +use dashcore::secp256k1::{ecdsa, Message, PublicKey, Secp256k1}; +use dashcore::{Network, Transaction, Txid}; +use key_wallet::account::account_type::StandardAccountType; +use key_wallet::signer::{Signer, SignerMethod}; +use key_wallet::test_utils::TestWalletContext; +use key_wallet::transaction_checking::TransactionContext; +use key_wallet::{DerivationPath, Wallet}; +use key_wallet_manager::WalletManager; +use tokio::sync::RwLock; + +use crate::broadcaster::{BroadcastError, TransactionBroadcaster}; +use crate::wallet::core::WalletBalance; +use crate::wallet::identity::IdentityManager; +use crate::wallet::platform_wallet::{PlatformWalletInfo, WalletId}; + +/// Broadcaster whose first call fails with a definitive pre-send rejection +/// and which succeeds afterwards, to model a transient broadcast error +/// followed by a user retry. +pub(crate) struct RejectFirstBroadcaster { + failed_once: AtomicBool, +} + +impl RejectFirstBroadcaster { + pub(crate) fn new() -> Self { + Self { + failed_once: AtomicBool::new(false), + } + } +} + +#[async_trait] +impl TransactionBroadcaster for RejectFirstBroadcaster { + async fn broadcast(&self, transaction: &Transaction) -> Result { + if self.failed_once.swap(true, Ordering::SeqCst) { + Ok(transaction.txid()) + } else { + Err(BroadcastError::Rejected { + reason: "simulated pre-send rejection".to_string(), + }) + } + } +} + +/// Broadcaster that always fails with a definitive pre-send rejection. +pub(crate) struct AlwaysRejectedBroadcaster; + +#[async_trait] +impl TransactionBroadcaster for AlwaysRejectedBroadcaster { + async fn broadcast(&self, _transaction: &Transaction) -> Result { + Err(BroadcastError::Rejected { + reason: "simulated pre-send rejection".to_string(), + }) + } +} + +/// Broadcaster that always fails with an *ambiguous* result β€” the network +/// may already have accepted the transaction β€” so its inputs must NOT be +/// released on failure. +pub(crate) struct AlwaysMaybeSentBroadcaster; + +#[async_trait] +impl TransactionBroadcaster for AlwaysMaybeSentBroadcaster { + async fn broadcast(&self, _transaction: &Transaction) -> Result { + Err(BroadcastError::MaybeSent { + reason: "simulated ambiguous broadcast".to_string(), + }) + } +} + +/// Soft signer that derives keys straight from a test wallet's seed. Stands +/// in for the FFI keychain-backed signer used in production. +pub(crate) struct WalletSigner { + wallet: Wallet, +} + +#[async_trait] +impl Signer for WalletSigner { + type Error = String; + + fn supported_methods(&self) -> &[SignerMethod] { + &[SignerMethod::Digest] + } + + async fn sign_ecdsa( + &self, + path: &DerivationPath, + sighash: [u8; 32], + ) -> Result<(ecdsa::Signature, PublicKey), Self::Error> { + let secp = Secp256k1::new(); + let key = self + .wallet + .derive_private_key(path) + .map_err(|e| e.to_string())?; + let message = Message::from_digest(sighash); + Ok(( + secp.sign_ecdsa(&message, &key), + PublicKey::from_secret_key(&secp, &key), + )) + } + + async fn public_key(&self, path: &DerivationPath) -> Result { + let secp = Secp256k1::new(); + let key = self + .wallet + .derive_private_key(path) + .map_err(|e| e.to_string())?; + Ok(PublicKey::from_secret_key(&secp, &key)) + } +} + +/// Builds a testnet wallet manager whose `account_type`/index-0 account +/// holds a single spendable UTXO (10_000_000 duffs) β€” the whole balance +/// rides on that one input, so a leaked reservation strands it. Returns +/// the manager, the wallet id, the shared balance handle, and a soft +/// signer over the wallet's seed. +pub(crate) async fn funded_wallet_manager( + account_type: StandardAccountType, +) -> ( + Arc>>, + WalletId, + Arc, + WalletSigner, +) { + let mut ctx = TestWalletContext::new_random(); + + // `new_random()` already derives a BIP44 receive address; only the + // BIP32 arm needs a hand-rolled derivation. + let receive_address = match account_type { + StandardAccountType::BIP44Account => ctx.receive_address.clone(), + StandardAccountType::BIP32Account => { + let xpub = ctx + .wallet + .accounts + .standard_bip32_accounts + .get(&0) + .expect("bip32 account") + .account_xpub; + ctx.managed_wallet + .first_bip32_managed_account_mut() + .expect("bip32 managed account") + .next_receive_address(Some(&xpub), true) + .expect("bip32 receive address") + } + }; + + let funding_tx = Transaction::dummy(&receive_address, 0..1, &[10_000_000]); + let result = ctx + .check_transaction(&funding_tx, TransactionContext::Mempool) + .await; + assert!( + result.is_relevant, + "funding tx should be relevant to {account_type:?}" + ); + assert!(result.is_new_transaction); + + let signer = WalletSigner { + wallet: ctx.wallet.clone(), + }; + + let balance = Arc::new(WalletBalance::new()); + let info = PlatformWalletInfo { + core_wallet: ctx.managed_wallet, + balance: Arc::clone(&balance), + 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, balance, signer) +} diff --git a/packages/rs-platform-wallet/src/wallet/asset_lock/build.rs b/packages/rs-platform-wallet/src/wallet/asset_lock/build.rs index d182c3bc680..bb62d0244c1 100644 --- a/packages/rs-platform-wallet/src/wallet/asset_lock/build.rs +++ b/packages/rs-platform-wallet/src/wallet/asset_lock/build.rs @@ -345,27 +345,27 @@ impl AssetLockManager { "Asset lock tracked as Built and queued for persistence; broadcasting." ); - // 3. Broadcast, releasing the build's UTXO reservation if the - // broadcast is definitively rejected pre-send (the asset-lock - // builder funds from the BIP44 account at `account_index`). On - // rejection the `Built` row is untracked too: with the inputs - // released for re-selection, leaving it resumable would let a - // later `resume_asset_lock` re-broadcast a transaction whose - // inputs may already be re-spent. A `MaybeSent` failure keeps - // both the reservation and the resumable row. - if let Err(e) = crate::wallet::reservations::broadcast_releasing_on_rejection( - self.broadcaster.as_ref(), - &self.wallet_manager, - &self.wallet_id, - key_wallet::account::account_type::StandardAccountType::BIP44Account, - account_index, - &tx, - ) - .await - { + // 3. Broadcast. On a definitive pre-send rejection, untrack the + // `Built` row BEFORE releasing the funding reservation (the + // asset-lock builder funds from the BIP44 account at + // `account_index`): while the reservation is held the inputs + // cannot be re-selected by a new build, and once the row is gone + // `resume_asset_lock` can no longer re-drive the rejected + // transaction β€” so at no point is the row resumable while its + // inputs are re-spendable. A `MaybeSent` failure keeps both the + // reservation and the resumable row. + if let Err(e) = self.broadcaster.broadcast(&tx).await { if matches!(e, crate::broadcaster::BroadcastError::Rejected { .. }) { let cs_untrack = self.untrack_asset_lock(&out_point).await; self.queue_asset_lock_changeset(cs_untrack); + crate::wallet::reservations::release_reservation_after_rejected_broadcast( + &self.wallet_manager, + &self.wallet_id, + key_wallet::account::account_type::StandardAccountType::BIP44Account, + account_index, + &tx, + ) + .await; } return Err(e.into()); } @@ -402,3 +402,216 @@ impl AssetLockManager { Ok((proof, path, out_point)) } } + +#[cfg(test)] +mod tests { + use std::sync::{Arc, Mutex}; + + use dashcore::OutPoint; + use key_wallet::account::account_type::StandardAccountType; + use tokio::sync::Notify; + + use crate::broadcaster::TransactionBroadcaster; + use crate::changeset::{ + ClientStartState, PersistenceError, PlatformWalletChangeSet, PlatformWalletPersistence, + }; + use crate::test_support::{ + funded_wallet_manager, AlwaysMaybeSentBroadcaster, AlwaysRejectedBroadcaster, WalletSigner, + }; + use crate::wallet::asset_lock::manager::AssetLockManager; + use crate::wallet::asset_lock::tracked::AssetLockStatus; + use crate::wallet::persister::WalletPersister; + use crate::wallet::platform_wallet::WalletId; + use crate::{AssetLockFundingType, PlatformWalletError}; + + /// Persistence stub that records every stored changeset so tests can + /// assert what the asset-lock flow queued. + #[derive(Default)] + struct CapturingPersistence { + stored: Mutex>, + } + + impl CapturingPersistence { + /// Outpoints queued for persisted-row deletion across all stored + /// changesets. + fn removed_outpoints(&self) -> Vec { + self.stored + .lock() + .expect("capturing persistence mutex") + .iter() + .filter_map(|cs| cs.asset_locks.as_ref()) + .flat_map(|al| al.removed.iter().copied()) + .collect() + } + } + + impl PlatformWalletPersistence for CapturingPersistence { + fn store( + &self, + _wallet_id: WalletId, + changeset: PlatformWalletChangeSet, + ) -> Result<(), PersistenceError> { + self.stored + .lock() + .expect("capturing persistence mutex") + .push(changeset); + Ok(()) + } + + fn flush(&self, _wallet_id: WalletId) -> Result<(), PersistenceError> { + Ok(()) + } + + fn load(&self) -> Result { + Ok(ClientStartState::default()) + } + } + + /// Builds an `AssetLockManager` over the shared BIP44-funded fixture. + async fn funded_asset_lock_manager( + broadcaster: Arc, + ) -> ( + Arc>, + WalletSigner, + Arc, + ) { + let (wallet_manager, wallet_id, _balance, signer) = + funded_wallet_manager(StandardAccountType::BIP44Account).await; + + let persistence = Arc::new(CapturingPersistence::default()); + let sdk = Arc::new(dash_sdk::SdkBuilder::new_mock().build().expect("mock sdk")); + let manager = Arc::new(AssetLockManager::new( + sdk, + wallet_manager, + wallet_id, + Arc::new(Notify::new()), + broadcaster, + WalletPersister::new( + wallet_id, + Arc::clone(&persistence) as Arc, + ), + )); + + (manager, signer, persistence) + } + + /// A definitively rejected asset-lock broadcast must untrack the `Built` + /// row (in-memory and via the changeset's `removed` set) and release the + /// funding reservation, so nothing can resume the dead transaction and a + /// fresh funding attempt can reselect the inputs immediately. + #[tokio::test] + async fn rejected_asset_lock_broadcast_untracks_row_and_releases_reservation() { + let (manager, signer, persistence) = + funded_asset_lock_manager(Arc::new(AlwaysRejectedBroadcaster)).await; + + let result = manager + .create_funded_asset_lock_proof( + 1_000_000, + 0, + AssetLockFundingType::IdentityRegistration, + 0, + &signer, + ) + .await; + assert!( + matches!(result, Err(PlatformWalletError::TransactionBroadcast(_))), + "rejected broadcast should surface as TransactionBroadcast, got {result:?}" + ); + + // The Built row is gone in memory… + { + let wm = manager.wallet_manager.read().await; + let (_, info) = wm + .get_wallet_and_info(&manager.wallet_id) + .expect("wallet still present"); + assert!( + info.tracked_asset_locks.is_empty(), + "rejected lock must be untracked, got {:?}", + info.tracked_asset_locks + ); + } + // …and its persisted row was queued for deletion. + assert_eq!( + persistence.removed_outpoints().len(), + 1, + "exactly the rejected lock's outpoint should be queued as removed" + ); + + // The funding reservation was released: a fresh build over the same + // single-UTXO wallet can reselect the inputs immediately. + let rebuild = manager + .build_asset_lock_transaction( + 1_000_000, + 0, + AssetLockFundingType::IdentityRegistration, + 0, + &signer, + ) + .await; + assert!( + rebuild.is_ok(), + "rebuild after a rejected broadcast should reselect the released \ + inputs, got {rebuild:?}" + ); + } + + /// An *ambiguous* asset-lock broadcast failure must keep both the funding + /// reservation and the resumable `Built` row: the transaction may already + /// be propagating, so a retry must not double-spend and a resume must + /// stay possible. + #[tokio::test] + async fn ambiguous_asset_lock_broadcast_keeps_reservation_and_built_row() { + let (manager, signer, persistence) = + funded_asset_lock_manager(Arc::new(AlwaysMaybeSentBroadcaster)).await; + + let result = manager + .create_funded_asset_lock_proof( + 1_000_000, + 0, + AssetLockFundingType::IdentityRegistration, + 0, + &signer, + ) + .await; + assert!( + matches!( + result, + Err(PlatformWalletError::TransactionBroadcastUnconfirmed(_)) + ), + "ambiguous broadcast should surface as TransactionBroadcastUnconfirmed, got {result:?}" + ); + + // The Built row survives for a later resume… + { + let wm = manager.wallet_manager.read().await; + let (_, info) = wm + .get_wallet_and_info(&manager.wallet_id) + .expect("wallet still present"); + assert_eq!(info.tracked_asset_locks.len(), 1); + let lock = info.tracked_asset_locks.values().next().expect("built row"); + assert_eq!(lock.status, AssetLockStatus::Built); + } + // …no persisted-row deletion was queued… + assert!( + persistence.removed_outpoints().is_empty(), + "ambiguous failure must not queue a row deletion" + ); + + // …and the reservation is kept: a fresh build cannot reselect the + // single reserved UTXO and fails at input selection. + let rebuild = manager + .build_asset_lock_transaction( + 1_000_000, + 0, + AssetLockFundingType::IdentityRegistration, + 0, + &signer, + ) + .await; + assert!( + matches!(rebuild, Err(PlatformWalletError::AssetLockTransaction(_))), + "rebuild must fail at input selection while the reservation is \ + kept, got {rebuild:?}" + ); + } +} diff --git a/packages/rs-platform-wallet/src/wallet/asset_lock/sync/tracking.rs b/packages/rs-platform-wallet/src/wallet/asset_lock/sync/tracking.rs index d3f619c67e2..1fef4fe1700 100644 --- a/packages/rs-platform-wallet/src/wallet/asset_lock/sync/tracking.rs +++ b/packages/rs-platform-wallet/src/wallet/asset_lock/sync/tracking.rs @@ -37,14 +37,27 @@ impl AssetLockManager { /// re-broadcast a transaction whose inputs may have been re-spent. /// /// Idempotent: returns an empty changeset if the outpoint is not - /// tracked. The caller queues the changeset (call sites live in - /// `asset_lock/build.rs`, inside the module). + /// tracked. Guarded on the row still being + /// [`Built`](AssetLockStatus::Built): if a concurrent flow advanced it + /// (e.g. a `resume_asset_lock` that re-broadcast in the window between + /// the rejected broadcast and this cleanup), the progress is kept + /// rather than clobbered. The caller queues the changeset (call sites + /// live in `asset_lock/build.rs`, inside the module). pub(crate) async fn untrack_asset_lock(&self, out_point: &OutPoint) -> AssetLockChangeSet { let mut wm = self.wallet_manager.write().await; let mut cs = AssetLockChangeSet::default(); if let Some(info) = wm.get_wallet_info_mut(&self.wallet_id) { - if info.tracked_asset_locks.remove(out_point).is_some() { - cs.removed.insert(*out_point); + match info.tracked_asset_locks.get(out_point) { + Some(entry) if entry.status == AssetLockStatus::Built => { + info.tracked_asset_locks.remove(out_point); + cs.removed.insert(*out_point); + } + Some(entry) => tracing::warn!( + outpoint = %out_point, + status = ?entry.status, + "untrack_asset_lock: lock advanced past Built concurrently β€” leaving it tracked" + ), + None => {} } } cs diff --git a/packages/rs-platform-wallet/src/wallet/core/broadcast.rs b/packages/rs-platform-wallet/src/wallet/core/broadcast.rs index c617f8e7d5e..a15ce6db8a3 100644 --- a/packages/rs-platform-wallet/src/wallet/core/broadcast.rs +++ b/packages/rs-platform-wallet/src/wallet/core/broadcast.rs @@ -161,166 +161,26 @@ impl CoreWallet { #[cfg(test)] mod tests { - use std::collections::BTreeMap; - use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; - use async_trait::async_trait; - use dashcore::secp256k1::{ecdsa, Message, PublicKey, Secp256k1}; - use dashcore::{Address as DashAddress, Network, Transaction, Txid}; + use dashcore::{Address as DashAddress, Network}; use key_wallet::account::account_type::StandardAccountType; - use key_wallet::signer::{Signer, SignerMethod}; - use key_wallet::test_utils::TestWalletContext; - use key_wallet::{DerivationPath, Wallet}; - use key_wallet_manager::WalletManager; - use tokio::sync::RwLock; - use crate::broadcaster::{BroadcastError, TransactionBroadcaster}; - use crate::wallet::core::{CoreWallet, WalletBalance}; - use crate::wallet::identity::IdentityManager; - use crate::wallet::platform_wallet::PlatformWalletInfo; + use crate::broadcaster::TransactionBroadcaster; + use crate::test_support::{ + funded_wallet_manager, AlwaysMaybeSentBroadcaster, RejectFirstBroadcaster, WalletSigner, + }; + use crate::wallet::core::CoreWallet; use crate::PlatformWalletError; - /// Broadcaster whose first call fails with a definitive pre-send - /// rejection and which succeeds afterwards, to model a transient - /// broadcast error followed by a user retry. - struct RejectFirstBroadcaster { - failed_once: AtomicBool, - } - - impl RejectFirstBroadcaster { - fn new() -> Self { - Self { - failed_once: AtomicBool::new(false), - } - } - } - - #[async_trait] - impl TransactionBroadcaster for RejectFirstBroadcaster { - async fn broadcast(&self, transaction: &Transaction) -> Result { - if self.failed_once.swap(true, Ordering::SeqCst) { - Ok(transaction.txid()) - } else { - Err(BroadcastError::Rejected { - reason: "simulated pre-send rejection".to_string(), - }) - } - } - } - - /// Broadcaster that always fails with an *ambiguous* result β€” the network - /// may already have accepted the transaction β€” so its inputs must NOT be - /// released on failure. - struct AlwaysMaybeSentBroadcaster; - - #[async_trait] - impl TransactionBroadcaster for AlwaysMaybeSentBroadcaster { - async fn broadcast(&self, _transaction: &Transaction) -> Result { - Err(BroadcastError::MaybeSent { - reason: "simulated ambiguous broadcast".to_string(), - }) - } - } - - /// Soft signer that derives keys straight from a test wallet's seed. Stands - /// in for the FFI keychain-backed signer used in production. - struct WalletSigner { - wallet: Wallet, - } - - #[async_trait] - impl Signer for WalletSigner { - type Error = String; - - fn supported_methods(&self) -> &[SignerMethod] { - &[SignerMethod::Digest] - } - - async fn sign_ecdsa( - &self, - path: &DerivationPath, - sighash: [u8; 32], - ) -> Result<(ecdsa::Signature, PublicKey), Self::Error> { - let secp = Secp256k1::new(); - let key = self - .wallet - .derive_private_key(path) - .map_err(|e| e.to_string())?; - let message = Message::from_digest(sighash); - Ok(( - secp.sign_ecdsa(&message, &key), - PublicKey::from_secret_key(&secp, &key), - )) - } - - async fn public_key(&self, path: &DerivationPath) -> Result { - let secp = Secp256k1::new(); - let key = self - .wallet - .derive_private_key(path) - .map_err(|e| e.to_string())?; - Ok(PublicKey::from_secret_key(&secp, &key)) - } - } - - /// Builds a testnet `CoreWallet` whose `account_type`/index-0 account holds a - /// single spendable UTXO (10_000_000 duffs) β€” the whole balance rides on that - /// one input, so a leaked reservation strands it. Returns the wallet, a soft - /// signer over its seed, and a 1_000_000-duff payment to a dummy recipient. + /// Builds a testnet `CoreWallet` over the shared funded fixture and 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)>) { - use key_wallet::transaction_checking::TransactionContext; - - let mut ctx = TestWalletContext::new_random(); - - // `new_random()` already derives a BIP44 receive address; only the - // BIP32 arm needs a hand-rolled derivation. - let receive_address = match account_type { - StandardAccountType::BIP44Account => ctx.receive_address.clone(), - StandardAccountType::BIP32Account => { - let xpub = ctx - .wallet - .accounts - .standard_bip32_accounts - .get(&0) - .expect("bip32 account") - .account_xpub; - ctx.managed_wallet - .first_bip32_managed_account_mut() - .expect("bip32 managed account") - .next_receive_address(Some(&xpub), true) - .expect("bip32 receive address") - } - }; - - let funding_tx = Transaction::dummy(&receive_address, 0..1, &[10_000_000]); - let result = ctx - .check_transaction(&funding_tx, TransactionContext::Mempool) - .await; - assert!( - result.is_relevant, - "funding tx should be relevant to {account_type:?}" - ); - assert!(result.is_new_transaction); - - let signer = WalletSigner { - wallet: ctx.wallet.clone(), - }; - - let balance = Arc::new(WalletBalance::new()); - let info = PlatformWalletInfo { - core_wallet: ctx.managed_wallet, - balance: Arc::clone(&balance), - 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"); - let wallet_manager = Arc::new(RwLock::new(wm)); + 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); diff --git a/packages/rs-platform-wallet/src/wallet/reservations.rs b/packages/rs-platform-wallet/src/wallet/reservations.rs index c2cb4e6ed85..096a7c2af12 100644 --- a/packages/rs-platform-wallet/src/wallet/reservations.rs +++ b/packages/rs-platform-wallet/src/wallet/reservations.rs @@ -9,7 +9,10 @@ //! //! Every build-then-broadcast path must go through //! [`broadcast_releasing_on_rejection`] so the cleanup exists once instead of -//! per call site. +//! per call site β€” except paths with rejection-specific cleanup of their own +//! that must run *before* the release (the asset-lock flow untracks its +//! `Built` row first); those call the broadcaster directly and then +//! [`release_reservation_after_rejected_broadcast`]. use dashcore::{Transaction, Txid}; use key_wallet::account::account_type::StandardAccountType; @@ -34,11 +37,8 @@ use crate::wallet::platform_wallet::{PlatformWalletInfo, WalletId}; /// `ReservationSet` holds the inputs β€” the same account handed to /// `set_funding` when the transaction was built. /// -/// Returns the still-typed [`BroadcastError`] so callers with -/// rejection-specific cleanup of their own (e.g. the asset-lock flow -/// untracking its `Built` row) can branch on the variant; `?` converts it -/// into [`PlatformWalletError`](crate::PlatformWalletError) for everyone -/// else. +/// Returns the still-typed [`BroadcastError`]; `?` converts it into +/// [`PlatformWalletError`](crate::PlatformWalletError) at the call sites. pub(crate) async fn broadcast_releasing_on_rejection( broadcaster: &B, wallet_manager: &RwLock>, @@ -51,32 +51,57 @@ pub(crate) async fn broadcast_releasing_on_rejection Ok(txid), Err(e) => { if matches!(e, BroadcastError::Rejected { .. }) { - // `release_reservation` takes `&self` and the manager map is - // untouched, so a read lock suffices β€” this cleanup does not - // serialize concurrent sends. - let wm = wallet_manager.read().await; - let account = - wm.get_wallet_and_info(wallet_id) - .and_then(|(_, info)| match account_type { - StandardAccountType::BIP44Account => info - .core_wallet - .bip44_managed_account_at_index(account_index), - StandardAccountType::BIP32Account => info - .core_wallet - .bip32_managed_account_at_index(account_index), - }); - match account { - Some(account) => account.release_reservation(tx), - None => tracing::warn!( - wallet_id = %hex::encode(wallet_id), - ?account_type, - account_index, - "could not release UTXO reservation after rejected broadcast: \ - wallet or funds account not found" - ), - } + release_reservation_after_rejected_broadcast( + wallet_manager, + wallet_id, + account_type, + account_index, + tx, + ) + .await; } Err(e) } } } + +/// Release the funding account's UTXO reservation for `tx` after its +/// broadcast came back [`BroadcastError::Rejected`]. +/// +/// Callers that pair the release with other rejection cleanup must order +/// that cleanup **before** this call when it removes state a concurrent +/// flow could act on β€” while the reservation is still held the inputs +/// cannot be re-selected by a new build, so the pre-release window is +/// safe. +pub(crate) async fn release_reservation_after_rejected_broadcast( + wallet_manager: &RwLock>, + wallet_id: &WalletId, + account_type: StandardAccountType, + account_index: u32, + tx: &Transaction, +) { + // `release_reservation` takes `&self` and the manager map is + // untouched, so a read lock suffices β€” this cleanup does not + // serialize concurrent sends. + let wm = wallet_manager.read().await; + let account = wm + .get_wallet_and_info(wallet_id) + .and_then(|(_, info)| match account_type { + StandardAccountType::BIP44Account => info + .core_wallet + .bip44_managed_account_at_index(account_index), + StandardAccountType::BIP32Account => info + .core_wallet + .bip32_managed_account_at_index(account_index), + }); + match account { + Some(account) => account.release_reservation(tx), + None => tracing::warn!( + wallet_id = %hex::encode(wallet_id), + ?account_type, + account_index, + "could not release UTXO reservation after rejected broadcast: \ + wallet or funds account not found" + ), + } +} From 5df222b15df8d20779fd58a58de697c8548fac08 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Sat, 4 Jul 2026 08:00:43 +0700 Subject: [PATCH 19/21] fix(platform-wallet): gate the rejected-asset-lock release on the Built row actually being removed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the untrack guard fires β€” a concurrent resume_asset_lock advanced the row past Built in the rejection window, positive evidence the transaction reached the network β€” the funding reservation must stay held exactly like a MaybeSent outcome, or the still-tracked row would be resumable while its inputs are re-spendable. Release now runs only when untrack_asset_lock actually removed the row (cs_untrack.removed.contains(&out_point)). Pinned by rejected_broadcast_racing_concurrent_resume_keeps_row_and_reservation: a mock broadcaster advances the row to Broadcast mid-broadcast and then returns Rejected; the test asserts the row survives as Broadcast, no persisted-row deletion is queued, and a fresh build still fails at input selection because the reservation was kept. Co-Authored-By: Claude Fable 5 --- .../src/wallet/asset_lock/build.rs | 140 ++++++++++++++++-- 1 file changed, 131 insertions(+), 9 deletions(-) diff --git a/packages/rs-platform-wallet/src/wallet/asset_lock/build.rs b/packages/rs-platform-wallet/src/wallet/asset_lock/build.rs index bb62d0244c1..887aa08dc53 100644 --- a/packages/rs-platform-wallet/src/wallet/asset_lock/build.rs +++ b/packages/rs-platform-wallet/src/wallet/asset_lock/build.rs @@ -357,15 +357,25 @@ impl AssetLockManager { if let Err(e) = self.broadcaster.broadcast(&tx).await { if matches!(e, crate::broadcaster::BroadcastError::Rejected { .. }) { let cs_untrack = self.untrack_asset_lock(&out_point).await; + // Release only when the Built row was actually removed. If + // the untrack guard fired instead β€” a concurrent + // `resume_asset_lock` advanced the row past `Built`, positive + // evidence the transaction reached the network after all β€” + // the inputs must stay reserved exactly like a `MaybeSent` + // outcome, or the still-tracked row would be resumable while + // its inputs are re-spendable. + let removed_built_row = cs_untrack.removed.contains(&out_point); self.queue_asset_lock_changeset(cs_untrack); - crate::wallet::reservations::release_reservation_after_rejected_broadcast( - &self.wallet_manager, - &self.wallet_id, - key_wallet::account::account_type::StandardAccountType::BIP44Account, - account_index, - &tx, - ) - .await; + if removed_built_row { + crate::wallet::reservations::release_reservation_after_rejected_broadcast( + &self.wallet_manager, + &self.wallet_id, + key_wallet::account::account_type::StandardAccountType::BIP44Account, + account_index, + &tx, + ) + .await; + } } return Err(e.into()); } @@ -411,7 +421,12 @@ mod tests { use key_wallet::account::account_type::StandardAccountType; use tokio::sync::Notify; - use crate::broadcaster::TransactionBroadcaster; + use async_trait::async_trait; + use dashcore::{Transaction, Txid}; + use key_wallet_manager::WalletManager; + use tokio::sync::RwLock; + + use crate::broadcaster::{BroadcastError, TransactionBroadcaster}; use crate::changeset::{ ClientStartState, PersistenceError, PlatformWalletChangeSet, PlatformWalletPersistence, }; @@ -421,6 +436,7 @@ mod tests { use crate::wallet::asset_lock::manager::AssetLockManager; use crate::wallet::asset_lock::tracked::AssetLockStatus; use crate::wallet::persister::WalletPersister; + use crate::wallet::platform_wallet::PlatformWalletInfo; use crate::wallet::platform_wallet::WalletId; use crate::{AssetLockFundingType, PlatformWalletError}; @@ -614,4 +630,110 @@ mod tests { kept, got {rebuild:?}" ); } + + /// Broadcaster that simulates the racing interleave the release gate + /// exists for: "during" the broadcast a concurrent `resume_asset_lock` + /// advances the tracked row to `Broadcast`, then the original call still + /// comes back `Rejected`. The advanced row is positive evidence the + /// transaction reached the network, so the cleanup must keep it AND keep + /// the funding reservation. + struct RejectAfterConcurrentResumeBroadcaster { + wallet_manager: Arc>>, + wallet_id: WalletId, + } + + #[async_trait] + impl TransactionBroadcaster for RejectAfterConcurrentResumeBroadcaster { + async fn broadcast(&self, _transaction: &Transaction) -> Result { + let mut wm = self.wallet_manager.write().await; + let info = wm + .get_wallet_info_mut(&self.wallet_id) + .expect("wallet present"); + let lock = info + .tracked_asset_locks + .values_mut() + .next() + .expect("Built row tracked before broadcast"); + lock.status = AssetLockStatus::Broadcast; + drop(wm); + Err(BroadcastError::Rejected { + reason: "simulated rejection racing a concurrent resume".to_string(), + }) + } + } + + /// If a concurrent resume advanced the row past `Built` in the rejection + /// window, the cleanup must keep the row (guard) AND keep the funding + /// reservation (release gate) β€” otherwise the still-tracked transaction + /// would be resumable while its inputs are re-spendable. + #[tokio::test] + async fn rejected_broadcast_racing_concurrent_resume_keeps_row_and_reservation() { + let (wallet_manager, wallet_id, _balance, signer) = + funded_wallet_manager(StandardAccountType::BIP44Account).await; + + let broadcaster = Arc::new(RejectAfterConcurrentResumeBroadcaster { + wallet_manager: Arc::clone(&wallet_manager), + wallet_id, + }); + let persistence = Arc::new(CapturingPersistence::default()); + let sdk = Arc::new(dash_sdk::SdkBuilder::new_mock().build().expect("mock sdk")); + let manager = Arc::new(AssetLockManager::new( + sdk, + Arc::clone(&wallet_manager), + wallet_id, + Arc::new(Notify::new()), + broadcaster, + WalletPersister::new( + wallet_id, + Arc::clone(&persistence) as Arc, + ), + )); + + let result = manager + .create_funded_asset_lock_proof( + 1_000_000, + 0, + AssetLockFundingType::IdentityRegistration, + 0, + &signer, + ) + .await; + assert!( + matches!(result, Err(PlatformWalletError::TransactionBroadcast(_))), + "rejection should still surface, got {result:?}" + ); + + // The concurrently-advanced row survives the cleanup… + { + let wm = wallet_manager.read().await; + let (_, info) = wm + .get_wallet_and_info(&manager.wallet_id) + .expect("wallet still present"); + assert_eq!(info.tracked_asset_locks.len(), 1); + let lock = info.tracked_asset_locks.values().next().expect("row kept"); + assert_eq!(lock.status, AssetLockStatus::Broadcast); + } + // …no persisted-row deletion was queued… + assert!( + persistence.removed_outpoints().is_empty(), + "advanced row must not be queued for deletion" + ); + + // …and the reservation was NOT released: a fresh build cannot + // reselect the single reserved UTXO. + let rebuild = manager + .build_asset_lock_transaction( + 1_000_000, + 0, + AssetLockFundingType::IdentityRegistration, + 0, + &signer, + ) + .await; + assert!( + matches!(rebuild, Err(PlatformWalletError::AssetLockTransaction(_))), + "rebuild must fail at input selection while the reservation is \ + kept for the advanced row, got {rebuild:?}" + ); + } } From 04ab5cde71dccf067d8c50b4240caeccfcf86a76 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Sat, 4 Jul 2026 15:51:41 +0700 Subject: [PATCH 20/21] fix(platform-wallet): reserve FFI code 19 for the v4.0-dev shielded-anchor error v4.0-dev already ships ErrorShieldedNoRecordedAnchor = 19; move ErrorTransactionBroadcastUnconfirmed to 20 (Rust enum + Swift mirror) so the FFI codes stay numerically identical across both release lines when the 4.0 -> 4.1 sync lands, following the ErrorArithmeticOverflow reserved-slot precedent. Co-Authored-By: Claude Fable 5 --- packages/rs-platform-wallet-ffi/src/error.rs | 7 ++++++- .../SwiftDashSDK/PlatformWallet/PlatformWalletResult.swift | 5 ++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/packages/rs-platform-wallet-ffi/src/error.rs b/packages/rs-platform-wallet-ffi/src/error.rs index 60a8c38750f..ddc90167fee 100644 --- a/packages/rs-platform-wallet-ffi/src/error.rs +++ b/packages/rs-platform-wallet-ffi/src/error.rs @@ -124,6 +124,11 @@ pub enum PlatformWalletFFIResultCode { /// must NOT auto-retry β€” a retry would select different unreserved notes /// and could double-send if the original spend landed. ErrorShieldedSpendUnconfirmed = 18, + // Slot 19 is `ErrorShieldedNoRecordedAnchor` on v4.0-dev, arriving here + // via the next v4.0-dev β†’ v4.1-dev sync. Skipping it keeps the FFI codes + // (and the Swift/Kotlin mirror enums) numerically identical across both + // release lines β€” the same convention as the `ErrorArithmeticOverflow` + // reserved slot above. /// Maps `PlatformWalletError::TransactionBroadcastUnconfirmed`. A core /// transaction broadcast (send-to-addresses, DashPay payment, or /// asset-lock funding) failed with an AMBIGUOUS outcome β€” the transaction @@ -134,7 +139,7 @@ pub enum PlatformWalletFFIResultCode { /// selection instead of double-spending; the reservation TTL or a sync /// observing the transaction reconciles the outcome. The host must NOT /// auto-retry. Shielded sibling: [`Self::ErrorShieldedSpendUnconfirmed`]. - ErrorTransactionBroadcastUnconfirmed = 19, + ErrorTransactionBroadcastUnconfirmed = 20, NotFound = 98, // Used exclusively for all the Option that are retuned as errors ErrorUnknown = 99, diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swift index c634868abf6..519d37101c1 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swift @@ -39,13 +39,16 @@ public enum PlatformWalletResultCode: Int32, Sendable { /// outcome. Do NOT auto-retry β€” a retry would rebuild the bundle and /// could double-execute if the original landed. case errorShieldedSpendUnconfirmed = 18 + // Raw value 19 is `errorShieldedNoRecordedAnchor` on v4.0-dev, arriving + // here via the next v4.0-dev β†’ v4.1-dev sync; skipped to keep this + // mirror numerically identical across both release lines. /// A core transaction broadcast (send, DashPay payment, or asset-lock /// funding) failed with an ambiguous outcome β€” the transaction may /// already be on the network. The wallet keeps the spent inputs' UTXO /// reservation, so an immediate retry fails at input selection instead /// of double-spending; the reservation TTL or a sync reconciles the /// outcome. Do NOT auto-retry. - case errorTransactionBroadcastUnconfirmed = 19 + case errorTransactionBroadcastUnconfirmed = 20 case notFound = 98 case errorUnknown = 99 From 79caf9b101e1b8070895320984b22d41c53bd956 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Sat, 4 Jul 2026 15:59:30 +0700 Subject: [PATCH 21/21] test(platform-wallet): pin the SPV broadcast Rejected/MaybeSent classification Extract the dash-spv error mapping into classify_spv_broadcast_error and pin all three arms at the crate boundary: unstarted client -> Rejected, NetworkError::NotConnected -> Rejected (dash-spv raises it from the zero-peers check before any bytes leave the process), and every other SpvError -> MaybeSent. A dash-spv semantic change now trips a test here instead of silently reopening the release-and-retry double-spend window. Co-Authored-By: Claude Fable 5 --- .../rs-platform-wallet/src/spv/runtime.rs | 114 ++++++++++++++++-- 1 file changed, 104 insertions(+), 10 deletions(-) diff --git a/packages/rs-platform-wallet/src/spv/runtime.rs b/packages/rs-platform-wallet/src/spv/runtime.rs index fa300662316..5f63ac27e19 100644 --- a/packages/rs-platform-wallet/src/spv/runtime.rs +++ b/packages/rs-platform-wallet/src/spv/runtime.rs @@ -33,6 +33,31 @@ pub struct SpvRuntime { client: RwLock>, task: Mutex>>, } +/// Classify a dash-spv broadcast failure per the +/// [`TransactionBroadcaster::broadcast`] contract. +/// +/// dash-spv's `broadcast_transaction` raises +/// `NetworkError::NotConnected` from its zero-connected-peers check +/// *before* handing the transaction to any peer, so it is the only +/// error safe to classify [`BroadcastError::Rejected`]; anything else +/// may follow a partial peer send and must stay +/// [`BroadcastError::MaybeSent`]. Pinned by the tests below so a +/// dash-spv semantic change is caught at this crate's boundary. +/// +/// [`TransactionBroadcaster::broadcast`]: crate::broadcaster::TransactionBroadcaster::broadcast +fn classify_spv_broadcast_error(error: dash_spv::error::SpvError) -> BroadcastError { + use dash_spv::error::{NetworkError, SpvError}; + + match error { + SpvError::Network(NetworkError::NotConnected) => BroadcastError::Rejected { + reason: "SPV broadcast failed: no connected peers".to_string(), + }, + other => BroadcastError::MaybeSent { + reason: format!("SPV broadcast failed: {}", other), + }, + } +} + // TODO: We want it better impl SpvRuntime { /// Create a new SPV runtime. @@ -105,8 +130,6 @@ impl SpvRuntime { &self, tx: &Transaction, ) -> Result<(), BroadcastError> { - use dash_spv::error::{NetworkError, SpvError}; - let client_guard = self.client.read().await; let client = client_guard.as_ref().ok_or(BroadcastError::Rejected { reason: "SPV client not started".to_string(), @@ -115,14 +138,7 @@ impl SpvRuntime { client .broadcast_transaction(tx) .await - .map_err(|e| match e { - SpvError::Network(NetworkError::NotConnected) => BroadcastError::Rejected { - reason: "SPV broadcast failed: no connected peers".to_string(), - }, - other => BroadcastError::MaybeSent { - reason: format!("SPV broadcast failed: {}", other), - }, - })?; + .map_err(classify_spv_broadcast_error)?; Ok(()) } @@ -303,3 +319,81 @@ impl std::fmt::Debug for SpvRuntime { .finish() } } + +#[cfg(test)] +mod tests { + use std::sync::Arc; + + use dash_spv::error::{NetworkError, SpvError}; + use dashcore::Network; + use key_wallet_manager::WalletManager; + use tokio::sync::RwLock; + + use super::{classify_spv_broadcast_error, SpvRuntime}; + use crate::broadcaster::BroadcastError; + use crate::events::PlatformEventManager; + use crate::wallet::platform_wallet::PlatformWalletInfo; + + /// A minimal valid transaction β€” the unstarted-client arm never + /// inspects it. + fn dummy_tx() -> dashcore::Transaction { + dashcore::Transaction { + version: 2, + lock_time: 0, + input: vec![], + output: vec![], + special_transaction_payload: None, + } + } + + /// An unstarted SPV client fails before any bytes leave the process, + /// so the failure must classify `Rejected` (safe to release the + /// transaction's input reservation). + #[tokio::test] + async fn broadcast_on_unstarted_client_is_rejected() { + let wallet_manager = Arc::new(RwLock::new(WalletManager::::new( + Network::Testnet, + ))); + let runtime = SpvRuntime::new(wallet_manager, Arc::new(PlatformEventManager::new(vec![]))); + + let result = runtime.broadcast_transaction(&dummy_tx()).await; + assert!( + matches!(result, Err(BroadcastError::Rejected { .. })), + "unstarted client must classify Rejected, got {result:?}" + ); + } + + /// dash-spv raises `NetworkError::NotConnected` from its + /// zero-connected-peers check before handing the transaction to any + /// peer, so it is the one client error safe to classify `Rejected`. + /// If dash-spv ever starts raising `NotConnected` after a partial + /// send, this pin must be revisited β€” releasing on a post-send + /// failure reopens the double-spend-on-retry window. + #[test] + fn not_connected_classifies_rejected() { + let result = classify_spv_broadcast_error(SpvError::Network(NetworkError::NotConnected)); + assert!( + matches!(result, BroadcastError::Rejected { .. }), + "NotConnected must classify Rejected, got {result:?}" + ); + } + + /// Every other dash-spv error may follow a partial peer send and must + /// stay `MaybeSent`, keeping the reservation for the TTL backstop. + #[test] + fn any_other_spv_error_classifies_maybe_sent() { + for error in [ + SpvError::Network(NetworkError::Timeout), + SpvError::Network(NetworkError::PeerDisconnected), + SpvError::Network(NetworkError::ConnectionFailed("reset by peer".to_string())), + SpvError::Config("bad config".to_string()), + ] { + let rendered = error.to_string(); + let result = classify_spv_broadcast_error(error); + assert!( + matches!(result, BroadcastError::MaybeSent { .. }), + "{rendered} must classify MaybeSent, got {result:?}" + ); + } + } +}