[QuantumExplorer]

Basic explanation

What this does: From protocol version 15, Platform checks each document against its document type's schema with its own code, instead of the third-party jsonschema library it uses today. It reads the typed copy of the schema that #5486 added, and repeats the library's checks rule by rule, including its quirks, so the same documents pass and fail, but for two deliberate rules listed under Breaking Changes. A document type whose schema the typed copy cannot represent has its documents refused, so at protocol version 15 the library checks no document at all.

Value: Document validation no longer depends on the internals of a pinned fork of jsonschema for new blocks. The rules are now Platform code that can be read, tested and later extended, which the compiled document types and storage changes planned for 5.1 build on.

Risks: Consensus changes at protocol version 15 only; protocol version 14 and earlier keep the library. If the new code disagreed with the library on some document, nodes at protocol version 15 would accept or refuse it differently from today (all nodes alike, so no split). The tests compare both on hand-picked cases for every rule and on generated schemas and documents. A document type whose schema the typed copy cannot represent has its documents refused at protocol version 15. No registered contract has such a type today (see the census caveat under Breaking Changes). Failed documents keep the same error code and fees. The error a client sees can change: the message is worded differently, and when a document breaks more than one rule, another violation can be listed first.

Issue being fixed or feature implemented

Step 1 of the 5.1 contract-definition plan: a native document validator replacing the jsonschema fork at protocol version 15. Builds on #5486 (contract corpus and typed validation schema, merged).

What was done?

Native validator (data_contract/document_type/schema/validation_schema/validate)

• DocumentSchemaValidator checks a document's validating JSON against a ValidationSchema and returns its first violation, as the JsonSchemaError reporting it, or none. Consensus reports only the first error, so the walk stops there, and checks nothing more of a value once it has failed.
• Each keyword repeats the pinned fork's check, with the crates the fork uses:
  • bounds: exact comparisons across unsigned, signed and float readings (num-cmp);
  • multipleOf: f64 division as the fork does it, with fraction deciding when the quotient overflows;
  • type: a float with no fractional part is an integer;
  • enum, const: numbers by value, const numbers within f64::EPSILON, objects key by key in written order;
  • uniqueItems: up to 15 elements pairwise by value, more by JSON value (1 and 1.0 distinct, objects equal in any key order);
  • minLength/maxLength count characters; pattern is the fork's ECMA conversion compiled with regex, searched for anywhere;
  • formats date, date-time (time), email, idn-email, hostname, ipv4, ipv6 (std), regex, time, uri (url), each as the fork reads them;
  • byteArray: refuses a non-array, a float, a negative number or one above 255; inside contains, which only asks whether an element matches, it is not applied (the builder refuses it there anyway).
• maxBytes and encryptedFor are not checked here: document validation checks them in their own steps, as today.
• The builder compiles each pattern once and the validator reuses it. Building the validator refuses a chain of $refs that returns to where it started without entering the value, and a property declared twice.
• The builder has its own version entry, DocumentTypeSchemaVersions::validation_schema: None through protocol version 14, Some(0) at 15. document_type_validation_schema and DocumentSchemaValidator::try_from_root refuse a version that selects none, so a later change to the builder ships as a new generation. Generation 0 of the builder (validation_schema/v0) and of the walk (validate/v0) are frozen in their own modules: the validator records the generation that built it, validate dispatches on it, and the cache on a document type holds one slot per generation.
• The formats the validator knows are one enum, ValidatorFormat, which the walk matches exhaustively. Each builder generation lists by name the formats it admits (generation 0 in validation_schema/v0), so a variant added for a later generation is not admitted at protocol version 15.
• num-cmp, fraction and time join url as optional dependencies of the validation-schema feature, at the versions the fork already pulls in.

Document validation (data_contract/methods/validate_document)

• validate_document / validate_document_properties generation 2: generation 1's checks (depth, repeated keys, field sizes, maxBytes, generatedFrom, property constraints, in the same order), now shared as validate_document_properties_v1_checks with the schema step passed in, here the native validator. Violations become BasicError::JsonSchemaErrors through a new JsonSchemaError::new, so a failed transition carries the same error code and fees. Only info, which the results hash does not cover, can differ: the message is worded differently, and the walk meets violations in a fixed order ($ref, type, enum, const, then the keywords of the value's kind) rather than in the order the schema writes them, so a document breaking two keywords, such as {"enum": ["a"], "type": "string"} with 1, can get another violation first (type here, enum before).
• The native validator is built on first use and kept on the document type, in the slot of StatelessJsonSchemaLazyValidator for the builder generation the protocol version selects. A document type whose schema the builder refuses has its documents refused with BasicError::ContractError(InvalidContractStructure) (code 10231) before any other check. The refusal is kept in the same slot, so it is decided once per document type. Generation 2 never calls the jsonschema crate.
• CONTRACT_VERSIONS_V7, used by protocol version 15 only, selects validate_document: 2 and validation_schema: Some(0).

Example: a property {"type": "integer", "maximum": 10} and the document {"count": 3.0}.

Before (protocol version 14): the jsonschema fork accepts it (3.0 has no fractional part).

After (protocol version 15): DocumentSchemaValidator accepts it for the same reason; {"count": 11} is refused by both with a JsonSchemaError on maximum.

How Has This Been Tested?

• cargo test -p dpp --lib validation_schema and cargo test -p dpp --lib validate_document::v2:
  • fixed cases for numbers, array elements (uniqueItems both sides of 15, contains, byteArray), strings and every format, objects and references, each checked against the existing validator as well as an expected verdict;
  • a generated differential: 800 generated schemas of the keywords the tree holds, with keys the validator ignores among them; for each one the builder builds and the existing validator compiles, both validators judge 34 generated documents and must agree (a planted fault in the integer check fails it);
  • the first violation and its paths, a $ref cycle and a property declared twice (refused when the validator is built);
  • through validate_document_properties (validate_document/v2/tests.rs): protocol version 15 against 14 on a registered contract (same verdict and same first error code on 13 documents), and a schema with not (no validation schema), checked through the library at protocol version 14 and every document refused at 15, and a type whose references cycle, refused at 15;
  • the structured fields of the first violation against the existing validator's, including additionalProperties: false without properties (falseSchema), escaped required names and a repeated type, the walk's order of violations, and a contains check that builds no violation.
