feat(drive-abci): improve token name localization validation #2593
Conversation
|
""" WalkthroughThis update introduces stricter validation for token and keyword fields in data contracts, including new error types for invalid characters, lengths, and decimal limits. The maximum allowed keywords increases from 20 to 50. The decimals field type changes from Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DataContract
participant Validator
participant ErrorTypes
User->>DataContract: Submit contract with tokens/keywords
DataContract->>Validator: validate_localizations_v0 / validate_update_v0
Validator->>ErrorTypes: Check for invalid characters, length, decimals, language code
alt Validation fails
ErrorTypes-->>Validator: Return specific error
Validator-->>DataContract: Return error
DataContract-->>User: Error response
else Validation passes
Validator-->>DataContract: Success
DataContract-->>User: Success response
end
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (20)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
QuantumExplorer
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
packages/rs-dpp/src/data_contract/associated_token/token_configuration_convention/methods/validate_localizations/v0/mod.rs (2)
31-36: Hard-code replaced with magic number – promote to a named constant
16is repeated here as the upper limit for decimals. Surfacing this number as aconst MAX_DECIMALS: u8 = 16;at module scope (or re-using an existing constant if one already exists) will:
- avoid “magic number” drift when specs change,
- make unit tests clearer, and
- give the compiler a single source of truth.
+const MAX_DECIMALS: u8 = 16; ... -// Max decimals is defined as 16 -if self.decimals() > 16 { +// Upper-bound decimals +if self.decimals() > MAX_DECIMALS { return SimpleConsensusValidationResult::new_with_error( - DecimalsOverLimitError::new(self.decimals(), 16).into(), + DecimalsOverLimitError::new(self.decimals(), MAX_DECIMALS).into(), ); }
38-68: Minor duplication – consider extracting a helper for character validationThe three blocks that validate control/whitespace characters for
singular_form,plural_form, andlanguageare identical apart from the labels. A small helper would remove duplication and lower the cognitive load for future changes.packages/rs-dpp/src/data_contract/associated_token/token_configuration_convention/accessors/mod.rs (1)
46-61: Setter implementation forwards only V0 – remember to extend when new versions appearThe match arms delegate correctly for V0, but future enum variants will silently ignore setters unless this block is updated. A
#[allow(unreachable_patterns)]+_ => unreachable!()arm (ortodo!()placeholder) can safeguard against accidental omissions.packages/rs-dpp/src/errors/consensus/basic/data_contract/invalid_token_name_length_error.rs (1)
1-56: Well-structured error type with minor style inconsistency.The error type provides clear information about length validation failures with appropriate fields and a descriptive error message. The implementation follows the standard pattern with one minor style inconsistency.
Line 54 uses
ConsensusError::BasicErrorwhile other similar implementations useSelf::BasicError. For consistency, consider:- ConsensusError::BasicError(BasicError::InvalidTokenNameLengthError(err)) + Self::BasicError(BasicError::InvalidTokenNameLengthError(err))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
packages/rs-dpp/src/data_contract/associated_token/token_configuration_convention/accessors/mod.rs(2 hunks)packages/rs-dpp/src/data_contract/associated_token/token_configuration_convention/accessors/v0/mod.rs(2 hunks)packages/rs-dpp/src/data_contract/associated_token/token_configuration_convention/methods/validate_localizations/v0/mod.rs(2 hunks)packages/rs-dpp/src/data_contract/associated_token/token_configuration_convention/v0/mod.rs(3 hunks)packages/rs-dpp/src/data_contract/methods/validate_update/v0/mod.rs(3 hunks)packages/rs-dpp/src/data_contract/v1/serialization/mod.rs(1 hunks)packages/rs-dpp/src/errors/consensus/basic/basic_error.rs(2 hunks)packages/rs-dpp/src/errors/consensus/basic/data_contract/invalid_keyword_character_error.rs(1 hunks)packages/rs-dpp/src/errors/consensus/basic/data_contract/invalid_token_language_code_error.rs(1 hunks)packages/rs-dpp/src/errors/consensus/basic/data_contract/invalid_token_name_character_error.rs(1 hunks)packages/rs-dpp/src/errors/consensus/basic/data_contract/invalid_token_name_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(2 hunks)packages/rs-dpp/src/errors/consensus/basic/data_contract/token_decimals_over_limit_error.rs(1 hunks)packages/rs-dpp/src/errors/consensus/codes.rs(1 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_create/basic_structure/v0/mod.rs(3 hunks)packages/wasm-dpp/src/errors/consensus/consensus_error.rs(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_create/basic_structure/v0/mod.rs (2)
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-dpp/src/data_contract/methods/validate_update/v0/mod.rs (2)
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 (ensuring it's between 3 and 100 characters) is already handled at the data contract validation level in packages/rs-dpp/src/data_contract/methods/validate_update/v0/mod.rs, making additional checks in the update operations redundant.
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.
🧬 Code Graph Analysis (8)
packages/rs-dpp/src/data_contract/associated_token/token_configuration_convention/accessors/v0/mod.rs (3)
packages/rs-dpp/src/data_contract/associated_token/token_configuration_convention/v0/mod.rs (2)
decimals(87-89)set_decimals(100-102)packages/rs-dpp/src/data_contract/associated_token/token_configuration_convention/accessors/mod.rs (2)
decimals(39-43)set_decimals(56-60)packages/rs-dpp/src/errors/consensus/basic/data_contract/token_decimals_over_limit_error.rs (1)
decimals(31-33)
packages/rs-dpp/src/data_contract/associated_token/token_configuration_convention/methods/validate_localizations/v0/mod.rs (4)
packages/rs-dpp/src/errors/consensus/basic/data_contract/invalid_token_language_code_error.rs (1)
new(21-23)packages/rs-dpp/src/errors/consensus/basic/data_contract/invalid_token_name_length_error.rs (1)
new(26-33)packages/rs-dpp/src/errors/consensus/basic/data_contract/invalid_token_name_character_error.rs (1)
new(24-26)packages/rs-dpp/src/errors/consensus/basic/data_contract/token_decimals_over_limit_error.rs (1)
new(24-29)
packages/rs-dpp/src/errors/consensus/basic/data_contract/token_decimals_over_limit_error.rs (6)
packages/rs-dpp/src/errors/consensus/basic/data_contract/invalid_keyword_character_error.rs (2)
new(25-30)from(42-44)packages/rs-dpp/src/errors/consensus/basic/data_contract/invalid_token_language_code_error.rs (2)
new(21-23)from(31-33)packages/rs-dpp/src/errors/consensus/basic/data_contract/invalid_token_name_length_error.rs (2)
new(26-33)from(53-55)packages/rs-dpp/src/errors/consensus/basic/data_contract/invalid_token_name_character_error.rs (2)
new(24-26)from(38-40)packages/rs-dpp/src/errors/consensus/basic/data_contract/keywords_over_limit.rs (2)
new(27-32)from(44-46)packages/rs-dpp/src/errors/consensus/basic/basic_error.rs (1)
from(560-562)
packages/rs-dpp/src/errors/consensus/basic/data_contract/invalid_token_language_code_error.rs (6)
packages/rs-dpp/src/errors/consensus/basic/data_contract/invalid_keyword_character_error.rs (2)
new(25-30)from(42-44)packages/rs-dpp/src/errors/consensus/basic/data_contract/invalid_token_name_length_error.rs (2)
new(26-33)from(53-55)packages/rs-dpp/src/errors/consensus/basic/data_contract/invalid_token_name_character_error.rs (2)
new(24-26)from(38-40)packages/rs-dpp/src/errors/consensus/basic/data_contract/keywords_over_limit.rs (2)
new(27-32)from(44-46)packages/rs-dpp/src/errors/consensus/basic/data_contract/token_decimals_over_limit_error.rs (2)
new(24-29)from(41-43)packages/rs-dpp/src/errors/consensus/basic/basic_error.rs (1)
from(560-562)
packages/rs-dpp/src/data_contract/methods/validate_update/v0/mod.rs (2)
packages/rs-dpp/src/errors/consensus/basic/data_contract/invalid_keyword_character_error.rs (2)
keyword(36-38)new(25-30)packages/rs-dpp/src/errors/consensus/basic/data_contract/keywords_over_limit.rs (1)
new(27-32)
packages/rs-dpp/src/errors/consensus/basic/data_contract/invalid_keyword_character_error.rs (4)
packages/rs-dpp/src/errors/consensus/basic/data_contract/invalid_token_language_code_error.rs (2)
new(21-23)from(31-33)packages/rs-dpp/src/errors/consensus/basic/data_contract/invalid_token_name_character_error.rs (2)
new(24-26)from(38-40)packages/rs-dpp/src/errors/consensus/basic/data_contract/keywords_over_limit.rs (3)
new(27-32)data_contract_id(34-36)from(44-46)packages/rs-dpp/src/errors/consensus/basic/basic_error.rs (1)
from(560-562)
packages/rs-dpp/src/errors/consensus/basic/data_contract/invalid_token_name_character_error.rs (3)
packages/rs-dpp/src/errors/consensus/basic/data_contract/invalid_token_language_code_error.rs (2)
new(21-23)from(31-33)packages/rs-dpp/src/errors/consensus/basic/data_contract/invalid_token_name_length_error.rs (3)
new(26-33)form(47-49)from(53-55)packages/rs-dpp/src/errors/consensus/basic/basic_error.rs (1)
from(560-562)
packages/rs-dpp/src/data_contract/associated_token/token_configuration_convention/v0/mod.rs (3)
packages/rs-dpp/src/data_contract/associated_token/token_configuration_convention/accessors/v0/mod.rs (4)
decimals(17-17)set_localizations(23-26)localizations(11-11)set_decimals(29-29)packages/rs-dpp/src/data_contract/associated_token/token_configuration_convention/accessors/mod.rs (4)
decimals(39-43)set_localizations(47-54)localizations(27-31)set_decimals(56-60)packages/rs-dpp/src/errors/consensus/basic/data_contract/token_decimals_over_limit_error.rs (1)
decimals(31-33)
⏰ Context from checks skipped due to timeout of 90000ms (17)
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (dash-sdk) / Formatting
- GitHub Check: Rust packages (drive-abci) / Linting
- GitHub Check: Rust packages (dash-sdk) / Check each feature
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive-abci) / Check each feature
- GitHub Check: Rust packages (drive-abci) / Unused dependencies
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / 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 (28)
packages/rs-dpp/src/errors/consensus/basic/data_contract/keywords_over_limit.rs (1)
13-13: Updated error message to reflect new keyword limitThe error message has been updated to indicate that the maximum number of keywords allowed is now 50 (previously 20), which aligns with the PR objective to improve error handling and validation for data contracts.
packages/rs-dpp/src/data_contract/v1/serialization/mod.rs (1)
176-179: Improved keyword normalization during deserializationThis change ensures all keywords are converted to lowercase during deserialization, which is a good practice for maintaining consistency. This prevents potential issues with case-sensitive comparisons and improves the reliability of keyword matching.
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_create/basic_structure/v0/mod.rs (3)
4-4: LGTM: Added import for new error typeThe import for
InvalidKeywordCharacterErrorhas been correctly added to support the new validation checks.
120-121: Updated keyword limit from 20 to 50The comment and validation logic have been updated to reflect the new maximum limit of 50 keywords, which aligns with the changes in the error message from the first file. This is consistent with the PR objectives to improve data contract validation.
147-160: Added validation for invalid keyword charactersNew validation logic has been added to ensure keywords don't contain control or whitespace characters, improving the robustness of the data contract validation process. This check is properly positioned between length validation and uniqueness validation, forming a logical sequence of validation steps.
packages/rs-dpp/src/data_contract/associated_token/token_configuration_convention/accessors/v0/mod.rs (2)
17-17: Changed decimals type from u16 to u8The return type of
decimals()method has been changed fromu16tou8, which aligns with the broader changes in the PR to refine decimal validation. Since decimal places in tokens typically don't exceed 18 (and certainly not 255), this change is appropriate and more memory-efficient.
29-29: Updated decimals setter parameter type to match getterThe parameter type of
set_decimals()method has been changed fromu16tou8to maintain consistency with the getter method. This change is correctly synchronized with the corresponding implementation changes in the actual struct, ensuring type consistency throughout the codebase.packages/rs-dpp/src/data_contract/methods/validate_update/v0/mod.rs (3)
10-11: Updated imports reflect new keyword validation functionality.Imports were updated to include
InvalidKeywordCharacterErrorfor the new keyword character validation logic.
271-272: Keyword limit increased from 20 to 50.The maximum allowed number of keywords has been increased from 20 to 50, providing more flexibility for data contract metadata.
288-300: Added keyword character validation.Added new validation to ensure keywords don't contain control characters or whitespace. This check occurs before the uniqueness validation, providing a better validation sequence.
packages/wasm-dpp/src/errors/consensus/consensus_error.rs (2)
64-64: Extended import list for error types.The import list has been expanded to include new token and keyword validation error types that are now being handled in the conversion functions.
795-809: Added handlers for new error types.Added conversion logic for the new error types to ensure proper JavaScript value conversion in the WebAssembly bindings:
InvalidKeywordCharacterErrorInvalidTokenNameCharacterErrorDecimalsOverLimitErrorInvalidTokenNameLengthErrorInvalidTokenLanguageCodeErrorThis ensures that the WebAssembly interface correctly exposes these validation errors.
packages/rs-dpp/src/errors/consensus/basic/data_contract/mod.rs (2)
31-31: Added new error module declarations.Added module declarations for new error types related to token and keyword validation:
invalid_keyword_character_errorinvalid_token_language_code_errorinvalid_token_name_character_errorinvalid_token_name_length_errortoken_decimals_over_limit_errorAlso applies to: 38-40, 46-46
92-92: Added public exports for new error types.Added corresponding public exports for the new error modules, making them available for use in other parts of the codebase. This maintains the module's pattern of exporting all error types.
Also applies to: 99-101, 106-106
packages/rs-dpp/src/errors/consensus/codes.rs (1)
111-118: Added error codes for new validation error types.Updated error codes and added new ones for the newly introduced validation errors:
InvalidDescriptionLengthError-> 10264NewTokensDestinationIdentityOptionRequiredError-> 10265InvalidTokenNameCharacterError-> 10266InvalidTokenNameLengthError-> 10267InvalidTokenLanguageCodeError-> 10268InvalidKeywordCharacterError-> 10269InvalidKeywordLengthError-> 10270DecimalsOverLimitError-> 10271These codes ensure that each error type has a unique identifier in the consensus system.
packages/rs-dpp/src/errors/consensus/basic/data_contract/invalid_token_language_code_error.rs (1)
30-34: Looks good – error is correctly wired intoConsensusErrorConversion into
ConsensusErrorfollows the existing pattern and will propagate cleanly through the consensus layer.packages/rs-dpp/src/data_contract/associated_token/token_configuration_convention/accessors/mod.rs (1)
39-43: Return type update is consistent with type change elsewhereChanging
decimals()to returnu8aligns with the underlying struct’s refactor. No functional issues spotted.packages/rs-dpp/src/errors/consensus/basic/data_contract/token_decimals_over_limit_error.rs (1)
1-44: Well-structured error type for decimal limit validation.The implementation follows the standard error pattern in the codebase with all necessary traits, clear error messaging, and proper conversion to the general consensus error type. The use of
u8for decimal limits is appropriate since decimals are typically small values and this type provides sufficient range while being memory efficient.packages/rs-dpp/src/errors/consensus/basic/data_contract/invalid_keyword_character_error.rs (1)
1-45: Well-implemented error type for keyword character validation.The error type correctly captures both the contract ID and the problematic keyword, providing clear context in the error message about the validation requirements. The implementation follows the standard error pattern in the codebase with appropriate traits and conversion methods.
packages/rs-dpp/src/errors/consensus/basic/data_contract/invalid_token_name_character_error.rs (1)
1-41: Properly implemented error type for token name character validation.The error correctly tracks both the form and token name with invalid characters, providing clear context in the error message. The implementation follows the established error pattern in the codebase with all necessary traits and conversion methods.
packages/rs-dpp/src/errors/consensus/basic/basic_error.rs (3)
11-17: Appropriate expansion of error imports.The imports are correctly updated to include the new error types for token and keyword validation.
19-25: Good organization of related imports.The new error types are properly grouped with related token validation imports, maintaining code organization and readability.
543-556: Well-structured error variant additions.The new error variants for token and keyword validation are properly implemented:
- Each variant uses
#[error(transparent)]for correct error propagation- Naming follows established conventions in the codebase
- Variants are added at the end of the enum, preserving compatibility
These additions align with the PR objective of improving error handling for decimal validation errors.
packages/rs-dpp/src/data_contract/associated_token/token_configuration_convention/v0/mod.rs (5)
1-3: Import updated to include new setter trait.The import statement is correctly expanded to include both getter and setter traits.
39-39: Type refinement: u16 to u8 for decimals field.The decimals field type change from
u16tou8is appropriate since token decimal places are typically small values. This change aligns with the PR objective to improve decimal validation and is consistent with the relevant code changes in other files.
43-43: Default function return type updated.The return type for
default_decimals()is correctly updated to match the field type change.
87-87: Getter method return type updated.The
decimals()method return type is correctly changed tou8to maintain consistency with the field type change.
92-103: Good addition of setter methods.The implementation of
TokenConfigurationConventionV0Setterstrait with methods for modifying localizations and decimals is well-structured and follows established patterns in the codebase. This addition improves the API without breaking existing functionality.
packages/rs-dpp/src/data_contract/methods/validate_update/v0/mod.rs
Outdated
Show resolved
Hide resolved
...act/associated_token/token_configuration_convention/methods/validate_localizations/v0/mod.rs
Outdated
Show resolved
Hide resolved
QuantumExplorer
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/rs-dpp/src/data_contract/associated_token/token_configuration_convention/methods/validate_localizations/v0/mod.rs (1)
97-101: Fixed parameter in error constructorThe correct parameter
language.len()is now used in theInvalidTokenNameLengthErrorconstructor, which addresses a previously identified issue. This ensures that error messages correctly reflect the actual length of the language code.
🧹 Nitpick comments (1)
packages/rs-dpp/src/data_contract/associated_token/token_configuration_convention/methods/validate_localizations/v0/mod.rs (1)
38-108: Consider refactoring duplicate validation logicThe validation logic for singular and plural forms contains nearly identical code blocks. Consider extracting this into a helper method to reduce duplication.
impl TokenConfigurationConvention { #[inline(always)] pub(super) fn validate_localizations_v0(&self) -> SimpleConsensusValidationResult { // ... existing code ... + + fn validate_token_name(name: &str, form_type: &str) -> Option<Box<dyn anyhow::Error>> { + if !name.chars().all(|c| !c.is_control() && !c.is_whitespace()) { + return Some( + InvalidTokenNameCharacterError::new( + form_type.to_string(), + name.to_string(), + ) + .into(), + ); + } + + if name.len() < 3 || name.len() > 25 { + return Some( + InvalidTokenNameLengthError::new(name.len(), 3, 25, form_type) + .into(), + ); + } + + None + } for (language, localization) in self.localizations() { let singular_form = localization.singular_form(); let plural_form = localization.plural_form(); - if !singular_form - .chars() - .all(|c| !c.is_control() && !c.is_whitespace()) - { - // This would mean we have an invalid character - return SimpleConsensusValidationResult::new_with_error( - InvalidTokenNameCharacterError::new( - "singular form".to_string(), - singular_form.to_string(), - ) - .into(), - ); - } - - if !plural_form - .chars() - .all(|c| !c.is_control() && !c.is_whitespace()) - { - // This would mean we have an invalid character - return SimpleConsensusValidationResult::new_with_error( - InvalidTokenNameCharacterError::new( - "plural form".to_string(), - plural_form.to_string(), - ) - .into(), - ); - } + if let Some(err) = validate_token_name(singular_form, "singular form") { + return SimpleConsensusValidationResult::new_with_error(err); + } + + if let Some(err) = validate_token_name(plural_form, "plural form") { + return SimpleConsensusValidationResult::new_with_error(err); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/rs-dpp/src/data_contract/associated_token/token_configuration_convention/methods/validate_localizations/v0/mod.rs(2 hunks)packages/rs-dpp/src/data_contract/methods/validate_update/v0/mod.rs(3 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_create/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/src/drive/votes/resolved/vote_polls/contested_document_resource_vote_poll/resolve.rs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/rs-drive/src/drive/votes/resolved/vote_polls/contested_document_resource_vote_poll/resolve.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/rs-dpp/src/data_contract/methods/validate_update/v0/mod.rs
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: QuantumExplorer
PR: dashpay/platform#2257
File: packages/rs-drive-abci/src/mimic/test_quorum.rs:159-164
Timestamp: 2024-11-20T16:16:01.830Z
Learning: QuantumExplorer prefers not to receive auto-generated messages asking to post on social media.
🧬 Code Graph Analysis (1)
packages/rs-dpp/src/data_contract/associated_token/token_configuration_convention/methods/validate_localizations/v0/mod.rs (4)
packages/rs-dpp/src/errors/consensus/basic/data_contract/invalid_token_language_code_error.rs (1)
new(21-23)packages/rs-dpp/src/errors/consensus/basic/data_contract/invalid_token_name_character_error.rs (1)
new(24-26)packages/rs-dpp/src/errors/consensus/basic/data_contract/invalid_token_name_length_error.rs (1)
new(26-33)packages/rs-dpp/src/errors/consensus/basic/data_contract/token_decimals_over_limit_error.rs (1)
new(24-29)
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: Rust packages (drive-abci) / Check each feature
- GitHub Check: Rust packages (dash-sdk) / Detect immutable structure changes
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (dash-sdk) / Formatting
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (dash-sdk) / Unused dependencies
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (drive) / Formatting
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (wasm-dpp) / Unused dependencies
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Linting
- 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 Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: Rust crates security audit
🔇 Additional comments (11)
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_create/mod.rs (2)
3349-3349: LGTM: Updated test reflects new keyword limit of 50This test function name clearly indicates it's testing the new 50 keyword limit as mentioned in the PR.
3377-3377: Appropriate test case for validating the new limitTest correctly creates 51 keywords to verify that the system rejects contracts exceeding the 50 keyword limit (previously 20).
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_update/mod.rs (5)
2499-2501: Change correctly implements increased keyword limit from 20 to 50.The test case has been updated to test the new maximum keyword limit of 50 by adding keywords "kw21" through "kw50". This aligns with the PR's changes to increase the maximum allowed keywords in data contracts.
2572-2572: Updated test case to expect lowercase keywords.This change correctly accounts for the new behavior where keywords are converted to lowercase during deserialization, ensuring tests align with the implementation.
2581-2583: Updated assertions to match lowercase keyword expectations.These assertion changes correctly verify that the lowercase versions of keywords ("newa", "newb", "newc") are stored in the database, consistent with the lowercase conversion during deserialization.
1213-1220: All imports look good for token configuration validation.The imports related to token configuration and validation are properly structured to support the enhanced token name localization validation described in the PR objectives.
1731-1824: Test case properly validates token localization requirements.This test correctly verifies that tokens must have valid localizations, which aligns with the PR objective of enhancing validation for token localizations. The test specifically checks that empty localization maps are rejected.
packages/rs-dpp/src/data_contract/associated_token/token_configuration_convention/methods/validate_localizations/v0/mod.rs (4)
1-4: Good error import organizationThe imports are well-structured, clearly identifying the specific error types needed for token validation.
10-15: Well-implemented BCP 47 regex patternUsing
once_cell::Lazyensures the regex is compiled only once, which is efficient. The pattern correctly implements the BCP 47 language tag format with appropriate segments for language, script, region, variant, extension, and private use.
31-36: Good implementation of decimals limit validationThe 16 decimal limit is properly enforced with an appropriate error message containing both the actual and maximum allowed values.
103-107: Comprehensive language code validationThe implementation properly validates language codes against the BCP 47 standard using the pre-compiled regex pattern. The error message includes the invalid language code for easier debugging.
…tform into fixdecimal-validation-errors
|
Self Reviewed |
|
This is technically a breaking change due to tightening some constrains (e.g., token name length), correct? |
Issue being fixed or feature implemented
This PR improves validation of token localizations within token configuration conventions. It ensures that localization entries adhere to expected constraints on structure, character validity, and length.
What was done?
Introduced new consensus validation errors to ensure correctness and clarity:
• InvalidKeywordCharacterError
• InvalidTokenNameCharacterError
• InvalidTokenNameLengthError
• InvalidTokenLanguageCodeError
• DecimalsOverLimitError
Updated TokenConfigurationConvention::validate_localizations_v0 to enforce the following validation rules:
• Token names (singular and plural) must not contain whitespace or control characters.
• Token name lengths must be between 3 and 25 characters.
• Language codes must:
• Match the BCP 47-compatible regex format.
• Have a length between 2 and 12 characters.
• At least one localization must be provided in English.
The decimals value must not exceed 16.
Registered all new errors in the ConsensusError conversion match block via generic_consensus_error!.
How Has This Been Tested?
Not yet tested, will do so in DET
Breaking Changes
None
Checklist
For repository code-owners and collaborators only
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Improvements
Bug Fixes