Skip to content

feat(dpp)!: add identifier and bytes property type shorthands (PV14) - #5355

Merged
QuantumExplorer merged 5 commits into
v5.0-devfrom
claude/identifier-bytes-type-shorthands-pv14
Oct 9, 2026
Merged

QuantumExplorer merged 5 commits into
v5.0-devfrom
claude/identifier-bytes-type-shorthands-pv14

Conversation

@QuantumExplorer

@QuantumExplorer QuantumExplorer commented Oct 9, 2026 •

Copy link
Copy Markdown
Member

Basic explanation

Writing an identifier or a fixed-size byte array in a contract schema takes five keywords today. From protocol version 14 there are two short spellings:

  • "type": "identifier" means exactly "type": "array", "byteArray": true, "minItems": 32, "maxItems": 32, "contentMediaType": "application/x.dash.dpp.identifier".
  • "type": "bytes", "size": 32 means exactly "type": "array", "byteArray": true, "minItems": 32, "maxItems": 32.

The contract is stored exactly as it was sent. Whenever the platform reads a schema (to check it, parse it, validate documents against it, or compare it on an update), it first rewrites the short spelling into the long one, so both spellings behave identically and a document is stored in the same bytes under either.

Before and after

Before:

"recipientId": {
  "type": "array",
  "byteArray": true,
  "minItems": 32,
  "maxItems": 32,
  "contentMediaType": "application/x.dash.dpp.identifier",
  "position": 1
},
"txHash": {
  "type": "array",
  "byteArray": true,
  "minItems": 32,
  "maxItems": 32,
  "position": 2
}

After:

"recipientId": { "type": "identifier", "position": 1 },
"txHash": { "type": "bytes", "size": 32, "position": 2 }

Both work wherever the long form does: top-level properties, object members at any depth, the items of a typed array, and schemaDefs definitions reached by $ref:

"schemaDefs": { "owner": { "type": "identifier" } },
"documentSchemas": {
  "payment": {
    "type": "object",
    "properties": {
      "sponsorId": { "$ref": "#/$defs/owner", "position": 0 },
      "witnesses": {
        "type": "array",
        "maxItems": 4,
        "items": { "type": "identifier", "refersTo": { "type": "identity" } },
        "position": 1
      },
      "hashes": { "type": "array", "maxItems": 4, "items": { "type": "bytes", "size": 20 }, "position": 2 }
    },
    "additionalProperties": false
  }
}

Refused with InvalidContractStructure (10231):

{ "type": "identifier", "minItems": 32 }          // byteArray, minItems, maxItems, contentMediaType: never beside a shorthand
{ "type": "identifier", "size": 32 }             // an identifier is always 32 bytes
{ "type": "bytes" }                              // bytes needs a size
{ "type": "bytes", "size": 0 }                   // size is 1 to 5120
{ "type": "bytes", "size": 6000 }                // above max_field_value_size, at registration

Before protocol version 14 both names are refused, as they always were (meta-schema v2: JsonSchemaError 10101; parser generations 0 to 2: unsupported property type).

What changed

One versioned expansion step. DocumentType::expand_property_type_shorthands (and expand_schema_defs_property_type_shorthands for the $defs map), versioned on the new DocumentTypeSchemaVersions::expand_property_type_shorthands: OptionalFeatureVersion (Some(0) in CONTRACT_VERSIONS_V6, None in V1 to V5). It returns None when nothing needs rewriting, so a contract without shorthands is not cloned; an unknown generation is UnknownVersionMismatch, a refusal 10231. The walk keeps its own stack and descends only through properties and items (one rule, children_below, shared by the scan and the rewrite), so a type key inside refersTo, enum or any other keyword's value is never taken for a property type. The contract's $defs are expanded once per contract parse, in create_document_types_from_document_schemas, and handed to every document type's parse; a contract without document types reads no definition, so its $defs are left as sent and unchecked, as before PV14.

Only registration refuses. Under full validation the refusals below apply. Without it (a contract read back from state, or one only parsed in check_tx or by a client) nothing is refused: a shorthand the expansion cannot rewrite is left exactly as sent. A contract stored before PV14 can only hold one where no reader looks (the $defs of a contract without document types, an entry a repeated key shadows), because every earlier meta-schema and parser refuses both names wherever they read one; anywhere else the parser still refuses it as an unknown type. This keeps every stored contract readable at PV14, which the upgrade's version-item backfill (add_version_items_to_all_contracts) requires.

