feat: masternode payment reallocation from coin base to platform #5342

knst · 2023-04-21T15:51:02Z

Issue being fixed or feature implemented

It resolves this issue: https://github.com/dashpay/dash-issues/issues/38

What was done?

Move some funds from the coinbase, into the Asset Lock Pool. 37.5% of current masternode reward are moved to platform.
beside that, evo MN will receive reward only once - not quad reward (4 time continously) as in v19.

This is to incentivize MNs to upgrade to platform, because only MNs running platform will get these migrated rewards

This PR depends on that should be merged before:

How Has This Been Tested?

There were added couple new tests in feature_asset_lock.py that test functionality of credit_pool.py

To avoid conflicting with other tests, such as evo_deterministicmns_tests that is testing old behavior, activation height is chosen as far in future (3000 blocks) and not achieved by most of tests, include feature_asset_lock.py before all other functionality such as asset-unlocks are tested.

Breaking Changes

There are breaking changes, because it change consensus rules: changed CbTx output accordingly

Checklist:

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
need to update some DIP accordingly to that changes
I have assigned this pull request to a milestone

github-actions · 2023-04-26T04:47:03Z

This pull request has conflicts, please rebase.

github-actions · 2023-05-09T03:36:52Z

This pull request has conflicts, please rebase.

github-actions · 2023-05-17T11:13:56Z

This pull request has conflicts, please rebase.

PastaPastaPasta · 2023-08-30T02:49:27Z

src/evo/creditpool.cpp

    }
+    masternodeReward = 0.375 * GetMasternodePayment(cbTx.nHeight, blockReward, params.BRRHeight);


can we get rid of this instance of the 0.375 as well?

thanks for catch, completely missed this one. Please, check new commits

UdjinM6

re-utACK

ogabrielides

LGTM

test/functional/feature_asset_locks.py

not sure about operator reward share

UdjinM6 · 2023-09-01T10:53:01Z

It feels like with current implementation MN owners are "stealing" 37.5% from MN operators' share. EDIT: I guess it's inevitable cause otherwise you could trick the system by paying 100% to operator and 0 to platform.

knst · 2023-09-04T10:31:37Z

It feels like with current implementation MN owners are "stealing" 37.5% from MN operators' share. EDIT: I guess it's inevitable cause otherwise you could trick the system by paying 100% to operator and 0 to platform.

yes, platform reward is substracted before operator's reward calculation.
What if I will update calculation of operator reward like that? In this case it would be affected (stolen) operator's share only in extreme cases (such as 66%+ of operator reward) @UdjinM6

thephez · 2023-09-04T12:56:35Z

It feels like with current implementation MN owners are "stealing" 37.5% from MN operators' share. EDIT: I guess it's inevitable cause otherwise you could trick the system by paying 100% to operator and 0 to platform.

yes, platform reward is substracted before operator's reward calculation. What if I will update calculation of operator reward like that? In this case it would be affected (stolen) operator's share only in extreme cases (such as 66%+ of operator reward) @UdjinM6

Not sure I'm following the discussion correctly, but my understanding is that operators will be paid on platform side as well (just like owners) - see dashpay/platform#320. So if the concern here is that operators will miss out due to the partial movement of the block subsidy to platform, that shouldn't be an issue. We can confirm with platform team if necessary. Or, if I entirely misread the discussion, just ignore my comment 😄

UdjinM6 · 2023-09-04T14:58:16Z

It feels like with current implementation MN owners are "stealing" 37.5% from MN operators' share. EDIT: I guess it's inevitable cause otherwise you could trick the system by paying 100% to operator and 0 to platform.

yes, platform reward is substracted before operator's reward calculation. What if I will update calculation of operator reward like that? In this case it would be affected (stolen) operator's share only in extreme cases (such as 66%+ of operator reward) @UdjinM6

Not sure I'm following the discussion correctly, but my understanding is that operators will be paid on platform side as well (just like owners) - see dashpay/platform#320. So if the concern here is that operators will miss out due to the partial movement of the block subsidy to platform, that shouldn't be an issue. We can confirm with platform team if necessary. Or, if I entirely misread the discussion, just ignore my comment 😄

Oh, wow! I didn't know that. Would be nice if someone from the Platform team could confirm that. @shumkov or maybe @QuantumExplorer?

shumkov · 2023-09-04T15:00:04Z

Yes, operators will be paid on platform side as well

UdjinM6 · 2023-09-04T15:01:44Z

Yes, operators will be paid on platform side as well

Cool, thanks!

@knst I guess we can safely drop 286128d then.

UdjinM6

LGTM, utACK

ogabrielides

utACK

PastaPastaPasta

LGTM just need release notes

ogabrielides

LGTM

PastaPastaPasta · 2023-09-05T16:25:23Z

I tried to comment "utACK for merge via merge commit"

approved

knst force-pushed the reward-reallocation branch from 68204d7 to bc086b9 Compare April 24, 2023 08:49

knst added this to the 20 milestone Apr 24, 2023

knst added the Needs Release Notes This PR includes breaking changes for which release notes have not yet been merged. label Apr 24, 2023

knst marked this pull request as ready for review April 24, 2023 08:54

knst marked this pull request as draft April 24, 2023 20:05

knst force-pushed the reward-reallocation branch 2 times, most recently from 9569bb9 to d6dd942 Compare April 25, 2023 09:01

github-actions bot added the needs rebase label Apr 26, 2023

knst force-pushed the reward-reallocation branch from d6dd942 to 5ea6da9 Compare April 26, 2023 07:35

