Merge bitcoin#24661: refactor: Use clang-tidy syntax for C++ named arguments

37a16ff refactor: fix clang-tidy named args usage (fanquake)

Pull request description:

  > Incorrect named args are source of bugs, like bitcoin#22979.

  > To allow them being checked by clang-tidy, use a format it can understand.

  Picks up bitcoin#23545, with some additional changes and some feedback addressed.

  With these changes invoking `./autogen.sh && ./configure CC=clang-12 CXX=clang++-12 && make clean && bear make -j9 && ( cd ./src/ && run-clang-tidy-12 -j9 )` no-longer results in named argument errors out of `clang-tidy`.

  Ultimately I think we should just add `clang-tidy-*` jobs to the CI and automate things away.

ACKs for top commit:
  MarcoFalke:
    cr ACK 37a16ff

Tree-SHA512: 9bfc0d006eb187755b4fdb0bd92cee9266fc0816be42065ef7dcd885b9020ff12e3cdd7ca3a831613a56a0206d448e690ee4e1fa37628fa2013860e17f416ff3

Author: MarcoFalke

src/addrdb.cpp

@@ -185,7 +185,7 @@ void ReadFromStream(AddrMan& addr, CDataStream& ssPeers)
 std::optional<bilingual_str> LoadAddrman(const std::vector<bool>& asmap, const ArgsManager& args, std::unique_ptr<AddrMan>& addrman)
 {
     auto check_addrman = std::clamp<int32_t>(args.GetIntArg("-checkaddrman", DEFAULT_ADDRMAN_CONSISTENCY_CHECKS), 0, 1000000);
-    addrman = std::make_unique<AddrMan>(asmap, /* deterministic */ false, /* consistency_check_ratio */ check_addrman);
+    addrman = std::make_unique<AddrMan>(asmap, /*deterministic=*/false, /*consistency_check_ratio=*/check_addrman);
 
     int64_t nStart = GetTimeMillis();
     const auto path_addr{args.GetDataDirNet() / "peers.dat"};
@@ -194,7 +194,7 @@ std::optional<bilingual_str> LoadAddrman(const std::vector<bool>& asmap, const A
         LogPrintf("Loaded %i addresses from peers.dat  %dms\n", addrman->size(), GetTimeMillis() - nStart);
     } catch (const DbNotFoundError&) {
         // Addrman can be in an inconsistent state after failure, reset it
-        addrman = std::make_unique<AddrMan>(asmap, /* deterministic */ false, /* consistency_check_ratio */ check_addrman);
+        addrman = std::make_unique<AddrMan>(asmap, /*deterministic=*/false, /*consistency_check_ratio=*/check_addrman);
         LogPrintf("Creating peers.dat because the file was not found (%s)\n", fs::quoted(fs::PathToString(path_addr)));
         DumpPeerAddresses(args, *addrman);
     } catch (const InvalidAddrManVersionError&) {
@@ -203,7 +203,7 @@ std::optional<bilingual_str> LoadAddrman(const std::vector<bool>& asmap, const A
             return strprintf(_("Failed to rename invalid peers.dat file. Please move or delete it and try again."));
         }
         // Addrman can be in an inconsistent state after failure, reset it
-        addrman = std::make_unique<AddrMan>(asmap, /* deterministic */ false, /* consistency_check_ratio */ check_addrman);
+        addrman = std::make_unique<AddrMan>(asmap, /*deterministic=*/false, /*consistency_check_ratio=*/check_addrman);
         LogPrintf("Creating new peers.dat because the file version was not compatible (%s). Original backed up to peers.dat.bak\n", fs::quoted(fs::PathToString(path_addr)));
         DumpPeerAddresses(args, *addrman);
     } catch (const std::exception& e) {

src/bench/addrman.cpp

@@ -101,7 +101,7 @@ static void AddrManGetAddr(benchmark::Bench& bench)
     FillAddrMan(addrman);
 
     bench.run([&] {
-        const auto& addresses = addrman.GetAddr(/* max_addresses */ 2500, /* max_pct */ 23, /* network */ std::nullopt);
+        const auto& addresses = addrman.GetAddr(/*max_addresses=*/2500, /*max_pct=*/23, /*network=*/std::nullopt);
         assert(addresses.size() > 0);
     });
 }

src/bench/mempool_stress.cpp

@@ -86,7 +86,7 @@ static void ComplexMemPool(benchmark::Bench& bench)
     if (bench.complexityN() > 1) {
         childTxs = static_cast<int>(bench.complexityN());
     }
-    std::vector<CTransactionRef> ordered_coins = CreateOrderedCoins(det_rand, childTxs, /* min_ancestors */ 1);
+    std::vector<CTransactionRef> ordered_coins = CreateOrderedCoins(det_rand, childTxs, /*min_ancestors=*/1);
     const auto testing_setup = MakeNoLogFileContext<const TestingSetup>(CBaseChainParams::MAIN);
     CTxMemPool pool;
     LOCK2(cs_main, pool.cs);
@@ -103,15 +103,15 @@ static void MempoolCheck(benchmark::Bench& bench)
 {
     FastRandomContext det_rand{true};
     const int childTxs = bench.complexityN() > 1 ? static_cast<int>(bench.complexityN()) : 2000;
-    const std::vector<CTransactionRef> ordered_coins = CreateOrderedCoins(det_rand, childTxs, /* min_ancestors */ 5);
+    const std::vector<CTransactionRef> ordered_coins = CreateOrderedCoins(det_rand, childTxs, /*min_ancestors=*/5);
     const auto testing_setup = MakeNoLogFileContext<const TestingSetup>(CBaseChainParams::MAIN, {"-checkmempool=1"});
     CTxMemPool pool;
     LOCK2(cs_main, pool.cs);
     const CCoinsViewCache& coins_tip = testing_setup.get()->m_node.chainman->ActiveChainstate().CoinsTip();
     for (auto& tx : ordered_coins) AddTx(tx, pool);
 
     bench.run([&]() NO_THREAD_SAFETY_ANALYSIS {
-        pool.check(coins_tip, /* spendheight */ 2);
+        pool.check(coins_tip, /*spendheight=*/2);
     });
 }
 

src/bench/rpc_mempool.cpp

@@ -29,11 +29,11 @@ static void RpcMempool(benchmark::Bench& bench)
         tx.vout[0].scriptPubKey = CScript() << OP_1 << OP_EQUAL;
         tx.vout[0].nValue = i;
         const CTransactionRef tx_r{MakeTransactionRef(tx)};
-        AddTx(tx_r, /* fee */ i, pool);
+        AddTx(tx_r, /*fee=*/i, pool);
     }
 
     bench.run([&] {
-        (void)MempoolToJSON(pool, /*verbose*/ true);
+        (void)MempoolToJSON(pool, /*verbose=*/true);
     });
 }
 

src/bench/wallet_balance.cpp

@@ -52,10 +52,10 @@ static void WalletBalance(benchmark::Bench& bench, const bool set_dirty, const b
     });
 }
 
-static void WalletBalanceDirty(benchmark::Bench& bench) { WalletBalance(bench, /* set_dirty */ true, /* add_mine */ true); }
-static void WalletBalanceClean(benchmark::Bench& bench) { WalletBalance(bench, /* set_dirty */ false, /* add_mine */ true); }
-static void WalletBalanceMine(benchmark::Bench& bench) { WalletBalance(bench, /* set_dirty */ false, /* add_mine */ true); }
-static void WalletBalanceWatch(benchmark::Bench& bench) { WalletBalance(bench, /* set_dirty */ false, /* add_mine */ false); }
+static void WalletBalanceDirty(benchmark::Bench& bench) { WalletBalance(bench, /*set_dirty=*/true, /*add_mine=*/true); }
+static void WalletBalanceClean(benchmark::Bench& bench) { WalletBalance(bench, /*set_dirty=*/false, /*add_mine=*/true); }
+static void WalletBalanceMine(benchmark::Bench& bench) { WalletBalance(bench, /*set_dirty=*/false, /*add_mine=*/true); }
+static void WalletBalanceWatch(benchmark::Bench& bench) { WalletBalance(bench, /*set_dirty=*/false, /*add_mine=*/false); }
 
 BENCHMARK(WalletBalanceDirty);
 BENCHMARK(WalletBalanceClean);

src/bitcoin-chainstate.cpp

@@ -192,7 +192,7 @@ int main(int argc, char* argv[])
         bool new_block;
         auto sc = std::make_shared<submitblock_StateCatcher>(block.GetHash());
         RegisterSharedValidationInterface(sc);
-        bool accepted = chainman.ProcessNewBlock(chainparams, blockptr, /* force_processing */ true, /* new_block */ &new_block);
+        bool accepted = chainman.ProcessNewBlock(chainparams, blockptr, /*force_processing=*/true, /*new_block=*/&new_block);
         UnregisterSharedValidationInterface(sc);
         if (!new_block && accepted) {
             std::cerr << "duplicate" << std::endl;

src/net.cpp

@@ -2782,7 +2782,7 @@ std::vector<CAddress> CConnman::GetAddresses(CNode& requestor, size_t max_addres
     auto r = m_addr_response_caches.emplace(cache_id, CachedAddrResponse{});
     CachedAddrResponse& cache_entry = r.first->second;
     if (cache_entry.m_cache_entry_expiration < current_time) { // If emplace() added new one it has expiration 0.
-        cache_entry.m_addrs_response_cache = GetAddresses(max_addresses, max_pct, /* network */ std::nullopt);
+        cache_entry.m_addrs_response_cache = GetAddresses(max_addresses, max_pct, /*network=*/std::nullopt);
         // Choosing a proper cache lifetime is a trade-off between the privacy leak minimization
         // and the usefulness of ADDR responses to honest users.
         //

src/net_processing.cpp

@@ -3562,7 +3562,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
         BlockValidationState state;
         if (!m_chainman.ProcessNewBlockHeaders({cmpctblock.header}, state, m_chainparams, &pindex)) {
             if (state.IsInvalid()) {
-                MaybePunishNodeForBlock(pfrom.GetId(), state, /*via_compact_block*/ true, "invalid header via cmpctblock");
+                MaybePunishNodeForBlock(pfrom.GetId(), state, /*via_compact_block=*/true, "invalid header via cmpctblock");
                 return;
             }
         }
@@ -3902,7 +3902,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
         peer->m_addrs_to_send.clear();
         std::vector<CAddress> vAddr;
         if (pfrom.HasPermission(NetPermissionFlags::Addr)) {
-            vAddr = m_connman.GetAddresses(MAX_ADDR_TO_SEND, MAX_PCT_ADDR_TO_SEND, /* network */ std::nullopt);
+            vAddr = m_connman.GetAddresses(MAX_ADDR_TO_SEND, MAX_PCT_ADDR_TO_SEND, /*network=*/std::nullopt);
         } else {
             vAddr = m_connman.GetAddresses(pfrom, MAX_ADDR_TO_SEND, MAX_PCT_ADDR_TO_SEND);
         }

src/node/interfaces.cpp

@@ -590,7 +590,7 @@ class ChainImpl : public Chain
         bool relay,
         std::string& err_string) override
     {
-        const TransactionError err = BroadcastTransaction(m_node, tx, err_string, max_tx_fee, relay, /*wait_callback*/ false);
+        const TransactionError err = BroadcastTransaction(m_node, tx, err_string, max_tx_fee, relay, /*wait_callback=*/false);
         // Chain clients only care about failures to accept the tx to the mempool. Disregard non-mempool related failures.
         // Note: this will need to be updated if BroadcastTransactions() is updated to return other non-mempool failures
         // that Chain clients do not need to know about.

src/qt/bitcoingui.cpp

@@ -853,7 +853,7 @@ void BitcoinGUI::aboutClicked()
     if(!clientModel)
         return;
 
-    auto dlg = new HelpMessageDialog(this, /* about */ true);
+    auto dlg = new HelpMessageDialog(this, /*about=*/true);
     GUIUtil::ShowModalDialogAsynchronously(dlg);
 }