[QuantumExplorer]

Basic explanation

What this does: When a document "consumes" a commitment (as registering a DPNS name deletes the name's preorder), the platform was reading the commitment from storage twice: once to check it, and once more to delete it, and it billed both reads. From protocol version 15 the delete reuses what the first read found.

Value: A create that consumes a commitment costs one document read less in processing fees: a DPNS v3 name create 10,660 credits less, a chooseFirst reveal 12,020 less. The same document is deleted and the same storage refund is paid. It applies to any contract whose refersTo declares consume, not only DPNS.

Risks: Low. It is a consensus change at protocol version 15 only: protocol version 14 keeps both reads and the fees v5.0-dev ships, which a test pins. The cheaper path deletes from the document and storage flags the lookup read, so a mismatch there would refund the wrong owner or remove the wrong entries; a Drive test checks its operations equal the force delete's after that read.

Stacked on #5337 (DPNS v3, also protocol version 15), which is stacked on the sync PR #5404.

Retargeted from protocol version 14 to 15

This PR first edited protocol version 14 in place. With DPNS v3 moved to v5.1-dev and protocol version 15, protocol version 14 is the shipped 5.0 line, so the single read now has its own version:

• DriveDocumentDeleteMethodVersions gains delete_consumed_document_operations, 0 in every existing table. DRIVE_DOCUMENT_METHOD_VERSIONS_V5 sets it to 1, and DRIVE_VERSION_V10, which only PLATFORM_V15 selects, uses that table.
• The lookup is gated too, not only the delete. query_documents_with_flags and query_documents run the same path query, but through different GroveDB calls (query vs query_item_value, which differ in how they treat a missing intermediate tree), so at 0 the lookup keeps its plain query_documents and its cost, also for a reveal that finds nothing.
• The v14 notes are v5.0-dev's again; protocol version 15 note 2 describes the change.

Issue being fixed or feature implemented

A create that consumes a commitment (a refersTo lookup with consume: true) read the commitment twice:

1. document reference validation found it through the lookup's index query (billed);
2. converting the create to Drive operations deleted it with force_delete_document_for_contract_operations, which reads the document again by id to know which index entries to remove and whom to refund (billed again).

What was done?

• fetch_document_through_lookup returns the document with its stored element's storage flags: at delete_consumed_document_operations 1 through query_documents_with_flags, at 0 through query_documents with no flags.
• ConsumedDocument carries the document and storage flags the lookup read, instead of only its id.
• Drive::force_delete_read_document_for_contract_operations dispatches on delete_consumed_document_operations: 0 is force_delete_document_for_contract_operations by the document's id, exactly what protocol version 14 runs; 1 is its operations less its read, with the same guards (refuses indexOnly and history-keeping types).
• AddDocumentAndDeleteConsumed deletes each consumed document through it. A contested create's consume uses the new ForceDeleteReadDocument operation, which converts exactly as ForceDeleteDocument did at 0; ForceDeleteDocument keeps serving moderator deletes and ttl expiry.

Before and after

	protocol version 14	protocol version 15 before	protocol version 15 after
DPNS name create (should_charge_a_name_at_protocol_versions_14_and_15), processing	1,846,280 (DPNS v2, no consume)	3,404,300	3,393,640
chooseFirst reveal, accepted (should_pin_the_fees_of_a_reveal_choosing_the_first_of_its_preorders), processing	3,574,720	3,574,720	3,562,700

Storage fees and refunds are unchanged everywhere. The chooseFirst reveal is pinned at both versions: at 14 it is the fee v5.0-dev ships.

In-place changes to shipped generations

• Batch state 0 (fetch_document_through_lookup): every protocol version selects it. It branches on the new slot, which is 0 in every table before protocol version 15, and at 0 it runs exactly the query it ran (same call, same arguments, cost added the same way). The returned storage flags are None there and nothing reads them.
• Document reference validation 0 and create action conversion 0: they carry the document and flags into ConsumedDocument; at 0 the delete reads only its id, as before.
• DocumentOperationType::AddDocumentAndDeleteConsumed and the contested create's consume go through force_delete_read_document_for_contract_operations, which at 0 calls force_delete_document_for_contract_operations with the same arguments the old code passed.

How Has This Been Tested?

• Drive: should_delete_a_read_document_as_a_force_delete_does_less_its_read (latest: the operations equal the force delete's after its read) and new should_delete_a_consumed_document_by_its_id_at_protocol_version_14 (the operations equal the by-id force delete's).
• should_pin_the_fees_of_a_reveal_choosing_the_first_of_its_preorders now runs at 14 and 15 (the commit and reveal fixture takes a protocol version): 3,574,720 at 14, unchanged, and 3,562,700 at 15.
• cargo test -p drive-abci --lib -- dpns_v3_tests commit_reveal consume choose_first only_when_consumed refers_to dpns: 120 passed.
• cargo test -p drive --lib -- drive::document::delete util::batch::drive_op_batch: 52 passed.
• cargo clippy -p drive -p drive-abci -p platform-version --all-targets and cargo fmt --all -- --check: clean.

Breaking Changes

Consensus change at protocol version 15: a create that consumes a commitment is charged one document read less. Nothing changes at protocol version 14 or for clients.

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 · f338eb1

• Bots — coderabbitai skipped after the window · thepastaclaw ✓
• Self-review — post /self-reviewed
• Build green
• Approvals
  • files with no dedicated owner — you own it
  • system-contracts — you own it
  • js-wasm-sdk (packages/js-dash-sdk/src/SDK/Client/Platform/methods/contracts/get.spec.ts, packages/js-dash-sdk/src/SDK/Client/Platform/methods/contracts/get.ts, packages/js-dash-sdk/src/SDK/Client/Platform/methods/names/register.spec.ts and 3 more) — shumkov
  • dpp — you own it
  • rs-drive-abci — you own it
  • rs-drive — you own it
  • rust-sdk (packages/rs-sdk/src/platform/dpns_usernames/mod.rs, packages/rs-sdk/src/sdk.rs) — lklimek or shumkov

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.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

• Source mode: Auto
• Repository: dashpay/platform
• Branch: claude/dpns-consume-single-read
• File: .coderabbit.yaml
• Commit: f338eb1
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: ccc46bd9-4f80-4ba2-8bb3-f0d1c672af92

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

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

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[thepastaclaw]

✅ Final review complete — no blockers (commit f338eb1) · triage: critical

[thepastaclaw]

Final validation — Phase 1 + Phase 2

Verified the supplied findings against head cec6d0a and the PR's base-to-head diff. No blocking defects were confirmed; the overlapping cloning findings combine into one performance suggestion, and the new deletion guards lack direct negative-path coverage. This was static verification only: no builds or tests were run, and the supplied CI snapshot shows Kotlin checks queued without completed Rust validation.

🟡 2 suggestion(s)

1 finding(s) not shown inline (the lines are not part of this PR's diff)

🟡 Suggestion: Move consumed document payloads instead of deep-cloning them
packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/document/document_create_transition.rs:58

ConsumedDocument now owns a full Document and its storage flags, so this to_vec() recursively clones each document's property map and values. The reference-validation block also clones the owned lookup result into ConsumedDocument before dropping the original. Both copies are avoidable: this converter owns mut self, and the subsequent owned document conversion discards the original consumed_documents field. The PR therefore introduces copying proportional to the consumed document bodies on every consuming create, including contracts with larger commitments than DPNS preorders. After completing the borrowed reference checks, move the lookup tuple into ConsumedDocument; add a take_consumed_documents accessor using std::mem::take and use it here, matching the existing take_* accessors.

_{source: muse-spark-1.3-contributor (phase1-reviewer: general, rust-quality); gpt-6.1-sol (phase2-reviewer: general, architecture-layering, platform-versioning, rust-quality, security-auditor)}

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: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 6: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 7: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: critical by gpt-6.1-sol (effort low) — The cross-layer change to lookup state, create conversion, and batch deletion is intricate and directly changes consensus-visible fee accounting and storage-refund inputs through force_delete_read_document_for_contract_operations_v0 and DocumentOperationType conversion.
• 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 (lane 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; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-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-drive/src/state_transition_action/action_convert_to_operations/batch/document/document_create_transition.rs`:
- [SUGGESTION] packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/document/document_create_transition.rs:58: Move consumed document payloads instead of deep-cloning them
  ConsumedDocument now owns a full Document and its storage flags, so this to_vec() recursively clones each document's property map and values. The reference-validation block also clones the owned lookup result into ConsumedDocument before dropping the original. Both copies are avoidable: this converter owns mut self, and the subsequent owned document conversion discards the original consumed_documents field. The PR therefore introduces copying proportional to the consumed document bodies on every consuming create, including contracts with larger commitments than DPNS preorders. After completing the borrowed reference checks, move the lookup tuple into ConsumedDocument; add a take_consumed_documents accessor using std::mem::take and use it here, matching the existing take_* accessors.

In `packages/rs-drive/src/drive/document/delete/delete_document_for_contract_operations/v0/mod.rs`:
- [SUGGESTION] packages/rs-drive/src/drive/document/delete/delete_document_for_contract_operations/v0/mod.rs:222-236: Add negative-path tests for the new read-document deletion guards
  The new helper independently duplicates the indexOnly and history-keeping guards from the existing force-delete path. The added operation-equivalence test uses a normal document type, so it does not verify either rejection; the existing history-deletion test exercises the old deletion path rather than this helper. Contract registration currently excludes these types from consumption, so this is a defensive API coverage gap, not a demonstrated execution defect. Add direct assert_matches! cases for the new entry point with an indexOnly type and a history-keeping type, asserting CorruptedCodeExecution and InvalidDeletionOfDocumentThatKeepsHistory respectively, to pin the guard parity promised by this PR.

[thepastaclaw]

🟡 Suggestion: Add negative-path tests for the new read-document deletion guards

The new helper independently duplicates the indexOnly and history-keeping guards from the existing force-delete path. The added operation-equivalence test uses a normal document type, so it does not verify either rejection; the existing history-deletion test exercises the old deletion path rather than this helper. Contract registration currently excludes these types from consumption, so this is a defensive API coverage gap, not a demonstrated execution defect. Add direct assert_matches! cases for the new entry point with an indexOnly type and a history-keeping type, asserting CorruptedCodeExecution and InvalidDeletionOfDocumentThatKeepsHistory respectively, to pin the guard parity promised by this PR.

_{source: muse-spark-1.3-contributor (phase1-reviewer: general, rust-quality)}

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

At the exact reviewed head, the version tables confine the lookup-and-delete optimization to protocol version 15 while preserving the historical query and by-id deletion paths; no blocking defect was confirmed. Both prior suggestions remain valid, and the new estimation branch also lacks direct equivalence coverage. Verification was static only: the supplied CI snapshot lists successful checks but contains no targeted Rust test or clippy results.

🟡 3 suggestion(s)

1 finding(s) not shown inline (the lines are not part of this PR's diff)

🟡 Suggestion: Move consumed document payloads instead of deep-cloning them
packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/document/document_create_transition.rs:58

This PR expands ConsumedDocument from an identifier and type name to an owned Document and storage flags, making this existing to_vec() deep-clone every consumed document's property tree. The converter already owns mutable self, and the subsequent owned document conversion discards the original consumed-document vector, so the copy adds allocations and CPU work proportional to the consumed payload size without preserving anything needed later. Move the vector out with mem::take, directly or through a take-style accessor; the generated operations and protocol-version-specific fees remain unchanged.

                let consumed_documents = match &mut self {
                    DocumentCreateTransitionAction::V0(action) => {
                        std::mem::take(&mut action.consumed_documents)
                    }
                };

_{source: muse-spark-1.3-contributor (phase1-reviewer: general, rust-quality); gpt-6.1-sol (phase2-reviewer: general, rust-quality)}

1 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.

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: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 6: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 7: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 10: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 11: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 12: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: critical by gpt-6.1-sol (effort low) — The intricate cross-layer change in fetch_document_through_lookup and Drive::force_delete_read_document_for_contract_operations changes consensus-visible processing fees and deletion/refund operations, with protocol-version dispatch preserving historical behavior.
• 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 14% left, 5h 100% left), 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 final gate: an independent Phase-2 review ran after iterative findings were reconciled
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-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-drive/src/state_transition_action/action_convert_to_operations/batch/document/document_create_transition.rs`:
- [SUGGESTION] packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/document/document_create_transition.rs:58: Move consumed document payloads instead of deep-cloning them
  This PR expands `ConsumedDocument` from an identifier and type name to an owned `Document` and storage flags, making this existing `to_vec()` deep-clone every consumed document's property tree. The converter already owns mutable `self`, and the subsequent owned document conversion discards the original consumed-document vector, so the copy adds allocations and CPU work proportional to the consumed payload size without preserving anything needed later. Move the vector out with `mem::take`, directly or through a take-style accessor; the generated operations and protocol-version-specific fees remain unchanged.

In `packages/rs-drive/src/drive/document/delete/delete_document_for_contract_operations/v0/mod.rs`:
- [SUGGESTION] packages/rs-drive/src/drive/document/delete/delete_document_for_contract_operations/v0/mod.rs:239-250: Cover the read-document deletion's estimation branch
  Both new operation-equivalence tests pass `&mut None`, so neither exercises this estimation branch. Unlike stateful deletion, it ignores the supplied document and flags, adds estimated layer information, and passes `DocumentEstimatedAverageSize` downstream. Extend the equivalence fixture to run with separate `Some(HashMap::new())` estimation maps for the by-id and read-document entry points. At protocol version 14, assert identical estimated operations and layer information; at the latest version, assert equivalent downstream operations and layer information with only the redundant read-cost operation removed. This directly tests the estimation contract changed by the optimization.
- [SUGGESTION] packages/rs-drive/src/drive/document/delete/delete_document_for_contract_operations/v0/mod.rs:222-236: Add negative-path tests for the new read-document deletion guards
  (existing thread: https://github.com/dashpay/platform/pull/5360#discussion_r4231399014)
  The new read-document implementation independently rejects indexOnly and history-keeping types, but both added equivalence tests use an ordinary `person` document. Existing by-id deletion tests would still pass if either guard disappeared from this implementation. Contract validation excludes these consumption targets, so this is a defensive coverage gap rather than a demonstrated production defect. Add focused tests through `force_delete_read_document_for_contract_operations` at `PlatformVersion::latest()`, asserting `CorruptedCodeExecution` and `InvalidDeletionOfDocumentThatKeepsHistory`. Exercise both stateful and estimation modes to pin these refusals before either branch runs.

[thepastaclaw]

🟡 Suggestion: Cover the read-document deletion's estimation branch

Both new operation-equivalence tests pass &mut None, so neither exercises this estimation branch. Unlike stateful deletion, it ignores the supplied document and flags, adds estimated layer information, and passes DocumentEstimatedAverageSize downstream. Extend the equivalence fixture to run with separate Some(HashMap::new()) estimation maps for the by-id and read-document entry points. At protocol version 14, assert identical estimated operations and layer information; at the latest version, assert equivalent downstream operations and layer information with only the redundant read-cost operation removed. This directly tests the estimation contract changed by the optimization.

_{source: gpt-6.1-sol (phase2-reviewer: rust-quality)}

[github-actions]

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]

Bots are done — your move: post /self-reviewed.
Full checklist in the description.