Skip to content

feat(dpp)!: check document schemas natively at registration from protocol version 15 - #5538

Open
QuantumExplorer wants to merge 37 commits into
claude/native-document-validatorfrom
claude/native-registration-checks
Open

QuantumExplorer wants to merge 37 commits into
claude/native-document-validatorfrom
claude/native-registration-checks

Conversation

@QuantumExplorer

@QuantumExplorer QuantumExplorer commented Oct 22, 2026 •

Copy link
Copy Markdown
Member

Basic explanation

What this does: When someone registers a data contract, every document type's schema is checked against a "meta-schema", a JSON Schema describing what a valid document type looks like. Today a third-party JSON Schema library (our fork of the jsonschema crate) runs that check. From protocol version 15, Platform runs the same rules in its own Rust code instead. Every rule of the meta-schema is ported, with the same verdict and the same error code, but for two deliberate rules listed under Breaking Changes. It also requires every new document type to have a typed validation schema, which its documents are checked against natively (#5533).

Value: Contract registration no longer depends on the jsonschema crate at protocol version 15. Together with #5533, which checks documents natively, this moves the crate out of new consensus paths, a step toward dropping the fork. Index property names are also held to the 255 bytes a GroveDB key can hold.

Risks: Low to medium. The change is consensus-relevant but only at protocol version 15, which is unreleased. Protocol versions up to 14 run exactly the code they ran. The risk is a ported rule that differs from the meta-schema. That is covered by comparing both on every document type ever registered on mainnet and testnet and on about 5,700 generated variants (30,000 in a one-off run), all with equal verdicts. Two kinds of refusal change their error class at 15, listed under Breaking Changes.

Issue being fixed or feature implemented

