feat: implement DIP0026 #5799

panleone · 2024-01-04T16:02:09Z

Implement DIP0026 following the official documentation https://github.com/dashpay/dips/blob/master/dip-0026.md
With this PR it is possible to create a masternode that havs up to 32 payees and it is a first step toward trustless masternode shares

What was done?

In ProTxs the field CScript scriptPayout has been changed to std::vector<PayoutShare> payoutShares; which is a vector of scripts/ rewards shares.
Serialiation and Deserialization of ProTxs has been changed accordingly
Finally I added the regtest parameters to activate DIP0026 with a soft fork

How Has This Been Tested?

I have added unit tests that test creation, consensus and payment of masternodes with more than one payee

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
I have assigned this pull request to a milestone

knst

please check my comments for PR.

That's not a final version of my review, but that's what I have seen at the moment

src/evo/providertx.cpp

src/evo/providertx.h

src/rpc/evo.cpp

src/rpc/masternode.cpp

test/functional/rpc_masternode.py

src/test/evo_trivialvalidation.cpp

src/qt/masternodelist.cpp

knst · 2024-01-06T19:26:12Z

please address these warnings from CI also:

The subject line of commit hash 6dbc490ff109af21ac2611b3b1f4fe4a7e207271 is followed by a non-empty line. Subject lines should always be followed by a blank line.
The subject line of commit hash 9a6f9e9f44d347646a9e47219a532a936ac99e73 is followed by a non-empty line. Subject lines should always be followed by a blank line.
src/evo/deterministicmns.cpp:1711:72: warning: Consider using std::any_of algorithm instead of a raw loop. [useStlAlgorithm]
src/evo/providertx.cpp:26:16: warning: Variable 'payoutShare' can be declared as reference to const [constVariable]
src/evo/providertx.cpp:68:16: warning: Variable 'payoutShare' can be declared as reference to const [constVariable]
src/rpc/evo.cpp:1319:71: warning: Consider using std::any_of algorithm instead of a raw loop. [useStlAlgorithm]
src/rpc/evo.cpp:1395:84: warning: Consider using std::any_of algorithm instead of a raw loop. [useStlAlgorithm]

https://gitlab.com/dashpay/dash/-/jobs/5871076186
they are cause CI tail.

github-actions · 2024-01-07T01:29:07Z

This pull request has conflicts, please rebase.

PastaPastaPasta · 2024-01-07T01:29:16Z

I haven't reviewed this yet; but thank you for your contribution!

Expand CProRegTx and CProUpRegTx according to DIP0026 standard, change serialization and deserialization methods in order to take in account the new field, Add consensus rules specified in the DIP0026 (max 32 payees, sum of payment shares cannot be greater than 10000,...) change dmnstate in order to take in account the new field change consensus rules in such a way that masternodes will pay all the payees add multiple payees in the qt masternode table

In this way any deployment that is activated with DIP0023 will be deployed on regtest after activating spork 24

And improve dynamically_prepare_masternode in such a way to support multiple payees

src/bloom.cpp

src/consensus/params.h

src/deploymentinfo.cpp

src/evo/deterministicmns.cpp

src/evo/dmnstate.cpp

UdjinM6 · 2024-01-08T15:11:21Z

src/llmq/ehf_signals.cpp

@@ -54,8 +55,15 @@ void CEHFSignalsHandler::UpdatedBlockTip(const CBlockIndex* const pindexNew)
        // TODO: v20 will never attempt to create EHF messages on main net; if this is needed it will be done by v20.1 or v21 nodes
        return;
    }
-    // TODO: should do this for all not-yet-signied bits
-    trySignEHFSignal(Params().GetConsensus().vDeployments[Consensus::DEPLOYMENT_MN_RR].bit, pindexNew);
+


e24cb23 feat(consensus): Generalize regtest ehf should be in its own separate PR imo. Thoughts @knst @PastaPastaPasta ?

Unlikely we are going to have any other EHF fork before dip0026 is merged.
But to speed up process review, merging new code and reduce conflicts (dip0026 goes for v21, not v20.x) it's reasonable to split

src/qt/masternodelist.cpp

src/rpc/evo.cpp

UdjinM6 · 2024-01-08T16:16:23Z

src/masternode/payments.cpp

+            if (payeeReward > 0) {
+                voutMasternodePaymentsRet.emplace_back(payeeReward, payoutShare.scriptPayout);
+            }
+        }


There is a set of rules which really makes you want to keep Operator related things separated I think:

only Operator must be able to change his payout script

Operator must not be able to change anyone else's payouts

UdjinM6 · 2024-01-08T16:24:17Z

Looks very solid. Needs some cleanups and small fixes but great job overall @panleone ! 👍

A helper class has been added which has the role to wrap and correctly serialize and deserialize a vector of payoutShare.

knst · 2024-01-10T09:45:42Z

@panleone @PastaPastaPasta @UdjinM6
I've just observed another crucial aspect. With the "mn_rr" hard fork, a portion of the masternodes' rewards will be reallocated between the masternodes directly and the platform reward (approximately 37%). The platform is responsible for managing its share of the reward and distributing it among evo nodes.

Therefore, once the mn_rr is activated (in conjunction with the platform release witch is expected before DIP0026 hard fork), we will require the platform to also support an array of payees.

src/evo/providertx.cpp

panleone · 2024-01-10T10:50:31Z

