Merge #1808: Introduce canonicalization parameters

53afb35 test(graph): Add additional witness scenarios (Wei Chen)
46af2a5 feat(chain)!: Add ability to modify canonicalization algorithm (志宇)
58fcf32 refactor(chain): Make private fields in `CanonicalIter` concise. (志宇)
5eb5e86 feat(chain): Signed txs should displace unsigned txs in `TxGraph`. (志宇)

Pull request description:

  Partially Fixes bitcoindevkit/bdk_wallet#40

  ### Description

  Add the ability to modify the canonicalization algorithm. Right now, the only modifier is `assume_canonical` which takes in a `Vec` (ordered list) of txids and superimposes it on the canonicalization algorithm. Txs later in the list (higher index) have a higher priority (in case of conflicts).

  ### Notes to the reviewers

  None at the moment.

  ### Changelog notice

  * Added `CanonicalizationParams` to allow the caller to modify the canonicalization algorithm. This in a new parameter on `CanonicalIter::new`.
  * Changed `TxGraph::insert_tx` to allow for updating a transaction's witness field. This is useful for initially introducing an unsigned tx and adding witnesses later on.

  ### Checklists

  #### All Submissions:

  * [x] I've signed all my commits
  * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
  * [x] I ran `cargo fmt` and `cargo clippy` before committing

  #### New Features:

  * [x] I've added tests for the new feature
  * [x] I've added docs for the new feature

  #### Bugfixes:

  * [x] This pull request breaks the existing API
  * [x] I've added tests to reproduce the issue which are now passing
  * [x] I'm linking the issue being fixed by this PR

ACKs for top commit:
  LagginTimes:
    ACK 53afb35
  ValuedMammal:
    ACK 53afb35

Tree-SHA512: 9fd070a3829d194f4d216c873922d9d47e047fa4b618628b875456f5cf4661d6b6c08a3a0dd4739b3a0f765e47c7c99acca53f195379e37f0f9cfed027bde17b

Author: ValuedMammal

crates/bitcoind_rpc/examples/filter_iter.rs