Step 2 of the 5.1 contract definition work (Refs #5271), stacked on #5533 (native document validation). After #5533, documents of types registered at protocol version 15 are checked natively, but registration still ran the document meta-schema through the jsonschema fork and compiled every schema with it. This PR removes that last use at registration.

What was done?

The schema check, versioned in the tables (check_document_type_schema, a new DocumentTypeSchemaVersions field):

  • 0, in every shipped table: full validation checks the enriched schema against the document meta-schema through the jsonschema crate and compiles it, as before.
  • 1, in CONTRACT_VERSIONS_V7 only (protocol version 15): native_schema::check_document_type_schema checks it natively, and once the type is parsed, require_validation_schema requires the type's native validator to build (DocumentSchemaValidator::try_from_root, the builder validation_schema selects), so a type whose documents protocol version 15 would refuse, such as one whose references cycle without entering the value, is refused at registration.
  • The shared validate_document_type_schema and require_validation_schema dispatch on it with &PlatformVersion. The parser stays generation 3 (try_from_schema 3) at protocol version 15, and loading a stored contract runs no schema check.

Native checks (try_from_schema/native_schema/), one file per area of the meta-schema (schema/meta_schemas/document/v3/document-meta.json):

  • mod.rs: the entry points, and the primitives read as the crate read them. A repeated key reads as its last entry, and a float without a fractional part counts as an integer. Bounds compare exactly, a const number matches within f64::EPSILON, and list uniqueness uses the native document validator's comparison.
  • document_type.rs: the root's closed key set, the document-type keywords and their conditions.
  • properties.rs: property schemas (at every level and in $defs) and typed array element schemas. This covers the closed key sets, identifiers, byte arrays, pattern/format lengths, encryptedFor, generatedFrom, maxBytes and the rest.
  • indexes.rs: every index keyword and the conditions between them.
  • references.rs: refersTo, ownerRefersTo, creatorRefersTo, with the findBy, where and constraints grammars.
  • constraints.rs: the rule language of propertyConstraints and deleteConstraints.

A refusal is the JsonSchemaError (code 10101) the meta-schema check returned, with the same keyword, location (instance_path), parameters and property name. Two fields differ: the message text, and schema_path, which is empty (as is the schemaPath nested in a property name's error), since the native checks have no location in the meta-schema. All of it goes to info, which is not part of the results hash.

The port reproduces the meta-schema's JSON Schema semantics where they matter. For example, its if passes when the key it tests is absent, so a property schema with neither type nor $ref must declare properties and additionalProperties. A failed anyOf/oneOf is reported at the value. A property's contains is not checked.

One new rule: an index property name is at most 255 bytes, the longest key GroveDB stores. The meta-schema admitted 256 characters.

Validation schema builder: a plain-name $id may also contain $, as refersTo data carries ("$id": "$ownerId"). The builder only runs at protocol version 15.

The meta-schema's numbers and the cap are constants of the native checks, beside the rules that read them, as the meta-schema JSON held them before. They are not SystemLimits: they belong to check_document_type_schema 1 alone.

Before and after

A document type with a null flag:

{ "type": "object", "documentsMutable": null, "properties": { "name": { "type": "string", "maxLength": 63, "position": 0 } }, "additionalProperties": false }
  • Protocol version 14: refused by the meta-schema check in the jsonschema crate, JsonSchemaError (10101) at /documentsMutable.
  • Protocol version 15: refused by the native check, JsonSchemaError (10101) at /documentsMutable.

An index on a property named with 128 two-byte characters (256 bytes):

  • Protocol version 14: admitted by the meta-schema.
  • Protocol version 15: refused, JsonSchemaError (10101) at /indices/0/properties/0: "the index property name … is longer than 255 bytes".

A document type the validation schema does not represent, for example maxBytes beside a property's $ref, which document validation never checked:

  • Protocol version 14: registered.
  • Protocol version 15: refused, InvalidContractStructure (10231): "maxBytes is read beside a property's $ref …".

How Has This Been Tested?

  • native_schema/tests.rs compares the native checks with the meta-schema, and registration at the latest protocol version with check_document_type_schema 1 against the same with 0:
    • every document type of the contract corpus (1,503, tests/fixtures/contract_corpus): same verdict, same parsed document type;
    • nine hand-written document types covering what the corpus uses little: references, encryption, fees, token costs, moderation with retractedWhen, constraints, windowed and ranked indexes, index-only types;
    • 2,700 variants generated from those and 3,006 from the corpus types, each with one to three random changes. Every verdict is equal. A refusal keeps its error code, except the two moves listed under Breaking Changes.
    • targeted tests for each rule area, where an error is reported, the repeated-key reading and the 255-byte cap;
    • the error shape: keyword, location, parameters and property name equal the meta-schema check's on 37 single-rule violations.
  • One-off runs, not committed:
    • the generated comparison with other seeds and 30,000 variants: equal;
    • every contract fixture of rs-drive, rs-drive-abci and rs-dpp and every system contract (207 contracts) registered at protocol versions 14 and 15: same verdict and code.
  • validation_schema tests: types the builder refuses now register at protocol version 14, and protocol version 15 refuses them.
  • cargo test -p dpp --lib data_contract (3,118), state_transition (1,270), --test contract_corpus; cargo clippy -p dpp -p platform-version --all-targets --features dpp/all_features; cargo check -p dpp with validation-schema, validation, state-transitions and json-schema-validation alone.

Breaking Changes

None for released protocol versions. At protocol version 15 (unreleased):

  • A document type without a typed validation schema is refused at registration (InvalidContractStructure).
  • A schema the meta-schema admitted but the jsonschema crate failed to compile, refused before with JsonSchemaError, is now refused by the validation schema builder (InvalidContractStructure).
  • An index property name over 255 bytes is refused.
  • In a list that must be unique, such as an enum's members, 0.0 and -0.0 are one value however long the list, as in feat(dpp)!: validate documents natively at protocol version 15 #5533.

In-place changes to shipped generations

  • DocumentTypeSchemaVersions gains check_document_type_schema, 0 in every shipped table (CONTRACT_VERSIONS_V1 to V6).
  • The shared validate_document_type_schema dispatches on it, and 0 runs validate_against_meta_schema_and_compile exactly as before.
  • Generation 3's parse calls require_validation_schema last under full validation, which returns at once under 0.
  • validator_regex now builds through validator_regex_within(pattern, None), the same compile with the crate's default size limit. The builder and the native document validator run only at protocol version 15.

Checklist:

  • I have performed a self-review of my own code
  • My Rust, FFI, WASM, Swift or Kotlin changes follow the coding conventions
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have added "!" to the title and described breaking changes in the corresponding section if my code contains any
  • I have made corresponding changes to the documentation if needed
  • If I added or changed GroveDB structure, I described it in the area's structure.rs, regenerated grovedb-structure.json, and checked the structure viewer link posted on this pull request

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

🤖 Generated with Claude Code

QuantumExplorer and others added 4 commits October 22, 2026 14:59
…otocol version 15

Protocol version 15 selects parser generation 4 (`try_from_schema` 4 in
`CONTRACT_VERSIONS_V7`): generation 3's parse, with full validation
checking the enriched schema natively instead of against the document
meta-schema through the `jsonschema` crate, which also stops compiling
it. Each new document type must then have a typed validation schema,
which its documents are checked against natively.

This commit ports the root shape of a document schema and the rules of
property schemas, at every level and in `$defs`, and of typed array
element schemas. A schema breaking one is refused with the
`JsonSchemaError` the meta-schema check returned, so the error code
stays. The native checks read the JSON the meta-schema check read: a
repeated key's last entry, and the crate's readings of integers, bounds,
`const` numbers and unique lists.

The validation schema builder also admits `$` in a plain-name `$id`, as
`refersTo` data carries (`"$id": "$ownerId"`).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…yte names

Generation 4 checks each index as the document meta-schema did: its
closed key set, `name`, `properties` (each one property, `asc` only),
the flags, `contested`, the count, sum and average keywords, `ranked*`,
`timeRange`, `integerRange`, `terminal`, `skipIfAbsent` and
`summableOffCountIndex`, with the meta-schema's conditions between them.

One rule differs: an index property name is at most 255 bytes, the
longest key GroveDB stores, where the meta-schema admitted 256
characters.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nt grammars

Generation 4 checks the document-type keywords as the document
meta-schema did (the flags, `canBeDeleted`, the security and restriction
modes, `documentsSummable` and `documentsAverageable`,
`moderatorAbilities`, `ttl`, `entryPayload`, `actionFees`, `tokenCost`,
`transient`, `immutable`, `retractedWhen` and their conditions), and the
grammars the meta-schema held: `refersTo` on properties, on array
elements and as `ownerRefersTo` or `creatorRefersTo`, with its
`findBy`, `where` and `constraints`, and the rule language of
`propertyConstraints` and `deleteConstraints`.

With this, every rule of the meta-schema is checked natively, and a
schema breaking one is refused with a `JsonSchemaError` as before.

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

The native checks are compared with the document meta-schema, and
generation 4's parse with generation 3's, on:
- every document type of the contract corpus (1,503), which both
  generations parse to the same document type;
- hand-written document types using the keywords the corpus uses little
  (references, encryption, fees, token costs, moderation, constraints,
  windows and ranked indexes, index-only types);
- schemas generated by changing those at random places (2,700) and the
  corpus types (3,006).

Every case gets the same verdict from both sides. A refusal keeps its
error code, except where the `jsonschema` compile refused a schema the
meta-schema admitted, which the validation schema builder now refuses,
and the 255-byte index property name cap. Targeted tests pin each rule
area, where an error is reported, and the cap.

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

coderabbitai Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • 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.

@thepastaclaw

thepastaclaw commented Oct 22, 2026 •

Copy link
Copy Markdown
Collaborator

🕓 Review not started yet because the new head is waiting for the 30-minute push debounce.

  • Request normal review — click when the PR is ready for review.
  • Request priority review — click to move this review to the front of the queue.

Commit ae07137. Normal review starts when eligible; priority review starts as soon as a slot is available.

QuantumExplorer and others added 2 commits October 22, 2026 15:25
…n-checks

Brings in v5.1-dev through the base branch, including DPNS v3, whose
`CONTRACT_VERSIONS_V7` (V6 with `parse_property_constraints` 1, written
by struct update) the base merged with `validate_document` 2. The merged
table also sets `try_from_schema` 4, overriding V6's class method
versions the same way.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…n 15's changes

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@QuantumExplorer
QuantumExplorer marked this pull request as ready for review October 22, 2026 12:33
…n-checks

Brings in the validator debug fix: a document type prints its
validators as before the native one was added.

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

@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

Static verification of head 7991a1d found no blocking correctness or released-protocol compatibility defect. The remaining findings concern the shared validation-algorithm flag, an incorrect nested error location, undocumented structured-error changes, and a redundant numeric comparison branch. Current PR checks pass, including Kotlin SDK build/tests; no dedicated DPP Rust test result is listed, and no local builds or tests were run.

🟡 3 suggestion(s) | 💬 1 nitpick(s)

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 5: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 7: 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) — This is a large, intricate change to protocol-version-gated document schema validation that directly alters consensus-critical contract registration behavior.
  • Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
  • Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity quota lookup failed), glm-5.3-flash (not used above high effort; tier asks max)
  • 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/common/mod.rs`:
- [SUGGESTION] packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/common/mod.rs:225-229: Keep validation algorithm selection at a versioned boundary
  Generation 4 calls `try_from_schema_generation_3` with this flag set to true, selecting both native schema validation and the typed-validation-schema requirement inside code also reached by historical generations. Existing callers pass false, so this does not demonstrate a released-protocol regression. However, it violates the book's “Shared helpers get a versioned method, not a capability flag” rule: this selects an algorithm rather than supplying a generation-owned grammar constant. Give generation 4 its own implementation with unconditional native checks, or version the shared validation helper through an `OptionalFeatureVersion` table entry. Either approach preserves the existing protocol-15 activation while making the compatibility boundary structural.

In `packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/native_schema/properties.rs`:
- [SUGGESTION] packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/native_schema/properties.rs:299-302: Include the properties segment in nested-position errors
  Here `self` identifies the containing object schema, not its `properties` map. Removing `position` from the existing fixture's `meta.properties.a` therefore produces `/properties/meta/a` instead of `/properties/meta/properties/a`, pointing clients at a nonexistent location. Enter the `properties` segment before the member name. Extend the existing missing-position test to assert the path: the differential harness currently treats any pair of refusals as equivalent, so it cannot detect this defect.

In `packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/native_schema/mod.rs`:
- [SUGGESTION] packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/native_schema/mod.rs:123-141: Preserve or disclose changes to structured schema errors
  The native errors differ from the old errors beyond message text. `fail_with` always supplies an empty `schema_path`, type failures omit the previous `{type: ...}` parameter, and `name_error` omits the nested property-name diagnostic parameters. Native `oneOf` failures also use keyword `oneOf`, whereas the existing conversion in `error_data.rs` emits `oneOfNotValid` when no branch matches. These fields are exposed by `JsonSchemaError`, so consumers inspecting structured diagnostics observe a change despite the PR's “Only the message text differs” claim. Preserve the intended fields, or explicitly document these protocol-15 diagnostic changes and add focused assertions for the supported error shape.
- [NITPICK] packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/native_schema/mod.rs:568-574: Remove the redundant branch in numeric equality
  `right_integer` is the unchanged `right` value, so both branches perform exactly `left == right`. The fractional-part check has no effect and suggests an integer-specific comparison that does not exist. Collapse the arm to the comparison it actually performs.

QuantumExplorer and others added 6 commits October 22, 2026 18:42
…e field

The choice between the document meta-schema check and the native one
was a `native_schema_checks` flag on the shared `ParserGeneration`
context, set by a parser generation 4 that otherwise ran generation 3.
The coding conventions ask for a versioned method there instead: the
flag moved the version decision out of the tables.

`DocumentTypeSchemaVersions` gains `check_document_type_schema`: 0 (every
shipped table) checks a schema against the meta-schema through the
`jsonschema` crate and compiles it, as before; 1 (`CONTRACT_VERSIONS_V7`,
protocol version 15) checks it natively and requires a typed validation
schema. The shared `validate_document_type_schema` and the new
`require_validation_schema`, run last in generation 3's parse, dispatch
on it. Generation 4, the flag and the parse parameter are gone, and
protocol version 15 keeps `try_from_schema` 3. The comparison tests
register each case at the latest protocol version with the field at 1
and at 0.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The native check named the member of an object's `properties` that has
no `position` from the object itself, `/properties/meta/a`, where the
meta-schema check reported `/properties/meta/properties/a`. It now
enters `properties` first, and a test pins the path.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…and parameters

The native checks kept the old error code but not every structured
field of the `JsonSchemaError`. They now report as the meta-schema check
did:
- `type` with its `{type}` parameter (several types in sorted order);
- a closed property schema as `unevaluatedProperties` with
  `{unexpected}`, and `additionalProperties` without a property name;
- `oneOfNotValid` for a `oneOf` no branch of which matches;
- `not` with the schema it negates, and the element reference check at
  `refersTo/type`;
- a property name's error at the object that holds it, with the name's
  own error nested as `{instancePath, instance, params, keyword,
  propertyName}`;
- the meta-schema's exact pattern in every `pattern` error, and a
  token contract id byte as `type` or `maximum`.

A test pins keyword, location, parameters and property name against the
meta-schema check on 36 single-rule violations. The `schema_path` stays
empty and the nested `schemaPath` absent: the meta-schema location has
no native counterpart. The redundant branch of the numeric comparison
is also gone.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…n-checks

Brings in the native document validator's review fixes: violations
reported as the jsonschema validator reports them, match-only checks
without diagnostics, and activation assertions.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…n-checks

Brings in the native document validator's second round of review fixes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…n-checks

Brings in the additionalProperties key-search skip.

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

@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 b13a8be, the native registration checks are isolated to protocol version 15, and no blocking defect was confirmed. Three prior findings are fixed; one non-blocking structured-error compatibility gap remains. This was a static review with no builds or tests run; current CI checks pass but provide no targeted Rust/DPP test result.

🟡 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: platform-versioning); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 5: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 6: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); 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 large native_schema implementation, particularly check_document_type_schema, replaces consensus-relevant contract registration acceptance rules at protocol version 15 with intricate native validation logic.
  • Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
  • Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity quota lookup failed), glm-5.3-flash (not used above high effort; tier asks max)
  • 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 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/native_schema/indexes.rs`:
