From 453dbdd451ac9e74aeb3822c26cc480700a4abdb Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Fri, 31 Jul 2026 14:43:20 -0700 Subject: [PATCH 1/7] feat(platform-wallet): classic Dash signed-message primitive (sign_message) over FFI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CoreWallet::sign_message signs a string with the key behind one of the wallet's own P2PKH addresses (signable funds accounts only) and returns the 65-byte BIP-137-style recoverable signature, base64 — same semantics as dashj ECKey.signMessage and Dash Core's signmessage RPC, verified byte-for-byte against dashj (RFC6979 golden + dashj test vector). The digest and serialization come from dashcore::sign_message; the recovery id is found by trial against the signer's pubkey, so every Signer backend gets signed-message support without a recoverable-signing method. Key material never crosses the FFI: core_wallet_sign_message takes the caller's MnemonicResolverHandle like the send paths do. Unknown/watch-only addresses map to ErrorSigningKeyUnavailable (31). Serves the Android/iOS wallets' CrowdNode integrations (API withdrawal and email registration are signed-message proofs of address ownership). Co-Authored-By: Claude Fable 5 --- .../src/core_wallet/mod.rs | 2 + .../src/core_wallet/sign_message.rs | 143 ++++++ packages/rs-platform-wallet-ffi/src/error.rs | 102 +++++ packages/rs-platform-wallet/Cargo.toml | 11 +- packages/rs-platform-wallet/src/error.rs | 49 ++ .../rs-platform-wallet/src/test_support.rs | 79 ++++ .../rs-platform-wallet/src/wallet/core/mod.rs | 2 + .../src/wallet/core/sign_message.rs | 433 ++++++++++++++++++ 8 files changed, 819 insertions(+), 2 deletions(-) create mode 100644 packages/rs-platform-wallet-ffi/src/core_wallet/sign_message.rs create mode 100644 packages/rs-platform-wallet/src/wallet/core/sign_message.rs diff --git a/packages/rs-platform-wallet-ffi/src/core_wallet/mod.rs b/packages/rs-platform-wallet-ffi/src/core_wallet/mod.rs index 8e12ebc1783..5a3b0d7399b 100644 --- a/packages/rs-platform-wallet-ffi/src/core_wallet/mod.rs +++ b/packages/rs-platform-wallet-ffi/src/core_wallet/mod.rs @@ -4,10 +4,12 @@ mod addresses; mod broadcast; +mod sign_message; mod transaction_builder; mod wallet; pub use addresses::*; pub use broadcast::*; +pub use sign_message::*; pub use transaction_builder::*; pub use wallet::*; diff --git a/packages/rs-platform-wallet-ffi/src/core_wallet/sign_message.rs b/packages/rs-platform-wallet-ffi/src/core_wallet/sign_message.rs new file mode 100644 index 00000000000..62422ebb97e --- /dev/null +++ b/packages/rs-platform-wallet-ffi/src/core_wallet/sign_message.rs @@ -0,0 +1,143 @@ +//! FFI binding for classic Dash **message signing**. +//! +//! Proves ownership of one of the wallet's P2PKH addresses by signing an +//! arbitrary short string with the key behind it, returning the base64 +//! signature Dash Core's `verifymessage` RPC and dashj's `ECKey.verifyMessage` +//! accept. Unlike every other entry point in this module it moves no value: +//! nothing is selected, reserved, signed into a transaction, broadcast, or +//! persisted, so there is no reservation for the caller to discharge and no +//! funding-domain question to answer. +//! +//! See `platform_wallet::wallet::core::sign_message` for the digest +//! construction (the historical `"\x19DarkCoin Signed Message:\n"` prefix), the +//! 65-byte recoverable encoding, and why the recovery id is found by trial. + +use crate::error::*; +use crate::handle::{Handle, CORE_WALLET_STORAGE}; +use crate::runtime::runtime; +use crate::{check_ptr, unwrap_option_or_return, unwrap_result_or_return}; +use platform_wallet::PlatformWalletError; +use rs_sdk_ffi::{MnemonicResolverCoreSigner, MnemonicResolverHandle}; +use std::ffi::CString; +use std::os::raw::c_char; + +/// Read a UTF-8 string argument passed as pointer + length. +/// +/// Pointer + length rather than a NUL-terminated C string because the message is +/// caller text, not an identifier: a length-delimited read cannot silently +/// truncate at an embedded NUL and sign a shorter message than the caller +/// believes it signed — which would verify for a message they never sent, a +/// divergence invisible on this side of the boundary. The address argument uses +/// the same shape for symmetry. +/// +/// `on_error` builds the typed error, so each argument reports itself rather +/// than borrowing the other's phrasing. +/// +/// `len == 0` yields the empty string WITHOUT dereferencing `ptr`, which may +/// legitimately be null: Swift's established marshalling +/// (`Array(s.utf8).withUnsafeBufferPointer`) hands back a nil `baseAddress` for +/// an empty string, and an empty message is signable. `from_raw_parts` is UB on +/// a null pointer even at length 0, so the zero case must short-circuit rather +/// than rely on the slice being empty. +/// +/// # Safety +/// When `len > 0`, `ptr` must be non-null and readable for `len` bytes. +unsafe fn read_utf8( + ptr: *const u8, + len: usize, + on_error: impl FnOnce(std::str::Utf8Error) -> PlatformWalletError, +) -> Result { + if len == 0 { + return Ok(String::new()); + } + let bytes = std::slice::from_raw_parts(ptr, len); + std::str::from_utf8(bytes) + .map(|s| s.to_string()) + .map_err(on_error) +} + +/// Sign `message` with the private key behind `address` and return the base64 +/// signature — a classic Dash signed message. +/// +/// * `handle` — a core-wallet handle (`platform_wallet_get_core`). +/// * `address_ptr`/`address_len` — the UTF-8 P2PKH address whose key signs. Must +/// be one of this wallet's own addresses, on the wallet's network, and belong +/// to a signable funds account (BIP44 / BIP32 / CoinJoin / +/// DashPay-receiving). A foreign address, or a watch-only DashPay *external* +/// account's address, fails with +/// [`PlatformWalletFFIResultCode::ErrorSigningKeyUnavailable`] (31); an +/// unparseable, wrong-network, or non-P2PKH address fails with +/// [`PlatformWalletFFIResultCode::ErrorInvalidParameter`] (2). +/// * `message_ptr`/`message_len` — the UTF-8 message to sign, verbatim. It is +/// length-prefixed into the digest, so trailing whitespace and newlines are +/// significant and the verifier must receive the identical bytes. An empty +/// message is valid and signable, and `message_ptr` MAY be null when +/// `message_len` is 0 — the shape host marshalling naturally produces for an +/// empty string. +/// * `core_signer_handle` — the caller's `MnemonicResolverHandle`; ownership is +/// retained by the caller (this function does NOT destroy it). +/// * `out_signature` — receives a heap-allocated C string holding the base64 +/// signature. Free with [`super::core_wallet_free_address`]. +/// +/// # Safety +/// `address_ptr` must be non-null and readable for `address_len` bytes; +/// `message_ptr` must be readable for `message_len` bytes when that length is +/// non-zero (it may be null when the length is 0). Both must stay valid for the +/// duration of the call; `out_signature` must point to writable memory for one +/// `*mut c_char`. +#[no_mangle] +pub unsafe extern "C" fn core_wallet_sign_message( + handle: Handle, + address_ptr: *const u8, + address_len: usize, + message_ptr: *const u8, + message_len: usize, + core_signer_handle: *mut MnemonicResolverHandle, + out_signature: *mut *mut c_char, +) -> PlatformWalletFFIResult { + // An address is never legitimately empty, so its pointer must be present. + // `message_ptr` is deliberately NOT checked: a null pointer with + // `message_len == 0` is the empty message, which is signable (see + // [`read_utf8`]). It is only dereferenced when the length is non-zero. + check_ptr!(address_ptr); + check_ptr!(core_signer_handle); + check_ptr!(out_signature); + *out_signature = std::ptr::null_mut(); + + let signer_addr = core_signer_handle as usize; + + let option = CORE_WALLET_STORAGE.with_item(handle, |wallet| { + let address = read_utf8(address_ptr, address_len, |e| { + PlatformWalletError::MessageSigningAddressInvalid { + address: "".to_string(), + reason: format!("address is not valid UTF-8: {e}"), + } + })?; + // A non-UTF-8 message is reported against the (now known) address, so the + // error names the signing target the caller asked about. + let message = read_utf8(message_ptr, message_len, |e| { + PlatformWalletError::MessageSigningFailed { + address: address.clone(), + reason: format!("message is not valid UTF-8: {e}"), + } + })?; + let network = wallet.network(); + let wallet_id = wallet.wallet_id(); + // SAFETY: `signer_addr` came from `core_signer_handle`, which the caller + // pinned alive for this call; the `MnemonicResolverCoreSigner` lives + // only on this stack frame and is dropped before returning. + let signer = MnemonicResolverCoreSigner::new( + signer_addr as *mut MnemonicResolverHandle, + wallet_id, + network, + ); + runtime().block_on(wallet.sign_message(&address, &message, &signer)) + }); + + let result = unwrap_option_or_return!(option); + let signature = unwrap_result_or_return!(result); + + let c_str = unwrap_result_or_return!(CString::new(signature)); + *out_signature = c_str.into_raw(); + PlatformWalletFFIResult::ok() +} diff --git a/packages/rs-platform-wallet-ffi/src/error.rs b/packages/rs-platform-wallet-ffi/src/error.rs index 95683cb131d..f87a487ad64 100644 --- a/packages/rs-platform-wallet-ffi/src/error.rs +++ b/packages/rs-platform-wallet-ffi/src/error.rs @@ -234,6 +234,13 @@ pub enum PlatformWalletFFIResultCode { /// Hosts route this to key repair instead of treating it as an opaque /// wallet-operation failure. Not retryable as-is — the key must be /// (re-)derived first. + /// + /// Also produced WITHOUT the signer round-trip, by + /// [`CoreWallet::sign_message`](platform_wallet::CoreWallet::sign_message): + /// a message-signing address that belongs to no signable funds account of + /// this wallet means no key can exist for it, which is the same conclusion + /// this code exists to carry — hosts route both to key repair / address + /// correction rather than to an opaque wallet-operation failure. ErrorSigningKeyUnavailable = 31, NotFound = 98, // Used exclusively for all the Option that are retuned as errors @@ -432,6 +439,49 @@ impl From for PlatformWalletFFIResult { PlatformWalletError::TxMetadataPayloadTooLarge { .. } => { PlatformWalletFFIResultCode::ErrorInvalidParameter } + // An unparseable / wrong-network / non-P2PKH message-signing + // address: a caller-input error, so it is routed to the + // already-mirrored ErrorInvalidParameter rather than spending a new + // numeric code (and churning the Swift/Kotlin mirror enums) on a + // case hosts handle by correcting the input. The typed Display + // names which of the three it was. + PlatformWalletError::MessageSigningAddressInvalid { .. } => { + PlatformWalletFFIResultCode::ErrorInvalidParameter + } + // A second, signer-free producer of code 31 (the arm above is the + // first): a message-signing address that belongs to no signable + // funds account means no key can exist for it — the same conclusion + // the code carries — so hosts route it to key repair / address + // correction instead of an opaque wallet-operation failure. + PlatformWalletError::MessageSigningKeyUnavailable { .. } => { + PlatformWalletFFIResultCode::ErrorSigningKeyUnavailable + } + // NOTE: `MessageSigningFailed` is deliberately NOT matched, so it + // falls to the `ErrorUnknown` catch-all below. Its causes are + // internal invariant breaks (a public key that does not own the + // address, no recovery id that recovers it) which should read as a + // bug rather than as a key-repair prompt, and it carries the + // signer's own `Display`, which reaches the host in the message + // either way. + // + // It is NOT promoted to code 31 by the key-unavailable arm above, + // and that is deliberate rather than an oversight. That arm matches + // STRUCTURALLY — `Sdk(Protocol(Generic(s)))` with the marker at + // position 0 — because #4183's review rejected sniffing the marker + // as a substring of the rendered error: a foreign signer can merely + // mention the token in human-readable text. `MessageSigningFailed` + // is a different variant, and `sign_message` composes its `reason` + // as "signer rejected the digest at {path}: {e}", so the marker + // could only ever appear mid-string. Matching it here would mean + // exactly the substring sniff that review ruled out. + // + // Consequence worth knowing: a Keystore/Keychain key-unavailable + // completion reaching `sign_message` surfaces as ErrorUnknown, not + // 31. Closing that needs a structural change at the producer — have + // `sign_message` classify the signer's own rendering before it wraps + // it — not a looser match here. `platform-wallet` cannot see the + // marker constant (it does not depend on `rs-sdk-ffi`), so that is + // its own piece of work. _ => PlatformWalletFFIResultCode::ErrorUnknown, }; PlatformWalletFFIResult::err(code, error.to_string()) @@ -984,6 +1034,22 @@ mod tests { ); } + /// A second producer of code 31, reached with no signer round-trip and no + /// marker sniffing at all: the wallet simply holds no key for the address. + /// Hosts branch on it to correct the address or repair the key, so it must + /// not flatten to ErrorUnknown. + #[test] + fn message_signing_key_unavailable_maps_to_code_31() { + let err = PlatformWalletError::MessageSigningKeyUnavailable { + address: "yRd4FhXfVGHXpsuZXPNkMrfD9GVj46pnjt".to_string(), + }; + let result: PlatformWalletFFIResult = err.into(); + assert_eq!( + result.code, + PlatformWalletFFIResultCode::ErrorSigningKeyUnavailable + ); + } + /// A generic protocol error without the prefix keeps the historical /// mapping — no message sniffing beyond the machine prefix. #[test] @@ -1029,4 +1095,40 @@ mod tests { PlatformWalletFFIResultCode::ErrorSigningKeyUnavailable ); } + + /// A bad message-signing address is caller input, so it maps to the + /// already-mirrored ErrorInvalidParameter rather than ErrorUnknown. + #[test] + fn message_signing_address_invalid_maps_to_invalid_parameter() { + let err = PlatformWalletError::MessageSigningAddressInvalid { + address: "not-an-address".to_string(), + reason: "not a valid Dash address".to_string(), + }; + let result: PlatformWalletFFIResult = err.into(); + assert_eq!( + result.code, + PlatformWalletFFIResultCode::ErrorInvalidParameter + ); + } + + /// `MessageSigningFailed` is intentionally unmapped: its causes are + /// internal invariant breaks, which should read as a bug rather than as a + /// key-repair prompt, so it falls through to ErrorUnknown carrying the + /// signer's own rendering. Pinned so a future arm cannot silently claim it. + /// + /// #4183's key-unavailable promotion does NOT reach this variant, by + /// design: it matches `Sdk(Protocol(Generic(s)))` structurally with the + /// marker at position 0, because that review rejected sniffing the marker as + /// a substring. `sign_message` composes `reason` as + /// "signer rejected the digest at {path}: {e}", so a marker could only ever + /// sit mid-string here. See the NOTE on the mapping arm. + #[test] + fn message_signing_failed_falls_through_to_unknown() { + let internal = PlatformWalletError::MessageSigningFailed { + address: "yRd4FhXfVGHXpsuZXPNkMrfD9GVj46pnjt".to_string(), + reason: "no recovery id in 0..=3 recovers the signing public key".to_string(), + }; + let result: PlatformWalletFFIResult = internal.into(); + assert_eq!(result.code, PlatformWalletFFIResultCode::ErrorUnknown); + } } diff --git a/packages/rs-platform-wallet/Cargo.toml b/packages/rs-platform-wallet/Cargo.toml index 6c7e86a5b8e..791fa0ab939 100644 --- a/packages/rs-platform-wallet/Cargo.toml +++ b/packages/rs-platform-wallet/Cargo.toml @@ -17,8 +17,15 @@ key-wallet = { workspace = true } key-wallet-manager = { workspace = true, features = [] } dash-spv = { workspace = true } -# Core dependencies -dashcore = { workspace = true } +# Core dependencies. +# +# `base64` is NOT among dashcore's default features (those are `secp-recovery` +# and `bincode`), but `MessageSignature::to_base64` / `from_base64` are gated on +# it — and base64 is the only wire form a classic Dash signed message has, the +# one CrowdNode and Dash Core's `verifymessage` RPC accept. `secp-recovery` +# arrives by default and carries `secp256k1/recovery`, which +# `RecoverableSignature` / `recover_ecdsa` need. See `wallet::core::sign_message`. +dashcore = { workspace = true, features = ["base64"] } # Standard dependencies thiserror = "1.0" diff --git a/packages/rs-platform-wallet/src/error.rs b/packages/rs-platform-wallet/src/error.rs index 7495b74956a..ef31312289b 100644 --- a/packages/rs-platform-wallet/src/error.rs +++ b/packages/rs-platform-wallet/src/error.rs @@ -108,6 +108,55 @@ pub enum PlatformWalletError { #[error("Transaction building failed: {0}")] TransactionBuild(String), + /// The address handed to [`CoreWallet::sign_message`] cannot be a signing + /// target at all: unparseable, encoded for a different network than the + /// wallet's, or not P2PKH. A caller-input error — the classic Dash + /// signed-message format recovers a public key and compares its + /// `PubkeyHash` payload, so P2SH / SegWit payloads have no defined + /// verification and are refused rather than signed into something no + /// verifier accepts. `reason` names which of the three it was. + /// + /// [`CoreWallet::sign_message`]: crate::wallet::core::CoreWallet::sign_message + #[error("message-signing address {address:?} is unusable: {reason}")] + MessageSigningAddressInvalid { address: String, reason: String }, + + /// [`CoreWallet::sign_message`] was given a well-formed P2PKH address for + /// the right network that this wallet holds no signing key for: it belongs + /// to no *signable* funds account (BIP44 / BIP32 / CoinJoin / + /// DashPay-receiving), or it belongs to a watch-only DashPay **external** + /// account — a contact's receiving address, whose keys we never had. + /// + /// Distinct from a signer *failure*: nothing was attempted, because no + /// derivation path resolves the address. Carries no retry value as-is; the + /// caller must supply an address the wallet owns. + /// + /// [`CoreWallet::sign_message`]: crate::wallet::core::CoreWallet::sign_message + #[error( + "no signing key for message-signing address {address}: it belongs to no \ + signable funds account of this wallet" + )] + MessageSigningKeyUnavailable { address: String }, + + /// [`CoreWallet::sign_message`] resolved a derivation path for the address + /// but could not produce a signature over it. Three causes, all carried in + /// `reason`: the [`Signer`] itself failed (Keystore/Keychain round-trip); + /// the public key it returned does not hash to the target address (a + /// path-resolution bug — the guard exists so a wrong-key signature can + /// never be handed out as if it were the address owner's); or no recovery + /// id in `0..=3` recovers that public key. + /// + /// Deliberately NOT given a dedicated FFI code: a signer that reports its + /// typed key-unavailable completion carries the stable machine prefix in + /// its `Display`, which the FFI conversion's catch-all promotes to + /// `ErrorSigningKeyUnavailable`. The remaining causes are genuine internal + /// invariant breaks and should surface as unknown rather than as a + /// key-repair prompt. + /// + /// [`CoreWallet::sign_message`]: crate::wallet::core::CoreWallet::sign_message + /// [`Signer`]: key_wallet::signer::Signer + #[error("message signing failed for address {address}: {reason}")] + MessageSigningFailed { address: String, reason: String }, + /// Atomic Core finalization could not select enough unreserved funds. #[error( "insufficient unreserved Core funds on {account_type:?} account {account_index}: \ diff --git a/packages/rs-platform-wallet/src/test_support.rs b/packages/rs-platform-wallet/src/test_support.rs index 8fd146dc77c..8820eef0bb4 100644 --- a/packages/rs-platform-wallet/src/test_support.rs +++ b/packages/rs-platform-wallet/src/test_support.rs @@ -277,3 +277,82 @@ pub async fn funded_spv_core_wallet( signer, ) } + +/// Canonical all-`abandon` BIP-39 test vector. Fixed (not +/// `TestWalletContext::new_random`) so every key it derives is a stable golden — +/// which is what lets the signed-message tests pin an RFC6979-deterministic +/// signature and cross-check it against dashj. +#[cfg(test)] +pub(crate) const MESSAGE_SIGNING_TEST_MNEMONIC: &str = + "abandon abandon abandon abandon abandon abandon \ + abandon abandon abandon abandon abandon about"; + +/// Builds a testnet wallet manager from a KNOWN mnemonic with one derived BIP44 +/// external address, and NO funding. +/// +/// The unfunded counterpart of [`funded_wallet_manager`], for operations that +/// prove key ownership rather than move value — signing a message needs a +/// derivation path and a signer, never a UTXO. Returns the manager, the wallet +/// id, a soft signer over the wallet's seed, and that first receive address. +/// +/// `#[cfg(test)]` rather than `test-utils`-gated: only this crate's own unit +/// tests consume it, so under the FFI crate's `test-utils` build it would +/// compile with no user and trip `dead_code`. +#[cfg(test)] +pub(crate) async fn mnemonic_wallet_manager( + phrase: &str, +) -> ( + Arc>>, + WalletId, + WalletSigner, + dashcore::Address, +) { + use key_wallet::wallet::initialization::WalletAccountCreationOptions; + use key_wallet::wallet::ManagedWalletInfo; + use key_wallet::{Language, Mnemonic}; + + let mnemonic = Mnemonic::from_phrase(phrase, Language::English).expect("valid test mnemonic"); + let wallet = Wallet::from_mnemonic( + mnemonic, + Network::Testnet, + WalletAccountCreationOptions::Default, + ) + .expect("wallet construction from a known mnemonic"); + let mut managed_wallet = + ManagedWalletInfo::from_wallet_with_name(&wallet, "SignMessage".to_string(), 0); + + let xpub = wallet + .accounts + .standard_bip44_accounts + .get(&0) + .expect("default options create BIP44 account 0") + .account_xpub; + // `true` registers the address in the external pool, which is what makes it + // findable by `address_derivation_path` — an unregistered gap-limit address + // is deliberately not signable. + let receive_address = managed_wallet + .first_bip44_managed_account_mut() + .expect("managed BIP44 account 0") + .next_receive_address(Some(&xpub), true) + .expect("first BIP44 receive address"); + + let signer = WalletSigner { + wallet: wallet.clone(), + }; + let info = PlatformWalletInfo { + core_wallet: managed_wallet, + balance: Arc::new(WalletBalance::new()), + identity_manager: IdentityManager::new(), + tracked_asset_locks: BTreeMap::new(), + }; + + let mut wm = WalletManager::::new(Network::Testnet); + let wallet_id = wm.insert_wallet(wallet, info).expect("insert wallet"); + + ( + Arc::new(RwLock::new(wm)), + wallet_id, + signer, + receive_address, + ) +} diff --git a/packages/rs-platform-wallet/src/wallet/core/mod.rs b/packages/rs-platform-wallet/src/wallet/core/mod.rs index 5481362ae8b..ed6424d20aa 100644 --- a/packages/rs-platform-wallet/src/wallet/core/mod.rs +++ b/packages/rs-platform-wallet/src/wallet/core/mod.rs @@ -1,6 +1,8 @@ pub mod balance; pub mod balance_handler; mod broadcast; +// Inherent `CoreWallet::sign_message` only — no types to re-export. +mod sign_message; mod transaction; pub mod wallet; diff --git a/packages/rs-platform-wallet/src/wallet/core/sign_message.rs b/packages/rs-platform-wallet/src/wallet/core/sign_message.rs new file mode 100644 index 00000000000..dddb4c541d3 --- /dev/null +++ b/packages/rs-platform-wallet/src/wallet/core/sign_message.rs @@ -0,0 +1,433 @@ +//! Classic Dash **signed message** production. +//! +//! [`CoreWallet::sign_message`] signs an arbitrary short string with the +//! private key behind one of the wallet's own P2PKH addresses and returns the +//! base64 signature. Same semantics as dashj's `ECKey.signMessage` and Dash +//! Core's `signmessage` RPC, so a signature produced here verifies with +//! `verifymessage` and with any dashj-based verifier. +//! +//! ## The format +//! +//! The digest is `SHA256d(DASH_SIGNED_MSG_PREFIX ‖ varint(len(msg)) ‖ msg)`, +//! where the prefix is the historical `"\x19DarkCoin Signed Message:\n"` — +//! **not** `"Dash"`. Dash inherited the string from its pre-rename days and it +//! is consensus-visible in every existing verifier, so it can never be updated. +//! [`dashcore::sign_message::signed_msg_hash`] owns that construction; nothing +//! here re-implements it. +//! +//! The signature is the 65-byte BIP-137-style recoverable form — +//! `header ‖ r ‖ s`, with `header = 27 + recovery_id + 4` (the `+ 4` marking a +//! compressed public key) — base64-encoded. Verification recovers the public +//! key from the digest and compares its `PubkeyHash` to the address, which is +//! why the recovery id has to be right and why only P2PKH addresses are +//! signable: no other payload has a defined recovery comparison. +//! +//! ## Why the recovery id is found by trial +//! +//! [`Signer`] is a *plain*-ECDSA interface: `sign_ecdsa` hands back a +//! non-recoverable [`ecdsa::Signature`] plus the compressed public key, which is +//! all a P2PKH `scriptSig` needs. The recovery id is not recoverable from that +//! pair analytically, so it is found by trying all four candidates and keeping +//! the one that recovers the signer's own public key — exactly what dashj's +//! `ECKey.findRecoveryId` does. Keeping the trial here means every signer +//! backend (Keystore, Keychain, hardware) gets signed-message support without +//! growing a recoverable-signing method it would have to implement. +//! +//! ## Consumer +//! +//! The Android wallet's CrowdNode integration signs short strings — withdrawal +//! amounts, the account email — which CrowdNode verifies server-side against +//! the address that funded the account. That is a *proof of address ownership*, +//! not a spend: nothing is selected, reserved, broadcast, or persisted, and the +//! wallet-manager lock is held only long enough to resolve a derivation path. + +use std::str::FromStr; + +use dashcore::address::Payload; +use dashcore::hashes::Hash; +use dashcore::secp256k1::ecdsa::{RecoverableSignature, RecoveryId}; +use dashcore::secp256k1::{Message, Secp256k1}; +use dashcore::sign_message::{signed_msg_hash, MessageSignature}; +use dashcore::{Address as DashAddress, AddressType, PublicKey as DashPublicKey}; +use key_wallet::managed_account::managed_account_trait::ManagedAccountTrait; +use key_wallet::signer::Signer; +use key_wallet::ManagedAccountType; + +use crate::broadcaster::TransactionBroadcaster; +use crate::error::PlatformWalletError; +use crate::wallet::core::CoreWallet; + +/// Every recovery id a compact ECDSA signature can carry. Ids 2 and 3 encode an +/// overflowing `r` (x-coordinate ≥ the curve order), astronomically improbable +/// but valid, so all four are tried rather than just 0 and 1. +const RECOVERY_IDS: [i32; 4] = [0, 1, 2, 3]; + +/// Whether a funds account can be *signed for* by the local mnemonic. +/// +/// Only `DashpayExternalAccount`s are watch-only: they hold a **contact's** +/// receiving addresses (we keep the contact's xpub to build payments *to* them +/// and to watch that side), so no private key of ours derives their addresses +/// and signing for them would fail. Every other funds account +/// (BIP44 / BIP32 / CoinJoin / DashPay-receiving) comes from our own seed. +/// +/// This is a **signability** filter and nothing more. A watch-only account must +/// be refused even when a caller names its address explicitly. +/// +/// Deliberately a private local helper rather than a shared one: the +/// funding-privacy work introduces `wallet::funding_privacy:: +/// is_signable_funding_account` with this exact body and semantics, but that +/// module does not exist on this branch. Keeping the predicate here — same +/// name, same body — means the two converge to a single call site by deleting +/// this function when that module lands, with no behavior change to review. +fn is_signable_funding_account(managed_type: &ManagedAccountType) -> bool { + !matches!( + managed_type, + ManagedAccountType::DashpayExternalAccount { .. } + ) +} + +impl CoreWallet { + /// Sign `message` with the private key behind `address` and return the + /// base64 signature — a classic Dash signed message, verifiable by Dash + /// Core's `verifymessage` RPC, dashj's `ECKey.verifyMessage`, and + /// CrowdNode's server-side check. + /// + /// `address` must be a P2PKH address of THIS wallet, on this wallet's + /// network, belonging to a *signable* funds account (BIP44 / BIP32 / + /// CoinJoin / DashPay-receiving). A watch-only DashPay **external** account + /// holds a contact's addresses whose keys we never had, so it is refused + /// like any other unowned address — + /// [`MessageSigningKeyUnavailable`](PlatformWalletError::MessageSigningKeyUnavailable), + /// which the FFI surfaces as `ErrorSigningKeyUnavailable`. + /// + /// Not a spend: no UTXO is selected, reserved, or spent, and the accounts + /// are visited only to look ONE address up. + /// + /// # Arguments + /// + /// * `address` — the P2PKH address whose key signs. Must be one of this + /// wallet's own, already present in an account's address pool (i.e. + /// generated — a gap-limit address the wallet has never derived is not + /// findable and is refused). + /// * `message` — the string to sign, verbatim. It is length-prefixed into + /// the digest, so trailing whitespace and newlines are significant; the + /// verifier must be handed the identical bytes. + /// * `signer` — the ECDSA signer that holds the key material (the + /// Keystore/Keychain-backed `MnemonicResolverCoreSigner` in production). + /// No private key crosses the boundary. + /// + /// # Determinism + /// + /// key-wallet's signers produce RFC6979 deterministic, low-s normalized + /// signatures, so the same `(address, message)` pair on the same seed always + /// yields the same base64 — which is what lets the roundtrip test pin a + /// golden vector. + pub async fn sign_message( + &self, + address: &str, + message: &str, + signer: &S, + ) -> Result { + // Resolve the address to a leaf derivation path under a READ lock, then + // release it: the signer round-trip below can be a Keystore/Keychain + // call (or a hardware round-trip) and must not hold the wallet-manager + // lock, which every balance read and send path also needs. Nothing is + // mutated here, so there is no validate-then-mutate window to protect. + let (target, path) = { + let wm = self.wallet_manager.read().await; + let (_, info) = wm + .get_wallet_and_info(&self.wallet_id) + .ok_or_else(|| PlatformWalletError::WalletNotFound(hex::encode(self.wallet_id)))?; + + // The wallet's own network, not `self.network()` (the SDK's): the + // address must be checked against the keys it could belong to. + let network = info.core_wallet.network(); + + let parsed = DashAddress::from_str(address).map_err(|e| { + PlatformWalletError::MessageSigningAddressInvalid { + address: address.to_string(), + reason: format!("not a valid Dash address: {e}"), + } + })?; + let target = parsed.require_network(network).map_err(|e| { + PlatformWalletError::MessageSigningAddressInvalid { + address: address.to_string(), + reason: format!("encoded for another network than {network:?}: {e}"), + } + })?; + if target.address_type() != Some(AddressType::P2pkh) { + return Err(PlatformWalletError::MessageSigningAddressInvalid { + address: address.to_string(), + reason: format!( + "signed messages verify by recovering a public key and comparing its \ + PubkeyHash, so only P2PKH addresses can sign; this is {:?}", + target.address_type() + ), + }); + } + + // PRIVACY-DOMAIN-OK: this iterates funds accounts to LOOK ONE + // ADDRESS UP and stops at the first pool that owns it. No UTXO is + // selected, no value moves, and no transaction is built, so there is + // no on-chain link to create and nothing is accumulated across + // accounts. Watch-only accounts are skipped because their keys are a + // contact's, not ours. + let path = info + .core_wallet + .accounts + .all_funding_accounts() + .into_iter() + .filter(|acc| is_signable_funding_account(acc.managed_account_type())) + .find_map(|acc| acc.address_derivation_path(&target)) + .ok_or_else(|| PlatformWalletError::MessageSigningKeyUnavailable { + address: target.to_string(), + })?; + + (target, path) + }; + + let hash = signed_msg_hash(message); + let (signature, public_key) = signer + .sign_ecdsa(&path, hash.to_byte_array()) + .await + .map_err(|e| PlatformWalletError::MessageSigningFailed { + address: target.to_string(), + reason: format!("signer rejected the digest at {path}: {e}"), + })?; + + // Signers return compressed public keys, so the address the signed + // message will verify against is `P2PKH(compressed(pubkey))`. + let dash_public_key = DashPublicKey { + inner: public_key, + compressed: true, + }; + + // The address a verifier will derive must be the address the caller + // asked for. If path resolution ever hands back the wrong leaf, this is + // the difference between a clear failure and silently vouching for an + // address with someone else's key. Compared on the payload so the check + // is network-independent. + if *target.payload() != Payload::p2pkh(&dash_public_key) { + return Err(PlatformWalletError::MessageSigningFailed { + address: target.to_string(), + reason: format!( + "the key at {path} does not own this address — its public key hashes to a \ + different P2PKH payload" + ), + }); + } + + // `recover_ecdsa` needs only a `Verification` context — the signing + // tables a full `Secp256k1::new()` would also allocate are dead weight + // here, since the signature itself came from the signer. + let secp = Secp256k1::verification_only(); + let digest = Message::from_digest(hash.to_byte_array()); + let compact = signature.serialize_compact(); + + let recoverable = RECOVERY_IDS + .into_iter() + .filter_map(|id| RecoveryId::try_from(id).ok()) + .filter_map(|recid| RecoverableSignature::from_compact(&compact, recid).ok()) + .find(|candidate| { + secp.recover_ecdsa(&digest, candidate) + .is_ok_and(|recovered| recovered == public_key) + }) + .ok_or_else(|| PlatformWalletError::MessageSigningFailed { + address: target.to_string(), + reason: "no recovery id in 0..=3 recovers the signing public key".to_string(), + })?; + + Ok(MessageSignature::new(recoverable, true).to_base64()) + } +} + +#[cfg(test)] +mod tests { + use std::str::FromStr; + use std::sync::Arc; + + use dashcore::secp256k1::Secp256k1; + use dashcore::sign_message::{signed_msg_hash, MessageSignature}; + use dashcore::{Address as DashAddress, Network}; + + use crate::test_support::{ + mnemonic_wallet_manager, AlwaysRejectedBroadcaster, MESSAGE_SIGNING_TEST_MNEMONIC, + }; + use crate::wallet::core::balance::WalletBalance; + use crate::wallet::core::CoreWallet; + use crate::wallet::platform_wallet::WalletId; + use crate::PlatformWalletError; + + /// The message every fixture signs. Short and ASCII, like the withdrawal + /// amounts and emails CrowdNode signs in production, and the same message + /// dashj's own `ECKeyTest.verifyMessage` vector uses. + const MESSAGE: &str = "hello"; + + /// A `CoreWallet` over a manager fixture. Message signing never broadcasts, + /// so the broadcaster is irrelevant. + fn core_wallet( + wallet_manager: Arc< + tokio::sync::RwLock< + key_wallet_manager::WalletManager< + crate::wallet::platform_wallet::PlatformWalletInfo, + >, + >, + >, + wallet_id: WalletId, + ) -> CoreWallet { + let sdk = Arc::new(dash_sdk::SdkBuilder::new_mock().build().expect("mock sdk")); + CoreWallet::new( + sdk, + wallet_manager, + wallet_id, + Arc::new(AlwaysRejectedBroadcaster), + Arc::new(WalletBalance::new()), + ) + } + + /// Verify `signature_base64` against `address` the way an external verifier + /// would: decode, re-derive the digest from the message alone, recover. + fn verifies_for(signature_base64: &str, address: &DashAddress) -> bool { + MessageSignature::from_base64(signature_base64) + .expect("signature is valid base64 of a 65-byte recoverable signature") + .is_signed_by_address(&Secp256k1::new(), address, signed_msg_hash(MESSAGE)) + .expect("P2PKH address is a supported verification target") + } + + /// **Cross-implementation parity.** A signature produced by dashj + /// (`ECKeyTest.verifyMessage`) must verify under this crate's + /// `signed_msg_hash` + `MessageSignature`, proving the two agree on the + /// `"\x19DarkCoin Signed Message:\n"` prefix, the varint length prefix, the + /// double-SHA256, the 65-byte header encoding, and the compressed-key flag. + /// + /// This is the anchor the golden vector in + /// [`roundtrip_signature_is_a_stable_golden`] is meaningful against: without + /// it, a self-consistent but format-divergent implementation would pass + /// every other test here and still be rejected by CrowdNode. + #[test] + fn dashj_vector_verifies() { + let address = DashAddress::from_str("Xt5QmmzX2LaMgt81dcXGHhdf8pAhaVJKUW") + .expect("valid mainnet P2PKH address") + .require_network(Network::Mainnet) + .expect("mainnet address on mainnet"); + assert!( + verifies_for( + "HPygR8+G/HJ0kSp0azMeW6bvzd1tGg0Nx1mCJ/ls5Yh2Z1WgA10Nc/yPbVYU4HbF8Z98vvXFC8iqGTGsdDgqfe4=", + &address, + ), + "dashj's ECKeyTest.verifyMessage signature must verify here — a failure means the \ + signed-message format has diverged and CrowdNode will reject our signatures" + ); + } + + /// A signature over one of the wallet's own receive addresses verifies + /// against that address — the end-to-end contract, checked through the + /// public verification path rather than by re-deriving the key. + #[tokio::test] + async fn signature_verifies_for_the_signing_address() { + let (wm, wallet_id, signer, address) = + mnemonic_wallet_manager(MESSAGE_SIGNING_TEST_MNEMONIC).await; + let core = core_wallet(wm, wallet_id); + + let signature = core + .sign_message(&address.to_string(), MESSAGE, &signer) + .await + .expect("a derived receive address of this wallet must be signable"); + + assert!( + verifies_for(&signature, &address), + "signature {signature} must verify for {address}" + ); + } + + /// RFC6979-deterministic golden. The signer signs deterministically and the + /// mnemonic is fixed, so this exact base64 is stable across runs, platforms, + /// and rebuilds — a change here means the digest construction, the + /// derivation path, or the signature encoding moved. + /// + /// Cross-check against dashj by importing + /// [`MESSAGE_SIGNING_TEST_MNEMONIC`], deriving `m/44'/1'/0'/0/0` (testnet + /// coin type 1), and calling `ECKey.signMessage("hello")`; the strings must + /// be identical. Verified 2026-07-31 against dashj (`HDKeyDerivation` + + /// `ECKey.signMessage`, dashj-core test run): dashj produced this exact + /// base64 for this address, byte-for-byte. + #[tokio::test] + async fn roundtrip_signature_is_a_stable_golden() { + let (wm, wallet_id, signer, address) = + mnemonic_wallet_manager(MESSAGE_SIGNING_TEST_MNEMONIC).await; + let core = core_wallet(wm, wallet_id); + + // The first BIP44 external address of the fixed mnemonic on testnet, + // pinned alongside the signature so a derivation change fails loudly + // here rather than silently producing a valid signature for a + // different address. + assert_eq!( + address.to_string(), + "yRd4FhXfVGHXpsuZXPNkMrfD9GVj46pnjt", + "fixture address drifted — the golden signature below is for the old address" + ); + + let signature = core + .sign_message(&address.to_string(), MESSAGE, &signer) + .await + .expect("fixture address is signable"); + + assert_eq!( + signature, + "HzW1qnS7pYhopcbz1ZbiQY+axMMDQ2cMXBjey37IQS15OOluF6l/rfEre7Bl8AzUOwYdANkLYRelpKJmIH4kfZM=", + "RFC6979-deterministic golden vector" + ); + } + + /// An address the wallet does not own is refused as + /// `MessageSigningKeyUnavailable` (FFI code 31), not as a generic failure — + /// the host routes that code to key repair / address correction. + #[tokio::test] + async fn unknown_address_is_key_unavailable() { + let (wm, wallet_id, signer, _) = + mnemonic_wallet_manager(MESSAGE_SIGNING_TEST_MNEMONIC).await; + let core = core_wallet(wm, wallet_id); + + // A well-formed testnet P2PKH address from a different seed. + let (_, _, _, foreign) = mnemonic_wallet_manager( + "legal winner thank year wave sausage worth useful legal winner thank yellow", + ) + .await; + + let result = core + .sign_message(&foreign.to_string(), MESSAGE, &signer) + .await; + + match result { + Err(PlatformWalletError::MessageSigningKeyUnavailable { address }) => { + assert_eq!(address, foreign.to_string()); + } + other => panic!("expected MessageSigningKeyUnavailable, got {other:?}"), + } + } + + /// A mainnet address against a testnet wallet is rejected at the network + /// check, before any account lookup — so a mainnet address that happens to + /// share a payload with a testnet address the wallet owns can never be + /// signed for. + #[tokio::test] + async fn wrong_network_address_is_rejected() { + let (wm, wallet_id, signer, _) = + mnemonic_wallet_manager(MESSAGE_SIGNING_TEST_MNEMONIC).await; + let core = core_wallet(wm, wallet_id); + + let result = core + .sign_message("Xt5QmmzX2LaMgt81dcXGHhdf8pAhaVJKUW", MESSAGE, &signer) + .await; + + match result { + Err(PlatformWalletError::MessageSigningAddressInvalid { reason, .. }) => { + assert!( + reason.contains("another network"), + "expected a network-mismatch reason, got {reason:?}" + ); + } + other => panic!("expected MessageSigningAddressInvalid, got {other:?}"), + } + } +} From 32c9ee79e76e0b834cc84d4d5f6a086c7f7f1907 Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Fri, 31 Jul 2026 14:43:21 -0700 Subject: [PATCH 2/7] feat(sdk): signMessage exposed through JNI, Kotlin, and Swift MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Kotlin: ManagedPlatformWallet.signMessage(address, message, coreSignerHandle) suspend over the coreWalletSignMessage JNI binding. Swift: ManagedCoreWallet.signMessage(address:message:) constructing the per-call MnemonicResolver like every other seed-backed Swift call site. The Swift marshalling is what surfaced the FFI's empty-message contract bug (empty Swift strings marshal as a nil base address) — the read_utf8 len == 0 short-circuit shipped with the primitive commit. Co-Authored-By: Claude Fable 5 --- .../dashsdk/errors/DashSdkError.kt | 7 ++ .../dashsdk/ffi/WalletManagerNative.kt | 22 ++++ .../dashsdk/wallet/ManagedCoreWallet.kt | 18 ++++ .../dashsdk/wallet/ManagedPlatformWallet.kt | 73 +++++++++++++ .../rs-unified-sdk-jni/src/wallet_manager.rs | 90 ++++++++++++++++ .../CoreWallet/ManagedCoreWallet.swift | 101 ++++++++++++++++++ .../PlatformWallet/PlatformWalletResult.swift | 9 ++ 7 files changed, 320 insertions(+) diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/errors/DashSdkError.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/errors/DashSdkError.kt index 9c57d763b10..71150c50f4f 100644 --- a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/errors/DashSdkError.kt +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/errors/DashSdkError.kt @@ -192,6 +192,13 @@ sealed class DashSdkError( * `PlatformWalletManager.repairIdentityKey`) instead of treating it * as an opaque [Generic] failure. Not retryable as-is — the key must * be (re-)derived first. + * + * Also raised WITHOUT any signer round-trip by + * [org.dashfoundation.dashsdk.wallet.ManagedPlatformWallet.signMessage], + * for a message-signing address this wallet does not own — or owns only + * watch-only, since a DashPay *external* account holds a contact's + * addresses whose private keys we never had. Same conclusion, same host + * response: correct the address or repair the keys, do not retry. */ class SigningKeyUnavailable(message: String, cause: Throwable? = null) : PlatformWallet(message, cause) { diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/WalletManagerNative.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/WalletManagerNative.kt index 205fe225ba0..59d95957f95 100644 --- a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/WalletManagerNative.kt +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/WalletManagerNative.kt @@ -214,6 +214,28 @@ internal object WalletManagerNative { */ external fun platformWalletGetCore(walletHandle: Long): Long + /** + * `core_wallet_sign_message` — sign [message] with the private key behind + * [address] and return the base64 signature (a classic Dash signed message). + * + * [coreHandle] is a core-wallet handle from [platformWalletGetCore]. + * [address] must be a P2PKH address of THIS wallet on its network, owned by + * a signable funds account: a foreign or watch-only address throws + * `ErrorSigningKeyUnavailable` (31), while an unparseable, wrong-network, or + * non-P2PKH address throws `ErrorInvalidParameter` (2). [message] is signed + * verbatim — it is length-prefixed into the digest, so trailing whitespace + * and newlines are significant, and an empty string is valid and signable. + * [coreSignerHandle] is the manager's `MnemonicResolverHandle`. + * + * Moves no value: nothing is selected, reserved, broadcast, or persisted. + */ + external fun coreWalletSignMessage( + coreHandle: Long, + address: String, + message: String, + coreSignerHandle: Long, + ): String + /** * `core_wallet_broadcast_transaction` — broadcast a transaction built by * [coreTxBuilderBuildSigned]. [accountType]/[accountIndex] identify the diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedCoreWallet.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedCoreWallet.kt index 8a0e661d0ed..19f5429021a 100644 --- a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedCoreWallet.kt +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedCoreWallet.kt @@ -55,6 +55,24 @@ class ManagedCoreWallet internal constructor(handle: Long) : AutoCloseable { ) } + /** + * Sign [message] with the private key behind [address] and return the base64 + * signature — a classic Dash signed message. See + * [ManagedPlatformWallet.signMessage] for the full contract; drive this + * through it, not directly. + */ + internal fun signMessage( + address: String, + message: String, + coreSignerHandle: Long, + ): String = + WalletManagerNative.coreWalletSignMessage( + handle, + address, + message, + coreSignerHandle, + ) + override fun close() { cleanable.clean() } diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedPlatformWallet.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedPlatformWallet.kt index 27c846cd3b9..ea0b06d9ff7 100644 --- a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedPlatformWallet.kt +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedPlatformWallet.kt @@ -173,6 +173,79 @@ class ManagedPlatformWallet internal constructor( } } + /** + * Sign [message] with the private key behind [address] and return the + * signature as base64 — a **classic Dash signed message**, byte-for-byte + * compatible with dashj's `ECKey.signMessage` and Dash Core's `signmessage` + * RPC, and verifiable by `verifymessage`, `ECKey.verifyMessage`, and + * CrowdNode's server-side check. + * + * **The format.** The signed digest is + * `SHA256d(prefix ‖ varint(message.length) ‖ message)`, where the prefix is + * the historical `"\x19DarkCoin Signed Message:\n"` — *not* `"Dash"`. Dash + * inherited that string from before the rename and every existing verifier + * depends on it, so it can never change. The returned signature is the + * 65-byte BIP-137-style recoverable form (`header ‖ r ‖ s`, with + * `header = 27 + recoveryId + 4`, the `+ 4` marking a compressed public + * key), base64-encoded. A verifier recovers the public key from the digest + * and compares its hash to [address] — which is why only P2PKH addresses + * can sign: no other payload has a defined recovery comparison. + * + * **The CrowdNode use case.** This is the primitive the Android wallet's + * CrowdNode integration needs: it signs short strings — a withdrawal amount, + * the account email — which CrowdNode verifies against the address that + * funded the account. It is a **proof of address ownership, not a spend**: + * no UTXO is selected, reserved, spent, or broadcast, no balance changes, and + * nothing is persisted. Calling it repeatedly is free and side-effect-free. + * + * **Which addresses can sign.** [address] must be a P2PKH address of *this* + * wallet, on this wallet's network, already derived into one of its address + * pools, and belonging to a **signable funds account** (BIP44 / BIP32 / + * CoinJoin / DashPay-*receiving*). A watch-only DashPay **external** account + * holds a contact's receiving addresses whose private keys we never had, so + * those are refused exactly like any other address the wallet does not own: + * [org.dashfoundation.dashsdk.errors.DashSdkError.PlatformWallet.SigningKeyUnavailable]. + * + * An unparseable, wrong-network, or non-P2PKH address is caller input and + * surfaces natively as `ErrorInvalidParameter` (2), which has no dedicated + * Kotlin arm today and therefore arrives as + * [org.dashfoundation.dashsdk.errors.DashSdkError.PlatformWallet.Generic] + * with `code == 2`; the message names which of the three it was. Branch on + * [org.dashfoundation.dashsdk.errors.DashSdkError.PlatformWallet.SigningKeyUnavailable] + * to tell "wallet does not own this address" apart from "this is not a + * usable address". + * + * **Determinism.** Signing is RFC6979 deterministic and low-s normalized, so + * the same ([address], [message]) pair on the same seed always returns the + * same string. Two calls yielding different signatures means the key or the + * message differed. + * + * Runs through the manager's [TeardownGate] like every other native op. + * + * @param address the P2PKH address whose key signs; must be one this wallet + * owns and has derived. + * @param message the string to sign, **verbatim**. It is length-prefixed + * into the digest, so trailing whitespace and newlines are significant and + * the verifier must receive the identical bytes. An empty string is valid. + * @param coreSignerHandle the manager's `MnemonicResolverHandle` + * (`PlatformWalletManager.mnemonicResolverHandle`); no private key crosses + * the boundary. + * @return the base64 signature (88 characters for the 65-byte payload). + */ + suspend fun signMessage( + address: String, + message: String, + coreSignerHandle: Long, + ): String = gate.op { + require(address.isNotEmpty()) { "address must not be empty" } + + mapNativeErrors { + coreWallet().use { core -> + core.signMessage(address, message, coreSignerHandle) + } + } + } + /** * The wallet's Platform-payment addresses that currently hold credits, * each as a [FundingInput] whose `credits` is the full cached balance — diff --git a/packages/rs-unified-sdk-jni/src/wallet_manager.rs b/packages/rs-unified-sdk-jni/src/wallet_manager.rs index 78c25061fa6..3ae5ba099a5 100644 --- a/packages/rs-unified-sdk-jni/src/wallet_manager.rs +++ b/packages/rs-unified-sdk-jni/src/wallet_manager.rs @@ -1006,6 +1006,96 @@ pub extern "system" fn Java_org_dashfoundation_dashsdk_ffi_WalletManagerNative_c }) } +/// `core_wallet_sign_message` — sign `message` with the private key behind +/// `address` and return the base64 signature: a classic Dash signed message, +/// verifiable by Dash Core's `verifymessage` RPC, dashj's +/// `ECKey.verifyMessage`, and CrowdNode's server-side check. +/// +/// `core_handle` is the transient core-wallet `Handle` from +/// [platformWalletGetCore]. `address` must be a P2PKH address of THIS wallet on +/// its network, belonging to a signable funds account — a foreign or watch-only +/// address throws `ErrorSigningKeyUnavailable` (31), while an unparseable, +/// wrong-network, or non-P2PKH address throws `ErrorInvalidParameter` (2). +/// `message` is signed verbatim (it is length-prefixed into the digest, so +/// trailing whitespace is significant). `core_signer_handle` is the manager's +/// `MnemonicResolverHandle`. +/// +/// Moves no value: nothing is selected, reserved, broadcast, or persisted. +/// Returns the base64 signature as a `String`, or null after throwing. +#[no_mangle] +pub extern "system" fn Java_org_dashfoundation_dashsdk_ffi_WalletManagerNative_coreWalletSignMessage( + mut env: JNIEnv, + _class: JClass, + core_handle: jlong, + address: JString, + message: JString, + core_signer_handle: jlong, +) -> jstring { + guard(&mut env, ptr::null_mut(), |env| { + if core_handle == 0 { + throw_sdk_exception(env, 1, "core handle is 0"); + return ptr::null_mut(); + } + if core_signer_handle == 0 { + throw_sdk_exception(env, 1, "coreSignerHandle is 0"); + return ptr::null_mut(); + } + let Some(address) = read_cstring_required(env, &address, "address") else { + return ptr::null_mut(); + }; + // The message is read leniently on emptiness — unlike `address`, an empty + // string is a legitimate thing to sign (the digest length-prefixes it), + // so `read_cstring_required` (which rejects empty) is wrong here. A JNI + // read error still throws: silently signing the empty message when the + // caller supplied text would produce a signature that verifies for a + // message they never sent. + if message.is_null() { + throw_sdk_exception(env, 1, "message was null"); + return ptr::null_mut(); + } + let message: String = match env.get_string(&message) { + Ok(v) => v.into(), + Err(_) => { + let _ = env.exception_clear(); + throw_sdk_exception(env, 1, "message string was invalid"); + return ptr::null_mut(); + } + }; + + // Both cross as UTF-8 bytes + length (no trailing NUL), so an embedded + // NUL cannot truncate what actually gets signed. + let address_bytes = address.as_bytes(); + let message_bytes = message.as_bytes(); + + let mut out_signature: *mut c_char = ptr::null_mut(); + let result = unsafe { + platform_wallet_ffi::core_wallet_sign_message( + core_handle as Handle, + address_bytes.as_ptr(), + address_bytes.len(), + message_bytes.as_ptr(), + message_bytes.len(), + core_signer_handle as *mut rs_sdk_ffi::MnemonicResolverHandle, + &mut out_signature as *mut *mut c_char, + ) + }; + if take_pwffi_error(env, result) { + return ptr::null_mut(); + } + if out_signature.is_null() { + throw_sdk_exception(env, 1, "sign_message returned a NULL signature"); + return ptr::null_mut(); + } + let signature = unsafe { CStr::from_ptr(out_signature) } + .to_string_lossy() + .into_owned(); + unsafe { platform_wallet_ffi::core_wallet_free_address(out_signature) }; + env.new_string(signature) + .map(|s| s.into_raw()) + .unwrap_or(ptr::null_mut()) + }) +} + /// `platform_wallet_get_core` — resolve the transient core-wallet `Handle` /// (as `jlong`) from a `PlatformWallet` handle, for [coreWalletBroadcastTransaction]. /// Free with [coreWalletDestroy]. Returns 0 after throwing. diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/CoreWallet/ManagedCoreWallet.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/CoreWallet/ManagedCoreWallet.swift index 553424f6a34..b680fbb1e26 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/CoreWallet/ManagedCoreWallet.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/CoreWallet/ManagedCoreWallet.swift @@ -134,6 +134,107 @@ public class ManagedCoreWallet { try core_wallet_set_gap_limit(handle, accountType.ffi, accountIndex, gapLimit).check() } + // MARK: - Message Signing + + /// Sign `message` with the private key behind `address` and return the + /// signature as base64 — a **classic Dash signed message**, byte-for-byte + /// compatible with dashj's `ECKey.signMessage` and Dash Core's `signmessage` + /// RPC, and verifiable by `verifymessage`, `ECKey.verifyMessage`, and + /// CrowdNode's server-side check. + /// + /// The signed digest is `SHA256d(prefix ‖ varint(message.count) ‖ message)`, + /// where the prefix is the historical `"\u{19}DarkCoin Signed Message:\n"` — + /// *not* `"Dash"`. Dash inherited that string from before the rename and + /// every existing verifier depends on it, so it can never change. The + /// returned signature is the 65-byte BIP-137-style recoverable form + /// (`header ‖ r ‖ s`, with `header = 27 + recoveryId + 4`, the `+ 4` marking + /// a compressed public key), base64-encoded. A verifier recovers the public + /// key from the digest and compares its hash to `address` — which is why + /// only P2PKH addresses can sign: no other payload has a defined recovery + /// comparison. + /// + /// This is a **proof of address ownership, not a spend** — the shape + /// CrowdNode needs, where short strings (a withdrawal amount, the account + /// email) are signed with the key behind the address that funded the + /// account. No UTXO is selected, reserved, spent, or broadcast, no balance + /// changes, and nothing is persisted, so calling it repeatedly is free and + /// side-effect-free. + /// + /// `address` must be a P2PKH address of *this* wallet, on this wallet's + /// network, already derived into one of its address pools, and belonging to + /// a **signable funds account** (BIP44 / BIP32 / CoinJoin / + /// DashPay-*receiving*). A watch-only DashPay **external** account holds a + /// contact's receiving addresses whose private keys we never had, so those + /// are refused exactly like any other address the wallet does not own. + /// + /// Signing is RFC6979 deterministic and low-s normalized, so the same + /// (`address`, `message`) pair on the same seed always returns the same + /// string. + /// + /// The key is derived through the Keychain mnemonic resolver Rust-side, like + /// every other seed-backed path here, so no private key crosses the boundary + /// and no resident seed is needed. + /// + /// - Parameters: + /// - address: the P2PKH address whose key signs; must be one this wallet + /// owns and has derived. + /// - message: the string to sign, **verbatim**. It is length-prefixed into + /// the digest, so trailing whitespace and newlines are significant and + /// the verifier must receive the identical bytes. An empty string is + /// valid and signable. + /// - Returns: the base64 signature (88 characters for the 65-byte payload). + /// - Throws: `PlatformWalletError.signingKeyUnavailable` when `address` + /// belongs to no signable funds account of this wallet; + /// `PlatformWalletError.invalidParameter` when it is unparseable, encoded + /// for another network, or not P2PKH. + public func signMessage(address: String, message: String) throws -> String { + guard !address.isEmpty else { + throw PlatformWalletError.invalidParameter("address must not be empty") + } + + // Resolver-backed core signer, as on every other seed-backed path. + let coreSigner = MnemonicResolver() + // Both arguments cross as raw UTF-8 bytes with an explicit length (no NUL + // terminator), so an embedded NUL cannot truncate what actually gets + // signed. `address` is non-empty per the guard above, so its + // `baseAddress` is non-nil; an empty `message` yields a nil + // `baseAddress`, which the FFI reads as the empty message at length 0 — + // the one case where it accepts a null pointer. + let addressBytes = Array(address.utf8) + let messageBytes = Array(message.utf8) + + var signaturePtr: UnsafeMutablePointer? = nil + // `withExtendedLifetime` keeps the resolver alive across the synchronous + // FFI call — the optimizer can otherwise drop it mid-call and a vtable + // callback would use-after-free. + let result: PlatformWalletFFIResult = withExtendedLifetime(coreSigner) { + addressBytes.withUnsafeBufferPointer { + addressBuf -> PlatformWalletFFIResult in + messageBytes.withUnsafeBufferPointer { + messageBuf -> PlatformWalletFFIResult in + core_wallet_sign_message( + handle, + addressBuf.baseAddress, + UInt(addressBuf.count), + messageBuf.baseAddress, + UInt(messageBuf.count), + coreSigner.handle, + &signaturePtr + ) + } + } + } + try result.check() + + guard let ptr = signaturePtr else { + throw PlatformWalletError.nullPointer( + "core_wallet_sign_message returned a NULL signature pointer" + ) + } + defer { core_wallet_free_address(ptr) } + return String(cString: ptr) + } + // MARK: - Transactions /// Broadcast a transaction built by `CoreTransactionBuilder.buildSigned`. diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swift index a194918ad55..4cdb3c7a1d0 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swift @@ -84,6 +84,11 @@ public enum PlatformWalletResultCode: Int32, Sendable { /// usable private key for the requested public key — restored from the /// structured signer completion code (dashpay/platform#4060 finding 7). /// Route to key repair; not retryable as-is. + /// + /// Also produced with no signer round-trip by + /// `ManagedCoreWallet.signMessage`, for a message-signing address this + /// wallet does not own — or owns only watch-only, a DashPay *external* + /// account holding a contact's addresses. case errorSigningKeyUnavailable = 31 case notFound = 98 case errorUnknown = 99 @@ -279,6 +284,10 @@ public enum PlatformWalletError: LocalizedError { /// Restored from the structured signer completion code /// (dashpay/platform#4060 finding 7); route to key repair. Kotlin /// parity: `DashSdkError.PlatformWallet.SigningKeyUnavailable`. + /// + /// `signMessage` also raises it for an address the wallet does not own. + /// Distinct from `invalidParameter`, which means the address itself is + /// unusable (unparseable, wrong network, or not P2PKH). case signingKeyUnavailable(String) case notFound(String) case unknown(String) From e5e63c034c8c5720142040ce1d717c61cc9121af Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Sat, 1 Aug 2026 08:44:52 -0700 Subject: [PATCH 3/7] fix(platform-wallet): refuse non-digest signers, and stop holding the handle guard across the host callback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review findings 1, 3 and 4 on #4259. `Signer` assigns method dispatch to the caller and documents `sign_ecdsa` as valid only when the backend advertises `SignerMethod::Digest`. `sign_message` called it unconditionally, so a transaction-only hardware signer — one whose entire policy is to re-hash and display what it signs — could reject it, panic on an unreachable method, or blind-sign the host's digest and defeat that policy. Refused up front instead, mirroring the pre-check key-wallet performs in `TransactionSigner::sig_and_pubkey`. The refusal reuses `MessageSigningFailed` rather than taking a new code: this is the crate's "a path resolved but no signature came back" bucket, key-wallet folds the same refusal into `BuilderError::SigningFailed`, and it is unreachable with any shipping signer, so it does not justify an FFI code and the host mirror-enum churn one brings. The FFI entry point held `HandleStorage`'s process-global read guard across `runtime().block_on(...)` — and therefore across the host mnemonic-resolver callback, which in production is a Keychain/Keystore call that can block on a biometric prompt. That stalled every other wallet-handle operation for as long as the user took to respond, and would deadlock if the callback re-entered the FFI on a write-guard path. Clones the Arc-backed wallet out first, like `core_wallet_broadcast_*`. Tests: a transaction-only signer whose `sign_ecdsa` panics, proving the refusal happens before the backend is reached; `""` signed end-to-end and verified against `signed_msg_hash("")`, which nothing covered; and the FFI boundary contract that a null `message_ptr` at length 0 is accepted (verified to fail if an unconditional `check_ptr!(message_ptr)` returns). Co-Authored-By: Claude Fable 5 --- .../src/core_wallet/sign_message.rs | 158 ++++++++++++++---- packages/rs-platform-wallet/src/error.rs | 23 ++- .../src/wallet/core/sign_message.rs | 135 ++++++++++++++- 3 files changed, 278 insertions(+), 38 deletions(-) diff --git a/packages/rs-platform-wallet-ffi/src/core_wallet/sign_message.rs b/packages/rs-platform-wallet-ffi/src/core_wallet/sign_message.rs index 62422ebb97e..20eb0faacbd 100644 --- a/packages/rs-platform-wallet-ffi/src/core_wallet/sign_message.rs +++ b/packages/rs-platform-wallet-ffi/src/core_wallet/sign_message.rs @@ -15,7 +15,7 @@ use crate::error::*; use crate::handle::{Handle, CORE_WALLET_STORAGE}; use crate::runtime::runtime; -use crate::{check_ptr, unwrap_option_or_return, unwrap_result_or_return}; +use crate::{check_ptr, unwrap_result_or_return}; use platform_wallet::PlatformWalletError; use rs_sdk_ffi::{MnemonicResolverCoreSigner, MnemonicResolverHandle}; use std::ffi::CString; @@ -106,38 +106,138 @@ pub unsafe extern "C" fn core_wallet_sign_message( let signer_addr = core_signer_handle as usize; - let option = CORE_WALLET_STORAGE.with_item(handle, |wallet| { - let address = read_utf8(address_ptr, address_len, |e| { - PlatformWalletError::MessageSigningAddressInvalid { - address: "".to_string(), - reason: format!("address is not valid UTF-8: {e}"), - } - })?; - // A non-UTF-8 message is reported against the (now known) address, so the - // error names the signing target the caller asked about. - let message = read_utf8(message_ptr, message_len, |e| { - PlatformWalletError::MessageSigningFailed { - address: address.clone(), - reason: format!("message is not valid UTF-8: {e}"), - } - })?; - let network = wallet.network(); - let wallet_id = wallet.wallet_id(); - // SAFETY: `signer_addr` came from `core_signer_handle`, which the caller - // pinned alive for this call; the `MnemonicResolverCoreSigner` lives - // only on this stack frame and is dropped before returning. - let signer = MnemonicResolverCoreSigner::new( - signer_addr as *mut MnemonicResolverHandle, - wallet_id, - network, + // Clone the Arc-backed `CoreWallet` OUT of handle storage before doing any + // work. `with_item` holds `HandleStorage`'s process-global read guard for + // the whole closure, and the signing round-trip below re-enters the HOST + // through the mnemonic-resolver callback — which in production is a + // Keychain / Android Keystore call that can block on user presence + // (biometric or PIN prompt). Holding a global read guard across that would + // stall every other wallet-handle operation for as long as the user takes + // to respond, and deadlock outright if the host callback re-enters the FFI + // on any path that needs the write guard (parking_lot's RwLock is neither + // reentrant nor reader-preferring, so one waiting writer blocks us). + // + // Mirrors `core_wallet_broadcast_signed_transaction*`, which clone out for + // the same reason. + let Some(wallet) = CORE_WALLET_STORAGE.with_item(handle, Clone::clone) else { + return PlatformWalletFFIResult::err( + PlatformWalletFFIResultCode::ErrorInvalidHandle, + "invalid core wallet handle".to_string(), ); - runtime().block_on(wallet.sign_message(&address, &message, &signer)) - }); + }; + + let address = unwrap_result_or_return!(read_utf8(address_ptr, address_len, |e| { + PlatformWalletError::MessageSigningAddressInvalid { + address: "".to_string(), + reason: format!("address is not valid UTF-8: {e}"), + } + })); + // A non-UTF-8 message is reported against the (now known) address, so the + // error names the signing target the caller asked about. + let message = unwrap_result_or_return!(read_utf8(message_ptr, message_len, |e| { + PlatformWalletError::MessageSigningFailed { + address: address.clone(), + reason: format!("message is not valid UTF-8: {e}"), + } + })); - let result = unwrap_option_or_return!(option); - let signature = unwrap_result_or_return!(result); + // SAFETY: `signer_addr` came from `core_signer_handle`, which the caller + // pinned alive for this call; the `MnemonicResolverCoreSigner` lives only on + // this stack frame and is dropped before returning. + let signer = MnemonicResolverCoreSigner::new( + signer_addr as *mut MnemonicResolverHandle, + wallet.wallet_id(), + wallet.network(), + ); + let signature = unwrap_result_or_return!( + runtime().block_on(wallet.sign_message(&address, &message, &signer)) + ); let c_str = unwrap_result_or_return!(CString::new(signature)); *out_signature = c_str.into_raw(); PlatformWalletFFIResult::ok() } + +#[cfg(test)] +mod tests { + use super::*; + use rs_sdk_ffi::{dash_sdk_mnemonic_resolver_create, dash_sdk_mnemonic_resolver_destroy}; + use std::os::raw::c_void; + + unsafe extern "C" fn never_resolve( + _ctx: *const c_void, + _wallet_id_bytes: *const u8, + _out_buf: *mut c_char, + _out_capacity: usize, + _out_len: *mut usize, + ) -> i32 { + unreachable!("the handle is rejected long before any mnemonic is resolved"); + } + + unsafe extern "C" fn noop_destroy(_ctx: *mut c_void) {} + + /// `read_utf8` must treat `len == 0` as the empty string without ever + /// touching `ptr` — a null pointer at length 0 is the shape Swift produces + /// for an empty `String`, and `from_raw_parts` is UB on null even for an + /// empty slice. + #[test] + fn read_utf8_accepts_a_null_pointer_at_zero_length() { + let read = unsafe { + read_utf8(std::ptr::null(), 0, |e| { + panic!("must not attempt a UTF-8 decode: {e}") + }) + }; + assert_eq!(read.expect("null at length 0 is the empty string"), ""); + } + + /// **The empty-message contract at the boundary.** An empty Swift `String` + /// marshals to a nil base address with length 0, so + /// `core_wallet_sign_message` must NOT reject a null `message_ptr` — only + /// `address_ptr`, `core_signer_handle` and `out_signature` are + /// null-checked. + /// + /// Asserted by discrimination rather than by signing: with a deliberately + /// invalid wallet handle the call must fail at the handle lookup + /// (`ErrorInvalidHandle`), which it can only reach by having accepted the + /// null message. If someone later adds an unconditional + /// `check_ptr!(message_ptr)`, the pointer checks run first and the code + /// becomes `ErrorNullPointer` — failing this test and catching exactly the + /// regression that would break iOS signing of an empty string. + /// + /// The semantic half — that `""` actually signs and verifies against + /// `signed_msg_hash("")` — is pinned by `empty_message_is_signable` in + /// `platform_wallet::wallet::core::sign_message`, where the wallet fixtures + /// live. + #[test] + fn empty_message_is_not_a_null_pointer_error() { + let resolver = unsafe { + dash_sdk_mnemonic_resolver_create(std::ptr::null_mut(), never_resolve, noop_destroy) + }; + assert!(!resolver.is_null(), "resolver handle must be created"); + + let address = b"yRd4FhXfVGHXpsuZXPNkMrfD9GVj46pnjt"; + let mut out_signature: *mut c_char = std::ptr::null_mut(); + + let result = unsafe { + core_wallet_sign_message( + Handle::MAX, // never a live core-wallet handle + address.as_ptr(), + address.len(), + std::ptr::null(), // the empty message, as Swift marshals it + 0, + resolver, + &mut out_signature, + ) + }; + + assert_eq!( + result.code, + PlatformWalletFFIResultCode::ErrorInvalidHandle, + "a null message at length 0 must be accepted and the call must fail \ + at the handle lookup, not as a null-pointer error" + ); + assert!(out_signature.is_null()); + + unsafe { dash_sdk_mnemonic_resolver_destroy(resolver) }; + } +} diff --git a/packages/rs-platform-wallet/src/error.rs b/packages/rs-platform-wallet/src/error.rs index ef31312289b..ad54f5be56e 100644 --- a/packages/rs-platform-wallet/src/error.rs +++ b/packages/rs-platform-wallet/src/error.rs @@ -138,12 +138,22 @@ pub enum PlatformWalletError { MessageSigningKeyUnavailable { address: String }, /// [`CoreWallet::sign_message`] resolved a derivation path for the address - /// but could not produce a signature over it. Three causes, all carried in - /// `reason`: the [`Signer`] itself failed (Keystore/Keychain round-trip); - /// the public key it returned does not hash to the target address (a - /// path-resolution bug — the guard exists so a wrong-key signature can - /// never be handed out as if it were the address owner's); or no recovery - /// id in `0..=3` recovers that public key. + /// but could not produce a signature over it. Four causes, all carried in + /// `reason`: the signer backend does not advertise + /// [`SignerMethod::Digest`], so it cannot sign a host-computed digest at + /// all and is refused before it is ever invoked; the [`Signer`] itself + /// failed (Keystore/Keychain round-trip); the public key it returned does + /// not hash to the target address (a path-resolution bug — the guard exists + /// so a wrong-key signature can never be handed out as if it were the + /// address owner's); or no recovery id in `0..=3` recovers that public key. + /// + /// The capability refusal shares this variant rather than taking a + /// dedicated one because this is the crate's "a path resolved but no + /// signature came back" bucket, and because key-wallet folds the very same + /// refusal into its ordinary `BuilderError::SigningFailed`. It is + /// unreachable with any signer that ships today — the production mnemonic + /// resolver advertises `Digest` — so it does not warrant a new FFI code and + /// the host mirror-enum churn that follows one. /// /// Deliberately NOT given a dedicated FFI code: a signer that reports its /// typed key-unavailable completion carries the stable machine prefix in @@ -154,6 +164,7 @@ pub enum PlatformWalletError { /// /// [`CoreWallet::sign_message`]: crate::wallet::core::CoreWallet::sign_message /// [`Signer`]: key_wallet::signer::Signer + /// [`SignerMethod::Digest`]: key_wallet::signer::SignerMethod::Digest #[error("message signing failed for address {address}: {reason}")] MessageSigningFailed { address: String, reason: String }, diff --git a/packages/rs-platform-wallet/src/wallet/core/sign_message.rs b/packages/rs-platform-wallet/src/wallet/core/sign_message.rs index dddb4c541d3..0c9dc3aec27 100644 --- a/packages/rs-platform-wallet/src/wallet/core/sign_message.rs +++ b/packages/rs-platform-wallet/src/wallet/core/sign_message.rs @@ -50,7 +50,7 @@ use dashcore::secp256k1::{Message, Secp256k1}; use dashcore::sign_message::{signed_msg_hash, MessageSignature}; use dashcore::{Address as DashAddress, AddressType, PublicKey as DashPublicKey}; use key_wallet::managed_account::managed_account_trait::ManagedAccountTrait; -use key_wallet::signer::Signer; +use key_wallet::signer::{Signer, SignerMethod}; use key_wallet::ManagedAccountType; use crate::broadcaster::TransactionBroadcaster; @@ -114,7 +114,11 @@ impl CoreWallet { /// verifier must be handed the identical bytes. /// * `signer` — the ECDSA signer that holds the key material (the /// Keystore/Keychain-backed `MnemonicResolverCoreSigner` in production). - /// No private key crosses the boundary. + /// No private key crosses the boundary. It must advertise + /// [`SignerMethod::Digest`]; a `Transaction`-only backend cannot sign a + /// message and is refused with + /// [`MessageSigningFailed`](PlatformWalletError::MessageSigningFailed) + /// before it is invoked. /// /// # Determinism /// @@ -186,6 +190,31 @@ impl CoreWallet { (target, path) }; + // `Signer` advertises the methods it can perform and assigns dispatch to + // the CALLER: `sign_ecdsa` is documented as valid only when the backend + // supports `SignerMethod::Digest`. A classic signed message is + // inherently a host-computed-digest operation — there is no transaction + // for a device to re-hash and display — so a `Transaction`-only backend + // (a hardware wallet whose whole policy is to never blind-sign) cannot + // serve this call, and asking it to would defeat that policy. + // + // Refused here rather than left to the backend because an unadvertised + // method has no defined behavior: an implementation may reject it, panic + // because it believed the method unreachable, or — worst — blind-sign + // anyway. Mirrors the identical pre-check key-wallet performs before its + // own `sign_ecdsa` dispatch in `TransactionSigner::sig_and_pubkey`. + if !signer.supports(SignerMethod::Digest) { + return Err(PlatformWalletError::MessageSigningFailed { + address: target.to_string(), + reason: format!( + "signer backend cannot sign message digests: it advertises {:?}, but a \ + classic signed message requires {:?}", + signer.supported_methods(), + SignerMethod::Digest, + ), + }); + } + let hash = signed_msg_hash(message); let (signature, public_key) = signer .sign_ecdsa(&path, hash.to_byte_array()) @@ -246,9 +275,11 @@ mod tests { use std::str::FromStr; use std::sync::Arc; - use dashcore::secp256k1::Secp256k1; + use dashcore::secp256k1::{ecdsa, PublicKey, Secp256k1}; use dashcore::sign_message::{signed_msg_hash, MessageSignature}; use dashcore::{Address as DashAddress, Network}; + use key_wallet::signer::{Signer, SignerMethod, TransactionCategory}; + use key_wallet::DerivationPath; use crate::test_support::{ mnemonic_wallet_manager, AlwaysRejectedBroadcaster, MESSAGE_SIGNING_TEST_MNEMONIC, @@ -320,6 +351,36 @@ mod tests { ); } + /// **The empty message is signable.** Nothing about the format excludes it — + /// the digest length-prefixes the message, so `""` prefixes as `varint(0)` + /// and hashes to a perfectly ordinary digest. + /// + /// This is a contract the boundary layers depend on, not a curiosity: an + /// empty Swift `String` marshals to a **nil** base address with length 0, so + /// the FFI must accept a null `message_ptr` at `message_len == 0` rather + /// than rejecting it as a null-pointer error. Signing `""` end-to-end here + /// pins the semantic half of that contract; `empty_message_is_not_a_null_ + /// pointer_error` in the FFI crate pins the boundary half. + #[tokio::test] + async fn empty_message_is_signable() { + let (wm, wallet_id, signer, address) = + mnemonic_wallet_manager(MESSAGE_SIGNING_TEST_MNEMONIC).await; + let core = core_wallet(wm, wallet_id); + + let signature = core + .sign_message(&address.to_string(), "", &signer) + .await + .expect("the empty message is signable"); + + assert!( + MessageSignature::from_base64(&signature) + .expect("valid base64 of a 65-byte recoverable signature") + .is_signed_by_address(&Secp256k1::new(), &address, signed_msg_hash("")) + .expect("P2PKH address is a supported verification target"), + "a signature over the empty message must verify against signed_msg_hash(\"\")" + ); + } + /// A signature over one of the wallet's own receive addresses verifies /// against that address — the end-to-end contract, checked through the /// public verification path rather than by re-deriving the key. @@ -430,4 +491,72 @@ mod tests { other => panic!("expected MessageSigningAddressInvalid, got {other:?}"), } } + + /// A hardware-style backend that can only sign whole transactions it + /// re-hashes and displays, and never a host-computed digest. `sign_ecdsa` + /// panics rather than returning an error: the `Signer` contract makes an + /// unadvertised method undefined to call, so reaching it at all is the bug + /// this fixture exists to catch. A backend that instead blind-signed would + /// silently defeat the policy it advertises, which no `Result` could + /// express. + struct TransactionOnlySigner; + + #[async_trait::async_trait] + impl Signer for TransactionOnlySigner { + type Error = String; + + fn supported_methods(&self) -> &[SignerMethod] { + &[SignerMethod::Transaction(TransactionCategory::Classical)] + } + + async fn sign_ecdsa( + &self, + _path: &DerivationPath, + _sighash: [u8; 32], + ) -> Result<(ecdsa::Signature, PublicKey), Self::Error> { + panic!( + "sign_ecdsa must never be reached on a signer that does not advertise \ + SignerMethod::Digest — sign_message is required to refuse first" + ); + } + + async fn public_key(&self, _path: &DerivationPath) -> Result { + panic!("public_key is not part of the signed-message path"); + } + } + + /// **Capability dispatch.** `Signer` assigns method dispatch to the caller, + /// and documents `sign_ecdsa` as valid only when the backend advertises + /// `SignerMethod::Digest`. A transaction-only backend must therefore be + /// refused with a typed error *before* it is invoked — the fixture's + /// `sign_ecdsa` panics, so a regression that drops the guard fails loudly + /// here rather than blind-signing on a device whose policy forbids it. + /// + /// The address is one the wallet genuinely owns, so the refusal cannot be + /// mistaken for the key-unavailable path: this pins the capability check + /// specifically. + #[tokio::test] + async fn transaction_only_signer_is_refused_before_signing() { + let (wm, wallet_id, _, address) = + mnemonic_wallet_manager(MESSAGE_SIGNING_TEST_MNEMONIC).await; + let core = core_wallet(wm, wallet_id); + + let result = core + .sign_message(&address.to_string(), MESSAGE, &TransactionOnlySigner) + .await; + + match result { + Err(PlatformWalletError::MessageSigningFailed { + address: reported, + reason, + }) => { + assert_eq!(reported, address.to_string()); + assert!( + reason.contains("cannot sign message digests"), + "the reason must name the capability refusal, got {reason:?}" + ); + } + other => panic!("expected MessageSigningFailed, got {other:?}"), + } + } } From 9ff9941868e319f039957cb711c45ca62ea91ed1 Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Sat, 1 Aug 2026 08:45:12 -0700 Subject: [PATCH 4/7] fix(sdk): reject unpaired surrogates before signing, and correct the length-prefix docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review findings 2 and 5 on #4259. The Kotlin and Swift docs described the digest's length prefix in each language's own string units — `message.length` (UTF-16 code units) and `message.count` (grapheme clusters). The format prefixes the UTF-8 BYTE count; the three agree only for ASCII. Corrected to `message.toByteArray(Charsets.UTF_8).size` and `message.utf8.count`. That doc bug has a real counterpart: a Kotlin `String` is an unvalidated UTF-16 sequence and may hold an unpaired surrogate, which has no UTF-8 encoding. Every conversion below is lenient and they do not even agree — the JNI bridge's string read substitutes U+FFFD, while `toByteArray(Charsets.UTF_8)` substitutes '?' (verified on JDK 17). So the wallet would sign bytes the caller never wrote and return a signature that verifies for a different message, silently, and "verbatim" in the docs would be false. Rejected at the Kotlin entry point, the last layer that still holds the exact UTF-16 and can explain why. Marshalling a UTF-8 ByteArray across the JNI instead — the other option raised in review — would NOT fix this: Kotlin's own encoder is equally lossy, so it would relocate the silent substitution and change it from U+FFFD to '?' while widening the FFI surface. Well-formed strings are unaffected. The lossy `get_string` read is the shared `read_cstring_required` behaviour on every JNI string parameter in the crate, so it is a codebase-wide convention rather than something this path introduced; flagged rather than changed here. Co-Authored-By: Claude Fable 5 --- .../dashsdk/wallet/ManagedPlatformWallet.kt | 51 +++++++++++++++++-- .../CoreWallet/ManagedCoreWallet.swift | 7 ++- 2 files changed, 52 insertions(+), 6 deletions(-) diff --git a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedPlatformWallet.kt b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedPlatformWallet.kt index ea0b06d9ff7..247ed4cf972 100644 --- a/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedPlatformWallet.kt +++ b/packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedPlatformWallet.kt @@ -181,7 +181,10 @@ class ManagedPlatformWallet internal constructor( * CrowdNode's server-side check. * * **The format.** The signed digest is - * `SHA256d(prefix ‖ varint(message.length) ‖ message)`, where the prefix is + * `SHA256d(prefix ‖ varint(bytes.size) ‖ bytes)` over + * `bytes = message.toByteArray(Charsets.UTF_8)`: the length prefix counts + * **UTF-8 bytes**, not `String.length`, which counts UTF-16 code units and + * diverges for any non-ASCII text. The prefix is * the historical `"\x19DarkCoin Signed Message:\n"` — *not* `"Dash"`. Dash * inherited that string from before the rename and every existing verifier * depends on it, so it can never change. The returned signature is the @@ -224,9 +227,13 @@ class ManagedPlatformWallet internal constructor( * * @param address the P2PKH address whose key signs; must be one this wallet * owns and has derived. - * @param message the string to sign, **verbatim**. It is length-prefixed - * into the digest, so trailing whitespace and newlines are significant and - * the verifier must receive the identical bytes. An empty string is valid. + * @param message the string to sign, **verbatim** as UTF-8. It is + * length-prefixed into the digest, so trailing whitespace and newlines are + * significant and the verifier must receive the identical bytes. An empty + * string is valid. Must be well-formed text: a string holding an unpaired + * UTF-16 surrogate has no UTF-8 encoding and is rejected with + * [IllegalArgumentException] rather than signed after a silent + * substitution. * @param coreSignerHandle the manager's `MnemonicResolverHandle` * (`PlatformWalletManager.mnemonicResolverHandle`); no private key crosses * the boundary. @@ -238,6 +245,18 @@ class ManagedPlatformWallet internal constructor( coreSignerHandle: Long, ): String = gate.op { require(address.isNotEmpty()) { "address must not be empty" } + // The digest commits to the message's UTF-8 bytes, but a Kotlin String + // is an unvalidated UTF-16 sequence and may hold an unpaired surrogate, + // which has no UTF-8 encoding at all. Every conversion below is LENIENT + // and they do not even agree: the JNI bridge's String read substitutes + // U+FFFD, while `toByteArray(Charsets.UTF_8)` substitutes '?'. Either + // way the wallet would sign bytes the caller never wrote and hand back a + // signature that verifies for a different message — silently. Rejected + // here, the one layer that still has the exact UTF-16 and can say why. + require(message.hasNoUnpairedSurrogate()) { + "message must be well-formed text: it contains an unpaired UTF-16 surrogate, " + + "which has no UTF-8 encoding and would be silently substituted before signing" + } mapNativeErrors { coreWallet().use { core -> @@ -246,6 +265,30 @@ class ManagedPlatformWallet internal constructor( } } + /** + * Whether every UTF-16 surrogate in this string is part of a well-formed + * high/low pair — i.e. whether the string has an exact UTF-8 encoding. + * + * Scanned directly rather than via a strict `CharsetEncoder` to keep the + * check allocation-free on the hot path; the two agree on exactly which + * strings are encodable. + */ + private fun String.hasNoUnpairedSurrogate(): Boolean { + var i = 0 + while (i < length) { + val c = this[i] + when { + c.isHighSurrogate() -> { + if (i + 1 >= length || !this[i + 1].isLowSurrogate()) return false + i += 2 + } + c.isLowSurrogate() -> return false + else -> i++ + } + } + return true + } + /** * The wallet's Platform-payment addresses that currently hold credits, * each as a [FundingInput] whose `credits` is the full cached balance — diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/CoreWallet/ManagedCoreWallet.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/CoreWallet/ManagedCoreWallet.swift index b680fbb1e26..012cfea638c 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/CoreWallet/ManagedCoreWallet.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/CoreWallet/ManagedCoreWallet.swift @@ -142,8 +142,11 @@ public class ManagedCoreWallet { /// RPC, and verifiable by `verifymessage`, `ECKey.verifyMessage`, and /// CrowdNode's server-side check. /// - /// The signed digest is `SHA256d(prefix ‖ varint(message.count) ‖ message)`, - /// where the prefix is the historical `"\u{19}DarkCoin Signed Message:\n"` — + /// The signed digest is + /// `SHA256d(prefix ‖ varint(message.utf8.count) ‖ message.utf8)`: the length + /// prefix counts **UTF-8 bytes**, not `String.count`, which counts grapheme + /// clusters and diverges for any non-ASCII text. + /// The prefix is the historical `"\u{19}DarkCoin Signed Message:\n"` — /// *not* `"Dash"`. Dash inherited that string from before the rename and /// every existing verifier depends on it, so it can never change. The /// returned signature is the 65-byte BIP-137-style recoverable form From 5b77dfd8f18b1d33f4593e2484283f8d841fb9dc Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Tue, 4 Aug 2026 00:17:49 -0700 Subject: [PATCH 5/7] fix(platform-wallet-ffi): malformed message bytes are caller input, not an internal failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding on #4259. `core_wallet_sign_message` takes the message as raw bytes, so it can be handed something that is not valid UTF-8. That mapped through `MessageSigningFailed` to `ErrorUnknown` — telling the host "internal failure" about an argument it could simply fix, while the *address* argument in the same function already reported malformed bytes as `ErrorInvalidParameter`. Two equivalent caller-input errors, two different answers. Adds a dedicated `MessageSigningMessageInvalid` and maps it to ErrorInvalidParameter, matching the address arm. A dedicated variant rather than reusing `MessageSigningAddressInvalid`: that variant renders "message-signing address {address} is unusable", so reusing it would blame a perfectly valid address for a message problem. These variants exist precisely to name which argument the caller must fix — the crate already splits AddressInvalid from KeyUnavailable on the same principle — and it maps to the already-mirrored ErrorInvalidParameter, so no new numeric code and no Kotlin/Swift mirror churn. Reachable only across the FFI; a Rust or Kotlin caller cannot build an ill-formed string. `read_utf8`'s contract, the `message_ptr` parameter doc and the `# Safety` section now state the code, and Safety notes that encoding is checked rather than assumed. The `MessageSigningFailed` fall-through test still pins ErrorUnknown for what genuinely remains internal (no recovering recovery id), with a note that malformed bytes have moved off it. Co-Authored-By: Claude Fable 5 --- .../src/core_wallet/sign_message.rs | 22 ++++++++--- packages/rs-platform-wallet-ffi/src/error.rs | 37 +++++++++++++++++++ packages/rs-platform-wallet/src/error.rs | 15 ++++++++ 3 files changed, 69 insertions(+), 5 deletions(-) diff --git a/packages/rs-platform-wallet-ffi/src/core_wallet/sign_message.rs b/packages/rs-platform-wallet-ffi/src/core_wallet/sign_message.rs index 20eb0faacbd..ebfca5aae5a 100644 --- a/packages/rs-platform-wallet-ffi/src/core_wallet/sign_message.rs +++ b/packages/rs-platform-wallet-ffi/src/core_wallet/sign_message.rs @@ -31,7 +31,11 @@ use std::os::raw::c_char; /// the same shape for symmetry. /// /// `on_error` builds the typed error, so each argument reports itself rather -/// than borrowing the other's phrasing. +/// than borrowing the other's phrasing: a malformed address yields +/// `MessageSigningAddressInvalid` and malformed message bytes yield +/// `MessageSigningMessageInvalid`. Both are caller input and both surface as +/// [`PlatformWalletFFIResultCode::ErrorInvalidParameter`] (2) — neither is an +/// internal failure, so neither may reach `ErrorUnknown`. /// /// `len == 0` yields the empty string WITHOUT dereferencing `ptr`, which may /// legitimately be null: Swift's established marshalling @@ -73,14 +77,20 @@ unsafe fn read_utf8( /// significant and the verifier must receive the identical bytes. An empty /// message is valid and signable, and `message_ptr` MAY be null when /// `message_len` is 0 — the shape host marshalling naturally produces for an -/// empty string. +/// empty string. Bytes that are not valid UTF-8 fail with +/// [`PlatformWalletFFIResultCode::ErrorInvalidParameter`] (2), the same code +/// a malformed address yields: there is no string to sign, and it is the +/// caller's argument to fix. /// * `core_signer_handle` — the caller's `MnemonicResolverHandle`; ownership is /// retained by the caller (this function does NOT destroy it). /// * `out_signature` — receives a heap-allocated C string holding the base64 /// signature. Free with [`super::core_wallet_free_address`]. /// /// # Safety -/// `address_ptr` must be non-null and readable for `address_len` bytes; +/// Encoding is CHECKED, not assumed: either buffer failing to be valid UTF-8 is +/// a safe, typed `ErrorInvalidParameter` return rather than undefined behavior. +/// What the caller must guarantee is memory validity — `address_ptr` must be +/// non-null and readable for `address_len` bytes; /// `message_ptr` must be readable for `message_len` bytes when that length is /// non-zero (it may be null when the length is 0). Both must stay valid for the /// duration of the call; `out_signature` must point to writable memory for one @@ -133,9 +143,11 @@ pub unsafe extern "C" fn core_wallet_sign_message( } })); // A non-UTF-8 message is reported against the (now known) address, so the - // error names the signing target the caller asked about. + // error names the signing target the caller asked about — but as a MESSAGE + // error, not an address one: the address parsed fine, and this maps to + // ErrorInvalidParameter like any other malformed argument. let message = unwrap_result_or_return!(read_utf8(message_ptr, message_len, |e| { - PlatformWalletError::MessageSigningFailed { + PlatformWalletError::MessageSigningMessageInvalid { address: address.clone(), reason: format!("message is not valid UTF-8: {e}"), } diff --git a/packages/rs-platform-wallet-ffi/src/error.rs b/packages/rs-platform-wallet-ffi/src/error.rs index f87a487ad64..b5fa32c5733 100644 --- a/packages/rs-platform-wallet-ffi/src/error.rs +++ b/packages/rs-platform-wallet-ffi/src/error.rs @@ -448,6 +448,13 @@ impl From for PlatformWalletFFIResult { PlatformWalletError::MessageSigningAddressInvalid { .. } => { PlatformWalletFFIResultCode::ErrorInvalidParameter } + // Message bytes that are not valid UTF-8: the same kind of + // caller-input error as the address arm above, so it gets the same + // code. It previously fell through to ErrorUnknown, which told a + // host "internal failure" about a malformed argument it could fix. + PlatformWalletError::MessageSigningMessageInvalid { .. } => { + PlatformWalletFFIResultCode::ErrorInvalidParameter + } // A second, signer-free producer of code 31 (the arm above is the // first): a message-signing address that belongs to no signable // funds account means no key can exist for it — the same conclusion @@ -1096,6 +1103,31 @@ mod tests { ); } + /// Malformed MESSAGE bytes are caller input just like a malformed address, + /// so they map to the same ErrorInvalidParameter — not ErrorUnknown, which + /// would report an internal failure for an argument the caller can fix. + /// Only reachable across the FFI, where the message arrives as raw bytes. + #[test] + fn message_signing_message_invalid_maps_to_invalid_parameter() { + let err = PlatformWalletError::MessageSigningMessageInvalid { + address: "yRd4FhXfVGHXpsuZXPNkMrfD9GVj46pnjt".to_string(), + reason: "invalid utf-8 sequence of 1 bytes from index 2".to_string(), + }; + let result: PlatformWalletFFIResult = err.into(); + assert_eq!( + result.code, + PlatformWalletFFIResultCode::ErrorInvalidParameter + ); + // The rendering must blame the message, not the address. + let msg = unsafe { std::ffi::CStr::from_ptr(result.message) } + .to_string_lossy() + .into_owned(); + assert!( + msg.contains("message to sign") && !msg.contains("address is not valid"), + "the Display must name the message as the malformed argument: {msg}" + ); + } + /// A bad message-signing address is caller input, so it maps to the /// already-mirrored ErrorInvalidParameter rather than ErrorUnknown. #[test] @@ -1116,6 +1148,11 @@ mod tests { /// key-repair prompt, so it falls through to ErrorUnknown carrying the /// signer's own rendering. Pinned so a future arm cannot silently claim it. /// + /// Note this variant no longer carries malformed-message-bytes, which used + /// to land here and therefore on ErrorUnknown; they now have their own + /// `MessageSigningMessageInvalid` mapping to ErrorInvalidParameter. What + /// remains here is genuinely internal. + /// /// #4183's key-unavailable promotion does NOT reach this variant, by /// design: it matches `Sdk(Protocol(Generic(s)))` structurally with the /// marker at position 0, because that review rejected sniffing the marker as diff --git a/packages/rs-platform-wallet/src/error.rs b/packages/rs-platform-wallet/src/error.rs index ad54f5be56e..eb3a322a5df 100644 --- a/packages/rs-platform-wallet/src/error.rs +++ b/packages/rs-platform-wallet/src/error.rs @@ -120,6 +120,21 @@ pub enum PlatformWalletError { #[error("message-signing address {address:?} is unusable: {reason}")] MessageSigningAddressInvalid { address: String, reason: String }, + /// The bytes handed to `core_wallet_sign_message` as the message are not + /// valid UTF-8, so there is no string to sign. Caller input, exactly like + /// [`Self::MessageSigningAddressInvalid`] — and given its own variant for + /// the same reason the address case has one: these errors exist to name + /// *which argument* the caller must fix, and reusing the address variant + /// for a message problem would render "address … is unusable" over a + /// perfectly good address. + /// + /// Only reachable across the FFI, where the message arrives as raw bytes; a + /// Rust or Kotlin caller cannot construct an ill-formed `&str`/`String`. + /// `address` is the (already validated as UTF-8) signing target, carried + /// for log correlation like every sibling — the *message* is what failed. + #[error("the message to sign for address {address} is not valid UTF-8: {reason}")] + MessageSigningMessageInvalid { address: String, reason: String }, + /// [`CoreWallet::sign_message`] was given a well-formed P2PKH address for /// the right network that this wallet holds no signing key for: it belongs /// to no *signable* funds account (BIP44 / BIP32 / CoinJoin / From f597866a5ad2564aea585cc310c78662040c4dd5 Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Tue, 4 Aug 2026 05:47:31 -0700 Subject: [PATCH 6/7] docs(platform-wallet): prove why signer key-unavailable cannot reach code 31 from message signing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding on #4259 (CodeRabbit, Major). Investigated and found NOT fixable at the producer as described; recording the evidence in-code and pinning it with a test so it does not have to be re-derived. The finding asks `sign_message` to route signer key-unavailable failures through `preserve_signer_key_unavailable_or` before building `MessageSigningFailed`. That helper does exist (#4183 shipped it in this crate) and is the right tool — for the STATE-TRANSITION signing paths, whose failures are `dash_sdk::Error`, which is where document replace, DPNS and token transfer already use it. Message signing is a different surface. It calls key-wallet's `Signer::sign_ecdsa`, whose error is the associated type `S::Error`, bounded only by `Display + Send + Sync + 'static`. There is no enum to match, and passing it to the helper does not compile: error[E0308]: mismatched types expected enum `dash_sdk::Error` Nor does a producer exist. The only production impl, `rs_sdk_ffi::MnemonicResolverCoreSigner`, has `Error = MnemonicResolverSignerError` — a typed enum that never stamps the reserved marker; its `NotFound` ("mnemonic not found in keychain") IS the key-unavailable case but is indistinguishable once `Display`ed. The marker is produced only in rs-sdk-ffi's state-transition completion callback (`SignResult = Result, ProtocolError>`), never on a `sign_ecdsa` path. So a position-0 check would match nothing today, and a `contains` check is the substring sniff #4183's review rejected. Corrects an earlier note of mine that blamed the marker constant's visibility: it IS visible here now, mirrored as `SIGNER_KEY_UNAVAILABLE_PREFIX` and pinned byte-identical by a compile-time assertion. The blocker is the error type. Closing it needs an upstream change — the signer rendering its key-unavailable variants with the marker at position 0, or key-wallet tightening `Signer::Error` to something matchable. Both touch shared, externally-consumed surfaces and want their own review. `signer_key_unavailable_is_not_preserved_during_message_signing` pins the current behaviour and shows the mechanism: the marker survives but mid-string, which is precisely why a position-0 match cannot recover it. It is written to FAIL if an upstream fix ever makes code 31 reachable, with instructions to flip it. The existing fall-through test continues to pin genuinely-internal failures, and capability refusals still take the `MessageSigningFailed` path unchanged. Co-Authored-By: Claude Fable 5 --- packages/rs-platform-wallet-ffi/src/error.rs | 50 ++++++++-- .../src/wallet/core/sign_message.rs | 94 +++++++++++++++++++ 2 files changed, 137 insertions(+), 7 deletions(-) diff --git a/packages/rs-platform-wallet-ffi/src/error.rs b/packages/rs-platform-wallet-ffi/src/error.rs index b5fa32c5733..d099ae611eb 100644 --- a/packages/rs-platform-wallet-ffi/src/error.rs +++ b/packages/rs-platform-wallet-ffi/src/error.rs @@ -482,13 +482,49 @@ impl From for PlatformWalletFFIResult { // could only ever appear mid-string. Matching it here would mean // exactly the substring sniff that review ruled out. // - // Consequence worth knowing: a Keystore/Keychain key-unavailable - // completion reaching `sign_message` surfaces as ErrorUnknown, not - // 31. Closing that needs a structural change at the producer — have - // `sign_message` classify the signer's own rendering before it wraps - // it — not a looser match here. `platform-wallet` cannot see the - // marker constant (it does not depend on `rs-sdk-ffi`), so that is - // its own piece of work. + // Consequence worth knowing: a signer-reported key-unavailable + // condition reaching `sign_message` surfaces as ErrorUnknown, not + // 31. That is NOT closable at this layer, and — having chased it — + // not closable at the producer either without an upstream change. + // The type chain is the whole story: + // + // * `preserve_signer_key_unavailable_or` (platform-wallet's own + // helper, #4183) takes a `dash_sdk::Error` and matches + // `Protocol(Generic(s))` with the marker at position 0. It is + // the right tool — for the STATE-TRANSITION signing paths + // (document replace, DPNS, token transfer), whose failures ARE + // `dash_sdk::Error`, which is where it is used. + // * Message signing does not use that surface at all. It calls + // key-wallet's `Signer::sign_ecdsa`, whose error is the + // associated type `S::Error`, bounded only by + // `Display + Send + Sync + 'static`. There is no enum to match: + // no `dash_sdk::Error`, no `ProtocolError`, nothing structural. + // * The one production impl, `MnemonicResolverCoreSigner` + // (rs-sdk-ffi), has `Error = MnemonicResolverSignerError` — a + // typed enum that never stamps the marker. Its `NotFound` + // ("mnemonic not found in keychain") IS the key-unavailable + // case, but nothing distinguishes it once it is `Display`ed. + // * The marker is produced only in `rs-sdk-ffi`'s state-transition + // completion callback (`SignResult = Result, + // ProtocolError>`), never on a `Signer::sign_ecdsa` path. + // + // So a position-0 check on the signer's rendering would have zero + // producers today, and a `contains` check is the substring sniff + // #4183's review rejected. Note the marker constant IS visible here + // now (#4183 mirrored it as + // `platform_wallet::error::SIGNER_KEY_UNAVAILABLE_PREFIX`, pinned + // byte-identical by a compile-time assertion in this crate) — the + // blocker is the error TYPE, not the constant, which corrects an + // earlier note in this file's history. + // + // The fix belongs upstream, in ONE of: + // (a) `MnemonicResolverCoreSigner` rendering its key-unavailable + // variants with the marker at position 0, after which a + // position-0 check in `sign_message` becomes principled; or + // (b) key-wallet tightening `Signer::Error` so callers can match + // a typed key-unavailable variant instead of a string. + // Both change shared, externally-consumed surfaces and want their + // own review; neither is in scope for message signing. _ => PlatformWalletFFIResultCode::ErrorUnknown, }; PlatformWalletFFIResult::err(code, error.to_string()) diff --git a/packages/rs-platform-wallet/src/wallet/core/sign_message.rs b/packages/rs-platform-wallet/src/wallet/core/sign_message.rs index 0c9dc3aec27..1c781df1063 100644 --- a/packages/rs-platform-wallet/src/wallet/core/sign_message.rs +++ b/packages/rs-platform-wallet/src/wallet/core/sign_message.rs @@ -216,6 +216,28 @@ impl CoreWallet { } let hash = signed_msg_hash(message); + // Every signer failure becomes `MessageSigningFailed` (→ `ErrorUnknown`), + // including a key-unavailable one. That is a known limitation, not an + // oversight, and it cannot be fixed here. + // + // `preserve_signer_key_unavailable_or` — this crate's own helper for + // exactly this problem — takes a `dash_sdk::Error` and matches + // `Protocol(Generic(s))` with the reserved marker at position 0. It + // serves the STATE-TRANSITION signing paths, whose failures are + // `dash_sdk::Error`. `sign_ecdsa` is a different surface: its error is + // `S::Error`, bounded only by `Display + Send + Sync + 'static`, so + // there is no enum here to match — passing it to that helper will not + // type-check. The one production impl + // (`rs_sdk_ffi::MnemonicResolverCoreSigner`, `Error = + // MnemonicResolverSignerError`) never stamps the marker either, so even + // a position-0 check on `e.to_string()` would have no producer, while a + // `contains` check is the substring sniff #4183's review rejected. + // + // Closing it needs an upstream change — the signer rendering its + // key-unavailable variants with the marker at position 0, or key-wallet + // tightening `Signer::Error` to something matchable. See the NOTE on + // the `MessageSigningFailed` arm in `platform-wallet-ffi`'s error + // conversion for the full chain. let (signature, public_key) = signer .sign_ecdsa(&path, hash.to_byte_array()) .await @@ -525,6 +547,78 @@ mod tests { } } + /// A digest-capable backend that fails every signature the way a Keystore + /// or Keychain reports a missing key: its `Display` begins with the reserved + /// key-unavailable marker, the representation the state-transition signing + /// paths rely on to recover FFI code 31. + struct KeyUnavailableSigner; + + #[async_trait::async_trait] + impl Signer for KeyUnavailableSigner { + 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> { + Err(format!( + "{}no private key stored for this derivation path", + crate::error::SIGNER_KEY_UNAVAILABLE_PREFIX + )) + } + + async fn public_key(&self, _path: &DerivationPath) -> Result { + panic!("public_key is not part of the signed-message path"); + } + } + + /// **Pins a known limitation, so nobody has to re-derive it.** A signer that + /// reports key-unavailable during message signing does NOT reach FFI code + /// 31; it lands on `MessageSigningFailed`, which the FFI flattens to + /// `ErrorUnknown`. + /// + /// The mechanism is visible in the assertion: the marker survives, but + /// MID-STRING, because `reason` is composed as + /// "signer rejected the digest at {path}: {e}". A position-0 match — the + /// only kind #4183's review permits — therefore cannot see it, and + /// `preserve_signer_key_unavailable_or` cannot be applied because it takes a + /// `dash_sdk::Error` while `sign_ecdsa` yields `S::Error: Display`. + /// + /// If an upstream change ever makes this reachable as code 31 (the signer + /// stamping the marker at position 0 of its own error, or key-wallet + /// tightening `Signer::Error`), THIS TEST SHOULD FAIL — flip it to assert + /// `MessageSigningKeyUnavailable` and delete the surrounding notes. + #[tokio::test] + async fn signer_key_unavailable_is_not_preserved_during_message_signing() { + let (wm, wallet_id, _, address) = + mnemonic_wallet_manager(MESSAGE_SIGNING_TEST_MNEMONIC).await; + let core = core_wallet(wm, wallet_id); + + let result = core + .sign_message(&address.to_string(), MESSAGE, &KeyUnavailableSigner) + .await; + + match result { + Err(PlatformWalletError::MessageSigningFailed { reason, .. }) => { + assert!( + reason.contains(crate::error::SIGNER_KEY_UNAVAILABLE_PREFIX), + "the signer's marker should still be present: {reason:?}" + ); + assert!( + !reason.starts_with(crate::error::SIGNER_KEY_UNAVAILABLE_PREFIX), + "the marker is mid-string once wrapped, which is exactly why a \ + position-0 match cannot recover it: {reason:?}" + ); + } + other => panic!("expected MessageSigningFailed, got {other:?}"), + } + } + /// **Capability dispatch.** `Signer` assigns method dispatch to the caller, /// and documents `sign_ecdsa` as valid only when the backend advertises /// `SignerMethod::Digest`. A transaction-only backend must therefore be From 09a27ac60db94baf942d2123817f85c8eb8d2371 Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Tue, 4 Aug 2026 13:17:51 -0700 Subject: [PATCH 7/7] docs(platform-wallet): MessageSigningFailed doc no longer claims a code-31 promotion the FFI does not perform MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The variant doc still carried the qa3-era claim that a signer's typed key-unavailable completion gets promoted to ErrorSigningKeyUnavailable by the FFI catch-all — the opposite of the documented behavior everywhere else after the #4183 convergence. Reworded to state the real contract: signer failures fall through to ErrorUnknown, only address-resolution key-unavailable reaches code 31, with pointers to the FFI conversion NOTE that carries the full type chain. Co-Authored-By: Claude Fable 5 --- packages/rs-platform-wallet/src/error.rs | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/packages/rs-platform-wallet/src/error.rs b/packages/rs-platform-wallet/src/error.rs index eb3a322a5df..e427001a184 100644 --- a/packages/rs-platform-wallet/src/error.rs +++ b/packages/rs-platform-wallet/src/error.rs @@ -170,12 +170,18 @@ pub enum PlatformWalletError { /// resolver advertises `Digest` — so it does not warrant a new FFI code and /// the host mirror-enum churn that follows one. /// - /// Deliberately NOT given a dedicated FFI code: a signer that reports its - /// typed key-unavailable completion carries the stable machine prefix in - /// its `Display`, which the FFI conversion's catch-all promotes to - /// `ErrorSigningKeyUnavailable`. The remaining causes are genuine internal - /// invariant breaks and should surface as unknown rather than as a - /// key-repair prompt. + /// Deliberately NOT given a dedicated FFI code: [`Signer::Error`] is + /// generic and bounded only by `Display`, so it cannot be classified + /// structurally here. Signer failures — including a signer-reported + /// key-unavailable completion — remain `MessageSigningFailed` and fall + /// through to `ErrorUnknown`. Only [`MessageSigningKeyUnavailable`] + /// (address resolution failing before a signer is ever invoked) reaches + /// FFI code 31. See the `MessageSigningFailed` arm's NOTE in + /// `platform-wallet-ffi`'s error conversion for the full type chain and + /// the upstream change that would be needed to close this gap. + /// + /// [`MessageSigningKeyUnavailable`]: Self::MessageSigningKeyUnavailable + /// [`Signer::Error`]: key_wallet::signer::Signer::Error /// /// [`CoreWallet::sign_message`]: crate::wallet::core::CoreWallet::sign_message /// [`Signer`]: key_wallet::signer::Signer