From f3a2b5237688e9f574444e793724664b00fb7f2a Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Wed, 22 Nov 2023 14:47:00 -0500 Subject: [PATCH 001/868] serialization: Support for multiple parameters This commit makes a minimal change to the ParamsStream class to let it retrieve multiple parameters. Followup commits after this commit clean up code using ParamsStream and make it easier to set multiple parameters. Currently it is only possible to attach one serialization parameter to a stream at a time. For example, it is not possible to set a parameter controlling the transaction format and a parameter controlling the address format at the same time because one parameter will override the other. This limitation is inconvenient for multiprocess code since it is not possible to create just one type of stream and serialize any object to it. Instead it is necessary to create different streams for different object types, which requires extra boilerplate and makes using the new parameter fields a lot more awkward than the older version and type fields. Fix this problem by allowing an unlimited number of serialization stream parameters to be set, and allowing them to be requested by type. Later parameters will still override earlier parameters, but only if they have the same type. This change requires replacing the stream.GetParams() method with a stream.GetParams() method in order for serialization code to retrieve the desired parameters. This change is more verbose, but probably a good thing for readability because previously it could be difficult to know what type the GetParams() method would return, and now it is more obvious. --- src/netaddress.h | 4 ++-- src/primitives/transaction.h | 6 +++--- src/protocol.h | 3 ++- src/serialize.h | 39 ++++++++++++++++-------------------- src/test/serialize_tests.cpp | 7 ++++--- 5 files changed, 28 insertions(+), 31 deletions(-) diff --git a/src/netaddress.h b/src/netaddress.h index 08dd77c0ffab5..5e5c412586f7f 100644 --- a/src/netaddress.h +++ b/src/netaddress.h @@ -241,7 +241,7 @@ class CNetAddr template void Serialize(Stream& s) const { - if (s.GetParams().enc == Encoding::V2) { + if (s.template GetParams().enc == Encoding::V2) { SerializeV2Stream(s); } else { SerializeV1Stream(s); @@ -254,7 +254,7 @@ class CNetAddr template void Unserialize(Stream& s) { - if (s.GetParams().enc == Encoding::V2) { + if (s.template GetParams().enc == Encoding::V2) { UnserializeV2Stream(s); } else { UnserializeV1Stream(s); diff --git a/src/primitives/transaction.h b/src/primitives/transaction.h index ccbeb3ec49bb6..d15b8005f90e0 100644 --- a/src/primitives/transaction.h +++ b/src/primitives/transaction.h @@ -326,7 +326,7 @@ class CTransaction template inline void Serialize(Stream& s) const { - SerializeTransaction(*this, s, s.GetParams()); + SerializeTransaction(*this, s, s.template GetParams()); } /** This deserializing constructor is provided instead of an Unserialize method. @@ -386,12 +386,12 @@ struct CMutableTransaction template inline void Serialize(Stream& s) const { - SerializeTransaction(*this, s, s.GetParams()); + SerializeTransaction(*this, s, s.template GetParams()); } template inline void Unserialize(Stream& s) { - UnserializeTransaction(*this, s, s.GetParams()); + UnserializeTransaction(*this, s, s.template GetParams()); } template diff --git a/src/protocol.h b/src/protocol.h index e405253632da0..58a7287d03667 100644 --- a/src/protocol.h +++ b/src/protocol.h @@ -406,9 +406,10 @@ class CAddress : public CService static constexpr SerParams V1_DISK{{CNetAddr::Encoding::V1}, Format::Disk}; static constexpr SerParams V2_DISK{{CNetAddr::Encoding::V2}, Format::Disk}; - SERIALIZE_METHODS_PARAMS(CAddress, obj, SerParams, params) + SERIALIZE_METHODS(CAddress, obj) { bool use_v2; + auto& params = SER_PARAMS(SerParams); if (params.fmt == Format::Disk) { // In the disk serialization format, the encoding (v1 or v2) is determined by a flag version // that's part of the serialization itself. ADDRV2_FORMAT in the stream version only determines diff --git a/src/serialize.h b/src/serialize.h index 19585c630ab2a..44e2db7ac7b66 100644 --- a/src/serialize.h +++ b/src/serialize.h @@ -181,9 +181,8 @@ const Out& AsBase(const In& x) static void SerializationOps(Type& obj, Stream& s, Operation ser_action) /** - * Variant of FORMATTER_METHODS that supports a declared parameter type. - * - * If a formatter has a declared parameter type, it must be invoked directly or + * Formatter methods can retrieve parameters attached to a stream using the + * SER_PARAMS(type) macro as long as the stream is created directly or * indirectly with a parameter of that type. This permits making serialization * depend on run-time context in a type-safe way. * @@ -191,7 +190,8 @@ const Out& AsBase(const In& x) * struct BarParameter { bool fancy; ... }; * struct Bar { ... }; * struct FooFormatter { - * FORMATTER_METHODS(Bar, obj, BarParameter, param) { + * FORMATTER_METHODS(Bar, obj) { + * auto& param = SER_PARAMS(BarParameter); * if (param.fancy) { * READWRITE(VARINT(obj.value)); * } else { @@ -213,13 +213,7 @@ const Out& AsBase(const In& x) * Compilation will fail in any context where serialization is invoked but * no parameter of a type convertible to BarParameter is provided. */ -#define FORMATTER_METHODS_PARAMS(cls, obj, paramcls, paramobj) \ - template \ - static void Ser(Stream& s, const cls& obj) { SerializationOps(obj, s, ActionSerialize{}, s.GetParams()); } \ - template \ - static void Unser(Stream& s, cls& obj) { SerializationOps(obj, s, ActionUnserialize{}, s.GetParams()); } \ - template \ - static void SerializationOps(Type& obj, Stream& s, Operation ser_action, const paramcls& paramobj) +#define SER_PARAMS(type) (s.template GetParams()) #define BASE_SERIALIZE_METHODS(cls) \ template \ @@ -246,15 +240,6 @@ const Out& AsBase(const In& x) BASE_SERIALIZE_METHODS(cls) \ FORMATTER_METHODS(cls, obj) -/** - * Variant of SERIALIZE_METHODS that supports a declared parameter type. - * - * See FORMATTER_METHODS_PARAMS for more information on parameters. - */ -#define SERIALIZE_METHODS_PARAMS(cls, obj, paramcls, paramobj) \ - BASE_SERIALIZE_METHODS(cls) \ - FORMATTER_METHODS_PARAMS(cls, obj, paramcls, paramobj) - // Templates for serializing to anything that looks like a stream, // i.e. anything that supports .read(Span) and .write(Span) // @@ -1134,9 +1119,19 @@ class ParamsStream void ignore(size_t num) { m_substream.ignore(num); } bool eof() const { return m_substream.eof(); } size_t size() const { return m_substream.size(); } - const Params& GetParams() const { return m_params; } int GetVersion() = delete; // Deprecated with Params usage int GetType() = delete; // Deprecated with Params usage + + //! Get reference to stream parameters. + template + const auto& GetParams() const + { + if constexpr (std::is_convertible_v) { + return m_params; + } else { + return m_substream.template GetParams

(); + } + } }; /** Wrapper that serializes objects with the specified parameters. */ @@ -1176,7 +1171,7 @@ class ParamsWrapper /** \ * Return a wrapper around t that (de)serializes it with specified parameter params. \ * \ - * See FORMATTER_METHODS_PARAMS for more information on serialization parameters. \ + * See SER_PARAMS for more information on serialization parameters. \ */ \ template \ auto operator()(T&& t) const \ diff --git a/src/test/serialize_tests.cpp b/src/test/serialize_tests.cpp index d75eb499b4a3c..18356f2df03fb 100644 --- a/src/test/serialize_tests.cpp +++ b/src/test/serialize_tests.cpp @@ -289,7 +289,7 @@ class Base template void Serialize(Stream& s) const { - if (s.GetParams().m_base_format == BaseFormat::RAW) { + if (s.template GetParams().m_base_format == BaseFormat::RAW) { s << m_base_data; } else { s << Span{HexStr(Span{&m_base_data, 1})}; @@ -299,7 +299,7 @@ class Base template void Unserialize(Stream& s) { - if (s.GetParams().m_base_format == BaseFormat::RAW) { + if (s.template GetParams().m_base_format == BaseFormat::RAW) { s >> m_base_data; } else { std::string hex{"aa"}; @@ -327,8 +327,9 @@ class Derived : public Base public: std::string m_derived_data; - SERIALIZE_METHODS_PARAMS(Derived, obj, DerivedAndBaseFormat, fmt) + SERIALIZE_METHODS(Derived, obj) { + auto& fmt = SER_PARAMS(DerivedAndBaseFormat); READWRITE(fmt.m_base_format(AsBase(obj))); if (ser_action.ForRead()) { From 3bf00e13609eefa6ddb11353519bb1aec2342513 Mon Sep 17 00:00:00 2001 From: Marnix <93143998+MarnixCroes@users.noreply.github.com> Date: Wed, 17 Jan 2024 08:48:03 +0100 Subject: [PATCH 002/868] gui: debugwindow: update session ID tooltip remove "if any" --- src/qt/forms/debugwindow.ui | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qt/forms/debugwindow.ui b/src/qt/forms/debugwindow.ui index 60e9bcde337ad..eeea53864a43d 100644 --- a/src/qt/forms/debugwindow.ui +++ b/src/qt/forms/debugwindow.ui @@ -1113,7 +1113,7 @@ - The BIP324 session ID string in hex, if any. + The BIP324 session ID string in hex. Session ID From 436e88f4336199998184cbfa5d1c889ffaefbfb5 Mon Sep 17 00:00:00 2001 From: ismaelsadeeq Date: Sun, 25 Jun 2023 16:54:35 +0100 Subject: [PATCH 003/868] bumpfee: ignore WALLET_INCREMENTAL_RELAY_FEE when user specifies fee rate This commit update CheckFeeRate's incrementalRelayFee to use relayIncrementalFee not max of (walletIncrementalRelayfee and relayIncrementalFee). The restriction is not needed since user provided the fee rate. --- src/wallet/feebumper.cpp | 2 +- test/functional/wallet_bumpfee.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index 6a8453965be71..c65133006f1f3 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -92,7 +92,7 @@ static feebumper::Result CheckFeeRate(const CWallet& wallet, const CMutableTrans } CAmount new_total_fee = newFeerate.GetFee(maxTxSize) + combined_bump_fee.value(); - CFeeRate incrementalRelayFee = std::max(wallet.chain().relayIncrementalFee(), CFeeRate(WALLET_INCREMENTAL_RELAY_FEE)); + CFeeRate incrementalRelayFee = wallet.chain().relayIncrementalFee(); // Min total fee is old fee + relay fee CAmount minTotalFee = old_fee + incrementalRelayFee.GetFee(maxTxSize); diff --git a/test/functional/wallet_bumpfee.py b/test/functional/wallet_bumpfee.py index fea933a93b2b8..e3d34ba7670b8 100755 --- a/test/functional/wallet_bumpfee.py +++ b/test/functional/wallet_bumpfee.py @@ -815,7 +815,7 @@ def test_feerate_checks_replaced_outputs(self, rbf_node, peer_node): # Since the bumped tx will replace all of the outputs with a single output, we can estimate that its size will 31 * (len(outputs) - 1) bytes smaller tx_size = tx_details["decoded"]["vsize"] est_bumped_size = tx_size - (len(tx_details["decoded"]["vout"]) - 1) * 31 - inc_fee_rate = max(rbf_node.getmempoolinfo()["incrementalrelayfee"], Decimal(0.00005000)) # Wallet has a fixed incremental relay fee of 5 sat/vb + inc_fee_rate = rbf_node.getmempoolinfo()["incrementalrelayfee"] # RPC gives us fee as negative min_fee = (-tx_details["fee"] + get_fee(est_bumped_size, inc_fee_rate)) * Decimal(1e8) min_fee_rate = (min_fee / est_bumped_size).quantize(Decimal("1.000")) From f58beabe754363cb7d5b24032fd392654b9514ac Mon Sep 17 00:00:00 2001 From: ismaelsadeeq Date: Mon, 26 Jun 2023 22:06:25 +0100 Subject: [PATCH 004/868] test: bumpfee with user specified fee_rate ignores walletIncrementalRelayFee --- test/functional/wallet_bumpfee.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/test/functional/wallet_bumpfee.py b/test/functional/wallet_bumpfee.py index e3d34ba7670b8..dd3559f20dddf 100755 --- a/test/functional/wallet_bumpfee.py +++ b/test/functional/wallet_bumpfee.py @@ -116,6 +116,7 @@ def run_test(self): # Context independent tests test_feerate_checks_replaced_outputs(self, rbf_node, peer_node) + test_bumpfee_with_feerate_ignores_walletincrementalrelayfee(self, rbf_node, peer_node) def test_invalid_parameters(self, rbf_node, peer_node, dest_address): self.log.info('Test invalid parameters') @@ -829,5 +830,27 @@ def test_feerate_checks_replaced_outputs(self, rbf_node, peer_node): self.clear_mempool() +def test_bumpfee_with_feerate_ignores_walletincrementalrelayfee(self, rbf_node, peer_node): + self.log.info('Test that bumpfee with fee_rate ignores walletincrementalrelayfee') + # Make sure there is enough balance + peer_node.sendtoaddress(rbf_node.getnewaddress(), 2) + self.generate(peer_node, 1) + + dest_address = peer_node.getnewaddress(address_type="bech32") + tx = rbf_node.send(outputs=[{dest_address: 1}], fee_rate=2) + + # Ensure you can not fee bump with a fee_rate below or equal to the original fee_rate + assert_raises_rpc_error(-8, "Insufficient total fee", rbf_node.bumpfee, tx["txid"], {"fee_rate": 1}) + assert_raises_rpc_error(-8, "Insufficient total fee", rbf_node.bumpfee, tx["txid"], {"fee_rate": 2}) + + # Ensure you can not fee bump if the fee_rate is more than original fee_rate but the total fee from new fee_rate is + # less than (original fee + incrementalrelayfee) + assert_raises_rpc_error(-8, "Insufficient total fee", rbf_node.bumpfee, tx["txid"], {"fee_rate": 2.8}) + + # You can fee bump as long as the new fee set from fee_rate is atleast (original fee + incrementalrelayfee) + rbf_node.bumpfee(tx["txid"], {"fee_rate": 3}) + self.clear_mempool() + + if __name__ == "__main__": BumpFeeTest().main() From 100e8a75bf5d8196c005331bd8f2ed42ada6d8d0 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Thu, 24 Aug 2023 15:20:55 +0200 Subject: [PATCH 005/868] rpc: check and throw specific pubkey parsing errors in `HexToPubKey` In the helper `HexToPubKey`, check for three different causes of legacy public key parsing errors (in this order): - pubkey is not a hex string - pubkey doesn't have a valid length (33 or 65 bytes) [NEW] - pubkey is cryptographically invalid, i.e. not on curve (`IsFullyValid` check) and throw a specific error message for each one. Note that the error code is identical for all of them (-5), so this doesn't break RPC API compatibility. The helper is currently used for the RPCs `createmultisig` and `addmultisigaddress`. The length checks can be removed from the call-sites and error message checks in the functional tests are adapted. --- src/rpc/output_script.cpp | 6 +----- src/rpc/util.cpp | 7 +++++-- test/functional/rpc_rawtransaction.py | 4 ++-- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/rpc/output_script.cpp b/src/rpc/output_script.cpp index f9343f48a897e..474d9076be031 100644 --- a/src/rpc/output_script.cpp +++ b/src/rpc/output_script.cpp @@ -124,11 +124,7 @@ static RPCHelpMan createmultisig() const UniValue& keys = request.params[1].get_array(); std::vector pubkeys; for (unsigned int i = 0; i < keys.size(); ++i) { - if (IsHex(keys[i].get_str()) && (keys[i].get_str().length() == 66 || keys[i].get_str().length() == 130)) { - pubkeys.push_back(HexToPubKey(keys[i].get_str())); - } else { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Invalid public key: %s\n.", keys[i].get_str())); - } + pubkeys.push_back(HexToPubKey(keys[i].get_str())); } // Get the output type diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index cf48ee11e7b22..cc11dc4a46387 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -180,11 +180,14 @@ std::string HelpExampleRpcNamed(const std::string& methodname, const RPCArgList& CPubKey HexToPubKey(const std::string& hex_in) { if (!IsHex(hex_in)) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid public key: " + hex_in); + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Pubkey \"" + hex_in + "\" must be a hex string"); + } + if (hex_in.length() != 66 && hex_in.length() != 130) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Pubkey \"" + hex_in + "\" must have a length of either 33 or 65 bytes"); } CPubKey vchPubKey(ParseHex(hex_in)); if (!vchPubKey.IsFullyValid()) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid public key: " + hex_in); + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Pubkey \"" + hex_in + "\" must be cryptographically valid."); } return vchPubKey; } diff --git a/test/functional/rpc_rawtransaction.py b/test/functional/rpc_rawtransaction.py index c12865b5e39b6..e5d7cea13594c 100755 --- a/test/functional/rpc_rawtransaction.py +++ b/test/functional/rpc_rawtransaction.py @@ -491,11 +491,11 @@ def raw_multisig_transaction_legacy_tests(self): addr2Obj = self.nodes[2].getaddressinfo(addr2) # Tests for createmultisig and addmultisigaddress - assert_raises_rpc_error(-5, "Invalid public key", self.nodes[0].createmultisig, 1, ["01020304"]) + assert_raises_rpc_error(-5, 'Pubkey "01020304" must have a length of either 33 or 65 bytes', self.nodes[0].createmultisig, 1, ["01020304"]) # createmultisig can only take public keys self.nodes[0].createmultisig(2, [addr1Obj['pubkey'], addr2Obj['pubkey']]) # addmultisigaddress can take both pubkeys and addresses so long as they are in the wallet, which is tested here - assert_raises_rpc_error(-5, "Invalid public key", self.nodes[0].createmultisig, 2, [addr1Obj['pubkey'], addr1]) + assert_raises_rpc_error(-5, f'Pubkey "{addr1}" must be a hex string', self.nodes[0].createmultisig, 2, [addr1Obj['pubkey'], addr1]) mSigObj = self.nodes[2].addmultisigaddress(2, [addr1Obj['pubkey'], addr1])['address'] From c740b154d193b91ca42f18759098d3fef6eaab05 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Thu, 24 Aug 2023 16:05:44 +0200 Subject: [PATCH 006/868] rpc: use `HexToPubKey` helper for all legacy pubkey-parsing RPCs This deduplicates code and leads to more consistent and detailed error messages. Affected are legacy import RPCs (`importpubkey`, `importmulti`) and other ones where solving data can be provided (`fundrawtransaction`, `walletcreatefundedpsbt`, `send`, `sendall`). --- src/wallet/rpc/backup.cpp | 17 ++--------------- src/wallet/rpc/spend.cpp | 10 +--------- test/functional/wallet_basic.py | 7 ++++--- test/functional/wallet_fundrawtransaction.py | 4 ++-- 4 files changed, 9 insertions(+), 29 deletions(-) diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp index 99c548bf1d5e5..d17a22060352a 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -460,12 +460,7 @@ RPCHelpMan importpubkey() throw JSONRPCError(RPC_WALLET_ERROR, "Wallet is currently rescanning. Abort existing rescan or wait."); } - if (!IsHex(request.params[0].get_str())) - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Pubkey must be a hex string"); - std::vector data(ParseHex(request.params[0].get_str())); - CPubKey pubKey(data); - if (!pubKey.IsFullyValid()) - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Pubkey is not a valid public key"); + CPubKey pubKey = HexToPubKey(request.params[0].get_str()); { LOCK(pwallet->cs_wallet); @@ -986,15 +981,7 @@ static UniValue ProcessImportLegacy(ImportData& import_data, std::map(parsed_witnessscript.begin(), parsed_witnessscript.end()); } for (size_t i = 0; i < pubKeys.size(); ++i) { - const auto& str = pubKeys[i].get_str(); - if (!IsHex(str)) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Pubkey \"" + str + "\" must be a hex string"); - } - auto parsed_pubkey = ParseHex(str); - CPubKey pubkey(parsed_pubkey); - if (!pubkey.IsFullyValid()) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Pubkey \"" + str + "\" is not a valid public key"); - } + CPubKey pubkey = HexToPubKey(pubKeys[i].get_str()); pubkey_map.emplace(pubkey.GetID(), pubkey); ordered_pubkeys.push_back(pubkey.GetID()); } diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index 6060f017ce994..1a364a75edcd2 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -627,15 +627,7 @@ CreatedTransactionResult FundTransaction(CWallet& wallet, const CMutableTransact const UniValue solving_data = options["solving_data"].get_obj(); if (solving_data.exists("pubkeys")) { for (const UniValue& pk_univ : solving_data["pubkeys"].get_array().getValues()) { - const std::string& pk_str = pk_univ.get_str(); - if (!IsHex(pk_str)) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("'%s' is not hex", pk_str)); - } - const std::vector data(ParseHex(pk_str)); - const CPubKey pubkey(data.begin(), data.end()); - if (!pubkey.IsFullyValid()) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("'%s' is not a valid public key", pk_str)); - } + const CPubKey pubkey = HexToPubKey(pk_univ.get_str()); coinControl.m_external_provider.pubkeys.emplace(pubkey.GetID(), pubkey); // Add witness script for pubkeys const CScript wit_script = GetScriptForDestination(WitnessV0KeyHash(pubkey)); diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index f798eee365cfb..dbb78e8d35960 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -459,10 +459,11 @@ def run_test(self): assert_raises_rpc_error(-5, "Invalid Bitcoin address or script", self.nodes[0].importaddress, "invalid") # This will raise an exception for attempting to import a pubkey that isn't in hex - assert_raises_rpc_error(-5, "Pubkey must be a hex string", self.nodes[0].importpubkey, "not hex") + assert_raises_rpc_error(-5, 'Pubkey "not hex" must be a hex string', self.nodes[0].importpubkey, "not hex") - # This will raise an exception for importing an invalid pubkey - assert_raises_rpc_error(-5, "Pubkey is not a valid public key", self.nodes[0].importpubkey, "5361746f736869204e616b616d6f746f") + # This will raise an exception for importing a pubkey with invalid length + too_short_pubkey = "5361746f736869204e616b616d6f746f" + assert_raises_rpc_error(-5, f'Pubkey "{too_short_pubkey}" must have a length of either 33 or 65 bytes', self.nodes[0].importpubkey, too_short_pubkey) # Bech32m addresses cannot be imported into a legacy wallet assert_raises_rpc_error(-5, "Bech32m addresses cannot be imported into legacy wallets", self.nodes[0].importaddress, "bcrt1p0xlxvlhemja6c4dqv22uapctqupfhlxm9h8z3k2e72q4k9hcz7vqc8gma6") diff --git a/test/functional/wallet_fundrawtransaction.py b/test/functional/wallet_fundrawtransaction.py index d886a59ac15bb..083da3d0b7769 100755 --- a/test/functional/wallet_fundrawtransaction.py +++ b/test/functional/wallet_fundrawtransaction.py @@ -1055,8 +1055,8 @@ def test_external_inputs(self): assert_raises_rpc_error(-4, "Not solvable pre-selected input COutPoint(%s, %s)" % (ext_utxo["txid"][0:10], ext_utxo["vout"]), wallet.fundrawtransaction, raw_tx) # Error conditions - assert_raises_rpc_error(-5, "'not a pubkey' is not hex", wallet.fundrawtransaction, raw_tx, solving_data={"pubkeys":["not a pubkey"]}) - assert_raises_rpc_error(-5, "'01234567890a0b0c0d0e0f' is not a valid public key", wallet.fundrawtransaction, raw_tx, solving_data={"pubkeys":["01234567890a0b0c0d0e0f"]}) + assert_raises_rpc_error(-5, 'Pubkey "not a pubkey" must be a hex string', wallet.fundrawtransaction, raw_tx, solving_data={"pubkeys":["not a pubkey"]}) + assert_raises_rpc_error(-5, 'Pubkey "01234567890a0b0c0d0e0f" must have a length of either 33 or 65 bytes', wallet.fundrawtransaction, raw_tx, solving_data={"pubkeys":["01234567890a0b0c0d0e0f"]}) assert_raises_rpc_error(-5, "'not a script' is not hex", wallet.fundrawtransaction, raw_tx, solving_data={"scripts":["not a script"]}) assert_raises_rpc_error(-8, "Unable to parse descriptor 'not a descriptor'", wallet.fundrawtransaction, raw_tx, solving_data={"descriptors":["not a descriptor"]}) assert_raises_rpc_error(-8, "Invalid parameter, missing vout key", wallet.fundrawtransaction, raw_tx, input_weights=[{"txid": ext_utxo["txid"]}]) From 98570fe29bb08d7edc48011aa6b9731c6ab4ed2e Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Thu, 24 Aug 2023 16:46:36 +0200 Subject: [PATCH 007/868] test: add coverage for parsing cryptographically invalid pubkeys --- test/functional/wallet_basic.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index dbb78e8d35960..3daec4dbd1488 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -461,9 +461,11 @@ def run_test(self): # This will raise an exception for attempting to import a pubkey that isn't in hex assert_raises_rpc_error(-5, 'Pubkey "not hex" must be a hex string', self.nodes[0].importpubkey, "not hex") - # This will raise an exception for importing a pubkey with invalid length + # This will raise exceptions for importing a pubkeys with invalid length / invalid coordinates too_short_pubkey = "5361746f736869204e616b616d6f746f" assert_raises_rpc_error(-5, f'Pubkey "{too_short_pubkey}" must have a length of either 33 or 65 bytes', self.nodes[0].importpubkey, too_short_pubkey) + not_on_curve_pubkey = bytes([4] + [0]*64).hex() # pubkey with coordinates (0,0) is not on curve + assert_raises_rpc_error(-5, f'Pubkey "{not_on_curve_pubkey}" must be cryptographically valid', self.nodes[0].importpubkey, not_on_curve_pubkey) # Bech32m addresses cannot be imported into a legacy wallet assert_raises_rpc_error(-5, "Bech32m addresses cannot be imported into legacy wallets", self.nodes[0].importaddress, "bcrt1p0xlxvlhemja6c4dqv22uapctqupfhlxm9h8z3k2e72q4k9hcz7vqc8gma6") From 544131f3fba9ea07fee29f9d3ee0116cd5d8a5b2 Mon Sep 17 00:00:00 2001 From: ishaanam Date: Thu, 30 Nov 2023 16:12:24 -0500 Subject: [PATCH 008/868] rpc, test: test sendall spends unconfirmed change and unconfirmed inputs when specified --- src/wallet/rpc/spend.cpp | 2 +- test/functional/wallet_sendall.py | 27 +++++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index 5a13b5ac8ecfa..0a43293b2258d 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -1292,7 +1292,7 @@ RPCHelpMan sendall() { return RPCHelpMan{"sendall", "EXPERIMENTAL warning: this call may be changed in future releases.\n" - "\nSpend the value of all (or specific) confirmed UTXOs in the wallet to one or more recipients.\n" + "\nSpend the value of all (or specific) confirmed UTXOs and unconfirmed change in the wallet to one or more recipients.\n" "Unconfirmed inbound UTXOs and locked UTXOs will not be spent. Sendall will respect the avoid_reuse wallet flag.\n" "If your wallet contains many small inputs, either because it received tiny payments or as a result of accumulating change, consider using `send_max` to exclude inputs that are worth less than the fees needed to spend them.\n", { diff --git a/test/functional/wallet_sendall.py b/test/functional/wallet_sendall.py index c2b800df2189e..70b52641c352e 100755 --- a/test/functional/wallet_sendall.py +++ b/test/functional/wallet_sendall.py @@ -379,6 +379,27 @@ def sendall_with_maxconf(self): assert_equal(len(self.wallet.listunspent()), 1) assert_equal(self.wallet.listunspent()[0]['confirmations'], 6) + @cleanup + def sendall_spends_unconfirmed_change(self): + self.log.info("Test that sendall spends unconfirmed change") + self.add_utxos([17]) + self.wallet.sendtoaddress(self.remainder_target, 10) + assert_greater_than(self.wallet.getbalances()["mine"]["trusted"], 6) + self.test_sendall_success(sendall_args = [self.remainder_target]) + + assert_equal(self.wallet.getbalance(), 0) + + @cleanup + def sendall_spends_unconfirmed_inputs_if_specified(self): + self.log.info("Test that sendall spends specified unconfirmed inputs") + self.def_wallet.sendtoaddress(self.wallet.getnewaddress(), 17) + self.wallet.syncwithvalidationinterfacequeue() + assert_equal(self.wallet.getbalances()["mine"]["untrusted_pending"], 17) + unspent = self.wallet.listunspent(minconf=0)[0] + + self.wallet.sendall(recipients=[self.remainder_target], inputs=[unspent]) + assert_equal(self.wallet.getbalance(), 0) + # This tests needs to be the last one otherwise @cleanup will fail with "Transaction too large" error def sendall_fails_with_transaction_too_large(self): self.log.info("Test that sendall fails if resulting transaction is too large") @@ -460,6 +481,12 @@ def run_test(self): # Sendall only uses outputs with less than a given number of confirmation when using minconf self.sendall_with_maxconf() + # Sendall spends unconfirmed change + self.sendall_spends_unconfirmed_change() + + # Sendall spends unconfirmed inputs if they are specified + self.sendall_spends_unconfirmed_inputs_if_specified() + # Sendall fails when many inputs result to too large transaction self.sendall_fails_with_transaction_too_large() From 36757941a05b65c2b61a83820afdf5effd8fc9a2 Mon Sep 17 00:00:00 2001 From: ishaanam Date: Sat, 9 Dec 2023 23:05:15 -0500 Subject: [PATCH 009/868] wallet, rpc: implement ancestor aware funding for sendall --- src/wallet/rpc/spend.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index 0a43293b2258d..1474c4ef4ee7f 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -1467,10 +1467,18 @@ RPCHelpMan sendall() } } + std::vector outpoints_spent; + outpoints_spent.reserve(rawTx.vin.size()); + + for (const CTxIn& tx_in : rawTx.vin) { + outpoints_spent.push_back(tx_in.prevout); + } + // estimate final size of tx const TxSize tx_size{CalculateMaximumSignedTxSize(CTransaction(rawTx), pwallet.get())}; const CAmount fee_from_size{fee_rate.GetFee(tx_size.vsize)}; - const CAmount effective_value{total_input_value - fee_from_size}; + const std::optional total_bump_fees{pwallet->chain().calculateCombinedBumpFee(outpoints_spent, fee_rate)}; + CAmount effective_value = total_input_value - fee_from_size - total_bump_fees.value_or(0); if (fee_from_size > pwallet->m_default_max_tx_fee) { throw JSONRPCError(RPC_WALLET_ERROR, TransactionErrorString(TransactionError::MAX_FEE_EXCEEDED).original); From cccddc03f0c625daeac7158eb20c1508aea5df39 Mon Sep 17 00:00:00 2001 From: Hernan Marino Date: Tue, 14 Mar 2023 14:01:17 -0300 Subject: [PATCH 010/868] Wallet encrypt on create, allow to navigate options --- src/qt/askpassphrasedialog.cpp | 35 +++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/src/qt/askpassphrasedialog.cpp b/src/qt/askpassphrasedialog.cpp index 246dff0069215..f0efa250d7ef6 100644 --- a/src/qt/askpassphrasedialog.cpp +++ b/src/qt/askpassphrasedialog.cpp @@ -104,10 +104,14 @@ void AskPassphraseDialog::accept() // Cannot encrypt with empty passphrase break; } - QMessageBox::StandardButton retval = QMessageBox::question(this, tr("Confirm wallet encryption"), - tr("Warning: If you encrypt your wallet and lose your passphrase, you will LOSE ALL OF YOUR BITCOINS!") + "

" + tr("Are you sure you wish to encrypt your wallet?"), - QMessageBox::Yes|QMessageBox::Cancel, - QMessageBox::Cancel); + QMessageBox msgBoxConfirm(QMessageBox::Question, + tr("Confirm wallet encryption"), + tr("Warning: If you encrypt your wallet and lose your passphrase, you will LOSE ALL OF YOUR BITCOINS!") + "

" + tr("Are you sure you wish to encrypt your wallet?"), + QMessageBox::Cancel | QMessageBox::Yes, this); + msgBoxConfirm.button(QMessageBox::Yes)->setText(tr("Continue")); + msgBoxConfirm.button(QMessageBox::Cancel)->setText(tr("Back")); + msgBoxConfirm.setDefaultButton(QMessageBox::Cancel); + QMessageBox::StandardButton retval = (QMessageBox::StandardButton)msgBoxConfirm.exec(); if(retval == QMessageBox::Yes) { if(newpass1 == newpass2) @@ -116,10 +120,19 @@ void AskPassphraseDialog::accept() "your bitcoins from being stolen by malware infecting your computer."); if (m_passphrase_out) { m_passphrase_out->assign(newpass1); - QMessageBox::warning(this, tr("Wallet to be encrypted"), - "" + - tr("Your wallet is about to be encrypted. ") + encryption_reminder + - ""); + QMessageBox msgBoxWarning(QMessageBox::Warning, + tr("Wallet to be encrypted"), + "" + + tr("Your wallet is about to be encrypted. ") + encryption_reminder + " " + + tr("Are you sure you wish to encrypt your wallet?") + + "", + QMessageBox::Cancel | QMessageBox::Yes, this); + msgBoxWarning.setDefaultButton(QMessageBox::Cancel); + QMessageBox::StandardButton retval = (QMessageBox::StandardButton)msgBoxWarning.exec(); + if (retval == QMessageBox::Cancel) { + QDialog::reject(); + return; + } } else { assert(model != nullptr); if (model->setWalletEncrypted(newpass1)) { @@ -145,11 +158,7 @@ void AskPassphraseDialog::accept() tr("The supplied passphrases do not match.")); } } - else - { - QDialog::reject(); // Cancelled - } - } break; + } break; case Unlock: try { if (!model->setWalletLocked(false, oldpass)) { From 992b1bbd5da95ee782515fb0f5674bb7a02684b2 Mon Sep 17 00:00:00 2001 From: Jadi Date: Wed, 14 Feb 2024 17:26:19 +0330 Subject: [PATCH 011/868] qt: keep focus on "Hide" while ModalOverlay is visible During the initial sync, the Tab moves the focus to the widgets of the main window, even when the ModalOverlay is visible. This creates some weird rectangular *selections on the screen*. This PR fixes this by keeping the focus on the "Hide" button while the ModalOverlay is visible. Fixes #783 --- src/qt/modaloverlay.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/qt/modaloverlay.cpp b/src/qt/modaloverlay.cpp index b09e230bce76b..1591baea3e96d 100644 --- a/src/qt/modaloverlay.cpp +++ b/src/qt/modaloverlay.cpp @@ -23,6 +23,7 @@ ModalOverlay::ModalOverlay(bool enable_wallet, QWidget* parent) parent->installEventFilter(this); raise(); } + ui->closeButton->installEventFilter(this); blockProcessTime.clear(); setVisible(false); @@ -58,6 +59,11 @@ bool ModalOverlay::eventFilter(QObject * obj, QEvent * ev) { raise(); } } + + if (obj == ui->closeButton && ev->type() == QEvent::FocusOut && layerIsVisible) { + ui->closeButton->setFocus(Qt::OtherFocusReason); + } + return QWidget::eventFilter(obj, ev); } @@ -185,6 +191,10 @@ void ModalOverlay::showHide(bool hide, bool userRequested) m_animation.setEndValue(QPoint(0, hide ? height() : 0)); m_animation.start(QAbstractAnimation::KeepWhenStopped); layerIsVisible = !hide; + + if (layerIsVisible) { + ui->closeButton->setFocus(Qt::OtherFocusReason); + } } void ModalOverlay::closeClicked() From 84502b755bcc35413ad466047893b5edf134c53f Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Wed, 21 Feb 2024 07:07:50 -0500 Subject: [PATCH 012/868] serialization: Drop references to GetVersion/GetType Drop references to stream GetVersion()/GetType() methods which no longer exist --- src/serialize.h | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/serialize.h b/src/serialize.h index 44e2db7ac7b66..08d9a80f96706 100644 --- a/src/serialize.h +++ b/src/serialize.h @@ -1103,12 +1103,12 @@ size_t GetSerializeSize(const T& t) return (SizeComputer() << t).size(); } -/** Wrapper that overrides the GetParams() function of a stream (and hides GetVersion/GetType). */ +/** Wrapper that overrides the GetParams() function of a stream. */ template class ParamsStream { const Params& m_params; - SubStream& m_substream; // private to avoid leaking version/type into serialization code that shouldn't see it + SubStream& m_substream; public: ParamsStream(const Params& params LIFETIMEBOUND, SubStream& substream LIFETIMEBOUND) : m_params{params}, m_substream{substream} {} @@ -1119,8 +1119,6 @@ class ParamsStream void ignore(size_t num) { m_substream.ignore(num); } bool eof() const { return m_substream.eof(); } size_t size() const { return m_substream.size(); } - int GetVersion() = delete; // Deprecated with Params usage - int GetType() = delete; // Deprecated with Params usage //! Get reference to stream parameters. template From 83436d14f06551026bcf5529df3b63b4e8a679fb Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Wed, 22 Nov 2023 16:39:32 -0500 Subject: [PATCH 013/868] serialization: Drop unnecessary ParamsStream references Drop unnecessary ParamsStream references from CTransaction and CMutableTransaction constructors. This just couples these classes unnecessarily to the ParamsStream class, making the ParamsStream class harder to modify, and making the transaction classes in some cases (depending on parameter order) unable to work with stream classes that have multiple parameters set. --- src/primitives/transaction.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/primitives/transaction.h b/src/primitives/transaction.h index d15b8005f90e0..976542cfae676 100644 --- a/src/primitives/transaction.h +++ b/src/primitives/transaction.h @@ -334,7 +334,7 @@ class CTransaction template CTransaction(deserialize_type, const TransactionSerParams& params, Stream& s) : CTransaction(CMutableTransaction(deserialize, params, s)) {} template - CTransaction(deserialize_type, ParamsStream& s) : CTransaction(CMutableTransaction(deserialize, s)) {} + CTransaction(deserialize_type, Stream& s) : CTransaction(CMutableTransaction(deserialize, s)) {} bool IsNull() const { return vin.empty() && vout.empty(); @@ -400,7 +400,7 @@ struct CMutableTransaction } template - CMutableTransaction(deserialize_type, ParamsStream& s) { + CMutableTransaction(deserialize_type, Stream& s) { Unserialize(s); } From cb28849a88339c1e7ba03ffc7e38998339074e6e Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Wed, 22 Nov 2023 16:39:32 -0500 Subject: [PATCH 014/868] serialization: Reverse ParamsStream constructor order Move parameter argument after stream argument so will be possible to accept multiple variadic parameter arguments in the following commit. Also reverse template parameter order for consistency. --- src/addrman.cpp | 4 ++-- src/net.cpp | 2 +- src/serialize.h | 8 ++++---- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/addrman.cpp b/src/addrman.cpp index a8206de6ee291..b649950eced3a 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -171,7 +171,7 @@ void AddrManImpl::Serialize(Stream& s_) const */ // Always serialize in the latest version (FILE_FORMAT). - ParamsStream s{CAddress::V2_DISK, s_}; + ParamsStream s{s_, CAddress::V2_DISK}; s << static_cast(FILE_FORMAT); @@ -236,7 +236,7 @@ void AddrManImpl::Unserialize(Stream& s_) s_ >> Using>(format); const auto ser_params = (format >= Format::V3_BIP155 ? CAddress::V2_DISK : CAddress::V1_DISK); - ParamsStream s{ser_params, s_}; + ParamsStream s{s_, ser_params}; uint8_t compat; s >> compat; diff --git a/src/net.cpp b/src/net.cpp index 0cc794c2c3fcf..ba764e28e195c 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -199,7 +199,7 @@ static std::vector ConvertSeeds(const std::vector &vSeedsIn) std::vector vSeedsOut; FastRandomContext rng; DataStream underlying_stream{vSeedsIn}; - ParamsStream s{CAddress::V2_NETWORK, underlying_stream}; + ParamsStream s{underlying_stream, CAddress::V2_NETWORK}; while (!s.eof()) { CService endpoint; s >> endpoint; diff --git a/src/serialize.h b/src/serialize.h index 08d9a80f96706..8ad5ad5147b2f 100644 --- a/src/serialize.h +++ b/src/serialize.h @@ -1104,14 +1104,14 @@ size_t GetSerializeSize(const T& t) } /** Wrapper that overrides the GetParams() function of a stream. */ -template +template class ParamsStream { const Params& m_params; SubStream& m_substream; public: - ParamsStream(const Params& params LIFETIMEBOUND, SubStream& substream LIFETIMEBOUND) : m_params{params}, m_substream{substream} {} + ParamsStream(SubStream& substream LIFETIMEBOUND, const Params& params LIFETIMEBOUND) : m_params{params}, m_substream{substream} {} template ParamsStream& operator<<(const U& obj) { ::Serialize(*this, obj); return *this; } template ParamsStream& operator>>(U&& obj) { ::Unserialize(*this, obj); return *this; } void write(Span src) { m_substream.write(src); } @@ -1145,13 +1145,13 @@ class ParamsWrapper template void Serialize(Stream& s) const { - ParamsStream ss{m_params, s}; + ParamsStream ss{s, m_params}; ::Serialize(ss, m_object); } template void Unserialize(Stream& s) { - ParamsStream ss{m_params, s}; + ParamsStream ss{s, m_params}; ::Unserialize(ss, m_object); } }; From e6794e475c84d9edca4a2876e2342cbb1d85f804 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Wed, 22 Nov 2023 17:56:04 -0500 Subject: [PATCH 015/868] serialization: Accept multiple parameters in ParamsStream constructor Before this change it was possible but awkward to create ParamStream streams with multiple parameter objects. After this change it is straightforward. The change to support multiple parameters is implemented by letting ParamsStream contain substream instances, instead of just references to external substreams. So a side-effect of this change is that ParamStream can now accept rvalue stream arguments and be easier to use in some other cases. A test for rvalues is added in this commit, and some simplifications to non-test code are made in the next commit. --- src/serialize.h | 32 ++++++++++++++- src/test/serialize_tests.cpp | 79 ++++++++++++++++++++++++++++++++++++ 2 files changed, 109 insertions(+), 2 deletions(-) diff --git a/src/serialize.h b/src/serialize.h index 8ad5ad5147b2f..8bca4d8ada1a2 100644 --- a/src/serialize.h +++ b/src/serialize.h @@ -1108,10 +1108,22 @@ template class ParamsStream { const Params& m_params; - SubStream& m_substream; + // If ParamsStream constructor is passed an lvalue argument, Substream will + // be a reference type, and m_substream will reference that argument. + // Otherwise m_substream will be a substream instance and move from the + // argument. Letting ParamsStream contain a substream instance instead of + // just a reference is useful to make the ParamsStream object self contained + // and let it do cleanup when destroyed, for example by closing files if + // SubStream is a file stream. + SubStream m_substream; public: - ParamsStream(SubStream& substream LIFETIMEBOUND, const Params& params LIFETIMEBOUND) : m_params{params}, m_substream{substream} {} + ParamsStream(SubStream&& substream, const Params& params LIFETIMEBOUND) : m_params{params}, m_substream{std::forward(substream)} {} + + template + ParamsStream(NestedSubstream&& s, const Params1& params1 LIFETIMEBOUND, const Params2& params2 LIFETIMEBOUND, const NestedParams&... params LIFETIMEBOUND) + : ParamsStream{::ParamsStream{std::forward(s), params2, params...}, params1} {} + template ParamsStream& operator<<(const U& obj) { ::Serialize(*this, obj); return *this; } template ParamsStream& operator>>(U&& obj) { ::Unserialize(*this, obj); return *this; } void write(Span src) { m_substream.write(src); } @@ -1132,6 +1144,22 @@ class ParamsStream } }; +/** + * Explicit template deduction guide is required for single-parameter + * constructor so Substream&& is treated as a forwarding reference, and + * SubStream is deduced as reference type for lvalue arguments. + */ +template +ParamsStream(Substream&&, const Params&) -> ParamsStream; + +/** + * Template deduction guide for multiple params arguments that creates a nested + * ParamsStream. + */ +template +ParamsStream(Substream&& s, const Params1& params1, const Params2& params2, const Params&... params) -> + ParamsStream(s), params2, params...}), Params1>; + /** Wrapper that serializes objects with the specified parameters. */ template class ParamsWrapper diff --git a/src/test/serialize_tests.cpp b/src/test/serialize_tests.cpp index 18356f2df03fb..ad4852d13d21a 100644 --- a/src/test/serialize_tests.cpp +++ b/src/test/serialize_tests.cpp @@ -15,6 +15,18 @@ BOOST_FIXTURE_TEST_SUITE(serialize_tests, BasicTestingSetup) +// For testing move-semantics, declare a version of datastream that can be moved +// but is not copyable. +class UncopyableStream : public DataStream +{ +public: + using DataStream::DataStream; + UncopyableStream(const UncopyableStream&) = delete; + UncopyableStream& operator=(const UncopyableStream&) = delete; + UncopyableStream(UncopyableStream&&) noexcept = default; + UncopyableStream& operator=(UncopyableStream&&) noexcept = default; +}; + class CSerializeMethodsTestSingle { protected: @@ -344,6 +356,73 @@ class Derived : public Base } }; +struct OtherParam { + uint8_t param; + SER_PARAMS_OPFUNC +}; + +//! Checker for value of OtherParam. When being serialized, serializes the +//! param to the stream. When being unserialized, verifies the value in the +//! stream matches the param. +class OtherParamChecker +{ +public: + template + void Serialize(Stream& s) const + { + const uint8_t param = s.template GetParams().param; + s << param; + } + + template + void Unserialize(Stream& s) const + { + const uint8_t param = s.template GetParams().param; + uint8_t value; + s >> value; + BOOST_CHECK_EQUAL(value, param); + } +}; + +//! Test creating a stream with multiple parameters and making sure +//! serialization code requiring different parameters can retrieve them. Also +//! test that earlier parameters take precedence if the same parameter type is +//! specified twice. (Choice of whether earlier or later values take precedence +//! or multiple values of the same type are allowed was arbitrary, and just +//! decided based on what would require smallest amount of ugly C++ template +//! code. Intent of the test is to just ensure there is no unexpected behavior.) +BOOST_AUTO_TEST_CASE(with_params_multi) +{ + const OtherParam other_param_used{.param = 0x10}; + const OtherParam other_param_ignored{.param = 0x11}; + const OtherParam other_param_override{.param = 0x12}; + const OtherParamChecker check; + DataStream stream; + ParamsStream pstream{stream, RAW, other_param_used, other_param_ignored}; + + Base base1{0x20}; + pstream << base1 << check << other_param_override(check); + BOOST_CHECK_EQUAL(stream.str(), "\x20\x10\x12"); + + Base base2; + pstream >> base2 >> check >> other_param_override(check); + BOOST_CHECK_EQUAL(base2.m_base_data, 0x20); +} + +//! Test creating a ParamsStream that moves from a stream argument. +BOOST_AUTO_TEST_CASE(with_params_move) +{ + UncopyableStream stream{}; + ParamsStream pstream{std::move(stream), RAW, HEX, RAW}; + + Base base1{0x20}; + pstream << base1; + + Base base2; + pstream >> base2; + BOOST_CHECK_EQUAL(base2.m_base_data, 0x20); +} + BOOST_AUTO_TEST_CASE(with_params_base) { Base b{0x0F}; From 951203bcc496c4415b7754cd764544659b76067f Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Tue, 20 Feb 2024 17:45:47 -0500 Subject: [PATCH 016/868] net: Simplify ParamsStream usage Simplify ParamsStream usage in ConvertSeeds now that ParamsStream supports rvalue substream arguments. --- src/net.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index ba764e28e195c..8bb1558ee9c0e 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -198,8 +198,7 @@ static std::vector ConvertSeeds(const std::vector &vSeedsIn) const auto one_week{7 * 24h}; std::vector vSeedsOut; FastRandomContext rng; - DataStream underlying_stream{vSeedsIn}; - ParamsStream s{underlying_stream, CAddress::V2_NETWORK}; + ParamsStream s{DataStream{vSeedsIn}, CAddress::V2_NETWORK}; while (!s.eof()) { CService endpoint; s >> endpoint; From 8d491ae9ecf1948ea29f67b50ca7259123f602aa Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Wed, 21 Feb 2024 07:35:38 -0500 Subject: [PATCH 017/868] serialization: Add ParamsStream GetStream() method Add GetStream() method useful for accessing underlying stream. Use to improve ParamsStream test coverage. --- src/serialize.h | 32 +++++++++++++++++++++++++++----- src/test/serialize_tests.cpp | 5 ++++- 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/src/serialize.h b/src/serialize.h index 8bca4d8ada1a2..d3eb1acb1ffce 100644 --- a/src/serialize.h +++ b/src/serialize.h @@ -1103,6 +1103,10 @@ size_t GetSerializeSize(const T& t) return (SizeComputer() << t).size(); } +//! Check if type contains a stream by seeing if has a GetStream() method. +template +concept ContainsStream = requires(T t) { t.GetStream(); }; + /** Wrapper that overrides the GetParams() function of a stream. */ template class ParamsStream @@ -1126,11 +1130,11 @@ class ParamsStream template ParamsStream& operator<<(const U& obj) { ::Serialize(*this, obj); return *this; } template ParamsStream& operator>>(U&& obj) { ::Unserialize(*this, obj); return *this; } - void write(Span src) { m_substream.write(src); } - void read(Span dst) { m_substream.read(dst); } - void ignore(size_t num) { m_substream.ignore(num); } - bool eof() const { return m_substream.eof(); } - size_t size() const { return m_substream.size(); } + void write(Span src) { GetStream().write(src); } + void read(Span dst) { GetStream().read(dst); } + void ignore(size_t num) { GetStream().ignore(num); } + bool eof() const { return GetStream().eof(); } + size_t size() const { return GetStream().size(); } //! Get reference to stream parameters. template @@ -1142,6 +1146,24 @@ class ParamsStream return m_substream.template GetParams

(); } } + + //! Get reference to underlying stream. + auto& GetStream() + { + if constexpr (ContainsStream) { + return m_substream.GetStream(); + } else { + return m_substream; + } + } + const auto& GetStream() const + { + if constexpr (ContainsStream) { + return m_substream.GetStream(); + } else { + return m_substream; + } + } }; /** diff --git a/src/test/serialize_tests.cpp b/src/test/serialize_tests.cpp index ad4852d13d21a..b28e1b41966b6 100644 --- a/src/test/serialize_tests.cpp +++ b/src/test/serialize_tests.cpp @@ -412,11 +412,14 @@ BOOST_AUTO_TEST_CASE(with_params_multi) //! Test creating a ParamsStream that moves from a stream argument. BOOST_AUTO_TEST_CASE(with_params_move) { - UncopyableStream stream{}; + UncopyableStream stream{MakeByteSpan(std::string_view{"abc"})}; ParamsStream pstream{std::move(stream), RAW, HEX, RAW}; + BOOST_CHECK_EQUAL(pstream.GetStream().str(), "abc"); + pstream.GetStream().clear(); Base base1{0x20}; pstream << base1; + BOOST_CHECK_EQUAL(pstream.GetStream().str(), "\x20"); Base base2; pstream >> base2; From 2fa9de06c2c8583ee8e2434dc97014b26e218ab5 Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Thu, 16 Nov 2023 12:34:27 +0100 Subject: [PATCH 018/868] net: make the list of known message types a compile time constant Turn the `std::vector` to `std::array` because it is cheaper and allows us to have the number of the messages as a compile time constant: `ALL_NET_MESSAGE_TYPES.size()` which can be used in future code to build other `std::array`s with that size. --- src/net.cpp | 3 +- src/protocol.cpp | 46 ------------------- src/protocol.h | 42 +++++++++++++++-- src/test/fuzz/p2p_transport_serialization.cpp | 3 +- src/test/fuzz/process_message.cpp | 2 +- test/fuzz/test_runner.py | 4 +- 6 files changed, 45 insertions(+), 55 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 7c82f01d75730..74f1eea4855c6 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -3701,8 +3701,9 @@ CNode::CNode(NodeId idIn, { if (inbound_onion) assert(conn_type_in == ConnectionType::INBOUND); - for (const std::string &msg : getAllNetMessageTypes()) + for (const auto& msg : ALL_NET_MESSAGE_TYPES) { mapRecvBytesPerMsgType[msg] = 0; + } mapRecvBytesPerMsgType[NET_MESSAGE_TYPE_OTHER] = 0; if (fLogIPs) { diff --git a/src/protocol.cpp b/src/protocol.cpp index 0da160768d1ca..b85571315ad5c 100644 --- a/src/protocol.cpp +++ b/src/protocol.cpp @@ -47,47 +47,6 @@ const char* WTXIDRELAY = "wtxidrelay"; const char* SENDTXRCNCL = "sendtxrcncl"; } // namespace NetMsgType -/** All known message types. Keep this in the same order as the list of - * messages above and in protocol.h. - */ -const static std::vector g_all_net_message_types{ - NetMsgType::VERSION, - NetMsgType::VERACK, - NetMsgType::ADDR, - NetMsgType::ADDRV2, - NetMsgType::SENDADDRV2, - NetMsgType::INV, - NetMsgType::GETDATA, - NetMsgType::MERKLEBLOCK, - NetMsgType::GETBLOCKS, - NetMsgType::GETHEADERS, - NetMsgType::TX, - NetMsgType::HEADERS, - NetMsgType::BLOCK, - NetMsgType::GETADDR, - NetMsgType::MEMPOOL, - NetMsgType::PING, - NetMsgType::PONG, - NetMsgType::NOTFOUND, - NetMsgType::FILTERLOAD, - NetMsgType::FILTERADD, - NetMsgType::FILTERCLEAR, - NetMsgType::SENDHEADERS, - NetMsgType::FEEFILTER, - NetMsgType::SENDCMPCT, - NetMsgType::CMPCTBLOCK, - NetMsgType::GETBLOCKTXN, - NetMsgType::BLOCKTXN, - NetMsgType::GETCFILTERS, - NetMsgType::CFILTER, - NetMsgType::GETCFHEADERS, - NetMsgType::CFHEADERS, - NetMsgType::GETCFCHECKPT, - NetMsgType::CFCHECKPT, - NetMsgType::WTXIDRELAY, - NetMsgType::SENDTXRCNCL, -}; - CMessageHeader::CMessageHeader(const MessageStartChars& pchMessageStartIn, const char* pszCommand, unsigned int nMessageSizeIn) : pchMessageStart{pchMessageStartIn} { @@ -164,11 +123,6 @@ std::string CInv::ToString() const } } -const std::vector &getAllNetMessageTypes() -{ - return g_all_net_message_types; -} - /** * Convert a service flag (NODE_*) to a human readable string. * It supports unknown service flags which will be returned as "UNKNOWN[...]". diff --git a/src/protocol.h b/src/protocol.h index 243cd23e6e6d9..350e1d986621e 100644 --- a/src/protocol.h +++ b/src/protocol.h @@ -55,7 +55,7 @@ class CMessageHeader /** * Bitcoin protocol message types. When adding new message types, don't forget - * to update allNetMessageTypes in protocol.cpp. + * to update ALL_NET_MESSAGE_TYPES below. */ namespace NetMsgType { @@ -267,8 +267,44 @@ extern const char* WTXIDRELAY; extern const char* SENDTXRCNCL; }; // namespace NetMsgType -/* Get a vector of all valid message types (see above) */ -const std::vector& getAllNetMessageTypes(); +/** All known message types (see above). Keep this in the same order as the list of messages above. */ +inline const std::array ALL_NET_MESSAGE_TYPES{std::to_array({ + NetMsgType::VERSION, + NetMsgType::VERACK, + NetMsgType::ADDR, + NetMsgType::ADDRV2, + NetMsgType::SENDADDRV2, + NetMsgType::INV, + NetMsgType::GETDATA, + NetMsgType::MERKLEBLOCK, + NetMsgType::GETBLOCKS, + NetMsgType::GETHEADERS, + NetMsgType::TX, + NetMsgType::HEADERS, + NetMsgType::BLOCK, + NetMsgType::GETADDR, + NetMsgType::MEMPOOL, + NetMsgType::PING, + NetMsgType::PONG, + NetMsgType::NOTFOUND, + NetMsgType::FILTERLOAD, + NetMsgType::FILTERADD, + NetMsgType::FILTERCLEAR, + NetMsgType::SENDHEADERS, + NetMsgType::FEEFILTER, + NetMsgType::SENDCMPCT, + NetMsgType::CMPCTBLOCK, + NetMsgType::GETBLOCKTXN, + NetMsgType::BLOCKTXN, + NetMsgType::GETCFILTERS, + NetMsgType::CFILTER, + NetMsgType::GETCFHEADERS, + NetMsgType::CFHEADERS, + NetMsgType::GETCFCHECKPT, + NetMsgType::CFCHECKPT, + NetMsgType::WTXIDRELAY, + NetMsgType::SENDTXRCNCL, +})}; /** nServices flags */ enum ServiceFlags : uint64_t { diff --git a/src/test/fuzz/p2p_transport_serialization.cpp b/src/test/fuzz/p2p_transport_serialization.cpp index a205ce19f4a3a..fee218a054e51 100644 --- a/src/test/fuzz/p2p_transport_serialization.cpp +++ b/src/test/fuzz/p2p_transport_serialization.cpp @@ -21,13 +21,12 @@ namespace { -std::vector g_all_messages; +auto g_all_messages = ALL_NET_MESSAGE_TYPES; void initialize_p2p_transport_serialization() { ECC_Start(); SelectParams(ChainType::REGTEST); - g_all_messages = getAllNetMessageTypes(); std::sort(g_all_messages.begin(), g_all_messages.end()); } diff --git a/src/test/fuzz/process_message.cpp b/src/test/fuzz/process_message.cpp index 56b391ed5c884..0203b62535466 100644 --- a/src/test/fuzz/process_message.cpp +++ b/src/test/fuzz/process_message.cpp @@ -37,7 +37,7 @@ void initialize_process_message() { if (const auto val{std::getenv("LIMIT_TO_MESSAGE_TYPE")}) { LIMIT_TO_MESSAGE_TYPE = val; - Assert(std::count(getAllNetMessageTypes().begin(), getAllNetMessageTypes().end(), LIMIT_TO_MESSAGE_TYPE)); // Unknown message type passed + Assert(std::count(ALL_NET_MESSAGE_TYPES.begin(), ALL_NET_MESSAGE_TYPES.end(), LIMIT_TO_MESSAGE_TYPE)); // Unknown message type passed } static const auto testing_setup = MakeNoLogFileContext( diff --git a/test/fuzz/test_runner.py b/test/fuzz/test_runner.py index ff3b6e6b6dde3..10caaa456e39a 100755 --- a/test/fuzz/test_runner.py +++ b/test/fuzz/test_runner.py @@ -205,12 +205,12 @@ def transform_process_message_target(targets, src_dir): p2p_msg_target = "process_message" if (p2p_msg_target, {}) in targets: lines = subprocess.run( - ["git", "grep", "--function-context", "g_all_net_message_types{", src_dir / "src" / "protocol.cpp"], + ["git", "grep", "--function-context", "ALL_NET_MESSAGE_TYPES{", src_dir / "src" / "protocol.h"], check=True, stdout=subprocess.PIPE, text=True, ).stdout.splitlines() - lines = [l.split("::", 1)[1].split(",")[0].lower() for l in lines if l.startswith("src/protocol.cpp- NetMsgType::")] + lines = [l.split("::", 1)[1].split(",")[0].lower() for l in lines if l.startswith("src/protocol.h- NetMsgType::")] assert len(lines) targets += [(p2p_msg_target, {"LIMIT_TO_MESSAGE_TYPE": m}) for m in lines] return targets From b3efb486732f3caf8b8a8e9d744e6d20ae4255ef Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Thu, 15 Feb 2024 15:32:28 +0100 Subject: [PATCH 019/868] protocol: make message types constexpr As a side effect the names of the constants do not have to be repeated in `src/protocol.cpp`. --- src/protocol.cpp | 40 --------------------------- src/protocol.h | 71 ++++++++++++++++++++++++------------------------ 2 files changed, 35 insertions(+), 76 deletions(-) diff --git a/src/protocol.cpp b/src/protocol.cpp index b85571315ad5c..0439d398c742e 100644 --- a/src/protocol.cpp +++ b/src/protocol.cpp @@ -7,46 +7,6 @@ #include -#include - -namespace NetMsgType { -const char* VERSION = "version"; -const char* VERACK = "verack"; -const char* ADDR = "addr"; -const char* ADDRV2 = "addrv2"; -const char* SENDADDRV2 = "sendaddrv2"; -const char* INV = "inv"; -const char* GETDATA = "getdata"; -const char* MERKLEBLOCK = "merkleblock"; -const char* GETBLOCKS = "getblocks"; -const char* GETHEADERS = "getheaders"; -const char* TX = "tx"; -const char* HEADERS = "headers"; -const char* BLOCK = "block"; -const char* GETADDR = "getaddr"; -const char* MEMPOOL = "mempool"; -const char* PING = "ping"; -const char* PONG = "pong"; -const char* NOTFOUND = "notfound"; -const char* FILTERLOAD = "filterload"; -const char* FILTERADD = "filteradd"; -const char* FILTERCLEAR = "filterclear"; -const char* SENDHEADERS = "sendheaders"; -const char* FEEFILTER = "feefilter"; -const char* SENDCMPCT = "sendcmpct"; -const char* CMPCTBLOCK = "cmpctblock"; -const char* GETBLOCKTXN = "getblocktxn"; -const char* BLOCKTXN = "blocktxn"; -const char* GETCFILTERS = "getcfilters"; -const char* CFILTER = "cfilter"; -const char* GETCFHEADERS = "getcfheaders"; -const char* CFHEADERS = "cfheaders"; -const char* GETCFCHECKPT = "getcfcheckpt"; -const char* CFCHECKPT = "cfcheckpt"; -const char* WTXIDRELAY = "wtxidrelay"; -const char* SENDTXRCNCL = "sendtxrcncl"; -} // namespace NetMsgType - CMessageHeader::CMessageHeader(const MessageStartChars& pchMessageStartIn, const char* pszCommand, unsigned int nMessageSizeIn) : pchMessageStart{pchMessageStartIn} { diff --git a/src/protocol.h b/src/protocol.h index 350e1d986621e..da2216bc1867a 100644 --- a/src/protocol.h +++ b/src/protocol.h @@ -58,103 +58,102 @@ class CMessageHeader * to update ALL_NET_MESSAGE_TYPES below. */ namespace NetMsgType { - /** * The version message provides information about the transmitting node to the * receiving node at the beginning of a connection. */ -extern const char* VERSION; +inline constexpr const char* VERSION{"version"}; /** * The verack message acknowledges a previously-received version message, * informing the connecting node that it can begin to send other messages. */ -extern const char* VERACK; +inline constexpr const char* VERACK{"verack"}; /** * The addr (IP address) message relays connection information for peers on the * network. */ -extern const char* ADDR; +inline constexpr const char* ADDR{"addr"}; /** * The addrv2 message relays connection information for peers on the network just * like the addr message, but is extended to allow gossiping of longer node * addresses (see BIP155). */ -extern const char *ADDRV2; +inline constexpr const char* ADDRV2{"addrv2"}; /** * The sendaddrv2 message signals support for receiving ADDRV2 messages (BIP155). * It also implies that its sender can encode as ADDRV2 and would send ADDRV2 * instead of ADDR to a peer that has signaled ADDRV2 support by sending SENDADDRV2. */ -extern const char *SENDADDRV2; +inline constexpr const char* SENDADDRV2{"sendaddrv2"}; /** * The inv message (inventory message) transmits one or more inventories of * objects known to the transmitting peer. */ -extern const char* INV; +inline constexpr const char* INV{"inv"}; /** * The getdata message requests one or more data objects from another node. */ -extern const char* GETDATA; +inline constexpr const char* GETDATA{"getdata"}; /** * The merkleblock message is a reply to a getdata message which requested a * block using the inventory type MSG_MERKLEBLOCK. * @since protocol version 70001 as described by BIP37. */ -extern const char* MERKLEBLOCK; +inline constexpr const char* MERKLEBLOCK{"merkleblock"}; /** * The getblocks message requests an inv message that provides block header * hashes starting from a particular point in the block chain. */ -extern const char* GETBLOCKS; +inline constexpr const char* GETBLOCKS{"getblocks"}; /** * The getheaders message requests a headers message that provides block * headers starting from a particular point in the block chain. * @since protocol version 31800. */ -extern const char* GETHEADERS; +inline constexpr const char* GETHEADERS{"getheaders"}; /** * The tx message transmits a single transaction. */ -extern const char* TX; +inline constexpr const char* TX{"tx"}; /** * The headers message sends one or more block headers to a node which * previously requested certain headers with a getheaders message. * @since protocol version 31800. */ -extern const char* HEADERS; +inline constexpr const char* HEADERS{"headers"}; /** * The block message transmits a single serialized block. */ -extern const char* BLOCK; +inline constexpr const char* BLOCK{"block"}; /** * The getaddr message requests an addr message from the receiving node, * preferably one with lots of IP addresses of other receiving nodes. */ -extern const char* GETADDR; +inline constexpr const char* GETADDR{"getaddr"}; /** * The mempool message requests the TXIDs of transactions that the receiving * node has verified as valid but which have not yet appeared in a block. * @since protocol version 60002 as described by BIP35. * Only available with service bit NODE_BLOOM, see also BIP111. */ -extern const char* MEMPOOL; +inline constexpr const char* MEMPOOL{"mempool"}; /** * The ping message is sent periodically to help confirm that the receiving * peer is still connected. */ -extern const char* PING; +inline constexpr const char* PING{"ping"}; /** * The pong message replies to a ping message, proving to the pinging node that * the ponging node is still alive. * @since protocol version 60001 as described by BIP31. */ -extern const char* PONG; +inline constexpr const char* PONG{"pong"}; /** * The notfound message is a reply to a getdata message which requested an * object the receiving node does not have available for relay. * @since protocol version 70001. */ -extern const char* NOTFOUND; +inline constexpr const char* NOTFOUND{"notfound"}; /** * The filterload message tells the receiving peer to filter all relayed * transactions and requested merkle blocks through the provided filter. @@ -162,7 +161,7 @@ extern const char* NOTFOUND; * Only available with service bit NODE_BLOOM since protocol version * 70011 as described by BIP111. */ -extern const char* FILTERLOAD; +inline constexpr const char* FILTERLOAD{"filterload"}; /** * The filteradd message tells the receiving peer to add a single element to a * previously-set bloom filter, such as a new public key. @@ -170,7 +169,7 @@ extern const char* FILTERLOAD; * Only available with service bit NODE_BLOOM since protocol version * 70011 as described by BIP111. */ -extern const char* FILTERADD; +inline constexpr const char* FILTERADD{"filteradd"}; /** * The filterclear message tells the receiving peer to remove a previously-set * bloom filter. @@ -178,19 +177,19 @@ extern const char* FILTERADD; * Only available with service bit NODE_BLOOM since protocol version * 70011 as described by BIP111. */ -extern const char* FILTERCLEAR; +inline constexpr const char* FILTERCLEAR{"filterclear"}; /** * Indicates that a node prefers to receive new block announcements via a * "headers" message rather than an "inv". * @since protocol version 70012 as described by BIP130. */ -extern const char* SENDHEADERS; +inline constexpr const char* SENDHEADERS{"sendheaders"}; /** * The feefilter message tells the receiving peer not to inv us any txs * which do not meet the specified min fee rate. * @since protocol version 70013 as described by BIP133 */ -extern const char* FEEFILTER; +inline constexpr const char* FEEFILTER{"feefilter"}; /** * Contains a 1-byte bool and 8-byte LE version number. * Indicates that a node is willing to provide blocks via "cmpctblock" messages. @@ -198,36 +197,36 @@ extern const char* FEEFILTER; * "cmpctblock" message rather than an "inv", depending on message contents. * @since protocol version 70014 as described by BIP 152 */ -extern const char* SENDCMPCT; +inline constexpr const char* SENDCMPCT{"sendcmpct"}; /** * Contains a CBlockHeaderAndShortTxIDs object - providing a header and * list of "short txids". * @since protocol version 70014 as described by BIP 152 */ -extern const char* CMPCTBLOCK; +inline constexpr const char* CMPCTBLOCK{"cmpctblock"}; /** * Contains a BlockTransactionsRequest * Peer should respond with "blocktxn" message. * @since protocol version 70014 as described by BIP 152 */ -extern const char* GETBLOCKTXN; +inline constexpr const char* GETBLOCKTXN{"getblocktxn"}; /** * Contains a BlockTransactions. * Sent in response to a "getblocktxn" message. * @since protocol version 70014 as described by BIP 152 */ -extern const char* BLOCKTXN; +inline constexpr const char* BLOCKTXN{"blocktxn"}; /** * getcfilters requests compact filters for a range of blocks. * Only available with service bit NODE_COMPACT_FILTERS as described by * BIP 157 & 158. */ -extern const char* GETCFILTERS; +inline constexpr const char* GETCFILTERS{"getcfilters"}; /** * cfilter is a response to a getcfilters request containing a single compact * filter. */ -extern const char* CFILTER; +inline constexpr const char* CFILTER{"cfilter"}; /** * getcfheaders requests a compact filter header and the filter hashes for a * range of blocks, which can then be used to reconstruct the filter headers @@ -235,36 +234,36 @@ extern const char* CFILTER; * Only available with service bit NODE_COMPACT_FILTERS as described by * BIP 157 & 158. */ -extern const char* GETCFHEADERS; +inline constexpr const char* GETCFHEADERS{"getcfheaders"}; /** * cfheaders is a response to a getcfheaders request containing a filter header * and a vector of filter hashes for each subsequent block in the requested range. */ -extern const char* CFHEADERS; +inline constexpr const char* CFHEADERS{"cfheaders"}; /** * getcfcheckpt requests evenly spaced compact filter headers, enabling * parallelized download and validation of the headers between them. * Only available with service bit NODE_COMPACT_FILTERS as described by * BIP 157 & 158. */ -extern const char* GETCFCHECKPT; +inline constexpr const char* GETCFCHECKPT{"getcfcheckpt"}; /** * cfcheckpt is a response to a getcfcheckpt request containing a vector of * evenly spaced filter headers for blocks on the requested chain. */ -extern const char* CFCHECKPT; +inline constexpr const char* CFCHECKPT{"cfcheckpt"}; /** * Indicates that a node prefers to relay transactions via wtxid, rather than * txid. * @since protocol version 70016 as described by BIP 339. */ -extern const char* WTXIDRELAY; +inline constexpr const char* WTXIDRELAY{"wtxidrelay"}; /** * Contains a 4-byte version number and an 8-byte salt. * The salt is used to compute short txids needed for efficient * txreconciliation, as described by BIP 330. */ -extern const char* SENDTXRCNCL; +inline constexpr const char* SENDTXRCNCL{"sendtxrcncl"}; }; // namespace NetMsgType /** All known message types (see above). Keep this in the same order as the list of messages above. */ From 6ec1ca7c85a4009b77e149a798a331592b96ea42 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Mon, 12 Feb 2024 14:26:00 +0000 Subject: [PATCH 020/868] build: Fix test for SSE4.1 intrinsics This change uses the `_mm_blend_epi16` SSE4.1 function used in our code and fixes false-positive cases, for example, when CXXFLAGS="-mno-sse4.1" provided. --- configure.ac | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/configure.ac b/configure.ac index 4f715158731fd..2c73e0920c922 100644 --- a/configure.ac +++ b/configure.ac @@ -504,11 +504,12 @@ TEMP_CXXFLAGS="$CXXFLAGS" CXXFLAGS="$SSE41_CXXFLAGS $CXXFLAGS" AC_MSG_CHECKING([for SSE4.1 intrinsics]) AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[ - #include #include ]],[[ - __m128i l = _mm_set1_epi32(0); - return _mm_extract_epi32(l, 3); + __m128i a = _mm_set1_epi32(0); + __m128i b = _mm_set1_epi32(1); + __m128i r = _mm_blend_epi16(a, b, 0xFF); + return _mm_extract_epi32(r, 3); ]])], [ AC_MSG_RESULT([yes]); enable_sse41=yes; AC_DEFINE([ENABLE_SSE41], [1], [Define this symbol to build code that uses SSE4.1 intrinsics]) ], [ AC_MSG_RESULT([no])] From d440f13db02c82c842000abe4fe4d0c721a4ad3b Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Mon, 12 Feb 2024 14:26:16 +0000 Subject: [PATCH 021/868] crypto: Guard code with `ENABLE_SSE41` macro The code in `sha_x86_shani.cpp` uses the `_mm_blend_epi16` function from the SSE4.1 instruction set. However, it is possible that SHA-NI is enabled even when SSE4.1 is disabled. This changes avoid compilation errors in such a condition. --- src/Makefile.am | 10 +++++----- src/crypto/sha256.cpp | 2 +- src/crypto/sha256_x86_shani.cpp | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index b5d5c4652ab0c..9c1d890f5e59c 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -53,15 +53,15 @@ LIBBITCOIN_CRYPTO = $(LIBBITCOIN_CRYPTO_BASE) if ENABLE_SSE41 LIBBITCOIN_CRYPTO_SSE41 = crypto/libbitcoin_crypto_sse41.la LIBBITCOIN_CRYPTO += $(LIBBITCOIN_CRYPTO_SSE41) +if ENABLE_X86_SHANI +LIBBITCOIN_CRYPTO_X86_SHANI = crypto/libbitcoin_crypto_x86_shani.la +LIBBITCOIN_CRYPTO += $(LIBBITCOIN_CRYPTO_X86_SHANI) +endif endif if ENABLE_AVX2 LIBBITCOIN_CRYPTO_AVX2 = crypto/libbitcoin_crypto_avx2.la LIBBITCOIN_CRYPTO += $(LIBBITCOIN_CRYPTO_AVX2) endif -if ENABLE_X86_SHANI -LIBBITCOIN_CRYPTO_X86_SHANI = crypto/libbitcoin_crypto_x86_shani.la -LIBBITCOIN_CRYPTO += $(LIBBITCOIN_CRYPTO_X86_SHANI) -endif if ENABLE_ARM_SHANI LIBBITCOIN_CRYPTO_ARM_SHANI = crypto/libbitcoin_crypto_arm_shani.la LIBBITCOIN_CRYPTO += $(LIBBITCOIN_CRYPTO_ARM_SHANI) @@ -612,7 +612,7 @@ crypto_libbitcoin_crypto_x86_shani_la_LDFLAGS = $(AM_LDFLAGS) -static crypto_libbitcoin_crypto_x86_shani_la_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) -static crypto_libbitcoin_crypto_x86_shani_la_CPPFLAGS = $(AM_CPPFLAGS) crypto_libbitcoin_crypto_x86_shani_la_CXXFLAGS += $(X86_SHANI_CXXFLAGS) -crypto_libbitcoin_crypto_x86_shani_la_CPPFLAGS += -DENABLE_X86_SHANI +crypto_libbitcoin_crypto_x86_shani_la_CPPFLAGS += -DENABLE_SSE41 -DENABLE_X86_SHANI crypto_libbitcoin_crypto_x86_shani_la_SOURCES = crypto/sha256_x86_shani.cpp # See explanation for -static in crypto_libbitcoin_crypto_base_la's LDFLAGS and diff --git a/src/crypto/sha256.cpp b/src/crypto/sha256.cpp index 4c7bb6f20ff50..bb98cd09d2dc5 100644 --- a/src/crypto/sha256.cpp +++ b/src/crypto/sha256.cpp @@ -623,7 +623,7 @@ std::string SHA256AutoDetect(sha256_implementation::UseImplementation use_implem } } -#if defined(ENABLE_X86_SHANI) +#if defined(ENABLE_SSE41) && defined(ENABLE_X86_SHANI) if (have_x86_shani) { Transform = sha256_x86_shani::Transform; TransformD64 = TransformD64Wrapper; diff --git a/src/crypto/sha256_x86_shani.cpp b/src/crypto/sha256_x86_shani.cpp index 79871bfcc112a..7471828193970 100644 --- a/src/crypto/sha256_x86_shani.cpp +++ b/src/crypto/sha256_x86_shani.cpp @@ -6,7 +6,7 @@ // Written and placed in public domain by Jeffrey Walton. // Based on code from Intel, and by Sean Gulley for the miTLS project. -#ifdef ENABLE_X86_SHANI +#if defined(ENABLE_SSE41) && defined(ENABLE_X86_SHANI) #include #include From 4202c170da37a3203e05a9f39f303d7df19b6d81 Mon Sep 17 00:00:00 2001 From: Matthew Zipkin Date: Mon, 27 Feb 2023 13:00:37 -0500 Subject: [PATCH 022/868] test: refactor interface_rpc.py Refactors the helper functions in the test to provide more fine-grained control over RPC requests and responses than the usual AuthProxy methods. No change in test behavior, the same RPC requests are made. --- test/functional/interface_rpc.py | 112 +++++++++++++++++++++++-------- 1 file changed, 83 insertions(+), 29 deletions(-) diff --git a/test/functional/interface_rpc.py b/test/functional/interface_rpc.py index e873e2da0bffc..501a841805a83 100755 --- a/test/functional/interface_rpc.py +++ b/test/functional/interface_rpc.py @@ -4,22 +4,72 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Tests some generic aspects of the RPC interface.""" +import json import os -from test_framework.authproxy import JSONRPCException +from dataclasses import dataclass from test_framework.test_framework import BitcoinTestFramework from test_framework.util import assert_equal, assert_greater_than_or_equal from threading import Thread +from typing import Optional import subprocess -def expect_http_status(expected_http_status, expected_rpc_code, - fcn, *args): - try: - fcn(*args) - raise AssertionError(f"Expected RPC error {expected_rpc_code}, got none") - except JSONRPCException as exc: - assert_equal(exc.error["code"], expected_rpc_code) - assert_equal(exc.http_status, expected_http_status) +RPC_INVALID_PARAMETER = -8 +RPC_METHOD_NOT_FOUND = -32601 +RPC_INVALID_REQUEST = -32600 + + +@dataclass +class BatchOptions: + version: Optional[int] = None + notification: bool = False + request_fields: Optional[dict] = None + response_fields: Optional[dict] = None + + +def format_request(options, idx, fields): + request = {} + if options.version == 1: + request.update(version="1.1") + elif options.version == 2: + request.update(jsonrpc="2.0") + elif options.version is not None: + raise NotImplementedError(f"Unknown JSONRPC version {options.version}") + if not options.notification: + request.update(id=idx) + request.update(fields) + if options.request_fields: + request.update(options.request_fields) + return request + + +def format_response(options, idx, fields): + response = {} + response.update(id=None if options.notification else idx) + response.update(result=None, error=None) + response.update(fields) + if options.response_fields: + response.update(options.response_fields) + return response + + +def send_raw_rpc(node, raw_body: bytes) -> tuple[object, int]: + return node._request("POST", "/", raw_body) + + +def send_json_rpc(node, body: object) -> tuple[object, int]: + raw = json.dumps(body).encode("utf-8") + return send_raw_rpc(node, raw) + + +def expect_http_rpc_status(expected_http_status, expected_rpc_error_code, node, method, params, version=1, notification=False): + req = format_request(BatchOptions(version, notification), 0, {"method": method, "params": params}) + response, status = send_json_rpc(node, req) + + if expected_rpc_error_code is not None: + assert_equal(response["error"]["code"], expected_rpc_error_code) + + assert_equal(status, expected_http_status) def test_work_queue_getblock(node, got_exceeded_error): @@ -51,34 +101,38 @@ def test_getrpcinfo(self): def test_batch_request(self): self.log.info("Testing basic JSON-RPC batch request...") - results = self.nodes[0].batch([ + calls = [ # A basic request that will work fine. - {"method": "getblockcount", "id": 1}, + {"method": "getblockcount"}, # Request that will fail. The whole batch request should still # work fine. - {"method": "invalidmethod", "id": 2}, + {"method": "invalidmethod"}, # Another call that should succeed. - {"method": "getblockhash", "id": 3, "params": [0]}, - ]) - - result_by_id = {} - for res in results: - result_by_id[res["id"]] = res - - assert_equal(result_by_id[1]['error'], None) - assert_equal(result_by_id[1]['result'], 0) - - assert_equal(result_by_id[2]['error']['code'], -32601) - assert_equal(result_by_id[2]['result'], None) - - assert_equal(result_by_id[3]['error'], None) - assert result_by_id[3]['result'] is not None + {"method": "getblockhash", "params": [0]}, + ] + results = [ + {"result": 0}, + {"error": {"code": RPC_METHOD_NOT_FOUND, "message": "Method not found"}}, + {"result": "0f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206"}, + {"error": {"code": RPC_INVALID_REQUEST, "message": "Missing method"}}, + ] + + request = [] + response = [] + for idx, (call, result) in enumerate(zip(calls, results), 1): + options = BatchOptions() + request.append(format_request(options, idx, call)) + response.append(format_response(options, idx, result)) + + rpc_response, http_status = send_json_rpc(self.nodes[0], request) + assert_equal(http_status, 200) + assert_equal(rpc_response, response) def test_http_status_codes(self): self.log.info("Testing HTTP status codes for JSON-RPC requests...") - expect_http_status(404, -32601, self.nodes[0].invalidmethod) - expect_http_status(500, -8, self.nodes[0].getblockhash, 42) + expect_http_rpc_status(404, RPC_METHOD_NOT_FOUND, self.nodes[0], "invalidmethod", []) + expect_http_rpc_status(500, RPC_INVALID_PARAMETER, self.nodes[0], "getblockhash", [42]) def test_work_queue_exceeded(self): self.log.info("Testing work queue exceeded...") From 09416f9ec445e4d6bb277400758083b0b4e8b174 Mon Sep 17 00:00:00 2001 From: Matthew Zipkin Date: Sun, 31 Dec 2023 07:55:03 -0500 Subject: [PATCH 023/868] test: cover JSONRPC 2.0 requests, batches, and notifications --- test/functional/interface_rpc.py | 97 ++++++++++++++++++++++++++++---- 1 file changed, 87 insertions(+), 10 deletions(-) diff --git a/test/functional/interface_rpc.py b/test/functional/interface_rpc.py index 501a841805a83..748d83858ae7f 100755 --- a/test/functional/interface_rpc.py +++ b/test/functional/interface_rpc.py @@ -14,9 +14,11 @@ import subprocess -RPC_INVALID_PARAMETER = -8 -RPC_METHOD_NOT_FOUND = -32601 -RPC_INVALID_REQUEST = -32600 +RPC_INVALID_ADDRESS_OR_KEY = -5 +RPC_INVALID_PARAMETER = -8 +RPC_METHOD_NOT_FOUND = -32601 +RPC_INVALID_REQUEST = -32600 +RPC_PARSE_ERROR = -32700 @dataclass @@ -98,9 +100,7 @@ def test_getrpcinfo(self): assert_greater_than_or_equal(command['duration'], 0) assert_equal(info['logpath'], os.path.join(self.nodes[0].chain_path, 'debug.log')) - def test_batch_request(self): - self.log.info("Testing basic JSON-RPC batch request...") - + def test_batch_request(self, call_options): calls = [ # A basic request that will work fine. {"method": "getblockcount"}, @@ -109,6 +109,8 @@ def test_batch_request(self): {"method": "invalidmethod"}, # Another call that should succeed. {"method": "getblockhash", "params": [0]}, + # Invalid request format + {"pizza": "sausage"} ] results = [ {"result": 0}, @@ -120,7 +122,9 @@ def test_batch_request(self): request = [] response = [] for idx, (call, result) in enumerate(zip(calls, results), 1): - options = BatchOptions() + options = call_options(idx) + if options is None: + continue request.append(format_request(options, idx, call)) response.append(format_response(options, idx, result)) @@ -128,11 +132,84 @@ def test_batch_request(self): assert_equal(http_status, 200) assert_equal(rpc_response, response) - def test_http_status_codes(self): - self.log.info("Testing HTTP status codes for JSON-RPC requests...") + def test_batch_requests(self): + self.log.info("Testing empty batch request...") + self.test_batch_request(lambda idx: None) + + self.log.info("Testing basic JSON-RPC 2.0 batch request...") + self.test_batch_request(lambda idx: BatchOptions(version=2)) + + self.log.info("Testing JSON-RPC 2.0 batch with notifications...") + self.test_batch_request(lambda idx: BatchOptions(version=2, notification=idx < 2)) + + self.log.info("Testing JSON-RPC 2.0 batch of ALL notifications...") + self.test_batch_request(lambda idx: BatchOptions(version=2, notification=True)) + + # JSONRPC 1.1 does not support batch requests, but test them for backwards compatibility. + self.log.info("Testing nonstandard JSON-RPC 1.1 batch request...") + self.test_batch_request(lambda idx: BatchOptions(version=1)) + self.log.info("Testing nonstandard mixed JSON-RPC 1.1/2.0 batch request...") + self.test_batch_request(lambda idx: BatchOptions(version=2 if idx % 2 else 1)) + + self.log.info("Testing nonstandard batch request without version numbers...") + self.test_batch_request(lambda idx: BatchOptions()) + + self.log.info("Testing nonstandard batch request without version numbers or ids...") + self.test_batch_request(lambda idx: BatchOptions(notification=True)) + + self.log.info("Testing nonstandard jsonrpc 1.0 version number is accepted...") + self.test_batch_request(lambda idx: BatchOptions(request_fields={"jsonrpc": "1.0"})) + + self.log.info("Testing unrecognized jsonrpc version number is accepted...") + self.test_batch_request(lambda idx: BatchOptions(request_fields={"jsonrpc": "2.1"})) + + def test_http_status_codes(self): + self.log.info("Testing HTTP status codes for JSON-RPC 1.1 requests...") + # OK + expect_http_rpc_status(200, None, self.nodes[0], "getblockhash", [0]) + # Errors expect_http_rpc_status(404, RPC_METHOD_NOT_FOUND, self.nodes[0], "invalidmethod", []) expect_http_rpc_status(500, RPC_INVALID_PARAMETER, self.nodes[0], "getblockhash", [42]) + # force-send empty request + response, status = send_raw_rpc(self.nodes[0], b"") + assert_equal(response, {"id": None, "result": None, "error": {"code": RPC_PARSE_ERROR, "message": "Parse error"}}) + assert_equal(status, 500) + # force-send invalidly formatted request + response, status = send_raw_rpc(self.nodes[0], b"this is bad") + assert_equal(response, {"id": None, "result": None, "error": {"code": RPC_PARSE_ERROR, "message": "Parse error"}}) + assert_equal(status, 500) + + self.log.info("Testing HTTP status codes for JSON-RPC 2.0 requests...") + # OK + expect_http_rpc_status(200, None, self.nodes[0], "getblockhash", [0], 2, False) + # RPC errors and HTTP errors + expect_http_rpc_status(404, RPC_METHOD_NOT_FOUND, self.nodes[0], "invalidmethod", [], 2, False) + expect_http_rpc_status(500, RPC_INVALID_PARAMETER, self.nodes[0], "getblockhash", [42], 2, False) + # force-send invalidly formatted requests + response, status = send_json_rpc(self.nodes[0], {"jsonrpc": 2, "method": "getblockcount"}) + assert_equal(response, {"error": None, "id": None, "result": 0}) + assert_equal(status, 200) + response, status = send_json_rpc(self.nodes[0], {"jsonrpc": "3.0", "method": "getblockcount"}) + assert_equal(response, {"error": None, "id": None, "result": 0}) + assert_equal(status, 200) + + self.log.info("Testing HTTP status codes for JSON-RPC 2.0 notifications...") + # Not notification: id exists + response, status = send_json_rpc(self.nodes[0], {"jsonrpc": "2.0", "id": None, "method": "getblockcount"}) + assert_equal(response["result"], 0) + assert_equal(status, 200) + # Not notification: JSON 1.1 + expect_http_rpc_status(200, None, self.nodes[0], "getblockcount", [], 1) + # Not notification: has "id" field + expect_http_rpc_status(200, None, self.nodes[0], "getblockcount", [], 2, False) + block_count = self.nodes[0].getblockcount() + expect_http_rpc_status(200, None, self.nodes[0], "generatetoaddress", [1, "bcrt1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqdku202"], 2, True) + # The command worked even though there was no response + assert_equal(block_count + 1, self.nodes[0].getblockcount()) + expect_http_rpc_status(500, RPC_INVALID_ADDRESS_OR_KEY, self.nodes[0], "generatetoaddress", [1, "invalid_address"], 2, True) + # Sanity check: command was not executed + assert_equal(block_count + 1, self.nodes[0].getblockcount()) def test_work_queue_exceeded(self): self.log.info("Testing work queue exceeded...") @@ -148,7 +225,7 @@ def test_work_queue_exceeded(self): def run_test(self): self.test_getrpcinfo() - self.test_batch_request() + self.test_batch_requests() self.test_http_status_codes() self.test_work_queue_exceeded() From df6e3756d6feaf1856e7886820b70874209fd90b Mon Sep 17 00:00:00 2001 From: Matthew Zipkin Date: Tue, 23 Jan 2024 10:33:26 -0500 Subject: [PATCH 024/868] rpc: Avoid copies in JSONRPCReplyObj() Change parameters from const references to values, so they can be moved into the reply instead of copied. Also update callers to move instead of copy. --- src/bitcoin-cli.cpp | 6 +++--- src/httprpc.cpp | 10 +++++----- src/rpc/request.cpp | 14 ++++---------- src/rpc/request.h | 3 +-- src/rpc/server.cpp | 6 +++--- 5 files changed, 16 insertions(+), 23 deletions(-) diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index 45db7a9a66cb1..3591f7248bf89 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -302,7 +302,7 @@ class AddrinfoRequestHandler : public BaseRequestHandler } addresses.pushKV("total", total); result.pushKV("addresses_known", addresses); - return JSONRPCReplyObj(result, NullUniValue, 1); + return JSONRPCReplyObj(std::move(result), NullUniValue, 1); } }; @@ -371,7 +371,7 @@ class GetinfoRequestHandler: public BaseRequestHandler } result.pushKV("relayfee", batch[ID_NETWORKINFO]["result"]["relayfee"]); result.pushKV("warnings", batch[ID_NETWORKINFO]["result"]["warnings"]); - return JSONRPCReplyObj(result, NullUniValue, 1); + return JSONRPCReplyObj(std::move(result), NullUniValue, 1); } }; @@ -709,7 +709,7 @@ class GenerateToAddressRequestHandler : public BaseRequestHandler UniValue result(UniValue::VOBJ); result.pushKV("address", address_str); result.pushKV("blocks", reply.get_obj()["result"]); - return JSONRPCReplyObj(result, NullUniValue, 1); + return JSONRPCReplyObj(std::move(result), NullUniValue, 1); } protected: std::string address_str; diff --git a/src/httprpc.cpp b/src/httprpc.cpp index c72dbf10bc958..aef20c24fc685 100644 --- a/src/httprpc.cpp +++ b/src/httprpc.cpp @@ -73,7 +73,7 @@ static std::vector> g_rpcauth; static std::map> g_rpc_whitelist; static bool g_rpc_whitelist_default = false; -static void JSONErrorReply(HTTPRequest* req, const UniValue& objError, const UniValue& id) +static void JSONErrorReply(HTTPRequest* req, UniValue objError, UniValue id) { // Send error reply from json-rpc error object int nStatus = HTTP_INTERNAL_SERVER_ERROR; @@ -84,7 +84,7 @@ static void JSONErrorReply(HTTPRequest* req, const UniValue& objError, const Uni else if (code == RPC_METHOD_NOT_FOUND) nStatus = HTTP_NOT_FOUND; - std::string strReply = JSONRPCReply(NullUniValue, objError, id); + std::string strReply = JSONRPCReplyObj(NullUniValue, std::move(objError), std::move(id)).write() + "\n"; req->WriteHeader("Content-Type", "application/json"); req->WriteReply(nStatus, strReply); @@ -203,7 +203,7 @@ static bool HTTPReq_JSONRPC(const std::any& context, HTTPRequest* req) UniValue result = tableRPC.execute(jreq); // Send reply - strReply = JSONRPCReply(result, NullUniValue, jreq.id); + strReply = JSONRPCReplyObj(std::move(result), NullUniValue, jreq.id).write() + "\n"; // array of requests } else if (valRequest.isArray()) { @@ -230,8 +230,8 @@ static bool HTTPReq_JSONRPC(const std::any& context, HTTPRequest* req) req->WriteHeader("Content-Type", "application/json"); req->WriteReply(HTTP_OK, strReply); - } catch (const UniValue& objError) { - JSONErrorReply(req, objError, jreq.id); + } catch (UniValue& e) { + JSONErrorReply(req, std::move(e), jreq.id); return false; } catch (const std::exception& e) { JSONErrorReply(req, JSONRPCError(RPC_PARSE_ERROR, e.what()), jreq.id); diff --git a/src/rpc/request.cpp b/src/rpc/request.cpp index b7acd62ee3d4b..12726ecc88599 100644 --- a/src/rpc/request.cpp +++ b/src/rpc/request.cpp @@ -37,24 +37,18 @@ UniValue JSONRPCRequestObj(const std::string& strMethod, const UniValue& params, return request; } -UniValue JSONRPCReplyObj(const UniValue& result, const UniValue& error, const UniValue& id) +UniValue JSONRPCReplyObj(UniValue result, UniValue error, UniValue id) { UniValue reply(UniValue::VOBJ); if (!error.isNull()) reply.pushKV("result", NullUniValue); else - reply.pushKV("result", result); - reply.pushKV("error", error); - reply.pushKV("id", id); + reply.pushKV("result", std::move(result)); + reply.pushKV("error", std::move(error)); + reply.pushKV("id", std::move(id)); return reply; } -std::string JSONRPCReply(const UniValue& result, const UniValue& error, const UniValue& id) -{ - UniValue reply = JSONRPCReplyObj(result, error, id); - return reply.write() + "\n"; -} - UniValue JSONRPCError(int code, const std::string& message) { UniValue error(UniValue::VOBJ); diff --git a/src/rpc/request.h b/src/rpc/request.h index a682c58d966f0..116ebb8aac15b 100644 --- a/src/rpc/request.h +++ b/src/rpc/request.h @@ -12,8 +12,7 @@ #include UniValue JSONRPCRequestObj(const std::string& strMethod, const UniValue& params, const UniValue& id); -UniValue JSONRPCReplyObj(const UniValue& result, const UniValue& error, const UniValue& id); -std::string JSONRPCReply(const UniValue& result, const UniValue& error, const UniValue& id); +UniValue JSONRPCReplyObj(UniValue result, UniValue error, UniValue id); UniValue JSONRPCError(int code, const std::string& message); /** Generate a new RPC authentication cookie and write it to disk */ diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index 4883341522f88..fd18a7f9ec084 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -366,11 +366,11 @@ static UniValue JSONRPCExecOne(JSONRPCRequest jreq, const UniValue& req) jreq.parse(req); UniValue result = tableRPC.execute(jreq); - rpc_result = JSONRPCReplyObj(result, NullUniValue, jreq.id); + rpc_result = JSONRPCReplyObj(std::move(result), NullUniValue, jreq.id); } - catch (const UniValue& objError) + catch (UniValue& e) { - rpc_result = JSONRPCReplyObj(NullUniValue, objError, jreq.id); + rpc_result = JSONRPCReplyObj(NullUniValue, std::move(e), jreq.id); } catch (const std::exception& e) { From a64a2b77e09bff784a2635ba19ff4aa6582bb5a5 Mon Sep 17 00:00:00 2001 From: Matthew Zipkin Date: Mon, 27 Feb 2023 13:03:45 -0500 Subject: [PATCH 025/868] rpc: refactor single/batch requests Simplify the request handling flow so that errors and results only come from JSONRPCExec() --- src/httprpc.cpp | 24 ++++++++++++++++++------ src/rpc/server.cpp | 33 +++++---------------------------- src/rpc/server.h | 2 +- 3 files changed, 24 insertions(+), 35 deletions(-) diff --git a/src/httprpc.cpp b/src/httprpc.cpp index aef20c24fc685..53f994efd7f0f 100644 --- a/src/httprpc.cpp +++ b/src/httprpc.cpp @@ -185,7 +185,7 @@ static bool HTTPReq_JSONRPC(const std::any& context, HTTPRequest* req) // Set the URI jreq.URI = req->GetURI(); - std::string strReply; + UniValue reply; bool user_has_whitelist = g_rpc_whitelist.count(jreq.authUser); if (!user_has_whitelist && g_rpc_whitelist_default) { LogPrintf("RPC User %s not allowed to call any methods\n", jreq.authUser); @@ -200,13 +200,12 @@ static bool HTTPReq_JSONRPC(const std::any& context, HTTPRequest* req) req->WriteReply(HTTP_FORBIDDEN); return false; } - UniValue result = tableRPC.execute(jreq); - // Send reply - strReply = JSONRPCReplyObj(std::move(result), NullUniValue, jreq.id).write() + "\n"; + reply = JSONRPCExec(jreq); // array of requests } else if (valRequest.isArray()) { + // Check authorization for each request's method if (user_has_whitelist) { for (unsigned int reqIdx = 0; reqIdx < valRequest.size(); reqIdx++) { if (!valRequest[reqIdx].isObject()) { @@ -223,13 +222,26 @@ static bool HTTPReq_JSONRPC(const std::any& context, HTTPRequest* req) } } } - strReply = JSONRPCExecBatch(jreq, valRequest.get_array()); + + // Execute each request + reply = UniValue::VARR; + for (size_t i{0}; i < valRequest.size(); ++i) { + // Batches include errors in the batch response, they do not throw + try { + jreq.parse(valRequest[i]); + reply.push_back(JSONRPCExec(jreq)); + } catch (UniValue& e) { + reply.push_back(JSONRPCReplyObj(NullUniValue, std::move(e), jreq.id)); + } catch (const std::exception& e) { + reply.push_back(JSONRPCReplyObj(NullUniValue, JSONRPCError(RPC_PARSE_ERROR, e.what()), jreq.id)); + } + } } else throw JSONRPCError(RPC_PARSE_ERROR, "Top-level object parse error"); req->WriteHeader("Content-Type", "application/json"); - req->WriteReply(HTTP_OK, strReply); + req->WriteReply(HTTP_OK, reply.write() + "\n"); } catch (UniValue& e) { JSONErrorReply(req, std::move(e), jreq.id); return false; diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index fd18a7f9ec084..f19991409596e 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -358,36 +358,13 @@ bool IsDeprecatedRPCEnabled(const std::string& method) return find(enabled_methods.begin(), enabled_methods.end(), method) != enabled_methods.end(); } -static UniValue JSONRPCExecOne(JSONRPCRequest jreq, const UniValue& req) +UniValue JSONRPCExec(const JSONRPCRequest& jreq) { - UniValue rpc_result(UniValue::VOBJ); + // Might throw exception. Single requests will throw and send HTTP error codes + // but inside a batch, we just include the error object and return HTTP 200 + UniValue result = tableRPC.execute(jreq); - try { - jreq.parse(req); - - UniValue result = tableRPC.execute(jreq); - rpc_result = JSONRPCReplyObj(std::move(result), NullUniValue, jreq.id); - } - catch (UniValue& e) - { - rpc_result = JSONRPCReplyObj(NullUniValue, std::move(e), jreq.id); - } - catch (const std::exception& e) - { - rpc_result = JSONRPCReplyObj(NullUniValue, - JSONRPCError(RPC_PARSE_ERROR, e.what()), jreq.id); - } - - return rpc_result; -} - -std::string JSONRPCExecBatch(const JSONRPCRequest& jreq, const UniValue& vReq) -{ - UniValue ret(UniValue::VARR); - for (unsigned int reqIdx = 0; reqIdx < vReq.size(); reqIdx++) - ret.push_back(JSONRPCExecOne(jreq, vReq[reqIdx])); - - return ret.write() + "\n"; + return JSONRPCReplyObj(std::move(result), NullUniValue, jreq.id); } /** diff --git a/src/rpc/server.h b/src/rpc/server.h index 9a49d825703a9..2761bcd9db1f7 100644 --- a/src/rpc/server.h +++ b/src/rpc/server.h @@ -181,7 +181,7 @@ extern CRPCTable tableRPC; void StartRPC(); void InterruptRPC(); void StopRPC(); -std::string JSONRPCExecBatch(const JSONRPCRequest& jreq, const UniValue& vReq); +UniValue JSONRPCExec(const JSONRPCRequest& jreq); // Drop witness when serializing for RPC? bool RPCSerializationWithoutWitness(); From bf9669af9ccc33dcade09bceb27d6745e9d9a75a Mon Sep 17 00:00:00 2001 From: stratospher <44024636+stratospher@users.noreply.github.com> Date: Tue, 30 Jan 2024 19:27:21 +0530 Subject: [PATCH 026/868] test: Rename early key response test and move random_bitflip to util Early key response test is a special kind of test which requires modified v2 handshake functions. More such tests can be added where v2 handshake functions send incorrect garbage terminator, excess garbage bytes etc.. Hence, rename p2p_v2_earlykey.py to a general test file name - p2p_v2_misbehaving.py. random_bitflip function (used in signature tests prior to this commit) can be used in p2p_v2_misbehaving test to generate wrong garbage terminator, wrong garbage bytes etc.. So, move the function to util. --- .../{p2p_v2_earlykeyresponse.py => p2p_v2_misbehaving.py} | 0 test/functional/test_framework/key.py | 6 +----- test/functional/test_framework/util.py | 7 +++++++ test/functional/test_runner.py | 2 +- 4 files changed, 9 insertions(+), 6 deletions(-) rename test/functional/{p2p_v2_earlykeyresponse.py => p2p_v2_misbehaving.py} (100%) diff --git a/test/functional/p2p_v2_earlykeyresponse.py b/test/functional/p2p_v2_misbehaving.py similarity index 100% rename from test/functional/p2p_v2_earlykeyresponse.py rename to test/functional/p2p_v2_misbehaving.py diff --git a/test/functional/test_framework/key.py b/test/functional/test_framework/key.py index 06252f89960ab..939c7cbef617f 100644 --- a/test/functional/test_framework/key.py +++ b/test/functional/test_framework/key.py @@ -14,6 +14,7 @@ import unittest from test_framework.crypto import secp256k1 +from test_framework.util import random_bitflip # Point with no known discrete log. H_POINT = "50929b74c1a04954b78b4b6035e97a5e078a5a0f28ec96d547bfee9ace803ac0" @@ -292,11 +293,6 @@ def sign_schnorr(key, msg, aux=None, flip_p=False, flip_r=False): class TestFrameworkKey(unittest.TestCase): def test_ecdsa_and_schnorr(self): """Test the Python ECDSA and Schnorr implementations.""" - def random_bitflip(sig): - sig = list(sig) - sig[random.randrange(len(sig))] ^= (1 << (random.randrange(8))) - return bytes(sig) - byte_arrays = [generate_privkey() for _ in range(3)] + [v.to_bytes(32, 'big') for v in [0, ORDER - 1, ORDER, 2**256 - 1]] keys = {} for privkey_bytes in byte_arrays: # build array of key/pubkey pairs diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py index b4b05b159728d..53f4a8ee4da82 100644 --- a/test/functional/test_framework/util.py +++ b/test/functional/test_framework/util.py @@ -14,6 +14,7 @@ import os import pathlib import platform +import random import re import time @@ -230,6 +231,12 @@ def ceildiv(a, b): return -(-a // b) +def random_bitflip(data): + data = list(data) + data[random.randrange(len(data))] ^= (1 << (random.randrange(8))) + return bytes(data) + + def get_fee(tx_size, feerate_btc_kvb): """Calculate the fee in BTC given a feerate is BTC/kvB. Reflects CFeeRate::GetFee""" feerate_sat_kvb = int(feerate_btc_kvb * Decimal(1e8)) # Fee in sat/kvb as an int to avoid float precision errors diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 4d66ea97c84c8..964b7ab6aa22f 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -264,7 +264,7 @@ 'p2p_invalid_tx.py --v2transport', 'p2p_v2_transport.py', 'p2p_v2_encrypted.py', - 'p2p_v2_earlykeyresponse.py', + 'p2p_v2_misbehaving.py', 'example_test.py', 'mempool_accept_v3.py', 'wallet_txn_doublespend.py --legacy-wallet', From 86cca2cba230c10324c6aedd12ae9655b83b2856 Mon Sep 17 00:00:00 2001 From: stratospher <44024636+stratospher@users.noreply.github.com> Date: Tue, 13 Feb 2024 10:37:23 +0530 Subject: [PATCH 027/868] test: Support disconnect waiting for add_p2p_connection Adds a new boolean parameter `expect_success` to the `add_p2p_connection` method. If set, the node under test doesn't wait for connection to be established and is useful for testing scenarios when disconnection is expected. Without this parameter, intermittent test failures can happen if the disconnection happens before wait_until for is_connected is hit inside `add_p2p_connection`. Co-Authored-By: Sebastian Falbesoner --- test/functional/test_framework/test_node.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index 3baa78fd79f90..d6cb5262ba1fa 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -667,7 +667,7 @@ def assert_start_raises_init_error(self, extra_args=None, expected_msg=None, mat assert_msg += "with expected error " + expected_msg self._raise_assertion_error(assert_msg) - def add_p2p_connection(self, p2p_conn, *, wait_for_verack=True, send_version=True, supports_v2_p2p=None, wait_for_v2_handshake=True, **kwargs): + def add_p2p_connection(self, p2p_conn, *, wait_for_verack=True, send_version=True, supports_v2_p2p=None, wait_for_v2_handshake=True, expect_success=True, **kwargs): """Add an inbound p2p connection to the node. This method adds the p2p connection to the self.p2ps list and also @@ -687,7 +687,6 @@ def add_p2p_connection(self, p2p_conn, *, wait_for_verack=True, send_version=Tru if supports_v2_p2p is None: supports_v2_p2p = self.use_v2transport - p2p_conn.p2p_connected_to_node = True if self.use_v2transport: kwargs['services'] = kwargs.get('services', P2P_SERVICES) | NODE_P2P_V2 @@ -695,6 +694,8 @@ def add_p2p_connection(self, p2p_conn, *, wait_for_verack=True, send_version=Tru p2p_conn.peer_connect(**kwargs, send_version=send_version, net=self.chain, timeout_factor=self.timeout_factor, supports_v2_p2p=supports_v2_p2p)() self.p2ps.append(p2p_conn) + if not expect_success: + return p2p_conn p2p_conn.wait_until(lambda: p2p_conn.is_connected, check_connected=False) if supports_v2_p2p and wait_for_v2_handshake: p2p_conn.wait_until(lambda: p2p_conn.v2_state.tried_v2_handshake) From d1ed09a7643b567e021b2ecb756bc925c48fc708 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Thu, 14 Mar 2024 23:56:48 +0000 Subject: [PATCH 028/868] Bugfix: GUI: Help messages already have a trailing newline, so don't add an extra one --- src/qt/utilitydialog.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qt/utilitydialog.cpp b/src/qt/utilitydialog.cpp index 6509a701f36ba..bdaef47406d06 100644 --- a/src/qt/utilitydialog.cpp +++ b/src/qt/utilitydialog.cpp @@ -124,7 +124,7 @@ HelpMessageDialog::~HelpMessageDialog() void HelpMessageDialog::printToConsole() { // On other operating systems, the expected action is to print the message to the console. - tfm::format(std::cout, "%s\n", qPrintable(text)); + tfm::format(std::cout, "%s", qPrintable(text)); } void HelpMessageDialog::showOrPrint() From 32c80413fdb063199f3bee719c4651bd63f05fce Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Wed, 20 Dec 2023 15:41:05 -0500 Subject: [PATCH 029/868] bench: add benchmark for checkblockindex --- src/Makefile.bench.include | 1 + src/bench/checkblockindex.cpp | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+) create mode 100644 src/bench/checkblockindex.cpp diff --git a/src/Makefile.bench.include b/src/Makefile.bench.include index 7ba0111fa686f..2ba72c9e76b7f 100644 --- a/src/Makefile.bench.include +++ b/src/Makefile.bench.include @@ -23,6 +23,7 @@ bench_bench_bitcoin_SOURCES = \ bench/ccoins_caching.cpp \ bench/chacha20.cpp \ bench/checkblock.cpp \ + bench/checkblockindex.cpp \ bench/checkqueue.cpp \ bench/crypto_hash.cpp \ bench/data.cpp \ diff --git a/src/bench/checkblockindex.cpp b/src/bench/checkblockindex.cpp new file mode 100644 index 0000000000000..e8a848dbd409b --- /dev/null +++ b/src/bench/checkblockindex.cpp @@ -0,0 +1,20 @@ +// Copyright (c) 2023-present 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 +#include +#include + +static void CheckBlockIndex(benchmark::Bench& bench) +{ + auto testing_setup{MakeNoLogFileContext()}; + // Mine some more blocks + testing_setup->mineBlocks(1000); + bench.run([&] { + testing_setup->m_node.chainman->CheckBlockIndex(); + }); +} + + +BENCHMARK(CheckBlockIndex, benchmark::PriorityLevel::HIGH); From 4a6d1d1e3bc3d559cd13d134b4b827f22635ac4b Mon Sep 17 00:00:00 2001 From: Andrew Toth Date: Thu, 17 Aug 2023 16:17:18 -0400 Subject: [PATCH 030/868] validation: don't clear cache on periodic flush --- src/validation.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/validation.cpp b/src/validation.cpp index 5c585438d14da..94c470ce2cc42 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2745,8 +2745,10 @@ bool Chainstate::FlushStateToDisk( return FatalError(m_chainman.GetNotifications(), state, _("Disk space is too low!")); } // Flush the chainstate (which may refer to block index entries). - if (!CoinsTip().Flush()) + const auto empty_cache{(mode == FlushStateMode::ALWAYS) || fCacheLarge || fCacheCritical || fFlushForPrune}; + if (empty_cache ? !CoinsTip().Flush() : !CoinsTip().Sync()) { return FatalError(m_chainman.GetNotifications(), state, _("Failed to write to coin database.")); + } m_last_flush = nNow; full_flush_completed = true; TRACE5(utxocache, flush, From 671b7a32516d62e1e79393ded4b45910bd08010a Mon Sep 17 00:00:00 2001 From: furszy Date: Fri, 29 Mar 2024 11:40:38 -0300 Subject: [PATCH 031/868] gui: fix create unsigned transaction fee bump --- src/qt/walletmodel.cpp | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index 1bdf94d3b57c4..a481092327485 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -529,12 +529,6 @@ bool WalletModel::bumpFee(uint256 hash, uint256& new_hash) return false; } - WalletModel::UnlockContext ctx(requestUnlock()); - if(!ctx.isValid()) - { - return false; - } - // Short-circuit if we are returning a bumped transaction PSBT to clipboard if (retval == QMessageBox::Save) { // "Create Unsigned" clicked @@ -549,10 +543,15 @@ bool WalletModel::bumpFee(uint256 hash, uint256& new_hash) DataStream ssTx{}; ssTx << psbtx; GUIUtil::setClipboard(EncodeBase64(ssTx.str()).c_str()); - Q_EMIT message(tr("PSBT copied"), tr("Copied to clipboard", "Fee-bump PSBT saved"), CClientUIInterface::MSG_INFORMATION); + Q_EMIT message(tr("PSBT copied"), tr("Fee-bump PSBT copied to clipboard"), CClientUIInterface::MSG_INFORMATION | CClientUIInterface::MODAL); return true; } + WalletModel::UnlockContext ctx(requestUnlock()); + if (!ctx.isValid()) { + return false; + } + assert(!m_wallet->privateKeysDisabled() || wallet().hasExternalSigner()); // sign bumped transaction From 23dc0c19acd54cad1bed2f14df024b6b533f2330 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Sat, 30 Mar 2024 09:34:20 +0000 Subject: [PATCH 032/868] msvc, bench: Add missing source files to bench_bitcoin project --- build_msvc/bench_bitcoin/bench_bitcoin.vcxproj.in | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/build_msvc/bench_bitcoin/bench_bitcoin.vcxproj.in b/build_msvc/bench_bitcoin/bench_bitcoin.vcxproj.in index a5702a83ba3e5..972d6d05d7897 100644 --- a/build_msvc/bench_bitcoin/bench_bitcoin.vcxproj.in +++ b/build_msvc/bench_bitcoin/bench_bitcoin.vcxproj.in @@ -10,6 +10,12 @@ @SOURCE_FILES@ + + + + + + From 31a15f0aff79d2b34a9640909b9e6fb39a647b60 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Sun, 31 Mar 2024 10:24:20 +0100 Subject: [PATCH 033/868] bench: Disable WalletCreate* benchmarks when building with MSVC --- src/bench/wallet_create.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/bench/wallet_create.cpp b/src/bench/wallet_create.cpp index 32f55f51e153e..d7e772ac82726 100644 --- a/src/bench/wallet_create.cpp +++ b/src/bench/wallet_create.cpp @@ -51,9 +51,14 @@ static void WalletCreate(benchmark::Bench& bench, bool encrypted) static void WalletCreatePlain(benchmark::Bench& bench) { WalletCreate(bench, /*encrypted=*/false); } static void WalletCreateEncrypted(benchmark::Bench& bench) { WalletCreate(bench, /*encrypted=*/true); } +#ifndef _MSC_VER +// TODO: Being built with MSVC, the fs::remove_all() call in +// the WalletCreate() fails with the error "The process cannot +// access the file because it is being used by another process." #ifdef USE_SQLITE BENCHMARK(WalletCreatePlain, benchmark::PriorityLevel::LOW); BENCHMARK(WalletCreateEncrypted, benchmark::PriorityLevel::LOW); #endif +#endif } // namespace wallet From ca18aea5c4975ace4e307be96c74641d203fa389 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Tue, 2 Jan 2024 16:34:50 -0500 Subject: [PATCH 034/868] Add AutoFile::seek and tell It's useful to be able to seek to a specific position in a file. Allow AutoFile to seek by using fseek. It's also useful to be able to get the current position in a file. Allow AutoFile to tell by using ftell. --- src/streams.cpp | 22 ++++++++++++++++++++++ src/streams.h | 3 +++ 2 files changed, 25 insertions(+) diff --git a/src/streams.cpp b/src/streams.cpp index 6921dad677336..cdd36a86feb17 100644 --- a/src/streams.cpp +++ b/src/streams.cpp @@ -21,6 +21,28 @@ std::size_t AutoFile::detail_fread(Span dst) } } +void AutoFile::seek(int64_t offset, int origin) +{ + if (IsNull()) { + throw std::ios_base::failure("AutoFile::seek: file handle is nullptr"); + } + if (std::fseek(m_file, offset, origin) != 0) { + throw std::ios_base::failure(feof() ? "AutoFile::seek: end of file" : "AutoFile::seek: fseek failed"); + } +} + +int64_t AutoFile::tell() +{ + if (IsNull()) { + throw std::ios_base::failure("AutoFile::tell: file handle is nullptr"); + } + int64_t r{std::ftell(m_file)}; + if (r < 0) { + throw std::ios_base::failure("AutoFile::tell: ftell failed"); + } + return r; +} + void AutoFile::read(Span dst) { if (detail_fread(dst) != dst.size()) { diff --git a/src/streams.h b/src/streams.h index bc04a2babdace..57fc600646cc7 100644 --- a/src/streams.h +++ b/src/streams.h @@ -435,6 +435,9 @@ class AutoFile /** Implementation detail, only used internally. */ std::size_t detail_fread(Span dst); + void seek(int64_t offset, int origin); + int64_t tell(); + // // Stream subset // From 10c5275ba4532fb1bf54057d2f61fc35b51f1e85 Mon Sep 17 00:00:00 2001 From: willcl-ark Date: Thu, 4 Apr 2024 21:08:38 +0100 Subject: [PATCH 035/868] gui: don't permit port in proxy IP option Fixes: #809 Previously it was possible through the GUI to enter an IP address:port into the "Proxy IP" configuration box. After the node was restarted the errant setting would prevent the node starting back up until manually removed from settings.json. --- src/qt/optionsdialog.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/qt/optionsdialog.cpp b/src/qt/optionsdialog.cpp index dd654a7abe27c..6d0904adb8c19 100644 --- a/src/qt/optionsdialog.cpp +++ b/src/qt/optionsdialog.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include @@ -482,7 +483,10 @@ QValidator(parent) QValidator::State ProxyAddressValidator::validate(QString &input, int &pos) const { Q_UNUSED(pos); - // Validate the proxy + uint16_t port{0}; + std::string hostname; + if (!SplitHostPort(input.toStdString(), port, hostname) || port != 0) return QValidator::Invalid; + CService serv(LookupNumeric(input.toStdString(), DEFAULT_GUI_PROXY_PORT)); Proxy addrProxy = Proxy(serv, true); if (addrProxy.IsValid()) From 41a1a8615dd48fdd9811b9824c49ceb934c6375e Mon Sep 17 00:00:00 2001 From: "@RandyMcMillan" Date: Thu, 28 Mar 2024 15:39:49 -0400 Subject: [PATCH 036/868] gui: Hide peers details Add a close (X) button to the Peers Detail panel. Reuse the same icon used in the Console Tab. The close button deselects the peer highlighted in the PeerTableView and hides the detail panel. This PR addresses issue #485: Co-authored-by: @w0xlt --- src/qt/forms/debugwindow.ui | 147 +++++++++++++++++++++++++++--------- src/qt/rpcconsole.cpp | 2 + 2 files changed, 113 insertions(+), 36 deletions(-) diff --git a/src/qt/forms/debugwindow.ui b/src/qt/forms/debugwindow.ui index 33308cd68c0a4..c8d24d773afd2 100644 --- a/src/qt/forms/debugwindow.ui +++ b/src/qt/forms/debugwindow.ui @@ -984,42 +984,117 @@ - - - - - 0 - 0 - - - - - 0 - 32 - - - - - 10 - - - - IBeamCursor - - - Select a peer to view detailed information. - - - Qt::AlignHCenter|Qt::AlignTop - - - true - - - Qt::LinksAccessibleByMouse|Qt::TextSelectableByKeyboard|Qt::TextSelectableByMouse - - - + + + + 0 + + + 0 + + + + + + 0 + 0 + + + + + 0 + 32 + + + + + 10 + + + + IBeamCursor + + + Select a peer to view detailed information. + + + Qt::AlignHCenter|Qt::AlignTop + + + true + + + Qt::LinksAccessibleByMouse|Qt::TextSelectableByKeyboard|Qt::TextSelectableByMouse + + + + + + + true + + + + 32 + 32 + + + + + 32 + 32 + + + + + + 0 + 0 + 32 + 32 + + + + + 32 + 32 + + + + + 32 + 32 + + + + Hide Peers Detail + + + Qt::LeftToRight + + + + + + + :/icons/remove + :/icons/remove + + + + + 22 + 22 + + + + Ctrl+X + + + + + + diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index a07686ab2b766..96b6ebf134679 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -550,6 +550,7 @@ RPCConsole::RPCConsole(interfaces::Node& node, const PlatformStyle *_platformSty ui->lineEdit->setMaxLength(16 * 1024 * 1024); ui->messagesWidget->installEventFilter(this); + connect(ui->hidePeersDetailButton, &QAbstractButton::clicked, this, &RPCConsole::clearSelectedNode); connect(ui->clearButton, &QAbstractButton::clicked, [this] { clear(); }); connect(ui->fontBiggerButton, &QAbstractButton::clicked, this, &RPCConsole::fontBigger); connect(ui->fontSmallerButton, &QAbstractButton::clicked, this, &RPCConsole::fontSmaller); @@ -1221,6 +1222,7 @@ void RPCConsole::updateDetailWidget() ui->peerRelayTxes->setText(stats->nodeStateStats.m_relay_txs ? ts.yes : ts.no); } + ui->hidePeersDetailButton->setIcon(platformStyle->SingleColorIcon(QStringLiteral(":/icons/remove"))); ui->peersTabRightPanel->show(); } From 9e13ccc50eec9d2efe0f472e6d50dc822df70d84 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Thu, 11 Apr 2024 10:51:43 -0400 Subject: [PATCH 037/868] psbt: Check non witness utxo outpoint early A common issue that our fuzzers keep finding is that outpoints don't exist in the non witness utxos. Instead of trying to track this down and checking in various individual places, do the check early during deserialization. --- src/psbt.h | 9 +++++++-- test/functional/data/rpc_psbt.json | 4 +++- test/functional/rpc_psbt.py | 6 ++---- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/psbt.h b/src/psbt.h index 3f7408371707c..a6837fda493fb 100644 --- a/src/psbt.h +++ b/src/psbt.h @@ -1173,8 +1173,13 @@ struct PartiallySignedTransaction inputs.push_back(input); // Make sure the non-witness utxo matches the outpoint - if (input.non_witness_utxo && input.non_witness_utxo->GetHash() != tx->vin[i].prevout.hash) { - throw std::ios_base::failure("Non-witness UTXO does not match outpoint hash"); + if (input.non_witness_utxo) { + if (input.non_witness_utxo->GetHash() != tx->vin[i].prevout.hash) { + throw std::ios_base::failure("Non-witness UTXO does not match outpoint hash"); + } + if (tx->vin[i].prevout.n >= input.non_witness_utxo->vout.size()) { + throw std::ios_base::failure("Input specifies output index that does not exist"); + } } ++i; } diff --git a/test/functional/data/rpc_psbt.json b/test/functional/data/rpc_psbt.json index 31273508728ea..1ccc5e0ba0c19 100644 --- a/test/functional/data/rpc_psbt.json +++ b/test/functional/data/rpc_psbt.json @@ -38,7 +38,9 @@ "cHNidP8BAF4CAAAAAZvUh2UjC/mnLmYgAflyVW5U8Mb5f+tWvLVgDYF/aZUmAQAAAAD/////AUjmBSoBAAAAIlEgAw2k/OT32yjCyylRYx4ANxOFZZf+ljiCy1AOaBEsymMAAAAAAAEBKwDyBSoBAAAAIlEgwiR++/2SrEf29AuNQtFpF1oZ+p+hDkol1/NetN2FtpJBFCyxOsaCSN6AaqajZZzzwD62gh0JyBFKToaP696GW7bSzZcOFfU/wMgvlQ/VYP+pGbdhcr4Bc2iomROvB09ACwlCiXVqo3OczGiewPzzo2C+MswLWbFuk6Hou0YFcmssp6P/cGxBdmSWMrLMaOH5ErileONxnOdxCIXHqWb0m81DywEBAAA=", "cHNidP8BAF4CAAAAAZvUh2UjC/mnLmYgAflyVW5U8Mb5f+tWvLVgDYF/aZUmAQAAAAD/////AUjmBSoBAAAAIlEgAw2k/OT32yjCyylRYx4ANxOFZZf+ljiCy1AOaBEsymMAAAAAAAEBKwDyBSoBAAAAIlEgwiR++/2SrEf29AuNQtFpF1oZ+p+hDkol1/NetN2FtpJBFCyxOsaCSN6AaqajZZzzwD62gh0JyBFKToaP696GW7bSzZcOFfU/wMgvlQ/VYP+pGbdhcr4Bc2iomROvB09ACwk5iXVqo3OczGiewPzzo2C+MswLWbFuk6Hou0YFcmssp6P/cGxBdmSWMrLMaOH5ErileONxnOdxCIXHqWb0m81DywAA", "cHNidP8BAF4CAAAAAZvUh2UjC/mnLmYgAflyVW5U8Mb5f+tWvLVgDYF/aZUmAQAAAAD/////AUjmBSoBAAAAIlEgAw2k/OT32yjCyylRYx4ANxOFZZf+ljiCy1AOaBEsymMAAAAAAAEBKwDyBSoBAAAAIlEgwiR++/2SrEf29AuNQtFpF1oZ+p+hDkol1/NetN2FtpJjFcFQkpt0waBJVLeLS2A16XpeB4paDyjsltVHv+6azoA6wG99YgWelJehpKJnVp2YdtpgEBr/OONSm5uTnOf5GulwEV8uSQr3zEXE94UR82BXzlxaXFYyWin7RN/CA/NW4fgAIyAssTrGgkjegGqmo2Wc88A+toIdCcgRSk6Gj+vehlu20qzAAAA=", - "cHNidP8BAF4CAAAAAZvUh2UjC/mnLmYgAflyVW5U8Mb5f+tWvLVgDYF/aZUmAQAAAAD/////AUjmBSoBAAAAIlEgAw2k/OT32yjCyylRYx4ANxOFZZf+ljiCy1AOaBEsymMAAAAAAAEBKwDyBSoBAAAAIlEgwiR++/2SrEf29AuNQtFpF1oZ+p+hDkol1/NetN2FtpJhFcFQkpt0waBJVLeLS2A16XpeB4paDyjsltVHv+6azoA6wG99YgWelJehpKJnVp2YdtpgEBr/OONSm5uTnOf5GulwEV8uSQr3zEXE94UR82BXzlxaXFYyWin7RN/CA/NW4SMgLLE6xoJI3oBqpqNlnPPAPraCHQnIEUpOho/r3oZbttKswAAA" + "cHNidP8BAF4CAAAAAZvUh2UjC/mnLmYgAflyVW5U8Mb5f+tWvLVgDYF/aZUmAQAAAAD/////AUjmBSoBAAAAIlEgAw2k/OT32yjCyylRYx4ANxOFZZf+ljiCy1AOaBEsymMAAAAAAAEBKwDyBSoBAAAAIlEgwiR++/2SrEf29AuNQtFpF1oZ+p+hDkol1/NetN2FtpJhFcFQkpt0waBJVLeLS2A16XpeB4paDyjsltVHv+6azoA6wG99YgWelJehpKJnVp2YdtpgEBr/OONSm5uTnOf5GulwEV8uSQr3zEXE94UR82BXzlxaXFYyWin7RN/CA/NW4SMgLLE6xoJI3oBqpqNlnPPAPraCHQnIEUpOho/r3oZbttKswAAA", + "cHNidP8BAHUCAAAAAQCBcTce3/KF6Tet7qSze3gADAVmy7OtZGQXE8pCFxv2AAAAAAD+////AtPf9QUAAAAAGXapFNDFmQPFusKGh2DpD9UhpGZap2UgiKwA4fUFAAAAABepFDVF5uM7gyxHBQ8k0+65PJwDlIvHh7MuEwAAAQD9pQEBAAAAAAECiaPHHqtNIOA3G7ukzGmPopXJRjr6Ljl/hTPMti+VZ+UBAAAAFxYAFL4Y0VKpsBIDna89p95PUzSe7LmF/////4b4qkOnHf8USIk6UwpyN+9rRgi7st0tAXHmOuxqSJC0AQAAABcWABT+Pp7xp0XpdNkCxDVZQ6vLNL1TU/////8CAMLrCwAAAAAZdqkUhc/xCX/Z4Ai7NK9wnGIZeziXikiIrHL++E4sAAAAF6kUM5cluiHv1irHU6m80GfWx6ajnQWHAkcwRAIgJxK+IuAnDzlPVoMR3HyppolwuAJf3TskAinwf4pfOiQCIAGLONfc0xTnNMkna9b7QPZzMlvEuqFEyADS8vAtsnZcASED0uFWdJQbrUqZY3LLh+GFbTZSYG2YVi/jnF6efkE/IQUCSDBFAiEA0SuFLYXc2WHS9fSrZgZU327tzHlMDDPOXMMJ/7X85Y0CIGczio4OFyXBl/saiK9Z9R5E5CVbIBZ8hoQDHAXR8lkqASECI7cr7vCWXRC+B3jv7NYfysb3mk6haTkzgHNEZPhPKrMAAAAAAAAA", + "cHNidP8BAHUCAAAAASaBcTce3/KF6Tet7qSze3gADAVmy7OtZGQXE8pCFxv2AAAAAgD+////AtPf9QUAAAAAGXapFNDFmQPFusKGh2DpD9UhpGZap2UgiKwA4fUFAAAAABepFDVF5uM7gyxHBQ8k0+65PJwDlIvHh7MuEwAAAQD9pQEBAAAAAAECiaPHHqtNIOA3G7ukzGmPopXJRjr6Ljl/hTPMti+VZ+UBAAAAFxYAFL4Y0VKpsBIDna89p95PUzSe7LmF/////4b4qkOnHf8USIk6UwpyN+9rRgi7st0tAXHmOuxqSJC0AQAAABcWABT+Pp7xp0XpdNkCxDVZQ6vLNL1TU/////8CAMLrCwAAAAAZdqkUhc/xCX/Z4Ai7NK9wnGIZeziXikiIrHL++E4sAAAAF6kUM5cluiHv1irHU6m80GfWx6ajnQWHAkcwRAIgJxK+IuAnDzlPVoMR3HyppolwuAJf3TskAinwf4pfOiQCIAGLONfc0xTnNMkna9b7QPZzMlvEuqFEyADS8vAtsnZcASED0uFWdJQbrUqZY3LLh+GFbTZSYG2YVi/jnF6efkE/IQUCSDBFAiEA0SuFLYXc2WHS9fSrZgZU327tzHlMDDPOXMMJ/7X85Y0CIGczio4OFyXBl/saiK9Z9R5E5CVbIBZ8hoQDHAXR8lkqASECI7cr7vCWXRC+B3jv7NYfysb3mk6haTkzgHNEZPhPKrMAAAAAAAAA" ], "invalid_with_msg": [ [ diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index 016aa3ba119bc..4426221006c73 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -701,11 +701,9 @@ def test_psbt_input_keys(psbt_input, keys): assert_equal(analysis['next'], 'creator') assert_equal(analysis['error'], 'PSBT is not valid. Output amount invalid') - analysis = self.nodes[0].analyzepsbt('cHNidP8BAJoCAAAAAkvEW8NnDtdNtDpsmze+Ht2LH35IJcKv00jKAlUs21RrAwAAAAD/////S8Rbw2cO1020OmybN74e3Ysffkglwq/TSMoCVSzbVGsBAAAAAP7///8CwLYClQAAAAAWABSNJKzjaUb3uOxixsvh1GGE3fW7zQD5ApUAAAAAFgAUKNw0x8HRctAgmvoevm4u1SbN7XIAAAAAAAEAnQIAAAACczMa321tVHuN4GKWKRncycI22aX3uXgwSFUKM2orjRsBAAAAAP7///9zMxrfbW1Ue43gYpYpGdzJwjbZpfe5eDBIVQozaiuNGwAAAAAA/v///wIA+QKVAAAAABl2qRT9zXUVA8Ls5iVqynLHe5/vSe1XyYisQM0ClQAAAAAWABRmWQUcjSjghQ8/uH4Bn/zkakwLtAAAAAAAAQEfQM0ClQAAAAAWABRmWQUcjSjghQ8/uH4Bn/zkakwLtAAAAA==') - assert_equal(analysis['next'], 'creator') - assert_equal(analysis['error'], 'PSBT is not valid. Input 0 specifies invalid prevout') + assert_raises_rpc_error(-22, "TX decode failed", self.nodes[0].analyzepsbt, "cHNidP8BAJoCAAAAAkvEW8NnDtdNtDpsmze+Ht2LH35IJcKv00jKAlUs21RrAwAAAAD/////S8Rbw2cO1020OmybN74e3Ysffkglwq/TSMoCVSzbVGsBAAAAAP7///8CwLYClQAAAAAWABSNJKzjaUb3uOxixsvh1GGE3fW7zQD5ApUAAAAAFgAUKNw0x8HRctAgmvoevm4u1SbN7XIAAAAAAAEAnQIAAAACczMa321tVHuN4GKWKRncycI22aX3uXgwSFUKM2orjRsBAAAAAP7///9zMxrfbW1Ue43gYpYpGdzJwjbZpfe5eDBIVQozaiuNGwAAAAAA/v///wIA+QKVAAAAABl2qRT9zXUVA8Ls5iVqynLHe5/vSe1XyYisQM0ClQAAAAAWABRmWQUcjSjghQ8/uH4Bn/zkakwLtAAAAAAAAQEfQM0ClQAAAAAWABRmWQUcjSjghQ8/uH4Bn/zkakwLtAAAAA==") - assert_raises_rpc_error(-25, 'Inputs missing or spent', self.nodes[0].walletprocesspsbt, 'cHNidP8BAJoCAAAAAkvEW8NnDtdNtDpsmze+Ht2LH35IJcKv00jKAlUs21RrAwAAAAD/////S8Rbw2cO1020OmybN74e3Ysffkglwq/TSMoCVSzbVGsBAAAAAP7///8CwLYClQAAAAAWABSNJKzjaUb3uOxixsvh1GGE3fW7zQD5ApUAAAAAFgAUKNw0x8HRctAgmvoevm4u1SbN7XIAAAAAAAEAnQIAAAACczMa321tVHuN4GKWKRncycI22aX3uXgwSFUKM2orjRsBAAAAAP7///9zMxrfbW1Ue43gYpYpGdzJwjbZpfe5eDBIVQozaiuNGwAAAAAA/v///wIA+QKVAAAAABl2qRT9zXUVA8Ls5iVqynLHe5/vSe1XyYisQM0ClQAAAAAWABRmWQUcjSjghQ8/uH4Bn/zkakwLtAAAAAAAAQEfQM0ClQAAAAAWABRmWQUcjSjghQ8/uH4Bn/zkakwLtAAAAA==') + assert_raises_rpc_error(-22, "TX decode failed", self.nodes[0].walletprocesspsbt, "cHNidP8BAJoCAAAAAkvEW8NnDtdNtDpsmze+Ht2LH35IJcKv00jKAlUs21RrAwAAAAD/////S8Rbw2cO1020OmybN74e3Ysffkglwq/TSMoCVSzbVGsBAAAAAP7///8CwLYClQAAAAAWABSNJKzjaUb3uOxixsvh1GGE3fW7zQD5ApUAAAAAFgAUKNw0x8HRctAgmvoevm4u1SbN7XIAAAAAAAEAnQIAAAACczMa321tVHuN4GKWKRncycI22aX3uXgwSFUKM2orjRsBAAAAAP7///9zMxrfbW1Ue43gYpYpGdzJwjbZpfe5eDBIVQozaiuNGwAAAAAA/v///wIA+QKVAAAAABl2qRT9zXUVA8Ls5iVqynLHe5/vSe1XyYisQM0ClQAAAAAWABRmWQUcjSjghQ8/uH4Bn/zkakwLtAAAAAAAAQEfQM0ClQAAAAAWABRmWQUcjSjghQ8/uH4Bn/zkakwLtAAAAA==") self.log.info("Test that we can fund psbts with external inputs specified") From e912717ff63f111d8f1cd7ed1fcf054e28f36409 Mon Sep 17 00:00:00 2001 From: umiumi <9@umi.cat> Date: Wed, 24 Apr 2024 09:51:01 +0800 Subject: [PATCH 038/868] test: add missing comparison of node1's mempool in MempoolPackagesTest --- test/functional/mempool_packages.py | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/test/functional/mempool_packages.py b/test/functional/mempool_packages.py index dcb66b2ca1add..e83c62915e865 100755 --- a/test/functional/mempool_packages.py +++ b/test/functional/mempool_packages.py @@ -199,13 +199,13 @@ def run_test(self): assert set(mempool1).issubset(set(mempool0)) for tx in chain[:CUSTOM_ANCESTOR_LIMIT]: assert tx in mempool1 - # TODO: more detailed check of node1's mempool (fees etc.) - # check transaction unbroadcast info (should be false if in both mempools) - mempool = self.nodes[0].getrawmempool(True) - for tx in mempool: - assert_equal(mempool[tx]['unbroadcast'], False) - - # TODO: test ancestor size limits + entry0 = self.nodes[0].getmempoolentry(tx) + entry1 = self.nodes[1].getmempoolentry(tx) + assert not entry0['unbroadcast'] + assert not entry1['unbroadcast'] + assert_equal(entry1['fees']['base'], entry0['fees']['base']) + assert_equal(entry1['vsize'], entry0['vsize']) + assert_equal(entry1['depends'], entry0['depends']) # Now test descendant chain limits @@ -251,10 +251,14 @@ def run_test(self): assert tx in mempool1 for tx in chain[CUSTOM_DESCENDANT_LIMIT:]: assert tx not in mempool1 - # TODO: more detailed check of node1's mempool (fees etc.) - - # TODO: test descendant size limits - + for tx in mempool1: + entry0 = self.nodes[0].getmempoolentry(tx) + entry1 = self.nodes[1].getmempoolentry(tx) + assert not entry0['unbroadcast'] + assert not entry1['unbroadcast'] + assert_equal(entry1['fees']['base'], entry0['fees']['base']) + assert_equal(entry1['vsize'], entry0['vsize']) + assert_equal(entry1['depends'], entry0['depends']) # Test reorg handling # First, the basics: self.generate(self.nodes[0], 1) From 7c69baf227252511455bc06e315f6a3c7fc5a398 Mon Sep 17 00:00:00 2001 From: Max Edwards Date: Thu, 25 Apr 2024 14:00:10 +0100 Subject: [PATCH 039/868] depends: pass verbose through to cmake based make When running depends with V=1 this will now enable verbose output from makefiles generated by cmake. --- depends/funcs.mk | 1 + 1 file changed, 1 insertion(+) diff --git a/depends/funcs.mk b/depends/funcs.mk index 494ed5d32457e..d8a72421b4abb 100644 --- a/depends/funcs.mk +++ b/depends/funcs.mk @@ -182,6 +182,7 @@ $(1)_cmake=env CC="$$($(1)_cc)" \ cmake -DCMAKE_INSTALL_PREFIX:PATH="$$($($(1)_type)_prefix)" \ -DCMAKE_INSTALL_LIBDIR=lib/ \ -DCMAKE_POSITION_INDEPENDENT_CODE=ON \ + -DCMAKE_VERBOSE_MAKEFILE:BOOL=$(V) \ $$($(1)_config_opts) ifeq ($($(1)_type),build) $(1)_cmake += -DCMAKE_INSTALL_RPATH:PATH="$$($($(1)_type)_prefix)/lib" From 6abe772a17e09fe96e68cd3311280d5a30f6378b Mon Sep 17 00:00:00 2001 From: Fabian Jahr Date: Wed, 1 Nov 2023 20:05:18 +0100 Subject: [PATCH 040/868] contrib: Add asmap-tool Co-authored-by: Pieter Wuille --- contrib/asmap/README.md | 12 +++ contrib/asmap/asmap-tool.py | 156 ++++++++++++++++++++++++++++++ contrib/{seeds => asmap}/asmap.py | 6 +- contrib/seeds/makeseeds.py | 5 +- 4 files changed, 176 insertions(+), 3 deletions(-) create mode 100644 contrib/asmap/README.md create mode 100755 contrib/asmap/asmap-tool.py rename contrib/{seeds => asmap}/asmap.py (99%) diff --git a/contrib/asmap/README.md b/contrib/asmap/README.md new file mode 100644 index 0000000000000..5fab4b285e213 --- /dev/null +++ b/contrib/asmap/README.md @@ -0,0 +1,12 @@ +# ASMap Tool + +Tool for performing various operations on textual and binary asmap files, +particularly encoding/compressing the raw data to the binary format that can +be used in Bitcoin Core with the `-asmap` option. + +Example usage: +``` +python3 asmap-tool.py encode /path/to/input.file /path/to/output.file +python3 asmap-tool.py decode /path/to/input.file /path/to/output.file +python3 asmap-tool.py diff /path/to/first.file /path/to/second.file +``` diff --git a/contrib/asmap/asmap-tool.py b/contrib/asmap/asmap-tool.py new file mode 100755 index 0000000000000..09c28725e4bb8 --- /dev/null +++ b/contrib/asmap/asmap-tool.py @@ -0,0 +1,156 @@ +#!/usr/bin/env python3 +# Copyright (c) 2022 Pieter Wuille +# Distributed under the MIT software license, see the accompanying +# file LICENSE or http://www.opensource.org/licenses/mit-license.php. + +import argparse +import sys +import ipaddress +import math + +import asmap + +def load_file(input_file): + try: + contents = input_file.read() + except OSError as err: + sys.exit(f"Input file '{input_file.name}' cannot be read: {err.strerror}.") + try: + bin_asmap = asmap.ASMap.from_binary(contents) + except ValueError: + bin_asmap = None + txt_error = None + entries = None + try: + txt_contents = str(contents, encoding="utf-8") + except UnicodeError: + txt_error = "invalid UTF-8" + txt_contents = None + if txt_contents is not None: + entries = [] + for line in txt_contents.split("\n"): + idx = line.find('#') + if idx >= 0: + line = line[:idx] + line = line.lstrip(' ').rstrip(' \t\r\n') + if len(line) == 0: + continue + fields = line.split(' ') + if len(fields) != 2: + txt_error = f"unparseable line '{line}'" + entries = None + break + prefix, asn = fields + if len(asn) <= 2 or asn[:2] != "AS" or any(c < '0' or c > '9' for c in asn[2:]): + txt_error = f"invalid ASN '{asn}'" + entries = None + break + try: + net = ipaddress.ip_network(prefix) + except ValueError: + txt_error = f"invalid network '{prefix}'" + entries = None + break + entries.append((asmap.net_to_prefix(net), int(asn[2:]))) + if entries is not None and bin_asmap is not None and len(contents) > 0: + sys.exit(f"Input file '{input_file.name}' is ambiguous.") + if entries is not None: + state = asmap.ASMap() + state.update_multi(entries) + return state + if bin_asmap is not None: + return bin_asmap + sys.exit(f"Input file '{input_file.name}' is neither a valid binary asmap file nor valid text input ({txt_error}).") + + +def save_binary(output_file, state, fill): + contents = state.to_binary(fill=fill) + try: + output_file.write(contents) + output_file.close() + except OSError as err: + sys.exit(f"Output file '{output_file.name}' cannot be written to: {err.strerror}.") + +def save_text(output_file, state, fill, overlapping): + for prefix, asn in state.to_entries(fill=fill, overlapping=overlapping): + net = asmap.prefix_to_net(prefix) + try: + print(f"{net} AS{asn}", file=output_file) + except OSError as err: + sys.exit(f"Output file '{output_file.name}' cannot be written to: {err.strerror}.") + try: + output_file.close() + except OSError as err: + sys.exit(f"Output file '{output_file.name}' cannot be written to: {err.strerror}.") + +def main(): + parser = argparse.ArgumentParser(description="Tool for performing various operations on textual and binary asmap files.") + subparsers = parser.add_subparsers(title="valid subcommands", dest="subcommand") + + parser_encode = subparsers.add_parser("encode", help="convert asmap data to binary format") + parser_encode.add_argument('-f', '--fill', dest="fill", default=False, action="store_true", + help="permit reassigning undefined network ranges arbitrarily to reduce size") + parser_encode.add_argument('infile', nargs='?', type=argparse.FileType('rb'), default=sys.stdin.buffer, + help="input asmap file (text or binary); default is stdin") + parser_encode.add_argument('outfile', nargs='?', type=argparse.FileType('wb'), default=sys.stdout.buffer, + help="output binary asmap file; default is stdout") + + parser_decode = subparsers.add_parser("decode", help="convert asmap data to text format") + parser_decode.add_argument('-f', '--fill', dest="fill", default=False, action="store_true", + help="permit reassigning undefined network ranges arbitrarily to reduce length") + parser_decode.add_argument('-n', '--nonoverlapping', dest="overlapping", default=True, action="store_false", + help="output strictly non-overall ping network ranges (increases output size)") + parser_decode.add_argument('infile', nargs='?', type=argparse.FileType('rb'), default=sys.stdin.buffer, + help="input asmap file (text or binary); default is stdin") + parser_decode.add_argument('outfile', nargs='?', type=argparse.FileType('w'), default=sys.stdout, + help="output text file; default is stdout") + + parser_diff = subparsers.add_parser("diff", help="compute the difference between two asmap files") + parser_diff.add_argument('-i', '--ignore-unassigned', dest="ignore_unassigned", default=False, action="store_true", + help="ignore unassigned ranges in the first input (useful when second input is filled)") + parser_diff.add_argument('infile1', type=argparse.FileType('rb'), + help="first file to compare (text or binary)") + parser_diff.add_argument('infile2', type=argparse.FileType('rb'), + help="second file to compare (text or binary)") + + args = parser.parse_args() + if args.subcommand is None: + parser.print_help() + elif args.subcommand == "encode": + state = load_file(args.infile) + save_binary(args.outfile, state, fill=args.fill) + elif args.subcommand == "decode": + state = load_file(args.infile) + save_text(args.outfile, state, fill=args.fill, overlapping=args.overlapping) + elif args.subcommand == "diff": + state1 = load_file(args.infile1) + state2 = load_file(args.infile2) + ipv4_changed = 0 + ipv6_changed = 0 + for prefix, old_asn, new_asn in state1.diff(state2): + if args.ignore_unassigned and old_asn == 0: + continue + net = asmap.prefix_to_net(prefix) + if isinstance(net, ipaddress.IPv4Network): + ipv4_changed += 1 << (32 - net.prefixlen) + elif isinstance(net, ipaddress.IPv6Network): + ipv6_changed += 1 << (128 - net.prefixlen) + if new_asn == 0: + print(f"# {net} was AS{old_asn}") + elif old_asn == 0: + print(f"{net} AS{new_asn} # was unassigned") + else: + print(f"{net} AS{new_asn} # was AS{old_asn}") + ipv4_change_str = "" if ipv4_changed == 0 else f" (2^{math.log2(ipv4_changed):.2f})" + ipv6_change_str = "" if ipv6_changed == 0 else f" (2^{math.log2(ipv6_changed):.2f})" + + print( + f"# {ipv4_changed}{ipv4_change_str} IPv4 addresses changed; " + f"{ipv6_changed}{ipv6_change_str} IPv6 addresses changed" + ) + else: + parser.print_help() + sys.exit("No command provided.") + +if __name__ == '__main__': + main() diff --git a/contrib/seeds/asmap.py b/contrib/asmap/asmap.py similarity index 99% rename from contrib/seeds/asmap.py rename to contrib/asmap/asmap.py index 214805b5a5790..2ae84a3f3119b 100644 --- a/contrib/seeds/asmap.py +++ b/contrib/asmap/asmap.py @@ -489,12 +489,14 @@ def candidate(ctx: Optional[int], arg1, arg2, func: Callable): if ctx not in ret or cand.size < ret[ctx].size: ret[ctx] = cand - for ctx in set(left) | set(right): + union = set(left) | set(right) + sorted_union = sorted(union, key=lambda x: (x is None, x)) + for ctx in sorted_union: candidate(ctx, left.get(ctx), right.get(ctx), _BinNode.make_branch) candidate(ctx, left.get(None), right.get(ctx), _BinNode.make_branch) candidate(ctx, left.get(ctx), right.get(None), _BinNode.make_branch) if not hole: - for ctx in set(ret) - set([None]): + for ctx in sorted(set(ret) - set([None])): candidate(None, ctx, ret[ctx], _BinNode.make_default) if None in ret: ret = {ctx:enc for ctx, enc in ret.items() diff --git a/contrib/seeds/makeseeds.py b/contrib/seeds/makeseeds.py index f03c2ab5e808f..fd8c7511d0947 100755 --- a/contrib/seeds/makeseeds.py +++ b/contrib/seeds/makeseeds.py @@ -9,11 +9,14 @@ import argparse import collections import ipaddress +from pathlib import Path import re import sys from typing import Union -from asmap import ASMap, net_to_prefix +asmap_dir = Path(__file__).parent.parent / "asmap" +sys.path.append(str(asmap_dir)) +from asmap import ASMap, net_to_prefix # noqa: E402 NSEEDS=512 From b259b0e8d360726b062c4b0453d1cf5a68e1933f Mon Sep 17 00:00:00 2001 From: Alfonso Roman Zubeldia Date: Fri, 26 Apr 2024 10:50:00 -0300 Subject: [PATCH 041/868] [Test] Assumeutxo: ensure failure when importing a snapshot twice --- test/functional/feature_assumeutxo.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/functional/feature_assumeutxo.py b/test/functional/feature_assumeutxo.py index 19cbbcffdb592..ffbbfc89499f8 100755 --- a/test/functional/feature_assumeutxo.py +++ b/test/functional/feature_assumeutxo.py @@ -395,6 +395,10 @@ def check_tx_counts(final: bool) -> None: assert_equal(snapshot['snapshot_blockhash'], dump_output['base_hash']) assert_equal(snapshot['validated'], False) + self.log.info("Check that loading the snapshot again will fail because there is already an active snapshot.") + with n2.assert_debug_log(expected_msgs=["[snapshot] can't activate a snapshot-based chainstate more than once"]): + assert_raises_rpc_error(-32603, "Unable to load UTXO snapshot", n2.loadtxoutset, dump_output['path']) + self.connect_nodes(0, 2) self.wait_until(lambda: n2.getchainstates()['chainstates'][-1]['blocks'] == FINAL_HEIGHT) self.sync_blocks() From a8d9a0edc7cef2c31a557ef53eb45520976b0d65 Mon Sep 17 00:00:00 2001 From: Sergi Delgado Segura Date: Wed, 20 Dec 2023 10:53:49 -0500 Subject: [PATCH 042/868] test: adds outbound eviction functional tests, updates comment in ConsiderEviction --- src/net_processing.cpp | 11 +- test/functional/p2p_outbound_eviction.py | 224 +++++++++++++++++++++++ test/functional/test_runner.py | 1 + 3 files changed, 232 insertions(+), 4 deletions(-) create mode 100755 test/functional/p2p_outbound_eviction.py diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 39ffff97d2bdc..f936f1b666cdf 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -5165,16 +5165,19 @@ void PeerManagerImpl::ConsiderEviction(CNode& pto, Peer& peer, std::chrono::seco // unless it's invalid, in which case we should find that out and // disconnect from them elsewhere). if (state.pindexBestKnownBlock != nullptr && state.pindexBestKnownBlock->nChainWork >= m_chainman.ActiveChain().Tip()->nChainWork) { + // The outbound peer has sent us a block with at least as much work as our current tip, so reset the timeout if it was set if (state.m_chain_sync.m_timeout != 0s) { state.m_chain_sync.m_timeout = 0s; state.m_chain_sync.m_work_header = nullptr; state.m_chain_sync.m_sent_getheaders = false; } } else if (state.m_chain_sync.m_timeout == 0s || (state.m_chain_sync.m_work_header != nullptr && state.pindexBestKnownBlock != nullptr && state.pindexBestKnownBlock->nChainWork >= state.m_chain_sync.m_work_header->nChainWork)) { - // Our best block known by this peer is behind our tip, and we're either noticing - // that for the first time, OR this peer was able to catch up to some earlier point - // where we checked against our tip. - // Either way, set a new timeout based on current tip. + // At this point we know that the outbound peer has either never sent us a block/header or they have, but its tip is behind ours + // AND + // we are noticing this for the first time (m_timeout is 0) + // OR we noticed this at some point within the last CHAIN_SYNC_TIMEOUT + HEADERS_RESPONSE_TIME seconds and set a timeout + // for them, they caught up to our tip at the time of setting the timer but not to our current one (we've also advanced). + // Either way, set a new timeout based on our current tip. state.m_chain_sync.m_timeout = time_in_seconds + CHAIN_SYNC_TIMEOUT; state.m_chain_sync.m_work_header = m_chainman.ActiveChain().Tip(); state.m_chain_sync.m_sent_getheaders = false; diff --git a/test/functional/p2p_outbound_eviction.py b/test/functional/p2p_outbound_eviction.py new file mode 100755 index 0000000000000..43dec4df83aba --- /dev/null +++ b/test/functional/p2p_outbound_eviction.py @@ -0,0 +1,224 @@ +#!/usr/bin/env python3 +# Copyright (c) 2019-2021 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 outbound peer eviction logic + +A subset of our outbound peers are subject to eviction logic if they cannot keep up +with our vision of the best chain. This criteria applies only to non-protected peers, +and can be triggered by either not learning about any blocks from an outbound peer after +a certain deadline, or by them not being able to catch up fast enough (under the same deadline). + +This tests the different eviction paths based on the peer's behavior and on whether they are protected +or not. +""" +import time + +from test_framework.messages import ( + from_hex, + msg_headers, + CBlockHeader, +) +from test_framework.p2p import P2PInterface +from test_framework.test_framework import BitcoinTestFramework + +# Timeouts (in seconds) +CHAIN_SYNC_TIMEOUT = 20 * 60 +HEADERS_RESPONSE_TIME = 2 * 60 + + +class P2POutEvict(BitcoinTestFramework): + def set_test_params(self): + self.num_nodes = 1 + + def test_outbound_eviction_unprotected(self): + # This tests the eviction logic for **unprotected** outbound peers (that is, PeerManagerImpl::ConsiderEviction) + node = self.nodes[0] + cur_mock_time = node.mocktime + + # Get our tip header and its parent + tip_header = from_hex(CBlockHeader(), node.getblockheader(node.getbestblockhash(), False)) + prev_header = from_hex(CBlockHeader(), node.getblockheader(f"{tip_header.hashPrevBlock:064x}", False)) + + self.log.info("Create an outbound connection and don't send any headers") + # Test disconnect due to no block being announced in 22+ minutes (headers are not even exchanged) + peer = node.add_outbound_p2p_connection(P2PInterface(), p2p_idx=0, connection_type="outbound-full-relay") + # Wait for over 20 min to trigger the first eviction timeout. This sets the last call past 2 min in the future. + cur_mock_time += (CHAIN_SYNC_TIMEOUT + 1) + node.setmocktime(cur_mock_time) + peer.sync_with_ping() + # Wait for over 2 more min to trigger the disconnection + peer.wait_for_getheaders(block_hash=tip_header.hashPrevBlock) + cur_mock_time += (HEADERS_RESPONSE_TIME + 1) + node.setmocktime(cur_mock_time) + self.log.info("Test that the peer gets evicted") + peer.wait_for_disconnect() + + self.log.info("Create an outbound connection and send header but never catch up") + # Mimic a node that just falls behind for long enough + # This should also apply for a node doing IBD that does not catch up in time + # Connect a peer and make it send us headers ending in our tip's parent + peer = node.add_outbound_p2p_connection(P2PInterface(), p2p_idx=0, connection_type="outbound-full-relay") + peer.send_and_ping(msg_headers([prev_header])) + + # Trigger the timeouts + cur_mock_time += (CHAIN_SYNC_TIMEOUT + 1) + node.setmocktime(cur_mock_time) + peer.sync_with_ping() + peer.wait_for_getheaders(block_hash=tip_header.hashPrevBlock) + cur_mock_time += (HEADERS_RESPONSE_TIME + 1) + node.setmocktime(cur_mock_time) + self.log.info("Test that the peer gets evicted") + peer.wait_for_disconnect() + + self.log.info("Create an outbound connection and keep lagging behind, but not too much") + # Test that if the peer never catches up with our current tip, but it does with the + # expected work that we set when setting the timer (that is, our tip at the time) + # we do not disconnect the peer + peer = node.add_outbound_p2p_connection(P2PInterface(), p2p_idx=0, connection_type="outbound-full-relay") + + self.log.info("Mine a block so our peer starts lagging") + prev_prev_hash = tip_header.hashPrevBlock + best_block_hash = self.generateblock(node, output="raw(42)", transactions=[])["hash"] + peer.sync_with_ping() + + self.log.info("Keep catching up with the old tip and check that we are not evicted") + for i in range(10): + # Generate an additional block so the peers is 2 blocks behind + prev_header = from_hex(CBlockHeader(), node.getblockheader(best_block_hash, False)) + best_block_hash = self.generateblock(node, output="raw(42)", transactions=[])["hash"] + peer.sync_with_ping() + + # Advance time but not enough to evict the peer + cur_mock_time += (CHAIN_SYNC_TIMEOUT + 1) + node.setmocktime(cur_mock_time) + peer.sync_with_ping() + + # Wait until we get out last call (by receiving a getheaders) + peer.wait_for_getheaders(block_hash=prev_prev_hash) + + # Send a header with the previous tip (so we go back to 1 block behind) + peer.send_and_ping(msg_headers([prev_header])) + prev_prev_hash = tip_header.hash + + self.log.info("Create an outbound connection and take some time to catch up, but do it in time") + # Check that if the peer manages to catch up within time, the timeouts are removed (and the peer is not disconnected) + # We are reusing the peer from the previous case which already sent us a valid (but old) block and whose timer is ticking + + # Send an updated headers message matching our tip + peer.send_and_ping(msg_headers([from_hex(CBlockHeader(), node.getblockheader(best_block_hash, False))])) + + # Wait for long enough for the timeouts to have triggered and check that we are still connected + cur_mock_time += (CHAIN_SYNC_TIMEOUT + 1) + node.setmocktime(cur_mock_time) + peer.sync_with_ping() + cur_mock_time += (HEADERS_RESPONSE_TIME + 1) + node.setmocktime(cur_mock_time) + self.log.info("Test that the peer does not get evicted") + peer.sync_with_ping() + + node.disconnect_p2ps() + + def test_outbound_eviction_protected(self): + # This tests the eviction logic for **protected** outbound peers (that is, PeerManagerImpl::ConsiderEviction) + # Outbound connections are flagged as protected as long as they have sent us a connecting block with at least as + # much work as our current tip and we have enough empty protected_peers slots. + node = self.nodes[0] + cur_mock_time = node.mocktime + tip_header = from_hex(CBlockHeader(), node.getblockheader(node.getbestblockhash(), False)) + + self.log.info("Create an outbound connection to a peer that shares our tip so it gets granted protection") + peer = node.add_outbound_p2p_connection(P2PInterface(), p2p_idx=0, connection_type="outbound-full-relay") + peer.send_and_ping(msg_headers([tip_header])) + + self.log.info("Mine a new block and sync with our peer") + self.generateblock(node, output="raw(42)", transactions=[]) + peer.sync_with_ping() + + self.log.info("Let enough time pass for the timeouts to go off") + # Trigger the timeouts and check how we are still connected + cur_mock_time += (CHAIN_SYNC_TIMEOUT + 1) + node.setmocktime(cur_mock_time) + peer.sync_with_ping() + peer.wait_for_getheaders(block_hash=tip_header.hashPrevBlock) + cur_mock_time += (HEADERS_RESPONSE_TIME + 1) + node.setmocktime(cur_mock_time) + self.log.info("Test that the node does not get evicted") + peer.sync_with_ping() + + node.disconnect_p2ps() + + def test_outbound_eviction_mixed(self): + # This tests the outbound eviction logic for a mix of protected and unprotected peers. + node = self.nodes[0] + cur_mock_time = node.mocktime + + self.log.info("Create a mix of protected and unprotected outbound connections to check against eviction") + + # Let's try this logic having multiple peers, some protected and some unprotected + # We protect up to 4 peers as long as they have provided a block with the same amount of work as our tip + self.log.info("The first 4 peers are protected by sending us a valid block with enough work") + tip_header = from_hex(CBlockHeader(), node.getblockheader(node.getbestblockhash(), False)) + headers_message = msg_headers([tip_header]) + protected_peers = [] + for i in range(4): + peer = node.add_outbound_p2p_connection(P2PInterface(), p2p_idx=i, connection_type="outbound-full-relay") + peer.send_and_ping(headers_message) + protected_peers.append(peer) + + # We can create 4 additional outbound connections to peers that are unprotected. 2 of them will be well behaved, + # whereas the other 2 will misbehave (1 sending no headers, 1 sending old ones) + self.log.info("The remaining 4 peers will be mixed between honest (2) and misbehaving peers (2)") + prev_header = from_hex(CBlockHeader(), node.getblockheader(f"{tip_header.hashPrevBlock:064x}", False)) + headers_message = msg_headers([prev_header]) + honest_unprotected_peers = [] + for i in range(2): + peer = node.add_outbound_p2p_connection(P2PInterface(), p2p_idx=4+i, connection_type="outbound-full-relay") + peer.send_and_ping(headers_message) + honest_unprotected_peers.append(peer) + + misbehaving_unprotected_peers = [] + for i in range(2): + peer = node.add_outbound_p2p_connection(P2PInterface(), p2p_idx=6+i, connection_type="outbound-full-relay") + if i%2==0: + peer.send_and_ping(headers_message) + misbehaving_unprotected_peers.append(peer) + + self.log.info("Mine a new block and keep the unprotected honest peer on sync, all the rest off-sync") + # Mine a block so all peers become outdated + target_hash = prev_header.rehash() + tip_hash = self.generateblock(node, output="raw(42)", transactions=[])["hash"] + tip_header = from_hex(CBlockHeader(), node.getblockheader(tip_hash, False)) + tip_headers_message = msg_headers([tip_header]) + + # Let the timeouts hit and check back + cur_mock_time += (CHAIN_SYNC_TIMEOUT + 1) + node.setmocktime(cur_mock_time) + for peer in protected_peers + misbehaving_unprotected_peers: + peer.sync_with_ping() + peer.wait_for_getheaders(block_hash=target_hash) + for peer in honest_unprotected_peers: + peer.send_and_ping(tip_headers_message) + peer.wait_for_getheaders(block_hash=target_hash) + + cur_mock_time += (HEADERS_RESPONSE_TIME + 1) + node.setmocktime(cur_mock_time) + self.log.info("Check how none of the honest nor protected peers was evicted but all the misbehaving unprotected were") + for peer in protected_peers + honest_unprotected_peers: + peer.sync_with_ping() + for peer in misbehaving_unprotected_peers: + peer.wait_for_disconnect() + + node.disconnect_p2ps() + + + def run_test(self): + self.nodes[0].setmocktime(int(time.time())) + self.test_outbound_eviction_unprotected() + self.test_outbound_eviction_protected() + self.test_outbound_eviction_mixed() + + +if __name__ == '__main__': + P2POutEvict().main() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 32b55813a8c7d..ec9096f92915c 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -292,6 +292,7 @@ 'p2p_leak_tx.py --v1transport', 'p2p_leak_tx.py --v2transport', 'p2p_eviction.py', + 'p2p_outbound_eviction.py', 'p2p_ibd_stalling.py --v1transport', 'p2p_ibd_stalling.py --v2transport', 'p2p_net_deadlock.py --v1transport', From d53d84834747c37f4060a9ef379e0a6b50155246 Mon Sep 17 00:00:00 2001 From: Sergi Delgado Segura Date: Fri, 1 Mar 2024 11:08:39 -0500 Subject: [PATCH 043/868] test: adds outbound eviction tests for non outbound-full-relay peers Peer protection is only given to outbound-full-relay peers. Add a negative test to check that other type of outbound peers are not given protection under the circumstances that outbound-full-relay would --- test/functional/p2p_outbound_eviction.py | 29 ++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/test/functional/p2p_outbound_eviction.py b/test/functional/p2p_outbound_eviction.py index 43dec4df83aba..8d8968899900e 100755 --- a/test/functional/p2p_outbound_eviction.py +++ b/test/functional/p2p_outbound_eviction.py @@ -212,12 +212,41 @@ def test_outbound_eviction_mixed(self): node.disconnect_p2ps() + def test_outbound_eviction_blocks_relay_only(self): + # The logic for outbound eviction protection only applies to outbound-full-relay peers + # This tests that other types of peers (blocks-relay-only for instance) are not granted protection + node = self.nodes[0] + cur_mock_time = node.mocktime + tip_header = from_hex(CBlockHeader(), node.getblockheader(node.getbestblockhash(), False)) + + self.log.info("Create an blocks-only outbound connection to a peer that shares our tip. This would usually grant protection") + peer = node.add_outbound_p2p_connection(P2PInterface(), p2p_idx=0, connection_type="block-relay-only") + peer.send_and_ping(msg_headers([tip_header])) + + self.log.info("Mine a new block and sync with our peer") + self.generateblock(node, output="raw(42)", transactions=[]) + peer.sync_with_ping() + + self.log.info("Let enough time pass for the timeouts to go off") + # Trigger the timeouts and check how the peer gets evicted, since protection is only given to outbound-full-relay peers + cur_mock_time += (CHAIN_SYNC_TIMEOUT + 1) + node.setmocktime(cur_mock_time) + peer.sync_with_ping() + peer.wait_for_getheaders(block_hash=tip_header.hash) + cur_mock_time += (HEADERS_RESPONSE_TIME + 1) + node.setmocktime(cur_mock_time) + self.log.info("Test that the peer gets evicted") + peer.wait_for_disconnect() + + node.disconnect_p2ps() + def run_test(self): self.nodes[0].setmocktime(int(time.time())) self.test_outbound_eviction_unprotected() self.test_outbound_eviction_protected() self.test_outbound_eviction_mixed() + self.test_outbound_eviction_blocks_relay_only() if __name__ == '__main__': From d5a631b9597e5029a5048d9b8ad84ea4536bbac0 Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Wed, 23 Aug 2023 14:05:42 -0400 Subject: [PATCH 044/868] validation: improve performance of CheckBlockIndex by not saving all indexes in a std::multimap, but only those that are not part of the best header chain. The indexes of the best header chain are stored in a vector, which, in the typical case of a mostly linear chain with a few forks, results in a much smaller multimap, and increases performance noticeably for long chains. This does not change the actual consistency checks that are being performed for each index, just the way the block index tree is stored and traversed. Co-authored-by: Ryan Ofsky --- src/validation.cpp | 50 +++++++++++++++++++++++++++++++++------------- 1 file changed, 36 insertions(+), 14 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index b6d0c38f391da..9dad39e6579e5 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -5050,19 +5050,28 @@ void ChainstateManager::CheckBlockIndex() return; } - // Build forward-pointing map of the entire block tree. + // Build forward-pointing data structure for the entire block tree. + // For performance reasons, indexes of the best header chain are stored in a vector (within CChain). + // All remaining blocks are stored in a multimap. + // The best header chain can differ from the active chain: E.g. its entries may belong to blocks that + // are not yet validated. + CChain best_hdr_chain; + assert(m_best_header); + best_hdr_chain.SetTip(*m_best_header); + std::multimap forward; for (auto& [_, block_index] : m_blockman.m_block_index) { - forward.emplace(block_index.pprev, &block_index); + // Only save indexes in forward that are not part of the best header chain. + if (!best_hdr_chain.Contains(&block_index)) { + // Only genesis, which must be part of the best header chain, can have a nullptr parent. + assert(block_index.pprev); + forward.emplace(block_index.pprev, &block_index); + } } + assert(forward.size() + best_hdr_chain.Height() + 1 == m_blockman.m_block_index.size()); - assert(forward.size() == m_blockman.m_block_index.size()); - - std::pair::iterator,std::multimap::iterator> rangeGenesis = forward.equal_range(nullptr); - CBlockIndex *pindex = rangeGenesis.first->second; - rangeGenesis.first++; - assert(rangeGenesis.first == rangeGenesis.second); // There is only one index entry with parent nullptr. - + CBlockIndex* pindex = best_hdr_chain[0]; + assert(pindex); // Iterate over the entire block tree, using depth-first search. // Along the way, remember whether there are blocks on the path from genesis // block being explored which are the first to have certain properties. @@ -5274,14 +5283,21 @@ void ChainstateManager::CheckBlockIndex() // assert(pindex->GetBlockHash() == pindex->GetBlockHeader().GetHash()); // Perhaps too slow // End: actual consistency checks. - // Try descending into the first subnode. + + // Try descending into the first subnode. Always process forks first and the best header chain after. snap_update_firsts(); std::pair::iterator,std::multimap::iterator> range = forward.equal_range(pindex); if (range.first != range.second) { - // A subnode was found. + // A subnode not part of the best header chain was found. pindex = range.first->second; nHeight++; continue; + } else if (best_hdr_chain.Contains(pindex)) { + // Descend further into best header chain. + nHeight++; + pindex = best_hdr_chain[nHeight]; + if (!pindex) break; // we are finished, since the best header chain is always processed last + continue; } // This is a leaf node. // Move upwards until we reach a node of which we have not yet visited the last child. @@ -5307,9 +5323,15 @@ void ChainstateManager::CheckBlockIndex() // Proceed to the next one. rangePar.first++; if (rangePar.first != rangePar.second) { - // Move to the sibling. + // Move to a sibling not part of the best header chain. pindex = rangePar.first->second; break; + } else if (pindexPar == best_hdr_chain[nHeight - 1]) { + // Move to pindex's sibling on the best-chain, if it has one. + pindex = best_hdr_chain[nHeight]; + // There will not be a next block if (and only if) parent block is the best header. + assert((pindex == nullptr) == (pindexPar == best_hdr_chain.Tip())); + break; } else { // Move up further. pindex = pindexPar; @@ -5319,8 +5341,8 @@ void ChainstateManager::CheckBlockIndex() } } - // Check that we actually traversed the entire map. - assert(nNodes == forward.size()); + // Check that we actually traversed the entire block index. + assert(nNodes == forward.size() + best_hdr_chain.Height() + 1); } std::string Chainstate::ToString() From 5bc2077e8f592442b089affdf0b5795fbc053bb8 Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Thu, 21 Dec 2023 17:24:07 -0500 Subject: [PATCH 045/868] validation: allow to specify frequency for -checkblockindex This makes it similar to -checkaddrman and -checkmempool, which also allow to run the check occasionally instead of always / never. Co-authored-by: Ryan Ofsky --- src/init.cpp | 2 +- src/kernel/chainstatemanager_opts.h | 2 +- src/node/chainstatemanager_args.cpp | 5 ++++- src/test/util/setup_common.cpp | 2 +- src/validation.cpp | 8 ++++++++ src/validation.h | 2 +- 6 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 0aa04755cbeba..927786e3d5950 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -604,7 +604,7 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-checkblocks=", strprintf("How many blocks to check at startup (default: %u, 0 = all)", DEFAULT_CHECKBLOCKS), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-checklevel=", strprintf("How thorough the block verification of -checkblocks is: %s (0-4, default: %u)", Join(CHECKLEVEL_DOC, ", "), DEFAULT_CHECKLEVEL), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); - argsman.AddArg("-checkblockindex", strprintf("Do a consistency check for the block tree, chainstate, and other validation data structures occasionally. (default: %u, regtest: %u)", defaultChainParams->DefaultConsistencyChecks(), regtestChainParams->DefaultConsistencyChecks()), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); + argsman.AddArg("-checkblockindex", strprintf("Do a consistency check for the block tree, chainstate, and other validation data structures every operations. Use 0 to disable. (default: %u, regtest: %u)", defaultChainParams->DefaultConsistencyChecks(), regtestChainParams->DefaultConsistencyChecks()), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-checkaddrman=", strprintf("Run addrman consistency checks every operations. Use 0 to disable. (default: %u)", DEFAULT_ADDRMAN_CONSISTENCY_CHECKS), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-checkmempool=", strprintf("Run mempool consistency checks every transactions. Use 0 to disable. (default: %u, regtest: %u)", defaultChainParams->DefaultConsistencyChecks(), regtestChainParams->DefaultConsistencyChecks()), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-checkpoints", strprintf("Enable rejection of any forks from the known historical chain until block %s (default: %u)", defaultChainParams->Checkpoints().GetHeight(), DEFAULT_CHECKPOINTS_ENABLED), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); diff --git a/src/kernel/chainstatemanager_opts.h b/src/kernel/chainstatemanager_opts.h index de5f78494a261..076841c3c9d4a 100644 --- a/src/kernel/chainstatemanager_opts.h +++ b/src/kernel/chainstatemanager_opts.h @@ -33,7 +33,7 @@ namespace kernel { struct ChainstateManagerOpts { const CChainParams& chainparams; fs::path datadir; - std::optional check_block_index{}; + std::optional check_block_index{}; bool checkpoints_enabled{DEFAULT_CHECKPOINTS_ENABLED}; //! If set, it will override the minimum work we will assume exists on some valid chain. std::optional minimum_chain_work{}; diff --git a/src/node/chainstatemanager_args.cpp b/src/node/chainstatemanager_args.cpp index 1cc126cb051c3..bc4a815a3edf8 100644 --- a/src/node/chainstatemanager_args.cpp +++ b/src/node/chainstatemanager_args.cpp @@ -24,7 +24,10 @@ namespace node { util::Result ApplyArgsManOptions(const ArgsManager& args, ChainstateManager::Options& opts) { - if (auto value{args.GetBoolArg("-checkblockindex")}) opts.check_block_index = *value; + if (auto value{args.GetIntArg("-checkblockindex")}) { + // Interpret bare -checkblockindex argument as 1 instead of 0. + opts.check_block_index = args.GetArg("-checkblockindex")->empty() ? 1 : *value; + } if (auto value{args.GetBoolArg("-checkpoints")}) opts.checkpoints_enabled = *value; diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 2c18184261e48..666a7a03034fd 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -238,7 +238,7 @@ ChainTestingSetup::ChainTestingSetup(const ChainType chainType, const std::vecto const ChainstateManager::Options chainman_opts{ .chainparams = chainparams, .datadir = m_args.GetDataDirNet(), - .check_block_index = true, + .check_block_index = 1, .notifications = *m_node.notifications, .signals = m_node.validation_signals.get(), .worker_threads_num = 2, diff --git a/src/validation.cpp b/src/validation.cpp index 9dad39e6579e5..a547614fc140e 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -5034,6 +5034,14 @@ void ChainstateManager::LoadExternalBlockFile( LogPrintf("Loaded %i blocks from external file in %dms\n", nLoaded, Ticks(SteadyClock::now() - start)); } +bool ChainstateManager::ShouldCheckBlockIndex() const +{ + // Assert to verify Flatten() has been called. + if (!*Assert(m_options.check_block_index)) return false; + if (GetRand(*m_options.check_block_index) >= 1) return false; + return true; +} + void ChainstateManager::CheckBlockIndex() { if (!ShouldCheckBlockIndex()) { diff --git a/src/validation.h b/src/validation.h index bcf153719af9a..15a1cbdc8de85 100644 --- a/src/validation.h +++ b/src/validation.h @@ -933,7 +933,7 @@ class ChainstateManager const CChainParams& GetParams() const { return m_options.chainparams; } const Consensus::Params& GetConsensus() const { return m_options.chainparams.GetConsensus(); } - bool ShouldCheckBlockIndex() const { return *Assert(m_options.check_block_index); } + bool ShouldCheckBlockIndex() const; const arith_uint256& MinimumChainWork() const { return *Assert(m_options.minimum_chain_work); } const uint256& AssumedValidBlock() const { return *Assert(m_options.assumed_valid_block); } kernel::Notifications& GetNotifications() const { return m_options.notifications; }; From 7766dd280d9a4a7ffdfcec58224d0985cfd4169b Mon Sep 17 00:00:00 2001 From: laanwj <126646+laanwj@users.noreply.github.com> Date: Sun, 28 Apr 2024 11:35:27 +0200 Subject: [PATCH 046/868] net: Replace ifname check with IFF_LOOPBACK in Discover Checking the interface name is kind of brittle. In the age of network namespaces and containers, there is no reason a loopback interface can't be called differently. Check for the `IFF_LOOPBACK` flag to detect loopback interface instead. --- src/net.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 4801f5c1f9c49..7e60ef6df5d21 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -3077,8 +3077,7 @@ void Discover() { if (ifa->ifa_addr == nullptr) continue; if ((ifa->ifa_flags & IFF_UP) == 0) continue; - if (strcmp(ifa->ifa_name, "lo") == 0) continue; - if (strcmp(ifa->ifa_name, "lo0") == 0) continue; + if ((ifa->ifa_flags & IFF_LOOPBACK) != 0) continue; if (ifa->ifa_addr->sa_family == AF_INET) { struct sockaddr_in* s4 = (struct sockaddr_in*)(ifa->ifa_addr); From a68fed111be393ddbbcd7451f78bc63601253ee0 Mon Sep 17 00:00:00 2001 From: laanwj <126646+laanwj@users.noreply.github.com> Date: Sun, 28 Apr 2024 11:40:19 +0200 Subject: [PATCH 047/868] net: Fix misleading comment for Discover All network addresses are being iterated over and added, not just the first one per interface. --- src/net.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/net.h b/src/net.h index 46d9422695263..55209324b1ec3 100644 --- a/src/net.h +++ b/src/net.h @@ -137,8 +137,7 @@ struct CSerializedNetMsg { /** * Look up IP addresses from all interfaces on the machine and add them to the * list of local addresses to self-advertise. - * The loopback interface is skipped and only the first address from each - * interface is used. + * The loopback interface is skipped. */ void Discover(); From 357ad110548d726021547d85b5b2bfcf3191d7e3 Mon Sep 17 00:00:00 2001 From: Brandon Odiwuor Date: Sun, 28 Jan 2024 11:33:17 +0300 Subject: [PATCH 048/868] test: Handle functional test disk-full error --- test/functional/test_runner.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index f6eccab2b372a..2b9faa6368432 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -29,6 +29,13 @@ os.environ["REQUIRE_WALLET_TYPE_SET"] = "1" +# Minimum amount of space to run the tests. +MIN_FREE_SPACE = 1.1 * 1024 * 1024 * 1024 +# Additional space to run an extra job. +ADDITIONAL_SPACE_PER_JOB = 100 * 1024 * 1024 +# Minimum amount of space required for --nocleanup +MIN_NO_CLEANUP_SPACE = 12 * 1024 * 1024 * 1024 + # Formatting. Default colors to empty strings. DEFAULT, BOLD, GREEN, RED = ("", ""), ("", ""), ("", ""), ("", "") try: @@ -424,6 +431,8 @@ def main(): parser.add_argument('--tmpdirprefix', '-t', default=tempfile.gettempdir(), help="Root directory for datadirs") parser.add_argument('--failfast', '-F', action='store_true', help='stop execution after the first test failure') parser.add_argument('--filter', help='filter scripts to run by regular expression') + parser.add_argument("--nocleanup", dest="nocleanup", default=False, action="store_true", + help="Leave bitcoinds and test.* datadir on exit or error") args, unknown_args = parser.parse_known_args() @@ -518,6 +527,13 @@ def main(): subprocess.check_call([sys.executable, os.path.join(config["environment"]["SRCDIR"], 'test', 'functional', test_list[0].split()[0]), '-h']) sys.exit(0) + # Warn if there is not enough space on tmpdir to run the tests with --nocleanup + if args.nocleanup: + if shutil.disk_usage(tmpdir).free < MIN_NO_CLEANUP_SPACE: + print(f"{BOLD[1]}WARNING!{BOLD[0]} There may be insufficient free space in {tmpdir} to run the functional test suite with --nocleanup. " + f"A minimum of {MIN_NO_CLEANUP_SPACE // (1024 * 1024 * 1024)} GB of free space is required.") + passon_args.append("--nocleanup") + check_script_list(src_dir=config["environment"]["SRCDIR"], fail_on_warn=args.ci) check_script_prefixes() @@ -554,6 +570,11 @@ def run_tests(*, test_list, src_dir, build_dir, tmpdir, jobs=1, enable_coverage= if os.path.isdir(cache_dir): print("%sWARNING!%s There is a cache directory here: %s. If tests fail unexpectedly, try deleting the cache directory." % (BOLD[1], BOLD[0], cache_dir)) + # Warn if there is not enough space on the testing dir + min_space = MIN_FREE_SPACE + (jobs - 1) * ADDITIONAL_SPACE_PER_JOB + if shutil.disk_usage(tmpdir).free < min_space: + print(f"{BOLD[1]}WARNING!{BOLD[0]} There may be insufficient free space in {tmpdir} to run the Bitcoin functional test suite. " + f"Running the test suite with fewer than {min_space // (1024 * 1024)} MB of free space might cause tests to fail.") tests_dir = src_dir + '/test/functional/' # This allows `test_runner.py` to work from an out-of-source build directory using a symlink, @@ -623,6 +644,11 @@ def run_tests(*, test_list, src_dir, build_dir, tmpdir, jobs=1, enable_coverage= logging.debug("Early exiting after test failure") break + if "[Errno 28] No space left on device" in stdout: + sys.exit(f"Early exiting after test failure due to insuffient free space in {tmpdir}\n" + f"Test execution data left in {tmpdir}.\n" + f"Additional storage is needed to execute testing.") + print_results(test_results, max_len_name, (int(time.time() - start_time))) if coverage: From 07aba8dd215b23b06853b1a9fe04ac8b08f62932 Mon Sep 17 00:00:00 2001 From: Greg Sanders Date: Mon, 29 Apr 2024 11:53:04 -0400 Subject: [PATCH 049/868] functional test: ensure confirmed utxo being sourced for 2nd chain --- test/functional/mempool_package_onemore.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/functional/mempool_package_onemore.py b/test/functional/mempool_package_onemore.py index 921c190668e77..281bb046b7ae9 100755 --- a/test/functional/mempool_package_onemore.py +++ b/test/functional/mempool_package_onemore.py @@ -41,7 +41,7 @@ def run_test(self): for _ in range(DEFAULT_ANCESTOR_LIMIT - 4): utxo, = self.chain_tx([utxo]) chain.append(utxo) - second_chain, = self.chain_tx([self.wallet.get_utxo()]) + second_chain, = self.chain_tx([self.wallet.get_utxo(confirmed_only=True)]) # Check mempool has DEFAULT_ANCESTOR_LIMIT + 1 transactions in it assert_equal(len(self.nodes[0].getrawmempool()), DEFAULT_ANCESTOR_LIMIT + 1) From 95897ff181c0757e445f0e066a2a590a0a0120d2 Mon Sep 17 00:00:00 2001 From: kevkevin Date: Mon, 29 Apr 2024 08:57:58 -0500 Subject: [PATCH 050/868] doc: removed help text saying that peers may not connect automatically This help text was introduced about two years ago and now a significant portion of users are using version 23.0 and higher --- src/init.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index c19d596c7fd6b..3a071bce07432 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -537,9 +537,7 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-peerbloomfilters", strprintf("Support filtering of blocks and transaction with bloom filters (default: %u)", DEFAULT_PEERBLOOMFILTERS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-peerblockfilters", strprintf("Serve compact block filters to peers per BIP 157 (default: %u)", DEFAULT_PEERBLOCKFILTERS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-txreconciliation", strprintf("Enable transaction reconciliations per BIP 330 (default: %d)", DEFAULT_TXRECONCILIATION_ENABLE), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CONNECTION); - // TODO: remove the sentence "Nodes not using ... incoming connections." once the changes from - // https://github.com/bitcoin/bitcoin/pull/23542 have become widespread. - argsman.AddArg("-port=", strprintf("Listen for connections on . Nodes not using the default ports (default: %u, testnet: %u, signet: %u, regtest: %u) are unlikely to get incoming connections. Not relevant for I2P (see doc/i2p.md).", defaultChainParams->GetDefaultPort(), testnetChainParams->GetDefaultPort(), signetChainParams->GetDefaultPort(), regtestChainParams->GetDefaultPort()), ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION); + argsman.AddArg("-port=", strprintf("Listen for connections on (default: %u, testnet: %u, signet: %u, regtest: %u). Not relevant for I2P (see doc/i2p.md).", defaultChainParams->GetDefaultPort(), testnetChainParams->GetDefaultPort(), signetChainParams->GetDefaultPort(), regtestChainParams->GetDefaultPort()), ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION); #if HAVE_SOCKADDR_UN argsman.AddArg("-proxy=", "Connect through SOCKS5 proxy, set -noproxy to disable (default: disabled). May be a local file path prefixed with 'unix:' if the proxy supports it.", ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_ELISION, OptionsCategory::CONNECTION); #else From 8fee5355ee1d2f2b410c548bac9e85f2a21a696d Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Mon, 29 Apr 2024 20:44:37 +0200 Subject: [PATCH 051/868] guix: fix suggested fake date for openssl -1.1.1l Also fix layout of instructions. --- contrib/guix/INSTALL.md | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/contrib/guix/INSTALL.md b/contrib/guix/INSTALL.md index 35ea83e585ba3..10f5835bb881e 100644 --- a/contrib/guix/INSTALL.md +++ b/contrib/guix/INSTALL.md @@ -671,6 +671,8 @@ More information: https://github.com/python/cpython/issues/81765 OpenSSL includes tests that will fail once some certificate has expired. The workarounds from the GnuTLS section immediately below can be used. +For openssl-1.1.1l use 2022-05-01 as the date. + ### GnuTLS: test-suite FAIL: status-request-revoked *The derivation is likely identified by: `/gnu/store/vhphki5sg9xkdhh2pbc8gi6vhpfzryf0-gnutls-3.6.12.drv`* @@ -705,11 +707,12 @@ authorized. This workaround was described [here](https://issues.guix.gnu.org/44559#5). Basically: -2. Turn off NTP -3. Set system time to 2020-10-01 -4. guix build --no-substitutes /gnu/store/vhphki5sg9xkdhh2pbc8gi6vhpfzryf0-gnutls-3.6.12.drv -5. Set system time back to accurate current time -6. Turn NTP back on + +1. Turn off NTP +2. Set system time to 2020-10-01 +3. guix build --no-substitutes /gnu/store/vhphki5sg9xkdhh2pbc8gi6vhpfzryf0-gnutls-3.6.12.drv +4. Set system time back to accurate current time +5. Turn NTP back on For example, From 855dd8d592c951a2b3239867ffbf66bb8677d470 Mon Sep 17 00:00:00 2001 From: Matthew Zipkin Date: Wed, 8 Feb 2023 13:23:28 -0500 Subject: [PATCH 052/868] system: use %LOCALAPPDATA% as default datadir on windows --- doc/bitcoin-conf.md | 2 +- doc/files.md | 2 +- doc/managing-wallets.md | 2 +- src/common/args.cpp | 11 +++++++++-- 4 files changed, 12 insertions(+), 5 deletions(-) diff --git a/doc/bitcoin-conf.md b/doc/bitcoin-conf.md index 1ebfb4c1de944..76711d0e7d212 100644 --- a/doc/bitcoin-conf.md +++ b/doc/bitcoin-conf.md @@ -59,7 +59,7 @@ The `includeconf=` option in the `bitcoin.conf` file can be used to includ Operating System | Data Directory | Example Path -- | -- | -- -Windows | `%APPDATA%\Bitcoin\` | `C:\Users\username\AppData\Roaming\Bitcoin\bitcoin.conf` +Windows | `%LOCALAPPDATA%\Bitcoin\` | `C:\Users\username\AppData\Local\Bitcoin\bitcoin.conf` Linux | `$HOME/.bitcoin/` | `/home/username/.bitcoin/bitcoin.conf` macOS | `$HOME/Library/Application Support/Bitcoin/` | `/Users/username/Library/Application Support/Bitcoin/bitcoin.conf` diff --git a/doc/files.md b/doc/files.md index f88d3f91a1c16..03e52f02c928d 100644 --- a/doc/files.md +++ b/doc/files.md @@ -28,7 +28,7 @@ Platform | Data directory path ---------|-------------------- Linux | `$HOME/.bitcoin/` macOS | `$HOME/Library/Application Support/Bitcoin/` -Windows | `%APPDATA%\Bitcoin\` [\[1\]](#note1) +Windows | `%LOCALAPPDATA%\Bitcoin\` [\[1\]](#note1) 2. A custom data directory path can be specified with the `-datadir` option. diff --git a/doc/managing-wallets.md b/doc/managing-wallets.md index 22e006c9639e9..e561e15427b43 100644 --- a/doc/managing-wallets.md +++ b/doc/managing-wallets.md @@ -20,7 +20,7 @@ By default, wallets are created in the `wallets` folder of the data directory, w | Operating System | Default wallet directory | | -----------------|:------------------------------------------------------------| | Linux | `/home//.bitcoin/wallets` | -| Windows | `C:\Users\\AppData\Roaming\Bitcoin\wallets` | +| Windows | `C:\Users\\AppData\Local\Bitcoin\wallets` | | macOS | `/Users//Library/Application Support/Bitcoin/wallets` | ### 1.2 Encrypting the Wallet diff --git a/src/common/args.cpp b/src/common/args.cpp index c90eb0c6856d8..caff36fdb3054 100644 --- a/src/common/args.cpp +++ b/src/common/args.cpp @@ -696,12 +696,19 @@ bool HasTestOption(const ArgsManager& args, const std::string& test_option) fs::path GetDefaultDataDir() { - // Windows: C:\Users\Username\AppData\Roaming\Bitcoin + // Windows: + // old: C:\Users\Username\AppData\Roaming\Bitcoin + // new: C:\Users\Username\AppData\Local\Bitcoin // macOS: ~/Library/Application Support/Bitcoin // Unix-like: ~/.bitcoin #ifdef WIN32 // Windows - return GetSpecialFolderPath(CSIDL_APPDATA) / "Bitcoin"; + // Check for existence of datadir in old location and keep it there + fs::path legacy_path = GetSpecialFolderPath(CSIDL_APPDATA) / "Bitcoin"; + if (fs::exists(legacy_path)) return legacy_path; + + // Otherwise, fresh installs can start in the new, "proper" location + return GetSpecialFolderPath(CSIDL_LOCAL_APPDATA) / "Bitcoin"; #else fs::path pathRet; char* pszHome = getenv("HOME"); From 84900ac34f6888b7a851d0a6a5885192155f865c Mon Sep 17 00:00:00 2001 From: Matthew Zipkin Date: Wed, 26 Apr 2023 14:06:39 +0100 Subject: [PATCH 053/868] doc: add release-notes-27064.md --- doc/release-notes/release-notes-27064.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 doc/release-notes/release-notes-27064.md diff --git a/doc/release-notes/release-notes-27064.md b/doc/release-notes/release-notes-27064.md new file mode 100644 index 0000000000000..be3ecee1b8e9c --- /dev/null +++ b/doc/release-notes/release-notes-27064.md @@ -0,0 +1,7 @@ +Files +----- + +The default data directory on Windows has been moved from `C:\Users\Username\AppData\Roaming\Bitcoin` +to `C:\Users\Username\AppData\Local\Bitcoin`. Bitcoin Core will check the existence +of the old directory first and continue to use that directory for backwards +compatibility if it is present. \ No newline at end of file From fd6a7d3a13d89d74e161095b0e9bd3570210a40c Mon Sep 17 00:00:00 2001 From: Matthew Zipkin Date: Tue, 30 Apr 2024 14:03:41 -0400 Subject: [PATCH 054/868] test: use sleepy wait-for-log in reindex readonly Also rename the busy wait-for-log method to prevent recurrence --- test/functional/feature_init.py | 2 +- test/functional/feature_reindex_readonly.py | 2 +- test/functional/test_framework/test_node.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/functional/feature_init.py b/test/functional/feature_init.py index 268009b0f4660..22ae0c307b79f 100755 --- a/test/functional/feature_init.py +++ b/test/functional/feature_init.py @@ -85,7 +85,7 @@ def check_clean_start(): for terminate_line in lines_to_terminate_after: self.log.info(f"Starting node and will exit after line {terminate_line}") - with node.wait_for_debug_log([terminate_line]): + with node.busy_wait_for_debug_log([terminate_line]): node.start(extra_args=['-txindex=1', '-blockfilterindex=1', '-coinstatsindex=1']) self.log.debug("Terminating node after terminate line was found") sigterm_node() diff --git a/test/functional/feature_reindex_readonly.py b/test/functional/feature_reindex_readonly.py index 25cff87a3b7b1..52c0bb26a6041 100755 --- a/test/functional/feature_reindex_readonly.py +++ b/test/functional/feature_reindex_readonly.py @@ -75,7 +75,7 @@ def reindex_readonly(self): if undo_immutable: self.log.debug("Attempt to restart and reindex the node with the unwritable block file") - with self.nodes[0].wait_for_debug_log([b"Reindexing finished"]): + with self.nodes[0].assert_debug_log(["Reindexing finished"], timeout=60): self.start_node(0, extra_args=['-reindex', '-fastprune']) assert block_count == self.nodes[0].getblockcount() undo_immutable() diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index 67e0be52801a7..d228bd8991a7b 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -490,7 +490,7 @@ def assert_debug_log(self, expected_msgs, unexpected_msgs=None, timeout=2): self._raise_assertion_error('Expected messages "{}" does not partially match log:\n\n{}\n\n'.format(str(expected_msgs), print_log)) @contextlib.contextmanager - def wait_for_debug_log(self, expected_msgs, timeout=60): + def busy_wait_for_debug_log(self, expected_msgs, timeout=60): """ Block until we see a particular debug log message fragment or until we exceed the timeout. Return: From dddd40ba8267dea11a3eb03d5cf8b51dbb99be5d Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Mon, 26 Feb 2024 13:36:30 +0100 Subject: [PATCH 055/868] scripted-diff: Add IWYU pragma keep to bitcoin-config.h includes -BEGIN VERIFY SCRIPT- perl -0777 -pi -e 's/#if defined\(HAVE_CONFIG_H\)\n#include .*\n#endif.*\n/#include \/\/ IWYU pragma: keep\n/g' $( git grep -l '#include ' ) -END VERIFY SCRIPT- --- src/addrdb.cpp | 4 +--- src/addrman.cpp | 4 +--- src/bench/wallet_create.cpp | 4 +--- src/bench/wallet_ismine.cpp | 4 +--- src/bench/wallet_loading.cpp | 4 +--- src/bitcoin-cli.cpp | 4 +--- src/bitcoin-tx.cpp | 4 +--- src/bitcoin-util.cpp | 4 +--- src/bitcoin-wallet.cpp | 4 +--- src/bitcoind.cpp | 4 +--- src/clientversion.cpp | 4 +--- src/clientversion.h | 4 +--- src/common/run_command.cpp | 4 +--- src/common/settings.cpp | 4 +--- src/common/system.cpp | 4 +--- src/common/system.h | 4 +--- src/crypto/sha256.cpp | 4 +--- src/httpserver.cpp | 4 +--- src/init.cpp | 4 +--- src/init/common.cpp | 4 +--- src/mapport.cpp | 4 +--- src/net.cpp | 4 +--- src/netbase.cpp | 4 +--- src/node/interfaces.cpp | 4 +--- src/node/kernel_notifications.cpp | 4 +--- src/qt/bitcoin.cpp | 4 +--- src/qt/bitcoin.h | 4 +--- src/qt/bitcoingui.cpp | 4 +--- src/qt/bitcoingui.h | 4 +--- src/qt/clientmodel.cpp | 4 +--- src/qt/createwalletdialog.cpp | 4 +--- src/qt/guiutil.cpp | 4 +--- src/qt/intro.cpp | 4 +--- src/qt/modaloverlay.cpp | 4 +--- src/qt/notificator.cpp | 4 +--- src/qt/notificator.h | 4 +--- src/qt/optionsdialog.cpp | 4 +--- src/qt/optionsmodel.cpp | 4 +--- src/qt/qrimagewidget.cpp | 4 +--- src/qt/receiverequestdialog.cpp | 4 +--- src/qt/rpcconsole.cpp | 4 +--- src/qt/rpcconsole.h | 4 +--- src/qt/sendcoinsdialog.cpp | 4 +--- src/qt/splashscreen.cpp | 4 +--- src/qt/test/optiontests.cpp | 4 +--- src/qt/test/test_main.cpp | 4 +--- src/qt/utilitydialog.cpp | 4 +--- src/random.cpp | 4 +--- src/randomenv.cpp | 4 +--- src/rest.cpp | 4 +--- src/rpc/external_signer.cpp | 4 +--- src/rpc/mining.cpp | 4 +--- src/rpc/node.cpp | 4 +--- src/rpc/register.h | 4 +--- src/rpc/server.cpp | 4 +--- src/rpc/util.cpp | 4 +--- src/test/system_tests.cpp | 4 +--- src/test/util/setup_common.cpp | 4 +--- src/test/util_threadnames_tests.cpp | 4 +--- src/util/check.cpp | 4 +--- src/util/fs_helpers.cpp | 4 +--- src/util/syserror.cpp | 4 +--- src/util/threadnames.cpp | 4 +--- src/util/tokenpipe.cpp | 4 +--- src/util/trace.h | 4 +--- src/validation.cpp | 4 +--- src/wallet/init.cpp | 4 +--- src/wallet/rpc/addresses.cpp | 4 +--- src/wallet/rpc/backup.cpp | 4 +--- src/wallet/rpc/wallet.cpp | 4 +--- src/wallet/sqlite.cpp | 4 +--- src/wallet/test/db_tests.cpp | 4 +--- src/wallet/test/util.h | 4 +--- src/wallet/wallet.cpp | 4 +--- src/wallet/walletdb.cpp | 4 +--- src/wallet/wallettool.cpp | 4 +--- src/warnings.cpp | 4 +--- 77 files changed, 77 insertions(+), 231 deletions(-) diff --git a/src/addrdb.cpp b/src/addrdb.cpp index 14dc314c3650f..4d34c24ba95d0 100644 --- a/src/addrdb.cpp +++ b/src/addrdb.cpp @@ -3,9 +3,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include diff --git a/src/addrman.cpp b/src/addrman.cpp index ef8ed92bb5310..eb9c37255076d 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -3,9 +3,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include #include diff --git a/src/bench/wallet_create.cpp b/src/bench/wallet_create.cpp index 32f55f51e153e..85280e5835cb7 100644 --- a/src/bench/wallet_create.cpp +++ b/src/bench/wallet_create.cpp @@ -2,9 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or https://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include #include diff --git a/src/bench/wallet_ismine.cpp b/src/bench/wallet_ismine.cpp index 3f922e18a58de..753404b526193 100644 --- a/src/bench/wallet_ismine.cpp +++ b/src/bench/wallet_ismine.cpp @@ -2,9 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif // HAVE_CONFIG_H +#include // IWYU pragma: keep #include #include #include diff --git a/src/bench/wallet_loading.cpp b/src/bench/wallet_loading.cpp index 6305126c7d452..02582deda4a8d 100644 --- a/src/bench/wallet_loading.cpp +++ b/src/bench/wallet_loading.cpp @@ -2,9 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include #include diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index 8901d10ef63d4..c7ba2204c3fda 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -3,9 +3,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include #include diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp index 320624c419b80..1c5b0c074c9e4 100644 --- a/src/bitcoin-tx.cpp +++ b/src/bitcoin-tx.cpp @@ -2,9 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include #include diff --git a/src/bitcoin-util.cpp b/src/bitcoin-util.cpp index 96387e8c71e8a..c8f5bc50262e8 100644 --- a/src/bitcoin-util.cpp +++ b/src/bitcoin-util.cpp @@ -2,9 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include #include diff --git a/src/bitcoin-wallet.cpp b/src/bitcoin-wallet.cpp index fe90958a5f5a2..bee052bc46de2 100644 --- a/src/bitcoin-wallet.cpp +++ b/src/bitcoin-wallet.cpp @@ -2,9 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include #include diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp index 54796c5abbfc2..7685bdea79c5a 100644 --- a/src/bitcoind.cpp +++ b/src/bitcoind.cpp @@ -3,9 +3,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include #include diff --git a/src/clientversion.cpp b/src/clientversion.cpp index bf5579ee69783..3a5e060a39789 100644 --- a/src/clientversion.cpp +++ b/src/clientversion.cpp @@ -2,9 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include #include diff --git a/src/clientversion.h b/src/clientversion.h index 9da0cd0b39428..73aaf868e4980 100644 --- a/src/clientversion.h +++ b/src/clientversion.h @@ -7,9 +7,7 @@ #include -#if defined(HAVE_CONFIG_H) -#include -#endif //HAVE_CONFIG_H +#include // IWYU pragma: keep // Check that required client information is defined #if !defined(CLIENT_VERSION_MAJOR) || !defined(CLIENT_VERSION_MINOR) || !defined(CLIENT_VERSION_BUILD) || !defined(CLIENT_VERSION_IS_RELEASE) || !defined(COPYRIGHT_YEAR) diff --git a/src/common/run_command.cpp b/src/common/run_command.cpp index 347b4860955fe..67608b985fbfb 100644 --- a/src/common/run_command.cpp +++ b/src/common/run_command.cpp @@ -2,9 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include diff --git a/src/common/settings.cpp b/src/common/settings.cpp index db1001111a20a..c1520dacd252b 100644 --- a/src/common/settings.cpp +++ b/src/common/settings.cpp @@ -4,9 +4,7 @@ #include -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include #include diff --git a/src/common/system.cpp b/src/common/system.cpp index 1fa53a5f34cab..ddd0feda3b843 100644 --- a/src/common/system.cpp +++ b/src/common/system.cpp @@ -3,9 +3,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include diff --git a/src/common/system.h b/src/common/system.h index 83280d46ee0ee..d9115d3b333ef 100644 --- a/src/common/system.h +++ b/src/common/system.h @@ -6,9 +6,7 @@ #ifndef BITCOIN_COMMON_SYSTEM_H #define BITCOIN_COMMON_SYSTEM_H -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include #include diff --git a/src/crypto/sha256.cpp b/src/crypto/sha256.cpp index 301f22a248885..c883bd2f03f2f 100644 --- a/src/crypto/sha256.cpp +++ b/src/crypto/sha256.cpp @@ -2,9 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include #include diff --git a/src/httpserver.cpp b/src/httpserver.cpp index 71134d442fe84..b1d4dc9234788 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -2,9 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include diff --git a/src/init.cpp b/src/init.cpp index 4d7638cd6e87c..1637d8369c876 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -3,9 +3,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include diff --git a/src/init/common.cpp b/src/init/common.cpp index 0800cd93d8b02..3a6df3e8bd519 100644 --- a/src/init/common.cpp +++ b/src/init/common.cpp @@ -2,9 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include #include diff --git a/src/mapport.cpp b/src/mapport.cpp index 08b365db4b669..80670230c73a8 100644 --- a/src/mapport.cpp +++ b/src/mapport.cpp @@ -2,9 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include diff --git a/src/net.cpp b/src/net.cpp index 1eda98253a4f8..4a42ddec2cb48 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -3,9 +3,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include diff --git a/src/netbase.cpp b/src/netbase.cpp index f0fa2983783d2..e231766487f85 100644 --- a/src/netbase.cpp +++ b/src/netbase.cpp @@ -3,9 +3,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 4d2d83812e086..40927c990afbf 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -52,9 +52,7 @@ #include #include -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include #include diff --git a/src/node/kernel_notifications.cpp b/src/node/kernel_notifications.cpp index 99f909ff75c7d..6578b383f7acf 100644 --- a/src/node/kernel_notifications.cpp +++ b/src/node/kernel_notifications.cpp @@ -4,9 +4,7 @@ #include -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include #include diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index b1a8461d0291d..44a858c16b32a 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -2,9 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include diff --git a/src/qt/bitcoin.h b/src/qt/bitcoin.h index 9622c9d57d7cc..1423a8bbc698f 100644 --- a/src/qt/bitcoin.h +++ b/src/qt/bitcoin.h @@ -5,9 +5,7 @@ #ifndef BITCOIN_QT_BITCOIN_H #define BITCOIN_QT_BITCOIN_H -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include #include diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp index 5f132b817e074..a43009d954253 100644 --- a/src/qt/bitcoingui.cpp +++ b/src/qt/bitcoingui.cpp @@ -2,9 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include diff --git a/src/qt/bitcoingui.h b/src/qt/bitcoingui.h index ba91eeb1ffab8..73adbda5a5f4b 100644 --- a/src/qt/bitcoingui.h +++ b/src/qt/bitcoingui.h @@ -5,9 +5,7 @@ #ifndef BITCOIN_QT_BITCOINGUI_H #define BITCOIN_QT_BITCOINGUI_H -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include #include diff --git a/src/qt/clientmodel.cpp b/src/qt/clientmodel.cpp index 05172cfbd2780..2f3bad37e6ae8 100644 --- a/src/qt/clientmodel.cpp +++ b/src/qt/clientmodel.cpp @@ -2,9 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include diff --git a/src/qt/createwalletdialog.cpp b/src/qt/createwalletdialog.cpp index 6557280d891b2..3e8d1461e5ad7 100644 --- a/src/qt/createwalletdialog.cpp +++ b/src/qt/createwalletdialog.cpp @@ -2,9 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include #include diff --git a/src/qt/guiutil.cpp b/src/qt/guiutil.cpp index e094b686d40a0..ee841ce626b82 100644 --- a/src/qt/guiutil.cpp +++ b/src/qt/guiutil.cpp @@ -2,9 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include diff --git a/src/qt/intro.cpp b/src/qt/intro.cpp index 5371dbaa30408..26b42deb64124 100644 --- a/src/qt/intro.cpp +++ b/src/qt/intro.cpp @@ -2,9 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include #include diff --git a/src/qt/modaloverlay.cpp b/src/qt/modaloverlay.cpp index 667db06574e01..7bc6ccdc49468 100644 --- a/src/qt/modaloverlay.cpp +++ b/src/qt/modaloverlay.cpp @@ -2,9 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include #include diff --git a/src/qt/notificator.cpp b/src/qt/notificator.cpp index 551c0ffd1372a..85bdeee49a0df 100644 --- a/src/qt/notificator.cpp +++ b/src/qt/notificator.cpp @@ -2,9 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include diff --git a/src/qt/notificator.h b/src/qt/notificator.h index 1fd8181a22684..8808716aa4dca 100644 --- a/src/qt/notificator.h +++ b/src/qt/notificator.h @@ -5,9 +5,7 @@ #ifndef BITCOIN_QT_NOTIFICATOR_H #define BITCOIN_QT_NOTIFICATOR_H -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include #include diff --git a/src/qt/optionsdialog.cpp b/src/qt/optionsdialog.cpp index dd654a7abe27c..15103baf59354 100644 --- a/src/qt/optionsdialog.cpp +++ b/src/qt/optionsdialog.cpp @@ -2,9 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include #include diff --git a/src/qt/optionsmodel.cpp b/src/qt/optionsmodel.cpp index d0f7c64357ed3..0c21c6748d0c5 100644 --- a/src/qt/optionsmodel.cpp +++ b/src/qt/optionsmodel.cpp @@ -2,9 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include diff --git a/src/qt/qrimagewidget.cpp b/src/qt/qrimagewidget.cpp index 00f928b355796..f6e712a047a3e 100644 --- a/src/qt/qrimagewidget.cpp +++ b/src/qt/qrimagewidget.cpp @@ -15,9 +15,7 @@ #include #include -#if defined(HAVE_CONFIG_H) -#include /* for USE_QRCODE */ -#endif +#include // IWYU pragma: keep #ifdef USE_QRCODE #include diff --git a/src/qt/receiverequestdialog.cpp b/src/qt/receiverequestdialog.cpp index 3453857f9854e..b4322ddc0f436 100644 --- a/src/qt/receiverequestdialog.cpp +++ b/src/qt/receiverequestdialog.cpp @@ -14,9 +14,7 @@ #include #include -#if defined(HAVE_CONFIG_H) -#include /* for USE_QRCODE */ -#endif +#include // IWYU pragma: keep ReceiveRequestDialog::ReceiveRequestDialog(QWidget* parent) : QDialog(parent, GUIUtil::dialog_flags), diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index bafef62945a60..702ca44395df2 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -2,9 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include #include diff --git a/src/qt/rpcconsole.h b/src/qt/rpcconsole.h index 358f68c3c8c66..d6a5035c33dc0 100644 --- a/src/qt/rpcconsole.h +++ b/src/qt/rpcconsole.h @@ -5,9 +5,7 @@ #ifndef BITCOIN_QT_RPCCONSOLE_H #define BITCOIN_QT_RPCCONSOLE_H -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include #include diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp index 89bd35eb1bcb0..0d8c0f7a636b4 100644 --- a/src/qt/sendcoinsdialog.cpp +++ b/src/qt/sendcoinsdialog.cpp @@ -2,9 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include #include diff --git a/src/qt/splashscreen.cpp b/src/qt/splashscreen.cpp index 8872f8be321ce..ffd6689910b0d 100644 --- a/src/qt/splashscreen.cpp +++ b/src/qt/splashscreen.cpp @@ -2,9 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include diff --git a/src/qt/test/optiontests.cpp b/src/qt/test/optiontests.cpp index 5f9f2cb4491e4..0f82f65f3eab3 100644 --- a/src/qt/test/optiontests.cpp +++ b/src/qt/test/optiontests.cpp @@ -2,9 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include #include diff --git a/src/qt/test/test_main.cpp b/src/qt/test/test_main.cpp index c5405cca98435..f310d0a02b712 100644 --- a/src/qt/test/test_main.cpp +++ b/src/qt/test/test_main.cpp @@ -2,9 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include #include diff --git a/src/qt/utilitydialog.cpp b/src/qt/utilitydialog.cpp index 22fe219def857..b55b4cbe64dc3 100644 --- a/src/qt/utilitydialog.cpp +++ b/src/qt/utilitydialog.cpp @@ -2,9 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include diff --git a/src/random.cpp b/src/random.cpp index 4fc90997048ba..239d5bc6fe958 100644 --- a/src/random.cpp +++ b/src/random.cpp @@ -3,9 +3,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include diff --git a/src/randomenv.cpp b/src/randomenv.cpp index 123b5cc06c117..aeec959c28c61 100644 --- a/src/randomenv.cpp +++ b/src/randomenv.cpp @@ -3,9 +3,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include diff --git a/src/rest.cpp b/src/rest.cpp index 89c033b8a31d4..9fc5d4af047cf 100644 --- a/src/rest.cpp +++ b/src/rest.cpp @@ -3,9 +3,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include diff --git a/src/rpc/external_signer.cpp b/src/rpc/external_signer.cpp index 8d06ae42589cd..27c739490995d 100644 --- a/src/rpc/external_signer.cpp +++ b/src/rpc/external_signer.cpp @@ -2,9 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include #include diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 454c262803d70..b908265c4b521 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -3,9 +3,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include #include diff --git a/src/rpc/node.cpp b/src/rpc/node.cpp index ffc2ee5ab077f..447be2cf6440a 100644 --- a/src/rpc/node.cpp +++ b/src/rpc/node.cpp @@ -3,9 +3,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include #include diff --git a/src/rpc/register.h b/src/rpc/register.h index fd23dde75b714..65fd29ff08fe2 100644 --- a/src/rpc/register.h +++ b/src/rpc/register.h @@ -5,9 +5,7 @@ #ifndef BITCOIN_RPC_REGISTER_H #define BITCOIN_RPC_REGISTER_H -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep /** These are in one header file to avoid creating tons of single-function * headers for everything under src/rpc/ */ diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index e7d1e3db4e590..a800451f4a04f 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -3,9 +3,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index f68387805494b..a81300e3bfafb 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -2,9 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include #include diff --git a/src/test/system_tests.cpp b/src/test/system_tests.cpp index 2de147deeaeb1..baa759e42c26b 100644 --- a/src/test/system_tests.cpp +++ b/src/test/system_tests.cpp @@ -3,9 +3,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. // -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include #include #include diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 38350b33cc5e0..0fb5e4eb45e7e 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -2,9 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include diff --git a/src/test/util_threadnames_tests.cpp b/src/test/util_threadnames_tests.cpp index 45d3a58fd3ce1..df5b1a4461d77 100644 --- a/src/test/util_threadnames_tests.cpp +++ b/src/test/util_threadnames_tests.cpp @@ -11,9 +11,7 @@ #include #include -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include diff --git a/src/util/check.cpp b/src/util/check.cpp index c4d4b0cc28f21..eb3885832b2f3 100644 --- a/src/util/check.cpp +++ b/src/util/check.cpp @@ -4,9 +4,7 @@ #include -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include #include diff --git a/src/util/fs_helpers.cpp b/src/util/fs_helpers.cpp index bce56024620ab..8952f20f7994f 100644 --- a/src/util/fs_helpers.cpp +++ b/src/util/fs_helpers.cpp @@ -5,9 +5,7 @@ #include -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include #include diff --git a/src/util/syserror.cpp b/src/util/syserror.cpp index bac498a23d194..6f3a724483642 100644 --- a/src/util/syserror.cpp +++ b/src/util/syserror.cpp @@ -2,9 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include #include diff --git a/src/util/threadnames.cpp b/src/util/threadnames.cpp index 91883fe4ffdc0..ea597dd05a77c 100644 --- a/src/util/threadnames.cpp +++ b/src/util/threadnames.cpp @@ -2,9 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include #include diff --git a/src/util/tokenpipe.cpp b/src/util/tokenpipe.cpp index 3c27d5e523105..16fbb664ea170 100644 --- a/src/util/tokenpipe.cpp +++ b/src/util/tokenpipe.cpp @@ -3,9 +3,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #ifndef WIN32 diff --git a/src/util/trace.h b/src/util/trace.h index b7c275f19b7f8..d9ed65e3aab56 100644 --- a/src/util/trace.h +++ b/src/util/trace.h @@ -5,9 +5,7 @@ #ifndef BITCOIN_UTIL_TRACE_H #define BITCOIN_UTIL_TRACE_H -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #ifdef ENABLE_TRACING diff --git a/src/validation.cpp b/src/validation.cpp index f57851b4f7e01..c2b56555a2ce0 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3,9 +3,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index f151fad740283..14e988ec1aa2c 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -3,9 +3,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include #include diff --git a/src/wallet/rpc/addresses.cpp b/src/wallet/rpc/addresses.cpp index bed9ec029aa04..65587f0b18802 100644 --- a/src/wallet/rpc/addresses.cpp +++ b/src/wallet/rpc/addresses.cpp @@ -2,9 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include #include diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp index ae2dfe57951e1..c05600484eaee 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -2,9 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include #include diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp index a684d4e191a2a..f1cb595271e5e 100644 --- a/src/wallet/rpc/wallet.cpp +++ b/src/wallet/rpc/wallet.cpp @@ -3,9 +3,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include #include diff --git a/src/wallet/sqlite.cpp b/src/wallet/sqlite.cpp index 34f18bf0b1cde..5e3a8179a272c 100644 --- a/src/wallet/sqlite.cpp +++ b/src/wallet/sqlite.cpp @@ -2,9 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include diff --git a/src/wallet/test/db_tests.cpp b/src/wallet/test/db_tests.cpp index f783424df8b37..438dfceb7ff99 100644 --- a/src/wallet/test/db_tests.cpp +++ b/src/wallet/test/db_tests.cpp @@ -2,9 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include diff --git a/src/wallet/test/util.h b/src/wallet/test/util.h index 9f2974ece6b22..a3e6ede81ef8c 100644 --- a/src/wallet/test/util.h +++ b/src/wallet/test/util.h @@ -5,9 +5,7 @@ #ifndef BITCOIN_WALLET_TEST_UTIL_H #define BITCOIN_WALLET_TEST_UTIL_H -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include #include diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 8f4171eb15d4b..45f69f52d1dce 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -5,9 +5,7 @@ #include -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include #include #include diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index b1ce7ee4e70de..3ba43cdb73edc 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -3,9 +3,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include diff --git a/src/wallet/wallettool.cpp b/src/wallet/wallettool.cpp index cda344ab19cc3..7a1930fd316e9 100644 --- a/src/wallet/wallettool.cpp +++ b/src/wallet/wallettool.cpp @@ -2,9 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include diff --git a/src/warnings.cpp b/src/warnings.cpp index d55eecc48deea..da41b02b27231 100644 --- a/src/warnings.cpp +++ b/src/warnings.cpp @@ -3,9 +3,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif +#include // IWYU pragma: keep #include From fa09451f8e6799682d7e7c863f25334fd1c7dce3 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Mon, 26 Feb 2024 14:25:51 +0100 Subject: [PATCH 056/868] Add lint check for bitcoin-config.h include IWYU pragma Also, remove the no longer needed, remaining definitions and checks of HAVE_CONFIG_H. --- build-aux/m4/bitcoin_qt.m4 | 2 +- build_msvc/bitcoin-qt/bitcoin-qt.vcxproj | 4 ++-- build_msvc/common.init.vcxproj.in | 2 +- test/lint/test_runner/src/main.rs | 12 ++++++++++-- 4 files changed, 14 insertions(+), 6 deletions(-) diff --git a/build-aux/m4/bitcoin_qt.m4 b/build-aux/m4/bitcoin_qt.m4 index a716cd9a27795..337e2208a9c29 100644 --- a/build-aux/m4/bitcoin_qt.m4 +++ b/build-aux/m4/bitcoin_qt.m4 @@ -227,7 +227,7 @@ AC_DEFUN([BITCOIN_QT_CONFIGURE],[ BITCOIN_QT_PATH_PROGS([LUPDATE], [lupdate-qt5 lupdate5 lupdate],$qt_bin_path, yes) BITCOIN_QT_PATH_PROGS([LCONVERT], [lconvert-qt5 lconvert5 lconvert], $qt_bin_path, yes) - MOC_DEFS='-DHAVE_CONFIG_H -I$(srcdir)' + MOC_DEFS='-I$(srcdir)' case $host in *darwin*) BITCOIN_QT_CHECK([ diff --git a/build_msvc/bitcoin-qt/bitcoin-qt.vcxproj b/build_msvc/bitcoin-qt/bitcoin-qt.vcxproj index fe01da28c880a..ff98d37cf89fe 100644 --- a/build_msvc/bitcoin-qt/bitcoin-qt.vcxproj +++ b/build_msvc/bitcoin-qt/bitcoin-qt.vcxproj @@ -62,7 +62,7 @@ ..\..\src; - HAVE_CONFIG_H;_UNICODE;UNICODE;%(PreprocessorDefinitions) + _UNICODE;UNICODE;%(PreprocessorDefinitions) @@ -75,7 +75,7 @@ ..\..\src; - HAVE_CONFIG_H;_UNICODE;UNICODE;%(PreprocessorDefinitions) + _UNICODE;UNICODE;%(PreprocessorDefinitions) diff --git a/build_msvc/common.init.vcxproj.in b/build_msvc/common.init.vcxproj.in index 71fceb6c660c3..6468abcd061f5 100644 --- a/build_msvc/common.init.vcxproj.in +++ b/build_msvc/common.init.vcxproj.in @@ -90,7 +90,7 @@ /utf-8 /Zc:preprocessor /Zc:__cplusplus /std:c++20 %(AdditionalOptions) 4018;4244;4267;4715;4805 true - _SILENCE_CXX17_CODECVT_HEADER_DEPRECATION_WARNING;SECP256K1_STATIC;ZMQ_STATIC;NOMINMAX;WIN32;HAVE_CONFIG_H;_CRT_SECURE_NO_WARNINGS;_CONSOLE;_WIN32_WINNT=0x0601;_WIN32_IE=0x0501;WIN32_LEAN_AND_MEAN;PROVIDE_FUZZ_MAIN_FUNCTION;%(PreprocessorDefinitions) + _SILENCE_CXX17_CODECVT_HEADER_DEPRECATION_WARNING;SECP256K1_STATIC;ZMQ_STATIC;NOMINMAX;WIN32;_CRT_SECURE_NO_WARNINGS;_CONSOLE;_WIN32_WINNT=0x0601;_WIN32_IE=0x0501;WIN32_LEAN_AND_MEAN;PROVIDE_FUZZ_MAIN_FUNCTION;%(PreprocessorDefinitions) ..\..\src;..\..\src\minisketch\include;..\..\src\univalue\include;..\..\src\secp256k1\include;..\..\src\leveldb\include;..\..\src\leveldb\helpers\memenv;%(AdditionalIncludeDirectories) diff --git a/test/lint/test_runner/src/main.rs b/test/lint/test_runner/src/main.rs index e22e047e4b19c..9795f9ca3466d 100644 --- a/test/lint/test_runner/src/main.rs +++ b/test/lint/test_runner/src/main.rs @@ -178,7 +178,6 @@ Please add any false positives, such as subtrees, or externally sourced files to fn lint_includes_build_config() -> LintResult { let config_path = "./src/config/bitcoin-config.h.in"; - let include_directive = "#include "; if !Path::new(config_path).is_file() { assert!(Command::new("./autogen.sh") .status() @@ -235,7 +234,11 @@ fn lint_includes_build_config() -> LintResult { } else { "--files-with-matches" }, - include_directive, + if mode { + "^#include // IWYU pragma: keep$" + } else { + "#include " // Catch redundant includes with and without the IWYU pragma + }, "--", ]) .args(defines_files.lines()) @@ -256,6 +259,11 @@ even though bitcoin-config.h indicates that a faster feature is available and sh If you are unsure which symbol is used, you can find it with this command: git grep --perl-regexp '{}' -- file_name + +Make sure to include it with the IWYU pragma. Otherwise, IWYU may falsely instruct to remove the +include again. + +#include // IWYU pragma: keep "#, defines_regex )); From 42fb5311b19582361409d65c6fddeadbee14bb97 Mon Sep 17 00:00:00 2001 From: stickies-v Date: Wed, 10 Apr 2024 12:16:21 +0200 Subject: [PATCH 057/868] rpc: return warnings as an array instead of just a single one The RPC documentation for `getblockchaininfo`, `getmininginfo` and `getnetworkinfo` states that "warnings" returns "any network and blockchain warnings". In practice, only a single warning is returned. Fix that by returning all warnings as an array. As a side benefit, cleans up the GetWarnings() logic. --- doc/release-notes-29845.md | 8 +++++++ src/node/interfaces.cpp | 3 ++- src/rpc/blockchain.cpp | 12 +++++++--- src/rpc/mining.cpp | 12 +++++++--- src/rpc/net.cpp | 12 +++++++--- src/rpc/util.cpp | 17 +++++++++++++ src/rpc/util.h | 2 ++ src/warnings.cpp | 24 +++++++------------ src/warnings.h | 10 +++----- .../functional/feature_versionbits_warning.py | 8 +++---- 10 files changed, 71 insertions(+), 37 deletions(-) create mode 100644 doc/release-notes-29845.md diff --git a/doc/release-notes-29845.md b/doc/release-notes-29845.md new file mode 100644 index 0000000000000..4994d0a34dfe5 --- /dev/null +++ b/doc/release-notes-29845.md @@ -0,0 +1,8 @@ +RPC +--- + +- the `warnings` field in `getblockchaininfo`, `getmininginfo` and + `getnetworkinfo` now returns all the active node warnings as an array + of strings, instead of just a single warning. The current behaviour + can temporarily be restored by running bitcoind with configuration + option `-deprecatedrpc=warnings`. \ No newline at end of file diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 4d2d83812e086..bb13e537382d7 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -47,6 +47,7 @@ #include #include #include +#include #include #include #include @@ -91,7 +92,7 @@ class NodeImpl : public Node explicit NodeImpl(NodeContext& context) { setContext(&context); } void initLogging() override { InitLogging(args()); } void initParameterInteraction() override { InitParameterInteraction(args()); } - bilingual_str getWarnings() override { return GetWarnings(true); } + bilingual_str getWarnings() override { return Join(GetWarnings(), Untranslated("

")); } int getExitStatus() override { return Assert(m_context)->exit_status.load(); } uint32_t getLogCategories() override { return LogInstance().GetCategoryMask(); } bool baseInitialize() override diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index eed004806a83b..c9997ae063894 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -47,7 +47,6 @@ #include #include #include -#include #include @@ -1260,7 +1259,14 @@ RPCHelpMan getblockchaininfo() {RPCResult::Type::NUM, "pruneheight", /*optional=*/true, "height of the last block pruned, plus one (only present if pruning is enabled)"}, {RPCResult::Type::BOOL, "automatic_pruning", /*optional=*/true, "whether automatic pruning is enabled (only present if pruning is enabled)"}, {RPCResult::Type::NUM, "prune_target_size", /*optional=*/true, "the target size used by pruning (only present if automatic pruning is enabled)"}, - {RPCResult::Type::STR, "warnings", "any network and blockchain warnings"}, + (IsDeprecatedRPCEnabled("warnings") ? + RPCResult{RPCResult::Type::STR, "warnings", "any network and blockchain warnings (DEPRECATED)"} : + RPCResult{RPCResult::Type::ARR, "warnings", "any network and blockchain warnings (run with `-deprecatedrpc=warnings` to return the latest warning as a single string)", + { + {RPCResult::Type::STR, "", "warning"}, + } + } + ), }}, RPCExamples{ HelpExampleCli("getblockchaininfo", "") @@ -1298,7 +1304,7 @@ RPCHelpMan getblockchaininfo() } } - obj.pushKV("warnings", GetWarnings(false).original); + obj.pushKV("warnings", GetNodeWarnings(IsDeprecatedRPCEnabled("warnings"))); return obj; }, }; diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 454c262803d70..f36665a819a4b 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -39,7 +39,6 @@ #include #include #include -#include #include #include @@ -426,7 +425,14 @@ static RPCHelpMan getmininginfo() {RPCResult::Type::NUM, "networkhashps", "The network hashes per second"}, {RPCResult::Type::NUM, "pooledtx", "The size of the mempool"}, {RPCResult::Type::STR, "chain", "current network name (main, test, signet, regtest)"}, - {RPCResult::Type::STR, "warnings", "any network and blockchain warnings"}, + (IsDeprecatedRPCEnabled("warnings") ? + RPCResult{RPCResult::Type::STR, "warnings", "any network and blockchain warnings (DEPRECATED)"} : + RPCResult{RPCResult::Type::ARR, "warnings", "any network and blockchain warnings (run with `-deprecatedrpc=warnings` to return the latest warning as a single string)", + { + {RPCResult::Type::STR, "", "warning"}, + } + } + ), }}, RPCExamples{ HelpExampleCli("getmininginfo", "") @@ -448,7 +454,7 @@ static RPCHelpMan getmininginfo() obj.pushKV("networkhashps", getnetworkhashps().HandleRequest(request)); obj.pushKV("pooledtx", (uint64_t)mempool.size()); obj.pushKV("chain", chainman.GetParams().GetChainTypeString()); - obj.pushKV("warnings", GetWarnings(false).original); + obj.pushKV("warnings", GetNodeWarnings(IsDeprecatedRPCEnabled("warnings"))); return obj; }, }; diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index 739ff1a48ea43..04f9410b32b8c 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -29,7 +29,6 @@ #include #include #include -#include #include @@ -657,7 +656,14 @@ static RPCHelpMan getnetworkinfo() {RPCResult::Type::NUM, "score", "relative score"}, }}, }}, - {RPCResult::Type::STR, "warnings", "any network and blockchain warnings"}, + (IsDeprecatedRPCEnabled("warnings") ? + RPCResult{RPCResult::Type::STR, "warnings", "any network and blockchain warnings (DEPRECATED)"} : + RPCResult{RPCResult::Type::ARR, "warnings", "any network and blockchain warnings (run with `-deprecatedrpc=warnings` to return the latest warning as a single string)", + { + {RPCResult::Type::STR, "", "warning"}, + } + } + ), } }, RPCExamples{ @@ -707,7 +713,7 @@ static RPCHelpMan getnetworkinfo() } } obj.pushKV("localaddresses", localAddresses); - obj.pushKV("warnings", GetWarnings(false).original); + obj.pushKV("warnings", GetNodeWarnings(IsDeprecatedRPCEnabled("warnings"))); return obj; }, }; diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index f68387805494b..0ac2c4e7b0df2 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -18,16 +18,19 @@ #include