Merge bitcoin/bitcoin#28878: Remove version field from GetSerializeSize

83986f4 Include version.h in fewer places (Anthony Towns)
c7b61fd Convert some CDataStream to DataStream (Anthony Towns)
1410d30 serialize: Drop useless version param from GetSerializeSize() (Anthony Towns)
bf574a7 serialize: drop GetSerializeSizeMany (Anthony Towns)
efa9eb6 serialize: Drop nVersion from [C]SizeComputer (Anthony Towns)

Pull request description:

  Drops the version field from `GetSerializeSize()`, simplifying the code in various places. Also drop `GetSerializeSizeMany()` (as just removing the version parameter could result in silent bugs) and remove unnecessary instances of `#include <version.h>`.

ACKs for top commit:
  maflcko:
    ACK 83986f4 📒
  theuni:
    ACK 83986f4.

Tree-SHA512: 36617b6dfbb1b4b0afbf673e905525fc6d623d3f568d3f86e3b9d4f69820db97d099e83a88007bfff881f731ddca6755ebf1549e8d8a7762437dfadbf434c62e

src/bench/checkblock.cpp

@@ -18,7 +18,7 @@
 
 static void DeserializeBlockTest(benchmark::Bench& bench)
 {
-    CDataStream stream(benchmark::data::block413567, SER_NETWORK, PROTOCOL_VERSION);
+    DataStream stream(benchmark::data::block413567);
     std::byte a{0};
     stream.write({&a, 1}); // Prevent compaction
 
@@ -32,7 +32,7 @@ static void DeserializeBlockTest(benchmark::Bench& bench)
 
 static void DeserializeAndCheckBlockTest(benchmark::Bench& bench)
 {
-    CDataStream stream(benchmark::data::block413567, SER_NETWORK, PROTOCOL_VERSION);
+    DataStream stream(benchmark::data::block413567);
     std::byte a{0};
     stream.write({&a, 1}); // Prevent compaction
 

src/bench/verify_script.cpp

@@ -61,7 +61,7 @@ static void VerifyScriptBench(benchmark::Bench& bench)
         assert(success);
 
 #if defined(HAVE_CONSENSUS_LIB)
-        CDataStream stream(SER_NETWORK, PROTOCOL_VERSION);
+        DataStream stream;
         stream << TX_WITH_WITNESS(txSpend);
         int csuccess = bitcoinconsensus_verify_script_with_amount(
             txCredit.vout[0].scriptPubKey.data(),

src/bitcoin-util.cpp

@@ -19,7 +19,6 @@
 #include <util/exception.h>
 #include <util/strencodings.h>
 #include <util/translation.h>
-#include <version.h>
 
 #include <atomic>
 #include <cstdio>

src/blockencodings.cpp

@@ -164,7 +164,7 @@ ReadStatus PartiallyDownloadedBlock::InitData(const CBlockHeaderAndShortTxIDs& c
             break;
     }
 
-    LogPrint(BCLog::CMPCTBLOCK, "Initialized PartiallyDownloadedBlock for block %s using a cmpctblock of size %lu\n", cmpctblock.header.GetHash().ToString(), GetSerializeSize(cmpctblock, PROTOCOL_VERSION));
+    LogPrint(BCLog::CMPCTBLOCK, "Initialized PartiallyDownloadedBlock for block %s using a cmpctblock of size %lu\n", cmpctblock.header.GetHash().ToString(), GetSerializeSize(cmpctblock));
 
     return READ_STATUS_OK;
 }

src/coins.cpp

@@ -8,7 +8,6 @@
 #include <logging.h>
 #include <random.h>
 #include <util/trace.h>
-#include <version.h>
 
 bool CCoinsView::GetCoin(const COutPoint &outpoint, Coin &coin) const { return false; }
 uint256 CCoinsView::GetBestBlock() const { return uint256(); }
@@ -339,7 +338,7 @@ void CCoinsViewCache::SanityCheck() const
     assert(recomputed_usage == cachedCoinsUsage);
 }
 
-static const size_t MIN_TRANSACTION_OUTPUT_WEIGHT = WITNESS_SCALE_FACTOR * ::GetSerializeSize(CTxOut(), PROTOCOL_VERSION);
+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;
 
 const Coin& AccessByTxid(const CCoinsViewCache& view, const uint256& txid)

src/consensus/validation.h

@@ -7,7 +7,6 @@
 #define BITCOIN_CONSENSUS_VALIDATION_H
 
 #include <string>
-#include <version.h>
 #include <consensus/consensus.h>
 #include <primitives/transaction.h>
 #include <primitives/block.h>

src/core_read.cpp

@@ -12,7 +12,6 @@
 #include <streams.h>
 #include <util/result.h>
 #include <util/strencodings.h>
-#include <version.h>
 
 #include <algorithm>
 #include <string>

src/external_signer.cpp

@@ -2,12 +2,14 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
+#include <external_signer.h>
+
 #include <chainparams.h>
 #include <common/run_command.h>
 #include <core_io.h>
 #include <psbt.h>
 #include <util/strencodings.h>
-#include <external_signer.h>
+#include <version.h>
 
 #include <algorithm>
 #include <stdexcept>

src/hash.h

@@ -14,7 +14,6 @@
 #include <serialize.h>
 #include <span.h>
 #include <uint256.h>
-#include <version.h>
 
 #include <string>
 #include <vector>

src/index/blockfilterindex.cpp

@@ -174,8 +174,8 @@ size_t BlockFilterIndex::WriteFilterToDisk(FlatFilePos& pos, const BlockFilter&
     assert(filter.GetFilterType() == GetFilterType());
 
     size_t data_size =
-        GetSerializeSize(filter.GetBlockHash(), CLIENT_VERSION) +
-        GetSerializeSize(filter.GetEncodedFilter(), CLIENT_VERSION);
+        GetSerializeSize(filter.GetBlockHash()) +
+        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/kernel/coinstats.cpp

@@ -21,7 +21,6 @@
 #include <util/check.h>
 #include <util/overflow.h>
 #include <validation.h>
-#include <version.h>
 
 #include <cassert>
 #include <iosfwd>

src/net.h

@@ -30,6 +30,7 @@
 #include <util/check.h>
 #include <util/sock.h>
 #include <util/threadinterrupt.h>
+#include <version.h>
 
 #include <atomic>
 #include <condition_variable>

src/node/blockstorage.cpp

@@ -662,7 +662,7 @@ bool BlockManager::UndoWriteToDisk(const CBlockUndo& blockundo, FlatFilePos& pos
     }
 
     // Write index header
-    unsigned int nSize = GetSerializeSize(blockundo, CLIENT_VERSION);
+    unsigned int nSize = GetSerializeSize(blockundo);
     fileout << GetParams().MessageStart() << nSize;
 
     // Write undo data
@@ -979,7 +979,7 @@ bool BlockManager::WriteUndoDataForBlock(const CBlockUndo& blockundo, BlockValid
     // Write undo information to disk
     if (block.GetUndoPos().IsNull()) {
         FlatFilePos _pos;
-        if (!FindUndoPos(state, block.nFile, _pos, ::GetSerializeSize(blockundo, CLIENT_VERSION) + 40)) {
+        if (!FindUndoPos(state, block.nFile, _pos, ::GetSerializeSize(blockundo) + 40)) {
             return error("ConnectBlock(): FindUndoPos failed");
         }
         if (!UndoWriteToDisk(blockundo, _pos, block.pprev->GetBlockHash())) {

src/primitives/transaction.cpp

@@ -13,7 +13,6 @@
 #include <uint256.h>
 #include <util/strencodings.h>
 #include <util/transaction_identifier.h>
-#include <version.h>
 
 #include <cassert>
 #include <stdexcept>

src/psbt.cpp

@@ -8,6 +8,7 @@
 #include <script/signingprovider.h>
 #include <util/check.h>
 #include <util/strencodings.h>
+#include <version.h>
 
 
 PartiallySignedTransaction::PartiallySignedTransaction(const CMutableTransaction& tx) : tx(tx)

src/psbt.h

@@ -89,7 +89,9 @@ struct PSBTProprietary
 template<typename Stream, typename... X>
 void SerializeToVector(Stream& s, const X&... args)
 {
-    WriteCompactSize(s, GetSerializeSizeMany(s.GetVersion(), args...));
+    SizeComputer sizecomp;
+    SerializeMany(sizecomp, args...);
+    WriteCompactSize(s, sizecomp.size());
     SerializeMany(s, args...);
 }
 

src/rest.cpp

@@ -28,7 +28,6 @@
 #include <util/check.h>
 #include <util/strencodings.h>
 #include <validation.h>
-#include <version.h>
 
 #include <any>
 #include <string>

src/rpc/blockchain.cpp

@@ -1861,7 +1861,7 @@ static RPCHelpMan getblockstats()
             for (const CTxOut& out : tx->vout) {
                 tx_total_out += out.nValue;
 
-                size_t out_size = GetSerializeSize(out, PROTOCOL_VERSION) + PER_UTXO_OVERHEAD;
+                size_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
@@ -1913,7 +1913,7 @@ static RPCHelpMan getblockstats()
                 const CTxOut& prevoutput = coin.out;
 
                 tx_total_in += prevoutput.nValue;
-                size_t prevout_size = GetSerializeSize(prevoutput, PROTOCOL_VERSION) + PER_UTXO_OVERHEAD;
+                size_t prevout_size = GetSerializeSize(prevoutput) + PER_UTXO_OVERHEAD;
                 utxo_size_inc -= prevout_size;
                 utxo_size_inc_actual -= prevout_size;
             }

src/rpc/rawtransaction.cpp

@@ -40,6 +40,7 @@
 #include <util/vector.h>
 #include <validation.h>
 #include <validationinterface.h>
+#include <version.h>
 
 #include <numeric>
 #include <stdint.h>

src/script/bitcoinconsensus.cpp

@@ -8,7 +8,6 @@
 #include <primitives/transaction.h>
 #include <pubkey.h>
 #include <script/interpreter.h>
-#include <version.h>
 
 namespace {
 

src/script/interpreter.cpp

@@ -1934,7 +1934,7 @@ static bool VerifyWitnessProgram(const CScriptWitness& witness, int witversion,
             if ((control[0] & TAPROOT_LEAF_MASK) == TAPROOT_LEAF_TAPSCRIPT) {
                 // Tapscript (leaf version 0xc0)
                 exec_script = CScript(script.begin(), script.end());
-                execdata.m_validation_weight_left = ::GetSerializeSize(witness.stack, PROTOCOL_VERSION) + VALIDATION_WEIGHT_OFFSET;
+                execdata.m_validation_weight_left = ::GetSerializeSize(witness.stack) + VALIDATION_WEIGHT_OFFSET;
                 execdata.m_validation_weight_left_init = true;
                 return ExecuteWitnessScript(stack, exec_script, flags, SigVersion::TAPSCRIPT, checker, execdata, serror);
             }

src/script/sign.cpp

@@ -379,7 +379,7 @@ static bool SignTaproot(const SigningProvider& provider, const BaseSignatureCrea
             result_stack.emplace_back(std::begin(script), std::end(script)); // Push the script
             result_stack.push_back(*control_blocks.begin()); // Push the smallest control block
             if (smallest_result_stack.size() == 0 ||
-                GetSerializeSize(result_stack, PROTOCOL_VERSION) < GetSerializeSize(smallest_result_stack, PROTOCOL_VERSION)) {
+                GetSerializeSize(result_stack) < GetSerializeSize(smallest_result_stack)) {
                 smallest_result_stack = std::move(result_stack);
             }
         }

src/serialize.h

@@ -124,7 +124,7 @@ template<typename Stream> inline uint64_t ser_readdata64(Stream &s)
 // i.e. anything that supports .read(Span<std::byte>) and .write(Span<const std::byte>)
 //
 
-class CSizeComputer;
+class SizeComputer;
 
 enum
 {
@@ -324,7 +324,7 @@ constexpr inline unsigned int GetSizeOfCompactSize(uint64_t nSize)
     else                         return sizeof(unsigned char) + sizeof(uint64_t);
 }
 
-inline void WriteCompactSize(CSizeComputer& os, uint64_t nSize);
+inline void WriteCompactSize(SizeComputer& os, uint64_t nSize);
 
 template<typename Stream>
 void WriteCompactSize(Stream& os, uint64_t nSize)
@@ -450,7 +450,7 @@ inline unsigned int GetSizeOfVarInt(I n)
 }
 
 template<typename I>
-inline void WriteVarInt(CSizeComputer& os, I n);
+inline void WriteVarInt(SizeComputer& os, I n);
 
 template<typename Stream, VarIntMode Mode, typename I>
 void WriteVarInt(Stream& os, I n)
@@ -1070,22 +1070,21 @@ struct ActionUnserialize {
 /* ::GetSerializeSize implementations
  *
  * Computing the serialized size of objects is done through a special stream
- * object of type CSizeComputer, which only records the number of bytes written
+ * object of type SizeComputer, which only records the number of bytes written
  * to it.
  *
  * If your Serialize or SerializationOp method has non-trivial overhead for
  * serialization, it may be worthwhile to implement a specialized version for
- * CSizeComputer, which uses the s.seek() method to record bytes that would
+ * SizeComputer, which uses the s.seek() method to record bytes that would
  * be written instead.
  */
-class CSizeComputer
+class SizeComputer
 {
 protected:
     size_t nSize{0};
 
-    const int nVersion;
 public:
-    explicit CSizeComputer(int nVersionIn) : nVersion(nVersionIn) {}
+    SizeComputer() {}
 
     void write(Span<const std::byte> src)
     {
@@ -1099,7 +1098,7 @@ class CSizeComputer
     }
 
     template<typename T>
-    CSizeComputer& operator<<(const T& obj)
+    SizeComputer& operator<<(const T& obj)
     {
         ::Serialize(*this, obj);
         return (*this);
@@ -1108,33 +1107,23 @@ class CSizeComputer
     size_t size() const {
         return nSize;
     }
-
-    int GetVersion() const { return nVersion; }
 };
 
 template<typename I>
-inline void WriteVarInt(CSizeComputer &s, I n)
+inline void WriteVarInt(SizeComputer &s, I n)
 {
     s.seek(GetSizeOfVarInt<I>(n));
 }
 
-inline void WriteCompactSize(CSizeComputer &s, uint64_t nSize)
+inline void WriteCompactSize(SizeComputer &s, uint64_t nSize)
 {
     s.seek(GetSizeOfCompactSize(nSize));
 }
 
 template <typename T>
-size_t GetSerializeSize(const T& t, int nVersion = 0)
-{
-    return (CSizeComputer(nVersion) << t).size();
-}
-
-template <typename... T>
-size_t GetSerializeSizeMany(int nVersion, const T&... t)
+size_t GetSerializeSize(const T& t)
 {
-    CSizeComputer sc(nVersion);
-    SerializeMany(sc, t...);
-    return sc.size();
+    return (SizeComputer() << t).size();
 }
 
 /** Wrapper that overrides the GetParams() function of a stream (and hides GetVersion/GetType). */

src/signet.cpp

@@ -22,6 +22,7 @@
 #include <streams.h>
 #include <uint256.h>
 #include <util/strencodings.h>
+#include <version.h>
 
 static constexpr uint8_t SIGNET_HEADER[4] = {0xec, 0xc7, 0xda, 0xa2};
 

src/test/flatfile_tests.cpp

@@ -40,7 +40,7 @@ BOOST_AUTO_TEST_CASE(flatfile_open)
                       "lost if a trusted third party is still required to prevent double-spending.");
 
     size_t pos1 = 0;
-    size_t pos2 = pos1 + GetSerializeSize(line1, CLIENT_VERSION);
+    size_t pos2 = pos1 + GetSerializeSize(line1);
 
     // Write first line to file.
     {

src/test/fuzz/miniscript.cpp

@@ -1121,7 +1121,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, PROTOCOL_VERSION) - GetSizeOfCompactSize(stack_nonmal.size());
+        const size_t wit_size = GetSerializeSize(stack_nonmal) - GetSizeOfCompactSize(stack_nonmal.size());
         assert(wit_size <= *node->GetWitnessSize());
 
         // Test non-malleable satisfaction.

src/test/miniscript_tests.cpp

@@ -369,7 +369,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, PROTOCOL_VERSION) - GetSizeOfCompactSize(witness_nonmal.stack.size());
+            const size_t wit_size = GetSerializeSize(witness_nonmal.stack) - GetSizeOfCompactSize(witness_nonmal.stack.size());
             SatisfactionToWitness(converter.MsContext(), witness_nonmal, script, builder);
 
             if (nonmal_success) {