refactor: Avoid copies by using const references or by move-construction

maflcko commented at 8:41 pm on January 13, 2025: member

Top level const in declarations is problematic for many reasons:

It is often a typo, where one wanted to denote a const reference. For example bool PSBTInputSignedAndVerified(const PartiallySignedTransaction psbt, ... is missing the &. This will create a redundant copy of the value.
In constructors it prevents move construction.
It can incorrectly imply some data is const, like in an imaginary example std::span<int> Shuffle(const std::span<int>);, where the ints are not const.
The compiler ignores the const from the declaration in the implementation.
It isn’t used consistently anyway, not even on the same line.

Fix some issues by:

Using a const reference to avoid a copy, where read-only of the value is intended. This is only done for values that may be expensive to copy.
Using move-construction to avoid a copy

A prior version of this pull request also applied the readability-avoid-const-params-in-decls check via clang-tidy. However, this is left as a follow-up, because it is unclear how to deal with const in definitions (not declarations).

DrahtBot commented at 8:41 pm on January 13, 2025: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31650.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	hebasto, fanquake, l0rinc
Stale ACK	ryanofsky

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#bitcoin-core/gui/841 (Decouple WalletModel from RPCExecutor by furszy)
#29680 (wallet: fix unrelated parent conflict doesn’t cause child tx to be marked as conflict by Eunovo)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

DrahtBot renamed this:
~~refactor: Enforce readability-avoid-const-params-in-decls~~
refactor: Enforce readability-avoid-const-params-in-decls
on Jan 13, 2025

DrahtBot added the label Refactoring on Jan 13, 2025

hebasto commented at 9:01 am on January 14, 2025: member

Concept ACK.

maflcko force-pushed on Jan 14, 2025

DrahtBot added the label Needs rebase on Jan 16, 2025

maflcko force-pushed on Jan 20, 2025

DrahtBot removed the label Needs rebase on Jan 20, 2025

fanquake commented at 8:51 pm on February 20, 2025: member

Concept ACK - has enforcement, few conflicts, added to 30.x.

fanquake added this to the milestone 30.0 on Feb 20, 2025

maflcko force-pushed on Feb 20, 2025

DrahtBot added the label Needs rebase on Mar 20, 2025

maflcko force-pushed on Mar 20, 2025

DrahtBot removed the label Needs rebase on Mar 20, 2025

in src/external_signer.h:47 in fafe09b016 outdated

43@@ -44,7 +44,7 @@ class ExternalSigner
44     //! @param[in,out] signers  vector to which new signers (with a unique master key fingerprint) are added
45     //! @param chain            "main", "test", "regtest" or "signet"
46     //! @returns success
47-    static bool Enumerate(const std::string& command, std::vector<ExternalSigner>& signers, const std::string chain);
48+    static bool Enumerate(const std::string& command, std::vector<ExternalSigner>& signers, const std::string& chain);

ryanofsky commented at 6:53 pm on March 27, 2025:

In commit “refactor: Avoid copies by using const references or by move-construction” (fafe09b0161c8c8d22a1e2e1bad0428b51fca813)

This change seems reasonable but would point out that sometimes short string copies like this can be intentional: #24306 (review). I’d probably go with this change, but I don’t actually know if copying might be better.

maflcko commented at 9:31 pm on March 27, 2025:

I’d be surprised if anything made a difference here in this function and I think anything is fine here (std::string copy, const std::string& reference, or even a span or std::string_view).

in src/crypto/hex_base.h:19 in fa8e57951a outdated

13@@ -14,9 +14,9 @@
14 /**
15  * Convert a span of bytes to a lower-case hexadecimal string.
16  */
17-std::string HexStr(const std::span<const uint8_t> s);
18-inline std::string HexStr(const std::span<const char> s) { return HexStr(MakeUCharSpan(s)); }
19-inline std::string HexStr(const std::span<const std::byte> s) { return HexStr(MakeUCharSpan(s)); }
20+std::string HexStr(std::span<const uint8_t> s);
21+inline std::string HexStr(std::span<const char> s) { return HexStr(MakeUCharSpan(s)); }
22+inline std::string HexStr(std::span<const std::byte> s) { return HexStr(MakeUCharSpan(s)); }

ryanofsky commented at 6:58 pm on March 27, 2025:

In commit “refactor: Enforce readability-avoid-const-params-in-decls” (fa8e57951abc5c737ab68694b8f6aa77551faa6b)

Since these are definitions not declarations, I don’t think these actually need to change to fix the warnings. Does seem good to change, though.

maflcko commented at 8:18 am on March 31, 2025:

(dropped commit for now)

ryanofsky approved

ryanofsky commented at 7:01 pm on March 27, 2025: contributor

Code review ACK fa8e57951abc5c737ab68694b8f6aa77551faa6b. Agree it would be nice to enforce this check to reduce noise and prevent potential bugs

DrahtBot requested review from hebasto on Mar 27, 2025

DrahtBot requested review from fanquake on Mar 27, 2025

in src/deploymentinfo.h:30 in fa8e57951a outdated

26@@ -27,6 +27,6 @@ inline std::string DeploymentName(Consensus::DeploymentPos pos)
27     return VersionBitsDeploymentInfo[pos].name;
28 }
29 
30-std::optional<Consensus::BuriedDeployment> GetBuriedDeployment(const std::string_view deployment_name);
31+std::optional<Consensus::BuriedDeployment> GetBuriedDeployment(std::string_view deployment_name);

