Merge #1385: Fix last seen unconfirmed

a2a64ff fix(electrum)!: Remove `seen_at` param from `into_tx_graph` (valued mammal)
37fca35 feat(tx_graph): Add method update_last_seen_unconfirmed (valued mammal)

Pull request description:

  A method `update_last_seen_unconfirmed` is added for `TxGraph` that allows inserting a `seen_at` time for all unanchored transactions.

  fixes #1336

  ### Changelog notice

  Changed:
  - Methods `into_tx_graph` and `into_confirmation_time_tx_graph`for `RelevantTxids` are changed to no longer accept a `seen_at` parameter.

  Added:
  - Added method `update_last_seen_unconfirmed` for `TxGraph`.

  ### 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:
  evanlinjin:
    ACK a2a64ff

Tree-SHA512: 9011e63314b0e3ffcd50dbf5024f82b365bab1cc834c0455d7410b682338339ed5284caa721ffc267c65fa26d35ff6ee86cde6052e82a6a79768547fbb7a45eb

Author: evanlinjin

crates/chain/src/tx_graph.rs

@@ -554,14 +554,77 @@ impl<A: Clone + Ord> TxGraph<A> {
 
     /// Inserts the given `seen_at` for `txid` into [`TxGraph`].
     ///
-    /// Note that [`TxGraph`] only keeps track of the latest `seen_at`.
+    /// Note that [`TxGraph`] only keeps track of the latest `seen_at`. To batch
+    /// update all unconfirmed transactions with the latest `seen_at`, see
+    /// [`update_last_seen_unconfirmed`].
+    ///
+    /// [`update_last_seen_unconfirmed`]: Self::update_last_seen_unconfirmed
     pub fn insert_seen_at(&mut self, txid: Txid, seen_at: u64) -> ChangeSet<A> {
         let mut update = Self::default();
         let (_, _, update_last_seen) = update.txs.entry(txid).or_default();
         *update_last_seen = seen_at;
         self.apply_update(update)
     }
 
+    /// Update the last seen time for all unconfirmed transactions.
+    ///
+    /// This method updates the last seen unconfirmed time for this [`TxGraph`] by inserting
+    /// the given `seen_at` for every transaction not yet anchored to a confirmed block,
+    /// and returns the [`ChangeSet`] after applying all updates to `self`.
+    ///
+    /// This is useful for keeping track of the latest time a transaction was seen
+    /// unconfirmed, which is important for evaluating transaction conflicts in the same
+    /// [`TxGraph`]. For details of how [`TxGraph`] resolves conflicts, see the docs for
+    /// [`try_get_chain_position`].
+    ///
+    /// A normal use of this method is to call it with the current system time. Although
+    /// block headers contain a timestamp, using the header time would be less effective
+    /// at tracking mempool transactions, because it can drift from actual clock time, plus
+    /// we may want to update a transaction's last seen time repeatedly between blocks.
+    ///
+    /// # Example
+    ///
+    /// ```rust
+    /// # use bdk_chain::example_utils::*;
+    /// # use std::time::UNIX_EPOCH;
+    /// # let tx = tx_from_hex(RAW_TX_1);
+    /// # let mut tx_graph = bdk_chain::TxGraph::<()>::new([tx]);
+    /// let now = std::time::SystemTime::now()
+    ///     .duration_since(UNIX_EPOCH)
+    ///     .expect("valid duration")
+    ///     .as_secs();
+    /// let changeset = tx_graph.update_last_seen_unconfirmed(now);
+    /// assert!(!changeset.last_seen.is_empty());
+    /// ```
+    ///
+    /// Note that [`TxGraph`] only keeps track of the latest `seen_at`, so the given time must
+    /// by strictly greater than what is currently stored for a transaction to have an effect.
+    /// To insert a last seen time for a single txid, see [`insert_seen_at`].
+    ///
+    /// [`insert_seen_at`]: Self::insert_seen_at
+    /// [`try_get_chain_position`]: Self::try_get_chain_position
+    pub fn update_last_seen_unconfirmed(&mut self, seen_at: u64) -> ChangeSet<A> {
+        let mut changeset = ChangeSet::default();
+        let unanchored_txs: Vec<Txid> = self
+            .txs
+            .iter()
+            .filter_map(
+                |(&txid, (_, anchors, _))| {
+                    if anchors.is_empty() {
+                        Some(txid)
+                    } else {
+                        None
+                    }
+                },
+            )
+            .collect();
+
+        for txid in unanchored_txs {
+            changeset.append(self.insert_seen_at(txid, seen_at));
+        }
+        changeset
+    }
+
     /// Extends this graph with another so that `self` becomes the union of the two sets of
     /// transactions.
     ///

crates/chain/tests/test_tx_graph.rs

@@ -1058,6 +1058,34 @@ fn test_changeset_last_seen_append() {
     }
 }
 
