Repository navigation
fix(platform)!: a moderator's restore must meet the type's countOf and sumOf rules (PV14) - #5335
QuantumExplorer wants to merge 3 commits into
Conversation
…d sumOf rules (PV14) A moderator's RestoreDocument puts the removed document back into every count and sum tree its type keeps, but the restore transform checked only the type's unique indexes, not its propertyConstraints. A countOf cap or a sumOf budget that other documents filled while the document was gone was exceeded once it came back, and the owner's later replaces were refused with 10422. The restore now judges the document by every propertyConstraints rule, as a create does: its own owner, the times and heights it comes back with, and the totals read (billed) as they will be once it is back (read_property_constraint_aggregates_for_restore). A broken rule refuses the restore with DocumentPropertyConstraintViolatedError (10422), paid by bumping the moderator's contract nonce like every restore refusal, in the mempool too. Moderation runs only from protocol version 14, so no released version changes. PV14 note 87 and the book describe it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…alue it is judged on Follows the shape #5330 gave the delete helper: the restore builds the document's properties as one map value once, reads its totals from it and judges the rules on it, rather than copying the properties twice. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
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 38 minutes. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (8)
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-10-08T12:43:03.844Z |
|
Waiting for bot review — coderabbitai not yet · thepastaclaw ✓. Wait for the missing reviews, or a writer can post |
|
✅ Final review complete — no blockers (commit c1899da) · triage: normal |
thepastaclaw
left a comment
There was a problem hiding this comment.
Final validation — Phase 1 + Phase 2
Verified the supplied reviewer evidence against exact head c1899da and found no actionable in-scope defects. The restore uses the document's own values, reads and bills post-restore aggregates through the existing helper, and returns a paid constraint refusal before constructing the restoration action; activation remains restricted to unreleased PV14. This was static verification only: Rust workspace tests and PR Hygiene remained pending in the supplied CI snapshot from 2026-10-08T13:14:26Z.
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: rust-quality); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
normalbygpt-6.1-sol(effort low) — The restore validation and aggregate-read changes affect consensus rules but are contained reuse of existing create-path checks, not a large or intricate critical-surface change. - 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 below 15% reserve: weekly 15% left, 5h 100% left),glm-5.3-flash(not used above high effort; tier asks max) - Fresh verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— platform-versioning (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— security-auditor (completed, effort high); agentphase2-reviewer
|
Closing as a duplicate of #5329, which makes the same fix with broader tests. The doc and ordering suggestions from this PR are in #5329 (comment). |
Basic explanation
What this does: A document type can set limits that span many documents, such as "each author may have at most 1 post" (
countOf) or "all posts together may spend at most 100" (sumOf). Creating or editing a document checks those limits. A moderator who deletes a document can later restore it, and the restore put the document back without checking those limits. If someone used the free room while the document was gone, the restore pushed the total over the limit. This change makes a restore check the limits the same way a create does, and refuses the restore when it would break one.Value: Limits a contract declares now hold after a moderator's restore. Before, a restore could leave an author with 2 posts under a limit of 1, and after that every edit the author made to either post was refused, because each edit was judged against a total that was already over the limit. A budget of 100 could reach 120 the same way.
Risks: Low. The restore reads the same totals and runs the same rule check a create already uses. Moderation is new in protocol version 14, which is not live on testnet or mainnet, so no released protocol version changes. Devnets on v5.0 betas need all nodes upgraded together.
Issue being fixed or feature implemented
A moderator's
RestoreDocument(#4885) brings a removed document back through the ordinary insert, so it re-enters every count and sum tree its type keeps. The restore transform (transform_document_restore_v0incontract_user_moderation/state/v0) checked the record, the window, the hash, the type'sttland its unique indexes, but not itspropertyConstraints. AcountOforsumOfrule reads a total from those trees, and other documents may fill the room the removed document left while it is gone. The restore then pushed the total past the rule, and each later replace of the owner's documents was refused withDocumentPropertyConstraintViolatedError(10422), since a replace is judged against the total as it stands.Observed on a devnet running 5.0.0-beta.2, and still present on
v5.0-devat 683dfc5:The same with
"budget": sumOf(post.amount) <= 100: a post spending 20 is deleted, another spends 100, and the restore leaves the total at 120.What was done?
transform_document_restore_v0(drive-abci, moderation state v0): after thettlcheck and before the unique-index check (the order a create runs them in), a type that declarespropertyConstraintshas the restored document judged by every rule, as a create judges one. The rules see the document's own owner, the times and heights it comes back with, and everycountOfandsumOftotal as it will be once the document is back.read_property_constraint_aggregates_for_restore(drive-abci, beside the batch transformer's other total reads): reads those totals as for a create (the restored document as the written version, no stored version), billed to the transition. Like the delete helper, it takes the document's properties as the one map value the rules are then judged on, so they are copied once.DocumentPropertyConstraintViolatedError(10422), the error a create or replace gets for the same rule. It is paid by bumping the moderator's contract nonce, like every other restore refusal, and the mempool refuses it too, since the moderation transform runs incheck_tx.propertyConstraintsare fixed with the type (10246). Judging them all keeps the restore's check identical to a create's.v14.rs(note 21 points to it), and the book: the restore's checks incontract-moderation.md#restoring-documents, and a "Moderator restores" entry inproperty-constraints.md.Example, the
onePostPerAuthorcontract above at step 4:And the budget contract:
In-place changes to shipped generations
None that any released protocol version reaches.
contract_user_moderationstate v0 runs only from protocol version 14:is_allowedrefuses everyContractUserModerationtransition belowCONTRACT_USER_MODERATION_INITIAL_PROTOCOL_VERSION(14), so the edited restore path is new behaviour of the unreleased version. The batch transformer'sproperty_constraint_aggregates.rsgains a new function only that path calls; nothing it already did changes.How Has This Been Tested?
New tests in
contract_user_moderation/tests/restore_property_constraints.rs(drive-abci), throughprocess_raw_state_transitionsatPlatformVersion::latest():should_refuse_a_restore_that_would_exceed_a_count_of_cap: a cap of 1 post per author. After the author writes a second post while the first is removed, the restore is refused incheck_txand in a block (paid, 10422, ruleonePostPerAuthor), the post stays gone and its record unrestored. The total read is the author's, not the moderator's. Once the author deletes the other post, the restore succeeds, and a further create by the author is refused by the cap.should_refuse_a_restore_that_would_exceed_a_sum_of_budget: a shared budget of 100. 100 + 20 is refused (paid, 10422, rulebudget); with the other post down to 80, the restore succeeds at exactly 100.Both tests fail with the new check disabled: the cap case's
check_txadmits the restore, and the budget case's restore succeeds instead of 10422.Run on the branch after merging the latest
v5.0-dev:cargo test -p drive-abci --lib: 4061 passed, 0 failed (every moderation restore test, the batchpropertyConstraintsanddeleteConstraintstests included).cargo test -p drive-abci --test strategy_testswithRUST_MIN_STACK=16777216: 112 passed, 0 failed, 4 ignored. At the default 2 MiB debug stack the binary aborts on a stack overflow inrun_chain_address_transitions, which never reaches the moderation transform (drive-abci builds at opt-level 0, so the added code only grows the restore function's own frame); several strategy tests sit at the edge of that stack on the base branch too.cargo test -p drive-abci --doc: 12 passed.cargo test -p platform-version: 25 passed, plus 2 doc tests.cargo fmt --all -- --checkandcargo clippy -p drive-abci -p platform-version --all-features --all-targets -- -D warnings: clean.Breaking Changes
Consensus change at protocol version 14 only: a moderator's restore that would break a
propertyConstraintsrule of its document type is now refused (10422, paid). Protocol version 14 is unreleased; devnets running v5.0 betas need all nodes upgraded together.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 ·
c1899da/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.