l0rinc commented at 2:15 pm on March 28, 2025:

Was the const std::string_view kept in the .cpp file on purpose? I’ve noticed we usually avoid consting a view…

I see we even have const std::string_view& - would it make sense to unify those usages here as well?

maflcko commented at 12:29 pm on March 30, 2025:

The goal of this pull is to avoid const params in declarations, not to avoid const params in definitions.

I can understand the confusion, so I am happy to do any of the following:

Close this pull
Just take the first commit and drop the second one
Add a commit to remove const in definitions as well, though, this at this point we may enter the territory where it isn’t worth it to spend review time on this refactor

l0rinc commented at 8:09 pm on March 30, 2025:

I can understand the confusion

I saw both were changed here, I think it’s a good change, I like consistency and though a few other similar ones could be added here.

Not sure why you’d want to close the PR because of my comments, if they’re not useful, you can explain it to me or just wait for other reviewers.

Feel free to resolve my comments.

maflcko commented at 8:18 am on March 31, 2025:

I think it’s a good change, I like consistency and though a few other similar ones could be added here.

I don’t think it is possible to achieve “consistency” here. There is no automated check to catch those in definitions. The goals here are:

Performance improvements (by the first commit). However, there are no benchmarks, so this is questionable.
Readability and style related improvements. However, they are not enforced consistently, so this is questionable as well.

So I’ll drop the second commit for now.

in src/wallet/wallet.cpp:2586 in fafe09b016 outdated

2582@@ -2583,7 +2583,7 @@ bool CWallet::TopUpKeyPool(unsigned int kpSize)
2583     return res;
2584 }
2585 
2586-util::Result<CTxDestination> CWallet::GetNewDestination(const OutputType type, const std::string label)
2587+util::Result<CTxDestination> CWallet::GetNewDestination(const OutputType type, const std::string& label)

l0rinc commented at 7:17 pm on March 28, 2025:

argsman_tests.cpp contains a few more similar ones:

0    void ReadConfigString(const std::string& str_config)
1    void SetNetworkOnlyArg(const std::string& arg)

maflcko commented at 8:18 am on March 31, 2025:

thx, done

in src/external_signer.cpp:19 in fafe09b016 outdated

14@@ -15,14 +15,15 @@
15 #include <string>
16 #include <vector>
17 
18-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) {}
19+ExternalSigner::ExternalSigner(std::string command, std::string chain, std::string fingerprint, std::string name)
20+    : m_command{std::move(command)}, m_chain{std::move(chain)}, m_fingerprint{std::move(fingerprint)}, m_name{std::move(name)} {}

