diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -130,8 +130,13 @@ auto locked_chain = wallet->chain().lock(); WalletRescanReserver reserver(wallet.get()); reserver.reserve(); - wallet->ScanForWalletTransactions(::ChainActive().Genesis(), nullptr, - reserver, true); + const CBlockIndex *const null_block = nullptr; + const CBlockIndex *stop_block; + QCOMPARE(wallet->ScanForWalletTransactions( + ::ChainActive().Genesis(), nullptr, reserver, stop_block, + true /* fUpdate */), + CWallet::ScanResult::SUCCESS); + QCOMPARE(stop_block, null_block); } wallet->SetBroadcastTransactions(true); diff --git a/src/test/setup_common.h b/src/test/setup_common.h --- a/src/test/setup_common.h +++ b/src/test/setup_common.h @@ -15,6 +15,8 @@ #include #include +#include + /** * Version of Boost::test prior to 1.64 have issues when dealing with nullptr_t. * In order to work around this, we ensure that the null pointers are typed in a @@ -24,6 +26,14 @@ */ #define NULLPTR(T) static_cast(nullptr) +// Enable BOOST_CHECK_EQUAL for enum class types +template +std::ostream &operator<<( + typename std::enable_if::value, std::ostream>::type &stream, + const T &e) { + return stream << static_cast::type>(e); +} + /** * This global and the helpers that use it are not thread-safe. * diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -4092,18 +4092,20 @@ } } - const CBlockIndex *stopBlock = pwallet->ScanForWalletTransactions( - pindexStart, pindexStop, reserver, true); - if (!stopBlock) { - if (pwallet->IsAbortingRescan()) { + const CBlockIndex *stopBlock; + CWallet::ScanResult result = pwallet->ScanForWalletTransactions( + pindexStart, pindexStop, reserver, stopBlock, true); + switch (result) { + case CWallet::ScanResult::SUCCESS: + stopBlock = pindexStop ? pindexStop : pChainTip; + break; + case CWallet::ScanResult::FAILURE: + throw JSONRPCError( + RPC_MISC_ERROR, + "Rescan failed. Potentially corrupted data files."); + case CWallet::ScanResult::USER_ABORT: throw JSONRPCError(RPC_MISC_ERROR, "Rescan aborted."); - } - // if we got a nullptr returned, ScanForWalletTransactions did rescan up - // to the requested stopindex - stopBlock = pindexStop ? pindexStop : pChainTip; - } else { - throw JSONRPCError(RPC_MISC_ERROR, - "Rescan failed. Potentially corrupted data files."); + // no default case, so the compiler can warn about missing cases } UniValue response(UniValue::VOBJ); response.pushKV("start_height", pindexStart->nHeight); diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -37,7 +37,7 @@ auto chain = interfaces::MakeChain(); // Cap last block file size, and mine new block in a new block file. - CBlockIndex *const nullBlock = nullptr; + const CBlockIndex *const null_block = nullptr; CBlockIndex *oldTip = ::ChainActive().Tip(); GetBlockFileInfo(oldTip->GetBlockPos().nFile)->nSize = MAX_BLOCKFILE_SIZE; CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); @@ -54,8 +54,11 @@ AddKey(wallet, coinbaseKey); WalletRescanReserver reserver(&wallet); reserver.reserve(); - BOOST_CHECK_EQUAL(nullBlock, wallet.ScanForWalletTransactions( - oldTip, nullptr, reserver)); + const CBlockIndex *stop_block; + BOOST_CHECK_EQUAL(wallet.ScanForWalletTransactions( + oldTip, nullptr, reserver, stop_block), + CWallet::ScanResult::SUCCESS); + BOOST_CHECK_EQUAL(stop_block, null_block); BOOST_CHECK_EQUAL(wallet.GetImmatureBalance(), 100 * COIN); } @@ -71,8 +74,11 @@ AddKey(wallet, coinbaseKey); WalletRescanReserver reserver(&wallet); reserver.reserve(); - BOOST_CHECK_EQUAL(oldTip, wallet.ScanForWalletTransactions( - oldTip, nullptr, reserver)); + const CBlockIndex *stop_block; + BOOST_CHECK_EQUAL(wallet.ScanForWalletTransactions( + oldTip, nullptr, reserver, stop_block), + CWallet::ScanResult::FAILURE); + BOOST_CHECK_EQUAL(oldTip, stop_block); BOOST_CHECK_EQUAL(wallet.GetImmatureBalance(), 50 * COIN); } @@ -310,8 +316,13 @@ AddKey(*wallet, coinbaseKey); WalletRescanReserver reserver(wallet.get()); reserver.reserve(); - wallet->ScanForWalletTransactions(::ChainActive().Genesis(), nullptr, - reserver); + const CBlockIndex *const null_block = nullptr; + const CBlockIndex *stop_block; + BOOST_CHECK_EQUAL( + wallet->ScanForWalletTransactions(ChainActive().Genesis(), nullptr, + reserver, stop_block), + CWallet::ScanResult::SUCCESS); + BOOST_CHECK_EQUAL(stop_block, null_block); } ~ListCoinsTestingSetup() { wallet.reset(); } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1062,11 +1062,13 @@ BlockDisconnected(const std::shared_ptr &pblock) override; int64_t RescanFromTime(int64_t startTime, const WalletRescanReserver &reserver, bool update); - const CBlockIndex * - ScanForWalletTransactions(const CBlockIndex *const pindexStart, - const CBlockIndex *const pindexStop, - const WalletRescanReserver &reserver, - bool fUpdate = false); + + enum class ScanResult { SUCCESS, FAILURE, USER_ABORT }; + ScanResult ScanForWalletTransactions(const CBlockIndex *const pindexStart, + const CBlockIndex *const pindexStop, + const WalletRescanReserver &reserver, + const CBlockIndex *&failed_block, + bool fUpdate = false); void TransactionRemovedFromMempool(const CTransactionRef &ptx) override; void ReacceptWalletTransactions(); void ResendWalletTransactions(int64_t nBestBlockTime, diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1802,9 +1802,11 @@ } if (startBlock) { - const CBlockIndex *const failedBlock = - ScanForWalletTransactions(startBlock, nullptr, reserver, update); - if (failedBlock) { + const CBlockIndex *failedBlock; + // TODO: this should take into account failure by ScanResult::USER_ABORT + if (ScanResult::FAILURE == + ScanForWalletTransactions(startBlock, nullptr, reserver, + failedBlock, update)) { return failedBlock->GetBlockTimeMax() + TIMESTAMP_WINDOW + 1; } } @@ -1816,20 +1818,24 @@ * us. If fUpdate is true, found transactions that already exist in the wallet * will be updated. * - * Returns null if scan was successful. Otherwise, if a complete rescan was not - * possible (due to pruning or corruption), returns pointer to the most recent - * block that could not be scanned. + * @param[in] pindexStop if not a nullptr, the scan will stop at this + * block-index + * @param[out] failed_block if FAILURE is returned, will be set to the most + * recent block that could not be scanned, otherwise nullptr. * - * If pindexStop is not a nullptr, the scan will stop at the block-index - * defined by pindexStop + * @return ScanResult indicating success or failure of the scan. SUCCESS if + * scan was successful. FAILURE if a complete rescan was not possible (due to + * pruning or corruption). USER_ABORT if the rescan was aborted before it + * could complete. * - * Caller needs to make sure pindexStop (and the optional pindexStart) are on - * the main chain after to the addition of any new keys you want to detect + * @pre Caller needs to make sure pindexStop (and the optional pindexStart) are + * on the main chain after to the addition of any new keys you want to detect * transactions for. */ -const CBlockIndex *CWallet::ScanForWalletTransactions( +CWallet::ScanResult CWallet::ScanForWalletTransactions( const CBlockIndex *const pindexStart, const CBlockIndex *const pindexStop, - const WalletRescanReserver &reserver, bool fUpdate) { + const WalletRescanReserver &reserver, const CBlockIndex *&failed_block, + bool fUpdate) { int64_t nNow = GetTime(); assert(reserver.isReserved()); @@ -1838,7 +1844,7 @@ } const CBlockIndex *pindex = pindexStart; - const CBlockIndex *ret = nullptr; + failed_block = nullptr; if (pindex) { WalletLogPrintf("Rescan started from block %d...\n", pindex->nHeight); @@ -1893,7 +1899,7 @@ // Abort scan if current block is no longer active, to // prevent marking transactions as coming from the wrong // block. - ret = pindex; + failed_block = pindex; break; } for (size_t posInBlock = 0; posInBlock < block.vtx.size(); @@ -1902,7 +1908,7 @@ fUpdate); } } else { - ret = pindex; + failed_block = pindex; } if (pindex == pindexStop) { break; @@ -1921,20 +1927,22 @@ } } + // Hide progress dialog in GUI. + ShowProgress(strprintf("%s " + _("Rescanning..."), GetDisplayName()), + 100); if (pindex && fAbortRescan) { WalletLogPrintf("Rescan aborted at block %d. Progress=%f\n", pindex->nHeight, progress_current); + return ScanResult::USER_ABORT; } else if (pindex && ShutdownRequested()) { WalletLogPrintf("Rescan interrupted by shutdown request at block " "%d. Progress=%f\n", pindex->nHeight, progress_current); + return ScanResult::USER_ABORT; } - - // Hide progress dialog in GUI. - ShowProgress(strprintf("%s " + _("Rescanning..."), GetDisplayName()), - 100); } - return ret; + + return failed_block ? ScanResult::FAILURE : ScanResult::SUCCESS; } void CWallet::ReacceptWalletTransactions() { @@ -4714,13 +4722,15 @@ nStart = GetTimeMillis(); { WalletRescanReserver reserver(walletInstance.get()); - if (!reserver.reserve()) { + const CBlockIndex *stop_block; + if (!reserver.reserve() || + (ScanResult::SUCCESS != + walletInstance->ScanForWalletTransactions( + pindexRescan, nullptr, reserver, stop_block, true))) { InitError( _("Failed to rescan the wallet during initialization")); return nullptr; } - walletInstance->ScanForWalletTransactions(pindexRescan, nullptr, - reserver, true); } walletInstance->WalletLogPrintf("Rescan completed in %15dms\n", GetTimeMillis() - nStart);