diff options
42 files changed, 795 insertions, 711 deletions
diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 7909cb4a0f..03cd9133c8 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -32,6 +32,7 @@ FUZZ_TARGETS = \ test/fuzz/checkqueue \ test/fuzz/coins_deserialize \ test/fuzz/coins_view \ + test/fuzz/crypto_common \ test/fuzz/cuckoocache \ test/fuzz/decode_tx \ test/fuzz/descriptor_parse \ @@ -478,6 +479,12 @@ test_fuzz_coins_view_LDADD = $(FUZZ_SUITE_LD_COMMON) test_fuzz_coins_view_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) test_fuzz_coins_view_SOURCES = test/fuzz/coins_view.cpp +test_fuzz_crypto_common_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) +test_fuzz_crypto_common_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) +test_fuzz_crypto_common_LDADD = $(FUZZ_SUITE_LD_COMMON) +test_fuzz_crypto_common_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) +test_fuzz_crypto_common_SOURCES = test/fuzz/crypto_common.cpp + test_fuzz_cuckoocache_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) test_fuzz_cuckoocache_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) test_fuzz_cuckoocache_LDADD = $(FUZZ_SUITE_LD_COMMON) diff --git a/src/index/base.cpp b/src/index/base.cpp index 74ea421e13..1d09f2e577 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -21,7 +21,7 @@ template<typename... Args> static void FatalError(const char* fmt, const Args&... args) { std::string strMessage = tfm::format(fmt, args...); - SetMiscWarning(strMessage); + SetMiscWarning(Untranslated(strMessage)); LogPrintf("*** %s\n", strMessage); uiInterface.ThreadSafeMessageBox( Untranslated("Error: A fatal internal error occurred, see debug.log for details"), diff --git a/src/init.cpp b/src/init.cpp index 56d75c90c7..fd7c8d0f80 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1465,7 +1465,7 @@ bool AppInitMain(const util::Ref& context, NodeContext& node) if (Lookup(strAddr, addrLocal, GetListenPort(), fNameLookup) && addrLocal.IsValid()) AddLocal(addrLocal, LOCAL_MANUAL); else - return InitError(Untranslated(ResolveErrMsg("externalip", strAddr))); + return InitError(ResolveErrMsg("externalip", strAddr)); } // Read asmap file if configured @@ -1904,21 +1904,21 @@ bool AppInitMain(const util::Ref& context, NodeContext& node) for (const std::string& strBind : gArgs.GetArgs("-bind")) { CService addrBind; if (!Lookup(strBind, addrBind, GetListenPort(), false)) { - return InitError(Untranslated(ResolveErrMsg("bind", strBind))); + return InitError(ResolveErrMsg("bind", strBind)); } connOptions.vBinds.push_back(addrBind); } for (const std::string& strBind : gArgs.GetArgs("-whitebind")) { NetWhitebindPermissions whitebind; - std::string error; - if (!NetWhitebindPermissions::TryParse(strBind, whitebind, error)) return InitError(Untranslated(error)); + bilingual_str error; + if (!NetWhitebindPermissions::TryParse(strBind, whitebind, error)) return InitError(error); connOptions.vWhiteBinds.push_back(whitebind); } for (const auto& net : gArgs.GetArgs("-whitelist")) { NetWhitelistPermissions subnet; - std::string error; - if (!NetWhitelistPermissions::TryParse(net, subnet, error)) return InitError(Untranslated(error)); + bilingual_str error; + if (!NetWhitelistPermissions::TryParse(net, subnet, error)) return InitError(error); connOptions.vWhitelistedRange.push_back(subnet); } diff --git a/src/interfaces/node.cpp b/src/interfaces/node.cpp index bd1b36fea5..d420788dbe 100644 --- a/src/interfaces/node.cpp +++ b/src/interfaces/node.cpp @@ -56,7 +56,7 @@ namespace { class NodeImpl : public Node { public: - void initError(const std::string& message) override { InitError(Untranslated(message)); } + void initError(const bilingual_str& message) override { InitError(message); } bool parseParameters(int argc, const char* const argv[], std::string& error) override { return gArgs.ParseParameters(argc, argv, error); @@ -71,7 +71,7 @@ public: std::string getNetwork() override { return Params().NetworkIDString(); } void initLogging() override { InitLogging(); } void initParameterInteraction() override { InitParameterInteraction(); } - std::string getWarnings() override { return GetWarnings(true); } + bilingual_str getWarnings() override { return GetWarnings(true); } uint32_t getLogCategories() override { return LogInstance().GetCategoryMask(); } bool baseInitialize() override { diff --git a/src/interfaces/node.h b/src/interfaces/node.h index 0b7fb6736a..877a40568f 100644 --- a/src/interfaces/node.h +++ b/src/interfaces/node.h @@ -10,6 +10,7 @@ #include <net_types.h> // For banmap_t #include <netaddress.h> // For Network #include <support/allocators/secure.h> // For SecureString +#include <util/translation.h> #include <functional> #include <memory> @@ -45,7 +46,7 @@ public: virtual ~Node() {} //! Send init error. - virtual void initError(const std::string& message) = 0; + virtual void initError(const bilingual_str& message) = 0; //! Set command line arguments. virtual bool parseParameters(int argc, const char* const argv[], std::string& error) = 0; @@ -81,7 +82,7 @@ public: virtual void initParameterInteraction() = 0; //! Get warnings. - virtual std::string getWarnings() = 0; + virtual bilingual_str getWarnings() = 0; // Get log flags. virtual uint32_t getLogCategories() = 0; diff --git a/src/net_permissions.cpp b/src/net_permissions.cpp index d76a512a6c..da09149856 100644 --- a/src/net_permissions.cpp +++ b/src/net_permissions.cpp @@ -16,8 +16,10 @@ const std::vector<std::string> NET_PERMISSIONS_DOC{ "mempool (allow requesting BIP35 mempool contents)", }; +namespace { + // The parse the following format "perm1,perm2@xxxxxx" -bool TryParsePermissionFlags(const std::string str, NetPermissionFlags& output, size_t& readen, std::string& error) +bool TryParsePermissionFlags(const std::string str, NetPermissionFlags& output, size_t& readen, bilingual_str& error) { NetPermissionFlags flags = PF_NONE; const auto atSeparator = str.find('@'); @@ -48,7 +50,7 @@ bool TryParsePermissionFlags(const std::string str, NetPermissionFlags& output, else if (permission == "relay") NetPermissions::AddFlag(flags, PF_RELAY); else if (permission.length() == 0); // Allow empty entries else { - error = strprintf(_("Invalid P2P permission: '%s'").translated, permission); + error = strprintf(_("Invalid P2P permission: '%s'"), permission); return false; } } @@ -56,10 +58,12 @@ bool TryParsePermissionFlags(const std::string str, NetPermissionFlags& output, } output = flags; - error = ""; + error = Untranslated(""); return true; } +} + std::vector<std::string> NetPermissions::ToStrings(NetPermissionFlags flags) { std::vector<std::string> strings; @@ -71,7 +75,7 @@ std::vector<std::string> NetPermissions::ToStrings(NetPermissionFlags flags) return strings; } -bool NetWhitebindPermissions::TryParse(const std::string str, NetWhitebindPermissions& output, std::string& error) +bool NetWhitebindPermissions::TryParse(const std::string str, NetWhitebindPermissions& output, bilingual_str& error) { NetPermissionFlags flags; size_t offset; @@ -84,17 +88,17 @@ bool NetWhitebindPermissions::TryParse(const std::string str, NetWhitebindPermis return false; } if (addrBind.GetPort() == 0) { - error = strprintf(_("Need to specify a port with -whitebind: '%s'").translated, strBind); + error = strprintf(_("Need to specify a port with -whitebind: '%s'"), strBind); return false; } output.m_flags = flags; output.m_service = addrBind; - error = ""; + error = Untranslated(""); return true; } -bool NetWhitelistPermissions::TryParse(const std::string str, NetWhitelistPermissions& output, std::string& error) +bool NetWhitelistPermissions::TryParse(const std::string str, NetWhitelistPermissions& output, bilingual_str& error) { NetPermissionFlags flags; size_t offset; @@ -104,12 +108,12 @@ bool NetWhitelistPermissions::TryParse(const std::string str, NetWhitelistPermis CSubNet subnet; LookupSubNet(net, subnet); if (!subnet.IsValid()) { - error = strprintf(_("Invalid netmask specified in -whitelist: '%s'").translated, net); + error = strprintf(_("Invalid netmask specified in -whitelist: '%s'"), net); return false; } output.m_flags = flags; output.m_subnet = subnet; - error = ""; + error = Untranslated(""); return true; } diff --git a/src/net_permissions.h b/src/net_permissions.h index d35c5ee0cd..e004067e75 100644 --- a/src/net_permissions.h +++ b/src/net_permissions.h @@ -10,6 +10,8 @@ #ifndef BITCOIN_NET_PERMISSIONS_H #define BITCOIN_NET_PERMISSIONS_H +struct bilingual_str; + extern const std::vector<std::string> NET_PERMISSIONS_DOC; enum NetPermissionFlags @@ -54,14 +56,14 @@ public: class NetWhitebindPermissions : public NetPermissions { public: - static bool TryParse(const std::string str, NetWhitebindPermissions& output, std::string& error); + static bool TryParse(const std::string str, NetWhitebindPermissions& output, bilingual_str& error); CService m_service; }; class NetWhitelistPermissions : public NetPermissions { public: - static bool TryParse(const std::string str, NetWhitelistPermissions& output, std::string& error); + static bool TryParse(const std::string str, NetWhitelistPermissions& output, bilingual_str& error); CSubNet m_subnet; }; diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index 3e7a3e9271..e0b9345a32 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -33,6 +33,7 @@ #include <ui_interface.h> #include <uint256.h> #include <util/system.h> +#include <util/translation.h> #include <util/threadnames.h> #include <validation.h> @@ -154,7 +155,7 @@ BitcoinCore::BitcoinCore(interfaces::Node& node) : void BitcoinCore::handleRunawayException(const std::exception *e) { PrintExceptionContinue(e, "Runaway exception"); - Q_EMIT runawayException(QString::fromStdString(m_node.getWarnings())); + Q_EMIT runawayException(QString::fromStdString(m_node.getWarnings().translated)); } void BitcoinCore::initialize() @@ -457,7 +458,7 @@ int GuiMain(int argc, char* argv[]) SetupUIArgs(); std::string error; if (!node->parseParameters(argc, argv, error)) { - node->initError(strprintf("Error parsing command line arguments: %s\n", error)); + node->initError(strprintf(Untranslated("Error parsing command line arguments: %s\n"), error)); // Create a message box, because the gui has neither been created nor has subscribed to core signals QMessageBox::critical(nullptr, PACKAGE_NAME, // message can not be translated because translations have not been initialized @@ -498,13 +499,13 @@ int GuiMain(int argc, char* argv[]) /// 6. Determine availability of data directory and parse bitcoin.conf /// - Do not call GetDataDir(true) before this step finishes if (!CheckDataDirOption()) { - node->initError(strprintf("Specified data directory \"%s\" does not exist.\n", gArgs.GetArg("-datadir", ""))); + node->initError(strprintf(Untranslated("Specified data directory \"%s\" does not exist.\n"), gArgs.GetArg("-datadir", ""))); QMessageBox::critical(nullptr, PACKAGE_NAME, QObject::tr("Error: Specified data directory \"%1\" does not exist.").arg(QString::fromStdString(gArgs.GetArg("-datadir", "")))); return EXIT_FAILURE; } if (!node->readConfigFiles(error)) { - node->initError(strprintf("Error reading configuration file: %s\n", error)); + node->initError(strprintf(Untranslated("Error reading configuration file: %s\n"), error)); QMessageBox::critical(nullptr, PACKAGE_NAME, QObject::tr("Error: Cannot parse configuration file: %1.").arg(QString::fromStdString(error))); return EXIT_FAILURE; @@ -520,7 +521,7 @@ int GuiMain(int argc, char* argv[]) try { node->selectParams(gArgs.GetChainName()); } catch(std::exception &e) { - node->initError(strprintf("%s\n", e.what())); + node->initError(Untranslated(strprintf("%s\n", e.what()))); QMessageBox::critical(nullptr, PACKAGE_NAME, QObject::tr("Error: %1").arg(e.what())); return EXIT_FAILURE; } @@ -598,10 +599,10 @@ int GuiMain(int argc, char* argv[]) } } catch (const std::exception& e) { PrintExceptionContinue(&e, "Runaway exception"); - app.handleRunawayException(QString::fromStdString(node->getWarnings())); + app.handleRunawayException(QString::fromStdString(node->getWarnings().translated)); } catch (...) { PrintExceptionContinue(nullptr, "Runaway exception"); - app.handleRunawayException(QString::fromStdString(node->getWarnings())); + app.handleRunawayException(QString::fromStdString(node->getWarnings().translated)); } return rv; } diff --git a/src/qt/clientmodel.cpp b/src/qt/clientmodel.cpp index c1d358f7f6..7822d4c5f3 100644 --- a/src/qt/clientmodel.cpp +++ b/src/qt/clientmodel.cpp @@ -166,7 +166,7 @@ enum BlockSource ClientModel::getBlockSource() const QString ClientModel::getStatusBarWarnings() const { - return QString::fromStdString(m_node.getWarnings()); + return QString::fromStdString(m_node.getWarnings().translated); } OptionsModel *ClientModel::getOptionsModel() diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index eedb1c3d71..8252af3ee6 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -32,6 +32,7 @@ #include <util/ref.h> #include <util/strencodings.h> #include <util/system.h> +#include <util/translation.h> #include <validation.h> #include <validationinterface.h> #include <warnings.h> @@ -1278,7 +1279,7 @@ UniValue getblockchaininfo(const JSONRPCRequest& request) BIP9SoftForkDescPushBack(softforks, "testdummy", consensusParams, Consensus::DEPLOYMENT_TESTDUMMY); obj.pushKV("softforks", softforks); - obj.pushKV("warnings", GetWarnings(false)); + obj.pushKV("warnings", GetWarnings(false).original); return obj; } diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 3db0cb04ed..a2de175c85 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -29,6 +29,7 @@ #include <util/strencodings.h> #include <util/string.h> #include <util/system.h> +#include <util/translation.h> #include <validation.h> #include <validationinterface.h> #include <versionbitsinfo.h> @@ -416,7 +417,7 @@ static UniValue getmininginfo(const JSONRPCRequest& request) obj.pushKV("networkhashps", getnetworkhashps(request)); obj.pushKV("pooledtx", (uint64_t)mempool.size()); obj.pushKV("chain", Params().NetworkIDString()); - obj.pushKV("warnings", GetWarnings(false)); + obj.pushKV("warnings", GetWarnings(false).original); return obj; } diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index e29aa03695..df1e0fe623 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -22,6 +22,7 @@ #include <util/strencodings.h> #include <util/string.h> #include <util/system.h> +#include <util/translation.h> #include <validation.h> #include <version.h> #include <warnings.h> @@ -548,7 +549,7 @@ static UniValue getnetworkinfo(const JSONRPCRequest& request) } } obj.pushKV("localaddresses", localAddresses); - obj.pushKV("warnings", GetWarnings(false)); + obj.pushKV("warnings", GetWarnings(false).original); return obj; } diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 39bf05fbbd..e7afef4cac 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -10,6 +10,7 @@ #include <tinyformat.h> #include <util/strencodings.h> #include <util/string.h> +#include <util/translation.h> #include <tuple> @@ -285,7 +286,7 @@ UniValue JSONRPCTransactionError(TransactionError terr, const std::string& err_s if (err_string.length() > 0) { return JSONRPCError(RPCErrorFromTransactionError(terr), err_string); } else { - return JSONRPCError(RPCErrorFromTransactionError(terr), TransactionErrorString(terr)); + return JSONRPCError(RPCErrorFromTransactionError(terr), TransactionErrorString(terr).original); } } diff --git a/src/test/fuzz/crypto_common.cpp b/src/test/fuzz/crypto_common.cpp new file mode 100644 index 0000000000..7ccb125216 --- /dev/null +++ b/src/test/fuzz/crypto_common.cpp @@ -0,0 +1,70 @@ +// Copyright (c) 2020 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include <crypto/common.h> +#include <test/fuzz/FuzzedDataProvider.h> +#include <test/fuzz/fuzz.h> +#include <test/fuzz/util.h> + +#include <array> +#include <cassert> +#include <cstdint> +#include <cstring> +#include <vector> + +void test_one_input(const std::vector<uint8_t>& buffer) +{ + FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; + const uint16_t random_u16 = fuzzed_data_provider.ConsumeIntegral<uint16_t>(); + const uint32_t random_u32 = fuzzed_data_provider.ConsumeIntegral<uint32_t>(); + const uint64_t random_u64 = fuzzed_data_provider.ConsumeIntegral<uint64_t>(); + const std::vector<uint8_t> random_bytes_2 = ConsumeFixedLengthByteVector(fuzzed_data_provider, 2); + const std::vector<uint8_t> random_bytes_4 = ConsumeFixedLengthByteVector(fuzzed_data_provider, 4); + const std::vector<uint8_t> random_bytes_8 = ConsumeFixedLengthByteVector(fuzzed_data_provider, 8); + + std::array<uint8_t, 2> writele16_arr; + WriteLE16(writele16_arr.data(), random_u16); + assert(ReadLE16(writele16_arr.data()) == random_u16); + + std::array<uint8_t, 4> writele32_arr; + WriteLE32(writele32_arr.data(), random_u32); + assert(ReadLE32(writele32_arr.data()) == random_u32); + + std::array<uint8_t, 8> writele64_arr; + WriteLE64(writele64_arr.data(), random_u64); + assert(ReadLE64(writele64_arr.data()) == random_u64); + + std::array<uint8_t, 4> writebe32_arr; + WriteBE32(writebe32_arr.data(), random_u32); + assert(ReadBE32(writebe32_arr.data()) == random_u32); + + std::array<uint8_t, 8> writebe64_arr; + WriteBE64(writebe64_arr.data(), random_u64); + assert(ReadBE64(writebe64_arr.data()) == random_u64); + + const uint16_t readle16_result = ReadLE16(random_bytes_2.data()); + std::array<uint8_t, 2> readle16_arr; + WriteLE16(readle16_arr.data(), readle16_result); + assert(std::memcmp(random_bytes_2.data(), readle16_arr.data(), 2) == 0); + + const uint32_t readle32_result = ReadLE32(random_bytes_4.data()); + std::array<uint8_t, 4> readle32_arr; + WriteLE32(readle32_arr.data(), readle32_result); + assert(std::memcmp(random_bytes_4.data(), readle32_arr.data(), 4) == 0); + + const uint64_t readle64_result = ReadLE64(random_bytes_8.data()); + std::array<uint8_t, 8> readle64_arr; + WriteLE64(readle64_arr.data(), readle64_result); + assert(std::memcmp(random_bytes_8.data(), readle64_arr.data(), 8) == 0); + + const uint32_t readbe32_result = ReadBE32(random_bytes_4.data()); + std::array<uint8_t, 4> readbe32_arr; + WriteBE32(readbe32_arr.data(), readbe32_result); + assert(std::memcmp(random_bytes_4.data(), readbe32_arr.data(), 4) == 0); + + const uint64_t readbe64_result = ReadBE64(random_bytes_8.data()); + std::array<uint8_t, 8> readbe64_arr; + WriteBE64(readbe64_arr.data(), readbe64_result); + assert(std::memcmp(random_bytes_8.data(), readbe64_arr.data(), 8) == 0); +} diff --git a/src/test/fuzz/kitchen_sink.cpp b/src/test/fuzz/kitchen_sink.cpp index af6dc71322..82cbc00a3a 100644 --- a/src/test/fuzz/kitchen_sink.cpp +++ b/src/test/fuzz/kitchen_sink.cpp @@ -7,6 +7,7 @@ #include <test/fuzz/fuzz.h> #include <test/fuzz/util.h> #include <util/error.h> +#include <util/translation.h> #include <cstdint> #include <vector> diff --git a/src/test/fuzz/net_permissions.cpp b/src/test/fuzz/net_permissions.cpp index c071283467..ae531f4462 100644 --- a/src/test/fuzz/net_permissions.cpp +++ b/src/test/fuzz/net_permissions.cpp @@ -6,6 +6,7 @@ #include <test/fuzz/FuzzedDataProvider.h> #include <test/fuzz/fuzz.h> #include <test/fuzz/util.h> +#include <util/translation.h> #include <cassert> #include <cstdint> @@ -29,7 +30,7 @@ void test_one_input(const std::vector<uint8_t>& buffer) static_cast<NetPermissionFlags>(fuzzed_data_provider.ConsumeIntegral<uint32_t>()); NetWhitebindPermissions net_whitebind_permissions; - std::string error_net_whitebind_permissions; + bilingual_str error_net_whitebind_permissions; if (NetWhitebindPermissions::TryParse(s, net_whitebind_permissions, error_net_whitebind_permissions)) { (void)NetPermissions::ToStrings(net_whitebind_permissions.m_flags); (void)NetPermissions::AddFlag(net_whitebind_permissions.m_flags, net_permission_flags); @@ -39,7 +40,7 @@ void test_one_input(const std::vector<uint8_t>& buffer) } NetWhitelistPermissions net_whitelist_permissions; - std::string error_net_whitelist_permissions; + bilingual_str error_net_whitelist_permissions; if (NetWhitelistPermissions::TryParse(s, net_whitelist_permissions, error_net_whitelist_permissions)) { (void)NetPermissions::ToStrings(net_whitelist_permissions.m_flags); (void)NetPermissions::AddFlag(net_whitelist_permissions.m_flags, net_permission_flags); diff --git a/src/test/fuzz/util.h b/src/test/fuzz/util.h index f26878a704..1c1b2cd254 100644 --- a/src/test/fuzz/util.h +++ b/src/test/fuzz/util.h @@ -214,4 +214,18 @@ NODISCARD inline bool ContainsSpentInput(const CTransaction& tx, const CCoinsVie return false; } +/** + * Returns a byte vector of specified size regardless of the number of remaining bytes available + * from the fuzzer. Pads with zero value bytes if needed to achieve the specified size. + */ +NODISCARD inline std::vector<uint8_t> ConsumeFixedLengthByteVector(FuzzedDataProvider& fuzzed_data_provider, const size_t length) noexcept +{ + std::vector<uint8_t> result(length); + const std::vector<uint8_t> random_bytes = fuzzed_data_provider.ConsumeBytes<uint8_t>(length); + if (!random_bytes.empty()) { + std::memcpy(result.data(), random_bytes.data(), random_bytes.size()); + } + return result; +} + #endif // BITCOIN_TEST_FUZZ_UTIL_H diff --git a/src/test/netbase_tests.cpp b/src/test/netbase_tests.cpp index 2e1972cc3f..d0ec401f9d 100644 --- a/src/test/netbase_tests.cpp +++ b/src/test/netbase_tests.cpp @@ -6,6 +6,7 @@ #include <netbase.h> #include <test/util/setup_common.h> #include <util/strencodings.h> +#include <util/translation.h> #include <string> @@ -325,15 +326,15 @@ BOOST_AUTO_TEST_CASE(netbase_parsenetwork) BOOST_AUTO_TEST_CASE(netpermissions_test) { - std::string error; + bilingual_str error; NetWhitebindPermissions whitebindPermissions; NetWhitelistPermissions whitelistPermissions; // Detect invalid white bind BOOST_CHECK(!NetWhitebindPermissions::TryParse("", whitebindPermissions, error)); - BOOST_CHECK(error.find("Cannot resolve -whitebind address") != std::string::npos); + BOOST_CHECK(error.original.find("Cannot resolve -whitebind address") != std::string::npos); BOOST_CHECK(!NetWhitebindPermissions::TryParse("127.0.0.1", whitebindPermissions, error)); - BOOST_CHECK(error.find("Need to specify a port with -whitebind") != std::string::npos); + BOOST_CHECK(error.original.find("Need to specify a port with -whitebind") != std::string::npos); BOOST_CHECK(!NetWhitebindPermissions::TryParse("", whitebindPermissions, error)); // If no permission flags, assume backward compatibility @@ -377,11 +378,11 @@ BOOST_AUTO_TEST_CASE(netpermissions_test) // Detect invalid flag BOOST_CHECK(!NetWhitebindPermissions::TryParse("bloom,forcerelay,oopsie@1.2.3.4:32", whitebindPermissions, error)); - BOOST_CHECK(error.find("Invalid P2P permission") != std::string::npos); + BOOST_CHECK(error.original.find("Invalid P2P permission") != std::string::npos); // Check whitelist error BOOST_CHECK(!NetWhitelistPermissions::TryParse("bloom,forcerelay,noban@1.2.3.4:32", whitelistPermissions, error)); - BOOST_CHECK(error.find("Invalid netmask specified in -whitelist") != std::string::npos); + BOOST_CHECK(error.original.find("Invalid netmask specified in -whitelist") != std::string::npos); // Happy path for whitelist parsing BOOST_CHECK(NetWhitelistPermissions::TryParse("noban@1.2.3.4", whitelistPermissions, error)); diff --git a/src/test/timedata_tests.cpp b/src/test/timedata_tests.cpp index 8c880babd1..1dcee23bbb 100644 --- a/src/test/timedata_tests.cpp +++ b/src/test/timedata_tests.cpp @@ -9,6 +9,7 @@ #include <test/util/setup_common.h> #include <timedata.h> #include <util/string.h> +#include <util/translation.h> #include <warnings.h> #include <string> @@ -66,7 +67,7 @@ BOOST_AUTO_TEST_CASE(addtimedata) MultiAddTimeData(1, DEFAULT_MAX_TIME_ADJUSTMENT + 1); //filter size 5 } - BOOST_CHECK(GetWarnings(true).find("clock is wrong") != std::string::npos); + BOOST_CHECK(GetWarnings(true).original.find("clock is wrong") != std::string::npos); // nTimeOffset is not changed if the median of offsets exceeds DEFAULT_MAX_TIME_ADJUSTMENT BOOST_CHECK_EQUAL(GetTimeOffset(), 0); diff --git a/src/timedata.cpp b/src/timedata.cpp index c67a3c96e8..16dac24a48 100644 --- a/src/timedata.cpp +++ b/src/timedata.cpp @@ -95,7 +95,7 @@ void AddTimeData(const CNetAddr& ip, int64_t nOffsetSample) if (!fMatch) { fDone = true; bilingual_str strMessage = strprintf(_("Please check that your computer's date and time are correct! If your clock is wrong, %s will not work properly."), PACKAGE_NAME); - SetMiscWarning(strMessage.translated); + SetMiscWarning(strMessage); uiInterface.ThreadSafeMessageBox(strMessage, "", CClientUIInterface::MSG_WARNING); } } diff --git a/src/util/error.cpp b/src/util/error.cpp index 72a6e87cde..c4d9ffd037 100644 --- a/src/util/error.cpp +++ b/src/util/error.cpp @@ -8,37 +8,37 @@ #include <util/system.h> #include <util/translation.h> -std::string TransactionErrorString(const TransactionError err) +bilingual_str TransactionErrorString(const TransactionError err) { switch (err) { case TransactionError::OK: - return "No error"; + return Untranslated("No error"); case TransactionError::MISSING_INPUTS: - return "Missing inputs"; + return Untranslated("Missing inputs"); case TransactionError::ALREADY_IN_CHAIN: - return "Transaction already in block chain"; + return Untranslated("Transaction already in block chain"); case TransactionError::P2P_DISABLED: - return "Peer-to-peer functionality missing or disabled"; + return Untranslated("Peer-to-peer functionality missing or disabled"); case TransactionError::MEMPOOL_REJECTED: - return "Transaction rejected by AcceptToMemoryPool"; + return Untranslated("Transaction rejected by AcceptToMemoryPool"); case TransactionError::MEMPOOL_ERROR: - return "AcceptToMemoryPool failed"; + return Untranslated("AcceptToMemoryPool failed"); case TransactionError::INVALID_PSBT: - return "PSBT is not sane"; + return Untranslated("PSBT is not sane"); case TransactionError::PSBT_MISMATCH: - return "PSBTs not compatible (different transactions)"; + return Untranslated("PSBTs not compatible (different transactions)"); case TransactionError::SIGHASH_MISMATCH: - return "Specified sighash value does not match existing value"; + return Untranslated("Specified sighash value does not match existing value"); case TransactionError::MAX_FEE_EXCEEDED: - return "Fee exceeds maximum configured by -maxtxfee"; + return Untranslated("Fee exceeds maximum configured by -maxtxfee"); // no default case, so the compiler can warn about missing cases } assert(false); } -std::string ResolveErrMsg(const std::string& optname, const std::string& strBind) +bilingual_str ResolveErrMsg(const std::string& optname, const std::string& strBind) { - return strprintf(_("Cannot resolve -%s address: '%s'").translated, optname, strBind); + return strprintf(_("Cannot resolve -%s address: '%s'"), optname, strBind); } bilingual_str AmountHighWarn(const std::string& optname) diff --git a/src/util/error.h b/src/util/error.h index 61af88ddea..b9830c9eea 100644 --- a/src/util/error.h +++ b/src/util/error.h @@ -32,9 +32,9 @@ enum class TransactionError { MAX_FEE_EXCEEDED, }; -std::string TransactionErrorString(const TransactionError error); +bilingual_str TransactionErrorString(const TransactionError error); -std::string ResolveErrMsg(const std::string& optname, const std::string& strBind); +bilingual_str ResolveErrMsg(const std::string& optname, const std::string& strBind); bilingual_str AmountHighWarn(const std::string& optname); diff --git a/src/validation.cpp b/src/validation.cpp index 51d29dd869..cbe89443b8 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1666,7 +1666,7 @@ bool UndoReadFromDisk(CBlockUndo& blockundo, const CBlockIndex* pindex) // TODO: AbortNode() should take bilingual_str userMessage parameter. static bool AbortNode(const std::string& strMessage, const std::string& userMessage = "", unsigned int prefix = 0) { - SetMiscWarning(strMessage); + SetMiscWarning(Untranslated(strMessage)); LogPrintf("*** %s\n", strMessage); if (!userMessage.empty()) { uiInterface.ThreadSafeMessageBox(Untranslated(userMessage), "", CClientUIInterface::MSG_ERROR | prefix); @@ -2429,20 +2429,20 @@ void CChainState::PruneAndFlush() { } } -static void DoWarning(const std::string& strWarning) +static void DoWarning(const bilingual_str& warning) { static bool fWarned = false; - SetMiscWarning(strWarning); + SetMiscWarning(warning); if (!fWarned) { - AlertNotify(strWarning); + AlertNotify(warning.original); fWarned = true; } } /** Private helper function that concatenates warning messages. */ -static void AppendWarning(std::string& res, const std::string& warn) +static void AppendWarning(bilingual_str& res, const bilingual_str& warn) { - if (!res.empty()) res += ", "; + if (!res.empty()) res += Untranslated(", "); res += warn; } @@ -2459,7 +2459,7 @@ void static UpdateTip(const CBlockIndex* pindexNew, const CChainParams& chainPar g_best_block_cv.notify_all(); } - std::string warningMessages; + bilingual_str warning_messages; if (!::ChainstateActive().IsInitialBlockDownload()) { int nUpgraded = 0; @@ -2468,11 +2468,11 @@ void static UpdateTip(const CBlockIndex* pindexNew, const CChainParams& chainPar WarningBitsConditionChecker checker(bit); ThresholdState state = checker.GetStateFor(pindex, chainParams.GetConsensus(), warningcache[bit]); if (state == ThresholdState::ACTIVE || state == ThresholdState::LOCKED_IN) { - const std::string strWarning = strprintf(_("Warning: unknown new rules activated (versionbit %i)").translated, bit); + const bilingual_str warning = strprintf(_("Warning: unknown new rules activated (versionbit %i)"), bit); if (state == ThresholdState::ACTIVE) { - DoWarning(strWarning); + DoWarning(warning); } else { - AppendWarning(warningMessages, strWarning); + AppendWarning(warning_messages, warning); } } } @@ -2485,14 +2485,14 @@ void static UpdateTip(const CBlockIndex* pindexNew, const CChainParams& chainPar pindex = pindex->pprev; } if (nUpgraded > 0) - AppendWarning(warningMessages, strprintf(_("%d of last 100 blocks have unexpected version").translated, nUpgraded)); + AppendWarning(warning_messages, strprintf(_("%d of last 100 blocks have unexpected version"), nUpgraded)); } LogPrintf("%s: new best=%s height=%d version=0x%08x log2_work=%.8g tx=%lu date='%s' progress=%f cache=%.1fMiB(%utxo)%s\n", __func__, pindexNew->GetBlockHash().ToString(), pindexNew->nHeight, pindexNew->nVersion, log(pindexNew->nChainWork.getdouble())/log(2.0), (unsigned long)pindexNew->nChainTx, FormatISO8601DateTime(pindexNew->GetBlockTime()), GuessVerificationProgress(chainParams.TxData(), pindexNew), ::ChainstateActive().CoinsTip().DynamicMemoryUsage() * (1.0 / (1<<20)), ::ChainstateActive().CoinsTip().GetCacheSize(), - !warningMessages.empty() ? strprintf(" warning='%s'", warningMessages) : ""); + !warning_messages.empty() ? strprintf(" warning='%s'", warning_messages.original) : ""); } diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index d5f6d63a46..d9fe741a6e 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -92,12 +92,6 @@ static void RescanWallet(CWallet& wallet, const WalletRescanReserver& reserver, UniValue importprivkey(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - CWallet* const pwallet = wallet.get(); - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"importprivkey", "\nAdds a private key (as returned by dumpprivkey) to your wallet. Requires a new wallet backup.\n" "Hint: use importmulti to import more than one private key.\n" @@ -124,6 +118,10 @@ UniValue importprivkey(const JSONRPCRequest& request) }, }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + CWallet* const pwallet = wallet.get(); + if (pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { throw JSONRPCError(RPC_WALLET_ERROR, "Cannot import private keys to a wallet with private keys disabled"); } @@ -195,12 +193,6 @@ UniValue importprivkey(const JSONRPCRequest& request) UniValue abortrescan(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - CWallet* const pwallet = wallet.get(); - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"abortrescan", "\nStops current wallet rescan triggered by an RPC call, e.g. by an importprivkey call.\n" "Note: Use \"getwalletinfo\" to query the scanning progress.\n", @@ -216,6 +208,10 @@ UniValue abortrescan(const JSONRPCRequest& request) }, }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + CWallet* const pwallet = wallet.get(); + if (!pwallet->IsScanning() || pwallet->IsAbortingRescan()) return false; pwallet->AbortRescan(); return true; @@ -223,12 +219,6 @@ UniValue abortrescan(const JSONRPCRequest& request) UniValue importaddress(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - CWallet* const pwallet = wallet.get(); - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"importaddress", "\nAdds an address or script (in hex) that can be watched as if it were in your wallet but cannot be used to spend. Requires a new wallet backup.\n" "\nNote: This call can take over an hour to complete if rescan is true, during that time, other rpc calls\n" @@ -255,6 +245,10 @@ UniValue importaddress(const JSONRPCRequest& request) }, }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + CWallet* const pwallet = wallet.get(); + EnsureLegacyScriptPubKeyMan(*pwallet, true); std::string strLabel; @@ -325,12 +319,6 @@ UniValue importaddress(const JSONRPCRequest& request) UniValue importprunedfunds(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - CWallet* const pwallet = wallet.get(); - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"importprunedfunds", "\nImports funds without rescan. Corresponding address or script must previously be included in wallet. Aimed towards pruned wallets. The end-user is responsible to import additional transactions that subsequently spend the imported outputs or rescan after the point in the blockchain the transaction is included.\n", { @@ -341,6 +329,10 @@ UniValue importprunedfunds(const JSONRPCRequest& request) RPCExamples{""}, }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + CWallet* const pwallet = wallet.get(); + CMutableTransaction tx; if (!DecodeHexTx(tx, request.params[0].get_str())) throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed"); @@ -383,12 +375,6 @@ UniValue importprunedfunds(const JSONRPCRequest& request) UniValue removeprunedfunds(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - CWallet* const pwallet = wallet.get(); - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"removeprunedfunds", "\nDeletes the specified transaction from the wallet. Meant for use with pruned wallets and as a companion to importprunedfunds. This will affect wallet balances.\n", { @@ -402,6 +388,10 @@ UniValue removeprunedfunds(const JSONRPCRequest& request) }, }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + CWallet* const pwallet = wallet.get(); + LOCK(pwallet->cs_wallet); uint256 hash(ParseHashV(request.params[0], "txid")); @@ -422,12 +412,6 @@ UniValue removeprunedfunds(const JSONRPCRequest& request) UniValue importpubkey(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - CWallet* const pwallet = wallet.get(); - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"importpubkey", "\nAdds a public key (in hex) that can be watched as if it were in your wallet but cannot be used to spend. Requires a new wallet backup.\n" "Hint: use importmulti to import more than one public key.\n" @@ -450,6 +434,10 @@ UniValue importpubkey(const JSONRPCRequest& request) }, }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + CWallet* const pwallet = wallet.get(); + EnsureLegacyScriptPubKeyMan(*wallet, true); std::string strLabel; @@ -509,12 +497,6 @@ UniValue importpubkey(const JSONRPCRequest& request) UniValue importwallet(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - CWallet* const pwallet = wallet.get(); - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"importwallet", "\nImports keys from a wallet dump file (see dumpwallet). Requires a new wallet backup to include imported keys.\n" "Note: Use \"getwalletinfo\" to query the scanning progress.\n", @@ -532,6 +514,10 @@ UniValue importwallet(const JSONRPCRequest& request) }, }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + CWallet* const pwallet = wallet.get(); + EnsureLegacyScriptPubKeyMan(*wallet, true); if (pwallet->chain().havePruned()) { @@ -667,12 +653,6 @@ UniValue importwallet(const JSONRPCRequest& request) UniValue dumpprivkey(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - const CWallet* const pwallet = wallet.get(); - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"dumpprivkey", "\nReveals the private key corresponding to 'address'.\n" "Then the importprivkey can be used with this output\n", @@ -689,6 +669,10 @@ UniValue dumpprivkey(const JSONRPCRequest& request) }, }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + const CWallet* const pwallet = wallet.get(); + LegacyScriptPubKeyMan& spk_man = EnsureLegacyScriptPubKeyMan(*wallet); LOCK2(pwallet->cs_wallet, spk_man.cs_KeyStore); @@ -714,11 +698,6 @@ UniValue dumpprivkey(const JSONRPCRequest& request) UniValue dumpwallet(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const pwallet = GetWalletForJSONRPCRequest(request); - if (!EnsureWalletIsAvailable(pwallet.get(), request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"dumpwallet", "\nDumps all wallet keys in a human-readable format to a server-side file. This does not allow overwriting existing files.\n" "Imported scripts are included in the dumpfile, but corresponding BIP173 addresses, etc. may not be added automatically by importwallet.\n" @@ -739,6 +718,9 @@ UniValue dumpwallet(const JSONRPCRequest& request) }, }.Check(request); + std::shared_ptr<CWallet> const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; + CWallet& wallet = *pwallet; LegacyScriptPubKeyMan& spk_man = EnsureLegacyScriptPubKeyMan(wallet); @@ -1259,12 +1241,6 @@ static int64_t GetImportTimestamp(const UniValue& data, int64_t now) UniValue importmulti(const JSONRPCRequest& mainRequest) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(mainRequest); - CWallet* const pwallet = wallet.get(); - if (!EnsureWalletIsAvailable(pwallet, mainRequest.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"importmulti", "\nImport addresses/scripts (with private or public keys, redeem script (P2SH)), optionally rescanning the blockchain from the earliest creation time of the imported scripts. Requires a new wallet backup.\n" "If an address/script is imported without all of the private keys required to spend from that address, it will be watchonly. The 'watchonly' option must be set to true in this case or a warning will be returned.\n" @@ -1340,6 +1316,9 @@ UniValue importmulti(const JSONRPCRequest& mainRequest) }, }.Check(mainRequest); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(mainRequest); + if (!wallet) return NullUniValue; + CWallet* const pwallet = wallet.get(); RPCTypeCheck(mainRequest.params, {UniValue::VARR, UniValue::VOBJ}); @@ -1585,14 +1564,8 @@ static UniValue ProcessDescriptorImport(CWallet * const pwallet, const UniValue& return result; } -UniValue importdescriptors(const JSONRPCRequest& main_request) { - // Acquire the wallet - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(main_request); - CWallet* const pwallet = wallet.get(); - if (!EnsureWalletIsAvailable(pwallet, main_request.fHelp)) { - return NullUniValue; - } - +UniValue importdescriptors(const JSONRPCRequest& main_request) +{ RPCHelpMan{"importdescriptors", "\nImport descriptors. This will trigger a rescan of the blockchain based on the earliest timestamp of all descriptors being imported. Requires a new wallet backup.\n" "\nNote: This call can take over an hour to complete if using an early timestamp; during that time, other rpc calls\n" @@ -1644,6 +1617,10 @@ UniValue importdescriptors(const JSONRPCRequest& main_request) { }, }.Check(main_request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(main_request); + if (!wallet) return NullUniValue; + CWallet* const pwallet = wallet.get(); + // Make sure wallet is a descriptor wallet if (!pwallet->IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS)) { throw JSONRPCError(RPC_WALLET_ERROR, "importdescriptors is not available for non-descriptor wallets"); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index ae2b19e923..0907f2aabd 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -93,6 +93,7 @@ bool GetWalletNameFromJSONRPCRequest(const JSONRPCRequest& request, std::string& std::shared_ptr<CWallet> GetWalletForJSONRPCRequest(const JSONRPCRequest& request) { + CHECK_NONFATAL(!request.fHelp); std::string wallet_name; if (GetWalletNameFromJSONRPCRequest(request, wallet_name)) { std::shared_ptr<CWallet> pwallet = GetWallet(wallet_name); @@ -101,13 +102,10 @@ std::shared_ptr<CWallet> GetWalletForJSONRPCRequest(const JSONRPCRequest& reques } std::vector<std::shared_ptr<CWallet>> wallets = GetWallets(); - return wallets.size() == 1 || (request.fHelp && wallets.size() > 0) ? wallets[0] : nullptr; -} + if (wallets.size() == 1) { + return wallets[0]; + } -bool EnsureWalletIsAvailable(const CWallet* pwallet, bool avoidException) -{ - if (pwallet) return true; - if (avoidException) return false; if (!HasWallets()) { throw JSONRPCError( RPC_METHOD_NOT_FOUND, "Method not found (wallet method is disabled because no wallet is loaded)"); @@ -195,13 +193,6 @@ static std::string LabelFromValue(const UniValue& value) static UniValue getnewaddress(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - CWallet* const pwallet = wallet.get(); - - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"getnewaddress", "\nReturns a new Bitcoin address for receiving payments.\n" "If 'label' is specified, it is added to the address book \n" @@ -219,6 +210,10 @@ static UniValue getnewaddress(const JSONRPCRequest& request) }, }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + CWallet* const pwallet = wallet.get(); + LOCK(pwallet->cs_wallet); if (!pwallet->CanGetAddresses()) { @@ -248,13 +243,6 @@ static UniValue getnewaddress(const JSONRPCRequest& request) static UniValue getrawchangeaddress(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - CWallet* const pwallet = wallet.get(); - - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"getrawchangeaddress", "\nReturns a new Bitcoin address, for receiving change.\n" "This is for use with raw transactions, NOT normal use.\n", @@ -270,6 +258,10 @@ static UniValue getrawchangeaddress(const JSONRPCRequest& request) }, }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + CWallet* const pwallet = wallet.get(); + LOCK(pwallet->cs_wallet); if (!pwallet->CanGetAddresses(true)) { @@ -294,13 +286,6 @@ static UniValue getrawchangeaddress(const JSONRPCRequest& request) static UniValue setlabel(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - CWallet* const pwallet = wallet.get(); - - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"setlabel", "\nSets the label associated with the given address.\n", { @@ -314,6 +299,10 @@ static UniValue setlabel(const JSONRPCRequest& request) }, }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + CWallet* const pwallet = wallet.get(); + LOCK(pwallet->cs_wallet); CTxDestination dest = DecodeDestination(request.params[0].get_str()); @@ -366,13 +355,6 @@ static CTransactionRef SendMoney(CWallet* const pwallet, const CTxDestination& a static UniValue sendtoaddress(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - CWallet* const pwallet = wallet.get(); - - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"sendtoaddress", "\nSend an amount to a given address." + HELP_REQUIRING_PASSPHRASE, @@ -406,6 +388,10 @@ static UniValue sendtoaddress(const JSONRPCRequest& request) }, }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + CWallet* const pwallet = wallet.get(); + // Make sure the results are valid at least up to the most recent block // the user could have gotten from another RPC command prior to now pwallet->BlockUntilSyncedToCurrentChain(); @@ -461,13 +447,6 @@ static UniValue sendtoaddress(const JSONRPCRequest& request) static UniValue listaddressgroupings(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - const CWallet* const pwallet = wallet.get(); - - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"listaddressgroupings", "\nLists groups of addresses which have had their common ownership\n" "made public by common use as inputs or as the resulting change\n" @@ -493,6 +472,10 @@ static UniValue listaddressgroupings(const JSONRPCRequest& request) }, }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + const CWallet* const pwallet = wallet.get(); + // Make sure the results are valid at least up to the most recent block // the user could have gotten from another RPC command prior to now pwallet->BlockUntilSyncedToCurrentChain(); @@ -523,13 +506,6 @@ static UniValue listaddressgroupings(const JSONRPCRequest& request) static UniValue signmessage(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - const CWallet* const pwallet = wallet.get(); - - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"signmessage", "\nSign a message with the private key of an address" + HELP_REQUIRING_PASSPHRASE, @@ -552,6 +528,10 @@ static UniValue signmessage(const JSONRPCRequest& request) }, }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + const CWallet* const pwallet = wallet.get(); + LOCK(pwallet->cs_wallet); EnsureWalletIsUnlocked(pwallet); @@ -628,13 +608,6 @@ static CAmount GetReceived(const CWallet& wallet, const UniValue& params, bool b static UniValue getreceivedbyaddress(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - const CWallet* const pwallet = wallet.get(); - - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"getreceivedbyaddress", "\nReturns the total amount received by the given address in transactions with at least minconf confirmations.\n", { @@ -656,6 +629,10 @@ static UniValue getreceivedbyaddress(const JSONRPCRequest& request) }, }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + const CWallet* const pwallet = wallet.get(); + // Make sure the results are valid at least up to the most recent block // the user could have gotten from another RPC command prior to now pwallet->BlockUntilSyncedToCurrentChain(); @@ -668,13 +645,6 @@ static UniValue getreceivedbyaddress(const JSONRPCRequest& request) static UniValue getreceivedbylabel(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - const CWallet* const pwallet = wallet.get(); - - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"getreceivedbylabel", "\nReturns the total amount received by addresses with <label> in transactions with at least [minconf] confirmations.\n", { @@ -696,6 +666,10 @@ static UniValue getreceivedbylabel(const JSONRPCRequest& request) }, }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + const CWallet* const pwallet = wallet.get(); + // Make sure the results are valid at least up to the most recent block // the user could have gotten from another RPC command prior to now pwallet->BlockUntilSyncedToCurrentChain(); @@ -708,13 +682,6 @@ static UniValue getreceivedbylabel(const JSONRPCRequest& request) static UniValue getbalance(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - const CWallet* const pwallet = wallet.get(); - - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"getbalance", "\nReturns the total available balance.\n" "The available balance is what the wallet considers currently spendable, and is\n" @@ -738,6 +705,10 @@ static UniValue getbalance(const JSONRPCRequest& request) }, }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + const CWallet* const pwallet = wallet.get(); + // Make sure the results are valid at least up to the most recent block // the user could have gotten from another RPC command prior to now pwallet->BlockUntilSyncedToCurrentChain(); @@ -765,13 +736,6 @@ static UniValue getbalance(const JSONRPCRequest& request) static UniValue getunconfirmedbalance(const JSONRPCRequest &request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - const CWallet* const pwallet = wallet.get(); - - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"getunconfirmedbalance", "DEPRECATED\nIdentical to getbalances().mine.untrusted_pending\n", {}, @@ -779,6 +743,10 @@ static UniValue getunconfirmedbalance(const JSONRPCRequest &request) RPCExamples{""}, }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + const CWallet* const pwallet = wallet.get(); + // Make sure the results are valid at least up to the most recent block // the user could have gotten from another RPC command prior to now pwallet->BlockUntilSyncedToCurrentChain(); @@ -791,13 +759,6 @@ static UniValue getunconfirmedbalance(const JSONRPCRequest &request) static UniValue sendmany(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - CWallet* const pwallet = wallet.get(); - - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"sendmany", "\nSend multiple times. Amounts are double-precision floating point numbers." + HELP_REQUIRING_PASSPHRASE, @@ -841,6 +802,10 @@ static UniValue sendmany(const JSONRPCRequest& request) }, }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + CWallet* const pwallet = wallet.get(); + // Make sure the results are valid at least up to the most recent block // the user could have gotten from another RPC command prior to now pwallet->BlockUntilSyncedToCurrentChain(); @@ -925,13 +890,6 @@ static UniValue sendmany(const JSONRPCRequest& request) static UniValue addmultisigaddress(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - CWallet* const pwallet = wallet.get(); - - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"addmultisigaddress", "\nAdd an nrequired-to-sign multisignature address to the wallet. Requires a new wallet backup.\n" "Each key is a Bitcoin address or hex-encoded public key.\n" @@ -964,6 +922,10 @@ static UniValue addmultisigaddress(const JSONRPCRequest& request) }, }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + CWallet* const pwallet = wallet.get(); + LegacyScriptPubKeyMan& spk_man = EnsureLegacyScriptPubKeyMan(*pwallet); LOCK2(pwallet->cs_wallet, spk_man.cs_KeyStore); @@ -1167,13 +1129,6 @@ static UniValue ListReceived(const CWallet* const pwallet, const UniValue& param static UniValue listreceivedbyaddress(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - const CWallet* const pwallet = wallet.get(); - - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"listreceivedbyaddress", "\nList balances by receiving address.\n", { @@ -1207,6 +1162,10 @@ static UniValue listreceivedbyaddress(const JSONRPCRequest& request) }, }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + const CWallet* const pwallet = wallet.get(); + // Make sure the results are valid at least up to the most recent block // the user could have gotten from another RPC command prior to now pwallet->BlockUntilSyncedToCurrentChain(); @@ -1218,13 +1177,6 @@ static UniValue listreceivedbyaddress(const JSONRPCRequest& request) static UniValue listreceivedbylabel(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - const CWallet* const pwallet = wallet.get(); - - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"listreceivedbylabel", "\nList received transactions by label.\n", { @@ -1251,6 +1203,10 @@ static UniValue listreceivedbylabel(const JSONRPCRequest& request) }, }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + const CWallet* const pwallet = wallet.get(); + // Make sure the results are valid at least up to the most recent block // the user could have gotten from another RPC command prior to now pwallet->BlockUntilSyncedToCurrentChain(); @@ -1379,13 +1335,6 @@ static const std::vector<RPCResult> TransactionDescriptionString() UniValue listtransactions(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - const CWallet* const pwallet = wallet.get(); - - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"listtransactions", "\nIf a label name is provided, this will return only incoming transactions paying to addresses with the specified label.\n" "\nReturns up to 'count' most recent transactions skipping the first 'from' transactions.\n", @@ -1433,6 +1382,10 @@ UniValue listtransactions(const JSONRPCRequest& request) }, }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + const CWallet* const pwallet = wallet.get(); + // Make sure the results are valid at least up to the most recent block // the user could have gotten from another RPC command prior to now pwallet->BlockUntilSyncedToCurrentChain(); @@ -1492,12 +1445,6 @@ UniValue listtransactions(const JSONRPCRequest& request) static UniValue listsinceblock(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const pwallet = GetWalletForJSONRPCRequest(request); - - if (!EnsureWalletIsAvailable(pwallet.get(), request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"listsinceblock", "\nGet all transactions in blocks since block [blockhash], or all transactions if omitted.\n" "If \"blockhash\" is no longer a part of the main chain, transactions from the fork point onward are included.\n" @@ -1551,6 +1498,9 @@ static UniValue listsinceblock(const JSONRPCRequest& request) }, }.Check(request); + std::shared_ptr<CWallet> const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; + const CWallet& wallet = *pwallet; // Make sure the results are valid at least up to the most recent block // the user could have gotten from another RPC command prior to now @@ -1633,13 +1583,6 @@ static UniValue listsinceblock(const JSONRPCRequest& request) static UniValue gettransaction(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - const CWallet* const pwallet = wallet.get(); - - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"gettransaction", "\nGet detailed information about in-wallet transaction <txid>\n", { @@ -1694,6 +1637,10 @@ static UniValue gettransaction(const JSONRPCRequest& request) }, }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + const CWallet* const pwallet = wallet.get(); + // Make sure the results are valid at least up to the most recent block // the user could have gotten from another RPC command prior to now pwallet->BlockUntilSyncedToCurrentChain(); @@ -1746,13 +1693,6 @@ static UniValue gettransaction(const JSONRPCRequest& request) static UniValue abandontransaction(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - CWallet* const pwallet = wallet.get(); - - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"abandontransaction", "\nMark in-wallet transaction <txid> as abandoned\n" "This will mark this transaction and all its in-wallet descendants as abandoned which will allow\n" @@ -1769,6 +1709,10 @@ static UniValue abandontransaction(const JSONRPCRequest& request) }, }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + CWallet* const pwallet = wallet.get(); + // Make sure the results are valid at least up to the most recent block // the user could have gotten from another RPC command prior to now pwallet->BlockUntilSyncedToCurrentChain(); @@ -1790,13 +1734,6 @@ static UniValue abandontransaction(const JSONRPCRequest& request) static UniValue backupwallet(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - const CWallet* const pwallet = wallet.get(); - - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"backupwallet", "\nSafely copies current wallet file to destination, which can be a directory or a path with filename.\n", { @@ -1809,6 +1746,10 @@ static UniValue backupwallet(const JSONRPCRequest& request) }, }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + const CWallet* const pwallet = wallet.get(); + // Make sure the results are valid at least up to the most recent block // the user could have gotten from another RPC command prior to now pwallet->BlockUntilSyncedToCurrentChain(); @@ -1826,13 +1767,6 @@ static UniValue backupwallet(const JSONRPCRequest& request) static UniValue keypoolrefill(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - CWallet* const pwallet = wallet.get(); - - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"keypoolrefill", "\nFills the keypool."+ HELP_REQUIRING_PASSPHRASE, @@ -1846,6 +1780,10 @@ static UniValue keypoolrefill(const JSONRPCRequest& request) }, }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + CWallet* const pwallet = wallet.get(); + if (pwallet->IsLegacy() && pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { throw JSONRPCError(RPC_WALLET_ERROR, "Error: Private keys are disabled for this wallet"); } @@ -1873,13 +1811,6 @@ static UniValue keypoolrefill(const JSONRPCRequest& request) static UniValue walletpassphrase(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - CWallet* const pwallet = wallet.get(); - - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"walletpassphrase", "\nStores the wallet decryption key in memory for 'timeout' seconds.\n" "This is needed prior to performing transactions related to private keys such as sending bitcoins\n" @@ -1901,6 +1832,10 @@ static UniValue walletpassphrase(const JSONRPCRequest& request) }, }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + CWallet* const pwallet = wallet.get(); + int64_t nSleepTime; int64_t relock_time; // Prevent concurrent calls to walletpassphrase with the same wallet. @@ -1970,13 +1905,6 @@ static UniValue walletpassphrase(const JSONRPCRequest& request) static UniValue walletpassphrasechange(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - CWallet* const pwallet = wallet.get(); - - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"walletpassphrasechange", "\nChanges the wallet passphrase from 'oldpassphrase' to 'newpassphrase'.\n", { @@ -1990,6 +1918,10 @@ static UniValue walletpassphrasechange(const JSONRPCRequest& request) }, }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + CWallet* const pwallet = wallet.get(); + LOCK(pwallet->cs_wallet); if (!pwallet->IsCrypted()) { @@ -2020,13 +1952,6 @@ static UniValue walletpassphrasechange(const JSONRPCRequest& request) static UniValue walletlock(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - CWallet* const pwallet = wallet.get(); - - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"walletlock", "\nRemoves the wallet encryption key from memory, locking the wallet.\n" "After calling this method, you will need to call walletpassphrase again\n" @@ -2045,6 +1970,10 @@ static UniValue walletlock(const JSONRPCRequest& request) }, }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + CWallet* const pwallet = wallet.get(); + LOCK(pwallet->cs_wallet); if (!pwallet->IsCrypted()) { @@ -2060,13 +1989,6 @@ static UniValue walletlock(const JSONRPCRequest& request) static UniValue encryptwallet(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - CWallet* const pwallet = wallet.get(); - - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"encryptwallet", "\nEncrypts the wallet with 'passphrase'. This is for first time encryption.\n" "After this, any calls that interact with private keys such as sending or signing \n" @@ -2091,6 +2013,10 @@ static UniValue encryptwallet(const JSONRPCRequest& request) }, }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + CWallet* const pwallet = wallet.get(); + LOCK(pwallet->cs_wallet); if (pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { @@ -2120,13 +2046,6 @@ static UniValue encryptwallet(const JSONRPCRequest& request) static UniValue lockunspent(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - CWallet* const pwallet = wallet.get(); - - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"lockunspent", "\nUpdates list of temporarily unspendable outputs.\n" "Temporarily lock (unlock=false) or unlock (unlock=true) specified transaction outputs.\n" @@ -2165,6 +2084,10 @@ static UniValue lockunspent(const JSONRPCRequest& request) }, }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + CWallet* const pwallet = wallet.get(); + // Make sure the results are valid at least up to the most recent block // the user could have gotten from another RPC command prior to now pwallet->BlockUntilSyncedToCurrentChain(); @@ -2246,13 +2169,6 @@ static UniValue lockunspent(const JSONRPCRequest& request) static UniValue listlockunspent(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - const CWallet* const pwallet = wallet.get(); - - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"listlockunspent", "\nReturns list of temporarily unspendable outputs.\n" "See the lockunspent call to lock and unlock transactions for spending.\n", @@ -2281,6 +2197,10 @@ static UniValue listlockunspent(const JSONRPCRequest& request) }, }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + const CWallet* const pwallet = wallet.get(); + LOCK(pwallet->cs_wallet); std::vector<COutPoint> vOutpts; @@ -2301,13 +2221,6 @@ static UniValue listlockunspent(const JSONRPCRequest& request) static UniValue settxfee(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - CWallet* const pwallet = wallet.get(); - - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"settxfee", "\nSet the transaction fee per kB for this wallet. Overrides the global -paytxfee command line parameter.\n" "Can be deactivated by passing 0 as the fee. In that case automatic fee selection will be used by default.\n", @@ -2323,6 +2236,10 @@ static UniValue settxfee(const JSONRPCRequest& request) }, }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + CWallet* const pwallet = wallet.get(); + LOCK(pwallet->cs_wallet); CAmount nAmount = AmountFromValue(request.params[0]); @@ -2344,12 +2261,6 @@ static UniValue settxfee(const JSONRPCRequest& request) static UniValue getbalances(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const rpc_wallet = GetWalletForJSONRPCRequest(request); - if (!EnsureWalletIsAvailable(rpc_wallet.get(), request.fHelp)) { - return NullUniValue; - } - CWallet& wallet = *rpc_wallet; - RPCHelpMan{ "getbalances", "Returns an object with all balances in " + CURRENCY_UNIT + ".\n", @@ -2377,6 +2288,10 @@ static UniValue getbalances(const JSONRPCRequest& request) HelpExampleRpc("getbalances", "")}, }.Check(request); + std::shared_ptr<CWallet> const rpc_wallet = GetWalletForJSONRPCRequest(request); + if (!rpc_wallet) return NullUniValue; + CWallet& wallet = *rpc_wallet; + // Make sure the results are valid at least up to the most recent block // the user could have gotten from another RPC command prior to now wallet.BlockUntilSyncedToCurrentChain(); @@ -2411,13 +2326,6 @@ static UniValue getbalances(const JSONRPCRequest& request) static UniValue getwalletinfo(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - const CWallet* const pwallet = wallet.get(); - - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"getwalletinfo", "Returns an object containing various wallet state info.\n", {}, @@ -2453,6 +2361,10 @@ static UniValue getwalletinfo(const JSONRPCRequest& request) }, }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + const CWallet* const pwallet = wallet.get(); + // Make sure the results are valid at least up to the most recent block // the user could have gotten from another RPC command prior to now pwallet->BlockUntilSyncedToCurrentChain(); @@ -2560,12 +2472,7 @@ static UniValue listwallets(const JSONRPCRequest& request) UniValue obj(UniValue::VARR); for (const std::shared_ptr<CWallet>& wallet : GetWallets()) { - if (!EnsureWalletIsAvailable(wallet.get(), request.fHelp)) { - return NullUniValue; - } - LOCK(wallet->cs_wallet); - obj.push_back(wallet->GetName()); } @@ -2621,13 +2528,6 @@ static UniValue loadwallet(const JSONRPCRequest& request) static UniValue setwalletflag(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - CWallet* const pwallet = wallet.get(); - - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - std::string flags = ""; for (auto& it : WALLET_FLAG_MAP) if (it.second & MUTABLE_WALLET_FLAGS) @@ -2652,6 +2552,10 @@ static UniValue setwalletflag(const JSONRPCRequest& request) }, }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + CWallet* const pwallet = wallet.get(); + std::string flag_str = request.params[0].get_str(); bool value = request.params[1].isNull() || request.params[1].get_bool(); @@ -2804,13 +2708,6 @@ static UniValue unloadwallet(const JSONRPCRequest& request) static UniValue listunspent(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - const CWallet* const pwallet = wallet.get(); - - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{ "listunspent", "\nReturns array of unspent transaction outputs\n" @@ -2868,6 +2765,10 @@ static UniValue listunspent(const JSONRPCRequest& request) }, }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + const CWallet* const pwallet = wallet.get(); + int nMinDepth = 1; if (!request.params[0].isNull()) { RPCTypeCheckArgument(request.params[0], UniValue::VNUM); @@ -3137,13 +3038,6 @@ void FundTransaction(CWallet* const pwallet, CMutableTransaction& tx, CAmount& f static UniValue fundrawtransaction(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - CWallet* const pwallet = wallet.get(); - - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"fundrawtransaction", "\nAdd inputs to a transaction until it has enough in value to meet its out value.\n" "This will not modify existing inputs, and will add at most one change output to the outputs.\n" @@ -3213,6 +3107,10 @@ static UniValue fundrawtransaction(const JSONRPCRequest& request) }, }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + CWallet* const pwallet = wallet.get(); + RPCTypeCheck(request.params, {UniValue::VSTR, UniValueType(), UniValue::VBOOL}); // parse hex string from parameter @@ -3237,13 +3135,6 @@ static UniValue fundrawtransaction(const JSONRPCRequest& request) UniValue signrawtransactionwithwallet(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - const CWallet* const pwallet = wallet.get(); - - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"signrawtransactionwithwallet", "\nSign inputs for raw transaction (serialized, hex-encoded).\n" "The second optional argument (may be null) is an array of previous transaction outputs that\n" @@ -3297,6 +3188,10 @@ UniValue signrawtransactionwithwallet(const JSONRPCRequest& request) }, }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + const CWallet* const pwallet = wallet.get(); + RPCTypeCheck(request.params, {UniValue::VSTR, UniValue::VARR, UniValue::VSTR}, true); CMutableTransaction mtx; @@ -3331,13 +3226,6 @@ UniValue signrawtransactionwithwallet(const JSONRPCRequest& request) static UniValue bumpfee(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - CWallet* const pwallet = wallet.get(); - - - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) - return NullUniValue; - RPCHelpMan{"bumpfee", "\nBumps the fee of an opt-in-RBF transaction T, replacing it with a new transaction B.\n" "An opt-in RBF transaction with the given txid must be in the wallet.\n" @@ -3389,6 +3277,10 @@ static UniValue bumpfee(const JSONRPCRequest& request) }, }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + CWallet* const pwallet = wallet.get(); + RPCTypeCheck(request.params, {UniValue::VSTR, UniValue::VOBJ}); uint256 hash(ParseHashV(request.params[0], "txid")); @@ -3503,13 +3395,6 @@ static UniValue bumpfee(const JSONRPCRequest& request) UniValue rescanblockchain(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - CWallet* const pwallet = wallet.get(); - - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"rescanblockchain", "\nRescan the local blockchain for wallet related transactions.\n" "Note: Use \"getwalletinfo\" to query the scanning progress.\n", @@ -3530,6 +3415,10 @@ UniValue rescanblockchain(const JSONRPCRequest& request) }, }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + CWallet* const pwallet = wallet.get(); + WalletRescanReserver reserver(*pwallet); if (!reserver.reserve()) { throw JSONRPCError(RPC_WALLET_ERROR, "Wallet is currently rescanning. Abort existing rescan or wait."); @@ -3702,13 +3591,6 @@ static UniValue AddressBookDataToJSON(const CAddressBookData& data, const bool v UniValue getaddressinfo(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - const CWallet* const pwallet = wallet.get(); - - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"getaddressinfo", "\nReturn information about the given bitcoin address.\n" "Some of the information will only be present if the address is in the active wallet.\n", @@ -3769,6 +3651,10 @@ UniValue getaddressinfo(const JSONRPCRequest& request) }, }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + const CWallet* const pwallet = wallet.get(); + LOCK(pwallet->cs_wallet); UniValue ret(UniValue::VOBJ); @@ -3845,13 +3731,6 @@ UniValue getaddressinfo(const JSONRPCRequest& request) static UniValue getaddressesbylabel(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - const CWallet* const pwallet = wallet.get(); - - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"getaddressesbylabel", "\nReturns the list of addresses assigned the specified label.\n", { @@ -3872,6 +3751,10 @@ static UniValue getaddressesbylabel(const JSONRPCRequest& request) }, }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + const CWallet* const pwallet = wallet.get(); + LOCK(pwallet->cs_wallet); std::string label = LabelFromValue(request.params[0]); @@ -3905,13 +3788,6 @@ static UniValue getaddressesbylabel(const JSONRPCRequest& request) static UniValue listlabels(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - const CWallet* const pwallet = wallet.get(); - - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"listlabels", "\nReturns the list of all labels, or labels that are assigned to addresses with a specific purpose.\n", { @@ -3935,6 +3811,10 @@ static UniValue listlabels(const JSONRPCRequest& request) }, }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + const CWallet* const pwallet = wallet.get(); + LOCK(pwallet->cs_wallet); std::string purpose; @@ -3961,13 +3841,6 @@ static UniValue listlabels(const JSONRPCRequest& request) UniValue sethdseed(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - CWallet* const pwallet = wallet.get(); - - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"sethdseed", "\nSet or generate a new HD wallet seed. Non-HD wallets will not be upgraded to being a HD wallet. Wallets that are already\n" "HD will have a new HD seed set so that new keys added to the keypool will be derived from this new seed.\n" @@ -3990,6 +3863,10 @@ UniValue sethdseed(const JSONRPCRequest& request) }, }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + CWallet* const pwallet = wallet.get(); + LegacyScriptPubKeyMan& spk_man = EnsureLegacyScriptPubKeyMan(*pwallet, true); if (pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { @@ -4034,13 +3911,6 @@ UniValue sethdseed(const JSONRPCRequest& request) UniValue walletprocesspsbt(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - const CWallet* const pwallet = wallet.get(); - - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"walletprocesspsbt", "\nUpdate a PSBT with input information from our wallet and then sign inputs\n" "that we can sign for." + @@ -4069,6 +3939,10 @@ UniValue walletprocesspsbt(const JSONRPCRequest& request) }, }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + const CWallet* const pwallet = wallet.get(); + RPCTypeCheck(request.params, {UniValue::VSTR, UniValue::VBOOL, UniValue::VSTR}); // Unserialize the transaction @@ -4101,13 +3975,6 @@ UniValue walletprocesspsbt(const JSONRPCRequest& request) UniValue walletcreatefundedpsbt(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - CWallet* const pwallet = wallet.get(); - - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"walletcreatefundedpsbt", "\nCreates and funds a transaction in the Partially Signed Transaction format. Inputs will be added if supplied inputs are not enough\n" "Implements the Creator and Updater roles.\n", @@ -4182,6 +4049,10 @@ UniValue walletcreatefundedpsbt(const JSONRPCRequest& request) }, }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + CWallet* const pwallet = wallet.get(); + RPCTypeCheck(request.params, { UniValue::VARR, UniValueType(), // ARR or OBJ, checked later @@ -4226,13 +4097,6 @@ UniValue walletcreatefundedpsbt(const JSONRPCRequest& request) static UniValue upgradewallet(const JSONRPCRequest& request) { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - CWallet* const pwallet = wallet.get(); - - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - RPCHelpMan{"upgradewallet", "\nUpgrade the wallet. Upgrades to the latest version if no version number is specified\n" "New keys may be generated and a new wallet backup will need to be made.", @@ -4246,6 +4110,10 @@ static UniValue upgradewallet(const JSONRPCRequest& request) } }.Check(request); + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + CWallet* const pwallet = wallet.get(); + RPCTypeCheck(request.params, {UniValue::VNUM}, true); EnsureWalletIsUnlocked(pwallet); diff --git a/src/wallet/rpcwallet.h b/src/wallet/rpcwallet.h index d00221a04c..fb1e91282b 100644 --- a/src/wallet/rpcwallet.h +++ b/src/wallet/rpcwallet.h @@ -31,7 +31,6 @@ Span<const CRPCCommand> GetWalletRPCCommands(); std::shared_ptr<CWallet> GetWalletForJSONRPCRequest(const JSONRPCRequest& request); void EnsureWalletIsUnlocked(const CWallet*); -bool EnsureWalletIsAvailable(const CWallet*, bool avoidException); WalletContext& EnsureWalletContext(const util::Ref& context); LegacyScriptPubKeyMan& EnsureLegacyScriptPubKeyMan(CWallet& wallet, bool also_create = false); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 89737ca7b5..a59aa8b980 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2997,7 +2997,7 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CTransac } if (nFeeRet > m_default_max_tx_fee) { - error = Untranslated(TransactionErrorString(TransactionError::MAX_FEE_EXCEEDED)); + error = TransactionErrorString(TransactionError::MAX_FEE_EXCEEDED); return false; } diff --git a/src/warnings.cpp b/src/warnings.cpp index e04e800687..501bf7e637 100644 --- a/src/warnings.cpp +++ b/src/warnings.cpp @@ -6,18 +6,21 @@ #include <warnings.h> #include <sync.h> +#include <util/string.h> #include <util/system.h> #include <util/translation.h> +#include <vector> + static Mutex g_warnings_mutex; -static std::string strMiscWarning GUARDED_BY(g_warnings_mutex); +static bilingual_str g_misc_warnings GUARDED_BY(g_warnings_mutex); static bool fLargeWorkForkFound GUARDED_BY(g_warnings_mutex) = false; static bool fLargeWorkInvalidChainFound GUARDED_BY(g_warnings_mutex) = false; -void SetMiscWarning(const std::string& strWarning) +void SetMiscWarning(const bilingual_str& warning) { LOCK(g_warnings_mutex); - strMiscWarning = strWarning; + g_misc_warnings = warning; } void SetfLargeWorkForkFound(bool flag) @@ -38,34 +41,36 @@ void SetfLargeWorkInvalidChainFound(bool flag) fLargeWorkInvalidChainFound = flag; } -std::string GetWarnings(bool verbose) +bilingual_str GetWarnings(bool verbose) { - std::string warnings_concise; - std::string warnings_verbose; - const std::string warning_separator = "<hr />"; + bilingual_str warnings_concise; + std::vector<bilingual_str> warnings_verbose; LOCK(g_warnings_mutex); // Pre-release build warning if (!CLIENT_VERSION_IS_RELEASE) { - warnings_concise = "This is a pre-release test build - use at your own risk - do not use for mining or merchant applications"; - warnings_verbose = _("This is a pre-release test build - use at your own risk - do not use for mining or merchant applications").translated; + warnings_concise = _("This is a pre-release test build - use at your own risk - do not use for mining or merchant applications"); + warnings_verbose.emplace_back(warnings_concise); } // Misc warnings like out of disk space and clock is wrong - if (strMiscWarning != "") { - warnings_concise = strMiscWarning; - warnings_verbose += (warnings_verbose.empty() ? "" : warning_separator) + strMiscWarning; + if (!g_misc_warnings.empty()) { + warnings_concise = g_misc_warnings; + warnings_verbose.emplace_back(warnings_concise); } if (fLargeWorkForkFound) { - warnings_concise = "Warning: The network does not appear to fully agree! Some miners appear to be experiencing issues."; - warnings_verbose += (warnings_verbose.empty() ? "" : warning_separator) + _("Warning: The network does not appear to fully agree! Some miners appear to be experiencing issues.").translated; + warnings_concise = _("Warning: The network does not appear to fully agree! Some miners appear to be experiencing issues."); + warnings_verbose.emplace_back(warnings_concise); } else if (fLargeWorkInvalidChainFound) { - warnings_concise = "Warning: We do not appear to fully agree with our peers! You may need to upgrade, or other nodes may need to upgrade."; - warnings_verbose += (warnings_verbose.empty() ? "" : warning_separator) + _("Warning: We do not appear to fully agree with our peers! You may need to upgrade, or other nodes may need to upgrade.").translated; + warnings_concise = _("Warning: We do not appear to fully agree with our peers! You may need to upgrade, or other nodes may need to upgrade."); + warnings_verbose.emplace_back(warnings_concise); + } + + if (verbose) { + return Join(warnings_verbose, Untranslated("<hr />")); } - if (verbose) return warnings_verbose; - else return warnings_concise; + return warnings_concise; } diff --git a/src/warnings.h b/src/warnings.h index 83b1add1ee..28546eb753 100644 --- a/src/warnings.h +++ b/src/warnings.h @@ -8,16 +8,18 @@ #include <string> -void SetMiscWarning(const std::string& strWarning); +struct bilingual_str; + +void SetMiscWarning(const bilingual_str& warning); void SetfLargeWorkForkFound(bool flag); bool GetfLargeWorkForkFound(); void SetfLargeWorkInvalidChainFound(bool flag); /** Format a string that describes several potential problems detected by the core. * @param[in] verbose bool - * - if true, get all warnings, translated (where possible), separated by <hr /> + * - if true, get all warnings separated by <hr /> * - if false, get the most important warning * @returns the warning string */ -std::string GetWarnings(bool verbose); +bilingual_str GetWarnings(bool verbose); #endif // BITCOIN_WARNINGS_H diff --git a/test/functional/feature_notifications.py b/test/functional/feature_notifications.py index fb0c7ceed4..33a308ad1b 100755 --- a/test/functional/feature_notifications.py +++ b/test/functional/feature_notifications.py @@ -19,7 +19,7 @@ from test_framework.util import ( # Windows disallow control characters (0-31) and /\?%:|"<> FILE_CHAR_START = 32 if os.name == 'nt' else 1 FILE_CHAR_END = 128 -FILE_CHAR_BLACKLIST = '/\\?%*:|"<>' if os.name == 'nt' else '/' +FILE_CHAR_BLOCKLIST = '/\\?%*:|"<>' if os.name == 'nt' else '/' def notify_outputname(walletname, txid): @@ -32,7 +32,7 @@ class NotificationsTest(BitcoinTestFramework): self.setup_clean_chain = True def setup_network(self): - self.wallet = ''.join(chr(i) for i in range(FILE_CHAR_START, FILE_CHAR_END) if chr(i) not in FILE_CHAR_BLACKLIST) + self.wallet = ''.join(chr(i) for i in range(FILE_CHAR_START, FILE_CHAR_END) if chr(i) not in FILE_CHAR_BLOCKLIST) self.alertnotify_dir = os.path.join(self.options.tmpdir, "alertnotify") self.blocknotify_dir = os.path.join(self.options.tmpdir, "blocknotify") self.walletnotify_dir = os.path.join(self.options.tmpdir, "walletnotify") diff --git a/test/functional/mining_basic.py b/test/functional/mining_basic.py index 86d7c78d63..63d1ccfb36 100755 --- a/test/functional/mining_basic.py +++ b/test/functional/mining_basic.py @@ -18,11 +18,9 @@ from test_framework.blocktools import ( from test_framework.messages import ( CBlock, CBlockHeader, - BLOCK_HEADER_SIZE -) -from test_framework.mininode import ( - P2PDataStore, + BLOCK_HEADER_SIZE, ) +from test_framework.mininode import P2PDataStore from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, @@ -30,10 +28,15 @@ from test_framework.util import ( connect_nodes, ) + def assert_template(node, block, expect, rehash=True): if rehash: block.hashMerkleRoot = block.calc_merkle_root() - rsp = node.getblocktemplate(template_request={'data': block.serialize().hex(), 'mode': 'proposal', 'rules': ['segwit']}) + rsp = node.getblocktemplate(template_request={ + 'data': block.serialize().hex(), + 'mode': 'proposal', + 'rules': ['segwit'], + }) assert_equal(rsp, expect) @@ -85,10 +88,9 @@ class MiningTest(BitcoinTestFramework): next_height = int(tmpl["height"]) coinbase_tx = create_coinbase(height=next_height) # sequence numbers must not be max for nLockTime to have effect - coinbase_tx.vin[0].nSequence = 2 ** 32 - 2 + coinbase_tx.vin[0].nSequence = 2**32 - 2 coinbase_tx.rehash() - # round-trip the encoded bip34 block height commitment block = CBlock() block.nVersion = tmpl["version"] block.hashPrevBlock = int(tmpl["previousblockhash"], 16) @@ -116,7 +118,11 @@ class MiningTest(BitcoinTestFramework): assert_raises_rpc_error(-22, "Block does not start with a coinbase", node.submitblock, bad_block.serialize().hex()) self.log.info("getblocktemplate: Test truncated final transaction") - assert_raises_rpc_error(-22, "Block decode failed", node.getblocktemplate, {'data': block.serialize()[:-1].hex(), 'mode': 'proposal', 'rules': ['segwit']}) + assert_raises_rpc_error(-22, "Block decode failed", node.getblocktemplate, { + 'data': block.serialize()[:-1].hex(), + 'mode': 'proposal', + 'rules': ['segwit'], + }) self.log.info("getblocktemplate: Test duplicate transaction") bad_block = copy.deepcopy(block) @@ -135,7 +141,7 @@ class MiningTest(BitcoinTestFramework): self.log.info("getblocktemplate: Test nonfinal transaction") bad_block = copy.deepcopy(block) - bad_block.vtx[0].nLockTime = 2 ** 32 - 1 + bad_block.vtx[0].nLockTime = 2**32 - 1 bad_block.vtx[0].rehash() assert_template(node, bad_block, 'bad-txns-nonfinal') assert_submitblock(bad_block, 'bad-txns-nonfinal') @@ -145,7 +151,11 @@ class MiningTest(BitcoinTestFramework): bad_block_sn = bytearray(block.serialize()) assert_equal(bad_block_sn[BLOCK_HEADER_SIZE], 1) bad_block_sn[BLOCK_HEADER_SIZE] += 1 - assert_raises_rpc_error(-22, "Block decode failed", node.getblocktemplate, {'data': bad_block_sn.hex(), 'mode': 'proposal', 'rules': ['segwit']}) + assert_raises_rpc_error(-22, "Block decode failed", node.getblocktemplate, { + 'data': bad_block_sn.hex(), + 'mode': 'proposal', + 'rules': ['segwit'], + }) self.log.info("getblocktemplate: Test bad bits") bad_block = copy.deepcopy(block) @@ -160,7 +170,7 @@ class MiningTest(BitcoinTestFramework): self.log.info("getblocktemplate: Test bad timestamps") bad_block = copy.deepcopy(block) - bad_block.nTime = 2 ** 31 - 1 + bad_block.nTime = 2**31 - 1 assert_template(node, bad_block, 'time-too-new') assert_submitblock(bad_block, 'time-too-new', 'time-too-new') bad_block.nTime = 0 diff --git a/test/functional/p2p_eviction.py b/test/functional/p2p_eviction.py new file mode 100755 index 0000000000..b2b3a89aab --- /dev/null +++ b/test/functional/p2p_eviction.py @@ -0,0 +1,129 @@ +#!/usr/bin/env python3 +# Copyright (c) 2019 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. + +""" Test node eviction logic + +When the number of peers has reached the limit of maximum connections, +the next connecting inbound peer will trigger the eviction mechanism. +We cannot currently test the parts of the eviction logic that are based on +address/netgroup since in the current framework, all peers are connecting from +the same local address. See Issue #14210 for more info. +Therefore, this test is limited to the remaining protection criteria. +""" + +import time + +from test_framework.test_framework import BitcoinTestFramework +from test_framework.mininode import P2PInterface, P2PDataStore +from test_framework.util import assert_equal, wait_until +from test_framework.blocktools import create_block, create_coinbase +from test_framework.messages import CTransaction, FromHex, msg_pong, msg_tx + + +class SlowP2PDataStore(P2PDataStore): + def on_ping(self, message): + time.sleep(0.1) + self.send_message(msg_pong(message.nonce)) + +class SlowP2PInterface(P2PInterface): + def on_ping(self, message): + time.sleep(0.1) + self.send_message(msg_pong(message.nonce)) + +class P2PEvict(BitcoinTestFramework): + def set_test_params(self): + self.setup_clean_chain = True + self.num_nodes = 1 + # The choice of maxconnections=32 results in a maximum of 21 inbound connections + # (32 - 10 outbound - 1 feeler). 20 inbound peers are protected from eviction: + # 4 by netgroup, 4 that sent us blocks, 4 that sent us transactions and 8 via lowest ping time + self.extra_args = [['-maxconnections=32']] + + def run_test(self): + protected_peers = set() # peers that we expect to be protected from eviction + current_peer = -1 + node = self.nodes[0] + node.generatetoaddress(101, node.get_deterministic_priv_key().address) + + self.log.info("Create 4 peers and protect them from eviction by sending us a block") + for _ in range(4): + block_peer = node.add_p2p_connection(SlowP2PDataStore()) + current_peer += 1 + block_peer.sync_with_ping() + best_block = node.getbestblockhash() + tip = int(best_block, 16) + best_block_time = node.getblock(best_block)['time'] + block = create_block(tip, create_coinbase(node.getblockcount() + 1), best_block_time + 1) + block.solve() + block_peer.send_blocks_and_test([block], node, success=True) + protected_peers.add(current_peer) + + self.log.info("Create 5 slow-pinging peers, making them eviction candidates") + for _ in range(5): + node.add_p2p_connection(SlowP2PInterface()) + current_peer += 1 + + self.log.info("Create 4 peers and protect them from eviction by sending us a tx") + for i in range(4): + txpeer = node.add_p2p_connection(SlowP2PInterface()) + current_peer += 1 + txpeer.sync_with_ping() + + prevtx = node.getblock(node.getblockhash(i + 1), 2)['tx'][0] + rawtx = node.createrawtransaction( + inputs=[{'txid': prevtx['txid'], 'vout': 0}], + outputs=[{node.get_deterministic_priv_key().address: 50 - 0.00125}], + ) + sigtx = node.signrawtransactionwithkey( + hexstring=rawtx, + privkeys=[node.get_deterministic_priv_key().key], + prevtxs=[{ + 'txid': prevtx['txid'], + 'vout': 0, + 'scriptPubKey': prevtx['vout'][0]['scriptPubKey']['hex'], + }], + )['hex'] + txpeer.send_message(msg_tx(FromHex(CTransaction(), sigtx))) + protected_peers.add(current_peer) + + self.log.info("Create 8 peers and protect them from eviction by having faster pings") + for _ in range(8): + fastpeer = node.add_p2p_connection(P2PInterface()) + current_peer += 1 + wait_until(lambda: "ping" in fastpeer.last_message, timeout=10) + + # Make sure by asking the node what the actual min pings are + peerinfo = node.getpeerinfo() + pings = {} + for i in range(len(peerinfo)): + pings[i] = peerinfo[i]['minping'] if 'minping' in peerinfo[i] else 1000000 + sorted_pings = sorted(pings.items(), key=lambda x: x[1]) + + # Usually the 8 fast peers are protected. In rare case of unreliable pings, + # one of the slower peers might have a faster min ping though. + for i in range(8): + protected_peers.add(sorted_pings[i][0]) + + self.log.info("Create peer that triggers the eviction mechanism") + node.add_p2p_connection(SlowP2PInterface()) + + # One of the non-protected peers must be evicted. We can't be sure which one because + # 4 peers are protected via netgroup, which is identical for all peers, + # and the eviction mechanism doesn't preserve the order of identical elements. + evicted_peers = [] + for i in range(len(node.p2ps)): + if not node.p2ps[i].is_connected: + evicted_peers.append(i) + + self.log.info("Test that one peer was evicted") + self.log.debug("{} evicted peer: {}".format(len(evicted_peers), set(evicted_peers))) + assert_equal(len(evicted_peers), 1) + + self.log.info("Test that no peer expected to be protected was evicted") + self.log.debug("{} protected peers: {}".format(len(protected_peers), protected_peers)) + assert evicted_peers[0] not in protected_peers + +if __name__ == '__main__': + P2PEvict().main() diff --git a/test/functional/p2p_filter.py b/test/functional/p2p_filter.py index 15955a938c..5726a73e40 100755 --- a/test/functional/p2p_filter.py +++ b/test/functional/p2p_filter.py @@ -16,13 +16,15 @@ from test_framework.messages import ( msg_filterclear, msg_filterload, msg_getdata, + msg_mempool, + msg_version, ) -from test_framework.mininode import P2PInterface +from test_framework.mininode import P2PInterface, mininode_lock from test_framework.script import MAX_SCRIPT_ELEMENT_SIZE from test_framework.test_framework import BitcoinTestFramework -class FilterNode(P2PInterface): +class P2PBloomFilter(P2PInterface): # This is a P2SH watch-only wallet watch_script_pubkey = 'a914ffffffffffffffffffffffffffffffffffffffff87' # The initial filter (n=10, fp=0.000001) with just the above scriptPubKey added @@ -34,6 +36,11 @@ class FilterNode(P2PInterface): nFlags=1, ) + def __init__(self): + super().__init__() + self._tx_received = False + self._merkleblock_received = False + def on_inv(self, message): want = msg_getdata() for i in message.inv: @@ -46,10 +53,30 @@ class FilterNode(P2PInterface): self.send_message(want) def on_merkleblock(self, message): - self.merkleblock_received = True + self._merkleblock_received = True def on_tx(self, message): - self.tx_received = True + self._tx_received = True + + @property + def tx_received(self): + with mininode_lock: + return self._tx_received + + @tx_received.setter + def tx_received(self, value): + with mininode_lock: + self._tx_received = value + + @property + def merkleblock_received(self): + with mininode_lock: + return self._merkleblock_received + + @merkleblock_received.setter + def merkleblock_received(self, value): + with mininode_lock: + self._merkleblock_received = value class FilterTest(BitcoinTestFramework): @@ -64,95 +91,144 @@ class FilterTest(BitcoinTestFramework): def skip_test_if_missing_module(self): self.skip_if_no_wallet() - def test_size_limits(self, filter_node): + def test_size_limits(self, filter_peer): self.log.info('Check that too large filter is rejected') with self.nodes[0].assert_debug_log(['Misbehaving']): - filter_node.send_and_ping(msg_filterload(data=b'\xbb'*(MAX_BLOOM_FILTER_SIZE+1))) + filter_peer.send_and_ping(msg_filterload(data=b'\xbb'*(MAX_BLOOM_FILTER_SIZE+1))) self.log.info('Check that max size filter is accepted') with self.nodes[0].assert_debug_log([], unexpected_msgs=['Misbehaving']): - filter_node.send_and_ping(msg_filterload(data=b'\xbb'*(MAX_BLOOM_FILTER_SIZE))) - filter_node.send_and_ping(msg_filterclear()) + filter_peer.send_and_ping(msg_filterload(data=b'\xbb'*(MAX_BLOOM_FILTER_SIZE))) + filter_peer.send_and_ping(msg_filterclear()) self.log.info('Check that filter with too many hash functions is rejected') with self.nodes[0].assert_debug_log(['Misbehaving']): - filter_node.send_and_ping(msg_filterload(data=b'\xaa', nHashFuncs=MAX_BLOOM_HASH_FUNCS+1)) + filter_peer.send_and_ping(msg_filterload(data=b'\xaa', nHashFuncs=MAX_BLOOM_HASH_FUNCS+1)) self.log.info('Check that filter with max hash functions is accepted') with self.nodes[0].assert_debug_log([], unexpected_msgs=['Misbehaving']): - filter_node.send_and_ping(msg_filterload(data=b'\xaa', nHashFuncs=MAX_BLOOM_HASH_FUNCS)) + filter_peer.send_and_ping(msg_filterload(data=b'\xaa', nHashFuncs=MAX_BLOOM_HASH_FUNCS)) # Don't send filterclear until next two filteradd checks are done self.log.info('Check that max size data element to add to the filter is accepted') with self.nodes[0].assert_debug_log([], unexpected_msgs=['Misbehaving']): - filter_node.send_and_ping(msg_filteradd(data=b'\xcc'*(MAX_SCRIPT_ELEMENT_SIZE))) + filter_peer.send_and_ping(msg_filteradd(data=b'\xcc'*(MAX_SCRIPT_ELEMENT_SIZE))) self.log.info('Check that too large data element to add to the filter is rejected') with self.nodes[0].assert_debug_log(['Misbehaving']): - filter_node.send_and_ping(msg_filteradd(data=b'\xcc'*(MAX_SCRIPT_ELEMENT_SIZE+1))) + filter_peer.send_and_ping(msg_filteradd(data=b'\xcc'*(MAX_SCRIPT_ELEMENT_SIZE+1))) - filter_node.send_and_ping(msg_filterclear()) + filter_peer.send_and_ping(msg_filterclear()) - def run_test(self): - filter_node = self.nodes[0].add_p2p_connection(FilterNode()) + def test_msg_mempool(self): + self.log.info("Check that a node with bloom filters enabled services p2p mempool messages") + filter_peer = P2PBloomFilter() - self.test_size_limits(filter_node) + self.log.info("Create a tx relevant to the peer before connecting") + filter_address = self.nodes[0].decodescript(filter_peer.watch_script_pubkey)['addresses'][0] + txid = self.nodes[0].sendtoaddress(filter_address, 90) - self.log.info('Add filtered P2P connection to the node') - filter_node.send_and_ping(filter_node.watch_filter_init) - filter_address = self.nodes[0].decodescript(filter_node.watch_script_pubkey)['addresses'][0] + self.log.info("Send a mempool msg after connecting and check that the tx is received") + self.nodes[0].add_p2p_connection(filter_peer) + filter_peer.send_and_ping(filter_peer.watch_filter_init) + self.nodes[0].p2p.send_message(msg_mempool()) + filter_peer.wait_for_tx(txid) + + def test_frelay_false(self, filter_peer): + self.log.info("Check that a node with fRelay set to false does not receive invs until the filter is set") + filter_peer.tx_received = False + filter_address = self.nodes[0].decodescript(filter_peer.watch_script_pubkey)['addresses'][0] + self.nodes[0].sendtoaddress(filter_address, 90) + # Sync to make sure the reason filter_peer doesn't receive the tx is not p2p delays + filter_peer.sync_with_ping() + assert not filter_peer.tx_received + + # Clear the mempool so that this transaction does not impact subsequent tests + self.nodes[0].generate(1) + + def test_filter(self, filter_peer): + # Set the bloomfilter using filterload + filter_peer.send_and_ping(filter_peer.watch_filter_init) + # If fRelay is not already True, sending filterload sets it to True + assert self.nodes[0].getpeerinfo()[0]['relaytxes'] + filter_address = self.nodes[0].decodescript(filter_peer.watch_script_pubkey)['addresses'][0] self.log.info('Check that we receive merkleblock and tx if the filter matches a tx in a block') block_hash = self.nodes[0].generatetoaddress(1, filter_address)[0] txid = self.nodes[0].getblock(block_hash)['tx'][0] - filter_node.wait_for_merkleblock(block_hash) - filter_node.wait_for_tx(txid) + filter_peer.wait_for_merkleblock(block_hash) + filter_peer.wait_for_tx(txid) self.log.info('Check that we only receive a merkleblock if the filter does not match a tx in a block') - filter_node.tx_received = False + filter_peer.tx_received = False block_hash = self.nodes[0].generatetoaddress(1, self.nodes[0].getnewaddress())[0] - filter_node.wait_for_merkleblock(block_hash) - assert not filter_node.tx_received + filter_peer.wait_for_merkleblock(block_hash) + assert not filter_peer.tx_received self.log.info('Check that we not receive a tx if the filter does not match a mempool tx') - filter_node.merkleblock_received = False - filter_node.tx_received = False + filter_peer.merkleblock_received = False + filter_peer.tx_received = False self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 90) - filter_node.sync_with_ping() - filter_node.sync_with_ping() - assert not filter_node.merkleblock_received - assert not filter_node.tx_received + filter_peer.sync_with_ping() + filter_peer.sync_with_ping() + assert not filter_peer.merkleblock_received + assert not filter_peer.tx_received - self.log.info('Check that we receive a tx in reply to a mempool msg if the filter matches a mempool tx') - filter_node.merkleblock_received = False + self.log.info('Check that we receive a tx if the filter matches a mempool tx') + filter_peer.merkleblock_received = False txid = self.nodes[0].sendtoaddress(filter_address, 90) - filter_node.wait_for_tx(txid) - assert not filter_node.merkleblock_received + filter_peer.wait_for_tx(txid) + assert not filter_peer.merkleblock_received self.log.info('Check that after deleting filter all txs get relayed again') - filter_node.send_and_ping(msg_filterclear()) + filter_peer.send_and_ping(msg_filterclear()) for _ in range(5): txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 7) - filter_node.wait_for_tx(txid) + filter_peer.wait_for_tx(txid) self.log.info('Check that request for filtered blocks is ignored if no filter is set') - filter_node.merkleblock_received = False - filter_node.tx_received = False + filter_peer.merkleblock_received = False + filter_peer.tx_received = False with self.nodes[0].assert_debug_log(expected_msgs=['received getdata']): block_hash = self.nodes[0].generatetoaddress(1, self.nodes[0].getnewaddress())[0] - filter_node.wait_for_inv([CInv(MSG_BLOCK, int(block_hash, 16))]) - filter_node.sync_with_ping() - assert not filter_node.merkleblock_received - assert not filter_node.tx_received + filter_peer.wait_for_inv([CInv(MSG_BLOCK, int(block_hash, 16))]) + filter_peer.sync_with_ping() + assert not filter_peer.merkleblock_received + assert not filter_peer.tx_received self.log.info('Check that sending "filteradd" if no filter is set is treated as misbehavior') with self.nodes[0].assert_debug_log(['Misbehaving']): - filter_node.send_and_ping(msg_filteradd(data=b'letsmisbehave')) + filter_peer.send_and_ping(msg_filteradd(data=b'letsmisbehave')) self.log.info("Check that division-by-zero remote crash bug [CVE-2013-5700] is fixed") - filter_node.send_and_ping(msg_filterload(data=b'', nHashFuncs=1)) - filter_node.send_and_ping(msg_filteradd(data=b'letstrytocrashthisnode')) + filter_peer.send_and_ping(msg_filterload(data=b'', nHashFuncs=1)) + filter_peer.send_and_ping(msg_filteradd(data=b'letstrytocrashthisnode')) + self.nodes[0].disconnect_p2ps() + def run_test(self): + filter_peer = self.nodes[0].add_p2p_connection(P2PBloomFilter()) + self.log.info('Test filter size limits') + self.test_size_limits(filter_peer) + + self.log.info('Test BIP 37 for a node with fRelay = True (default)') + self.test_filter(filter_peer) + self.nodes[0].disconnect_p2ps() + + self.log.info('Test BIP 37 for a node with fRelay = False') + # Add peer but do not send version yet + filter_peer_without_nrelay = self.nodes[0].add_p2p_connection(P2PBloomFilter(), send_version=False, wait_for_verack=False) + # Send version with fRelay=False + filter_peer_without_nrelay.wait_until(lambda: filter_peer_without_nrelay.is_connected, timeout=10) + version_without_fRelay = msg_version() + version_without_fRelay.nRelay = 0 + filter_peer_without_nrelay.send_message(version_without_fRelay) + filter_peer_without_nrelay.wait_for_verack() + assert not self.nodes[0].getpeerinfo()[0]['relaytxes'] + self.test_frelay_false(filter_peer_without_nrelay) + self.test_filter(filter_peer_without_nrelay) + + self.log.info('Test msg_mempool') + self.test_msg_mempool() if __name__ == '__main__': FilterTest().main() diff --git a/test/functional/p2p_invalid_messages.py b/test/functional/p2p_invalid_messages.py index 81302374c9..771fac08ca 100755 --- a/test/functional/p2p_invalid_messages.py +++ b/test/functional/p2p_invalid_messages.py @@ -4,8 +4,6 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test node responses to invalid network messages.""" import asyncio -import struct -import sys from test_framework.messages import ( CBlockHeader, @@ -24,6 +22,8 @@ from test_framework.mininode import ( ) from test_framework.test_framework import BitcoinTestFramework +MSG_LIMIT = 4 * 1000 * 1000 # 4MB, per MAX_PROTOCOL_MESSAGE_LENGTH +VALID_DATA_LIMIT = MSG_LIMIT - 5 # Account for the 5-byte length prefix class msg_unrecognized: """Nonsensical message. Modeled after similar types in test_framework.messages.""" @@ -46,114 +46,12 @@ class InvalidMessagesTest(BitcoinTestFramework): self.setup_clean_chain = True def run_test(self): - """ - . Test msg header - 0. Send a bunch of large (4MB) messages of an unrecognized type. Check to see - that it isn't an effective DoS against the node. - - 1. Send an oversized (4MB+) message and check that we're disconnected. - - 2. Send a few messages with an incorrect data size in the header, ensure the - messages are ignored. - """ self.test_magic_bytes() self.test_checksum() self.test_size() self.test_msgtype() self.test_large_inv() - - node = self.nodes[0] - self.node = node - node.add_p2p_connection(P2PDataStore()) - conn2 = node.add_p2p_connection(P2PDataStore()) - - msg_limit = 4 * 1000 * 1000 # 4MB, per MAX_PROTOCOL_MESSAGE_LENGTH - valid_data_limit = msg_limit - 5 # Account for the 4-byte length prefix - - # - # 0. - # - # Send as large a message as is valid, ensure we aren't disconnected but - # also can't exhaust resources. - # - msg_at_size = msg_unrecognized(str_data="b" * valid_data_limit) - assert len(msg_at_size.serialize()) == msg_limit - - self.log.info("Sending a bunch of large, junk messages to test memory exhaustion. May take a bit...") - - # Run a bunch of times to test for memory exhaustion. - for _ in range(80): - node.p2p.send_message(msg_at_size) - - # Check that, even though the node is being hammered by nonsense from one - # connection, it can still service other peers in a timely way. - for _ in range(20): - conn2.sync_with_ping(timeout=2) - - # Peer 1, despite serving up a bunch of nonsense, should still be connected. - self.log.info("Waiting for node to drop junk messages.") - node.p2p.sync_with_ping(timeout=400) - assert node.p2p.is_connected - - # - # 1. - # - # Send an oversized message, ensure we're disconnected. - # - # Under macOS this test is skipped due to an unexpected error code - # returned from the closing socket which python/asyncio does not - # yet know how to handle. - # - if sys.platform != 'darwin': - msg_over_size = msg_unrecognized(str_data="b" * (valid_data_limit + 1)) - assert len(msg_over_size.serialize()) == (msg_limit + 1) - - # An unknown message type (or *any* message type) over - # MAX_PROTOCOL_MESSAGE_LENGTH should result in a disconnect. - node.p2p.send_message(msg_over_size) - node.p2p.wait_for_disconnect(timeout=4) - - node.disconnect_p2ps() - conn = node.add_p2p_connection(P2PDataStore()) - conn.wait_for_verack() - else: - self.log.info("Skipping test p2p_invalid_messages/1 (oversized message) under macOS") - - # - # 2. - # - # Send messages with an incorrect data size in the header. - # - actual_size = 100 - msg = msg_unrecognized(str_data="b" * actual_size) - - # TODO: handle larger-than cases. I haven't been able to pin down what behavior to expect. - for wrong_size in (2, 77, 78, 79): - self.log.info("Sending a message with incorrect size of {}".format(wrong_size)) - - # Unmodified message should submit okay. - node.p2p.send_and_ping(msg) - - # A message lying about its data size results in a disconnect when the incorrect - # data size is less than the actual size. - # - # TODO: why does behavior change at 78 bytes? - # - node.p2p.send_raw_message(self._tweak_msg_data_size(msg, wrong_size)) - - # For some reason unknown to me, we sometimes have to push additional data to the - # peer in order for it to realize a disconnect. - try: - node.p2p.send_message(msg_ping(nonce=123123)) - except IOError: - pass - - node.p2p.wait_for_disconnect(timeout=10) - node.disconnect_p2ps() - node.add_p2p_connection(P2PDataStore()) - - # Node is still up. - conn = node.add_p2p_connection(P2PDataStore()) + self.test_resource_exhaustion() def test_magic_bytes(self): conn = self.nodes[0].add_p2p_connection(P2PDataStore()) @@ -189,13 +87,9 @@ class InvalidMessagesTest(BitcoinTestFramework): def test_size(self): conn = self.nodes[0].add_p2p_connection(P2PDataStore()) with self.nodes[0].assert_debug_log(['']): - msg = conn.build_message(msg_unrecognized(str_data="d")) - cut_len = ( - 4 + # magic - 12 # msgtype - ) - # modify len to MAX_SIZE + 1 - msg = msg[:cut_len] + struct.pack("<I", 0x02000000 + 1) + msg[cut_len + 4:] + # Create a message with oversized payload + msg = msg_unrecognized(str_data="d"*(VALID_DATA_LIMIT + 1)) + msg = conn.build_message(msg) self.nodes[0].p2p.send_raw_message(msg) conn.wait_for_disconnect(timeout=1) self.nodes[0].disconnect_p2ps() @@ -225,25 +119,28 @@ class InvalidMessagesTest(BitcoinTestFramework): conn.send_and_ping(msg) self.nodes[0].disconnect_p2ps() - def _tweak_msg_data_size(self, message, wrong_size): - """ - Return a raw message based on another message but with an incorrect data size in - the message header. - """ - raw_msg = self.node.p2p.build_message(message) - - bad_size_bytes = struct.pack("<I", wrong_size) - num_header_bytes_before_size = 4 + 12 - - # Replace the correct data size in the message with an incorrect one. - raw_msg_with_wrong_size = ( - raw_msg[:num_header_bytes_before_size] + - bad_size_bytes + - raw_msg[(num_header_bytes_before_size + len(bad_size_bytes)):] - ) - assert len(raw_msg) == len(raw_msg_with_wrong_size) - - return raw_msg_with_wrong_size + def test_resource_exhaustion(self): + conn = self.nodes[0].add_p2p_connection(P2PDataStore()) + conn2 = self.nodes[0].add_p2p_connection(P2PDataStore()) + msg_at_size = msg_unrecognized(str_data="b" * VALID_DATA_LIMIT) + assert len(msg_at_size.serialize()) == MSG_LIMIT + + self.log.info("Sending a bunch of large, junk messages to test memory exhaustion. May take a bit...") + + # Run a bunch of times to test for memory exhaustion. + for _ in range(80): + conn.send_message(msg_at_size) + + # Check that, even though the node is being hammered by nonsense from one + # connection, it can still service other peers in a timely way. + for _ in range(20): + conn2.sync_with_ping(timeout=2) + + # Peer 1, despite being served up a bunch of nonsense, should still be connected. + self.log.info("Waiting for node to drop junk messages.") + conn.sync_with_ping(timeout=400) + assert conn.is_connected + self.nodes[0].disconnect_p2ps() if __name__ == '__main__': diff --git a/test/functional/p2p_mempool.py b/test/functional/p2p_mempool.py deleted file mode 100755 index a8fcb181e6..0000000000 --- a/test/functional/p2p_mempool.py +++ /dev/null @@ -1,34 +0,0 @@ -#!/usr/bin/env python3 -# Copyright (c) 2015-2018 The Bitcoin Core developers -# Distributed under the MIT software license, see the accompanying -# file COPYING or http://www.opensource.org/licenses/mit-license.php. -"""Test p2p mempool message. - -Test that nodes are disconnected if they send mempool messages when bloom -filters are not enabled. -""" - -from test_framework.messages import msg_mempool -from test_framework.mininode import P2PInterface -from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import assert_equal - -class P2PMempoolTests(BitcoinTestFramework): - def set_test_params(self): - self.setup_clean_chain = True - self.num_nodes = 1 - self.extra_args = [["-peerbloomfilters=0"]] - - def run_test(self): - # Add a p2p connection - self.nodes[0].add_p2p_connection(P2PInterface()) - - #request mempool - self.nodes[0].p2p.send_message(msg_mempool()) - self.nodes[0].p2p.wait_for_disconnect() - - #mininode must be disconnected at this point - assert_equal(len(self.nodes[0].getpeerinfo()), 0) - -if __name__ == '__main__': - P2PMempoolTests().main() diff --git a/test/functional/p2p_nobloomfilter_messages.py b/test/functional/p2p_nobloomfilter_messages.py new file mode 100755 index 0000000000..41af74ebb8 --- /dev/null +++ b/test/functional/p2p_nobloomfilter_messages.py @@ -0,0 +1,43 @@ +#!/usr/bin/env python3 +# Copyright (c) 2015-2018 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test invalid p2p messages for nodes with bloom filters disabled. + +Test that, when bloom filters are not enabled, nodes are disconnected if: +1. They send a p2p mempool message +2. They send a p2p filterload message +3. They send a p2p filteradd message +""" + +from test_framework.messages import msg_mempool, msg_filteradd, msg_filterload +from test_framework.mininode import P2PInterface +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import assert_equal + + +class P2PNobloomfilterMessages(BitcoinTestFramework): + def set_test_params(self): + self.setup_clean_chain = True + self.num_nodes = 1 + self.extra_args = [["-peerbloomfilters=0"]] + + def test_message_causes_disconnect(self, message): + # Add a p2p connection that sends a message and check that it disconnects + peer = self.nodes[0].add_p2p_connection(P2PInterface()) + peer.send_message(message) + peer.wait_for_disconnect() + assert_equal(len(self.nodes[0].getpeerinfo()), 0) + + def run_test(self): + self.log.info("Test that node is disconnected if it sends mempool message") + self.test_message_causes_disconnect(msg_mempool()) + + self.log.info("Test that node is disconnected if it sends filterload message") + self.test_message_causes_disconnect(msg_filterload()) + + self.log.info("Test that node is disconnected if it sends filteradd message") + self.test_message_causes_disconnect(msg_filteradd(data=b'\xcc')) + +if __name__ == '__main__': + P2PNobloomfilterMessages().main() diff --git a/test/functional/rpc_createmultisig.py b/test/functional/rpc_createmultisig.py index 56e9ecfcc2..3c81a4a4e2 100755 --- a/test/functional/rpc_createmultisig.py +++ b/test/functional/rpc_createmultisig.py @@ -3,21 +3,21 @@ # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test multisig RPCs""" +import binascii +import decimal +import itertools +import json +import os from test_framework.authproxy import JSONRPCException from test_framework.descriptors import descsum_create, drop_origins +from test_framework.key import ECPubKey, ECKey from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_raises_rpc_error, assert_equal, ) -from test_framework.key import ECPubKey, ECKey, bytes_to_wif - -import binascii -import decimal -import itertools -import json -import os +from test_framework.wallet_util import bytes_to_wif class RpcCreateMultiSigTest(BitcoinTestFramework): def set_test_params(self): diff --git a/test/functional/rpc_help.py b/test/functional/rpc_help.py index 027ae368e7..9b981b864e 100755 --- a/test/functional/rpc_help.py +++ b/test/functional/rpc_help.py @@ -18,6 +18,8 @@ class HelpRpcTest(BitcoinTestFramework): def run_test(self): self.test_categories() self.dump_help() + if self.is_wallet_compiled(): + self.wallet_help() def test_categories(self): node = self.nodes[0] @@ -53,6 +55,11 @@ class HelpRpcTest(BitcoinTestFramework): # Make sure the node can generate the help at runtime without crashing f.write(self.nodes[0].help(call)) + def wallet_help(self): + assert 'getnewaddress ( "label" "address_type" )' in self.nodes[0].help('getnewaddress') + self.restart_node(0, extra_args=['-nowallet=1']) + assert 'getnewaddress ( "label" "address_type" )' in self.nodes[0].help('getnewaddress') + if __name__ == '__main__': HelpRpcTest().main() diff --git a/test/functional/test_framework/key.py b/test/functional/test_framework/key.py index f2d6fba4a6..912c0ca978 100644 --- a/test/functional/test_framework/key.py +++ b/test/functional/test_framework/key.py @@ -8,8 +8,6 @@ keys, and is trivially vulnerable to side channel attacks. Do not use for anything but tests.""" import random -from .address import byte_to_base58 - def modinv(a, n): """Compute the modular inverse of a modulo n @@ -386,14 +384,3 @@ class ECKey(): rb = r.to_bytes((r.bit_length() + 8) // 8, 'big') sb = s.to_bytes((s.bit_length() + 8) // 8, 'big') return b'\x30' + bytes([4 + len(rb) + len(sb), 2, len(rb)]) + rb + bytes([2, len(sb)]) + sb - -def bytes_to_wif(b, compressed=True): - if compressed: - b += b'\x01' - return byte_to_base58(b, 239) - -def generate_wif_key(): - # Makes a WIF privkey for imports - k = ECKey() - k.generate() - return bytes_to_wif(k.get_bytes(), k.is_compressed) diff --git a/test/functional/test_framework/wallet_util.py b/test/functional/test_framework/wallet_util.py index 1b6686ff45..b9c0fb6691 100755 --- a/test/functional/test_framework/wallet_util.py +++ b/test/functional/test_framework/wallet_util.py @@ -6,6 +6,7 @@ from collections import namedtuple from test_framework.address import ( + byte_to_base58, key_to_p2pkh, key_to_p2sh_p2wpkh, key_to_p2wpkh, @@ -13,10 +14,7 @@ from test_framework.address import ( script_to_p2sh_p2wsh, script_to_p2wsh, ) -from test_framework.key import ( - bytes_to_wif, - ECKey, -) +from test_framework.key import ECKey from test_framework.script import ( CScript, OP_0, @@ -120,3 +118,14 @@ def test_address(node, address, **kwargs): raise AssertionError("key {} unexpectedly returned in getaddressinfo.".format(key)) elif addr_info[key] != value: raise AssertionError("key {} value {} did not match expected value {}".format(key, addr_info[key], value)) + +def bytes_to_wif(b, compressed=True): + if compressed: + b += b'\x01' + return byte_to_base58(b, 239) + +def generate_wif_key(): + # Makes a WIF privkey for imports + k = ECKey() + k.generate() + return bytes_to_wif(k.get_bytes(), k.is_compressed) diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 0ea65c68b8..8b11dc8376 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -164,7 +164,7 @@ BASE_SCRIPTS = [ 'wallet_keypool.py', 'wallet_keypool.py --descriptors', 'wallet_descriptor.py', - 'p2p_mempool.py', + 'p2p_nobloomfilter_messages.py', 'p2p_filter.py', 'rpc_setban.py', 'p2p_blocksonly.py', @@ -190,6 +190,7 @@ BASE_SCRIPTS = [ 'rpc_preciousblock.py', 'wallet_importprunedfunds.py', 'p2p_leak_tx.py', + 'p2p_eviction.py', 'rpc_signmessage.py', 'rpc_generateblock.py', 'wallet_balance.py', diff --git a/test/sanitizer_suppressions/tsan b/test/sanitizer_suppressions/tsan index fd2e1a329c..b8fe75c5c5 100644 --- a/test/sanitizer_suppressions/tsan +++ b/test/sanitizer_suppressions/tsan @@ -20,6 +20,7 @@ race:fHaveGenesis race:ProcessNewBlock race:ThreadImport race:WalletBatch::WriteHDChain +race:BerkeleyDatabase race:zmq::* race:bitcoin-qt # deadlock (TODO fix) |