foster/bitcoin-core
0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-03-05 14:06:27 -05:00
Code Issues Activity

Merge #21167: net: make CNode::m_inbound_onion public, initialize explicitly

Browse source 
2ee4a7a9ec net: remove CNode::m_inbound_onion defaults for explicitness (Jon Atack)
24bda56c29 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:

  - https://github.com/bitcoin/bitcoin/pull/20210#discussion_r528835313
  - https://github.com/bitcoin/bitcoin/pull/20210#discussion_r550860416
  - https://github.com/bitcoin/bitcoin/pull/20210#issuecomment-766093925

  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 2ee4a7a9ec 🏀
  vasild:
    ACK 2ee4a7a9ec

Tree-SHA512: 72961c91168885a9d881756b10bad9d587f5ce297d5a6493c23e267b7178ff22b697bc6868e7761d6304e372d2781453d30011a020afd506b1e308b4ffa5feee
This commit is contained in:
MarcoFalke 2021-02-15 15:20:20 +01:00
commit cb073bed00
No known key found for this signature in database
GPG key ID: D2EA4850E7528B25
4 changed files with 18 additions and 21 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
src/net.cpp
View file

@ -476,7 +476,7 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
NodeId id = GetNewNodeId(); NodeId id = GetNewNodeId();
uint64_t nonce = GetDeterministicRandomizer(RANDOMIZER_ID_LOCALHOSTNONCE).Write(id).Finalize(); uint64_t nonce = GetDeterministicRandomizer(RANDOMIZER_ID_LOCALHOSTNONCE).Write(id).Finalize();
CAddress addr_bind = GetBindAddress(sock->Get()); CAddress addr_bind = GetBindAddress(sock->Get());
CNode* pnode = new CNode(id, nLocalServices, sock->Release(), addrConnect, CalculateKeyedNetGroup(addrConnect), nonce, addr_bind, pszDest ? pszDest : "", conn_type); CNode* pnode = new CNode(id, nLocalServices, sock->Release(), addrConnect, CalculateKeyedNetGroup(addrConnect), nonce, addr_bind, pszDest ? pszDest : "", conn_type, /* inbound_onion */ false);
pnode->AddRef(); pnode->AddRef();
// We're making a new connection, harvest entropy from the time (and our peer count) // 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
: nTimeConnected(GetSystemTimeInSeconds()), : nTimeConnected(GetSystemTimeInSeconds()),
addr(addrIn), addr(addrIn),
addrBind(addrBindIn), addrBind(addrBindIn),
m_inbound_onion(inbound_onion),
nKeyedNetGroup(nKeyedNetGroupIn), nKeyedNetGroup(nKeyedNetGroupIn),
id(idIn), id(idIn),
nLocalHostNonce(nLocalHostNonceIn), nLocalHostNonce(nLocalHostNonceIn),
m_conn_type(conn_type_in), m_conn_type(conn_type_in),
nLocalServices(nLocalServicesIn), nLocalServices(nLocalServicesIn)
m_inbound_onion(inbound_onion)
{ {
if (inbound_onion) assert(conn_type_in == ConnectionType::INBOUND); if (inbound_onion) assert(conn_type_in == ConnectionType::INBOUND);
hSocket = hSocketIn; hSocket = hSocketIn;

10
src/net.h
View file

@ -429,6 +429,8 @@ public:
const CAddress addr; const CAddress addr;
// Bind address of our side of the connection // Bind address of our side of the connection
const CAddress addrBind; const CAddress addrBind;
//! Whether this peer is an inbound onion, i.e. connected via our Tor onion service.
const bool m_inbound_onion;
std::atomic<int> nVersion{0}; std::atomic<int> nVersion{0};
RecursiveMutex cs_SubVer; RecursiveMutex cs_SubVer;
/** /**
@ -601,7 +603,7 @@ public:
// Whether a ping is requested. // Whether a ping is requested.
std::atomic<bool> fPingQueued{false}; std::atomic<bool> fPingQueued{false};
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); 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);
~CNode(); ~CNode();
CNode(const CNode&) = delete; CNode(const CNode&) = delete;
CNode& operator=(const CNode&) = delete; CNode& operator=(const CNode&) = delete;
@ -719,9 +721,6 @@ public:
std::string ConnectionTypeAsString() const { return ::ConnectionTypeAsString(m_conn_type); } std::string ConnectionTypeAsString() const { return ::ConnectionTypeAsString(m_conn_type); }
/** Whether this peer is an inbound onion, e.g. connected via our Tor onion service. */
bool IsInboundOnion() const { return m_inbound_onion; }
private: private:
const NodeId id; const NodeId id;
const uint64_t nLocalHostNonce; const uint64_t nLocalHostNonce;
@ -754,9 +753,6 @@ private:
CService addrLocal GUARDED_BY(cs_addrLocal); CService addrLocal GUARDED_BY(cs_addrLocal);
mutable RecursiveMutex cs_addrLocal; mutable RecursiveMutex cs_addrLocal;
//! Whether this peer is an inbound onion, e.g. connected via our Tor onion service.
const bool m_inbound_onion{false};
mapMsgCmdSize mapSendBytesPerMsgCmd GUARDED_BY(cs_vSend); mapMsgCmdSize mapSendBytesPerMsgCmd GUARDED_BY(cs_vSend);
mapMsgCmdSize mapRecvBytesPerMsgCmd GUARDED_BY(cs_vRecv); mapMsgCmdSize mapRecvBytesPerMsgCmd GUARDED_BY(cs_vRecv);
}; };