@@ -92,6 +92,7 @@ fn main() -> anyhow::Result<()> {
         .filter_chain_unspents(
             &chain,
             chain.tip().block_id(),
+            Default::default(),
             graph.index.outpoints().clone(),
         )
         .collect();

crates/bitcoind_rpc/tests/test_emitter.rs

@@ -5,7 +5,7 @@ use bdk_chain::{
     bitcoin::{Address, Amount, Txid},
     local_chain::{CheckPoint, LocalChain},
     spk_txout::SpkTxOutIndex,
-    Balance, BlockId, IndexedTxGraph, Merge,
+    Balance, BlockId, CanonicalizationParams, IndexedTxGraph, Merge,
 };
 use bdk_testenv::{anyhow, TestEnv};
 use bitcoin::{hashes::Hash, Block, OutPoint, ScriptBuf, WScriptHash};
@@ -306,9 +306,13 @@ fn get_balance(
 ) -> anyhow::Result<Balance> {
     let chain_tip = recv_chain.tip().block_id();
     let outpoints = recv_graph.index.outpoints().clone();
-    let balance = recv_graph
-        .graph()
-        .balance(recv_chain, chain_tip, outpoints, |_, _| true);
+    let balance = recv_graph.graph().balance(
+        recv_chain,
+        chain_tip,
+        CanonicalizationParams::default(),
+        outpoints,
+        |_, _| true,
+    );
     Ok(balance)
 }
 

crates/chain/benches/canonicalization.rs

@@ -1,3 +1,4 @@
+use bdk_chain::CanonicalizationParams;
 use bdk_chain::{keychain_txout::KeychainTxOutIndex, local_chain::LocalChain, IndexedTxGraph};
 use bdk_core::{BlockId, CheckPoint};
 use bdk_core::{ConfirmationBlockTime, TxUpdate};
@@ -90,16 +91,19 @@ fn setup<F: Fn(&mut KeychainTxGraph, &LocalChain)>(f: F) -> (KeychainTxGraph, Lo
 }
 
 fn run_list_canonical_txs(tx_graph: &KeychainTxGraph, chain: &LocalChain, exp_txs: usize) {
-    let txs = tx_graph
-        .graph()
-        .list_canonical_txs(chain, chain.tip().block_id());
+    let txs = tx_graph.graph().list_canonical_txs(
+        chain,
+        chain.tip().block_id(),
+        CanonicalizationParams::default(),
+    );
     assert_eq!(txs.count(), exp_txs);
 }
 
 fn run_filter_chain_txouts(tx_graph: &KeychainTxGraph, chain: &LocalChain, exp_txos: usize) {
     let utxos = tx_graph.graph().filter_chain_txouts(
         chain,
         chain.tip().block_id(),
+        CanonicalizationParams::default(),
         tx_graph.index.outpoints().clone(),
     );
     assert_eq!(utxos.count(), exp_txos);
@@ -109,6 +113,7 @@ fn run_filter_chain_unspents(tx_graph: &KeychainTxGraph, chain: &LocalChain, exp
     let utxos = tx_graph.graph().filter_chain_unspents(
         chain,
         chain.tip().block_id(),
+        CanonicalizationParams::default(),
         tx_graph.index.outpoints().clone(),
     );
     assert_eq!(utxos.count(), exp_utxos);

crates/chain/src/canonical_iter.rs

@@ -11,16 +11,27 @@ use bitcoin::{Transaction, Txid};
 type CanonicalMap<A> = HashMap<Txid, (Arc<Transaction>, CanonicalReason<A>)>;
 type NotCanonicalSet = HashSet<Txid>;
 
+/// Modifies the canonicalization algorithm.
+#[derive(Debug, Default, Clone)]
+pub struct CanonicalizationParams {
+    /// Transactions that will supercede all other transactions.
+    ///
+    /// In case of conflicting transactions within `assume_canonical`, transactions that appear
+    /// later in the list (have higher index) have precedence.
+    pub assume_canonical: Vec<Txid>,
+}
+
 /// Iterates over canonical txs.
 pub struct CanonicalIter<'g, A, C> {
     tx_graph: &'g TxGraph<A>,
     chain: &'g C,
     chain_tip: BlockId,
 
-    unprocessed_txs_with_anchors:
+    unprocessed_assumed_txs: Box<dyn Iterator<Item = (Txid, Arc<Transaction>)> + 'g>,
+    unprocessed_anchored_txs:
         Box<dyn Iterator<Item = (Txid, Arc<Transaction>, &'g BTreeSet<A>)> + 'g>,
-    unprocessed_txs_with_last_seens: Box<dyn Iterator<Item = (Txid, Arc<Transaction>, u64)> + 'g>,
-    unprocessed_txs_left_over: VecDeque<(Txid, Arc<Transaction>, u32)>,
+    unprocessed_seen_txs: Box<dyn Iterator<Item = (Txid, Arc<Transaction>, u64)> + 'g>,
+    unprocessed_leftover_txs: VecDeque<(Txid, Arc<Transaction>, u32)>,
 
     canonical: CanonicalMap<A>,
     not_canonical: NotCanonicalSet,
@@ -30,14 +41,26 @@ pub struct CanonicalIter<'g, A, C> {
 
 impl<'g, A: Anchor, C: ChainOracle> CanonicalIter<'g, A, C> {
     /// Constructs [`CanonicalIter`].
-    pub fn new(tx_graph: &'g TxGraph<A>, chain: &'g C, chain_tip: BlockId) -> Self {
+    pub fn new(
+        tx_graph: &'g TxGraph<A>,
+        chain: &'g C,
+        chain_tip: BlockId,
+        params: CanonicalizationParams,
+    ) -> Self {
         let anchors = tx_graph.all_anchors();
-        let pending_anchored = Box::new(
+        let unprocessed_assumed_txs = Box::new(
+            params
+                .assume_canonical
+                .into_iter()
+                .rev()
+                .filter_map(|txid| Some((txid, tx_graph.get_tx(txid)?))),
+        );
+        let unprocessed_anchored_txs = Box::new(
             tx_graph
                 .txids_by_descending_anchor_height()
                 .filter_map(|(_, txid)| Some((txid, tx_graph.get_tx(txid)?, anchors.get(&txid)?))),
         );
-        let pending_last_seen = Box::new(
+        let unprocessed_seen_txs = Box::new(
             tx_graph
                 .txids_by_descending_last_seen()
                 .filter_map(|(last_seen, txid)| Some((txid, tx_graph.get_tx(txid)?, last_seen))),
@@ -46,9 +69,10 @@ impl<'g, A: Anchor, C: ChainOracle> CanonicalIter<'g, A, C> {
             tx_graph,
             chain,
             chain_tip,
-            unprocessed_txs_with_anchors: pending_anchored,
-            unprocessed_txs_with_last_seens: pending_last_seen,
-            unprocessed_txs_left_over: VecDeque::new(),
+            unprocessed_assumed_txs,
+            unprocessed_anchored_txs,
+            unprocessed_seen_txs,
+            unprocessed_leftover_txs: VecDeque::new(),
             canonical: HashMap::new(),
             not_canonical: HashSet::new(),
             queue: VecDeque::new(),
@@ -77,7 +101,7 @@ impl<'g, A: Anchor, C: ChainOracle> CanonicalIter<'g, A, C> {
             }
         }
         // cannot determine
-        self.unprocessed_txs_left_over.push_back((
+        self.unprocessed_leftover_txs.push_back((
             txid,
             tx,
             anchors
@@ -190,7 +214,13 @@ impl<A: Anchor, C: ChainOracle> Iterator for CanonicalIter<'_, A, C> {
                 return Some(Ok((txid, tx, reason)));
             }
 
-            if let Some((txid, tx, anchors)) = self.unprocessed_txs_with_anchors.next() {
+            if let Some((txid, tx)) = self.unprocessed_assumed_txs.next() {
+                if !self.is_canonicalized(txid) {
+                    self.mark_canonical(txid, tx, CanonicalReason::assumed());
+                }
+            }
+
+            if let Some((txid, tx, anchors)) = self.unprocessed_anchored_txs.next() {
                 if !self.is_canonicalized(txid) {
                     if let Err(err) = self.scan_anchors(txid, tx, anchors) {
                         return Some(Err(err));
@@ -199,15 +229,15 @@ impl<A: Anchor, C: ChainOracle> Iterator for CanonicalIter<'_, A, C> {
                 continue;
             }
 
-            if let Some((txid, tx, last_seen)) = self.unprocessed_txs_with_last_seens.next() {
+            if let Some((txid, tx, last_seen)) = self.unprocessed_seen_txs.next() {
                 if !self.is_canonicalized(txid) {
                     let observed_in = ObservedIn::Mempool(last_seen);
                     self.mark_canonical(txid, tx, CanonicalReason::from_observed_in(observed_in));
                 }
                 continue;
             }
 
-            if let Some((txid, tx, height)) = self.unprocessed_txs_left_over.pop_front() {
+            if let Some((txid, tx, height)) = self.unprocessed_leftover_txs.pop_front() {
                 if !self.is_canonicalized(txid) {
                     let observed_in = ObservedIn::Block(height);
                     self.mark_canonical(txid, tx, CanonicalReason::from_observed_in(observed_in));
@@ -232,6 +262,12 @@ pub enum ObservedIn {
 /// The reason why a transaction is canonical.
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub enum CanonicalReason<A> {
+    /// This transaction is explicitly assumed to be canonical by the caller, superceding all other
+    /// canonicalization rules.
+    Assumed {
+        /// Whether it is a descendant that is assumed to be canonical.
+        descendant: Option<Txid>,
+    },
     /// This transaction is anchored in the best chain by `A`, and therefore canonical.
     Anchor {
         /// The anchor that anchored the transaction in the chain.
@@ -250,6 +286,12 @@ pub enum CanonicalReason<A> {
 }
 
 impl<A: Clone> CanonicalReason<A> {
+    /// Constructs a [`CanonicalReason`] for a transaction that is assumed to supercede all other
+    /// transactions.
+    pub fn assumed() -> Self {
+        Self::Assumed { descendant: None }
+    }
+
     /// Constructs a [`CanonicalReason`] from an `anchor`.
     pub fn from_anchor(anchor: A) -> Self {
         Self::Anchor {
@@ -272,6 +314,9 @@ impl<A: Clone> CanonicalReason<A> {
     /// descendant, but is transitively relevant.
     pub fn to_transitive(&self, descendant: Txid) -> Self {
         match self {
+            CanonicalReason::Assumed { .. } => Self::Assumed {
+                descendant: Some(descendant),
+            },
             CanonicalReason::Anchor { anchor, .. } => Self::Anchor {
                 anchor: anchor.clone(),
                 descendant: Some(descendant),
@@ -287,6 +332,7 @@ impl<A: Clone> CanonicalReason<A> {
     /// descendant.
     pub fn descendant(&self) -> &Option<Txid> {
         match self {
+            CanonicalReason::Assumed { descendant, .. } => descendant,
             CanonicalReason::Anchor { descendant, .. } => descendant,
             CanonicalReason::ObservedIn { descendant, .. } => descendant,
         }