docs(dip-26): update masternode multi-party payouts#184
docs(dip-26): update masternode multi-party payouts#184PastaPastaPasta wants to merge 13 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
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 (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 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
Estimated code review effort: 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
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
|
|
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
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.
thephez
left a comment
thephez
left a comment
There was a problem hiding this comment.
A couple more questions and suggestions
| payout entries, each with a basis-point reward share. A masternode with one | ||
| owner uses the same structure as a masternode with multiple owners: one payout | ||
| entry with a reward value of 10000. |
There was a problem hiding this comment.
This last sentence is slightly confusing and I'm not sure it's necessary anyway. The point is that all payouts are defined the same whether there's one or multiple payout scripts. The constraint is that the sum of all payout entries must equal 10000. Suggest clarifying the sentence or dropping.
| | payoutShares | payoutShare[] | 1-32 | A set of `payoutShare` items | | ||
| | scriptPayoutSize | compactSize uint | 1-9 | Size of the payout script. | | ||
| | scriptPayout | Script | Variable | Owner payout script. | | ||
| | reward | uint16_t | 2 | Basis-point share of the owner reward. | |
There was a problem hiding this comment.
Per validation rules, it looks like 100 is the minimum allowed reward value for an entry. Probably would be helpful to include here.
| | reward | uint16_t | 2 | Basis-point share of the owner reward. | | |
| | reward | uint16_t | 2 | Basis-point share of the owner reward (minimum: 100). | |
| After v24 activation: | ||
|
|
||
| * New version 4 registrations may use 1 to 8 owner payout entries. | ||
| * Version 4 registrar updates may change the payout list. |
There was a problem hiding this comment.
Meaning a legacy MN can send a v4 update to migrate to use a payout list?
There was a problem hiding this comment.
This applies to masternodes of all types (regular or evonode), correct?
|
Opened a follow-up PR against the source branch for these comments: PastaPastaPasta#4 It covers the v3 reserved row, abstract/scope wording, minimum reward note, legacy migration wording, and regular MN/evonode scope clarification. |
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 a Version 4 payout-list model, including payout-list validation, deterministic reward splitting rules, updated masternode payout construction/verification expectations, and revised Dash Core v24 activation/compatibility guidance.#26title and structure.#3ProRegTx/ProUpRegTx “different versions” and validation guidance to cover Version 4, aligning payout validation with the DIP#26payout-list model.