From 6fe5c69dfed7357bd68abe06f2dbed7addbb14b3 Mon Sep 17 00:00:00 2001 From: marcoagner Date: Thu, 19 Jul 2018 12:14:05 +0100 Subject: [PATCH 01/31] tests: fixes mininode's P2PConnection sending messages on closing transport - checks if _transport.is_closing() (added in python3.4.4/python3.5.1) before attempting to send messages on P2PConnection's send_message method. --- test/functional/test_framework/mininode.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/functional/test_framework/mininode.py b/test/functional/test_framework/mininode.py index 29bf33fa5b..d5b1a90687 100755 --- a/test/functional/test_framework/mininode.py +++ b/test/functional/test_framework/mininode.py @@ -179,7 +179,7 @@ def send_message(self, message): raise IOError('Not connected') self._log_message("send", message) tmsg = self._build_message(message) - NetworkThread.network_event_loop.call_soon_threadsafe(lambda: self._transport and self._transport.write(tmsg)) + NetworkThread.network_event_loop.call_soon_threadsafe(lambda: self._transport and not self._transport.is_closing() and self._transport.write(tmsg)) # Class utility methods From eb26c2e4f79deb17622791037f6adf022e869885 Mon Sep 17 00:00:00 2001 From: Lawrence Nahum Date: Tue, 17 Jul 2018 20:21:20 +0200 Subject: [PATCH 02/31] depends: disable Werror for zmqlib release, causes ndk build to break --- depends/packages/zeromq.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/depends/packages/zeromq.mk b/depends/packages/zeromq.mk index 9412c181e6..d5fd1f39ab 100644 --- a/depends/packages/zeromq.mk +++ b/depends/packages/zeromq.mk @@ -6,7 +6,7 @@ $(package)_sha256_hash=8f1e2b2aade4dbfde98d82366d61baef2f62e812530160d2e6d0a5bb2 $(package)_patches=0001-fix-build-with-older-mingw64.patch 0002-disable-pthread_set_name_np.patch define $(package)_set_vars - $(package)_config_opts=--without-docs --disable-shared --without-libsodium --disable-curve --disable-curve-keygen --disable-perf + $(package)_config_opts=--without-docs --disable-shared --without-libsodium --disable-curve --disable-curve-keygen --disable-perf --disable-Werror $(package)_config_opts_linux=--with-pic $(package)_cxxflags=-std=c++11 endef From c8a7b10b98815687999499c85b3c48d14da0ba22 Mon Sep 17 00:00:00 2001 From: John Newbery Date: Wed, 25 Apr 2018 13:08:35 -0500 Subject: [PATCH 03/31] [wallet] [rpc] Fix importaddress help text --- src/wallet/rpcdump.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 8c6e1891b6..dad9a2e178 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -257,9 +257,9 @@ UniValue importaddress(const JSONRPCRequest& request) if (request.fHelp || request.params.size() < 1 || request.params.size() > 4) throw std::runtime_error( "importaddress \"address\" ( \"label\" rescan p2sh )\n" - "\nAdds a script (in hex) or address that can be watched as if it were in your wallet but cannot be used to spend. Requires a new wallet backup.\n" + "\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" "\nArguments:\n" - "1. \"script\" (string, required) The hex-encoded script (or address)\n" + "1. \"address\" (string, required) The Bitcoin address (or hex-encoded script)\n" "2. \"label\" (string, optional, default=\"\") An optional label\n" "3. rescan (boolean, optional, default=true) Rescan the wallet for transactions\n" "4. p2sh (boolean, optional, default=false) Add the P2SH version of the script as well\n" @@ -269,12 +269,12 @@ UniValue importaddress(const JSONRPCRequest& request) "\nNote: If you import a non-standard raw script in hex form, outputs sending to it will be treated\n" "as change, and not show up in many RPCs.\n" "\nExamples:\n" - "\nImport a script with rescan\n" - + HelpExampleCli("importaddress", "\"myscript\"") + + "\nImport an address with rescan\n" + + HelpExampleCli("importaddress", "\"myaddress\"") + "\nImport using a label without rescan\n" - + HelpExampleCli("importaddress", "\"myscript\" \"testing\" false") + + + HelpExampleCli("importaddress", "\"myaddress\" \"testing\" false") + "\nAs a JSON-RPC call\n" - + HelpExampleRpc("importaddress", "\"myscript\", \"testing\", false") + + HelpExampleRpc("importaddress", "\"myaddress\", \"testing\", false") ); From e4cfbaa7fc8a2740f8e6409a24f4f17296bf4ee1 Mon Sep 17 00:00:00 2001 From: practicalswift Date: Thu, 19 Jul 2018 10:30:20 +0200 Subject: [PATCH 04/31] wallet: Add error handling. Check return value of ParseUInt32(...) in ParseHDKeypath(...). --- src/wallet/rpcwallet.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 913ad74aec..948bf1b191 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -4433,7 +4433,9 @@ bool ParseHDKeypath(std::string keypath_str, std::vector& keypath) return false; } uint32_t number; - ParseUInt32(item, &number); + if (!ParseUInt32(item, &number)) { + return false; + } path |= number; keypath.push_back(path); From a9bedcadc2af6fd2ecd01c827a4837fea4680469 Mon Sep 17 00:00:00 2001 From: practicalswift Date: Thu, 19 Jul 2018 14:52:09 +0200 Subject: [PATCH 05/31] wallet: Add tests for ParseHDKeypath(...) --- src/wallet/test/psbt_wallet_tests.cpp | 76 +++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/src/wallet/test/psbt_wallet_tests.cpp b/src/wallet/test/psbt_wallet_tests.cpp index a74ca85349..2cc995bf04 100644 --- a/src/wallet/test/psbt_wallet_tests.cpp +++ b/src/wallet/test/psbt_wallet_tests.cpp @@ -13,6 +13,8 @@ #include #include +extern bool ParseHDKeypath(std::string keypath_str, std::vector& keypath); + BOOST_FIXTURE_TEST_SUITE(psbt_wallet_tests, WalletTestingSetup) BOOST_AUTO_TEST_CASE(psbt_updater_test) @@ -71,4 +73,78 @@ BOOST_AUTO_TEST_CASE(psbt_updater_test) BOOST_CHECK_EQUAL(final_hex, "70736274ff01009a020000000258e87a21b56daf0c23be8e7070456c336f7cbaa5c8757924f545887bb2abdd750000000000ffffffff838d0427d0ec650a68aa46bb0b098aea4422c071b2ca78352a077959d07cea1d0100000000ffffffff0270aaf00800000000160014d85c2b71d0060b09c9886aeb815e50991dda124d00e1f5050000000016001400aea9a2e5f0f876a588df5546e8742d1d87008f00000000000100bb0200000001aad73931018bd25f84ae400b68848be09db706eac2ac18298babee71ab656f8b0000000048473044022058f6fc7c6a33e1b31548d481c826c015bd30135aad42cd67790dab66d2ad243b02204a1ced2604c6735b6393e5b41691dd78b00f0c5942fb9f751856faa938157dba01feffffff0280f0fa020000000017a9140fb9463421696b82c833af241c78c17ddbde493487d0f20a270100000017a91429ca74f8a08f81999428185c97b5d852e4063f6187650000000104475221029583bf39ae0a609747ad199addd634fa6108559d6c5cd39b4c2183f1ab96e07f2102dab61ff49a14db6a7d02b0cd1fbb78fc4b18312b5b4e54dae4dba2fbfef536d752ae2206029583bf39ae0a609747ad199addd634fa6108559d6c5cd39b4c2183f1ab96e07f10d90c6a4f000000800000008000000080220602dab61ff49a14db6a7d02b0cd1fbb78fc4b18312b5b4e54dae4dba2fbfef536d710d90c6a4f0000008000000080010000800001012000c2eb0b0000000017a914b7f5faf40e3d40a5a459b1db3535f2b72fa921e88701042200208c2353173743b595dfb4a07b72ba8e42e3797da74e87fe7d9d7497e3b2028903010547522103089dc10c7ac6db54f91329af617333db388cead0c231f723379d1b99030b02dc21023add904f3d6dcf59ddb906b0dee23529b7ffb9ed50e5e86151926860221f0e7352ae2206023add904f3d6dcf59ddb906b0dee23529b7ffb9ed50e5e86151926860221f0e7310d90c6a4f000000800000008003000080220603089dc10c7ac6db54f91329af617333db388cead0c231f723379d1b99030b02dc10d90c6a4f00000080000000800200008000220203a9a4c37f5996d3aa25dbac6b570af0650394492942460b354753ed9eeca5877110d90c6a4f000000800000008004000080002202027f6399757d2eff55a136ad02c684b1838b6556e5f1b6b34282a94b6b5005109610d90c6a4f00000080000000800500008000"); } +BOOST_AUTO_TEST_CASE(parse_hd_keypath) +{ + std::vector keypath; + + BOOST_CHECK(ParseHDKeypath("1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1", keypath)); + BOOST_CHECK(!ParseHDKeypath("///////////////////////////", keypath)); + + BOOST_CHECK(ParseHDKeypath("1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1'/1", keypath)); + BOOST_CHECK(!ParseHDKeypath("//////////////////////////'/", keypath)); + + BOOST_CHECK(ParseHDKeypath("1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/", keypath)); + BOOST_CHECK(!ParseHDKeypath("1///////////////////////////", keypath)); + + BOOST_CHECK(ParseHDKeypath("1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1'/", keypath)); + BOOST_CHECK(!ParseHDKeypath("1/'//////////////////////////", keypath)); + + BOOST_CHECK(ParseHDKeypath("", keypath)); + BOOST_CHECK(!ParseHDKeypath(" ", keypath)); + + BOOST_CHECK(ParseHDKeypath("0", keypath)); + BOOST_CHECK(!ParseHDKeypath("O", keypath)); + + BOOST_CHECK(ParseHDKeypath("0000'/0000'/0000'", keypath)); + BOOST_CHECK(!ParseHDKeypath("0000,/0000,/0000,", keypath)); + + BOOST_CHECK(ParseHDKeypath("01234", keypath)); + BOOST_CHECK(!ParseHDKeypath("0x1234", keypath)); + + BOOST_CHECK(ParseHDKeypath("1", keypath)); + BOOST_CHECK(!ParseHDKeypath(" 1", keypath)); + + BOOST_CHECK(ParseHDKeypath("42", keypath)); + BOOST_CHECK(!ParseHDKeypath("m42", keypath)); + + BOOST_CHECK(ParseHDKeypath("4294967295", keypath)); // 4294967295 == 0xFFFFFFFF (uint32_t max) + BOOST_CHECK(!ParseHDKeypath("4294967296", keypath)); // 4294967296 == 0xFFFFFFFF (uint32_t max) + 1 + + BOOST_CHECK(ParseHDKeypath("m", keypath)); + BOOST_CHECK(!ParseHDKeypath("n", keypath)); + + BOOST_CHECK(ParseHDKeypath("m/", keypath)); + BOOST_CHECK(!ParseHDKeypath("n/", keypath)); + + BOOST_CHECK(ParseHDKeypath("m/0", keypath)); + BOOST_CHECK(!ParseHDKeypath("n/0", keypath)); + + BOOST_CHECK(ParseHDKeypath("m/0'", keypath)); + BOOST_CHECK(!ParseHDKeypath("m/0''", keypath)); + + BOOST_CHECK(ParseHDKeypath("m/0'/0'", keypath)); + BOOST_CHECK(!ParseHDKeypath("m/'0/0'", keypath)); + + BOOST_CHECK(ParseHDKeypath("m/0/0", keypath)); + BOOST_CHECK(!ParseHDKeypath("n/0/0", keypath)); + + BOOST_CHECK(ParseHDKeypath("m/0/0/00", keypath)); + BOOST_CHECK(!ParseHDKeypath("m/0/0/f00", keypath)); + + BOOST_CHECK(ParseHDKeypath("m/0/0/000000000000000000000000000000000000000000000000000000000000000000000000000000000000", keypath)); + BOOST_CHECK(!ParseHDKeypath("m/1/1/111111111111111111111111111111111111111111111111111111111111111111111111111111111111", keypath)); + + BOOST_CHECK(ParseHDKeypath("m/0/00/0", keypath)); + BOOST_CHECK(!ParseHDKeypath("m/0'/00/'0", keypath)); + + BOOST_CHECK(ParseHDKeypath("m/1/", keypath)); + BOOST_CHECK(!ParseHDKeypath("m/1//", keypath)); + + BOOST_CHECK(ParseHDKeypath("m/0/4294967295", keypath)); // 4294967295 == 0xFFFFFFFF (uint32_t max) + BOOST_CHECK(!ParseHDKeypath("m/0/4294967296", keypath)); // 4294967296 == 0xFFFFFFFF (uint32_t max) + 1 + + BOOST_CHECK(ParseHDKeypath("m/4294967295", keypath)); // 4294967295 == 0xFFFFFFFF (uint32_t max) + BOOST_CHECK(!ParseHDKeypath("m/4294967296", keypath)); // 4294967296 == 0xFFFFFFFF (uint32_t max) + 1 +} + BOOST_AUTO_TEST_SUITE_END() From 988771050d901fe6bc6658e289448872c8231c18 Mon Sep 17 00:00:00 2001 From: Jonas Schnelli Date: Thu, 16 Feb 2017 14:22:18 +0100 Subject: [PATCH 06/31] Add facility to store wallet flags (64 bits) --- src/wallet/wallet.cpp | 22 ++++++++++++++++++++++ src/wallet/wallet.h | 10 ++++++++++ src/wallet/walletdb.cpp | 11 ++++++++++- src/wallet/walletdb.h | 1 + 4 files changed, 43 insertions(+), 1 deletion(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index a85334aa7d..7b83d09ea7 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1505,6 +1505,28 @@ bool CWallet::IsHDEnabled() const return !hdChain.seed_id.IsNull(); } +void CWallet::SetWalletFlag(uint64_t flags) +{ + LOCK(cs_wallet); + m_wallet_flags |= flags; + if (!WalletBatch(*database).WriteWalletFlags(m_wallet_flags)) + throw std::runtime_error(std::string(__func__) + ": writing wallet flags failed"); +} + +bool CWallet::IsWalletFlagSet(uint64_t flag) +{ + return (m_wallet_flags & flag); +} + +void CWallet::SetWalletFlags(uint64_t overwriteFlags, bool memonly) +{ + LOCK(cs_wallet); + m_wallet_flags = overwriteFlags; + if (!memonly && !WalletBatch(*database).WriteWalletFlags(m_wallet_flags)) { + throw std::runtime_error(std::string(__func__) + ": writing wallet flags failed"); + } +} + int64_t CWalletTx::GetTxTime() const { int64_t n = nTimeSmart; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index b5991a3b80..3c06769688 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -726,6 +726,7 @@ class CWallet final : public CCryptoKeyStore, public CValidationInterface std::set set_pre_split_keypool; int64_t m_max_keypool_index = 0; std::map m_pool_key_to_index; + std::atomic m_wallet_flags{0}; int64_t nTimeFirstKey = 0; @@ -1185,6 +1186,15 @@ class CWallet final : public CCryptoKeyStore, public CValidationInterface /** Whether a given output is spendable by this wallet */ bool OutputEligibleForSpending(const COutput& output, const CoinEligibilityFilter& eligibility_filter) const; + + /** set a single wallet flag */ + void SetWalletFlag(uint64_t flags); + + /** check if a certain wallet flag is set */ + bool IsWalletFlagSet(uint64_t flag); + + /** overwrite all flags by the given uint64_t */ + void SetWalletFlags(uint64_t overwriteFlags, bool memOnly); }; /** A key allocated from the key pool. */ diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 9c25ee7d76..7615e43275 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -510,7 +510,11 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, strErr = "Error reading wallet database: SetHDChain failed"; return false; } - } else if (strType != "bestblock" && strType != "bestblock_nomerkle"){ + } else if (strType == "flags") { + uint64_t flags; + ssValue >> flags; + pwallet->SetWalletFlags(flags, true); + } else if (strType != "bestblock" && strType != "bestblock_nomerkle") { wss.m_unknown_records++; } } catch (...) @@ -840,6 +844,11 @@ bool WalletBatch::WriteHDChain(const CHDChain& chain) return WriteIC(std::string("hdchain"), chain); } +bool WalletBatch::WriteWalletFlags(const uint64_t flags) +{ + return WriteIC(std::string("flags"), flags); +} + bool WalletBatch::TxnBegin() { return m_batch.TxnBegin(); diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index 3237376f63..674d1c2201 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -234,6 +234,7 @@ class WalletBatch //! write the hdchain model (external chain child index counter) bool WriteHDChain(const CHDChain& chain); + bool WriteWalletFlags(const uint64_t flags); //! Begin a new transaction bool TxnBegin(); //! Commit current transaction From de45beb478eef999743401b5fbd28931bfb547cc Mon Sep 17 00:00:00 2001 From: Jonas Schnelli Date: Fri, 5 May 2017 08:53:39 +0200 Subject: [PATCH 07/31] Add option to disable private keys during internal wallet creation --- src/wallet/rpcwallet.cpp | 40 ++++++++++++++++++++---------- src/wallet/wallet.cpp | 53 ++++++++++++++++++++++++++++++++++------ src/wallet/wallet.h | 17 ++++++++++--- src/wallet/walletdb.cpp | 13 +++++++--- 4 files changed, 95 insertions(+), 28 deletions(-) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 948bf1b191..83b8cd62f6 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -161,6 +161,10 @@ static UniValue getnewaddress(const JSONRPCRequest& request) + HelpExampleRpc("getnewaddress", "") ); + if (pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { + throw JSONRPCError(RPC_WALLET_ERROR, "Error: Private keys are disabled for this wallet"); + } + LOCK2(cs_main, pwallet->cs_wallet); // Parse the label first so we don't generate a key if there's an error @@ -268,6 +272,10 @@ static UniValue getrawchangeaddress(const JSONRPCRequest& request) + HelpExampleRpc("getrawchangeaddress", "") ); + if (pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { + throw JSONRPCError(RPC_WALLET_ERROR, "Error: Private keys are disabled for this wallet"); + } + LOCK2(cs_main, pwallet->cs_wallet); if (!pwallet->IsLocked()) { @@ -2506,6 +2514,10 @@ static UniValue keypoolrefill(const JSONRPCRequest& request) + HelpExampleRpc("keypoolrefill", "") ); + if (pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { + throw JSONRPCError(RPC_WALLET_ERROR, "Error: Private keys are disabled for this wallet"); + } + LOCK2(cs_main, pwallet->cs_wallet); // 0 is interpreted by TopUpKeyPool() as the default keypool size given by -keypool @@ -2990,19 +3002,20 @@ static UniValue getwalletinfo(const JSONRPCRequest& request) "Returns an object containing various wallet state info.\n" "\nResult:\n" "{\n" - " \"walletname\": xxxxx, (string) the wallet name\n" - " \"walletversion\": xxxxx, (numeric) the wallet version\n" - " \"balance\": xxxxxxx, (numeric) the total confirmed balance of the wallet in " + CURRENCY_UNIT + "\n" - " \"unconfirmed_balance\": xxx, (numeric) the total unconfirmed balance of the wallet in " + CURRENCY_UNIT + "\n" - " \"immature_balance\": xxxxxx, (numeric) the total immature balance of the wallet in " + CURRENCY_UNIT + "\n" - " \"txcount\": xxxxxxx, (numeric) the total number of transactions in the wallet\n" - " \"keypoololdest\": xxxxxx, (numeric) the timestamp (seconds since Unix epoch) of the oldest pre-generated key in the key pool\n" - " \"keypoolsize\": xxxx, (numeric) how many new keys are pre-generated (only counts external keys)\n" - " \"keypoolsize_hd_internal\": xxxx, (numeric) how many new keys are pre-generated for internal use (used for change outputs, only appears if the wallet is using this feature, otherwise external keys are used)\n" - " \"unlocked_until\": ttt, (numeric) the timestamp in seconds since epoch (midnight Jan 1 1970 GMT) that the wallet is unlocked for transfers, or 0 if the wallet is locked\n" - " \"paytxfee\": x.xxxx, (numeric) the transaction fee configuration, set in " + CURRENCY_UNIT + "/kB\n" - " \"hdseedid\": \"\" (string, optional) the Hash160 of the HD seed (only present when HD is enabled)\n" - " \"hdmasterkeyid\": \"\" (string, optional) alias for hdseedid retained for backwards-compatibility. Will be removed in V0.18.\n" + " \"walletname\": xxxxx, (string) the wallet name\n" + " \"walletversion\": xxxxx, (numeric) the wallet version\n" + " \"balance\": xxxxxxx, (numeric) the total confirmed balance of the wallet in " + CURRENCY_UNIT + "\n" + " \"unconfirmed_balance\": xxx, (numeric) the total unconfirmed balance of the wallet in " + CURRENCY_UNIT + "\n" + " \"immature_balance\": xxxxxx, (numeric) the total immature balance of the wallet in " + CURRENCY_UNIT + "\n" + " \"txcount\": xxxxxxx, (numeric) the total number of transactions in the wallet\n" + " \"keypoololdest\": xxxxxx, (numeric) the timestamp (seconds since Unix epoch) of the oldest pre-generated key in the key pool\n" + " \"keypoolsize\": xxxx, (numeric) how many new keys are pre-generated (only counts external keys)\n" + " \"keypoolsize_hd_internal\": xxxx, (numeric) how many new keys are pre-generated for internal use (used for change outputs, only appears if the wallet is using this feature, otherwise external keys are used)\n" + " \"unlocked_until\": ttt, (numeric) the timestamp in seconds since epoch (midnight Jan 1 1970 GMT) that the wallet is unlocked for transfers, or 0 if the wallet is locked\n" + " \"paytxfee\": x.xxxx, (numeric) the transaction fee configuration, set in " + CURRENCY_UNIT + "/kB\n" + " \"hdseedid\": \"\" (string, optional) the Hash160 of the HD seed (only present when HD is enabled)\n" + " \"hdmasterkeyid\": \"\" (string, optional) alias for hdseedid retained for backwards-compatibility. Will be removed in V0.18.\n" + " \"private_keys_enabled\": true|false (boolean) false if privatekeys are disabled for this wallet (enforced watch-only wallet)\n" "}\n" "\nExamples:\n" + HelpExampleCli("getwalletinfo", "") @@ -3038,6 +3051,7 @@ static UniValue getwalletinfo(const JSONRPCRequest& request) obj.pushKV("hdseedid", seed_id.GetHex()); obj.pushKV("hdmasterkeyid", seed_id.GetHex()); } + obj.pushKV("private_keys_enabled", !pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)); return obj; } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 7b83d09ea7..917623fa89 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -164,6 +164,7 @@ const CWalletTx* CWallet::GetWalletTx(const uint256& hash) const CPubKey CWallet::GenerateNewKey(WalletBatch &batch, bool internal) { + assert(!IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)); AssertLockHeld(cs_wallet); // mapKeyMetadata bool fCompressed = CanSupportFeature(FEATURE_COMPRPUBKEY); // default to compressed public keys if we want 0.6.0 wallets @@ -1444,6 +1445,7 @@ CAmount CWallet::GetChange(const CTransaction& tx) const CPubKey CWallet::GenerateNewSeed() { + assert(!IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)); CKey key; key.MakeNewKey(true); return DeriveNewSeed(key); @@ -1518,13 +1520,19 @@ bool CWallet::IsWalletFlagSet(uint64_t flag) return (m_wallet_flags & flag); } -void CWallet::SetWalletFlags(uint64_t overwriteFlags, bool memonly) +bool CWallet::SetWalletFlags(uint64_t overwriteFlags, bool memonly) { LOCK(cs_wallet); m_wallet_flags = overwriteFlags; + if (((overwriteFlags & g_known_wallet_flags) >> 32) ^ (overwriteFlags >> 32)) { + // contains unknown non-tolerable wallet flags + return false; + } if (!memonly && !WalletBatch(*database).WriteWalletFlags(m_wallet_flags)) { throw std::runtime_error(std::string(__func__) + ": writing wallet flags failed"); } + + return true; } int64_t CWalletTx::GetTxTime() const @@ -2742,6 +2750,10 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CTransac // post-backup change. // Reserve a new key pair from key pool + if (IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { + strFailReason = _("Can't generate a change-address key. Private keys are disabled for this wallet."); + return false; + } CPubKey vchPubKey; bool ret; ret = reservekey.GetReservedKey(vchPubKey, true); @@ -3142,7 +3154,7 @@ DBErrors CWallet::LoadWallet(bool& fFirstRunRet) { LOCK(cs_KeyStore); // This wallet is in its first run if all of these are empty - fFirstRunRet = mapKeys.empty() && mapCryptedKeys.empty() && mapWatchKeys.empty() && setWatchOnly.empty() && mapScripts.empty(); + fFirstRunRet = mapKeys.empty() && mapCryptedKeys.empty() && mapWatchKeys.empty() && setWatchOnly.empty() && mapScripts.empty() && !IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS); } if (nLoadWalletRet != DBErrors::LOAD_OK) @@ -3266,6 +3278,9 @@ const std::string& CWallet::GetLabelName(const CScript& scriptPubKey) const */ bool CWallet::NewKeyPool() { + if (IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { + return false; + } { LOCK(cs_wallet); WalletBatch batch(*database); @@ -3324,6 +3339,9 @@ void CWallet::LoadKeyPool(int64_t nIndex, const CKeyPool &keypool) bool CWallet::TopUpKeyPool(unsigned int kpSize) { + if (IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { + return false; + } { LOCK(cs_wallet); @@ -3448,6 +3466,10 @@ void CWallet::ReturnKey(int64_t nIndex, bool fInternal, const CPubKey& pubkey) bool CWallet::GetKeyFromPool(CPubKey& result, bool internal) { + if (IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { + return false; + } + CKeyPool keypool; { LOCK(cs_wallet); @@ -3987,7 +4009,7 @@ bool CWallet::Verify(std::string wallet_file, bool salvage_wallet, std::string& return WalletBatch::VerifyDatabaseFile(wallet_path, warning_string, error_string); } -std::shared_ptr CWallet::CreateWalletFromFile(const std::string& name, const fs::path& path) +std::shared_ptr CWallet::CreateWalletFromFile(const std::string& name, const fs::path& path, uint64_t wallet_creation_flags) { const std::string& walletFile = name; @@ -4112,18 +4134,33 @@ std::shared_ptr CWallet::CreateWalletFromFile(const std::string& name, } walletInstance->SetMinVersion(FEATURE_LATEST); - // generate a new seed - CPubKey seed = walletInstance->GenerateNewSeed(); - if (!walletInstance->SetHDSeed(seed)) - throw std::runtime_error(std::string(__func__) + ": Storing HD seed failed"); + if ((wallet_creation_flags & WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { + //selective allow to set flags + walletInstance->SetWalletFlag(WALLET_FLAG_DISABLE_PRIVATE_KEYS); + } else { + // generate a new seed + CPubKey seed = walletInstance->GenerateNewSeed(); + if (!walletInstance->SetHDSeed(seed)) { + throw std::runtime_error(std::string(__func__) + ": Storing HD seed failed"); + } + } // Top up the keypool - if (!walletInstance->TopUpKeyPool()) { + if (!walletInstance->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) && !walletInstance->TopUpKeyPool()) { InitError(_("Unable to generate initial keys") += "\n"); return nullptr; } walletInstance->ChainStateFlushed(chainActive.GetLocator()); + } else if (wallet_creation_flags & WALLET_FLAG_DISABLE_PRIVATE_KEYS) { + // Make it impossible to disable private keys after creation + InitError(strprintf(_("Error loading %s: Private keys can only be disabled during creation"), walletFile)); + return NULL; + } else if (walletInstance->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { + LOCK(walletInstance->cs_KeyStore); + if (!walletInstance->mapKeys.empty() || !walletInstance->mapCryptedKeys.empty()) { + InitWarning(strprintf(_("Warning: Private keys detected in wallet {%s} with disabled private keys"), walletFile)); + } } else if (gArgs.IsArgSet("-usehd")) { bool useHD = gArgs.GetBoolArg("-usehd", true); if (walletInstance->IsHDEnabled() && !useHD) { diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 3c06769688..5cfd507b79 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -100,6 +100,16 @@ constexpr OutputType DEFAULT_ADDRESS_TYPE{OutputType::P2SH_SEGWIT}; //! Default for -changetype constexpr OutputType DEFAULT_CHANGE_TYPE{OutputType::CHANGE_AUTO}; +enum WalletFlags : uint64_t { + // wallet flags in the upper section (> 1 << 31) will lead to not opening the wallet if flag is unknown + // unkown wallet flags in the lower section <= (1 << 31) will be tolerated + + // will enforce the rule that the wallet can't contain any private keys (only watch-only/pubkeys) + WALLET_FLAG_DISABLE_PRIVATE_KEYS = (1ULL << 32), +}; + +static constexpr uint64_t g_known_wallet_flags = WALLET_FLAG_DISABLE_PRIVATE_KEYS; + /** A key pool entry */ class CKeyPool { @@ -1133,7 +1143,7 @@ class CWallet final : public CCryptoKeyStore, public CValidationInterface static bool Verify(std::string wallet_file, bool salvage_wallet, std::string& error_string, std::string& warning_string); /* Initializes the wallet, returns a new CWallet instance or a null pointer in case of an error */ - static std::shared_ptr CreateWalletFromFile(const std::string& name, const fs::path& path); + static std::shared_ptr CreateWalletFromFile(const std::string& name, const fs::path& path, uint64_t wallet_creation_flags = 0); /** * Wallet post-init setup @@ -1193,8 +1203,9 @@ class CWallet final : public CCryptoKeyStore, public CValidationInterface /** check if a certain wallet flag is set */ bool IsWalletFlagSet(uint64_t flag); - /** overwrite all flags by the given uint64_t */ - void SetWalletFlags(uint64_t overwriteFlags, bool memOnly); + /** overwrite all flags by the given uint64_t + returns false if unknown, non-tolerable flags are present */ + bool SetWalletFlags(uint64_t overwriteFlags, bool memOnly); }; /** A key allocated from the key pool. */ diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 7615e43275..67fcaa725b 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -513,7 +513,10 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, } else if (strType == "flags") { uint64_t flags; ssValue >> flags; - pwallet->SetWalletFlags(flags, true); + if (!pwallet->SetWalletFlags(flags, true)) { + strErr = "Error reading wallet database: Unknown non-tolerable wallet flags found"; + return false; + } } else if (strType != "bestblock" && strType != "bestblock_nomerkle") { wss.m_unknown_records++; } @@ -574,10 +577,12 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) { // losing keys is considered a catastrophic error, anything else // we assume the user can live with: - if (IsKeyType(strType) || strType == "defaultkey") + if (IsKeyType(strType) || strType == "defaultkey") { result = DBErrors::CORRUPT; - else - { + } else if(strType == "flags") { + // reading the wallet flags can only fail if unknown flags are present + result = DBErrors::TOO_NEW; + } else { // Leave other errors alone, if we try to fix them we might make things worse. fNoncriticalErrors = true; // ... but do warn the user there is something wrong. if (strType == "tx") From d830dfb0b02a0aed061aeca558de71efffde5063 Mon Sep 17 00:00:00 2001 From: Jonas Schnelli Date: Wed, 1 Feb 2017 13:54:28 +0100 Subject: [PATCH 08/31] [Qt] Disable creating receive addresses when private keys are disabled --- src/interfaces/wallet.cpp | 1 + src/interfaces/wallet.h | 3 +++ src/qt/receivecoinsdialog.cpp | 3 +++ src/qt/walletmodel.cpp | 5 +++++ src/qt/walletmodel.h | 1 + 5 files changed, 13 insertions(+) diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp index b42c9b63df..28081c7c2c 100644 --- a/src/interfaces/wallet.cpp +++ b/src/interfaces/wallet.cpp @@ -426,6 +426,7 @@ class WalletImpl : public Wallet } unsigned int getConfirmTarget() override { return m_wallet.m_confirm_target; } bool hdEnabled() override { return m_wallet.IsHDEnabled(); } + bool IsWalletFlagSet(uint64_t flag) override { return m_wallet.IsWalletFlagSet(flag); } OutputType getDefaultAddressType() override { return m_wallet.m_default_address_type; } OutputType getDefaultChangeType() override { return m_wallet.m_default_change_type; } std::unique_ptr handleUnload(UnloadFn fn) override diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index 96e742eaca..ae54d42442 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -236,6 +236,9 @@ class Wallet // Return whether HD enabled. virtual bool hdEnabled() = 0; + // check if a certain wallet flag is set. + virtual bool IsWalletFlagSet(uint64_t flag) = 0; + // Get default address type. virtual OutputType getDefaultAddressType() = 0; diff --git a/src/qt/receivecoinsdialog.cpp b/src/qt/receivecoinsdialog.cpp index e458a52856..8c430e0af1 100644 --- a/src/qt/receivecoinsdialog.cpp +++ b/src/qt/receivecoinsdialog.cpp @@ -99,6 +99,9 @@ void ReceiveCoinsDialog::setModel(WalletModel *_model) } else { ui->useBech32->setCheckState(Qt::Unchecked); } + + // eventually disable the main receive button if private key operations are disabled + ui->receiveButton->setEnabled(!model->privateKeysDisabled()); } } diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index 389acf0a95..cd55b40b71 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -558,6 +558,11 @@ bool WalletModel::isWalletEnabled() return !gArgs.GetBoolArg("-disablewallet", DEFAULT_DISABLE_WALLET); } +bool WalletModel::privateKeysDisabled() const +{ + return m_wallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS); +} + QString WalletModel::getWalletName() const { return QString::fromStdString(m_wallet->getWalletName()); diff --git a/src/qt/walletmodel.h b/src/qt/walletmodel.h index 35ededb121..d8935c2fa8 100644 --- a/src/qt/walletmodel.h +++ b/src/qt/walletmodel.h @@ -197,6 +197,7 @@ class WalletModel : public QObject bool bumpFee(uint256 hash); static bool isWalletEnabled(); + bool privateKeysDisabled() const; interfaces::Node& node() const { return m_node; } interfaces::Wallet& wallet() const { return *m_wallet; } From 821403592d7d1ab0e543a8aa04c6a550cd69be05 Mon Sep 17 00:00:00 2001 From: Jonas Schnelli Date: Wed, 13 Jun 2018 20:35:41 +0200 Subject: [PATCH 09/31] Add disable privatekeys option to createwallet --- src/rpc/client.cpp | 1 + src/wallet/rpcwallet.cpp | 16 +++++++++++----- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 933846a162..1fac89b03b 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -177,6 +177,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "echojson", 9, "arg9" }, { "rescanblockchain", 0, "start_height"}, { "rescanblockchain", 1, "stop_height"}, + { "createwallet", 1, "disable_private_keys"}, }; class CRPCConvertTable diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 83b8cd62f6..8682bfa20f 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3142,12 +3142,13 @@ static UniValue loadwallet(const JSONRPCRequest& request) static UniValue createwallet(const JSONRPCRequest& request) { - if (request.fHelp || request.params.size() != 1) { + if (request.fHelp || request.params.size() < 1 || request.params.size() > 2) { throw std::runtime_error( - "createwallet \"wallet_name\"\n" + "createwallet \"wallet_name\" ( disable_private_keys )\n" "\nCreates and loads a new wallet.\n" "\nArguments:\n" - "1. \"wallet_name\" (string, required) The name for the new wallet. If this is a path, the wallet will be created at the path location.\n" + "1. \"wallet_name\" (string, required) The name for the new wallet. If this is a path, the wallet will be created at the path location.\n" + "2. disable_private_keys (boolean, optional, default: false) Disable the possibility of private keys (only watchonlys are possible in this mode).\n" "\nResult:\n" "{\n" " \"name\" : , (string) The wallet name if created successfully. If the wallet was created using a full path, the wallet_name will be the full path.\n" @@ -3162,6 +3163,11 @@ static UniValue createwallet(const JSONRPCRequest& request) std::string error; std::string warning; + bool disable_privatekeys = false; + if (!request.params[1].isNull()) { + disable_privatekeys = request.params[1].get_bool(); + } + fs::path wallet_path = fs::absolute(wallet_name, GetWalletDir()); if (fs::symlink_status(wallet_path).type() != fs::file_not_found) { throw JSONRPCError(RPC_WALLET_ERROR, "Wallet " + wallet_name + " already exists."); @@ -3172,7 +3178,7 @@ static UniValue createwallet(const JSONRPCRequest& request) throw JSONRPCError(RPC_WALLET_ERROR, "Wallet file verification failed: " + error); } - std::shared_ptr const wallet = CWallet::CreateWalletFromFile(wallet_name, fs::absolute(wallet_name, GetWalletDir())); + std::shared_ptr const wallet = CWallet::CreateWalletFromFile(wallet_name, fs::absolute(wallet_name, GetWalletDir()), (disable_privatekeys ? (uint64_t)WALLET_FLAG_DISABLE_PRIVATE_KEYS : 0)); if (!wallet) { throw JSONRPCError(RPC_WALLET_ERROR, "Wallet creation failed."); } @@ -4770,7 +4776,7 @@ static const CRPCCommand commands[] = { "hidden", "addwitnessaddress", &addwitnessaddress, {"address","p2sh"} }, { "wallet", "backupwallet", &backupwallet, {"destination"} }, { "wallet", "bumpfee", &bumpfee, {"txid", "options"} }, - { "wallet", "createwallet", &createwallet, {"wallet_name"} }, + { "wallet", "createwallet", &createwallet, {"wallet_name", "disable_private_keys"} }, { "wallet", "dumpprivkey", &dumpprivkey, {"address"} }, { "wallet", "dumpwallet", &dumpwallet, {"filename"} }, { "wallet", "encryptwallet", &encryptwallet, {"passphrase"} }, From 056b0ea5dca106d68478110e98c29fc01aa6f174 Mon Sep 17 00:00:00 2001 From: Jonas Schnelli Date: Wed, 13 Jun 2018 21:19:06 +0200 Subject: [PATCH 10/31] [QA] add createwallet disableprivatekey test --- src/wallet/test/wallet_tests.cpp | 9 ++++++ test/functional/test_runner.py | 2 ++ test/functional/wallet_disableprivatekeys.py | 32 ++++++++++++++++++++ 3 files changed, 43 insertions(+) create mode 100755 test/functional/wallet_disableprivatekeys.py diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index a946b565f1..c89b8f252f 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -365,4 +365,13 @@ BOOST_FIXTURE_TEST_CASE(ListCoins, ListCoinsTestingSetup) BOOST_CHECK_EQUAL(list.begin()->second.size(), 2U); } +BOOST_FIXTURE_TEST_CASE(wallet_disableprivkeys, TestChain100Setup) +{ + std::shared_ptr wallet = std::make_shared("dummy", WalletDatabase::CreateDummy()); + wallet->SetWalletFlag(WALLET_FLAG_DISABLE_PRIVATE_KEYS); + BOOST_CHECK(!wallet->TopUpKeyPool(1000)); + CPubKey pubkey; + BOOST_CHECK(!wallet->GetKeyFromPool(pubkey, false)); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index d8a09c22e5..4c5acf69a9 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -98,6 +98,8 @@ 'mempool_persist.py', 'wallet_multiwallet.py', 'wallet_multiwallet.py --usecli', + 'wallet_disableprivatekeys.py', + 'wallet_disableprivatekeys.py --usecli', 'interface_http.py', 'rpc_psbt.py', 'rpc_users.py', diff --git a/test/functional/wallet_disableprivatekeys.py b/test/functional/wallet_disableprivatekeys.py new file mode 100755 index 0000000000..0ba2cfe9be --- /dev/null +++ b/test/functional/wallet_disableprivatekeys.py @@ -0,0 +1,32 @@ +#!/usr/bin/env python3 +# Copyright (c) 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 disable-privatekeys mode. +""" + +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import ( + assert_raises_rpc_error, +) + + +class DisablePrivateKeysTest(BitcoinTestFramework): + def set_test_params(self): + self.setup_clean_chain = False + self.num_nodes = 1 + self.supports_cli = True + + def run_test(self): + node = self.nodes[0] + self.log.info("Test disableprivatekeys creation.") + self.nodes[0].createwallet('w1', True) + self.nodes[0].createwallet('w2') + w1 = node.get_wallet_rpc('w1') + w2 = node.get_wallet_rpc('w2') + assert_raises_rpc_error(-4,"Error: Private keys are disabled for this wallet", w1.getnewaddress) + assert_raises_rpc_error(-4,"Error: Private keys are disabled for this wallet", w1.getrawchangeaddress) + w1.importpubkey(w2.getaddressinfo(w2.getnewaddress())['pubkey']) + +if __name__ == '__main__': + DisablePrivateKeysTest().main() From 95d254620f571119b2858652c5916cbeba44cd76 Mon Sep 17 00:00:00 2001 From: Jonas Schnelli Date: Thu, 14 Jun 2018 21:38:01 +0200 Subject: [PATCH 11/31] QA: Fix bug in -usecli logic that converts booleans to non-lowercase strings --- test/functional/test_framework/test_node.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index 287dc0e53e..50942aec40 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -351,8 +351,7 @@ def batch(self, requests): def send_cli(self, command=None, *args, **kwargs): """Run bitcoin-cli command. Deserializes returned string as python object.""" - - pos_args = [str(arg) for arg in args] + pos_args = [str(arg).lower() if type(arg) is bool else str(arg) for arg in args] named_args = [str(key) + "=" + str(value) for (key, value) in kwargs.items()] assert not (pos_args and named_args), "Cannot use positional arguments and named arguments in the same bitcoin-cli call" p_args = [self.binary, "-datadir=" + self.datadir] + self.options From 159eb20824fdfef7452167b574a777b8c000efa6 Mon Sep 17 00:00:00 2001 From: Mason Simon Date: Thu, 19 Jul 2018 12:27:42 -0700 Subject: [PATCH 12/31] docs: Specify preferred Python string formatting technique --- test/functional/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/test/functional/README.md b/test/functional/README.md index e6365222ff..6929ab5991 100644 --- a/test/functional/README.md +++ b/test/functional/README.md @@ -28,6 +28,7 @@ don't have test cases for. - When subclassing the BitcoinTestFramwork, place overrides for the `set_test_params()`, `add_options()` and `setup_xxxx()` methods at the top of the subclass, then locally-defined helper methods, then the `run_test()` method. +- Use `'{}'.format(x)` for string formatting, not `'%s' % x`. #### Naming guidelines From baed22be6de5bd19bd80e32b7c6fdc8baa33d492 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Wed, 8 Nov 2017 13:43:28 -0500 Subject: [PATCH 13/31] Remove dead service bits code --- src/net_processing.cpp | 11 --------- test/functional/p2p_leak.py | 27 +--------------------- test/functional/test_framework/messages.py | 2 -- 3 files changed, 1 insertion(+), 39 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index a477c26703..a3851e5f39 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1661,17 +1661,6 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr return false; } - if (nServices & ((1 << 7) | (1 << 5))) { - if (GetTime() < 1533096000) { - // Immediately disconnect peers that use service bits 6 or 8 until August 1st, 2018 - // These bits have been used as a flag to indicate that a node is running incompatible - // consensus rules instead of changing the network magic, so we're stuck disconnecting - // based on these service bits, at least for a while. - pfrom->fDisconnect = true; - return false; - } - } - if (nVersion < MIN_PEER_PROTO_VERSION) { // disconnect from peers older than this proto version diff --git a/test/functional/p2p_leak.py b/test/functional/p2p_leak.py index ecb9a56fe1..186c35e0e1 100755 --- a/test/functional/p2p_leak.py +++ b/test/functional/p2p_leak.py @@ -8,11 +8,7 @@ received a VERACK. This test connects to a node and sends it a few messages, trying to entice it -into sending us something it shouldn't. - -Also test that nodes that send unsupported service bits to bitcoind are disconnected -and don't receive a VERACK. Unsupported service bits are currently 1 << 5 and -1 << 7 (until August 1st 2018).""" +into sending us something it shouldn't.""" from test_framework.mininode import * from test_framework.test_framework import BitcoinTestFramework @@ -95,19 +91,13 @@ def set_test_params(self): self.extra_args = [['-banscore='+str(banscore)]] def run_test(self): - self.nodes[0].setmocktime(1501545600) # August 1st 2017 - no_version_bannode = self.nodes[0].add_p2p_connection(CNodeNoVersionBan(), send_version=False) no_version_idlenode = self.nodes[0].add_p2p_connection(CNodeNoVersionIdle(), send_version=False) no_verack_idlenode = self.nodes[0].add_p2p_connection(CNodeNoVerackIdle()) - unsupported_service_bit5_node = self.nodes[0].add_p2p_connection(CLazyNode(), services=NODE_NETWORK|NODE_UNSUPPORTED_SERVICE_BIT_5) - unsupported_service_bit7_node = self.nodes[0].add_p2p_connection(CLazyNode(), services=NODE_NETWORK|NODE_UNSUPPORTED_SERVICE_BIT_7) wait_until(lambda: no_version_bannode.ever_connected, timeout=10, lock=mininode_lock) wait_until(lambda: no_version_idlenode.ever_connected, timeout=10, lock=mininode_lock) wait_until(lambda: no_verack_idlenode.version_received, timeout=10, lock=mininode_lock) - wait_until(lambda: unsupported_service_bit5_node.ever_connected, timeout=10, lock=mininode_lock) - wait_until(lambda: unsupported_service_bit7_node.ever_connected, timeout=10, lock=mininode_lock) # Mine a block and make sure that it's not sent to the connected nodes self.nodes[0].generate(1) @@ -118,10 +108,6 @@ def run_test(self): #This node should have been banned assert not no_version_bannode.is_connected - # These nodes should have been disconnected - assert not unsupported_service_bit5_node.is_connected - assert not unsupported_service_bit7_node.is_connected - self.nodes[0].disconnect_p2ps() # Wait until all connections are closed @@ -131,17 +117,6 @@ def run_test(self): assert(no_version_bannode.unexpected_msg == False) assert(no_version_idlenode.unexpected_msg == False) assert(no_verack_idlenode.unexpected_msg == False) - assert not unsupported_service_bit5_node.unexpected_msg - assert not unsupported_service_bit7_node.unexpected_msg - - self.log.info("Service bits 5 and 7 are allowed after August 1st 2018") - self.nodes[0].setmocktime(1533168000) # August 2nd 2018 - - allowed_service_bit5_node = self.nodes[0].add_p2p_connection(P2PInterface(), services=NODE_NETWORK|NODE_UNSUPPORTED_SERVICE_BIT_5) - allowed_service_bit7_node = self.nodes[0].add_p2p_connection(P2PInterface(), services=NODE_NETWORK|NODE_UNSUPPORTED_SERVICE_BIT_7) - - wait_until(lambda: allowed_service_bit5_node.message_count["verack"], lock=mininode_lock) - wait_until(lambda: allowed_service_bit7_node.message_count["verack"], lock=mininode_lock) if __name__ == '__main__': diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index df8d424d01..af57e61f2f 100755 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -42,8 +42,6 @@ # NODE_GETUTXO = (1 << 1) NODE_BLOOM = (1 << 2) NODE_WITNESS = (1 << 3) -NODE_UNSUPPORTED_SERVICE_BIT_5 = (1 << 5) -NODE_UNSUPPORTED_SERVICE_BIT_7 = (1 << 7) NODE_NETWORK_LIMITED = (1 << 10) MSG_TX = 1 From c833aca755f98c4d6b8536c1ab9cb2bf25669983 Mon Sep 17 00:00:00 2001 From: Ben Woosley Date: Tue, 17 Jul 2018 16:50:09 -0400 Subject: [PATCH 14/31] lint: Add linter for circular dependencies Protects against added circular depencies, makes it explicit in the code when circular dependencies have been removed. Modeled after EXPECTED_BOOST_INCLUDES in lint-includes.sh --- test/lint/lint-circular-dependencies.sh | 84 +++++++++++++++++++++++++ 1 file changed, 84 insertions(+) create mode 100755 test/lint/lint-circular-dependencies.sh diff --git a/test/lint/lint-circular-dependencies.sh b/test/lint/lint-circular-dependencies.sh new file mode 100755 index 0000000000..b8d105b49b --- /dev/null +++ b/test/lint/lint-circular-dependencies.sh @@ -0,0 +1,84 @@ +#!/usr/bin/env bash +# +# Copyright (c) 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. +# +# Check for circular dependencies + +export LC_ALL=C + +EXPECTED_CIRCULAR_DEPENDENCIES=( + "chainparamsbase -> util -> chainparamsbase" + "checkpoints -> validation -> checkpoints" + "index/txindex -> validation -> index/txindex" + "policy/fees -> txmempool -> policy/fees" + "policy/policy -> validation -> policy/policy" + "qt/addresstablemodel -> qt/walletmodel -> qt/addresstablemodel" + "qt/bantablemodel -> qt/clientmodel -> qt/bantablemodel" + "qt/bitcoingui -> qt/utilitydialog -> qt/bitcoingui" + "qt/bitcoingui -> qt/walletframe -> qt/bitcoingui" + "qt/bitcoingui -> qt/walletview -> qt/bitcoingui" + "qt/clientmodel -> qt/peertablemodel -> qt/clientmodel" + "qt/paymentserver -> qt/walletmodel -> qt/paymentserver" + "qt/recentrequeststablemodel -> qt/walletmodel -> qt/recentrequeststablemodel" + "qt/sendcoinsdialog -> qt/walletmodel -> qt/sendcoinsdialog" + "qt/transactiontablemodel -> qt/walletmodel -> qt/transactiontablemodel" + "qt/walletmodel -> qt/walletmodeltransaction -> qt/walletmodel" + "rpc/rawtransaction -> wallet/rpcwallet -> rpc/rawtransaction" + "txmempool -> validation -> txmempool" + "validation -> validationinterface -> validation" + "wallet/coincontrol -> wallet/wallet -> wallet/coincontrol" + "wallet/fees -> wallet/wallet -> wallet/fees" + "wallet/rpcwallet -> wallet/wallet -> wallet/rpcwallet" + "wallet/wallet -> wallet/walletdb -> wallet/wallet" + "policy/fees -> policy/policy -> validation -> policy/fees" + "policy/rbf -> txmempool -> validation -> policy/rbf" + "qt/addressbookpage -> qt/bitcoingui -> qt/walletview -> qt/addressbookpage" + "qt/guiutil -> qt/walletmodel -> qt/optionsmodel -> qt/guiutil" + "txmempool -> validation -> validationinterface -> txmempool" + "qt/addressbookpage -> qt/bitcoingui -> qt/walletview -> qt/receivecoinsdialog -> qt/addressbookpage" + "qt/addressbookpage -> qt/bitcoingui -> qt/walletview -> qt/signverifymessagedialog -> qt/addressbookpage" + "qt/guiutil -> qt/walletmodel -> qt/optionsmodel -> qt/intro -> qt/guiutil" + "qt/addressbookpage -> qt/bitcoingui -> qt/walletview -> qt/sendcoinsdialog -> qt/sendcoinsentry -> qt/addressbookpage" +) + +EXIT_CODE=0 + +CIRCULAR_DEPENDENCIES=() + +IFS=$'\n' +for CIRC in $(cd src && ../contrib/devtools/circular-dependencies.py {*,*/*,*/*/*}.{h,cpp} | sed -e 's/^Circular dependency: //'); do + CIRCULAR_DEPENDENCIES+=($CIRC) + IS_EXPECTED_CIRC=0 + for EXPECTED_CIRC in "${EXPECTED_CIRCULAR_DEPENDENCIES[@]}"; do + if [[ "${CIRC}" == "${EXPECTED_CIRC}" ]]; then + IS_EXPECTED_CIRC=1 + break + fi + done + if [[ ${IS_EXPECTED_CIRC} == 0 ]]; then + echo "A new circular dependency in the form of \"${CIRC}\" appears to have been introduced." + echo + EXIT_CODE=1 + fi +done + +for EXPECTED_CIRC in "${EXPECTED_CIRCULAR_DEPENDENCIES[@]}"; do + IS_PRESENT_EXPECTED_CIRC=0 + for CIRC in "${CIRCULAR_DEPENDENCIES[@]}"; do + if [[ "${CIRC}" == "${EXPECTED_CIRC}" ]]; then + IS_PRESENT_EXPECTED_CIRC=1 + break + fi + done + if [[ ${IS_PRESENT_EXPECTED_CIRC} == 0 ]]; then + echo "Good job! The circular dependency \"${EXPECTED_CIRC}\" is no longer present." + echo "Please remove it from EXPECTED_CIRCULAR_DEPENDENCIES in $0" + echo "to make sure this circular dependency is not accidentally reintroduced." + echo + EXIT_CODE=1 + fi +done + +exit ${EXIT_CODE} From c472ae0346b3832472350953cc5f471410404f49 Mon Sep 17 00:00:00 2001 From: Chun Kuan Lee Date: Sat, 16 Jun 2018 02:31:26 +0000 Subject: [PATCH 15/31] Replace boost program_options --- src/util.cpp | 47 ++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 38 insertions(+), 9 deletions(-) diff --git a/src/util.cpp b/src/util.cpp index 4313ab2ecc..e238c8605d 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -72,7 +72,6 @@ #endif #include -#include #include #include #include @@ -811,17 +810,47 @@ fs::path GetConfigFile(const std::string& confPath) return AbsPathForConfigVal(fs::path(confPath), false); } +static std::string TrimString(const std::string& str, const std::string& pattern) +{ + std::string::size_type front = str.find_first_not_of(pattern); + if (front == std::string::npos) { + return std::string(); + } + std::string::size_type end = str.find_last_not_of(pattern); + return str.substr(front, end - front + 1); +} + +static std::vector> GetConfigOptions(std::istream& stream) +{ + std::vector> options; + std::string str, prefix; + std::string::size_type pos; + while (std::getline(stream, str)) { + if ((pos = str.find('#')) != std::string::npos) { + str = str.substr(0, pos); + } + const static std::string pattern = " \t\r\n"; + str = TrimString(str, pattern); + if (!str.empty()) { + if (*str.begin() == '[' && *str.rbegin() == ']') { + prefix = str.substr(1, str.size() - 2) + '.'; + } else if ((pos = str.find('=')) != std::string::npos) { + std::string name = prefix + TrimString(str.substr(0, pos), pattern); + std::string value = TrimString(str.substr(pos + 1), pattern); + options.emplace_back(name, value); + } + } + } + return options; +} + bool ArgsManager::ReadConfigStream(std::istream& stream, std::string& error, bool ignore_invalid_keys) { LOCK(cs_args); - std::set setOptions; - setOptions.insert("*"); - - for (boost::program_options::detail::config_file_iterator it(stream, setOptions), end; it != end; ++it) - { - std::string strKey = std::string("-") + it->string_key; - std::string strValue = it->value[0]; + for (const std::pair& option : GetConfigOptions(stream)) { + std::string strKey = std::string("-") + option.first; + std::string strValue = option.second; if (InterpretNegatedOption(strKey, strValue)) { m_config_args[strKey].clear(); @@ -831,7 +860,7 @@ bool ArgsManager::ReadConfigStream(std::istream& stream, std::string& error, boo // Check that the arg is known if (!IsArgKnown(strKey, error) && !ignore_invalid_keys) { - error = strprintf("Invalid configuration value %s", it->string_key.c_str()); + error = strprintf("Invalid configuration value %s", option.first.c_str()); return false; } } From db06816932dd0e19746313bf8833e35a023c27f1 Mon Sep 17 00:00:00 2001 From: Chun Kuan Lee Date: Sat, 16 Jun 2018 04:04:21 +0000 Subject: [PATCH 16/31] Remove program options from build system --- build-aux/m4/ax_boost_program_options.m4 | 108 ----------------------- configure.ac | 3 +- depends/packages/boost.mk | 2 +- doc/build-unix.md | 2 +- test/lint/lint-includes.sh | 1 - 5 files changed, 3 insertions(+), 113 deletions(-) delete mode 100644 build-aux/m4/ax_boost_program_options.m4 diff --git a/build-aux/m4/ax_boost_program_options.m4 b/build-aux/m4/ax_boost_program_options.m4 deleted file mode 100644 index 2bdb593716..0000000000 --- a/build-aux/m4/ax_boost_program_options.m4 +++ /dev/null @@ -1,108 +0,0 @@ -# ============================================================================ -# http://www.gnu.org/software/autoconf-archive/ax_boost_program_options.html -# ============================================================================ -# -# SYNOPSIS -# -# AX_BOOST_PROGRAM_OPTIONS -# -# DESCRIPTION -# -# Test for program options library from the Boost C++ libraries. The macro -# requires a preceding call to AX_BOOST_BASE. Further documentation is -# available at . -# -# This macro calls: -# -# AC_SUBST(BOOST_PROGRAM_OPTIONS_LIB) -# -# And sets: -# -# HAVE_BOOST_PROGRAM_OPTIONS -# -# LICENSE -# -# Copyright (c) 2009 Thomas Porschberg -# -# Copying and distribution of this file, with or without modification, are -# permitted in any medium without royalty provided the copyright notice -# and this notice are preserved. This file is offered as-is, without any -# warranty. - -#serial 24 - -AC_DEFUN([AX_BOOST_PROGRAM_OPTIONS], -[ - AC_ARG_WITH([boost-program-options], - AS_HELP_STRING([--with-boost-program-options@<:@=special-lib@:>@], - [use the program options library from boost - it is possible to specify a certain library for the linker - e.g. --with-boost-program-options=boost_program_options-gcc-mt-1_33_1 ]), - [ - if test "$withval" = "no"; then - want_boost="no" - elif test "$withval" = "yes"; then - want_boost="yes" - ax_boost_user_program_options_lib="" - else - want_boost="yes" - ax_boost_user_program_options_lib="$withval" - fi - ], - [want_boost="yes"] - ) - - if test "x$want_boost" = "xyes"; then - AC_REQUIRE([AC_PROG_CC]) - export want_boost - CPPFLAGS_SAVED="$CPPFLAGS" - CPPFLAGS="$CPPFLAGS $BOOST_CPPFLAGS" - export CPPFLAGS - LDFLAGS_SAVED="$LDFLAGS" - LDFLAGS="$LDFLAGS $BOOST_LDFLAGS" - export LDFLAGS - AC_CACHE_CHECK([whether the Boost::Program_Options library is available], - ax_cv_boost_program_options, - [AC_LANG_PUSH(C++) - AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[@%:@include - ]], - [[boost::program_options::error err("Error message"); - return 0;]])], - ax_cv_boost_program_options=yes, ax_cv_boost_program_options=no) - AC_LANG_POP([C++]) - ]) - if test "$ax_cv_boost_program_options" = yes; then - AC_DEFINE(HAVE_BOOST_PROGRAM_OPTIONS,,[define if the Boost::PROGRAM_OPTIONS library is available]) - BOOSTLIBDIR=`echo $BOOST_LDFLAGS | sed -e 's/@<:@^\/@:>@*//'` - if test "x$ax_boost_user_program_options_lib" = "x"; then - for libextension in `ls $BOOSTLIBDIR/libboost_program_options*.so* 2>/dev/null | sed 's,.*/,,' | sed -e 's;^lib\(boost_program_options.*\)\.so.*$;\1;'` `ls $BOOSTLIBDIR/libboost_program_options*.dylib* 2>/dev/null | sed 's,.*/,,' | sed -e 's;^lib\(boost_program_options.*\)\.dylib.*$;\1;'` `ls $BOOSTLIBDIR/libboost_program_options*.a* 2>/dev/null | sed 's,.*/,,' | sed -e 's;^lib\(boost_program_options.*\)\.a.*$;\1;'` ; do - ax_lib=${libextension} - AC_CHECK_LIB($ax_lib, exit, - [BOOST_PROGRAM_OPTIONS_LIB="-l$ax_lib"; AC_SUBST(BOOST_PROGRAM_OPTIONS_LIB) link_program_options="yes"; break], - [link_program_options="no"]) - done - if test "x$link_program_options" != "xyes"; then - for libextension in `ls $BOOSTLIBDIR/boost_program_options*.dll* 2>/dev/null | sed 's,.*/,,' | sed -e 's;^\(boost_program_options.*\)\.dll.*$;\1;'` `ls $BOOSTLIBDIR/boost_program_options*.a* 2>/dev/null | sed 's,.*/,,' | sed -e 's;^\(boost_program_options.*\)\.a.*$;\1;'` ; do - ax_lib=${libextension} - AC_CHECK_LIB($ax_lib, exit, - [BOOST_PROGRAM_OPTIONS_LIB="-l$ax_lib"; AC_SUBST(BOOST_PROGRAM_OPTIONS_LIB) link_program_options="yes"; break], - [link_program_options="no"]) - done - fi - else - for ax_lib in $ax_boost_user_program_options_lib boost_program_options-$ax_boost_user_program_options_lib; do - AC_CHECK_LIB($ax_lib, main, - [BOOST_PROGRAM_OPTIONS_LIB="-l$ax_lib"; AC_SUBST(BOOST_PROGRAM_OPTIONS_LIB) link_program_options="yes"; break], - [link_program_options="no"]) - done - fi - if test "x$ax_lib" = "x"; then - AC_MSG_ERROR(Could not find a version of the boost_program_options library!) - fi - if test "x$link_program_options" != "xyes"; then - AC_MSG_ERROR([Could not link against [$ax_lib] !]) - fi - fi - CPPFLAGS="$CPPFLAGS_SAVED" - LDFLAGS="$LDFLAGS_SAVED" - fi -]) diff --git a/configure.ac b/configure.ac index 1198244053..58aab1b85e 100644 --- a/configure.ac +++ b/configure.ac @@ -902,7 +902,6 @@ if test x$want_boost = xno; then fi AX_BOOST_SYSTEM AX_BOOST_FILESYSTEM -AX_BOOST_PROGRAM_OPTIONS AX_BOOST_THREAD AX_BOOST_CHRONO @@ -971,7 +970,7 @@ fi if test x$use_boost = xyes; then -BOOST_LIBS="$BOOST_LDFLAGS $BOOST_SYSTEM_LIB $BOOST_FILESYSTEM_LIB $BOOST_PROGRAM_OPTIONS_LIB $BOOST_THREAD_LIB $BOOST_CHRONO_LIB" +BOOST_LIBS="$BOOST_LDFLAGS $BOOST_SYSTEM_LIB $BOOST_FILESYSTEM_LIB $BOOST_THREAD_LIB $BOOST_CHRONO_LIB" dnl If boost (prior to 1.57) was built without c++11, it emulated scoped enums diff --git a/depends/packages/boost.mk b/depends/packages/boost.mk index bf773ccd14..61806c7509 100644 --- a/depends/packages/boost.mk +++ b/depends/packages/boost.mk @@ -19,7 +19,7 @@ $(package)_toolset_$(host_os)=gcc $(package)_archiver_$(host_os)=$($(package)_ar) $(package)_toolset_darwin=darwin $(package)_archiver_darwin=$($(package)_libtool) -$(package)_config_libraries=chrono,filesystem,program_options,system,thread,test +$(package)_config_libraries=chrono,filesystem,system,thread,test $(package)_cxxflags=-std=c++11 -fvisibility=hidden $(package)_cxxflags_linux=-fPIC endef diff --git a/doc/build-unix.md b/doc/build-unix.md index e884c0ab67..d9208fed54 100644 --- a/doc/build-unix.md +++ b/doc/build-unix.md @@ -70,7 +70,7 @@ tuned to conserve memory with additional CXXFLAGS: Build requirements: - sudo apt-get install build-essential libtool autotools-dev automake pkg-config libssl-dev libevent-dev bsdmainutils python3 libboost-system-dev libboost-filesystem-dev libboost-chrono-dev libboost-program-options-dev libboost-test-dev libboost-thread-dev + sudo apt-get install build-essential libtool autotools-dev automake pkg-config libssl-dev libevent-dev bsdmainutils python3 libboost-system-dev libboost-filesystem-dev libboost-chrono-dev libboost-test-dev libboost-thread-dev BerkeleyDB is required for the wallet. diff --git a/test/lint/lint-includes.sh b/test/lint/lint-includes.sh index 40d28ed3e0..52b327f955 100755 --- a/test/lint/lint-includes.sh +++ b/test/lint/lint-includes.sh @@ -67,7 +67,6 @@ EXPECTED_BOOST_INCLUDES=( boost/optional.hpp boost/preprocessor/cat.hpp boost/preprocessor/stringize.hpp - boost/program_options/detail/config_file.hpp boost/scoped_array.hpp boost/signals2/connection.hpp boost/signals2/last_value.hpp From 2670be25f9b72d802b133f39e98633dd3ca072b6 Mon Sep 17 00:00:00 2001 From: Ben Woosley Date: Fri, 20 Jul 2018 14:14:08 -0400 Subject: [PATCH 17/31] Fix bitcoin-cli --version By declaring the relevant option. Note contrib/devtools/gen-manpages.sh relies on this version information. --- src/bitcoin-cli.cpp | 1 + test/functional/interface_bitcoin_cli.py | 3 +++ 2 files changed, 4 insertions(+) diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index 2ca599dd17..6297ba542f 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -35,6 +35,7 @@ static void SetupCliArgs() const auto testnetBaseParams = CreateBaseChainParams(CBaseChainParams::TESTNET); gArgs.AddArg("-?", "This help message", false, OptionsCategory::OPTIONS); + gArgs.AddArg("-version", "Print version and exit", false, OptionsCategory::OPTIONS); gArgs.AddArg("-conf=", strprintf("Specify configuration file. Relative paths will be prefixed by datadir location. (default: %s)", BITCOIN_CONF_FILENAME), false, OptionsCategory::OPTIONS); gArgs.AddArg("-datadir=", "Specify data directory", false, OptionsCategory::OPTIONS); gArgs.AddArg("-getinfo", "Get general information from the remote server. Note that unlike server-side RPC calls, the results of -getinfo is the result of multiple non-atomic requests. Some entries in the result may represent results from different states (e.g. wallet balance may be as of a different block from the chain state reported)", false, OptionsCategory::OPTIONS); diff --git a/test/functional/interface_bitcoin_cli.py b/test/functional/interface_bitcoin_cli.py index e29fdc84e7..b097c64b13 100755 --- a/test/functional/interface_bitcoin_cli.py +++ b/test/functional/interface_bitcoin_cli.py @@ -15,6 +15,9 @@ def set_test_params(self): def run_test(self): """Main test logic""" + cli_response = self.nodes[0].cli("-version").send_cli() + assert("Bitcoin Core RPC client version" in cli_response) + self.log.info("Compare responses from gewalletinfo RPC and `bitcoin-cli getwalletinfo`") cli_response = self.nodes[0].cli.getwalletinfo() rpc_response = self.nodes[0].getwalletinfo() From 7dd7dfa2ffc6b3773d681fc9bb259f5b15a59aeb Mon Sep 17 00:00:00 2001 From: 251 <13120787+251Labs@users.noreply.github.com> Date: Fri, 20 Jul 2018 23:48:26 +0200 Subject: [PATCH 18/31] Removes the boost/algorithm/string/join dependency This commit removes the `boost/algorithm/string/join` dependency from the project by replacing `boost::algorithm::join` with a simple helper function. --- src/validation.cpp | 16 +++++++++++----- test/lint/lint-includes.sh | 1 - 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index 84532c9e92..7c04d7e909 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -47,7 +47,6 @@ #include #include -#include #include #if defined(NDEBUG) @@ -2227,6 +2226,13 @@ static void DoWarning(const std::string& strWarning) } } +/** Private helper function that concatenates warning messages. */ +static void AppendWarning(std::string& res, const std::string& warn) +{ + if (!res.empty()) res += ", "; + res += warn; +} + /** Check warning conditions and do some notifications on new chain tip set. */ void static UpdateTip(const CBlockIndex *pindexNew, const CChainParams& chainParams) { // New best block @@ -2238,7 +2244,7 @@ void static UpdateTip(const CBlockIndex *pindexNew, const CChainParams& chainPar g_best_block_cv.notify_all(); } - std::vector warningMessages; + std::string warningMessages; if (!IsInitialBlockDownload()) { int nUpgraded = 0; @@ -2251,7 +2257,7 @@ void static UpdateTip(const CBlockIndex *pindexNew, const CChainParams& chainPar if (state == ThresholdState::ACTIVE) { DoWarning(strWarning); } else { - warningMessages.push_back(strWarning); + AppendWarning(warningMessages, strWarning); } } } @@ -2264,7 +2270,7 @@ void static UpdateTip(const CBlockIndex *pindexNew, const CChainParams& chainPar pindex = pindex->pprev; } if (nUpgraded > 0) - warningMessages.push_back(strprintf(_("%d of last 100 blocks have unexpected version"), nUpgraded)); + AppendWarning(warningMessages, strprintf(_("%d of last 100 blocks have unexpected version"), nUpgraded)); if (nUpgraded > 100/2) { std::string strWarning = _("Warning: Unknown block versions being mined! It's possible unknown rules are in effect"); @@ -2278,7 +2284,7 @@ void static UpdateTip(const CBlockIndex *pindexNew, const CChainParams& chainPar FormatISO8601DateTime(pindexNew->GetBlockTime()), GuessVerificationProgress(chainParams.TxData(), pindexNew), pcoinsTip->DynamicMemoryUsage() * (1.0 / (1<<20)), pcoinsTip->GetCacheSize()); if (!warningMessages.empty()) - LogPrintf(" warning='%s'", boost::algorithm::join(warningMessages, ", ")); /* Continued */ + LogPrintf(" warning='%s'", warningMessages); /* Continued */ LogPrintf("\n"); if (pindexBestHeader->pprev) diff --git a/test/lint/lint-includes.sh b/test/lint/lint-includes.sh index 52b327f955..2faaef8954 100755 --- a/test/lint/lint-includes.sh +++ b/test/lint/lint-includes.sh @@ -49,7 +49,6 @@ EXPECTED_BOOST_INCLUDES=( boost/algorithm/string.hpp boost/algorithm/string/case_conv.hpp boost/algorithm/string/classification.hpp - boost/algorithm/string/join.hpp boost/algorithm/string/predicate.hpp boost/algorithm/string/replace.hpp boost/algorithm/string/split.hpp From 62e516e83281f778c69e1730f1132cb05fbc72d0 Mon Sep 17 00:00:00 2001 From: practicalswift Date: Tue, 17 Jul 2018 10:22:10 +0200 Subject: [PATCH 19/31] wallet: Avoid potential null pointer dereference in CWalletTx::GetAvailableCredit(...) --- src/wallet/wallet.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 917623fa89..512b7b722c 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1977,6 +1977,7 @@ CAmount CWalletTx::GetAvailableCredit(bool fUseCache, const isminefilter& filter if (cache) { *cache = nCredit; + assert(cache_used); *cache_used = true; } return nCredit; From 19203c0e3e25adadf871072ff44f3db6cc7a200a Mon Sep 17 00:00:00 2001 From: Nikolay Mitev Date: Fri, 20 Jul 2018 09:48:49 +0300 Subject: [PATCH 20/31] trivial: Replace CPubKey::operator[] with CPubKey::vch where possible --- src/pubkey.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/pubkey.cpp b/src/pubkey.cpp index 6e601a6f13..1eb126434b 100644 --- a/src/pubkey.cpp +++ b/src/pubkey.cpp @@ -171,7 +171,7 @@ bool CPubKey::Verify(const uint256 &hash, const std::vector& vchS return false; secp256k1_pubkey pubkey; secp256k1_ecdsa_signature sig; - if (!secp256k1_ec_pubkey_parse(secp256k1_context_verify, &pubkey, &(*this)[0], size())) { + if (!secp256k1_ec_pubkey_parse(secp256k1_context_verify, &pubkey, vch, size())) { return false; } if (!ecdsa_signature_parse_der_lax(secp256k1_context_verify, &sig, vchSig.data(), vchSig.size())) { @@ -207,14 +207,14 @@ bool CPubKey::IsFullyValid() const { if (!IsValid()) return false; secp256k1_pubkey pubkey; - return secp256k1_ec_pubkey_parse(secp256k1_context_verify, &pubkey, &(*this)[0], size()); + return secp256k1_ec_pubkey_parse(secp256k1_context_verify, &pubkey, vch, size()); } bool CPubKey::Decompress() { if (!IsValid()) return false; secp256k1_pubkey pubkey; - if (!secp256k1_ec_pubkey_parse(secp256k1_context_verify, &pubkey, &(*this)[0], size())) { + if (!secp256k1_ec_pubkey_parse(secp256k1_context_verify, &pubkey, vch, size())) { return false; } unsigned char pub[PUBLIC_KEY_SIZE]; @@ -232,7 +232,7 @@ bool CPubKey::Derive(CPubKey& pubkeyChild, ChainCode &ccChild, unsigned int nChi BIP32Hash(cc, nChild, *begin(), begin()+1, out); memcpy(ccChild.begin(), out+32, 32); secp256k1_pubkey pubkey; - if (!secp256k1_ec_pubkey_parse(secp256k1_context_verify, &pubkey, &(*this)[0], size())) { + if (!secp256k1_ec_pubkey_parse(secp256k1_context_verify, &pubkey, vch, size())) { return false; } if (!secp256k1_ec_pubkey_tweak_add(secp256k1_context_verify, &pubkey, out)) { From 8a875d6e36822fb2cc7841bacbd1adbc0a9bf299 Mon Sep 17 00:00:00 2001 From: practicalswift Date: Tue, 17 Jul 2018 19:21:33 +0200 Subject: [PATCH 21/31] Remove redundant forward declaration --- src/qt/clientmodel.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/qt/clientmodel.cpp b/src/qt/clientmodel.cpp index 046b197957..e2ffefba07 100644 --- a/src/qt/clientmodel.cpp +++ b/src/qt/clientmodel.cpp @@ -29,8 +29,6 @@ #include #include -class CBlockIndex; - static int64_t nLastHeaderTipUpdateNotification = 0; static int64_t nLastBlockTipUpdateNotification = 0; From 0cdc6a09c821cec9d1c36d8387755b4451a7e9a5 Mon Sep 17 00:00:00 2001 From: practicalswift Date: Tue, 17 Jul 2018 19:22:17 +0200 Subject: [PATCH 22/31] Remove redundant unused variables --- src/script/standard.cpp | 2 -- src/test/miner_tests.cpp | 2 +- src/test/transaction_tests.cpp | 1 - src/txmempool.cpp | 1 - 4 files changed, 1 insertion(+), 5 deletions(-) diff --git a/src/script/standard.cpp b/src/script/standard.cpp index f0b2c62a91..d7b1724790 100644 --- a/src/script/standard.cpp +++ b/src/script/standard.cpp @@ -324,8 +324,6 @@ CScript GetScriptForMultisig(int nRequired, const std::vector& keys) CScript GetScriptForWitness(const CScript& redeemscript) { - CScript ret; - txnouttype typ; std::vector > vSolutions; if (Solver(redeemscript, typ, vSolutions)) { diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index 9a325f5f4c..10c7fd00e8 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -209,7 +209,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) const CChainParams& chainparams = *chainParams; CScript scriptPubKey = CScript() << ParseHex("04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38c4f35504e51ec112de5c384df7ba0b8d578a4c702b6bf11d5f") << OP_CHECKSIG; std::unique_ptr pblocktemplate; - CMutableTransaction tx,tx2; + CMutableTransaction tx; CScript script; uint256 hash; TestMemPoolEntryHelper entry; diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 45dc0e3571..aff270942e 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -547,7 +547,6 @@ BOOST_AUTO_TEST_CASE(test_witness) CTransactionRef output1, output2; CMutableTransaction input1, input2; - SignatureData sigdata; // Normal pay-to-compressed-pubkey. CreateCreditAndSpend(keystore, scriptPubkey1, output1, input1); diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 8090172e3f..1db006ecd1 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -716,7 +716,6 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const while (!waitingOnDependants.empty()) { const CTxMemPoolEntry* entry = waitingOnDependants.front(); waitingOnDependants.pop_front(); - CValidationState state; if (!mempoolDuplicate.HaveInputs(entry->GetTx())) { waitingOnDependants.push_back(entry); stepsSinceLastRemove++; From e32c97850e745ccec9ab0881560a065cf72d56d1 Mon Sep 17 00:00:00 2001 From: practicalswift Date: Tue, 17 Jul 2018 19:23:46 +0200 Subject: [PATCH 23/31] Remove redundant statement --- src/wallet/coinselection.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp index a403411e5b..78258cdec2 100644 --- a/src/wallet/coinselection.cpp +++ b/src/wallet/coinselection.cpp @@ -115,7 +115,7 @@ bool SelectCoinsBnB(std::vector& utxo_pool, const CAmount& target_va while (!curr_selection.empty() && !curr_selection.back()) { curr_selection.pop_back(); curr_available_value += utxo_pool.at(curr_selection.size()).effective_value; - }; + } if (curr_selection.empty()) { // We have walked back to the first utxo and no branch is untraversed. All solutions searched break; From 22fbbfbb6962fc38bbb0f3b7b440a7732da20df4 Mon Sep 17 00:00:00 2001 From: Conor Scott Date: Thu, 22 Mar 2018 18:21:28 +0400 Subject: [PATCH 24/31] [RPC] Remove field in getblocktemplate help that has never been used --- src/rpc/mining.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index a257511a21..fcb4bf8758 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -340,7 +340,6 @@ static UniValue getblocktemplate(const JSONRPCRequest& request) " \"fee\": n, (numeric) difference in value between transaction inputs and outputs (in satoshis); for coinbase transactions, this is a negative Number of the total collected block fees (ie, not including the block subsidy); if key is not present, fee is unknown and clients MUST NOT assume there isn't one\n" " \"sigops\" : n, (numeric) total SigOps cost, as counted for purposes of block limits; if key is not present, sigop cost is unknown and clients MUST NOT assume it is zero\n" " \"weight\" : n, (numeric) total transaction weight, as counted for purposes of block limits\n" - " \"required\" : true|false (boolean) if provided and true, this transaction must be in the final block\n" " }\n" " ,...\n" " ],\n" From 093090581ac02ee9c5f59f401cc5e6ba87d6e79e Mon Sep 17 00:00:00 2001 From: practicalswift Date: Thu, 22 Feb 2018 22:16:54 +0100 Subject: [PATCH 25/31] Avoid locking mutexes that are already held by the same thread --- src/txmempool.cpp | 2 +- src/txmempool.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 1db006ecd1..d579f131cd 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -542,7 +542,7 @@ void CTxMemPool::removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMem void CTxMemPool::removeConflicts(const CTransaction &tx) { // Remove transactions which depend on inputs of tx, recursively - LOCK(cs); + AssertLockHeld(cs); for (const CTxIn &txin : tx.vin) { auto it = mapNextTx.find(txin.prevout); if (it != mapNextTx.end()) { diff --git a/src/txmempool.h b/src/txmempool.h index ebfcf36e11..784d5453b2 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -544,7 +544,7 @@ class CTxMemPool void removeRecursive(const CTransaction &tx, MemPoolRemovalReason reason = MemPoolRemovalReason::UNKNOWN); void removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMemPoolHeight, int flags); - void removeConflicts(const CTransaction &tx); + void removeConflicts(const CTransaction &tx) EXCLUSIVE_LOCKS_REQUIRED(cs); void removeForBlock(const std::vector& vtx, unsigned int nBlockHeight); void clear(); From 56268b407824abd93ad891cce31c46328b9f4840 Mon Sep 17 00:00:00 2001 From: AtsukiTak Date: Sat, 21 Jul 2018 18:53:54 +0900 Subject: [PATCH 26/31] tiny refactor for ArgsManager This commit contains 2 refactors. 1. mark "const" on ArgsManager::GetHelpMessage and IsArgKnown. 2. remove unused "error" argument from ArgsManager::IsArgKnown. Firstly, I mark "const" on where it is possible to. It is mentioned before (e.g. https://github.com/bitcoin/bitcoin/pull/13190#pullrequestreview-118823133). And about 2nd change, ArgsManager::IsArgKnown was added at commit #4f8704d which was merged at PR #13112. But from its beggining, "error" argument never be used. I think it should be refactored. --- src/util.cpp | 8 ++++---- src/util.h | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/util.cpp b/src/util.cpp index e238c8605d..4e21f93ca5 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -446,7 +446,7 @@ bool ArgsManager::ParseParameters(int argc, const char* const argv[], std::strin // Check that the arg is known if (!(IsSwitchChar(key[0]) && key.size() == 1)) { - if (!IsArgKnown(key, error)) { + if (!IsArgKnown(key)) { error = strprintf("Invalid parameter %s", key.c_str()); return false; } @@ -466,7 +466,7 @@ bool ArgsManager::ParseParameters(int argc, const char* const argv[], std::strin return true; } -bool ArgsManager::IsArgKnown(const std::string& key, std::string& error) +bool ArgsManager::IsArgKnown(const std::string& key) const { size_t option_index = key.find('.'); std::string arg_no_net; @@ -591,7 +591,7 @@ void ArgsManager::AddHiddenArgs(const std::vector& names) } } -std::string ArgsManager::GetHelpMessage() +std::string ArgsManager::GetHelpMessage() const { const bool show_debug = gArgs.GetBoolArg("-help-debug", false); @@ -859,7 +859,7 @@ bool ArgsManager::ReadConfigStream(std::istream& stream, std::string& error, boo } // Check that the arg is known - if (!IsArgKnown(strKey, error) && !ignore_invalid_keys) { + if (!IsArgKnown(strKey) && !ignore_invalid_keys) { error = strprintf("Invalid configuration value %s", option.first.c_str()); return false; } diff --git a/src/util.h b/src/util.h index f3d713165f..64d0e0daf4 100644 --- a/src/util.h +++ b/src/util.h @@ -277,12 +277,12 @@ class ArgsManager /** * Get the help string */ - std::string GetHelpMessage(); + std::string GetHelpMessage() const; /** * Check whether we know of this arg */ - bool IsArgKnown(const std::string& key, std::string& error); + bool IsArgKnown(const std::string& key) const; }; extern ArgsManager gArgs; From b1d7b1e7bcbfcc753dbe7f3ca9500a49cfeb7a95 Mon Sep 17 00:00:00 2001 From: Ben Woosley Date: Thu, 5 Jul 2018 23:43:54 -0400 Subject: [PATCH 27/31] Drop dead code from Stacks Stacks is local to this file, and only used in DataFromTransaction, so it's easy to confirm this code is unused. --- src/script/sign.cpp | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/src/script/sign.cpp b/src/script/sign.cpp index f1ac1f411a..c2ae4ff2ea 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -288,18 +288,11 @@ struct Stacks std::vector script; std::vector witness; - Stacks() {} - explicit Stacks(const std::vector& scriptSigStack_) : script(scriptSigStack_), witness() {} + Stacks() = delete; + Stacks(const Stacks&) = delete; explicit Stacks(const SignatureData& data) : witness(data.scriptWitness.stack) { EvalScript(script, data.scriptSig, SCRIPT_VERIFY_STRICTENC, BaseSignatureChecker(), SigVersion::BASE); } - - SignatureData Output() const { - SignatureData result; - result.scriptSig = PushAll(script); - result.scriptWitness.stack = witness; - return result; - } }; } From 66f9bb9ef530cb9894b7e88c90c43bfbecd6ea60 Mon Sep 17 00:00:00 2001 From: lmanners Date: Thu, 10 May 2018 18:23:22 +0200 Subject: [PATCH 28/31] Net: Fixed a race condition when disabling the network. This change addresses a race condition where setnetworkactive=false wouldn't always disconnect all peers. Before this change, the following could happen: 1. Thread A -- Begins connecting to a node. 2. Thread B -- Sets kNetworkActive=false and disconnects connected nodes. 3. Thread A -- Finishes connecting and adds node to list of connected nodes. The node that was connected from Thread A remains connected and active, even though kNetworkActive=false. To fix the race, disconnections when kNetworkActive=false are now handled in the main network loop. fixes #13038 --- src/net.cpp | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index b13a93236f..290fdf64d3 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1163,6 +1163,17 @@ void CConnman::ThreadSocketHandler() // { LOCK(cs_vNodes); + + if (!fNetworkActive) { + // Disconnect any connected nodes + for (CNode* pnode : vNodes) { + if (!pnode->fDisconnect) { + LogPrint(BCLog::NET, "Network not active, dropping peer=%d\n", pnode->GetId()); + pnode->fDisconnect = true; + } + } + } + // Disconnect unused nodes std::vector vNodesCopy = vNodes; for (CNode* pnode : vNodesCopy) @@ -2198,14 +2209,6 @@ void CConnman::SetNetworkActive(bool active) fNetworkActive = active; - if (!fNetworkActive) { - LOCK(cs_vNodes); - // Close sockets to all nodes - for (CNode* pnode : vNodes) { - pnode->CloseSocketDisconnect(); - } - } - uiInterface.NotifyNetworkActiveChanged(fNetworkActive); } From 2c613380ad97b879d2975c028344fb8eeceb858b Mon Sep 17 00:00:00 2001 From: Nikolay Mitev Date: Sun, 22 Jul 2018 16:49:14 +0300 Subject: [PATCH 29/31] trivial: remove unneeded include --- src/bitcoind.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp index f3edaaf63d..7b07e367ae 100644 --- a/src/bitcoind.cpp +++ b/src/bitcoind.cpp @@ -21,8 +21,6 @@ #include #include -#include - #include /* Introduction text for doxygen: */ From 83975fdd8c8be5073c47020b18b837f8b75db179 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Wed, 18 Jul 2018 17:52:43 -0700 Subject: [PATCH 30/31] Avoid creating a temporary vector for size-prefixed elements --- src/script/sign.h | 18 ++++++++---------- src/serialize.h | 8 ++++++++ 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/src/script/sign.h b/src/script/sign.h index e3a6196b28..250b637035 100644 --- a/src/script/sign.h +++ b/src/script/sign.h @@ -116,26 +116,24 @@ static constexpr uint8_t PSBT_OUT_BIP32_DERIVATION = 0x02; // as a 0 length key which indicates that this is the separator. The separator has no value. static constexpr uint8_t PSBT_SEPARATOR = 0x00; -// Takes a stream and multiple arguments and serializes them into a vector and then into the stream +// Takes a stream and multiple arguments and serializes them as if first serialized into a vector and then into the stream // The resulting output into the stream has the total serialized length of all of the objects followed by all objects concatenated with each other. template void SerializeToVector(Stream& s, const X&... args) { - std::vector ret; - CVectorWriter ss(SER_NETWORK, PROTOCOL_VERSION, ret, 0); - SerializeMany(ss, args...); - s << ret; + WriteCompactSize(s, GetSerializeSizeMany(s, args...)); + SerializeMany(s, args...); } // Takes a stream and multiple arguments and unserializes them first as a vector then each object individually in the order provided in the arguments template void UnserializeFromVector(Stream& s, X&... args) { - std::vector data; - s >> data; - CDataStream ss(data, SER_NETWORK, PROTOCOL_VERSION); - UnserializeMany(ss, args...); - if (!ss.eof()) { + size_t expected_size = ReadCompactSize(s); + size_t remaining_before = s.size(); + UnserializeMany(s, args...); + size_t remaining_after = s.size(); + if (remaining_after + expected_size != remaining_before) { throw std::ios_base::failure("Size of value was not the stated size"); } } diff --git a/src/serialize.h b/src/serialize.h index df3b47ba87..627225b6ef 100644 --- a/src/serialize.h +++ b/src/serialize.h @@ -991,4 +991,12 @@ size_t GetSerializeSize(const S& s, const T& t) return (CSizeComputer(s.GetType(), s.GetVersion()) << t).size(); } +template +size_t GetSerializeSizeMany(const S& s, const T&... t) +{ + CSizeComputer sc(s.GetType(), s.GetVersion()); + SerializeMany(sc, t...); + return sc.size(); +} + #endif // BITCOIN_SERIALIZE_H From 4cf7d34a2058fc922e1a0c5cd1c2107f9d14ff6d Mon Sep 17 00:00:00 2001 From: Daniel Kraft Date: Mon, 23 Jul 2018 14:43:45 +0200 Subject: [PATCH 31/31] Skip is_closing() check when not available. https://github.com/bitcoin/bitcoin/pull/13715 introduced a new check for _transport.is_closing() in mininode's P2PConnection's. This function is only available from Python 3.4.4, though, while Bitcoin is supposed to support all Python 3.4 versions. In this change, we make the check conditional on is_closing() being available. If it is not, then we revert to the behaviour before the check was introduced; this means that https://github.com/bitcoin/bitcoin/issues/13579 is not fixed for old systems, but at least the tests work as they used to do before. This includes a small refactoring from a one-line lambda to an inline function, because this makes the code easier to read with more and more conditions being added. Fixes https://github.com/bitcoin/bitcoin/issues/13745. --- test/functional/test_framework/mininode.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/test/functional/test_framework/mininode.py b/test/functional/test_framework/mininode.py index d5b1a90687..ccf767d357 100755 --- a/test/functional/test_framework/mininode.py +++ b/test/functional/test_framework/mininode.py @@ -179,7 +179,17 @@ def send_message(self, message): raise IOError('Not connected') self._log_message("send", message) tmsg = self._build_message(message) - NetworkThread.network_event_loop.call_soon_threadsafe(lambda: self._transport and not self._transport.is_closing() and self._transport.write(tmsg)) + + def maybe_write(): + if not self._transport: + return + # Python <3.4.4 does not have is_closing, so we have to check for + # its existence explicitly as long as Bitcoin Core supports all + # Python 3.4 versions. + if hasattr(self._transport, 'is_closing') and self._transport.is_closing(): + return + self._transport.write(tmsg) + NetworkThread.network_event_loop.call_soon_threadsafe(maybe_write) # Class utility methods