feat(platform)!: delete and erase lifecycle for keep-history documents - #4657
Open
shumkov wants to merge 42 commits into
Open
feat(platform)!: delete and erase lifecycle for keep-history documents#4657shumkov wants to merge 42 commits into
shumkov wants to merge 42 commits into
Conversation
Store immutable revisions in per-document provable count trees with composite time and revision keys, and keep the current pointer in primary storage. Select the new write, estimate, reference, and ordinary-read layout at protocol 14; preserve older replay behavior. Test would have caught this in CI: ✖ before fix, should_retain_both_replacements_in_the_same_block retained [1, 3]; ✔ after fix it retains [1, 2, 3]. cargo test -p drive --lib: 3579 passed, 5 existing ignored tests. cargo test -p drive --lib history: 84 passed.
Authenticate history pages and raw count-tree metadata separately, preserve complete cursors, and verify revision offsets and subtree absence at protocol 14. Keep released protocol selectors unchanged. Test would have caught the pointer-size underestimate: RED before fix (57 estimated bytes versus 59 serialized, then 316 versus 317 at a flags encoding threshold), GREEN after including reference hops and the flags-length allowance. Selector, absence, deep-history fee, and migration checks pass; workspace all-features clippy passes.
Move legacy revision Items into per-document provable count trees with deterministic staged batches, preserve flags and sums, rewrite index references, and retain the existing cache refresh. Add a rollback-only offline-copy inventory benchmark and an actual 13-to-14 signed-operation upgrade/restart strategy. Migration matrix and rollback/retry checks pass, including overwritten-revision gaps, empty types and deep summable histories. Upgrade strategy passes; activation dispatch, signed lifecycle writes and legacy repair tests pass. The parallel-suite repair test failed before its queries explicitly selected protocols 13 and 14 and passed after the fixture correction. Synthetic 256-revision migration: 686.34 ms in a loaded debug build; authentic testnet inventory and validator budget remain the release gate.
Add time/cursor and revision selectors to the versioned wire API and expose ordered entries with authenticated lifecycle counts across the Rust, WASM and JavaScript SDKs. Verify both GroveDB proofs and quorum signatures, including absent documents and empty pages, while preserving the legacy wire format for released protocols. Validation: signed quorum/API round trip and tampering tests pass; proof verifier 267, DPP 4114 and Rust SDK 189 tests pass; version activation/replay tests pass. WASM Rust history tests pass (2), WASM unit suites pass (424 each in Node and Chrome), and Evo SDK unit suites pass (222 each in Node and Chrome); TypeScript lint/build and workspace all-features clippy pass.
Declare each history-root child as AllProvableCountTrees so propagation includes its authenticated count. Pin the four rewritten ancestors to an extra eight bytes each in should_charge_count_tree_overhead_when_propagating_history_roots. Test would have caught this in CI: red before fix (568 replaced bytes versus 600 required), green after fix (1 passed).
Route history v1 through the four existing versioned entry points with known versions [0, 1], retaining explicit legacy adapters for replay. A legacy proof at protocol 14 now reports an unsupported query shape instead of UnknownVersionMismatch. Authenticate the latest sequence from the resolved primary document before accepting ordinal revision offsets. Both Revision(3) and StartAtRevision(3) reject retained [1,3], including a valid proof of the otherwise-empty positional page. Pin positive selector results and the sixteen-byte decoder gate. Test would have caught this in CI: should_report_legacy_history_as_unsupported_after_activation failed before dispatch routing and passed after; should_migrate_revisions_and_indexes_without_recovering_overwritten_revisions failed when fetch/prove/verify all accepted the empty gap page and passed after. All nine history tests and the migration gap regression passed in the Drive library suite. BREAKING CHANGE: SDKs older than this release cannot read keep-history documents after protocol 14 activates. History v0 and block_time_ms point-in-time reads for keep-history types are rejected at that boundary.
Enumerate contracts through ascending fetch_contract_ids pagination. Allow only the reserved document-type keys 0, 1, and 2 with their permitted shapes, or declared index roots, and reject any other child before migrating that type. Assert that each document rewrites every collected index reference. Expose the largest per-type index inventory and a conservative owned-buffer bound in the benchmark output. Structural-flag assertions now distinguish contract flags from writer flags for document trees and both populated and empty history roots. Test would have caught this in CI: should_reject_unrecognised_type_children_during_migration_inventory failed before the explicit inventory validation and passed after. All four migration tests passed, including rollback/retry, overwritten revisions, depth measurements, and distinct structural flags.
Exercise fresh and migrated two-document histories through unique, non-unique, and summable indexes, with proved and unproved reads after reopen. Pin reference paths, hops, flags, current bodies, index sums, and primary counts and sums. Execute signed replace and transfer after activation on migrated documents and verify every resulting history page and proof. Retain protocol 13 fee fixtures and add protocol 14 twins for direct, indexed, and estimated replacements with and without transactions. Remove the unused primary-path version argument and the redundant version argument in the legacy-only path-size helper. Validation: cargo test -p drive --lib passed 3596 tests with 5 existing ignored tests; should_read_and_prove_fresh_and_migrated_history_through_every_index_kind and all three should_pin_protocol14_history tests passed. cargo test -p drive-abci --lib keep_history passed all 4 tests, including should_replace_and_transfer_migrated_history_through_signed_transitions. These are coverage additions and behavior-preserving helper cleanup, so no production-fix red-to-green claim is made.
Describe structure B, the composite time/sequence key, transactional migration, and history v1 in the protocol 14 documentation. Correct the Drive, ABCI methods, query, and verification annotations. Documentation only; the existing protocol 12/13 selections and protocol 14 activation slots are unchanged, so no behavioral red-to-green test applies. BREAKING CHANGE: SDKs older than this release cannot read keep-history documents after protocol 14 activates. History v0 and block_time_ms point-in-time reads for keep-history types are rejected after activation.
Add DocumentHistoryProofInfo with the verified page, original v1 response, and an independent verification method. Rust Fetch and mock serialization preserve the complete response; WASM and Evo SDK return a history-specific result with separate entriesProof and metadataProof fields. Execute the actual ABCI history handler with prove: true against signed committed-block metadata, and verify its response unchanged for populated pages, empty selections, and authenticated absence. Re-verify the exported proof-info response without obtaining any missing proof from the original request flow. Test would have caught this in CI: should_round_trip_history_api_through_quorum_and_grove_proof_verification failed before the fix because the exported generic proof omitted the entries proof, then passed with DocumentHistoryProofInfo. All three history_wasm_tests passed, including should_export_both_history_proofs_without_combining_them. BREAKING CHANGE: getDocumentHistoryWithProofInfo now returns a history-specific result containing entriesProof and metadataProof instead of the generic single proof field. SDKs older than this release cannot read keep-history documents after protocol 14 activates.
Run the fresh/migrated index and signed-action matrices for SumTree as well as CountSumTree primary storage. Assert exact sums after create, replace, transfer, price update, and purchase, and exact counts where the primary tree is countable. Validation: should_read_and_prove_fresh_and_migrated_history_through_every_index_kind passed all four fresh/migrated and countable/non-countable variants; cargo test -p drive-abci --lib keep_history passed all 4 tests with the expanded signed-action variants. These are protocol 14 coverage twins for the retained protocol 13 fixtures, with no production behavior change and therefore no production-fix red-to-green claim.
Document that migration errors deliberately halt the activation block for all validators and that its transaction cannot commit partial migration. Valid pre-14 state cannot reach the migration corruption checks. No behavior changes; regression tests are not applicable to source comments. The untracked PR draft carries the authentic-state release blocker checklist.
Require the GroveDB v1 envelope for both entries and metadata proofs in Drive and the SDK verifier. Legacy envelopes return an unsupported proof version error because they do not hash-bind the terminal count-tree metadata. Keep legacy history v0 verification unchanged. Regression tests were written and run before the version check: RED -> GREEN: should_migrate_revisions_and_indexes_without_recovering_overwritten_revisions rejects legacy metadata envelopes for both revision selectors on retained [1,3], including a terminal count that still yields the committed root. RED -> GREEN: should_reject_legacy_proof_envelopes_in_history_v1_through_sdk rejects the same legacy format through signed response verification. RED -> GREEN: should_round_trip_history_api_through_quorum_and_grove_proof_verification rejects legacy entries envelopes while valid history v1 responses verify. BREAKING CHANGE: SDKs older than this release cannot read keep-history documents after protocol 14 activates.
Expose contract-enumeration operation costs through the existing versioned helper and accumulate every page, including the terminal empty page. Preserve the compatibility adapter and the older migration caller. Replace the self-derived rewrite-length comparison with stored-reference readback against each document's inventory. Compare targets, hop limits, flags and sums, and halt activation if the match count differs. Include readback costs in statistics and document transactional halt semantics. RED -> GREEN: should_account_for_contract_enumeration_even_when_no_contracts_exist previously reported zero work for a nonzero-cost terminal page. RED -> GREEN: should_halt_migration_when_an_inventoried_index_reference_was_not_rewritten failed against a behavior-preserving extraction of the old length check; readback now rejects the unchanged reference and accepts the migrated one. Pin incremental loaded bytes for 256 versus 16 revisions independently of fixed inventory overhead, and retain migration/index/retry regressions.
The latest-version integration test freezes roots for the legacy storage layout and expects a blockTime read to succeed. Preserve every assertion under protocol 13, and add a protocol-14 twin covering its composite root, all ten current documents, primary references, four retained timestamps, history proofs and the explicit point-in-time rejection. RED -> GREEN: cargo test -p drive --test query_tests_history previously failed at the legacy root assertion (2 passed, 1 failed). The protocol-13 replay and protocol-14 twin now pass with the original first-version test and setup test (4 passed). No production behavior was changed.
Return Error::Generic when a caller's history limit cannot fit in u16, so the WASM SDK does not describe invalid input as a malformed proved response. Keep the existing message and selector validation. RED -> GREEN: should_report_oversized_history_limits_as_request_errors was written first and failed for the old InvalidProvedResponse mapping. It now passes for 65536 and u32::MAX; all three history_query_tests pass.
Match the enclosing verification module's server-or-verify feature gate for the v1 implementation, its envelope validator and public wrapper. Default drive-abci builds must not rely on test dependency feature unification to make the dispatcher target available. RED -> GREEN: cargo build --package drive-abci --locked reproduced E0599 for the missing verify_document_history_v1_impl before the change and now succeeds. Independent no-default-feature checks for server and verify also pass. This changes compile-time availability, not protocol behavior.
Assert that the retained source revision count equals the destination history length before comparing revision bytes and beneficiary flags. This prevents zip from silently accepting a shortened destination. This strengthens existing migration coverage without changing production behavior; no red-to-green claim applies. The full Drive suite exercises this assertion through the summable-history migration fixtures.
Contributor
|
Important Review skippedToo many files! This PR contains 194 files, which is 94 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (194)
You can disable this status message by setting the 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 |
Collaborator
|
⛔ Final review complete — 1 blocking finding(s) (commit 6e27f84) · triage: critical · Phase 2 only (queue backlog) |
Protocol version 14 gains the slots the delete and erase lifecycle of keep-history documents needs: the erase transition's wire bounds, its structure and state validation, the lifecycle read every stateful check goes through, the keep-history branch of the document delete, the erase storage operation and its estimation, and the bound on how many retained revisions one erase transition may remove. Every new field is dormant in the released tables and in the hand-written mock, so no released protocol version changes behaviour. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Generation 3 of the document type parser no longer refuses documentsKeepHistory together with canBeDeleted. A delete on such a type removes the document from ordinary reads and leaves its retained revisions readable, so the two settings no longer contradict each other. It also admits canBeErased, which additionally allows a deleted document's revisions to be purged. The flag defaults to false, requires both documentsKeepHistory and canBeDeleted, and is immutable across contract updates: narrowing it once an erasure had begun would strand a partially erased document forever, and widening it would hand an irreversible operation to a type registered without it. The existing canBeDeleted narrowing stays, except on an erasable type, which could never reach a state erase acts on again. A keep-history type may not carry a contested index: a contested resource is awarded outside transition validation, at an id derived from the winner rather than from the contested values, and that award can land on an id whose retained history already exists. BREAKING CHANGE: at protocol version 14 a contract may combine documentsKeepHistory with canBeDeleted, which earlier drafts of that protocol version rejected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
DocumentTransition::Erase is appended after IndexOnlyDelete so every existing variant keeps its bincode discriminant, and carries only the base transition: which chunk it removes, and whether it starts or continues an erasure, are read from the document's committed lifecycle rather than signed by the submitter. Its JSON action is "erase" and it maps to a new DocumentTransitionActionType. The kind is gated on document_erase_state_transition, which is None before protocol version 14, so a node running an older protocol version refuses it with UnsupportedVersionError and agrees with software that cannot decode it at all. A result proof for an erase shows the document absent by id, which it already was before the transition ran, so the proof classifier reports the outcome as affected state rather than proved execution. A subscription filter never matches an erase: it carries no document values and acts on a document no document query can see. BREAKING CHANGE: DocumentTransition and DocumentTransitionActionType each gain a variant, so exhaustive matches over them must be extended. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Deleting a keep-history document now removes its current pointer and the index references that lead to it, leaves every retained revision where it is, and writes a lifecycle record naming the block time it was deleted at. The record lives in a per-type tree at a reserved key next to the history tree, is created on demand so that a type whose documents are never deleted never pays for one, and carries the deleter's flags so its bytes come back to them. Non-keep-history deletes are delegated to the previous implementation unchanged. Erasing removes a bounded chunk of an already deleted document's revisions, newest first, so a partial erasure leaves the oldest content and a contiguous sequence behind. The enumeration asks for one revision more than a chunk may remove, so the operation knows before emitting anything whether it is the last: a terminal chunk also removes the record and the now empty history subtree, which GroveDB accepts only because the revision deletes are already in the same batch, while a non-terminal first chunk instead overwrites the record with the erasure it authorizes. The two never coincide, so one batch never carries two operations on the record's key. A document whose revisions are still retained cannot be created again: the storage writer refuses an insert whose history subtree already exists, which covers writers that never pass through transition validation. A dry run skips the probe and pays for it as a fixed cost so estimation and execution stay aligned. History v1 gains the deleted and erasing states, derived identically in the fetch and the verifier from the same authenticated record. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The erase transition action mirrors the delete one: only a base, no token cost of its own, and a nonce bump alongside the storage operation it converts into. It is appended to the document action enum and to the action type mapping. The parser tests are rewritten around the rule generation 3 now applies: a keep-history type may allow deletion, may additionally allow erasure, may not carry a contested index, and may withdraw deletion only while it is not erasable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…erasing Deleting a keep-history document now consults its lifecycle rather than the ordinary by-id read, which cannot see a document that has already been deleted. A second delete is therefore a paid consensus error rather than a delete of something that never existed, and no delete has any path to removing a revision. Erasing is authorized once. Its structure validation checks what the contract alone decides: the type retains revisions, allows them to be purged, carries no contested index, and offers no token payment, which erase has no use for since the deletion it follows was already charged. Its state validation then reads the document's lifecycle: a current document must be deleted first, a merely deleted one may be committed to erasure only by its owner, and an erasure already committed may be continued by any identity, because the record left in state is itself the evidence that destruction was authorized. An owner who loses their keys, their funds or their permission can no longer strand a half-erased document. Creating over a deleted document's id is refused: the id stays reserved while its revisions are retained, and becomes free again once an erase has removed the last one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The history response's lifecycle block gains the two new states and the four times the lifecycle record carries. The proof verifier checks every one of them against the proof, so a node cannot claim a deletion time or an erasure that its own state does not authenticate. The batch action enums in both WebAssembly bindings gain the erase kind, keeping the two hand-written discriminant tables in agreement. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The batch insert has no support for an element of unknown size, so a dry run of a keep-history delete failed outright. It never needed one: the record is a fixed-width encoding of the deletion time and the deleter is an input to the delete, so both runs write the same element. The erase dry run, which genuinely cannot know the record it overwrites, prices one of the right shape instead. Test: at protocol 14 a keep-history delete now runs end to end through the batch pipeline, leaving the document invisible to ordinary reads and its retained revisions readable. Red before this fix with "inserting unsized documents into a batch is not currently supported", green after. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Rust builder mirrors the delete one, minus the token payment erase has no use for, and the SDK method broadcasts one chunk. Its result says only that the erase was accepted: the proof authenticates that the document is absent by id, which it already was before the erase ran, so it is evidence about the state rather than about this transition having executed. A separately named lifecycle read answers how much history is left, and is named apart on purpose, because its answer describes committed state at the moment of the read and another erase or a re-create may land in between. The WebAssembly and JavaScript SDKs pass the same identifiers through to it; a document that is no longer visible has no values a client could carry anyway. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Covers what a delete leaves behind and what an erase removes: the document leaves ordinary reads while every revision stays readable and provable, a history shorter than a chunk goes in one transition, a longer one is committed to erasure and finished by the chunks after it, and the record those chunks read is written once and never rewritten. Also pinned: the id stays reserved until the last revision is gone and is reusable afterwards, the history subtree cannot be dropped while a revision survives, each revision is refunded to whoever wrote it, and the admission estimate covers a full chunk whatever the document actually retains. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Covers the validation rules end to end at protocol 14: delete then erase, a second delete refused, a create over a reserved id refused, an erase of a document that has not been deleted refused, an erase of a type that never asked for erasure refused, an erase carrying token payment information refused, and an erase start by an identity that does not own the document refused. The wire gate is pinned at protocols 12 and 13, where the erase kind does not exist, and the version slots carry semantic pins naming what each number selects rather than only its value. The token payment refusal moved from the erase structure validation to the transformer. The base action keeps only the cost the contract sets, not the payment the submitter offered, so by the time structure validation runs there is nothing left to see and the check could never have fired. Red before the move, green after. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A lifecycle record says an erasure has begun by carrying the revision it began from and the time it began at. Reading that off the time made a block whose clock reads zero indistinguishable from a document whose erasure had never started, so the first chunk of such a block committed nothing and every chunk after it re-authorized as a fresh start. A history sequence is one or more for every real revision, so zero there can only mean the field was never written. Test: a document retaining more than one chunk is committed to erasure by its owner and finished by an identity that owns nothing, and a delete of it in between is refused. Red before this fix, where the erase start left the record saying merely deleted, green after. Also covered: two erases sharing one block, and a delete and the erase that follows it sharing one block, each seeing the previous transition's effect through the block transaction. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The protocol 13 to 14 upgrade test now deletes and erases the document the activation migrated, so the lifecycle is pinned against revision keys and trees the migration produced rather than only against history written after activation. A contract registered before protocol 14 cannot declare canBeErased, since the grammar that admits the keyword did not exist then, so this exercises the storage operations directly. A functional spec covers the whole story against a running network: register a keep-history erasable contract, edit a note into a history, delete it and watch it leave ordinary reads while its revisions stay readable, fail to delete it twice, then erase it. Its erase and history calls go through the Evo SDK, which is where those surfaces live. The stale reference-validation comment is corrected: a contract update may withdraw deletion but never grant it, which is what keeps a validated reference from ever dangling. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A millisecond timestamp does not survive a JavaScript number, so every lifecycle time crosses the boundary the same way the counts and revisions beside it already do. The TypeScript declaration names the two new states and the four times a caller can now read. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The counted and summed entry of a keep-history type is its current pointer, so removing it decrements both aggregates by construction and a deleted document stops contributing without any bookkeeping of its own. Its index entries go with it, freeing the values it held for another document to take, while every revision stays where it is. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An erase acts on a document that was already deleted, so its by-id proof shows the same absence whether or not it ran, and the proof classifier reports it as affected state. The strict wait refuses exactly that, so every valid erase came back to the caller as ExecutionNotProved: the transition had executed, revisions were gone, and the SDK reported a failure. Retrying on that error submits another chunk or hits not found after a terminal one. It now takes the affected-state wait, and the result is named for what it is: the document is absent by id as of the proof's block, which it already was, so it is an observation of the state the erase affected rather than evidence that this erase executed. The lifecycle read stays the way to learn how much history is left. Test: the outcome an erase produces, driven through both gates. Red before the fix, where the strict wait the erase used refuses its own valid outcome; green after. A companion test pins the classification itself, asserting an erase is affected state while a delete over the very same proof stays execution-proved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The two new cross-flag guards, erasure asked for by a type with no history or no deletion and a contested index on a type that keeps history, returned a bare data-contract error. Only the consensus variant becomes a paid rejection with a nonce bump when a signed contract create or update carries the combination; the bare one escapes the transformation as an internal execution error, so the submitter pays nothing, learns nothing, and can replay the same transition. Test: a signed create and a signed update whose wire schema is rewritten into each refused combination, which is all a client has to do since the parser will not build one. Both must be invalid-paid with the nonce bump persisted. Red before the fix, where the create is classified as an internal error and no nonce is written; green after. The parser tests now also assert the exact error variant rather than only the message. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…imate The per-type lifecycle container was being flagged to whoever deleted first. It is shared by every document of the type and outlives all of them, since an erase removes records and per-document history trees and never this, so flagging it charged the first deleter for structure nobody refunds and made the committed encoding depend on which document went first. It is unflagged now; the record inside it keeps the deleter's flags, which is the byte an erase does refund. Its insert also now consults the running batch, so two deletes of different documents of the same type prepared before either is applied insert the shared key once rather than twice. The erase estimation slot was declared in every version table but never read: the implementation was called directly, so selecting an unsupported version silently kept using the old estimate. It has the normal dispatcher now, with the body in v0 and an unknown-version refusal. Tests: the raw container element is unflagged while the record names its deleter; only the first delete of a type pays for the container; and a table selecting an unimplemented estimation version is refused. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Removing revisions credits whoever paid for each of them, and those balance updates are applied after the transition's fee result is formed, against identities that had nothing to do with it. The chunk bound limits how many there can be but pays for none of them, so a small erase could demand a hundred balance reads and writes that appeared in neither its admission accounting nor its charged work. A Drive helper now prices that work: it measures one balance update through the ordinary estimation path, which mutates nothing and touches no real identity, and scales it by the beneficiaries one erase can credit -- a full chunk of revisions plus the lifecycle record, the per-document history subtree and the current pointer, all of which a terminal chunk removes. Erase validation bills it through the execution context, so it reaches both the estimate that admits the transition and the fee actually charged. Test: an erase's charged fee and its admission estimate must both cover the priced recipient work. Red before this change, where the whole erase costs 1,149,580 against 450,944,300 of unpriced recipient work; green after. A Drive test pins that the price scales with the bound and creates no balance of its own. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The erase estimate now prices what the enumeration actually does. GroveDB exposes no key-only result shape over a range, so the revision bodies come back whether or not the caller wants them; the estimate charges for one revision more than the chunk removes, at the full body size, and for the record read a non-terminal chunk performs. The comment says so rather than claiming a key-only read. New evidence: refunds are keyed under the epoch each byte was written in, with two writers six epochs apart compared like for like; a refund is a credit a beneficiary's balance actually gains; a one-revision erase is admitted only against the full chunk estimate; an erase commits the same state and costs the same with GroveDB's batch consistency checking on and off; the erasing state survives the proof round trip for populated, empty and by-revision pages, as does the terminal absence, and a claim that differs in any of the four times or in the count is not what the verifier derives; revisions still number from one after a partial erasure; and a removed revision is not answered with its neighbour. At the transition level: refused erases persist their nonce bumps on both refusal paths, a create of a deleted id is refused when it shares the delete's block, and a replace of a deleted document is not found. Two deletes of one type combined into a single batch before either is applied both emit the shared container's insert, because each document operation is converted without sight of its siblings. GroveDB refuses the batch rather than committing one of them, and that fail-closed behaviour is now pinned along with the one-transition cap that keeps consensus away from the shape. Also: the indexOnly parser documentation is back on the indexOnly parser, the erase converter names where the token offer is really refused, an error message loses a run of literal spaces, and the unreachable history-metadata arm is gone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
shumkov
force-pushed
the
keep-history-lifecycle
branch
from
September 11, 2026 00:47
492f4b5 to
feea7d5
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## v4.2-dev #4657 +/- ##
============================================
- Coverage 85.34% 82.74% -2.61%
============================================
Files 2795 2827 +32
Lines 373566 388057 +14491
============================================
+ Hits 318827 321082 +2255
- Misses 54739 66975 +12236
🚀 New features to boost your workflow:
|
The suite drives writes through the legacy client, which exposes no getNetwork, so the browser runner threw a TypeError out of the before-all hook and every case in the spec failed before it ran. Read NETWORK from the environment instead, the way the other functional specs that reach for the Evo SDK already do. dotenv-safe requires the key and karma's EnvironmentPlugin copies it into the bundle, so the one expression serves both the node and the browser runner. The failure was a TypeError in the hook rather than a wrong value, so there is no behaviour to pin: the assertion that catches a regression here is the hook completing at all, which the suite run itself is. Verified with yarn workspace @dashevo/platform-test-suite lint, whose only complaint about this file is the unresolved "dash" import that every functional spec reports in a worktree where the legacy SDK is not built. The suite itself needs a running network and was not run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… replace The legacy SDK signs revision + 1 from the document object it is handed and never bumps that local copy, so a second replace of the same object resent revision 2 and Drive rejected it (both the node and the browser suite failed on the first KeepHistoryDocument case). Refetch the stored document before each edit, the way Document.spec.js already does. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The erase helper signed with identity key 0, the MASTER key, and Drive rejected it: document transitions require a HIGH or CRITICAL key. Use key 1, the HIGH authentication key the legacy SDK registers. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…efore erasing The browser suite timed out on the erase: the Evo SDK fetched the identity contract nonce from a node that had not yet committed the block holding the refused delete, reused nonce 6, and that node's mempool dropped the erase as a duplicate sender while the broadcast still reported success. Every other broadcast in the spec is followed by waitForSTPropagated(); the refused delete was the one exception. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
thepastaclaw
suggested changes
Sep 11, 2026
thepastaclaw
left a comment
thepastaclaw
left a comment
Collaborator
There was a problem hiding this comment.
Final validation — Phase 2 only (queue backlog)
The lifecycle and erasure implementation is generally well covered, but two correctness issues remain. The WASM/JavaScript API cannot represent the documented continuation case where an identity other than the document owner submits a later erase chunk, and the history proof verifier accepts incomplete metadata proofs that can be interpreted as false lifecycle states.
🔴 1 blocking | 🟡 1 suggestion(s)
Review provenance
Source: reviewer 1: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 2: gpt-6-astra (agent: phase2-reviewer, role: ffi-engineer); reviewer 3: gpt-6-astra (agent: phase2-reviewer, role: rust-quality); reviewer 4: 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 cross-cutting change that directly modifies consensus validation and state-transition serialization, funds accounting/refunds, peer-facing protobuf/query deserialization, and storage migration/lifecycle deletion behavior. - Phase 1 reviewers: not run (skipped for throughput: 15 PRs queued, above the 10 limit)
- Fresh verifier:
gpt-6-astra— final-verifier; agentastra-verifier - Phase 2 reviewers:
gpt-6-astra— general (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— ffi-engineer (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/wasm-sdk/src/state_transitions/document.rs`:
- [SUGGESTION] packages/wasm-sdk/src/state_transitions/document.rs:597-625: Expose the erase submitter separately from the document owner
The WASM wrapper derives `owner_id` from the target document (`doc_inner.owner_id()` or the plain object's documented `ownerId`) and passes it as the final argument to `DocumentEraseTransitionBuilder::new`. On the Rust side, that field is the identity submitting and paying for the erase: it is used to fetch the contract nonce and to construct the outer batch transition. Therefore a continuation signed by an unrelated identity is built as though it were submitted by the original document owner, so the signer and transition owner do not match and validation rejects the continuation. Add a separately named submitter or payer identity to `DocumentEraseOptions`, use it for the builder and nonce lookup, and retain the document owner only as target metadata where needed. Defaulting the submitter to the document owner can preserve the first-owner call.
In `packages/rs-drive/src/drive/document/history/mod.rs`:
- [BLOCKING] packages/rs-drive/src/drive/document/history/mod.rs:569-575: Require absence proofs for lifecycle metadata lookups
`metadata_query` performs exact-key lookups for the current document pointer, the lifecycle record, and the per-document history tree, but verification sets `absence_proofs_for_non_existing_searched_keys` to `false`. With that option, GroveDB may return no row for a requested exact key without proving that the key is absent. `query.lifecycle()` then treats omitted metadata as absent and can derive `ABSENT`, `DELETED`, or another incorrect lifecycle state even when the authenticated root contains the omitted pointer or lifecycle record. A compromised DAPI server can consequently make clients accept a false lifecycle result. Verify this metadata query with absence proofs enabled and give the merged query a finite limit sufficient for all three exact terminal-key lookups.
Comment on lines
+597
to
+625
| ( | ||
| doc.id().into(), | ||
| doc_inner.owner_id(), | ||
| doc.data_contract_id().into(), | ||
| doc.document_type_name(), | ||
| ) | ||
| } else { | ||
| ( | ||
| IdentifierWasm::try_from_options(&document_js, "id")?.into(), | ||
| IdentifierWasm::try_from_options(&document_js, "ownerId")?.into(), | ||
| IdentifierWasm::try_from_options(&document_js, "dataContractId")?.into(), | ||
| try_from_options_with(&document_js, "documentTypeName", |v| { | ||
| try_to_string(v, "documentTypeName") | ||
| })?, | ||
| ) | ||
| }; | ||
|
|
||
| let identity_key_wasm = IdentityPublicKeyWasm::try_from_options(&options, "identityKey")?; | ||
| let identity_key: IdentityPublicKey = identity_key_wasm.into(); | ||
| let signer = IdentitySignerWasm::try_from_options(&options, "signer")?; | ||
| let data_contract = self.get_or_fetch_contract(contract_id).await?; | ||
| let settings = | ||
| try_from_options_optional::<PutSettingsInput>(&options, "settings")?.map(Into::into); | ||
|
|
||
| let builder = DocumentEraseTransitionBuilder::new( | ||
| Arc::new(data_contract), | ||
| document_type_name, | ||
| document_id, | ||
| owner_id, |
Collaborator
There was a problem hiding this comment.
🟡 Suggestion: Expose the erase submitter separately from the document owner
The WASM wrapper derives owner_id from the target document (doc_inner.owner_id() or the plain object's documented ownerId) and passes it as the final argument to DocumentEraseTransitionBuilder::new. On the Rust side, that field is the identity submitting and paying for the erase: it is used to fetch the contract nonce and to construct the outer batch transition. Therefore a continuation signed by an unrelated identity is built as though it were submitted by the original document owner, so the signer and transition owner do not match and validation rejects the continuation. Add a separately named submitter or payer identity to DocumentEraseOptions, use it for the builder and nonce lookup, and retain the document owner only as target metadata where needed. Defaulting the submitter to the document owner can preserve the first-owner call.
source: gpt-6-astra (phase2-reviewer: ffi-engineer)
Comment on lines
+569
to
+575
| absence_proofs_for_non_existing_searched_keys: false, | ||
| verify_proof_succinctness: true, | ||
| include_empty_trees_in_result: true, | ||
| }, | ||
| &version.drive.grove_version, | ||
| )?; | ||
| let (lifecycle, present) = query.lifecycle(metadata, document_type, version)?; |
Collaborator
There was a problem hiding this comment.
🔴 Blocking: Require absence proofs for lifecycle metadata lookups
metadata_query performs exact-key lookups for the current document pointer, the lifecycle record, and the per-document history tree, but verification sets absence_proofs_for_non_existing_searched_keys to false. With that option, GroveDB may return no row for a requested exact key without proving that the key is absent. query.lifecycle() then treats omitted metadata as absent and can derive ABSENT, DELETED, or another incorrect lifecycle state even when the authenticated root contains the omitted pointer or lifecycle record. A compromised DAPI server can consequently make clients accept a false lifecycle result. Verify this metadata query with absence proofs enabled and give the merged query a finite limit sufficient for all three exact terminal-key lookups.
source: gpt-6-astra (phase2-reviewer: security-auditor)
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
2 participants
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.
Issue being fixed or feature implemented
Document types with
documentsKeepHistory: truestore every revision, and untilnow their documents could not be removed at any layer. The shared-editing ACL
work requires history on shared-editing types, so without a lifecycle every such
document would be permanent.
This adds one: a delete removes a keep-history document from ordinary reads and
leaves its revisions readable, and a separate, separately permitted erase purges
those revisions when they are no longer wanted. It supersedes the product rule of
#4218, which treated
documentsKeepHistoryandcanBeDeletedas a contradictionbecause the storage layer refused the delete. The storage layer no longer does.
What was done?
Contract grammar. A keep-history type may allow deletion. It may
additionally set the new
canBeErased, which requires history and deletion, isimmutable across updates, and defaults to false. A keep-history type may not
carry a contested index: a contested resource is awarded outside transition
validation, at an id derived from the winner rather than from the contested
values, and that award can land on an id whose retained history already exists.
Four states. Active while the document is visible; Deleted once the current
pointer and its index references are gone and a lifecycle record naming the
deletion time stands in their place; Erasing once an authorized erasure has
begun; Absent when nothing is left and the id is free again. The record lives in
a per-type tree beside the history tree and exists exactly while a document is
deleted or erasing.
Delete never escalates. It has no code path that removes a revision, so a
second delete is a paid consensus error rather than a deeper removal. A document
that has been deleted keeps its id: creating over it is refused both in
transition validation and in the storage writer, which is what covers writers
that never pass through validation at all.
Erasure is authorized once. The owner commits the document to erasure; the
record that commitment leaves in state is then the evidence that destruction was
authorized, so any identity may submit the remaining chunks and pay for them. An
owner who loses their keys, their funds or their permission cannot strand a
half-erased document. One transition removes at most
max_document_revisions_erased_per_transitionrevisions, newest first, so apartial erasure leaves the oldest content and a contiguous sequence behind; the
terminal chunk also drops the record and the now empty history subtree, which
GroveDB accepts only because the revision deletes are in the same batch.
Observability.
getDocumentHistoryresponse v1 reports the two new statesand the four times the record carries, derived identically in the fetch and the
verifier, and the proof verifier checks every field against the proof.
Clients. rs-sdk gains
DocumentEraseTransitionBuilder,Sdk::document_eraseand a separately named
Sdk::document_current_lifecycle; wasm-sdk gainsdocumentErase; js-evo-sdk gainsdocuments.erase(). The erase's own proofshows the document absent by id, which it already was before the erase ran, so
it is classified as affected state rather than proved execution. That is why the
SDK takes the affected-state wait: the strict wait refuses exactly this
classification, and would return an error for an erase that had executed. The
result is named for what it authenticates, and the lifecycle read is offered
under its own name.
Accounting. Removing revisions credits whoever paid for each of them, in
balance updates applied after the transition's fee result is formed against
identities that had nothing to do with it. The chunk bound limits how many there
can be; the erase is charged for them, in both the estimate that admits it and
the fee it pays. The shared per-type lifecycle container is unflagged, so no
single deleter is charged for structure nobody refunds; the record inside it
carries the deleter's flags and is what an erase refunds.
Everything rides protocol 14, which is unreleased. Every new version-table field
is dormant in the released tables and in the mock, and protocol 12 and 13 replay
unchanged: a keep-history delete still ends in an internal error there, and an
erase is refused as an unsupported version.
How Has This Been Tested?
cargo test -p drive --lib: 3,633 passed, 5 existing ignored.cargo test -p drive-abci --lib: 2,845 passed, 13 existing ignored.cargo test -p dpp -p platform-version -p drive-proof-verifier -p dash-sdk --lib:4,152 / 17 / 267 / 191 passed.
cargo test -p drive-abci --test strategy_tests run_chain_v13_to_v14_migrates_document_history_and_survives_restart:passed. The protocol 13 to 14 upgrade now also deletes and erases the document
the activation migrated, so the lifecycle is pinned against revision keys and
trees the migration produced.
cargo clippy --workspace --all-features --all-targetsandcargo fmt --all:clean.
cargo test -p wasm-sdk --target wasm32-unknown-unknown history_wasm_tests(4 passed),
yarn workspace @dashevo/wasm-sdk buildandtest:unit(424passing),
yarn workspace @dashevo/evo-sdk build,lintandtest:unit(224 passing).
New coverage worth calling out: erasure at one revision, exactly one chunk, one
more than a chunk and more than two chunks; the record written once and never
rewritten by a continuation; a continuation submitted by an identity that owns
nothing; two erases, a delete-plus-erase, and a delete-plus-create sharing one
block; refunds keyed under the epoch each byte was written in and credited to a
real balance; the admission estimate covering a full chunk whatever the document
retains; the erasing state proved and its every claimed field pinned against
what the verifier derives; identical state and fees with GroveDB's batch
consistency checking on and off; nonce bumps persisted on refusals; a populated
history subtree refusing to be dropped; and the erase wire gate at protocols 12,
13 and 14.
A functional spec,
KeepHistoryDocument.spec.js, walks the whole story against arunning network. It is not run in CI-less local verification and needs
yarn start.Breaking Changes
documentsKeepHistory: truewithcanBeDeleted: true, which earlier drafts of that protocol version rejected.Protocol 12 and 13 are unaffected.
DocumentTransitionandDocumentTransitionActionTypeeach gain a variant,appended so every existing bincode discriminant is preserved. Exhaustive
matches over either must be extended.
getDocumentHistoryresponse v1's lifecycle state gainsDELETEDandERASING; a client that matched exhaustively on the two previous states needsupdating.
DocumentOperationType::DeleteDocumentgains adeleter_idfield, and theDrive delete-operation family takes the block it belongs to.
Checklist:
For repository code-owners and collaborators only
Notes for reviewers
Class C activation-migration rehearsal on authentic state, and a benchmark of
a maximum legal block of worst-case erases on validator hardware to confirm
the chunk bound of 100.
is applied and before the type has a lifecycle container, both emit that
container's insert, because each document operation is converted without sight
of its siblings. GroveDB refuses the batch rather than committing half of it,
and a test pins that. Consensus cannot reach the shape: a batch state
transition carries exactly one document transition at every protocol version.
erase's revision enumeration reads bodies it discards. The estimate charges for
that honestly rather than claiming a key-only read.
🤖 Generated with Claude Code