fix(dpp)!: refuse lookup key sources inside a transient object (PV14) - #4943
Conversation
The referring-side check of a refersTo lookup refused a transient key source by exact name, but transient_fields() holds the paths as declared: with `transient: ["meta"]`, a key reading "meta.charterId" passed, although Drive strips the whole object before storage and no reader could rebuild the key. Check the path and every dotted prefix. The check runs on every parse of generation 3, so it must be tightened before protocol version 14 ships; afterwards it would need a gate. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
github-actions
Bot
added
the
waiting-bots
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: dashpay/platform/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughLookup validation now rejects key sources that are transient or inside a transient object. The documentation and schema description state this requirement. Tests cover stored and transient parent objects. ChangesTransient lookup paths
Priority: ⚪ Not assessed Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to No actionable issue remains identified for this change; it is mergeable after normal checks. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
|
📖 Book Preview built successfully. Download the preview from the workflow artifacts. Updated at 2026-09-23T13:21:36.110Z |
|
🕓 Queued for automated review — 6th in line, estimated start in ~35 min (commit 8fab81f)
|
…ment Merges anyOf/allOf (#4942) and the transient-object fixes (#4943, #4948), and reworks listElement to the shape Sam proposed: "refersTo": { "type": "listElement", "documentType": "electedCharter", "propertyAgreement": { "electedCharterId": "$id" }, "inList": "members" } - The list's document is found by the agreement pair with `$id` on the referenced side (exactly one, read from a stored identifier property, never $ownerId); `documentProperty`/`list` are gone. `$id` joins $ownerId and $creatorId as a referenced-side agreement name for every document reference. - A list element is a document reference: contractId allowed, `as_any_document_reference` carries it with `in_list`, so registration checks its contract, type and pairs through the shared code; the $id property needs no refersTo of its own. - Write time: the document is fetched by id through a per-write memo shared with every by-id reference (one fetch for the charter and its list elements); lists are collected once into a set. Replace triggers are the agreement's (binds_a_changed_property). - listElement is a combinable leaf of anyOf/allOf (target variant 9). - Tests, fixtures, meta-schema, changelog item 34, book and wasm-dpp2 updated. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2 participants
Issue being fixed or feature implemented
A
refersTolookup (#4930, protocol version 14, unreleased) assembles its key from properties of the referring document. The referring-side check requires each property it reads to be stored, so that a reader can rebuild the same key from the stored document. It refused a transient key source withdeclaring.transient_fields().contains(path).transient_fields()holds the paths as declared, so a source nested in a transient object passed:transient: ["meta"]does not contain"meta.charterId".Drive strips transient values before storage (
data.retain(|key, _| !transient_fields.contains(key))in the document create action), so the wholemetaobject is gone from the stored document. No reader could reassemble the key. Consensus was never at risk, because writes are judged on the transition's data, not the stored document.It has to be fixed before 4.2 ships.
validate_reference_lookup_sourcesruns on every parse of generation 3, not only under full validation. After protocol version 14 is live on mainnet, tightening it would make stored contracts that use this shape unparseable, so the fix would need a version gate. Today it is an in-place edit of an unreleased generation.What was done?
DocumentReferenceLookup::referring_side_errornow checks the source path and every dotted prefix of it againsttransient_fields(), through a privateis_transient(document_type, path)helper inreference_lookup.rs. The helper is identical to the one #4940 (still open) adds inlist_element_reference.rs. Whichever PR lands second should drop its copy and import the other.lookupdescription in meta-schema v3 (description text only, no validation change) and the lookup section ofbook/src/data-model/documents.mdnow state the nested case.reference_lookup.rsthat had come loose fromindex_property_value_kindand was sitting onowner_can_change.Before (v4.2-dev), this
electedCharterschema registered. Its lookup readsmeta.charterId, andmetais required and transient:After, every parse refuses it, with or without full validation:
The same schema without
"transient": ["meta"]still registers.No system contract or fixture uses this shape. Among JSON schemas, only DPNS declares
transient(preorderSalt), and DPNS declares no lookup. None of the four lookup fixtures inrs-drive-abci/tests/supporting_files/contract/reference-validation/declarestransient. The open contract PRs (#4898, #4933, #4940, #4941, #4942) don't declare it either.In-place changes to shipped generations
None.
referring_side_erroris only called fromvalidate_reference_lookup_sources, which only parser generation 3 (try_from_schema: 3) calls. That generation is selected only byCONTRACT_VERSIONS_V6, which only protocol version 14 uses, and 14 is unreleased. Earlier generations never parse a lookup.How Has This Been Tested?
should_refuse_a_lookup_source_inside_a_transient_required_objectintry_from_schema/v3/reference_lookup_tests.rs. It shows the schema above registers withouttransient, and is refused withtransient: ["meta"]on both a validating and a non-validating parse.expect_errpanics because the contract registers.cargo test -p dpp --all-features --lib -- reference_lookup: 29 passed.cargo test -p dpp --all-features --lib -- meta_schema: 11 passed (the v3 meta-schema description changed).cargo clippy -p dpp --all-features --tests -- -D warnings: clean.cargo fmt --all -- --check: clean.Breaking Changes
Consensus-breaking at protocol version 14 only, which is unreleased. A contract whose lookup reads a property inside a transient object is now refused on every parse, so nodes running protocol version 14 (devnets) must run the same binary. A devnet that already registered such a contract could not parse it after upgrading. No released protocol version changes.
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 ·
8fab81f/skip-botsproceeds without the ones not yet reported/self-reviewedonce the bots are doneWhen every box is checked the
PR Hygienecheck passes and this can merge.Summary by CodeRabbit