Merge #1838: Make full-scan/sync flow easier to reason about.

800f358 feat!: Improve spk-based syncing flow (志宇)
ee52745 feat(core)!: Make `TxUpdate` non-exhaustive (志宇)

Pull request description:

  ### Description

  #1811 introduces the `evicted-at` concept. While adding this to `TxUpdate`, I realized there were some shortcomings in our full-scan & sync flow:

  * Chain sources that use `TxUpdate` to convey updates cannot choose to add transactions without a `seen_at` timestamp as the `TxGraph::apply_update_at` logic adds this timestamp for all unanchored txs if the `seen_at` parameter is specified. Purposefully adding uncanonical txs is useful for wallets that want to know about replaced txs.
  * The `TxGraph::apply_update_at` logic is hard to reason about. `TxUpdate::seen_ats` already has the `seen_at` timestamp per tx, but `apply_update_at()` also takes in a `seen_at` timestamp`.
  * `TxUpdate::seen_ats` currently forces us to only have one `seen-at` per tx as it's a map of `txid -> seen_at`. However, in the future we want to add a `first-seen` timestamp to `TxGraph` which is basically the earliest `seen-at` timestamp introduced so we may want to have multiple `seen_at`s per tx. The other issue is if we merge `TxUpdate`s, we will loose data. I.e. we can only keep the `last_seen` or the `first_seen`.

  These problems were exacerbated when introducing `evicted-at`. In the old design, the chain-source has no concept of sync time (as sync time was introduced after-the-fact when applying the `TxUpdate`). Therefore the introduced `TxUpdate::evicted` field was a `HashSet<Txid>` (no timestamps) and relied on the `TxGraph::apply_upate_at` logic to introduce the `evicted-at` timestamp. All this makes the sync logic harder to understand. What happens if the `seen_at` input is `None`? What happens if updates were applied out of order? What happens when we merge `TxUpdates` before applying?

  The following changes are made in this PR to simplify the sync/full-scan logic to be easier to understand and robust:

  * The `sync_time` is added to the `SyncRequest` and `FullScanRequest`. `sync_time` is mandatory and is added as an input of `builder()`. If the `std` feature is enabled, the `builder_now()` is available which uses the current timestamp. The chain source is now responsible to add this `sync_time` timestamp as `seen_at` for mempool txs. Non-canonical and non-anchored txs can be added without the `seen_at` timestamp.
  * `TxUpdate::seen_ats` is now a `HashSet` of `(Txid, u64)`. This allows multiple `seen_at`s per tx. This is also a much easier to use API as the chain source can just insert into this `HashSet` without checking previous data.
  * `TxGraph::apply_update_at` is removed as we no longer needs to introduce a fallback `seen_at` timestamp after-the-fact. `TxGraph::apply_update` is no longer `std`-only and the logic of applying updates is greatly simplified.

  Additionally, `TxUpdate` is updated to be `#[non_exhaustive]` for backwards-compatibility protection.

  ### Notes to the reviewers

  This is based on #1837. Merge that first.

  These are breaking changes to `bdk_core`. It needs to be breaking to properly fix all the issues.

  As mentioned by @notmandatory, `bdk_wallet` changes will be added to a new PR once the new `bdk_wallet` repo is ready. But the PR won't be merged until we're ready for a `bdk_wallet` 2.0.

  ### Changelog notice

  * Change `FullScanRequest::builder` and `SyncRequest::builder` methods to depend on `feature = "std"`. This is because requests now have a `start_time`, instead of specifying a `seen_at` when applying the update.
  * Add `FullScanRequest::builder_at` and `SyncRequest::builder_at` methods which are the non-std version of the `..Request::builder` methods.
  * Change `TxUpdate::seen_ats` field to be a `HashSet` of `(Txid, u64)`. This allows a single update to have multiple `seen_at`s per tx.
  * Change `TxUpdate` to be `non-exhaustive`.
  * Remove `apply_update_at` as we no longer need to apply with a timestamp after-the-fact.

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

  ~* [ ] I've added tests for the new feature~ No tests needed as it's more of a refactor.

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

ACKs for top commit:
  notmandatory:
    utACK 800f358

Tree-SHA512: 85af8452ac60c4a8087962403fd10c5c67592d3711f7665ae09a2d9c48a868583a41e54f28d639e47bd264ccf95d9254efc8d0d6248c8bcc9343c8290502e061

Author: evanlinjin

