Skip to content
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

[token-cli] Remove is_amount_or_all, is_amount, and is_parsable validators #7448

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

samkim-crypto
Copy link
Contributor

@samkim-crypto samkim-crypto commented Nov 3, 2024

Problem

The token-cli now uses clap-v3, but there are still some deprecated functions that should be removed (we ultimately want to remove #![allow(deprecated)] at the top of clap_app.rs). In particular, with clap-v3, input validation has been deprecated in favor of just parsing the inputs.

All of the occurrences of Arg::validator in the token-cli should be replaced with Arg::value_parser. Since there are many of these occurrences, I think these can be removed in a sequence of smaller PRs.

Summary of Changes

In this PR, I started removing validator functions related to numbers and amounts. I think the changes should be pretty straightforward:

318244b: I started with removing validator is_mint_decimals from the clap app and replacing it with the clap builtin parser clap::value_parser!(u8) instead. When actually parsing the inputs, we either used the deprecated value_t_or_exit or parsed the input as a string and converted it to u8. I now updated it to directly use get_one::<u8>("decimals") instead.

eff3494: For arguments related to token amount, we used the is_amount_or_all validator that checks whether it is an unsigned integer (u64), decimals (f64), or the All keyword. In clap-v3, the Amount type was added to parse these type of arguments, so I used Amount::parse to replace is_amount_or_all. When actually parsing the inputs, I had to use match statements, which makes things a little bit more verbose, but I think the code itself should be a lot more readable.

06b0c1f: For the arguments, we use the is_amount validator function as well, which can either be an unsigned integer (u64) or decimals (f64) (but not All keyword). I replaced these with either Amount::parse or clap::value_parser!(u64) whichever made sense. In the process, I noticed that we had some inconsistency in the way we name the argument for maximum fee. We sometimes use MAXIMUM_FEE and sometimes use TOKEN_AMOUNT. I ended up fixing this to all use MAXIMUM_FEE instead for consistency. This renaming should just be a cosmetic change.

ac50d8f: Finally, there were occurrences of validator is_parsable::<TYPE>. I replaced these with direct clap parsers clap::value_parser!(TYPE).

There are other validator functions like is_valid_signer and is_valid_pubkey that we use throughout the code. These will be removed in follow-up PRs.

@samkim-crypto samkim-crypto marked this pull request as ready for review November 4, 2024 02:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant