From 97abde6e20bd4310b475879dae06c0a8e9f3ee63 Mon Sep 17 00:00:00 2001 From: PastaClaw Date: Sun, 12 Jul 2026 08:44:46 -0500 Subject: [PATCH 1/4] test: reuse the peer helper in per_object_vote_sync_is_fulfilled_request_limited The test built its CNode inline, duplicating the setup MakeGovernanceInvPeer already performs for every other test in this file. Reuse the helper. The helper derives the peer address from the node id, so the peer's address changes from 0x01020305 to 0x01020306. Nothing depends on the literal value: the netfulfilledman request keys are derived from peer->addr in the test itself, so they stay consistent. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/test/governance_inv_tests.cpp | 119 +++++++++++++----------------- 1 file changed, 53 insertions(+), 66 deletions(-) diff --git a/src/test/governance_inv_tests.cpp b/src/test/governance_inv_tests.cpp index fd22cb296e72..0630f05cfc39 100644 --- a/src/test/governance_inv_tests.cpp +++ b/src/test/governance_inv_tests.cpp @@ -186,21 +186,8 @@ BOOST_AUTO_TEST_CASE(per_object_vote_sync_is_fulfilled_request_limited) m_node.mn_sync->SwitchToNextAsset(); BOOST_REQUIRE(m_node.mn_sync->IsSynced()); - in_addr peer_in_addr{}; - peer_in_addr.s_addr = htonl(0x01020305); - CNode peer{/*id=*/1, - /*sock=*/nullptr, - /*addrIn=*/CAddress{CService{peer_in_addr, 8333}, NODE_NETWORK}, - /*nKeyedNetGroupIn=*/0, - /*nLocalHostNonceIn=*/0, - /*addrBindIn=*/CAddress{}, - /*addrNameIn=*/std::string{}, - /*conn_type_in=*/ConnectionType::INBOUND, - /*inbound_onion=*/false}; - peer.nVersion = PROTOCOL_VERSION; - peer.SetCommonVersion(PROTOCOL_VERSION); - m_node.peerman->InitializeNode(peer, NODE_NETWORK); - peer.fSuccessfullyConnected = true; + auto peer{MakeGovernanceInvPeer(/*id=*/1)}; + m_node.peerman->InitializeNode(*peer, NODE_NETWORK); auto make_request_stream = [](const uint256& object_hash, const CBloomFilter& filter) { CDataStream stream{SER_NETWORK, PROTOCOL_VERSION}; @@ -212,7 +199,7 @@ BOOST_AUTO_TEST_CASE(per_object_vote_sync_is_fulfilled_request_limited) *m_node.netfulfilledman, *m_node.connman); auto& connman = static_cast(*m_node.connman); CNodeStateStats stats; - BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(peer.GetId(), stats)); + BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(peer->GetId(), stats)); BOOST_CHECK_EQUAL(stats.m_misbehavior_score, 0); const CBloomFilter vote_filter{1, 0.001, 0, BLOOM_UPDATE_NONE}; @@ -223,16 +210,16 @@ BOOST_AUTO_TEST_CASE(per_object_vote_sync_is_fulfilled_request_limited) const std::string postponed_vote_sync_request{strprintf("%s-votes-%s", NetMsgType::MNGOVERNANCESYNC, postponed_object_hash.ToString())}; auto postponed_stream = make_request_stream(postponed_object_hash, vote_filter); - net_gov.ProcessMessage(peer, NetMsgType::MNGOVERNANCESYNC, postponed_stream); + net_gov.ProcessMessage(*peer, NetMsgType::MNGOVERNANCESYNC, postponed_stream); - BOOST_CHECK(!m_node.netfulfilledman->HasFulfilledRequest(peer.addr, postponed_vote_sync_request)); - BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(peer.GetId(), stats)); + BOOST_CHECK(!m_node.netfulfilledman->HasFulfilledRequest(peer->addr, postponed_vote_sync_request)); + BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(peer->GetId(), stats)); BOOST_CHECK_EQUAL(stats.m_misbehavior_score, 0); auto duplicate_postponed_stream = make_request_stream(postponed_object_hash, vote_filter); - net_gov.ProcessMessage(peer, NetMsgType::MNGOVERNANCESYNC, duplicate_postponed_stream); + net_gov.ProcessMessage(*peer, NetMsgType::MNGOVERNANCESYNC, duplicate_postponed_stream); - BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(peer.GetId(), stats)); + BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(peer->GetId(), stats)); BOOST_CHECK_EQUAL(stats.m_misbehavior_score, 0); CGovernanceObject syncable_object{uint256(), /*revision=*/1, GetTime() + 1, uint256S("02"), /*data=*/{}}; @@ -241,103 +228,103 @@ BOOST_AUTO_TEST_CASE(per_object_vote_sync_is_fulfilled_request_limited) const uint256 syncable_object_hash{syncable_object.GetHash()}; const std::string syncable_vote_sync_request{strprintf("%s-votes-%s", NetMsgType::MNGOVERNANCESYNC, syncable_object_hash.ToString())}; - BOOST_CHECK(!m_node.netfulfilledman->HasFulfilledRequest(peer.addr, syncable_vote_sync_request)); + BOOST_CHECK(!m_node.netfulfilledman->HasFulfilledRequest(peer->addr, syncable_vote_sync_request)); - connman.FlushSendBuffer(peer); + connman.FlushSendBuffer(*peer); auto syncable_object_fetch_stream = make_request_stream(syncable_object_hash, CBloomFilter{}); - net_gov.ProcessMessage(peer, NetMsgType::MNGOVERNANCESYNC, syncable_object_fetch_stream); + net_gov.ProcessMessage(*peer, NetMsgType::MNGOVERNANCESYNC, syncable_object_fetch_stream); - BOOST_CHECK_EQUAL(CountQueuedInventory(peer, CInv{MSG_GOVERNANCE_OBJECT, syncable_object_hash}), 1U); - BOOST_CHECK(!m_node.netfulfilledman->HasFulfilledRequest(peer.addr, syncable_vote_sync_request)); - BOOST_CHECK_EQUAL(CountQueuedMessages(peer, NetMsgType::SYNCSTATUSCOUNT), 0U); - BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(peer.GetId(), stats)); + BOOST_CHECK_EQUAL(CountQueuedInventory(*peer, CInv{MSG_GOVERNANCE_OBJECT, syncable_object_hash}), 1U); + BOOST_CHECK(!m_node.netfulfilledman->HasFulfilledRequest(peer->addr, syncable_vote_sync_request)); + BOOST_CHECK_EQUAL(CountQueuedMessages(*peer, NetMsgType::SYNCSTATUSCOUNT), 0U); + BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(peer->GetId(), stats)); BOOST_CHECK_EQUAL(stats.m_misbehavior_score, 0); - connman.FlushSendBuffer(peer); + connman.FlushSendBuffer(*peer); auto duplicate_syncable_object_fetch_stream = make_request_stream(syncable_object_hash, CBloomFilter{}); - net_gov.ProcessMessage(peer, NetMsgType::MNGOVERNANCESYNC, duplicate_syncable_object_fetch_stream); + net_gov.ProcessMessage(*peer, NetMsgType::MNGOVERNANCESYNC, duplicate_syncable_object_fetch_stream); - BOOST_CHECK_EQUAL(CountQueuedInventory(peer, CInv{MSG_GOVERNANCE_OBJECT, syncable_object_hash}), 1U); - BOOST_CHECK(!m_node.netfulfilledman->HasFulfilledRequest(peer.addr, syncable_vote_sync_request)); - BOOST_CHECK_EQUAL(CountQueuedMessages(peer, NetMsgType::SYNCSTATUSCOUNT), 0U); - BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(peer.GetId(), stats)); + BOOST_CHECK_EQUAL(CountQueuedInventory(*peer, CInv{MSG_GOVERNANCE_OBJECT, syncable_object_hash}), 1U); + BOOST_CHECK(!m_node.netfulfilledman->HasFulfilledRequest(peer->addr, syncable_vote_sync_request)); + BOOST_CHECK_EQUAL(CountQueuedMessages(*peer, NetMsgType::SYNCSTATUSCOUNT), 0U); + BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(peer->GetId(), stats)); BOOST_CHECK_EQUAL(stats.m_misbehavior_score, 0); - connman.FlushSendBuffer(peer); + connman.FlushSendBuffer(*peer); auto syncable_stream = make_request_stream(syncable_object_hash, vote_filter); - net_gov.ProcessMessage(peer, NetMsgType::MNGOVERNANCESYNC, syncable_stream); + net_gov.ProcessMessage(*peer, NetMsgType::MNGOVERNANCESYNC, syncable_stream); - BOOST_CHECK(m_node.netfulfilledman->HasFulfilledRequest(peer.addr, syncable_vote_sync_request)); - BOOST_CHECK_EQUAL(CountQueuedMessages(peer, NetMsgType::SYNCSTATUSCOUNT), 1U); - BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(peer.GetId(), stats)); + BOOST_CHECK(m_node.netfulfilledman->HasFulfilledRequest(peer->addr, syncable_vote_sync_request)); + BOOST_CHECK_EQUAL(CountQueuedMessages(*peer, NetMsgType::SYNCSTATUSCOUNT), 1U); + BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(peer->GetId(), stats)); BOOST_CHECK_EQUAL(stats.m_misbehavior_score, 0); auto duplicate_syncable_stream = make_request_stream(syncable_object_hash, vote_filter); - net_gov.ProcessMessage(peer, NetMsgType::MNGOVERNANCESYNC, duplicate_syncable_stream); + net_gov.ProcessMessage(*peer, NetMsgType::MNGOVERNANCESYNC, duplicate_syncable_stream); - BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(peer.GetId(), stats)); + BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(peer->GetId(), stats)); BOOST_CHECK_EQUAL(stats.m_misbehavior_score, 20); - connman.FlushSendBuffer(peer); + connman.FlushSendBuffer(*peer); CGovernanceObject empty_filter_object{uint256(), /*revision=*/1, GetTime() + 2, uint256S("03"), /*data=*/{}}; m_node.govman->AddPostponedObject(empty_filter_object); const uint256 empty_filter_object_hash{empty_filter_object.GetHash()}; const std::string empty_filter_vote_request{strprintf("%s-votes-%s", NetMsgType::MNGOVERNANCESYNC, empty_filter_object_hash.ToString())}; auto empty_filter_stream = make_request_stream(empty_filter_object_hash, CBloomFilter{}); - net_gov.ProcessMessage(peer, NetMsgType::MNGOVERNANCESYNC, empty_filter_stream); + net_gov.ProcessMessage(*peer, NetMsgType::MNGOVERNANCESYNC, empty_filter_stream); - BOOST_CHECK_EQUAL(CountQueuedInventory(peer, CInv{MSG_GOVERNANCE_OBJECT, empty_filter_object_hash}), 1U); - BOOST_CHECK(!m_node.netfulfilledman->HasFulfilledRequest(peer.addr, empty_filter_vote_request)); - BOOST_CHECK_EQUAL(CountQueuedMessages(peer, NetMsgType::SYNCSTATUSCOUNT), 0U); - BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(peer.GetId(), stats)); + BOOST_CHECK_EQUAL(CountQueuedInventory(*peer, CInv{MSG_GOVERNANCE_OBJECT, empty_filter_object_hash}), 1U); + BOOST_CHECK(!m_node.netfulfilledman->HasFulfilledRequest(peer->addr, empty_filter_vote_request)); + BOOST_CHECK_EQUAL(CountQueuedMessages(*peer, NetMsgType::SYNCSTATUSCOUNT), 0U); + BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(peer->GetId(), stats)); BOOST_CHECK_EQUAL(stats.m_misbehavior_score, 20); - connman.FlushSendBuffer(peer); + connman.FlushSendBuffer(*peer); auto duplicate_empty_filter_stream = make_request_stream(empty_filter_object_hash, CBloomFilter{}); - net_gov.ProcessMessage(peer, NetMsgType::MNGOVERNANCESYNC, duplicate_empty_filter_stream); + net_gov.ProcessMessage(*peer, NetMsgType::MNGOVERNANCESYNC, duplicate_empty_filter_stream); - BOOST_CHECK_EQUAL(CountQueuedInventory(peer, CInv{MSG_GOVERNANCE_OBJECT, empty_filter_object_hash}), 1U); - BOOST_CHECK(!m_node.netfulfilledman->HasFulfilledRequest(peer.addr, empty_filter_vote_request)); - BOOST_CHECK_EQUAL(CountQueuedMessages(peer, NetMsgType::SYNCSTATUSCOUNT), 0U); - BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(peer.GetId(), stats)); + BOOST_CHECK_EQUAL(CountQueuedInventory(*peer, CInv{MSG_GOVERNANCE_OBJECT, empty_filter_object_hash}), 1U); + BOOST_CHECK(!m_node.netfulfilledman->HasFulfilledRequest(peer->addr, empty_filter_vote_request)); + BOOST_CHECK_EQUAL(CountQueuedMessages(*peer, NetMsgType::SYNCSTATUSCOUNT), 0U); + BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(peer->GetId(), stats)); BOOST_CHECK_EQUAL(stats.m_misbehavior_score, 20); const uint256 unknown_vote_hash{uint256S("09")}; const std::string unknown_vote_request{strprintf("%s-votes-%s", NetMsgType::MNGOVERNANCESYNC, unknown_vote_hash.ToString())}; auto unknown_vote_stream = make_request_stream(unknown_vote_hash, vote_filter); - net_gov.ProcessMessage(peer, NetMsgType::MNGOVERNANCESYNC, unknown_vote_stream); + net_gov.ProcessMessage(*peer, NetMsgType::MNGOVERNANCESYNC, unknown_vote_stream); - BOOST_CHECK(!m_node.netfulfilledman->HasFulfilledRequest(peer.addr, unknown_vote_request)); - BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(peer.GetId(), stats)); + BOOST_CHECK(!m_node.netfulfilledman->HasFulfilledRequest(peer->addr, unknown_vote_request)); + BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(peer->GetId(), stats)); BOOST_CHECK_EQUAL(stats.m_misbehavior_score, 20); auto duplicate_unknown_vote_stream = make_request_stream(unknown_vote_hash, vote_filter); - net_gov.ProcessMessage(peer, NetMsgType::MNGOVERNANCESYNC, duplicate_unknown_vote_stream); + net_gov.ProcessMessage(*peer, NetMsgType::MNGOVERNANCESYNC, duplicate_unknown_vote_stream); - BOOST_CHECK(!m_node.netfulfilledman->HasFulfilledRequest(peer.addr, unknown_vote_request)); - BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(peer.GetId(), stats)); + BOOST_CHECK(!m_node.netfulfilledman->HasFulfilledRequest(peer->addr, unknown_vote_request)); + BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(peer->GetId(), stats)); BOOST_CHECK_EQUAL(stats.m_misbehavior_score, 20); const uint256 object_fetch_hash{uint256S("0a")}; const std::string object_fetch_request{strprintf("%s-votes-%s", NetMsgType::MNGOVERNANCESYNC, object_fetch_hash.ToString())}; auto object_fetch_stream = make_request_stream(object_fetch_hash, CBloomFilter{}); - net_gov.ProcessMessage(peer, NetMsgType::MNGOVERNANCESYNC, object_fetch_stream); + net_gov.ProcessMessage(*peer, NetMsgType::MNGOVERNANCESYNC, object_fetch_stream); - BOOST_CHECK(!m_node.netfulfilledman->HasFulfilledRequest(peer.addr, object_fetch_request)); - BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(peer.GetId(), stats)); + BOOST_CHECK(!m_node.netfulfilledman->HasFulfilledRequest(peer->addr, object_fetch_request)); + BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(peer->GetId(), stats)); BOOST_CHECK_EQUAL(stats.m_misbehavior_score, 20); auto duplicate_object_fetch_stream = make_request_stream(object_fetch_hash, CBloomFilter{}); - net_gov.ProcessMessage(peer, NetMsgType::MNGOVERNANCESYNC, duplicate_object_fetch_stream); + net_gov.ProcessMessage(*peer, NetMsgType::MNGOVERNANCESYNC, duplicate_object_fetch_stream); - BOOST_CHECK(!m_node.netfulfilledman->HasFulfilledRequest(peer.addr, object_fetch_request)); - BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(peer.GetId(), stats)); + BOOST_CHECK(!m_node.netfulfilledman->HasFulfilledRequest(peer->addr, object_fetch_request)); + BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(peer->GetId(), stats)); BOOST_CHECK_EQUAL(stats.m_misbehavior_score, 20); - m_node.peerman->FinalizeNode(peer); + m_node.peerman->FinalizeNode(*peer); } BOOST_AUTO_TEST_CASE(governance_objects_require_peer_announcement_or_request) From 83d8dd7851cc08d24d5efd8c49dd8292c068c255 Mon Sep 17 00:00:00 2001 From: PastaClaw Date: Sun, 12 Jul 2026 08:49:54 -0500 Subject: [PATCH 2/4] test: drop the governance inv fixture setup left over from CheckAndRemove The CheckAndRemove tests were removed from this file, leaving their setup behind: - and were only used by the deleted net_governance_schedule_drives_check_and_remove test. - The mn_metaman validity check and LoadCache(false) were only needed because CheckAndRemove asserts metaman.IsValid(). No remaining test reaches a path that touches the meta manager: the vote tests have no parent object, so CGovernanceManager::ProcessVote returns on the orphan branch, and the object test pre-seeds the object, so ProcessObject returns on the "already seen" branch before CheckOrphanVotes. Drop the setup and with it. - The fixture comments still described CheckAndRemove's preconditions. - The mock-time comment claimed the anchor made "SetMockTime advances" deterministic, but nothing advances the clock anymore. It is anchored so the object and vote timestamps built from GetTime() are deterministic. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/test/governance_inv_tests.cpp | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/test/governance_inv_tests.cpp b/src/test/governance_inv_tests.cpp index 0630f05cfc39..6a216f3f44ce 100644 --- a/src/test/governance_inv_tests.cpp +++ b/src/test/governance_inv_tests.cpp @@ -7,14 +7,12 @@ #include #include #include -#include #include #include #include #include #include #include -#include #include #include #include @@ -30,7 +28,6 @@ #include #include #include -#include #include using namespace std::chrono_literals; @@ -39,16 +36,16 @@ namespace { struct GovernanceInvSetup : public TestingSetup { GovernanceInvSetup() : TestingSetup{CBaseChainParams::MAIN} { - // ConfirmInventoryRequest and CheckAndRemove short-circuit on - // !IsBlockchainSynced(); CheckAndRemove also asserts metaman.IsValid(). - // NetGovernance::AlreadyHave gates on m_gov_manager.IsValid(). + // ConfirmInventoryRequest and the NetGovernance object/vote handlers + // short-circuit on !IsBlockchainSynced(). BOOST_REQUIRE(m_node.mn_sync); m_node.mn_sync->SwitchToNextAsset(); BOOST_REQUIRE(m_node.mn_sync->IsBlockchainSynced()); BOOST_REQUIRE(m_node.mn_metaman); - BOOST_REQUIRE(m_node.mn_metaman->LoadCache(/*load_cache=*/false)); - + // Note: mn_metaman is left unloaded. No test here reaches + // CGovernanceObject::ProcessVote, which asserts metaman.IsValid() -- a vote whose + // parent object exists would, and would need it loaded first. m_node.govman = std::make_unique(*m_node.mn_metaman, *m_node.chainman, *m_node.chain_helper->superblocks, *m_node.dmnman, *m_node.mn_sync); // Match runtime preconditions: NetGovernance::AlreadyHave claims we // already have the inv when governance isn't loaded (e.g. @@ -73,7 +70,8 @@ struct GovernanceInvSetup : public TestingSetup { m_node.peerman.get(), *m_node.govman, *m_node.mn_sync, *m_node.netfulfilledman, *m_node.connman)); - // Anchor the mocked clock so SetMockTime advances are deterministic. + // Anchor the clock so the object and vote timestamps the tests build + // from GetTime() are deterministic. Nothing advances it. SetMockTime(1'700'000'000s); } ~GovernanceInvSetup() { From a8f2711f0dd445f41f666b584c92c2575a667a3f Mon Sep 17 00:00:00 2001 From: PastaClaw Date: Sun, 12 Jul 2026 08:51:52 -0500 Subject: [PATCH 3/4] refactor(governance): move RELIABLE_PROPAGATION_TIME next to its only uses The comment above the constant still described it as the lifetime of an entry in the governance inv request cache, but that cache is gone. The constant itself is still live: it is a margin on the MAX_TIME_FUTURE_DEVIATION limit for object creation times, used to decide when an object needs an additional relay (governance.cpp) and when that relay is due (CheckPostponedObjects). Both uses are in governance.cpp and both pair it with MAX_TIME_FUTURE_DEVIATION, which already lives in that file's anonymous namespace. Nothing outside governance.cpp referenced it, so move it there beside the constant it is always used with and give it a comment that matches what it does. That also drops the last use of in governance.h. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/governance/governance.cpp | 3 ++- src/governance/governance.h | 3 --- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index ede7ac1dfaef..2261d0e24367 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -32,7 +32,8 @@ namespace { constexpr std::chrono::seconds GOVERNANCE_DELETION_DELAY{10min}; constexpr std::chrono::seconds GOVERNANCE_ORPHAN_EXPIRATION_TIME{10min}; constexpr std::chrono::seconds MAX_TIME_FUTURE_DEVIATION{1h}; -using governance::RELIABLE_PROPAGATION_TIME; +// Margin below MAX_TIME_FUTURE_DEVIATION so lagging-clock peers don't reject near-limit objects. +constexpr std::chrono::seconds RELIABLE_PROPAGATION_TIME{60}; bool IsSyncableObject(const std::shared_ptr& govobj) { diff --git a/src/governance/governance.h b/src/governance/governance.h index 87833b8fee80..9907fd61ba3e 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -10,7 +10,6 @@ #include #include -#include #include #include #include @@ -39,8 +38,6 @@ class UniValue; namespace governance { class SuperblockManager; -// How long a requested governance inv hash remains in the request cache. -inline constexpr std::chrono::seconds RELIABLE_PROPAGATION_TIME{60}; } // namespace governance using vote_time_pair_t = std::pair; From 26732408f0d3b9f7b8dd0d0a1eb836b2174f3c6a Mon Sep 17 00:00:00 2001 From: PastaClaw Date: Sun, 12 Jul 2026 11:51:24 -0500 Subject: [PATCH 4/4] test: extract AssertMisbehaviorScore helper in governance inv tests The GetNodeStateStats + m_misbehavior_score assertion pair repeated ~20 times across the governance inventory tests. Extract a small AssertMisbehaviorScore(peerman, peer, expected) helper that preserves the existing BOOST_REQUIRE/BOOST_CHECK semantics and expected scores, and drop the now-unused local CNodeStateStats declarations. Co-Authored-By: Claude Opus 4.8 --- src/test/governance_inv_tests.cpp | 70 ++++++++++++------------------- 1 file changed, 27 insertions(+), 43 deletions(-) diff --git a/src/test/governance_inv_tests.cpp b/src/test/governance_inv_tests.cpp index 6a216f3f44ce..93e63e9ac33f 100644 --- a/src/test/governance_inv_tests.cpp +++ b/src/test/governance_inv_tests.cpp @@ -171,6 +171,13 @@ void ProcessGovernanceVote(NetGovernance& net_gov, CNode& peer, const CGovernanc vote_stream << vote; net_gov.ProcessMessage(peer, NetMsgType::MNGOVERNANCEOBJECTVOTE, vote_stream); } + +void AssertMisbehaviorScore(PeerManager& peerman, const CNode& peer, int expected) +{ + CNodeStateStats stats; + BOOST_REQUIRE(peerman.GetNodeStateStats(peer.GetId(), stats)); + BOOST_CHECK_EQUAL(stats.m_misbehavior_score, expected); +} } // namespace BOOST_FIXTURE_TEST_SUITE(governance_inv_tests, GovernanceInvSetup) @@ -196,9 +203,7 @@ BOOST_AUTO_TEST_CASE(per_object_vote_sync_is_fulfilled_request_limited) NetGovernance net_gov(m_node.peerman.get(), *m_node.govman, *m_node.mn_sync, *m_node.netfulfilledman, *m_node.connman); auto& connman = static_cast(*m_node.connman); - CNodeStateStats stats; - BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(peer->GetId(), stats)); - BOOST_CHECK_EQUAL(stats.m_misbehavior_score, 0); + AssertMisbehaviorScore(*m_node.peerman, *peer, 0); const CBloomFilter vote_filter{1, 0.001, 0, BLOOM_UPDATE_NONE}; CGovernanceObject postponed_object{uint256(), /*revision=*/1, GetTime(), uint256::ONE, /*data=*/{}}; @@ -211,14 +216,12 @@ BOOST_AUTO_TEST_CASE(per_object_vote_sync_is_fulfilled_request_limited) net_gov.ProcessMessage(*peer, NetMsgType::MNGOVERNANCESYNC, postponed_stream); BOOST_CHECK(!m_node.netfulfilledman->HasFulfilledRequest(peer->addr, postponed_vote_sync_request)); - BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(peer->GetId(), stats)); - BOOST_CHECK_EQUAL(stats.m_misbehavior_score, 0); + AssertMisbehaviorScore(*m_node.peerman, *peer, 0); auto duplicate_postponed_stream = make_request_stream(postponed_object_hash, vote_filter); net_gov.ProcessMessage(*peer, NetMsgType::MNGOVERNANCESYNC, duplicate_postponed_stream); - BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(peer->GetId(), stats)); - BOOST_CHECK_EQUAL(stats.m_misbehavior_score, 0); + AssertMisbehaviorScore(*m_node.peerman, *peer, 0); CGovernanceObject syncable_object{uint256(), /*revision=*/1, GetTime() + 1, uint256S("02"), /*data=*/{}}; m_node.govman->AddGovernanceObjectForTesting(syncable_object); @@ -235,8 +238,7 @@ BOOST_AUTO_TEST_CASE(per_object_vote_sync_is_fulfilled_request_limited) BOOST_CHECK_EQUAL(CountQueuedInventory(*peer, CInv{MSG_GOVERNANCE_OBJECT, syncable_object_hash}), 1U); BOOST_CHECK(!m_node.netfulfilledman->HasFulfilledRequest(peer->addr, syncable_vote_sync_request)); BOOST_CHECK_EQUAL(CountQueuedMessages(*peer, NetMsgType::SYNCSTATUSCOUNT), 0U); - BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(peer->GetId(), stats)); - BOOST_CHECK_EQUAL(stats.m_misbehavior_score, 0); + AssertMisbehaviorScore(*m_node.peerman, *peer, 0); connman.FlushSendBuffer(*peer); auto duplicate_syncable_object_fetch_stream = make_request_stream(syncable_object_hash, CBloomFilter{}); @@ -245,8 +247,7 @@ BOOST_AUTO_TEST_CASE(per_object_vote_sync_is_fulfilled_request_limited) BOOST_CHECK_EQUAL(CountQueuedInventory(*peer, CInv{MSG_GOVERNANCE_OBJECT, syncable_object_hash}), 1U); BOOST_CHECK(!m_node.netfulfilledman->HasFulfilledRequest(peer->addr, syncable_vote_sync_request)); BOOST_CHECK_EQUAL(CountQueuedMessages(*peer, NetMsgType::SYNCSTATUSCOUNT), 0U); - BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(peer->GetId(), stats)); - BOOST_CHECK_EQUAL(stats.m_misbehavior_score, 0); + AssertMisbehaviorScore(*m_node.peerman, *peer, 0); connman.FlushSendBuffer(*peer); auto syncable_stream = make_request_stream(syncable_object_hash, vote_filter); @@ -254,14 +255,12 @@ BOOST_AUTO_TEST_CASE(per_object_vote_sync_is_fulfilled_request_limited) BOOST_CHECK(m_node.netfulfilledman->HasFulfilledRequest(peer->addr, syncable_vote_sync_request)); BOOST_CHECK_EQUAL(CountQueuedMessages(*peer, NetMsgType::SYNCSTATUSCOUNT), 1U); - BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(peer->GetId(), stats)); - BOOST_CHECK_EQUAL(stats.m_misbehavior_score, 0); + AssertMisbehaviorScore(*m_node.peerman, *peer, 0); auto duplicate_syncable_stream = make_request_stream(syncable_object_hash, vote_filter); net_gov.ProcessMessage(*peer, NetMsgType::MNGOVERNANCESYNC, duplicate_syncable_stream); - BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(peer->GetId(), stats)); - BOOST_CHECK_EQUAL(stats.m_misbehavior_score, 20); + AssertMisbehaviorScore(*m_node.peerman, *peer, 20); connman.FlushSendBuffer(*peer); CGovernanceObject empty_filter_object{uint256(), /*revision=*/1, GetTime() + 2, uint256S("03"), /*data=*/{}}; @@ -275,8 +274,7 @@ BOOST_AUTO_TEST_CASE(per_object_vote_sync_is_fulfilled_request_limited) BOOST_CHECK_EQUAL(CountQueuedInventory(*peer, CInv{MSG_GOVERNANCE_OBJECT, empty_filter_object_hash}), 1U); BOOST_CHECK(!m_node.netfulfilledman->HasFulfilledRequest(peer->addr, empty_filter_vote_request)); BOOST_CHECK_EQUAL(CountQueuedMessages(*peer, NetMsgType::SYNCSTATUSCOUNT), 0U); - BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(peer->GetId(), stats)); - BOOST_CHECK_EQUAL(stats.m_misbehavior_score, 20); + AssertMisbehaviorScore(*m_node.peerman, *peer, 20); connman.FlushSendBuffer(*peer); auto duplicate_empty_filter_stream = make_request_stream(empty_filter_object_hash, CBloomFilter{}); @@ -285,8 +283,7 @@ BOOST_AUTO_TEST_CASE(per_object_vote_sync_is_fulfilled_request_limited) BOOST_CHECK_EQUAL(CountQueuedInventory(*peer, CInv{MSG_GOVERNANCE_OBJECT, empty_filter_object_hash}), 1U); BOOST_CHECK(!m_node.netfulfilledman->HasFulfilledRequest(peer->addr, empty_filter_vote_request)); BOOST_CHECK_EQUAL(CountQueuedMessages(*peer, NetMsgType::SYNCSTATUSCOUNT), 0U); - BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(peer->GetId(), stats)); - BOOST_CHECK_EQUAL(stats.m_misbehavior_score, 20); + AssertMisbehaviorScore(*m_node.peerman, *peer, 20); const uint256 unknown_vote_hash{uint256S("09")}; const std::string unknown_vote_request{strprintf("%s-votes-%s", NetMsgType::MNGOVERNANCESYNC, @@ -295,15 +292,13 @@ BOOST_AUTO_TEST_CASE(per_object_vote_sync_is_fulfilled_request_limited) net_gov.ProcessMessage(*peer, NetMsgType::MNGOVERNANCESYNC, unknown_vote_stream); BOOST_CHECK(!m_node.netfulfilledman->HasFulfilledRequest(peer->addr, unknown_vote_request)); - BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(peer->GetId(), stats)); - BOOST_CHECK_EQUAL(stats.m_misbehavior_score, 20); + AssertMisbehaviorScore(*m_node.peerman, *peer, 20); auto duplicate_unknown_vote_stream = make_request_stream(unknown_vote_hash, vote_filter); net_gov.ProcessMessage(*peer, NetMsgType::MNGOVERNANCESYNC, duplicate_unknown_vote_stream); BOOST_CHECK(!m_node.netfulfilledman->HasFulfilledRequest(peer->addr, unknown_vote_request)); - BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(peer->GetId(), stats)); - BOOST_CHECK_EQUAL(stats.m_misbehavior_score, 20); + AssertMisbehaviorScore(*m_node.peerman, *peer, 20); const uint256 object_fetch_hash{uint256S("0a")}; const std::string object_fetch_request{strprintf("%s-votes-%s", NetMsgType::MNGOVERNANCESYNC, @@ -312,15 +307,13 @@ BOOST_AUTO_TEST_CASE(per_object_vote_sync_is_fulfilled_request_limited) net_gov.ProcessMessage(*peer, NetMsgType::MNGOVERNANCESYNC, object_fetch_stream); BOOST_CHECK(!m_node.netfulfilledman->HasFulfilledRequest(peer->addr, object_fetch_request)); - BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(peer->GetId(), stats)); - BOOST_CHECK_EQUAL(stats.m_misbehavior_score, 20); + AssertMisbehaviorScore(*m_node.peerman, *peer, 20); auto duplicate_object_fetch_stream = make_request_stream(object_fetch_hash, CBloomFilter{}); net_gov.ProcessMessage(*peer, NetMsgType::MNGOVERNANCESYNC, duplicate_object_fetch_stream); BOOST_CHECK(!m_node.netfulfilledman->HasFulfilledRequest(peer->addr, object_fetch_request)); - BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(peer->GetId(), stats)); - BOOST_CHECK_EQUAL(stats.m_misbehavior_score, 20); + AssertMisbehaviorScore(*m_node.peerman, *peer, 20); m_node.peerman->FinalizeNode(*peer); } @@ -343,9 +336,7 @@ BOOST_AUTO_TEST_CASE(governance_objects_require_peer_announcement_or_request) m_node.peerman->InitializeNode(*second_announcing_peer, NODE_NETWORK); m_node.peerman->InitializeNode(*unsolicited_peer, NODE_NETWORK); - CNodeStateStats stats; - BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(announcing_peer->GetId(), stats)); - BOOST_CHECK_EQUAL(stats.m_misbehavior_score, 0); + AssertMisbehaviorScore(*m_node.peerman, *announcing_peer, 0); const CGovernanceObject govobj{MakeGovernanceObject(GetTime().count(), uint256S("21"))}; const CInv object_inv{MSG_GOVERNANCE_OBJECT, govobj.GetHash()}; @@ -365,22 +356,19 @@ BOOST_AUTO_TEST_CASE(governance_objects_require_peer_announcement_or_request) ProcessGovernanceObject(net_gov, *announcing_peer, govobj); BOOST_CHECK( !WITH_LOCK(::cs_main, return m_node.peerman->PeerConsumeObjectRequest(announcing_peer->GetId(), object_inv))); - BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(announcing_peer->GetId(), stats)); - BOOST_CHECK_EQUAL(stats.m_misbehavior_score, 0); + AssertMisbehaviorScore(*m_node.peerman, *announcing_peer, 0); ProcessGovernanceObject(net_gov, *second_announcing_peer, govobj); // Consumption is per-peer: the second announcer's own entry is accepted and consumed, // independent of the first peer's already-consumed entry. BOOST_CHECK(!WITH_LOCK(::cs_main, return m_node.peerman->PeerConsumeObjectRequest(second_announcing_peer->GetId(), object_inv))); - BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(second_announcing_peer->GetId(), stats)); - BOOST_CHECK_EQUAL(stats.m_misbehavior_score, 0); + AssertMisbehaviorScore(*m_node.peerman, *second_announcing_peer, 0); const CGovernanceObject unsolicited_govobj{MakeGovernanceObject(GetTime().count() + 1, uint256S("22"))}; ProcessGovernanceObject(net_gov, *unsolicited_peer, unsolicited_govobj); BOOST_CHECK(!m_node.govman->HaveObjectForHash(unsolicited_govobj.GetHash())); - BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(unsolicited_peer->GetId(), stats)); - BOOST_CHECK_EQUAL(stats.m_misbehavior_score, 0); + AssertMisbehaviorScore(*m_node.peerman, *unsolicited_peer, 0); m_node.peerman->FinalizeNode(*announcing_peer); m_node.peerman->FinalizeNode(*second_announcing_peer); @@ -407,15 +395,13 @@ BOOST_AUTO_TEST_CASE(governance_votes_require_peer_announcement_or_request) m_node.peerman->InitializeNode(*unsolicited_peer, NODE_NETWORK); auto& connman = static_cast(*m_node.connman); - CNodeStateStats stats; const CGovernanceVote vote{MakeGovernanceVote(uint256S("31"))}; const CInv vote_inv{MSG_GOVERNANCE_OBJECT_VOTE, vote.GetHash()}; ProcessGovernanceVote(net_gov, *unsolicited_peer, vote); BOOST_CHECK_EQUAL(CountQueuedMessages(*unsolicited_peer, NetMsgType::MNGOVERNANCESYNC), 0U); - BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(unsolicited_peer->GetId(), stats)); - BOOST_CHECK_EQUAL(stats.m_misbehavior_score, 0); + AssertMisbehaviorScore(*m_node.peerman, *unsolicited_peer, 0); ProcessInv(*m_node.peerman, *announcing_peer, vote_inv); ProcessInv(*m_node.peerman, *second_announcing_peer, vote_inv); @@ -423,8 +409,7 @@ BOOST_AUTO_TEST_CASE(governance_votes_require_peer_announcement_or_request) connman.FlushSendBuffer(*announcing_peer); ProcessGovernanceVote(net_gov, *announcing_peer, vote); BOOST_CHECK_EQUAL(CountQueuedMessages(*announcing_peer, NetMsgType::MNGOVERNANCESYNC), 1U); - BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(announcing_peer->GetId(), stats)); - BOOST_CHECK_EQUAL(stats.m_misbehavior_score, 0); + AssertMisbehaviorScore(*m_node.peerman, *announcing_peer, 0); connman.FlushSendBuffer(*second_announcing_peer); ProcessGovernanceVote(net_gov, *second_announcing_peer, vote); @@ -434,8 +419,7 @@ BOOST_AUTO_TEST_CASE(governance_votes_require_peer_announcement_or_request) // orphan-request side effect. BOOST_CHECK(!WITH_LOCK(::cs_main, return m_node.peerman->PeerConsumeObjectRequest(second_announcing_peer->GetId(), vote_inv))); - BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(second_announcing_peer->GetId(), stats)); - BOOST_CHECK_EQUAL(stats.m_misbehavior_score, 0); + AssertMisbehaviorScore(*m_node.peerman, *second_announcing_peer, 0); m_node.peerman->FinalizeNode(*announcing_peer); m_node.peerman->FinalizeNode(*second_announcing_peer);