+#[test]
+fn update_last_seen_unconfirmed() {
+    let mut graph = TxGraph::<()>::default();
+    let tx = new_tx(0);
+    let txid = tx.txid();
+
+    // insert a new tx
+    // initially we have a last_seen of 0, and no anchors
+    let _ = graph.insert_tx(tx);
+    let tx = graph.full_txs().next().unwrap();
+    assert_eq!(tx.last_seen_unconfirmed, 0);
+    assert!(tx.anchors.is_empty());
+
+    // higher timestamp should update last seen
+    let changeset = graph.update_last_seen_unconfirmed(2);
+    assert_eq!(changeset.last_seen.get(&txid).unwrap(), &2);
+
+    // lower timestamp has no effect
+    let changeset = graph.update_last_seen_unconfirmed(1);
+    assert!(changeset.last_seen.is_empty());
+
+    // once anchored, last seen is not updated
+    let _ = graph.insert_anchor(txid, ());
+    let changeset = graph.update_last_seen_unconfirmed(4);
+    assert!(changeset.is_empty());
+    assert_eq!(graph.full_txs().next().unwrap().last_seen_unconfirmed, 2);
+}
+
 #[test]
 fn test_missing_blocks() {
     /// An anchor implementation for testing, made up of `(the_anchor_block, random_data)`.

crates/electrum/src/electrum_ext.rs

@@ -40,15 +40,11 @@ impl RelevantTxids {
     pub fn into_tx_graph(
         self,
         client: &Client,
-        seen_at: Option<u64>,
         missing: Vec<Txid>,
     ) -> Result<TxGraph<ConfirmationHeightAnchor>, Error> {
         let new_txs = client.batch_transaction_get(&missing)?;
         let mut graph = TxGraph::<ConfirmationHeightAnchor>::new(new_txs);
         for (txid, anchors) in self.0 {
-            if let Some(seen_at) = seen_at {
-                let _ = graph.insert_seen_at(txid, seen_at);
-            }
             for anchor in anchors {
                 let _ = graph.insert_anchor(txid, anchor);
             }
@@ -67,10 +63,9 @@ impl RelevantTxids {
     pub fn into_confirmation_time_tx_graph(
         self,
         client: &Client,
-        seen_at: Option<u64>,
         missing: Vec<Txid>,
     ) -> Result<TxGraph<ConfirmationTimeHeightAnchor>, Error> {
-        let graph = self.into_tx_graph(client, seen_at, missing)?;
+        let graph = self.into_tx_graph(client, missing)?;
 
         let relevant_heights = {
             let mut visited_heights = HashSet::new();

crates/electrum/tests/test_electrum.rs

@@ -68,7 +68,7 @@ fn scan_detects_confirmed_tx() -> Result<()> {
     } = client.sync(recv_chain.tip(), [spk_to_track], None, None, 5)?;
 
     let missing = relevant_txids.missing_full_txs(recv_graph.graph());
-    let graph_update = relevant_txids.into_confirmation_time_tx_graph(&client, None, missing)?;
+    let graph_update = relevant_txids.into_confirmation_time_tx_graph(&client, missing)?;
     let _ = recv_chain
         .apply_update(chain_update)
         .map_err(|err| anyhow::anyhow!("LocalChain update error: {:?}", err))?;
@@ -134,7 +134,7 @@ fn tx_can_become_unconfirmed_after_reorg() -> Result<()> {
     } = client.sync(recv_chain.tip(), [spk_to_track.clone()], None, None, 5)?;
 
     let missing = relevant_txids.missing_full_txs(recv_graph.graph());
-    let graph_update = relevant_txids.into_confirmation_time_tx_graph(&client, None, missing)?;
+    let graph_update = relevant_txids.into_confirmation_time_tx_graph(&client, missing)?;
     let _ = recv_chain
         .apply_update(chain_update)
         .map_err(|err| anyhow::anyhow!("LocalChain update error: {:?}", err))?;
@@ -164,8 +164,7 @@ fn tx_can_become_unconfirmed_after_reorg() -> Result<()> {
         } = client.sync(recv_chain.tip(), [spk_to_track.clone()], None, None, 5)?;
 
         let missing = relevant_txids.missing_full_txs(recv_graph.graph());
-        let graph_update =
-            relevant_txids.into_confirmation_time_tx_graph(&client, None, missing)?;
+        let graph_update = relevant_txids.into_confirmation_time_tx_graph(&client, missing)?;
         let _ = recv_chain
             .apply_update(chain_update)
             .map_err(|err| anyhow::anyhow!("LocalChain update error: {:?}", err))?;

example-crates/example_electrum/src/main.rs

@@ -299,12 +299,12 @@ fn main() -> anyhow::Result<()> {
         relevant_txids.missing_full_txs(graph.graph())
     };
 
+    let mut graph_update = relevant_txids.into_tx_graph(&client, missing_txids)?;
     let now = std::time::UNIX_EPOCH
         .elapsed()
         .expect("must get time")
         .as_secs();
-
-    let graph_update = relevant_txids.into_tx_graph(&client, Some(now), missing_txids)?;
+    let _ = graph_update.update_last_seen_unconfirmed(now);
 
     let db_changeset = {
         let mut chain = chain.lock().unwrap();

example-crates/example_esplora/src/main.rs

@@ -189,10 +189,14 @@ fn main() -> anyhow::Result<()> {
             // is reached. It returns a `TxGraph` update (`graph_update`) and a structure that
             // represents the last active spk derivation indices of keychains
             // (`keychain_indices_update`).
-            let (graph_update, last_active_indices) = client
+            let (mut graph_update, last_active_indices) = client
                 .full_scan(keychain_spks, *stop_gap, scan_options.parallel_requests)
                 .context("scanning for transactions")?;
 
+            // We want to keep track of the latest time a transaction was seen unconfirmed.
+            let now = std::time::UNIX_EPOCH.elapsed().unwrap().as_secs();
+            let _ = graph_update.update_last_seen_unconfirmed(now);
+
             let mut graph = graph.lock().expect("mutex must not be poisoned");
             // Because we did a stop gap based scan we are likely to have some updates to our
             // deriviation indices. Usually before a scan you are on a fresh wallet with no
@@ -307,9 +311,13 @@ fn main() -> anyhow::Result<()> {
                 }
             }
 
-            let graph_update =
+            let mut graph_update =
                 client.sync(spks, txids, outpoints, scan_options.parallel_requests)?;
 
+            // Update last seen unconfirmed
+            let now = std::time::UNIX_EPOCH.elapsed().unwrap().as_secs();
+            let _ = graph_update.update_last_seen_unconfirmed(now);
+
             graph.lock().unwrap().apply_update(graph_update)
         }
     };

example-crates/wallet_electrum/src/main.rs

@@ -66,7 +66,9 @@ fn main() -> Result<(), anyhow::Error> {
     println!();
 
     let missing = relevant_txids.missing_full_txs(wallet.as_ref());
-    let graph_update = relevant_txids.into_confirmation_time_tx_graph(&client, None, missing)?;
+    let mut graph_update = relevant_txids.into_confirmation_time_tx_graph(&client, missing)?;
+    let now = std::time::UNIX_EPOCH.elapsed().unwrap().as_secs();
+    let _ = graph_update.update_last_seen_unconfirmed(now);
 
     let wallet_update = Update {
         last_active_indices: keychain_update,

example-crates/wallet_esplora_async/src/main.rs

@@ -53,9 +53,12 @@ async fn main() -> Result<(), anyhow::Error> {
             (k, k_spks)
         })
         .collect();
-    let (update_graph, last_active_indices) = client
+    let (mut update_graph, last_active_indices) = client
         .full_scan(keychain_spks, STOP_GAP, PARALLEL_REQUESTS)
         .await?;
+
+    let now = std::time::UNIX_EPOCH.elapsed().unwrap().as_secs();
+    let _ = update_graph.update_last_seen_unconfirmed(now);
     let missing_heights = update_graph.missing_heights(wallet.local_chain());
     let chain_update = client.update_local_chain(prev_tip, missing_heights).await?;
     let update = Update {

example-crates/wallet_esplora_blocking/src/main.rs

@@ -53,8 +53,11 @@ fn main() -> Result<(), anyhow::Error> {
         })
         .collect();
 
-    let (update_graph, last_active_indices) =
+    let (mut update_graph, last_active_indices) =
         client.full_scan(keychain_spks, STOP_GAP, PARALLEL_REQUESTS)?;
+
+    let now = std::time::UNIX_EPOCH.elapsed().unwrap().as_secs();
+    let _ = update_graph.update_last_seen_unconfirmed(now);
     let missing_heights = update_graph.missing_heights(wallet.local_chain());
     let chain_update = client.update_local_chain(prev_tip, missing_heights)?;
     let update = Update {