fix(platform): resolve direct purchase from self issue causing chain stall #2663
Conversation
WalkthroughThe changes introduce a test covering self-purchase scenarios in token direct selling, update logic to skip balance transfers when the buyer and seller are the same identity, and restrict the visibility of a fee validation function. No public interfaces or function signatures were altered beyond visibility adjustments and the new test addition. Changes
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/rs-drive-abci/src/execution/platform_events/state_transition_processing/validate_fees_of_event/v0/mod.rs(1 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/direct_selling/mod.rs(4 hunks)packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_direct_purchase_transition.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.
⏰ Context from checks skipped due to timeout of 90000ms (14)
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (drive) / Formatting
- GitHub Check: Rust packages (dash-sdk) / Formatting
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (drive-abci) / Check each feature
- GitHub Check: Rust packages (dash-sdk) / Check each feature
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive-abci) / Unused dependencies
- GitHub Check: Rust packages (drive-abci) / Linting
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
🔇 Additional comments (4)
packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_direct_purchase_transition.rs (1)
52-71: LGTM! Correct implementation to prevent self-transfer operations.The conditional logic correctly identifies self-purchase scenarios and skips the balance transfer operations when the buyer and seller are the same identity. This prevents the chain stall issue while maintaining the token minting functionality. The comment clearly explains the intent.
packages/rs-drive-abci/src/execution/platform_events/state_transition_processing/validate_fees_of_event/v0/mod.rs (1)
39-39: Good encapsulation improvement.Restricting the visibility from
pub(in crate::execution)topub(super)appropriately limits the function's scope to where it's actually needed, improving code organization and encapsulation.packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/direct_selling/mod.rs (2)
337-340: Good cleanup of redundant pattern match prefix.Removing the
StateTransitionExecutionResult::prefix improves readability since the enum variant is unambiguous in this context.
576-733: Excellent comprehensive test for self-purchase scenario.This test thoroughly validates the fix for the self-purchase issue. It correctly:
- Sets up an identity that acts as both buyer and seller
- Verifies successful token purchase execution
- Confirms token balance increases by the purchased amount
- Validates that credit balance only decreases by transaction fees, not the purchase price
- Uses proper transaction handling and validation
The test directly corresponds to the conditional logic added in the main transition code and provides good coverage for the edge case that was causing chain stalls.
|
|
||
| use super::*; | ||
|
|
||
| use crate::error::Error; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify if the import is needed.
The Error import was added but doesn't appear to be used in the visible code. Please verify if this import is actually needed or if it can be removed.
🏁 Script executed:
#!/bin/bash
# Search for usage of Error type in this file
rg -n "Error::" packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/direct_selling/mod.rsLength of output: 426
🏁 Script executed:
#!/bin/bash
# Search for any plain uses of `Error` (excluding `StateError`, `ConsensusError`, etc.)
rg -En '\bError\b' packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/direct_selling/mod.rsLength of output: 182
#!/bin/bash
# Search for standalone uses of `Error` in the test file to confirm if the import is used
rg -n -w Error packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/direct_selling/mod.rsRemove unused Error import
The use crate::error::Error; on line 10 of
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/direct_selling/mod.rs
is not referenced in this test module and can be dropped:
- use crate::error::Error;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| use crate::error::Error; | |
| // (The `use crate::error::Error;` import has been removed as it was unused) |
🤖 Prompt for AI Agents
In
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/direct_selling/mod.rs
at line 10, the import statement 'use crate::error::Error;' is not used anywhere
in the file. Remove this unused import to clean up the code and avoid
unnecessary dependencies.
|
Self reviewed |
QuantumExplorer
changed the title
1 participant
Issue being fixed or feature implemented
Fixes a bug where a user could attempt to purchase tokens from themselves, causing a stall in the chain.
What was done?
validate_fees_of_event_v0function's visibility frompub(in crate::execution)topub(super).TokenDirectPurchaseTransitionActionto prevent identity balance operations if the owner is the same as the buyer.How Has This Been Tested?
New test cases were added to ensure that purchasing tokens from oneself is correctly handled without causing chain stalls.
Breaking Changes
None
Checklist
For repository code-owners and collaborators only
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Refactor