Repository navigation
feat(platform)!: multiple choice proposals take their description from a moderated text (PV15) - #5851
Open
QuantumExplorer wants to merge 49 commits into
Open
QuantumExplorer wants to merge 49 commits into
QuantumExplorer wants to merge 49 commits into
Conversation
…ntest among documents (PV15) The decision proposals contract gains a multiple choice proposal: a `multipleChoiceProposal` document is the question and each `choice` document entered for it is an answer. The choices of one proposal contend on a contested unique index keyed by the proposal's id, so masternodes vote among them with the existing contest machinery. A proposal states whether it is `open`: anyone enters choices for an open one, and only its owner for one that is not (a propertyConstraints rule on `choice` reading the proposal). The owner may edit the question's text for two hours after creating it; `open` never changes. So that an identity can enter as many choices as it wants, a contested index may declare `"resolution": 2` (`ContestedIndexResolution::MasternodeVoteOnDocuments`), admitted where `parse_contested_documents_resolution` is set (protocol version 15). It is resolution 0 (vote, abstain or lock) with each document a contender of its own: the contest keeps a contender under its document id instead of its owner's id, a vote towards a contender names the document id, the stored result names the winning document, and the winner is stored under the owner its document names. The generations that write, check and award a contender branch on that resolution in place. No contract registered before protocol version 15 can declare it, so every earlier version behaves as it did. Co-Authored-By: Claude Fable 5.2 <[email protected]>
…s variants The stored form mirrors `ContestedIndexResolution` variant for variant under the same names. With the vote among documents it has three, all starting `MasternodeVote`, which clippy's `enum_variant_names` refuses on a non-public enum. Co-Authored-By: Claude Fable 5.2 <[email protected]>
Review of the contest among documents: - The index parser took the decision to read `"resolution": 2` as a flag on its admissions, set by its callers from `parse_contested_documents_resolution` with `is_some()`. It is now a versioned method, `ContestedIndexResolution::try_from_schema_value`, taking the platform version: `None` converts the value as it always was, version 0 also reads 2, and any other version is refused with `UnknownVersionMismatch`. The schema parser and the registration cost both parse an index through `Index::try_from_value_map_for_platform_version`, which calls it; `Index::try_from_value_map` keeps the conversion that predates it. The admissions struct is as it was. - The comments of the decision proposals crate no longer say an identity enters one choice. - The contender, vote choice, winner and proof result docs say that the id naming a contender is its document's id in a vote among documents. - A test proves and verifies the vote state of a proposal whose choices include two from one identity, while the vote runs and after it ends. Co-Authored-By: Claude Fable 5.2 <[email protected]>
…(PV15) A `permanentDocument` or `moderatedDocument` `refersTo` on an identifier property may leave its target to the referring document: `contractIdProperty` and `documentTypeProperty` name the properties holding the contract and the document type, in place of `contractId` and `documentType`, and `documentTypeRequirements` says what the named type must look like: named slots, each a property it must have (`name`: `"$any"`, a property name, or a name read from the referring document; `type`; `maxBytes`; `distinctFrom`), and `require`, `anyOf` / `allOf` over slot names. Version 1 of `apply_property_reference` (protocol version 15) parses the declaration to `DocumentPropertyReferenceTarget::DynamicDocument`, appended, and refuses its keys wherever else a reference sits. Nothing about the target is known at registration, so `document_reference_validation` 1 checks it when a document is written: the contract and the type exist, the type is not private, meets the requirements (`ReferencedDocumentTypeRequirementNotMetError`, 40153) and is of the declared kind, and the document exists. A replace checks the reference again when its value or a property it reads the target from changed. Protocol version 14 and earlier keep `apply_property_reference` 0, which does not read the keys. Co-Authored-By: Claude Fable 5.2 <[email protected]>
…stration fee The registration cost of a state transition is read from the schema, where an index that does not parse is charged nothing. A vote among documents (`resolution` 2) parses there through the versioned index parse; this holds the fee to it, from the contract and from the serialization format, beside the two resolutions that predate it. Co-Authored-By: Claude Fable 5.2 <[email protected]>
…-named references (PV15)
A refersTo naming its target through the referring document may list both
kinds, ["permanentDocument", "moderatedDocument"]: the named type is then
either, and the reference is checked as one of the kind the type is.
A documentTypeRequirements slot may carry `when`, a condition in the forms
of a propertyConstraints condition read against the referenced type:
$kind, $self, $moderatorAbilities.deleteKeepsFields,
$moderatorAbilities.changeFields and { "const": ... }. A property the
condition does not hold for does not fill the slot. So a contract can ask
that a text it points at is one a moderator's removal takes away, only
when the type is a moderated one.
The condition is stored in the compiled contract and carried by consensus
errors, so its decoders refuse a nesting past 16, and the parser holds one
`when` to max_document_type_requirement_condition_nodes (16).
Co-Authored-By: Claude Fable 5.2 <[email protected]>
…ecision-proposal-description-reference Brings the multiple choice proposal types together with the references whose target the document names. The two protocol version 15 notes numbered 33 become 33 (a contest among documents) and 34 (a reference whose target the document names). Co-Authored-By: Claude Fable 5.2 <[email protected]>
…m a moderated text (PV15) A multipleChoiceProposal no longer holds a description. It points at a document saying what is being decided (descriptionId), of the contract and the document type it names (descriptionContractId, descriptionDocumentType): - the document type is a moderated one holding a string or a byte array that a moderator's removal takes away and that its author writes; - the contract declares moderators the masternodes elect. So a description can be taken down while the proposal, which is undeletable, and its vote stay. The three properties may change for two hours after the proposal is created, like its title and url. Co-Authored-By: Claude Fable 5.2 <[email protected]>
Contributor
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 47 minutes. View limit details
Pre-merge checks:
  |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Title check |  |
