Merge #20649: refactor: Remove nMyStartingHeight from CNode/Connman

faaa4f2b6a refactor: Remove nMyStartingHeight from CNode/Connman (MarcoFalke)

Pull request description:

  CNode and CConnman keep track of the active chain height when CNodes have been created, but apart from serializing the int once (when sending a version message), it is unused. So it can simply be removed in favor of a single int in PeerMan that can do the same.

ACKs for top commit:
  jnewbery:
    utACK faaa4f2b6a
  theStack:
    re-ACK faaa4f2b6a 

Tree-SHA512: 713919e49e0f799c4a5a2e5ab07d1ca1c92d4049829680c4f436f9b3b545c548ddbee78ff80611de50cc91a116a416bcc4602e80ee64953597dc04ca15f9806d
This commit is contained in:
MarcoFalke 2021-01-02 13:01:51 +01:00
commit 6f97172b42
No known key found for this signature in database
GPG Key ID: D2EA4850E7528B25
12 changed files with 33 additions and 55 deletions

View File

@ -1894,6 +1894,7 @@ bool AppInitMain(const util::Ref& context, NodeContext& node, interfaces::BlockA
}
}
LogPrintf("nBestHeight = %d\n", chain_active_height);
if (node.peerman) node.peerman->SetBestHeight(chain_active_height);
Discover();
@ -1909,7 +1910,6 @@ bool AppInitMain(const util::Ref& context, NodeContext& node, interfaces::BlockA
connOptions.m_max_outbound_block_relay = std::min(MAX_BLOCK_RELAY_ONLY_CONNECTIONS, connOptions.nMaxConnections-connOptions.m_max_outbound_full_relay);
connOptions.nMaxAddnode = MAX_ADDNODE_CONNECTIONS;
connOptions.nMaxFeeler = MAX_FEELER_CONNECTIONS;
connOptions.nBestHeight = chain_active_height;
connOptions.uiInterface = &uiInterface;
connOptions.m_banman = node.banman.get();
connOptions.m_msgproc = node.peerman.get();

View File

@ -481,7 +481,7 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
NodeId id = GetNewNodeId();
uint64_t nonce = GetDeterministicRandomizer(RANDOMIZER_ID_LOCALHOSTNONCE).Write(id).Finalize();
CAddress addr_bind = GetBindAddress(hSocket);
CNode* pnode = new CNode(id, nLocalServices, GetBestHeight(), hSocket, addrConnect, CalculateKeyedNetGroup(addrConnect), nonce, addr_bind, pszDest ? pszDest : "", conn_type);
CNode* pnode = new CNode(id, nLocalServices, hSocket, addrConnect, CalculateKeyedNetGroup(addrConnect), nonce, addr_bind, pszDest ? pszDest : "", conn_type);
pnode->AddRef();
// We're making a new connection, harvest entropy from the time (and our peer count)
@ -1116,7 +1116,7 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) {
}
const bool inbound_onion = std::find(m_onion_binds.begin(), m_onion_binds.end(), addr_bind) != m_onion_binds.end();
CNode* pnode = new CNode(id, nodeServices, GetBestHeight(), hSocket, addr, CalculateKeyedNetGroup(addr), nonce, addr_bind, "", ConnectionType::INBOUND, inbound_onion);
CNode* pnode = new CNode(id, nodeServices, hSocket, addr, CalculateKeyedNetGroup(addr), nonce, addr_bind, "", ConnectionType::INBOUND, inbound_onion);
pnode->AddRef();
pnode->m_permissionFlags = permissionFlags;
pnode->m_prefer_evict = discouraged;
@ -2923,19 +2923,9 @@ ServiceFlags CConnman::GetLocalServices() const
return nLocalServices;
}
void CConnman::SetBestHeight(int height)
{
nBestHeight.store(height, std::memory_order_release);
}
int CConnman::GetBestHeight() const
{
return nBestHeight.load(std::memory_order_acquire);
}
unsigned int CConnman::GetReceiveFloodSize() const { return nReceiveFloodSize; }
CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn, 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(NodeId idIn, 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)
: nTimeConnected(GetSystemTimeInSeconds()),
addr(addrIn),
addrBind(addrBindIn),
@ -2944,7 +2934,6 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn
nLocalHostNonce(nLocalHostNonceIn),
m_conn_type(conn_type_in),
nLocalServices(nLocalServicesIn),
nMyStartingHeight(nMyStartingHeightIn),
m_inbound_onion(inbound_onion)
{
if (inbound_onion) assert(conn_type_in == ConnectionType::INBOUND);

View File

@ -200,7 +200,6 @@ public:
int m_max_outbound_block_relay = 0;
int nMaxAddnode = 0;
int nMaxFeeler = 0;
int nBestHeight = 0;
CClientUIInterface* uiInterface = nullptr;
NetEventsInterface* m_msgproc = nullptr;
BanMan* m_banman = nullptr;
@ -228,7 +227,6 @@ public:
nMaxAddnode = connOptions.nMaxAddnode;
nMaxFeeler = connOptions.nMaxFeeler;
m_max_outbound = m_max_outbound_full_relay + m_max_outbound_block_relay + nMaxFeeler;
nBestHeight = connOptions.nBestHeight;
clientInterface = connOptions.uiInterface;
m_banman = connOptions.m_banman;
m_msgproc = connOptions.m_msgproc;
@ -382,9 +380,6 @@ public:
uint64_t GetTotalBytesRecv();
uint64_t GetTotalBytesSent();
void SetBestHeight(int height);
int GetBestHeight() const;
/** Get a unique deterministic randomizer. */
CSipHasher GetDeterministicRandomizer(uint64_t id) const;
@ -559,7 +554,6 @@ private:
int nMaxFeeler;
int m_max_outbound;
bool m_use_addrman_outgoing;
std::atomic<int> nBestHeight;
CClientUIInterface* clientInterface;
NetEventsInterface* m_msgproc;
/** Pointer to this node's banman. May be nullptr - check existence before dereferencing. */
@ -1056,7 +1050,7 @@ public:
// Whether a ping is requested.
std::atomic<bool> fPingQueued{false};
CNode(NodeId id, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn, 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 = false);
~CNode();
CNode(const CNode&) = delete;
CNode& operator=(const CNode&) = delete;
@ -1084,7 +1078,6 @@ private:
//! service advertisements.
const ServiceFlags nLocalServices;
const int nMyStartingHeight;
NetPermissionFlags m_permissionFlags{ PF_NONE };
std::list<CNetMessage> vRecvMsg; // Used only by SocketHandler thread
@ -1108,10 +1101,6 @@ public:
return nLocalHostNonce;
}
int GetMyStartingHeight() const {
return nMyStartingHeight;
}
int GetRefCount() const
{
assert(nRefCount >= 0);

View File

@ -690,7 +690,7 @@ void PeerManager::PushNodeVersion(CNode& pnode, int64_t nTime)
// peer.
ServiceFlags nLocalNodeServices = pnode.GetLocalServices();
uint64_t nonce = pnode.GetLocalNonce();
int nNodeStartingHeight = pnode.GetMyStartingHeight();
const int nNodeStartingHeight{m_best_height};
NodeId nodeid = pnode.GetId();
CAddress addr = pnode.addr;
@ -1294,8 +1294,9 @@ void PeerManager::NewPoWValidBlock(const CBlockIndex *pindex, const std::shared_
* Update our best height and announce any block hashes which weren't previously
* in ::ChainActive() to our peers.
*/
void PeerManager::UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload) {
m_connman.SetBestHeight(pindexNew->nHeight);
void PeerManager::UpdatedBlockTip(const CBlockIndex* pindexNew, const CBlockIndex* pindexFork, bool fInitialDownload)
{
m_best_height = pindexNew->nHeight;
SetServiceFlagsIBDCache(!fInitialDownload);
// Don't relay inventory during initial block download.

View File

@ -160,8 +160,11 @@ public:
/** Get statistics from node state */
bool GetNodeStateStats(NodeId nodeid, CNodeStateStats& stats);
/** Set the best height */
void SetBestHeight(int height) { m_best_height = height; };
/** Whether this node ignores txs received over p2p. */
bool IgnoresIncomingTxs() {return m_ignore_incoming_txs;};
bool IgnoresIncomingTxs() { return m_ignore_incoming_txs; };
private:
/** Get a shared pointer to the Peer object.
@ -224,9 +227,12 @@ private:
CTxMemPool& m_mempool;
TxRequestTracker m_txrequest GUARDED_BY(::cs_main);
/** The height of the best chain */
std::atomic<int> m_best_height{-1};
int64_t m_stale_tip_check_time; //!< Next time to check for stale tip
//* Whether this node is running in blocks only mode */
/** Whether this node is running in blocks only mode */
const bool m_ignore_incoming_txs;
/** Whether we've completed initial sync yet, for determining when to turn

View File

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

View File

@ -32,7 +32,7 @@ FUZZ_TARGET_INIT(connman, initialize_connman)
CSubNet random_subnet;
std::string random_string;
while (fuzzed_data_provider.ConsumeBool()) {
switch (fuzzed_data_provider.ConsumeIntegralInRange<int>(0, 28)) {
switch (fuzzed_data_provider.ConsumeIntegralInRange<int>(0, 27)) {
case 0:
random_address = ConsumeAddress(fuzzed_data_provider);
break;
@ -125,21 +125,17 @@ FUZZ_TARGET_INIT(connman, initialize_connman)
break;
}
case 25:
connman.SetBestHeight(fuzzed_data_provider.ConsumeIntegral<int>());
break;
case 26:
connman.SetNetworkActive(fuzzed_data_provider.ConsumeBool());
break;
case 27:
case 26:
connman.SetServices(random_service, static_cast<ServiceFlags>(fuzzed_data_provider.ConsumeIntegral<uint64_t>()));
break;
case 28:
case 27:
connman.SetTryNewOutboundPeer(fuzzed_data_provider.ConsumeBool());
break;
}
}
(void)connman.GetAddedNodeInfo();
(void)connman.GetBestHeight();
(void)connman.GetExtraFullOutboundCount();
(void)connman.GetLocalServices();
(void)connman.GetMaxOutboundTarget();

View File

@ -117,7 +117,6 @@ FUZZ_TARGET_INIT(net, initialize_net)
(void)node.GetId();
(void)node.GetLocalNonce();
(void)node.GetLocalServices();
(void)node.GetMyStartingHeight();
const int ref_count = node.GetRefCount();
assert(ref_count >= 0);
(void)node.GetCommonVersion();

View File

@ -64,7 +64,7 @@ void fuzz_target(const std::vector<uint8_t>& buffer, const std::string& LIMIT_TO
const bool jump_out_of_ibd{fuzzed_data_provider.ConsumeBool()};
if (jump_out_of_ibd) chainstate.JumpOutOfIbd();
CDataStream random_bytes_data_stream{fuzzed_data_provider.ConsumeRemainingBytes<unsigned char>(), SER_NETWORK, PROTOCOL_VERSION};
CNode& p2p_node = *MakeUnique<CNode>(0, ServiceFlags(NODE_NETWORK | NODE_WITNESS | NODE_BLOOM), 0, INVALID_SOCKET, CAddress{CService{in_addr{0x0100007f}, 7777}, NODE_NETWORK}, 0, 0, CAddress{}, std::string{}, ConnectionType::OUTBOUND_FULL_RELAY).release();
CNode& p2p_node = *MakeUnique<CNode>(0, ServiceFlags(NODE_NETWORK | NODE_WITNESS | NODE_BLOOM), INVALID_SOCKET, CAddress{CService{in_addr{0x0100007f}, 7777}, NODE_NETWORK}, 0, 0, CAddress{}, std::string{}, ConnectionType::OUTBOUND_FULL_RELAY).release();
p2p_node.fSuccessfullyConnected = true;
p2p_node.nVersion = PROTOCOL_VERSION;
p2p_node.SetCommonVersion(PROTOCOL_VERSION);

View File

@ -49,7 +49,7 @@ FUZZ_TARGET_INIT(process_messages, initialize_process_messages)
for (int i = 0; i < num_peers_to_add; ++i) {
const ServiceFlags service_flags = ServiceFlags(fuzzed_data_provider.ConsumeIntegral<uint64_t>());
const ConnectionType conn_type = fuzzed_data_provider.PickValueInArray({ConnectionType::INBOUND, ConnectionType::OUTBOUND_FULL_RELAY, ConnectionType::MANUAL, ConnectionType::FEELER, ConnectionType::BLOCK_RELAY, ConnectionType::ADDR_FETCH});
peers.push_back(MakeUnique<CNode>(i, service_flags, 0, INVALID_SOCKET, CAddress{CService{in_addr{0x0100007f}, 7777}, NODE_NETWORK}, 0, 0, CAddress{}, std::string{}, conn_type).release());
peers.push_back(MakeUnique<CNode>(i, service_flags, INVALID_SOCKET, CAddress{CService{in_addr{0x0100007f}, 7777}, NODE_NETWORK}, 0, 0, CAddress{}, std::string{}, conn_type).release());
CNode& p2p_node = *peers.back();
p2p_node.fSuccessfullyConnected = true;

View File

@ -290,7 +290,6 @@ inline CNode ConsumeNode(FuzzedDataProvider& fuzzed_data_provider) noexcept
{
const NodeId node_id = fuzzed_data_provider.ConsumeIntegral<NodeId>();
const ServiceFlags local_services = static_cast<ServiceFlags>(fuzzed_data_provider.ConsumeIntegral<uint64_t>());
const int my_starting_height = fuzzed_data_provider.ConsumeIntegral<int>();
const SOCKET socket = INVALID_SOCKET;
const CAddress address = ConsumeAddress(fuzzed_data_provider);
const uint64_t keyed_net_group = fuzzed_data_provider.ConsumeIntegral<uint64_t>();
@ -299,7 +298,7 @@ inline CNode ConsumeNode(FuzzedDataProvider& fuzzed_data_provider) noexcept
const std::string addr_name = fuzzed_data_provider.ConsumeRandomLengthString(64);
const ConnectionType conn_type = fuzzed_data_provider.PickValueInArray({ConnectionType::INBOUND, ConnectionType::OUTBOUND_FULL_RELAY, ConnectionType::MANUAL, ConnectionType::FEELER, ConnectionType::BLOCK_RELAY, ConnectionType::ADDR_FETCH});
const bool inbound_onion{conn_type == ConnectionType::INBOUND ? fuzzed_data_provider.ConsumeBool() : false};
return {node_id, local_services, my_starting_height, socket, address, keyed_net_group, local_host_nonce, addr_bind, addr_name, conn_type, inbound_onion};
return {node_id, local_services, socket, address, keyed_net_group, local_host_nonce, addr_bind, addr_name, conn_type, inbound_onion};
}
inline void InitializeFuzzingContext(const std::string& chain_name = CBaseChainParams::REGTEST)

View File

@ -181,7 +181,6 @@ BOOST_AUTO_TEST_CASE(cnode_simple_test)
{
SOCKET hSocket = INVALID_SOCKET;
NodeId id = 0;
int height = 0;
in_addr ipv4Addr;
ipv4Addr.s_addr = 0xa0b0c001;
@ -190,7 +189,7 @@ BOOST_AUTO_TEST_CASE(cnode_simple_test)
std::string pszDest;
std::unique_ptr<CNode> pnode1 = MakeUnique<CNode>(
id++, NODE_NETWORK, height, hSocket, addr,
id++, NODE_NETWORK, hSocket, addr,
/* nKeyedNetGroupIn = */ 0,
/* nLocalHostNonceIn = */ 0,
CAddress(), pszDest, ConnectionType::OUTBOUND_FULL_RELAY);
@ -204,7 +203,7 @@ BOOST_AUTO_TEST_CASE(cnode_simple_test)
BOOST_CHECK_EQUAL(pnode1->ConnectedThroughNetwork(), Network::NET_IPV4);
std::unique_ptr<CNode> pnode2 = MakeUnique<CNode>(
id++, NODE_NETWORK, height, hSocket, addr,
id++, NODE_NETWORK, hSocket, addr,
/* nKeyedNetGroupIn = */ 1,
/* nLocalHostNonceIn = */ 1,
CAddress(), pszDest, ConnectionType::INBOUND,
@ -219,7 +218,7 @@ BOOST_AUTO_TEST_CASE(cnode_simple_test)
BOOST_CHECK_EQUAL(pnode2->ConnectedThroughNetwork(), Network::NET_IPV4);
std::unique_ptr<CNode> pnode3 = MakeUnique<CNode>(
id++, NODE_NETWORK, height, hSocket, addr,
id++, NODE_NETWORK, hSocket, addr,
/* nKeyedNetGroupIn = */ 0,
/* nLocalHostNonceIn = */ 0,
CAddress(), pszDest, ConnectionType::OUTBOUND_FULL_RELAY,
@ -234,7 +233,7 @@ BOOST_AUTO_TEST_CASE(cnode_simple_test)
BOOST_CHECK_EQUAL(pnode3->ConnectedThroughNetwork(), Network::NET_IPV4);
std::unique_ptr<CNode> pnode4 = MakeUnique<CNode>(
id++, NODE_NETWORK, height, hSocket, addr,
id++, NODE_NETWORK, hSocket, addr,
/* nKeyedNetGroupIn = */ 1,
/* nLocalHostNonceIn = */ 1,
CAddress(), pszDest, ConnectionType::INBOUND,
@ -680,7 +679,7 @@ BOOST_AUTO_TEST_CASE(ipv4_peer_with_ipv6_addrMe_test)
in_addr ipv4AddrPeer;
ipv4AddrPeer.s_addr = 0xa0b0c001;
CAddress addr = CAddress(CService(ipv4AddrPeer, 7777), NODE_NETWORK);
std::unique_ptr<CNode> pnode = MakeUnique<CNode>(0, NODE_NETWORK, 0, 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, 0, 0, CAddress{}, std::string{}, ConnectionType::OUTBOUND_FULL_RELAY);
pnode->fSuccessfullyConnected.store(true);
// the peer claims to be reaching us via IPv6