fix(drive-abci): refuse indexOnly creates that collide within one batch - #4813
Conversation
Every transition of a documents batch is validated against the same unapplied state and the batch is then applied as one GroveDB batch. An indexOnly document has no primary row: its index entries are the rows, so two creates by one owner in one batch can address the same entry under a shorter index while differing under a longer one. The create probe reads committed state and sees neither, the storage walker's if-not-exists insert reads the same state, and GroveDB files batch ops by path and key, so the second insert silently replaces the first. The earlier document is left with one entry carrying the other row's commitment and its remaining entries orphaned: undeletable (the commitment probe fails) and unrecreatable (its surviving entries are duplicates). Track the entries every accepted indexOnly create of a batch writes and refuse a later create in the same batch that claims any of them, with the same DuplicateUniqueIndexError the state probe raises for a collision against committed state. The entry paths come from the same derivation the walkers write with; nothing is read, so nothing is billed. The shape is unreachable from the network today: the batch cap (max_transitions_in_documents_batch) is 1 at every protocol version and two transitions in one block apply as separate GroveDB batches. The tracker is what keeps indexOnly types safe on the day that cap is raised; the test drives the transformer and batch state validation directly to pin it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 36 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
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 |
PR HygieneState: waiting-bots · commit
Self-review is an author attestation that you have read the diff: This report does not bypass CI or repository protection rules. |
|
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v4.2-dev #4813 +/- ##
============================================
+ Coverage 77.99% 79.81% +1.81%
============================================
Files 2961 2962 +1
Lines 424039 424221 +182
============================================
+ Hits 330729 338591 +7862
+ Misses 93310 85630 -7680
🚀 New features to boost your workflow:
|
2 participants
Issue being fixed or feature implemented
Review finding: two indexOnly creates by one owner in the same documents batch that collide under one index (say the shorter
[postId]projection) while differing under another are both accepted, and the batch then corrupts the loser's row.Every transition of a batch is validated against the same, not yet applied state, and the batch is applied as ONE GroveDB batch. An indexOnly document has no primary row (its index entries are the rows), so:
has_index_only_document_entry) reads committed state and sees neither create's entries;add_index_only_terminal_item_operations) reads the same state and emits an insert-or-replace op;BatchStructurefiles ops by path and key, so the second insert silently replaces the first (the shipped default disables the batch consistency check).The earlier create ends up with its shared entry carrying the other row's commitment while its remaining entries stand: undeletable (the delete-side commitment probe fails on the replaced entry) and unrecreatable (its surviving entries are duplicates). Verified empirically at the drive layer with two colliding
markcreates in oneapply_drive_operationscall under the shipped batching config.Reachability. This shape cannot reach the write path from the network today:
SystemLimits::max_transitions_in_documents_batchis 1 at every protocol version (pinned by a unit test and observed refusing a two-transition batch inranked_group_drain.rs), and two state transitions in one block apply as two GroveDB batches, so the second one's stateful probe sees the first one's entries. It is a latent defect in the family the cap already guards (phantom index groups, #4383). Closing it now is what keeps indexOnly types safe on the day the cap is raised, instead of adding another#[ignore]d case to un-ignore first.What was done?
batch/state/v0/index_only_batch_entries.rs:IndexOnlyBatchEntries, a batch-scoped tracker of the(entry path, member key)pairs every accepted indexOnly create writes. A later create in the same batch that claims any of them is refused with the sameDuplicateUniqueIndexErrorthe state probe raises for a collision against committed state (naming the colliding index's properties plus terminal). Entries are derived withDrive::index_only_entry_paths_and_key, the derivation the index walkers write with, so the check cannot drift from storage. Nothing is read, so nothing is billed. A no-op for stored document types.batch/state/v0/mod.rs: the accept path of the per-transition loop is flattened withcontinues, and the tracker check runs after state validation and data triggers pass, immediately before the transition is accepted. A refused create gets the errors added and a nonce bump pushed, identical to the existing refusal paths. Edited in place rather than as a new version:index_only()can only be true on a PV14+ contract and the cap keeps the branch dormant, so no historical batch changes outcome.The storage-layer backstop stays stateful, matching the stored-document unique-index walker; the cap is what stands between it and a live path, as documented on
max_transitions_in_documents_batch.How Has This Been Tested?
test_colliding_index_only_creates_in_one_batch_are_refusedin the drive-abci indexOnly suite. It drivestransform_into_actionand the batchvalidate_statedirectly (the pipeline refuses any two-transition batch at basic structure) with twomarkcreates colliding onbyAand differing onbyB. Before the fix: zero errors, both creates accepted. After: oneDuplicateUniqueIndexErrornaming the second document and[a, $ownerId], the second becomes a nonce bump, the first stands; a pair sharing no entry passes untouched.cargo test -p drive-abci --lib -- index_only: 15 passed.cargo test -p drive-abci --lib -- batch::tests::document: 168 passed.cargo clippy -p drive-abci --lib --tests: clean;cargo fmt --check: clean.Breaking Changes
None. The only batches whose validation outcome changes cannot be produced from the network at any protocol version (batch cap of 1).
Checklist:
For repository code-owners and collaborators only
🤖 Generated with Claude Code