diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 7abd17d31e9..09eda0c28e4 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1548,8 +1548,11 @@ void CWallet::blockDisconnected(const interfaces::BlockInfo& block) int disconnect_height = block.height; - for (const CTransactionRef& ptx : Assert(block.data)->vtx) { - SyncTransaction(ptx, TxStateInactive{}); + for (size_t index = 0; index < block.data->vtx.size(); index++) { + const CTransactionRef& ptx = Assert(block.data)->vtx[index]; + // Coinbase transactions are not only inactive but also abandoned, + // meaning they should never be relayed standalone via the p2p protocol. + SyncTransaction(ptx, TxStateInactive{/*abandoned=*/index == 0}); for (const CTxIn& tx_in : ptx->vin) { // No other wallet transactions conflicted with this transaction diff --git a/test/functional/wallet_reorgsrestore.py b/test/functional/wallet_reorgsrestore.py index a600930acd8..d9be3bd2e9a 100755 --- a/test/functional/wallet_reorgsrestore.py +++ b/test/functional/wallet_reorgsrestore.py @@ -19,6 +19,7 @@ import shutil from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, + assert_greater_than, assert_raises_rpc_error ) @@ -90,6 +91,54 @@ class ReorgsRestoreTest(BitcoinTestFramework): # Verify the coinbase descendant was also marked as abandoned assert_equal(wallet0.gettransaction(descendant_tx_id)['details'][0]['abandoned'], True) + def test_reorg_handling_during_unclean_shutdown(self): + self.log.info("Test that wallet doesn't crash due to a duplicate block disconnection event after an unclean shutdown") + node = self.nodes[0] + # Receive coinbase reward on a new wallet + node.createwallet(wallet_name="reorg_crash", load_on_startup=True) + wallet = node.get_wallet_rpc("reorg_crash") + self.generatetoaddress(node, 1, wallet.getnewaddress(), sync_fun=self.no_op) + + # Restart to ensure node and wallet are flushed + self.restart_node(0) + wallet = node.get_wallet_rpc("reorg_crash") + assert_greater_than(wallet.getwalletinfo()['immature_balance'], 0) + + # Disconnect tip and sync wallet state + tip = wallet.getbestblockhash() + wallet.invalidateblock(tip) + wallet.syncwithvalidationinterfacequeue() + + # Tip was disconnected, ensure coinbase has been abandoned + assert_equal(wallet.getwalletinfo()['immature_balance'], 0) + coinbase_tx_id = wallet.getblock(tip, verbose=1)["tx"][0] + assert_equal(wallet.gettransaction(coinbase_tx_id)['details'][0]['abandoned'], True) + + # Abort process abruptly to mimic an unclean shutdown (no chain state flush to disk) + node.process.kill() + + # Restart the node and confirm that it has not persisted the last chain state changes to disk + self.start_node(0) + assert_equal(node.getbestblockhash(), tip) + + # Due to an existing bug, the wallet incorrectly keeps the transaction in an abandoned state, even though that's + # no longer the case (after the unclean shutdown, the node's chain returned to the pre-invalidation tip). + # This issue blocks any future spending and results in an incorrect balance display. + wallet = node.get_wallet_rpc("reorg_crash") + assert_equal(wallet.getwalletinfo()['immature_balance'], 0) # FIXME: #31824. + + # Previously, a bug caused the node to crash if two block disconnection events occurred consecutively. + # Ensure this is no longer the case by simulating a new reorg. + node.invalidateblock(tip) + assert(node.getbestblockhash() != tip) + # Ensure wallet state is consistent now + assert_equal(wallet.gettransaction(coinbase_tx_id)['details'][0]['abandoned'], True) + assert_equal(wallet.getwalletinfo()['immature_balance'], 0) + + # And finally, verify the state if the block ends up being into the best chain again + node.reconsiderblock(tip) + assert_equal(wallet.gettransaction(coinbase_tx_id)['details'][0]['abandoned'], False) + assert_greater_than(wallet.getwalletinfo()['immature_balance'], 0) def run_test(self): # Send a tx from which to conflict outputs later @@ -163,6 +212,9 @@ class ReorgsRestoreTest(BitcoinTestFramework): # Verify we mark coinbase txs, and their descendants, as abandoned during startup self.test_coinbase_automatic_abandon_during_startup() + # Verify reorg behavior during an unclean shutdown + self.test_reorg_handling_during_unclean_shutdown() + if __name__ == '__main__': ReorgsRestoreTest(__file__).main()