From 758a4d6578aba487ae6ea06d32ece247525aba8b Mon Sep 17 00:00:00 2001
From: Antoine Poinsot <darosior@protonmail.com>
Date: Tue, 21 Nov 2023 11:13:51 +0100
Subject: [PATCH] bitcoin: poller: fix spend expiration from mempool, again

Always mark a spend dropped from mempool as expired if it, or a conflict spending this coin, wasn't mined. The previous logic was confused: it would only do so if a conflict was detected, but it's not the only reason for a tx to be dropped from the mempool.
---
 src/bitcoin/mod.rs | 47 +++++++++++++++++-----------------------------
 1 file changed, 17 insertions(+), 30 deletions(-)

diff --git a/src/bitcoin/mod.rs b/src/bitcoin/mod.rs
index f9369d0e..bfed816a 100644
--- a/src/bitcoin/mod.rs
+++ b/src/bitcoin/mod.rs
@@ -269,44 +269,31 @@ impl BitcoinInterface for d::BitcoinD {
 
             // If a conflicting transaction was confirmed instead, replace the txid of the
             // spender for this coin with it and mark it as confirmed.
-            // If a conflicting transaction which doesn't spend this coin was mined or accepted in
-            // our local mempool, mark this spend as expired.
-            enum Conflict {
-                // A replacement spending transaction was confirmed.
-                Replaced((bitcoin::Txid, Block)),
-                // A transaction conflicting with the former spending transaction was confirmed or
-                // included in our local mempool.
-                Dropped,
-            }
             let conflict = res.conflicting_txs.iter().find_map(|txid| {
                 tx_getter.get_transaction(txid).and_then(|tx| {
-                    tx.block
-                        .map(|block| {
-                            // Being part of our watchonly wallet isn't enough, as it could be a
-                            // conflicting transaction which spends a different set of coins. Make sure
-                            // it does actually spend this coin.
-                            for txin in tx.tx.input {
-                                if &txin.previous_output == op {
-                                    return Conflict::Replaced((*txid, block));
-                                }
-                            }
-                            Conflict::Dropped
-                        })
-                        .or_else(|| {
-                            // If the coin is actually being spent, but by another transaction, it
-                            // will just be set at the next poll in `spending_coins()`.
-                            if self.is_in_mempool(txid) {
-                                Some(Conflict::Dropped)
+                    tx.block.and_then(|block| {
+                        // Being part of our watchonly wallet isn't enough, as it could be a
+                        // conflicting transaction which spends a different set of coins. Make sure
+                        // it does actually spend this coin.
+                        tx.tx.input.iter().find_map(|txin| {
+                            if &txin.previous_output == op {
+                                Some((*txid, block))
                             } else {
                                 None
                             }
                         })
+                    })
                 })
             });
-            match conflict {
-                Some(Conflict::Replaced((txid, block))) => spent.push((*op, txid, block)),
-                Some(Conflict::Dropped) => expired.push(*op),
-                None => {}
+            if let Some((txid, block)) = conflict {
+                spent.push((*op, txid, block));
+                continue;
+            }
+
+            // If the transaction was not confirmed, a conflicting transaction spending this coin
+            // too wasn't mined, but still isn't in our mempool anymore, mark the spend as expired.
+            if !self.is_in_mempool(txid) {
+                expired.push(*op);
             }
         }