fix(platform)!: ensure correct critical security level for token transitions and allow any security level key in signing if allowed to do so with options#2597
Conversation
|
Warning Rate limit exceeded@QuantumExplorer has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 11 minutes and 10 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (5)
WalkthroughThe changes refactor multiple document and token batch transition creation methods to replace separate optional feature version parameters with a single Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant BatchTransition
participant Document
participant Token
Caller->>BatchTransition: combined_security_level_requirement(get_data_contract_security_level_requirement)
alt Document variant
BatchTransition->>Document: get document type and contract id
Document->>BatchTransition: security level requirement
BatchTransition->>BatchTransition: update highest security level if needed
else Token variant
BatchTransition->>BatchTransition: set highest security level to CRITICAL unless MASTER
end
BatchTransition->>Caller: return vector of security levels from CRITICAL to highest or MASTER
sequenceDiagram
participant Caller
participant StateTransition
participant Signer
Caller->>StateTransition: sign_external_with_options(identity_public_key, signer, get_data_contract_security_level_requirement, options)
StateTransition->>StateTransition: verify_public_key_level_and_purpose(identity_public_key, options)
alt Batch variant
StateTransition->>StateTransition: if options.allow_signing_with_any_purpose skip purpose check
StateTransition->>StateTransition: if options.allow_signing_with_any_security_level skip security level check
end
StateTransition->>Signer: sign transition
Signer->>StateTransition: signature
StateTransition->>Caller: return result
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ 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
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: 4
🔭 Outside diff range comments (2)
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v1_methods.rs (1)
29-41:⚠️ Potential issueImport will be unused when the
state-transition-signingfeature is OFF
StateTransitionCreationOptionsis only referenced inside#[cfg(feature = "state-transition-signing')]fns, but the import itself is always compiled.
This triggers anunused importlint and will failcargo deny/CI in non-signing builds.-use crate::state_transition::batch_transition::methods::StateTransitionCreationOptions; +#[cfg(feature = "state-transition-signing")] +use crate::state_transition::batch_transition::methods::StateTransitionCreationOptions;🧰 Tools
🪛 GitHub Check: Rust packages (drive) / Linting
[warning] 40-40: unused import:
crate::state_transition::batch_transition::methods::StateTransitionCreationOptions
warning: unused import:crate::state_transition::batch_transition::methods::StateTransitionCreationOptions
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v1_methods.rs:40:5
|
40 | use crate::state_transition::batch_transition::methods::StateTransitionCreationOptions;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/mod.rs (1)
42-45:⚠️ Potential issue
FeatureVersionimport is hidden behind a feature gate -> compile error
StateTransitionCreationOptions(lines 49-56) is always compiled, but it referencesFeatureVersion, which is only imported when thestate-transition-signingfeature is enabled.
This causesE0412when the feature is disabled.-#[cfg(feature = "state-transition-signing")] -use platform_version::version::{FeatureVersion, PlatformVersion}; +use platform_version::version::{FeatureVersion, PlatformVersion};Do the same for
platform_value::Identifierif it’s also needed outside the feature flag.
♻️ Duplicate comments (1)
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v1_methods.rs (1)
139-152: SameSecurityLevel::HIGHissue repeatedThe same incorrect security level appears in the other
new_token_*_transitionhelpers listed in this range; fix as per previous comment to avoid inconsistent security requirements.Also applies to: 217-230, 280-293, 359-371, 435-448, 514-527, 591-604, 668-681, 722-734, 804-816, 858-870
🧹 Nitpick comments (5)
packages/rs-dpp/src/state_transition/traits/state_transition_identity_signed.rs (1)
45-62: Consider borrowingoptionsand document the new bypass behaviour
verify_public_key_level_and_purposenow trusts callers to passStateTransitionSigningOptions.
Two small follow-ups will make the change safer and clearer:
optionscan be taken by reference (&StateTransitionSigningOptions) – the struct is tiny, but borrowing expresses intent and avoids a copy.- Update the doc-comment to mention that checks can be skipped when the respective
allow_*flags aretrue, to prevent accidental misuse by future callers.No code diff included because both tweaks touch several lines; let me know if you’d like a ready-made patch.
packages/rs-drive/src/state_transition_action/batch/mod.rs (1)
121-126: Doc example still references the removed method nameThe code example uses
contract_based_security_level_requirement, which no longer exists.-/// let required_levels = batch_transition_action.contract_based_security_level_requirement()?; +/// let required_levels = batch_transition_action.combined_security_level_requirement()?;packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/v0/mod.rs (1)
119-126: Doc example refers to the obsolete method nameSame issue as in the Drive crate – the documentation block still shows
contract_based_security_level_requirement.-/// let required_levels = batch_transition_action.contract_based_security_level_requirement()?; +/// let required_levels = batch_transition_action.combined_security_level_requirement()?;packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v1_methods.rs (1)
139-152: Heavy code duplication while signingEach helper copies the same
if let Some(options)/elsesigning block.
Consider extracting:fn sign_batch_transition<S: Signer>( st: &mut StateTransition, identity_pk: &IdentityPublicKey, signer: &S, options: Option<StateTransitionCreationOptions>, ) -> Result<(), ProtocolError> { … }This will cut ~140 lines, ease future maintenance and avoid mistakes like the security-level mismatch above.
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/mod.rs (1)
99-136:resolved_optionscomputed but not forwardedYou create
let resolved_options = options.unwrap_or_default();for version dispatch, but you still pass the originaloptionsdown toBatchTransitionV*::new_*helpers.If the caller supplied
Noneyou just re-did the defaulting work, but the callee will redo it again.
ForwardingSome(resolved_options)would avoid the redundancy and keep behaviour symmetrical ifoptionsbecomesSomelater in the method.Not critical, yet worth a cleanup.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (38)
packages/rs-dpp/src/state_transition/mod.rs(5 hunks)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/mod.rs(42 hunks)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/v0/mod.rs(8 hunks)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/v1/mod.rs(12 hunks)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v0/v0_methods.rs(14 hunks)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v0_methods.rs(13 hunks)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v1_methods.rs(24 hunks)packages/rs-dpp/src/state_transition/traits/state_transition_identity_signed.rs(2 hunks)packages/rs-drive-abci/src/execution/check_tx/v0/mod.rs(0 hunks)packages/rs-drive-abci/src/execution/platform_events/block_processing_end_events/tests.rs(0 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/advanced_structure/v0/mod.rs(1 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/creation.rs(0 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/deletion.rs(0 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/dpns.rs(0 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/nft.rs(0 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/replacement.rs(0 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/transfer.rs(0 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/direct_selling/mod.rs(0 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/distribution/perpetual/block_based.rs(0 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/distribution/perpetual/time_based.rs(0 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/distribution/pre_programmed.rs(0 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/mod.rs(0 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/mod.rs(0 hunks)packages/rs-drive/src/state_transition_action/batch/mod.rs(2 hunks)packages/rs-sdk/src/platform/transition/fungible_tokens/burn.rs(3 hunks)packages/rs-sdk/src/platform/transition/fungible_tokens/claim.rs(3 hunks)packages/rs-sdk/src/platform/transition/fungible_tokens/config_update.rs(3 hunks)packages/rs-sdk/src/platform/transition/fungible_tokens/destroy.rs(3 hunks)packages/rs-sdk/src/platform/transition/fungible_tokens/emergency_action.rs(3 hunks)packages/rs-sdk/src/platform/transition/fungible_tokens/freeze.rs(3 hunks)packages/rs-sdk/src/platform/transition/fungible_tokens/mint.rs(3 hunks)packages/rs-sdk/src/platform/transition/fungible_tokens/transfer.rs(3 hunks)packages/rs-sdk/src/platform/transition/fungible_tokens/unfreeze.rs(3 hunks)packages/rs-sdk/src/platform/transition/purchase_document.rs(1 hunks)packages/rs-sdk/src/platform/transition/put_document.rs(1 hunks)packages/rs-sdk/src/platform/transition/put_settings.rs(2 hunks)packages/rs-sdk/src/platform/transition/transfer_document.rs(1 hunks)packages/rs-sdk/src/platform/transition/update_price_of_document.rs(1 hunks)
💤 Files with no reviewable changes (14)
- packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/deletion.rs
- packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/dpns.rs
- packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/distribution/perpetual/time_based.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/check_tx/v0/mod.rs
- packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/transfer.rs
- packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/replacement.rs
- packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/distribution/pre_programmed.rs
- packages/rs-drive-abci/src/execution/platform_events/block_processing_end_events/tests.rs
- packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/mod.rs
- packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/distribution/perpetual/block_based.rs
- packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/creation.rs
- packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/nft.rs
- packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/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 (3)
packages/rs-drive/src/state_transition_action/batch/mod.rs (1)
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/v0/mod.rs (1)
combined_security_level_requirement(119-165)
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/v0/mod.rs (1)
packages/rs-drive/src/state_transition_action/batch/mod.rs (1)
combined_security_level_requirement(126-168)
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v0/v0_methods.rs (2)
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/document_base_transition/v1/from_document.rs (1)
state_transition(9-22)packages/rs-dpp/src/state_transition/mod.rs (1)
owner_id(488-490)
🪛 GitHub Check: Rust packages (drive) / Linting
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v1_methods.rs
[warning] 40-40: unused import: crate::state_transition::batch_transition::methods::StateTransitionCreationOptions
warning: unused import: crate::state_transition::batch_transition::methods::StateTransitionCreationOptions
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v1_methods.rs:40:5
|
40 | use crate::state_transition::batch_transition::methods::StateTransitionCreationOptions;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/v0/mod.rs
[warning] 24-24: unused import: crate::state_transition::batch_transition::methods::StateTransitionCreationOptions
warning: unused import: crate::state_transition::batch_transition::methods::StateTransitionCreationOptions
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/v0/mod.rs:24:5
|
24 | use crate::state_transition::batch_transition::methods::StateTransitionCreationOptions;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: #[warn(unused_imports)] on by default
packages/rs-dpp/src/state_transition/traits/state_transition_identity_signed.rs
[warning] 19-19: unused import: StateTransitionSigningOptions
warning: unused import: StateTransitionSigningOptions
--> packages/rs-dpp/src/state_transition/traits/state_transition_identity_signed.rs:19:52
|
19 | use crate::state_transition::{StateTransitionLike, StateTransitionSigningOptions};
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/mod.rs
[failure] 55-55: cannot find type FeatureVersion in this scope
error[E0412]: cannot find type FeatureVersion in this scope
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/mod.rs:55:38
|
55 | pub base_feature_version: Option,
| ^^^^^^^^^^^^^^ not found in this scope
|
help: consider importing one of these type aliases
|
13 + use crate::version::FeatureVersion;
|
13 + use platform_version::version::FeatureVersion;
|
[failure] 54-54: cannot find type FeatureVersion in this scope
error[E0412]: cannot find type FeatureVersion in this scope
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/mod.rs:54:40
|
54 | pub method_feature_version: Option,
| ^^^^^^^^^^^^^^ not found in this scope
|
help: consider importing one of these type aliases
|
13 + use crate::version::FeatureVersion;
|
13 + use platform_version::version::FeatureVersion;
|
[failure] 53-53: cannot find type FeatureVersion in this scope
error[E0412]: cannot find type FeatureVersion in this scope
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/mod.rs:53:39
|
53 | pub batch_feature_version: Option,
| ^^^^^^^^^^^^^^ not found in this scope
|
help: consider importing one of these type aliases
|
13 + use crate::version::FeatureVersion;
|
13 + use platform_version::version::FeatureVersion;
|
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v0_methods.rs
[warning] 44-44: unused import: crate::state_transition::batch_transition::methods::StateTransitionCreationOptions
warning: unused import: crate::state_transition::batch_transition::methods::StateTransitionCreationOptions
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v0_methods.rs:44:5
|
44 | use crate::state_transition::batch_transition::methods::StateTransitionCreationOptions;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v0/v0_methods.rs
[warning] 40-40: unused import: crate::state_transition::batch_transition::methods::StateTransitionCreationOptions
warning: unused import: crate::state_transition::batch_transition::methods::StateTransitionCreationOptions
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v0/v0_methods.rs:40:5
|
40 | use crate::state_transition::batch_transition::methods::StateTransitionCreationOptions;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
⏰ Context from checks skipped due to timeout of 90000ms (12)
- GitHub Check: Rust packages (dash-sdk) / Unused dependencies
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (dash-sdk) / Check each feature
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (drive-abci) / Linting
- GitHub Check: Rust packages (dpp) / Check each feature
- 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
🔇 Additional comments (38)
packages/rs-sdk/src/platform/transition/transfer_document.rs (1)
71-82: Refactoring method calls to use the new options parameterThe code now uses
settings.state_transition_creation_optionsinstead of multipleNonearguments, which aligns with the broader refactoring to consolidate multiple optional feature version parameters into a single parameter. This makes the API cleaner and more maintainable.packages/rs-sdk/src/platform/transition/put_settings.rs (2)
3-4: Adding import for StateTransitionCreationOptionsThis import adds support for the new options parameter being used throughout the codebase.
12-14: Adding state_transition_creation_options field to PutSettingsThis addition is essential for the PR objective of ensuring correct security level for token transitions. The new field allows passing security configuration options through the SDK methods to the underlying state transition mechanisms.
packages/rs-sdk/src/platform/transition/put_document.rs (1)
63-74: Refactoring method calls to use the new options parameterSimilar to the changes in transfer_document.rs, this modification replaces multiple
Nonearguments with the new consolidatedsettings.state_transition_creation_options. This consistent pattern across the codebase improves both API clarity and maintainability.packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/advanced_structure/v0/mod.rs (2)
65-65: Critical security level enforcement via combined_security_level_requirementThis change is the core of the PR objective - renaming from
contract_based_security_level_requirementtocombined_security_level_requirementreflects the enhanced functionality to ensure token transitions require aCRITICALsecurity level. This is essential for maintaining proper security for token operations.The method now properly handles both document and token transition actions, ensuring appropriate security levels are enforced during validation.
69-93: Verification of security levels during validationThis existing code performs the actual security level verification. The updated
combined_security_level_requirement()call now ensures that token transitions will require the appropriate security level, and this validation code will enforce it by checking if the signing key's security level is included in the required levels.When the security level is inappropriate, it properly handles the error by creating a bump action and returning an
InvalidSignaturePublicKeySecurityLevelError.packages/rs-sdk/src/platform/transition/update_price_of_document.rs (1)
81-81: Good refactoring: simplified method signatureThis change properly passes the consolidated state transition options to the update price transition creator, replacing the previous multiple
Noneparameters. This aligns with the PR objective of ensuring correct security levels for transitions.packages/rs-sdk/src/platform/transition/purchase_document.rs (1)
85-85: Looking good: refactored parameter approachThis change correctly uses the consolidated state transition creation options approach, which will help enforce the critical security level requirements for token transitions.
packages/rs-sdk/src/platform/transition/fungible_tokens/mint.rs (2)
12-12: Properly added import for new options typeCorrectly imported the new
StateTransitionCreationOptionstype which is essential for handling security level requirements.
159-159: Critical security enhancement: properly enforces token transition securityThese changes are key to the PR objective - they ensure that token mint transitions can receive proper security level requirements (set to CRITICAL) through the options parameter. This addresses the security issue mentioned in the PR description.
Also applies to: 189-189
packages/rs-sdk/src/platform/transition/fungible_tokens/burn.rs (2)
12-12: Correct import for security optionsThe import for
StateTransitionCreationOptionshas been properly added to support the enhanced security requirements.
131-131: Good security enhancement: ensures critical security level for token burnsThese changes properly incorporate the security level requirements for token burn operations, supporting the PR's goal of enforcing CRITICAL security for token transitions.
Also applies to: 160-160
packages/rs-sdk/src/platform/transition/fungible_tokens/claim.rs (1)
11-11:Details
✅ Verification successful
LGTM: Parameter update for security level enforcement
These changes support the goal of ensuring token transitions have the correct security level by adding a
StateTransitionCreationOptionsparameter, which is correctly wired through to the inner token claim transition creation.Also applies to: 120-120, 148-148
🏁 Script executed:
#!/bin/bash # Search for implementation that enforces CRITICAL security level for token transitions # First look for combined_security_level_requirement which was mentioned in the PR summary rg "combined_security_level_requirement" --type rust -A 10 -B 3 # Then check for any code that specifically sets security level for token transitions rg "SecurityLevel::CRITICAL" --type rust -A 5 -B 5Length of output: 75183
LGTM: CRITICAL security level enforcement verified for token claim transitions
Verified that the new
StateTransitionCreationOptionsparameter is correctly passed through and that all token transitions enforceSecurityLevel::CRITICAL:
- In
packages/rs-drive/src/state_transition_action/batch/mod.rs,BatchedTransitionAction::TokenActionsetshighest_security_level = SecurityLevel::CRITICAL.- In
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/v0/mod.rs,BatchedTransitionRef::Token(_)similarly elevates toSecurityLevel::CRITICAL.No further changes required.
packages/rs-sdk/src/platform/transition/fungible_tokens/freeze.rs (1)
11-11: LGTM: Consistent parameter updateThe changes properly add support for
StateTransitionCreationOptionsto the token freeze transition, maintaining consistency with the pattern applied across other transition builders.Also applies to: 139-139, 168-168
packages/rs-sdk/src/platform/transition/fungible_tokens/transfer.rs (1)
11-11: LGTM: Parameter update for token transferThe token transfer builder now properly supports the
StateTransitionCreationOptionsparameter, ensuring it can specify the required security level for token transitions.Also applies to: 163-163, 194-194
packages/rs-sdk/src/platform/transition/fungible_tokens/config_update.rs (1)
12-12: LGTM: Parameter update maintains consistencyThe token config update builder correctly implements the parameter update pattern, maintaining consistency with other transition builders and supporting the PR's goal of enforcing correct security levels.
Also applies to: 125-125, 154-154
packages/rs-sdk/src/platform/transition/fungible_tokens/emergency_action.rs (3)
11-11: Added import for StateTransitionCreationOptions.Appropriate import added to support the new parameter in the
signmethod.
168-168: Parameter added to support token security level requirements.The addition of
options: Option<StateTransitionCreationOptions>aligns with the PR objective to ensure correct security level for token transitions.
197-197: Option parameter correctly passed to BatchTransition.The
optionsparameter is appropriately forwarded to thenew_token_emergency_action_transitionmethod, which ensures that security level requirements are properly enforced.packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/v1/mod.rs (2)
19-19: Added import for StateTransitionCreationOptions.Appropriate import added to support the new parameter in all methods of the trait.
78-78: Uniformly updated all token transition methods to include options parameter.All methods in the
DocumentsBatchTransitionMethodsV1trait now accept a consistentOption<StateTransitionCreationOptions>parameter, which consolidates version and signing options handling. This change supports the PR objective to ensure token transitions use the correct security level.Also applies to: 110-110, 146-146, 180-180, 212-212, 244-244, 279-279, 314-314, 347-347, 416-416
packages/rs-sdk/src/platform/transition/fungible_tokens/unfreeze.rs (3)
11-11: Added import for StateTransitionCreationOptions.Appropriate import added to support the new parameter in the
signmethod.
139-139: Parameter added to support token security level requirements.The addition of
options: Option<StateTransitionCreationOptions>aligns with the PR objective to ensure correct security level for unfreeze token transitions.
168-168: Option parameter correctly passed to BatchTransition.The
optionsparameter is appropriately forwarded to thenew_token_unfreeze_transitionmethod, which ensures that security level requirements are properly enforced.packages/rs-sdk/src/platform/transition/fungible_tokens/destroy.rs (3)
11-11: Added import for StateTransitionCreationOptions.Appropriate import added to support the new parameter in the
signmethod.
139-139: Parameter added to support token security level requirements.The addition of
options: Option<StateTransitionCreationOptions>aligns with the PR objective to ensure correct security level for token destroy transitions.
168-168: Option parameter correctly passed to BatchTransition.The
optionsparameter is appropriately forwarded to thenew_token_destroy_frozen_funds_transitionmethod, which ensures that security level requirements are properly enforced.packages/rs-drive/src/state_transition_action/batch/mod.rs (1)
152-156: Edge case: token + document mix where a document requiresHIGHCurrent logic upgrades
highest_security_levelto at leastCRITICALwhen a token transition is present, which is correct.
However, if any document requiresMASTER, the token branch is skipped (because the level is alreadyMASTER).
Please double-check that this behaviour matches the intended “token → ≥ CRITICAL unless already MASTER” rule and add a short comment clarifying it to future readers.packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/v0/mod.rs (1)
149-154: Minor: consolidate the comment on token transitionsThe inline comment duplicates the explanation in the Drive implementation.
Consider shortening to a single line, e.g.// Token transitions require ≥ CRITICAL.No code change strictly needed; mentioning for consistency only.
packages/rs-dpp/src/state_transition/mod.rs (1)
512-528: Nice use of delegationWrapping the original
sign_externalaroundsign_external_with_optionskeeps the public API unchanged while avoiding code duplication.
👍packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v0_methods.rs (4)
41-44: New import aligns with the code refactoring.The import of
StateTransitionCreationOptionsis necessary for the method signature changes. The static analysis warning appears to be a false positive, as this import is clearly used throughout the file.🧰 Tools
🪛 GitHub Check: Rust packages (drive) / Linting
[warning] 44-44: unused import:
crate::state_transition::batch_transition::methods::StateTransitionCreationOptions
warning: unused import:crate::state_transition::batch_transition::methods::StateTransitionCreationOptions
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v0_methods.rs:44:5
|
44 | use crate::state_transition::batch_transition::methods::StateTransitionCreationOptions;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
97-103: Refactoring simplifies method signatures and enhances signing options.The method signature change consolidates multiple feature version parameters into a single
StateTransitionCreationOptionsparameter, making the API cleaner. The unwrapping with default ensures backward compatibility. The change fromsign_externaltosign_external_with_optionsand the addition of signing options support the PR objective of ensuring proper security levels for transitions.Also applies to: 122-127
109-112: Using resolved option fields maintains functionality.Using
resolved_options.method_feature_versionandresolved_options.base_feature_versioncorrectly maintains the same functionality while making the code more maintainable through a unified options structure.
139-168: Consistent application of the refactoring pattern.The same pattern of using
resolved_optionsto unwrap the options parameter and applying its fields is consistently applied across all methods. This consistency makes the code more maintainable and reduces the possibility of bugs from inconsistent handling.Also applies to: 181-252, 270-295, 313-338
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v0/v0_methods.rs (4)
36-40: New import supports simplified method signatures.The import of
StateTransitionCreationOptionsis needed for the method signature changes in this file. The static analysis warning appears to be a false positive as this import is used throughout the implementation.🧰 Tools
🪛 GitHub Check: Rust packages (drive) / Linting
[warning] 40-40: unused import:
crate::state_transition::batch_transition::methods::StateTransitionCreationOptions
warning: unused import:crate::state_transition::batch_transition::methods::StateTransitionCreationOptions
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v0/v0_methods.rs:40:5
|
40 | use crate::state_transition::batch_transition::methods::StateTransitionCreationOptions;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
94-107: Refactoring aligns V0 implementation with V1.The changes mirror those in the V1 implementation, ensuring consistent interfaces across versions. This refactoring simplifies method signatures by using a structured options parameter and enhances signing flexibility with
sign_external_with_options.Also applies to: 117-122
136-163: Consistent implementation across all methods.The pattern of unwrapping options, using the resolved fields for feature versions, and passing signing options is consistently applied across all methods. This consistency promotes code maintainability and reduces the risk of subtle bugs.
Also applies to: 178-206, 220-247, 262-290, 306-333
337-345: Token transition filtering remains unchanged.This method correctly preserves the implementation detail where BatchTransitionV0 only handles document transitions (filtering out token transitions). This maintains backward compatibility while allowing the interface to evolve.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v1_methods.rs (2)
37-39: Gate optional imports with the same feature flag as their usages
StateTransitionCreationOptionsandGetDataContractSecurityLevelRequirementFnare used exclusively inside#[cfg(feature = "state-transition-signing")]functions.
When the crate is built without that feature, these imports trigger anunused importwarning (already reported by the CI linter).- use crate::state_transition::batch_transition::methods::StateTransitionCreationOptions; +#[cfg(feature = "state-transition-signing")] +use crate::state_transition::batch_transition::methods::StateTransitionCreationOptions; … - use crate::state_transition::GetDataContractSecurityLevelRequirementFn; +#[cfg(feature = "state-transition-signing")] +use crate::state_transition::GetDataContractSecurityLevelRequirementFn;Also applies to: 70-70
🧰 Tools
🪛 GitHub Check: Rust packages (drive-abci) / Linting
[warning] 38-38: unused import:
crate::state_transition::batch_transition::methods::StateTransitionCreationOptions
warning: unused import:crate::state_transition::batch_transition::methods::StateTransitionCreationOptions
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v1_methods.rs:38:5
|
38 | use crate::state_transition::batch_transition::methods::StateTransitionCreationOptions;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^🪛 GitHub Check: Rust packages (drive) / Linting
[warning] 38-38: unused import:
crate::state_transition::batch_transition::methods::StateTransitionCreationOptions
warning: unused import:crate::state_transition::batch_transition::methods::StateTransitionCreationOptions
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v1_methods.rs:38:5
|
38 | use crate::state_transition::batch_transition::methods::StateTransitionCreationOptions;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
138-151: Factor-out repeated signing boiler-plateThe
if let Some(options)… else …block that chooses between
sign_external_with_optionsandsign_externalis copied verbatim in every
new_token_*_transitionconstructor (10× in this file).
Duplicated logic is error-prone and inflates compile time.Consider extracting a small helper:
#[cfg(feature = "state-transition-signing")] fn sign_state_transition<S: Signer>( st: &mut StateTransition, identity_key: &IdentityPublicKey, signer: &S, options: Option<StateTransitionCreationOptions>, ) -> Result<(), ProtocolError> { match options { Some(opts) => st.sign_external_with_options( identity_key, signer, None::<GetDataContractSecurityLevelRequirementFn>, opts.signing_options, ), None => st.sign_external( identity_key, signer, None::<GetDataContractSecurityLevelRequirementFn>, ), } }and then call:
sign_state_transition(&mut state_transition, identity_public_key, signer, options)?;This keeps each constructor focused on building the transition, removes 150+ LOC,
and guarantees future fixes are applied in one place.packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/v0/mod.rs (1)
21-25: Same unused-import warning – apply the gating here as wellFor consistency with the v1 implementation and to silence the linter:
- use crate::state_transition::batch_transition::methods::StateTransitionCreationOptions; +#[cfg(feature = "state-transition-signing")] +use crate::state_transition::batch_transition::methods::StateTransitionCreationOptions;🧰 Tools
🪛 GitHub Check: Rust packages (drive-abci) / Linting
[warning] 24-24: unused import:
crate::state_transition::batch_transition::methods::StateTransitionCreationOptions
warning: unused import:crate::state_transition::batch_transition::methods::StateTransitionCreationOptions
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/v0/mod.rs:24:5
|
24 | use crate::state_transition::batch_transition::methods::StateTransitionCreationOptions;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note:#[warn(unused_imports)]on by default🪛 GitHub Check: Rust packages (drive) / Linting
[warning] 24-24: unused import:
crate::state_transition::batch_transition::methods::StateTransitionCreationOptions
warning: unused import:crate::state_transition::batch_transition::methods::StateTransitionCreationOptions
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/v0/mod.rs:24:5
|
24 | use crate::state_transition::batch_transition::methods::StateTransitionCreationOptions;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note:#[warn(unused_imports)]on by default
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
packages/rs-dpp/src/state_transition/mod.rs(5 hunks)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/v0/mod.rs(8 hunks)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v0/v0_methods.rs(14 hunks)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v0_methods.rs(13 hunks)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v1_methods.rs(25 hunks)packages/rs-dpp/src/state_transition/traits/state_transition_identity_signed.rs(2 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/mod.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/rs-dpp/src/state_transition/traits/state_transition_identity_signed.rs
- packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/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.
🪛 GitHub Check: Rust packages (drive-abci) / Linting
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v1_methods.rs
[warning] 38-38: unused import: crate::state_transition::batch_transition::methods::StateTransitionCreationOptions
warning: unused import: crate::state_transition::batch_transition::methods::StateTransitionCreationOptions
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v1_methods.rs:38:5
|
38 | use crate::state_transition::batch_transition::methods::StateTransitionCreationOptions;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[warning] 70-70: unused import: crate::state_transition::GetDataContractSecurityLevelRequirementFn
warning: unused import: crate::state_transition::GetDataContractSecurityLevelRequirementFn
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v1_methods.rs:70:5
|
70 | use crate::state_transition::GetDataContractSecurityLevelRequirementFn;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/v0/mod.rs
[warning] 24-24: unused import: crate::state_transition::batch_transition::methods::StateTransitionCreationOptions
warning: unused import: crate::state_transition::batch_transition::methods::StateTransitionCreationOptions
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/v0/mod.rs:24:5
|
24 | use crate::state_transition::batch_transition::methods::StateTransitionCreationOptions;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: #[warn(unused_imports)] on by default
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v0/v0_methods.rs
[warning] 39-39: unused import: crate::state_transition::batch_transition::methods::StateTransitionCreationOptions
warning: unused import: crate::state_transition::batch_transition::methods::StateTransitionCreationOptions
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v0/v0_methods.rs:39:5
|
39 | use crate::state_transition::batch_transition::methods::StateTransitionCreationOptions;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[warning] 35-35: unused import: crate::data_contract::document_type::accessors::DocumentTypeV0Getters
warning: unused import: crate::data_contract::document_type::accessors::DocumentTypeV0Getters
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v0/v0_methods.rs:35:5
|
35 | use crate::data_contract::document_type::accessors::DocumentTypeV0Getters;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v0_methods.rs
[warning] 43-43: unused import: crate::state_transition::batch_transition::methods::StateTransitionCreationOptions
warning: unused import: crate::state_transition::batch_transition::methods::StateTransitionCreationOptions
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v0_methods.rs:43:5
|
43 | use crate::state_transition::batch_transition::methods::StateTransitionCreationOptions;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[warning] 40-40: unused import: crate::data_contract::document_type::accessors::DocumentTypeV0Getters
warning: unused import: crate::data_contract::document_type::accessors::DocumentTypeV0Getters
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v0_methods.rs:40:5
|
40 | use crate::data_contract::document_type::accessors::DocumentTypeV0Getters;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
🪛 GitHub Check: Rust packages (drive) / Linting
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v1_methods.rs
[warning] 38-38: unused import: crate::state_transition::batch_transition::methods::StateTransitionCreationOptions
warning: unused import: crate::state_transition::batch_transition::methods::StateTransitionCreationOptions
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v1_methods.rs:38:5
|
38 | use crate::state_transition::batch_transition::methods::StateTransitionCreationOptions;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[warning] 70-70: unused import: crate::state_transition::GetDataContractSecurityLevelRequirementFn
warning: unused import: crate::state_transition::GetDataContractSecurityLevelRequirementFn
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v1_methods.rs:70:5
|
70 | use crate::state_transition::GetDataContractSecurityLevelRequirementFn;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/v0/mod.rs
[warning] 24-24: unused import: crate::state_transition::batch_transition::methods::StateTransitionCreationOptions
warning: unused import: crate::state_transition::batch_transition::methods::StateTransitionCreationOptions
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/v0/mod.rs:24:5
|
24 | use crate::state_transition::batch_transition::methods::StateTransitionCreationOptions;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: #[warn(unused_imports)] on by default
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v0/v0_methods.rs
[warning] 39-39: unused import: crate::state_transition::batch_transition::methods::StateTransitionCreationOptions
warning: unused import: crate::state_transition::batch_transition::methods::StateTransitionCreationOptions
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v0/v0_methods.rs:39:5
|
39 | use crate::state_transition::batch_transition::methods::StateTransitionCreationOptions;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[warning] 35-35: unused import: crate::data_contract::document_type::accessors::DocumentTypeV0Getters
warning: unused import: crate::data_contract::document_type::accessors::DocumentTypeV0Getters
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v0/v0_methods.rs:35:5
|
35 | use crate::data_contract::document_type::accessors::DocumentTypeV0Getters;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v0_methods.rs
[warning] 43-43: unused import: crate::state_transition::batch_transition::methods::StateTransitionCreationOptions
warning: unused import: crate::state_transition::batch_transition::methods::StateTransitionCreationOptions
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v0_methods.rs:43:5
|
43 | use crate::state_transition::batch_transition::methods::StateTransitionCreationOptions;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[warning] 40-40: unused import: crate::data_contract::document_type::accessors::DocumentTypeV0Getters
warning: unused import: crate::data_contract::document_type::accessors::DocumentTypeV0Getters
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v0_methods.rs:40:5
|
40 | use crate::data_contract::document_type::accessors::DocumentTypeV0Getters;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
⏰ Context from checks skipped due to timeout of 90000ms (14)
- GitHub Check: Rust packages (wasm-dpp) / Unused dependencies
- GitHub Check: Rust packages (wasm-dpp) / Tests
- 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 (dash-sdk) / Check each feature
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (drive-abci) / Check each feature
- GitHub Check: Rust packages (drive-abci) / Tests
- 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 (16)
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v0_methods.rs (8)
98-111: Refactoring to use consolidated options parameter.The code has been successfully refactored to replace multiple separate feature version parameters with a single
StateTransitionCreationOptionsparameter. The options are properly unwrapped with defaults and their fields are correctly used in the document creation transition.
121-127: Enhanced security with dynamic security level requirement.The security level requirement is now dynamically determined from the document type instead of using a hardcoded value. This improves security by ensuring that each document type's specific security requirements are enforced during signing.
141-169: Consistent pattern applied to replacement transitions.The same refactoring pattern has been correctly applied to document replacement transitions, maintaining consistency throughout the codebase.
183-211: Consistent pattern applied to deletion transitions.The same refactoring pattern has been correctly applied to document deletion transitions, maintaining consistency throughout the codebase.
226-255: Consistent pattern applied to transfer transitions.The same refactoring pattern has been correctly applied to document transfer transitions, maintaining consistency throughout the codebase.
270-299: Consistent pattern applied to document update price transitions.The same refactoring pattern has been correctly applied to document update price transitions, maintaining consistency throughout the codebase.
315-343: Consistent pattern applied to document purchase transitions.The same refactoring pattern has been correctly applied to document purchase transitions, maintaining consistency throughout the codebase.
39-43:Details
✅ Verification successful
Imports are correctly added but flagged as unused by linter.
The new imports for
DocumentTypeV0GettersandStateTransitionCreationOptionsare correctly used throughout the code but are flagged by the linter as unused. This is a false positive since they're used for accessing the security level requirement method and for type declarations respectively.
🏁 Script executed:
#!/bin/bash # Verify that these imports are actually used in the code echo "Checking usage of DocumentTypeV0Getters:" grep -n "security_level_requirement" packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v0_methods.rs echo -e "\nChecking usage of StateTransitionCreationOptions:" grep -n "options: Option<StateTransitionCreationOptions>" packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v0_methods.rsLength of output: 1379
False positive unused import warnings for DocumentTypeV0Getters and StateTransitionCreationOptions
Although the linter flags these imports as unused, they are in fact referenced throughout this file:
- DocumentTypeV0Getters is used by calls to
document_type.security_level_requirement()(e.g., lines 121, 163, 205, 249, 293, 337).- StateTransitionCreationOptions appears in method signatures as
options: Option<StateTransitionCreationOptions>(e.g., lines 98, 141, 183, 226, 270, 315).Please retain these imports. To suppress the false positives, consider adding:
#[allow(unused_imports)]above the affected
usestatements or adjust your lint configuration accordingly.🧰 Tools
🪛 GitHub Check: Rust packages (drive-abci) / Linting
[warning] 43-43: unused import:
crate::state_transition::batch_transition::methods::StateTransitionCreationOptions
warning: unused import:crate::state_transition::batch_transition::methods::StateTransitionCreationOptions
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v0_methods.rs:43:5
|
43 | use crate::state_transition::batch_transition::methods::StateTransitionCreationOptions;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[warning] 40-40: unused import:
crate::data_contract::document_type::accessors::DocumentTypeV0Getters
warning: unused import:crate::data_contract::document_type::accessors::DocumentTypeV0Getters
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v0_methods.rs:40:5
|
40 | use crate::data_contract::document_type::accessors::DocumentTypeV0Getters;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^🪛 GitHub Check: Rust packages (drive) / Linting
[warning] 43-43: unused import:
crate::state_transition::batch_transition::methods::StateTransitionCreationOptions
warning: unused import:crate::state_transition::batch_transition::methods::StateTransitionCreationOptions
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v0_methods.rs:43:5
|
43 | use crate::state_transition::batch_transition::methods::StateTransitionCreationOptions;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[warning] 40-40: unused import:
crate::data_contract::document_type::accessors::DocumentTypeV0Getters
warning: unused import:crate::data_contract::document_type::accessors::DocumentTypeV0Getters
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v0_methods.rs:40:5
|
40 | use crate::data_contract::document_type::accessors::DocumentTypeV0Getters;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v0/v0_methods.rs (8)
93-123: Refactoring to use consolidated options parameter and dynamic security level.The document creation transition method has been successfully refactored to:
- Use a single consolidated options parameter
- Dynamically determine the security level requirement from the document type
- Use the signing options from the parameters
This aligns with the PR objective to ensure correct security levels for transitions.
136-166: Consistent pattern applied to replacement transitions.The same refactoring pattern has been correctly applied to document replacement transitions, maintaining consistency throughout the codebase.
179-210: Consistent pattern applied to transfer transitions.The same refactoring pattern has been correctly applied to document transfer transitions, maintaining consistency throughout the codebase.
222-252: Consistent pattern applied to deletion transitions.The same refactoring pattern has been correctly applied to document deletion transitions, maintaining consistency throughout the codebase.
265-296: Consistent pattern applied to update price transitions.The same refactoring pattern has been correctly applied to document update price transitions, maintaining consistency throughout the codebase.
310-340: Consistent pattern applied to purchase transitions.The same refactoring pattern has been correctly applied to document purchase transitions, maintaining consistency throughout the codebase.
34-39:Details
✅ Verification successful
Imports are correctly added but flagged as unused by linter.
The new imports for
DocumentTypeV0GettersandStateTransitionCreationOptionsare correctly used throughout the code but are flagged by the linter as unused. This is a false positive since they're used for accessing the security level requirement method and for type declarations respectively.
🏁 Script executed:
#!/bin/bash # Verify that these imports are actually used in the code echo "Checking usage of DocumentTypeV0Getters:" grep -n "security_level_requirement" packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v0/v0_methods.rs echo -e "\nChecking usage of StateTransitionCreationOptions:" grep -n "options: Option<StateTransitionCreationOptions>" packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v0/v0_methods.rsLength of output: 1379
False-positive linter warnings for new imports in v0_methods.rs
I’ve verified that both imports are indeed used in this file:
DocumentTypeV0Gettersis invoked viasecurity_level_requirement()at lines 116, 158, 202, 244, 288, and 332.StateTransitionCreationOptionsappears in the method signatures at lines 93, 136, 179, 222, 265, and 310.These linter errors are false positives—these types are required for security level checks and transition‐creation options. You can safely ignore the warnings or suppress them via a
#[allow(unused_imports)]on this module.🧰 Tools
🪛 GitHub Check: Rust packages (drive-abci) / Linting
[warning] 39-39: unused import:
crate::state_transition::batch_transition::methods::StateTransitionCreationOptions
warning: unused import:crate::state_transition::batch_transition::methods::StateTransitionCreationOptions
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v0/v0_methods.rs:39:5
|
39 | use crate::state_transition::batch_transition::methods::StateTransitionCreationOptions;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[warning] 35-35: unused import:
crate::data_contract::document_type::accessors::DocumentTypeV0Getters
warning: unused import:crate::data_contract::document_type::accessors::DocumentTypeV0Getters
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v0/v0_methods.rs:35:5
|
35 | use crate::data_contract::document_type::accessors::DocumentTypeV0Getters;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^🪛 GitHub Check: Rust packages (drive) / Linting
[warning] 39-39: unused import:
crate::state_transition::batch_transition::methods::StateTransitionCreationOptions
warning: unused import:crate::state_transition::batch_transition::methods::StateTransitionCreationOptions
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v0/v0_methods.rs:39:5
|
39 | use crate::state_transition::batch_transition::methods::StateTransitionCreationOptions;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[warning] 35-35: unused import:
crate::data_contract::document_type::accessors::DocumentTypeV0Getters
warning: unused import:crate::data_contract::document_type::accessors::DocumentTypeV0Getters
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v0/v0_methods.rs:35:5
|
35 | use crate::data_contract::document_type::accessors::DocumentTypeV0Getters;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
342-350:Details
❓ Verification inconclusive
Verify callers have been updated.
This method implementation remains unchanged, but ensure all callers have been updated to use the new parameter structure for creating batch transitions.
🏁 Script executed:
#!/bin/bash # Search for calls to methods in DocumentsBatchTransitionMethodsV0 to verify they've been updated echo "Looking for method calls that might need updating:" rg "new_document_.*_transition_from_document" --type rust | grep -v "fn new_document"Length of output: 31255
🏁 Script executed:
#!/bin/bash # Show the signatures of document batch transition constructors to verify expected parameters rg "pub fn new_document_.*_transition_from_document" -n packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/mod.rs -A2Length of output: 157
🏁 Script executed:
#!/bin/bash # Locate definitions of document batch transition constructors to inspect their signatures rg -n "fn new_document_.*_transition_from_document" packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/mod.rs -C2Length of output: 1394
Verify new batch‐transition constructor signature usage
The document batch‐transition methods now require two parameters—
Documentand itsDocumentTypeRef—so all calls toBatchTransition::new_document_*_transition_from_document(...)must be updated to pass both arguments.
Please review and update the following call sites (and any others in your workspace):
- packages/rs-sdk/src/platform/transition/{put_document.rs, update_price_of_document.rs, transfer_document.rs, purchase_document.rs}
- packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/mod.rs (definitions)
- packages/rs-drive-abci/src/execution/check_tx/v0/mod.rs
- packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/{mod.rs, batch/tests/document/*.rs}
- packages/rs-drive-abci/src/execution/platform_events/block_processing_end_events/tests.rs
You can locate any remaining occurrences with:
rg "BatchTransition::new_document_.*_transition_from_document" -nand ensure each invocation now supplies both
documentand itsdocument_type.
QuantumExplorer
left a comment
QuantumExplorer
left a comment
There was a problem hiding this comment.
Self reviewed.
1 participant
Issue being fixed or feature implemented
This PR addresses an issue where the security level for token transitions was not being correctly set to
CRITICALwhen necessary.The PR also refactors signing state transitions to allow for signing with invalid security level keys for testing.
What was done?
DocumentsBatchTransitionMethodsV0to ensure that all token transitions require aCRITICALsecurity level.How Has This Been Tested?
The changes have been tested by updating the existing test cases in
token_tests.rsto ensure that the security level is correctly set and that the transitions function as expected.Breaking Changes
None
Checklist
For repository code-owners and collaborators only
Summary by CodeRabbit