-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: avoid voting for the same trigger multiple times #6155
Conversation
fc7425b
to
761f5c0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NOTE to myself: do clang-format
done
why not increase this timeout? |
src/governance/governance.cpp
Outdated
ranges::any_of(voteRecord.mapInstances, [&](const auto& voteInstancePair) { | ||
if (voteInstancePair.first == VOTE_SIGNAL_FUNDING) { | ||
if (voteInstancePair.second.eOutcome == VOTE_OUTCOME_YES) { | ||
votedFundingYesTriggerHash = gov_sb_hash; | ||
} | ||
LogPrint(BCLog::GOBJECT, /* Continued */ | ||
"CGovernanceManager::%s " | ||
"Not voting YES-FUNDING for trigger:%s, we voted %s for it already\n", | ||
strFunc, gov_sb_hash.ToString(), | ||
CGovernanceVoting::ConvertOutcomeToString(voteInstancePair.second.eOutcome)); | ||
return true; | ||
} | ||
return false; | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm confused.. you're doing ranges::any_of, but discarding the return values?
11b6dba
to
4d61b7b
Compare
src/governance/governance.cpp
Outdated
if (vote_rec_t voteRecord; trigger_opt.value().GetCurrentMNVotes(mn_activeman.GetOutPoint(), voteRecord)) { | ||
const auto& strFunc = __func__; | ||
// Let's see if there is a VOTE_SIGNAL_FUNDING vote from us already | ||
ranges::any_of(voteRecord.mapInstances, [&](const auto& voteInstancePair) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
still discarding this return value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK 3e78081; I do wish there was a regression test maybe, but otherwise looks good
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK e92aad7
ping @knst |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM e92aad7
Issue being fixed or feature implemented
We just had a sb voting period and I noticed that the network is way too spammy and produces too many votes (10x+ the expected numbers). It turns out that relying on
ProcessVoteAndRelay
on mainnet is not enough because rate-check expires too soon and MNs are able to vote again and again. On testnet it was never an issue because the voting period there is really short.What was done?
Check known votes to make sure we never voted earlier.
How Has This Been Tested?
Run tests, run a MN on mainnet and check logs.
Breaking Changes
n/a
Checklist: