Skip to content

Commit dd7669b

Browse files
laanwjcodablock
laanwj
authored and
codablock
committed
Merge bitcoin#9050: net: make a few values immutable, and use deterministic randomness for the localnonce
59ac5c5 net: Use deterministic randomness for CNode's nonce, and make it const (Cory Fields) aff6584 net: constify a few CNode vars to indicate that they're threadsafe (Cory Fields)
1 parent 6f8720b commit dd7669b

File tree

4 files changed

+31
-22
lines changed

4 files changed

+31
-22
lines changed

‎src/net.cpp‎

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ constexpr const CConnman::CFullyConnectedOnly CConnman::FullyConnectedOnly;
7171
constexpr const CConnman::CAllNodes CConnman::AllNodes;
7272

7373
static const uint64_t RANDOMIZER_ID_NETGROUP = 0x6c0edd8036ef4036ULL; // SHA256("netgroup")[0:8]
74+
static const uint64_t RANDOMIZER_ID_LOCALHOSTNONCE = 0xd93e69e2bbfa5735ULL; // SHA256("localhostnonce")[0:8]
7475
//
7576
// Global state variables
7677
//
@@ -413,7 +414,9 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
413414
addrman.Attempt(addrConnect, fCountFailure);
414415

415416
// Add node
416-
CNode* pnode = new CNode(GetNewNodeId(), nLocalServices, GetBestHeight(), hSocket, addrConnect, CalculateKeyedNetGroup(addrConnect), pszDest ? pszDest : "", false, true);
417+
NodeId id = GetNewNodeId();
418+
uint64_t nonce = GetDeterministicRandomizer(RANDOMIZER_ID_LOCALHOSTNONCE).Write(id).Finalize();
419+
CNode* pnode = new CNode(id, nLocalServices, GetBestHeight(), hSocket, addrConnect, CalculateKeyedNetGroup(addrConnect), nonce, pszDest ? pszDest : "", false, true);
417420

418421
pnode->nServicesExpected = ServiceFlags(addrConnect.nServices & nRelevantServices);
419422
pnode->nTimeConnected = GetSystemTimeInSeconds();
@@ -1079,7 +1082,10 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) {
10791082
return;
10801083
}
10811084

1082-
CNode* pnode = new CNode(GetNewNodeId(), nLocalServices, GetBestHeight(), hSocket, addr, CalculateKeyedNetGroup(addr), "", true);
1085+
NodeId id = GetNewNodeId();
1086+
uint64_t nonce = GetDeterministicRandomizer(RANDOMIZER_ID_LOCALHOSTNONCE).Write(id).Finalize();
1087+
1088+
CNode* pnode = new CNode(id, nLocalServices, GetBestHeight(), hSocket, addr, CalculateKeyedNetGroup(addr), nonce, "", true);
10831089
pnode->fWhitelisted = whitelisted;
10841090
GetNodeSignals().InitializeNode(pnode, *this);
10851091

@@ -2223,7 +2229,11 @@ bool CConnman::Start(CScheduler& scheduler, std::string& strNodeError, Options c
22232229
if (pnodeLocalHost == NULL) {
22242230
CNetAddr local;
22252231
LookupHost("127.0.0.1", local, false);
2226-
pnodeLocalHost = new CNode(GetNewNodeId(), nLocalServices, GetBestHeight(), INVALID_SOCKET, CAddress(CService(local, 0), nLocalServices), 0);
2232+
2233+
NodeId id = GetNewNodeId();
2234+
uint64_t nonce = GetDeterministicRandomizer(RANDOMIZER_ID_LOCALHOSTNONCE).Write(id).Finalize();
2235+
2236+
pnodeLocalHost = new CNode(id, nLocalServices, GetBestHeight(), INVALID_SOCKET, CAddress(CService(local, 0), nLocalServices), 0, nonce);
22272237
GetNodeSignals().InitializeNode(pnodeLocalHost, *this);
22282238
}
22292239

@@ -2622,11 +2632,16 @@ int CConnman::GetBestHeight() const
26222632
unsigned int CConnman::GetReceiveFloodSize() const { return nReceiveFloodSize; }
26232633
unsigned int CConnman::GetSendBufferSize() const{ return nSendBufferMaxSize; }
26242634

2625-
CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn, SOCKET hSocketIn, const CAddress& addrIn, uint64_t nKeyedNetGroupIn, const std::string& addrNameIn, bool fInboundIn, bool fNetworkNodeIn) :
2635+
CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn, SOCKET hSocketIn, const CAddress& addrIn, uint64_t nKeyedNetGroupIn, uint64_t nLocalHostNonceIn, const std::string& addrNameIn, bool fInboundIn, bool fNetworkNodeIn) :
26262636
addr(addrIn),
2637+
fInbound(fInboundIn),
2638+
id(idIn),
26272639
nKeyedNetGroup(nKeyedNetGroupIn),
26282640
addrKnown(5000, 0.001),
26292641
filterInventoryKnown(50000, 0.000001),
2642+
nLocalHostNonce(nLocalHostNonceIn),
2643+
nLocalServices(nLocalServicesIn),
2644+
nMyStartingHeight(nMyStartingHeightIn),
26302645
nSendVersion(0)
26312646
{
26322647
nServices = NODE_NONE;
@@ -2648,7 +2663,6 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn
26482663
fOneShot = false;
26492664
fClient = false; // set by version message
26502665
fFeeler = false;
2651-
fInbound = fInboundIn;
26522666
fNetworkNode = fNetworkNodeIn;
26532667
fSuccessfullyConnected = false;
26542668
fDisconnect = false;
@@ -2678,15 +2692,10 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn
26782692
minFeeFilter = 0;
26792693
lastSentFeeFilter = 0;
26802694
nextSendTimeFeeFilter = 0;
2681-
id = idIn;
2682-
nLocalServices = nLocalServicesIn;
26832695
fPauseRecv = false;
26842696
fPauseSend = false;
26852697
nProcessQueueSize = 0;
26862698

2687-
GetRandBytes((unsigned char*)&nLocalHostNonce, sizeof(nLocalHostNonce));
2688-
nMyStartingHeight = nMyStartingHeightIn;
2689-
26902699
BOOST_FOREACH(const std::string &msg, getAllNetMessageTypes())
26912700
mapRecvBytesPerMsgCmd[msg] = 0;
26922701
mapRecvBytesPerMsgCmd[NET_MESSAGE_COMMAND_OTHER] = 0;

‎src/net.h‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -709,7 +709,7 @@ class CNode
709709
bool fFeeler; // If true this node is being used as a short lived feeler.
710710
bool fOneShot;
711711
bool fClient;
712-
bool fInbound;
712+
const bool fInbound;
713713
bool fNetworkNode;
714714
std::atomic_bool fSuccessfullyConnected;
715715
bool fDisconnect;
@@ -726,7 +726,7 @@ class CNode
726726
CCriticalSection cs_filter;
727727
CBloomFilter* pfilter;
728728
int nRefCount;
729-
NodeId id;
729+
const NodeId id;
730730

731731
const uint64_t nKeyedNetGroup;
732732

@@ -796,7 +796,7 @@ class CNode
796796
CAmount lastSentFeeFilter;
797797
int64_t nextSendTimeFeeFilter;
798798

799-
CNode(NodeId id, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn, SOCKET hSocketIn, const CAddress &addrIn, uint64_t nKeyedNetGroupIn, const std::string &addrNameIn = "", bool fInboundIn = false, bool fNetworkNodeIn = false);
799+
CNode(NodeId id, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn, SOCKET hSocketIn, const CAddress &addrIn, uint64_t nKeyedNetGroupIn, uint64_t nLocalHostNonceIn, const std::string &addrNameIn = "", bool fInboundIn = false, bool fNetworkNodeIn = false);
800800
~CNode();
801801

802802
private:
@@ -806,10 +806,10 @@ class CNode
806806
void operator=(const CNode&);
807807

808808

809-
uint64_t nLocalHostNonce;
809+
const uint64_t nLocalHostNonce;
810810
// Services offered to this peer
811-
ServiceFlags nLocalServices;
812-
int nMyStartingHeight;
811+
const ServiceFlags nLocalServices;
812+
const int nMyStartingHeight;
813813
int nSendVersion;
814814
std::list<CNetMessage> vRecvMsg; // Used only by SocketHandler thread
815815
public:

‎src/test/DoS_tests.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ BOOST_AUTO_TEST_CASE(DoS_banning)
5050

5151
connman->ClearBanned();
5252
CAddress addr1(ip(0xa0b0c001), NODE_NONE);
53-
CNode dummyNode1(id++, NODE_NETWORK, 0, INVALID_SOCKET, addr1, 0, "", true);
53+
CNode dummyNode1(id++, NODE_NETWORK, 0, INVALID_SOCKET, addr1, 0, 0, "", true);
5454
dummyNode1.SetSendVersion(PROTOCOL_VERSION);
5555
GetNodeSignals().InitializeNode(&dummyNode1, *connman);
5656
dummyNode1.nVersion = 1;
@@ -61,7 +61,7 @@ BOOST_AUTO_TEST_CASE(DoS_banning)
6161
BOOST_CHECK(!connman->IsBanned(ip(0xa0b0c001|0x0000ff00))); // Different IP, not banned
6262

6363
CAddress addr2(ip(0xa0b0c002), NODE_NONE);
64-
CNode dummyNode2(id++, NODE_NETWORK, 0, INVALID_SOCKET, addr2, 1, "", true);
64+
CNode dummyNode2(id++, NODE_NETWORK, 0, INVALID_SOCKET, addr2, 1, 1, "", true);
6565
dummyNode2.SetSendVersion(PROTOCOL_VERSION);
6666
GetNodeSignals().InitializeNode(&dummyNode2, *connman);
6767
dummyNode2.nVersion = 1;
@@ -82,7 +82,7 @@ BOOST_AUTO_TEST_CASE(DoS_banscore)
8282
connman->ClearBanned();
8383
mapArgs["-banscore"] = "111"; // because 11 is my favorite number
8484
CAddress addr1(ip(0xa0b0c001), NODE_NONE);
85-
CNode dummyNode1(id++, NODE_NETWORK, 0, INVALID_SOCKET, addr1, 3, "", true);
85+
CNode dummyNode1(id++, NODE_NETWORK, 0, INVALID_SOCKET, addr1, 3, 1, "", true);
8686
dummyNode1.SetSendVersion(PROTOCOL_VERSION);
8787
GetNodeSignals().InitializeNode(&dummyNode1, *connman);
8888
dummyNode1.nVersion = 1;
@@ -108,7 +108,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime)
108108
SetMockTime(nStartTime); // Overrides future calls to GetTime()
109109

