feat(platform)!: a private type's update may only add properties after its existing ones (PV15)

book/src/contract-keywords.md

@@ -146,7 +146,7 @@ Every key a contract can write, grouped by where it goes. **Since** is the proto
 | `rangeCountable`, `rangeSummable`, `rangeAverageable` | boolean | Provable counts, sums or averages over ranges of document ids. | 12 | [Document type range keys](contract-keywords/aggregates.md#document-type-rangecountable-rangesummable-rangeaverageable) |
 | `indexOnly` | boolean | Documents are never stored whole: the index entries are the rows. | 14 | [indexOnly](contract-keywords/index-only.md#indexonly) · [internals](drive/index-only-document-types.md) |
 | `entryPayload` | array of 1 to 16 names | On an index-only type, properties carried in each entry's value instead of a key. | 14 | [entryPayload](contract-keywords/index-only.md#entrypayload) |
-| `private` | object: `entrySize`, 1 to 4096 | A private type: its documents are encrypted entries of `entrySize` bytes, which the chain never reads. Its `properties` describe the plaintext. It declares only `type`, `properties`, `required`, `additionalProperties`, `minProperties`, `maxProperties`, `dependentRequired`, `$comment`, `description`, the two bounded key keys, and `documentsMutable`, `canBeDeleted` and `documentsKeepHistory` as `false`, each at most once; neither its properties nor `required` nor `dependentRequired` name a system field, no property at any depth declares `refersTo`, `distinctFrom`, `generatedFrom`, `encryptedFor` or `requiredSince`, and the contract's elected moderators may not moderate it. No reference, `countOf` or `sumOf` may name it, an update cannot change it, and a Batch refuses every document transition on it (`DocumentTypePrivateError`, 10425). Entries are appended only by `PrivateDocumentCreateFromShieldedPool` (type 29), from 1 to 16 distinct entries of exactly `entrySize` bytes, paid from the credit shielded pool. The chain does not compare `entrySize` with the properties; the SDK refuses to register a private type whose `entrySize` is below its largest payload plus the private document envelope's 299 bytes, or whose properties are not all bounded (`minimum_private_document_entry_size` in dpp). The largest payload is read from the property types alone: an `enum` or a `maxProperties` that would make it smaller is not counted. | 15 | |
+| `private` | object: `entrySize`, 1 to 4096 | A private type: its documents are encrypted entries of `entrySize` bytes, which the chain never reads. Its `properties` describe the plaintext. It declares only `type`, `properties`, `required`, `additionalProperties`, `minProperties`, `maxProperties`, `dependentRequired`, `$comment`, `description`, the two bounded key keys, and `documentsMutable`, `canBeDeleted` and `documentsKeepHistory` as `false`, each at most once; neither its properties nor `required` nor `dependentRequired` name a system field, no property at any depth declares `refersTo`, `distinctFrom`, `generatedFrom`, `encryptedFor` or `requiredSince`, and the contract's elected moderators may not moderate it. No reference, `countOf` or `sumOf` may name it, an update cannot change it and may only add properties after the existing ones, at the end of the type and of each object (its entries are read with the type as it is now), and a Batch refuses every document transition on it (`DocumentTypePrivateError`, 10425). Entries are appended only by `PrivateDocumentCreateFromShieldedPool` (type 29), from 1 to 16 distinct entries of exactly `entrySize` bytes, paid from the credit shielded pool. The chain does not compare `entrySize` with the properties; the SDK refuses to register a private type whose `entrySize` is below its largest payload plus the private document envelope's 299 bytes, or whose properties are not all bounded (`minimum_private_document_entry_size` in dpp). The largest payload is read from the property types alone: an `enum` or a `maxProperties` that would make it smaller is not counted. | 15 | |
 
 ### Property
 

packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v3/private_document_tests.rs

@@ -944,6 +944,215 @@ fn should_refuse_an_update_changing_whether_a_type_is_private_or_its_entry_size(
     assert!(unchanged.is_valid(), "{:?}", unchanged.errors);
 }
 
+/// A private type of `properties`, in their map order.
+fn private_type_with(properties: Value) -> DocumentType {
+    parse(
+        private_schema(platform_value!({ "properties": properties })),
+        false,
+    )
+    .expect("parses")
+}
+
+fn account_reference() -> Value {
+    platform_value!({ "type": "integer", "minimum": 0, "maximum": 4_294_967_295u64, "position": 0 })
+}
+
+fn encrypted_public_key() -> Value {
+    platform_value!({ "type": "array", "byteArray": true, "minItems": 96, "maxItems": 96, "position": 1 })
+}
+
+/// An object property at `position` of `members`, in their map order.
+fn object(members: Value, position: u32) -> Value {
+    platform_value!({
+        "type": "object",
+        "properties": members,
+        "additionalProperties": false,
+        "position": position
+    })
+}
+
+fn boolean(position: u32) -> Value {
+    platform_value!({ "type": "boolean", "position": position })
+}
+
+/// An entry of a private type is immutable and is read with the type as it is now, in the order
+/// the type parses its properties (by `position`) and each object's members (in the schema's
+/// map order), so an update may only add properties after the existing ones, at the end of the
+/// type and of every object. One putting a property before them, or reordering them, is refused;
+/// appending is accepted, however the schema's map lists the type's own properties. These types
+/// are parsed without full validation, which would require top-level positions without a gap:
+/// the gap case is defensive, and the registrable case follows.
+#[test]
+fn should_refuse_an_update_that_does_not_keep_a_private_type_s_property_order() {
+    let platform_version = PlatformVersion::latest();
+    // `meta` at position 4 leaves positions 2 and 3 free for a property to sort before it.
+    let meta = |members: Value| object(members, 4);
+    let old = private_type_with(platform_value!({
+        "accountReference": account_reference(),
+        "encryptedPublicKey": encrypted_public_key(),
+        "meta": meta(platform_value!({ "y": boolean(0), "z": boolean(1) })),
+    }));
+    let update = |new: &DocumentType| {
+        old.as_ref()
+            .validate_update(new.as_ref(), 2, platform_version)
+            .expect("validate_update should not error")
+    };
+
+    for (case, new, list) in [
+        (
+            "a member put before the object's existing ones",
+            private_type_with(platform_value!({
+                "accountReference": account_reference(),
+                "encryptedPublicKey": encrypted_public_key(),
+                "meta": meta(platform_value!({ "x": boolean(2), "y": boolean(0), "z": boolean(1) })),
+            })),
+            "the object meta",
+        ),
+        (
+            "the object's members reordered",
+            private_type_with(platform_value!({
+                "accountReference": account_reference(),
+                "encryptedPublicKey": encrypted_public_key(),
+                "meta": meta(platform_value!({ "z": boolean(1), "y": boolean(0) })),
+            })),
+            "the object meta",
+        ),
+        (
+            "a property at a position before an existing one",
+            private_type_with(platform_value!({
+                "accountReference": account_reference(),
+                "encryptedPublicKey": encrypted_public_key(),
+                "meta": meta(platform_value!({ "y": boolean(0), "z": boolean(1) })),
+                "note": boolean(3),
+            })),
+            "the type",
+        ),
+    ] {
+        let result = update(&new);
+        assert!(
+            result.errors.iter().any(|error| matches!(
+                error,
+                ConsensusError::StateError(StateError::DocumentTypeUpdateError(error))
+                    if error.additional_message().contains("can only add properties after")
+                        && error.additional_message().contains(list)
+            )),
+            "{case}: expected the property order refusal naming {list}, got {:?}",
+            result.errors
+        );
+    }
+
+    for (case, new) in [
+        (
+            "a member added after the object's existing ones",
+            private_type_with(platform_value!({
+                "accountReference": account_reference(),
+                "encryptedPublicKey": encrypted_public_key(),
+                "meta": meta(platform_value!({ "y": boolean(0), "z": boolean(1), "x": boolean(2) })),
+            })),
+        ),
+        (
+            "a property at a position after the existing ones, listed first",
+            private_type_with(platform_value!({
+                "note": boolean(5),
+                "accountReference": account_reference(),
+                "encryptedPublicKey": encrypted_public_key(),
+                "meta": meta(platform_value!({ "y": boolean(0), "z": boolean(1) })),
+            })),
+        ),
+    ] {
+        let result = update(&new);
+        assert!(result.is_valid(), "{case}: {:?}", result.errors);
+    }
+}
+
+/// The same rule on a type that registers under full validation (top-level positions run on
+/// without a gap) and one level deeper: `meta` holds an unchanged object `side` before the object
+/// `inner` an update changes, so the check must look past an unchanged sibling and into a nested
+/// object. Prepending to `inner`, reordering it or removing one of its members is refused and
+/// names `the object meta.inner`; appending to it, or appending a property to the type at the
+/// next position, is accepted.
+#[test]
+fn should_keep_the_property_order_of_a_registrable_private_type_at_every_depth() {
+    let platform_version = PlatformVersion::latest();
+    let registrable = |inner: Value, extra: Option<(&str, Value)>| {
+        let mut properties = platform_value!({
+            "accountReference": account_reference(),
+            "encryptedPublicKey": encrypted_public_key(),
+            "meta": object(
+                platform_value!({
+                    "side": object(platform_value!({ "s": boolean(0) }), 0),
+                    "inner": object(inner, 1),
+                }),
+                2
+            ),
+        });
+        if let (Value::Map(map), Some((name, property))) = (&mut properties, extra) {
+            map.push((Value::Text(name.to_string()), property));
+        }
+        parse(
+            private_schema(platform_value!({ "properties": properties })),
+            true,
+        )
+        .expect("the type registers under full validation")
+    };
+    let old = registrable(platform_value!({ "p": boolean(0), "q": boolean(1) }), None);
+    let update = |new: &DocumentType| {
+        old.as_ref()
+            .validate_update(new.as_ref(), 2, platform_version)
+            .expect("validate_update should not error")
+    };
+
+    for (case, new) in [
+        (
+            "a member put before inner's existing ones",
+            registrable(
+                platform_value!({ "o": boolean(2), "p": boolean(0), "q": boolean(1) }),
+                None,
+            ),
+        ),
+        (
+            "inner's members reordered",
+            registrable(platform_value!({ "q": boolean(1), "p": boolean(0) }), None),
+        ),
+        (
+            "a member removed from inner",
+            registrable(platform_value!({ "p": boolean(0) }), None),
+        ),
+    ] {
+        let result = update(&new);
+        assert!(
+            result.errors.iter().any(|error| matches!(
+                error,
+                ConsensusError::StateError(StateError::DocumentTypeUpdateError(error))
+                    if error.additional_message().contains("can only add properties after")
+                        && error.additional_message().contains("the object meta.inner")
+            )),
+            "{case}: expected the property order refusal naming the object meta.inner, got {:?}",
+            result.errors
+        );
+    }
+
+    for (case, new) in [
+        (
+            "a member added after inner's existing ones",
+            registrable(
+                platform_value!({ "p": boolean(0), "q": boolean(1), "r": boolean(2) }),
+                None,
+            ),
+        ),
+        (
+            "a property appended to the type at the next position",
+            registrable(
+                platform_value!({ "p": boolean(0), "q": boolean(1) }),
+                Some(("note", boolean(3))),
+            ),
+        ),
+    ] {
+        let result = update(&new);
+        assert!(result.is_valid(), "{case}: {:?}", result.errors);
+    }
+}
+
 fn old_and_new_unchanged(
     document_type: &DocumentType,
     platform_version: &PlatformVersion,

packages/rs-dpp/src/data_contract/document_type/methods/validate_update/common/mod.rs

@@ -9,12 +9,15 @@ use crate::consensus::state::data_contract::document_type_update_error::Document
 use crate::data_contract::document_type::accessors::{
     DocumentTypeV0Getters, DocumentTypeV2Getters,
 };
-use crate::data_contract::document_type::property::{ByteArrayPropertySizes, DocumentPropertyType};
+use crate::data_contract::document_type::property::{
+    ByteArrayPropertySizes, DocumentProperty, DocumentPropertyType,
+};
 use crate::data_contract::document_type::schema::validate_schema_compatibility;
 use crate::data_contract::document_type::{DocumentTypeRef, PrivateDocumentConfig};
 use crate::data_contract::errors::DataContractError;
 use crate::validation::SimpleConsensusValidationResult;
 use crate::ProtocolError;
+use indexmap::IndexMap;
 use platform_version::version::PlatformVersion;
 
 /// Per-update exceptions selected by the versioned validator. Defaults retain
@@ -419,6 +422,35 @@ impl DocumentTypeRef<'_> {
             );
         }
 
+        // A private type's entries are immutable and do not say which version of the contract
+        // wrote them, so every one is read with the type as it is now. The document property
+        // encoding writes the type's properties, and each object's members, in the order the
+        // type parses them (inside an object, the schema's map order), so an update may only add
+        // properties after the existing ones, at the end of the type and of every object: an
+        // entry written before then reads the added ones as absent. Schema compatibility already
+        // freezes each existing property's position and type, but not that order. In place: only
+        // protocol version 15 parses a private type, so `private_document()` is `None` on both
+        // sides at every earlier version and the check is skipped there.
+        if self.private_document().is_some() {
+            if let Some(member_list) =
+                first_reordered_member_list(self.properties(), new_document_type.properties(), None)
+            {
+                return SimpleConsensusValidationResult::new_with_error(
+                    DocumentTypeUpdateError::new(
+                        self.data_contract_id(),
+                        self.name(),
+                        format!(
+                            "a private document type can only add properties after its existing \
+                             ones: {member_list} would no longer start with its existing \
+                             properties in their order, so the entries written before would read \
+                             wrong"
+                        ),
+                    )
+                    .into(),
+                );
+            }
+        }
+
         // The entry payload is part of every stored entry's value layout:
         // adding, dropping or renaming a payload property would leave the
         // existing entries undecodable (and their commitments unrecomputable).
@@ -524,6 +556,38 @@ impl DocumentTypeRef<'_> {
     }
 }
 