• cargo test -p dpp --test contract_corpus, cargo test -p dpp --lib property_type_shorthand_tests, cargo test -p dpp --lib validate_document, cargo test -p platform-version.
• cargo test -p drive-abci --lib -- batch::tests::document::property_type_shorthands batch::tests::document::creation: 100 tests, at protocol version 15 by default, so through the native validator.
• cargo clippy -p dpp --all-features --lib --tests -- -D warnings; cargo check -p dpp --no-default-features --features validation-schema.

In-place changes to shipped generations

• StatelessJsonSchemaLazyValidator (document_type/validator.rs), held by every document type at every protocol version, gains slots for the native validator, or the refusal of the type's schema, one per generation of the validation schema's builder. Validate-document generations 0 and 1 (protocol versions 1 to 14) read only the first slot, the jsonschema validator, through the unchanged Deref; clone still resets both. Consensus cannot change there: nothing in those generations reads the new slot.
• validate_document_properties_v1 (generation 1, protocol version 14): its checks move, unchanged and in the same order, into validate_document_properties_v1_checks, which takes the schema step as a closure. Generation 1 passes its jsonschema check, which still runs after the JSON conversion. Consensus cannot change: the same code runs in the same order with the same errors.
• DocumentTypeSchemaVersions gains validation_schema, None in every shipped table (CONTRACT_VERSIONS_V1 to V6). Nothing at those versions reads it.

Breaking Changes

None for released protocol versions or released crates. One Rust API change to unreleased code: FormatConstraints::format, added on v5.1-dev by #5486 and in no release yet, is now Option<ValidatorFormat> instead of Option<String>. Nothing outside dpp reads it.

At protocol version 15 (unreleased), two rules are deliberate rather than copied:

• uniqueItems treats 0.0 and -0.0 as one value however long the list is, so a list holding both is refused at any length. Other numbers compare as before: above 15 elements 1 and 1.0 still differ.

• A document type whose schema the typed validation schema cannot represent has its documents refused with InvalidContractStructure (10231). Examples: a not keyword, or maxBytes or byteArray inside contains.

  Before (protocol version 14): a type with "name": {"type": "string", "not": {"const": "x"}} accepts {"name": "y"} through the library.

  After (protocol version 15): {"name": "y"} is refused: "document type item has no validation schema, which its documents are checked against: …".

Census caveat: every document type in the contract corpus has a validation schema, so no registered contract is affected today. The corpus holds all 55 mainnet contracts and the 694 distinct testnet schemas, 1,503 types, taken on 18 October 2026, and contract_corpus asserts that each one builds. A contract registered at protocol version 14 after that date, with a schema the typed copy cannot represent, would have its documents refused once 15 activates. Re-take the corpus and rerun cargo test -p dpp --test contract_corpus before protocol version 15 activates. From 15, #5538 refuses such a type at registration.

Checklist:

• I have performed a self-review of my own code
• 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

PR Hygiene · 8bf3aad

• Bots — coderabbitai not yet · thepastaclaw not yet — /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.

[coderabbitai]

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

• Source mode: Auto
• Repository: dashpay/platform
• Branch: claude/native-document-validator
• File: .coderabbit.yaml
• Commit: 8bf3aad
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: d8e0f400-1fbe-4a23-a64a-ccf137478c41

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[thepastaclaw]

🕓 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 8bf3aad. Normal review starts when eligible; priority review starts as soon as a slot is available.

[github-actions]

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]

Waiting for bot review — coderabbitai not yet · thepastaclaw requested changes — dismiss the review or push a fix, 1 thread unresolved — resolve it. 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]

Final validation — Phase 1 + Phase 2

At c2b300b, native validation is correctly isolated to protocol version 15, but signed-zero hashing introduces an unintended document-validity change that contradicts the promised legacy parity. Structured error metadata also differs, and the integration tests do not prove native activation or fallback initialization. This was static verification only; current CI shows Rust workspace tests passing, with Swift SDK build/tests and PR Hygiene pending.