crates/chain/benches/canonicalization.rs

@@ -132,11 +132,10 @@ pub fn many_conflicting_unconfirmed(c: &mut Criterion) {
                 }],
                 ..new_tx(i)
             };
-            let update = TxUpdate {
-                txs: vec![Arc::new(tx)],
-                ..Default::default()
-            };
-            let _ = tx_graph.apply_update_at(update, Some(i as u64));
+            let mut update = TxUpdate::default();
+            update.seen_ats = [(tx.compute_txid(), i as u64)].into();
+            update.txs = vec![Arc::new(tx)];
+            let _ = tx_graph.apply_update(update);
         }
     }));
     c.bench_function("many_conflicting_unconfirmed::list_canonical_txs", {
@@ -169,11 +168,10 @@ pub fn many_chained_unconfirmed(c: &mut Criterion) {
                 ..new_tx(i)
             };
             let txid = tx.compute_txid();
-            let update = TxUpdate {
-                txs: vec![Arc::new(tx)],
-                ..Default::default()
-            };
-            let _ = tx_graph.apply_update_at(update, Some(i as u64));
+            let mut update = TxUpdate::default();
+            update.seen_ats = [(txid, i as u64)].into();
+            update.txs = vec![Arc::new(tx)];
+            let _ = tx_graph.apply_update(update);
             // Store the next prevout.
             previous_output = OutPoint::new(txid, 0);
         }

crates/chain/src/indexed_tx_graph.rs

@@ -91,37 +91,12 @@ where
     /// Apply an `update` directly.
     ///
     /// `update` is a [`tx_graph::TxUpdate<A>`] and the resultant changes is returned as [`ChangeSet`].
-    #[cfg(feature = "std")]
-    #[cfg_attr(docsrs, doc(cfg(feature = "std")))]
     pub fn apply_update(&mut self, update: tx_graph::TxUpdate<A>) -> ChangeSet<A, I::ChangeSet> {
         let tx_graph = self.graph.apply_update(update);
         let indexer = self.index_tx_graph_changeset(&tx_graph);
         ChangeSet { tx_graph, indexer }
     }
 
-    /// Apply the given `update` with an optional `seen_at` timestamp.
-    ///
-    /// `seen_at` represents when the update is seen (in unix seconds). It is used to determine the
-    /// `last_seen`s for all transactions in the update which have no corresponding anchor(s). The
-    /// `last_seen` value is used internally to determine precedence of conflicting unconfirmed
-    /// transactions (where the transaction with the lower `last_seen` value is omitted from the
-    /// canonical history).
-    ///
-    /// Not setting a `seen_at` value means unconfirmed transactions introduced by this update will
-    /// not be part of the canonical history of transactions.
-    ///
-    /// Use [`apply_update`](IndexedTxGraph::apply_update) to have the `seen_at` value automatically
-    /// set to the current time.
-    pub fn apply_update_at(
-        &mut self,
-        update: tx_graph::TxUpdate<A>,
-        seen_at: Option<u64>,
-    ) -> ChangeSet<A, I::ChangeSet> {
-        let tx_graph = self.graph.apply_update_at(update, seen_at);
-        let indexer = self.index_tx_graph_changeset(&tx_graph);
-        ChangeSet { tx_graph, indexer }
-    }
-
     /// Insert a floating `txout` of given `outpoint`.
     pub fn insert_txout(&mut self, outpoint: OutPoint, txout: TxOut) -> ChangeSet<A, I::ChangeSet> {
         let graph = self.graph.insert_txout(outpoint, txout);

crates/chain/src/tx_graph.rs

@@ -110,26 +110,26 @@ use core::{
 
 impl<A: Ord> From<TxGraph<A>> for TxUpdate<A> {
     fn from(graph: TxGraph<A>) -> Self {
-        Self {
-            txs: graph.full_txs().map(|tx_node| tx_node.tx).collect(),
-            txouts: graph
-                .floating_txouts()
-                .map(|(op, txo)| (op, txo.clone()))
-                .collect(),
-            anchors: graph
-                .anchors
-                .into_iter()
-                .flat_map(|(txid, anchors)| anchors.into_iter().map(move |a| (a, txid)))
-                .collect(),
-            seen_ats: graph.last_seen.into_iter().collect(),
-        }
+        let mut tx_update = TxUpdate::default();
+        tx_update.txs = graph.full_txs().map(|tx_node| tx_node.tx).collect();
+        tx_update.txouts = graph
+            .floating_txouts()
+            .map(|(op, txo)| (op, txo.clone()))
+            .collect();
+        tx_update.anchors = graph
+            .anchors
+            .into_iter()
+            .flat_map(|(txid, anchors)| anchors.into_iter().map(move |a| (a, txid)))
+            .collect();
+        tx_update.seen_ats = graph.last_seen.into_iter().collect();
+        tx_update
     }
 }
 
 impl<A: Anchor> From<TxUpdate<A>> for TxGraph<A> {
     fn from(update: TxUpdate<A>) -> Self {
         let mut graph = TxGraph::<A>::default();
-        let _ = graph.apply_update_at(update, None);
+        let _ = graph.apply_update(update);
         graph
     }
 }
@@ -719,52 +719,20 @@ impl<A: Anchor> TxGraph<A> {
     ///
     /// The returned [`ChangeSet`] is the set difference between `update` and `self` (transactions that
     /// exist in `update` but not in `self`).
-    #[cfg(feature = "std")]
-    #[cfg_attr(docsrs, doc(cfg(feature = "std")))]
     pub fn apply_update(&mut self, update: TxUpdate<A>) -> ChangeSet<A> {
-        use std::time::*;
-        let now = SystemTime::now()
-            .duration_since(UNIX_EPOCH)
-            .expect("current time must be greater than epoch anchor");
-        self.apply_update_at(update, Some(now.as_secs()))
-    }
-
-    /// Extends this graph with the given `update` alongside an optional `seen_at` timestamp.
-    ///
-    /// `seen_at` represents when the update is seen (in unix seconds). It is used to determine the
-    /// `last_seen`s for all transactions in the update which have no corresponding anchor(s). The
-    /// `last_seen` value is used internally to determine precedence of conflicting unconfirmed
-    /// transactions (where the transaction with the lower `last_seen` value is omitted from the
-    /// canonical history).
-    ///
-    /// Not setting a `seen_at` value means unconfirmed transactions introduced by this update will
-    /// not be part of the canonical history of transactions.
-    ///
-    /// Use [`apply_update`](TxGraph::apply_update) to have the `seen_at` value automatically set
-    /// to the current time.
-    pub fn apply_update_at(&mut self, update: TxUpdate<A>, seen_at: Option<u64>) -> ChangeSet<A> {
         let mut changeset = ChangeSet::<A>::default();
-        let mut unanchored_txs = HashSet::<Txid>::new();
         for tx in update.txs {
-            if unanchored_txs.insert(tx.compute_txid()) {
-                changeset.merge(self.insert_tx(tx));
-            }
+            changeset.merge(self.insert_tx(tx));
         }
         for (outpoint, txout) in update.txouts {
             changeset.merge(self.insert_txout(outpoint, txout));
         }
         for (anchor, txid) in update.anchors {
-            unanchored_txs.remove(&txid);
             changeset.merge(self.insert_anchor(txid, anchor));
         }
         for (txid, seen_at) in update.seen_ats {
             changeset.merge(self.insert_seen_at(txid, seen_at));
         }
-        if let Some(seen_at) = seen_at {
-            for txid in unanchored_txs {
-                changeset.merge(self.insert_seen_at(txid, seen_at));
-            }
-        }
         changeset
     }
 

crates/chain/tests/test_tx_graph.rs

@@ -94,7 +94,7 @@ fn insert_txouts() {
             // Insert partials transactions.
             update.txouts.insert(*outpoint, txout.clone());
             // Mark them unconfirmed.
-            update.seen_ats.insert(outpoint.txid, unconf_seen_at);
+            update.seen_ats.insert((outpoint.txid, unconf_seen_at));
         }
 
         // Insert the full transaction.
@@ -1231,74 +1231,65 @@ fn tx_graph_update_conversion() {
 
     let test_cases: &[TestCase] = &[
         ("empty_update", TxUpdate::default()),
-        (
-            "single_tx",
-            TxUpdate {
-                txs: vec![make_tx(0).into()],
-                ..Default::default()
-            },
-        ),
-        (
-            "two_txs",
-            TxUpdate {
-                txs: vec![make_tx(0).into(), make_tx(1).into()],
-                ..Default::default()
-            },
-        ),
-        (
-            "with_floating_txouts",
-            TxUpdate {
-                txs: vec![make_tx(0).into(), make_tx(1).into()],
-                txouts: [
-                    (OutPoint::new(hash!("a"), 0), make_txout(0)),
-                    (OutPoint::new(hash!("a"), 1), make_txout(1)),
-                    (OutPoint::new(hash!("b"), 0), make_txout(2)),
-                ]
-                .into(),
-                ..Default::default()
-            },
-        ),
-        (
-            "with_anchors",
-            TxUpdate {
-                txs: vec![make_tx(0).into(), make_tx(1).into()],
-                txouts: [
-                    (OutPoint::new(hash!("a"), 0), make_txout(0)),
-                    (OutPoint::new(hash!("a"), 1), make_txout(1)),
-                    (OutPoint::new(hash!("b"), 0), make_txout(2)),
-                ]
-                .into(),
-                anchors: [
-                    (ConfirmationBlockTime::default(), hash!("a")),
-                    (ConfirmationBlockTime::default(), hash!("b")),
-                ]
-                .into(),
-                ..Default::default()
-            },
-        ),
-        (
-            "with_seen_ats",
-            TxUpdate {
-                txs: vec![make_tx(0).into(), make_tx(1).into()],
-                txouts: [
-                    (OutPoint::new(hash!("a"), 0), make_txout(0)),
-                    (OutPoint::new(hash!("a"), 1), make_txout(1)),
-                    (OutPoint::new(hash!("d"), 0), make_txout(2)),
-                ]
-                .into(),
-                anchors: [
-                    (ConfirmationBlockTime::default(), hash!("a")),
-                    (ConfirmationBlockTime::default(), hash!("b")),
-                ]
-                .into(),
-                seen_ats: [(hash!("c"), 12346)].into_iter().collect(),
-            },
-        ),
+        ("single_tx", {
+            let mut tx_update = TxUpdate::default();
+            tx_update.txs = vec![make_tx(0).into()];
+            tx_update
+        }),
+        ("two_txs", {
+            let mut tx_update = TxUpdate::default();
+            tx_update.txs = vec![make_tx(0).into(), make_tx(1).into()];
+            tx_update
+        }),
+        ("with_floating_txouts", {
+            let mut tx_update = TxUpdate::default();
+            tx_update.txs = vec![make_tx(0).into(), make_tx(1).into()];
+            tx_update.txouts = [
+                (OutPoint::new(hash!("a"), 0), make_txout(0)),
+                (OutPoint::new(hash!("a"), 1), make_txout(1)),
+                (OutPoint::new(hash!("b"), 0), make_txout(2)),
+            ]
+            .into();
+            tx_update
+        }),
+        ("with_anchors", {
+            let mut tx_update = TxUpdate::default();
+            tx_update.txs = vec![make_tx(0).into(), make_tx(1).into()];
+            tx_update.txouts = [
+                (OutPoint::new(hash!("a"), 0), make_txout(0)),
+                (OutPoint::new(hash!("a"), 1), make_txout(1)),
+                (OutPoint::new(hash!("b"), 0), make_txout(2)),
+            ]
+            .into();
+            tx_update.anchors = [
+                (ConfirmationBlockTime::default(), hash!("a")),
+                (ConfirmationBlockTime::default(), hash!("b")),
+            ]
+            .into();
+            tx_update
+        }),
+        ("with_seen_ats", {
+            let mut tx_update = TxUpdate::default();
+            tx_update.txs = vec![make_tx(0).into(), make_tx(1).into()];
+            tx_update.txouts = [
+                (OutPoint::new(hash!("a"), 0), make_txout(0)),
+                (OutPoint::new(hash!("a"), 1), make_txout(1)),
+                (OutPoint::new(hash!("d"), 0), make_txout(2)),
+            ]
+            .into();
+            tx_update.anchors = [
+                (ConfirmationBlockTime::default(), hash!("a")),
+                (ConfirmationBlockTime::default(), hash!("b")),
+            ]
+            .into();
+            tx_update.seen_ats = [(hash!("c"), 12346)].into_iter().collect();
+            tx_update
+        }),
     ];
 
     for (test_name, update) in test_cases {
         let mut tx_graph = TxGraph::<ConfirmationBlockTime>::default();
-        let _ = tx_graph.apply_update_at(update.clone(), None);
+        let _ = tx_graph.apply_update(update.clone());
         let update_from_tx_graph: TxUpdate<ConfirmationBlockTime> = tx_graph.into();
 
         assert_eq!(