From 5a7c95c94d1deb951583f383512ecb7d5ca418c0 Mon Sep 17 00:00:00 2001 From: nymius <155548262+nymius@users.noreply.github.com> Date: Fri, 29 May 2026 10:22:46 -0300 Subject: [PATCH] refactor: remove min_confirmations parameter from CanonicalView::balance Co-authored-by: Dmenec --- crates/bitcoind_rpc/tests/test_emitter.rs | 2 +- crates/chain/benches/indexer.rs | 2 +- crates/chain/src/canonical.rs | 25 ++------ crates/chain/tests/test_canonical_view.rs | 63 +++++++++++++++---- crates/chain/tests/test_indexed_tx_graph.rs | 9 ++- crates/chain/tests/test_tx_graph_conflicts.rs | 9 +-- crates/electrum/tests/test_electrum.rs | 2 +- 7 files changed, 65 insertions(+), 47 deletions(-) diff --git a/crates/bitcoind_rpc/tests/test_emitter.rs b/crates/bitcoind_rpc/tests/test_emitter.rs index 2aeede628..0e147d258 100644 --- a/crates/bitcoind_rpc/tests/test_emitter.rs +++ b/crates/bitcoind_rpc/tests/test_emitter.rs @@ -325,7 +325,7 @@ fn get_balance( recv_chain.tip().block_id(), Default::default(), ) - .balance(outpoints, |_, _| true, 0); + .balance(outpoints, |_, _| true); Ok(balance) } diff --git a/crates/chain/benches/indexer.rs b/crates/chain/benches/indexer.rs index 97917ce35..ace78a0ce 100644 --- a/crates/chain/benches/indexer.rs +++ b/crates/chain/benches/indexer.rs @@ -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); } diff --git a/crates/chain/src/canonical.rs b/crates/chain/src/canonical.rs index c2aecb756..2c8555f43 100644 --- a/crates/chain/src/canonical.rs +++ b/crates/chain/src/canonical.rs @@ -428,18 +428,16 @@ impl CanonicalView { /// # 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 + 'v, mut trust_predicate: impl FnMut(&O, &CanonicalTxOut>) -> bool, - min_confirmations: u32, ) -> Balance { let mut immature = Amount::ZERO; let mut trusted_pending = Amount::ZERO; @@ -448,23 +446,8 @@ impl CanonicalView { 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; diff --git a/crates/chain/tests/test_canonical_view.rs b/crates/chain/tests/test_canonical_view.rs index 2cbe02103..94f51ac0d 100644 --- a/crates/chain/tests/test_canonical_view.rs +++ b/crates/chain/tests/test_canonical_view.rs @@ -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); @@ -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) @@ -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, @@ -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, diff --git a/crates/chain/tests/test_indexed_tx_graph.rs b/crates/chain/tests/test_indexed_tx_graph.rs index 96cafcb8e..d3d59a33f 100644 --- a/crates/chain/tests/test_indexed_tx_graph.rs +++ b/crates/chain/tests/test_indexed_tx_graph.rs @@ -480,11 +480,10 @@ fn test_list_owned_txouts() { .filter_unspent_outpoints(graph.index.outpoints().iter().cloned()) .collect::>(); - 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() diff --git a/crates/chain/tests/test_tx_graph_conflicts.rs b/crates/chain/tests/test_tx_graph_conflicts.rs index f1f166984..9b700b861 100644 --- a/crates/chain/tests/test_tx_graph_conflicts.rs +++ b/crates/chain/tests/test_tx_graph_conflicts.rs @@ -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", diff --git a/crates/electrum/tests/test_electrum.rs b/crates/electrum/tests/test_electrum.rs index c569b065d..15770f40c 100644 --- a/crates/electrum/tests/test_electrum.rs +++ b/crates/electrum/tests/test_electrum.rs @@ -62,7 +62,7 @@ fn get_balance( recv_chain.tip().block_id(), Default::default(), ) - .balance(outpoints, |_, _| true, 0); + .balance(outpoints, |_, _| true); Ok(balance) }