Skip to content

Commit cb073be

Browse files
author
MarcoFalke
committed
Merge #21167: net: make CNode::m_inbound_onion public, initialize explicitly
2ee4a7a net: remove CNode::m_inbound_onion defaults for explicitness (Jon Atack) 24bda56 net: make CNode::m_inbound_onion public, drop getter, update tests (Jon Atack) Pull request description: Refactoring only, no change in behavior. This is a quick follow-up to #20210 to address these review comments: - bitcoin/bitcoin#20210 (comment) - bitcoin/bitcoin#20210 (comment) - bitcoin/bitcoin#20210 (comment) Changes: - make the `CNode::m_inbound_onion class` member public, update the Doxygen comment, drop the getter, and update the tests - remove the `CNode::m_inbound_onion` default value initialization in the ctor declaration and the member initializer in favor of always passing it explicitly to the ctor where we initialize it dynamically, to both clarify the caller code and to allow the compiler to warn if it is uninitialized in the ctor or omitted in the caller ACKs for top commit: MarcoFalke: review ACK 2ee4a7a 🏀 vasild: ACK 2ee4a7a Tree-SHA512: 72961c91168885a9d881756b10bad9d587f5ce297d5a6493c23e267b7178ff22b697bc6868e7761d6304e372d2781453d30011a020afd506b1e308b4ffa5feee
2 parents d19639d + 2ee4a7a commit cb073be

File tree

4 files changed

+18
-21
lines changed

4 files changed

+18
-21
lines changed

‎src/net.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -476,7 +476,7 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
476476
NodeId id = GetNewNodeId();
477477
uint64_t nonce = GetDeterministicRandomizer(RANDOMIZER_ID_LOCALHOSTNONCE).Write(id).Finalize();
478478
CAddress addr_bind = GetBindAddress(sock->Get());
479-
CNode* pnode = new CNode(id, nLocalServices, sock->Release(), addrConnect, CalculateKeyedNetGroup(addrConnect), nonce, addr_bind, pszDest ? pszDest : "", conn_type);
479+
CNode* pnode = new CNode(id, nLocalServices, sock->Release(), addrConnect, CalculateKeyedNetGroup(addrConnect), nonce, addr_bind, pszDest ? pszDest : "", conn_type, /* inbound_onion */ false);
480480
pnode->AddRef();
481481

482482
// We're making a new connection, harvest entropy from the time (and our peer count)
@@ -2855,12 +2855,12 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, SOCKET hSocketIn, const
28552855
: nTimeConnected(GetSystemTimeInSeconds()),
28562856
addr(addrIn),
28572857
addrBind(addrBindIn),
2858+
m_inbound_onion(inbound_onion),
28582859
nKeyedNetGroup(nKeyedNetGroupIn),
28592860
id(idIn),
28602861
nLocalHostNonce(nLocalHostNonceIn),
28612862
m_conn_type(conn_type_in),
2862-
nLocalServices(nLocalServicesIn),
2863-
m_inbound_onion(inbound_onion)
2863+
nLocalServices(nLocalServicesIn)
28642864
{
28652865
if (inbound_onion) assert(conn_type_in == ConnectionType::INBOUND);
28662866
hSocket = hSocketIn;

‎src/net.h

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -429,6 +429,8 @@ class CNode
429429
const CAddress addr;
430430
// Bind address of our side of the connection
431431
const CAddress addrBind;
432+
//! Whether this peer is an inbound onion, i.e. connected via our Tor onion service.
433+
const bool m_inbound_onion;
432434
std::atomic<int> nVersion{0};
433435
RecursiveMutex cs_SubVer;
434436
/**
@@ -601,7 +603,7 @@ class CNode
601603
// Whether a ping is requested.
602604
std::atomic<bool> fPingQueued{false};
603605

604-
CNode(NodeId id, ServiceFlags nLocalServicesIn, SOCKET hSocketIn, const CAddress& addrIn, uint64_t nKeyedNetGroupIn, uint64_t nLocalHostNonceIn, const CAddress& addrBindIn, const std::string& addrNameIn, ConnectionType conn_type_in, bool inbound_onion = false);
606+
CNode(NodeId id, ServiceFlags nLocalServicesIn, SOCKET hSocketIn, const CAddress& addrIn, uint64_t nKeyedNetGroupIn, uint64_t nLocalHostNonceIn, const CAddress& addrBindIn, const std::string& addrNameIn, ConnectionType conn_type_in, bool inbound_onion);
605607
~CNode();
606608
CNode(const CNode&) = delete;
607609
CNode& operator=(const CNode&) = delete;
@@ -719,9 +721,6 @@ class CNode
719721

720722
std::string ConnectionTypeAsString() const { return ::ConnectionTypeAsString(m_conn_type); }
721723

722-
/** Whether this peer is an inbound onion, e.g. connected via our Tor onion service. */
723-
bool IsInboundOnion() const { return m_inbound_onion; }
724-
725724
private:
726725
const NodeId id;
727726
const uint64_t nLocalHostNonce;
@@ -754,9 +753,6 @@ class CNode
754753
CService addrLocal GUARDED_BY(cs_addrLocal);
755754
mutable RecursiveMutex cs_addrLocal;
756755

757-
//! Whether this peer is an inbound onion, e.g. connected via our Tor onion service.
758-
const bool m_inbound_onion{false};
759-
760756
mapMsgCmdSize mapSendBytesPerMsgCmd GUARDED_BY(cs_vSend);
761757
mapMsgCmdSize mapRecvBytesPerMsgCmd GUARDED_BY(cs_vRecv);
762758
};

