Skip to content

Commit

Permalink
fix: expire triggers that are too far into the future (#5646)
Browse files Browse the repository at this point in the history
## Issue being fixed or feature implemented
`gobject count all`

before:
>Governance Objects: 1195 (Proposals: 9, Triggers: 1186, Other: 0;
Erased: 1), Votes: 135064

after (in 10-ish minutes after gov sync is done):
>Governance Objects: 11 (Proposals: 9, Triggers: 2, Other: 0; Erased:
1), Votes: 702

I _think_ it happens when a node can't follow the right chain for some
reason but it keeps receiving triggers and votes from other nodes which
means triggers never expire on such node. This wouldn't be a problem for
us if we wouldn't reorg testnet/devnets from time to time. Once we reorg
the stuck node happily spams us with all the triggers it saved in the
meantime.

## What was done?
2 sb cycles into the future should be enough for all legit triggers,
drop the ones that have their height even higher

## How Has This Been Tested?
run a node, check rpc

## Breaking Changes
n/a

## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
  • Loading branch information
UdjinM6 authored Oct 26, 2023
1 parent 297b50a commit 3e732a9
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 4 deletions.
10 changes: 9 additions & 1 deletion src/governance/classes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ bool CGovernanceTriggerManager::AddNewTrigger(uint256 nHash)

mapTrigger.insert(std::make_pair(nHash, pSuperblock));

return true;
return !pSuperblock->IsExpired(*governance);
}

/**
Expand Down Expand Up @@ -724,6 +724,14 @@ bool CSuperblock::IsExpired(const CGovernanceManager& governanceManager) const
return true;
}

if (Params().NetworkIDString() != CBaseChainParams::MAIN) {
// NOTE: this can happen on testnet/devnets due to reorgs, should never happen on mainnet
if (governanceManager.GetCachedBlockHeight() + Params().GetConsensus().nSuperblockCycle * 2 < nBlockHeight) {
LogPrint(BCLog::GOBJECT, "CSuperblock::IsExpired -- Trigger is too far into the future\n");
return true;
}
}

return false;
}

Expand Down
6 changes: 3 additions & 3 deletions src/governance/governance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1203,8 +1203,8 @@ int CGovernanceManager::RequestGovernanceObjectVotes(Span<CNode*> vNodesCopy, CC

if (mapObjects.empty()) return -2;

for (const auto& objPair : mapObjects) {
uint256 nHash = objPair.first;
for (const auto& [nHash, govobj] : mapObjects) {
if (govobj.IsSetCachedDelete()) continue;
if (mapAskedRecently.count(nHash)) {
auto it = mapAskedRecently[nHash].begin();
while (it != mapAskedRecently[nHash].end()) {
Expand All @@ -1217,7 +1217,7 @@ int CGovernanceManager::RequestGovernanceObjectVotes(Span<CNode*> vNodesCopy, CC
if (mapAskedRecently[nHash].size() >= nPeersPerHashMax) continue;
}

if (objPair.second.GetObjectType() == GovernanceObject::TRIGGER) {
if (govobj.GetObjectType() == GovernanceObject::TRIGGER) {
vTriggerObjHashes.push_back(nHash);
} else {
vOtherObjHashes.push_back(nHash);
Expand Down

0 comments on commit 3e732a9

Please sign in to comment.