This assessment has not completed. The check will be retried on the next pre-merge check run. |
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled.
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
Contributor
|
📖 Book Preview built successfully. Download the preview from the workflow artifacts. Updated at 2026-11-12T11:39:04.510Z |
Contributor
|
🌳 GroveDB structure This pull request changes the described GroveDB structure. Open it in the structure viewer: new nodes glow, removed ones stay as ghosts, and the tour walks through each change. Changed (1 node)
Compared |
Contributor
|
📜 System contracts This pull request adds or changes system contract document schemas. The contract viewer diagrams each one, and a comparison marks what is added, changed and removed, with the update rule each change meets.
Compared |
Collaborator
|
🕓 Review not started yet because the new head is waiting for the 30-minute push debounce.
Commit 2bbbb4f. Normal review starts when eligible; priority review starts as soon as a slot is available. |
…n reference inputs Review of the references whose target the document names: - A document naming an indexOnly document type is refused, paid, with ReferencedDocumentTypeIndexOnlyError (40146) before anything is fetched. Drive refuses to fetch such a document by its id, which failed the transition with an internal error and no fee. - The properties a dynamic reference reads its target from (the contract id, the document type name, a slot's name property) can no longer be listed under moderatorAbilities.changeFields: a moderator's change checks no reference again. - A rule reading through a dynamic reference is refused with a message that says why. - The schema shape of documentTypeRequirements is written by rs-dpp (to_schema_value, which the parser reads back); wasm-dpp2 delegates. - A replace changing a slot's name property is covered by a test. Co-Authored-By: Claude Fable 5.2 <[email protected]>
…-when Brings in the review fixes of the base: the indexOnly refusal (40146), the reference inputs a moderator may not write, the rule message, and the schema shape of documentTypeRequirements written by rs-dpp. A slot's `when` follows that shape into rs-dpp (SlotCondition::to_schema_value, which the parser reads back), so wasm-dpp2 no longer builds it itself. Co-Authored-By: Claude Fable 5.2 <[email protected]>
…posal-description-reference Brings in the review fixes of the references whose target the document names: the indexOnly refusal (40146), the reference inputs a moderator may not write, and the schema shape of documentTypeRequirements written by rs-dpp. Co-Authored-By: Claude Fable 5.2 <[email protected]>
The message refusing a refersTo that names its target through the referring document on a property that is not an identifier held a run of spaces where its two lines join. It travels in a consensus error, so it is fixed before protocol version 15 ships, and its test now holds the whole sentence. Co-Authored-By: Claude Fable 5.2 <[email protected]>
…laude/dynamic-refers-to-when Brings in the restored line continuation of a dynamic reference refusal. Co-Authored-By: Claude Fable 5.2 <[email protected]>
…posal-description-reference Brings in the restored line continuation of a dynamic reference refusal. Co-Authored-By: Claude Fable 5.2 <[email protected]>
thepastaclaw
suggested changes
Nov 11, 2026
thepastaclaw
left a comment
thepastaclaw
left a comment
Collaborator
There was a problem hiding this comment.
Final validation — Phase 1 + Phase 2
Static verification of a7f9bd3 confirms that the description requirements admit documents elected moderators cannot remove; the overlapping reports are consolidated into one blocking finding. Two documentation suggestions remain, while the separate removal-record test request is redundant with existing shared integration coverage. Current CI shows Kotlin build/tests and the book preview passing, no Rust test check, and the automated review pending; no local builds or tests were run.
🔴 1 blocking | 🟡 2 suggestion(s)
Review provenance
Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 4: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: tenderdash-consensus-security); reviewer 8: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
criticalbygpt-6.1-sol(effort low) — The large, cross-cutting diff changes consensus rules for contested-document voting and winner handling in packages/rs-drive-abci/src/execution/platform_events/voting/award_document_to_winner/v0/mod.rs, alongside consensus-enforced contract schemas and PV15 activation. - Phase 1 reviewers:
muse-spark-1.3-contributor— general (completed, effort xhigh); agentphase1-reviewer,muse-spark-1.3-contributor— rust-quality (completed, effort xhigh); agentphase1-reviewer - Phase 1 model:
muse-spark-1.3-contributor— not quota-gated; passed overgemini-3.8-flash-high(antigravity quota lookup failed),glm-5.3-flash(not used above high effort; tier asks max) - Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
- Fresh verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— platform-versioning (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— security-auditor (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— tenderdash-consensus-security (completed, effort xhigh); agentphase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review (PastaClaw). How to work through them:
- Verify each finding against the code; fix only what is real, as narrowly as the finding.
- If a finding is wrong, decline it on its thread in one or two sentences; the bot answers once and stops.
- To defer one, link a tracking issue on its thread (or reply asking the bot to file one).
- Suggestions don't block the merge, but most are worth fixing in this PR; judge each on its merits.
- Keep the PR's scope; don't grow it to satisfy hardening suggestions.
- Carried items were raised in an earlier round: read the thread first, and don't re-fix what you already fixed, declined or deferred there. Disputed and deferred items are already handled and are not listed.
In `packages/decision-proposals-contract/schema/v1/decision-proposals-contract-documents.json`:
- [BLOCKING] packages/decision-proposals-contract/schema/v1/decision-proposals-contract-documents.json:231-235: Require a description that elected moderators can actually remove
Combining `moderatedDocument` with the separate `moderation: "elected"` requirement does not establish an elected team's ability to remove the selected description. `DocumentTypeV2::document_reference_kind` checks deletion flags and record retention, while `ContractReferenceModeration::is_met_by` checks only that an elected declaration exists. Valid attacker-controlled hosts can therefore bypass the intended takedown boundary in three ways:
- Omit `DeleteDocuments` for the selected type from `moderated_document_types`, while retaining `moderatorAbilities.delete: true` on that type. Registration permits this unless the type also requests settled deletion. After seating, `Moderators::lacks` consults `ElectedModerators::allows`, and `deletion_authority` refuses deletion with 41201.
- Use an immutable, owner-undeletable type with `deleteWithin: 1`, required `$createdAt`, and no `deleteSettled`. The text still qualifies after its window expires, but ordinary deletion returns 41116 and settled deletion returns 41204.
- Declare `ownerProtected: true` and have the host's owner write the description. `fetch_deletable_document` consults `Moderators::protects` and refuses both ordinary and settled deletion with 41102, including after an election.
These are lasting exemptions after a team is seated, not just the disclosed interim limitation. Because proposal authors choose the host, they can publish descriptions that satisfy every new reference predicate yet remain unremovable. Require `ownerProtected: false` using the existing contract requirement, and extend the shared DPP reference requirements and generic ABCI validation to check the selected type's elected deletion grant and durable deletion capability. Add adversarial-host admission tests and actual deletion regressions for these cases.
In `docs/protocol/decision-proposals.md`:
- [SUGGESTION] docs/protocol/decision-proposals.md:218-220: Document how long a host can prevent its first moderation election
The interim limitation also depends on the host's `electionDelay`, which `moderation: "elected"` does not inspect. `ElectedModerators` accepts a delay of up to `u32::MAX` seconds, so an otherwise qualifying host can prevent the first election for roughly 136 years while its owner or nobody moderates. This is finite rather than literally permanent, but it materially changes the practical takedown guarantee. State this explicitly alongside the interim limitation. The existing `electionOpen` requirement can exclude hosts whose election delay has not elapsed if that stronger admission policy is intended; it still would not guarantee a seated team.
- [SUGGESTION] docs/protocol/decision-proposals.md:205-211: Include private and index-only target refusals in the description error table
The new table omits two refusals clients can receive when a proposal names an unsuitable description type: `ReferencedDocumentTypePrivateError` (40150) and `ReferencedDocumentTypeIndexOnlyError` (40146). The generic dynamic-reference validator checks these before the property requirements and reference kind, so an index-only target returns 40146 rather than the listed wrong-kind error. Add both cases. If the table is intended to describe validation precedence, also put document-type requirements before kind; otherwise clarify that its rows are not ordered by precedence.
Second review of the references whose target the document names: - `require` is judged on every write that checks the reference, and only the number of slots was bounded. It now names nothing twice: no term, no slot within a group, no group with its slots in another order. With at most four slots that is at most fifteen terms, where a contract could repeat one slot thousands of times. - The book's rule for which property fills which slot now starts by choosing the set of slots, as consensus does, so a client reads every reference Platform accepts. - A test makes the assignment search take a property back and then succeed. Co-Authored-By: Claude Fable 5.2 <[email protected]>
…laude/dynamic-refers-to-when Brings in the second review's fixes of the base: `require` names nothing twice, the book's client assignment rule chooses the set of slots first, and a test of the assignment search taking a property back. Co-Authored-By: Claude Fable 5.2 <[email protected]>
…posal-description-reference Brings in the second review's fixes of the references whose target the document names: `require` names nothing twice, and the book's client assignment rule chooses the set of slots first. Co-Authored-By: Claude Fable 5.2 <[email protected]>
…Id and documentType
A refersTo whose target the referring document names no longer has keys of
its own for it. It writes the keys every document reference has, read from
the document:
"contractId": "$self.descriptionContractId",
"documentType": "$self.descriptionDocumentType"
in place of contractIdProperty and documentTypeProperty. `$self` is the
document being written, as `$old` is the stored one in a rule. A slot's
name read from the document follows: "$self.bodyProperty" in place of
{ "property": "bodyProperty" }.
Both are read from the document or neither: one of them literal, or left
out, beside the other is refused, as is documentTypeRequirements beside a
target the schema names. Protocol version 14 gives the prefix no meaning
and refuses such a declaration. What is stored is unchanged.
Co-Authored-By: Claude Fable 5.2 <[email protected]>
thepastaclaw
suggested changes
Nov 11, 2026
thepastaclaw
left a comment
thepastaclaw
left a comment
Collaborator
There was a problem hiding this comment.
Re-review — Final validation — Phase 1 + Phase 2
At head 3746732, the changes since the previous review tighten dynamic-reference requirement deduplication and clarify binding order; static inspection found no additional defect in that delta. The proposal schema and cited documentation sections are unchanged, so all three prior findings remain valid. Current CI reports no failed checks, including successful Kotlin build/tests, but lists no Rust-suite result; the review check remains pending, and no builds or tests were run locally.
🔴 1 blocking | 🟡 2 suggestion(s)
Still open from earlier rounds (3)
- 🔴 Blocking: Require a description that elected moderators can actually remove —
packages/decision-proposals-contract/schema/v1/decision-proposals-contract-documents.json:231-235(thread) - 🟡 Suggestion: Include private and index-only target refusals in the description error table —
docs/protocol/decision-proposals.md:205-211(thread) - 🟡 Suggestion: Document how long a host can prevent its first moderation election —
docs/protocol/decision-proposals.md:218-220(thread)
Review provenance
Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 6: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: tenderdash-consensus-security); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
criticalbygpt-6.1-sol(effort low) — The large diff changes consensus-enforced contested-document identity and winner ownership in packages/rs-drive/src/drive/document/insert_contested/add_contested_indices_for_contract_operations/v1/mod.rs and packages/rs-drive-abci/src/execution/platform_events/voting/award_document_to_winner/v0/mod.rs. - Phase 1 reviewers:
muse-spark-1.3-contributor— general (completed, effort xhigh); agentphase1-reviewer,muse-spark-1.3-contributor— rust-quality (completed, effort xhigh); agentphase1-reviewer - Phase 1 model:
muse-spark-1.3-contributor— not quota-gated; passed overgemini-3.8-flash-high(antigravity quota lookup failed),glm-5.3-flash(not used above high effort; tier asks max) - Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
- Fresh verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— platform-versioning (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— security-auditor (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— tenderdash-consensus-security (completed, effort xhigh); agentphase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review (PastaClaw). How to work through them:
- Verify each finding against the code; fix only what is real, as narrowly as the finding.
- If a finding is wrong, decline it on its thread in one or two sentences; the bot answers once and stops.
- To defer one, link a tracking issue on its thread (or reply asking the bot to file one).
- Suggestions don't block the merge, but most are worth fixing in this PR; judge each on its merits.
- Keep the PR's scope; don't grow it to satisfy hardening suggestions.
- Carried items were raised in an earlier round: read the thread first, and don't re-fix what you already fixed, declined or deferred there. Disputed and deferred items are already handled and are not listed.
In `packages/decision-proposals-contract/schema/v1/decision-proposals-contract-documents.json`:
- [BLOCKING] packages/decision-proposals-contract/schema/v1/decision-proposals-contract-documents.json:231-235: Require a description that elected moderators can actually remove
(carried; thread: https://github.com/dashpay/platform/pull/5851#discussion_r4533070930)
In `docs/protocol/decision-proposals.md`:
- [SUGGESTION] docs/protocol/decision-proposals.md:205-211: Include private and index-only target refusals in the description error table
(carried; thread: https://github.com/dashpay/platform/pull/5851#discussion_r4533070944)
- [SUGGESTION] docs/protocol/decision-proposals.md:218-220: Document how long a host can prevent its first moderation election
(carried; thread: https://github.com/dashpay/platform/pull/5851#discussion_r4533070939)
…laude/dynamic-refers-to-when
Brings in the target read through `$self.` on contractId and documentType,
and a slot name read as "$self.<property>".
With `$self` now the referring document, the operand of a slot's `when`
that reads the property tried for the slot is renamed `$property`:
{ "not": { "contains": ["$moderatorAbilities.deleteKeepsFields", "$property"] } }
so each word means one thing in a declaration. `$self` in a `when` is
refused: a condition on the referenced type does not read the referring
document.
Co-Authored-By: Claude Fable 5.2 <[email protected]>
…posal-description-reference
Brings in the target read through `$self.` and the `when` operand renamed
`$property`, and writes the multiple choice proposal's description with
them:
"contractId": "$self.descriptionContractId",
"documentType": "$self.descriptionDocumentType",
... { "not": { "contains": ["$moderatorAbilities.deleteKeepsFields", "$property"] } }
What the reference means is unchanged. The schema's bytes are not, so the
ten genesis root pins of nullifier_batch_guard move with it.
Co-Authored-By: Claude Fable 5.2 <[email protected]>
A string property declaring both maxLength and maxBytes was held to its maxBytes alone when a slot's maxBytes was checked, so one of at most two characters (eight bytes) with a maxBytes of 100 failed a slot of eight bytes it can never exceed. The bound is now the smaller of maxBytes and four bytes a character of maxLength. Co-Authored-By: Claude Fable 5.2 <[email protected]>
…laude/dynamic-refers-to-when Brings in the tighter of a string property's two bounds when a slot's maxBytes is checked. Co-Authored-By: Claude Fable 5.2 <[email protected]>
…posal-description-reference Brings in the tighter of a string property's two bounds when a slot's maxBytes is checked. Co-Authored-By: Claude Fable 5.2 <[email protected]>
Contributor
|
Your move: thepastaclaw requested changes on this head; dismiss the review or push a fix; thepastaclaw left review threads unresolved; resolve them. |
github-actions
Bot
added
the
waiting-self-review
…lete (PV15) Review of the multiple choice proposal's description: a host could give a description that passed every check and could never be removed, by leaving its type out of what the elected team may delete, or by letting moderators delete only within a window with no rule for later. documentTypeRequirements takes a `when` of its own: the condition of a slot's `when`, judged once for the named type as a whole, reading no $property. A `when` may now also look into $electedAbilities, the abilities the type's contract gives its elected moderation team on the type, and test with `present` or `absent` whether the type declares moderatorAbilities.deleteWithin or moderatorAbilities.deleteSettled. The proposal's description asks for both: the team holds deleteDocuments on the type, and the type sets no window or says who deletes past it. The schema's bytes change, so the ten genesis root pins of nullifier_batch_guard move with it. Co-Authored-By: Claude Fable 5.2 <[email protected]>
Contributor
|
Waiting for bot review — thepastaclaw not yet. Wait for the missing reviews, or a writer can post |
github-actions
Bot
added
waiting-bots
…roposal-contract-1db678 v5.1-dev gained a protocol version 15 note numbered 35 (composite document history, #4652): the note of a contest among documents becomes 36. Two pinned roots depend on the system contracts and on what v5.1-dev changed since: the ten of nullifier_batch_guard and the state after the protocol version 15 activation. Both are recomputed for the two together. Co-Authored-By: Claude Fable 5.2 <[email protected]>
…ecision-proposal-description-reference Brings in v5.1-dev at 78d691d, which now holds the either-kind targets and slot `when` change (#5850) as a squash, together with the multiple choice proposals branch this one is built on. - Files #5850's squash left identical to its last head keep this branch's version, which adds the type-level `when`, `$electedAbilities` and the `present`/`absent` conditions on top of it. - document_reference_validation 1 keeps reading the elected team's abilities for the named type before judging its requirements. - Protocol version 15 notes read 33 (reference whose target the document names, with the type-level `when`), 34 (keys with a budget), 35 (composite document history), 36 (a contest among documents). - The ten nullifier_batch_guard roots and the activation root are recomputed for this branch's decision proposals schema on the new base. Co-Authored-By: Claude Fable 5.2 <[email protected]>
Contributor
|
Waiting for bot review — thepastaclaw not yet. Wait for the missing reviews, or a writer can post |
…the decision proposals contract `should_pin_the_fees_of_a_consume_through_a_plain_and_a_contested_create` came in with v5.1-dev (#5874) and pins the fee of a contested create at protocol version 15. On this branch the decision proposals contract, registered at genesis from 15, has a contested index of its own (`choice`), so the tree of contracts holding contests has one entry more and a write below it pays for the larger tree: 3_528_980 rather than 3_478_880. The plain create, the storage fee, the refund and every protocol version 14 figure are unchanged. Co-Authored-By: Claude Fable 5.2 <[email protected]>
…ecision-proposal-description-reference
Contributor
|
Waiting for bot review — thepastaclaw not yet. Wait for the missing reviews, or a writer can post |
…roposal-contract-1db678 Brings in v5.1-dev at 8b4ca80: the pin of the bytes protocol version 15 stores contracts in (#5880) and the array depth fix of validate_max_depth 2 (#5881). The merge is clean. The stored contract fixture is regenerated for the one contract this branch changes: the decision proposals contract, which gains `multipleChoiceProposal` and `choice` at protocol version 15 (unreleased). No other line of `stored.jsonl` changes. Co-Authored-By: Claude Fable 5.2 <[email protected]>
- A choice may be neither changed nor deleted by anyone, yet the end of its vote removes the choices that were not kept: the README said "never deleted". - The fund a choice pays doubles once the vote holds 250 choices and again for every 50 more, as the protocol guide already says; the README gave 0.1 Dash alone. - The protocol guide says where the ids of the choices not kept are read: the stored result, which the vote state query returns, lists every choice by its document id with its tally. Co-Authored-By: Claude Fable 5.2 <[email protected]>
…ecision-proposal-description-reference Brings in v5.1-dev at 8b4ca80 (the pin of the bytes protocol version 15 stores contracts in, #5880, and the array depth fix of validate_max_depth 2, #5881) and the multiple choice proposals branch's README and protocol guide wording on when a choice leaves state and how its fund grows. The stored contract fixture is regenerated for the one contract this branch changes: the decision proposals contract, whose `multipleChoiceProposal` names its description through a reference here. No other line of `stored.jsonl` changes. Co-Authored-By: Claude Fable 5.2 <[email protected]>
Contributor
|
Waiting for bot review — thepastaclaw not yet. Wait for the missing reviews, or a writer can post |
…roposal-contract-1db678 Brings in v5.1-dev at 1f36c10: an insert writes no entry in an expired time range bucket (#5877), which took protocol version 15 note 36. The one conflict is the note list: the base's note stays 36 and this branch's "A contest among documents" becomes 37. No root pin, fee pin or stored contract fixture line changes. Co-Authored-By: Claude Fable 5.2 <[email protected]>
…ecision-proposal-description-reference Brings in v5.1-dev at 1f36c10: an insert writes no entry in an expired time range bucket (#5877), which took protocol version 15 note 36, so the multiple choice proposals branch's "A contest among documents" is 37. The merge is clean and no root pin, fee pin or stored contract fixture line changes. Co-Authored-By: Claude Fable 5.2 <[email protected]>
Contributor
|
Waiting for bot review — thepastaclaw not yet. Wait for the missing reviews, or a writer can post |
thepastaclaw
suggested changes
Nov 12, 2026
thepastaclaw
left a comment
thepastaclaw
left a comment
Collaborator
There was a problem hiding this comment.
Re-review — Final validation — Phase 1 + Phase 2
The current implementation still admits a moderated description type whose settled-deletion quorum can never be satisfied for a pre-election document, so the PR's takedown guarantee remains bypassable. Rust, wasm32, JS build, Swift, and Kotlin checks succeeded in the supplied CI snapshot; JS tests/lint and several Docker jobs are still pending.
🔴 1 blocking | 🟡 1 suggestion(s)
Still open from earlier rounds (1)
- 🔴 Blocking: Require a description that elected moderators can actually remove —
packages/decision-proposals-contract/schema/v1/decision-proposals-contract-documents.json:247-253(thread)
Review provenance
Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 7: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 8: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
criticalbygpt-6.1-sol(effort low) — The large, cross-cutting diff changes consensus-enforced document reference validation in packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/action_validation/document/document_reference_validation/v1/mod.rs and contested-index state updates in packages/rs-drive/src/drive/document/insert_contested/add_contested_indices_for_contract_operations/v1/mod.rs. - Phase 1 reviewers:
muse-spark-1.3-contributor— general (completed, effort xhigh); agentphase1-reviewer,muse-spark-1.3-contributor— rust-quality (completed, effort xhigh); agentphase1-reviewer - Phase 1 model:
muse-spark-1.3-contributor— not quota-gated; passed overgemini-3.8-flash-high(antigravity quota lookup failed),glm-5.3-flash(not used above high effort; tier asks max) - Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
- Fresh verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— ffi-engineer (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— platform-versioning (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— security-auditor (completed, effort xhigh); agentphase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review (PastaClaw). How to work through them:
- Verify each finding against the code; fix only what is real, as narrowly as the finding.
- If a finding is wrong, decline it on its thread in one or two sentences; the bot answers once and stops.
- To defer one, link a tracking issue on its thread (or reply asking the bot to file one).
- Suggestions don't block the merge, but most are worth fixing in this PR; judge each on its merits.
- Keep the PR's scope; don't grow it to satisfy hardening suggestions.
- Carried items were raised in an earlier round: read the thread first, and don't re-fix what you already fixed, declined or deferred there. Disputed and deferred items are already handled and are not listed.
In `packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/dynamic_reference.rs`:
- [SUGGESTION] packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/dynamic_reference.rs:74-82: Centralize the list of electable abilities instead of hardcoding it at the parser
`ELECTED_ABILITY_NAMES` duplicates all five `ModerationAbility` variants. If a sixth ability is added to the enum, the parser still compiles but silently rejects it in `$electedAbilities`, and the error message is generated from the same stale list. Derive the accepted names from the enum, such as through a centralized `all()` method, or add an exhaustiveness safeguard so this parser cannot drift from `ModerationAbility`.
In `packages/decision-proposals-contract/schema/v1/decision-proposals-contract-documents.json`:
- [BLOCKING] packages/decision-proposals-contract/schema/v1/decision-proposals-contract-documents.json:247-253: Require a description that elected moderators can actually remove
(carried; thread: https://github.com/dashpay/platform/pull/5851#discussion_r4533070930)
Comment on lines
+74
to
+82
| /// The names of the abilities a contract may give its elected moderation team on a document | ||
| /// type, what `$electedAbilities` lists. | ||
| const ELECTED_ABILITY_NAMES: [ModerationAbility; 5] = [ | ||
| ModerationAbility::DeleteDocuments, | ||
| ModerationAbility::Ban, | ||
| ModerationAbility::Suspend, | ||
| ModerationAbility::Warn, | ||
| ModerationAbility::ChangeDocumentFields, | ||
| ]; |
Collaborator
There was a problem hiding this comment.
🟡 Suggestion: Centralize the list of electable abilities instead of hardcoding it at the parser
ELECTED_ABILITY_NAMES duplicates all five ModerationAbility variants. If a sixth ability is added to the enum, the parser still compiles but silently rejects it in $electedAbilities, and the error message is generated from the same stale list. Derive the accepted names from the enum, such as through a centralized all() method, or add an exhaustiveness safeguard so this parser cannot drift from ModerationAbility.
source: muse-spark-1.3-contributor (phase1-reviewer: rust-quality)
Contributor
|
Your move: thepastaclaw requested changes on this head; dismiss the review or push a fix. |
github-actions
Bot
added
waiting-self-review
…roposal-contract-1db678 Brings in v5.1-dev at 97fb5f9: a refersTo is read as dynamic only where protocol version 14 cannot read it (#5887). The merge is clean and no root pin, fee pin or stored contract fixture line changes. Co-Authored-By: Claude Fable 5.2 <[email protected]>
…ecision-proposal-description-reference Brings in v5.1-dev at 97fb5f9: a refersTo is read as dynamic only where protocol version 14 cannot read it (#5887). The one conflict is protocol version 15 note 33, where both sides added a sentence: it keeps this branch's use of the reference by the decision proposals contract and the base's statement of which declarations version 1 reads as dynamic. The parser and its tests merge cleanly: the description reference is a `moderatedDocument` whose `contractId` is written `$self.`, which stays dynamic. No root pin, fee pin or stored contract fixture line changes. Co-Authored-By: Claude Fable 5.2 <[email protected]>
A tie goes to the earliest choice by creation time, then block height, then core block height, and choices entered in the same block share all three: among them the lower document id wins, not the one entered first. The protocol guide and the README said only "the earliest choice", and the guide that a vote nobody votes in keeps the first choice entered. Co-Authored-By: Claude Fable 5.2 <[email protected]>
…ecision-proposal-description-reference
Contributor
|
Waiting for bot review — thepastaclaw not yet. Wait for the missing reviews, or a writer can post |
github-actions
Bot
added
waiting-bots
This branch has not been deployed
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
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.
Basic explanation
What this does: A multiple choice proposal used to carry its own description text. Now it carries none. Instead it points at a text stored somewhere else: a document in another contract, of a kind that moderators can remove, in a contract whose moderators are elected by the masternodes.
Value: A proposal can never be deleted, so text stored in it could never be taken down, and creating one costs almost nothing. With this, a description that turns out to be spam or abuse can be removed by moderators while the question and its vote stay in place.
Risks: Medium. It changes the schema of a system contract that has not shipped yet (protocol version 15), so nothing stored is affected. It makes multiple choice proposals depend on a contract outside the system contracts: no system contract hosts moderated text today. The text can still change after the proposal's two-hour edit window if the type hosting it allows edits.
titleandurlare still free text. The JS spec was updated but not run locally; no full suite or mobile build was run.Issue being fixed or feature implemented
#5848 adds
multipleChoiceProposalwith a required free-textdescriptionof up to 4096 bytes. The proposal is undeletable and costs only its storage, and nothing obliges anyone to enter a choice for it, so the description is a cheap place to put text that then stays for good.#5849 and #5850 add a reference whose target the document names, with requirements on the named document type. This PR uses it for the description.
How this is stacked. #5849 and #5850 are merged, so the base is
v5.1-dev. The branch still has #5848's branch merged in, since the description belongs to the document type #5848 adds, so until #5848 merges this diff also shows #5848's changes. The commits to review ared2e04ba810(the change),065f9a2875(review fixes) ande2bea23c63(the condition on the type, below); the rest are merges. The protocol version 15 notes followv5.1-dev: 33 is a reference whose target the document names, 34 came with #5861, 35 with #4652, 36 with #5877, and #5848's contest among documents is 37.What was done?
The contract
multipleChoiceProposallosesdescriptionand gains three required properties:where
R, what is asked of the type as a whole, is{ "allOf": [ { "contains": ["$electedAbilities", { "const": "deleteDocuments" }] }, { "anyOf": [ { "absent": "$moderatorAbilities.deleteWithin" }, { "present": "$moderatorAbilities.deleteSettled" } ] } ] }and
W, what is asked of the property holding the text, is{ "allOf": [ { "not": { "contains": ["$moderatorAbilities.deleteKeepsFields", "$property"] } }, { "not": { "contains": ["$moderatorAbilities.changeFields", "$property"] } } ] }In words: the document type is a moderated one, it holds a string or a byte array that a moderator's removal takes away and that the document's author writes, and its contract declares moderators the masternodes elect. The three properties sit under the same two-hour
immutablecondition astitleandurl, so the owner can point the proposal at another description for two hours. Property positions are renumbered (url4,open5); the contract is unreleased.The yes/no
proposaltype andchoiceare unchanged.Before and after
Creating a multiple choice proposal:
After a moderator removes the description document:
Where it lives
packages/decision-proposals-contract: the schema;multiple_choice_proposal::properties::{DESCRIPTION_ID, DESCRIPTION_CONTRACT_ID, DESCRIPTION_DOCUMENT_TYPE}in place ofDESCRIPTION; the Rust and JS contract tests; the README.rs-drive-abci: the multiple choice tests register two description hosts (one with elected moderation, one moderated by its owner) and write a text before each proposal. A new test covers the accepted case and each refusal; the two-hour edit test now repoints the description. The ten genesis root pins innullifier_batch_guardmove with the schema.docs/protocol/decision-proposals.md: a section on the description, what is checked and what it does and does not give. The book chapter on these references and note 33 of protocol version 15 name this use.Choices a reviewer may want to question
moderatedDocument, not the either-kind list feat(platform)!: either-kind targets and slot conditions for document-named references (PV15) #5850 adds: a permanent text could never be taken down, which is the point of the change.deleteDocuments, and a type moderators delete only within adeleteWithinwindow with no settled deletion. Nothing a reference could declare stated either, so this PR adds the vocabulary:documentTypeRequirementstakes awhenof its own, judged once for the named type and reading no$property; awhenmay look into$electedAbilities(what the type's contract gives its elected team on the type) and testpresent/absentonmoderatorAbilities.deleteWithinanddeleteSettled. The operand names are mine and easy to change. What is left is the interim: a settled document is deleted by a seated team alone."moderation": "elected"says the contract declares a team the masternodes elect. It does not say one is seated: until then the contract's interim moderators apply, which may be its owner or nobody. Requiring a seated team would need a newcontractRequirementsvalue.positionorder that fillsbody, thenbodyBytes), not named by the proposal.title,urland a choice'sdescriptionremain free text. A choice pays the contest fund; a proposal's title does not.How Has This Been Tested?
Targeted runs only, on macOS:
RUST_MIN_STACK=16777216 cargo test -p drive-abci --lib -- multiple_choice_proposal decision_proposal dynamic_document_reference create_genesis_state data_contract_create: 238 passed. The 12 multiple choice tests run against a hosted description; new:should_take_the_description_from_a_moderated_text_of_an_elected_contract.cargo test -p drive-abci --lib -- nullifier_batch_guard::should_preserve_single_entry: 2 passed with the re-pinned roots.cargo test -p dpp --lib --features all-system_contracts,fixtures-and-mocks,validation -- system_data_contract decision_proposal dynamic_reference slot_condition: 67 passed (the contract loads and registers at protocol version 15).cargo test -p decision-proposals-contract -p platform-version: passed.cargo clippy --all-targets --all-features --locked -- --no-deps -D warningson drive-abci, decision-proposals-contract and platform-version: clean.Not run: the JS contract spec (
decisionProposalsContract.spec.jswas updated and passesnode --check, but mocha and eslint need the wasm build and installed dependencies), any full suite, the JS and wasm32 builds, the mobile builds.In-place changes to shipped generations
The first commit edits no versioned method. The condition on the type extends
apply_property_reference1,check_document_type_schema1 anddocument_reference_validation1, all selected by protocol version 15 alone and unreleased;DocumentTypeRequirementsgains a field andSlotConditiontwo variants at the tail, on types no released version has stored. It changes the document schema of the decision proposals contract, which is registered from protocol version 15 (unreleased) and exists on no released network, test fixtures and pins, and documentation. The merge commit brings in #5848, whose table of in-place changes is in that PR.Breaking Changes
The decision proposals contract's
multipleChoiceProposalschema changes at protocol version 15, before its first release:descriptionis replaced bydescriptionId,descriptionContractIdanddescriptionDocumentType. Protocol version 14 and earlier are unchanged.Checklist:
structure.rs, regeneratedgrovedb-structure.json, and checked the structure viewer link posted on this pull requestFor repository code-owners and collaborators only
🤖 Generated with Claude Code
PR Hygiene ·
2bbbb4f/skip-botsproceeds without the ones not yet reported/self-reviewedWhen every merge requirement is met, the
PR Hygienecheck passes. Reviewer limits do not block merging; other required GitHub checks and protections still apply.