- [SUGGESTION] packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/native_schema/indexes.rs:79-82: Preserve or disclose changes to structured schema errors
  The structured-error corrections leave this exclusion branch returning different parameters. An otherwise valid document schema containing an index such as `{"name":"i","summableOffCountIndex":"base","rangeSummable":true,"terminal":"name"}` reaches `fail("not", ...)`, which supplies `{}`. The meta-schema's corresponding `then.not` condition instead produces `params.not = {"anyOf":[{"required":["terminal"]},{"required":["summable"]},{"required":["averageable"]}]}`: `error_data.rs` explicitly preserves the negated schema. This is an observable parameter difference beyond the message and schema-path exceptions disclosed in the PR description. Use the existing `not_error` helper here and add this single-rule violation to the focused error-shape comparison, or explicitly disclose the additional parameter difference.

The refusal of a `terminal`, `summable` or `averageable` beside
`summableOffCountIndex` kept `not` with empty parameters, where the
meta-schema check gives the negated schema. It now uses `not_error`
like the other `not` refusals, and the error-shape comparison gains the
case (37 single-rule violations).

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

@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 23c99b2, protocol-15 activation remains isolated from shipped versions, and all five prior findings are fixed; no blocking issue remains. One non-blocking signed-zero uniqueness difference needs focused coverage and disclosure. This was a static review: current GitHub checks pass, but no dedicated DPP Rust test result is listed, and no builds or tests were run locally.

