docs: add Decentralized Masternode Shares DIP#187
Open
PastaPastaPasta wants to merge 2 commits into
Open
Conversation
Pre-DIP (alias dip-pasta-shared-masternode-collateral per BIP2 naming) specifying trustless shared masternode collateral: 2-8 participants fund one masternode atomically, consensus splits owner rewards by contribution, and a covenant-enforced dissolution transaction (ProDisTx) is the only way collateral can move. Builds on DIP-0003 and extends DIP-0026 provider transaction version 4. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
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 due to trivial changes (1)
📝 WalkthroughWalkthroughThis PR adds a DIP defining decentralized shared masternode collateral for 2–8 participants, including shared registration, reward splitting, dissolution, lifecycle updates, consensus validation, activation behavior, tests, and security considerations. It also updates the spell-checker word list. ChangesShared Masternode Collateral DIP
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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-shared-masternode-collateral.md`:
- Around line 286-291: The monotonicity note in the ProDisTx/requiredPenalty
description is too strong for the zero-penalty case; rewrite the sentence so
validity is defined by paying at least requiredPenalty, and say a unilateral
ProDisTx is invalid whenever it pays less than requiredPenalty rather than only
“until the early period ends.” Update the surrounding explanation in the section
that discusses requiredPenalty, earlyPenalty, and ProDisTx validity to preserve
the monotonicity claim without excluding earlyPenalty = 0.
- Around line 292-307: The ProDisTx validity rules in the actor/output section
currently allow the actor refund output to be optional whenever it is non-zero,
which can let a valid dissolution omit the actor’s remaining principal. Update
the wording around the `actorIndex`, `shares[a].refundScript`, and
`requiredPenalty` rules to make the actor output mandatory whenever
`shares[a].amount - P > 0`, and only omit it in the genuinely zero-value case;
keep the rest of the output-order and no-extra-outputs constraints unchanged.
🪄 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: 5f33eb94-07ee-4903-8e91-242fb4c1f4e8
📒 Files selected for processing (2)
dip-pasta-shared-masternode-collateral.mdproject-words.txt
Comment on lines
+286
to
+291
| All output rules are **minimum-based**: paying more penalty than required is | ||
| always valid. Because `requiredPenalty` is non-increasing, a ProDisTx that is | ||
| valid at height `h` is valid at every height `h' >= h`: no dissolution is | ||
| silently invalidated by chain progress. The converse is intentionally not true: | ||
| a unilateral ProDisTx paying no penalty is invalid until the early period ends. | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Rewrite the monotonicity note to account for zero penalties.
The current sentence is false when earlyPenalty = 0, which the spec explicitly allows. Say “invalid whenever it pays less than requiredPenalty” instead of tying validity to the end of the early period.
🤖 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-shared-masternode-collateral.md` around lines 286 - 291, The
monotonicity note in the ProDisTx/requiredPenalty description is too strong for
the zero-penalty case; rewrite the sentence so validity is defined by paying at
least requiredPenalty, and say a unilateral ProDisTx is invalid whenever it pays
less than requiredPenalty rather than only “until the early period ends.” Update
the surrounding explanation in the section that discusses requiredPenalty,
earlyPenalty, and ProDisTx validity to preserve the monotonicity claim without
excluding earlyPenalty = 0.
Comment on lines
+292
to
+307
| Let `a = actorIndex`, `W` = the sum of non-actor share amounts, and | ||
| `P = requiredPenalty`. A ProDisTx is valid only if: | ||
|
|
||
| 1. `actorIndex < sharesCount`. | ||
| 2. There is exactly one output per non-actor share `i`, paying | ||
| `shares[i].refundScript`, in share order, plus optionally one final output | ||
| paying `shares[a].refundScript` (the actor output). A zero-value actor | ||
| output must be omitted. No other outputs are permitted. | ||
| 3. For each non-actor output, with `bonus[i] = value[i] - shares[i].amount`: | ||
| `bonus[i] >= floor(P * shares[i].amount / W)`. | ||
| 4. The sum of all `bonus[i]` is at least `P`. | ||
|
|
||
| Everything not paid to these outputs is the transaction fee, which by value | ||
| conservation can only come from the actor's share; the actor output is | ||
| therefore implicitly capped at `shares[a].amount - P`. | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Require the actor refund output whenever it is non-zero.
As written, the actor output is optional even when shares[a].amount - P > 0, which lets an implementation accept a dissolution that silently drops the actor’s remaining principal. Make the actor output mandatory except for the genuinely zero-value case.
🛠️ Suggested wording fix
-2. There is exactly one output per non-actor share `i`, paying
- `shares[i].refundScript`, in share order, plus optionally one final output
- paying `shares[a].refundScript` (the actor output). A zero-value actor
- output must be omitted. No other outputs are permitted.
+2. There is exactly one output per non-actor share `i`, paying
+ `shares[i].refundScript`, in share order, plus one final output paying
+ `shares[a].refundScript` whenever that output is non-zero. A zero-value
+ actor output must be omitted. No other outputs are permitted.📝 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.
Suggested change
| Let `a = actorIndex`, `W` = the sum of non-actor share amounts, and | |
| `P = requiredPenalty`. A ProDisTx is valid only if: | |
| 1. `actorIndex < sharesCount`. | |
| 2. There is exactly one output per non-actor share `i`, paying | |
| `shares[i].refundScript`, in share order, plus optionally one final output | |
| paying `shares[a].refundScript` (the actor output). A zero-value actor | |
| output must be omitted. No other outputs are permitted. | |
| 3. For each non-actor output, with `bonus[i] = value[i] - shares[i].amount`: | |
| `bonus[i] >= floor(P * shares[i].amount / W)`. | |
| 4. The sum of all `bonus[i]` is at least `P`. | |
| Everything not paid to these outputs is the transaction fee, which by value | |
| conservation can only come from the actor's share; the actor output is | |
| therefore implicitly capped at `shares[a].amount - P`. | |
| Let `a = actorIndex`, `W` = the sum of non-actor share amounts, and | |
| `P = requiredPenalty`. A ProDisTx is valid only if: | |
| 1. `actorIndex < sharesCount`. | |
| 2. There is exactly one output per non-actor share `i`, paying | |
| `shares[i].refundScript`, in share order, plus one final output paying | |
| `shares[a].refundScript` whenever that output is non-zero. A zero-value | |
| actor output must be omitted. No other outputs are permitted. | |
| 3. For each non-actor output, with `bonus[i] = value[i] - shares[i].amount`: | |
| `bonus[i] >= floor(P * shares[i].amount / W)`. | |
| 4. The sum of all `bonus[i]` is at least `P`. | |
| Everything not paid to these outputs is the transaction fee, which by value | |
| conservation can only come from the actor's share; the actor output is | |
| therefore implicitly capped at `shares[a].amount - P`. |
🤖 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-shared-masternode-collateral.md` around lines 292 - 307, The
ProDisTx validity rules in the actor/output section currently allow the actor
refund output to be optional whenever it is non-zero, which can let a valid
dissolution omit the actor’s remaining principal. Update the wording around the
`actorIndex`, `shares[a].refundScript`, and `requiredPenalty` rules to make the
actor output mandatory whenever `shares[a].amount - P > 0`, and only omit it in
the genuinely zero-value case; keep the rest of the output-order and
no-extra-outputs constraints unchanged.
|
<3 |
- Commit the signature count into SharedDisHash so the unilateral/unanimous mode cannot be malleated by dropping or adding signatures after signing. - Remove the non-normative replace-by-higher-fee relay suggestion; Dash relay has no replacement mechanism, so the first ProDisTx for a collateral wins. - Remove operatorReward from the ProUpSharedRegTx payload: the operator reward is fixed at registration, matching the DIP-0003 ProUpRegTx model. - Specify that a validated ProDisTx removes its masternode in the collateral-spend phase of list construction, after the block's other provider transactions apply, so a same-masternode update and dissolution are valid together in one block in either order. - Rewrite the filtering section: the registration matches on every share refund script, reward script and owner key; the lifecycle transactions are matched by proTxHash (plus ProUpShareTx's new reward script), consistent with the DIP-0003 treatment of ProUpRegTx and ProUpRevTx, since they do not carry the share table and stateless filters perform no list lookup. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
5 tasks
|
Following up on my #186 closing comment. This is the Platform-side half I said What ranI implemented most of this draft on a fork based on merged DIP-0026 ( On the tested paths, the draft behaves as specified:
On that tested path, nothing is ever pre-signed against a transaction id, so the failure Notes on the draft text from implementing itThese are the places where building it required decisions the text does not currently pin
The Platform-side companionThis is the half I promised when closing the RFC. Everything here ran on a local Platform One property carries the whole design. A single asset-lock transaction can hold one credit On top of that, a small document contract (pool, share, membership request, reward accrual,
Two design notes for anyone building this layer:
Asks
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Decentralized Masternode Shares (pre-DIP)
Adds
dip-pasta-shared-masternode-collateral.md(alias naming per BIP2 conventions, pending DIP number assignment): a specification for trustless shared masternode ownership — 2 to 8 participants fund one masternode's collateral atomically in a single registration transaction, consensus splits the owner reward by recorded contribution, and the collateral can only move through a consensus-enforced dissolution transaction that pays each participant's principal to a refund script fixed at registration.Relationship to DIP-0026 / #184
This is a strict superset of multi-party payouts: DIP-0026 gives a protocol-enforced recurring reward split but leaves the registrar in control of the payout list and the collateral spendable by whoever holds it — its Security Considerations explicitly defer immutable payout rights to a future protocol mechanism. This DIP is that mechanism. It extends the (unreleased) provider transaction version 4 from #184; a version 4 payload with an empty share list behaves exactly as DIP-0026 specifies. The two deploy together in v24 (EHF).
Key design points
04445348437551, exact match) — creation is valid only inside a shared registration, spending only viaProDisTx; the DIP states the anyone-can-spend-on-non-enforcing-chains trade-off explicitly.A full reference implementation plan exists and Core work is staged to follow number assignment / editorial review.
🤖 Generated with Claude Code
Summary by CodeRabbit