Skip to content

Commit 5c7cdda

Browse files
fanquakefanquake
committed
Merge bitcoin#28473: refactor: Serialization parameter cleanups
fb6a2ab scripted-diff: use SER_PARAMS_OPFUNC (Anthony Towns) 5e5c8f8 serialize: add SER_PARAMS_OPFUNC (Anthony Towns) 33203f5 serialize: specify type for ParamsWrapper not ref (Anthony Towns) bf147bf serialize: move ser_action functions out of global namespace (Anthony Towns) Pull request description: Cleanups after bitcoin#25284: * ser_action namespacing - bitcoin#25284 (comment) * make reference implicit - bitcoin#25284 (comment) * function notation - bitcoin#25284 (comment) ACKs for top commit: MarcoFalke: lgtm ACK fb6a2ab 💨 TheCharlatan: ACK fb6a2ab Tree-SHA512: aacca2ee9cfec360ade6b394606e13d1dfe05bc29c5fbdd48a4e6992bd420312d4ed0d32218d95c560646af326e9977728dc2e759990636298e326947f6f9526
2 parents 717a4d8 + fb6a2ab commit 5c7cdda

File tree

10 files changed

+87
-71
lines changed

10 files changed

+87
-71
lines changed

‎src/addrdb.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -216,14 +216,14 @@ util::Result<std::unique_ptr<AddrMan>> LoadAddrman(const NetGroupManager& netgro
216216
void DumpAnchors(const fs::path& anchors_db_path, const std::vector<CAddress>& anchors)
217217
{
218218
LOG_TIME_SECONDS(strprintf("Flush %d outbound block-relay-only peer addresses to anchors.dat", anchors.size()));
219-
SerializeFileDB("anchors", anchors_db_path, WithParams(CAddress::V2_DISK, anchors));
219+
SerializeFileDB("anchors", anchors_db_path, CAddress::V2_DISK(anchors));
220220
}
221221

222222
std::vector<CAddress> ReadAnchors(const fs::path& anchors_db_path)
223223
{
224224
std::vector<CAddress> anchors;
225225
try {
226-
DeserializeFileDB(anchors_db_path, WithParams(CAddress::V2_DISK, anchors));
226+
DeserializeFileDB(anchors_db_path, CAddress::V2_DISK(anchors));
227227
LogPrintf("Loaded %i addresses from %s\n", anchors.size(), fs::quoted(fs::PathToString(anchors_db_path.filename())));
228228
} catch (const std::exception&) {
229229
anchors.clear();

‎src/net_processing.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1415,8 +1415,8 @@ void PeerManagerImpl::PushNodeVersion(CNode& pnode, const Peer& peer)
14151415

14161416
const bool tx_relay{!RejectIncomingTxs(pnode)};
14171417
m_connman.PushMessage(&pnode, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::VERSION, PROTOCOL_VERSION, my_services, nTime,
1418-
your_services, WithParams(CNetAddr::V1, addr_you), // Together the pre-version-31402 serialization of CAddress "addrYou" (without nTime)
1419-
my_services, WithParams(CNetAddr::V1, CService{}), // Together the pre-version-31402 serialization of CAddress "addrMe" (without nTime)
1418+
your_services, CNetAddr::V1(addr_you), // Together the pre-version-31402 serialization of CAddress "addrYou" (without nTime)
1419+
my_services, CNetAddr::V1(CService{}), // Together the pre-version-31402 serialization of CAddress "addrMe" (without nTime)
14201420
nonce, strSubVersion, nNodeStartingHeight, tx_relay));
14211421

14221422
if (fLogIPs) {
@@ -3293,7 +3293,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
32933293
nTime = 0;
32943294
}
32953295
vRecv.ignore(8); // Ignore the addrMe service bits sent by the peer
3296-
vRecv >> WithParams(CNetAddr::V1, addrMe);
3296+
vRecv >> CNetAddr::V1(addrMe);
32973297
if (!pfrom.IsInboundConn())
32983298
{
32993299
m_addrman.SetServices(pfrom.addr, nServices);

‎src/netaddress.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,7 @@ class CNetAddr
218218
};
219219
struct SerParams {
220220
const Encoding enc;
221+
SER_PARAMS_OPFUNC
221222
};
222223
static constexpr SerParams V1{Encoding::V1};
223224
static constexpr SerParams V2{Encoding::V2};

‎src/protocol.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -396,6 +396,7 @@ class CAddress : public CService
396396
};
397397
struct SerParams : CNetAddr::SerParams {
398398
const Format fmt;
399+
SER_PARAMS_OPFUNC
399400
};
400401
static constexpr SerParams V1_NETWORK{{CNetAddr::Encoding::V1}, Format::Network};
401402
static constexpr SerParams V2_NETWORK{{CNetAddr::Encoding::V2}, Format::Network};

‎src/serialize.h‎

Lines changed: 70 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -167,9 +167,9 @@ const Out& AsBase(const In& x)
167167
return x;
168168
}
169169

170-
#define READWRITE(...) (::SerReadWriteMany(s, ser_action, __VA_ARGS__))
171-
#define SER_READ(obj, code) ::SerRead(s, ser_action, obj, [&](Stream& s, typename std::remove_const<Type>::type& obj) { code; })
172-
#define SER_WRITE(obj, code) ::SerWrite(s, ser_action, obj, [&](Stream& s, const Type& obj) { code; })
170+
#define READWRITE(...) (ser_action.SerReadWriteMany(s, __VA_ARGS__))
171+
#define SER_READ(obj, code) ser_action.SerRead(s, obj, [&](Stream& s, typename std::remove_const<Type>::type& obj) { code; })
172+
#define SER_WRITE(obj, code) ser_action.SerWrite(s, obj, [&](Stream& s, const Type& obj) { code; })
173173

174174
/**
175175
* Implement the Ser and Unser methods needed for implementing a formatter (see Using below).
@@ -1008,17 +1008,65 @@ void Unserialize(Stream& is, std::shared_ptr<const T>& p)
10081008
p = std::make_shared<const T>(deserialize, is);
10091009
}
10101010

1011+
/**
1012+
* Support for (un)serializing many things at once
1013+
*/
1014+
1015+
template <typename Stream, typename... Args>
1016+
void SerializeMany(Stream& s, const Args&... args)
1017+
{
1018+
(::Serialize(s, args), ...);
1019+
}
1020+
1021+
template <typename Stream, typename... Args>
1022+
inline void UnserializeMany(Stream& s, Args&&... args)
1023+
{
1024+
(::Unserialize(s, args), ...);
1025+
}
10111026

10121027
/**
10131028
* Support for all macros providing or using the ser_action parameter of the SerializationOps method.
10141029
*/
10151030
struct ActionSerialize {
1016-
constexpr bool ForRead() const { return false; }
1031+
static constexpr bool ForRead() { return false; }
1032+
1033+
template<typename Stream, typename... Args>
1034+
static void SerReadWriteMany(Stream& s, const Args&... args)
1035+
{
1036+
::SerializeMany(s, args...);
1037+
}
1038+
1039+
template<typename Stream, typename Type, typename Fn>
1040+
static void SerRead(Stream& s, Type&&, Fn&&)
1041+
{
1042+
}
1043+
1044+
template<typename Stream, typename Type, typename Fn>
1045+
static void SerWrite(Stream& s, Type&& obj, Fn&& fn)
1046+
{
1047+
fn(s, std::forward<Type>(obj));
1048+
}
10171049
};
10181050
struct ActionUnserialize {
1019-
constexpr bool ForRead() const { return true; }
1020-
};
1051+
static constexpr bool ForRead() { return true; }
10211052

1053+
template<typename Stream, typename... Args>
1054+
static void SerReadWriteMany(Stream& s, Args&&... args)
1055+
{
1056+
::UnserializeMany(s, args...);
1057+
}
1058+
1059+
template<typename Stream, typename Type, typename Fn>
1060+
static void SerRead(Stream& s, Type&& obj, Fn&& fn)
1061+
{
1062+
fn(s, std::forward<Type>(obj));
1063+
}
1064+
1065+
template<typename Stream, typename Type, typename Fn>
1066+
static void SerWrite(Stream& s, Type&&, Fn&&)
1067+
{
1068+
}
1069+
};
10221070

