Skip to content

fix(platform)!: a moderator's restore must meet the type's countOf and sumOf rules (PV14) - #5335

Closed
QuantumExplorer wants to merge 3 commits into
v5.0-devfrom
fix/moderator-restore-property-constraints
Closed

QuantumExplorer wants to merge 3 commits into
v5.0-devfrom
fix/moderator-restore-property-constraints

Conversation

@QuantumExplorer

@QuantumExplorer QuantumExplorer commented Oct 8, 2026 •

Copy link
Copy Markdown
Member

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_v0 in contract_user_moderation/state/v0) checked the record, the window, the hash, the type's ttl and its unique indexes, but not its propertyConstraints. A countOf or sumOf rule 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 with DocumentPropertyConstraintViolatedError (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-dev at 683dfc5:

post: "onePostPerAuthor": countOf(post by $ownerId) <= 1

1. author creates post A                 count 1
2. moderator deletes A                   count 0
3. author creates post B                 count 1
4. moderator restores A                  count 2   <- accepted
5. author replaces A or B                refused 10422, until one of them is deleted

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 the ttl check and before the unique-index check (the order a create runs them in), a type that declares propertyConstraints has 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 every countOf and sumOf total 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.
  • The refusal is 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 in check_tx.
  • Every rule is judged, not only those reading a total. The others give the same answer they gave when the document was last written: it comes back byte for byte, and propertyConstraints are fixed with the type (10246). Judging them all keeps the restore's check identical to a create's.
  • PV14 note 87 in v14.rs (note 21 points to it), and the book: the restore's checks in contract-moderation.md#restoring-documents, and a "Moderator restores" entry in property-constraints.md.

Example, the onePostPerAuthor contract above at step 4:

Before: restore accepted, the author holds 2 posts, later replaces refused (10422)
After:  restore refused, paid (10422, rule "onePostPerAuthor"); A stays removed and its
        record stays unrestored. Once the author deletes B, the same restore succeeds.

And the budget contract:

Before: B spends 100, restoring A (20) is accepted, the total is 120
After:  restoring A is refused (10422, rule "budget"); with B replaced by a post spending
        80, restoring A succeeds at exactly 100

In-place changes to shipped generations

None that any released protocol version reaches. contract_user_moderation state v0 runs only from protocol version 14: is_allowed refuses every ContractUserModeration transition below CONTRACT_USER_MODERATION_INITIAL_PROTOCOL_VERSION (14), so the edited restore path is new behaviour of the unreleased version. The batch transformer's property_constraint_aggregates.rs gains 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), through process_raw_state_transitions at PlatformVersion::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 in check_tx and in a block (paid, 10422, rule onePostPerAuthor), 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, rule budget); 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_tx admits 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 batch propertyConstraints and deleteConstraints tests included).
  • cargo test -p drive-abci --test strategy_tests with RUST_MIN_STACK=16777216: 112 passed, 0 failed, 4 ignored. At the default 2 MiB debug stack the binary aborts on a stack overflow in run_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 -- --check and cargo 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 propertyConstraints rule 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:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have added "!" to the title and described breaking changes in the corresponding section if my code contains any
  • I have made corresponding changes to the documentation if needed
  • If I added or changed GroveDB structure, I described it in the area's structure.rs, regenerated grovedb-structure.json, and checked the structure viewer link posted on this pull request

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

🤖 Generated with Claude Code

PR Hygiene · c1899da

  • Bots — coderabbitai not yet · thepastaclaw ✓ — /skip-bots proceeds without the ones not yet reported
  • Self-review — post /self-reviewed
  • Build running
  • Approvals — you own every area touched; none needed

When every merge requirement is met, the PR Hygiene check passes. Reviewer limits do not block merging; other required GitHub checks and protections still apply.

QuantumExplorer and others added 3 commits October 8, 2026 13:08
…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>
@github-actions github-actions Bot added this to the v5.0.0 milestone Oct 8, 2026
@coderabbitai

coderabbitai Bot commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You'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.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository: dashpay/platform/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 89db9c75-8f0c-4701-8d3d-cedd461d357e
📥 Commits

Reviewing files that changed from the base of the PR and between ad63a48 and c1899da.

📒 Files selected for processing (8)
  • book/src/contract-keywords/property-constraints.md
  • book/src/data-model/contract-moderation.md
  • packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/mod.rs
  • packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/transformer/v0/property_constraint_aggregates.rs
  • packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/contract_user_moderation/state/v0/mod.rs
  • packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/contract_user_moderation/tests.rs
  • packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/contract_user_moderation/tests/restore_property_constraints.rs
  • packages/rs-platform-version/src/version/v14.rs
  • Autopilot · 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

📖 Book Preview built successfully.

Download the preview from the workflow artifacts.
To view locally: download the artifact, unzip, and open index.html.

Updated at 2026-10-08T12:43:03.844Z

@github-actions

github-actions Bot commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Waiting for bot review — coderabbitai not yet · thepastaclaw ✓. Wait for the missing reviews, or a writer can post /skip-bots to proceed without them; blocking findings still need addressing.
Full checklist in the description.

@github-actions github-actions Bot added the waiting-bots Waiting for the review bots to report on this head label Oct 8, 2026
@thepastaclaw

thepastaclaw commented Oct 8, 2026 •

Copy link
Copy Markdown
Collaborator

✅ Final review complete — no blockers (commit c1899da) · triage: normal

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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: normal by gpt-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); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
  • Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-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; agent sol-verifier
  • Phase 2 reviewers: gpt-6.1-sol — general (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort high); agent phase2-reviewer

@QuantumExplorer

Copy link
Copy Markdown
Member Author

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

waiting-bots Waiting for the review bots to report on this head

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants