Skip to content

Commit d51fb9c

Browse files
committed
Merge bitcoin#28503: refactor: Remove WithParams serialization helper, use SER_PARAMS_OPFUNC
9999019 Remove WithParams serialization helper (MarcoFalke) ffffb4a scripted-diff: Use ser params operator (MarcoFalke) fae9054 test: Use SER_PARAMS_OPFUNC in serialize_tests.cpp (MarcoFalke) Pull request description: Every serialization parameter struct already has the `SER_PARAMS_OPFUNC`, except for one in the tests. For consistency, and to remove verbose code, convert the test to `SER_PARAMS_OPFUNC`, and use it everywhere, then remove the `WithParams` helper. ACKs for top commit: ajtowns: reACK 9999019 TheCharlatan: Re-ACK 9999019 Tree-SHA512: be9cae4225a502486fe8d552aaf4b2cd2904a9f73cce9d931c6b7c757594ff1982fcc2c30d00d012cd12b0a9531fd609f8bcd7c94b811e965ac087eb8a3589d3
2 parents 4458ae8 + 9999019 commit d51fb9c

File tree

7 files changed

+79
-82
lines changed

7 files changed

+79
-82
lines changed

src/net_processing.cpp

+4-8
Original file line numberDiff line numberDiff line change
@@ -3770,7 +3770,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
37703770

37713771
std::vector<CAddress> vAddr;
37723772

3773-
vRecv >> WithParams(ser_params, vAddr);
3773+
vRecv >> ser_params(vAddr);
37743774

37753775
if (!SetupAddressRelay(pfrom, *peer)) {
37763776
LogPrint(BCLog::NET, "ignoring %s message from %s peer=%d\n", msg_type, pfrom.ConnectionTypeAsString(), pfrom.GetId());
@@ -5375,16 +5375,12 @@ void PeerManagerImpl::MaybeSendAddr(CNode& node, Peer& peer, std::chrono::micros
53755375
// No addr messages to send
53765376
if (peer.m_addrs_to_send.empty()) return;
53775377

5378-
const char* msg_type;
5379-
CNetAddr::Encoding ser_enc;
5378+
CNetMsgMaker mm(node.GetCommonVersion());
53805379
if (peer.m_wants_addrv2) {
5381-
msg_type = NetMsgType::ADDRV2;
5382-
ser_enc = CNetAddr::Encoding::V2;
5380+
m_connman.PushMessage(&node, mm.Make(NetMsgType::ADDRV2, CAddress::V2_NETWORK(peer.m_addrs_to_send)));
53835381
} else {
5384-
msg_type = NetMsgType::ADDR;
5385-
ser_enc = CNetAddr::Encoding::V1;
5382+
m_connman.PushMessage(&node, mm.Make(NetMsgType::ADDR, CAddress::V1_NETWORK(peer.m_addrs_to_send)));
53865383
}
5387-
m_connman.PushMessage(&node, CNetMsgMaker(node.GetCommonVersion()).Make(msg_type, WithParams(CAddress::SerParams{{ser_enc}, CAddress::Format::Network}, peer.m_addrs_to_send)));
53885384
peer.m_addrs_to_send.clear();
53895385

53905386
// we only send the big addr message once

src/protocol.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -445,7 +445,7 @@ class CAddress : public CService
445445
}
446446
// Invoke V1/V2 serializer for CService parent object.
447447
const auto ser_params{use_v2 ? CNetAddr::V2 : CNetAddr::V1};
448-
READWRITE(WithParams(ser_params, AsBase<CService>(obj)));
448+
READWRITE(ser_params(AsBase<CService>(obj)));
449449
}
450450

451451
//! Always included in serialization. The behavior is unspecified if the value is not representable as uint32_t.

src/serialize.h

+14-17
Original file line numberDiff line numberDiff line change
@@ -214,11 +214,11 @@ const Out& AsBase(const In& x)
214214
* }
215215
* };
216216
* which would then be invoked as
217-
* READWRITE(WithParams(BarParameter{...}, Using<FooFormatter>(obj.foo)))
217+
* READWRITE(BarParameter{...}(Using<FooFormatter>(obj.foo)))
218218
*
219-
* WithParams(parameter, obj) can be invoked anywhere in the call stack; it is
219+
* parameter(obj) can be invoked anywhere in the call stack; it is
220220
* passed down recursively into all serialization code, until another
221-
* WithParams overrides it.
221+
* serialization parameter overrides it.
222222
*
223223
* Parameters will be implicitly converted where appropriate. This means that
224224
* "parent" serialization code can use a parameter that derives from, or is
@@ -1182,17 +1182,6 @@ class ParamsWrapper
11821182
}
11831183
};
11841184

1185-
/**
1186-
* Return a wrapper around t that (de)serializes it with specified parameter params.
1187-
*
1188-
* See FORMATTER_METHODS_PARAMS for more information on serialization parameters.
1189-
*/
1190-
template <typename Params, typename T>
1191-
static auto WithParams(const Params& params, T&& t)
1192-
{
1193-
return ParamsWrapper<Params, T>{params, t};
1194-
}
1195-
11961185
/**
11971186
* Helper macro for SerParams structs
11981187
*
@@ -1202,8 +1191,16 @@ static auto WithParams(const Params& params, T&& t)
12021191
* constexpr SerParams FOO{....};
12031192
* ss << FOO(obj);
12041193
*/
1205-
#define SER_PARAMS_OPFUNC \
1206-
template <typename T> \
1207-
auto operator()(T&& t) const { return WithParams(*this, t); }
1194+
#define SER_PARAMS_OPFUNC \
1195+
/** \
1196+
* Return a wrapper around t that (de)serializes it with specified parameter params. \
1197+
* \
1198+
* See FORMATTER_METHODS_PARAMS for more information on serialization parameters. \
1199+
*/ \
1200+
template <typename T> \
1201+
auto operator()(T&& t) const \
1202+
{ \
1203+
return ParamsWrapper{*this, t}; \
1204+
}
12081205

12091206
#endif // BITCOIN_SERIALIZE_H

src/test/fuzz/deserialize.cpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -66,15 +66,15 @@ template <typename T, typename P>
6666
DataStream Serialize(const T& obj, const P& params)
6767
{
6868
DataStream ds{};
69-
ds << WithParams(params, obj);
69+
ds << params(obj);
7070
return ds;
7171
}
7272

7373
template <typename T, typename P>
7474
T Deserialize(DataStream&& ds, const P& params)
7575
{
7676
T obj;
77-
ds >> WithParams(params, obj);
77+
ds >> params(obj);
7878
return obj;
7979
}
8080

