Skip to content

Commit fe4e83f

Browse files
fanquakefanquake
committed
Merge bitcoin#28912: refactor: VectorWriter and SpanReader without nVersion
fae76a1 scripted-diff: Use DataStream in most places (MarcoFalke) fac39b5 refactor: SpanReader without nVersion (MarcoFalke) Pull request description: The serialize version is unused, so remove it. This also allows to remove `GCS_SER_VERSION` and allows a scripted-diff to remove most of `CDataStream`. ACKs for top commit: ajtowns: ACK fae76a1 ryanofsky: Code review ACK fae76a1 Tree-SHA512: 3b487dba8ea380f1eacff9fdfb9197f025dbc30906813d3f4c3e6f1e9e4d9f2a169c6f163f51d135e18af538be78e2d2b13d694073ad25c5762980ae971a4c83
2 parents 31ce305 + fae76a1 commit fe4e83f

21 files changed

+78
-87
lines changed

‎src/blockfilter.cpp

+4-7
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,6 @@
1616
#include <util/golombrice.h>
1717
#include <util/string.h>
1818

19-
/// Protocol version used to serialize parameters in GCS filter encoding.
20-
static constexpr int GCS_SER_VERSION = 0;
21-
2219
static const std::map<BlockFilterType, std::string> g_filter_types = {
2320
{BlockFilterType::BASIC, "basic"},
2421
};
@@ -49,7 +46,7 @@ GCSFilter::GCSFilter(const Params& params)
4946
GCSFilter::GCSFilter(const Params& params, std::vector<unsigned char> encoded_filter, bool skip_decode_check)
5047
: m_params(params), m_encoded(std::move(encoded_filter))
5148
{
52-
SpanReader stream{GCS_SER_VERSION, m_encoded};
49+
SpanReader stream{m_encoded};
5350

5451
uint64_t N = ReadCompactSize(stream);
5552
m_N = static_cast<uint32_t>(N);
@@ -62,7 +59,7 @@ GCSFilter::GCSFilter(const Params& params, std::vector<unsigned char> encoded_fi
6259

6360
// Verify that the encoded filter contains exactly N elements. If it has too much or too little
6461
// data, a std::ios_base::failure exception will be raised.
65-
BitStreamReader<SpanReader> bitreader{stream};
62+
BitStreamReader bitreader{stream};
6663
for (uint64_t i = 0; i < m_N; ++i) {
6764
GolombRiceDecode(bitreader, m_params.m_P);
6865
}
@@ -103,13 +100,13 @@ GCSFilter::GCSFilter(const Params& params, const ElementSet& elements)
103100

104101
bool GCSFilter::MatchInternal(const uint64_t* element_hashes, size_t size) const
105102
{
106-
SpanReader stream{GCS_SER_VERSION, m_encoded};
103+
SpanReader stream{m_encoded};
107104

108105
// Seek forward by size of N
109106
uint64_t N = ReadCompactSize(stream);
110107
assert(N == m_N);
111108

112-
BitStreamReader<SpanReader> bitreader{stream};
109+
BitStreamReader bitreader{stream};
113110

114111
uint64_t value = 0;
115112
size_t hashes_index = 0;

‎src/external_signer.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ UniValue ExternalSigner::GetDescriptors(const int account)
7474
bool ExternalSigner::SignTransaction(PartiallySignedTransaction& psbtx, std::string& error)
7575
{
7676
// Serialize the PSBT
77-
CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION);
77+
DataStream ssTx{};
7878
ssTx << psbtx;
7979
// parse ExternalSigner master fingerprint
8080
std::vector<unsigned char> parsed_m_fingerprint = ParseHex(m_fingerprint);

‎src/psbt.h

+6-6
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,7 @@ struct PSBTInput
382382
}
383383

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

388388
// Do stuff based on type
@@ -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.GetVersion(), Span{key}.subspan(1)};
593+
SpanReader s_key{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.GetVersion(), Span{key}.subspan(1)};
635+
SpanReader s_key{Span{key}.subspan(1)};
636636
XOnlyPubKey xonly;
637637
s_key >> xonly;
638638
std::set<uint256> leaf_hashes;
@@ -807,7 +807,7 @@ struct PSBTOutput
807807
}
808808

809809
// Type is compact size uint at beginning of key
810-
SpanReader skey{s.GetVersion(), key};
810+
SpanReader skey{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.GetVersion(), tree_v};
859+
SpanReader s_tree{tree_v};
860860
if (s_tree.empty()) {
861861
throw std::ios_base::failure("Output Taproot tree must not be empty");
862862
}
@@ -1060,7 +1060,7 @@ struct PartiallySignedTransaction
10601060
}
10611061

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

10661066
// Do stuff based on type

