Repository navigation
refactor(dpp)!: keep the property type shorthand expansion out of shipped generations - #5357
Conversation
…pped generations Follow-ups to the review of the identifier/bytes shorthands: - The contract update comparison now rewrites shorthands inside validate_schema_compatibility 1 (protocol version 14 only), for the document type diff and the $defs diff alike; the validate_update helpers shared with generation 0 are back to their earlier code. - create_document_types_from_document_schemas 1 (protocol versions 2 to 14) no longer expands the contract's $defs; parser generation 3 already does for each document type. - Docs: the wasm-dpp2 typed-array typing, the DocumentType::schema() doc and the typed-arrays book chapter were wrong for shorthands; v14 note 91 names the new place of the update comparison's rewrite. A valid contract parses and compares as before. At protocol version 14 a contract with both a `-` in its first document type's name and a malformed shorthand definition is now refused for the name first. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 37 minutes. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (10)
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 |
|
📖 Book Preview built successfully. Download the preview from the workflow artifacts. Updated at 2026-10-09T12:41:04.113Z |
|
Waiting for bot review — coderabbitai not yet · thepastaclaw not yet. Wait for the missing reviews, or a writer can post |
|
🕓 Review not started yet because the new head is waiting for the 30-minute push debounce.
Commit e113bf4. Normal review starts when eligible; priority review starts as soon as a slot is available. |
Basic explanation
Follow-ups to #5355 (the
"type": "identifier"and"type": "bytes"shorthands) from its review, which landed after it merged. Nothing changes for a valid contract. The shorthand rewrite now lives only in code that protocol version 14 alone selects, instead of being added to code that also runs for protocol versions 1 to 13, and three documentation statements that were wrong for shorthands are corrected.Issue being fixed or feature implemented
The review of #5355 found three blocking defects, fixed in that PR before it merged, and a set of non-blocking ones that it never got to. This PR takes the ones worth fixing:
validate_updategeneration 0, andcreate_document_types_from_document_schemasgeneration 1 (protocol versions 2 to 14). Both edits were inert before 14, but each needed a written argument that it was, and the second is the code that briefly made stored token-only contracts unreadable at 14 (fixed in feat(dpp)!: add identifier and bytes property type shorthands (PV14) #5355). The repository rule is that a shipped generation is edited only when that cannot be avoided.DocumentType::schema()doc, and the typed-arrays book chapter.What was done?
The update comparison expands inside
validate_schema_compatibility1. That generation is selected only by protocol version 14, and both comparisons go through it: the document type schema diff and the{"$defs": ..}diff. It now rewrites both sides (with_shorthands_expanded: JSON toValue, expand, back to JSON, only when a shorthand is present) before its existingprepared_for_diff. The two shared helpers (DocumentTypeRef::validate_schema_with_options,DataContract::validate_update_schema_defs) are back to their code before #5355.Before (in a helper shared with generation 0):
After (generation 1 only):
The contract-level
$defsexpansion increate_document_types_from_document_schemas1 is removed. Parser generation 3 already expands the contract's$defsfor each document type (it has to, sinceDataContract::set_document_schemahands it the raw definitions), so the contract-level copy only saved one clone per document type, and only when a definition uses a shorthand. The one contract-level reader that saw the expanded definitions,validate_property_constraint_aggregates, reads onlyenum, which the rewrite never changes. A contract without document types still has its$defsleft as sent: no document type parse runs.Docs.
DocumentTypedArrayItembyteselement reports itssizeasminItems/maxItems; an identifier reports noneDocumentTypedArrayPropertycontract.toJSON()and the accessors "line up key for key"toJSON()shows the schema as sent;itemsgives the parsed kind and boundsDocumentTypeV0Getters::schemaexpand_property_type_shorthandsof this schemaschema_defs()may hold shorthands too; a raw read that follows a$refexpands the enriched root, which rewritespropertiesand$defstogethersizesize(on abyteselement) addedv14 note 91 now names
validate_schema_compatibility1 as the update comparison's rewrite and mentions the check_tx depth pre-check, which #5355 made walk the rewritten schema.In-place changes to shipped generations
None added. This PR removes the two that #5355 made:
validate_updatehelpers shared with generation 0 (validate_schema_with_options,validate_update_schema_defs)create_document_types_from_document_schemas1validate_schema_compatibility1 and parser generation 3 are selected only by protocol version 14, which is unreleased.How Has This Been Tested?
validate_schema_compatibility/v1:should_compare_a_shorthand_as_the_long_form_it_stands_for(a property and a definition rewritten between the spellings, both directions, no change) andshould_still_report_a_change_behind_a_shorthand(an identifier definition turned intobytesis reported at/$defs/owner/contentMediaType).DataContract::validate_update, protocol version 13 judging updates as before, contracts registered at 13 with unread shorthand-shaped values reading back at 14, a shorthand and its long form parsing to identical document types.add_version_items_to_all_contractsreads contracts stored at 13 that hold such values.cargo test -p dpp,cargo test -p platform-version,cargo test -p wasm-dpp2; targeteddriveanddrive-abcifilters;cargo clippy --all-features --all-targets -D warningson dpp, platform-version and wasm-dpp2;cargo fmt --all.Breaking Changes
None for any released protocol version: every edit is inert before protocol version 14, and two edits restore code that runs before 14 to its exact previous form.
At protocol version 14 (unreleased) a valid contract parses and compares exactly as before. The one observable difference is the order of two refusals: a malformed shorthand in the contract's
$defsused to be refused before any document type was parsed, and is now refused inside the first document type's parse, after that parse's check for a-in the type name. A contract that breaks both (a-in its first document type's name and a malformed shorthand definition) now getsInvalidDocumentTypeNameErrorinstead ofInvalidContractStructure(10231). Nodes running PV14 builds from before and after this change could therefore disagree on that one refusal, hence the!.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 ·
e113bf4/skip-botsproceeds without the ones not yet reported/self-reviewedWhen every merge requirement is met, the
PR Hygienecheck passes. Reviewer limits do not block merging; other required GitHub checks and protections still apply.