-
Notifications
You must be signed in to change notification settings - Fork 43
fix(platform): start as paused was not working #2582
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThis update introduces the ability to configure tokens to start in a paused state and ensures that this status is correctly handled during contract insertion and token transfers. New setter methods and trait implementations are added to manage the paused status in token configuration. Tests are introduced to verify that transfers are blocked when tokens are paused and allowed when resumed. The contract insertion logic is updated to persist the initial token status. Minor comment corrections are also included. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ContractInserter
participant Database
User->>ContractInserter: Create data contract with token (start_as_paused)
ContractInserter->>Database: Insert token configuration
ContractInserter->>Database: Insert initial TokenStatus (paused/unpaused)
Note over Database: Token status is now persisted
sequenceDiagram
participant User
participant TokenTransfer
participant Database
User->>TokenTransfer: Attempt token transfer
TokenTransfer->>Database: Check TokenStatus
alt Token is paused
TokenTransfer-->>User: Fail with TokenIsPausedError
else Token is unpaused
TokenTransfer->>Database: Update balances
TokenTransfer-->>User: Transfer succeeds
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 (14)
✨ 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (3)
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/mod.rs (3)
3416-3419
: Clarify serialized-transition variable naming
resume_transition_transition
is a bit of a tongue-twister and doesn’t immediately convey that this is the serialized form of the state-transition. Consider renaming it to something likeresume_serialized_transition
(or simplyserialized_resume_transition
) to improve readability.- let resume_transition_transition = resume_transition - .serialize_to_bytes() - .expect("expected documents batch serialized state transition"); + let resume_serialized_transition = resume_transition + .serialize_to_bytes() + .expect("expected serialized resume transition");
3572-3589
: Variable name does not match the action performedHere the variable is called
resume_transition
, but the emergency action isTokenEmergencyAction::Pause
.
Renaming it topause_transition
(and the subsequent serialized variable) will avoid confusion when skimming the test.- let resume_transition = BatchTransition::new_token_emergency_action_transition( + let pause_transition = BatchTransition::new_token_emergency_action_transition( token_id, identity.id(), contract.id(), 0, - TokenEmergencyAction::Pause, + TokenEmergencyAction::Pause, @@ - let resume_transition_transition = resume_transition + let pause_serialized_transition = pause_transition .serialize_to_bytes() - .expect("expected documents batch serialized state transition"); + .expect("expected serialized pause transition");
3591-3593
: Apply same naming fix for consistencyIf you adopt the previous suggestion, adjust the variable used in the subsequent
process_raw_state_transitions
call accordingly:- &[resume_transition_transition.clone()], + &[pause_serialized_transition.clone()],Keeping terminology consistent throughout the test makes maintenance less error-prone.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/rs-dpp/src/data_contract/associated_token/token_configuration/accessors/mod.rs
(1 hunks)packages/rs-dpp/src/data_contract/associated_token/token_configuration/accessors/v0/mod.rs
(2 hunks)packages/rs-dpp/src/data_contract/associated_token/token_configuration/v0/accessors.rs
(2 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/mod.rs
(2 hunks)packages/rs-drive/src/drive/contract/insert/insert_contract/v1/mod.rs
(2 hunks)packages/rs-drive/src/util/batch/drive_op_batch/contract.rs
(1 hunks)
🧰 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-dpp/src/data_contract/associated_token/token_configuration/accessors/v0/mod.rs (2)
packages/rs-dpp/src/data_contract/associated_token/token_configuration/v0/accessors.rs (2)
set_start_as_paused
(192-194)start_as_paused
(49-51)packages/rs-dpp/src/data_contract/associated_token/token_configuration/accessors/mod.rs (2)
set_start_as_paused
(198-202)start_as_paused
(60-64)
packages/rs-dpp/src/data_contract/associated_token/token_configuration/accessors/mod.rs (2)
packages/rs-dpp/src/data_contract/associated_token/token_configuration/accessors/v0/mod.rs (2)
set_start_as_paused
(93-93)start_as_paused
(30-30)packages/rs-dpp/src/data_contract/associated_token/token_configuration/v0/accessors.rs (2)
set_start_as_paused
(192-194)start_as_paused
(49-51)
packages/rs-dpp/src/data_contract/associated_token/token_configuration/v0/accessors.rs (2)
packages/rs-dpp/src/data_contract/associated_token/token_configuration/accessors/v0/mod.rs (2)
set_start_as_paused
(93-93)start_as_paused
(30-30)packages/rs-dpp/src/data_contract/associated_token/token_configuration/accessors/mod.rs (2)
set_start_as_paused
(198-202)start_as_paused
(60-64)
🪛 GitHub Check: Rust packages (drive-abci) / Linting
packages/rs-dpp/src/data_contract/associated_token/token_configuration/accessors/v0/mod.rs
[warning] 7-7: unused import: TokenConfiguration
warning: unused import: TokenConfiguration
--> packages/rs-dpp/src/data_contract/associated_token/token_configuration/accessors/v0/mod.rs:7:51
|
7 | use crate::data_contract::{GroupContractPosition, TokenConfiguration};
| ^^^^^^^^^^^^^^^^^^
|
= note: #[warn(unused_imports)]
on by default
packages/rs-dpp/src/data_contract/associated_token/token_configuration/v0/accessors.rs
[warning] 13-13: unused import: TokenConfiguration
warning: unused import: TokenConfiguration
--> packages/rs-dpp/src/data_contract/associated_token/token_configuration/v0/accessors.rs:13:51
|
13 | use crate::data_contract::{GroupContractPosition, TokenConfiguration};
| ^^^^^^^^^^^^^^^^^^
🪛 GitHub Check: Rust packages (drive) / Linting
packages/rs-dpp/src/data_contract/associated_token/token_configuration/accessors/v0/mod.rs
[warning] 7-7: unused import: TokenConfiguration
warning: unused import: TokenConfiguration
--> packages/rs-dpp/src/data_contract/associated_token/token_configuration/accessors/v0/mod.rs:7:51
|
7 | use crate::data_contract::{GroupContractPosition, TokenConfiguration};
| ^^^^^^^^^^^^^^^^^^
|
= note: #[warn(unused_imports)]
on by default
packages/rs-dpp/src/data_contract/associated_token/token_configuration/v0/accessors.rs
[warning] 13-13: unused import: TokenConfiguration
warning: unused import: TokenConfiguration
--> packages/rs-dpp/src/data_contract/associated_token/token_configuration/v0/accessors.rs:13:51
|
13 | use crate::data_contract::{GroupContractPosition, TokenConfiguration};
| ^^^^^^^^^^^^^^^^^^
🪛 GitHub Check: Rust packages (wasm-dpp) / Linting
packages/rs-dpp/src/data_contract/associated_token/token_configuration/accessors/v0/mod.rs
[warning] 7-7: unused import: TokenConfiguration
warning: unused import: TokenConfiguration
--> packages/rs-dpp/src/data_contract/associated_token/token_configuration/accessors/v0/mod.rs:7:51
|
7 | use crate::data_contract::{GroupContractPosition, TokenConfiguration};
| ^^^^^^^^^^^^^^^^^^
|
= note: #[warn(unused_imports)]
on by default
packages/rs-dpp/src/data_contract/associated_token/token_configuration/v0/accessors.rs
[warning] 13-13: unused import: TokenConfiguration
warning: unused import: TokenConfiguration
--> packages/rs-dpp/src/data_contract/associated_token/token_configuration/v0/accessors.rs:13:51
|
13 | use crate::data_contract::{GroupContractPosition, TokenConfiguration};
| ^^^^^^^^^^^^^^^^^^
🪛 GitHub Check: Rust packages (dpp) / Linting
packages/rs-dpp/src/data_contract/associated_token/token_configuration/accessors/v0/mod.rs
[failure] 7-7: unused import: TokenConfiguration
error: unused import: TokenConfiguration
--> packages/rs-dpp/src/data_contract/associated_token/token_configuration/accessors/v0/mod.rs:7:51
|
7 | use crate::data_contract::{GroupContractPosition, TokenConfiguration};
| ^^^^^^^^^^^^^^^^^^
|
= note: -D unused-imports
implied by -D warnings
= help: to override -D warnings
add #[allow(unused_imports)]
packages/rs-dpp/src/data_contract/associated_token/token_configuration/v0/accessors.rs
[failure] 13-13: unused import: TokenConfiguration
error: unused import: TokenConfiguration
--> packages/rs-dpp/src/data_contract/associated_token/token_configuration/v0/accessors.rs:13:51
|
13 | use crate::data_contract::{GroupContractPosition, TokenConfiguration};
| ^^^^^^^^^^^^^^^^^^
⏰ Context from checks skipped due to timeout of 90000ms (13)
- GitHub Check: Rust packages (dash-sdk) / Check each feature
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (dash-sdk) / Unused dependencies
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive-abci) / Unused dependencies
- GitHub Check: Rust packages (drive-abci) / Check each feature
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build JS packages / Build JS
🔇 Additional comments (6)
packages/rs-drive/src/util/batch/drive_op_batch/contract.rs (1)
37-37
: Minor comment correction.The comment has been corrected by adding the word "to", making it grammatically accurate.
packages/rs-dpp/src/data_contract/associated_token/token_configuration/accessors/v0/mod.rs (1)
92-94
: LGTM: New setter for token start-as-paused status.This new setter method correctly adds the ability to configure tokens to start in a paused state, addressing the PR objective to fix the "start as paused" functionality. The implementation is consistent with the existing API patterns.
packages/rs-dpp/src/data_contract/associated_token/token_configuration/v0/accessors.rs (1)
191-194
: LGTM: Implementation of start-as-paused setter.The implementation is straightforward and correct, directly setting the
start_as_paused
field in theTokenConfigurationV0
struct. This complements the existing getter method (lines 49-51) and completes the API for controlling the token's initial paused state.packages/rs-dpp/src/data_contract/associated_token/token_configuration/accessors/mod.rs (1)
197-202
: LGTM: Enum dispatch implementation for start-as-paused.This implementation properly routes the
set_start_as_paused
method to the appropriate version-specific implementation (currently just V0). The pattern follows the established pattern used for other accessors in this file, maintaining consistency in the codebase.packages/rs-drive/src/drive/contract/insert/insert_contract/v1/mod.rs (2)
15-15
: Appropriate imports added for token status functionality.The added imports correctly bring in the necessary components for managing token statuses, including path utilities and serialization capabilities.
Also applies to: 19-19, 23-24
235-248
: Good implementation of token pause status persistence.This change correctly addresses the issue where "start as paused" functionality wasn't working during token creation. The implementation:
- Creates a TokenStatus instance using the configured start_as_paused value from token configuration
- Serializes the status to bytes
- Persists it in the database at the appropriate path alongside other token information
The code follows the existing pattern of token-related batch operations and has proper error handling.
packages/rs-dpp/src/data_contract/associated_token/token_configuration/accessors/v0/mod.rs
Outdated
Show resolved
Hide resolved
packages/rs-dpp/src/data_contract/associated_token/token_configuration/v0/accessors.rs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Self reviewed.
Issue being fixed or feature implemented
In the token config start as paused was not working, as it was only being used on token update and not insert.
What was done?
Fixed it and wrote tests.
How Has This Been Tested?
Wrote unit tests.
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit