-
Notifications
You must be signed in to change notification settings - Fork 43
feat(platform): direct selling of tokens to users #2534
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
WalkthroughThe changes introduce comprehensive support for direct purchase pricing functionality. New message types, fields, accessors, and transition methods have been added across protocol buffers, data contracts, state transitions, drive operations, and token history queries. These updates expand the system’s ability to process direct purchase events, validate new transition types, and manage associated pricing schedules consistently throughout the codebase. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Proto as Platform Proto
participant DPP as RS-DPP (State Transition)
participant Drive as Drive Layer
participant History as Token History
Client->>Proto: Sends UpdateDirectPurchasePriceEvent
Proto->>DPP: Process direct purchase pricing update
DPP->>Drive: Write pricing schedule & transition actions
Drive->>History: Update token direct purchase history
History-->>Client: Confirm update processing
Suggested reviewers
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 4
🧹 Nitpick comments (39)
packages/rs-drive/src/drive/tokens/mod.rs (1)
51-51
: New module added for direct token purchases.The module adds functionality for direct token purchases. However, unlike most other modules in this file, it's missing a documentation comment describing its purpose.
Consider adding a doc comment to the module declaration similar to other modules in this file:
+/// Handles operations related to direct purchases of tokens. mod direct_purchase;
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/mod.rs (2)
1-1
: New Module Introduction:direct_selling
The addition of the
direct_selling
module at the top of this file signals that new functionality for direct token selling is being introduced. Please ensure that this module includes comprehensive tests to cover all the new direct selling behaviors and edge cases. Also, verify that any shared test setups or utilities (e.g. state transition processing, identity setup) are integrated consistently with the rest of the token tests.
3-3
: Minor Formatting/Ordering NoteA change marker is present on line 3; if this is intended as part of module declaration ordering or a minor formatting update, please double-check that the ordering of module declarations (with
direct_selling
on line 1 anddistribution
following) aligns with the project’s stylistic and logical grouping guidelines. This will help maintain readability and clarity as more modules are added in the future.packages/rs-dpp/src/data_contract/associated_token/token_distribution_rules/v0/mod.rs (1)
61-63
: Fix formatting in Display implementationThere's a minor formatting issue in the Display implementation - an extra colon in the field name.
- minting_allow_choosing_destination_rules: {},\n \ - change_direct_purchase_pricing_rules:: {}\n\ + minting_allow_choosing_destination_rules: {},\n \ + change_direct_purchase_pricing_rules: {}\n\packages/rs-dpp/src/data_contract/associated_token/token_keeps_history_rules/v0/mod.rs (1)
60-66
: Minor formatting issue in Display implementationThere's a spacing inconsistency in the Display implementation's formatting string.
The line breaks and spaces in the formatting string are inconsistent. Consider this fix:
- "TokenKeepsHistoryRulesV0 {{\n keeps_transfer_history: {},\n keeps_freezing_history: {},\n keeps_minting_history: {},\n keeps_burning_history: {},\n keeps_direct_pricing_history: {}, \n keeps_direct_purchase_history: {}\n", + "TokenKeepsHistoryRulesV0 {{\n keeps_transfer_history: {},\n keeps_freezing_history: {},\n keeps_minting_history: {},\n keeps_burning_history: {},\n keeps_direct_pricing_history: {},\n keeps_direct_purchase_history: {}\n",The current format has a space before "keeps_direct_purchase_history" instead of two spaces like the other fields.
packages/rs-dpp/src/data_contract/associated_token/token_distribution_rules/v0/accessors.rs (2)
71-77
: Add documentation for new getter methods.The new getter methods for
change_direct_purchase_pricing_rules
lack documentation comments while all other methods in this file have detailed documentation.+ /// Returns the rules for changing direct purchase pricing. fn change_direct_purchase_pricing_rules(&self) -> &ChangeControlRules { &self.change_direct_purchase_pricing_rules } + /// Returns the rules for changing direct purchase pricing (mutable). fn change_direct_purchase_pricing_rules_mut(&mut self) -> &mut ChangeControlRules { &mut self.change_direct_purchase_pricing_rules }
123-125
: Add documentation for new setter method.The new setter method lacks documentation comments, unlike other setter methods in this file.
+ /// Sets the rules for changing direct purchase pricing. fn set_change_direct_purchase_pricing_rules(&mut self, rules: ChangeControlRules) { self.change_direct_purchase_pricing_rules = rules; }
packages/rs-drive/src/verify/state_transition/verify_state_transition_was_executed_with_proof/v0/mod.rs (1)
502-520
: DirectPurchase verification logic looks correct but contains code duplication.The verification implementation for
DirectPurchase
correctly handles both cases:
- Using historical query if the token keeps direct purchase history
- Verifying token balance otherwise
However, this implementation duplicates the logic from the
Burn
transition case. Consider refactoring this common pattern into a shared function to reduce code duplication.packages/rs-dpp/src/tokens/token_pricing_schedule.rs (1)
32-41
: Consider handling empty SetPrices case more explicitlyThe
minimum_purchase_amount_and_price
method falls back tounwrap_or_default()
(which returns(0, 0)
) when the SetPrices map is empty. This might not be a valid state for token purchases.impl TokenPricingSchedule { pub fn minimum_purchase_amount_and_price(&self) -> (TokenAmount, Credits) { match self { TokenPricingSchedule::SinglePrice(price) => (1, *price), TokenPricingSchedule::SetPrices(prices) => prices .first_key_value() .map(|(amount, cost)| (*amount, *cost)) - .unwrap_or_default(), + .unwrap_or((1, 0)), // Explicit default with minimum quantity 1 } } }packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_set_price_for_direct_purchase_transition_action/v0/mod.rs (3)
9-19
: Fix incorrect documentation commentThe struct is incorrectly documented as "Token issuance transition action v0" but it's actually for setting prices for direct purchases.
- /// Token issuance transition action v0 + /// Token set price for direct purchase transition action v0
21-32
: Fix incorrect trait documentationThe trait is incorrectly documented as "Accessors for
TokenIssuanceTransitionActionV0
" but should reference the correct struct.- /// Accessors for `TokenIssuanceTransitionActionV0` + /// Accessors for `TokenSetPriceForDirectPurchaseTransitionActionV0`
32-33
: Fix incorrect method documentationThe
set_price
method is incorrectly documented as setting the amount of tokens to issuance.- /// Sets the amount of tokens to issuance + /// Sets the price schedule for direct purchasespackages/rs-drive/src/drive/tokens/estimated_costs/for_token_direct_selling_prices/mod.rs (1)
19-21
: Documentation parameter mismatchThe documentation mentions a
token_id
parameter that doesn't exist in the method signature. This parameter isn't used in the actual implementation.-/// # Parameters -/// - `token_id`: A 32-byte identifier for the token whose balance changes are being estimated. -/// - `estimated_costs_only_with_layer_info`: A mutable reference to a HashMap that holds +/// # Parameters +/// - `estimated_costs_only_with_layer_info`: A mutable reference to a HashMap that holdspackages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/document_transition.rs (1)
8-8
: Consider breaking long import lineThis import line is quite long and could be split into multiple lines for better readability.
-use crate::state_transition::batch_transition::{DocumentCreateTransition, DocumentDeleteTransition, DocumentReplaceTransition, TokenBurnTransition, TokenConfigUpdateTransition, TokenDestroyFrozenFundsTransition, TokenEmergencyActionTransition, TokenFreezeTransition, TokenMintTransition, TokenClaimTransition, TokenTransferTransition, TokenUnfreezeTransition, TokenDirectPurchaseTransition, TokenSetPriceForDirectPurchaseTransition}; +use crate::state_transition::batch_transition::{ + DocumentCreateTransition, DocumentDeleteTransition, DocumentReplaceTransition, + TokenBurnTransition, TokenClaimTransition, TokenConfigUpdateTransition, + TokenDestroyFrozenFundsTransition, TokenDirectPurchaseTransition, + TokenEmergencyActionTransition, TokenFreezeTransition, TokenMintTransition, + TokenSetPriceForDirectPurchaseTransition, TokenTransferTransition, TokenUnfreezeTransition +};packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_set_price_for_direct_purchase_transition_action/mod.rs (1)
12-13
: Rename doc comment to reflect price-setting rather than issuance.The doc comment says "/// Token issuance transition action", which likely carries over from a prior pattern. Rename it to something like "/// Token direct purchase price-setting transition action" for clarity.
-/// Token issuance transition action +/// Token direct purchase price-setting transition actionpackages/dapi-grpc/protos/platform/v0/platform.proto (1)
1668-1682
: Consider specifying maximum length constraints or usage guidance forpublic_note
.Although the code in Rust enforces a maximum length on
public_note
, that limit is not explicitly reflected here in the proto. Having a note or docstring about it (e.g., "Max 256 bytes") can guide client usage and reduce confusion. Otherwise, the newUpdateDirectPurchasePriceEvent
structure is straightforward and aligns well with the design for setting direct purchase prices.packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/resolvers/v0/mod.rs (1)
33-35
: Slight naming consistency suggestion.
Method name is longer; consider something more succinct, likeas_transition_token_set_direct_purchase_price()
, to mirroras_transition_token_direct_purchase()
.packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/mod.rs (2)
1000-1063
: Fix misleading comment when creating direct purchase price transitions.
The inline comment references “emergency action transition,” likely carried over from copying. Update it to accurately describe setting direct purchase price transitions.
1064-1121
: Correct the comment referencing “emergency action transition.”
This method is specifically creating a direct purchase transition. Adjust the comment to reflect the intended functionality.packages/rs-drive/src/drive/tokens/direct_purchase/set_direct_purchase_price/mod.rs (1)
57-80
: Handle future versions proactively.The matching on
platform_version.drive.methods.token.update.mint
only recognizes version0
, returning an error otherwise. This is a good safeguard; however, planning for future versions can reduce friction when introducing new Drive method versions.You can create a placeholder match arm or logging mechanism to capture new or under-development versions, making the transition to future versions more seamless.
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/action_validation/token/token_set_price_for_direct_purchase_transition_action/state_v0/mod.rs (1)
16-26
: Strengthen the trait’s documentation.While the trait
TokenSetPriceForDirectPurchaseTransitionActionStateValidationV0
clearly outlines the parameters required for validation, adding thorough doc comments describing expected invariants and any edge-case behaviors will benefit maintainability and readability.packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v0_methods.rs (1)
1093-1165
:new_token_change_direct_purchase_price_transition
method extends functionality neatly.
- Accepts an optional
TokenPricingSchedule
to update direct purchase price.- Patterns for group info and signature usage mirror other transitions.
- Suggest verifying that none of the optional fields need special fallback logic if omitted (e.g., default or previous price).
Overall structure is correct. Consider adding usage examples or inline doc comments to guide future maintainers.
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_transition.rs (1)
479-488
: Handle new token events for direct purchase and price updates
This logic is sound. However, note that unlike other token events, theDirectPurchase
variant does not store a public note—verify whether that is intentional or if you plan to include one.packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_direct_purchase_transition_action/v0/transformer.rs (2)
44-123
: Consider validating token_count and total_agreed_price
While the logic properly delegates toTokenBaseTransitionAction
for base transition handling, there's no explicit check to ensure that the token_count or total_agreed_price are non-negative or meet the expected constraints. If such validations occur elsewhere, please ensure they are consistently applied. Otherwise, consider adding checks here to prevent invalid data from propagating.Suggestion
Add a small validation for these fields before proceeding with fee calculation:let TokenDirectPurchaseTransitionV0 { base, token_count, total_agreed_price, } = value; + if token_count == 0 { + return Err(Error::ProtocolError(ProtocolError::GenericError( + "token_count must be greater than zero".to_string(), + ))); + } + if total_agreed_price == 0 { + return Err(Error::ProtocolError(ProtocolError::GenericError( + "total_agreed_price must be greater than zero".to_string(), + ))); + } let mut drive_operations = vec![];
125-234
: DRY concerns for borrowed vs. owned transitions
This second method effectively duplicates much of the logic from the first. While it’s valid, consider centralizing common logic (like fee calculation and the base action validity check) to reduce code duplication, which makes the code more maintainable.packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_set_price_for_direct_purchase_transition_action/transformer.rs (1)
69-118
: Repeat logic for borrowed references
Similar to the direct purchase transformer, this function largely replicates the single-ownership version. If additional complexity arises, consider extracting common code into a shared helper to maintain DRY principles.packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_set_price_for_direct_purchase_transition/v0_methods.rs (2)
31-63
: Optional typed validation forprice
Currently,price
is anOption<TokenPricingSchedule>
. If the code requires certain constraints (e.g., non-zero purchase price or well-formed schedule), ensure these are checked either in the caller or in a validation method.
73-95
: Consider including the minimum purchase amount in the action ID
Here, only the price is hashed. If the minimum purchase amount is also relevant to the action’s identity, consider incorporating it into the bytes slice for full context. Otherwise, this is a reasonable approach for generating a unique action ID.packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_set_price_for_direct_purchase_transition_action/v0/transformer.rs (4)
22-23
: Typographical correction in method documentation.The documentation repeatedly uses the phrase "set_price_for_direct_purchaseing," which appears to be a minor typographical or grammatical slip. Consider revising to "setting the price for direct purchase" for clarity.
44-60
: Consider reducing the number of parameters or using a builder pattern.
try_from_token_set_price_for_direct_purchase_transition_with_contract_lookup
has many parameters, leading to potential confusion and reduced readability. A builder pattern or a dedicated parameters struct might improve maintainability.
111-124
: Add test coverage for successful and failed fee calculations.The code calls
Drive::calculate_fee
with a constructed set ofdrive_operations
. It would be beneficial to add explicit test scenarios verifying correctness when fees are calculated successfully and when fee calculation errors occur.
127-153
: Duplicate logic identified across similar methods.
try_from_borrowed_token_set_price_for_direct_purchase_transition_with_contract_lookup
largely repeats the logic of the previous method, except for borrowing vs. owning the transition. If you frequently add or change logic here, consider refactoring common parts to a helper function to reduce duplication and potential inconsistencies.packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_direct_purchase_transition/v0/v0_methods.rs (1)
7-19
: Doc comments recommended for trait accessor methods.While the
TokenBaseTransitionAccessors
trait is properly implemented, adding brief doc comments for thebase()
,base_mut()
, andset_base()
methods would improve clarity for future maintainers.packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_set_price_for_direct_purchase_transition/v0/v0_methods.rs (1)
10-22
: Maintain consistent documentation for accessor methods.Although the accessor methods for
base
,base_mut
, andset_base
follow the trait contract, adding short doc comments would help new contributors understand how these base transitions integrate with direct purchase transitions.packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_direct_purchase_transition_action/v0/mod.rs (2)
10-11
: Fix doc comment to match struct name.The doc comment incorrectly says "Token issuance transition action v0" but should say "Token direct purchase transition action v0" to match the struct it's documenting.
-/// Token issuance transition action v0 +/// Token direct purchase transition action v0
22-23
: Fix doc comment to match trait name.The doc comment incorrectly refers to
TokenIssuanceTransitionActionV0
instead ofTokenDirectPurchaseTransitionActionV0
.-/// Accessors for `TokenIssuanceTransitionActionV0` +/// Accessors for `TokenDirectPurchaseTransitionActionV0`packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/action_validation/token/token_set_price_for_direct_purchase_transition_action/mod.rs (3)
15-25
: Document the validation trait more extensively.Currently, the trait lacks doc comments describing the validation steps or expected behavior. Adding Rust doc comments will help others understand usage, inputs, outputs, and expected outcomes.
Consider adding something like:
+/// Provides methods to validate the state of a `TokenSetPriceForDirectPurchaseTransitionAction`. pub trait TokenSetPriceForDirectPurchaseTransitionActionValidation { fn validate_state( &self, ...
30-62
: Validate unknown version paths in tests.The version handling returns an error for unknown versions, but it may be beneficial to include specific test coverage to ensure the error path remains robust and consistent.
Would you like to add a negative test case that passes an invalid platform version and asserts that an
UnknownVersionMismatch
error is returned?
39-60
: Confirm that handling new versions does not require frequent code changes.The match statement explicitly checks for version 0 and raises an error otherwise. If new versions are frequently released, this might become cumbersome to maintain. Consider using a more extensible pattern (e.g., fallback to the closest known version or a dynamic dispatch) if version proliferation is expected.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (94)
packages/dapi-grpc/protos/platform/v0/platform.proto
(2 hunks)packages/rs-dpp/src/data_contract/associated_token/token_configuration/v0/mod.rs
(4 hunks)packages/rs-dpp/src/data_contract/associated_token/token_distribution_rules/accessors/mod.rs
(2 hunks)packages/rs-dpp/src/data_contract/associated_token/token_distribution_rules/accessors/v0/mod.rs
(2 hunks)packages/rs-dpp/src/data_contract/associated_token/token_distribution_rules/v0/accessors.rs
(2 hunks)packages/rs-dpp/src/data_contract/associated_token/token_distribution_rules/v0/mod.rs
(3 hunks)packages/rs-dpp/src/data_contract/associated_token/token_keeps_history_rules/accessors/mod.rs
(2 hunks)packages/rs-dpp/src/data_contract/associated_token/token_keeps_history_rules/accessors/v0/mod.rs
(2 hunks)packages/rs-dpp/src/data_contract/associated_token/token_keeps_history_rules/v0/accessors.rs
(2 hunks)packages/rs-dpp/src/data_contract/associated_token/token_keeps_history_rules/v0/mod.rs
(3 hunks)packages/rs-dpp/src/state_transition/mod.rs
(1 hunks)packages/rs-dpp/src/state_transition/proof_result.rs
(2 hunks)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/document_transition.rs
(2 hunks)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/mod.rs
(1 hunks)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/resolvers.rs
(3 hunks)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_direct_purchase_transition/mod.rs
(1 hunks)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_direct_purchase_transition/v0/mod.rs
(1 hunks)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_direct_purchase_transition/v0/v0_methods.rs
(1 hunks)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_direct_purchase_transition/v0_methods.rs
(1 hunks)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_direct_purchase_transition/validate_structure/mod.rs
(1 hunks)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_direct_purchase_transition/validate_structure/v0/mod.rs
(1 hunks)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_set_price_for_direct_purchase_transition/mod.rs
(1 hunks)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_set_price_for_direct_purchase_transition/v0/mod.rs
(1 hunks)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_set_price_for_direct_purchase_transition/v0/v0_methods.rs
(1 hunks)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_set_price_for_direct_purchase_transition/v0_methods.rs
(1 hunks)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_set_price_for_direct_purchase_transition/validate_structure/mod.rs
(1 hunks)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_set_price_for_direct_purchase_transition/validate_structure/v0/mod.rs
(1 hunks)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_transition.rs
(10 hunks)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_transition_action_type.rs
(4 hunks)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/mod.rs
(2 hunks)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/v1/mod.rs
(3 hunks)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/mod.rs
(1 hunks)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/resolvers/v0/mod.rs
(2 hunks)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v0_methods.rs
(4 hunks)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/validation/validate_basic_structure/v0/mod.rs
(2 hunks)packages/rs-dpp/src/tokens/mod.rs
(1 hunks)packages/rs-dpp/src/tokens/token_event.rs
(5 hunks)packages/rs-dpp/src/tokens/token_pricing_schedule.rs
(1 hunks)packages/rs-drive-abci/src/execution/platform_events/protocol_upgrade/perform_events_on_first_block_of_protocol_change/v0/mod.rs
(2 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/action_validation/token/mod.rs
(1 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/action_validation/token/token_direct_purchase_transition_action/mod.rs
(1 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/action_validation/token/token_direct_purchase_transition_action/state_v0/mod.rs
(1 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/action_validation/token/token_set_price_for_direct_purchase_transition_action/mod.rs
(1 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/action_validation/token/token_set_price_for_direct_purchase_transition_action/state_v0/mod.rs
(1 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/state/v0/mod.rs
(2 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/mod.rs
(1 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/direct_selling/mod.rs
(1 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/mod.rs
(1 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/transformer/v0/mod.rs
(2 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/mod.rs
(2 hunks)packages/rs-drive-abci/src/query/group_queries/group_actions/v0/mod.rs
(3 hunks)packages/rs-drive/src/drive/initialization/v1/mod.rs
(2 hunks)packages/rs-drive/src/drive/tokens/direct_purchase/mod.rs
(1 hunks)packages/rs-drive/src/drive/tokens/direct_purchase/queries.rs
(1 hunks)packages/rs-drive/src/drive/tokens/direct_purchase/set_direct_purchase_price/mod.rs
(1 hunks)packages/rs-drive/src/drive/tokens/direct_purchase/set_direct_purchase_price/v0/mod.rs
(1 hunks)packages/rs-drive/src/drive/tokens/estimated_costs/for_token_direct_selling_prices/mod.rs
(1 hunks)packages/rs-drive/src/drive/tokens/estimated_costs/for_token_direct_selling_prices/v0/mod.rs
(1 hunks)packages/rs-drive/src/drive/tokens/estimated_costs/mod.rs
(1 hunks)packages/rs-drive/src/drive/tokens/mod.rs
(1 hunks)packages/rs-drive/src/drive/tokens/paths.rs
(2 hunks)packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/mod.rs
(1 hunks)packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_direct_purchase_transition.rs
(1 hunks)packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_set_price_for_direct_purchase_transition.rs
(1 hunks)packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_transition.rs
(3 hunks)packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/mod.rs
(7 hunks)packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_direct_purchase_transition_action/mod.rs
(1 hunks)packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_direct_purchase_transition_action/transformer.rs
(1 hunks)packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_direct_purchase_transition_action/v0/mod.rs
(1 hunks)packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_direct_purchase_transition_action/v0/transformer.rs
(1 hunks)packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_set_price_for_direct_purchase_transition_action/mod.rs
(1 hunks)packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_set_price_for_direct_purchase_transition_action/transformer.rs
(1 hunks)packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_set_price_for_direct_purchase_transition_action/v0/mod.rs
(1 hunks)packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_set_price_for_direct_purchase_transition_action/v0/transformer.rs
(1 hunks)packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_transition_action_type.rs
(1 hunks)packages/rs-drive/src/util/batch/drive_op_batch/token.rs
(3 hunks)packages/rs-drive/src/util/batch/grovedb_op_batch/mod.rs
(3 hunks)packages/rs-drive/src/verify/state_transition/verify_state_transition_was_executed_with_proof/v0/mod.rs
(2 hunks)packages/rs-drive/src/verify/tokens/mod.rs
(1 hunks)packages/rs-drive/src/verify/tokens/verify_token_direct_selling_price/mod.rs
(1 hunks)packages/rs-drive/src/verify/tokens/verify_token_direct_selling_price/v0/mod.rs
(1 hunks)packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/mod.rs
(1 hunks)packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v1.rs
(1 hunks)packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v2.rs
(1 hunks)packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v3.rs
(1 hunks)packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v4.rs
(1 hunks)packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v5.rs
(1 hunks)packages/rs-platform-version/src/version/drive_versions/drive_identity_method_versions/mod.rs
(1 hunks)packages/rs-platform-version/src/version/drive_versions/drive_identity_method_versions/v1.rs
(1 hunks)packages/rs-platform-version/src/version/drive_versions/drive_state_transition_method_versions/mod.rs
(1 hunks)packages/rs-platform-version/src/version/drive_versions/drive_state_transition_method_versions/v1.rs
(1 hunks)packages/rs-platform-version/src/version/drive_versions/drive_verify_method_versions/mod.rs
(1 hunks)packages/rs-platform-version/src/version/drive_versions/drive_verify_method_versions/v1.rs
(1 hunks)packages/token-history-contract/schema/v1/token-history-contract-documents.json
(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
packages/rs-drive/src/drive/tokens/estimated_costs/for_token_direct_selling_prices/mod.rs (1)
Learnt from: QuantumExplorer
PR: dashpay/platform#2431
File: packages/rs-drive/src/drive/tokens/system/fetch_token_total_aggregated_identity_balances/v0/mod.rs:42-50
Timestamp: 2025-04-06T00:27:16.216Z
Learning: The `add_estimation_costs_for_token_balances` method in Drive is a properly implemented estimation logic method used for token balance operations, and the warning about incomplete estimation logic was incorrect.
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (token-history-contract) / Tests
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (token-history-contract) / Unused dependencies
- GitHub Check: Rust packages (drive-abci) / Unused dependencies
- GitHub Check: Rust packages (dash-sdk) / Check each feature
- GitHub Check: Rust packages (dash-sdk) / Formatting
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (drive-abci) / Linting
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (drive-abci) / 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 Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: Rust crates security audit
🔇 Additional comments (188)
packages/rs-dpp/src/tokens/mod.rs (1)
16-16
: New module added for token pricing schedules.This adds a public module for handling token pricing schedules, which will likely support the token selling feature. The addition follows the same pattern as other module declarations in this file.
packages/rs-drive/src/verify/tokens/mod.rs (1)
4-4
: New verification module for token direct selling prices added.This module adds functionality to verify token direct selling prices, which is consistent with other verification modules in this file. All follow the same naming pattern prefixed with
verify_token_
.packages/rs-platform-version/src/version/drive_versions/drive_verify_method_versions/v1.rs (1)
49-49
: Version tracking added for token direct selling price verification.The new field adds version tracking for the token direct selling price verification method, initialized to 0. This follows the established pattern for versioning new methods in the platform.
packages/rs-drive/src/drive/tokens/mod.rs (1)
51-51
: Missing conditional compilation flag for server feature.Most operation-handling modules in this file are conditionally compiled with the
#[cfg(feature = "server")]
attribute. Verify if this module should also be server-specific.#!/bin/bash # Check if other files in the direct_purchase module use server-specific features fd direct_purchase --type f --exec cat {} \; | grep -E "feature|server" # Check if similar modules consistently use the server feature grep -A 3 -B 3 "mod [a-z_]" packages/rs-drive/src/drive/tokens/mod.rspackages/rs-platform-version/src/version/drive_versions/drive_identity_method_versions/mod.rs (1)
76-76
: New field for token selling prices added correctly.The addition of
for_token_selling_prices: FeatureVersion
to theDriveIdentityCostEstimationMethodVersions
struct extends the identity cost estimation capabilities to support token selling price operations, which aligns with the pull request's focus on selling tokens.packages/rs-platform-version/src/version/drive_versions/drive_verify_method_versions/mod.rs (1)
67-67
: New verification method for token direct selling price added correctly.The addition of
verify_token_direct_selling_price: FeatureVersion
to theDriveVerifyTokenMethodVersions
struct enables validation capabilities for direct token selling prices, which is consistent with the PR's objective of implementing token selling functionality.packages/rs-platform-version/src/version/drive_versions/drive_identity_method_versions/v1.rs (1)
137-137
: Initialization of new token selling prices field looks good.The
for_token_selling_prices
field is correctly initialized to version0
in theDRIVE_IDENTITY_METHOD_VERSIONS_V1
constant, consistent with the versioning pattern used for other fields in this structure.packages/rs-drive/src/drive/tokens/estimated_costs/mod.rs (1)
21-23
: Module for direct selling prices added with proper documentation.The new module
for_token_direct_selling_prices
with appropriate documentation comment follows the established pattern for module declarations in this file. This addition supports the token selling functionality being implemented in this PR.packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/mod.rs (1)
27-27
: Import aligned with new direct selling test functionality.This import adds the
process_test_state_transition
function which is used in the newly added test cases for direct selling functionality. This function is essential for handling state transitions during these tests, ensuring proper validation of direct purchase transactions.packages/rs-dpp/src/state_transition/mod.rs (1)
365-370
: Support for new token transition types.The implementation correctly extends the
name
method with handlers for two new token transition types:DirectPurchase
andSetPriceForDirectPurchase
. These additions are necessary to support the token selling functionality introduced in this PR.The naming convention matches existing patterns, which maintains code consistency.
packages/rs-dpp/src/state_transition/proof_result.rs (2)
7-7
: New import for token pricing functionality.This import properly introduces the
TokenPricingSchedule
type which is needed for the new enum variant.
19-19
: New proof result for token pricing verification.This addition expands the
StateTransitionProofResult
enum to include verification of token pricing schedules. TheVerifiedTokenPricingSchedule
variant appropriately stores both the token identifier and its optional pricing schedule, which is necessary for the direct purchase functionality being implemented.packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/mod.rs (1)
5-5
: Module declarations for direct purchase operations.These module declarations properly integrate the new direct purchase and pricing functionality into the existing token operations framework. The naming follows the established pattern, maintaining consistency with the existing codebase structure.
Also applies to: 9-9
packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_transition_action_type.rs (1)
22-27
: Implementation follows consistent pattern for new token transition typesThe additions of
DirectPurchaseAction
andSetPriceForDirectPurchaseAction
to the match statement follow the existing pattern, maintaining consistency with other token transition action types. The implementation looks correct and properly exposes the new action types through theTokenTransitionActionTypeGetter
trait.packages/rs-drive/src/drive/tokens/direct_purchase/mod.rs (1)
1-2
: New modules for direct purchase functionalityThe addition of the
queries
andset_direct_purchase_price
modules establishes the foundation for the token direct purchase feature, following the modular structure of the codebase. These modules will contain the necessary functionality for retrieving and setting direct purchase prices.#!/bin/bash # Verify that the implementation files for the new modules exist echo "Checking for implementation files..." find packages/rs-drive/src/drive/tokens/direct_purchase/ -name "*.rs"packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/action_validation/token/mod.rs (1)
6-10
: New validation modules for direct purchase actionsThe addition of the validation modules for
token_direct_purchase_transition_action
andtoken_set_price_for_direct_purchase_transition_action
completes the integration of the new token features by ensuring proper validation. This follows the established pattern for action validation in the codebase.#!/bin/bash # Verify that the implementation files for the new validation modules exist echo "Checking for implementation files for the new validation modules..." find packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/action_validation/token/ \ -name "token_direct_purchase_transition_action.rs" -o \ -name "token_set_price_for_direct_purchase_transition_action.rs"packages/rs-platform-version/src/version/drive_versions/drive_state_transition_method_versions/v1.rs (1)
44-45
: New token transitions added for direct purchase functionalityThe addition of
token_direct_purchase_transition
andtoken_set_price_for_direct_purchase_transition
to theDRIVE_STATE_TRANSITION_METHOD_VERSIONS_V1
constant properly extends the platform to support direct token purchases and price setting capabilities. Both transitions are initialized to version 0.packages/rs-platform-version/src/version/drive_versions/drive_state_transition_method_versions/mod.rs (1)
42-43
: New feature version fields added for token direct purchase functionalityThe addition of these fields to the
DriveStateTransitionActionConvertToHighLevelOperationsMethodVersions
struct appropriately complements the changes in the v1.rs file, establishing feature versioning for the new token transitions. These changes are consistent with the existing structure and naming conventions.packages/rs-dpp/src/data_contract/associated_token/token_distribution_rules/v0/mod.rs (2)
31-32
: New change control rules added for direct purchase pricingThe addition of
change_direct_purchase_pricing_rules
with a default value that restricts changes (usingAuthorizedActionTakers::NoOne
) provides a secure baseline for controlling who can modify direct purchase pricing.
80-80
: Display implementation updated for new fieldThe Display implementation has been correctly updated to include the new field in the formatted output.
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/mod.rs (2)
22-22
: New module added for token direct purchase transitionThe addition of the
token_direct_purchase_transition
module is consistent with the other changes and follows the project's module organization pattern.
26-26
: New module added for token price setting transitionThe addition of the
token_set_price_for_direct_purchase_transition
module is consistent with the other changes and follows the project's module organization pattern.packages/rs-drive/src/util/batch/grovedb_op_batch/mod.rs (3)
10-10
: Correctly added new TokenDirectSellPriceRoot variant to importsThe import statement is properly updated to include the new TokenDirectSellPriceRoot variant, maintaining alphabetical ordering with other token-related imports.
60-60
: New TokenDirectSellPriceRoot enum variant correctly addedThe TokenDirectSellPriceRoot variant is properly added to the KnownPath enum as a Level 2 path, following the established pattern for token-related paths.
243-245
: Correctly implemented readable_key_info handler for TOKEN_DIRECT_SELL_PRICE_KEYThe pattern match for TOKEN_DIRECT_SELL_PRICE_KEY is implemented consistently with other token key handlers, properly returning a formatted string and the associated TokenDirectSellPriceRoot path.
packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v1.rs (1)
154-157
: Added validation fields for token direct purchase functionalityThe four new validation fields are correctly added to support the token direct purchase and price setting functionality:
- Structure validation for token direct purchase transitions
- State validation for token direct purchase transitions
- Structure validation for setting prices for direct purchases
- State validation for setting prices for direct purchases
All fields are properly initialized to 0, consistent with the pattern for new validators in this version configuration.
packages/rs-drive-abci/src/execution/platform_events/protocol_upgrade/perform_events_on_first_block_of_protocol_change/v0/mod.rs (2)
28-31
: Updated token path imports to include TOKEN_DIRECT_SELL_PRICE_KEYThe imports are properly updated to include TOKEN_DIRECT_SELL_PRICE_KEY along with other token-related constants while maintaining alphabetical ordering.
405-412
: Added initialization for token direct sell price treeThe code correctly initializes an empty tree for storing token direct sell prices during the transition to version 9, following the same pattern as other token-related tree initializations in this function.
packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v2.rs (1)
154-157
: Added validation fields for token direct purchase functionalityThe same four validation fields added to v1 are correctly added to v2 configuration:
- Structure validation for token direct purchase transitions
- State validation for token direct purchase transitions
- Structure validation for setting prices for direct purchases
- State validation for setting prices for direct purchases
All fields are properly initialized to 0, maintaining consistency across version configurations.
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_set_price_for_direct_purchase_transition/v0/mod.rs (1)
1-56
: Well-structured implementation for token direct purchase pricingThe
TokenSetPriceForDirectPurchaseTransitionV0
struct looks well-designed with appropriate fields for the base transition, optional pricing schedule, and optional public note. The implementation includes proper serialization support and a clean Display trait implementation.A few observations:
- The documentation comment on line 27 nicely explains that setting price to None makes the token no longer purchasable
- The Display implementation properly handles the optional fields
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_direct_purchase_transition/v0/mod.rs (1)
1-39
: Clean implementation for token direct purchase transactionThe
TokenDirectPurchaseTransitionV0
struct is well-designed with fields for the base transition, token count, and total agreed price. The implementation follows the same pattern as other transition structures in the codebase.The documentation comments effectively explain each field's purpose, especially noting that
total_agreed_price
is what the user will pay.packages/rs-dpp/src/data_contract/associated_token/token_keeps_history_rules/v0/mod.rs (2)
28-35
: Appropriate extensions to history tracking capabilitiesThe addition of
keeps_direct_pricing_history
andkeeps_direct_purchase_history
fields is a logical extension to the existing history tracking functionality. Each field includes proper serde attributes to default to true.
45-46
: Default implementation correctly updatedThe Default implementation has been properly updated to initialize the new fields to true, maintaining consistency with the other history tracking fields.
packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v4.rs (1)
157-160
: Consistent validation framework extensionThe addition of validation fields for direct purchase transitions and price setting transitions properly extends the validation framework to support the new functionality.
All fields are initialized to 0, which follows the pattern of other validation fields in this structure, maintaining consistency with the existing validation system.
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_set_price_for_direct_purchase_transition/mod.rs (1)
1-26
: Implementation looks solid and follows the established pattern.The new
TokenSetPriceForDirectPurchaseTransition
enum follows the versioning pattern used throughout the codebase, with proper derivations for serialization and a default implementation.packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v3.rs (1)
154-157
: Added new validation fields with appropriate version numbers.These new validation fields for direct purchase transitions properly follow the same pattern as other token transition validations, with all fields initialized to version 0.
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_transition_action_type.rs (4)
17-18
: New action types added toTokenTransitionActionType
enum.The
DirectPurchase
andSetPriceForDirectPurchase
variants align with the new functionality for token selling.
33-34
: Display implementation updated for new action types.The formatting is consistent with existing patterns.
56-59
: Action type matching implemented forSetPriceForDirectPurchase
.The implementation matches the pattern used for other action types.
82-85
: String conversion for new action types implemented correctly.Both snake_case (
direct_purchase
) and camelCase (directPurchase
) formats are supported, following the existing pattern.packages/rs-drive/src/drive/tokens/paths.rs (2)
90-104
: New path functions for token direct selling implement correct patterns.The implementation of
token_direct_selling_root_path()
andtoken_direct_selling_root_path_vec()
follows the established pattern for other token-related paths.
18-19
: Non-consistent value chosen forTOKEN_DIRECT_SELL_PRICE_KEY
.Consider whether
92
is the intended value forTOKEN_DIRECT_SELL_PRICE_KEY
. Most other token-related keys in this file use values that are powers of 2 (64, 128, 192) or related values (32). Using a different pattern might be intentional but could be worth verifying.#!/bin/bash # Find all token key value constants in the code to check the pattern rg --type rust "pub const TOKEN_.*_KEY: u8 = \d+;" --no-filename | sortpackages/rs-drive/src/util/batch/drive_op_batch/token.rs (3)
16-16
: LGTM: Import added to support direct purchase pricing feature.The import of
TokenPricingSchedule
is correctly added to support the new token direct purchase pricing functionality.
119-126
: LGTM: Well-structured token pricing enum variant.This new variant properly extends the
TokenOperationType
enum to support setting prices for direct token purchases. The documentation is clear, and theNone
option for the price field appropriately indicates when a token is not for sale.
301-309
: LGTM: Implementation follows existing patterns.The implementation of the
TokenSetPriceForDirectPurchase
match arm correctly follows the established pattern in this file:
- It extracts the token ID and converts it to a buffer
- It passes the price and other standard parameters to the appropriate Drive method
- It returns the batch operations
The implementation is consistent with other token operations in this module.
packages/rs-drive/src/drive/initialization/v1/mod.rs (3)
9-12
: LGTM: Import updated to include new token direct sell price key.The import list has been properly updated to include the
TOKEN_DIRECT_SELL_PRICE_KEY
constant.
86-87
: Minor optimization: Added clone to prevent ownership issue.Good change to add
.clone()
since thetokens_root_path
variable is now used multiple times after this point. This properly handles Rust's ownership rules.
91-95
: LGTM: Initialization code for direct sell price storage.This addition properly initializes the empty tree structure needed to store token direct sell prices, following the same pattern as other token-related initializations in this function.
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_direct_purchase_transition/mod.rs (4)
1-4
: LGTM: Well-structured module organization.The module structure follows the project's conventions with separate modules for version-specific implementations, methods, and validation. This organization supports future extensibility.
5-10
: LGTM: Appropriate imports and re-exports.The imports include all necessary dependencies, and the conditional import for serialization features is properly implemented with the feature flag. The re-export of the V0 implementation makes it accessible to consumers of this module.
11-19
: LGTM: Well-defined versioned transition type.The
TokenDirectPurchaseTransition
enum follows the project's versioning pattern, allowing for future versions while maintaining backward compatibility. The derivation of appropriate traits is complete.
21-26
: LGTM: Default implementation for versioned transition.The default implementation appropriately initializes the enum to the V0 variant. The comment on line 24 serves as a useful reminder that only V0 is currently supported.
packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_direct_purchase_transition_action/mod.rs (3)
1-13
: LGTM: Module setup and imports.The module organization with imports and submodules follows the project's pattern for transition actions. The transformer module is public while implementation details are kept private, which is good module design.
14-19
: LGTM: Version-aware transition action enum.The
TokenDirectPurchaseTransitionAction
enum follows the project's versioning pattern, supporting future extensibility.
20-55
: LGTM: Comprehensive accessor implementation.The implementation of
TokenDirectPurchaseTransitionActionAccessorsV0
provides a complete set of accessors for the underlying data structure:
- Read access to base transition action (both reference and owned versions)
- Read and write access to token count
- Read and write access to total agreed price
This implementation follows the pattern used throughout the codebase for accessor traits, ensuring type-safe interaction with the action.
packages/rs-dpp/src/tokens/token_event.rs (4)
9-9
: Appropriately added new imports and type aliases.The addition of the
Credits
import,TokenPricingSchedule
import, and thePurchaserIdentifier
type alias is well-structured and follows the existing code patterns.Also applies to: 28-28, 34-34
61-62
: New TokenEvent variants correctly implemented.The two new variants,
SetPriceForDirectPurchase
andDirectPurchase
, are properly implemented with appropriate parameters. TheSetPriceForDirectPurchase
correctly uses an optional pricing schedule with a public note, whileDirectPurchase
captures both token amount and credits cost.
77-78
: Document type name mappings appropriately defined.The implementation correctly maps the new token event variants to their respective document type names: "directPurchase" and "setPriceForDirectPurchase".
256-282
: Document creation for new token events handled properly.The implementation for building historical documents for
SetPriceForDirectPurchase
andDirectPurchase
events is correct. The code properly:
- Handles optional pricing schedule serialization
- Correctly includes the token ID in both event types
- Properly manages the optional note for the pricing event
- Captures both token amount and purchase cost for direct purchases
This follows the established pattern for other token events.
packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_transition.rs (3)
16-17
: Appropriate import additions for new token transition actions.The new imports for
TokenDirectPurchaseTransitionActionAccessorsV0
andTokenSetPriceForDirectPurchaseTransitionActionAccessorsV0
follow the established pattern and are necessary for accessing the properties of the new action types.
62-66
: Handling of new token transition actions in batch operations.The implementation correctly extends the
into_high_level_batch_drive_operations
method to handle the newDirectPurchaseAction
andSetPriceForDirectPurchaseAction
variants, following the same pattern as the existing variants.
125-136
: Associated token event mapping properly implemented.The implementation correctly maps the new transition actions to their corresponding token events. For
DirectPurchaseAction
, it properly extracts the token count and total agreed price. ForSetPriceForDirectPurchaseAction
, it correctly extracts the price schedule and public note, handling optional values appropriately.packages/rs-dpp/src/data_contract/associated_token/token_distribution_rules/accessors/v0/mod.rs (2)
44-48
: New getter methods for direct purchase pricing rules.The addition of
change_direct_purchase_pricing_rules
andchange_direct_purchase_pricing_rules_mut
methods to theTokenDistributionRulesV0Getters
trait is appropriate. These methods follow the established pattern for accessing configuration rules in the system.
80-81
: New setter method for direct purchase pricing rules.The addition of the
set_change_direct_purchase_pricing_rules
method to theTokenDistributionRulesV0Setters
trait is appropriate. The method signature follows the established pattern for rule setters in the system.packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/state/v0/mod.rs (2)
28-28
: Added imports for new token action validation.The imports for
TokenDirectPurchaseTransitionActionValidation
andTokenSetPriceForDirectPurchaseTransitionActionValidation
are correctly added, following the same pattern as other token action validation imports.Also applies to: 32-32
234-253
: New token action validation implemented consistently.The implementation adds validation handling for
DirectPurchaseAction
andSetPriceForDirectPurchaseAction
in thevalidate_state_v0
method. The code correctly follows the established pattern by:
- Calling the appropriate
validate_state
method on each action- Passing all required parameters consistently
- Properly handling the validation result
This ensures that the new token actions undergo the same validation process as existing actions.
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/validation/validate_basic_structure/v0/mod.rs (2)
30-31
: New token transition validators added correctly.The two new imports for token transition validation structures follow the established pattern for other token transition validators.
175-182
: New token transition validation cases integrated properly.The implementation for validating the two new token transition types follows the same pattern as existing token transitions, correctly calling their respective
validate_structure
methods with theplatform_version
parameter.packages/rs-drive/src/drive/tokens/direct_purchase/set_direct_purchase_price/v0/mod.rs (2)
1-11
: Imports look appropriate for token pricing operations.All necessary imports are included for implementing token direct purchase price operations.
12-49
: Implementation for setting direct purchase price operations looks correct.The implementation follows the established pattern for drive operations in this codebase:
- It properly handles both adding a new price (serializing and inserting) and removing an existing price (deleting)
- It correctly adds estimation costs when needed
- Error handling follows the codebase conventions
The method is correctly marked as
pub(super)
indicating it's only accessible from the parent module.packages/rs-drive/src/verify/state_transition/verify_state_transition_was_executed_with_proof/v0/mod.rs (2)
43-43
: Successfully updated imports to include the new verification result type.The import statement was properly updated to include the
VerifiedTokenPricingSchedule
variant for handling the new token transition verification results.
521-540
: SetPriceForDirectPurchase verification logic implemented correctly.The verification implementation for
SetPriceForDirectPurchase
properly:
- Uses historical query if the token keeps direct pricing history
- Verifies the token direct selling price otherwise, returning the proper result type
The implementation follows the same pattern as other token transitions and appears to be correct.
packages/rs-drive/src/drive/tokens/estimated_costs/for_token_direct_selling_prices/v0/mod.rs (6)
8-12
: Nice implementation of the token selling price path constantsThe code correctly utilizes the token path constants from the path module, which ensures consistency across the codebase. The inclusion of
DEFAULT_HASH_SIZE_U8
is appropriate for hash-based key sizes.
13-25
: Well-structured implementation block for DriveThe implementation follows the standard pattern used in other estimation methods within the codebase. The proper
pub(super)
visibility restricts access as needed.
26-51
: Excellent documentation for the estimation methodThe documentation is thorough and precisely explains the purpose, parameters, and structure of the estimations across the three layers. The hierarchical documentation with numbered points makes it easy to understand the cost model.
52-69
: LGTM: Correct implementation of top-level estimationThe top-level estimation correctly uses an empty path and sets up appropriate parameters for the tree structure. The comment explains the rationale behind the estimation values, which is helpful for future maintenance.
71-89
: LGTM: Proper token root path estimationThe token root path estimation appropriately configures the weights for different tree types, including sum trees and normal trees. The weights are balanced to reflect the expected usage patterns.
91-98
: LGTM: Well-defined direct selling path estimationThe direct selling path estimation uses appropriate parameters with
DEFAULT_HASH_SIZE_U8
for key size and reasonable assumptions about the number of items (8) and depth (10 levels).packages/rs-drive/src/drive/tokens/direct_purchase/queries.rs (3)
1-5
: LGTM: Appropriate imports for query functionalityThe imports are minimal and focused on what's needed for the query functionality. Good use of the path helper function
token_direct_selling_root_path_vec
.
8-21
: LGTM: Well-implemented bulk token price queryThe implementation correctly handles querying for multiple token prices by constructing a
PathQuery
with all token IDs. The use ofSizedQuery
with the token count is appropriate for performance optimization.
23-32
: LGTM: Efficient single token price query implementationThe single token query implementation follows the same pattern as the bulk query but is optimized for a single token. Good reuse of the path helper function.
packages/rs-drive-abci/src/query/group_queries/group_actions/v0/mod.rs (3)
7-11
: LGTM: Good organization of imports for pricing componentsThe updated imports correctly include all necessary types for handling direct purchase pricing events. The hierarchical organization of the imports makes it clear which components are being used.
200-222
: Well-implemented handler for SetPriceForDirectPurchase token eventThe implementation correctly translates the internal
TokenPricingSchedule
into the protobufPrice
type through a detailed mapping logic. The code handles both single price and variable price schedules appropriately.
223-225
: Efficient consolidation of non-relevant event typesGood approach to consolidate the handling of
Transfer
,DirectPurchase
, andClaim
events into a single return statement that results inNone
. This simplifies the code and makes it more maintainable.packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_direct_purchase_transition/v0_methods.rs (3)
1-7
: LGTM: Properly structured importsThe imports correctly include all necessary components for the implementation of token direct purchase transition methods. Good organization of imports with clear separation of concerns.
8-26
: LGTM: Correct implementation of TokenBaseTransitionAccessorsThe implementation follows the standard pattern for delegating trait methods to enum variants. All accessor methods (
base
,base_mut
, andset_base
) are correctly implemented to work with the V0 variant.
28-52
: LGTM: Well-structured TokenDirectPurchaseTransitionV0Methods implementationThe implementation correctly handles token count and total agreed price getters and setters. The consistent pattern of matching on the enum variant and delegating to the underlying implementation makes the code clean and maintainable.
packages/rs-dpp/src/data_contract/associated_token/token_configuration/v0/mod.rs (4)
78-79
: Feature added: Direct pricing and purchase history trackingThese new fields extend token history capabilities to track direct pricing changes and purchase events, complementing the existing transfer, freezing, minting, and burning history options.
110-116
: New control rules for direct purchase pricingThe addition of
change_direct_purchase_pricing_rules
with the most restrictive default permissions (AuthorizedActionTakers::NoOne
) ensures that direct purchase pricing changes are properly governed by authorization controls. This is a security best practice to prevent unauthorized price modifications.
187-188
: Consistent history tracking in most restrictive configurationThese fields are appropriately mirrored in the
default_most_restrictive
implementation, maintaining consistency with the default history tracking settings.
228-236
: Consistent pricing control in most restrictive configurationThe implementation correctly applies the same restrictive direct purchase pricing rules in the
default_most_restrictive
method, ensuring consistent security controls across all configuration creation methods.packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v5.rs (1)
158-161
: New validation fields for direct purchase transitionsThe addition of these four validation fields with initial versions set to 0 properly integrates the direct purchase and price setting functionality into the validation framework. This allows for distinct versioning of both structure and state validation for these new transition types, supporting future validation enhancements.
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_direct_purchase_transition/validate_structure/v0/mod.rs (1)
1-24
: Well-structured token purchase validation implementationThe implementation correctly validates token purchase amounts against the established maximum limit (
MAX_DISTRIBUTION_PARAM
), preventing excessive token purchases in a single transaction. The code follows established patterns:
- Uses trait-based version handling for future extensibility
- Implements proper error handling with specific error types
- Returns appropriate validation results based on the token count check
This approach ensures token purchase actions conform to system constraints while maintaining the codebase's architectural patterns.
packages/rs-drive/src/verify/tokens/verify_token_direct_selling_price/mod.rs (1)
1-67
: Well-documented token price verification implementationThe new
verify_token_direct_selling_price
method is thoroughly documented with comprehensive explanations of parameters, return values, and potential error cases. The implementation follows best practices by:
- Using version-specific delegation through the platform version object
- Providing clear error messages for version mismatches
- Following the established pattern for proof verification methods
- Using proper type definitions (
RootHash
,TokenPricingSchedule
)This implementation provides the foundation for verifying token selling prices through cryptographic proofs, a crucial component for secure token sales functionality.
packages/rs-dpp/src/data_contract/associated_token/token_distribution_rules/accessors/mod.rs (2)
97-107
: Well-structured addition of direct purchase pricing rule accessors.These getter methods follow the established pattern in the codebase, correctly delegating to the inner V0 implementation through pattern matching. The methods provide both immutable and mutable access to the direct purchase pricing rules.
171-175
: LGTM! Setter implementation is consistent with existing patterns.The implementation for setting direct purchase pricing rules maintains the same pattern as other setters in this file, ensuring a consistent API design.
packages/rs-drive/src/verify/tokens/verify_token_direct_selling_price/v0/mod.rs (1)
1-54
: Robust implementation for token direct selling price verification.The implementation properly:
- Constructs the appropriate path query
- Handles both subset and full proof verification
- Correctly deserializes the result into a
TokenPricingSchedule
- Handles error cases with appropriate domain-specific errors
The method follows the same pattern as other verification methods in the codebase, maintaining consistency in the API design and error handling.
However, ensure there are unit tests that cover all the path conditions, especially:
- The case when proof verification succeeds and a valid pricing schedule exists
- The case when proof verification succeeds but no pricing schedule exists (line 42)
- Error cases for incorrect value sizes and wrong element counts
Are there corresponding tests for this new verification method?
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/v1/mod.rs (2)
216-234
: Well-defined transition method for changing direct purchase price.The method signature is comprehensive and follows the established pattern in the codebase with appropriate feature gating. The parameters cover all necessary aspects of changing a token's direct purchase price, including:
- Token identification (token_id, owner_id, data_contract_id, token_contract_position)
- New pricing information (token_pricing_schedule)
- Metadata and control information (public_note, using_group_info)
- Authentication and fees (identity_public_key, identity_contract_nonce, user_fee_increase, signer)
- Version information for feature compatibility
236-252
: Well-defined transition method for direct token purchase.The method signature is correctly structured for direct token purchases, including all necessary parameters:
- Token identification (token_id, owner_id, data_contract_id, token_contract_position)
- Purchase details (amount, agreed_total_cost)
- Authentication and fees (identity_public_key, identity_contract_nonce, user_fee_increase, signer)
- Version information for feature compatibility
Note that unlike other token operations, this method doesn't include parameters for notes (public_note, etc.) or group info. This appears intentional but should be verified if these are needed for direct purchases.
Is the omission of public_note and using_group_info parameters intentional for direct purchases, unlike other token operations?
packages/rs-dpp/src/data_contract/associated_token/token_keeps_history_rules/accessors/v0/mod.rs (2)
15-17
: Appropriate addition of history tracking getters for direct pricing and purchases.These methods follow the established pattern for history tracking getters in this trait, maintaining API consistency.
34-36
: Complete implementation of history tracking setters for direct pricing and purchases.The setter methods correctly complement the getters added above, maintaining a consistent API design. The documentation is clear and consistent with existing methods.
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/mod.rs (1)
24-32
: LGTM: New token transitions properly added to exported entitiesThe additions of
token_direct_purchase_transition
,TokenDirectPurchaseTransition
,token_set_price_for_direct_purchase_transition
, andTokenSetPriceForDirectPurchaseTransition
are properly integrated into the existing export list, maintaining the module's organizational structure.packages/rs-dpp/src/tokens/token_pricing_schedule.rs (2)
26-29
: Well-designed pricing schedule enum with two appropriate variantsThe
TokenPricingSchedule
enum provides a clean abstraction for token pricing with two logical variants - a simple flat price and a more complex price schedule based on token amounts.
43-61
: Good implementation of Display traitThe
Display
implementation provides clear and readable string representations for both pricing variants, making debugging and logging easier.packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/mod.rs (6)
26-30
: LGTM: New token transition action modules properly addedThe new modules for token direct purchase and price setting transitions are correctly included in the public interface.
57-58
: LGTM: Import statements properly addedImport statements for the new transition actions are correctly included.
81-84
: LGTM: New enum variants with descriptive commentsThe new variants for direct purchase and price setting are properly added to the
TokenTransitionAction
enum with clear, descriptive comments.
100-101
: LGTM: Base accessors correctly updatedThe
base()
andbase_owned()
methods are properly updated to handle the new token transition actions.Also applies to: 117-118
134-135
: LGTM: Historical document type names set appropriatelyNew historical document type names "directPurchase" and "directPricing" are well-chosen and consistent with the existing naming pattern.
193-198
: Verify history keeping rules implementationsThe code relies on
keeps_direct_purchase_history()
andkeeps_direct_pricing_history()
methods. Make sure these methods are properly implemented in the token configuration's history rules.#!/bin/bash # Check if the history keeping methods are properly implemented # First, find the files that might contain these methods echo "Searching for history keeping rules implementation..." rg -A 3 "keeps_direct_purchase_history|keeps_direct_pricing_history" --type rustpackages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_set_price_for_direct_purchase_transition_action/v0/mod.rs (1)
70-100
: LGTM: Trait implementation is correct and completeThe implementation of
TokenSetPriceForDirectPurchaseTransitionActionAccessorsV0
for the transition action struct is complete and correctly handles all required accessor methods.packages/rs-drive/src/drive/tokens/estimated_costs/for_token_direct_selling_prices/mod.rs (1)
32-54
: LGTM: Well-structured version handlingThe implementation follows the established pattern for cost estimation methods, with proper version checking and error handling.
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/resolvers.rs (3)
8-10
: LGTM: Properly updated importsThe imports have been correctly updated to include the new transition types for direct purchase functionality.
114-130
: LGTM: Well-implemented token direct purchase resolversThe implementation for
as_transition_token_direct_purchase
andas_transition_token_set_price_for_direct_purchase
follows the established pattern of other resolver methods.
234-250
: LGTM: Consistent implementation for reference typeThe implementation for
BatchedTransitionRef
correctly mirrors the implementation forBatchedTransition
, maintaining consistency across the codebase.packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/document_transition.rs (1)
123-131
: LGTM: Properly implemented token direct purchase resolversThe implementation correctly returns
None
for both methods sinceDocumentTransition
doesn't support token transitions.packages/token-history-contract/schema/v1/token-history-contract-documents.json (2)
763-812
: LGTM: Well-structured direct pricing document typeThe
directPricing
document type is well-defined with appropriate properties and constraints for tracking token pricing history.
813-908
: LGTM: Comprehensive direct purchase document typeThe
directPurchase
document type includes appropriate properties and multiple indices that will allow for efficient querying of purchase history.packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/mod.rs (1)
123-126
: Feature additions for token selling capabilities look goodThese new fields properly extend the validation capabilities to support direct token purchases and price setting, matching the established pattern for feature versioning. The naming follows the existing convention which enhances maintainability.
packages/rs-dpp/src/data_contract/associated_token/token_keeps_history_rules/v0/accessors.rs (2)
24-30
: Implementation of getter methods for history tracking is correctThese getters correctly expose the new direct pricing and purchase history tracking capabilities, following the existing pattern of the codebase.
51-57
: Implementation of setter methods for history tracking is correctThe setter methods properly allow the modification of the direct pricing and purchase history tracking flags, matching the existing pattern of the other history tracking settings.
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/direct_selling/mod.rs (2)
3-101
: Test for successful direct purchase is thorough and well structuredThis test properly validates the end-to-end flow of setting a token price and completing a purchase, with appropriate balance checks. The test creates the necessary platform setup, identities, and token contract before executing the transitions.
I especially like the validation of both token balance and credit balance after the purchase, which confirms the complete transaction flow works correctly.
103-199
: Error handling test for insufficient funds is comprehensiveThis test correctly validates that the system properly rejects purchases when the buyer has insufficient credits, and verifies both the error type and that the token balance remains unchanged. Good approach to test the negative case.
packages/rs-dpp/src/data_contract/associated_token/token_keeps_history_rules/accessors/mod.rs (2)
34-44
: Getter implementations for TokenKeepsHistoryRules enum are correctThese methods correctly implement the getters by delegating to the inner V0 struct, following the established pattern for version-based accessors.
73-83
: Setter implementations for TokenKeepsHistoryRules enum are correctThe setter implementations properly modify the underlying struct fields through the enum variant, maintaining consistency with the existing codebase patterns.
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_set_price_for_direct_purchase_transition/validate_structure/v0/mod.rs (2)
17-18
: Validation approach for price.The comment explains that there's no need to validate the numeric price because a very high price will simply discourage purchases, which is logically acceptable. However, ensure other components (e.g., external calls or UI constraints) handle extremely large or invalid prices gracefully.
19-29
: Public note length check.The code ensures
public_note
does not exceedMAX_TOKEN_NOTE_LEN
, returning an appropriate error if exceeded. This prevents potential memory bloat or malicious oversize inputs. Confirm that the other fields in the transition do not silently allow oversized values in adjacent modules.packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_set_price_for_direct_purchase_transition_action/mod.rs (2)
14-17
: Single-variant enum design check.Currently,
TokenSetPriceForDirectPurchaseTransitionAction
includes only aV0
variant. This design nicely anticipates forward-compatibility for future versions. No issues identified.
19-63
: Accessor methods are consistent.All getters and setters (
base
,price
,public_note
) appear to be straightforward and correct. They properly funnel requests to thev0
variant. No logical errors observed here.packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_set_price_for_direct_purchase_transition.rs (3)
35-45
: Efficient nonce update.The identity nonce update operation for each transition is clearly modeled. Ensure relevant tests validate different contract IDs and nonces to detect any oversights if multiple transitions happen concurrently or in rapid succession.
47-76
: Conditional group operation insertion.Storing the action in a group only if the transition is associated with group data makes sense. The usage of
TokenEvent::SetPriceForDirectPurchase(...)
is consistent with the event-driven approach. No issues found here.
78-96
: Optional historical logging of direct purchase pricing.This block conditionally appends a token history record if the token configuration supports direct pricing history. This is well-structured, and returning early for non-history tokens avoids extra overhead. Confirm test coverage for both historical and non-historical paths.
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/resolvers/v0/mod.rs (3)
5-5
: Import addition looks good.
No issues observed.
10-10
: Confirm ongoing usage ofTokenTransferTransition
.
It appears bothTokenSetPriceForDirectPurchaseTransition
andTokenTransferTransition
coexist. Verify thatTokenTransferTransition
is intentionally retained and not deprecated.
32-32
: Consistent addition of direct purchase transition resolver.
Implementation aligns well with existing trait patterns.packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_direct_purchase_transition.rs (2)
44-51
: Verify minting logic for direct purchase.
These lines mint new tokens to the purchaser. Confirm this matches your system’s token supply model (e.g., whether tokens should be minted or transferred from a finite pool).
52-68
: Balance updates and token history logging appear correct.
Subtracting the total agreed price from the purchaser’s balance and adding it to the contract owner, plus optionally logging a direct purchase event in history, aligns with typical purchase flows. No immediate correctness issues found.Also applies to: 70-85
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_direct_purchase_transition/validate_structure/mod.rs (3)
1-7
: Imports and module organization look good.All imported crates and modules appear consistent with the responsibilities of this file. No issues spotted.
9-14
: Trait definition is clear and extensible.Defining a dedicated trait for transition structure validation is a clean, modular approach. This pattern allows future extension (e.g., additional platform versions) while keeping the validation logic encapsulated.
16-36
: Version-specific validation pattern is well-structured.The
match
statement onplatform_version.drive_abci.validation_and_processing.state_transitions.batch_state_transition.token_direct_purchase_transition_structure_validation
is consistent with the approach used elsewhere. It is easy to extend if new validation versions are introduced.packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_set_price_for_direct_purchase_transition/validate_structure/mod.rs (3)
1-7
: Imports and module division are consistent.No issues found with the imports or the file structure; it mirrors the pattern seen in other transition validation modules.
9-14
: Trait declaration is concise and transparent.Exposing a separate trait for the set-price transition validation straightforwardly segregates logic for different token transition types. Good use of consistent naming.
16-38
: Version handling supports extensibility.Just like the direct purchase transition, checking for known versions and returning a clear error for unknown ones is a sound approach. This encourages forward-compatible validation logic.
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/transformer/v0/mod.rs (4)
63-64
: New imports for direct purchase transitions are consistent.Bringing in
TokenDirectPurchaseTransitionAction
andTokenSetPriceForDirectPurchaseTransitionAction
aligns with the new transitions. No concerns.
607-616
: Claim transition logic follows established pattern.The
TokenTransition::Claim
arm properly calls thetry_from_borrowed_token_claim_transition_with_contract_lookup
method and applies the resulting fee. This is consistent with the approach used for other token transitions.
617-626
: Direct purchase transition logic integrated smoothly.Invoking
TokenDirectPurchaseTransitionAction::try_from_borrowed_token_direct_purchase_transition_with_contract_lookup
mirrors other transitions. Ensure that any deeper validations (e.g., user balance checks) are handled either here or in the invoked method.
627-636
: Set price for direct purchase transition logic is well-aligned.Similarly, the
TokenSetPriceForDirectPurchaseTransitionAction
is handled the same way as its counterparts. Confirm the underlying code correctly restricts or validates the new price, especially for corner cases like zero or negative prices, if applicable.packages/rs-drive/src/drive/tokens/direct_purchase/set_direct_purchase_price/mod.rs (1)
14-24
: Validate input parameters and edge cases.Currently, there's no explicit validation of parameters such as
token_id
length orblock_info
. Consider adding checks to ensure data is within acceptable bounds (for example, avoiding a null or zerotoken_id
) and ensuringblock_info
orprice
are valid. This helps prevent unexpected behavior if invalid parameters are passed.Please confirm all call sites pass valid
token_id
andprice
objects, or implement the necessary validations here. Below is a sample script to locate function calls totoken_set_direct_purchase_price
throughout the repository:#!/bin/bash rg -A6 "token_set_direct_purchase_price"packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/action_validation/token/token_set_price_for_direct_purchase_transition_action/state_v0/mod.rs (1)
27-74
: Confirm ownership and access rights.The
validate_state_v0
method checks group membership viavalidate_group_action
, relying on token configuration’smain_control_group
. Verify that ownership checks correctly reject unauthorized entities attempting to set token prices. For instance, ensure that partial or missing group membership data triggers meaningful errors, preventing accidental misconfiguration of direct purchase prices.Please confirm that test coverage includes scenarios where a non-owner attempts to set the price, verifying that the validation logic rejects such requests. If no such test exists, consider adding one.
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/action_validation/token/token_direct_purchase_transition_action/mod.rs (4)
1-12
: Imports and module references look consistent.All references from
dpp
anddrive
appear standard. No issues found with the import structure.
13-14
: Modular pattern is aligned with existing approach.Splitting out
state_v0
logic is consistent. Keep ensuring any incremental version updates remain self-contained.
15-25
: Validation trait design is coherent.Similar to the previous file, the trait is well-defined and consistent with the codebase’s architecture.
27-59
: Implementation defers to version-specific logic.Following the platform version to dispatch logic is correct. Future versions should be appended similarly. Everything else looks good.
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v0_methods.rs (5)
30-30
: New imports grouped logically.Adding
TokenDirectPurchaseTransition
andTokenSetPriceForDirectPurchaseTransition
to existing batch transitions is consistent with the existing structure.
74-74
: Import forTokenDirectPurchaseTransitionV0
recognized.No conflicts found; import usage is consistent with the newly introduced transition logic.
81-81
: Import forTokenSetPriceForDirectPurchaseTransitionV0
recognized.Implementation references appear correct.
92-92
: AddedTokenPricingSchedule
import.Well-aligned with new direct purchase price transitions.
1046-1091
:new_token_direct_purchase_transition
method is consistent.
- Constructs a
TokenDirectPurchaseTransition
with correct base fields and cost parameters.- Signs externally using the provided identity key.
- Returns a
StateTransition
.Implementation aligns well with the existing pattern for transition creation. Ensure test coverage to confirm expected transitions and signing behaviors.
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_transition.rs (9)
21-21
: Add missing import for the new token transition type
This addition cleanly introducesTokenSetPriceForDirectPurchaseTransition
.
36-38
: Introduce direct purchase transition imports
The newly added imports for direct purchase transitions are consistent with the existing token transition pattern.
80-85
: Add enum variants for direct purchase and set price transitions
These two new variants,DirectPurchase
andSetPriceForDirectPurchase
, follow the established naming conventions and maintain the enum’s clarity.
181-187
: Addas_transition_token_direct_purchase
method
This new method correctly parallels the otheras_transition_*
methods, returning a reference to the containedTokenDirectPurchaseTransition
.
189-197
: Addas_transition_token_set_price_for_direct_purchase
method
This complements the other optional accessor methods, ensuring a consistent approach for the new transition type.
260-261
: Access the base transition for direct purchase variants
IncludingDirectPurchase
andSetPriceForDirectPurchase
in thebase()
match is aligned with the existing pattern.
296-297
: No action ID for direct purchase
The transition returnsNone
forDirectPurchase
and a valid action ID forSetPriceForDirectPurchase
. Please confirm that skipping action IDs for direct purchases is intentional.
309-314
: Refinecan_calculate_action_id
logic
MarkingSetPriceForDirectPurchase
as capable andDirectPurchase
as incapable matches the preceding match arms.
349-350
: Introduce new historical document type names
Returning"directPurchase"
and"setPriceForDirectPurchase"
helps keep historical naming consistent with other transitions.packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/action_validation/token/token_direct_purchase_transition_action/state_v0/mod.rs (3)
1-18
: New file for token direct purchase transition state validation
The imports and module structure are consistent with the Drive/Platform architecture.
19-29
: Define trait for direct purchase transition state validation
TheTokenDirectPurchaseTransitionActionStateValidationV0
trait clearly outlines thevalidate_state_v0
method, preparing for versioned logic.
31-105
: Implementvalidate_state_v0
Validation ensures the base transition is valid, checks max supply bounds, handles potential overflow, and returns an appropriate error. This is thorough and aligns with the existing token supply checks.packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_direct_purchase_transition_action/transformer.rs (3)
1-15
: New file: transformer forTokenDirectPurchaseTransitionAction
All necessary imports for transformation logic appear intact, with consistent naming.
16-67
: Implementtry_from_token_direct_purchase_transition_with_contract_lookup
Converts the transition into an action, splitting by version (V0
) and delegating appropriately. This approach is aligned with multi-version handling.
69-118
: Implementtry_from_borrowed_token_direct_purchase_transition_with_contract_lookup
Mirrors the same logic as the owned variant, now for a borrowed reference. This helps avoid unnecessary data clones.packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_direct_purchase_transition_action/v0/transformer.rs (2)
1-18
: Imports and declarations look good
These imports, along with the initial crate references, appear necessary and follow Rust best practices.
19-43
: Comprehensive documentation
The documentation clearly explains the purpose, parameters, and return value for the upcoming function. This is highly appreciated for maintainability.packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_set_price_for_direct_purchase_transition_action/transformer.rs (2)
1-15
: Initial setup is straightforward
The imports and type references are consistent with the rest of the codebase. No changes needed.
16-67
: Version handling is future-proof
Using an enum match onTokenSetPriceForDirectPurchaseTransition::V0
is a good way to handle forward compatibility if future versions arise.packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_set_price_for_direct_purchase_transition/v0_methods.rs (3)
1-10
: Appropriate module and trait usage
The imports and module references are well-structured, aligning with the project's architecture.
11-29
: Clear base transition accessors
ImplementingTokenBaseTransitionAccessors
here cleanly exposes and updates the underlying transition data. This is consistent with typical Rust patterns for trait-based code.
65-71
: Multi-party action compliance
AllowedAsMultiPartyAction
is well-defined here, and the design is consistent with Rust’s pattern matching approach for versioned transitions.packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_set_price_for_direct_purchase_transition_action/v0/transformer.rs (1)
90-109
: Verify pronounced branching logic for invalid base transition.When
base_action_validation_result
is invalid, the code immediately creates aBumpIdentityDataContractNonceAction
and returns. Ensure that unusual partial-update scenarios are handled or intentionally unsupported. If partial updates might be needed in future, consider documenting or refactoring accordingly.packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_direct_purchase_transition/v0/v0_methods.rs (1)
31-47
: Extend unit test coverage.Implementing direct purchase logic typically involves business rules. Ensure these newly introduced methods (
token_count
,set_token_count
,total_agreed_price
,set_total_agreed_price
) are covered by unit tests, including boundary cases (e.g., zero token count) to avoid future regressions.packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_set_price_for_direct_purchase_transition/v0/v0_methods.rs (1)
65-76
: Check for collisions incalculate_action_id
.
calculate_action_id
relies ontoken_id
,owner_id
,identity_contract_nonce
, and the optional price reference to generate an identifier. Unless guaranteed by the higher-level design, ensure no collision or hashing pitfalls exist when multiple tokens share the same ID with different conditions. Consider additional fields or a different approach if collisions might be problematic.packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_direct_purchase_transition_action/v0/mod.rs (4)
12-20
: LGTM: Good structure with well-defined fields.The struct is properly defined with clear field types and purpose. Using
TokenAmount
for token count andCredits
for price is appropriate, and the comments explain the purpose of each field well.
23-66
: LGTM: Comprehensive accessor trait with well-defined methods.The trait provides a complete set of accessors for all fields and delegated properties, with appropriate methods for both reference and ownership scenarios. The design follows Rust best practices for trait design.
68-92
: LGTM: Clean implementation of the accessor trait.The implementation is straightforward and correctly implements all the required methods from the trait. The delegation to base methods and direct field access is done properly.
1-93
: Consider adding validation for token count and price values.While the implementation is functionally correct, consider whether there should be any validation for token count and agreed price values, such as minimum/maximum values or other business rules. This validation would likely be implemented in the transformer module.
#!/bin/bash # Look for validation logic in the transformer module or related files echo "Checking for validation in transformer module..." fd --glob "transformer.rs" packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_direct_purchase_transition_action/v0/ --exec cat {} echo "Checking for other validation related to token amounts and prices..." rg "validate.*token_count|validate.*price" packages/rs-drive/src/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/action_validation/token/token_set_price_for_direct_purchase_transition_action/mod.rs (1)
13-13
: Add basic unit test coverage forstate_v0
.This file references the
state_v0
module, but there's no clear indication of how its logic is being tested. Including tests will help confirm correctness and prevent regressions.To verify coverage, consider searching your tests for references to the
state_v0
module:#!/bin/bash # Description: Locate all test references to `state_v0`. rg 'state_v0'If there are no matches, please add a dedicated test module for it.
packages/rs-drive/src/drive/tokens/direct_purchase/set_direct_purchase_price/mod.rs
Show resolved
Hide resolved
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/mod.rs
Show resolved
Hide resolved
...cument/batch_transition/batched_transition/token_direct_purchase_transition/v0/v0_methods.rs
Show resolved
Hide resolved
...ransition/batched_transition/token_set_price_for_direct_purchase_transition/v0/v0_methods.rs
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.
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: 0
🧹 Nitpick comments (3)
packages/dapi-grpc/protos/platform/v0/platform.proto (3)
1423-1431
: New Request Message: GetTokenDirectPurchasePricesRequest
The new request message cleanly defines the parameters for retrieving direct purchase prices by accepting a list of token IDs and a boolean flagprove
. The oneof versioning approach is a good practice for future-proofing the API. Consider adding brief comments on the intended usage (e.g., what “prove” should trigger).
1433-1469
: New Response Message: GetTokenDirectPurchasePricesResponse
This new response message is well-structured and provides a comprehensive nested definition for pricing details including:
•PriceForQuantity
– Associating a price with a specific quantity.
•PricingSchedule
– Enabling variable pricing via repeated entries.
•Price
– Allowing either a fixed or a variable pricing schedule through a oneof field.
•TokenDirectPurchasePriceEntry
andTokenDirectPurchasePrices
– Grouping these entries.The use of a oneof
result
field to support either delivering the prices or a cryptographic proof is appropriate. It would be beneficial to include documentation comments on the intended units for prices and quantities or any boundary conditions expected.
1717-1733
: New Event Message: UpdateDirectPurchasePriceEvent
The addition of theUpdateDirectPurchasePriceEvent
message within the group events introduces a clear mechanism for updating a token’s direct purchase price. The nested structure—with its ownPriceForQuantity
andPricingSchedule
nested messages and a oneof field to select between a fixed or variable pricing model—mirrors the approach used in the response message, which promotes consistency across the protocol. Consider enhancing the inline documentation for each field (for example, clarifying the expected value units or constraints) to aid future maintainability and developer understanding.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/dapi-grpc/protos/platform/v0/platform.proto
(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (18)
- GitHub Check: Rust packages (wasm-dpp) / Linting
- GitHub Check: Rust packages (token-history-contract) / Formatting
- GitHub Check: Rust packages (token-history-contract) / Unused dependencies
- GitHub Check: Rust packages (dapi-grpc) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Formatting
- GitHub Check: Rust packages (dapi-grpc) / Tests
- GitHub Check: Rust packages (dapi-grpc) / Check each feature
- GitHub Check: Rust packages (dapi-grpc) / Linting
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dash-sdk) / Check each feature
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (rs-dapi-client) / Tests
- GitHub Check: Rust packages (rs-dapi-client) / 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 JS packages / Build JS
🔇 Additional comments (1)
packages/dapi-grpc/protos/platform/v0/platform.proto (1)
1772-1773
: Integration into TokenEvent: Inclusion of UpdateDirectPurchasePriceEvent
The inclusion ofUpdateDirectPurchasePriceEvent update_price
in the oneof field within theTokenEvent
message integrates the new direct purchase price update functionality into the overall token event flow. The numerical tags assigned (7 forTokenConfigUpdateEvent
and 8 forUpdateDirectPurchasePriceEvent
) appear consistent; just ensure that they remain unique and do not conflict with future oneof field additions.
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
🔭 Outside diff range comments (1)
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_transition.rs (1)
1-491
: 💡 Verification agent🧩 Analysis chain
Verify test coverage for new direct purchase functionality.
The implementation looks correct, but this significant feature addition should be thoroughly tested to ensure correct behavior in all scenarios, including edge cases.
🏁 Script executed:
#!/bin/bash # Check for tests covering the new direct purchase functionality echo "Searching for direct purchase test cases..." rg -t rust -i "test.*direct.*purchase|test.*set.*price" --glob "test_*.rs" | grep -v "#\[ignore\]" echo "Checking for transition method tests..." rg -A 10 -t rust "fn test.*token_direct_purchase" --glob "test_*.rs" rg -A 10 -t rust "fn test.*token_set_price" --glob "test_*.rs"Length of output: 422
Action Required: Add Missing Test Coverage for Direct Purchase Functionality
Although the core implementation for direct purchase appears correct, our search did not locate any tests covering:
- The
TokenDirectPurchaseTransition
behavior.- The
TokenSetPriceForDirectPurchaseTransition
functionality.Please add comprehensive tests—including edge cases—to ensure that both transitions work reliably and as expected.
🧹 Nitpick comments (23)
packages/rs-dpp/src/tokens/token_event.rs (2)
61-62
: New token event types properly implemented.Two new event types have been added to support direct purchase functionality:
ChangePriceForDirectPurchase
- for updating a token's price scheduleDirectPurchase
- for recording an actual purchaseConsider adding a purchaser identifier to the
DirectPurchase
variant to track who purchased the tokens, as you've defined thePurchaserIdentifier
type but it's not being used:- DirectPurchase(TokenAmount, Credits), + DirectPurchase(TokenAmount, Credits, PurchaserIdentifier),
274-282
: DirectPurchase historical document implementation is missing note field.The implementation for the
DirectPurchase
event type correctly includes the token ID, token amount, and purchase cost. However, unlike other event types, it doesn't include a note field.Consider adding a note field to maintain consistency with other event types:
- DirectPurchase(TokenAmount, Credits), + DirectPurchase(TokenAmount, Credits, TokenEventPublicNote),And update the implementation accordingly:
TokenEvent::DirectPurchase(amount, total_cost) => { - let properties = BTreeMap::from([ + let mut properties = BTreeMap::from([ ("tokenId".to_string(), token_id.into()), ("tokenAmount".to_string(), amount.into()), ("purchaseCost".to_string(), total_cost.into()), ]); + if let Some(note) = note { + properties.insert("note".to_string(), note.into()); + } + properties }packages/rs-drive/src/drive/tokens/paths.rs (3)
18-19
: Align "buy" vs. "sell" terminology for clarity.Currently, the docstring suggests "directly buying," while the constant name is
TOKEN_DIRECT_SELL_PRICE_KEY
. This could be confusing. Unifying the naming (e.g., consistently using either "purchase" or "sell") would improve clarity.For example:
-/// Key that sets the pricing schedule for directly buying the token. -pub const TOKEN_DIRECT_SELL_PRICE_KEY: u8 = 92; +/// Key that sets the pricing schedule for directly purchasing the token. +pub const TOKEN_DIRECT_PURCHASE_PRICE_KEY: u8 = 92;
90-96
: Unify doc comment with function name.The doc mentions a "direct selling price tree," while the function name references "purchase." Consider standardizing one term—e.g., "purchase" or "sell"—across both to avoid confusion.
-/// The root path of token direct selling price tree -pub fn token_direct_purchase_root_path() -> [&'static [u8]; 2] { +/// The root path of token direct purchase price tree +pub fn token_direct_purchase_root_path() -> [&'static [u8]; 2] {
98-104
: Maintain consistency in naming.Here as well, the doc comment says "selling price tree," whereas the function name uses "purchase." Use a single terminology for clarity across the codebase.
-/// The root path of token direct selling price tree -pub fn token_direct_purchase_root_path_vec() -> Vec<Vec<u8>> { +/// The root path of token direct purchase price tree +pub fn token_direct_purchase_root_path_vec() -> Vec<Vec<u8>> {packages/rs-drive/src/drive/tokens/direct_purchase/mod.rs (1)
1-3
: Consider standardizing module names.These modules are neatly separated, but the folder is named
direct_purchase
while the constant referencesTOKEN_DIRECT_SELL_PRICE_KEY
. Maintaining a single naming approach (all about “purchase” or “sell”) reduces confusion.packages/rs-drive/src/drive/tokens/direct_purchase/fetch_token_direct_purchase_price/mod.rs (3)
22-25
: Correct the return type reference in the doc comment.The doc block states:
/// * `Result<Option<TokenStatus>, Error>` - The token direct_purchase_price if successful, or an error.
However, the function returns
Result<Option<TokenPricingSchedule>, Error>
. Update the doc to match the actual return type.-/// * `Result<Option<TokenStatus>, Error>` - The token direct_purchase_price if successful, or an error. +/// * `Result<Option<TokenPricingSchedule>, Error>` - The token direct_purchase_price if successful, or an error.
47-55
: Expand the doc comment to mention the FeeResult in the return.
fetch_token_direct_purchase_price_with_costs
returns a tuple of(Option<TokenPricingSchedule>, FeeResult)
within aResult
. The current doc comment does not reflect that it also returns the fee result./// Fetches the Token direct_purchase_price with costs (if `apply = true`) /// and returns associated fee result. +/// +/// # Returns +/// +/// * `Result<(Option<TokenPricingSchedule>, FeeResult), Error>` — The token direct_purchase_price +/// and the associated fee, if successful, or an error.
90-91
: Update doc comment to align with actual return type.The doc block references
Result<Option<TokenStatus>, Error>
again, but the function returnsResult<Option<TokenPricingSchedule>, Error>
.-/// * `Result<Option<TokenStatus>, Error>` - The token info of the Identity if successful, or an error. +/// * `Result<Option<TokenPricingSchedule>, Error>` - The direct purchase price schedule if successful, or an error.packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/direct_selling/mod.rs (4)
107-122
: Consider making balance assertions more explicitThe balance assertion uses a magic number that's hard to understand. Consider calculating the expected value explicitly to make the test more maintainable.
- assert_eq!(buyer_credit_balance, Some(699_868_054_220)); // 10.0 - 3.0 spent - fees =~ 7 dash left + // Calculate expected balance: Initial balance - purchase cost - fees + let initial_balance = dash_to_credits!(10.0); + let purchase_cost = dash_to_credits!(3.0); + let expected_fees = dash_to_credits!(0.0013); // Approximate fees based on the execution + let expected_balance = initial_balance - purchase_cost - expected_fees; + assert!( + (buyer_credit_balance.unwrap_or(0) - expected_balance).abs() < dash_to_credits!(0.001), + "Balance {buyer_credit_balance:?} should be close to expected {expected_balance}" + );
125-158
: Consider extracting common setup codeThere's significant duplication in the test setup across all three tests. Consider extracting a helper function for the common setup of identities and token contract to improve maintainability.
+ fn setup_token_contract_with_pricing( + platform: &mut TestPlatform, + seller_balance: u64, + buyer_balance: u64, + ) -> ( + Identity, // seller + IdentitySigner, // seller_signer + IdentityPublicKey, // seller_key + Identity, // buyer + IdentitySigner, // buyer_signer + IdentityPublicKey, // buyer_key + DataContract, // contract + Identifier, // token_id + PlatformVersion, // platform_version + ) { + let platform_version = PlatformVersion::latest(); + let mut rng = StdRng::seed_from_u64(12345); + + let (seller, seller_signer, seller_key) = + setup_identity(platform, rng.gen(), seller_balance); + let (buyer, buyer_signer, buyer_key) = + setup_identity(platform, rng.gen(), buyer_balance); + + let (contract, token_id) = create_token_contract_with_owner_identity( + platform, + seller.id(), + Some(|token_configuration: &mut TokenConfiguration| { + token_configuration + .distribution_rules_mut() + .set_change_direct_purchase_pricing_rules(ChangeControlRules::V0( + ChangeControlRulesV0 { + authorized_to_make_change: AuthorizedActionTakers::ContractOwner, + admin_action_takers: AuthorizedActionTakers::NoOne, + changing_authorized_action_takers_to_no_one_allowed: false, + changing_admin_action_takers_to_no_one_allowed: false, + self_changing_admin_action_takers_allowed: false, + }, + )); + }), + None, + None, + platform_version, + ); + + ( + seller, + seller_signer, + seller_key, + buyer, + buyer_signer, + buyer_key, + contract, + token_id, + platform_version, + ) + }Then use this helper function in your test:
#[test] fn test_direct_purchase_single_price_not_paying_full_price() { let mut platform = TestPlatformBuilder::new() .with_latest_protocol_version() .build_with_mock_rpc() .set_genesis_state(); let ( seller, seller_signer, seller_key, buyer, buyer_signer, buyer_key, contract, token_id, platform_version, ) = setup_token_contract_with_pricing( &mut platform, dash_to_credits!(1.0), dash_to_credits!(10.0), ); // Continue with the test... }
317-333
: Consider clarifying magic numbersThe nonce values (like
2
in line 325) appear several times throughout the code without explanation. Consider using named constants to explain what these values represent.+ // Constants for commonly used values + const DEFAULT_IDENTITY_NONCE: IdentityNonce = 2; + const DEFAULT_USER_FEE_INCREASE: UserFeeIncrease = 0; let purchase_transition = BatchTransition::new_token_direct_purchase_transition( token_id, buyer.id(), contract.id(), 0, 1, // Buying 1 token dash_to_credits!(1.0), // Agreed price per token &buyer_key, - 2, - 0, + DEFAULT_IDENTITY_NONCE, + DEFAULT_USER_FEE_INCREASE, &buyer_signer, platform_version, None, None, None, )
1-360
: Consider adding tests for additional edge casesWhile the tests cover the basic success and failure scenarios, consider adding tests for:
- Minimum purchase amounts if applicable
- Tiered pricing schedules if supported
- Edge cases around token ownership transfers during purchase
- Concurrent purchase attempts
Would you like me to provide examples for any of these additional test cases?
packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_set_price_for_direct_purchase_transition.rs (1)
20-33
: Version matching approach is correct.The method dispatch based on
platform_version.drive.methods.state_transitions.convert_to_high_level_operations.token_set_price_for_direct_purchase_transition
is well-structured. Consider adding any missing branch coverage in tests if future versions appear.packages/rs-dpp/src/errors/consensus/state/token/token_direct_purchase_user_price_too_low.rs (1)
4-4
: Remove unusedProtocolError
import.
ProtocolError
is never referenced in this file, so removing it will help keep dependencies clean.-use crate::ProtocolError;
packages/rs-dpp/src/errors/consensus/state/token/token_amount_under_minimum_sale_amount.rs (1)
3-3
: Remove unusedProtocolError
import.
ProtocolError
is never referenced in this file, so removing it will help keep dependencies clean.-use crate::ProtocolError;
packages/rs-drive/src/drive/tokens/direct_purchase/fetch_token_direct_purchase_price/v0/mod.rs (1)
30-69
: Consider avoiding magic number inQueryTargetValue(8)
.Currently,
8
is embedded directly in the code. Defining it as a constant may improve readability and maintainability. Otherwise, the method is well-structured, correctly handles all GroveDB query outcomes, and leveragesdeserialize_from_bytes
with safe error propagation.packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_transition.rs (1)
296-297
: Consider documenting action ID behavior for new transitions.The code correctly implements action ID handling for the new transitions, but lacks documentation explaining why
DirectPurchase
doesn't need an action ID whileSetPriceForDirectPurchase
does. Adding comments would improve code maintainability.+ // DirectPurchase doesn't need an action ID as it's a reactive transaction TokenTransition::DirectPurchase(_) => None, + // SetPriceForDirectPurchase needs an action ID for uniqueness validation TokenTransition::SetPriceForDirectPurchase(t) => Some(t.calculate_action_id(owner_id)),Also applies to: 309-313
packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_direct_purchase_transition_action/v0/transformer.rs (5)
27-28
: Correct the "direct_purchaseing" terminology in the doc comments.
The repeated term "direct_purchaseing" is grammatically awkward. Consider using "direct purchase" to align with the function name and avoid confusion.- /// This method processes the token direct_purchaseing transition ... - /// looking up necessary data contracts and applying the relevant direct_purchaseing logic. + /// This method processes the token direct purchase transition ... + /// looking up necessary data contracts and applying the relevant direct purchase logic.
53-69
: Consider refactoring to reduce the number of function parameters.
While disabling the Clippy warning is acceptable in some cases, having this many parameters can reduce readability and maintainability. You might consider introducing a builder pattern or a parameter struct to organize the arguments.
253-267
: Confirm the intended behavior when the user’s agreed price exceeds the required price.
This code always setstotal_agreed_price
torequired_price
, even if the user has offered to pay more. If your goal is to capture the exact user offering, you may need to store or reflect the additional amount. Otherwise, explicitly document that the user’s price is floored to the required price.
225-251
: Validate the scenario of an emptySetPrices
map.
The code uses.expect("Map is not empty")
at line 243, implyingSetPrices
is guaranteed to contain entries. Confirm that no valid transitions permit an empty map. If emptiness is possible, returning a more explicit error might be preferable.
121-132
: Observe possible performance implications of repeated fetch operations.
fetch_token_direct_purchase_price_with_costs()
retrieves the schedule and its associated cost. If you anticipate multiple lookups or expansions in the future, consider caching results to avoid repeated I/O operations.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
packages/rs-dpp/src/errors/consensus/codes.rs
(1 hunks)packages/rs-dpp/src/errors/consensus/state/state_error.rs
(2 hunks)packages/rs-dpp/src/errors/consensus/state/token/mod.rs
(2 hunks)packages/rs-dpp/src/errors/consensus/state/token/token_amount_under_minimum_sale_amount.rs
(1 hunks)packages/rs-dpp/src/errors/consensus/state/token/token_direct_purchase_user_price_too_low.rs
(1 hunks)packages/rs-dpp/src/errors/consensus/state/token/token_not_for_direct_sale.rs
(1 hunks)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_transition.rs
(10 hunks)packages/rs-dpp/src/tokens/token_event.rs
(5 hunks)packages/rs-drive-abci/src/execution/types/execution_event/mod.rs
(1 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/direct_selling/mod.rs
(1 hunks)packages/rs-drive-abci/src/query/group_queries/group_actions/v0/mod.rs
(3 hunks)packages/rs-drive-abci/src/query/service.rs
(2 hunks)packages/rs-drive/src/drive/tokens/direct_purchase/fetch_token_direct_purchase_price/mod.rs
(1 hunks)packages/rs-drive/src/drive/tokens/direct_purchase/fetch_token_direct_purchase_price/v0/mod.rs
(1 hunks)packages/rs-drive/src/drive/tokens/direct_purchase/mod.rs
(1 hunks)packages/rs-drive/src/drive/tokens/direct_purchase/queries.rs
(1 hunks)packages/rs-drive/src/drive/tokens/direct_purchase/set_direct_purchase_price/v0/mod.rs
(1 hunks)packages/rs-drive/src/drive/tokens/estimated_costs/for_token_direct_selling_prices/v0/mod.rs
(1 hunks)packages/rs-drive/src/drive/tokens/paths.rs
(2 hunks)packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_set_price_for_direct_purchase_transition.rs
(1 hunks)packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_transition.rs
(3 hunks)packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_direct_purchase_transition_action/transformer.rs
(1 hunks)packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_direct_purchase_transition_action/v0/transformer.rs
(1 hunks)packages/rs-drive/src/state_transition_action/batch/v0/mod.rs
(2 hunks)packages/rs-platform-version/src/version/drive_versions/drive_token_method_versions/mod.rs
(1 hunks)packages/rs-platform-version/src/version/drive_versions/drive_token_method_versions/v1.rs
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/rs-drive-abci/src/execution/types/execution_event/mod.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/rs-drive/src/drive/tokens/direct_purchase/set_direct_purchase_price/v0/mod.rs
- packages/rs-drive/src/drive/tokens/direct_purchase/queries.rs
🧰 Additional context used
🧠 Learnings (1)
packages/rs-dpp/src/errors/consensus/state/token/mod.rs (1)
Learnt from: QuantumExplorer
PR: dashpay/platform#2439
File: packages/rs-dpp/src/errors/consensus/state/token/mod.rs:4-22
Timestamp: 2025-04-06T00:27:16.216Z
Learning: The user QuantumExplorer prefers to handle documentation of breaking changes separately from the code changes, particularly for token-related error types and validation rules.
🧬 Code Graph Analysis (12)
packages/rs-drive/src/drive/tokens/direct_purchase/mod.rs (1)
packages/rs-drive/src/drive/tokens/direct_purchase/fetch_token_direct_purchase_price/mod.rs (1)
fetch_token_direct_purchase_price
(25-45)
packages/rs-drive-abci/src/query/service.rs (2)
packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts (2)
GetStatusRequest
(5449-5464)GetStatusResponse
(5493-5508)packages/dapi-grpc/clients/platform/v0/web/platform_pb_service.d.ts (1)
Status
(442-442)
packages/rs-drive-abci/src/query/group_queries/group_actions/v0/mod.rs (1)
packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts (11)
BurnEvent
(8071-8088)DestroyFrozenFundsEvent
(8153-8175)EmergencyActionEvent
(8245-8262)FreezeEvent
(8097-8116)GroupActions
(8546-8560)MintEvent
(8039-8061)TokenConfigUpdateEvent
(8278-8297)TokenEvent
(8448-8493)UnfreezeEvent
(8125-8144)GetGroupActionsResponseV0
(8005-8030)Type
(1919-1919)
packages/rs-drive/src/drive/tokens/estimated_costs/for_token_direct_selling_prices/v0/mod.rs (1)
packages/rs-drive/src/drive/tokens/paths.rs (2)
token_direct_purchase_root_path
(91-96)tokens_root_path
(68-70)
packages/rs-dpp/src/tokens/token_event.rs (6)
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_set_price_for_direct_purchase_transition/v0/v0_methods.rs (2)
price
(27-27)price
(44-46)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_set_price_for_direct_purchase_transition/v0_methods.rs (1)
price
(34-38)packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_set_price_for_direct_purchase_transition_action/v0/mod.rs (3)
price
(30-30)price
(81-83)token_id
(41-43)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_transition.rs (2)
token_id
(209-209)token_id
(321-323)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_transfer_transition/v0_methods.rs (1)
amount
(30-34)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_mint_transition/v0/v0_methods.rs (2)
amount
(31-31)amount
(56-58)
packages/rs-dpp/src/errors/consensus/state/token/token_direct_purchase_user_price_too_low.rs (3)
packages/rs-dpp/src/errors/consensus/state/token/token_not_for_direct_sale.rs (3)
new
(19-21)token_id
(23-25)from
(29-31)packages/rs-dpp/src/errors/consensus/state/token/token_amount_under_minimum_sale_amount.rs (3)
new
(23-29)token_id
(31-33)from
(45-47)packages/rs-dpp/src/errors/consensus/state/state_error.rs (1)
from
(304-306)
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_transition.rs (3)
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/document_transition.rs (2)
as_transition_token_direct_purchase
(123-125)as_transition_token_set_price_for_direct_purchase
(127-131)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/resolvers.rs (4)
as_transition_token_direct_purchase
(114-119)as_transition_token_direct_purchase
(234-239)as_transition_token_set_price_for_direct_purchase
(121-130)as_transition_token_set_price_for_direct_purchase
(241-250)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/resolvers/v0/mod.rs (2)
as_transition_token_direct_purchase
(32-32)as_transition_token_set_price_for_direct_purchase
(33-35)
packages/rs-dpp/src/errors/consensus/state/token/token_not_for_direct_sale.rs (3)
packages/rs-dpp/src/errors/consensus/state/token/token_direct_purchase_user_price_too_low.rs (3)
new
(24-30)token_id
(32-34)from
(46-48)packages/rs-dpp/src/errors/consensus/state/token/token_amount_under_minimum_sale_amount.rs (3)
new
(23-29)token_id
(31-33)from
(45-47)packages/rs-dpp/src/errors/consensus/state/state_error.rs (1)
from
(304-306)
packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_direct_purchase_transition_action/v0/transformer.rs (3)
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_direct_purchase_transition/v0_methods.rs (3)
base
(9-13)token_count
(29-33)total_agreed_price
(41-45)packages/rs-dpp/src/errors/consensus/state/token/token_direct_purchase_user_price_too_low.rs (3)
new
(24-30)token_id
(32-34)required_price
(40-42)packages/rs-dpp/src/errors/consensus/state/token/token_amount_under_minimum_sale_amount.rs (2)
new
(23-29)token_id
(31-33)
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/direct_selling/mod.rs (3)
packages/rs-dpp/src/errors/consensus/state/token/token_direct_purchase_user_price_too_low.rs (2)
new
(24-30)token_id
(32-34)packages/rs-dpp/src/errors/consensus/state/token/token_amount_under_minimum_sale_amount.rs (2)
new
(23-29)token_id
(31-33)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/v1/mod.rs (2)
new_token_change_direct_purchase_price_transition
(218-234)new_token_direct_purchase_transition
(237-252)
packages/rs-drive/src/drive/tokens/direct_purchase/fetch_token_direct_purchase_price/mod.rs (5)
packages/rs-drive/src/state_transition_action/batch/batched_transition/document_transition/document_create_transition_action/v0/mod.rs (1)
block_info
(53-53)packages/rs-dpp/src/errors/consensus/state/token/token_direct_purchase_user_price_too_low.rs (1)
token_id
(32-34)packages/rs-dpp/src/errors/consensus/state/token/token_not_for_direct_sale.rs (1)
token_id
(23-25)packages/rs-dpp/src/errors/consensus/state/token/token_amount_under_minimum_sale_amount.rs (1)
token_id
(31-33)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_transition.rs (2)
token_id
(209-209)token_id
(321-323)
packages/rs-dpp/src/errors/consensus/state/token/token_amount_under_minimum_sale_amount.rs (3)
packages/rs-dpp/src/errors/consensus/state/token/token_direct_purchase_user_price_too_low.rs (3)
new
(24-30)token_id
(32-34)from
(46-48)packages/rs-dpp/src/errors/consensus/state/token/token_not_for_direct_sale.rs (3)
new
(19-21)token_id
(23-25)from
(29-31)packages/rs-dpp/src/errors/consensus/state/state_error.rs (1)
from
(304-306)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: Rust packages (rs-dapi-client) / Unused dependencies
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Rust packages (drive) / Formatting
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: Rust crates security audit
🔇 Additional comments (56)
packages/rs-platform-version/src/version/drive_versions/drive_token_method_versions/mod.rs (1)
37-37
: Addition of token direct purchase price feature looks goodThis new field follows the established pattern of the codebase and properly extends the
DriveTokenFetchMethodVersions
struct to support token direct purchase price functionality, which aligns with the PR objectives for token selling features.packages/rs-platform-version/src/version/drive_versions/drive_token_method_versions/v1.rs (1)
21-21
: Initialization of the new field is consistentThe new
token_direct_purchase_price
field is properly initialized to version 0, matching the pattern of other fields in the structure. This ensures consistent versioning across all token fetch methods.packages/rs-dpp/src/tokens/token_event.rs (4)
28-28
: Dependencies added correctly for the new token pricing features.The imports for
TokenPricingSchedule
andCredits
are correctly added to support the new functionality.Also applies to: 9-9
34-34
: New type alias for purchase tracking added.A new type alias
PurchaserIdentifier
has been added, which will help with type safety when identifying token purchasers.
77-78
: Document type names properly defined for new events.The
associated_document_type_name
method has been correctly updated to return appropriate string identifiers for the new event types.
256-273
: ChangePriceForDirectPurchase historical document implementation looks good.The implementation for building historical documents for the
ChangePriceForDirectPurchase
event follows the same pattern as other events:
- Includes the token ID
- Includes the serialized price schedule when present
- Includes the note when present
packages/rs-drive/src/drive/tokens/estimated_costs/for_token_direct_selling_prices/v0/mod.rs (1)
1-101
: Confirm naming approach and depth estimate.
- The file is called "direct_selling_prices," but references across the code also mention "purchase." Unify the terminology.
- The estimated depth is set to 10. If this is intentional as a conservative estimate, consider documenting the rationale more explicitly. Otherwise, it might be an overestimation.
[request_verification, suggest_nitpick]packages/rs-drive/src/drive/tokens/direct_purchase/fetch_token_direct_purchase_price/mod.rs (2)
1-2
: Module inclusion looks good.The inclusion of
mod v0;
follows the versioned module pattern appropriately.
13-13
: Version-based fetching approach is well-structured.The approach of matching on
platform_version.drive.methods.token.fetch.token_direct_purchase_price
ensures backward compatibility and clarity for future expansions.packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/direct_selling/mod.rs (7)
3-5
: Good modular organization!The module organization is clear and the imports look appropriate for token selling tests.
6-39
: Well-structured test setupThe test creates a platform with seller and buyer identities, and sets up a token contract with appropriate permission structures for direct purchase pricing.
43-74
: Price setting logic correctly implementedThe seller correctly sets a single price for the token using the appropriate transition function and the test verifies the execution result.
195-213
: Effective error condition testGood test for validating inadequate payment scenario. The test correctly sets up a condition where the buyer offers less payment than required.
221-227
: Clear assertion for expected error typeThe assertion correctly checks for the specific consensus error type
TokenDirectPurchaseUserPriceTooLow
, which is appropriate for this test case.
247-280
: Appropriate test for insufficient balance edge caseThe test correctly sets up a buyer with insufficient funds (0.01 DASH) for a 1 DASH purchase to validate the error handling.
342-358
: Correct error validation for insufficient fundsThe test properly validates that the transaction fails with an
IdentityInsufficientBalanceError
and that no tokens are transferred.packages/rs-drive-abci/src/query/service.rs (1)
40-41
: Import additions for new token direct purchase prices functionalityThe imports have been expanded to include
GetTokenDirectPurchasePricesRequest
andGetTokenDirectPurchasePricesResponse
, which aligns with the new method added to the service.packages/rs-drive-abci/src/query/group_queries/group_actions/v0/mod.rs (5)
7-7
: Updated imports for direct purchase price event supportThe import list has been expanded to include
UpdateDirectPurchasePriceEvent
along with other event types, which is necessary for implementing the new token price change functionality.
11-11
: Added pricing-related structure importsAppropriate imports have been added for the pricing components (Price, PriceForQuantity, PricingSchedule) needed to implement the direct purchase price functionality.
21-21
: Added TokenPricingSchedule importThis import is necessary for handling the pricing schedule data from the token event.
200-222
: Well-implemented handler for ChangePriceForDirectPurchase eventThe implementation for handling the direct purchase price change event is well structured, mapping from the internal
TokenPricingSchedule
representation to the gRPC response format. The code correctly handles both single price and variable pricing schedule cases.
223-225
:✅ Verification successful
Behavior change: Transfer, DirectPurchase, and Claim events now filtered out
This code now explicitly filters out Transfer, DirectPurchase, and Claim token events by returning None, which means they won't appear in query results. If this is a change in behavior, ensure this is the intended outcome.
Verify that filtering out these events is intentional and won't break existing functionality:
🏁 Script executed:
#!/bin/bash # Check if there are any clients or tests that depend on these events being returned # Look for references to Transfer, DirectPurchase, or Claim events in client code echo "Searching for references to filtered token events in client code" rg -A 2 -B 2 "TokenEvent.*Transfer|Transfer.*TokenEvent|DirectPurchase|Claim" --type-not "json" --type-not "md"Length of output: 68518
Verified: Filtering of Transfer, DirectPurchase, and Claim Events is Intentional
The grep search confirms that in the group actions query (packages/rs-drive-abci/src/query/group_queries/group_actions/v0/mod.rs, lines 223–225) these events are purposefully filtered out. There’s no evidence in the client or test code that they should be returned in query responses. Please ensure that any documentation or release notes reflect this intentional behavior change.
packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_direct_purchase_transition_action/transformer.rs (1)
1-68
: Implementation looks well-structured and properly documentedThe implementation of
TokenDirectPurchaseTransitionAction
follows the expected pattern for transition actions. The code is well-organized with comprehensive documentation, proper error handling, and version handling through the match statement.packages/rs-dpp/src/errors/consensus/codes.rs (1)
310-312
: Error codes are appropriately defined and categorizedThe three new error variants are correctly added to the "Token errors" section (40700-40799 range) with sequential error codes. The naming is descriptive and clearly indicates the specific token direct purchase error conditions.
packages/rs-drive/src/state_transition_action/batch/v0/mod.rs (2)
9-10
: Imports are properly added for new token purchase functionalityThe new imports correctly bring in the necessary components for handling token direct purchase transitions.
41-47
: Token purchase calculation correctly integrated into existing logicThe code properly handles both document purchases and the new token direct purchases by extracting the total agreed price. Variable renaming from
purchase
todocument_purchase
improves code clarity.packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_transition.rs (3)
16-17
: Required imports for new token purchase functionalityThe imports for the new token direct purchase and price setting action accessors have been properly added.
62-66
: New token actions correctly implementedThe implementation of the operation conversion for the new
DirectPurchaseAction
andSetPriceForDirectPurchaseAction
variants follows the existing pattern consistently.
125-136
: Token event mapping properly implementedThe code correctly maps the new transition action variants to their corresponding token events with appropriate parameters.
packages/rs-dpp/src/errors/consensus/state/state_error.rs (2)
44-44
: New token-related imports appear consistent with existing patterns.These newly added error variants align well with the established naming convention. They also match the usage pattern demonstrated in the rest of the file’s imports.
292-300
: New token error variants extend StateError in a compatible manner.Appending these variants at the end respects the "DO NOT CHANGE ORDER" rule. Each variant is properly annotated with
#[error(transparent)]
, ensuring transparent error wrapping. Please confirm that all calling sites handle these errors (e.g., test coverage or usage in transitions).Would you like a helper script to locate all references to these new variants and confirm test coverage?
packages/rs-dpp/src/errors/consensus/state/token/mod.rs (3)
18-19
: New modules for token sale logic.Defining separate modules for minimum sale amount and direct purchase price errors keeps error definitions well-organized and modular.
22-22
: Introducedtoken_not_for_direct_sale
module.This addition complements the existing token sale modules, providing a structured approach to each logical error type.
45-46
: Exports for new token error modules are consistent.These
pub use
statements ensure each newly introduced error is accessible throughout the codebase for unified error handling.Also applies to: 49-49
packages/rs-dpp/src/errors/consensus/state/token/token_not_for_direct_sale.rs (4)
1-7
: Imports and external dependencies look appropriate.The modules and traits imported (e.g.,
thiserror::Error
,platform_serialization_derive
) match the established coding style across similar token-related errors.
9-16
: Structured error type for non-sale tokens.Defining
TokenNotForDirectSale
with#[derive(...)]
properly integrates it into the overarching error-handling system, complete with serialization capabilities.
18-26
: Constructor and accessor usage.The
new
constructor andtoken_id
accessor provide a straightforward, minimal API. This matches the pattern used by other token errors (e.g.,TokenAmountUnderMinimumSaleAmount
).
28-32
:From<TokenNotForDirectSale>
implementation.Allowing seamless conversion to
ConsensusError
ensures consistent handling of all token sale conditions. This maintains error consistency throughout the consensus module.packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_set_price_for_direct_purchase_transition.rs (5)
1-19
: Imports reflect the new direct purchase logic.You’re selecting the correct modules for token, group, and identity operations. Aligning them under
platform_version
sets a good foundation for version-based branching.
34-45
: Identity nonce update logic.Including the
UpdateIdentityContractNonce
operation is consistent with how other transitions handle identity updates. No issues found.
47-76
: Group action creation for direct purchase.Checking
store_in_group()
and pushing aGroupOperation
is a neat pattern. The logic that conditionally initializes an event is clear and aligns with existingGroupAction
flows.
78-96
: Token operation flow ensures correct price updates and optional history.Conditionally pushing
TokenSetPriceForDirectPurchase
is well handled. The subsequent check forkeeps_direct_pricing_history()
is a clean approach to logging historical pricing.
98-106
: Graceful handling of unknown versions.Emitting an error for unsupported versions helps maintain safe operation.
packages/rs-dpp/src/errors/consensus/state/token/token_direct_purchase_user_price_too_low.rs (3)
10-21
: Struct definition looks correct.The fields and derived traits accurately describe the error type. No further issues spotted.
23-43
: Implementation of getters is clear and consistent.All methods follow a straightforward getter pattern, which is easy to understand and maintain.
45-49
: Conversion toConsensusError
is well-structured.The
From
trait implementation aligns with existing error-handling patterns in the codebase.packages/rs-dpp/src/errors/consensus/state/token/token_amount_under_minimum_sale_amount.rs (3)
9-20
: Struct definition is accurate.The fields and their derivations correctly capture this error scenario.
22-42
: Constructor and getters are straightforward.They provide a clear API for initializing and querying the error's data.
44-48
:From
trait implementation matches the codebase pattern.Properly converts this error into a
ConsensusError
.packages/rs-drive/src/drive/tokens/direct_purchase/fetch_token_direct_purchase_price/v0/mod.rs (1)
14-28
: Function implementation is coherent.
fetch_token_direct_purchase_price_v0
encapsulates retrieval logic properly by delegating to the_operations_v0
variant, maintaining clean separation of concerns.packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_transition.rs (4)
81-85
: Implementation of new direct purchase token transition variants looks good.The new enum variants for direct purchase functionality follow the established pattern in the codebase, with proper display attributes and consistent naming conventions.
181-197
: Accessor implementations conform to expected pattern.The implementations for accessing the new transition types correctly follow the established pattern in the code, with proper use of pattern matching to extract the variants safely.
260-261
: Base accessors properly extended for new variants.The base() and base_mut() methods are correctly updated to handle the new transition types, maintaining consistency with the existing pattern.
Also applies to: 276-277
479-488
: Token event creation for new transitions is handled correctly.The associated_token_event method is properly extended to handle the new transition types, with
DirectPurchase
creating a token event with count and price information, andSetPriceForDirectPurchase
creating a price change event with the new price and optional public note.packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_direct_purchase_transition_action/v0/transformer.rs (2)
190-223
: Double-check boundary logic for tiered pricing.
You retrieve the highest threshold price tier usingrange(..=token_count).next_back()
. Ensure it’s correct for all edge cases, including token counts just below or above the defined tiers, to prevent off-by-one errors.
161-162
:❓ Verification inconclusive
Reassess the use of saturating multiplication.
Usingsaturating_mul
can silently clamp values on overflow rather than returning an error. Verify this is intended, since users purchasing extremely large quantities of tokens may unknowingly hit the saturation limit.
🏁 Script executed:
#!/bin/bash # Search for all occurrences of saturating_mul in the codebase to confirm usage. fd . -t f -e rs --exec rg 'saturating_mul'Length of output: 932
Saturating Multiplication Usage Verification
The verification shows that
saturating_mul
is used consistently across the repository (e.g., in price calculations and fee computations), which suggests this behavior is an intentional design choice. However, in the context of token purchases (calculatingrequired_price
here on lines 161-162 ofpackages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_direct_purchase_transition_action/v0/transformer.rs
), the silent clamping on overflow might lead to unexpected outcomes for extremely large token counts. Please double-check that this behavior is acceptable from a business logic standpoint—if not, consider handling overflows explicitly (e.g., by returning an error).
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: 0
🧹 Nitpick comments (3)
packages/rs-drive/src/verify/tokens/verify_token_direct_selling_prices/mod.rs (1)
49-68
: Future-proofing the version dispatcher.Currently, the match statement only supports version 0. While this is acceptable for now, consider proactively adding a default branch or explicit handling for future versions to streamline maintenance.
packages/rs-drive/src/verify/tokens/verify_token_direct_selling_prices/v0/mod.rs (2)
15-23
: Check behavior with an emptytoken_ids
array.If
token_ids
is empty, the method still constructs a query and verifies proofs. Consider returning early or documenting the behavior to ensure clarity for edge cases involving no tokens.
39-53
: Clear error reporting for unexpected proof elements.The code correctly distinguishes between present and absent items, but the
_ => Err(...)
branch treats all unrecognized elements as a size issue. Consider adding a more descriptive error message or a dedicated branch for future expansions ofElement
types.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/rs-drive/src/verify/tokens/mod.rs
(1 hunks)packages/rs-drive/src/verify/tokens/verify_token_direct_selling_prices/mod.rs
(1 hunks)packages/rs-drive/src/verify/tokens/verify_token_direct_selling_prices/v0/mod.rs
(1 hunks)packages/rs-platform-version/src/version/drive_versions/drive_verify_method_versions/mod.rs
(1 hunks)packages/rs-platform-version/src/version/drive_versions/drive_verify_method_versions/v1.rs
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/rs-drive/src/verify/tokens/mod.rs
- packages/rs-platform-version/src/version/drive_versions/drive_verify_method_versions/mod.rs
- packages/rs-platform-version/src/version/drive_versions/drive_verify_method_versions/v1.rs
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/rs-drive/src/verify/tokens/verify_token_direct_selling_prices/v0/mod.rs (1)
packages/rs-drive/src/drive/tokens/direct_purchase/queries.rs (1)
token_direct_purchase_prices_query
(8-21)
packages/rs-drive/src/verify/tokens/verify_token_direct_selling_prices/mod.rs (1)
packages/rs-drive/src/verify/tokens/verify_token_direct_selling_prices/v0/mod.rs (1)
verify_token_direct_selling_prices_v0
(15-64)
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (dapi-grpc) / Tests
- GitHub Check: Rust packages (dapi-grpc) / Linting
- GitHub Check: Rust packages (dapi-grpc) / Check each feature
- GitHub Check: Rust packages (rs-dapi-client) / Tests
- GitHub Check: Rust packages (rs-dapi-client) / Check each feature
- GitHub Check: Rust packages (rs-dapi-client) / Linting
- GitHub Check: Rust packages (wasm-dpp) / Formatting
- GitHub Check: Rust packages (wasm-dpp) / Linting
- GitHub Check: Rust packages (token-history-contract) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Unused dependencies
- GitHub Check: Rust packages (token-history-contract) / Unused dependencies
- GitHub Check: Rust packages (token-history-contract) / Formatting
- GitHub Check: Rust packages (token-history-contract) / Linting
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
🔇 Additional comments (1)
packages/rs-drive/src/verify/tokens/verify_token_direct_selling_prices/mod.rs (1)
13-40
: Good use of descriptive doc comments.The thorough documentation helps clarify parameters, return types, and possible errors. This is beneficial for maintainability and comprehension.
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: 0
🧹 Nitpick comments (4)
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v0_methods.rs (2)
1047-1091
: Variable name doesn't match its purpose.The variable name
claim_transition
should be renamed todirect_purchase_transition
to match its actual purpose and improve code readability.- let claim_transition = TokenDirectPurchaseTransition::V0(TokenDirectPurchaseTransitionV0 { + let direct_purchase_transition = TokenDirectPurchaseTransition::V0(TokenDirectPurchaseTransitionV0 { base: TokenBaseTransition::V0(TokenBaseTransitionV0 { identity_contract_nonce, token_contract_position, data_contract_id, token_id, using_group_info: None, }), token_count: amount, total_agreed_price: agreed_total_cost, }); let batch_transition: BatchTransition = BatchTransitionV1 { owner_id, - transitions: vec![BatchedTransition::Token(claim_transition.into())], + transitions: vec![BatchedTransition::Token(direct_purchase_transition.into())], user_fee_increase, signature_public_key_id: 0, signature: Default::default(), }
1093-1166
: Consider adding documentation for this method.This method implements a key part of the token selling functionality, but lacks documentation explaining its purpose and parameters. Adding documentation would help future developers understand how to use this method correctly.
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/v1/mod.rs (2)
216-235
: Provide unit tests and consider parameter grouping
A new method for changing direct purchase price is introduced with many parameters. This mirrors existing transitions but could benefit from grouping parameters into a dedicated struct for clarity. Also ensure thorough tests to verify different pricing schedule scenarios, error handling, and boundary cases.
236-253
: Ensure thorough testing of direct purchase transitions
This method for direct token purchases introduces numerous parameters. As with the previous transition, consider refactoring them into a structured type, and add tests that verify various purchase paths (valid, invalid, edge values).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/v1/mod.rs
(3 hunks)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v0_methods.rs
(4 hunks)packages/rs-dpp/src/tokens/token_event.rs
(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/rs-dpp/src/tokens/token_event.rs
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/v1/mod.rs (1)
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/mod.rs (2)
new_token_change_direct_purchase_price_transition
(1003-1062)new_token_direct_purchase_transition
(1065-1121)
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v0_methods.rs (1)
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/mod.rs (2)
new_token_direct_purchase_transition
(1065-1121)new_token_change_direct_purchase_price_transition
(1003-1062)
🪛 GitHub Check: Rust packages (drive-abci) / Linting
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/v1/mod.rs
[warning] 7-7: unused import: crate::fee::Credits
warning: unused import: crate::fee::Credits
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/v1/mod.rs:7:5
|
7 | use crate::fee::Credits;
| ^^^^^^^^^^^^^^^^^^^
[warning] 21-21: unused import: crate::tokens::token_pricing_schedule::TokenPricingSchedule
warning: unused import: crate::tokens::token_pricing_schedule::TokenPricingSchedule
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/v1/mod.rs:21:5
|
21 | use crate::tokens::token_pricing_schedule::TokenPricingSchedule;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v0_methods.rs
[warning] 30-30: unused imports: TokenDirectPurchaseTransition
and TokenSetPriceForDirectPurchaseTransition
warning: unused imports: TokenDirectPurchaseTransition
and TokenSetPriceForDirectPurchaseTransition
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v0_methods.rs:30:68
|
30 | use crate::state_transition::batch_transition::{BatchTransitionV1, TokenDirectPurchaseTransition, TokenSetPriceForDirectPurchaseTransition};
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[warning] 81-81: unused import: crate::state_transition::batch_transition::token_set_price_for_direct_purchase_transition::TokenSetPriceForDirectPurchaseTransitionV0
warning: unused import: crate::state_transition::batch_transition::token_set_price_for_direct_purchase_transition::TokenSetPriceForDirectPurchaseTransitionV0
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v0_methods.rs:81:5
|
81 | use crate::state_transition::batch_transition::token_set_price_for_direct_purchase_transition::TokenSetPriceForDirectPurchaseTransitionV0;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[warning] 74-74: unused import: crate::state_transition::batch_transition::token_direct_purchase_transition::TokenDirectPurchaseTransitionV0
warning: unused import: crate::state_transition::batch_transition::token_direct_purchase_transition::TokenDirectPurchaseTransitionV0
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v0_methods.rs:74:5
|
74 | use crate::state_transition::batch_transition::token_direct_purchase_transition::TokenDirectPurchaseTransitionV0;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[warning] 92-92: unused import: crate::tokens::token_pricing_schedule::TokenPricingSchedule
warning: unused import: crate::tokens::token_pricing_schedule::TokenPricingSchedule
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v0_methods.rs:92:5
|
92 | use crate::tokens::token_pricing_schedule::TokenPricingSchedule;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: Rust packages (dapi-grpc) / Unused dependencies
- GitHub Check: Rust packages (dapi-grpc) / Check each feature
- GitHub Check: Rust packages (dapi-grpc) / Linting
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Build JS packages / Build JS
🔇 Additional comments (5)
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v0_methods.rs (3)
30-30
: Resolved static analysis warnings about unused imports.The static analysis tool reports that several imports are unused, but they're actually all used in the newly added methods. I've verified that:
TokenDirectPurchaseTransition
andTokenSetPriceForDirectPurchaseTransition
are used in lines 1064 and 1112TokenDirectPurchaseTransitionV0
is used in line 1064TokenSetPriceForDirectPurchaseTransitionV0
is used in line 1112TokenPricingSchedule
is used as a parameter type in line 1099Also applies to: 74-74, 81-81, 92-92
🧰 Tools
🪛 GitHub Check: Rust packages (drive-abci) / Linting
[warning] 30-30: unused imports:
TokenDirectPurchaseTransition
andTokenSetPriceForDirectPurchaseTransition
warning: unused imports:TokenDirectPurchaseTransition
andTokenSetPriceForDirectPurchaseTransition
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v0_methods.rs:30:68
|
30 | use crate::state_transition::batch_transition::{BatchTransitionV1, TokenDirectPurchaseTransition, TokenSetPriceForDirectPurchaseTransition};
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
1047-1091
: Implementation follows the established pattern.The method correctly creates a token direct purchase transition with the necessary parameters and properly signs it with HIGH security level. This implementation follows the same pattern as other token transition methods in this file.
1093-1166
: Well-structured implementation for price change transition.The method follows the established pattern for token transitions, including proper handling of group information through pattern matching. The implementation correctly creates a token price change transition and properly signs it with HIGH security level.
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/v1/mod.rs (2)
21-21
:❓ Verification inconclusive
Validate actual usage of
TokenPricingSchedule
import
Similarly, static analysis marksTokenPricingSchedule
as unused, but it’s actually leveraged in the new function parameter list under the feature flag.Confirm correct usage detection by running:
🏁 Script executed:
#!/bin/bash cargo check --all-features
Length of output: 124
Manual Verification Needed: Validate Usage of
TokenPricingSchedule
The automated check couldn’t be completed because the environment did not have the
cargo
command available (error:cargo: command not found
). Since this command is expected to verify thatTokenPricingSchedule
is actively used (under the feature flag in the new function parameter list), please run the following command manually in your local development environment to confirm correct usage:cargo check --all-featuresOnce verified, please update the review accordingly.
🧰 Tools
🪛 GitHub Check: Rust packages (drive-abci) / Linting
[warning] 21-21: unused import:
crate::tokens::token_pricing_schedule::TokenPricingSchedule
warning: unused import:crate::tokens::token_pricing_schedule::TokenPricingSchedule
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/v1/mod.rs:21:5
|
21 | use crate::tokens::token_pricing_schedule::TokenPricingSchedule;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
7-7
:❓ Verification inconclusive
Validate actual usage of
Credits
import
Static analysis warns thatCredits
might be unused. However, it's invoked in the newly introduced function signature when thestate-transition-signing
feature is enabled.Run the following command (with all features) to confirm correct usage detection:
🏁 Script executed:
#!/bin/bash cargo check --all-features
Length of output: 124
Action Required: Manually verify
Credits
usage with the feature enabledThe automated check couldn’t validate the usage of
Credits
because the cargo command wasn’t executed successfully (it returned "cargo: command not found"). Please runcargo check --all-features
locally to confirm that theCredits
import is indeed used (in the function signature guarded by thestate-transition-signing
feature).
- Verify that the function signature in the file
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/v1/mod.rs
correctly depends on the feature flag.- Confirm that static analysis warnings regarding unused imports do not occur when all features are enabled.
🧰 Tools
🪛 GitHub Check: Rust packages (drive-abci) / Linting
[warning] 7-7: unused import:
crate::fee::Credits
warning: unused import:crate::fee::Credits
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/v1/mod.rs:7:5
|
7 | use crate::fee::Credits;
| ^^^^^^^^^^^^^^^^^^^
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: 1
🧹 Nitpick comments (10)
packages/rs-drive-abci/tests/strategy_tests/voting_tests.rs (1)
1305-1727
: Consider creating a test utility function for thread-wrapped tests.Since multiple tests in this file now use the same thread-wrapping pattern, consider extracting this logic into a helper function to improve maintainability and reduce duplication.
fn run_test_with_larger_stack<F>(test_name: &str, test_func: F) where F: FnOnce() + Send + 'static, { let stack_size = 4 * 1024 * 1024; let builder = std::thread::Builder::new() .stack_size(stack_size) .name(format!("{}_thread", test_name)); let handler = builder.spawn(test_func).expect("Failed to create thread with custom stack size"); handler.join().expect("Thread has panicked"); } // Usage example: #[test] fn run_chain_with_voting_after_won_by_identity_no_specialized_funds_distribution() { run_test_with_larger_stack("voting_test", || { // Test logic here }); }packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_direct_purchase_transition_action/v0/transformer.rs (5)
27-28
: Typographical fix in docstring.The word "direct_purchaseing" appears to be a small typographical slip. Consider correcting it to "direct purchasing" for consistency and readability in the documentation.
-/// This method processes the token direct_purchaseing transition... +/// This method processes the token direct purchasing transition...
78-89
: Potential duplication of contract lookup steps.You're calling
try_from_borrowed_base_transition_with_contract_lookup
to validate the base action and then calculating the fee separately usingDrive::calculate_fee
. These chunks of logic (retrieving contracts, computing fees, etc.) repeat in multiple places. Consider extracting versions of these tasks into smaller helper functions to clarify the workflow and to maintain DRY principles.
190-224
: Optional: Simplify range matching for set_prices.Using
set_prices.range(..=token_count).next_back()
is logical to find the largest threshold under the specified token_count. However, consider a dedicated method or helper instead of directly matching inside the transition logic, which may improve readability and testability of boundary conditions (e.g., exact matches, largest bracket thresholds).
225-250
: Repeated error handling logic.The steps for creating a
BumpIdentityDataContractNonceAction
and returning a wrapped error are repeated across multiple branches. Factor this out into a dedicated helper to avoid duplication.
255-267
: Function size consideration.
try_from_borrowed_token_direct_purchase_transition_with_contract_lookup
is quite large and incorporates multiple layers of validation, fee calculation, and error handling. Breaking it into smaller, well-named sub-functions (e.g., “validate_price_against_schedule,” “handle_pricing_schedule,” etc.) may improve maintainability.packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_set_price_for_direct_purchase_transition_action/v0/transformer.rs (4)
22-23
: Typographical fix in docstring.Similar to the direct purchase file, "set_price_for_direct_purchaseing" is slightly off. Changing it to "setting the price for direct purchase" or "set direct purchase price" will be clearer.
-/// This method processes the token set_price_for_direct_purchaseing transition... +/// This method processes the token set direct purchase price transition...
44-60
: Extend doc comments with rationale for contract lookups.You are performing a contract fetch for token configuration. Provide a sentence in the doc comment clarifying why it’s critical to re-verify token data from the contract at this step. This ensures future maintainers understand the reason for performing the lookup here before setting the price for direct purchase.
93-108
: Duplicate error-handling code pattern.Again, the creation of a
BumpIdentityDataContractNonceAction
and the assembly of aBatchedTransitionAction::BumpIdentityDataContractNonce
is repeated here. Refactoring into a helper would unify your error-handling flow and prevent code drift in the future.
127-165
: Large function size and repeated logic.
try_from_borrowed_token_set_price_for_direct_purchase_transition_with_contract_lookup
has a logic flow repeating some of the base action validation and fee calculation done in the analogous function that takes ownership. Factor out these repeated steps into smaller shared helper methods that both functions can call.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
packages/dapi-grpc/clients/platform/v0/java/org/dash/platform/dapi/v0/PlatformGrpc.java
(9 hunks)packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.h
(7 hunks)packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.m
(7 hunks)packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbrpc.h
(3 hunks)packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbrpc.m
(1 hunks)packages/dapi-grpc/clients/platform/v0/python/platform_pb2_grpc.py
(4 hunks)packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts
(5 hunks)packages/dapi-grpc/clients/platform/v0/web/platform_pb_service.d.ts
(3 hunks)packages/dapi-grpc/clients/platform/v0/web/platform_pb_service.js
(2 hunks)packages/rs-drive-abci/src/query/service.rs
(2 hunks)packages/rs-drive-abci/tests/strategy_tests/voting_tests.rs
(2 hunks)packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_direct_purchase_transition_action/v0/transformer.rs
(1 hunks)packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_set_price_for_direct_purchase_transition_action/v0/transformer.rs
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/rs-drive-abci/src/query/service.rs
🧰 Additional context used
🧬 Code Graph Analysis (7)
packages/dapi-grpc/clients/platform/v0/python/platform_pb2_grpc.py (1)
packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts (2)
GetTokenDirectPurchasePricesRequest
(7042-7057)GetTokenDirectPurchasePricesResponse
(7098-7113)
packages/dapi-grpc/clients/platform/v0/web/platform_pb_service.d.ts (1)
packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts (2)
GetTokenDirectPurchasePricesRequest
(7042-7057)GetTokenDirectPurchasePricesResponse
(7098-7113)
packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_set_price_for_direct_purchase_transition_action/v0/transformer.rs (1)
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_set_price_for_direct_purchase_transition/v0_methods.rs (3)
base
(12-16)price
(34-38)public_note
(46-50)
packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_direct_purchase_transition_action/v0/transformer.rs (4)
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_direct_purchase_transition/v0/v0_methods.rs (5)
base
(8-10)token_count
(22-22)token_count
(32-34)total_agreed_price
(26-26)total_agreed_price
(40-42)packages/rs-dpp/src/errors/consensus/state/token/token_amount_under_minimum_sale_amount.rs (2)
new
(23-29)token_id
(31-33)packages/rs-dpp/src/errors/consensus/state/token/token_direct_purchase_user_price_too_low.rs (3)
new
(24-30)token_id
(32-34)required_price
(40-42)packages/rs-dpp/src/errors/consensus/state/token/token_not_for_direct_sale.rs (2)
new
(19-21)token_id
(23-25)
packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbrpc.h (2)
packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts (2)
GetTokenDirectPurchasePricesRequest
(7042-7057)GetTokenDirectPurchasePricesResponse
(7098-7113)packages/dapi-grpc/clients/platform/v0/python/platform_pb2_grpc.py (2)
getTokenDirectPurchasePrices
(469-473)getTokenDirectPurchasePrices
(1374-1388)
packages/rs-drive-abci/tests/strategy_tests/voting_tests.rs (5)
packages/rs-drive-abci/src/query/service.rs (1)
new
(70-72)packages/rs-drive-abci/src/config.rs (10)
default_minimal_verifications
(887-895)default_100_67
(368-376)default_100_67
(480-488)default_100_67
(548-556)default
(360-363)default
(444-453)default
(534-543)default
(670-677)default
(681-683)default
(900-908)packages/rs-dpp/src/data_contract/document_type/property/mod.rs (2)
default
(2095-2099)from
(2103-2107)packages/rs-dpp/src/errors/consensus/basic/basic_error.rs (1)
from
(521-523)packages/rs-drive-abci/tests/strategy_tests/execution.rs (2)
run_chain_for_strategy
(51-780)continue_chain_for_strategy
(915-1232)
packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.h (1)
packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts (2)
GetTokenDirectPurchasePricesRequest
(7042-7057)GetTokenDirectPurchasePricesResponse
(7098-7113)
⏰ Context from checks skipped due to timeout of 90000ms (18)
- GitHub Check: Rust packages (token-history-contract) / Formatting
- GitHub Check: Rust packages (dash-sdk) / Check each feature
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (rs-dapi-client) / Unused dependencies
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (dash-sdk) / Unused dependencies
- GitHub Check: Rust packages (rs-dapi-client) / Linting
- GitHub Check: Rust packages (rs-dapi-client) / Check each feature
- GitHub Check: Rust packages (rs-dapi-client) / Tests
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive-abci) / Linting
- 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 (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 (43)
packages/rs-drive-abci/tests/strategy_tests/voting_tests.rs (5)
1306-1307
: Stack size increase is a good mitigation for stack overflow issues.Increasing the thread stack size from the default 2MB to 4MB is a reasonable approach when dealing with deep recursion or large stack frames in test code.
1309-1312
: Thread builder configuration looks appropriate.The thread builder is correctly configured with the custom stack size and a descriptive name that helps identify the thread in debugging scenarios.
1313-1314
: Thread spawn implementation with error handling.The
spawn
call is correctly wrapped in abuilder
context, and the return value is captured for later checking.
1315-1724
: Encapsulating test logic in a thread closure with PlatformVersion specificity.The test logic has been successfully moved into the thread closure. I notice that this implementation uses
PlatformVersion::get(7).unwrap()
(line 1336) with an explicit version number, while other tests usePlatformVersion::latest()
. This is consistent with the.with_initial_protocol_version(7)
call, indicating this test is targeting a specific platform version.
1725-1726
: Thread completion check is well-implemented.The implementation correctly waits for the thread to complete using
join()
and checks for panic conditions. This ensures that any test failures within the thread are properly propagated to the test runner.packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts (4)
7042-7096
: Well-structured request class for retrieving token direct purchase pricesThe
GetTokenDirectPurchasePricesRequest
class is implemented following the established pattern in the codebase with proper versioning (V0) and necessary fields to specify token IDs and proof requirements.
7098-7296
: Comprehensive response structure for token pricing informationThe
GetTokenDirectPurchasePricesResponse
class provides a well-designed hierarchy to support both fixed and variable pricing models:
Price
can represent either a simple fixed price or a variable pricing schedulePricingSchedule
contains multiple price points based on quantity- The implementation properly handles the results with proof mechanism following the established pattern
This structure gives good flexibility for different pricing models while maintaining consistent API patterns.
8562-8647
: Well-implemented event class for price updatesThe
UpdateDirectPurchasePriceEvent
class follows the same pattern as the response types, supporting both fixed and variable pricing models. The integration of the public note field will be useful for accountability and providing context for price changes.
8827-8831
: Complete integration with the token event systemThe addition of the
updatePrice
field to theTokenEvent
class and the corresponding TypeCase entry ensures proper event dispatching and handling within the existing event framework.Also applies to: 8852-8852, 8864-8864
packages/dapi-grpc/clients/platform/v0/python/platform_pb2_grpc.py (3)
202-206
: Implementation of new gRPC method in PlatformStub properly follows established pattern.The addition of the
getTokenDirectPurchasePrices
method to the PlatformStub class is correctly implemented, following the same pattern as other methods.
469-474
: Appropriate server-side stub for the new method.The implementation in PlatformServicer correctly raises NotImplementedError, as expected for a stub implementation that will be overridden by concrete implementations.
1373-1389
: Static helper method properly implemented for client-side usage.The static method in the Platform class is correctly implemented following the established pattern, allowing clients to easily make calls to this new gRPC endpoint.
packages/dapi-grpc/clients/platform/v0/web/platform_pb_service.js (2)
346-353
: Service method definition properly configured.The method definition for
getTokenDirectPurchasePrices
is correctly added to the Platform object with proper configuration of method name, service, stream settings, and request/response types.
1563-1592
: Client implementation follows established patterns.The implementation in PlatformClient.prototype correctly:
- Handles callback arguments
- Sets up the unary gRPC call
- Processes the response with proper error handling
- Provides a cancel function to abort the request
This follows the same pattern as other methods in the codebase.
packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbrpc.m (1)
878-897
: Objective-C client implementation properly follows established patterns.The implementation adds all three required methods for the Objective-C client:
- The public facing method that starts the RPC call
- The method that creates and returns the RPC object
- The method that supports the newer gRPC API style with response handlers
All methods follow the same implementation pattern as other methods in the file, maintaining consistency and providing complete functionality for Objective-C clients.
packages/dapi-grpc/clients/platform/v0/web/platform_pb_service.d.ts (3)
340-347
: Well-structured type definition for the new token purchase prices API.The type definition for
PlatformgetTokenDirectPurchasePrices
follows the established pattern in the codebase, correctly defining a non-streaming gRPC method with the appropriate request and response types.
442-442
: Properly registered method in the Platform class.The addition of the static property
getTokenDirectPurchasePrices
to the Platform class correctly references the type defined earlier, maintaining consistency with the rest of the API structure.
816-824
: Client implementation follows established pattern.The implementation of both overloaded methods for
getTokenDirectPurchasePrices
in the PlatformClient class correctly follows the existing pattern:
- First method allows passing metadata
- Second method provides a simpler interface without metadata
- Both return a UnaryResponse and use the appropriate callback signature
This consistent implementation ensures the new functionality integrates seamlessly with the existing client API.
packages/dapi-grpc/clients/platform/v0/java/org/dash/platform/dapi/v0/PlatformGrpc.java (5)
1165-1194
: Implementation of new gRPC method for token direct purchase prices looks goodThe addition of the
getGetTokenDirectPurchasePricesMethod
follows the established pattern of other gRPC method descriptors in the file, with proper type definitions and configuration.
1704-1709
: Well-structured implementation in the service base classThe
getTokenDirectPurchasePrices
method is properly added to thePlatformImplBase
class with the correct signature and default unimplemented handler.
2014-2020
: Properly registered method in the service bindingThe new token direct purchase prices method is correctly registered in the service definition and handler methods, maintaining consistency with other RPC methods.
Also applies to: 3379-3382, 3505-3505
2737-2742
: Client-side stub implementations are completeThe method is properly implemented in all three client stubs (blocking, async, and future), allowing consumers to interact with this new API through any of the standard gRPC patterns.
Also applies to: 2392-2398, 3112-3118
3206-3212
: Method ID assignment is correct but causes ID shiftsThe new method ID
METHODID_GET_TOKEN_DIRECT_PURCHASE_PRICES
is assigned the value of 37, which shifts the IDs of subsequent methods by 1. This is the expected behavior when adding a new method, but ensure that all client implementations are updated to align with this change.This ID shift is a breaking change for any manually implemented clients not using the generated stubs. Verify that this change is reflected in all related client implementations across different languages.
packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbrpc.h (3)
90-91
: Forward declarations for new token purchase pricing classes added correctly.The forward declarations for
GetTokenDirectPurchasePricesRequest
andGetTokenDirectPurchasePricesResponse
have been properly added, following the established pattern in the file for request/response class pairs.
297-299
: New gRPC method added to Platform2 protocol.The
getTokenDirectPurchasePricesWithMessage:responseHandler:callOptions:
method has been correctly added to the modernPlatform2
protocol with proper pragma mark and parameter types, maintaining consistency with other API methods.
642-647
:❓ Verification inconclusive
Corresponding methods added to deprecated protocol for backward compatibility.
The implementation for the older
Platform
protocol is complete with both the direct handler and the RPC helper method, ensuring backward compatibility while encouraging users to migrate to the newer API.Consider adding a documentation comment for this method similar to other methods in the protocol to maintain consistency across the API:
🏁 Script executed:
#!/bin/bash # Check if other token-related methods in the deprecated protocol include documentation comments grep -A 4 "pragma mark getToken" packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbrpc.h | grep "This method belongs to a set of APIs"Length of output: 148
Documentation Consistency Verification Required
The deprecated protocol methods for token-related functionalities are correctly implemented to maintain backward compatibility. However, our initial automated check found no evidence of existing documentation comments (e.g., “This method belongs to a set of APIs”) for these token-related methods. Please manually verify whether similar documentation exists elsewhere in the protocol. If not, consider adding a documentation comment to the following method block to ensure API consistency:
- Location:
packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbrpc.h
(lines 642-647)- Suggestion: Add a comment above the
getTokenDirectPurchasePricesWithRequest:handler:
method similar to:// This method belongs to a set of APIs that support token-based operations.
packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.m (7)
134-136
: New class declarations look consistent.
These forward declarations for the new DirectPurchasePrice event classes align well with the surrounding code and gRPC/Protobuf class naming conventions. No issues found.
268-276
: Additional Forward Declarations for Direct Purchase Prices.
These added class declarations for GetTokenDirectPurchasePrices requests/responses match the established naming pattern and appear correctly integrated.
15021-15546
: Implementation of GetTokenDirectPurchasePrices (Request/Response) Structures.
All the newly generated classes and fields (e.g., token IDs, pricing schedules, proofs, metadata) appear valid and consistent with standard Protobuf-Objective-C patterns. There are no obvious logic, concurrency, or security issues for these message definitions.
18284-18467
: Definition of UpdateDirectPurchasePriceEvent and Nested Classes.
These additions for handling fixed or variable prices, including arrays of PriceForQuantity, follow the same Protobuf structure. No anomalies detected in field indices or naming.
18767-18767
: Added dynamic property for updatePrice in TokenEvent.
This property is properly declared and seems aligned with the newly introduced UpdateDirectPurchasePriceEvent.
18778-18778
: storage struct updated for updatePrice.
The new field in the storage struct for the updatePrice event is consistent with the field definitions above.
18850-18858
: Descriptor fields extended for updatePrice.
This completes the integration of the updatePrice field into the TokenEvent descriptor. The indexing and offsets look correct.packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.h (7)
105-107
: New classes for supporting token direct purchase price events.These forward declarations properly prepare the Objective-C runtime for the new classes that will be used in the token direct purchase functionality. This follows the existing code pattern for forward declarations.
197-203
: New class forward declarations for token direct purchase pricing.These forward class declarations properly define the class hierarchy needed for handling token direct purchase prices in the protocol buffer implementation. The naming convention follows the established pattern in the codebase.
869-907
: Implementation of GetTokenDirectPurchasePricesRequest classes.This code properly implements the request structure for fetching token direct purchase prices. The class includes appropriate fields for token IDs and proof requests, following the same patterns as other request structures in the codebase.
909-1052
: Implementation of GetTokenDirectPurchasePricesResponse classes.This code correctly implements the response structure for token direct purchase prices with appropriate nested classes for different pricing models. The implementation supports both fixed pricing and variable (quantity-based) pricing schedules. The
TokenDirectPurchasePrices
container class properly organizes token price entries.
7014-7078
: Implementation of UpdateDirectPurchasePriceEvent classes.This implementation provides the event structure for updating token direct purchase prices. It correctly supports both fixed pricing and variable pricing through a pricing schedule with quantity tiers. The class offers a public note field for documenting pricing changes, which is good for transparency and audit purposes.
7200-7200
: Adding UpdatePrice field to TokenEvent enum.Properly extends the TokenEvent field number and type case enumerations to include the new UpdatePrice action at position 8. This maintains the appropriate structure of the protocol buffer definition while adding the new functionality.
Also applies to: 7212-7212
7243-7245
: Adding updatePrice property to TokenEvent class.This property declaration correctly implements the accessor for the UpdatePrice event type, which will contain the direct purchase pricing information. The comment accurately describes the purpose of this property.
packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_direct_purchase_transition_action/v0/transformer.rs (1)
157-189
: Validate zero token_count scenario.When using
SinglePrice(price_per_token)
the code saturates multiplication (price_per_token.saturating_mul(*token_count)
), which is safe from overflow, but confirm that a token_count of zero is treated appropriately. A zero token_count might allow unintended transitions or break further logic if not fully accounted for. Verify if there should be a guard or an explicit user-facing error in zero-count scenarios.packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_set_price_for_direct_purchase_transition_action/v0/transformer.rs (2)
69-88
: Align partial-state changes with atomic operations.Similar to the direct purchase transition, verify that any partial changes in
drive_operations
or ephemeral data are reverted or never committed if an error is returned. Otherwise, partial state modifications might be left in memory, especially if the code is run in a multi-threaded or asynchronous environment.
178-237
: Check for concurrency or parallel scenarios.While setting the direct purchase price, if multiple updates for the same token or contract are triggered concurrently, confirm there is locking or conflict resolution. If not, you might end up with race conditions in event of simultaneous transitions. If concurrency is not in scope, at least add documentation clarifying expected usage or assumptions about synchronization.
...atched_transition/token_transition/token_direct_purchase_transition_action/v0/transformer.rs
Show resolved
Hide resolved
...ions/document/batch_transition/batched_transition/token_direct_purchase_transition/v0/mod.rs
Outdated
Show resolved
Hide resolved
...sitions/document/batch_transition/batched_transition/token_direct_purchase_transition/mod.rs
Show resolved
Hide resolved
...nt/batch_transition/batched_transition/token_set_price_for_direct_purchase_transition/mod.rs
Show resolved
Hide resolved
...ate_transitions/document/batch_transition/batched_transition/token_transition_action_type.rs
Show resolved
Hide resolved
drive_operations: &mut Vec<LowLevelDriveOperation>, | ||
platform_version: &PlatformVersion, | ||
) -> Result<Option<TokenPricingSchedule>, Error> { | ||
let direct_query_type = if apply { |
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.
why don't use a token_direct_purchase_price_query
function?
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.
As a rule we haven't been using path queries for single key lookups.
/// - Estimated to contain 8 items, each with hash-sized keys and no child trees. | ||
/// - Estimated depth: 10 levels. | ||
/// | ||
/// This setup provides a balanced estimation model for token price operations, |
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.
Do I understand correcty that the cost depends on number of price schedules defined for a token?
Why the estimated depth is 10 levels, if it contains only 8 items?
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.
bad docs, fixed.
balance_to_remove: self.total_agreed_price(), | ||
}, | ||
)); | ||
ops.push(IdentityOperation( |
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.
I think it's irrelevant in this context, but I would prefer to first take funds from users, and then credit them with new funds/tokens.
This way, if sth goes wrong, the network will be on the safe side, and a potential loss will be on the user initiating transaction.
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.
All things put into ops are executed atomically.
event: GroupActionEvent::TokenEvent(event), | ||
})) | ||
} else { | ||
None |
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.
Is it safe to not include action details here? Where do we verify that a non-proposer doesn't submit an event with a different price than the one submitted by the proposer?
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.
In most cases it doesn't really matter, you set the action id that you agree with and sign. However to create the action Id you generally have to put the same information. Sometimes not exactly the same information, but it wouldn't really matter, because you are basically saying you agree with the proposer if you sign your transaction referencing theirs.
)? | ||
else { | ||
return Err(Error::Proof(ProofError::IncorrectProof( | ||
format!("proof did not contain token balance for identity {} expected to exist because of state transition (token burn)", owner_id)))); |
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.
double-check this is correct, at least the message refers to token burn
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: 4
♻️ Duplicate comments (13)
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v0_methods.rs (1)
1-406
: File name inconsistency persists.This file is named
v0_methods.rs
but implementsDocumentsBatchTransitionMethodsV0
for theBatchTransitionV1
struct.Consider renaming this file to better reflect its purpose, such as
batch_transition_v1_v0_methods.rs
or similar, to avoid confusion.packages/rs-dpp/src/tokens/token_pricing_schedule.rs (1)
10-47
: Enhance documentation for the SetPrices variantWhile the overall documentation for the enum is good, the documentation for the
SetPrices
variant was previously flagged as missing or insufficient. Consider expanding the current documentation to explain more about how tiered pricing works, especially regarding the minimum purchase comment./// This allows for more complex pricing structures, such as /// volume discounts or progressive pricing. The map keys /// represent token amount thresholds, and the values are the /// corresponding credit prices. /// If the first token amount is greater than 1 this means that the user can only /// purchase that amount as a minimum at a time. + /// + /// For example, a map with entries {10: 100, 50: 450, 100: 850} means: + /// - 10 tokens cost 100 credits (10 credits per token) + /// - 50 tokens cost 450 credits (9 credits per token) + /// - 100 tokens cost 850 credits (8.5 credits per token) + /// In this case, the minimum purchase amount would be 10 tokens.packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/v1/mod.rs (11)
45-59
: Docs fornew_token_mint_transition
are thorough.This function signature contains many parameters, which was flagged in a previous review (line 385) as overly complex. Consider a builder or struct to group them.
81-94
: Good documentation fornew_token_burn_transition
.As with other methods, note the large parameter list. Reiterating past review feedback to possibly reduce argument complexity in the future.
115-130
: Clear description fornew_token_transfer_transition
.This method also has many arguments, consistent with the other transitions. Consider factoring out common parameters into a dedicated struct or builder approach.
153-168
: Documentation for freezing tokens is well-organized.Again, we see the same pattern of numerous parameters. If feasible, unify them for better maintainability.
189-202
: Unfreezing tokens method is well documented.Same parameter count observation as before. Reuse a parameter container or builder pattern if practical.
223-236
: Clear docs for destroying frozen funds.Various arguments repeat across transitions. A single “TokenActionParams” struct, for example, might make sense in the future.
257-273
: Documentation for emergency actions is concise.Emergency actions naturally require broad context, but the methods are quite parameter-heavy. Consider the same approach to reduce argument complexity.
294-310
: Configuration update docs look good.As with the other transitions, monitor the function signature size for maintainability.
331-346
: Token claim transition is well described.Reiterate prior feedback on consolidating parameters to keep signatures more manageable.
366-385
: Nice addition: setting or disabling direct purchase price.The large signature pattern remains. If you decide to refactor, consider a single data structure to handle version and signing info.
403-422
: Direct purchase transition docs are detailed.Great clarity on how the user pays up to a maximum. Again, the extensive parameter list might be more convenient to manage via a specialized object.
🧹 Nitpick comments (13)
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_direct_purchase_transition/v0/mod.rs (2)
11-12
: Consider documenting re-exported identifiers.While the re-export of
IDENTIFIER_FIELDS
is appropriate, it would be helpful to add a brief explanation of what these identifier fields contain and how they're used in the context ofTokenDirectPurchaseTransition
.
27-30
: Consider renamingtotal_agreed_price
for clarity.The field name
total_agreed_price
might be slightly misleading since the comment indicates it's actually a maximum amount the user is willing to pay, not necessarily the final price. Consider a name that better reflects its role as an upper bound, such asmax_willing_to_pay
orprice_upper_limit
.packages/rs-dpp/src/tokens/token_event.rs (2)
38-50
: Extended enum documentation is helpful
The expanded doc comments clarifyingTokenEvent
usage and metadata are beneficial for future maintainers.If relevant, add a quick mention of concurrency or reentrancy considerations (e.g., “Events are purely informational, no concurrency concerns.”), if any apply in the broader system.
331-348
: Constructing historical document for price changes
The logic to serialize the optional pricing schedule is robust. Just double-check large or complex schedules for potential serialization overhead or performance pitfalls.Consider compressing or chunking large schedule data if future expansions become unwieldy.
packages/rs-drive/src/drive/tokens/estimated_costs/for_token_direct_selling_prices/v0/mod.rs (1)
1-101
: New cost estimation logic for direct selling prices
This module’s layered tree estimations help GroveDB optimize performance and fee calculations for direct purchase entries. The design of deeper layers (10-level depth) is conservative but might be overestimated.Review if 10-level depth is truly needed or if a shallower structure remains sufficient. Overestimation could affect cost projections slightly but may be acceptable if it provides a margin of safety.
packages/dapi-grpc/clients/platform/v0/python/platform_pb2_grpc.py (1)
458-463
: Add documentation to the service methodThe method implementation in the
PlatformServicer
class is currently missing documentation. All other methods have commented documentation (even if it's just "Missing associated documentation comment"), but this method should also include a brief description of its purpose related to token direct purchase pricing.def getTokenDirectPurchasePrices(self, request, context): - """Missing associated documentation comment in .proto file.""" + """Retrieves the direct purchase pricing information for specified tokens.""" context.set_code(grpc.StatusCode.UNIMPLEMENTED) context.set_details('Method not implemented!') raise NotImplementedError('Method not implemented!')packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/mod.rs (2)
1003-1062
: Needs documentation for the new methodThis method implements the ability for token owners to change direct purchase prices, matching the PR objective. However, it's missing documentation comments that would explain its purpose and parameters.
Consider adding documentation comments to describe the method's purpose and parameters, similar to other methods in this file. For example:
#[cfg(feature = "state-transition-signing")] #[allow(clippy::too_many_arguments)] +/// Creates a state transition that changes the direct purchase price of a token. +/// +/// # Arguments +/// +/// * `token_id` - The ID of the token +/// * `owner_id` - The ID of the token owner +/// * `data_contract_id` - The ID of the data contract +/// * `token_contract_position` - The position of the token in the contract +/// * `token_pricing_schedule` - Optional pricing schedule for the token +/// * `public_note` - Optional public note associated with the transition +/// * `using_group_info` - Optional group info status +/// * `identity_public_key` - The identity public key used for signing +/// * `identity_contract_nonce` - The contract nonce for the identity +/// * `user_fee_increase` - The fee increase set by the user +/// * `signer` - The signer to use for signing the transition +/// * `platform_version` - The platform version +/// * `batch_feature_version` - Optional batch feature version +/// * `config_update_feature_version` - Optional config update feature version +/// * `base_feature_version` - Optional base feature version +/// +/// # Returns +/// +/// A state transition result fn new_token_change_direct_purchase_price_transition<S: Signer>(
1065-1121
: Needs documentation for the new methodThis method implements the ability for users to directly purchase tokens, but it lacks documentation comments to explain its purpose and parameters.
Consider adding documentation comments to describe the method's purpose and parameters. For example:
#[cfg(feature = "state-transition-signing")] +/// Creates a state transition for directly purchasing tokens. +/// +/// # Arguments +/// +/// * `token_id` - The ID of the token to purchase +/// * `owner_id` - The ID of the token owner (seller) +/// * `data_contract_id` - The ID of the data contract +/// * `token_contract_position` - The position of the token in the contract +/// * `amount` - The amount of tokens to purchase +/// * `total_agreed_price` - The total price agreed for the purchase +/// * `identity_public_key` - The identity public key used for signing +/// * `identity_contract_nonce` - The contract nonce for the identity +/// * `user_fee_increase` - The fee increase set by the user +/// * `signer` - The signer to use for signing the transition +/// * `platform_version` - The platform version +/// * `batch_feature_version` - Optional batch feature version +/// * `config_update_feature_version` - Optional config update feature version +/// * `base_feature_version` - Optional base feature version +/// +/// # Returns +/// +/// A state transition result fn new_token_direct_purchase_transition<S: Signer>(packages/dapi-grpc/protos/platform/v0/platform.proto (1)
1341-1389
: Validate edge cases for variable pricing.
ThePricingSchedule
andPrice
messages allow for flexible pricing, but consider potential large-volume edge cases and ensuring the client handles integer overflow safely. Also, confirm that any discount or minimum purchase constraints are enforced at the application level.packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v1_methods.rs (4)
1-68
: Remove or conditionally compile unused imports.
Static analysis shows multiple unused imports (e.g.Credits
,IdentityNonce
,BatchedTransition
,TokenDirectPurchaseTransition
, etc.). Removing or gating them behind the appropriate feature flags clarifies intent and tidies the code.- use crate::fee::Credits; - use crate::prelude::IdentityNonce; - use crate::state_transition::batch_transition::batched_transition::BatchedTransition; - use crate::state_transition::batch_transition::{TokenDirectPurchaseTransition, TokenSetPriceForDirectPurchaseTransition}; - use crate::ProtocolError; // etc...🧰 Tools
🪛 GitHub Check: Rust packages (drive-abci) / Linting
[warning] 58-58: unused import:
crate::state_transition::batch_transition::token_set_price_for_direct_purchase_transition::TokenSetPriceForDirectPurchaseTransitionV0
warning: unused import:crate::state_transition::batch_transition::token_set_price_for_direct_purchase_transition::TokenSetPriceForDirectPurchaseTransitionV0
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v1_methods.rs:58:5
|
58 | use crate::state_transition::batch_transition::token_set_price_for_direct_purchase_transition::TokenSetPriceForDirectPurchaseTransitionV0;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[warning] 51-51: unused import:
crate::state_transition::batch_transition::token_direct_purchase_transition::TokenDirectPurchaseTransitionV0
warning: unused import:crate::state_transition::batch_transition::token_direct_purchase_transition::TokenDirectPurchaseTransitionV0
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v1_methods.rs:51:5
|
51 | use crate::state_transition::batch_transition::token_direct_purchase_transition::TokenDirectPurchaseTransitionV0;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[warning] 19-19: unused import:
crate::ProtocolError
warning: unused import:crate::ProtocolError
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v1_methods.rs:19:5
|
19 | use crate::ProtocolError;
| ^^^^^^^^^^^^^^^^^^^^
[warning] 12-12: unused imports:
TokenDirectPurchaseTransition
andTokenSetPriceForDirectPurchaseTransition
warning: unused imports:TokenDirectPurchaseTransition
andTokenSetPriceForDirectPurchaseTransition
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v1_methods.rs:12:68
|
12 | use crate::state_transition::batch_transition::{BatchTransitionV1, TokenDirectPurchaseTransition, TokenSetPriceForDirectPurchaseTransition};
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[warning] 11-11: unused import:
crate::state_transition::batch_transition::batched_transition::BatchedTransition
warning: unused import:crate::state_transition::batch_transition::batched_transition::BatchedTransition
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v1_methods.rs:11:5
|
11 | use crate::state_transition::batch_transition::batched_transition::BatchedTransition;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[warning] 6-6: unused import:
crate::prelude::IdentityNonce
warning: unused import:crate::prelude::IdentityNonce
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v1_methods.rs:6:5
|
6 | use crate::prelude::IdentityNonce;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[warning] 1-1: unused import:
crate::fee::Credits
warning: unused import:crate::fee::Credits
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v1_methods.rs:1:5
|
1 | use crate::fee::Credits;
| ^^^^^^^^^^^^^^^^^^^
267-333
: new_token_freeze_transition: consider partial freeze edge cases.
If partial freezing or multi-account freezing is introduced, you might need more robust logic for “partially” frozen amounts. Currently looks fine for a full freeze approach.
405-474
: new_token_destroy_frozen_funds_transition: highlight irreversible nature.
Destroying funds is irreversible; consider requiring a higher authorization level or more confirmations. Otherwise, the implementation is correct.
663-735
: new_token_change_direct_purchase_price_transition: handle abrupt price changes.
Use external logic or versioning to ensure that an unexpected price update doesn’t disrupt in-flight purchases. The method’s structure is correct.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (25)
packages/dapi-grpc/clients/platform/v0/java/org/dash/platform/dapi/v0/PlatformGrpc.java
(9 hunks)packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.h
(7 hunks)packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.m
(7 hunks)packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbrpc.h
(3 hunks)packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbrpc.m
(1 hunks)packages/dapi-grpc/clients/platform/v0/python/platform_pb2_grpc.py
(4 hunks)packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts
(5 hunks)packages/dapi-grpc/clients/platform/v0/web/platform_pb_service.d.ts
(3 hunks)packages/dapi-grpc/clients/platform/v0/web/platform_pb_service.js
(2 hunks)packages/dapi-grpc/protos/platform/v0/platform.proto
(4 hunks)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_direct_purchase_transition/mod.rs
(1 hunks)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_direct_purchase_transition/v0/mod.rs
(1 hunks)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_set_price_for_direct_purchase_transition/mod.rs
(1 hunks)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_transition_action_type.rs
(4 hunks)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/mod.rs
(2 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/v1/mod.rs
(1 hunks)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v0_methods.rs
(3 hunks)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v1_methods.rs
(1 hunks)packages/rs-dpp/src/tokens/token_event.rs
(4 hunks)packages/rs-dpp/src/tokens/token_pricing_schedule.rs
(1 hunks)packages/rs-drive-abci/src/query/proofs/v0/mod.rs
(1 hunks)packages/rs-drive-abci/src/query/service.rs
(2 hunks)packages/rs-drive/src/drive/tokens/estimated_costs/for_token_direct_selling_prices/v0/mod.rs
(1 hunks)packages/rs-drive/src/verify/state_transition/verify_state_transition_was_executed_with_proof/v0/mod.rs
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_transition_action_type.rs
- packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_set_price_for_direct_purchase_transition/mod.rs
- packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_direct_purchase_transition/mod.rs
- packages/dapi-grpc/clients/platform/v0/web/platform_pb_service.d.ts
- packages/rs-drive-abci/src/query/service.rs
- packages/dapi-grpc/clients/platform/v0/java/org/dash/platform/dapi/v0/PlatformGrpc.java
- packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.m
🧰 Additional context used
🧬 Code Graph Analysis (11)
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_direct_purchase_transition/v0/mod.rs (1)
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_direct_purchase_transition/v0_methods.rs (3)
base
(9-13)token_count
(29-33)total_agreed_price
(41-45)
packages/rs-dpp/src/tokens/token_event.rs (3)
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_set_price_for_direct_purchase_transition/v0/v0_methods.rs (2)
price
(27-27)price
(44-46)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_set_price_for_direct_purchase_transition/v0_methods.rs (1)
price
(34-38)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_transition.rs (2)
token_id
(209-209)token_id
(321-323)
packages/rs-dpp/src/tokens/token_pricing_schedule.rs (2)
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_set_price_for_direct_purchase_transition/v0_methods.rs (1)
price
(34-38)packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_set_price_for_direct_purchase_transition_action/v0/mod.rs (2)
price
(30-30)price
(81-83)
packages/dapi-grpc/clients/platform/v0/python/platform_pb2_grpc.py (1)
packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts (2)
GetTokenDirectPurchasePricesRequest
(6639-6654)GetTokenDirectPurchasePricesResponse
(6695-6710)
packages/rs-drive/src/verify/state_transition/verify_state_transition_was_executed_with_proof/v0/mod.rs (2)
packages/rs-drive/src/verify/tokens/verify_token_balance_for_identity_id/mod.rs (1)
verify_token_balance_for_identity_id
(47-74)packages/rs-drive/src/verify/tokens/verify_token_direct_selling_price/mod.rs (1)
verify_token_direct_selling_price
(41-66)
packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbrpc.h (2)
packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts (2)
GetTokenDirectPurchasePricesRequest
(6639-6654)GetTokenDirectPurchasePricesResponse
(6695-6710)packages/dapi-grpc/clients/platform/v0/python/platform_pb2_grpc.py (2)
getTokenDirectPurchasePrices
(458-462)getTokenDirectPurchasePrices
(1341-1355)
packages/rs-drive/src/drive/tokens/estimated_costs/for_token_direct_selling_prices/v0/mod.rs (1)
packages/rs-drive/src/drive/tokens/paths.rs (2)
token_direct_purchase_root_path
(91-96)tokens_root_path
(68-70)
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v1_methods.rs (1)
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/v1/mod.rs (11)
new_token_mint_transition
(62-79)new_token_burn_transition
(97-113)new_token_transfer_transition
(133-151)new_token_freeze_transition
(171-187)new_token_unfreeze_transition
(205-221)new_token_destroy_frozen_funds_transition
(239-255)new_token_emergency_action_transition
(276-292)new_token_config_update_transition
(313-329)new_token_claim_transition
(349-364)new_token_change_direct_purchase_price_transition
(385-401)new_token_direct_purchase_transition
(422-437)
packages/dapi-grpc/clients/platform/v0/web/platform_pb_service.js (3)
packages/dapi-grpc/clients/platform/v0/python/platform_pb2_grpc.py (3)
Platform
(725-1457)getTokenDirectPurchasePrices
(458-462)getTokenDirectPurchasePrices
(1341-1355)packages/dapi-grpc/clients/platform/v0/web/platform_pb_service.d.ts (2)
Platform
(394-439)PlatformClient
(469-860)packages/dapi-grpc/clients/platform/v0/nodejs/PlatformPromiseClient.js (1)
grpc
(1-1)
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v0_methods.rs (1)
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/mod.rs (3)
new_document_deletion_transition_from_document
(266-323)new_document_transfer_transition_from_document
(202-263)all_document_purchases_amount
(48-53)
packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.h (1)
packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts (2)
GetTokenDirectPurchasePricesRequest
(6639-6654)GetTokenDirectPurchasePricesResponse
(6695-6710)
🪛 GitHub Check: Rust packages (drive-abci) / Linting
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/mod.rs
[warning] 35-35: unused import: crate::tokens::token_pricing_schedule::TokenPricingSchedule
warning: unused import: crate::tokens::token_pricing_schedule::TokenPricingSchedule
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/mod.rs:35:5
|
35 | use crate::tokens::token_pricing_schedule::TokenPricingSchedule;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: #[warn(unused_imports)]
on by default
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v1_methods.rs
[warning] 58-58: unused import: crate::state_transition::batch_transition::token_set_price_for_direct_purchase_transition::TokenSetPriceForDirectPurchaseTransitionV0
warning: unused import: crate::state_transition::batch_transition::token_set_price_for_direct_purchase_transition::TokenSetPriceForDirectPurchaseTransitionV0
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v1_methods.rs:58:5
|
58 | use crate::state_transition::batch_transition::token_set_price_for_direct_purchase_transition::TokenSetPriceForDirectPurchaseTransitionV0;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[warning] 51-51: unused import: crate::state_transition::batch_transition::token_direct_purchase_transition::TokenDirectPurchaseTransitionV0
warning: unused import: crate::state_transition::batch_transition::token_direct_purchase_transition::TokenDirectPurchaseTransitionV0
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v1_methods.rs:51:5
|
51 | use crate::state_transition::batch_transition::token_direct_purchase_transition::TokenDirectPurchaseTransitionV0;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[warning] 19-19: unused import: crate::ProtocolError
warning: unused import: crate::ProtocolError
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v1_methods.rs:19:5
|
19 | use crate::ProtocolError;
| ^^^^^^^^^^^^^^^^^^^^
[warning] 12-12: unused imports: TokenDirectPurchaseTransition
and TokenSetPriceForDirectPurchaseTransition
warning: unused imports: TokenDirectPurchaseTransition
and TokenSetPriceForDirectPurchaseTransition
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v1_methods.rs:12:68
|
12 | use crate::state_transition::batch_transition::{BatchTransitionV1, TokenDirectPurchaseTransition, TokenSetPriceForDirectPurchaseTransition};
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[warning] 11-11: unused import: crate::state_transition::batch_transition::batched_transition::BatchedTransition
warning: unused import: crate::state_transition::batch_transition::batched_transition::BatchedTransition
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v1_methods.rs:11:5
|
11 | use crate::state_transition::batch_transition::batched_transition::BatchedTransition;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[warning] 6-6: unused import: crate::prelude::IdentityNonce
warning: unused import: crate::prelude::IdentityNonce
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v1_methods.rs:6:5
|
6 | use crate::prelude::IdentityNonce;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[warning] 1-1: unused import: crate::fee::Credits
warning: unused import: crate::fee::Credits
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v1_methods.rs:1:5
|
1 | use crate::fee::Credits;
| ^^^^^^^^^^^^^^^^^^^
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/v1/mod.rs
[warning] 7-7: unused import: crate::fee::Credits
warning: unused import: crate::fee::Credits
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/v1/mod.rs:7:5
|
7 | use crate::fee::Credits;
| ^^^^^^^^^^^^^^^^^^^
[warning] 21-21: unused import: crate::tokens::token_pricing_schedule::TokenPricingSchedule
warning: unused import: crate::tokens::token_pricing_schedule::TokenPricingSchedule
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/v1/mod.rs:21:5
|
21 | use crate::tokens::token_pricing_schedule::TokenPricingSchedule;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dapi-grpc) / Formatting
- GitHub Check: Rust packages (dapi-grpc) / Check each feature
- GitHub Check: Rust packages (dapi-grpc) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Unused dependencies
- GitHub Check: Rust packages (token-history-contract) / Linting
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Linting
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (rs-dapi-client) / Tests
- GitHub Check: Rust packages (dash-sdk) / Check each feature
- GitHub Check: Rust packages (drive-abci) / Check each feature
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive-abci) / Linting
- 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: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
🔇 Additional comments (52)
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_direct_purchase_transition/v0/mod.rs (2)
1-2
: Looks good, module organization is clean.The separation of the implementation details into a
v0_methods
module follows good software engineering practices by keeping the core struct definition separate from its methods.
33-41
: Display implementation is clean and helpful.The implementation of
Display
provides a clear, readable representation of the transition object, which will be valuable for debugging and logging.packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v0_methods.rs (3)
384-404
: Implementation of purchase amount calculation looks good.The
all_document_purchases_amount
method correctly iterates through the transitions to calculate the total amount of document purchases, with proper error handling for overflow scenarios.
172-210
:✅ Verification successful
Function implementation looks correct, but check for consistency.
The implementation of
new_document_deletion_transition_from_document
now correctly creates aDocumentDeleteTransition
. The parameter name has been updated fromtransfer_feature_version
todelete_feature_version
which matches its purpose.
🏁 Script executed:
#!/bin/bash # Check if this function is used elsewhere in the codebase to ensure all callers pass the correct parameter. rg -A 2 "new_document_deletion_transition_from_document" --glob "*.rs"Length of output: 9511
Consistency Confirmed: Parameter Renaming Verified Across Usage
The implementation of
new_document_deletion_transition_from_document
correctly creates aDocumentDeleteTransition
and now consistently uses thedelete_feature_version
parameter instead of the outdatedtransfer_feature_version
. A search across the codebase confirms that all callers—including those in test files—are correctly passing parameters in line with the updated function signature. No further changes are needed.
213-253
:✅ Verification successful
Function implementation looks correct, but check for consistency.
The implementation of
new_document_transfer_transition_from_document
now correctly creates aDocumentTransferTransition
and includes the necessaryrecipient_owner_id
parameter. The parameter name has been updated fromdelete_feature_version
totransfer_feature_version
which matches its purpose.
🏁 Script executed:
#!/bin/bash # Check if this function is used elsewhere in the codebase to ensure all callers pass the correct parameters. rg -A 2 "new_document_transfer_transition_from_document" --glob "*.rs"Length of output: 5952
Function parameter consistency has been verified.
- The implementation in
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v0_methods.rs
correctly creates aDocumentTransferTransition
using the updatedtransfer_feature_version
parameter.- A repository-wide search confirms that all call sites—including those in
packages/rs-sdk/src/platform/transition/transfer_document.rs
, various v0 implementations, and tests inpackages/rs-drive-abci
—are consistently passing the expected parameters.- The original parameter renaming (from
delete_feature_version
totransfer_feature_version
) is now uniformly applied across the codebase.packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/mod.rs (1)
7-7
:✅ Verification successful
New module addition looks good.
Adding a
v1_methods
module alongside the existingv0_methods
module is consistent with the versioning pattern and supports the new direct token selling functionality.
🏁 Script executed:
#!/bin/bash # Check the content of the v1_methods module to understand its implementation cat packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v1_methods.rs 2>/dev/null || echo "v1_methods.rs file not found" # Look for references to v1_methods in the codebase rg "v1_methods" --glob "*.rs"Length of output: 34507
Module addition verified – the new
v1_methods
module is correctly integratedThe declaration of
mod v1_methods;
inpackages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/mod.rs
and its implementation inv1_methods.rs
align with the established versioning pattern. The module is properly wired up across the codebase, and its functionality for direct token selling is supported by the references found in other parts of the system.packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts (3)
6639-6893
: Well-structured implementation of Token Direct Purchase Price APIThe implementation adds new request and response classes for retrieving token direct purchase prices. The design supports both fixed pricing and variable pricing schedules, which allows for implementing bulk purchase discounts as mentioned in the PR objectives.
The TypeScript definitions follow the same patterns used throughout the codebase, with appropriate getters, setters, and type definitions. The nested structure allows for cleanly representing complex pricing models through
PriceForQuantity
andPricingSchedule
classes.
8159-8244
: Good implementation of price update eventsThe
UpdateDirectPurchasePriceEvent
class is well designed to track price changes for tokens, supporting both fixed and variable pricing models. The class includes all necessary fields and methods, following the same pattern as other event types in the system.The inclusion of a public note field is useful for providing context about price changes, which improves transparency for users.
8424-8429
: Proper integration with existing event systemThe changes correctly integrate the new price update events into the existing
TokenEvent
class, updating type definitions and maintaining consistency with the existing architecture.packages/rs-drive-abci/src/query/proofs/v0/mod.rs (2)
261-273
: Implementing direct purchase match arm is well-structured
This newTokenTransition::DirectPurchase(_)
branch correctly separates historical querying from the balance-only check. The logic flow is clear and mirrors the approach used in other arms.
275-285
: Set price transition logic matches existing pattern
Similarly, thisTokenTransition::SetPriceForDirectPurchase(_)
arm cleanly integrates historical vs. non-historical branching. Confirm thatDrive::token_direct_purchase_price_query
is well-tested to avoid inconsistencies.Please confirm downstream usage of
Drive::token_direct_purchase_price_query
is covered by tests and no older transitions invoke now-removed logic.packages/rs-drive/src/verify/state_transition/verify_state_transition_was_executed_with_proof/v0/mod.rs (3)
43-43
: New proof result import
ImportingVerifiedTokenPricingSchedule
complements the new direct purchase pricing transitions. Ensure all references to it are consistent across other modules.
502-520
: Direct purchase proof verification
The logic to verify direct purchase transitions is consistent with how other token transitions are verified. It checks history vs. balance queries, ensuring we either have a valid historical document or an up-to-date balance.
521-540
: Ensure correctness when verifying direct purchase price
This code cleanly verifies the newly introduced direct purchase pricing schedule. Confirm that any complex pricing details are fully tested.Could you run specialized tests or an integration test verifying edge cases, e.g., zero or very large price schedules, to ensure no overflow or mismatch occurs?
packages/rs-dpp/src/tokens/token_event.rs (6)
9-9
: New imports for credits and scheduling
Adding these imports forCredits
andTokenPricingSchedule
is appropriate given the new direct purchase logic.Also applies to: 28-28
32-36
: New type aliases clarify event roles
RecipientIdentifier
andPurchaserIdentifier
provide clearer meaning to identities involved in token transactions. This improves code readability.
55-55
: Unversioned platform serialization
Marking the enum with#[platform_serialize(unversioned)]
is consistent with handling variant-level logic internally. Verify new variants do not break older assumptions about serialization.
57-64
: New token event variants
These variants—especiallyChangePriceForDirectPurchase
andDirectPurchase
—fit logically with existing transitions likeMint
,Burn
, etc. The parameters (Option<TokenPricingSchedule>
,Credits
) are well-chosen for direct purchases and price updates.Also applies to: 127-133
152-153
: Document type name additions
MappingDirectPurchase
to"directPurchase"
andChangePriceForDirectPurchase
to"directPricing"
is straightforward. Confirm that these types match the doc type definitions in the token history contract.
349-353
: Storing direct purchase transaction details
RecordingtokenAmount
andpurchaseCost
is consistent with existing patterns. This ensures clear auditing of direct purchase events.packages/dapi-grpc/clients/platform/v0/web/platform_pb_service.js (2)
337-344
: Implementation follows consistent pattern for gRPC service method definitionThe addition of the
getTokenDirectPurchasePrices
method to the Platform service follows the established pattern in the codebase, correctly specifying the method name, service reference, stream types, and request/response message types.
1523-1552
: Client implementation correctly follows existing patternsThe client-side implementation of
getTokenDirectPurchasePrices
properly follows the established pattern in the codebase, with appropriate error handling and cancellation support. The implementation is consistent with other methods in the PlatformClient prototype.packages/rs-dpp/src/tokens/token_pricing_schedule.rs (3)
1-9
: Appropriate imports for token pricing functionalityThe imports cover all necessary components for the token pricing functionality, including serialization/deserialization traits, error handling, and data structures like
BTreeMap
for ordered storage.
49-59
: Well-implemented minimum purchase logicThe
minimum_purchase_amount_and_price
method elegantly handles both pricing variants, providing a default fallback if theSetPrices
map is empty. This is a good utility function for clients to determine the minimum entry point for token purchases.
61-79
: Clear display implementation for debuggingThe implementation of the
Display
trait provides a clean, readable string representation of the pricing schedule, which will be helpful for debugging, logging, and error messages.packages/dapi-grpc/clients/platform/v0/python/platform_pb2_grpc.py (3)
197-201
: Client stub correctly defines the new methodThe
getTokenDirectPurchasePrices
method is properly added to thePlatformStub
class, following the consistent pattern of the other methods in terms of serialization/deserialization and API path definition.
683-687
: Method handler correctly registeredThe RPC method handler for
getTokenDirectPurchasePrices
is properly registered in theadd_PlatformServicer_to_server
function, maintaining consistency with other method handlers.
1340-1356
: Static method follows consistent patternThe static method implementation for
getTokenDirectPurchasePrices
follows the same pattern as other methods in thePlatform
class, correctly setting up the gRPC channel with appropriate parameters and serialization/deserialization functions.packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbrpc.m (1)
858-876
: LGTM: New RPC methods for token direct purchase pricingThe implementation properly adds the required methods to support retrieving token direct purchase prices, following the same pattern established for other RPC methods in this file:
- A method to initiate the RPC call
- A method to create and return a not-yet-started RPC object
- A method supporting the newer gRPC interface with message and handler objects
These additions align well with the PR objective of enabling contract owners to sell tokens directly to users.
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/mod.rs (1)
35-35
: LGTM: Necessary import for token pricingThis import correctly adds the TokenPricingSchedule type which is used in the new direct purchase functions.
🧰 Tools
🪛 GitHub Check: Rust packages (drive-abci) / Linting
[warning] 35-35: unused import:
crate::tokens::token_pricing_schedule::TokenPricingSchedule
warning: unused import:crate::tokens::token_pricing_schedule::TokenPricingSchedule
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/mod.rs:35:5
|
35 | use crate::tokens::token_pricing_schedule::TokenPricingSchedule;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note:#[warn(unused_imports)]
on by defaultpackages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbrpc.h (3)
88-89
: LGTM: New request/response class forward declarationsThese forward declarations are correctly added for the new token direct purchase prices functionality.
291-293
: LGTM: New method declaration in Platform2 protocolThis method declaration properly follows the established pattern for gRPC unary calls in the newer Protocol2 interface.
629-634
: LGTM: New method declarations in Platform protocolThese method declarations properly follow the pattern for the older Platform protocol interface, including both the direct call method and the RPC method that returns the not-yet-started call object.
packages/dapi-grpc/protos/platform/v0/platform.proto (1)
62-62
: RPC naming consistency looks good.
No issues found with the newly added RPC. It aligns well with existing naming conventions likegetToken{...}
methods.packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/v1_methods.rs (8)
69-137
: new_token_mint_transition logic looks consistent.
The method correctly creates aTokenMintTransition
, wraps it in aBatchTransitionV1
, and applies the external signature. This pattern matches other token transitions.
139-208
: new_token_burn_transition approach is sound.
Code consistently follows the batch-wrapping pattern and signing approach. Just ensure thorough testing for boundary conditions (e.g., burning amounts exceeding current balance).Would you like a helper script to verify that burned amounts never exceed the user's current balance in the test suite?
210-264
: new_token_transfer_transition has robust encryption options.
The presence ofshared_encrypted_note
andprivate_encrypted_note
is commendable. Confirm that encryption keys are validated externally before this call.
336-402
: new_token_unfreeze_transition: similarly consistent.
Ensure events and transitional states (freeze → unfreeze) are thoroughly tested. The method signature is clean and consistent with other transitions.
477-544
: new_token_emergency_action_transition: ensure restricted usage.
Emergency actions (pause/resume) can be powerful. Validate appropriate triggers and permissions at the higher-level logic.Would you like a script to scan for references to this emergency transition to confirm it’s only invoked by privileged roles?
547-614
: new_token_config_update_transition with CRITICAL security level.
Raising the security level for config updates is a best practice. The pattern is consistent with the other transitions.
617-660
: new_token_claim_transition: routine approach is consistent.
The claim logic and final signature approach appear to align with your standard batch pattern.
738-782
: new_token_direct_purchase_transition: watch for integer overflows.
Confirm that largeamount
ortotal_agreed_price
values won’t exceed theuint64
range. The rest matches the overall token transition pattern.packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.h (8)
105-107
: New classes for token direct purchase price events added.These class declarations are necessary to support the token direct selling feature, specifically for events that update pricing.
187-193
: New token direct purchase price request/response classes added.These class declarations properly support the direct token selling functionality, providing the necessary structures to retrieve pricing information.
5498-5561
: GetTokenDirectPurchasePricesRequest class implementation looks good.This implementation follows the established pattern for request messages in the protocol buffers. It correctly includes fields for token IDs and proof requests.
5562-5594
: GetTokenDirectPurchasePricesResponse class implementation is correct.The response class follows the standard structure using the version one-of pattern, which is consistent with other response classes in the codebase.
5595-5651
: GetTokenDirectPurchasePricesResponseV0 implementation is well-structured.This class properly implements the result one-of pattern to handle both successful responses and proofs, with additional metadata.
5652-5709
: Price models are properly implemented.The implementation supports both fixed prices and variable pricing schedules, allowing for the bulk purchase discounts mentioned in the PR objectives.
6644-6708
: UpdateDirectPurchasePriceEvent implementation looks good.This event class correctly implements the pricing models and includes a public note field, following the pattern of other event classes in the codebase.
6830-6830
: TokenEvent properly updated to include the new UpdatePrice field.The TokenEvent class has been correctly extended to support the new UpdatePrice event type, maintaining consistency with the existing event handling pattern.
Also applies to: 6873-6874
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/v1/mod.rs (2)
33-43
: Doc-block addition for the trait is clear.Thank you for detailing the purpose and scope of this trait. These top-level docs help make it more approachable for new contributors.
21-21
:✅ Verification successful
Gate or remove this import if it's unused.
Similar to
Credits
,TokenPricingSchedule
is also flagged as unused unlessstate-transition-signing
is enabled. Double-check if it should also be conditionally imported or removed.
🏁 Script executed:
#!/bin/bash # Check if `TokenPricingSchedule` is referenced outside the "state-transition-signing" feature rg -A 5 $'TokenPricingSchedule'Length of output: 62365
TokenPricingSchedule Import Usage Confirmed
After verifying the usage across the repository, we found that the
TokenPricingSchedule
type is referenced in multiple areas—including in the file under review—even outside of thestate-transition-signing
conditional. In particular, its presence in function signatures (as seen in thebatch_transition/methods/v1/mod.rs
file) and its usage in modules like token events and proof results confirm that this import is actively required. There’s no need to gate or remove it based on thestate-transition-signing
feature flag.🧰 Tools
🪛 GitHub Check: Rust packages (drive-abci) / Linting
[warning] 21-21: unused import:
crate::tokens::token_pricing_schedule::TokenPricingSchedule
warning: unused import:crate::tokens::token_pricing_schedule::TokenPricingSchedule
--> packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/v1/mod.rs:21:5
|
21 | use crate::tokens::token_pricing_schedule::TokenPricingSchedule;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
...ions/document/batch_transition/batched_transition/token_direct_purchase_transition/v0/mod.rs
Show resolved
Hide resolved
...ions/document/batch_transition/batched_transition/token_direct_purchase_transition/v0/mod.rs
Show resolved
Hide resolved
...es/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/v1/mod.rs
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.
Actionable comments posted: 0
🧹 Nitpick comments (12)
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/v1/mod.rs (12)
34-44
: Helpful trait-level documentation.The expanded doc comments clearly describe the trait’s purpose and usage context. Consider adding quick example snippets to illustrate how clients can invoke these methods.
46-80
: Large parameter list warning.While the mint transition method is clearly documented, there are many parameters. It might be more maintainable to encapsulate related fields into a struct or builder pattern.
82-114
: Burn transition method details look correct.Same note as above regarding numerous parameters. Consider grouping them to simplify the function signature.
116-152
: Transfer transition is well-defined.The approach is consistent. The large parameter set might be streamlined with a dedicated data structure to reduce function argument verbosity.
154-188
: Freeze transition method.The logic and documentation read well. Again, the number of parameters is extensive, which can be refactored for clarity.
190-222
: Unfreeze transition method.Following the same pattern: descriptive, but quite a few parameters. A cohesive parameter object may help.
224-256
: Destroy frozen funds transition.The method is clearly documented; consider modularizing or grouping parameters to align with consistent styling in the rest of the code.
258-293
: Emergency action transition.Documentation is thorough, and emergency actions are clearly separated out. Same suggestion regarding parameter consolidation.
295-330
: Token config update transition.Nicely documented. If more updates are expected, a configuration struct might reduce repetitive parameters.
332-365
: Claim transition.Maintains the same style as other transitions. The abundance of parameters could be addressed with a more compact pattern.
367-403
: Change direct purchase price method is well structured.This effectively sets pricing logic. The parameter approach is consistent with the other methods, so the same grouping suggestion applies here.
404-439
: Direct purchase transition.The final method thoroughly covers direct purchase flows. Everything appears consistent, but again, consider grouping parameters for improved readability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/v1/mod.rs
(12 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: Rust packages (dapi-grpc) / Unused dependencies
- GitHub Check: Rust packages (dapi-grpc) / Tests
- GitHub Check: Rust packages (drive-abci) / Formatting
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (dapi-grpc) / Linting
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Rust packages (rs-dapi-client) / Check each feature
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Linting
- GitHub Check: Rust packages (dash-sdk) / Check each feature
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (rs-dapi-client) / Linting
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build JS packages / Build JS
🔇 Additional comments (2)
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/v1/mod.rs (2)
8-9
: Confirming import usage.This import is now utilized in the new direct purchase transition (
total_agreed_price: Credits
). Looks good — it's properly gated under thestate-transition-signing
feature and no longer unused.
22-22
: New import for pricing schedules.The
TokenPricingSchedule
import is correctly referenced in the direct purchase price transition method. No issues here.
Merging this in because of tight deadline after most comments were dealt with, other comments can be fixed in new PR. |
Issue being fixed or feature implemented
We needed the ability for contract owners to sell tokens directly to clients.
What was done?
Added the ability for contract owners to sell tokens as a direct purchase.
Contract owners or the party responsible can set the price of a token.
Users can buy this token.
Discounts can be applied if buying more than one token at a time with a price schedule.
The party responsible for token prices (generally contract owner) can stop selling the token.
The party responsible for token prices (generally contract owner) can set a minimum amount of tokens that can be bought at a time.
How Has This Been Tested?
Added unit tests.
Breaking Changes
A new big feature, not really breaking though.
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
Summary by CodeRabbit
Platform
service with new methods for retrieving token direct purchase prices.