size range. 1 to system_limits.max_field_value_size (5120 at PV14): has_data_larger_than(max_field_value_size) refuses any larger field value at document validation, so a larger fixed size could never be filled. The upper bound is read from the table and, like every table bound in the parser (max_typed_array_items and others), checked under full validation only, so a stored contract keeps parsing whatever a later table says. A size that is no integer from 1 to 65535 (the u16 range of ByteArrayPropertySizes) cannot be expanded at all; without full validation it is left as sent. Indexed byte arrays keep their existing 255 cap (10205), applied after expansion.

Meta-schema. The expansion runs before the meta-schema, so meta-schema v3 validates the long form and its identifier rules (refersTo, distinctFrom, encryptedFor, the contentMediaType dependencies, typed array items) apply unchanged to the shorthands. Meta-schema v3 only gains two description annotations saying so; no validation rule changed. The rejected alternative was teaching meta-schema v3 the raw shorthand, which would have meant duplicating every identifier-only dependentSchemas rule for a second spelling.

Fees. The schema validation fee (DocumentTypeSchemaValidationForSize) is computed on the expanded schema, so a shorthand contract pays the same validation fee as its long form. Storage is charged on the bytes sent. Schema depth is identical in both forms (the rewrite only adds scalar keys at the same level). The check_tx depth pre-check also walks the expanded schema, because the check resolves every $ref and a reference to a keyword only the long form writes (for example #/properties/hash/byteArray) must resolve there as it does in the block.

Every reader of a raw document schema

The main risk of this change is a reader that sees the raw shorthand. Each reader found, and what it sees:

Reader Sees
try_from_schema v3 (parse_generation_3): doctype keyword reads, parse_document_type_core (meta-schema validation, depth and size check, jsonschema compile, insert_values / insert_values_nested, parse_typed_array, $ref resolution via resolve_uri and the enter_ref cycle walk, indexes, preallocated indexes), apply_immutable_fields, apply_retracted_when, validate_encrypted_for_declarations, apply_property_constraints (schema_at_path, enum_admits, is_key_id_schema), the summed-value bound checks, transient and refersTo checks Expanded schema and expanded $defs; DocumentTypeV2::schema is set back to the schema as sent before returning
DataContract::validate_document_properties v1: lazy compile of the per-type JSON Schema validator for a contract read from state Expanded (new call, PV14-only generation)
DocumentTypeRef::validate_schema_with_options and DataContract::validate_update_schema_defs (contract update comparison) Expanded old and new schemas and $defs (see In-place section)
validate_property_constraint_aggregates (contract-level, after parse): enum_admits on counted.schema() Expanded $defs, schema as sent; reads only enum of string properties, which the rewrite never touches (commented there)
resolve_derived_index_properties, validate_preallocated_indexes_kept_on_removal, validate_summable_off_count_indexes_lossless Parsed properties only
validate_document_schemas_depth_for_check_tx (drive-abci) Expanded schema and $defs (without refusing anything), so $refs resolve against the same long form the block checks
registration_cost v1/v2 Raw indices only
drive-abci contract create/update basic structure: requiredSince pre-scan, ContractModerationConfig::validate, DocumentActionFees::first_document_type_charging_moderators Raw doctype-level keywords and requiredSince, none type-dependent
document_schemas() / schema_defs() for serialization, proofs, wasm-dpp getDocumentSchema(s), rs-sdk-ffi schema JSON, wasm-dpp2 and FFI propertyConstraints / immutable readers Raw, as sent, by design
find_identifier_and_binary_paths, random_document, estimated_size, Drive serialization and keys, getBinaryProperties (legacy wasm-dpp) Parsed property types only

DocumentTypeV0Getters::schema now documents that it returns the schema as sent, which may hold shorthands, and that property types are read from the parsed properties or the expanded schema.

Errors about a shorthand property name the long form, since that is what was checked (a size change is refused for changing minItems/maxItems, 40212; identifier to bytes at .../contentMediaType, 10246); the book says so.

Not expanded: a JSON Schema contains subschema. It is not a property schema, the parser ignores it, and the meta-schema checks it against plain draft 2020-12, so a shorthand there stays refused at registration.

Follow-up outside this PR: the Swift SDK's DataContractParser (and possibly Kotlin) parses the raw schema JSON from rs-sdk-ffi itself, so the example apps will show a shorthand property's type as written.

wasm-dpp2, wasm-sdk and js-evo-sdk carry no typings for raw property schemas (schemas are passed as objects); the wasm-dpp2 DocumentTypedArrayItem typing doc now mentions that the shorthands parse to the same element kinds.

In-place changes to shipped generations

Edited code Selected by Why consensus cannot change there
DocumentTypeRef::validate_schema_with_options (document_type/methods/validate_update/common) document type validate_update 0 (protocol versions 1 to 13) and 1 (14) The new call is expand_property_type_shorthands, whose slot is None in CONTRACT_VERSIONS_V1 to V5; its dispatcher then returns Ok(None) without reading the schema, and the caller compares the schema it already held, so the diffed JSON is byte-identical to before.
DataContract::validate_update_schema_defs (data_contract/methods/validate_update/common) DataContract::validate_update 0 (1 to 13; generation 1 at 14 delegates to it) Same: expand_schema_defs_property_type_shorthands returns Ok(None) under None, and the $defs are compared as before.
DocumentType::create_document_types_from_document_schemas_v1 protocol versions 2 to 14 Expands the $defs once before the document types are parsed (not for a contract without document types); under None (2 to 13) it returns Ok(None) and the definitions are passed on as given.
DocumentTypeSchemaVersions gains expand_property_type_shorthands every table None in CONTRACT_VERSIONS_V1 to V5.

should_leave_the_schema_as_sent_before_protocol_version_14 and should_judge_updates_at_protocol_version_13_as_before run both helpers at protocol version 13. Parser generation 3, validate_document 1 and meta-schema v3 are selected only by protocol version 14, which is unreleased.

Protocol version 14 change note 91 is added to v14.rs (89 and 90 are taken by open PRs).

Tests

  • expand_property_type_shorthands::tests: both expansions, nested members, typed array items and $defs, None when nothing to rewrite and before PV14, each refusal, the size bound under full validation only.
  • try_from_schema/v3/property_type_shorthand_tests.rs:
    • a shorthand and its long form parse to identical document types (everything but the stored schema equal, with and without full validation), across top-level, nested, typed array items with refersTo, distinctFrom, indexes and $ref;
    • a document serializes byte-identically under both spellings and reads back alike;
    • platform serialization round trip keeps the shorthand byte-identical; JSON round trip writes the same JSON;
    • documents are validated against the long form, by a freshly registered contract and by one read back from state (lazy validator compile);
    • $ref to a shorthand definition;
    • every refusal, on both parse paths;
    • protocol version 13 refuses both (meta-schema and parser);
    • an update between the spellings, in either direction and in $defs, is accepted as no change, while identifier to bytes, a changed size and a changed definition are still refused;
    • protocol version 13 judges updates as before;
    • a repeated type key is read by its last entry, as the parser and the JSON are;
    • a $ref to a shorthand below a definition (#/$defs/memo/properties/author);
    • an unknown expansion generation is a version mismatch; without full validation a shorthand that can't be rewritten is left as sent;
    • contracts registered at protocol version 13 that hold shorthand-shaped values no reader looks at (a token-only contract's $defs, a repeated top-level property name, a shadowed nested properties keyword) read back at protocol version 14 with their schemas and bytes unchanged.
  • Drive add_version_items_to_all_contracts: the PV14 backfill reads such contracts stored at PV13.
  • drive-abci contract_structure_error_tests: a shorthand property with contains: {"$ref": "#/properties/hash/byteArray"}, and its long form, pass both check_tx and the block.
  • drive-abci batch/tests/document/property_type_shorthands.rs: a contract with identifier and bytes registered by a signed contract create, stored as sent, documents created against the stored contract, wrong-length values refused with JsonSchemaError, queries by recipientId and txHash answered with and without a proof (proof verified), then a signed contract update rewriting both in full accepted as no change, with the documents still answering; the response kind is asserted to match the requested proof mode.

Ran: cargo test -p dpp, cargo test -p platform-version, cargo test -p drive-abci <filter>, cargo test -p wasm-dpp2, clippy with --all-features --all-targets -D warnings on dpp, platform-version, drive-abci and wasm-dpp2, cargo fmt --all.

🤖 Generated with Claude Code

PR Hygiene · be66154

  • Bots — coderabbitai not yet · thepastaclaw ✓ — /skip-bots proceeds without the ones not yet reported
  • Self-review — post /self-reviewed
  • Build running
  • Approvals — you own every area touched; none needed

When every merge requirement is met, the PR Hygiene check passes. Reviewer limits do not block merging; other required GitHub checks and protections still apply.

Summary by CodeRabbit

  • New Features
    • Protocol version 14 supports identifier and fixed-size bytes shorthand types in document schemas, schema definitions, and typed arrays.
    • Shorthand schemas are validated and interpreted like their equivalent long forms while retaining their submitted spelling in stored contracts.
    • Contract updates can switch between shorthand and equivalent long-form schemas without breaking schema compatibility.
  • Documentation
    • Updated references and examples to explain shorthand usage, limits, and validation rules.

From protocol version 14 a property schema may write `"type": "identifier"`
for the 32-byte identifier byte array and `"type": "bytes", "size": n` for a
byte array of exactly n bytes, on a property at any level, typed array
`items` and `schemaDefs` definitions.

One versioned step, `DocumentType::expand_property_type_shorthands` (None
before PV14), rewrites them into the long form before parser generation 3,
the meta-schema, the document validator and the contract update comparison
read the schema. The contract is stored exactly as sent, so the stored
contract and the transition stay byte-identical.

Refused (10231): byteArray, minItems, maxItems or contentMediaType beside a
shorthand, size on identifier, bytes without a size from 1 to 65535, and,
under full validation, a size above max_field_value_size.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: dashpay/platform/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: f74ab049-4c5c-454f-a09c-707caac64a80
📥 Commits

Reviewing files that changed from the base of the PR and between 4ca59b1 and e4e2fd1.

📒 Files selected for processing (29)
  • book/src/contract-keywords.md
  • book/src/contract-keywords/property-schemas.md
  • book/src/contract-keywords/typed-arrays.md
  • packages/rs-dpp/schema/meta_schemas/document/v3/document-meta.json
  • packages/rs-dpp/src/data_contract/document_type/accessors/v0/mod.rs
  • packages/rs-dpp/src/data_contract/document_type/class_methods/create_document_types_from_document_schemas/v1/mod.rs
  • packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/mod.rs
  • packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v3/derived_index_property_tests.rs
  • packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v3/mod.rs
  • packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v3/property_type_shorthand_tests.rs
  • packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v3/refusal_test_support.rs
  • packages/rs-dpp/src/data_contract/document_type/methods/validate_update/common/mod.rs
  • packages/rs-dpp/src/data_contract/document_type/schema/expand_property_type_shorthands/mod.rs
  • packages/rs-dpp/src/data_contract/document_type/schema/expand_property_type_shorthands/v0/mod.rs
  • packages/rs-dpp/src/data_contract/document_type/schema/mod.rs
  • packages/rs-dpp/src/data_contract/methods/validate_document/v1/mod.rs
  • packages/rs-dpp/src/data_contract/methods/validate_update/common/mod.rs
  • packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/mod.rs
  • packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/property_type_shorthands.rs
  • packages/rs-drive/src/drive/contract/migration/add_version_items_to_all_contracts.rs
  • packages/rs-platform-version/src/version/dpp_versions/dpp_contract_versions/mod.rs
  • packages/rs-platform-version/src/version/dpp_versions/dpp_contract_versions/v1.rs
  • packages/rs-platform-version/src/version/dpp_versions/dpp_contract_versions/v2.rs
  • packages/rs-platform-version/src/version/dpp_versions/dpp_contract_versions/v3.rs
  • packages/rs-platform-version/src/version/dpp_versions/dpp_contract_versions/v4.rs
  • packages/rs-platform-version/src/version/dpp_versions/dpp_contract_versions/v5.rs
  • packages/rs-platform-version/src/version/dpp_versions/dpp_contract_versions/v6.rs
  • packages/rs-platform-version/src/version/v14.rs
  • packages/wasm-dpp2/src/data_contract/document_type_typed_arrays.rs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Protocol version 14 adds identifier and fixed-size bytes property-type shorthands. The code expands shorthands for parsing, validation, and update comparison while preserving the schema spelling submitted in the contract.

Changes

Property-Type Shorthand Support

Layer / File(s) Summary
Versioned shorthand expansion
packages/rs-platform-version/src/version/dpp_versions/dpp_contract_versions/*, packages/rs-platform-version/src/version/v14.rs, packages/rs-dpp/src/data_contract/document_type/schema/expand_property_type_shorthands/*
A platform-version feature setting enables shorthand expansion. Generation 0 expands identifier and sized bytes in supported schema locations. Full validation checks shorthand constraints, including conflicting keywords and byte-size limits.
Parsing and document validation
packages/rs-dpp/src/data_contract/document_type/class_methods/*, packages/rs-dpp/src/data_contract/methods/validate_document/v1/mod.rs, packages/rs-dpp/schema/meta_schemas/document/v3/document-meta.json, packages/wasm-dpp2/src/data_contract/document_type_typed_arrays.rs, book/src/contract-keywords*
Document-type parsing and validator compilation use expanded schemas and definitions. Parsing retains the submitted schema. Tests cover parsing, serialization, validation, references, protocol-version behavior, and shorthand documentation.
Update compatibility and integration checks
packages/rs-dpp/src/data_contract/document_type/methods/validate_update/common/mod.rs, packages/rs-dpp/src/data_contract/methods/validate_update/common/mod.rs, packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/*, packages/rs-drive/src/drive/contract/migration/add_version_items_to_all_contracts.rs
Update checks compare expanded schemas and definitions when available. Tests cover shorthand-to-long-form updates, state transitions, and version-item backfill for contracts containing shorthand-shaped values.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant ContractSchema
  participant DocumentTypeParser
  participant ShorthandExpansion
  participant DocumentValidator
  ContractSchema->>DocumentTypeParser: Submit document schema and definitions
  DocumentTypeParser->>ShorthandExpansion: Expand property-type shorthands
  ShorthandExpansion-->>DocumentTypeParser: Return expanded schema and definitions
  DocumentTypeParser-->>ContractSchema: Retain submitted schema
  DocumentValidator->>ShorthandExpansion: Expand enriched schema before validator compilation
Loading

Suggested reviewers: thepastaclaw

Merge Risk: ⚪ Minimal · up to e4e2f

Protocol version 14 adds identifier and fixed-size bytes shorthands. Contracts are expanded for validation and update comparison but stored as submitted. Previously stored contracts with unusual shorthand-shaped values reportedly remain readable during the upgrade. No outstanding merge-blocking issue was identified.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage Passed Docstring coverage is 81.25% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 80 functions across 25 files. (4 skipped: 4…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely identifies the main change: adding identifier and bytes property type shorthands for protocol version 14.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

📖 Book Preview built successfully.

Download the preview from the workflow artifacts.
To view locally: download the artifact, unzip, and open index.html.

Updated at 2026-10-09T07:20:26.094Z

@github-actions github-actions Bot added this to the v5.0.0 milestone Oct 9, 2026
@github-actions

github-actions Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Waiting for bot review — coderabbitai not yet · thepastaclaw not yet. Wait for the missing reviews, or a writer can post /skip-bots to proceed without them; blocking findings still need addressing.
Full checklist in the description.

@github-actions github-actions Bot added the waiting-bots Waiting for the review bots to report on this head label Oct 9, 2026
@thepastaclaw

thepastaclaw commented Oct 9, 2026 •

Copy link
Copy Markdown
Collaborator

✅ Final review complete — no blockers (commit be66154) · triage: critical

…-bytes-type-shorthands-pv14

# Conflicts:
#	packages/rs-platform-version/src/version/v14.rs
@github-actions

github-actions Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Waiting for bot review — coderabbitai not yet · thepastaclaw not yet. Wait for the missing reviews, or a writer can post /skip-bots to proceed without them; blocking findings still need addressing.
Full checklist in the description.

- Expand the contract's $defs once per contract parse in
  create_document_types_from_document_schemas (inert before PV14) instead
  of once per document type.
- The expansion dispatchers return ProtocolError: an unknown generation is
  UnknownVersionMismatch, a refusal the usual 10231.
- One rule (children_below) names where property schemas sit, shared by the
  scan and the rewrite; comments no longer claim the walk avoids recursion.
- The size refusal states the bound of the path that refused it (65535
  without full validation).
- Document that DocumentType::schema() is the schema as sent and may hold
  shorthands, and in the book how errors name the long form.
- Tests: a signed contract update between spellings end to end, a repeated
  type key, a $ref below a definition, error kinds asserted; the contract
  value builder is shared.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Waiting for bot review — coderabbitai not yet · thepastaclaw requested changes — dismiss the review or push a fix, 2 threads unresolved — resolve them. Wait for the missing reviews, or a writer can post /skip-bots to proceed without them; blocking findings still need addressing.
Full checklist in the description.

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Final validation — Phase 1 + Phase 2

The shorthand expansion is correctly version-gated and preserves submitted schemas, but two historical-state compatibility regressions can make previously accepted contracts unreadable at PV14 and prevent the upgrade migration from completing. The new integration test also needs an assertion that proof requests actually return proofs. This was a static verification at the exact head; the supplied CI snapshot reports successful Rust workspace and WASM tests, while functional, browser, platform-suite, and local-network E2E checks remained pending.

🔴 2 blocking | 🟡 1 suggestion(s)

Review provenance

Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 7: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 8: gpt-6-astra (agent: phase2-reviewer, role: architecture-layering); reviewer 9: gemini-3.8-flash-high (agent: phase1-reviewer, role: general); reviewer 10: gpt-6-astra (agent: phase2-reviewer, role: ffi-engineer); reviewer 11: gpt-6-astra (agent: phase2-reviewer, role: platform-versioning); reviewer 12: gemini-3.8-flash-high (agent: phase1-reviewer, role: rust-quality); reviewer 13: gpt-6-astra (agent: phase2-reviewer, role: rust-quality); reviewer 14: gpt-6-astra (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

  • Triage: critical by gpt-6.1-sol (effort low) — The intricate schema expansion and its integration into DocumentType::try_from_schema_v3, validate_document/v1, and contract update validation change consensus-visible contract acceptance and document validation rules at PV14.
  • Phase 1 reviewers: gemini-3.8-flash-high — general (completed, effort high); agent phase1-reviewer, gemini-3.8-flash-high — rust-quality (completed, effort high); agent phase1-reviewer
  • Phase 1 model: gemini-3.8-flash-high — antigravity quota: weekly 58% left, 5h 27% left
  • Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
  • Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
  • Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — general (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — security-auditor (completed, effort xhigh); agent phase2-reviewer
  • Model comparison: every Phase-2 reviewer also ran on gpt-6-astra; the verifier weighed both sets without knowing which model wrote which
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-dpp/src/data_contract/document_type/class_methods/create_document_types_from_document_schemas/v1/mod.rs`:
- [BLOCKING] packages/rs-dpp/src/data_contract/document_type/class_methods/create_document_types_from_document_schemas/v1/mod.rs:52-60: Preserve readability of legacy token-only contracts during the PV14 upgrade
  The new definitions expansion runs even when `document_schemas` is empty. At PV13, an otherwise valid token-only contract can contain `schemaDefs: {"unused": {"type": "bytes"}}`: `has_tokens` permits the empty document map, and no document parser or document meta-schema checks the unused definition. At PV14, expansion rejects its missing `size` even with `full_validation = false`. Drive's `fetch_contract_v0` reconstructs stored contracts through `DataContract::versioned_deserialize_trusted(..., false, platform_version)`, so the same persisted bytes become unreadable. This also affects activation: `transition_to_version_14` calls `add_version_items_to_all_contracts`, which fetches every contract using the new platform version and propagates reconstruction errors. One such historical contract can therefore prevent the upgrade block from completing. Keep unused definitions inert when reconstructing token-only contracts, preserve that compatibility in the definitions update-comparison path, and add a regression covering PV13 registration and serialization followed by PV14 trusted decoding and migration.

In `packages/rs-dpp/src/data_contract/document_type/schema/expand_property_type_shorthands/v0/mod.rs`:
- [BLOCKING] packages/rs-dpp/src/data_contract/document_type/schema/expand_property_type_shorthands/v0/mod.rs:264-267: Do not validate shadowed property entries during expansion
  `Value::Map` stores a vector of entries, and this walk processes every member rather than only the effective members used by validation and parsing. A PV13 top-level `properties` map can contain two entries named `value`, both at position 0: first `{"type":"bytes","position":0}`, then `{"type":"string","maxLength":10,"position":0}`. Validating JSON retains the latter entry, and `map_ref_into_indexed_string_map` also retains it after the stable position sort, so registration parses a valid string property. Platform serialization nevertheless preserves both raw entries. At PV14, the new expansion processes the hidden `bytes` entry and rejects its missing `size`, including during trusted decoding without full validation. The nested traversal has the same problem with repeated `properties` keywords whose earlier subtree is shadowed. This makes previously accepted state unreadable and can also fail the fetch-all migration during PV14 activation. Make both the scan and rewrite respect the effective entries of the existing readers, without changing the stored schema. Add PV13-to-PV14 persisted-contract regressions for repeated property names and shadowed nested schema keywords; the existing repeated-`type` test covers neither case.

In `packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/property_type_shorthands.rs`:
- [SUGGESTION] packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/property_type_shorthands.rs:311: Assert that the response variant matches the requested proof mode
  The helper accepts either response variant regardless of `prove`. If a regression returns ordinary documents for `prove: true`, the integration test still passes through the document-decoding branch without executing `verify_proof`, silently losing the proof coverage this test is intended to provide. Assert the response kind before dispatching; this also detects an unexpected proof response for the non-proof request.
Out-of-scope follow-up suggestions (1)

These are valid observations, but they are outside this PR's scope and should be handled in separate issues or author/maintainer-requested PRs rather than blocking this review.

  • Add mobile schema-discovery support for shorthand properties and array elements — The explicitly deferred mobile compatibility issue is concrete: Swift's DataContractParser persists the literal shorthand type without deriving byte-array or identifier metadata, and Swift's DocumentTypedArray and Kotlin's TypedArrays.kt recognize these element kinds only through the long-form array schema. Consequently, shorthand properties lose dedicated editor handling, and typed arrays with shorthand items are omitted from typed-array discovery. Raw schema exports must remain unchanged for round trips, so this belongs in a separate mobile metadata/editor follow-up rather than a change to the raw FFI schema representation.
    • Follow-up: Track a separate maintainer-requested change to expose parsed property metadata through thin mobile bindings and consume it in Swift and Kotlin schema-driven editors, including typed-array items and referenced definitions.

Only full validation refuses a property type shorthand: without it (a
contract read back from state, or one only parsed) a shorthand that can not
be rewritten is left as sent. A contract stored before protocol version 14
can hold one only where no reader looks, the definitions of a contract
without document types or an entry a repeated key shadows, and keeps
loading. The definitions of a contract without document types are no longer
expanded at all, as before 14.

Tests: contracts registered at 13 with such values read back at 14, and
the version-item backfill reads them; the end-to-end query test checks the
response kind matches the requested proof mode.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Waiting for bot review — coderabbitai ✓ · thepastaclaw not yet. Wait for the missing reviews, or a writer can post /skip-bots to proceed without them; blocking findings still need addressing.
Full checklist in the description.

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review — Final validation — Phase 1 + Phase 2

At head e4e2fd1, all three prior findings are fixed. One blocking integration issue remains: the mempool depth precheck resolves references against raw schemas, rejecting some shorthand contracts that full block validation and their equivalent long forms accept. Verification was static only; the supplied CI snapshot still showed principal build and Rust test checks pending or in progress.

🔴 1 blocking

Review provenance

Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 6: gemini-3.8-flash-high (agent: phase1-reviewer, role: general); reviewer 7: gemini-3.8-flash-high (agent: phase1-reviewer, role: rust-quality); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

  • Triage: critical by gpt-6.1-sol (effort low) — The intricate schema expansion and its integration into try_from_schema/v3, validate_document/v1, and validate_update/common change consensus-critical contract acceptance, document validation, and update compatibility rules.
  • Phase 1 reviewers: gemini-3.8-flash-high — general (completed, effort high); agent phase1-reviewer, gemini-3.8-flash-high — rust-quality (completed, effort high); agent phase1-reviewer
  • Phase 1 model: gemini-3.8-flash-high — antigravity quota: weekly 52% left, 5h 84% left
  • Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
  • Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
  • Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v3/mod.rs`:
- [BLOCKING] packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v3/mod.rs:411-415: Expand shorthands before the check_tx schema-depth precheck
  Full validation checks the expanded schema, but `validate_document_schemas_depth_for_check_tx` enriches and checks the raw schema before this parser runs. Matching structural depth does not guarantee matching results: `validate_max_depth_v1` also resolves every `$ref`. For example, `{"type":"object","properties":{"hash":{"type":"bytes","size":32,"position":0,"contains":{"$ref":"#/properties/hash/byteArray"}}},"additionalProperties":false}` expands to a valid fixed-size byte-array schema with a `contains` reference to the boolean schema `true`. The meta-schema admits this `contains` subschema, the property parser ignores it, and the expanded depth check accepts its scalar reference target. In the raw shorthand, however, `byteArray` does not exist, so the mempool precheck returns `InvalidJsonSchemaRefError`. Both contract-create and contract-update transformation invoke this precheck before parsing, making these valid transitions fail CheckTx and RecheckTx while their equivalent long forms are admitted. Apply the versioned expansion to document schemas and definitions before the mempool depth check, and add a regression comparing mempool admission with full validation for a reference to an expansion-generated keyword.

@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Your move: thepastaclaw requested changes on this head; dismiss the review or push a fix; thepastaclaw left review threads unresolved; resolve them.
Full checklist in the description.

@github-actions github-actions Bot added waiting-self-review Waiting for the author to post /self-reviewed and removed waiting-bots Waiting for the review bots to report on this head labels Oct 9, 2026
The block checks schema depth on the schema with its property type
shorthands rewritten into the long form, and the check resolves every
`$ref`, so a reference to a keyword only the long form writes (such as
`#/properties/hash/byteArray`) resolved in the block but not in the
check_tx pre-check, which refused a contract the block accepts. The
pre-check now rewrites the document schemas and `$defs` the same way,
without refusing anything. Inert before protocol version 14: both the
pre-check and the rewrite are off there.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Waiting for bot review — coderabbitai not yet · thepastaclaw ✓. Wait for the missing reviews, or a writer can post /skip-bots to proceed without them; blocking findings still need addressing.
Full checklist in the description.

@github-actions github-actions Bot added waiting-bots Waiting for the review bots to report on this head and removed waiting-self-review Waiting for the author to post /self-reviewed labels Oct 9, 2026

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review — Final validation — Phase 1 + Phase 2

Independently reviewed the complete diff at be66154 and confirmed all four prior findings are fixed, with no remaining actionable in-scope findings. Shorthand expansion is gated to PV14, reaches the relevant validation and comparison paths, and preserves submitted schemas and historical contract readability. Validation was static only: git diff --check passed; the supplied CI snapshot still showed Rust workspace tests, JS and Docker builds, and Kotlin build/tests pending.

🔴 0 blocking | 🟡 0 suggestion(s) | 💬 0 nitpick(s)

Review provenance

Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 6: gemini-3.8-flash-high (agent: phase1-reviewer, role: general); reviewer 7: gemini-3.8-flash-high (agent: phase1-reviewer, role: rust-quality); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 10: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 11: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 12: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

  • Triage: critical by gpt-6.1-sol (effort low) — The intricate schema expansion in packages/rs-dpp/src/data_contract/document_type/schema/expand_property_type_shorthands/v0/mod.rs and its integration into contract parsing, document validation, and update compatibility change consensus acceptance rules at PV14.
  • Phase 1 reviewers: gemini-3.8-flash-high — general (completed, effort high); agent phase1-reviewer, gemini-3.8-flash-high — rust-quality (completed, effort high); agent phase1-reviewer
  • Phase 1 model: gemini-3.8-flash-high — antigravity quota: weekly 51% left, 5h 78% left
  • Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
  • Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
  • Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
  • Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify the current code and confirm that no unresolved issues remain.

No unresolved findings remain from the prior review on this head.

@QuantumExplorer

Copy link
Copy Markdown
Member Author

Reviewed

@QuantumExplorer
QuantumExplorer merged commit ac12d70 into v5.0-dev Oct 9, 2026
49 of 51 checks passed
@QuantumExplorer
QuantumExplorer deleted the claude/identifier-bytes-type-shorthands-pv14 branch October 9, 2026 12:19
QuantumExplorer added a commit that referenced this pull request Oct 9, 2026
Now that `identifier` and `bytes` + `size` are in v5.0-dev (#5355), the
DPNS v3 schema uses them: `preorderSalt` and `saltedDomainHash` are
`"type": "bytes", "size": 32`, `records.identity` is `"type":
"identifier"`. The parsed types are unchanged; the stored contract keeps
the schema as written, so the check_tx contract-update fee pin moves
(27003089990 to 27003077990).

Protocol 13 cannot decode the shorthands, so the SDK test of a response
parsed under an older version now uses DPNS v2 as the contract read
under 13.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

waiting-bots Waiting for the review bots to report on this head

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants