From 7810059ed0f23cae7dee61fe587a1c8f3f49480a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= <hello@evanlinjin.me>
Date: Fri, 31 Mar 2023 14:15:34 +0800
Subject: [PATCH] [bdk_chain_redesign] `TxGraph` tweaks

* Rename `TxNode::last_seen` to `last_seen_unconfirmed` and improve docs
* Improve `try_get_chain_position` logic and tweak comments
---
 crates/chain/src/tx_graph.rs | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/crates/chain/src/tx_graph.rs b/crates/chain/src/tx_graph.rs
index 3aca6a6f..bbdd7bdb 100644
--- a/crates/chain/src/tx_graph.rs
+++ b/crates/chain/src/tx_graph.rs
@@ -103,8 +103,8 @@ pub struct TxNode<'a, T, A> {
     pub tx: &'a T,
     /// The blocks that the transaction is "anchored" in.
     pub anchors: &'a BTreeSet<A>,
-    /// The last-seen unix timestamp of the transaction.
-    pub last_seen: u64,
+    /// The last-seen unix timestamp of the transaction as unconfirmed.
+    pub last_seen_unconfirmed: u64,
 }
 
 impl<'a, T, A> Deref for TxNode<'a, T, A> {
@@ -121,7 +121,7 @@ impl<'a, A> TxNode<'a, Transaction, A> {
             txid: tx.txid(),
             tx,
             anchors,
-            last_seen: 0,
+            last_seen_unconfirmed: 0,
         }
     }
 }
@@ -168,7 +168,7 @@ impl<A> TxGraph<A> {
                     txid,
                     tx,
                     anchors,
-                    last_seen: *last_seen,
+                    last_seen_unconfirmed: *last_seen,
                 }),
                 TxNodeInternal::Partial(_) => None,
             })
@@ -190,7 +190,7 @@ impl<A> TxGraph<A> {
                 txid,
                 tx,
                 anchors,
-                last_seen: *last_seen,
+                last_seen_unconfirmed: *last_seen,
             }),
             _ => None,
         }
@@ -290,7 +290,7 @@ impl<A> TxGraph<A> {
                     txid,
                     tx: partial,
                     anchors,
-                    last_seen: *last_seen,
+                    last_seen_unconfirmed: *last_seen,
                 }),
             })
     }
@@ -628,8 +628,8 @@ impl<A: BlockAnchor> TxGraph<A> {
             }
         };
 
-        // [TODO] Is this logic correct? I do not think so, but it should be good enough for now!
-        let mut latest_last_seen = 0_u64;
+        // If a conflicting tx is in the best chain, or has `last_seen` higher than this tx, then
+        // this tx cannot exist in the best chain
         for conflicting_tx in self.walk_conflicts(tx, |_, txid| self.get_tx_node(txid)) {
             for block_id in conflicting_tx.anchors.iter().map(A::anchor_block) {
                 if chain.is_block_in_best_chain(block_id)? {
@@ -637,15 +637,12 @@ impl<A: BlockAnchor> TxGraph<A> {
                     return Ok(None);
                 }
             }
-            if conflicting_tx.last_seen > latest_last_seen {
-                latest_last_seen = conflicting_tx.last_seen;
+            if conflicting_tx.last_seen_unconfirmed > last_seen {
+                return Ok(None);
             }
         }
-        if last_seen >= latest_last_seen {
-            Ok(Some(ObservedIn::Mempool(last_seen)))
-        } else {
-            Ok(None)
-        }
+
+        Ok(Some(ObservedIn::Mempool(last_seen)))
     }
 
     pub fn get_chain_position<C>(&self, chain: C, txid: Txid) -> Option<ObservedIn<&A>>