bitcoin: track expired unconfirmed deposits, remove them from DB

src/bitcoin/d/mod.rs

@@ -964,6 +964,22 @@ impl BitcoinD {
             |h| self.get_block_stats(h),
         )
     }
+
+    /// Whether this transaction is in the mempool.
+    pub fn is_in_mempool(&self, txid: &bitcoin::Txid) -> bool {
+        match self
+            .make_fallible_node_request("getmempoolentry", &params!(Json::String(txid.to_string())))
+        {
+            Ok(_) => true,
+            Err(BitcoindError::Server(jsonrpc::Error::Rpc(jsonrpc::error::RpcError {
+                code: -5,
+                ..
+            }))) => false,
+            Err(e) => {
+                panic!("Unexpected error returned by bitcoind {}", e);
+            }
+        }
+    }
 }
 
 /// An entry in the 'listdescriptors' result.

src/bitcoin/mod.rs

@@ -58,11 +58,12 @@ pub trait BitcoinInterface: Send {
         descs: &[descriptors::InheritanceDescriptor],
     ) -> Vec<UTxO>;
 
-    /// Get all coins that were confirmed, and at what height and time.
+    /// Get all coins that were confirmed, and at what height and time. Along with "expired"
+    /// unconfirmed coins (for instance whose creating transaction may have been replaced).
     fn confirmed_coins(
         &self,
         outpoints: &[bitcoin::OutPoint],
-    ) -> Vec<(bitcoin::OutPoint, i32, u32)>;
+    ) -> (Vec<(bitcoin::OutPoint, i32, u32)>, Vec<bitcoin::OutPoint>);
 
     /// Get all coins that are being spent, and the spending txid.
     fn spending_coins(
@@ -166,9 +167,10 @@ impl BitcoinInterface for d::BitcoinD {
     fn confirmed_coins(
         &self,
         outpoints: &[bitcoin::OutPoint],
-    ) -> Vec<(bitcoin::OutPoint, i32, u32)> {
-        // The confirmed coins.
+    ) -> (Vec<(bitcoin::OutPoint, i32, u32)>, Vec<bitcoin::OutPoint>) {
+        // The confirmed and expired coins to be returned.
         let mut confirmed = Vec::with_capacity(outpoints.len());
+        let mut expired = Vec::new();
         // Cached calls to `gettransaction`.
         let mut tx_getter = CachedTxGetter::new(self);
 
@@ -185,9 +187,14 @@ impl BitcoinInterface for d::BitcoinD {
                 confirmed.push((*op, block.height, block.time));
                 continue;
             }
+
+            // If the transaction was dropped from the mempool, discard the coin.
+            if !self.is_in_mempool(&op.txid) {
+                expired.push(*op);
+            }
         }
 
-        confirmed
+        (confirmed, expired)
     }
 
     fn spending_coins(
@@ -340,7 +347,7 @@ impl BitcoinInterface for sync::Arc<sync::Mutex<dyn BitcoinInterface + 'static>>
     fn confirmed_coins(
         &self,
         outpoints: &[bitcoin::OutPoint],
-    ) -> Vec<(bitcoin::OutPoint, i32, u32)> {
+    ) -> (Vec<(bitcoin::OutPoint, i32, u32)>, Vec<bitcoin::OutPoint>) {
         self.lock().unwrap().confirmed_coins(outpoints)
     }
 

src/bitcoin/poller/looper.rs

@@ -15,6 +15,7 @@ use miniscript::bitcoin::{self, secp256k1};
 struct UpdatedCoins {
     pub received: Vec<Coin>,
     pub confirmed: Vec<(bitcoin::OutPoint, i32, u32)>,
+    pub expired: Vec<bitcoin::OutPoint>,
     pub spending: Vec<(bitcoin::OutPoint, bitcoin::Txid)>,
     pub spent: Vec<(bitcoin::OutPoint, bitcoin::Txid, i32, u32)>,
 }
@@ -91,8 +92,9 @@ fn update_coins(
             }
         })
         .collect();
-    let confirmed = bit.confirmed_coins(&to_be_confirmed);
+    let (confirmed, expired) = bit.confirmed_coins(&to_be_confirmed);
     log::debug!("Newly confirmed coins: {:?}", confirmed);
+    log::debug!("Expired coins: {:?}", expired);
 
     // We need to take the newly received ones into account as well, as they may have been
     // spent within the previous tip and the current one, and we may not poll this chunk of the
@@ -131,6 +133,7 @@ fn update_coins(
     UpdatedCoins {
         received,
         confirmed,
+        expired,
         spending,
         spent,
     }
@@ -219,6 +222,7 @@ fn updates(
     // Having the tip in database means that, as far as the chain is concerned, we've got all
     // updates up to this block. But not more.
     db_conn.new_unspent_coins(&updated_coins.received);
+    db_conn.remove_coins(&updated_coins.expired);
     db_conn.confirm_coins(&updated_coins.confirmed);
     db_conn.spend_coins(&updated_coins.spending);
     db_conn.confirm_spend(&updated_coins.spent);

src/testutils.rs

@@ -65,8 +65,11 @@ impl BitcoinInterface for DummyBitcoind {
         Vec::new()
     }
 
-    fn confirmed_coins(&self, _: &[bitcoin::OutPoint]) -> Vec<(bitcoin::OutPoint, i32, u32)> {
-        Vec::new()
+    fn confirmed_coins(
+        &self,
+        _: &[bitcoin::OutPoint],
+    ) -> (Vec<(bitcoin::OutPoint, i32, u32)>, Vec<bitcoin::OutPoint>) {
+        (Vec::new(), Vec::new())
     }
 
     fn spending_coins(&self, _: &[bitcoin::OutPoint]) -> Vec<(bitcoin::OutPoint, bitcoin::Txid)> {

tests/test_chain.py

@@ -76,13 +76,10 @@ def test_reorg_exclusion(lianad, bitcoind):
     bitcoind.simple_reorg(initial_height, shift=-1)
     wait_for(lambda: lianad.rpc.getinfo()["block_height"] == current_height + 1)
 
-    # They must all be marked as unconfirmed.
-    new_coin_a = get_coin(lianad, coin_a["outpoint"])
-    assert new_coin_a["block_height"] is None
-    new_coin_b = get_coin(lianad, coin_b["outpoint"])
-    assert new_coin_b["block_height"] is None
-    new_coin_c = get_coin(lianad, coin_c["outpoint"])
-    assert new_coin_c["block_height"] is None
+    # For a too deep reorg bitcoind doesn't update the mempool. The deposit transactions were
+    # dropped. And we discard the unconfirmed coins whose deposit tx isn't part of our mempool
+    # anymore: the coins must have been marked as unconfirmed and subsequently discarded.
+    wait_for(lambda: len(lianad.rpc.listcoins()["coins"]) == 0)
 
     # And if we now confirm everything, they'll be marked as such. The one that was 'spending'
     # will now be spent (its spending transaction will be confirmed) and the one that was spent