Skip to content

refactor(chain)!: remove min_confirmations parameter from CanonicalView::balance #2221

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.

Sign up for GitHub

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

Open
Dmenec wants to merge 1 commit into
base: master
Choose a base branch
from
Open

refactor(chain)!: remove min_confirmations parameter from CanonicalView::balance #2221

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion crates/bitcoind_rpc/tests/test_emitter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ fn get_balance(
recv_chain.tip().block_id(),
Default::default(),
)
.balance(outpoints, |_, _| true, 0);
.balance(outpoints, |_, _| true);
Ok(balance)
}

Expand Down
2 changes: 1 addition & 1 deletion crates/chain/benches/indexer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ fn do_bench(indexed_tx_graph: &KeychainTxGraph, chain: &LocalChain) {
let op = graph.index.outpoints().clone();
let bal = chain
.canonical_view(graph.graph(), chain.tip().block_id(), Default::default())
.balance(op, |_, _| false, 1);
.balance(op, |_, _| false);
assert_eq!(bal.total(), AMOUNT * TX_CT as u64);
}

Expand Down
25 changes: 4 additions & 21 deletions crates/chain/src/canonical.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -428,18 +428,16 @@ impl<A: Anchor> CanonicalView<A> {
/// # let chain_tip = chain.tip().block_id();
/// # let view = chain.canonical_view(&tx_graph, chain_tip, CanonicalParams::default());
/// # let indexer = KeychainTxOutIndex::<&str>::default();
/// // Calculate balance with 6 confirmations, trusting all outputs
/// // Calculate balance, trusting all outputs
/// let balance = view.balance(
/// indexer.outpoints().into_iter().map(|(k, op)| (k.clone(), *op)),
/// |_keychain, _script| true, // Trust all outputs
/// 6, // Require 6 confirmations
/// |_keychain, _txout| true,
/// );
/// ```
pub fn balance<'v, O: Clone + 'v>(
&'v self,
outpoints: impl IntoIterator<Item = (O, OutPoint)> + 'v,
mut trust_predicate: impl FnMut(&O, &CanonicalTxOut<ChainPosition<A>>) -> bool,
min_confirmations: u32,
) -> Balance {
let mut immature = Amount::ZERO;
let mut trusted_pending = Amount::ZERO;
Expand All @@ -448,23 +446,8 @@ impl<A: Anchor> CanonicalView<A> {

for (spk_i, txout) in self.filter_unspent_outpoints(outpoints) {
match &txout.pos {
ChainPosition::Confirmed { anchor, .. } => {
let confirmation_height = anchor.confirmation_height_upper_bound();
let confirmations = self
.tip
.height
.saturating_sub(confirmation_height)
.saturating_add(1);
let min_confirmations = min_confirmations.max(1); // 0 and 1 behave identically

if confirmations < min_confirmations {
// Not enough confirmations, treat as trusted/untrusted pending
if trust_predicate(&spk_i, &txout) {
trusted_pending += txout.txout.value;
} else {
untrusted_pending += txout.txout.value;
}
} else if txout.is_confirmed_and_spendable(self.tip.height) {
ChainPosition::Confirmed { .. } => {
if txout.is_confirmed_and_spendable(self.tip.height) {
confirmed += txout.txout.value;
} else if !txout.is_mature(self.tip.height) {
immature += txout.txout.value;
Expand Down
63 changes: 51 additions & 12 deletions crates/chain/tests/test_canonical_view.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,42 +53,67 @@ fn test_min_confirmations_parameter() {
};
let _ = tx_graph.insert_anchor(txid, anchor_height_5);

let min_confirmations = 1;
let min_conf_height = chain.tip().height() - min_confirmations + 1;
let min_conf_tip = chain.get(min_conf_height).expect("block should exist");

let canonical_view =
chain.canonical_view(&tx_graph, chain.tip().block_id(), Default::default());
chain.canonical_view(&tx_graph, min_conf_tip.block_id(), Default::default());

// Test min_confirmations = 1: Should be confirmed (has 6 confirmations)
// Test min_confirmations = 1: Should be confirmed (has 5 confirmations)
let balance_1_conf = canonical_view.balance(
[((), outpoint)],
|_, _| true, // trust all
1,
);

assert_eq!(balance_1_conf.confirmed, Amount::from_sat(50_000));
assert_eq!(balance_1_conf.trusted_pending, Amount::ZERO);

// Test min_confirmations = 6: Should be confirmed (has exactly 6 confirmations)
let min_confirmations = 6;
let min_conf_height = chain.tip().height() - min_confirmations + 1;
let min_conf_tip = chain.get(min_conf_height).expect("block should exist");

let canonical_view =
chain.canonical_view(&tx_graph, min_conf_tip.block_id(), Default::default());

// Test min_confirmations = 6: Should be confirmed (has exactly 1 confirmations)
let balance_6_conf = canonical_view.balance(
[((), outpoint)],
|_, _| true, // trust all
6,
);
assert_eq!(balance_6_conf.confirmed, Amount::from_sat(50_000));
assert_eq!(balance_6_conf.trusted_pending, Amount::ZERO);

let min_confirmations = 7;
let min_conf_height = chain.tip().height() - min_confirmations + 1;
let min_conf_tip = chain.get(min_conf_height).expect("block should exist");

let canonical_view =
chain.canonical_view(&tx_graph, min_conf_tip.block_id(), Default::default());

// Test min_confirmations = 7: Should be trusted pending (only has 6 confirmations)
let balance_7_conf = canonical_view.balance(
[((), outpoint)],
|_, _| true, // trust all
7,
);
assert_eq!(balance_7_conf.confirmed, Amount::ZERO);
assert_eq!(balance_7_conf.trusted_pending, Amount::from_sat(50_000));

let min_confirmations = 0;
let min_conf_tip = if min_confirmations == 0 {
chain.tip()
} else {
let min_conf_height = chain.tip().height() - min_confirmations + 1;
chain.get(min_conf_height).expect("block should exist")
};

let canonical_view =
chain.canonical_view(&tx_graph, min_conf_tip.block_id(), Default::default());

// Test min_confirmations = 0: Should behave same as 1 (confirmed)
let balance_0_conf = canonical_view.balance(
[((), outpoint)],
|_, _| true, // trust all
0,
);
assert_eq!(balance_0_conf.confirmed, Amount::from_sat(50_000));
assert_eq!(balance_0_conf.trusted_pending, Amount::ZERO);
Expand Down Expand Up @@ -141,14 +166,17 @@ fn test_min_confirmations_with_untrusted_tx() {
};
let _ = tx_graph.insert_anchor(txid, anchor);

let min_confirmations = 5;
let min_conf_height = chain.tip().height() - min_confirmations + 1;
let min_conf_tip = chain.get(min_conf_height).expect("block should exist");

let canonical_view =
chain.canonical_view(&tx_graph, chain.tip().block_id(), Default::default());
chain.canonical_view(&tx_graph, min_conf_tip.block_id(), Default::default());

// Test with min_confirmations = 5 and untrusted predicate
let balance = canonical_view.balance(
[((), outpoint)],
|_, _| false, // don't trust
5,
);

// Should be untrusted pending (not enough confirmations and not trusted)
Expand Down Expand Up @@ -259,14 +287,18 @@ fn test_min_confirmations_multiple_transactions() {
);
outpoints.push(((), outpoint2));

let min_confirmations = 5;
let min_conf_height = chain.tip().height() - min_confirmations + 1;
let min_conf_tip = chain.get(min_conf_height).expect("block should exist");

let canonical_view =
chain.canonical_view(&tx_graph, chain.tip().block_id(), Default::default());
chain.canonical_view(&tx_graph, min_conf_tip.block_id(), Default::default());

// Test with min_confirmations = 5
// tx0: 11 confirmations -> confirmed
// tx1: 6 confirmations -> confirmed
// tx2: 3 confirmations -> trusted pending
let balance = canonical_view.balance(outpoints.clone(), |_, _| true, 5);
let balance = canonical_view.balance(outpoints.clone(), |_, _| true);

assert_eq!(
balance.confirmed,
Expand All @@ -278,11 +310,18 @@ fn test_min_confirmations_multiple_transactions() {
);
assert_eq!(balance.untrusted_pending, Amount::ZERO);

let min_confirmations = 10;
let min_conf_height = chain.tip().height() - min_confirmations + 1;
let min_conf_tip = chain.get(min_conf_height).expect("block should exist");

let canonical_view =
chain.canonical_view(&tx_graph, min_conf_tip.block_id(), Default::default());

// Test with min_confirmations = 10
// tx0: 11 confirmations -> confirmed
// tx1: 6 confirmations -> trusted pending
// tx2: 3 confirmations -> trusted pending
let balance_high = canonical_view.balance(outpoints, |_, _| true, 10);
let balance_high = canonical_view.balance(outpoints, |_, _| true);

assert_eq!(
balance_high.confirmed,
Expand Down
9 changes: 4 additions & 5 deletions crates/chain/tests/test_indexed_tx_graph.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -480,11 +480,10 @@ fn test_list_owned_txouts() {
.filter_unspent_outpoints(graph.index.outpoints().iter().cloned())
.collect::<Vec<_>>();

let balance = canonical_view.balance(
graph.index.outpoints().iter().cloned(),
|_, txout| trusted_spks.contains(&txout.txout.script_pubkey),
0,
);
let balance = canonical_view
.balance(graph.index.outpoints().iter().cloned(), |_, txout| {
trusted_spks.contains(&txout.txout.script_pubkey)
});

let confirmed_txouts_txid = txouts
.iter()
Expand Down
9 changes: 3 additions & 6 deletions crates/chain/tests/test_tx_graph_conflicts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1027,15 +1027,12 @@ fn test_tx_conflict_handling() {
scenario.name
);

let balance = canonical_view.balance(
env.indexer.outpoints().iter().cloned(),
|_, txout| {
let balance =
canonical_view.balance(env.indexer.outpoints().iter().cloned(), |_, txout| {
env.indexer
.index_of_spk(txout.txout.script_pubkey.as_script())
.is_some()
},
0,
);
});
assert_eq!(
balance, scenario.exp_balance,
"\n[{}] 'balance' failed",
Expand Down
2 changes: 1 addition & 1 deletion crates/electrum/tests/test_electrum.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ fn get_balance(
recv_chain.tip().block_id(),
Default::default(),
)
.balance(outpoints, |_, _| true, 0);
.balance(outpoints, |_, _| true);
Ok(balance)
}

Expand Down