From 7bb8eb1517d110589e4001b18c914a5ad1f4fac9 Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Mon, 31 Aug 2026 17:30:48 +0300 Subject: [PATCH 1/5] feat(platform-wallet): sweep changeset types, FFI seam, and persistence capability bits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The seam a durable sweep needs, with no producer behind it yet. Every line here is inert while `CoreChangeSet::sweeps` is empty, and it always is: nothing in this commit emits a sweep. `SweepBatch` carries one upstream sweep — the transactions it removed, the transaction that beat them, that winner's mined height when it had one, and the coins the removal actually freed. Batches are kept ordered rather than folded into one removal list plus one release set, because each batch describes the wallet at the moment it fired and those descriptions can disagree: an early sweep frees a coin, something later spends it, and a later sweep removes that spender while keeping the coin spent. Union the release sets and the first answer outlives the last one that is true. `merge` therefore appends, never folds, and the `Merge` trait's own contract is corrected to match — ordered and associative, NOT commutative. `sweeps` is the one subtractive field on an otherwise additive type, which is exactly why it has to exist: a persister that only appends keeps dead rows and replays them on the next load. It is `serde(default)`, so a payload written before the field existed still deserializes, with the only backward-compatible reading — no sweeps. On the FFI surface, sweeps travel through a new terminal slot on `PersistenceCallbacksExtension`, read only when the host's declared `struct_size` proves the field exists. That read is now one authority: `negotiated_extension_slot!` replaces the per-call-site size arithmetic (the DPNS and tracked-masternode readers adopt it), so a host whose struct stops mid-way keeps exactly the earlier slots it allocated and nothing is ever dereferenced past its allocation. Delivery is else-less: a legacy host processes the rest of the round, returns success, and never sees the sweeps at all. Two capability bits declare what a backend actually implements. `CORE_SWEEP_REMOVAL` (bit 11) is derived only when the negotiated sweeps slot, the legacy changeset slot, a begin/end pair AND `ATOMIC_CHANGESETS` are all present, then intersected with the host's own declaration; `DASHPAY_PAYMENTS` (bit 12) requires its wired callback plus the declaration. Both gain `names()` entries, and a new test walks every declarable bit to keep an unnamed one from ever gating behaviour invisibly again — which is what `DASHPAY_PAYMENTS` did until now. Tests: legacy-sized extensions refuse the sweeps slot while keeping DPNS; a hand-built changeset's sweeps reach the host through the extension after the changeset itself; a sweep attested without an atomic round is refused across the six-case table; a pre-sweep serde payload still loads. --- .../src/core_wallet_types.rs | 175 ++++- .../rs-platform-wallet-ffi/src/invitation.rs | 12 +- .../rs-platform-wallet-ffi/src/manager.rs | 260 +++++-- .../rs-platform-wallet-ffi/src/persistence.rs | 729 +++++++++++++++++- .../src/changeset/changeset.rs | 120 ++- .../rs-platform-wallet/src/changeset/merge.rs | 16 +- .../src/changeset/persistence_capabilities.rs | 62 +- 7 files changed, 1288 insertions(+), 86 deletions(-) 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 400fd8b87c9..86e6ce7311c 100644 --- a/packages/rs-platform-wallet-ffi/src/core_wallet_types.rs +++ b/packages/rs-platform-wallet-ffi/src/core_wallet_types.rs @@ -15,6 +15,21 @@ pub struct OutPointFFI { pub vout: u32, } +impl From<&dashcore::OutPoint> for OutPointFFI { + /// The one authority for `OutPoint` → FFI conversion. This value is + /// the join key sweep releases use to find additive-path rows on the + /// host side, so a byte-order drift between hand-rolled copies would + /// silently unlink them — every conversion site routes through here. + fn from(outpoint: &dashcore::OutPoint) -> Self { + let mut txid = [0u8; 32]; + txid.copy_from_slice(outpoint.txid.as_ref()); + Self { + txid, + vout: outpoint.vout, + } + } +} + /// Outpoint of a TXO that was spent, paired with the spending /// transaction's txid. Replaces the bare `OutPointFFI` on /// `AccountChangeSetFFI.utxos_spent` so the Swift persister can @@ -237,6 +252,74 @@ pub struct WalletChangeSetFFI { /// `proof.rs` can't fire until SPV re-applies a fresh CL). pub last_applied_chain_lock_bytes: *mut u8, pub last_applied_chain_lock_bytes_len: usize, + // This struct's layout is FROZEN here. It crosses the C ABI by bare + // pointer — `on_persist_wallet_changeset_fn` carries no size or version + // field — so appending anything makes the pairing of a new callback + // with an older native producer read past the end of the producer's + // allocation: the callback signature and the manager-create entry + // points are unchanged, so nothing stops that pairing, and a capability + // bit gates semantics, not memory layout — it cannot make an + // out-of-bounds read safe. The round's sweep batches, briefly appended + // here, now travel through the size-tagged + // `PersistenceCallbacksExtension` sweep callback instead (see + // `persistence.rs`), whose declared `struct_size` is exactly the proof + // of presence this struct cannot give. New per-round payloads must take + // that same route. +} + +/// One sweep: the transactions it removed, the transaction that beat them, +/// and the coins its removal actually freed. +/// +/// Delivered through `PersistenceCallbacksExtension`'s +/// `on_persist_wallet_changeset_sweeps_fn` — deliberately NOT a field on +/// [`WalletChangeSetFFI`], whose bare-pointer ABI cannot prove to a newer +/// consumer that an older producer allocated the field (see the layout note +/// there). The batches arrive in the order the wallet emitted them, and the +/// only subtractive part of a persistence round rides here: each entry +/// describes the wallet as that sweep saw it, and a later entry can keep a +/// coin spent that an earlier one freed. **A persister must apply them in +/// sequence** — folding them together lets the first answer outlive the +/// last one that is actually true. Ignoring them leaves dead rows that are +/// handed back at the next load and re-create a balance the wallet has +/// already corrected. +#[repr(C)] +pub struct SweepBatchFFI { + /// Removed transactions, raw 32-byte txids. Delete these rows and every + /// UTXO they created. + pub txids: *const [u8; 32], + pub txids_count: usize, + /// The transaction whose arrival settled the inputs. Final, and not + /// necessarily wallet-relevant — it can pay entirely to outside + /// addresses and never reach this store at all, which is why what it + /// took cannot be worked out by looking it up. + pub superseded_by: [u8; 32], + /// Of the inputs the removed transactions claimed, the ones that came + /// free. Everything else they claimed was taken by `superseded_by` and + /// stays spent — a persister holds every input of what it deletes, so + /// this is the only thing telling it which to hand back. + pub released_outpoints: *const OutPointFFI, + pub released_outpoints_count: usize, + /// Whether `winner_mined_height` is meaningful. `false` means the sweep + /// was triggered by an InstantSend-locked winner still waiting to be + /// mined (upstream's only other trigger — an unlocked mempool arrival + /// never sweeps), and the winner has NO finality horizon: a persister + /// must still create a durable placeholder for a held-but-unfunded + /// input — under DIP-10 the lock alone settles it, and the placeholder + /// is the only claim that survives a restart — but must leave it + /// UNSTAMPED and never collect an unstamped placeholder (the winner has + /// no mining deadline, so no watermark proves its funding output + /// delivered-or-never; only funding materialisation, a later + /// block-context re-stamp, or a release resolves it). Re-pointing an + /// existing placeholder on such a sweep must keep (not clear) any + /// stamp it already carries. + pub has_winner_mined_height: bool, + /// Mined height of `superseded_by` when `has_winner_mined_height` — + /// the winner's own block, carried from the sweep event because the + /// winner may never appear anywhere else in this wallet's stream. A + /// persister stamps it onto the placeholder it writes for a + /// held-but-unfunded input, and collects that placeholder exactly when + /// `min(chainlock_height, synced_height)` reaches the stamp. + pub winner_mined_height: u32, } // --------------------------------------------------------------------------- @@ -520,6 +603,82 @@ impl WalletChangeSetFFI { } } +/// Backing storage for one [`SweepBatchFFI`]'s nested buffers. The C struct +/// borrows into it, so the caller keeps this alive for the callback window — +/// the same `(entries, storage)` discipline +/// `build_address_pools_for_callback` uses, rather than `Box::into_raw` + +/// a paired free: nothing outlives the call, so nothing needs a free path. +pub(crate) struct SweepBatchStorage { + txids: Vec<[u8; 32]>, + released: Vec, +} + +/// Build the C mirrors of a changeset's sweep batches for the extension +/// sweep callback (`on_persist_wallet_changeset_sweeps_fn`), preserving the +/// wallet's emission order — the one property a persister cannot recover on +/// its own, since a later batch can keep a coin spent that an earlier one +/// freed. Sweeps travel wallet-scoped, not per account: the upstream events +/// are wallet-scoped, and the persister deletes by txid — the row it +/// deletes carries its own account link. +pub(crate) fn build_sweep_batches_for_callback( + cs: &platform_wallet::changeset::CoreChangeSet, +) -> (Vec, Vec) { + let storage: Vec = cs + .sweeps + .iter() + .map(|batch| SweepBatchStorage { + txids: batch + .txids + .iter() + .map(|txid| { + let mut raw = [0u8; 32]; + raw.copy_from_slice(txid.as_ref()); + raw + }) + .collect(), + released: batch + .released_outpoints + .iter() + .map(OutPointFFI::from) + .collect(), + }) + .collect(); + + let batches: Vec = cs + .sweeps + .iter() + .zip(storage.iter()) + .map(|(batch, backing)| { + let mut superseded_by = [0u8; 32]; + superseded_by.copy_from_slice(batch.superseded_by.as_ref()); + SweepBatchFFI { + // `*const`, built straight from `as_ptr()`: the storage is + // borrowed immutably here, and `Vec::as_ptr` does not permit + // writes through the pointer or anything derived from it. + // Casting to `*mut` would advertise a C ABI that a callback + // could take literally, breaking Rust's aliasing rules. + txids: if backing.txids.is_empty() { + std::ptr::null() + } else { + backing.txids.as_ptr() + }, + txids_count: backing.txids.len(), + superseded_by, + released_outpoints: if backing.released.is_empty() { + std::ptr::null() + } else { + backing.released.as_ptr() + }, + released_outpoints_count: backing.released.len(), + has_winner_mined_height: batch.winner_mined_height.is_some(), + winner_mined_height: batch.winner_mined_height.unwrap_or(0), + } + }) + .collect(); + + (batches, storage) +} + /// Returns the account "index" the FFI surfaces in `account_index`. /// /// For variants with a natural index field (`Standard`, `CoinJoin`, @@ -926,13 +1085,8 @@ fn record_spent_outpoints_ffi( .iter() .filter_map(|d| { let input = rec.transaction.input.get(d.index as usize)?; - let mut txid = [0u8; 32]; - txid.copy_from_slice(input.previous_output.txid.as_ref()); Some(SpentOutPointFFI { - outpoint: OutPointFFI { - txid, - vout: input.previous_output.vout, - }, + outpoint: OutPointFFI::from(&input.previous_output), spending_txid, }) }) @@ -1309,14 +1463,7 @@ fn tx_record_to_ffi( tr.transaction .input .iter() - .map(|input| { - let mut prev_txid = [0u8; 32]; - prev_txid.copy_from_slice(input.previous_output.txid.as_ref()); - OutPointFFI { - txid: prev_txid, - vout: input.previous_output.vout, - } - }) + .map(|input| OutPointFFI::from(&input.previous_output)) .collect() }; let input_outpoints_count = input_outpoints_vec.len(); diff --git a/packages/rs-platform-wallet-ffi/src/invitation.rs b/packages/rs-platform-wallet-ffi/src/invitation.rs index e007a3de056..721f0910b10 100644 --- a/packages/rs-platform-wallet-ffi/src/invitation.rs +++ b/packages/rs-platform-wallet-ffi/src/invitation.rs @@ -208,15 +208,11 @@ pub unsafe extern "C" fn platform_wallet_create_invitation( let result = unwrap_option_or_return!(option); let invitation = unwrap_result_or_return!(result); - // Marshal the funding outpoint out. `Txid: AsRef<[u8]>`, matching the - // conversion convention used across this crate's changeset FFI. - let mut txid = [0u8; 32]; - txid.copy_from_slice(invitation.out_point.txid.as_ref()); + // Marshal the funding outpoint out through the crate's one conversion + // authority (`From<&OutPoint> for OutPointFFI`) — this value joins the + // same outpoint-keyed rows the sweep releases match on. unsafe { - *out_outpoint = OutPointFFI { - txid, - vout: invitation.out_point.vout, - }; + *out_outpoint = OutPointFFI::from(&invitation.out_point); } // The URI is a secret (embeds the voucher key). Do NOT log it — the error diff --git a/packages/rs-platform-wallet-ffi/src/manager.rs b/packages/rs-platform-wallet-ffi/src/manager.rs index 4180a774462..3b0cd0b9013 100644 --- a/packages/rs-platform-wallet-ffi/src/manager.rs +++ b/packages/rs-platform-wallet-ffi/src/manager.rs @@ -9,7 +9,8 @@ use crate::event_handler::{ use crate::handle::*; use crate::persistence::{ FFIPersister, FreeTrackedMasternodesFn, LoadTrackedMasternodesFn, PersistDpnsNameStatesFn, - PersistTrackedMasternodesFn, PersistenceCallbacks, PersistenceCallbacksExtension, + PersistTrackedMasternodesFn, PersistWalletChangesetChainLockHeightFn, + PersistWalletChangesetSweepsFn, PersistenceCallbacks, PersistenceCallbacksExtension, PersistenceCapabilitiesFFI, PersistenceExtensionCallbacks, PLATFORM_WALLET_PERSISTENCE_CALLBACKS_EXTENSION_VERSION, }; @@ -172,71 +173,94 @@ pub unsafe extern "C" fn platform_wallet_manager_create_with_extensions( ) } +/// Read one negotiated slot out of a size/version-tagged extension struct +/// — the single authority for the gate every reader below applies. A slot +/// is read only when the host's declared `struct_size` proves it was +/// allocated, so an extension built before the slot existed keeps its +/// earlier callbacks and simply never has the new one read — the +/// fail-closed half of the negotiation a bare-pointer callback struct +/// cannot perform itself (dashpay/platform#4406, finding 2). The version +/// check stays an exact match on purpose: the version names the field +/// ordering, and appending under it is what `struct_size` exists for. +/// +/// # Safety +/// `$extension` must point to a live extension struct of type `$ext_ty` +/// whose `struct_size` honestly describes its allocation. +macro_rules! negotiated_extension_slot { + ($extension:expr, $ext_ty:ty, $version_const:expr, $field:ident, $fn_ty:ty) => {{ + let extension: *const $ext_ty = $extension; + let supplied_size = std::ptr::addr_of!((*extension).struct_size).read(); + let version_end = std::mem::offset_of!($ext_ty, version) + std::mem::size_of::(); + let callback_end = + std::mem::offset_of!($ext_ty, $field) + std::mem::size_of::>(); + if supplied_size < version_end + || std::ptr::addr_of!((*extension).version).read() != $version_const + || supplied_size < callback_end + { + None + } else { + std::ptr::addr_of!((*extension).$field).read() + } + }}; +} + +/// Read every negotiated persistence-extension slot through +/// [`negotiated_extension_slot!`] — one gate authority, applied per slot, +/// so a host whose `struct_size` stops mid-struct keeps exactly the +/// earlier slots it allocated. unsafe fn persistence_extension_callbacks( extension: *const PersistenceCallbacksExtension, ) -> PersistenceExtensionCallbacks { - let supplied_size = std::ptr::addr_of!((*extension).struct_size).read(); - let version_end = - std::mem::offset_of!(PersistenceCallbacksExtension, version) + std::mem::size_of::(); - if supplied_size < version_end { - return PersistenceExtensionCallbacks::default(); - } - let version = std::ptr::addr_of!((*extension).version).read(); - if version != PLATFORM_WALLET_PERSISTENCE_CALLBACKS_EXTENSION_VERSION { - return PersistenceExtensionCallbacks::default(); - } - - /// Read one size-gated `Option` field: present only when the - /// caller's `struct_size` proves the complete field exists. - macro_rules! gated { - ($field:ident, $callback:ty) => {{ - let end = std::mem::offset_of!(PersistenceCallbacksExtension, $field) - + std::mem::size_of::>(); - if supplied_size < end { - None - } else { - std::ptr::addr_of!((*extension).$field).read() - } - }}; + macro_rules! slot { + ($field:ident, $fn_ty:ty) => { + negotiated_extension_slot!( + extension, + PersistenceCallbacksExtension, + PLATFORM_WALLET_PERSISTENCE_CALLBACKS_EXTENSION_VERSION, + $field, + $fn_ty + ) + }; } PersistenceExtensionCallbacks { - dpns_name_states: gated!(on_persist_dpns_name_states_fn, PersistDpnsNameStatesFn), - persist_tracked_masternodes: gated!( + dpns_name_states: slot!(on_persist_dpns_name_states_fn, PersistDpnsNameStatesFn), + persist_tracked_masternodes: slot!( on_persist_tracked_masternodes_fn, PersistTrackedMasternodesFn ), - load_tracked_masternodes: gated!(on_load_tracked_masternodes_fn, LoadTrackedMasternodesFn), - load_tracked_masternodes_free: gated!( + load_tracked_masternodes: slot!(on_load_tracked_masternodes_fn, LoadTrackedMasternodesFn), + load_tracked_masternodes_free: slot!( on_load_tracked_masternodes_free_fn, FreeTrackedMasternodesFn ), + wallet_changeset_sweeps: slot!( + on_persist_wallet_changeset_sweeps_fn, + PersistWalletChangesetSweepsFn + ), + wallet_changeset_chain_lock_height: slot!( + on_persist_wallet_changeset_chain_lock_height_fn, + PersistWalletChangesetChainLockHeightFn + ), } } unsafe fn event_extension_dpns_callback( extension: *const EventHandlerCallbacksExtension, ) -> Option { - let supplied_size = std::ptr::addr_of!((*extension).struct_size).read(); - let version_end = - std::mem::offset_of!(EventHandlerCallbacksExtension, version) + std::mem::size_of::(); - if supplied_size < version_end { - return None; - } - let version = std::ptr::addr_of!((*extension).version).read(); - if version != PLATFORM_WALLET_EVENT_CALLBACKS_EXTENSION_VERSION { - return None; - } - let callback_end = std::mem::offset_of!( + negotiated_extension_slot!( + extension, EventHandlerCallbacksExtension, - on_dpns_marketplace_sync_completed_fn - ) + std::mem::size_of::>(); - if supplied_size < callback_end { - return None; - } - std::ptr::addr_of!((*extension).on_dpns_marketplace_sync_completed_fn).read() + PLATFORM_WALLET_EVENT_CALLBACKS_EXTENSION_VERSION, + on_dpns_marketplace_sync_completed_fn, + DpnsMarketplaceSyncCompletedFn + ) } +// The C entry point's own shape: every callback table and out-param the +// hosts pass, threaded straight through. Splitting it would only move the +// same arguments behind a struct the FFI cannot express. +#[allow(clippy::too_many_arguments)] unsafe fn platform_wallet_manager_create_impl( sdk_ptr: *const c_void, persistence: *const PersistenceCallbacks, @@ -806,6 +830,48 @@ mod tests { on_persist_dpns_name_states_fn: Option, } + unsafe extern "C" fn persist_wallet_changeset_sweeps( + _context: *mut c_void, + _wallet_id: *const u8, + _sweeps: *const crate::core_wallet_types::SweepBatchFFI, + _sweeps_count: usize, + ) -> i32 { + 0 + } + + unsafe extern "C" fn persist_wallet_changeset_chain_lock_height( + _context: *mut c_void, + _wallet_id: *const u8, + _chain_lock_height: u32, + ) -> i32 { + 0 + } + + unsafe extern "C" fn persist_tracked_masternodes( + _context: *mut c_void, + _network: *const std::os::raw::c_char, + _rows: *const crate::persistence::TrackedMasternodeFFI, + _rows_count: usize, + ) -> i32 { + 0 + } + + unsafe extern "C" fn load_tracked_masternodes( + _context: *mut c_void, + _network: *const std::os::raw::c_char, + _out_rows: *mut *const crate::persistence::TrackedMasternodeFFI, + _out_count: *mut usize, + ) -> i32 { + 0 + } + + unsafe extern "C" fn load_tracked_masternodes_free( + _context: *mut c_void, + _rows: *const crate::persistence::TrackedMasternodeFFI, + _count: usize, + ) { + } + fn persistence_callbacks() -> PersistenceCallbacks { PersistenceCallbacks { on_changeset_begin_fn: Some(begin_changeset), @@ -1118,19 +1184,25 @@ mod tests { on_persist_dpns_name_states_fn ), on_persist_dpns_name_states_fn: Some(persist_dpns_name_states), + on_persist_wallet_changeset_sweeps_fn: Some(persist_wallet_changeset_sweeps), ..Default::default() }; let unknown = PersistenceCallbacksExtension { version: PLATFORM_WALLET_PERSISTENCE_CALLBACKS_EXTENSION_VERSION + 1, on_persist_dpns_name_states_fn: Some(persist_dpns_name_states), + on_persist_wallet_changeset_sweeps_fn: Some(persist_wallet_changeset_sweeps), ..Default::default() }; let read_short = unsafe { persistence_extension_callbacks(&short) }; assert!(read_short.dpns_name_states.is_none()); assert!(read_short.persist_tracked_masternodes.is_none()); + assert!(read_short.wallet_changeset_sweeps.is_none()); + assert!(read_short.wallet_changeset_chain_lock_height.is_none()); let read_unknown = unsafe { persistence_extension_callbacks(&unknown) }; assert!(read_unknown.dpns_name_states.is_none()); assert!(read_unknown.load_tracked_masternodes.is_none()); + assert!(read_unknown.wallet_changeset_sweeps.is_none()); + assert!(read_unknown.wallet_changeset_chain_lock_height.is_none()); } /// A caller whose `struct_size` covers only the dpns field (an @@ -1160,6 +1232,104 @@ mod tests { assert!(read.persist_tracked_masternodes.is_none()); assert!(read.load_tracked_masternodes.is_none()); assert!(read.load_tracked_masternodes_free.is_none()); + assert!(read.wallet_changeset_sweeps.is_none()); + assert!(read.wallet_changeset_chain_lock_height.is_none()); + } + + /// The exact cross-version pairing the size-negotiated extension + /// exists for: a host built when the extension ended at an earlier + /// slot declares that smaller `struct_size` — bytes it filled with a + /// live callback are still bytes, so nothing but the declared size + /// distinguishes it from a current struct. Every later slot must be + /// refused, never read (reading it would be exactly the + /// past-the-allocation dereference the changeset struct could not + /// prevent), while every slot the size does prove keeps working. + /// Walks each historical boundary: DPNS-only, the tracked-masternode + /// trio, the sweeps slot, and the terminal chainlock-height slot. + #[test] + fn a_legacy_sized_extension_refuses_the_sweeps_slot_but_keeps_dpns() { + // DPNS-era host: everything after the DPNS slot is refused. + let legacy_size = std::mem::offset_of!( + PersistenceCallbacksExtension, + on_persist_tracked_masternodes_fn + ); + let legacy = PersistenceCallbacksExtension { + struct_size: legacy_size, + on_persist_dpns_name_states_fn: Some(persist_dpns_name_states), + // Set in the fixture to prove the gate never LOOKS: were the + // size check wrong, the read would find a live pointer and the + // assertion below would catch it. + on_persist_tracked_masternodes_fn: Some(persist_tracked_masternodes), + on_persist_wallet_changeset_sweeps_fn: Some(persist_wallet_changeset_sweeps), + ..Default::default() + }; + let read = unsafe { persistence_extension_callbacks(&legacy) }; + assert!(read.dpns_name_states.is_some()); + assert!(read.persist_tracked_masternodes.is_none()); + assert!(read.load_tracked_masternodes.is_none()); + assert!(read.load_tracked_masternodes_free.is_none()); + assert!(read.wallet_changeset_sweeps.is_none()); + assert!(read.wallet_changeset_chain_lock_height.is_none()); + + // A host built when the extension ended at the tracked-masternode + // trio: the trio negotiates, the sweeps and chainlock-height + // slots are refused, never read. + let masternodes_era_size = std::mem::offset_of!( + PersistenceCallbacksExtension, + on_persist_wallet_changeset_sweeps_fn + ); + let masternodes_era = PersistenceCallbacksExtension { + struct_size: masternodes_era_size, + on_persist_dpns_name_states_fn: Some(persist_dpns_name_states), + on_persist_tracked_masternodes_fn: Some(persist_tracked_masternodes), + on_load_tracked_masternodes_fn: Some(load_tracked_masternodes), + on_load_tracked_masternodes_free_fn: Some(load_tracked_masternodes_free), + on_persist_wallet_changeset_sweeps_fn: Some(persist_wallet_changeset_sweeps), + ..Default::default() + }; + let read = unsafe { persistence_extension_callbacks(&masternodes_era) }; + assert!(read.dpns_name_states.is_some()); + assert!(read.persist_tracked_masternodes.is_some()); + assert!(read.load_tracked_masternodes.is_some()); + assert!(read.load_tracked_masternodes_free.is_some()); + assert!(read.wallet_changeset_sweeps.is_none()); + assert!(read.wallet_changeset_chain_lock_height.is_none()); + + // A host built when the extension ended at the sweeps slot: sweeps + // negotiate, the chainlock-height slot is refused, never read. + let sweeps_era_size = std::mem::offset_of!( + PersistenceCallbacksExtension, + on_persist_wallet_changeset_chain_lock_height_fn + ); + let sweeps_era = PersistenceCallbacksExtension { + struct_size: sweeps_era_size, + on_persist_dpns_name_states_fn: Some(persist_dpns_name_states), + on_persist_wallet_changeset_sweeps_fn: Some(persist_wallet_changeset_sweeps), + on_persist_wallet_changeset_chain_lock_height_fn: Some( + persist_wallet_changeset_chain_lock_height, + ), + ..Default::default() + }; + let read = unsafe { persistence_extension_callbacks(&sweeps_era) }; + assert!(read.dpns_name_states.is_some()); + assert!(read.wallet_changeset_sweeps.is_some()); + assert!(read.wallet_changeset_chain_lock_height.is_none()); + + let current = PersistenceCallbacksExtension { + on_persist_dpns_name_states_fn: Some(persist_dpns_name_states), + on_persist_tracked_masternodes_fn: Some(persist_tracked_masternodes), + on_load_tracked_masternodes_fn: Some(load_tracked_masternodes), + on_load_tracked_masternodes_free_fn: Some(load_tracked_masternodes_free), + on_persist_wallet_changeset_sweeps_fn: Some(persist_wallet_changeset_sweeps), + on_persist_wallet_changeset_chain_lock_height_fn: Some( + persist_wallet_changeset_chain_lock_height, + ), + ..Default::default() + }; + let read = unsafe { persistence_extension_callbacks(¤t) }; + assert!(read.persist_tracked_masternodes.is_some()); + assert!(read.wallet_changeset_sweeps.is_some()); + assert!(read.wallet_changeset_chain_lock_height.is_some()); } } diff --git a/packages/rs-platform-wallet-ffi/src/persistence.rs b/packages/rs-platform-wallet-ffi/src/persistence.rs index 3b540ffbf8d..951994386d6 100644 --- a/packages/rs-platform-wallet-ffi/src/persistence.rs +++ b/packages/rs-platform-wallet-ffi/src/persistence.rs @@ -44,7 +44,9 @@ use crate::contact_persistence::{ free_contact_requests_ffi, ContactIgnoredSenderFFI, ContactRequestFFI, ContactRequestRemovalFFI, }; use crate::core_address_types::{AddressPoolTypeTagFFI, CoreAddressEntryFFI, KeyTypeTagFFI}; -use crate::core_wallet_types::{free_wallet_changeset_ffi, WalletChangeSetFFI}; +use crate::core_wallet_types::{ + build_sweep_batches_for_callback, free_wallet_changeset_ffi, SweepBatchFFI, WalletChangeSetFFI, +}; use crate::dashpay_payment::{build_payment_persist_entries, DashpayPaymentPersistEntryFFI}; use crate::dpns_name_state_persistence::{ build_dpns_name_state_entries, free_dpns_name_state_entries, DpnsNameStateFFI, @@ -132,6 +134,8 @@ pub const PLATFORM_WALLET_PERSISTENCE_CAPABILITY_TRACKED_ASSET_LOCKS: u64 = 1 << /// `on_persist_tracked_masternodes_fn` + `on_load_tracked_masternodes_fn` /// + `on_load_tracked_masternodes_free_fn`, and the host declaring the bit. pub const PLATFORM_WALLET_PERSISTENCE_CAPABILITY_TRACKED_MASTERNODES: u64 = 1 << 10; +pub const PLATFORM_WALLET_PERSISTENCE_CAPABILITY_CORE_SWEEP_REMOVAL: u64 = 1 << 11; +pub const PLATFORM_WALLET_PERSISTENCE_CAPABILITY_DASHPAY_PAYMENTS: u64 = 1 << 12; /// Version of [`PersistenceCallbacksExtension`]. The extension is deliberately /// separate from [`PersistenceCallbacks`]: existing hosts pass the latter by @@ -181,6 +185,38 @@ pub type LoadTrackedMasternodesFn = unsafe extern "C" fn( pub type FreeTrackedMasternodesFn = unsafe extern "C" fn(context: *mut c_void, rows: *const TrackedMasternodeFFI, count: usize); +/// Carries a round's sweep batches — the removals of transactions a later, +/// final transaction provably beat to an input. Fired between the same +/// begin/end pair as the round's other per-kind callbacks, immediately +/// after `on_persist_wallet_changeset_fn`, so the additive half of the +/// round (including a wallet-relevant winner's own record) is already +/// staged when the removal decides which links point at a dead +/// transaction. Batches arrive in emission order and must be applied in +/// sequence; see [`SweepBatchFFI`]. A non-zero return fails the round like +/// any other per-kind callback — a deletion silently skipped would let +/// Rust clear the sweep while the dead row survives. +pub type PersistWalletChangesetSweepsFn = unsafe extern "C" fn( + context: *mut c_void, + wallet_id: *const u8, + sweeps: *const SweepBatchFFI, + sweeps_count: usize, +) -> i32; + +/// Carries the NUMERIC block height of the round's applied chainlock — +/// the same watermark whose bincode blob rides +/// `WalletChangeSetFFI::last_applied_chain_lock_bytes`, which is opaque to +/// a non-Rust host. The height is one half of the sweep-tombstone +/// collection boundary `min(chainlock_height, synced_height)` (see +/// [`SweepBatchFFI::winner_mined_height`]); without it a host either +/// cannot collect at all or has to guess from the synced height alone, +/// which is not finality. Fired inside the round's begin/end bracket, +/// after `on_persist_wallet_changeset_fn`, only when the round advanced +/// the chainlock watermark. Monotonic-max semantics at the host: chain +/// locks only move forward, so store `max(stored, incoming)`. A non-zero +/// return fails the round like any other per-kind callback. +pub type PersistWalletChangesetChainLockHeightFn = + unsafe extern "C" fn(context: *mut c_void, wallet_id: *const u8, chain_lock_height: u32) -> i32; + /// Size- and version-tagged additive persistence callbacks. /// /// `context` is the context in the accompanying [`PersistenceCallbacks`] @@ -244,6 +280,39 @@ pub struct PersistenceCallbacksExtension { pub on_load_tracked_masternodes_free_fn: Option< unsafe extern "C" fn(context: *mut c_void, rows: *const TrackedMasternodeFFI, count: usize), >, + /// The round's sweep batches (see [`PersistWalletChangesetSweepsFn`]). + /// Lives here rather than on [`WalletChangeSetFFI`] because that struct + /// crosses by bare pointer with no size field: appending the batches + /// there would let a newer callback dereference fields an older native + /// producer never allocated. Appended under the same version — the + /// version names the stable field ordering, and `struct_size` is what + /// proves how much of it a given host actually supplied: Rust reads + /// this slot only when the host's declared size covers it, so an older + /// extension simply never has its sweeps read rather than being + /// rejected outright (which a version bump would do, taking its DPNS + /// callback down with it). + pub on_persist_wallet_changeset_sweeps_fn: Option< + unsafe extern "C" fn( + context: *mut c_void, + wallet_id: *const u8, + sweeps: *const SweepBatchFFI, + sweeps_count: usize, + ) -> i32, + >, + /// The round's numeric chainlock height (see + /// [`PersistWalletChangesetChainLockHeightFn`]). Appended under the + /// same version for the same reason as the sweeps slot above: + /// `struct_size` proves whether a host allocated it, and a host that + /// did not simply never has it read. Purely additive — a host without + /// it keeps working, it just cannot compute the tombstone-collection + /// finality boundary and must hold its tombstones instead. + pub on_persist_wallet_changeset_chain_lock_height_fn: Option< + unsafe extern "C" fn( + context: *mut c_void, + wallet_id: *const u8, + chain_lock_height: u32, + ) -> i32, + >, } impl Default for PersistenceCallbacksExtension { @@ -256,6 +325,8 @@ impl Default for PersistenceCallbacksExtension { on_persist_tracked_masternodes_fn: None, on_load_tracked_masternodes_fn: None, on_load_tracked_masternodes_free_fn: None, + on_persist_wallet_changeset_sweeps_fn: None, + on_persist_wallet_changeset_chain_lock_height_fn: None, } } } @@ -269,6 +340,8 @@ pub struct PersistenceExtensionCallbacks { pub persist_tracked_masternodes: Option, pub load_tracked_masternodes: Option, pub load_tracked_masternodes_free: Option, + pub wallet_changeset_sweeps: Option, + pub wallet_changeset_chain_lock_height: Option, } /// C callback vtable for wallet persistence. @@ -1046,6 +1119,18 @@ pub struct FFIPersister { callbacks: PersistenceCallbacks, /// Additive callbacks negotiated outside the legacy unsized vtable. dpns_name_states_callback: Option, + /// `Some` only when the host's extension `struct_size` proved the slot + /// was allocated (see `persistence_extension_sweeps_callback` in + /// `manager.rs`) — which is also what makes it a real structural + /// attestation of `CORE_SWEEP_REMOVAL`, unlike the legacy changeset + /// callback whose unchanged signature proves nothing. + wallet_changeset_sweeps_callback: Option, + /// `Some` only when the host's extension `struct_size` proved the slot + /// was allocated. Carries the numeric chainlock height a non-Rust host + /// cannot read out of the bincode blob on the changeset struct; a host + /// without it simply never collects sweep tombstones (safe — held, not + /// leaked to the unspent set). + wallet_changeset_chain_lock_height_callback: Option, /// Additive tracked-masternode persistence trio (persist / load / /// free), likewise extension-negotiated. tracked_masternodes_callbacks: PersistenceExtensionCallbacks, @@ -1112,12 +1197,46 @@ impl FFIPersister { callbacks: PersistenceCallbacks, declared_capabilities: PersistenceCapabilities, dpns_name_states_callback: Option, + ) -> Self { + Self::new_with_persistence_capabilities_and_extension_callbacks( + callbacks, + declared_capabilities, + dpns_name_states_callback, + None, + ) + } + + pub fn new_with_persistence_capabilities_and_extension_callbacks( + callbacks: PersistenceCallbacks, + declared_capabilities: PersistenceCapabilities, + dpns_name_states_callback: Option, + wallet_changeset_sweeps_callback: Option, + ) -> Self { + Self::new_with_persistence_capabilities_and_all_extension_callbacks( + callbacks, + declared_capabilities, + dpns_name_states_callback, + wallet_changeset_sweeps_callback, + None, + ) + } + + pub fn new_with_persistence_capabilities_and_all_extension_callbacks( + callbacks: PersistenceCallbacks, + declared_capabilities: PersistenceCapabilities, + dpns_name_states_callback: Option, + wallet_changeset_sweeps_callback: Option, + wallet_changeset_chain_lock_height_callback: Option< + PersistWalletChangesetChainLockHeightFn, + >, ) -> Self { Self::new_with_persistence_capabilities_and_extensions( callbacks, declared_capabilities, PersistenceExtensionCallbacks { dpns_name_states: dpns_name_states_callback, + wallet_changeset_sweeps: wallet_changeset_sweeps_callback, + wallet_changeset_chain_lock_height: wallet_changeset_chain_lock_height_callback, ..Default::default() }, ) @@ -1131,6 +1250,9 @@ impl FFIPersister { Self { callbacks, dpns_name_states_callback: extensions.dpns_name_states, + wallet_changeset_sweeps_callback: extensions.wallet_changeset_sweeps, + wallet_changeset_chain_lock_height_callback: extensions + .wallet_changeset_chain_lock_height, tracked_masternodes_callbacks: extensions, declared_capabilities, pending: RwLock::new(BTreeMap::new()), @@ -1192,6 +1314,48 @@ impl FFIPersister { if self.callbacks.on_persist_token_balances_fn.is_some() { capabilities = capabilities.union(PersistenceCapabilities::UNSIGNED_TOKEN_STORAGE); } + // The dashpay-payments slot is what the sweep's Failed flip rides + // (`dashpay_payments_overlay` on the store round). A host that + // never wired it — Android deliberately keeps payment recording + // in-memory-only — must not read as payment-durable, or the + // wallet-event adapter would couple the flip to a round that + // silently drops it: the accepted-and-ignored shape the sweep + // bit's own gating exists to prevent, reproduced one channel over. + if self.callbacks.on_persist_dashpay_payments_fn.is_some() { + capabilities = capabilities.union(PersistenceCapabilities::DASHPAY_PAYMENTS); + } + // Sweeps travel through the size-tagged extension callback, so — + // unlike the legacy `on_persist_wallet_changeset_fn`, whose + // unchanged C signature proves nothing about what a host actually + // reads — this slot being `Some` is a genuine structural + // attestation: it exists only when the host's declared extension + // `struct_size` covered the field. The changeset callback is still + // required alongside it because a sweep only corrects state that + // callback persists; a sweeps slot with no changeset slot would + // attest removals against rows the host never writes. The bit is + // still additionally gated by `declared_capabilities` in + // `persistence_capabilities()` below, like every other bit: the + // host must attest the semantic contract, not just wire pointers. + // + // The begin/end pair and `ATOMIC_CHANGESETS` are required on top, + // and only for this bit, because moving sweeps onto their own slot + // split one logical `CoreChangeSet` across two calls. Without a + // round that commits or rolls back as a unit, the changeset call + // can make the watermark and the additive rows durable and the + // process can stop before the sweep call applies the removal — + // leaving a host that restarts past a deletion it never performed + // and reloads the dead transaction. Nothing before sweeps could + // fail this way: every core field arrived through one callback. + if self.wallet_changeset_sweeps_callback.is_some() + && self.callbacks.on_persist_wallet_changeset_fn.is_some() + && self.callbacks.on_changeset_begin_fn.is_some() + && self.callbacks.on_changeset_end_fn.is_some() + && self + .declared_capabilities + .contains(PersistenceCapabilities::ATOMIC_CHANGESETS) + { + capabilities = capabilities.union(PersistenceCapabilities::CORE_SWEEP_REMOVAL); + } #[cfg(feature = "shielded")] if self.callbacks.on_persist_shielded_viewing_keys_fn.is_some() && self.callbacks.on_load_shielded_viewing_keys_fn.is_some() @@ -1647,6 +1811,60 @@ impl PlatformWalletPersistence for FFIPersister { round_success = false; } } + + // The numeric chainlock height rides its own size-negotiated + // extension slot for the same layout reason the sweeps below do: + // the bincode blob on the changeset struct is opaque to a + // non-Rust host, and the frozen `WalletChangeSetFFI` cannot grow + // a numeric field. Fired before the sweeps so a round carrying + // both has the boundary stored before any tombstone the sweep + // writes could be measured against it. + if let Some(cl) = core_cs.last_applied_chain_lock.as_ref() { + if let Some(cb) = self.wallet_changeset_chain_lock_height_callback { + let result = + unsafe { cb(self.callbacks.context, wallet_id.as_ptr(), cl.block_height) }; + if result != 0 { + eprintln!( + "Wallet changeset chainlock-height persistence callback returned \ + error code {}", + result + ); + round_success = false; + } + } + } + + // The round's sweeps ride their own size-negotiated extension + // callback rather than the changeset struct (see the layout note + // on `WalletChangeSetFFI`), fired immediately after it — still + // inside the same begin/end bracket — so the additive half of + // the round, a wallet-relevant winner's own record included, is + // already staged when the removal decides which links point at a + // dead transaction. A host without the slot simply never sees + // them; that is safe to leave silent here because such a host + // can never attest `CORE_SWEEP_REMOVAL`, and the core bridge + // already freezes the sync watermark for a sweep-carrying round + // against a persister without that capability. + if !core_cs.sweeps.is_empty() { + if let Some(cb) = self.wallet_changeset_sweeps_callback { + let (batches, _batch_storage) = build_sweep_batches_for_callback(core_cs); + let result = unsafe { + cb( + self.callbacks.context, + wallet_id.as_ptr(), + batches.as_ptr(), + batches.len(), + ) + }; + if result != 0 { + eprintln!( + "Wallet changeset sweeps persistence callback returned error code {}", + result + ); + round_success = false; + } + } + } } // Send identity scalar changeset — upserts and removals. @@ -6305,6 +6523,22 @@ mod tests { ) -> i32 { 0 } + unsafe extern "C" fn noop_dashpay_payments( + _ctx: *mut c_void, + _wallet_id: *const u8, + _entries: *const DashpayPaymentPersistEntryFFI, + _count: usize, + ) -> i32 { + 0 + } + unsafe extern "C" fn noop_wallet_changeset_sweeps( + _ctx: *mut c_void, + _wallet_id: *const u8, + _sweeps: *const SweepBatchFFI, + _sweeps_count: usize, + ) -> i32 { + 0 + } unsafe extern "C" fn noop_token_balances( _ctx: *mut c_void, _wallet_id: *const u8, @@ -6650,6 +6884,415 @@ mod tests { assert!(!capabilities.contains(PersistenceCapabilities::WALLET_RESTORE)); } + /// `CORE_SWEEP_REMOVAL` requires the extension's size-negotiated + /// sweeps slot, the legacy changeset callback it corrects, AND the + /// host's explicit declaration. The legacy callback alone must never + /// attest it: its C signature never changed, so an out-of-tree host + /// built before sweeps existed still has that pointer wired — the + /// extension slot is the only structural fact that distinguishes a + /// sweep-aware host, because it exists only when the host's declared + /// `struct_size` proved it. + /// `DASHPAY_PAYMENTS` requires the payments slot AND the declaration — + /// the flip channel's mirror of the sweep bit's gating. Android's + /// vtable leaves `on_persist_dashpay_payments_fn` unset, so even a + /// host blindly OR-ing the bit must read as payments-blind: the + /// wallet-event adapter keys the sweep's Failed-flip staging on this + /// bit, and an accepted-and-dropped overlay is exactly the shape the + /// gating exists to prevent. + #[test] + fn dashpay_payments_requires_the_slot_and_the_declaration() { + fn persister_with( + callbacks: PersistenceCallbacks, + declared: PersistenceCapabilities, + ) -> FFIPersister { + FFIPersister::new_with_persistence_capabilities(callbacks, declared) + } + // Declared but slot unwired (the Android shape): absent. + assert!(!persister_with( + PersistenceCallbacks::default(), + PersistenceCapabilities::DASHPAY_PAYMENTS + ) + .persistence_capabilities() + .contains(PersistenceCapabilities::DASHPAY_PAYMENTS)); + + // Slot wired but never declared: absent. + assert!(!persister_with( + PersistenceCallbacks { + on_persist_dashpay_payments_fn: Some(noop_dashpay_payments), + ..Default::default() + }, + PersistenceCapabilities::NONE + ) + .persistence_capabilities() + .contains(PersistenceCapabilities::DASHPAY_PAYMENTS)); + + // Wired and declared: attested. + assert!(persister_with( + PersistenceCallbacks { + on_persist_dashpay_payments_fn: Some(noop_dashpay_payments), + ..Default::default() + }, + PersistenceCapabilities::DASHPAY_PAYMENTS + ) + .persistence_capabilities() + .contains(PersistenceCapabilities::DASHPAY_PAYMENTS)); + } + + #[test] + fn core_sweep_removal_requires_the_extension_slot_and_the_declaration() { + fn persister_with( + callbacks: PersistenceCallbacks, + declared: PersistenceCapabilities, + sweeps: Option, + ) -> FFIPersister { + FFIPersister::new_with_persistence_capabilities_and_extension_callbacks( + callbacks, declared, None, sweeps, + ) + } + fn wired_callbacks() -> PersistenceCallbacks { + PersistenceCallbacks { + on_persist_wallet_changeset_fn: Some(noop_wallet_changeset), + on_changeset_begin_fn: Some(noop_begin), + on_changeset_end_fn: Some(noop_end), + ..Default::default() + } + } + /// Everything the bit needs except the atomic round. + fn declared() -> PersistenceCapabilities { + PersistenceCapabilities::CORE_SWEEP_REMOVAL + .union(PersistenceCapabilities::ATOMIC_CHANGESETS) + } + + // The pre-sweep-aware binary shape: legacy changeset callback + // wired, declaration present (a host blindly OR-ing bits), but no + // extension slot — absent. + assert!(!persister_with(wired_callbacks(), declared(), None) + .persistence_capabilities() + .contains(PersistenceCapabilities::CORE_SWEEP_REMOVAL)); + + // Extension slot wired and declared, but no changeset callback to + // persist the rows a sweep would correct: absent. + assert!(!persister_with( + PersistenceCallbacks::default(), + declared(), + Some(noop_wallet_changeset_sweeps) + ) + .persistence_capabilities() + .contains(PersistenceCapabilities::CORE_SWEEP_REMOVAL)); + + // Structurally complete but never declared: absent. + assert!(!persister_with( + wired_callbacks(), + PersistenceCapabilities::NONE, + Some(noop_wallet_changeset_sweeps) + ) + .persistence_capabilities() + .contains(PersistenceCapabilities::CORE_SWEEP_REMOVAL)); + + // Structurally complete and declared, but without the atomic round + // the split transport needs: absent. Sweeps arrive on their own + // call, so a host with no begin/end boundary can make the changeset + // durable and stop before the removal lands. + assert!(!persister_with( + wired_callbacks(), + PersistenceCapabilities::CORE_SWEEP_REMOVAL, + Some(noop_wallet_changeset_sweeps) + ) + .persistence_capabilities() + .contains(PersistenceCapabilities::CORE_SWEEP_REMOVAL)); + + // Declared atomic, but the begin/end pair is not actually wired: + // absent. The declaration alone cannot bracket the two calls. + assert!(!persister_with( + PersistenceCallbacks { + on_persist_wallet_changeset_fn: Some(noop_wallet_changeset), + ..Default::default() + }, + declared(), + Some(noop_wallet_changeset_sweeps) + ) + .persistence_capabilities() + .contains(PersistenceCapabilities::CORE_SWEEP_REMOVAL)); + + // Everything present: attested. + assert!(persister_with( + wired_callbacks(), + declared(), + Some(noop_wallet_changeset_sweeps) + ) + .persistence_capabilities() + .contains(PersistenceCapabilities::CORE_SWEEP_REMOVAL)); + } + + /// The delivery contract of the extension transport itself: a + /// sweep-carrying round hands its batches to the extension slot AFTER + /// the changeset callback, within the same round, in emission order and + /// with payloads intact — order is the one property a persister cannot + /// reconstruct, since a later batch can keep a coin spent that an + /// earlier one freed. The same round against a persister whose + /// extension never proved the slot must still succeed with the sweeps + /// simply undelivered: the adapter's `CORE_SWEEP_REMOVAL` gate is what + /// turns that into a withheld watermark rather than a false success. + #[test] + fn store_delivers_sweeps_through_the_extension_slot_after_the_changeset() { + use dashcore::hashes::Hash as _; + use platform_wallet::changeset::changeset::SweepBatch; + use platform_wallet::changeset::CoreChangeSet; + + #[derive(Default)] + struct Sink { + events: std::sync::Mutex>, + } + unsafe extern "C" fn record_changeset( + ctx: *mut c_void, + _wallet_id: *const u8, + _changeset: *const WalletChangeSetFFI, + ) -> i32 { + let sink = &*(ctx as *const Sink); + sink.events.lock().unwrap().push("changeset".into()); + 0 + } + unsafe extern "C" fn record_sweeps( + ctx: *mut c_void, + _wallet_id: *const u8, + sweeps: *const SweepBatchFFI, + sweeps_count: usize, + ) -> i32 { + let sink = &*(ctx as *const Sink); + let mut events = sink.events.lock().unwrap(); + for batch in slice::from_raw_parts(sweeps, sweeps_count) { + let txids = slice::from_raw_parts(batch.txids, batch.txids_count); + let released = if batch.released_outpoints.is_null() { + &[][..] + } else { + slice::from_raw_parts(batch.released_outpoints, batch.released_outpoints_count) + }; + let winner_height = if batch.has_winner_mined_height { + format!("Some({})", batch.winner_mined_height) + } else { + "None".to_string() + }; + events.push(format!( + "sweep txids={:?} winner={} height={} released={:?}", + txids.iter().map(|t| t[0]).collect::>(), + batch.superseded_by[0], + winner_height, + released + .iter() + .map(|o| (o.txid[0], o.vout)) + .collect::>(), + )); + } + 0 + } + + fn sweep_changeset() -> PlatformWalletChangeSet { + PlatformWalletChangeSet { + core: Some(CoreChangeSet { + sweeps: vec![ + // Block-context: the winner's mined height crosses. + SweepBatch { + txids: vec![dashcore::Txid::from_byte_array([0x11; 32])], + superseded_by: dashcore::Txid::from_byte_array([0x22; 32]), + winner_mined_height: Some(910), + released_outpoints: vec![dashcore::OutPoint::new( + dashcore::Txid::from_byte_array([0x33; 32]), + 7, + )], + }, + // IS-locked winner: no height — the consumer must + // see the absence, not a fabricated zero. + SweepBatch { + txids: vec![ + dashcore::Txid::from_byte_array([0x44; 32]), + dashcore::Txid::from_byte_array([0x55; 32]), + ], + superseded_by: dashcore::Txid::from_byte_array([0x66; 32]), + winner_mined_height: None, + released_outpoints: vec![], + }, + ], + ..Default::default() + }), + ..Default::default() + } + } + + let sink = Sink::default(); + let callbacks = PersistenceCallbacks { + context: &sink as *const Sink as *mut c_void, + on_persist_wallet_changeset_fn: Some(record_changeset), + ..PersistenceCallbacks::default() + }; + let persister = FFIPersister::new_with_persistence_capabilities_and_extension_callbacks( + callbacks, + PersistenceCapabilities::CORE_SWEEP_REMOVAL, + None, + Some(record_sweeps), + ); + persister + .store([1u8; 32], sweep_changeset()) + .expect("sweep round must succeed"); + assert_eq!( + sink.events.lock().unwrap().clone(), + vec![ + "changeset".to_string(), + "sweep txids=[17] winner=34 height=Some(910) released=[(51, 7)]".to_string(), + "sweep txids=[68, 85] winner=102 height=None released=[]".to_string(), + ], + ); + drop(persister); + + // No extension slot: the round still succeeds, the changeset + // callback still fires, and the sweeps are never delivered — the + // legacy-host shape, safe because such a persister can never attest + // CORE_SWEEP_REMOVAL (see the capability test above). + let sink = Sink::default(); + let callbacks = PersistenceCallbacks { + context: &sink as *const Sink as *mut c_void, + on_persist_wallet_changeset_fn: Some(record_changeset), + ..PersistenceCallbacks::default() + }; + let persister = FFIPersister::new_with_persistence_capabilities( + callbacks, + PersistenceCapabilities::NONE, + ); + persister + .store([1u8; 32], sweep_changeset()) + .expect("sweepless-host round must still succeed"); + assert_eq!( + sink.events.lock().unwrap().clone(), + vec!["changeset".to_string()] + ); + drop(persister); + } + + /// The numeric chainlock height reaches the host through its own + /// size-negotiated extension slot: a chainlock-advancing round fires it + /// after the changeset callback with the height a non-Rust host cannot + /// read out of the bincode blob, a round with no chainlock never fires + /// it, and a host without the slot still succeeds — it just never + /// learns the finality boundary and must hold its sweep tombstones. + #[test] + fn store_delivers_the_chainlock_height_through_the_extension_slot() { + use platform_wallet::changeset::CoreChangeSet; + + #[derive(Default)] + struct Sink { + events: std::sync::Mutex>, + } + unsafe extern "C" fn record_changeset( + ctx: *mut c_void, + _wallet_id: *const u8, + _changeset: *const WalletChangeSetFFI, + ) -> i32 { + let sink = &*(ctx as *const Sink); + sink.events.lock().unwrap().push("changeset".into()); + 0 + } + unsafe extern "C" fn record_chain_lock_height( + ctx: *mut c_void, + _wallet_id: *const u8, + chain_lock_height: u32, + ) -> i32 { + let sink = &*(ctx as *const Sink); + sink.events + .lock() + .unwrap() + .push(format!("chain_lock_height={chain_lock_height}")); + 0 + } + fn chain_lock_at(height: u32) -> dashcore::ephemerealdata::chain_lock::ChainLock { + use dashcore::bls_sig_utils::BLSSignature; + use dashcore::hashes::Hash as _; + use dashcore::BlockHash; + dashcore::ephemerealdata::chain_lock::ChainLock { + block_height: height, + block_hash: BlockHash::from_byte_array([0xCC; 32]), + signature: BLSSignature::from([0u8; 96]), + } + } + + let sink = Sink::default(); + let callbacks = PersistenceCallbacks { + context: &sink as *const Sink as *mut c_void, + on_persist_wallet_changeset_fn: Some(record_changeset), + ..PersistenceCallbacks::default() + }; + let persister = FFIPersister::new_with_persistence_capabilities_and_all_extension_callbacks( + callbacks, + PersistenceCapabilities::NONE, + None, + None, + Some(record_chain_lock_height), + ); + // A round with no chainlock: the slot stays silent. + persister + .store( + [1u8; 32], + PlatformWalletChangeSet { + core: Some(CoreChangeSet { + synced_height: Some(10), + ..Default::default() + }), + ..Default::default() + }, + ) + .expect("chainlock-less round must succeed"); + // A chainlock-advancing round: the numeric height crosses, after + // the changeset callback. + persister + .store( + [1u8; 32], + PlatformWalletChangeSet { + core: Some(CoreChangeSet { + last_applied_chain_lock: Some(chain_lock_at(4_242)), + ..Default::default() + }), + ..Default::default() + }, + ) + .expect("chainlock round must succeed"); + assert_eq!( + sink.events.lock().unwrap().clone(), + vec![ + "changeset".to_string(), + "changeset".to_string(), + "chain_lock_height=4242".to_string(), + ], + ); + drop(persister); + + // Host without the slot: the same round still succeeds. + let sink = Sink::default(); + let callbacks = PersistenceCallbacks { + context: &sink as *const Sink as *mut c_void, + on_persist_wallet_changeset_fn: Some(record_changeset), + ..PersistenceCallbacks::default() + }; + let persister = FFIPersister::new_with_persistence_capabilities( + callbacks, + PersistenceCapabilities::NONE, + ); + persister + .store( + [1u8; 32], + PlatformWalletChangeSet { + core: Some(CoreChangeSet { + last_applied_chain_lock: Some(chain_lock_at(4_242)), + ..Default::default() + }), + ..Default::default() + }, + ) + .expect("slotless-host chainlock round must still succeed"); + assert_eq!( + sink.events.lock().unwrap().clone(), + vec!["changeset".to_string()] + ); + drop(persister); + } + #[test] fn asset_lock_reconciliation_requires_every_callback_leg() { fn complete_callbacks() -> PersistenceCallbacks { @@ -6704,7 +7347,8 @@ mod tests { .union(PersistenceCapabilities::PROVIDER_TRANSACTIONS) .union(PersistenceCapabilities::UNSIGNED_TOKEN_STORAGE) .union(PersistenceCapabilities::WALLET_RESTORE) - .union(PersistenceCapabilities::TRACKED_ASSET_LOCKS); + .union(PersistenceCapabilities::TRACKED_ASSET_LOCKS) + .union(PersistenceCapabilities::CORE_SWEEP_REMOVAL); cb.on_changeset_begin_fn = Some(noop_begin); cb.on_changeset_end_fn = Some(noop_end); cb.on_persist_account_registrations_fn = Some(noop_registrations); @@ -6715,7 +7359,15 @@ mod tests { cb.on_load_wallet_list_free_fn = Some(noop_free_wallets); cb.on_persist_wallet_changeset_fn = Some(noop_wallet_changeset); cb.on_persist_token_balances_fn = Some(noop_token_balances); - let capabilities = declared_persister(cb, expected).persistence_capabilities(); + // "Fully wired" includes the extension's sweeps slot — the legacy + // vtable alone can no longer attest CORE_SWEEP_REMOVAL. + let capabilities = FFIPersister::new_with_persistence_capabilities_and_extension_callbacks( + cb, + expected, + None, + Some(noop_wallet_changeset_sweeps), + ) + .persistence_capabilities(); assert_eq!(capabilities, expected); assert!(capabilities.contains(PersistenceCapabilities::INVITATION_CREATION)); @@ -6784,24 +7436,71 @@ mod tests { std::mem::size_of::() ); assert_eq!(PLATFORM_WALLET_PERSISTENCE_CALLBACKS_EXTENSION_VERSION, 1); - // The extension grows ADDITIVELY under version 1 (size-gated - // reads); pin the current field order and terminal slot so an - // accidental reorder — which would silently misread every older - // host's callbacks — fails here. - assert!( + // The extension is append-only under version 1 (size-gated reads): + // pin the exact slot adjacency so every historical struct_size + // boundary keeps meaning what it meant when a host declared it. + // The DPNS slot's end is exactly where the tracked-masternode trio + // begins (mainline shipped the trio at those offsets before the + // sweeps/chainlock slots merged in, so the trio keeps them), the + // trio's end is where the sweeps slot begins, the sweeps slot's + // end is where the chainlock-height slot begins, and the + // chainlock-height slot is currently terminal. Reordering any of + // them would silently misread every extension already in the field. + assert_eq!( std::mem::offset_of!( PersistenceCallbacksExtension, on_persist_dpns_name_states_fn - ) < std::mem::offset_of!( + ) + std::mem::size_of::>(), + std::mem::offset_of!( PersistenceCallbacksExtension, on_persist_tracked_masternodes_fn ) ); + assert_eq!( + std::mem::offset_of!( + PersistenceCallbacksExtension, + on_persist_tracked_masternodes_fn + ) + std::mem::size_of::>(), + std::mem::offset_of!( + PersistenceCallbacksExtension, + on_load_tracked_masternodes_fn + ) + ); + assert_eq!( + std::mem::offset_of!( + PersistenceCallbacksExtension, + on_load_tracked_masternodes_fn + ) + std::mem::size_of::>(), + std::mem::offset_of!( + PersistenceCallbacksExtension, + on_load_tracked_masternodes_free_fn + ) + ); assert_eq!( std::mem::offset_of!( PersistenceCallbacksExtension, on_load_tracked_masternodes_free_fn ) + std::mem::size_of::>(), + std::mem::offset_of!( + PersistenceCallbacksExtension, + on_persist_wallet_changeset_sweeps_fn + ) + ); + assert_eq!( + std::mem::offset_of!( + PersistenceCallbacksExtension, + on_persist_wallet_changeset_sweeps_fn + ) + std::mem::size_of::>(), + std::mem::offset_of!( + PersistenceCallbacksExtension, + on_persist_wallet_changeset_chain_lock_height_fn + ) + ); + assert_eq!( + std::mem::offset_of!( + PersistenceCallbacksExtension, + on_persist_wallet_changeset_chain_lock_height_fn + ) + std::mem::size_of::>(), std::mem::size_of::() ); assert_eq!( @@ -6848,6 +7547,18 @@ mod tests { PLATFORM_WALLET_PERSISTENCE_CAPABILITY_TRACKED_ASSET_LOCKS, PersistenceCapabilities::TRACKED_ASSET_LOCKS.bits() ); + assert_eq!( + PLATFORM_WALLET_PERSISTENCE_CAPABILITY_TRACKED_MASTERNODES, + PersistenceCapabilities::TRACKED_MASTERNODES.bits() + ); + assert_eq!( + PLATFORM_WALLET_PERSISTENCE_CAPABILITY_CORE_SWEEP_REMOVAL, + PersistenceCapabilities::CORE_SWEEP_REMOVAL.bits() + ); + assert_eq!( + PLATFORM_WALLET_PERSISTENCE_CAPABILITY_DASHPAY_PAYMENTS, + PersistenceCapabilities::DASHPAY_PAYMENTS.bits() + ); assert_eq!( PLATFORM_WALLET_PERSISTENCE_CAPABILITY_ACCOUNT_ADDRESS_POOLS, PLATFORM_WALLET_PERSISTENCE_CAPABILITY_ASSET_LOCK_FUNDING_INDICES diff --git a/packages/rs-platform-wallet/src/changeset/changeset.rs b/packages/rs-platform-wallet/src/changeset/changeset.rs index d36fcfdc76b..16e5fe13570 100644 --- a/packages/rs-platform-wallet/src/changeset/changeset.rs +++ b/packages/rs-platform-wallet/src/changeset/changeset.rs @@ -63,12 +63,13 @@ use crate::wallet::identity::{ /// `WalletEvent` bus delivers. /// /// Built by the platform-wallet event adapter from `WalletEvent` variants -/// emitted by `WalletManager`. The merge implementation coalesces the -/// record vecs newest-wins (by txid for the wallet-level `records`, by +/// emitted by `WalletManager`. Every field is additive except +/// [`Self::sweeps`]. The merge implementation coalesces the record vecs +/// newest-wins (by txid for the wallet-level `records`, by /// `(txid, account)` for `account_records` — see /// [`fold_same_txid_records`]), uses monotonic-max for the height -/// watermarks, `extend` for the utxo vecs, and last-write-wins for the -/// IS-lock map. +/// watermarks, `extend` for the utxo vecs and for `sweeps` (in emission +/// order — see the field), and last-write-wins for the IS-lock map. /// /// # Why a projection instead of the upstream type /// @@ -232,6 +233,80 @@ pub struct CoreChangeSet { /// lower height never overwrites a higher one — chain locks are /// strictly forward-advancing per upstream's contract). pub last_applied_chain_lock: Option, + + /// Sweeps this batch carries, in the order the wallet emitted them. + /// + /// The one subtractive part of this type. Every other field is additive, + /// which is exactly why this one has to exist: a persister that only ever + /// appends keeps the dead rows and replays them on the next load, + /// re-creating a balance the wallet has already corrected. + /// + /// Kept as ordered batches rather than folded into one removal list plus + /// one release set. Each sweep describes the wallet at the moment it + /// fired, and those descriptions can disagree: an early sweep frees a + /// coin, something later spends it, and a later sweep removes that + /// spender while keeping the coin spent because its own winner took it. + /// Union the release sets and the first answer outlives the last one that + /// is actually true. Applied in order, each batch corrects the one before + /// it, which is what the wallet itself did. + /// `serde(default)`: this field postdates the serialized representation, + /// so a payload written before it necessarily omits it. An empty vec is + /// the exact backward-compatible reading — a changeset from then could + /// not have carried a sweep. + #[cfg_attr(feature = "serde", serde(default))] + pub sweeps: Vec, +} + +/// One `TransactionsSwept` event: the transactions it removed, the +/// transaction that beat them, and the coins its removal actually freed. +/// +/// The grouping is what makes ordering expressible. `released_outpoints` is +/// only true relative to the wallet as this event saw it, so it belongs with +/// the removals it came from rather than in a set shared with every other +/// sweep in the batch. +#[derive(Debug, Clone, PartialEq, Eq)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +pub struct SweepBatch { + /// The removed transactions. Their rows and every UTXO they created go. + pub txids: Vec, + /// The transaction whose arrival settled the inputs — final, and + /// therefore the reason the removed ones can never confirm. Not + /// necessarily wallet-relevant: it can pay entirely to outside addresses + /// and still sweep, which is why it cannot be looked up to work out what + /// it took. + pub superseded_by: Txid, + /// Mined height of `superseded_by` when the sweep was triggered by its + /// arrival in a block; `None` when it was triggered by an + /// InstantSend-locked winner still waiting to be mined (upstream's only + /// two triggers — an unlocked mempool arrival never sweeps). + /// + /// This is the winner's finality context, straight from the event: the + /// winner need not be wallet-relevant, so no persister can look its + /// height up in its own records. A held-but-unfunded input is mirrored + /// as a durable placeholder in EITHER case; this field decides the + /// placeholder's lifetime. `Some` stamps the winner's own block height + /// — the projection of upstream's `observed_spent_outpoints` — and the + /// placeholder is collectible once `min(chainlock_height, + /// synced_height)` reaches it, exactly upstream's + /// `prune_finalized_observed_spends` boundary. `None` (IS-locked + /// winner, unmined) leaves the placeholder UNSTAMPED and never + /// collectible: under DIP-10 the lock alone settles the input — + /// upstream retains it in the account's `spent_outpoints`, a hold with + /// no height that no record survives to rebuild — and an IS-locked + /// winner has no mining deadline, so no watermark can ever prove the + /// funding output delivered-or-never. An unstamped placeholder + /// resolves only through proof: funding materialisation, a later + /// block-context sweep's re-stamp, or a release. + /// + /// `serde(default)`: a journaled payload written before this field + /// existed reads back as `None` — the conservative reading (no new + /// placeholder, existing stamps kept). + #[cfg_attr(feature = "serde", serde(default))] + pub winner_mined_height: Option, + /// Of the inputs those removed transactions claimed, the ones that came + /// free — no surviving transaction spends them too. Everything else they + /// claimed was taken by `superseded_by` and stays spent. + pub released_outpoints: Vec, } /// Highest-used derivation index per pool slot for one account, as @@ -610,11 +685,18 @@ impl Merge for CoreChangeSet { .or_default() .merge_max(indexes); } + + // Sweeps: appended, never folded. Order is the whole point — a later + // batch's decision to keep a coin spent has to survive an earlier + // batch's decision to free it, and only replaying them in sequence + // preserves that. + self.sweeps.extend(other.sweeps); } fn is_empty(&self) -> bool { self.records.is_empty() && self.account_records.is_empty() + && self.sweeps.is_empty() && self.spent_utxos.is_empty() && self.new_utxos.is_empty() && self.instant_locks_for_non_final_records.is_empty() @@ -2098,6 +2180,36 @@ impl Merge for PlatformWalletChangeSet { } } +#[cfg(all(test, feature = "serde"))] +mod serde_compat_tests { + use super::*; + + /// A changeset serialized before `sweeps` existed must still load. The + /// field postdates the representation, so an older payload simply omits + /// it — and an empty vec is the exact reading, since nothing back then + /// could have carried a sweep. Without `serde(default)` the whole + /// deserialization fails and every pre-sweep payload becomes unreadable. + #[test] + fn a_pre_sweep_payload_deserializes_with_no_sweeps() { + let json = r#"{ + "records": [], + "spent_utxos": [], + "new_utxos": [], + "instant_locks_for_non_final_records": {}, + "last_processed_height": 1000, + "synced_height": 900, + "account_highest_used": {}, + "last_applied_chain_lock": null + }"#; + + let cs: CoreChangeSet = + serde_json::from_str(json).expect("a pre-sweep payload must still deserialize"); + assert!(cs.sweeps.is_empty()); + assert_eq!(cs.last_processed_height, Some(1000)); + assert_eq!(cs.synced_height, Some(900)); + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/packages/rs-platform-wallet/src/changeset/merge.rs b/packages/rs-platform-wallet/src/changeset/merge.rs index 9c0d97fad27..e59045aec18 100644 --- a/packages/rs-platform-wallet/src/changeset/merge.rs +++ b/packages/rs-platform-wallet/src/changeset/merge.rs @@ -1,14 +1,20 @@ //! The `Merge` trait for composing changeset deltas. //! -//! Changesets are commutative and associative so that multiple deltas can be -//! batched and reordered without affecting the final result. +//! Changeset merging is an ORDERED, associative operation: a stream of +//! deltas may be folded together in any grouping, but only in the order +//! the deltas were produced. It is NOT commutative — `CoreChangeSet` is +//! the load-bearing example: its sweep batches append in emission order, +//! so a later batch's spend decision replays over an earlier one's +//! release. Reordering or parallelizing a fold can therefore persist a +//! different spend decision, not just a differently-arranged changeset. use std::collections::{BTreeMap, BTreeSet}; -/// Combine two changesets. Changesets are commutative and associative -/// for safe batching and reordering. +/// Combine two changesets: `self` is the earlier delta, `other` the later +/// one. Associative (safe to regroup a fold) but NOT commutative — see +/// the module doc; callers must keep operands in production order. pub trait Merge: Default { - /// Merge another changeset into `self`. + /// Merge `other`, the LATER delta, into `self`. fn merge(&mut self, other: Self); /// Returns `true` if this changeset contains no changes. diff --git a/packages/rs-platform-wallet/src/changeset/persistence_capabilities.rs b/packages/rs-platform-wallet/src/changeset/persistence_capabilities.rs index cd260cc5fae..86d059786a7 100644 --- a/packages/rs-platform-wallet/src/changeset/persistence_capabilities.rs +++ b/packages/rs-platform-wallet/src/changeset/persistence_capabilities.rs @@ -51,13 +51,46 @@ impl PersistenceCapabilities { /// Tracked asset-lock rows, including status and proof updates, can be /// persisted. Restart hydration is the separate `WALLET_RESTORE` contract. pub const TRACKED_ASSET_LOCKS: Self = Self(1 << 9); - /// Tracked (wallet-independent) masternodes are persisted AND restored /// across restarts /// ([`persist_tracked_masternodes`](super::PlatformWalletPersistence::persist_tracked_masternodes) /// / [`load_tracked_masternodes`](super::PlatformWalletPersistence::load_tracked_masternodes)). /// Without this bit, tracking is session-scoped. pub const TRACKED_MASTERNODES: Self = Self(1 << 10); + /// A stored `CoreChangeSet` whose `sweeps` are non-empty is durably + /// applied batch by batch and in order: each swept transaction and its + /// outputs are excluded from every restore and enumeration path (whether + /// by physical deletion or a durable marker), each released outpoint is + /// freed unless a later surviving claim supersedes that release, and each + /// non-released input retains a durable spend claim even when its funding + /// TXO has not materialized yet. Physical row deletion is an + /// implementation detail, not the contract — the in-tree stores keep an + /// inert globally-swept row until every wallet's scoped cleanup lands, + /// and a detached tombstone MUST outlive its loser or the consumed coin + /// later reads unspent. On the FFI surface sweeps travel through the + /// persistence extension's size-negotiated sweep callback — a slot Rust + /// never reads unless the host's declared `struct_size` proved it exists + /// — so an older host processes the rest of the round, returns success, + /// and never sees the sweeps at all; this bit tells the wallet that the + /// complete sweep contract was implemented rather than silently + /// truncated. + pub const CORE_SWEEP_REMOVAL: Self = Self(1 << 11); + /// A stored changeset's `dashpay_payments_overlay` rows are durably + /// applied. This is what lets the wallet-event adapter couple a sweep's + /// payment consequence (`Pending → Failed` for the losers' sent + /// entries) to the sweep's own atomic store round: the flip is staged + /// onto the round ONLY for a backend attesting this bit, because a + /// sweep never re-emits once its round is durable — an + /// accepted-and-ignored overlay would leave the adapter believing a + /// flip persisted that a host without a payments store silently + /// dropped. A non-attesting backend keeps the in-memory flip (the + /// truthful session state; the transaction IS dead) with nothing + /// round-coupled — funds-safe, since payment entries are display + /// metadata; the funds-critical half of the sweep still gates on + /// `CORE_SWEEP_REMOVAL`. On the FFI surface Rust honours the + /// declaration only when `on_persist_dashpay_payments_fn` is actually + /// wired. + pub const DASHPAY_PAYMENTS: Self = Self(1 << 12); /// Capabilities required before exporting and funding an invitation voucher. pub const INVITATION_CREATION: Self = Self( @@ -142,6 +175,14 @@ impl PersistenceCapabilities { PersistenceCapabilities::TRACKED_MASTERNODES, "tracked_masternodes", ), + ( + PersistenceCapabilities::CORE_SWEEP_REMOVAL, + "core_sweep_removal", + ), + ( + PersistenceCapabilities::DASHPAY_PAYMENTS, + "dashpay_payments", + ), ]; KNOWN @@ -172,6 +213,8 @@ mod tests { assert_eq!(PersistenceCapabilities::DPNS_NAME_STATES.bits(), 0x100); assert_eq!(PersistenceCapabilities::TRACKED_ASSET_LOCKS.bits(), 0x200); assert_eq!(PersistenceCapabilities::TRACKED_MASTERNODES.bits(), 0x400); + assert_eq!(PersistenceCapabilities::CORE_SWEEP_REMOVAL.bits(), 0x800); + assert_eq!(PersistenceCapabilities::DASHPAY_PAYMENTS.bits(), 0x1000); assert_eq!( PersistenceCapabilities::ASSET_LOCK_RECONCILIATION.bits(), 0x281 @@ -188,4 +231,21 @@ mod tests { vec!["asset_lock_funding_indices", "wallet_restore"] ); } + + /// Every declarable bit must be nameable. A bit missing from `KNOWN` + /// still gates behaviour but vanishes from every diagnostic that + /// reports capabilities by name, so a host debugging why its rows + /// never landed sees nothing about the capability that withheld them + /// — which is exactly what `DASHPAY_PAYMENTS` did until this test. + #[test] + fn every_declared_bit_has_a_stable_name() { + for shift in 0..13u32 { + let bit = PersistenceCapabilities::from_bits_retain(1 << shift); + assert_eq!( + bit.names().len(), + 1, + "bit 1 << {shift} is declarable but has no name in KNOWN" + ); + } + } } From ae1ce33859c0fc3c9bcb3c22bfbe426cff073960 Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Thu, 3 Sep 2026 11:11:26 +0300 Subject: [PATCH 2/5] docs(platform-wallet-ffi): name the reader that actually gates the sweeps slot The field doc pointed at `persistence_extension_sweeps_callback`, which does not exist. The negotiated read is `persistence_extension_callbacks` in `manager.rs`, through the `negotiated_extension_slot!` macro this PR introduces as the single gate authority. --- packages/rs-platform-wallet-ffi/src/persistence.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/rs-platform-wallet-ffi/src/persistence.rs b/packages/rs-platform-wallet-ffi/src/persistence.rs index 951994386d6..1cd0c4b3447 100644 --- a/packages/rs-platform-wallet-ffi/src/persistence.rs +++ b/packages/rs-platform-wallet-ffi/src/persistence.rs @@ -1120,10 +1120,12 @@ pub struct FFIPersister { /// Additive callbacks negotiated outside the legacy unsized vtable. dpns_name_states_callback: Option, /// `Some` only when the host's extension `struct_size` proved the slot - /// was allocated (see `persistence_extension_sweeps_callback` in - /// `manager.rs`) — which is also what makes it a real structural - /// attestation of `CORE_SWEEP_REMOVAL`, unlike the legacy changeset - /// callback whose unchanged signature proves nothing. + /// was allocated — read by `persistence_extension_callbacks` in + /// `manager.rs` through the `negotiated_extension_slot!` macro, which + /// is the single gate authority for every negotiated slot. That proof + /// is also what makes this a real structural attestation of + /// `CORE_SWEEP_REMOVAL`, unlike the legacy changeset callback whose + /// unchanged signature proves nothing. wallet_changeset_sweeps_callback: Option, /// `Some` only when the host's extension `struct_size` proved the slot /// was allocated. Carries the numeric chainlock height a non-Rust host From 3bd0973f16b3b163d629e7a659b336bf7da4f17a Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Thu, 3 Sep 2026 19:03:10 +0300 Subject: [PATCH 3/5] fix(platform-wallet-ffi): tie the slot gate to its field, and stop claiming what the seam does not do MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups on this PR. Two are real hazards, the rest are the code overstating itself. `negotiated_extension_slot!` took the slot's type as a second argument and sized the gate from it, so a copy-paste mismatch compiled and computed `callback_end` against the wrong width — refusing a slot the host allocated, or accepting a read past its allocation if the real field were wider. The gate now measures the field itself (`size_of_val`), so the slot being read and the size it is checked against are one fact; the parameter is gone, and a future slot may hold something that is not an `Option`. The header is read once for the whole negotiation instead of per slot. Six expansions each re-read `struct_size` and `version`, which let a host mutating its extension mid-`create` — or one whose struct lives in shared memory — have slot 1 negotiated against one declared layout and slot 6 against another, yielding a callback set that matched no declaration the host ever made. `SweepBatchFFI` sets both pointers null at count 0, and the reference consumer guarded only `released_outpoints`: `from_raw_parts(null, 0)` is undefined behaviour, not an empty slice, and this consumer is what a host binding copies. Both are guarded now, and the struct documents that either pointer may be null. `OutPointFFI`'s doc claimed every conversion routed through it while `record_utxos_ffi` — the producer of the very rows a release joins against — built the value by hand. It now goes through a shared `OutPointFFI::new(&Txid, u32)`, which the `From<&OutPoint>` impl also delegates to, so the claim is true rather than aspirational. Deleted `FFIPersister::pending`: nothing has ever read it, while every store deep-cloned the whole changeset into it and only `flush` cleared it — so on a host that rarely flushes it grew for the process lifetime, and this PR had just added `SweepBatch` to what it copies. Docs corrected where the same diff contradicted them: the chainlock height fires on every round carrying one (not only on an advance); the sweeps callback fires after that slot, not immediately after the changeset; a test helper's doc described the case below it; and the `names()` guard no longer claims a history `DASHPAY_PAYMENTS` cannot have, since this PR introduces the bit. `CORE_SWEEP_REMOVAL` now states what it does NOT cover. Retention is in the contract; collection is not. A host can attest the bit without wiring the chain-lock-height slot and will then hold stamped tombstones forever — correct, and the safe direction, but not "the complete sweep contract". Requiring the slot was the alternative and is worse: it would freeze the watermark of a host that implements removal but not collection. Tests: `every_declared_bit_has_a_stable_name` walked a hardcoded `0..13`, so the next bit added without a `KNOWN` entry would slip past the very guard it exists to be. The bound is now `HIGHEST_DECLARED_BIT`, declared beside the bit constants, plus an assertion that nothing above it is named — which is what catches a bit added without bumping it. And `a_populated_sweep_batch_round_trips` covers what the missing-field test cannot: that `Txid` and `OutPoint` serialize at all. The `serde(default)` note now scopes its own claim to self-describing encodings — bincode, which this workspace persists blobs with, cannot absorb an appended field — while noting nothing in-tree serializes a changeset today. --- .../src/core_wallet_types.rs | 47 +++++++---- .../rs-platform-wallet-ffi/src/manager.rs | 79 ++++++++++--------- .../rs-platform-wallet-ffi/src/persistence.rs | 63 ++++++++------- .../src/changeset/changeset.rs | 53 ++++++++++++- .../src/changeset/persistence_capabilities.rs | 50 ++++++++++-- 5 files changed, 201 insertions(+), 91 deletions(-) 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 86e6ce7311c..e8e717d0a18 100644 --- a/packages/rs-platform-wallet-ffi/src/core_wallet_types.rs +++ b/packages/rs-platform-wallet-ffi/src/core_wallet_types.rs @@ -15,18 +15,29 @@ pub struct OutPointFFI { pub vout: u32, } +impl OutPointFFI { + /// The one authority for building this value, for callers that hold a + /// txid and an index rather than an `OutPoint` — the additive UTXO path + /// (`record_utxos_ffi`) is exactly that shape. + /// + /// This value is the join key a sweep's `released_outpoints` uses to + /// find additive-path rows on the host side, so byte-order drift + /// between hand-rolled copies would silently unlink them: the release + /// would match nothing and the coin would stay spent. Both this and the + /// `From<&OutPoint>` impl below exist so no site has to spell the copy + /// out again. + pub fn new(txid: &dashcore::Txid, vout: u32) -> Self { + let mut bytes = [0u8; 32]; + bytes.copy_from_slice(txid.as_ref()); + Self { txid: bytes, vout } + } +} + impl From<&dashcore::OutPoint> for OutPointFFI { - /// The one authority for `OutPoint` → FFI conversion. This value is - /// the join key sweep releases use to find additive-path rows on the - /// host side, so a byte-order drift between hand-rolled copies would - /// silently unlink them — every conversion site routes through here. + /// Conversion for callers holding a whole `OutPoint`; delegates to + /// [`OutPointFFI::new`], which is where the byte copy lives. fn from(outpoint: &dashcore::OutPoint) -> Self { - let mut txid = [0u8; 32]; - txid.copy_from_slice(outpoint.txid.as_ref()); - Self { - txid, - vout: outpoint.vout, - } + Self::new(&outpoint.txid, outpoint.vout) } } @@ -283,6 +294,13 @@ pub struct WalletChangeSetFFI { /// handed back at the next load and re-create a balance the wallet has /// already corrected. #[repr(C)] +/// # Null at count 0 +/// +/// `txids` and `released_outpoints` are BOTH null when their count is zero — +/// a batch can carry an empty release set, and (defensively) an empty txid +/// list. A consumer must check each pointer before forming a slice from it: +/// `slice::from_raw_parts(null, 0)` is undefined behaviour in Rust, not a +/// harmless empty slice, and a naive host binding would dereference null. pub struct SweepBatchFFI { /// Removed transactions, raw 32-byte txids. Delete these rows and every /// UTXO they created. @@ -1051,13 +1069,10 @@ fn record_new_utxos_ffi( let script_bytes = txout.script_pubkey.as_bytes().to_vec(); let script_len = script_bytes.len(); let script_ptr = vec_to_ptr_u8(script_bytes, script_len); - let mut txid = [0u8; 32]; - txid.copy_from_slice(rec.txid.as_ref()); Some(UtxoEntryFFI { - outpoint: OutPointFFI { - txid, - vout: d.index, - }, + // Through the shared authority: this is the row a sweep's + // release later joins against by outpoint. + outpoint: OutPointFFI::new(&rec.txid, d.index), amount: txout.value, address: address.into_raw(), script_pubkey: script_ptr, diff --git a/packages/rs-platform-wallet-ffi/src/manager.rs b/packages/rs-platform-wallet-ffi/src/manager.rs index 3b0cd0b9013..2080204602d 100644 --- a/packages/rs-platform-wallet-ffi/src/manager.rs +++ b/packages/rs-platform-wallet-ffi/src/manager.rs @@ -8,9 +8,7 @@ use crate::event_handler::{ }; use crate::handle::*; use crate::persistence::{ - FFIPersister, FreeTrackedMasternodesFn, LoadTrackedMasternodesFn, PersistDpnsNameStatesFn, - PersistTrackedMasternodesFn, PersistWalletChangesetChainLockHeightFn, - PersistWalletChangesetSweepsFn, PersistenceCallbacks, PersistenceCallbacksExtension, + FFIPersister, PersistDpnsNameStatesFn, PersistenceCallbacks, PersistenceCallbacksExtension, PersistenceCapabilitiesFFI, PersistenceExtensionCallbacks, PLATFORM_WALLET_PERSISTENCE_CALLBACKS_EXTENSION_VERSION, }; @@ -187,16 +185,18 @@ pub unsafe extern "C" fn platform_wallet_manager_create_with_extensions( /// `$extension` must point to a live extension struct of type `$ext_ty` /// whose `struct_size` honestly describes its allocation. macro_rules! negotiated_extension_slot { - ($extension:expr, $ext_ty:ty, $version_const:expr, $field:ident, $fn_ty:ty) => {{ + ($extension:expr, $ext_ty:ty, $supplied_size:expr, $version_ok:expr, $field:ident) => {{ let extension: *const $ext_ty = $extension; - let supplied_size = std::ptr::addr_of!((*extension).struct_size).read(); - let version_end = std::mem::offset_of!($ext_ty, version) + std::mem::size_of::(); + // The gate is computed from the FIELD's own width, not from a type + // named at the call site: a slot's size and the slot being read are + // then the same fact, so a mismatched second argument cannot compute + // `callback_end` against the wrong width — refusing a slot the host + // allocated, or (if the real field is wider) accepting a read past + // the host's allocation. It also lets a future slot hold something + // that is not an `Option` without the macro needing to know. let callback_end = - std::mem::offset_of!($ext_ty, $field) + std::mem::size_of::>(); - if supplied_size < version_end - || std::ptr::addr_of!((*extension).version).read() != $version_const - || supplied_size < callback_end - { + std::mem::offset_of!($ext_ty, $field) + std::mem::size_of_val(&(*extension).$field); + if !$version_ok || $supplied_size < callback_end { None } else { std::ptr::addr_of!((*extension).$field).read() @@ -211,49 +211,56 @@ macro_rules! negotiated_extension_slot { unsafe fn persistence_extension_callbacks( extension: *const PersistenceCallbacksExtension, ) -> PersistenceExtensionCallbacks { + // ONE snapshot of the header for the whole negotiation. Reading + // `struct_size` and `version` per slot would let a host that mutates its + // extension while `create` runs — or one whose struct lives in shared or + // mapped memory — have slot 1 negotiated against one declared layout and + // slot 6 against another, producing a callback set that never + // corresponded to any single declaration the host made. + let supplied_size = std::ptr::addr_of!((*extension).struct_size).read(); + let version_end = + std::mem::offset_of!(PersistenceCallbacksExtension, version) + std::mem::size_of::(); + let version_ok = supplied_size >= version_end + && std::ptr::addr_of!((*extension).version).read() + == PLATFORM_WALLET_PERSISTENCE_CALLBACKS_EXTENSION_VERSION; + macro_rules! slot { - ($field:ident, $fn_ty:ty) => { + ($field:ident) => { negotiated_extension_slot!( extension, PersistenceCallbacksExtension, - PLATFORM_WALLET_PERSISTENCE_CALLBACKS_EXTENSION_VERSION, - $field, - $fn_ty + supplied_size, + version_ok, + $field ) }; } PersistenceExtensionCallbacks { - dpns_name_states: slot!(on_persist_dpns_name_states_fn, PersistDpnsNameStatesFn), - persist_tracked_masternodes: slot!( - on_persist_tracked_masternodes_fn, - PersistTrackedMasternodesFn - ), - load_tracked_masternodes: slot!(on_load_tracked_masternodes_fn, LoadTrackedMasternodesFn), - load_tracked_masternodes_free: slot!( - on_load_tracked_masternodes_free_fn, - FreeTrackedMasternodesFn - ), - wallet_changeset_sweeps: slot!( - on_persist_wallet_changeset_sweeps_fn, - PersistWalletChangesetSweepsFn - ), - wallet_changeset_chain_lock_height: slot!( - on_persist_wallet_changeset_chain_lock_height_fn, - PersistWalletChangesetChainLockHeightFn - ), + dpns_name_states: slot!(on_persist_dpns_name_states_fn), + persist_tracked_masternodes: slot!(on_persist_tracked_masternodes_fn), + load_tracked_masternodes: slot!(on_load_tracked_masternodes_fn), + load_tracked_masternodes_free: slot!(on_load_tracked_masternodes_free_fn), + wallet_changeset_sweeps: slot!(on_persist_wallet_changeset_sweeps_fn), + wallet_changeset_chain_lock_height: slot!(on_persist_wallet_changeset_chain_lock_height_fn), } } unsafe fn event_extension_dpns_callback( extension: *const EventHandlerCallbacksExtension, ) -> Option { + let supplied_size = std::ptr::addr_of!((*extension).struct_size).read(); + let version_end = + std::mem::offset_of!(EventHandlerCallbacksExtension, version) + std::mem::size_of::(); + let version_ok = supplied_size >= version_end + && std::ptr::addr_of!((*extension).version).read() + == PLATFORM_WALLET_EVENT_CALLBACKS_EXTENSION_VERSION; negotiated_extension_slot!( extension, EventHandlerCallbacksExtension, - PLATFORM_WALLET_EVENT_CALLBACKS_EXTENSION_VERSION, - on_dpns_marketplace_sync_completed_fn, - DpnsMarketplaceSyncCompletedFn + supplied_size, + version_ok, + on_dpns_marketplace_sync_completed_fn ) } diff --git a/packages/rs-platform-wallet-ffi/src/persistence.rs b/packages/rs-platform-wallet-ffi/src/persistence.rs index 1cd0c4b3447..3d398a5b45d 100644 --- a/packages/rs-platform-wallet-ffi/src/persistence.rs +++ b/packages/rs-platform-wallet-ffi/src/persistence.rs @@ -19,7 +19,7 @@ use key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoIn use key_wallet::wallet::managed_wallet_info::ManagedWalletInfo; use key_wallet::wallet::Wallet; use key_wallet::AddressInfo; -use parking_lot::{Mutex, RwLock}; +use parking_lot::Mutex; use std::str::FromStr; use crate::types::{FFINetwork, Network}; @@ -210,9 +210,11 @@ pub type PersistWalletChangesetSweepsFn = unsafe extern "C" fn( /// [`SweepBatchFFI::winner_mined_height`]); without it a host either /// cannot collect at all or has to guess from the synced height alone, /// which is not finality. Fired inside the round's begin/end bracket, -/// after `on_persist_wallet_changeset_fn`, only when the round advanced -/// the chainlock watermark. Monotonic-max semantics at the host: chain -/// locks only move forward, so store `max(stored, incoming)`. A non-zero +/// after `on_persist_wallet_changeset_fn`, on every round whose changeset +/// carries a chainlock — including a re-application at a height already +/// stored, since Rust does not track what the host has. Monotonic-max +/// semantics at the host are what make that harmless: chain locks only +/// move forward, so store `max(stored, incoming)`. A non-zero /// return fails the round like any other per-kind callback. pub type PersistWalletChangesetChainLockHeightFn = unsafe extern "C" fn(context: *mut c_void, wallet_id: *const u8, chain_lock_height: u32) -> i32; @@ -1140,7 +1142,6 @@ pub struct FFIPersister { /// vtable by the additive manager-create API. Keeping this out of /// `PersistenceCallbacks` preserves that established C struct's size. declared_capabilities: PersistenceCapabilities, - pending: RwLock>, /// Serializes the ENTIRE begin→per-kind→end callback round of /// [`Self::store`]. Every round producer (the core-changeset bridge, /// platform-address sync, shielded sync, spawned DashPay tasks) shares @@ -1257,7 +1258,6 @@ impl FFIPersister { .wallet_changeset_chain_lock_height, tracked_masternodes_callbacks: extensions, declared_capabilities, - pending: RwLock::new(BTreeMap::new()), round_lock: Mutex::new(RoundGuardState::default()), } } @@ -1838,15 +1838,21 @@ impl PlatformWalletPersistence for FFIPersister { // The round's sweeps ride their own size-negotiated extension // callback rather than the changeset struct (see the layout note - // on `WalletChangeSetFFI`), fired immediately after it — still - // inside the same begin/end bracket — so the additive half of - // the round, a wallet-relevant winner's own record included, is - // already staged when the removal decides which links point at a - // dead transaction. A host without the slot simply never sees - // them; that is safe to leave silent here because such a host - // can never attest `CORE_SWEEP_REMOVAL`, and the core bridge - // already freezes the sync watermark for a sweep-carrying round - // against a persister without that capability. + // on `WalletChangeSetFFI`), fired after the changeset callback + // and after the chainlock-height slot, still inside the same + // begin/end bracket — so the additive half of the round, a + // wallet-relevant winner's own record included, is already + // staged when the removal decides which links point at a dead + // transaction. + // + // A host without the slot simply never sees them, and this block + // stays silent about that on purpose: such a host can never + // attest `CORE_SWEEP_REMOVAL` (the derivation below requires the + // slot structurally), so the round is refused one layer up + // instead. NOTE: that refusal is the watermark-strip gate in the + // core bridge, which lands with the producer — nothing in THIS + // crate consults `CORE_SWEEP_REMOVAL` yet, and until the + // producer exists no round can carry sweeps at all. if !core_cs.sweeps.is_empty() { if let Some(cb) = self.wallet_changeset_sweeps_callback { let (batches, _batch_storage) = build_sweep_batches_for_callback(core_cs); @@ -2742,15 +2748,6 @@ impl PlatformWalletPersistence for FFIPersister { )); } - // Merge into pending changesets. No secret rides the changeset any - // more — the client derives identity keys on demand from the Keychain - // seed at the breadcrumb path, so nothing here needs scrubbing. - let mut pending = self.pending.write(); - pending - .entry(wallet_id) - .and_modify(|existing| existing.merge(changeset.clone())) - .or_insert(changeset); - // Preserve the legacy notification phase. With an end callback, the // host transaction is already committed and a notification failure is // advisory. Without that atomic boundary, preserve the established @@ -2792,10 +2789,6 @@ impl PlatformWalletPersistence for FFIPersister { } } - // Clear pending after successful flush notification. - let mut pending = self.pending.write(); - pending.remove(&wallet_id); - Ok(()) } @@ -6959,7 +6952,9 @@ mod tests { ..Default::default() } } - /// Everything the bit needs except the atomic round. + /// Everything the bit needs, atomic round included — the "without + /// the atomic round" case is the one below, which passes + /// `CORE_SWEEP_REMOVAL` on its own. fn declared() -> PersistenceCapabilities { PersistenceCapabilities::CORE_SWEEP_REMOVAL .union(PersistenceCapabilities::ATOMIC_CHANGESETS) @@ -7063,7 +7058,15 @@ mod tests { let sink = &*(ctx as *const Sink); let mut events = sink.events.lock().unwrap(); for batch in slice::from_raw_parts(sweeps, sweeps_count) { - let txids = slice::from_raw_parts(batch.txids, batch.txids_count); + // Both pointers are null at count 0 (see `SweepBatchFFI`), and + // `from_raw_parts(null, 0)` is UB — not merely a no-op — so + // the guard is symmetric with `released_outpoints` below. A + // host binding copying this consumer inherits the same shape. + let txids = if batch.txids.is_null() { + &[][..] + } else { + slice::from_raw_parts(batch.txids, batch.txids_count) + }; let released = if batch.released_outpoints.is_null() { &[][..] } else { diff --git a/packages/rs-platform-wallet/src/changeset/changeset.rs b/packages/rs-platform-wallet/src/changeset/changeset.rs index 16e5fe13570..8227dcec755 100644 --- a/packages/rs-platform-wallet/src/changeset/changeset.rs +++ b/packages/rs-platform-wallet/src/changeset/changeset.rs @@ -249,10 +249,19 @@ pub struct CoreChangeSet { /// Union the release sets and the first answer outlives the last one that /// is actually true. Applied in order, each batch corrects the one before /// it, which is what the wallet itself did. - /// `serde(default)`: this field postdates the serialized representation, - /// so a payload written before it necessarily omits it. An empty vec is - /// the exact backward-compatible reading — a changeset from then could - /// not have carried a sweep. + /// `serde(default)` so a payload written before this field existed still + /// reads, as an empty vec — the exact backward-compatible meaning, since + /// a changeset from then could not have carried a sweep. + /// + /// Scope of that claim: it holds for SELF-DESCRIBING encodings (JSON and + /// friends), where a missing field is a fact the decoder can see. It does + /// NOT hold for a non-self-describing one — bincode, which is what this + /// workspace persists every stored blob with — where appending a field is + /// a wire break `default` cannot absorb. That is not a live hazard today: + /// nothing in-tree serializes a changeset at all (the derive is behind the + /// optional `serde` feature for out-of-tree consumers), and this note + /// exists so nobody starts persisting one with bincode believing the + /// attribute makes it upgrade-safe. #[cfg_attr(feature = "serde", serde(default))] pub sweeps: Vec, } @@ -2208,6 +2217,42 @@ mod serde_compat_tests { assert_eq!(cs.last_processed_height, Some(1000)); assert_eq!(cs.synced_height, Some(900)); } + + /// The compat test above only proves a MISSING `sweeps` reads as empty. + /// This one proves a present one survives the trip at all: `SweepBatch` + /// carries `Txid` and `OutPoint` from `dashcore`, whose `Serialize` / + /// `Deserialize` arrive through that crate's own feature wiring — if + /// that wiring were wrong or absent, every sweep-carrying changeset + /// would silently fail to round-trip and nothing else here would catch + /// it. + #[test] + fn a_populated_sweep_batch_round_trips() { + use dashcore::hashes::Hash; + + let loser = Txid::from_byte_array([0x11; 32]); + let winner = Txid::from_byte_array([0x22; 32]); + let released = OutPoint::new(Txid::from_byte_array([0x33; 32]), 7); + let cs = CoreChangeSet { + sweeps: vec![SweepBatch { + txids: vec![loser], + superseded_by: winner, + winner_mined_height: Some(4_242), + released_outpoints: vec![released], + }], + ..Default::default() + }; + + let encoded = serde_json::to_string(&cs).expect("a sweep-carrying changeset serializes"); + let decoded: CoreChangeSet = + serde_json::from_str(&encoded).expect("and reads back identically"); + + assert_eq!(decoded.sweeps.len(), 1); + let batch = &decoded.sweeps[0]; + assert_eq!(batch.txids, vec![loser]); + assert_eq!(batch.superseded_by, winner); + assert_eq!(batch.winner_mined_height, Some(4_242)); + assert_eq!(batch.released_outpoints, vec![released]); + } } #[cfg(test)] diff --git a/packages/rs-platform-wallet/src/changeset/persistence_capabilities.rs b/packages/rs-platform-wallet/src/changeset/persistence_capabilities.rs index 86d059786a7..4e30e23d516 100644 --- a/packages/rs-platform-wallet/src/changeset/persistence_capabilities.rs +++ b/packages/rs-platform-wallet/src/changeset/persistence_capabilities.rs @@ -72,8 +72,22 @@ impl PersistenceCapabilities { /// never reads unless the host's declared `struct_size` proved it exists /// — so an older host processes the rest of the round, returns success, /// and never sees the sweeps at all; this bit tells the wallet that the - /// complete sweep contract was implemented rather than silently - /// truncated. + /// removal half was implemented rather than silently truncated. + /// + /// # What this bit does NOT cover: collection + /// + /// Retention is in the contract, collection is not. A tombstone stamped + /// with its winner's mined height becomes collectible at + /// `min(chainlock_height, synced_height)`, and a non-Rust host learns + /// the chainlock half only through the extension's separate + /// chain-lock-height slot. That slot is deliberately NOT required here: + /// gating removal on it would refuse the rounds of a host that + /// implements removal but not collection, freezing its watermark over + /// what is only unbounded retention. Such a host is correct but keeps + /// stamped tombstones forever — holding a coin spent is the safe + /// direction, and never leaks one back into the unspent set. A host + /// that wants the rows collected must wire the chain-lock-height slot + /// as well; both in-tree mobile persisters do. pub const CORE_SWEEP_REMOVAL: Self = Self(1 << 11); /// A stored changeset's `dashpay_payments_overlay` rows are durably /// applied. This is what lets the wallet-event adapter couple a sweep's @@ -92,6 +106,14 @@ impl PersistenceCapabilities { /// wired. pub const DASHPAY_PAYMENTS: Self = Self(1 << 12); + /// Index of the highest bit declared above. It lives here, beside the + /// constants, so adding a bit and bumping this is one edit in one place + /// — and `every_declared_bit_has_a_stable_name` walks up to it, so a new + /// bit that never reaches `KNOWN` fails a test instead of gating + /// behaviour invisibly. The same test asserts nothing above it is named, + /// which is what catches a bit added without bumping this. + const HIGHEST_DECLARED_BIT: u32 = 12; + /// Capabilities required before exporting and funding an invitation voucher. pub const INVITATION_CREATION: Self = Self( Self::ATOMIC_CHANGESETS.0 @@ -236,16 +258,34 @@ mod tests { /// still gates behaviour but vanishes from every diagnostic that /// reports capabilities by name, so a host debugging why its rows /// never landed sees nothing about the capability that withheld them - /// — which is exactly what `DASHPAY_PAYMENTS` did until this test. + /// — the shape `DASHPAY_PAYMENTS` would have had if it had shipped + /// without its `KNOWN` entry, which is what this test now prevents for + /// every bit added after it. #[test] fn every_declared_bit_has_a_stable_name() { - for shift in 0..13u32 { - let bit = PersistenceCapabilities::from_bits_retain(1 << shift); + // Derived from the declared set, not hardcoded: a hardcoded bound + // stops at today's highest bit, so the NEXT bit added without a + // `KNOWN` entry would gate behaviour while staying invisible to + // every `names()`-based diagnostic — exactly what this test exists + // to prevent. Walking the whole width cannot go stale. + for shift in 0..=PersistenceCapabilities::HIGHEST_DECLARED_BIT { + let bit = PersistenceCapabilities::from_bits_retain(1u64 << shift); assert_eq!( bit.names().len(), 1, "bit 1 << {shift} is declarable but has no name in KNOWN" ); } + // Keeps the bound honest: a bit added above without bumping + // `HIGHEST_DECLARED_BIT` would otherwise sit outside the loop and + // stay invisible, which is the very failure the loop guards. + let above = PersistenceCapabilities::from_bits_retain( + 1u64 << (PersistenceCapabilities::HIGHEST_DECLARED_BIT + 1), + ); + assert!( + above.names().is_empty(), + "bit 1 << {} is named, so HIGHEST_DECLARED_BIT is stale", + PersistenceCapabilities::HIGHEST_DECLARED_BIT + 1 + ); } } From 002ad4d83dc9ed6947c750cee00b7deb6e4057e9 Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Thu, 3 Sep 2026 19:21:41 +0300 Subject: [PATCH 4/5] fix(platform-wallet-ffi): size the slot gate from the type, not from a reference into the host MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up: the previous commit sized the gate with `size_of_val(&(*extension).$field)`, which forms a reference to the field BEFORE the size check — and the field lying past the host's declared `struct_size` is precisely the case the gate exists for. A reference to memory outside the allocation is undefined behaviour in Rust even when it is never read, so a legacy-sized host could hit UB on the very path built to protect it. The width comes from the type again, and the property that motivated the change is kept by other means: the read binds to `Option<$fn_ty>`, so a `$fn_ty` that disagrees with the field fails to COMPILE rather than sizing the gate against the wrong width. Before the gate passes, only `offset_of!` and `size_of` arithmetic runs; nothing touches the host allocation. Also drops the `Merge` import left unused when `FFIPersister::pending` was deleted. --- .../rs-platform-wallet-ffi/src/manager.rs | 62 +++++++++++++------ .../rs-platform-wallet-ffi/src/persistence.rs | 2 +- 2 files changed, 43 insertions(+), 21 deletions(-) diff --git a/packages/rs-platform-wallet-ffi/src/manager.rs b/packages/rs-platform-wallet-ffi/src/manager.rs index 2080204602d..84ef9bddac9 100644 --- a/packages/rs-platform-wallet-ffi/src/manager.rs +++ b/packages/rs-platform-wallet-ffi/src/manager.rs @@ -8,7 +8,9 @@ use crate::event_handler::{ }; use crate::handle::*; use crate::persistence::{ - FFIPersister, PersistDpnsNameStatesFn, PersistenceCallbacks, PersistenceCallbacksExtension, + FFIPersister, FreeTrackedMasternodesFn, LoadTrackedMasternodesFn, PersistDpnsNameStatesFn, + PersistTrackedMasternodesFn, PersistWalletChangesetChainLockHeightFn, + PersistWalletChangesetSweepsFn, PersistenceCallbacks, PersistenceCallbacksExtension, PersistenceCapabilitiesFFI, PersistenceExtensionCallbacks, PLATFORM_WALLET_PERSISTENCE_CALLBACKS_EXTENSION_VERSION, }; @@ -185,21 +187,27 @@ pub unsafe extern "C" fn platform_wallet_manager_create_with_extensions( /// `$extension` must point to a live extension struct of type `$ext_ty` /// whose `struct_size` honestly describes its allocation. macro_rules! negotiated_extension_slot { - ($extension:expr, $ext_ty:ty, $supplied_size:expr, $version_ok:expr, $field:ident) => {{ + ($extension:expr, $ext_ty:ty, $supplied_size:expr, $version_ok:expr, $field:ident, $fn_ty:ty) => {{ let extension: *const $ext_ty = $extension; - // The gate is computed from the FIELD's own width, not from a type - // named at the call site: a slot's size and the slot being read are - // then the same fact, so a mismatched second argument cannot compute - // `callback_end` against the wrong width — refusing a slot the host - // allocated, or (if the real field is wider) accepting a read past - // the host's allocation. It also lets a future slot hold something - // that is not an `Option` without the macro needing to know. + // Width from the TYPE, deliberately — not `size_of_val` of the + // field. Taking `&(*extension).$field` would form a reference into + // memory the host may not have allocated (a field past `struct_size` + // is the very case this gate exists for), and a reference to invalid + // memory is undefined behaviour even when it is never read. + // + // The type still cannot silently disagree with the field: the read + // below binds to `Option<$fn_ty>`, so a mismatched `$fn_ty` fails to + // COMPILE rather than sizing the gate against the wrong width — + // refusing a slot the host allocated, or accepting a read past its + // allocation. Before the gate passes nothing but `offset_of!` and + // `size_of` arithmetic happens; the host allocation is not touched. let callback_end = - std::mem::offset_of!($ext_ty, $field) + std::mem::size_of_val(&(*extension).$field); + std::mem::offset_of!($ext_ty, $field) + std::mem::size_of::>(); if !$version_ok || $supplied_size < callback_end { None } else { - std::ptr::addr_of!((*extension).$field).read() + let slot: Option<$fn_ty> = std::ptr::addr_of!((*extension).$field).read(); + slot } }}; } @@ -225,24 +233,37 @@ unsafe fn persistence_extension_callbacks( == PLATFORM_WALLET_PERSISTENCE_CALLBACKS_EXTENSION_VERSION; macro_rules! slot { - ($field:ident) => { + ($field:ident, $fn_ty:ty) => { negotiated_extension_slot!( extension, PersistenceCallbacksExtension, supplied_size, version_ok, - $field + $field, + $fn_ty ) }; } PersistenceExtensionCallbacks { - dpns_name_states: slot!(on_persist_dpns_name_states_fn), - persist_tracked_masternodes: slot!(on_persist_tracked_masternodes_fn), - load_tracked_masternodes: slot!(on_load_tracked_masternodes_fn), - load_tracked_masternodes_free: slot!(on_load_tracked_masternodes_free_fn), - wallet_changeset_sweeps: slot!(on_persist_wallet_changeset_sweeps_fn), - wallet_changeset_chain_lock_height: slot!(on_persist_wallet_changeset_chain_lock_height_fn), + dpns_name_states: slot!(on_persist_dpns_name_states_fn, PersistDpnsNameStatesFn), + persist_tracked_masternodes: slot!( + on_persist_tracked_masternodes_fn, + PersistTrackedMasternodesFn + ), + load_tracked_masternodes: slot!(on_load_tracked_masternodes_fn, LoadTrackedMasternodesFn), + load_tracked_masternodes_free: slot!( + on_load_tracked_masternodes_free_fn, + FreeTrackedMasternodesFn + ), + wallet_changeset_sweeps: slot!( + on_persist_wallet_changeset_sweeps_fn, + PersistWalletChangesetSweepsFn + ), + wallet_changeset_chain_lock_height: slot!( + on_persist_wallet_changeset_chain_lock_height_fn, + PersistWalletChangesetChainLockHeightFn + ), } } @@ -260,7 +281,8 @@ unsafe fn event_extension_dpns_callback( EventHandlerCallbacksExtension, supplied_size, version_ok, - on_dpns_marketplace_sync_completed_fn + on_dpns_marketplace_sync_completed_fn, + DpnsMarketplaceSyncCompletedFn ) } diff --git a/packages/rs-platform-wallet-ffi/src/persistence.rs b/packages/rs-platform-wallet-ffi/src/persistence.rs index 3d398a5b45d..2ba409b0eb8 100644 --- a/packages/rs-platform-wallet-ffi/src/persistence.rs +++ b/packages/rs-platform-wallet-ffi/src/persistence.rs @@ -25,7 +25,7 @@ use std::str::FromStr; use crate::types::{FFINetwork, Network}; use platform_wallet::changeset::{ AccountAddressPoolEntry, AccountRegistrationEntry, ClientStartState, ClientWalletStartState, - ListedCoreTxid, Merge, PersistenceCapabilities, PersistenceError, PlatformWalletChangeSet, + ListedCoreTxid, PersistenceCapabilities, PersistenceError, PlatformWalletChangeSet, PlatformWalletPersistence, ProviderKeyAccountEntry, ProviderKeyExtendedPubKey, PERSISTENCE_CAPABILITIES_VERSION, }; From 3003ceae96e4566a59482714a3c848fda33e03c3 Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Thu, 3 Sep 2026 19:41:33 +0300 Subject: [PATCH 5/5] fix(platform-wallet): keep HIGHEST_DECLARED_BIT alive outside cfg(test) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI builds these crates with `-D warnings`, and the constant was read only by `every_declared_bit_has_a_stable_name`, so a non-test build saw dead code and the wallet job failed to compile. Local `cargo check` does not carry that flag, which is why it passed here first. Rather than silence it, a module-level `const _` now asserts the constant names the bit that actually is the highest. That makes it a real use in every build AND closes the hole the constant left open: bumping the bit without the constant (or the reverse) failed nothing before — the test would still pass while a live bit sat outside the range it walks, which is precisely what the test exists to catch. --- .../src/changeset/persistence_capabilities.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/packages/rs-platform-wallet/src/changeset/persistence_capabilities.rs b/packages/rs-platform-wallet/src/changeset/persistence_capabilities.rs index 4e30e23d516..d930daf1ec9 100644 --- a/packages/rs-platform-wallet/src/changeset/persistence_capabilities.rs +++ b/packages/rs-platform-wallet/src/changeset/persistence_capabilities.rs @@ -214,6 +214,19 @@ impl PersistenceCapabilities { } } +/// Ties `HIGHEST_DECLARED_BIT` to the bit that actually is the highest. +/// +/// Bumping one without the other stops the build here instead of silently +/// shrinking what `every_declared_bit_has_a_stable_name` walks — the test +/// would then pass while a real bit sat outside its range, which is the +/// failure the test exists to catch. Written as a module-level `const _` so +/// it is evaluated in every build, test or not. +const _: () = assert!( + PersistenceCapabilities::DASHPAY_PAYMENTS.bits() + == 1u64 << PersistenceCapabilities::HIGHEST_DECLARED_BIT, + "HIGHEST_DECLARED_BIT must name the highest declared capability bit" +); + #[cfg(test)] mod tests { use super::*;