Merge bitcoin/bitcoin#33724: refactor: Return uint64_t from GetSerializeSize

fa6c0bedd3 refactor: Return uint64_t from GetSerializeSize (MarcoFalke)
fad0c8680e refactor: Use uint64_t over size_t for serialized-size values (MarcoFalke)
fa4f388fc9 refactor: Use fixed size ints over (un)signed ints for serialized values (MarcoFalke)
fa01f38e53 move-only: Move CBlockFileInfo to kernel namespace (MarcoFalke)
fa2bbc9e4c refactor: [rpc] Remove cast when reporting serialized size (MarcoFalke)
fa364af89b test: Remove outdated comment (MarcoFalke)

Pull request description:

  Consensus code should arrive at the same conclusion, regardless of the architecture it runs on. Using architecture-specific types such as `size_t` can lead to issues, such as the low-severity [CVE-2025-46597](https://bitcoincore.org/en/2025/10/24/disclose-cve-2025-46597/).

  The CVE was already worked around, but it may be good to still fix the underlying issue.

  Fixes https://github.com/bitcoin/bitcoin/issues/33709 with a few refactors to use explicit fixed-sized integer types in serialization-size related code and concluding with a refactor to return `uint64_t` from `GetSerializeSize`. The refactors should not change any behavior, because the CVE was already worked around.

ACKs for top commit:
  Crypt-iQ:
    crACK fa6c0bedd3
  l0rinc:
    ACK fa6c0bedd3
  laanwj:
    Code review ACK fa6c0bedd3

Tree-SHA512: f45057bd86fb46011e4cb3edf0dc607057d72ed869fd6ad636562111ae80fea233b2fc45c34b02256331028359a9c3f4fa73e9b882b225bdc089d00becd0195e

src/chain.cpp

@@ -6,12 +6,6 @@
 #include <chain.h>
 #include <tinyformat.h>
 #include <util/check.h>
-#include <util/time.h>
-
-std::string CBlockFileInfo::ToString() const
-{
-    return strprintf("CBlockFileInfo(blocks=%u, size=%u, heights=%u...%u, time=%s...%s)", nBlocks, nSize, nHeightFirst, nHeightLast, FormatISO8601Date(nTimeFirst), FormatISO8601Date(nTimeLast));
-}
 
 std::string CBlockIndex::ToString() const
 {

src/chain.h

@@ -47,47 +47,6 @@ static constexpr int32_t SEQ_ID_INIT_FROM_DISK = 1;
  */
 static constexpr int64_t MAX_BLOCK_TIME_GAP = 90 * 60;
 
-class CBlockFileInfo
-{
-public:
-    unsigned int nBlocks{};      //!< number of blocks stored in file
-    unsigned int nSize{};        //!< number of used bytes of block file
-    unsigned int nUndoSize{};    //!< number of used bytes in the undo file
-    unsigned int nHeightFirst{}; //!< lowest height of block in file
-    unsigned int nHeightLast{};  //!< highest height of block in file
-    uint64_t nTimeFirst{};       //!< earliest time of block in file
-    uint64_t nTimeLast{};        //!< latest time of block in file
-
-    SERIALIZE_METHODS(CBlockFileInfo, obj)
-    {
-        READWRITE(VARINT(obj.nBlocks));
-        READWRITE(VARINT(obj.nSize));
-        READWRITE(VARINT(obj.nUndoSize));
-        READWRITE(VARINT(obj.nHeightFirst));
-        READWRITE(VARINT(obj.nHeightLast));
-        READWRITE(VARINT(obj.nTimeFirst));
-        READWRITE(VARINT(obj.nTimeLast));
-    }
-
-    CBlockFileInfo() = default;
-
-    std::string ToString() const;
-
-    /** update statistics (does not update nSize) */
-    void AddBlock(unsigned int nHeightIn, uint64_t nTimeIn)
-    {
-        if (nBlocks == 0 || nHeightFirst > nHeightIn)
-            nHeightFirst = nHeightIn;
-        if (nBlocks == 0 || nTimeFirst > nTimeIn)
-            nTimeFirst = nTimeIn;
-        nBlocks++;
-        if (nHeightIn > nHeightLast)
-            nHeightLast = nHeightIn;
-        if (nTimeIn > nTimeLast)
-            nTimeLast = nTimeIn;
-    }
-};
-
 enum BlockStatus : uint32_t {
     //! Unused.
     BLOCK_VALID_UNKNOWN      =    0,

src/coins.cpp

@@ -349,8 +349,8 @@ void CCoinsViewCache::SanityCheck() const
     assert(recomputed_usage == cachedCoinsUsage);
 }
 
-static const size_t MIN_TRANSACTION_OUTPUT_WEIGHT = WITNESS_SCALE_FACTOR * ::GetSerializeSize(CTxOut());
-static const size_t MAX_OUTPUTS_PER_BLOCK = MAX_BLOCK_WEIGHT / MIN_TRANSACTION_OUTPUT_WEIGHT;
+static const uint64_t MIN_TRANSACTION_OUTPUT_WEIGHT{WITNESS_SCALE_FACTOR * ::GetSerializeSize(CTxOut())};
+static const uint64_t MAX_OUTPUTS_PER_BLOCK{MAX_BLOCK_WEIGHT / MIN_TRANSACTION_OUTPUT_WEIGHT};
 
 const Coin& AccessByTxid(const CCoinsViewCache& view, const Txid& txid)
 {

src/flatfile.h

@@ -13,16 +13,16 @@
 
 struct FlatFilePos
 {
-    int nFile{-1};
-    unsigned int nPos{0};
+    int32_t nFile{-1};
+    uint32_t nPos{0};
 
     SERIALIZE_METHODS(FlatFilePos, obj) { READWRITE(VARINT_MODE(obj.nFile, VarIntMode::NONNEGATIVE_SIGNED), VARINT(obj.nPos)); }
 
     FlatFilePos() = default;
 
-    FlatFilePos(int nFileIn, unsigned int nPosIn) :
-        nFile(nFileIn),
-        nPos(nPosIn)
+    FlatFilePos(int32_t nFileIn, uint32_t nPosIn)
+        : nFile{nFileIn},
+          nPos{nPosIn}
     {}
 
     friend bool operator==(const FlatFilePos &a, const FlatFilePos &b) {

src/index/blockfilterindex.cpp

@@ -222,9 +222,9 @@ size_t BlockFilterIndex::WriteFilterToDisk(FlatFilePos& pos, const BlockFilter&
 {
     assert(filter.GetFilterType() == GetFilterType());
 
-    size_t data_size =
+    uint64_t data_size{
         GetSerializeSize(filter.GetBlockHash()) +
-        GetSerializeSize(filter.GetEncodedFilter());
+        GetSerializeSize(filter.GetEncodedFilter())};
 
     // If writing the filter would overflow the file, flush and move to the next one.
     if (pos.nPos + data_size > MAX_FLTR_FILE_SIZE) {

src/index/disktxpos.h

@@ -10,14 +10,14 @@
 
 struct CDiskTxPos : public FlatFilePos
 {
-    unsigned int nTxOffset{0}; // after header
+    uint32_t nTxOffset{0}; // after header
 
     SERIALIZE_METHODS(CDiskTxPos, obj)
     {
         READWRITE(AsBase<FlatFilePos>(obj), VARINT(obj.nTxOffset));
     }
 
-    CDiskTxPos(const FlatFilePos &blockIn, unsigned int nTxOffsetIn) : FlatFilePos(blockIn.nFile, blockIn.nPos), nTxOffset(nTxOffsetIn) {
+    CDiskTxPos(const FlatFilePos& blockIn, uint32_t nTxOffsetIn) : FlatFilePos{blockIn.nFile, blockIn.nPos}, nTxOffset{nTxOffsetIn} {
     }
 
     CDiskTxPos() = default;

src/node/blockstorage.cpp

@@ -35,6 +35,7 @@
 #include <util/signalinterrupt.h>
 #include <util/strencodings.h>
 #include <util/syserror.h>
+#include <util/time.h>
 #include <util/translation.h>
 #include <validation.h>
 
@@ -151,6 +152,11 @@ bool BlockTreeDB::LoadBlockIndexGuts(const Consensus::Params& consensusParams, s
 
     return true;
 }
+
+std::string CBlockFileInfo::ToString() const
+{
+    return strprintf("CBlockFileInfo(blocks=%u, size=%u, heights=%u...%u, time=%s...%s)", nBlocks, nSize, nHeightFirst, nHeightLast, FormatISO8601Date(nTimeFirst), FormatISO8601Date(nTimeLast));
+}
 } // namespace kernel
 
 namespace node {

src/node/blockstorage.h

@@ -47,6 +47,47 @@ class SignalInterrupt;
 } // namespace util
 
 namespace kernel {
+class CBlockFileInfo
+{
+public:
+    uint32_t nBlocks{};      //!< number of blocks stored in file
+    uint32_t nSize{};        //!< number of used bytes of block file
+    uint32_t nUndoSize{};    //!< number of used bytes in the undo file
+    uint32_t nHeightFirst{}; //!< lowest height of block in file
+    uint32_t nHeightLast{};  //!< highest height of block in file
+    uint64_t nTimeFirst{};   //!< earliest time of block in file
+    uint64_t nTimeLast{};    //!< latest time of block in file
+
+    SERIALIZE_METHODS(CBlockFileInfo, obj)
+    {
+        READWRITE(VARINT(obj.nBlocks));
+        READWRITE(VARINT(obj.nSize));
+        READWRITE(VARINT(obj.nUndoSize));
+        READWRITE(VARINT(obj.nHeightFirst));
+        READWRITE(VARINT(obj.nHeightLast));
+        READWRITE(VARINT(obj.nTimeFirst));
+        READWRITE(VARINT(obj.nTimeLast));
+    }
+
+    CBlockFileInfo() = default;
+
+    std::string ToString() const;
+
+    /** update statistics (does not update nSize) */
+    void AddBlock(unsigned int nHeightIn, uint64_t nTimeIn)
+    {
+        if (nBlocks == 0 || nHeightFirst > nHeightIn)
+            nHeightFirst = nHeightIn;
+        if (nBlocks == 0 || nTimeFirst > nTimeIn)
+            nTimeFirst = nTimeIn;
+        nBlocks++;
+        if (nHeightIn > nHeightLast)
+            nHeightLast = nHeightIn;
+        if (nTimeIn > nTimeLast)
+            nTimeLast = nTimeIn;
+    }
+};
+
 /** Access to the block database (blocks/index/) */
 class BlockTreeDB : public CDBWrapper
 {
@@ -65,6 +106,7 @@ public:
 } // namespace kernel
 
 namespace node {
+using kernel::CBlockFileInfo;
 using kernel::BlockTreeDB;
 
 /** The pre-allocation chunk size for blk?????.dat files (since 0.8) */

src/policy/policy.cpp

@@ -42,7 +42,7 @@ CAmount GetDustThreshold(const CTxOut& txout, const CFeeRate& dustRelayFeeIn)
     if (txout.scriptPubKey.IsUnspendable())
         return 0;
 
-    size_t nSize = GetSerializeSize(txout);
+    uint64_t nSize{GetSerializeSize(txout)};
     int witnessversion = 0;
     std::vector<unsigned char> witnessprogram;
 

src/rpc/blockchain.cpp

@@ -185,9 +185,9 @@ UniValue blockToJSON(BlockManager& blockman, const CBlock& block, const CBlockIn
 {
     UniValue result = blockheaderToJSON(tip, blockindex, pow_limit);
 
-    result.pushKV("strippedsize", (int)::GetSerializeSize(TX_NO_WITNESS(block)));
-    result.pushKV("size", (int)::GetSerializeSize(TX_WITH_WITNESS(block)));
-    result.pushKV("weight", (int)::GetBlockWeight(block));
+    result.pushKV("strippedsize", ::GetSerializeSize(TX_NO_WITNESS(block)));
+    result.pushKV("size", ::GetSerializeSize(TX_WITH_WITNESS(block)));
+    result.pushKV("weight", ::GetBlockWeight(block));
     UniValue txs(UniValue::VARR);
     txs.reserve(block.vtx.size());
 
@@ -2033,7 +2033,7 @@ static RPCHelpMan getblockstats()
             for (const CTxOut& out : tx->vout) {
                 tx_total_out += out.nValue;
 
-                size_t out_size = GetSerializeSize(out) + PER_UTXO_OVERHEAD;
+                uint64_t out_size{GetSerializeSize(out) + PER_UTXO_OVERHEAD};
                 utxo_size_inc += out_size;
 
                 // The Genesis block and the repeated BIP30 block coinbases don't change the UTXO
@@ -2085,7 +2085,7 @@ static RPCHelpMan getblockstats()
                 const CTxOut& prevoutput = coin.out;
 
                 tx_total_in += prevoutput.nValue;
-                size_t prevout_size = GetSerializeSize(prevoutput) + PER_UTXO_OVERHEAD;
+                uint64_t prevout_size{GetSerializeSize(prevoutput) + PER_UTXO_OVERHEAD};
                 utxo_size_inc -= prevout_size;
                 utxo_size_inc_actual -= prevout_size;
             }

src/serialize.h

@@ -1051,31 +1051,32 @@ struct ActionUnserialize {
 class SizeComputer
 {
 protected:
-    size_t nSize{0};
+    uint64_t m_size{0};
 
 public:
     SizeComputer() = default;
 
     void write(std::span<const std::byte> src)
     {
-        this->nSize += src.size();
+        m_size += src.size();
     }
 
-    /** Pretend _nSize bytes are written, without specifying them. */
-    void seek(size_t _nSize)
+    /** Pretend this many bytes are written, without specifying them. */
+    void seek(uint64_t num)
     {
-        this->nSize += _nSize;
+        m_size += num;
     }
 
-    template<typename T>
+    template <typename T>
     SizeComputer& operator<<(const T& obj)
     {
         ::Serialize(*this, obj);
-        return (*this);
+        return *this;
     }
 
-    size_t size() const {
-        return nSize;
+    uint64_t size() const
+    {
+        return m_size;
     }
 };
 
@@ -1091,7 +1092,7 @@ inline void WriteCompactSize(SizeComputer &s, uint64_t nSize)
 }
 
 template <typename T>
-size_t GetSerializeSize(const T& t)
+uint64_t GetSerializeSize(const T& t)
 {
     return (SizeComputer() << t).size();
 }

src/test/arith_uint256_tests.cpp

@@ -368,7 +368,7 @@ static bool almostEqual(double d1, double d2)
     return fabs(d1-d2) <= 4*fabs(d1)*std::numeric_limits<double>::epsilon();
 }
 
-BOOST_AUTO_TEST_CASE(methods) // GetHex operator= size() GetLow64 GetSerializeSize, Serialize, Unserialize
+BOOST_AUTO_TEST_CASE(methods)
 {
     BOOST_CHECK(R1L.GetHex() == R1L.ToString());
     BOOST_CHECK(R2L.GetHex() == R2L.ToString());

src/test/blockmanager_tests.cpp

@@ -17,6 +17,7 @@
 #include <test/util/logging.h>
 #include <test/util/setup_common.h>
 
+using kernel::CBlockFileInfo;
 using node::STORAGE_HEADER_BYTES;
 using node::BlockManager;
 using node::KernelNotifications;

src/test/flatfile_tests.cpp

@@ -39,8 +39,8 @@ BOOST_AUTO_TEST_CASE(flatfile_open)
     std::string line2("Digital signatures provide part of the solution, but the main benefits are "
                       "lost if a trusted third party is still required to prevent double-spending.");
 
-    size_t pos1 = 0;
-    size_t pos2 = pos1 + GetSerializeSize(line1);
+    uint64_t pos1{0};
+    uint64_t pos2{pos1 + GetSerializeSize(line1)};
 
     // Write first line to file.
     {

src/test/fuzz/block_index.cpp

@@ -12,6 +12,8 @@
 #include <txdb.h>
 #include <validation.h>
 
+using kernel::CBlockFileInfo;
+
 namespace {
 
 const BasicTestingSetup* g_setup;

src/test/fuzz/deserialize.cpp

@@ -17,6 +17,7 @@
 #include <net.h>
 #include <netbase.h>
 #include <netgroup.h>
+#include <node/blockstorage.h>
 #include <node/utxo_snapshot.h>
 #include <primitives/block.h>
 #include <protocol.h>
@@ -34,6 +35,7 @@
 #include <optional>
 #include <stdexcept>
 
+using kernel::CBlockFileInfo;
 using node::SnapshotMetadata;
 
 namespace {

src/test/fuzz/miniscript.cpp

@@ -1120,7 +1120,7 @@ void TestNode(const MsCtx script_ctx, const NodeRef& node, FuzzedDataProvider& p
         assert(mal_success);
         assert(stack_nonmal == stack_mal);
         // Compute witness size (excluding script push, control block, and witness count encoding).
-        const size_t wit_size = GetSerializeSize(stack_nonmal) - GetSizeOfCompactSize(stack_nonmal.size());
+        const uint64_t wit_size{GetSerializeSize(stack_nonmal) - GetSizeOfCompactSize(stack_nonmal.size())};
         assert(wit_size <= *node->GetWitnessSize());
 
         // Test non-malleable satisfaction.

src/test/miner_tests.cpp

@@ -188,7 +188,7 @@ void MinerTestingSetup::TestPackageSelection(const CScript& scriptPubKey, const
     tx.vout[0].nValue = 5000000000LL - 1000 - 50000; // 0 fee
     Txid hashFreeTx = tx.GetHash();
     AddToMempool(tx_mempool, entry.Fee(0).FromTx(tx));
-    size_t freeTxSize = ::GetSerializeSize(TX_WITH_WITNESS(tx));
+    uint64_t freeTxSize{::GetSerializeSize(TX_WITH_WITNESS(tx))};
 
     // Calculate a fee on child transaction that will put the package just
     // below the block min tx fee (assuming 1 child tx of the same size).

src/test/miniscript_tests.cpp

@@ -372,7 +372,7 @@ void TestSatisfy(const KeyConverter& converter, const std::string& testcase, con
             CScriptWitness witness_nonmal;
             const bool nonmal_success = node->Satisfy(satisfier, witness_nonmal.stack, true) == miniscript::Availability::YES;
             // Compute witness size (excluding script push, control block, and witness count encoding).
-            const size_t wit_size = GetSerializeSize(witness_nonmal.stack) - GetSizeOfCompactSize(witness_nonmal.stack.size());
+            const uint64_t wit_size{GetSerializeSize(witness_nonmal.stack) - GetSizeOfCompactSize(witness_nonmal.stack.size())};
             SatisfactionToWitness(converter.MsContext(), witness_nonmal, script, builder);
 
             if (nonmal_success) {

src/wallet/spend.cpp

@@ -1034,7 +1034,7 @@ void DiscourageFeeSniping(CMutableTransaction& tx, FastRandomContext& rng_fast,
     }
 }
 
-size_t GetSerializeSizeForRecipient(const CRecipient& recipient)
+uint64_t GetSerializeSizeForRecipient(const CRecipient& recipient)
 {
     return ::GetSerializeSize(CTxOut(recipient.nAmount, GetScriptForDestination(recipient.dest)));
 }