🔴 1 blocking | 🟡 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-versioned document validation that directly changes consensus rules for accepted state transitions in the native validator and validation dispatch.
• 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/schema/validation_schema/validate/mod.rs`:
- [BLOCKING] packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/validate/mod.rs:864-869: Preserve signed-zero validity for long unique arrays
  Normalizing negative zero changes document acceptance rather than just error wording. The pinned jsonschema revision aacc1ab switches to AHashSet above 15 elements, and its HashedValue hashes floating-point numbers with number.to_bits(), distinguishing 0.0 from -0.0. Here both receive the same hash and then compare equal, so the native validator rejects their coexistence. For an array property with items: {"type":"number"}, uniqueItems: true and maxItems: 16, [0.0,-0.0,1,2,3,4,5,6,7,8,9,10,11,12,13,14] exposes this legacy/native acceptance difference. Both implementations reject the corresponding short array through pairwise numeric equality. This is an unintended consensus-validation change at activation, not a historical replay or cross-node split issue. Preserve the intended legacy verdict, or explicitly establish this exception as an intentional PV15 rule change rather than promising identical acceptance. Add signed-zero differential cases on both sides of the 15-element threshold.
- [SUGGESTION] packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/validate/mod.rs:402-404: Avoid constructing diagnostics for boolean-only contains checks
  Each nonmatching contains candidate builds SchemaViolation diagnostics even though this caller only reads whether the vector is empty. Walk::fail constructs paths, parameters, a property name and a message; enum and const failures additionally convert expected schema values into JSON before reaching fail. A contains check that eventually succeeds can therefore repeatedly copy a large enum or const and discard all those diagnostics. The pinned implementation instead uses its boolean is_valid path. Introduce a matching failure flag or boolean result and construct diagnostic arguments lazily, while retaining ProtocolError propagation. Suppressing only the push in fail would not avoid the eagerly evaluated json! and format! work.
- [NITPICK] packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/validate/mod.rs:954-960: Use the borrowed JSON converter instead of cloning the schema value
  json_of_value clones the complete platform Value before calling the consuming converter. platform_value already provides try_to_validating_json(&self), whose conversion semantics mirror the consuming method. Using it avoids an unnecessary intermediate copy of enum or const schema values on the diagnostic path.

In `packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/validate/tests.rs`:
- [SUGGESTION] packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/validate/tests.rs:430-435: Preserve structured error metadata, not just first error codes
  Every schema violation maps to 10101, so comparing only the first code cannot establish the claimed compatibility beyond message wording. Source comparison confirms several structured differences: native $ref traversal inserts /$ref where the pinned RefValidator directly appends the target error path; dependentRequired reports /required instead of /dependentRequired; byteArray failures acquire a schema path where the existing keyword supplies an empty path; and union-type params preserve schema order instead of the fork's primitive-type bitmap order. For example, type ["string","number"] produces params.type ["string","number"] natively but ["number","string"] in the fork. The native additionalProperties branch also sets property_name to the first unexpected key, whereas the existing JsonSchemaErrorData conversion leaves it empty. These fields are public JsonSchemaError data, including schema-path and property-name WASM accessors. Align the structured payloads or explicitly document the API differences, and add differential assertions for keyword, paths, params and property_name, excluding intentionally different summaries and accounting for error ordering.
- [SUGGESTION] packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/validate/tests.rs:427-435: Assert that the integration test actually activates the native validator
  These assertions also pass if PV15 dispatches to generation 1, generation 2 always falls back, or native initialization always caches None: all those paths produce the legacy verdict and first error code. Direct DocumentSchemaValidator tests cannot detect a broken connection to document validation. For this supported registered schema, validate at PV14 first and assert that the native OnceLock remains uninitialized; then validate at PlatformVersion::latest() and assert that the initialized slot contains Some(validator). In the unsupported-schema test, assert that the initialized slot contains None. This makes the tests protect activation and fallback selection rather than only equivalent outputs.

[github-actions]

Waiting for bot review — coderabbitai not yet · thepastaclaw ✓, 1 thread unresolved — resolve it. 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]

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

At ccf82a0, the PV15-only dispatch, legacy fallback, and cache lifecycle preserve historical validation, and three prior findings are fixed. The remaining signed-zero compatibility difference and structured-error mismatches are non-blocking, alongside two avoidable allocation patterns. Verification was static only; current CI shows Rust workspace tests pending, with the JavaScript build/tests and Kotlin checks passing.

🟡 4 suggestion(s)

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

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: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); 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); 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 native schema validator in packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/validate/mod.rs and validate_document/v2 change consensus-critical document acceptance rules when activated 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 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/validate/mod.rs`:
- [SUGGESTION] packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/validate/mod.rs:506-508: Preserve structured error metadata, not just first error codes
  The direct-keyword repairs are correct, but two supported compositions still change public error fields beyond the intentionally different summary. First, ByteArrayKeyword emits an empty schema path, but the pinned RefValidator::validate subsequently prepends the reference site's path. With properties.value referencing #/$defs/bytes, where bytes contains type: array and byteArray: true, {"value":[256]} therefore reports /properties/value in the legacy validator and an empty path here. Preserve the accumulated reference-origin prefix for these custom-keyword errors while retaining an empty path for nonreferenced byteArray errors. Second, the type diagnostic at lines 243–254 reports params.type = ["string"] for {"type":["string"]}, whereas the fork's type compiler routes singleton arrays through compile_single_type and reports params.type = "string". Normalize that diagnostic shape as well. Extend the structured differential cases with both examples; the current cases cover references and byteArray separately and only a multi-member type union.
- [SUGGESTION] packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/validate/mod.rs:417-423: Keep traversal paths borrowed until a violation needs rendering
  Every typed-array element allocates an owned "items" segment and converts its index into another String, including on successful validation. A valid 1,024-element array therefore performs 2,048 string allocations solely for path bookkeeping; object traversal likewise clones property names into both path stacks. These paths only need rendering when a violation is reported. Represent segments with borrowed/static names and numeric indices, format them in pointer() on failure, and move the invariant items segment outside the loop. This removes allocator traffic from the normal validation path without changing diagnostics.
- [SUGGESTION] packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/validate/mod.rs:861-869: Avoid allocating a collision vector for every unique element
  For ordinary distinct elements, each new hash creates an empty Vec and bucket.push(item) allocates its backing storage. A 1,024-element unique array therefore adds approximately 1,024 individual vector allocations, in addition to the BTreeMap storage, on every validation. The pinned implementation instead reserves one hash-set table. Keep the first bucket member inline and allocate overflow storage only for actual collisions, or use a pre-sized set with a Hash/Eq-consistent borrowed wrapper implementing the chosen JSON semantics. The inline-bucket approach can remove these allocations without changing the current equality behavior.
