fix(platform): reject keep-history document deletes cleanly

packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v2/mod.rs

@@ -1,5 +1,7 @@
 use crate::data_contract::config::DataContractConfig;
-use crate::data_contract::document_type::class_methods::consensus_or_protocol_value_error;
+use crate::data_contract::document_type::class_methods::{
+    consensus_or_protocol_data_contract_error, consensus_or_protocol_value_error,
+};
 use crate::data_contract::document_type::property::DocumentPropertyType;
 use crate::data_contract::document_type::property_names::{
     DOCUMENTS_AVERAGEABLE, DOCUMENTS_COUNTABLE, DOCUMENTS_SUMMABLE, RANGE_AVERAGEABLE,
@@ -302,6 +304,40 @@ impl DocumentTypeV2 {
             platform_version,
         )?;
 
+        // `documentsKeepHistory: true` + `canBeDeleted: true` is self-contradictory:
+        // 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`.
+        //
+        // Gated by `full_validation` so already-deployed contradictory contracts
+        // (e.g. testnet `5CBPiadGmx3Zsjc26g5onopcx7pdxHPbrRAUD2T2yAbC` document
+        // type `note`) continue to load when re-parsed at v12+ — the drive-abci
+        // delete-transition guard turns their deletes into normal invalid (paid)
+        // transitions instead of internal errors at that layer.
+        //
+        // Use `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.
+        if full_validation && v1.documents_keep_history && v1.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 one of the two flags to \
+                     false (or omit it).",
+                    name,
+                )),
+            ));
+        }
+
         // Convert to V2 and set the new fields
         let mut v2: DocumentTypeV2 = v1.into();
         v2.documents_countable = documents_countable || range_countable;
@@ -689,6 +725,9 @@ mod tests {
     /// AND that both flags survive into the parsed `v2`.
     #[test]
     fn doctype_keep_history_with_documents_summable_accepted() {
+        // `canBeDeleted: false` is required alongside `documentsKeepHistory: true`
+        // because the contract config's default for `canBeDeleted` is `true` and
+        // the cross-flag check rejects `keepHistory && canBeDeleted`.
         let schema = platform_value!({
             "type": "object",
             "properties": {
@@ -702,6 +741,7 @@ mod tests {
             "required": ["score"],
             "additionalProperties": false,
             "documentsKeepHistory": true,
+            "canBeDeleted": false,
             "documentsSummable": "score",
         });
         let v2 = parse(schema).expect(
@@ -727,6 +767,8 @@ mod tests {
     /// `CountSumTree` / `ProvableCountSumTree` variant.
     #[test]
     fn doctype_keep_history_with_documents_averageable_accepted() {
+        // `canBeDeleted: false` is required alongside `documentsKeepHistory: true`
+        // — see sibling `doctype_keep_history_with_documents_summable_accepted`.
         let schema = platform_value!({
             "type": "object",
             "properties": {
@@ -740,6 +782,7 @@ mod tests {
             "required": ["score"],
             "additionalProperties": false,
             "documentsKeepHistory": true,
+            "canBeDeleted": false,
             "documentsAverageable": "score",
         });
         let v2 = parse(schema).expect(
@@ -758,6 +801,8 @@ mod tests {
     /// every existing keep-history doctype.
     #[test]
     fn doctype_keep_history_without_summable_accepted() {
+        // `canBeDeleted: false` is required alongside `documentsKeepHistory: true`
+        // — see sibling `doctype_keep_history_with_documents_summable_accepted`.
         let schema = platform_value!({
             "type": "object",
             "properties": {
@@ -769,6 +814,7 @@ mod tests {
             },
             "additionalProperties": false,
             "documentsKeepHistory": true,
+            "canBeDeleted": false,
         });
         let v2 = parse(schema).expect("keep-history without summable must parse cleanly");
         assert!(
@@ -920,6 +966,157 @@ mod tests {
         );
     }
 
+    /// `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 schema = platform_value!({
+            "type": "object",
+            "properties": {
+                "label": {
+                    "type": "string",
+                    "maxLength": 50,
+                    "position": 0,
+                },
+            },
+            "additionalProperties": false,
+            "documentsKeepHistory": true,
+            "canBeDeleted": true,
+        });
+        let result = parse(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:?}"
+        );
+    }
+
+    /// `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 v12+ — 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.
+    /// Mirrors the gating in `try_from_schema` (search for
+    /// `full_validation && v1.documents_keep_history`).
+    #[test]
+    fn doctype_keep_history_with_can_be_deleted_accepted_without_full_validation() {
+        let schema = platform_value!({
+            "type": "object",
+            "properties": {
+                "label": {
+                    "type": "string",
+                    "maxLength": 50,
+                    "position": 0,
+                },
+            },
+            "additionalProperties": false,
+            "documentsKeepHistory": true,
+            "canBeDeleted": true,
+        });
+        let platform_version = PlatformVersion::latest();
+        let config = DataContractConfig::default_for_version(platform_version)
+            .expect("default config available on latest platform version");
+        let v2 = DocumentTypeV2::try_from_schema(
+            Identifier::new([1; 32]),
+            1,
+            config.version(),
+            "test_doc",
+            schema,
+            None,
+            &BTreeMap::new(),
+            &config,
+            false,
+            &mut vec![],
+            platform_version,
+        )
+        .expect(
+            "documentsKeepHistory: true + canBeDeleted: true must be accepted when \
+             full_validation: false so already-deployed contradictory contracts continue to load",
+        );
+        assert!(v2.documents_keep_history);
+        assert!(v2.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. Same for `documentsKeepHistory:
+    /// true` with `canBeDeleted` omitted — covered by the existing
+    /// `doctype_keep_history_without_summable_accepted` test, which
+    /// leaves `canBeDeleted` at its config default (false).
+    #[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 v2 = parse(schema).expect(
+            "documentsKeepHistory: true + canBeDeleted: false is consistent and must parse",
+        );
+        assert!(v2.documents_keep_history);
+        assert!(!v2.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 v2 = parse(schema)
+            .expect("canBeDeleted: true without documentsKeepHistory must parse cleanly");
+        assert!(!v2.documents_keep_history);
+        assert!(v2.documents_can_be_deleted);
+    }
+
     /// Symmetric: `documentsSummable` on a NON-keep-history doctype
     /// stays valid. Guards against a rejection that triggers on
     /// summable alone instead of the AND.

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<SimpleConsensusValidationResult, Error>;
+}
+
+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_11 and earlier chains —
+    /// which historically classified these deletes as InternalError — replay
+    /// bit-for-bit. See issue #3927.
+    fn validate_structure_v1(&self) -> Result<SimpleConsensusValidationResult, Error> {
+        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())
+    }
+}

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,
             })),
         }