Merge bitcoin/bitcoin#24627: test: Limit scope of id global which is shared between subtests

fa9086d085f664a96561eeb5dd29fc1a4e4f926a test: Limit scope of id global which is shared between subtests (MarcoFalke)

Pull request description:

  Globals aren't too nice when testing, as leak state between subtests run in the same process. For example, when checking peer ids in the tests, they might pass/fail depending on other tests run in the same process.

  Fix this by making `id` not a global.

ACKs for top commit:
  promag:
    Code review ACK fa9086d085f664a96561eeb5dd29fc1a4e4f926a.

Tree-SHA512: 0a53dde428570086f4557b23112e6460d6413bedf6ef487bd56e88f83cd5f4526f44effa8076cdeaf4761ecc062c346948e0bff434808bbf9b558eabd81328e3
This commit is contained in:
fanquake 2022-03-22 08:58:30 +00:00 committed by pasta
parent 5587bf15db
commit 9ee82d223e
No known key found for this signature in database
GPG Key ID: 52527BEDABE87984

View File

@ -44,8 +44,6 @@ static CService ip(uint32_t i)
return CService(CNetAddr(s), Params().GetDefaultPort());
}
static NodeId id = 0;
void UpdateLastBlockAnnounceTime(NodeId node, int64_t time_in_seconds);
BOOST_FIXTURE_TEST_SUITE(denialofservice_tests, TestingSetup)
@ -71,6 +69,7 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction)
// Mock an outbound peer
CAddress addr1(ip(0xa0b0c001), NODE_NONE);
NodeId id{0};
CNode dummyNode1{id++,
ServiceFlags(NODE_NETWORK),
/*sock=*/nullptr,
@ -127,7 +126,7 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction)
peerLogic->FinalizeNode(dummyNode1);
}
static void AddRandomOutboundPeer(std::vector<CNode*>& vNodes, PeerManager& peerLogic, ConnmanTestMsg& connman, ConnectionType connType)
static void AddRandomOutboundPeer(NodeId& id, std::vector<CNode*>& vNodes, PeerManager& peerLogic, ConnmanTestMsg& connman, ConnectionType connType)
{
CAddress addr(ip(g_insecure_rand_ctx.randbits(32)), NODE_NONE);
vNodes.emplace_back(new CNode{id++,
@ -151,6 +150,7 @@ static void AddRandomOutboundPeer(std::vector<CNode*>& vNodes, PeerManager& peer
BOOST_AUTO_TEST_CASE(stale_tip_peer_management)
{
NodeId id{0};
const CChainParams& chainparams = Params();
auto connman = std::make_unique<ConnmanTestMsg>(0x1337, 0x1337, *m_node.addrman);
auto peerLogic = PeerManager::make(chainparams, *connman, *m_node.addrman, nullptr, *m_node.scheduler,
@ -172,7 +172,7 @@ BOOST_AUTO_TEST_CASE(stale_tip_peer_management)
// Mock some outbound peers
for (int i = 0; i < max_outbound_full_relay; ++i) {
AddRandomOutboundPeer(vNodes, *peerLogic, *connman, ConnectionType::OUTBOUND_FULL_RELAY);
AddRandomOutboundPeer(id, vNodes, *peerLogic, *connman, ConnectionType::OUTBOUND_FULL_RELAY);
}
peerLogic->CheckForStaleTipAndEvictPeers();
@ -198,7 +198,7 @@ BOOST_AUTO_TEST_CASE(stale_tip_peer_management)
// on the next check (since we're mocking the time to be in the future, the
// required time connected check should be satisfied).
SetMockTime(time_init);
AddRandomOutboundPeer(vNodes, *peerLogic, *connman, ConnectionType::OUTBOUND_FULL_RELAY);
AddRandomOutboundPeer(id, vNodes, *peerLogic, *connman, ConnectionType::OUTBOUND_FULL_RELAY);
SetMockTime(time_later);
peerLogic->CheckForStaleTipAndEvictPeers();
@ -230,6 +230,7 @@ BOOST_AUTO_TEST_CASE(stale_tip_peer_management)
BOOST_AUTO_TEST_CASE(block_relay_only_eviction)
{
NodeId id{0};
const CChainParams& chainparams = Params();
auto connman = std::make_unique<ConnmanTestMsg>(0x1337, 0x1337, *m_node.addrman);
auto peerLogic = PeerManager::make(chainparams, *connman, *m_node.addrman, nullptr, *m_node.scheduler,
@ -249,7 +250,7 @@ BOOST_AUTO_TEST_CASE(block_relay_only_eviction)
// Add block-relay-only peers up to the limit
for (int i = 0; i < max_outbound_block_relay; ++i) {
AddRandomOutboundPeer(vNodes, *peerLogic, *connman, ConnectionType::BLOCK_RELAY);
AddRandomOutboundPeer(id, vNodes, *peerLogic, *connman, ConnectionType::BLOCK_RELAY);
}
peerLogic->CheckForStaleTipAndEvictPeers();
@ -258,7 +259,7 @@ BOOST_AUTO_TEST_CASE(block_relay_only_eviction)
}
// Add an extra block-relay-only peer breaking the limit (mocks logic in ThreadOpenConnections)
AddRandomOutboundPeer(vNodes, *peerLogic, *connman, ConnectionType::BLOCK_RELAY);
AddRandomOutboundPeer(id, vNodes, *peerLogic, *connman, ConnectionType::BLOCK_RELAY);
peerLogic->CheckForStaleTipAndEvictPeers();
// The extra peer should only get marked for eviction after MINIMUM_CONNECT_TIME
@ -316,6 +317,7 @@ BOOST_AUTO_TEST_CASE(peer_discouragement)
std::array<CNode*, 3> nodes;
banman->ClearBanned();
NodeId id{0};
nodes[0] = new CNode{id++,
NODE_NETWORK,
/*sock=*/nullptr,
@ -424,6 +426,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime)
SetMockTime(nStartTime); // Overrides future calls to GetTime()
CAddress addr(ip(0xa0b0c001), NODE_NONE);
NodeId id{0};
CNode dummyNode{id++,
NODE_NETWORK,
/*sock=*/nullptr,