Merge bitcoin/bitcoin#30167: doc, rpc: Release notes and follow-ups for #29612

efc1b5be8a test: Add coverage for txid coins count check when loading snapshot (Fabian Jahr)
6b6084850b assumeutxo: Add network magic ctor param to SnapshotMetadata (Fabian Jahr)
1f1f998455 assumeutxo: Deserialize trailing byte instead of Txid (Fabian Jahr)
359967e310 doc: Add release notes for #29612 (Fabian Jahr)

Pull request description:

  This adds release notes for #29612 and addresses post-merge review comments.

ACKs for top commit:
  maflcko:
    utACK efc1b5be8a
  theStack:
    utACK efc1b5be8a

Tree-SHA512: 3b270202e4f7b2576090ef1d970fd54a6840d96fc3621dddd28e888fb8696a97ff69af2e000bcee3b364316ca3f6e2a9b2f1694c6184f0e704dc487823127ce4

doc/release-notes-29612.md

@@ -0,0 +1,8 @@
+RPC
+---
+
+- The `dumptxoutset` RPC now returns the UTXO set dump in a new and
+  improved format. At the same time the `loadtxoutset` RPC now
+  expects this new format in dumps it tries to load. Dumps with the
+  old format are no longer supported and need to be recreated using
+  the new format in order to be usable.

src/kernel/chainparams.cpp

@@ -554,7 +554,7 @@ std::vector<int> CChainParams::GetAvailableSnapshotHeights() const
     return heights;
 }
 
-std::optional<ChainType> GetNetworkForMagic(MessageStartChars& message)
+std::optional<ChainType> GetNetworkForMagic(const MessageStartChars& message)
 {
     const auto mainnet_msg = CChainParams::Main()->MessageStart();
     const auto testnet_msg = CChainParams::TestNet()->MessageStart();

src/kernel/chainparams.h

@@ -184,6 +184,6 @@ protected:
     ChainTxData chainTxData;
 };
 
-std::optional<ChainType> GetNetworkForMagic(MessageStartChars& pchMessageStart);
+std::optional<ChainType> GetNetworkForMagic(const MessageStartChars& pchMessageStart);
 
 #endif // BITCOIN_KERNEL_CHAINPARAMS_H

src/node/utxo_snapshot.h

@@ -13,6 +13,7 @@
 #include <sync.h>
 #include <uint256.h>
 #include <util/chaintype.h>
+#include <util/check.h>
 #include <util/fs.h>
 
 #include <cstdint>
