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.
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.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
For detailed information about the code coverage, see the test coverage report.
See the guideline for information on the review process.
Type | Reviewers |
---|---|
ACK | ajtowns, TheCharlatan |
Concept ACK | theuni |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
Reviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
-BEGIN VERIFY SCRIPT-
sed -i 's|WithParams(\([a-zA-Z:._]\+\), |\1(|g' $( git grep -l WithParams )
-END VERIFY SCRIPT-
5385- }
5386- m_connman.PushMessage(&node, CNetMsgMaker(node.GetCommonVersion()).Make(msg_type, WithParams(CAddress::SerParams{{ser_enc}, CAddress::Format::Network}, peer.m_addrs_to_send)));
5387+ const bool wants_addrv2{peer.m_wants_addrv2};
5388+ const auto& msg_type{wants_addrv2 ? NetMsgType::ADDRV2 : NetMsgType::ADDR};
5389+ const auto& ser_params{wants_addrv2 ? CAddress::V2_NETWORK : CAddress::V1_NETWORK};
5390+ m_connman.PushMessage(&node, CNetMsgMaker(node.GetCommonVersion()).Make(msg_type, ser_params(peer.m_addrs_to_send)));
Might have been clearer to write:
0CNetMsgMaker mm(node.GetCommonVersion());
1if (peer.m_wants_addrv2) {
2 m_connman.PushMessage(&node, mm.Make(NetMsgType::ADDRV2, CAddress::V2_NETWORK(peer.m_addrs_to_send)));
3} else {
4 m_connman.PushMessage(&node, mm.Make(NetMsgType::ADDR, CAddress::V1_NETWORK(peer.m_addrs_to_send)));
5}
Was thinking about moving PushMessage
from connman to cnode though (#28686 etc), so could reconsider this at that time.