Skip to content

Commit e58605e

Browse files
achow101achow101
committed
Merge bitcoin/bitcoin#31854: net: reduce CAddress usage to CService or CNetAddr
cd4bfae net: reduce CAddress usage to CService or CNetAddr (Vasil Dimov) Pull request description: 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`. ACKs for top commit: Sjors: ACK cd4bfae achow101: ACK cd4bfae hodlinator: ACK cd4bfae laanwj: Code review ACK cd4bfae Tree-SHA512: 0b41c1519784eeeaf9926c6a4d24f583b90c3376741f37a3199a3808b0dd6d143d3f929bd7c06f87b031f4fc1c2bd7a6dfc7d715ec1f79bf36b862c00fd67085
2 parents 06b9236 + cd4bfae commit e58605e

File tree

3 files changed

+23
-24
lines changed

3 files changed

+23
-24
lines changed

‎src/net.cpp‎

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -380,10 +380,10 @@ bool CConnman::CheckIncomingNonce(uint64_t nonce)
380380
return true;
381381
}
382382

383-
/** Get the bind address for a socket as CAddress */
384-
static CAddress GetBindAddress(const Sock& sock)
383+
/** Get the bind address for a socket as CService. */
384+
static CService GetBindAddress(const Sock& sock)
385385
{
386-
CAddress addr_bind;
386+
CService addr_bind;
387387
struct sockaddr_storage sockaddr_bind;
388388
socklen_t sockaddr_bind_len = sizeof(sockaddr_bind);
389389
if (!sock.GetSockName((struct sockaddr*)&sockaddr_bind, &sockaddr_bind_len)) {
@@ -458,7 +458,7 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
458458
// Connect
459459
std::unique_ptr<Sock> sock;
460460
Proxy proxy;
461-
CAddress addr_bind;
461+
CService addr_bind;
462462
assert(!addr_bind.IsValid());
463463
std::unique_ptr<i2p::sam::Session> i2p_transient_session;
464464

@@ -495,7 +495,7 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
495495

496496
if (connected) {
497497
sock = std::move(conn.sock);
498-
addr_bind = CAddress{conn.me, NODE_NONE};
498+
addr_bind = conn.me;
499499
}
500500
} else if (use_proxy) {
501501
LogPrintLevel(BCLog::PROXY, BCLog::Level::Debug, "Using proxy: %s to connect to %s\n", proxy.ToString(), target_addr.ToStringAddrPort());
@@ -1736,7 +1736,6 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) {
17361736
struct sockaddr_storage sockaddr;
17371737
socklen_t len = sizeof(sockaddr);
17381738
auto sock = hListenSocket.sock->Accept((struct sockaddr*)&sockaddr, &len);
1739-
CAddress addr;
17401739

17411740
if (!sock) {
17421741
const int nErr = WSAGetLastError();
@@ -1746,13 +1745,14 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) {
17461745
return;
17471746
}
17481747

1748+
CService addr;
17491749
if (!addr.SetSockAddr((const struct sockaddr*)&sockaddr, len)) {
17501750
LogPrintLevel(BCLog::NET, BCLog::Level::Warning, "Unknown socket family\n");
17511751
} else {
1752-
addr = CAddress{MaybeFlipIPv6toCJDNS(addr), NODE_NONE};
1752+
addr = MaybeFlipIPv6toCJDNS(addr);
17531753
}
17541754

1755-
const CAddress addr_bind{MaybeFlipIPv6toCJDNS(GetBindAddress(*sock)), NODE_NONE};
1755+
const CService addr_bind{MaybeFlipIPv6toCJDNS(GetBindAddress(*sock))};
17561756

17571757
NetPermissionFlags permission_flags = NetPermissionFlags::None;
17581758
hListenSocket.AddSocketPermissionFlags(permission_flags);
@@ -1762,8 +1762,8 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) {
17621762

17631763
void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr<Sock>&& sock,
17641764
NetPermissionFlags permission_flags,
1765-
const CAddress& addr_bind,
1766-
const CAddress& addr)
1765+
const CService& addr_bind,
1766+
const CService& addr)
17671767
{
17681768
int nInbound = 0;
17691769

@@ -1830,7 +1830,7 @@ void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr<Sock>&& sock,
18301830

18311831
CNode* pnode = new CNode(id,
18321832
std::move(sock),
1833-
addr,
1833+
CAddress{addr, NODE_NONE},
18341834
CalculateKeyedNetGroup(addr),
18351835
nonce,
18361836
addr_bind,
@@ -3105,8 +3105,7 @@ void CConnman::ThreadI2PAcceptIncoming()
31053105
continue;
31063106
}
31073107

3108-
CreateNodeFromAcceptedSocket(std::move(conn.sock), NetPermissionFlags::None,
3109-
CAddress{conn.me, NODE_NONE}, CAddress{conn.peer, NODE_NONE});
3108+
CreateNodeFromAcceptedSocket(std::move(conn.sock), NetPermissionFlags::None, conn.me, conn.peer);
31103109

31113110
err_wait = err_wait_begin;
31123111
}
@@ -3796,7 +3795,7 @@ CNode::CNode(NodeId idIn,
37963795
const CAddress& addrIn,
37973796
uint64_t nKeyedNetGroupIn,
37983797
uint64_t nLocalHostNonceIn,
3799-
const CAddress& addrBindIn,
3798+
const CService& addrBindIn,
38003799
const std::string& addrNameIn,
38013800
ConnectionType conn_type_in,
38023801
bool inbound_onion,
@@ -3933,7 +3932,7 @@ CSipHasher CConnman::GetDeterministicRandomizer(uint64_t id) const
39333932
return CSipHasher(nSeed0, nSeed1).Write(id);
39343933
}
39353934

3936-
uint64_t CConnman::CalculateKeyedNetGroup(const CAddress& address) const
3935+
uint64_t CConnman::CalculateKeyedNetGroup(const CNetAddr& address) const
39373936
{
39383937
std::vector<unsigned char> vchNetGroup(m_netgroupman.GetGroup(address));
39393938

‎src/net.h‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ class CNodeStats
211211
// Address of this peer
212212
CAddress addr;
213213
// Bind address of our side of the connection
214-
CAddress addrBind;
214+
CService addrBind;
215215
// Network the peer connected through
216216
Network m_network;
217217
uint32_t m_mapped_as;
@@ -707,7 +707,7 @@ class CNode
707707
// Address of this peer
708708
const CAddress addr;
709709
// Bind address of our side of the connection
710-
const CAddress addrBind;
710+
const CService addrBind;
711711
const std::string m_addr_name;
712712
/** The pszDest argument provided to ConnectNode(). Only used for reconnections. */
713713
const std::string m_dest;
@@ -883,7 +883,7 @@ class CNode
883883
const CAddress& addrIn,
884884
uint64_t nKeyedNetGroupIn,
885885
uint64_t nLocalHostNonceIn,
886-
const CAddress& addrBindIn,
886+
const CService& addrBindIn,
887887
const std::string& addrNameIn,
888888
ConnectionType conn_type_in,
889889
bool inbound_onion,
@@ -1312,8 +1312,8 @@ class CConnman
13121312
*/
13131313
void CreateNodeFromAcceptedSocket(std::unique_ptr<Sock>&& sock,
13141314
NetPermissionFlags permission_flags,
1315-
const CAddress& addr_bind,
1316-
const CAddress& addr);
1315+
const CService& addr_bind,
1316+
const CService& addr);
13171317

13181318
void DisconnectNodes() EXCLUSIVE_LOCKS_REQUIRED(!m_reconnections_mutex, !m_nodes_mutex);
13191319
void NotifyNumConnectionsChanged();
@@ -1350,7 +1350,7 @@ class CConnman
13501350
void ThreadSocketHandler() EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex, !mutexMsgProc, !m_nodes_mutex, !m_reconnections_mutex);
13511351
void ThreadDNSAddressSeed() EXCLUSIVE_LOCKS_REQUIRED(!m_addr_fetches_mutex, !m_nodes_mutex);
13521352

1353-
uint64_t CalculateKeyedNetGroup(const CAddress& ad) const;
1353+
uint64_t CalculateKeyedNetGroup(const CNetAddr& ad) const;
13541354

13551355
CNode* FindNode(const CNetAddr& ip);
13561356
CNode* FindNode(const std::string& addrName);

‎src/test/net_tests.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -671,7 +671,7 @@ BOOST_AUTO_TEST_CASE(get_local_addr_for_peer_port)
671671
/*addrIn=*/CAddress{CService{peer_out_in_addr, 8333}, NODE_NETWORK},
672672
/*nKeyedNetGroupIn=*/0,
673673
/*nLocalHostNonceIn=*/0,
674-
/*addrBindIn=*/CAddress{},
674+
/*addrBindIn=*/CService{},
675675
/*addrNameIn=*/std::string{},
676676
/*conn_type_in=*/ConnectionType::OUTBOUND_FULL_RELAY,
677677
/*inbound_onion=*/false};
@@ -692,7 +692,7 @@ BOOST_AUTO_TEST_CASE(get_local_addr_for_peer_port)
692692
/*addrIn=*/CAddress{CService{peer_in_in_addr, 8333}, NODE_NETWORK},
693693
/*nKeyedNetGroupIn=*/0,
694694
/*nLocalHostNonceIn=*/0,
695-
/*addrBindIn=*/CAddress{},
695+
/*addrBindIn=*/CService{},
696696
/*addrNameIn=*/std::string{},
697697
/*conn_type_in=*/ConnectionType::INBOUND,
698698
/*inbound_onion=*/false};
@@ -829,7 +829,7 @@ BOOST_AUTO_TEST_CASE(initial_advertise_from_version_message)
829829
/*addrIn=*/CAddress{CService{peer_in_addr, 8333}, NODE_NETWORK},
830830
/*nKeyedNetGroupIn=*/0,
831831
/*nLocalHostNonceIn=*/0,
832-
/*addrBindIn=*/CAddress{},
832+
/*addrBindIn=*/CService{},
833833
/*addrNameIn=*/std::string{},
834834
/*conn_type_in=*/ConnectionType::OUTBOUND_FULL_RELAY,
835835
/*inbound_onion=*/false};

0 commit comments

Comments
 (0)