- [SUGGESTION] packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/validate/mod.rs:891-896: Preserve signed-zero validity for long unique arrays
  (existing thread: https://github.com/dashpay/platform/pull/5533#discussion_r4350439514)
  The normalization introduces an exception to the promised verdict compatibility. At the pinned aacc1ab revision, unique_items::HashedValue hashes floating-point numbers using their raw to_bits(), although its derived equality treats +0.0 and -0.0 as equal. Consequently, its hash-set path for arrays longer than 15 can accept both representations, whereas this implementation always places them in the same bucket and rejects them. An otherwise distinct 16-element array such as [0.0,-0.0,1,2,3,4,5,6,7,8,9,10,11,12,13,14] exposes the difference; signed zeros nested inside elements have the same issue. Platform Value's validating-JSON conversion preserves these signs. PV15-only activation makes this a permitted upgrade-time behavior change, not a historical replay or same-version consensus blocker. Explicitly document the correction and add boundary coverage at lengths 15 and 16, including nested values, or implement a deliberately defined compatibility rule. Simply restoring an inconsistent Hash/Eq pairing is not a robust fix.

[github-actions]

Waiting for bot review — coderabbitai not yet · thepastaclaw ✓, 1 thread unresolved — resolve it. 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]

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

At c6d2e0d, the new validator is correctly isolated to PV15, and seven prior findings are fixed. Two nonblocking issues remain: signed-zero compatibility for long unique arrays and quadratic additional-property membership checks; the signed-zero difference is not a historical-replay or cross-node consensus violation. This was a static review without local builds or tests; current CI reports passing Rust workspace tests and JS builds, with PR Hygiene pending.

🟡 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: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 3: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); 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, intricate change to consensus-critical document validation, including protocol-version-gated acceptance rules in the native validator and validate_document paths.
• 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/validate/mod.rs`:
- [SUGGESTION] packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/validate/mod.rs:360-368: Index declared property names for additionalProperties checks
  Every instance key restarts a linear search through the declared-properties vector. A valid object containing all 100 supported properties therefore performs 5,050 name comparisons just to establish that no additional properties exist, after the preceding traversal has already validated the properties. The pinned implementation instead selects a hash-backed BigValidatorsMap at 40 declarations. Cache a membership index when preparing the native validator while retaining the vector for traversal order, or count recognized properties during the preceding traversal and skip unexpected-key enumeration when every supplied key was recognized.
- [SUGGESTION] packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/validate/mod.rs:925-931: Preserve signed-zero validity for long unique arrays
  (existing thread: https://github.com/dashpay/platform/pull/5533#discussion_r4350439514)
  The long-array path still changes the promised validation verdicts. The pinned jsonschema revision aacc1ab hashes floating-point values using their raw to_bits() and uses AHashSet above 15 elements, so an array containing 0.0, -0.0, and the fourteen distinct values 1 through 14 can pass its uniqueness check. Native hashing puts both zeros in the same group, where serde_json equality rejects them as duplicates. This is reachable through a supported typed number-array property: platform-value's validating JSON conversion preserves both floating-point representations. The PV15 boundary correctly protects historical replay, so this is a compatibility discrepancy rather than a chain-split finding. Add registered-contract differential coverage at lengths 15 and 16, then either preserve the legacy distinction in generation 2 or explicitly document and test the stricter signed-zero rule as intentional PV15 behavior instead of claiming unchanged verdicts.

[github-actions]

Waiting for bot review — coderabbitai not yet · thepastaclaw ✓, 1 thread unresolved — resolve it. 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]

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

At head 558f3a8, native validation is correctly isolated to protocol version 15, with historical validation and unsupported-schema fallback preserved. Eight prior findings are fixed; the signed-zero compatibility discrepancy remains as a non-blocking request to document and test the new rule, not a historical-replay or demonstrated cross-node consensus defect. This was a static review; current CI shows Rust workspace tests, JS builds/tests, Kotlin checks, and Swift checks passing, with PR Hygiene pending.

🟡 1 suggestion(s)

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

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: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); 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) — The intricate native schema implementation in validation_schema/validate/mod.rs and its integration in validate_document/v2/mod.rs directly change consensus document acceptance rules 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 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/validate/mod.rs`:
- [SUGGESTION] packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/validate/mod.rs:930-935: Preserve signed-zero validity for long unique arrays
  (existing thread: https://github.com/dashpay/platform/pull/5533#discussion_r4350439514)
  The normalization is consistent with JSON equality, but it is not acceptance-compatible with the pinned fork. At aacc1ab5140daac30eb65d376852f01f5381979d, jsonschema/src/keywords/unique_items.rs hashes floats using raw to_bits() and uses AHashSet above 15 elements, despite comparing floating-point signed zeros equal. A 16-element array containing 0.0, -0.0, and the otherwise distinct numbers 1 through 14 can therefore pass the legacy check, while native validation always rejects it. This is reachable with a supported typed number array, and the validating-JSON conversion preserves both float signs. The existing fixed and generated cases do not cover this difference.

  Generation 2 is correctly confined to unreleased PV15, so this does not establish a historical-replay failure or a split between nodes running the new validator. Treat the stricter result as an explicit PV15 compatibility exception: update the same-verdict claims in the PR description, generation-2 documentation, and v15 notes, and add regression coverage below and above the 15-element threshold through both the native API and document-validation dispatcher. Because the legacy implementation violates Hash/Eq consistency, do not assert that it always accepts the long example or blindly copy its broken hash contract as the repair.

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.

• Audit the legacy validator's signed-zero hash/equality inconsistency — The pinned dashpay/jsonschema-rs uniqueItems implementation hashes floating-point signed zeros differently while deriving equality from serde_json::Value, which considers them equal. Its long-array branch uses a randomly seeded AHashSet, so duplicate detection can depend on whether the hash-table lookup compares those equal values. The same implementation is present on the dependency's default branch. This predates the PR and remains relevant to historical validation and unsupported-schema fallback; the native normalization does not introduce it.
  • Follow-up: Track a separate dashpay/jsonschema-rs investigation with explicit hash-seed regression coverage and a versioned Platform adoption policy. Do not repair historical validator behavior in place.

[github-actions]

Waiting for bot review — coderabbitai not yet · thepastaclaw 1 thread unresolved — resolve it. 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]

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.

[thepastaclaw]

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

