From 5ad1ef1b103c71871ee05ad5e10b1c368a0ee11c Mon Sep 17 00:00:00 2001 From: PastaClaw Date: Tue, 7 Jul 2026 16:50:10 -0500 Subject: [PATCH] fix: throttle governance vote sync requests --- src/governance/governance.cpp | 51 +++++++- src/governance/governance.h | 8 ++ src/governance/net_governance.cpp | 58 +++++++-- src/test/governance_inv_tests.cpp | 198 ++++++++++++++++++++++++++++++ 4 files changed, 303 insertions(+), 12 deletions(-) diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index ec4419bce7b5..bc0ccfa4a4ab 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -34,6 +34,12 @@ constexpr std::chrono::seconds GOVERNANCE_ORPHAN_EXPIRATION_TIME{10min}; constexpr std::chrono::seconds MAX_TIME_FUTURE_DEVIATION{1h}; using governance::RELIABLE_PROPAGATION_TIME; +bool IsSyncableObject(const std::shared_ptr& govobj) +{ + const auto& obj = *Assert(govobj); + return !obj.IsSetCachedDelete() && !obj.IsSetExpired(); +} + class ScopedLockBool { bool& ref; @@ -137,6 +143,38 @@ bool CGovernanceManager::HaveObjectForHash(const uint256& nHash) const return (mapObjects.count(nHash) == 1 || mapPostponedObjects.count(nHash) == 1); } +bool CGovernanceManager::HaveObjectForFetch(const uint256& nHash) const +{ + LOCK(cs_store); + + if (mapErasedGovernanceObjects.count(nHash) != 0) { + return false; + } + + auto it = mapObjects.find(nHash); + if (it != mapObjects.end()) { + return IsSyncableObject(it->second); + } + + it = mapPostponedObjects.find(nHash); + if (it != mapPostponedObjects.end()) { + return IsSyncableObject(it->second); + } + + return false; +} + +bool CGovernanceManager::HaveSyncableObjectForHash(const uint256& nHash) const +{ + LOCK(cs_store); + const auto it = mapObjects.find(nHash); + if (it == mapObjects.end()) { + return false; + } + + return IsSyncableObject(it->second); +} + bool CGovernanceManager::SerializeObjectForHash(const uint256& nHash, CDataStream& ss) const { LOCK(cs_store); @@ -334,6 +372,13 @@ void CGovernanceManager::AddGovernanceObject(CGovernanceObject& govobj, const st AddGovernanceObjectInternal(govobj, peer_str); } +void CGovernanceManager::AddGovernanceObjectForTesting(const CGovernanceObject& govobj) +{ + AssertLockNotHeld(cs_store); + LOCK(cs_store); + mapObjects.emplace(govobj.GetHash(), std::make_shared(govobj)); +} + void CGovernanceManager::CheckAndRemove() { AssertLockNotHeld(cs_store); @@ -624,14 +669,14 @@ std::vector CGovernanceManager::GetSyncableVoteInvs(const uint256& nProp, return {}; } - const auto& govobj = *Assert(it->second); - if (govobj.IsSetCachedDelete() || govobj.IsSetExpired()) { + if (!IsSyncableObject(it->second)) { return {}; } std::vector invs; const auto tip_mn_list = m_dmnman.GetListAtChainTip(); + const auto& govobj = *Assert(it->second); LOCK(govobj.cs); const auto& fileVotes = govobj.GetVoteFile(); for (const auto& vote : fileVotes.GetVotes()) { @@ -656,7 +701,7 @@ std::vector CGovernanceManager::GetSyncableObjectInvs() const invs.reserve(mapObjects.size()); for (const auto& [nHash, govobj] : mapObjects) { - if (Assert(govobj)->IsSetCachedDelete() || govobj->IsSetExpired()) { + if (!IsSyncableObject(govobj)) { continue; } invs.emplace_back(MSG_GOVERNANCE_OBJECT, nHash); diff --git a/src/governance/governance.h b/src/governance/governance.h index dff378c20d47..fa35b6fe2119 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -323,10 +323,18 @@ class CGovernanceManager : public GovernanceStore EXCLUSIVE_LOCKS_REQUIRED(!cs_store); void AddGovernanceObject(CGovernanceObject& govobj, const std::string& peer_str) EXCLUSIVE_LOCKS_REQUIRED(!cs_store, !cs_relay); + /** Test-only helper: inserts an object into the syncable object store without + * running collateral or chain validation. */ + void AddGovernanceObjectForTesting(const CGovernanceObject& govobj) + EXCLUSIVE_LOCKS_REQUIRED(!cs_store); // Thread-safe accessors bool HaveObjectForHash(const uint256& nHash) const EXCLUSIVE_LOCKS_REQUIRED(!cs_store); + bool HaveObjectForFetch(const uint256& nHash) const + EXCLUSIVE_LOCKS_REQUIRED(!cs_store); + bool HaveSyncableObjectForHash(const uint256& nHash) const + EXCLUSIVE_LOCKS_REQUIRED(!cs_store); bool HaveVoteForHash(const uint256& nHash) const EXCLUSIVE_LOCKS_REQUIRED(!cs_store); bool SerializeObjectForHash(const uint256& nHash, CDataStream& ss) const diff --git a/src/governance/net_governance.cpp b/src/governance/net_governance.cpp index 97e21b9025a8..da27cf57b453 100644 --- a/src/governance/net_governance.cpp +++ b/src/governance/net_governance.cpp @@ -15,9 +15,27 @@ #include #include #include +#include +#include + +#include class CConnman; +namespace { +bool IsEmptyBloomFilter(const CBloomFilter& filter) +{ + // CBloomFilter does not expose its backing bytes. Governance uses an empty + // filter as an object-fetch signal, so inspect the serialized vector field. + CDataStream serialized_filter{SER_NETWORK, PROTOCOL_VERSION}; + serialized_filter << filter; + + std::vector filter_data; + serialized_filter >> filter_data; + return filter_data.empty(); +} +} // namespace + void NetGovernance::Schedule(CScheduler& scheduler) { // Code below is meant to be running only if governance validation is enabled @@ -86,17 +104,39 @@ void NetGovernance::ProcessMessage(CNode& peer, const std::string& msg_type, CDa } LogPrint(BCLog::GOBJECT, "MNGOVERNANCESYNC -- syncing governance objects to our peer %s\n", peer.GetLogString()); - if (nProp == uint256()) { - // Full sync of all governance objects - assert(m_netfulfilledman.IsValid()); - if (m_netfulfilledman.HasFulfilledRequest(peer.addr, NetMsgType::MNGOVERNANCESYNC)) { - // Asking for the whole list multiple times in a short period of time is no good - LogPrint(BCLog::GOBJECT, "MNGOVERNANCESYNC -- peer already asked me for the list\n"); - m_peer_manager->PeerMisbehaving(peer.GetId(), 20); - return; + const bool full_sync{nProp == uint256()}; + const bool object_fetch{!full_sync && IsEmptyBloomFilter(filter)}; + // Nonzero govsync with an empty filter is used to retry missing-object + // fetches for orphan votes. Only full sync and actual known-object vote + // sync are fulfilled-request limited. + const bool track_request{full_sync || (!object_fetch && m_gov_manager.HaveSyncableObjectForHash(nProp))}; + const std::string fulfilled_request{full_sync ? + NetMsgType::MNGOVERNANCESYNC : + strprintf("%s-votes-%s", NetMsgType::MNGOVERNANCESYNC, + nProp.ToString())}; + assert(m_netfulfilledman.IsValid()); + if (track_request && m_netfulfilledman.HasFulfilledRequest(peer.addr, fulfilled_request)) { + // Asking for the same governance data multiple times in a short period of time is no good + LogPrint(BCLog::GOBJECT, "MNGOVERNANCESYNC -- peer already asked me for %s\n", + full_sync ? "the list" : strprintf("votes for %s", nProp.ToString())); + m_peer_manager->PeerMisbehaving(peer.GetId(), 20); + return; + } + if (track_request) { + m_netfulfilledman.AddFulfilledRequest(peer.addr, fulfilled_request); + } + + if (object_fetch) { + if (m_gov_manager.HaveObjectForFetch(nProp)) { + CNetMsgMaker msgMaker(peer.GetCommonVersion()); + m_connman.PushMessage(&peer, msgMaker.Make(NetMsgType::INV, + std::vector{CInv{MSG_GOVERNANCE_OBJECT, nProp}})); } - m_netfulfilledman.AddFulfilledRequest(peer.addr, NetMsgType::MNGOVERNANCESYNC); + return; + } + if (full_sync) { + // Full sync of all governance objects auto invs = m_gov_manager.GetSyncableObjectInvs(); LogPrint(BCLog::GOBJECT, "MNGOVERNANCESYNC -- syncing %d objects to peer=%d\n", invs.size(), peer.GetId()); diff --git a/src/test/governance_inv_tests.cpp b/src/test/governance_inv_tests.cpp index 9a5c06561199..5da39063e58a 100644 --- a/src/test/governance_inv_tests.cpp +++ b/src/test/governance_inv_tests.cpp @@ -2,8 +2,10 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include #include #include +#include #include #include #include @@ -17,6 +19,7 @@ #include #include +#include #include #include @@ -103,6 +106,38 @@ void CheckInvExpirationCycle(CGovernanceManager& govman, const CInv& inv) BOOST_CHECK(govman.ConfirmInventoryRequest(inv)); BOOST_CHECK_EQUAL(govman.RequestedHashCacheSizeForTesting(), 1U); } + +size_t CountQueuedMessages(const CNode& peer, const std::string& msg_type) +{ + LOCK(peer.cs_vSend); + size_t count{0}; + for (const auto& msg : peer.vSendMsg) { + if (msg.m_type == msg_type) { + ++count; + } + } + return count; +} + +size_t CountQueuedInventory(const CNode& peer, const CInv& expected_inv) +{ + LOCK(peer.cs_vSend); + size_t count{0}; + for (const auto& msg : peer.vSendMsg) { + if (msg.m_type != NetMsgType::INV) { + continue; + } + CDataStream stream{msg.data, SER_NETWORK, PROTOCOL_VERSION}; + std::vector invs; + stream >> invs; + for (const auto& inv : invs) { + if (inv.type == expected_inv.type && inv.hash == expected_inv.hash) { + ++count; + } + } + } + return count; +} } // namespace BOOST_FIXTURE_TEST_SUITE(governance_inv_tests, GovernanceInvSetup) @@ -216,4 +251,167 @@ BOOST_AUTO_TEST_CASE(net_governance_schedule_drives_check_and_remove) BOOST_CHECK_EQUAL(m_node.govman->RequestedHashCacheSizeForTesting(), 0U); } +BOOST_AUTO_TEST_CASE(per_object_vote_sync_is_fulfilled_request_limited) +{ + LOCK(NetEventsInterface::g_msgproc_mutex); + + // NetGovernance::ProcessMessage ignores MNGOVERNANCESYNC until sync is + // fully finished; advance from GOVERNANCE to FINISHED. + 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 make_request_stream = [](const uint256& object_hash, const CBloomFilter& filter) { + CDataStream stream{SER_NETWORK, PROTOCOL_VERSION}; + stream << object_hash << filter; + return stream; + }; + + 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); + + const CBloomFilter vote_filter{1, 0.001, 0, BLOOM_UPDATE_NONE}; + CGovernanceObject postponed_object{uint256(), /*revision=*/1, GetTime(), uint256::ONE, /*data=*/{}}; + m_node.govman->AddPostponedObject(postponed_object); + + const uint256 postponed_object_hash{postponed_object.GetHash()}; + 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); + + 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); + + 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=*/{}}; + m_node.govman->AddGovernanceObjectForTesting(syncable_object); + + 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)); + + 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); + + 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); + auto duplicate_syncable_object_fetch_stream = make_request_stream(syncable_object_hash, CBloomFilter{}); + 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(stats.m_misbehavior_score, 0); + + connman.FlushSendBuffer(peer); + auto syncable_stream = make_request_stream(syncable_object_hash, vote_filter); + 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_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); + + BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(peer.GetId(), stats)); + BOOST_CHECK_EQUAL(stats.m_misbehavior_score, 20); + + 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); + + 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); + auto duplicate_empty_filter_stream = make_request_stream(empty_filter_object_hash, CBloomFilter{}); + 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(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); + + 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); + + 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); + + 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); + + 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); +} + BOOST_AUTO_TEST_SUITE_END()