Skip to content

Commit 48b8910

Browse files
fanquakefanquake
committed
Merge bitcoin/bitcoin#28508: refactor: Remove SER_GETHASH, hard-code client version in CKeyPool serialize
fac29a0 Remove SER_GETHASH, hard-code client version in CKeyPool serialize (MarcoFalke) fa72f09 Remove CHashWriter type (MarcoFalke) fa4a9c0 Remove unused GetType() from OverrideStream, CVectorWriter, SpanReader (MarcoFalke) Pull request description: Removes a bunch of redundant, dead or duplicate code. Uses the idea from and finishes the idea bitcoin/bitcoin#28428 by theuni ACKs for top commit: ajtowns: ACK fac29a0 kevkevinpal: added one nit but otherwise ACK [fac29a0](bitcoin/bitcoin@fac29a0) Tree-SHA512: cc805e2f38e73869a6691fdb5da09fa48524506b87fc93f05d32c336ad3033425a2d7608e317decd3141fde3f084403b8de280396c0c39132336fe0f7510af9e
2 parents 0f9307c + fac29a0 commit 48b8910

19 files changed

+63
-95
lines changed

‎src/blockfilter.cpp

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,6 @@
1616
#include <util/golombrice.h>
1717
#include <util/string.h>
1818

19-
/// SerType used to serialize parameters in GCS filter encoding.
20-
static constexpr int GCS_SER_TYPE = SER_NETWORK;
21-
2219
/// Protocol version used to serialize parameters in GCS filter encoding.
2320
static constexpr int GCS_SER_VERSION = 0;
2421

@@ -52,7 +49,7 @@ GCSFilter::GCSFilter(const Params& params)
5249
GCSFilter::GCSFilter(const Params& params, std::vector<unsigned char> encoded_filter, bool skip_decode_check)
5350
: m_params(params), m_encoded(std::move(encoded_filter))
5451
{
55-
SpanReader stream{GCS_SER_TYPE, GCS_SER_VERSION, m_encoded};
52+
SpanReader stream{GCS_SER_VERSION, m_encoded};
5653

5754
uint64_t N = ReadCompactSize(stream);
5855
m_N = static_cast<uint32_t>(N);
@@ -84,7 +81,7 @@ GCSFilter::GCSFilter(const Params& params, const ElementSet& elements)
8481
}
8582
m_F = static_cast<uint64_t>(m_N) * static_cast<uint64_t>(m_params.m_M);
8683

87-
CVectorWriter stream(GCS_SER_TYPE, GCS_SER_VERSION, m_encoded, 0);
84+
CVectorWriter stream(GCS_SER_VERSION, m_encoded, 0);
8885

8986
WriteCompactSize(stream, m_N);
9087

@@ -106,7 +103,7 @@ GCSFilter::GCSFilter(const Params& params, const ElementSet& elements)
106103

107104
bool GCSFilter::MatchInternal(const uint64_t* element_hashes, size_t size) const
108105
{
109-
SpanReader stream{GCS_SER_TYPE, GCS_SER_VERSION, m_encoded};
106+
SpanReader stream{GCS_SER_VERSION, m_encoded};
110107

111108
// Seek forward by size of N
112109
uint64_t N = ReadCompactSize(stream);

‎src/hash.h

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -149,13 +149,11 @@ class HashWriter
149149
class CHashWriter : public HashWriter
150150
{
151151
private:
152-
const int nType;
153152
const int nVersion;
154153

155154
public:
156-
CHashWriter(int nTypeIn, int nVersionIn) : nType(nTypeIn), nVersion(nVersionIn) {}
155+
CHashWriter(int nVersionIn) : nVersion{nVersionIn} {}
157156

158-
int GetType() const { return nType; }
159157
int GetVersion() const { return nVersion; }
160158

161159
template<typename T>
@@ -223,15 +221,6 @@ class HashedSourceWriter : public HashWriter
223221
}
224222
};
225223

226-
/** Compute the 256-bit hash of an object's serialization. */
227-
template<typename T>
228-
uint256 SerializeHash(const T& obj, int nType=SER_GETHASH, int nVersion=PROTOCOL_VERSION)
229-
{
230-
CHashWriter ss(nType, nVersion);
231-
ss << obj;
232-
return ss.GetHash();
233-
}
234-
235224
/** Single-SHA256 a 32-byte input (represented as uint256). */
236225
[[nodiscard]] uint256 SHA256Uint256(const uint256& input);
237226

‎src/net.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -855,7 +855,7 @@ bool V1Transport::SetMessageToSend(CSerializedNetMsg& msg) noexcept
855855

856856
// serialize header
857857
m_header_to_send.clear();
858-
CVectorWriter{SER_NETWORK, INIT_PROTO_VERSION, m_header_to_send, 0, hdr};
858+
CVectorWriter{INIT_PROTO_VERSION, m_header_to_send, 0, hdr};
859859

860860
// update state
861861
m_message_to_send = std::move(msg);

‎src/netmessagemaker.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ class CNetMsgMaker
1919
{
2020
CSerializedNetMsg msg;
2121
msg.m_type = std::move(msg_type);
22-
CVectorWriter{ SER_NETWORK, nFlags | nVersion, msg.data, 0, std::forward<Args>(args)... };
22+
CVectorWriter{nFlags | nVersion, msg.data, 0, std::forward<Args>(args)...};
2323
return msg;
2424
}
2525

‎src/primitives/block.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010

1111
uint256 CBlockHeader::GetHash() const
1212
{
13-
return SerializeHash(*this);
13+
return (CHashWriter{PROTOCOL_VERSION} << *this).GetHash();
1414
}
1515

1616
std::string CBlock::ToString() const

‎src/primitives/transaction.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,20 +67,20 @@ CMutableTransaction::CMutableTransaction(const CTransaction& tx) : vin(tx.vin),
6767

6868
uint256 CMutableTransaction::GetHash() const
6969
{
70-
return SerializeHash(*this, SER_GETHASH, SERIALIZE_TRANSACTION_NO_WITNESS);
70+
return (CHashWriter{SERIALIZE_TRANSACTION_NO_WITNESS} << *this).GetHash();
7171
}
7272

7373
uint256 CTransaction::ComputeHash() const
7474
{
75-
return SerializeHash(*this, SER_GETHASH, SERIALIZE_TRANSACTION_NO_WITNESS);
75+
return (CHashWriter{SERIALIZE_TRANSACTION_NO_WITNESS} << *this).GetHash();
7676
}
7777

7878
uint256 CTransaction::ComputeWitnessHash() const
7979
{
8080
if (!HasWitness()) {
8181
return hash;
8282
}
83-
return SerializeHash(*this, SER_GETHASH, 0);
83+
return (CHashWriter{0} << *this).GetHash();
8484
}
8585

8686
CTransaction::CTransaction(const CMutableTransaction& tx) : vin(tx.vin), vout(tx.vout), nVersion(tx.nVersion), nLockTime(tx.nLockTime), hash{ComputeHash()}, m_witness_hash{ComputeWitnessHash()} {}

‎src/psbt.h

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ struct PSBTInput
226226
// Write the utxo
227227
if (non_witness_utxo) {
228228
SerializeToVector(s, CompactSizeWriter(PSBT_IN_NON_WITNESS_UTXO));
229-
OverrideStream<Stream> os(&s, s.GetType(), s.GetVersion() | SERIALIZE_TRANSACTION_NO_WITNESS);
229+
OverrideStream<Stream> os{&s, s.GetVersion() | SERIALIZE_TRANSACTION_NO_WITNESS};
230230
SerializeToVector(os, non_witness_utxo);
231231
}
232232
if (!witness_utxo.IsNull()) {
@@ -315,7 +315,7 @@ struct PSBTInput
315315
const auto& [leaf_hashes, origin] = leaf_origin;
316316
SerializeToVector(s, PSBT_IN_TAP_BIP32_DERIVATION, xonly);
317317
std::vector<unsigned char> value;
318-
CVectorWriter s_value(s.GetType(), s.GetVersion(), value, 0);
318+
CVectorWriter s_value{s.GetVersion(), value, 0};
319319
s_value << leaf_hashes;
320320
SerializeKeyOrigin(s_value, origin);
321321
s << value;
@@ -381,7 +381,7 @@ struct PSBTInput
381381
}
382382