+/// The first member list of `new` that does not start with `old`'s members in their order: the
+/// type's own properties when `path` is `None`, else those of the object at `path`, searched at
+/// every depth. `None` when every list of `old` is a prefix of the matching list of `new`.
+fn first_reordered_member_list(
+    old: &IndexMap<String, DocumentProperty>,
+    new: &IndexMap<String, DocumentProperty>,
+    path: Option<&str>,
+) -> Option<String> {
+    let keeps_order =
+        new.len() >= old.len() && old.keys().zip(new.keys()).all(|(old, new)| old == new);
+    if !keeps_order {
+        return Some(path.map_or_else(
+            || "the type".to_string(),
+            |path| format!("the object {path}"),
+        ));
+    }
+    old.iter().find_map(|(name, old_property)| {
+        let (
+            DocumentPropertyType::Object(old_members),
+            Some(DocumentPropertyType::Object(new_members)),
+        ) = (
+            &old_property.property_type,
+            new.get(name).map(|property| &property.property_type),
+        )
+        else {
+            return None;
+        };
+        let member_path = path.map_or_else(|| name.clone(), |path| format!("{path}.{name}"));
+        first_reordered_member_list(old_members, new_members, Some(&member_path))
+    })
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

packages/rs-platform-version/src/version/v15.rs

@@ -281,7 +281,9 @@ pub const PROTOCOL_VERSION_15: ProtocolVersion = 15;
 ///     system field; no elected moderation), and registration holds `entrySize` to
 ///     `max_private_document_entry_size`, 4096 bytes in `SYSTEM_LIMITS_V5`. The compiled
 ///     stored form of note 8 records the declaration. A contract update cannot add, remove or
-///     change it (`DocumentTypeUpdateError`). No reference may name a private type: a
+///     change it (`DocumentTypeUpdateError`), and may only add properties after a private type's
+///     existing ones, at the end of the type and of each object, since its immutable entries are
+///     read with the type as it is now (`DocumentTypeUpdateError` too). No reference may name a private type: a
 ///     `findBy`, an `inList` list, a `countOf` or a `sumOf` naming one of the same contract is
 ///     refused by the contract parse (`InvalidContractStructure`), and every other reference,
 ///     to a type of the same contract or another, by contract registration