🟡 1 suggestion(s)

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); 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: muse-spark-1.3-contributor (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) — This is a large and intricate change that directly alters protocol-versioned consensus validation rules for data-contract registration, including native schema acceptance and rejection behavior.
  • Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
  • Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity quota lookup failed), glm-5.3-flash (not used above high effort; tier asks max)
  • 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 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/native_schema/mod.rs`:
- [SUGGESTION] packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/native_schema/mod.rs:482-486: Cover and disclose the signed-zero uniqueness difference
  The reused `is_unique` helper differs from the legacy meta-schema check above the 15-element threshold. In the pinned `dashpay/jsonschema-rs` revision `aacc1ab5140d`, `unique_items.rs` hashes floats using their unchanged `to_bits()`, whereas the native helper normalizes both signed zeros before hashing. Consequently, an otherwise valid numeric property's 16-element enum `[0.0, -0.0, 1, 2, ..., 14]` can pass the legacy check but is rejected here with `uniqueItems`. The validating JSON conversion preserves signed zero, and the typed schema builder can represent this enum, so the existing documented exceptions do not cover the difference. Add focused 15/16-element boundary fixtures and disclose this stricter protocol-15 behavior in the module documentation and PR description. Keep the native helper's correct zero normalization rather than reproducing the fork's hash/equality defect; any upstream correction belongs in `dashpay/jsonschema-rs`.

QuantumExplorer and others added 8 commits October 23, 2026 12:11
…n-checks

Brings in v5.1-dev's tradeable tokens through the base branch, which
renumbered native document validation to note 6 of `PLATFORM_V15`.
Native registration checks become note 7, and both the contract
versions comment and note 7's cross-reference follow.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…n-checks

Brings in the uniqueness rule's doc comment.

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

# Conflicts:
#	packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/validate/mod.rs
The module doc said the native checks differ from the document
meta-schema in one rule, the 255-byte index property name. List
uniqueness, an `enum`'s members among them, also compares numbers by
value however long the list, so `0.0` and `-0.0` are one value: a
deliberate rule of protocol version 15, as in the native document
validator.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…n-checks

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…15's registration note

Note 7 named the 255-byte index property cap as a rule of the native
registration checks but not the other one their module doc lists: a list
that must be unique compares numbers by value however long the list.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…n-checks

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… 15 exactly

Note 7 and the native schema checks' module doc said a unique list
compares numbers by value however long it is. Above 15 elements `1` and
`1.0` still differ; the rule of protocol version 15 is only that `0.0`
and `-0.0` are one value at every length.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…n-checks

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…n-checks

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
QuantumExplorer and others added 4 commits October 23, 2026 21:03
…n-checks

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…n-checks

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ot build

`check_validation_schema` now builds the document type's
`DocumentSchemaValidator` as well as its validation schema, so a type
whose documents protocol version 15 would refuse, such as one whose
references cycle without entering the value, is refused when it is
registered, with the same `InvalidContractStructure`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…n-checks

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

@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

The current head correctly version-gates native schema checks, and the previously reported parity and dispatch issues are addressed. I found no blocking defect: the alleged root dependentRequired omission is refuted by the v3 meta-schema, while two non-blocking issues remain—the signed-zero boundary lacks a regression fixture, and allowing $ in validation-schema $id names changes the protocol-14 public builder behavior. The CI snapshot contains no failed checks.

🟡 2 suggestion(s)

1 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); 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) — This is a large, intricate change that directly alters consensus-critical document-schema acceptance and protocol-version-gated contract registration behavior in the native validator implementation.
  • Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
  • Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity quota lookup failed), glm-5.3-flash (not used above high effort; tier asks max)
  • 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 each finding against the current code and only fix it if needed.

In `packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/mod.rs`:
- [SUGGESTION] packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/mod.rs:824-827: Gate `$id` acceptance to the protocol-15 validation-schema generation
  The `$id` scanner now accepts `$`, but `DataContract::document_type_validation_schema` remains callable with protocol version 14 and only gates on `validate_max_bytes`; it does not consult `check_document_type_schema`. Its call to `ValidationSchema::try_from_root` therefore now accepts values such as `"$ownerId"` under protocol 14 that the previous released behavior rejected. The PR describes this spelling as a protocol-15 builder change, so preserve the protocol-14 behavior by making the identifier rule generation/version-aware or by using a protocol-15-specific builder path, and add a protocol-14 regression test.

In `packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/native_schema/mod.rs`:
- [SUGGESTION] packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/native_schema/mod.rs:13-16: Cover and disclose the signed-zero uniqueness difference
  (existing thread: https://github.com/dashpay/platform/pull/5538#discussion_r4353307235)
  The protocol-15 signed-zero rule is now documented, but there is still no focused regression fixture covering the uniqueness threshold. Add paired 15- and 16-element `enum`/`uniqueItems` cases containing `0.0` and `-0.0`: the 15-element case should pin the pairwise behavior, while the 16-element case should pin the intentional native rejection that differs from the legacy validator. Without these fixtures, a future change to the pairwise/hash boundary or zero normalization could silently alter the documented protocol-15 behavior.

Comment thread packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/mod.rs Outdated
QuantumExplorer and others added 2 commits October 24, 2026 10:23
…n-checks

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

# Conflicts:
#	packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/mod.rs
#	packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/tests.rs
#	packages/rs-platform-version/src/version/dpp_versions/dpp_contract_versions/v7.rs
…ersioned builder

`check_validation_schema` builds the type's native validator with
`DocumentSchemaValidator::try_from_root`, the builder protocol version 15
selects (`validation_schema`), reusing the patterns it compiles, instead
of calling the unversioned builder and compiling every pattern again.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@QuantumExplorer QuantumExplorer changed the title feat(dpp): check document schemas natively at registration from protocol version 15 feat(dpp)!: check document schemas natively at registration from protocol version 15 Oct 24, 2026
…n-checks

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

@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

One blocking upgrade-compatibility defect remains: protocol 15 switches document validation to the native validator for contracts loaded from state, so some contracts validly registered under protocol 14 become unusable after activation. Two non-blocking issues also remain: mixed large integer/float equality can diverge from the pinned JSON Schema fork, and the signed-zero uniqueness boundary is not pinned by tests. The supplied CI snapshot shows no failed checks, though it does not include a DPP build or test result.

🔴 1 blocking | 🟡 2 suggestion(s)

Review provenance

Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 5: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 7: 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) — This is a large, intricate change to consensus-critical document-contract validation and protocol-version dispatch, directly changing which registrations are accepted at protocol version 15.
  • Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
  • Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity quota lookup failed), glm-5.3-flash (not used above high effort; tier asks max)
  • 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-platform-version/src/version/dpp_versions/dpp_contract_versions/v7.rs`:
