Merge bitcoin/bitcoin#28958: refactor: Use Txid in CMerkleBlock

fa02c08 refactor: Use Txid in CMerkleBlock (MarcoFalke)

Pull request description:

  This should also fix a gcc-13 compiler warning, see bitcoin/bitcoin#28922 (comment)

  ```
  rpc/txoutproof.cpp: In lambda function:
  rpc/txoutproof.cpp:72:33: error: possibly dangling reference to a temporary [-Werror=dangling-reference]
     72 |                     const Coin& coin = AccessByTxid(active_chainstate.CoinsTip(), Txid::FromUint256(tx));
        |                                 ^~~~
  rpc/txoutproof.cpp:72:52: note: the temporary was destroyed at the end of the full expression ‘AccessByTxid((*(const CCoinsViewCache*)(&(& active_chainstate)->Chainstate::CoinsTip())), transaction_identifier<false>::FromUint256((* & tx)))’
     72 |                     const Coin& coin = AccessByTxid(active_chainstate.CoinsTip(), Txid::FromUint256(tx));
        |                                        ~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  cc1plus: all warnings being treated as errors

ACKs for top commit:
  TheCharlatan:
    Re-ACK fa02c08
  dergoegge:
    reACK fa02c08

Tree-SHA512: 2e6837b9d0c90bd6e9d766330e7086d68c6ec80bb27fe2cfc4702b251b00d91a79f8bfbc76d998cbcd90bee5317402cf617f61099eee96d94e7ac8f37ba7a642

src/merkleblock.cpp

@@ -27,7 +27,7 @@ std::vector<bool> BytesToBits(const std::vector<unsigned char>& bytes)
     return ret;
 }
 
-CMerkleBlock::CMerkleBlock(const CBlock& block, CBloomFilter* filter, const std::set<uint256>* txids)
+CMerkleBlock::CMerkleBlock(const CBlock& block, CBloomFilter* filter, const std::set<Txid>* txids)
 {
     header = block.GetBlockHeader();
 
@@ -39,7 +39,7 @@ CMerkleBlock::CMerkleBlock(const CBlock& block, CBloomFilter* filter, const std:
 
     for (unsigned int i = 0; i < block.vtx.size(); i++)
     {
-        const uint256& hash = block.vtx[i]->GetHash();
+        const Txid& hash{block.vtx[i]->GetHash()};
         if (txids && txids->count(hash)) {
             vMatch.push_back(true);
         } else if (filter && filter->IsRelevantAndUpdate(*block.vtx[i])) {

src/merkleblock.h

@@ -11,6 +11,7 @@
 #include <serialize.h>
 #include <uint256.h>
 
+#include <set>
 #include <vector>
 
 // Helper functions for serialization.
@@ -144,15 +145,15 @@ class CMerkleBlock
     CMerkleBlock(const CBlock& block, CBloomFilter& filter) : CMerkleBlock(block, &filter, nullptr) { }
 
     // Create from a CBlock, matching the txids in the set
-    CMerkleBlock(const CBlock& block, const std::set<uint256>& txids) : CMerkleBlock(block, nullptr, &txids) { }
+    CMerkleBlock(const CBlock& block, const std::set<Txid>& txids) : CMerkleBlock{block, nullptr, &txids} {}
 
     CMerkleBlock() {}
 
     SERIALIZE_METHODS(CMerkleBlock, obj) { READWRITE(obj.header, obj.txn); }
 
 private:
     // Combined constructor to consolidate code
-    CMerkleBlock(const CBlock& block, CBloomFilter* filter, const std::set<uint256>* txids);
+    CMerkleBlock(const CBlock& block, CBloomFilter* filter, const std::set<Txid>* txids);
 };
 
 #endif // BITCOIN_MERKLEBLOCK_H

src/rpc/txoutproof.cpp

@@ -41,13 +41,13 @@ static RPCHelpMan gettxoutproof()
         RPCExamples{""},
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
         {
-            std::set<uint256> setTxids;
+            std::set<Txid> setTxids;
             UniValue txids = request.params[0].get_array();
             if (txids.empty()) {
                 throw JSONRPCError(RPC_INVALID_PARAMETER, "Parameter 'txids' cannot be empty");
             }
             for (unsigned int idx = 0; idx < txids.size(); idx++) {
-                auto ret = setTxids.insert(ParseHashV(txids[idx], "txid"));
+                auto ret{setTxids.insert(Txid::FromUint256(ParseHashV(txids[idx], "txid")))};
                 if (!ret.second) {
                     throw JSONRPCError(RPC_INVALID_PARAMETER, std::string("Invalid parameter, duplicated txid: ") + txids[idx].get_str());
                 }
@@ -69,7 +69,7 @@ static RPCHelpMan gettxoutproof()
 
                 // Loop through txids and try to find which block they're in. Exit loop once a block is found.
                 for (const auto& tx : setTxids) {
-                    const Coin& coin = AccessByTxid(active_chainstate.CoinsTip(), Txid::FromUint256(tx));
+                    const Coin& coin{AccessByTxid(active_chainstate.CoinsTip(), tx)};
                     if (!coin.IsSpent()) {
                         pblockindex = active_chainstate.m_chain[coin.nHeight];
                         break;

src/test/fuzz/merkleblock.cpp

@@ -29,13 +29,13 @@ FUZZ_TARGET(merkleblock)
             CMerkleBlock merkle_block;
             const std::optional<CBlock> opt_block = ConsumeDeserializable<CBlock>(fuzzed_data_provider, TX_WITH_WITNESS);
             CBloomFilter bloom_filter;
-            std::set<uint256> txids;
+            std::set<Txid> txids;
             if (opt_block && !opt_block->vtx.empty()) {
                 if (fuzzed_data_provider.ConsumeBool()) {
                     merkle_block = CMerkleBlock{*opt_block, bloom_filter};
                 } else if (fuzzed_data_provider.ConsumeBool()) {
                     LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 10000) {
-                        txids.insert(ConsumeUInt256(fuzzed_data_provider));
+                        txids.insert(Txid::FromUint256(ConsumeUInt256(fuzzed_data_provider)));
                     }
                     merkle_block = CMerkleBlock{*opt_block, txids};
                 }

src/test/merkleblock_tests.cpp

@@ -21,13 +21,13 @@ BOOST_AUTO_TEST_CASE(merkleblock_construct_from_txids_found)
 {
     CBlock block = getBlock13b8a();
 
-    std::set<uint256> txids;
+    std::set<Txid> txids;
 
     // Last txn in block.
-    uint256 txhash1 = uint256S("0x74d681e0e03bafa802c8aa084379aa98d9fcd632ddc2ed9782b586ec87451f20");
+    Txid txhash1{TxidFromString("0x74d681e0e03bafa802c8aa084379aa98d9fcd632ddc2ed9782b586ec87451f20")};
 
     // Second txn in block.
-    uint256 txhash2 = uint256S("0xf9fc751cb7dc372406a9f8d738d5e6f8f63bab71986a39cf36ee70ee17036d07");
+    Txid txhash2{TxidFromString("0xf9fc751cb7dc372406a9f8d738d5e6f8f63bab71986a39cf36ee70ee17036d07")};
 
     txids.insert(txhash1);
     txids.insert(txhash2);
@@ -62,8 +62,8 @@ BOOST_AUTO_TEST_CASE(merkleblock_construct_from_txids_not_found)
 {
     CBlock block = getBlock13b8a();
 
-    std::set<uint256> txids2;
-    txids2.insert(uint256S("0xc0ffee00003bafa802c8aa084379aa98d9fcd632ddc2ed9782b586ec87451f20"));
+    std::set<Txid> txids2;
+    txids2.insert(TxidFromString("0xc0ffee00003bafa802c8aa084379aa98d9fcd632ddc2ed9782b586ec87451f20"));
     CMerkleBlock merkleBlock(block, txids2);
 
     BOOST_CHECK_EQUAL(merkleBlock.header.GetHash().GetHex(), block.GetHash().GetHex());