10
src/test/denialofservice_tests.cpp
View file

@ -85,7 +85,7 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction)
// Mock an outbound peer // Mock an outbound peer
CAddress addr1(ip(0xa0b0c001), NODE_NONE); CAddress addr1(ip(0xa0b0c001), NODE_NONE);
CNode dummyNode1(id++, ServiceFlags(NODE_NETWORK | NODE_WITNESS), INVALID_SOCKET, addr1, 0, 0, CAddress(), "", ConnectionType::OUTBOUND_FULL_RELAY); CNode dummyNode1(id++, ServiceFlags(NODE_NETWORK | NODE_WITNESS), INVALID_SOCKET, addr1, /* nKeyedNetGroupIn */ 0, /* nLocalHostNonceIn */ 0, CAddress(), /* pszDest */ "", ConnectionType::OUTBOUND_FULL_RELAY, /* inbound_onion */ false);
dummyNode1.SetCommonVersion(PROTOCOL_VERSION); dummyNode1.SetCommonVersion(PROTOCOL_VERSION);
peerLogic->InitializeNode(&dummyNode1); peerLogic->InitializeNode(&dummyNode1);
@ -136,7 +136,7 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction)
static void AddRandomOutboundPeer(std::vector<CNode *> &vNodes, PeerManager &peerLogic, CConnmanTest* connman) static void AddRandomOutboundPeer(std::vector<CNode *> &vNodes, PeerManager &peerLogic, CConnmanTest* connman)
{ {
CAddress addr(ip(g_insecure_rand_ctx.randbits(32)), NODE_NONE); CAddress addr(ip(g_insecure_rand_ctx.randbits(32)), NODE_NONE);
vNodes.emplace_back(new CNode(id++, ServiceFlags(NODE_NETWORK | NODE_WITNESS), INVALID_SOCKET, addr, 0, 0, CAddress(), "", ConnectionType::OUTBOUND_FULL_RELAY)); 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));
CNode &node = *vNodes.back(); CNode &node = *vNodes.back();
node.SetCommonVersion(PROTOCOL_VERSION); node.SetCommonVersion(PROTOCOL_VERSION);
@ -229,7 +229,7 @@ BOOST_AUTO_TEST_CASE(peer_discouragement)
banman->ClearBanned(); banman->ClearBanned();
CAddress addr1(ip(0xa0b0c001), NODE_NONE); CAddress addr1(ip(0xa0b0c001), NODE_NONE);
CNode dummyNode1(id++, NODE_NETWORK, INVALID_SOCKET, addr1, 0, 0, CAddress(), "", ConnectionType::INBOUND); CNode dummyNode1(id++, NODE_NETWORK, INVALID_SOCKET, addr1, /* nKeyedNetGroupIn */ 0, /* nLocalHostNonceIn */ 0, CAddress(), /* pszDest */ "", ConnectionType::INBOUND, /* inbound_onion */ false);
dummyNode1.SetCommonVersion(PROTOCOL_VERSION); dummyNode1.SetCommonVersion(PROTOCOL_VERSION);
peerLogic->InitializeNode(&dummyNode1); peerLogic->InitializeNode(&dummyNode1);
dummyNode1.fSuccessfullyConnected = true; dummyNode1.fSuccessfullyConnected = true;
@ -242,7 +242,7 @@ BOOST_AUTO_TEST_CASE(peer_discouragement)
BOOST_CHECK(!banman->IsDiscouraged(ip(0xa0b0c001|0x0000ff00))); // Different IP, not discouraged BOOST_CHECK(!banman->IsDiscouraged(ip(0xa0b0c001|0x0000ff00))); // Different IP, not discouraged
CAddress addr2(ip(0xa0b0c002), NODE_NONE); CAddress addr2(ip(0xa0b0c002), NODE_NONE);
CNode dummyNode2(id++, NODE_NETWORK, INVALID_SOCKET, addr2, 1, 1, CAddress(), "", ConnectionType::INBOUND); CNode dummyNode2(id++, NODE_NETWORK, INVALID_SOCKET, addr2, /* nKeyedNetGroupIn */ 1, /* nLocalHostNonceIn */ 1, CAddress(), /* pszDest */ "", ConnectionType::INBOUND, /* inbound_onion */ false);
dummyNode2.SetCommonVersion(PROTOCOL_VERSION); dummyNode2.SetCommonVersion(PROTOCOL_VERSION);
peerLogic->InitializeNode(&dummyNode2); peerLogic->InitializeNode(&dummyNode2);
dummyNode2.fSuccessfullyConnected = true; dummyNode2.fSuccessfullyConnected = true;
@ -279,7 +279,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime)
SetMockTime(nStartTime); // Overrides future calls to GetTime() SetMockTime(nStartTime); // Overrides future calls to GetTime()
CAddress addr(ip(0xa0b0c001), NODE_NONE); CAddress addr(ip(0xa0b0c001), NODE_NONE);
CNode dummyNode(id++, NODE_NETWORK, INVALID_SOCKET, addr, 4, 4, CAddress(), "", ConnectionType::INBOUND); CNode dummyNode(id++, NODE_NETWORK, INVALID_SOCKET, addr, /* nKeyedNetGroupIn */ 4, /* nLocalHostNonceIn */ 4, CAddress(), /* pszDest */ "", ConnectionType::INBOUND, /* inbound_onion */ false);
dummyNode.SetCommonVersion(PROTOCOL_VERSION); dummyNode.SetCommonVersion(PROTOCOL_VERSION);
peerLogic->InitializeNode(&dummyNode); peerLogic->InitializeNode(&dummyNode);
dummyNode.fSuccessfullyConnected = true; dummyNode.fSuccessfullyConnected = true;

