From 931684b24a89aba884cb18c13fa67ccca339ee8c Mon Sep 17 00:00:00 2001 From: James O'Beirne Date: Wed, 14 Apr 2021 13:29:27 -0400 Subject: validation: fix ActivateSnapshot to use hardcoded nChainTx 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. --- src/node/utxo_snapshot.h | 2 ++ src/test/validation_chainstatemanager_tests.cpp | 20 +++++++++++++++++++- src/validation.cpp | 2 +- 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/node/utxo_snapshot.h b/src/node/utxo_snapshot.h index fe78cb46bd..4767d49889 100644 --- a/src/node/utxo_snapshot.h +++ b/src/node/utxo_snapshot.h @@ -24,6 +24,8 @@ public: //! Necessary to "fake" the base nChainTx so that we can estimate progress during //! initial block download for the assumeutxo chainstate. + //! + //! TODO: this is unused and should be removed. unsigned int m_nchaintx = 0; SnapshotMetadata() { } diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index ab31662f97..15fc9b7cbd 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/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; @@ -265,7 +266,16 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_activate_snapshot, TestChain100Setup) metadata.m_base_blockhash = uint256::ONE; })); - BOOST_REQUIRE(CreateAndActivateUTXOSnapshot(m_node, m_path_root)); + constexpr int bad_nchaintx = 9999; + + BOOST_REQUIRE(CreateAndActivateUTXOSnapshot( + m_node, m_path_root, [](CAutoFile& auto_infile, SnapshotMetadata& metadata) { + // Provide an nChainTx that differs from the hardcoded one. + // + // Ultimately this malleation check should be removed when we remove + // the now-unnecessary nChainTx from the user-specified snapshot metadata. + metadata.m_nchaintx = bad_nchaintx; + })); // Ensure our active chain is the snapshot chainstate. BOOST_CHECK(!chainman.ActiveChainstate().m_from_snapshot_blockhash.IsNull()); @@ -273,6 +283,14 @@ 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(); + + // Ensure that, despite a bad nChainTx value being in the snapshot, activation + // uses the hardcoded value from chainparams. + BOOST_CHECK_EQUAL(tip->nChainTx, au_data.nChainTx); + BOOST_CHECK(tip->nChainTx != bad_nchaintx); + // To be checked against later when we try loading a subsequent snapshot. uint256 loaded_snapshot_blockhash{*chainman.SnapshotBlockhash()}; diff --git a/src/validation.cpp b/src/validation.cpp index 332cb581b8..a105624510 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -5346,7 +5346,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", -- cgit v1.2.3 From 91d93aac4e3fe6fff5ef492ed152c4d8fa6f2672 Mon Sep 17 00:00:00 2001 From: James O'Beirne Date: Fri, 23 Apr 2021 13:29:53 -0400 Subject: validation: remove nchaintx from assumeutxo metadata This value is no longer used and is instead specified statically in chainparams. This change means that previously generated snapshots will no longer be usable. --- src/node/utxo_snapshot.h | 11 ++--------- src/test/validation_chainstatemanager_tests.cpp | 14 +------------- test/functional/rpc_dumptxoutset.py | 2 +- 3 files changed, 4 insertions(+), 23 deletions(-) diff --git a/src/node/utxo_snapshot.h b/src/node/utxo_snapshot.h index 4767d49889..61292cdcc5 100644 --- a/src/node/utxo_snapshot.h +++ b/src/node/utxo_snapshot.h @@ -22,22 +22,15 @@ public: //! 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. - //! - //! TODO: this is unused and should be removed. - 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 diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index 15fc9b7cbd..82e70b5cdc 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -266,16 +266,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_activate_snapshot, TestChain100Setup) metadata.m_base_blockhash = uint256::ONE; })); - constexpr int bad_nchaintx = 9999; - - BOOST_REQUIRE(CreateAndActivateUTXOSnapshot( - m_node, m_path_root, [](CAutoFile& auto_infile, SnapshotMetadata& metadata) { - // Provide an nChainTx that differs from the hardcoded one. - // - // Ultimately this malleation check should be removed when we remove - // the now-unnecessary nChainTx from the user-specified snapshot metadata. - metadata.m_nchaintx = bad_nchaintx; - })); + BOOST_REQUIRE(CreateAndActivateUTXOSnapshot(m_node, m_path_root)); // Ensure our active chain is the snapshot chainstate. BOOST_CHECK(!chainman.ActiveChainstate().m_from_snapshot_blockhash.IsNull()); @@ -286,10 +277,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_activate_snapshot, TestChain100Setup) const AssumeutxoData& au_data = *ExpectedAssumeutxo(snapshot_height, ::Params()); const CBlockIndex* tip = chainman.ActiveTip(); - // Ensure that, despite a bad nChainTx value being in the snapshot, activation - // uses the hardcoded value from chainparams. BOOST_CHECK_EQUAL(tip->nChainTx, au_data.nChainTx); - BOOST_CHECK(tip->nChainTx != bad_nchaintx); // To be checked against later when we try loading a subsequent snapshot. uint256 loaded_snapshot_blockhash{*chainman.SnapshotBlockhash()}; diff --git a/test/functional/rpc_dumptxoutset.py b/test/functional/rpc_dumptxoutset.py index e65787ce08..dc469ba552 100755 --- a/test/functional/rpc_dumptxoutset.py +++ b/test/functional/rpc_dumptxoutset.py @@ -41,7 +41,7 @@ class DumptxoutsetTest(BitcoinTestFramework): 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( -- cgit v1.2.3