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

Remove irrelevant height branching after FCC #1324

Closed
wants to merge 7 commits into from
Closed

Remove irrelevant height branching after FCC #1324

Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7c4a044
Improve mocknet simulation params
prasannavl Jun 3, 2022
50b1c5d
Move additional params only on mocknet guard
prasannavl Jun 3, 2022
e25fab5
Remove unnecessary condition beyond FCC
prasannavl Jun 3, 2022
79a38f1
Restore attrs check
Bushstar Jun 3, 2022
b7de2be
Merge branch 'master' into pvl/remove-cond
prasannavl Jun 3, 2022
9d36793
Merge branch 'master' into pvl/remove-cond
prasannavl Jun 22, 2022
758a445
Merge branch 'master' into pvl/remove-cond
prasannavl Aug 3, 2022
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
10 changes: 2 additions & 8 deletions src/masternodes/mn_checks.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1547,11 +1547,7 @@ class CCustomTxApplyVisitor : public CCustomTxVisitor
CDataStructureV0 liveKey{AttributeTypes::Live, ParamIDs::Economy, EconomyKeys::DFIP2203Current};
auto balances = attributes->GetValue(liveKey, CBalances{});

// Can be removed after the hard fork, since it will be backward compatible
// but have to keep it around for pre 2.8.0 nodes for now
if (height >= static_cast<uint32_t>(consensus.FortCanningCrunchHeight)) {
CalculateOwnerRewards(obj.owner);
}
CalculateOwnerRewards(obj.owner);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to remain guarded. Otherwise full sync nodes will end up with some accounts that performed a FutureSwap before FCC having a higher balance than others which could lead to consensus issues.

Copy link
Member Author

@prasannavl prasannavl Aug 3, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But each action that does require a balance in consensus (except for this bug), should already be expected to do
CalculateOwnerRewards before such an action anyway?

Is there anything else I'm missing - otherwise, let's just do a confirmation anyway with full a sync and proceed to remove?


if (obj.withdraw) {
std::map<CFuturesUserKey, CFuturesUserValue> userFuturesValues;
Expand Down Expand Up @@ -1710,9 +1706,7 @@ class CCustomTxApplyVisitor : public CCustomTxVisitor
return Res::Err("%s: %s", obj.govVar->GetName(), "Cannot set via setgovheight.");
}

// Validate GovVariables before storing
// TODO remove GW check after fork height. No conflict expected as attrs should not been set by height before.
if (height >= uint32_t(consensus.FortCanningCrunchHeight) && obj.govVar->GetName() == "ATTRIBUTES") {
if (obj.govVar->GetName() == "ATTRIBUTES") {

auto govVar = mnview.GetAttributes();
if (!govVar) {
Expand Down