13
src/test/net_tests.cpp
View file

@ -192,14 +192,15 @@ BOOST_AUTO_TEST_CASE(cnode_simple_test)
id++, NODE_NETWORK, hSocket, addr, id++, NODE_NETWORK, hSocket, addr,
/* nKeyedNetGroupIn = */ 0, /* nKeyedNetGroupIn = */ 0,
/* nLocalHostNonceIn = */ 0, /* nLocalHostNonceIn = */ 0,
CAddress(), pszDest, ConnectionType::OUTBOUND_FULL_RELAY); CAddress(), pszDest, ConnectionType::OUTBOUND_FULL_RELAY,
/* inbound_onion = */ false);
BOOST_CHECK(pnode1->IsFullOutboundConn() == true); BOOST_CHECK(pnode1->IsFullOutboundConn() == true);
BOOST_CHECK(pnode1->IsManualConn() == false); BOOST_CHECK(pnode1->IsManualConn() == false);
BOOST_CHECK(pnode1->IsBlockOnlyConn() == false); BOOST_CHECK(pnode1->IsBlockOnlyConn() == false);
BOOST_CHECK(pnode1->IsFeelerConn() == false); BOOST_CHECK(pnode1->IsFeelerConn() == false);
BOOST_CHECK(pnode1->IsAddrFetchConn() == false); BOOST_CHECK(pnode1->IsAddrFetchConn() == false);
BOOST_CHECK(pnode1->IsInboundConn() == false); BOOST_CHECK(pnode1->IsInboundConn() == false);
BOOST_CHECK(pnode1->IsInboundOnion() == false); BOOST_CHECK(pnode1->m_inbound_onion == false);
BOOST_CHECK_EQUAL(pnode1->ConnectedThroughNetwork(), Network::NET_IPV4); BOOST_CHECK_EQUAL(pnode1->ConnectedThroughNetwork(), Network::NET_IPV4);
std::unique_ptr<CNode> pnode2 = MakeUnique<CNode>( std::unique_ptr<CNode> pnode2 = MakeUnique<CNode>(
@ -214,7 +215,7 @@ BOOST_AUTO_TEST_CASE(cnode_simple_test)
BOOST_CHECK(pnode2->IsFeelerConn() == false); BOOST_CHECK(pnode2->IsFeelerConn() == false);
BOOST_CHECK(pnode2->IsAddrFetchConn() == false); BOOST_CHECK(pnode2->IsAddrFetchConn() == false);
BOOST_CHECK(pnode2->IsInboundConn() == true); BOOST_CHECK(pnode2->IsInboundConn() == true);
BOOST_CHECK(pnode2->IsInboundOnion() == false); BOOST_CHECK(pnode2->m_inbound_onion == false);
BOOST_CHECK_EQUAL(pnode2->ConnectedThroughNetwork(), Network::NET_IPV4); BOOST_CHECK_EQUAL(pnode2->ConnectedThroughNetwork(), Network::NET_IPV4);
std::unique_ptr<CNode> pnode3 = MakeUnique<CNode>( std::unique_ptr<CNode> pnode3 = MakeUnique<CNode>(
@ -229,7 +230,7 @@ BOOST_AUTO_TEST_CASE(cnode_simple_test)
BOOST_CHECK(pnode3->IsFeelerConn() == false); BOOST_CHECK(pnode3->IsFeelerConn() == false);
BOOST_CHECK(pnode3->IsAddrFetchConn() == false); BOOST_CHECK(pnode3->IsAddrFetchConn() == false);
BOOST_CHECK(pnode3->IsInboundConn() == false); BOOST_CHECK(pnode3->IsInboundConn() == false);
BOOST_CHECK(pnode3->IsInboundOnion() == false); BOOST_CHECK(pnode3->m_inbound_onion == false);
BOOST_CHECK_EQUAL(pnode3->ConnectedThroughNetwork(), Network::NET_IPV4); BOOST_CHECK_EQUAL(pnode3->ConnectedThroughNetwork(), Network::NET_IPV4);
std::unique_ptr<CNode> pnode4 = MakeUnique<CNode>( std::unique_ptr<CNode> pnode4 = MakeUnique<CNode>(
@ -244,7 +245,7 @@ BOOST_AUTO_TEST_CASE(cnode_simple_test)
BOOST_CHECK(pnode4->IsFeelerConn() == false); BOOST_CHECK(pnode4->IsFeelerConn() == false);
BOOST_CHECK(pnode4->IsAddrFetchConn() == false); BOOST_CHECK(pnode4->IsAddrFetchConn() == false);
BOOST_CHECK(pnode4->IsInboundConn() == true); BOOST_CHECK(pnode4->IsInboundConn() == true);
BOOST_CHECK(pnode4->IsInboundOnion() == true); BOOST_CHECK(pnode4->m_inbound_onion == true);
BOOST_CHECK_EQUAL(pnode4->ConnectedThroughNetwork(), Network::NET_ONION); BOOST_CHECK_EQUAL(pnode4->ConnectedThroughNetwork(), Network::NET_ONION);
} }
@ -679,7 +680,7 @@ BOOST_AUTO_TEST_CASE(ipv4_peer_with_ipv6_addrMe_test)
in_addr ipv4AddrPeer; in_addr ipv4AddrPeer;
ipv4AddrPeer.s_addr = 0xa0b0c001; ipv4AddrPeer.s_addr = 0xa0b0c001;
CAddress addr = CAddress(CService(ipv4AddrPeer, 7777), NODE_NETWORK); CAddress addr = CAddress(CService(ipv4AddrPeer, 7777), NODE_NETWORK);
std::unique_ptr<CNode> pnode = MakeUnique<CNode>(0, NODE_NETWORK, INVALID_SOCKET, addr, 0, 0, CAddress{}, std::string{}, ConnectionType::OUTBOUND_FULL_RELAY); 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);
pnode->fSuccessfullyConnected.store(true); pnode->fSuccessfullyConnected.store(true);
// the peer claims to be reaching us via IPv6 // the peer claims to be reaching us via IPv6