The native document validator is correctly isolated to protocol version 15, and the historical validation generations remain unchanged. Four non-blocking issues remain: one structured-error compatibility gap, missing signed-zero activation coverage, an inaccurate protocol changelog statement, and avoidable uniqueness work after a failed match-only check. Rust workspace tests and the relevant SDK/build checks pass in CI; only workflow hygiene and queued setup checks remain pending.

🟡 4 suggestion(s)

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

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: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); 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) — The new DocumentSchemaValidator in packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/validate/mod.rs implements intricate schema-validation rules that generation-2 document validation activates at protocol version 15, directly changing consensus acceptance rules.
• 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/validate/tests.rs`:
- [SUGGESTION] packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/validate/tests.rs:99-123: Pin the intentional signed-zero rule with regression tests
  The intentional protocol-15 exception depends on `hash_json` normalizing `-0.0` to `0.0`, but none of the direct uniqueness cases contains both signed zeros, and the generated values contain neither that pair nor a long array. Removing the normalization would therefore leave the current tests passing while silently restoring the pinned validator's long-array verdict. Add explicit short and long arrays containing `0.0` and `-0.0`, asserting that the short array is rejected by both validators and that the long array is accepted by protocol 14 but rejected by protocol 15 through `validate_document_properties`. Keep the long-array activation assertion separate from the ordinary differential agreement helper because the disagreement is intentional.

In `packages/rs-platform-version/src/version/v15.rs`:
- [SUGGESTION] packages/rs-platform-version/src/version/v15.rs:140-146: Document the signed-zero exception in the protocol changelog
  The protocol-15 changelog currently says native validation reaches the same verdicts as the `jsonschema` crate, but `is_unique` intentionally changes the verdict for signed-zero pairs in arrays longer than 15 elements. The PR Breaking Changes section documents this exception, but the authoritative `PLATFORM_V15` release note does not. Update this numbered changelog item to state the signed-zero uniqueness exception and qualify the claim of equivalent verdicts.

In `packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/validate/mod.rs`:
- [SUGGESTION] packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/validate/mod.rs:494-498: Skip uniqueness work after a match-only walk has failed
  A match-only walk can already be marked failed by `contains`, `minItems`, or `maxItems`, but the code still evaluates `is_unique(items)`. For an oversized candidate with `uniqueItems: true`, this allocates, hashes every element, and sorts the result even though the candidate cannot match. Guarding the uniqueness check with `!self.done()` preserves ordinary diagnostic validation while avoiding this unnecessary work in boolean-only `contains` checks.
- [SUGGESTION] packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/validate/mod.rs:363-383: Preserve structured error metadata, not just first error codes
  (existing thread: https://github.com/dashpay/platform/pull/5533#discussion_r4351193770)
  The native `additionalProperties: false` branch still does not preserve the pinned validator's structured parameters when the schema has no `properties` keyword. In that case, the pinned validator selects `AdditionalPropertiesFalseValidator`, whose `FalseSchema` conversion reports keyword `additionalProperties` with empty parameters. The native branch instead enumerates the unexpected keys and reports `{ "additionalProperties": ["extra"] }` for a schema such as `{ "additionalProperties": false }` and document `{ "extra": 1 }`. The verdict and error code agree, but public `JsonSchemaError` metadata differs. Add a differential case for absent versus empty `properties` and preserve the pinned distinction.

[thepastaclaw]

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

The protocol-15 native validator is correctly dispatched and fails closed for unsupported schemas, and the previously reported validator correctness, caching, diagnostics, cycle, and documentation issues are addressed. Two suggestions remain: preserve the public protocol-14 typed-schema builder behavior, and isolate generation 0 in its own versioned module; the intentional signed-zero rule also still lacks regression coverage on both sides of the 15-element threshold. Rust workspace tests and several image/build checks were still in progress for this exact head.

🟡 3 suggestion(s)

2 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 to consensus-critical document validation and protocol-version dispatch, directly altering which documents 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 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:384-389: Keep validation-schema generation 0 in a dedicated v0 module
  `ValidationSchema::build` now has the correct fail-closed dispatch, but the generation-0 implementation remains in the dispatcher module as `try_from_root_v0`. The repository's versioning convention isolates each generation under its own `vN` implementation module so later generations cannot accidentally edit the implementation selected by an existing protocol version. Move the generation-0 implementation and generation-specific tests/helpers under `validation_schema/v0/`, leaving `mod.rs` as the dispatcher.
- [SUGGESTION] packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/mod.rs:452-459: Preserve the protocol-14 public schema-builder behavior
  (existing thread: https://github.com/dashpay/platform/pull/5533#discussion_r4366599634)
  `document_type_validation_schema` is a public API that accepted protocol 14 in the v5.1-dev base: protocol 14 selected the document parser's `validate_max_bytes` generation, and the method directly built and returned a `ValidationSchema`. The new implementation routes this method through `ValidationSchema::build`, which rejects protocol 14 because its `validation_schema` entry is `None`. This does not affect protocol-14 consensus document validation, but it is an observable API regression for callers that use the typed schema independently. Keep the legacy public builder path when no native-validator generation is selected, or make this method non-public if refusing protocol 14 is intentional and document the API break explicitly.

In `packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/validate/tests.rs`:
- [SUGGESTION] packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/validate/tests.rs:91-121: Pin the intentional signed-zero rule with regression tests
  (existing thread: https://github.com/dashpay/platform/pull/5533#discussion_r4358271036)
  Protocol 15 intentionally treats `0.0` and `-0.0` as equal for `uniqueItems`, including arrays longer than 15 elements, and the implementation normalizes signed zero in the long-array hash path. The current tests cover `1` versus `1.0`, object ordering, and ordinary duplicates, but no signed-zero case. Add explicit short-array coverage such as `[0.0, -0.0]`, where native and legacy validation both reject, and long-array coverage containing that pair, where protocol-15 native validation rejects while the legacy validator accepts. This pins the documented consensus rule and the 15-element boundary.

[github-actions]

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.

[thepastaclaw]

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

The protocol-15 native validator is correctly selected and the generation-0 builder is now isolated in its own module. The remaining actionable issue is missing regression coverage for the intentional signed-zero uniqueness rule; two additional non-blocking version-isolation concerns remain around sharing generation-1 checks and caching the native validator without its selected generation. CI checks covering the Rust workspace and changed images were still in progress at the evidence snapshot, with no failed code check reported.

🟡 3 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 to consensus-critical document validation and protocol-version-gated acceptance behavior, specifically in the native validator and document validation dispatch.
• 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/methods/validate_document/v1/mod.rs`:
- [SUGGESTION] packages/rs-dpp/src/data_contract/methods/validate_document/v1/mod.rs:79-90: Keep generation-1 document checks isolated from generation 2
  Generation 2 now calls `validate_document_properties_v1_checks` defined in the generation-1 module. Although the current extraction preserves protocol-14 behavior, a future correction to the helper for generation 1 would also change protocol-15 behavior without a generation-2 version-table change. Keep the frozen checks duplicated in each generation, or move only genuinely version-neutral logic to a separately owned helper whose versioning contract is explicit.