383383
// Type is compact size uint at beginning of key
384-
SpanReader skey(s.GetType(), s.GetVersion(), key);
384+
SpanReader skey{s.GetVersion(), key};
385385
uint64_t type = ReadCompactSize(skey);
386386

387387
// Do stuff based on type
@@ -394,7 +394,7 @@ struct PSBTInput
394394
throw std::ios_base::failure("Non-witness utxo key is more than one byte type");
395395
}
396396
// Set the stream to unserialize with witness since this is always a valid network transaction
397-
OverrideStream<Stream> os(&s, s.GetType(), s.GetVersion() & ~SERIALIZE_TRANSACTION_NO_WITNESS);
397+
OverrideStream<Stream> os{&s, s.GetVersion() & ~SERIALIZE_TRANSACTION_NO_WITNESS};
398398
UnserializeFromVector(os, non_witness_utxo);
399399
break;
400400
}
@@ -590,7 +590,7 @@ struct PSBTInput
590590
} else if (key.size() != 65) {
591591
throw std::ios_base::failure("Input Taproot script signature key is not 65 bytes");
592592
}
593-
SpanReader s_key(s.GetType(), s.GetVersion(), Span{key}.subspan(1));
593+
SpanReader s_key{s.GetVersion(), Span{key}.subspan(1)};
594594
XOnlyPubKey xonly;
595595
uint256 hash;
596596
s_key >> xonly;
@@ -632,7 +632,7 @@ struct PSBTInput
632632
} else if (key.size() != 33) {
633633
throw std::ios_base::failure("Input Taproot BIP32 keypath key is not at 33 bytes");
634634
}
635-
SpanReader s_key(s.GetType(), s.GetVersion(), Span{key}.subspan(1));
635+
SpanReader s_key{s.GetVersion(), Span{key}.subspan(1)};
636636
XOnlyPubKey xonly;
637637
s_key >> xonly;
638638
std::set<uint256> leaf_hashes;
@@ -757,7 +757,7 @@ struct PSBTOutput
757757
if (!m_tap_tree.empty()) {
758758
SerializeToVector(s, PSBT_OUT_TAP_TREE);
759759
std::vector<unsigned char> value;
760-
CVectorWriter s_value(s.GetType(), s.GetVersion(), value, 0);
760+
CVectorWriter s_value{s.GetVersion(), value, 0};
761761
for (const auto& [depth, leaf_ver, script] : m_tap_tree) {
762762
s_value << depth;
763763
s_value << leaf_ver;
@@ -771,7 +771,7 @@ struct PSBTOutput
771771
const auto& [leaf_hashes, origin] = leaf;
772772
SerializeToVector(s, PSBT_OUT_TAP_BIP32_DERIVATION, xonly);
773773
std::vector<unsigned char> value;
774-
CVectorWriter s_value(s.GetType(), s.GetVersion(), value, 0);
774+
CVectorWriter s_value{s.GetVersion(), value, 0};
775775
s_value << leaf_hashes;
776776
SerializeKeyOrigin(s_value, origin);
777777
s << value;
@@ -807,7 +807,7 @@ struct PSBTOutput
807807
}
808808

809809
// Type is compact size uint at beginning of key
810-
SpanReader skey(s.GetType(), s.GetVersion(), key);
810+
SpanReader skey{s.GetVersion(), key};
811811
uint64_t type = ReadCompactSize(skey);
812812

813813
// Do stuff based on type
@@ -856,7 +856,7 @@ struct PSBTOutput
856856
}
857857
std::vector<unsigned char> tree_v;
858858
s >> tree_v;
859-
SpanReader s_tree(s.GetType(), s.GetVersion(), tree_v);
859+
SpanReader s_tree{s.GetVersion(), tree_v};
860860
if (s_tree.empty()) {
861861
throw std::ios_base::failure("Output Taproot tree must not be empty");
862862
}
@@ -984,7 +984,7 @@ struct PartiallySignedTransaction
984984
SerializeToVector(s, CompactSizeWriter(PSBT_GLOBAL_UNSIGNED_TX));
985985