‎src/test/denialofservice_tests.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction)
8585

8686
// Mock an outbound peer
8787
CAddress addr1(ip(0xa0b0c001), NODE_NONE);
88-
CNode dummyNode1(id++, ServiceFlags(NODE_NETWORK | NODE_WITNESS), INVALID_SOCKET, addr1, 0, 0, CAddress(), "", ConnectionType::OUTBOUND_FULL_RELAY);
88+
CNode dummyNode1(id++, ServiceFlags(NODE_NETWORK | NODE_WITNESS), INVALID_SOCKET, addr1, /* nKeyedNetGroupIn */ 0, /* nLocalHostNonceIn */ 0, CAddress(), /* pszDest */ "", ConnectionType::OUTBOUND_FULL_RELAY, /* inbound_onion */ false);
8989
dummyNode1.SetCommonVersion(PROTOCOL_VERSION);
9090

9191
peerLogic->InitializeNode(&dummyNode1);
@@ -136,7 +136,7 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction)
136136
static void AddRandomOutboundPeer(std::vector<CNode *> &vNodes, PeerManager &peerLogic, CConnmanTest* connman)
137137
{
138138
CAddress addr(ip(g_insecure_rand_ctx.randbits(32)), NODE_NONE);
139-
vNodes.emplace_back(new CNode(id++, ServiceFlags(NODE_NETWORK | NODE_WITNESS), INVALID_SOCKET, addr, 0, 0, CAddress(), "", ConnectionType::OUTBOUND_FULL_RELAY));
139+
vNodes.emplace_back(new CNode(id++, ServiceFlags(NODE_NETWORK | NODE_WITNESS), INVALID_SOCKET, addr, /* nKeyedNetGroupIn */ 0, /* nLocalHostNonceIn */ 0, CAddress(), /* pszDest */ "", ConnectionType::OUTBOUND_FULL_RELAY, /* inbound_onion */ false));
140140
CNode &node = *vNodes.back();
141141
node.SetCommonVersion(PROTOCOL_VERSION);
142142

@@ -229,7 +229,7 @@ BOOST_AUTO_TEST_CASE(peer_discouragement)
229229

230230
banman->ClearBanned();
231231
CAddress addr1(ip(0xa0b0c001), NODE_NONE);
232-
CNode dummyNode1(id++, NODE_NETWORK, INVALID_SOCKET, addr1, 0, 0, CAddress(), "", ConnectionType::INBOUND);
232+
CNode dummyNode1(id++, NODE_NETWORK, INVALID_SOCKET, addr1, /* nKeyedNetGroupIn */ 0, /* nLocalHostNonceIn */ 0, CAddress(), /* pszDest */ "", ConnectionType::INBOUND, /* inbound_onion */ false);
233233
dummyNode1.SetCommonVersion(PROTOCOL_VERSION);
234234
peerLogic->InitializeNode(&dummyNode1);
235235
dummyNode1.fSuccessfullyConnected = true;
@@ -242,7 +242,7 @@ BOOST_AUTO_TEST_CASE(peer_discouragement)
242242
BOOST_CHECK(!banman->IsDiscouraged(ip(0xa0b0c001|0x0000ff00))); // Different IP, not discouraged
243243

244244
CAddress addr2(ip(0xa0b0c002), NODE_NONE);
245-
CNode dummyNode2(id++, NODE_NETWORK, INVALID_SOCKET, addr2, 1, 1, CAddress(), "", ConnectionType::INBOUND);
245+
CNode dummyNode2(id++, NODE_NETWORK, INVALID_SOCKET, addr2, /* nKeyedNetGroupIn */ 1, /* nLocalHostNonceIn */ 1, CAddress(), /* pszDest */ "", ConnectionType::INBOUND, /* inbound_onion */ false);
246246
dummyNode2.SetCommonVersion(PROTOCOL_VERSION);
247247
peerLogic->InitializeNode(&dummyNode2);
248248
dummyNode2.fSuccessfullyConnected = true;
@@ -279,7 +279,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime)
279279
SetMockTime(nStartTime); // Overrides future calls to GetTime()
280280

281281
CAddress addr(ip(0xa0b0c001), NODE_NONE);
282-
CNode dummyNode(id++, NODE_NETWORK, INVALID_SOCKET, addr, 4, 4, CAddress(), "", ConnectionType::INBOUND);
282+
CNode dummyNode(id++, NODE_NETWORK, INVALID_SOCKET, addr, /* nKeyedNetGroupIn */ 4, /* nLocalHostNonceIn */ 4, CAddress(), /* pszDest */ "", ConnectionType::INBOUND, /* inbound_onion */ false);
283283
dummyNode.SetCommonVersion(PROTOCOL_VERSION);
284284
peerLogic->InitializeNode(&dummyNode);
285285
dummyNode.fSuccessfullyConnected = true;

