From 6143053f4c841d166fdf2b56b9487b6a4f43ad94 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 24 Jul 2026 00:05:30 +0800 Subject: [PATCH 1/5] fix(platform): reject contradictory keep-history document deletes Document types could set both documentsKeepHistory: true and canBeDeleted: true, so delete attempts surfaced as InternalError instead of a clean consensus rejection. - Reject the combination during DPP schema parsing via a new try_from_schema v3, active from protocol version 13 (CONTRACT_VERSIONS_V5). v2 stays untouched: protocol version 12 is released and consensus-frozen. - Add a drive-abci delete-transition structure-validation v1 that rejects deletes on keep-history document types as invalid (paid) transitions, gated at protocol version 13 via DRIVE_ABCI_VALIDATION_VERSIONS_V9. - Regression coverage on both sides of the v12/v13 boundary, including a contradictory fixture for the already-deployed-contract path. Fixes #3927 Co-Authored-By: PastaClaw Co-Authored-By: Claude Fable 5 --- .../class_methods/try_from_schema/mod.rs | 16 +- .../class_methods/try_from_schema/v3/mod.rs | 298 ++++++++++++++++++ .../advanced_structure_v1/mod.rs | 66 ++++ .../document_delete_transition_action/mod.rs | 5 +- .../batch/tests/document/deletion.rs | 193 ++++++++++++ ...tract-keep-history-and-can-be-deleted.json | 29 ++ packages/rs-drive/src/drive/contract/mod.rs | 8 +- .../add_document_to_primary_storage/v0/mod.rs | 1 + .../dpp_versions/dpp_contract_versions/v5.rs | 6 +- .../drive_abci_validation_versions/v9.rs | 12 +- 10 files changed, 629 insertions(+), 5 deletions(-) create mode 100644 packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v3/mod.rs create mode 100644 packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/action_validation/document/document_delete_transition_action/advanced_structure_v1/mod.rs create mode 100644 packages/rs-drive-abci/tests/supporting_files/contract/note/note-contract-keep-history-and-can-be-deleted.json diff --git a/packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/mod.rs b/packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/mod.rs index a9bcfc28f95..6471c67417d 100644 --- a/packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/mod.rs +++ b/packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/mod.rs @@ -18,6 +18,7 @@ use std::collections::{BTreeMap, BTreeSet}; mod v0; mod v1; mod v2; +mod v3; const NOT_ALLOWED_SYSTEM_PROPERTIES: [&str; 1] = ["$id"]; @@ -87,9 +88,22 @@ impl DocumentType { validation_operations, platform_version, ), + 3 => DocumentType::try_from_schema_v3( + data_contract_id, + data_contract_system_version, + contract_config_version, + name, + schema, + schema_defs, + token_configurations, + data_contact_config, + full_validation, + validation_operations, + platform_version, + ), version => Err(ProtocolError::UnknownVersionMismatch { method: "try_from_schema".to_string(), - known_versions: vec![0, 1, 2], + known_versions: vec![0, 1, 2, 3], received: version, }), } diff --git a/packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v3/mod.rs b/packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v3/mod.rs new file mode 100644 index 00000000000..88787740858 --- /dev/null +++ b/packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v3/mod.rs @@ -0,0 +1,298 @@ +use crate::data_contract::config::DataContractConfig; +use crate::data_contract::document_type::accessors::DocumentTypeV0Getters; +use crate::data_contract::document_type::class_methods::consensus_or_protocol_data_contract_error; +use crate::data_contract::document_type::DocumentType; +use crate::data_contract::errors::DataContractError; +use crate::data_contract::{TokenConfiguration, TokenContractPosition}; +use crate::validation::operations::ProtocolValidationOperation; +use crate::version::PlatformVersion; +use crate::ProtocolError; +use platform_value::{Identifier, Value}; +use std::collections::BTreeMap; + +impl DocumentType { + /// V3 delegates parsing to the V2 parser, then rejects the + /// self-contradictory `documentsKeepHistory: true` + `canBeDeleted: true` + /// combination during full validation. + /// + /// The combination is contradictory because rs-drive unconditionally + /// refuses to delete a document whose type keeps history + /// (`force_delete_document_for_contract_operations_v0` returns + /// `InvalidDeletionOfDocumentThatKeepsHistory`), so `canBeDeleted: true` + /// advertises a capability the storage layer will always reject. Catching + /// it at parse time turns the contradiction into a clean validation error + /// at contract creation, before any delete is attempted. Mirrors the + /// existing cross-flag rule for + /// `ContestedUniqueIndexOnMutableDocumentTypeError`. + /// + /// The check lives in a new `try_from_schema` version (active from + /// protocol version 13, via `CONTRACT_VERSIONS_V5`) rather than inside the + /// V2 parser because V2 is shared with protocol version 12, which is + /// released and therefore consensus-frozen — contracts accepted at v12 + /// must replay identically. + /// + /// Gated by `full_validation` so already-deployed contradictory contracts + /// (e.g. testnet `5CBPiadGmx3Zsjc26g5onopcx7pdxHPbrRAUD2T2yAbC` document + /// type `note`) continue to load when re-parsed at v13+ — the drive-abci + /// delete-transition guard turns their deletes into normal invalid (paid) + /// transitions instead of internal errors at that layer. + /// + /// Uses `consensus_or_protocol_data_contract_error` so that with the + /// `validation` feature this surfaces as `ProtocolError::ConsensusError`; + /// drive-abci's `transform_into_action_v0` only converts that variant + /// into an invalid (paid) transition with a bump action — a bare + /// `ProtocolError::DataContractError` would propagate as an internal + /// execution error in validator mode. + #[allow(clippy::too_many_arguments)] + pub(in crate::data_contract::document_type::class_methods) fn try_from_schema_v3( + data_contract_id: Identifier, + data_contract_system_version: u16, + contract_config_version: u16, + name: &str, + schema: Value, + schema_defs: Option<&BTreeMap>, + token_configurations: &BTreeMap, + data_contact_config: &DataContractConfig, + full_validation: bool, + validation_operations: &mut impl Extend, + platform_version: &PlatformVersion, + ) -> Result { + let document_type = DocumentType::try_from_schema_v2( + data_contract_id, + data_contract_system_version, + contract_config_version, + name, + schema, + schema_defs, + token_configurations, + data_contact_config, + full_validation, + validation_operations, + platform_version, + )?; + + // The flags are read from the parsed result (not the raw schema) so + // the check sees `canBeDeleted` resolved against the contract config + // default (`true` when the key is omitted). + if full_validation + && document_type.documents_keep_history() + && document_type.documents_can_be_deleted() + { + return Err(consensus_or_protocol_data_contract_error( + DataContractError::InvalidContractStructure(format!( + "document type \"{}\" sets both `documentsKeepHistory: true` and \ + `canBeDeleted: true`, but the storage layer unconditionally refuses to \ + delete a document whose type keeps history. Set `canBeDeleted` to false or \ + disable `documentsKeepHistory`.", + name, + )), + )); + } + + Ok(document_type) + } +} + +#[cfg(test)] +mod tests { + //! Regression tests for the `documentsKeepHistory` + `canBeDeleted` + //! cross-flag rule added in `try_from_schema` v3 (protocol version 13). + use super::*; + use platform_value::platform_value; + + /// Parses through the public dispatcher at the given protocol version so + /// the test exercises the same `try_from_schema` version routing consensus + /// code uses (v3 at protocol version 13, v2 at 12). + fn parse_at_version( + schema: Value, + protocol_version: u32, + full_validation: bool, + ) -> Result { + let platform_version = + PlatformVersion::get(protocol_version).expect("expected platform version"); + let config = DataContractConfig::default_for_version(platform_version) + .expect("default config available"); + DocumentType::try_from_schema( + Identifier::new([1; 32]), + 1, + config.version(), + "test_doc", + schema, + None, + &BTreeMap::new(), + &config, + full_validation, + &mut vec![], + platform_version, + ) + } + + fn parse(schema: Value) -> Result { + parse_at_version(schema, 13, true) + } + + fn keep_history_deletable_schema() -> Value { + platform_value!({ + "type": "object", + "properties": { + "label": { + "type": "string", + "maxLength": 50, + "position": 0, + }, + }, + "additionalProperties": false, + "documentsKeepHistory": true, + "canBeDeleted": true, + }) + } + + /// `documentsKeepHistory: true` + `canBeDeleted: true` is + /// self-contradictory: rs-drive unconditionally refuses to delete + /// a document whose type keeps history + /// (`InvalidDeletionOfDocumentThatKeepsHistory`), so `canBeDeleted: + /// true` advertises a capability the storage layer will always + /// reject. The parser must reject the combination at contract + /// creation time so an SDK user gets a clean validation error + /// instead of the delete failing as an internal error at execution. + /// + /// With the `validation` feature enabled the rejection must surface + /// as `ProtocolError::ConsensusError` (not bare + /// `ProtocolError::DataContractError`) — drive-abci's + /// `transform_into_action_v0` only turns the consensus variant into + /// a clean invalid (paid) transition with a bump action; the + /// data-contract-error variant propagates as an internal execution + /// error in validator mode. + #[test] + fn doctype_keep_history_with_can_be_deleted_rejected() { + let result = parse(keep_history_deletable_schema()); + assert!( + result.is_err(), + "documentsKeepHistory: true + canBeDeleted: true must be rejected" + ); + let err = result.unwrap_err(); + let msg = format!("{:?}", err); + assert!( + msg.contains("documentsKeepHistory") && msg.contains("canBeDeleted"), + "error must reference both documentsKeepHistory and canBeDeleted; got {msg}" + ); + #[cfg(feature = "validation")] + assert!( + matches!(err, ProtocolError::ConsensusError(_)), + "with `validation` feature the rejection must be ProtocolError::ConsensusError so \ + drive-abci's transform_into_action turns it into an invalid (paid) transition \ + with a bump action rather than propagating as an internal execution error; got \ + {err:?}" + ); + } + + /// Omitting `canBeDeleted` exercises the contract-config default boundary: + /// the latest config defaults it to `true`, so a keep-history document type + /// remains contradictory and must be rejected during full validation. + #[test] + fn doctype_keep_history_with_can_be_deleted_omitted_rejected() { + let schema = platform_value!({ + "type": "object", + "properties": { + "label": { + "type": "string", + "maxLength": 50, + "position": 0, + }, + }, + "additionalProperties": false, + "documentsKeepHistory": true, + }); + let result = parse(schema); + assert!( + result.is_err(), + "omitted canBeDeleted must default to true and conflict with documentsKeepHistory" + ); + let msg = format!("{:?}", result.unwrap_err()); + assert!( + msg.contains("documentsKeepHistory") && msg.contains("canBeDeleted"), + "error must reference both documentsKeepHistory and defaulted canBeDeleted; got {msg}" + ); + } + + /// `documentsKeepHistory: true` + `canBeDeleted: true` is rejected + /// ONLY when `full_validation: true`. With `full_validation: false` + /// (the restore / migration / cache-warmup path) the same schema must + /// parse cleanly so already-deployed contradictory contracts continue + /// to load at v13+ — the drive-abci delete-transition guard turns + /// their deletes into clean invalid (paid) transitions instead of + /// rejecting them as internal errors at the contract-load layer. + #[test] + fn doctype_keep_history_with_can_be_deleted_accepted_without_full_validation() { + let document_type = parse_at_version(keep_history_deletable_schema(), 13, false).expect( + "documentsKeepHistory: true + canBeDeleted: true must be accepted when \ + full_validation: false so already-deployed contradictory contracts continue to load", + ); + assert!(document_type.documents_keep_history()); + assert!(document_type.documents_can_be_deleted()); + } + + /// Protocol version 12 routes to `try_from_schema` v2, which has no + /// cross-flag rule — the combination must stay accepted there even under + /// full validation, because v12 is released and consensus-frozen: + /// contracts accepted at v12 must replay identically. + #[test] + fn doctype_keep_history_with_can_be_deleted_accepted_at_protocol_version_12() { + let document_type = parse_at_version(keep_history_deletable_schema(), 12, true).expect( + "documentsKeepHistory: true + canBeDeleted: true must stay accepted at protocol \ + version 12 (consensus-frozen v2 parser) for replay compatibility", + ); + assert!(document_type.documents_keep_history()); + assert!(document_type.documents_can_be_deleted()); + } + + /// Guard against an over-broad fix: `documentsKeepHistory: true` + + /// `canBeDeleted: false` is consistent (the doctype is append-only) + /// and must continue to parse cleanly. The sibling omitted-key regression + /// covers the distinct default-`true` boundary and therefore expects + /// rejection rather than acceptance. + #[test] + fn doctype_keep_history_with_can_be_deleted_false_accepted() { + let schema = platform_value!({ + "type": "object", + "properties": { + "label": { + "type": "string", + "maxLength": 50, + "position": 0, + }, + }, + "additionalProperties": false, + "documentsKeepHistory": true, + "canBeDeleted": false, + }); + let document_type = parse(schema).expect( + "documentsKeepHistory: true + canBeDeleted: false is consistent and must parse", + ); + assert!(document_type.documents_keep_history()); + assert!(!document_type.documents_can_be_deleted()); + } + + /// Symmetric guard: `canBeDeleted: true` on a non-keep-history + /// doctype must continue to parse cleanly. Catches a predicate that + /// triggers on `canBeDeleted: true` alone instead of the AND. + #[test] + fn doctype_can_be_deleted_without_keep_history_accepted() { + let schema = platform_value!({ + "type": "object", + "properties": { + "label": { + "type": "string", + "maxLength": 50, + "position": 0, + }, + }, + "additionalProperties": false, + "canBeDeleted": true, + }); + let document_type = parse(schema) + .expect("canBeDeleted: true without documentsKeepHistory must parse cleanly"); + assert!(!document_type.documents_keep_history()); + assert!(document_type.documents_can_be_deleted()); + } +} diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/action_validation/document/document_delete_transition_action/advanced_structure_v1/mod.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/action_validation/document/document_delete_transition_action/advanced_structure_v1/mod.rs new file mode 100644 index 00000000000..cc748b4df48 --- /dev/null +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/action_validation/document/document_delete_transition_action/advanced_structure_v1/mod.rs @@ -0,0 +1,66 @@ +use dpp::consensus::basic::document::{InvalidDocumentTransitionActionError, InvalidDocumentTypeError}; +use dpp::data_contract::accessors::v0::DataContractV0Getters; +use dpp::data_contract::document_type::accessors::DocumentTypeV0Getters; +use dpp::validation::SimpleConsensusValidationResult; +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_delete_transition_action::DocumentDeleteTransitionAction; +use drive::state_transition_action::batch::batched_transition::document_transition::document_delete_transition_action::v0::DocumentDeleteTransitionActionAccessorsV0; + +use crate::error::Error; + +pub(in crate::execution::validation::state_transition::state_transitions::batch::action_validation) trait DocumentDeleteTransitionActionStructureValidationV1 { + fn validate_structure_v1(&self) -> Result; +} + +impl DocumentDeleteTransitionActionStructureValidationV1 for DocumentDeleteTransitionAction { + /// V1 adds the `documents_keep_history()` guard alongside the V0 + /// `documents_can_be_deleted()` guard. + /// + /// Pre-V1, a delete against a keep-history doctype passed structure + /// validation, reached `force_delete_document_for_contract_operations_v0`, + /// and returned `DriveError::InvalidDeletionOfDocumentThatKeepsHistory`. + /// The batch processor reclassifies that drive-layer error as + /// `ExecutionResult::InternalError` — the transition is neither valid nor + /// invalid-paid, leaving the SDK with no clean accept/reject signal. + /// + /// Rejecting at the structure layer turns the contradiction into a normal + /// invalid (paid) consensus error. Gated behind a new validation version + /// (rather than mutating V0) so PROTOCOL_VERSION_12 and earlier chains — + /// which historically classified these deletes as InternalError — replay + /// bit-for-bit. See issue #3927. + fn validate_structure_v1(&self) -> Result { + let contract_fetch_info = self.base().data_contract_fetch_info(); + let data_contract = &contract_fetch_info.contract; + let document_type_name = self.base().document_type_name(); + + let Some(document_type) = data_contract.document_type_optional_for_name(document_type_name) + else { + return Ok(SimpleConsensusValidationResult::new_with_error( + InvalidDocumentTypeError::new(document_type_name.clone(), data_contract.id()) + .into(), + )); + }; + + if !document_type.documents_can_be_deleted() { + return Ok(SimpleConsensusValidationResult::new_with_error( + InvalidDocumentTransitionActionError::new(format!( + "documents of type {} can not be deleted", + document_type_name + )) + .into(), + )); + } + + if document_type.documents_keep_history() { + return Ok(SimpleConsensusValidationResult::new_with_error( + InvalidDocumentTransitionActionError::new(format!( + "documents of type {} keep history and therefore can not be deleted", + document_type_name + )) + .into(), + )); + } + + Ok(SimpleConsensusValidationResult::new()) + } +} diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/action_validation/document/document_delete_transition_action/mod.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/action_validation/document/document_delete_transition_action/mod.rs index cc1a6cfa0d1..35b5210125a 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/action_validation/document/document_delete_transition_action/mod.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/action_validation/document/document_delete_transition_action/mod.rs @@ -9,9 +9,11 @@ use crate::error::execution::ExecutionError; use crate::execution::types::state_transition_execution_context::StateTransitionExecutionContext; use crate::execution::validation::state_transition::batch::action_validation::document::document_delete_transition_action::state_v0::DocumentDeleteTransitionActionStateValidationV0; use crate::execution::validation::state_transition::batch::action_validation::document::document_delete_transition_action::advanced_structure_v0::DocumentDeleteTransitionActionStructureValidationV0; +use crate::execution::validation::state_transition::batch::action_validation::document::document_delete_transition_action::advanced_structure_v1::DocumentDeleteTransitionActionStructureValidationV1; use crate::platform_types::platform::PlatformStateRef; mod advanced_structure_v0; +mod advanced_structure_v1; mod state_v0; pub trait DocumentDeleteTransitionActionValidation { @@ -44,9 +46,10 @@ impl DocumentDeleteTransitionActionValidation for DocumentDeleteTransitionAction .document_delete_transition_structure_validation { 0 => self.validate_structure_v0(), + 1 => self.validate_structure_v1(), version => Err(Error::Execution(ExecutionError::UnknownVersionMismatch { method: "DocumentDeleteTransitionAction::validate_structure".to_string(), - known_versions: vec![0], + known_versions: vec![0, 1], received: version, })), } diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/deletion.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/deletion.rs index beacc1093a3..41a4c9c6cf4 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/deletion.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/deletion.rs @@ -361,6 +361,199 @@ mod deletion_tests { assert_eq!(processing_result.aggregated_fees().processing_fee, 445700); } + /// PROTOCOL_VERSION_13 rejects deletes against contradictory keep-history + /// document types as invalid-paid consensus errors. + #[tokio::test] + async fn test_document_delete_on_document_type_that_keeps_history_is_rejected_protocol_version_13( + ) { + run_document_delete_on_document_type_that_keeps_history_at_protocol_version(13, true).await; + } + + /// PROTOCOL_VERSION_12 preserves the historical InternalError result for + /// replay compatibility. The keep-history structure guard must not run. + #[tokio::test] + async fn test_document_delete_on_document_type_that_keeps_history_replays_protocol_version_12() + { + run_document_delete_on_document_type_that_keeps_history_at_protocol_version(12, false) + .await; + } + + /// Exercises an already-deployed contradictory contract at both sides of + /// the v13 validation-version boundary. Loading with `full_validation: + /// false` is intentional: reparsing deployed contracts must remain allowed. + async fn run_document_delete_on_document_type_that_keeps_history_at_protocol_version( + protocol_version: dpp::version::ProtocolVersion, + expect_invalid_paid: bool, + ) { + let platform_version = PlatformVersion::get(protocol_version) + .expect("expected platform version for the requested protocol_version"); + let mut platform = TestPlatformBuilder::new() + .with_initial_protocol_version(protocol_version) + .build_with_mock_rpc() + .set_initial_state_structure(); + + let contract_path = "tests/supporting_files/contract/note/note-contract-keep-history-and-can-be-deleted.json"; + + // `full_validation: false` bypasses the DPP cross-flag check so the + // intentionally-contradictory fixture loads — mirrors the + // already-deployed-contract scenario this guard is meant to handle. + let note_contract = json_document_to_contract(contract_path, false, platform_version) + .expect("expected to get data contract"); + platform + .drive + .apply_contract( + ¬e_contract, + BlockInfo::default(), + true, + StorageFlags::optional_default_as_cow(), + None, + platform_version, + ) + .expect("expected to apply contract successfully"); + + let mut rng = StdRng::seed_from_u64(437); + + let platform_state = platform.state.load(); + + let (identity, signer, key) = setup_identity(&mut platform, 958, dash_to_credits!(0.1)); + + let note_document_type = note_contract + .document_type_for_name("note") + .expect("expected the note document type"); + + assert!( + note_document_type.documents_keep_history(), + "fixture sanity: doctype must keep history" + ); + assert!( + note_document_type.documents_can_be_deleted(), + "fixture sanity: doctype must advertise canBeDeleted" + ); + + let entropy = Bytes32::random_with_rng(&mut rng); + + let document = note_document_type + .random_document_with_identifier_and_entropy( + &mut rng, + identity.id(), + entropy, + DocumentFieldFillType::FillIfNotRequired, + DocumentFieldFillSize::AnyDocumentFillSize, + platform_version, + ) + .expect("expected a random document"); + + let mut altered_document = document.clone(); + altered_document.set_revision(Some(1)); + + // Create the document (must succeed — keep-history doctypes accept + // creates, the contradiction only bites at delete time). + let documents_batch_create_transition = + BatchTransition::new_document_creation_transition_from_document( + document, + note_document_type, + entropy.0, + &key, + 2, + 0, + None, + &signer, + platform_version, + None, + ) + .await + .expect("expect to create documents batch transition"); + + let documents_batch_create_serialized_transition = documents_batch_create_transition + .serialize_to_bytes() + .expect("expected documents batch serialized state transition"); + + let transaction = platform.drive.grove.start_transaction(); + + let processing_result = platform + .platform + .process_raw_state_transitions( + &vec![documents_batch_create_serialized_transition.clone()], + &platform_state, + &BlockInfo::default(), + &transaction, + platform_version, + false, + None, + ) + .expect("expected to process state transition"); + + assert_eq!(processing_result.valid_count(), 1); + + platform + .drive + .grove + .commit_transaction(transaction) + .unwrap() + .expect("expected to commit transaction"); + + // V13 rejects during structure validation; v12 reaches rs-drive and + // retains the historical InternalError classification for replay. + let documents_batch_deletion_transition = + BatchTransition::new_document_deletion_transition_from_document( + altered_document, + note_document_type, + &key, + 3, + 0, + None, + &signer, + platform_version, + None, + ) + .await + .expect("expect to create documents batch transition"); + + let documents_batch_deletion_serialized_transition = documents_batch_deletion_transition + .serialize_to_bytes() + .expect("expected documents batch serialized state transition"); + + let transaction = platform.drive.grove.start_transaction(); + + let processing_result = platform + .platform + .process_raw_state_transitions( + &vec![documents_batch_deletion_serialized_transition.clone()], + &platform_state, + &BlockInfo::default(), + &transaction, + platform_version, + false, + None, + ) + .expect("expected to process state transition"); + + platform + .drive + .grove + .commit_transaction(transaction) + .unwrap() + .expect("expected to commit transaction"); + + assert_eq!( + processing_result.invalid_paid_count(), + usize::from(expect_invalid_paid), + "unexpected invalid-paid classification at protocol version {protocol_version}" + ); + assert_eq!(processing_result.invalid_unpaid_count(), 0); + assert_eq!(processing_result.valid_count(), 0); + let internal_error_count = processing_result + .execution_results() + .iter() + .filter(|result| matches!(result, StateTransitionExecutionResult::InternalError(_))) + .count(); + assert_eq!( + internal_error_count, + usize::from(!expect_invalid_paid), + "unexpected InternalError classification at protocol version {protocol_version}" + ); + } + #[tokio::test] async fn test_document_delete_on_document_type_that_is_not_mutable_and_can_be_deleted() { run_document_delete_on_document_type_that_is_not_mutable_and_can_be_deleted_at_protocol_version( diff --git a/packages/rs-drive-abci/tests/supporting_files/contract/note/note-contract-keep-history-and-can-be-deleted.json b/packages/rs-drive-abci/tests/supporting_files/contract/note/note-contract-keep-history-and-can-be-deleted.json new file mode 100644 index 00000000000..f3db87fdc40 --- /dev/null +++ b/packages/rs-drive-abci/tests/supporting_files/contract/note/note-contract-keep-history-and-can-be-deleted.json @@ -0,0 +1,29 @@ +{ + "$formatVersion": "1", + "id": "4Bqs6itzfoDXzmgQibYZQABbqYsXmawVf7SKe3mKDQVe", + "ownerId": "2b994p95akyNFKtkDnDvBRUotDbkH54MHwGbhQLr5gcU", + "version": 1, + "keywords": [], + "documentSchemas": { + "note": { + "type": "object", + "documentsKeepHistory": true, + "documentsMutable": true, + "canBeDeleted": true, + "properties": { + "message": { + "type": "string", + "maxLength": 256, + "position": 0 + } + }, + "required": [ + "message" + ], + "additionalProperties": false, + "$comment": "Self-contradictory fixture: keep-history doctypes cannot be deleted at the storage layer (rs-drive returns InvalidDeletionOfDocumentThatKeepsHistory). Loaded with full_validation=false so the DPP cross-flag rule in DocumentType::try_from_schema does not reject it — exercises the rs-drive-abci delete-transition guard that turns the contradiction into a clean invalid-paid consensus error for already-deployed contracts." + } + }, + "groups": {}, + "tokens": {} +} diff --git a/packages/rs-drive/src/drive/contract/mod.rs b/packages/rs-drive/src/drive/contract/mod.rs index 7c3e5aae88e..0e859c0295b 100644 --- a/packages/rs-drive/src/drive/contract/mod.rs +++ b/packages/rs-drive/src/drive/contract/mod.rs @@ -2335,10 +2335,16 @@ mod tests { ) .expect("expected to apply contract successfully"); - // Now try to update with the same document type but documentsKeepHistory=true + // Now try to update with the same document type but documentsKeepHistory=true. + // `canBeDeleted: false` is required alongside `documentsKeepHistory: true` — + // the schema parser (try_from_schema v3, protocol version 13+) rejects the + // keep-history + canBeDeleted combination (canBeDeleted's config default is + // true), so the schema must opt out of delete to reach the intended + // `ChangingDocumentTypeKeepsHistory` assertion at `update_contract`. let history_schema = platform_value!({ "type": "object", "documentsKeepHistory": true, + "canBeDeleted": false, "properties": { "name": { "type": "string", diff --git a/packages/rs-drive/src/drive/document/insert/add_document_to_primary_storage/v0/mod.rs b/packages/rs-drive/src/drive/document/insert/add_document_to_primary_storage/v0/mod.rs index 4142b12b9fd..a05c38d1fb0 100644 --- a/packages/rs-drive/src/drive/document/insert/add_document_to_primary_storage/v0/mod.rs +++ b/packages/rs-drive/src/drive/document/insert/add_document_to_primary_storage/v0/mod.rs @@ -832,6 +832,7 @@ mod keep_history_summable_e2e { "required": ["amount"], "additionalProperties": false, "documentsKeepHistory": true, + "canBeDeleted": false, "documentsSummable": "amount", }); let schemas = platform_value!({ DOCTYPE_NAME: document_schema }); diff --git a/packages/rs-platform-version/src/version/dpp_versions/dpp_contract_versions/v5.rs b/packages/rs-platform-version/src/version/dpp_versions/dpp_contract_versions/v5.rs index 41ebed1f4fd..798b2ea3886 100644 --- a/packages/rs-platform-version/src/version/dpp_versions/dpp_contract_versions/v5.rs +++ b/packages/rs-platform-version/src/version/dpp_versions/dpp_contract_versions/v5.rs @@ -36,7 +36,11 @@ pub const CONTRACT_VERSIONS_V5: DPPContractVersions = DPPContractVersions { index_levels_from_indices: 0, }, class_method_versions: DocumentTypeClassMethodVersions { - try_from_schema: 2, + // changed: v3 rejects the self-contradictory `documentsKeepHistory: + // true` + `canBeDeleted: true` combination during full validation. + // v2 stays as-is for protocol version 12, which is released and + // consensus-frozen. See issue #3927. + try_from_schema: 3, create_document_types_from_document_schemas: 1, }, structure_version: 0, diff --git a/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v9.rs b/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v9.rs index 0a2119580e8..2345c2b6d12 100644 --- a/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v9.rs +++ b/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v9.rs @@ -174,7 +174,17 @@ pub const DRIVE_ABCI_VALIDATION_VERSIONS_V9: DriveAbciValidationVersions = }, is_allowed: 0, document_create_transition_structure_validation: 0, - document_delete_transition_structure_validation: 0, + // PROTOCOL_VERSION_13: structure validator now rejects + // deletes against `documentsKeepHistory: true` doctypes as + // invalid (paid) consensus errors. Pre-v13 the delete reached + // `force_delete_document_for_contract_operations_v0`, + // returned `InvalidDeletionOfDocumentThatKeepsHistory`, and + // the batch processor reclassified it as + // `ExecutionResult::InternalError` (neither valid nor + // invalid-paid). Gated here so PROTOCOL_VERSION_12 and + // earlier chain history stays bit-for-bit reproducible. See + // issue #3927. + document_delete_transition_structure_validation: 1, document_replace_transition_structure_validation: 0, document_transfer_transition_structure_validation: 0, document_purchase_transition_structure_validation: 0, From 6266f354e481aa7acc5a4507a83f989f6b3c5abf Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Wed, 9 Sep 2026 04:00:41 +0700 Subject: [PATCH 2/5] fix(dpp): use existing protocol 14 schema parser for history validation --- .../class_methods/try_from_schema/mod.rs | 16 +- .../try_from_schema/v3/keep_history_tests.rs | 362 +++++++++++++++ .../class_methods/try_from_schema/v3/mod.rs | 35 +- .../class_methods/try_from_schema/v4/mod.rs | 435 ------------------ packages/rs-drive/src/drive/contract/mod.rs | 2 +- .../dpp_versions/dpp_contract_versions/v6.rs | 8 +- .../tests/keep_history_delete_versions.rs | 2 +- 7 files changed, 402 insertions(+), 458 deletions(-) create mode 100644 packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v3/keep_history_tests.rs delete mode 100644 packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v4/mod.rs diff --git a/packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/mod.rs b/packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/mod.rs index 1f00b199937..5c006d220d9 100644 --- a/packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/mod.rs +++ b/packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/mod.rs @@ -22,7 +22,6 @@ mod v0; mod v1; mod v2; mod v3; -mod v4; const NOT_ALLOWED_SYSTEM_PROPERTIES: [&str; 1] = ["$id"]; @@ -105,22 +104,9 @@ impl DocumentType { validation_operations, platform_version, ), - 4 => DocumentType::try_from_schema_v4( - data_contract_id, - data_contract_system_version, - contract_config_version, - name, - schema, - schema_defs, - token_configurations, - data_contact_config, - full_validation, - validation_operations, - platform_version, - ), version => Err(ProtocolError::UnknownVersionMismatch { method: "try_from_schema".to_string(), - known_versions: vec![0, 1, 2, 3, 4], + known_versions: vec![0, 1, 2, 3], received: version, }), } diff --git a/packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v3/keep_history_tests.rs b/packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v3/keep_history_tests.rs new file mode 100644 index 00000000000..f0ceb8f5dcc --- /dev/null +++ b/packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v3/keep_history_tests.rs @@ -0,0 +1,362 @@ +//! Regression tests for the `documentsKeepHistory` + `canBeDeleted` +//! cross-flag rule added in `try_from_schema` v3 (protocol version 14). +use super::*; +use platform_value::platform_value; + +/// Parses through the public dispatcher at the given protocol version so +/// the test exercises the same `try_from_schema` version routing consensus +/// code uses (v3 at protocol version 14, v2 at 12 and 13). +fn parse_at_version( + schema: Value, + protocol_version: u32, + full_validation: bool, +) -> Result { + let platform_version = + PlatformVersion::get(protocol_version).expect("expected platform version"); + let config = DataContractConfig::default_for_version(platform_version) + .expect("default config available"); + DocumentType::try_from_schema( + Identifier::new([1; 32]), + 1, + config.version(), + "test_doc", + schema, + None, + &BTreeMap::new(), + &config, + full_validation, + &mut vec![], + platform_version, + ) +} + +fn parse(schema: Value) -> Result { + parse_at_version(schema, 14, true) +} + +fn keep_history_deletable_schema() -> Value { + platform_value!({ + "type": "object", + "properties": { + "label": { + "type": "string", + "maxLength": 50, + "position": 0, + }, + }, + "additionalProperties": false, + "documentsKeepHistory": true, + "canBeDeleted": true, + }) +} + +/// `documentsKeepHistory: true` + `canBeDeleted: true` is +/// self-contradictory: rs-drive unconditionally refuses to delete +/// a document whose type keeps history +/// (`InvalidDeletionOfDocumentThatKeepsHistory`), so `canBeDeleted: +/// true` advertises a capability the storage layer will always +/// reject. The parser must reject the combination at contract +/// creation time so an SDK user gets a clean validation error +/// instead of the delete failing as an internal error at execution. +/// +/// With the `validation` feature enabled the rejection must surface +/// as `ProtocolError::ConsensusError` (not bare +/// `ProtocolError::DataContractError`) — drive-abci's +/// `transform_into_action_v0` only turns the consensus variant into +/// a clean invalid (paid) transition with a bump action; the +/// data-contract-error variant propagates as an internal execution +/// error in validator mode. +#[test] +fn doctype_keep_history_with_can_be_deleted_rejected() { + let result = parse(keep_history_deletable_schema()); + assert!( + result.is_err(), + "documentsKeepHistory: true + canBeDeleted: true must be rejected" + ); + let err = result.unwrap_err(); + let msg = format!("{:?}", err); + assert!( + msg.contains("documentsKeepHistory") && msg.contains("canBeDeleted"), + "error must reference both documentsKeepHistory and canBeDeleted; got {msg}" + ); + #[cfg(feature = "validation")] + assert!( + matches!(err, ProtocolError::ConsensusError(_)), + "with `validation` feature the rejection must be ProtocolError::ConsensusError so \ + drive-abci's transform_into_action turns it into an invalid (paid) transition \ + with a bump action rather than propagating as an internal execution error; got \ + {err:?}" + ); +} + +/// Omitting `canBeDeleted` exercises the contract-config default boundary: +/// the latest config defaults it to `true`, so a keep-history document type +/// remains contradictory and must be rejected during full validation. +#[test] +fn doctype_keep_history_with_can_be_deleted_omitted_rejected() { + let schema = platform_value!({ + "type": "object", + "properties": { + "label": { + "type": "string", + "maxLength": 50, + "position": 0, + }, + }, + "additionalProperties": false, + "documentsKeepHistory": true, + }); + let result = parse(schema); + assert!( + result.is_err(), + "omitted canBeDeleted must default to true and conflict with documentsKeepHistory" + ); + let msg = format!("{:?}", result.unwrap_err()); + assert!( + msg.contains("documentsKeepHistory") && msg.contains("canBeDeleted"), + "error must reference both documentsKeepHistory and defaulted canBeDeleted; got {msg}" + ); +} + +/// `documentsKeepHistory: true` + `canBeDeleted: true` is rejected +/// ONLY when `full_validation: true`. With `full_validation: false` +/// (the restore / migration / cache-warmup path) the same schema must +/// parse cleanly so already-deployed contradictory contracts continue +/// to load at v14+ — the drive-abci delete-transition guard turns +/// their deletes into clean invalid (paid) transitions instead of +/// rejecting them as internal errors at the contract-load layer. +#[test] +fn doctype_keep_history_with_can_be_deleted_accepted_without_full_validation() { + let document_type = parse_at_version(keep_history_deletable_schema(), 14, false).expect( + "documentsKeepHistory: true + canBeDeleted: true must be accepted when \ + full_validation: false so already-deployed contradictory contracts continue to load", + ); + assert!(document_type.documents_keep_history()); + assert!(document_type.documents_can_be_deleted()); +} + +/// Protocol version 12 routes to `try_from_schema` v2, which has no +/// cross-flag rule — the combination must stay accepted there even under +/// full validation, because v12 is released and consensus-frozen: +/// contracts accepted at v12 must replay identically. +#[test] +fn doctype_keep_history_with_can_be_deleted_accepted_at_protocol_version_12() { + let document_type = parse_at_version(keep_history_deletable_schema(), 12, true).expect( + "documentsKeepHistory: true + canBeDeleted: true must stay accepted at protocol \ + version 12 (consensus-frozen v2 parser) for replay compatibility", + ); + assert!(document_type.documents_keep_history()); + assert!(document_type.documents_can_be_deleted()); +} + +/// Guard against an over-broad fix: `documentsKeepHistory: true` + +/// `canBeDeleted: false` is consistent (the doctype is append-only) +/// and must continue to parse cleanly. The sibling omitted-key regression +/// covers the distinct default-`true` boundary and therefore expects +/// rejection rather than acceptance. +#[test] +fn doctype_keep_history_with_can_be_deleted_false_accepted() { + let schema = platform_value!({ + "type": "object", + "properties": { + "label": { + "type": "string", + "maxLength": 50, + "position": 0, + }, + }, + "additionalProperties": false, + "documentsKeepHistory": true, + "canBeDeleted": false, + }); + let document_type = parse(schema) + .expect("documentsKeepHistory: true + canBeDeleted: false is consistent and must parse"); + assert!(document_type.documents_keep_history()); + assert!(!document_type.documents_can_be_deleted()); +} + +/// Symmetric guard: `canBeDeleted: true` on a non-keep-history +/// doctype must continue to parse cleanly. Catches a predicate that +/// triggers on `canBeDeleted: true` alone instead of the AND. +#[test] +fn doctype_can_be_deleted_without_keep_history_accepted() { + let schema = platform_value!({ + "type": "object", + "properties": { + "label": { + "type": "string", + "maxLength": 50, + "position": 0, + }, + }, + "additionalProperties": false, + "canBeDeleted": true, + }); + let document_type = + parse(schema).expect("canBeDeleted: true without documentsKeepHistory must parse cleanly"); + assert!(!document_type.documents_keep_history()); + assert!(document_type.documents_can_be_deleted()); +} +#[test] +fn should_accept_contradictory_keep_history_schema_at_protocol_13() { + let document_type = parse_at_version(keep_history_deletable_schema(), 13, true) + .expect("released protocol 13 must still accept the schema"); + assert!(document_type.documents_keep_history()); + assert!(document_type.documents_can_be_deleted()); +} + +fn repair_schema(keep_history: bool, can_be_deleted: bool) -> Value { + platform_value!({ + "type": "object", + "properties": { + "label": {"type": "string", "maxLength": 50, "position": 0}, + }, + "additionalProperties": false, + "documentsKeepHistory": keep_history, + "canBeDeleted": can_be_deleted, + }) +} + +#[test] +fn should_repair_legacy_keep_history_delete_flag_at_protocol_14() { + let legacy_schemas = [ + repair_schema(true, true), + platform_value!({ + "type": "object", + "properties": { + "label": {"type": "string", "maxLength": 50, "position": 0}, + }, + "additionalProperties": false, + "documentsKeepHistory": true, + }), + ]; + for schema in legacy_schemas { + let old = parse_at_version(schema, 13, true).unwrap(); + let repaired = parse_at_version(repair_schema(true, false), 14, true).unwrap(); + let result = old + .as_ref() + .validate_update(repaired.as_ref(), 2, PlatformVersion::get(14).unwrap()) + .expect("repair must reach a consensus result"); + assert!(result.is_valid(), "repair rejected: {:?}", result.errors); + } +} + +#[test] +fn should_preserve_legacy_keep_history_repair_rejection_through_protocol_13() { + for protocol in [12, 13] { + let old = parse_at_version(repair_schema(true, true), protocol, true).unwrap(); + let repaired = parse_at_version(repair_schema(true, false), protocol, true).unwrap(); + let result = old + .as_ref() + .validate_update( + repaired.as_ref(), + 2, + PlatformVersion::get(protocol).unwrap(), + ) + .unwrap(); + assert!( + !result.is_valid(), + "protocol {protocol} must still reject repair" + ); + } +} + +#[test] +fn should_reject_other_delete_and_history_flag_changes_at_protocol_14() { + for (old_flags, new_flags) in [ + ((false, true), (false, false)), + ((false, false), (false, true)), + ((true, false), (true, true)), + ((true, true), (false, false)), + ((false, true), (true, false)), + ] { + let old = parse_at_version(repair_schema(old_flags.0, old_flags.1), 13, true).unwrap(); + // A caller may already have a parsed contract; update validation must + // enforce immutability even without the full-validation parser guard. + let new = parse_at_version(repair_schema(new_flags.0, new_flags.1), 14, false).unwrap(); + let result = old + .as_ref() + .validate_update(new.as_ref(), 2, PlatformVersion::get(14).unwrap()) + .unwrap(); + assert!( + !result.is_valid(), + "unexpectedly accepted {old_flags:?} -> {new_flags:?}" + ); + } +} + +#[test] +fn should_reject_incompatible_properties_during_keep_history_repair() { + let old = parse_at_version(repair_schema(true, true), 13, true).unwrap(); + let new = parse_at_version( + platform_value!({ + "type": "object", + "properties": { + "label": {"type": "integer", "position": 0}, + }, + "additionalProperties": false, + "documentsKeepHistory": true, + "canBeDeleted": false, + }), + 14, + true, + ) + .unwrap(); + let result = old + .as_ref() + .validate_update(new.as_ref(), 2, PlatformVersion::get(14).unwrap()) + .unwrap(); + assert!( + !result.is_valid(), + "repair must not bypass schema compatibility" + ); +} + +#[test] +fn should_reject_mutability_change_during_keep_history_repair() { + let old = parse_at_version(repair_schema(true, true), 13, true).unwrap(); + let mut schema = repair_schema(true, false); + schema.set_value("documentsMutable", false.into()).unwrap(); + let new = parse_at_version(schema, 14, true).unwrap(); + let result = old + .as_ref() + .validate_update(new.as_ref(), 2, PlatformVersion::get(14).unwrap()) + .unwrap(); + assert!( + !result.is_valid(), + "repair must not bypass other configuration checks" + ); +} + +#[test] +fn should_still_validate_property_named_can_be_deleted_during_keep_history_repair() { + let mut old_schema = repair_schema(true, true); + old_schema + .set_value( + "properties", + platform_value!({ + "canBeDeleted": {"type": "string", "maxLength": 50, "position": 0}, + }), + ) + .unwrap(); + let mut new_schema = old_schema.clone(); + new_schema.set_value("canBeDeleted", false.into()).unwrap(); + new_schema + .set_value( + "properties", + platform_value!({ + "canBeDeleted": {"type": "integer", "position": 0}, + }), + ) + .unwrap(); + let old = parse_at_version(old_schema, 13, true).unwrap(); + let new = parse_at_version(new_schema, 14, true).unwrap(); + let result = old + .as_ref() + .validate_update(new.as_ref(), 2, PlatformVersion::get(14).unwrap()) + .unwrap(); + assert!( + !result.is_valid(), + "only the top-level config flag may be stripped" + ); +} diff --git a/packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v3/mod.rs b/packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v3/mod.rs index c682f5bdd0d..3b3b4229dae 100644 --- a/packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v3/mod.rs +++ b/packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v3/mod.rs @@ -15,6 +15,8 @@ //! index-key length ceilings, and the constants they are derived from. use crate::data_contract::config::DataContractConfig; +use crate::data_contract::document_type::accessors::DocumentTypeV0Getters; +use crate::data_contract::document_type::class_methods::consensus_or_protocol_data_contract_error; // Only the ranked key-length rule below names `Index`, and it is validation-only. #[cfg(feature = "validation")] use crate::data_contract::document_type::index::Index; @@ -23,6 +25,7 @@ use crate::data_contract::document_type::index::IndexGrammarAdmissions; use crate::data_contract::document_type::property::DocumentPropertyType; use crate::data_contract::document_type::v2::DocumentTypeV2; use crate::data_contract::document_type::DocumentType; +use crate::data_contract::errors::DataContractError; use crate::data_contract::{TokenConfiguration, TokenContractPosition}; use crate::validation::operations::ProtocolValidationOperation; use crate::version::PlatformVersion; @@ -308,7 +311,10 @@ fn try_from_schema_generation_3( } impl DocumentType { - /// Dispatches to this module's generation-3 parser and wraps the result. + /// Parses protocol 14 document types and validates the keep-history/delete flags. + /// Stored contracts bypass full validation so legacy contradictory schemas + /// remain readable. Owners can repair them by setting `canBeDeleted: false` + /// during a contract update without changing history or the storage layout. #[allow(clippy::too_many_arguments)] pub(in crate::data_contract::document_type::class_methods) fn try_from_schema_v3( data_contract_id: Identifier, @@ -323,7 +329,7 @@ impl DocumentType { validation_operations: &mut impl Extend, platform_version: &PlatformVersion, ) -> Result { - try_from_schema_generation_3( + let document_type = try_from_schema_generation_3( data_contract_id, data_contract_system_version, contract_config_version, @@ -336,13 +342,36 @@ impl DocumentType { validation_operations, platform_version, ) - .map(DocumentType::V2) + .map(DocumentType::V2)?; + + // The flags are read from the parsed result (not the raw schema) so + // the check sees `canBeDeleted` resolved against the contract config + // default (`true` when the key is omitted). + if full_validation + && document_type.documents_keep_history() + && document_type.documents_can_be_deleted() + { + return Err(consensus_or_protocol_data_contract_error( + DataContractError::InvalidContractStructure(format!( + "document type \"{}\" sets both `documentsKeepHistory: true` and \ + `canBeDeleted: true`, but the storage layer unconditionally refuses to \ + delete a document whose type keeps history. Set `canBeDeleted` to false or \ + disable `documentsKeepHistory`.", + name, + )), + )); + } + + Ok(document_type) } } #[cfg(test)] mod index_only_tests; +#[cfg(test)] +mod keep_history_tests; + #[cfg(test)] mod tests { //! Ranked aggregate index keywords — parser-generation gating. diff --git a/packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v4/mod.rs b/packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v4/mod.rs deleted file mode 100644 index 6b460c77e41..00000000000 --- a/packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v4/mod.rs +++ /dev/null @@ -1,435 +0,0 @@ -use crate::data_contract::config::DataContractConfig; -use crate::data_contract::document_type::accessors::DocumentTypeV0Getters; -use crate::data_contract::document_type::class_methods::consensus_or_protocol_data_contract_error; -use crate::data_contract::document_type::DocumentType; -use crate::data_contract::errors::DataContractError; -use crate::data_contract::{TokenConfiguration, TokenContractPosition}; -use crate::validation::operations::ProtocolValidationOperation; -use crate::version::PlatformVersion; -use crate::ProtocolError; -use platform_value::{Identifier, Value}; -use std::collections::BTreeMap; - -impl DocumentType { - /// Adds the keep-history/delete cross-flag check to the protocol 14 parser. - /// Stored contracts bypass full validation so legacy contradictory schemas - /// remain readable. Owners can repair them by setting `canBeDeleted: false` - /// during a contract update; protocol 14 update validation permits exactly - /// that correction without changing history or the document storage layout. - /// Released protocols through 13 retain their original parser and outcomes. - #[allow(clippy::too_many_arguments)] - pub(in crate::data_contract::document_type::class_methods) fn try_from_schema_v4( - data_contract_id: Identifier, - data_contract_system_version: u16, - contract_config_version: u16, - name: &str, - schema: Value, - schema_defs: Option<&BTreeMap>, - token_configurations: &BTreeMap, - data_contact_config: &DataContractConfig, - full_validation: bool, - validation_operations: &mut impl Extend, - platform_version: &PlatformVersion, - ) -> Result { - let document_type = DocumentType::try_from_schema_v3( - data_contract_id, - data_contract_system_version, - contract_config_version, - name, - schema, - schema_defs, - token_configurations, - data_contact_config, - full_validation, - validation_operations, - platform_version, - )?; - - // The flags are read from the parsed result (not the raw schema) so - // the check sees `canBeDeleted` resolved against the contract config - // default (`true` when the key is omitted). - if full_validation - && document_type.documents_keep_history() - && document_type.documents_can_be_deleted() - { - return Err(consensus_or_protocol_data_contract_error( - DataContractError::InvalidContractStructure(format!( - "document type \"{}\" sets both `documentsKeepHistory: true` and \ - `canBeDeleted: true`, but the storage layer unconditionally refuses to \ - delete a document whose type keeps history. Set `canBeDeleted` to false or \ - disable `documentsKeepHistory`.", - name, - )), - )); - } - - Ok(document_type) - } -} - -#[cfg(test)] -mod tests { - //! Regression tests for the `documentsKeepHistory` + `canBeDeleted` - //! cross-flag rule added in `try_from_schema` v4 (protocol version 14). - use super::*; - use platform_value::platform_value; - - /// Parses through the public dispatcher at the given protocol version so - /// the test exercises the same `try_from_schema` version routing consensus - /// code uses (v4 at protocol version 14, v2 at 12 and 13). - fn parse_at_version( - schema: Value, - protocol_version: u32, - full_validation: bool, - ) -> Result { - let platform_version = - PlatformVersion::get(protocol_version).expect("expected platform version"); - let config = DataContractConfig::default_for_version(platform_version) - .expect("default config available"); - DocumentType::try_from_schema( - Identifier::new([1; 32]), - 1, - config.version(), - "test_doc", - schema, - None, - &BTreeMap::new(), - &config, - full_validation, - &mut vec![], - platform_version, - ) - } - - fn parse(schema: Value) -> Result { - parse_at_version(schema, 14, true) - } - - fn keep_history_deletable_schema() -> Value { - platform_value!({ - "type": "object", - "properties": { - "label": { - "type": "string", - "maxLength": 50, - "position": 0, - }, - }, - "additionalProperties": false, - "documentsKeepHistory": true, - "canBeDeleted": true, - }) - } - - /// `documentsKeepHistory: true` + `canBeDeleted: true` is - /// self-contradictory: rs-drive unconditionally refuses to delete - /// a document whose type keeps history - /// (`InvalidDeletionOfDocumentThatKeepsHistory`), so `canBeDeleted: - /// true` advertises a capability the storage layer will always - /// reject. The parser must reject the combination at contract - /// creation time so an SDK user gets a clean validation error - /// instead of the delete failing as an internal error at execution. - /// - /// With the `validation` feature enabled the rejection must surface - /// as `ProtocolError::ConsensusError` (not bare - /// `ProtocolError::DataContractError`) — drive-abci's - /// `transform_into_action_v0` only turns the consensus variant into - /// a clean invalid (paid) transition with a bump action; the - /// data-contract-error variant propagates as an internal execution - /// error in validator mode. - #[test] - fn doctype_keep_history_with_can_be_deleted_rejected() { - let result = parse(keep_history_deletable_schema()); - assert!( - result.is_err(), - "documentsKeepHistory: true + canBeDeleted: true must be rejected" - ); - let err = result.unwrap_err(); - let msg = format!("{:?}", err); - assert!( - msg.contains("documentsKeepHistory") && msg.contains("canBeDeleted"), - "error must reference both documentsKeepHistory and canBeDeleted; got {msg}" - ); - #[cfg(feature = "validation")] - assert!( - matches!(err, ProtocolError::ConsensusError(_)), - "with `validation` feature the rejection must be ProtocolError::ConsensusError so \ - drive-abci's transform_into_action turns it into an invalid (paid) transition \ - with a bump action rather than propagating as an internal execution error; got \ - {err:?}" - ); - } - - /// Omitting `canBeDeleted` exercises the contract-config default boundary: - /// the latest config defaults it to `true`, so a keep-history document type - /// remains contradictory and must be rejected during full validation. - #[test] - fn doctype_keep_history_with_can_be_deleted_omitted_rejected() { - let schema = platform_value!({ - "type": "object", - "properties": { - "label": { - "type": "string", - "maxLength": 50, - "position": 0, - }, - }, - "additionalProperties": false, - "documentsKeepHistory": true, - }); - let result = parse(schema); - assert!( - result.is_err(), - "omitted canBeDeleted must default to true and conflict with documentsKeepHistory" - ); - let msg = format!("{:?}", result.unwrap_err()); - assert!( - msg.contains("documentsKeepHistory") && msg.contains("canBeDeleted"), - "error must reference both documentsKeepHistory and defaulted canBeDeleted; got {msg}" - ); - } - - /// `documentsKeepHistory: true` + `canBeDeleted: true` is rejected - /// ONLY when `full_validation: true`. With `full_validation: false` - /// (the restore / migration / cache-warmup path) the same schema must - /// parse cleanly so already-deployed contradictory contracts continue - /// to load at v14+ — the drive-abci delete-transition guard turns - /// their deletes into clean invalid (paid) transitions instead of - /// rejecting them as internal errors at the contract-load layer. - #[test] - fn doctype_keep_history_with_can_be_deleted_accepted_without_full_validation() { - let document_type = parse_at_version(keep_history_deletable_schema(), 14, false).expect( - "documentsKeepHistory: true + canBeDeleted: true must be accepted when \ - full_validation: false so already-deployed contradictory contracts continue to load", - ); - assert!(document_type.documents_keep_history()); - assert!(document_type.documents_can_be_deleted()); - } - - /// Protocol version 12 routes to `try_from_schema` v2, which has no - /// cross-flag rule — the combination must stay accepted there even under - /// full validation, because v12 is released and consensus-frozen: - /// contracts accepted at v12 must replay identically. - #[test] - fn doctype_keep_history_with_can_be_deleted_accepted_at_protocol_version_12() { - let document_type = parse_at_version(keep_history_deletable_schema(), 12, true).expect( - "documentsKeepHistory: true + canBeDeleted: true must stay accepted at protocol \ - version 12 (consensus-frozen v2 parser) for replay compatibility", - ); - assert!(document_type.documents_keep_history()); - assert!(document_type.documents_can_be_deleted()); - } - - /// Guard against an over-broad fix: `documentsKeepHistory: true` + - /// `canBeDeleted: false` is consistent (the doctype is append-only) - /// and must continue to parse cleanly. The sibling omitted-key regression - /// covers the distinct default-`true` boundary and therefore expects - /// rejection rather than acceptance. - #[test] - fn doctype_keep_history_with_can_be_deleted_false_accepted() { - let schema = platform_value!({ - "type": "object", - "properties": { - "label": { - "type": "string", - "maxLength": 50, - "position": 0, - }, - }, - "additionalProperties": false, - "documentsKeepHistory": true, - "canBeDeleted": false, - }); - let document_type = parse(schema).expect( - "documentsKeepHistory: true + canBeDeleted: false is consistent and must parse", - ); - assert!(document_type.documents_keep_history()); - assert!(!document_type.documents_can_be_deleted()); - } - - /// Symmetric guard: `canBeDeleted: true` on a non-keep-history - /// doctype must continue to parse cleanly. Catches a predicate that - /// triggers on `canBeDeleted: true` alone instead of the AND. - #[test] - fn doctype_can_be_deleted_without_keep_history_accepted() { - let schema = platform_value!({ - "type": "object", - "properties": { - "label": { - "type": "string", - "maxLength": 50, - "position": 0, - }, - }, - "additionalProperties": false, - "canBeDeleted": true, - }); - let document_type = parse(schema) - .expect("canBeDeleted: true without documentsKeepHistory must parse cleanly"); - assert!(!document_type.documents_keep_history()); - assert!(document_type.documents_can_be_deleted()); - } - #[test] - fn should_accept_contradictory_keep_history_schema_at_protocol_13() { - let document_type = parse_at_version(keep_history_deletable_schema(), 13, true) - .expect("released protocol 13 must still accept the schema"); - assert!(document_type.documents_keep_history()); - assert!(document_type.documents_can_be_deleted()); - } - - fn repair_schema(keep_history: bool, can_be_deleted: bool) -> Value { - platform_value!({ - "type": "object", - "properties": { - "label": {"type": "string", "maxLength": 50, "position": 0}, - }, - "additionalProperties": false, - "documentsKeepHistory": keep_history, - "canBeDeleted": can_be_deleted, - }) - } - - #[test] - fn should_repair_legacy_keep_history_delete_flag_at_protocol_14() { - let legacy_schemas = [ - repair_schema(true, true), - platform_value!({ - "type": "object", - "properties": { - "label": {"type": "string", "maxLength": 50, "position": 0}, - }, - "additionalProperties": false, - "documentsKeepHistory": true, - }), - ]; - for schema in legacy_schemas { - let old = parse_at_version(schema, 13, true).unwrap(); - let repaired = parse_at_version(repair_schema(true, false), 14, true).unwrap(); - let result = old - .as_ref() - .validate_update(repaired.as_ref(), 2, PlatformVersion::get(14).unwrap()) - .expect("repair must reach a consensus result"); - assert!(result.is_valid(), "repair rejected: {:?}", result.errors); - } - } - - #[test] - fn should_preserve_legacy_keep_history_repair_rejection_through_protocol_13() { - for protocol in [12, 13] { - let old = parse_at_version(repair_schema(true, true), protocol, true).unwrap(); - let repaired = parse_at_version(repair_schema(true, false), protocol, true).unwrap(); - let result = old - .as_ref() - .validate_update( - repaired.as_ref(), - 2, - PlatformVersion::get(protocol).unwrap(), - ) - .unwrap(); - assert!( - !result.is_valid(), - "protocol {protocol} must still reject repair" - ); - } - } - - #[test] - fn should_reject_other_delete_and_history_flag_changes_at_protocol_14() { - for (old_flags, new_flags) in [ - ((false, true), (false, false)), - ((false, false), (false, true)), - ((true, false), (true, true)), - ((true, true), (false, false)), - ((false, true), (true, false)), - ] { - let old = parse_at_version(repair_schema(old_flags.0, old_flags.1), 13, true).unwrap(); - // A caller may already have a parsed contract; update validation must - // enforce immutability even without the full-validation parser guard. - let new = parse_at_version(repair_schema(new_flags.0, new_flags.1), 14, false).unwrap(); - let result = old - .as_ref() - .validate_update(new.as_ref(), 2, PlatformVersion::get(14).unwrap()) - .unwrap(); - assert!( - !result.is_valid(), - "unexpectedly accepted {old_flags:?} -> {new_flags:?}" - ); - } - } - - #[test] - fn should_reject_incompatible_properties_during_keep_history_repair() { - let old = parse_at_version(repair_schema(true, true), 13, true).unwrap(); - let new = parse_at_version( - platform_value!({ - "type": "object", - "properties": { - "label": {"type": "integer", "position": 0}, - }, - "additionalProperties": false, - "documentsKeepHistory": true, - "canBeDeleted": false, - }), - 14, - true, - ) - .unwrap(); - let result = old - .as_ref() - .validate_update(new.as_ref(), 2, PlatformVersion::get(14).unwrap()) - .unwrap(); - assert!( - !result.is_valid(), - "repair must not bypass schema compatibility" - ); - } - - #[test] - fn should_reject_mutability_change_during_keep_history_repair() { - let old = parse_at_version(repair_schema(true, true), 13, true).unwrap(); - let mut schema = repair_schema(true, false); - schema.set_value("documentsMutable", false.into()).unwrap(); - let new = parse_at_version(schema, 14, true).unwrap(); - let result = old - .as_ref() - .validate_update(new.as_ref(), 2, PlatformVersion::get(14).unwrap()) - .unwrap(); - assert!( - !result.is_valid(), - "repair must not bypass other configuration checks" - ); - } - - #[test] - fn should_still_validate_property_named_can_be_deleted_during_keep_history_repair() { - let mut old_schema = repair_schema(true, true); - old_schema - .set_value( - "properties", - platform_value!({ - "canBeDeleted": {"type": "string", "maxLength": 50, "position": 0}, - }), - ) - .unwrap(); - let mut new_schema = old_schema.clone(); - new_schema.set_value("canBeDeleted", false.into()).unwrap(); - new_schema - .set_value( - "properties", - platform_value!({ - "canBeDeleted": {"type": "integer", "position": 0}, - }), - ) - .unwrap(); - let old = parse_at_version(old_schema, 13, true).unwrap(); - let new = parse_at_version(new_schema, 14, true).unwrap(); - let result = old - .as_ref() - .validate_update(new.as_ref(), 2, PlatformVersion::get(14).unwrap()) - .unwrap(); - assert!( - !result.is_valid(), - "only the top-level config flag may be stripped" - ); - } -} diff --git a/packages/rs-drive/src/drive/contract/mod.rs b/packages/rs-drive/src/drive/contract/mod.rs index fa50e0d6241..fb24adfbd74 100644 --- a/packages/rs-drive/src/drive/contract/mod.rs +++ b/packages/rs-drive/src/drive/contract/mod.rs @@ -2339,7 +2339,7 @@ mod tests { // Now try to update with the same document type but documentsKeepHistory=true. // `canBeDeleted: false` is required alongside `documentsKeepHistory: true` — - // the schema parser (try_from_schema v4, protocol version 14+) rejects the + // the schema parser (try_from_schema v3, protocol version 14+) rejects the // keep-history + canBeDeleted combination (canBeDeleted's config default is // true), so the schema must opt out of delete to reach the intended // `ChangingDocumentTypeKeepsHistory` assertion at `update_contract`. diff --git a/packages/rs-platform-version/src/version/dpp_versions/dpp_contract_versions/v6.rs b/packages/rs-platform-version/src/version/dpp_versions/dpp_contract_versions/v6.rs index a05ef1f8284..ca9f6492769 100644 --- a/packages/rs-platform-version/src/version/dpp_versions/dpp_contract_versions/v6.rs +++ b/packages/rs-platform-version/src/version/dpp_versions/dpp_contract_versions/v6.rs @@ -11,8 +11,7 @@ use versioned_feature_core::FeatureVersionBounds; // pre-activation validation is unchanged — under v2 those keys still fail an // index entry's `additionalProperties: false`. // -// `try_from_schema` moves to 4, wrapping generation 3 with the keep-history/delete -// cross-flag check. Ranked grammar is still provided by the document-type parser +// `try_from_schema` moves to 3, selecting a new document-type parser // generation (`try_from_schema/v3`). New-protocol-version grammar gets its own // generation module rather than a version gate inside a shipped one, so the // generation-0/1/2 parsers stay byte-identical to the code consensus already @@ -20,6 +19,9 @@ use versioned_feature_core::FeatureVersionBounds; // it. Generation 3 admits the ranked keywords unconditionally — it exists if // and only if the meta-schema is v3, so it needs no version read of its own. // +// Generation 3 also rejects keep-history document types that allow deletion +// during full validation. Earlier parser generations retain released behavior. +// // `document_type_schema` moves to 3 in the same step: generation 3 and // meta-schema v3 are introduced together and pair by construction. Under v2 the // ranked keys still fail an index entry's `additionalProperties: false`, so v5 @@ -65,7 +67,7 @@ pub const CONTRACT_VERSIONS_V6: DPPContractVersions = DPPContractVersions { index_levels_from_indices: 0, }, class_method_versions: DocumentTypeClassMethodVersions { - try_from_schema: 4, // changed: generation 4 adds the keep-history/delete cross-flag check + try_from_schema: 3, // changed: parser generation 3 — generation 2 plus the ranked index keywords create_document_types_from_document_schemas: 1, }, structure_version: 0, diff --git a/packages/rs-platform-version/tests/keep_history_delete_versions.rs b/packages/rs-platform-version/tests/keep_history_delete_versions.rs index b42ccaf382a..81491b87f87 100644 --- a/packages/rs-platform-version/tests/keep_history_delete_versions.rs +++ b/packages/rs-platform-version/tests/keep_history_delete_versions.rs @@ -37,7 +37,7 @@ fn should_activate_keep_history_validation_at_protocol_14() { .document_type_versions .class_method_versions .try_from_schema, - 4 + 3 ); assert_eq!( version From d0dea835313468ceb6b469acbb0d8a2fae3a32f0 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Wed, 9 Sep 2026 04:05:40 +0700 Subject: [PATCH 3/5] refactor(dpp): validate history flags inside generation 3 parser --- .../class_methods/try_from_schema/v3/mod.rs | 47 +++++++++---------- 1 file changed, 21 insertions(+), 26 deletions(-) diff --git a/packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v3/mod.rs b/packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v3/mod.rs index 3b3b4229dae..92002b8c6ad 100644 --- a/packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v3/mod.rs +++ b/packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v3/mod.rs @@ -227,6 +227,9 @@ const RANKED_INDEX_KEY_LENGTH_CHECK: common::RankedIndexKeyLengthCheck = /// /// This parser is only reachable from protocol version 14+ (via /// CONTRACT_VERSIONS_V6). +/// Full validation rejects keep-history document types that allow deletion. +/// Stored contracts bypass this check so legacy contradictory schemas remain +/// readable and can be repaired by setting `canBeDeleted: false` on update. #[allow(clippy::too_many_arguments)] fn try_from_schema_generation_3( data_contract_id: Identifier, @@ -307,14 +310,26 @@ fn try_from_schema_generation_3( // indexOnly type does not have), so it has to see them already applied. common::apply_index_only(&mut v2, index_only, name)?; + // The flags are read from the parsed result (not the raw schema) so + // the check sees `canBeDeleted` resolved against the contract config + // default (`true` when the key is omitted). + if full_validation && v2.documents_keep_history() && v2.documents_can_be_deleted() { + return Err(consensus_or_protocol_data_contract_error( + DataContractError::InvalidContractStructure(format!( + "document type \"{}\" sets both `documentsKeepHistory: true` and \ + `canBeDeleted: true`, but the storage layer unconditionally refuses to \ + delete a document whose type keeps history. Set `canBeDeleted` to false or \ + disable `documentsKeepHistory`.", + name, + )), + )); + } + Ok(v2) } impl DocumentType { - /// Parses protocol 14 document types and validates the keep-history/delete flags. - /// Stored contracts bypass full validation so legacy contradictory schemas - /// remain readable. Owners can repair them by setting `canBeDeleted: false` - /// during a contract update without changing history or the storage layout. + /// Dispatches to this module's generation-3 parser and wraps the result. #[allow(clippy::too_many_arguments)] pub(in crate::data_contract::document_type::class_methods) fn try_from_schema_v3( data_contract_id: Identifier, @@ -329,7 +344,7 @@ impl DocumentType { validation_operations: &mut impl Extend, platform_version: &PlatformVersion, ) -> Result { - let document_type = try_from_schema_generation_3( + try_from_schema_generation_3( data_contract_id, data_contract_system_version, contract_config_version, @@ -342,27 +357,7 @@ impl DocumentType { validation_operations, platform_version, ) - .map(DocumentType::V2)?; - - // The flags are read from the parsed result (not the raw schema) so - // the check sees `canBeDeleted` resolved against the contract config - // default (`true` when the key is omitted). - if full_validation - && document_type.documents_keep_history() - && document_type.documents_can_be_deleted() - { - return Err(consensus_or_protocol_data_contract_error( - DataContractError::InvalidContractStructure(format!( - "document type \"{}\" sets both `documentsKeepHistory: true` and \ - `canBeDeleted: true`, but the storage layer unconditionally refuses to \ - delete a document whose type keeps history. Set `canBeDeleted` to false or \ - disable `documentsKeepHistory`.", - name, - )), - )); - } - - Ok(document_type) + .map(DocumentType::V2) } } From afa3cca13c86b36d84f223717d7229e714fddfe7 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Wed, 9 Sep 2026 04:15:31 +0700 Subject: [PATCH 4/5] refactor(dpp): share options for document type update validation --- .../methods/validate_update/common/mod.rs | 33 +++++++++++++------ .../methods/validate_update/v1/mod.rs | 21 ++++++------ 2 files changed, 33 insertions(+), 21 deletions(-) diff --git a/packages/rs-dpp/src/data_contract/document_type/methods/validate_update/common/mod.rs b/packages/rs-dpp/src/data_contract/document_type/methods/validate_update/common/mod.rs index 2a2bd7ac685..fb9bf77de5b 100644 --- a/packages/rs-dpp/src/data_contract/document_type/methods/validate_update/common/mod.rs +++ b/packages/rs-dpp/src/data_contract/document_type/methods/validate_update/common/mod.rs @@ -1,8 +1,8 @@ //! Helpers shared by every generation of `DocumentTypeRef::validate_update` //! (`v0`, `v1`, …). Only the parts of the update-validation flow that differ //! between generations live in the per-version modules; the config, byte-array -//! encoding and JSON-schema compatibility checks below are generation -//! independent. +//! encoding and JSON-schema compatibility checks below use options selected by +//! the versioned validator. use crate::consensus::basic::data_contract::IncompatibleDocumentTypeSchemaError; use crate::consensus::state::data_contract::document_type_update_error::DocumentTypeUpdateError; @@ -17,6 +17,15 @@ use crate::validation::SimpleConsensusValidationResult; use crate::ProtocolError; use platform_version::version::PlatformVersion; +/// Per-update exceptions selected by the versioned validator. Defaults retain +/// the original immutable-config behavior for earlier protocol versions. +#[derive(Default)] +pub(super) struct UpdateValidationOptions { + /// Allow the verified true-to-false deletion flag correction while both + /// the old and new document types keep history. + pub(super) allow_history_delete_repair: bool, +} + impl DocumentTypeRef<'_> { /// A byte array property whose `minItems == maxItems` is serialized as raw, /// fixed-length bytes with no length prefix; any other size bounds make it @@ -89,15 +98,15 @@ impl DocumentTypeRef<'_> { &self, new_document_type: DocumentTypeRef, ) -> SimpleConsensusValidationResult { - self.validate_config_with_history_delete_repair(new_document_type, false) + self.validate_config_with_options(new_document_type, &UpdateValidationOptions::default()) } /// Only protocol 14's update validator permits repairing the unusable delete /// flag. Earlier generations keep the original immutable-config behavior. - pub(super) fn validate_config_with_history_delete_repair( + pub(super) fn validate_config_with_options( &self, new_document_type: DocumentTypeRef, - allow_history_delete_repair: bool, + options: &UpdateValidationOptions, ) -> SimpleConsensusValidationResult { if new_document_type.creation_restriction_mode() != self.creation_restriction_mode() { return SimpleConsensusValidationResult::new_with_error( @@ -145,7 +154,7 @@ impl DocumentTypeRef<'_> { } if new_document_type.documents_can_be_deleted() != self.documents_can_be_deleted() - && !allow_history_delete_repair + && !options.allow_history_delete_repair { return SimpleConsensusValidationResult::new_with_error( DocumentTypeUpdateError::new( @@ -389,14 +398,18 @@ impl DocumentTypeRef<'_> { new_document_type: DocumentTypeRef, platform_version: &PlatformVersion, ) -> Result { - self.validate_schema_with_history_delete_repair(new_document_type, platform_version, false) + self.validate_schema_with_options( + new_document_type, + platform_version, + &UpdateValidationOptions::default(), + ) } - pub(super) fn validate_schema_with_history_delete_repair( + pub(super) fn validate_schema_with_options( &self, new_document_type: DocumentTypeRef, platform_version: &PlatformVersion, - allow_history_delete_repair: bool, + options: &UpdateValidationOptions, ) -> Result { // All good if schema is the same if self.schema() == new_document_type.schema() { @@ -430,7 +443,7 @@ impl DocumentTypeRef<'_> { } }; - if allow_history_delete_repair { + if options.allow_history_delete_repair { // The parsed flags already proved this is the one permitted config // correction. It changes neither property encoding nor history. // Strip only the top-level flag, including the legacy omitted-key diff --git a/packages/rs-dpp/src/data_contract/document_type/methods/validate_update/v1/mod.rs b/packages/rs-dpp/src/data_contract/document_type/methods/validate_update/v1/mod.rs index 95ceaf87c4d..9f8b980f842 100644 --- a/packages/rs-dpp/src/data_contract/document_type/methods/validate_update/v1/mod.rs +++ b/packages/rs-dpp/src/data_contract/document_type/methods/validate_update/v1/mod.rs @@ -29,6 +29,8 @@ use crate::validation::SimpleConsensusValidationResult; use crate::ProtocolError; use platform_version::version::PlatformVersion; +use super::common::UpdateValidationOptions; + impl DocumentTypeRef<'_> { #[inline(always)] pub(super) fn validate_update_v1( @@ -40,12 +42,13 @@ impl DocumentTypeRef<'_> { // Legacy keep-history types advertised deletes that Drive never allowed. // Permit only true -> false for that flag while keeping history enabled. // Every other config and schema check still runs, and v0 stays immutable. - let repair_history_delete = self.documents_keep_history() - && new_document_type.documents_keep_history() - && self.documents_can_be_deleted() - && !new_document_type.documents_can_be_deleted(); - let result = self - .validate_config_with_history_delete_repair(new_document_type, repair_history_delete); + let options = UpdateValidationOptions { + allow_history_delete_repair: self.documents_keep_history() + && new_document_type.documents_keep_history() + && self.documents_can_be_deleted() + && !new_document_type.documents_can_be_deleted(), + }; + let result = self.validate_config_with_options(new_document_type, &options); if !result.is_valid() { return Ok(result); @@ -75,11 +78,7 @@ impl DocumentTypeRef<'_> { } // Validate schema compatibility - self.validate_schema_with_history_delete_repair( - new_document_type, - platform_version, - repair_history_delete, - ) + self.validate_schema_with_options(new_document_type, platform_version, &options) } /// Top-level requiredness may only change in one way: a brand-new From d4e3dca20e96869b7c7972696206aa3363d01e76 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Wed, 9 Sep 2026 04:24:17 +0700 Subject: [PATCH 5/5] refactor(drive-abci): combine document deletion eligibility checks --- .../advanced_structure_v1/mod.rs | 53 ++++++++++--------- .../batch/tests/document/deletion.rs | 2 +- 2 files changed, 29 insertions(+), 26 deletions(-) diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/action_validation/document/document_delete_transition_action/advanced_structure_v1/mod.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/action_validation/document/document_delete_transition_action/advanced_structure_v1/mod.rs index 4f1570088df..b90d4f2a309 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/action_validation/document/document_delete_transition_action/advanced_structure_v1/mod.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/action_validation/document/document_delete_transition_action/advanced_structure_v1/mod.rs @@ -1,7 +1,6 @@ -use super::advanced_structure_v0::DocumentDeleteTransitionActionStructureValidationV0; use dpp::consensus::basic::document::{InvalidDocumentTransitionActionError, InvalidDocumentTypeError}; use dpp::data_contract::accessors::v0::DataContractV0Getters; -use dpp::data_contract::document_type::accessors::DocumentTypeV0Getters; +use dpp::data_contract::document_type::accessors::{DocumentTypeV0Getters, DocumentTypeV2Getters}; use dpp::validation::SimpleConsensusValidationResult; 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_delete_transition_action::DocumentDeleteTransitionAction; @@ -12,33 +11,17 @@ use crate::error::Error; pub(in crate::execution::validation::state_transition::state_transitions::batch::action_validation) trait DocumentDeleteTransitionActionStructureValidationV1 { fn validate_structure_v1(&self) -> Result; } - impl DocumentDeleteTransitionActionStructureValidationV1 for DocumentDeleteTransitionAction { - /// V1 runs all V0 checks, including the indexOnly transition-kind check, - /// then rejects deletes against legacy keep-history document types. - /// - /// Pre-V1, a delete against a keep-history doctype passed structure - /// validation, reached `force_delete_document_for_contract_operations_v0`, - /// and returned `DriveError::InvalidDeletionOfDocumentThatKeepsHistory`. - /// The batch processor reclassifies that drive-layer error as - /// `ExecutionResult::InternalError` — the transition is neither valid nor - /// invalid-paid, leaving the SDK with no clean accept/reject signal. - /// - /// Rejecting at the structure layer turns the contradiction into a normal - /// invalid (paid) consensus error. Gated behind a new validation version - /// (rather than mutating V0) so PROTOCOL_VERSION_13 and earlier chains — - /// which historically classified these deletes as InternalError — replay - /// bit-for-bit. See issue #3927. + /// Protocol 14 checks both deletion permission and history retention before + /// executing a delete. Legacy history-bearing types may advertise deletion, + /// but Drive refuses it; return the usual paid consensus rejection here. + /// V0 retains the historical InternalError outcome for protocol 13 and earlier. fn validate_structure_v1(&self) -> Result { - let result = self.validate_structure_v0()?; - if !result.is_valid() { - return Ok(result); - } - let contract_fetch_info = self.base().data_contract_fetch_info(); let data_contract = &contract_fetch_info.contract; let document_type_name = self.base().document_type_name(); + // Make sure that the document type is defined in the contract let Some(document_type) = data_contract.document_type_optional_for_name(document_type_name) else { return Ok(SimpleConsensusValidationResult::new_with_error( @@ -47,10 +30,30 @@ impl DocumentDeleteTransitionActionStructureValidationV1 for DocumentDeleteTrans )); }; - if document_type.documents_keep_history() { + // A legacy contract may enable both flags, but retaining history always + // prevents deletion, regardless of the advertised canBeDeleted value. + if !document_type.documents_can_be_deleted() || document_type.documents_keep_history() { + return Ok(SimpleConsensusValidationResult::new_with_error( + InvalidDocumentTransitionActionError::new(format!( + "documents of type {} can not be deleted", + document_type_name + )) + .into(), + )); + } + + // Pair the delete KIND with the doctype's storage mode: an + // indexOnly document has no primary-storage row a by-id delete + // could fetch, so its deletes must come as the indexOnlyDelete + // (delete-by-values) kind — its structure validation enforces the + // mirror rule. `index_only()` can only be true on a PV14+ + // contract, so this branch is unreachable for every historical + // transition. + if document_type.index_only() { return Ok(SimpleConsensusValidationResult::new_with_error( InvalidDocumentTransitionActionError::new(format!( - "documents of type {} keep history and therefore can not be deleted", + "documents of indexOnly type {} must be deleted with an indexOnlyDelete \ + (delete-by-values) transition carrying the document's values", document_type_name )) .into(), diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/deletion.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/deletion.rs index 00c96d719ad..0c47b893b05 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/deletion.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/deletion.rs @@ -592,7 +592,7 @@ mod deletion_tests { dpp::consensus::basic::BasicError::InvalidDocumentTransitionActionError(error) ), .. - }] if error.action() == "documents of type note keep history and therefore can not be deleted" + }] if error.action() == "documents of type note can not be deleted" ); } }