In `packages/rs-dpp/src/data_contract/document_type/validator.rs`:
- [SUGGESTION] packages/rs-dpp/src/data_contract/document_type/validator.rs:16-23: Make the cached native validator version-aware
  The native-validator cache stores only one `Result<DocumentSchemaValidator, DataContractError>` and does not record the `validation_schema` generation used to build it. `native_validator` returns that cached result after the first initialization regardless of the later `platform_version`; if a later protocol selects generation 1 while reusing this validation path, an existing document type can retain the generation-0 validator or refusal. Key the cache by the selected generation or provide generation-specific slots, and cover validation across a protocol-version transition.

In `packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/validate/tests.rs`:
- [SUGGESTION] packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/validate/tests.rs:91-121: Pin the intentional signed-zero rule with regression tests
  (existing thread: https://github.com/dashpay/platform/pull/5533#discussion_r4358271036)
  The implementation and protocol-15 documentation intentionally treat `0.0` and `-0.0` as equal for `uniqueItems`, including arrays longer than the 15-element pairwise threshold. The current tests cover ordinary numeric and object cases on both sides of that threshold but contain no signed-zero pair. Add native-only assertions for `[0.0, -0.0]` and a 16-element array containing that pair; the long-array case must not use `agreed_verdict`, because the documented protocol-15 behavior intentionally differs from the legacy validator there.

[github-actions]

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]

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

The protocol-15 native validator is generally fail-closed and correctly selected through the version tables, but two consensus-relevant defects remain: shadowed $id entries can cause false protocol-15 refusal, and the native validation walk has no independent frozen generation. The signed-zero exception and an invalid-object additional-property scan also remain insufficiently covered or optimized; CI checks for the changed Rust code were still in progress at the reviewed head.

