From 3e1fd2b0a2b39668e5ab29c6968e85949623e447 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Tue, 17 Dec 2024 13:46:58 +1100 Subject: [PATCH 1/2] fix(wallet): `transactions` method should only return relevant txs Also update documentation. --- crates/wallet/src/wallet/mod.rs | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/crates/wallet/src/wallet/mod.rs b/crates/wallet/src/wallet/mod.rs index 3af424d37..d64f4b8e2 100644 --- a/crates/wallet/src/wallet/mod.rs +++ b/crates/wallet/src/wallet/mod.rs @@ -19,6 +19,7 @@ use alloc::{ sync::Arc, vec::Vec, }; +use chain::Indexer; use core::{cmp::Ordering, fmt, mem, ops::Deref}; use bdk_chain::{ @@ -1062,14 +1063,30 @@ impl Wallet { .find(|tx| tx.tx_node.txid == txid) } - /// Iterate over the transactions in the wallet. + /// Iterate over relevant and canonical transactions in the wallet. + /// + /// A transaction is relevant when it spends from or spends to at least one tracked output. A + /// transaction is canonical when it is confirmed in the best chain, or does not conflict + /// with any transaction confirmed in the best chain. + /// + /// To iterate over all transactions, including those that are irrelevant and not canonical, use + /// [`TxGraph::full_txs`]. + /// + /// To iterate over all canonical transactions, including those that are irrelevant, use + /// [`TxGraph::list_canonical_txs`]. pub fn transactions(&self) -> impl Iterator + '_ { - self.indexed_graph - .graph() + let tx_graph = self.indexed_graph.graph(); + let tx_index = &self.indexed_graph.index; + tx_graph .list_canonical_txs(&self.chain, self.chain.tip().block_id()) + .filter(|c_tx| tx_index.is_tx_relevant(&c_tx.tx_node.tx)) } - /// Array of transactions in the wallet sorted with a comparator function. + /// Array of relevant and canonical transactions in the wallet sorted with a comparator + /// function. + /// + /// This is a helper method equivalent to collecting the result of [`Wallet::transactions`] + /// into a [`Vec`] and then sorting it. /// /// # Example /// From 75fae3ef56be838787306fc81862eb5ae500f635 Mon Sep 17 00:00:00 2001 From: Steve Myers Date: Tue, 17 Dec 2024 16:10:54 -0600 Subject: [PATCH 2/2] test(wallet): verify Wallet::transactions method only returns relevant txs --- crates/wallet/tests/wallet.rs | 53 +++++++++++++++++++++++++++++++++-- 1 file changed, 51 insertions(+), 2 deletions(-) diff --git a/crates/wallet/tests/wallet.rs b/crates/wallet/tests/wallet.rs index afb346905..7ca60022e 100644 --- a/crates/wallet/tests/wallet.rs +++ b/crates/wallet/tests/wallet.rs @@ -4,7 +4,7 @@ use std::sync::Arc; use anyhow::Context; use assert_matches::assert_matches; -use bdk_chain::{BlockId, ChainPosition, ConfirmationBlockTime}; +use bdk_chain::{BlockId, ChainPosition, ConfirmationBlockTime, TxUpdate}; use bdk_wallet::coin_selection::{self, LargestFirstCoinSelection}; use bdk_wallet::descriptor::{calc_checksum, DescriptorError, IntoWalletDescriptor}; use bdk_wallet::error::CreateTxError; @@ -12,7 +12,7 @@ use bdk_wallet::psbt::PsbtUtils; use bdk_wallet::signer::{SignOptions, SignerError}; use bdk_wallet::test_utils::*; use bdk_wallet::tx_builder::AddForeignUtxoError; -use bdk_wallet::{AddressInfo, Balance, ChangeSet, Wallet, WalletPersister, WalletTx}; +use bdk_wallet::{AddressInfo, Balance, ChangeSet, Update, Wallet, WalletPersister, WalletTx}; use bdk_wallet::{KeychainKind, LoadError, LoadMismatch, LoadWithPersistError}; use bitcoin::constants::{ChainHash, COINBASE_MATURITY}; use bitcoin::hashes::Hash; @@ -4239,3 +4239,52 @@ fn test_tx_builder_is_send_safe() { let (mut wallet, _txid) = get_funded_wallet_wpkh(); let _box: Box = Box::new(wallet.build_tx()); } + +#[test] +fn test_wallet_transactions_relevant() { + let (mut test_wallet, _txid) = get_funded_wallet_wpkh(); + let relevant_tx_count_before = test_wallet.transactions().count(); + let full_tx_count_before = test_wallet.tx_graph().full_txs().count(); + let chain_tip = test_wallet.local_chain().tip().block_id(); + let canonical_tx_count_before = test_wallet + .tx_graph() + .list_canonical_txs(test_wallet.local_chain(), chain_tip) + .count(); + + // add not relevant transaction to test wallet + let (other_external_desc, other_internal_desc) = get_test_tr_single_sig_xprv_and_change_desc(); + let (other_wallet, other_txid) = get_funded_wallet(other_internal_desc, other_external_desc); + let other_tx_node = other_wallet.get_tx(other_txid).unwrap().tx_node; + let other_tx_confirmationblocktime = other_tx_node.anchors.iter().last().unwrap(); + let other_tx_update = TxUpdate { + txs: vec![other_tx_node.tx], + txouts: Default::default(), + anchors: [(*other_tx_confirmationblocktime, other_txid)].into(), + seen_ats: [(other_txid, other_tx_confirmationblocktime.confirmation_time)].into(), + }; + let test_wallet_update = Update { + last_active_indices: Default::default(), + tx_update: other_tx_update, + chain: None, + }; + test_wallet.apply_update(test_wallet_update).unwrap(); + + // verify transaction from other wallet was added but is not it relevant transactions list. + let relevant_tx_count_after = test_wallet.transactions().count(); + let full_tx_count_after = test_wallet.tx_graph().full_txs().count(); + let canonical_tx_count_after = test_wallet + .tx_graph() + .list_canonical_txs(test_wallet.local_chain(), chain_tip) + .count(); + + assert_eq!(relevant_tx_count_before, relevant_tx_count_after); + assert!(!test_wallet + .transactions() + .any(|wallet_tx| wallet_tx.tx_node.txid == other_txid)); + assert!(test_wallet + .tx_graph() + .list_canonical_txs(test_wallet.local_chain(), chain_tip) + .any(|wallet_tx| wallet_tx.tx_node.txid == other_txid)); + assert!(full_tx_count_before < full_tx_count_after); + assert!(canonical_tx_count_before < canonical_tx_count_after); +}