From 8c34407f0b759dbca0851e120dcd67d057bfcca1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Tue, 19 Nov 2024 17:26:50 +1100 Subject: [PATCH] feat(chain)!: rm `get_chain_position` and associated methods --- crates/chain/src/tx_graph.rs | 211 -------------------- crates/chain/tests/test_indexed_tx_graph.rs | 21 +- crates/chain/tests/test_tx_graph.rs | 156 ++++++++------- crates/wallet/src/wallet/mod.rs | 91 +++++---- 4 files changed, 152 insertions(+), 327 deletions(-) diff --git a/crates/chain/src/tx_graph.rs b/crates/chain/src/tx_graph.rs index 24480110d..04149cf74 100644 --- a/crates/chain/src/tx_graph.rs +++ b/crates/chain/src/tx_graph.rs @@ -739,217 +739,6 @@ impl TxGraph { } impl TxGraph { - /// Get the position of the transaction in `chain` with tip `chain_tip`. - /// - /// Chain data is fetched from `chain`, a [`ChainOracle`] implementation. - /// - /// This method returns `Ok(None)` if the transaction is not found in the chain, and no longer - /// belongs in the mempool. The following factors are used to approximate whether an - /// unconfirmed transaction exists in the mempool (not evicted): - /// - /// 1. Unconfirmed transactions that conflict with confirmed transactions are evicted. - /// 2. Unconfirmed transactions that spend from transactions that are evicted, are also - /// evicted. - /// 3. Given two conflicting unconfirmed transactions, the transaction with the lower - /// `last_seen_unconfirmed` parameter is evicted. A transaction's `last_seen_unconfirmed` - /// parameter is the max of all it's descendants' `last_seen_unconfirmed` parameters. If the - /// final `last_seen_unconfirmed`s are the same, the transaction with the lower `txid` (by - /// lexicographical order) is evicted. - /// - /// # Error - /// - /// An error will occur if the [`ChainOracle`] implementation (`chain`) fails. If the - /// [`ChainOracle`] is infallible, [`get_chain_position`] can be used instead. - /// - /// [`get_chain_position`]: Self::get_chain_position - pub fn try_get_chain_position( - &self, - chain: &C, - chain_tip: BlockId, - txid: Txid, - ) -> Result>, C::Error> { - let tx_node = match self.txs.get(&txid) { - Some(v) => v, - None => return Ok(None), - }; - - for anchor in self.anchors.get(&txid).unwrap_or(&self.empty_anchors) { - match chain.is_block_in_chain(anchor.anchor_block(), chain_tip)? { - Some(true) => return Ok(Some(ChainPosition::Confirmed(anchor))), - _ => continue, - } - } - - // If no anchors are in best chain and we don't have a last_seen, we can return - // early because by definition the tx doesn't have a chain position. - let last_seen = match self.last_seen.get(&txid) { - Some(t) => *t, - None => return Ok(None), - }; - - // The tx is not anchored to a block in the best chain, which means that it - // might be in mempool, or it might have been dropped already. - // Let's check conflicts to find out! - let tx = match tx_node { - TxNodeInternal::Whole(tx) => { - // A coinbase tx that is not anchored in the best chain cannot be unconfirmed and - // should always be filtered out. - if tx.is_coinbase() { - return Ok(None); - } - tx.clone() - } - TxNodeInternal::Partial(_) => { - // Partial transactions (outputs only) cannot have conflicts. - return Ok(None); - } - }; - - // We want to retrieve all the transactions that conflict with us, plus all the - // transactions that conflict with our unconfirmed ancestors, since they conflict with us - // as well. - // We only traverse unconfirmed ancestors since conflicts of confirmed transactions - // cannot be in the best chain. - - // First of all, we retrieve all our ancestors. Since we're using `new_include_root`, the - // resulting array will also include `tx` - let unconfirmed_ancestor_txs = - TxAncestors::new_include_root(self, tx.clone(), |_, ancestor_tx: Arc| { - let tx_node = self.get_tx_node(ancestor_tx.as_ref().compute_txid())?; - // We're filtering the ancestors to keep only the unconfirmed ones (= no anchors in - // the best chain) - for block in tx_node.anchors { - match chain.is_block_in_chain(block.anchor_block(), chain_tip) { - Ok(Some(true)) => return None, - Err(e) => return Some(Err(e)), - _ => continue, - } - } - Some(Ok(tx_node)) - }) - .collect::, C::Error>>()?; - - // We determine our tx's last seen, which is the max between our last seen, - // and our unconf descendants' last seen. - let unconfirmed_descendants_txs = TxDescendants::new_include_root( - self, - tx.as_ref().compute_txid(), - |_, descendant_txid: Txid| { - let tx_node = self.get_tx_node(descendant_txid)?; - // We're filtering the ancestors to keep only the unconfirmed ones (= no anchors in - // the best chain) - for block in tx_node.anchors { - match chain.is_block_in_chain(block.anchor_block(), chain_tip) { - Ok(Some(true)) => return None, - Err(e) => return Some(Err(e)), - _ => continue, - } - } - Some(Ok(tx_node)) - }, - ) - .collect::, C::Error>>()?; - - let tx_last_seen = unconfirmed_descendants_txs - .iter() - .max_by_key(|tx| tx.last_seen_unconfirmed) - .map(|tx| tx.last_seen_unconfirmed) - .expect("descendants always includes at least one transaction (the root tx"); - - // Now we traverse our ancestors and consider all their conflicts - for tx_node in unconfirmed_ancestor_txs { - // We retrieve all the transactions conflicting with this specific ancestor - let conflicting_txs = - self.walk_conflicts(tx_node.tx.as_ref(), |_, txid| self.get_tx_node(txid)); - - // If a conflicting tx is in the best chain, or has `last_seen` higher than this ancestor, then - // this tx cannot exist in the best chain - for conflicting_tx in conflicting_txs { - for block in conflicting_tx.anchors { - if chain.is_block_in_chain(block.anchor_block(), chain_tip)? == Some(true) { - return Ok(None); - } - } - if conflicting_tx.last_seen_unconfirmed > tx_last_seen { - return Ok(None); - } - if conflicting_tx.last_seen_unconfirmed == Some(last_seen) - && conflicting_tx.as_ref().compute_txid() > tx.as_ref().compute_txid() - { - // Conflicting tx has priority if txid of conflicting tx > txid of original tx - return Ok(None); - } - } - } - - Ok(Some(ChainPosition::Unconfirmed(last_seen))) - } - - /// Get the position of the transaction in `chain` with tip `chain_tip`. - /// - /// This is the infallible version of [`try_get_chain_position`]. - /// - /// [`try_get_chain_position`]: Self::try_get_chain_position - pub fn get_chain_position>( - &self, - chain: &C, - chain_tip: BlockId, - txid: Txid, - ) -> Option> { - self.try_get_chain_position(chain, chain_tip, txid) - .expect("error is infallible") - } - - /// Get the txid of the spending transaction and where the spending transaction is observed in - /// the `chain` of `chain_tip`. - /// - /// If no in-chain transaction spends `outpoint`, `None` will be returned. - /// - /// # Error - /// - /// An error will occur only if the [`ChainOracle`] implementation (`chain`) fails. - /// - /// If the [`ChainOracle`] is infallible, [`get_chain_spend`] can be used instead. - /// - /// [`get_chain_spend`]: Self::get_chain_spend - pub fn try_get_chain_spend( - &self, - chain: &C, - chain_tip: BlockId, - outpoint: OutPoint, - ) -> Result, Txid)>, C::Error> { - if self - .try_get_chain_position(chain, chain_tip, outpoint.txid)? - .is_none() - { - return Ok(None); - } - if let Some(spends) = self.spends.get(&outpoint) { - for &txid in spends { - if let Some(observed_at) = self.try_get_chain_position(chain, chain_tip, txid)? { - return Ok(Some((observed_at, txid))); - } - } - } - Ok(None) - } - - /// Get the txid of the spending transaction and where the spending transaction is observed in - /// the `chain` of `chain_tip`. - /// - /// This is the infallible version of [`try_get_chain_spend`] - /// - /// [`try_get_chain_spend`]: Self::try_get_chain_spend - pub fn get_chain_spend>( - &self, - chain: &C, - static_block: BlockId, - outpoint: OutPoint, - ) -> Option<(ChainPosition<&A>, Txid)> { - self.try_get_chain_spend(chain, static_block, outpoint) - .expect("error is infallible") - } - /// List graph transactions that are in `chain` with `chain_tip`. /// /// Each transaction is represented as a [`CanonicalTx`] that contains where the transaction is diff --git a/crates/chain/tests/test_indexed_tx_graph.rs b/crates/chain/tests/test_indexed_tx_graph.rs index 3236d2b2a..575eef015 100644 --- a/crates/chain/tests/test_indexed_tx_graph.rs +++ b/crates/chain/tests/test_indexed_tx_graph.rs @@ -587,14 +587,17 @@ fn test_get_chain_position() { } // check chain position - let res = graph + let chain_pos = graph .graph() - .get_chain_position(chain, chain.tip().block_id(), txid); - assert_eq!( - res.map(ChainPosition::cloned), - exp_pos, - "failed test case: {name}" - ); + .list_canonical_txs(chain, chain.tip().block_id()) + .find_map(|canon_tx| { + if canon_tx.tx_node.txid == txid { + Some(canon_tx.chain_position) + } else { + None + } + }); + assert_eq!(chain_pos, exp_pos, "failed test case: {name}"); } [ @@ -651,7 +654,7 @@ fn test_get_chain_position() { exp_pos: Some(ChainPosition::Unconfirmed(2)), }, TestCase { - name: "tx unknown anchor - no chain pos", + name: "tx unknown anchor - unconfirmed", tx: Transaction { output: vec![TxOut { value: Amount::ONE_BTC, @@ -661,7 +664,7 @@ fn test_get_chain_position() { }, anchor: Some(block_id!(2, "B'")), last_seen: None, - exp_pos: None, + exp_pos: Some(ChainPosition::UnconfirmedAndNotSeen), }, ] .into_iter() diff --git a/crates/chain/tests/test_tx_graph.rs b/crates/chain/tests/test_tx_graph.rs index 7f94713e9..1c9f917d0 100644 --- a/crates/chain/tests/test_tx_graph.rs +++ b/crates/chain/tests/test_tx_graph.rs @@ -877,52 +877,66 @@ fn test_chain_spends() { ); } - // Assert that confirmed spends are returned correctly. - assert_eq!( - graph.get_chain_spend( - &local_chain, - tip.block_id(), - OutPoint::new(tx_0.compute_txid(), 0) - ), - Some(( - ChainPosition::Confirmed(&ConfirmationBlockTime { - block_id: BlockId { - hash: tip.get(98).unwrap().hash(), - height: 98, - }, - confirmation_time: 100 - }), - tx_1.compute_txid(), - )), - ); + let build_canonical_spends = + |chain: &LocalChain, tx_graph: &TxGraph| -> HashMap { + tx_graph + .filter_chain_txouts( + chain, + tip.block_id(), + tx_graph.all_txouts().map(|(op, _)| ((), op)), + ) + .filter_map(|(_, full_txo)| Some((full_txo.outpoint, full_txo.spent_by?))) + .collect() + }; + let build_canonical_positions = |chain: &LocalChain, + tx_graph: &TxGraph| + -> HashMap> { + tx_graph + .list_canonical_txs(chain, tip.block_id()) + .map(|canon_tx| (canon_tx.tx_node.txid, canon_tx.chain_position)) + .collect() + }; - // Check if chain position is returned correctly. - assert_eq!( - graph.get_chain_position(&local_chain, tip.block_id(), tx_0.compute_txid()), - // Some(ObservedAs::Confirmed(&local_chain.get_block(95).expect("block expected"))), - Some(ChainPosition::Confirmed(&ConfirmationBlockTime { - block_id: BlockId { - hash: tip.get(95).unwrap().hash(), - height: 95, - }, - confirmation_time: 100 - })) - ); + { + let canonical_spends = build_canonical_spends(&local_chain, &graph); + let canonical_positions = build_canonical_positions(&local_chain, &graph); + + // Assert that confirmed spends are returned correctly. + assert_eq!( + canonical_spends + .get(&OutPoint::new(tx_0.compute_txid(), 0)) + .cloned(), + Some(( + ChainPosition::Confirmed(ConfirmationBlockTime { + block_id: tip.get(98).unwrap().block_id(), + confirmation_time: 100 + }), + tx_1.compute_txid(), + )), + ); + // Check if chain position is returned correctly. + assert_eq!( + canonical_positions.get(&tx_0.compute_txid()).cloned(), + Some(ChainPosition::Confirmed(ConfirmationBlockTime { + block_id: tip.get(95).unwrap().block_id(), + confirmation_time: 100 + })) + ); + } // Mark the unconfirmed as seen and check correct ObservedAs status is returned. let _ = graph.insert_seen_at(tx_2.compute_txid(), 1234567); + { + let canonical_spends = build_canonical_spends(&local_chain, &graph); - // Check chain spend returned correctly. - assert_eq!( - graph - .get_chain_spend( - &local_chain, - tip.block_id(), - OutPoint::new(tx_0.compute_txid(), 1) - ) - .unwrap(), - (ChainPosition::Unconfirmed(1234567), tx_2.compute_txid()) - ); + // Check chain spend returned correctly. + assert_eq!( + canonical_spends + .get(&OutPoint::new(tx_0.compute_txid(), 1)) + .cloned(), + Some((ChainPosition::Unconfirmed(1234567), tx_2.compute_txid())) + ); + } // A conflicting transaction that conflicts with tx_1. let tx_1_conflict = Transaction { @@ -933,11 +947,14 @@ fn test_chain_spends() { ..new_tx(0) }; let _ = graph.insert_tx(tx_1_conflict.clone()); + { + let canonical_positions = build_canonical_positions(&local_chain, &graph); - // Because this tx conflicts with an already confirmed transaction, chain position should return none. - assert!(graph - .get_chain_position(&local_chain, tip.block_id(), tx_1_conflict.compute_txid()) - .is_none()); + // Because this tx conflicts with an already confirmed transaction, chain position should return none. + assert!(canonical_positions + .get(&tx_1_conflict.compute_txid()) + .is_none()); + } // Another conflicting tx that conflicts with tx_2. let tx_2_conflict = Transaction { @@ -947,38 +964,35 @@ fn test_chain_spends() { }], ..new_tx(0) }; - // Insert in graph and mark it as seen. let _ = graph.insert_tx(tx_2_conflict.clone()); let _ = graph.insert_seen_at(tx_2_conflict.compute_txid(), 1234568); + { + let canonical_spends = build_canonical_spends(&local_chain, &graph); + let canonical_positions = build_canonical_positions(&local_chain, &graph); - // This should return a valid observation with correct last seen. - assert_eq!( - graph - .get_chain_position(&local_chain, tip.block_id(), tx_2_conflict.compute_txid()) - .expect("position expected"), - ChainPosition::Unconfirmed(1234568) - ); + // This should return a valid observation with correct last seen. + assert_eq!( + canonical_positions + .get(&tx_2_conflict.compute_txid()) + .cloned(), + Some(ChainPosition::Unconfirmed(1234568)) + ); - // Chain_spend now catches the new transaction as the spend. - assert_eq!( - graph - .get_chain_spend( - &local_chain, - tip.block_id(), - OutPoint::new(tx_0.compute_txid(), 1) - ) - .expect("expect observation"), - ( - ChainPosition::Unconfirmed(1234568), - tx_2_conflict.compute_txid() - ) - ); + // Chain_spend now catches the new transaction as the spend. + assert_eq!( + canonical_spends + .get(&OutPoint::new(tx_0.compute_txid(), 1)) + .cloned(), + Some(( + ChainPosition::Unconfirmed(1234568), + tx_2_conflict.compute_txid() + )) + ); - // Chain position of the `tx_2` is now none, as it is older than `tx_2_conflict` - assert!(graph - .get_chain_position(&local_chain, tip.block_id(), tx_2.compute_txid()) - .is_none()); + // Chain position of the `tx_2` is now none, as it is older than `tx_2_conflict` + assert!(canonical_positions.get(&tx_2.compute_txid()).is_none()); + } } /// Ensure that `last_seen` values only increase during [`Merge::merge`]. diff --git a/crates/wallet/src/wallet/mod.rs b/crates/wallet/src/wallet/mod.rs index 5cc469f28..e2cf81a80 100644 --- a/crates/wallet/src/wallet/mod.rs +++ b/crates/wallet/src/wallet/mod.rs @@ -20,6 +20,7 @@ use alloc::{ vec::Vec, }; use core::{cmp::Ordering, fmt, mem, ops::Deref}; +use std::collections::HashSet; use bdk_chain::{ indexed_tx_graph, @@ -1060,13 +1061,9 @@ impl Wallet { /// [`Anchor`]: bdk_chain::Anchor pub fn get_tx(&self, txid: Txid) -> Option { let graph = self.indexed_graph.graph(); - - Some(WalletTx { - chain_position: graph - .get_chain_position(&self.chain, self.chain.tip().block_id(), txid)? - .cloned(), - tx_node: graph.get_tx_node(txid)?, - }) + graph + .list_canonical_txs(&self.chain, self.chain.tip().block_id()) + .find(|tx| tx.tx_node.txid == txid) } /// Iterate over the transactions in the wallet. @@ -1587,6 +1584,10 @@ impl Wallet { let graph = self.indexed_graph.graph(); let txout_index = &self.indexed_graph.index; let chain_tip = self.chain.tip().block_id(); + let chain_positions = graph + .list_canonical_txs(&self.chain, chain_tip) + .map(|canon_tx| (canon_tx.tx_node.txid, canon_tx.chain_position)) + .collect::>(); let mut tx = graph .get_tx(txid) @@ -1594,10 +1595,11 @@ impl Wallet { .as_ref() .clone(); - let pos = graph - .get_chain_position(&self.chain, chain_tip, txid) - .ok_or(BuildFeeBumpError::TransactionNotFound(txid))?; - if let ChainPosition::Confirmed(_) = pos { + if chain_positions + .get(&txid) + .ok_or(BuildFeeBumpError::TransactionNotFound(txid))? + .is_confirmed() + { return Err(BuildFeeBumpError::TransactionConfirmed(txid)); } @@ -1628,10 +1630,10 @@ impl Wallet { .ok_or(BuildFeeBumpError::UnknownUtxo(txin.previous_output))?; let txout = &prev_tx.output[txin.previous_output.vout as usize]; - let chain_position = graph - .get_chain_position(&self.chain, chain_tip, txin.previous_output.txid) - .ok_or(BuildFeeBumpError::UnknownUtxo(txin.previous_output))? - .cloned(); + let chain_position = chain_positions + .get(&txin.previous_output.txid) + .cloned() + .ok_or(BuildFeeBumpError::UnknownUtxo(txin.previous_output))?; let weighted_utxo = match txout_index.index_of_spk(txout.script_pubkey.clone()) { Some(&(keychain, derivation_index)) => { @@ -1830,9 +1832,31 @@ impl Wallet { psbt: &mut Psbt, sign_options: SignOptions, ) -> Result { + let tx = &psbt.unsigned_tx; let chain_tip = self.chain.tip().block_id(); + let prev_txids = tx + .input + .iter() + .map(|txin| txin.previous_output.txid) + .collect::>(); + let confirmation_heights = + self.indexed_graph + .graph() + .list_canonical_txs(&self.chain, chain_tip) + .filter(|canon_tx| prev_txids.contains(&canon_tx.tx_node.txid)) + .take(prev_txids.len()) + .map(|canon_tx| { + let txid = canon_tx.tx_node.txid; + match canon_tx.chain_position { + ChainPosition::Confirmed(a) + | ChainPosition::ConfirmedByTransitivity(_, a) => (txid, a.block_id.height), + ChainPosition::Unconfirmed(_) | ChainPosition::UnconfirmedAndNotSeen => { + (txid, u32::MAX) + } + } + }) + .collect::>(); - let tx = &psbt.unsigned_tx; let mut finished = true; for (n, input) in tx.input.iter().enumerate() { @@ -1843,16 +1867,9 @@ impl Wallet { if psbt_input.final_script_sig.is_some() || psbt_input.final_script_witness.is_some() { continue; } - let confirmation_height = self - .indexed_graph - .graph() - .get_chain_position(&self.chain, chain_tip, input.previous_output.txid) - .map(|chain_position| match chain_position { - ChainPosition::Confirmed(a) - // TODO: This is the upper-bound of the confirmation height. Is this okay? - | ChainPosition::ConfirmedByTransitivity(_, a) => a.block_id.height, - ChainPosition::Unconfirmed(_) | ChainPosition::UnconfirmedAndNotSeen => u32::MAX, - }); + let confirmation_height = confirmation_heights + .get(&input.previous_output.txid) + .copied(); let current_height = sign_options .assume_height .unwrap_or_else(|| self.chain.tip().height()); @@ -2012,20 +2029,22 @@ impl Wallet { return (must_spend, vec![]); } + let canon_txs = self + .indexed_graph + .graph() + .list_canonical_txs(&self.chain, chain_tip) + .map(|canon_tx| (canon_tx.tx_node.txid, canon_tx)) + .collect::>(); + let satisfies_confirmed = may_spend .iter() .map(|u| -> bool { let txid = u.0.outpoint.txid; - let tx = match self.indexed_graph.graph().get_tx(txid) { - Some(tx) => tx, - None => return false, - }; - let chain_position = match self.indexed_graph.graph().get_chain_position( - &self.chain, - chain_tip, - txid, - ) { - Some(chain_position) => chain_position.cloned(), + let (chain_position, tx) = match canon_txs.get(&txid) { + Some(CanonicalTx { + chain_position, + tx_node, + }) => (chain_position, tx_node.tx.clone()), None => return false, };