-
Notifications
You must be signed in to change notification settings - Fork 54
feat(platform)!: withdrawal limits #2182
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
Changes from all commits
b1f45b2
35062e3
6c3e318
211ec4f
fa53e84
82ac2c0
538f017
8af069d
802811c
385b78f
f5c07d2
9d2a56a
29b774a
baa33ea
16cb5b7
5cd1e83
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,3 +27,33 @@ macro_rules! dash_to_credits { | |
| credits as u64 | ||
| }}; | ||
| } | ||
|
|
||
| #[macro_export] | ||
| macro_rules! dash_to_duffs { | ||
| // The macro takes a string literal representing the Dash amount. | ||
| ($dash:expr) => {{ | ||
| let dash_str = stringify!($dash); | ||
|
|
||
| // Parsing the input string to separate the whole and fractional parts. | ||
| let parts: Vec<&str> = dash_str.split('.').collect(); | ||
| let mut credits: u128 = 0; | ||
|
|
||
| // Process the whole number part if it exists. | ||
| if let Some(whole) = parts.get(0) { | ||
| if let Ok(whole_number) = whole.parse::<u128>() { | ||
| credits += whole_number * 100_000_000; // Whole Dash amount to credits | ||
| } | ||
| } | ||
|
|
||
| // Process the fractional part if it exists. | ||
| if let Some(fraction) = parts.get(1) { | ||
| let fraction_length = fraction.len(); | ||
| let fraction_number = fraction.parse::<u128>().unwrap_or(0); | ||
| // Calculate the multiplier based on the number of digits in the fraction. | ||
| let multiplier = 10u128.pow(8 - fraction_length as u32); | ||
| credits += fraction_number * multiplier; // Fractional Dash to credits | ||
| } | ||
|
|
||
| credits as u64 | ||
| }}; | ||
| } | ||
|
Comment on lines
+31
to
+59
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider refactoring to reduce code duplication The You could create a helper macro that takes the multiplication factors as parameters: macro_rules! dash_to_units {
($dash:expr, $whole_multiplier:expr, $fraction_digits:expr) => {{
// Common logic here
}};
}
macro_rules! dash_to_credits {
($dash:expr) => {
dash_to_units!($dash, 100_000_000_000, 11)
};
}
macro_rules! dash_to_duffs {
($dash:expr) => {
dash_to_units!($dash, 100_000_000, 8)
};
}This approach would centralize the conversion logic and make it easier to maintain and extend in the future. Add input validation for negative numbers and invalid characters The current implementation doesn't validate the input for negative numbers or invalid characters. This could lead to unexpected behavior or panics. Consider adding input validation at the beginning of the macro: let dash_str = stringify!($dash);
if dash_str.starts_with('-') {
panic!("Negative Dash amounts are not supported");
}
if !dash_str.chars().all(|c| c.is_digit(10) || c == '.') {
panic!("Invalid characters in Dash amount");
}This will ensure that only valid, positive Dash amounts are processed. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| use crate::fee::Credits; | ||
| use crate::withdrawal::daily_withdrawal_limit::v0::daily_withdrawal_limit_v0; | ||
| use crate::ProtocolError; | ||
| use platform_version::version::PlatformVersion; | ||
|
|
||
| mod v0; | ||
|
|
||
| pub fn daily_withdrawal_limit( | ||
| total_credits_in_platform: Credits, | ||
| platform_version: &PlatformVersion, | ||
| ) -> Result<Credits, ProtocolError> { | ||
| match platform_version.dpp.methods.daily_withdrawal_limit { | ||
| 0 => Ok(daily_withdrawal_limit_v0(total_credits_in_platform)), | ||
| v => Err(ProtocolError::UnknownVersionError(format!( | ||
| "Unknown daily_withdrawal_limit version {v}" | ||
| ))), | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| use crate::fee::Credits; | ||
|
|
||
| /// Calculates the daily withdrawal limit based on the total credits available in the platform. | ||
| /// | ||
| /// The function enforces the following rules: | ||
| /// | ||
| /// 1. If the total credits are 1000 Dash in Credits or more: | ||
| /// - The withdrawal limit is set to 10% of the total credits. | ||
| /// 2. If the total credits are between 100 and 999 Dash in Credits: | ||
| /// - The withdrawal limit is capped at 100 credits. | ||
| /// 3. If the total credits are less than 100 Dash in Credits: | ||
| /// - The withdrawal limit is the total available credits, as no more than the available amount can be withdrawn. | ||
| /// | ||
| /// # Parameters | ||
| /// | ||
| /// * `total_credits_in_platform`: The total amount of credits available in the platform. | ||
| /// | ||
| /// # Returns | ||
| /// | ||
| /// * `Credits`: The calculated daily withdrawal limit based on the available credits. | ||
| /// | ||
| pub fn daily_withdrawal_limit_v0(total_credits_in_platform: Credits) -> Credits { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it credits in dash?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I mean it's credits
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, fixed. |
||
| if total_credits_in_platform >= 100_000_000_000_000 { | ||
| // 1000 Dash | ||
| total_credits_in_platform / 10 | ||
| } else if total_credits_in_platform >= 10_000_000_000_000 { | ||
| // 100 Dash | ||
| 10_000_000_000_000 | ||
| } else { | ||
| total_credits_in_platform | ||
| } | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use super::*; | ||
| use crate::dash_to_credits; | ||
|
|
||
| #[test] | ||
| fn test_daily_withdrawal_limit() { | ||
| assert_eq!( | ||
| daily_withdrawal_limit_v0(dash_to_credits!(2000)), | ||
| dash_to_credits!(200) | ||
| ); | ||
| assert_eq!( | ||
| daily_withdrawal_limit_v0(dash_to_credits!(500)), | ||
| dash_to_credits!(100) | ||
| ); | ||
| assert_eq!( | ||
| daily_withdrawal_limit_v0(dash_to_credits!(50)), | ||
| dash_to_credits!(50) | ||
| ); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,3 @@ | ||
| mod check_for_desired_protocol_upgrade; | ||
| mod perform_events_on_first_block_of_protocol_change; | ||
| mod upgrade_protocol_version; |
Uh oh!
There was an error while loading. Please reload this page.
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.
Potential overflow when casting to u64
The final conversion
credits as u64may cause an overflow for large Dash amounts. Consider adding a check to ensure the value fits within u64 before casting.You could add a runtime check:
Or, for a compile-time check (which would be preferable), you could use the
TryIntotrait: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.
No need as this is not used in production code.
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.
@QuantumExplorer, understood. Thanks for the clarification.
(•.•)?
✏️ Learnings added
Uh oh!
There was an error while loading. Please reload this page.
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.
@QuantumExplorer Please add test flag then if it's not using in production code
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.