Merge bitcoin/bitcoin#24984: wallet: ignore chainStateFlushed notifications while attaching chain

2052e3aa9aa666bdc86dac370f1dd8fb978d3497 wallet: ignore chainStateFlushed notifications while attaching chain (Martin Zumsande)

Pull request description:

  Fixes #24487

  When a rescan is performed during `CWallet::AttachChain()` (e.g. when loading an old wallet) but this is interrupted by a shutdown signal, the wallet will currently stop the rescan, receive a `chainStateFlushed` signal, set the saved best block to the tip and shut down. At next startup, the rescan is not continued or repeated because of this. But some blocks have never been scanned by the wallet, which could lead to an incorrect balance.

  Fix this by ignoring `chainStateFlushed` notifications until the chain is attached. Since `CWallet::chainStateFlushed` is being manually called by `AttachChain()` anyway after finishing with the rescan, it is not a problem if intermediate notifications are ignored.

  Manual rescans started / aborted by the `rescanblockchain` / `abortrescan` RPCs are not affected by this.

  I didn't choose alternative ways of fixing this issue that would delay the validationinterface registration or change anything else about the handling of `blockConnected` signals for the reasons mentioned in [this existing comment](https://github.com/bitcoin/bitcoin/blob/master/src/wallet/wallet.cpp#L2937-L2944).

ACKs for top commit:
  achow101:
    ACK 2052e3aa9aa666bdc86dac370f1dd8fb978d3497
  ryanofsky:
    Code review ACK 2052e3aa9aa666bdc86dac370f1dd8fb978d3497. This is a straightforward fix for the bug described in #24487 where a wallet could skip scanning blocks if is shut down in the middle of a sync and a chainStateFlushed notification was received during the sync. It would be nice to write a test for this but probably would be tricky to write.
  w0xlt:
    Code Review ACK 2052e3aa9a

Tree-SHA512: a6186173d72b26bd4adbf2315e11af365004a723ea5565a0f7b868584dc47c321a6572eafaeb2420bd21eed1c7ad92b47e6218c5eb72313a3c6bee58364e2247
This commit is contained in:
Andrew Chow 2022-04-28 14:47:20 -04:00 committed by pasta
parent 93e7c38788
commit 247fe418d6
No known key found for this signature in database
GPG Key ID: 52527BEDABE87984
2 changed files with 10 additions and 0 deletions

View File

@ -495,6 +495,11 @@ bool CWallet::ChangeWalletPassphrase(const SecureString& strOldWalletPassphrase,
void CWallet::chainStateFlushed(const CBlockLocator& loc)
{
// Don't update the best block until the chain is attached so that in case of a shutdown,
// the rescan will be restarted at next startup.
if (m_attaching_chain) {
return;
}
WalletBatch batch(GetDatabase());
batch.WriteBestBlock(loc);
}
@ -4989,6 +4994,8 @@ bool CWallet::AttachChain(const std::shared_ptr<CWallet>& walletInstance, interf
// 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
// 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.
walletInstance->m_chain_notifications_handler = walletInstance->chain().handleNotifications(walletInstance);
@ -5016,6 +5023,7 @@ bool CWallet::AttachChain(const std::shared_ptr<CWallet>& walletInstance, interf
if (tip_height && *tip_height != rescan_height)
{
walletInstance->m_attaching_chain = true; //ignores chainStateFlushed notifications
if (chain.havePruned()) {
int block_height = *tip_height;
while (block_height > 0 && chain.haveBlockOnDisk(block_height - 1) && rescan_height != block_height) {
@ -5058,6 +5066,7 @@ bool CWallet::AttachChain(const std::shared_ptr<CWallet>& walletInstance, interf
return false;
}
}
walletInstance->m_attaching_chain = false;
walletInstance->chainStateFlushed(chain.getTipLocator());
walletInstance->GetDatabase().IncrementUpdateCounter();
}

View File

@ -720,6 +720,7 @@ private:
std::atomic<bool> fAbortRescan{false}; // reset by WalletRescanReserver::reserve()
std::atomic<bool> fScanningWallet{false}; // controlled by WalletRescanReserver
std::atomic<bool> m_attaching_chain{false};
std::atomic<int64_t> m_scanning_start{0};
std::atomic<double> m_scanning_progress{0};
friend class WalletRescanReserver;