10231071
/* ::GetSerializeSize implementations
10241072
*
@@ -1065,52 +1113,6 @@ class CSizeComputer
10651113
int GetVersion() const { return nVersion; }
10661114
};
10671115

1068-
template <typename Stream, typename... Args>
1069-
void SerializeMany(Stream& s, const Args&... args)
1070-
{
1071-
(::Serialize(s, args), ...);
1072-
}
1073-
1074-
template <typename Stream, typename... Args>
1075-
inline void UnserializeMany(Stream& s, Args&&... args)
1076-
{
1077-
(::Unserialize(s, args), ...);
1078-
}
1079-
1080-
template<typename Stream, typename... Args>
1081-
inline void SerReadWriteMany(Stream& s, ActionSerialize ser_action, const Args&... args)
1082-
{
1083-
::SerializeMany(s, args...);
1084-
}
1085-
1086-
template<typename Stream, typename... Args>
1087-
inline void SerReadWriteMany(Stream& s, ActionUnserialize ser_action, Args&&... args)
1088-
{
1089-
::UnserializeMany(s, args...);
1090-
}
1091-
1092-
template<typename Stream, typename Type, typename Fn>
1093-
inline void SerRead(Stream& s, ActionSerialize ser_action, Type&&, Fn&&)
1094-
{
1095-
}
1096-
1097-
template<typename Stream, typename Type, typename Fn>
1098-
inline void SerRead(Stream& s, ActionUnserialize ser_action, Type&& obj, Fn&& fn)
1099-
{
1100-
fn(s, std::forward<Type>(obj));
1101-
}
1102-
1103-
template<typename Stream, typename Type, typename Fn>
1104-
inline void SerWrite(Stream& s, ActionSerialize ser_action, Type&& obj, Fn&& fn)
1105-
{
1106-
fn(s, std::forward<Type>(obj));
1107-
}
1108-
1109-
template<typename Stream, typename Type, typename Fn>
1110-
inline void SerWrite(Stream& s, ActionUnserialize ser_action, Type&&, Fn&&)
1111-
{
1112-
}
1113-
11141116
template<typename I>
11151117
inline void WriteVarInt(CSizeComputer &s, I n)
11161118
{
@@ -1161,12 +1163,11 @@ class ParamsStream
11611163
template <typename Params, typename T>
11621164
class ParamsWrapper
11631165
{
1164-
static_assert(std::is_lvalue_reference<T>::value, "ParamsWrapper needs an lvalue reference type T");
11651166
const Params& m_params;
1166-
T m_object;
1167+
T& m_object;
11671168

11681169
public:
1169-
explicit ParamsWrapper(const Params& params, T obj) : m_params{params}, m_object{obj} {}
1170+
explicit ParamsWrapper(const Params& params, T& obj) : m_params{params}, m_object{obj} {}
11701171

11711172
template <typename Stream>
11721173
void Serialize(Stream& s) const
@@ -1190,7 +1191,20 @@ class ParamsWrapper
11901191
template <typename Params, typename T>
11911192
static auto WithParams(const Params& params, T&& t)
11921193
{
1193-
return ParamsWrapper<Params, T&>{params, t};
1194+
return ParamsWrapper<Params, T>{params, t};
11941195
}
11951196

1197+
/**
1198+
* Helper macro for SerParams structs
1199+
*
1200+
* Allows you define SerParams instances and then apply them directly
1201+
* to an object via function call syntax, eg:
1202+
*
1203+
* constexpr SerParams FOO{....};
1204+
* ss << FOO(obj);
1205+
*/
1206+
#define SER_PARAMS_OPFUNC \
1207+
template <typename T> \
1208+
auto operator()(T&& t) const { return WithParams(*this, t); }
1209+
11961210
#endif // BITCOIN_SERIALIZE_H

‎src/test/addrman_tests.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1019,7 +1019,7 @@ static auto MakeCorruptPeersDat()
10191019
std::optional<CNetAddr> resolved{LookupHost("252.2.2.2", false)};
10201020
BOOST_REQUIRE(resolved.has_value());
10211021
AddrInfo info = AddrInfo(addr, resolved.value());
1022-
s << WithParams(CAddress::V1_DISK, info);
1022+
s << CAddress::V1_DISK(info);
10231023

10241024
return s;
10251025
}

‎src/test/fuzz/addrman.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ CNetAddr RandAddr(FuzzedDataProvider& fuzzed_data_provider, FastRandomContext& f
8383
s << net;
8484
s << fast_random_context.randbytes(net_len_map.at(net));
8585

86-
s >> WithParams(CAddress::V2_NETWORK, addr);
86+
s >> CAddress::V2_NETWORK(addr);
8787
}
8888

8989
// Return a dummy IPv4 5.5.5.5 if we generated an invalid address.

‎src/test/net_tests.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -850,7 +850,7 @@ BOOST_AUTO_TEST_CASE(initial_advertise_from_version_message)
850850
std::chrono::microseconds time_received_dummy{0};
851851

852852
const auto msg_version =
853-
msg_maker.Make(NetMsgType::VERSION, PROTOCOL_VERSION, services, time, services, WithParams(CAddress::V1_NETWORK, peer_us));
853+
msg_maker.Make(NetMsgType::VERSION, PROTOCOL_VERSION, services, time, services, CAddress::V1_NETWORK(peer_us));
854854
CDataStream msg_version_stream{msg_version.data, SER_NETWORK, PROTOCOL_VERSION};
855855

856856
m_node.peerman->ProcessMessage(
@@ -876,7 +876,7 @@ BOOST_AUTO_TEST_CASE(initial_advertise_from_version_message)
876876
DataStream s{data};
877877
std::vector<CAddress> addresses;
878878

879-
s >> WithParams(CAddress::V1_NETWORK, addresses);
879+
s >> CAddress::V1_NETWORK(addresses);
880880

881881
for (const auto& addr : addresses) {
882882
if (addr == expected) {

‎src/test/netbase_tests.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -561,7 +561,7 @@ BOOST_AUTO_TEST_CASE(caddress_serialize_v1)
561561
{
562562
DataStream s{};
563563

564-
s << WithParams(CAddress::V1_NETWORK, fixture_addresses);
564+
s << CAddress::V1_NETWORK(fixture_addresses);
565565
BOOST_CHECK_EQUAL(HexStr(s), stream_addrv1_hex);
566566
}
567567

@@ -570,15 +570,15 @@ BOOST_AUTO_TEST_CASE(caddress_unserialize_v1)
570570
DataStream s{ParseHex(stream_addrv1_hex)};
571571
std::vector<CAddress> addresses_unserialized;
572572

573-
s >> WithParams(CAddress::V1_NETWORK, addresses_unserialized);
573+
s >> CAddress::V1_NETWORK(addresses_unserialized);
574574
BOOST_CHECK(fixture_addresses == addresses_unserialized);
575575
}
576576

577577
BOOST_AUTO_TEST_CASE(caddress_serialize_v2)
578578
{
579579
DataStream s{};
580580

581-
s << WithParams(CAddress::V2_NETWORK, fixture_addresses);
581+
s << CAddress::V2_NETWORK(fixture_addresses);
582582
BOOST_CHECK_EQUAL(HexStr(s), stream_addrv2_hex);
583583
}
584584

@@ -587,7 +587,7 @@ BOOST_AUTO_TEST_CASE(caddress_unserialize_v2)
587587
DataStream s{ParseHex(stream_addrv2_hex)};
588588
std::vector<CAddress> addresses_unserialized;
589589

590-
s >> WithParams(CAddress::V2_NETWORK, addresses_unserialized);
590+
s >> CAddress::V2_NETWORK(addresses_unserialized);
591591
BOOST_CHECK(fixture_addresses == addresses_unserialized);
592592
}
593593

‎src/test/util/net.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,9 @@ void ConnmanTestMsg::Handshake(CNode& node,
3333
Using<CustomUintFormatter<8>>(remote_services), //
3434
int64_t{}, // dummy time
3535
int64_t{}, // ignored service bits
36-
WithParams(CNetAddr::V1, CService{}), // dummy
36+
CNetAddr::V1(CService{}), // dummy
3737
int64_t{}, // ignored service bits
38-
WithParams(CNetAddr::V1, CService{}), // ignored
38+
CNetAddr::V1(CService{}), // ignored
3939
uint64_t{1}, // dummy nonce
4040
std::string{}, // dummy subver
4141
int32_t{}, // dummy starting_height

0 commit comments

Comments
 (0)