986986
// Write serialized tx to a stream
987-
OverrideStream<Stream> os(&s, s.GetType(), s.GetVersion() | SERIALIZE_TRANSACTION_NO_WITNESS);
987+
OverrideStream<Stream> os{&s, s.GetVersion() | SERIALIZE_TRANSACTION_NO_WITNESS};
988988
SerializeToVector(os, *tx);
989989

990990
// Write xpubs
@@ -1061,7 +1061,7 @@ struct PartiallySignedTransaction
10611061
}
10621062

10631063
// Type is compact size uint at beginning of key
1064-
SpanReader skey(s.GetType(), s.GetVersion(), key);
1064+
SpanReader skey{s.GetVersion(), key};
10651065
uint64_t type = ReadCompactSize(skey);
10661066

10671067
// Do stuff based on type
@@ -1075,7 +1075,7 @@ struct PartiallySignedTransaction
10751075
}
10761076
CMutableTransaction mtx;
10771077
// Set the stream to serialize with non-witness since this should always be non-witness
1078-
OverrideStream<Stream> os(&s, s.GetType(), s.GetVersion() | SERIALIZE_TRANSACTION_NO_WITNESS);
1078+
OverrideStream<Stream> os{&s, s.GetVersion() | SERIALIZE_TRANSACTION_NO_WITNESS};
10791079
UnserializeFromVector(os, mtx);
10801080
tx = std::move(mtx);
10811081
// Make sure that all scriptSigs and scriptWitnesses are empty

‎src/serialize.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,6 @@ enum
131131
// primary actions
132132
SER_NETWORK = (1 << 0),
133133
SER_DISK = (1 << 1),
134-
SER_GETHASH = (1 << 2),
135134
};
136135

