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 bd950436ea6..8e872772233 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 @@ -336,6 +336,39 @@ mod tests { ); } + // Reordering the `indices` array without changing the definition set is + // a semantic no-op (indices are keyed by name), so the name-keyed + // comparison passes — and the schema-compatibility check must not trip + // over the surviving `/indices` JSON diff. Under protocol v13 that diff + // hit the unsupported-keyword hard error (an internal error, not a + // consensus-invalid result); at v14 `validate_schema_compatibility` v1 + // strips `indices` before diffing and the update validates cleanly. + #[test] + fn should_pass_when_indices_are_reordered_without_changes() { + let platform_version = PlatformVersion::latest(); + + let old = old_doc_type(platform_version); + + let new = doc_type_with_indices( + platform_value!([ + {"name": "k", "properties": [{"a": "asc"}, {"b": "asc"}]}, + {"name": "j", "properties": [{"c": "asc"}]}, + ]), + platform_version, + ); + + let result = old + .as_ref() + .validate_update(new.as_ref(), platform_version) + .expect("validate_update should not error"); + + assert!( + result.is_valid(), + "a reorder-only indices update should be accepted, got {:?}", + result.errors + ); + } + #[test] fn should_pass_when_indices_are_unchanged() { let platform_version = PlatformVersion::latest(); diff --git a/packages/rs-dpp/src/data_contract/document_type/schema/validate_schema_compatibility/mod.rs b/packages/rs-dpp/src/data_contract/document_type/schema/validate_schema_compatibility/mod.rs index b55f51a3385..0bbca751f45 100644 --- a/packages/rs-dpp/src/data_contract/document_type/schema/validate_schema_compatibility/mod.rs +++ b/packages/rs-dpp/src/data_contract/document_type/schema/validate_schema_compatibility/mod.rs @@ -3,6 +3,7 @@ use crate::ProtocolError; use platform_version::version::PlatformVersion; mod v0; +mod v1; #[cfg(test)] mod byte_array_widen_accepted_tests; @@ -28,9 +29,10 @@ pub fn validate_schema_compatibility( .validate_schema_compatibility { 0 => v0::validate_schema_compatibility_v0(original_schema, new_schema), + 1 => v1::validate_schema_compatibility_v1(original_schema, new_schema), version => Err(ProtocolError::UnknownVersionMismatch { method: "validate_schema_compatibility".to_string(), - known_versions: vec![0], + known_versions: vec![0, 1], received: version, }), } diff --git a/packages/rs-dpp/src/data_contract/document_type/schema/validate_schema_compatibility/v1/mod.rs b/packages/rs-dpp/src/data_contract/document_type/schema/validate_schema_compatibility/v1/mod.rs new file mode 100644 index 00000000000..bd369d8ed2b --- /dev/null +++ b/packages/rs-dpp/src/data_contract/document_type/schema/validate_schema_compatibility/v1/mod.rs @@ -0,0 +1,252 @@ +//! Protocol v14 generation of the JSON-schema compatibility check. +//! +//! The compatibility validator walks the JSON diff between the old and new +//! document type schemas and hard-errors (`UnsupportedSchemaKeywordError`, +//! surfaced as an internal error rather than a consensus-invalid result) on +//! any keyword it has no rule for — and it has no rule for `indices`. Index +//! changes are not this check's concern: `validate_update` v1 compares the +//! parsed index definitions by name and rejects any added, removed or +//! modified index with a clean consensus error before schema compatibility +//! runs. The only `/indices` diff that could survive to this check is a +//! reordering of the array that leaves the definition set identical — a +//! semantic no-op (indices are keyed by name) that under v0 still hit the +//! hard error. +//! +//! v1 therefore strips the top-level `indices` key from both schemas before +//! diffing, so index definitions are validated in exactly one place. Only +//! the document type's own `indices` keyword is removed; a *property* named +//! `indices` lives under `/properties/indices` and is still validated. + +use crate::data_contract::document_type::schema::IncompatibleJsonSchemaOperation; +use crate::data_contract::errors::{DataContractError, JsonSchemaError}; +use crate::data_contract::JsonValue; +use crate::validation::SimpleValidationResult; +use crate::ProtocolError; +use json_schema_compatibility_validator::{ + validate_schemas_compatibility, CompatibilityRulesCollection, Options, + KEYWORD_COMPATIBILITY_RULES, +}; +use once_cell::sync::Lazy; +use std::borrow::Cow; +use std::ops::Deref; + +static OPTIONS: Lazy = Lazy::new(|| { + let mut required_rule = KEYWORD_COMPATIBILITY_RULES + .get("required") + .expect("required rule must be present") + .clone(); + + required_rule.allow_removal = false; + required_rule + .inner + .as_mut() + .expect("required rule must have inner rules") + .allow_removal = false; + + Options { + override_rules: CompatibilityRulesCollection::from_iter([("required", required_rule)]), + } +}); + +fn without_indices(schema: &JsonValue) -> Cow<'_, JsonValue> { + match schema { + JsonValue::Object(map) if map.contains_key("indices") => { + let mut map = map.clone(); + map.remove("indices"); + Cow::Owned(JsonValue::Object(map)) + } + _ => Cow::Borrowed(schema), + } +} + +/// Pairing invariant: stripping `indices` unconditionally is only safe +/// because every `PlatformVersion` that selects this generation +/// (`validate_schema_compatibility: 1`) also selects a `validate_update` +/// generation of at least 1 (`dpp.validation.document_type.validate_update`), +/// which rejects every real index change before this check runs. A future +/// version table that bumps one without the other would let index changes +/// bypass compatibility validation entirely. +pub(super) fn validate_schema_compatibility_v1( + original_schema: &JsonValue, + new_schema: &JsonValue, +) -> Result, ProtocolError> { + let original_schema = without_indices(original_schema); + let new_schema = without_indices(new_schema); + + validate_schemas_compatibility(&original_schema, &new_schema, OPTIONS.deref()) + .map(|result| { + let errors = result + .into_changes() + .into_iter() + .map(|change| IncompatibleJsonSchemaOperation { + name: change.name().to_string(), + path: change.path().to_string(), + }) + .collect::>(); + + SimpleValidationResult::new_with_errors(errors) + }) + .map_err(|error| { + ProtocolError::DataContractError(DataContractError::JsonSchema( + JsonSchemaError::SchemaCompatibilityValidationError(error.to_string()), + )) + }) +} + +#[cfg(test)] +mod tests { + use super::super::validate_schema_compatibility; + use crate::data_contract::errors::{DataContractError, JsonSchemaError}; + use crate::ProtocolError; + use assert_matches::assert_matches; + use platform_version::version::PlatformVersion; + use serde_json::json; + + #[test] + fn should_ignore_indices_reordering() { + let platform_version = PlatformVersion::latest(); + + let original_schema = json!({ + "type": "object", + "properties": { + "a": {"type": "string", "position": 0}, + "b": {"type": "string", "position": 1}, + }, + "indices": [ + {"name": "j", "properties": [{"a": "asc"}]}, + {"name": "k", "properties": [{"b": "asc"}]}, + ], + "additionalProperties": false, + }); + + let new_schema = json!({ + "type": "object", + "properties": { + "a": {"type": "string", "position": 0}, + "b": {"type": "string", "position": 1}, + }, + "indices": [ + {"name": "k", "properties": [{"b": "asc"}]}, + {"name": "j", "properties": [{"a": "asc"}]}, + ], + "additionalProperties": false, + }); + + let result = validate_schema_compatibility(&original_schema, &new_schema, platform_version) + .expect("an indices-only diff must not error"); + + assert!( + result.is_valid(), + "an indices-only diff must be ignored, got {:?}", + result.errors + ); + } + + // Stripping `indices` must not mask incompatible changes elsewhere in + // the schema. + #[test] + fn should_still_report_incompatible_property_change_alongside_indices_diff() { + let platform_version = PlatformVersion::latest(); + + let original_schema = json!({ + "type": "object", + "properties": { + "a": {"type": "string", "position": 0}, + }, + "indices": [ + {"name": "j", "properties": [{"a": "asc"}]}, + ], + "additionalProperties": false, + }); + + let new_schema = json!({ + "type": "object", + "properties": { + "a": {"type": "number", "position": 0}, + }, + "indices": [ + {"name": "k", "properties": [{"a": "asc"}]}, + ], + "additionalProperties": false, + }); + + let result = validate_schema_compatibility(&original_schema, &new_schema, platform_version) + .expect("schema compatibility validation should not error"); + + assert_matches!( + result.errors.as_slice(), + [change] if change.name == "replace" && change.path == "/properties/a/type" + ); + } + + // Only the document type's own top-level `indices` keyword is stripped; + // a property that happens to be named "indices" sits under + // `/properties/indices` and must still be validated. + #[test] + fn should_still_validate_property_named_indices() { + let platform_version = PlatformVersion::latest(); + + let original_schema = json!({ + "type": "object", + "properties": { + "indices": {"type": "string", "position": 0}, + }, + "additionalProperties": false, + }); + + let new_schema = json!({ + "type": "object", + "properties": { + "indices": {"type": "number", "position": 0}, + }, + "additionalProperties": false, + }); + + let result = validate_schema_compatibility(&original_schema, &new_schema, platform_version) + .expect("schema compatibility validation should not error"); + + assert_matches!( + result.errors.as_slice(), + [change] if change.name == "replace" && change.path == "/properties/indices/type" + ); + } + + // Replay-safety pin: protocol version 13 dispatches to v0, where an + // `/indices` diff still hits the unsupported-keyword hard error. + #[test] + fn v0_should_error_on_indices_diff() { + let platform_version = PlatformVersion::get(13).expect("protocol version 13 must exist"); + + let original_schema = json!({ + "type": "object", + "properties": { + "a": {"type": "string", "position": 0}, + }, + "indices": [ + {"name": "j", "properties": [{"a": "asc"}]}, + ], + "additionalProperties": false, + }); + + let new_schema = json!({ + "type": "object", + "properties": { + "a": {"type": "string", "position": 0}, + }, + "indices": [ + {"name": "j", "properties": [{"a": "asc"}], "unique": true}, + ], + "additionalProperties": false, + }); + + let error = validate_schema_compatibility(&original_schema, &new_schema, platform_version) + .expect_err("an indices diff must hard-error under v0"); + + assert_matches!( + error, + ProtocolError::DataContractError(DataContractError::JsonSchema( + JsonSchemaError::SchemaCompatibilityValidationError(message) + )) if message == "schema keyword 'indices' at path '/indices/0/unique' is not supported" + ); + } +} 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 5268c041c68..712faccc3de 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 @@ -23,6 +23,15 @@ use versioned_feature_core::FeatureVersionBounds; // 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 // (protocol version 13) keeps pre-activation validation unchanged. +// +// `validate_schema_compatibility` moves to 1: the compatibility validator has +// no keyword rule for `indices`, so under generation 0 any contract-update +// schema diff under `/indices` that survived the index checks hard-errored +// (an internal error, not a consensus-invalid result). Index definitions are +// validated by `validate_update` v1's name-keyed comparison at protocol +// version 14, so generation 1 strips the top-level `indices` key before +// diffing — an index-order-only update (a semantic no-op) now validates +// cleanly, while real index changes keep their dedicated consensus error. pub const CONTRACT_VERSIONS_V6: DPPContractVersions = DPPContractVersions { max_serialized_size: 65000, contract_serialization_version: FeatureVersionBounds { @@ -64,7 +73,7 @@ pub const CONTRACT_VERSIONS_V6: DPPContractVersions = DPPContractVersions { recursive_schema_validator_versions: RecursiveSchemaValidatorVersions { traversal_validator: 0, }, - validate_schema_compatibility: 0, + validate_schema_compatibility: 1, // changed: strips `indices` before diffing — index changes are validated by `validate_update` v1, so an index-order-only diff no longer hard-errors }, methods: DocumentTypeMethodVersions { create_document_from_data: 0, diff --git a/packages/rs-platform-version/src/version/v14.rs b/packages/rs-platform-version/src/version/v14.rs index 717d0d25924..a5c0aa7433b 100644 --- a/packages/rs-platform-version/src/version/v14.rs +++ b/packages/rs-platform-version/src/version/v14.rs @@ -75,14 +75,20 @@ pub const PROTOCOL_VERSION_14: ProtocolVersion = 14; /// shared-prefix shapes. /// /// Until a contract uses the ranked grammar, the only v14 behavior changes -/// are the shared-prefix fix and the contested-index cross-check; everything -/// else matches v13: +/// are the shared-prefix fix, the contested-index cross-check and the +/// index-reorder schema-compatibility fix; everything else matches v13: /// /// * `CONTRACT_VERSIONS_V6` points `document_type_schema` at the v3 document /// meta-schema, which hosts the ranked index keywords /// (`rankedCountable` / `rankedSummable` / `rankedAverageable`). v13 keeps /// validating against meta-schema v2, where those keys are rejected as /// unknown properties, so a pre-v14 contract cannot smuggle them in. +/// It also bumps `validate_schema_compatibility` to 1, which strips the +/// top-level `indices` key before diffing the old and new document type +/// schemas: index immutability is enforced by `validate_update` v1's +/// name-keyed comparison, so a contract update that merely reorders the +/// `indices` array validates cleanly instead of hitting the +/// unsupported-keyword hard error (an internal error under v13). /// * `DRIVE_VERSION_V9` carries `DRIVE_DOCUMENT_METHOD_VERSIONS_V4`, adding /// the `detect_ranked_mode` routing slot, plus the grove-method slots for /// creating the three indexed tree variants and the verify-method slot for