From 462f52404679fe3bc17af80ae820182c47e4e688 Mon Sep 17 00:00:00 2001 From: romchornyi Date: Mon, 2 Nov 2026 17:15:05 +0200 Subject: [PATCH 1/2] fix(platform-wallet): carry a resumed asset lock's change into the store An asset lock whose InstantSend lock was not persisted before the app stopped is restored at load as a synthetic record with no output details. When the lock or the block arrives, key-wallet confirms the change in memory and re-emits that record; the FFI stores derive coin flags only from a record's output details, so the change row kept is_confirmed=false / is_instantlocked=false for good and the coin never became spendable after the next launch. The restored record now carries the coins its account holds from the transaction, classified with key-wallet's own account match (held_output_details), so the re-emit writes the change's new flags. Co-Authored-By: Claude Opus 5.5 --- .../rs-platform-wallet-ffi/src/persistence.rs | 339 +++++++++++++++++- .../src/wallet/asset_lock/mod.rs | 1 + .../src/wallet/asset_lock/restored_record.rs | 89 +++++ 3 files changed, 421 insertions(+), 8 deletions(-) create mode 100644 packages/rs-platform-wallet/src/wallet/asset_lock/restored_record.rs diff --git a/packages/rs-platform-wallet-ffi/src/persistence.rs b/packages/rs-platform-wallet-ffi/src/persistence.rs index aba5e2ed668..28d1f886bf8 100644 --- a/packages/rs-platform-wallet-ffi/src/persistence.rs +++ b/packages/rs-platform-wallet-ffi/src/persistence.rs @@ -36,6 +36,7 @@ use platform_wallet::changeset::{ PlatformWalletChangeSet, PlatformWalletPersistence, ProviderKeyAccountEntry, ProviderKeyExtendedPubKey, PERSISTENCE_CAPABILITIES_VERSION, }; +use platform_wallet::wallet::asset_lock::restored_record::held_output_details; use platform_wallet::wallet::platform_wallet::WalletId; #[cfg(feature = "shielded")] use platform_wallet::wallet::shielded::ShieldedActivityStatus; @@ -5864,13 +5865,16 @@ fn build_wallet_start_state( // `AssetLockManager::resolve_status_with_in_memory` path // builds a `ChainAssetLockProof` from the row's block info. // - // Each restored record is synthetic: `input_details` / - // `output_details` are empty (we don't have the per-account - // role classification at load time), `net_amount` is zero, and - // `account_type` reflects the funding BIP44 slot. None of - // those fields are read by the chain-lock cascade — only - // `context` and `height()` matter for the - // `apply_chain_lock` → bridge → persister loop. + // Each restored record is synthetic: `input_details` are empty, + // `net_amount` is zero and the direction is `Internal`, so a + // re-emit of the record writes the transaction row as a + // zero-amount internal transfer. The asset-lock proof path reads + // only `context` and `height()`. `output_details` carry the coins + // this account still holds from the transaction + // (`held_output_details`, built from the UTXOs restored above): + // when the InstantSend lock or block arrives, key-wallet re-emits + // this record and the store takes those coins' new flags from it. + // Without them the change would stay unconfirmed in the store. let unresolved_recs: &[UnresolvedAssetLockTxRecordFFI] = if entry.unresolved_asset_lock_tx_records.is_null() || entry.unresolved_asset_lock_tx_records_count == 0 @@ -7087,6 +7091,11 @@ fn restore_unresolved_asset_lock_tx_records( }; let account_type = account.managed_account_type().to_account_type(); + // The coins this account holds from the transaction, classified the + // way the live checker would. When the transaction's InstantSend + // lock or block arrives, key-wallet re-emits this record as-is, and + // the store takes each coin's new flags from these details. + let output_details = held_output_details(account, &tx); // Classify from the transaction itself, the way the upstream // router does: an `AssetLockPayloadType` special-tx payload IS // the definition of an asset lock. This array carries both the @@ -7117,7 +7126,7 @@ fn restore_unresolved_asset_lock_tx_records( // direction; this is just the most-correct tag. TransactionDirection::Internal, Vec::new(), - Vec::new(), + output_details, 0, ); account.transactions_mut().insert(record.txid, record); @@ -10109,16 +10118,26 @@ mod tests { ); } + use crate::core_wallet_types::{free_wallet_changeset_ffi, WalletChangeSetFFI}; use dashcore::blockdata::transaction::txin::TxIn; use dashcore::blockdata::transaction::txout::TxOut; use dashcore::blockdata::transaction::Transaction; use dashcore::consensus::encode::serialize; + use dashcore::ephemerealdata::instant_lock::InstantLock; + use dashcore::hashes::Hash; use dashcore::secp256k1::Secp256k1; + use dashcore::transaction::special_transaction::asset_lock::AssetLockPayload; + use dashcore::transaction::special_transaction::TransactionPayload; + use dashcore::{BlockHash, OutPoint, Txid}; use dashcore::{Network, ScriptBuf}; use key_wallet::account::{Account, AccountType, StandardAccountType}; use key_wallet::bip32::{ExtendedPrivKey, ExtendedPubKey}; use key_wallet::managed_account::managed_account_trait::ManagedAccountTrait; + use key_wallet::managed_account::transaction_record::{OutputRole, TransactionRecord}; use key_wallet::mnemonic::Mnemonic; + use key_wallet::test_utils::TestWalletContext; + use key_wallet::transaction_checking::{BlockInfo, TransactionContext}; + use key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; use key_wallet::wallet::Wallet; /// Regression: restored pool addresses must be tagged with the @@ -10255,6 +10274,310 @@ mod tests { drop(tx_buf); } + /// Where the asset lock in [`reloaded_after_unlocked_asset_lock`] returns + /// its change. + #[derive(Clone, Copy)] + enum ChangeTo { + /// The funding BIP44 account's own change pool. + FundingAccount, + /// The BIP32 account — not the one the restored record is filed under. + OtherAccount, + } + + const ASSET_LOCK_CREDITS: u64 = 60_000_000; + const ASSET_LOCK_CHANGE: u64 = 39_999_000; + + /// A wallet that locked part of one confirmed coin into an asset lock + /// (burn output at vout 0, the credit output in the payload) and got the + /// rest back as change at vout 1, saw the lock only in the mempool (no + /// InstantSend lock), and was then reloaded: the coins are restored, the + /// lock's record is not, and the funding transaction comes back as an + /// unresolved asset-lock record — what a store hands back after the app + /// was killed between the broadcast and the lock. Returns the wallet, the + /// funding transaction, and the outpoint of its change. + fn reloaded_after_unlocked_asset_lock( + rt: &tokio::runtime::Runtime, + change_to: ChangeTo, + ) -> (TestWalletContext, Transaction, OutPoint) { + let mut ctx = TestWalletContext::new_random(); + let funding = Transaction::dummy( + &ctx.receive_address, + 0..1, + &[ASSET_LOCK_CREDITS + ASSET_LOCK_CHANGE + 1_000], + ); + rt.block_on(ctx.check_transaction( + &funding, + TransactionContext::InChainLockedBlock(BlockInfo::new( + 1, + BlockHash::all_zeros(), + 1_700_000_000, + )), + )); + + let change_address = match change_to { + ChangeTo::FundingAccount => ctx + .managed_wallet + .first_bip44_managed_account_mut() + .expect("bip44 managed account") + .next_change_address(Some(&ctx.xpub), true) + .expect("bip44 change address"), + ChangeTo::OtherAccount => { + let xpub = ctx + .wallet + .accounts + .standard_bip32_accounts + .get(&0) + .expect("bip32 account") + .account_xpub; + ctx.managed_wallet + .first_bip32_managed_account_mut() + .expect("bip32 managed account") + .next_receive_address(Some(&xpub), true) + .expect("bip32 receive address") + } + }; + let credit_to = TestWalletContext::new_random().receive_address; + let lock = Transaction { + version: 3, + lock_time: 0, + input: vec![TxIn { + previous_output: OutPoint { + txid: funding.txid(), + vout: 0, + }, + script_sig: ScriptBuf::new(), + sequence: 0xffffffff, + witness: dashcore::Witness::default(), + }], + output: vec![ + TxOut { + value: ASSET_LOCK_CREDITS, + script_pubkey: ScriptBuf::new_op_return(&[]), + }, + TxOut { + value: ASSET_LOCK_CHANGE, + script_pubkey: change_address.script_pubkey(), + }, + ], + special_transaction_payload: Some(TransactionPayload::AssetLockPayloadType( + AssetLockPayload::new(vec![TxOut { + value: ASSET_LOCK_CREDITS, + script_pubkey: credit_to.script_pubkey(), + }]), + )), + }; + let seen = rt.block_on(ctx.check_transaction(&lock, TransactionContext::Mempool)); + assert!(seen.is_relevant, "the asset lock must be the wallet's"); + let change = OutPoint { + txid: lock.txid(), + vout: 1, + }; + + // Reload: the in-memory records go, the coins stay. + for account in ctx.managed_wallet.accounts.all_funding_accounts_mut() { + account.transactions_mut().remove(&lock.txid()); + } + restore_mempool_lock_record(&mut ctx.managed_wallet, &lock); + + (ctx, lock, change) + } + + /// Restore `tx` as an unresolved asset-lock record at account index 0, + /// as a store does for a lock it last saw in the mempool. + fn restore_mempool_lock_record(wallet_info: &mut ManagedWalletInfo, tx: &Transaction) { + let mut tx_buf = serialize(tx); + let rec = UnresolvedAssetLockTxRecordFFI { + account_index: 0, + tx_bytes: tx_buf.as_mut_ptr(), + tx_bytes_len: tx_buf.len(), + context_raw: 0, + block_height: 0, + block_hash: [0u8; 32], + block_timestamp: 0, + first_seen: 1_700_000_100, + }; + let stats = restore_unresolved_asset_lock_tx_records(wallet_info, &[rec]) + .expect("a well-formed row restores"); + assert_eq!(stats.restored, 1); + drop(tx_buf); + } + + /// Every account's record of `txid` — the slices the bridge re-reads + /// from the wallet when an InstantSend lock arrives for a known + /// transaction. + fn wallet_slices(wallet_info: &ManagedWalletInfo, txid: &Txid) -> Vec { + wallet_info + .accounts + .all_accounts() + .into_iter() + .filter_map(|account| account.transactions().get(txid).cloned()) + .collect() + } + + /// The coins a store adds or refreshes for `records`, as + /// `(outpoint, height, is_confirmed, is_instantlocked)` — read through the + /// same projection the Swift and Kotlin stores apply. + fn stored_coin_flags(records: Vec) -> Vec<(OutPoint, u32, bool, bool)> { + let cs = platform_wallet::changeset::CoreChangeSet { + records: records.clone(), + account_records: records, + ..Default::default() + }; + let ffi = WalletChangeSetFFI::from_changeset(&cs); + let mut flags = Vec::new(); + let accounts: &[_] = if ffi.accounts.is_null() { + &[] + } else { + unsafe { slice::from_raw_parts(ffi.accounts, ffi.accounts_count) } + }; + for account in accounts { + if account.utxos_added.is_null() { + continue; + } + let utxos = + unsafe { slice::from_raw_parts(account.utxos_added, account.utxos_added_count) }; + for utxo in utxos { + flags.push(( + OutPoint { + txid: Txid::from_byte_array(utxo.outpoint.txid), + vout: utxo.outpoint.vout, + }, + utxo.height, + utxo.is_confirmed, + utxo.is_instantlocked, + )); + } + } + unsafe { free_wallet_changeset_ffi(&ffi) }; + flags + } + + /// The restored record names the change the lock pays back to its own + /// account — the only coin of the lock that account holds — and nothing + /// else: not the burn output, not the credit output. + #[test] + fn should_name_the_held_change_on_a_restored_lock_record() { + let rt = tokio::runtime::Runtime::new().expect("runtime"); + let (ctx, lock, change) = reloaded_after_unlocked_asset_lock(&rt, ChangeTo::FundingAccount); + + let record = ctx + .managed_wallet + .first_bip44_managed_account() + .expect("bip44 managed account") + .transactions() + .get(&lock.txid()) + .expect("restored record"); + assert_eq!(record.output_details.len(), 1); + let detail = &record.output_details[0]; + assert_eq!(detail.index, change.vout); + assert_eq!(detail.role, OutputRole::Change); + assert_eq!(detail.value, ASSET_LOCK_CHANGE); + } + + /// A change coin the account no longer holds gets no detail: a store + /// whose row for it is already gone would insert it as unspent. + #[test] + fn should_leave_a_coin_no_longer_held_off_a_restored_lock_record() { + let rt = tokio::runtime::Runtime::new().expect("runtime"); + let (mut ctx, lock, change) = + reloaded_after_unlocked_asset_lock(&rt, ChangeTo::FundingAccount); + let account = ctx + .managed_wallet + .first_bip44_managed_account_mut() + .expect("bip44 managed account"); + account.utxos.remove(&change); + account.transactions_mut().remove(&lock.txid()); + restore_mempool_lock_record(&mut ctx.managed_wallet, &lock); + + let record = ctx + .managed_wallet + .first_bip44_managed_account() + .expect("bip44 managed account") + .transactions() + .get(&lock.txid()) + .expect("restored record"); + assert!(record.output_details.is_empty()); + } + + /// The block that confirms a restored lock reaches the store: the change + /// is written back confirmed, at the block's height. + #[test] + fn should_store_the_change_confirmed_when_a_restored_lock_is_mined() { + let rt = tokio::runtime::Runtime::new().expect("runtime"); + let (mut ctx, lock, change) = + reloaded_after_unlocked_asset_lock(&rt, ChangeTo::FundingAccount); + + let result = rt.block_on(ctx.check_transaction( + &lock, + TransactionContext::InBlock(BlockInfo::new(500, BlockHash::all_zeros(), 1_700_000_500)), + )); + let records = result + .new_records + .into_iter() + .chain(result.updated_records) + .collect(); + + let flags = stored_coin_flags(records); + assert!( + flags.contains(&(change, 500, true, false)), + "the store must receive the change as confirmed at 500, got {flags:?}" + ); + } + + /// An InstantSend lock arriving for a restored lock reaches the store: + /// the change is written back locked, so it is spendable before the + /// block. + #[test] + fn should_store_the_change_locked_when_a_restored_lock_gets_its_instant_lock() { + let rt = tokio::runtime::Runtime::new().expect("runtime"); + let (mut ctx, lock, change) = + reloaded_after_unlocked_asset_lock(&rt, ChangeTo::FundingAccount); + + assert!(ctx + .managed_wallet + .mark_instant_send_utxos(&lock.txid(), &InstantLock::default())); + + let flags = stored_coin_flags(wallet_slices(&ctx.managed_wallet, &lock.txid())); + assert!( + flags.contains(&(change, 0, false, true)), + "the store must receive the change as InstantSend-locked, got {flags:?}" + ); + } + + /// Change paid to another account than the one the restored record is + /// filed under: the restored record does not claim it, and the block + /// still reaches the store through that account's own record. + #[test] + fn should_store_another_accounts_change_confirmed_when_a_restored_lock_is_mined() { + let rt = tokio::runtime::Runtime::new().expect("runtime"); + let (mut ctx, lock, change) = + reloaded_after_unlocked_asset_lock(&rt, ChangeTo::OtherAccount); + let restored = ctx + .managed_wallet + .first_bip44_managed_account() + .expect("bip44 managed account") + .transactions() + .get(&lock.txid()) + .expect("restored record"); + assert!(restored.output_details.is_empty()); + + let result = rt.block_on(ctx.check_transaction( + &lock, + TransactionContext::InBlock(BlockInfo::new(500, BlockHash::all_zeros(), 1_700_000_500)), + )); + let records = result + .new_records + .into_iter() + .chain(result.updated_records) + .collect(); + + let flags = stored_coin_flags(records); + assert!( + flags.contains(&(change, 500, true, false)), + "the store must receive the other account's change as confirmed, got {flags:?}" + ); + } + /// Helper: build a `ManagedWalletInfo` with a single BIP44 /// account at `index`. Uses a hard-coded valid xpub so the /// construction is deterministic; the test cares about the diff --git a/packages/rs-platform-wallet/src/wallet/asset_lock/mod.rs b/packages/rs-platform-wallet/src/wallet/asset_lock/mod.rs index 9cbc65944ff..28469ef9be7 100644 --- a/packages/rs-platform-wallet/src/wallet/asset_lock/mod.rs +++ b/packages/rs-platform-wallet/src/wallet/asset_lock/mod.rs @@ -7,6 +7,7 @@ mod build; pub mod lock_notify_handler; pub mod manager; pub mod orchestration; +pub mod restored_record; pub(crate) mod sync; pub mod tracked; diff --git a/packages/rs-platform-wallet/src/wallet/asset_lock/restored_record.rs b/packages/rs-platform-wallet/src/wallet/asset_lock/restored_record.rs new file mode 100644 index 00000000000..3ac07b4e345 --- /dev/null +++ b/packages/rs-platform-wallet/src/wallet/asset_lock/restored_record.rs @@ -0,0 +1,89 @@ +//! Output details for an asset-lock funding transaction re-inserted at load. +//! +//! A persister that restores an unresolved asset lock re-inserts its funding +//! transaction as a record without the per-account classification the live +//! checker computes. When that transaction's InstantSend lock or block later +//! arrives, key-wallet finds the record already present: it updates the +//! record's context and its own UTXOs, and re-emits the record as it stands. +//! A store that projects coin flags from the emitted record's +//! `output_details` (the FFI persisters do) learns nothing from an empty +//! list, so the funding transaction's change keeps the flags it had when it +//! was first seen — unconfirmed and not InstantSend-locked — after the +//! engine has confirmed it. + +use dashcore::{Address, OutPoint, Transaction}; +use key_wallet::managed_account::managed_account_trait::ManagedAccountTrait; +use key_wallet::managed_account::managed_core_funds_account::ManagedCoreFundsAccount; +use key_wallet::managed_account::transaction_record::{OutputDetail, OutputRole}; + +/// The outputs of `tx` that `account` holds as coins at restore time, with +/// the role the live checker gives them. +/// +/// Roles come from key-wallet's own read-only match of `tx` against the +/// account, in the order `record_transaction` applies them: a receive +/// address is [`OutputRole::Received`], a change address +/// [`OutputRole::Change`]. +/// +/// Only outputs present in the account's `utxos` at restore time are returned. +/// An output already spent then is left out: a store whose row for it is gone +/// would insert it again as an unspent coin. A coin spent after restore is the +/// re-emit's credit verdict to settle, as for any live record. An output +/// paying another of the wallet's accounts is left out too: it is that +/// account's coin, not this one's. +pub fn held_output_details( + account: &ManagedCoreFundsAccount, + tx: &Transaction, +) -> Vec { + let txid = tx.txid(); + let holds_a_coin = (0..tx.output.len() as u32) + .any(|vout| account.utxos.contains_key(&OutPoint { txid, vout })); + let Some(account_match) = account.check_transaction_for_match(tx, None) else { + if holds_a_coin { + // The coin's address is missing from the restored address pool, + // so nothing will carry its InstantSend lock or block to the store. + tracing::warn!( + %txid, + "restore: account holds a coin of this transaction but does not match it" + ); + } + return Vec::new(); + }; + let receive: Vec<&Address> = account_match + .account_type_match + .involved_receive_addresses() + .iter() + .map(|info| &info.address) + .collect(); + let change: Vec<&Address> = account_match + .account_type_match + .involved_change_addresses() + .iter() + .map(|info| &info.address) + .collect(); + let network = account.network(); + + tx.output + .iter() + .enumerate() + .filter_map(|(index, output)| { + let index = index as u32; + if !account.utxos.contains_key(&OutPoint { txid, vout: index }) { + return None; + } + let address = Address::from_script(&output.script_pubkey, network).ok()?; + let role = if receive.contains(&&address) { + OutputRole::Received + } else if change.contains(&&address) { + OutputRole::Change + } else { + return None; + }; + Some(OutputDetail { + index, + role, + address: Some(address), + value: output.value, + }) + }) + .collect() +} From 30697bf4184320edf730ae05d39b4a761e5baed3 Mon Sep 17 00:00:00 2001 From: romchornyi Date: Mon, 2 Nov 2026 22:08:10 +0200 Subject: [PATCH 2/2] fix(platform-wallet): log a held lock output the restore cannot classify held_output_details left out, without a log, a held output whose address the account's match does not name as receive or change, or whose script has no address. Each now logs a warning with the txid, vout and account. The whole-transaction no-match warning names the held vouts. The held outputs are collected once, and the account is matched only when it holds one. Tests in platform-wallet capture both warnings. Co-Authored-By: Claude Opus 5.5 --- .../rs-platform-wallet-ffi/src/persistence.rs | 6 +- .../src/wallet/asset_lock/restored_record.rs | 167 +++++++++++++++--- 2 files changed, 151 insertions(+), 22 deletions(-) diff --git a/packages/rs-platform-wallet-ffi/src/persistence.rs b/packages/rs-platform-wallet-ffi/src/persistence.rs index 28d1f886bf8..3c511ed772b 100644 --- a/packages/rs-platform-wallet-ffi/src/persistence.rs +++ b/packages/rs-platform-wallet-ffi/src/persistence.rs @@ -5870,8 +5870,10 @@ fn build_wallet_start_state( // re-emit of the record writes the transaction row as a // zero-amount internal transfer. The asset-lock proof path reads // only `context` and `height()`. `output_details` carry the coins - // this account still holds from the transaction - // (`held_output_details`, built from the UTXOs restored above): + // this account still holds from the transaction and can classify + // (`held_output_details`, built from the UTXOs restored above; a held + // coin it cannot classify is left out and logged as `load: account + // holds …`): // when the InstantSend lock or block arrives, key-wallet re-emits // this record and the store takes those coins' new flags from it. // Without them the change would stay unconfirmed in the store. diff --git a/packages/rs-platform-wallet/src/wallet/asset_lock/restored_record.rs b/packages/rs-platform-wallet/src/wallet/asset_lock/restored_record.rs index 3ac07b4e345..4e22dbd2ccb 100644 --- a/packages/rs-platform-wallet/src/wallet/asset_lock/restored_record.rs +++ b/packages/rs-platform-wallet/src/wallet/asset_lock/restored_record.rs @@ -11,7 +11,7 @@ //! was first seen — unconfirmed and not InstantSend-locked — after the //! engine has confirmed it. -use dashcore::{Address, OutPoint, Transaction}; +use dashcore::{Address, OutPoint, Transaction, TxOut}; use key_wallet::managed_account::managed_account_trait::ManagedAccountTrait; use key_wallet::managed_account::managed_core_funds_account::ManagedCoreFundsAccount; use key_wallet::managed_account::transaction_record::{OutputDetail, OutputRole}; @@ -29,23 +29,37 @@ use key_wallet::managed_account::transaction_record::{OutputDetail, OutputRole}; /// would insert it again as an unspent coin. A coin spent after restore is the /// re-emit's credit verdict to settle, as for any live record. An output /// paying another of the wallet's accounts is left out too: it is that -/// account's coin, not this one's. +/// account's coin, not this one's. A held output on an address the match +/// does not name as receive or change has no role to give it and is left +/// out, with a warning naming the output, as is a held output whose script +/// has no address. A transaction the account does not match at all gives no +/// details, with one warning naming the held outputs. pub fn held_output_details( account: &ManagedCoreFundsAccount, tx: &Transaction, ) -> Vec { let txid = tx.txid(); - let holds_a_coin = (0..tx.output.len() as u32) - .any(|vout| account.utxos.contains_key(&OutPoint { txid, vout })); + let held: Vec<(u32, &TxOut)> = tx + .output + .iter() + .enumerate() + .map(|(index, output)| (index as u32, output)) + .filter(|(vout, _)| account.utxos.contains_key(&OutPoint { txid, vout: *vout })) + .collect(); + if held.is_empty() { + return Vec::new(); + } + let account_type = account.managed_account_type().to_account_type(); let Some(account_match) = account.check_transaction_for_match(tx, None) else { - if holds_a_coin { - // The coin's address is missing from the restored address pool, - // so nothing will carry its InstantSend lock or block to the store. - tracing::warn!( - %txid, - "restore: account holds a coin of this transaction but does not match it" - ); - } + // The coins' addresses are missing from the restored address pools, + // so nothing will carry their InstantSend lock or block to the store. + let vouts: Vec = held.iter().map(|(vout, _)| *vout).collect(); + tracing::warn!( + %txid, + ?vouts, + account = ?account_type, + "load: account holds outputs of this transaction but does not match it" + ); return Vec::new(); }; let receive: Vec<&Address> = account_match @@ -62,20 +76,34 @@ pub fn held_output_details( .collect(); let network = account.network(); - tx.output - .iter() - .enumerate() + // A held output left out here keeps the flags the store has for it: + // nothing will carry its InstantSend lock or block there. + held.into_iter() .filter_map(|(index, output)| { - let index = index as u32; - if !account.utxos.contains_key(&OutPoint { txid, vout: index }) { - return None; - } - let address = Address::from_script(&output.script_pubkey, network).ok()?; + let address = match Address::from_script(&output.script_pubkey, network) { + Ok(address) => address, + Err(error) => { + tracing::warn!( + %txid, + vout = index, + account = ?account_type, + %error, + "load: account holds this output but its script has no address" + ); + return None; + } + }; let role = if receive.contains(&&address) { OutputRole::Received } else if change.contains(&&address) { OutputRole::Change } else { + tracing::warn!( + %txid, + vout = index, + account = ?account_type, + "load: account holds this output but the match does not classify it" + ); return None; }; Some(OutputDetail { @@ -87,3 +115,102 @@ pub fn held_output_details( }) .collect() } + +#[cfg(test)] +mod tests { + use super::*; + use crate::test_support::tracing_capture::{RecordedEvents, RecordingGuard}; + use key_wallet::test_utils::TestWalletContext; + use key_wallet::Utxo; + use tracing::Level; + + /// Hold `vout` of `tx` as a coin of `account`, as a store restores it: + /// on the address its script pays, whether or not a pool holds it. + fn hold(account: &mut ManagedCoreFundsAccount, tx: &Transaction, vout: u32) { + let outpoint = OutPoint { + txid: tx.txid(), + vout, + }; + let output = tx.output[vout as usize].clone(); + let address = Address::from_script(&output.script_pubkey, account.network()) + .expect("a test coin pays an address"); + account + .utxos + .insert(outpoint, Utxo::new(outpoint, output, address, 0, false)); + } + + fn warnings(recorder: &RecordedEvents, message: &str) -> usize { + recorder + .entries() + .iter() + .filter(|(level, msg)| *level == Level::WARN && msg == message) + .count() + } + + /// A held output on an address the match does not name is left out with + /// a warning; the change next to it is still named. + #[test] + fn should_warn_and_leave_out_a_held_output_the_match_does_not_classify() { + let mut ctx = TestWalletContext::new_random(); + let foreign = TestWalletContext::new_random().receive_address; + let account = ctx + .managed_wallet + .first_bip44_managed_account_mut() + .expect("bip44 managed account"); + let change = account + .next_change_address(Some(&ctx.xpub), true) + .expect("bip44 change address"); + let mut tx = Transaction::dummy(&change, 0..1, &[2_000]); + tx.output.push(TxOut { + value: 1_000, + script_pubkey: foreign.script_pubkey(), + }); + hold(account, &tx, 0); + hold(account, &tx, 1); + + let recorder = RecordedEvents::default(); + let _guard = RecordingGuard::install(recorder.clone()); + let details = held_output_details(account, &tx); + + assert_eq!(details.len(), 1); + assert_eq!(details[0].index, 0); + assert_eq!(details[0].role, OutputRole::Change); + assert_eq!( + warnings( + &recorder, + "load: account holds this output but the match does not classify it" + ), + 1, + "{:?}", + recorder.entries() + ); + } + + /// A transaction the account does not match names no coin, and a coin of + /// it the account holds is reported. + #[test] + fn should_warn_and_name_nothing_when_the_account_does_not_match_a_held_coins_transaction() { + let mut ctx = TestWalletContext::new_random(); + let foreign = TestWalletContext::new_random().receive_address; + let tx = Transaction::dummy(&foreign, 0..1, &[1_000]); + let account = ctx + .managed_wallet + .first_bip44_managed_account_mut() + .expect("bip44 managed account"); + hold(account, &tx, 0); + + let recorder = RecordedEvents::default(); + let _guard = RecordingGuard::install(recorder.clone()); + + assert!(held_output_details(account, &tx).is_empty()); + assert_eq!( + warnings( + &recorder, + "load: account holds outputs of this transaction but does not match it" + ), + 1, + "{:?}", + recorder.entries() + ); + } +}