fix(platform)!: drop transient values on replace and refuse declarations that read them (PV14) - #4950
Merged
Merged
Conversation
…ons that read them (PV14) A transient property is judged on the transition and never stored, but: a replace stored whatever it carried (only a create dropped the values); an index, a lookup's index, a propertyAgreement's referenced side and a key reference's identity could all read a transient value no stored document holds; transient entries naming nested paths, system or unknown properties were accepted and not dropped; and changing the transient list on update failed as an unsupported keyword, an internal error. - document_from_replace_transition_action 1 drops transient values by top-level name, as the create action does - parser generation 3 (full validation) refuses a transient entry that is not a top-level property and an index reading a transient property - referenced_side_error refuses a lookup index keyed by one - data_contract_reference_validation 0 refuses a transient referenced agreement property and a stored key id paired with a transient identity - validate_schema_compatibility 1 freezes the transient list Every generation touched is only selected by, or only reached from, protocol version 14, which is unreleased. A census of all mainnet and testnet contracts found transient only on immutable DPNS-shaped domain types, which none of the new rules refuse. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
Warning Review limit reachedNext included review available in 59 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Repository: dashpay/platform/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (26)
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 |
github-actions
Bot
added
the
waiting-bots
Collaborator
|
🕓 Queued for automated review — 8th in line, estimated start in ~50 min (commit d17a1ae)
|
Contributor
|
📖 Book Preview built successfully. Download the preview from the workflow artifacts. Updated at 2026-09-23T15:56:14.211Z |
- the frozen transient list is compared as a set: sorted and deduplicated before the schema diff, so a reordering is no change - one drop_transient_values helper for the create action (pure refactor) and the replace conversion, which now drops before moving the data instead of cloning the transient set - end-to-end replace test through process_raw_state_transitions at protocol versions 14 and 13 - drive-abci fixtures for a transient object around an agreement's referenced property or a key reference's identity, and the keyIdProperty pair - inertness comments at the in-place edits of data_contract_reference_validation 0, a should_ test name, shared v3 test helpers, a hint only for dotted transient entries, and the update limitation documented at the parser check Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
A transient property is validated on the transition and not stored. An audit of how
transientis handled after #4943 and #4948 found seven gaps:propertyAgreementmay name a transient property on its referenced side. No stored document carries that value, so a referring type whose side is required can never be written.refersTolookup may go through an index keyed by a transient property, and can then never find a document.transientlist on a contract update fails as an unsupported keyword in the schema compatibility check. That is an internal error: the transition is dropped unpaid and the client gets no consensus error.transiententries are never validated. A dotted path such as"meta.tag"marks the property transient in the parsed type, but Drive drops values by top-level name, so the value is stored anyway.All of this is protocol version 14, unreleased. Before it ships, each fix is an in-place edit of an unreleased generation. Afterwards, the parser rules would need a gate so stored contracts stay readable.
Census (2026-09-23). I pulled the schema of every contract from the platform explorer: 52 on mainnet and 3,890 on testnet. The ones without a schema are token-only contracts with an empty
{}.transientappears only on DPNS-shapeddomaintypes: DPNS on mainnet, DPNS and five copies on testnet. All of them listpreorderSaltonly, a top-level property that no index reads, on an immutable type. No existing contract is refused by the new rules, and since no mutable type declarestransient, the replace change alters no stored document.What was done?
Every rule reads a path through
is_transient(the path and every enclosing object's). It is nowpubso drive-abci can use it.1. A replace drops transient values (
document_from_replace_transition_action1, rs-drive). The conversion that builds the stored document from a replace action drops the document type's transient values by top-level name. It goes through the samedrop_transient_valueshelper the create action now uses, so the two cannot drift apart. Validation still sees the transition's values, as it does for a create.Before, with a mutable type that declares
"transient": ["code"]:After:
2. No index reads a transient property (parser generation 3, full validation only).
Before: registered, and the unique index enforced nothing. After: refused with
index "byCode" of document type "note" reads "code", which is transient or inside a transient object: its value is never stored, so the index would never hold it. An index overmeta.tagwith"transient": ["meta"]is refused the same way.3. A
propertyAgreementnames no transient property on its referenced side (data_contract_reference_validation0, drive-abci). Take anotetype with"transient": ["topic"], and amessage.noteIddeclaring"propertyAgreement": { "topic": "topic" }.messagewith atopicwas refused withReferencedDocumentPropertyMismatchError.ReferencedDocumentPropertyAgreementInvalidError:the referenced property is transient or inside a transient object: no stored document carries its value, so none could be agreed with.$ownerId.4. A lookup's index keys documents by no transient property (
DocumentReferenceLookup::referenced_side_error). Registration already refuses such an index (2), but a type parsed without full validation still reaches this check. The refusal readsindex "bySubmittedCharter" of "joinRequest" keys documents by "submittedCharterId", which is transient or inside a transient object: its value is never stored, so the lookup could never find a document.5. A stored key id never pairs with a transient identity (
data_contract_reference_validation0), in both forms:ReferencedKeyIdPropertyInvalidError:the key id is stored but the identity property toUserId is transient or inside a transient object: a reader could not tell whose key it is.keyIdPropertyform (the reference on a transient identity property, pointing at a stored key id) is refused the same way.6. A changed
transientlist is an incompatible schema change (validate_schema_compatibility1). The list gets the frozen rulerefersTohas, asownerRefersToandcreatorRefersToalready do. The parse reads it as a set, so it is sorted and deduplicated before the diff: reordering or repeating a name is not a change. Adding"transient": ["name"]to an existing type:InternalError("... schema keyword 'transient' at path '/transient' is not supported"), dropped unpaid.IncompatibleDocumentTypeSchemaErrorforadd /transient, a paid consensus error.Generation 0 (protocol version 13) keeps the internal error, pinned by a test.
7. Every
transiententry names a top-level property (parser generation 3, full validation only). Before:"transient": ["meta.tag"],["ghost"]and["$ownerId"]all registered. After: each is refused withdocument type "note" lists "meta.tag" as transient, but it is not a top-level property of the document type: transient values are dropped by top-level name, so list the object around a nested property.Known limitation. Rules 2 and 7 run under full validation, and an update re-parses the whole contract that way. Neither the
transientlist nor an index can change on update, so a contract registered before protocol version 14 with one of these shapes could no longer be updated. The census found none on mainnet or testnet. This is the same trade-off the protocol 14 word-characters-only name rule made.Docs. Added a "Transient Properties" section to
book/src/data-model/documents.md, a description on the meta-schema v3transientkeyword (description text only), and item 35 of the protocol version 14 change list inv14.rs.In-place changes to shipped generations
data_contract_reference_validation0 (drive-abci; item 3 and item 5). Every table selects it, but only contract create and update state validation 1 call it, and only protocol version 14 selects those. Earlier versions never run it. Its module comment already records the same argument for the reference expressions added in place.DocumentReferenceLookup::referenced_side_error(dpp, unversioned; item 4). A lookup only parses at protocol version 14, so no earlier version reaches it.data.retain(...)strips now call the shareddrop_transient_values, which runs the sameretain.Everything else is edited in generations that only protocol version 14 selects:
document_from_replace_transition_action1 (DRIVE_STATE_TRANSITION_METHOD_VERSIONS_V4, onlyDRIVE_VERSION_V9), parser generation 3 (CONTRACT_VERSIONS_V6) andvalidate_schema_compatibility1.How Has This Been Tested?
transient_tests.rs(generation 3): top-level entries and objects are accepted; a bad entry and an index over a transient property are refused under full validation, still parse without it, and still register at protocol version 13.should_refuse_a_lookup_into_an_index_reading_a_transient_property.should_report_every_transient_list_change_as_incompatible(add, remove, append, replace),should_accept_a_reordered_or_repeated_transient_list, andshould_hard_error_on_a_transient_diff_at_protocol_version_13pinning protocol version 13.should_drop_transient_values_on_replace_conversion_from_protocol_version_14. The DPNSpreorderSaltis dropped by a replace at protocol version 14 and kept at 13; a create drops it at both.should_store_a_replaced_document_without_its_transient_valuescreates and replaces a document of a mutable type throughprocess_raw_state_transitionsand reads the stored document back.should_store_the_transient_values_of_a_replace_at_protocol_version_13pins the old behaviour.process_raw_state_transitionswith new fixtures:should_reject_agreement_on_a_transient_referenced_property(the property itself, and one inside a transient object) andshould_register_agreement_on_a_transient_referring_property.should_reject_a_stored_key_id_paired_with_a_transient_identity(both forms, plus an identity inside a transient object) andshould_register_a_key_reference_whose_key_id_and_identity_are_both_transient(both forms).should_refuse_an_update_changing_the_transient_list_as_an_incompatible_schema(contract update state validation).cargo test -p dpp --all-features --lib: 4775 passed.cargo test -p drive -p drive-abci --lib -- transient replace_conversion data_contract_create data_contract_update replacement dpns reference_validation: drive 54 passed, drive-abci 256 passed.cargo clippy -p dpp -p drive -p drive-abci --all-features --all-targets -- -D warnings: clean.cargo fmt --all -- --check: clean.Breaking Changes
Consensus-breaking at protocol version 14 only, which is unreleased. Nodes running it (devnets) must run the same binary:
transientchange on update becomes a paid consensus error instead of an internal error.The census found no mainnet or testnet contract affected. 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 ·
d17a1ae/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.