From a7b581423e44c51fb7d177c5a15fe2cc2ab8aa43 Mon Sep 17 00:00:00 2001 From: Calin Culianu Date: Wed, 26 Mar 2025 08:12:39 -0500 Subject: [PATCH 01/25] Fix 11-year-old mis-categorized error code in OP_IF evaluation This was introduced by commit ab9edbd6b6eb3efbca11f16fa467c3c0ef905708. It appears the original author may have gotten tired and pasted the wrong error code into this 1 place. Every other situation where the value stack lacks the required number of arguments for the op-code, SCRIPT_ERR_INVALID_STACK_OPERATION is reported. Not so here. This commit fixes the situation. Also in this commit: - Fix script_tests to adjust to the corrected error message - Fix p2p_invalid_tx functional test to produce the desired error message --- src/script/interpreter.cpp | 2 +- src/test/data/script_tests.json | 24 ++++++++++++------------ test/functional/data/invalid_txs.py | 2 +- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp index 61ea7f4503c2..ec34de5e7602 100644 --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -607,7 +607,7 @@ bool EvalScript(std::vector >& stack, const CScript& if (fExec) { if (stack.size() < 1) - return set_error(serror, SCRIPT_ERR_UNBALANCED_CONDITIONAL); + return set_error(serror, SCRIPT_ERR_INVALID_STACK_OPERATION); valtype& vch = stacktop(-1); // Tapscript requires minimal IF/NOTIF inputs as a consensus rule. if (sigversion == SigVersion::TAPSCRIPT) { diff --git a/src/test/data/script_tests.json b/src/test/data/script_tests.json index ad05240369b3..83f20ff44ee0 100644 --- a/src/test/data/script_tests.json +++ b/src/test/data/script_tests.json @@ -1027,8 +1027,8 @@ ["1", "IF 1", "P2SH,STRICTENC", "UNBALANCED_CONDITIONAL", "IF without ENDIF"], ["1 IF 1", "ENDIF", "P2SH,STRICTENC", "UNBALANCED_CONDITIONAL", "IFs don't carry over"], -["NOP", "IF 1 ENDIF", "P2SH,STRICTENC", "UNBALANCED_CONDITIONAL", "The following tests check the if(stack.size() < N) tests in each opcode"], -["NOP", "NOTIF 1 ENDIF", "P2SH,STRICTENC", "UNBALANCED_CONDITIONAL", "They are here to catch copy-and-paste errors"], +["NOP", "IF 1 ENDIF", "P2SH,STRICTENC", "INVALID_STACK_OPERATION", "The following tests check the if(stack.size() < N) tests in each opcode"], +["NOP", "NOTIF 1 ENDIF", "P2SH,STRICTENC", "INVALID_STACK_OPERATION", "They are here to catch copy-and-paste errors"], ["NOP", "VERIFY 1", "P2SH,STRICTENC", "INVALID_STACK_OPERATION", "Most of them are duplicated elsewhere,"], ["NOP", "TOALTSTACK 1", "P2SH,STRICTENC", "INVALID_STACK_OPERATION", "but, hey, more is always better, right?"], @@ -2546,14 +2546,14 @@ ["0x02 0x0100 0x03 0x635168", "HASH160 0x14 0xe7309652a8e3f600f06f5d8d52d6df03d2176cc3 EQUAL", "P2SH,WITNESS,MINIMALIF", "OK"], ["0 0x03 0x635168", "HASH160 0x14 0xe7309652a8e3f600f06f5d8d52d6df03d2176cc3 EQUAL", "P2SH,WITNESS,MINIMALIF", "EVAL_FALSE"], ["0x01 0x00 0x03 0x635168", "HASH160 0x14 0xe7309652a8e3f600f06f5d8d52d6df03d2176cc3 EQUAL", "P2SH,WITNESS,MINIMALIF", "EVAL_FALSE"], -["0x03 0x635168", "HASH160 0x14 0xe7309652a8e3f600f06f5d8d52d6df03d2176cc3 EQUAL", "P2SH,WITNESS,MINIMALIF", "UNBALANCED_CONDITIONAL"], +["0x03 0x635168", "HASH160 0x14 0xe7309652a8e3f600f06f5d8d52d6df03d2176cc3 EQUAL", "P2SH,WITNESS,MINIMALIF", "INVALID_STACK_OPERATION"], ["Normal P2SH NOTIF 1 ENDIF"], ["1 0x03 0x645168", "HASH160 0x14 0x0c3f8fe3d6ca266e76311ecda544c67d15fdd5b0 EQUAL", "P2SH,WITNESS,MINIMALIF", "EVAL_FALSE"], ["2 0x03 0x645168", "HASH160 0x14 0x0c3f8fe3d6ca266e76311ecda544c67d15fdd5b0 EQUAL", "P2SH,WITNESS,MINIMALIF", "EVAL_FALSE"], ["0x02 0x0100 0x03 0x645168", "HASH160 0x14 0x0c3f8fe3d6ca266e76311ecda544c67d15fdd5b0 EQUAL", "P2SH,WITNESS,MINIMALIF", "EVAL_FALSE"], ["0 0x03 0x645168", "HASH160 0x14 0x0c3f8fe3d6ca266e76311ecda544c67d15fdd5b0 EQUAL", "P2SH,WITNESS,MINIMALIF", "OK"], ["0x01 0x00 0x03 0x645168", "HASH160 0x14 0x0c3f8fe3d6ca266e76311ecda544c67d15fdd5b0 EQUAL", "P2SH,WITNESS,MINIMALIF", "OK"], -["0x03 0x645168", "HASH160 0x14 0x0c3f8fe3d6ca266e76311ecda544c67d15fdd5b0 EQUAL", "P2SH,WITNESS,MINIMALIF", "UNBALANCED_CONDITIONAL"], +["0x03 0x645168", "HASH160 0x14 0x0c3f8fe3d6ca266e76311ecda544c67d15fdd5b0 EQUAL", "P2SH,WITNESS,MINIMALIF", "INVALID_STACK_OPERATION"], ["P2WSH IF 1 ENDIF"], [["01", "635168", 0.00000001], "", "0 0x20 0xc7eaf06d5ae01a58e376e126eb1e6fab2036076922b96b2711ffbec1e590665d", "P2SH,WITNESS", "OK"], [["02", "635168", 0.00000001], "", "0 0x20 0xc7eaf06d5ae01a58e376e126eb1e6fab2036076922b96b2711ffbec1e590665d", "P2SH,WITNESS", "OK"], @@ -2565,8 +2565,8 @@ [["0100", "635168", 0.00000001], "", "0 0x20 0xc7eaf06d5ae01a58e376e126eb1e6fab2036076922b96b2711ffbec1e590665d", "P2SH,WITNESS,MINIMALIF", "MINIMALIF"], [["", "635168", 0.00000001], "", "0 0x20 0xc7eaf06d5ae01a58e376e126eb1e6fab2036076922b96b2711ffbec1e590665d", "P2SH,WITNESS,MINIMALIF", "CLEANSTACK"], [["00", "635168", 0.00000001], "", "0 0x20 0xc7eaf06d5ae01a58e376e126eb1e6fab2036076922b96b2711ffbec1e590665d", "P2SH,WITNESS,MINIMALIF", "MINIMALIF"], -[["635168", 0.00000001], "", "0 0x20 0xc7eaf06d5ae01a58e376e126eb1e6fab2036076922b96b2711ffbec1e590665d", "P2SH,WITNESS", "UNBALANCED_CONDITIONAL"], -[["635168", 0.00000001], "", "0 0x20 0xc7eaf06d5ae01a58e376e126eb1e6fab2036076922b96b2711ffbec1e590665d", "P2SH,WITNESS,MINIMALIF", "UNBALANCED_CONDITIONAL"], +[["635168", 0.00000001], "", "0 0x20 0xc7eaf06d5ae01a58e376e126eb1e6fab2036076922b96b2711ffbec1e590665d", "P2SH,WITNESS", "INVALID_STACK_OPERATION"], +[["635168", 0.00000001], "", "0 0x20 0xc7eaf06d5ae01a58e376e126eb1e6fab2036076922b96b2711ffbec1e590665d", "P2SH,WITNESS,MINIMALIF", "INVALID_STACK_OPERATION"], ["P2WSH NOTIF 1 ENDIF"], [["01", "645168", 0.00000001], "", "0 0x20 0xf913eacf2e38a5d6fc3a8311d72ae704cb83866350a984dd3e5eb76d2a8c28e8", "P2SH,WITNESS", "CLEANSTACK"], [["02", "645168", 0.00000001], "", "0 0x20 0xf913eacf2e38a5d6fc3a8311d72ae704cb83866350a984dd3e5eb76d2a8c28e8", "P2SH,WITNESS", "CLEANSTACK"], @@ -2578,8 +2578,8 @@ [["0100", "645168", 0.00000001], "", "0 0x20 0xf913eacf2e38a5d6fc3a8311d72ae704cb83866350a984dd3e5eb76d2a8c28e8", "P2SH,WITNESS,MINIMALIF", "MINIMALIF"], [["", "645168", 0.00000001], "", "0 0x20 0xf913eacf2e38a5d6fc3a8311d72ae704cb83866350a984dd3e5eb76d2a8c28e8", "P2SH,WITNESS,MINIMALIF", "OK"], [["00", "645168", 0.00000001], "", "0 0x20 0xf913eacf2e38a5d6fc3a8311d72ae704cb83866350a984dd3e5eb76d2a8c28e8", "P2SH,WITNESS,MINIMALIF", "MINIMALIF"], -[["645168", 0.00000001], "", "0 0x20 0xf913eacf2e38a5d6fc3a8311d72ae704cb83866350a984dd3e5eb76d2a8c28e8", "P2SH,WITNESS", "UNBALANCED_CONDITIONAL"], -[["645168", 0.00000001], "", "0 0x20 0xf913eacf2e38a5d6fc3a8311d72ae704cb83866350a984dd3e5eb76d2a8c28e8", "P2SH,WITNESS,MINIMALIF", "UNBALANCED_CONDITIONAL"], +[["645168", 0.00000001], "", "0 0x20 0xf913eacf2e38a5d6fc3a8311d72ae704cb83866350a984dd3e5eb76d2a8c28e8", "P2SH,WITNESS", "INVALID_STACK_OPERATION"], +[["645168", 0.00000001], "", "0 0x20 0xf913eacf2e38a5d6fc3a8311d72ae704cb83866350a984dd3e5eb76d2a8c28e8", "P2SH,WITNESS,MINIMALIF", "INVALID_STACK_OPERATION"], @@ -2594,8 +2594,8 @@ [["0100", "635168", 0.00000001], "0x22 0x0020c7eaf06d5ae01a58e376e126eb1e6fab2036076922b96b2711ffbec1e590665d", "HASH160 0x14 0x9b27ee6d9010c21bf837b334d043be5d150e7ba7 EQUAL", "P2SH,WITNESS,MINIMALIF", "MINIMALIF"], [["", "635168", 0.00000001], "0x22 0x0020c7eaf06d5ae01a58e376e126eb1e6fab2036076922b96b2711ffbec1e590665d", "HASH160 0x14 0x9b27ee6d9010c21bf837b334d043be5d150e7ba7 EQUAL", "P2SH,WITNESS,MINIMALIF", "CLEANSTACK"], [["00", "635168", 0.00000001], "0x22 0x0020c7eaf06d5ae01a58e376e126eb1e6fab2036076922b96b2711ffbec1e590665d", "HASH160 0x14 0x9b27ee6d9010c21bf837b334d043be5d150e7ba7 EQUAL", "P2SH,WITNESS,MINIMALIF", "MINIMALIF"], -[["635168", 0.00000001], "0x22 0x0020c7eaf06d5ae01a58e376e126eb1e6fab2036076922b96b2711ffbec1e590665d", "HASH160 0x14 0x9b27ee6d9010c21bf837b334d043be5d150e7ba7 EQUAL", "P2SH,WITNESS", "UNBALANCED_CONDITIONAL"], -[["635168", 0.00000001], "0x22 0x0020c7eaf06d5ae01a58e376e126eb1e6fab2036076922b96b2711ffbec1e590665d", "HASH160 0x14 0x9b27ee6d9010c21bf837b334d043be5d150e7ba7 EQUAL", "P2SH,WITNESS,MINIMALIF", "UNBALANCED_CONDITIONAL"], +[["635168", 0.00000001], "0x22 0x0020c7eaf06d5ae01a58e376e126eb1e6fab2036076922b96b2711ffbec1e590665d", "HASH160 0x14 0x9b27ee6d9010c21bf837b334d043be5d150e7ba7 EQUAL", "P2SH,WITNESS", "INVALID_STACK_OPERATION"], +[["635168", 0.00000001], "0x22 0x0020c7eaf06d5ae01a58e376e126eb1e6fab2036076922b96b2711ffbec1e590665d", "HASH160 0x14 0x9b27ee6d9010c21bf837b334d043be5d150e7ba7 EQUAL", "P2SH,WITNESS,MINIMALIF", "INVALID_STACK_OPERATION"], ["P2SH-P2WSH NOTIF 1 ENDIF"], [["01", "645168", 0.00000001], "0x22 0x0020f913eacf2e38a5d6fc3a8311d72ae704cb83866350a984dd3e5eb76d2a8c28e8", "HASH160 0x14 0xdbb7d1c0a56b7a9c423300c8cca6e6e065baf1dc EQUAL", "P2SH,WITNESS", "CLEANSTACK"], [["02", "645168", 0.00000001], "0x22 0x0020f913eacf2e38a5d6fc3a8311d72ae704cb83866350a984dd3e5eb76d2a8c28e8", "HASH160 0x14 0xdbb7d1c0a56b7a9c423300c8cca6e6e065baf1dc EQUAL", "P2SH,WITNESS", "CLEANSTACK"], @@ -2607,8 +2607,8 @@ [["0100", "645168", 0.00000001], "0x22 0x0020f913eacf2e38a5d6fc3a8311d72ae704cb83866350a984dd3e5eb76d2a8c28e8", "HASH160 0x14 0xdbb7d1c0a56b7a9c423300c8cca6e6e065baf1dc EQUAL", "P2SH,WITNESS,MINIMALIF", "MINIMALIF"], [["", "645168", 0.00000001], "0x22 0x0020f913eacf2e38a5d6fc3a8311d72ae704cb83866350a984dd3e5eb76d2a8c28e8", "HASH160 0x14 0xdbb7d1c0a56b7a9c423300c8cca6e6e065baf1dc EQUAL", "P2SH,WITNESS,MINIMALIF", "OK"], [["00", "645168", 0.00000001], "0x22 0x0020f913eacf2e38a5d6fc3a8311d72ae704cb83866350a984dd3e5eb76d2a8c28e8", "HASH160 0x14 0xdbb7d1c0a56b7a9c423300c8cca6e6e065baf1dc EQUAL", "P2SH,WITNESS,MINIMALIF", "MINIMALIF"], -[["645168", 0.00000001], "0x22 0x0020f913eacf2e38a5d6fc3a8311d72ae704cb83866350a984dd3e5eb76d2a8c28e8", "HASH160 0x14 0xdbb7d1c0a56b7a9c423300c8cca6e6e065baf1dc EQUAL", "P2SH,WITNESS", "UNBALANCED_CONDITIONAL"], -[["645168", 0.00000001], "0x22 0x0020f913eacf2e38a5d6fc3a8311d72ae704cb83866350a984dd3e5eb76d2a8c28e8", "HASH160 0x14 0xdbb7d1c0a56b7a9c423300c8cca6e6e065baf1dc EQUAL", "P2SH,WITNESS,MINIMALIF", "UNBALANCED_CONDITIONAL"], +[["645168", 0.00000001], "0x22 0x0020f913eacf2e38a5d6fc3a8311d72ae704cb83866350a984dd3e5eb76d2a8c28e8", "HASH160 0x14 0xdbb7d1c0a56b7a9c423300c8cca6e6e065baf1dc EQUAL", "P2SH,WITNESS", "INVALID_STACK_OPERATION"], +[["645168", 0.00000001], "0x22 0x0020f913eacf2e38a5d6fc3a8311d72ae704cb83866350a984dd3e5eb76d2a8c28e8", "HASH160 0x14 0xdbb7d1c0a56b7a9c423300c8cca6e6e065baf1dc EQUAL", "P2SH,WITNESS,MINIMALIF", "INVALID_STACK_OPERATION"], ["NULLFAIL should cover all signatures and signatures only"], ["0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0", "0x01 0x14 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 0x01 0x14 CHECKMULTISIG NOT", "DERSIG", "OK", "BIP66 and NULLFAIL-compliant"], diff --git a/test/functional/data/invalid_txs.py b/test/functional/data/invalid_txs.py index d2d7202d8601..f0f2e61a592e 100644 --- a/test/functional/data/invalid_txs.py +++ b/test/functional/data/invalid_txs.py @@ -229,7 +229,7 @@ class InvalidOPIFConstruction(BadTxTemplate): def get_tx(self): return create_tx_with_script( - self.spend_tx, 0, script_sig=b'\x64' * 35, + self.spend_tx, 0, script_sig=b'\x68' * 35, amount=(self.spend_avail // 2)) From e842eb90bb6db39076a43b010c0c7898d50b8d92 Mon Sep 17 00:00:00 2001 From: Novo Date: Tue, 22 Jul 2025 07:46:06 +0100 Subject: [PATCH 02/25] descriptors: add HavePrivateKeys() Previously, to determine if a desc is watchonly, `ToPrivateString()`, was used. It returns `false` if there is at least one pubkey in the descriptor for which the provider does not have a private key. ToPrivateString() behaviour will change in the following commits to only return `false` if no priv keys could be found for the pub keys in the descriptor. HavePrivateKeys() is added here to replace the use of ToPrivateString() for determining if a descriptor is 'watchonly'. Co-authored-by: rkrux --- src/script/descriptor.cpp | 19 +++++++++++++++++++ src/script/descriptor.h | 7 +++++++ src/test/descriptor_tests.cpp | 5 ++++- src/wallet/test/walletload_tests.cpp | 1 + 4 files changed, 31 insertions(+), 1 deletion(-) diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index a42943318d65..e2f7b8e873a6 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -835,6 +835,25 @@ class DescriptorImpl : public Descriptor return true; } + // NOLINTNEXTLINE(misc-no-recursion) + bool HavePrivateKeys(const SigningProvider& arg) const override + { + if (m_pubkey_args.empty() && m_subdescriptor_args.empty()) return false; + + for (const auto& sub: m_subdescriptor_args) { + if (!sub->HavePrivateKeys(arg)) return false; + } + + FlatSigningProvider tmp_provider; + for (const auto& pubkey : m_pubkey_args) { + tmp_provider.keys.clear(); + pubkey->GetPrivKey(0, arg, tmp_provider); + if (tmp_provider.keys.empty()) return false; + } + + return true; + } + // NOLINTNEXTLINE(misc-no-recursion) bool IsRange() const final { diff --git a/src/script/descriptor.h b/src/script/descriptor.h index 9a018300ebe5..c7863933ef96 100644 --- a/src/script/descriptor.h +++ b/src/script/descriptor.h @@ -111,6 +111,13 @@ struct Descriptor { /** Whether this descriptor will return one scriptPubKey or multiple (aka is or is not combo) */ virtual bool IsSingleType() const = 0; + /** Whether the given provider has all private keys required by this descriptor. + * @return `false` if the descriptor doesn't have any keys or subdescriptors, + * or if the provider does not have all private keys required by + * the descriptor. + */ + virtual bool HavePrivateKeys(const SigningProvider& provider) const = 0; + /** Convert the descriptor to a private string. This fails if the provided provider does not have the relevant private keys. */ virtual bool ToPrivateString(const SigningProvider& provider, std::string& out) const = 0; diff --git a/src/test/descriptor_tests.cpp b/src/test/descriptor_tests.cpp index 098007c4cf90..17c756b60338 100644 --- a/src/test/descriptor_tests.cpp +++ b/src/test/descriptor_tests.cpp @@ -49,7 +49,7 @@ constexpr int SIGNABLE = 1 << 3; // We can sign with this descriptor (this is no constexpr int DERIVE_HARDENED = 1 << 4; // The final derivation is hardened, i.e. ends with *' or *h constexpr int MIXED_PUBKEYS = 1 << 5; constexpr int XONLY_KEYS = 1 << 6; // X-only pubkeys are in use (and thus inferring/caching may swap parity of pubkeys/keyids) -constexpr int MISSING_PRIVKEYS = 1 << 7; // Not all private keys are available, so ToPrivateString will fail. +constexpr int MISSING_PRIVKEYS = 1 << 7; // Not all private keys are available. ToPrivateString() will fail and HavePrivateKeys() will return `false`. constexpr int SIGNABLE_FAILS = 1 << 8; // We can sign with this descriptor, but actually trying to sign will fail constexpr int MUSIG = 1 << 9; // This is a MuSig so key counts will have an extra key constexpr int MUSIG_DERIVATION = 1 << 10; // MuSig with BIP 328 derivation from the aggregate key @@ -243,6 +243,9 @@ void DoCheck(std::string prv, std::string pub, const std::string& norm_pub, int } else { BOOST_CHECK_MESSAGE(EqualDescriptor(prv, prv1), "Private ser: " + prv1 + " Private desc: " + prv); } + BOOST_CHECK(!parse_priv->HavePrivateKeys(keys_pub)); + BOOST_CHECK(parse_pub->HavePrivateKeys(keys_priv)); + BOOST_CHECK(!parse_priv->ToPrivateString(keys_pub, prv1)); BOOST_CHECK(parse_pub->ToPrivateString(keys_priv, prv1)); if (expected_prv) { diff --git a/src/wallet/test/walletload_tests.cpp b/src/wallet/test/walletload_tests.cpp index 0c69849d0b64..adbd9aa1baec 100644 --- a/src/wallet/test/walletload_tests.cpp +++ b/src/wallet/test/walletload_tests.cpp @@ -26,6 +26,7 @@ class DummyDescriptor final : public Descriptor { bool IsRange() const override { return false; } bool IsSolvable() const override { return false; } bool IsSingleType() const override { return true; } + bool HavePrivateKeys(const SigningProvider&) const override { return false; } bool ToPrivateString(const SigningProvider& provider, std::string& out) const override { return false; } bool ToNormalizedString(const SigningProvider& provider, std::string& out, const DescriptorCache* cache = nullptr) const override { return false; } bool Expand(int pos, const SigningProvider& provider, std::vector& output_scripts, FlatSigningProvider& out, DescriptorCache* write_cache = nullptr) const override { return false; }; From 2dc74e3f4e5e6f01c8810359b91041bc6865f1c7 Mon Sep 17 00:00:00 2001 From: Novo Date: Tue, 13 May 2025 13:32:06 +0100 Subject: [PATCH 03/25] wallet/migration: use HavePrivateKeys in place of ToPrivateString ToPrivateString() behaviour will be modified in the following commits. In order to keep the scope of this PR limited to the RPC behaviour, this commit updates wallet migration to use 'Descriptor::HavePrivateKeys()' in place of 'Descriptor::ToPrivateString()' to determine watchonly descriptors. A follow-up PR can be opened to update migration logic to exclude descriptors with some private keys from the watchonly migration wallet. --- src/wallet/scriptpubkeyman.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 276817146c6e..708f59892eba 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -717,10 +717,9 @@ std::optional LegacyDataSPKM::MigrateToDescriptor() std::vector desc_spks; - // Make the descriptor string with private keys - std::string desc_str; - bool watchonly = !desc->ToPrivateString(*this, desc_str); - if (watchonly && !m_storage.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { + // If we can't provide all private keys for this inferred descriptor, + // but this wallet is not watch-only, migrate it to the watch-only wallet. + if (!desc->HavePrivateKeys(*this) && !m_storage.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { out.watch_descs.emplace_back(desc->ToString(), creation_time); // Get the scriptPubKeys without writing this to the wallet From 5c4db25b61d417a567f152169f4ab21a491afb95 Mon Sep 17 00:00:00 2001 From: Novo Date: Fri, 1 Aug 2025 14:39:11 +0100 Subject: [PATCH 04/25] descriptor: refactor ToPrivateString for providers This commit modifies the Pubkey providers to return the public string if private data is not available. This is setup for a future commit to make Descriptor::ToPrivateString return strings with missing private key information. Co-authored-by: rkrux --- src/script/descriptor.cpp | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index e2f7b8e873a6..89af1d9367fc 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -204,7 +204,11 @@ struct PubkeyProvider /** Get the descriptor string form. */ virtual std::string ToString(StringType type=StringType::PUBLIC) const = 0; - /** Get the descriptor string form including private data (if available in arg). */ + /** Get the descriptor string form including private data (if available in arg). + * If the private data is not available, the output string in the "out" parameter + * will not contain any private key information, + * and this function will return "false". + */ virtual bool ToPrivateString(const SigningProvider& arg, std::string& out) const = 0; /** Get the descriptor string form with the xpub at the last hardened derivation, @@ -260,9 +264,9 @@ class OriginPubkeyProvider final : public PubkeyProvider bool ToPrivateString(const SigningProvider& arg, std::string& ret) const override { std::string sub; - if (!m_provider->ToPrivateString(arg, sub)) return false; + bool has_priv_key{m_provider->ToPrivateString(arg, sub)}; ret = "[" + OriginString(StringType::PUBLIC) + "]" + std::move(sub); - return true; + return has_priv_key; } bool ToNormalizedString(const SigningProvider& arg, std::string& ret, const DescriptorCache* cache) const override { @@ -329,7 +333,10 @@ class ConstPubkeyProvider final : public PubkeyProvider bool ToPrivateString(const SigningProvider& arg, std::string& ret) const override { std::optional key = GetPrivKey(arg); - if (!key) return false; + if (!key) { + ret = ToString(StringType::PUBLIC); + return false; + } ret = EncodeSecret(*key); return true; } @@ -492,7 +499,10 @@ class BIP32PubkeyProvider final : public PubkeyProvider bool ToPrivateString(const SigningProvider& arg, std::string& out) const override { CExtKey key; - if (!GetExtKey(arg, key)) return false; + if (!GetExtKey(arg, key)) { + out = ToString(StringType::PUBLIC); + return false; + } out = EncodeExtKey(key) + FormatHDKeypath(m_path, /*apostrophe=*/m_apostrophe); if (IsRange()) { out += "/*"; @@ -710,17 +720,14 @@ class MuSigPubkeyProvider final : public PubkeyProvider std::string tmp; if (pubkey->ToPrivateString(arg, tmp)) { any_privkeys = true; - out += tmp; - } else { - out += pubkey->ToString(); } + out += tmp; } out += ")"; out += FormatHDKeypath(m_path); if (IsRangedDerivation()) { out += "/*"; } - if (!any_privkeys) out.clear(); return any_privkeys; } bool ToNormalizedString(const SigningProvider& arg, std::string& out, const DescriptorCache* cache = nullptr) const override From 9e5e9824f11b1b0f9e2a4e28124edbb1616af519 Mon Sep 17 00:00:00 2001 From: Novo Date: Fri, 1 Aug 2025 14:31:25 +0100 Subject: [PATCH 05/25] descriptor: ToPrivateString() pass if at least 1 priv key exists - Refactor Descriptor::ToPrivateString() to allow descriptors with missing private keys to be printed. Useful in descriptors with multiple keys e.g tr() etc. - The existing behaviour of listdescriptors is preserved as much as possible, if no private keys are availablle ToPrivateString will return false --- src/script/descriptor.cpp | 68 +++++++++++++++++++++++++---------- src/script/descriptor.h | 8 ++++- src/script/miniscript.h | 26 ++++++++++---- src/test/descriptor_tests.cpp | 8 ++++- src/test/fuzz/miniscript.cpp | 8 +++-- src/test/miniscript_tests.cpp | 2 +- 6 files changed, 90 insertions(+), 30 deletions(-) diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index 89af1d9367fc..89766b52cffb 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -877,13 +877,19 @@ class DescriptorImpl : public Descriptor virtual bool ToStringSubScriptHelper(const SigningProvider* arg, std::string& ret, const StringType type, const DescriptorCache* cache = nullptr) const { size_t pos = 0; + bool is_private{type == StringType::PRIVATE}; + // For private string output, track if at least one key has a private key available. + // Initialize to true for non-private types. + bool any_success{!is_private}; for (const auto& scriptarg : m_subdescriptor_args) { if (pos++) ret += ","; std::string tmp; - if (!scriptarg->ToStringHelper(arg, tmp, type, cache)) return false; + bool subscript_res{scriptarg->ToStringHelper(arg, tmp, type, cache)}; + if (!is_private && !subscript_res) return false; + any_success = any_success || subscript_res; ret += tmp; } - return true; + return any_success; } // NOLINTNEXTLINE(misc-no-recursion) @@ -892,6 +898,11 @@ class DescriptorImpl : public Descriptor std::string extra = ToStringExtra(); size_t pos = extra.size() > 0 ? 1 : 0; std::string ret = m_name + "(" + extra; + bool is_private{type == StringType::PRIVATE}; + // For private string output, track if at least one key has a private key available. + // Initialize to true for non-private types. + bool any_success{!is_private}; + for (const auto& pubkey : m_pubkey_args) { if (pos++) ret += ","; std::string tmp; @@ -900,7 +911,7 @@ class DescriptorImpl : public Descriptor if (!pubkey->ToNormalizedString(*arg, tmp, cache)) return false; break; case StringType::PRIVATE: - if (!pubkey->ToPrivateString(*arg, tmp)) return false; + any_success = pubkey->ToPrivateString(*arg, tmp) || any_success; break; case StringType::PUBLIC: tmp = pubkey->ToString(); @@ -912,10 +923,12 @@ class DescriptorImpl : public Descriptor ret += tmp; } std::string subscript; - if (!ToStringSubScriptHelper(arg, subscript, type, cache)) return false; + bool subscript_res{ToStringSubScriptHelper(arg, subscript, type, cache)}; + if (!is_private && !subscript_res) return false; + any_success = any_success || subscript_res; if (pos && subscript.size()) ret += ','; out = std::move(ret) + std::move(subscript) + ")"; - return true; + return any_success; } std::string ToString(bool compat_format) const final @@ -927,9 +940,9 @@ class DescriptorImpl : public Descriptor bool ToPrivateString(const SigningProvider& arg, std::string& out) const override { - bool ret = ToStringHelper(&arg, out, StringType::PRIVATE); + bool has_priv_key{ToStringHelper(&arg, out, StringType::PRIVATE)}; out = AddChecksum(out); - return ret; + return has_priv_key; } bool ToNormalizedString(const SigningProvider& arg, std::string& out, const DescriptorCache* cache) const override final @@ -1410,8 +1423,20 @@ class TRDescriptor final : public DescriptorImpl } bool ToStringSubScriptHelper(const SigningProvider* arg, std::string& ret, const StringType type, const DescriptorCache* cache = nullptr) const override { - if (m_depths.empty()) return true; + if (m_depths.empty()) { + // If there are no sub-descriptors and a PRIVATE string + // is requested, return `false` to indicate that the presence + // of a private key depends solely on the internal key (which is checked + // in the caller), not on any sub-descriptor. This ensures correct behavior for + // descriptors like tr(internal_key) when checking for private keys. + return type != StringType::PRIVATE; + } std::vector path; + bool is_private{type == StringType::PRIVATE}; + // For private string output, track if at least one key has a private key available. + // Initialize to true for non-private types. + bool any_success{!is_private}; + for (size_t pos = 0; pos < m_depths.size(); ++pos) { if (pos) ret += ','; while ((int)path.size() <= m_depths[pos]) { @@ -1419,7 +1444,9 @@ class TRDescriptor final : public DescriptorImpl path.push_back(false); } std::string tmp; - if (!m_subdescriptor_args[pos]->ToStringHelper(arg, tmp, type, cache)) return false; + bool subscript_res{m_subdescriptor_args[pos]->ToStringHelper(arg, tmp, type, cache)}; + if (!is_private && !subscript_res) return false; + any_success = any_success || subscript_res; ret += tmp; while (!path.empty() && path.back()) { if (path.size() > 1) ret += '}'; @@ -1427,7 +1454,7 @@ class TRDescriptor final : public DescriptorImpl } if (!path.empty()) path.back() = true; } - return true; + return any_success; } public: TRDescriptor(std::unique_ptr internal_key, std::vector> descs, std::vector depths) : @@ -1520,15 +1547,16 @@ class StringMaker { const DescriptorCache* cache LIFETIMEBOUND) : m_arg(arg), m_pubkeys(pubkeys), m_type(type), m_cache(cache) {} - std::optional ToString(uint32_t key) const + std::optional ToString(uint32_t key, bool& has_priv_key) const { std::string ret; + has_priv_key = false; switch (m_type) { case DescriptorImpl::StringType::PUBLIC: ret = m_pubkeys[key]->ToString(); break; case DescriptorImpl::StringType::PRIVATE: - if (!m_pubkeys[key]->ToPrivateString(*m_arg, ret)) return {}; + has_priv_key = m_pubkeys[key]->ToPrivateString(*m_arg, ret); break; case DescriptorImpl::StringType::NORMALIZED: if (!m_pubkeys[key]->ToNormalizedString(*m_arg, ret, m_cache)) return {}; @@ -1568,11 +1596,15 @@ class MiniscriptDescriptor final : public DescriptorImpl bool ToStringHelper(const SigningProvider* arg, std::string& out, const StringType type, const DescriptorCache* cache = nullptr) const override { - if (const auto res = m_node->ToString(StringMaker(arg, m_pubkey_args, type, cache))) { - out = *res; - return true; + bool has_priv_key{false}; + auto res = m_node->ToString(StringMaker(arg, m_pubkey_args, type, cache), has_priv_key); + if (res) out = *res; + if (type == StringType::PRIVATE) { + Assume(res.has_value()); + return has_priv_key; + } else { + return res.has_value(); } - return false; } bool IsSolvable() const override { return true; } @@ -2110,7 +2142,7 @@ struct KeyParser { return key; } - std::optional ToString(const Key& key) const + std::optional ToString(const Key& key, bool&) const { return m_keys.at(key).at(0)->ToString(); } @@ -2507,7 +2539,7 @@ std::vector> ParseScript(uint32_t& key_exp_index // Try to find the first insane sub for better error reporting. auto insane_node = node.get(); if (const auto sub = node->FindInsaneSub()) insane_node = sub; - if (const auto str = insane_node->ToString(parser)) error = *str; + error = *insane_node->ToString(parser); if (!insane_node->IsValid()) { error += " is invalid"; } else if (!node->IsSane()) { diff --git a/src/script/descriptor.h b/src/script/descriptor.h index c7863933ef96..47c568857e51 100644 --- a/src/script/descriptor.h +++ b/src/script/descriptor.h @@ -118,7 +118,13 @@ struct Descriptor { */ virtual bool HavePrivateKeys(const SigningProvider& provider) const = 0; - /** Convert the descriptor to a private string. This fails if the provided provider does not have the relevant private keys. */ + /** Convert the descriptor to a private string. This uses public keys if the relevant private keys are not in the SigningProvider. + * If none of the relevant private keys are available, the output string in the "out" parameter will not contain any private key information, + * and this function will return "false". + * @param[in] provider The SigningProvider to query for private keys. + * @param[out] out The resulting descriptor string, containing private keys if available. + * @returns true if at least one private key available. + */ virtual bool ToPrivateString(const SigningProvider& provider, std::string& out) const = 0; /** Convert the descriptor to a normalized string. Normalized descriptors have the xpub at the last hardened step. This fails if the provided provider does not have the private keys to derive that xpub. */ diff --git a/src/script/miniscript.h b/src/script/miniscript.h index 54ae777cf926..6e684e6d9f26 100644 --- a/src/script/miniscript.h +++ b/src/script/miniscript.h @@ -826,6 +826,12 @@ struct Node { template std::optional ToString(const CTx& ctx) const { + bool dummy{false}; + return ToString(ctx, dummy); + } + + template + std::optional ToString(const CTx& ctx, bool& has_priv_key) const { // To construct the std::string representation for a Miniscript object, we use // the TreeEvalMaybe algorithm. The State is a boolean: whether the parent node is a // wrapper. If so, non-wrapper expressions must be prefixed with a ":". @@ -838,10 +844,16 @@ struct Node { (node.fragment == Fragment::OR_I && node.subs[0]->fragment == Fragment::JUST_0) || (node.fragment == Fragment::OR_I && node.subs[1]->fragment == Fragment::JUST_0)); }; + auto toString = [&ctx, &has_priv_key](Key key) -> std::optional { + bool fragment_has_priv_key{false}; + auto key_str{ctx.ToString(key, fragment_has_priv_key)}; + if (key_str) has_priv_key = has_priv_key || fragment_has_priv_key; + return key_str; + }; // The upward function computes for a node, given whether its parent is a wrapper, // and the string representations of its child nodes, the string representation of the node. const bool is_tapscript{IsTapscript(m_script_ctx)}; - auto upfn = [&ctx, is_tapscript](bool wrapped, const Node& node, std::span subs) -> std::optional { + auto upfn = [is_tapscript, &toString](bool wrapped, const Node& node, std::span subs) -> std::optional { std::string ret = wrapped ? ":" : ""; switch (node.fragment) { @@ -850,13 +862,13 @@ struct Node { case Fragment::WRAP_C: if (node.subs[0]->fragment == Fragment::PK_K) { // pk(K) is syntactic sugar for c:pk_k(K) - auto key_str = ctx.ToString(node.subs[0]->keys[0]); + auto key_str = toString(node.subs[0]->keys[0]); if (!key_str) return {}; return std::move(ret) + "pk(" + std::move(*key_str) + ")"; } if (node.subs[0]->fragment == Fragment::PK_H) { // pkh(K) is syntactic sugar for c:pk_h(K) - auto key_str = ctx.ToString(node.subs[0]->keys[0]); + auto key_str = toString(node.subs[0]->keys[0]); if (!key_str) return {}; return std::move(ret) + "pkh(" + std::move(*key_str) + ")"; } @@ -877,12 +889,12 @@ struct Node { } switch (node.fragment) { case Fragment::PK_K: { - auto key_str = ctx.ToString(node.keys[0]); + auto key_str = toString(node.keys[0]); if (!key_str) return {}; return std::move(ret) + "pk_k(" + std::move(*key_str) + ")"; } case Fragment::PK_H: { - auto key_str = ctx.ToString(node.keys[0]); + auto key_str = toString(node.keys[0]); if (!key_str) return {}; return std::move(ret) + "pk_h(" + std::move(*key_str) + ")"; } @@ -908,7 +920,7 @@ struct Node { CHECK_NONFATAL(!is_tapscript); auto str = std::move(ret) + "multi(" + util::ToString(node.k); for (const auto& key : node.keys) { - auto key_str = ctx.ToString(key); + auto key_str = toString(key); if (!key_str) return {}; str += "," + std::move(*key_str); } @@ -918,7 +930,7 @@ struct Node { CHECK_NONFATAL(is_tapscript); auto str = std::move(ret) + "multi_a(" + util::ToString(node.k); for (const auto& key : node.keys) { - auto key_str = ctx.ToString(key); + auto key_str = toString(key); if (!key_str) return {}; str += "," + std::move(*key_str); } diff --git a/src/test/descriptor_tests.cpp b/src/test/descriptor_tests.cpp index 17c756b60338..d49a4885ba3d 100644 --- a/src/test/descriptor_tests.cpp +++ b/src/test/descriptor_tests.cpp @@ -49,7 +49,7 @@ constexpr int SIGNABLE = 1 << 3; // We can sign with this descriptor (this is no constexpr int DERIVE_HARDENED = 1 << 4; // The final derivation is hardened, i.e. ends with *' or *h constexpr int MIXED_PUBKEYS = 1 << 5; constexpr int XONLY_KEYS = 1 << 6; // X-only pubkeys are in use (and thus inferring/caching may swap parity of pubkeys/keyids) -constexpr int MISSING_PRIVKEYS = 1 << 7; // Not all private keys are available. ToPrivateString() will fail and HavePrivateKeys() will return `false`. +constexpr int MISSING_PRIVKEYS = 1 << 7; // Not all private keys are available. ToPrivateString() will return true if there is at least one private key and HavePrivateKeys() will return `false`. constexpr int SIGNABLE_FAILS = 1 << 8; // We can sign with this descriptor, but actually trying to sign will fail constexpr int MUSIG = 1 << 9; // This is a MuSig so key counts will have an extra key constexpr int MUSIG_DERIVATION = 1 << 10; // MuSig with BIP 328 derivation from the aggregate key @@ -264,6 +264,12 @@ void DoCheck(std::string prv, std::string pub, const std::string& norm_pub, int parse_pub->ExpandPrivate(0, keys_priv, pub_prov); BOOST_CHECK_MESSAGE(EqualSigningProviders(priv_prov, pub_prov), "Private desc: " + prv + " Pub desc: " + pub); + } else if (keys_priv.keys.size() > 0) { + // If there is at least one private key, ToPrivateString() should return true and include that key + std::string prv_str; + BOOST_CHECK(parse_priv->ToPrivateString(keys_priv, prv_str)); + size_t checksum_len = 9; // Including the '#' character + BOOST_CHECK_MESSAGE(prv == prv_str.substr(0, prv_str.length() - checksum_len), prv); } // Check that private can produce the normalized descriptors diff --git a/src/test/fuzz/miniscript.cpp b/src/test/fuzz/miniscript.cpp index 421953130294..edc9772a9ccd 100644 --- a/src/test/fuzz/miniscript.cpp +++ b/src/test/fuzz/miniscript.cpp @@ -124,10 +124,14 @@ struct ParserContext { return a < b; } - std::optional ToString(const Key& key) const + std::optional ToString(const Key& key, bool& has_priv_key) const { + has_priv_key = false; auto it = TEST_DATA.dummy_key_idx_map.find(key); - if (it == TEST_DATA.dummy_key_idx_map.end()) return {}; + if (it == TEST_DATA.dummy_key_idx_map.end()) { + return HexStr(key); + } + has_priv_key = true; uint8_t idx = it->second; return HexStr(std::span{&idx, 1}); } diff --git a/src/test/miniscript_tests.cpp b/src/test/miniscript_tests.cpp index 4d31968c787f..9f7d4b7e46fb 100644 --- a/src/test/miniscript_tests.cpp +++ b/src/test/miniscript_tests.cpp @@ -188,7 +188,7 @@ struct KeyConverter { return g_testdata->pkmap.at(keyid); } - std::optional ToString(const Key& key) const { + std::optional ToString(const Key& key, bool&) const { return HexStr(ToPKBytes(key)); } From ed945a685473712c1a822379effa42fd49223515 Mon Sep 17 00:00:00 2001 From: Novo Date: Mon, 12 May 2025 11:55:19 +0100 Subject: [PATCH 06/25] walletrpc: reject listdes with priv key on w-only wallets --- src/wallet/rpc/backup.cpp | 8 +++++--- test/functional/wallet_listdescriptors.py | 4 ++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp index 5cb40304a714..ebc678aac070 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -493,6 +494,9 @@ RPCHelpMan listdescriptors() if (!wallet) return UniValue::VNULL; const bool priv = !request.params[0].isNull() && request.params[0].get_bool(); + if (wallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) && priv) { + throw JSONRPCError(RPC_WALLET_ERROR, "Can't get private descriptor string for watch-only wallets"); + } if (priv) { EnsureWalletIsUnlocked(*wallet); } @@ -519,9 +523,7 @@ RPCHelpMan listdescriptors() LOCK(desc_spk_man->cs_desc_man); const auto& wallet_descriptor = desc_spk_man->GetWalletDescriptor(); std::string descriptor; - if (!desc_spk_man->GetDescriptorString(descriptor, priv)) { - throw JSONRPCError(RPC_WALLET_ERROR, "Can't get descriptor string."); - } + CHECK_NONFATAL(desc_spk_man->GetDescriptorString(descriptor, priv)); const bool is_range = wallet_descriptor.descriptor->IsRange(); wallet_descriptors.push_back({ descriptor, diff --git a/test/functional/wallet_listdescriptors.py b/test/functional/wallet_listdescriptors.py index aaa5daac8bda..da73d5ee925c 100755 --- a/test/functional/wallet_listdescriptors.py +++ b/test/functional/wallet_listdescriptors.py @@ -107,7 +107,7 @@ def run_test(self): 'desc': descsum_create('wpkh(' + xpub_acc + ')'), 'timestamp': TIME_GENESIS_BLOCK, }]) - assert_raises_rpc_error(-4, 'Can\'t get descriptor string', watch_only_wallet.listdescriptors, True) + assert_raises_rpc_error(-4, 'Can\'t get private descriptor string for watch-only wallets', watch_only_wallet.listdescriptors, True) self.log.info('Test non-active non-range combo descriptor') node.createwallet(wallet_name='w4', blank=True) @@ -122,7 +122,7 @@ def run_test(self): {'active': False, 'desc': 'combo(0227d85ba011276cf25b51df6a188b75e604b38770a462b2d0e9fb2fc839ef5d3f)#np574htj', 'timestamp': TIME_GENESIS_BLOCK}, - ] + ], } assert_equal(expected, wallet.listdescriptors()) From 9c7e4771b13d4729fd20ea08b7e2e3209b134fff Mon Sep 17 00:00:00 2001 From: Novo Date: Mon, 12 May 2025 13:39:03 +0100 Subject: [PATCH 07/25] test: Test listdescs with priv works even with missing priv keys Co-authored-by: rkrux --- test/functional/wallet_listdescriptors.py | 30 +++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/test/functional/wallet_listdescriptors.py b/test/functional/wallet_listdescriptors.py index da73d5ee925c..12b0576c538d 100755 --- a/test/functional/wallet_listdescriptors.py +++ b/test/functional/wallet_listdescriptors.py @@ -146,6 +146,36 @@ def run_test(self): } assert_equal(expected, wallet.listdescriptors()) + self.log.info('Test descriptor with missing private keys') + node.createwallet(wallet_name='w6', blank=True) + wallet = node.get_wallet_rpc('w6') + + expected_descs = { + descsum_create('tr(' + node.get_deterministic_priv_key().key + + ',{pk(03cdabb7f2dce7bfbd8a0b9570c6fd1e712e5d64045e9d6b517b3d5072251dc204)' + + ',pk([d34db33f/44h/0h/0h]tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B/0)})'), + descsum_create('wsh(and_v(v:ripemd160(095ff41131e5946f3c85f79e44adbcf8e27e080e),multi(1,' + node.get_deterministic_priv_key().key + + ',tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B/0)))'), + descsum_create('tr(03dff1d77f2a671c5f36183726db2341be58feae1da2deced843240f7b502ba659,pk(musig(tprv8ZgxMBicQKsPeNLUGrbv3b7qhUk1LQJZAGMuk9gVuKh9sd4BWGp1eMsehUni6qGb8bjkdwBxCbgNGdh2bYGACK5C5dRTaif9KBKGVnSezxV,tpubD6NzVbkrYhZ4XcACN3PEwNjRpR1g4tZjBVk5pdMR2B6dbd3HYhdGVZNKofAiFZd9okBserZvv58A6tBX4pE64UpXGNTSesfUW7PpW36HuKz)/7/8/*))'), + descsum_create('tr(03dff1d77f2a671c5f36183726db2341be58feae1da2deced843240f7b502ba659,pk(musig(tprv8ZgxMBicQKsPeNLUGrbv3b7qhUk1LQJZAGMuk9gVuKh9sd4BWGp1eMsehUni6qGb8bjkdwBxCbgNGdh2bYGACK5C5dRTaif9KBKGVnSezxV/10,tpubD6NzVbkrYhZ4XcACN3PEwNjRpR1g4tZjBVk5pdMR2B6dbd3HYhdGVZNKofAiFZd9okBserZvv58A6tBX4pE64UpXGNTSesfUW7PpW36HuKz/11)/*))'), + descsum_create('tr(03dff1d77f2a671c5f36183726db2341be58feae1da2deced843240f7b502ba659,{pk(musig(tpubD6NzVbkrYhZ4Wo2WcFSgSqRD9QWkGxddo6WSqsVBx7uQ8QEtM7WncKDRjhFEexK119NigyCsFygA4b7sAPQxqebyFGAZ9XVV1BtcgNzbCRR,tprv8ZgxMBicQKsPen4PGtDwURYnCtVMDejyE8vVwMGhQWfVqB2FBPdekhTacDW4vmsKTsgC1wsncVqXiZdX2YFGAnKoLXYf42M78fQJFzuDYFN)/12/*),pk(musig(tprv8ZgxMBicQKsPeNLUGrbv3b7qhUk1LQJZAGMuk9gVuKh9sd4BWGp1eMsehUni6qGb8bjkdwBxCbgNGdh2bYGACK5C5dRTaif9KBKGVnSezxV,tpubD6NzVbkrYhZ4XWb6fGPjyhgLxapUhXszv7ehQYrQWDgDX4nYWcNcbgWcM2RhYo9s2mbZcfZJ8t5LzYcr24FK79zVybsw5Qj3Rtqug8jpJMy)/13/*)})'), + descsum_create('tr(03dff1d77f2a671c5f36183726db2341be58feae1da2deced843240f7b502ba659,{pk(musig(tpubD6NzVbkrYhZ4Wo2WcFSgSqRD9QWkGxddo6WSqsVBx7uQ8QEtM7WncKDRjhFEexK119NigyCsFygA4b7sAPQxqebyFGAZ9XVV1BtcgNzbCRR,tpubD6NzVbkrYhZ4Wc3i6L6N1Pp7cyVeyMcdLrFGXGDGzCfdCa5F4Zs3EY46N72Ws8QDEUYBVwXfDfda2UKSseSdU1fsBegJBhGCZyxkf28bkQ6)/12/*),pk(musig(tprv8ZgxMBicQKsPeNLUGrbv3b7qhUk1LQJZAGMuk9gVuKh9sd4BWGp1eMsehUni6qGb8bjkdwBxCbgNGdh2bYGACK5C5dRTaif9KBKGVnSezxV,tpubD6NzVbkrYhZ4XWb6fGPjyhgLxapUhXszv7ehQYrQWDgDX4nYWcNcbgWcM2RhYo9s2mbZcfZJ8t5LzYcr24FK79zVybsw5Qj3Rtqug8jpJMy)/13/*)})') + } + + descs_to_import = [] + for desc in expected_descs: + descs_to_import.append({'desc': desc, 'timestamp': TIME_GENESIS_BLOCK}) + + wallet.importdescriptors(descs_to_import) + result = wallet.listdescriptors(True) + actual_descs = [d['desc'] for d in result['descriptors']] + + assert_equal(len(actual_descs), len(expected_descs)) + for desc in actual_descs: + if desc not in expected_descs: + raise AssertionError(f"{desc} missing") + + if __name__ == '__main__': ListDescriptorsTest(__file__).main() From faf0c2d942c8de7868a3fd3afc7fc9ea700c91d4 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Mon, 13 Jan 2025 21:39:03 +0100 Subject: [PATCH 08/25] refactor: Avoid copies by using const references or by move-construction --- src/external_signer.cpp | 5 +++-- src/external_signer.h | 4 ++-- src/netbase.h | 3 ++- src/psbt.cpp | 2 +- src/psbt.h | 2 +- src/test/argsman_tests.cpp | 4 ++-- src/wallet/rpc/util.cpp | 2 +- src/wallet/rpc/util.h | 2 +- src/wallet/wallet.cpp | 2 +- src/wallet/wallet.h | 2 +- 10 files changed, 15 insertions(+), 13 deletions(-) diff --git a/src/external_signer.cpp b/src/external_signer.cpp index 331291997756..84d98a199062 100644 --- a/src/external_signer.cpp +++ b/src/external_signer.cpp @@ -15,14 +15,15 @@ #include #include -ExternalSigner::ExternalSigner(const std::string& command, const std::string chain, const std::string& fingerprint, const std::string name): m_command(command), m_chain(chain), m_fingerprint(fingerprint), m_name(name) {} +ExternalSigner::ExternalSigner(std::string command, std::string chain, std::string fingerprint, std::string name) + : m_command{std::move(command)}, m_chain{std::move(chain)}, m_fingerprint{std::move(fingerprint)}, m_name{std::move(name)} {} std::string ExternalSigner::NetworkArg() const { return " --chain " + m_chain; } -bool ExternalSigner::Enumerate(const std::string& command, std::vector& signers, const std::string chain) +bool ExternalSigner::Enumerate(const std::string& command, std::vector& signers, const std::string& chain) { // Call enumerate const UniValue result = RunCommandParseJSON(command + " enumerate"); diff --git a/src/external_signer.h b/src/external_signer.h index 318b3bc062c7..5d23cef461f2 100644 --- a/src/external_signer.h +++ b/src/external_signer.h @@ -31,7 +31,7 @@ class ExternalSigner //! @param[in] fingerprint master key fingerprint of the signer //! @param[in] chain "main", "test", "regtest" or "signet" //! @param[in] name device name - ExternalSigner(const std::string& command, const std::string chain, const std::string& fingerprint, const std::string name); + ExternalSigner(std::string command, std::string chain, std::string fingerprint, std::string name); //! Master key fingerprint of the signer std::string m_fingerprint; @@ -44,7 +44,7 @@ class ExternalSigner //! @param[in,out] signers vector to which new signers (with a unique master key fingerprint) are added //! @param chain "main", "test", "regtest" or "signet" //! @returns success - static bool Enumerate(const std::string& command, std::vector& signers, const std::string chain); + static bool Enumerate(const std::string& command, std::vector& signers, const std::string& chain); //! Display address on the device. Calls ` displayaddress --desc `. //! @param[in] descriptor Descriptor specifying which address to display. diff --git a/src/netbase.h b/src/netbase.h index d3c263f9e8a8..174eae43e295 100644 --- a/src/netbase.h +++ b/src/netbase.h @@ -60,7 +60,8 @@ class Proxy public: Proxy() : m_is_unix_socket(false), m_tor_stream_isolation(false) {} explicit Proxy(const CService& _proxy, bool tor_stream_isolation = false) : proxy(_proxy), m_is_unix_socket(false), m_tor_stream_isolation(tor_stream_isolation) {} - explicit Proxy(const std::string path, bool tor_stream_isolation = false) : m_unix_socket_path(path), m_is_unix_socket(true), m_tor_stream_isolation(tor_stream_isolation) {} + explicit Proxy(std::string path, bool tor_stream_isolation = false) + : m_unix_socket_path(std::move(path)), m_is_unix_socket(true), m_tor_stream_isolation(tor_stream_isolation) {} CService proxy; std::string m_unix_socket_path; diff --git a/src/psbt.cpp b/src/psbt.cpp index 9339bf7c0922..a1eafbc001dc 100644 --- a/src/psbt.cpp +++ b/src/psbt.cpp @@ -322,7 +322,7 @@ bool PSBTInputSigned(const PSBTInput& input) return !input.final_script_sig.empty() || !input.final_script_witness.IsNull(); } -bool PSBTInputSignedAndVerified(const PartiallySignedTransaction psbt, unsigned int input_index, const PrecomputedTransactionData* txdata) +bool PSBTInputSignedAndVerified(const PartiallySignedTransaction& psbt, unsigned int input_index, const PrecomputedTransactionData* txdata) { CTxOut utxo; assert(input_index < psbt.inputs.size()); diff --git a/src/psbt.h b/src/psbt.h index 9d7fc05b791a..1a426a2eeb3f 100644 --- a/src/psbt.h +++ b/src/psbt.h @@ -1413,7 +1413,7 @@ PrecomputedTransactionData PrecomputePSBTData(const PartiallySignedTransaction& bool PSBTInputSigned(const PSBTInput& input); /** Checks whether a PSBTInput is already signed by doing script verification using final fields. */ -bool PSBTInputSignedAndVerified(const PartiallySignedTransaction psbt, unsigned int input_index, const PrecomputedTransactionData* txdata); +bool PSBTInputSignedAndVerified(const PartiallySignedTransaction& psbt, unsigned int input_index, const PrecomputedTransactionData* txdata); /** Signs a PSBTInput, verifying that all provided data matches what is being signed. * diff --git a/src/test/argsman_tests.cpp b/src/test/argsman_tests.cpp index 5236d5db21c9..e91ae5e96f3a 100644 --- a/src/test/argsman_tests.cpp +++ b/src/test/argsman_tests.cpp @@ -52,7 +52,7 @@ BOOST_AUTO_TEST_CASE(util_datadir) struct TestArgsManager : public ArgsManager { TestArgsManager() { m_network_only_args.clear(); } - void ReadConfigString(const std::string str_config) + void ReadConfigString(const std::string& str_config) { std::istringstream streamConfig(str_config); { @@ -63,7 +63,7 @@ struct TestArgsManager : public ArgsManager std::string error; BOOST_REQUIRE(ReadConfigStream(streamConfig, "", error)); } - void SetNetworkOnlyArg(const std::string arg) + void SetNetworkOnlyArg(const std::string& arg) { LOCK(cs_args); m_network_only_args.insert(arg); diff --git a/src/wallet/rpc/util.cpp b/src/wallet/rpc/util.cpp index d68e6c652694..a699b226150b 100644 --- a/src/wallet/rpc/util.cpp +++ b/src/wallet/rpc/util.cpp @@ -124,7 +124,7 @@ void PushParentDescriptors(const CWallet& wallet, const CScript& script_pubkey, entry.pushKV("parent_descs", std::move(parent_descs)); } -void HandleWalletError(const std::shared_ptr wallet, DatabaseStatus& status, bilingual_str& error) +void HandleWalletError(const std::shared_ptr& wallet, DatabaseStatus& status, bilingual_str& error) { if (!wallet) { // Map bad format to not found, since bad format is returned when the diff --git a/src/wallet/rpc/util.h b/src/wallet/rpc/util.h index d649721431a2..89729218d1a1 100644 --- a/src/wallet/rpc/util.h +++ b/src/wallet/rpc/util.h @@ -54,7 +54,7 @@ std::string LabelFromValue(const UniValue& value); //! Fetch parent descriptors of this scriptPubKey. void PushParentDescriptors(const CWallet& wallet, const CScript& script_pubkey, UniValue& entry); -void HandleWalletError(const std::shared_ptr wallet, DatabaseStatus& status, bilingual_str& error); +void HandleWalletError(const std::shared_ptr& wallet, DatabaseStatus& status, bilingual_str& error); void AppendLastProcessedBlock(UniValue& entry, const CWallet& wallet) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet); } // namespace wallet diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 56c676c189f1..abbb6c1267d0 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2534,7 +2534,7 @@ bool CWallet::TopUpKeyPool(unsigned int kpSize) return res; } -util::Result CWallet::GetNewDestination(const OutputType type, const std::string label) +util::Result CWallet::GetNewDestination(const OutputType type, const std::string& label) { LOCK(cs_wallet); auto spk_man = GetScriptPubKeyMan(type, /*internal=*/false); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 24d6b07d3562..d23c1a943ec8 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -783,7 +783,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati */ void MarkDestinationsDirty(const std::set& destinations) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - util::Result GetNewDestination(const OutputType type, const std::string label); + util::Result GetNewDestination(const OutputType type, const std::string& label); util::Result GetNewChangeDestination(const OutputType type); bool IsMine(const CTxDestination& dest) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); From fa64d8424b8de49e219bffb842a33d484fb03212 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Thu, 10 Jul 2025 12:49:36 +0200 Subject: [PATCH 09/25] refactor: Enforce readability-avoid-const-params-in-decls --- src/.clang-tidy | 1 + src/addrman.h | 2 +- src/addrman_impl.h | 4 ++-- src/chainparams.h | 4 ++-- src/chainparamsbase.h | 4 ++-- src/common/bloom.h | 4 ++-- src/common/messages.h | 2 +- src/common/signmessage.h | 2 +- src/core_io.h | 4 ++-- src/crypto/hex_base.h | 6 +++--- src/deploymentinfo.h | 2 +- src/external_signer.h | 2 +- src/interfaces/mining.h | 2 +- src/interfaces/wallet.h | 4 ++-- src/kernel/bitcoinkernel.h | 4 ++-- src/net.h | 4 ++-- src/net_processing.cpp | 2 +- src/net_processing.h | 2 +- src/netaddress.h | 2 +- src/node/blockstorage.h | 2 +- src/node/miner.h | 2 +- src/node/transaction.h | 2 +- src/policy/fees/block_policy_estimator.h | 2 +- src/pow.h | 2 +- src/qt/addresstablemodel.h | 2 +- src/qt/networkstyle.h | 4 ++-- src/qt/rpcconsole.h | 8 ++++---- src/random.h | 2 +- src/rpc/blockchain.h | 4 ++-- src/rpc/rawtransaction_util.h | 2 +- src/rpc/util.h | 8 ++++---- src/script/miniscript.h | 2 +- src/script/script_error.h | 2 +- src/script/sigcache.h | 2 +- src/test/fuzz/util.h | 6 +++--- src/test/util/setup_common.h | 8 ++++---- src/test/validation_block_tests.cpp | 2 +- src/txmempool.h | 4 ++-- src/util/moneystr.h | 2 +- src/wallet/coinselection.h | 10 +++++----- src/wallet/crypter.h | 2 +- src/wallet/scriptpubkeyman.h | 6 +++--- src/wallet/spend.h | 2 +- src/wallet/test/init_test_fixture.h | 2 +- src/wallet/test/util.h | 2 +- src/wallet/test/wallet_test_fixture.h | 2 +- src/wallet/wallet.h | 10 +++++----- src/wallet/walletdb.h | 4 ++-- 48 files changed, 83 insertions(+), 82 deletions(-) diff --git a/src/.clang-tidy b/src/.clang-tidy index 011536498e1b..a4378cea308f 100644 --- a/src/.clang-tidy +++ b/src/.clang-tidy @@ -24,6 +24,7 @@ performance-*, -performance-no-int-to-ptr, -performance-noexcept-move-constructor, -performance-unnecessary-value-param, +readability-avoid-const-params-in-decls, readability-const-return-type, readability-container-contains, readability-redundant-declaration, diff --git a/src/addrman.h b/src/addrman.h index 3a3230143c19..8368e30b7b76 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -172,7 +172,7 @@ class AddrMan * * @return A vector of randomly selected addresses from vRandom. */ - std::vector GetAddr(size_t max_addresses, size_t max_pct, std::optional network, const bool filtered = true) const; + std::vector GetAddr(size_t max_addresses, size_t max_pct, std::optional network, bool filtered = true) const; /** * Returns an information-location pair for all addresses in the selected addrman table. diff --git a/src/addrman_impl.h b/src/addrman_impl.h index f825d20b032c..c1716f891d62 100644 --- a/src/addrman_impl.h +++ b/src/addrman_impl.h @@ -135,7 +135,7 @@ class AddrManImpl std::pair Select(bool new_only, const std::unordered_set& networks) const EXCLUSIVE_LOCKS_REQUIRED(!cs); - std::vector GetAddr(size_t max_addresses, size_t max_pct, std::optional network, const bool filtered = true) const + std::vector GetAddr(size_t max_addresses, size_t max_pct, std::optional network, bool filtered = true) const EXCLUSIVE_LOCKS_REQUIRED(!cs); std::vector> GetEntries(bool from_tried) const @@ -267,7 +267,7 @@ class AddrManImpl * */ nid_type GetEntry(bool use_tried, size_t bucket, size_t position) const EXCLUSIVE_LOCKS_REQUIRED(cs); - std::vector GetAddr_(size_t max_addresses, size_t max_pct, std::optional network, const bool filtered = true) const EXCLUSIVE_LOCKS_REQUIRED(cs); + std::vector GetAddr_(size_t max_addresses, size_t max_pct, std::optional network, bool filtered = true) const EXCLUSIVE_LOCKS_REQUIRED(cs); std::vector> GetEntries_(bool from_tried) const EXCLUSIVE_LOCKS_REQUIRED(cs); diff --git a/src/chainparams.h b/src/chainparams.h index 571e9e177b2b..732949e82f69 100644 --- a/src/chainparams.h +++ b/src/chainparams.h @@ -15,7 +15,7 @@ class ArgsManager; /** * Creates and returns a std::unique_ptr of the chosen chain. */ -std::unique_ptr CreateChainParams(const ArgsManager& args, const ChainType chain); +std::unique_ptr CreateChainParams(const ArgsManager& args, ChainType chain); /** * Return the currently selected parameters. This won't change after app @@ -26,6 +26,6 @@ const CChainParams &Params(); /** * Sets the params returned by Params() to those for the given chain type. */ -void SelectParams(const ChainType chain); +void SelectParams(ChainType chain); #endif // BITCOIN_CHAINPARAMS_H diff --git a/src/chainparamsbase.h b/src/chainparamsbase.h index 37e69b3285fc..1407f196f3b8 100644 --- a/src/chainparamsbase.h +++ b/src/chainparamsbase.h @@ -35,7 +35,7 @@ class CBaseChainParams /** * Creates and returns a std::unique_ptr of the chosen chain. */ -std::unique_ptr CreateBaseChainParams(const ChainType chain); +std::unique_ptr CreateBaseChainParams(ChainType chain); /** *Set the arguments for chainparams @@ -49,7 +49,7 @@ void SetupChainParamsBaseOptions(ArgsManager& argsman); const CBaseChainParams& BaseParams(); /** Sets the params returned by Params() to those for the given chain. */ -void SelectBaseParams(const ChainType chain); +void SelectBaseParams(ChainType chain); /** List of possible chain / network names */ #define LIST_CHAIN_NAMES "main, test, testnet4, signet, regtest" diff --git a/src/common/bloom.h b/src/common/bloom.h index bff96a23a37a..97007e1ff517 100644 --- a/src/common/bloom.h +++ b/src/common/bloom.h @@ -61,7 +61,7 @@ class CBloomFilter * It should generally always be a random value (and is largely only exposed for unit testing) * nFlags should be one of the BLOOM_UPDATE_* enums (not _MASK) */ - CBloomFilter(const unsigned int nElements, const double nFPRate, const unsigned int nTweak, unsigned char nFlagsIn); + CBloomFilter(unsigned int nElements, double nFPRate, unsigned int nTweak, unsigned char nFlagsIn); CBloomFilter() : nHashFuncs(0), nTweak(0), nFlags(0) {} SERIALIZE_METHODS(CBloomFilter, obj) { READWRITE(obj.vData, obj.nHashFuncs, obj.nTweak, obj.nFlags); } @@ -108,7 +108,7 @@ class CBloomFilter class CRollingBloomFilter { public: - CRollingBloomFilter(const unsigned int nElements, const double nFPRate); + CRollingBloomFilter(unsigned int nElements, double nFPRate); void insert(std::span vKey); bool contains(std::span vKey) const; diff --git a/src/common/messages.h b/src/common/messages.h index 4cabdc79f4df..60fdaa18625e 100644 --- a/src/common/messages.h +++ b/src/common/messages.h @@ -31,7 +31,7 @@ std::string FeeModeInfo(std::pair& mode); std::string FeeModesDetail(std::string default_info); std::string InvalidEstimateModeErrorMessage(); bilingual_str PSBTErrorString(PSBTError error); -bilingual_str TransactionErrorString(const node::TransactionError error); +bilingual_str TransactionErrorString(node::TransactionError error); bilingual_str ResolveErrMsg(const std::string& optname, const std::string& strBind); bilingual_str InvalidPortErrMsg(const std::string& optname, const std::string& strPort); bilingual_str AmountHighWarn(const std::string& optname); diff --git a/src/common/signmessage.h b/src/common/signmessage.h index 4533875a757b..c2152f353369 100644 --- a/src/common/signmessage.h +++ b/src/common/signmessage.h @@ -72,6 +72,6 @@ bool MessageSign( */ uint256 MessageHash(const std::string& message); -std::string SigningResultString(const SigningResult res); +std::string SigningResultString(SigningResult res); #endif // BITCOIN_COMMON_SIGNMESSAGE_H diff --git a/src/core_io.h b/src/core_io.h index f768590ca2d9..113a961f5009 100644 --- a/src/core_io.h +++ b/src/core_io.h @@ -34,7 +34,7 @@ enum class TxVerbosity { // core_read.cpp CScript ParseScript(const std::string& s); -std::string ScriptToAsmStr(const CScript& script, const bool fAttemptSighashDecode = false); +std::string ScriptToAsmStr(const CScript& script, bool fAttemptSighashDecode = false); [[nodiscard]] bool DecodeHexTx(CMutableTransaction& tx, const std::string& hex_tx, bool try_no_witness = false, bool try_witness = true); [[nodiscard]] bool DecodeHexBlk(CBlock&, const std::string& strHexBlk); bool DecodeHexBlockHeader(CBlockHeader&, const std::string& hex_header); @@ -42,7 +42,7 @@ bool DecodeHexBlockHeader(CBlockHeader&, const std::string& hex_header); [[nodiscard]] util::Result SighashFromStr(const std::string& sighash); // core_write.cpp -UniValue ValueFromAmount(const CAmount amount); +UniValue ValueFromAmount(CAmount amount); std::string FormatScript(const CScript& script); std::string EncodeHexTx(const CTransaction& tx); std::string SighashToStr(unsigned char sighash_type); diff --git a/src/crypto/hex_base.h b/src/crypto/hex_base.h index 9975f7f6137e..286720af0216 100644 --- a/src/crypto/hex_base.h +++ b/src/crypto/hex_base.h @@ -15,9 +15,9 @@ /** * Convert a span of bytes to a lower-case hexadecimal string. */ -std::string HexStr(const std::span s); -inline std::string HexStr(const std::span s) { return HexStr(MakeUCharSpan(s)); } -inline std::string HexStr(const std::span s) { return HexStr(MakeUCharSpan(s)); } +std::string HexStr(std::span s); +inline std::string HexStr(std::span s) { return HexStr(MakeUCharSpan(s)); } +inline std::string HexStr(std::span s) { return HexStr(MakeUCharSpan(s)); } signed char HexDigit(char c); diff --git a/src/deploymentinfo.h b/src/deploymentinfo.h index 875505c1465e..dc3ef723f17d 100644 --- a/src/deploymentinfo.h +++ b/src/deploymentinfo.h @@ -30,6 +30,6 @@ inline std::string DeploymentName(Consensus::DeploymentPos pos) return VersionBitsDeploymentInfo[pos].name; } -std::optional GetBuriedDeployment(const std::string_view deployment_name); +std::optional GetBuriedDeployment(std::string_view deployment_name); #endif // BITCOIN_DEPLOYMENTINFO_H diff --git a/src/external_signer.h b/src/external_signer.h index 5d23cef461f2..1b36d49622e1 100644 --- a/src/external_signer.h +++ b/src/external_signer.h @@ -55,7 +55,7 @@ class ExternalSigner //! Calls ` getdescriptors --account ` //! @param[in] account which BIP32 account to use (e.g. `m/44'/0'/account'`) //! @returns see doc/external-signer.md - UniValue GetDescriptors(const int account); + UniValue GetDescriptors(int account); //! Sign PartiallySignedTransaction on the device. //! Calls ` signtransaction` and passes the PSBT via stdin. diff --git a/src/interfaces/mining.h b/src/interfaces/mining.h index 07ec5f9c3e0c..993f70bd7aaf 100644 --- a/src/interfaces/mining.h +++ b/src/interfaces/mining.h @@ -105,7 +105,7 @@ class BlockTemplate * On testnet this will additionally return a template with difficulty 1 if * the tip is more than 20 minutes old. */ - virtual std::unique_ptr waitNext(const node::BlockWaitOptions options = {}) = 0; + virtual std::unique_ptr waitNext(node::BlockWaitOptions options = {}) = 0; /** * Interrupts the current wait for the next block template. diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index 3ca0738c879b..40c612a4c511 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -95,7 +95,7 @@ class Wallet virtual std::string getWalletName() = 0; // Get a new address. - virtual util::Result getNewDestination(const OutputType type, const std::string& label) = 0; + virtual util::Result getNewDestination(OutputType type, const std::string& label) = 0; //! Get public key. virtual bool getPubKey(const CScript& script, const CKeyID& address, CPubKey& pub_key) = 0; @@ -130,7 +130,7 @@ class Wallet virtual util::Result displayAddress(const CTxDestination& dest) = 0; //! Lock coin. - virtual bool lockCoin(const COutPoint& output, const bool write_to_db) = 0; + virtual bool lockCoin(const COutPoint& output, bool write_to_db) = 0; //! Unlock coin. virtual bool unlockCoin(const COutPoint& output) = 0; diff --git a/src/kernel/bitcoinkernel.h b/src/kernel/bitcoinkernel.h index 0ee41adcc52c..53a760cd2ffe 100644 --- a/src/kernel/bitcoinkernel.h +++ b/src/kernel/bitcoinkernel.h @@ -760,7 +760,7 @@ BITCOINKERNEL_API void btck_logging_disable(); * * @param[in] options Sets formatting options of the log messages. */ -BITCOINKERNEL_API void btck_logging_set_options(const btck_LoggingOptions options); +BITCOINKERNEL_API void btck_logging_set_options(btck_LoggingOptions options); /** * @brief Set the log level of the global internal logger. This does not @@ -835,7 +835,7 @@ BITCOINKERNEL_API void btck_logging_connection_destroy(btck_LoggingConnection* l * @return An allocated chain parameters opaque struct. */ BITCOINKERNEL_API btck_ChainParameters* BITCOINKERNEL_WARN_UNUSED_RESULT btck_chain_parameters_create( - const btck_ChainType chain_type); + btck_ChainType chain_type); /** * Copy the chain parameters. diff --git a/src/net.h b/src/net.h index e7047c5bff20..2fa7c2deaa93 100644 --- a/src/net.h +++ b/src/net.h @@ -421,7 +421,7 @@ class V1Transport final : public Transport size_t m_bytes_sent GUARDED_BY(m_send_mutex) {0}; public: - explicit V1Transport(const NodeId node_id) noexcept; + explicit V1Transport(NodeId node_id) noexcept; bool ReceivedMessageComplete() const override EXCLUSIVE_LOCKS_REQUIRED(!m_recv_mutex) { @@ -1294,7 +1294,7 @@ class CConnman * @param[in] network Select only addresses of this network (nullopt = all). * @param[in] filtered Select only addresses that are considered high quality (false = all). */ - std::vector GetAddressesUnsafe(size_t max_addresses, size_t max_pct, std::optional network, const bool filtered = true) const; + std::vector GetAddressesUnsafe(size_t max_addresses, size_t max_pct, std::optional network, bool filtered = true) const; /** * Return addresses from the per-requestor cache. If no cache entry exists, it is populated with * randomly selected addresses. This function can be used in untrusted contexts. diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 84c0ba9c41bd..8939156b9a85 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -552,7 +552,7 @@ class PeerManagerImpl final : public PeerManager }; void UnitTestMisbehaving(NodeId peer_id) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex) { Misbehaving(*Assert(GetPeerRef(peer_id)), ""); }; void ProcessMessage(CNode& pfrom, const std::string& msg_type, DataStream& vRecv, - const std::chrono::microseconds time_received, const std::atomic& interruptMsgProc) override + std::chrono::microseconds time_received, const std::atomic& interruptMsgProc) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_most_recent_block_mutex, !m_headers_presync_mutex, g_msgproc_mutex, !m_tx_download_mutex); void UpdateLastBlockAnnounceTime(NodeId node, int64_t time_in_seconds) override; ServiceFlags GetDesirableServiceFlags(ServiceFlags services) const override; diff --git a/src/net_processing.h b/src/net_processing.h index 09f348c86bd4..4b221c5d9795 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -149,7 +149,7 @@ class PeerManager : public CValidationInterface, public NetEventsInterface /** Process a single message from a peer. Public for fuzz testing */ virtual void ProcessMessage(CNode& pfrom, const std::string& msg_type, DataStream& vRecv, - const std::chrono::microseconds time_received, const std::atomic& interruptMsgProc) EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex) = 0; + std::chrono::microseconds time_received, const std::atomic& interruptMsgProc) EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex) = 0; /** This function is used for testing the stale tip eviction logic, see denialofservice_tests.cpp */ virtual void UpdateLastBlockAnnounceTime(NodeId node, int64_t time_in_seconds) = 0; diff --git a/src/netaddress.h b/src/netaddress.h index c65568098e6f..2191da54b76e 100644 --- a/src/netaddress.h +++ b/src/netaddress.h @@ -206,7 +206,7 @@ class CNetAddr std::vector GetAddrBytes() const; int GetReachabilityFrom(const CNetAddr& paddrPartner) const; - explicit CNetAddr(const struct in6_addr& pipv6Addr, const uint32_t scope = 0); + explicit CNetAddr(const struct in6_addr& pipv6Addr, uint32_t scope = 0); bool GetIn6Addr(struct in6_addr* pipv6Addr) const; friend bool operator==(const CNetAddr& a, const CNetAddr& b); diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h index 3fb6cc3e5bfc..d8b716e98b14 100644 --- a/src/node/blockstorage.h +++ b/src/node/blockstorage.h @@ -370,7 +370,7 @@ class BlockManager CBlockIndex* InsertBlockIndex(const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main); //! Mark one block file as pruned (modify associated database entries) - void PruneOneBlockFile(const int fileNumber) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + void PruneOneBlockFile(int fileNumber) EXCLUSIVE_LOCKS_REQUIRED(cs_main); CBlockIndex* LookupBlockIndex(const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main); const CBlockIndex* LookupBlockIndex(const uint256& hash) const EXCLUSIVE_LOCKS_REQUIRED(cs_main); diff --git a/src/node/miner.h b/src/node/miner.h index f4bebb51d335..0c268f1826d5 100644 --- a/src/node/miner.h +++ b/src/node/miner.h @@ -128,7 +128,7 @@ class BlockAssembler * accounts for the BIP94 timewarp rule, so does not necessarily reflect the * consensus limit. */ -int64_t GetMinimumTime(const CBlockIndex* pindexPrev, const int64_t difficulty_adjustment_interval); +int64_t GetMinimumTime(const CBlockIndex* pindexPrev, int64_t difficulty_adjustment_interval); int64_t UpdateTime(CBlockHeader* pblock, const Consensus::Params& consensusParams, const CBlockIndex* pindexPrev); diff --git a/src/node/transaction.h b/src/node/transaction.h index e2f237ec383d..d27057a4a936 100644 --- a/src/node/transaction.h +++ b/src/node/transaction.h @@ -70,7 +70,7 @@ static const CAmount DEFAULT_MAX_BURN_AMOUNT{0}; * @param[out] hashBlock The block hash, if the tx was found via -txindex or block_index * @returns The tx if found, otherwise nullptr */ -CTransactionRef GetTransaction(const CBlockIndex* const block_index, const CTxMemPool* const mempool, const Txid& hash, const BlockManager& blockman, uint256& hashBlock); +CTransactionRef GetTransaction(const CBlockIndex* block_index, const CTxMemPool* mempool, const Txid& hash, const BlockManager& blockman, uint256& hashBlock); } // namespace node #endif // BITCOIN_NODE_TRANSACTION_H diff --git a/src/policy/fees/block_policy_estimator.h b/src/policy/fees/block_policy_estimator.h index ec432dcbf537..505eed0867d3 100644 --- a/src/policy/fees/block_policy_estimator.h +++ b/src/policy/fees/block_policy_estimator.h @@ -200,7 +200,7 @@ class CBlockPolicyEstimator : public CValidationInterface const fs::path m_estimation_filepath; public: /** Create new BlockPolicyEstimator and initialize stats tracking classes with default values */ - CBlockPolicyEstimator(const fs::path& estimation_filepath, const bool read_stale_estimates); + CBlockPolicyEstimator(const fs::path& estimation_filepath, bool read_stale_estimates); virtual ~CBlockPolicyEstimator(); /** Process all the transactions that have been included in a block */ diff --git a/src/pow.h b/src/pow.h index 7ee1bddd3c3e..26004c46187c 100644 --- a/src/pow.h +++ b/src/pow.h @@ -24,7 +24,7 @@ class arith_uint256; * @return the proof-of-work target or nullopt if the nBits value * is invalid (due to overflow or exceeding pow_limit) */ -std::optional DeriveTarget(unsigned int nBits, const uint256 pow_limit); +std::optional DeriveTarget(unsigned int nBits, uint256 pow_limit); unsigned int GetNextWorkRequired(const CBlockIndex* pindexLast, const CBlockHeader *pblock, const Consensus::Params&); unsigned int CalculateNextWorkRequired(const CBlockIndex* pindexLast, int64_t nFirstBlockTime, const Consensus::Params&); diff --git a/src/qt/addresstablemodel.h b/src/qt/addresstablemodel.h index 4bfacc56854f..127378ed2f45 100644 --- a/src/qt/addresstablemodel.h +++ b/src/qt/addresstablemodel.h @@ -70,7 +70,7 @@ class AddressTableModel : public QAbstractTableModel /* Add an address to the model. Returns the added address on success, and an empty string otherwise. */ - QString addRow(const QString &type, const QString &label, const QString &address, const OutputType address_type); + QString addRow(const QString& type, const QString& label, const QString& address, OutputType address_type); /** Look up label for address in address book, if not found return empty string. */ QString labelForAddress(const QString &address) const; diff --git a/src/qt/networkstyle.h b/src/qt/networkstyle.h index ac103ee73270..2c6cc8214780 100644 --- a/src/qt/networkstyle.h +++ b/src/qt/networkstyle.h @@ -16,7 +16,7 @@ class NetworkStyle { public: /** Get style associated with provided network id, or 0 if not known */ - static const NetworkStyle* instantiate(const ChainType networkId); + static const NetworkStyle* instantiate(ChainType networkId); const QString &getAppName() const { return appName; } const QIcon &getAppIcon() const { return appIcon; } @@ -24,7 +24,7 @@ class NetworkStyle const QString &getTitleAddText() const { return titleAddText; } private: - NetworkStyle(const QString &appName, const int iconColorHueShift, const int iconColorSaturationReduction, const char *titleAddText); + NetworkStyle(const QString& appName, int iconColorHueShift, int iconColorSaturationReduction, const char* titleAddText); QString appName; QIcon appIcon; diff --git a/src/qt/rpcconsole.h b/src/qt/rpcconsole.h index 53dccd7dacfb..bbd469cd3dd6 100644 --- a/src/qt/rpcconsole.h +++ b/src/qt/rpcconsole.h @@ -48,7 +48,7 @@ class RPCConsole: public QWidget explicit RPCConsole(interfaces::Node& node, const PlatformStyle *platformStyle, QWidget *parent); ~RPCConsole(); - static bool RPCParseCommandLine(interfaces::Node* node, std::string &strResult, const std::string &strCommand, bool fExecute, std::string * const pstrFilteredOut = nullptr, const QString& wallet_name = {}); + static bool RPCParseCommandLine(interfaces::Node* node, std::string& strResult, const std::string& strCommand, bool fExecute, std::string* pstrFilteredOut = nullptr, const QString& wallet_name = {}); static bool RPCExecuteCommandLine(interfaces::Node& node, std::string &strResult, const std::string &strCommand, std::string * const pstrFilteredOut = nullptr, const QString& wallet_name = {}) { return RPCParseCommandLine(&node, strResult, strCommand, true, pstrFilteredOut, wallet_name); } @@ -56,8 +56,8 @@ class RPCConsole: public QWidget void setClientModel(ClientModel *model = nullptr, int bestblock_height = 0, int64_t bestblock_date = 0, double verification_progress = 0.0); #ifdef ENABLE_WALLET - void addWallet(WalletModel* const walletModel); - void removeWallet(WalletModel* const walletModel); + void addWallet(WalletModel* walletModel); + void removeWallet(WalletModel* walletModel); #endif // ENABLE_WALLET enum MessageClass { @@ -138,7 +138,7 @@ public Q_SLOTS: void setTabFocus(enum TabTypes tabType); #ifdef ENABLE_WALLET /** Set the current (ie - active) wallet */ - void setCurrentWallet(WalletModel* const wallet_model); + void setCurrentWallet(WalletModel* wallet_model); #endif // ENABLE_WALLET private: diff --git a/src/random.h b/src/random.h index 330c10a36b88..f0c0dcee8162 100644 --- a/src/random.h +++ b/src/random.h @@ -95,7 +95,7 @@ void RandAddPeriodic() noexcept; * * Thread-safe. */ -void RandAddEvent(const uint32_t event_info) noexcept; +void RandAddEvent(uint32_t event_info) noexcept; /* =========================== BASE RANDOMNESS GENERATION FUNCTIONS =========================== diff --git a/src/rpc/blockchain.h b/src/rpc/blockchain.h index 0e42bed94799..d14a43b22d06 100644 --- a/src/rpc/blockchain.h +++ b/src/rpc/blockchain.h @@ -36,10 +36,10 @@ static constexpr int NUM_GETBLOCKSTATS_PERCENTILES = 5; double GetDifficulty(const CBlockIndex& blockindex); /** Block description to JSON */ -UniValue blockToJSON(node::BlockManager& blockman, const CBlock& block, const CBlockIndex& tip, const CBlockIndex& blockindex, TxVerbosity verbosity, const uint256 pow_limit) LOCKS_EXCLUDED(cs_main); +UniValue blockToJSON(node::BlockManager& blockman, const CBlock& block, const CBlockIndex& tip, const CBlockIndex& blockindex, TxVerbosity verbosity, uint256 pow_limit) LOCKS_EXCLUDED(cs_main); /** Block header to JSON */ -UniValue blockheaderToJSON(const CBlockIndex& tip, const CBlockIndex& blockindex, const uint256 pow_limit) LOCKS_EXCLUDED(cs_main); +UniValue blockheaderToJSON(const CBlockIndex& tip, const CBlockIndex& blockindex, uint256 pow_limit) LOCKS_EXCLUDED(cs_main); /** Used by getblockstats to get feerates at different percentiles by weight */ void CalculatePercentilesByWeight(CAmount result[NUM_GETBLOCKSTATS_PERCENTILES], std::vector>& scores, int64_t total_weight); diff --git a/src/rpc/rawtransaction_util.h b/src/rpc/rawtransaction_util.h index 65e65c378928..7bd051220a70 100644 --- a/src/rpc/rawtransaction_util.h +++ b/src/rpc/rawtransaction_util.h @@ -54,7 +54,7 @@ std::vector> ParseOutputs(const UniValue& out void AddOutputs(CMutableTransaction& rawTx, const UniValue& outputs_in); /** Create a transaction from univalue parameters */ -CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniValue& outputs_in, const UniValue& locktime, std::optional rbf, const uint32_t version); +CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniValue& outputs_in, const UniValue& locktime, std::optional rbf, uint32_t version); /** Explain the UniValue "decoded" transaction object, may include extra fields if processed by wallet **/ std::vector DecodeTxDoc(const std::string& txid_field_doc, bool wallet); diff --git a/src/rpc/util.h b/src/rpc/util.h index f82ccbf832e3..37530876d734 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -136,7 +136,7 @@ std::string HelpExampleRpc(const std::string& methodname, const std::string& arg std::string HelpExampleRpcNamed(const std::string& methodname, const RPCArgList& args); CPubKey HexToPubKey(const std::string& hex_in); -CTxDestination AddAndGetMultisigDestination(const int required, const std::vector& pubkeys, OutputType type, FlatSigningProvider& keystore, CScript& script_out); +CTxDestination AddAndGetMultisigDestination(int required, const std::vector& pubkeys, OutputType type, FlatSigningProvider& keystore, CScript& script_out); UniValue DescribeAddress(const CTxDestination& dest); @@ -154,7 +154,7 @@ UniValue JSONRPCTransactionError(node::TransactionError terr, const std::string& std::pair ParseDescriptorRange(const UniValue& value); /** Evaluate a descriptor given as a string, or as a {"desc":...,"range":...} object, with default range of 1000. */ -std::vector EvalDescriptorStringOrObject(const UniValue& scanobject, FlatSigningProvider& provider, const bool expand_priv = false); +std::vector EvalDescriptorStringOrObject(const UniValue& scanobject, FlatSigningProvider& provider, bool expand_priv = false); /** * Serializing JSON objects depends on the outer type. Only arrays and @@ -372,7 +372,7 @@ struct RPCResult { : RPCResult{type, std::move(m_key_name), /*optional=*/false, std::move(description), std::move(inner), skip_type_check} {} /** Append the sections of the result. */ - void ToSections(Sections& sections, OuterType outer_type = OuterType::NONE, const int current_indent = 0) const; + void ToSections(Sections& sections, OuterType outer_type = OuterType::NONE, int current_indent = 0) const; /** Return the type string of the result when it is in an object (dict). */ std::string ToStringObj() const; /** Return the description string, including the result type. */ @@ -527,6 +527,6 @@ std::vector ScriptPubKeyDoc(); * * @return the target */ -uint256 GetTarget(const CBlockIndex& blockindex, const uint256 pow_limit); +uint256 GetTarget(const CBlockIndex& blockindex, uint256 pow_limit); #endif // BITCOIN_RPC_UTIL_H diff --git a/src/script/miniscript.h b/src/script/miniscript.h index c71203ed2607..955ede236fef 100644 --- a/src/script/miniscript.h +++ b/src/script/miniscript.h @@ -1761,7 +1761,7 @@ enum class ParseContext { CLOSE_BRACKET, }; -int FindNextChar(std::span in, const char m); +int FindNextChar(std::span in, char m); /** Parse a key string ending at the end of the fragment's text representation. */ template diff --git a/src/script/script_error.h b/src/script/script_error.h index 198d037c2a9d..58ac7db53933 100644 --- a/src/script/script_error.h +++ b/src/script/script_error.h @@ -88,6 +88,6 @@ typedef enum ScriptError_t #define SCRIPT_ERR_LAST SCRIPT_ERR_ERROR_COUNT -std::string ScriptErrorString(const ScriptError error); +std::string ScriptErrorString(ScriptError error); #endif // BITCOIN_SCRIPT_SCRIPT_ERROR_H diff --git a/src/script/sigcache.h b/src/script/sigcache.h index ca4b44910e65..fb3880966d49 100644 --- a/src/script/sigcache.h +++ b/src/script/sigcache.h @@ -55,7 +55,7 @@ class SignatureCache void ComputeEntrySchnorr(uint256& entry, const uint256 &hash, std::span sig, const XOnlyPubKey& pubkey) const; - bool Get(const uint256& entry, const bool erase); + bool Get(const uint256& entry, bool erase); void Set(const uint256& entry); }; diff --git a/src/test/fuzz/util.h b/src/test/fuzz/util.h index 0575b157ea87..a7b1bfd54ed0 100644 --- a/src/test/fuzz/util.h +++ b/src/test/fuzz/util.h @@ -146,11 +146,11 @@ template [[nodiscard]] int64_t ConsumeTime(FuzzedDataProvider& fuzzed_data_provider, const std::optional& min = std::nullopt, const std::optional& max = std::nullopt) noexcept; -[[nodiscard]] CMutableTransaction ConsumeTransaction(FuzzedDataProvider& fuzzed_data_provider, const std::optional>& prevout_txids, const int max_num_in = 10, const int max_num_out = 10) noexcept; +[[nodiscard]] CMutableTransaction ConsumeTransaction(FuzzedDataProvider& fuzzed_data_provider, const std::optional>& prevout_txids, int max_num_in = 10, int max_num_out = 10) noexcept; -[[nodiscard]] CScriptWitness ConsumeScriptWitness(FuzzedDataProvider& fuzzed_data_provider, const size_t max_stack_elem_size = 32) noexcept; +[[nodiscard]] CScriptWitness ConsumeScriptWitness(FuzzedDataProvider& fuzzed_data_provider, size_t max_stack_elem_size = 32) noexcept; -[[nodiscard]] CScript ConsumeScript(FuzzedDataProvider& fuzzed_data_provider, const bool maybe_p2wsh = false) noexcept; +[[nodiscard]] CScript ConsumeScript(FuzzedDataProvider& fuzzed_data_provider, bool maybe_p2wsh = false) noexcept; [[nodiscard]] uint32_t ConsumeSequence(FuzzedDataProvider& fuzzed_data_provider) noexcept; diff --git a/src/test/util/setup_common.h b/src/test/util/setup_common.h index fdb0951eee3c..05851e41e32d 100644 --- a/src/test/util/setup_common.h +++ b/src/test/util/setup_common.h @@ -73,7 +73,7 @@ struct BasicTestingSetup { m_rng.Reseed(GetRandHash()); } - explicit BasicTestingSetup(const ChainType chainType = ChainType::MAIN, TestOpts = {}); + explicit BasicTestingSetup(ChainType chainType = ChainType::MAIN, TestOpts = {}); ~BasicTestingSetup(); fs::path m_path_root; @@ -109,7 +109,7 @@ struct ChainTestingSetup : public BasicTestingSetup { bool m_block_tree_db_in_memory{true}; std::function m_make_chainman{}; - explicit ChainTestingSetup(const ChainType chainType = ChainType::MAIN, TestOpts = {}); + explicit ChainTestingSetup(ChainType chainType = ChainType::MAIN, TestOpts = {}); ~ChainTestingSetup(); // Supplies a chainstate, if one is needed @@ -120,7 +120,7 @@ struct ChainTestingSetup : public BasicTestingSetup { */ struct TestingSetup : public ChainTestingSetup { explicit TestingSetup( - const ChainType chainType = ChainType::MAIN, + ChainType chainType = ChainType::MAIN, TestOpts = {}); }; @@ -145,7 +145,7 @@ class CScript; */ struct TestChain100Setup : public TestingSetup { TestChain100Setup( - const ChainType chain_type = ChainType::REGTEST, + ChainType chain_type = ChainType::REGTEST, TestOpts = {}); /** diff --git a/src/test/validation_block_tests.cpp b/src/test/validation_block_tests.cpp index 9f574be86eff..ec2a4c9c1a8f 100644 --- a/src/test/validation_block_tests.cpp +++ b/src/test/validation_block_tests.cpp @@ -28,7 +28,7 @@ struct MinerTestingSetup : public RegTestingSetup { std::shared_ptr GoodBlock(const uint256& prev_hash); std::shared_ptr BadBlock(const uint256& prev_hash); std::shared_ptr FinalizeBlock(std::shared_ptr pblock); - void BuildChain(const uint256& root, int height, const unsigned int invalid_rate, const unsigned int branch_rate, const unsigned int max_size, std::vector>& blocks); + void BuildChain(const uint256& root, int height, unsigned int invalid_rate, unsigned int branch_rate, unsigned int max_size, std::vector>& blocks); }; } // namespace validation_block_tests diff --git a/src/txmempool.h b/src/txmempool.h index 166a024823ba..bef711d4c713 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -553,7 +553,7 @@ class CTxMemPool bool CheckPolicyLimits(const CTransactionRef& tx); /** Removes a transaction from the unbroadcast set */ - void RemoveUnbroadcastTx(const Txid& txid, const bool unchecked = false); + void RemoveUnbroadcastTx(const Txid& txid, bool unchecked = false); /** Returns transactions in unbroadcast set */ std::set GetUnbroadcastTxs() const @@ -662,7 +662,7 @@ class CTxMemPool using TxHandle = CTxMemPool::txiter; - TxHandle StageAddition(const CTransactionRef& tx, const CAmount fee, int64_t time, unsigned int entry_height, uint64_t entry_sequence, bool spends_coinbase, int64_t sigops_cost, LockPoints lp); + TxHandle StageAddition(const CTransactionRef& tx, CAmount fee, int64_t time, unsigned int entry_height, uint64_t entry_sequence, bool spends_coinbase, int64_t sigops_cost, LockPoints lp); void StageRemoval(CTxMemPool::txiter it); diff --git a/src/util/moneystr.h b/src/util/moneystr.h index ecadf6469133..ae30bd5f2807 100644 --- a/src/util/moneystr.h +++ b/src/util/moneystr.h @@ -17,7 +17,7 @@ /* Do not use these functions to represent or parse monetary amounts to or from * JSON but use AmountFromValue and ValueFromAmount for that. */ -std::string FormatMoney(const CAmount n); +std::string FormatMoney(CAmount n); /** Parse an amount denoted in full coins. E.g. "0.0034" supplied on the command line. **/ std::optional ParseMoney(const std::string& str); diff --git a/src/wallet/coinselection.h b/src/wallet/coinselection.h index 79fee40dc1d9..0f67d4e36ad8 100644 --- a/src/wallet/coinselection.h +++ b/src/wallet/coinselection.h @@ -306,7 +306,7 @@ typedef std::map FilteredOutputGroups * @param[in] payment_value Average payment value of the transaction output(s). * @param[in] change_fee Fee for creating a change output. */ -[[nodiscard]] CAmount GenerateChangeTarget(const CAmount payment_value, const CAmount change_fee, FastRandomContext& rng); +[[nodiscard]] CAmount GenerateChangeTarget(CAmount payment_value, CAmount change_fee, FastRandomContext& rng); enum class SelectionAlgorithm : uint8_t { @@ -317,7 +317,7 @@ enum class SelectionAlgorithm : uint8_t MANUAL = 4, }; -std::string GetAlgorithmName(const SelectionAlgorithm algo); +std::string GetAlgorithmName(SelectionAlgorithm algo); struct SelectionResult { @@ -371,7 +371,7 @@ struct SelectionResult void AddInputs(const std::set>& inputs, bool subtract_fee_outputs); /** How much individual inputs overestimated the bump fees for shared ancestries */ - void SetBumpFeeDiscount(const CAmount discount); + void SetBumpFeeDiscount(CAmount discount); /** Calculates and stores the waste for this result given the cost of change * and the opportunity cost of spending these inputs now vs in the future. @@ -385,7 +385,7 @@ struct SelectionResult * used if there is change, in which case it must be non-negative. * @param[in] change_fee The fee for creating a change output */ - void RecalculateWaste(const CAmount min_viable_change, const CAmount change_cost, const CAmount change_fee); + void RecalculateWaste(CAmount min_viable_change, CAmount change_cost, CAmount change_fee); [[nodiscard]] CAmount GetWaste() const; /** Tracks that algorithm was able to exhaustively search the entire combination space before hitting limit of tries */ @@ -432,7 +432,7 @@ struct SelectionResult * @returns Amount for change output, 0 when there is no change. * */ - CAmount GetChange(const CAmount min_viable_change, const CAmount change_fee) const; + CAmount GetChange(CAmount min_viable_change, CAmount change_fee) const; CAmount GetTarget() const { return m_target; } diff --git a/src/wallet/crypter.h b/src/wallet/crypter.h index 0737e561e33c..90871245e573 100644 --- a/src/wallet/crypter.h +++ b/src/wallet/crypter.h @@ -79,7 +79,7 @@ friend class wallet_crypto_tests::TestCrypter; // for test access to chKey/chIV int BytesToKeySHA512AES(std::span salt, const SecureString& key_data, int count, unsigned char* key, unsigned char* iv) const; public: - bool SetKeyFromPassphrase(const SecureString& key_data, std::span salt, const unsigned int rounds, const unsigned int derivation_method); + bool SetKeyFromPassphrase(const SecureString& key_data, std::span salt, unsigned int rounds, unsigned int derivation_method); bool Encrypt(const CKeyingMaterial& vchPlaintext, std::vector &vchCiphertext) const; bool Decrypt(std::span ciphertext, CKeyingMaterial& plaintext) const; bool SetKey(const CKeyingMaterial& new_key, std::span new_iv); diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index fcb107a47628..00dd6eed4d1a 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -337,13 +337,13 @@ class DescriptorScriptPubKeyMan : public ScriptPubKeyMan mutable RecursiveMutex cs_desc_man; - util::Result GetNewDestination(const OutputType type) override; + util::Result GetNewDestination(OutputType type) override; bool IsMine(const CScript& script) const override; bool CheckDecryptionKey(const CKeyingMaterial& master_key) override; bool Encrypt(const CKeyingMaterial& master_key, WalletBatch* batch) override; - util::Result GetReservedDestination(const OutputType type, bool internal, int64_t& index) override; + util::Result GetReservedDestination(OutputType type, bool internal, int64_t& index) override; void ReturnDestination(int64_t index, bool internal, const CTxDestination& addr) override; // Tops up the descriptor cache and m_map_script_pub_keys. The cache is stored in the wallet file @@ -402,7 +402,7 @@ class DescriptorScriptPubKeyMan : public ScriptPubKeyMan std::unordered_set GetScriptPubKeys(int32_t minimum_index) const; int32_t GetEndRange() const; - [[nodiscard]] bool GetDescriptorString(std::string& out, const bool priv) const; + [[nodiscard]] bool GetDescriptorString(std::string& out, bool priv) const; void UpgradeDescriptorCache(); }; diff --git a/src/wallet/spend.h b/src/wallet/spend.h index a22499f3cd66..b10317f49a02 100644 --- a/src/wallet/spend.h +++ b/src/wallet/spend.h @@ -23,7 +23,7 @@ namespace wallet { /** Get the marginal bytes if spending the specified output from this transaction. * Use CoinControl to determine whether to expect signature grinding when calculating the size of the input spend. */ int CalculateMaximumSignedInputSize(const CTxOut& txout, const CWallet* pwallet, const CCoinControl* coin_control); -int CalculateMaximumSignedInputSize(const CTxOut& txout, const COutPoint outpoint, const SigningProvider* pwallet, bool can_grind_r, const CCoinControl* coin_control); +int CalculateMaximumSignedInputSize(const CTxOut& txout, COutPoint outpoint, const SigningProvider* pwallet, bool can_grind_r, const CCoinControl* coin_control); struct TxSize { int64_t vsize{-1}; int64_t weight{-1}; diff --git a/src/wallet/test/init_test_fixture.h b/src/wallet/test/init_test_fixture.h index b435527a8350..a9e38db0b10f 100644 --- a/src/wallet/test/init_test_fixture.h +++ b/src/wallet/test/init_test_fixture.h @@ -14,7 +14,7 @@ namespace wallet { struct InitWalletDirTestingSetup: public BasicTestingSetup { - explicit InitWalletDirTestingSetup(const ChainType chain_type = ChainType::MAIN); + explicit InitWalletDirTestingSetup(ChainType chain_type = ChainType::MAIN); ~InitWalletDirTestingSetup(); void SetWalletDir(const fs::path& walletdir_path); diff --git a/src/wallet/test/util.h b/src/wallet/test/util.h index ecedbd90807f..33f36eb62d9a 100644 --- a/src/wallet/test/util.h +++ b/src/wallet/test/util.h @@ -117,7 +117,7 @@ class MockableDatabase : public WalletDatabase std::unique_ptr CreateMockableWalletDatabase(MockableData records = {}); MockableDatabase& GetMockableDatabase(CWallet& wallet); -DescriptorScriptPubKeyMan* CreateDescriptor(CWallet& keystore, const std::string& desc_str, const bool success); +DescriptorScriptPubKeyMan* CreateDescriptor(CWallet& keystore, const std::string& desc_str, bool success); } // namespace wallet #endif // BITCOIN_WALLET_TEST_UTIL_H diff --git a/src/wallet/test/wallet_test_fixture.h b/src/wallet/test/wallet_test_fixture.h index b268463a173e..8c5b6661f7a7 100644 --- a/src/wallet/test/wallet_test_fixture.h +++ b/src/wallet/test/wallet_test_fixture.h @@ -20,7 +20,7 @@ namespace wallet { /** Testing setup and teardown for wallet. */ struct WalletTestingSetup : public TestingSetup { - explicit WalletTestingSetup(const ChainType chainType = ChainType::MAIN); + explicit WalletTestingSetup(ChainType chainType = ChainType::MAIN); ~WalletTestingSetup(); std::unique_ptr m_wallet_loader; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index d23c1a943ec8..a36fc1214704 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -444,7 +444,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati * block locator and m_last_block_processed, and registering for * notifications about new blocks and transactions. */ - static bool AttachChain(const std::shared_ptr& wallet, interfaces::Chain& chain, const bool rescan_required, bilingual_str& error, std::vector& warnings); + static bool AttachChain(const std::shared_ptr& wallet, interfaces::Chain& chain, bool rescan_required, bilingual_str& error, std::vector& warnings); static NodeClock::time_point GetDefaultNextResend(); @@ -648,7 +648,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati //! USER_ABORT. uint256 last_failed_block; }; - ScanResult ScanForWalletTransactions(const uint256& start_block, int start_height, std::optional max_height, const WalletRescanReserver& reserver, bool fUpdate, const bool save_progress); + ScanResult ScanForWalletTransactions(const uint256& start_block, int start_height, std::optional max_height, const WalletRescanReserver& reserver, bool fUpdate, bool save_progress); void transactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason) override; /** Set the next time this wallet should resend transactions to 12-36 hours from now, ~1 day on average. */ void SetNextResend() { m_next_resend = GetDefaultNextResend(); } @@ -768,7 +768,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati /** * Retrieve all the known labels in the address book */ - std::set ListAddrBookLabels(const std::optional purpose) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + std::set ListAddrBookLabels(std::optional purpose) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); /** * Walk-through the address book entries. @@ -783,8 +783,8 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati */ void MarkDestinationsDirty(const std::set& destinations) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - util::Result GetNewDestination(const OutputType type, const std::string& label); - util::Result GetNewChangeDestination(const OutputType type); + util::Result GetNewDestination(OutputType type, const std::string& label); + util::Result GetNewChangeDestination(OutputType type); bool IsMine(const CTxDestination& dest) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); bool IsMine(const CScript& script) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index a867a28b9706..c4466bfaef3f 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -227,7 +227,7 @@ class WalletBatch bool WriteTx(const CWalletTx& wtx); bool EraseTx(Txid hash); - bool WriteKeyMetadata(const CKeyMetadata& meta, const CPubKey& pubkey, const bool overwrite); + bool WriteKeyMetadata(const CKeyMetadata& meta, const CPubKey& pubkey, bool overwrite); bool WriteKey(const CPubKey& vchPubKey, const CPrivKey& vchPrivKey, const CKeyMetadata &keyMeta); bool WriteCryptedKey(const CPubKey& vchPubKey, const std::vector& vchCryptedSecret, const CKeyMetadata &keyMeta); bool WriteMasterKey(unsigned int nID, const CMasterKey& kMasterKey); @@ -268,7 +268,7 @@ class WalletBatch //! Delete records of the given types bool EraseRecords(const std::unordered_set& types); - bool WriteWalletFlags(const uint64_t flags); + bool WriteWalletFlags(uint64_t flags); //! Begin a new transaction bool TxnBegin(); //! Commit current transaction From fac7fed397f0fa3924600c1806a46d235a62ee5f Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Thu, 15 Jan 2026 15:06:10 +0100 Subject: [PATCH 10/25] refactor: Use std::reference_wrapper in Connman The addrman field is already a reference. However, some tests would benefit from the reference being re-seatable, so that they do not have to create a full Connman each time. --- src/net.cpp | 40 ++++++++++++++++++++-------------------- src/net.h | 2 +- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index d92cb72ccc37..3fb2d8f55e52 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -506,7 +506,7 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, if (!proxyConnectionFailed) { // If a connection to the node was attempted, and failure (if any) is not caused by a problem connecting to // the proxy, mark this as an attempt. - addrman.Attempt(target_addr, fCountFailure); + addrman.get().Attempt(target_addr, fCountFailure); } } else if (pszDest && GetNameProxy(proxy)) { std::string host; @@ -2301,7 +2301,7 @@ void CConnman::ThreadDNSAddressSeed() if (gArgs.GetBoolArg("-forcednsseed", DEFAULT_FORCEDNSSEED)) { // When -forcednsseed is provided, query all. seeds_right_now = seeds.size(); - } else if (addrman.Size() == 0) { + } else if (addrman.get().Size() == 0) { // If we have no known peers, query all. // This will occur on the first run, or if peers.dat has been // deleted. @@ -2323,13 +2323,13 @@ void CConnman::ThreadDNSAddressSeed() // DNS seeds, and if that fails too, also try the fixed seeds. // (done in ThreadOpenConnections) int found = 0; - const std::chrono::seconds seeds_wait_time = (addrman.Size() >= DNSSEEDS_DELAY_PEER_THRESHOLD ? DNSSEEDS_DELAY_MANY_PEERS : DNSSEEDS_DELAY_FEW_PEERS); + const std::chrono::seconds seeds_wait_time = (addrman.get().Size() >= DNSSEEDS_DELAY_PEER_THRESHOLD ? DNSSEEDS_DELAY_MANY_PEERS : DNSSEEDS_DELAY_FEW_PEERS); for (const std::string& seed : seeds) { if (seeds_right_now == 0) { seeds_right_now += DNSSEEDS_TO_QUERY_AT_ONCE; - if (addrman.Size() > 0) { + if (addrman.get().Size() > 0) { LogInfo("Waiting %d seconds before querying DNS seeds.\n", seeds_wait_time.count()); std::chrono::seconds to_wait = seeds_wait_time; while (to_wait.count() > 0) { @@ -2389,7 +2389,7 @@ void CConnman::ThreadDNSAddressSeed() vAdd.push_back(addr); found++; } - addrman.Add(vAdd, resolveSource); + addrman.get().Add(vAdd, resolveSource); } else { // If the seed does not support a subdomain with our desired service bits, // we make an ADDR_FETCH connection to the DNS resolved peer address for the @@ -2412,7 +2412,7 @@ void CConnman::DumpAddresses() DumpPeerAddresses(::gArgs, addrman); LogDebug(BCLog::NET, "Flushed %d addresses to peers.dat %dms\n", - addrman.Size(), Ticks(SteadyClock::now() - start)); + addrman.get().Size(), Ticks(SteadyClock::now() - start)); } void CConnman::ProcessAddrFetch() @@ -2506,7 +2506,7 @@ std::unordered_set CConnman::GetReachableEmptyNetworks() const for (int n = 0; n < NET_MAX; n++) { enum Network net = (enum Network)n; if (net == NET_UNROUTABLE || net == NET_INTERNAL) continue; - if (g_reachable_nets.Contains(net) && addrman.Size(net, std::nullopt) == 0) { + if (g_reachable_nets.Contains(net) && addrman.get().Size(net, std::nullopt) == 0) { networks.insert(net); } } @@ -2526,7 +2526,7 @@ bool CConnman::MaybePickPreferredNetwork(std::optional& network) LOCK(m_nodes_mutex); for (const auto net : nets) { - if (g_reachable_nets.Contains(net) && m_network_conn_counts[net] == 0 && addrman.Size(net) != 0) { + if (g_reachable_nets.Contains(net) && m_network_conn_counts[net] == 0 && addrman.get().Size(net) != 0) { network = net; return true; } @@ -2578,7 +2578,7 @@ void CConnman::ThreadOpenConnections(const std::vector connect, std const bool use_seednodes{!gArgs.GetArgs("-seednode").empty()}; auto seed_node_timer = NodeClock::now(); - bool add_addr_fetch{addrman.Size() == 0 && !seed_nodes.empty()}; + bool add_addr_fetch{addrman.get().Size() == 0 && !seed_nodes.empty()}; constexpr std::chrono::seconds ADD_NEXT_SEEDNODE = 10s; if (!add_fixed_seeds) { @@ -2591,7 +2591,7 @@ void CConnman::ThreadOpenConnections(const std::vector connect, std const auto& seed{SpanPopBack(seed_nodes)}; AddAddrFetch(seed); - if (addrman.Size() == 0) { + if (addrman.get().Size() == 0) { LogInfo("Empty addrman, adding seednode (%s) to addrfetch\n", seed); } else { LogInfo("Couldn't connect to peers from addrman after %d seconds. Adding seednode (%s) to addrfetch\n", ADD_NEXT_SEEDNODE.count(), seed); @@ -2646,7 +2646,7 @@ void CConnman::ThreadOpenConnections(const std::vector connect, std seed_addrs.end()); CNetAddr local; local.SetInternal("fixedseeds"); - addrman.Add(seed_addrs, local); + addrman.get().Add(seed_addrs, local); add_fixed_seeds = false; LogInfo("Added %d fixed seeds from reachable networks.\n", seed_addrs.size()); } @@ -2778,7 +2778,7 @@ void CConnman::ThreadOpenConnections(const std::vector connect, std continue; } - addrman.ResolveCollisions(); + addrman.get().ResolveCollisions(); const auto current_time{NodeClock::now()}; int nTries = 0; @@ -2809,21 +2809,21 @@ void CConnman::ThreadOpenConnections(const std::vector connect, std if (fFeeler) { // First, try to get a tried table collision address. This returns // an empty (invalid) address if there are no collisions to try. - std::tie(addr, addr_last_try) = addrman.SelectTriedCollision(); + std::tie(addr, addr_last_try) = addrman.get().SelectTriedCollision(); if (!addr.IsValid()) { // No tried table collisions. Select a new table address // for our feeler. - std::tie(addr, addr_last_try) = addrman.Select(true, reachable_nets); + std::tie(addr, addr_last_try) = addrman.get().Select(true, reachable_nets); } else if (AlreadyConnectedToAddress(addr)) { // If test-before-evict logic would have us connect to a // peer that we're already connected to, just mark that // address as Good(). We won't be able to initiate the // connection anyway, so this avoids inadvertently evicting // a currently-connected peer. - addrman.Good(addr); + addrman.get().Good(addr); // Select a new table address for our feeler instead. - std::tie(addr, addr_last_try) = addrman.Select(true, reachable_nets); + std::tie(addr, addr_last_try) = addrman.get().Select(true, reachable_nets); } } else { // Not a feeler @@ -2831,8 +2831,8 @@ void CConnman::ThreadOpenConnections(const std::vector connect, std // peer from that network. The eviction logic in net_processing // ensures that a peer from another network will be evicted. std::tie(addr, addr_last_try) = preferred_net.has_value() - ? addrman.Select(false, {*preferred_net}) - : addrman.Select(false, reachable_nets); + ? addrman.get().Select(false, {*preferred_net}) + : addrman.get().Select(false, reachable_nets); } // Require outbound IPv4/IPv6 connections, other than feelers, to be to distinct network groups @@ -3244,7 +3244,7 @@ void CConnman::ThreadPrivateBroadcast() continue; } - const auto [addr, _] = addrman.Select(/*new_only=*/false, {net.value()}); + const auto [addr, _] = addrman.get().Select(/*new_only=*/false, {net.value()}); if (!addr.IsValid() || IsLocal(addr)) { ++addrman_num_bad_addresses; @@ -3695,7 +3695,7 @@ CConnman::~CConnman() std::vector CConnman::GetAddressesUnsafe(size_t max_addresses, size_t max_pct, std::optional network, const bool filtered) const { - std::vector addresses = addrman.GetAddr(max_addresses, max_pct, network, filtered); + std::vector addresses = addrman.get().GetAddr(max_addresses, max_pct, network, filtered); if (m_banman) { addresses.erase(std::remove_if(addresses.begin(), addresses.end(), [this](const CAddress& addr){return m_banman->IsDiscouraged(addr) || m_banman->IsBanned(addr);}), diff --git a/src/net.h b/src/net.h index e7047c5bff20..c23fac5fcd79 100644 --- a/src/net.h +++ b/src/net.h @@ -1592,7 +1592,7 @@ class CConnman std::vector vhListenSocket; std::atomic fNetworkActive{true}; bool fAddressesInitialized{false}; - AddrMan& addrman; + std::reference_wrapper addrman; const NetGroupManager& m_netgroupman; std::deque m_addr_fetches GUARDED_BY(m_addr_fetches_mutex); Mutex m_addr_fetches_mutex; From fabf8d1c5bdb6a944762792cdc762caa6c1a760b Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Thu, 15 Jan 2026 12:50:46 +0100 Subject: [PATCH 11/25] fuzz: Restore SendMessages coverage in process_message(s) fuzz targets --- src/test/fuzz/process_message.cpp | 27 ++++++++++++++++----------- src/test/fuzz/process_messages.cpp | 27 ++++++++++++++++----------- src/test/util/net.h | 2 ++ 3 files changed, 34 insertions(+), 22 deletions(-) diff --git a/src/test/fuzz/process_message.cpp b/src/test/fuzz/process_message.cpp index 809831a6efea..ef8cb686ceee 100644 --- a/src/test/fuzz/process_message.cpp +++ b/src/test/fuzz/process_message.cpp @@ -2,6 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include #include #include #include @@ -67,27 +68,31 @@ FUZZ_TARGET(process_message, .init = initialize_process_message) SeedRandomStateForTest(SeedRand::ZEROS); FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); - auto& connman = static_cast(*g_setup->m_node.connman); + auto& node{g_setup->m_node}; + auto& connman{static_cast(*node.connman)}; connman.ResetAddrCache(); connman.ResetMaxOutboundCycle(); - auto& chainman = static_cast(*g_setup->m_node.chainman); + auto& chainman{static_cast(*node.chainman)}; const auto block_index_size{WITH_LOCK(chainman.GetMutex(), return chainman.BlockIndex().size())}; SetMockTime(1610000000); // any time to successfully reset ibd chainman.ResetIbd(); chainman.DisableNextWrite(); - node::Warnings warnings{}; - NetGroupManager netgroupman{{}}; - AddrMan addrman{netgroupman, /*deterministic=*/true, /*consistency_check_ratio=*/0}; - auto peerman = PeerManager::make(connman, addrman, + // Reset, so that dangling pointers can be detected by sanitizers. + node.banman.reset(); + node.addrman.reset(); + node.peerman.reset(); + node.addrman = std::make_unique(*node.netgroupman, /*deterministic=*/true, /*consistency_check_ratio=*/0); + node.peerman = PeerManager::make(connman, *node.addrman, /*banman=*/nullptr, chainman, - *g_setup->m_node.mempool, warnings, + *node.mempool, *node.warnings, PeerManager::Options{ .reconcile_txs = true, .deterministic_rng = true, }); - connman.SetMsgProc(peerman.get()); + connman.SetMsgProc(node.peerman.get()); + connman.SetAddrman(*node.addrman); LOCK(NetEventsInterface::g_msgproc_mutex); const std::string random_message_type{fuzzed_data_provider.ConsumeBytesAsString(CMessageHeader::MESSAGE_TYPE_SIZE).c_str()}; @@ -116,10 +121,10 @@ FUZZ_TARGET(process_message, .init = initialize_process_message) more_work = connman.ProcessMessagesOnce(p2p_node); } catch (const std::ios_base::failure&) { } - g_setup->m_node.peerman->SendMessages(&p2p_node); + node.peerman->SendMessages(&p2p_node); } - g_setup->m_node.validation_signals->SyncWithValidationInterfaceQueue(); - g_setup->m_node.connman->StopNodes(); + node.validation_signals->SyncWithValidationInterfaceQueue(); + node.connman->StopNodes(); if (block_index_size != WITH_LOCK(chainman.GetMutex(), return chainman.BlockIndex().size())) { // Reuse the global chainman, but reset it when it is dirty ResetChainman(*g_setup); diff --git a/src/test/fuzz/process_messages.cpp b/src/test/fuzz/process_messages.cpp index baaeffe3dbf0..f36f528b0e39 100644 --- a/src/test/fuzz/process_messages.cpp +++ b/src/test/fuzz/process_messages.cpp @@ -2,6 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include #include #include #include @@ -57,26 +58,30 @@ FUZZ_TARGET(process_messages, .init = initialize_process_messages) SeedRandomStateForTest(SeedRand::ZEROS); FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); - auto& connman = static_cast(*g_setup->m_node.connman); + auto& node{g_setup->m_node}; + auto& connman{static_cast(*node.connman)}; connman.ResetAddrCache(); connman.ResetMaxOutboundCycle(); - auto& chainman = static_cast(*g_setup->m_node.chainman); + auto& chainman{static_cast(*node.chainman)}; const auto block_index_size{WITH_LOCK(chainman.GetMutex(), return chainman.BlockIndex().size())}; SetMockTime(1610000000); // any time to successfully reset ibd chainman.ResetIbd(); chainman.DisableNextWrite(); - node::Warnings warnings{}; - NetGroupManager netgroupman{{}}; - AddrMan addrman{netgroupman, /*deterministic=*/true, /*consistency_check_ratio=*/0}; - auto peerman = PeerManager::make(connman, addrman, + // Reset, so that dangling pointers can be detected by sanitizers. + node.banman.reset(); + node.addrman.reset(); + node.peerman.reset(); + node.addrman = std::make_unique(*node.netgroupman, /*deterministic=*/true, /*consistency_check_ratio=*/0); + node.peerman = PeerManager::make(connman, *node.addrman, /*banman=*/nullptr, chainman, - *g_setup->m_node.mempool, warnings, + *node.mempool, *node.warnings, PeerManager::Options{ .reconcile_txs = true, .deterministic_rng = true, }); - connman.SetMsgProc(peerman.get()); + connman.SetMsgProc(node.peerman.get()); + connman.SetAddrman(*node.addrman); LOCK(NetEventsInterface::g_msgproc_mutex); @@ -115,11 +120,11 @@ FUZZ_TARGET(process_messages, .init = initialize_process_messages) more_work = connman.ProcessMessagesOnce(random_node); } catch (const std::ios_base::failure&) { } - g_setup->m_node.peerman->SendMessages(&random_node); + node.peerman->SendMessages(&random_node); } } - g_setup->m_node.validation_signals->SyncWithValidationInterfaceQueue(); - g_setup->m_node.connman->StopNodes(); + node.validation_signals->SyncWithValidationInterfaceQueue(); + node.connman->StopNodes(); if (block_index_size != WITH_LOCK(chainman.GetMutex(), return chainman.BlockIndex().size())) { // Reuse the global chainman, but reset it when it is dirty ResetChainman(*g_setup); diff --git a/src/test/util/net.h b/src/test/util/net.h index 605b2fa81a07..ee02d404ec0d 100644 --- a/src/test/util/net.h +++ b/src/test/util/net.h @@ -40,6 +40,8 @@ struct ConnmanTestMsg : public CConnman { m_msgproc = msgproc; } + void SetAddrman(AddrMan& in) { addrman = in; } + void SetPeerConnectTimeout(std::chrono::seconds timeout) { m_peer_connect_timeout = timeout; From de509c6df97910cc11c5025d5f80fb05b9a88e80 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Thu, 15 Jan 2026 17:23:09 +0000 Subject: [PATCH 12/25] iwyu: Add missed line to IWYU patch This makes IWYU suggest `` over ``. --- ci/test/01_iwyu.patch | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ci/test/01_iwyu.patch b/ci/test/01_iwyu.patch index e14316bfb3c6..e7d75f4ee7ae 100644 --- a/ci/test/01_iwyu.patch +++ b/ci/test/01_iwyu.patch @@ -539,10 +539,11 @@ See: https://github.com/include-what-you-use/include-what-you-use/blob/clang_21/ }; const IncludeMapEntry stdlib_c_include_map[] = { -@@ -601,31 +601,31 @@ const IncludeMapEntry stdlib_c_include_map[] = { +@@ -600,32 +600,32 @@ const IncludeMapEntry stdlib_c_include_map[] = { + // https://github.com/cplusplus/draft/blob/c+%2B20/source/lib-intro.tex // // $ curl -s -N https://raw.githubusercontent.com/cplusplus/draft/c%2B%2B20/source/lib-intro.tex | sed -n '/begin{multicolfloattable}.*{headers.cpp.c}/,/end{multicolfloattable}/p' | grep tcode | perl -nle 'm/tcode{}/ && print qq@ { "<$1.h>", kPublic, "", kPublic },@' | sort - { "", kPublic, "", kPublic }, +- { "", kPublic, "", kPublic }, - { "", kPublic, "", kPublic }, - { "", kPublic, "", kPublic }, - { "", kPublic, "", kPublic }, @@ -568,6 +569,7 @@ See: https://github.com/include-what-you-use/include-what-you-use/blob/clang_21/ - { "", kPublic, "", kPublic }, - { "", kPublic, "", kPublic }, - { "", kPublic, "", kPublic }, ++ { "", kPrivate, "", kPublic }, + { "", kPrivate, "", kPublic }, + { "", kPrivate, "", kPublic }, + { "", kPrivate, "", kPublic }, From a5a8c4139c811e697b3c0b4d87737e04b60c53c8 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Fri, 16 Jan 2026 14:25:45 +0000 Subject: [PATCH 13/25] ci, iwyu: Fix warnings in `src/kernel` and treat them as errors --- ci/test/03_test_script.sh | 2 +- src/kernel/bitcoinkernel.cpp | 8 ++++---- src/kernel/chainparams.cpp | 6 +++++- src/kernel/chainparams.h | 3 +-- src/kernel/checks.cpp | 2 +- src/kernel/coinstats.cpp | 8 ++------ src/kernel/coinstats.h | 3 +-- src/kernel/disconnected_transactions.h | 2 ++ 8 files changed, 17 insertions(+), 17 deletions(-) diff --git a/ci/test/03_test_script.sh b/ci/test/03_test_script.sh index 1bd9fcfc69b0..0bb7bcbfa01c 100755 --- a/ci/test/03_test_script.sh +++ b/ci/test/03_test_script.sh @@ -215,7 +215,7 @@ fi if [[ "${RUN_IWYU}" == true ]]; then # TODO: Consider enforcing IWYU across the entire codebase. - FILES_WITH_ENFORCED_IWYU="/src/((crypto|index)/.*\\.cpp|node/blockstorage.cpp|node/utxo_snapshot.cpp|core_read.cpp|signet.cpp|kernel/chain.cpp)" + FILES_WITH_ENFORCED_IWYU="/src/((crypto|index|kernel)/.*\\.cpp|node/blockstorage.cpp|node/utxo_snapshot.cpp|core_read.cpp|signet.cpp)" jq --arg patterns "$FILES_WITH_ENFORCED_IWYU" 'map(select(.file | test($patterns)))' "${BASE_BUILD_DIR}/compile_commands.json" > "${BASE_BUILD_DIR}/compile_commands_iwyu_errors.json" jq --arg patterns "$FILES_WITH_ENFORCED_IWYU" 'map(select(.file | test($patterns) | not))' "${BASE_BUILD_DIR}/compile_commands.json" > "${BASE_BUILD_DIR}/compile_commands_iwyu_warnings.json" diff --git a/src/kernel/bitcoinkernel.cpp b/src/kernel/bitcoinkernel.cpp index bbcfd66b9301..8c7abef59f15 100644 --- a/src/kernel/bitcoinkernel.cpp +++ b/src/kernel/bitcoinkernel.cpp @@ -8,13 +8,12 @@ #include #include -#include #include +#include #include #include #include #include -#include #include #include #include @@ -27,9 +26,9 @@ #include #include #include -#include #include #include +#include #include #include #include @@ -38,14 +37,15 @@ #include #include -#include #include #include #include #include +#include #include #include #include +#include #include #include #include diff --git a/src/kernel/chainparams.cpp b/src/kernel/chainparams.cpp index 300df95fb8ed..c25455d0f1cc 100644 --- a/src/kernel/chainparams.cpp +++ b/src/kernel/chainparams.cpp @@ -21,10 +21,14 @@ #include #include +#include #include #include #include -#include +#include +#include +#include +#include using namespace util::hex_literals; diff --git a/src/kernel/chainparams.h b/src/kernel/chainparams.h index 99437ee727d4..f7209bee1b06 100644 --- a/src/kernel/chainparams.h +++ b/src/kernel/chainparams.h @@ -14,13 +14,12 @@ #include #include +#include #include -#include #include #include #include #include -#include #include struct AssumeutxoHash : public BaseHash { diff --git a/src/kernel/checks.cpp b/src/kernel/checks.cpp index fd010cc5b798..38ddfeef345f 100644 --- a/src/kernel/checks.cpp +++ b/src/kernel/checks.cpp @@ -8,7 +8,7 @@ #include #include -#include +#include namespace kernel { diff --git a/src/kernel/coinstats.cpp b/src/kernel/coinstats.cpp index 18a2de51ffa7..13c0bee836fa 100644 --- a/src/kernel/coinstats.cpp +++ b/src/kernel/coinstats.cpp @@ -12,23 +12,19 @@ #include #include #include