Merge bitcoin/bitcoin#21681: validation: fix ActivateSnapshot to use hardcoded nChainTx

91d93aa validation: remove nchaintx from assumeutxo metadata (James O'Beirne)
931684b validation: fix ActivateSnapshot to use hardcoded nChainTx (James O'Beirne)

Pull request description:

  This fixes an oversight from the move of nChainTx from the user-supplied
  snapshot metadata into the hardcoded assumeutxo chainparams.

  Since the nChainTx is now unused in the metadata, it should be removed
  in a future commit.

  See: bitcoin/bitcoin#19806 (comment)

ACKs for top commit:
  Sjors:
    utACK 91d93aa
  ryanofsky:
    Code review ACK 91d93aa. No change to previous commit, just new commit removing now unused utxo snapshot field and updating tests.

Tree-SHA512: 445bdd738faf007451f40bbcf360dd1fb4675e17a4c96546e6818c12e33dd336dadd95cf8d4b5f8df1d6ccfbc4bf5496864bb5528e416cea894857b6b732140c

Author: MarcoFalke

src/node/utxo_snapshot.h

@@ -22,20 +22,15 @@ class SnapshotMetadata
     //! during snapshot load to estimate progress of UTXO set reconstruction.
     uint64_t m_coins_count = 0;
 
-    //! Necessary to "fake" the base nChainTx so that we can estimate progress during
-    //! initial block download for the assumeutxo chainstate.
-    unsigned int m_nchaintx = 0;
-
     SnapshotMetadata() { }
     SnapshotMetadata(
         const uint256& base_blockhash,
         uint64_t coins_count,
         unsigned int nchaintx) :
             m_base_blockhash(base_blockhash),
-            m_coins_count(coins_count),
-            m_nchaintx(nchaintx) { }
+            m_coins_count(coins_count) { }
 
-    SERIALIZE_METHODS(SnapshotMetadata, obj) { READWRITE(obj.m_base_blockhash, obj.m_coins_count, obj.m_nchaintx); }
+    SERIALIZE_METHODS(SnapshotMetadata, obj) { READWRITE(obj.m_base_blockhash, obj.m_coins_count); }
 };
 
 #endif // BITCOIN_NODE_UTXO_SNAPSHOT_H

src/test/validation_chainstatemanager_tests.cpp

@@ -233,6 +233,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_activate_snapshot, TestChain100Setup)
 
     // Mine 10 more blocks, putting at us height 110 where a valid assumeutxo value can
     // be found.
+    constexpr int snapshot_height = 110;
     mineBlocks(10);
     initial_size += 10;
     initial_total_coins += 10;
@@ -273,6 +274,11 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_activate_snapshot, TestChain100Setup)
         chainman.ActiveChainstate().m_from_snapshot_blockhash,
         *chainman.SnapshotBlockhash());
 
+    const AssumeutxoData& au_data = *ExpectedAssumeutxo(snapshot_height, ::Params());
+    const CBlockIndex* tip = chainman.ActiveTip();
+
+    BOOST_CHECK_EQUAL(tip->nChainTx, au_data.nChainTx);
+
     // To be checked against later when we try loading a subsequent snapshot.
     uint256 loaded_snapshot_blockhash{*chainman.SnapshotBlockhash()};
 

src/validation.cpp

@@ -4975,7 +4975,7 @@ bool ChainstateManager::PopulateAndValidateSnapshot(
     }
 
     assert(index);
-    index->nChainTx = metadata.m_nchaintx;
+    index->nChainTx = au_data.nChainTx;
     snapshot_chainstate.setBlockIndexCandidates.insert(snapshot_start_block);
 
     LogPrintf("[snapshot] validated snapshot (%.2f MB)\n",

test/functional/rpc_dumptxoutset.py

@@ -41,7 +41,7 @@ def run_test(self):
             digest = hashlib.sha256(f.read()).hexdigest()
             # UTXO snapshot hash should be deterministic based on mocked time.
             assert_equal(
-                digest, 'be032e5f248264ba08e11099ac09dbd001f6f87ffc68bf0f87043d8146d50664')
+                digest, '7ae82c986fa5445678d2a21453bb1c86d39e47af13da137640c2b1cf8093691c')
 
         # Specifying a path to an existing file will fail.
         assert_raises_rpc_error(