From c922addeaab5876f615b8c2ee721ee0e92b1ab15 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Sat, 8 Aug 2026 07:27:18 +0700 Subject: [PATCH 1/3] feat(platform-wallet): derive owner/voting provider keys Rust-side MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The secp256k1 provider families were the only key material the iOS app derived in Swift, and the only ones sourced from a throwaway key-wallet rebuilt from the mnemonic. Everything else — payments via `CoreTransactionBuilder`, identity/DPNS via the identity signer, and the BLS operator / Ed25519 platform-node provider keys via `derive_provider_key_at_index` — already derives from the running wallet on the Rust side. Owner and voting were on the odd path only because `ProviderKeyKind` never had variants for them. Adds `ProviderKeyKind::{Owner, Voting}` (account tags 9 / 8) with a secp256k1 branch: the public key and P2PKH address come off the account xpub (non-hardened, no seed needed, as with BLS operator keys), and the private key from `derive_from_seed_private_key_at`, which applies the DIP-3 account path exactly once. The branch runs the same seed-vs-xpub cross-check the operator family does. That guard is precisely what catches this class of bug: the two sides derive independently, and a mismatched (public, private) pair has no local symptom — it signs for an address nobody expects. On the voting family that surfaces only as Platform rejecting the vote as having no voter identity, because the voter identity is derived from the signing key's own hash160. `ProviderDerivedKey` / the FFI struct gain `address` and `private_key_wif`, both non-null only for the secp256k1 families. WIF is encoded Rust-side so the network byte and compression flag have one home. The WIF is zeroized on free like the hex scalar. Tests pin owner and voting derivation against explicit `m/9'/{5'|1'}/3'/{2'|1'}/index` paths at indexes 0/1/19 on both networks, plus a guard that the doubled path is not what we derive. Note: this is a `#[repr(C)]` change — consumers need a rebuilt xcframework. Co-Authored-By: Claude Opus 5 --- .../src/provider_key_at_index.rs | 59 +++++- .../src/wallet/provider_key_at_index.rs | 193 +++++++++++++++++- .../ManagedPlatformWallet.swift | 27 ++- 3 files changed, 270 insertions(+), 9 deletions(-) diff --git a/packages/rs-platform-wallet-ffi/src/provider_key_at_index.rs b/packages/rs-platform-wallet-ffi/src/provider_key_at_index.rs index 75c5265efe6..7a6954cb6b9 100644 --- a/packages/rs-platform-wallet-ffi/src/provider_key_at_index.rs +++ b/packages/rs-platform-wallet-ffi/src/provider_key_at_index.rs @@ -50,6 +50,10 @@ use rs_sdk_ffi::MnemonicResolverHandle; pub const PROVIDER_KEY_KIND_OPERATOR: u8 = 10; /// See [`PROVIDER_KEY_KIND_OPERATOR`]. pub const PROVIDER_KEY_KIND_PLATFORM_NODE: u8 = 11; +/// secp256k1 masternode voting keys (`ProviderVotingKeys`, tag 8). +pub const PROVIDER_KEY_KIND_VOTING: u8 = 8; +/// secp256k1 masternode owner keys (`ProviderOwnerKeys`, tag 9). +pub const PROVIDER_KEY_KIND_OWNER: u8 = 9; /// One provider key derived at a single index, in the hex forms the host /// renders. @@ -77,9 +81,20 @@ pub struct ProviderKeyAtIndexFFI { pub node_id_hex: *mut c_char, /// Null-terminated lowercase hex of the raw 32-byte private scalar /// (64 chars), populated only when `include_private` was set. BLS / - /// Ed25519 keys have no WIF, so this is the only private form. Null - /// otherwise; zeroized by the free function. + /// Ed25519 keys have no WIF, so for those this is the only private + /// form. Null otherwise; zeroized by the free function. pub private_key_hex: *mut c_char, + /// Null-terminated P2PKH address of the key. Non-null only for the + /// secp256k1 families (voting tag 8 / owner tag 9), whose keys appear + /// on-chain as the ProRegTx voting / owner addresses; null for BLS and + /// Ed25519 (no address form) and on the empty state. + pub address: *mut c_char, + /// Null-terminated WIF encoding of the private key, on the same terms + /// as `private_key_hex`: non-null only for the secp256k1 families, and + /// only when `include_private` was set. Encoded Rust-side so the + /// network byte and compression flag have one home. Zeroized by the + /// free function. + pub private_key_wif: *mut c_char, } impl ProviderKeyAtIndexFFI { @@ -92,6 +107,8 @@ impl ProviderKeyAtIndexFFI { legacy_public_key_hex: std::ptr::null_mut(), node_id_hex: std::ptr::null_mut(), private_key_hex: std::ptr::null_mut(), + address: std::ptr::null_mut(), + private_key_wif: std::ptr::null_mut(), } } } @@ -106,8 +123,10 @@ impl ProviderKeyAtIndexFFI { /// lacks resident private keys (see the module docs). May be null for /// an operator public listing on any wallet, or for a resident-key /// wallet. -/// - `kind` — [`PROVIDER_KEY_KIND_OPERATOR`] (BLS, tag 10) or -/// [`PROVIDER_KEY_KIND_PLATFORM_NODE`] (Ed25519, tag 11). +/// - `kind` — [`PROVIDER_KEY_KIND_OPERATOR`] (BLS, tag 10), +/// [`PROVIDER_KEY_KIND_PLATFORM_NODE`] (Ed25519, tag 11), +/// [`PROVIDER_KEY_KIND_VOTING`] (secp256k1, tag 8) or +/// [`PROVIDER_KEY_KIND_OWNER`] (secp256k1, tag 9). /// - `index` — the key index to derive. /// - `include_private` — also return the raw private scalar. /// - `out` — populated on success. Release with @@ -138,6 +157,8 @@ pub unsafe extern "C" fn platform_wallet_provider_key_at_index( let kind = match kind { PROVIDER_KEY_KIND_OPERATOR => ProviderKeyKind::Operator, PROVIDER_KEY_KIND_PLATFORM_NODE => ProviderKeyKind::PlatformNode, + PROVIDER_KEY_KIND_VOTING => ProviderKeyKind::Voting, + PROVIDER_KEY_KIND_OWNER => ProviderKeyKind::Owner, other => { return PlatformWalletFFIResult::err( PlatformWalletFFIResultCode::ErrorInvalidParameter, @@ -231,6 +252,8 @@ pub unsafe extern "C" fn platform_wallet_provider_key_at_index( legacy_public_key_bytes, node_id, private_key, + address, + private_key_wif, } = derived; let public_key_hex = unwrap_result_or_return!(CString::new(hex::encode(public_key_bytes))); @@ -255,6 +278,20 @@ pub unsafe extern "C" fn platform_wallet_provider_key_at_index( None => std::ptr::null_mut(), }; + // Public material — plain `CString::new`. + let address_ptr = match address { + Some(a) => unwrap_result_or_return!(CString::new(a)).into_raw(), + None => std::ptr::null_mut(), + }; + // Secret: same `secret_string_into_raw` marshalling as the hex form, so + // the WIF never leaves an un-zeroized plaintext copy on the heap. + let private_key_wif_ptr = match private_key_wif { + Some(wif) => { + unwrap_result_or_return!(crate::address_private_key::secret_string_into_raw(wif)) + } + None => std::ptr::null_mut(), + }; + unsafe { *out = ProviderKeyAtIndexFFI { index, @@ -262,6 +299,8 @@ pub unsafe extern "C" fn platform_wallet_provider_key_at_index( legacy_public_key_hex, node_id_hex, private_key_hex, + address: address_ptr, + private_key_wif: private_key_wif_ptr, }; } PlatformWalletFFIResult::ok() @@ -297,6 +336,18 @@ pub unsafe extern "C" fn platform_wallet_provider_key_at_index_free( let _ = unsafe { CString::from_raw(out.node_id_hex) }; out.node_id_hex = std::ptr::null_mut(); } + // The address is public material — free without scrubbing. + if !out.address.is_null() { + let _ = unsafe { CString::from_raw(out.address) }; + out.address = std::ptr::null_mut(); + } + // The WIF encodes the private key — zeroize on the same terms as the + // hex form below. + if !out.private_key_wif.is_null() { + let mut bytes = unsafe { CString::from_raw(out.private_key_wif) }.into_bytes_with_nul(); + bytes.zeroize(); + out.private_key_wif = std::ptr::null_mut(); + } // The private-key hex is sensitive — zeroize its bytes before free. if !out.private_key_hex.is_null() { let mut bytes = unsafe { CString::from_raw(out.private_key_hex) }.into_bytes_with_nul(); diff --git a/packages/rs-platform-wallet/src/wallet/provider_key_at_index.rs b/packages/rs-platform-wallet/src/wallet/provider_key_at_index.rs index 54f579e3247..961246600bb 100644 --- a/packages/rs-platform-wallet/src/wallet/provider_key_at_index.rs +++ b/packages/rs-platform-wallet/src/wallet/provider_key_at_index.rs @@ -73,7 +73,9 @@ //! The seed and any returned scalar are wrapped in [`Zeroizing`] so they //! are scrubbed when dropped. +use key_wallet::account::derivation::AccountDerivation; use key_wallet::account::{AccountType, BLSAccount, EdDSAAccount}; +use key_wallet::managed_account::address_pool::AddressPoolType; use zeroize::Zeroizing; use super::platform_wallet::PlatformWallet; @@ -285,6 +287,12 @@ pub enum ProviderKeyKind { /// Ed25519 platform-node keys /// ([`AccountType::ProviderPlatformKeys`], FFI tag 11). PlatformNode, + /// secp256k1 masternode owner keys + /// ([`AccountType::ProviderOwnerKeys`], FFI tag 9). + Owner, + /// secp256k1 masternode voting keys + /// ([`AccountType::ProviderVotingKeys`], FFI tag 8). + Voting, } impl ProviderKeyKind { @@ -293,6 +301,8 @@ impl ProviderKeyKind { match self { ProviderKeyKind::Operator => AccountType::ProviderOperatorKeys, ProviderKeyKind::PlatformNode => AccountType::ProviderPlatformKeys, + ProviderKeyKind::Owner => AccountType::ProviderOwnerKeys, + ProviderKeyKind::Voting => AccountType::ProviderVotingKeys, } } } @@ -323,9 +333,19 @@ pub struct ProviderDerivedKey { /// the raw 48-byte BLS public key, not a hash). pub node_id: Option<[u8; 20]>, /// Raw private-key scalar (32 bytes), present only when the caller - /// asked for it. BLS / Ed25519 keys have no WIF, so this is the - /// only private form. Zeroized on drop. + /// asked for it. BLS / Ed25519 keys have no WIF, so for those this is + /// the only private form. Zeroized on drop. pub private_key: Option>>, + /// The key's P2PKH address. `Some` only for the secp256k1 families + /// ([`ProviderKeyKind::Owner`] / [`ProviderKeyKind::Voting`]), whose + /// keys appear on-chain as the ProRegTx owner / voting addresses; + /// `None` for BLS / Ed25519, which have no address form. + pub address: Option, + /// WIF encoding of `private_key`, on the same terms: `Some` only for + /// the secp256k1 families and only when a private key was requested. + /// Encoded here rather than by the caller so the network byte and + /// compression flag have one home. Zeroized on drop. + pub private_key_wif: Option>, } /// The operator seed↔xpub cross-check guard, factored out so it can be @@ -493,6 +513,9 @@ impl PlatformWallet { legacy_public_key_bytes, node_id: None, private_key, + // BLS keys have no address or WIF form. + address: None, + private_key_wif: None, }) } ProviderKeyKind::PlatformNode => { @@ -546,6 +569,86 @@ impl PlatformWallet { legacy_public_key_bytes: None, node_id: Some(node_id), private_key, + // Ed25519 platform-node keys have no address or WIF form. + address: None, + private_key_wif: None, + }) + } + ProviderKeyKind::Owner | ProviderKeyKind::Voting => { + let account = state + .wallet() + .accounts + .account_of_type(account_type) + .ok_or_else(|| { + PlatformWalletError::AddressNotFound(format!( + "wallet has no secp256k1 {} account", + match kind { + ProviderKeyKind::Owner => "provider-owner-keys", + _ => "provider-voting-keys", + } + )) + })?; + + // Provider key accounts hold one pool with no + // internal/external split, derived at a non-hardened child + // index — so the public side comes off the account xpub and + // needs no seed, exactly like the BLS operator family. + let public_key = account + .derive_public_key_at(AddressPoolType::Absent, index, None) + .map_err(|e| { + PlatformWalletError::KeyDerivation(format!( + "failed to derive provider public key at index {index}: {e}" + )) + })?; + let address = account + .derive_address_at(AddressPoolType::Absent, index, None) + .map_err(|e| { + PlatformWalletError::KeyDerivation(format!( + "failed to derive provider address at index {index}: {e}" + )) + })?; + + let (private_key, private_key_wif) = match &seed { + Some(seed) => { + // Seed-derived, so it applies the account's DIP-3 path + // exactly once. Cross-checked against the xpub-derived + // public key below for the same reason the operator + // family is: the two derive independently, and a pair + // that disagrees signs for an address nobody expects — + // which on the voting family means Platform rejects the + // vote as having no voter identity, with no local + // symptom at all. + let private = account + .derive_from_seed_private_key_at(seed.as_ref(), index) + .map_err(|e| { + PlatformWalletError::KeyDerivation(format!( + "failed to derive provider private key at index {index}: {e}" + )) + })?; + let derived_public = private.public_key(&dashcore::key::Secp256k1::new()); + if derived_public != public_key { + return Err(PlatformWalletError::KeyDerivation(format!( + "provider key at index {index} is inconsistent: the seed-derived \ + private key's public key does not match the account xpub's" + ))); + } + ( + Some(Zeroizing::new(private.inner.secret_bytes().to_vec())), + Some(Zeroizing::new(private.to_wif())), + ) + } + None => (None, None), + }; + + Ok(ProviderDerivedKey { + index, + public_key_bytes: public_key.to_bytes(), + // secp256k1 keys have no BLS legacy variant. + legacy_public_key_bytes: None, + node_id: None, + private_key, + address: Some(address.to_string()), + private_key_wif, }) } } @@ -884,4 +987,90 @@ mod tests { "highest_generated must advance to the last populated index" ); } + + /// The secp256k1 (owner / voting) families must derive at the DIP-3 + /// account path applied EXACTLY once: `m/9'/coin'/3'/{2'|1'}/index`. + /// + /// This is the invariant the Swift key-wallet route silently violated — + /// it pre-derived the account root and handed that in as the "master", + /// so the account path was applied twice and every owner/voting key came + /// from `m/9'/5'/3'/1'/9'/5'/3'/1'/index`. Nothing failed locally: the + /// key was well-formed and deterministic, and only a counterparty + /// holding the real key could tell (Platform rejected the masternode + /// vote as having no voter identity, because the voter identity is + /// derived from the signing key's own hash160). + #[test] + fn ecdsa_provider_keys_derive_at_the_dip3_path() { + use key_wallet::bip32::{DerivationPath, ExtendedPrivKey}; + use std::str::FromStr; + + for (network, coin) in [(Network::Mainnet, "5'"), (Network::Testnet, "1'")] { + let wallet = seed_bearing_wallet(network); + let seed = wallet.wallet_seed_bytes().expect("resident seed"); + let master = ExtendedPrivKey::new_master(network, &seed).expect("master xpriv"); + let secp = dashcore::key::Secp256k1::new(); + + for (kind, family) in [ + (ProviderKeyKind::Owner, "2'"), + (ProviderKeyKind::Voting, "1'"), + ] { + let account = wallet + .accounts + .account_of_type(kind.account_type()) + .expect("provider account"); + + for index in [0u32, 1, 19] { + let derived = account + .derive_from_seed_private_key_at(&seed, index) + .expect("account-based derivation"); + + let path = + DerivationPath::from_str(&format!("m/9'/{coin}/3'/{family}/{index}")) + .expect("explicit DIP-3 path"); + let expected = master.derive_priv(&secp, &path).expect("path derivation"); + + assert_eq!( + derived.inner.secret_bytes(), + expected.private_key.secret_bytes(), + "{kind:?} key at index {index} on {network:?} must come from \ + m/9'/{coin}/3'/{family}/{index}" + ); + } + } + } + } + + /// The doubled path must NOT be what we derive. Guards the specific + /// regression rather than merely asserting the correct value, so a future + /// change that reintroduces pre-derivation fails here explicitly. + #[test] + fn ecdsa_provider_keys_do_not_double_apply_the_account_path() { + use key_wallet::bip32::{DerivationPath, ExtendedPrivKey}; + use std::str::FromStr; + + let wallet = seed_bearing_wallet(Network::Mainnet); + let seed = wallet.wallet_seed_bytes().expect("resident seed"); + let account = wallet + .accounts + .account_of_type(ProviderKeyKind::Voting.account_type()) + .expect("voting account"); + + let derived = account + .derive_from_seed_private_key_at(&seed, 19) + .expect("account-based derivation"); + + let master = ExtendedPrivKey::new_master(Network::Mainnet, &seed).expect("master"); + let doubled = master + .derive_priv( + &dashcore::key::Secp256k1::new(), + &DerivationPath::from_str("m/9'/5'/3'/1'/9'/5'/3'/1'/19").expect("doubled path"), + ) + .expect("doubled derivation"); + + assert_ne!( + derived.inner.secret_bytes(), + doubled.private_key.secret_bytes(), + "the account derivation path is being applied twice again" + ); + } } diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/ManagedPlatformWallet.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/ManagedPlatformWallet.swift index 73bb1f8db73..b3c6595a5fd 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/ManagedPlatformWallet.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/ManagedPlatformWallet.swift @@ -932,6 +932,10 @@ extension ManagedPlatformWallet { case operatorBLS = 10 /// Ed25519 platform-node keys (`ProviderPlatformKeys`, tag 11). case platformNodeEdDSA = 11 + /// secp256k1 masternode voting keys (`ProviderVotingKeys`, tag 8). + case votingECDSA = 8 + /// secp256k1 masternode owner keys (`ProviderOwnerKeys`, tag 9). + case ownerECDSA = 9 } /// One provider key derived at a single index, in the hex forms the @@ -955,8 +959,17 @@ extension ManagedPlatformWallet { public let nodeIdHex: String? /// Lowercase hex of the raw 32-byte private scalar, present only /// when the reveal requested it. BLS / Ed25519 keys have no WIF, - /// so this is the only private form. + /// so for those this is the only private form. public let privateKeyHex: String? + /// P2PKH address of the key. Non-nil only for the secp256k1 + /// families (``ProviderKeyKind/votingECDSA`` / + /// ``ProviderKeyKind/ownerECDSA``), whose keys appear on-chain as + /// the ProRegTx voting / owner addresses. + public let address: String? + /// WIF encoding of the private key, on the same terms as + /// ``privateKeyHex``. Encoded on the Rust side so the network byte + /// and compression flag have one home. + public let privateKeyWIF: String? /// Public memberwise init so hosts can build display rows from the /// persisted platform-node core-address rows (typed @@ -968,13 +981,17 @@ extension ManagedPlatformWallet { publicKeyHex: String, legacyPublicKeyHex: String?, nodeIdHex: String?, - privateKeyHex: String? + privateKeyHex: String?, + address: String? = nil, + privateKeyWIF: String? = nil ) { self.index = index self.publicKeyHex = publicKeyHex self.legacyPublicKeyHex = legacyPublicKeyHex self.nodeIdHex = nodeIdHex self.privateKeyHex = privateKeyHex + self.address = address + self.privateKeyWIF = privateKeyWIF } } @@ -1035,12 +1052,16 @@ extension ManagedPlatformWallet { let legacyPublicKeyHex = out.legacy_public_key_hex.map { String(cString: $0) } let nodeIdHex = out.node_id_hex.map { String(cString: $0) } let privateKeyHex = out.private_key_hex.map { String(cString: $0) } + let address = out.address.map { String(cString: $0) } + let privateKeyWIF = out.private_key_wif.map { String(cString: $0) } return ProviderDerivedKey( index: out.index, publicKeyHex: publicKeyHex, legacyPublicKeyHex: legacyPublicKeyHex, nodeIdHex: nodeIdHex, - privateKeyHex: privateKeyHex + privateKeyHex: privateKeyHex, + address: address, + privateKeyWIF: privateKeyWIF ) } } From db24fe30308b1740789375da6074d62d263b354e Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Sat, 8 Aug 2026 07:51:24 +0700 Subject: [PATCH 2/3] fix(platform-wallet): derive ECDSA provider keys without the watch-only gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Account::derive_from_seed_private_key_at` gates on `is_watch_only`, so it refused exactly the external-signable wallets the seed argument exists to serve — the iOS app's shape, where the seed lives in the Keychain and the caller resolves it on demand. On device: "Watch-only wallet: private keys not available". The module docs already warned about this asymmetry and are why the BLS and Ed25519 families use key-wallet's gate-free #881 entry points. secp256k1 has no such entry point, so derive inline in the same shape as `BLSAccount::operator_private_key_at`: raw seed → master xpriv → the account type's own DIP-3 path → non-hardened child. The path comes from the account type, so it is still applied exactly once. The existing tests passed throughout because they build seed-bearing wallets and never reach the gate. Adds a watch-only test that pins both halves: the gated wrapper is expected to refuse such an account, and the gate-free derivation must still match m/9'/5'/3'/{2'|1'}/index. Co-Authored-By: Claude Opus 5 --- .../src/wallet/provider_key_at_index.rs | 126 ++++++++++++++++-- 1 file changed, 115 insertions(+), 11 deletions(-) diff --git a/packages/rs-platform-wallet/src/wallet/provider_key_at_index.rs b/packages/rs-platform-wallet/src/wallet/provider_key_at_index.rs index 961246600bb..2152aca0761 100644 --- a/packages/rs-platform-wallet/src/wallet/provider_key_at_index.rs +++ b/packages/rs-platform-wallet/src/wallet/provider_key_at_index.rs @@ -75,6 +75,7 @@ use key_wallet::account::derivation::AccountDerivation; use key_wallet::account::{AccountType, BLSAccount, EdDSAAccount}; +use key_wallet::bip32::{ChildNumber, ExtendedPrivKey}; use key_wallet::managed_account::address_pool::AddressPoolType; use zeroize::Zeroizing; @@ -610,21 +611,49 @@ impl PlatformWallet { let (private_key, private_key_wif) = match &seed { Some(seed) => { - // Seed-derived, so it applies the account's DIP-3 path - // exactly once. Cross-checked against the xpub-derived - // public key below for the same reason the operator - // family is: the two derive independently, and a pair - // that disagrees signs for an address nobody expects — - // which on the voting family means Platform rejects the - // vote as having no voter identity, with no local - // symptom at all. - let private = account - .derive_from_seed_private_key_at(seed.as_ref(), index) + // Derived inline rather than through + // `Account::derive_from_seed_private_key_at`, which + // gates on `is_watch_only` and so refuses exactly the + // external-signable wallets this seed argument exists + // to serve (see the module docs on the BLS / Ed25519 + // families using the gate-free #881 entry points for + // the same reason; secp256k1 has no such entry point). + // + // Shape mirrors `BLSAccount::operator_private_key_at`: + // raw seed → master xpriv → the account's own DIP-3 + // path → non-hardened child `index`. The account path + // is resolved from the account type, so it is applied + // exactly once. + let master = + ExtendedPrivKey::new_master(network, seed.as_ref()).map_err(|e| { + PlatformWalletError::KeyDerivation(format!( + "failed to build master xpriv: {e}" + )) + })?; + let secp = dashcore::key::Secp256k1::new(); + let account_path = account_type.derivation_path(network).map_err(|e| { + PlatformWalletError::KeyDerivation(format!( + "failed to resolve the provider account path: {e}" + )) + })?; + let child = ChildNumber::from_normal_idx(index).map_err(|e| { + PlatformWalletError::KeyDerivation(format!( + "invalid provider key index {index}: {e}" + )) + })?; + let private = master + .derive_priv(&secp, &account_path) + .and_then(|acct| { + let child_path: key_wallet::bip32::DerivationPath = + vec![child].into(); + acct.derive_priv(&secp, &child_path) + }) .map_err(|e| { PlatformWalletError::KeyDerivation(format!( "failed to derive provider private key at index {index}: {e}" )) - })?; + })? + .to_priv(); let derived_public = private.public_key(&dashcore::key::Secp256k1::new()); if derived_public != public_key { return Err(PlatformWalletError::KeyDerivation(format!( @@ -1073,4 +1102,79 @@ mod tests { "the account derivation path is being applied twice again" ); } + + /// A WATCH-ONLY account must still derive its secp256k1 private key from + /// a supplied seed. + /// + /// This is the shape the iOS app actually runs: the wallet is + /// external-signable, its seed lives in the Keychain, and the caller + /// resolves it on demand. The first cut of this branch went through + /// `Account::derive_from_seed_private_key_at`, which gates on + /// `is_watch_only` and so refused exactly the wallets the seed argument + /// exists to serve — "Watch-only wallet: private keys not available". + /// + /// The seed-bearing tests above passed the whole time, which is the point + /// of this one: they never exercised the gate. + #[test] + fn ecdsa_provider_keys_derive_for_a_watch_only_account() { + use key_wallet::bip32::{DerivationPath, ExtendedPrivKey}; + use std::str::FromStr; + + let wallet = seed_bearing_wallet(Network::Mainnet); + let seed = wallet.wallet_seed_bytes().expect("resident seed"); + let secp = dashcore::key::Secp256k1::new(); + let master = ExtendedPrivKey::new_master(Network::Mainnet, &seed).expect("master"); + + for (kind, family) in [ + (ProviderKeyKind::Owner, "2'"), + (ProviderKeyKind::Voting, "1'"), + ] { + let account_type = kind.account_type(); + let watch_only = wallet + .accounts + .account_of_type(account_type) + .expect("provider account") + .to_watch_only(); + assert!( + watch_only.is_watch_only, + "precondition: account is watch-only" + ); + + // The gated wrapper refuses this account — pinned so a future + // change back to it fails here rather than on a device. + assert!( + watch_only + .derive_from_seed_private_key_at(&seed, 19) + .is_err(), + "the gated wrapper is expected to refuse a watch-only account" + ); + + // The path this module actually uses is gate-free and must agree + // with the explicit DIP-3 path. + let account_path = account_type + .derivation_path(Network::Mainnet) + .expect("account path"); + let child = ChildNumber::from_normal_idx(19).expect("child index"); + let child_path: DerivationPath = vec![child].into(); + let derived = master + .derive_priv(&secp, &account_path) + .and_then(|acct| acct.derive_priv(&secp, &child_path)) + .expect("gate-free derivation") + .to_priv(); + + let expected = master + .derive_priv( + &secp, + &DerivationPath::from_str(&format!("m/9'/5'/3'/{family}/19")) + .expect("explicit path"), + ) + .expect("path derivation"); + + assert_eq!( + derived.inner.secret_bytes(), + expected.private_key.secret_bytes(), + "{kind:?} watch-only derivation must match m/9'/5'/3'/{family}/19" + ); + } + } } From 61bf72841fc4adf5f05ac480fcc1ff42240f2202 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Sat, 8 Aug 2026 15:42:32 +0700 Subject: [PATCH 3/3] fix(platform-wallet): resolve the provider path from the account; wipe the scalar MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups on the secp256k1 provider arm. **Path source (found by the new end-to-end tests).** The private side resolved the account path from `account_type.derivation_path(network)` using the WALLET's network, while the public side comes off `account.account_xpub`, built from the ACCOUNT's own path and network. Two independent sources for one path: they disagree the moment those networks differ, and the cross-check then rejected a correct seed. Now taken from `account.derivation_path()`, so the sides agree by construction and the cross-check verifies the seed rather than the path. This is exactly the gap review flagged: the path tests re-derived the private side and never called `derive_provider_key_at_index`, so nothing compared it against the account xpub. **Scalar wiping (P2).** `dashcore::PrivateKey` is `Copy` with no `Drop`, so unlike the `Zeroizing` outputs nothing scrubbed it. Both copies are now erased with `non_secure_erase` — the `PrivateKey` and the extended key it came from — after the outputs are produced and before returning. **Invalid-kind diagnostic (P3).** Now names all four accepted kinds instead of only operator and platform node. **Tests.** New `provider_ecdsa_key_tests` drives the real entry point: a public listing carries an address and no private material; a private reveal is internally consistent (WIF ↔ scalar ↔ public key ↔ P2PKH on the wallet's own network); indexes and the two families produce distinct keys; a foreign seed is refused by the cross-check; and an explicitly supplied seed matches the resident one, which is the external-signable shape the app runs. They also document that `derive_provider_key_at_index` is synchronous and takes the manager lock with `blocking_read`, so it must be called off the async executor — the tests use `block_in_place`, as a caller must. Co-Authored-By: Claude Opus 5 --- .../src/provider_key_at_index.rs | 6 +- packages/rs-platform-wallet/src/wallet/mod.rs | 2 + .../src/wallet/provider_ecdsa_key_tests.rs | 249 ++++++++++++++++++ .../src/wallet/provider_key_at_index.rs | 45 +++- 4 files changed, 289 insertions(+), 13 deletions(-) create mode 100644 packages/rs-platform-wallet/src/wallet/provider_ecdsa_key_tests.rs diff --git a/packages/rs-platform-wallet-ffi/src/provider_key_at_index.rs b/packages/rs-platform-wallet-ffi/src/provider_key_at_index.rs index 7a6954cb6b9..d25d32324d1 100644 --- a/packages/rs-platform-wallet-ffi/src/provider_key_at_index.rs +++ b/packages/rs-platform-wallet-ffi/src/provider_key_at_index.rs @@ -163,8 +163,10 @@ pub unsafe extern "C" fn platform_wallet_provider_key_at_index( return PlatformWalletFFIResult::err( PlatformWalletFFIResultCode::ErrorInvalidParameter, format!( - "unknown provider key kind {other} (expected {PROVIDER_KEY_KIND_OPERATOR} \ - operator or {PROVIDER_KEY_KIND_PLATFORM_NODE} platform node)" + "unknown provider key kind {other} (expected \ + {PROVIDER_KEY_KIND_VOTING} voting, {PROVIDER_KEY_KIND_OWNER} owner, \ + {PROVIDER_KEY_KIND_OPERATOR} operator or \ + {PROVIDER_KEY_KIND_PLATFORM_NODE} platform node)" ), ); } diff --git a/packages/rs-platform-wallet/src/wallet/mod.rs b/packages/rs-platform-wallet/src/wallet/mod.rs index 96e11a5ae67..f300c6656b3 100644 --- a/packages/rs-platform-wallet/src/wallet/mod.rs +++ b/packages/rs-platform-wallet/src/wallet/mod.rs @@ -7,6 +7,8 @@ pub mod persister; pub mod platform_addresses; pub mod platform_wallet; mod platform_wallet_traits; +#[cfg(test)] +mod provider_ecdsa_key_tests; pub mod provider_key_at_index; pub(crate) mod reservations; #[cfg(feature = "shielded")] diff --git a/packages/rs-platform-wallet/src/wallet/provider_ecdsa_key_tests.rs b/packages/rs-platform-wallet/src/wallet/provider_ecdsa_key_tests.rs new file mode 100644 index 00000000000..2e38a340120 --- /dev/null +++ b/packages/rs-platform-wallet/src/wallet/provider_ecdsa_key_tests.rs @@ -0,0 +1,249 @@ +//! End-to-end coverage for the secp256k1 arm of +//! [`PlatformWallet::derive_provider_key_at_index`]. +//! +//! The sibling unit tests in `provider_key_at_index.rs` pin the derivation +//! *paths* by re-deriving them from a bare key-wallet — useful, but they never +//! call the entry point the FFI calls, so `include_private`, the address / WIF +//! outputs, and the seed-vs-xpub cross-check went uncovered. That gap has +//! already cost once: the first cut of this arm used a wrapper that refuses +//! watch-only accounts, and every path test passed while the iOS app failed on +//! device, because those tests build seed-bearing wallets and never reach the +//! gate. +//! +//! These drive the real method. +//! +//! `derive_provider_key_at_index` is synchronous and takes the wallet-manager +//! lock with `blocking_read`, so it panics if called on an async executor +//! thread. The tests therefore run on a multi-thread runtime and call it +//! through `block_in_place` — which is also how a caller must treat it. + +use std::sync::Arc; + +use tokio::task::block_in_place; + +use dashcore::Network; +use key_wallet::account::StandardAccountType; + +use crate::test_support::{funded_wallet_manager, NoopTestPersister}; +use crate::wallet::platform_wallet::PlatformWallet; +use crate::wallet::provider_key_at_index::ProviderKeyKind; + +/// A `PlatformWallet` over a fresh, seed-bearing test wallet. +async fn platform_wallet() -> PlatformWallet { + let (wallet_manager, wallet_id, balance, _signer) = + funded_wallet_manager(StandardAccountType::BIP44Account).await; + let sdk = Arc::new(dash_sdk::Sdk::new_mock()); + let spv = Arc::new(crate::spv::SpvRuntime::new( + Arc::clone(&wallet_manager), + Arc::new(crate::events::PlatformEventManager::new(Vec::new())), + )); + PlatformWallet::new( + sdk, + wallet_id, + wallet_manager, + balance, + Arc::new(tokio::sync::Notify::new()), + Arc::new(NoopTestPersister) as Arc, + Arc::new(crate::broadcaster::SpvBroadcaster::new(spv)), + ) +} + +/// The network the test wallet was created on, so address expectations are not +/// hardcoded to one chain. +async fn wallet_network(wallet: &PlatformWallet) -> Network { + let manager = wallet.wallet_manager().read().await; + manager + .get_wallet(&wallet.wallet_id()) + .expect("wallet present") + .network +} + +/// A public listing must carry the address but no private material — the +/// secp256k1 public side comes off the account xpub and needs no seed at all. +#[tokio::test(flavor = "multi_thread")] +async fn public_listing_has_address_and_no_private_material() { + let wallet = platform_wallet().await; + + for kind in [ProviderKeyKind::Owner, ProviderKeyKind::Voting] { + let derived = block_in_place(|| wallet.derive_provider_key_at_index(kind, 0, None, false)) + .expect("public listing"); + + assert!( + derived.private_key.is_none(), + "{kind:?}: no private scalar was requested" + ); + assert!( + derived.private_key_wif.is_none(), + "{kind:?}: no WIF without a private reveal" + ); + assert!( + derived.address.is_some(), + "{kind:?}: secp256k1 keys have an on-chain address" + ); + assert!( + derived.legacy_public_key_bytes.is_none() && derived.node_id.is_none(), + "{kind:?}: BLS legacy form and platform node id belong to other curves" + ); + assert_eq!(derived.index, 0); + } +} + +/// A private reveal must return a scalar and a WIF that agree with each other +/// and with the public key / address the same call reports. +/// +/// This is the invariant a wrong derivation path breaks silently: every field +/// is individually well-formed, and only their agreement with the account's +/// real key exposes the mismatch. +#[tokio::test(flavor = "multi_thread")] +async fn private_reveal_is_internally_consistent() { + use dashcore::PrivateKey; + + let wallet = platform_wallet().await; + let network = wallet_network(&wallet).await; + let secp = dashcore::key::Secp256k1::new(); + + for kind in [ProviderKeyKind::Owner, ProviderKeyKind::Voting] { + for index in [0u32, 1, 19] { + let derived = + block_in_place(|| wallet.derive_provider_key_at_index(kind, index, None, true)) + .expect("private reveal"); + + let scalar = derived.private_key.as_ref().expect("scalar requested"); + let wif = derived.private_key_wif.as_ref().expect("wif requested"); + let address = derived.address.as_ref().expect("address"); + + // WIF and raw scalar must be the same key. + let from_wif = PrivateKey::from_wif(wif).expect("valid WIF"); + assert_eq!( + from_wif.inner.secret_bytes().to_vec(), + **scalar, + "{kind:?}#{index}: WIF and raw scalar disagree" + ); + + // The reported public key must be this private key's. + assert_eq!( + from_wif.public_key(&secp).to_bytes(), + derived.public_key_bytes, + "{kind:?}#{index}: public key does not belong to the returned private key" + ); + + // ...and the reported address must be that public key's P2PKH on + // this wallet's own network, not a hardcoded chain. + let expected = dashcore::Address::p2pkh(&from_wif.public_key(&secp), network); + assert_eq!( + address, + &expected.to_string(), + "{kind:?}#{index}: address is not the returned key's P2PKH" + ); + } + } +} + +/// Distinct indexes must produce distinct keys — a derivation that ignored the +/// index would otherwise pass every consistency check above. +#[tokio::test(flavor = "multi_thread")] +async fn indexes_produce_distinct_keys() { + let wallet = platform_wallet().await; + + let keys: Vec> = (0u32..5) + .map(|index| { + block_in_place(|| { + wallet.derive_provider_key_at_index(ProviderKeyKind::Voting, index, None, false) + }) + .expect("public listing") + .public_key_bytes + }) + .collect(); + + let mut unique = keys.clone(); + unique.sort(); + unique.dedup(); + assert_eq!(unique.len(), keys.len(), "indexes collided"); +} + +/// Owner and voting keys live on different DIP-3 branches, so the same index +/// must not yield the same key for both. +#[tokio::test(flavor = "multi_thread")] +async fn owner_and_voting_do_not_share_keys() { + let wallet = platform_wallet().await; + + let owner = block_in_place(|| { + wallet.derive_provider_key_at_index(ProviderKeyKind::Owner, 0, None, false) + }) + .expect("owner"); + let voting = block_in_place(|| { + wallet.derive_provider_key_at_index(ProviderKeyKind::Voting, 0, None, false) + }) + .expect("voting"); + + assert_ne!( + owner.public_key_bytes, voting.public_key_bytes, + "owner and voting keys must come from different account paths" + ); +} + +/// A supplied seed that does not belong to this wallet must be refused by the +/// cross-check rather than returning a (public, private) pair that disagrees. +/// +/// Without this guard the caller signs for an address nobody expects, which has +/// no local symptom at all — on the voting family it surfaces only as Platform +/// rejecting the vote as having no voter identity. +#[tokio::test(flavor = "multi_thread")] +async fn foreign_seed_is_rejected_by_the_cross_check() { + let wallet = platform_wallet().await; + + // A valid seed, just not this wallet's. + let foreign_seed = [7u8; 64]; + let result = block_in_place(|| { + wallet.derive_provider_key_at_index(ProviderKeyKind::Voting, 0, Some(&foreign_seed), true) + }); + + // `expect_err` would require `Debug` on `ProviderDerivedKey`, which holds + // private key material — deriving it to satisfy a test would be a leak + // waiting to happen in a log line. + match result { + Ok(_) => panic!("a foreign seed must not produce a key"), + Err(err) => assert!( + format!("{err:?}").contains("inconsistent"), + "expected the cross-check to reject it, got: {err:?}" + ), + } +} + +/// Supplying this wallet's own seed explicitly — the external-signable path, +/// where the app resolves the mnemonic from the Keychain — must produce the +/// same key as letting the wallet use its resident seed. +/// +/// This is the shape that failed on device: the first cut went through a +/// wrapper gated on `is_watch_only`, which refuses exactly this caller. +#[tokio::test(flavor = "multi_thread")] +async fn explicitly_supplied_seed_matches_the_resident_one() { + let wallet = platform_wallet().await; + + let resident = block_in_place(|| { + wallet.derive_provider_key_at_index(ProviderKeyKind::Voting, 3, None, true) + }) + .expect("resident-seed derivation"); + + let seed = { + let manager = wallet.wallet_manager().read().await; + manager + .get_wallet(&wallet.wallet_id()) + .expect("wallet present") + .wallet_seed_bytes() + .expect("test wallet is seed-bearing") + }; + + let supplied = block_in_place(|| { + wallet.derive_provider_key_at_index(ProviderKeyKind::Voting, 3, Some(&seed), true) + }) + .expect("supplied-seed derivation"); + + assert_eq!(resident.public_key_bytes, supplied.public_key_bytes); + assert_eq!(resident.address, supplied.address); + assert_eq!( + resident.private_key.as_deref(), + supplied.private_key.as_deref(), + "the same seed must derive the same scalar whichever way it is supplied" + ); +} diff --git a/packages/rs-platform-wallet/src/wallet/provider_key_at_index.rs b/packages/rs-platform-wallet/src/wallet/provider_key_at_index.rs index 2152aca0761..629a7c574cc 100644 --- a/packages/rs-platform-wallet/src/wallet/provider_key_at_index.rs +++ b/packages/rs-platform-wallet/src/wallet/provider_key_at_index.rs @@ -631,7 +631,14 @@ impl PlatformWallet { )) })?; let secp = dashcore::key::Secp256k1::new(); - let account_path = account_type.derivation_path(network).map_err(|e| { + // The ACCOUNT's own path, not `account_type.derivation_path(network)`. + // The public side above comes off `account.account_xpub`, built from + // this path using the ACCOUNT's network; resolving it again from the + // type plus the wallet's network makes the coin type an independent + // input, and the two disagree the moment those networks differ. + // Taking it from the account is agreement by construction, so the + // cross-check below verifies the seed rather than the path. + let account_path = account.derivation_path().map_err(|e| { PlatformWalletError::KeyDerivation(format!( "failed to resolve the provider account path: {e}" )) @@ -641,7 +648,7 @@ impl PlatformWallet { "invalid provider key index {index}: {e}" )) })?; - let private = master + let mut child_xpriv = master .derive_priv(&secp, &account_path) .and_then(|acct| { let child_path: key_wallet::bip32::DerivationPath = @@ -652,19 +659,35 @@ impl PlatformWallet { PlatformWalletError::KeyDerivation(format!( "failed to derive provider private key at index {index}: {e}" )) - })? - .to_priv(); - let derived_public = private.public_key(&dashcore::key::Secp256k1::new()); - if derived_public != public_key { + })?; + let mut private = child_xpriv.to_priv(); + let derived_public = private.public_key(&secp); + + // Produce every output while the scalar is still live, + // then erase it. `dashcore::PrivateKey` is `Copy` and + // has no `Drop`, so unlike the `Zeroizing` outputs below + // nothing scrubs it on the way out — the raw scalar + // would otherwise be left behind in this frame. Both + // copies are erased: the extended key it came from, and + // the `PrivateKey` itself. + let outputs = if derived_public == public_key { + Some(( + Zeroizing::new(private.inner.secret_bytes().to_vec()), + Zeroizing::new(private.to_wif()), + )) + } else { + None + }; + private.inner.non_secure_erase(); + child_xpriv.private_key.non_secure_erase(); + + let Some((scalar, wif)) = outputs else { return Err(PlatformWalletError::KeyDerivation(format!( "provider key at index {index} is inconsistent: the seed-derived \ private key's public key does not match the account xpub's" ))); - } - ( - Some(Zeroizing::new(private.inner.secret_bytes().to_vec())), - Some(Zeroizing::new(private.to_wif())), - ) + }; + (Some(scalar), Some(wif)) } None => (None, None), };