diff --git a/src/init.cpp b/src/init.cpp index 4efe76104..ce8322dc4 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -2654,8 +2654,19 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler) nStart = GetTimeMillis(); pwalletMain->ScanForWalletTransactions(pindexRescan, true); LogPrintf(" rescan %15dms\n", GetTimeMillis() - nStart); - pwalletMain->SetBestChain(chainActive.GetLocator()); - nWalletDBUpdated++; + // ONLY record "scanned to the tip" if the scan actually reached it. An aborted or + // shutdown-interrupted scan that stamps a tip locator tells the next startup there is + // nothing left to scan (the chainActive.Tip() != pindexRescan guard above then skips + // the rescan entirely), so every transaction in the unscanned range stays out of + // mapWallet permanently: invisible to getbalance and unspendable. The scan writes its + // own locator at the interrupt point instead. + if (pwalletMain->fLastRescanCompleted) { + pwalletMain->SetBestChain(chainActive.GetLocator()); + nWalletDBUpdated++; + } else { + LogPrintf("Rescan did not complete; leaving the best-block locator at the scan's own " + "checkpoint so the remaining range is rescanned on the next start\n"); + } // Restore wallet transaction metadata after -zapwallettxes=1 if (GetBoolArg("-zapwallettxes", false) && GetArg("-zapwallettxes", "1") != "2") diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index ca29538ad..20df4ea69 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -801,10 +801,18 @@ bool CWallet::CommitAutomatedTx(const CTransaction& tx) { void CWallet::SetBestChain(const CBlockLocator& loc) { + // Default ctor => fFlushOnClose=true => ~CDB runs a full BDB txn_checkpoint over the entire + // cache. Fine for the hourly/shutdown callers; ruinous inside a scan (see SetBestChainNoFlush). CWalletDB walletdb(strWalletFile); SetBestChainINTERNAL(walletdb, loc); } +bool CWallet::SetBestChainNoFlush(const CBlockLocator& loc) +{ + CWalletDB walletdb(strWalletFile, "r+", false); + return SetBestChainINTERNAL(walletdb, loc); +} + std::set> CWallet::GetNullifiersForAddresses( const std::set & addresses) { @@ -1436,8 +1444,9 @@ void CWallet::BuildWitnessCache(const CBlockIndex* pindex, bool witnessOnly) return; } if (pwalletMain->fAbortRescan) { + // Do NOT clear fRescanning here: a witness rebuild is not a rescan, and clearing it from + // this path desynchronises the flag from ScanForWalletTransactions, which owns it. LogPrintf("%s: rescan aborted during witness rebuild\n", __func__); - pwalletMain->fRescanning = false; return; } int h = pbi->GetHeight(); @@ -3434,6 +3443,12 @@ int CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate) if(fZdebug) LogPrintf("%s: fUpdate=%d now=%li\n",__func__,fUpdate,nNow); + // fAbortRescan is sticky: nothing else in the tree ever clears it, so without this a single + // `abortrescan` RPC would disable every later scan AND every later BuildWitnessCache for the + // lifetime of the process (BuildWitnessCache bails on the same flag), freezing witness heights + // while the chain advances and progressively rendering notes unspendable. + pwalletMain->fAbortRescan = false; + pwalletMain->fLastRescanCompleted = false; pwalletMain->fRescanning = true; CBlockIndex* pindex = pindexStart; pwalletMain->rescanStartHeight = pindex->GetHeight(); @@ -3456,15 +3471,18 @@ int CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate) { pwalletMain->rescanHeight = pindex->GetHeight(); if(pwalletMain->fAbortRescan) { - //TODO: should we update witness caches? - LogPrintf("%s: Rescan aborted at block %d\n", pwalletMain->rescanHeight); - pwalletMain->fRescanning = false; + // The witness caches do NOT need updating here: on resume each note's witnesses are + // re-derived from its own witnessHeight, independently of the locator, and + // witnessRootValidated is in-memory-only so a full validation pass runs next boot. + // What DOES need saving is the locator -- see the checkpoint below. + pwalletMain->fRescanning = false; + pwalletMain->fAbortRescan = false; // consume it; see the note at scan entry + LogPrintf("%s: Rescan aborted at block %d\n", __func__, pwalletMain->rescanHeight); return ret; } if (ShutdownRequested()) { - //TODO: should we update witness caches? - LogPrintf("%s: Rescan interrupted by shutdown request at block %d\n", pwalletMain->rescanHeight); pwalletMain->fRescanning = false; + LogPrintf("%s: Rescan interrupted by shutdown request at block %d\n", __func__, pwalletMain->rescanHeight); return ret; } @@ -3525,6 +3543,9 @@ int CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate) // we are no longer rescanning pwalletMain->fRescanning = false; + // Reached only by running the loop to the end of the active chain. Callers use this to decide + // whether it is honest to record the wallet as scanned up to the tip. + pwalletMain->fLastRescanCompleted = true; return ret; } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 95527738b..107814971 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -833,6 +833,10 @@ public: bool fAutoShieldRunning = false; std::atomic fAbortRescan{false}; + //! True only when the last ScanForWalletTransactions ran to completion. An aborted or + //! shutdown-interrupted scan leaves this false, so callers must not record the wallet as + //! scanned up to the chain tip -- doing so makes the unscanned range permanently invisible. + bool fLastRescanCompleted = false; // abort current rescan void AbortRescan() { fAbortRescan = true; } // Are we currently aborting a rescan? @@ -904,17 +908,22 @@ protected: */ void DecrementNoteWitnesses(const CBlockIndex* pindex); + //! Returns true only if the atomic write actually committed. Callers that checkpoint during a + //! long scan need to know: a silently-failing checkpoint would otherwise be retried forever at + //! full cost while never making progress. template - void SetBestChainINTERNAL(WalletDB& walletdb, const CBlockLocator& loc) { + bool SetBestChainINTERNAL(WalletDB& walletdb, const CBlockLocator& loc) { if (!walletdb.TxnBegin()) { // This needs to be done atomically, so don't do it at all LogPrintf("SetBestChain(): Couldn't start atomic write\n"); - return; + return false; } try { LOCK(cs_wallet); for (std::pair& wtxItem : mapWallet) { - auto wtx = wtxItem.second; + // By reference: a copy here deep-copies every note's witness deque + // (WITNESS_CACHE_SIZE entries) for every transaction, on every call. + const CWalletTx& wtx = wtxItem.second; // We skip transactions for which mapSaplingNoteData // is empty. This covers transactions that have no Sapling data // (i.e. are purely transparent), as well as shielding and unshielding @@ -923,32 +932,33 @@ protected: if (!walletdb.WriteTx(wtxItem.first, wtx)) { LogPrintf("SetBestChain(): Failed to write CWalletTx, aborting atomic write\n"); walletdb.TxnAbort(); - return; + return false; } } } if (!walletdb.WriteWitnessCacheSize(nWitnessCacheSize)) { LogPrintf("SetBestChain(): Failed to write nWitnessCacheSize, aborting atomic write\n"); walletdb.TxnAbort(); - return; + return false; } if (!walletdb.WriteBestBlock(loc)) { LogPrintf("SetBestChain(): Failed to write best block, aborting atomic write\n"); walletdb.TxnAbort(); - return; + return false; } } catch (const std::exception &exc) { // Unexpected failure LogPrintf("SetBestChain(): Unexpected error during atomic write:\n"); LogPrintf("%s\n", exc.what()); walletdb.TxnAbort(); - return; + return false; } if (!walletdb.TxnCommit()) { // Couldn't commit all to db, but in-memory state is fine LogPrintf("SetBestChain(): Couldn't commit atomic write\n"); - return; + return false; } + return true; } private: @@ -1282,8 +1292,12 @@ public: void RunSaplingConsolidation(int blockHeight); void RunAutoShieldCoinbase(int blockHeight); bool CommitAutomatedTx(const CTransaction& tx); - /** Saves witness caches and best block locator to disk. */ + /** Saves witness caches and best block locator to disk. Overrides CValidationInterface. */ void SetBestChain(const CBlockLocator& loc); + /** As SetBestChain, but for use INSIDE a long scan: opens the wallet DB with fFlushOnClose=false + * so the call does not trigger a full BDB txn_checkpoint over the whole cache (see the comment + * at CWallet::SetBestChain), and reports whether the write actually committed. */ + bool SetBestChainNoFlush(const CBlockLocator& loc); std::set> GetNullifiersForAddresses(const std::set & addresses); bool IsNoteSaplingChange(const std::set> & nullifierSet, const libzcash::PaymentAddress & address, const SaplingOutPoint & entry);