Repository navigation
fix(platform)!: a preallocated agreement source must fit a tree key (PV14) - #5123
Conversation
…PV14) A preallocated index keyed through a refersTo propertyAgreement writes the referenced document's value as a tree key when that document is created. When the referenced property could hold more than 255 bytes, the contract registered, and then every create of a referenced document carrying the value failed with an internal error (the estimate sized the layer from the referenced property's midway size). Contract create and update state validation 1 now refuse such a pair, paid, with ReferencedDocumentPropertyAgreementInvalidError (40126). Drive sizes the preallocated layer from the referring index property, as an entry insert does, and preallocates nothing for a referenced value wider than the referring property can hold, so a contract of that shape applied before the check still takes documents. PV14 note 55. Split out of #5102, which keeps the value comparison of agreement pairs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
📖 Book Preview built successfully. Download the preview from the workflow artifacts. Updated at 2026-09-28T11:57:43.663Z |
|
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 (14)
✨ 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 |
This pull request now only makes propertyAgreement pairs compare values instead of index keys. The registration rule that a preallocated index's agreement source fits a tree key, and the Drive preallocation changes that go with it, move unchanged to #5123 with their tests and fixtures. The PV14 note is renumbered from 53, which #5099 took on v4.2-dev, to 56 (54 is claimed by an open pull request, 55 by #5123), and now covers only the comparison. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
🕓 Queued for automated review — 3rd in line, estimated start in ~25 min (commit 29ccb40)
|
Basic explanation
What this does: A
refersToreference can declare apropertyAgreement, for example "a like must carry the same hashtag as the post it likes". Apreallocatedindex on the like type makes Platform create the like's index trees as soon as the post is created, keyed by the post's hashtag. An index key holds at most 255 bytes. If the post's hashtag could be longer than that, those trees could not be made, and creating a post failed with an internal error. The contract still registered, so a post type could end up where no tagged post could ever be created.From protocol version 14 such a contract, or a contract update that adds such an index, is refused at registration with a clear error. Drive also stops failing on a contract of that shape that already exists: it sizes the cost estimate from the like's hashtag, and it creates no trees for a post value that no like could ever carry.
This was split out of #5102, which keeps the other half: agreement pairs compare values instead of index keys.
Example:
post.hashtagis a string of up to 280 characters, bound to a like's preallocatedbyHashtagPostInternalError("fee: overflow error: referenced document field is too big for being an index")ReferencedDocumentPropertyAgreementInvalidError(40126), naming the indexbyHashtagPosttreespost.hashtagatmaxLength63 (252 bytes)Value: A contract author who binds a preallocated index to a property that is too wide finds out when registering, instead of after the post type has stopped working for good.
Risks: Consensus change at protocol version 14, which is not on mainnet or testnet. Replaying devnet history that contains such contracts gives different results: a contract that registered with a too-wide preallocated agreement source is refused, and the preallocation estimate for an agreement-keyed level changes (it feeds the balance pre-check). The drive-abci module edited in place is listed at older protocol versions too; the section below says why the edit cannot run there.
Issue being fixed or feature implemented
A preallocated index keyed by an agreement source wider than any tree key (from #4509). Take
post.hashtagwithmaxLength280 and no index, and an indexOnlylikewhose preallocatedbyHashtagPostindex is keyed throughpropertyAgreement { hashtag: hashtag }, withlike.hashtagatmaxLength63. The contract registers. Creating a post then preallocates the like trees:validate_fees_of_eventruns the insert withapply = false) sized the hashtag level from the referenced property's midway size, 2 × (minLength + maxLength) bytes for a string, 562 here, and returnedFeeError::Overflowabove 255. Every post carrying a hashtag failed, however short.Contract registration (
data_contract_reference_validationv0) never looked at the size. A contract update addingliketo a live contract withpostdid the same to an existing type.What was done?
data_contract_reference_validationv0 refuses, with 40126, an agreement pair through which a preallocated index of the declaring type is keyed (found with the sameIndex::preallocation_bindingsthe insert uses) when the referenced property'ssaturating_max_byte_sizeis aboveMAX_INDEX_SIZE. This runs on contract create and update. Agreements that key no preallocated index are not bounded: their values are never tree keys.add_preallocated_index_tree_operations, reached only fromadd_document_for_contract_operations1, which onlyDRIVE_DOCUMENT_METHOD_VERSIONS_V4, protocol version 14, selects):max_byte_sizeis not preallocated, as an absent or null value already is: no referring document could ever agree with it.DocumentTypeRefinstead of its name and index structure, which also drops a second lookup of the type by name.packages/rs-platform-version/src/version/v14.rs.contract-keywords/refers-to.md,contract-keywords/index-only.md) states the new registration rule and the skipped preallocation.Before and after, for the preallocated fixture with
post.hashtagup to 280 characters:With
post.hashtagatmaxLength63 (252 bytes) the contract registers and posts and likes are created, before and after.In-place changes to shipped generations
data_contract_reference_validation0 (drive-abci) is listed by everyDRIVE_ABCI_VALIDATION_VERSIONS_*table, includingV9at protocol version 13, but it is called only from contract create and update state validation 1, which onlyDRIVE_ABCI_VALIDATION_VERSIONS_V10(protocol version 14) selects. Protocol versions 1 to 13 run state validation 0, which never calls it, so the new check cannot run there.preallocateditself only parses from meta-schema v3, also protocol version 14. The existing in-place comments of this module make the same argument, and the new check carries one.add_preallocated_index_tree_operations_for_referring_typesis called only fromadd_document_for_contract_operations1, which onlyDRIVE_DOCUMENT_METHOD_VERSIONS_V4(DRIVE_VERSION_V9, protocol version 14) selects.How Has This Been Tested?
The code and tests come unchanged from #5102. Two batch tests moved into their own file, and two contract-create tests moved a few tests down in the same module, so that this pull request and #5102 do not touch the same lines. All test names start with
should_:batch/tests/document/preallocated_agreement_source.rs(full ABCI pipeline):should_create_posts_and_likes_when_the_agreement_source_fits_a_tree_key:maxLength63, including a 63-emoji hashtag.should_create_posts_under_an_agreement_source_wider_than_a_tree_key: a too-wide contract applied directly, as one registered before this check would be; a 70-emoji post and a short-hashtag post with its like are created.should_reject_an_agreement_keying_a_preallocated_index_by_a_property_wider_than_a_tree_key,should_register_an_agreement_keying_a_preallocated_index_by_a_property_that_fits_a_tree_key. Contract update:should_reject_contract_update_keying_a_preallocated_index_by_a_property_wider_than_a_tree_key.should_skip_preallocation_for_a_bound_value_wider_than_the_referring_property(the estimate and the insert of a 280-byte-hashtag post succeed, the estimate upper-bounds the applied storage fee, nobyHashtagPosttrees,byPoststill preallocated, a short hashtag still preallocated), andshould_size_an_agreement_bound_layer_as_the_referring_property(the estimated layer is 128 bytes, fromlike.hashtag).None of these tests depends on #5102's value comparison: every agreement they exercise holds under the old key comparison too.
For this split no cargo build or test was run locally, because the build machine is short on disk, so this pull request relies on CI. Checked locally:
git diff --check,rustfmt --edition 2021 --checkon every touched Rust file, the three JSON fixtures parse, and a trial merge of this branch with the trimmed #5102 is clean and gives back #5102's original code.Breaking Changes
Consensus change at protocol version 14 (unreleased), PV14 note 55. At protocol version 14 a contract create or update is refused with
ReferencedDocumentPropertyAgreementInvalidError(40126) when a preallocated index is keyed through an agreement whose referenced property can hold more than 255 bytes, and the preallocation estimate for an agreement-keyed level is sized from the referring property. Protocol versions 1 to 13 are unchanged.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 ·
29ccb40/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