From e85de65678e75ff0702e21dc42195f5ed0cf875c Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 3 Jul 2026 02:24:23 +0700 Subject: [PATCH 1/2] refactor(platform-wallet): consolidate address-balance reconciliation into one guarded seam The four hand-rolled copies of the platform-address balance reconciliation (transfer, withdrawal, fund-from-asset-lock, and the top-up path added in #3969) are collapsed into a single apply-and-persist seam, PlatformAddressWallet::reconcile_address_infos, and the flows that still discarded proof-attested AddressInfos are wired into it. - build_top_up_balance_entries / build_transfer_persistence_entries are generalized into build_address_balance_entries over an index resolver; every path now resolves through the provider's persisted index<->address bijection (restored-wallet coverage) with the live pools as fallback for addresses derived since the last sync. - withdrawal's `address_index ... unwrap_or(0)` fallback is gone: an input that is no longer in the live derived pool resolves through the persisted bijection instead of corrupting derivation index 0. - register_from_addresses and transfer_to_addresses now return their proof-attested AddressInfos and are reconciled (the TODO asking for exactly this is resolved); external recipients are skipped by the seam. - The top-up orchestration moves into composite PlatformWallet methods (top_up_from_addresses, register_from_addresses, transfer_credits_to_addresses_with_external_signer); the FFI makes one call and the "caller MUST reconcile" doc-contract disappears. - Freshness guard against the 15s background sync: the seam commits through PlatformPaymentAddressProvider::commit_reconciliation under the provider write lock, dropping entries whose nonce is below the committed `found` seed's and updating that seed (the sync-diff baseline and SDK sync seed) so reconciliation and sync stop diverging; proof-attested removals (zero funds) drop the address from the seed, mirroring absent handling. sync_balances now persists its diff before releasing the provider lock so a fresher reconciliation row can never be overwritten by an older sync diff on disk. Co-Authored-By: Claude Fable 5 --- .../src/identity_registration_with_signer.rs | 7 +- .../src/identity_top_up.rs | 34 +- .../src/identity_transfer.rs | 13 +- .../network/register_from_addresses.rs | 29 +- .../identity/network/top_up_from_addresses.rs | 30 +- .../identity/network/transfer_to_addresses.rs | 23 +- .../fund_from_asset_lock.rs | 137 +----- .../src/wallet/platform_addresses/provider.rs | 455 ++++++++++++++++-- .../src/wallet/platform_addresses/sync.rs | 9 +- .../src/wallet/platform_addresses/transfer.rs | 224 +-------- .../src/wallet/platform_addresses/wallet.rs | 217 ++++++--- .../wallet/platform_addresses/withdrawal.rs | 64 +-- .../src/wallet/platform_wallet.rs | 129 ++++- 13 files changed, 821 insertions(+), 550 deletions(-) diff --git a/packages/rs-platform-wallet-ffi/src/identity_registration_with_signer.rs b/packages/rs-platform-wallet-ffi/src/identity_registration_with_signer.rs index 55a04023129..07374002323 100644 --- a/packages/rs-platform-wallet-ffi/src/identity_registration_with_signer.rs +++ b/packages/rs-platform-wallet-ffi/src/identity_registration_with_signer.rs @@ -440,7 +440,7 @@ pub unsafe extern "C" fn platform_wallet_register_identity_with_signer( )); let option = PLATFORM_WALLET_STORAGE.with_item(wallet_handle, |wallet| { - let identity_wallet = wallet.identity().clone(); + let wallet = wallet.clone(); let placeholder = Identity::V0(IdentityV0 { id: Identifier::default(), @@ -455,7 +455,10 @@ pub unsafe extern "C" fn platform_wallet_register_identity_with_signer( let address_signer: &VTableSigner = unsafe { &*(signer_address_addr as *const VTableSigner) }; - identity_wallet + // The composite registers the identity AND reconciles the + // spent funding addresses' platform-address balances from + // the proof. + wallet .register_from_addresses( &placeholder, input_map, diff --git a/packages/rs-platform-wallet-ffi/src/identity_top_up.rs b/packages/rs-platform-wallet-ffi/src/identity_top_up.rs index 177da07164d..70d90bf488b 100644 --- a/packages/rs-platform-wallet-ffi/src/identity_top_up.rs +++ b/packages/rs-platform-wallet-ffi/src/identity_top_up.rs @@ -3,8 +3,9 @@ //! //! Mirrors [`crate::identity_registration_with_signer`] (registration) //! but for an *existing* identity. The single entry point — -//! [`platform_wallet_top_up_from_addresses_with_signer`] — wraps -//! [`IdentityWallet::top_up_from_addresses`](platform_wallet::IdentityWallet::top_up_from_addresses) +//! [`platform_wallet_top_up_from_addresses_with_signer`] — wraps the +//! composite +//! [`PlatformWallet::top_up_from_addresses`](platform_wallet::PlatformWallet::top_up_from_addresses) //! and reuses the same address-input shape (`IdentityFundingInputFFI`) //! the registration FFI exposes. //! @@ -17,10 +18,10 @@ //! //! On success the function writes the post-transition credit balance //! back through `out_new_balance`. The local `ManagedIdentity` -//! manager is updated synchronously inside the library call (see -//! [`top_up_from_addresses`](platform_wallet::IdentityWallet::top_up_from_addresses) -//! for the bookkeeping); callers can re-read the balance via -//! `ManagedIdentity` once this returns. +//! manager is updated and the spent platform-address balances are +//! reconciled synchronously inside the composite library call; +//! callers can re-read the balance via `ManagedIdentity` once this +//! returns. use std::collections::BTreeMap; use std::slice; @@ -117,22 +118,17 @@ pub unsafe extern "C" fn platform_wallet_top_up_from_addresses_with_signer( let signer_addr = signer_address_handle as usize; let option = PLATFORM_WALLET_STORAGE.with_item(wallet_handle, |wallet| { - let identity_wallet = wallet.identity().clone(); - let platform_wallet = wallet.platform().clone(); + let wallet = wallet.clone(); block_on_worker(async move { let address_signer: &VTableSigner = unsafe { &*(signer_addr as *const VTableSigner) }; - let (address_infos, new_balance) = identity_wallet + // The composite tops up the identity AND reconciles the spent + // platform-address balances from the proof, so the wallet's + // displayed balance and next input selection reflect the spend + // (covering addresses restored from disk that are no longer in + // a live derived pool). + wallet .top_up_from_addresses(&identity_id, input_map, address_signer, None) - .await?; - // Reconcile the spent platform-address balances from the proof so - // the wallet's displayed balance and next input selection reflect - // the spend. Resolves spent addresses via the address provider, so - // it covers addresses restored from disk that are no longer in a - // live derived pool. - platform_wallet - .apply_top_up_reconciliation(&address_infos) - .await; - Ok::<_, platform_wallet::PlatformWalletError>(new_balance) + .await }) }); let result = unwrap_option_or_return!(option); diff --git a/packages/rs-platform-wallet-ffi/src/identity_transfer.rs b/packages/rs-platform-wallet-ffi/src/identity_transfer.rs index 654a7420ee4..4bee11835fd 100644 --- a/packages/rs-platform-wallet-ffi/src/identity_transfer.rs +++ b/packages/rs-platform-wallet-ffi/src/identity_transfer.rs @@ -97,8 +97,10 @@ pub unsafe extern "C" fn platform_wallet_transfer_credits_with_signer( /// [`PlatformAddressCreditOutputFFI`] recipients using the supplied /// `signer_handle`. /// -/// Wraps -/// [`IdentityWallet::transfer_credits_to_addresses_with_external_signer`](platform_wallet::IdentityWallet::transfer_credits_to_addresses_with_external_signer). +/// Wraps the composite +/// [`PlatformWallet::transfer_credits_to_addresses_with_external_signer`](platform_wallet::PlatformWallet::transfer_credits_to_addresses_with_external_signer), +/// which also reconciles wallet-owned recipient addresses' platform +/// balances from the transfer proof. /// /// `out_new_balance` (when non-null) receives the sender's remaining /// balance after the transfer. @@ -149,10 +151,13 @@ pub unsafe extern "C" fn platform_wallet_transfer_credits_to_addresses_with_sign let signer_addr = signer_handle as usize; let option = PLATFORM_WALLET_STORAGE.with_item(wallet_handle, |wallet| { - let identity_wallet = wallet.identity().clone(); + let wallet = wallet.clone(); block_on_worker(async move { let signer: &VTableSigner = &*(signer_addr as *const VTableSigner); - identity_wallet + // The composite transfers the credits AND reconciles any + // wallet-owned recipient addresses' platform balances from + // the proof (third-party recipients are skipped). + wallet .transfer_credits_to_addresses_with_external_signer( &from_id, output_map, signer, None, ) diff --git a/packages/rs-platform-wallet/src/wallet/identity/network/register_from_addresses.rs b/packages/rs-platform-wallet/src/wallet/identity/network/register_from_addresses.rs index 521a9134035..74867714726 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/network/register_from_addresses.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/network/register_from_addresses.rs @@ -37,6 +37,19 @@ impl IdentityWallet { /// DIP-9 auth pubkeys) and both signers; the wallet struct /// carries no key material. /// + /// Returns the registered identity alongside the proof-attested + /// post-spend `AddressInfos` for the funding addresses. Prefer the + /// composite [`PlatformWallet::register_from_addresses`], which feeds + /// the returned `AddressInfos` through + /// [`PlatformAddressWallet::reconcile_address_infos`] so the spent + /// balances and advanced nonces are reflected locally without waiting + /// for the next BLAST sync round. + /// + /// [`PlatformWallet::register_from_addresses`]: + /// crate::wallet::PlatformWallet::register_from_addresses + /// [`PlatformAddressWallet::reconcile_address_infos`]: + /// crate::wallet::PlatformAddressWallet::reconcile_address_infos + /// /// # Arguments /// /// * `identity` — fully-constructed placeholder identity (with @@ -74,7 +87,7 @@ impl IdentityWallet { identity_signer: &IS, input_address_signer: &AS, settings: Option, - ) -> Result { + ) -> Result<(Identity, dash_sdk::query_types::AddressInfos), PlatformWalletError> { if inputs.is_empty() { return Err(PlatformWalletError::InvalidIdentityData( "At least one input address is required".to_string(), @@ -84,7 +97,7 @@ impl IdentityWallet { // Route through the auto-fetching SDK variant so the caller // doesn't need to maintain its own nonce cache — Platform is // always the source of truth at submit time. - let (mut registered_identity, _address_infos) = identity + let (mut registered_identity, address_infos) = identity .put_with_address_funding_fetching_nonces( &self.sdk, inputs, @@ -136,12 +149,10 @@ impl IdentityWallet { )?; } - // TODO(platform-wallet): mirror `transfer()` and push the - // returned `address_infos` through the platform-address - // balance cache so SwiftData reflects the spent balances + - // advanced nonces immediately. For now the next BLAST sync - // round refreshes them. - - Ok(identity) + // The spent platform-address balances are reconciled by the + // composite `PlatformWallet::register_from_addresses`, which routes + // the returned `AddressInfos` through the platform-address wallet's + // shared reconciliation seam. + Ok((identity, address_infos)) } } diff --git a/packages/rs-platform-wallet/src/wallet/identity/network/top_up_from_addresses.rs b/packages/rs-platform-wallet/src/wallet/identity/network/top_up_from_addresses.rs index a689acbb493..e9b4a124eb8 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/network/top_up_from_addresses.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/network/top_up_from_addresses.rs @@ -28,14 +28,20 @@ impl IdentityWallet { /// Uses the `TopUpIdentityFromAddresses` SDK trait. Address nonces are /// looked up automatically. /// - /// Returns the proof-attested post-spend `AddressInfos` alongside the new - /// identity balance. The caller MUST reconcile the spent platform-address - /// balances from the `AddressInfos` via - /// [`PlatformAddressWallet::apply_top_up_reconciliation`] — this method - /// only owns the identity-side balance update, because resolving a spent - /// address back to its derivation index needs the address provider, which - /// lives on the platform-address wallet (and covers addresses restored - /// from disk that are no longer in a live derived pool). + /// This method owns only the identity-side balance update and returns + /// the proof-attested post-spend `AddressInfos` alongside the new + /// identity balance. Prefer the composite + /// [`PlatformWallet::top_up_from_addresses`], which feeds the returned + /// `AddressInfos` through + /// [`PlatformAddressWallet::reconcile_address_infos`] — the + /// platform-address wallet holds the address provider needed to map a + /// spent address back to its derivation index (including addresses + /// restored from disk that are no longer in a live derived pool). + /// + /// [`PlatformWallet::top_up_from_addresses`]: + /// crate::wallet::PlatformWallet::top_up_from_addresses + /// [`PlatformAddressWallet::reconcile_address_infos`]: + /// crate::wallet::PlatformAddressWallet::reconcile_address_infos /// /// # Arguments /// @@ -100,10 +106,10 @@ impl IdentityWallet { } } - // The spent platform-address balances are reconciled by the caller via - // `PlatformAddressWallet::apply_top_up_reconciliation`, which has the - // address provider needed to map restored addresses back to their - // derivation index. + // The spent platform-address balances are reconciled by the + // composite `PlatformWallet::top_up_from_addresses`, which routes + // the returned `AddressInfos` through the platform-address wallet's + // shared reconciliation seam. Ok((address_infos, new_balance)) } } diff --git a/packages/rs-platform-wallet/src/wallet/identity/network/transfer_to_addresses.rs b/packages/rs-platform-wallet/src/wallet/identity/network/transfer_to_addresses.rs index 33c46a47e5b..5d1ada2dbd4 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/network/transfer_to_addresses.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/network/transfer_to_addresses.rs @@ -63,13 +63,26 @@ impl IdentityWallet { /// /// Signing is routed through the supplied `&S: Signer`. /// Required for external-signable wallets. + /// + /// Returns the proof-attested post-transfer `AddressInfos` for the + /// recipient addresses alongside the sender's new balance. Prefer the + /// composite [`PlatformWallet::transfer_credits_to_addresses_with_external_signer`], + /// which feeds the returned `AddressInfos` through + /// [`PlatformAddressWallet::reconcile_address_infos`] so recipients + /// owned by this wallet see their new balance immediately instead of + /// waiting for the next BLAST sync round. + /// + /// [`PlatformWallet::transfer_credits_to_addresses_with_external_signer`]: + /// crate::wallet::PlatformWallet::transfer_credits_to_addresses_with_external_signer + /// [`PlatformAddressWallet::reconcile_address_infos`]: + /// crate::wallet::PlatformAddressWallet::reconcile_address_infos pub async fn transfer_credits_to_addresses_with_external_signer( &self, identity_id: &Identifier, recipient_addresses: BTreeMap, signer: &S, settings: Option, - ) -> Result + ) -> Result<(dash_sdk::query_types::AddressInfos, Credits), PlatformWalletError> where S: Signer + Send + Sync, { @@ -87,7 +100,7 @@ impl IdentityWallet { .ok_or(PlatformWalletError::IdentityNotFound(*identity_id))? }; - let (_address_infos, new_balance) = identity + let (address_infos, new_balance) = identity .transfer_credits_to_addresses( &self.sdk, recipient_addresses, @@ -126,6 +139,10 @@ impl IdentityWallet { } } - Ok(new_balance) + // Recipient platform-address balances are reconciled by the + // composite `PlatformWallet::transfer_credits_to_addresses_with_external_signer`, + // which routes the returned `AddressInfos` through the + // platform-address wallet's shared reconciliation seam. + Ok((address_infos, new_balance)) } } diff --git a/packages/rs-platform-wallet/src/wallet/platform_addresses/fund_from_asset_lock.rs b/packages/rs-platform-wallet/src/wallet/platform_addresses/fund_from_asset_lock.rs index c84478589e1..255c17fd01e 100644 --- a/packages/rs-platform-wallet/src/wallet/platform_addresses/fund_from_asset_lock.rs +++ b/packages/rs-platform-wallet/src/wallet/platform_addresses/fund_from_asset_lock.rs @@ -25,7 +25,6 @@ //! tracked outpoint so the row is marked `Consumed` (terminal) //! and dropped from the in-memory tracked-lock map. -use crate::changeset::Merge; use crate::wallet::asset_lock::orchestration::{ out_point_from_proof, submit_with_cl_height_retry, AssetLockFunding, FundingResolution, ResolvedFunding, CL_FALLBACK_TIMEOUT, @@ -273,39 +272,26 @@ impl PlatformAddressWallet { // or all recipients. validate_address_infos_complete(&addresses, &address_infos)?; - let cs = self - .write_address_balances_changeset(platform_account_index, &address_infos) - .await; - - // Mirror `transfer.rs` / `sync.rs`: push the post-submit - // balances through the persister so any external store stays - // in sync with the in-memory account state we just updated. - // Without this, persisted rows for these recipients stay - // frozen at pre-top-up values until the next BLAST sync - // overwrites them. On the next process start before that - // sync, `initialize_from_persisted` would seed + // The shared seam applies the proof-attested balances to the + // managed accounts, updates the provider's sync seed, and + // persists — without the persist, rows for these recipients + // stay frozen at pre-top-up values until the next BLAST sync; + // on a process restart before that sync, + // `initialize_from_persisted` would seed // `account.address_credit_balance` from the stale rows while - // the asset-lock record is already `Consumed` — leaving + // the asset-lock record is already `Consumed`, leaving // `auto_select_inputs` to under-budget and produce // protocol-level rejections until a sync repairs them. // - // The persist MUST happen before `consume_asset_lock` so - // we never have a Consumed lock paired with a stale balance - // row on disk. - // - // Log-on-error rather than propagate: Platform already - // accepted the transition, and a persistence hiccup shouldn't - // mask that. A subsequent sync reconciles. - if !cs.is_empty() { - if let Err(e) = self.persister.store(cs.clone().into()) { - tracing::error!( - error = %e, - "Failed to persist fund-from-asset-lock changeset; \ - in-memory balances are updated but durable rows are stale \ - until the next BLAST sync" - ); - } - } + // The seam persists before returning, and the persist MUST + // happen before `consume_asset_lock` so we never have a + // Consumed lock paired with a stale balance row on disk. + // Persistence errors are logged inside the seam rather than + // propagated: Platform already accepted the transition, and a + // persistence hiccup shouldn't mask that. + let cs = self + .reconcile_address_infos(&address_infos, "fund from asset lock") + .await; if let Some(out_point) = tracked_out_point { // Platform DID accept the top-up — propagating an Err @@ -348,97 +334,6 @@ impl PlatformAddressWallet { Ok(cs) } - - /// Apply proof-attested credit balances to the - /// `ManagedPlatformAccount` for each recipient address, emitting - /// a `PlatformAddressChangeSet` describing the new balances. - async fn write_address_balances_changeset( - &self, - platform_account_index: u32, - address_infos: &AddressInfos, - ) -> PlatformAddressChangeSet { - let key_source = { - let guard = self.provider.read().await; - guard - .as_ref() - .and_then(|p| p.key_source(&self.wallet_id, platform_account_index)) - }; - - let mut wm = self.wallet_manager.write().await; - let mut cs = PlatformAddressChangeSet::default(); - if let Some(info) = wm.get_wallet_info_mut(&self.wallet_id) { - if let Some(account) = info - .core_wallet - .platform_payment_managed_account_at_index_mut(platform_account_index) - { - for (addr, maybe_info) in address_infos.iter() { - let PlatformAddress::P2pkh(hash) = *addr else { - continue; - }; - let p2pkh = PlatformP2PKHAddress::new(hash); - // Platform's proof must carry an `AddressInfo` - // for every recipient we asked to fund. A `None` - // is a protocol-contract violation, not a - // zero-credit funding — skip and log so a missed - // recipient is visible to operators instead of - // silently writing a "credited 0" row. - let Some(ai) = maybe_info else { - tracing::error!( - address = %p2pkh, - "Platform proof returned None AddressInfo for a recipient that should have been credited; skipping balance write to avoid recording 'credited 0'" - ); - continue; - }; - let funds = dash_sdk::platform::address_sync::AddressFunds { - balance: ai.balance, - nonce: ai.nonce, - }; - // The recipient must exist in the account's - // address pool — `validate_recipient_addresses` - // verified that upstream. A miss here would - // mean the pool was mutated between pre-flight - // and now; skip and log rather than mis-attribute - // credits to whichever address lives at slot 0. - // - // Look this up BEFORE mutating the in-memory - // balance: if we mutated first and then `continue`d - // on a pool miss, the changeset would be missing - // this recipient's entry while the in-memory state - // already carried the new balance — defeating the - // persist-before-consume invariant the caller - // relies on (the persisted row would stay stale - // while the asset lock is consumed regardless). - let Some(address_index) = - account - .addresses - .addresses - .iter() - .find_map(|(&idx, ainfo)| { - PlatformP2PKHAddress::from_address(&ainfo.address) - .ok() - .filter(|found| *found == p2pkh) - .map(|_| idx) - }) - else { - tracing::error!( - address = %p2pkh, - "Recipient address not found in account address pool; skipping balance write to avoid mis-attributing credits to slot 0" - ); - continue; - }; - account.set_address_credit_balance(p2pkh, funds.balance, key_source.as_ref()); - cs.addresses.push(crate::PlatformAddressBalanceEntry { - wallet_id: self.wallet_id, - account_index: platform_account_index, - address_index, - address: p2pkh, - funds, - }); - } - } - } - cs - } } /// Pre-flight check for the recipient address map: diff --git a/packages/rs-platform-wallet/src/wallet/platform_addresses/provider.rs b/packages/rs-platform-wallet/src/wallet/platform_addresses/provider.rs index 797e603b6dc..86108cfce90 100644 --- a/packages/rs-platform-wallet/src/wallet/platform_addresses/provider.rs +++ b/packages/rs-platform-wallet/src/wallet/platform_addresses/provider.rs @@ -826,21 +826,26 @@ impl AddressProvider for PlatformPaymentAddressProvider { } } -/// Resolve each spent platform address in a top-up's proof-attested -/// `address_infos` to its `(account_index, address_index)` using the -/// per-account index bijections, and pair it with the proof's post-spend -/// balance + nonce. Resolving against the bijections — rather than the live -/// derived address pool — means addresses restored from disk (present in the -/// persisted index map but not in `ManagedPlatformAccount::addresses`) are -/// still reconciled; without this, a top-up that spends a restored cached row -/// would leave its stale balance behind, preserving the phantom balance. +/// Translate a state transition's proof-attested `address_infos` into +/// persistence-changeset entries, resolving each address's +/// `(account_index, address_index)` through `resolve_index`. /// -/// Addresses the proof returns that the wallet doesn't own (no bijection -/// entry) are skipped. Pure and lock-free so the reconciliation is -/// unit-testable. -pub(crate) fn build_top_up_balance_entries( +/// Non-P2PKH addresses and addresses the resolver doesn't recognise +/// (external recipients, or addresses the wallet doesn't own) are skipped. +/// Missing per-address info (`None`) maps to zero balance / zero nonce — +/// the on-chain post-transition state for an address removed from state +/// (e.g. a fully consumed input). Pure and lock-free so every caller's +/// translation is unit-testable. +/// +/// Callers supply the resolver so every reconciliation path can resolve +/// through the provider's persisted `index <-> address` bijection — +/// covering addresses restored from disk that are no longer in a live +/// derived pool — with the live pool as fallback for addresses derived +/// since the last sync (see +/// [`PlatformPaymentAddressProvider::commit_reconciliation`]). +pub(crate) fn build_address_balance_entries( wallet_id: WalletId, - per_account_addresses: &[(u32, &BiBTreeMap)], + resolve_index: impl Fn(&PlatformP2PKHAddress) -> Option<(u32, AddressIndex)>, address_infos: &AddressInfos, ) -> Vec { let mut entries = Vec::new(); @@ -849,6 +854,9 @@ pub(crate) fn build_top_up_balance_entries( continue; }; let p2pkh = PlatformP2PKHAddress::new(*hash); + let Some((account_index, address_index)) = resolve_index(&p2pkh) else { + continue; + }; let funds = match maybe_info { Some(ai) => AddressFunds { balance: ai.balance, @@ -859,20 +867,145 @@ pub(crate) fn build_top_up_balance_entries( nonce: 0, }, }; - for &(account_index, bimap) in per_account_addresses { - if let Some(&address_index) = bimap.get_by_right(&p2pkh) { - entries.push(PlatformAddressBalanceEntry { - wallet_id, - account_index, - address_index, - address: p2pkh, - funds, - }); - break; + entries.push(PlatformAddressBalanceEntry { + wallet_id, + account_index, + address_index, + address: p2pkh, + funds, + }); + } + entries +} + +/// What [`PlatformPaymentAddressProvider::commit_reconciliation`] did with +/// a proof-attested `address_infos` map: the entries that survived the +/// freshness guard (already committed to the provider's `found` seed), +/// plus counters so the caller can log why entries were dropped. +#[derive(Default)] +pub(crate) struct ReconciliationOutcome { + /// Entries to apply to the managed accounts and persist. Already + /// committed to the provider's `found` map / bijection. + pub(crate) entries: Vec, + /// How many proof addresses resolved to a wallet-owned slot at all + /// (before the freshness guard). Zero with a non-empty proof means + /// either every address belongs to a third party or resolution failed. + pub(crate) resolved: usize, + /// Resolved entries dropped because the committed `found` seed already + /// carries a higher nonce — a background sync (or a later transition) + /// committed fresher state after this proof was produced. + pub(crate) stale_skipped: usize, + /// Resolved entries dropped as no-ops (funds identical to the + /// committed seed) to avoid persister churn. + pub(crate) unchanged_skipped: usize, +} + +impl PlatformPaymentAddressProvider { + /// Resolve a state transition's proof-attested `address_infos` for + /// `wallet_id`, apply the freshness guard, and commit the survivors to + /// the provider's `found` map (the sync-diff baseline and the seed + /// [`current_balances`](AddressProvider::current_balances) hands the + /// SDK) so reconciliation and the background sync cannot diverge. + /// + /// Resolution goes through the persisted `index <-> address` bijection + /// first — covering addresses restored from disk that are no longer in + /// a live derived pool — then falls back to `pool_indexes` (the live + /// pools, covering addresses derived since the last sync, e.g. a fresh + /// change address). Pool-resolved addresses are merged into the + /// bijection so `current_balances` can yield their committed funds. + /// + /// Freshness guard, per resolved entry: + /// * zero funds (balance 0, nonce 0 — the address was removed from + /// Platform state, e.g. a fully consumed input) is an authoritative + /// removal: always applied, and the address is dropped from `found` + /// (mirroring the sync's absent handling); + /// * otherwise, an entry whose nonce is *below* the committed seed's + /// nonce is stale — a concurrent sync pass or later transition + /// already committed fresher state — and is dropped; + /// * entries identical to the committed seed are dropped as no-ops. + /// + /// Callers must hold the provider write lock (i.e. call through + /// `&mut self`) across this commit AND the managed-account balance + /// write that follows, so a background sync — which holds the same + /// lock across its scan — can never interleave between the two. + pub(crate) fn commit_reconciliation( + &mut self, + wallet_id: &WalletId, + address_infos: &AddressInfos, + pool_indexes: &BTreeMap, + ) -> ReconciliationOutcome { + let mut outcome = ReconciliationOutcome::default(); + let Some(wallet_state) = self.per_wallet.get_mut(wallet_id) else { + return outcome; + }; + + let resolved_entries = build_address_balance_entries( + *wallet_id, + |p2pkh| { + for (&account_index, account_state) in wallet_state.iter() { + if let Some(&address_index) = account_state.addresses.get_by_right(p2pkh) { + return Some((account_index, address_index)); + } + } + pool_indexes.get(p2pkh).copied() + }, + address_infos, + ); + outcome.resolved = resolved_entries.len(); + + for entry in resolved_entries { + let account_state = wallet_state.get_mut(&entry.account_index); + let existing = account_state + .as_ref() + .and_then(|s| s.found.get(&entry.address)) + .copied(); + // Zero funds = the address no longer exists in Platform state. + // That removal is attested by the proof, so it bypasses the + // nonce guard (the pre-spend seed necessarily has a lower + // nonce than "gone"). + let is_removal = entry.funds.balance == 0 && entry.funds.nonce == 0; + if !is_removal { + if let Some(existing) = existing { + if existing.nonce > entry.funds.nonce { + outcome.stale_skipped += 1; + continue; + } + if existing == entry.funds { + outcome.unchanged_skipped += 1; + continue; + } + } + } + if let Some(state) = account_state { + if is_removal { + state.found.remove(&entry.address); + } else { + state.found.insert(entry.address, entry.funds); + } + // Merge pool-resolved addresses into the bijection so + // `current_balances` can pair the fresh funds with a + // derivation index. Never overwrite an existing pairing — + // `BiBTreeMap::insert` evicts conflicting pairs, which + // would orphan another address's `found` entry. + if state.addresses.get_by_right(&entry.address).is_none() { + if state.addresses.contains_left(&entry.address_index) { + tracing::error!( + account_index = entry.account_index, + address_index = entry.address_index, + address = %entry.address, + "commit_reconciliation: derivation index already \ + maps to a different address — state drift; \ + leaving the bijection untouched" + ); + } else { + state.addresses.insert(entry.address_index, entry.address); + } + } } + outcome.entries.push(entry); } + outcome } - entries } #[cfg(test)] @@ -908,7 +1041,7 @@ mod tests { /// The original fix scanned only the live pool, so it left restored /// rows stale, preserving the phantom Platform Balance the PR targets. #[test] - fn build_top_up_entries_resolves_restored_address_outside_live_pool() { + fn build_entries_resolves_restored_address_outside_live_pool() { use dash_sdk::query_types::AddressInfo; // Present in the persisted bijection at index 3, but NOT in a live @@ -930,9 +1063,11 @@ mod tests { }), ); - let per_account: [(u32, &BiBTreeMap); 1] = - [(ACCOUNT, &bimap)]; - let entries = build_top_up_balance_entries(WALLET, &per_account, &address_infos); + let entries = build_address_balance_entries( + WALLET, + |p2pkh| bimap.get_by_right(p2pkh).map(|&idx| (ACCOUNT, idx)), + &address_infos, + ); assert_eq!( entries.len(), @@ -953,6 +1088,50 @@ mod tests { assert_eq!(e.funds.nonce, 4, "records the bumped nonce"); } + /// `transfer_address_funds` returns address info for the full + /// `inputs ∪ outputs` set, including external recipients the wallet + /// does not own. The builder must keep entries only for addresses the + /// resolver recognises — persisting a recipient under a fabricated + /// derivation index would poison the account's address map on restore. + /// Missing per-address info maps to zero funds (the post-transition + /// state for a fully consumed input elided from the proved set). + #[test] + fn build_entries_drops_unresolved_and_zeroes_missing_info() { + use dash_sdk::query_types::AddressInfo; + + let owned = p2pkh(0x01); + let mut bimap: BiBTreeMap = BiBTreeMap::new(); + bimap.insert(7, owned); + + let owned_addr = PlatformAddress::P2pkh([0x01; 20]); + let external_addr = PlatformAddress::P2pkh([0xEE; 20]); + let mut address_infos = AddressInfos::new(); + // Fully consumed input: drive elides the info. + address_infos.insert(owned_addr, None); + // External recipient: resolver won't know it. + address_infos.insert( + external_addr, + Some(AddressInfo { + address: external_addr, + nonce: 0, + balance: 5_000_000, + }), + ); + + let entries = build_address_balance_entries( + WALLET, + |p2pkh| bimap.get_by_right(p2pkh).map(|&idx| (ACCOUNT, idx)), + &address_infos, + ); + + assert_eq!(entries.len(), 1, "external recipient must be filtered out"); + let e = &entries[0]; + assert_eq!(e.address, owned); + assert_eq!(e.address_index, 7); + assert_eq!(e.funds.balance, 0, "missing info means removed from state"); + assert_eq!(e.funds.nonce, 0); + } + /// Build a provider whose committed `per_wallet` tracks a single /// account on `wallet_id` with one funded address (index 0), backed /// by the supplied wallet manager. @@ -1281,16 +1460,16 @@ mod tests { } } - /// Integration regression for the top-up reconciliation *contract* — not - /// just the pure entry builder. `apply_top_up_reconciliation` must build + /// Integration regression for the reconciliation *contract* — not + /// just the pure entry builder. `reconcile_address_infos` must build /// AND **persist** a `PlatformAddressChangeSet` carrying the proof's /// post-spend balance for a spent address resolved via the provider's /// persisted state. The reported bug was the *missing persist* (the SDK's /// `address_infos` were discarded), so this pins that `store` actually /// fires with the decremented entry — a helper-only test would still pass - /// if `apply_top_up_reconciliation` stopped persisting. + /// if `reconcile_address_infos` stopped persisting. #[tokio::test] - async fn apply_top_up_reconciliation_persists_decremented_balance() { + async fn reconcile_address_infos_persists_decremented_balance() { use crate::broadcaster::SpvBroadcaster; use crate::events::PlatformEventManager; use crate::spv::SpvRuntime; @@ -1303,7 +1482,7 @@ mod tests { let recorder = Arc::new(CapturingPersister::default()); // Wallet wired to the capturing persister. The rest mirrors the - // short-circuit fixture — `apply_top_up_reconciliation` only touches + // short-circuit fixture — `reconcile_address_infos` only touches // provider / wallet_manager / persister. let sdk = Arc::new(dash_sdk::SdkBuilder::new_mock().build().expect("mock sdk")); let wallet_manager = Arc::new(RwLock::new(WalletManager::new(sdk.network))); @@ -1346,23 +1525,209 @@ mod tests { }), ); - wallet.apply_top_up_reconciliation(&address_infos).await; + wallet + .reconcile_address_infos(&address_infos, "test top-up") + .await; // The reconciliation must have PERSISTED the decremented entry — the // contract the original bug broke by discarding `address_infos`. - let stored = recorder.stored.lock().expect("persister mutex"); - let entry = stored - .iter() - .filter_map(|cs| cs.platform_addresses.as_ref()) - .flat_map(|pa| pa.addresses.iter()) - .find(|e| e.address == addr) - .expect("a persisted platform-address entry for the spent address"); - assert_eq!(entry.account_index, ACCOUNT); + // Scope the std mutex guard so it isn't held across the provider + // read await below. + { + let stored = recorder.stored.lock().expect("persister mutex"); + let entry = stored + .iter() + .filter_map(|cs| cs.platform_addresses.as_ref()) + .flat_map(|pa| pa.addresses.iter()) + .find(|e| e.address == addr) + .expect("a persisted platform-address entry for the spent address"); + assert_eq!(entry.account_index, ACCOUNT); + assert_eq!( + entry.funds.balance, 5, + "persists the proof's post-spend balance, not the stale pre-spend value" + ); + assert_eq!(entry.funds.nonce, 4, "persists the bumped nonce"); + } + + // ...and the provider's committed `found` seed — the sync-diff + // baseline and the seed `current_balances()` hands the SDK — must + // agree with what was just applied, so reconciliation and the + // background sync stop diverging. + let guard = wallet.provider.read().await; + let seed: Vec<_> = guard + .as_ref() + .expect("provider present") + .current_balances() + .collect(); + assert_eq!(seed.len(), 1); + assert_eq!( + seed[0].2, + funds(5, 4), + "committed found seed must carry the reconciled funds" + ); + } + + /// Freshness guard: an entry whose nonce is below the committed seed's + /// (a background sync — or a later transition — already committed + /// fresher state) must be dropped, not applied over the fresher value. + #[test] + fn commit_reconciliation_drops_stale_nonce() { + use dash_sdk::query_types::AddressInfo; + + let addr = p2pkh(0x11); + // Committed seed already at nonce 5 (e.g. the 15s background sync + // landed a fresher state while the proof was in flight). + let mut provider = provider_with_one_funded_address(addr, funds(700, 5)); + + let spent = PlatformAddress::P2pkh([0x11; 20]); + let mut address_infos = AddressInfos::new(); + address_infos.insert( + spent, + Some(AddressInfo { + address: spent, + nonce: 3, + balance: 100, + }), + ); + + let outcome = provider.commit_reconciliation(&WALLET, &address_infos, &BTreeMap::new()); + + assert_eq!(outcome.resolved, 1); + assert_eq!(outcome.stale_skipped, 1); + assert!( + outcome.entries.is_empty(), + "stale entry must not be applied" + ); + // The fresher committed funds survive. + let seed: Vec<_> = provider.current_balances().collect(); + assert_eq!(seed.len(), 1); + assert_eq!(seed[0].2, funds(700, 5)); + } + + /// An entry identical to the committed seed is a no-op and is dropped + /// to avoid persister churn. + #[test] + fn commit_reconciliation_drops_unchanged_entry() { + use dash_sdk::query_types::AddressInfo; + + let addr = p2pkh(0x11); + let mut provider = provider_with_one_funded_address(addr, funds(700, 5)); + + let same = PlatformAddress::P2pkh([0x11; 20]); + let mut address_infos = AddressInfos::new(); + address_infos.insert( + same, + Some(AddressInfo { + address: same, + nonce: 5, + balance: 700, + }), + ); + + let outcome = provider.commit_reconciliation(&WALLET, &address_infos, &BTreeMap::new()); + + assert_eq!(outcome.resolved, 1); + assert_eq!(outcome.unchanged_skipped, 1); + assert!(outcome.entries.is_empty()); + } + + /// A fresh entry (nonce at or above the seed's) is applied and + /// committed to `found`, replacing the older funds. + #[test] + fn commit_reconciliation_commits_fresh_entry_to_found() { + use dash_sdk::query_types::AddressInfo; + + let addr = p2pkh(0x11); + let mut provider = provider_with_one_funded_address(addr, funds(700, 3)); + + let spent = PlatformAddress::P2pkh([0x11; 20]); + let mut address_infos = AddressInfos::new(); + address_infos.insert( + spent, + Some(AddressInfo { + address: spent, + nonce: 4, + balance: 50, + }), + ); + + let outcome = provider.commit_reconciliation(&WALLET, &address_infos, &BTreeMap::new()); + + assert_eq!(outcome.entries.len(), 1); + assert_eq!(outcome.entries[0].funds, funds(50, 4)); + assert_eq!(outcome.entries[0].address_index, 0); + let seed: Vec<_> = provider.current_balances().collect(); + assert_eq!(seed.len(), 1); + assert_eq!(seed[0].2, funds(50, 4)); + } + + /// Zero funds (balance 0, nonce 0) means the address was removed from + /// Platform state (fully consumed input). That removal is authoritative: + /// it bypasses the nonce guard and drops the address from the committed + /// `found` seed, mirroring the sync's absent handling. + #[test] + fn commit_reconciliation_zero_funds_removes_from_found() { + let addr = p2pkh(0x11); + let mut provider = provider_with_one_funded_address(addr, funds(700, 5)); + + let consumed = PlatformAddress::P2pkh([0x11; 20]); + let mut address_infos = AddressInfos::new(); + // Drive elides the info for a fully consumed input. + address_infos.insert(consumed, None); + + let outcome = provider.commit_reconciliation(&WALLET, &address_infos, &BTreeMap::new()); + + assert_eq!(outcome.entries.len(), 1); + assert_eq!(outcome.entries[0].funds, funds(0, 0)); + assert!( + provider.current_balances().next().is_none(), + "consumed address must be dropped from the found seed" + ); + } + + /// An address missing from the provider bijection (derived since the + /// last sync, e.g. a fresh change address) resolves through the + /// live-pool fallback, and the pair is merged into the bijection so + /// `current_balances` can yield its committed funds. + #[test] + fn commit_reconciliation_pool_fallback_extends_bijection() { + use dash_sdk::query_types::AddressInfo; + + let known = p2pkh(0x11); + let mut provider = provider_with_one_funded_address(known, funds(700, 3)); + + // Fresh change address: NOT in the bijection, only in the live pool. + let fresh = p2pkh(0x22); + let mut pool_indexes = BTreeMap::new(); + pool_indexes.insert(fresh, (ACCOUNT, 9u32)); + + let fresh_addr = PlatformAddress::P2pkh([0x22; 20]); + let mut address_infos = AddressInfos::new(); + address_infos.insert( + fresh_addr, + Some(AddressInfo { + address: fresh_addr, + nonce: 0, + balance: 1_234, + }), + ); + + let outcome = provider.commit_reconciliation(&WALLET, &address_infos, &pool_indexes); + + assert_eq!(outcome.entries.len(), 1); assert_eq!( - entry.funds.balance, 5, - "persists the proof's post-spend balance, not the stale pre-spend value" + outcome.entries[0].address_index, 9, + "index resolved from the live-pool fallback" ); - assert_eq!(entry.funds.nonce, 4, "persists the bumped nonce"); + // The bijection gained the pair, so the committed funds are part + // of the next sync's seed. + let seed: Vec<_> = provider.current_balances().collect(); + let fresh_row = seed + .iter() + .find(|(_, a, _)| *a == fresh) + .expect("fresh address must appear in the found seed"); + assert_eq!(fresh_row.0, (WALLET, ACCOUNT, 9)); + assert_eq!(fresh_row.2, funds(1_234, 0)); } /// `reset_sync_state` must zero the incremental watermark AND drop diff --git a/packages/rs-platform-wallet/src/wallet/platform_addresses/sync.rs b/packages/rs-platform-wallet/src/wallet/platform_addresses/sync.rs index c68e4745383..22fe1e1aa92 100644 --- a/packages/rs-platform-wallet/src/wallet/platform_addresses/sync.rs +++ b/packages/rs-platform-wallet/src/wallet/platform_addresses/sync.rs @@ -153,13 +153,20 @@ impl PlatformAddressWallet { } provider.update_sync_state(&result); - drop(guard); + // Persist BEFORE releasing the provider lock. The reconciliation + // seam (`reconcile_address_infos`) serializes on this lock and + // persists its proof-attested entries after acquiring it, so + // keeping this store inside the critical section guarantees any + // reconciliation that observed this pass's committed state also + // lands on disk after it — releasing first would let a fresher + // reconciliation row be overwritten by this (older) diff. if !cs.is_empty() { if let Err(e) = self.persister.store(cs.into()) { tracing::error!("Failed to persist address sync changeset: {}", e); } } + drop(guard); Ok(result) } diff --git a/packages/rs-platform-wallet/src/wallet/platform_addresses/transfer.rs b/packages/rs-platform-wallet/src/wallet/platform_addresses/transfer.rs index c5eb0a51100..72fe95d6ecf 100644 --- a/packages/rs-platform-wallet/src/wallet/platform_addresses/transfer.rs +++ b/packages/rs-platform-wallet/src/wallet/platform_addresses/transfer.rs @@ -8,7 +8,6 @@ use dpp::version::PlatformVersion; use dpp::version::LATEST_PLATFORM_VERSION; use key_wallet::PlatformP2PKHAddress; -use crate::changeset::Merge; use crate::wallet::PlatformAddressWallet; use crate::{PlatformAddressChangeSet, PlatformWalletError}; use dash_sdk::platform::transition::transfer_address_funds::TransferAddressFunds; @@ -250,73 +249,13 @@ impl PlatformAddressWallet { } }; - let key_source = { - let guard = self.provider.read().await; - guard - .as_ref() - .and_then(|p| p.key_source(&self.wallet_id, account_index)) - }; - - let mut wm = self.wallet_manager.write().await; - let mut cs = PlatformAddressChangeSet::default(); - if let Some(info) = wm.get_wallet_info_mut(&self.wallet_id) { - if let Some(account) = info - .core_wallet - .platform_payment_managed_account_at_index_mut(account_index) - { - // `transfer_address_funds` returns address info for the full - // `inputs ∪ outputs` set, including external recipients the - // wallet does not own. Build a lookup of derived addresses - // up-front so we can skip non-owned entries — persisting a - // recipient under a fake derivation index would poison the - // account's address map on restore. - let owned: std::collections::BTreeMap = account - .addresses - .addresses - .iter() - .filter_map(|(&idx, info)| { - match PlatformP2PKHAddress::from_address(&info.address) { - Ok(p) => Some((p, idx)), - Err(e) => { - tracing::warn!( - address = %info.address, - index = idx, - error = %e, - "skipping account address that failed P2PKH conversion", - ); - None - } - } - }) - .collect(); - - for entry in build_transfer_persistence_entries( - self.wallet_id, - account_index, - &owned, - address_infos.iter().map(|(a, i)| (a, i.as_ref())), - ) { - account.set_address_credit_balance( - entry.address, - entry.funds.balance, - key_source.as_ref(), - ); - cs.addresses.push(entry); - } - } - } - drop(wm); - - // Mirror `sync.rs`: persist post-broadcast balances so a restart - // doesn't reseed `auto_select_inputs` from stale rows. Log-on-error - // because the on-chain transition already succeeded. - if !cs.is_empty() { - if let Err(e) = self.persister.store(cs.clone().into()) { - tracing::error!("Failed to persist transfer changeset: {}", e); - } - } - - Ok(cs) + // `transfer_address_funds` returns address info for the full + // `inputs ∪ outputs` set, including external recipients the wallet + // does not own — the shared seam filters those out, applies the + // proof-attested balances, updates the sync seed, and persists. + Ok(self + .reconcile_address_infos(&address_infos, "address transfer") + .await) } /// Transfer credits with an address-keyed fee strategy and an optional @@ -610,56 +549,6 @@ impl PlatformAddressWallet { } } -/// Translate `transfer_address_funds`'s `inputs ∪ outputs` address infos into -/// the persistence-changeset entries for this wallet. Non-P2PKH addresses and -/// addresses outside `owned` (i.e. external recipients) are filtered out — the -/// caller persists only entries that belong to the wallet's derived address -/// pool. Missing per-address info defaults to zero balance / zero nonce, which -/// matches the on-chain post-transition state for a fully consumed input. -fn build_transfer_persistence_entries<'a, I>( - wallet_id: [u8; 32], - account_index: u32, - owned: &BTreeMap, - address_infos: I, -) -> Vec -where - I: IntoIterator< - Item = ( - &'a PlatformAddress, - Option<&'a dash_sdk::query_types::AddressInfo>, - ), - >, -{ - let mut entries = Vec::new(); - for (addr, maybe_info) in address_infos { - let PlatformAddress::P2pkh(hash) = addr else { - continue; - }; - let p2pkh = PlatformP2PKHAddress::new(*hash); - let Some(&address_index) = owned.get(&p2pkh) else { - continue; - }; - let funds = match maybe_info { - Some(ai) => dash_sdk::platform::address_sync::AddressFunds { - balance: ai.balance, - nonce: ai.nonce, - }, - None => dash_sdk::platform::address_sync::AddressFunds { - balance: 0, - nonce: 0, - }, - }; - entries.push(crate::PlatformAddressBalanceEntry { - wallet_id, - account_index, - address_index, - address: p2pkh, - funds, - }); - } - entries -} - /// Build the auto-selection candidate list: keep only addresses whose balance /// reaches `min_input_amount`, drop any address that is also a destination /// output (the protocol forbids the same address being both input and output), @@ -1991,101 +1880,10 @@ mod auto_select_tests { assert!(matches!(err, PlatformWalletError::AddressOperation(_))); } - /// CMT-002: `transfer_address_funds` returns address info for the full - /// `inputs ∪ outputs` set, including external recipients. The persistence - /// builder must keep entries for wallet-owned addresses only — persisting - /// a recipient under a fabricated derivation index would poison the - /// account's address map on restore. - #[test] - fn persistence_filter_drops_external_recipients() { - use dash_sdk::query_types::AddressInfo; - - let wallet_id = [0xAAu8; 32]; - let account_index = 0u32; - - let owned_input = PlatformP2PKHAddress::new([0x01; 20]); - // External recipient — not in `owned`. - let external_recipient_hash = [0xEE; 20]; - let external_recipient = PlatformAddress::P2pkh(external_recipient_hash); - let owned_input_addr = PlatformAddress::P2pkh([0x01; 20]); - - // Wallet's derived pool: only the input address. - let mut owned: BTreeMap = BTreeMap::new(); - owned.insert(owned_input, 7); - - // The proved address-info set drive returns spans inputs ∪ outputs. - // We model the input fully consumed (balance = 0, nonce bumped) and - // the external recipient receiving credits. - let input_info = AddressInfo { - address: owned_input_addr, - nonce: 1, - balance: 0, - }; - let recipient_info = AddressInfo { - address: external_recipient, - nonce: 0, - balance: 5_000_000, - }; - let address_infos: BTreeMap> = [ - (owned_input_addr, Some(input_info)), - (external_recipient, Some(recipient_info)), - ] - .into_iter() - .collect(); - - let entries = build_transfer_persistence_entries( - wallet_id, - account_index, - &owned, - address_infos.iter().map(|(a, i)| (a, i.as_ref())), - ); - - assert_eq!(entries.len(), 1, "external recipient must be filtered out"); - let entry = &entries[0]; - assert_eq!(entry.wallet_id, wallet_id); - assert_eq!(entry.account_index, account_index); - assert_eq!(entry.address, owned_input); - assert_eq!( - entry.address_index, 7, - "owned address must keep its real derivation index" - ); - assert_eq!(entry.funds.balance, 0); - assert_eq!(entry.funds.nonce, 1); - assert!( - !entries - .iter() - .any(|e| e.address == PlatformP2PKHAddress::new(external_recipient_hash)), - "external recipient must not appear in any entry", - ); - } - - /// Missing per-address info defaults to zero balance / zero nonce — this - /// is the on-chain post-transition state for a fully consumed input that - /// drive elided from the proved set. - #[test] - fn persistence_filter_treats_missing_info_as_zero() { - let wallet_id = [0xBBu8; 32]; - let account_index = 3u32; - let owned_addr = PlatformP2PKHAddress::new([0x42; 20]); - let owned_platform = PlatformAddress::P2pkh([0x42; 20]); - - let mut owned: BTreeMap = BTreeMap::new(); - owned.insert(owned_addr, 11); - - let address_infos: BTreeMap> = - [(owned_platform, None)].into_iter().collect(); - - let entries = build_transfer_persistence_entries( - wallet_id, - account_index, - &owned, - address_infos.iter().map(|(a, i)| (a, i.as_ref())), - ); - assert_eq!(entries.len(), 1); - assert_eq!(entries[0].funds.balance, 0); - assert_eq!(entries[0].funds.nonce, 0); - assert_eq!(entries[0].address_index, 11); - } + // The `inputs ∪ outputs` translation (external-recipient filtering, + // missing-info-as-zero) now lives in the shared reconciliation seam — + // see `build_entries_drops_unresolved_and_zeroes_missing_info` in + // `provider.rs`. /// Signer used only by tests that exercise paths which short-circuit /// before any signing happens. Never produces a signature. diff --git a/packages/rs-platform-wallet/src/wallet/platform_addresses/wallet.rs b/packages/rs-platform-wallet/src/wallet/platform_addresses/wallet.rs index 9a0531f84f7..2512df0a5a0 100644 --- a/packages/rs-platform-wallet/src/wallet/platform_addresses/wallet.rs +++ b/packages/rs-platform-wallet/src/wallet/platform_addresses/wallet.rs @@ -1,9 +1,11 @@ //! Platform address wallet for DIP-17 platform payment addresses. +use std::collections::BTreeMap; use std::sync::Arc; use dpp::address_funds::PlatformAddress; use dpp::fee::Credits; +use key_wallet::PlatformP2PKHAddress; use tokio::sync::RwLock; use crate::broadcaster::SpvBroadcaster; @@ -28,8 +30,11 @@ pub struct PlatformAddressWallet { pub(crate) wallet_id: WalletId, /// Single provider covering every platform payment account on the /// wallet. `None` until [`initialize`] runs so that no-account - /// wallets don't allocate empty state. Sync takes a `write` lock; - /// transfer/withdraw paths take `read` for key_source lookups. + /// wallets don't allocate empty state. Both sync and the + /// post-broadcast reconciliation seam + /// ([`reconcile_address_infos`](Self::reconcile_address_infos)) + /// take the `write` lock across their whole apply sequence, which + /// is what serializes them against each other. pub(crate) provider: Arc>>, /// Shared asset-lock manager. Threaded in so the orchestrated /// `fund_from_asset_lock` path can drive @@ -159,72 +164,151 @@ impl PlatformAddressWallet { Ok(()) } - /// Reconcile platform-address balances after an identity - /// top-up-from-addresses, from the proof-attested `address_infos` the SDK - /// returns. Each spent address's `(account_index, address_index)` is - /// resolved from the persisted provider state — covering addresses - /// restored from disk that are no longer in a live derived pool — its - /// in-memory balance is set to the proof's post-spend value, and a - /// `PlatformAddressChangeSet` is persisted so the displayed balance and - /// the next input selection both reflect on-chain reality. + /// Reconcile platform-address balances from the proof-attested + /// `address_infos` an address-funds state transition returns (transfer, + /// withdrawal, asset-lock funding, identity registration / top-up / + /// credit transfer). This is the single apply-and-persist seam every + /// flow routes through. /// - /// Without this, the local balances stay frozen at their pre-top-up - /// values: the wallet keeps displaying a stale "Platform Balance" and the - /// next top-up over-selects the now-drained addresses, which Drive - /// rejects with "Insufficient combined address balances". + /// Each address's `(account_index, address_index)` is resolved through + /// the provider's persisted `index <-> address` bijection — covering + /// addresses restored from disk that are no longer in a live derived + /// pool — with the live pools as fallback for addresses derived since + /// the last sync (e.g. a fresh change address). Non-P2PKH addresses and + /// addresses the wallet doesn't own (external recipients) are skipped. /// - /// Locks are released before persisting (the persistence backend runs its - /// callbacks inline), and errors are logged rather than propagated — - /// Platform already accepted the top-up, and a later sync reconciles. - pub async fn apply_top_up_reconciliation(&self, address_infos: &AddressInfos) { - // Resolve spent addresses to balance entries under the provider read - // lock, then drop it. - let entries = { - let guard = self.provider.read().await; - match guard - .as_ref() - .and_then(|p| p.per_wallet_state(&self.wallet_id)) - { - Some(state) => { - let per_account: Vec<_> = state - .iter() - .map(|(idx, acct)| (*idx, acct.addresses())) - .collect(); - super::provider::build_top_up_balance_entries( - self.wallet_id, - &per_account, - address_infos, - ) - } - None => { - tracing::warn!( - wallet_id = ?self.wallet_id, - "Top-up reconciliation skipped: no platform-address \ - provider state for this wallet; local balances stay \ - stale until the next platform-address sync" - ); - return; + /// For each surviving entry the in-memory account balance is set to the + /// proof's attested value, the provider's committed `found` seed is + /// updated (so the background sync's diff baseline agrees with what we + /// just applied), and a `PlatformAddressChangeSet` is persisted so the + /// displayed balance and the next input selection both reflect on-chain + /// reality. Without this, local balances stay frozen at their + /// pre-transition values: the wallet displays a stale "Platform + /// Balance" and the next input selection over-selects drained + /// addresses, which Drive rejects with "Insufficient combined address + /// balances". + /// + /// A freshness guard protects against racing the 15s background sync: + /// entries whose nonce is below the committed seed's are dropped (a + /// fresher state was already committed), see + /// [`PlatformPaymentAddressProvider::commit_reconciliation`]. The + /// provider write lock is held across the provider commit AND the + /// account-balance write, so a sync pass — which holds the same lock + /// across its scan and its own persist — can never interleave between + /// the two; the lock order (provider → wallet manager) matches the + /// sync callbacks. + /// + /// Persistence errors are logged rather than propagated — Platform + /// already accepted the transition, and a later sync reconciles. + /// + /// [`PlatformPaymentAddressProvider::commit_reconciliation`]: + /// super::provider::PlatformPaymentAddressProvider::commit_reconciliation + pub async fn reconcile_address_infos( + &self, + address_infos: &AddressInfos, + context: &'static str, + ) -> crate::PlatformAddressChangeSet { + if address_infos.is_empty() { + return crate::PlatformAddressChangeSet::default(); + } + + let mut guard = self.provider.write().await; + let Some(provider) = guard.as_mut() else { + tracing::warn!( + wallet_id = ?self.wallet_id, + context, + "Address reconciliation skipped: no platform-address \ + provider for this wallet; local balances stay stale \ + until the next platform-address sync" + ); + return crate::PlatformAddressChangeSet::default(); + }; + if provider.per_wallet_state(&self.wallet_id).is_none() { + tracing::warn!( + wallet_id = ?self.wallet_id, + context, + "Address reconciliation skipped: no platform-address \ + provider state for this wallet; local balances stay \ + stale until the next platform-address sync" + ); + return crate::PlatformAddressChangeSet::default(); + } + + // Live-pool fallback indexes for addresses derived since the last + // sync (not yet merged into the provider bijection). Taking the + // wallet-manager read lock while holding the provider write lock + // follows the provider → wallet-manager order the sync callbacks + // use. + let pool_indexes: BTreeMap = { + let wm = self.wallet_manager.read().await; + let mut out = BTreeMap::new(); + if let Some(info) = wm.get_wallet_info(&self.wallet_id) { + for account in info.core_wallet.all_platform_payment_managed_accounts() { + // The provider tracks key-class-0 accounts only; other + // key classes have no per-account provider state to + // reconcile against. + if account.key_class != 0 { + continue; + } + for (&index, addr_info) in &account.addresses.addresses { + if let Ok(p2pkh) = PlatformP2PKHAddress::from_address(&addr_info.address) { + out.entry(p2pkh).or_insert((account.account, index)); + } + } } } + out }; - if entries.is_empty() { - if !address_infos.is_empty() { - tracing::warn!( - wallet_id = ?self.wallet_id, - spent_addresses = address_infos.len(), - "Top-up reconciliation resolved none of the proof's spent \ - addresses through the persisted provider state; local \ - balances stay stale until the next platform-address sync" - ); - } - return; + + let outcome = provider.commit_reconciliation(&self.wallet_id, address_infos, &pool_indexes); + + if outcome.resolved == 0 { + tracing::warn!( + wallet_id = ?self.wallet_id, + context, + proof_addresses = address_infos.len(), + "Address reconciliation resolved none of the proof's \ + addresses to a wallet-owned slot. Expected when every \ + address belongs to a third party; otherwise local \ + balances stay stale until the next platform-address sync" + ); + return crate::PlatformAddressChangeSet::default(); + } + if outcome.stale_skipped > 0 || outcome.unchanged_skipped > 0 { + tracing::debug!( + wallet_id = ?self.wallet_id, + context, + stale_skipped = outcome.stale_skipped, + unchanged_skipped = outcome.unchanged_skipped, + "Address reconciliation dropped entries superseded by (or \ + identical to) the committed sync seed" + ); } - // Apply the proof-attested post-spend balances in memory, then drop - // the write lock. + if outcome.entries.is_empty() { + return crate::PlatformAddressChangeSet::default(); + } + + // Apply the proof-attested balances to the managed accounts while + // STILL holding the provider write lock, so a background sync can't + // interleave between the provider commit above and this write. + // The per-account key source drives gap-limit extension when a + // previously unfunded address (e.g. a change output) becomes funded. { + let key_sources: BTreeMap = outcome + .entries + .iter() + .map(|e| e.account_index) + .collect::>() + .into_iter() + .filter_map(|account_index| { + provider + .key_source(&self.wallet_id, account_index) + .map(|ks| (account_index, ks)) + }) + .collect(); let mut wm = self.wallet_manager.write().await; if let Some(info) = wm.get_wallet_info_mut(&self.wallet_id) { - for entry in &entries { + for entry in &outcome.entries { if let Some(account) = info .core_wallet .platform_payment_managed_account_at_index_mut(entry.account_index) @@ -232,25 +316,30 @@ impl PlatformAddressWallet { account.set_address_credit_balance( entry.address, entry.funds.balance, - None, + key_sources.get(&entry.account_index), ); } } } } - // Persist with no locks held. + drop(guard); + + // Persist with no locks held (the persistence backend runs its + // callbacks inline). let cs = crate::PlatformAddressChangeSet { - addresses: entries, + addresses: outcome.entries, ..Default::default() }; - if let Err(e) = self.persister.store(cs.into()) { + if let Err(e) = self.persister.store(cs.clone().into()) { tracing::error!( + context, error = %e, - "Failed to persist top-up platform-address reconciliation; \ + "Failed to persist platform-address reconciliation; \ in-memory balances are updated but durable rows stay stale \ until the next platform-address sync" ); } + cs } /// Get the network from the SDK. diff --git a/packages/rs-platform-wallet/src/wallet/platform_addresses/withdrawal.rs b/packages/rs-platform-wallet/src/wallet/platform_addresses/withdrawal.rs index 61695829700..d98116fc457 100644 --- a/packages/rs-platform-wallet/src/wallet/platform_addresses/withdrawal.rs +++ b/packages/rs-platform-wallet/src/wallet/platform_addresses/withdrawal.rs @@ -106,62 +106,14 @@ impl PlatformAddressWallet { } }; - // Get the cached key source from the unified provider for gap - // limit maintenance. - let key_source = { - let guard = self.provider.read().await; - guard - .as_ref() - .and_then(|p| p.key_source(&self.wallet_id, account_index)) - }; - - // Update balances in the ManagedPlatformAccount. - let mut wm = self.wallet_manager.write().await; - let mut cs = PlatformAddressChangeSet::default(); - if let Some(info) = wm.get_wallet_info_mut(&self.wallet_id) { - if let Some(account) = info - .core_wallet - .platform_payment_managed_account_at_index_mut(account_index) - { - for (addr, maybe_info) in address_infos.iter() { - let PlatformAddress::P2pkh(hash) = addr else { - continue; - }; - let p2pkh = PlatformP2PKHAddress::new(*hash); - let funds = match maybe_info { - Some(ai) => dash_sdk::platform::address_sync::AddressFunds { - balance: ai.balance, - nonce: ai.nonce, - }, - None => dash_sdk::platform::address_sync::AddressFunds { - balance: 0, - nonce: 0, - }, - }; - account.set_address_credit_balance(p2pkh, funds.balance, key_source.as_ref()); - let address_index = account - .addresses - .addresses - .iter() - .find_map(|(&idx, info)| { - PlatformP2PKHAddress::from_address(&info.address) - .ok() - .filter(|found| *found == p2pkh) - .map(|_| idx) - }) - .unwrap_or(0); - cs.addresses.push(crate::PlatformAddressBalanceEntry { - wallet_id: self.wallet_id, - account_index, - address_index, - address: p2pkh, - funds, - }); - } - } - } - - Ok(cs) + // Apply + persist the proof-attested post-withdrawal balances via + // the shared seam. Input addresses resolve through the provider's + // persisted index bijection (with live-pool fallback), so a + // restored address that is no longer in a live derived pool keeps + // its real derivation index instead of corrupting index 0. + Ok(self + .reconcile_address_infos(&address_infos, "address withdrawal") + .await) } /// Auto-select all funded addresses for withdrawal. Withdrawals consume diff --git a/packages/rs-platform-wallet/src/wallet/platform_wallet.rs b/packages/rs-platform-wallet/src/wallet/platform_wallet.rs index 7c22401f9c3..3faae689637 100644 --- a/packages/rs-platform-wallet/src/wallet/platform_wallet.rs +++ b/packages/rs-platform-wallet/src/wallet/platform_wallet.rs @@ -24,8 +24,13 @@ use crate::broadcaster::SpvBroadcaster; use crate::changeset::{ ClientStartState, PersistenceError, PlatformWalletChangeSet, PlatformWalletPersistence, }; -#[cfg(feature = "shielded")] use crate::error::PlatformWalletError; +use dash_sdk::platform::transition::put_settings::PutSettings; +use dpp::address_funds::PlatformAddress; +use dpp::fee::Credits; +use dpp::identity::signer::Signer; +use dpp::identity::{Identity, IdentityPublicKey}; +use dpp::prelude::Identifier; /// Unique identifier for a wallet (32-byte hash). pub type WalletId = [u8; 32]; @@ -218,6 +223,128 @@ impl PlatformWallet { } } + // ----------------------------------------------------------------- + // Address-funded identity flows + // + // Composite orchestration: each flow spends or credits platform + // addresses through the identity sub-wallet, then routes the + // proof-attested `AddressInfos` through the platform-address + // sub-wallet's shared reconciliation seam + // (`PlatformAddressWallet::reconcile_address_infos`) so displayed + // balances and the next input selection reflect on-chain reality + // without waiting for the next BLAST sync round. Only this struct + // owns both sub-wallets, which is why the composition lives here. + // ----------------------------------------------------------------- + + /// Top up an existing identity's credit balance by spending platform + /// address balances, then reconcile the spent addresses' local + /// balances and nonces from the proof-attested post-spend + /// `AddressInfos`. + /// + /// See [`IdentityWallet::top_up_from_addresses`] for the identity-side + /// semantics. Reconciliation failures are logged inside the seam + /// rather than propagated — Platform already accepted the top-up, and + /// a later sync reconciles. + /// + /// Returns the identity's new credit balance. + pub async fn top_up_from_addresses + Send + Sync>( + &self, + identity_id: &Identifier, + inputs: BTreeMap, + address_signer: &S, + settings: Option, + ) -> Result { + let (address_infos, new_balance) = self + .identity + .top_up_from_addresses(identity_id, inputs, address_signer, settings) + .await?; + self.platform + .reconcile_address_infos(&address_infos, "identity top-up") + .await; + Ok(new_balance) + } + + /// Register a new identity funded by platform-address balances, then + /// reconcile the spent funding addresses' local balances and nonces + /// from the proof-attested post-spend `AddressInfos`. + /// + /// See [`IdentityWallet::register_from_addresses`] for the + /// identity-side semantics (placeholder construction, signer + /// responsibilities, local-manager registration). Reconciliation + /// failures are logged inside the seam rather than propagated — + /// Platform already accepted the registration, and a later sync + /// reconciles. + /// + /// Returns the registered identity. + #[allow(clippy::too_many_arguments)] + pub async fn register_from_addresses( + &self, + identity: &Identity, + inputs: BTreeMap, + output: Option<(PlatformAddress, Credits)>, + identity_index: u32, + identity_signer: &IS, + input_address_signer: &AS, + settings: Option, + ) -> Result + where + IS: Signer, + AS: Signer + Send + Sync, + { + let (registered_identity, address_infos) = self + .identity + .register_from_addresses( + identity, + inputs, + output, + identity_index, + identity_signer, + input_address_signer, + settings, + ) + .await?; + self.platform + .reconcile_address_infos(&address_infos, "identity registration") + .await; + Ok(registered_identity) + } + + /// Transfer credits from an identity to platform addresses, then + /// reconcile any wallet-owned recipient addresses' local balances + /// from the proof-attested `AddressInfos` (recipients belonging to + /// third parties are skipped by the seam). + /// + /// See [`IdentityWallet::transfer_credits_to_addresses_with_external_signer`] + /// for the identity-side semantics. Reconciliation failures are + /// logged inside the seam rather than propagated — Platform already + /// accepted the transfer, and a later sync reconciles. + /// + /// Returns the sender identity's new credit balance. + pub async fn transfer_credits_to_addresses_with_external_signer( + &self, + identity_id: &Identifier, + recipient_addresses: BTreeMap, + signer: &S, + settings: Option, + ) -> Result + where + S: Signer + Send + Sync, + { + let (address_infos, new_balance) = self + .identity + .transfer_credits_to_addresses_with_external_signer( + identity_id, + recipient_addresses, + signer, + settings, + ) + .await?; + self.platform + .reconcile_address_infos(&address_infos, "credit transfer to addresses") + .await; + Ok(new_balance) + } + /// Non-blocking read-lock. Returns `None` if the lock is currently /// held by a writer, or cannot be acquired without parking the /// thread. Safe to call from any context — never panics, never From 1d48d6fb5239d64ef81a20e3d8797f5578eb7a50 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 3 Jul 2026 03:06:39 +0700 Subject: [PATCH 2/2] fix(platform-wallet): order seam persist under provider lock; restrict pool fallback to tracked accounts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups on the reconciliation seam: - reconcile_address_infos now persists BEFORE releasing the provider write lock, mirroring sync_balances, so both writers' stores are totally ordered by the lock and an older reconciliation row can never overwrite a fresher sync row on disk (the invariant the sync.rs comment documents now holds on both sides). - commit_reconciliation's live-pool fallback is restricted to accounts the provider tracks, and entries are emitted only after the found / bijection commit — restoring the ReconciliationOutcome::entries contract that every emitted entry is committed to the sync seed. An account added after the provider snapshot stays unresolved until initialize / add_provider re-snapshots it; new regression test pins the skip. Co-Authored-By: Claude Fable 5 --- .../src/wallet/platform_addresses/provider.rs | 121 +++++++++++++----- .../src/wallet/platform_addresses/sync.rs | 9 +- .../src/wallet/platform_addresses/wallet.rs | 25 ++-- 3 files changed, 110 insertions(+), 45 deletions(-) diff --git a/packages/rs-platform-wallet/src/wallet/platform_addresses/provider.rs b/packages/rs-platform-wallet/src/wallet/platform_addresses/provider.rs index 86108cfce90..7c29ac6883d 100644 --- a/packages/rs-platform-wallet/src/wallet/platform_addresses/provider.rs +++ b/packages/rs-platform-wallet/src/wallet/platform_addresses/provider.rs @@ -911,8 +911,13 @@ impl PlatformPaymentAddressProvider { /// first — covering addresses restored from disk that are no longer in /// a live derived pool — then falls back to `pool_indexes` (the live /// pools, covering addresses derived since the last sync, e.g. a fresh - /// change address). Pool-resolved addresses are merged into the - /// bijection so `current_balances` can yield their committed funds. + /// change address). The fallback is restricted to accounts the + /// provider tracks: every emitted entry is committed to the `found` + /// seed, so an account without per-account provider state resolves + /// to nothing (it reconciles once `initialize` / `add_provider` + /// re-snapshots the account set and the next sync runs). + /// Pool-resolved addresses are merged into the bijection so + /// `current_balances` can yield their committed funds. /// /// Freshness guard, per resolved entry: /// * zero funds (balance 0, nonce 0 — the address was removed from @@ -947,18 +952,36 @@ impl PlatformPaymentAddressProvider { return Some((account_index, address_index)); } } - pool_indexes.get(p2pkh).copied() + // Pool fallback only for accounts the provider tracks — + // an account added to the live wallet after the provider + // snapshot has no per-account state to commit into, so + // resolving it here would break the contract that every + // emitted entry is committed to the `found` seed. Such + // addresses stay unresolved until `initialize` / + // `add_provider` re-snapshots the account set. + pool_indexes + .get(p2pkh) + .copied() + .filter(|(account_index, _)| wallet_state.contains_key(account_index)) }, address_infos, ); outcome.resolved = resolved_entries.len(); for entry in resolved_entries { - let account_state = wallet_state.get_mut(&entry.account_index); - let existing = account_state - .as_ref() - .and_then(|s| s.found.get(&entry.address)) - .copied(); + // The resolver only yields accounts present in `wallet_state`, + // so a miss here is unreachable; skip defensively rather than + // emit an entry the seed never committed. + let Some(state) = wallet_state.get_mut(&entry.account_index) else { + tracing::warn!( + account_index = entry.account_index, + address = %entry.address, + "commit_reconciliation: resolved account has no tracked \ + provider state; dropping entry" + ); + continue; + }; + let existing = state.found.get(&entry.address).copied(); // Zero funds = the address no longer exists in Platform state. // That removal is attested by the proof, so it bypasses the // nonce guard (the pre-spend seed necessarily has a lower @@ -976,30 +999,28 @@ impl PlatformPaymentAddressProvider { } } } - if let Some(state) = account_state { - if is_removal { - state.found.remove(&entry.address); + if is_removal { + state.found.remove(&entry.address); + } else { + state.found.insert(entry.address, entry.funds); + } + // Merge pool-resolved addresses into the bijection so + // `current_balances` can pair the fresh funds with a + // derivation index. Never overwrite an existing pairing — + // `BiBTreeMap::insert` evicts conflicting pairs, which + // would orphan another address's `found` entry. + if state.addresses.get_by_right(&entry.address).is_none() { + if state.addresses.contains_left(&entry.address_index) { + tracing::error!( + account_index = entry.account_index, + address_index = entry.address_index, + address = %entry.address, + "commit_reconciliation: derivation index already \ + maps to a different address — state drift; \ + leaving the bijection untouched" + ); } else { - state.found.insert(entry.address, entry.funds); - } - // Merge pool-resolved addresses into the bijection so - // `current_balances` can pair the fresh funds with a - // derivation index. Never overwrite an existing pairing — - // `BiBTreeMap::insert` evicts conflicting pairs, which - // would orphan another address's `found` entry. - if state.addresses.get_by_right(&entry.address).is_none() { - if state.addresses.contains_left(&entry.address_index) { - tracing::error!( - account_index = entry.account_index, - address_index = entry.address_index, - address = %entry.address, - "commit_reconciliation: derivation index already \ - maps to a different address — state drift; \ - leaving the bijection untouched" - ); - } else { - state.addresses.insert(entry.address_index, entry.address); - } + state.addresses.insert(entry.address_index, entry.address); } } outcome.entries.push(entry); @@ -1685,6 +1706,44 @@ mod tests { ); } + /// The live-pool fallback must NOT resolve addresses belonging to an + /// account the provider doesn't track: there is no per-account state + /// to commit the funds into, so emitting the entry would violate the + /// contract that every emitted entry is committed to the `found` seed + /// (the applied balance would silently diverge from the sync-diff + /// baseline). + #[test] + fn commit_reconciliation_pool_fallback_skips_untracked_account() { + use dash_sdk::query_types::AddressInfo; + + let known = p2pkh(0x11); + let mut provider = provider_with_one_funded_address(known, funds(700, 3)); + + // Live-pool address on an account (7) the provider has no state for. + let untracked = p2pkh(0x33); + let mut pool_indexes = BTreeMap::new(); + pool_indexes.insert(untracked, (7u32, 0u32)); + + let untracked_addr = PlatformAddress::P2pkh([0x33; 20]); + let mut address_infos = AddressInfos::new(); + address_infos.insert( + untracked_addr, + Some(AddressInfo { + address: untracked_addr, + nonce: 0, + balance: 9_999, + }), + ); + + let outcome = provider.commit_reconciliation(&WALLET, &address_infos, &pool_indexes); + + assert_eq!( + outcome.resolved, 0, + "an untracked account's pool address must stay unresolved" + ); + assert!(outcome.entries.is_empty()); + } + /// An address missing from the provider bijection (derived since the /// last sync, e.g. a fresh change address) resolves through the /// live-pool fallback, and the pair is merged into the bijection so diff --git a/packages/rs-platform-wallet/src/wallet/platform_addresses/sync.rs b/packages/rs-platform-wallet/src/wallet/platform_addresses/sync.rs index 22fe1e1aa92..b332e351898 100644 --- a/packages/rs-platform-wallet/src/wallet/platform_addresses/sync.rs +++ b/packages/rs-platform-wallet/src/wallet/platform_addresses/sync.rs @@ -156,11 +156,10 @@ impl PlatformAddressWallet { // Persist BEFORE releasing the provider lock. The reconciliation // seam (`reconcile_address_infos`) serializes on this lock and - // persists its proof-attested entries after acquiring it, so - // keeping this store inside the critical section guarantees any - // reconciliation that observed this pass's committed state also - // lands on disk after it — releasing first would let a fresher - // reconciliation row be overwritten by this (older) diff. + // persists its proof-attested entries while holding it too, so + // both writers' stores are totally ordered by the lock: whichever + // commits its seed later also lands on disk later, and an older + // diff can never overwrite a fresher row. if !cs.is_empty() { if let Err(e) = self.persister.store(cs.into()) { tracing::error!("Failed to persist address sync changeset: {}", e); diff --git a/packages/rs-platform-wallet/src/wallet/platform_addresses/wallet.rs b/packages/rs-platform-wallet/src/wallet/platform_addresses/wallet.rs index 2512df0a5a0..7261247398a 100644 --- a/packages/rs-platform-wallet/src/wallet/platform_addresses/wallet.rs +++ b/packages/rs-platform-wallet/src/wallet/platform_addresses/wallet.rs @@ -192,11 +192,12 @@ impl PlatformAddressWallet { /// entries whose nonce is below the committed seed's are dropped (a /// fresher state was already committed), see /// [`PlatformPaymentAddressProvider::commit_reconciliation`]. The - /// provider write lock is held across the provider commit AND the - /// account-balance write, so a sync pass — which holds the same lock - /// across its scan and its own persist — can never interleave between - /// the two; the lock order (provider → wallet manager) matches the - /// sync callbacks. + /// provider write lock is held across the provider commit, the + /// account-balance write, AND the persist — mirroring + /// [`sync_balances`](Self::sync_balances), so the two writers' stores + /// are totally ordered by the lock and a sync pass can never + /// interleave between (or persist across) the seam's steps; the lock + /// order (provider → wallet manager) matches the sync callbacks. /// /// Persistence errors are logged rather than propagated — Platform /// already accepted the transition, and a later sync reconciles. @@ -322,10 +323,15 @@ impl PlatformAddressWallet { } } } - drop(guard); - - // Persist with no locks held (the persistence backend runs its - // callbacks inline). + // Persist BEFORE releasing the provider lock, mirroring + // `sync_balances`. Both writers persisting inside the same + // critical section totally orders the stores with the lock: a + // sync pass (or another reconciliation) that commits a fresher + // seed after us also persists after us, so an older row can + // never overwrite a fresher one on disk. Persistence callbacks + // must not re-enter wallet APIs (the pre-existing contract — + // stores already run under the wallet-manager write lock + // elsewhere). let cs = crate::PlatformAddressChangeSet { addresses: outcome.entries, ..Default::default() @@ -339,6 +345,7 @@ impl PlatformAddressWallet { until the next platform-address sync" ); } + drop(guard); cs }