From 351370a1d211615e3d5b158ccb0400cd79c5c085 Mon Sep 17 00:00:00 2001 From: Fabian Jahr Date: Wed, 18 Oct 2023 17:06:36 +0200 Subject: [PATCH 1/6] coinstats: Fix hash_serialized2 calculation The legacy serialization was vulnerable to maleation and is fixed by adopting the same serialization procedure as was already in use for MuHash. This also includes necessary test fixes where the hash_serialized2 was hardcoded as well as correction of the regtest chainparams. Co-authored-by: Sebastian Falbesoner --- src/index/coinstatsindex.cpp | 11 ++-- src/kernel/chainparams.cpp | 4 +- src/kernel/coinstats.cpp | 66 ++++++++++-------------- src/kernel/coinstats.h | 4 +- src/test/validation_tests.cpp | 4 +- test/functional/feature_assumeutxo.py | 10 ++-- test/functional/feature_utxo_set_hash.py | 2 +- test/functional/rpc_dumptxoutset.py | 2 +- 8 files changed, 49 insertions(+), 54 deletions(-) diff --git a/src/index/coinstatsindex.cpp b/src/index/coinstatsindex.cpp index 9dab8ca901..ecd3fd21b5 100644 --- a/src/index/coinstatsindex.cpp +++ b/src/index/coinstatsindex.cpp @@ -15,9 +15,10 @@ #include #include +using kernel::ApplyCoinHash; using kernel::CCoinsStats; using kernel::GetBogoSize; -using kernel::TxOutSer; +using kernel::RemoveCoinHash; static constexpr uint8_t DB_BLOCK_HASH{'s'}; static constexpr uint8_t DB_BLOCK_HEIGHT{'t'}; @@ -166,7 +167,7 @@ bool CoinStatsIndex::CustomAppend(const interfaces::BlockInfo& block) continue; } - m_muhash.Insert(MakeUCharSpan(TxOutSer(outpoint, coin))); + ApplyCoinHash(m_muhash, outpoint, coin); if (tx->IsCoinBase()) { m_total_coinbase_amount += coin.out.nValue; @@ -187,7 +188,7 @@ bool CoinStatsIndex::CustomAppend(const interfaces::BlockInfo& block) Coin coin{tx_undo.vprevout[j]}; COutPoint outpoint{tx->vin[j].prevout.hash, tx->vin[j].prevout.n}; - m_muhash.Remove(MakeUCharSpan(TxOutSer(outpoint, coin))); + RemoveCoinHash(m_muhash, outpoint, coin); m_total_prevout_spent_amount += coin.out.nValue; @@ -443,7 +444,7 @@ bool CoinStatsIndex::ReverseBlock(const CBlock& block, const CBlockIndex* pindex continue; } - m_muhash.Remove(MakeUCharSpan(TxOutSer(outpoint, coin))); + RemoveCoinHash(m_muhash, outpoint, coin); if (tx->IsCoinBase()) { m_total_coinbase_amount -= coin.out.nValue; @@ -464,7 +465,7 @@ bool CoinStatsIndex::ReverseBlock(const CBlock& block, const CBlockIndex* pindex Coin coin{tx_undo.vprevout[j]}; COutPoint outpoint{tx->vin[j].prevout.hash, tx->vin[j].prevout.n}; - m_muhash.Insert(MakeUCharSpan(TxOutSer(outpoint, coin))); + ApplyCoinHash(m_muhash, outpoint, coin); m_total_prevout_spent_amount -= coin.out.nValue; diff --git a/src/kernel/chainparams.cpp b/src/kernel/chainparams.cpp index 3ac8756e41..f928520d3e 100644 --- a/src/kernel/chainparams.cpp +++ b/src/kernel/chainparams.cpp @@ -494,14 +494,14 @@ public: m_assumeutxo_data = { { .height = 110, - .hash_serialized = AssumeutxoHash{uint256S("0x1ebbf5850204c0bdb15bf030f47c7fe91d45c44c712697e4509ba67adb01c618")}, + .hash_serialized = AssumeutxoHash{uint256S("0x6657b736d4fe4db0cbc796789e812d5dba7f5c143764b1b6905612f1830609d1")}, .nChainTx = 111, .blockhash = uint256S("0x696e92821f65549c7ee134edceeeeaaa4105647a3c4fd9f298c0aec0ab50425c") }, { // For use by test/functional/feature_assumeutxo.py .height = 299, - .hash_serialized = AssumeutxoHash{uint256S("0xef45ccdca5898b6c2145e4581d2b88c56564dd389e4bd75a1aaf6961d3edd3c0")}, + .hash_serialized = AssumeutxoHash{uint256S("0x61d9c2b29a2571a5fe285fe2d8554f91f93309666fc9b8223ee96338de25ff53")}, .nChainTx = 300, .blockhash = uint256S("0x7e0517ef3ea6ecbed9117858e42eedc8eb39e8698a38dcbd1b3962a283233f4c") }, diff --git a/src/kernel/coinstats.cpp b/src/kernel/coinstats.cpp index 527433f45e..9bd755ed27 100644 --- a/src/kernel/coinstats.cpp +++ b/src/kernel/coinstats.cpp @@ -48,15 +48,35 @@ uint64_t GetBogoSize(const CScript& script_pub_key) script_pub_key.size() /* scriptPubKey */; } -DataStream TxOutSer(const COutPoint& outpoint, const Coin& coin) +template +static void TxOutSer(T& ss, const COutPoint& outpoint, const Coin& coin) +{ + ss << outpoint; + ss << static_cast((coin.nHeight << 1) + coin.fCoinBase); + ss << coin.out; +} + +static void ApplyCoinHash(HashWriter& ss, const COutPoint& outpoint, const Coin& coin) +{ + TxOutSer(ss, outpoint, coin); +} + +void ApplyCoinHash(MuHash3072& muhash, const COutPoint& outpoint, const Coin& coin) { DataStream ss{}; - ss << outpoint; - ss << static_cast(coin.nHeight * 2 + coin.fCoinBase); - ss << coin.out; - return ss; + TxOutSer(ss, outpoint, coin); + muhash.Insert(MakeUCharSpan(ss)); } +void RemoveCoinHash(MuHash3072& muhash, const COutPoint& outpoint, const Coin& coin) +{ + DataStream ss{}; + TxOutSer(ss, outpoint, coin); + muhash.Remove(MakeUCharSpan(ss)); +} + +static void ApplyCoinHash(std::nullptr_t, const COutPoint& outpoint, const Coin& coin) {} + //! Warning: be very careful when changing this! assumeutxo and UTXO snapshot //! validation commitments are reliant on the hash constructed by this //! function. @@ -69,32 +89,13 @@ DataStream TxOutSer(const COutPoint& outpoint, const Coin& coin) //! It is also possible, though very unlikely, that a change in this //! construction could cause a previously invalid (and potentially malicious) //! UTXO snapshot to be considered valid. -static void ApplyHash(HashWriter& ss, const uint256& hash, const std::map& outputs) -{ - for (auto it = outputs.begin(); it != outputs.end(); ++it) { - if (it == outputs.begin()) { - ss << hash; - ss << VARINT(it->second.nHeight * 2 + it->second.fCoinBase ? 1u : 0u); - } - - ss << VARINT(it->first + 1); - ss << it->second.out.scriptPubKey; - ss << VARINT_MODE(it->second.out.nValue, VarIntMode::NONNEGATIVE_SIGNED); - - if (it == std::prev(outputs.end())) { - ss << VARINT(0u); - } - } -} - -static void ApplyHash(std::nullptr_t, const uint256& hash, const std::map& outputs) {} - -static void ApplyHash(MuHash3072& muhash, const uint256& hash, const std::map& outputs) +template +static void ApplyHash(T& hash_obj, const uint256& hash, const std::map& outputs) { for (auto it = outputs.begin(); it != outputs.end(); ++it) { COutPoint outpoint = COutPoint(hash, it->first); Coin coin = it->second; - muhash.Insert(MakeUCharSpan(TxOutSer(outpoint, coin))); + ApplyCoinHash(hash_obj, outpoint, coin); } } @@ -118,8 +119,6 @@ static bool ComputeUTXOStats(CCoinsView* view, CCoinsStats& stats, T hash_obj, c std::unique_ptr pcursor(view->Cursor()); assert(pcursor); - PrepareHash(hash_obj, stats); - uint256 prevkey; std::map outputs; while (pcursor->Valid()) { @@ -180,15 +179,6 @@ std::optional ComputeUTXOStats(CoinStatsHashType hash_type, CCoinsV return stats; } -// The legacy hash serializes the hashBlock -static void PrepareHash(HashWriter& ss, const CCoinsStats& stats) -{ - ss << stats.hashBlock; -} -// MuHash does not need the prepare step -static void PrepareHash(MuHash3072& muhash, CCoinsStats& stats) {} -static void PrepareHash(std::nullptr_t, CCoinsStats& stats) {} - static void FinalizeHash(HashWriter& ss, CCoinsStats& stats) { stats.hashSerialized = ss.GetHash(); diff --git a/src/kernel/coinstats.h b/src/kernel/coinstats.h index 54d0e4f664..c0c363a842 100644 --- a/src/kernel/coinstats.h +++ b/src/kernel/coinstats.h @@ -6,6 +6,7 @@ #define BITCOIN_KERNEL_COINSTATS_H #include +#include #include #include @@ -72,7 +73,8 @@ struct CCoinsStats { uint64_t GetBogoSize(const CScript& script_pub_key); -DataStream TxOutSer(const COutPoint& outpoint, const Coin& coin); +void ApplyCoinHash(MuHash3072& muhash, const COutPoint& outpoint, const Coin& coin); +void RemoveCoinHash(MuHash3072& muhash, const COutPoint& outpoint, const Coin& coin); std::optional ComputeUTXOStats(CoinStatsHashType hash_type, CCoinsView* view, node::BlockManager& blockman, const std::function& interruption_point = {}); } // namespace kernel diff --git a/src/test/validation_tests.cpp b/src/test/validation_tests.cpp index 2692037273..14440571eb 100644 --- a/src/test/validation_tests.cpp +++ b/src/test/validation_tests.cpp @@ -137,11 +137,11 @@ BOOST_AUTO_TEST_CASE(test_assumeutxo) } const auto out110 = *params->AssumeutxoForHeight(110); - BOOST_CHECK_EQUAL(out110.hash_serialized.ToString(), "1ebbf5850204c0bdb15bf030f47c7fe91d45c44c712697e4509ba67adb01c618"); + BOOST_CHECK_EQUAL(out110.hash_serialized.ToString(), "6657b736d4fe4db0cbc796789e812d5dba7f5c143764b1b6905612f1830609d1"); BOOST_CHECK_EQUAL(out110.nChainTx, 111U); const auto out110_2 = *params->AssumeutxoForBlockhash(uint256S("0x696e92821f65549c7ee134edceeeeaaa4105647a3c4fd9f298c0aec0ab50425c")); - BOOST_CHECK_EQUAL(out110_2.hash_serialized.ToString(), "1ebbf5850204c0bdb15bf030f47c7fe91d45c44c712697e4509ba67adb01c618"); + BOOST_CHECK_EQUAL(out110_2.hash_serialized.ToString(), "6657b736d4fe4db0cbc796789e812d5dba7f5c143764b1b6905612f1830609d1"); BOOST_CHECK_EQUAL(out110_2.nChainTx, 111U); } diff --git a/test/functional/feature_assumeutxo.py b/test/functional/feature_assumeutxo.py index 2765509f84..9c265649d5 100755 --- a/test/functional/feature_assumeutxo.py +++ b/test/functional/feature_assumeutxo.py @@ -94,8 +94,10 @@ class AssumeutxoTest(BitcoinTestFramework): self.log.info(" - snapshot file with alternated UTXO data") cases = [ - [b"\xff" * 32, 0, "29926acf3ac81f908cf4f22515713ca541c08bb0f0ef1b2c3443a007134d69b8"], # wrong outpoint hash - [(1).to_bytes(4, "little"), 32, "798266c2e1f9a98fe5ce61f5951cbf47130743f3764cf3cbc254be129142cf9d"], # wrong outpoint index + [b"\xff" * 32, 0, "05030e506678f2eca8d624ffed97090ab3beadad1b51ee6e5985ba91c5720e37"], # wrong outpoint hash + [(1).to_bytes(4, "little"), 32, "7d29cfe2c1e242bc6f103878bb70cfffa8b4dac20dbd001ff6ce24b7de2d2399"], # wrong outpoint index + [b"\x81", 36, "f03939a195531f96d5dff983e294a1af62af86049fa7a19a7627246f237c03f1"], # wrong coin code VARINT((coinbase ? 1 : 0) | (height << 1)) + [b"\x83", 36, "e4577da84590fb288c0f7967e89575e1b0aa46624669640f6f5dfef028d39930"], # another wrong coin code ] for content, offset, wrong_hash in cases: @@ -103,7 +105,7 @@ class AssumeutxoTest(BitcoinTestFramework): f.write(valid_snapshot_contents[:(32 + 8 + offset)]) f.write(content) f.write(valid_snapshot_contents[(32 + 8 + offset + len(content)):]) - expected_error(log_msg=f"[snapshot] bad snapshot content hash: expected ef45ccdca5898b6c2145e4581d2b88c56564dd389e4bd75a1aaf6961d3edd3c0, got {wrong_hash}") + expected_error(log_msg=f"[snapshot] bad snapshot content hash: expected 61d9c2b29a2571a5fe285fe2d8554f91f93309666fc9b8223ee96338de25ff53, got {wrong_hash}") def run_test(self): """ @@ -150,7 +152,7 @@ class AssumeutxoTest(BitcoinTestFramework): assert_equal( dump_output['txoutset_hash'], - 'ef45ccdca5898b6c2145e4581d2b88c56564dd389e4bd75a1aaf6961d3edd3c0') + '61d9c2b29a2571a5fe285fe2d8554f91f93309666fc9b8223ee96338de25ff53') assert_equal(dump_output['nchaintx'], 300) assert_equal(n0.getblockchaininfo()["blocks"], SNAPSHOT_BASE_HEIGHT) diff --git a/test/functional/feature_utxo_set_hash.py b/test/functional/feature_utxo_set_hash.py index 0f510ced89..49c01eee1d 100755 --- a/test/functional/feature_utxo_set_hash.py +++ b/test/functional/feature_utxo_set_hash.py @@ -69,7 +69,7 @@ class UTXOSetHashTest(BitcoinTestFramework): assert_equal(finalized[::-1].hex(), node_muhash) self.log.info("Test deterministic UTXO set hash results") - assert_equal(node.gettxoutsetinfo()['hash_serialized_2'], "f9aa4fb5ffd10489b9a6994e70ccf1de8a8bfa2d5f201d9857332e9954b0855d") + assert_equal(node.gettxoutsetinfo()['hash_serialized_2'], "d1c7fec1c0623f6793839878cbe2a531eb968b50b27edd6e2a57077a5aed6094") assert_equal(node.gettxoutsetinfo("muhash")['muhash'], "d1725b2fe3ef43e55aa4907480aea98d406fc9e0bf8f60169e2305f1fbf5961b") def run_test(self): diff --git a/test/functional/rpc_dumptxoutset.py b/test/functional/rpc_dumptxoutset.py index f378878181..1ea6cf52d1 100755 --- a/test/functional/rpc_dumptxoutset.py +++ b/test/functional/rpc_dumptxoutset.py @@ -46,7 +46,7 @@ class DumptxoutsetTest(BitcoinTestFramework): 'b1bacb602eacf5fbc9a7c2ef6eeb0d229c04e98bdf0c2ea5929012cd0eae3830') assert_equal( - out['txoutset_hash'], '1f7e3befd45dc13ae198dfbb22869a9c5c4196f8e9ef9735831af1288033f890') + out['txoutset_hash'], 'a0b7baa3bf5ccbd3279728f230d7ca0c44a76e9923fca8f32dbfd08d65ea496a') assert_equal(out['nchaintx'], 101) # Specifying a path to an existing or invalid file will fail. From cb0336817edc2b6aee2eca818f133841f613a767 Mon Sep 17 00:00:00 2001 From: Fabian Jahr Date: Thu, 19 Oct 2023 14:54:41 +0200 Subject: [PATCH 2/6] scripted-diff: Rename hash_serialized_2 to hash_serialized_3 -BEGIN VERIFY SCRIPT- sed -i 's/hash_serialized_2/hash_serialized_3/g' $( git grep -l 'hash_serialized_2' ./src ./contrib ./test ) -END VERIFY SCRIPT- --- contrib/devtools/utxo_snapshot.sh | 2 +- src/rpc/blockchain.cpp | 10 +++++----- test/functional/feature_coinstatsindex.py | 6 +++--- test/functional/feature_dbcrash.py | 8 ++++---- test/functional/feature_utxo_set_hash.py | 2 +- test/functional/rpc_blockchain.py | 12 ++++++------ 6 files changed, 20 insertions(+), 20 deletions(-) diff --git a/contrib/devtools/utxo_snapshot.sh b/contrib/devtools/utxo_snapshot.sh index dee25ff67b..ad2ec26651 100755 --- a/contrib/devtools/utxo_snapshot.sh +++ b/contrib/devtools/utxo_snapshot.sh @@ -34,7 +34,7 @@ ${BITCOIN_CLI_CALL} invalidateblock "${PIVOT_BLOCKHASH}" if [[ "${OUTPUT_PATH}" = "-" ]]; then (>&2 echo "Generating txoutset info...") - ${BITCOIN_CLI_CALL} gettxoutsetinfo | grep hash_serialized_2 | sed 's/^.*: "\(.\+\)\+",/\1/g' + ${BITCOIN_CLI_CALL} gettxoutsetinfo | grep hash_serialized_3 | sed 's/^.*: "\(.\+\)\+",/\1/g' else (>&2 echo "Generating UTXO snapshot...") ${BITCOIN_CLI_CALL} dumptxoutset "${OUTPUT_PATH}" diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 6d2b84cb6c..7b84747a3f 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -820,7 +820,7 @@ static RPCHelpMan pruneblockchain() CoinStatsHashType ParseHashType(const std::string& hash_type_input) { - if (hash_type_input == "hash_serialized_2") { + if (hash_type_input == "hash_serialized_3") { return CoinStatsHashType::HASH_SERIALIZED; } else if (hash_type_input == "muhash") { return CoinStatsHashType::MUHASH; @@ -867,7 +867,7 @@ static RPCHelpMan gettxoutsetinfo() "\nReturns statistics about the unspent transaction output set.\n" "Note this call may take some time if you are not using coinstatsindex.\n", { - {"hash_type", RPCArg::Type::STR, RPCArg::Default{"hash_serialized_2"}, "Which UTXO set hash should be calculated. Options: 'hash_serialized_2' (the legacy algorithm), 'muhash', 'none'."}, + {"hash_type", RPCArg::Type::STR, RPCArg::Default{"hash_serialized_3"}, "Which UTXO set hash should be calculated. Options: 'hash_serialized_3' (the legacy algorithm), 'muhash', 'none'."}, {"hash_or_height", RPCArg::Type::NUM, RPCArg::DefaultHint{"the current best block"}, "The block hash or height of the target height (only available with coinstatsindex).", RPCArgOptions{ .skip_type_check = true, @@ -882,7 +882,7 @@ static RPCHelpMan gettxoutsetinfo() {RPCResult::Type::STR_HEX, "bestblock", "The hash of the block at which these statistics are calculated"}, {RPCResult::Type::NUM, "txouts", "The number of unspent transaction outputs"}, {RPCResult::Type::NUM, "bogosize", "Database-independent, meaningless metric indicating the UTXO set size"}, - {RPCResult::Type::STR_HEX, "hash_serialized_2", /*optional=*/true, "The serialized hash (only present if 'hash_serialized_2' hash_type is chosen)"}, + {RPCResult::Type::STR_HEX, "hash_serialized_3", /*optional=*/true, "The serialized hash (only present if 'hash_serialized_3' hash_type is chosen)"}, {RPCResult::Type::STR_HEX, "muhash", /*optional=*/true, "The serialized hash (only present if 'muhash' hash_type is chosen)"}, {RPCResult::Type::NUM, "transactions", /*optional=*/true, "The number of transactions with unspent outputs (not available when coinstatsindex is used)"}, {RPCResult::Type::NUM, "disk_size", /*optional=*/true, "The estimated size of the chainstate on disk (not available when coinstatsindex is used)"}, @@ -942,7 +942,7 @@ static RPCHelpMan gettxoutsetinfo() } if (hash_type == CoinStatsHashType::HASH_SERIALIZED) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "hash_serialized_2 hash type cannot be queried for a specific block"); + throw JSONRPCError(RPC_INVALID_PARAMETER, "hash_serialized_3 hash type cannot be queried for a specific block"); } if (!index_requested) { @@ -971,7 +971,7 @@ static RPCHelpMan gettxoutsetinfo() ret.pushKV("txouts", (int64_t)stats.nTransactionOutputs); ret.pushKV("bogosize", (int64_t)stats.nBogoSize); if (hash_type == CoinStatsHashType::HASH_SERIALIZED) { - ret.pushKV("hash_serialized_2", stats.hashSerialized.GetHex()); + ret.pushKV("hash_serialized_3", stats.hashSerialized.GetHex()); } if (hash_type == CoinStatsHashType::MUHASH) { ret.pushKV("muhash", stats.hashSerialized.GetHex()); diff --git a/test/functional/feature_coinstatsindex.py b/test/functional/feature_coinstatsindex.py index 2ffb182946..d6c1567e64 100755 --- a/test/functional/feature_coinstatsindex.py +++ b/test/functional/feature_coinstatsindex.py @@ -293,11 +293,11 @@ class CoinStatsIndexTest(BitcoinTestFramework): def _test_index_rejects_hash_serialized(self): self.log.info("Test that the rpc raises if the legacy hash is passed with the index") - msg = "hash_serialized_2 hash type cannot be queried for a specific block" - assert_raises_rpc_error(-8, msg, self.nodes[1].gettxoutsetinfo, hash_type='hash_serialized_2', hash_or_height=111) + msg = "hash_serialized_3 hash type cannot be queried for a specific block" + assert_raises_rpc_error(-8, msg, self.nodes[1].gettxoutsetinfo, hash_type='hash_serialized_3', hash_or_height=111) for use_index in {True, False, None}: - assert_raises_rpc_error(-8, msg, self.nodes[1].gettxoutsetinfo, hash_type='hash_serialized_2', hash_or_height=111, use_index=use_index) + assert_raises_rpc_error(-8, msg, self.nodes[1].gettxoutsetinfo, hash_type='hash_serialized_3', hash_or_height=111, use_index=use_index) def _test_init_index_after_reorg(self): self.log.info("Test a reorg while the index is deactivated") diff --git a/test/functional/feature_dbcrash.py b/test/functional/feature_dbcrash.py index 3f94bbc9d1..afd0246209 100755 --- a/test/functional/feature_dbcrash.py +++ b/test/functional/feature_dbcrash.py @@ -85,7 +85,7 @@ class ChainstateWriteCrashTest(BitcoinTestFramework): # Any of these RPC calls could throw due to node crash self.start_node(node_index) self.nodes[node_index].waitforblock(expected_tip) - utxo_hash = self.nodes[node_index].gettxoutsetinfo()['hash_serialized_2'] + utxo_hash = self.nodes[node_index].gettxoutsetinfo()['hash_serialized_3'] return utxo_hash except Exception: # An exception here should mean the node is about to crash. @@ -130,7 +130,7 @@ class ChainstateWriteCrashTest(BitcoinTestFramework): If any nodes crash while updating, we'll compare utxo hashes to ensure recovery was successful.""" - node3_utxo_hash = self.nodes[3].gettxoutsetinfo()['hash_serialized_2'] + node3_utxo_hash = self.nodes[3].gettxoutsetinfo()['hash_serialized_3'] # Retrieve all the blocks from node3 blocks = [] @@ -172,12 +172,12 @@ class ChainstateWriteCrashTest(BitcoinTestFramework): """Verify that the utxo hash of each node matches node3. Restart any nodes that crash while querying.""" - node3_utxo_hash = self.nodes[3].gettxoutsetinfo()['hash_serialized_2'] + node3_utxo_hash = self.nodes[3].gettxoutsetinfo()['hash_serialized_3'] self.log.info("Verifying utxo hash matches for all nodes") for i in range(3): try: - nodei_utxo_hash = self.nodes[i].gettxoutsetinfo()['hash_serialized_2'] + nodei_utxo_hash = self.nodes[i].gettxoutsetinfo()['hash_serialized_3'] except OSError: # probably a crash on db flushing nodei_utxo_hash = self.restart_node(i, self.nodes[3].getbestblockhash()) diff --git a/test/functional/feature_utxo_set_hash.py b/test/functional/feature_utxo_set_hash.py index 49c01eee1d..ce2a5ab8ac 100755 --- a/test/functional/feature_utxo_set_hash.py +++ b/test/functional/feature_utxo_set_hash.py @@ -69,7 +69,7 @@ class UTXOSetHashTest(BitcoinTestFramework): assert_equal(finalized[::-1].hex(), node_muhash) self.log.info("Test deterministic UTXO set hash results") - assert_equal(node.gettxoutsetinfo()['hash_serialized_2'], "d1c7fec1c0623f6793839878cbe2a531eb968b50b27edd6e2a57077a5aed6094") + assert_equal(node.gettxoutsetinfo()['hash_serialized_3'], "d1c7fec1c0623f6793839878cbe2a531eb968b50b27edd6e2a57077a5aed6094") assert_equal(node.gettxoutsetinfo("muhash")['muhash'], "d1725b2fe3ef43e55aa4907480aea98d406fc9e0bf8f60169e2305f1fbf5961b") def run_test(self): diff --git a/test/functional/rpc_blockchain.py b/test/functional/rpc_blockchain.py index 18a0a0c6cc..53163720bb 100755 --- a/test/functional/rpc_blockchain.py +++ b/test/functional/rpc_blockchain.py @@ -340,7 +340,7 @@ class BlockchainTest(BitcoinTestFramework): assert size > 6400 assert size < 64000 assert_equal(len(res['bestblock']), 64) - assert_equal(len(res['hash_serialized_2']), 64) + assert_equal(len(res['hash_serialized_3']), 64) self.log.info("Test gettxoutsetinfo works for blockchain with just the genesis block") b1hash = node.getblockhash(1) @@ -353,7 +353,7 @@ class BlockchainTest(BitcoinTestFramework): assert_equal(res2['txouts'], 0) assert_equal(res2['bogosize'], 0), assert_equal(res2['bestblock'], node.getblockhash(0)) - assert_equal(len(res2['hash_serialized_2']), 64) + assert_equal(len(res2['hash_serialized_3']), 64) self.log.info("Test gettxoutsetinfo returns the same result after invalidate/reconsider block") node.reconsiderblock(b1hash) @@ -365,20 +365,20 @@ class BlockchainTest(BitcoinTestFramework): assert_equal(res, res3) self.log.info("Test gettxoutsetinfo hash_type option") - # Adding hash_type 'hash_serialized_2', which is the default, should + # Adding hash_type 'hash_serialized_3', which is the default, should # not change the result. - res4 = node.gettxoutsetinfo(hash_type='hash_serialized_2') + res4 = node.gettxoutsetinfo(hash_type='hash_serialized_3') del res4['disk_size'] assert_equal(res, res4) # hash_type none should not return a UTXO set hash. res5 = node.gettxoutsetinfo(hash_type='none') - assert 'hash_serialized_2' not in res5 + assert 'hash_serialized_3' not in res5 # hash_type muhash should return a different UTXO set hash. res6 = node.gettxoutsetinfo(hash_type='muhash') assert 'muhash' in res6 - assert res['hash_serialized_2'] != res6['muhash'] + assert res['hash_serialized_3'] != res6['muhash'] # muhash should not be returned unless requested. for r in [res, res2, res3, res4, res5]: From 66865446a771327be9e972cdaf01154ea1bdff6d Mon Sep 17 00:00:00 2001 From: Fabian Jahr Date: Thu, 19 Oct 2023 15:02:38 +0200 Subject: [PATCH 3/6] docs: Add release notes for #28685 --- doc/release-notes-28685.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 doc/release-notes-28685.md diff --git a/doc/release-notes-28685.md b/doc/release-notes-28685.md new file mode 100644 index 0000000000..6f04d8d542 --- /dev/null +++ b/doc/release-notes-28685.md @@ -0,0 +1,4 @@ +RPC +--- + +The `hash_serialized_2` value has been removed from `gettxoutsetinfo` since the value it calculated contained a bug and did not take all data into account. It is superseded by `hash_serialized_3` which provides the same functionality but serves the correctly calculated hash. From f6213929c519d0e615cacd3d6f479f1517be1662 Mon Sep 17 00:00:00 2001 From: Fabian Jahr Date: Thu, 19 Oct 2023 15:49:04 +0200 Subject: [PATCH 4/6] assumeutxo: Check deserialized coins for out of range values --- src/validation.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/validation.cpp b/src/validation.cpp index 2600f0f9fe..a6cab6b095 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -5399,6 +5399,11 @@ bool ChainstateManager::PopulateAndValidateSnapshot( coins_count - coins_left); return false; } + if (!MoneyRange(coin.out.nValue)) { + LogPrintf("[snapshot] bad snapshot data after deserializing %d coins - bad tx out value\n", + coins_count - coins_left); + return false; + } coins_cache.EmplaceCoinInternalDANGER(std::move(outpoint), std::move(coin)); From a503cd0f0b55736743bcf8d2c46d271064772bef Mon Sep 17 00:00:00 2001 From: Fabian Jahr Date: Thu, 19 Oct 2023 14:08:19 +0200 Subject: [PATCH 5/6] chainparams, assumeutxo: Fix testnet txoutset hash Review hint: You can use devtools/utxo_snapshot.sh to validate this. ./contrib/devtools/utxo_snapshot.sh 2500000 testnet-utxo.dat ./src/bitcoin-cli --- src/kernel/chainparams.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/kernel/chainparams.cpp b/src/kernel/chainparams.cpp index f928520d3e..fe47306fb5 100644 --- a/src/kernel/chainparams.cpp +++ b/src/kernel/chainparams.cpp @@ -269,7 +269,7 @@ public: m_assumeutxo_data = { { .height = 2'500'000, - .hash_serialized = AssumeutxoHash{uint256S("0x2a8fdefef3bf75fa00540ccaaaba4b5281bea94229327bdb0f7416ef1e7a645c")}, + .hash_serialized = AssumeutxoHash{uint256S("0xf841584909f68e47897952345234e37fcd9128cd818f41ee6c3ca68db8071be7")}, .nChainTx = 66484552, .blockhash = uint256S("0x0000000000000093bcb68c03a9a168ae252572d348a2eaeba2cdf9231d73206f") } From 4bfaad4eca01674a9c84a447a17594dc2b9a4c39 Mon Sep 17 00:00:00 2001 From: Fabian Jahr Date: Thu, 19 Oct 2023 14:10:18 +0200 Subject: [PATCH 6/6] chainparams, assumeutxo: Fix signet txoutset hash Review hint: You can use devtools/utxo_snapshot.sh to validate this. ./contrib/devtools/utxo_snapshot.sh 160000 signet-utxo.dat ./src/bitcoin-cli --- src/kernel/chainparams.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/kernel/chainparams.cpp b/src/kernel/chainparams.cpp index fe47306fb5..73ba330ff0 100644 --- a/src/kernel/chainparams.cpp +++ b/src/kernel/chainparams.cpp @@ -378,7 +378,7 @@ public: m_assumeutxo_data = { { .height = 160'000, - .hash_serialized = AssumeutxoHash{uint256S("0x5225141cb62dee63ab3be95f9b03d60801f264010b1816d4bd00618b2736e7be")}, + .hash_serialized = AssumeutxoHash{uint256S("0xfe0a44309b74d6b5883d246cb419c6221bcccf0b308c9b59b7d70783dbdf928a")}, .nChainTx = 2289496, .blockhash = uint256S("0x0000003ca3c99aff040f2563c2ad8f8ec88bd0fd6b8f0895cfaf1ef90353a62c") }