refactor: move DecreaseScores method to be inside CDeterministicMNList class (#5615)

## Issue being fixed or feature implemented
It makes more sense for DecreaseScores to be inside of the MNList itself
imo

## What was done?
Refactored as such


## How Has This Been Tested?
Reindexed

I had originally expected some performance improvements due to the
removal of `GetMN` but in my benchmarking I didn't see any noticeable
perf changes. I do still think the removal of `GetMN` and using a
shared_ptr the whole time is better as it removes the chance of the
master node disappearing from the list (which would have previously
thrown, but is now impossible).

## Breaking Changes
None

## Checklist:
_Go over all the following points, and put an `x` in all the boxes that
apply._
- [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)_

---------

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
This commit is contained in:
PastaPastaPasta 2023-10-18 04:57:35 -05:00 committed by GitHub
parent 63ed462c54
commit 1c6f643c46
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 25 additions and 30 deletions

View File

@ -366,17 +366,31 @@ void CDeterministicMNList::PoSePunish(const uint256& proTxHash, int penalty, boo
UpdateMN(proTxHash, newState);
}
void CDeterministicMNList::PoSeDecrease(const uint256& proTxHash)
void CDeterministicMNList::DecreaseScores()
{
auto dmn = GetMN(proTxHash);
if (!dmn) {
throw(std::runtime_error(strprintf("%s: Can't find a masternode with proTxHash=%s", __func__, proTxHash.ToString())));
std::vector<CDeterministicMNCPtr> toDecrease;
toDecrease.reserve(GetAllMNsCount() / 10);
// only iterate and decrease for valid ones (not PoSe banned yet)
// if a MN ever reaches the maximum, it stays in PoSe banned state until revived
ForEachMNShared(true /* onlyValid */, [&toDecrease](auto& dmn) {
// There is no reason to check if this MN is banned here since onlyValid=true will only run on non-banned MNs
if (dmn->pdmnState->nPoSePenalty > 0) {
toDecrease.emplace_back(dmn);
}
assert(dmn->pdmnState->nPoSePenalty > 0 && !dmn->pdmnState->IsBanned());
});
auto newState = std::make_shared<CDeterministicMNState>(*dmn->pdmnState);
for (const auto& proTxHash : toDecrease) {
PoSeDecrease(*proTxHash);
}
}
void CDeterministicMNList::PoSeDecrease(const CDeterministicMN& dmn)
{
assert(dmn.pdmnState->nPoSePenalty > 0 && !dmn.pdmnState->IsBanned());
auto newState = std::make_shared<CDeterministicMNState>(*dmn.pdmnState);
newState->nPoSePenalty--;
UpdateMN(proTxHash, newState);
UpdateMN(dmn, newState);
}
CDeterministicMNListDiff CDeterministicMNList::BuildDiff(const CDeterministicMNList& to) const
@ -723,7 +737,7 @@ bool CDeterministicMNManager::BuildNewListFromBlock(const CBlock& block, const C
}
});
DecreasePoSePenalties(newList);
newList.DecreaseScores();
bool isMNRewardReallocation = llmq::utils::IsMNRewardReallocationActive(pindexPrev);
@ -1002,24 +1016,6 @@ void CDeterministicMNManager::HandleQuorumCommitment(const llmq::CFinalCommitmen
}
}
void CDeterministicMNManager::DecreasePoSePenalties(CDeterministicMNList& mnList)
{
std::vector<uint256> toDecrease;
toDecrease.reserve(mnList.GetAllMNsCount() / 10);
// only iterate and decrease for valid ones (not PoSe banned yet)
// if a MN ever reaches the maximum, it stays in PoSe banned state until revived
mnList.ForEachMN(true /* onlyValid */, [&toDecrease](auto& dmn) {
// There is no reason to check if this MN is banned here since onlyValid=true will only run on non-banned MNs
if (dmn.pdmnState->nPoSePenalty > 0) {
toDecrease.emplace_back(dmn.proTxHash);
}
});
for (const auto& proTxHash : toDecrease) {
mnList.PoSeDecrease(proTxHash);
}
}
CDeterministicMNList CDeterministicMNManager::GetListForBlockInternal(const CBlockIndex* pindex)
{
AssertLockHeld(cs);

View File

@ -385,12 +385,12 @@ public:
*/
void PoSePunish(const uint256& proTxHash, int penalty, bool debugLogs);
void DecreaseScores();
/**
* Decrease penalty score of MN by 1.
* Only allowed on non-banned MNs.
* @param proTxHash
*/
void PoSeDecrease(const uint256& proTxHash);
void PoSeDecrease(const CDeterministicMN& dmn);
[[nodiscard]] CDeterministicMNListDiff BuildDiff(const CDeterministicMNList& to) const;
[[nodiscard]] CDeterministicMNList ApplyDiff(const CBlockIndex* pindex, const CDeterministicMNListDiff& diff) const;
@ -609,7 +609,6 @@ public:
bool BuildNewListFromBlock(const CBlock& block, const CBlockIndex* pindexPrev, BlockValidationState& state, const CCoinsViewCache& view,
CDeterministicMNList& mnListRet, bool debugLogs) EXCLUSIVE_LOCKS_REQUIRED(cs);
static void HandleQuorumCommitment(const llmq::CFinalCommitment& qc, const CBlockIndex* pQuorumBaseBlockIndex, CDeterministicMNList& mnList, bool debugLogs);
static void DecreasePoSePenalties(CDeterministicMNList& mnList);
CDeterministicMNList GetListForBlock(const CBlockIndex* pindex) LOCKS_EXCLUDED(cs) {
LOCK(cs);