‎src/qt/psbtoperationsdialog.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -128,14 +128,14 @@ void PSBTOperationsDialog::broadcastTransaction()
128128
}
129129

130130
void PSBTOperationsDialog::copyToClipboard() {
131-
CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION);
131+
DataStream ssTx{};
132132
ssTx << m_transaction_data;
133133
GUIUtil::setClipboard(EncodeBase64(ssTx.str()).c_str());
134134
showStatus(tr("PSBT copied to clipboard."), StatusLevel::INFO);
135135
}
136136

137137
void PSBTOperationsDialog::saveTransaction() {
138-
CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION);
138+
DataStream ssTx{};
139139
ssTx << m_transaction_data;
140140

141141
QString selected_filter;

‎src/qt/sendcoinsdialog.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -397,7 +397,7 @@ bool SendCoinsDialog::PrepareSendText(QString& question_string, QString& informa
397397
void SendCoinsDialog::presentPSBT(PartiallySignedTransaction& psbtx)
398398
{
399399
// Serialize the PSBT
400-
CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION);
400+
DataStream ssTx{};
401401
ssTx << psbtx;
402402
GUIUtil::setClipboard(EncodeBase64(ssTx.str()).c_str());
403403
QMessageBox msgBox(this);

‎src/qt/walletmodel.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -543,7 +543,7 @@ bool WalletModel::bumpFee(uint256 hash, uint256& new_hash)
543543
return false;
544544
}
545545
// Serialize the PSBT
546-
CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION);
546+
DataStream ssTx{};
547547
ssTx << psbtx;
548548
GUIUtil::setClipboard(EncodeBase64(ssTx.str()).c_str());
549549
Q_EMIT message(tr("PSBT copied"), tr("Copied to clipboard", "Fee-bump PSBT saved"), CClientUIInterface::MSG_INFORMATION);

‎src/rpc/rawtransaction.cpp

+7-7
Original file line numberDiff line numberDiff line change
@@ -1493,7 +1493,7 @@ static RPCHelpMan combinepsbt()
14931493
throw JSONRPCTransactionError(error);
14941494
}
14951495

1496-
CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION);
1496+
DataStream ssTx{};
14971497
ssTx << merged_psbt;
14981498
return EncodeBase64(ssTx);
14991499
},
@@ -1538,7 +1538,7 @@ static RPCHelpMan finalizepsbt()
15381538
bool complete = FinalizeAndExtractPSBT(psbtx, mtx);
15391539

15401540
UniValue result(UniValue::VOBJ);
1541-
CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION);
1541+
DataStream ssTx{};
15421542
std::string result_str;
15431543

15441544
if (complete && extract) {
@@ -1589,7 +1589,7 @@ static RPCHelpMan createpsbt()
15891589
}
15901590

15911591
// Serialize the PSBT
1592-
CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION);
1592+
DataStream ssTx{};
15931593
ssTx << psbtx;
15941594

15951595
return EncodeBase64(ssTx);
@@ -1656,7 +1656,7 @@ static RPCHelpMan converttopsbt()
16561656
}
16571657

16581658
// Serialize the PSBT
1659-
CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION);
1659+
DataStream ssTx{};
16601660
ssTx << psbtx;
16611661

16621662
return EncodeBase64(ssTx);
@@ -1703,7 +1703,7 @@ static RPCHelpMan utxoupdatepsbt()
17031703
/*sighash_type=*/SIGHASH_ALL,
17041704
/*finalize=*/false);
17051705

1706-
CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION);
1706+
DataStream ssTx{};
17071707
ssTx << psbtx;
17081708
return EncodeBase64(ssTx);
17091709
},
@@ -1804,7 +1804,7 @@ static RPCHelpMan joinpsbts()
18041804
}
18051805
shuffled_psbt.unknown.insert(merged_psbt.unknown.begin(), merged_psbt.unknown.end());
18061806

1807-
CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION);
1807+
DataStream ssTx{};
18081808
ssTx << shuffled_psbt;
18091809
return EncodeBase64(ssTx);
18101810
},
@@ -1984,7 +1984,7 @@ RPCHelpMan descriptorprocesspsbt()
19841984
complete &= PSBTInputSigned(input);
19851985
}
19861986

1987-
CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION);
1987+
DataStream ssTx{};
19881988
ssTx << psbtx;
19891989

19901990
UniValue result(UniValue::VOBJ);

‎src/signet.cpp

+1-2
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
#include <streams.h>
2323
#include <uint256.h>
2424
#include <util/strencodings.h>
25-
#include <version.h>
2625

2726
static constexpr uint8_t SIGNET_HEADER[4] = {0xec, 0xc7, 0xda, 0xa2};
2827

@@ -99,7 +98,7 @@ std::optional<SignetTxs> SignetTxs::Create(const CBlock& block, const CScript& c
9998
// no signet solution -- allow this to support OP_TRUE as trivial block challenge
10099
} else {
101100
try {
102-
SpanReader v{INIT_PROTO_VERSION, signet_solution};
101+
SpanReader v{signet_solution};
103102
v >> tx_spending.vin[0].scriptSig;
104103
v >> tx_spending.vin[0].scriptWitness.stack;
105104
if (!v.empty()) return std::nullopt; // extraneous data encountered

‎src/streams.h

+1-6
Original file line numberDiff line numberDiff line change
@@ -100,16 +100,13 @@ class VectorWriter
100100
class SpanReader
101101
{
102102
private:
103-
const int m_version;
104103
Span<const unsigned char> m_data;
105104

106105
public:
107106
/**
108-
* @param[in] version Serialization Version (including any flags)
109107
* @param[in] data Referenced byte vector to overwrite/append
110108
*/
111-
SpanReader(int version, Span<const unsigned char> data)
112-
: m_version{version}, m_data{data} {}
109+
explicit SpanReader(Span<const unsigned char> data) : m_data{data} {}
113110

114111
template<typename T>
115112
SpanReader& operator>>(T&& obj)
@@ -118,8 +115,6 @@ class SpanReader
118115
return (*this);
119116
}
120117

121-
int GetVersion() const { return m_version; }
122-
123118
size_t size() const { return m_data.size(); }
124119
bool empty() const { return m_data.empty(); }
125120

‎src/test/blockencodings_tests.cpp

+6-6
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ BOOST_AUTO_TEST_CASE(SimpleRoundTripTest)
6868
{
6969
CBlockHeaderAndShortTxIDs shortIDs{block};
7070

71-
CDataStream stream(SER_NETWORK, PROTOCOL_VERSION);
71+
DataStream stream{};
7272
stream << shortIDs;
7373

7474
CBlockHeaderAndShortTxIDs shortIDs2;
@@ -119,15 +119,15 @@ class TestHeaderAndShortIDs {
119119
std::vector<PrefilledTransaction> prefilledtxn;
120120

121121
explicit TestHeaderAndShortIDs(const CBlockHeaderAndShortTxIDs& orig) {
122-
CDataStream stream(SER_NETWORK, PROTOCOL_VERSION);
122+
DataStream stream{};
123123
stream << orig;
124124
stream >> *this;
125125
}
126126
explicit TestHeaderAndShortIDs(const CBlock& block) :
127127
TestHeaderAndShortIDs(CBlockHeaderAndShortTxIDs{block}) {}
128128

129129
uint64_t GetShortID(const uint256& txhash) const {
130-
CDataStream stream(SER_NETWORK, PROTOCOL_VERSION);
130+
DataStream stream{};
131131
stream << *this;
132132
CBlockHeaderAndShortTxIDs base;
133133
stream >> base;
@@ -158,7 +158,7 @@ BOOST_AUTO_TEST_CASE(NonCoinbasePreforwardRTTest)
158158
shortIDs.shorttxids[0] = shortIDs.GetShortID(block.vtx[0]->GetHash());
159159
shortIDs.shorttxids[1] = shortIDs.GetShortID(block.vtx[2]->GetHash());
160160

161-
CDataStream stream(SER_NETWORK, PROTOCOL_VERSION);
161+
DataStream stream{};
162162
stream << shortIDs;
163163

164164
CBlockHeaderAndShortTxIDs shortIDs2;
@@ -228,7 +228,7 @@ BOOST_AUTO_TEST_CASE(SufficientPreforwardRTTest)
228228
shortIDs.shorttxids.resize(1);
229229
shortIDs.shorttxids[0] = shortIDs.GetShortID(block.vtx[1]->GetHash());
230230

231-
CDataStream stream(SER_NETWORK, PROTOCOL_VERSION);
231+
DataStream stream{};
232232
stream << shortIDs;
233233

234234
CBlockHeaderAndShortTxIDs shortIDs2;
@@ -283,7 +283,7 @@ BOOST_AUTO_TEST_CASE(EmptyBlockRoundTripTest)
283283
{
284284
CBlockHeaderAndShortTxIDs shortIDs{block};
285285

286-
CDataStream stream(SER_NETWORK, PROTOCOL_VERSION);
286+
DataStream stream{};
287287
stream << shortIDs;
288288

289289
CBlockHeaderAndShortTxIDs shortIDs2;

‎src/test/fuzz/golomb_rice.cpp

+4-4
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,8 @@ FUZZ_TARGET(golomb_rice)
6868

6969
std::vector<uint64_t> decoded_deltas;
7070
{
71-
SpanReader stream{0, golomb_rice_data};
72-
BitStreamReader<SpanReader> bitreader{stream};
71+
SpanReader stream{golomb_rice_data};
72+
BitStreamReader bitreader{stream};
7373
const uint32_t n = static_cast<uint32_t>(ReadCompactSize(stream));
7474
for (uint32_t i = 0; i < n; ++i) {
7575
decoded_deltas.push_back(GolombRiceDecode(bitreader, BASIC_FILTER_P));
@@ -80,14 +80,14 @@ FUZZ_TARGET(golomb_rice)
8080

8181
{
8282
const std::vector<uint8_t> random_bytes = ConsumeRandomLengthByteVector(fuzzed_data_provider, 1024);
83-
SpanReader stream{0, random_bytes};
83+
SpanReader stream{random_bytes};
8484
uint32_t n;
8585
try {
8686
n = static_cast<uint32_t>(ReadCompactSize(stream));
8787
} catch (const std::ios_base::failure&) {
8888
return;
8989
}
90-
BitStreamReader<SpanReader> bitreader{stream};
90+
BitStreamReader bitreader{stream};
9191
for (uint32_t i = 0; i < std::min<uint32_t>(n, 1024); ++i) {
9292
try {
9393
(void)GolombRiceDecode(bitreader, BASIC_FILTER_P);

‎src/test/fuzz/script_assets_test_minimizer.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ CMutableTransaction TxFromHex(const std::string& str)
5454
{
5555
CMutableTransaction tx;
5656
try {
57-
SpanReader{0, CheckedParseHex(str)} >> TX_NO_WITNESS(tx);
57+
SpanReader{CheckedParseHex(str)} >> TX_NO_WITNESS(tx);
5858
} catch (const std::ios_base::failure&) {
5959
throw std::runtime_error("Tx deserialization failure");
6060
}
@@ -68,7 +68,7 @@ std::vector<CTxOut> TxOutsFromJSON(const UniValue& univalue)
6868
for (size_t i = 0; i < univalue.size(); ++i) {
6969
CTxOut txout;
7070
try {
71-
SpanReader{0, CheckedParseHex(univalue[i].get_str())} >> txout;
71+
SpanReader{CheckedParseHex(univalue[i].get_str())} >> txout;
7272
} catch (const std::ios_base::failure&) {
7373
throw std::runtime_error("Prevout invalid format");
7474
}

‎src/test/net_tests.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -1126,7 +1126,7 @@ class V2TransportTester
11261126
void SendV1Version(const MessageStartChars& magic)
11271127
{
11281128
CMessageHeader hdr(magic, "version", 126 + InsecureRandRange(11));
1129-
CDataStream ser(SER_NETWORK, CLIENT_VERSION);
1129+
DataStream ser{};
11301130
ser << hdr;
11311131
m_to_send.insert(m_to_send.end(), UCharCast(ser.data()), UCharCast(ser.data() + ser.size()));
11321132
}

‎src/test/script_tests.cpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -1470,7 +1470,7 @@ BOOST_AUTO_TEST_CASE(script_HasValidOps)
14701470
static CMutableTransaction TxFromHex(const std::string& str)
14711471
{
14721472
CMutableTransaction tx;
1473-
SpanReader{0, ParseHex(str)} >> TX_NO_WITNESS(tx);
1473+
SpanReader{ParseHex(str)} >> TX_NO_WITNESS(tx);
14741474
return tx;
14751475
}
14761476

@@ -1480,7 +1480,7 @@ static std::vector<CTxOut> TxOutsFromJSON(const UniValue& univalue)
14801480
std::vector<CTxOut> prevouts;
14811481
for (size_t i = 0; i < univalue.size(); ++i) {
14821482
CTxOut txout;
1483-
SpanReader{0, ParseHex(univalue[i].get_str())} >> txout;
1483+
SpanReader{ParseHex(univalue[i].get_str())} >> txout;
14841484
prevouts.push_back(std::move(txout));
14851485
}
14861486
return prevouts;
@@ -1816,7 +1816,7 @@ BOOST_AUTO_TEST_CASE(bip341_keypath_test_vectors)
18161816
for (const auto& vec : vectors.getValues()) {
18171817
auto txhex = ParseHex(vec["given"]["rawUnsignedTx"].get_str());
18181818
CMutableTransaction tx;
1819-
SpanReader{PROTOCOL_VERSION, txhex} >> TX_WITH_WITNESS(tx);
1819+
SpanReader{txhex} >> TX_WITH_WITNESS(tx);
18201820
std::vector<CTxOut> utxos;
18211821
for (const auto& utxo_spent : vec["given"]["utxosSpent"].getValues()) {
18221822
auto script_bytes = ParseHex(utxo_spent["scriptPubKey"].get_str());

0 commit comments

Comments
 (0)