From 67d5fa695f3f748d176c182d25be5ef70f5127ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Mon, 2 Sep 2024 17:32:19 +0800 Subject: [PATCH 1/3] feat(chain): make various insert tx methods more generic Instead of having `Transaction` as input, we have a generic parameter where the bound is `Into>` for the following methods: * `IndexedTxGraph::insert_tx` * `IndexedTxGraph::batch_insert_unconfirmed` * `TxGraph::batch_insert_unconfirmed` --- crates/chain/src/indexed_tx_graph.rs | 17 +++++++---------- crates/chain/src/tx_graph.rs | 5 +++-- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/crates/chain/src/indexed_tx_graph.rs b/crates/chain/src/indexed_tx_graph.rs index ed2a1f0ce..b2da7bf75 100644 --- a/crates/chain/src/indexed_tx_graph.rs +++ b/crates/chain/src/indexed_tx_graph.rs @@ -2,7 +2,7 @@ //! [`IndexedTxGraph`] documentation for more. use core::fmt::Debug; -use alloc::vec::Vec; +use alloc::{sync::Arc, vec::Vec}; use bitcoin::{Block, OutPoint, Transaction, TxOut, Txid}; use crate::{ @@ -133,13 +133,10 @@ where } /// Insert and index a transaction into the graph. - pub fn insert_tx(&mut self, tx: Transaction) -> ChangeSet { - let graph = self.graph.insert_tx(tx); - let indexer = self.index_tx_graph_changeset(&graph); - ChangeSet { - tx_graph: graph, - indexer, - } + pub fn insert_tx>>(&mut self, tx: T) -> ChangeSet { + let tx_graph = self.graph.insert_tx(tx); + let indexer = self.index_tx_graph_changeset(&tx_graph); + ChangeSet { tx_graph, indexer } } /// Insert an `anchor` for a given transaction. @@ -239,9 +236,9 @@ where /// To filter out irrelevant transactions, use [`batch_insert_relevant_unconfirmed`] instead. /// /// [`batch_insert_relevant_unconfirmed`]: IndexedTxGraph::batch_insert_relevant_unconfirmed - pub fn batch_insert_unconfirmed( + pub fn batch_insert_unconfirmed>>( &mut self, - txs: impl IntoIterator, + txs: impl IntoIterator, ) -> ChangeSet { let graph = self.graph.batch_insert_unconfirmed(txs); let indexer = self.index_tx_graph_changeset(&graph); diff --git a/crates/chain/src/tx_graph.rs b/crates/chain/src/tx_graph.rs index 127b47cf2..9a32ccdfc 100644 --- a/crates/chain/src/tx_graph.rs +++ b/crates/chain/src/tx_graph.rs @@ -606,12 +606,13 @@ impl TxGraph { /// Items of `txs` are tuples containing the transaction and a *last seen* timestamp. The /// *last seen* communicates when the transaction is last seen in mempool which is used for /// conflict-resolution (refer to [`TxGraph::insert_seen_at`] for details). - pub fn batch_insert_unconfirmed( + pub fn batch_insert_unconfirmed>>( &mut self, - txs: impl IntoIterator, + txs: impl IntoIterator, ) -> ChangeSet { let mut changeset = ChangeSet::::default(); for (tx, seen_at) in txs { + let tx: Arc = tx.into(); changeset.merge(self.insert_seen_at(tx.compute_txid(), seen_at)); changeset.merge(self.insert_tx(tx)); } From c39284d8299c5a8b34b73ce947e93425d5cdc121 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Tue, 3 Sep 2024 13:18:20 +0800 Subject: [PATCH 2/3] feat(wallet): make `Wallet::insert_tx` generic Instead of having `Transaction` as input, have `T: Into>`. --- crates/wallet/src/wallet/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/wallet/src/wallet/mod.rs b/crates/wallet/src/wallet/mod.rs index 5cad6bb86..8513251ae 100644 --- a/crates/wallet/src/wallet/mod.rs +++ b/crates/wallet/src/wallet/mod.rs @@ -1093,7 +1093,7 @@ impl Wallet { /// By default the inserted `tx` won't be considered "canonical" because it's not known /// whether the transaction exists in the best chain. To know whether it exists, the tx /// must be broadcast to the network and the wallet synced via a chain source. - pub fn insert_tx(&mut self, tx: Transaction) -> bool { + pub fn insert_tx>>(&mut self, tx: T) -> bool { let mut changeset = ChangeSet::default(); changeset.merge(self.indexed_graph.insert_tx(tx).into()); let ret = !changeset.is_empty(); From 87e61212f556bccfaa2a64b74f727cedc0f70e9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Mon, 2 Sep 2024 17:59:09 +0800 Subject: [PATCH 3/3] feat(chain,wallet)!: change methods to take in generic instead of `&Transaction` * `Wallet::apply_unconfirmed_txs` * `IndexedTxGraph::batch_insert_relevant` * `IndexedTxGraph::batch_insert_relevant_unconfirmed` --- crates/chain/src/indexed_tx_graph.rs | 31 ++++++++++--------- crates/chain/tests/test_indexed_tx_graph.rs | 9 +++--- crates/wallet/src/wallet/mod.rs | 4 +-- .../example_bitcoind_rpc_polling/src/main.rs | 11 +++---- example-crates/wallet_rpc/src/main.rs | 2 +- 5 files changed, 30 insertions(+), 27 deletions(-) diff --git a/crates/chain/src/indexed_tx_graph.rs b/crates/chain/src/indexed_tx_graph.rs index b2da7bf75..9cb1e820e 100644 --- a/crates/chain/src/indexed_tx_graph.rs +++ b/crates/chain/src/indexed_tx_graph.rs @@ -156,9 +156,9 @@ where /// /// Relevancy is determined by the [`Indexer::is_tx_relevant`] implementation of `I`. Irrelevant /// transactions in `txs` will be ignored. `txs` do not need to be in topological order. - pub fn batch_insert_relevant<'t>( + pub fn batch_insert_relevant>>( &mut self, - txs: impl IntoIterator)>, + txs: impl IntoIterator)>, ) -> ChangeSet { // The algorithm below allows for non-topologically ordered transactions by using two loops. // This is achieved by: @@ -166,28 +166,28 @@ where // 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().collect::>(); + let txs = txs + .into_iter() + .map(|(tx, anchors)| (>>::into(tx), anchors)) + .collect::>(); let mut indexer = I::ChangeSet::default(); for (tx, _) in &txs { indexer.merge(self.index.index_tx(tx)); } - let mut graph = tx_graph::ChangeSet::default(); + let mut tx_graph = tx_graph::ChangeSet::default(); for (tx, anchors) in txs { - if self.index.is_tx_relevant(tx) { + if self.index.is_tx_relevant(&tx) { let txid = tx.compute_txid(); - graph.merge(self.graph.insert_tx(tx.clone())); + tx_graph.merge(self.graph.insert_tx(tx.clone())); for anchor in anchors { - graph.merge(self.graph.insert_anchor(txid, anchor)); + tx_graph.merge(self.graph.insert_anchor(txid, anchor)); } } } - ChangeSet { - tx_graph: graph, - indexer, - } + ChangeSet { tx_graph, indexer } } /// Batch insert unconfirmed transactions, filtering out those that are irrelevant. @@ -198,9 +198,9 @@ where /// Items of `txs` are tuples containing the transaction and a *last seen* timestamp. The /// *last seen* communicates when the transaction is last seen in the mempool which is used for /// conflict-resolution in [`TxGraph`] (refer to [`TxGraph::insert_seen_at`] for details). - pub fn batch_insert_relevant_unconfirmed<'t>( + pub fn batch_insert_relevant_unconfirmed>>( &mut self, - unconfirmed_txs: impl IntoIterator, + unconfirmed_txs: impl IntoIterator, ) -> ChangeSet { // The algorithm below allows for non-topologically ordered transactions by using two loops. // This is achieved by: @@ -208,7 +208,10 @@ where // 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 = unconfirmed_txs.into_iter().collect::>(); + let txs = unconfirmed_txs + .into_iter() + .map(|(tx, last_seen)| (>>::into(tx), last_seen)) + .collect::>(); let mut indexer = I::ChangeSet::default(); for (tx, _) in &txs { diff --git a/crates/chain/tests/test_indexed_tx_graph.rs b/crates/chain/tests/test_indexed_tx_graph.rs index 361e5ab5c..1fbbcd0df 100644 --- a/crates/chain/tests/test_indexed_tx_graph.rs +++ b/crates/chain/tests/test_indexed_tx_graph.rs @@ -81,7 +81,7 @@ fn insert_relevant_txs() { }; assert_eq!( - graph.batch_insert_relevant(txs.iter().map(|tx| (tx, None))), + graph.batch_insert_relevant(txs.iter().cloned().map(|tx| (tx, None))), changeset, ); @@ -237,10 +237,10 @@ fn test_list_owned_txouts() { // Insert unconfirmed txs with a last_seen timestamp let _ = - graph.batch_insert_relevant([&tx1, &tx2, &tx3, &tx6].iter().enumerate().map(|(i, tx)| { + graph.batch_insert_relevant([&tx1, &tx2, &tx3, &tx6].iter().enumerate().map(|(i, &tx)| { let height = i as u32; ( - *tx, + tx.clone(), local_chain .get(height) .map(|cp| cp.block_id()) @@ -251,7 +251,8 @@ fn test_list_owned_txouts() { ) })); - let _ = graph.batch_insert_relevant_unconfirmed([&tx4, &tx5].iter().map(|tx| (*tx, 100))); + let _ = + graph.batch_insert_relevant_unconfirmed([&tx4, &tx5].iter().map(|&tx| (tx.clone(), 100))); // A helper lambda to extract and filter data from the graph. let fetch = diff --git a/crates/wallet/src/wallet/mod.rs b/crates/wallet/src/wallet/mod.rs index 8513251ae..118fae088 100644 --- a/crates/wallet/src/wallet/mod.rs +++ b/crates/wallet/src/wallet/mod.rs @@ -2441,9 +2441,9 @@ impl Wallet { /// **WARNING**: You must persist the changes resulting from one or more calls to this method /// if you need the applied unconfirmed transactions to be reloaded after closing the wallet. /// See [`Wallet::reveal_next_address`]. - pub fn apply_unconfirmed_txs<'t>( + pub fn apply_unconfirmed_txs>>( &mut self, - unconfirmed_txs: impl IntoIterator, + unconfirmed_txs: impl IntoIterator, ) { let indexed_graph_changeset = self .indexed_graph diff --git a/example-crates/example_bitcoind_rpc_polling/src/main.rs b/example-crates/example_bitcoind_rpc_polling/src/main.rs index d1833b071..95c547967 100644 --- a/example-crates/example_bitcoind_rpc_polling/src/main.rs +++ b/example-crates/example_bitcoind_rpc_polling/src/main.rs @@ -201,9 +201,10 @@ fn main() -> anyhow::Result<()> { } let mempool_txs = emitter.mempool()?; - let graph_changeset = graph.lock().unwrap().batch_insert_relevant_unconfirmed( - mempool_txs.iter().map(|(tx, time)| (tx, *time)), - ); + let graph_changeset = graph + .lock() + .unwrap() + .batch_insert_relevant_unconfirmed(mempool_txs); { let db = &mut *db.lock().unwrap(); db_stage.merge(ChangeSet { @@ -286,9 +287,7 @@ fn main() -> anyhow::Result<()> { (chain_changeset, graph_changeset) } Emission::Mempool(mempool_txs) => { - let graph_changeset = graph.batch_insert_relevant_unconfirmed( - mempool_txs.iter().map(|(tx, time)| (tx, *time)), - ); + let graph_changeset = graph.batch_insert_relevant_unconfirmed(mempool_txs); (local_chain::ChangeSet::default(), graph_changeset) } Emission::Tip(h) => { diff --git a/example-crates/wallet_rpc/src/main.rs b/example-crates/wallet_rpc/src/main.rs index 388ccaf67..af6dcafc4 100644 --- a/example-crates/wallet_rpc/src/main.rs +++ b/example-crates/wallet_rpc/src/main.rs @@ -157,7 +157,7 @@ fn main() -> anyhow::Result<()> { } Emission::Mempool(mempool_emission) => { let start_apply_mempool = Instant::now(); - wallet.apply_unconfirmed_txs(mempool_emission.iter().map(|(tx, time)| (tx, *time))); + wallet.apply_unconfirmed_txs(mempool_emission); wallet.persist(&mut db)?; println!( "Applied unconfirmed transactions in {}s",