l0rinc commented at 7:18 pm on March 28, 2025:

A similar one may be

 0diff --git a/src/netbase.h b/src/netbase.h
 1--- a/src/netbase.h	(revision 29a5a50583920e839fc3020c08f28b2f12a64d74)
 2+++ b/src/netbase.h	(date 1743172625447)
 3@@ -60,7 +60,7 @@
 4 public:
 5     Proxy() : m_is_unix_socket(false), m_randomize_credentials(false) {}
 6     explicit Proxy(const CService& _proxy, bool _randomize_credentials = false) : proxy(_proxy), m_is_unix_socket(false), m_randomize_credentials(_randomize_credentials) {}
 7-    explicit Proxy(const std::string path, bool _randomize_credentials = false) : m_unix_socket_path(path), m_is_unix_socket(true), m_randomize_credentials(_randomize_credentials) {}
 8+    explicit Proxy(std::string path, bool _randomize_credentials = false) : m_unix_socket_path(std::move(path)), m_is_unix_socket(true), m_randomize_credentials(_randomize_credentials) {}
 9 
10     CService proxy;
11     std::string m_unix_socket_path;

maflcko commented at 8:18 am on March 31, 2025:

thx, done

l0rinc commented at 7:41 pm on March 28, 2025: contributor

Concept ACK - this should clear up a few const usages.