@panleone @PastaPastaPasta @UdjinM6 I've just observed another crucial aspect. With the "mn_rr" hard fork, a portion of the masternodes' rewards will be reallocated between the masternodes directly and the platform reward (approximately 37%). The platform is responsible for managing its share of the reward and distributing it among evo nodes.

Therefore, once the mn_rr is activated (in conjunction with the platform release witch is expected before DIP0026 hard fork), we will require the platform to also support an array of payees.

Good point, then we must also implement dip0026 on platform. At the moment due to exams I don't have enough free time to do it, but since v21 is still far away it should not be a problem

github-actions · 2024-01-12T03:45:22Z

This pull request has conflicts, please rebase.

changes to isTriviallyValid and if formatting

1821d92 test: add activate_ehf_by_name (Alessandro Rezzi) de38dca feat(consensus): Generalize ehf activation (Alessandro Rezzi) Pull request description: ## Issue being fixed or feature implemented Try to sign/mine any ehf deployment and not only mn_rr. As asked in the review this decouples (and improves) commit [e24cb23](e24cb23) from PR #5799 ## What was done? See commit description ## How Has This Been Tested? ## Breaking Changes ## Checklist: - [x] I have performed a self-review of my own code - [x] 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 Top commit has no ACKs. Tree-SHA512: 7112a5214b473dea29a892563e6598eca089d2e17fdff8bda08c7777738f1054d2cb07e0a7dccf66214911fec64a6441e84100273ac2ed52abe1d33fb960e8cc

knst added this to the 21 milestone Jan 4, 2024

knst reviewed Jan 4, 2024

View reviewed changes

src/qt/masternodelist.cpp Outdated Show resolved Hide resolved

panleone force-pushed the dip0026 branch from 21f8f04 to 95d34ac Compare January 6, 2024 23:54

github-actions bot added the needs rebase label Jan 7, 2024

panleone force-pushed the dip0026 branch from b5abbf0 to 881d568 Compare January 7, 2024 17:54

panleone added 8 commits January 7, 2024 13:15

feat: add DIP0026 to regtest chain params

fc643f6

test: DIP0026 unit tests

094c791

feat(rpc): Update RPC

de32c9d

test: Update existing functional test

c6cb351

feat(consensus): Generalize regtest ehf

e24cb23

In this way any deployment that is activated with DIP0023 will be deployed on regtest after activating spork 24

test: add activate_dip_0026

cce757c

And improve dynamically_prepare_masternode in such a way to support multiple payees

test: add dip0026 functional tests

2f3a1ab

panleone force-pushed the dip0026 branch from 881d568 to 2f3a1ab Compare January 7, 2024 18:35

github-actions bot removed the needs rebase label Jan 7, 2024

UdjinM6 requested changes Jan 8, 2024

View reviewed changes

panleone added 2 commits January 8, 2024 16:43

feat: Serialize and deserialize correctly

c55efd7

A helper class has been added which has the role to wrap and correctly serialize and deserialize a vector of payoutShare.

[squash]: do not initialize payoutShares

a93068e

knst reviewed Jan 10, 2024

View reviewed changes

src/evo/providertx.cpp Outdated Show resolved Hide resolved

github-actions bot added the needs rebase label Jan 12, 2024

panleone added 2 commits January 12, 2024 03:26

[squash]: revert changes to lines I did not touch

4c60ebd

[squash]: use ranges in place of for loop

3ba7618

panleone added 3 commits January 12, 2024 03:26

[squash]: apply review

97be000

changes to isTriviallyValid and if formatting

[squash]: include all payoutShares in ToString() logs

84d9fd6

[squash]: require fee address if tx is multi payee

4b5c4aa

panleone mentioned this pull request Jan 14, 2024

feat: generalize ehf activation #5824

Merged

5 tasks

panleone mentioned this pull request Feb 27, 2024

feat: Extend json structs to support dip0026 multi payees dashpay/rust-dashcore-rpc#42

Open

UdjinM6 removed this from the 21 milestone Jul 26, 2024

coderabbitai bot mentioned this pull request Jan 10, 2025

fix: more compact cli options #6526

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: implement DIP0026 #5799

feat: implement DIP0026 #5799

panleone commented Jan 4, 2024 •

edited by knst

Loading

knst left a comment

knst commented Jan 6, 2024 •

edited

Loading

github-actions bot commented Jan 7, 2024

PastaPastaPasta commented Jan 7, 2024

UdjinM6 Jan 8, 2024

knst Jan 8, 2024

UdjinM6 Jan 8, 2024

UdjinM6 commented Jan 8, 2024

knst commented Jan 10, 2024

panleone commented Jan 10, 2024

github-actions bot commented Jan 12, 2024

feat: implement DIP0026 #5799

Are you sure you want to change the base?

feat: implement DIP0026 #5799

Conversation

panleone commented Jan 4, 2024 • edited by knst Loading

What was done?

How Has This Been Tested?

Checklist:

knst left a comment

Choose a reason for hiding this comment

knst commented Jan 6, 2024 • edited Loading

github-actions bot commented Jan 7, 2024

PastaPastaPasta commented Jan 7, 2024

UdjinM6 Jan 8, 2024

Choose a reason for hiding this comment

knst Jan 8, 2024

Choose a reason for hiding this comment

UdjinM6 Jan 8, 2024

Choose a reason for hiding this comment

UdjinM6 commented Jan 8, 2024

knst commented Jan 10, 2024

panleone commented Jan 10, 2024

github-actions bot commented Jan 12, 2024

panleone commented Jan 4, 2024 •

edited by knst

Loading

knst commented Jan 6, 2024 •

edited

Loading