docs(dip-26): update masternode multi-party payouts#184
docs(dip-26): update masternode multi-party payouts#184PastaPastaPasta wants to merge 12 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR finalizes DIP-26 as “Masternode Multi-Party Payouts,” adds version 4 payout-list semantics to DIP-0003, and updates the README DIP index entry. The DIP-26 document also rewrites its specification, validation, deployment, rationale, test, and security sections. ChangesDIP-26 Multi-Party Payouts Specification
🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PastaPastaPasta
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@dip-pasta-multi-party-payouts.md`:
- Around line 164-180: The external-collateral sign message was changed to start
with <payoutListStr>, but the required DIP-0003 prefix <magicString> was
omitted; restore the original magicString prefix in the external collateral
signing contract so the full sign string is
"<magicString>|<payoutListStr>|<operatorReward>|<ownerKeyAddress>|<votingKeyAddress>|<payloadHash>"
(i.e. prepend magicString wherever the ProRegTx/external-collateral sign string
is constructed/serialized) to preserve the original signature domain and
compatibility with existing verification logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2a1b9a01-111a-4ddc-b0d7-eefda89f8d00
📒 Files selected for processing (1)
dip-pasta-multi-party-payouts.md
| For external collaterals, the collateral ownership sign string must include the | ||
| new payout list representation instead of the previous single payout string. The | ||
| payout list string is produced by concatenating each payout entry in serialized | ||
| order as: | ||
|
|
||
| ```text | ||
| <reward>:<payoutStr> | ||
| ``` | ||
|
|
||
| Entries are joined with commas. `<payoutStr>` is the Dash address corresponding | ||
| to the entry's `scriptPayout` when the script is a standard address script, or | ||
| the hex representation of the script otherwise. The ProRegTx sign string | ||
| therefore becomes: | ||
|
|
||
| ```text | ||
| <payoutListStr>|<operatorReward>|<ownerKeyAddress>|<votingKeyAddress>|<payloadHash> | ||
| ``` |
There was a problem hiding this comment.
Preserve magicString in the external-collateral sign message contract
Line 179 appears to redefine the signed message as starting with <payoutListStr>, but DIP-0003’s signed message format includes a required <magicString> prefix. Dropping that prefix would change the signature domain and break compatibility with existing signing/verification behavior for external collateral proofs.
Suggested wording update
-<payoutListStr>|<operatorReward>|<ownerKeyAddress>|<votingKeyAddress>|<payloadHash>
+<magicString><payoutListStr>|<operatorReward>|<ownerKeyAddress>|<votingKeyAddress>|<payloadHash>-The ProRegTx sign string
-therefore becomes:
+The ProRegTx sign message keeps the existing `magicString` prefix from DIP-0003;
+only `<payoutStr>` is replaced with `<payoutListStr>`. The sign string therefore becomes:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| For external collaterals, the collateral ownership sign string must include the | |
| new payout list representation instead of the previous single payout string. The | |
| payout list string is produced by concatenating each payout entry in serialized | |
| order as: | |
| ```text | |
| <reward>:<payoutStr> | |
| ``` | |
| Entries are joined with commas. `<payoutStr>` is the Dash address corresponding | |
| to the entry's `scriptPayout` when the script is a standard address script, or | |
| the hex representation of the script otherwise. The ProRegTx sign string | |
| therefore becomes: | |
| ```text | |
| <payoutListStr>|<operatorReward>|<ownerKeyAddress>|<votingKeyAddress>|<payloadHash> | |
| ``` | |
| For external collaterals, the collateral ownership sign string must include the | |
| new payout list representation instead of the previous single payout string. The | |
| payout list string is produced by concatenating each payout entry in serialized | |
| order as: | |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@dip-pasta-multi-party-payouts.md` around lines 164 - 180, The
external-collateral sign message was changed to start with <payoutListStr>, but
the required DIP-0003 prefix <magicString> was omitted; restore the original
magicString prefix in the external collateral signing contract so the full sign
string is
"<magicString>|<payoutListStr>|<operatorReward>|<ownerKeyAddress>|<votingKeyAddress>|<payloadHash>"
(i.e. prepend magicString wherever the ProRegTx/external-collateral sign string
is constructed/serialized) to preserve the original signature domain and
compatibility with existing verification logic.
Move the Masternode Multi-Party Payouts spec into dip-0026.md (assigning DIP number 0026) and remove the dip-pasta-multi-party-payouts.md alias, replacing the older DIP-26 draft. Update the README row (title, author) and renumber the reward-payment steps so the interleaved code fences render as one sequential list. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace the duplicated full ProRegTx/ProUpRegTx field tables with the payout delta only (scriptPayout -> payouts), referencing DIP-0003 for all unchanged fields. Drop the unfounded version-3 addressing lineage and the netInfo/platformNetInfo redefinition; keep version 4 and state that the BLS scheme and addressing fields are unchanged from DIP-0003. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Remove the RPC and JSON Interfaces section (protx info/diff/listdiff, masternode winners/payments, getblocktemplate, payoutAddress, SML JSON) and its two RPC-only test cases - reference-client surface, not protocol/consensus; the SML (DIP-0004) carries no payout data. Keep Special Transaction Filtering (P2P/SPV-level). Also drop DIP-0004 from Prior Work since it is no longer referenced anywhere in the spec. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Remove the unreachable hex-fallback for <payoutStr>: validation already restricts payout scripts to P2PKH/P2SH, both of which have a Dash address, so the fallback can never trigger (matching DIP-0003, which has none). Show the <magicString> prefix inline in the sign string and defer to DIP-0003 for its definition, consistent with how DIP-0003 presents it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Place scriptPayout before reward in MasternodePayoutShare and use <payoutStr>:<reward> in the sign string, matching the payout-then-reward convention of DIP-0003 and the previous DIP-26 draft. Keeps the unambiguous : / , / | delimiter scheme. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
PastaPastaPasta
changed the title
| DIP stands for Dash Improvement Proposal. Similar to Bitcoin's [BIPs](https://github.com/bitcoin/bips/), a DIP is a design document providing information to the Dash community, or describing a new feature for Dash or its processes or environment. The DIP should provide a concise technical specification of the feature and a rationale for the feature. | ||
|
|
||
| Because Dash is forked from the Bitcoin codebase, many of the BIPs can be applied to Dash as well (a list of the BIPs updated to include Dash-specific details can be found [here](https://github.com/dashevo/bips)). The purpose of the DIPs is not to duplicate those which exist as BIPs, but to introduce protocol upgrades or feature specifications which are unique to Dash. | ||
| Because Dash is forked from the Bitcoin codebase, many of the BIPs can be applied to Dash as well (a list of the BIPs updated to include Dash-specific details can be found in the [Dash BIPs repository](https://github.com/dashevo/bips)). The purpose of the DIPs is not to duplicate those which exist as BIPs, but to introduce protocol upgrades or feature specifications which are unique to Dash. |
|
|
thephez
left a comment
thephez
left a comment
There was a problem hiding this comment.
Forgot to mention on the last review, but DIP-3 likely needs at least some edits or it's going to be incorrect.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Line 5: The generic link text "[here]" violates Markdown linting rule MD059
which requires descriptive link text. Replace the generic "[here]" link text
with descriptive text that explains what the link contains, such as
"Dash-specific BIPs" or "list of Dash-specific BIPs", to make the link more
scannable and meaningful in the sentence context.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d7466733-dff8-44f1-85f4-1f0c7c9b402a
📒 Files selected for processing (3)
README.mddip-0003.mddip-0026.md
🚧 Files skipped from review as they are similar to previous changes (1)
- dip-0026.md
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Line 5: The generic link text "[here]" violates Markdown linting rule MD059
which requires descriptive link text. Replace the generic "[here]" link text
with descriptive text that explains what the link contains, such as
"Dash-specific BIPs" or "list of Dash-specific BIPs", to make the link more
scannable and meaningful in the sentence context.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d7466733-dff8-44f1-85f4-1f0c7c9b402a
📒 Files selected for processing (3)
README.mddip-0003.mddip-0026.md
🚧 Files skipped from review as they are similar to previous changes (1)
- dip-0026.md
🛑 Comments failed to post (1)
README.md (1)
5-5:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReplace the generic link text.
[here]triggers MD059 and makes the sentence harder to scan. Use descriptive link text instead.Suggested fix
- Because Dash is forked from the Bitcoin codebase, many of the BIPs can be applied to Dash as well (a list of the BIPs updated to include Dash-specific details can be found [here](https://github.com/dashevo/bips)). + Because Dash is forked from the Bitcoin codebase, many of the BIPs can be applied to Dash as well (a list of the BIPs updated to include Dash-specific details can be found in the [Dash-specific BIP list](https://github.com/dashevo/bips)).As per coding guidelines, Markdown linting is enforced on changed
.mdfiles and MD059 requires descriptive link text.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.Because Dash is forked from the Bitcoin codebase, many of the BIPs can be applied to Dash as well (a list of the BIPs updated to include Dash-specific details can be found in the [Dash-specific BIP list](https://github.com/dashevo/bips)). The purpose of the DIPs is not to duplicate those which exist as BIPs, but to introduce protocol upgrades or feature specifications which are unique to Dash.🧰 Tools
🪛 GitHub Actions: .github/workflows/markdownlint.yml / 0_lint.txt
[error] 5-5: markdownlint-cli2 (markdownlint v0.38.0) reported MD059/descriptive-link-text: Link text should be descriptive [Context: "[here]"]
🪛 GitHub Actions: .github/workflows/markdownlint.yml / lint
[error] 5-5: markdownlint-cli2: MD059/descriptive-link-text Link text should be descriptive [Context: "[here]"]
🪛 GitHub Check: lint
[failure] 5-5: Link text should be descriptive
README.md:5:174 MD059/descriptive-link-text Link text should be descriptive [Context: "[here]"] https://github.com/DavidAnson/markdownlint/blob/v0.38.0/doc/md059.md🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` at line 5, The generic link text "[here]" violates Markdown linting rule MD059 which requires descriptive link text. Replace the generic "[here]" link text with descriptive text that explains what the link contains, such as "Dash-specific BIPs" or "list of Dash-specific BIPs", to make the link more scannable and meaningful in the sentence context.Sources: Coding guidelines, Linters/SAST tools
There was a problem hiding this comment.
Please match the formatting style of the existing DIP. It does not do line wrapping.
Unwrap newly added prose paragraphs and revert table-column-width normalization on tables that gained no content. Only the two version tables that add the version 4 row keep widened columns, and they use the existing DIP's no-space pipe-separator style.
4 participants
Summary
Testing
npx markdownlint-cli2 dip-0026.md README.md --config .markdownlint.json.This pull request was created by Codex.
Summary by CodeRabbit
#26“Masternode Multi-Party Payouts” specification with the new Version 4 payout-list model, including payout validation, deterministic reward splitting, and Dash Core v24 activation/compatibility notes.#26title and structure.#3ProRegTx/ProUpRegTx “different versions” and validation guidance to cover Version 4, aligning payout-list validation semantics accordingly.