From 6c006495ef07f163d0734ec35d3cd1589a4aae9d Mon Sep 17 00:00:00 2001 From: Samuel Dobson Date: Tue, 28 Sep 2021 21:07:09 +1300 Subject: Remove outdated dummy wallet -salvagewallet arg --- src/dummywallet.cpp | 1 - 1 file changed, 1 deletion(-) (limited to 'src') diff --git a/src/dummywallet.cpp b/src/dummywallet.cpp index 8caeb32627..b7890ab80b 100644 --- a/src/dummywallet.cpp +++ b/src/dummywallet.cpp @@ -40,7 +40,6 @@ void DummyWalletInit::AddWalletOptions(ArgsManager& argsman) const "-mintxfee=", "-paytxfee=", "-rescan", - "-salvagewallet", "-signer=", "-spendzeroconfchange", "-txconfirmtarget=", -- cgit v1.2.3 From f963b0fa8cdd5223feb828c5faf6c57bc4107c8a Mon Sep 17 00:00:00 2001 From: Samuel Dobson Date: Tue, 28 Sep 2021 22:00:44 +1300 Subject: Corrupt wallet tx shouldn't trigger rescan of all wallets --- src/wallet/wallet.cpp | 17 ++++++++++------- src/wallet/wallet.h | 2 +- src/wallet/walletdb.cpp | 8 ++++++-- src/wallet/walletdb.h | 3 ++- src/wallet/wallettool.cpp | 4 ++++ 5 files changed, 23 insertions(+), 11 deletions(-) (limited to 'src') diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 6885499be2..1888d04f6d 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2007,10 +2007,7 @@ DBErrors CWallet::LoadWallet() assert(m_internal_spk_managers.empty()); } - if (nLoadWalletRet != DBErrors::LOAD_OK) - return nLoadWalletRet; - - return DBErrors::LOAD_OK; + return nLoadWalletRet; } DBErrors CWallet::ZapSelectTx(std::vector& vHashIn, std::vector& vHashOut) @@ -2542,6 +2539,7 @@ std::shared_ptr CWallet::Create(WalletContext& context, const std::stri // TODO: Can't use std::make_shared because we need a custom deleter but // should be possible to use std::allocate_shared. std::shared_ptr walletInstance(new CWallet(chain, name, std::move(database)), ReleaseWallet); + bool rescan_required = false; DBErrors nLoadWalletRet = walletInstance->LoadWallet(); if (nLoadWalletRet != DBErrors::LOAD_OK) { if (nLoadWalletRet == DBErrors::CORRUPT) { @@ -2562,6 +2560,10 @@ std::shared_ptr CWallet::Create(WalletContext& context, const std::stri { error = strprintf(_("Wallet needed to be rewritten: restart %s to complete"), PACKAGE_NAME); return nullptr; + } else if (nLoadWalletRet == DBErrors::RESCAN_REQUIRED) { + warnings.push_back(strprintf(_("Error reading %s! Transaction data may be missing or incorrect." + " Rescanning wallet."), walletFile)); + rescan_required = true; } else { error = strprintf(_("Error loading %s"), walletFile); @@ -2753,7 +2755,7 @@ std::shared_ptr CWallet::Create(WalletContext& context, const std::stri LOCK(walletInstance->cs_wallet); - if (chain && !AttachChain(walletInstance, *chain, error, warnings)) { + if (chain && !AttachChain(walletInstance, *chain, rescan_required, error, warnings)) { return nullptr; } @@ -2775,7 +2777,7 @@ std::shared_ptr CWallet::Create(WalletContext& context, const std::stri return walletInstance; } -bool CWallet::AttachChain(const std::shared_ptr& walletInstance, interfaces::Chain& chain, bilingual_str& error, std::vector& warnings) +bool CWallet::AttachChain(const std::shared_ptr& walletInstance, interfaces::Chain& chain, const bool rescan_required, bilingual_str& error, std::vector& warnings) { LOCK(walletInstance->cs_wallet); // allow setting the chain if it hasn't been set already but prevent changing it @@ -2792,8 +2794,9 @@ bool CWallet::AttachChain(const std::shared_ptr& walletInstance, interf // interface. walletInstance->m_chain_notifications_handler = walletInstance->chain().handleNotifications(walletInstance); + // If either rescan_required = true or -rescan is set, rescan_height remains equal to 0 int rescan_height = 0; - if (!gArgs.GetBoolArg("-rescan", false)) + if (!rescan_required && !gArgs.GetBoolArg("-rescan", false)) { WalletBatch batch(walletInstance->GetDatabase()); CBlockLocator locator; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 15a5933424..cedccf7d44 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -337,7 +337,7 @@ private: * block locator and m_last_block_processed, and registering for * notifications about new blocks and transactions. */ - static bool AttachChain(const std::shared_ptr& wallet, interfaces::Chain& chain, bilingual_str& error, std::vector& warnings); + static bool AttachChain(const std::shared_ptr& wallet, interfaces::Chain& chain, const bool rescan_required, bilingual_str& error, std::vector& warnings); public: /** diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index c697534c06..11c35e70e9 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -755,6 +755,7 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) { CWalletScanState wss; bool fNoncriticalErrors = false; + bool rescan_required = false; DBErrors result = DBErrors::LOAD_OK; LOCK(pwallet->cs_wallet); @@ -823,7 +824,7 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) fNoncriticalErrors = true; // ... but do warn the user there is something wrong. if (strType == DBKeys::TX) // Rescan if there is a bad transaction record: - gArgs.SoftSetBoolArg("-rescan", true); + rescan_required = true; } } if (!strErr.empty()) @@ -859,8 +860,11 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) ((DescriptorScriptPubKeyMan*)spk_man)->AddCryptedKey(desc_key_pair.first.second, desc_key_pair.second.first, desc_key_pair.second.second); } - if (fNoncriticalErrors && result == DBErrors::LOAD_OK) + if (rescan_required && result == DBErrors::LOAD_OK) { + result = DBErrors::RESCAN_REQUIRED; + } else if (fNoncriticalErrors && result == DBErrors::LOAD_OK) { result = DBErrors::NONCRITICAL_ERROR; + } // Any wallet corruption at all: skip any rewriting or // upgrading, we don't want to make it worse. diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index a549c8039c..39d1e2615c 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -48,7 +48,8 @@ enum class DBErrors NONCRITICAL_ERROR, TOO_NEW, LOAD_FAIL, - NEED_REWRITE + NEED_REWRITE, + RESCAN_REQUIRED }; namespace DBKeys { diff --git a/src/wallet/wallettool.cpp b/src/wallet/wallettool.cpp index 50b6c9d29f..4abfde76e2 100644 --- a/src/wallet/wallettool.cpp +++ b/src/wallet/wallettool.cpp @@ -76,6 +76,10 @@ static std::shared_ptr MakeWallet(const std::string& name, const fs::pa } else if (load_wallet_ret == DBErrors::NEED_REWRITE) { tfm::format(std::cerr, "Wallet needed to be rewritten: restart %s to complete", PACKAGE_NAME); return nullptr; + } else if (load_wallet_ret == DBErrors::RESCAN_REQUIRED) { + tfm::format(std::cerr, "Error reading %s! Some transaction data might be missing or" + " incorrect. Wallet requires a rescan.", + name); } else { tfm::format(std::cerr, "Error loading %s", name); return nullptr; -- cgit v1.2.3 From bccd1d942d971e70e7a0f4f5628e1b74b3ac15e0 Mon Sep 17 00:00:00 2001 From: Samuel Dobson Date: Wed, 29 Sep 2021 16:32:07 +1300 Subject: Remove -rescan startup parameter --- src/dummywallet.cpp | 1 - src/init.cpp | 2 +- src/wallet/init.cpp | 1 - src/wallet/rpcdump.cpp | 4 ++-- src/wallet/rpcwallet.cpp | 2 +- src/wallet/salvage.cpp | 2 +- src/wallet/test/wallet_tests.cpp | 4 ++-- src/wallet/wallet.cpp | 6 +++--- 8 files changed, 10 insertions(+), 12 deletions(-) (limited to 'src') diff --git a/src/dummywallet.cpp b/src/dummywallet.cpp index b7890ab80b..7f6471740f 100644 --- a/src/dummywallet.cpp +++ b/src/dummywallet.cpp @@ -39,7 +39,6 @@ void DummyWalletInit::AddWalletOptions(ArgsManager& argsman) const "-maxtxfee=", "-mintxfee=", "-paytxfee=", - "-rescan", "-signer=", "-spendzeroconfchange", "-txconfirmtarget=", diff --git a/src/init.cpp b/src/init.cpp index 25b9c6b9ec..210e4d5f36 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -397,7 +397,7 @@ void SetupServerArgs(ArgsManager& argsman) -GetNumCores(), MAX_SCRIPTCHECK_THREADS, DEFAULT_SCRIPTCHECK_THREADS), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-persistmempool", strprintf("Whether to save the mempool on shutdown and load on restart (default: %u)", DEFAULT_PERSIST_MEMPOOL), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-pid=", strprintf("Specify pid file. Relative paths will be prefixed by a net-specific datadir location. (default: %s)", BITCOIN_PID_FILENAME), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); - argsman.AddArg("-prune=", strprintf("Reduce storage requirements by enabling pruning (deleting) of old blocks. This allows the pruneblockchain RPC to be called to delete specific blocks, and enables automatic pruning of old blocks if a target size in MiB is provided. This mode is incompatible with -txindex, -coinstatsindex and -rescan. " + argsman.AddArg("-prune=", strprintf("Reduce storage requirements by enabling pruning (deleting) of old blocks. This allows the pruneblockchain RPC to be called to delete specific blocks, and enables automatic pruning of old blocks if a target size in MiB is provided. This mode is incompatible with -txindex and -coinstatsindex. " "Warning: Reverting this setting requires re-downloading the entire blockchain. " "(default: 0 = disable pruning blocks, 1 = allow manual pruning via RPC, >=%u = automatically prune block files to stay under the specified target size in MiB)", MIN_DISK_SPACE_FOR_BLOCK_FILES / 1024 / 1024), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-reindex", "Rebuild chain state and block index from the blk*.dat files on disk", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index 7abdbb0e55..59a59f9794 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -62,7 +62,6 @@ void WalletInit::AddWalletOptions(ArgsManager& argsman) const CURRENCY_UNIT, FormatMoney(DEFAULT_TRANSACTION_MINFEE)), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); argsman.AddArg("-paytxfee=", strprintf("Fee rate (in %s/kvB) to add to transactions you send (default: %s)", CURRENCY_UNIT, FormatMoney(CFeeRate{DEFAULT_PAY_TX_FEE}.GetFeePerK())), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); - argsman.AddArg("-rescan", "Rescan the block chain for missing wallet transactions on startup", ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); #ifdef ENABLE_EXTERNAL_SIGNER argsman.AddArg("-signer=", "External signing tool, see doc/external-signer.md", ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); #endif diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 382e8b6116..4d7fb2d38c 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -1439,7 +1439,7 @@ RPCHelpMan importmulti() "and coins using this key may not appear in the wallet. This error could be " "caused by pruning or data corruption (see bitcoind log for details) and could " "be dealt with by downloading and rescanning the relevant blocks (see -reindex " - "and -rescan options).", + "option and rescanblockchain RPC).", GetImportTimestamp(request, now), scannedTime - TIMESTAMP_WINDOW - 1, TIMESTAMP_WINDOW))); response.push_back(std::move(result)); } @@ -1744,7 +1744,7 @@ RPCHelpMan importdescriptors() "and coins using this desc may not appear in the wallet. This error could be " "caused by pruning or data corruption (see bitcoind log for details) and could " "be dealt with by downloading and rescanning the relevant blocks (see -reindex " - "and -rescan options).", + "option and rescanblockchain RPC).", GetImportTimestamp(request, now), scanned_time - TIMESTAMP_WINDOW - 1, TIMESTAMP_WINDOW))); response.push_back(std::move(result)); } diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 7bc2dc7b21..f6cf8868de 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -2632,7 +2632,7 @@ static RPCHelpMan loadwallet() return RPCHelpMan{"loadwallet", "\nLoads a wallet from a wallet file or directory." "\nNote that all wallet command-line options used when starting bitcoind will be" - "\napplied to the new wallet (eg -rescan, etc).\n", + "\napplied to the new wallet.\n", { {"filename", RPCArg::Type::STR, RPCArg::Optional::NO, "The wallet directory or .dat file."}, {"load_on_startup", RPCArg::Type::BOOL, RPCArg::Optional::OMITTED_NAMED_ARG, "Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged."}, diff --git a/src/wallet/salvage.cpp b/src/wallet/salvage.cpp index ea045eb6d8..4151099c1f 100644 --- a/src/wallet/salvage.cpp +++ b/src/wallet/salvage.cpp @@ -45,7 +45,7 @@ bool RecoverDatabaseFile(const fs::path& file_path, bilingual_str& error, std::v // Call Salvage with fAggressive=true to // get as much data as possible. // Rewrite salvaged data to fresh wallet file - // Set -rescan so any missing transactions will be + // Rescan so any missing transactions will be // found. int64_t now = GetTime(); std::string newFilename = strprintf("%s.%d.bak", filename, now); diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 5431a38bee..9938380369 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -232,8 +232,8 @@ BOOST_FIXTURE_TEST_CASE(importmulti_rescan, TestChain100Setup) "seconds of key creation, and could contain transactions pertaining to the key. As a result, " "transactions and coins using this key may not appear in the wallet. This error could be caused " "by pruning or data corruption (see bitcoind log for details) and could be dealt with by " - "downloading and rescanning the relevant blocks (see -reindex and -rescan " - "options).\"}},{\"success\":true}]", + "downloading and rescanning the relevant blocks (see -reindex option and rescanblockchain " + "RPC).\"}},{\"success\":true}]", 0, oldTip->GetBlockTimeMax(), TIMESTAMP_WINDOW)); RemoveWallet(context, wallet, /* load_on_start= */ std::nullopt); } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 1888d04f6d..8dc30bfaeb 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1600,7 +1600,7 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc WalletLogPrintf("Rescan started from block %s...\n", start_block.ToString()); fAbortRescan = false; - ShowProgress(strprintf("%s " + _("Rescanning…").translated, GetDisplayName()), 0); // show rescan progress in GUI as dialog or on splashscreen, if -rescan on startup + ShowProgress(strprintf("%s " + _("Rescanning…").translated, GetDisplayName()), 0); // show rescan progress in GUI as dialog or on splashscreen, if rescan required on startup (e.g. due to corruption) uint256 tip_hash = WITH_LOCK(cs_wallet, return GetLastBlockHash()); uint256 end_hash = tip_hash; if (max_height) chain().findAncestorByHeight(tip_hash, *max_height, FoundBlock().hash(end_hash)); @@ -2794,9 +2794,9 @@ bool CWallet::AttachChain(const std::shared_ptr& walletInstance, interf // interface. walletInstance->m_chain_notifications_handler = walletInstance->chain().handleNotifications(walletInstance); - // If either rescan_required = true or -rescan is set, rescan_height remains equal to 0 + // If rescan_required = true, rescan_height remains equal to 0 int rescan_height = 0; - if (!rescan_required && !gArgs.GetBoolArg("-rescan", false)) + if (!rescan_required) { WalletBatch batch(walletInstance->GetDatabase()); CBlockLocator locator; -- cgit v1.2.3