-
Notifications
You must be signed in to change notification settings - Fork 44
fix(platform)!: token distribution fixes and tests #2494
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
|
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 (
|
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: 3
🔭 Outside diff range comments (1)
packages/rs-drive/src/drive/tokens/distribution/mark_pre_programmed_release_as_distributed/mod.rs (1)
38-49:⚠️ Potential issueParameter changes need documentation update
The method signature has been updated to use
recipient_idinstead of the previous parameters, but the function documentation (lines 22-24) still references the old parametersowner_idandidentity_idwhich no longer exist in the signature.Update the documentation to reflect the new signature:
/// # Parameters /// /// - `token_id`: The unique 32‑byte identifier of the token. -/// - `owner_id`: The unique 32‑byte identifier of the owner initiating the distribution. -/// - `identity_id`: The 32‑byte identity identifier for which the pre‑programmed release was scheduled. +/// - `recipient_id`: The unique 32‑byte identifier of the recipient of the distribution. /// - `release_time`: The scheduled release time (in milliseconds).
🧹 Nitpick comments (23)
packages/rs-platform-version/src/version/system_limits/v1.rs (1)
12-12: New system limit added for token redemption cyclesAdding a maximum limit for token redemption cycles is important to prevent potential abuse or resource exhaustion in token distribution processes.
Consider adding a documentation comment explaining what token redemption cycles are and why the limit is set to 128. This would make the code more maintainable and help future developers understand the purpose of this limit.
pub const SYSTEM_LIMITS_V1: SystemLimits = SystemLimits { estimated_contract_max_serialized_size: 16384, max_field_value_size: 5120, //5 KiB max_state_transition_size: 20480, //20 KiB max_transitions_in_documents_batch: 1, withdrawal_transactions_per_block_limit: 4, retry_signing_expired_withdrawal_documents_per_block_limit: 1, max_withdrawal_amount: 50_000_000_000_000, //500 Dash max_contract_group_size: 256, + // Maximum number of token redemption cycles allowed to prevent resource exhaustion max_token_redemption_cycles: 128, };packages/rs-drive/src/drive/tokens/distribution/mod.rs (1)
15-16: Module commented out instead of being removed.The module
set_perpetual_distribution_next_event_for_identity_idhas been commented out rather than properly removed. This could lead to confusion for future maintainers as commented out code is often overlooked during refactoring.Consider properly removing the commented lines if this module is no longer needed:
-// #[cfg(feature = "server")] -// mod set_perpetual_distribution_next_event_for_identity_id;Alternatively, if this is temporary, add a comment explaining why it's commented out rather than removed.
packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/methods/v0/mod.rs (2)
24-25: Outdated comment after return type changeThe comment on line 24 states "we use u64 as a catch-all for any type of interval", but the method now returns
RewardDistributionMomentinstead ofu64.Update the comment to reflect the new return type:
- /// we use u64 as a catch-all for any type of interval we might have, eg time, block or epoch + /// Returns the next distribution moment based on the provided block information
27-27: New method for retrieving current intervalThe addition of the
current_intervalmethod complements the existingnext_intervalmethod, providing a more complete API.Consider adding a documentation comment for this method to maintain consistency with the rest of the API:
+ /// Returns the current distribution moment based on the provided block information fn current_interval(&self, block_info: &BlockInfo) -> RewardDistributionMoment;packages/rs-drive/src/drive/tokens/distribution/fetch/pre_programmed_distribution_last_paid_time_ms/v0/mod.rs (1)
14-32: Revisit the doc comment to align it with the actual return type.The doc references returning
Some(RewardDistributionMoment)but the code actually returnsSome(TimestampMillis)as au64. Updating the documentation will improve accuracy and prevent confusion for future maintainers.packages/rs-drive/src/drive/tokens/estimated_costs/for_token_pre_programmed_distribution/v0/mod.rs (1)
27-29: Documentation needs update for the optional times parameterThe parameter type has changed to
Option<I>, but the documentation on line 25 hasn't been updated to reflect that this parameter is now optional.Update the documentation to reflect that
timesis now optional:/// # Parameters /// /// - `token_id`: The identifier for the token. -/// - `times`: A vector of timestamps (in milliseconds) for which pre-programmed distributions exist. +/// - `times`: An optional vector of timestamps (in milliseconds) for which pre-programmed distributions exist. /// - `estimated_costs_only_with_layer_info`: A mutable hashmap that holds estimated layer information.packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/distribution_function/evaluate.rs (2)
9-10: Consider providing a usage example in the doc comments.
Adding simple examples helps users quickly understand how to pass the newcontract_registration_stepparameter.
114-156: Refactored linear function logic is comprehensive.
- The use of
saturating_suband careful overflow checks are commendable.- Check that large negative slopes do not cause repeated clamping to 0 if a lower bound is not provided.
packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_claim_transition_action/v0/transformer.rs (1)
2-2: Avoid unused importstd::process::id; check if it’s required.
If not used, removing it reduces clutter.packages/rs-drive/src/drive/tokens/paths.rs (1)
217-228: Fix doc comment for pre-programmed distribution function.The doc comment says "The path for the token perpetual distributions tree," but this function actually returns the path for the pre-programmed distribution last-claimed time. Update the doc comment to reflect the correct functionality.
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/distribution/pre_programmed.rs (2)
14-187: Improve reuse of setup logic in first test.In
test_token_pre_programmed_distribution_two_claims, the setup patterns (creating identities, contracts, and performing claims) are repeated across other tests. Extracting common setup steps into a helper method may reduce duplication and enhance maintainability.
750-870: Consider clarifying combination of distributions.The final test (
test_token_pre_programmed_distribution_claim_no_pre_programmed_rewards_for_recipient_when_they_have_perpetual) checks mixed distribution rules. It might be beneficial to add explicit inline comments explaining why the perpetual rewards do not affect the pre-programmed claims or vice versa, to minimize confusion for future maintainers.packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/reward_distribution_type/mod.rs (1)
139-193: Newmax_cycle_momentmethod.
This saturating approach for capping reward distribution is logical. If partial overflows must be detected rather than silently clamped, consider using checked arithmetic in future iterations.packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/reward_distribution_moment/mod.rs (1)
492-665: Extensive test coverage.
Mismatched distributions, zero-step, and boundary checks are well-covered. Consider adding tests for near-overflow scenarios to provide comprehensive arithmetic validation.packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/distribution/perpetual/time_based.rs (1)
503-690: Potential improvement for repetitive test code.
This test function nearly duplicates the structure in other tests (e.g., creating a claim transition, processing it, checking balances). Consider extracting a helper function or macro to reduce duplication and improve maintainability across similar tests.packages/rs-drive/src/drive/tokens/mint/mod.rs (1)
21-34: Add documentation forallow_saturation.
The new parameter is introduced but not documented within the method doc comment. Provide a brief explanation of its significance to ensure clarity for future maintainers./// Mints (issues) new tokens by increasing the total supply and adding them to an identity's balance. -pub fn token_mint( +/// +/// # Parameters +/// * `allow_saturation` - If true, ... +pub fn token_mint(packages/rs-drive/src/drive/tokens/system/add_to_token_total_supply/mod.rs (2)
20-22: Consider documenting the newallow_saturationparameter.
Addingallow_saturationand switchingamounttoTokenAmountis a great step toward robust handling of large amounts. However, these changes would benefit from additional documentation explaining what saturation means in this context and why it’s sometimes desirable.Also applies to: 27-27
57-59: Align parameter ordering and provide usage clarity in docs.
Inadd_to_token_total_supply_add_to_operations, the newallow_saturationparameter is also introduced. Confirm the parameter order matches the analogous function to minimize confusion, and ensure the doc comments clarify how to use these parameters effectively.Also applies to: 64-64
packages/rs-drive/src/drive/tokens/system/add_to_token_total_supply/v0/mod.rs (3)
23-23: Consider robust validation when switching toTokenAmount.
Ensuringallow_saturationlogic is well understood is crucial. Provide clarity on any upper bounds or edge cases that might trigger a saturating add versus an error.You could, for instance, factor out the logic of saturating vs. non-saturating addition into a helper function for improved readability:
+fn add_amount(current: i64, amount: i64, allow_saturation: bool) -> Result<i64, DriveError> { + if allow_saturation { + Ok(current.saturating_add(amount)) + } else { + current.checked_add(amount).ok_or_else(|| { + DriveError::CorruptedCodeExecution("overflow in non-saturating addition") + }) + } +}Also applies to: 28-28, 31-31, 36-36, 51-51
68-68: Ensure minimal duplication of logic.
add_to_token_total_supply_operations_v0is called here, and repeated logic about saturation is also performed in these lines. Consider centralizing or reusing code to unify overflow checks and operation building steps.Also applies to: 72-72, 84-85
123-160: Thorough overflow handling.
The saturating vs. checked addition logic is clearly delineated. The code fails early if an overflow risk is detected withchecked_addor if the minted amount exceeds thei64::MAX. This is a robust approach, but ensure you have:
- Adequate test coverage for boundary cases (e.g. near integer limits).
- Clear documentation about why
i64::MAXis the cap.packages/rs-drive/src/drive/tokens/distribution/add_pre_programmed_distribution/v0/mod.rs (1)
194-202: Added tracking for identity's last pre-programmed distribution claim.Added a new tree insertion for tracking the last claim timestamp for each identity in pre-programmed distributions. This is an important addition that enables proper management of token distribution claims, preventing duplicate claims and maintaining distribution state.
This enhancement is part of a broader token distribution fix that ensures proper tracking of claims. Consider also implementing validation to ensure that claimed distributions are properly recorded to avoid potential race conditions.
packages/rs-drive-abci/src/execution/platform_events/fee_pool_outwards_distribution/add_epoch_pool_to_proposers_payout_operations/mod.rs (1)
42-42: Switched from Vec to BTreeMap for proposer rewards.Changed the return type from a vector of tuples
Vec<(Identifier, u64)>to aBTreeMap<Identifier, u64>for storing proposer identifiers and their associated rewards.This is a significant architectural improvement as BTreeMap provides:
- O(log n) lookup times compared to O(n) with vectors
- Automatic sorting of keys (identifiers)
- Deduplication of identifiers (prevents duplicate entries)
- More idiomatic access patterns through key-value pairs
This change likely requires corresponding updates in the implementation and any code that consumes this interface.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (79)
packages/rs-dpp/src/block/finalized_epoch_info/getters.rs(2 hunks)packages/rs-dpp/src/block/finalized_epoch_info/v0/getters.rs(3 hunks)packages/rs-dpp/src/block/finalized_epoch_info/v0/mod.rs(2 hunks)packages/rs-dpp/src/data_contract/associated_token/token_configuration_convention/accessors/mod.rs(0 hunks)packages/rs-dpp/src/data_contract/associated_token/token_configuration_localization/mod.rs(0 hunks)packages/rs-dpp/src/data_contract/associated_token/token_distribution_key.rs(3 hunks)packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/distribution_function/encode.rs(15 hunks)packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/distribution_function/evaluate.rs(57 hunks)packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/distribution_function/evaluate_interval.rs(1 hunks)packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/distribution_function/mod.rs(12 hunks)packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/distribution_function/reward_ratio.rs(1 hunks)packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/distribution_function/validation.rs(73 hunks)packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/methods/mod.rs(2 hunks)packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/methods/v0/mod.rs(2 hunks)packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/reward_distribution_moment/mod.rs(5 hunks)packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/reward_distribution_type/accessors.rs(0 hunks)packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/reward_distribution_type/evaluate_interval.rs(2 hunks)packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/reward_distribution_type/mod.rs(3 hunks)packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/v0/methods.rs(1 hunks)packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v0/mod.rs(0 hunks)packages/rs-dpp/src/data_contract/document_type/property/mod.rs(1 hunks)packages/rs-dpp/src/data_contract/v0/methods/schema.rs(0 hunks)packages/rs-dpp/src/data_contract/v0/serialization/mod.rs(0 hunks)packages/rs-dpp/src/data_contract/v1/methods/schema.rs(0 hunks)packages/rs-dpp/src/data_contract/v1/serialization/mod.rs(0 hunks)packages/rs-dpp/src/errors/consensus/codes.rs(2 hunks)packages/rs-dpp/src/errors/consensus/state/state_error.rs(2 hunks)packages/rs-dpp/src/errors/consensus/state/token/invalid_token_claim_no_current_rewards.rs(1 hunks)packages/rs-dpp/src/errors/consensus/state/token/invalid_token_claim_wrong_claimant.rs(1 hunks)packages/rs-dpp/src/errors/consensus/state/token/mod.rs(2 hunks)packages/rs-dpp/src/errors/protocol_error.rs(1 hunks)packages/rs-drive-abci/src/execution/platform_events/fee_pool_outwards_distribution/add_epoch_pool_to_proposers_payout_operations/mod.rs(2 hunks)packages/rs-drive-abci/src/execution/platform_events/fee_pool_outwards_distribution/add_epoch_pool_to_proposers_payout_operations/v0/mod.rs(3 hunks)packages/rs-drive-abci/src/execution/platform_events/fee_pool_outwards_distribution/find_oldest_epoch_needing_payment/v0/mod.rs(0 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/distribution/mod.rs(1 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/distribution/perpetual.rs(0 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/distribution/perpetual/block_based.rs(1 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/distribution/perpetual/mod.rs(1 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/distribution/perpetual/time_based.rs(1 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/distribution/pre_programmed.rs(1 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/mod.rs(40 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_create/state/v0/mod.rs(0 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_update/state/v0/mod.rs(0 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/mod.rs(3 hunks)packages/rs-drive/src/drive/contract/insert/insert_contract/v1/mod.rs(0 hunks)packages/rs-drive/src/drive/initialization/v1/mod.rs(1 hunks)packages/rs-drive/src/drive/tokens/distribution/add_perpetual_distribution/mod.rs(0 hunks)packages/rs-drive/src/drive/tokens/distribution/add_perpetual_distribution/v0/mod.rs(2 hunks)packages/rs-drive/src/drive/tokens/distribution/add_pre_programmed_distribution/v0/mod.rs(3 hunks)packages/rs-drive/src/drive/tokens/distribution/fetch/mod.rs(1 hunks)packages/rs-drive/src/drive/tokens/distribution/fetch/pre_programmed_distribution_last_paid_time_ms/mod.rs(1 hunks)packages/rs-drive/src/drive/tokens/distribution/fetch/pre_programmed_distribution_last_paid_time_ms/v0/mod.rs(1 hunks)packages/rs-drive/src/drive/tokens/distribution/mark_perpetual_release_as_distributed/mod.rs(2 hunks)packages/rs-drive/src/drive/tokens/distribution/mark_perpetual_release_as_distributed/v0/mod.rs(1 hunks)packages/rs-drive/src/drive/tokens/distribution/mark_pre_programmed_release_as_distributed/mod.rs(2 hunks)packages/rs-drive/src/drive/tokens/distribution/mark_pre_programmed_release_as_distributed/v0/mod.rs(7 hunks)packages/rs-drive/src/drive/tokens/distribution/mod.rs(1 hunks)packages/rs-drive/src/drive/tokens/estimated_costs/for_token_perpetual_distribution/mod.rs(0 hunks)packages/rs-drive/src/drive/tokens/estimated_costs/for_token_perpetual_distribution/v0/mod.rs(0 hunks)packages/rs-drive/src/drive/tokens/estimated_costs/for_token_pre_programmed_distribution/mod.rs(2 hunks)packages/rs-drive/src/drive/tokens/estimated_costs/for_token_pre_programmed_distribution/v0/mod.rs(2 hunks)packages/rs-drive/src/drive/tokens/mint/mod.rs(6 hunks)packages/rs-drive/src/drive/tokens/mint/v0/mod.rs(6 hunks)packages/rs-drive/src/drive/tokens/mint_many/v0/mod.rs(1 hunks)packages/rs-drive/src/drive/tokens/paths.rs(2 hunks)packages/rs-drive/src/drive/tokens/system/add_to_token_total_supply/mod.rs(7 hunks)packages/rs-drive/src/drive/tokens/system/add_to_token_total_supply/v0/mod.rs(5 hunks)packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_claim_transition.rs(2 hunks)packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_mint_transition.rs(1 hunks)packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_claim_transition_action/v0/mod.rs(1 hunks)packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_claim_transition_action/v0/transformer.rs(8 hunks)packages/rs-drive/src/util/batch/drive_op_batch/token.rs(5 hunks)packages/rs-drive/src/util/batch/grovedb_op_batch/mod.rs(0 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)packages/rs-platform-version/src/version/mocks/v2_test.rs(1 hunks)packages/rs-platform-version/src/version/system_limits/mod.rs(1 hunks)packages/rs-platform-version/src/version/system_limits/v1.rs(1 hunks)packages/wasm-dpp/src/errors/consensus/consensus_error.rs(2 hunks)
💤 Files with no reviewable changes (17)
- packages/rs-drive/src/drive/tokens/estimated_costs/for_token_perpetual_distribution/mod.rs
- packages/rs-dpp/src/data_contract/associated_token/token_configuration_localization/mod.rs
- packages/rs-dpp/src/data_contract/v0/serialization/mod.rs
- packages/rs-dpp/src/data_contract/v0/methods/schema.rs
- packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_update/state/v0/mod.rs
- packages/rs-drive/src/drive/tokens/estimated_costs/for_token_perpetual_distribution/v0/mod.rs
- packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/reward_distribution_type/accessors.rs
- packages/rs-dpp/src/data_contract/v1/serialization/mod.rs
- packages/rs-drive/src/util/batch/grovedb_op_batch/mod.rs
- packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v0/mod.rs
- packages/rs-dpp/src/data_contract/v1/methods/schema.rs
- packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/distribution/perpetual.rs
- packages/rs-drive-abci/src/execution/platform_events/fee_pool_outwards_distribution/find_oldest_epoch_needing_payment/v0/mod.rs
- packages/rs-drive/src/drive/contract/insert/insert_contract/v1/mod.rs
- packages/rs-dpp/src/data_contract/associated_token/token_configuration_convention/accessors/mod.rs
- packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_create/state/v0/mod.rs
- packages/rs-drive/src/drive/tokens/distribution/add_perpetual_distribution/mod.rs
🧰 Additional context used
🧠 Learnings (3)
packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/methods/mod.rs (1)
Learnt from: QuantumExplorer
PR: dashpay/platform#2450
File: packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/v0/methods.rs:10-12
Timestamp: 2025-03-13T13:59:31.360Z
Learning: Block interval calculations in token distribution logic should use checked arithmetic operations (checked_sub, checked_add) to prevent potential overflows, especially when dealing with block heights and intervals.
packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/v0/methods.rs (1)
Learnt from: QuantumExplorer
PR: dashpay/platform#2450
File: packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/v0/methods.rs:10-12
Timestamp: 2025-03-13T13:59:31.360Z
Learning: Block interval calculations in token distribution logic should use checked arithmetic operations (checked_sub, checked_add) to prevent potential overflows, especially when dealing with block heights and intervals.
packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/reward_distribution_type/mod.rs (1)
Learnt from: QuantumExplorer
PR: dashpay/platform#2450
File: packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/v0/methods.rs:10-12
Timestamp: 2025-03-13T13:59:31.360Z
Learning: Block interval calculations in token distribution logic should use checked arithmetic operations (checked_sub, checked_add) to prevent potential overflows, especially when dealing with block heights and intervals.
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive-abci) / Linting
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Build JS packages / Build JS
🔇 Additional comments (245)
packages/rs-drive/src/drive/initialization/v1/mod.rs (2)
7-12: Removal of unused imports improves code cleanlinessThe consolidation of imports and removal of unused ones (token_perpetual_distributions_path_vec, token_root_perpetual_distributions_path_vec, BatchInsertTreeApplyType, TreeType) makes the code cleaner and easier to maintain. This change aligns with the PR's focus on token distribution fixes.
18-18: Simplified import statementThe removal of TreeType from this import statement is consistent with the overall cleanup of unused imports.
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/distribution/mod.rs (1)
2-2:✅ Verification successful
Test coverage expanded with pre-programmed distribution tests
Adding a new test module for pre-programmed distributions aligns with the PR objective of improving token distribution functionality and increasing test coverage.
🏁 Script executed:
#!/bin/bash # Check that the pre_programmed.rs test file exists and contains tests fd -t f "pre_programmed.rs" packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/distribution/ | xargs catLength of output: 29971
Pre-Programmed Distribution Tests Verified
The new test module has been correctly added. The file
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/distribution/pre_programmed.rscontains comprehensive tests covering various scenarios for pre-programmed token distributions, including successful claims and handling of cases where no rewards are available. This aligns well with the PR objective of expanding test coverage for token distribution functionality.packages/rs-drive/src/drive/tokens/distribution/fetch/mod.rs (1)
2-2:❓ Verification inconclusive
New functionality for tracking pre-programmed distribution payments
This module addition introduces capabilities to track the last payment timestamp for pre-programmed token distributions, which is essential for proper token distribution management.
🏁 Script executed:
#!/bin/bash # Check the implementation of the new module fd -t f "pre_programmed_distribution_last_paid_time_ms.rs" packages/rs-drive/src/drive/tokens/distribution/fetch/ | xargs catLength of output: 125
Attention: Verify Existence of Implementation for Tracking Module
The new module declaration in
packages/rs-drive/src/drive/tokens/distribution/fetch/mod.rs:mod pre_programmed_distribution_last_paid_time_ms;appears to be missing its corresponding implementation file (
pre_programmed_distribution_last_paid_time_ms.rs). The initial verification command returned no output, which may indicate one of the following:
- The file is missing from the repository.
- The file is misnamed or located elsewhere.
- The implementation is pending, and the module declaration was added prematurely.
Action Required:
- Manual Verification: Please verify that the file exists, is correctly named, and contains the intended logic to track the last payment timestamp for pre-programmed token distributions.
- Update if Needed: If the file is missing or mislocated, update the repository accordingly to ensure the new functionality works as expected.
packages/rs-platform-version/src/version/drive_versions/drive_token_method_versions/v1.rs (1)
20-20: Integration of pre-programmed distribution tracking.A new field
pre_programmed_distribution_last_paid_timehas been added with an initial version of 0, which aligns with the token distribution fixes mentioned in the PR title. This field will enable tracking the last payment time for pre-programmed distributions, complementing the existingperpetual_distribution_last_paid_timefield.packages/rs-platform-version/src/version/drive_versions/drive_token_method_versions/mod.rs (1)
36-36: New feature field for tracking pre-programmed distribution payment times.The addition of this field to the
DriveTokenFetchMethodVersionsstruct complements the existing perpetual distribution tracking, completing the API for both distribution types. This change matches the initialization in the v1.rs file.packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/distribution_function/reward_ratio.rs (1)
1-5: New ratio representation for reward calculations.The
RewardRatiostruct provides a clean way to represent reward ratios with numerator and denominator values. This structure is appropriately derived withDebug,Copy, andClonetraits for convenient use in token distribution logic.packages/rs-dpp/src/data_contract/document_type/property/mod.rs (1)
19-19: Removed unused import for BTreeValueMapPathHelper.The removal of this import suggests a cleanup or refactoring of the BTree operations. Since the import is no longer needed, this change reduces unnecessary dependencies, which is a good practice.
packages/rs-platform-version/src/version/mocks/v2_test.rs (1)
394-395: Consistent implementation of max token redemption cycles limit.The added
max_token_redemption_cyclesfield with value 128 aligns with the new system limit definition. This ensures the test platform version correctly represents the new constraint for token redemption cycles.packages/rs-platform-version/src/version/system_limits/mod.rs (1)
13-17: Well-documented field for redemption cycle limitation.The addition of
max_token_redemption_cycleswith clear documentation properly explains the performance considerations behind this limit. The comments effectively communicate why constant distributions are more efficient and why other distribution methods need this limitation.packages/rs-dpp/src/errors/protocol_error.rs (1)
284-286: Added new error variant for missing epoch information.The new
MissingEpochInfoerror variant enhances error reporting capabilities related to epoch data, which aligns with the token distribution fixes in this PR.packages/rs-drive/src/drive/tokens/mint_many/v0/mod.rs (1)
132-143:✅ Verification successful
Updated API call to handle saturation control and tuple return.
The modification adds a
falseparameter forallow_saturationand extracts only the first element of the returned tuple with.0. This change:
- Provides explicit control over token supply saturation during minting
- Adapts to the updated API that now returns multiple values
Ensure all other callers of this method have been similarly updated to handle the new return type and parameter.
🏁 Script executed:
#!/bin/bash # Check if other callers of add_to_token_total_supply_operations have been updated # Search for all calls to this method echo "Searching for all calls to add_to_token_total_supply_operations:" rg "add_to_token_total_supply_operations\s*\(" --type rustLength of output: 534
Verified: All Callers Updated for New API Signature
The changes in the mint-many file correctly add the
falseparameter and extract the first element from the returned tuple. Our search confirmed that the other call sites inpackages/rs-drive/src/drive/tokens/mint/v0/mod.rsandpackages/rs-drive/src/drive/tokens/system/add_to_token_total_supply/mod.rsare also updated accordingly. No further modifications are needed.packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_claim_transition_action/v0/mod.rs (1)
106-106:✅ Verification successful
Simplified match pattern for TokenDistributionInfo::Perpetual variant.
The match pattern for the
TokenDistributionInfo::Perpetualvariant has been updated to expect only two parameters rather than three, which aligns with structural changes to theTokenDistributionInfoenum elsewhere in the codebase.
🏁 Script executed:
#!/bin/bash # Verify that the TokenDistributionInfo::Perpetual variant definition is consistent with this usage # Look for the TokenDistributionInfo enum definition rg -A 20 "enum TokenDistributionInfo" --type rustLength of output: 2685
Approved: Match pattern update for TokenDistributionInfo::Perpetual is correct
- Verified that the
TokenDistributionInfo::Perpetualvariant is defined as accepting exactly two parameters (aRewardDistributionMomentand aTokenDistributionResolvedRecipient) inpackages/rs-dpp/src/data_contract/associated_token/token_distribution_key.rs.- The updated match arm in
packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_claim_transition_action/v0/mod.rscorrectly deconstructs the variant into(_, resolved_recipient).The change aligns with the current enum definition, so no further modifications are needed.
packages/wasm-dpp/src/errors/consensus/consensus_error.rs (2)
87-87: Added new token claim error types to imports.The import statement for the
tokenmodule has been updated to include the new error typesInvalidTokenClaimNoCurrentRewardsandInvalidTokenClaimWrongClaimant.These new error types will improve error handling for token claim operations.
381-386: Added handler cases for new token claim error types.Two new cases have been added to the
from_state_errorfunction to handle theInvalidTokenClaimNoCurrentRewardsandInvalidTokenClaimWrongClaimantstate errors.These additions properly integrate the new error types into the existing error handling system.
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/distribution/perpetual/mod.rs (1)
1-3: New test modules for perpetual token distribution.Added new test modules for block-based and time-based perpetual token distribution.
This is a good addition that will improve test coverage for different types of perpetual token distribution mechanisms. The structure with specialized modules for each distribution type (block_based and time_based) follows good organization practices.
packages/rs-dpp/src/block/finalized_epoch_info/v0/mod.rs (1)
8-8: Good change: Vec<(Identifier, u64)> → BTreeMap<Identifier, u64>Switching from a vector of tuples to a BTreeMap improves both semantics and performance:
- Provides O(log n) lookups by Identifier (vs O(n) in vector)
- Maintains ordering based on keys
- Ensures uniqueness of Identifiers
- Makes the code more self-documenting
Also applies to: 32-32
packages/rs-dpp/src/block/finalized_epoch_info/getters.rs (1)
6-6: Good change: Updated return type to match FinalizedEpochInfoV0 changesCorrectly updated the return type of
block_proposers()from&Vec<(Identifier, u64)>to&BTreeMap<Identifier, u64>to maintain consistency with the struct field type change. This ensures type safety and proper API expectations.Also applies to: 63-63
packages/rs-dpp/src/block/finalized_epoch_info/v0/getters.rs (1)
5-5: Good change: Updated both trait definition and implementation return typesProperly updated both the trait definition and implementation to return
&BTreeMap<Identifier, u64>instead of&Vec<(Identifier, u64)>, maintaining consistency with the struct field type change.Also applies to: 37-37, 83-83
packages/rs-dpp/src/errors/consensus/state/state_error.rs (1)
245-249: Good error handling expansionAdding these two specific error variants improves the system's ability to provide clear and precise feedback for token claim operations. The
InvalidTokenClaimNoCurrentRewardsandInvalidTokenClaimWrongClaimanterrors will help developers diagnose claim-related issues more effectively.Using
#[error(transparent)]ensures the underlying error details are properly surfaced, maintaining the error context.packages/rs-drive/src/drive/tokens/estimated_costs/for_token_pre_programmed_distribution/mod.rs (1)
28-28: Good API improvement making thetimesparameter optionalMaking the
timesparameter optional is a good improvement to the API. This change allows for more flexibility, as callers can now omit the timestamps when they're not needed, while maintaining backward compatibility with existing code that provides timestamps.The documentation has been properly updated to reflect this change.
Also applies to: 44-44
packages/rs-dpp/src/data_contract/associated_token/token_distribution_key.rs (2)
51-54: Simplified data model for Perpetual distributionsThe
Perpetualvariant ofTokenDistributionInfohas been simplified to contain only oneRewardDistributionMomentinstead of two. This streamlines the data structure, making it more maintainable and easier to understand.The documentation comments have been updated appropriately to reflect the change in structure.
63-64: Updated implementations to match the simplified data modelThe
Fromtrait implementations have been correctly updated to match the simplifiedPerpetualvariant structure, ensuring that the conversion logic remains consistent with the data model changes.Also applies to: 76-77
packages/rs-dpp/src/errors/consensus/state/token/mod.rs (1)
6-8: Good addition of token claim-specific error modulesThe addition of the two new error modules for token claims (
invalid_token_claim_no_current_rewardsandinvalid_token_claim_wrong_claimant) with their correspondingpub usestatements enhances the error handling system.These modules correspond to the new error variants added to the
StateErrorenum, providing a more granular and accurate approach to error handling for token claim operations.Also applies to: 25-27
packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/methods/mod.rs (3)
5-5: New import for RewardDistributionMoment addedThis import is required for the updated return types of the methods below. Good addition.
40-44: Method return type updated to use more descriptive typeThe method now returns a
RewardDistributionMomentinstead of au64. This enhances type safety and provides better semantic meaning to what was previously a primitive numeric value.
46-50: New method for getting current intervalThe addition of the
current_intervalmethod provides a complementary function tonext_interval, improving the API by allowing clients to determine the current distribution moment.packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/methods/v0/mod.rs (1)
3-3: New import for RewardDistributionMomentThis import is required for the updated trait method return types.
packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_claim_transition.rs (3)
72-73: Field name standardization in TokenMarkPreProgrammedReleaseAsDistributedThe
recipientfield has been renamed torecipient_idfor more consistent naming across the codebase.
77-85: Updated TokenMarkPerpetualReleaseAsDistributed fieldsThe fields for the perpetual release distribution have been updated:
- Pattern matching now only captures
claim_moment- Changed field from
owner_idtorecipient_id- Added new field
cycle_start_momentThese changes improve the tracking of distribution cycles and standardize recipient identification.
62-62:❓ Verification inconclusive
New parameter for token minting allows saturation
The
allow_saturationparameter has been added and set totrue. This allows token minting to potentially reach the maximum supply limit. Make sure this is the intended behavior as it could affect token economics.Run the following script to check if there are tests validating the behavior with
allow_saturationset totrue:
🏁 Script executed:
#!/bin/bash # Search for test cases that verify token saturation behavior rg -A 5 -B 5 "allow_saturation.*true" --type rust | grep -E "test|assert"Length of output: 75
Action Required: Verify Token Saturation Behavior in Tests
It appears that automated searching for tests explicitly validating the behavior of
allow_saturation: truedid not yield any results. This raises a concern regarding whether the token minting behavior—allowing saturation to possibly reach the maximum supply limit—is properly covered by the test suite. Please manually verify that:
- Appropriate test cases exist which validate the behavior when
allow_saturationis set totrue.- The current tests (if any, or tests that cover related functionality) robustly ensure that the token economics are maintained as intended.
- If such tests are missing, consider adding tests to explicitly cover this scenario.
File:
packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_claim_transition.rs(line 62)
Snippet:allow_saturation: true,packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/distribution_function/encode.rs (4)
44-46: Improved variable naming in Linear distribution functionRenaming
stostart_stepandbtostarting_amountimproves code readability by using descriptive names that clearly indicate the purpose of these variables.
63-64: More descriptive variable naming in mathematical distribution functionsRenaming
stostart_momentin the Polynomial, Exponential, Logarithmic, and InvertedLogarithmic variants improves code clarity by using a descriptive name that clearly indicates the purpose of this variable.Also applies to: 85-86, 107-108, 129-130
195-197: Consistent naming in Decode implementationThe variable naming updates have been consistently applied in the
Decodeimplementation, ensuring that the code remains coherent throughout the encoding and decoding processes.Also applies to: 217-218, 239-240, 261-262, 283-284
341-343: Consistent naming in BorrowDecode implementationThe variable naming updates have been consistently applied in the
BorrowDecodeimplementation, maintaining coherence throughout all serialization and deserialization code paths.Also applies to: 363-364, 385-386, 407-408, 429-430
packages/rs-drive/src/drive/tokens/distribution/fetch/pre_programmed_distribution_last_paid_time_ms/v0/mod.rs (2)
53-62: Decode logic is correct and carefully validates the 8-byte length.This section properly checks that the stored value has a length of 8 bytes and then converts it to a
u64viafrom_be_bytes. This ensures well-defined behavior and avoids accidentally decoding corrupted or malformed data.
64-71: Thoughtful handling of missing or invalid items.Returning
Ok(None)when the key is not found simplifies usage for callers by uniformly signaling “no record found.” Handling unexpected item types and other GroveDB errors by returning appropriate Drive errors covers all edge cases and reduces the chance of silent failures.packages/rs-drive/src/drive/tokens/distribution/mark_pre_programmed_release_as_distributed/mod.rs (1)
57-65: LGTM - Function call updated consistentlyThe function call to the versioned implementation has been correctly updated to pass
recipient_idas the parameter.packages/rs-drive/src/drive/tokens/estimated_costs/for_token_pre_programmed_distribution/v0/mod.rs (2)
62-64: Good defensive coding with fallback valueThe code now gracefully handles the case when
timesisNoneby providing a default value of 128 for the estimated layer count.
69-83: LGTM - Conditional wrapping prevents potential runtime errorsThe loop for processing each timestamp is now properly wrapped in a conditional to execute only when
timesisSome. This prevents potential runtime errors whentimesisNone.packages/rs-drive/src/util/batch/drive_op_batch/token.rs (6)
41-43: Well-documented saturation parameterGood job adding explanatory comments for the new
allow_saturationparameter that clarifies its purpose and provides an example.
58-65: Improved token release distribution with better documentationThe
TokenMarkPerpetualReleaseAsDistributedvariant has been improved by:
- Replacing multiple fields with the clearer
recipient_id- Adding the
cycle_start_momentfield- Including detailed documentation for the new fields
This change makes the API more intuitive and better documented.
69-73: Consistent parameter naming with TokenMarkPerpetualReleaseAsDistributedThe
TokenMarkPreProgrammedReleaseAsDistributedvariant now uses consistent parameter naming with the perpetual release variant, usingrecipient_idinstead of separate owner and identity identifiers.
155-156: Saturation parameter correctly propagatedThe new
allow_saturationparameter is correctly propagated from the enum variant to the drive function call.Also applies to: 164-165
262-275: Implementation correctly updated for TokenMarkPerpetualReleaseAsDistributedThe implementation for
TokenMarkPerpetualReleaseAsDistributedhas been correctly updated to use the new parameters.
276-292: Implementation correctly updated for TokenMarkPreProgrammedReleaseAsDistributedThe implementation for
TokenMarkPreProgrammedReleaseAsDistributedhas been correctly updated to use the new parameters.packages/rs-drive/src/drive/tokens/distribution/add_perpetual_distribution/v0/mod.rs (2)
2-5: New token distribution constants importedThe code now imports important distribution-related constants that will be used later in the function.
80-88:✅ Verification successful
Storage structure updated for identity tracking
The function now creates an empty tree with
TOKEN_PERPETUAL_DISTRIBUTIONS_FOR_IDENTITIES_LAST_CLAIM_KEYinstead of the previous handling based on block info and intervals. This change appears to be part of a simplified approach to tracking distribution claims per identity.Let's check if this constant is properly defined:
🏁 Script executed:
#!/bin/bash # Look for the definition of TOKEN_PERPETUAL_DISTRIBUTIONS_FOR_IDENTITIES_LAST_CLAIM_KEY rg -A 1 "TOKEN_PERPETUAL_DISTRIBUTIONS_FOR_IDENTITIES_LAST_CLAIM_KEY" --type rustLength of output: 2774
Token Constant Verified: Storage Structure Consistency Confirmed
The constant
TOKEN_PERPETUAL_DISTRIBUTIONS_FOR_IDENTITIES_LAST_CLAIM_KEYis defined correctly inpackages/rs-drive/src/drive/tokens/paths.rs(set to192) and is consistently referenced across the related modules. The changes reflecting a simplified approach to tracking distribution claims per identity using an empty tree are verified to rely on the proper constant definition.
- Verified definition in
packages/rs-drive/src/drive/tokens/paths.rs- Consistent usage in modules such as
add_perpetual_distributionandmark_perpetual_release_as_distributedNo issues remain regarding this change.
packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/v0/methods.rs (2)
3-4: Good modularization of the reward distribution frameworkThese imports show a clear separation of reward distribution moment types, which makes the code more maintainable and extensible.
29-47: The current_interval method includes an important edge case for epoch intervalThe method now correctly handles the special case when interval equals 1, directly returning the current epoch index. This is an important edge case that prevents unnecessary calculations.
The return types have been properly changed to the appropriate variant of
RewardDistributionMoment, aligning with the refactoring of the next_interval method and providing consistency across the API.packages/rs-dpp/src/errors/consensus/state/token/invalid_token_claim_wrong_claimant.rs (4)
1-8: Appropriate imports for the new error typeAll necessary imports for error handling, serialization/deserialization, and protocol structures are included.
9-23: Well-structured error type with clear error messageThe error type is well-designed with:
- Appropriate trait derivations for error handling, debugging, and serialization
- A descriptive error message that clearly indicates what went wrong
- Proper fields to hold all relevant identifiers
The unversioned annotation for platform serialization indicates this error structure won't need versioning.
25-53: Complete implementation with constructor and accessorsThe implementation provides a constructor and accessors for all fields, following Rust best practices. Each method is properly documented with clear comments explaining its purpose.
55-60: Proper integration with the error hierarchyThe implementation of
From<InvalidTokenClaimWrongClaimant> for ConsensusErrorensures this error type is properly integrated into the existing error handling infrastructure, allowing it to propagate through the system.packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/reward_distribution_type/evaluate_interval.rs (3)
3-4: Added imports for the enhanced reward calculation logicThe addition of imports for
EpochIndexandRewardRatioreflects the expanded functionality to handle epoch-based rewards.
13-16: Updated documentation with improved clarityThe documentation has been enhanced to more accurately describe the function's behavior, particularly regarding distribution start and end moments. The parameter documentation now correctly reflects the renamed parameters and new functionality.
Also applies to: 20-25
31-47: Significantly improved function signature and implementationThe method has been enhanced in several ways:
- Added generics to support a flexible callback for epoch reward ratios
- Renamed parameters for better clarity
- Added a separate parameter for the distribution start moment
- Switched to calling
evaluate_intervalinstead ofevaluate_interval_in_boundsThese changes make the function more flexible and better structured for different distribution scenarios.
packages/rs-drive/src/drive/tokens/distribution/fetch/pre_programmed_distribution_last_paid_time_ms/mod.rs (3)
1-11: Well-organized imports and module structureThe module structure and imports are clean and well-organized, including only necessary dependencies for the functionality.
12-44: Clear public API method with comprehensive documentationThe
fetch_pre_programmed_distribution_last_paid_time_msmethod provides a clean public API with:
- Detailed documentation explaining its purpose and behavior
- Clear parameter descriptions
- Proper return type handling with
Option<TimestampMillis>- Delegation to the operations variant for actual implementation
This follows the pattern of separating the public API from the operations tracking logic.
45-91: Robust versioned implementation with proper error handlingThe operations method:
- Provides thorough documentation
- Handles versioning through the platform version parameter
- Implements proper error handling for unknown versions
- Follows the repository's pattern for versioned method dispatch
The error message for unknown versions is specific and includes the method name and known versions, which will be helpful for debugging.
packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/distribution_function/evaluate.rs (57)
15-19: Addition ofcontract_registration_steplooks good.
This parameter enables a more precise reference point for distribution. Ensure that any calling code is updated accordingly.
64-64: Good use ofunwrap_or(contract_registration_step).
This fallback ensures logic remains consistent ifsis absent.
178-178: Renaming the start parameter tostart_momentimproves clarity.
This name is more expressive and helps prevent confusion about whether it refers to time or an index.
193-193: Consistent fallback tocontract_registration_step.
Matches the approach used in other distribution variants.
246-246: Use ofstart_momentin the exponential distribution is consistent.
Maintains parallel naming across all distribution types.
261-261: Extracteds_valfromstart_momentis consistent with other code paths.
This keeps logic uniform across multiple distribution branches.
314-314: Implementation mirrors other distribution functions.
Keeping identical naming avoids confusion between distribution variants.
327-327:s_valfallback is coherent.
No issues found; ensures the log-based function has a sane initial offset.
376-376: Newstart_momentfield for inverted logarithmic distribution.
Maintains consistency with the other variants.
399-399: Fallback tocontract_registration_stepfor inverted logarithmic.
Appropriate for ensuring a uniform baseline across distributions.
480-482: Tests fortest_fixed_amountare comprehensive.
They validate multiple periods to confirm the function is constant.
493-499: Stepwise emission tests offer good coverage.
Verifies boundary periods (both ends of step ranges).
512-518: Checks on step decreasing function are thorough.
The test ensures the function doesn’t go below themin_value.
532-534: Proper handling of a divide-by-zero scenario.
The test ensures the function gracefully fails when the denominator is 0.
544-545: Random distribution test with range check.
Confirms that values stay within the specified minimum and maximum.
559-560: Single-value random distribution test is valid.
Ensures the function behaves deterministically if min == max.
572-577: Error path for invalid range is verified.
Correctly expects an Overflow error whenmin > max.
584-586: Ensures deterministic output for same seedx.
Important property for verifying predictable random generation.
597-599: Test confirms variation across differentx.
Verifies that changing seed yields different outputs.
613-623: Linear function increasing test is well-scoped.
Covers both the base offset and additive slope.
636-639: Linear function decreasing test.
Ensures correct floor clamp tomin_value.
653-655: Divide-by-zero for linear function is properly tested.
Verifies the function rejectsd=0.
674-678: Polynomial function test checks multiple x values.
Good coverage for verifying exponent and offset logic.
694-700: Overflow test for large coefficient.
Ensures the function fails gracefully if results exceed numeric limits.
717-718: Fractional exponent in polynomial function is validated.
Confirms(3/2)exponent yields correct results.
735-736: Test for negativeaverifies non-negative result clamp.
Guarantees negative outcomes become zero.
754-757: Shift parameterstart_momentis tested.
Ensures polynomial function uses shifted domain.
775-776: Offset parameterotested.
Checks whether the function applies offset before exponentiation.
794-795: Constant polynomial function scenario is confirmed.
(x - s + o)^0= 1 for valid domain, ensuring we get a flat outcome.
813-814: Linear-like polynomial test (m=1, n=1).
Proper coverage ensuresf(x)=ax+bworks within polynomial logic.
831-832: Cubic function test.
Verifiesx^3usage and correct final values.
850-851: Combined offset and shift for polynomial.
Ensures the function handles multiple transformations.
869-871: Basic exponential function.
Checks both base offset (c) and exponent correctness.
887-889: Divide-by-zero for exponential function.
Test ensures an error is thrown whend = 0.
907-909: Exponential function with smallxcoverage.
Verifies results for incrementalxchanges and bases.
926-929: Slow-growth exponential test.
Confirms that smaller exponents produce smaller increments over largex.
945-950: Rapid-growth exponential is clamped.
Tests upper bound to verify no overflow beyondmax_value.
966-969: Exponential with negative exponent saturating to a minimum.
Verifies results never drop below 10 ifmin_valueis set.
985-988: Exponential function with custommin_value.
Ensures the result never falls below 11.
1005-1014: Start at max scenario test.
Guaranteed clamping for initial call ensures no overshoot of the max bound.
1030-1036: Overflow test for hugex.
Properly rejects extremely large arguments that exceed numeric bounds.
1054-1056: Logarithmic function basic test.
Checks initial offset to ensure valid log domain.
1072-1074: Enforcesmin_valueandmax_valuefor logarithmic.
Ensures result is correctly clamped.
1091-1093: Handles undefined log domain.
Tests a scenario withlog(0)to ensure overflow is reported.
1110-1113: Largexscenario for log function.
Checks no overflow occurs and that results remain finite.
1130-1132: Divide-by-zero indfor log function.
Asserts the function halts gracefully whend=0.
1150-1152: Divide-by-zero innfor log function.
Prevents invalid exponent denominator usage.
1171-1173: Inverted log decreasing test.
Checks that consecutive blocks yield decreasing amounts.
1190-1193: Inverted log increasing test.
Confirms correct growth whenais negative andnis large, reversing the slope.
1212-1213: Clamp to 0 if final amount is negative.
The test ensures negative outcomes become zero.
1229-1230: Clamps result tomin_valuefor negative outcomes.
Verifies distribution won’t drop below set minimum.
1247-1248: Test ensuresmax_valueis enforced.
Keeps the result within an upper bound for large negativea.
1265-1267: Ensures log argument isn’t zero or negative.
Catches an overflow error ifocauses(x - s + o)to be 0.
1285-1287: Zeronin inverted log triggers divide-by-zero error.
Protects from invalid domain.
1305-1307: Zerodin inverted log triggers divide-by-zero error.
Ensures no attempt to divide by zero occurs.
1325-1328:min_valueclamp for increasing inverted log distribution.
The function’s initial result starts atmin_valueand ends atmax_value.
1351-1354: Final clamp tomin_valuein a decreasing distribution.
Prevents negative or smaller-than-min outcomes.packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_claim_transition_action/v0/transformer.rs (16)
5-5: Explicit import ofTokenAmountis clear.
Ensures the balance type is well-defined.
9-9: Use ofFinalizedEpochInfoGettersV0is consistent.
Allows retrieving epoch details more easily.
12-12: Introduction of new errors:InvalidTokenClaimNoCurrentRewards,InvalidTokenClaimWrongClaimant.
These specific errors improve clarity and user feedback.
17-17:RewardRatiousage for distributing partial tokens is good.
Confirms more flexible calculations for distribution events.
19-19: Leveraging method trait fromTokenPerpetualDistributionV0Methods.
This helps unify distribution logic.
21-21:TokenPreProgrammedDistributionV0Methodsimport
Allows direct usage of pre-programmed distribution logic.
29-29: Combined usage ofConsensusValidationResultandUserFeeIncrease.
Streamlines how we pass fee adjustments and consensus checks.
225-307: Retrieval of the oldest unclaimed distribution is correct.
- Filters future timestamps and uses the earliest unclaimed for the identity.
- Properly raises
InvalidTokenClaimNoCurrentRewardswhen none are available.
309-312: Well-defined final output for pre-programmed scenario.
TokenDistributionInfo::PreProgrammedcaptures the distribution type and moment.
344-383: Claimant verification logic in perpetual distribution.
- Ensures only the designated identity can claim.
- Raises
InvalidTokenClaimWrongClaimantwith explicit info.
410-411: Obtaining contract creation moment fromdistribution_type.
Helps compare it against the last-paid moment for correct intervals.
423-424:current_intervalusage for block-based or time-based distribution.
Matches the updated logic in distribution crates.
426-433: Compute the maximum cycle to avoid indefinite claims.
Leverages a platform limit (max_token_redemption_cycles).
435-447: Accumulative reward calculation viarewards_in_interval.
Correctly sums up partial or total rewards across epochs.
504-530: Handling zero-amount claims withInvalidTokenClaimNoCurrentRewards.
Prevents repeated claims of zero tokens.
534-535: Return of(amount, TokenDistributionInfo::Perpetual(...)).
Captures the needed distribution variant and moment.packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/distribution/perpetual/block_based.rs (32)
1-4: Import of test helpers and standard crates.
Provides a well-structured base for setting up platform states and identities.
5-9: Successfully referencesTokenConfigurationandBatchTransition.
Makes state transitions straightforward to construct and serialize.
10-11: Usage ofrand::prelude::StdRng
Ensures reproducible random data for test consistency.
12-19: Definition ofmod perpetual_distribution_block.
Logical grouping for block-based distribution tests.
22-28: Sets up platform with the latest protocol version.
Prepares a stable environment for block-based distribution tests.
29-30: Random seed usage enhances reproducibility.
Prevents flakiness in tests that rely on random identity creation.
33-35: Generates first identity for test.
Allocates 0.5 Dash in credits for immediate contract usage.
36-55: Creates and configures a token contract with a fixed 50-credit emission.
Implementing block-based intervals ensures repeated reward cycles.
57-57:fast_forward_to_block(..., 40, ...)simulates block progression.
Effectively checks reward accrual over a large skip.
59-75: Creates a token claim transition
Ensures usage of the new perpetual distribution claim logic with block-based triggers.
77-79: Serializes the batch transition for processing.
Confirms that raw transitions can be submitted to the platform.
81-82: Transaction context usage.
Starts a fresh transaction in Grove for each test scenario.
83-99: Processes the raw transition
Ensures the platform can parse and validate the claim.
101-104: Asserts successful state transition execution.
Verifies no errors arise when the claim is valid and rewards exist.
106-111: Commits transaction
Saves the state to finalize the distribution claim.
113-124: Fetches and confirms token balance
Validates that the user’s account is credited with the correct reward.
125-125: Fast-forwards to a new block
Sets up a second claim under different height/time conditions.
127-143: Second claim transition
Tests the scenario where no new rewards were generated, expecting an error.
145-147: Re-serialization for the second claim
Proves transitions remain consistent across repeated requests.
149-166: Platform processes second claim
Verifies that the engine detects “no new rewards” and throws an error.
169-175: ExpectsInvalidTokenClaimNoCurrentRewardsconsensus error.
Accurately reflects the scenario of zero additional distribution.
177-183: Commits state for the second transition
Even though it failed, the transaction finalizes updated state or logs.
184-194: Balance check
Confirms that unsuccessful claims do not alter existing token holdings.
195-195: Fast-forward again
Simulates additional blocks for another claim attempt.
197-213: Third claim attempt
Ensures that newly created intervals produce a successful claim.
215-218: Serialization check
Same process repeated to confirm stable handling of transitions.
219-236: Process the third claim
Expects success due to newly matured rewards.
239-242: Asserts a successful result
Indicates that the periodic logic properly recognized new rewards.
244-250: Final commit
Writes the newly updated token balance after the successful claim.
251-262: Confirms final token balance increment
Demonstrates that an extra 50 tokens have been added.
265-386:test_token_perpetual_distribution_not_claimant
- Validates that only the specified recipient can claim token rewards.
- Ensures the error matches
InvalidTokenClaimWrongClaimant.
388-495:test_token_perpetual_distribution_block_claim_linear_given_to_specific_identity
- Ensures that distribution is properly assigned to a single non-owner identity.
- The final balance check confirms correct accumulation.
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/mod.rs (3)
59-59: Added TimestampMillis import for improved contract time handling.This addition is part of enhancing the contract creation functionality to support customizable timestamps.
2295-2295: Added contract_start_time parameter for flexible contract creation.This parameter allows for specifying when a contract starts, improving the flexibility of contract creation by allowing timestamps other than the default value.
2308-2308: Improved contract creation time handling.The code now uses the provided start time or defaults to 0 instead of hardcoding the value, which aligns with the function signature change.
packages/rs-dpp/src/errors/consensus/codes.rs (2)
236-237: Expanded document errors range for better organization.The document errors range has been expanded from 40100-40149 to 40100-40199, providing more space for document-related error codes.
288-307: Added new token error codes for improved error handling.A new range for token-specific errors has been introduced (40700-40799), including specialized errors for token claims. This improves error handling granularity for token operations.
Key additions include:
- 40716: InvalidTokenClaimNoCurrentRewards
- 40717: InvalidTokenClaimWrongClaimant
These new error types support the token distribution fixes mentioned in the PR title.
packages/rs-drive-abci/src/execution/platform_events/fee_pool_outwards_distribution/add_epoch_pool_to_proposers_payout_operations/v0/mod.rs (3)
3-3: Added BTreeMap import for improved proposer data structure.This import supports the change from vector to BTreeMap for proposers' data.
36-36: Changed return type to use BTreeMap instead of vector for proposers.Using BTreeMap instead of a vector of tuples provides more efficient lookup by Identifier and maintains a consistent order of proposers.
169-169: Updated return statement to convert proposers to BTreeMap.The implementation now correctly converts the vector of tuples into a BTreeMap to align with the return type change.
packages/rs-drive/src/drive/tokens/distribution/mark_perpetual_release_as_distributed/mod.rs (6)
14-15: Improved function description for better clarity.The documentation now correctly describes the function as marking a token distribution as distributed rather than a token release.
16-22: Enhanced documentation with clearer explanation of function behavior.The updated documentation provides more details about what actions the function performs, making it easier to understand its purpose and effects.
24-29: Updated parameter descriptions for better API documentation.The parameter descriptions are now more precise, particularly clarifying the purpose of
recipient_idandcurrent_moment.
31-33: Updated return type documentation.The return type has been changed to more accurately reflect that the function returns batch operations for storage updates.
35-35: Simplified versioning documentation.The versioning section has been streamlined to be more concise while still providing the necessary information.
39-40: Simplified parameter list for token distribution.Parameters have been refactored from
owner_id,previous_moment, andnext_momentto the more appropriaterecipient_idandcurrent_moment. This change streamlines the function and makes its purpose clearer.packages/rs-drive/src/drive/tokens/paths.rs (3)
62-64: Watch out for potential key collisions.Both
TOKEN_PERPETUAL_DISTRIBUTIONS_FOR_IDENTITIES_LAST_CLAIM_KEYandTOKEN_PRE_PROGRAMMED_DISTRIBUTIONS_FOR_IDENTITIES_LAST_CLAIM_KEYshare the same value (192). This may lead to collisions in certain contexts if they are not distinctly separated in their respective path structures. Consider assigning a unique value if these constants are meant to be logically separate.
230-241: Maintain naming consistency.The
_vecversion precisely mirrors the_pathversion, which is good. Ensure both functions remain in sync if future parameters or logic are introduced or changed.
243-271: Validate usage of extended identity path.The new functions
token_perpetual_distributions_identity_last_claimed_by_identity_path/_vecexpand the path to six segments. Confirm that all relevant code properly handles this deeper path to avoid indexing mistakes or mismatches.packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/distribution/pre_programmed.rs (3)
188-359: Validate progression of block time for subsequent claims.The second test (
test_token_pre_programmed_distribution_claim_again_when_none_left) successfully checks the scenario of claiming after rewards are exhausted. It might be beneficial to verify edge cases such as extremely close subsequent blocks or missing time increments.
361-642: Solid coverage for delayed distributions.
test_token_pre_programmed_distribution_claim_again_when_none_readycomprehensively tests deferred rewards. The approach looks correct and ensures error handling is robust for scenarios where the claim is too early.
644-748: Good negative test coverage for impossible claims.
test_token_pre_programmed_distribution_claim_no_rewards_for_recipientaccurately demonstrates the scenario where rewards were never allocated to the claimant. This helps ensure that the system properly rejects such claims.packages/rs-drive/src/drive/tokens/distribution/mark_pre_programmed_release_as_distributed/v0/mod.rs (4)
2-5: No apparent issues with new imports.The newly added imports appear relevant to the updated logic. There are no obvious duplications or unused references, so this looks fine.
Also applies to: 13-13, 15-15, 26-29
58-58: Parameter rename torecipient_idis clear.Replacing
owner_id/identity_idwithrecipient_idstreamlines the API and better describes its use. This improves clarity for the caller.
68-69: Good addition of estimation logic.The inserted estimation logic for
pre_programmed_distributions_pathhelps ensure that storage costs are accurately calculated. The approach of updatingestimated_costs_only_with_layer_infowith relevant layer sizes appears consistent with existing patterns.Also applies to: 72-73, 81-89, 96-103
162-170: Verify that new insert key won't overwrite multiple scheduled distributions.Storing
release_timeas the value underrecipient_idmight cause overwrites if there are multiple releases for the same recipient. Confirm whether only one claim is intended per identity at a time or if additional key segmentation is necessary.packages/rs-drive/src/drive/tokens/distribution/mark_perpetual_release_as_distributed/v0/mod.rs (7)
1-1: Good usage of tokens::paths.
No concerns, as it ensures correct referencing of distribution paths.
8-11: Imports for Grovedb cost estimation are properly introduced.
These newly added imports perfectly align with the batch operation usage and cost annotation needs.
19-20: Function signature update torecipient_idandcurrent_moment.
Switching from the old parameters (owner_id, previous/next moment) to these streamlined arguments looks consistent with the updated distribution approach.
28-30: Revised path usage for perpetual distributions.
Usingtoken_perpetual_distributions_identity_last_claimed_time_path_vecto define the storage path neatly aligns with the focus on the current moment.
38-42: Unclear rationale for epoch-based estimated layer depth.
Currently set toEstimatedLevel(10, false)for epoch-based distributions butEstimatedLevel(0, false)for block/time-based. Ensure this difference is intentional and accurately reflects domain requirements.
44-52: Insertion of estimated layer info for perpetual distributions.
The usage ofAllItems(1, 8, None)inEstimatedLayerInformationfollows the new approach for storing moment data. No issues identified here.
58-58: Storing the distribution moment as a raw item.
Usingcurrent_moment.to_be_bytes_vec()is a clean, type-safe way of persisting the numeric representation.packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/reward_distribution_type/mod.rs (3)
5-5: New imports for interval-based distributions.
Bringing in block/time/epoch intervals is consistent with the revised approach to reward distribution.
15-36: Refined documentation on block/time/epoch-based distribution variants.
The clarifications help show exactly when distributions begin and how they're triggered.
197-197: Updatedfmt::Displayarms for distributions.
Removing references tostartandendkeeps the display consistent with the simplified variants.Also applies to: 205-205, 213-213
packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/reward_distribution_moment/mod.rs (8)
6-6: AddedAddandDivtraits in the standard library import.
Facilitates new arithmetic operations on reward moments.
44-106: Introducedcycle_startmethod.
Smartly snaps the moment to the greatest multiple of the step by using modulo and saturating subtraction, while handling zero-step as an error.
108-173:steps_tilllogic for block/time-based calculations.
The inclusive/exclusive boundary flags appear correctly applied, and the zero-step case properly triggers an error.
175-206: Epoch-basedsteps_till.
This matches the block/time-based approach, ensuring consistent calculation while avoiding overflow via saturating arithmetic.
208-210: Fallback for mismatched moment types.
ReturningAddingDifferentTypeserror is a clear safeguard against invalid type usage.
222-251: Implementation ofAdd<u64>for distribution moments.
checked_addensures that arithmetic overflow surfaces as an explicit error rather than silently wrapping.
286-315: Implementation ofDiv<u64>.
Rejecting division by zero with a specific error is essential for robust numeric operations.
317-359:impl Div for RewardDistributionMoment.
Good handling of zero divisors via aDivideByZeroerror. Mismatched types also raise a clear error.packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/distribution/perpetual/time_based.rs (7)
22-264: Well-structured test scenario.
These lines thoroughly test time-based distribution with incremental claims and ensure correct detection of no-current-rewards after a maximum claim. The logic, especially the usage offast_forward_to_blockand subsequent claims, looks robust.
267-389: Test coverage for invalid claimant scenario.
Verifying that the transition rejects a claimant who is not the designated recipient is vital. These lines effectively ensure the error path is handled, demonstrating correct usage of the newInvalidTokenClaimWrongClaimanterror variant.
392-501: Accurate validation of specific identity claim logic.
These lines correctly confirm the correct recipient identity receives token rewards. The approach to verifying the final token balance and time-based intervals is clear and consistent.
693-883: Effective coverage of immediate re-claim error handling.
The test ensures no new rewards are distributed if the claim occurs too soon. This correctly covers negative path validation. No additional concerns here.
886-1074: Precise logic for delayed contract start.
These lines properly test a contract start offset (line 928) by fast-forwarding and verifying partial distribution. The test name clearly indicates the intent. Good job verifying time-based partial accrual.
1077-1258: Stress test for high-slope and starting amount.
This scenario checks large linear slopes and maximum distribution parameters, ensuring the distribution calculation does not overflow for typical usage. Great for boundary testing.
1261-1380: Robust loop-based overflow test.
Iterating the claim operation (lines 1312-1367) methodically validates incremental saturation or overflow behavior. This is a strong coverage approach for stress testing.packages/rs-drive/src/drive/tokens/mint/mod.rs (2)
54-67: Consider cross-check tests for saturation logic.
Adding calls totoken_mint_add_to_operationswithallow_saturationtoggled ON and OFF in your test suite ensures that the minting process is validated under both scenarios.Would you like a script to search existing tests in the codebase to verify coverage of these new parameters?
87-100: Maintain consistency in method parameters.
All three public methods (token_mint,token_mint_add_to_operations,token_mint_operations) now incorporateallow_saturation. This is consistent, but ensure any future expansions (e.g., alternative distribution strategies) share a similar approach for clarity.packages/rs-dpp/src/errors/consensus/state/token/invalid_token_claim_no_current_rewards.rs (2)
10-41: Excellent clarity in error struct.
The struct includes comprehensive fields and getters, conveying enough context to debug incomplete claims. The user-friendly display in#[error(...)]also improves troubleshootability.
72-78: Well-defined conversion intoConsensusError.
Mapping this error variant into aStateErroris neatly contained, aligning with the established consensus error patterns.packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/mod.rs (3)
56-63: Standardization of ExplicitNoneParameters in Contract Creation CallsThe diff adds an explicit
Noneargument to calls of
create_token_contract_with_owner_identity
across many test scenarios (token minting, token burning, token transfer, token configuration update, token freeze, etc.). This change aligns the tests with the updated API signature by standardizing the parameter list and improving clarity. Please verify that these insertedNonevalues correctly represent the intended default behavior for the newly required parameter.Also applies to: 144-153, 241-247, 3285-3291, 2969-2976, 3714-3717, 4388-4391, 4857-4860, 5159-5173
4925-4930: Consistent Update in Token Configuration Update TestsIn a couple of token configuration update tests (both in non-group and group scenarios), an explicit
Noneargument was also inserted in the contract–creation call. This adjustment ensures that the setup now conforms to the revised function signature. No logical changes have been introduced; the update is purely for consistency. Please double-check that the parameter position really represents the “unused” or default callback.Also applies to: 4953-4955
1-6133: Overall: Consistent Signature Adjustments Without Business Logic ChangesAcross the entire file, the only modifications consist of inserting explicit
Nonevalues into function calls (e.g. in the contract creation and configuration transition functions) to match updated signatures. There is no alteration to test logic or control flow. This update improves clarity and ensures consistency across the platform’s token distribution tests. Please ensure that any accompanying documentation or developer notes are updated to reflect the new API requirements.packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/distribution_function/evaluate_interval.rs (5)
10-27: Nicely documented interval evaluation.The expanded doc comments clearly explain the function behavior, parameters, and potential error cases. Including the optional
get_epoch_reward_ratioparameter highlights flexibility in epoch-based reward calculations. This improvement aids maintainability and readability.
50-50: Good defensive checks on step and interval bounds.Returning errors (or zero emission) when
step == 0orinterval_start_excluded >= interval_end_includedis appropriate for preventing invalid intervals. This avoids silent failures or undefined behavior in token emission calculations.Also applies to: 54-54
58-68: Efficient early return for fixed amounts.Short-circuiting for
DistributionFunction::FixedAmountavoids unnecessary iteration. Calculatingsteps_countup front is both performant and readable.
88-118: Robust epoch-based ratio handling.The block applying
get_ratio_fncarefully checks for missing epoch info and gracefully handles overflow viachecked_mul/checked_div. This reduces the risk of unexpected panics. Test coverage should confirm that partial ratio data triggers the intended error path.Would you like a test snippet covering scenario(s) with a missing epoch ratio to confirm correct error handling?
28-38:✅ Verification successful
Validate usage across callers.
evaluate_interval<F>now receives a genericFfor epoch-based ratio logic, which is flexible. Ensure that all call sites passing inget_epoch_reward_ratioalign with the new type signature. Where it’s unused, consider passingNone.
🏁 Script executed:
#!/bin/bash # Verifies usage of `evaluate_interval` across the codebase rg -A 10 "evaluate_interval"Length of output: 9960
Review Verification: Evaluate Interval Call Sites
After verifying the call sites, all usages of
evaluate_intervalare aligned with the updated generic signature. In particular, the caller in
packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/reward_distribution_type/evaluate_interval.rs
passesget_epoch_reward_ratiocorrectly. If epoch-based reward ratio logic isn’t needed at a specific call site, consider explicitly passingNonefor clarity.packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/distribution_function/mod.rs (2)
14-14: Added constant improves clarity.
MAX_LINEAR_SLOPE_PARAMbeing explicitly set to 256 centralizes a key bound. Ensure any validation logic or usage references this constant to avoid magic numbers.
132-132:✅ Verification successful
Consistent renaming enhances parameter clarity.
Updating fields from
stostart_moment(and similarly tostart_steporstarting_amount) clarifies intent for each distribution variant. This consistency makes the code more approachable. Verify all references to the old fields have been updated throughout the code.Also applies to: 239-240, 324-325, 382-383, 452-453, 511-511, 566-568, 592-593, 618-619, 644-645, 670-671
🏁 Script executed:
#!/bin/bash # Confirm no lingering references to old field names "s" or "b" rg '(\.s\b|\.b\b|s:|b:)'Length of output: 66200
Renaming Verification: Code Consistency Confirmed
The update from shorthand field names (e.g.,
sandb) to more descriptive identifiers such asstart_moment,start_step, andstarting_amounthas been applied consistently in the code. A repository-wide grep for the old identifiers produced no unwanted matches in the source code, confirming that all references have been updated.
- File Verified:
packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/distribution_function/mod.rs(and related distribution variant lines)- Grep Result: No lingering references to
.s,.b,s:, orb:were found in the relevant code files.The renaming not only clarifies the purpose of each parameter but also improves the overall maintainability of the code.
packages/rs-drive/src/drive/tokens/mint/v0/mod.rs (2)
17-17: New parameter improves overflow control.Introducing
allow_saturationadds an explicit mechanism to handle token supply saturation scenarios. Validate this parameter at call sites to ensure correct usage when near or at total supply limits.Also applies to: 30-30, 90-90
99-108: Refined logic for actual issuance amount.Capturing
(add_to_supply_operations, actual_issuance_amount)and then applyingactual_issuance_amountto the identity’s balance is a robust approach. This ensures token supply changes remain consistent with the newly computed issuance. Comprehensive tests should confirm partial issuance behavior when saturation blocks full issuance.Would you like a test scenario demonstrating partial issuance under max supply to verify correct behavior?
Also applies to: 110-110, 116-116, 122-122
packages/rs-drive/src/drive/tokens/system/add_to_token_total_supply/mod.rs (5)
7-7: No concerns with the new TokenAmount import.
Replaces a primitive type with a domain-specific TokenAmount, which should improve type safety.
74-74: Good concurrency of parameters.
Forwarding theallow_saturationparameter here maintains consistency across functions.
94-96: Check for potential integer size mismatch withTokenAmount.
Now returning(Vec<LowLevelDriveOperation>, TokenAmount). Confirm the rest of the pipeline expectsTokenAmountrather than raw integers and that no unexpected overflow or conversion occurs within the new return type usage.Also applies to: 102-102
114-114: Consistent call signature propagation.
Forwardingallow_saturationfrom this method to the v0 layer is cohesive with the rest of the code changes.
37-39:❓ Verification inconclusive
Double-check call site coverage.
You're forwardingallow_saturationtoadd_to_token_total_supply_v0. Verify that all external calls properly handle or propagate this bool. This ensures consistent logic across the codebase.
🏁 Script executed:
#!/bin/bash # Searching for usage of `add_to_token_total_supply` across the repository to confirm correct parameter usage rg "add_to_token_total_supply\(" -A 4 -B 4Length of output: 947
Please verify that all higher-level calls propagate
allow_saturationcorrectly.The grep search only returned the definition in
packages/rs-drive/src/drive/tokens/system/add_to_token_total_supply/mod.rs– we couldn’t identify additional external call sites from this search. This leaves some uncertainty as to whether every caller (or wrapper) that invokesadd_to_token_total_supplyeventually passes along the appropriate value forallow_saturationwhen callingadd_to_token_total_supply_v0. Please double-check that any higher-level (or external) invocations also handle or forward the boolean correctly, ensuring logic consistency across the codebase.packages/rs-drive/src/drive/tokens/system/add_to_token_total_supply/v0/mod.rs (3)
8-8: TokenAmount import is consistent with top-level changes.
This aligns with switching fromu64toTokenAmount.
93-93: New return struct captures the actual added token amount.
Returning(Vec<LowLevelDriveOperation>, TokenAmount)clarifies the operation results. Looks good for post-processing in calling layers.Also applies to: 99-99
57-59:✅ Verification successful
Confirm that external calls pass
TokenAmountcorrectly.
The function signature now takesTokenAmountand returnsTokenAmount. Double-check that all call sites handle or convert the value properly to avoid misunderstandings about the underlying numeric representation.Also applies to: 64-64
🏁 Script executed:
#!/bin/bash # Searching references to add_to_token_total_supply_add_to_operations_v0 function rg "add_to_token_total_supply_add_to_operations_v0" -A 5 -B 5Length of output: 3560
Confirmed proper handling of TokenAmount across calls
After reviewing the call sites in packages/rs-drive/src/drive/tokens/system/add_to_token_total_supply/v0/mod.rs, it’s clear that theTokenAmountparameter is passed intoadd_to_token_total_supply_add_to_operations_v0consistently and without any unintended conversions. Both the function signature and its usage (at lines 57–59 and line 64) indicate that external calls correctly provide and receiveTokenAmount, ensuring the underlying numeric representation is maintained as expected.packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/distribution_function/validation.rs (4)
8-8: Added new constant for better validation of linear slope parameters.The introduction of
MAX_LINEAR_SLOPE_PARAMas a separate constant fromMAX_DISTRIBUTION_PARAMallows for more precise constraints specifically for linear slope parameters.
145-146: Improved parameter naming for better readability.Renaming the
sparameter tostart_stepandbtostarting_amountin theLinearvariant significantly improves code clarity and maintainability. This descriptive naming makes the code more self-documenting.
167-173: Enhanced validation with dedicated slope parameter bounds.The validation logic now uses the dedicated
MAX_LINEAR_SLOPE_PARAMconstant instead of the genericMAX_DISTRIBUTION_PARAMfor the slope parameter (a). This provides more appropriate bounds specifically tailored for linear functions.
223-228: Updated function invocation with renamed parameters and contract registration step.The code now correctly uses the renamed parameters
start_stepandstarting_amountand also passes a new parameter (0) to theevaluatemethod. This suggests that the evaluation now needs a contract registration step parameter to function correctly.packages/rs-drive/src/drive/tokens/distribution/add_pre_programmed_distribution/v0/mod.rs (2)
4-7: Imported new token distribution path constants.Added new path constants for token distribution management, enhancing the module's capabilities to track last claims for both perpetual and pre-programmed distributions.
137-137: Modified cost estimation to use optional keys.Changed the argument from a direct keys reference to an optional reference using
Some(distribution.distributions().keys()). This change aligns with updated function signature requirements for cost estimation.packages/rs-drive-abci/src/execution/platform_events/fee_pool_outwards_distribution/add_epoch_pool_to_proposers_payout_operations/mod.rs (1)
6-6: Added BTreeMap import for improved data structure.Imported BTreeMap from standard collections, which will be used to replace the vector of tuples in the return type.
| identity_balance_holder_id: self.identity_balance_holder_id(), | ||
| mint_amount: self.mint_amount(), | ||
| allow_first_mint: false, | ||
| allow_saturation: false, |
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.
💡 Verification agent
🧩 Analysis chain
New parameter: allow_saturation added to TokenMint operation
The addition of the allow_saturation parameter (set to false by default) provides more control over token minting behavior. This is a good conservative default that prevents unexpected saturation conditions.
Can you confirm that all other implementations of TokenOperationType::TokenMint have been updated to include this new parameter? This will help ensure consistency across the codebase.
🏁 Script executed:
#!/bin/bash
# Search for other instances of TokenMint to ensure all have the allow_saturation parameter
rg -A 3 -B 3 "TokenOperationType::TokenMint" --type rustLength of output: 3585
Ensure Consistency Across All TokenMint Implementations
The updated code in token_mint_transition.rs correctly adds the new allow_saturation parameter (defaulting to false) to the TokenMint operation. However, the search results indicate that other instances of TokenOperationType::TokenMint—specifically within:
packages/rs-drive/src/util/batch/drive_op_batch/token.rspackages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_claim_transition.rs
do not include the new allow_saturation parameter. Please update these invocations to include allow_saturation for consistency across the codebase.
| fn next_interval(&self, block_info: &BlockInfo) -> RewardDistributionMoment { | ||
| match self.distribution_type { | ||
| // If the distribution is based on block height, return the next height where emissions occur. | ||
| RewardDistributionType::BlockBasedDistribution { interval, .. } => BlockBasedMoment( | ||
| (block_info.height - block_info.height % interval).saturating_add(interval), | ||
| ), | ||
|
|
||
| // If the distribution is based on time, return the next timestamp in milliseconds. | ||
| RewardDistributionType::TimeBasedDistribution { interval, .. } => TimeBasedMoment( | ||
| (block_info.time_ms - block_info.time_ms % interval).saturating_add(interval), | ||
| ), | ||
|
|
||
| // If the distribution is based on epochs, return the next epoch index. | ||
| RewardDistributionType::EpochBasedDistribution { interval, .. } => EpochBasedMoment( | ||
| (block_info.epoch.index - block_info.epoch.index % interval) | ||
| .saturating_add(interval), | ||
| ), | ||
| } | ||
| } |
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.
💡 Verification agent
🧩 Analysis chain
The next_interval method correctly returns structured reward distribution moments
The refactoring from returning raw u64 values to structured RewardDistributionMoment variants improves type safety and makes the code more self-documenting. The implementation handles all three distribution types appropriately.
However, I notice that saturating_add is still being used instead of checked_add. Based on previous feedback, consider using checked arithmetic to prevent potential overflows.
- (block_info.height - block_info.height % interval).saturating_add(interval),
+ block_info.height.checked_sub(block_info.height % interval)
+ .and_then(|result| result.checked_add(interval))
+ .unwrap_or(u64::MAX),Similar changes should be applied to the time-based and epoch-based calculations.
🏁 Script executed:
#!/bin/bash
# Check if there are any instances in the codebase where overflow might cause issues
rg "saturating_add|saturating_sub" --type rust packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/Length of output: 2404
Action Required: Replace Saturating Arithmetic with Checked Arithmetic
The refactoring to return strongly typed RewardDistributionMoment variants is a solid improvement, enhancing type safety and clarity. However, the use of saturating_add (and related saturating_sub) in the computations might silently mask overflow issues. Based on the repository-wide usage (confirmed by the search), we should update these arithmetic operations to use checked_sub/checked_add with proper fallback logic. For example, adjust the block-based distribution calculation as follows:
- (block_info.height - block_info.height % interval).saturating_add(interval),
+ block_info.height.checked_sub(block_info.height % interval)
+ .and_then(|result| result.checked_add(interval))
+ .unwrap_or(u64::MAX),Action Items:
- Apply the above pattern to the block-based, time-based, and epoch-based distribution calculations in
packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/v0/methods.rs. - Review similar arithmetic operations in other related modules (e.g., in
reward_distribution_moment/mod.rsanddistribution_function/evaluate.rs) to ensure consistent overflow protection.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fn next_interval(&self, block_info: &BlockInfo) -> RewardDistributionMoment { | |
| match self.distribution_type { | |
| // If the distribution is based on block height, return the next height where emissions occur. | |
| RewardDistributionType::BlockBasedDistribution { interval, .. } => BlockBasedMoment( | |
| (block_info.height - block_info.height % interval).saturating_add(interval), | |
| ), | |
| // If the distribution is based on time, return the next timestamp in milliseconds. | |
| RewardDistributionType::TimeBasedDistribution { interval, .. } => TimeBasedMoment( | |
| (block_info.time_ms - block_info.time_ms % interval).saturating_add(interval), | |
| ), | |
| // If the distribution is based on epochs, return the next epoch index. | |
| RewardDistributionType::EpochBasedDistribution { interval, .. } => EpochBasedMoment( | |
| (block_info.epoch.index - block_info.epoch.index % interval) | |
| .saturating_add(interval), | |
| ), | |
| } | |
| } | |
| fn next_interval(&self, block_info: &BlockInfo) -> RewardDistributionMoment { | |
| match self.distribution_type { | |
| // If the distribution is based on block height, return the next height where emissions occur. | |
| RewardDistributionType::BlockBasedDistribution { interval, .. } => BlockBasedMoment( | |
| block_info.height.checked_sub(block_info.height % interval) | |
| .and_then(|result| result.checked_add(interval)) | |
| .unwrap_or(u64::MAX), | |
| ), | |
| // If the distribution is based on time, return the next timestamp in milliseconds. | |
| RewardDistributionType::TimeBasedDistribution { interval, .. } => TimeBasedMoment( | |
| block_info.time_ms.checked_sub(block_info.time_ms % interval) | |
| .and_then(|result| result.checked_add(interval)) | |
| .unwrap_or(u64::MAX), | |
| ), | |
| // If the distribution is based on epochs, return the next epoch index. | |
| RewardDistributionType::EpochBasedDistribution { interval, .. } => EpochBasedMoment( | |
| block_info.epoch.index.checked_sub(block_info.epoch.index % interval) | |
| .and_then(|result| result.checked_add(interval)) | |
| .unwrap_or(u64::MAX), | |
| ), | |
| } | |
| } |
| Self::try_from_borrowed_token_claim_transition_with_contract_lookup( | ||
| drive, | ||
| owner_id, | ||
| &value, | ||
| approximate_without_state_for_costs, | ||
| transaction, | ||
| block_info, | ||
| user_fee_increase, | ||
| get_data_contract, | ||
| platform_version, | ||
| None, | ||
| )?; | ||
|
|
||
| let base_action = match base_action_validation_result.is_valid() { | ||
| true => base_action_validation_result.into_data()?, | ||
| false => { | ||
| let bump_action = BumpIdentityDataContractNonceAction::from_token_base_transition( | ||
| base, | ||
| owner_id, | ||
| user_fee_increase, | ||
| ); | ||
| let batched_action = | ||
| BatchedTransitionAction::BumpIdentityDataContractNonce(bump_action); | ||
|
|
||
| return Ok(( | ||
| ConsensusValidationResult::new_with_data_and_errors( | ||
| batched_action.into(), | ||
| base_action_validation_result.errors, | ||
| ), | ||
| fee_result, | ||
| )); | ||
| } | ||
| }; | ||
|
|
||
| Ok(( | ||
| BatchedTransitionAction::TokenAction(TokenTransitionAction::ClaimAction( | ||
| TokenClaimTransitionActionV0 { | ||
| base: base_action, | ||
| amount: 0, //todo | ||
| distribution_info: todo!(), | ||
| public_note, | ||
| } | ||
| .into(), | ||
| )) | ||
| .into(), | ||
| fee_result, | ||
| )) | ||
| ) | ||
| } |
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.
🛠️ Refactor suggestion
Refactor to try_from_borrowed_token_claim_transition_with_contract_lookup.
Eliminates data-copy overhead and unifies the logic in a single place.
|
Merging this in to make a pre-release. @shumkov please review when you can. |
chore: update to latest dash core 37 (#2483) feat(platform)!: token advanced distribution and updates (#2471) fix: token history contract (#2474) Co-authored-by: Ivan Shumkov <[email protected]> Co-authored-by: QuantumExplorer <[email protected]> fix(drive): using new rust dash core methods for reversed quorum hash to maintain backwards compatibility (#2489) feat: more granular integer document property types (#2455) Co-authored-by: Quantum Explorer <[email protected]> docs: update comment for data contract code range (#2476) feat: validate token name localizations (#2468) feat(sdk): get identity by non-unique keys build(deps): update grovedb to current develop test: test identity by non-unique pubkey hashes fix(sdk): dash core client fails to get quorum chore: minor fixes test(drive-abci): identity by non-unique pubkey start after chore: minor changes to verify feat(sdk): token and group queries (#2449) chore: revert limit 1 => limit none chore: add non-unique key to test identities test(sdk): test vectors for test_fetch_identity_by_non_unique_public_keys fix(platform)!: token distribution fixes and tests (#2494) chore(platform): bump to version 2.0.0-dev.1 (#2495) test: update assertion fix(sdk): make some things public (#2496) feat(platform): require token for document actions (#2498) fix: data contract proof doesn't work with new auto fields (#2501)
chore: update to latest dash core 37 (#2483) feat(platform)!: token advanced distribution and updates (#2471) fix: token history contract (#2474) Co-authored-by: Ivan Shumkov <[email protected]> Co-authored-by: QuantumExplorer <[email protected]> fix(drive): using new rust dash core methods for reversed quorum hash to maintain backwards compatibility (#2489) feat: more granular integer document property types (#2455) Co-authored-by: Quantum Explorer <[email protected]> docs: update comment for data contract code range (#2476) feat: validate token name localizations (#2468) feat(sdk): get identity by non-unique keys build(deps): update grovedb to current develop test: test identity by non-unique pubkey hashes fix(sdk): dash core client fails to get quorum chore: minor fixes test(drive-abci): identity by non-unique pubkey start after chore: minor changes to verify feat(sdk): token and group queries (#2449) chore: revert limit 1 => limit none chore: add non-unique key to test identities test(sdk): test vectors for test_fetch_identity_by_non_unique_public_keys fix(platform)!: token distribution fixes and tests (#2494) chore(platform): bump to version 2.0.0-dev.1 (#2495) test: update assertion fix(sdk): make some things public (#2496) feat(platform): require token for document actions (#2498) fix: data contract proof doesn't work with new auto fields (#2501)
Summary by CodeRabbit
New Features
Improvements
Tests
Checklist:
For repository code-owners and collaborators only