After rebasing I found a few other similar ones, but not sure if they were ignored on purpose or not. Please consider the ones that are related to this change - and just skip the ones that you’ve ignored on purpose.

  0diff --git a/src/cuckoocache.h b/src/cuckoocache.h
  1index 8370179395..9dcceec739 100644
  2--- a/src/cuckoocache.h
  3+++ b/src/cuckoocache.h
  4@@ -471,7 +471,7 @@ public:
  5      * flag is set
  6      * [@returns](/bitcoin-bitcoin/contributor/returns/) true if the element is found, false otherwise
  7      */
  8-    inline bool contains(const Element& e, const bool erase) const
  9+    inline bool contains(const Element& e, bool erase) const
 10     {
 11         std::array<uint32_t, 8> locs = compute_hashes(e);
 12         for (const uint32_t loc : locs)
 13diff --git a/src/deploymentinfo.cpp b/src/deploymentinfo.cpp
 14index 185a7dcb54..237de65820 100644
 15--- a/src/deploymentinfo.cpp
 16+++ b/src/deploymentinfo.cpp
 17@@ -37,7 +37,7 @@ std::string DeploymentName(Consensus::BuriedDeployment dep)
 18     return "";
 19 }
 20 
 21-std::optional<Consensus::BuriedDeployment> GetBuriedDeployment(const std::string_view name)
 22+std::optional<Consensus::BuriedDeployment> GetBuriedDeployment(std::string_view name)
 23 {
 24     if (name == "segwit") {
 25         return Consensus::BuriedDeployment::DEPLOYMENT_SEGWIT;
 26diff --git a/src/kernel/mempool_entry.h b/src/kernel/mempool_entry.h
 27index 2adeaea652..4d2fa5013b 100644
 28--- a/src/kernel/mempool_entry.h
 29+++ b/src/kernel/mempool_entry.h
 30@@ -207,7 +207,7 @@ struct TransactionInfo {
 31     /* The block height the transaction entered the mempool */
 32     const unsigned int txHeight;
 33 
 34-    TransactionInfo(const CTransactionRef& tx, const CAmount& fee, const int64_t vsize, const unsigned int height)
 35+    TransactionInfo(const CTransactionRef& tx, const CAmount& fee, int64_t vsize, unsigned int height)
 36         : m_tx{tx},
 37           m_fee{fee},
 38           m_virtual_transaction_size{vsize},
 39@@ -238,10 +238,10 @@ struct NewMempoolTransactionInfo {
 40     const bool m_has_no_mempool_parents;
 41 
 42     explicit NewMempoolTransactionInfo(const CTransactionRef& tx, const CAmount& fee,
 43-                                       const int64_t vsize, const unsigned int height,
 44-                                       const bool mempool_limit_bypassed, const bool submitted_in_package,
 45-                                       const bool chainstate_is_current,
 46-                                       const bool has_no_mempool_parents)
 47+                                       const int64_t vsize, unsigned int height,
 48+                                       bool mempool_limit_bypassed, bool submitted_in_package,
 49+                                       bool chainstate_is_current,
 50+                                       bool has_no_mempool_parents)
 51         : info{tx, fee, vsize, height},
 52           m_mempool_limit_bypassed{mempool_limit_bypassed},
 53           m_submitted_in_package{submitted_in_package},
 54diff --git a/src/logging.h b/src/logging.h
 55index fdc12c79b3..de0e1cab7e 100644
 56--- a/src/logging.h
 57+++ b/src/logging.h
 58@@ -239,7 +239,7 @@ static inline bool LogAcceptCategory(BCLog::LogFlags category, BCLog::Level leve
 59 bool GetLogCategory(BCLog::LogFlags& flag, std::string_view str);
 60 
 61 template <typename... Args>
 62-inline void LogPrintFormatInternal(std::string_view logging_function, std::string_view source_file, const int source_line, const BCLog::LogFlags flag, const BCLog::Level level, util::ConstevalFormatString<sizeof...(Args)> fmt, const Args&... args)
 63+inline void LogPrintFormatInternal(std::string_view logging_function, std::string_view source_file, int source_line, BCLog::LogFlags flag, BCLog::Level level, util::ConstevalFormatString<sizeof...(Args)> fmt, const Args&... args)
 64 {
 65     if (LogInstance().Enabled()) {
 66         std::string log_msg;
 67diff --git a/src/netbase.h b/src/netbase.h
 68index d7c6e1c215..02b61a29de 100644
 69--- a/src/netbase.h
 70+++ b/src/netbase.h
 71@@ -60,7 +60,7 @@ class Proxy
 72 public:
 73     Proxy() : m_is_unix_socket(false), m_randomize_credentials(false) {}
 74     explicit Proxy(const CService& _proxy, bool _randomize_credentials = false) : proxy(_proxy), m_is_unix_socket(false), m_randomize_credentials(_randomize_credentials) {}
 75-    explicit Proxy(const std::string path, bool _randomize_credentials = false) : m_unix_socket_path(path), m_is_unix_socket(true), m_randomize_credentials(_randomize_credentials) {}
 76+    explicit Proxy(std::string path, bool _randomize_credentials = false) : m_unix_socket_path(std::move(path)), m_is_unix_socket(true), m_randomize_credentials(_randomize_credentials) {}
 77 
 78     CService proxy;
 79     std::string m_unix_socket_path;
 80diff --git a/src/node/miner.h b/src/node/miner.h
 81index c09e9eb5d1..2940df5435 100644
 82--- a/src/node/miner.h
 83+++ b/src/node/miner.h
 84@@ -220,7 +220,7 @@ private:
 85  * accounts for the BIP94 timewarp rule, so does not necessarily reflect the
 86  * consensus limit.
 87  */
 88-int64_t GetMinimumTime(const CBlockIndex* pindexPrev, const int64_t difficulty_adjustment_interval);
 89+int64_t GetMinimumTime(const CBlockIndex* pindexPrev, int64_t difficulty_adjustment_interval);
 90 
 91 int64_t UpdateTime(CBlockHeader* pblock, const Consensus::Params& consensusParams, const CBlockIndex* pindexPrev);
 92 
 93diff --git a/src/qt/addresstablemodel.h b/src/qt/addresstablemodel.h
 94index 877b01d882..595b815afd 100644
 95--- a/src/qt/addresstablemodel.h
 96+++ b/src/qt/addresstablemodel.h
 97@@ -90,7 +90,7 @@ public:
 98     QString GetWalletDisplayName() const;
 99 
100 private:
101-    WalletModel* const walletModel;
102+    WalletModel* walletModel;
103     AddressTablePriv *priv = nullptr;
104     QStringList columns;
105     EditStatus editStatus = OK;
106diff --git a/src/qt/walletview.h b/src/qt/walletview.h
107index 475085044d..687f22234a 100644
108--- a/src/qt/walletview.h
109+++ b/src/qt/walletview.h
110@@ -56,7 +56,7 @@ private:
111     //! The wallet model represents a bitcoin wallet, and offers access to
112     //! the list of transactions, address book and sending functionality.
113     //!
114-    WalletModel* const walletModel;
115+    WalletModel* walletModel;
116 
117     OverviewPage *overviewPage;
118     QWidget *transactionsPage;
119diff --git a/src/script/miniscript.h b/src/script/miniscript.h
120index 041186cae0..feb650bcf2 100644
121--- a/src/script/miniscript.h
122+++ b/src/script/miniscript.h
123@@ -1773,7 +1773,7 @@ std::optional<std::pair<std::vector<unsigned char>, int>> ParseHexStrEnd(std::sp
124 
125 /** BuildBack pops the last two elements off `constructed` and wraps them in the specified Fragment */
126 template<typename Key>
127-void BuildBack(const MiniscriptContext script_ctx, Fragment nt, std::vector<NodeRef<Key>>& constructed, const bool reverse = false)
128+void BuildBack(const MiniscriptContext script_ctx, Fragment nt, std::vector<NodeRef<Key>>& constructed, bool reverse = false)
129 {
130     NodeRef<Key> child = std::move(constructed.back());
131     constructed.pop_back();
132@@ -1814,7 +1814,7 @@ inline NodeRef<Key> Parse(std::span<const char> in, const Ctx& ctx)
133     to_parse.emplace_back(ParseContext::WRAPPED_EXPR, -1, -1);
134 
135     // Parses a multi() or multi_a() from its string representation. Returns false on parsing error.
136-    const auto parse_multi_exp = [&](std::span<const char>& in, const bool is_multi_a) -> bool {
137+    const auto parse_multi_exp = [&](std::span<const char>& in, bool is_multi_a) -> bool {
138         const auto max_keys{is_multi_a ? MAX_PUBKEYS_PER_MULTI_A : MAX_PUBKEYS_PER_MULTISIG};
139         const auto required_ctx{is_multi_a ? MiniscriptContext::TAPSCRIPT : MiniscriptContext::P2WSH};
140         if (ctx.MsContext() != required_ctx) return false;
141diff --git a/src/streams.h b/src/streams.h
142index 20bdaf2c06..f6bc0478a3 100644
143--- a/src/streams.h
144+++ b/src/streams.h
145@@ -547,7 +547,7 @@ public:
146 
147     //! Move the read position ahead in the stream to the given position.
148     //! Use SetPos() to back up in the stream, not SkipTo().
149-    void SkipTo(const uint64_t file_pos)
150+    void SkipTo(uint64_t file_pos)
151     {
152         assert(file_pos >= m_read_pos);
153         while (m_read_pos < file_pos) AdvanceStream(file_pos - m_read_pos);
154diff --git a/src/test/argsman_tests.cpp b/src/test/argsman_tests.cpp
155index 297595a9cf..92ffacdfdc 100644
156--- a/src/test/argsman_tests.cpp
157+++ b/src/test/argsman_tests.cpp
158@@ -52,7 +52,7 @@ BOOST_AUTO_TEST_CASE(util_datadir)
159 struct TestArgsManager : public ArgsManager
160 {
161     TestArgsManager() { m_network_only_args.clear(); }
162-    void ReadConfigString(const std::string str_config)
163+    void ReadConfigString(const std::string& str_config)
164     {
165         std::istringstream streamConfig(str_config);
166         {
167@@ -63,7 +63,7 @@ struct TestArgsManager : public ArgsManager
168         std::string error;
169         BOOST_REQUIRE(ReadConfigStream(streamConfig, "", error));
170     }
171-    void SetNetworkOnlyArg(const std::string arg)
172+    void SetNetworkOnlyArg(const std::string& arg)
173     {
174         LOCK(cs_args);
175         m_network_only_args.insert(arg);
176diff --git a/src/test/util/setup_common.h b/src/test/util/setup_common.h
177index 52d7bd0efc..3bea74d9f7 100644
178--- a/src/test/util/setup_common.h
179+++ b/src/test/util/setup_common.h
180@@ -258,7 +258,7 @@ struct TestChain100Setup : public TestingSetup {
181  * be used in "hot loops", for example fuzzing or benchmarking.
182  */
183 template <class T = const BasicTestingSetup>
184-std::unique_ptr<T> MakeNoLogFileContext(const ChainType chain_type = ChainType::REGTEST, TestOpts opts = {})
185+std::unique_ptr<T> MakeNoLogFileContext(ChainType chain_type = ChainType::REGTEST, TestOpts opts = {})
186 {
187     opts.extra_args = Cat(
188         {
189diff --git a/src/uint256.cpp b/src/uint256.cpp
190index 2756a7f5cd..f9fe8187a6 100644
191--- a/src/uint256.cpp
192+++ b/src/uint256.cpp
193@@ -18,7 +18,7 @@ std::string base_blob<BITS>::GetHex() const
194 }
195 
196 template <unsigned int BITS>
197-void base_blob<BITS>::SetHexDeprecated(const std::string_view str)
198+void base_blob<BITS>::SetHexDeprecated(std::string_view str)
199 {
200     std::fill(m_data.begin(), m_data.end(), 0);
201 
202diff --git a/src/util/strencodings.h b/src/util/strencodings.h
203index fd713a555c..1875810ab8 100644
204--- a/src/util/strencodings.h
205+++ b/src/util/strencodings.h
206@@ -369,7 +369,7 @@ std::optional<uint64_t> ParseByteUnits(std::string_view str, ByteUnit default_mu
207 
208 namespace util {
209 /** consteval version of HexDigit() without the lookup table. */
210-consteval uint8_t ConstevalHexDigit(const char c)
211+consteval uint8_t ConstevalHexDigit(char c)
212 {
213     if (c >= '0' && c <= '9') return c - '0';
214     if (c >= 'a' && c <= 'f') return c - 'a' + 0xa;
215diff --git a/src/wallet/coinselection.h b/src/wallet/coinselection.h
216index e029f39430..c5e4526412 100644
217--- a/src/wallet/coinselection.h
218+++ b/src/wallet/coinselection.h
219@@ -93,7 +93,7 @@ public:
220         }
221     }
222 
223-    COutput(const COutPoint& outpoint, const CTxOut& txout, int depth, int input_bytes, bool spendable, bool solvable, bool safe, int64_t time, bool from_me, const CAmount fees)
224+    COutput(const COutPoint& outpoint, const CTxOut& txout, int depth, int input_bytes, bool spendable, bool solvable, bool safe, int64_t time, bool from_me, CAmount fees)
225         : COutput(outpoint, txout, depth, input_bytes, spendable, solvable, safe, time, from_me)
226     {
227         // if input_bytes is unknown, then fees should be 0, if input_bytes is known, then the fees should be a positive integer or 0 (input_bytes known and fees = 0 only happens in the tests)
228@@ -355,7 +355,7 @@ private:
229     }
230 
231 public:
232-    explicit SelectionResult(const CAmount target, SelectionAlgorithm algo)
233+    explicit SelectionResult(CAmount target, SelectionAlgorithm algo)
234         : m_target(target), m_algo(algo) {}
235 
236     SelectionResult() = delete;
237diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h
238index 91dfd35479..322770e197 100644
239--- a/src/wallet/scriptpubkeyman.h
240+++ b/src/wallet/scriptpubkeyman.h
241@@ -179,14 +179,14 @@ protected:
242 public:
243     explicit ScriptPubKeyMan(WalletStorage& storage) : m_storage(storage) {}
244     virtual ~ScriptPubKeyMan() = default;
245-    virtual util::Result<CTxDestination> GetNewDestination(const OutputType type) { return util::Error{Untranslated("Not supported")}; }
246+    virtual util::Result<CTxDestination> GetNewDestination(OutputType type) { return util::Error{Untranslated("Not supported")}; }
247     virtual isminetype IsMine(const CScript& script) const { return ISMINE_NO; }
248 
249     //! Check that the given decryption key is valid for this ScriptPubKeyMan, i.e. it decrypts all of the keys handled by it.
250     virtual bool CheckDecryptionKey(const CKeyingMaterial& master_key) { return false; }
251     virtual bool Encrypt(const CKeyingMaterial& master_key, WalletBatch* batch) { return false; }
252 
253-    virtual util::Result<CTxDestination> GetReservedDestination(const OutputType type, bool internal, int64_t& index, CKeyPool& keypool) { return util::Error{Untranslated("Not supported")}; }
254+    virtual util::Result<CTxDestination> GetReservedDestination(OutputType type, bool internal, int64_t& index, CKeyPool& keypool) { return util::Error{Untranslated("Not supported")}; }
255     virtual void KeepDestination(int64_t index, const OutputType& type) {}
256     virtual void ReturnDestination(int64_t index, bool internal, const CTxDestination& addr) {}
257 
258diff --git a/src/wallet/test/util.h b/src/wallet/test/util.h
259index 3acaba0202..e584cd10e5 100644
260--- a/src/wallet/test/util.h
261+++ b/src/wallet/test/util.h
262@@ -128,7 +128,7 @@ public:
263 std::unique_ptr<WalletDatabase> CreateMockableWalletDatabase(MockableData records = {});
264 MockableDatabase& GetMockableDatabase(CWallet& wallet);
265 
266-ScriptPubKeyMan* CreateDescriptor(CWallet& keystore, const std::string& desc_str, const bool success);
267+ScriptPubKeyMan* CreateDescriptor(CWallet& keystore, const std::string& desc_str, bool success);
268 } // namespace wallet
269 
270 #endif // BITCOIN_WALLET_TEST_UTIL_H
271diff --git a/src/zmq/zmqabstractnotifier.h b/src/zmq/zmqabstractnotifier.h
272index 17fa7bbaa9..1cf54b8a78 100644
273--- a/src/zmq/zmqabstractnotifier.h
274+++ b/src/zmq/zmqabstractnotifier.h
275@@ -35,7 +35,7 @@ public:
276     std::string GetAddress() const { return address; }
277     void SetAddress(const std::string &a) { address = a; }
278     int GetOutboundMessageHighWaterMark() const { return outbound_message_high_water_mark; }
279-    void SetOutboundMessageHighWaterMark(const int sndhwm) {
280+    void SetOutboundMessageHighWaterMark(int sndhwm) {
281         if (sndhwm >= 0) {
282             outbound_message_high_water_mark = sndhwm;
283         }

maflcko removed this from the milestone 30.0 on Mar 31, 2025

refactor: Avoid copies by using const references or by move-construction fa9cd7bd4f

maflcko renamed this:
~~refactor: Enforce readability-avoid-const-params-in-decls~~
refactor: Avoid copies by using const references or by move-construction
on Mar 31, 2025

DrahtBot renamed this:
~~refactor: Avoid copies by using const references or by move-construction~~
refactor: Avoid copies by using const references or by move-construction
on Mar 31, 2025

maflcko force-pushed on Mar 31, 2025

maflcko commented at 8:18 am on March 31, 2025: member

Dropped the second commit, changed pull title and description, and rebased. Should be trivial to re-review.

refactor: Avoid copies by using const references or by move-construction #31650

Code Coverage & Benchmarks

Reviews

Conflicts