github-actions bot removed the needs rebase label Apr 26, 2023

github-actions bot added the needs rebase label May 9, 2023

knst force-pushed the reward-reallocation branch from 5ea6da9 to d2137f9 Compare May 12, 2023 07:26

github-actions bot added needs rebase and removed needs rebase labels May 12, 2023

knst force-pushed the reward-reallocation branch from d2137f9 to 5657a78 Compare June 19, 2023 15:39

github-actions bot removed the needs rebase label Jun 19, 2023

knst marked this pull request as ready for review June 19, 2023 15:45

PastaPastaPasta requested a review from UdjinM6 July 4, 2023 17:31

This comment was marked as resolved.

Sign in to view

knst marked this pull request as draft July 5, 2023 15:25

knst force-pushed the reward-reallocation branch 3 times, most recently from 5aba960 to 41859d6 Compare July 12, 2023 21:20

knst mentioned this pull request Jul 17, 2023

refactor: hide some payments methods, replace a class to namespace #5494

Merged

5 tasks

knst force-pushed the reward-reallocation branch from 41859d6 to 539b1e5 Compare July 17, 2023 20:07

knst marked this pull request as ready for review July 17, 2023 20:07

knst requested a review from ogabrielides July 17, 2023 20:07

PastaPastaPasta reviewed Aug 30, 2023

View reviewed changes

knst dismissed UdjinM6’s stale review via fa47b79 August 31, 2023 19:23

knst force-pushed the reward-reallocation branch 2 times, most recently from 4d957ff to 9700eb9 Compare August 31, 2023 19:25

UdjinM6 previously approved these changes Sep 1, 2023

View reviewed changes

knst requested a review from PastaPastaPasta September 1, 2023 08:03

ogabrielides previously approved these changes Sep 1, 2023

View reviewed changes

ogabrielides reviewed Sep 1, 2023

View reviewed changes

test/functional/feature_asset_locks.py Outdated Show resolved Hide resolved

knst dismissed ogabrielides’s stale review via 286128d September 4, 2023 10:30

knst force-pushed the reward-reallocation branch from 286128d to 7ff210a Compare September 4, 2023 15:09

knst requested a review from UdjinM6 September 4, 2023 15:23

UdjinM6 previously approved these changes Sep 4, 2023

View reviewed changes

ogabrielides previously approved these changes Sep 4, 2023

View reviewed changes

PastaPastaPasta previously requested changes Sep 5, 2023

View reviewed changes

PastaPastaPasta dismissed stale reviews from ogabrielides and UdjinM6 via f04032c September 5, 2023 16:19

ogabrielides approved these changes Sep 5, 2023

View reviewed changes

PastaPastaPasta closed this Sep 5, 2023

PastaPastaPasta force-pushed the reward-reallocation branch from f04032c to 759a69e Compare September 5, 2023 16:42

PastaPastaPasta added a commit that referenced this pull request Sep 5, 2023

Merge pull request #5342 from knst/reward-reallocation

bc6360f

PastaPastaPasta removed the Needs Release Notes This PR includes breaking changes for which release notes have not yet been merged. label Sep 9, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: masternode payment reallocation from coin base to platform #5342

feat: masternode payment reallocation from coin base to platform #5342

knst commented Apr 21, 2023 •

edited

github-actions bot commented Apr 26, 2023

github-actions bot commented May 9, 2023

github-actions bot commented May 17, 2023

This comment was marked as resolved.

PastaPastaPasta Aug 30, 2023

knst Sep 1, 2023

UdjinM6 left a comment

ogabrielides left a comment

UdjinM6 commented Sep 1, 2023 •

edited

knst commented Sep 4, 2023

thephez commented Sep 4, 2023 •

edited

UdjinM6 commented Sep 4, 2023

shumkov commented Sep 4, 2023

UdjinM6 commented Sep 4, 2023

UdjinM6 left a comment

ogabrielides left a comment

PastaPastaPasta left a comment

ogabrielides left a comment

PastaPastaPasta commented Sep 5, 2023

		}
		masternodeReward = 0.375 * GetMasternodePayment(cbTx.nHeight, blockReward, params.BRRHeight);

feat: masternode payment reallocation from coin base to platform #5342

feat: masternode payment reallocation from coin base to platform #5342

Conversation

knst commented Apr 21, 2023 • edited

Issue being fixed or feature implemented

What was done?

How Has This Been Tested?

Breaking Changes

Checklist:

github-actions bot commented Apr 26, 2023

github-actions bot commented May 9, 2023

github-actions bot commented May 17, 2023

This comment was marked as resolved.

PastaPastaPasta Aug 30, 2023

Choose a reason for hiding this comment

knst Sep 1, 2023

Choose a reason for hiding this comment

UdjinM6 left a comment

Choose a reason for hiding this comment

ogabrielides left a comment

Choose a reason for hiding this comment

UdjinM6 commented Sep 1, 2023 • edited

knst commented Sep 4, 2023

thephez commented Sep 4, 2023 • edited

UdjinM6 commented Sep 4, 2023

shumkov commented Sep 4, 2023

UdjinM6 commented Sep 4, 2023

UdjinM6 left a comment

Choose a reason for hiding this comment

ogabrielides left a comment

Choose a reason for hiding this comment

PastaPastaPasta left a comment

Choose a reason for hiding this comment

ogabrielides left a comment

Choose a reason for hiding this comment

PastaPastaPasta commented Sep 5, 2023

knst commented Apr 21, 2023 •

edited

UdjinM6 commented Sep 1, 2023 •

edited

thephez commented Sep 4, 2023 •

edited