Skip to content
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.

Sign up for GitHub

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

[budget] Do not count votes twice after calling GetBudgetWithHighestVoteCount. #2658

Merged
merged 2 commits into from
Dec 14, 2021
Merged

[budget] Do not count votes twice after calling GetBudgetWithHighestVoteCount. #2658

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ef317c5
budget: do not count votes twice after calling GetBudgetWithHighestVo…
furszy Dec 1, 2021
f62b2a7
budget: remove budgetproposal dependency on masternodeman.
furszy Dec 2, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 20 additions & 13 deletions src/budget/budgetmanager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,8 @@ bool CBudgetManager::AddProposal(CBudgetProposal& budgetProposal)
}

// update expiration / heavily-downvoted
if (!budgetProposal.UpdateValid(nCurrentHeight)) {
int mnCount = mnodeman.CountEnabled();
if (!budgetProposal.UpdateValid(nCurrentHeight, mnCount)) {
LogPrint(BCLog::MNBUDGET,"%s: Invalid budget proposal %s %s\n", __func__, nHash.ToString(), budgetProposal.IsInvalidLogStr());
return false;
}
Expand All @@ -322,13 +323,16 @@ void CBudgetManager::CheckAndRemove()
std::map<uint256, CFinalizedBudget> tmpMapFinalizedBudgets;
std::map<uint256, CBudgetProposal> tmpMapProposals;

// Get MN count, used for the heavily down-voted check
int mnCount = mnodeman.CountEnabled();

// Check Proposals first
{
LOCK(cs_proposals);
LogPrint(BCLog::MNBUDGET, "%s: mapProposals cleanup - size before: %d\n", __func__, mapProposals.size());
for (auto& it: mapProposals) {
CBudgetProposal* pbudgetProposal = &(it.second);
if (!pbudgetProposal->UpdateValid(nCurrentHeight)) {
if (!pbudgetProposal->UpdateValid(nCurrentHeight, mnCount)) {
LogPrint(BCLog::MNBUDGET,"%s: Invalid budget proposal %s %s\n", __func__, (it.first).ToString(), pbudgetProposal->IsInvalidLogStr());
mapFeeTxToProposal.erase(pbudgetProposal->GetFeeTXHash());
} else {
Expand Down Expand Up @@ -419,7 +423,7 @@ void CBudgetManager::RemoveByFeeTxId(const uint256& feeTxId)
}
}

const CFinalizedBudget* CBudgetManager::GetBudgetWithHighestVoteCount(int chainHeight) const
CBudgetManager::HighestFinBudget CBudgetManager::GetBudgetWithHighestVoteCount(int chainHeight) const
{
LOCK(cs_budgets);
int highestVoteCount = 0;
Expand All @@ -434,13 +438,13 @@ const CFinalizedBudget* CBudgetManager::GetBudgetWithHighestVoteCount(int chainH
highestVoteCount = voteCount;
}
}
return pHighestBudget;
return {pHighestBudget, highestVoteCount};
}

int CBudgetManager::GetHighestVoteCount(int chainHeight) const
{
const CFinalizedBudget* pbudget = GetBudgetWithHighestVoteCount(chainHeight);
return (pbudget ? pbudget->GetVoteCount() : -1);
const auto& highestBudFin = GetBudgetWithHighestVoteCount(chainHeight);
return (highestBudFin.m_budget_fin ? highestBudFin.m_vote_count : -1);
}

bool CBudgetManager::GetPayeeAndAmount(int chainHeight, CScript& payeeRet, CAmount& nAmountRet) const
Expand All @@ -449,8 +453,9 @@ bool CBudgetManager::GetPayeeAndAmount(int chainHeight, CScript& payeeRet, CAmou
if (!IsBudgetPaymentBlock(chainHeight, nCountThreshold))
return false;

const CFinalizedBudget* pfb = GetBudgetWithHighestVoteCount(chainHeight);
return pfb && pfb->GetPayeeAndAmount(chainHeight, payeeRet, nAmountRet) && pfb->GetVoteCount() > nCountThreshold;
const auto& highestBudFin = GetBudgetWithHighestVoteCount(chainHeight);
const CFinalizedBudget* pfb = highestBudFin.m_budget_fin;
return pfb && pfb->GetPayeeAndAmount(chainHeight, payeeRet, nAmountRet) && highestBudFin.m_vote_count > nCountThreshold;
}

bool CBudgetManager::GetExpectedPayeeAmount(int chainHeight, CAmount& nAmountRet) const
Expand Down Expand Up @@ -670,10 +675,11 @@ TrxValidationStatus CBudgetManager::IsTransactionValid(const CTransaction& txNew
{
LOCK(cs_budgets);
// Get the finalized budget with the highest amount of votes..
const CFinalizedBudget* highestVotesBudget = GetBudgetWithHighestVoteCount(nBlockHeight);
const auto& highestBudFin = GetBudgetWithHighestVoteCount(nBlockHeight);
const CFinalizedBudget* highestVotesBudget = highestBudFin.m_budget_fin;
if (highestVotesBudget) {
// Need to surpass the threshold
if (highestVotesBudget->GetVoteCount() > nCountThreshold) {
if (highestBudFin.m_vote_count > nCountThreshold) {
fThreshold = true;
if (highestVotesBudget->IsTransactionValid(txNew, nBlockHash, nBlockHeight) ==
TrxValidationStatus::Valid) {
Expand Down Expand Up @@ -1374,7 +1380,8 @@ bool CBudgetManager::UpdateProposal(const CBudgetVote& vote, CNode* pfrom, std::
LOCK(cs_proposals);

const uint256& nProposalHash = vote.GetProposalHash();
if (!mapProposals.count(nProposalHash)) {
const auto& itProposal = mapProposals.find(nProposalHash);
if (itProposal == mapProposals.end()) {
if (pfrom) {
// only ask for missing items after our syncing process is complete --
// otherwise we'll think a full sync succeeded when they return a result
Expand All @@ -1393,8 +1400,8 @@ bool CBudgetManager::UpdateProposal(const CBudgetVote& vote, CNode* pfrom, std::
return false;
}


return mapProposals[nProposalHash].AddOrUpdateVote(vote, strError);
// Add or update vote
return itProposal->second.AddOrUpdateVote(vote, strError);
}

bool CBudgetManager::UpdateFinalizedBudget(CFinalizedBudgetVote& vote, CNode* pfrom, std::string& strError)
Expand Down
7 changes: 6 additions & 1 deletion src/budget/budgetmanager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,13 @@ class CBudgetManager : public CValidationInterface
// who's asked for the complete budget sync and the last time
std::map<CNetAddr, int64_t> mAskedUsForBudgetSync; // guarded by cs_budgets and cs_proposals.

struct HighestFinBudget {
const CFinalizedBudget* m_budget_fin{nullptr};
int m_vote_count{0};
};

// Returns a const pointer to the budget with highest vote count
const CFinalizedBudget* GetBudgetWithHighestVoteCount(int chainHeight) const;
HighestFinBudget GetBudgetWithHighestVoteCount(int chainHeight) const;
int GetHighestVoteCount(int chainHeight) const;
// Get the payee and amount for the budget with the highest vote count
bool GetPayeeAndAmount(int chainHeight, CScript& payeeRet, CAmount& nAmountRet) const;
Expand Down
12 changes: 6 additions & 6 deletions src/budget/budgetproposal.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

#include "budget/budgetproposal.h"

#include "masternodeman.h"
#include "chainparams.h"
#include "script/standard.h"

CBudgetProposal::CBudgetProposal():
nAllotted(0),
Expand Down Expand Up @@ -77,9 +77,9 @@ void CBudgetProposal::SyncVotes(CNode* pfrom, bool fPartial, int& nInvCount) con
}
}

bool CBudgetProposal::IsHeavilyDownvoted(bool fNewRules)
bool CBudgetProposal::IsHeavilyDownvoted(bool fNewRules, int mnCount)
{
if (GetNays() - GetYeas() > (fNewRules ? 3 : 1) * mnodeman.CountEnabled() / 10) {
if (GetNays() - GetYeas() > (fNewRules ? 3 : 1) * mnCount / 10) {
strInvalid = "Heavily Downvoted";
return true;
}
Expand Down Expand Up @@ -166,7 +166,7 @@ bool CBudgetProposal::updateExpired(int nCurrentHeight)
return false;
}

bool CBudgetProposal::UpdateValid(int nCurrentHeight)
bool CBudgetProposal::UpdateValid(int nCurrentHeight, int mnCount)
{
fValid = false;

Expand All @@ -175,7 +175,7 @@ bool CBudgetProposal::UpdateValid(int nCurrentHeight)

// Never kill a proposal before the first superblock
if (!fNewRules || nCurrentHeight > nBlockStart) {
if (IsHeavilyDownvoted(fNewRules)) return false;
if (IsHeavilyDownvoted(fNewRules, mnCount)) return false;
}

if (updateExpired(nCurrentHeight)) {
Expand Down
4 changes: 2 additions & 2 deletions src/budget/budgetproposal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class CBudgetProposal
std::string strInvalid;

// Functions used inside UpdateValid()/IsWellFormed - setting strInvalid
bool IsHeavilyDownvoted(bool fNewRules);
bool IsHeavilyDownvoted(bool fNewRules, int mnCount);
bool updateExpired(int nCurrentHeight);
bool CheckStartEnd();
bool CheckAmount(const CAmount& nTotalBudget);
Expand Down Expand Up @@ -64,7 +64,7 @@ class CBudgetProposal
void SyncVotes(CNode* pfrom, bool fPartial, int& nInvCount) const;

// sets fValid and strInvalid, returns fValid
bool UpdateValid(int nHeight);
bool UpdateValid(int nHeight, int mnCount);
// Static checks that should be done only once - sets strInvalid
bool IsWellFormed(const CAmount& nTotalBudget);
bool IsValid() const { return fValid; }
Expand Down