From 9ef429b6ae65f6ad3e9ac11c2d9c0a6c52beb865 Mon Sep 17 00:00:00 2001 From: furszy Date: Wed, 29 Jan 2025 13:33:22 -0500 Subject: [PATCH] wallet: fix crash on double block disconnection The wallet crashes if it processes the same block disconnection event twice in a row due to an incompatible coinbase transaction state. This happens because 'disconnectBlock' provides 'TxStateInactive' without the "abandoned" flag for coinbase transactions to 'SyncTransaction', while 'AddToWallet()' internally modifies it to retain the abandoned state. The flow is as follows: 1) On the first disconnection, the transaction state transitions from "confirmed" to "inactive," bypassing the state equality check since the provided state differs. Then, 'AddToWallet' internally updates the state to "inactive + abandoned" 2) On the second disconnection, as we provide only the "inactive" state to 'SyncTransaction()', the state equality assertion fails and crashes the wallet. --- src/wallet/wallet.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) 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