From 4e76c10a5da6a992cc186886cab53b765249475c Mon Sep 17 00:00:00 2001 From: pasta Date: Mon, 3 Aug 2026 09:58:40 -0500 Subject: [PATCH 1/3] fix: update masternode rate limit on failed governance trigger path AddGovernanceObjectInternal emplaced a trigger into mapObjects, attempted AddTrigger, and on failure returned early before MasternodeRateUpdate. MasternodeRateCheck treats a missing rate-buffer entry as allow, so a masternode that never lands a successful trigger was never rate-limited at all, defeating precisely the limiter meant to stop it. Each rejected trigger still cost a BLS verification and left a mapErasedGovernanceObjects entry retained for roughly 60 days on mainnet. Advance the rate buffer on the failure path too. Relay scheduling is extracted so a trigger just marked deleted is not added to the additional-relay set; that part is a self-correction for a regression this change would otherwise introduce, not a pre-existing bug. --- src/Makefile.test.include | 1 + src/governance/governance.cpp | 33 ++- src/governance/governance.h | 5 + .../governance_failed_trigger_rate_tests.cpp | 196 ++++++++++++++++++ 4 files changed, 226 insertions(+), 9 deletions(-) create mode 100644 src/test/governance_failed_trigger_rate_tests.cpp diff --git a/src/Makefile.test.include b/src/Makefile.test.include index faaee5aa1913..048aa2a61838 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -120,6 +120,7 @@ BITCOIN_TESTS =\ test/flatfile_tests.cpp \ test/fs_tests.cpp \ test/getarg_tests.cpp \ + test/governance_failed_trigger_rate_tests.cpp \ test/governance_inv_tests.cpp \ test/governance_superblock_tests.cpp \ test/governance_validators_tests.cpp \ diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index d87fef40fac5..483b85301c86 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -344,18 +344,25 @@ void CGovernanceManager::AddGovernanceObjectInternal(CGovernanceObject& insert_o LogPrint(BCLog::GOBJECT, "CGovernanceManager::AddGovernanceObject -- Before trigger block, GetDataAsPlainString = %s, nObjectType = %d\n", Assert(govobj)->GetDataAsPlainString(), std23::to_underlying(govobj->GetObjectType())); + // Count the attempt against the per-masternode rate buffer before any early + // return. Failed AddTrigger paths used to skip this, so a single operator + // key could flood mapObjects with unparseable triggers. + MasternodeRateUpdate(*govobj); + if (govobj->GetObjectType() == GovernanceObject::TRIGGER && !m_superblocks.AddTrigger(govobj, nCachedBlockHeight)) { LogPrint(BCLog::GOBJECT, "CGovernanceManager::AddGovernanceObject -- undo adding invalid trigger object: hash = %s\n", nHash.ToString()); govobj->PrepareDeletion(GetTime().count()); return; } + // Only objects we keep may be announced. Scheduling this before the AddTrigger + // check would make us re-announce, and serve on GETDATA, a trigger we just + // undid and marked for deletion. + ScheduleAdditionalRelay(*govobj); + LogPrint(BCLog::GOBJECT, "CGovernanceManager::AddGovernanceObject -- %s new, received from peer %s\n", strHash, peer_str); RelayObject(*govobj); - // Update the rate buffer - MasternodeRateUpdate(*govobj); - m_mn_sync.BumpAssetLastTime("CGovernanceManager::AddGovernanceObject"); // WE MIGHT HAVE PENDING/ORPHAN VOTES FOR THIS OBJECT @@ -701,15 +708,23 @@ void CGovernanceManager::MasternodeRateUpdate(const CGovernanceObject& govobj) it = mapLastMasternodeObject.insert(txout_m_t::value_type(masternodeOutpoint, last_object_rec(true))).first; } - int64_t nTimestamp = govobj.GetCreationTime(); - it->second.triggerBuffer.AddTimestamp(nTimestamp); + it->second.triggerBuffer.AddTimestamp(govobj.GetCreationTime()); + it->second.fStatusOK = true; +} + +void CGovernanceManager::ScheduleAdditionalRelay(const CGovernanceObject& govobj) +{ + AssertLockHeld(cs_store); - if (nTimestamp > GetTime() + count_seconds(MAX_TIME_FUTURE_DEVIATION) - count_seconds(RELIABLE_PROPAGATION_TIME)) { - // schedule additional relay for the object + if (govobj.GetObjectType() != GovernanceObject::TRIGGER) return; + + // An object created this close to the future-deviation limit is still too new for + // peers with a lagging clock to accept, so re-announce it once it has aged past + // RELIABLE_PROPAGATION_TIME (see CheckPostponedObjects). + if (govobj.GetCreationTime() > + GetTime() + count_seconds(MAX_TIME_FUTURE_DEVIATION) - count_seconds(RELIABLE_PROPAGATION_TIME)) { setAdditionalRelayObjects.insert(govobj.GetHash()); } - - it->second.fStatusOK = true; } bool CGovernanceManager::MasternodeRateCheck(const CGovernanceObject& govobj, bool fUpdateFailStatus) diff --git a/src/governance/governance.h b/src/governance/governance.h index cca00d8cdb60..280f62f67394 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -403,6 +403,11 @@ class CGovernanceManager : public GovernanceStore void MasternodeRateUpdate(const CGovernanceObject& govobj) EXCLUSIVE_LOCKS_REQUIRED(cs_store); + /** Queue a deferred re-announcement for a trigger that is too new to propagate + * reliably yet. Only call this for objects we are keeping. */ + void ScheduleAdditionalRelay(const CGovernanceObject& govobj) + EXCLUSIVE_LOCKS_REQUIRED(cs_store); + bool MasternodeRateCheck(const CGovernanceObject& govobj, bool fUpdateFailStatus, bool fForce, bool& fRateCheckBypassed) EXCLUSIVE_LOCKS_REQUIRED(cs_store); diff --git a/src/test/governance_failed_trigger_rate_tests.cpp b/src/test/governance_failed_trigger_rate_tests.cpp new file mode 100644 index 000000000000..8293362bae91 --- /dev/null +++ b/src/test/governance_failed_trigger_rate_tests.cpp @@ -0,0 +1,196 @@ +// Copyright (c) 2026 The Dash Core developers +// 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 +#include +#include +#include +#include