From 0cf3caff4306105047704cd0f06548fce59a41be Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 18 Sep 2026 18:50:01 +0700 Subject: [PATCH] fix(drive-abci): refuse indexOnly creates that collide within one batch 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 --- .../state/v0/index_only_batch_entries.rs | 135 ++++++++++++ .../state_transitions/batch/state/v0/mod.rs | 53 ++++- .../batch/tests/document/index_only.rs | 205 ++++++++++++++++++ 3 files changed, 386 insertions(+), 7 deletions(-) create mode 100644 packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/state/v0/index_only_batch_entries.rs diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/state/v0/index_only_batch_entries.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/state/v0/index_only_batch_entries.rs new file mode 100644 index 00000000000..74944ddbde5 --- /dev/null +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/state/v0/index_only_batch_entries.rs @@ -0,0 +1,135 @@ +//! Batch-scoped tracking of the index entries indexOnly creates write. +//! +//! Every transition of a batch is validated against the same, not yet +//! applied state, and the whole batch is then applied as ONE grove batch. +//! For a stored document type that is safe: two creates in one batch +//! cannot share an id (`find_duplicates_by_id` refuses that at basic +//! structure) and so write distinct primary rows. An indexOnly document +//! has no row — its index entries ARE the rows — and two creates by the +//! same owner can address the same entry under one index while differing +//! under another (a shorter index projects fewer properties). The create's +//! state probe (`has_index_only_document_entry`) reads committed state, so +//! it sees neither create's entries; the storage walker's if-not-exists +//! insert reads the same state; and grovedb files a batch's operations by +//! path and key, so the second insert silently replaces the first. The +//! result is one entry carrying the other document's row commitment while +//! the rest of the loser's entries stand: a document nobody can delete +//! (its commitment probe fails on the replaced entry) or recreate (its +//! surviving entries are duplicates). +//! +//! This tracker closes the gap the state probe cannot see. It records the +//! entries every create the batch has already accepted will write and +//! refuses a later create of the same batch that would write any of them, +//! with the `DuplicateUniqueIndexError` the state probe raises for the +//! same collision against committed state. Nothing is read: the entry +//! paths and member keys come from the action's own values through +//! [`Drive::index_only_entry_paths_and_key`], the derivation the index +//! walkers write with, so nothing is billed and the check cannot drift +//! from storage. +//! +//! `index_only()` can only be true on a PV14+ contract, so the tracker is +//! a no-op for every historical batch. It is also dormant today for a +//! second reason: `max_transitions_in_documents_batch` is 1 at every +//! protocol version, so basic structure validation refuses any batch +//! carrying two transitions before either reaches this loop, and two +//! transitions in one block apply as two grove batches (the second's +//! state probe sees the first's entries). The tracker is what keeps +//! indexOnly types safe on the day that cap is raised — the test in +//! `batch/tests/document/index_only.rs` drives the loop directly to pin +//! it. + +use crate::error::Error; +use dpp::consensus::basic::document::InvalidDocumentTypeError; +use dpp::consensus::state::document::duplicate_unique_index_error::DuplicateUniqueIndexError; +use dpp::data_contract::accessors::v0::DataContractV0Getters; +use dpp::data_contract::document_type::accessors::{DocumentTypeV0Getters, DocumentTypeV2Getters}; +use dpp::document::Document; +use dpp::identifier::Identifier; +use dpp::validation::SimpleConsensusValidationResult; +use dpp::version::PlatformVersion; +use drive::drive::Drive; +use drive::state_transition_action::batch::batched_transition::document_transition::document_base_transition_action::DocumentBaseTransitionActionAccessorsV0; +use drive::state_transition_action::batch::batched_transition::document_transition::document_create_transition_action::{ + DocumentCreateTransitionAction, DocumentCreateTransitionActionAccessorsV0, + DocumentFromCreateTransitionAction, +}; +use std::collections::BTreeSet; + +/// The `(entry path, member key)` pairs every accepted indexOnly create of +/// one batch writes. One tracker per batch state validation. +#[derive(Default)] +pub(super) struct IndexOnlyBatchEntries { + entries: BTreeSet<(Vec>, Vec)>, +} + +impl IndexOnlyBatchEntries { + /// Refuses `create_action` when any entry it would write is already + /// claimed by an earlier accepted create of the same batch, and claims + /// all of its entries otherwise. Call it only for a create that state + /// validation (and the data triggers) accepted: a refused create writes + /// nothing, so it must not block a later create in the batch. A no-op + /// for stored (non-indexOnly) document types. + pub(super) fn validate_and_record_create( + &mut self, + create_action: &DocumentCreateTransitionAction, + owner_id: Identifier, + platform_version: &PlatformVersion, + ) -> Result { + let contract_fetch_info = create_action.base().data_contract_fetch_info(); + let contract = &contract_fetch_info.contract; + let document_type_name = create_action.base().document_type_name(); + + // The create's own state validation resolves the document type + // first and refuses an unknown one, so this mirrors that refusal + // rather than treating it as a code error. + let Some(document_type) = contract.document_type_optional_for_name(document_type_name) + else { + return Ok(SimpleConsensusValidationResult::new_with_error( + InvalidDocumentTypeError::new(document_type_name.clone(), contract.id()).into(), + )); + }; + + if !document_type.index_only() { + return Ok(SimpleConsensusValidationResult::new()); + } + + let document = + Document::try_from_create_transition_action(create_action, owner_id, platform_version)?; + + let mut claimed = Vec::new(); + for index in document_type.indexes().values() { + let (paths, member_key) = Drive::index_only_entry_paths_and_key( + contract.id(), + document_type, + index, + &document, + platform_version, + ) + .map_err(Error::Drive)?; + for path in paths { + let entry = (path, member_key.clone()); + if self.entries.contains(&entry) { + return Ok(SimpleConsensusValidationResult::new_with_error( + DuplicateUniqueIndexError::new( + create_action.base().id(), + index + .properties + .iter() + .map(|property| property.name.clone()) + .chain(index.terminal.clone()) + .collect(), + ) + .into(), + )); + } + claimed.push(entry); + } + } + + // Claim only once every entry is known to be free, so a refused + // create leaves the tracker exactly as it found it. + self.entries.extend(claimed); + + Ok(SimpleConsensusValidationResult::new()) + } +} diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/state/v0/mod.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/state/v0/mod.rs index 4fd93181846..b27a00d8ecf 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/state/v0/mod.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/state/v0/mod.rs @@ -34,6 +34,8 @@ use crate::execution::validation::state_transition::batch::action_validation::to use crate::execution::validation::state_transition::batch::action_validation::token::token_transfer_transition_action::TokenTransferTransitionActionValidation; use crate::execution::validation::state_transition::batch::action_validation::token::token_unfreeze_transition_action::TokenUnfreezeTransitionActionValidation; use crate::execution::validation::state_transition::batch::data_triggers::{data_trigger_bindings_list, DataTriggerExecutionContext, DataTriggerExecutor}; +use crate::execution::validation::state_transition::batch::state::v0::index_only_batch_entries::IndexOnlyBatchEntries; +use drive::state_transition_action::batch::batched_transition::document_transition::document_create_transition_action::DocumentCreateTransitionActionAccessorsV0; use crate::platform_types::platform::{PlatformStateRef}; use crate::execution::validation::state_transition::state_transitions::batch::transformer::v0::BatchTransitionTransformerV0; use crate::execution::validation::state_transition::ValidationMode; @@ -41,6 +43,7 @@ use crate::platform_types::platform_state::PlatformStateV0Methods; pub mod fetch_contender; pub mod fetch_documents; +mod index_only_batch_entries; pub(in crate::execution::validation::state_transition::state_transitions::batch) trait DocumentsBatchStateTransitionStateValidationV0 { @@ -85,6 +88,13 @@ impl DocumentsBatchStateTransitionStateValidationV0 for BatchTransition { vec![] }; + // The entries every accepted indexOnly create of THIS batch writes. + // The per-create state probe reads committed state, which none of + // the batch's own creates have reached yet, and the batch applies as + // one grove batch where a second insert at the same path and key + // silently replaces the first — see `index_only_batch_entries`. + let mut index_only_batch_entries = IndexOnlyBatchEntries::default(); + // Next we need to validate the structure of all actions (this means with the data contract) for transition in state_transition_action.transitions_take() { let transition_validation_result = match &transition { @@ -276,7 +286,10 @@ impl DocumentsBatchStateTransitionStateValidationV0 for BatchTransition { state_transition_action.user_fee_increase(), )?, )); - } else if platform.config.execution.use_document_triggers { + continue; + } + + if platform.config.execution.use_document_triggers { if let BatchedTransitionAction::DocumentAction(document_transition) = &transition { // Pre-PR this site allocated a default-initialized local // `StateTransitionExecutionContext` and passed `&local` to @@ -338,15 +351,41 @@ impl DocumentsBatchStateTransitionStateValidationV0 for BatchTransition { state_transition_action.user_fee_increase(), ), )); - } else { - validated_transitions.push(transition); + continue; } - } else { - validated_transitions.push(transition); } - } else { - validated_transitions.push(transition); } + + // An accepted indexOnly create claims the entries it writes for + // the rest of the batch; a later create addressing any of them + // is refused here exactly as the state probe refuses the same + // collision against committed state. Only reachable for PV14+ + // contracts (`index_only()` cannot be true below that), so no + // historical batch takes this path. + if let BatchedTransitionAction::DocumentAction( + DocumentTransitionAction::CreateAction(create_action), + ) = &transition + { + let batch_entries_result = index_only_batch_entries.validate_and_record_create( + create_action, + owner_id, + platform_version, + )?; + if !batch_entries_result.is_valid() { + validation_result.add_errors(batch_entries_result.errors); + validated_transitions + .push(BatchedTransitionAction::BumpIdentityDataContractNonce( + BumpIdentityDataContractNonceAction::from_borrowed_document_base_transition_action( + create_action.base(), + owner_id, + state_transition_action.user_fee_increase(), + ), + )); + continue; + } + } + + validated_transitions.push(transition); } state_transition_action.set_transitions(validated_transitions); diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/index_only.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/index_only.rs index c240921806e..50f21380e58 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/index_only.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/index_only.rs @@ -1188,6 +1188,211 @@ pub(super) mod index_only_tests { ); } } + + /// An UNSIGNED `mark` create for the given property values, for + /// assembling batches that carry more than one transition (the factory + /// signs exactly one create per batch). Same shape as + /// [`signed_mark_create`]; the document comes back alongside so the + /// test can probe the entries its values address. + fn mark_create_transition( + contract: &DataContract, + owner: Identifier, + a: &str, + b: &str, + nonce: u64, + rng: &mut StdRng, + platform_version: &PlatformVersion, + ) -> ( + dpp::state_transition::batch_transition::batched_transition::DocumentCreateTransition, + Document, + ) { + use dpp::document::DocumentV0Setters; + use dpp::state_transition::batch_transition::batched_transition::DocumentCreateTransition; + let mark_type = contract + .document_type_for_name("mark") + .expect("mark doctype exists"); + let entropy = Bytes32::random_with_rng(rng); + let mut mark = mark_type + .random_document_with_identifier_and_entropy( + rng, + owner, + entropy, + DocumentFieldFillType::FillIfNotRequired, + DocumentFieldFillSize::AnyDocumentFillSize, + platform_version, + ) + .expect("expected a random mark"); + mark.set("a", a.into()); + mark.set("b", b.into()); + let create = DocumentCreateTransition::from_document( + mark.clone(), + mark_type, + entropy.0, + None, + nonce, + platform_version, + None, + None, + ) + .expect("expected the create transition"); + (create, mark) + } + + /// Two creates in ONE batch whose entries collide under one index + /// (`byA`: same `a`, same owner) but not under another (`byB`: + /// different `b`). Every transition of a batch is validated against + /// the same unapplied state, so the state probe sees neither create's + /// entries, and the batch is then applied as one grove batch, where a + /// second insert at the same path and key silently replaces the + /// first. Left alone, the loser's `byA` entry would carry the winner's + /// row commitment while its `byB` entry stood: a document nobody could + /// delete (the commitment probe fails on the replaced entry) or + /// recreate (its surviving entry is a duplicate). The batch-scoped + /// entry tracking must refuse the second create exactly as the state + /// probe refuses a collision with committed state, while a pair that + /// shares no entry passes untouched. + /// + /// Driven through the transformer and the batch state validation + /// directly: `max_transitions_in_documents_batch` is 1 at every + /// protocol version, so `process_raw_state_transitions` refuses any + /// two-transition batch at basic structure (pinned by + /// `ranked_group_drain`) and this shape cannot reach the write path + /// from the network today. The tracker is what keeps that true for + /// indexOnly types on the day the cap is raised. + #[tokio::test] + async fn test_colliding_index_only_creates_in_one_batch_are_refused() { + use crate::execution::types::state_transition_execution_context::StateTransitionExecutionContext; + use crate::execution::validation::state_transition::processor::state::StateTransitionStateValidation; + use crate::execution::validation::state_transition::transformer::StateTransitionActionTransformer; + use crate::execution::validation::state_transition::ValidationMode; + use crate::platform_types::platform::PlatformRef; + use dpp::version::DefaultForPlatformVersion; + use drive::state_transition_action::batch::batched_transition::document_transition::DocumentTransitionAction; + use drive::state_transition_action::batch::batched_transition::BatchedTransitionAction; + use drive::state_transition_action::StateTransitionAction; + + let platform_version = PlatformVersion::latest(); + let mut platform = TestPlatformBuilder::new() + .build_with_mock_rpc() + .set_genesis_state(); + let mut rng = StdRng::seed_from_u64(78056); + + let (alice, _alice_signer, _alice_key) = + setup_identity(&mut platform, 958, dash_to_credits!(1.0)); + let contract = register_likes(&platform, alice.id(), platform_version); + + let state = platform.state.load(); + let platform_ref = PlatformRef { + drive: &platform.drive, + state: &state, + config: &platform.config, + core_rpc: &platform.core_rpc, + }; + + // Transforms and state-validates an unsigned two-create batch (neither + // step checks signatures) and returns the consensus errors alongside + // whether each transition survived as a create. + let mut validate_pair = |pairs: [(&str, &str); 2]| { + let [(a_1, b_1), (a_2, b_2)] = pairs; + let (first, _) = mark_create_transition( + &contract, + alice.id(), + a_1, + b_1, + 2, + &mut rng, + platform_version, + ); + let (second, second_mark) = mark_create_transition( + &contract, + alice.id(), + a_2, + b_2, + 3, + &mut rng, + platform_version, + ); + let batch: StateTransition = BatchTransition::from(BatchTransitionV0 { + owner_id: alice.id(), + transitions: vec![first.into(), second.into()], + user_fee_increase: 0, + signature_public_key_id: 0, + signature: Default::default(), + }) + .into(); + + let mut execution_context = + StateTransitionExecutionContext::default_for_platform_version(platform_version) + .expect("expected an execution context"); + let transformed = batch + .transform_into_action( + &platform_ref, + &BlockInfo::default(), + &None, + ValidationMode::Validator, + &mut execution_context, + None, + ) + .expect("expected to transform the batch"); + assert!( + transformed.errors.is_empty(), + "the batch must transform cleanly: {:?}", + transformed.errors + ); + let action = transformed.data.expect("expected the batch action"); + + let validated = batch + .validate_state( + Some(action), + &platform_ref, + ValidationMode::Validator, + &BlockInfo::default(), + &mut execution_context, + None, + ) + .expect("expected to validate the batch against state"); + let Some(StateTransitionAction::BatchAction(action)) = validated.data else { + panic!("expected a batch action back from state validation"); + }; + let survived_as_creates: Vec = action + .transitions() + .iter() + .map(|transition| { + matches!( + transition, + BatchedTransitionAction::DocumentAction( + DocumentTransitionAction::CreateAction(_) + ) + ) + }) + .collect(); + (validated.errors, survived_as_creates, second_mark) + }; + + // ── colliding on `byA` (same `a`), differing on `byB` ────────── + let (errors, survived, second_mark) = validate_pair([("x", "one"), ("x", "two")]); + assert_matches!( + errors.as_slice(), + [ConsensusError::StateError(StateError::DuplicateUniqueIndexError(error))] + if error.document_id() == &second_mark.id() + && error.duplicating_properties() == &["a".to_string(), "$ownerId".to_string()], + "the second create collides with the first on `byA` and must be refused, \ + naming the second document and the colliding index: {errors:?}" + ); + assert_eq!( + survived, + vec![true, false], + "the first create must stand and the second must become a nonce bump" + ); + + // ── sharing no entry: both stand ─────────────────────────────── + let (errors, survived, _) = validate_pair([("y", "one"), ("z", "two")]); + assert!( + errors.is_empty(), + "two creates that share no entry must both pass: {errors:?}" + ); + assert_eq!(survived, vec![true, true]); + } } mod index_only_executed_proof_tests {