From facec1c643105d0ae74b5d32cf33d593f9e82a36 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Sun, 6 Oct 2019 17:52:05 -0400 Subject: [PATCH] wallet: Avoid showing GUI popups on RPC errors --- src/dummywallet.cpp | 5 +- src/interfaces/node.cpp | 12 ++-- src/interfaces/node.h | 4 +- src/qt/walletcontroller.cpp | 5 +- src/qt/walletcontroller.h | 2 +- src/wallet/db.cpp | 6 +- src/wallet/db.h | 2 +- src/wallet/load.cpp | 13 ++-- src/wallet/rpcwallet.cpp | 21 +++--- src/wallet/wallet.cpp | 97 +++++++++++++-------------- src/wallet/wallet.h | 8 +-- src/wallet/walletdb.cpp | 4 +- src/wallet/walletdb.h | 2 +- test/functional/wallet_multiwallet.py | 8 +-- 14 files changed, 91 insertions(+), 98 deletions(-) diff --git a/src/dummywallet.cpp b/src/dummywallet.cpp index 126e3479f..0edcb0286 100644 --- a/src/dummywallet.cpp +++ b/src/dummywallet.cpp @@ -2,7 +2,6 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#include #include #include #include @@ -71,12 +70,12 @@ std::vector> GetWallets() throw std::logic_error("Wallet function called in non-wallet build."); } -std::shared_ptr LoadWallet(interfaces::Chain& chain, const std::string& name, std::string& error, std::string& warning) +std::shared_ptr LoadWallet(interfaces::Chain& chain, const std::string& name, std::string& error, std::vector& warnings) { throw std::logic_error("Wallet function called in non-wallet build."); } -WalletCreationStatus CreateWallet(interfaces::Chain& chain, const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, std::string& error, std::string& warning, std::shared_ptr& result) +WalletCreationStatus CreateWallet(interfaces::Chain& chain, const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, std::string& error, std::vector& warnings, std::shared_ptr& result) { throw std::logic_error("Wallet function called in non-wallet build."); } diff --git a/src/interfaces/node.cpp b/src/interfaces/node.cpp index 3d89e1716..a703888d7 100644 --- a/src/interfaces/node.cpp +++ b/src/interfaces/node.cpp @@ -43,8 +43,8 @@ class CWallet; fs::path GetWalletDir(); std::vector ListWalletDir(); std::vector> GetWallets(); -std::shared_ptr LoadWallet(interfaces::Chain& chain, const std::string& name, std::string& error, std::string& warning); -WalletCreationStatus CreateWallet(interfaces::Chain& chain, const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, std::string& error, std::string& warning, std::shared_ptr& result); +std::shared_ptr LoadWallet(interfaces::Chain& chain, const std::string& name, std::string& error, std::vector& warnings); +WalletCreationStatus CreateWallet(interfaces::Chain& chain, const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, std::string& error, std::vector& warnings, std::shared_ptr& result); namespace interfaces { @@ -256,14 +256,14 @@ public: } return wallets; } - std::unique_ptr loadWallet(const std::string& name, std::string& error, std::string& warning) override + std::unique_ptr loadWallet(const std::string& name, std::string& error, std::vector& warnings) override { - return MakeWallet(LoadWallet(*m_interfaces.chain, name, error, warning)); + return MakeWallet(LoadWallet(*m_interfaces.chain, name, error, warnings)); } - WalletCreationStatus createWallet(const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, std::string& error, std::string& warning, std::unique_ptr& result) override + WalletCreationStatus createWallet(const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, std::string& error, std::vector& warnings, std::unique_ptr& result) override { std::shared_ptr wallet; - WalletCreationStatus status = CreateWallet(*m_interfaces.chain, passphrase, wallet_creation_flags, name, error, warning, wallet); + WalletCreationStatus status = CreateWallet(*m_interfaces.chain, passphrase, wallet_creation_flags, name, error, warnings, wallet); result = MakeWallet(wallet); return status; } diff --git a/src/interfaces/node.h b/src/interfaces/node.h index 688ff434b..4ee467014 100644 --- a/src/interfaces/node.h +++ b/src/interfaces/node.h @@ -200,10 +200,10 @@ public: //! Attempts to load a wallet from file or directory. //! The loaded wallet is also notified to handlers previously registered //! with handleLoadWallet. - virtual std::unique_ptr loadWallet(const std::string& name, std::string& error, std::string& warning) = 0; + virtual std::unique_ptr loadWallet(const std::string& name, std::string& error, std::vector& warnings) = 0; //! Create a wallet from file - virtual WalletCreationStatus createWallet(const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, std::string& error, std::string& warning, std::unique_ptr& result) = 0; + virtual WalletCreationStatus createWallet(const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, std::string& error, std::vector& warnings, std::unique_ptr& result) = 0; //! Register handler for init messages. using InitMessageFn = std::function; diff --git a/src/qt/walletcontroller.cpp b/src/qt/walletcontroller.cpp index fa6f9f3f1..a7edf442e 100644 --- a/src/qt/walletcontroller.cpp +++ b/src/qt/walletcontroller.cpp @@ -12,6 +12,7 @@ #include #include +#include #include @@ -226,7 +227,7 @@ void CreateWalletActivity::finish() if (!m_error_message.empty()) { QMessageBox::critical(m_parent_widget, tr("Create wallet failed"), QString::fromStdString(m_error_message)); } else if (!m_warning_message.empty()) { - QMessageBox::warning(m_parent_widget, tr("Create wallet warning"), QString::fromStdString(m_warning_message)); + QMessageBox::warning(m_parent_widget, tr("Create wallet warning"), QString::fromStdString(Join(m_warning_message, "\n"))); } if (m_wallet_model) Q_EMIT created(m_wallet_model); @@ -267,7 +268,7 @@ void OpenWalletActivity::finish() if (!m_error_message.empty()) { QMessageBox::critical(m_parent_widget, tr("Open wallet failed"), QString::fromStdString(m_error_message)); } else if (!m_warning_message.empty()) { - QMessageBox::warning(m_parent_widget, tr("Open wallet warning"), QString::fromStdString(m_warning_message)); + QMessageBox::warning(m_parent_widget, tr("Open wallet warning"), QString::fromStdString(Join(m_warning_message, "\n"))); } if (m_wallet_model) Q_EMIT opened(m_wallet_model); diff --git a/src/qt/walletcontroller.h b/src/qt/walletcontroller.h index fb37b7292..e50dd5c7e 100644 --- a/src/qt/walletcontroller.h +++ b/src/qt/walletcontroller.h @@ -100,7 +100,7 @@ protected: QProgressDialog* m_progress_dialog{nullptr}; WalletModel* m_wallet_model{nullptr}; std::string m_error_message; - std::string m_warning_message; + std::vector m_warning_message; }; diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index 26aeb754a..e48eee6c2 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -412,7 +412,7 @@ bool BerkeleyBatch::VerifyEnvironment(const fs::path& file_path, std::string& er return true; } -bool BerkeleyBatch::VerifyDatabaseFile(const fs::path& file_path, std::string& warningStr, std::string& errorStr, BerkeleyEnvironment::recoverFunc_type recoverFunc) +bool BerkeleyBatch::VerifyDatabaseFile(const fs::path& file_path, std::vector& warnings, std::string& errorStr, BerkeleyEnvironment::recoverFunc_type recoverFunc) { std::string walletFile; std::shared_ptr env = GetWalletEnv(file_path, walletFile); @@ -424,11 +424,11 @@ bool BerkeleyBatch::VerifyDatabaseFile(const fs::path& file_path, std::string& w BerkeleyEnvironment::VerifyResult r = env->Verify(walletFile, recoverFunc, backup_filename); if (r == BerkeleyEnvironment::VerifyResult::RECOVER_OK) { - warningStr = strprintf(_("Warning: Wallet file corrupt, data salvaged!" + warnings.push_back(strprintf(_("Warning: Wallet file corrupt, data salvaged!" " Original %s saved as %s in %s; if" " your balance or transactions are incorrect you should" " restore from a backup.").translated, - walletFile, backup_filename, walletDir); + walletFile, backup_filename, walletDir)); } if (r == BerkeleyEnvironment::VerifyResult::RECOVER_FAIL) { diff --git a/src/wallet/db.h b/src/wallet/db.h index 94f41eaf1..2204a97fe 100644 --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -246,7 +246,7 @@ public: /* verifies the database environment */ static bool VerifyEnvironment(const fs::path& file_path, std::string& errorStr); /* verifies the database file */ - static bool VerifyDatabaseFile(const fs::path& file_path, std::string& warningStr, std::string& errorStr, BerkeleyEnvironment::recoverFunc_type recoverFunc); + static bool VerifyDatabaseFile(const fs::path& file_path, std::vector& warnings, std::string& errorStr, BerkeleyEnvironment::recoverFunc_type recoverFunc); template bool Read(const K& key, T& value) diff --git a/src/wallet/load.cpp b/src/wallet/load.cpp index b5d3b8c30..071befaeb 100644 --- a/src/wallet/load.cpp +++ b/src/wallet/load.cpp @@ -7,6 +7,7 @@ #include #include +#include #include #include #include @@ -53,10 +54,10 @@ bool VerifyWallets(interfaces::Chain& chain, const std::vector& wal } std::string error_string; - std::string warning_string; - bool verify_success = CWallet::Verify(chain, location, salvage_wallet, error_string, warning_string); + std::vector warnings; + bool verify_success = CWallet::Verify(chain, location, salvage_wallet, error_string, warnings); if (!error_string.empty()) chain.initError(error_string); - if (!warning_string.empty()) chain.initWarning(warning_string); + if (!warnings.empty()) chain.initWarning(Join(warnings, "\n")); if (!verify_success) return false; } @@ -66,8 +67,12 @@ bool VerifyWallets(interfaces::Chain& chain, const std::vector& wal bool LoadWallets(interfaces::Chain& chain, const std::vector& wallet_files) { for (const std::string& walletFile : wallet_files) { - std::shared_ptr pwallet = CWallet::CreateWalletFromFile(chain, WalletLocation(walletFile)); + std::string error; + std::vector warnings; + std::shared_ptr pwallet = CWallet::CreateWalletFromFile(chain, WalletLocation(walletFile), error, warnings); + if (!warnings.empty()) chain.initWarning(Join(warnings, "\n")); if (!pwallet) { + chain.initError(error); return false; } AddWallet(pwallet); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 96fa50d42..b2a7f9845 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -2587,13 +2588,14 @@ static UniValue loadwallet(const JSONRPCRequest& request) } } - std::string error, warning; + std::string error; + std::vector warning; std::shared_ptr const wallet = LoadWallet(*g_rpc_interfaces->chain, location, error, warning); if (!wallet) throw JSONRPCError(RPC_WALLET_ERROR, error); UniValue obj(UniValue::VOBJ); obj.pushKV("name", wallet->GetName()); - obj.pushKV("warning", warning); + obj.pushKV("warning", Join(warning, "\n")); return obj; } @@ -2699,12 +2701,12 @@ static UniValue createwallet(const JSONRPCRequest& request) } SecureString passphrase; passphrase.reserve(100); - std::string warning; + std::vector warnings; if (!request.params[3].isNull()) { passphrase = request.params[3].get_str().c_str(); if (passphrase.empty()) { // Empty string means unencrypted - warning = "Empty string given as passphrase, wallet will not be encrypted."; + warnings.emplace_back("Empty string given as passphrase, wallet will not be encrypted."); } } @@ -2713,9 +2715,8 @@ static UniValue createwallet(const JSONRPCRequest& request) } std::string error; - std::string create_warning; std::shared_ptr wallet; - WalletCreationStatus status = CreateWallet(*g_rpc_interfaces->chain, passphrase, flags, request.params[0].get_str(), error, create_warning, wallet); + WalletCreationStatus status = CreateWallet(*g_rpc_interfaces->chain, passphrase, flags, request.params[0].get_str(), error, warnings, wallet); switch (status) { case WalletCreationStatus::CREATION_FAILED: throw JSONRPCError(RPC_WALLET_ERROR, error); @@ -2726,15 +2727,9 @@ static UniValue createwallet(const JSONRPCRequest& request) // no default case, so the compiler can warn about missing cases } - if (warning.empty()) { - warning = create_warning; - } else if (!warning.empty() && !create_warning.empty()){ - warning += "; " + create_warning; - } - UniValue obj(UniValue::VOBJ); obj.pushKV("name", wallet->GetName()); - obj.pushKV("warning", warning); + obj.pushKV("warning", Join(warnings, "\n")); return obj; } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 09f08220d..d38c261f1 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -140,16 +140,16 @@ void UnloadWallet(std::shared_ptr&& wallet) } } -std::shared_ptr LoadWallet(interfaces::Chain& chain, const WalletLocation& location, std::string& error, std::string& warning) +std::shared_ptr LoadWallet(interfaces::Chain& chain, const WalletLocation& location, std::string& error, std::vector& warnings) { - if (!CWallet::Verify(chain, location, false, error, warning)) { + if (!CWallet::Verify(chain, location, false, error, warnings)) { error = "Wallet file verification failed: " + error; return nullptr; } - std::shared_ptr wallet = CWallet::CreateWalletFromFile(chain, location); + std::shared_ptr wallet = CWallet::CreateWalletFromFile(chain, location, error, warnings); if (!wallet) { - error = "Wallet loading failed."; + error = "Wallet loading failed: " + error; return nullptr; } AddWallet(wallet); @@ -157,12 +157,12 @@ std::shared_ptr LoadWallet(interfaces::Chain& chain, const WalletLocati return wallet; } -std::shared_ptr LoadWallet(interfaces::Chain& chain, const std::string& name, std::string& error, std::string& warning) +std::shared_ptr LoadWallet(interfaces::Chain& chain, const std::string& name, std::string& error, std::vector& warnings) { - return LoadWallet(chain, WalletLocation(name), error, warning); + return LoadWallet(chain, WalletLocation(name), error, warnings); } -WalletCreationStatus CreateWallet(interfaces::Chain& chain, const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, std::string& error, std::string& warning, std::shared_ptr& result) +WalletCreationStatus CreateWallet(interfaces::Chain& chain, const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, std::string& error, std::vector& warnings, std::shared_ptr& result) { // Indicate that the wallet is actually supposed to be blank and not just blank to make it encrypted bool create_blank = (wallet_creation_flags & WALLET_FLAG_BLANK_WALLET); @@ -180,9 +180,8 @@ WalletCreationStatus CreateWallet(interfaces::Chain& chain, const SecureString& } // Wallet::Verify will check if we're trying to create a wallet with a duplicate name. - std::string wallet_error; - if (!CWallet::Verify(chain, location, false, wallet_error, warning)) { - error = "Wallet file verification failed: " + wallet_error; + if (!CWallet::Verify(chain, location, false, error, warnings)) { + error = "Wallet file verification failed: " + error; return WalletCreationStatus::CREATION_FAILED; } @@ -193,9 +192,9 @@ WalletCreationStatus CreateWallet(interfaces::Chain& chain, const SecureString& } // Make the wallet - std::shared_ptr wallet = CWallet::CreateWalletFromFile(chain, location, wallet_creation_flags); + std::shared_ptr wallet = CWallet::CreateWalletFromFile(chain, location, error, warnings, wallet_creation_flags); if (!wallet) { - error = "Wallet creation failed"; + error = "Wallet creation failed: " + error; return WalletCreationStatus::CREATION_FAILED; } @@ -4196,7 +4195,7 @@ void CWallet::MarkPreSplitKeys() } } -bool CWallet::Verify(interfaces::Chain& chain, const WalletLocation& location, bool salvage_wallet, std::string& error_string, std::string& warning_string) +bool CWallet::Verify(interfaces::Chain& chain, const WalletLocation& location, bool salvage_wallet, std::string& error_string, std::vector& warnings) { // Do some checking on wallet path. It should be either a: // @@ -4250,10 +4249,10 @@ bool CWallet::Verify(interfaces::Chain& chain, const WalletLocation& location, b } } - return WalletBatch::VerifyDatabaseFile(wallet_path, warning_string, error_string); + return WalletBatch::VerifyDatabaseFile(wallet_path, warnings, error_string); } -std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, const WalletLocation& location, uint64_t wallet_creation_flags) +std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, const WalletLocation& location, std::string& error, std::vector& warnings, uint64_t wallet_creation_flags) { const std::string walletFile = WalletDataFilePath(location.GetPath()).string(); @@ -4266,7 +4265,7 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, std::unique_ptr tempWallet = MakeUnique(&chain, location, WalletDatabase::Create(location.GetPath())); DBErrors nZapWalletRet = tempWallet->ZapWalletTx(vWtx); if (nZapWalletRet != DBErrors::LOAD_OK) { - chain.initError(strprintf(_("Error loading %s: Wallet corrupted").translated, walletFile)); + error = strprintf(_("Error loading %s: Wallet corrupted").translated, walletFile); return nullptr; } } @@ -4279,29 +4278,28 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, // should be possible to use std::allocate_shared. std::shared_ptr walletInstance(new CWallet(&chain, location, WalletDatabase::Create(location.GetPath())), ReleaseWallet); DBErrors nLoadWalletRet = walletInstance->LoadWallet(fFirstRun); - if (nLoadWalletRet != DBErrors::LOAD_OK) - { + if (nLoadWalletRet != DBErrors::LOAD_OK) { if (nLoadWalletRet == DBErrors::CORRUPT) { - chain.initError(strprintf(_("Error loading %s: Wallet corrupted").translated, walletFile)); + error = strprintf(_("Error loading %s: Wallet corrupted").translated, walletFile); return nullptr; } else if (nLoadWalletRet == DBErrors::NONCRITICAL_ERROR) { - chain.initWarning(strprintf(_("Error reading %s! All keys read correctly, but transaction data" + warnings.push_back(strprintf(_("Error reading %s! All keys read correctly, but transaction data" " or address book entries might be missing or incorrect.").translated, walletFile)); } else if (nLoadWalletRet == DBErrors::TOO_NEW) { - chain.initError(strprintf(_("Error loading %s: Wallet requires newer version of %s").translated, walletFile, PACKAGE_NAME)); + error = strprintf(_("Error loading %s: Wallet requires newer version of %s").translated, walletFile, PACKAGE_NAME); return nullptr; } else if (nLoadWalletRet == DBErrors::NEED_REWRITE) { - chain.initError(strprintf(_("Wallet needed to be rewritten: restart %s to complete").translated, PACKAGE_NAME)); + error = strprintf(_("Wallet needed to be rewritten: restart %s to complete").translated, PACKAGE_NAME); return nullptr; } else { - chain.initError(strprintf(_("Error loading %s").translated, walletFile)); + error = strprintf(_("Error loading %s").translated, walletFile); return nullptr; } } @@ -4320,7 +4318,7 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, walletInstance->WalletLogPrintf("Allowing wallet upgrade up to %i\n", nMaxVersion); if (nMaxVersion < walletInstance->GetVersion()) { - chain.initError(_("Cannot downgrade wallet").translated); + error = _("Cannot downgrade wallet").translated; return nullptr; } walletInstance->SetMaxVersion(nMaxVersion); @@ -4333,7 +4331,7 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, // Do not upgrade versions to any version between HD_SPLIT and FEATURE_PRE_SPLIT_KEYPOOL unless already supporting HD_SPLIT int max_version = walletInstance->GetVersion(); if (!walletInstance->CanSupportFeature(FEATURE_HD_SPLIT) && max_version >= FEATURE_HD_SPLIT && max_version < FEATURE_PRE_SPLIT_KEYPOOL) { - chain.initError(_("Cannot upgrade a non HD split wallet without upgrading to support pre split keypool. Please use -upgradewallet=169900 or -upgradewallet with no version specified.").translated); + error = _("Cannot upgrade a non HD split wallet without upgrading to support pre split keypool. Please use -upgradewallet=169900 or -upgradewallet with no version specified.").translated; return nullptr; } @@ -4361,7 +4359,7 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, // Regenerate the keypool if upgraded to HD if (hd_upgrade) { if (!walletInstance->TopUpKeyPool()) { - chain.initError(_("Unable to generate keys").translated); + error = _("Unable to generate keys").translated; return nullptr; } } @@ -4381,7 +4379,7 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, // Top up the keypool if (walletInstance->CanGenerateKeys() && !walletInstance->TopUpKeyPool()) { - chain.initError(_("Unable to generate initial keys").translated); + error = _("Unable to generate initial keys").translated; return nullptr; } @@ -4389,33 +4387,33 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, walletInstance->ChainStateFlushed(locked_chain->getTipLocator()); } else if (wallet_creation_flags & WALLET_FLAG_DISABLE_PRIVATE_KEYS) { // Make it impossible to disable private keys after creation - chain.initError(strprintf(_("Error loading %s: Private keys can only be disabled during creation").translated, walletFile)); + error = strprintf(_("Error loading %s: Private keys can only be disabled during creation").translated, walletFile); return NULL; } else if (walletInstance->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { LOCK(walletInstance->cs_KeyStore); if (!walletInstance->mapKeys.empty() || !walletInstance->mapCryptedKeys.empty()) { - chain.initWarning(strprintf(_("Warning: Private keys detected in wallet {%s} with disabled private keys").translated, walletFile)); + warnings.push_back(strprintf(_("Warning: Private keys detected in wallet {%s} with disabled private keys").translated, walletFile)); } } if (!gArgs.GetArg("-addresstype", "").empty() && !ParseOutputType(gArgs.GetArg("-addresstype", ""), walletInstance->m_default_address_type)) { - chain.initError(strprintf(_("Unknown address type '%s'").translated, gArgs.GetArg("-addresstype", ""))); + error = strprintf(_("Unknown address type '%s'").translated, gArgs.GetArg("-addresstype", "")); return nullptr; } if (!gArgs.GetArg("-changetype", "").empty() && !ParseOutputType(gArgs.GetArg("-changetype", ""), walletInstance->m_default_change_type)) { - chain.initError(strprintf(_("Unknown change type '%s'").translated, gArgs.GetArg("-changetype", ""))); + error = strprintf(_("Unknown change type '%s'").translated, gArgs.GetArg("-changetype", "")); return nullptr; } if (gArgs.IsArgSet("-mintxfee")) { CAmount n = 0; if (!ParseMoney(gArgs.GetArg("-mintxfee", ""), n) || 0 == n) { - chain.initError(AmountErrMsg("mintxfee", gArgs.GetArg("-mintxfee", "")).translated); + error = AmountErrMsg("mintxfee", gArgs.GetArg("-mintxfee", "")).translated; return nullptr; } if (n > HIGH_TX_FEE_PER_KB) { - chain.initWarning(AmountHighWarn("-mintxfee").translated + " " + + warnings.push_back(AmountHighWarn("-mintxfee").translated + " " + _("This is the minimum transaction fee you pay on every transaction.").translated); } walletInstance->m_min_fee = CFeeRate(n); @@ -4424,11 +4422,11 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, if (gArgs.IsArgSet("-fallbackfee")) { CAmount nFeePerK = 0; if (!ParseMoney(gArgs.GetArg("-fallbackfee", ""), nFeePerK)) { - chain.initError(strprintf(_("Invalid amount for -fallbackfee=: '%s'").translated, gArgs.GetArg("-fallbackfee", ""))); + error = strprintf(_("Invalid amount for -fallbackfee=: '%s'").translated, gArgs.GetArg("-fallbackfee", "")); return nullptr; } if (nFeePerK > HIGH_TX_FEE_PER_KB) { - chain.initWarning(AmountHighWarn("-fallbackfee").translated + " " + + warnings.push_back(AmountHighWarn("-fallbackfee").translated + " " + _("This is the transaction fee you may pay when fee estimates are not available.").translated); } walletInstance->m_fallback_fee = CFeeRate(nFeePerK); @@ -4439,11 +4437,11 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, if (gArgs.IsArgSet("-discardfee")) { CAmount nFeePerK = 0; if (!ParseMoney(gArgs.GetArg("-discardfee", ""), nFeePerK)) { - chain.initError(strprintf(_("Invalid amount for -discardfee=: '%s'").translated, gArgs.GetArg("-discardfee", ""))); + error = strprintf(_("Invalid amount for -discardfee=: '%s'").translated, gArgs.GetArg("-discardfee", "")); return nullptr; } if (nFeePerK > HIGH_TX_FEE_PER_KB) { - chain.initWarning(AmountHighWarn("-discardfee").translated + " " + + warnings.push_back(AmountHighWarn("-discardfee").translated + " " + _("This is the transaction fee you may discard if change is smaller than dust at this level").translated); } walletInstance->m_discard_rate = CFeeRate(nFeePerK); @@ -4451,41 +4449,40 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, if (gArgs.IsArgSet("-paytxfee")) { CAmount nFeePerK = 0; if (!ParseMoney(gArgs.GetArg("-paytxfee", ""), nFeePerK)) { - chain.initError(AmountErrMsg("paytxfee", gArgs.GetArg("-paytxfee", "")).translated); + error = AmountErrMsg("paytxfee", gArgs.GetArg("-paytxfee", "")).translated; return nullptr; } if (nFeePerK > HIGH_TX_FEE_PER_KB) { - chain.initWarning(AmountHighWarn("-paytxfee").translated + " " + + warnings.push_back(AmountHighWarn("-paytxfee").translated + " " + _("This is the transaction fee you will pay if you send a transaction.").translated); } walletInstance->m_pay_tx_fee = CFeeRate(nFeePerK, 1000); if (walletInstance->m_pay_tx_fee < chain.relayMinFee()) { - chain.initError(strprintf(_("Invalid amount for -paytxfee=: '%s' (must be at least %s)").translated, - gArgs.GetArg("-paytxfee", ""), chain.relayMinFee().ToString())); + error = strprintf(_("Invalid amount for -paytxfee=: '%s' (must be at least %s)").translated, + gArgs.GetArg("-paytxfee", ""), chain.relayMinFee().ToString()); return nullptr; } } - if (gArgs.IsArgSet("-maxtxfee")) - { + if (gArgs.IsArgSet("-maxtxfee")) { CAmount nMaxFee = 0; if (!ParseMoney(gArgs.GetArg("-maxtxfee", ""), nMaxFee)) { - chain.initError(AmountErrMsg("maxtxfee", gArgs.GetArg("-maxtxfee", "")).translated); + error = AmountErrMsg("maxtxfee", gArgs.GetArg("-maxtxfee", "")).translated; return nullptr; } if (nMaxFee > HIGH_MAX_TX_FEE) { - chain.initWarning(_("-maxtxfee is set very high! Fees this large could be paid on a single transaction.").translated); + warnings.push_back(_("-maxtxfee is set very high! Fees this large could be paid on a single transaction.").translated); } if (CFeeRate(nMaxFee, 1000) < chain.relayMinFee()) { - chain.initError(strprintf(_("Invalid amount for -maxtxfee=: '%s' (must be at least the minrelay fee of %s to prevent stuck transactions)").translated, - gArgs.GetArg("-maxtxfee", ""), chain.relayMinFee().ToString())); + error = strprintf(_("Invalid amount for -maxtxfee=: '%s' (must be at least the minrelay fee of %s to prevent stuck transactions)").translated, + gArgs.GetArg("-maxtxfee", ""), chain.relayMinFee().ToString()); return nullptr; } walletInstance->m_default_max_tx_fee = nMaxFee; } if (chain.relayMinFee().GetFeePerK() > HIGH_TX_FEE_PER_KB) { - chain.initWarning(AmountHighWarn("-minrelaytxfee").translated + " " + + warnings.push_back(AmountHighWarn("-minrelaytxfee").translated + " " + _("The wallet will avoid paying less than the minimum relay fee.").translated); } @@ -4535,7 +4532,7 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, } if (rescan_height != block_height) { - chain.initError(_("Prune: last wallet synchronisation goes beyond pruned data. You need to -reindex (download the whole blockchain again in case of pruned node)").translated); + error = _("Prune: last wallet synchronisation goes beyond pruned data. You need to -reindex (download the whole blockchain again in case of pruned node)").translated; return nullptr; } } @@ -4554,7 +4551,7 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, { WalletRescanReserver reserver(walletInstance.get()); if (!reserver.reserve() || (ScanResult::SUCCESS != walletInstance->ScanForWalletTransactions(locked_chain->getBlockHash(rescan_height), {} /* stop block */, reserver, true /* update */).status)) { - chain.initError(_("Failed to rescan the wallet during initialization").translated); + error = _("Failed to rescan the wallet during initialization").translated; return nullptr; } } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 006775e83..a301338b6 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -48,7 +48,7 @@ bool RemoveWallet(const std::shared_ptr& wallet); bool HasWallets(); std::vector> GetWallets(); std::shared_ptr GetWallet(const std::string& name); -std::shared_ptr LoadWallet(interfaces::Chain& chain, const WalletLocation& location, std::string& error, std::string& warning); +std::shared_ptr LoadWallet(interfaces::Chain& chain, const WalletLocation& location, std::string& error, std::vector& warnings); enum class WalletCreationStatus { SUCCESS, @@ -56,7 +56,7 @@ enum class WalletCreationStatus { ENCRYPTION_FAILED }; -WalletCreationStatus CreateWallet(interfaces::Chain& chain, const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, std::string& error, std::string& warning, std::shared_ptr& result); +WalletCreationStatus CreateWallet(interfaces::Chain& chain, const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, std::string& error, std::vector& warnings, std::shared_ptr& result); //! Default for -keypool static const unsigned int DEFAULT_KEYPOOL_SIZE = 1000; @@ -1321,10 +1321,10 @@ public: bool MarkReplaced(const uint256& originalHash, const uint256& newHash); //! Verify wallet naming and perform salvage on the wallet if required - static bool Verify(interfaces::Chain& chain, const WalletLocation& location, bool salvage_wallet, std::string& error_string, std::string& warning_string); + static bool Verify(interfaces::Chain& chain, const WalletLocation& location, bool salvage_wallet, std::string& error_string, std::vector& warnings); /* Initializes the wallet, returns a new CWallet instance or a null pointer in case of an error */ - static std::shared_ptr CreateWalletFromFile(interfaces::Chain& chain, const WalletLocation& location, uint64_t wallet_creation_flags = 0); + static std::shared_ptr CreateWalletFromFile(interfaces::Chain& chain, const WalletLocation& location, std::string& error, std::vector& warnings, uint64_t wallet_creation_flags = 0); /** * Wallet post-init setup diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 635997afc..d7f7635a8 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -729,9 +729,9 @@ bool WalletBatch::VerifyEnvironment(const fs::path& wallet_path, std::string& er return BerkeleyBatch::VerifyEnvironment(wallet_path, errorStr); } -bool WalletBatch::VerifyDatabaseFile(const fs::path& wallet_path, std::string& warningStr, std::string& errorStr) +bool WalletBatch::VerifyDatabaseFile(const fs::path& wallet_path, std::vector& warnings, std::string& errorStr) { - return BerkeleyBatch::VerifyDatabaseFile(wallet_path, warningStr, errorStr, WalletBatch::Recover); + return BerkeleyBatch::VerifyDatabaseFile(wallet_path, warnings, errorStr, WalletBatch::Recover); } bool WalletBatch::WriteDestData(const std::string &address, const std::string &key, const std::string &value) diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index 0fee35934..32475e4d2 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -263,7 +263,7 @@ public: /* verifies the database environment */ static bool VerifyEnvironment(const fs::path& wallet_path, std::string& errorStr); /* verifies the database file */ - static bool VerifyDatabaseFile(const fs::path& wallet_path, std::string& warningStr, std::string& errorStr); + static bool VerifyDatabaseFile(const fs::path& wallet_path, std::vector& warnings, std::string& errorStr); //! write the hdchain model (external chain child index counter) bool WriteHDChain(const CHDChain& chain); diff --git a/test/functional/wallet_multiwallet.py b/test/functional/wallet_multiwallet.py index 68bc45f98..ce0b7e878 100755 --- a/test/functional/wallet_multiwallet.py +++ b/test/functional/wallet_multiwallet.py @@ -339,14 +339,10 @@ class MultiWalletTest(BitcoinTestFramework): self.log.info("Fail -upgradewallet that results in downgrade") assert_raises_rpc_error( -4, - "Wallet loading failed.", + 'Wallet loading failed: Error loading {}: Wallet requires newer version of {}'.format( + wallet_dir('high_minversion', 'wallet.dat'), self.config['environment']['PACKAGE_NAME']), lambda: self.nodes[0].loadwallet(filename='high_minversion'), ) - self.stop_node( - i=0, - expected_stderr='Error: Error loading {}: Wallet requires newer version of Bitcoin Core'.format( - wallet_dir('high_minversion', 'wallet.dat')), - ) if __name__ == '__main__':