feat(platform): keyword search system contract #2523
Conversation
WalkthroughThis update introduces comprehensive support for keywords and descriptions in data contracts, tightly integrating a new Keyword Search system contract into the platform. The changes span contract creation, update, and validation logic, enforcing constraints on keyword count, uniqueness, and length, as well as description length. New modules and methods in the Drive and DPP layers manage the creation, updating, and storage of keyword and description documents within the Keyword Search contract. Extensive error handling and consensus error types have been added, and new tests ensure correct enforcement of restrictions and document synchronization. Platform versioning and system contract loading are updated to include the new contract. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Platform
participant Drive
participant KeywordSearchContract
User->>Platform: Create/Update DataContract (with keywords/description)
Platform->>Drive: Insert/Update DataContract
alt DataContract has keywords
Drive->>KeywordSearchContract: Add/Update contractKeywords documents
end
alt DataContract has description
Drive->>KeywordSearchContract: Add/Update shortDescription/fullDescription documents
end
Drive-->>Platform: Result (success or consensus error)
Platform-->>User: Acknowledge or return error
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🪧 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
🧹 Nitpick comments (12)
packages/rs-platform-version/src/version/drive_versions/drive_contract_method_versions/v2.rs (2)
20-20: Clarify Version Change forinsert_contract
The version forinsert_contracthas been updated to1for token insertion. Please ensure that this change is consistent with the intended token insertion behavior. It may also be helpful to provide a brief explanation in the documentation or inline comments on why this version update was necessary.
22-22: Clarify Version Change forupdate_contract
The version forupdate_contracthas been updated to1(for token insertion) as noted in the comment. Verify that this update aligns with the overall versioning strategy for contract updates involving token insertion. Consider adding additional context if needed so that future maintainers can clearly understand the rationale behind this change.Dockerfile (1)
71-89: Formatting Improvement in Dependency Installation
The multi-lineapk addcommand in the deps-base stage has been reformatted improving readability. There is no functional change here.packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_create/advanced_structure/v0/mod.rs (1)
174-227: Implement keyword validation logic with potential for refactoring.The implementation correctly validates that keywords are unique, have an appropriate length (3-50 chars), and use ASCII encoding. Each check properly returns the appropriate error when validation fails.
However, there's significant code duplication in the error handling pattern. Consider refactoring to extract the common error-handling logic into a helper function.
- // Current implementation with repeated code pattern + // Helper function approach + fn create_keyword_validation_error<E: Into<ConsensusError>>( + &self, + error: E, + ) -> Result<ConsensusValidationResult<StateTransitionAction>, Error> { + let bump_action = StateTransitionAction::BumpIdentityNonceAction( + BumpIdentityNonceAction::from_borrowed_data_contract_create_transition(self), + ); + + Ok(ConsensusValidationResult::new_with_data_and_errors( + bump_action, + vec![error.into()], + )) + } + + // Then use it like this in validation: + if !keyword.is_ascii() { + return self.create_keyword_validation_error( + ConsensusError::BasicError( + BasicError::InvalidKeywordEncodingError(InvalidKeywordEncodingError::new( + self.data_contract().id().to_string(Encoding::Base58), + keyword.to_string(), + )), + ), + ); + }packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/creation.rs (3)
2574-2574: Remove debug print statement before merging.The
println!statement should be removed as it's not needed in the final test code.- println!("Processing result: {:?}", processing_result);
2532-2535: Consider testing with multiple keywords to verify the limit enforcement.According to the PR description, contracts can have up to 20 unique keywords. Consider enhancing this test or adding another test to verify multiple keywords handling and uniqueness constraints.
- document.set("keyword", "meme".into()); + document.set("keyword", vec!["meme", "search", "platform", "test"].into()); document.set("contractId", Identifier::random().into());
2473-2475: Add a comment explaining the system design restriction.Consider adding a doc comment that explains why direct document creation on the search contract is restricted and what the proper path for creating these documents should be (presumably during contract registration).
#[test] + /// Tests that documents cannot be created directly on the Search system contract, + /// as documents should only be created through contract registration processes + /// that properly validate and index the keywords. fn test_document_creation_on_search_system_contract_fails_due_to_restriction() {packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_create/mod.rs (2)
2843-2959: Successfully verifies positive case with valid keywordsThis test properly verifies that contracts with valid keywords are successfully created and that the keywords are correctly stored in the contract.
Consider simplifying the data contract ID extraction logic at lines 2933-2944. The comment "Is there a simpler way to get the ID?" suggests there might be a more direct approach to access the contract ID after creation. A helper method could make this more readable and less error-prone.
- // Get the data contract ID from the transition - // Is there a simpler way to get the ID? - let unique_identifiers = data_contract_create_transition.unique_identifiers(); - let unique_identifier = unique_identifiers - .first() - .expect("expected at least one unique identifier"); - let unique_identifier_str = unique_identifier.as_str(); - let data_contract_id_str = unique_identifier_str - .split('-') - .last() - .expect("expected to extract data contract id from unique identifier"); - let data_contract_id = Identifier::from_string(data_contract_id_str, Encoding::Base58) - .expect("failed to create Identifier from string"); + // Get the data contract ID directly from the transition + let data_contract_id = data_contract_create_transition.data_contract().id();
2451-2960: Consider adding boundary test cases for keyword constraintsThe tests cover all the constraints well, but adding tests for boundary cases would improve coverage further.
Consider adding tests for:
- Exactly 20 keywords (the maximum limit)
- Keywords with exactly 3 characters (minimum length)
- Keywords with exactly 50 characters (maximum length)
These boundary cases would ensure that the validation logic handles edge cases correctly.
packages/rs-dpp/src/errors/consensus/basic/data_contract/invalid_keyword_length_error.rs (1)
9-21: Struct design and length validationThe derived traits and descriptive error message look consistent. However, consider referencing a centralized constant for the 3–50 range to avoid magic numbers in multiple places.
packages/rs-dpp/src/errors/consensus/basic/data_contract/keywords_over_limit.rs (1)
9-21: Struct design and max keywords constraintThe custom error clearly specifies the maximum of 20 keywords. Consider referencing a shared constant instead of a hardcoded value to improve maintainability.
packages/rs-drive/src/drive/contract/insert/add_new_keywords/mod.rs (1)
17-53: Version handling mismatchThe
matchstatement handles versions0and1, butknown_versionsincludes only[0]. Consider listing1withinknown_versionsfor clarity and to avoid confusion.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (55)
.pnp.cjs(3 hunks)Cargo.toml(1 hunks)Dockerfile(13 hunks)package.json(1 hunks)packages/data-contracts/Cargo.toml(1 hunks)packages/data-contracts/src/error.rs(1 hunks)packages/data-contracts/src/lib.rs(4 hunks)packages/rs-dpp/schema/meta_schemas/document/v0/document-meta.json(4 hunks)packages/rs-dpp/src/data_contract/accessors/mod.rs(3 hunks)packages/rs-dpp/src/data_contract/accessors/v1/mod.rs(2 hunks)packages/rs-dpp/src/data_contract/mod.rs(1 hunks)packages/rs-dpp/src/data_contract/serialized_version/mod.rs(2 hunks)packages/rs-dpp/src/data_contract/serialized_version/v1/mod.rs(4 hunks)packages/rs-dpp/src/data_contract/v1/accessors/mod.rs(2 hunks)packages/rs-dpp/src/data_contract/v1/data_contract.rs(2 hunks)packages/rs-dpp/src/data_contract/v1/serialization/mod.rs(3 hunks)packages/rs-dpp/src/errors/consensus/basic/basic_error.rs(2 hunks)packages/rs-dpp/src/errors/consensus/basic/data_contract/duplicate_keywords_error.rs(1 hunks)packages/rs-dpp/src/errors/consensus/basic/data_contract/invalid_keyword_encoding_error.rs(1 hunks)packages/rs-dpp/src/errors/consensus/basic/data_contract/invalid_keyword_length_error.rs(1 hunks)packages/rs-dpp/src/errors/consensus/basic/data_contract/keywords_over_limit.rs(1 hunks)packages/rs-dpp/src/errors/consensus/basic/data_contract/mod.rs(4 hunks)packages/rs-dpp/src/errors/consensus/codes.rs(1 hunks)packages/rs-drive-abci/src/execution/check_tx/v0/mod.rs(4 hunks)packages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/v1/mod.rs(1 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/tests/document/creation.rs(2 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_create/advanced_structure/v0/mod.rs(2 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_create/mod.rs(1 hunks)packages/rs-drive-abci/tests/supporting_files/contract/keyword_test/keyword_base_contract.json(1 hunks)packages/rs-drive/src/cache/system_contracts.rs(3 hunks)packages/rs-drive/src/drive/contract/insert/add_new_keywords/mod.rs(1 hunks)packages/rs-drive/src/drive/contract/insert/add_new_keywords/v0/mod.rs(1 hunks)packages/rs-drive/src/drive/contract/insert/insert_contract/v1/mod.rs(1 hunks)packages/rs-drive/src/drive/contract/insert/mod.rs(1 hunks)packages/rs-drive/src/drive/group/prove/prove_action_infos/v0/mod.rs(2 hunks)packages/rs-drive/src/drive/group/prove/prove_action_signers/v0/mod.rs(3 hunks)packages/rs-drive/src/drive/group/prove/prove_group_info/v0/mod.rs(2 hunks)packages/rs-drive/src/drive/group/prove/prove_group_infos/v0/mod.rs(2 hunks)packages/rs-drive/src/drive/tokens/balance/prove_identities_token_balances/v0/mod.rs(3 hunks)packages/rs-drive/src/drive/tokens/balance/prove_identity_token_balances/v0/mod.rs(3 hunks)packages/rs-drive/src/drive/tokens/info/prove_identities_token_infos/v0/mod.rs(3 hunks)packages/rs-drive/src/drive/tokens/info/prove_identity_token_infos/v0/mod.rs(2 hunks)packages/rs-drive/src/drive/tokens/status/prove_token_statuses/v0/mod.rs(1 hunks)packages/rs-drive/src/drive/tokens/system/prove_token_total_supply_and_aggregated_identity_balances/v0/mod.rs(2 hunks)packages/rs-platform-version/src/version/drive_versions/drive_contract_method_versions/v2.rs(1 hunks)packages/rs-platform-version/src/version/system_data_contract_versions/mod.rs(1 hunks)packages/rs-platform-version/src/version/system_data_contract_versions/v1.rs(1 hunks)packages/search-contract/Cargo.toml(1 hunks)packages/search-contract/lib/systemIds.js(1 hunks)packages/search-contract/schema/v1/search-contract-documents.json(1 hunks)packages/search-contract/src/lib.rs(2 hunks)packages/search-contract/src/v1/mod.rs(0 hunks)packages/token-history-contract/src/lib.rs(1 hunks)packages/wasm-dpp/src/errors/consensus/consensus_error.rs(2 hunks)
💤 Files with no reviewable changes (1)
- packages/search-contract/src/v1/mod.rs
🧰 Additional context used
🧬 Code Definitions (12)
packages/data-contracts/src/error.rs (1)
packages/rs-dpp/src/data_contract/serialized_version/v1/mod.rs (1)
from(100-175)
packages/rs-dpp/src/data_contract/serialized_version/v1/mod.rs (3)
packages/rs-dpp/src/data_contract/accessors/v1/mod.rs (1)
keywords(68-68)packages/rs-dpp/src/data_contract/v1/accessors/mod.rs (1)
keywords(229-231)packages/rs-dpp/src/data_contract/accessors/mod.rs (1)
keywords(273-278)
packages/rs-drive-abci/src/execution/check_tx/v0/mod.rs (1)
packages/rs-drive-abci/src/platform_types/state_transitions_processing_result/mod.rs (1)
aggregated_fees(99-101)
packages/rs-dpp/src/data_contract/v1/accessors/mod.rs (2)
packages/rs-dpp/src/data_contract/accessors/v1/mod.rs (3)
keywords(68-68)keywords_mut(71-71)set_keywords(106-106)packages/rs-dpp/src/data_contract/accessors/mod.rs (3)
keywords(273-278)keywords_mut(280-285)set_keywords(420-424)
packages/rs-dpp/src/data_contract/accessors/mod.rs (2)
packages/rs-dpp/src/data_contract/accessors/v1/mod.rs (3)
keywords(68-68)keywords_mut(71-71)set_keywords(106-106)packages/rs-dpp/src/data_contract/v1/accessors/mod.rs (3)
keywords(229-231)keywords_mut(234-236)set_keywords(287-289)
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_create/advanced_structure/v0/mod.rs (3)
packages/rs-dpp/src/errors/consensus/basic/data_contract/invalid_keyword_encoding_error.rs (2)
new(24-29)keyword(35-37)packages/rs-dpp/src/errors/consensus/basic/data_contract/invalid_keyword_length_error.rs (2)
new(24-29)keyword(35-37)packages/rs-dpp/src/data_contract/serialized_version/mod.rs (2)
id(53-58)keywords(102-107)
packages/rs-dpp/src/data_contract/accessors/v1/mod.rs (2)
packages/rs-dpp/src/data_contract/v1/accessors/mod.rs (3)
keywords(229-231)keywords_mut(234-236)set_keywords(287-289)packages/rs-dpp/src/data_contract/accessors/mod.rs (3)
keywords(273-278)keywords_mut(280-285)set_keywords(420-424)
packages/rs-dpp/src/errors/consensus/basic/data_contract/invalid_keyword_length_error.rs (2)
packages/rs-dpp/src/errors/consensus/basic/data_contract/invalid_keyword_encoding_error.rs (4)
new(24-29)contract_id(31-33)keyword(35-37)from(41-43)packages/rs-dpp/src/errors/consensus/basic/basic_error.rs (1)
from(537-539)
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_create/mod.rs (1)
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_update/mod.rs (1)
setup_identity(103-159)
packages/rs-drive/src/drive/contract/insert/add_new_keywords/v0/mod.rs (3)
packages/rs-dpp/src/data_contract/v1/accessors/mod.rs (2)
version(33-35)keywords(229-231)packages/rs-dpp/src/data_contract/accessors/mod.rs (2)
version(41-46)keywords(273-278)packages/rs-dpp/src/data_contract/accessors/v1/mod.rs (1)
keywords(68-68)
packages/rs-dpp/src/errors/consensus/basic/data_contract/invalid_keyword_encoding_error.rs (2)
packages/rs-dpp/src/errors/consensus/basic/data_contract/invalid_keyword_length_error.rs (4)
new(24-29)contract_id(31-33)keyword(35-37)from(41-43)packages/rs-dpp/src/errors/consensus/basic/basic_error.rs (1)
from(537-539)
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/creation.rs (5)
packages/rs-dpp/src/data_contract/document_type/accessors/v0/mod.rs (1)
creation_restriction_mode(74-74)packages/rs-dpp/src/data_contract/document_type/v0/accessors.rs (1)
creation_restriction_mode(98-100)packages/rs-dpp/src/data_contract/document_type/v1/accessors.rs (1)
creation_restriction_mode(100-102)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/mod.rs (1)
new_document_creation_transition_from_document(78-137)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v0_methods.rs (1)
new_document_creation_transition_from_document(131-171)
⏰ Context from checks skipped due to timeout of 90000ms (13)
- GitHub Check: Rust packages (wasm-dpp) / Detect immutable structure changes
- GitHub Check: Rust packages (wasm-dpp) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (wasm-dpp) / Formatting
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (token-history-contract) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (token-history-contract) / Tests
- GitHub Check: Rust packages (token-history-contract) / Linting
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- 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 JS packages / Build JS
🔇 Additional comments (100)
packages/data-contracts/Cargo.toml (1)
21-21: LGTM: Adding the search-contract dependency.This change appropriately adds the dependency to the search-contract package, allowing the data-contracts package to utilize the keyword search functionality as described in the PR objectives.
packages/rs-drive/src/drive/tokens/status/prove_token_statuses/v0/mod.rs (1)
97-97:❓ Verification inconclusive
Code change looks good, but consider adding keyword-specific tests.
The addition of the
keywordsfield to the DataContractV1 struct in the test is appropriate and aligns with the PR objective of implementing keyword support. However, this test doesn't specifically verify the keyword functionality.
🏁 Script executed:
#!/bin/bash # Check if there are tests specifically for the keywords functionality echo "Searching for tests related to keywords functionality..." rg -i "test.*keyword" --type rustLength of output: 4876
Action Required: Review Keyword Test Coverage
The code change to add the
keywords: Vec::new(),entry in theDataContractV1struct is correctly aligned with our objective of implementing keyword support. Our verification shows that several keyword-specific tests already exist in the repository (for example, in the data contract creation tests in packages/rs-drive-abci and the JSON schema compatibility tests) that validate conditions such as duplicate keywords and boundary constraints.
- Location:
packages/rs-drive/src/drive/tokens/status/prove_token_statuses/v0/mod.rs(line 97)- Observation: The current test in this module sets the default keyword value, but does not directly assert keyword-related behaviours.
- Suggestion: Consider adding a targeted test or extending this test to explicitly verify keyword functionality within this module to ensure full coverage — unless the existing integration tests are deemed sufficient.
packages/rs-drive/src/drive/group/prove/prove_group_info/v0/mod.rs (1)
109-109: Addition of keywords field is consistent across contract definitions.The
keywords: Vec::new()field has been correctly added to both DataContractV1 instances in this test file, maintaining consistency with the contract structure changes described in the PR objectives.Also applies to: 179-179
Cargo.toml (1)
33-33: LGTM: Adding search-contract package to workspace.This change appropriately adds the new search-contract package to the workspace members, enabling it to be built and managed along with other packages in the project.
packages/token-history-contract/src/lib.rs (1)
19-19: Fix inconsistent contract reference in match statementsThe code previously referenced
withdrawalsbut now correctly referencestoken_historyin the match statements and error messages. This aligns with the actual contract being used.Also applies to: 22-22, 29-29, 32-32
package.json (1)
70-71: Workspace configuration updated for search contractThe workspace array now includes the new
packages/search-contractpackage, ensuring it's properly integrated into the build and test workflows.packages/rs-drive/src/drive/tokens/info/prove_identity_token_infos/v0/mod.rs (2)
108-108: Added new keywords field to match contract structureThe
keywordsfield has been added to the DataContract test structure to align with the changes in the contract schema. This ensures that tests accurately reflect the new contract structure with keyword search capabilities.
256-256: Added new keywords field to match contract structureThe
keywordsfield has been consistently added to all DataContract test instances, keeping test data aligned with the updated contract schema.packages/rs-drive/src/drive/tokens/balance/prove_identities_token_balances/v0/mod.rs (1)
100-100: Added new keywords field to DataContract test structuresThe
keywordsfield has been consistently added to all DataContract test instances. This maintains consistency with the contract schema changes that support the new keyword search functionality.Also applies to: 201-201, 293-293
packages/rs-drive/src/drive/tokens/info/prove_identities_token_infos/v0/mod.rs (1)
100-100: Properly updated test fixtures with new keywords fieldAll instances of
DataContractV1in the test cases have been correctly updated to include the newkeywordsfield, maintaining consistency with the implementation changes for keyword search functionality.Also applies to: 201-201, 291-291
packages/rs-drive/src/drive/group/prove/prove_action_signers/v0/mod.rs (1)
151-151: Test fixtures correctly updated with keywords fieldAll test instances of the
DataContractV1struct have been properly updated to include the newkeywordsfield, maintaining compatibility with the keyword search system contract implementation.Also applies to: 320-320, 414-414
packages/rs-drive/src/drive/group/prove/prove_group_infos/v0/mod.rs (1)
137-137: Test fixtures correctly updated with keywords fieldThe new
keywordsfield has been properly added to allDataContractV1instances in the test cases, ensuring compatibility with the keyword search implementation.Also applies to: 282-282
packages/rs-drive/src/drive/tokens/balance/prove_identity_token_balances/v0/mod.rs (1)
105-105: Test fixtures properly updated for keywords supportAll test instances of
DataContractV1have been correctly updated to include the newkeywordsfield, ensuring compatibility with the keyword search system implementation.Also applies to: 266-266, 354-354
Dockerfile (6)
112-115: Consistent Formatting for Rust Toolchain Installation
The reformatting of the Rust toolchain installation options (e.g. using--profile minimal,-y, etc.) improves clarity. Please confirm that these options match your intended build configuration.
382-384: Integrate Search-Contract in Build-Planner Stage
The addition of thepackages/search-contractentry in the COPY command (line 383) ensures that the new keyword search system contract is included in the build-planner stage. Verify that the path/platform/packages/search-contractis correct and that any dependent build artifacts for the search contract are generated prior to this stage.
456-460: Include Search-Contract in Drive-ABCI Build
The updated COPY command now includespackages/search-contract(at line 459). This change is consistent with the PR objective of integrating the keyword search system contract into the drive-ABCI image. Please verify that this inclusion is required and does not lead to redundant copies in later stages.
555-557: Ensure Search-Contract Availability for JavaScript Build
The addition of thepackages/search-contractline in the COPY command for the JavaScript-related build stage ensures that the search contract package is available to JavaScript components. Double-check that any JS-specific build or runtime configurations for the search contract are handled elsewhere if needed.
680-680: Dashmate Helper Stage: Importing Search-Contract
The COPY command in the dashmate-helper stage (line 680) now includes thepackages/search-contractpackage. This ensures that the helper image incorporates the keyword search functionality. Please confirm that the destination path (packages/search-contract) aligns with the helper’s expected directory structure.
757-758: DAPI Image: Inclusion of Search-Contract Package
The dapi stage now copies thepackages/search-contractpackage into the final DAPI image (line 757). This integration is crucial for ensuring that the DAPI component is aware of the new search system contract. Confirm that any runtime dependencies (e.g. configuration or environment variables) specific to the search contract are properly set.packages/rs-drive-abci/src/execution/check_tx/v0/mod.rs (4)
633-633: Processing fee updated to match new contract implementation.The assertion for
processing_feehas been increased from 2488410 to 2488810, which aligns with the additional cost of supporting the new keyword search functionality in data contracts.
1144-1144: Processing fee updated for user fee increase scenario.The assertion for
processing_feehas been increased from 4976820 to 4977620, correctly accounting for both the new keyword search functionality cost and the user fee increase multiplier (100) applied to the transaction.
1616-1616: Processing fee updated for data contract update operation.The assertion for
processing_feehas been increased from 2488410 to 2488810, matching the additional cost introduced by the keyword search functionality.
2072-2072: Processing fee updated for invalid contract update scenario.The assertion for
processing_feehas been increased from 2488410 to 2488810, maintaining consistency with the other test cases after the introduction of the keyword search functionality.packages/rs-drive/src/drive/tokens/system/prove_token_total_supply_and_aggregated_identity_balances/v0/mod.rs (2)
91-91: LGTM: Keywords field correctly added to DataContractV1The addition of the
keywordsfield initialized as an empty vector is consistent with the implementation of the keyword search system contract. This change ensures the test fixtures remain compatible with the updatedDataContractV1structure.
217-217: LGTM: Keywords field correctly added to DataContractV1 in second testThe addition of the
keywordsfield is consistent with the first test case and ensures the test fixture remains compatible with the updated contract structure. This change has no impact on the test logic which focuses on token supply and balances.packages/search-contract/src/lib.rs (2)
19-19: LGTM: Correctly using search contract versionThe change correctly updates the version check to use
platform_version.system_data_contracts.searchinstead of withdrawals, which aligns with the implementation of the new keyword search system contract.
29-29: LGTM: Consistent version checking for search contractThis change correctly updates the second version check to use the search contract version, maintaining consistency with the previous change. The error handling for unknown versions is appropriate.
.pnp.cjs (3)
68-71: Addition of @dashevo/search-contract Dependency ObjectThe new dependency object is properly structured and inserted within the dependency list. It correctly specifies the package name and workspace reference following the established format.
108-108: Insertion of search-contract in Dependency TuplesThe new tuple for
@dashevo/search-contracthas been added to the dependency arrays with the proper workspace reference. This change is consistent with the existing entries.
2887-2904: Integration of Detailed Package Configuration for search-contractThe detailed configuration block for
@dashevo/search-contractis comprehensive:
- It correctly sets the
packageLocationand lists all necessary package dependencies.- The
linkTypeis appropriately set to "SOFT", aligning with the intended dependency resolution behavior.Please verify that the versions and virtual references (e.g., for
dirty-chai,eslint-config-airbnb-base, and others) are consistent with your internal dependency management policies.packages/rs-drive/src/drive/contract/insert/mod.rs (1)
2-2: LGTM: New module for keyword functionalityThis addition of
add_new_keywordsmodule aligns with the PR objective of implementing a keyword search system contract. The module is appropriately placed within the contract insertion flow.packages/search-contract/Cargo.toml (1)
3-3: Grammar fix in package descriptionThe description has been corrected to use proper grammar ("is used to" instead of "is using to"), which improves readability.
packages/search-contract/lib/systemIds.js (1)
3-3:✅ Verification successful
Verify that this contract ID is correctly generated
The contract ID for the search contract has been updated. Make sure this ID is correctly generated and consistent with other system identifiers.
🏁 Script executed:
#!/bin/bash # Check if this contract ID is referenced correctly in other files rg -l "7CSFGeF4WNzgDmx94zwvHkYaG3Dx4XEe5LFsFgJswLbm" --type js --type rustLength of output: 164
Contract ID Consistency Verified
- The contract ID "7CSFGeF4WNzgDmx94zwvHkYaG3Dx4XEe5LFsFgJswLbm" appears in both
packages/search-contract/lib/systemIds.jsandpackages/wallet-utils-contract/lib/systemIds.js, confirming consistent usage across modules.- No discrepancies were found in its generation and reference. Please ensure that this ID remains up to date with any further changes to the generation logic.
packages/rs-platform-version/src/version/system_data_contract_versions/v1.rs (1)
11-12: LGTM: Added version information for new system contractsThe addition of version information for both token_history and search contracts is appropriate. Both are initialized with version 1, which is correct for new system contracts.
packages/rs-dpp/src/data_contract/mod.rs (1)
76-80: LGTM: New static variable for empty keywords collectionThe added
EMPTY_KEYWORDSstatic variable follows the existing pattern for empty collections and is properly documented in the updated comment. This will serve as a useful default value for contract keywords.packages/rs-drive/src/drive/group/prove/prove_action_infos/v0/mod.rs (2)
156-157: LGTM: Keywords field added to test DataContractThe new
keywordsfield is correctly initialized as an empty vector, which aligns with the addition of keyword functionality to data contracts.
384-385: LGTM: Keywords field consistently added to all test DataContract instancesThe change consistently adds the required
keywordsfield to all test instances ofDataContractV1, ensuring tests remain valid with the updated contract structure.packages/data-contracts/src/error.rs (1)
140-155: LGTM: Added error handling for search contractThe implementation correctly follows the established pattern for contract error handling, mapping
search_contract::Errorvariants to the localErrorenum. This enables proper integration of the new search contract with the existing error handling system.packages/rs-dpp/schema/meta_schemas/document/v0/document-meta.json (3)
230-233: LGTM: Improved formatting for required propertiesThe multi-line formatting improves readability of the required properties array.
425-428: LGTM: Consistent formatting improvementThis formatting change maintains consistency with the earlier formatting improvements.
587-597: LGTM: Well-defined schema for the new keywords propertyThe keywords property is properly defined with appropriate constraints:
- Description clearly explains the purpose
- String items with min/max length constraints (3-50 chars)
- Maximum of 20 items as specified in requirements
- Uniqueness enforced with uniqueItems
This aligns perfectly with the PR objectives for implementing keyword search functionality.
packages/rs-dpp/src/data_contract/v1/serialization/mod.rs (3)
114-114: LGTM: Properly initialized keywords field for v0 format upgradeThe keywords field is correctly initialized with the default value (empty vector) when converting from v0 format, which maintains backward compatibility with older contract versions.
141-141: LGTM: Added keywords to DataContractInSerializationFormatV1 destructuringThe keywords field is appropriately extracted from the input data contract format structure.
170-170: LGTM: Properly propagated keywords to DataContractV1 constructionThe keywords field is correctly included in the construction of the DataContractV1 structure.
packages/rs-drive/src/drive/contract/insert/insert_contract/v1/mod.rs (1)
310-320: LGTM: Keywords insertion follows established patternThe implementation for handling keywords follows the same pattern used for groups and tokens, correctly checking if keywords exist before attempting to add them to the batch operations.
packages/rs-platform-version/src/version/system_data_contract_versions/mod.rs (1)
13-14: LGTM: Added required versioning support for new featuresThe addition of token_history and search feature versions allows proper feature detection and versioning across the platform.
packages/rs-dpp/src/data_contract/serialized_version/mod.rs (2)
24-24: LGTM: Correctly imported EMPTY_KEYWORDS constantProperly imported the EMPTY_KEYWORDS constant for use with version 0 contracts.
102-107: LGTM: Accessor method follows established patternThe implementation of the keywords accessor method follows the same pattern as other accessors in this file, correctly returning EMPTY_KEYWORDS for v0 format and the actual keywords for v1 format.
packages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/v1/mod.rs (1)
54-57: Integrating Search system contract looks good!The addition of the Search system data contract to the genesis state matches the pattern of other system data contracts and properly integrates with the existing initialization process.
packages/rs-dpp/src/data_contract/v1/data_contract.rs (2)
64-68: Documentation for keywords field is clear and comprehensive.The documentation clearly explains the purpose and constraints of the keywords field, including the uniqueness requirement and the maximum limit of 20 keywords. This provides good guidance for developers using this feature.
111-112: Keywords field implementation looks good.The implementation of the keywords field as a Vec is appropriate for storing a collection of searchable keywords.
packages/rs-drive-abci/tests/supporting_files/contract/keyword_test/keyword_base_contract.json (1)
1-40: Test contract structure is well defined.This base contract provides a good foundation for testing the keyword functionality. It includes all the necessary fields with an empty keywords array that can be populated in tests.
packages/rs-dpp/src/errors/consensus/codes.rs (1)
109-112: Keyword validation error codes are appropriate.The error types for keyword validation follow a logical sequence and address all necessary validation scenarios:
- Too many keywords (>20)
- Duplicate keywords
- Invalid keyword length
- Invalid keyword encoding
These align well with the documented constraints for the keywords field.
packages/rs-dpp/src/data_contract/v1/accessors/mod.rs (2)
228-236: Implementation of keyword accessor methods looks good.The addition of
keywordsandkeywords_mutmethods to theDataContractV1Gettersimplementation correctly exposes access to the contract's keywords vector, aligning with the keyword search functionality described in the PR objectives.
286-289: Implementation of keyword setter method looks good.The
set_keywordsmethod properly enables setting keywords for the contract, which is essential for the contract keyword search system. The implementation is consistent with other setters in the codebase.packages/rs-dpp/src/data_contract/accessors/v1/mod.rs (2)
67-71: Trait definition for keyword getters looks good.The addition of
keywordsandkeywords_mutmethods to theDataContractV1Getterstrait correctly defines the interface for accessing the keywords associated with a data contract.
105-106: Trait definition for keyword setter looks good.The addition of
set_keywordsmethod to theDataContractV1Setterstrait properly defines the interface for setting keywords on a data contract.packages/rs-dpp/src/data_contract/serialized_version/v1/mod.rs (3)
63-65: Addition of keywords field for serialization looks good.The
keywordsfield is properly annotated with#[serde(default)]to ensure it's initialized as an empty vector when not specified during deserialization.
131-131: Proper default initialization for V0 contracts.Correctly initializes keywords to the default value (empty vector) when converting from a V0 contract, which doesn't support keywords.
150-150: Proper keyword field handling in From implementation.The implementation correctly extracts and transfers the keywords field when converting from a V1 contract to the serialization format.
Also applies to: 171-171
packages/rs-dpp/src/data_contract/accessors/mod.rs (3)
21-21: Import of EMPTY_KEYWORDS looks good.The import of
EMPTY_KEYWORDSis appropriate for providing a default empty vector for V0 contracts that don't support keywords.
273-285: Implementation of keyword accessors for DataContract enum looks good.The implementation correctly handles both contract versions:
- For V0 contracts, returns an empty keywords list
- For V1 contracts, returns a reference to the actual keywords field
This approach ensures backward compatibility while fully supporting the new keywords feature.
419-424: Implementation of set_keywords for DataContract enum looks good.The method correctly handles both contract versions:
- For V0 contracts, it does nothing (as keywords aren't supported)
- For V1 contracts, it sets the keywords field
This pattern is consistent with other setters in the codebase.
packages/rs-dpp/src/errors/consensus/basic/basic_error.rs (2)
78-81: LGTM: Imports for new keyword-related error types.The code correctly imports the keyword validation error types that will be used in the new error variants.
523-533: New error variants for keyword validation.Good addition of error types to handle the various keyword validation scenarios (too many keywords, duplicates, invalid length, invalid encoding). The error variants follow the existing pattern of using
#[error(transparent)]attribute.Note that the comment on line 104 mentions "DO NOT CHANGE ORDER OF VARIANTS WITHOUT INTRODUCING OF NEW VERSION" - since you're adding to the end of the enum, this shouldn't cause issues, but be aware that these changes may impact serialization/deserialization if consumers are using a numeric representation of error types.
packages/rs-drive/src/cache/system_contracts.rs (3)
20-21: LGTM: Adding Search contract field.Proper addition of the search contract field with clear documentation comment.
50-53: LGTM: Initializing Search contract.Correct initialization of the search contract field following the same pattern as other system contracts.
82-85: LGTM: Adding accessor for Search contract.Added a properly documented public method to access the search contract, following the same pattern as other system contract accessors.
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_create/advanced_structure/v0/mod.rs (2)
1-2: LGTM: Adding imports for keyword validation.Appropriate imports for HashSet and the necessary error types for keyword validation.
Also applies to: 8-9, 13-13, 15-15, 18-18
157-172: Validate maximum keyword count.Good implementation of the validation check to ensure there are no more than 20 keywords in a data contract, returning a TooManyKeywordsError when the limit is exceeded.
packages/search-contract/schema/v1/search-contract-documents.json (2)
24-32: LGTM: Added contractId property to search schema.The contractId property is correctly defined as a byte array with appropriate constraints for DPP identifiers.
34-37: Updated required fields to use contractId.The required fields list correctly includes "contractId" and "keyword".
packages/rs-drive-abci/src/execution/platform_events/protocol_upgrade/perform_events_on_first_block_of_protocol_change/v0/mod.rs (2)
478-487: Improved variable naming for better clarityThe renaming of the variable from
contracttotoken_history_contractenhances code readability by clearly identifying the purpose of the loaded contract.
489-498: Implementation of search contract integration looks goodAdding the search contract loading and insertion following the same pattern as other system contracts ensures consistency in the codebase. This change properly integrates the new keyword search functionality during protocol transitions.
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/creation.rs (1)
2473-2606: Test implementation correctly verifies search contract document creation restriction.This test properly verifies that the search system contract doesn't allow direct document creation through the
NoCreationAllowedrestriction mode, which is essential for the keyword search system contract security model.packages/wasm-dpp/src/errors/consensus/consensus_error.rs (2)
64-64: New data contract error types properly importedThe import statement correctly includes new error types for keyword validation features, including
DuplicateKeywordsError,TooManyKeywordsError,InvalidKeywordLengthError, andInvalidKeywordEncodingError. This supports the PR objective of implementing keyword search system contracts.
764-775: Error handling properly implemented for keyword validationThe new match arms correctly handle the keyword validation errors introduced by the keyword search system contract feature. Each error type is properly converted to a JavaScript value using the
generic_consensus_error!macro, maintaining consistency with the existing error handling pattern in the codebase.packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_create/mod.rs (6)
2451-2459: Implementation of keyword validation tests looks goodThis new module adds comprehensive test coverage for the keywords functionality, aligning with the PR objectives of implementing a keyword search system.
2459-2539: Thorough validation test for keyword count limitThe test correctly verifies that data contracts with more than 20 keywords are rejected with the appropriate error.
2542-2624: Proper validation of duplicate keywordsThis test correctly ensures that contracts with duplicate keywords fail validation, enforcing the uniqueness constraint mentioned in the PR objectives.
2627-2701: Good minimum length validation for keywordsThis test ensures that keywords shorter than 3 characters are rejected, which is an important constraint for meaningful search terms.
2704-2771: Appropriate maximum length validation for keywordsThe test correctly verifies that keywords longer than 50 characters are rejected, preventing excessive keyword lengths.
2774-2840: Proper encoding validation for keywordsThis test ensures that keywords must be ASCII-only, which is important for consistent indexing and searching.
packages/data-contracts/src/lib.rs (5)
12-12: LGTM: New search contract module imported correctlyThe search contract module is properly imported and exposed as a public module, following the established pattern for system data contracts.
27-27: LGTM: New System Data Contract enum variant addedThe
Searchvariant is correctly added as the next sequential value (7) in theSystemDataContractenum.
48-48: LGTM: Search contract ID bytes correctly integratedThe ID bytes match pattern is properly extended to include the new Search contract variant.
104-104: Bug fix: Corrected version assignment for TokenHistoryFixed an incorrect version assignment that was using wallet version instead of token_history version.
108-114: LGTM: Search contract source implementationThe implementation of the Search contract source is consistent with other system data contracts, properly retrieving ID bytes, owner ID bytes, version, definitions, and document schemas from the search_contract module.
packages/rs-dpp/src/errors/consensus/basic/data_contract/duplicate_keywords_error.rs (1)
1-44: LGTM: Well-structured error type for duplicate keywordsThis error type follows the established pattern for consensus errors, with proper implementation of traits, field getters, and conversion to ConsensusError.
The error message is clear and provides context about which contract has the duplicate keyword. The warning comment about field order is important for serialization compatibility.
packages/rs-dpp/src/errors/consensus/basic/data_contract/invalid_keyword_encoding_error.rs (1)
1-44: LGTM: Well-structured error type for invalid keyword encodingThis error type is correctly implemented following the established pattern, with appropriate traits, getters, and ConsensusError conversion.
The error message is informative, explaining both the issue and the requirement (keywords must be ASCII).
packages/rs-dpp/src/errors/consensus/basic/data_contract/mod.rs (2)
12-12: LGTM: New module declarations for keyword-related errorsThe new module declarations are correctly placed and follow the established naming convention.
Also applies to: 30-31, 37-37
76-76: LGTM: Proper public exports for keyword-related errorsThe public use declarations correctly expose the new error types, maintaining alphabetical ordering and following the established pattern.
Also applies to: 85-86, 92-92
packages/rs-dpp/src/errors/consensus/basic/data_contract/invalid_keyword_length_error.rs (1)
40-44: Conversion viaFromtrait is consistentWrapping the error in
ConsensusError::BasicErrorfollows the project's established error handling pattern. No issues found.packages/rs-dpp/src/errors/consensus/basic/data_contract/keywords_over_limit.rs (1)
40-44:Fromtrait usage is consistentThe conversion to
ConsensusError::BasicErroris properly integrated.packages/rs-drive/src/drive/contract/insert/add_new_keywords/mod.rs (2)
55-93: Implementation logic appears straightforwardAdding creation operations through version-based routing is consistent. Ensure future versions are documented to maintain clarity.
95-133: Method naming is consistent and clear
add_new_contract_keywords_operationseffectively communicates its purpose. No performance or correctness concerns.packages/rs-drive/src/drive/contract/insert/add_new_keywords/v0/mod.rs (4)
18-48: Conditional application logicUsing
applyto determine whether to estimate costs or finalize them is straightforward. The approach ensures minimal overhead unless changes are actually applied.
50-86: Batch operation integrationAdding operations to
drive_operationswhile handling cost estimation or application ensures consistent usage of the Drive's batching mechanism.
88-135: Efficient looping over keywordsThe loop enumerates each keyword and constructs operations. Since there's a maximum of 20 keywords, performance is unlikely to be an issue.
137-175: Creation of keyword Document
build_contract_keyword_document_owned_v0systematically builds aDocumentwith relevant properties. This method supports future extension if additional metadata is required.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
packages/rs-drive/tests/deterministic_root_hash.rs (1)
285-286: Consider whether this debug print is necessaryDebug print statements like this can be useful during development but may add noise to test output. Consider removing it or wrapping it with a debug flag/condition if it's not essential for understanding test failures.
- println!("Dashpay contract: {:?}", dashpay_contract); + // println!("Dashpay contract: {:?}", dashpay_contract); // Uncomment for debuggingpackages/rs-drive/src/drive/contract/insert/add_new_keywords/v0/mod.rs (5)
22-22: Consider using&[String]instead of&Vec<String>for parameter typeUsing slice syntax (
&[String]) instead of&Vec<String>provides more flexibility as it allows accepting any type that can be borrowed as a slice, not justVec<String>. This is a common Rust best practice for function arguments.- pub(super) fn add_new_contract_keywords_v0( - &self, - contract_id: Identifier, - owner_id: Identifier, - keywords: &Vec<String>, + pub(super) fn add_new_contract_keywords_v0( + &self, + contract_id: Identifier, + owner_id: Identifier, + keywords: &[String],
55-55: Consider using&[String]instead of&Vec<String>for parameter typeSame as above, using slice syntax provides more flexibility and is considered idiomatic Rust.
- pub(super) fn add_new_contract_keywords_add_to_operations_v0( - &self, - contract_id: Identifier, - owner_id: Identifier, - keywords: &Vec<String>, + pub(super) fn add_new_contract_keywords_add_to_operations_v0( + &self, + contract_id: Identifier, + owner_id: Identifier, + keywords: &[String],
141-141: Consider using&strinstead of&Stringfor parameter typeUsing
&strinstead of&Stringis more flexible as it can accept bothStringand string literals, which is a Rust best practice.- pub(super) fn build_contract_keyword_document_owned_v0( - &self, - contract_id: Identifier, - owner_id: Identifier, - keyword: &String, + pub(super) fn build_contract_keyword_document_owned_v0( + &self, + contract_id: Identifier, + owner_id: Identifier, + keyword: &str,
153-153: Reference string conversion to avoid unnecessary cloningOn line 153,
keyword.into()creates a clone of the string. Sincekeywordis already a reference, consider usingkeyword.to_string().into()orkeyword.as_str().into()to avoid potentially unnecessary allocations.- ("keyword".to_string(), keyword.into()), + ("keyword".to_string(), keyword.as_str().into()),
16-176: Function documentation is good but could be enhancedThe function documentation comments are helpful, but could be enhanced with more details about the purpose, parameters, and return values. Consider adding Rust doc-style comments (
///) with parameter descriptions and examples.For example, for the
add_new_contract_keywords_v0method:/// Adds keywords to a contract by inserting a new keyword subtree structure to the `Identities` subtree. /// /// # Parameters /// * `contract_id` - The identifier of the contract to add keywords to /// * `owner_id` - The identifier of the contract owner /// * `keywords` - A vector of keywords to associate with the contract /// * `block_info` - Information about the current block /// * `apply` - Whether to apply the operations or just estimate costs /// * `transaction` - The transaction argument /// * `platform_version` - The platform version /// /// # Returns /// * `Result<FeeResult, Error>` - The result of the operation, containing fee information or an error pub(super) fn add_new_contract_keywords_v0( // ... )packages/rs-drive/src/drive/contract/insert/add_new_keywords/mod.rs (1)
14-14: Consider renaming the submodule or clarifying the versioning scheme.The code references version 1 yet calls implementation details from
v0. This naming may lead to confusion. Consider aligning the submodule name with its version support for clearer maintainability.Proposed snippet:
-mod v0; +mod v1;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/rs-drive-abci/src/execution/check_tx/v0/mod.rs(5 hunks)packages/rs-drive/src/drive/contract/insert/add_new_keywords/mod.rs(1 hunks)packages/rs-drive/src/drive/contract/insert/add_new_keywords/v0/mod.rs(1 hunks)packages/rs-drive/src/drive/contract/insert/mod.rs(1 hunks)packages/rs-drive/tests/deterministic_root_hash.rs(2 hunks)packages/rs-drive/tests/query_tests.rs(15 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/rs-drive/src/drive/contract/insert/mod.rs
- packages/rs-drive-abci/src/execution/check_tx/v0/mod.rs
🧰 Additional context used
🧬 Code Definitions (1)
packages/rs-drive/src/drive/contract/insert/add_new_keywords/v0/mod.rs (4)
packages/rs-dpp/src/data_contract/accessors/mod.rs (3)
version(41-46)owner_id(48-53)keywords(273-278)packages/rs-dpp/src/data_contract/v1/accessors/mod.rs (3)
version(33-35)owner_id(37-39)keywords(229-231)packages/rs-dpp/src/data_contract/accessors/v1/mod.rs (1)
keywords(68-68)packages/rs-drive/src/drive/contract/insert/insert_contract/v1/mod.rs (1)
None(103-103)
⏰ Context from checks skipped due to timeout of 90000ms (18)
- GitHub Check: Rust packages (rs-dapi-client) / Unused dependencies
- GitHub Check: Rust packages (dapi-grpc) / Unused dependencies
- GitHub Check: Rust packages (rs-dapi-client) / Linting
- GitHub Check: Rust packages (rs-dapi-client) / Tests
- GitHub Check: Rust packages (rs-dapi-client) / Check each feature
- GitHub Check: Rust packages (drive-abci) / Unused dependencies
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (dash-sdk) / Check each feature
- GitHub Check: Rust packages (drive-abci) / Formatting
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (drive) / Formatting
- GitHub Check: Rust packages (wasm-dpp) / Linting
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (token-history-contract) / Unused dependencies
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
🔇 Additional comments (13)
packages/rs-drive/tests/query_tests.rs (6)
2512-2513: Expected hash value updated to reflect contract keyword field addition.The change in the expected application hash value is aligned with the introduction of the keyword search system contract in this PR. This update ensures tests continue to pass with the new data structure.
3826-3827: Updated hash value correctly accommodates the new contract structure.This hash value modification reflects the changes to the underlying data structure resulting from the addition of the
keywordsfield to data contracts.
4429-4430: Hash value updated to maintain test validity with new contract fields.This is a necessary adjustment to the expected hash value to account for structural changes in the contract data model from adding keyword search functionality.
4552-4553: Correctly updated expected hash values for DPNS tests.These hash value changes ensure the DPNS (Dash Platform Name Service) tests continue to work properly with the new contract structure that includes keyword functionality.
Also applies to: 5446-5447
4646-4647: Updated hash value for withdrawals test to match new data structure.This change to the expected hash value maintains test integrity after the introduction of the keyword field in the contract system.
6251-6252: Hash value properly updated for null ID query tests.The modification to this expected hash value ensures that tests involving null ID queries continue to function correctly with the new contract schema that includes keyword support.
packages/rs-drive/tests/deterministic_root_hash.rs (1)
307-307:❓ Verification inconclusive
Hash update is expected due to contract structure changes
The hash change reflects the addition of the keyword search capability to the system. Ensure that this hash value is correct and will remain stable with the new contract structure.
This is an important assertion - verify that this hash remains consistent across environments:
🏁 Script executed:
#!/bin/bash # Run the test multiple times to ensure deterministic hash generation for i in {1..3}; do echo "Run $i:" cargo test --package rs-drive --test deterministic_root_hash test_root_hash_with_batches_for_version -- --nocapture | grep -A 3 "not matching after contract insertion" doneLength of output: 920
Deterministic Hash Verification Pending – Please Confirm in a Rust Environment
The hash update is expected with the recent contract structure changes, but our verification attempt was inconclusive due to the absence of the
cargocommand in the current environment. Please run the following test in an environment where Rust and Cargo are installed:
- Execute:
for i in {1..3}; do echo "Run $i:" cargo test --package rs-drive --test deterministic_root_hash test_root_hash_with_batches_for_version -- --nocapture | grep -A 3 "not matching after contract insertion" done- Confirm that the hash
remains deterministic across runs._ => "aa29a0d3dcdcb1dc1b446830931762e0b9c2e2d97d6f426f8dbe4e0eda8e6130",Kindly verify manually that this hash (reflecting the addition of the keyword search capability) is stable and correct.
packages/rs-drive/src/drive/contract/insert/add_new_keywords/v0/mod.rs (4)
105-132: LGTM: Efficient keyword document creation and operation handlingThe implementation correctly iterates through the keywords, builds a document for each one, and extends the operations vector with the operations for each document. This approach is clean and efficient.
152-155: LGTM: Clean document property constructionThe document properties are constructed clearly with a BTreeMap containing the keyword and contract ID, which seems appropriate for the keyword search functionality.
1-176: The overall structure aligns well with the PR objectivesThe implemented functionality aligns well with the PR objective of implementing a keyword search system contract. The code provides the necessary methods to add keywords to contracts and create corresponding documents in the Keyword Search system contract.
68-76:❌ Incorrect review comment
Method name inconsistency
The method is calling
add_new_contract_keywords_operations, but the method defined in this file is namedadd_new_contract_keywords_operations_v0. This suggests there might be a version-agnostic wrapper function elsewhere that's not visible in this file.
🏁 Script executed:
#!/bin/bash # Check if there's a version-agnostic wrapper function that calls the v0 implementation rg "fn add_new_contract_keywords_operations" -A 5 packages/rs-drive/src/Length of output: 1297
Verified: The method call is correct as-is.
After checking the repository, we found that there is indeed a version-agnostic wrapper defined in
packages/rs-drive/src/drive/contract/insert/add_new_keywords/mod.rsnamedadd_new_contract_keywords_operations. This wrapper is intended to abstract the version-specific implementations (likeadd_new_contract_keywords_operations_v0in the v0 directory). Given this design, the method invocation in the v0 file is correct and not an inconsistency.Likely an incorrect or invalid review comment.
packages/rs-drive/src/drive/contract/insert/add_new_keywords/mod.rs (2)
87-92: Duplicate of theknown_versionsmismatch issue.As with the first function, please ensure
[1]is added to reflect actual recognized versions.
127-132: Duplicate of theknown_versionsmismatch issue.Apply the same resolution here by adding
[1]to avoid confusion.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/rs-drive/src/drive/contract/insert/add_new_keywords/mod.rs (1)
55-86: Duplication of version matching logic.This function repeats the same match-based version checks as the previous function. Consider extracting a shared utility or macro to reduce duplication, ensuring maintainability as more versions are introduced.
packages/rs-drive/src/drive/contract/insert/add_new_keywords/v0/mod.rs (1)
87-135: Keywords loop is well-structured.The enumeration approach is straightforward. Each keyword is mapped into a document, then added to operations. The code is clear and well-documented. Potential improvement could be an early return if
keywordsis empty, but that’s optional.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/rs-drive/src/drive/contract/insert/add_new_keywords/mod.rs(1 hunks)packages/rs-drive/src/drive/contract/insert/add_new_keywords/v0/mod.rs(1 hunks)packages/rs-drive/tests/query_tests_history.rs(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: Rust packages (token-history-contract) / Tests
- GitHub Check: Rust packages (token-history-contract) / Linting
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive-abci) / Unused dependencies
- GitHub Check: Rust packages (drive-abci) / Detect immutable structure changes
- GitHub Check: Rust packages (drive-abci) / Linting
- GitHub Check: Rust packages (drive-abci) / Check each feature
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Rust packages (drive-abci) / Formatting
- GitHub Check: Rust packages (drive) / Formatting
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Linting
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (dash-sdk) / Check each feature
- 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 JS packages / Build JS
🔇 Additional comments (9)
packages/rs-drive/tests/query_tests_history.rs (1)
1669-1670: Root hash updates look good!The changes to the expected root hash values in the test are consistent with the implementation of the keyword search system contract. These hash changes reflect modifications to the underlying data structure, which is expected when adding new functionality to the system.
Also applies to: 3035-3036
packages/rs-drive/src/drive/contract/insert/add_new_keywords/mod.rs (5)
16-46: Code structure looks good.This function cleanly delegates to a version-specific implementation. No major correctness or performance issues stand out. Good use of the
Result<FeeResult, Error>pattern.
47-51: Fixknown_versionsmismatch.Within the match arm for version 1, the
known_versionsarray only includes[0]. This can confuse future maintainers and error reporting because version 1 is explicitly recognized. Including[1]in the array accurately reflects all recognized versions:- known_versions: vec![0], + known_versions: vec![0, 1],
87-92: Fixknown_versionsmismatch.As before, the
known_versionsarray only lists[0]despite handling version 1. Extend the array to include[1]for clarity and accuracy.
95-126: Operations generation logic is clear.This method correctly retrieves the versioned logic for building operations. The step-by-step approach, returning a vector of
LowLevelDriveOperation, is well-structured. No harmful data race, performance, or security issues are evident.
127-131: Fixknown_versionsmismatch.The array only includes
[0]here as well, despite handling version 1 in the match. Align it with other recognized versions.packages/rs-drive/src/drive/contract/insert/add_new_keywords/v0/mod.rs (3)
16-48: Ensure external validations are in place.The method delegates adding keywords to the operations pipeline but doesn’t explicitly validate constraints like uniqueness or ASCII. If these validations occur elsewhere (e.g., at the DPP layer), confirm usage of the relevant “TooManyKeywordsError” or “DuplicateKeywordsError” classes. If not, consider adding them here to prevent invalid data from propagating.
50-61: Conditional costs logic.Switching between
Noneand an initializedHashMapbased on theapplyflag is a neat approach to handle cost estimation. This is a sensible pattern for toggling between real application and dry-run cost estimation.
137-175: Document construction is correct.The function correctly creates and populates a keyword document, including time and block height details. No obvious issues; the approach to generate IDs using
Document::generate_document_id_v0aligns with typical patterns in the codebase.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
packages/rs-drive/src/drive/contract/insert/add_new_keywords/mod.rs (4)
14-14: Clarify module naming.The module name
v0might be confusing for maintainers given that version 1 maps to*_v0methods. Consider adding a brief comment noting that “v0” refers to the first stable implementation for version 1, or rename the module tov1for consistency.
35-38: Provide additional support or guidance for version 0.Returning
NotSupportedis appropriate here, but it might be beneficial to add a quick comment or log message explaining that version 0 does not support contract keywords. This helps future contributors quickly grasp why version 0 is entirely disabled.Also applies to: 74-77, 115-118
38-46: Rename or comment on_v0methods to match version 1 usage.Each match arm for version 1 calls a method named
_v0. If this is intentional (the code’s first production version labeled as v0), please clarify. Otherwise, consider renaming these methods (e.g.,_v1) to align with the version number in the match.Also applies to: 77-86, 118-126
16-134: Consider adding dedicated tests for these new keyword methods.While your PR mentions testing, ensure each of the three public methods (
add_new_contract_keywords,add_new_contract_keywords_add_to_operations,add_new_contract_keywords_operations) has targeted unit tests covering:
- Successful keyword insertion (version 1).
- Edge cases like empty keyword array, maximum keyword array length, etc.
- Mismatch and unsupported version scenarios.
Feel free to let me know if you’d like assistance drafting those tests.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/rs-drive/src/drive/contract/insert/add_new_keywords/mod.rs(1 hunks)packages/rs-drive/tests/deterministic_root_hash.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/rs-drive/tests/deterministic_root_hash.rs
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: Rust packages (dpp) / Formatting
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (drive) / Formatting
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (dash-sdk) / Unused dependencies
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (dash-sdk) / Check each feature
- GitHub Check: Rust packages (dash-sdk) / Formatting
- GitHub Check: Rust packages (token-history-contract) / Linting
- GitHub Check: Rust packages (dapi-grpc) / Tests
- GitHub Check: Rust packages (dapi-grpc) / Linting
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (dapi-grpc) / Check each feature
- GitHub Check: Rust packages (wasm-dpp) / Linting
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
🔇 Additional comments (1)
packages/rs-drive/src/drive/contract/insert/add_new_keywords/mod.rs (1)
49-49: Good job implementing the version mismatch fix.It appears you have already addressed the previous feedback by including
[0, 1]in yourknown_versionsarrays. This now correctly reflects that both version 0 and version 1 are recognized.Also applies to: 89-89, 129-129
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
packages/rs-dpp/src/data_contract/serialized_version/mod.rs (1)
116-145: 🛠️ Refactor suggestionConsider updating eq_without_auto_fields to include new fields.
The equality comparison for V1 contracts doesn't include the new keywords and description fields, which might lead to unexpected behavior when comparing contracts.
v1_self.document_schemas == v1_other.document_schemas && v1_self.groups == v1_other.groups && v1_self.tokens == v1_other.tokens + && v1_self.keywords == v1_other.keywords + && v1_self.description == v1_other.description
🧹 Nitpick comments (7)
packages/wasm-dpp/src/errors/consensus/consensus_error.rs (1)
64-64: Consider breaking down the long import line for better readabilityThe import line is extremely long (over 1000 characters), which makes it difficult to read and maintain. Consider breaking this import into multiple lines by functionality groups, similar to how other imports in this file are structured.
-use dpp::consensus::basic::data_contract::{ContestedUniqueIndexOnMutableDocumentTypeError, ContestedUniqueIndexWithUniqueIndexError, DataContractTokenConfigurationUpdateError, DuplicateKeywordsError, GroupExceedsMaxMembersError, GroupMemberHasPowerOfZeroError, GroupMemberHasPowerOverLimitError, GroupNonUnilateralMemberPowerHasLessThanRequiredPowerError, GroupPositionDoesNotExistError, GroupTotalPowerLessThanRequiredError, InvalidDescriptionLengthError, InvalidDocumentTypeRequiredSecurityLevelError, InvalidKeywordEncodingError, InvalidKeywordLengthError, InvalidTokenBaseSupplyError, InvalidTokenDistributionFunctionDivideByZeroError, InvalidTokenDistributionFunctionIncoherenceError, InvalidTokenDistributionFunctionInvalidParameterError, InvalidTokenDistributionFunctionInvalidParameterTupleError, NonContiguousContractGroupPositionsError, NonContiguousContractTokenPositionsError, TokenPaymentByBurningOnlyAllowedOnInternalTokenError, TooManyKeywordsError, UnknownDocumentActionTokenEffectError, UnknownDocumentCreationRestrictionModeError, UnknownGasFeesPaidByError, UnknownSecurityLevelError, UnknownStorageKeyRequirementsError, UnknownTradeModeError, UnknownTransferableTypeError}; +use dpp::consensus::basic::data_contract::{ + ContestedUniqueIndexOnMutableDocumentTypeError, ContestedUniqueIndexWithUniqueIndexError, + DataContractTokenConfigurationUpdateError, DuplicateKeywordsError, + GroupExceedsMaxMembersError, GroupMemberHasPowerOfZeroError, GroupMemberHasPowerOverLimitError, + GroupNonUnilateralMemberPowerHasLessThanRequiredPowerError, GroupPositionDoesNotExistError, + GroupTotalPowerLessThanRequiredError, InvalidDescriptionLengthError, + InvalidDocumentTypeRequiredSecurityLevelError, InvalidKeywordEncodingError, + InvalidKeywordLengthError, InvalidTokenBaseSupplyError, + InvalidTokenDistributionFunctionDivideByZeroError, + InvalidTokenDistributionFunctionIncoherenceError, + InvalidTokenDistributionFunctionInvalidParameterError, + InvalidTokenDistributionFunctionInvalidParameterTupleError, + NonContiguousContractGroupPositionsError, NonContiguousContractTokenPositionsError, + TokenPaymentByBurningOnlyAllowedOnInternalTokenError, TooManyKeywordsError, + UnknownDocumentActionTokenEffectError, UnknownDocumentCreationRestrictionModeError, + UnknownGasFeesPaidByError, UnknownSecurityLevelError, UnknownStorageKeyRequirementsError, + UnknownTradeModeError, UnknownTransferableTypeError +};packages/rs-sdk/tests/vectors/test_data_contracts_1_ok_1_nx/quorum_pubkey-106-3603d12872604475619be7ad682f8339ad60debd43b3f430bd12d51eac268936.json (1)
1-1: Test Vector Format – Non-Standard JSON StructureThe file contains a single hash value without using double quotes, which deviates from standard JSON formatting. However, as noted in the retrieved learnings, test vector JSON files in this directory are intentionally structured in a non-standard way to serve as mock data. That said, consider adding a trailing newline for consistency with common file formatting conventions.
🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: String values must be double quoted.
(parse)
packages/rs-sdk/tests/vectors/test_data_contract_read/quorum_pubkey-106-3603d12872604475619be7ad682f8339ad60debd43b3f430bd12d51eac268936.json (1)
1-1: Test Vector Format – Non-Standard JSON StructureThis file also contains a single hash value without double quotes. This non-standard formatting is intentional for mock data in the testing framework. As a minor nitpick, you might consider appending a newline at the end of the file to align with widely accepted file formatting practices.
🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: String values must be double quoted.
(parse)
packages/rs-sdk/tests/vectors/test_data_contract_read_not_found/quorum_pubkey-106-3603d12872604475619be7ad682f8339ad60debd43b3f430bd12d51eac268936.json (1)
1-1: Test Vector Format – Non-Standard JSON StringThe file presents a single hash value in a non-standard JSON format (lacking double quotes), which aligns with the project's intentional design for test vectors. While this is acceptable within our framework, adding a trailing newline might improve the overall consistency and readability of the file.
🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: String values must be double quoted.
(parse)
packages/rs-dpp/src/data_contract/associated_token/token_configuration/v0/accessors.rs (1)
137-156: Missing update in Display implementation.The
fmt::Displayimplementation forTokenConfigurationV0(around line 137) has not been updated to include the newdescriptionfield in the formatted output.Consider updating the
fmt::Displayimplementation to include the newdescriptionfield:write!( f, - "TokenConfigurationV0 {{\n conventions: {:?},\n conventions_change_rules: {:?},\n base_supply: {},\n max_supply: {:?},\n keeps_history: {},\n start_as_paused: {},\n max_supply_change_rules: {:?},\n distribution_rules: {},\n manual_minting_rules: {:?},\n manual_burning_rules: {:?},\n freeze_rules: {:?},\n unfreeze_rules: {:?},\n destroy_frozen_funds_rules: {:?},\n emergency_action_rules: {:?},\n main_control_group: {:?},\n main_control_group_can_be_modified: {:?}\n}}", + "TokenConfigurationV0 {{\n conventions: {:?},\n conventions_change_rules: {:?},\n base_supply: {},\n max_supply: {:?},\n keeps_history: {},\n start_as_paused: {},\n max_supply_change_rules: {:?},\n distribution_rules: {},\n manual_minting_rules: {:?},\n manual_burning_rules: {:?},\n freeze_rules: {:?},\n unfreeze_rules: {:?},\n destroy_frozen_funds_rules: {:?},\n emergency_action_rules: {:?},\n main_control_group: {:?},\n main_control_group_can_be_modified: {:?},\n description: {:?}\n}}", self.conventions, self.conventions_change_rules, self.base_supply, self.max_supply, self.keeps_history, self.start_as_paused, self.max_supply_change_rules, self.distribution_rules, self.manual_minting_rules, self.manual_burning_rules, self.freeze_rules, self.unfreeze_rules, self.destroy_frozen_funds_rules, self.emergency_action_rules, self.main_control_group, - self.main_control_group_can_be_modified + self.main_control_group_can_be_modified, + self.description )packages/rs-drive/src/drive/contract/insert/add_description/v0/mod.rs (2)
16-48: Overall implementation looks good.
This function gathers operations, calculates fees, and returns aFeeResult. The usage ofDrive::calculate_feewith appropriate parameters seems coherent. No performance or concurrency concerns spotted.Consider adding unit tests specifically targeting this function to ensure fee calculation remains correct when future modifications occur.
162-177: Consider using the passed platform version instead ofPlatformVersion::latest().
Using the hardcoded latest version forfetch_identity_noncecould cause discrepancies if the caller supplies a specificplatform_version.- match self.fetch_identity_nonce(owner_id.into(), true, None, PlatformVersion::latest()) { + match self.fetch_identity_nonce( + owner_id.into(), + true, + None, + platform_version + ) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (49)
packages/rs-dpp/src/data_contract/accessors/mod.rs(3 hunks)packages/rs-dpp/src/data_contract/accessors/v1/mod.rs(2 hunks)packages/rs-dpp/src/data_contract/associated_token/token_configuration/accessors/mod.rs(2 hunks)packages/rs-dpp/src/data_contract/associated_token/token_configuration/accessors/v0/mod.rs(5 hunks)packages/rs-dpp/src/data_contract/associated_token/token_configuration/v0/accessors.rs(2 hunks)packages/rs-dpp/src/data_contract/associated_token/token_configuration/v0/mod.rs(2 hunks)packages/rs-dpp/src/data_contract/serialized_version/mod.rs(2 hunks)packages/rs-dpp/src/data_contract/serialized_version/v1/mod.rs(4 hunks)packages/rs-dpp/src/data_contract/v1/accessors/mod.rs(2 hunks)packages/rs-dpp/src/data_contract/v1/data_contract.rs(2 hunks)packages/rs-dpp/src/data_contract/v1/serialization/mod.rs(3 hunks)packages/rs-dpp/src/errors/consensus/basic/basic_error.rs(2 hunks)packages/rs-dpp/src/errors/consensus/basic/data_contract/invalid_description_length_error.rs(1 hunks)packages/rs-dpp/src/errors/consensus/basic/data_contract/mod.rs(3 hunks)packages/rs-dpp/src/errors/consensus/codes.rs(1 hunks)packages/rs-drive-abci/src/execution/check_tx/v0/mod.rs(5 hunks)packages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/test/tokens.rs(5 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/creation.rs(2 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_create/advanced_structure/v0/mod.rs(2 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_create/mod.rs(1 hunks)packages/rs-drive/src/drive/contract/insert/add_description/mod.rs(1 hunks)packages/rs-drive/src/drive/contract/insert/add_description/v0/mod.rs(1 hunks)packages/rs-drive/src/drive/contract/insert/add_new_keywords/v0/mod.rs(1 hunks)packages/rs-drive/src/drive/contract/insert/insert_contract/v1/mod.rs(1 hunks)packages/rs-drive/src/drive/contract/insert/mod.rs(1 hunks)packages/rs-drive/src/drive/group/prove/prove_action_infos/v0/mod.rs(2 hunks)packages/rs-drive/src/drive/group/prove/prove_action_signers/v0/mod.rs(3 hunks)packages/rs-drive/src/drive/group/prove/prove_group_info/v0/mod.rs(2 hunks)packages/rs-drive/src/drive/group/prove/prove_group_infos/v0/mod.rs(2 hunks)packages/rs-drive/src/drive/tokens/balance/prove_identities_token_balances/v0/mod.rs(3 hunks)packages/rs-drive/src/drive/tokens/balance/prove_identity_token_balances/v0/mod.rs(3 hunks)packages/rs-drive/src/drive/tokens/info/prove_identities_token_infos/v0/mod.rs(3 hunks)packages/rs-drive/src/drive/tokens/info/prove_identity_token_infos/v0/mod.rs(2 hunks)packages/rs-drive/src/drive/tokens/status/prove_token_statuses/v0/mod.rs(1 hunks)packages/rs-drive/src/drive/tokens/system/prove_token_total_supply_and_aggregated_identity_balances/v0/mod.rs(2 hunks)packages/rs-drive/tests/deterministic_root_hash.rs(1 hunks)packages/rs-drive/tests/query_tests.rs(15 hunks)packages/rs-drive/tests/query_tests_history.rs(2 hunks)packages/rs-sdk/tests/vectors/document_list_document_query/quorum_pubkey-106-3603d12872604475619be7ad682f8339ad60debd43b3f430bd12d51eac268936.json(1 hunks)packages/rs-sdk/tests/vectors/document_list_drive_query/quorum_pubkey-106-3603d12872604475619be7ad682f8339ad60debd43b3f430bd12d51eac268936.json(1 hunks)packages/rs-sdk/tests/vectors/document_read/quorum_pubkey-106-3603d12872604475619be7ad682f8339ad60debd43b3f430bd12d51eac268936.json(1 hunks)packages/rs-sdk/tests/vectors/document_read_no_contract/quorum_pubkey-106-3603d12872604475619be7ad682f8339ad60debd43b3f430bd12d51eac268936.json(1 hunks)packages/rs-sdk/tests/vectors/document_read_no_document/quorum_pubkey-106-3603d12872604475619be7ad682f8339ad60debd43b3f430bd12d51eac268936.json(1 hunks)packages/rs-sdk/tests/vectors/test_data_contract_read/quorum_pubkey-106-3603d12872604475619be7ad682f8339ad60debd43b3f430bd12d51eac268936.json(1 hunks)packages/rs-sdk/tests/vectors/test_data_contract_read_not_found/quorum_pubkey-106-3603d12872604475619be7ad682f8339ad60debd43b3f430bd12d51eac268936.json(1 hunks)packages/rs-sdk/tests/vectors/test_data_contracts_1_ok_1_nx/quorum_pubkey-106-3603d12872604475619be7ad682f8339ad60debd43b3f430bd12d51eac268936.json(1 hunks)packages/search-contract/schema/v1/search-contract-documents.json(2 hunks)packages/search-contract/src/v1/mod.rs(1 hunks)packages/wasm-dpp/src/errors/consensus/consensus_error.rs(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (18)
- packages/rs-drive/src/drive/group/prove/prove_action_infos/v0/mod.rs
- packages/rs-drive/src/drive/group/prove/prove_group_info/v0/mod.rs
- packages/rs-drive/src/drive/tokens/status/prove_token_statuses/v0/mod.rs
- packages/rs-drive/src/drive/tokens/info/prove_identities_token_infos/v0/mod.rs
- packages/rs-drive/src/drive/tokens/info/prove_identity_token_infos/v0/mod.rs
- packages/rs-drive/src/drive/tokens/balance/prove_identities_token_balances/v0/mod.rs
- packages/rs-dpp/src/data_contract/v1/data_contract.rs
- packages/rs-drive-abci/src/execution/check_tx/v0/mod.rs
- packages/rs-drive/src/drive/tokens/balance/prove_identity_token_balances/v0/mod.rs
- packages/rs-drive/src/drive/group/prove/prove_group_infos/v0/mod.rs
- packages/rs-drive/tests/deterministic_root_hash.rs
- packages/rs-dpp/src/errors/consensus/basic/basic_error.rs
- packages/rs-drive/src/drive/tokens/system/prove_token_total_supply_and_aggregated_identity_balances/v0/mod.rs
- packages/rs-dpp/src/errors/consensus/basic/data_contract/mod.rs
- packages/rs-dpp/src/errors/consensus/codes.rs
- packages/rs-drive/tests/query_tests.rs
- packages/rs-drive/tests/query_tests_history.rs
- packages/rs-dpp/src/data_contract/v1/serialization/mod.rs
🧰 Additional context used
🧠 Learnings (8)
packages/rs-sdk/tests/vectors/document_list_document_query/quorum_pubkey-106-3603d12872604475619be7ad682f8339ad60debd43b3f430bd12d51eac268936.json (1)
Learnt from: lklimek
PR: dashpay/platform#2030
File: packages/rs-sdk/tests/vectors/test_asset_lock_proof/quorum_pubkey-100-4ce7fd81273c2b394c0f32367374fc5b09ba912e017aacb366d2171e9ca6f9d5.json:1-1
Timestamp: 2025-04-06T00:27:18.424Z
Learning: In the `packages/rs-sdk/tests/vectors/test_asset_lock_proof/` directory, files with the `.json` extension are mock data that may not follow standard JSON format; this is intentional and acceptable within the project's testing framework.
packages/rs-sdk/tests/vectors/document_list_drive_query/quorum_pubkey-106-3603d12872604475619be7ad682f8339ad60debd43b3f430bd12d51eac268936.json (1)
Learnt from: lklimek
PR: dashpay/platform#2030
File: packages/rs-sdk/tests/vectors/test_asset_lock_proof/quorum_pubkey-100-4ce7fd81273c2b394c0f32367374fc5b09ba912e017aacb366d2171e9ca6f9d5.json:1-1
Timestamp: 2025-04-06T00:27:18.424Z
Learning: In the `packages/rs-sdk/tests/vectors/test_asset_lock_proof/` directory, files with the `.json` extension are mock data that may not follow standard JSON format; this is intentional and acceptable within the project's testing framework.
packages/rs-sdk/tests/vectors/document_read/quorum_pubkey-106-3603d12872604475619be7ad682f8339ad60debd43b3f430bd12d51eac268936.json (1)
Learnt from: lklimek
PR: dashpay/platform#2030
File: packages/rs-sdk/tests/vectors/test_asset_lock_proof/quorum_pubkey-100-4ce7fd81273c2b394c0f32367374fc5b09ba912e017aacb366d2171e9ca6f9d5.json:1-1
Timestamp: 2025-04-06T00:27:18.424Z
Learning: In the `packages/rs-sdk/tests/vectors/test_asset_lock_proof/` directory, files with the `.json` extension are mock data that may not follow standard JSON format; this is intentional and acceptable within the project's testing framework.
packages/rs-sdk/tests/vectors/document_read_no_contract/quorum_pubkey-106-3603d12872604475619be7ad682f8339ad60debd43b3f430bd12d51eac268936.json (1)
Learnt from: lklimek
PR: dashpay/platform#2030
File: packages/rs-sdk/tests/vectors/test_asset_lock_proof/quorum_pubkey-100-4ce7fd81273c2b394c0f32367374fc5b09ba912e017aacb366d2171e9ca6f9d5.json:1-1
Timestamp: 2025-04-06T00:27:18.424Z
Learning: In the `packages/rs-sdk/tests/vectors/test_asset_lock_proof/` directory, files with the `.json` extension are mock data that may not follow standard JSON format; this is intentional and acceptable within the project's testing framework.
packages/rs-sdk/tests/vectors/document_read_no_document/quorum_pubkey-106-3603d12872604475619be7ad682f8339ad60debd43b3f430bd12d51eac268936.json (1)
Learnt from: lklimek
PR: dashpay/platform#2030
File: packages/rs-sdk/tests/vectors/test_asset_lock_proof/quorum_pubkey-100-4ce7fd81273c2b394c0f32367374fc5b09ba912e017aacb366d2171e9ca6f9d5.json:1-1
Timestamp: 2025-04-06T00:27:18.424Z
Learning: In the `packages/rs-sdk/tests/vectors/test_asset_lock_proof/` directory, files with the `.json` extension are mock data that may not follow standard JSON format; this is intentional and acceptable within the project's testing framework.
packages/rs-sdk/tests/vectors/test_data_contract_read/quorum_pubkey-106-3603d12872604475619be7ad682f8339ad60debd43b3f430bd12d51eac268936.json (1)
Learnt from: lklimek
PR: dashpay/platform#2030
File: packages/rs-sdk/tests/vectors/test_asset_lock_proof/quorum_pubkey-100-4ce7fd81273c2b394c0f32367374fc5b09ba912e017aacb366d2171e9ca6f9d5.json:1-1
Timestamp: 2025-04-06T00:27:18.424Z
Learning: In the `packages/rs-sdk/tests/vectors/test_asset_lock_proof/` directory, files with the `.json` extension are mock data that may not follow standard JSON format; this is intentional and acceptable within the project's testing framework.
packages/rs-sdk/tests/vectors/test_data_contract_read_not_found/quorum_pubkey-106-3603d12872604475619be7ad682f8339ad60debd43b3f430bd12d51eac268936.json (1)
Learnt from: lklimek
PR: dashpay/platform#2030
File: packages/rs-sdk/tests/vectors/test_asset_lock_proof/quorum_pubkey-100-4ce7fd81273c2b394c0f32367374fc5b09ba912e017aacb366d2171e9ca6f9d5.json:1-1
Timestamp: 2025-04-06T00:27:18.424Z
Learning: In the `packages/rs-sdk/tests/vectors/test_asset_lock_proof/` directory, files with the `.json` extension are mock data that may not follow standard JSON format; this is intentional and acceptable within the project's testing framework.
packages/rs-sdk/tests/vectors/test_data_contracts_1_ok_1_nx/quorum_pubkey-106-3603d12872604475619be7ad682f8339ad60debd43b3f430bd12d51eac268936.json (1)
Learnt from: lklimek
PR: dashpay/platform#2030
File: packages/rs-sdk/tests/vectors/test_asset_lock_proof/quorum_pubkey-100-4ce7fd81273c2b394c0f32367374fc5b09ba912e017aacb366d2171e9ca6f9d5.json:1-1
Timestamp: 2025-04-06T00:27:18.424Z
Learning: In the `packages/rs-sdk/tests/vectors/test_asset_lock_proof/` directory, files with the `.json` extension are mock data that may not follow standard JSON format; this is intentional and acceptable within the project's testing framework.
🧬 Code Graph Analysis (14)
packages/rs-dpp/src/data_contract/serialized_version/mod.rs (1)
packages/rs-dpp/src/data_contract/accessors/mod.rs (2)
keywords(273-278)description(287-292)
packages/rs-dpp/src/data_contract/associated_token/token_configuration/v0/accessors.rs (6)
packages/rs-dpp/src/data_contract/associated_token/token_configuration/accessors/v0/mod.rs (2)
description(70-70)set_description(118-118)packages/rs-dpp/src/data_contract/accessors/v1/mod.rs (2)
description(74-74)set_description(115-115)packages/rs-dpp/src/data_contract/associated_token/token_configuration/accessors/mod.rs (2)
description(147-151)set_description(252-256)packages/rs-dpp/src/data_contract/accessors/mod.rs (2)
description(287-292)set_description(441-445)packages/rs-dpp/src/data_contract/serialized_version/mod.rs (1)
description(109-114)packages/rs-dpp/src/data_contract/v1/accessors/mod.rs (2)
description(239-241)set_description(302-304)
packages/rs-dpp/src/data_contract/serialized_version/v1/mod.rs (3)
packages/rs-dpp/src/data_contract/accessors/v1/mod.rs (2)
keywords(68-68)description(74-74)packages/rs-dpp/src/data_contract/accessors/mod.rs (2)
keywords(273-278)description(287-292)packages/rs-dpp/src/data_contract/serialized_version/mod.rs (2)
keywords(102-107)description(109-114)
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_create/mod.rs (5)
packages/rs-dpp/src/data_contract/accessors/v1/mod.rs (1)
keywords(68-68)packages/rs-dpp/src/data_contract/v1/accessors/mod.rs (1)
keywords(229-231)packages/rs-dpp/src/errors/consensus/basic/data_contract/invalid_description_length_error.rs (1)
new(24-29)packages/rs-dpp/src/errors/consensus/basic/data_contract/invalid_keyword_encoding_error.rs (2)
new(24-29)keyword(35-37)packages/rs-dpp/src/errors/consensus/basic/data_contract/invalid_keyword_length_error.rs (2)
new(24-29)keyword(35-37)
packages/rs-dpp/src/data_contract/associated_token/token_configuration/accessors/v0/mod.rs (5)
packages/rs-dpp/src/data_contract/associated_token/token_configuration/v0/accessors.rs (2)
description(154-156)set_description(230-232)packages/rs-dpp/src/data_contract/accessors/v1/mod.rs (2)
description(74-74)set_description(115-115)packages/rs-dpp/src/data_contract/accessors/mod.rs (2)
description(287-292)set_description(441-445)packages/rs-dpp/src/data_contract/serialized_version/mod.rs (1)
description(109-114)packages/rs-dpp/src/data_contract/v1/accessors/mod.rs (2)
description(239-241)set_description(302-304)
packages/rs-drive/src/drive/contract/insert/add_description/mod.rs (4)
packages/rs-drive/src/state_transition_action/batch/batched_transition/document_transition/document_create_transition_action/v0/mod.rs (1)
block_info(53-53)packages/rs-dpp/src/data_contract/accessors/mod.rs (2)
version(41-46)description(287-292)packages/rs-dpp/src/data_contract/v1/accessors/mod.rs (2)
version(33-35)description(239-241)packages/rs-dpp/src/data_contract/accessors/v1/mod.rs (1)
description(74-74)
packages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/test/tokens.rs (1)
packages/wasm-dpp/src/data_contract/data_contract.rs (1)
token_configuration(435-443)
packages/rs-dpp/src/data_contract/accessors/v1/mod.rs (2)
packages/rs-dpp/src/data_contract/accessors/mod.rs (6)
keywords(273-278)keywords_mut(280-285)description(287-292)description_mut(294-299)set_keywords(434-438)set_description(441-445)packages/rs-dpp/src/data_contract/v1/accessors/mod.rs (6)
keywords(229-231)keywords_mut(234-236)description(239-241)description_mut(244-246)set_keywords(297-299)set_description(302-304)
packages/rs-dpp/src/data_contract/v1/accessors/mod.rs (2)
packages/rs-dpp/src/data_contract/accessors/v1/mod.rs (6)
keywords(68-68)keywords_mut(71-71)description(74-74)description_mut(77-77)set_keywords(112-112)set_description(115-115)packages/rs-dpp/src/data_contract/accessors/mod.rs (6)
keywords(273-278)keywords_mut(280-285)description(287-292)description_mut(294-299)set_keywords(434-438)set_description(441-445)
packages/rs-dpp/src/data_contract/accessors/mod.rs (2)
packages/rs-dpp/src/data_contract/serialized_version/mod.rs (2)
keywords(102-107)description(109-114)packages/rs-dpp/src/data_contract/v1/accessors/mod.rs (6)
keywords(229-231)keywords_mut(234-236)description(239-241)description_mut(244-246)set_keywords(297-299)set_description(302-304)
packages/rs-drive/src/drive/contract/insert/add_description/v0/mod.rs (4)
packages/rs-drive/src/state_transition_action/batch/batched_transition/document_transition/document_create_transition_action/v0/mod.rs (1)
block_info(53-53)packages/rs-dpp/src/data_contract/associated_token/token_configuration/accessors/v0/mod.rs (1)
description(70-70)packages/rs-dpp/src/data_contract/accessors/v1/mod.rs (1)
description(74-74)packages/rs-dpp/src/data_contract/associated_token/token_configuration/accessors/mod.rs (1)
description(147-151)
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/creation.rs (4)
packages/rs-dpp/src/data_contract/document_type/accessors/v0/mod.rs (1)
creation_restriction_mode(74-74)packages/rs-dpp/src/data_contract/document_type/v0/accessors.rs (1)
creation_restriction_mode(98-100)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v0_methods.rs (1)
new_document_creation_transition_from_document(131-171)packages/rs-drive-abci/src/platform_types/state_transitions_processing_result/mod.rs (1)
invalid_paid_count(79-81)
packages/rs-drive/src/drive/contract/insert/add_new_keywords/v0/mod.rs (4)
packages/rs-drive/src/state_transition_action/batch/batched_transition/document_transition/document_create_transition_action/v0/mod.rs (1)
block_info(53-53)packages/rs-dpp/src/data_contract/accessors/v1/mod.rs (1)
keywords(68-68)packages/rs-drive/src/drive/contract/insert/add_description/v0/mod.rs (1)
None(63-63)packages/rs-drive/src/drive/contract/insert/insert_contract/v1/mod.rs (1)
None(103-103)
packages/rs-dpp/src/data_contract/associated_token/token_configuration/accessors/mod.rs (5)
packages/rs-dpp/src/data_contract/associated_token/token_configuration/accessors/v0/mod.rs (2)
description(70-70)set_description(118-118)packages/rs-dpp/src/data_contract/accessors/v1/mod.rs (2)
description(74-74)set_description(115-115)packages/rs-dpp/src/data_contract/associated_token/token_configuration/v0/accessors.rs (2)
description(154-156)set_description(230-232)packages/rs-dpp/src/data_contract/accessors/mod.rs (2)
description(287-292)set_description(441-445)packages/rs-dpp/src/data_contract/serialized_version/mod.rs (1)
description(109-114)
🪛 Biome (1.9.4)
packages/rs-sdk/tests/vectors/document_list_document_query/quorum_pubkey-106-3603d12872604475619be7ad682f8339ad60debd43b3f430bd12d51eac268936.json
[error] 1-1: String values must be double quoted.
(parse)
packages/rs-sdk/tests/vectors/document_list_drive_query/quorum_pubkey-106-3603d12872604475619be7ad682f8339ad60debd43b3f430bd12d51eac268936.json
[error] 1-1: String values must be double quoted.
(parse)
packages/rs-sdk/tests/vectors/document_read/quorum_pubkey-106-3603d12872604475619be7ad682f8339ad60debd43b3f430bd12d51eac268936.json
[error] 1-1: String values must be double quoted.
(parse)
packages/rs-sdk/tests/vectors/document_read_no_contract/quorum_pubkey-106-3603d12872604475619be7ad682f8339ad60debd43b3f430bd12d51eac268936.json
[error] 1-1: String values must be double quoted.
(parse)
packages/rs-sdk/tests/vectors/document_read_no_document/quorum_pubkey-106-3603d12872604475619be7ad682f8339ad60debd43b3f430bd12d51eac268936.json
[error] 1-1: String values must be double quoted.
(parse)
packages/rs-sdk/tests/vectors/test_data_contract_read/quorum_pubkey-106-3603d12872604475619be7ad682f8339ad60debd43b3f430bd12d51eac268936.json
[error] 1-1: String values must be double quoted.
(parse)
packages/rs-sdk/tests/vectors/test_data_contract_read_not_found/quorum_pubkey-106-3603d12872604475619be7ad682f8339ad60debd43b3f430bd12d51eac268936.json
[error] 1-1: String values must be double quoted.
(parse)
packages/rs-sdk/tests/vectors/test_data_contracts_1_ok_1_nx/quorum_pubkey-106-3603d12872604475619be7ad682f8339ad60debd43b3f430bd12d51eac268936.json
[error] 1-1: String values must be double quoted.
(parse)
🔇 Additional comments (65)
packages/wasm-dpp/src/errors/consensus/consensus_error.rs (1)
764-778: LGTM! Proper error handling for the new keyword featuresThe implementation correctly adds error handlers for the newly introduced keyword and description validation errors, following the established pattern in the codebase with the
generic_consensus_error!macro.These new error handlers align well with the PR objective of adding keyword search capabilities, providing the necessary validation for:
- Maximum number of keywords (TooManyKeywordsError)
- Duplicate keywords (DuplicateKeywordsError)
- Keyword length constraints (InvalidKeywordLengthError)
- Keyword encoding validation (InvalidKeywordEncodingError)
- Description length validation (InvalidDescriptionLengthError)
packages/rs-sdk/tests/vectors/document_list_document_query/quorum_pubkey-106-3603d12872604475619be7ad682f8339ad60debd43b3f430bd12d51eac268936.json (1)
1-1: Test Vector File - Document List Document QueryThis file contains a single hash value intended as a test vector. Although it isn’t formatted as strict JSON (the hash is not quoted and there’s no newline), this deviation is intentional per the project's established testing conventions. The static analysis warning regarding double quotes can be safely ignored in this context.
🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: String values must be double quoted.
(parse)
packages/rs-sdk/tests/vectors/document_read/quorum_pubkey-106-3603d12872604475619be7ad682f8339ad60debd43b3f430bd12d51eac268936.json (1)
1-1: Test Vector File - Document ReadThis file provides a hash value for testing document read operations. Although it deviates from standard JSON formatting (missing double quotes and newline), this is an intentional design choice for test vector files in the project. The static analysis error is a false positive in this scenario.
🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: String values must be double quoted.
(parse)
packages/rs-sdk/tests/vectors/document_read_no_contract/quorum_pubkey-106-3603d12872604475619be7ad682f8339ad60debd43b3f430bd12d51eac268936.json (1)
1-1: Test Vector File - Document Read No ContractThis file is used as a test vector for scenarios where a contract is not present. The single-line hash, while not valid JSON (due to missing quotes and a trailing newline), aligns with the intentional mock data format used throughout the test suite. The static analysis message about quoting is not applicable in this context.
🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: String values must be double quoted.
(parse)
packages/rs-sdk/tests/vectors/document_read_no_document/quorum_pubkey-106-3603d12872604475619be7ad682f8339ad60debd43b3f430bd12d51eac268936.json (1)
1-1: Test Vector File - Document Read No DocumentThis file contains a hash for testing scenarios where no document is associated. Despite its non-standard JSON formatting (unquoted string and missing newline), this format is intentional and consistent with other test vector files in the project. The static analysis error regarding double quotes is a false positive here.
🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: String values must be double quoted.
(parse)
packages/rs-sdk/tests/vectors/document_list_drive_query/quorum_pubkey-106-3603d12872604475619be7ad682f8339ad60debd43b3f430bd12d51eac268936.json (1)
1-1: Test Vector File - Document List Drive QueryThis file, used for testing drive query functionality, contains a single hash string as a test vector. The non-standard JSON formatting (lack of double quotes and missing newline) is intentional within the testing framework, and the noteworthy static analysis warning should be disregarded.
🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: String values must be double quoted.
(parse)
packages/rs-dpp/src/errors/consensus/basic/data_contract/invalid_description_length_error.rs (3)
8-21: Implementation of InvalidDescriptionLengthError is well structured.The error struct is properly defined with appropriate derives for Error, Debug, Clone, and serialization traits. The error message clearly indicates the valid length constraints (3-100 characters).
23-38: Appropriate accessor methods are implemented.The implementation provides a clean constructor and getter methods for accessing the error fields, following Rust best practices.
40-44: From implementation for ConsensusError is correct.This implementation allows the error to be converted into a ConsensusError, which is essential for error propagation in the consensus system.
packages/search-contract/schema/v1/search-contract-documents.json (5)
2-2: Renamed object from 'contract' to 'contractKeywords'.This rename is more descriptive and accurately reflects the purpose of the object to store keywords associated with contracts.
24-31: Added contractId property to contractKeywords schema.The contractId is appropriately defined as a byte array with exactly 32 items and the correct media type for identifiers. This links keywords to specific contracts.
33-36: Updated required properties in contractKeywords.The required properties now include "contractId" instead of "$contractId", which aligns with the new schema structure.
39-77: Added 'shortDescription' document type with appropriate constraints.The shortDescription object is well-defined with:
- Immutability flag set to prevent modifications
- A unique index on contractId for efficient lookups
- A maximum description length of 100 characters
- Clear documentation explaining its purpose
78-116: Added 'fullDescription' document type for longer descriptions.The fullDescription object properly:
- Allows mutations (mutable=true) unlike shortDescription
- Has a unique index on contractId
- Supports much longer descriptions (10,000 chars)
- Contains clear documentation about its purpose and behavior
packages/rs-drive/src/drive/group/prove/prove_action_signers/v0/mod.rs (3)
151-152: Added keywords and description fields to DataContractV1 in test.These additions align with the changes to the DataContract structure throughout the codebase, ensuring test fixtures match the updated schema.
321-322: Added keywords and description fields to second test case.Consistent application of the new fields across all test cases ensures thorough test coverage of the feature.
416-417: Added keywords and description fields to third test case.Maintaining consistency by adding the new fields to all instances of DataContractV1 in tests.
packages/rs-dpp/src/data_contract/serialized_version/mod.rs (2)
24-24: Added import for EMPTY_KEYWORDS constant.Importing the empty keywords constant from the parent module to maintain consistency in default values.
102-114: Added accessor methods for keywords and description fields.These methods provide access to the keywords and description fields in the serialization format, properly handling both V0 and V1 variants:
- For V0 contracts, keywords returns EMPTY_KEYWORDS and description returns None
- For V1 contracts, the actual field values are returned
This maintains backward compatibility while supporting the new fields in V1.
packages/rs-dpp/src/data_contract/serialized_version/v1/mod.rs (4)
62-69: Appropriate fields added for contract keyword search functionality.The new
keywordsanddescriptionfields are correctly declared with default serialization attributes, supporting the keywords search system requirement mentioned in the PR objectives.
135-136: Good handling of default values for V0 contracts.Properly initializing new fields with default values for backward compatibility with older contract versions.
155-156: Correctly extracting new fields from V1 contracts.These lines properly extract the new fields from the V1 version of the contract structure.
177-178: Consistent field mapping for V1 contract serialization.The new fields are correctly included in the serialization format output, maintaining consistency with the contract structure.
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_create/advanced_structure/v0/mod.rs (4)
1-2: Well-structured imports for new validation functionality.The added imports correctly support the new validation logic for keywords and description fields, including necessary error types and utilities for uniqueness validation.
Also applies to: 8-10, 15-15, 20-20
159-174: Proper keyword count validation.The check ensures contracts don't exceed the maximum allowed keywords (20), enforcing the constraint mentioned in the PR objectives.
176-229: Comprehensive keyword validation.The implementation thoroughly validates keywords for:
- Length constraints (3-50 characters)
- ASCII encoding requirement
- Uniqueness across all keywords
The validation uses a
HashSetfor efficient uniqueness checking and properly returns specific error types with context for each validation failure.
231-250: Description length validation.The optional description field is correctly validated for length constraints (3-100 characters) when present. This complements the keywords validation to ensure both new fields meet system requirements.
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/creation.rs (1)
2473-2606: Thorough test for the Search contract's document creation restriction.This test effectively verifies that the
contractKeywordsdocument type in the Search contract has theNoCreationAllowedcreation restriction mode, and properly rejects direct document creation attempts. This aligns with the PR objective of implementing a keyword search system contract.The test setup is comprehensive:
- Sets up platform and identity
- Loads and validates the Search contract configuration
- Attempts to create a document
- Verifies failure with the correct error message
This test ensures that keywords can only be added through the contract registration process, not directly.
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_create/mod.rs (1)
2451-3326: Well-structured test suite for the new keyword and description features.The test suite comprehensively covers validation scenarios for both keywords and descriptions, including:
- Maximum keyword limit (20)
- Duplicate keyword detection
- Length constraints (3-50 chars)
- ASCII-only encoding requirement
- Successful creation with valid keywords
- Description length constraints
- Creation of appropriate entries in the Search contract documents
packages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/test/tokens.rs (3)
227-227: Token description field added correctly.The description field is properly added to TokenConfigurationV0, supporting the new functionality for describing tokens.
261-262: Contract description and keywords fields added correctly.These additions support the new search features, initializing with appropriate test values.
290-290: Added support for the new parameter in token_mint.The added
falseparameter aligns with the updated method signature for the token_mint function.Also applies to: 304-305, 319-320
packages/rs-dpp/src/data_contract/accessors/v1/mod.rs (2)
67-78: Comprehensive getter methods for keywords and description fields.These methods properly extend the DataContractV1Getters trait with the ability to access the new fields, following the established pattern of the existing methods.
111-116: Appropriate setter methods for keywords and description fields.These methods complete the DataContractV1Setters trait, allowing proper mutation of the new fields.
packages/search-contract/src/v1/mod.rs (2)
5-6: Renamed document type to better reflect its purpose.Renaming from "contract" to "contractKeywords" better describes the document type's purpose, improving code clarity.
13-27: Added new document types for contract descriptions.The new document types support storing and searching both short and full descriptions, enhancing the search functionality.
The indexing by "byContractId" enables efficient lookups of descriptions by contract ID, which aligns with the expected query patterns.
packages/rs-dpp/src/data_contract/v1/accessors/mod.rs (2)
228-246: Implementation of keyword and description getters looks good.These new getter methods appropriately expose the keywords and description fields for the contract while following the existing accessor patterns. The comment documentation is clear and describes the purpose of each method. The return types are consistent with the expected usage pattern where keywords is a required field (returns direct reference) and description is optional (wrapped in Option).
296-304: Implementation of keyword and description setters looks good.The new setter methods are correctly implemented, following the existing pattern in the codebase. The methods are properly documented and have appropriate parameter types that match the corresponding field types in the struct.
packages/rs-dpp/src/data_contract/accessors/mod.rs (3)
21-21: Adding necessary import for EMPTY_KEYWORDS.The import for EMPTY_KEYWORDS is correctly added, which will be used in the keywords method implementation for backward compatibility with V0 contracts.
273-299: Clean implementation of keyword and description getters.The getter implementations follow the established pattern for handling both V0 and V1 contract versions. For V0 contracts (which don't support these new fields), appropriate fallbacks are provided:
- keywords() returns EMPTY_KEYWORDS for V0
- keywords_mut() returns None for V0
- description() returns None for V0
- description_mut() returns None for V0
This ensures backward compatibility while providing the new functionality for V1 contracts.
433-445: Clean implementation of keyword and description setters.The setter implementations correctly use the
if letpattern to set values only for V1 contracts while silently doing nothing for V0 contracts. This approach maintains backward compatibility and is consistent with the pattern used for the epoch setters (lines 419-431).packages/rs-dpp/src/data_contract/associated_token/token_configuration/v0/mod.rs (2)
60-61: Addition of optional description field looks good.The new optional
descriptionfield with#[serde(default)]attribute ensures it will properly deserialize toNonewhen not provided.
270-270: Properly initialized default value.The
descriptionfield is correctly initialized toNonein thedefault_most_restrictivemethod, maintaining consistency with the struct's definition.packages/rs-drive/src/drive/contract/insert/mod.rs (1)
2-5: Addition of new modules for description and keywords handling.The new modules
add_descriptionandadd_new_keywordswith the#[allow(clippy::too_many_arguments)]attribute are appropriately defined to support the keyword search system contract functionality mentioned in the PR objectives.packages/rs-drive/src/drive/contract/insert/insert_contract/v1/mod.rs (2)
322-332: Implementation of contract description functionality.The conditional block properly checks for the presence of a description before extending batch operations with the result of
add_new_contract_description_operations.
310-320:❓ Verification inconclusive
Implementation of contract keywords functionality.
The conditional block correctly checks if keywords exist and extends batch operations with the result from
add_new_contract_keywords_operations, which aligns with the PR objective to implement a keyword search system contract.Consider verifying that the keyword limit (up to 20 unique ASCII keywords mentioned in PR objectives) is enforced in the implementation of
add_new_contract_keywords_operations.
🏁 Script executed:
#!/bin/bash # Check for keyword validation implementation echo "Searching for keyword validation code..." rg -A 10 "keywords" --type rust packages/rs-drive/src/drive/contract/insert/add_new_keywords/Length of output: 19421
Contract Keywords Functionality Implementation Verified
The conditional block in
packages/rs-drive/src/drive/contract/insert/insert_contract/v1/mod.rscorrectly invokesadd_new_contract_keywords_operationswhen keywords exist, aligning with the PR objective of supporting keyword search functionality.
- Please verify that the implementation enforces the required limit of 20 unique ASCII keywords. The search in the
add_new_contract_keywordsmodule (including version-specific implementations in thev0subdirectory) does not reveal an explicit check. If this validation is handled upstream or within helper functions, please ensure it is well documented and properly tested.packages/rs-dpp/src/data_contract/associated_token/token_configuration/v0/accessors.rs (2)
153-156: Accessor for token description.Appropriate getter method implementation for the new
descriptionfield, keeping consistent with the accessor pattern used throughout the codebase.
229-232: Setter for token description.Proper implementation of the setter method for the
descriptionfield, maintaining consistency with other accessor methods in the struct.packages/rs-dpp/src/data_contract/associated_token/token_configuration/accessors/mod.rs (2)
145-151: LGTM: New token description getter implementation.The implementation correctly follows the same pattern as other accessors in this file, using pattern matching to delegate to the appropriate versioned implementation.
250-256: LGTM: New token description setter implementation.The implementation correctly follows the same pattern as other setters in this file, providing a consistent API for token configuration management.
packages/rs-dpp/src/data_contract/associated_token/token_configuration/accessors/v0/mod.rs (2)
69-70: LGTM: New description getter in TokenConfigurationV0Getters trait.This method is consistent with the other getter methods in this trait and correctly specifies the return type.
117-118: LGTM: New description setter in TokenConfigurationV0Setters trait.This method is consistent with the other setter methods in this trait and correctly specifies the parameter type.
packages/rs-drive/src/drive/contract/insert/add_description/mod.rs (4)
1-16: LGTM: Appropriate imports for the new module.All necessary imports are included for the functionality being implemented.
95-133: LGTM: Operations method follows established patterns.The method correctly handles version checking and delegates appropriately. The error messages are clear and descriptive.
55-93:❓ Verification inconclusive
Similar naming pattern concern for operations method.
Like the previous method, this one delegates to a
_v0suffixed method for version 1 platform. This naming pattern should be verified for consistency with the rest of the codebase.
🏁 Script executed:
#!/bin/bash # Check for similar patterns in other modules to determine if this is a consistent pattern find packages/rs-drive/src/drive/contract/ -name "*.rs" | xargs grep -l "_v0" | xargs grep -l "version.*1.*=>"Length of output: 110
Review Comment: Verify Naming Pattern Consistency
The
_v0suffix inadd_new_contract_description_add_to_operationscorrectly delegates to a version-specific implementation (version 1). However, our initial automated search for similar patterns in the contract module didn’t return results. This inconclusive outcome suggests that a manual review is needed to ensure:
- Other versioned methods across the codebase follow a consistent naming convention.
- The pattern where a version check (e.g., matching on
version { 1 => ... }) delegates to a_v0suffixed function is applied uniformly.Please verify that this naming pattern is consistent throughout related modules.
17-53:❓ Verification inconclusive
Check version-specific method naming.
The method correctly handles version checking, but there's a potential naming inconsistency. The method delegates to
add_new_contract_description_v0for version 1, which suggests it's using a version 0 implementation for a version 1 feature.
🏁 Script executed:
#!/bin/bash # Check if the v0 implementation exists and is indeed intended for v1 platform usage find packages/rs-drive/src/drive/contract/insert/add_description/ -name "*.rs" | grep -B 10 -A 3 "add_new_contract_description_v0"Length of output: 130
Verify version-specific method naming consistency
It appears that for platform version 1 the code delegates to
add_new_contract_description_v0—using a v0 implementation for a version-1 scenario. However, a search of the codebase hasn’t clearly revealed the definition ofadd_new_contract_description_v0in the expected directory. Could you please verify that this naming is intentional? If it is, adding a comment to explain the rationale would improve clarity. If not, the method name might need updating to reflect the correct version.packages/rs-drive/src/drive/contract/insert/add_new_keywords/v0/mod.rs (5)
1-15: LGTM: Appropriate imports for keyword functionality.All necessary imports are included for document creation and manipulation.
16-48: LGTM: Well-structured method for adding contract keywords.The method properly handles the creation of drive operations and fee calculation.
87-135: LGTM: Comprehensive implementation for keyword operations.The method properly iterates through the keywords, builds documents for each, and collects the operations. The implementation follows the established patterns in the codebase.
137-175: LGTM: Well-structured document creation method.The method correctly creates a document with the necessary properties for a contract keyword, including proper ID generation and metadata.
50-85:✅ Verification successful
Verify method call reference.
On line 68, the method calls
add_new_contract_keywords_operationswithout a version suffix, but the method being implemented has a_v0suffix. Verify that this reference is correct.
🏁 Script executed:
#!/bin/bash # Check for the definition of add_new_contract_keywords_operations method grep -r "fn add_new_contract_keywords_operations" packages/rs-drive/src/Length of output: 321
Method Call Reference Verified as Correct
The grep output confirms that the non-versioned methodadd_new_contract_keywords_operationsis defined in the parent module (packages/rs-drive/src/drive/contract/insert/add_new_keywords/mod.rs) and is intentionally used here. Although the versioned implementation (add_new_contract_keywords_operations_v0) exists in the v0 submodule, this call is meant to reference the common base implementation. No changes are necessary.packages/rs-drive/src/drive/contract/insert/add_description/v0/mod.rs (4)
1-15: Imports appear correct and consistent.
No issues found with the imported modules or namespaces.
50-85: Logic for batching and applying operations is clear.
This method consistently handles the “apply” mode vs. cost estimation mode. The error handling withResult<(), Error>is correctly structured to fail fast. No further concerns.
87-160: Document creation logic is well-structured.
You properly fetch “shortDescription” and “fullDescription” document types, then create the corresponding documents. The usage ofadd_document_for_contract_operationsensures consistency with the platform’s existing pattern.
178-215: Document property assignments and metadata handling look correct.
The code sets timestamps and block heights accurately, which should ensure proper lifecycle tracking for the description documents. No issues found.
...-drive-abci/src/execution/platform_events/initialization/create_genesis_state/test/tokens.rs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/keyword-search-contract/schema/v1/keyword-search-contract-documents.json (1)
48-86: Issue:shortDescriptionDocument Immutability Mismatch
According to the system requirements, theshortDescriptiondocument must be immutable. However, the schema currently sets"documentsMutable": true(line 50) and describes the document type as mutable (line 84). Please update these to reflect immutability.
Below is an example diff to correct the issue:- "documentsMutable": true, + "documentsMutable": false,And update the description accordingly:
- "description": "Short description of the contract. This document type is mutable but creation is not allowed. Creation occurs automatically internally on DataContractCreate and mirrors the contract's description field initially.", + "description": "Short description of the contract. This document type is immutable and creation is not allowed. Creation occurs automatically internally on DataContractCreate and mirrors the contract's description field initially.",packages/rs-drive/src/drive/contract/insert/add_new_keywords/v1/mod.rs (1)
70-78: Method name mismatch between call and definitionThe method being called is
add_new_contract_keywords_operationsbut the actual implementation is namedadd_new_contract_keywords_operations_v1. This will cause a compilation error.- let batch_operations = self.add_new_contract_keywords_operations( + let batch_operations = self.add_new_contract_keywords_operations_v1(
🧹 Nitpick comments (3)
packages/rs-drive/src/drive/contract/insert/add_new_keywords/v1/mod.rs (1)
144-144: Consider using&strinstead of&Stringfor better API flexibilityUsing
&strinstead of&Stringfor the keyword parameter would be more idiomatic in Rust and provide more flexibility for callers.- pub(super) fn build_contract_keyword_document_owned_v1( - &self, - contract_id: Identifier, - owner_id: Identifier, - keyword: &String, - block_info: &BlockInfo, - ) -> Result<Document, Error> { + pub(super) fn build_contract_keyword_document_owned_v1( + &self, + contract_id: Identifier, + owner_id: Identifier, + keyword: &str, + block_info: &BlockInfo, + ) -> Result<Document, Error> {packages/rs-drive/src/drive/contract/update/update_contract/v1/update_keywords/v1/mod.rs (2)
134-146: Error handling improvement opportunity for keyword extraction.The error handling for missing keywords is functional but could be more informative.
Consider enhancing the error message to include more context about which document is missing the keyword:
- Error::Drive(DriveError::CorruptedDriveState( - "keyword should exist".to_string(), - )) + Error::Drive(DriveError::CorruptedDriveState( + format!("Keyword field missing in document with ID: {}", doc.id()) + ))
91-184:❓ Verification inconclusive
Consider optimizing query performance for contracts with many keywords.
The current implementation queries all existing keywords for a contract and then performs comparisons in memory. This approach may not scale efficiently for contracts with a large number of keywords.
Consider implementing a more targeted query approach or pagination when dealing with potentially large keyword sets. This would reduce memory usage and improve performance for contracts with many keywords.
Run the following script to check if there are any test cases or examples in the codebase with a large number of keywords to better understand potential performance implications:
🏁 Script executed:
#!/bin/bash # Search for test cases or examples that might involve large numbers of keywords rg -A 2 -B 2 "keywords.*(vec|array|list).*20" --type rustLength of output: 59
Performance Concern: Evaluate and Optimize Query Scalability
The current implementation retrieves all existing keywords for a contract and then filters them in memory. While this works for smaller sets, there’s a potential scalability issue if contracts eventually hold a large number of keywords.
- The query currently fetches all matching documents instead of using targeted queries or pagination.
- No tests or examples were found simulating large keyword sets, so it’s unclear how the current approach performs under load.
Please verify the performance manually with a high-volume dataset. If scalability issues are observed, consider refactoring to use a more targeted query strategy or pagination to efficiently handle large keyword sets.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/keyword-search-contract/schema/v1/keyword-search-contract-documents.json(1 hunks)packages/rs-drive/src/drive/contract/insert/add_new_keywords/v1/mod.rs(1 hunks)packages/rs-drive/src/drive/contract/update/update_contract/v1/update_keywords/v1/mod.rs(1 hunks)packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/rs-drive/src/drive/contract/update/update_contract/v1/update_keywords/v1/mod.rs (6)
packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs (1)
drive(43-481)packages/rs-drive/src/drive/contract/insert/add_new_keywords/v1/mod.rs (1)
None(65-65)packages/rs-drive/src/drive/contract/update/update_contract/v1/update_description/v1/mod.rs (1)
None(68-68)packages/rs-drive/src/drive/contract/insert/add_description/v1/mod.rs (1)
None(65-65)packages/rs-drive/src/drive/contract/update/update_contract/v1/mod.rs (1)
None(157-157)packages/rs-drive/src/query/mod.rs (1)
all_items_query(395-411)
packages/rs-drive/src/drive/contract/insert/add_new_keywords/v1/mod.rs (6)
packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs (1)
drive(43-481)packages/rs-drive/src/state_transition_action/batch/batched_transition/document_transition/document_create_transition_action/v0/mod.rs (1)
block_info(53-53)packages/rs-drive/src/drive/contract/update/update_contract/v1/update_keywords/v1/mod.rs (1)
None(67-67)packages/rs-drive/src/drive/contract/update/update_contract/v1/update_description/v1/mod.rs (1)
None(68-68)packages/rs-drive/src/drive/contract/insert/add_description/v1/mod.rs (1)
None(65-65)packages/rs-drive/src/drive/contract/update/update_contract/v1/mod.rs (1)
None(157-157)
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (rs-dapi-client) / Check each feature
- GitHub Check: Rust packages (wasm-dpp) / Formatting
- GitHub Check: Rust packages (wasm-dpp) / Linting
- GitHub Check: Rust packages (token-history-contract) / Unused dependencies
- GitHub Check: Rust packages (token-history-contract) / Formatting
- GitHub Check: Rust packages (rs-dapi-client) / Tests
- GitHub Check: Rust packages (dapi-grpc) / Linting
- GitHub Check: Rust packages (dapi-grpc) / Check each feature
- GitHub Check: Rust packages (dapi-grpc) / Tests
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive-abci) / Check each feature
- GitHub Check: Rust packages (drive-abci) / Linting
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
🔇 Additional comments (10)
packages/keyword-search-contract/schema/v1/keyword-search-contract-documents.json (2)
1-47: Review ofcontractKeywordsDocument Type Schema
The schema forcontractKeywordsis well-defined. The constraints on thekeywordandcontractIdfields, along with the proper index definitions and enforced required attributes, ensure robust validation and data integrity.
87-125: Review offullDescriptionDocument Type Schema
ThefullDescriptiondocument type schema is implemented according to the requirements. The mutability (documentsMutable": true), description constraints (up to 10,000 characters), and unique indexing oncontractIdare all appropriately defined.packages/rs-drive/src/drive/contract/insert/add_new_keywords/v1/mod.rs (3)
112-112: Good comment explaining the use of keyword as entropyThe comment clearly explains why the keyword is being used as a source of entropy for document ID generation.
16-49: Well-structured implementation for adding keywords to contractsThe main method is well-designed with clear separation of concerns:
- It creates a vector for operations
- Delegates the creation of operations to a separate method
- Calculates fees based on the operations
- Returns the fee result
This follows good software engineering principles and maintains consistency with other parts of the codebase.
159-175: Complete document initialization with all required metadataThe document creation correctly initializes all required fields, including transaction-specific information like block heights and timestamps. The code properly sets the revision and update fields to None for newly created documents.
packages/rs-drive/src/drive/contract/update/update_contract/v1/update_keywords/v1/mod.rs (5)
1-17: Appropriate imports for the module's functionality.The imports are well-organized and provide all necessary dependencies for the functionality being implemented. The code properly imports from both the crate's internal modules and external dependencies.
18-51: Well-structured fee calculation approach for keyword updates.This method effectively delegates the operational work to
update_contract_keywords_add_to_operations_v1and then calculates fees based on the resulting operations. The separation of concerns between operation generation and fee calculation is a good design pattern.
53-89: Clean implementation of operation generation and application.This method correctly determines whether to estimate costs based on the
applyflag and delegates to more specific methods for operation generation. The pattern of generating operations first and then applying them is a good approach for maintaining consistency and allowing for cost estimation.
149-161: Efficient approach for removing outdated keywords.The code correctly identifies and removes keywords that exist in the database but aren't in the new keyword set. This ensures that keywords are properly synchronized with each update.
163-181: Well-implemented addition of new keywords.The code efficiently identifies and adds only keywords that don't already exist in the database. This prevents duplicate operations and ensures data consistency.
packages/rs-drive/src/drive/contract/update/update_contract/v1/update_keywords/v1/mod.rs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
packages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/test/tokens.rs (1)
287-288: 💡 Verification agent🧩 Analysis chain
Verify the impact of the additional parameter in token_mint calls
An additional
falseparameter has been added to all token_mint calls. Please ensure this change is consistent across the codebase and properly documented.
🏁 Script executed:
#!/bin/bash # Check for other instances of token_mint to verify parameter usage consistency rg -A 3 "token_mint\(" --type rust | grep -v "false," || echo "All token_mint calls appear to have been updated"Length of output: 9559
Action Required: Ensure Consistent Update of Token Mint Parameters
The new
falseparameter(s) added in the genesis state tests (see packages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/test/tokens.rs, lines 287–288) do not appear in other token mint invocation sites (e.g., in packages/rs-drive/src/drive/tokens/mint/mod.rs, prove_token_total_supply_and_aggregated_identity_balances/v0/mod.rs, and balance proof modules). Please review whether all calls totoken_mintshould include the additionalfalseparameter. Update the inconsistent call sites or add documentation to clarify if any exceptions are intentional.
🧹 Nitpick comments (2)
packages/wasm-dpp/src/errors/consensus/consensus_error.rs (1)
64-64: Consider breaking the long import line into multiple lines for better readability.This import statement is extremely long and might be difficult to read and maintain. In Rust, it's a common practice to split long import lists into multiple lines, potentially grouping related imports together.
-use dpp::consensus::basic::data_contract::{ContestedUniqueIndexOnMutableDocumentTypeError, ContestedUniqueIndexWithUniqueIndexError, DataContractTokenConfigurationUpdateError, DuplicateKeywordsError, GroupExceedsMaxMembersError, GroupMemberHasPowerOfZeroError, GroupMemberHasPowerOverLimitError, GroupNonUnilateralMemberPowerHasLessThanRequiredPowerError, GroupPositionDoesNotExistError, GroupTotalPowerLessThanRequiredError, InvalidDescriptionLengthError, InvalidDocumentTypeRequiredSecurityLevelError, InvalidKeywordLengthError, InvalidTokenBaseSupplyError, InvalidTokenDistributionFunctionDivideByZeroError, InvalidTokenDistributionFunctionIncoherenceError, InvalidTokenDistributionFunctionInvalidParameterError, InvalidTokenDistributionFunctionInvalidParameterTupleError, NonContiguousContractGroupPositionsError, NonContiguousContractTokenPositionsError, TokenPaymentByBurningOnlyAllowedOnInternalTokenError, TooManyKeywordsError, UnknownDocumentActionTokenEffectError, UnknownDocumentCreationRestrictionModeError, UnknownGasFeesPaidByError, UnknownSecurityLevelError, UnknownStorageKeyRequirementsError, UnknownTradeModeError, UnknownTransferableTypeError}; +use dpp::consensus::basic::data_contract::{ + ContestedUniqueIndexOnMutableDocumentTypeError, ContestedUniqueIndexWithUniqueIndexError, + DataContractTokenConfigurationUpdateError, DuplicateKeywordsError, + GroupExceedsMaxMembersError, GroupMemberHasPowerOfZeroError, + GroupMemberHasPowerOverLimitError, GroupNonUnilateralMemberPowerHasLessThanRequiredPowerError, + GroupPositionDoesNotExistError, GroupTotalPowerLessThanRequiredError, + InvalidDescriptionLengthError, InvalidDocumentTypeRequiredSecurityLevelError, + InvalidKeywordLengthError, InvalidTokenBaseSupplyError, + InvalidTokenDistributionFunctionDivideByZeroError, + InvalidTokenDistributionFunctionIncoherenceError, + InvalidTokenDistributionFunctionInvalidParameterError, + InvalidTokenDistributionFunctionInvalidParameterTupleError, + NonContiguousContractGroupPositionsError, NonContiguousContractTokenPositionsError, + TokenPaymentByBurningOnlyAllowedOnInternalTokenError, TooManyKeywordsError, + UnknownDocumentActionTokenEffectError, UnknownDocumentCreationRestrictionModeError, + UnknownGasFeesPaidByError, UnknownSecurityLevelError, UnknownStorageKeyRequirementsError, + UnknownTradeModeError, UnknownTransferableTypeError +};packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/mod.rs (1)
2553-2746: Consider parameterizing the creation restriction tests.The three document creation restriction tests share similar structure and could be refactored into a parameterized test to reduce duplication.
Something like:
#[test] fn should_err_when_creating_restricted_documents() { let doc_types = ["contractKeywords", "shortDescription", "fullDescription"]; let seed_offsets = [0, 1, 2]; for (i, doc_type) in doc_types.iter().enumerate() { let seed = 42 + i as u64; // Test setup code... // Create document of specific type... let (doc, entropy) = build_random_doc_of_type( &mut rng, doc_type, identity.id(), &contract, platform_version, ); // Create and execute transition... // Assert error... } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
Cargo.toml(1 hunks)packages/data-contracts/Cargo.toml(1 hunks)packages/keyword-search-contract/Cargo.toml(1 hunks)packages/rs-dpp/src/data_contract/associated_token/token_configuration/v0/mod.rs(2 hunks)packages/rs-dpp/src/errors/consensus/codes.rs(1 hunks)packages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/test/tokens.rs(2 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/mod.rs(2 hunks)packages/wasm-dpp/src/errors/consensus/consensus_error.rs(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/data-contracts/Cargo.toml
- packages/rs-dpp/src/data_contract/associated_token/token_configuration/v0/mod.rs
- Cargo.toml
- packages/keyword-search-contract/Cargo.toml
- packages/rs-dpp/src/errors/consensus/codes.rs
- packages/rs-drive-abci/src/execution/platform_events/protocol_upgrade/perform_events_on_first_block_of_protocol_change/v0/mod.rs
⏰ Context from checks skipped due to timeout of 90000ms (18)
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Rust packages (drive) / Detect immutable structure changes
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (token-history-contract) / Linting
- GitHub Check: Rust packages (dapi-grpc) / Unused dependencies
- GitHub Check: Rust packages (token-history-contract) / Tests
- GitHub Check: Rust packages (dapi-grpc) / Formatting
- GitHub Check: Rust packages (drive-abci) / Check each feature
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive-abci) / Unused dependencies
- GitHub Check: Rust packages (drive-abci) / 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 (7)
packages/wasm-dpp/src/errors/consensus/consensus_error.rs (1)
773-784: New error handlers look good and follow the established pattern.The implementation of these new error handlers for keyword and description validation follows the consistent pattern used throughout the file. They correctly use the
generic_consensus_error!macro to convert the errors into JavaScript values.These handlers will properly translate the validation errors when users exceed the limits (e.g., more than 20 keywords), use duplicate keywords, or exceed the length restrictions for keywords and descriptions.
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/mod.rs (3)
2516-2959: Well-structured unit tests for the keyword search contract!The implementation thoroughly tests the key aspects of the keyword search contract functionality:
- Restricting direct creation of documents (contractKeywords, shortDescription, fullDescription)
- Allowing owners to update shortDescription documents
- Allowing owners to delete keyword documents
The tests align well with the PR objectives of adding keyword and description support to contracts.
2526-2551: Good helper function implementation!This utility function effectively creates random documents with proper identity binding and consistently handles entropy, preventing ID mismatches in tests.
2754-2819: Well-implemented contract setup function!This helper function correctly sets up a contract with keywords and description, demonstrating the intended functionality of the new fields while providing a clean test fixture.
packages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/test/tokens.rs (3)
225-225: ✅ Description added to TokenConfigurationThe addition of the description field to the TokenConfigurationV0 structure aligns with the PR objective of supporting descriptions for token configurations.
228-235: Token localization information added to support display featuresThe token localization features have been properly implemented with the previously undefined
should_capitalizevariable now correctly set tofalse.
259-260: ✅ Contract keywords and description implementedThe implementation adds both the keywords and description fields to data contracts as described in the PR objectives. The example includes two keywords ("cat" and "white") and a sample description.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_create/mod.rs (1)
3154-3165: Consider extracting contract ID parsing to a helper functionThis code segment for extracting the data contract ID from the unique identifier appears in multiple tests. Consider extracting it to a helper function to improve readability and maintainability.
let unique_identifiers = transition.unique_identifiers(); let unique_identifier = unique_identifiers .first() .expect("expected at least one unique identifier"); let data_contract_id_str = unique_identifier .as_str() .split('-') .last() .expect("split contract id"); let data_contract_id = Identifier::from_string(data_contract_id_str, Encoding::Base58) .expect("identifier");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_create/mod.rs(1 hunks)packages/rs-platform-version/src/version/v9.rs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/rs-platform-version/src/version/v9.rs
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_create/mod.rs (5)
packages/rs-dpp/src/data_contract/accessors/mod.rs (1)
keywords(273-278)packages/rs-dpp/src/data_contract/v1/serialization/mod.rs (3)
data_contract(43-71)data_contract(73-119)data_contract(121-177)packages/rs-drive-abci/src/platform_types/platform_state/mod.rs (1)
v0(116-120)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_update/mod.rs (1)
setup_identity(103-159)packages/rs-drive/src/query/mod.rs (1)
all_items_query(395-411)
⏰ Context from checks skipped due to timeout of 90000ms (19)
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Rust packages (drive) / Formatting
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Formatting
- GitHub Check: Rust packages (wasm-dpp) / Unused dependencies
- GitHub Check: Rust packages (wasm-dpp) / Linting
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (dapi-grpc) / Check each feature
- GitHub Check: Rust packages (dapi-grpc) / Tests
- GitHub Check: Rust packages (rs-dapi-client) / Linting
- GitHub Check: Rust packages (dash-sdk) / Check each feature
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- 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 JS packages / Build JS
🔇 Additional comments (4)
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_create/mod.rs (4)
2451-2940: Well-structured test module for keyword validationThis test module thoroughly covers all the validation rules for keywords in data contracts, including maximum count (20), uniqueness, and length constraints (3-50 characters). It also effectively verifies that valid keywords are correctly stored in both the contract and the keyword search system contract.
2956-2977: Good use of helper function to reduce code duplicationThe
base_contract_value_with_keywordhelper function effectively reduces code duplication across the description tests, making the code more maintainable.
2942-3260: Description validation tests ensure proper constraintsThese tests thoroughly validate that descriptions must be between 3 and 100 characters, and verify that both shortDescription and fullDescription documents are correctly created in the keyword search system contract.
3045-3050: Clear comment about different description length constraintsGood documentation explaining that this test checks a 101-char description that exceeds the shortDescription limit but would be valid for the contract itself (which has a 10,000 char limit). This clearly documents the different validation boundaries.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_update/mod.rs (2)
2177-2218: Consider enhancing description document retrieval to include full descriptions.The current
description_docs_for_contracthelper only fetches "shortDescription" documents. According to the PR objectives, there are also "fullDescription" documents that support up to 10,000 characters. Consider adding a parameter to this helper function to optionally fetch full descriptions as well, which would make it more versatile for testing both types of description documents.- fn description_docs_for_contract( - platform: &TempPlatform<MockCoreRPCLike>, - contract_id: Identifier, - platform_version: &PlatformVersion, - ) -> String { + fn description_docs_for_contract( + platform: &TempPlatform<MockCoreRPCLike>, + contract_id: Identifier, + platform_version: &PlatformVersion, + full_description: bool, + ) -> String { let search_contract = load_system_data_contract(SystemDataContract::KeywordSearch, platform_version) .unwrap(); let doc_type = search_contract - .document_type_for_name("shortDescription") + .document_type_for_name(if full_description { "fullDescription" } else { "shortDescription" }) .unwrap();
2289-2341: Add tests for full descriptions in addition to short descriptions.The current test only verifies the functionality of short descriptions (up to 100 chars). According to the PR objectives, there's also support for "fullDescription" documents that can hold up to 10,000 characters. Consider adding tests to verify that full descriptions are also correctly created and updated.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
packages/rs-dpp/src/data_contract/methods/validate_update/v0/mod.rs(2 hunks)packages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/test/tokens.rs(4 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/direct_selling/mod.rs(1 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_create/basic_structure/v0/mod.rs(2 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_create/mod.rs(1 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_update/mod.rs(1 hunks)packages/rs-platform-version/src/version/v9.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/rs-platform-version/src/version/v9.rs
- packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/direct_selling/mod.rs
- packages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/test/tokens.rs
- packages/rs-dpp/src/data_contract/methods/validate_update/v0/mod.rs
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: pauldelucia
PR: dashpay/platform#2523
File: packages/rs-drive/src/drive/contract/insert/add_new_keywords/v1/mod.rs:22-22
Timestamp: 2025-04-11T09:08:11.528Z
Learning: The validation for data contract keywords (ensuring no more than 20 unique keywords with appropriate length) is handled in the validation logic elsewhere in the codebase (particularly in the advanced structure validation and data contract update validation), not in the Drive implementation that handles storage operations.
Learnt from: pauldelucia
PR: dashpay/platform#2523
File: packages/rs-drive/src/drive/contract/insert/add_new_keywords/v1/mod.rs:22-22
Timestamp: 2025-04-11T09:08:11.528Z
Learning: The validation for data contract keywords (ensuring no more than 20 unique keywords with appropriate length) is handled in the validation logic elsewhere in the codebase (particularly in the advanced structure validation and data contract update validation), not in the Drive implementation that handles storage operations.
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_create/basic_structure/v0/mod.rs (3)
Learnt from: pauldelucia
PR: dashpay/platform#2523
File: packages/rs-drive/src/drive/contract/update/update_contract/v1/update_description/v1/mod.rs:147-151
Timestamp: 2025-04-11T09:08:05.652Z
Learning: Description length validation for data contracts is already handled in the data contract validation process, specifically in packages/rs-dpp/src/data_contract/methods/validate_update/v0/mod.rs.
Learnt from: pauldelucia
PR: dashpay/platform#2523
File: packages/rs-drive/src/drive/contract/insert/add_new_keywords/v1/mod.rs:22-22
Timestamp: 2025-04-11T09:08:11.528Z
Learning: The validation for data contract keywords (ensuring no more than 20 unique keywords with appropriate length) is handled in the validation logic elsewhere in the codebase (particularly in the advanced structure validation and data contract update validation), not in the Drive implementation that handles storage operations.
Learnt from: pauldelucia
PR: dashpay/platform#2523
File: packages/rs-drive/src/drive/contract/insert/add_new_keywords/v1/mod.rs:22-22
Timestamp: 2025-04-11T09:08:11.528Z
Learning: The validation for data contract keywords (ensuring no more than 20 unique keywords with appropriate length) is handled in the validation logic elsewhere in the codebase (particularly in the advanced structure validation and data contract update validation), not in the Drive implementation that handles storage operations.
⏰ Context from checks skipped due to timeout of 90000ms (18)
- GitHub Check: Rust packages (dapi-grpc) / Detect immutable structure changes
- GitHub Check: Rust packages (dapi-grpc) / Formatting
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (dpns-contract) / Unused dependencies
- GitHub Check: Rust packages (dpns-contract) / Linting
- GitHub Check: Rust packages (dpns-contract) / Tests
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Unused dependencies
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Tests
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Formatting
- GitHub Check: Rust packages (token-history-contract) / Tests
- GitHub Check: Rust packages (token-history-contract) / Linting
- GitHub Check: Rust packages (drive-abci) / Check each feature
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (dpp) / Check each feature
- 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 (12)
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_create/basic_structure/v0/mod.rs (4)
1-5: Clear import additions for the new error types.The added imports for
HashSetand the new error types are correctly organized. TheHashSetis used for uniqueness checking, and the new error types properly reflect the validation constraints being enforced.
82-92: Well-implemented keyword count validation.This validation properly ensures that data contracts don't exceed the maximum of 20 keywords. The error contains both the contract ID and the actual count for helpful debugging.
94-120: Comprehensive keyword validation with clear separation of concerns.The validation logic nicely separates the two constraints (length and uniqueness) into sequential checks:
- First validates that each keyword is between 3-50 characters
- Then ensures keywords are unique using a HashSet
This approach results in clean, understandable code with specific error messages. Error objects contain the problematic keyword for better error reporting.
122-136: Proper optional description length validation.The validation correctly handles the optional nature of the description field and enforces the 3-100 character length constraint. The error reporting includes both the contract ID and the problematic description.
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_create/mod.rs (2)
2474-2959: Well-structured and thorough tests for keyword validation and storage.These tests comprehensively cover all keyword validation rules:
- Maximum count (20)
- Uniqueness requirement
- Length constraints (3-50 characters)
The success test additionally verifies that keywords are properly:
- Persisted in the data contract
- Indexed in the keyword search contract with appropriate documents
- Correctly linked back to the source contract via its ID
This ensures both validation and storage/indexing are working as expected.
2961-3277: Excellent test coverage for description validation and document creation.These tests thoroughly verify:
- Description length constraints (3-100 characters)
- Proper persistence in the data contract
- Creation of both shortDescription and fullDescription documents
- Correct linking between these documents and the source contract
The helper function
base_contract_value_with_keywordis a good refactoring that eliminates code duplication and ensures test consistency.packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_update/mod.rs (6)
1612-1992: Well-structured test module for keyword updates with comprehensive test coverage.The
keyword_updatesmodule provides thorough testing for the new contract keyword functionality, including validation checks for length constraints (3-50 chars), duplicate detection, and maximum count (20). Helper functions are well-designed and the positive test case properly verifies that keyword updates replace old documents with new ones.
1637-1724: Good helper function implementation for creating contracts with keywords.The
create_contract_with_keywordshelper is well-implemented, loading a base contract from a JSON file, adding keywords, and properly handling the transaction lifecycle. It also correctly extracts the contract ID from the transition for subsequent operations.
1728-1802: Well-designed update helper with proper error handling.The
apply_keyword_updatefunction handles the complete flow of fetching an existing contract, modifying its keywords, creating and applying an update transition, and managing the transaction. The Result return type with error propagation is a good design choice.
1852-1899: Excellent use of macros for test generation.The
invalid_update_test!macro effectively reduces code duplication for negative test cases while ensuring consistent verification that:
- The error matches the expected type
- The original keyword documents remain unchanged after a failed update
1932-1991: Comprehensive positive test for keyword updates.This test properly verifies that:
- Initial contract creation with keywords succeeds
- Keywords are correctly stored and indexed
- Update operation replaces old keywords with new ones
- Both the contract and search documents reflect the changes
1994-2342: Well-structured test module for description updates with appropriate validation.The
description_updatesmodule follows a similar pattern to the keyword updates module, providing good test coverage for the new contract description functionality. The validation checks for description length (3-100 chars) are properly tested, and the positive case verifies that description updates work as expected.
QuantumExplorer
changed the title
packages/rs-drive/src/drive/contract/insert/add_description/mod.rs
Outdated
Show resolved
Hide resolved
packages/rs-drive/src/drive/contract/insert/add_description/mod.rs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (9)
packages/rs-drive/src/drive/contract/update/update_description/mod.rs (1)
53-55: Typo in doc‑string (“LowLeveLDriveOperations”)Minor, but this appears in several docs; consider fixing to
LowLevelDriveOperationsfor consistency.packages/rs-drive/src/drive/contract/update/update_description/v0/mod.rs (2)
135-140: Expose the corruption‑error string as a constantThe hard‑coded string will be duplicated in tests and other versions.
Extracting it avoids typos and enables future localisation.- return Err(Error::Drive(DriveError::CorruptedContractIndexes( - "There should be only one `shortDescription` document per contract in the Keyword Search contract".to_string(), - ))); + const DUPLICATE_SHORT_DESC_ERR: &str = + "There should be only one `shortDescription` document per contract in the Keyword Search contract"; + return Err(Error::Drive(DriveError::CorruptedContractIndexes( + DUPLICATE_SHORT_DESC_ERR.to_string(), + )));
164-165: Avoid unnecessary cloning of the contract
DocumentAndContractInfoonly needs a reference; cloning the wholeDataContractis wasteful.- contract: &contract.clone(), + contract: &contract,packages/rs-drive/src/drive/contract/update/update_keywords/v0/mod.rs (3)
130-135: Guard against duplicated keywords in the incoming sliceIf
keywordsaccidentally contains duplicates, we will try to insert duplicate
documents, triggering validation errors later. A fast, allocation‑free fix is
to deduplicate first:let uniq: std::collections::HashSet<_> = keywords.iter().collect();(or switch the entire algorithm to
HashSetmembership checks).
137-149: Replace O(n²) membership checks withHashSet
keywords.contains(kw)inside a loop overexistingmakes the worst‑case complexity quadratic.
TurningkeywordsintoHashSet<&str>once at the start brings it down to linear time
and simplifies the later “keywords_to_add” construction.
151-158:keywords_to_addcan be avoided entirelyAfter introducing the
HashSetabove, iterate directly:for kw in new_keywords_set.difference(&existing.keys().collect()) { … }reducing allocations and keeping the intent clearer.
packages/rs-drive/src/drive/contract/insert/add_description/mod.rs (1)
19-28: Prefer&strover&Stringin public APIsThe three public wrappers accept
description: &String.
Using&stris more idiomatic, avoids unnecessary borrowing conversions at call‑sites, and
makes the API consistent with other Drive methods.- description: &String, + description: &str,(Propagate the change to the internal v0 methods.)
packages/rs-drive/src/drive/contract/insert/add_description/v0/mod.rs (2)
24-25: Prefer&strover&Stringin public signaturesAccepting
&Stringis unnecessarily restrictive and forces callers to own aString.
Changing the type to&strprovides more ergonomic and general‑purpose APIs without any runtime cost.Example for one function (the same applies to the others):
-pub(super) fn add_new_contract_description_v0( - &self, - contract_id: Identifier, - owner_id: Identifier, - description: &String, +pub(super) fn add_new_contract_description_v0( + &self, + contract_id: Identifier, + owner_id: Identifier, + description: &str,This minor change improves usability and follows standard Rust conventions.
Also applies to: 58-59, 96-97, 175-176
52-54: Typo: “LowLeveL” → “Low‑Level”Both doc comments spell “LowLeveL” with a rogue capital L in the middle.
Consider fixing for readability:- /// Creates and applies the LowLeveLDriveOperations needed to create + /// Creates and applies the low‑level Drive operations needed to createAlso applies to: 90-92
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
packages/keyword-search-contract/schema/v1/keyword-search-contract-documents.json(1 hunks)packages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/test/tokens.rs(4 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_create/basic_structure/v0/mod.rs(2 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_update/mod.rs(3 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/mod.rs(17 hunks)packages/rs-drive/src/drive/contract/insert/add_description/mod.rs(1 hunks)packages/rs-drive/src/drive/contract/insert/add_description/v0/mod.rs(1 hunks)packages/rs-drive/src/drive/contract/insert/add_new_keywords/mod.rs(1 hunks)packages/rs-drive/src/drive/contract/insert/add_new_keywords/v0/mod.rs(1 hunks)packages/rs-drive/src/drive/contract/update/mod.rs(1 hunks)packages/rs-drive/src/drive/contract/update/update_contract/v1/mod.rs(2 hunks)packages/rs-drive/src/drive/contract/update/update_description/mod.rs(1 hunks)packages/rs-drive/src/drive/contract/update/update_description/v0/mod.rs(1 hunks)packages/rs-drive/src/drive/contract/update/update_keywords/mod.rs(1 hunks)packages/rs-drive/src/drive/contract/update/update_keywords/v0/mod.rs(1 hunks)packages/rs-drive/src/drive/document/delete/delete_document_for_contract_operations/mod.rs(1 hunks)packages/rs-drive/src/drive/document/delete/delete_document_for_contract_operations/v0/mod.rs(1 hunks)packages/rs-platform-version/src/version/drive_versions/drive_contract_method_versions/mod.rs(1 hunks)packages/rs-platform-version/src/version/drive_versions/drive_contract_method_versions/v1.rs(1 hunks)packages/rs-platform-version/src/version/drive_versions/drive_contract_method_versions/v2.rs(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- packages/rs-drive/src/drive/contract/update/mod.rs
- packages/rs-platform-version/src/version/drive_versions/drive_contract_method_versions/mod.rs
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/rs-drive/src/drive/contract/update/update_contract/v1/mod.rs
- packages/rs-platform-version/src/version/drive_versions/drive_contract_method_versions/v2.rs
- packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_create/basic_structure/v0/mod.rs
- packages/rs-drive/src/drive/contract/insert/add_new_keywords/mod.rs
- packages/keyword-search-contract/schema/v1/keyword-search-contract-documents.json
- packages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/test/tokens.rs
- packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/mod.rs
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Detect immutable structure changes
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Linting
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Tests
- GitHub Check: Rust packages (wallet-utils-contract) / Unused dependencies
- GitHub Check: Rust packages (wallet-utils-contract) / Formatting
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (dash-sdk) / Check each feature
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (dash-sdk) / Unused dependencies
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Unused dependencies
- GitHub Check: Rust packages (wasm-dpp) / Linting
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
🔇 Additional comments (10)
packages/rs-platform-version/src/version/drive_versions/drive_contract_method_versions/v1.rs (2)
21-22: LGTM: Version support for contract keyword functionalityThe additions to
DriveContractInsertMethodVersionscorrectly establish versioning for the new keyword management feature, following the existing pattern of initializing new methods at version 0.
25-27: LGTM: Version support for contract description functionalityThe expansion of
DriveContractUpdateMethodVersionsfollows the proper pattern for version control in the platform, enabling the implementation of contract keyword and description updates.packages/rs-drive/src/drive/document/delete/delete_document_for_contract_operations/mod.rs (1)
69-119: Well-implemented forced document deletion for system contractsThe new method correctly implements a mechanism to bypass document deletion restrictions specifically for system data contracts. The implementation follows existing patterns for versioned method dispatch and error handling.
This is an essential addition for the keyword search system contract, as it may need special handling of its documents outside normal contract restrictions.
packages/rs-drive/src/drive/document/delete/delete_document_for_contract_operations/v0/mod.rs (2)
58-59: Good refactoring: Clean separation of permission check and deletion logicThe
delete_document_for_contract_operations_v0method now properly separates permission checking from the actual deletion logic by delegating to the new forced deletion method after permission verification.
61-174: Well-structured implementation of forced document deletionThe extracted deletion logic maintains all the necessary steps from the original implementation:
- History preservation check
- Document path construction
- Cost estimation
- Document retrieval and validation
- Document and index removal
The refactoring follows good software design principles by separating concerns and enabling code reuse.
packages/rs-drive/src/drive/contract/update/update_keywords/mod.rs (3)
1-52: Well-designed primary method for updating contract keywordsThe implementation correctly follows the established pattern for versioned method dispatch and provides a comprehensive method for updating keywords with appropriate error handling.
53-89: Flexible approach to operation buildingThis helper method allows for integration with existing operation batches, providing flexibility in how keyword updates are applied within larger transactions.
91-127:✅ Verification successful
Comprehensive API design with cost estimation support
The third method variant completes the API by providing operations generation with optional cost estimation, following the established pattern in the codebase.
Verify that keyword validation (uniqueness, maximum count of 20 as mentioned in PR objectives) is implemented either in the v0 implementation or elsewhere in the validation pipeline:
🏁 Script executed:
#!/bin/bash # Check for keyword validation implementation rg -A 5 "keywords.*valid|valid.*keywords" --glob "*.rs"Length of output: 5059
🏁 Script executed:
#!/bin/bash # Locate v0 implementation of update_contract_keywords_operations_v0 rg -n -A10 "fn update_contract_keywords_operations_v0" --glob "*.rs" # Find the ABIC validation handler for data_contract_update rg -l "data_contract_update" packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions # Search for keyword validation logic in data_contract_update handlers rg -n -A10 "keywords" packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_updateLength of output: 25839
API design is sound; keyword validation enforced in state‑transition pipeline
The new
update_contract_keywords_operationsmethod—including optional cost estimation—follows the established Drive API pattern. Keyword constraints (maximum of 20, uniqueness, length) are already validated upstream in the DataContractUpdateTransition:• See tests in
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_update/mod.rs
– TooManyKeywordsError at lines ~1902–1908
– DuplicateKeywordsError at lines ~1911–1914
– InvalidKeywordLengthError at lines ~1917–1920No additional validation needed in the v0 Drive operations.
packages/rs-drive/src/drive/contract/insert/add_new_keywords/v0/mod.rs (1)
106-114: Entropy choice looks solid – just a quick 👍Using
contract_id || keywordas entropy guarantees deterministic, collision‑free IDs for unique keywords per contract; nice touch!packages/rs-drive/src/drive/contract/update/update_description/v0/mod.rs (1)
143-153: Double‑check the boolean argument order
add_new_contract_description_operations_v0receivestrueas the 4ᵗʰ positional argument.
Ininsert/add_descriptionthe 4ᵗʰ argument (is_update?,apply?, …) is passed asfalse.
If the two call‑sites are meant to convey different semantics, please document them clearly.
If they were meant to be identical, this is a silent bug that will survive compilation.
packages/rs-drive/src/drive/contract/update/update_description/mod.rs
Outdated
Show resolved
Hide resolved
...abci/src/execution/validation/state_transition/state_transitions/data_contract_update/mod.rs
Show resolved
Hide resolved
...abci/src/execution/validation/state_transition/state_transitions/data_contract_update/mod.rs
Outdated
Show resolved
Hide resolved
packages/rs-drive/src/drive/contract/update/update_description/v0/mod.rs
Show resolved
Hide resolved
packages/rs-drive/src/drive/contract/insert/add_description/v0/mod.rs
Outdated
Show resolved
Hide resolved
packages/rs-drive/src/drive/contract/insert/add_description/v0/mod.rs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/rs-drive/src/drive/contract/insert/add_description/v0/mod.rs (1)
201-205:⚠️ Potential issue
fullDescriptiondocuments must start withrevision = Some(1)
fullDescriptionis defined as mutable in the Keyword Search contract schema; consensus validation requires every mutable document to haverevisionset to1on creation.
Leaving it asNonewill cause the state transition to fail.- revision: None, + revision: if full_description { Some(1) } else { None },This issue was flagged earlier but the fix didn’t make it into the code — without it, contract insertion or update that includes a full description will be rejected at validation time.
🧹 Nitpick comments (4)
packages/rs-drive/src/drive/contract/insert/add_description/mod.rs (2)
21-29: Prefer&strinstead of&Stringfor borrowed string parametersAll three public APIs take
description: &String. Accepting&stris more idiomatic, removes an unnecessary layer of indirection, and lets callers pass string literals without explicit allocation.- description: &String, + description: &str,(Adjust the downstream
v0calls and helper functions accordingly.)
55-57: Typo in the doc‑comment (LowLeveLDriveOperations)The capital “L” in the middle of “LeveL” looks accidental and may confuse readers / search.
- /// Creates and applies the LowLeveLDriveOperations needed to create + /// Creates and applies the LowLevelDriveOperations needed to createpackages/rs-drive/src/drive/contract/insert/add_description/v0/mod.rs (2)
178-185: Borrow&strinstead of&StringAs in the public wrapper, the helper should take
&str:- description: &String, + description: &str,This change trickles down only within this private module and avoids needless
Stringallocations at call‑sites.
111-141: Minor duplication – consider extracting a helper to build & insert one description documentThe logic that (1) selects the document type, (2) builds the document and (3) invokes
add_document_for_contract_operationsis repeated for short and full descriptions. Extracting a small closure / helper would remove ~20 lines and reduce future maintenance overhead.Purely an optional clean‑up; no functional impact.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/rs-drive/src/drive/contract/insert/add_description/mod.rs(1 hunks)packages/rs-drive/src/drive/contract/insert/add_description/v0/mod.rs(1 hunks)packages/rs-drive/src/drive/contract/insert/insert_contract/v1/mod.rs(1 hunks)packages/rs-drive/src/drive/contract/update/update_description/v0/mod.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/rs-drive/src/drive/contract/update/update_description/v0/mod.rs
- packages/rs-drive/src/drive/contract/insert/insert_contract/v1/mod.rs
⏰ Context from checks skipped due to timeout of 90000ms (19)
- GitHub Check: Rust packages (dpp) / Formatting
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (drive-abci) / Check each feature
- GitHub Check: Rust packages (drive-abci) / Formatting
- GitHub Check: Rust packages (dapi-grpc) / Check each feature
- GitHub Check: Rust packages (dapi-grpc) / Formatting
- GitHub Check: Rust packages (dapi-grpc) / Linting
- GitHub Check: Rust packages (dapi-grpc) / Tests
- GitHub Check: Rust packages (dapi-grpc) / Unused dependencies
- GitHub Check: Rust packages (dpns-contract) / Formatting
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Tests
- GitHub Check: Rust packages (dpns-contract) / Unused dependencies
- GitHub Check: Rust packages (dpns-contract) / Linting
- GitHub Check: Rust packages (dpns-contract) / Tests
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Unused dependencies
- GitHub Check: Rust packages (withdrawals-contract) / Tests
- GitHub Check: Build JS packages / Build JS
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/rs-drive/src/drive/contract/insert/add_description/v0/mod.rs (1)
174-219: Consider adding validation for empty descriptionsThe method doesn't check for empty descriptions. While validation might happen elsewhere, consider adding a check here to prevent creating documents with empty descriptions, or document that this validation should happen at a higher level.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/rs-drive/src/drive/contract/insert/add_description/v0/mod.rs(1 hunks)packages/rs-drive/src/drive/contract/update/update_description/v0/mod.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/rs-drive/src/drive/contract/update/update_description/v0/mod.rs
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/rs-drive/src/drive/contract/insert/add_description/v0/mod.rs (7)
packages/rs-dpp/src/data_contract/v1/serialization/mod.rs (3)
data_contract(43-71)data_contract(73-119)data_contract(121-177)packages/rs-dpp/src/data_contract/accessors/v1/mod.rs (1)
description(74-74)packages/rs-dpp/src/data_contract/associated_token/token_configuration/accessors/v0/mod.rs (1)
description(70-70)packages/rs-drive/src/drive/contract/update/update_description/v0/mod.rs (1)
None(70-70)packages/rs-drive/src/drive/contract/insert/insert_contract/v1/mod.rs (1)
None(103-103)packages/rs-drive/src/drive/contract/insert/add_new_keywords/v0/mod.rs (1)
None(66-66)packages/rs-dpp/src/data_contract/serialized_version/v1/mod.rs (1)
from(104-182)
⏰ Context from checks skipped due to timeout of 90000ms (19)
- GitHub Check: Rust packages (wallet-utils-contract) / Tests
- GitHub Check: Rust packages (drive-abci) / Detect immutable structure changes
- GitHub Check: Rust packages (drive-abci) / Check each feature
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Unused dependencies
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (wasm-dpp) / Linting
- GitHub Check: Rust packages (wasm-dpp) / Formatting
- GitHub Check: Rust packages (drive) / Formatting
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (dpns-contract) / Linting
- GitHub Check: Rust packages (dpns-contract) / Tests
- GitHub Check: Rust packages (dpns-contract) / Unused dependencies
- GitHub Check: Rust packages (dash-sdk) / Check each feature
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
🔇 Additional comments (5)
packages/rs-drive/src/drive/contract/insert/add_description/v0/mod.rs (5)
17-52: LGTM: Well-structured method to handle contract description creationThe implementation correctly orchestrates the creation of description documents and fee calculation. Good job on correctly passing parameters through to the operations building function.
54-92: Implementation follows the established pattern for Drive operationsThis method appropriately handles optional cost estimation based on the
applyflag, following the same pattern seen in other Drive operation methods like insert_contract and add_new_keywords.
96-172: The operations building logic is well-implementedGood implementation that correctly handles both short and full description document creation based on the
short_onlyflag. The proper document types are retrieved from the keyword search contract and the operations are built and collected appropriately.
201-206: Correctly setting revision for both document typesSetting
revision: Some(1)is appropriate for both document types, ensuring that the mutablefullDescriptiondocument starts with the correct revision. This prevents validation failures on creation and enables subsequent updates.
1-16: Imports are well organizedAll necessary imports are included and properly organized.
Issue being fixed or feature implemented
Keyword search system contract for contract keywords.
What was done?
keywordsfield to the top level of data contracts where users can put a vector of up to 20 unique keywords associated with the contract. When the contract is registered, a document is created in the Keyword Search system contract'scontractKeywordsdocument type with the keyword and the contract ID. Contract updates can also edit thecontractKeywords.descriptionfield to the top level of data contracts where users can put a string of up to 100 characters meant for describing the contract. When the contract is registered, a document is created in the Keyword Search system contract'sshortDescriptionandfullDescriptionwith the description and the contract ID. ThefullDescriptiondocuments are mutable and can actually hold 10,000 characters, but they cannot be deleted, and creation is not allowed. TheshortDescriptiondocuments are immutable, cannot be deleted, and creation is not allowed. Contract updates can also edit theshortDescription.descriptionfield to the top level of token configurations.How Has This Been Tested?
Added tests plus tested with DET on local network.
Breaking Changes
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation
Chores