137136
/**

‎src/signet.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ std::optional<SignetTxs> SignetTxs::Create(const CBlock& block, const CScript& c
9898
// no signet solution -- allow this to support OP_TRUE as trivial block challenge
9999
} else {
100100
try {
101-
SpanReader v{SER_NETWORK, INIT_PROTO_VERSION, signet_solution};
101+
SpanReader v{INIT_PROTO_VERSION, signet_solution};
102102
v >> tx_spending.vin[0].scriptSig;
103103
v >> tx_spending.vin[0].scriptWitness.stack;
104104
if (!v.empty()) return std::nullopt; // extraneous data encountered
@@ -109,7 +109,7 @@ std::optional<SignetTxs> SignetTxs::Create(const CBlock& block, const CScript& c
109109
uint256 signet_merkle = ComputeModifiedMerkleRoot(modified_cb, block);
110110

111111
std::vector<uint8_t> block_data;
112-
CVectorWriter writer(SER_NETWORK, INIT_PROTO_VERSION, block_data, 0);
112+
CVectorWriter writer{INIT_PROTO_VERSION, block_data, 0};
113113
writer << block.nVersion;
114114
writer << block.hashPrevBlock;
115115
writer << signet_merkle;

‎src/streams.h

Lines changed: 6 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,10 @@ class OverrideStream
5050
{
5151
Stream* stream;
5252

53-
const int nType;
5453
const int nVersion;
5554

5655
public:
57-
OverrideStream(Stream* stream_, int nType_, int nVersion_) : stream(stream_), nType(nType_), nVersion(nVersion_) {}
56+
OverrideStream(Stream* stream_, int nVersion_) : stream{stream_}, nVersion{nVersion_} {}
5857

5958
template<typename T>
6059
OverrideStream<Stream>& operator<<(const T& obj)
@@ -81,7 +80,6 @@ class OverrideStream
8180
}
8281

8382
int GetVersion() const { return nVersion; }
84-
int GetType() const { return nType; }
8583
size_t size() const { return stream->size(); }
8684
void ignore(size_t size) { return stream->ignore(size); }
8785
};
@@ -95,13 +93,12 @@ class CVectorWriter
9593
public:
9694

9795
/*
98-
* @param[in] nTypeIn Serialization Type
9996
* @param[in] nVersionIn Serialization Version (including any flags)
10097
* @param[in] vchDataIn Referenced byte vector to overwrite/append
10198
* @param[in] nPosIn Starting position. Vector index where writes should start. The vector will initially
10299
* grow as necessary to max(nPosIn, vec.size()). So to append, use vec.size().
103100
*/
104-
CVectorWriter(int nTypeIn, int nVersionIn, std::vector<unsigned char>& vchDataIn, size_t nPosIn) : nType(nTypeIn), nVersion(nVersionIn), vchData(vchDataIn), nPos(nPosIn)
101+
CVectorWriter(int nVersionIn, std::vector<unsigned char>& vchDataIn, size_t nPosIn) : nVersion{nVersionIn}, vchData{vchDataIn}, nPos{nPosIn}
105102
{
106103
if(nPos > vchData.size())
107104
vchData.resize(nPos);
@@ -111,7 +108,7 @@ class CVectorWriter
111108
* @param[in] args A list of items to serialize starting at nPosIn.
112109
*/
113110
template <typename... Args>
114-
CVectorWriter(int nTypeIn, int nVersionIn, std::vector<unsigned char>& vchDataIn, size_t nPosIn, Args&&... args) : CVectorWriter(nTypeIn, nVersionIn, vchDataIn, nPosIn)
111+
CVectorWriter(int nVersionIn, std::vector<unsigned char>& vchDataIn, size_t nPosIn, Args&&... args) : CVectorWriter{nVersionIn, vchDataIn, nPosIn}
115112
{
116113
::SerializeMany(*this, std::forward<Args>(args)...);
117114
}
@@ -137,12 +134,8 @@ class CVectorWriter
137134
{
138135
return nVersion;
139136
}
140-
int GetType() const
141-
{
142-
return nType;
143-
}
137+
144138
private:
145-
const int nType;
146139
const int nVersion;
147140
std::vector<unsigned char>& vchData;
148141
size_t nPos;
@@ -153,19 +146,16 @@ class CVectorWriter
153146
class SpanReader
154147
{
155148
private:
156-
const int m_type;
157149
const int m_version;
158150
Span<const unsigned char> m_data;
159151

160152
public:
161-
162153
/**
163-
* @param[in] type Serialization Type
164154
* @param[in] version Serialization Version (including any flags)
165155
* @param[in] data Referenced byte vector to overwrite/append
166156
*/
167-
SpanReader(int type, int version, Span<const unsigned char> data)
168-
: m_type(type), m_version(version), m_data(data) {}
157+
SpanReader(int version, Span<const unsigned char> data)
158+
: m_version{version}, m_data{data} {}
169159

170160
template<typename T>
171161
SpanReader& operator>>(T&& obj)
@@ -175,7 +165,6 @@ class SpanReader
175165
}
176166

177167
int GetVersion() const { return m_version; }
178-
int GetType() const { return m_type; }
179168

180169
size_t size() const { return m_data.size(); }
181170
bool empty() const { return m_data.empty(); }

0 commit comments

Comments
 (0)