From f1532f8afcceb5ded299f5ddc425c0473de49871 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Thu, 2 Feb 2023 16:35:42 +0100 Subject: [PATCH] bitcoin: track expired unconfirmed deposits, remove them from DB --- src/bitcoin/d/mod.rs | 16 ++++++++++++++++ src/bitcoin/mod.rs | 19 +++++++++++++------ src/bitcoin/poller/looper.rs | 6 +++++- src/testutils.rs | 7 +++++-- tests/test_chain.py | 11 ++++------- 5 files changed, 43 insertions(+), 16 deletions(-) diff --git a/src/bitcoin/d/mod.rs b/src/bitcoin/d/mod.rs index 12d8a3fd..1f903e22 100644 --- a/src/bitcoin/d/mod.rs +++ b/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", ¶ms!(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. diff --git a/src/bitcoin/mod.rs b/src/bitcoin/mod.rs index 159e8b18..ecb96fc3 100644 --- a/src/bitcoin/mod.rs +++ b/src/bitcoin/mod.rs @@ -58,11 +58,12 @@ pub trait BitcoinInterface: Send { descs: &[descriptors::InheritanceDescriptor], ) -> Vec; - /// 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); /// 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) { + // 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> fn confirmed_coins( &self, outpoints: &[bitcoin::OutPoint], - ) -> Vec<(bitcoin::OutPoint, i32, u32)> { + ) -> (Vec<(bitcoin::OutPoint, i32, u32)>, Vec) { self.lock().unwrap().confirmed_coins(outpoints) } diff --git a/src/bitcoin/poller/looper.rs b/src/bitcoin/poller/looper.rs index 807965b3..2718be48 100644 --- a/src/bitcoin/poller/looper.rs +++ b/src/bitcoin/poller/looper.rs @@ -15,6 +15,7 @@ use miniscript::bitcoin::{self, secp256k1}; struct UpdatedCoins { pub received: Vec, pub confirmed: Vec<(bitcoin::OutPoint, i32, u32)>, + pub expired: Vec, 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); diff --git a/src/testutils.rs b/src/testutils.rs index cf063e15..dcdb1018 100644 --- a/src/testutils.rs +++ b/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) { + (Vec::new(), Vec::new()) } fn spending_coins(&self, _: &[bitcoin::OutPoint]) -> Vec<(bitcoin::OutPoint, bitcoin::Txid)> { diff --git a/tests/test_chain.py b/tests/test_chain.py index d919ee15..e8001cb4 100644 --- a/tests/test_chain.py +++ b/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