From 6d867c629d39677aad1ebf6660382f2edd39975c Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Mon, 29 Jun 2026 18:30:08 +0700 Subject: [PATCH 1/4] fix(platform-wallet): reconcile platform-address balances after top-up-from-addresses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Production report (2026-06-27): an identity top-up failed with "Insufficient combined address balances: total available is less than required 1500000" even though the wallet showed a Platform Balance of 1.19970434 DASH and the top-up sheet showed "Available: 1.19970434 DASH". Downstream, DPNS registration also failed ("Insufficient identity ... balance 35781660 required 42539440") because the identity could never be topped up. Root cause: IdentityWallet::top_up_from_addresses discarded the proof-attested address_infos the SDK returns — the new on-chain balance + bumped nonce of every platform address the top-up spent from. The local platform-address balances therefore stayed frozen at their pre-top-up values: the wallet kept displaying the stale "Platform Balance", and the next top-up's greedy input selection (driven by those stale local balances) over-selected the now-drained addresses, which Drive rejected. The sibling fund_from_asset_lock path already does the right thing via write_address_balances_changeset, with a comment describing this exact failure. This wires the same reconciliation into the top-up path: write each spent address's proof-attested post-spend balance back into its platform account (in memory) and persist a PlatformAddressChangeSet, so the displayed balance and the next selection both reflect on-chain reality. The shared entry-builder build_transfer_persistence_entries is promoted to pub(crate) and re-exported as build_platform_address_persistence_entries. Spent addresses are already funded, so the None key source is safe — set_address_credit_balance only consults it on a 0->funded transition. Test would have caught this in CI: the regression test top_up_records_post_spend_address_balance_not_stale is ✖ before the fix (cannot find function build_platform_address_persistence_entries — there was no reconciliation at all) and ✔ after. Note: the unit test pins the reconciliation contract; that top_up_from_addresses actually invokes it, and that Drive then accepts the follow-up top-up, are network-gated and need a testnet e2e (top-up -> second top-up succeeds) to fully close. Co-Authored-By: Claude Opus 4.8 --- .../identity/network/top_up_from_addresses.rs | 164 +++++++++++++++++- .../src/wallet/platform_addresses/mod.rs | 5 + .../src/wallet/platform_addresses/transfer.rs | 10 +- 3 files changed, 175 insertions(+), 4 deletions(-) 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 c6940328576..b48fe7d0861 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 @@ -12,7 +12,10 @@ use dash_sdk::platform::transition::top_up_identity_from_addresses::TopUpIdentit use dpp::address_funds::PlatformAddress; use dpp::fee::Credits; +use key_wallet::PlatformP2PKHAddress; + use crate::error::PlatformWalletError; +use crate::PlatformAddressChangeSet; use super::*; @@ -55,7 +58,7 @@ impl IdentityWallet { .ok_or(PlatformWalletError::IdentityNotFound(*identity_id))? }; - let (_address_infos, new_balance) = identity + let (address_infos, new_balance) = identity .top_up_from_addresses(&self.sdk, inputs, address_signer, settings) .await .map_err(|e| { @@ -87,8 +90,167 @@ impl IdentityWallet { ); } } + + // Reconcile the spent platform-address balances from the proof. + // + // The SDK trait returns proof-attested `AddressInfos` carrying the + // new on-chain balance + bumped nonce of every address we just + // spent from. Write those back into the wallet's platform accounts + // and persist them — exactly as the sibling `fund_from_asset_lock` + // path does via `write_address_balances_changeset`. + // + // Without this, the local platform-address balances stay frozen at + // their pre-top-up values: the wallet keeps displaying a stale + // "Platform Balance", and the next top-up's input selection + // over-selects the now-drained addresses, so Drive rejects the + // transition with "Insufficient combined address balances" even + // though the UI shows ample funds. + // + // The spent addresses are already funded, so the `None` key source + // is safe — `set_address_credit_balance` only consults it on a + // `0 -> funded` transition (gap-limit maintenance), never on the + // decrement a spend produces. + let mut addr_cs = PlatformAddressChangeSet::default(); + for account in info.core_wallet.all_platform_payment_managed_accounts_mut() { + let account_index = account.account; + // The spent addresses that belong to THIS account, mapped to + // their derivation index. + let mut owned: BTreeMap = BTreeMap::new(); + for (addr, _) in address_infos.iter() { + let PlatformAddress::P2pkh(hash) = *addr else { + continue; + }; + let p2pkh = PlatformP2PKHAddress::new(hash); + if let Some(index) = + account + .addresses + .addresses + .iter() + .find_map(|(&idx, ainfo)| { + PlatformP2PKHAddress::from_address(&ainfo.address) + .ok() + .filter(|found| *found == p2pkh) + .map(|_| idx) + }) + { + owned.insert(p2pkh, index); + } + } + if owned.is_empty() { + continue; + } + // Apply each proof-attested post-spend balance in memory. + for (addr, maybe_info) in address_infos.iter() { + let PlatformAddress::P2pkh(hash) = *addr else { + continue; + }; + let p2pkh = PlatformP2PKHAddress::new(hash); + if !owned.contains_key(&p2pkh) { + continue; + } + let balance = maybe_info.as_ref().map_or(0, |ai| ai.balance); + account.set_address_credit_balance(p2pkh, balance, None); + } + addr_cs.addresses.extend( + crate::wallet::platform_addresses::build_platform_address_persistence_entries( + self.wallet_id, + account_index, + &owned, + address_infos.iter().map(|(a, i)| (a, i.as_ref())), + ), + ); + } + if !addr_cs.addresses.is_empty() { + if let Err(e) = self.persister.store(addr_cs.into()) { + tracing::error!( + identity = %identity_id, + error = %e, + "Failed to persist platform-address reconciliation after \ + top_up_from_addresses; in-memory balances are updated but durable \ + rows stay stale until the next platform-address sync" + ); + } + } } Ok(new_balance) } } + +#[cfg(test)] +mod tests { + use std::collections::BTreeMap; + + use dash_sdk::query_types::AddressInfo; + use dpp::address_funds::PlatformAddress; + use key_wallet::PlatformP2PKHAddress; + + /// Pins the balance reconciliation that `top_up_from_addresses` MUST + /// perform on its success path. When a top-up spends platform addresses, + /// the SDK returns proof-attested `address_infos` carrying the *new* + /// (decremented) on-chain balance and bumped nonce of every address we + /// spent from. Those must be written back to the wallet's local + /// platform-address balances — exactly as the sibling + /// `fund_from_asset_lock` path does via `write_address_balances_changeset`. + /// + /// Production report (2026-06-27): `top_up_from_addresses` discarded the + /// returned `address_infos`, so the local platform-address balances stayed + /// frozen at their pre-top-up values. The wallet kept displaying the stale + /// "Platform Balance", and the next top-up's greedy input selection + /// over-selected those now-drained addresses, so Drive rejected the + /// transition with "Insufficient combined address balances: total + /// available is less than required …" even though the UI showed ample + /// funds. + /// + /// This test exercises the shared reconciliation helper the fix routes the + /// top-up path through; a spent address whose proof balance is now 5 must + /// produce a persistence entry of 5 (the on-chain truth), never the stale + /// pre-spend value. + #[test] + fn top_up_records_post_spend_address_balance_not_stale() { + let wallet_id = [0xCDu8; 32]; + let account_index = 0u32; + + // An owned platform address we spent FROM during the top-up. The + // wallet locally believed it held a large balance; the proof attests + // the post-spend balance is now 5 credits, nonce bumped to 4. + let spent_hash = [0x11u8; 20]; + let spent = PlatformP2PKHAddress::new(spent_hash); + let spent_addr = PlatformAddress::P2pkh(spent_hash); + + let mut owned: BTreeMap = BTreeMap::new(); + owned.insert(spent, 3); + + let post_spend = AddressInfo { + address: spent_addr, + nonce: 4, + balance: 5, + }; + let address_infos: BTreeMap> = + [(spent_addr, Some(post_spend))].into_iter().collect(); + + let entries = crate::wallet::platform_addresses::build_platform_address_persistence_entries( + wallet_id, + account_index, + &owned, + address_infos.iter().map(|(a, i)| (a, i.as_ref())), + ); + + assert_eq!( + entries.len(), + 1, + "the spent owned address must get a balance entry" + ); + let entry = &entries[0]; + assert_eq!(entry.address, spent); + assert_eq!( + entry.address_index, 3, + "must keep the address's real derivation index" + ); + assert_eq!( + entry.funds.balance, 5, + "must record the proof's post-spend balance, not the stale pre-spend value" + ); + assert_eq!(entry.funds.nonce, 4, "must record the bumped nonce"); + } +} diff --git a/packages/rs-platform-wallet/src/wallet/platform_addresses/mod.rs b/packages/rs-platform-wallet/src/wallet/platform_addresses/mod.rs index 2dd2d1e98d4..ec2ac6075f2 100644 --- a/packages/rs-platform-wallet/src/wallet/platform_addresses/mod.rs +++ b/packages/rs-platform-wallet/src/wallet/platform_addresses/mod.rs @@ -16,6 +16,11 @@ mod transfer; mod wallet; mod withdrawal; +/// Build platform-address persistence-changeset entries from proof-attested +/// post-transition `address_infos`. Shared by the transfer path and the +/// identity top-up-from-addresses balance reconciliation. +pub(crate) use transfer::build_transfer_persistence_entries as build_platform_address_persistence_entries; + /// Saturating sum over `Credits` (== `u64`) — total credit supply is far /// below `u64::MAX`, so saturation is unreachable in practice but the policy /// keeps debug-build panics off the table. Use this only for sums over 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..e4db6fff3d4 100644 --- a/packages/rs-platform-wallet/src/wallet/platform_addresses/transfer.rs +++ b/packages/rs-platform-wallet/src/wallet/platform_addresses/transfer.rs @@ -610,13 +610,17 @@ impl PlatformAddressWallet { } } -/// Translate `transfer_address_funds`'s `inputs ∪ outputs` address infos into -/// the persistence-changeset entries for this wallet. Non-P2PKH addresses and +/// Translate proof-attested `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>( +/// +/// Shared by `transfer_address_funds` and the identity top-up-from-addresses +/// reconciliation (re-exported as +/// [`build_platform_address_persistence_entries`](super::build_platform_address_persistence_entries)). +pub(crate) fn build_transfer_persistence_entries<'a, I>( wallet_id: [u8; 32], account_index: u32, owned: &BTreeMap, From 1bd68c3fd4734d8131e9bbaee0ba2ca14bf5cca3 Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Mon, 29 Jun 2026 23:46:16 +0700 Subject: [PATCH 2/4] fix(platform-wallet): resolve top-up-spent addresses via the provider (cover restored) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the code review on PR #3969: - BLOCKING: the reconciliation scanned only the live derived address pool (account.addresses.addresses), so a top-up that spent an address restored from disk — present in the persisted index bijection but not in the live pool, because initialize_from_persisted hydrates address_balances with a None key source and so never repopulates the pool — left its stale balance behind, preserving the phantom Platform Balance this PR targets. Resolution now goes through the address provider's per-account index<->address bijection (PlatformAddressWallet::apply_top_up_reconciliation), which covers restored addresses. The identity wallet returns the proof's AddressInfos; the FFI reconciles via the platform-address wallet (the side that owns the provider). - Persist after releasing locks: apply_top_up_reconciliation resolves entries under the provider read lock, applies the in-memory balances under the wallet-manager write lock, then persists with no locks held. - Test the reconciliation, not just a helper: extracted the resolution into the pure, unit-testable build_top_up_balance_entries. The new regression build_top_up_entries_resolves_restored_address_outside_live_pool pins that a spent address present only in the persisted bijection is resolved to its derivation index and recorded with the proof's post-spend balance — would fail with the prior live-pool-only lookup, passes now. Reverts the now-unused shared build_transfer_persistence_entries re-export. The FFI ABI is unchanged (Swift unaffected). platform-wallet: 203 tests pass; clippy + fmt clean. Co-Authored-By: Claude Opus 4.8 --- .../src/identity_top_up.rs | 14 +- .../identity/network/top_up_from_addresses.rs | 179 ++---------------- .../src/wallet/platform_addresses/mod.rs | 5 - .../src/wallet/platform_addresses/provider.rs | 116 ++++++++++++ .../src/wallet/platform_addresses/transfer.rs | 10 +- .../src/wallet/platform_addresses/wallet.rs | 79 ++++++++ 6 files changed, 226 insertions(+), 177 deletions(-) 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 44d57d7ecd2..177da07164d 100644 --- a/packages/rs-platform-wallet-ffi/src/identity_top_up.rs +++ b/packages/rs-platform-wallet-ffi/src/identity_top_up.rs @@ -118,11 +118,21 @@ pub unsafe extern "C" fn platform_wallet_top_up_from_addresses_with_signer( let option = PLATFORM_WALLET_STORAGE.with_item(wallet_handle, |wallet| { let identity_wallet = wallet.identity().clone(); + let platform_wallet = wallet.platform().clone(); block_on_worker(async move { let address_signer: &VTableSigner = unsafe { &*(signer_addr as *const VTableSigner) }; - identity_wallet + let (address_infos, new_balance) = identity_wallet .top_up_from_addresses(&identity_id, input_map, address_signer, None) - .await + .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) }) }); let result = unwrap_option_or_return!(option); 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 b48fe7d0861..a689acbb493 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 @@ -12,10 +12,9 @@ use dash_sdk::platform::transition::top_up_identity_from_addresses::TopUpIdentit use dpp::address_funds::PlatformAddress; use dpp::fee::Credits; -use key_wallet::PlatformP2PKHAddress; +use dash_sdk::query_types::AddressInfos; use crate::error::PlatformWalletError; -use crate::PlatformAddressChangeSet; use super::*; @@ -29,6 +28,15 @@ 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). + /// /// # Arguments /// /// * `identity_id` - The identity to top up. @@ -43,7 +51,7 @@ impl IdentityWallet { inputs: BTreeMap, address_signer: &S, settings: Option, - ) -> Result { + ) -> Result<(AddressInfos, Credits), PlatformWalletError> { let identity = { let wm = self.wallet_manager.read().await; let info = wm.get_wallet_info(&self.wallet_id).ok_or_else(|| { @@ -90,167 +98,12 @@ impl IdentityWallet { ); } } - - // Reconcile the spent platform-address balances from the proof. - // - // The SDK trait returns proof-attested `AddressInfos` carrying the - // new on-chain balance + bumped nonce of every address we just - // spent from. Write those back into the wallet's platform accounts - // and persist them — exactly as the sibling `fund_from_asset_lock` - // path does via `write_address_balances_changeset`. - // - // Without this, the local platform-address balances stay frozen at - // their pre-top-up values: the wallet keeps displaying a stale - // "Platform Balance", and the next top-up's input selection - // over-selects the now-drained addresses, so Drive rejects the - // transition with "Insufficient combined address balances" even - // though the UI shows ample funds. - // - // The spent addresses are already funded, so the `None` key source - // is safe — `set_address_credit_balance` only consults it on a - // `0 -> funded` transition (gap-limit maintenance), never on the - // decrement a spend produces. - let mut addr_cs = PlatformAddressChangeSet::default(); - for account in info.core_wallet.all_platform_payment_managed_accounts_mut() { - let account_index = account.account; - // The spent addresses that belong to THIS account, mapped to - // their derivation index. - let mut owned: BTreeMap = BTreeMap::new(); - for (addr, _) in address_infos.iter() { - let PlatformAddress::P2pkh(hash) = *addr else { - continue; - }; - let p2pkh = PlatformP2PKHAddress::new(hash); - if let Some(index) = - account - .addresses - .addresses - .iter() - .find_map(|(&idx, ainfo)| { - PlatformP2PKHAddress::from_address(&ainfo.address) - .ok() - .filter(|found| *found == p2pkh) - .map(|_| idx) - }) - { - owned.insert(p2pkh, index); - } - } - if owned.is_empty() { - continue; - } - // Apply each proof-attested post-spend balance in memory. - for (addr, maybe_info) in address_infos.iter() { - let PlatformAddress::P2pkh(hash) = *addr else { - continue; - }; - let p2pkh = PlatformP2PKHAddress::new(hash); - if !owned.contains_key(&p2pkh) { - continue; - } - let balance = maybe_info.as_ref().map_or(0, |ai| ai.balance); - account.set_address_credit_balance(p2pkh, balance, None); - } - addr_cs.addresses.extend( - crate::wallet::platform_addresses::build_platform_address_persistence_entries( - self.wallet_id, - account_index, - &owned, - address_infos.iter().map(|(a, i)| (a, i.as_ref())), - ), - ); - } - if !addr_cs.addresses.is_empty() { - if let Err(e) = self.persister.store(addr_cs.into()) { - tracing::error!( - identity = %identity_id, - error = %e, - "Failed to persist platform-address reconciliation after \ - top_up_from_addresses; in-memory balances are updated but durable \ - rows stay stale until the next platform-address sync" - ); - } - } } - Ok(new_balance) - } -} - -#[cfg(test)] -mod tests { - use std::collections::BTreeMap; - - use dash_sdk::query_types::AddressInfo; - use dpp::address_funds::PlatformAddress; - use key_wallet::PlatformP2PKHAddress; - - /// Pins the balance reconciliation that `top_up_from_addresses` MUST - /// perform on its success path. When a top-up spends platform addresses, - /// the SDK returns proof-attested `address_infos` carrying the *new* - /// (decremented) on-chain balance and bumped nonce of every address we - /// spent from. Those must be written back to the wallet's local - /// platform-address balances — exactly as the sibling - /// `fund_from_asset_lock` path does via `write_address_balances_changeset`. - /// - /// Production report (2026-06-27): `top_up_from_addresses` discarded the - /// returned `address_infos`, so the local platform-address balances stayed - /// frozen at their pre-top-up values. The wallet kept displaying the stale - /// "Platform Balance", and the next top-up's greedy input selection - /// over-selected those now-drained addresses, so Drive rejected the - /// transition with "Insufficient combined address balances: total - /// available is less than required …" even though the UI showed ample - /// funds. - /// - /// This test exercises the shared reconciliation helper the fix routes the - /// top-up path through; a spent address whose proof balance is now 5 must - /// produce a persistence entry of 5 (the on-chain truth), never the stale - /// pre-spend value. - #[test] - fn top_up_records_post_spend_address_balance_not_stale() { - let wallet_id = [0xCDu8; 32]; - let account_index = 0u32; - - // An owned platform address we spent FROM during the top-up. The - // wallet locally believed it held a large balance; the proof attests - // the post-spend balance is now 5 credits, nonce bumped to 4. - let spent_hash = [0x11u8; 20]; - let spent = PlatformP2PKHAddress::new(spent_hash); - let spent_addr = PlatformAddress::P2pkh(spent_hash); - - let mut owned: BTreeMap = BTreeMap::new(); - owned.insert(spent, 3); - - let post_spend = AddressInfo { - address: spent_addr, - nonce: 4, - balance: 5, - }; - let address_infos: BTreeMap> = - [(spent_addr, Some(post_spend))].into_iter().collect(); - - let entries = crate::wallet::platform_addresses::build_platform_address_persistence_entries( - wallet_id, - account_index, - &owned, - address_infos.iter().map(|(a, i)| (a, i.as_ref())), - ); - - assert_eq!( - entries.len(), - 1, - "the spent owned address must get a balance entry" - ); - let entry = &entries[0]; - assert_eq!(entry.address, spent); - assert_eq!( - entry.address_index, 3, - "must keep the address's real derivation index" - ); - assert_eq!( - entry.funds.balance, 5, - "must record the proof's post-spend balance, not the stale pre-spend value" - ); - assert_eq!(entry.funds.nonce, 4, "must record the bumped nonce"); + // 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. + Ok((address_infos, new_balance)) } } diff --git a/packages/rs-platform-wallet/src/wallet/platform_addresses/mod.rs b/packages/rs-platform-wallet/src/wallet/platform_addresses/mod.rs index ec2ac6075f2..2dd2d1e98d4 100644 --- a/packages/rs-platform-wallet/src/wallet/platform_addresses/mod.rs +++ b/packages/rs-platform-wallet/src/wallet/platform_addresses/mod.rs @@ -16,11 +16,6 @@ mod transfer; mod wallet; mod withdrawal; -/// Build platform-address persistence-changeset entries from proof-attested -/// post-transition `address_infos`. Shared by the transfer path and the -/// identity top-up-from-addresses balance reconciliation. -pub(crate) use transfer::build_transfer_persistence_entries as build_platform_address_persistence_entries; - /// Saturating sum over `Credits` (== `u64`) — total credit supply is far /// below `u64::MAX`, so saturation is unreachable in practice but the policy /// keeps debug-build panics off the table. Use this only for sums over 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 d56c004c122..4516f6d0416 100644 --- a/packages/rs-platform-wallet/src/wallet/platform_addresses/provider.rs +++ b/packages/rs-platform-wallet/src/wallet/platform_addresses/provider.rs @@ -32,9 +32,12 @@ use key_wallet_manager::WalletManager; use crate::error::PlatformWalletError; use crate::wallet::platform_wallet::{PlatformWalletInfo, WalletId}; +use crate::PlatformAddressBalanceEntry; use dash_sdk::platform::address_sync::{ AddressFunds, AddressIndex, AddressProvider, AddressSyncResult, }; +use dash_sdk::query_types::AddressInfos; +use dpp::address_funds::PlatformAddress; use tokio::sync::RwLock; /// DIP-17 address coordinates used as both the pending-bimap key and @@ -202,6 +205,18 @@ pub(crate) struct PlatformPaymentAddressProvider { } impl PlatformPaymentAddressProvider { + /// The committed per-account index/balance state for one wallet, or + /// `None` if the provider doesn't cover it. Exposes the full persisted + /// `index <-> address` bijection — including addresses restored from + /// disk that are no longer in a live derived pool — so callers can map a + /// spent address back to its derivation index. + pub(crate) fn per_wallet_state( + &self, + wallet_id: &WalletId, + ) -> Option<&PerWalletPlatformAddressState> { + self.per_wallet.get(wallet_id) + } + /// Build a provider covering every platform payment account on /// each wallet in `wallet_ids`. /// @@ -781,6 +796,55 @@ 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. +/// +/// 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( + wallet_id: WalletId, + per_account_addresses: &[(u32, &BiBTreeMap)], + address_infos: &AddressInfos, +) -> Vec { + let mut entries = Vec::new(); + 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) => AddressFunds { + balance: ai.balance, + nonce: ai.nonce, + }, + None => AddressFunds { + balance: 0, + 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 +} + #[cfg(test)] mod tests { use super::*; @@ -807,6 +871,58 @@ mod tests { AddressFunds { balance, nonce } } + /// Regression for the top-up reconciliation: a spent platform address + /// that exists only in the persisted index bijection (restored from + /// disk — not in any live derived pool) must still be resolved to its + /// derivation index and recorded with the proof's post-spend balance. + /// 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() { + use dash_sdk::query_types::AddressInfo; + + // Present in the persisted bijection at index 3, but NOT in a live + // derived address pool. + let restored = p2pkh(0x11); + let mut bimap: BiBTreeMap = BiBTreeMap::new(); + bimap.insert(3, restored); + + // The top-up spent it; the proof attests post-spend balance 5, + // nonce bumped to 4. + let restored_addr = PlatformAddress::P2pkh([0x11; 20]); + let mut address_infos = AddressInfos::new(); + address_infos.insert( + restored_addr, + Some(AddressInfo { + address: restored_addr, + nonce: 4, + balance: 5, + }), + ); + + let per_account: [(u32, &BiBTreeMap); 1] = + [(ACCOUNT, &bimap)]; + let entries = build_top_up_balance_entries(WALLET, &per_account, &address_infos); + + assert_eq!( + entries.len(), + 1, + "a restored spent address must still be reconciled" + ); + let e = &entries[0]; + assert_eq!(e.address, restored); + assert_eq!(e.account_index, ACCOUNT); + assert_eq!( + e.address_index, 3, + "index resolved from the persisted bijection, not the live pool" + ); + assert_eq!( + e.funds.balance, 5, + "records the proof's post-spend balance, not a stale value" + ); + assert_eq!(e.funds.nonce, 4, "records the bumped nonce"); + } + /// 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. 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 e4db6fff3d4..c5eb0a51100 100644 --- a/packages/rs-platform-wallet/src/wallet/platform_addresses/transfer.rs +++ b/packages/rs-platform-wallet/src/wallet/platform_addresses/transfer.rs @@ -610,17 +610,13 @@ impl PlatformAddressWallet { } } -/// Translate proof-attested `inputs ∪ outputs` address infos into the -/// persistence-changeset entries for this wallet. Non-P2PKH addresses and +/// 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. -/// -/// Shared by `transfer_address_funds` and the identity top-up-from-addresses -/// reconciliation (re-exported as -/// [`build_platform_address_persistence_entries`](super::build_platform_address_persistence_entries)). -pub(crate) fn build_transfer_persistence_entries<'a, I>( +fn build_transfer_persistence_entries<'a, I>( wallet_id: [u8; 32], account_index: u32, owned: &BTreeMap, 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 a4bb1bd1e53..2116d9f717e 100644 --- a/packages/rs-platform-wallet/src/wallet/platform_addresses/wallet.rs +++ b/packages/rs-platform-wallet/src/wallet/platform_addresses/wallet.rs @@ -16,6 +16,8 @@ use crate::wallet::persister::WalletPersister; use super::provider::PlatformPaymentAddressProvider; +use dash_sdk::query_types::AddressInfos; + /// Platform address wallet providing DIP-17 platform payment address functionality. #[derive(Clone)] pub struct PlatformAddressWallet { @@ -157,6 +159,83 @@ 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. + /// + /// 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". + /// + /// 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 => Vec::new(), + } + }; + if entries.is_empty() { + return; + } + // Apply the proof-attested post-spend balances in memory, then drop + // the write lock. + { + let mut wm = self.wallet_manager.write().await; + if let Some(info) = wm.get_wallet_info_mut(&self.wallet_id) { + for entry in &entries { + if let Some(account) = info + .core_wallet + .platform_payment_managed_account_at_index_mut(entry.account_index) + { + account.set_address_credit_balance( + entry.address, + entry.funds.balance, + None, + ); + } + } + } + } + // Persist with no locks held. + let cs = crate::PlatformAddressChangeSet { + addresses: entries, + ..Default::default() + }; + if let Err(e) = self.persister.store(cs.into()) { + tracing::error!( + error = %e, + "Failed to persist top-up platform-address reconciliation; \ + in-memory balances are updated but durable rows stay stale \ + until the next platform-address sync" + ); + } + } + /// Get the network from the SDK. pub fn network(&self) -> key_wallet::Network { self.sdk.network From c0d5ac78bf0130b5749a322cde1157ad65370f65 Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Tue, 30 Jun 2026 13:49:34 +0700 Subject: [PATCH 3/4] test(platform-wallet): pin apply_top_up_reconciliation persist contract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the carried-forward review finding (thepastaclaw #3): the prior regression coverage exercised only the pure entry builder (build_top_up_balance_entries), so a future change that stopped apply_top_up_reconciliation from persisting — the exact shape of the original stale-balance bug — would not be caught. Adds apply_top_up_reconciliation_persists_decremented_balance: a recording-persister integration test that injects a provider whose persisted bijection knows the spent address, runs the reconciliation, and asserts a PlatformAddressChangeSet carrying the proof's post-spend balance (decremented, bumped nonce) was actually persisted. Verified red→green: with the persist call in apply_top_up_reconciliation removed (simulating the original discard bug) the test fails ("a persisted platform-address entry for the spent address"); restored, it passes. 204 platform-wallet lib tests pass; fmt clean. Co-Authored-By: Claude Opus 4.8 --- .../src/wallet/platform_addresses/provider.rs | 113 ++++++++++++++++++ 1 file changed, 113 insertions(+) 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 4516f6d0416..717d81e783b 100644 --- a/packages/rs-platform-wallet/src/wallet/platform_addresses/provider.rs +++ b/packages/rs-platform-wallet/src/wallet/platform_addresses/provider.rs @@ -1221,4 +1221,117 @@ mod tests { "on_address_absent must zero the in-memory managed-account balance" ); } + + /// Records every changeset handed to `store`, so a test can assert what + /// the reconciliation actually persisted. + #[derive(Default)] + struct CapturingPersister { + stored: std::sync::Mutex>, + } + + impl crate::changeset::PlatformWalletPersistence for CapturingPersister { + fn store( + &self, + _wallet_id: WalletId, + changeset: crate::changeset::PlatformWalletChangeSet, + ) -> Result<(), crate::changeset::PersistenceError> { + self.stored.lock().expect("persister mutex").push(changeset); + Ok(()) + } + + fn flush(&self, _wallet_id: WalletId) -> Result<(), crate::changeset::PersistenceError> { + Ok(()) + } + + fn load( + &self, + ) -> Result + { + Ok(crate::changeset::ClientStartState::default()) + } + } + + /// Integration regression for the top-up reconciliation *contract* — not + /// just the pure entry builder. `apply_top_up_reconciliation` 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. + #[tokio::test] + async fn apply_top_up_reconciliation_persists_decremented_balance() { + use crate::broadcaster::SpvBroadcaster; + use crate::events::PlatformEventManager; + use crate::spv::SpvRuntime; + use crate::wallet::asset_lock::manager::AssetLockManager; + use crate::wallet::persister::WalletPersister; + use crate::wallet::platform_addresses::PlatformAddressWallet; + use dash_sdk::query_types::AddressInfo; + use tokio::sync::Notify; + + 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 + // 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))); + let persister = WalletPersister::new(WALLET, recorder.clone()); + let event_manager = Arc::new(PlatformEventManager::new(Vec::new())); + let spv = Arc::new(SpvRuntime::new(Arc::clone(&wallet_manager), event_manager)); + let broadcaster = Arc::new(SpvBroadcaster::new(spv)); + let asset_locks = Arc::new(AssetLockManager::new( + Arc::clone(&sdk), + Arc::clone(&wallet_manager), + WALLET, + Arc::new(Notify::new()), + broadcaster, + persister.clone(), + )); + let wallet = PlatformAddressWallet::new( + sdk, + Arc::clone(&wallet_manager), + WALLET, + asset_locks, + persister, + ); + + // The provider knows the spent address via its persisted bijection + // (pre-spend balance 100). + let addr = p2pkh(0x11); + let provider = + provider_tracking_address(Arc::clone(&wallet_manager), WALLET, addr, funds(100, 1)); + *wallet.provider.write().await = Some(provider); + + // The top-up spent it; the proof attests post-spend balance 5, nonce 4. + let spent = PlatformAddress::P2pkh([0x11; 20]); + let mut address_infos = AddressInfos::new(); + address_infos.insert( + spent, + Some(AddressInfo { + address: spent, + nonce: 4, + balance: 5, + }), + ); + + wallet.apply_top_up_reconciliation(&address_infos).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); + 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"); + } } From e9aff46b208232581b503f2aa0d58c051a4cb12c Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 3 Jul 2026 01:31:07 +0700 Subject: [PATCH 4/4] fix(platform-wallet): warn when top-up reconciliation cannot resolve spent addresses The two silent no-op paths in apply_top_up_reconciliation (no provider state for the wallet; none of the proof's spent addresses resolving through the persisted bijection) reproduce the phantom-balance symptom this PR fixes with zero diagnostic output. Emit a warn with the wallet id and spent-address count so the next field report is attributable. Co-Authored-By: Claude Fable 5 --- .../src/wallet/platform_addresses/wallet.rs | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) 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 12bf45ba4e1..9a0531f84f7 100644 --- a/packages/rs-platform-wallet/src/wallet/platform_addresses/wallet.rs +++ b/packages/rs-platform-wallet/src/wallet/platform_addresses/wallet.rs @@ -196,10 +196,27 @@ impl PlatformAddressWallet { address_infos, ) } - None => Vec::new(), + 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; + } } }; 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; } // Apply the proof-attested post-spend balances in memory, then drop