110110
CAddress addr(ip(0xa0b0c001), NODE_NONE);
111-
CNode dummyNode(id++, NODE_NETWORK, 0, INVALID_SOCKET, addr, 4, "", true);
111+
CNode dummyNode(id++, NODE_NETWORK, 0, INVALID_SOCKET, addr, 4, 4, "", true);
112112
dummyNode.SetSendVersion(PROTOCOL_VERSION);
113113
GetNodeSignals().InitializeNode(&dummyNode, *connman);
114114
dummyNode.nVersion = 1;

‎src/test/net_tests.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -164,12 +164,12 @@ BOOST_AUTO_TEST_CASE(cnode_simple_test)
164164
bool fInboundIn = false;
165165

166166
// Test that fFeeler is false by default.
167-
CNode* pnode1 = new CNode(id++, NODE_NETWORK, height, hSocket, addr, 0, pszDest, fInboundIn);
167+
CNode* pnode1 = new CNode(id++, NODE_NETWORK, height, hSocket, addr, 0, 0, pszDest, fInboundIn);
168168
BOOST_CHECK(pnode1->fInbound == false);
169169
BOOST_CHECK(pnode1->fFeeler == false);
170170

171171
fInboundIn = true;
172-
CNode* pnode2 = new CNode(id++, NODE_NETWORK, height, hSocket, addr, 1, pszDest, fInboundIn);
172+
CNode* pnode2 = new CNode(id++, NODE_NETWORK, height, hSocket, addr, 1, 1, pszDest, fInboundIn);
173173
BOOST_CHECK(pnode2->fInbound == true);
174174
BOOST_CHECK(pnode2->fFeeler == false);
175175
}

0 commit comments

Comments
 (0)