Merge bitcoindevkit/bdk#1643: feat(chain,wallet)!: rm ConfirmationTime

a3d4eef feat(chain,wallet)!: rm `ConfirmationTime` (志宇)

Pull request description:

  ### Description

  This PR removes `ConfirmationTime`, and favors `ChainPosition<ConfirmationBlockTime>` instead. The only difference between these two structures is that `ChainPosition<ConfirmationBlockTime>` contains an additional `BlockHash`. Additionally, `ConfirmationTime` was not used in many places. It was mainly for displaying information in `bdk_wallet::Wallet`.

  We also impl `serde::Deserialize` and `serde::Serialize` for `ChainPosition`.

  ### Notes to the reviewers

  ### Changelog notice

  * Remove `bdk_chain::ConfirmationTime`. Use `ChainPosition<ConfirmationBlockTime>` in place.

  ### 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

ACKs for top commit:
  LagginTimes:
    ACK a3d4eef
  oleonardolima:
    ACK a3d4eef
  ValuedMammal:
    ACK a3d4eef

Tree-SHA512: d94db70885e6987774da586b92ee826098a0da4ae808ff9b23632bd68bbb3d6babbba1aac9d79b78bcf4affa48404f5cca3c7c00ad2db02e1f47f78e094a5f76

Author: ValuedMammal

crates/chain/src/chain_data.rs

@@ -1,4 +1,3 @@
-use crate::ConfirmationBlockTime;
 use bitcoin::{OutPoint, TxOut, Txid};
 
 use crate::{Anchor, COINBASE_MATURITY};
@@ -7,6 +6,14 @@ use crate::{Anchor, COINBASE_MATURITY};
 ///
 /// The generic `A` should be a [`Anchor`] implementation.
 #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, core::hash::Hash)]
+#[cfg_attr(
+    feature = "serde",
+    derive(serde::Deserialize, serde::Serialize),
+    serde(bound(
+        deserialize = "A: Ord + serde::Deserialize<'de>",
+        serialize = "A: Ord + serde::Serialize",
+    ))
+)]
 pub enum ChainPosition<A> {
     /// The chain data is seen as confirmed, and in anchored by `A`.
     Confirmed(A),
@@ -41,48 +48,6 @@ impl<A: Anchor> ChainPosition<A> {
     }
 }
 
-/// Block height and timestamp at which a transaction is confirmed.
-#[derive(Debug, Clone, PartialEq, Eq, Copy, PartialOrd, Ord, core::hash::Hash)]
-#[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
-pub enum ConfirmationTime {
-    /// The transaction is confirmed
-    Confirmed {
-        /// Confirmation height.
-        height: u32,
-        /// Confirmation time in unix seconds.
-        time: u64,
-    },
-    /// The transaction is unconfirmed
-    Unconfirmed {
-        /// The last-seen timestamp in unix seconds.
-        last_seen: u64,
-    },
-}
-
-impl ConfirmationTime {
-    /// Construct an unconfirmed variant using the given `last_seen` time in unix seconds.
-    pub fn unconfirmed(last_seen: u64) -> Self {
-        Self::Unconfirmed { last_seen }
-    }
-
-    /// Returns whether [`ConfirmationTime`] is the confirmed variant.
-    pub fn is_confirmed(&self) -> bool {
-        matches!(self, Self::Confirmed { .. })
-    }
-}
-
-impl From<ChainPosition<ConfirmationBlockTime>> for ConfirmationTime {
-    fn from(observed_as: ChainPosition<ConfirmationBlockTime>) -> Self {
-        match observed_as {
-            ChainPosition::Confirmed(a) => Self::Confirmed {
-                height: a.block_id.height,
-                time: a.confirmation_time,
-            },
-            ChainPosition::Unconfirmed(last_seen) => Self::Unconfirmed { last_seen },
-        }
-    }
-}
-
 /// A `TxOut` with as much data as we can retrieve about it
 #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
 pub struct FullTxOut<A> {
@@ -159,6 +124,8 @@ impl<A: Anchor> FullTxOut<A> {
 
 #[cfg(test)]
 mod test {
+    use bdk_core::ConfirmationBlockTime;
+
     use crate::BlockId;
 
     use super::*;

crates/wallet/src/types.rs

@@ -10,9 +10,9 @@
 // licenses.
 
 use alloc::boxed::Box;
+use chain::{ChainPosition, ConfirmationBlockTime};
 use core::convert::AsRef;
 
-use bdk_chain::ConfirmationTime;
 use bitcoin::transaction::{OutPoint, Sequence, TxOut};
 use bitcoin::{psbt, Weight};
 
@@ -61,8 +61,8 @@ pub struct LocalOutput {
     pub is_spent: bool,
     /// The derivation index for the script pubkey in the wallet
     pub derivation_index: u32,
-    /// The confirmation time for transaction containing this utxo
-    pub confirmation_time: ConfirmationTime,
+    /// The position of the output in the blockchain.
+    pub chain_position: ChainPosition<ConfirmationBlockTime>,
 }
 
 /// A [`Utxo`] with its `satisfaction_weight`.

crates/wallet/src/wallet/coin_selection.rs

@@ -278,7 +278,7 @@ impl CoinSelectionAlgorithm for OldestFirstCoinSelection {
         // For utxo that doesn't exist in DB, they will have lowest priority to be selected
         let utxos = {
             optional_utxos.sort_unstable_by_key(|wu| match &wu.utxo {
-                Utxo::Local(local) => Some(local.confirmation_time),
+                Utxo::Local(local) => Some(local.chain_position),
                 Utxo::Foreign { .. } => None,
             });
 
@@ -733,11 +733,12 @@ where
 #[cfg(test)]
 mod test {
     use assert_matches::assert_matches;
+    use bitcoin::hashes::Hash;
+    use chain::{BlockId, ChainPosition, ConfirmationBlockTime};
     use core::str::FromStr;
     use rand::rngs::StdRng;
 
-    use bdk_chain::ConfirmationTime;
-    use bitcoin::{Amount, ScriptBuf, TxIn, TxOut};
+    use bitcoin::{Amount, BlockHash, ScriptBuf, TxIn, TxOut};
 
     use super::*;
     use crate::types::*;
@@ -752,7 +753,34 @@ mod test {
 
     const FEE_AMOUNT: u64 = 50;
 
-    fn utxo(value: u64, index: u32, confirmation_time: ConfirmationTime) -> WeightedUtxo {
+    fn unconfirmed_utxo(value: u64, index: u32, last_seen: u64) -> WeightedUtxo {
+        utxo(value, index, ChainPosition::Unconfirmed(last_seen))
+    }
+
+    fn confirmed_utxo(
+        value: u64,
+        index: u32,
+        confirmation_height: u32,
+        confirmation_time: u64,
+    ) -> WeightedUtxo {
+        utxo(
+            value,
+            index,
+            ChainPosition::Confirmed(ConfirmationBlockTime {
+                block_id: chain::BlockId {
+                    height: confirmation_height,
+                    hash: bitcoin::BlockHash::all_zeros(),
+                },
+                confirmation_time,
+            }),
+        )
+    }
+
+    fn utxo(
+        value: u64,
+        index: u32,
+        chain_position: ChainPosition<ConfirmationBlockTime>,
+    ) -> WeightedUtxo {
         assert!(index < 10);
         let outpoint = OutPoint::from_str(&format!(
             "000000000000000000000000000000000000000000000000000000000000000{}:0",
@@ -770,49 +798,24 @@ mod test {
                 keychain: KeychainKind::External,
                 is_spent: false,
                 derivation_index: 42,
-                confirmation_time,
+                chain_position,
             }),
         }
     }
 
     fn get_test_utxos() -> Vec<WeightedUtxo> {
         vec![
-            utxo(100_000, 0, ConfirmationTime::Unconfirmed { last_seen: 0 }),
-            utxo(
-                FEE_AMOUNT - 40,
-                1,
-                ConfirmationTime::Unconfirmed { last_seen: 0 },
-            ),
-            utxo(200_000, 2, ConfirmationTime::Unconfirmed { last_seen: 0 }),
+            unconfirmed_utxo(100_000, 0, 0),
+            unconfirmed_utxo(FEE_AMOUNT - 40, 1, 0),
+            unconfirmed_utxo(200_000, 2, 0),
         ]
     }
 
     fn get_oldest_first_test_utxos() -> Vec<WeightedUtxo> {
         // ensure utxos are from different tx
-        let utxo1 = utxo(
-            120_000,
-            1,
-            ConfirmationTime::Confirmed {
-                height: 1,
-                time: 1231006505,
-            },
-        );
-        let utxo2 = utxo(
-            80_000,
-            2,
-            ConfirmationTime::Confirmed {
-                height: 2,
-                time: 1231006505,
-            },
-        );
-        let utxo3 = utxo(
-            300_000,
-            3,
-            ConfirmationTime::Confirmed {
-                height: 3,
-                time: 1231006505,
-            },
-        );
+        let utxo1 = confirmed_utxo(120_000, 1, 1, 1231006505);
+        let utxo2 = confirmed_utxo(80_000, 2, 2, 1231006505);
+        let utxo3 = confirmed_utxo(300_000, 3, 3, 1231006505);
         vec![utxo1, utxo2, utxo3]
     }
 
@@ -834,13 +837,16 @@ mod test {
                     keychain: KeychainKind::External,
                     is_spent: false,
                     derivation_index: rng.next_u32(),
-                    confirmation_time: if rng.gen_bool(0.5) {
-                        ConfirmationTime::Confirmed {
-                            height: rng.next_u32(),
-                            time: rng.next_u64(),
-                        }
+                    chain_position: if rng.gen_bool(0.5) {
+                        ChainPosition::Confirmed(ConfirmationBlockTime {
+                            block_id: chain::BlockId {
+                                height: rng.next_u32(),
+                                hash: BlockHash::all_zeros(),
+                            },
+                            confirmation_time: rng.next_u64(),
+                        })
                     } else {
-                        ConfirmationTime::Unconfirmed { last_seen: 0 }
+                        ChainPosition::Unconfirmed(0)
                     },
                 }),
             });
@@ -865,7 +871,7 @@ mod test {
                     keychain: KeychainKind::External,
                     is_spent: false,
                     derivation_index: 42,
-                    confirmation_time: ConfirmationTime::Unconfirmed { last_seen: 0 },
+                    chain_position: ChainPosition::Unconfirmed(0),
                 }),
             })
             .collect()
@@ -1222,7 +1228,7 @@ mod test {
         optional.push(utxo(
             500_000,
             3,
-            ConfirmationTime::Unconfirmed { last_seen: 0 },
+            ChainPosition::<ConfirmationBlockTime>::Unconfirmed(0),
         ));
 
         // Defensive assertions, for sanity and in case someone changes the test utxos vector.
@@ -1584,10 +1590,13 @@ mod test {
                     keychain: KeychainKind::External,
                     is_spent: false,
                     derivation_index: 0,
-                    confirmation_time: ConfirmationTime::Confirmed {
-                        height: 12345,
-                        time: 12345,
-                    },
+                    chain_position: ChainPosition::Confirmed(ConfirmationBlockTime {
+                        block_id: BlockId {
+                            height: 12345,
+                            hash: BlockHash::all_zeros(),
+                        },
+                        confirmation_time: 12345,
+                    }),
                 }),
             }
         }

crates/wallet/src/wallet/mod.rs

@@ -34,8 +34,8 @@ use bdk_chain::{
         SyncResult,
     },
     tx_graph::{CanonicalTx, TxGraph, TxNode, TxUpdate},
-    BlockId, ChainPosition, ConfirmationBlockTime, ConfirmationTime, DescriptorExt, FullTxOut,
-    Indexed, IndexedTxGraph, Merge,
+    BlockId, ChainPosition, ConfirmationBlockTime, DescriptorExt, FullTxOut, Indexed,
+    IndexedTxGraph, Merge,
 };
 use bitcoin::sighash::{EcdsaSighashType, TapSighashType};
 use bitcoin::{
@@ -1660,11 +1660,10 @@ impl Wallet {
                     .ok_or(BuildFeeBumpError::UnknownUtxo(txin.previous_output))?;
                 let txout = &prev_tx.output[txin.previous_output.vout as usize];
 
-                let confirmation_time: ConfirmationTime = graph
+                let chain_position = graph
                     .get_chain_position(&self.chain, chain_tip, txin.previous_output.txid)
                     .ok_or(BuildFeeBumpError::UnknownUtxo(txin.previous_output))?
-                    .cloned()
-                    .into();
+                    .cloned();
 
                 let weighted_utxo = match txout_index.index_of_spk(txout.script_pubkey.clone()) {
                     Some(&(keychain, derivation_index)) => {
@@ -1679,7 +1678,7 @@ impl Wallet {
                                 keychain,
                                 is_spent: true,
                                 derivation_index,
-                                confirmation_time,
+                                chain_position,
                             }),
                             satisfaction_weight,
                         }
@@ -2051,33 +2050,33 @@ impl Wallet {
                     Some(tx) => tx,
                     None => return false,
                 };
-                let confirmation_time: ConfirmationTime = match self
-                    .indexed_graph
-                    .graph()
-                    .get_chain_position(&self.chain, chain_tip, txid)
-                {
-                    Some(chain_position) => chain_position.cloned().into(),
+                let chain_position = match self.indexed_graph.graph().get_chain_position(
+                    &self.chain,
+                    chain_tip,
+                    txid,
+                ) {
+                    Some(chain_position) => chain_position.cloned(),
                     None => return false,
                 };
 
                 // Whether the UTXO is mature and, if needed, confirmed
                 let mut spendable = true;
-                if must_only_use_confirmed_tx && !confirmation_time.is_confirmed() {
+                if must_only_use_confirmed_tx && !chain_position.is_confirmed() {
                     return false;
                 }
                 if tx.is_coinbase() {
                     debug_assert!(
-                        confirmation_time.is_confirmed(),
+                        chain_position.is_confirmed(),
                         "coinbase must always be confirmed"
                     );
                     if let Some(current_height) = current_height {
-                        match confirmation_time {
-                            ConfirmationTime::Confirmed { height, .. } => {
+                        match chain_position {
+                            ChainPosition::Confirmed(a) => {
                                 // https://github.com/bitcoin/bitcoin/blob/c5e67be03bb06a5d7885c55db1f016fbf2333fe3/src/validation.cpp#L373-L375
-                                spendable &=
-                                    (current_height.saturating_sub(height)) >= COINBASE_MATURITY;
+                                spendable &= (current_height.saturating_sub(a.block_id.height))
+                                    >= COINBASE_MATURITY;
                             }
-                            ConfirmationTime::Unconfirmed { .. } => spendable = false,
+                            ChainPosition::Unconfirmed { .. } => spendable = false,
                         }
                     }
                 }
@@ -2546,7 +2545,7 @@ fn new_local_utxo(
         outpoint: full_txo.outpoint,
         txout: full_txo.txout,
         is_spent: full_txo.spent_by.is_some(),
-        confirmation_time: full_txo.chain_position.into(),
+        chain_position: full_txo.chain_position,
         keychain,
         derivation_index,
     }