- [BLOCKING] packages/rs-platform-version/src/version/dpp_versions/dpp_contract_versions/v7.rs:24-38: Preserve legacy document validation for contracts registered before protocol 15
  `CONTRACT_VERSIONS_V7` selects `validate_document: 2` and `validation_schema: Some(0)` for the current platform version. The protocol-15 validator lazily calls `document_type_native_validator`, while contracts fetched from state are reconstructed with `full_validation: false` and carry no registration-generation marker that would select the legacy validator. Consequently, a contract registered under protocol 14 with a schema admitted by the old meta-schema but rejected by the typed validation-schema builder—such as a property containing `$ref` together with `maxBytes`, covered by the protocol-14 registration fixtures—will load successfully after the upgrade but every document validation will return `InvalidContractStructure`. This changes existing state behavior and can permanently disable creates, replaces, and other document operations for that contract. Preserve the validator semantics used when the contract was registered, or add an upgrade/migration mechanism that makes these legacy schemas representable before protocol 15 document validation is activated.

In `packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/native_schema/mod.rs`:
- [SUGGESTION] packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/native_schema/mod.rs:678-685: Preserve exact mixed integer/float equality in native schema uniqueness checks
  The numeric equality fallback converts both numbers to `f64` whenever the two values do not have the same integer representation. Thus an integer such as `9007199254740993` and the float `9007199254740992.0` compare equal after rounding in the native registration checker, even though the pinned `jsonschema-rs` fork compares mixed numeric representations with exact `NumCmp` semantics and treats them as distinct. This affects the `uniqueItems` check used for schema `enum` members, creating an additional protocol-15 verdict difference beyond the two documented changes. Reuse the exact mixed-number comparison already implemented by the native document validator and add a regression fixture for these values.

