feat(dpp): token configuration presets #2561
Conversation
WalkthroughThis update introduces a new preset-based system for constructing default token configurations in the codebase. It adds an enum to categorize feature sets, a struct to encapsulate preset features and authorized action takers, and several methods to generate change control and distribution rules according to these presets. The logic for creating default token configurations is centralized and modularized, replacing previous hardcoded defaults. Additionally, a utility method is added to uniformly set history-keeping rules, and the Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant TokenConfigurationPreset
participant TokenConfigurationV0
Caller->>TokenConfigurationPreset: create with features and action_taker
Caller->>TokenConfigurationPreset: token_configuration_v0(params)
TokenConfigurationPreset->>TokenConfigurationPreset: generate change control rules
TokenConfigurationPreset->>TokenConfigurationPreset: generate distribution rules
TokenConfigurationPreset->>TokenConfigurationV0: construct TokenConfigurationV0 with rules and params
TokenConfigurationV0-->>Caller: return configured instance
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🧰 Additional context used🧠 Learnings (1)📓 Common learnings⏰ Context from checks skipped due to timeout of 90000ms (20)
🔇 Additional comments (12)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/rs-dpp/src/data_contract/associated_token/token_keeps_history_rules/v0/mod.rs (1)
38-49:default_for_keeping_all_history– considerconst fn+ clearer namingThe helper is useful, but:
- The word default is misleading because the caller supplies the value. Something like
uniform(keeps_all_history: bool)(ornew_all(…)) expresses intent better.- Marking it
const fnallows compile‑time evaluation in most call‑sites:-impl TokenKeepsHistoryRulesV0 { - pub fn default_for_keeping_all_history(keeps_all_history: bool) -> TokenKeepsHistoryRulesV0 { +impl TokenKeepsHistoryRulesV0 { + pub const fn uniform(keeps_all_history: bool) -> Self {No behaviour changes, but readability and usability improve.
packages/rs-dpp/src/data_contract/associated_token/token_configuration/v0/mod.rs (2)
329-364:token_configuration_v0hard‑codesallow_transfer_to_frozen_balance = trueFree transfers into frozen accounts can be a security/UX decision that callers may want to opt out of.
Adding a boolean parameter (mirroringwith_direct_pricing) or exposing a.with_allow_transfer_to_frozen_balance(bool)builder would make the preset system more flexible.
369-383: Magic numbers & decimals indefault_most_restrictive
base_supplyis hard‑coded to100_000anddecimalsto8. These magic numbers may surprise library users expecting a “blank” preset.Propose exposing them as constants or parameters:
pub fn default_most_restrictive( base_supply: TokenAmount, decimals: u8, ) -> Self { … }Keeps the convenience while avoiding silent assumptions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/rs-dpp/src/data_contract/associated_token/token_configuration/v0/mod.rs(3 hunks)packages/rs-dpp/src/data_contract/associated_token/token_keeps_history_rules/v0/mod.rs(1 hunks)packages/rs-dpp/src/data_contract/change_control_rules/authorized_action_takers.rs(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: QuantumExplorer
PR: dashpay/platform#2257
File: packages/rs-drive-abci/src/mimic/test_quorum.rs:159-164
Timestamp: 2024-11-20T16:16:01.830Z
Learning: QuantumExplorer prefers not to receive auto-generated messages asking to post on social media.
⏰ Context from checks skipped due to timeout of 90000ms (18)
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dpp) / Formatting
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (drive-abci) / Check each feature
- GitHub Check: Rust packages (drive-abci) / Tests
- 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 (drive) / Tests
- GitHub Check: Rust packages (drive-abci) / Linting
- GitHub Check: Rust packages (drive-abci) / Formatting
- GitHub Check: Rust packages (drive) / 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 Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build JS packages / Build JS
🔇 Additional comments (3)
packages/rs-dpp/src/data_contract/change_control_rules/authorized_action_takers.rs (1)
11-13:Copyderive could fail to compile and/or change semanticsAdding
Copyrequires that every payload type in every variant (Identifier,GroupContractPosition) also implementsCopy.
Please double‑check that these types already derive/implementCopy; otherwise the crate will fail to compile.
Even if compilation succeeds, turning a formerlyClone‑only enum intoCopymeans the compiler will silently duplicate values instead of moving them, which can mask logic bugs when mutability is later introduced.If any inner type is not trivially copyable or you want to enforce move semantics, drop the
Copyderive:-#[derive(Serialize, Deserialize, Decode, Encode, Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Default)] +#[derive(Serialize, Deserialize, Decode, Encode, Debug, Clone, PartialEq, Eq, PartialOrd, Default)]Would you run
cargo check --workspaceto ensure the new derive compiles across all dependent crates?packages/rs-dpp/src/data_contract/associated_token/token_configuration/v0/mod.rs (2)
178-196: Visibility ofTokenConfigurationPreset{Features,}ispubbut inner fields are private
TokenConfigurationPresetFeaturesispuband the structTokenConfigurationPresetitself ispub, yet its fields are private.
Because consumers cannot construct the struct manually, you should either:
- Expose a constructor (builder/static
new()), or- Make the fields
pub(crate)/pubto avoid frustration.Right now the only entry‑point is the preset methods implemented further below; confirm this matches the intended API surface.
296-327:with_direct_pricinggate only affects change rules – typo or missing logic?When
with_direct_pricing == falseyou still set:minting_allow_choosing_destination: true,and leave
perpetual_distribution&pre_programmed_distributionuntouched.
Is it intentional that the token can not be directly priced yet the destination for newly minted tokens can still be chosen freely?If direct pricing is disabled, consider also setting
minting_allow_choosing_destinationtofalseor document why the two concepts are orthogonal.
packages/rs-dpp/src/data_contract/associated_token/token_configuration/v0/mod.rs
Show resolved
Hide resolved
2 participants
Issue being fixed or feature implemented
We added token config presets to make selecting a token configuration easier.
How Has This Been Tested?
Not consensus code. Only presets.
Breaking Changes
None
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
New Features
Refactor