fix(dpp)!: typed array review fixes: hyphenated list paths, element constraints, untrusted lists, Swift refusal - #4924
Merged
Conversation
…onstraints, untrusted lists, Swift refusal Follow-up to #4922 from its review. - platform-value: `is_array_path` accepts `-` in a list name, as a document property name may carry it; `member-ids[]` was never converted before. One `replace_leaf` serves the Value and the map path replacers, so a `Bytes32` member replaced as binary bytes keeps its kind on both. - `TypedArrayProperty::item_constraints` carries the `enum`, `minimum` and `maximum` of the items schema, parsed on both paths with the shape rules the meta-schema now states for elements (member types, no enum on byte array or identifier elements, minimum not above maximum); random document generation draws enum members and bounded numbers, so random documents validate against schemas with such elements. - `ExtendedDocument::set_untrusted` converts the members of a list set at a `path[]` path, and reads binary values as binary. - Census of every contract create and update transition on mainnet (72) and testnet (4593), decoded with dpp: no `uniqueItems` anywhere, so the PV14 refusal on identifiers stays; recorded on the test that pins it. - wasm-dpp2 reports an element's `enum`, `minimum` and `maximum`. - The Swift DataContractParser refuses a typed array explicitly instead of persisting it as a bare array. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Contributor
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Repository: dashpay/platform/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (17)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Contributor
|
📖 Book Preview built successfully. Download the preview from the workflow artifacts. Updated at 2026-09-22T23:29:31.236Z |
github-actions
Bot
added
the
waiting-bots
Collaborator
|
|
…pe) into fix/typed-array-review-fixes The element constraints now sit on the boxed scalar item type and random integer elements come back in the element type's own value kind.
…from PV14 Instead of teaching the path syntax the `-` it was never written for, the name rule is tightened: meta-schema v3 refuses `-` in a property name (top level, nested, and in the property paths of refersTo declarations), and generation 3 of the document type parser refuses it in a document type name, both under full validation. Every earlier version admitted the character; a census of every data contract create and update transition on mainnet (72) and testnet (4593) found no name carrying one, and neither does any repository fixture, so nothing stored is affected. Stored contracts are read as they are; protocol version 13 is unchanged. The `is_array_path` widening is reverted accordingly. v14 changelog item 26, book note, and `name_rules_tests` pinning 14 refuses / 13 admits. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Keeps the distinctFrom changelog entry as item 26 and moves the word-characters-only name rule to item 27. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This was referenced Sep 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue being fixed or feature implemented
Follow-up to #4922 (typed scalar arrays, PV14): the fixes from its post-merge review. The review found one functional defect on the client conversion path (settled by tightening the name rule instead, see below), two half-supported item keywords, a blind spot in
ExtendedDocument::set_untrusted, an unverified compatibility claim, a duplicated helper, a test gap and a conventions nit.Merged over #4923 (elements encoded as their scalar property type), #4887 and #4925: the element constraints below sit on the boxed scalar
item_type, and random integer elements are produced in the element type's own value kind (aU8element yieldsValue::U8), which is what the codec reads back. #4925's integer width guard and this branch touch the property module in different places and merge cleanly.What was done?
Property and document type names are word characters only from protocol version 14
The review found that
is_array_pathin platform-value accepted only alphanumeric and_in a list name while the meta-schema admitted-in a property name, so a hyphenated typed array (member-ids[]) never had its elements converted. Rather than teach the path syntax a character it was never written for, the name rule is tightened: meta-schema v3 refuses-in property names (top-level and nested, and in the property paths ofrefersTodeclarations), and generation 3 of the document type parser refuses it in a document type name, both under full validation. A census of every data contract create and update transition on mainnet (72) and testnet (4593), decoded from the raw bytes with dpp, found no property or document type name carrying-, so no stored contract is affected and none is left unable to update. Stored contracts are read as they are; protocol version 13 keeps admitting the character.Before: registered at protocol version 14; a document with
"member-ids": ["9k3RE6kH..."]built from JSON kept the string as text (the conversion pathmember-ids[]was never recognized) and validation refused it.After: registration at protocol version 14 refuses the contract,
JsonSchemaErrorfor the property name andInvalidDocumentTypeNameErrorfor the type name;memberIdsandmember_idsregister as before. At protocol version 13 both names are still admitted.Item
enum,minimumandmaximumare parsed and honoredTypedArrayPropertygainsitem_constraints: ArrayItemConstraints { allowed_values, minimum, maximum }, read from theitemsschema byparse_typed_arrayv0 next to the scalar element parse, on both parse paths. Meta-schema v3'sdocumentArrayItemgains matching member-type rules. (The scalar parse already sizes an integer element from these bounds; the constraints are what keeps a randomU8element under amaximumof 100.)Before:
random_documentdrewtagsfrom random alphanumeric strings (["4Dcej73k5Ap5tkyTtz"]) andscoresfrom the wholeu8range ([200, 17]), andvalidate_documentrefused both, so fixtures and strategy tests could not use such a type.After:
tagsis drawn from the set (["abuse", "spam"], distinct underuniqueItems; the shortest member at the minimum fill, the longest at the maximum) andscoreswithin the bounds ([57, 82],0at the minimum fill,100at the maximum), and the documents validate and round-trip.Contract registration, on both parse paths, now also refuses schemas no element could satisfy:
{ "type": "integer", "enum": ["a"] } // every enum member of a typed array's elements must be a integer value, found "a" { "type": "string", "enum": [] } // the enum ... must hold at least one value { "type": "array", "byteArray": true, "minItems": 32, "maxItems": 32, "contentMediaType": "application/x.dash.dpp.identifier", "enum": [[1]] } // enum is not supported on byte array or identifier elements { "type": "integer", "minimum": 5, "maximum": 4 } // the minimum ... may not exceed their maximum { "type": "number", "minimum": "low" } // the minimum ... must be a number value, found "low"Before: all five registered; the first four could never hold a document, and the last was ignored.
ExtendedDocument::set_untrustedconverts the members of a listBefore:
reasonsmatched neitheridentifier_paths(which holdsreasons[]) norbinary_paths, so both lists were stored asValue::Textelements and the document failed validation. The binary branch also read a scalar binary value withto_identifier_bytes, so a base64 string on a binary path was refused.After:
reasonsholdsValue::Identifierelements,digestsholdsValue::Bytes(vec![1, 2, 3, 4]), a non-list set at a list path is an error, and a scalar binary path reads base64 as binary.One leaf replacement helper
Value::replace_at_pathnow uses the samereplace_leafas the map replacer.Before:
Bytes32([8; 32])becameBytes(vec![8; 32])on the value path and stayedBytes32on the map path, so a document converted throughExtendedDocument::from_untrusted_platform_value, which runs both, ended with whichever ran last.After:
Bytes32on both paths.uniqueItemson identifiers: censusThe review flagged the PV14 refusal of
uniqueItemson identifier properties as unverified against live contracts. A census of every data contract create and update transition on mainnet (72) and testnet (4593), pulled from the pshenmic explorers and decoded from the raw bytes with dpp, found nouniqueItemson any property of any contract, anditemsonly on 22 testnet creates that were refused. The refusal stays; the census is recorded on the test that pins the rule.wasm-dpp2 reports the element constraints
Before:
{ path: 'scores', items: { type: 'integer' }, maxItems: 4, uniqueItems: false }After:
{ path: 'scores', items: { type: 'integer', minimum: 0, maximum: 100 }, maxItems: 4, uniqueItems: false }anditems: { type: 'string', minLength: 1, maxLength: 20, enum: ['spam', 'abuse', 'offTopic'] }forlabels; the TypeScript types and the spec are updated.Swift refuses typed arrays explicitly
Before:
parsePropertiespersisted the property as a barearrayrow (no element type), so a later document with that property was decoded with the byte-array codec.After: it throws
DataContractParser.ParseError.unsupportedTypedArray(documentType: "charter", property: "member-ids")with the message "typed arrays (an array property declared by an items schema) are not supported by the Swift SDK yet". Three tests inDataContractParserTypedArrayTests.swiftcover a string list, an identifier list and a plain byte array that still parses. Nested typed arrays are neither refused nor mis-parsed there, since the Swift parser keeps nested schemas as raw JSON. Kotlin needs nothing: the Kotlin SDK has no schema parser of its own, and the example app already treats an array withoutbyteArrayas a list of values.Smaller
IdentifierandEncodinginstead of spellingcrate::paths inline.How Has This Been Tested?
New tests (all names start with
should):rs-dpp,name_rules_tests.rs: a hyphen in a top-level or nested property name is refused at protocol version 14 and admitted at 13 (JsonSchemaError), a hyphen in a document type name is refused at 14 and admitted at 13 (InvalidDocumentTypeNameError), the stored path reads both as they are, and word-character names stay admitted.rs-dpp,typed_array_tests.rs:should_convert_the_elements_of_typed_arrays_when_creating_a_document_from_datanow covers lists nested in an object (team.leads[],team.digests[]) and validates the result;should_convert_the_members_of_a_typed_array_set_on_an_extended_document(identifier and byte array lists throughset_untrusted, a non-list refused);should_refuse_element_constraints_no_element_could_satisfy_on_both_paths(a wrong-typed or emptyenum, anenumon an identifier element, aminimumabove themaximum, a non-numericminimum, on the validating and the stored path); the charter fixture gains an enum list, a bounded integer list and a bounded number list, soshould_generate_random_documents_that_validate_against_their_own_schemanow proves random elements stay insideenum,minimumandmaximumat every fill size and round-trip in the element's own value kind;should_refuse_items_on_a_byte_array_and_unique_items_on_an_identifiercarries the census.rs-dpp,parse_typed_array/v0:should_parse_the_enum_minimum_and_maximum_of_the_elements,should_refuse_element_constraints_that_no_element_could_satisfy.rs-platform-value:should_keep_the_fixed_size_kind_of_a_replaced_member.swift-sdk:DataContractParserTypedArrayTests.swift(a string list and an identifier list refused with the new error and nothing persisted, a plain byte array still parses).Run locally:
cargo test -p dpp --all-features --lib: 4600 passed after the merge of feat(dpp)!: encode typed array elements as their scalar property type (PV14) #4923; the typed array, parse, property, extended document andvalidate_updatemodules again after merging feat(platform)!: a document batch proof carries the owner's credit balance #4887 and fix(dpp)!: contract updates may not change an integer property's width or signedness (PV14) #4925 (632 passed)cargo test -p platform-value --lib: 1053 passedcargo check -p drive -p drive-abci -p dash-platform-queries --all-targets,cargo check -p wasm-dpp2 --target wasm32-unknown-unknown,cargo check -p wasm-sdk --target wasm32-unknown-unknowncargo clippy -p dpp -p drive -p drive-abci -p platform-value -p platform-version --all-targets --all-features -- -D warnings,cargo clippy -p wasm-dpp2 --target wasm32-unknown-unknown -- -D warnings,cargo fmt --allyarn workspace @dashevo/wasm-dpp2 buildthenmocha tests/unit/DocumentTypedArrays.spec.ts: 6 passingSwiftDashSDKmodule type-checked and the parser test bundle run (44 tests, 3 new) in a scratch harness against an existing xcframework; this worktree has noDashSDKFFI.xcframework, soswift testand the SwiftExampleApp build were not rununiqueItems, and-in names): every contract create and update transition listed by the mainnet and testnet explorers (4665 rows, 0 decode failures) decoded with a throwawayrs-drivetest that was removed afterwardsBreaking Changes
Protocol version 14 (unreleased) only: contract registration now refuses
-in a property name (meta-schema v3) and in a document type name (parser generation 3), a character every earlier version admitted and no live contract uses; and it refuses, on a typed array'sitems, anenumwith no member or with a member of another type, anenumon a byte array or identifier element, a non-numericminimum/maximumon a number element, and aminimumabove themaximum. Every such schema was unsatisfiable or contradictory before.TypedArrayPropertygains theitem_constraintsfield.Checklist:
structure.rs, regeneratedgrovedb-structure.json, and checked the structure viewer link posted on this pull requestFor repository code-owners and collaborators only
🤖 Generated with Claude Code
PR Hygiene ·
88487cb/skip-botsproceeds without the ones not yet reported/self-reviewedonce the bots are donedpp— you own itswift-sdk(packages/swift-sdk/Sources/SwiftDashSDK/Core/Utils/DataContractParser.swift,packages/swift-sdk/SwiftTests/SwiftDashSDKTests/DataContractParserTypedArrayTests.swift) — llbartekll or romchornyiWhen every box is checked the
PR Hygienecheck passes and this can merge.Summary by CodeRabbit
enum,minimum, andmaximumconstraints. These bounds are also available through the JavaScript interface and are respected when generating sample values.