feat(platform)!: validate contested index parameters and bind the native award to its poll - #4784
DCG-Claude wants to merge 10 commits into
Conversation
…ard and parameter validation slots Protocol version 17 is the provisional 5.0 version from the DashVM allocation register; 15 and 16 are the placeholders the other 5.0 branches carry, byte-identical to theirs so a forward merge resolves by taking either side. Three new table entries gate this task's changes at 17 only: - DPP_VALIDATION_VERSIONS_V6 turns on document_type .validate_contested_index_parameters, a new OptionalFeatureVersion that is None on V1 to V5, so stored contracts and pre-17 history are never re-judged. - DRIVE_DOCUMENT_METHOD_VERSIONS_V5, selected by the new DRIVE_VERSION_V10, turns on insert_contested.award_contested_document_vote_poll, a new OptionalFeatureVersion that is None on V1 to V4 so no historical table can dispatch the native award operation. - DRIVE_ABCI_METHOD_VERSIONS_V11 selects check_for_ended_vote_polls v1. The implementations behind the slots follow in the next commits. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A contract may parameterize a contest only through the native contested declaration, and the parameters it declares must be ones the native contest machinery can honour. The parser checked the declaration's shape but never whether the vote poll key, the contested tree walker, the contest detection and the award could all read the declared properties the same way. validate_contested_index_parameters is a new versioned class method called from the shared document-type parser core under full validation, dispatched on the validation table (None on every version before 17, so stored contracts and pre-17 history are never re-judged). Generation 0 requires every contested index property to be a top-level required user property and every field match to name a string property of the index. Each rule closes a failure the node exhibits today: a nested or system property gives the poll key and the tree path different segments, an optional property reaches the walker as a null, a match outside the index lets equal index values take different insert paths so the award collides in the unique index during block execution, and a match on a non-string never matches so the contest can never start. Rejections carry the new ContestedIndexInvalidParametersError (code 10277, provisional, next free in the data contract band), appended to BasicError with an arm in the legacy wasm-dpp mirror. Every contested fixture in the tree passes the rules. validate_update v1 already freezes the parameters on contract update through its name-keyed index comparison; the new tests pin that a changed pattern, an added match, a changed matched field and a removed declaration are all rejected while a description-only edit stays accepted. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ive operation award_contested_document_vote_poll is the native award: it takes the resolved poll and the end date the finalization sweep found it under, and nothing else. It re-derives every fact from state before writing anything: the stored status must be Started, the block time must have reached the end date, the poll's unique id must be present in the end-date queue under that end date, the winner is the top tally with the native tie-break (greatest creation time, then block height, core height and document id, the rule the block executor applied), a lock tally strictly above the top locks, and the awarded bytes are the contender document as stored when the contest was joined. The insert is the same generic add_document_for_contract call the block executor made, so a legitimate award writes the same bytes; no data trigger, creation restriction or other ordinary rule runs on it. Every other call is a typed DriveError::ContestedAwardRejected with the root hash unchanged: award before the end, award under a forged end date, award of an awarded, locked, not-started or unknown poll. A second call between the award and the record keeper's status flip fails on the primary storage existence check of the insert and cannot diverge. The slot is None on every document method table before V5, so the dispatcher refuses the operation on historical tables. The operation is on no batched action, no document batch operation and no Drive batch operation enum; a test enumerates all of them without a wildcard so a future variant that could carry an award has to revisit it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…operation check_for_ended_vote_polls v1, selected by DRIVE_ABCI_METHOD_VERSIONS_V11 at protocol version 17, replaces generation 0's in-crate "tally, sort, deserialize the top contenders, tie-break, award" with one call to Drive::award_contested_document_vote_poll per ended poll. The returned contenders feed the unchanged voter fetch and the record keeper, and the cleanup runs as before, so award, finalization record and cleanup stay one block transaction in one event. The event supplies no contender, so it cannot redirect an award. Generation 0's two testnet repair branches cannot trigger at protocol 17 and are not carried; v0 and award_document_to_winner v0 stay byte-identical. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ed parameter gating - check_for_ended_vote_polls through the dispatcher on both sides of the gate: generation 1 at the latest version and generation 0 pinned at protocol version 14 award the same winner and leave the same record, and a sweep before the end date awards nothing and leaves the poll queued. - document_distribution: on the DPNS system contract with its data triggers enabled, both contenders delete their preorder before the poll ends so a create-scope rule re-run on the award would reject; the award succeeds, the winner's replace and delete of the awarded domain are still rejected by the ordinary rules, the stored info carries exactly one finalized event, and a later sweep finds nothing to award. - data_contract_create: a field match outside the contested index is accepted at protocol version 14 and rejected at the latest version with ContestedIndexInvalidParametersError, on mainnet. - data_contract_update: a changed contested regex pattern is rejected as a changed index; a description-only edit is accepted. The private contest helper that returns the preorder documents becomes visible to the execution module for the rule-scope test. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ative award A new book chapter covers the contested declaration and its three members, the four supported-parameter rules and the protocol version they gate at, the freeze of the parameters on update, the two rule scopes (ordinary document actions under ordinary rules, the award under the native contested rules only), the four state-derived checks of the native award operation, finalization once and atomically, and the lifecycle note. The indexes chapter points at it and the error code table gains 10277. The contract update tests target the id the create transition registered and fund the identity for both contested transitions. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… native award The award operation took a resolved poll carrying an owned contract and used that contract for contender deserialization and the winning insert, while only the tally query re-resolved from state. A caller holding a Drive could therefore reuse a real poll id with altered document-type or index metadata. award_contested_document_vote_poll now takes the unresolved ContestedDocumentResourceVotePoll (contract id, document type name, index name, index values) and fetches the contract from state by that id before any other check; a poll naming a contract that is not in state is rejected with ContestedAwardRejected and an index the committed contract does not define with ContestedIndexNotFound, both with the root hash unchanged. The outcome carries the poll as the award resolved it, and the finalization event records and cleans up with that poll, so no contract metadata crosses the call in either direction. Two Drive tests pin the forged contract id and the forged index name. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
|
📖 Book Preview built successfully. Download the preview from the workflow artifacts. Updated at 2026-09-17T07:08:43.517Z |
|
✅ Final review complete — no blockers (commit d6b1541) · triage: critical · Phase 2 only (queue backlog) |
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Final validation — Phase 1 + Phase 2
The native award preserves the existing selection rule and resolves its inputs from committed state. However, the new registration validator still accepts required transient contested properties, leaving an admission gap that defeats the PR’s stated goal of rejecting declarations the contest machinery cannot support. One documentation correction is also warranted.
🔴 1 blocking | 💬 1 nitpick(s)
Review provenance
Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: architecture-layering); reviewer 3: muse-spark-1.3-contributor (agent: phase1-reviewer, role: ffi-engineer); reviewer 4: muse-spark-1.3-contributor (agent: phase1-reviewer, role: platform-versioning); reviewer 5: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 6: muse-spark-1.3-contributor (agent: phase1-reviewer, role: security-auditor); reviewer 7: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 8: gpt-6-astra (agent: phase2-reviewer, role: architecture-layering); reviewer 9: gpt-6-astra (agent: phase2-reviewer, role: ffi-engineer); reviewer 10: gpt-6-astra (agent: phase2-reviewer, role: platform-versioning); reviewer 11: gpt-6-astra (agent: phase2-reviewer, role: rust-quality); reviewer 12: gpt-6-astra (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6-astra (agent: astra-verifier, role: final-verifier)
- Triage:
criticalbygpt-6-astra(effort low) — The diff introduces intricate, cross-crate consensus changes through DocumentType::validate_contested_index_parameters, Drive::award_contested_document_vote_poll, and check_for_ended_vote_polls/v1, changing contract acceptance and state-derived winner selection and award execution under protocol-version gating. - Phase 1 reviewers:
muse-spark-1.3-contributor— general (completed, effort xhigh); agentphase1-reviewer,muse-spark-1.3-contributor— architecture-layering (completed, effort xhigh); agentphase1-reviewer,muse-spark-1.3-contributor— ffi-engineer (completed, effort xhigh); agentphase1-reviewer,muse-spark-1.3-contributor— platform-versioning (completed, effort xhigh); agentphase1-reviewer,muse-spark-1.3-contributor— rust-quality (completed, effort xhigh); agentphase1-reviewer,muse-spark-1.3-contributor— security-auditor (completed, effort xhigh); agentphase1-reviewer - Phase 1 model:
muse-spark-1.3-contributor— not quota-gated; passed overgemini-3.8-flash-high(antigravity below 15% reserve: weekly 11% left, 5h 100% left),glm-5.3-flash(not used above high effort; tier asks max) - Fresh verifier:
gpt-6-astra— final-verifier; agentastra-verifier - Phase 2 reviewers:
gpt-6-astra— general (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— architecture-layering (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— ffi-engineer (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— platform-versioning (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— security-auditor (completed, effort xhigh); agentphase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `packages/rs-dpp/src/data_contract/document_type/class_methods/validate_contested_index_parameters/v0/mod.rs`:
- [BLOCKING] packages/rs-dpp/src/data_contract/document_type/class_methods/validate_contested_index_parameters/v0/mod.rs:68-73: Reject transient properties in contested indexes
Being listed in `required_fields` does not guarantee that an index property survives into contested storage. A required string property such as `normalizedLabel` can also appear in the document schema’s `transient` array; neither this validator nor the preceding `validate_index_properties` rejects that combination. Contest classification and poll values use the submitted properties, but `Document::try_from_owned_create_transition_action_v0` removes transient properties before storage, and the contested index walker reads the resulting missing property as an empty key. The poll and storage paths therefore disagree, leaving a contract that passes the new admission checks but whose contender creation fails internally—the same class of failure this validation is intended to prevent. Check `DocumentProperty.transient` for every contested index property, including properties without a field match, and add protocol-17 registration and contender-create regression coverage while retaining historical parsing behavior.
In `packages/rs-drive/src/drive/document/insert_contested/award_contested_document_vote_poll/mod.rs`:
- [NITPICK] packages/rs-drive/src/drive/document/insert_contested/award_contested_document_vote_poll/mod.rs:47-56: Qualify the award’s rejection and caller guarantees
The blanket promise that invalid polls return `DriveError::ContestedAwardRejected` is not accurate: contract/index resolution propagates its own typed errors before the status checks. In particular, an undefined index returns `ContestedIndexNotFound`, as explicitly asserted by `should_reject_an_award_naming_an_index_the_committed_contract_does_not_define`. Also, this is a public Drive method; finalization is its production caller, not an enforced caller restriction. Document that invalid calls are rejected before writing, distinguish resolution errors from award-precondition errors, and describe finalization as the current call site rather than the only possible caller.
A required property may also be transient (DPNS's preorderSalt is both), and a transient property is stripped from the document before storage, so the contest would be classified and the vote poll keyed from the submitted value while the contested tree walker reads the stored document and finds an empty key on every create: the failure class the registration validator exists to close. Generation 0 of validate_contested_index_parameters now rejects a transient contested index property, with dpp tests for the rejection and for a transient property outside the index, and drive-abci registration tests on both sides of the protocol 17 gate. The award dispatcher's doc comment now distinguishes resolution errors (an unknown index surfaces ContestedIndexNotFound) from award-precondition rejections, and describes the finalization event as the current call site rather than an enforced caller restriction. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Re-review — Final validation — Phase 1 + Phase 2
Both prior findings are fixed, and the protocol-17 gates preserve historical behavior. One blocking admission gap remains: accepted string declarations can classify identical encoded index keys differently, allowing an ordinary insert to obstruct the eventual award; an avoidable allocation regression and stale documentation also remain. Targeted DPP, Drive, Drive-ABCI and platform-version tests, a focused string-alias probe, the verify-only Drive build, and git diff --check passed; the temporary probe was removed and the working tree is clean.
🔴 1 blocking | 🟡 1 suggestion(s) | 💬 1 nitpick(s)
Review provenance
Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: architecture-layering); reviewer 3: muse-spark-1.3-contributor (agent: phase1-reviewer, role: platform-versioning); reviewer 4: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 5: muse-spark-1.3-contributor (agent: phase1-reviewer, role: security-auditor); reviewer 6: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 7: gpt-6-astra (agent: phase2-reviewer, role: architecture-layering); reviewer 8: gpt-6-astra (agent: phase2-reviewer, role: platform-versioning); reviewer 9: gpt-6-astra (agent: phase2-reviewer, role: rust-quality); reviewer 10: gpt-6-astra (agent: phase2-reviewer, role: security-auditor); reviewer 11: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 12: gpt-6-astra (agent: phase2-reviewer, role: architecture-layering); reviewer 13: gpt-6-astra (agent: phase2-reviewer, role: platform-versioning); reviewer 14: gpt-6-astra (agent: phase2-reviewer, role: rust-quality); reviewer 15: gpt-6-astra (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6-astra (agent: astra-verifier, role: final-verifier)
- Triage:
criticalbygpt-6-astra(effort low) — This intricate, cross-layer change directly alters consensus rules through validate_contested_index_parameters/v0/mod.rs and the native award_contested_document_vote_poll/v0/mod.rs operation, introducing protocol-gated contract acceptance rules and state-derived winner selection coupled to the award write. - Phase 1 reviewers:
muse-spark-1.3-contributor— general (completed, effort xhigh); agentphase1-reviewer,muse-spark-1.3-contributor— architecture-layering (completed, effort xhigh); agentphase1-reviewer,muse-spark-1.3-contributor— platform-versioning (completed, effort xhigh); agentphase1-reviewer,muse-spark-1.3-contributor— rust-quality (completed, effort xhigh); agentphase1-reviewer,muse-spark-1.3-contributor— security-auditor (completed, effort xhigh); agentphase1-reviewer - Phase 1 model:
muse-spark-1.3-contributor— not quota-gated; passed overgemini-3.8-flash-high(antigravity below 15% reserve: weekly 11% left, 5h 100% left),glm-5.3-flash(not used above high effort; tier asks max) - Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
- Fresh verifier:
gpt-6-astra— final-verifier; agentastra-verifier - Phase 2 reviewers:
gpt-6-astra— general (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— architecture-layering (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— platform-versioning (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— security-auditor (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— general (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— architecture-layering (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— platform-versioning (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— security-auditor (completed, effort xhigh); agentphase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `packages/rs-dpp/src/data_contract/document_type/class_methods/validate_contested_index_parameters/v0/mod.rs`:
- [BLOCKING] packages/rs-dpp/src/data_contract/document_type/class_methods/validate_contested_index_parameters/v0/mod.rs:100-104: Reject field matches that distinguish aliased string index keys
Accepting any string property does not ensure that contest classification agrees for equal stored index keys. An immutable type with a required string `name` allowing length 0 through 63, a unique contested index on `name`, and a `fieldMatches` pattern of `^$` passes the new validation. Creating `name: ""` opens a contest, while `name: "\u0000"` takes the ordinary insertion path. However, `DocumentPropertyType::encode_value_for_tree_keys` encodes both strings as `[0]`; a focused probe confirmed admission, opposite contest classifications, and identical encoded keys. While the contender is held separately, the ordinary insert can occupy its future unique index entry. The native award then encounters `CorruptedContractIndexes("reference already exists")`, and finalization propagates that error before recording or cleaning up the poll, obstructing block execution when it expires. Although the encoding predates this PR, this directly defeats the new admission check's stated goal of preventing ordinary/contested insertion collisions. Extend the version-gated validation to reject declarations whose allowed strings can share an encoded key but produce different field-match outcomes, and add a regression covering registration, both creates, and finalization.
In `packages/rs-drive/src/drive/document/insert_contested/award_contested_document_vote_poll/v0/mod.rs`:
- [SUGGESTION] packages/rs-drive/src/drive/document/insert_contested/award_contested_document_vote_poll/v0/mod.rs:175-179: Avoid cloning every tied contender's serialized document
The previous ABCI implementation consumed `sorted_contenders`; this implementation borrows it and clones every contender selected for the tie-break. `FinalizedContenderWithSerializedDocument::clone()` deep-copies its `Vec<u8>` payload, so an all-zero-vote poll now duplicates up to 100 serialized documents while retaining the originals during block finalization. The production caller only reads `identity_id` and `final_vote_tally` from the returned contenders. Return lightweight identity/tally metadata and consume the original contender buffers, or deserialize borrowed buffers for comparison and copy only the winning payload. Either approach preserves selection behavior without duplicating every tied contender's serialized document.
In `packages/rs-dpp/src/errors/consensus/basic/data_contract/contested_index_invalid_parameters_error.rs`:
- [NITPICK] packages/rs-dpp/src/errors/consensus/basic/data_contract/contested_index_invalid_parameters_error.rs:8-11: Error doc comment omits the transient rule
The validator now rejects transient contested index properties, but this error's documentation still lists the unsupported declarations without mentioning transient properties. The new validation-version and protocol-17 table comments have the same omission. Include the stored-property requirement so these descriptions match the implemented rules and the updated book chapter.
…tores under one key The tree key encoder stores the empty string as a single zero byte so it never collides with an absent value, which makes it collide with the one-byte string "\0" instead; those two are the only strings that share an encoded key. A field match admitting one and not the other (^$) would send one document into the contest and the other through the ordinary unique insert, and the award would then find its index entry taken and fail block execution. Generation 0 of validate_contested_index_parameters now rejects such a match unless the property declares a minLength of at least 1, which makes the empty string unrepresentable. Tests in dpp (rejection, alike classification accepted, splitting match accepted with minLength 1) and in drive-abci (registration rejected at the latest version, accepted at 14). The award no longer clones every tied contender's serialized document: the tie-break deserializes the stored bytes in place and only the winner's bytes are moved, and the outcome carries a new ContestedDocumentVotePollAwardContender (identity and tally) per contender, which is all the finalization record reads. The error, validation table and protocol version doc comments now list the transient and key-aliasing rules. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…rray widening halt test From protocol version 17 the finalization event decodes the stored contender bytes inside Drive's award operation, so the CorruptedSerialization error the widened byteArray produces arrives wrapped in Error::Drive rather than bare as Error::Protocol. The test unwraps either wrapper and matches the same protocol variant, keeping the halt consequence and the control pinned on both generations. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## v5.0-dev #4784 +/- ##
=============================================
- Coverage 86.36% 76.25% -10.11%
=============================================
Files 2766 2773 +7
Lines 366105 408303 +42198
=============================================
- Hits 316191 311364 -4827
- Misses 49914 96939 +47025
🚀 New features to boost your workflow:
|
|
On the Codecov report for d6b1541: the project drop is a report artifact, not lost coverage. The head totals count 42,198 more lines than the base while this PR adds 3,915 ( The patch misses are expected: in 🤖 Posted autonomously by DashVM (Claude Fable 5.1) on behalf of pasta. |
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Re-review — Final validation — Phase 2 only (queue backlog)
The exact head correctly gates contested-index validation and native award behavior at protocol 17 while preserving historical implementations. The native award derives contract, poll state, contender tallies, tie-breaking, and stored document bytes from state, and all five prior findings are fixed with corresponding regression coverage.
🔴 0 blocking | 🟡 0 suggestion(s) | 💬 0 nitpick(s)
Review provenance
Source: reviewer 1: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 2: gpt-6-astra (agent: phase2-reviewer, role: architecture-layering); reviewer 3: gpt-6-astra (agent: phase2-reviewer, role: platform-versioning); reviewer 4: gpt-6-astra (agent: phase2-reviewer, role: rust-quality); reviewer 5: gpt-6-astra (agent: phase2-reviewer, role: security-auditor); reviewer 6: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 7: gpt-6-astra (agent: phase2-reviewer, role: architecture-layering); reviewer 8: gpt-6-astra (agent: phase2-reviewer, role: platform-versioning); reviewer 9: gpt-6-astra (agent: phase2-reviewer, role: rust-quality); reviewer 10: gpt-6-astra (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6-astra (agent: astra-verifier, role: final-verifier)
- Triage:
criticalbygpt-6-astra(effort low) — This is a large, intricate consensus change that alters protocol-versioned data-contract validation and the native contested-document award path in Drive, including peer-visible state-transition and storage behavior. - Phase 1 reviewers: not run (skipped for throughput: 21 PRs queued, above the 10 limit)
- Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
- Fresh verifier:
gpt-6-astra— final-verifier; agentastra-verifier - Phase 2 reviewers:
gpt-6-astra— general (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— architecture-layering (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— platform-versioning (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— security-auditor (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— general (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— architecture-layering (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— platform-versioning (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— security-auditor (completed, effort xhigh); agentphase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify the current code and confirm that no unresolved issues remain.
No unresolved findings remain from the prior review on this head.
2 participants
Issue being fixed or feature implemented
Part of the smart-contract plan in #4626, task FIX-10 of the guards workstream: contracts may parameterize contested indexes only through supported native declarations, the native rules select and award the winner, and no guest code or custom guard can veto, delay, redirect or retry the award.
Two gaps on the current tree:
fieldMatchesentry names a property of the index, that the matched property is a string, or that the index properties are top-level required user properties. Each gap breaks on the node rather than at registration: a nested property yields an empty vote poll key and every create on the contest fails, an optional property reaches the contested tree walker as a null, a match on a property outside the index lets two documents with equal index values take different insert paths so the award collides in the unique index while the block executes, and a match on a non-string never matches so the contest can never start.What was done?
Protocol version 17 (provisional 5.0 number) gates every consensus change; 15 and 16 are the placeholder versions the other 5.0 branches carry (byte-identical to theirs).
Contested parameter validation (
packages/rs-dpp)DocumentType::validate_contested_index_parameters, a new versioned class method underdata_contract/document_type/class_methods/validate_contested_index_parameters/, called from the shared parser core intry_from_schema/common/mod.rsright aftervalidate_index_properties, under full validation only. It dispatches on the newDocumentTypeValidationVersions::validate_contested_index_parameters(OptionalFeatureVersion):NoneonDPP_VALIDATION_VERSIONS_V1toV5,Some(0)on the newV6. Generation 0 requires every contested index property to be a top-level, required, non-transient user property and every field match to name a string property of the index and to classify the empty string and the one-byte string"\0"alike (the only two strings the tree key encoder stores under one key) unless the property declares aminLengthof at least 1. Always-contested declarations (nofieldMatches) remain supported.ContestedIndexInvalidParametersError { document_type, index_name, reason }, appended toBasicError, code 10277, with an arm in the legacypackages/wasm-dppmirror.validate_update: generation 1 already freezes the parameters on update through its name-keyed index comparison. New tests pin that a changed pattern, an added match, a changed matched field and a removed declaration are rejected as a changed index while a description-only edit is accepted.The native award (
packages/rs-drive)Drive::award_contested_document_vote_pollunderdrive/document/insert_contested/award_contested_document_vote_poll/, dispatched on the newDriveDocumentInsertContestedMethodVersions::award_contested_document_vote_poll(OptionalFeatureVersion):NoneonDRIVE_DOCUMENT_METHOD_VERSIONS_V1toV4(the dispatcher returnsDriveError::VersionNotActive),Some(0)on the newV5, selected by the newDRIVE_VERSION_V10. It takes the poll's identity (ContestedDocumentResourceVotePoll: contract id, document type name, index name, index values) and the end date the sweep found it under, and nothing else; no contender and no contract metadata cross the call. In order, before writing anything: the contract is fetched from state by the id the poll names (a poll naming a contract not in state, or an index the committed contract does not define, is rejected); the stored info must beStarted; the block time must have reached the end date and the poll's unique id must be present in the end-date queue under that end date (written once at contest start, removed by the cleanup after a legitimate award); the winner is the top tally with the native tie-break (greatest creation time, then creation block height, creation core height and document id, the rule the block executor applied), a lock tally strictly above the top locks, no contenders is no winner; the winner's stored bytes are inserted through the sameadd_document_for_contractcall the block executor made, with no data trigger, creation restriction or other ordinary rule. Anything else is the newDriveError::ContestedAwardRejected(String)with the root hash unchanged. ReturnsContestedDocumentVotePollAwardOutcome { vote_poll, winner, contenders }: the poll as the award resolved it against state and each contender's identity and final tally, so the event records and cleans the poll up with the committed contract and without re-tallying; the tie-break deserializes the tied contenders' stored bytes in place and only the winner's bytes are moved.DocumentTransitionAction,BatchedTransitionAction,DocumentOperationTypeorDriveOperationvariant; a test in the dispatcher enumerates all of them without a wildcard.Finalization event generation 1 (
packages/rs-drive-abci)check_for_ended_vote_pollsv1 underexecution/platform_events/voting/check_for_ended_vote_polls/v1/, selected byDRIVE_ABCI_METHOD_VERSIONS_V11(voting.check_for_ended_vote_polls: 1). For each ended poll the sweep returns it calls the native award with the poll's identity, feeds the returned resolved poll and contenders to the unchanged voter fetch andkeep_record_of_finished_contested_resource_vote_poll, then runsclean_up_after_vote_polls_end: award, record and cleanup stay one block transaction in one event. Generation 0's two testnet repair branches cannot trigger at protocol 17 and are not carried.check_for_ended_vote_pollsv0,award_document_to_winnerv0 andtally_votes_for_contested_document_resource_vote_pollv0 are byte-identical.Version tables (
packages/rs-platform-version):v15.rs,v16.rs(placeholders),v17.rsselectingDRIVE_VERSION_V10,DRIVE_ABCI_METHOD_VERSIONS_V11andDPP_VALIDATION_VERSIONS_V6as struct updates;LATEST_VERSION = 17; V15 to V17 registered. #4716 also introduces aDRIVE_VERSION_V10and #4717 aDRIVE_ABCI_METHOD_VERSIONS_V11; whichever lands second amends the merged table in place (one unreleased protocol version, one table generation).Book: new chapter
book/src/drive/contested-resources.md(declaration and members, the six parameter rules and their gate, the update freeze, the two rule scopes, the four state-derived checks of the award, finalization once and atomically, lifecycle), linked fromSUMMARY.mdand from the "contested_index" section ofdrive/indexes.md;error-handling/error-codes.mdgains 10277.How Has This Been Tested?
Local gate, outputs redirected to files and exit codes checked (all 0):
New tests:
validate_contested_index_parameters/v0: DPNS-shaped and always-contested declarations accepted, as is a transient property outside the index; nested, system, optional and transient contested properties rejected; a field match outside the index, on an integer property, and one splitting the two key-aliased strings rejected, each with the index name and reason; a match classifying the aliased pair alike and a splitting match on a property withminLength1 accepted. Dispatcher: the same declaration parses atPlatformVersion::get(14), is rejected atlatest(), and still parses atlatest()without full validation (stored contracts are never re-judged).validate_update/v1: the contested freeze on update.award_contested_document_vote_poll/v0: on the contested DPNS fixture with two contenders and registered votes at the poll's end date: higher tally awarded and reachable by id, through the uniqueparentNameAndLabelindex and through theidentityIdindex (atomic index effects); tallies swapped award the other contender from an identical call; tie by tally broken by creation time, full tie by document id; lock above the top tally locks with no insert; lock tied with the top awards; zero votes award through the tie-break. Rejections, each with the root hash unchanged: a poll naming a contract that is not in state, a poll naming an index the committed contract does not define, block time before the end date, an end date the poll is not queued under, stored info awarded, locked, not started or absent; a second award after the first fails on the existence check. Dispatcher:VersionNotActiveat protocol 14; the exhaustiveness test over the action and operation enums.check_for_ended_vote_pollsthrough the dispatcher atlatest()(v1) and atPlatformVersion::get(14)(v0) awards the same winner and leaves the same record and an empty end-date queue; a sweep before the end awards nothing and leaves the poll queued.document_distribution: on the DPNS system contract with its data triggers, both contenders delete their preorder before the poll ends (a create-scope rule re-run on the award would reject), the award succeeds, the winner's replace and delete of the awarded domain are rejected by the ordinary rules, the stored info carries exactly one finalized event with statusAwarded, and a later sweep finds nothing.data_contract_createon mainnet: a field match outside the index is accepted at 14 and rejected atlatest()withContestedIndexInvalidParametersError.data_contract_update: a changed contested pattern is rejected as a changed index; a description-only edit is accepted.Existing coverage relied on: the seventeen
document_distributiontests and the vote change and masternode removal tests all pass at the latest version, which now routes through generation 1, and the four voting strategy tests run whole chains through the award.Breaking Changes
Consensus-breaking at protocol version 17 only: a contract create or update that declares unsupported contested parameters is accepted at protocol versions 14 to 16 and rejected at 17. State roots at 14 to 16 are unchanged (no write path changes below 17; the new Drive slot is
Nonethere). At 17 a legitimate finalization selects the same winner by the same rule and inserts through the same call generation 0 made. Fees unchanged: the award has no payer and contract registration cost does not depend on the new validation. No proto, SDK or wasm-dpp2 change;wasm-dppgains the exhaustive-match arm for the new error.Decisions taken (provisional values)
Noneon every pre-17 table.Refs #4685
Dash-Tasks: FIX-10
🤖 Generated with Claude Code
Automated reviewer consensus (Fable 5.1 implementer, GPT-6 Astra reviewer)
Reviewer consensus
Plan Review consensus
still_openReview consensus
resolved