feat: implement DIP0026

[panleone]

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

[knst]

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]

This pull request has conflicts, please rebase.

[PastaPastaPasta]

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

[UdjinM6]

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

[knst]

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

[UdjinM6]

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]

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

[knst]

@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.

[panleone]

@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]

This pull request has conflicts, please rebase.

[DashCoreAutoGuix]

ℹ️ Backport Verification - Not Applicable

This PR implements DIP0026 (masternode payout shares), which is a Dash-specific feature, not a Bitcoin backport.

Original Bitcoin commit: N/A - This is a Dash-native implementation
Reviewed commit hash: 4b5c4aaac1-verify-1753723627

Why verification was skipped:

• This PR introduces new Dash functionality (DIP0026)
• No corresponding Bitcoin changes to verify against
• This is legitimate Dash development, not a backport

Recommendation: This PR should be reviewed through the standard Dash development process, not the backport verification system.

---

Note: The backport verification system only applies to PRs that port changes from Bitcoin Core to Dash Core.

[thepastaclaw]

Review Gate

Commit: 4b5c4aaa

• ✅ Debounce: 1235642m ago (need 30m)

• ✅ CI checks: no CI checks found

• ⏳ CodeRabbit review: no comment yet

• ✅ Off-peak hours: off-peak (03:33 AM PT Tuesday)

• Run review now (check to override)

[thepastaclaw]

Code Review

Found four blocking consensus/serialization issues in the DIP-0026 multi-payee implementation: (1) uint16_t accumulator in payout-share validation wraps, letting forged totals pass; (2) CDeterministicMNStateDiff deserializes payoutShares before nVersion, corrupting the stream for v3 diffs; (3) ProUpRegTx updates leave nVersion stale unless the operator key changes, silently truncating multi-payee data on subsequent serialization; (4) duplicate identical payout shares can underpay masternodes because the payment matcher uses set-membership semantics. Several lower-severity issues around input validation and migration fragility are also flagged.

Reviewed commit: 4b5c4aa

🔴 4 blocking | 🟡 2 suggestion(s) | 💬 3 nitpick(s)

blocking: uint16_t totalPayoutReward wraps; sum != 10000 can pass validation

src/evo/providertx.cpp (lines 15-41)

totalPayoutReward is uint16_t (max 65535), but with up to 32 payout shares each capped at 10000, the legitimate maximum sum is 320000. Arithmetic on uint16_t wraps modulo 65536, so an attacker can craft entries whose real sum is e.g. 75536 (= 65536 + 10000) — for instance, seven entries of 10000 plus one of 5536, each individually <= 10000 and the count <= 32 — and the accumulator settles at 10000, making totalPayoutReward != 10000 evaluate false. The tx passes TriviallyVerifyProRegPayees even though the shares total >100%. Downstream, GetBlockTxOuts in src/masternode/payments.cpp:63-67 distributes masternodeReward * payoutShareReward / 10000 per share, so the masternode would consume ~7.5x its share of the block reward and FillBlockPayments cannot produce a balanced coinbase — a chain-halt DoS vector that survives validation. Also note the order bug at line 32-35: the accumulator is incremented before the per-share > 10000 check, so the bound check should happen first. Existing tests only exercise sum > 10000 without overflow, so the wrap path is uncovered.

blocking: CDeterministicMNStateDiff reads payoutShares before nVersion, corrupting v3 diffs

src/evo/dmnstate.h (lines 314-379)

DMN_STATE_DIFF_ALL_FIELDS lists payoutShares (line 327) before nVersion (line 333). On SER_READ the macro iterates in declaration order: when payoutShares is processed, obj.state.nVersion is still the default LEGACY_BLS_VERSION (1), so PayoutSharesSerializerWrapper(..., obj.state.nVersion < MULTI_PAYOUT_VERSION) enters the single-payee branch and reads only one CScript. But if the diff was written with nVersion == MULTI_PAYOUT_VERSION (which the diff constructor at line 347 forces whenever Field_payoutShares is set), the stream actually encodes vector<PayoutShare> (count varint + repeated PayoutShare records each carrying an extra uint16_t). The wrong number of bytes are consumed and every field after — scriptOperatorPayout, nConsecutivePayments, platformNodeID, ports, finally nVersion itself — is decoded from the wrong offset. This produces silent state corruption as soon as any DB-loaded or wire-transmitted diff carries a DIP-0026 payout-share change. The pubKeyOperator precedent (with SetLegacy(...) after the fact) works only because BLS public keys are fixed-length across schemes; the same trick cannot fix payoutShares since the legacy and v3 encodings have different lengths. Fix: serialize nVersion first (write-format change) or special-case reading nVersion before payoutShares/pubKeyOperator. No unit test currently round-trips a CDeterministicMNStateDiff with multi-payout shares.

blocking: ProUpRegTx payout-share update does not propagate state nVersion

src/evo/deterministicmns.cpp (lines 870-880)

newState->nVersion is only assigned inside the if (newState->pubKeyOperator != proTx.pubKeyOperator) branch. When a registrar update keeps the operator key but changes payoutShares (e.g. switches a single-payee MN to multi-payee, or vice versa), newState->nVersion remains the previous DMN state version while newState->payoutShares is overwritten with the new vector. The CDeterministicMNState serializer at src/evo/dmnstate.h:239 branches on nVersion < MULTI_PAYOUT_VERSION; for a stale version this takes the legacy single-payee path and silently drops every payee after payoutShares[0]. The corruption appears in-memory only after the next DB round-trip or state diff, so block validation accepts the ProUpRegTx but the resulting state is wrong. Set newState->nVersion = proTx.nVersion; unconditionally before applying the rest of the update.

blocking: Duplicate payout shares can bypass masternode-payment enforcement

src/evo/providertx.cpp (lines 15-41)

TriviallyVerifyProRegPayees checks count, script type, per-share cap and total, but never rejects duplicate payout entries. A masternode can register two PayoutShares with identical scriptPayout and identical payoutShareReward. GetBlockTxOuts (src/masternode/payments.cpp:63) emits two identical CTxOuts, but the validator at src/masternode/payments.cpp:117-126 uses ranges::any_of(txNew.vout, ...) — set-membership semantics that do not consume matches. A malicious miner can include only one of the two identical outputs; both expected entries match the same actual output, the block passes validation, and the masternode is underpaid (the miner pockets the difference). This was not reachable pre-DIP-0026 because there was only one MN payee. Reject duplicates in TriviallyVerifyProRegPayees (e.g. dedup by (scriptPayout) — same script implies same destination — or fix the matcher to consume outputs as it pairs them).

suggestion: RPC silently wraps payoutShareReward via int -> uint16_t conversion

src/rpc/evo.cpp (lines 549-573)

payoutShares[i].get_array()[1].get_int() returns a signed int, then is passed to PayoutShare(CScript, uint16_t) where the implicit narrowing wraps modulo 65536. Out-of-range inputs (e.g. -1, 70000) become 65535, 4464, and so on. The downstream > 10000 validation in TriviallyVerifyProRegPayees catches some of these post-wrap, but the user sees an error referencing a value they never entered, and combined with the uint16_t accumulator overflow above, malformed RPC input can reach unintended states. Validate the int explicitly before constructing the PayoutShare.

suggestion: Zero-value payoutShareReward is accepted

src/evo/providertx.cpp (lines 26-36)

DIP-0026 specifies share values in [1, 10000]; the validator only enforces the upper bound (payoutShare.payoutShareReward > 10000). A payee with share == 0 is accepted: it contributes nothing to the sum, produces no output in GetBlockTxOuts (which skips payeeReward == 0), but inflates the on-chain payee vector, wasting storage/bandwidth and creating a fingerprinting surface. The functional test helper dynamically_prepare_masternode (test/functional/test_framework/test_framework.py:1212) already uses random.randint(0, ...) for shares, so 0 is reachable through normal flows. Reject 0 as well.

nitpick: PayoutShare default constructor leaves payoutShareReward = 0 while explicit ctor defaults to 10000

src/evo/providertx.h (lines 24-44)

PayoutShare() (the implicit default) uses the in-class initializer at line 28 and leaves payoutShareReward = 0. The explicit PayoutShare(scriptPayout) ctor defaults payoutShareReward = 10000. The legacy deserialization path (PayoutSharesSerializerWrapper at line 78) and the legacy-migration constructors in dmnstate.h:192,209 rely on the 10000 default to give the legacy sole payee 100% of the reward. New code that uses PayoutShare{} and forgets to set the share will silently produce 0 and trip bad-protx-payee-reward-sum. Make the two defaults consistent (drop the in-class {0} or have both ctors use the same canonical value).

nitpick: Legacy DMNState migration constructs PayoutShare relying on implicit default reward

src/evo/dmnstate.h (lines 179-213)

Both migration constructors use payoutShares({PayoutShare(s.scriptPayout)}), which depends on the explicit ctor's default argument (10000) to assign 100% to the sole legacy payee. This is correct today but fragile: if the explicit ctor's default ever changes, every historical record loaded from the old DB formats would silently start paying its sole payee a different share. Pass the value explicitly: PayoutShare(s.scriptPayout, 10000).

nitpick: protx_revoke dereferences payoutShares[0] without a non-empty guard

src/rpc/evo.cpp (lines 1228-1238)

dmn->pdmnState->payoutShares[0].scriptPayout != CScript() indexes element 0 before any size check. In normal operation a registered MN must have at least one payout share, but adding a defensive !payoutShares.empty() predicate (mirroring the pattern used elsewhere) would be safer; the existing if (payoutShares.size() > 1) immediately below is unreachable if the vector were ever empty due to corruption.

_Inline posting via scripts/review_poster.py failed, so I posted the verified findings in the review body instead: Traceback (most recent call last):
File "/Users/claw/.openclaw/workspace/scripts/review_poster.py", line 138, in
result = post_review(repo, pr_number, head_sha, verified, dry_run=dry_run)
File "/Users/claw/.openclaw/workspace/scripts/review_poster.py", line 107, in post_review
result = _gh(
File "/Users/claw/.openclaw/workspace/scripts/review_poster.py", line 23, in gh
raise RuntimeError(f"gh {' '.join(args)} failed: {result.stderr.strip()}")
RuntimeError: gh api /repos/dashpay/dash/pulls/5799/reviews --method POST --input - failed: gh: Unprocessable Entity (HTTP 422)