fix(platform): document serialization v1 to fix serialization and deserialization of integers #2578
Conversation
WalkthroughThis set of changes introduces support for a new document serialization version (v1) that encodes integer properties using their native sizes, alongside the existing v0 format. Method signatures for document serialization and deserialization are updated throughout the codebase to require an explicit Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Document
participant DataContract
participant PlatformVersion
Caller->>Document: serialize(document_type, data_contract, platform_version)
alt contract version 0 or config version 0
Document->>Document: serialize_v0(document_type)
else contract version > 0
alt platform_version specifies v0
Document->>Document: serialize_v0(document_type)
else platform_version specifies v1
Document->>Document: serialize_v1(document_type)
end
end
Document-->>Caller: serialized_bytes
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)📓 Common learnings⏰ Context from checks skipped due to timeout of 90000ms (3)
🔇 Additional comments (1)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
packages/rs-dpp/src/document/v0/serialize.rs (1)
271-481: 🛠️ Refactor suggestionLarge duplication between
serialize_v0andserialize_v1
serialize_v1is almost a verbatim copy ofserialize_v0, differing only in the integer-encoding branch. Keeping two 200-line copies will make future maintenance error-prone.Consider extracting the common body into an internal helper that takes a flag (or closure) describing how to encode integer properties, e.g.:
fn serialize_internal<F>(&self, document_type: DocumentTypeRef, int_encoder: F) -> Result<Vec<u8>, ProtocolError> where F: Fn(&DocumentPropertyType, &Value, bool) -> Result<Vec<u8>, ProtocolError>
serialize_v0would pass a closure that always forcesDocumentPropertyType::I64, whileserialize_v1would forward the realproperty.property_type.This reduces duplication and the risk of version skew.
🧹 Nitpick comments (8)
packages/rs-drive/src/cache/system_contracts.rs (1)
25-55: Excellent addition of atomic system contracts reload functionalityThis new method is well-designed and documented. It atomically reloads all system contracts when platform versions change, which is crucial for handling serialization version changes. The implementation correctly:
- Loads all contracts first before making any changes
- Fails fast if any contract loading fails
- Uses atomic operations via ArcSwap for safe concurrent access
- Provides clear documentation with usage guidance
This is particularly important given the new document serialization version, as system contracts need to use the correct serialization format after protocol upgrades.
Consider adding a debug log statement at the end of the method to record that system contracts were successfully reloaded. This would aid in debugging and monitoring during protocol upgrades.
packages/rs-platform-version/src/version/dpp_versions/dpp_document_versions/v2.rs (1)
6-31: Well-structured version definitions for document serialization.The constant
DOCUMENT_VERSIONS_V2properly defines the versioning information for document structures and serialization. The version configuration correctly supports both v0 and v1 document serialization (lines 8-12), with v1 set as the default current version. This aligns with the PR objective of fixing integer serialization and deserialization.One suggestion to consider:
Consider adding a brief documentation comment above the constant to explain its purpose and the key differences from previous versions:
+/// Document versions configuration that supports v1 document serialization, +/// which properly handles integer types according to their data contract definitions +/// rather than forcing all integers to be i64s as in v0. pub const DOCUMENT_VERSIONS_V2: DPPDocumentVersions = DPPDocumentVersions {packages/rs-dpp/src/voting/contender_structs/contender/v0/mod.rs (2)
40-46: Update the docs to reflect the newdata_contractparameterBoth
try_into_contender_with_serialized_documentandtry_to_contender_with_serialized_documentnow require a&DataContract, yet the function-level doc-comments (and IDE tool-tips) were not updated. Adding a short note about the new argument prevents callers from overlooking it and avoids stale documentation.
55-58: Possible redundant serialization work
Option::map(..).transpose()?is elegant, but the closure will be executed every time these helpers are called – even when the caller ultimately discards the serialized bytes.
If you foresee calling these helpers in hot paths where thedocumentis sometimes later ignored, consider caching the serialized form or exposing a variant that skips serialization when the caller knows it is unnecessary.packages/rs-dpp/src/voting/contender_structs/contender/mod.rs (1)
165-177: Micro-nit: avoid the extra temporary allocation
serialize()currently:
- Builds an intermediate
ContenderWithSerializedDocument,- Allocates a
Vec<u8>inside it,- Immediately copies that
Vec<u8>again viaserialize_to_bytes().If the inner
serialize_to_bytes()simply returnsself.serialized_documentwhen present, you can skip one allocation:- self.try_to_contender_with_serialized_document( - document_type, - data_contract, - platform_version, - )? - .serialize_to_bytes() + self.try_to_contender_with_serialized_document( + document_type, + data_contract, + platform_version, + )? + .take_serialized_document() + .unwrap_or_default()(Adjust depending on the exact API of
serialize_to_bytes.)packages/rs-dpp/src/document/v0/serialize.rs (3)
215-243: Boolean logic: prefer&&over bitwise&for readabilityThroughout the property loop you use the expression
property.required & !property.transient.
Although&works onbool,&&is the conventional logical operator and avoids the momentary mental context-switch to “bitwise-on-bool” for readers.- .encode_value_ref_with_size(value, property.required & !property.transient) + .encode_value_ref_with_size(value, property.required && !property.transient)Apply the same change in
serialize_v1,from_bytes_v0, andfrom_bytes_v1.
653-685: Same boolean bitwise issue during deserialization
property.property_type.read_optionally_from(&mut buf, property.required & !property.transient)
→ prefer&&(logical AND) here as well for clarity; the current bitwise form can be misread as an intentional mask.
994-1034: Fallback to v1 deserialization is great – add a unit testThe “try v0 then v1” logic is a valuable robustness feature. A dedicated test that:
- Serializes a document in v1,
- Manually rewrites the leading varint to
0,- Asserts that
from_bytesstill succeeds,will guard against accidental regressions in the fallback path.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
packages/rs-dpp/src/document/document_methods/hash/v0/mod.rs(1 hunks)packages/rs-dpp/src/document/extended_document/v0/serialize.rs(3 hunks)packages/rs-dpp/src/document/mod.rs(3 hunks)packages/rs-dpp/src/document/serialization_traits/platform_serialization_conversion/deserialize/v0/mod.rs(1 hunks)packages/rs-dpp/src/document/serialization_traits/platform_serialization_conversion/mod.rs(4 hunks)packages/rs-dpp/src/document/serialization_traits/platform_serialization_conversion/serialize/v0/mod.rs(1 hunks)packages/rs-dpp/src/document/serialization_traits/platform_serialization_conversion/v0/mod.rs(3 hunks)packages/rs-dpp/src/document/v0/serialize.rs(8 hunks)packages/rs-dpp/src/system_data_contracts.rs(1 hunks)packages/rs-dpp/src/voting/contender_structs/contender/mod.rs(2 hunks)packages/rs-dpp/src/voting/contender_structs/contender/v0/mod.rs(5 hunks)packages/rs-drive-abci/src/execution/platform_events/block_processing_end_events/tests.rs(8 hunks)packages/rs-drive-abci/src/execution/platform_events/protocol_upgrade/perform_events_on_first_block_of_protocol_change/v0/mod.rs(1 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/data_triggers/triggers/withdrawals/v0/mod.rs(1 hunks)packages/rs-drive-abci/tests/strategy_tests/verify_state_transitions.rs(1 hunks)packages/rs-drive/benches/benchmarks.rs(3 hunks)packages/rs-drive/src/cache/system_contracts.rs(1 hunks)packages/rs-drive/src/drive/document/delete/mod.rs(1 hunks)packages/rs-drive/src/drive/document/insert/add_document_to_primary_storage/v0/mod.rs(2 hunks)packages/rs-drive/src/drive/document/insert_contested/add_contested_document_to_primary_storage/v0/mod.rs(1 hunks)packages/rs-drive/src/drive/document/update/mod.rs(1 hunks)packages/rs-drive/src/drive/initialization/v0/mod.rs(1 hunks)packages/rs-drive/tests/query_tests.rs(3 hunks)packages/rs-platform-version/src/version/dpp_versions/dpp_document_versions/mod.rs(1 hunks)packages/rs-platform-version/src/version/dpp_versions/dpp_document_versions/v2.rs(1 hunks)packages/rs-platform-version/src/version/v9.rs(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
packages/rs-drive-abci/src/execution/platform_events/protocol_upgrade/perform_events_on_first_block_of_protocol_change/v0/mod.rs (1)
Learnt from: shumkov
PR: dashpay/platform#2345
File: packages/rs-drive-abci/src/execution/platform_events/protocol_upgrade/perform_events_on_first_block_of_protocol_change/v0/mod.rs:93-99
Timestamp: 2024-11-22T08:19:14.448Z
Learning: In `packages/rs-drive-abci/src/execution/platform_events/protocol_upgrade/perform_events_on_first_block_of_protocol_change/v0/mod.rs`, the `insert_contract` method requires an owned `BlockInfo`, so cloning `block_info` is necessary when calling it.
🧬 Code Graph Analysis (2)
packages/rs-drive/src/cache/system_contracts.rs (1)
packages/rs-dpp/src/system_data_contracts.rs (1)
load_system_data_contract(42-49)
packages/rs-drive/tests/query_tests.rs (3)
packages/rs-dpp/src/document/serialization_traits/platform_serialization_conversion/mod.rs (1)
serialize(19-30)packages/rs-dpp/src/document/serialization_traits/platform_serialization_conversion/v0/mod.rs (1)
serialize(14-19)packages/rs-dpp/src/document/v0/serialize.rs (3)
serialize(930-964)document_type(653-685)document_type(876-901)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dapi-grpc) / Linting
- GitHub Check: Rust packages (dapi-grpc) / Unused dependencies
- GitHub Check: Rust packages (rs-dapi-client) / Linting
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build JS packages / Build JS
🔇 Additional comments (52)
packages/rs-drive/src/drive/initialization/v0/mod.rs (1)
189-189: Added explicit type annotation for integer encodingThe change from
0.encode_var_vec()to0u64.encode_var_vec()makes the integer type explicit during encoding, which aligns with the PR's objective to fix serialization and deserialization of integers across different contract versions.packages/rs-drive-abci/tests/strategy_tests/verify_state_transitions.rs (1)
465-466: Updated document serialization call to include data contract referenceThis change adds the required data contract reference parameter to the document serialization call, consistent with the updated method signatures introduced in this PR to support the new document serialization version.
packages/rs-platform-version/src/version/dpp_versions/dpp_document_versions/mod.rs (1)
4-4: Added new document version moduleThis addition of
pub mod v2;integrates support for the new document serialization version (v1) that encodes integer properties using their native sizes, a core feature in this PR.packages/rs-drive-abci/src/execution/platform_events/protocol_upgrade/perform_events_on_first_block_of_protocol_change/v0/mod.rs (1)
69-72: Added system contract reload during protocol upgradesThis addition ensures that system data contracts are atomically reloaded during protocol upgrades, which is essential for proper functioning of the new document serialization methods that now require up-to-date data contract references.
packages/rs-dpp/src/document/mod.rs (3)
266-269: Correctly added contract reference to serialization call.The
&contractparameter is now passed to theserializemethod, which is necessary for proper integer type serialization according to the new document serialization version.
308-311: Correctly added contract reference to serialization call.The
&contractparameter is now passed to theserializemethod in the older version test scenario to maintain consistency with the updated method signature.
325-328: Correctly added contract reference to serialization call.The
&contractparameter has been properly added to theserializemethod in the newer version test scenario, ensuring consistent method signature usage throughout the test.packages/rs-drive/src/drive/document/delete/mod.rs (1)
325-327: Correctly updated serialize method call to include contract reference.The test now correctly passes the data contract to the
serializemethod, which is required for the new document serialization version that properly handles integer types.packages/rs-platform-version/src/version/v9.rs (2)
5-5: Updated to use the new document versions module.The import has been correctly changed to use
dpp_document_versions::v2::DOCUMENT_VERSIONS_V2which supports the new serialization version for integer types.
52-53: Platform V9 now uses document versions V2.The platform version 9 definition now correctly uses
DOCUMENT_VERSIONS_V2instead of V1, with a helpful comment explaining this change supports the new serialization approach for integers. This is a key part of the fix for integer serialization and deserialization across different contract versions.packages/rs-drive/src/drive/document/update/mod.rs (1)
916-919: Correctly added contract reference to document serialization.The serialization call in the test function now includes the data contract reference, which is necessary for the enhanced document serialization that properly handles integer types.
packages/rs-dpp/src/system_data_contracts.rs (1)
67-84: Good test addition to verify contract versioning.This test ensures that different platform versions (8 vs 9) produce different TokenHistory contract instances, which validates the version-aware serialization logic introduced in this PR. This is critical for ensuring the new document serialization functionality works correctly with different platform versions.
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/data_triggers/triggers/withdrawals/v0/mod.rs (1)
236-237: API update correctly implemented.The serialization call has been updated to include the required
&data_contractparameter, keeping it consistent with the new API that makes document serialization contract-aware. This ensures the proper serialization version is used based on the contract's version.packages/rs-dpp/src/document/document_methods/hash/v0/mod.rs (1)
21-21: Proper update to hash function serialization.The serialization call in the
hash_v0method now correctly passes thecontractparameter, ensuring the document hash is calculated using the appropriate serialization version based on the contract. This is critical for maintaining hash consistency across different contract versions.packages/rs-drive/benches/benchmarks.rs (1)
49-50: Benchmarks correctly updated to use new API.All serialization calls in the benchmarks have been updated to include the required
&contractparameter, ensuring they correctly test the new contract-aware serialization logic. This maintains the validity of the benchmarks and ensures they accurately measure the performance of the updated implementation.Also applies to: 81-82, 111-112
packages/rs-dpp/src/document/serialization_traits/platform_serialization_conversion/serialize/v0/mod.rs (2)
11-11: Clear documentation additionGood addition of documentation specifying how integers are encoded in the v0 serialization format. This helps developers understand the distinction between serialization versions.
14-19: Well-documented new serialization methodThe new
serialize_v1method with its documentation clearly explains the key difference from v0: encoding integers with their native size rather than always as i64. This aligns perfectly with the PR objective to fix integer serialization and deserialization issues.packages/rs-drive/src/drive/document/insert_contested/add_contested_document_to_primary_storage/v0/mod.rs (4)
94-109: Updated serialization call with required parametersThe document serialization calls now correctly include the data contract as a parameter, which is required for the new serialization approach that properly handles integer types.
122-137: Consistent serialization parameter updatesThe serialization call in the
DocumentOwnedInfocase has been correctly updated to include all required parameters (document_type, contract, platform_version) for proper integer serialization.
165-180: Consistent serialization parameter patternThe updates to this code block follow the same pattern as the others, ensuring consistent serialization parameter passing throughout the codebase.
181-196: Comprehensive serialization updatesAll serialization calls have been consistently updated to include the document type, contract, and platform version, ensuring proper integer handling during serialization.
packages/rs-dpp/src/document/extended_document/v0/serialize.rs (3)
34-34: Explicit type specification for version encodingThe change from
0.encode_var_vec()to0u64.encode_var_vec()makes the integer type explicit, which is good practice for clarity and prevents potential type inference issues.
43-47: Updated document serialization with data contract parameterThe serialization call now correctly passes the data contract as an additional parameter, which is required for proper integer type handling during serialization.
113-113: Correct versioning field for extended documentsThe change from
document_serialization_versiontoextended_document_serialization_versionensures that the correct versioning field is used specifically for extended documents, improving precision in version selection.packages/rs-dpp/src/document/serialization_traits/platform_serialization_conversion/deserialize/v0/mod.rs (3)
9-10: Well-documented integer handling behavior.The added documentation clearly explains the integer handling behavior in version 0, which helps developers understand the limitations of this version.
19-27: Good implementation of version 1 deserialization method.The addition of
from_bytes_v1properly addresses the integer serialization issue by supporting the data contract's encoded integer types. The method signature correctly mirrors the v0 version with identical parameters, making it easy to understand and use.
33-34: Consistent documentation across traits.Documentation is consistently applied to both traits, reinforcing the same information about integer handling in version 0.
packages/rs-drive/tests/query_tests.rs (3)
496-500: Added necessary contract parameter to Document serialization.The code correctly adds the required
&contractparameter to the serialize method call to match the updated method signature that now requires the data contract as a parameter. This allows the serialization method to access contract properties and determine the appropriate serialization version.
545-551: Correctly updated serialize method call with contract parameter.The code properly adds the required
&contractparameter to the serialization call in this test case, maintaining consistency with the updated method signature that requires the data contract reference to determine serialization behavior.
596-602: Updated serialization call with expected success behavior.This change correctly provides the
&contractparameter to the serialization method and explicitly expects success. The updated call pattern aligns with the new requirements in document serialization related to handling different contract versions.packages/rs-drive-abci/src/execution/platform_events/block_processing_end_events/tests.rs (8)
135-137: Data contract parameter added for document serialization.The
setup_initial_documentfunction has been updated to include thedashpaydata contract parameter that will be used for document serialization. This change aligns with the updated method signatures throughout the codebase that now require the data contract context during serialization.
176-178: Added data contract to document serialization call.The
serializemethod now includes the data contract parameter, which is necessary for the new document serialization approach that encodes integer properties using their native sizes based on the contract context.
302-310: Updated function call to pass data contract parameter.Function call updated to provide the data contract as required by the new signature of
setup_initial_document. This ensures the document is serialized with proper contract context.
407-415: Updated function call to pass data contract parameter.Another instance of updating the function call to include the required data contract parameter for proper document serialization with context.
517-525: Updated function call to pass data contract parameter.Function call updated to provide the data contract for consistent serialization behavior across the codebase.
623-631: Updated function call to pass data contract parameter.Function call updated to pass the data contract, maintaining consistency with the new serialization approach.
729-737: Updated function call to pass data contract parameter.The last instance of updating the function call to include the data contract parameter for proper document serialization.
836-844: Updated function call to pass data contract parameter.Function call updated to provide the data contract for proper serialization context.
packages/rs-drive/src/drive/document/insert/add_document_to_primary_storage/v0/mod.rs (3)
148-244: Enhanced document serialization with data contract context.The serialization code paths for various document info variants have been updated to consistently include the data contract as a parameter when calling
document.serialize(). This ensures that integer properties are correctly serialized according to the contract's schema, supporting the new document serialization version.The pattern is consistently applied across all document variants:
- DocumentRefAndSerialization
- DocumentAndSerialization
- DocumentOwnedInfo
- DocumentRefInfo
- DocumentEstimatedAverageSize
This implementation properly maintains the existing control flow while enhancing the serialization context.
297-365: Added data contract to serialization calls in insert_without_check branch.Similarly updated all document serialization calls in the insert_without_check branch to include the data contract parameter, ensuring consistent behavior across all code paths.
368-436: Added data contract to serialization calls in else branch.Updated all document serialization calls in the else branch to include the data contract parameter, completing the consistent application of the change across all code paths in this method.
packages/rs-dpp/src/document/serialization_traits/platform_serialization_conversion/mod.rs (7)
6-6: Added DataContract import for serialization context.Import added for the DataContract type that will be used in the updated method signatures.
19-23: Added data contract parameter to Document.serialize method.The
serializemethod signature forDocumentnow includes adata_contractparameter, which is necessary for proper integer serialization in the new document serialization version.
26-28: Updated inner call to pass data contract to DocumentV0 serialization.The implementation now correctly passes the data contract to the inner DocumentV0's serialize method, ensuring the context is maintained through the serialization hierarchy.
32-36: Added data contract parameter to serialize_specific_version method.The
serialize_specific_versionmethod is also updated to include the data contract parameter for consistent behavior across all serialization methods.
39-43: Updated inner call to pass data contract to DocumentV0 serialization_specific_version.The implementation now correctly passes the data contract to the inner DocumentV0's serialize_specific_version method, maintaining the serialization context.
125-127: Updated test to pass data contract to document.serialize.The test case has been updated to include the data contract parameter when calling
document.serialize(), ensuring that the tests validate the new serialization behavior.
140-142: Updated test loop to pass data contract parameter.Another instance of updating the test to include the data contract parameter for serialization, ensuring comprehensive test coverage.
packages/rs-dpp/src/document/serialization_traits/platform_serialization_conversion/v0/mod.rs (3)
2-2: Added DataContract import for trait definition.Import added for the DataContract type that will be used in the updated trait method signatures.
14-18: Updated trait method signature to include data contract parameter.The
serializemethod in theDocumentPlatformConversionMethodsV0trait now requires adata_contractparameter, formalizing the interface change across all implementations. This enables proper document serialization with integer type awareness based on contract context.
25-29: Updated serialize_specific_version signature to include data contract parameter.Similarly, the
serialize_specific_versionmethod in the trait now requires adata_contractparameter, ensuring consistent parameter requirements across all serialization methods.packages/rs-dpp/src/voting/contender_structs/contender/mod.rs (1)
130-146: Public API signature change – double-check downstream cratesIntroducing
data_contract: &DataContractinto the publicContender::*_with_serialized_documentfamily is a breaking change for any external crate that depended on the old 3-parameter form.
Please ensure:
#[cfg(feature = "legacy")]shims or re-exports exist where necessary, or- The crate’s semantic version is bumped accordingly and release notes call out the change.
Failing to do so will cause downstream compilation errors.
QuantumExplorer
deleted the
feat/clearDataContractCacheOnEveryVersionChange
branch
Issue being fixed or feature implemented
In data contract config v1 there is an option to have integers be based on the type that mosts suits the use case. For example using a u8 if we can only have 0, 1, 2. However serialization could happen on old contract versions that didn't have this feature, leading to deserialization problems where we didn't know what system was used.
What was done?
Introduced a new document serialization version that informs the deserializer if this feature was active.
Reload all system contracts whenever the network updates protocol versions. This will make data contracts in version 0 "transform" into version 1 on protocol version 9.
How Has This Been Tested?
Verified that some tests worked.
Breaking Changes
Not breaking.
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests
Chores