From ac336aa32f485cb253ac7ea5cae3b4bcc78cf507 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Sat, 22 Apr 2023 22:56:51 +0800 Subject: [PATCH] [bdk_chain_redesign] Make `insert_relevant_txs` topologically-agnostic The `insert_relevant_txs` test has also been changed to used `KeychainTxOutIndex` so that index additions can be checked (`SpkTxOutIndex` has no additions). Additionally, generic bounds of some `IndexedTxGraph` list methods have been fixed. --- crates/chain/src/indexed_tx_graph.rs | 50 ++++++++++----------- crates/chain/tests/test_indexed_tx_graph.rs | 22 +++++---- 2 files changed, 38 insertions(+), 34 deletions(-) diff --git a/crates/chain/src/indexed_tx_graph.rs b/crates/chain/src/indexed_tx_graph.rs index 60a6c646..20d9958e 100644 --- a/crates/chain/src/indexed_tx_graph.rs +++ b/crates/chain/src/indexed_tx_graph.rs @@ -117,7 +117,7 @@ where /// Insert relevant transactions from the given `txs` iterator. /// /// Relevancy is determined by the [`Indexer::is_tx_relevant`] implementation of `I`. Irrelevant - /// transactions in `txs` will be ignored. + /// transactions in `txs` will be ignored. Also, `txs` does not need to be in topological order. /// /// `anchors` can be provided to anchor the transactions to blocks. `seen_at` is a unix /// timestamp of when the transactions are last seen. @@ -127,28 +127,31 @@ where anchors: impl IntoIterator + Clone, seen_at: Option, ) -> IndexedAdditions { - // As mentioned by @LLFourn: This algorithm requires the transactions to be topologically - // sorted because most indexers cannot decide whether something is relevant unless you have - // first inserted its ancestors in the index. We can fix this if we instead do this: + // The algorithm below allows for non-topologically ordered transactions by using two loops. + // This is achieved by: // 1. insert all txs into the index. If they are irrelevant then that's fine it will just // not store anything about them. // 2. decide whether to insert them into the graph depending on whether `is_tx_relevant` // returns true or not. (in a second loop). - let txs = txs - .into_iter() - .inspect(|tx| { - let _ = self.index.index_tx(tx); - }) - .collect::>(); - txs.into_iter() - .filter_map(|tx| match self.index.is_tx_relevant(tx) { - true => Some(self.insert_tx(tx, anchors.clone(), seen_at)), - false => None, - }) - .fold(Default::default(), |mut acc, other| { - acc.append(other); - acc - }) + let mut additions = IndexedAdditions::::default(); + let mut transactions = Vec::new(); + for tx in txs.into_iter() { + additions.index_additions.append(self.index.index_tx(tx)); + transactions.push(tx); + } + additions.append( + transactions + .into_iter() + .filter_map(|tx| match self.index.is_tx_relevant(tx) { + true => Some(self.insert_tx(tx, anchors.clone(), seen_at)), + false => None, + }) + .fold(Default::default(), |mut acc, other| { + acc.append(other); + acc + }), + ); + additions } } @@ -170,7 +173,7 @@ impl IndexedTxGraph { }) } - pub fn list_owned_txouts<'a, C: ChainOracle + 'a>( + pub fn list_owned_txouts<'a, C: ChainOracle + 'a>( &'a self, chain: &'a C, chain_tip: BlockId, @@ -196,14 +199,11 @@ impl IndexedTxGraph { }) } - pub fn list_owned_unspents<'a, C>( + pub fn list_owned_unspents<'a, C: ChainOracle + 'a>( &'a self, chain: &'a C, chain_tip: BlockId, - ) -> impl Iterator>> + 'a - where - C: ChainOracle + 'a, - { + ) -> impl Iterator>> + 'a { self.try_list_owned_unspents(chain, chain_tip) .map(|r| r.expect("oracle is infallible")) } diff --git a/crates/chain/tests/test_indexed_tx_graph.rs b/crates/chain/tests/test_indexed_tx_graph.rs index e85d424e..26a30cb8 100644 --- a/crates/chain/tests/test_indexed_tx_graph.rs +++ b/crates/chain/tests/test_indexed_tx_graph.rs @@ -2,10 +2,12 @@ mod common; use bdk_chain::{ indexed_tx_graph::{IndexedAdditions, IndexedTxGraph}, + keychain::{DerivationAdditions, KeychainTxOutIndex}, tx_graph::Additions, - BlockId, SpkTxOutIndex, + BlockId, }; -use bitcoin::{hashes::hex::FromHex, OutPoint, Script, Transaction, TxIn, TxOut}; +use bitcoin::{secp256k1::Secp256k1, OutPoint, Transaction, TxIn, TxOut}; +use miniscript::Descriptor; /// Ensure [`IndexedTxGraph::insert_relevant_txs`] can successfully index transactions NOT presented /// in topological order. @@ -16,13 +18,15 @@ use bitcoin::{hashes::hex::FromHex, OutPoint, Script, Transaction, TxIn, TxOut}; /// agnostic. #[test] fn insert_relevant_txs() { - let mut graph = IndexedTxGraph::>::default(); + const DESCRIPTOR: &str = "tr([73c5da0a/86'/0'/0']xprv9xgqHN7yz9MwCkxsBPN5qetuNdQSUttZNKw1dcYTV4mkaAFiBVGQziHs3NRSWMkCzvgjEe3n9xV8oYywvM8at9yRqyaZVz6TYYhX98VjsUk/0/*)"; + let (descriptor, _) = Descriptor::parse_descriptor(&Secp256k1::signing_only(), DESCRIPTOR) + .expect("must be valid"); + let spk_0 = descriptor.at_derivation_index(0).script_pubkey(); + let spk_1 = descriptor.at_derivation_index(9).script_pubkey(); - // insert some spks - let spk_0 = Script::from_hex("0014034f9515cace31713707dff8194b8f550eb6d336").unwrap(); - let spk_1 = Script::from_hex("0014beaa39ab2b4f47995c77107d8c3f481d3bd33941").unwrap(); - graph.index.insert_spk(0, spk_0.clone()); - graph.index.insert_spk(1, spk_1.clone()); + let mut graph = IndexedTxGraph::>::default(); + graph.index.add_keychain((), descriptor); + graph.index.set_lookahead(&(), 10); let tx_a = Transaction { output: vec![ @@ -63,7 +67,7 @@ fn insert_relevant_txs() { tx: txs.into(), ..Default::default() }, - ..Default::default() + index_additions: DerivationAdditions([((), 9_u32)].into()), } ) }