From 5658c51871daedf97829f0d15bfa5657e42492c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Mon, 9 Dec 2024 13:25:25 +0100 Subject: [PATCH 1/3] coins,refactor: Migrate Coin#fCoinBase to bool --- src/coins.h | 2 +- src/core_write.cpp | 2 +- src/rpc/blockchain.cpp | 2 +- src/test/coins_tests.cpp | 8 ++++---- src/test/fuzz/utxo_snapshot.cpp | 4 ++-- src/validation.cpp | 2 +- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/coins.h b/src/coins.h index 61fb4af6420..0899e4c7360 100644 --- a/src/coins.h +++ b/src/coins.h @@ -36,7 +36,7 @@ public: CTxOut out; //! whether containing transaction was a coinbase - unsigned int fCoinBase : 1; + bool fCoinBase : 1; //! at which height this containing transaction was included in the active block chain uint32_t nHeight : 31; diff --git a/src/core_write.cpp b/src/core_write.cpp index 253dfde1006..b7da8eac319 100644 --- a/src/core_write.cpp +++ b/src/core_write.cpp @@ -219,7 +219,7 @@ void TxToUniv(const CTransaction& tx, const uint256& block_hash, UniValue& entry ScriptToUniv(prev_txout.scriptPubKey, /*out=*/o_script_pub_key, /*include_hex=*/true, /*include_address=*/true); UniValue p(UniValue::VOBJ); - p.pushKV("generated", bool(prev_coin.fCoinBase)); + p.pushKV("generated", prev_coin.IsCoinBase()); p.pushKV("height", uint64_t(prev_coin.nHeight)); p.pushKV("value", ValueFromAmount(prev_txout.nValue)); p.pushKV("scriptPubKey", std::move(o_script_pub_key)); diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 6a57a3c4878..52440464d82 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1160,7 +1160,7 @@ static RPCHelpMan gettxout() UniValue o(UniValue::VOBJ); ScriptToUniv(coin->out.scriptPubKey, /*out=*/o, /*include_hex=*/true, /*include_address=*/true); ret.pushKV("scriptPubKey", std::move(o)); - ret.pushKV("coinbase", (bool)coin->fCoinBase); + ret.pushKV("coinbase", coin->IsCoinBase()); return ret; }, diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index c46144b34b4..ff6e476a7bd 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -514,7 +514,7 @@ BOOST_AUTO_TEST_CASE(ccoins_serialization) DataStream ss1{"97f23c835800816115944e077fe7c803cfa57f29b36bf87c1d35"_hex}; Coin cc1; ss1 >> cc1; - BOOST_CHECK_EQUAL(cc1.fCoinBase, false); + BOOST_CHECK_EQUAL(cc1.IsCoinBase(), false); BOOST_CHECK_EQUAL(cc1.nHeight, 203998U); BOOST_CHECK_EQUAL(cc1.out.nValue, CAmount{60000000000}); BOOST_CHECK_EQUAL(HexStr(cc1.out.scriptPubKey), HexStr(GetScriptForDestination(PKHash(uint160("816115944e077fe7c803cfa57f29b36bf87c1d35"_hex_u8))))); @@ -523,7 +523,7 @@ BOOST_AUTO_TEST_CASE(ccoins_serialization) DataStream ss2{"8ddf77bbd123008c988f1a4a4de2161e0f50aac7f17e7f9555caa4"_hex}; Coin cc2; ss2 >> cc2; - BOOST_CHECK_EQUAL(cc2.fCoinBase, true); + BOOST_CHECK_EQUAL(cc2.IsCoinBase(), true); BOOST_CHECK_EQUAL(cc2.nHeight, 120891U); BOOST_CHECK_EQUAL(cc2.out.nValue, 110397); BOOST_CHECK_EQUAL(HexStr(cc2.out.scriptPubKey), HexStr(GetScriptForDestination(PKHash(uint160("8c988f1a4a4de2161e0f50aac7f17e7f9555caa4"_hex_u8))))); @@ -532,7 +532,7 @@ BOOST_AUTO_TEST_CASE(ccoins_serialization) DataStream ss3{"000006"_hex}; Coin cc3; ss3 >> cc3; - BOOST_CHECK_EQUAL(cc3.fCoinBase, false); + BOOST_CHECK_EQUAL(cc3.IsCoinBase(), false); BOOST_CHECK_EQUAL(cc3.nHeight, 0U); BOOST_CHECK_EQUAL(cc3.out.nValue, 0); BOOST_CHECK_EQUAL(cc3.out.scriptPubKey.size(), 0U); @@ -870,7 +870,7 @@ Coin MakeCoin() Coin coin; coin.out.nValue = m_rng.rand32(); coin.nHeight = m_rng.randrange(4096); - coin.fCoinBase = 0; + coin.fCoinBase = false; return coin; } diff --git a/src/test/fuzz/utxo_snapshot.cpp b/src/test/fuzz/utxo_snapshot.cpp index 1241bba8be3..c0413cbc7e8 100644 --- a/src/test/fuzz/utxo_snapshot.cpp +++ b/src/test/fuzz/utxo_snapshot.cpp @@ -104,7 +104,7 @@ void utxo_snapshot_fuzz(FuzzBufferType buffer) outfile << coinbase->GetHash(); WriteCompactSize(outfile, 1); // number of coins for the hash WriteCompactSize(outfile, 0); // index of coin - outfile << Coin(coinbase->vout[0], height, /*fCoinBaseIn=*/1); + outfile << Coin(coinbase->vout[0], height, /*fCoinBaseIn=*/true); height++; } } @@ -116,7 +116,7 @@ void utxo_snapshot_fuzz(FuzzBufferType buffer) outfile << coinbase->GetHash(); WriteCompactSize(outfile, 1); // number of coins for the hash WriteCompactSize(outfile, 999); // index of coin - outfile << Coin{coinbase->vout[0], /*nHeightIn=*/999, /*fCoinBaseIn=*/0}; + outfile << Coin{coinbase->vout[0], /*nHeightIn=*/999, /*fCoinBaseIn=*/false}; } } diff --git a/src/validation.cpp b/src/validation.cpp index 0227ccec2fd..60ec829cd78 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2332,7 +2332,7 @@ DisconnectResult Chainstate::DisconnectBlock(const CBlock& block, const CBlockIn COutPoint out(hash, o); Coin coin; bool is_spent = view.SpendCoin(out, &coin); - if (!is_spent || tx.vout[o] != coin.out || pindex->nHeight != coin.nHeight || is_coinbase != coin.fCoinBase) { + if (!is_spent || tx.vout[o] != coin.out || pindex->nHeight != coin.nHeight || is_coinbase != coin.IsCoinBase()) { if (!is_bip30_exception) { fClean = false; // transaction output mismatch } From ff9d484876ae2cc9403a0cc079b72bbf28404783 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Mon, 9 Dec 2024 13:30:04 +0100 Subject: [PATCH 2/3] coins,refactor: Unify Coin Serialization styles --- src/coins.h | 4 ++-- src/kernel/coinstats.cpp | 2 +- src/undo.h | 3 ++- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/coins.h b/src/coins.h index 0899e4c7360..a4069b43e6e 100644 --- a/src/coins.h +++ b/src/coins.h @@ -26,7 +26,7 @@ * A UTXO entry. * * Serialized format: - * - VARINT((coinbase ? 1 : 0) | (height << 1)) + * - VARINT((height << 1) | (coinbase ? 1 : 0)) * - the non-spent CTxOut (via TxOutCompression) */ class Coin @@ -61,7 +61,7 @@ public: template void Serialize(Stream &s) const { assert(!IsSpent()); - uint32_t code = nHeight * uint32_t{2} + fCoinBase; + uint32_t code{static_cast(nHeight << 1) | fCoinBase}; ::Serialize(s, VARINT(code)); ::Serialize(s, Using(out)); } diff --git a/src/kernel/coinstats.cpp b/src/kernel/coinstats.cpp index 81c496ab342..03ae6b100a3 100644 --- a/src/kernel/coinstats.cpp +++ b/src/kernel/coinstats.cpp @@ -51,7 +51,7 @@ template static void TxOutSer(T& ss, const COutPoint& outpoint, const Coin& coin) { ss << outpoint; - ss << static_cast((coin.nHeight << 1) + coin.fCoinBase); + ss << (static_cast(coin.nHeight << 1) | coin.fCoinBase); ss << coin.out; } diff --git a/src/undo.h b/src/undo.h index 1fb9ac06880..f66ea29410d 100644 --- a/src/undo.h +++ b/src/undo.h @@ -23,7 +23,8 @@ struct TxInUndoFormatter { template void Ser(Stream &s, const Coin& txout) { - ::Serialize(s, VARINT(txout.nHeight * uint32_t{2} + txout.fCoinBase )); + uint32_t nCode{static_cast(txout.nHeight << 1) | txout.fCoinBase}; + ::Serialize(s, VARINT(nCode)); if (txout.nHeight > 0) { // Required to maintain compatibility with older undo format. ::Serialize(s, (unsigned char)0); From 72ec3882447d489ab8f708b9e3ff2ea2422401c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Mon, 9 Dec 2024 14:02:49 +0100 Subject: [PATCH 3/3] rpc: Fix utxo_size_inc(_actual) calculation `nHeight` and `fCoinBase` are stored in `Coin` as bitfields and are serialized as a single 32 bit number, so the UTXO overhead calculation shouldn't include an additional boolean --- src/rpc/blockchain.cpp | 4 ++-- test/functional/data/rpc_getblockstats.json | 12 ++++++------ test/functional/rpc_getblockstats.py | 6 +++--- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 52440464d82..d6553ddbf49 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1828,8 +1828,8 @@ static inline bool SetHasKeys(const std::set& set, const Tk& key, const Args& return (set.count(key) != 0) || SetHasKeys(set, args...); } -// outpoint (needed for the utxo index) + nHeight + fCoinBase -static constexpr size_t PER_UTXO_OVERHEAD = sizeof(COutPoint) + sizeof(uint32_t) + sizeof(bool); +// outpoint (needed for the utxo index) + nHeight|fCoinBase +static constexpr size_t PER_UTXO_OVERHEAD = sizeof(COutPoint) + sizeof(uint32_t); static RPCHelpMan getblockstats() { diff --git a/test/functional/data/rpc_getblockstats.json b/test/functional/data/rpc_getblockstats.json index 7d7460aacc8..e0c0eb39fbc 100644 --- a/test/functional/data/rpc_getblockstats.json +++ b/test/functional/data/rpc_getblockstats.json @@ -143,8 +143,8 @@ "txs": 1, "utxo_increase": 2, "utxo_increase_actual": 1, - "utxo_size_inc": 163, - "utxo_size_inc_actual": 75 + "utxo_size_inc": 161, + "utxo_size_inc_actual": 74 }, { "avgfee": 4440, @@ -182,8 +182,8 @@ "txs": 2, "utxo_increase": 3, "utxo_increase_actual": 2, - "utxo_size_inc": 235, - "utxo_size_inc_actual": 147 + "utxo_size_inc": 232, + "utxo_size_inc_actual": 145 }, { "avgfee": 21390, @@ -221,8 +221,8 @@ "txs": 5, "utxo_increase": 6, "utxo_increase_actual": 4, - "utxo_size_inc": 441, - "utxo_size_inc_actual": 300 + "utxo_size_inc": 435, + "utxo_size_inc_actual": 296 } ] } \ No newline at end of file diff --git a/test/functional/rpc_getblockstats.py b/test/functional/rpc_getblockstats.py index 002763201a5..11dd1cdae1f 100755 --- a/test/functional/rpc_getblockstats.py +++ b/test/functional/rpc_getblockstats.py @@ -171,16 +171,16 @@ class GetblockstatsTest(BitcoinTestFramework): genesis_stats = self.nodes[0].getblockstats(0) assert_equal(genesis_stats["blockhash"], "0f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206") assert_equal(genesis_stats["utxo_increase"], 1) - assert_equal(genesis_stats["utxo_size_inc"], 117) + assert_equal(genesis_stats["utxo_size_inc"], 116) assert_equal(genesis_stats["utxo_increase_actual"], 0) assert_equal(genesis_stats["utxo_size_inc_actual"], 0) self.log.info('Test tip including OP_RETURN') tip_stats = self.nodes[0].getblockstats(tip) assert_equal(tip_stats["utxo_increase"], 6) - assert_equal(tip_stats["utxo_size_inc"], 441) + assert_equal(tip_stats["utxo_size_inc"], 435) assert_equal(tip_stats["utxo_increase_actual"], 4) - assert_equal(tip_stats["utxo_size_inc_actual"], 300) + assert_equal(tip_stats["utxo_size_inc_actual"], 296) self.log.info("Test when only header is known") block = self.generateblock(self.nodes[0], output="raw(55)", transactions=[], submit=False)