@@ -83,7 +83,7 @@ void DeserializeFromFuzzingInput(FuzzBufferType buffer, T&& obj, const P& params
8383
{
8484
DataStream ds{buffer};
8585
try {
86-
ds >> WithParams(params, obj);
86+
ds >> params(obj);
8787
} catch (const std::ios_base::failure&) {
8888
throw invalid_fuzzing_input_exception();
8989
}

src/test/fuzz/util.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ template <typename T, typename P>
107107
DataStream ds{buffer};
108108
T obj;
109109
try {
110-
ds >> WithParams(params, obj);
110+
ds >> params(obj);
111111
} catch (const std::ios_base::failure&) {
112112
return std::nullopt;
113113
}

src/test/net_tests.cpp

+30-30
Original file line numberDiff line numberDiff line change
@@ -331,30 +331,30 @@ BOOST_AUTO_TEST_CASE(cnetaddr_serialize_v1)
331331
DataStream s{};
332332
const auto ser_params{CAddress::V1_NETWORK};
333333

334-
s << WithParams(ser_params, addr);
334+
s << ser_params(addr);
335335
BOOST_CHECK_EQUAL(HexStr(s), "00000000000000000000000000000000");
336336
s.clear();
337337

338338
addr = LookupHost("1.2.3.4", false).value();
339-
s << WithParams(ser_params, addr);
339+
s << ser_params(addr);
340340
BOOST_CHECK_EQUAL(HexStr(s), "00000000000000000000ffff01020304");
341341
s.clear();
342342

343343
addr = LookupHost("1a1b:2a2b:3a3b:4a4b:5a5b:6a6b:7a7b:8a8b", false).value();
344-
s << WithParams(ser_params, addr);
344+
s << ser_params(addr);
345345
BOOST_CHECK_EQUAL(HexStr(s), "1a1b2a2b3a3b4a4b5a5b6a6b7a7b8a8b");
346346
s.clear();
347347

348348
// TORv2, no longer supported
349349
BOOST_CHECK(!addr.SetSpecial("6hzph5hv6337r6p2.onion"));
350350

351351
BOOST_REQUIRE(addr.SetSpecial("pg6mmjiyjmcrsslvykfwnntlaru7p5svn6y2ymmju6nubxndf4pscryd.onion"));
352-
s << WithParams(ser_params, addr);
352+
s << ser_params(addr);
353353
BOOST_CHECK_EQUAL(HexStr(s), "00000000000000000000000000000000");
354354
s.clear();
355355

356356
addr.SetInternal("a");
357-
s << WithParams(ser_params, addr);
357+
s << ser_params(addr);
358358
BOOST_CHECK_EQUAL(HexStr(s), "fd6b88c08724ca978112ca1bbdcafac2");
359359
s.clear();
360360
}
@@ -365,30 +365,30 @@ BOOST_AUTO_TEST_CASE(cnetaddr_serialize_v2)
365365
DataStream s{};
366366
const auto ser_params{CAddress::V2_NETWORK};
367367

368-
s << WithParams(ser_params, addr);
368+
s << ser_params(addr);
369369
BOOST_CHECK_EQUAL(HexStr(s), "021000000000000000000000000000000000");
370370
s.clear();
371371

372372
addr = LookupHost("1.2.3.4", false).value();
373-
s << WithParams(ser_params, addr);
373+
s << ser_params(addr);
374374
BOOST_CHECK_EQUAL(HexStr(s), "010401020304");
375375
s.clear();
376376

377377
addr = LookupHost("1a1b:2a2b:3a3b:4a4b:5a5b:6a6b:7a7b:8a8b", false).value();
378-
s << WithParams(ser_params, addr);
378+
s << ser_params(addr);
379379
BOOST_CHECK_EQUAL(HexStr(s), "02101a1b2a2b3a3b4a4b5a5b6a6b7a7b8a8b");
380380
s.clear();
381381

382382
// TORv2, no longer supported
383383
BOOST_CHECK(!addr.SetSpecial("6hzph5hv6337r6p2.onion"));
384384

385385
BOOST_REQUIRE(addr.SetSpecial("kpgvmscirrdqpekbqjsvw5teanhatztpp2gl6eee4zkowvwfxwenqaid.onion"));
386-
s << WithParams(ser_params, addr);
386+
s << ser_params(addr);
387387
BOOST_CHECK_EQUAL(HexStr(s), "042053cd5648488c4707914182655b7664034e09e66f7e8cbf1084e654eb56c5bd88");
388388
s.clear();
389389

390390
BOOST_REQUIRE(addr.SetInternal("a"));
391-
s << WithParams(ser_params, addr);
391+
s << ser_params(addr);
392392
BOOST_CHECK_EQUAL(HexStr(s), "0210fd6b88c08724ca978112ca1bbdcafac2");
393393
s.clear();
394394
}
@@ -403,7 +403,7 @@ BOOST_AUTO_TEST_CASE(cnetaddr_unserialize_v2)
403403
s << Span{ParseHex("01" // network type (IPv4)
404404
"04" // address length
405405
"01020304")}; // address
406-
s >> WithParams(ser_params, addr);
406+
s >> ser_params(addr);
407407
BOOST_CHECK(addr.IsValid());
408408
BOOST_CHECK(addr.IsIPv4());
409409
BOOST_CHECK(addr.IsAddrV1Compatible());
@@ -414,15 +414,15 @@ BOOST_AUTO_TEST_CASE(cnetaddr_unserialize_v2)
414414
s << Span{ParseHex("01" // network type (IPv4)
415415
"04" // address length
416416
"0102")}; // address
417-
BOOST_CHECK_EXCEPTION(s >> WithParams(ser_params, addr), std::ios_base::failure, HasReason("end of data"));
417+
BOOST_CHECK_EXCEPTION(s >> ser_params(addr), std::ios_base::failure, HasReason("end of data"));
418418
BOOST_REQUIRE(!s.empty()); // The stream is not consumed on invalid input.
419419
s.clear();
420420

421421
// Invalid IPv4, with bogus length.
422422
s << Span{ParseHex("01" // network type (IPv4)
423423
"05" // address length
424424
"01020304")}; // address
425-
BOOST_CHECK_EXCEPTION(s >> WithParams(ser_params, addr), std::ios_base::failure,
425+
BOOST_CHECK_EXCEPTION(s >> ser_params(addr), std::ios_base::failure,
426426
HasReason("BIP155 IPv4 address with length 5 (should be 4)"));
427427
BOOST_REQUIRE(!s.empty()); // The stream is not consumed on invalid input.
428428
s.clear();
@@ -431,7 +431,7 @@ BOOST_AUTO_TEST_CASE(cnetaddr_unserialize_v2)
431431
s << Span{ParseHex("01" // network type (IPv4)
432432
"fd0102" // address length (513 as CompactSize)
433433
"01020304")}; // address
434-
BOOST_CHECK_EXCEPTION(s >> WithParams(ser_params, addr), std::ios_base::failure,
434+
BOOST_CHECK_EXCEPTION(s >> ser_params(addr), std::ios_base::failure,
435435
HasReason("Address too long: 513 > 512"));
436436
BOOST_REQUIRE(!s.empty()); // The stream is not consumed on invalid input.
437437
s.clear();
@@ -440,7 +440,7 @@ BOOST_AUTO_TEST_CASE(cnetaddr_unserialize_v2)
440440
s << Span{ParseHex("02" // network type (IPv6)
441441
"10" // address length
442442
"0102030405060708090a0b0c0d0e0f10")}; // address
443-
s >> WithParams(ser_params, addr);
443+
s >> ser_params(addr);
444444
BOOST_CHECK(addr.IsValid());
445445
BOOST_CHECK(addr.IsIPv6());
446446
BOOST_CHECK(addr.IsAddrV1Compatible());
@@ -453,7 +453,7 @@ BOOST_AUTO_TEST_CASE(cnetaddr_unserialize_v2)
453453
"10" // address length
454454
"fd6b88c08724ca978112ca1bbdcafac2")}; // address: 0xfd + sha256("bitcoin")[0:5] +
455455
// sha256(name)[0:10]
456-
s >> WithParams(ser_params, addr);
456+
s >> ser_params(addr);
457457
BOOST_CHECK(addr.IsInternal());
458458
BOOST_CHECK(addr.IsAddrV1Compatible());
459459
BOOST_CHECK_EQUAL(addr.ToStringAddr(), "zklycewkdo64v6wc.internal");
@@ -463,7 +463,7 @@ BOOST_AUTO_TEST_CASE(cnetaddr_unserialize_v2)
463463
s << Span{ParseHex("02" // network type (IPv6)
464464
"04" // address length
465465
"00")}; // address
466-
BOOST_CHECK_EXCEPTION(s >> WithParams(ser_params, addr), std::ios_base::failure,
466+
BOOST_CHECK_EXCEPTION(s >> ser_params(addr), std::ios_base::failure,
467467
HasReason("BIP155 IPv6 address with length 4 (should be 16)"));
468468
BOOST_REQUIRE(!s.empty()); // The stream is not consumed on invalid input.
469469
s.clear();
@@ -472,23 +472,23 @@ BOOST_AUTO_TEST_CASE(cnetaddr_unserialize_v2)
472472
s << Span{ParseHex("02" // network type (IPv6)
473473
"10" // address length
474474
"00000000000000000000ffff01020304")}; // address
475-
s >> WithParams(ser_params, addr);
475+
s >> ser_params(addr);
476476
BOOST_CHECK(!addr.IsValid());
477477
BOOST_REQUIRE(s.empty());
478478

479479
// Invalid IPv6, contains embedded TORv2.
480480
s << Span{ParseHex("02" // network type (IPv6)
481481
"10" // address length
482482
"fd87d87eeb430102030405060708090a")}; // address
483-
s >> WithParams(ser_params, addr);
483+
s >> ser_params(addr);
484484
BOOST_CHECK(!addr.IsValid());
485485
BOOST_REQUIRE(s.empty());
486486

487487
// TORv2, no longer supported.
488488
s << Span{ParseHex("03" // network type (TORv2)
489489
"0a" // address length
490490
"f1f2f3f4f5f6f7f8f9fa")}; // address
491-
s >> WithParams(ser_params, addr);
491+
s >> ser_params(addr);
492492
BOOST_CHECK(!addr.IsValid());
493493
BOOST_REQUIRE(s.empty());
494494

@@ -498,7 +498,7 @@ BOOST_AUTO_TEST_CASE(cnetaddr_unserialize_v2)
498498
"79bcc625184b05194975c28b66b66b04" // address
499499
"69f7f6556fb1ac3189a79b40dda32f1f"
500500
)};
501-
s >> WithParams(ser_params, addr);
501+
s >> ser_params(addr);
502502
BOOST_CHECK(addr.IsValid());
503503
BOOST_CHECK(addr.IsTor());
504504
BOOST_CHECK(!addr.IsAddrV1Compatible());
@@ -511,7 +511,7 @@ BOOST_AUTO_TEST_CASE(cnetaddr_unserialize_v2)
511511
"00" // address length
512512
"00" // address
513513
)};
514-
BOOST_CHECK_EXCEPTION(s >> WithParams(ser_params, addr), std::ios_base::failure,
514+
BOOST_CHECK_EXCEPTION(s >> ser_params(addr), std::ios_base::failure,
515515
HasReason("BIP155 TORv3 address with length 0 (should be 32)"));
516516
BOOST_REQUIRE(!s.empty()); // The stream is not consumed on invalid input.
517517
s.clear();
@@ -521,7 +521,7 @@ BOOST_AUTO_TEST_CASE(cnetaddr_unserialize_v2)
521521
"20" // address length
522522
"a2894dabaec08c0051a481a6dac88b64" // address
523523
"f98232ae42d4b6fd2fa81952dfe36a87")};
524-
s >> WithParams(ser_params, addr);
524+
s >> ser_params(addr);
525525
BOOST_CHECK(addr.IsValid());
526526
BOOST_CHECK(addr.IsI2P());
527527
BOOST_CHECK(!addr.IsAddrV1Compatible());
@@ -534,7 +534,7 @@ BOOST_AUTO_TEST_CASE(cnetaddr_unserialize_v2)
534534
"03" // address length
535535
"00" // address
536536
)};
537-
BOOST_CHECK_EXCEPTION(s >> WithParams(ser_params, addr), std::ios_base::failure,
537+
BOOST_CHECK_EXCEPTION(s >> ser_params(addr), std::ios_base::failure,
538538
HasReason("BIP155 I2P address with length 3 (should be 32)"));
539539
BOOST_REQUIRE(!s.empty()); // The stream is not consumed on invalid input.
540540
s.clear();
@@ -544,7 +544,7 @@ BOOST_AUTO_TEST_CASE(cnetaddr_unserialize_v2)
544544
"10" // address length
545545
"fc000001000200030004000500060007" // address
546546
)};
547-
s >> WithParams(ser_params, addr);
547+
s >> ser_params(addr);
548548
BOOST_CHECK(addr.IsValid());
549549
BOOST_CHECK(addr.IsCJDNS());
550550
BOOST_CHECK(!addr.IsAddrV1Compatible());
@@ -556,7 +556,7 @@ BOOST_AUTO_TEST_CASE(cnetaddr_unserialize_v2)
556556
"10" // address length
557557
"aa000001000200030004000500060007" // address
558558
)};
559-
s >> WithParams(ser_params, addr);
559+
s >> ser_params(addr);
560560
BOOST_CHECK(addr.IsCJDNS());
561561
BOOST_CHECK(!addr.IsValid());
562562
BOOST_REQUIRE(s.empty());
@@ -566,7 +566,7 @@ BOOST_AUTO_TEST_CASE(cnetaddr_unserialize_v2)
566566
"01" // address length
567567
"00" // address
568568
)};
569-
BOOST_CHECK_EXCEPTION(s >> WithParams(ser_params, addr), std::ios_base::failure,
569+
BOOST_CHECK_EXCEPTION(s >> ser_params(addr), std::ios_base::failure,
570570
HasReason("BIP155 CJDNS address with length 1 (should be 16)"));
571571
BOOST_REQUIRE(!s.empty()); // The stream is not consumed on invalid input.
572572
s.clear();
@@ -576,7 +576,7 @@ BOOST_AUTO_TEST_CASE(cnetaddr_unserialize_v2)
576576
"fe00000002" // address length (CompactSize's MAX_SIZE)
577577
"01020304050607" // address
578578
)};
579-
BOOST_CHECK_EXCEPTION(s >> WithParams(ser_params, addr), std::ios_base::failure,
579+
BOOST_CHECK_EXCEPTION(s >> ser_params(addr), std::ios_base::failure,
580580
HasReason("Address too long: 33554432 > 512"));
581581
BOOST_REQUIRE(!s.empty()); // The stream is not consumed on invalid input.
582582
s.clear();
@@ -586,7 +586,7 @@ BOOST_AUTO_TEST_CASE(cnetaddr_unserialize_v2)
586586
"04" // address length
587587
"01020304" // address
588588
)};
589-
s >> WithParams(ser_params, addr);
589+
s >> ser_params(addr);
590590
BOOST_CHECK(!addr.IsValid());
591591
BOOST_REQUIRE(s.empty());
592592

@@ -595,7 +595,7 @@ BOOST_AUTO_TEST_CASE(cnetaddr_unserialize_v2)
595595
"00" // address length
596596
"" // address
597597
)};
598-
s >> WithParams(ser_params, addr);
598+
s >> ser_params(addr);
599599
BOOST_CHECK(!addr.IsValid());
600600
BOOST_REQUIRE(s.empty());
601601
}

0 commit comments

Comments
 (0)