‎src/test/net_tests.cpp

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -192,14 +192,15 @@ BOOST_AUTO_TEST_CASE(cnode_simple_test)
192192
id++, NODE_NETWORK, hSocket, addr,
193193
/* nKeyedNetGroupIn = */ 0,
194194
/* nLocalHostNonceIn = */ 0,
195-
CAddress(), pszDest, ConnectionType::OUTBOUND_FULL_RELAY);
195+
CAddress(), pszDest, ConnectionType::OUTBOUND_FULL_RELAY,
196+
/* inbound_onion = */ false);
196197
BOOST_CHECK(pnode1->IsFullOutboundConn() == true);
197198
BOOST_CHECK(pnode1->IsManualConn() == false);
198199
BOOST_CHECK(pnode1->IsBlockOnlyConn() == false);
199200
BOOST_CHECK(pnode1->IsFeelerConn() == false);
200201
BOOST_CHECK(pnode1->IsAddrFetchConn() == false);
201202
BOOST_CHECK(pnode1->IsInboundConn() == false);
202-
BOOST_CHECK(pnode1->IsInboundOnion() == false);
203+
BOOST_CHECK(pnode1->m_inbound_onion == false);
203204
BOOST_CHECK_EQUAL(pnode1->ConnectedThroughNetwork(), Network::NET_IPV4);
204205

205206
std::unique_ptr<CNode> pnode2 = MakeUnique<CNode>(
@@ -214,7 +215,7 @@ BOOST_AUTO_TEST_CASE(cnode_simple_test)
214215
BOOST_CHECK(pnode2->IsFeelerConn() == false);
215216
BOOST_CHECK(pnode2->IsAddrFetchConn() == false);
216217
BOOST_CHECK(pnode2->IsInboundConn() == true);
217-
BOOST_CHECK(pnode2->IsInboundOnion() == false);
218+
BOOST_CHECK(pnode2->m_inbound_onion == false);
218219
BOOST_CHECK_EQUAL(pnode2->ConnectedThroughNetwork(), Network::NET_IPV4);
219220

220221
std::unique_ptr<CNode> pnode3 = MakeUnique<CNode>(
@@ -229,7 +230,7 @@ BOOST_AUTO_TEST_CASE(cnode_simple_test)
229230
BOOST_CHECK(pnode3->IsFeelerConn() == false);
230231
BOOST_CHECK(pnode3->IsAddrFetchConn() == false);
231232
BOOST_CHECK(pnode3->IsInboundConn() == false);
232-
BOOST_CHECK(pnode3->IsInboundOnion() == false);
233+
BOOST_CHECK(pnode3->m_inbound_onion == false);
233234
BOOST_CHECK_EQUAL(pnode3->ConnectedThroughNetwork(), Network::NET_IPV4);
234235

235236
std::unique_ptr<CNode> pnode4 = MakeUnique<CNode>(
@@ -244,7 +245,7 @@ BOOST_AUTO_TEST_CASE(cnode_simple_test)
244245
BOOST_CHECK(pnode4->IsFeelerConn() == false);
245246
BOOST_CHECK(pnode4->IsAddrFetchConn() == false);
246247
BOOST_CHECK(pnode4->IsInboundConn() == true);
247-
BOOST_CHECK(pnode4->IsInboundOnion() == true);
248+
BOOST_CHECK(pnode4->m_inbound_onion == true);
248249
BOOST_CHECK_EQUAL(pnode4->ConnectedThroughNetwork(), Network::NET_ONION);
249250
}
250251

@@ -679,7 +680,7 @@ BOOST_AUTO_TEST_CASE(ipv4_peer_with_ipv6_addrMe_test)
679680
in_addr ipv4AddrPeer;
680681
ipv4AddrPeer.s_addr = 0xa0b0c001;
681682
CAddress addr = CAddress(CService(ipv4AddrPeer, 7777), NODE_NETWORK);
682-
std::unique_ptr<CNode> pnode = MakeUnique<CNode>(0, NODE_NETWORK, INVALID_SOCKET, addr, 0, 0, CAddress{}, std::string{}, ConnectionType::OUTBOUND_FULL_RELAY);
683+
std::unique_ptr<CNode> pnode = MakeUnique<CNode>(0, NODE_NETWORK, INVALID_SOCKET, addr, /* nKeyedNetGroupIn */ 0, /* nLocalHostNonceIn */ 0, CAddress{}, /* pszDest */ std::string{}, ConnectionType::OUTBOUND_FULL_RELAY, /* inbound_onion */ false);
683684
pnode->fSuccessfullyConnected.store(true);
684685

685686
// the peer claims to be reaching us via IPv6

0 commit comments

Comments
 (0)