diff --git a/src/node/utxo_snapshot.h b/src/node/utxo_snapshot.h index a7c4135787..e4eb6d60ad 100644 --- a/src/node/utxo_snapshot.h +++ b/src/node/utxo_snapshot.h @@ -28,16 +28,17 @@ class Chainstate; namespace node { //! Metadata describing a serialized version of a UTXO set from which an //! assumeutxo Chainstate can be constructed. +//! All metadata fields come from an untrusted file, so must be validated +//! before being used. Thus, new fields should be added only if needed. class SnapshotMetadata { - const uint16_t m_version{1}; - const std::set m_supported_versions{1}; + inline static const uint16_t VERSION{2}; + const std::set m_supported_versions{VERSION}; 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. uint256 m_base_blockhash; - uint32_t m_base_blockheight; //! The number of coins in the UTXO set contained in this snapshot. Used @@ -50,19 +51,16 @@ public: 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) { } template inline void Serialize(Stream& s) const { s << SNAPSHOT_MAGIC_BYTES; - s << m_version; + s << VERSION; s << m_network_magic; - s << m_base_blockheight; s << m_base_blockhash; s << m_coins_count; } @@ -98,7 +96,6 @@ public: } } - s >> m_base_blockheight; s >> m_base_blockhash; s >> m_coins_count; } diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index efee8cb24b..324dfb68e1 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -3017,7 +3017,7 @@ UniValue CreateUTXOSnapshot( tip->nHeight, tip->GetBlockHash().ToString(), fs::PathToString(path), fs::PathToString(temppath))); - SnapshotMetadata metadata{chainstate.m_chainman.GetParams().MessageStart(), tip->GetBlockHash(), tip->nHeight, maybe_stats->coins_count}; + SnapshotMetadata metadata{chainstate.m_chainman.GetParams().MessageStart(), tip->GetBlockHash(), maybe_stats->coins_count}; afile << metadata; @@ -3141,10 +3141,12 @@ static RPCHelpMan loadtxoutset() throw JSONRPCError(RPC_INTERNAL_ERROR, strprintf("Unable to load UTXO snapshot: %s. (%s)", util::ErrorString(activation_result).original, path.utf8string())); } + CBlockIndex& snapshot_index{*CHECK_NONFATAL(*activation_result)}; + UniValue result(UniValue::VOBJ); result.pushKV("coins_loaded", metadata.m_coins_count); - result.pushKV("tip_hash", metadata.m_base_blockhash.ToString()); - result.pushKV("base_height", metadata.m_base_blockheight); + result.pushKV("tip_hash", snapshot_index.GetBlockHash().ToString()); + result.pushKV("base_height", snapshot_index.nHeight); result.pushKV("path", fs::PathToString(path)); return result; }, diff --git a/src/test/fuzz/utxo_snapshot.cpp b/src/test/fuzz/utxo_snapshot.cpp index d82f166765..1d90414443 100644 --- a/src/test/fuzz/utxo_snapshot.cpp +++ b/src/test/fuzz/utxo_snapshot.cpp @@ -57,7 +57,7 @@ FUZZ_TARGET(utxo_snapshot, .init = initialize_chain) int base_blockheight{fuzzed_data_provider.ConsumeIntegralInRange(1, 2 * COINBASE_MATURITY)}; uint256 base_blockhash{g_chain->at(base_blockheight - 1)->GetHash()}; uint64_t m_coins_count{fuzzed_data_provider.ConsumeIntegralInRange(1, 3 * COINBASE_MATURITY)}; - SnapshotMetadata metadata{msg_start, base_blockhash, base_blockheight, m_coins_count}; + SnapshotMetadata metadata{msg_start, base_blockhash, m_coins_count}; outfile << metadata; } // Coins diff --git a/src/validation.cpp b/src/validation.cpp index 6dba6b586d..0063065dcb 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -6280,31 +6280,31 @@ Chainstate& ChainstateManager::InitializeChainstate(CTxMemPool* mempool) return destroyed && !fs::exists(db_path); } -util::Result ChainstateManager::ActivateSnapshot( +util::Result ChainstateManager::ActivateSnapshot( AutoFile& coins_file, const SnapshotMetadata& metadata, bool in_memory) { uint256 base_blockhash = metadata.m_base_blockhash; - int base_blockheight = metadata.m_base_blockheight; if (this->SnapshotBlockhash()) { return util::Error{Untranslated("Can't activate a snapshot-based chainstate more than once")}; } + CBlockIndex* snapshot_start_block{}; + { LOCK(::cs_main); if (!GetParams().AssumeutxoForBlockhash(base_blockhash).has_value()) { auto available_heights = GetParams().GetAvailableSnapshotHeights(); std::string heights_formatted = util::Join(available_heights, ", ", [&](const auto& i) { return util::ToString(i); }); - return util::Error{strprintf(Untranslated("assumeutxo block hash in snapshot metadata not recognized (hash: %s, height: %s). The following snapshot heights are available: %s"), + return util::Error{strprintf(Untranslated("assumeutxo block hash in snapshot metadata not recognized (hash: %s). The following snapshot heights are available: %s"), base_blockhash.ToString(), - base_blockheight, heights_formatted)}; } - CBlockIndex* snapshot_start_block = m_blockman.LookupBlockIndex(base_blockhash); + snapshot_start_block = m_blockman.LookupBlockIndex(base_blockhash); if (!snapshot_start_block) { return util::Error{strprintf(Untranslated("The base block header (%s) must appear in the headers chain. Make sure all headers are syncing, and call loadtxoutset again"), base_blockhash.ToString())}; @@ -6315,7 +6315,7 @@ util::Result ChainstateManager::ActivateSnapshot( return util::Error{strprintf(Untranslated("The base block header (%s) is part of an invalid chain"), base_blockhash.ToString())}; } - if (!m_best_header || m_best_header->GetAncestor(base_blockheight) != snapshot_start_block) { + if (!m_best_header || m_best_header->GetAncestor(snapshot_start_block->nHeight) != snapshot_start_block) { return util::Error{Untranslated("A forked headers-chain with more work than the chain with the snapshot base block header exists. Please proceed to sync without AssumeUtxo.")}; } @@ -6429,7 +6429,7 @@ util::Result ChainstateManager::ActivateSnapshot( m_snapshot_chainstate->CoinsTip().DynamicMemoryUsage() / (1000 * 1000)); this->MaybeRebalanceCaches(); - return {}; + return snapshot_start_block; } static void FlushSnapshotToDisk(CCoinsViewCache& coins_cache, bool snapshot_loaded) diff --git a/src/validation.h b/src/validation.h index 2c4f1a38df..b2b61833a6 100644 --- a/src/validation.h +++ b/src/validation.h @@ -1109,7 +1109,7 @@ public: //! faking nTx* block index data along the way. //! - Move the new chainstate to `m_snapshot_chainstate` and make it our //! ChainstateActive(). - [[nodiscard]] util::Result ActivateSnapshot( + [[nodiscard]] util::Result ActivateSnapshot( AutoFile& coins_file, const node::SnapshotMetadata& metadata, bool in_memory); //! Once the background validation chainstate has reached the height which diff --git a/test/functional/feature_assumeutxo.py b/test/functional/feature_assumeutxo.py index 2ca17a70e4..0cef151fdc 100755 --- a/test/functional/feature_assumeutxo.py +++ b/test/functional/feature_assumeutxo.py @@ -72,7 +72,7 @@ class AssumeutxoTest(BitcoinTestFramework): assert_raises_rpc_error(parsing_error_code, "Unable to parse metadata: Invalid UTXO set snapshot magic bytes. Please check if this is indeed a snapshot file or if you are using an outdated snapshot format.", node.loadtxoutset, bad_snapshot_path) self.log.info(" - snapshot file with unsupported version") - for version in [0, 2]: + for version in [0, 1, 3]: with open(bad_snapshot_path, 'wb') as f: f.write(valid_snapshot_contents[:5] + version.to_bytes(2, "little") + valid_snapshot_contents[7:]) assert_raises_rpc_error(parsing_error_code, f"Unable to parse metadata: Version of snapshot {version} does not match any of the supported versions.", node.loadtxoutset, bad_snapshot_path) @@ -99,21 +99,20 @@ class AssumeutxoTest(BitcoinTestFramework): bogus_block_hash = "0" * 64 # Represents any unknown block hash # The height is not used for anything critical currently, so we just # confirm the manipulation in the error message - bogus_height = 1337 for bad_block_hash in [bogus_block_hash, prev_block_hash]: with open(bad_snapshot_path, 'wb') as f: - f.write(valid_snapshot_contents[:11] + bogus_height.to_bytes(4, "little") + bytes.fromhex(bad_block_hash)[::-1] + valid_snapshot_contents[47:]) + f.write(valid_snapshot_contents[:11] + bytes.fromhex(bad_block_hash)[::-1] + valid_snapshot_contents[43:]) - msg = f"Unable to load UTXO snapshot: assumeutxo block hash in snapshot metadata not recognized (hash: {bad_block_hash}, height: {bogus_height}). The following snapshot heights are available: 110, 200, 299." + msg = f"Unable to load UTXO snapshot: assumeutxo block hash in snapshot metadata not recognized (hash: {bad_block_hash}). The following snapshot heights are available: 110, 200, 299." assert_raises_rpc_error(-32603, msg, node.loadtxoutset, bad_snapshot_path) self.log.info(" - snapshot file with wrong number of coins") - valid_num_coins = int.from_bytes(valid_snapshot_contents[47:47 + 8], "little") + valid_num_coins = int.from_bytes(valid_snapshot_contents[43:43 + 8], "little") for off in [-1, +1]: with open(bad_snapshot_path, 'wb') as f: - f.write(valid_snapshot_contents[:47]) + f.write(valid_snapshot_contents[:43]) f.write((valid_num_coins + off).to_bytes(8, "little")) - f.write(valid_snapshot_contents[47 + 8:]) + f.write(valid_snapshot_contents[43 + 8:]) expected_error(msg="Bad snapshot - coins left over after deserializing 298 coins." if off == -1 else "Bad snapshot format or truncated snapshot after deserializing 299 coins.") self.log.info(" - snapshot file with alternated but parsable UTXO data results in different hash") @@ -131,10 +130,10 @@ class AssumeutxoTest(BitcoinTestFramework): for content, offset, wrong_hash, custom_message in cases: with open(bad_snapshot_path, "wb") as f: - # Prior to offset: Snapshot magic, snapshot version, network magic, height, hash, coins count - f.write(valid_snapshot_contents[:(5 + 2 + 4 + 4 + 32 + 8 + offset)]) + # Prior to offset: Snapshot magic, snapshot version, network magic, hash, coins count + f.write(valid_snapshot_contents[:(5 + 2 + 4 + 32 + 8 + offset)]) f.write(content) - f.write(valid_snapshot_contents[(5 + 2 + 4 + 4 + 32 + 8 + offset + len(content)):]) + f.write(valid_snapshot_contents[(5 + 2 + 4 + 32 + 8 + offset + len(content)):]) msg = custom_message if custom_message is not None else f"Bad snapshot content hash: expected 4f046c8d36732314b2ccd00be35232e7a211eb87b0fd8858f898f511bc7fae64, got {wrong_hash}" expected_error(msg) diff --git a/test/functional/rpc_dumptxoutset.py b/test/functional/rpc_dumptxoutset.py index 8e3ab1cc46..0a298101fe 100755 --- a/test/functional/rpc_dumptxoutset.py +++ b/test/functional/rpc_dumptxoutset.py @@ -43,7 +43,7 @@ class DumptxoutsetTest(BitcoinTestFramework): # UTXO snapshot hash should be deterministic based on mocked time. assert_equal( sha256sum_file(str(expected_path)).hex(), - 'a03c0d60dac68719ebe541098da448ae1f3833133bf3a0f11c86cb7edcd61b38') + '239ff618e80073b5bbc71405fb9177a971c5709efc186608f591efdb0c60cfee') assert_equal( out['txoutset_hash'], '13dd6c482748a6d1817fc3671a8d409edce1d19ee498ebcc891720105d1e265a')