diff --git a/src/bitcoin/mod.rs b/src/bitcoin/mod.rs index dc0e7426..b44cfea1 100644 --- a/src/bitcoin/mod.rs +++ b/src/bitcoin/mod.rs @@ -76,11 +76,16 @@ pub trait BitcoinInterface: Send { outpoints: &[bitcoin::OutPoint], ) -> Vec<(bitcoin::OutPoint, bitcoin::Txid)>; - /// Get all coins that are spent with the final spend tx txid and blocktime. + /// Get all coins that are spent with the final spend tx txid and blocktime. Along with the + /// coins for which the spending transaction "expired" (a conflicting transaction was mined and + /// it wasn't spending this coin). fn spent_coins( &self, outpoints: &[(bitcoin::OutPoint, bitcoin::Txid)], - ) -> Vec<(bitcoin::OutPoint, bitcoin::Txid, Block)>; + ) -> ( + Vec<(bitcoin::OutPoint, bitcoin::Txid, Block)>, + Vec, + ); /// Get the common ancestor between the Bitcoin backend's tip and the given tip. fn common_ancestor(&self, tip: &BlockChainTip) -> Option; @@ -237,9 +242,14 @@ impl BitcoinInterface for d::BitcoinD { fn spent_coins( &self, outpoints: &[(bitcoin::OutPoint, bitcoin::Txid)], - ) -> Vec<(bitcoin::OutPoint, bitcoin::Txid, Block)> { + ) -> ( + Vec<(bitcoin::OutPoint, bitcoin::Txid, Block)>, + Vec, + ) { // Spend coins to be returned. let mut spent = Vec::with_capacity(outpoints.len()); + // Coins whose spending transaction isn't in our local mempool anymore. + let mut expired = Vec::new(); // Cached calls to `gettransaction`. let mut tx_getter = CachedTxGetter::new(self); @@ -259,29 +269,38 @@ 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. + enum Conflict { + // A replacement spending transaction was confirmed. + Replaced((bitcoin::Txid, Block)), + // A transaction conflicting with the former spending transaction was confirmed, + // but it doesn't spend this outpoint anymore. + Dropped, + } let conflict = res.conflicting_txs.iter().find_map(|txid| { tx_getter.get_transaction(txid).and_then(|tx| { // FIXME: if a conflict was mined we should somehow wipe the spend_txid of this // coin. - tx.block.and_then(|block| { + 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 Some((*txid, block)); + return Conflict::Replaced((*txid, block)); } } - None + Conflict::Dropped }) }) }); - if let Some((txid, block)) = conflict { - spent.push((*op, txid, block)); + match conflict { + Some(Conflict::Replaced((txid, block))) => spent.push((*op, txid, block)), + Some(Conflict::Dropped) => expired.push(*op), + None => {} } } - spent + (spent, expired) } fn common_ancestor(&self, tip: &BlockChainTip) -> Option { @@ -385,7 +404,10 @@ impl BitcoinInterface for sync::Arc> fn spent_coins( &self, outpoints: &[(bitcoin::OutPoint, bitcoin::Txid)], - ) -> Vec<(bitcoin::OutPoint, bitcoin::Txid, Block)> { + ) -> ( + Vec<(bitcoin::OutPoint, bitcoin::Txid, Block)>, + Vec, + ) { self.lock().unwrap().spent_coins(outpoints) } diff --git a/src/bitcoin/poller/looper.rs b/src/bitcoin/poller/looper.rs index 0d64eabe..54b9e82e 100644 --- a/src/bitcoin/poller/looper.rs +++ b/src/bitcoin/poller/looper.rs @@ -17,6 +17,7 @@ struct UpdatedCoins { pub confirmed: Vec<(bitcoin::OutPoint, i32, u32)>, pub expired: Vec, pub spending: Vec<(bitcoin::OutPoint, bitcoin::Txid)>, + pub expired_spending: Vec, pub spent: Vec<(bitcoin::OutPoint, bitcoin::Txid, i32, u32)>, } @@ -136,8 +137,8 @@ fn update_coins( .map(|coin| (coin.outpoint, coin.spend_txid.expect("Coin is spending"))) .chain(spending.iter().cloned()) .collect(); - let spent = bit - .spent_coins(spending_coins.as_slice()) + let (spent, expired_spending) = bit.spent_coins(spending_coins.as_slice()); + let spent = spent .into_iter() .map(|(oupoint, txid, block)| (oupoint, txid, block.height, block.time)) .collect(); @@ -148,6 +149,7 @@ fn update_coins( confirmed, expired, spending, + expired_spending, spent, } } @@ -238,6 +240,7 @@ fn updates( db_conn.remove_coins(&updated_coins.expired); db_conn.confirm_coins(&updated_coins.confirmed); db_conn.spend_coins(&updated_coins.spending); + db_conn.unspend_coins(&updated_coins.expired_spending); db_conn.confirm_spend(&updated_coins.spent); if latest_tip != current_tip { db_conn.update_tip(&latest_tip); diff --git a/src/testutils.rs b/src/testutils.rs index b2249cfd..4f0505b3 100644 --- a/src/testutils.rs +++ b/src/testutils.rs @@ -82,8 +82,11 @@ impl BitcoinInterface for DummyBitcoind { fn spent_coins( &self, _: &[(bitcoin::OutPoint, bitcoin::Txid)], - ) -> Vec<(bitcoin::OutPoint, bitcoin::Txid, Block)> { - Vec::new() + ) -> ( + Vec<(bitcoin::OutPoint, bitcoin::Txid, Block)>, + Vec, + ) { + (Vec::new(), Vec::new()) } fn common_ancestor(&self, _: &BlockChainTip) -> Option { diff --git a/tests/test_chain.py b/tests/test_chain.py index 5ae84ea2..493fe75f 100644 --- a/tests/test_chain.py +++ b/tests/test_chain.py @@ -480,7 +480,7 @@ def test_spend_replacement(lianad, bitcoind): == first_txid ) - # Even once the RBF gets mined, the first coin's spend txid isn't updated. + # Once the RBF gets mined, the first coin's spend txid is wiped. bitcoind.generate_block(1, wait_for_mempool=third_txid) wait_for( lambda: all( @@ -488,7 +488,7 @@ def test_spend_replacement(lianad, bitcoind): for c in lianad.rpc.listcoins([], second_outpoints)["coins"] ) ) - assert ( - lianad.rpc.listcoins([], [first_outpoints[0]])["coins"][0]["spend_info"]["txid"] - == first_txid + wait_for( + lambda: lianad.rpc.listcoins([], [first_outpoints[0]])["coins"][0]["spend_info"] + is None )