From faa90f6e7b6b8c531e1ae142a5c2f568b48502a9 Mon Sep 17 00:00:00 2001
From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
Date: Thu, 12 Oct 2023 11:11:48 +0200
Subject: [PATCH 1/2] refactor: Remove unused nchaintx from SnapshotMetadata
 constructor

Also, remove wrong nChainTx comment and cast.
---
 src/node/utxo_snapshot.h | 3 +--
 src/rpc/blockchain.cpp   | 6 ++----
 2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/src/node/utxo_snapshot.h b/src/node/utxo_snapshot.h
index a6dd3f3f13..1160bb55f0 100644
--- a/src/node/utxo_snapshot.h
+++ b/src/node/utxo_snapshot.h
@@ -35,8 +35,7 @@ public:
     SnapshotMetadata() { }
     SnapshotMetadata(
         const uint256& base_blockhash,
-        uint64_t coins_count,
-        unsigned int nchaintx) :
+        uint64_t coins_count) :
             m_base_blockhash(base_blockhash),
             m_coins_count(coins_count) { }
 
diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp
index 31dd8164fe..37a28e414a 100644
--- a/src/rpc/blockchain.cpp
+++ b/src/rpc/blockchain.cpp
@@ -2667,7 +2667,7 @@ UniValue CreateUTXOSnapshot(
         tip->nHeight, tip->GetBlockHash().ToString(),
         fs::PathToString(path), fs::PathToString(temppath)));
 
-    SnapshotMetadata metadata{tip->GetBlockHash(), maybe_stats->coins_count, tip->nChainTx};
+    SnapshotMetadata metadata{tip->GetBlockHash(), maybe_stats->coins_count};
 
     afile << metadata;
 
@@ -2694,9 +2694,7 @@ UniValue CreateUTXOSnapshot(
     result.pushKV("base_height", tip->nHeight);
     result.pushKV("path", path.u8string());
     result.pushKV("txoutset_hash", maybe_stats->hashSerialized.ToString());
-    // Cast required because univalue doesn't have serialization specified for
-    // `unsigned int`, nChainTx's type.
-    result.pushKV("nchaintx", uint64_t{tip->nChainTx});
+    result.pushKV("nchaintx", tip->nChainTx);
     return result;
 }
 

From fafde92f84fb7c245bc3c1cd946a32c891861e5e Mon Sep 17 00:00:00 2001
From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
Date: Thu, 12 Oct 2023 11:21:08 +0200
Subject: [PATCH 2/2] test: Check snapshot file with wrong number of coins

Also, fix a bug in an assert_debug_log call.
---
 test/functional/feature_assumeutxo.py | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/test/functional/feature_assumeutxo.py b/test/functional/feature_assumeutxo.py
index ea93b7ab88..c900736ae9 100755
--- a/test/functional/feature_assumeutxo.py
+++ b/test/functional/feature_assumeutxo.py
@@ -40,7 +40,7 @@ from test_framework.util import (
     assert_equal,
     assert_raises_rpc_error,
 )
-
+import struct
 
 START_HEIGHT = 199
 SNAPSHOT_BASE_HEIGHT = 299
@@ -68,23 +68,35 @@ class AssumeutxoTest(BitcoinTestFramework):
 
     def test_invalid_snapshot_scenarios(self, valid_snapshot_path):
         self.log.info("Test different scenarios of loading invalid snapshot files")
-        self.log.info("  - snapshot file refering to a block that is not in the assumeutxo parameters")
         with open(valid_snapshot_path, 'rb') as f:
             valid_snapshot_contents = f.read()
+        bad_snapshot_path = valid_snapshot_path + '.mod'
 
+        self.log.info("  - snapshot file refering to a block that is not in the assumeutxo parameters")
         # we can only test this with a block that is already known, as otherwise the `loadtxoutset` RPC
         # would time out (waiting to see the hash in the headers chain), rather than error immediately
         bad_snapshot_height = SNAPSHOT_BASE_HEIGHT - 1
-        bad_snapshot_path = valid_snapshot_path + '.mod'
         with open(bad_snapshot_path, 'wb') as f:
             bad_snapshot_block_hash = self.nodes[0].getblockhash(bad_snapshot_height)
             # block hash of the snapshot base is stored right at the start (first 32 bytes)
             f.write(bytes.fromhex(bad_snapshot_block_hash)[::-1] + valid_snapshot_contents[32:])
 
         expected_log = f"assumeutxo height in snapshot metadata not recognized ({bad_snapshot_height}) - refusing to load snapshot"
-        with self.nodes[1].assert_debug_log(expected_log):
+        with self.nodes[1].assert_debug_log([expected_log]):
             assert_raises_rpc_error(-32603, "Unable to load UTXO snapshot", self.nodes[1].loadtxoutset, bad_snapshot_path)
 
+        self.log.info("  - snapshot file with wrong number of coins")
+        valid_num_coins = struct.unpack("<I", valid_snapshot_contents[32:32 + 4])[0]
+        for off in [-1, +1]:
+            with open(bad_snapshot_path, 'wb') as f:
+                f.write(valid_snapshot_contents[:32])
+                f.write(struct.pack("<I", valid_num_coins + off))
+                f.write(valid_snapshot_contents[32 + 4:])
+
+            expected_log = f"bad snapshot - coins left over after deserializing 298 coins" if off == -1 else f"bad snapshot format or truncated snapshot after deserializing 299 coins"
+            with self.nodes[1].assert_debug_log([expected_log]):
+                assert_raises_rpc_error(-32603, "Unable to load UTXO snapshot", self.nodes[1].loadtxoutset, bad_snapshot_path)
+
     def run_test(self):
         """
         Bring up two (disconnected) nodes, mine some new blocks on the first,