[RFC] refactor: fix and then enforce param naming via `clang-tidy` #36781

38f497d refactor: use consistent parameter names in declarations and definitions:

I understand you chose the version with the minimal diff, but since we’re fixing inconsistencies, we might as well use in_bytes / out_bytes consistently for both Crypt methods.

<details> <summary>Use consistent ChaCha20 buffer parameter names</summary>

diff --git a/src/crypto/chacha20.cpp b/src/crypto/chacha20.cpp
index 6bdffe691a..ba20293225 100644
--- a/src/crypto/chacha20.cpp
+++ b/src/crypto/chacha20.cpp
@@ -299,32 +299,32 @@ void ChaCha20::Keystream(std::span<std::byte> out) noexcept
     }
 }
 
-void ChaCha20::Crypt(std::span<const std::byte> input, std::span<std::byte> output) noexcept
+void ChaCha20::Crypt(std::span<const std::byte> in_bytes, std::span<std::byte> out_bytes) noexcept
 {
-    assert(input.size() == output.size());
+    assert(in_bytes.size() == out_bytes.size());
 
-    if (!input.size()) return;
+    if (!in_bytes.size()) return;
     if (m_bufleft) {
-        unsigned reuse = std::min<size_t>(m_bufleft, input.size());
+        unsigned reuse = std::min<size_t>(m_bufleft, in_bytes.size());
         for (unsigned i = 0; i < reuse; i++) {
-            output[i] = input[i] ^ m_buffer[m_aligned.BLOCKLEN - m_bufleft + i];
+            out_bytes[i] = in_bytes[i] ^ m_buffer[m_aligned.BLOCKLEN - m_bufleft + i];
         }
         m_bufleft -= reuse;
-        output = output.subspan(reuse);
-        input = input.subspan(reuse);
+        out_bytes = out_bytes.subspan(reuse);
+        in_bytes = in_bytes.subspan(reuse);
     }
-    if (input.size() >= m_aligned.BLOCKLEN) {
-        size_t blocks = input.size() / m_aligned.BLOCKLEN;
-        m_aligned.Crypt(input.first(blocks * m_aligned.BLOCKLEN), output.first(blocks * m_aligned.BLOCKLEN));
-        output = output.subspan(blocks * m_aligned.BLOCKLEN);
-        input = input.subspan(blocks * m_aligned.BLOCKLEN);
+    if (in_bytes.size() >= m_aligned.BLOCKLEN) {
+        size_t blocks = in_bytes.size() / m_aligned.BLOCKLEN;
+        m_aligned.Crypt(in_bytes.first(blocks * m_aligned.BLOCKLEN), out_bytes.first(blocks * m_aligned.BLOCKLEN));
+        out_bytes = out_bytes.subspan(blocks * m_aligned.BLOCKLEN);
+        in_bytes = in_bytes.subspan(blocks * m_aligned.BLOCKLEN);
     }
-    if (!input.empty()) {
+    if (!in_bytes.empty()) {
         m_aligned.Keystream(m_buffer);
-        for (unsigned i = 0; i < input.size(); i++) {
-            output[i] = input[i] ^ m_buffer[i];
+        for (unsigned i = 0; i < in_bytes.size(); i++) {
+            out_bytes[i] = in_bytes[i] ^ m_buffer[i];
         }
-        m_bufleft = m_aligned.BLOCKLEN - input.size();
+        m_bufleft = m_aligned.BLOCKLEN - in_bytes.size();
     }
 }
 
diff --git a/src/crypto/chacha20.h b/src/crypto/chacha20.h
index 3c0906918f..9d8954ba6d 100644
--- a/src/crypto/chacha20.h
+++ b/src/crypto/chacha20.h
@@ -64,7 +64,7 @@ public:
     /** outputs the keystream into out, whose length must be a multiple of BLOCKLEN. */
     void Keystream(std::span<std::byte> out) noexcept;
 
-    /** en/deciphers the message <input> and write the result into <output>
+    /** en/deciphers the message <in_bytes> and write the result into <out_bytes>
      *
      * The size of in_bytes and out_bytes must be equal, and be a multiple of BLOCKLEN.
      */
@@ -107,9 +107,9 @@ public:
 
     /** en/deciphers the message <in_bytes> and write the result into <out_bytes>
      *
-     * The size of input and output must be equal.
+     * The size of in_bytes and out_bytes must be equal.
      */
-    void Crypt(std::span<const std::byte> input, std::span<std::byte> output) noexcept;
+    void Crypt(std::span<const std::byte> in_bytes, std::span<std::byte> out_bytes) noexcept;
 
     /** outputs the keystream to out. */
     void Keystream(std::span<std::byte> out) noexcept;

</details>

38f497d refactor: use consistent parameter names in declarations and definitions:

Same, matching the definition keeps the diff smaller, but keeping min_incremental_fee here and renaming the parameter in the definition would be closer to our current coding style.

<details> <summary>Keep the snake_case FeeFilterRounder parameter</summary>

diff --git a/src/policy/fees/block_policy_estimator.cpp b/src/policy/fees/block_policy_estimator.cpp
index 9035e36138..7dfbc8e574 100644
--- a/src/policy/fees/block_policy_estimator.cpp
+++ b/src/policy/fees/block_policy_estimator.cpp
@@ -1129,8 +1129,8 @@ static std::set<double> MakeFeeSet(const CFeeRate& min_incremental_fee,
     return fee_set;
 }
 
-FeeFilterRounder::FeeFilterRounder(const CFeeRate& minIncrementalFee, FastRandomContext& rng)
-    : m_fee_set{MakeFeeSet(minIncrementalFee, MAX_FILTER_FEERATE, FEE_FILTER_SPACING)},
+FeeFilterRounder::FeeFilterRounder(const CFeeRate& min_incremental_fee, FastRandomContext& rng)
+    : m_fee_set{MakeFeeSet(min_incremental_fee, MAX_FILTER_FEERATE, FEE_FILTER_SPACING)},
       insecure_rand{rng}
 {
 }
diff --git a/src/policy/fees/block_policy_estimator.h b/src/policy/fees/block_policy_estimator.h
index 473161ab58..6253d3954c 100644
--- a/src/policy/fees/block_policy_estimator.h
+++ b/src/policy/fees/block_policy_estimator.h
@@ -330,7 +330,7 @@ private:
 
 public:
     /** Create new FeeFilterRounder */
-    explicit FeeFilterRounder(const CFeeRate& minIncrementalFee, FastRandomContext& rng);
+    explicit FeeFilterRounder(const CFeeRate& min_incremental_fee, FastRandomContext& rng);
 
     /** Quantize a minimum fee for privacy purpose before broadcast. */
     CAmount round(CAmount currentMinFee) EXCLUSIVE_LOCKS_REQUIRED(!m_insecure_rand_mutex);

</details>

38f497d refactor: use consistent parameter names in declarations and definitions:

Alternatively we could keep script rather than in in the script hash constructors, and keyid rather than address in CreateSig, updating the definitions instead (the base CreateSig declaration already uses keyid).

<details> <summary>Keep descriptive script and key parameter names</summary>

diff --git a/src/addresstype.cpp b/src/addresstype.cpp
index b486951712..fca8bfa8bb 100644
--- a/src/addresstype.cpp
+++ b/src/addresstype.cpp
@@ -17,7 +17,7 @@
 
 typedef std::vector<unsigned char> valtype;
 
-ScriptHash::ScriptHash(const CScript& in) : BaseHash(Hash160(in)) {}
+ScriptHash::ScriptHash(const CScript& script) : BaseHash(Hash160(script)) {}
 
 PKHash::PKHash(const CPubKey& pubkey) : BaseHash(pubkey.GetID()) {}
 PKHash::PKHash(const CKeyID& pubkey_id) : BaseHash(pubkey_id) {}
@@ -40,9 +40,9 @@ CScriptID ToScriptID(const ScriptHash& script_hash)
     return CScriptID{uint160{script_hash}};
 }
 
-WitnessV0ScriptHash::WitnessV0ScriptHash(const CScript& in)
+WitnessV0ScriptHash::WitnessV0ScriptHash(const CScript& script)
 {
-    CSHA256().Write(in.data(), in.size()).Finalize(begin());
+    CSHA256().Write(script.data(), script.size()).Finalize(begin());
 }
 
 bool ExtractDestination(const CScript& scriptPubKey, CTxDestination& addressRet)
diff --git a/src/addresstype.h b/src/addresstype.h
index 7ed8a147a6..fc85ed0795 100644
--- a/src/addresstype.h
+++ b/src/addresstype.h
@@ -64,7 +64,7 @@ struct ScriptHash : public BaseHash<uint160>
     explicit ScriptHash(const PKHash& hash) = delete;
 
     explicit ScriptHash(const uint160& hash) : BaseHash(hash) {}
-    explicit ScriptHash(const CScript& in);
+    explicit ScriptHash(const CScript& script);
 };
 CScriptID ToScriptID(const ScriptHash& script_hash);
 
@@ -72,7 +72,7 @@ struct WitnessV0ScriptHash : public BaseHash<uint256>
 {
     WitnessV0ScriptHash() : BaseHash() {}
     explicit WitnessV0ScriptHash(const uint256& hash) : BaseHash(hash) {}
-    explicit WitnessV0ScriptHash(const CScript& in);
+    explicit WitnessV0ScriptHash(const CScript& script);
 };
 
 struct WitnessV0KeyHash : public BaseHash<uint160>
diff --git a/src/script/sign.cpp b/src/script/sign.cpp
index b6fc59c78d..c14f3aeeb2 100644
--- a/src/script/sign.cpp
+++ b/src/script/sign.cpp
@@ -51,12 +51,12 @@ MutableTransactionSignatureCreator::MutableTransactionSignatureCreator(const CMu
 {
 }
 
-bool MutableTransactionSignatureCreator::CreateSig(const SigningProvider& provider, std::vector<unsigned char>& vchSig, const CKeyID& address, const CScript& scriptCode, SigVersion sigversion) const
+bool MutableTransactionSignatureCreator::CreateSig(const SigningProvider& provider, std::vector<unsigned char>& vchSig, const CKeyID& keyid, const CScript& scriptCode, SigVersion sigversion) const
 {
     assert(sigversion == SigVersion::BASE || sigversion == SigVersion::WITNESS_V0);
 
     CKey key;
-    if (!provider.GetKey(address, key))
+    if (!provider.GetKey(keyid, key))
         return false;
 
     // Signing with uncompressed keys is disabled in witness scripts
diff --git a/src/script/sign.h b/src/script/sign.h
index a931f67f89..107abb9e84 100644
--- a/src/script/sign.h
+++ b/src/script/sign.h
@@ -65,7 +65,7 @@ public:
     MutableTransactionSignatureCreator(const CMutableTransaction& tx LIFETIMEBOUND, unsigned int input_idx, const CAmount& amount, const SignOptions& options);
     MutableTransactionSignatureCreator(const CMutableTransaction& tx LIFETIMEBOUND, unsigned int input_idx, const CAmount& amount, const PrecomputedTransactionData* txdata, const SignOptions& options);
     const BaseSignatureChecker& Checker() const override { return checker; }
-    bool CreateSig(const SigningProvider& provider, std::vector<unsigned char>& vchSig, const CKeyID& address, const CScript& scriptCode, SigVersion sigversion) const override;
+    bool CreateSig(const SigningProvider& provider, std::vector<unsigned char>& vchSig, const CKeyID& keyid, const CScript& scriptCode, SigVersion sigversion) const override;
     bool CreateSchnorrSig(const SigningProvider& provider, std::vector<unsigned char>& sig, const XOnlyPubKey& pubkey, const uint256* leaf_hash, const uint256* merkle_root, SigVersion sigversion) const override;
     std::vector<uint8_t> CreateMuSig2Nonce(const SigningProvider& provider, const CPubKey& aggregate_pubkey, const CPubKey& script_pubkey, const CPubKey& part_pubkey, const uint256* leaf_hash, const uint256* merkle_root, SigVersion sigversion, const SignatureData& sigdata) const override;
     bool CreateMuSig2PartialSig(const SigningProvider& provider, uint256& partial_sig, const CPubKey& aggregate_pubkey, const CPubKey& script_pubkey, const CPubKey& part_pubkey, const uint256* leaf_hash, const std::vector<std::pair<uint256, bool>>& tweaks, SigVersion sigversion, const SignatureData& sigdata) const override;

</details>

38f497d refactor: use consistent parameter names in declarations and definitions:

Since we’re unifying parameter names, it seems more consistent to me if we also add back the missing names.: the complementary readability-named-parameter check (with InsertPlainNamesInForwardDecls: true) could catch these cases.

<details> <summary>Add missing parameter names to headers</summary>

diff --git a/src/common/args.h b/src/common/args.h
index 5079ad1a3d..9736f7d45e 100644
--- a/src/common/args.h
+++ b/src/common/args.h
@@ -91,12 +91,12 @@ struct SectionInfo {
     int m_line;
 };
 
-std::optional<std::string> SettingToString(const common::SettingsValue&);
+std::optional<std::string> SettingToString(const common::SettingsValue& value);
 
 template <std::integral Int>
-std::optional<Int> SettingTo(const common::SettingsValue&);
+std::optional<Int> SettingTo(const common::SettingsValue& value);
 
-std::optional<bool> SettingToBool(const common::SettingsValue&);
+std::optional<bool> SettingToBool(const common::SettingsValue& value);
 
 class ArgsManager
 {
diff --git a/src/validationinterface.h b/src/validationinterface.h
index ec70cc0956..36b92d7f06 100644
--- a/src/validationinterface.h
+++ b/src/validationinterface.h
@@ -221,16 +221,16 @@ public:
      */
     void SyncWithValidationInterfaceQueue() LOCKS_EXCLUDED(cs_main);
 
-    void UpdatedBlockTip(const CBlockIndex *, const CBlockIndex *, bool fInitialDownload);
-    void ActiveTipChange(const CBlockIndex&, bool);
-    void TransactionAddedToMempool(const NewMempoolTransactionInfo&, uint64_t mempool_sequence);
-    void TransactionRemovedFromMempool(const CTransactionRef&, MemPoolRemovalReason, uint64_t mempool_sequence);
-    void MempoolTransactionsRemovedForBlock(std::shared_ptr<const CBlock>, std::vector<RemovedMempoolTransactionInfo>, unsigned int block_height);
-    void BlockConnected(const kernel::ChainstateRole&, std::shared_ptr<const CBlock>, const CBlockIndex* pindex);
-    void BlockDisconnected(std::shared_ptr<const CBlock>, const CBlockIndex* pindex);
-    void ChainStateFlushed(const kernel::ChainstateRole&, const CBlockLocator&);
-    void BlockChecked(const std::shared_ptr<const CBlock>&, const BlockValidationState&);
-    void NewPoWValidBlock(const CBlockIndex *, const std::shared_ptr<const CBlock>&);
+    void UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload);
+    void ActiveTipChange(const CBlockIndex& new_tip, bool is_ibd);
+    void TransactionAddedToMempool(const NewMempoolTransactionInfo& tx, uint64_t mempool_sequence);
+    void TransactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason, uint64_t mempool_sequence);
+    void MempoolTransactionsRemovedForBlock(std::shared_ptr<const CBlock> block, std::vector<RemovedMempoolTransactionInfo> txs_removed_for_block, unsigned int block_height);
+    void BlockConnected(const kernel::ChainstateRole& role, std::shared_ptr<const CBlock> pblock, const CBlockIndex* pindex);
+    void BlockDisconnected(std::shared_ptr<const CBlock> pblock, const CBlockIndex* pindex);
+    void ChainStateFlushed(const kernel::ChainstateRole& role, const CBlockLocator& locator);
+    void BlockChecked(const std::shared_ptr<const CBlock>& block, const BlockValidationState& state);
+    void NewPoWValidBlock(const CBlockIndex *pindex, const std::shared_ptr<const CBlock>& block);
 };
 
 #endif // BITCOIN_VALIDATIONINTERFACE_H
diff --git a/src/wallet/rpc/util.h b/src/wallet/rpc/util.h
index 3e349b7b1d..bcefeab08a 100644
--- a/src/wallet/rpc/util.h
+++ b/src/wallet/rpc/util.h
@@ -44,7 +44,7 @@ std::shared_ptr<CWallet> GetWalletForJSONRPCRequest(const JSONRPCRequest& reques
  */
 std::string EnsureUniqueWalletName(const JSONRPCRequest& request, std::optional<std::string_view> wallet_name);
 
-void EnsureWalletIsUnlocked(const CWallet&);
+void EnsureWalletIsUnlocked(const CWallet& wallet);
 WalletContext& EnsureWalletContext(const std::any& context);
 
 bool GetAvoidReuseFlag(const CWallet& wallet, const UniValue& param);

</details>