From 4818da809f0e300016390dd41e02c6067ffa008f Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Wed, 8 Jan 2025 18:31:39 -0500 Subject: [PATCH] wallet: fix rescanning inconsistency If the chain advances during a rescan, ScanForWalletTransactions would previously process the new blocks without adjusting m_last_processed_block, which would leave the wallet in an inconsistent state temporarily, and could lead to crashes in the GUI. Fix this by not rescanning blocks beyond the last_processed_block - for all blocks beyond that height, there will be pending BlockConnected notifications that will process them after the rescan is finished. Co-authored-by: Pablo Greco --- src/wallet/wallet.cpp | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index b971be5ddda..adf9b602f52 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1984,6 +1984,14 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc if (max_height && block_height >= *max_height) { break; } + // If rescanning was triggered with cs_wallet permanently locked (AttachChain), additional blocks that were connected during the rescan + // aren't processed here but will be processed with the pending blockConnected notifications after the lock is released. + // If rescanning without a permanent cs_wallet lock, additional blocks that were added during the rescan will be re-processed if + // the notification was processed and the last block height was updated. + if (block_height >= WITH_LOCK(cs_wallet, return GetLastBlockHeight())) { + break; + } + { if (!next_block) { // break successfully when rescan has reached the tip, or @@ -3266,12 +3274,12 @@ bool CWallet::AttachChain(const std::shared_ptr& walletInstance, interf } // Register wallet with validationinterface. It's done before rescan to avoid - // missing block connections between end of rescan and validation subscribing. - // Because of wallet lock being hold, block connection notifications are going to - // be pending on the validation-side until lock release. It's likely to have - // block processing duplicata (if rescan block range overlaps with notification one) - // but we guarantee at least than wallet state is correct after notifications delivery. - // However, chainStateFlushed notifications are ignored until the rescan is finished + // missing block connections during the rescan. + // Because of the wallet lock being held, block connection notifications are going to + // be pending on the validation-side until lock release. Blocks that are connected while the + // rescan is ongoing will not be processed in the rescan but with the block connected notifications, + // so the wallet will only be completeley synced after the notifications delivery. + // chainStateFlushed notifications are ignored until the rescan is finished // so that in case of a shutdown event, the rescan will be repeated at the next start. // This is temporary until rescan and notifications delivery are unified under same // interface.