@@ -31,6 +32,7 @@ class SnapshotMetadata
 {
     const uint16_t m_version{1};
     const std::set<uint16_t> m_supported_versions{1};
+    const MessageStartChars m_network_magic;
 public:
     //! The hash of the block that reflects the tip of the chain for the
     //! UTXO set contained in this snapshot.
@@ -42,11 +44,15 @@ public:
     //! during snapshot load to estimate progress of UTXO set reconstruction.
     uint64_t m_coins_count = 0;
 
-    SnapshotMetadata() { }
     SnapshotMetadata(
+        const MessageStartChars network_magic) :
+            m_network_magic(network_magic) { }
+    SnapshotMetadata(
+        const MessageStartChars network_magic,
         const uint256& base_blockhash,
         const int base_blockheight,
         uint64_t coins_count) :
+            m_network_magic(network_magic),
             m_base_blockhash(base_blockhash),
             m_base_blockheight(base_blockheight),
             m_coins_count(coins_count) { }
@@ -55,7 +61,7 @@ public:
     inline void Serialize(Stream& s) const {
         s << SNAPSHOT_MAGIC_BYTES;
         s << m_version;
-        s << Params().MessageStart();
+        s << m_network_magic;
         s << m_base_blockheight;
         s << m_base_blockhash;
         s << m_coins_count;
@@ -80,11 +86,13 @@ public:
         // Read the network magic (pchMessageStart)
         MessageStartChars message;
         s >> message;
-        if (!std::equal(message.begin(), message.end(), Params().MessageStart().data())) {
-            auto metadata_network = GetNetworkForMagic(message);
+        if (!std::equal(message.begin(), message.end(), m_network_magic.data())) {
+            auto metadata_network{GetNetworkForMagic(message)};
             if (metadata_network) {
                 std::string network_string{ChainTypeToString(metadata_network.value())};
-                throw std::ios_base::failure(strprintf("The network of the snapshot (%s) does not match the network of this node (%s).", network_string, Params().GetChainTypeString()));
+                auto node_network{GetNetworkForMagic(m_network_magic)};
+                std::string node_network_string{ChainTypeToString(node_network.value())};
+                throw std::ios_base::failure(strprintf("The network of the snapshot (%s) does not match the network of this node (%s).", network_string, node_network_string));
             } else {
                 throw std::ios_base::failure("This snapshot has been created for an unrecognized network. This could be a custom signet, a new testnet or possibly caused by data corruption.");
             }

src/rpc/blockchain.cpp

@@ -2697,7 +2697,7 @@ UniValue CreateUTXOSnapshot(
         tip->nHeight, tip->GetBlockHash().ToString(),
         fs::PathToString(path), fs::PathToString(temppath)));
 
-    SnapshotMetadata metadata{tip->GetBlockHash(), tip->nHeight, maybe_stats->coins_count};
+    SnapshotMetadata metadata{chainstate.m_chainman.GetParams().MessageStart(), tip->GetBlockHash(), tip->nHeight, maybe_stats->coins_count};
 
     afile << metadata;
 
@@ -2809,7 +2809,7 @@ static RPCHelpMan loadtxoutset()
             "Couldn't open file " + path.utf8string() + " for reading.");
     }
 
-    SnapshotMetadata metadata;
+    SnapshotMetadata metadata{chainman.GetParams().MessageStart()};
     try {
         afile >> metadata;
     } catch (const std::ios_base::failure& e) {

src/test/fuzz/deserialize.cpp

@@ -316,7 +316,8 @@ FUZZ_TARGET_DESERIALIZE(blocktransactionsrequest_deserialize, {
     DeserializeFromFuzzingInput(buffer, btr);
 })
 FUZZ_TARGET_DESERIALIZE(snapshotmetadata_deserialize, {
-    SnapshotMetadata snapshot_metadata;
+    auto msg_start = Params().MessageStart();
+    SnapshotMetadata snapshot_metadata{msg_start};
     DeserializeFromFuzzingInput(buffer, snapshot_metadata);
 })
 FUZZ_TARGET_DESERIALIZE(uint160_deserialize, {

src/test/fuzz/utxo_snapshot.cpp

@@ -47,7 +47,8 @@ FUZZ_TARGET(utxo_snapshot, .init = initialize_chain)
 
     const auto ActivateFuzzedSnapshot{[&] {
         AutoFile infile{fsbridge::fopen(snapshot_path, "rb")};
-        SnapshotMetadata metadata;
+        auto msg_start = Params().MessageStart();
+        SnapshotMetadata metadata{msg_start};
         try {
             infile >> metadata;
         } catch (const std::ios_base::failure&) {

src/test/util/chainstate.h

@@ -56,7 +56,7 @@ CreateAndActivateUTXOSnapshot(
     //
     FILE* infile{fsbridge::fopen(snapshot_path, "rb")};
     AutoFile auto_infile{infile};
-    node::SnapshotMetadata metadata;
+    node::SnapshotMetadata metadata{node.chainman->GetParams().MessageStart()};
     auto_infile >> metadata;
 
     malleation(auto_infile, metadata);

src/validation.cpp

@@ -5811,8 +5811,8 @@ bool ChainstateManager::PopulateAndValidateSnapshot(
 
     bool out_of_coins{false};
     try {
-        Txid txid;
-        coins_file >> txid;
+        std::byte left_over_byte;
+        coins_file >> left_over_byte;
     } catch (const std::ios_base::failure&) {
         // We expect an exception since we should be out of coins.
         out_of_coins = true;

test/functional/feature_assumeutxo.py

@@ -130,7 +130,8 @@ class AssumeutxoTest(BitcoinTestFramework):
         cases = [
             # (content, offset, wrong_hash, custom_message)
             [b"\xff" * 32, 0, "7d52155c9a9fdc4525b637ef6170568e5dad6fabd0b1fdbb9432010b8453095b", None],  # wrong outpoint hash
-            [(2).to_bytes(1, "little"), 32, None, "[snapshot] bad snapshot data after deserializing 1 coins"],  # wrong outpoint hash
+            [(2).to_bytes(1, "little"), 32, None, "[snapshot] bad snapshot data after deserializing 1 coins"],  # wrong txid coins count
+            [b"\xfd\xff\xff", 32, None, "[snapshot] mismatch in coins count in snapshot metadata and actual snapshot data"],  # txid coins count exceeds coins left
             [b"\x01", 33, "9f4d897031ab8547665b4153317ae2fdbf0130c7840b66427ebc48b881cb80ad", None],  # wrong outpoint index
             [b"\x81", 34, "3da966ba9826fb6d2604260e01607b55ba44e1a5de298606b08704bc62570ea8", None],  # wrong coin code VARINT
             [b"\x80", 34, "091e893b3ccb4334378709578025356c8bcb0a623f37c7c4e493133c988648e5", None],  # another wrong coin code