In `packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/native_schema/tests.rs`:
- [SUGGESTION] packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/native_schema/tests.rs:742-753: Cover and disclose the signed-zero uniqueness difference
  The protocol-15 signed-zero behavior is now documented, but the committed tests still do not exercise `0.0` and `-0.0` at the important 15/16-item boundary. `is_unique` uses pairwise comparison through 15 items and switches to the hashed path at 16, so the generated corpus is unlikely to pin the exact transition or protect the normalization rule from regression. Add explicit 15-item and 16-item enum or unique-items fixtures through the native registration checker, asserting the intended protocol-15 result.

Comment on lines +742 to +753
fn generated_values() -> Vec<JsonValue> {
vec![
json!(null),
json!(true),
json!(false),
json!(0),
json!(1),
json!(2),
json!(-1),
json!(1.5),
json!(2.0),
json!(32),

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.

🟡 Suggestion: Cover and disclose the signed-zero uniqueness difference

The protocol-15 signed-zero behavior is now documented, but the committed tests still do not exercise 0.0 and -0.0 at the important 15/16-item boundary. is_unique uses pairwise comparison through 15 items and switches to the hashed path at 16, so the generated corpus is unlikely to pin the exact transition or protect the normalization rule from regression. Add explicit 15-item and 16-item enum or unique-items fixtures through the native registration checker, asserting the intended protocol-15 result.

source: muse-spark-1.3-contributor (phase1-reviewer: general, rust-quality); gpt-6.1-sol (phase2-reviewer: rust-quality)

QuantumExplorer and others added 2 commits October 24, 2026 13:59
The registration checks compared an `enum` member, and a `const` that is
not a number, with their own `equal_json`, which read a mixed integer and
float pair through f64: `9007199254740993` equalled `9007199254740992.0`,
where the meta-schema check compares the readings exactly. They now use
the native document validator's `equal_json`, which compares numbers
exactly across their unsigned, signed and float readings, as they already
use its `is_unique`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…n-checks

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

# Conflicts:
#	packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/mod.rs

@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

Protocol 15's native registration and document validation paths are correctly version-gated, and the previously reported structured-error, numeric-equality, nested-path, and dispatch issues are fixed. A blocking compatibility gap remains: protocol-14 contracts can be stored with schemas accepted by the legacy checker but rejected when protocol 15 lazily builds the native validator, while the signed-zero boundary behavior still lacks focused regression coverage and two native schema rules diverge from the meta-schema.

🔴 1 blocking | 🟡 4 suggestion(s)

3 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); 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: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 7: 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) — This is a large, intricate change that directly alters protocol-versioned consensus validation rules for data contract registration, including native schema acceptance and error behavior in the critical registration path.
  • Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
  • Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity quota lookup failed), glm-5.3-flash (not used above high effort; tier asks max)
  • 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/native_schema/properties.rs`:
- [SUGGESTION] packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/native_schema/properties.rs:114-118: Validate the schema shape of a property's contains keyword
  The `contains` arm accepts every JSON value, but the v3 meta-schema delegates this keyword to the draft applicator vocabulary, where `contains` must be a schema object or boolean schema. A property schema containing `"contains": 1` is therefore rejected by the protocol-14 meta-schema but accepted by the protocol-15 native registration checker. Restrict the native check to object and boolean values so this unlisted verdict difference cannot reach registration.

In `packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/native_schema/indexes.rs`:
- [SUGGESTION] packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/native_schema/indexes.rs:6-7: Keep the protocol-visible index key limit in the version tables
  `MAX_INDEX_PROPERTY_NAME_BYTES` changes which contracts protocol 15 accepts, but the value is hardcoded inside the native checker. The repository's versioning conventions require protocol-visible limits to be owned by versioned tables or `SystemLimits`, so a later protocol can select a different value without editing the implementation selected by an already-shipped protocol. Move the limit to the appropriate versioned document-schema configuration and pass the selected value into the native check.

In `packages/rs-platform-version/src/version/dpp_versions/dpp_contract_versions/v7.rs`:
- [BLOCKING] packages/rs-platform-version/src/version/dpp_versions/dpp_contract_versions/v7.rs:35-38: Preserve legacy document validation for contracts registered before protocol 15
  (existing thread: https://github.com/dashpay/platform/pull/5538#discussion_r4366891896)
  Protocol 15 selects native document validation and requires the typed validation schema for every document type loaded under that platform version, but stored contracts do not record the validation generation used when they were registered. A contract accepted under protocol 14 by the legacy meta-schema, such as one with `maxBytes` beside a property `$ref`, can therefore load after activation and reach `document_type_native_validator`, where the protocol-15 builder refuses it with `InvalidContractStructure`. Every document create or replace for that type then fails, permanently disabling an otherwise valid pre-15 contract. The corpus census does not establish a protocol invariant because such a contract can still be registered before activation. Preserve registration-era validation semantics, persist or derive a compatibility marker, or provide a deterministic migration/gating mechanism before activating protocol 15.

In `packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/native_schema/tests.rs`:
- [SUGGESTION] packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/native_schema/tests.rs:742-752: Cover and disclose the signed-zero uniqueness difference
  (existing thread: https://github.com/dashpay/platform/pull/5538#discussion_r4366891902)
  The protocol-15 signed-zero rule is now disclosed in the module and PR documentation, but the registration differential tests still contain no `0.0`/`-0.0` pair and no 15/16-element boundary fixture. The generated corpus therefore cannot exercise the long-list hashing path where this behavior differs from the legacy checker. Add explicit 15- and 16-element `enum` or `uniqueItems` cases containing both signed zeros so the intentional protocol-15 behavior remains pinned at the registration layer.

In `packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/native_schema/mod.rs`:
- [SUGGESTION] packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/native_schema/mod.rs:493-499: Cover and disclose the signed-zero uniqueness difference
  (existing thread: https://github.com/dashpay/platform/pull/5538#discussion_r4353307235)
  The shared `is_unique` helper correctly normalizes `-0.0` and `0.0` in both its pairwise and hashed paths, but no focused validator test exercises that behavior at the 15/16-element threshold. The existing tests cover ordinary numeric equality and object ordering, not the documented signed-zero exception. Add direct validator fixtures for both sides of the boundary so a future change cannot silently reintroduce length-dependent behavior.

QuantumExplorer and others added 5 commits October 24, 2026 17:45
The 255-byte cap on an index property name, which the native
registration checks hold at protocol version 15, was a constant inside
the checker. It is now `SystemLimits::max_index_property_name_bytes`,
255 in every table and read only by `check_document_type_schema` 1, which
passes it down from `platform_version`, so a later protocol version
selects another cap by table.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The document meta-schema check does not constrain `contains`, so the
native checks admit `"contains": 1` as it did; registration still refuses
the type under either check, when the JSON Schema validator compiles the
schema or when the validation schema is built.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…n-checks

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…n-checks

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

# Conflicts:
#	packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/validate/mod.rs
…n-checks

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

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants