Merge bitcoin/bitcoin#34095: refactor: enable readability-container-contains clang-tidy rule

1e94e562f7 refactor: enable `readability-container-contains` clang-tidy rule (Lőrinc)
fd9f1accbd Fix compilation for old Boost versions (Lőrinc)

Pull request description:

  Replace the last few instances of `.count() != 0` and `.count() == 0` and bare `count()` patterns with the more expressive C++20 `.contains()` method:

  * `std::set<std::string>` in `getblocktemplate` RPC;
  * `std::map<std::string, ...>` in `transaction_tests`;
  * other bare `std::unordered_set` and `std::map` count calls.

  Also fixes https://github.com/bitcoin/bitcoin/issues/34101 by reverting `boost::multi_index::contains` calls not available in our minimum supported version.

  With no remaining violations, enable the `readability-container-contains` clang-tidy check to prevent future regressions.

  Follow-up to https://github.com/bitcoin/bitcoin/pull/33192

ACKs for top commit:
  hebasto:
    ACK 1e94e562f7.
  pablomartin4btc:
    re-ACK 1e94e562f7
  janb84:
    ACK 1e94e562f7
  rkrux:
    re-ACK 1e94e562f7

Tree-SHA512: d54a7821d319bf0d60b6c3a870917464a7d5b9279c6a86708c03a3516ec23bbf18f0e83de62b3b2b1607de96e1470f1144b4918d69a6c770e6b7e09863e7dbac

src/.clang-tidy

@@ -25,6 +25,7 @@ performance-*,
 -performance-noexcept-move-constructor,
 -performance-unnecessary-value-param,
 readability-const-return-type,
+readability-container-contains,
 readability-redundant-declaration,
 readability-redundant-string-init,
 '

src/qt/transactiondesc.cpp

@@ -124,7 +124,7 @@ QString TransactionDesc::toHTML(interfaces::Node& node, interfaces::Wallet& wall
     {
         strHTML += "<b>" + tr("Source") + ":</b> " + tr("Generated") + "<br>";
     }
-    else if (wtx.value_map.count("from") && !wtx.value_map["from"].empty())
+    else if (wtx.value_map.contains("from") && !wtx.value_map["from"].empty())
     {
         // Online transaction
         strHTML += "<b>" + tr("From") + ":</b> " + GUIUtil::HtmlEscape(wtx.value_map["from"]) + "<br>";
@@ -157,7 +157,7 @@ QString TransactionDesc::toHTML(interfaces::Node& node, interfaces::Wallet& wall
     //
     // To
     //
-    if (wtx.value_map.count("to") && !wtx.value_map["to"].empty())
+    if (wtx.value_map.contains("to") && !wtx.value_map["to"].empty())
     {
         // Online transaction
         std::string strAddress = wtx.value_map["to"];
@@ -212,7 +212,7 @@ QString TransactionDesc::toHTML(interfaces::Node& node, interfaces::Wallet& wall
                 if (toSelf && all_from_me)
                     continue;
 
-                if (!wtx.value_map.count("to") || wtx.value_map["to"].empty())
+                if (!wtx.value_map.contains("to") || wtx.value_map["to"].empty())
                 {
                     // Offline transaction
                     CTxDestination address;
@@ -273,9 +273,9 @@ QString TransactionDesc::toHTML(interfaces::Node& node, interfaces::Wallet& wall
     //
     // Message
     //
-    if (wtx.value_map.count("message") && !wtx.value_map["message"].empty())
+    if (wtx.value_map.contains("message") && !wtx.value_map["message"].empty())
         strHTML += "<br><b>" + tr("Message") + ":</b><br>" + GUIUtil::HtmlEscape(wtx.value_map["message"], true) + "<br>";
-    if (wtx.value_map.count("comment") && !wtx.value_map["comment"].empty())
+    if (wtx.value_map.contains("comment") && !wtx.value_map["comment"].empty())
         strHTML += "<br><b>" + tr("Comment") + ":</b><br>" + GUIUtil::HtmlEscape(wtx.value_map["comment"], true) + "<br>";
 
     strHTML += "<b>" + tr("Transaction ID") + ":</b> " + rec->getTxHash() + "<br>";

src/rpc/mining.cpp

@@ -846,12 +846,12 @@ static RPCHelpMan getblocktemplate()
     const Consensus::Params& consensusParams = chainman.GetParams().GetConsensus();
 
     // GBT must be called with 'signet' set in the rules for signet chains
-    if (consensusParams.signet_blocks && setClientRules.count("signet") != 1) {
+    if (consensusParams.signet_blocks && !setClientRules.contains("signet")) {
         throw JSONRPCError(RPC_INVALID_PARAMETER, "getblocktemplate must be called with the signet rule set (call with {\"rules\": [\"segwit\", \"signet\"]})");
     }
 
     // GBT must be called with 'segwit' set in the rules
-    if (setClientRules.count("segwit") != 1) {
+    if (!setClientRules.contains("segwit")) {
         throw JSONRPCError(RPC_INVALID_PARAMETER, "getblocktemplate must be called with the segwit rule set (call with {\"rules\": [\"segwit\"]})");
     }
 

src/test/transaction_tests.cpp

@@ -60,7 +60,7 @@ script_verify_flags ParseScriptFlags(std::string strFlags)
     std::vector<std::string> words = SplitString(strFlags, ',');
     for (const std::string& word : words)
     {
-        if (!mapFlagNames.count(word)) {
+        if (!mapFlagNames.contains(word)) {
             BOOST_ERROR("Bad test: unknown verification flag '" << word << "'");
             continue;
         }
@@ -90,7 +90,7 @@ bool CheckTxScripts(const CTransaction& tx, const std::map<COutPoint, CScript>&
     ScriptError err = expect_valid ? SCRIPT_ERR_UNKNOWN_ERROR : SCRIPT_ERR_OK;
     for (unsigned int i = 0; i < tx.vin.size() && tx_valid; ++i) {
         const CTxIn input = tx.vin[i];
-        const CAmount amount = map_prevout_values.count(input.prevout) ? map_prevout_values.at(input.prevout) : 0;
+        const CAmount amount = map_prevout_values.contains(input.prevout) ? map_prevout_values.at(input.prevout) : 0;
         try {
             tx_valid = VerifyScript(input.scriptSig, map_prevout_scriptPubKeys.at(input.prevout),
                 &input.scriptWitness, flags, TransactionSignatureChecker(&tx, i, amount, txdata, MissingDataBehavior::ASSERT_FAIL), &err);

src/txmempool.cpp

@@ -988,7 +988,7 @@ util::Result<std::pair<std::vector<FeeFrac>, std::vector<FeeFrac>>> CTxMemPool::
 CTxMemPool::ChangeSet::TxHandle CTxMemPool::ChangeSet::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)
 {
     LOCK(m_pool->cs);
-    Assume(!m_to_add.contains(tx->GetHash()));
+    Assume(m_to_add.find(tx->GetHash()) == m_to_add.end());
     Assume(!m_dependencies_processed);
 
     // We need to process dependencies after adding a new transaction.

src/txrequest.cpp

@@ -580,7 +580,7 @@ public:
         // Bail out if we already have a CANDIDATE_BEST announcement for this (txhash, peer) combination. The case
         // where there is a non-CANDIDATE_BEST announcement already will be caught by the uniqueness property of the
         // ByPeer index when we try to emplace the new object below.
-        if (m_index.get<ByPeer>().contains(ByPeerView{peer, true, gtxid.ToUint256()})) return;
+        if (m_index.get<ByPeer>().count(ByPeerView{peer, true, gtxid.ToUint256()})) return;
 
         // Try creating the announcement with CANDIDATE_DELAYED state (which will fail due to the uniqueness
         // of the ByPeer index if a non-CANDIDATE_BEST announcement already exists with the same txhash and peer).

src/wallet/test/fuzz/scriptpubkeyman.cpp

@@ -125,7 +125,7 @@ FUZZ_TARGET(scriptpubkeyman, .init = initialize_spkm)
             [&] {
                 const CScript script{ConsumeScript(fuzzed_data_provider)};
                 if (spk_manager->IsMine(script)) {
-                    assert(spk_manager->GetScriptPubKeys().count(script));
+                    assert(spk_manager->GetScriptPubKeys().contains(script));
                 }
             },
             [&] {