Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

transactions method should only return relevant transactions #1779

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 21 additions & 4 deletions crates/wallet/src/wallet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use alloc::{
sync::Arc,
vec::Vec,
};
use chain::Indexer;
notmandatory marked this conversation as resolved.
Show resolved Hide resolved
use core::{cmp::Ordering, fmt, mem, ops::Deref};

use bdk_chain::{
Expand Down Expand Up @@ -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`].
notmandatory marked this conversation as resolved.
Show resolved Hide resolved
pub fn transactions(&self) -> impl Iterator<Item = WalletTx> + '_ {
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
///
Expand Down
53 changes: 51 additions & 2 deletions crates/wallet/tests/wallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,15 @@ 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;
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;
Expand Down Expand Up @@ -4239,3 +4239,52 @@ fn test_tx_builder_is_send_safe() {
let (mut wallet, _txid) = get_funded_wallet_wpkh();
let _box: Box<dyn Send + Sync> = 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,
};
notmandatory marked this conversation as resolved.
Show resolved Hide resolved
test_wallet.apply_update(test_wallet_update).unwrap();

// verify transaction from other wallet was added but is not it relevant transactions list.
notmandatory marked this conversation as resolved.
Show resolved Hide resolved
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);
}
Loading