Changed `inflate_update` logic to not depend on `Cow`

As mentioned by @LLFourn:

1. We have a "sparse chain" from which there is a subset of txids M that are missing from graph.
2. There is also another subset C that are in the graph but their positions have changed.
3. We used the Cow to avoid copying/duplicating in memory transactions in subset C and M

Instead in inflate_update we could remove transactions in subset M and just clone data in subset C (which is usually tiny).

diff --git a/crates/chain/src/chain_graph.rs b/crates/chain/src/chain_graph.rs
index ab0db6d91c1a0b53ab2060bd2ba259652a1099b5..a446ed2073e97e5d567bca8b67348ce582b3d7c4 100644 (file)
--- a/crates/chain/src/chain_graph.rs
+++ b/crates/chain/src/chain_graph.rs
@@ -5,7 +5,7 @@ use crate::{
     tx_graph::{self, TxGraph},
     AsTransaction, BlockId, ForEachTxOut, FullTxOut, IntoOwned, TxHeight,
 };
-use alloc::{borrow::Cow, string::ToString, vec::Vec};
+use alloc::{string::ToString, vec::Vec};
 use bitcoin::{OutPoint, Transaction, TxOut, Txid};
 use core::fmt::Debug;
 
@@ -129,19 +129,47 @@ where
         &self,
         update: SparseChain<P>,
         new_txs: impl IntoIterator<Item = T>,
-    ) -> Result<ChainGraph<P, Cow<T>>, NewError<P>> {
+    ) -> Result<ChainGraph<P, T>, NewError<P>> {
+        let mut inflated_chain = SparseChain::default();
         let mut inflated_graph = TxGraph::default();
-        for (_, txid) in update.txids() {
-            if let Some(tx) = self.graph.get_tx(*txid) {
-                let _ = inflated_graph.insert_tx(Cow::Borrowed(tx));
+
+        for (height, hash) in update.checkpoints().clone().into_iter() {
+            let _ = inflated_chain
+                .insert_checkpoint(BlockId { height, hash })
+                .expect("must insert");
+        }
+
+        // [TODO] @evanlinjin: These need better comments
+        // - copy transactions that have changed positions into the graph
+        // - add new transactions to inflated chain
+        for (pos, txid) in update.txids() {
+            match self.chain.tx_position(*txid) {
+                Some(original_pos) => {
+                    if original_pos != pos {
+                        let tx = self
+                            .graph
+                            .get_tx(*txid)
+                            .expect("tx must exist as it is referenced in sparsechain")
+                            .clone();
+                        let _ = inflated_chain
+                            .insert_tx(*txid, pos.clone())
+                            .expect("must insert since this was already in update");
+                        let _ = inflated_graph.insert_tx(tx);
+                    }
+                }
+                None => {
+                    let _ = inflated_chain
+                        .insert_tx(*txid, pos.clone())
+                        .expect("must insert since this was already in update");
+                }
             }
         }
 
         for tx in new_txs {
-            let _ = inflated_graph.insert_tx(Cow::Owned(tx));
+            let _ = inflated_graph.insert_tx(tx);
         }
 
-        ChainGraph::new(update, inflated_graph)
+        ChainGraph::new(inflated_chain, inflated_graph)
     }
 
     /// Sets the checkpoint limit.

diff --git a/crates/electrum/src/lib.rs b/crates/electrum/src/lib.rs
index 8f07de594b6aae96ceac95149133131d3a83c9c6..9f5061caf309071da9aca6acebab3164d3ec8c7a 100644 (file)
--- a/crates/electrum/src/lib.rs
+++ b/crates/electrum/src/lib.rs
@@ -21,7 +21,6 @@
 //! [`bdk_electrum_example`]: https://github.com/LLFourn/bdk_core_staging/tree/master/bdk_electrum_example
 
 use std::{
-    borrow::Cow,
     collections::{BTreeMap, HashMap},
     fmt::Debug,
 };
@@ -249,7 +248,7 @@ impl<K: Ord + Clone + Debug, P: ChainPosition> ElectrumUpdate<K, P> {
         self,
         new_txs: Vec<T>,
         chain_graph: &CG,
-    ) -> Result<KeychainScan<K, P, Cow<T>>, chain_graph::NewError<P>>
+    ) -> Result<KeychainScan<K, P, T>, chain_graph::NewError<P>>
     where
         T: AsTransaction + Clone + Ord,
         CG: AsRef<ChainGraph<P, T>>,