-
Notifications
You must be signed in to change notification settings - Fork 60
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
rusk-wallet: Fix stake lower limit when performing topup #3431
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In addition to the comments, remember to update the CHANGELOG
rusk-wallet/src/bin/io/prompt.rs
Outdated
has_stake: bool, | ||
) -> Result<Dusk, Error> { | ||
let min: Dusk = { | ||
let min_stake = if has_stake { 0 } else { DEFAULT_MINIMUM_STAKE }; |
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.
You should not allow 0 as input
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.
Changed it to 1
let owner = match wallet.find_stake_owner_account(stake_pk).await { | ||
Ok(account) => account, | ||
Ok(account) => { |
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.
Having a stake owner is not enough to know if there is a stake amount
Indeed, if the user has no stake but still has rewards to withdraw, this match will incorrectly results in "has_stake=true"
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.
@herr-seppia I fixed this check please check again
021e39d
to
a9cccd7
Compare
a9cccd7
to
611a4aa
Compare
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.
rusk-wallet uses the parent Cargo.lock, please remove this file
@@ -169,6 +169,9 @@ pub(crate) async fn online( | |||
.public_key(stake_idx) | |||
.expect("public key to exists in interactive mode"); | |||
|
|||
let has_stake = | |||
wallet.check_stake_exists(stake_pk).await.unwrap_or(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.
If check_stake_exists
return an error, then it should be propagated.
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.
Additionally, do we really need to add the new check_stake_exists
when we already have the stake_info
method?
let has_stake = wallet
.stake_info(stake_idx)
.await?
.map(|s| s.amount.is_some())
.unwrap_or_default();
@@ -266,8 +266,15 @@ pub(crate) fn request_optional_token_amt( | |||
} | |||
|
|||
/// Request amount of tokens that can't be lower than MINIMUM_STAKE |
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.
Please update the comment
let min: Dusk = DEFAULT_MINIMUM_STAKE.into(); | ||
pub(crate) fn request_stake_token_amt( | ||
balance: Dusk, | ||
has_stake: bool, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why don't you get the min_value instead of has_stake?
has_stake: bool, | ||
) -> Result<Dusk, Error> { | ||
let min: Dusk = { | ||
let min_stake = if has_stake { 1 } else { DEFAULT_MINIMUM_STAKE }; |
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.
can you use MIN_CONVERTIBLE
instead of 1
?
Closes #3394