fix(sdk): prevent sized_integer_types config downgrade that breaks document#3071
Open
fix(sdk): prevent sized_integer_types config downgrade that breaks document#3071
Conversation
…cument deserialization
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughIntroduces V1 validation layers for data contract configuration, creation, and update state transitions that enforce minimum supported config versions derived from platform versioning. Includes new DPP and DRIVE_ABCI validation version constants (V3 and V8), updates platform version V12 to reference these, and restructures validation dispatch to conditionally route to V0 or V1 handlers. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Validator as Validator<br/>(V0 or V1)
participant PlatformVersion
participant ConfigMinVersion
Client->>Validator: validate_update(contract, new_config, version)
alt Version 1
Validator->>Validator: validate_basic_structure_v0()
alt V0 Validation Fails
Validator-->>Client: Return Error
else V0 Validation Passes
Validator->>PlatformVersion: Get dpp.contract_versions.config.min_version
PlatformVersion-->>Validator: min_version (e.g., 1)
Validator->>ConfigMinVersion: Compare new_config.version() vs min_version
alt Config Version < Min Version
Validator-->>Client: Return DataContractConfigUpdateError
else Config Version >= Min Version
Validator->>Validator: Check sized_integer_types transition
alt Disabling sized_integer_types (V1→V0)
Validator-->>Client: Return DataContractConfigUpdateError
else Valid Transition
Validator-->>Client: Return Success
end
end
end
else Version 0
Validator->>Validator: validate_basic_structure_v0()
Validator-->>Client: Return Result
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/rs-dpp/src/data_contract/document_type/mod.rs`:
- Around line 116-129: The recalc currently looks up property names directly in
the top-level schema, so dotted keys in flattened_properties (e.g.
"parent.child") never find their nested subschema; update
recalculate_integer_types_from_schema in DocumentType to handle
flattened_properties by resolving dotted keys into the nested schema before
calling recalculate_integer_types_from_schema(&mut properties, &schema): for
each key in flattened_properties split on '.' and walk the JSON Schema by
repeatedly entering "properties"[segment] (and following "items"/"properties"
for arrays/objects as needed) to locate the appropriate nested subschema, then
pass that subschema into the existing recalc routine; reference the
DocumentType::recalculate_integer_types_from_schema method and the calls that
currently pass v0.flattened_properties / v1.flattened_properties to locate and
change the behavior.
🧹 Nitpick comments (2)
packages/rs-dpp/src/data_contract/config/methods/validate_update/v1/mod.rs (1)
8-8:#[inline(always)]is unnecessary here.This is a validation method called during contract updates, not a hot-path function. The compiler will make a good inlining decision on its own for a function this size.
#[inline(always)]is typically reserved for tiny, performance-critical helpers.packages/rs-dpp/src/document/v0/serialize.rs (1)
1509-1553: Consider extracting the fallback pattern into a helper to reduce duplication.The identical fallback logic is repeated four times across
from_bytesandfrom_bytes_in_consensusfor version bytes 1 and 2. A helper closure or function parameterized on the deserialization function (from_bytes_v1/from_bytes_v2) could reduce the duplication. Not blocking — the current code is clear and each site has a helpful comment.Also applies to: 1612-1669
…type recalculation The recalculate function was using top-level schema lookups for flattened_properties, so dotted keys like "parent.child" would never match their nested subschema. Added resolve_schema_for_dotted_key that walks through nested "properties" objects to find the correct schema definition. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The test was calling setConfig without sizedIntegerTypes, which defaults to false. Since the contract was created with the platform default (sizedIntegerTypes: true), this triggered the new validation that blocks disabling sized integer types. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…_integer_types wasm-dpp uses PlatformVersion::first() which forces all configs to V0 during deserialization. This means SDKs always send ConfigV0 even when platform stores ConfigV1 with sized_integer_types=true. The validation now only rejects explicit V1(sized=false) configs, not V0 configs which simply lack the concept of sized_integer_types. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… sized_integer_types setConfig used PlatformVersion::first() which forced config deserialization to V0, silently dropping the sized_integer_types field. This caused contract updates to fail validation when the existing contract had sized_integer_types enabled. Now uses latest() so V1 configs are properly parsed. Also reverts the V0 special-case workaround in validate_update_v1, restoring the simple sized_integer_types true->false rejection for all config variants. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
QuantumExplorer
requested changes
Feb 9, 2026
packages/rs-platform-version/src/version/dpp_versions/dpp_validation_versions/v3.rs
Show resolved
Hide resolved
…pe deserialization The clone_with_sized_integer_types fallback attempted to recover from config downgrades at deserialization time. This approach is unnecessary now that config updates are properly validated upstream. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
QuantumExplorer
requested changes
Feb 12, 2026
Comment on lines
+25
to
+33
| if self.sized_integer_types() && !new_config.sized_integer_types() { | ||
| return SimpleConsensusValidationResult::new_with_error( | ||
| DataContractConfigUpdateError::new( | ||
| contract_id, | ||
| "contract can not disable sized integer types once enabled, as this would break deserialization of existing documents", | ||
| ) | ||
| .into(), | ||
| ); | ||
| } |
Member
There was a problem hiding this comment.
Are we sure you can go V0 to V1?, Also I would include in V1 a verification that the DataContractConfig is V1, makes no sense to register or update to V0 anymore.
Collaborator
Author
There was a problem hiding this comment.
Yes, you can upgrade from v0 to v1. Agree, we shouldn't allow v0.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_update/mod.rs (1)
54-57:⚠️ Potential issue | 🟡 MinorMinor:
known_versionsinVersionNotActivearm not updated to include1.The
Some(version)arm on Line 51 was updated tovec![0, 1], but theNonearm still reportsvec![0]. For consistent error messaging, this should also reflect the newly supported version.Proposed fix
None => Err(Error::Execution(ExecutionError::VersionNotActive { method: "data contract update transition: validate_basic_structure".to_string(), - known_versions: vec![0], + known_versions: vec![0, 1], })),packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_create/mod.rs (1)
106-109:⚠️ Potential issue | 🟡 Minor
known_versionsinVersionNotActivearm is stale.Line 108 still lists
known_versions: vec![0]but version 1 is now also supported (Line 103). This should bevec![0, 1]for consistency with theUnknownVersionMismatcharm.Proposed fix
None => Err(Error::Execution(ExecutionError::VersionNotActive { method: "data contract create transition: validate_basic_structure".to_string(), - known_versions: vec![0], + known_versions: vec![0, 1], })),
🧹 Nitpick comments (3)
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_create/basic_structure/v1/mod.rs (1)
36-48: Semantically misleading error type for creation context.
DataContractConfigUpdateErroris used here to reject a create transition, not an update. While the error message text is clear, the error type name may confuse consumers who filter or handle errors by variant. Consider whether a more general error type (or a new one) would be more appropriate.packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_create/mod.rs (1)
314-317: Misleading comment for protocol version 8 test.The comment says "required since protocol version 12" but this test explicitly targets protocol version 8, where V0 config is still valid. The config is being set to V1 here likely to satisfy some other constraint introduced by this PR, but the comment is confusing in this context. Consider clarifying why V1 config is being set here despite testing against an older protocol version.
packages/rs-dpp/src/data_contract/config/methods/validate_update/v1/mod.rs (1)
23-39: Consider the ordering: min_version check may mask the more specificsized_integer_typeserror.When a V1(sized=true) config attempts to downgrade to V0, both the min_version check (line 27) and the sized_integer_types check (line 46) would trigger. Currently, the user gets the generic "config version X is not supported" error rather than the more descriptive "contract can not disable sized integer types once enabled" message. This is functionally correct but could make the V1→V0 downgrade scenario less informative for the developer.
If the more descriptive error is preferred for that scenario, consider swapping the check order (sized_integer_types check before min_version). Otherwise this is fine as-is since both rejections are correct.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- qs: ^6.14.1 -> ^6.14.2 (fixes arrayLimit bypass DoS) - tar: 7.5.7 -> 7.5.9 (fixes deprecation warning) - webpack: 5.94.0 -> 5.105.2 (fixes buildHttp SSRF bypasses) Co-Authored-By: Claude Opus 4.6 <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
Changing
DataContractConfigfrom V1 (sized_integer_types=true) to V0 (sized_integer_types=false) breaks deserialization of existing documents that were serialized with sized integer types (version byte 1/2). When the config is downgraded, all integer property types become I64, but the binary data still contains sized integers (U8/U16/U32/U64), causing deserialization failures.This was the root cause of document deserialization failures observed on testnet where the JS SDK hardcodes protocol v1, which auto-downgrades config V1 to V0.
What was done?
Part 1: Validation — prevent future breakage
validate_update_v1that blockssized_integer_typesfrom being changed fromtruetofalse, as this direction is dangerous. The reverse(
false→true) remains allowed since version byte 0 documents usefrom_bytes_v0which forces all integers to I64 regardless of config.validate_update_v1delegates tovalidate_update_v0for all existing checks, then adds thesized_integer_typesguard.DPP_VALIDATION_VERSIONS_V3(validate_config_update: 1) in platform version v12.Part 2: Deserialization fallback — mitigate already-broken contracts in state
clone_with_sized_integer_types()onDocumentTypeRefthat reconstructs sized integer types from the schema's min/max constraints.from_bytesandfrom_bytes_in_consensusfor version bytes 1 and 2: if deserialization fails, reconstruct the documenttype with sized types from schema and retry.
find_integer_type_for_subschema_valuevisible withincrate::data_contractto support the fallback.How Has This Been Tested?
Validation tests (6) in
validate_update::v1::tests:Deserialization fallback tests (3) in
document::v0::serialize::tests:Breaking Changes
None. This is a purely additive change — existing valid contract updates continue to work. Only the previously-unvalidated dangerous config downgrade (sized_integer_types true→false) is now blocked for new protocol version.
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
Release Notes
New Features
Chores