diff --git a/.cargo/audit.toml b/.cargo/audit.toml index 1ddc9033595..c922c98cbf8 100644 --- a/.cargo/audit.toml +++ b/.cargo/audit.toml @@ -1,3 +1,5 @@ [advisories] -# TODO Remove it from here -ignore = [ "RUSTSEC-2020-0071"] # advisory IDs to ignore e.g. ["RUSTSEC-2019-0001", ...] +# Advisory IDs to ignore, e.g. ["RUSTSEC-2019-0001", ...]. Each entry +# must point at a live advisory in the resolved graph and carry a dated +# rationale; an entry matching nothing trains reviewers to skim the list. +ignore = [] diff --git a/.gitignore b/.gitignore index 983cad56c38..b243acea36e 100644 --- a/.gitignore +++ b/.gitignore @@ -103,3 +103,6 @@ __pycache__/ # Security audit reports (local-only, not committed) audits/ + +# Review scratch (grumpy-review / triage output, local-only) +.review-*/ diff --git a/Cargo.lock b/Cargo.lock index 4a46c8e326b..2752f46c379 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1206,7 +1206,7 @@ version = "3.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "faf9468729b8cbcea668e36183cb69d317348c2e08e994829fb56ebfdfbaac34" dependencies = [ - "windows-sys 0.59.0", + "windows-sys 0.52.0", ] [[package]] @@ -1637,7 +1637,7 @@ dependencies = [ [[package]] name = "dash-network" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=647fa9820f3614090e4e5f5f2b709961d68e538b#647fa9820f3614090e4e5f5f2b709961d68e538b" +source = "git+https://github.com/dashpay/rust-dashcore?rev=48a07d3fcd2f8d37e109f969eb5da0a8bbbb9aff#48a07d3fcd2f8d37e109f969eb5da0a8bbbb9aff" dependencies = [ "bincode", "bincode_derive", @@ -1648,7 +1648,7 @@ dependencies = [ [[package]] name = "dash-network-seeds" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=647fa9820f3614090e4e5f5f2b709961d68e538b#647fa9820f3614090e4e5f5f2b709961d68e538b" +source = "git+https://github.com/dashpay/rust-dashcore?rev=48a07d3fcd2f8d37e109f969eb5da0a8bbbb9aff#48a07d3fcd2f8d37e109f969eb5da0a8bbbb9aff" dependencies = [ "dash-network", ] @@ -1725,7 +1725,7 @@ dependencies = [ [[package]] name = "dash-spv" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=647fa9820f3614090e4e5f5f2b709961d68e538b#647fa9820f3614090e4e5f5f2b709961d68e538b" +source = "git+https://github.com/dashpay/rust-dashcore?rev=48a07d3fcd2f8d37e109f969eb5da0a8bbbb9aff#48a07d3fcd2f8d37e109f969eb5da0a8bbbb9aff" dependencies = [ "async-trait", "chrono", @@ -1754,7 +1754,7 @@ dependencies = [ [[package]] name = "dashcore" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=647fa9820f3614090e4e5f5f2b709961d68e538b#647fa9820f3614090e4e5f5f2b709961d68e538b" +source = "git+https://github.com/dashpay/rust-dashcore?rev=48a07d3fcd2f8d37e109f969eb5da0a8bbbb9aff#48a07d3fcd2f8d37e109f969eb5da0a8bbbb9aff" dependencies = [ "anyhow", "base64-compat", @@ -1780,12 +1780,12 @@ dependencies = [ [[package]] name = "dashcore-private" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=647fa9820f3614090e4e5f5f2b709961d68e538b#647fa9820f3614090e4e5f5f2b709961d68e538b" +source = "git+https://github.com/dashpay/rust-dashcore?rev=48a07d3fcd2f8d37e109f969eb5da0a8bbbb9aff#48a07d3fcd2f8d37e109f969eb5da0a8bbbb9aff" [[package]] name = "dashcore-rpc" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=647fa9820f3614090e4e5f5f2b709961d68e538b#647fa9820f3614090e4e5f5f2b709961d68e538b" +source = "git+https://github.com/dashpay/rust-dashcore?rev=48a07d3fcd2f8d37e109f969eb5da0a8bbbb9aff#48a07d3fcd2f8d37e109f969eb5da0a8bbbb9aff" dependencies = [ "dashcore-rpc-json", "hex", @@ -1798,7 +1798,7 @@ dependencies = [ [[package]] name = "dashcore-rpc-json" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=647fa9820f3614090e4e5f5f2b709961d68e538b#647fa9820f3614090e4e5f5f2b709961d68e538b" +source = "git+https://github.com/dashpay/rust-dashcore?rev=48a07d3fcd2f8d37e109f969eb5da0a8bbbb9aff#48a07d3fcd2f8d37e109f969eb5da0a8bbbb9aff" dependencies = [ "bincode", "dashcore", @@ -1813,7 +1813,7 @@ dependencies = [ [[package]] name = "dashcore_hashes" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=647fa9820f3614090e4e5f5f2b709961d68e538b#647fa9820f3614090e4e5f5f2b709961d68e538b" +source = "git+https://github.com/dashpay/rust-dashcore?rev=48a07d3fcd2f8d37e109f969eb5da0a8bbbb9aff#48a07d3fcd2f8d37e109f969eb5da0a8bbbb9aff" dependencies = [ "bincode", "dashcore-private", @@ -2428,7 +2428,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys 0.59.0", + "windows-sys 0.52.0", ] [[package]] @@ -2489,7 +2489,7 @@ checksum = "0ce92ff622d6dadf7349484f42c93271a0d49b7cc4d466a936405bacbe10aa78" dependencies = [ "cfg-if", "rustix 1.1.4", - "windows-sys 0.59.0", + "windows-sys 0.52.0", ] [[package]] @@ -2867,7 +2867,7 @@ dependencies = [ [[package]] name = "git-state" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=647fa9820f3614090e4e5f5f2b709961d68e538b#647fa9820f3614090e4e5f5f2b709961d68e538b" +source = "git+https://github.com/dashpay/rust-dashcore?rev=48a07d3fcd2f8d37e109f969eb5da0a8bbbb9aff#48a07d3fcd2f8d37e109f969eb5da0a8bbbb9aff" [[package]] name = "glob" @@ -3803,7 +3803,7 @@ checksum = "3640c1c38b8e4e43584d8df18be5fc6b0aa314ce6ebf51b53313d4306cca8e46" dependencies = [ "hermit-abi", "libc", - "windows-sys 0.59.0", + "windows-sys 0.52.0", ] [[package]] @@ -4034,7 +4034,7 @@ dependencies = [ [[package]] name = "key-wallet" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=647fa9820f3614090e4e5f5f2b709961d68e538b#647fa9820f3614090e4e5f5f2b709961d68e538b" +source = "git+https://github.com/dashpay/rust-dashcore?rev=48a07d3fcd2f8d37e109f969eb5da0a8bbbb9aff#48a07d3fcd2f8d37e109f969eb5da0a8bbbb9aff" dependencies = [ "aes", "async-trait", @@ -4063,7 +4063,7 @@ dependencies = [ [[package]] name = "key-wallet-ffi" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=647fa9820f3614090e4e5f5f2b709961d68e538b#647fa9820f3614090e4e5f5f2b709961d68e538b" +source = "git+https://github.com/dashpay/rust-dashcore?rev=48a07d3fcd2f8d37e109f969eb5da0a8bbbb9aff#48a07d3fcd2f8d37e109f969eb5da0a8bbbb9aff" dependencies = [ "cbindgen 0.29.4", "dash-network", @@ -4079,7 +4079,7 @@ dependencies = [ [[package]] name = "key-wallet-manager" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=647fa9820f3614090e4e5f5f2b709961d68e538b#647fa9820f3614090e4e5f5f2b709961d68e538b" +source = "git+https://github.com/dashpay/rust-dashcore?rev=48a07d3fcd2f8d37e109f969eb5da0a8bbbb9aff#48a07d3fcd2f8d37e109f969eb5da0a8bbbb9aff" dependencies = [ "async-trait", "bincode", @@ -4596,7 +4596,7 @@ version = "0.50.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7957b9740744892f114936ab4a57b3f487491bbeafaf8083688b16841a4240e5" dependencies = [ - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -5698,7 +5698,7 @@ dependencies = [ "once_cell", "socket2 0.5.10", "tracing", - "windows-sys 0.59.0", + "windows-sys 0.52.0", ] [[package]] @@ -6489,7 +6489,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys 0.4.15", - "windows-sys 0.59.0", + "windows-sys 0.52.0", ] [[package]] @@ -6502,7 +6502,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys 0.12.1", - "windows-sys 0.59.0", + "windows-sys 0.52.0", ] [[package]] @@ -6561,7 +6561,7 @@ dependencies = [ "security-framework", "security-framework-sys", "webpki-root-certs", - "windows-sys 0.59.0", + "windows-sys 0.52.0", ] [[package]] @@ -7421,7 +7421,7 @@ dependencies = [ "getrandom 0.4.2", "once_cell", "rustix 1.1.4", - "windows-sys 0.59.0", + "windows-sys 0.52.0", ] [[package]] @@ -8870,7 +8870,7 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" dependencies = [ - "windows-sys 0.59.0", + "windows-sys 0.52.0", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 3fbbd5c2552..7fcb00ef8f7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -50,14 +50,17 @@ members = [ ] [workspace.dependencies] -dashcore = { git = "https://github.com/dashpay/rust-dashcore", rev = "647fa9820f3614090e4e5f5f2b709961d68e538b" } -dash-network-seeds = { git = "https://github.com/dashpay/rust-dashcore", rev = "647fa9820f3614090e4e5f5f2b709961d68e538b" } -dash-spv = { git = "https://github.com/dashpay/rust-dashcore", rev = "647fa9820f3614090e4e5f5f2b709961d68e538b" } -key-wallet = { git = "https://github.com/dashpay/rust-dashcore", rev = "647fa9820f3614090e4e5f5f2b709961d68e538b" } -key-wallet-ffi = { git = "https://github.com/dashpay/rust-dashcore", rev = "647fa9820f3614090e4e5f5f2b709961d68e538b" } -key-wallet-manager = { git = "https://github.com/dashpay/rust-dashcore", rev = "647fa9820f3614090e4e5f5f2b709961d68e538b" } -dash-network = { git = "https://github.com/dashpay/rust-dashcore", rev = "647fa9820f3614090e4e5f5f2b709961d68e538b" } -dashcore-rpc = { git = "https://github.com/dashpay/rust-dashcore", rev = "647fa9820f3614090e4e5f5f2b709961d68e538b" } +# TODO(pin): tracks dashpay/rust-dashcore#851 (draft) — key-wallet +# out-of-order UTXO spend fix (#649). Bump to the merged `dev` SHA once +# that PR lands. +dashcore = { git = "https://github.com/dashpay/rust-dashcore", rev = "48a07d3fcd2f8d37e109f969eb5da0a8bbbb9aff" } +dash-network-seeds = { git = "https://github.com/dashpay/rust-dashcore", rev = "48a07d3fcd2f8d37e109f969eb5da0a8bbbb9aff" } +dash-spv = { git = "https://github.com/dashpay/rust-dashcore", rev = "48a07d3fcd2f8d37e109f969eb5da0a8bbbb9aff" } +key-wallet = { git = "https://github.com/dashpay/rust-dashcore", rev = "48a07d3fcd2f8d37e109f969eb5da0a8bbbb9aff" } +key-wallet-ffi = { git = "https://github.com/dashpay/rust-dashcore", rev = "48a07d3fcd2f8d37e109f969eb5da0a8bbbb9aff" } +key-wallet-manager = { git = "https://github.com/dashpay/rust-dashcore", rev = "48a07d3fcd2f8d37e109f969eb5da0a8bbbb9aff" } +dash-network = { git = "https://github.com/dashpay/rust-dashcore", rev = "48a07d3fcd2f8d37e109f969eb5da0a8bbbb9aff" } +dashcore-rpc = { git = "https://github.com/dashpay/rust-dashcore", rev = "48a07d3fcd2f8d37e109f969eb5da0a8bbbb9aff" } tokio-metrics = "0.5" diff --git a/packages/rs-platform-wallet-ffi/src/core_wallet_types.rs b/packages/rs-platform-wallet-ffi/src/core_wallet_types.rs index c8ebc1348fe..1d3dc910e02 100644 --- a/packages/rs-platform-wallet-ffi/src/core_wallet_types.rs +++ b/packages/rs-platform-wallet-ffi/src/core_wallet_types.rs @@ -964,7 +964,11 @@ fn tx_record_to_ffi( } } -fn vec_to_ptr(v: Vec) -> *mut T { +/// Convert a `Vec` into a raw heap pointer for a C out-array: null for +/// empty, `Box::into_raw(boxed_slice)` otherwise. The caller owns the +/// allocation and must free it by reconstructing the boxed slice with +/// the ORIGINAL length. +pub(crate) fn vec_to_ptr(v: Vec) -> *mut T { if v.is_empty() { std::ptr::null_mut() } else { diff --git a/packages/rs-platform-wallet-ffi/src/error.rs b/packages/rs-platform-wallet-ffi/src/error.rs index cec0966b9c2..cd5b24fcc62 100644 --- a/packages/rs-platform-wallet-ffi/src/error.rs +++ b/packages/rs-platform-wallet-ffi/src/error.rs @@ -145,6 +145,40 @@ pub enum PlatformWalletFFIResultCode { /// observing the transaction reconciles the outcome. The host must NOT /// auto-retry. Shielded sibling: [`Self::ErrorShieldedSpendUnconfirmed`]. ErrorTransactionBroadcastUnconfirmed = 20, + /// Maps `PlatformWalletError::AddressNonceMismatch`. Platform rejected an + /// address-funds transition (shield, or identity top-up-from-addresses) + /// because the submitted address nonce raced Platform's expected next + /// value (a lagging DAPI replica stale read; consensus code 40603). Same + /// definitively-failed / notes-released / safe-to-retry contract as + /// [`Self::ErrorShieldedBroadcastFailed`] — the transition did NOT execute + /// and any note reservations were released (a shield reserves none) — but + /// as its OWN code so hosts can recognize this specific, self-healing + /// failure and retry: the retry re-fetches the address nonce, resolving + /// the mismatch without host intervention. The submitted and Platform- + /// expected nonce values travel in the result `message` (the typed + /// `Display`); they are not exposed as structured out-fields (that would + /// require an ABI-breaking change to `PlatformWalletFFIResult`). + ErrorAddressNonceMismatch = 21, + /// Maps `PlatformWalletError::PersisterLoad` when the wrapped + /// [`PersistenceError`] classifies as transient + /// (`is_transient() == true`, e.g. `SQLITE_BUSY`). Rehydration could not + /// load the persisted client state, but the backend reports the failure + /// as recoverable, so the host MAY retry the load with backoff. Split + /// from [`Self::ErrorPersisterLoadFatal`] so the retry classification + /// proven on the Rust side survives the boundary instead of flattening + /// to `ErrorUnknown`. + /// + /// [`PersistenceError`]: platform_wallet::changeset::PersistenceError + ErrorPersisterLoadTransient = 22, + /// Maps `PlatformWalletError::PersisterLoad` when the wrapped + /// [`PersistenceError`] is non-transient (a `Fatal` or `Constraint` + /// backend failure, or a poisoned lock). Rehydration could not load the + /// persisted client state and the failure is unrecoverable, so the host + /// MUST NOT retry the same load. Sibling of + /// [`Self::ErrorPersisterLoadTransient`]. + /// + /// [`PersistenceError`]: platform_wallet::changeset::PersistenceError + ErrorPersisterLoadFatal = 23, NotFound = 98, // Used exclusively for all the Option that are retuned as errors ErrorUnknown = 99, @@ -285,6 +319,27 @@ impl From for PlatformWalletFFIResult { PlatformWalletError::TransactionBroadcastUnconfirmed(..) => { PlatformWalletFFIResultCode::ErrorTransactionBroadcastUnconfirmed } + // A definitively-failed address-nonce race (reaches the blanket impl + // via identity `top_up_from_addresses` → `?`/`.into()`). Exposing + // provided/expected nonce as structured out-fields is INTENTIONALLY + // out of scope: `PlatformWalletFFIResult` is by-value / ABI-frozen, so + // the values travel in the message string and an FFI retry re-fetches + // the nonce. + PlatformWalletError::AddressNonceMismatch { .. } => { + PlatformWalletFFIResultCode::ErrorAddressNonceMismatch + } + // A rehydration persister-load failure. The Rust side already proves + // the transient-vs-fatal classification (rehydration_load.rs), so + // split the code on `is_transient()` — flattening to `ErrorUnknown` + // would erase the retry signal. Fatal/Constraint/LockPoisoned all + // read as non-transient, i.e. do-not-retry. + PlatformWalletError::PersisterLoad(inner) => { + if inner.is_transient() { + PlatformWalletFFIResultCode::ErrorPersisterLoadTransient + } else { + PlatformWalletFFIResultCode::ErrorPersisterLoadFatal + } + } _ => PlatformWalletFFIResultCode::ErrorUnknown, }; PlatformWalletFFIResult::err(code, error.to_string()) @@ -664,6 +719,88 @@ mod tests { assert_eq!(msg, rendered, "Display payload must survive verbatim"); } + /// `AddressNonceMismatch` maps to the dedicated `ErrorAddressNonceMismatch` + /// FFI code through the blanket `From` impl (the path identity + /// `top_up_from_addresses` takes via `?`/`.into()`) rather than flattening + /// to `ErrorUnknown`. The typed Display rendering — carrying the submitted + /// and expected nonce values — survives across the boundary as the message. + #[test] + fn address_nonce_mismatch_maps_to_dedicated_code() { + let err = PlatformWalletError::AddressNonceMismatch { + address: dpp::address_funds::PlatformAddress::P2pkh([7u8; 20]), + provided_nonce: 1, + expected_nonce: 2, + }; + let rendered = err.to_string(); + let result: PlatformWalletFFIResult = err.into(); + assert_eq!( + result.code, + PlatformWalletFFIResultCode::ErrorAddressNonceMismatch, + "AddressNonceMismatch should map to ErrorAddressNonceMismatch (rendered: {rendered})" + ); + let msg = unsafe { std::ffi::CStr::from_ptr(result.message) } + .to_string_lossy() + .into_owned(); + assert_eq!( + msg, rendered, + "Display payload must survive the FFI boundary verbatim" + ); + // Pin the EXACT rendered substrings, not bare digits, so a + // provided/expected transposition would fail the test. + assert!( + msg.contains("submitted nonce 1"), + "submitted (provided) nonce must render exactly: {msg}" + ); + assert!( + msg.contains("Platform expected 2"), + "expected nonce must render exactly: {msg}" + ); + } + + /// A rehydration `PersisterLoad` failure keeps its transient-vs-fatal + /// classification across the boundary: a transient backend failure maps to + /// `ErrorPersisterLoadTransient` (retryable) and every non-transient case + /// (`Fatal`, `Constraint`, `LockPoisoned`) maps to `ErrorPersisterLoadFatal` + /// (do-not-retry), rather than both flattening to `ErrorUnknown`. The typed + /// Display rendering survives verbatim as the message. + #[test] + fn persister_load_maps_by_retry_classification() { + use platform_wallet::changeset::{PersistenceError, PersistenceErrorKind}; + + let transient: Vec = vec![PlatformWalletError::PersisterLoad( + PersistenceError::backend_with_kind(PersistenceErrorKind::Transient, "database busy"), + )]; + let fatal: Vec = vec![ + PlatformWalletError::PersisterLoad(PersistenceError::backend("schema corrupt")), + PlatformWalletError::PersisterLoad(PersistenceError::backend_with_kind( + PersistenceErrorKind::Constraint, + "foreign key violation", + )), + PlatformWalletError::PersisterLoad(PersistenceError::LockPoisoned), + ]; + + for (cases, expected) in [ + ( + transient, + PlatformWalletFFIResultCode::ErrorPersisterLoadTransient, + ), + (fatal, PlatformWalletFFIResultCode::ErrorPersisterLoadFatal), + ] { + for err in cases { + let rendered = err.to_string(); + let result: PlatformWalletFFIResult = err.into(); + assert_eq!( + result.code, expected, + "PersisterLoad must map by retry classification (rendered: {rendered})" + ); + let msg = unsafe { std::ffi::CStr::from_ptr(result.message) } + .to_string_lossy() + .into_owned(); + assert_eq!(msg, rendered, "Display payload must survive verbatim"); + } + } + } + /// Other wallet-error variants without a dedicated FFI arm still /// fall through to `ErrorUnknown` while carrying the typed /// Display rendering as the message. Pin this so the catch-all diff --git a/packages/rs-platform-wallet-ffi/src/manager.rs b/packages/rs-platform-wallet-ffi/src/manager.rs index 7e553a64bc0..64cb08052c9 100644 --- a/packages/rs-platform-wallet-ffi/src/manager.rs +++ b/packages/rs-platform-wallet-ffi/src/manager.rs @@ -176,6 +176,93 @@ unsafe fn create_wallet_from_mnemonic_impl( PlatformWalletFFIResult::ok() } +/// `reason_code`: the persisted row had no usable account manifest to +/// rebuild the account collection from. +pub const LOAD_SKIP_REASON_MISSING_MANIFEST: u32 = 100; +/// `reason_code`: a manifest `account_xpub` failed to parse as a +/// well-formed extended public key. +pub const LOAD_SKIP_REASON_MALFORMED_XPUB: u32 = 101; +/// `reason_code`: any other structural decode / projection failure on +/// the persisted row. +pub const LOAD_SKIP_REASON_DECODE_ERROR: u32 = 102; +/// `reason_code`: the carried managed-info snapshot does not describe its +/// persisted row (wallet_id/network differ, or its account set diverges +/// from the row's account manifest) — a wrong-row snapshot. +pub const LOAD_SKIP_REASON_SNAPSHOT_IDENTITY_MISMATCH: u32 = 103; +/// `reason_code`: an unrecognized `CorruptKind` — forward-compat +/// fallback until this crate maps a newly added corrupt-row family. +pub const LOAD_SKIP_REASON_CORRUPT_OTHER: u32 = 199; +/// `reason_code`: an unrecognized `SkipReason` — forward-compat +/// fallback until this crate maps a newly added skip reason. +pub const LOAD_SKIP_REASON_OTHER: u32 = 200; +/// `reason_code`: the wallet was already registered before this load +/// pass reached it (a prior load, or a runtime-created wallet), so its +/// persisted row was not freshly loaded. Not corruption. +pub const LOAD_SKIP_REASON_ALREADY_REGISTERED: u32 = 300; + +/// One wallet skipped during `load_from_persistor` because its +/// persisted row was structurally corrupt (per-row decode failure). +/// The load path is seedless and watch-only, so this is the only skip +/// reason. `reason_code` is per-`CorruptKind` family — see its table. +#[repr(C)] +#[derive(Debug, Clone, Copy)] +pub struct SkippedWalletFFI { + /// The (public) 32-byte wallet id that was skipped. + pub wallet_id: [u8; 32], + /// Skip reason — one of the `LOAD_SKIP_REASON_*` constants: + /// [`LOAD_SKIP_REASON_MISSING_MANIFEST`] (100), + /// [`LOAD_SKIP_REASON_MALFORMED_XPUB`] (101), + /// [`LOAD_SKIP_REASON_DECODE_ERROR`] (102), + /// [`LOAD_SKIP_REASON_SNAPSHOT_IDENTITY_MISMATCH`] (103), + /// [`LOAD_SKIP_REASON_CORRUPT_OTHER`] (199), + /// [`LOAD_SKIP_REASON_OTHER`] (200), or + /// [`LOAD_SKIP_REASON_ALREADY_REGISTERED`] (300). No secret material + /// is ever carried. + pub reason_code: u32, +} + +/// C-visible summary of one `load_from_persistor` pass so the host can +/// see which wallets loaded and which were skipped (and why) instead +/// of the outcome being silently discarded. +/// +/// The count pair encodes the Rust `LoadOutcome` 3-state: `skipped_count +/// == 0` is a full load, `loaded_count == 0` with skips is +/// nothing-usable, and both non-zero is a partial load. +/// +/// `skipped` is a heap array of length `skipped_count`; pass this +/// struct (by pointer) to +/// [`platform_wallet_load_outcome_free`] exactly once to release it. +#[repr(C)] +#[derive(Debug)] +pub struct LoadOutcomeFFI { + /// Number of wallets fully reconstructed + registered. + pub loaded_count: usize, + /// Length of the `skipped` array. + pub skipped_count: usize, + /// Heap-allocated skipped-wallet array (null iff `skipped_count` + /// is 0). Owned by Rust until `platform_wallet_load_outcome_free`. + pub skipped: *mut SkippedWalletFFI, +} + +fn skip_reason_code(reason: &platform_wallet::SkipReason) -> u32 { + use platform_wallet::manager::load_outcome::CorruptKind; + match reason { + platform_wallet::SkipReason::CorruptPersistedRow { kind } => match kind { + CorruptKind::MissingManifest => LOAD_SKIP_REASON_MISSING_MANIFEST, + CorruptKind::MalformedXpub => LOAD_SKIP_REASON_MALFORMED_XPUB, + CorruptKind::SnapshotIdentityMismatch => LOAD_SKIP_REASON_SNAPSHOT_IDENTITY_MISMATCH, + CorruptKind::DecodeError(_) => LOAD_SKIP_REASON_DECODE_ERROR, + // `CorruptKind` is #[non_exhaustive]; a future variant maps to a + // generic corrupt-row code until this mapping is extended. + _ => LOAD_SKIP_REASON_CORRUPT_OTHER, + }, + platform_wallet::SkipReason::AlreadyRegistered => LOAD_SKIP_REASON_ALREADY_REGISTERED, + // `SkipReason` is #[non_exhaustive]; a future reason maps to a + // generic skip code until this mapping is extended. + _ => LOAD_SKIP_REASON_OTHER, + } +} + /// Create a wallet from raw seed bytes (64 bytes). /// /// On success, `out_wallet_handle` is set to a `PlatformWallet` handle and @@ -298,23 +385,115 @@ pub unsafe extern "C" fn platform_wallet_manager_create_wallet_from_mnemonic_wit /// /// Triggers `on_load_wallet_list_fn` on the persistence callbacks to /// fetch the persisted wallet list from the client side (SwiftData), -/// reconstructs each wallet as **watch-only** via its stored root + -/// per-account xpubs, and registers them inside the manager. Does not -/// produce wallet handles — the caller should follow up with -/// [`platform_wallet_manager_get_wallet`] per `wallet_id` it knows -/// about. +/// builds a keyless reconstruction payload per wallet, then registers +/// each one as a **watch-only** wallet. No signing keys are derived +/// here — signing happens later, on demand, via the configured +/// `MnemonicResolverHandle` (`sign_with_mnemonic_resolver` and its +/// siblings), which fail-closed gate the resolver-supplied seed +/// against the loaded `wallet_id`. Does not produce wallet handles — +/// follow up with [`platform_wallet_manager_get_wallet`] per +/// `wallet_id`. +/// +/// A wallet whose persisted row is structurally corrupt is +/// **skipped**, not failed: the call still returns `Success`, every +/// skipped `(wallet_id, reason)` is logged, and — when `out_outcome` +/// is non-null — surfaced through it. +/// +/// # Safety +/// - `out_outcome` may be null (caller doesn't want the summary); +/// otherwise it must point to writable `LoadOutcomeFFI` storage and +/// the caller must later release it via +/// [`platform_wallet_load_outcome_free`]. #[no_mangle] pub unsafe extern "C" fn platform_wallet_manager_load_from_persistor( manager_handle: Handle, + out_outcome: *mut LoadOutcomeFFI, ) -> PlatformWalletFFIResult { + // Initialize the out-param first so every early-return path below + // leaves it releasable (zeroed counts, null `skipped`) — matches this + // crate's null-init-first out-pointer idiom and keeps + // `platform_wallet_load_outcome_free` safe on the error paths too. + if !out_outcome.is_null() { + std::ptr::write( + out_outcome, + LoadOutcomeFFI { + loaded_count: 0, + skipped_count: 0, + skipped: std::ptr::null_mut(), + }, + ); + } + let option = PLATFORM_WALLET_MANAGER_STORAGE.with_item(manager_handle, |manager| { runtime().block_on(manager.load_from_persistor()) }); let result = unwrap_option_or_return!(option); - unwrap_result_or_return!(result); + let outcome = unwrap_result_or_return!(result); + + // Never silently drop the outcome: log a structured summary plus + // one line per skipped wallet (the host can inspect / clear the + // corrupt rows). The `loaded_count`/`skipped_count` pair below + // encodes the Rust `LoadOutcome` 3-state for the host: skipped == 0 + // is a full load, loaded == 0 with skips is nothing-usable, and both + // non-zero is a partial load. + tracing::info!( + loaded = outcome.loaded().len(), + skipped = outcome.skipped().len(), + "platform_wallet_manager_load_from_persistor complete" + ); + for (wid, reason) in outcome.skipped() { + tracing::warn!( + wallet_id = %hex::encode(wid), + reason = %reason, + "load_from_persistor skipped a persisted wallet" + ); + } + + if !out_outcome.is_null() { + let skipped_vec: Vec = outcome + .skipped() + .iter() + .map(|(wid, reason)| SkippedWalletFFI { + wallet_id: *wid, + reason_code: skip_reason_code(reason), + }) + .collect(); + let skipped_count = skipped_vec.len(); + let skipped_ptr = crate::core_wallet_types::vec_to_ptr(skipped_vec); + std::ptr::write( + out_outcome, + LoadOutcomeFFI { + loaded_count: outcome.loaded().len(), + skipped_count, + skipped: skipped_ptr, + }, + ); + } PlatformWalletFFIResult::ok() } +/// Release the heap `skipped` array a successful +/// [`platform_wallet_manager_load_from_persistor`] wrote into a +/// `LoadOutcomeFFI`. Idempotent: nulls the pointer after freeing, and +/// a null `outcome` (or already-freed array) is a no-op. +/// +/// # Safety +/// `outcome` must point to a `LoadOutcomeFFI` previously populated by +/// `platform_wallet_manager_load_from_persistor`, not freed already. +#[no_mangle] +pub unsafe extern "C" fn platform_wallet_load_outcome_free(outcome: *mut LoadOutcomeFFI) { + if outcome.is_null() { + return; + } + let o = &mut *outcome; + if !o.skipped.is_null() && o.skipped_count > 0 { + let slice = std::slice::from_raw_parts_mut(o.skipped, o.skipped_count); + drop(Box::from_raw(slice as *mut [SkippedWalletFFI])); + } + o.skipped = std::ptr::null_mut(); + o.skipped_count = 0; +} + /// Get a `PlatformWallet` handle for a wallet registered in the /// manager. Returns `NotFound` if no wallet with the given /// id is currently held. @@ -417,4 +596,63 @@ mod tests { assert_eq!(birth_height_override_opt(false, 0), None); assert_eq!(birth_height_override_opt(false, 99), None); } + + #[test] + fn load_skip_reason_wire_values_are_stable() { + // FFI consumers hardcode these numbers; the ABI must not drift. + assert_eq!(LOAD_SKIP_REASON_MISSING_MANIFEST, 100); + assert_eq!(LOAD_SKIP_REASON_MALFORMED_XPUB, 101); + assert_eq!(LOAD_SKIP_REASON_DECODE_ERROR, 102); + assert_eq!(LOAD_SKIP_REASON_SNAPSHOT_IDENTITY_MISMATCH, 103); + assert_eq!(LOAD_SKIP_REASON_CORRUPT_OTHER, 199); + assert_eq!(LOAD_SKIP_REASON_OTHER, 200); + } + + #[test] + fn skip_reason_code_maps_known_kinds_to_constants() { + use platform_wallet::manager::load_outcome::CorruptKind; + use platform_wallet::SkipReason; + + let corrupt = |kind| SkipReason::CorruptPersistedRow { kind }; + assert_eq!( + skip_reason_code(&corrupt(CorruptKind::MissingManifest)), + LOAD_SKIP_REASON_MISSING_MANIFEST + ); + assert_eq!( + skip_reason_code(&corrupt(CorruptKind::MalformedXpub)), + LOAD_SKIP_REASON_MALFORMED_XPUB + ); + assert_eq!( + skip_reason_code(&corrupt(CorruptKind::SnapshotIdentityMismatch)), + LOAD_SKIP_REASON_SNAPSHOT_IDENTITY_MISMATCH + ); + assert_eq!( + skip_reason_code(&corrupt(CorruptKind::DecodeError("boom".into()))), + LOAD_SKIP_REASON_DECODE_ERROR + ); + } + + #[test] + fn load_from_persistor_initializes_out_param_on_early_return() { + // An unknown handle early-returns before the success block. The + // out-param must be reset to a releasable zeroed state so a caller + // that later calls `platform_wallet_load_outcome_free` never does + // `Box::from_raw` on the uninitialized `skipped` pointer. + let mut outcome = LoadOutcomeFFI { + loaded_count: 42, + skipped_count: 7, + skipped: std::ptr::NonNull::::dangling().as_ptr(), + }; + + let result = + unsafe { platform_wallet_manager_load_from_persistor(NULL_HANDLE, &mut outcome) }; + + assert_ne!(result.code, PlatformWalletFFIResultCode::Success); + assert_eq!(outcome.loaded_count, 0); + assert_eq!(outcome.skipped_count, 0); + assert!(outcome.skipped.is_null()); + + // Null `skipped` now makes the release path a safe no-op. + unsafe { platform_wallet_load_outcome_free(&mut outcome) }; + } } diff --git a/packages/rs-platform-wallet-ffi/src/persistence.rs b/packages/rs-platform-wallet-ffi/src/persistence.rs index 4d614581c2f..a6f501d1ec6 100644 --- a/packages/rs-platform-wallet-ffi/src/persistence.rs +++ b/packages/rs-platform-wallet-ffi/src/persistence.rs @@ -23,6 +23,7 @@ use platform_wallet::changeset::{ AccountAddressPoolEntry, AccountRegistrationEntry, ClientStartState, ClientWalletStartState, Merge, PersistenceError, PlatformWalletChangeSet, PlatformWalletPersistence, }; +use platform_wallet::manager::load_outcome::{CorruptKind, SkipReason}; use platform_wallet::wallet::platform_wallet::WalletId; use platform_wallet::wallet::{PerAccountPlatformAddressState, PerWalletPlatformAddressState}; use std::collections::BTreeMap; @@ -156,11 +157,10 @@ pub struct PersistenceCallbacks { ) -> i32, >, /// Invoked on [`FFIPersister::load`] to pull the persisted wallet - /// list back into Rust for external-signable reconstruction. - /// (The function name still reads "watch-only" in older docs; the - /// reconstructed `Wallet` is built via - /// `Wallet::new_external_signable` so the signer surface routes - /// back to the host's keychain.) + /// list back into Rust. Each entry is rebuilt into a transient + /// `Wallet` used only to shape the keyless start-state projection; + /// the manager then re-registers the wallet watch-only and signs on + /// demand via the host mnemonic resolver. /// /// Implementations must set `*out_entries` to a Swift-allocated /// array of `WalletRestoreEntryFFI` and `*out_count` to the @@ -1611,11 +1611,36 @@ impl PlatformWalletPersistence for FFIPersister { // fires before we leave this function. let entries = unsafe { slice::from_raw_parts(entries_ptr, count) }; for entry in entries { - let (wallet_state, platform_address_state) = build_wallet_start_state(entry)?; - out.wallets.insert(entry.wallet_id, wallet_state); - if let Some(platform_address_state) = platform_address_state { - out.platform_addresses - .insert(entry.wallet_id, platform_address_state); + match build_wallet_start_state(entry) { + Ok((wallet_state, platform_address_state)) => { + out.wallets.insert(entry.wallet_id, wallet_state); + if let Some(platform_address_state) = platform_address_state { + out.platform_addresses + .insert(entry.wallet_id, platform_address_state); + } + } + Err(e) => { + // One corrupt persisted row must never abort the whole + // restore. Errors from `build_wallet_start_state` are + // inherently per-row (decode / projection of THIS entry, + // e.g. a malformed account xpub), so record the wallet as + // skipped and continue — the manager folds this into + // `LoadOutcome::skipped` and fires + // `on_wallet_skipped_on_load`, and the other rows still + // load. `PersistenceError`'s Display is structural (no + // raw row bytes / key material), safe for `DecodeError`. + tracing::warn!( + wallet_id = %hex::encode(entry.wallet_id), + error = %e, + "load: skipping corrupt wallet restore-entry; continuing with the rest" + ); + out.skipped.push(( + entry.wallet_id, + SkipReason::CorruptPersistedRow { + kind: corrupt_kind_from_build_err(&e), + }, + )); + } } } @@ -2822,12 +2847,50 @@ impl Drop for LoadGuard { } } -/// Reconstruct an external-signable [`Wallet`] + matching start-state -/// bucket from a single `WalletRestoreEntryFFI`. The mnemonic / seed -/// stays in the host's keychain; signing requests route back through -/// the configured signer surface (see -/// `Wallet::new_external_signable`). Earlier revisions of this code -/// path produced a `WatchOnly` wallet — that has been replaced. +/// Marker error: an account xpub failed to bincode-decode into a +/// well-formed extended public key. Boxed into the +/// `PersistenceError::Backend` `source` so [`corrupt_kind_from_build_err`] +/// recovers the classification by downcast — a typed discriminator +/// rather than a `Display`-text match. +#[derive(Debug)] +struct MalformedXpubError(String); + +impl std::fmt::Display for MalformedXpubError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "failed to decode account xpub: {}", self.0) + } +} + +impl std::error::Error for MalformedXpubError {} + +/// Classify a [`build_wallet_start_state`] failure for the FFI +/// `reason_code`: a boxed [`MalformedXpubError`] in the backend `source` +/// maps to [`CorruptKind::MalformedXpub`] (101), anything else to +/// [`CorruptKind::DecodeError`] (102). +fn corrupt_kind_from_build_err(e: &PersistenceError) -> CorruptKind { + if let PersistenceError::Backend { source, .. } = e { + if source.downcast_ref::().is_some() { + return CorruptKind::MalformedXpub; + } + } + CorruptKind::DecodeError(e.to_string()) +} + +/// Reconstruct the keyless [`ClientWalletStartState`] (and optional +/// platform-address bucket) for one persisted `WalletRestoreEntryFFI`. +/// +/// A transient `Wallet` is built here solely to shape the account +/// manifest and core-state projection returned below; it never leaves +/// this function. The manager rehydrates each wallet **watch-only** +/// (via `Wallet::new_watch_only`) from that manifest and signs on +/// demand through the host mnemonic resolver — no seed crosses this +/// boundary. +/// +/// # Errors +/// +/// Returns [`PersistenceError`] on any per-row decode/projection +/// failure (e.g. a malformed account xpub); the caller records the +/// wallet as skipped and continues restoring the rest. fn build_wallet_start_state( entry: &WalletRestoreEntryFFI, ) -> Result< @@ -2878,25 +2941,32 @@ fn build_wallet_start_state( let xpub_bytes = unsafe { slice_from_raw(spec.account_xpub_bytes, spec.account_xpub_bytes_len) }; let (account_xpub, _): (ExtendedPubKey, usize) = - bincode::decode_from_slice(xpub_bytes, config::standard()).map_err(|e| { - PersistenceError::backend(format!("failed to decode account xpub: {}", e)) - })?; + bincode::decode_from_slice(xpub_bytes, config::standard()) + .map_err(|e| PersistenceError::backend(MalformedXpubError(e.to_string())))?; + // Same xpub-failure family as the bincode-decode above: a + // well-decoded xpub that `Account::from_xpub` still rejects is a + // malformed key, so mark it so `corrupt_kind_from_build_err` + // classifies it as MalformedXpub (101), not the generic + // decode-error reason code (102). let account = Account::from_xpub(Some(entry.wallet_id), account_type, account_xpub, network) .map_err(|e| { - PersistenceError::backend(format!("Account::from_xpub failed: {:?}", e)) + PersistenceError::backend(MalformedXpubError(format!( + "Account::from_xpub failed: {e:?}" + ))) })?; accounts.insert(account).map_err(|e| { PersistenceError::backend(format!("AccountCollection::insert failed: {}", e)) })?; } - // External-signable wallet — the mnemonic / seed lives in the - // iOS Keychain, not in this Rust handle. Signing requests route - // back to the host through the configured signer surface; the - // host fetches the mnemonic from the Keychain on demand. The - // wallet_id is passed in directly (no recomputation from a root - // xpub the snapshot doesn't carry). + // Transient scratch wallet — used only to shape the account + // manifest and core-state projection below, then dropped; its + // `WalletType` never reaches the manager, which re-registers the + // wallet watch-only and signs on demand via the host mnemonic + // resolver (no seed crosses this boundary). The wallet_id is passed + // in directly (no recomputation from a root xpub the snapshot + // doesn't carry). let wallet = Wallet::new_external_signable(network, entry.wallet_id, accounts); // Stamp the persisted core-chain sync metadata onto the rebuilt @@ -2917,14 +2987,24 @@ fn build_wallet_start_state( } // Persisted `last_applied_chain_lock` — bincode-decoded from the - // bytes Swift handed back. Restoring this before the wallet - // enters the manager means the asset-lock-resume CL-from-metadata - // fallback (`proof.rs`) can fire immediately at app launch on - // any tracked lock whose funding block height is `<= cl.block_height`, - // without waiting for SPV to re-apply a fresh CL. SPV persists - // its own `best_chainlock` independently; this is the symmetric + // bytes Swift handed back onto the local `wallet_info` metadata. The + // manager consumes this snapshot verbatim, so the asset-lock-resume + // CL-from-metadata fallback (`proof.rs`) fires at app launch on any + // tracked lock whose funding block height is `<= cl.block_height`, + // without waiting for SPV to re-apply a fresh CL. SPV persists its + // own `best_chainlock` independently; this is the symmetric // wallet-side restore. // + // TRUST BOUNDARY: this chain lock is read from the unauthenticated + // local store and is NOT re-verified here — decode enforces the + // struct shape only; no BLS/quorum signature check runs on this + // path. Treat the value as a cache hint, not a trusted source. It + // merely seeds the asset-lock-resume fallback; data integrity for + // that path rests on the DOWNSTREAM network re-verification of the + // asset-lock proof itself (`proof.rs`), which is authoritative. A + // forged/stale local CL can at most trigger an earlier resume + // attempt whose proof then fails network verification. + // // Decode failure is treated as miss: malformed bytes here are // either a serialisation-shape regression in upstream `ChainLock` // or a corrupted SwiftData row — neither is recoverable in-flight, @@ -3451,9 +3531,39 @@ fn build_wallet_start_state( // status without rebroadcasting. let unused_asset_locks = build_unused_asset_locks(entry)?; + // Hand the fully-restored `wallet_info` across as the keyless + // snapshot: no `Wallet`, seed, private key, unencrypted seed, or + // password ever crosses `load()` — `ManagedWalletInfo` carries only + // balances / pools / UTXOs, never key material. The manager rebuilds + // a watch-only wallet from the + // manifest via `Wallet::new_watch_only` and consumes this snapshot + // directly, so everything the decode blocks above restored survives + // verbatim: per-account UTXO and tx-record attribution (including + // the unresolved asset-lock funding records), exact pool contents + // with per-index `used` flags (the address-reuse guard and the SPV + // watch set), and the sync metadata / chainlock. Signing happens + // later via the on-demand `sign_with_mnemonic_resolver` path, which + // fail-closed gates the resolver-supplied seed against the loaded + // `wallet_id`. The locally-built `wallet` is dropped — it was only + // needed to shape the account collection / UTXO routing above. + let account_manifest: Vec = wallet + .accounts + .all_accounts() + .into_iter() + .map(|a| AccountRegistrationEntry { + account_type: a.account_type, + account_xpub: a.account_xpub, + }) + .collect(); + + // Identity PUBLIC keys and DashPay contacts are already restored + // into `identity_manager` by `build_wallet_identity_bucket` + // (contacts inline via `restore_dashpay_contacts`). let wallet_state = ClientWalletStartState { - wallet, - wallet_info, + network, + birth_height: entry.birth_height, + account_manifest, + wallet_info: Box::new(wallet_info), identity_manager, unused_asset_locks, }; @@ -3659,6 +3769,10 @@ fn status_from_u8(b: u8) -> Result Result, PersistenceError> { @@ -4310,13 +4424,17 @@ fn is_legacy_removed_account_tag(type_tag: u8) -> bool { /// Read `len` bytes from a Swift-owned pointer as a `&[u8]`. /// +/// A host-supplied `len` exceeding `isize::MAX` (the `from_raw_parts` +/// bound) is treated as a corrupt length and yields an empty slice rather +/// than being handed to `from_raw_parts` (UB). +/// /// # Safety /// /// `ptr` must point to at least `len` valid bytes for the duration of /// the callback. Caller holds the callback window open via /// `LoadGuard`. unsafe fn slice_from_raw<'a>(ptr: *const u8, len: usize) -> &'a [u8] { - if ptr.is_null() || len == 0 { + if ptr.is_null() || len == 0 || len > isize::MAX as usize { &[] } else { slice::from_raw_parts(ptr, len) @@ -4485,6 +4603,103 @@ mod tests { use key_wallet::mnemonic::{Language, Mnemonic}; use key_wallet::wallet::Wallet; + /// A malformed-xpub failure must surface as `MalformedXpub` (FFI + /// `reason_code` 101), distinct from the generic `DecodeError` (102), + /// so the host can special-case unrecoverable key-material corruption. + #[test] + fn malformed_xpub_error_maps_to_dedicated_corrupt_kind() { + // A boxed `MalformedXpubError` must be recovered by downcast, + // independently of its human-readable `Display` text. + let xpub_err = + PersistenceError::backend(MalformedXpubError("invalid checksum".to_string())); + assert_eq!( + corrupt_kind_from_build_err(&xpub_err), + CorruptKind::MalformedXpub, + "an xpub-decode failure must surface as MalformedXpub (code 101)" + ); + + // Any unrelated structural failure keeps the generic family — + // even when its message happens to mention "decode account xpub". + let other_err = PersistenceError::backend("failed to decode account xpub: bad network"); + assert!( + matches!( + corrupt_kind_from_build_err(&other_err), + CorruptKind::DecodeError(_) + ), + "non-xpub failures must stay DecodeError (code 102)" + ); + } + + /// A `len` past `isize::MAX` is a corrupt host-supplied length — + /// handing it to `from_raw_parts` would be UB, so the guard yields an + /// empty slice. The pointer is non-null but never dereferenced: the + /// guard short-circuits before any read. + #[test] + fn slice_from_raw_rejects_overflowing_len() { + let ptr = std::ptr::NonNull::::dangling().as_ptr() as *const u8; + let slice = unsafe { slice_from_raw(ptr, isize::MAX as usize + 1) }; + assert_eq!( + slice, + &[] as &[u8], + "a len exceeding isize::MAX must yield an empty slice, not touch the pointer" + ); + } + + /// End-to-end coverage of the malformed-xpub classification at its + /// real call site: `build_wallet_start_state` fed a well-formed + /// buffer that is not a decodable `ExtendedPubKey` must surface a + /// `MalformedXpub` (code 101), not the generic decode-error family. + #[test] + fn build_wallet_start_state_malformed_xpub_classifies_as_malformed() { + let bad_xpub: [u8; 4] = [0xde, 0xad, 0xbe, 0xef]; + let spec = AccountSpecFFI { + type_tag: AccountTypeTagFFI::Standard as u8, + standard_tag: StandardAccountTypeTagFFI::Bip44 as u8, + index: 0, + registration_index: 0, + key_class: 0, + user_identity_id: [0u8; 32], + friend_identity_id: [0u8; 32], + account_xpub_bytes: bad_xpub.as_ptr(), + account_xpub_bytes_len: bad_xpub.len(), + }; + let entry = WalletRestoreEntryFFI { + wallet_id: [0u8; 32], + network: FFINetwork::Testnet, + accounts: &spec, + accounts_count: 1, + platform_address_balances: std::ptr::null(), + platform_address_balances_count: 0, + platform_sync_height: 0, + platform_sync_timestamp: 0, + platform_last_known_recent_block: 0, + identities: std::ptr::null(), + identities_count: 0, + birth_height: 0, + synced_height: 0, + last_processed_height: 0, + last_synced: 0, + utxos: std::ptr::null(), + utxos_count: 0, + tracked_asset_locks: std::ptr::null(), + tracked_asset_locks_count: 0, + unresolved_asset_lock_tx_records: std::ptr::null(), + unresolved_asset_lock_tx_records_count: 0, + core_address_pools: std::ptr::null(), + core_address_pools_count: 0, + last_applied_chain_lock_bytes: std::ptr::null(), + last_applied_chain_lock_bytes_len: 0, + }; + + let err = build_wallet_start_state(&entry) + .expect_err("a malformed account xpub must fail the rebuild"); + assert_eq!( + corrupt_kind_from_build_err(&err), + CorruptKind::MalformedXpub, + "a malformed account xpub must classify as MalformedXpub (code 101) end-to-end" + ); + } + /// Regression: restored pool addresses must be tagged with the /// WALLET's network, not the network the base58 string parses as. /// Devnet shares testnet's base58 prefixes, so a devnet wallet's diff --git a/packages/rs-platform-wallet-ffi/src/shielded_send.rs b/packages/rs-platform-wallet-ffi/src/shielded_send.rs index 94fbbf1269a..dbdead34c63 100644 --- a/packages/rs-platform-wallet-ffi/src/shielded_send.rs +++ b/packages/rs-platform-wallet-ffi/src/shielded_send.rs @@ -516,6 +516,15 @@ fn map_spend_result( PlatformWalletFFIResultCode::ErrorShieldedBroadcastFailed, format!("{operation} failed: {e}"), ), + // Definitively failed on an address-nonce race (a shield spends platform + // address funds; a shield reserves no notes). Its own code carries the + // safe-to-retry contract AND lets the host recognize the self-healing + // nonce mismatch — a plain retry re-fetches the nonce. Without this arm + // it would regress to the generic `ErrorWalletOperation` below. + Err(e @ PlatformWalletError::AddressNonceMismatch { .. }) => PlatformWalletFFIResult::err( + PlatformWalletFFIResultCode::ErrorAddressNonceMismatch, + format!("{operation} failed: {e}"), + ), Err(e) => PlatformWalletFFIResult::err( PlatformWalletFFIResultCode::ErrorWalletOperation, format!("{operation} failed: {e}"), @@ -1438,4 +1447,35 @@ mod tests { PlatformWalletFFIResultCode::Success ); } + + /// A shield (Type 15) definitively rejected on an address-nonce race must + /// map to the dedicated `ErrorAddressNonceMismatch` — NOT regress to the + /// generic `ErrorWalletOperation` — so hosts keep the safe-to-retry signal. + /// The submitted/expected nonce values must survive in the message. + #[test] + fn map_spend_result_maps_address_nonce_mismatch_to_dedicated_code() { + let mismatch: Result<(), PlatformWalletError> = + Err(PlatformWalletError::AddressNonceMismatch { + address: PlatformAddress::P2pkh([7u8; 20]), + provided_nonce: 1, + expected_nonce: 2, + }); + let result = map_spend_result(mismatch, "shielded shield"); + assert_eq!( + result.code, + PlatformWalletFFIResultCode::ErrorAddressNonceMismatch, + "shield nonce rejection must not regress to ErrorWalletOperation" + ); + let msg = message_of(&result); + // Pin the EXACT rendered substrings, not bare digits, so a + // provided/expected transposition would fail the test. + assert!( + msg.contains("submitted nonce 1"), + "submitted (provided) nonce must render exactly: {msg}" + ); + assert!( + msg.contains("Platform expected 2"), + "expected nonce must render exactly: {msg}" + ); + } } diff --git a/packages/rs-platform-wallet-ffi/src/wallet_restore_types.rs b/packages/rs-platform-wallet-ffi/src/wallet_restore_types.rs index 95629084629..0be67e2ff60 100644 --- a/packages/rs-platform-wallet-ffi/src/wallet_restore_types.rs +++ b/packages/rs-platform-wallet-ffi/src/wallet_restore_types.rs @@ -5,12 +5,11 @@ //! On write: `on_persist_account_registrations_fn` fires with the //! `AccountSpecFFI` shape so Swift can store accounts in SwiftData. //! On load: `on_load_wallet_list_fn` returns an array of -//! `WalletRestoreEntryFFI` which Rust assembles into an -//! external-signable `Wallet` via `Wallet::new_external_signable` + -//! per-account `Account::from_xpub`. (The mnemonic stays in the -//! host's keychain; signing routes back through the configured -//! signer surface. Earlier revisions reconstructed a `WatchOnly` -//! wallet — that path has been replaced.) +//! `WalletRestoreEntryFFI` which Rust assembles into a transient +//! `Wallet` (via `Wallet::new_external_signable` + per-account +//! `Account::from_xpub`) used only to shape the keyless start-state +//! projection; the manager then re-registers the wallet watch-only and +//! signs on demand via the host mnemonic resolver. //! //! All `*const u8` pointers must stay valid for the duration of the //! load callback. Swift owns the allocation and is asked to free it diff --git a/packages/rs-platform-wallet/README.md b/packages/rs-platform-wallet/README.md index d91bafa525e..e873e968125 100644 --- a/packages/rs-platform-wallet/README.md +++ b/packages/rs-platform-wallet/README.md @@ -85,6 +85,99 @@ The package is structured as follows: - Active/inactive status - Note: Credit balance and revision are accessed from the Identity itself +## Persistence architecture + +This section is normative: it records the agreed model for how wallet +state, the persister, and clients relate. Changes that violate these +invariants need an explicit architecture discussion first, not just a +code review. + +``` + commands (send, register, sync, …) + client ──────────────────────────────────────▶ platform-wallet + │ │ + │ reads (display) changesets │ (single writer) + ▼ ▼ + ┌─────────────────────── persisted store ──────────────────────┐ + │ wallet-state tables: written ONLY by platform-wallet │ + │ client-owned tables (UI prefs etc.): written by client │ + └───────────────────────────────────────────────────────────────┘ + ▲ + │ load(persister) at launch — verbatim + platform-wallet +``` + +### Roles + +- **platform-wallet** is the authority for state *transitions*. Every + mutation of wallet state happens here and is emitted as a changeset + to the persister. Its in-memory state is volatile — a cache that is + empty at process start. +- **The persisted store** is the authority for state *history*: it is + the only copy of the wallet that survives a restart, and it doubles + as the client's **read model** — UIs *may* render persisted rows directly + and reactively. Display therefore never blocks on platform-wallet + being unlocked or synced; the local seedless restore is still a startup gate. +- **Clients** (dash-evo-tool, the iOS SDK app, …) issue commands to + platform-wallet and read the store freely. They never write + wallet-state rows. + +### Invariants + +1. **Single writer** (enforced by review, not the storage layer). Only platform-wallet's changesets mutate + wallet-state tables. Clients may keep their own tables (UI + preferences, view state) in the same database; ownership is per + table family, never shared. +2. **The store schema is a versioned public contract.** Two parties + depend on it — the persister's writes and every client's reads — so + schema changes are breaking changes for clients, not private + refactors. +3. **Reads never feed back into writes** except through platform-wallet + commands. A client that computes something from persisted rows and + wants it stored must go through a platform-wallet API. +4. **`load()` is verbatim.** At launch, platform-wallet reconstructs + itself from the store through + [`PlatformWalletPersistence::load`]; the store contains exactly what + platform-wallet wrote, so the load path must consume it as-is. + Re-deriving, re-inferring, or "repairing" state during load is + forbidden — a lossy round-trip here silently diverges the wallet + from its own history (per-account attribution, address-pool + `used` flags, and SPV watch-set coverage are the historical + casualties). Anything genuinely missing from the store re-warms on + the next sync, never inside `load()`. +5. **Persist errors are hard errors.** The store is the only durable + copy, and part of it — the account manifest, address used-flags, + birth heights, identity/contact associations — is *local-only*: no + chain rescan can ever reconstruct it. A swallowed persister write + error is silent, permanent data loss discovered at the next launch. +6. **Load is seedless.** The store never carries a seed or a + `Wallet`; restore produces watch-only wallets + (`Wallet::new_watch_only`) and signing keys are derived on demand + via the resolver-backed sign paths. See the trust-boundary notes on + [`PlatformWalletPersistence::load`] for what is (and is not) + authenticated on this path. + +### What restore is for + +Because the store is the read model, restoring platform-wallet at +launch is **not** about showing balances or history — the client +already renders those from the store. It exists to refill the +operational state that only lives in platform-wallet's memory: + +- **Detection** — the SPV watch set is the address-pool contents; + without it, incoming payments to existing addresses are not seen. +- **Spending** — coin/input selection runs against the in-memory UTXO + set. +- **Resume** — sync watermarks, tracked asset locks mid-registration, + and fresh-receive-address (`used`) state. + +Persisters that can reconstruct the full keyless snapshot hand it back +as `ClientWalletStartState::wallet_info` (consumed verbatim, per +invariant 4). The flattened projection fields +(`core_state`/`used_core_addresses`) are a transitional fallback for +persisters that cannot build a snapshot yet, and are slated for +removal once every in-tree persister produces snapshots. + ## Key Features ### Wallet Operations (via ManagedWalletInfo) diff --git a/packages/rs-platform-wallet/src/changeset/changeset.rs b/packages/rs-platform-wallet/src/changeset/changeset.rs index 8dc2c705427..a5a5b72661d 100644 --- a/packages/rs-platform-wallet/src/changeset/changeset.rs +++ b/packages/rs-platform-wallet/src/changeset/changeset.rs @@ -1178,8 +1178,12 @@ pub struct PlatformWalletChangeSet { /// the merge policy (plain `Vec::extend`, dedup is the apply-side /// caller's job). pub account_registrations: Vec, - /// Address-pool snapshots emitted at wallet create (initial - /// gap-limit population) and on any pool extension / "used" flip. + /// Full address-pool snapshots: emitted once at wallet registration. + /// Incremental derivations are delivered via `core.addresses_derived` + /// (the `WalletEvent` bus / FFI path); no per-block in-band pool + /// snapshot is written. The storage persister intentionally ignores this + /// field (UTXO attribution is hardcoded to account 0); non-storage + /// consumers (e.g. the iOS FFI address registry) may still read it. /// See [`AccountAddressPoolEntry`] for the merge policy. pub account_address_pools: Vec, /// Deferred contact-crypto ops enqueued by the seedless background sweep diff --git a/packages/rs-platform-wallet/src/changeset/client_start_state.rs b/packages/rs-platform-wallet/src/changeset/client_start_state.rs index c63e5a262be..e8e1ed962b7 100644 --- a/packages/rs-platform-wallet/src/changeset/client_start_state.rs +++ b/packages/rs-platform-wallet/src/changeset/client_start_state.rs @@ -13,6 +13,7 @@ use crate::changeset::client_wallet_start_state::ClientWalletStartState; use crate::changeset::platform_address_sync_start_state::PlatformAddressSyncStartState; #[cfg(feature = "shielded")] use crate::changeset::shielded_sync_start_state::ShieldedSyncStartState; +use crate::manager::load_outcome::SkipReason; use crate::wallet::platform_wallet::WalletId; /// Snapshot of everything a persister hands back on @@ -32,6 +33,13 @@ pub struct ClientStartState { /// Per-wallet startup slices (UTXOs and unused asset locks, each /// bucketed by account index). pub wallets: BTreeMap, + /// Wallets the persister itself rejected as structurally corrupt + /// before they could be reconstructed (e.g. a malformed account xpub + /// that aborts decode). They never appear in `wallets`; the manager + /// folds them into the load outcome's `skipped` set and notifies + /// handlers, so one bad persisted row never blocks the rest of the + /// batch. Empty for persisters that decode every row up front. + pub skipped: Vec<(WalletId, SkipReason)>, /// Restored shielded sub-wallet state — per-`SubwalletId` /// notes + sync watermarks. Consumed at `bind_shielded` time /// to rehydrate the in-memory `SubwalletState` so spending / @@ -42,7 +50,11 @@ pub struct ClientStartState { impl ClientStartState { pub fn is_empty(&self) -> bool { - let core_empty = self.platform_addresses.is_empty() && self.wallets.is_empty(); + // A skipped-only load (rows rejected, no wallets) is NOT empty: + // the manager must still fire its skip notifications. + let core_empty = self.platform_addresses.is_empty() + && self.wallets.is_empty() + && self.skipped.is_empty(); #[cfg(feature = "shielded")] { core_empty && self.shielded.is_empty() @@ -53,3 +65,28 @@ impl ClientStartState { } } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::manager::load_outcome::CorruptKind; + + /// A skipped-only start state must report non-empty so the manager + /// still surfaces `LoadOutcome::skipped` and fires skip handlers. + #[test] + fn skipped_only_state_is_not_empty() { + let mut state = ClientStartState::default(); + assert!(state.is_empty(), "a freshly defaulted state is empty"); + + state.skipped.push(( + [0u8; 32], + SkipReason::CorruptPersistedRow { + kind: CorruptKind::MalformedXpub, + }, + )); + assert!( + !state.is_empty(), + "a skipped-only state must be non-empty so skip notifications fire" + ); + } +} diff --git a/packages/rs-platform-wallet/src/changeset/client_wallet_start_state.rs b/packages/rs-platform-wallet/src/changeset/client_wallet_start_state.rs index 83b6d860742..28f23124935 100644 --- a/packages/rs-platform-wallet/src/changeset/client_wallet_start_state.rs +++ b/packages/rs-platform-wallet/src/changeset/client_wallet_start_state.rs @@ -1,36 +1,58 @@ //! Per-wallet portion of [`ClientStartState`](crate::changeset::ClientStartState). //! -//! Everything a single wallet contributes to the startup snapshot: the -//! key-wallet [`Wallet`] + [`ManagedWalletInfo`] pair, a lean -//! identity-manager snapshot, and still-unused asset locks bucketed by -//! account index. +//! **Keyless by type.** This carries everything needed to *reconstruct* +//! a watch-only wallet — network, birth height, the account manifest, +//! the managed-state snapshot, identities, filtered asset locks — but +//! **no** [`Wallet`](key_wallet::Wallet) and no seed. The persister +//! can never mint a `Wallet`; the manager rebuilds a watch-only one via +//! [`Wallet::new_watch_only`](key_wallet::wallet::Wallet::new_watch_only) +//! from the manifest, applies this state, and defers signing-key +//! derivation to the on-demand sign path (`rs-platform-wallet-ffi`'s +//! `dash_sdk_sign_with_mnemonic_resolver_and_path` and its siblings). use std::collections::BTreeMap; use crate::changeset::identity_manager_start_state::IdentityManagerStartState; +use crate::changeset::AccountRegistrationEntry; use crate::wallet::asset_lock::tracked::TrackedAssetLock; use dashcore::OutPoint; -use key_wallet::wallet::ManagedWalletInfo; -use key_wallet::Wallet; +use key_wallet::wallet::managed_wallet_info::ManagedWalletInfo; +use key_wallet::Network; -/// Per-wallet slice of the startup snapshot. +/// Keyless per-wallet slice of the startup snapshot. /// -/// Used as the value type in [`ClientStartState::wallets`](crate::changeset::ClientStartState::wallets). +/// Used as the value type in +/// [`ClientStartState::wallets`](crate::changeset::ClientStartState::wallets). +/// The structural absence of a `Wallet`/seed field is the SECRETS.md +/// boundary, enforced by type rather than convention. #[derive(Debug)] pub struct ClientWalletStartState { - /// The key-wallet [`Wallet`] to rehydrate on startup. Carries the - /// HD key material and account configuration the rest of the - /// per-wallet state hangs off of. - pub wallet: Wallet, - /// Managed wallet info holding non-key-material state (balances, - /// account metadata, UTXO set, etc.) for this wallet. - pub wallet_info: ManagedWalletInfo, + /// Network the wallet is bound to. + pub network: Network, + /// Best estimate of the chain tip at creation time (`0` = scan + /// from genesis / unknown). + pub birth_height: u32, + /// Keyless account manifest — the account-set oracle for building the + /// watch-only wallet (one watch-only account per entry's xpub). + pub account_manifest: Vec, + /// Full keyless managed-wallet snapshot: pools with exact derivation + /// indices and `used` flags, per-account UTXO and tx-record + /// attribution, IS-lock set, and sync metadata. [`ManagedWalletInfo`] + /// carries **no key material** (see its docs: balances, account + /// metadata, UTXO set), so the SECRETS.md boundary holds: still no + /// `Wallet`, no seed. + /// + /// The manager consumes it directly after validating its + /// `wallet_id`/`network` against the row and its account set against + /// the manifest — preserving per-account attribution, the full SPV + /// watch set, and pool used-state verbatim, without re-deriving + /// anything. The FFI/iOS persister populates this. + pub wallet_info: Box, /// Lean snapshot of this wallet's - /// [`IdentityManager`](crate::wallet::identity::IdentityManager): - /// owned + watched identities, primary selection, and the - /// gap-limit scan watermark. + /// [`IdentityManager`](crate::wallet::identity::IdentityManager). pub identity_manager: IdentityManagerStartState, - /// Asset locks that have not yet been consumed by an identity - /// registration / top-up, keyed by account index → outpoint. + /// Asset locks not yet consumed by an identity registration / + /// top-up, keyed by account index → outpoint. Terminal `Consumed` + /// rows are already filtered out by the asset-lock reader. pub unused_asset_locks: BTreeMap>, } diff --git a/packages/rs-platform-wallet/src/changeset/core_bridge.rs b/packages/rs-platform-wallet/src/changeset/core_bridge.rs index 3cf5dd28f13..389490ce882 100644 --- a/packages/rs-platform-wallet/src/changeset/core_bridge.rs +++ b/packages/rs-platform-wallet/src/changeset/core_bridge.rs @@ -135,12 +135,9 @@ async fn build_core_changeset( new_utxos: derive_new_utxos(record), spent_utxos: derive_spent_utxos(record), records: vec![(**record).clone()], - // Mirror the upstream-emitted derived addresses - // through to the persister so newly-extended pool - // rows are written transactionally with the tx that - // triggered the extension. See - // `CoreChangeSet.addresses_derived` for the cascade- - // link rationale. + // Forward the upstream-emitted derived addresses to the + // persister; the FFI layer feeds the iOS address registry + // from this delta. See `CoreChangeSet.addresses_derived`. addresses_derived: addresses_derived.clone(), ..CoreChangeSet::default() } @@ -171,7 +168,10 @@ async fn build_core_changeset( .. } => { let mut cs = CoreChangeSet::default(); - // Inserted records bring fresh UTXOs and may consume previous ones. + // Inserted records bring fresh UTXOs and may consume previous + // ones — always project. Per-account attribution is resolved by + // the storage layer via the address→account_index lookup over + // `addresses_derived` (forwarded below). for r in inserted { cs.new_utxos.extend(derive_new_utxos(r)); cs.spent_utxos.extend(derive_spent_utxos(r)); @@ -357,3 +357,120 @@ impl CoreChangeSet { && self.addresses_derived.is_empty() } } + +#[cfg(test)] +mod tests { + use std::collections::BTreeMap; + + use dashcore::blockdata::transaction::Transaction; + use dashcore::hashes::Hash; + use key_wallet::account::{AccountType, StandardAccountType}; + use key_wallet::managed_account::transaction_record::{ + OutputDetail, TransactionDirection, TransactionRecord, + }; + use key_wallet::transaction_checking::{BlockInfo, TransactionContext, TransactionType}; + use key_wallet::WalletCoreBalance; + + use super::*; + + fn standard(index: u32) -> AccountType { + AccountType::Standard { + index, + standard_account_type: StandardAccountType::BIP44Account, + } + } + + /// A throwaway testnet P2PKH address keyed off `seed`. + fn p2pkh(seed: u8) -> dashcore::Address { + use dashcore::address::Payload; + use dashcore::PubkeyHash; + dashcore::Address::new( + dashcore::Network::Testnet, + Payload::PubkeyHash(PubkeyHash::from_byte_array([seed; 20])), + ) + } + + /// A confirmed `TransactionRecord` owned by `account_type` carrying a + /// single `Received` output worth `value` at `addr`, so + /// `derive_new_utxos` yields exactly one UTXO. + fn record_with_received_output( + account_type: AccountType, + addr: &dashcore::Address, + value: u64, + ) -> TransactionRecord { + let tx = Transaction { + version: 3, + lock_time: 0, + input: vec![], + output: vec![dashcore::TxOut { + value, + script_pubkey: addr.script_pubkey(), + }], + special_transaction_payload: None, + }; + TransactionRecord::new( + tx, + account_type, + TransactionContext::InChainLockedBlock(BlockInfo::new( + 42, + dashcore::BlockHash::from_byte_array([3u8; 32]), + 1_735_689_600, + )), + TransactionType::Standard, + TransactionDirection::Incoming, + Vec::new(), + vec![OutputDetail { + index: 0, + role: OutputRole::Received, + address: Some(addr.clone()), + value, + }], + value as i64, + ) + } + + /// Project a `TransactionDetected` for `record` through the real bridge + /// path. `balance`/`account_balances` are unused by the projection. + async fn changeset_for(record: TransactionRecord) -> CoreChangeSet { + let wm = Arc::new(RwLock::new(WalletManager::::new( + key_wallet::Network::Testnet, + ))); + let event = WalletEvent::TransactionDetected { + wallet_id: [0u8; 32], + record: Box::new(record), + balance: WalletCoreBalance::default(), + account_balances: BTreeMap::new(), + addresses_derived: Vec::new(), + }; + build_core_changeset(&wm, &event).await + } + + /// A default-account (index 0) UTXO is projected into the changeset. + #[tokio::test] + async fn default_account_utxo_persists() { + let addr = p2pkh(0x11); + let cs = changeset_for(record_with_received_output(standard(0), &addr, 500_000)).await; + assert_eq!( + cs.new_utxos.len(), + 1, + "the default-account UTXO must be projected" + ); + assert_eq!(cs.new_utxos[0].value(), 500_000); + } + + /// REGRESSION (fund-loss): a non-default-account (index != 0) UTXO is + /// projected — never dropped. Storage resolves its account attribution + /// from the derived-address table; dropping it would undercount the + /// balance. + #[tokio::test] + async fn non_default_account_utxo_persists() { + let addr = p2pkh(0x22); + let cs = changeset_for(record_with_received_output(standard(7), &addr, 900_000)).await; + assert_eq!( + cs.new_utxos.len(), + 1, + "a non-default-account UTXO must NOT be dropped" + ); + assert_eq!(cs.new_utxos[0].value(), 900_000, "funds preserved"); + } +} diff --git a/packages/rs-platform-wallet/src/changeset/traits.rs b/packages/rs-platform-wallet/src/changeset/traits.rs index 9ed22a5542e..dbd2e374c90 100644 --- a/packages/rs-platform-wallet/src/changeset/traits.rs +++ b/packages/rs-platform-wallet/src/changeset/traits.rs @@ -255,6 +255,20 @@ pub trait PlatformWalletPersistence: Send + Sync { /// per-wallet one, because `ClientStartState::platform_addresses` is /// already keyed by wallet id and the sub-changesets carry their own /// wallet attribution where needed. + /// + /// # Trust boundary + /// + /// This trait does not authenticate the [`ClientStartState`] it + /// returns: nothing here binds a wallet's account manifest to the + /// `wallet_id` it is returned under, and this contract neither + /// mandates nor performs such a check. Verifying manifest integrity + /// against a tampered or untrusted-backup store is the storage + /// layer's responsibility (a persisted commitment over the manifest, + /// keyed to the store) — see the manifest-integrity work in the + /// `platform-wallet-storage` crate. Implementors and callers must not + /// assume a manifest handed back here has been verified authentic; + /// `build_watch_only_wallet` in the `rehydrate` module documents the + /// concrete key-substitution risk this leaves open. fn load(&self) -> Result; /// Look up a single core transaction record by `txid` for `wallet_id`. diff --git a/packages/rs-platform-wallet/src/error.rs b/packages/rs-platform-wallet/src/error.rs index 473e82e5491..3c50a6eafae 100644 --- a/packages/rs-platform-wallet/src/error.rs +++ b/packages/rs-platform-wallet/src/error.rs @@ -1,6 +1,8 @@ use dpp::address_funds::PlatformAddress; +use dpp::consensus::state::address_funds::AddressInvalidNonceError; use dpp::fee::Credits; use dpp::identifier::Identifier; +use dpp::prelude::AddressNonce; use key_wallet::account::StandardAccountType; use key_wallet::Network; @@ -10,6 +12,18 @@ pub enum PlatformWalletError { #[error("Wallet creation failed: {0}")] WalletCreation(String), + /// The persister failed to load the client start state during + /// rehydration. Carries the typed [`PersistenceError`] so callers keep + /// its retry classification (`is_transient()` / + /// [`PersistenceErrorKind`]) instead of a flattened string — a + /// transient backend hiccup (e.g. `SQLITE_BUSY`) stays distinguishable + /// from a permanent failure and can be retried. + /// + /// [`PersistenceError`]: crate::changeset::PersistenceError + /// [`PersistenceErrorKind`]: crate::changeset::PersistenceErrorKind + #[error("failed to load persisted client state: {0}")] + PersisterLoad(#[from] crate::changeset::PersistenceError), + #[error("Wallet not found: {0}")] WalletNotFound(String), @@ -102,6 +116,26 @@ pub enum PlatformWalletError { #[error("SDK error: {0}")] Sdk(#[from] dash_sdk::Error), + /// Platform rejected an address-funds transition because a spent + /// address's provided nonce did not equal Platform's expected next value + /// (DPP consensus code 40603, `AddressInvalidNonceError`): the optimistic + /// `fetched + 1` nonce raced a lagging DAPI replica's stale read. + /// + /// The rejection is by design — the address-nonce path is optimistic and + /// the caller owns the retry. This variant delivers Platform's + /// `expected_nonce` verbatim so the caller can rebuild the transition with + /// it (no re-fetch needed) instead of parsing a flattened string. + #[error( + "Address nonce mismatch for {address}: submitted nonce {provided_nonce}, \ + Platform expected {expected_nonce}; retry the operation with the \ + expected nonce" + )] + AddressNonceMismatch { + address: PlatformAddress, + provided_nonce: AddressNonce, + expected_nonce: AddressNonce, + }, + #[error("Address sync failed: {0}")] AddressSync(String), @@ -397,3 +431,164 @@ pub fn as_asset_lock_proof_cl_height_too_low( _ => None, } } + +/// Extract the `AddressInvalidNonceError` (DPP consensus code 40603) from an +/// SDK error if Platform rejected an address-funds transition because a spent +/// address's provided nonce did not equal its expected next value. +/// +/// Returns `Some(&error)` for both `dash_sdk::Error` shapes that carry a +/// consensus verdict — `StateTransitionBroadcastError` (wait-stream rejection) +/// and `Protocol(ProtocolError::ConsensusError)` (CheckTx rejection) — +/// exposing `address()`, `provided_nonce()`, and `expected_nonce()`. Returns +/// `None` for everything else. +/// +/// The address-nonce path is optimistic by design: the client submits +/// `fetched + 1` and Platform rejects a stale/replayed value under a lagging +/// replica read. This extractor lets a caller recover `expected_nonce` and +/// retry per that contract. Recurses through +/// [`dash_sdk::Error::NoAvailableAddressesToRetry`] so it stays in lockstep +/// with its sibling `broadcast_definitely_failed`; re-audit if a future +/// `dash_sdk::Error` variant starts carrying consensus errors through yet +/// another shape. +pub fn as_address_invalid_nonce(error: &dash_sdk::Error) -> Option<&AddressInvalidNonceError> { + use dpp::consensus::state::state_error::StateError; + use dpp::consensus::ConsensusError; + + let consensus_error = match error { + dash_sdk::Error::StateTransitionBroadcastError(broadcast_err) => { + broadcast_err.cause.as_ref() + } + dash_sdk::Error::Protocol(dpp::ProtocolError::ConsensusError(ce)) => Some(ce.as_ref()), + // A consensus rejection can arrive wrapped when the dapi-client + // exhausted every address mid-retry; recurse so this predicate stays + // in lockstep with `broadcast_definitely_failed`. + dash_sdk::Error::NoAvailableAddressesToRetry(inner) => { + return as_address_invalid_nonce(inner) + } + _ => None, + }; + match consensus_error { + Some(ConsensusError::StateError(StateError::AddressInvalidNonceError(e))) => Some(e), + _ => None, + } +} + +/// Promote a nonce-rejection SDK error to the typed +/// [`PlatformWalletError::AddressNonceMismatch`] so callers can recover +/// `expected_nonce` and retry, instead of receiving the rejection flattened +/// to a string. +/// +/// Returns `None` for any error [`as_address_invalid_nonce`] does not match, +/// leaving the caller free to keep its existing fallback mapping. +pub fn promote_address_nonce_error(error: &dash_sdk::Error) -> Option { + as_address_invalid_nonce(error).map(|e| PlatformWalletError::AddressNonceMismatch { + address: *e.address(), + provided_nonce: e.provided_nonce(), + expected_nonce: e.expected_nonce(), + }) +} + +#[cfg(test)] +mod address_nonce_tests { + use super::*; + use dash_sdk::error::StateTransitionBroadcastError; + + const ADDR_BYTES: [u8; 20] = [7u8; 20]; + + /// An `AddressInvalidNonceError` wrapped as a `ConsensusError`, plus the + /// address it names, for asserting round-trip field fidelity. + fn nonce_consensus_error( + provided: AddressNonce, + expected: AddressNonce, + ) -> (PlatformAddress, dpp::consensus::ConsensusError) { + let address = PlatformAddress::P2pkh(ADDR_BYTES); + let err = AddressInvalidNonceError::new(address, provided, expected); + (address, err.into()) + } + + /// `Protocol(ConsensusError)` — the CheckTx-rejection shape. + fn protocol_shape(provided: AddressNonce, expected: AddressNonce) -> dash_sdk::Error { + let (_, cause) = nonce_consensus_error(provided, expected); + dash_sdk::Error::Protocol(dpp::ProtocolError::ConsensusError(Box::new(cause))) + } + + /// `StateTransitionBroadcastError` — the wait-stream-rejection shape. + fn broadcast_shape(provided: AddressNonce, expected: AddressNonce) -> dash_sdk::Error { + let (_, cause) = nonce_consensus_error(provided, expected); + dash_sdk::Error::StateTransitionBroadcastError(StateTransitionBroadcastError { + code: 40603, + message: "invalid address nonce".to_string(), + cause: Some(cause), + }) + } + + #[test] + fn extracts_nonce_error_from_protocol_shape() { + let err = protocol_shape(1, 2); + let got = as_address_invalid_nonce(&err).expect("protocol shape must match"); + assert_eq!(*got.address(), PlatformAddress::P2pkh(ADDR_BYTES)); + assert_eq!(got.provided_nonce(), 1); + assert_eq!(got.expected_nonce(), 2); + } + + #[test] + fn extracts_nonce_error_from_broadcast_shape() { + let err = broadcast_shape(5, 6); + let got = as_address_invalid_nonce(&err).expect("broadcast shape must match"); + assert_eq!(*got.address(), PlatformAddress::P2pkh(ADDR_BYTES)); + assert_eq!(got.provided_nonce(), 5); + assert_eq!(got.expected_nonce(), 6); + } + + #[test] + fn ignores_unrelated_and_causeless_errors() { + // A plainly unrelated SDK error. + assert!(as_address_invalid_nonce(&dash_sdk::Error::Generic("boom".to_string())).is_none()); + // The DAPI wait-timeout shape: a broadcast error with no consensus + // cause must NOT be misread as a nonce rejection. + let causeless = + dash_sdk::Error::StateTransitionBroadcastError(StateTransitionBroadcastError { + code: 0, + message: "timeout".to_string(), + cause: None, + }); + assert!(as_address_invalid_nonce(&causeless).is_none()); + } + + #[test] + fn promotes_both_shapes_to_typed_variant() { + for err in [protocol_shape(1, 2), broadcast_shape(1, 2)] { + match promote_address_nonce_error(&err) { + Some(PlatformWalletError::AddressNonceMismatch { + address, + provided_nonce, + expected_nonce, + }) => { + assert_eq!(address, PlatformAddress::P2pkh(ADDR_BYTES)); + assert_eq!(provided_nonce, 1); + assert_eq!(expected_nonce, 2); + } + other => panic!("expected AddressNonceMismatch, got {other:?}"), + } + } + } + + #[test] + fn promotion_leaves_unrelated_errors_for_the_fallback() { + assert!( + promote_address_nonce_error(&dash_sdk::Error::Generic("boom".to_string())).is_none() + ); + } + + #[test] + fn extracts_nonce_error_wrapped_in_no_available_addresses_to_retry() { + // The dapi-client wraps the last rejection in `NoAvailableAddressesToRetry` + // when every address is exhausted mid-retry; the extractor must recurse + // into it (lockstep with `broadcast_definitely_failed`). + let inner = Box::new(protocol_shape(9, 10)); + let wrapped = dash_sdk::Error::NoAvailableAddressesToRetry(inner); + let got = as_address_invalid_nonce(&wrapped).expect("must unwrap the retry envelope"); + assert_eq!(got.provided_nonce(), 9); + assert_eq!(got.expected_nonce(), 10); + } +} diff --git a/packages/rs-platform-wallet/src/events.rs b/packages/rs-platform-wallet/src/events.rs index 9ac256e8730..d24b137b76b 100644 --- a/packages/rs-platform-wallet/src/events.rs +++ b/packages/rs-platform-wallet/src/events.rs @@ -16,9 +16,11 @@ use arc_swap::ArcSwap; pub use dash_spv::EventHandler; pub use key_wallet_manager::WalletEvent; +use crate::manager::load_outcome::SkipReason; use crate::manager::platform_address_sync::PlatformAddressSyncSummary; #[cfg(feature = "shielded")] use crate::manager::shielded_sync::ShieldedSyncPassSummary; +use crate::wallet::platform_wallet::WalletId; /// Extension of [`EventHandler`] for platform-wallet consumers. /// @@ -44,6 +46,13 @@ pub trait PlatformEventHandler: EventHandler { #[cfg(feature = "shielded")] fn on_shielded_sync_completed(&self, _summary: &ShieldedSyncPassSummary) {} + /// Fired once per wallet that + /// [`load_from_persistor`](crate::PlatformWalletManager::load_from_persistor) + /// skipped because its persisted row was corrupt. + /// + /// Default impl is a no-op so existing handlers don't have to care. + fn on_wallet_skipped_on_load(&self, _wallet_id: WalletId, _reason: &SkipReason) {} + /// Fired periodically during a shielded sync pass — once per /// completed chunk inside `sync_shielded_notes`. Carries the /// cumulative count of encrypted notes scanned so far in the @@ -142,6 +151,17 @@ impl PlatformEventManager { } } + /// Dispatch a wallet-skipped-on-load notification to every handler. + /// + /// Not on the SPV hot path — called at most once per wallet during + /// a single `load_from_persistor` pass. + pub fn on_wallet_skipped_on_load(&self, wallet_id: WalletId, reason: &SkipReason) { + let handlers = self.handlers.load(); + for h in handlers.iter() { + h.on_wallet_skipped_on_load(wallet_id, reason); + } + } + /// Dispatch a shielded sync progress event to every handler. /// /// Called from inside `sync_shielded_notes`'s chunk loop, once diff --git a/packages/rs-platform-wallet/src/lib.rs b/packages/rs-platform-wallet/src/lib.rs index fb1077a1ae7..7a7d6697019 100644 --- a/packages/rs-platform-wallet/src/lib.rs +++ b/packages/rs-platform-wallet/src/lib.rs @@ -47,6 +47,7 @@ pub use manager::identity_sync::{ DEFAULT_SYNC_INTERVAL_SECS as IDENTITY_SYNC_DEFAULT_INTERVAL_SECS, MAX_TOKENS_PER_BALANCE_BATCH as IDENTITY_SYNC_MAX_TOKENS_PER_BATCH, }; +pub use manager::load_outcome::{LoadOutcome, SkipReason}; pub use manager::platform_address_sync::{ PlatformAddressSyncManager, PlatformAddressSyncSummary, WalletSyncOutcome, DEFAULT_SYNC_INTERVAL_SECS, diff --git a/packages/rs-platform-wallet/src/manager/load.rs b/packages/rs-platform-wallet/src/manager/load.rs index f355eca497a..7cfdfad2f53 100644 --- a/packages/rs-platform-wallet/src/manager/load.rs +++ b/packages/rs-platform-wallet/src/manager/load.rs @@ -3,8 +3,11 @@ use std::collections::BTreeMap; use std::sync::Arc; +use key_wallet::wallet::managed_wallet_info::ManagedWalletInfo; + use crate::changeset::{ClientStartState, ClientWalletStartState, PlatformWalletPersistence}; use crate::error::PlatformWalletError; +use crate::manager::load_outcome::{CorruptKind, LoadOutcome, SkipReason}; use crate::wallet::core::WalletBalance; use crate::wallet::identity::IdentityManager; use crate::wallet::platform_wallet::{PlatformWalletInfo, WalletId}; @@ -13,80 +16,192 @@ use crate::wallet::PlatformWallet; use super::PlatformWalletManager; impl PlatformWalletManager

{ - /// Load the full [`ClientStartState`] from the configured persister - /// and rehydrate the manager's `wallet_manager` and `wallets` maps. + /// Restore every persisted wallet as a **watch-only** entry — no + /// signing key material is derived here. The persister hands back a + /// keyless reconstruction snapshot; each wallet is rebuilt via + /// [`Wallet::new_watch_only`](key_wallet::wallet::Wallet::new_watch_only) + /// from its [`AccountRegistrationEntry`](crate::changeset::AccountRegistrationEntry) + /// manifest, the managed core state is restored, and the result is + /// registered into the manager. + /// + /// Core state arrives as a full keyless snapshot + /// ([`ClientWalletStartState::wallet_info`]) — consumed directly, + /// preserving per-account UTXO/record attribution and exact pool + /// contents — after its `wallet_id`/`network`/account-set are + /// validated against the row. + /// + /// The load path never touches the seed, so it performs no wrong-seed + /// check. Signing happens later, on demand, via the configured + /// `MnemonicResolverHandle` (`rs-sdk-ffi`). + /// + /// # Skip vs hard-fail /// - /// For each persisted wallet this builds a `PlatformWalletInfo` from - /// the snapshot (core wallet info, identity manager, tracked asset - /// locks) and inserts the `(Wallet, PlatformWalletInfo)` pair into - /// the inner [`WalletManager`]. A matching [`PlatformWallet`] handle - /// is then constructed and registered in `self.wallets`. + /// Returns a [`LoadOutcome`] describing the pass: + /// [`Loaded`](LoadOutcome::Loaded) (all wallets loaded), + /// [`Partial`](LoadOutcome::Partial) (some loaded, some skipped), or + /// [`NoneUsable`](LoadOutcome::NoneUsable) (rows present, all skipped). /// - /// If the snapshot includes platform-address provider state, each - /// per-wallet slice is handed to - /// [`PlatformAddressWallet::initialize_from_persisted`](crate::wallet::platform_addresses::PlatformAddressWallet::initialize_from_persisted); - /// wallets missing from that slice get a fresh - /// [`PlatformAddressWallet::initialize`](crate::wallet::platform_addresses::PlatformAddressWallet::initialize). + /// - **Per-row decode failure** (empty manifest, malformed xpub, + /// snapshot/row mismatch, …): the wallet is **skipped** — never + /// inserted into `wallet_manager` / `self.wallets`, recorded in the + /// outcome's skip set with a structural + /// [`SkipReason::CorruptPersistedRow`], and + /// [`on_wallet_skipped_on_load`](crate::PlatformEventHandler::on_wallet_skipped_on_load) + /// fires on each registered handler. One bad row never aborts the + /// others; the call still returns `Ok`. + /// - **Already present** (a repeat restore or a runtime-created + /// wallet, detected either at the pre-reconstruction idempotency + /// check or as `WalletExists` at insert): the wallet is **skipped** + /// with [`SkipReason::AlreadyRegistered`] and left untouched — kept + /// out of the rollback set so a later hard-fail never evicts it. A + /// second `load_from_persistor` therefore mutates no state and returns + /// `Ok(LoadOutcome)` — a repeat where every wallet is already + /// registered is a [`NoneUsable`](LoadOutcome::NoneUsable) no-op, not a + /// failure — and the caller can tell an already-present wallet from one + /// freshly loaded via [`loaded`](LoadOutcome::loaded) / + /// [`skipped`](LoadOutcome::skipped). + /// - **Whole-load failure** (persister I/O, programmer error, + /// registering a persisted wallet in `WalletManager`): + /// `Err(_)` — every wallet inserted earlier in this pass is + /// rolled back. Skipped wallets never entered the maps so the + /// rollback path never sees them. /// - /// [`WalletManager`]: key_wallet_manager::WalletManager - pub async fn load_from_persistor(&self) -> Result<(), PlatformWalletError> { + /// Platform-address provider state is restored per wallet via + /// [`initialize_from_persisted`](crate::wallet::platform_addresses::PlatformAddressWallet::initialize_from_persisted), + /// or a fresh + /// [`initialize`](crate::wallet::platform_addresses::PlatformAddressWallet::initialize) + /// when the snapshot carries no slice for it. + /// + /// # Trust boundary + /// + /// The persisted account manifest is trusted as-is — it is **not** + /// cryptographically bound to its `wallet_id` (see `build_watch_only_wallet` + /// in `rehydrate`). A corrupted or tampered store can rebuild a wallet whose + /// receive addresses derive from the wrong key under the original id; + /// authenticating the manifest on load is a tracked storage-schema follow-up. + pub async fn load_from_persistor(&self) -> Result { let ClientStartState { mut platform_addresses, wallets, + skipped: persister_skipped, // Shielded restore happens lazily on `bind_shielded`, // not here — drop the snapshot at this entry point. #[cfg(feature = "shielded")] shielded: _, - } = self.persister.load().map_err(|e| { - PlatformWalletError::WalletCreation(format!( - "Failed to load persisted client state: {}", - e - )) - })?; + } = self.persister.load()?; let persister_dyn: Arc = Arc::clone(&self.persister) as _; - // Track every wallet successfully inserted into - // `wallet_manager` and `self.wallets` during this call so the - // batch is transactional: if any later iteration fails (id - // mismatch, `initialize_from_persisted` error), we walk back - // every prior insert before bailing. Without this, a clean - // retry would collide on `WalletManager::insert_wallet` - // returning `WalletAlreadyExists` for every previously-loaded - // wallet — half-poisoning the manager until the process - // restarts. The orphan state is observable across the FFI - // boundary with no Swift-side reset path, so transactional - // semantics matter for this hydration API. + // Transactional batch: every wallet inserted into + // `wallet_manager` / `self.wallets` is tracked so a later hard + // error walks back every prior insert. Skipped wallets never + // enter either map, so the rollback path never sees them. let mut inserted_in_manager: Vec = Vec::new(); let mut inserted_in_wallets: Vec = Vec::new(); let mut load_error: Option = None; + let mut loaded: Vec = Vec::new(); + let mut skipped: Vec<(WalletId, SkipReason)> = Vec::new(); + + // Rows the persister rejected as corrupt before reconstruction + // (e.g. a malformed xpub that aborts FFI decode) never reach the + // rebuild loop below — fold them into the skip set and notify, so + // one bad persisted row never blocks the batch. + for (wallet_id, reason) in persister_skipped { + self.event_manager + .on_wallet_skipped_on_load(wallet_id, &reason); + skipped.push((wallet_id, reason)); + } 'load: for (expected_wallet_id, wallet_state) in wallets { let ClientWalletStartState { - wallet, + network, + // The carried snapshot supplies its own sync metadata, so + // the row's birth height is not needed on this path. + birth_height: _, + account_manifest, wallet_info, identity_manager, unused_asset_locks, } = wallet_state; - // Flatten the (account → outpoint → lock) map into the flat - // OutPoint → TrackedAssetLock map that `PlatformWalletInfo` - // holds today. + // Idempotency, checked FIRST: a wallet already registered (a + // prior load pass, or a runtime create) is not freshly loaded + // by this pass — report it as an `AlreadyRegistered` skip so + // the caller can tell it apart from a genuine load. Checking + // before any reconstruction work matters — the rebuild below + // derives eager gap windows (and possibly a deep discovery + // scan), all of which the `WalletExists` arm at insert time + // would only throw away. + { + let wm = self.wallet_manager.read().await; + if wm.get_wallet(&expected_wallet_id).is_some() { + let reason = SkipReason::AlreadyRegistered; + self.event_manager + .on_wallet_skipped_on_load(expected_wallet_id, &reason); + skipped.push((expected_wallet_id, reason)); + continue 'load; + } + } + + // Build the watch-only wallet from the keyless manifest. A + // structural decode failure skips this row (per-row + // resilience) — it never aborts the batch and never inserts + // a degraded placeholder. + let wallet = match super::rehydrate::build_watch_only_wallet( + network, + expected_wallet_id, + &account_manifest, + ) { + Ok(w) => w, + Err(kind) => { + let reason = SkipReason::CorruptPersistedRow { kind }; + skipped.push((expected_wallet_id, reason.clone())); + self.event_manager + .on_wallet_skipped_on_load(expected_wallet_id, &reason); + continue 'load; + } + }; + + // Full keyless snapshot carried by the persister: consume it + // directly. This preserves per-account UTXO/record attribution, + // the exact pool contents (derived-but-unused addresses stay in + // the SPV watch set), and per-index used flags. + let wallet_info = { + let mut info = *wallet_info; + // The snapshot must describe this row's wallet and its + // account set must agree with the manifest that built the + // watch-only wallet above. Either mismatch is a wrong-row + // snapshot — skipped like any structural failure, kept + // distinct from unreadable bytes. + if info.wallet_id != expected_wallet_id + || info.network != network + || !snapshot_accounts_match_manifest(&info, &account_manifest) + { + let reason = SkipReason::CorruptPersistedRow { + kind: CorruptKind::SnapshotIdentityMismatch, + }; + skipped.push((expected_wallet_id, reason.clone())); + self.event_manager + .on_wallet_skipped_on_load(expected_wallet_id, &reason); + continue 'load; + } + // Recompute totals from the carried UTXO set so the + // lock-free balance mirrored below can never drift from it + // (no-silent-zero holds by recomputation). + { + use key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; + info.update_balance(); + } + info + }; + + // Flatten the (account → outpoint → lock) map. let mut tracked_asset_locks = BTreeMap::new(); for (_account_index, account_locks) in unused_asset_locks { tracked_asset_locks.extend(account_locks); } let balance = Arc::new(WalletBalance::new()); - // Mirror the inner `ManagedWalletInfo.balance` (already - // recomputed from the freshly-loaded UTXO set on the FFI - // side via `update_balance`) into the lock-free `Arc` the - // UI reads. Without this, `wallet.balance()` reports zero - // for restored wallets even though the per-account totals - // and the inner `core_wallet.balance` are correct. - // `WalletBalance::set` is `pub(crate)`, which is why this - // step has to live inside `platform_wallet` rather than - // the FFI loader. let core_balance = &wallet_info.balance; balance.set( core_balance.confirmed(), @@ -94,21 +209,36 @@ impl PlatformWalletManager

{ core_balance.immature(), core_balance.locked(), ); + // Build the identity manager from the snapshot; public keys + // and contacts are already reconstructed into it upstream by + // the FFI persister (`build_wallet_identity_bucket`). + let identity_manager = IdentityManager::from(identity_manager); let platform_info = PlatformWalletInfo { core_wallet: wallet_info, balance: Arc::clone(&balance), - identity_manager: IdentityManager::from(identity_manager), + identity_manager, tracked_asset_locks, }; - // Insert into `wallet_manager` first so we have a wallet - // handle to validate against. Track success in - // `inserted_in_manager` so the batch-rollback at the - // bottom can unwind on any later-iteration failure. let wallet_id = { let mut wm = self.wallet_manager.write().await; match wm.insert_wallet(wallet, platform_info) { Ok(id) => id, + Err(key_wallet_manager::WalletError::WalletExists(_)) => { + // Idempotent restore, lost the insert race: a + // concurrent pass (or a runtime create) registered + // this wallet after our pre-reconstruction check. + // Re-registering must not abort the batch — record + // it as an `AlreadyRegistered` skip and continue. It + // was NOT inserted by this pass, so it stays out of + // the rollback set and a later hard-fail never evicts + // the pre-existing wallet. + let reason = SkipReason::AlreadyRegistered; + self.event_manager + .on_wallet_skipped_on_load(expected_wallet_id, &reason); + skipped.push((expected_wallet_id, reason)); + continue 'load; + } Err(e) => { load_error = Some(PlatformWalletError::WalletCreation(format!( "Failed to register persisted wallet in WalletManager: {}", @@ -120,15 +250,6 @@ impl PlatformWalletManager

{ }; inserted_in_manager.push(wallet_id); - if wallet_id != expected_wallet_id { - load_error = Some(PlatformWalletError::WalletCreation(format!( - "Persisted wallet id {} does not match recomputed id {}", - hex::encode(expected_wallet_id), - hex::encode(wallet_id) - ))); - break 'load; - } - let broadcaster = Arc::new(crate::broadcaster::SpvBroadcaster::new(Arc::clone( &self.spv_manager, ))); @@ -142,10 +263,6 @@ impl PlatformWalletManager

{ broadcaster, ); - // Initialize the platform-address provider. If the snapshot - // carried a slice for this wallet, restore it directly; - // otherwise do a fresh scan from the live wallet manager. - // Failures break to the rollback path below. if let Some(persisted) = platform_addresses.remove(&wallet_id) { if let Err(e) = platform_wallet .platform() @@ -167,13 +284,10 @@ impl PlatformWalletManager

{ wallets_guard.insert(wallet_id, platform_wallet); drop(wallets_guard); inserted_in_wallets.push(wallet_id); + loaded.push(wallet_id); } if let Some(err) = load_error { - // Walk back every wallet committed in this call so the - // manager state matches what it was before. Order: - // remove from `self.wallets` first (UI surface), then - // from the inner `wallet_manager`. if !inserted_in_wallets.is_empty() { let mut wallets_guard = self.wallets.write().await; for id in &inserted_in_wallets { @@ -195,6 +309,220 @@ impl PlatformWalletManager

{ return Err(err); } - Ok(()) + Ok(LoadOutcome::from_parts(loaded, skipped)) + } +} + +/// Whether the snapshot's account set matches the row's account manifest. +/// +/// A **self-consistency** check between two pieces of the same persisted +/// row, not an authenticity guard: the manifest is the account-set oracle +/// used to build the watch-only wallet, and a snapshot carrying a +/// different set of account types is internally inconsistent with it and +/// must not be consumed. It does not attest that the manifest itself is +/// genuine — that trust boundary lives in `build_watch_only_wallet`. +/// +/// The manifest is enumerated from `Wallet::all_accounts` (ECDSA-only: +/// carries `PlatformPayment`, omits the BLS `ProviderOperatorKeys` / +/// EdDSA `ProviderPlatformKeys`); the snapshot from +/// `ManagedWalletInfo::all_managed_accounts` (the mirror: carries the +/// BLS/EdDSA provider keys, omits `PlatformPayment`). Comparison is +/// restricted to the families both enumerations can carry so this known +/// asymmetry never rejects a legitimate snapshot. +fn snapshot_accounts_match_manifest( + info: &ManagedWalletInfo, + manifest: &[crate::changeset::AccountRegistrationEntry], +) -> bool { + use key_wallet::account::AccountType; + use std::collections::BTreeSet; + + fn comparable(t: &AccountType) -> bool { + !matches!( + t, + AccountType::ProviderOperatorKeys + | AccountType::ProviderPlatformKeys + | AccountType::PlatformPayment { .. } + ) + } + + let manifest_types: BTreeSet = manifest + .iter() + .map(|e| e.account_type) + .filter(comparable) + .collect(); + let snapshot_types: BTreeSet = info + .all_managed_accounts() + .iter() + .map(|a| a.managed_account_type().to_account_type()) + .filter(comparable) + .collect(); + manifest_types == snapshot_types +} + +#[cfg(test)] +mod rollback_tests { + //! In-crate because constructing the mid-batch hard-fail needs + //! `PerAccountPlatformAddressState` + `bimap::BiBTreeMap`, which are + //! not reachable from the external integration-test crate. + + use std::sync::Arc; + + use bimap::BiBTreeMap; + use key_wallet::wallet::initialization::WalletAccountCreationOptions; + use key_wallet::wallet::managed_wallet_info::ManagedWalletInfo; + use key_wallet::wallet::Wallet; + + use crate::changeset::{ + AccountRegistrationEntry, ClientStartState, ClientWalletStartState, PersistenceError, + PlatformWalletChangeSet, PlatformWalletPersistence, + }; + use crate::error::PlatformWalletError; + use crate::events::{EventHandler, PlatformEventHandler}; + use crate::wallet::platform_wallet::WalletId; + use crate::wallet::{PerAccountPlatformAddressState, PerWalletPlatformAddressState}; + use crate::{PlatformAddressSyncStartState, PlatformWalletManager}; + + fn slice(seed: [u8; 64]) -> (WalletId, ClientWalletStartState) { + let wallet = Wallet::from_seed_bytes( + seed, + key_wallet::Network::Testnet, + WalletAccountCreationOptions::Default, + ) + .unwrap(); + let id = wallet.compute_wallet_id(); + let account_manifest: Vec = wallet + .accounts + .all_accounts() + .into_iter() + .map(|a| AccountRegistrationEntry { + account_type: a.account_type, + account_xpub: a.account_xpub, + }) + .collect(); + let info = ManagedWalletInfo::from_wallet(&wallet, 1); + ( + id, + ClientWalletStartState { + network: key_wallet::Network::Testnet, + birth_height: 1, + account_manifest, + wallet_info: Box::new(info), + identity_manager: Default::default(), + unused_asset_locks: Default::default(), + }, + ) + } + + /// Two healthy wallets plus a platform-address restore state for the + /// second that references an account index the wallet does not have + /// as a platform-payment account, so `initialize_from_persisted` + /// hard-fails mid-batch. Rebuilt fresh on every `load` — the platform + /// address state types are not `Clone`. + struct RollbackPersister { + healthy_seed: [u8; 64], + fail_seed: [u8; 64], + } + + impl PlatformWalletPersistence for RollbackPersister { + fn store(&self, _: WalletId, _: PlatformWalletChangeSet) -> Result<(), PersistenceError> { + Ok(()) + } + fn flush(&self, _: WalletId) -> Result<(), PersistenceError> { + Ok(()) + } + fn load(&self) -> Result { + let mut st = ClientStartState::default(); + let (healthy_id, healthy) = slice(self.healthy_seed); + let (fail_id, fail) = slice(self.fail_seed); + let bogus_xpub = fail.account_manifest[0].account_xpub; + st.wallets.insert(healthy_id, healthy); + st.wallets.insert(fail_id, fail); + + // A per-account restore entry keyed to an account index the + // wallet has no platform-payment account for: `from_persisted` + // returns `AddressSync`, which the manager treats as a hard + // failure (not a per-row skip). + let mut per_account = PerWalletPlatformAddressState::new(); + per_account.insert( + 9_999, + PerAccountPlatformAddressState::from_persisted( + bogus_xpub, + BiBTreeMap::new(), + std::collections::BTreeMap::new(), + ), + ); + st.platform_addresses.insert( + fail_id, + PlatformAddressSyncStartState { + per_account, + sync_height: 0, + sync_timestamp: 0, + last_known_recent_block: 0, + }, + ); + Ok(st) + } + } + + struct NoopHandler; + impl EventHandler for NoopHandler {} + impl PlatformEventHandler for NoopHandler {} + + /// A hard failure part-way through a batch must roll back every wallet + /// already inserted this pass — from BOTH `self.wallets` and + /// `wallet_manager` — and surface as `Err`. Nothing asserted this + /// before. + #[tokio::test] + async fn hard_fail_rolls_back_already_loaded_healthy_wallet() { + // Order the seeds so the healthy wallet sorts BEFORE the failing + // one (wallets load in BTreeMap key order), guaranteeing the + // healthy wallet is fully inserted before the failure aborts. + let (id_a, _) = slice([0xA1; 64]); + let (id_b, _) = slice([0xB2; 64]); + let (healthy_seed, fail_seed) = if id_a < id_b { + ([0xA1; 64], [0xB2; 64]) + } else { + ([0xB2; 64], [0xA1; 64]) + }; + let (healthy_id, _) = slice(healthy_seed); + + let persister = Arc::new(RollbackPersister { + healthy_seed, + fail_seed, + }); + let sdk = Arc::new(dash_sdk::Sdk::new_mock()); + let handler: Arc = Arc::new(NoopHandler); + let mgr = Arc::new(PlatformWalletManager::new(sdk, persister, handler)); + + let err = mgr + .load_from_persistor() + .await + .expect_err("a mid-batch hard failure must surface as Err"); + assert!( + matches!(err, PlatformWalletError::WalletCreation(_)), + "a platform-address restore failure is a WalletCreation hard error, got {err:?}" + ); + + // The healthy wallet was fully inserted, then rolled back on the + // failure — gone from self.wallets... + assert!( + mgr.get_wallet(&healthy_id).await.is_none(), + "the already-loaded healthy wallet must be evicted from self.wallets" + ); + assert!( + mgr.wallet_ids().await.is_empty(), + "self.wallets must be fully rolled back" + ); + // ...and from wallet_manager (read via the blocking accessor). + let rows = { + let mgr = Arc::clone(&mgr); + tokio::task::spawn_blocking(move || mgr.account_balances_blocking(&healthy_id)) + .await + .unwrap() + }; + assert!( + rows.is_empty(), + "the already-loaded healthy wallet must be evicted from wallet_manager too" + ); } } diff --git a/packages/rs-platform-wallet/src/manager/load_outcome.rs b/packages/rs-platform-wallet/src/manager/load_outcome.rs new file mode 100644 index 00000000000..6c284544e72 --- /dev/null +++ b/packages/rs-platform-wallet/src/manager/load_outcome.rs @@ -0,0 +1,146 @@ +//! Aggregate result of [`load_from_persistor`]. +//! +//! [`load_from_persistor`]: super::PlatformWalletManager::load_from_persistor + +use crate::wallet::platform_wallet::WalletId; + +/// Why a persisted wallet row was passed over during a load pass. +/// +/// Load is **watch-only** (no seed material involved): signing keys are +/// derived later, on demand, via the `MnemonicResolverHandle` +/// (`rs-sdk-ffi`) sign path. A skip therefore never means a wrong or +/// unavailable seed — the load path never touches one. Either the row +/// itself was unusable ([`CorruptPersistedRow`](Self::CorruptPersistedRow), +/// a per-row decode/structural failure) or the wallet was already +/// registered before this pass reached it +/// ([`AlreadyRegistered`](Self::AlreadyRegistered)). Variants carry no +/// key material (SECRETS.md SEC-REQ-2.0.1). +#[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] +#[non_exhaustive] +pub enum SkipReason { + /// The persisted row could not be reconstructed: a structural decode + /// failure on the keyless account manifest or carried snapshot. + /// `kind` distinguishes the failure mode without leaking row bytes. + #[error("persisted wallet row corrupt: {kind}")] + CorruptPersistedRow { + /// Structural family of the decode/projection failure. + kind: CorruptKind, + }, + /// The wallet was already registered before this load pass reached it + /// (a prior load, or a wallet created at runtime), so its persisted + /// row was not freshly loaded. Not corruption — it lets the caller + /// tell an already-present wallet apart from one that genuinely loaded + /// this pass. + #[error("wallet already registered before this load pass")] + AlreadyRegistered, +} + +/// Structural family of [`SkipReason::CorruptPersistedRow`]. +/// +/// The variants are deliberately coarse — a finer split would require +/// the persister to round-trip backend error context that may carry +/// row-derived bytes. Apps drive their UI from the *family*, not from +/// the inner message. +#[derive(Debug, Clone, PartialEq, Eq)] +#[non_exhaustive] +pub enum CorruptKind { + /// The wallet row exists but has no usable `AccountRegistrationEntry` + /// manifest to rebuild the account collection from. + MissingManifest, + /// One or more manifest `account_xpub` bytes failed to parse as a + /// well-formed extended public key. + MalformedXpub, + /// The carried [`ManagedWalletInfo`] snapshot does not describe the + /// persisted row it is attached to: its `wallet_id`/`network` differ + /// from the row, or its account set diverges from the row's account + /// manifest. This is a wrong-row/structurally-inconsistent snapshot — + /// distinct from unreadable bytes ([`Self::DecodeError`]). + /// + /// [`ManagedWalletInfo`]: key_wallet::wallet::managed_wallet_info::ManagedWalletInfo + SnapshotIdentityMismatch, + /// Any other structural decode / projection failure surfaced by the + /// persister. The string is a structural projection — never a raw + /// row byte slice or a hex-encoded key. + DecodeError(String), +} + +impl std::fmt::Display for CorruptKind { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::MissingManifest => f.write_str("missing account manifest"), + Self::MalformedXpub => f.write_str("malformed account xpub"), + Self::SnapshotIdentityMismatch => { + f.write_str("snapshot does not match its persisted row") + } + Self::DecodeError(s) => write!(f, "decode error: {s}"), + } + } +} + +/// Aggregate, synchronous view of one +/// [`load_from_persistor`](super::PlatformWalletManager::load_from_persistor) +/// pass that did not hard-fail. +/// +/// Three states, so the caller can tell a clean load from one that left +/// wallets behind without conflating either with a whole-load `Err` +/// (persister I/O, programmer error — that stays the `Err` arm of the +/// call). The load path is watch-only and never touches the seed, so no +/// wrong-seed outcome appears here. +/// +/// Inspect the outcome via [`loaded`](Self::loaded) / [`skipped`](Self::skipped): +/// the skip reasons are what separate a harmless repeat +/// ([`AlreadyRegistered`](SkipReason::AlreadyRegistered)) from genuine +/// corruption ([`CorruptPersistedRow`](SkipReason::CorruptPersistedRow)) — a +/// distinction a flat `Result` shape would erase. +#[derive(Debug, Clone, PartialEq, Eq)] +#[non_exhaustive] +pub enum LoadOutcome { + /// Full success: every persisted wallet was reconstructed and + /// registered. Also the empty-store first run — `loaded` is then + /// empty and nothing was skipped. + Loaded { + /// Wallets reconstructed and registered, in load order. + loaded: Vec, + }, + /// Some wallets loaded, at least one was skipped (a corrupt row, or + /// one already registered). The batch did not abort. + Partial { + /// Wallets reconstructed and registered, in load order. + loaded: Vec, + /// Wallets skipped, in load order. + skipped: Vec<(WalletId, SkipReason)>, + }, + /// Nothing usable: the persister returned rows but every one was + /// skipped, so no wallet loaded even though the persister succeeded. + NoneUsable { + /// Wallets skipped, in load order. + skipped: Vec<(WalletId, SkipReason)>, + }, +} + +impl LoadOutcome { + /// Pick the variant matching the `(loaded, skipped)` tallies of a pass. + pub(crate) fn from_parts(loaded: Vec, skipped: Vec<(WalletId, SkipReason)>) -> Self { + match (loaded.is_empty(), skipped.is_empty()) { + (_, true) => Self::Loaded { loaded }, + (true, false) => Self::NoneUsable { skipped }, + (false, false) => Self::Partial { loaded, skipped }, + } + } + + /// Wallets reconstructed and registered this pass, in load order. + pub fn loaded(&self) -> &[WalletId] { + match self { + Self::Loaded { loaded } | Self::Partial { loaded, .. } => loaded, + Self::NoneUsable { .. } => &[], + } + } + + /// Wallets skipped this pass (corrupt row, or already registered). + pub fn skipped(&self) -> &[(WalletId, SkipReason)] { + match self { + Self::Partial { skipped, .. } | Self::NoneUsable { skipped } => skipped, + Self::Loaded { .. } => &[], + } + } +} diff --git a/packages/rs-platform-wallet/src/manager/mod.rs b/packages/rs-platform-wallet/src/manager/mod.rs index 7962d6551f1..9e9ca6cfa24 100644 --- a/packages/rs-platform-wallet/src/manager/mod.rs +++ b/packages/rs-platform-wallet/src/manager/mod.rs @@ -4,8 +4,10 @@ pub mod accessors; pub mod dashpay_sync; pub mod identity_sync; mod load; +pub mod load_outcome; mod loop_cancel; pub mod platform_address_sync; +mod rehydrate; #[cfg(feature = "shielded")] pub mod shielded_sync; mod wallet_lifecycle; @@ -84,14 +86,19 @@ pub struct PlatformWalletManager { #[cfg(feature = "shielded")] pub(super) shielded_coordinator: Arc>>>, - /// Shared `PlatformEventManager` — held on the manager so - /// `configure_shielded` can install a per-chunk progress handler - /// onto the freshly-created `NetworkShieldedCoordinator` that - /// forwards into `on_shielded_sync_progress`. Sub-managers + /// Shared `PlatformEventManager`, retained on the manager for the + /// two callers that fan out platform-wallet events directly: + /// `load_from_persistor` surfaces per-wallet wallet-skipped-on-load + /// notifications to the app handler via + /// [`on_wallet_skipped_on_load`](crate::PlatformEventHandler::on_wallet_skipped_on_load), + /// and (under the `shielded` + /// feature) `configure_shielded` installs a per-chunk progress + /// handler onto the freshly-created `NetworkShieldedCoordinator` + /// that forwards into `on_shielded_sync_progress`. Sub-managers /// (`SpvRuntime`, `PlatformAddressSyncManager`, etc.) hold their - /// own clones already, so `configure_shielded` is the only reader of - /// this retained handle — hence it is `shielded`-gated. - #[cfg(feature = "shielded")] + /// own clones already. Retained unconditionally because + /// `load_from_persistor` reads it regardless of the `shielded` + /// feature. pub(super) event_manager: Arc, pub(super) persister: Arc

, /// Cancellation token + join handle for the wallet-event adapter @@ -187,7 +194,6 @@ impl PlatformWalletManager

{ shielded_sync_manager: shielded_sync, #[cfg(feature = "shielded")] shielded_coordinator, - #[cfg(feature = "shielded")] event_manager, persister, event_adapter_cancel, diff --git a/packages/rs-platform-wallet/src/manager/rehydrate.rs b/packages/rs-platform-wallet/src/manager/rehydrate.rs new file mode 100644 index 00000000000..578710d3dca --- /dev/null +++ b/packages/rs-platform-wallet/src/manager/rehydrate.rs @@ -0,0 +1,110 @@ +//! Watch-only wallet reconstruction from the keyless account manifest. +//! +//! Load is seedless — each wallet is rebuilt watch-only from its manifest and +//! the manager consumes the carried snapshot directly, so no wrong-seed check +//! runs here; that gate lives in the resolver-backed signing entrypoints. + +use key_wallet::account::account_collection::AccountCollection; +use key_wallet::account::Account; +use key_wallet::wallet::Wallet; +use key_wallet::Network; + +use crate::changeset::AccountRegistrationEntry; +use crate::manager::load_outcome::CorruptKind; + +/// Build a watch-only [`Wallet`] from the keyless account manifest, stamping +/// `expected_wallet_id` onto the reconstructed [`AccountCollection`]. Returns +/// [`CorruptKind`] when the row is structurally unusable; no key material +/// crosses this function. +/// +/// # Trust boundary +/// +/// `expected_wallet_id` is **not** cryptographically bound to the manifest: the +/// id hashes the *root* xpub, but only account-level xpubs are persisted, so the +/// root cannot be recovered here to re-verify it. A well-formed but **wrong** +/// `account_xpub` is therefore accepted — anyone able to write the backing store +/// can swap in their own xpub under the unchanged id and silently redirect +/// incoming funds. Closing this needs storage-layer manifest authentication (a +/// MAC over `{wallet_id, network, manifest}`, verified fail-closed on load), +/// tracked in the `platform-wallet-storage` crate. +pub(super) fn build_watch_only_wallet( + network: Network, + expected_wallet_id: [u8; 32], + manifest: &[AccountRegistrationEntry], +) -> Result { + if manifest.is_empty() { + return Err(CorruptKind::MissingManifest); + } + let mut accounts = AccountCollection::new(); + for entry in manifest { + // `Account::from_xpub` is infallible in the pinned key-wallet rev; this + // map_err is a defensive guard for when that signature becomes fallible. + let account = Account::from_xpub( + Some(expected_wallet_id), + entry.account_type, + entry.account_xpub, + network, + ) + .map_err(|_| CorruptKind::MalformedXpub)?; + accounts + .insert(account) + .map_err(|e| CorruptKind::DecodeError(e.to_string()))?; + } + Ok(Wallet::new_watch_only( + network, + expected_wallet_id, + accounts, + )) +} + +#[cfg(test)] +mod tests { + use super::*; + use key_wallet::wallet::initialization::WalletAccountCreationOptions; + + fn manifest_for(w: &Wallet) -> Vec { + w.accounts + .all_accounts() + .into_iter() + .map(|a| AccountRegistrationEntry { + account_type: a.account_type, + account_xpub: a.account_xpub, + }) + .collect() + } + + #[test] + fn watch_only_rebuild_round_trips_manifest_and_id() { + let seed = [3u8; 64]; + let w = Wallet::from_seed_bytes( + seed, + Network::Testnet, + WalletAccountCreationOptions::Default, + ) + .unwrap(); + let id = w.compute_wallet_id(); + let manifest = manifest_for(&w); + + let restored = build_watch_only_wallet(Network::Testnet, id, &manifest).unwrap(); + assert_eq!(restored.wallet_id, id); + assert_eq!(restored.compute_wallet_id(), id); + let restored_types: Vec<_> = restored + .accounts + .all_accounts() + .into_iter() + .map(|a| a.account_type) + .collect(); + let manifest_types: Vec<_> = manifest.iter().map(|e| e.account_type).collect(); + assert_eq!(restored_types.len(), manifest_types.len()); + for t in &manifest_types { + assert!(restored_types.contains(t)); + } + } + + #[test] + fn empty_manifest_is_missing_manifest() { + let err = build_watch_only_wallet(Network::Testnet, [0u8; 32], &[]) + .expect_err("empty manifest must be MissingManifest"); + assert!(matches!(err, CorruptKind::MissingManifest)); + } +} diff --git a/packages/rs-platform-wallet/src/manager/wallet_lifecycle.rs b/packages/rs-platform-wallet/src/manager/wallet_lifecycle.rs index e8a38200715..c6fb50b8e23 100644 --- a/packages/rs-platform-wallet/src/manager/wallet_lifecycle.rs +++ b/packages/rs-platform-wallet/src/manager/wallet_lifecycle.rs @@ -375,6 +375,7 @@ impl PlatformWalletManager

{ let crate::changeset::ClientStartState { mut platform_addresses, wallets: _, + skipped: _, #[cfg(feature = "shielded")] shielded: _, } = match platform_wallet.load_persisted() { diff --git a/packages/rs-platform-wallet/src/test_support.rs b/packages/rs-platform-wallet/src/test_support.rs index 51b76637af9..bfaa4006f58 100644 --- a/packages/rs-platform-wallet/src/test_support.rs +++ b/packages/rs-platform-wallet/src/test_support.rs @@ -156,11 +156,12 @@ pub(crate) async fn funded_wallet_manager( } }; + // Funded as a chain-locked (confirmed) UTXO, not mempool: since + // rust-dashcore#836, asset-lock building excludes unconfirmed/non- + // InstantSend inputs (`require_final_inputs`), so a mempool-only fixture + // would starve `build_asset_lock`'s coin selection before ever reaching + // the broadcast-rejection paths these tests exist to exercise. let funding_tx = Transaction::dummy(&receive_address, 0..1, &[10_000_000]); - // Chain-locked funding, not `Mempool`: asset-lock builders only - // select final (confirmed / InstantSend-locked) inputs since - // rust-dashcore#836, so a mempool-funded fixture leaves the - // asset-lock tests with no eligible UTXO. let result = ctx .check_transaction( &funding_tx, diff --git a/packages/rs-platform-wallet/src/wallet/apply.rs b/packages/rs-platform-wallet/src/wallet/apply.rs index 6100e19d57c..a2f82d6779c 100644 --- a/packages/rs-platform-wallet/src/wallet/apply.rs +++ b/packages/rs-platform-wallet/src/wallet/apply.rs @@ -154,114 +154,17 @@ impl PlatformWalletInfo { } } - // 2b. Identity keys. Runs after the scalar identity pass so - // the owning ManagedIdentity is guaranteed to exist before - // we layer keys into it. Upserts land first, then removals, - // matching the discipline used across the rest of this - // function. Orphan entries (owner not in the wallet) are - // logged and skipped by the per-entry apply helpers. - if let Some(keys_cs) = identity_keys { - let crate::changeset::IdentityKeysChangeSet { upserts, removed } = keys_cs; - // Thread the wallet network through so the key-apply - // path can reproduce DIP-9 derivation paths for any - // entry that carries `(wallet_id, derivation_indices)`. - let network = wallet.network; - for (_key, entry) in upserts { - self.identity_manager - .apply_identity_key_entry(entry, network); - } - for (identity_id, key_id) in removed { - self.identity_manager - .apply_identity_key_removal(&identity_id, key_id); - } - } - - // 3. Contacts. Each entry routes to its owning ManagedIdentity by - // `(owner, contact)` key; orphans (owner not in the wallet) - // are logged and skipped. Every map mutation goes through the - // `apply_*` replay methods — the relationship maps are sealed - // to the state layer, and the replay methods reproduce - // persisted state without re-running the live invariants - // (`apply_established_contact` additionally drops both - // pending sides per the contract). - if let Some(contact_cs) = contacts { - let crate::changeset::ContactChangeSet { - sent_requests, - removed_sent, - incoming_requests, - removed_incoming, - established, - ignored, - unignored, - } = contact_cs; - - for (key, entry) in sent_requests { - match self.identity_manager.managed_identity_mut(&key.owner_id) { - Some(managed) => { - managed.apply_sent_contact_request(entry.request); - } - None => tracing::warn!( - owner = %key.owner_id, - "skipping sent contact request during apply: owner identity not in wallet" - ), - } - } - for (key, entry) in incoming_requests { - match self.identity_manager.managed_identity_mut(&key.owner_id) { - Some(managed) => { - managed.apply_incoming_contact_request(entry.request); - } - None => tracing::warn!( - owner = %key.owner_id, - "skipping incoming contact request during apply: owner identity not in wallet" - ), - } - } - for key in removed_sent { - if let Some(managed) = self.identity_manager.managed_identity_mut(&key.owner_id) { - managed.apply_removed_sent(&key.recipient_id); - } - } - for key in removed_incoming { - if let Some(managed) = self.identity_manager.managed_identity_mut(&key.owner_id) { - managed.apply_removed_incoming(&key.sender_id); - } - } - // Established promotions — drop any matching pending - // entries on both sides per the auto-establishment contract. - for (key, established) in established { - match self.identity_manager.managed_identity_mut(&key.owner_id) { - Some(managed) => { - managed.apply_established_contact(established); - } - None => tracing::warn!( - owner = %key.owner_id, - "skipping established contact during apply: owner identity not in wallet" - ), - } - } - // Ignored senders (per-sender mute, local-only). Restore the - // in-memory suppression set so the sync ingest path won't - // resurrect an ignored sender's requests after a restart. - // `unignored` is applied AFTER `ignored` so an un-ignore in the - // same delta wins (the sender ends up not ignored). Orphan - // owners are logged and skipped. - for (owner_id, sender_id) in ignored { - match self.identity_manager.managed_identity_mut(&owner_id) { - Some(managed) => { - managed.apply_ignored_sender(sender_id); - } - None => tracing::warn!( - owner = %owner_id, - "skipping ignored sender during apply: owner identity not in wallet" - ), - } - } - for (owner_id, sender_id) in unignored { - if let Some(managed) = self.identity_manager.managed_identity_mut(&owner_id) { - managed.apply_unignored_sender(&sender_id); - } - } + // 2b/3. Identity keys + contacts. Keys are layered before + // contacts so a contact entry never lands before its + // owner's keys; orphans are logged and skipped. Single + // source of truth shared with the persister rehydration + // path (`load_from_persistor`). + if identity_keys.is_some() || contacts.is_some() { + self.identity_manager.apply_contacts_and_keys( + contacts.unwrap_or_default(), + identity_keys.unwrap_or_default(), + wallet.network, + ); } // 3b. DashPay profile/payment overlays. Applied AFTER identities @@ -797,6 +700,56 @@ mod tests { } } + /// TC-3692-025: a `ContactChangeSet` carrying `ignored` / `unignored` + /// replayed through `apply_changeset` must reach the owner's + /// ignored-sender set. Guards against `apply_contacts_and_keys` + /// destructuring `contacts` with `..` — which compiles clean but + /// silently drops per-sender mute state (dev-plan risk #1). + #[test] + fn apply_contacts_threads_ignored_and_unignored_through() { + let mut wallet = build_test_wallet(); + let mut info = empty_info(&wallet); + + // Register the owning identity so contact routing has a target. + let owner = Identifier::from([1u8; 32]); + let sender = Identifier::from([2u8; 32]); + let mut managed = ManagedIdentity::new(make_test_identity(1, 0), 0); + managed.wallet_id = Some([9u8; 32]); + let mut id_cs = IdentityChangeSet::default(); + id_cs + .identities + .insert(owner, IdentityEntry::from_managed(&managed)); + let mut cs = PlatformWalletChangeSet::default(); + cs.identities = Some(id_cs); + info.apply_changeset(&mut wallet, cs).expect("seed owner"); + + // Replay an `ignored` delta — the sender must land in the mute set. + let mut contact_cs = ContactChangeSet::default(); + contact_cs.ignored.insert((owner, sender)); + info.apply_changeset(&mut wallet, wrap_contacts(contact_cs)) + .expect("apply ignored"); + assert!(info + .identity_manager + .managed_identity(&owner) + .expect("owner present") + .dashpay() + .ignored_senders() + .contains(&sender)); + + // Replay an `unignored` delta for the same pair — it must clear. + let mut contact_cs = ContactChangeSet::default(); + contact_cs.unignored.insert((owner, sender)); + info.apply_changeset(&mut wallet, wrap_contacts(contact_cs)) + .expect("apply unignored"); + assert!(!info + .identity_manager + .managed_identity(&owner) + .expect("owner present") + .dashpay() + .ignored_senders() + .contains(&sender)); + } + /// Wallet id used by every wallet-owned round-trip test below. /// Same value on A and B so the manager's two-bucket lookup hits /// the same slot on both sides. diff --git a/packages/rs-platform-wallet/src/wallet/identity/network/top_up_from_addresses.rs b/packages/rs-platform-wallet/src/wallet/identity/network/top_up_from_addresses.rs index da15c81cd8d..35f79e8ac37 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/network/top_up_from_addresses.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/network/top_up_from_addresses.rs @@ -76,10 +76,12 @@ impl IdentityWallet { .top_up_from_addresses(&self.sdk, inputs, address_signer, settings) .await .map_err(|e| { - PlatformWalletError::InvalidIdentityData(format!( - "Failed to top up identity from addresses: {}", - e - )) + crate::error::promote_address_nonce_error(&e).unwrap_or_else(|| { + PlatformWalletError::InvalidIdentityData(format!( + "Failed to top up identity from addresses: {}", + e + )) + }) })?; // Update the identity's balance in the local manager and diff --git a/packages/rs-platform-wallet/src/wallet/identity/state/manager/apply.rs b/packages/rs-platform-wallet/src/wallet/identity/state/manager/apply.rs index 35c95eca81d..705f1863bd4 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/state/manager/apply.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/state/manager/apply.rs @@ -15,7 +15,7 @@ //! [`IdentityManager::apply_identity_key_entry`]. use super::{IdentityLocation, IdentityManager}; -use crate::changeset::{IdentityEntry, IdentityKeyEntry}; +use crate::changeset::{ContactChangeSet, IdentityEntry, IdentityKeyEntry, IdentityKeysChangeSet}; use crate::wallet::identity::state::managed_identity::ManagedIdentity; use dpp::identity::accessors::IdentityGettersV0; use dpp::identity::identity_public_key::accessors::v0::IdentityPublicKeyGettersV0; @@ -203,4 +203,91 @@ impl IdentityManager { managed.identity.public_keys_mut().remove(&key_id); } } + + /// Layer a [`ContactChangeSet`] + [`IdentityKeysChangeSet`] onto the + /// already-restored managed identities, for the runtime + /// changeset-replay path + /// ([`apply_changeset`](crate::wallet::PlatformWalletInfo::apply_changeset)). + /// Identity keys are applied first so a contact entry never lands + /// before its owner's keys; orphan entries (owner not in the + /// wallet) are logged and skipped, never fatal. `removed_*` and + /// `ignored`/`unignored` are honoured. + pub(crate) fn apply_contacts_and_keys( + &mut self, + contacts: ContactChangeSet, + identity_keys: IdentityKeysChangeSet, + network: Network, + ) { + let IdentityKeysChangeSet { upserts, removed } = identity_keys; + for (_key, entry) in upserts { + self.apply_identity_key_entry(entry, network); + } + for (identity_id, key_id) in removed { + self.apply_identity_key_removal(&identity_id, key_id); + } + + let ContactChangeSet { + sent_requests, + removed_sent, + incoming_requests, + removed_incoming, + established, + ignored, + unignored, + } = contacts; + for (key, entry) in sent_requests { + match self.managed_identity_mut(&key.owner_id) { + Some(managed) => managed.apply_sent_contact_request(entry.request), + None => tracing::warn!( + owner = %key.owner_id, + "skipping sent contact request: owner identity not in wallet" + ), + } + } + for (key, entry) in incoming_requests { + match self.managed_identity_mut(&key.owner_id) { + Some(managed) => managed.apply_incoming_contact_request(entry.request), + None => tracing::warn!( + owner = %key.owner_id, + "skipping incoming contact request: owner identity not in wallet" + ), + } + } + for key in removed_sent { + if let Some(managed) = self.managed_identity_mut(&key.owner_id) { + managed.apply_removed_sent(&key.recipient_id); + } + } + for key in removed_incoming { + if let Some(managed) = self.managed_identity_mut(&key.owner_id) { + managed.apply_removed_incoming(&key.sender_id); + } + } + for (key, established) in established { + match self.managed_identity_mut(&key.owner_id) { + Some(managed) => managed.apply_established_contact(established), + None => tracing::warn!( + owner = %key.owner_id, + "skipping established contact: owner identity not in wallet" + ), + } + } + // `ignored` is applied before `unignored` so a same-delta + // un-ignore wins (last-write-wins). Orphan owners are logged + // and skipped; un-ignore of an absent owner is a silent no-op. + for (owner_id, sender_id) in ignored { + match self.managed_identity_mut(&owner_id) { + Some(managed) => managed.apply_ignored_sender(sender_id), + None => tracing::warn!( + owner = %owner_id, + "skipping ignored sender: owner identity not in wallet" + ), + } + } + for (owner_id, sender_id) in unignored { + if let Some(managed) = self.managed_identity_mut(&owner_id) { + managed.apply_unignored_sender(&sender_id); + } + } + } } diff --git a/packages/rs-platform-wallet/src/wallet/platform_wallet.rs b/packages/rs-platform-wallet/src/wallet/platform_wallet.rs index 3b1e1ab2740..2e12993f4c5 100644 --- a/packages/rs-platform-wallet/src/wallet/platform_wallet.rs +++ b/packages/rs-platform-wallet/src/wallet/platform_wallet.rs @@ -1195,6 +1195,7 @@ impl PlatformWallet { let ClientStartState { mut platform_addresses, wallets: _, + skipped: _, #[cfg(feature = "shielded")] shielded: _, } = self.load_persisted()?; diff --git a/packages/rs-platform-wallet/src/wallet/shielded/operations.rs b/packages/rs-platform-wallet/src/wallet/shielded/operations.rs index 9fe357f431e..0c0b6020d41 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/operations.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/operations.rs @@ -556,7 +556,8 @@ pub async fn shield, P: OrchardPr format_addresses_with_info(rich.addresses_with_info(), network), )) } else { - PlatformWalletError::ShieldedBroadcastFailed(e.to_string()) + crate::error::promote_address_nonce_error(e) + .unwrap_or_else(|| PlatformWalletError::ShieldedBroadcastFailed(e.to_string())) } }; @@ -2335,7 +2336,8 @@ async fn broadcast_shielded_spend( match state_transition.broadcast(sdk, None).await { Ok(()) => {} Err(e) if broadcast_definitely_failed(&e) => { - return Err(PlatformWalletError::ShieldedBroadcastFailed(e.to_string())); + return Err(crate::error::promote_address_nonce_error(&e) + .unwrap_or_else(|| PlatformWalletError::ShieldedBroadcastFailed(e.to_string()))); } Err(e) => { warn!( @@ -2443,7 +2445,8 @@ fn classify_spend_wait_failure( wait_err: &dash_sdk::Error, ) -> PlatformWalletError { if carries_consensus_rejection(wait_err) { - PlatformWalletError::ShieldedBroadcastFailed(wait_err.to_string()) + crate::error::promote_address_nonce_error(wait_err) + .unwrap_or_else(|| PlatformWalletError::ShieldedBroadcastFailed(wait_err.to_string())) } else { warn!( operation, diff --git a/packages/rs-platform-wallet/tests/rehydration_load.rs b/packages/rs-platform-wallet/tests/rehydration_load.rs new file mode 100644 index 00000000000..000d17a31e5 --- /dev/null +++ b/packages/rs-platform-wallet/tests/rehydration_load.rs @@ -0,0 +1,1004 @@ +//! End-to-end coverage of `PlatformWalletManager::load_from_persistor`, the +//! seedless / watch-only load path, through a real `PlatformWalletManager`. +//! +//! Load reconstructs every persisted wallet **watch-only** from its keyless +//! account manifest. The load path never touches the seed, so it performs no +//! wrong-seed check; wrong-seed validation lives in the resolver-backed signing +//! entrypoints, not here. Per-row decode failures surface as +//! [`SkipReason::CorruptPersistedRow`] without aborting the batch. +//! +//! RT cases here: +//! - RT-WO: round-trip — watch-only wallet is registered after reload. +//! - RT-Corrupt: a row with an empty manifest is skipped with +//! `MissingManifest`, the other row loads, `on_wallet_skipped_on_load` +//! fires on the registered handler, `load` returns `Ok`. +//! - RT-Z: no key/seed material in any `LoadOutcome` / `SkipReason` +//! surface (the structural-only contract). +//! - RT-Snapshot: a carried `wallet_info` snapshot is consumed +//! verbatim — per-account UTXO attribution and derived-but-unused +//! deep pool addresses survive the reload; a snapshot whose +//! `wallet_id` mismatches its row is skipped as corrupt. + +use std::collections::BTreeMap; +use std::sync::{Arc, Mutex}; + +use dpp::prelude::Identifier; +use key_wallet::wallet::initialization::WalletAccountCreationOptions; +use key_wallet::wallet::Wallet; +use platform_wallet::changeset::{ + AccountRegistrationEntry, ClientStartState, ClientWalletStartState, IdentityManagerStartState, + PersistenceError, PersistenceErrorKind, PlatformWalletChangeSet, PlatformWalletPersistence, +}; +use platform_wallet::error::PlatformWalletError; +use platform_wallet::events::{EventHandler, PlatformEventHandler}; +use platform_wallet::manager::load_outcome::CorruptKind; +use platform_wallet::wallet::platform_wallet::WalletId; +use platform_wallet::ManagedIdentity; +use platform_wallet::{LoadOutcome, PlatformWalletManager, SkipReason}; + +// ---- test doubles ---- + +/// Persister whose `load()` returns a fixed keyless `ClientStartState`. +struct FixedLoadPersister { + state: Mutex>, +} + +impl FixedLoadPersister { + fn new() -> Self { + Self { + state: Mutex::new(None), + } + } + fn set(&self, s: ClientStartState) { + *self.state.lock().unwrap() = Some(s); + } +} + +impl PlatformWalletPersistence for FixedLoadPersister { + fn store(&self, _: WalletId, _: PlatformWalletChangeSet) -> Result<(), PersistenceError> { + Ok(()) + } + fn flush(&self, _: WalletId) -> Result<(), PersistenceError> { + Ok(()) + } + fn load(&self) -> Result { + // Rebuild a fresh ClientStartState each call (load may be + // called twice for the recoverability sub-case). + let guard = self.state.lock().unwrap(); + match guard.as_ref() { + None => Ok(ClientStartState::default()), + Some(s) => { + let mut out = ClientStartState::default(); + for (id, w) in &s.wallets { + out.wallets.insert( + *id, + ClientWalletStartState { + network: w.network, + birth_height: w.birth_height, + account_manifest: w.account_manifest.clone(), + wallet_info: w.wallet_info.clone(), + identity_manager: Default::default(), + unused_asset_locks: Default::default(), + }, + ); + } + out.skipped = s.skipped.clone(); + Ok(out) + } + } + } +} + +/// Persister whose `load()` always fails with a chosen [`PersistenceError`], +/// to exercise the typed error propagation out of `load_from_persistor`. +struct FailingLoadPersister { + transient: bool, +} + +impl PlatformWalletPersistence for FailingLoadPersister { + fn store(&self, _: WalletId, _: PlatformWalletChangeSet) -> Result<(), PersistenceError> { + Ok(()) + } + fn flush(&self, _: WalletId) -> Result<(), PersistenceError> { + Ok(()) + } + fn load(&self) -> Result { + if self.transient { + Err(PersistenceError::backend_with_kind( + PersistenceErrorKind::Transient, + "backend busy", + )) + } else { + Err(PersistenceError::backend("schema corrupt")) + } + } +} + +/// Event handler that records every wallet-skipped-on-load notification. +#[derive(Default)] +struct RecordingHandler { + skipped: Mutex>, +} +impl EventHandler for RecordingHandler {} +impl PlatformEventHandler for RecordingHandler { + fn on_wallet_skipped_on_load(&self, wallet_id: WalletId, reason: &SkipReason) { + self.skipped + .lock() + .unwrap() + .push((wallet_id, reason.clone())); + } +} + +// ---- harness ---- + +fn manifest_and_id(seed: [u8; 64]) -> (Vec, [u8; 32]) { + let w = Wallet::from_seed_bytes( + seed, + key_wallet::Network::Testnet, + WalletAccountCreationOptions::Default, + ) + .unwrap(); + let manifest = w + .accounts + .all_accounts() + .into_iter() + .map(|a| AccountRegistrationEntry { + account_type: a.account_type, + account_xpub: a.account_xpub, + }) + .collect(); + (manifest, w.compute_wallet_id()) +} + +fn slice(seed: [u8; 64]) -> (WalletId, ClientWalletStartState) { + let wallet = Wallet::from_seed_bytes( + seed, + key_wallet::Network::Testnet, + WalletAccountCreationOptions::Default, + ) + .unwrap(); + let id = wallet.compute_wallet_id(); + let account_manifest = wallet + .accounts + .all_accounts() + .into_iter() + .map(|a| AccountRegistrationEntry { + account_type: a.account_type, + account_xpub: a.account_xpub, + }) + .collect(); + let info = key_wallet::wallet::managed_wallet_info::ManagedWalletInfo::from_wallet(&wallet, 1); + ( + id, + ClientWalletStartState { + network: key_wallet::Network::Testnet, + birth_height: 1, + account_manifest, + wallet_info: Box::new(info), + identity_manager: Default::default(), + unused_asset_locks: Default::default(), + }, + ) +} + +async fn manager( + persister: Arc, + handler: Arc, +) -> Arc> { + let sdk = Arc::new(dash_sdk::Sdk::new_mock()); + Arc::new(PlatformWalletManager::new(sdk, persister, handler)) +} + +// ---- tests ---- + +/// RT-WO: seedless watch-only round-trip — a persisted wallet loads and +/// is registered after reload (no signing material needed). +#[tokio::test] +async fn rt_wo_watch_only_roundtrip() { + let seed = [0x11; 64]; + let p = Arc::new(FixedLoadPersister::new()); + let h = Arc::new(RecordingHandler::default()); + let (id, s) = slice(seed); + let mut st = ClientStartState::default(); + st.wallets.insert(id, s); + p.set(st); + + let mgr = manager(Arc::clone(&p), Arc::clone(&h)).await; + let outcome = mgr.load_from_persistor().await.expect("Ok"); + + assert_eq!(outcome.loaded(), vec![id].as_slice()); + assert!(outcome.skipped().is_empty()); + assert!( + mgr.get_wallet(&id).await.is_some(), + "watch-only restored wallet must be registered" + ); + assert_eq!(mgr.wallet_ids().await, vec![id]); +} + +/// RT-Idem: a second `load_from_persistor` with the wallet already +/// registered (a repeat restore, or a wallet created at runtime) must be +/// idempotent — never a hard error. The already-present wallet is +/// reported as an `AlreadyRegistered` skip (not a fresh load), so the +/// caller can tell it apart from one this pass genuinely loaded. +#[tokio::test] +async fn rt_idempotent_repeat_restore() { + let seed = [0x55; 64]; + let p = Arc::new(FixedLoadPersister::new()); + let h = Arc::new(RecordingHandler::default()); + let (id, s) = slice(seed); + let mut st = ClientStartState::default(); + st.wallets.insert(id, s); + p.set(st); + + let mgr = manager(Arc::clone(&p), Arc::clone(&h)).await; + + let first = mgr.load_from_persistor().await.expect("first load Ok"); + assert_eq!(first.loaded(), vec![id].as_slice()); + assert!(first.skipped().is_empty()); + assert!( + matches!(first, LoadOutcome::Loaded { .. }), + "a clean first load is the Loaded variant" + ); + + // Second load: the wallet is already registered. Must NOT hard-error; + // the row is reported as an AlreadyRegistered skip, not freshly loaded. + let second = mgr + .load_from_persistor() + .await + .expect("repeat load must be idempotent, not a hard error"); + assert!( + !second.loaded().contains(&id), + "an already-registered wallet is not reported as freshly loaded" + ); + assert_eq!( + second.skipped(), + [(id, SkipReason::AlreadyRegistered)].as_slice(), + "the already-present wallet surfaces as an AlreadyRegistered skip" + ); + assert!( + matches!(second, LoadOutcome::NoneUsable { .. }), + "nothing freshly loaded on the repeat pass" + ); + assert!( + mgr.get_wallet(&id).await.is_some(), + "wallet still present after the repeat load" + ); + assert_eq!(mgr.wallet_ids().await, vec![id]); +} + +/// RT-PersisterSkip: a wallet the persister itself rejected as corrupt +/// before reconstruction — surfaced via `ClientStartState::skipped` (e.g. +/// the FFI `load()` catching a malformed xpub per-row) — is folded into +/// `LoadOutcome::skipped` and fires `on_wallet_skipped_on_load`, while the +/// healthy wallet still loads. One bad persisted row never blocks the batch. +#[tokio::test] +async fn rt_persister_skipped_folds_into_outcome() { + let seed_ok = [0x71; 64]; + let p = Arc::new(FixedLoadPersister::new()); + let h = Arc::new(RecordingHandler::default()); + let (id_ok, s_ok) = slice(seed_ok); + + // A wallet id the persister could not decode (fabricated skip). + let bad_id: WalletId = [0x09; 32]; + let reason = SkipReason::CorruptPersistedRow { + kind: CorruptKind::DecodeError("malformed account xpub".to_string()), + }; + + let mut st = ClientStartState::default(); + st.wallets.insert(id_ok, s_ok); + st.skipped.push((bad_id, reason.clone())); + p.set(st); + + let mgr = manager(Arc::clone(&p), Arc::clone(&h)).await; + let outcome = mgr + .load_from_persistor() + .await + .expect("Ok despite a persister-rejected row"); + + assert!( + outcome.loaded().contains(&id_ok), + "healthy wallet still loads" + ); + assert!(!outcome.loaded().contains(&bad_id)); + assert_eq!(outcome.skipped().len(), 1, "the rejected row surfaces once"); + assert_eq!(outcome.skipped()[0], (bad_id, reason.clone())); + assert!(mgr.get_wallet(&id_ok).await.is_some()); + assert!( + mgr.get_wallet(&bad_id).await.is_none(), + "the rejected row is never registered" + ); + + // The skip notification fired exactly once for the bad row. + let skipped = h.skipped.lock().unwrap(); + assert_eq!(skipped.len(), 1, "exactly one skip notification"); + assert_eq!(skipped[0], (bad_id, reason)); +} + +/// RT-Corrupt: a corrupt row (empty manifest) is skipped with +/// `MissingManifest`; the other row loads cleanly; the load returns +/// `Ok`; `on_wallet_skipped_on_load` fires exactly once on the +/// registered handler for the skipped row. +#[tokio::test] +async fn rt_corrupt_row_skipped_and_other_loads() { + let seed_a = [0x31; 64]; + let seed_b = [0x32; 64]; + let p = Arc::new(FixedLoadPersister::new()); + let h = Arc::new(RecordingHandler::default()); + let (id_a, sa) = slice(seed_a); + let (id_b, mut sb_corrupt) = slice(seed_b); + + // B's row is structurally corrupt — empty manifest. The empty manifest + // aborts the watch-only rebuild before the snapshot is ever consulted. + sb_corrupt.account_manifest = Vec::new(); + + let mut st = ClientStartState::default(); + st.wallets.insert(id_a, sa); + st.wallets.insert(id_b, sb_corrupt); + p.set(st); + + let mgr = manager(Arc::clone(&p), Arc::clone(&h)).await; + let outcome = mgr + .load_from_persistor() + .await + .expect("Ok despite per-row skip"); + + assert!(outcome.loaded().contains(&id_a), "A loads fully"); + assert!( + !outcome.loaded().contains(&id_b), + "B is skipped, not loaded" + ); + assert_eq!(outcome.skipped().len(), 1); + let (skipped_id, skipped_reason) = &outcome.skipped()[0]; + assert_eq!(*skipped_id, id_b); + assert!(matches!( + skipped_reason, + SkipReason::CorruptPersistedRow { + kind: CorruptKind::MissingManifest + } + )); + assert!(mgr.get_wallet(&id_a).await.is_some()); + assert!( + mgr.get_wallet(&id_b).await.is_none(), + "corrupt row must be ABSENT, not a degraded placeholder" + ); + + // Exactly one on_wallet_skipped_on_load notification for B. + { + let skipped = h.skipped.lock().unwrap(); + assert_eq!(skipped.len(), 1, "exactly one skip notification expected"); + let (skipped_wallet_id, skipped_reason) = &skipped[0]; + assert_eq!(*skipped_wallet_id, id_b); + assert!(matches!( + skipped_reason, + SkipReason::CorruptPersistedRow { + kind: CorruptKind::MissingManifest + } + )); + } +} + +/// RT-Z: no key/seed material leaks into `LoadOutcome` / +/// `SkipReason::CorruptPersistedRow` surfaces. The seedless load path +/// never sees seed bytes so this is mostly a sentinel guard against +/// future regression where someone embeds row contents in `DecodeError`. +#[tokio::test] +async fn rt_z_secret_hygiene_surfaces() { + let seed = [0xAB; 64]; + let p = Arc::new(FixedLoadPersister::new()); + let h = Arc::new(RecordingHandler::default()); + let (id, mut corrupt) = slice(seed); + + // Corrupt row to force a skip and inspect every public surface. + corrupt.account_manifest = Vec::new(); + let mut st = ClientStartState::default(); + st.wallets.insert(id, corrupt); + p.set(st); + + let mgr = manager(Arc::clone(&p), Arc::clone(&h)).await; + let outcome = mgr.load_from_persistor().await.expect("Ok"); + let dbg = format!("{outcome:?}"); + // 0xAB seed bytes must not appear hex-rendered anywhere. + assert!(!dbg.to_lowercase().contains(&"ab".repeat(10))); + // The structural skip reason renders without any row bytes. + for (_, reason) in outcome.skipped() { + let rendered = format!("{reason} {reason:?}"); + assert!(!rendered.to_lowercase().contains(&"ab".repeat(10))); + } +} + +/// RT-Snapshot: a carried `wallet_info` snapshot is consumed +/// verbatim. Two properties the projection replay could NOT provide: +/// - per-account UTXO attribution — a CoinJoin-account UTXO stays on the +/// CoinJoin account (the fallback path routed every UTXO to the first +/// funds account, zeroing non-first-account balances); +/// - derived-but-unused deep pool addresses (idx 40, past the eager gap +/// window) stay in the pool, so the SPV watch set still covers a +/// handed-out-but-unpaid receive address after restart. +#[tokio::test] +async fn rt_snapshot_preserves_attribution_and_pools() { + use key_wallet::account::AccountType; + use key_wallet::managed_account::address_pool::KeySource; + use key_wallet::managed_account::managed_account_trait::ManagedAccountTrait; + use key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; + use key_wallet::wallet::managed_wallet_info::ManagedWalletInfo; + + let seed = [0x66; 64]; + let wallet = Wallet::from_seed_bytes( + seed, + key_wallet::Network::Testnet, + WalletAccountCreationOptions::Default, + ) + .unwrap(); + let id = wallet.compute_wallet_id(); + let manifest: Vec = wallet + .accounts + .all_accounts() + .into_iter() + .map(|a| AccountRegistrationEntry { + account_type: a.account_type, + account_xpub: a.account_xpub, + }) + .collect(); + + let mut info = ManagedWalletInfo::from_wallet(&wallet, 1); + + // A UTXO on the CoinJoin account's own idx-0 address, inserted where + // the persisted rows put it: on the CoinJoin account. + let cj_value = 250_000u64; + let (cj_type, cj_addr) = { + let cj = info + .accounts + .all_funding_accounts() + .into_iter() + .find(|a| { + matches!( + a.managed_account_type().to_account_type(), + AccountType::CoinJoin { .. } + ) + }) + .expect("Default creation includes a CoinJoin account"); + let addr = cj + .managed_account_type() + .address_pools() + .first() + .expect("CoinJoin account has a pool") + .address_at_index(0) + .expect("eager window covers idx 0"); + (cj.managed_account_type().to_account_type(), addr) + }; + { + let cj = info + .accounts + .all_funding_accounts_mut() + .into_iter() + .find(|a| a.managed_account_type().to_account_type() == cj_type) + .unwrap(); + cj.utxos.insert( + dashcore::OutPoint { + txid: dashcore::Txid::from([0x42u8; 32]), + vout: 0, + }, + key_wallet::Utxo { + outpoint: dashcore::OutPoint { + txid: dashcore::Txid::from([0x42u8; 32]), + vout: 0, + }, + txout: dashcore::TxOut { + value: cj_value, + script_pubkey: cj_addr.script_pubkey(), + }, + address: cj_addr, + height: 1, + is_coinbase: false, + is_confirmed: true, + is_instantlocked: false, + is_locked: false, + is_trusted: false, + }, + ); + } + + // Extend the FIRST funds account's first pool to idx 40 — a + // derived-but-UNUSED deep address (handed out, not yet paid). + let (first_type, deep_keys_total) = { + let first = info + .accounts + .all_funding_accounts_mut() + .into_iter() + .next() + .expect("a first funds account exists"); + let first_type = first.managed_account_type().to_account_type(); + let xpub = manifest + .iter() + .find(|e| e.account_type == first_type) + .map(|e| e.account_xpub) + .expect("first funds account xpub in manifest"); + let pools = first.managed_account_type_mut().address_pools_mut(); + let pool = pools.into_iter().next().expect("first pool"); + let highest = pool.highest_generated.expect("eager window derived"); + assert!( + highest < 40, + "fixture: idx 40 must be past the eager window" + ); + pool.generate_addresses(40 - highest, &KeySource::Public(xpub), true) + .unwrap(); + assert!( + pool.address_at_index(40).is_some(), + "fixture: idx 40 derived" + ); + (first_type, pool.addresses.len() as u32) + }; + info.update_balance(); + + let (_, mut s) = slice(seed); + s.wallet_info = Box::new(info); + let p = Arc::new(FixedLoadPersister::new()); + let h = Arc::new(RecordingHandler::default()); + let mut st = ClientStartState::default(); + st.wallets.insert(id, s); + p.set(st); + + let mgr = manager(Arc::clone(&p), Arc::clone(&h)).await; + let outcome = mgr.load_from_persistor().await.expect("Ok"); + assert_eq!(outcome.loaded(), vec![id].as_slice()); + assert!(outcome.skipped().is_empty()); + + let rows = { + let mgr = Arc::clone(&mgr); + tokio::task::spawn_blocking(move || mgr.account_balances_blocking(&id)) + .await + .unwrap() + }; + let cj_row = rows + .iter() + .find(|r| r.account_type == cj_type) + .expect("CoinJoin account row"); + assert_eq!( + cj_row.balance.total(), + cj_value, + "CoinJoin UTXO must stay attributed to the CoinJoin account" + ); + let first_row = rows + .iter() + .find(|r| r.account_type == first_type) + .expect("first funds account row"); + assert!( + first_row.keys_total >= deep_keys_total, + "derived-but-unused deep addresses must survive the reload \ + (watch-set coverage): got {} keys, snapshot had {}", + first_row.keys_total, + deep_keys_total, + ); +} + +/// RT-Snapshot-Mismatch: a snapshot whose `wallet_id` does not match its +/// row key is a corrupt row — skipped with `SnapshotIdentityMismatch`, +/// never registered, and the batch continues. +#[tokio::test] +async fn rt_snapshot_wallet_id_mismatch_is_skipped() { + use key_wallet::wallet::managed_wallet_info::ManagedWalletInfo; + + let seed = [0x77; 64]; + let other_seed = [0x78; 64]; + let p = Arc::new(FixedLoadPersister::new()); + let h = Arc::new(RecordingHandler::default()); + + // Row keyed by wallet A, snapshot built from wallet B. + let (id_a, mut s) = slice(seed); + let wallet_b = Wallet::from_seed_bytes( + other_seed, + key_wallet::Network::Testnet, + WalletAccountCreationOptions::Default, + ) + .unwrap(); + s.wallet_info = Box::new(ManagedWalletInfo::from_wallet(&wallet_b, 1)); + + let mut st = ClientStartState::default(); + st.wallets.insert(id_a, s); + p.set(st); + + let mgr = manager(Arc::clone(&p), Arc::clone(&h)).await; + let outcome = mgr.load_from_persistor().await.expect("Ok"); + + assert!(outcome.loaded().is_empty(), "mismatched row must not load"); + assert_eq!(outcome.skipped().len(), 1); + let (skipped_id, reason) = &outcome.skipped()[0]; + assert_eq!(*skipped_id, id_a); + assert!(matches!( + reason, + SkipReason::CorruptPersistedRow { + kind: CorruptKind::SnapshotIdentityMismatch + } + )); + assert!(mgr.get_wallet(&id_a).await.is_none()); + assert_eq!(h.skipped.lock().unwrap().len(), 1); +} + +/// RT-Snapshot-AccountMismatch: a snapshot whose `wallet_id`/`network` +/// agree with the row but whose account set diverges from the row's +/// account manifest is a wrong-row snapshot — skipped with +/// `SnapshotIdentityMismatch`, never registered. +#[tokio::test] +async fn rt_snapshot_account_set_mismatch_is_skipped() { + use key_wallet::wallet::managed_wallet_info::ManagedWalletInfo; + + let seed = [0x79; 64]; + let p = Arc::new(FixedLoadPersister::new()); + let h = Arc::new(RecordingHandler::default()); + + // Row keyed by wallet A with a full snapshot of A, but the row's + // manifest is truncated to a single account — the account sets diverge + // even though wallet_id and network match. + let wallet_a = Wallet::from_seed_bytes( + seed, + key_wallet::Network::Testnet, + WalletAccountCreationOptions::Default, + ) + .unwrap(); + let id_a = wallet_a.compute_wallet_id(); + let (full_manifest, _) = manifest_and_id(seed); + assert!( + full_manifest.len() > 1, + "fixture: Default creation yields more than one account" + ); + let truncated_manifest = vec![full_manifest[0].clone()]; + + let (_, mut s) = slice(seed); + s.account_manifest = truncated_manifest; + s.wallet_info = Box::new(ManagedWalletInfo::from_wallet(&wallet_a, 1)); + + let mut st = ClientStartState::default(); + st.wallets.insert(id_a, s); + p.set(st); + + let mgr = manager(Arc::clone(&p), Arc::clone(&h)).await; + let outcome = mgr.load_from_persistor().await.expect("Ok"); + + assert!( + outcome.loaded().is_empty(), + "account-set mismatch must not load" + ); + assert_eq!(outcome.skipped().len(), 1); + let (skipped_id, reason) = &outcome.skipped()[0]; + assert_eq!(*skipped_id, id_a); + assert!(matches!( + reason, + SkipReason::CorruptPersistedRow { + kind: CorruptKind::SnapshotIdentityMismatch + } + )); + assert!(mgr.get_wallet(&id_a).await.is_none()); + assert_eq!(h.skipped.lock().unwrap().len(), 1); +} + +/// RT-Snapshot-Mismatch-Combined: a snapshot-identity-mismatch skip and a +/// healthy snapshot load in the SAME batch. The mismatched row is skipped +/// with `SnapshotIdentityMismatch`; the healthy row loads fully; the batch +/// returns `Ok` and notifies the handler exactly once. Mirrors +/// `rt_corrupt_row_skipped_and_other_loads` for the snapshot path. +#[tokio::test] +async fn rt_snapshot_mismatch_skip_coexists_with_healthy_load() { + use key_wallet::wallet::managed_wallet_info::ManagedWalletInfo; + + let seed_ok = [0x81; 64]; + let seed_bad = [0x82; 64]; + let seed_other = [0x83; 64]; + let p = Arc::new(FixedLoadPersister::new()); + let h = Arc::new(RecordingHandler::default()); + + // Healthy row: snapshot built from its own wallet, matching its row. + let wallet_ok = Wallet::from_seed_bytes( + seed_ok, + key_wallet::Network::Testnet, + WalletAccountCreationOptions::Default, + ) + .unwrap(); + let id_ok = wallet_ok.compute_wallet_id(); + let (_, mut s_ok) = slice(seed_ok); + s_ok.wallet_info = Box::new(ManagedWalletInfo::from_wallet(&wallet_ok, 1)); + + // Mismatched row: keyed by wallet BAD, snapshot built from wallet OTHER. + let (id_bad, mut s_bad) = slice(seed_bad); + let wallet_other = Wallet::from_seed_bytes( + seed_other, + key_wallet::Network::Testnet, + WalletAccountCreationOptions::Default, + ) + .unwrap(); + s_bad.wallet_info = Box::new(ManagedWalletInfo::from_wallet(&wallet_other, 1)); + + let mut st = ClientStartState::default(); + st.wallets.insert(id_ok, s_ok); + st.wallets.insert(id_bad, s_bad); + p.set(st); + + let mgr = manager(Arc::clone(&p), Arc::clone(&h)).await; + let outcome = mgr + .load_from_persistor() + .await + .expect("Ok despite the per-row snapshot mismatch"); + + assert_eq!( + outcome.loaded(), + vec![id_ok].as_slice(), + "only the healthy row loads" + ); + assert_eq!(outcome.skipped().len(), 1); + let (skipped_id, reason) = &outcome.skipped()[0]; + assert_eq!(*skipped_id, id_bad); + assert!(matches!( + reason, + SkipReason::CorruptPersistedRow { + kind: CorruptKind::SnapshotIdentityMismatch + } + )); + assert!(mgr.get_wallet(&id_ok).await.is_some()); + assert!( + mgr.get_wallet(&id_bad).await.is_none(), + "mismatched row must be absent, not a degraded placeholder" + ); + + let skipped = h.skipped.lock().unwrap(); + assert_eq!(skipped.len(), 1, "exactly one skip notification"); + assert_eq!(skipped[0].0, id_bad); +} + +/// RT-PersisterLoad-Transient: a transient persister load failure +/// propagates as a typed `PersisterLoad` error whose retry classification +/// survives — `is_transient()` is `true` so callers may back off and retry. +#[tokio::test] +async fn rt_persister_load_transient_error_is_typed_and_retryable() { + let p = Arc::new(FailingLoadPersister { transient: true }); + let h = Arc::new(RecordingHandler::default()); + let sdk = Arc::new(dash_sdk::Sdk::new_mock()); + let mgr = Arc::new(PlatformWalletManager::new(sdk, Arc::clone(&p), h)); + + let err = mgr + .load_from_persistor() + .await + .expect_err("transient backend failure must surface"); + match err { + PlatformWalletError::PersisterLoad(inner) => { + assert!( + inner.is_transient(), + "transient classification must survive propagation" + ); + assert_eq!(inner.kind(), Some(PersistenceErrorKind::Transient)); + } + other => panic!("expected PersisterLoad, got {other:?}"), + } +} + +/// RT-PersisterLoad-Permanent: a fatal persister load failure propagates as +/// a typed `PersisterLoad` error classified non-transient, so callers do +/// not retry a permanent failure. +#[tokio::test] +async fn rt_persister_load_permanent_error_is_typed_and_not_retryable() { + let p = Arc::new(FailingLoadPersister { transient: false }); + let h = Arc::new(RecordingHandler::default()); + let sdk = Arc::new(dash_sdk::Sdk::new_mock()); + let mgr = Arc::new(PlatformWalletManager::new(sdk, Arc::clone(&p), h)); + + let err = mgr + .load_from_persistor() + .await + .expect_err("fatal backend failure must surface"); + match err { + PlatformWalletError::PersisterLoad(inner) => { + assert!( + !inner.is_transient(), + "fatal failure must not read as retryable" + ); + assert_eq!(inner.kind(), Some(PersistenceErrorKind::Fatal)); + } + other => panic!("expected PersisterLoad, got {other:?}"), + } +} + +/// RT-EmptyFirstRun: a genuinely fresh install — the persister returns +/// `ClientStartState::default()` with no rows at all — loads cleanly as +/// `LoadOutcome::Loaded` with empty loaded/skipped and fires zero skip +/// handlers. This is the condition every first launch hits. +#[tokio::test] +async fn rt_empty_first_run_is_clean_with_no_handler_calls() { + // `FixedLoadPersister::new()` without `set()` returns the default + // (empty) ClientStartState from `load()`. + let p = Arc::new(FixedLoadPersister::new()); + let h = Arc::new(RecordingHandler::default()); + + let mgr = manager(Arc::clone(&p), Arc::clone(&h)).await; + let outcome = mgr + .load_from_persistor() + .await + .expect("an empty store must load cleanly"); + + assert!( + matches!(outcome, LoadOutcome::Loaded { .. }), + "an empty store is a full (Loaded) outcome, not partial/none-usable" + ); + assert!( + outcome.loaded().is_empty(), + "nothing to load on a fresh install" + ); + assert!( + outcome.skipped().is_empty(), + "nothing to skip on a fresh install" + ); + assert!(mgr.wallet_ids().await.is_empty(), "no wallets registered"); + assert!( + h.skipped.lock().unwrap().is_empty(), + "no skip handler fires on a fresh install" + ); +} + +/// RT-Concurrent: two concurrent `load_from_persistor` passes against the +/// same manager and multi-wallet snapshot. Both must complete without +/// error, every wallet must register exactly once (the idempotency check +/// releases its read lock before the write-lock insert — a check-then-act +/// window), and the outcomes must be consistent: each wallet is freshly +/// loaded by exactly one pass and reported `AlreadyRegistered` by the +/// other (never a corruption skip, never a hard error). +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn rt_concurrent_loads_register_each_wallet_exactly_once() { + let p = Arc::new(FixedLoadPersister::new()); + let h = Arc::new(RecordingHandler::default()); + let (id1, s1) = slice([0xC1; 64]); + let (id2, s2) = slice([0xC2; 64]); + let mut st = ClientStartState::default(); + st.wallets.insert(id1, s1); + st.wallets.insert(id2, s2); + p.set(st); + + let mgr = manager(Arc::clone(&p), Arc::clone(&h)).await; + let m1 = Arc::clone(&mgr); + let m2 = Arc::clone(&mgr); + let t1 = tokio::spawn(async move { m1.load_from_persistor().await }); + let t2 = tokio::spawn(async move { m2.load_from_persistor().await }); + let o1 = t1.await.unwrap().expect("first concurrent load Ok"); + let o2 = t2.await.unwrap().expect("second concurrent load Ok"); + + // Each wallet registered exactly once, in both maps. + assert!(mgr.get_wallet(&id1).await.is_some()); + assert!(mgr.get_wallet(&id2).await.is_some()); + assert_eq!( + mgr.wallet_ids().await.len(), + 2, + "each wallet registered exactly once despite the concurrent passes" + ); + + // Every wallet is freshly loaded by exactly one of the two passes. + let mut loaded_union: std::collections::BTreeSet = + o1.loaded().iter().copied().collect(); + for id in o2.loaded() { + assert!( + loaded_union.insert(*id), + "a wallet must be freshly loaded by at most one pass" + ); + } + assert!( + loaded_union.contains(&id1) && loaded_union.contains(&id2), + "both wallets loaded across the two passes" + ); + + // Any overlap surfaces as an AlreadyRegistered skip, never corruption. + for outcome in [&o1, &o2] { + for (_, reason) in outcome.skipped() { + assert_eq!( + *reason, + SkipReason::AlreadyRegistered, + "a concurrent overlap is an AlreadyRegistered skip, not a corrupt row" + ); + } + } +} + +/// Persister for TC-3692-011. Rebuilds — fresh on each `load()`, since the +/// identity-manager snapshot is intentionally not `Clone` — a +/// `ClientStartState` carrying one wallet-owned identity whose +/// `Identity.public_keys` already holds a CRITICAL / AUTHENTICATION / +/// ECDSA_SECP256K1 key, the shape the FFI/iOS path produces via +/// `build_wallet_identity_bucket` (never through an `IdentityKeysChangeSet`). +struct IdentityKeyedLoadPersister { + seed: [u8; 64], + identity_id: Identifier, +} + +impl PlatformWalletPersistence for IdentityKeyedLoadPersister { + fn store(&self, _: WalletId, _: PlatformWalletChangeSet) -> Result<(), PersistenceError> { + Ok(()) + } + fn flush(&self, _: WalletId) -> Result<(), PersistenceError> { + Ok(()) + } + fn load(&self) -> Result { + use dpp::identity::identity_public_key::v0::IdentityPublicKeyV0; + use dpp::identity::identity_public_key::{IdentityPublicKey, Purpose}; + use dpp::identity::v0::IdentityV0; + use dpp::identity::{Identity, KeyType, SecurityLevel}; + + let (id, mut s) = slice(self.seed); + + let key = IdentityPublicKey::V0(IdentityPublicKeyV0 { + id: 0, + purpose: Purpose::AUTHENTICATION, + security_level: SecurityLevel::CRITICAL, + contract_bounds: None, + key_type: KeyType::ECDSA_SECP256K1, + read_only: false, + data: dpp::platform_value::BinaryData::new(vec![2u8; 33]), + disabled_at: None, + }); + let mut public_keys = BTreeMap::new(); + public_keys.insert(0, key); + let identity = Identity::V0(IdentityV0 { + id: self.identity_id, + public_keys, + balance: 0, + revision: 0, + }); + + let mut inner = BTreeMap::new(); + inner.insert(0u32, ManagedIdentity::new(identity, 0)); + let mut ims = IdentityManagerStartState::default(); + ims.wallet_identities.insert(id, inner); + s.identity_manager = ims; + + let mut st = ClientStartState::default(); + st.wallets.insert(id, s); + Ok(st) + } +} + +/// TC-3692-011 [CRITICAL]: the restored identity's signing key is +/// selectable via the exact `get_first_public_key_matching` predicate the +/// real signing path uses (`contract.rs`, `dpns.rs`, ...) immediately +/// after `load_from_persistor`, with zero sync. The FFI/iOS path populates +/// `Identity.public_keys` at construction, so dropping +/// `ClientWalletStartState.identity_keys` must not change this — the key +/// must sit in the exact map slot the lookup reads, with no reliance on +/// `apply_identity_key_entry` ever running on the load path. +#[tokio::test] +async fn rt_signing_key_selectable_immediately_after_load() { + use dpp::identity::accessors::IdentityGettersV0; + use dpp::identity::identity_public_key::accessors::v0::IdentityPublicKeyGettersV0; + use dpp::identity::identity_public_key::Purpose; + use dpp::identity::{KeyType, SecurityLevel}; + + let seed = [0x5A; 64]; + let identity_id = Identifier::from([0x11; 32]); + let p = Arc::new(IdentityKeyedLoadPersister { seed, identity_id }); + let h = Arc::new(RecordingHandler::default()); + let sdk = Arc::new(dash_sdk::Sdk::new_mock()); + let mgr = Arc::new(PlatformWalletManager::new(sdk, Arc::clone(&p), h)); + + let outcome = mgr.load_from_persistor().await.expect("Ok"); + assert!( + outcome.skipped().is_empty(), + "identity row must not be skipped" + ); + let id = outcome + .loaded() + .first() + .copied() + .expect("exactly one wallet loaded"); + + // Immediately — no SPV sync, no identity refresh — drive the exact + // predicate a real signing flow uses to pick its authentication key. + let wallet = mgr.get_wallet(&id).await.expect("wallet registered"); + let wm = wallet.wallet_manager().read().await; + let info = wm.get_wallet_info(&id).expect("wallet info present"); + let managed = info + .identity_manager + .identity(&identity_id) + .expect("identity restored into the manager"); + let selected = managed + .identity + .get_first_public_key_matching( + Purpose::AUTHENTICATION, + [SecurityLevel::CRITICAL].into(), + [KeyType::ECDSA_SECP256K1].into(), + false, + ) + .expect("CRITICAL/AUTH/ECDSA signing key must be selectable post-load, zero sync"); + assert_eq!(selected.id(), 0, "the exact installed key is selected"); +} diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift index 54154fe81f6..c8982df7377 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift @@ -180,6 +180,13 @@ public class PlatformWalletManager: ObservableObject { /// Last error from a wallet operation, if any. Cleared on successful op. @Published public private(set) var lastError: Error? + /// Wallets Rust skipped during the most recent [`loadFromPersistor`] + /// because their persisted row was structurally corrupt. Empty after + /// a clean load. Rust treats these as non-fatal — the load still + /// succeeds — so they are surfaced here rather than through + /// [`lastError`], letting UI offer to inspect / clear the bad rows. + @Published public private(set) var lastLoadSkippedWallets: [SkippedWalletOnLoad] = [] + // MARK: - Internals /// FFI handle; `NULL_HANDLE` until [`configure`] is called. @@ -405,6 +412,35 @@ public class PlatformWalletManager: ObservableObject { // MARK: - Watch-only restore from persister + /// One wallet Rust skipped during `load_from_persistor` because its + /// persisted row was skipped. `reasonCode` is one of the Rust-side + /// `LOAD_SKIP_REASON_*` constants (100 missing manifest, 101 malformed + /// xpub, 102 decode error, 103 snapshot identity mismatch, 199 other + /// corrupt row, 200 other skip, 300 already registered); + /// [`reasonDescription`] renders it for display. + public struct SkippedWalletOnLoad { + public let walletId: Data + public let reasonCode: UInt32 + + /// Human-readable rendering of `reasonCode`, mirroring the Rust + /// `LOAD_SKIP_REASON_*` constants. These numbers are the wire + /// contract defined in `rs-platform-wallet-ffi/src/manager.rs`; + /// they are not surfaced as named symbols in the generated C + /// header, so the cases are matched by value against that source. + public var reasonDescription: String { + switch reasonCode { + case 100: return "missing account manifest" + case 101: return "malformed account xpub" + case 102: return "decode error" + case 103: return "snapshot does not match its persisted row" + case 199: return "other corrupt row" + case 200: return "other skip" + case 300: return "already registered" + default: return "unknown skip reason (\(reasonCode))" + } + } + } + /// Rehydrate wallets from SwiftData on app launch. /// /// Calls `platform_wallet_manager_load_from_persistor` which fires @@ -431,7 +467,46 @@ public class PlatformWalletManager: ObservableObject { public func loadFromPersistor() throws -> [ManagedPlatformWallet] { try ensureConfigured() - try platform_wallet_manager_load_from_persistor(handle).check() + // Consume the load outcome so Rust's per-wallet skip summary + // isn't discarded. Rust writes `out_outcome` on every path + // (including early errors), so freeing it is safe even if the + // `.check()` below throws — defer the free before that throwing + // call. + var outcome = LoadOutcomeFFI(loaded_count: 0, skipped_count: 0, skipped: nil) + let loadResult = platform_wallet_manager_load_from_persistor(handle, &outcome) + defer { platform_wallet_load_outcome_free(&outcome) } + + // Collect the ids Rust skipped (a corrupt row, or one already + // registered). These are non-fatal on the Rust side, so they must + // not reach `lastError`: they are surfaced through + // `lastLoadSkippedWallets` and their ids are excluded from the + // per-id restore loop below (a skipped id is still in SwiftData, so + // `get_wallet` would return NotFound for it). + // + // Rust writes `out_outcome` on every path (zeroed on a hard + // failure), so assign `lastLoadSkippedWallets` BEFORE the throwing + // `.check()` — a failed load then clears stale skip state from a + // prior load instead of leaving it behind. + var skippedIds = Set() + var skippedWallets: [SkippedWalletOnLoad] = [] + if let skipped = outcome.skipped { + skippedWallets.reserveCapacity(Int(outcome.skipped_count)) + for i in 0.. 0 })? - .accountIndex ?? 0 + senderAccountIndex = 0 } // Input selection and surplus handling are owned // by the Rust Auto path (surplus stays on the