Using CAddress when only CService or CNetAddr is needed is excessive and confusing. Fix those occurrences to use the class they need:
CConnman::CalculateKeyedNetGroup() needs CNetAddr, not CAddress, thus change its argument.
Both callers of CConnman::CreateNodeFromAcceptedSocket() create a dummy CAddress from CService, so use CService instead.
GetBindAddress() only needs to return CService.
CNode::addrBind only needs to be CService.
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--006a51241073e994b41acfe9ec718e94-->
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31854.
<!--021abf342d371248e50ceaed478a90ca-->
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | hodlinator, Sjors, laanwj, achow101 |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
<!--174a7506f384e20aa4161008e828411d-->
No conflicts as of last run.
* `CConnman::CalculateKeyedNetGroup()` needs `CNetAddr`, not `CAddress`,
thus change its argument.
* Both callers of `CConnman::CreateNodeFromAcceptedSocket()` create a
dummy `CAddress` from `CService`, so use `CService` instead.
* `GetBindAddress()` only needs to return `CService`.
* `CNode::addrBind` only needs to be `CService`.
380 | @@ -381,9 +381,9 @@ bool CConnman::CheckIncomingNonce(uint64_t nonce)
381 | }
382 |
383 | /** Get the bind address for a socket as CAddress */
Still documented as returning a CAddress?
Should be "CService", fixed, thanks!
74f42a5920...cd4bfaee10: address #31854 (review)
Concept ACK
ACK cd4bfaee103591f212b88afd17969c16c1056eb6
Defers "faking" of CAddress by initializing CAddress::nServices = NODE_NONE when all we actually have is a CService (network address+port), to all the way down to the construction of CNode:
graph LR;
CService-->CNetAddr;
CAddress-->CService;
CAddress, which inherits from CService, has non-explicit constructors taking either 0 or 2 arguments, so implicit conversion CService -> CAddress is not an issue (verified to give compiler error). CAddress -> CService is allowed due to implicitly generated CService copy-constructor, but should be acceptable. Similarly, CService -> CNetAddr works, but the inverse does not (also verified).
ACK cd4bfaee10
Code review ACK cd4bfaee103591f212b88afd17969c16c1056eb6
ACK cd4bfaee103591f212b88afd17969c16c1056eb6
Post-merge ACK cd4bfaee103591f212b88afd17969c16c1056eb6. Good idea.