🔴 2 blocking | 🟡 3 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: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); 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-astra (agent: phase2-reviewer, role: general); reviewer 9: gpt-6-astra (agent: phase2-reviewer, role: architecture-layering); reviewer 10: gpt-6-astra (agent: phase2-reviewer, role: platform-versioning); reviewer 11: gpt-6-astra (agent: phase2-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) — This large, intricate diff changes protocol-version-gated document validation rules that directly affect consensus acceptance, making it a critical consensus surface in the native validator and validate_document implementations.
• 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, gpt-6-astra — general (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — architecture-layering (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
• 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/schema/validation_schema/v0/mod.rs`:
- [BLOCKING] packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/v0/mod.rs:237-255: Apply last-wins semantics when scanning shadowed `$id` entries
  The builder's effective schema representation uses last-wins semantics for duplicate map keys, but `refuse_identifiers_that_are_not_plain_names` scans every raw entry and rejects the first non-plain `$id`. A nested schema containing a non-plain `$id` followed by a plain `$id` is therefore accepted by the protocol-14 validator after JSON conversion, while protocol 15 refuses the document type as an invalid contract even though the effective schema contains only the final plain value. Scan effective last-wins entries when checking identifiers and add a regression covering a shadowed nested `$id`.

In `packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/validate/mod.rs`:
- [BLOCKING] packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/validate/mod.rs:147-155: Version the native validator walk independently of the builder
  `DocumentSchemaValidator::validate` contains the complete protocol-15 acceptance walk, but it has no platform-version parameter or generation dispatch. The cache slots only distinguish `validation_schema` builder generations; editing this walk later would change protocol-15 replay behavior without changing the selected builder generation or any validator version table. Put the walk behind a frozen generation-specific implementation and dispatch it through a validator-generation entry, or place the current implementation in an explicitly immutable generation module before further behavior changes are added.
- [SUGGESTION] packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/validate/mod.rs:421-425: Index declared property names for additionalProperties checks
  (existing thread: https://github.com/dashpay/platform/pull/5533#discussion_r4351983576)
  The declared-key count now avoids the membership scan for valid objects, but when an undeclared key exists the diagnostic still checks every instance key against the entire declared-property vector. Large invalid objects therefore retain an O(keys × declarations) path. Cache declared-property membership while retaining the existing vector for traversal order, and use the cache when constructing the unexpected-key diagnostic.

In `packages/rs-dpp/src/data_contract/methods/validate_document/v1/mod.rs`:
- [SUGGESTION] packages/rs-dpp/src/data_contract/methods/validate_document/v1/mod.rs:85-90: Keep generation-1 document checks isolated from generation 2
  (existing thread: https://github.com/dashpay/platform/pull/5533#discussion_r4367189684)
  Generation 2 invokes `validate_document_properties_v1_checks`, which is physically defined in the generation-1 module. The shared helper currently preserves both generations' behavior, but the module boundary does not enforce that a future correction to generation 1 cannot also change protocol 15. The repository's versioning guidance specifically requires a versioned method when shared behavior is reached from multiple generations; move genuinely version-neutral plumbing into an explicitly versioned shared helper, or give generation 2 its own frozen implementation.

In `packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/validate/tests.rs`:
- [SUGGESTION] packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/validate/tests.rs:99-123: Pin the intentional signed-zero rule with regression tests
  (existing thread: https://github.com/dashpay/platform/pull/5533#discussion_r4358271036)
  Protocol 15 intentionally treats `0.0` and `-0.0` as one `uniqueItems` value at every length, including the hash-based path above 15 elements. The current cases cover `1` versus `1.0`, object ordering, and repeated strings, but no signed-zero pair appears on either side of the threshold. Add explicit short and 16-element cases, asserting the native verdict and documenting the intentional long-array divergence from the legacy validator.

[github-actions]

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.

[thepastaclaw]

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

The protocol-15 native validator is correctly dispatched through generation-specific builder and validator paths, and the prior consensus, cache, duplicate-key, reference-cycle, and structured-diagnostic concerns are resolved. Three non-blocking issues remain: generation 2 reuses a generation-1-owned orchestration helper, the intentional signed-zero rule lacks threshold regression coverage, and schema key normalization performs avoidable quadratic work. Rust workspace tests and several image/build checks were still in progress at the captured head, with no failed code check reported.

🟡 3 suggestion(s)

2 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 to consensus-critical document validation rules activated at protocol version 15, specifically in the validation schema builder and document validation paths.
• 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/v0/mod.rs`:
- [SUGGESTION] packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/v0/mod.rs:269-280: Avoid quadratic work when normalizing schema map keys
  `last_wins` scans the accumulated output vector with `find` for every input entry, making duplicate-key normalization O(n²). The function is used repeatedly while building schemas and while recursively scanning `$id` values, and the schema is supplied by an untrusted contract owner while the native build is lazy. A size-bounded schema with many entries can therefore impose avoidable CPU work on the first protocol-15 document validation. Preserve first-key order and last-value semantics with an index keyed by the schema name.

In `packages/rs-dpp/src/data_contract/methods/validate_document/v1/mod.rs`:
- [SUGGESTION] packages/rs-dpp/src/data_contract/methods/validate_document/v1/mod.rs:85-90: Keep generation-1 document checks isolated from generation 2
  (existing thread: https://github.com/dashpay/platform/pull/5533#discussion_r4367189684)
  Generation 2 calls `validate_document_properties_v1_checks`, whose implementation is physically owned by the generation-1 module. The helper contains consensus-visible orchestration and is reached by both protocol 14 and protocol 15, but has no independently versioned method slot. A future correction or generation-specific behavior change cannot be made to one protocol without either editing the shared implementation or adding an ad hoc split, weakening the frozen-generation boundary. Keep the frozen checks independently owned by each generation, or move them behind an explicitly versioned shared helper with its own table entry.

In `packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/validate/v0/tests.rs`:
- [SUGGESTION] packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/validate/v0/tests.rs:93-123: Pin the intentional signed-zero rule with regression tests
  (existing thread: https://github.com/dashpay/platform/pull/5533#discussion_r4358271036)
  Protocol 15 intentionally treats `0.0` and `-0.0` as one value in `uniqueItems` arrays at every length, but the fixed cases cover ordinary numeric equality and duplicates without exercising signed zero. Add explicit short and long cases containing both representations, such as `[0.0, -0.0]` and a 16-element array with those values plus fillers. This pins both sides of the pairwise threshold and prevents a later equality, hashing, or threshold change from silently altering the documented protocol-15 rule.

[github-actions]

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.

[thepastaclaw]

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

The protocol-15 native validator is correctly isolated from the legacy validation path and the recent fixes preserve last-wins schema semantics, generation-aware caching, and native-walk dispatch. Three nonblocking issues remain: the public schema-builder API regresses for protocol 14, generation-2 document checks still depend directly on a generation-1-owned helper, and the intentional signed-zero uniqueness rule lacks focused regression coverage. Rust workspace tests and several build jobs were still in progress in the supplied CI snapshot.

🟡 3 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: 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 to document validation that directly changes protocol-versioned consensus rules in the native validation path, specifically the ValidationSchema validator and document validation dispatch.
• 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:380-387: Preserve the protocol-14 public schema-builder behavior
  (existing thread: https://github.com/dashpay/platform/pull/5533#discussion_r4366599634)
  `document_type_validation_schema` now always routes through `ValidationSchema::build`, whose `builder_generation` gate rejects protocol versions that select `validation_schema: None`. Protocol 14 selects no native builder generation, so this public method now returns `NotSupported` at protocol 14. At the review base, the same method accepted protocol 14 because its gate used `validate_max_bytes: Some(0)`, which protocol 14 selected. The fact that the v5.1 API is unreleased does not make this an unchanged behavior within the base branch. Keep the protocol-14-compatible public construction path separate from the protocol-15 native-validator activation gate, or explicitly version and document this API break.

In `packages/rs-dpp/src/data_contract/methods/validate_document/v1/mod.rs`:
- [SUGGESTION] packages/rs-dpp/src/data_contract/methods/validate_document/v1/mod.rs:85-90: Keep generation-1 document checks isolated from generation 2
  (existing thread: https://github.com/dashpay/platform/pull/5533#discussion_r4367189684)
  Generation 2 calls `validate_document_properties_v1_checks`, whose implementation is defined in the generation-1 module. The current extraction preserves both generations' behavior, but the repository's versioning guidance specifically requires shared helpers reached from multiple generations to have a versioned method. An in-place correction or refactor made for protocol 14 could therefore silently alter protocol 15, and a future protocol-15-only correction cannot be isolated at the helper boundary. Move genuinely version-neutral logic behind an explicitly versioned helper, or duplicate the frozen checks into the generation-2 implementation.

In `packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/validate/v0/tests.rs`:
- [SUGGESTION] packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/validate/v0/tests.rs:92-123: Pin the intentional signed-zero rule with regression tests
  (existing thread: https://github.com/dashpay/platform/pull/5533#discussion_r4358271036)
  The protocol-15 implementation intentionally treats `0.0` and `-0.0` as equal in the long-array hash path, while the current tests cover only ordinary numeric distinctions such as `1` versus `1.0` and reordered objects. Add explicit signed-zero cases below and above `UNIQUE_ITEMS_PAIRWISE_LIMIT`, such as a short `[0.0, -0.0]` array and a 16-element array containing that pair. This prevents a future change to `hash_json` or equality semantics from silently removing the protocol-15 rule.

[github-actions]

@coderabbitai review

No review for 7adc0926 yet, so PR Hygiene is asking once. If nothing arrives, the requirement is dropped for this commit and the pull request is labelled bot-review-skipped.

[github-actions]

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.

[thepastaclaw]

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

The protocol-15 native validator is correctly gated, generation-dispatched, fail-closed for unsupported schemas, and the prior blocking correctness findings are resolved. Three maintenance and test-coverage issues remain: generation-2 document checks and builder tests are still coupled to unversioned or shipped-generation code, and signed-zero behavior is not tested through protocol-level document dispatch; two additional non-blocking versioning/performance concerns are also present. The Rust workspace test check failed in unchanged rs-sdk code according to the supplied CI evidence, while several image and JS checks remain in progress.

🟡 6 suggestion(s)

1 finding(s) not shown inline (the lines are not part of this PR's diff)

🟡 Suggestion: Keep validation-schema generation 0 in a dedicated v0 module
packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/tests.rs:17-19

The generation-0 builder implementation is isolated in validation_schema/v0, but the generation-specific builder tests and helpers remain in the parent module. The build helper calls ValidationSchema::try_from_root directly, bypassing ValidationSchema::build and its version dispatcher, so these tests do not protect the frozen generation through the public dispatch boundary and can be reused accidentally by a future generation. Move v0-only helpers and tests beside v0/mod.rs, retaining only cross-generation or dispatcher-level tests in the parent module.

_{source: gpt-6.1-sol (phase2-reviewer: general, architecture-layering, platform-versioning, rust-quality, security-auditor)}

2 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 validation behavior and protocol-version dispatch in the native validator and validation paths.
• 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/tests.rs`:
- [SUGGESTION] packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/tests.rs:17-19: Keep validation-schema generation 0 in a dedicated v0 module
  The generation-0 builder implementation is isolated in `validation_schema/v0`, but the generation-specific builder tests and helpers remain in the parent module. The `build` helper calls `ValidationSchema::try_from_root` directly, bypassing `ValidationSchema::build` and its version dispatcher, so these tests do not protect the frozen generation through the public dispatch boundary and can be reused accidentally by a future generation. Move v0-only helpers and tests beside `v0/mod.rs`, retaining only cross-generation or dispatcher-level tests in the parent module.

In `packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/validate/v0/mod.rs`:
- [SUGGESTION] packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/validate/v0/mod.rs:416-424: Avoid quadratic membership scans for additional properties
  The lazy diagnostic closure avoids this work for successful objects and match-only `contains` walks, but an object with declared properties plus one unexpected key still scans the declared-property vector for every instance key. That makes the additional-properties diagnostic O(instance keys × declared properties) on every real validation failure. Cache a membership set with the immutable schema properties, or otherwise use an indexed lookup while retaining the declaration vector for traversal and diagnostic order.

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:312-315: Document the public FormatConstraints type change
  `FormatConstraints.format` was publicly exposed as `Option<String>` at the base revision and is now `Option<ValidatorFormat>`. Downstream Rust callers that construct this field or use string-oriented methods such as `as_deref` no longer compile. The PR documents protocol-level breaking rules but does not identify this public source-level API change or its release and semver treatment.

In `packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/v0/mod.rs`:
- [SUGGESTION] packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/v0/mod.rs:752-764: Freeze the format vocabulary used by validation-schema generation 0
  Generation 0 parses formats through the shared `ValidatorFormat` enum. Adding a format variant for a later native-validation generation would automatically make the protocol-15 generation-0 builder accept that format, changing protocol-15 schema admission without a protocol-table change. Keep the accepted format parser generation-local or dispatch format parsing through an explicitly versioned entry.

In `packages/rs-dpp/src/data_contract/methods/validate_document/v1/mod.rs`:
- [SUGGESTION] packages/rs-dpp/src/data_contract/methods/validate_document/v1/mod.rs:85-90: Keep generation-1 document checks isolated from generation 2
  (existing thread: https://github.com/dashpay/platform/pull/5533#discussion_r4367189684)
  Generation 2 calls `validate_document_properties_v1_checks`, which is defined in the generation-1 module and contains behavior-bearing consensus checks. The repository's versioning guidance requires shared helpers reached by multiple generations to have their own versioned method; otherwise a future edit intended for one generation can silently change the other. Keep the frozen generation-1 helper private to generation 1, or move only explicitly version-neutral logic behind a separately versioned helper with its own table selection.

In `packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/validate/v0/tests.rs`:
- [SUGGESTION] packages/rs-dpp/src/data_contract/document_type/schema/validation_schema/validate/v0/tests.rs:295-306: Pin the intentional signed-zero rule with regression tests
  (existing thread: https://github.com/dashpay/platform/pull/5533#discussion_r4358271036)
  The test directly invokes `DocumentSchemaValidator` and therefore proves the generation-0 walk, but it does not validate the same long signed-zero array through `DataContract::validate_document_properties` at protocol 14 and protocol 15. The existing integration test checks native-cache activation only with ordinary documents, so it could still pass if the protocol-15 path returned the legacy result for this edge case. Add a protocol-level case asserting the legacy protocol-14 verdict and the intentional protocol-15 refusal, while retaining a distinct long array as an acceptance control.

[github-actions]

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]

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.