This repository was archived by the owner on Mar 11, 2025. It is now read-only.
[token-cli] Remove is_amount_or_all, is_amount, and is_parsable validators#7448
Merged
samkim-crypto merged 5 commits intosolana-labs:masterfrom Nov 8, 2024
Merged
Conversation
87a679f to
099df00
Compare
099df00 to
3378e91
Compare
2501babe
previously approved these changes
Nov 7, 2024
3378e91 to
8d29c64
Compare
Pull request has been modified.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 ofclap_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::validatorin the token-cli should be replaced withArg::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_decimalsfrom the clap app and replacing it with the clap builtin parserclap::value_parser!(u8)instead. When actually parsing the inputs, we either used the deprecatedvalue_t_or_exitor parsed the input as a string and converted it tou8. I now updated it to directly useget_one::<u8>("decimals")instead.eff3494: For arguments related to token
amount, we used theis_amount_or_allvalidator that checks whether it is an unsigned integer (u64), decimals (f64), or theAllkeyword. In clap-v3, theAmounttype was added to parse these type of arguments, so I usedAmount::parseto replaceis_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_amountvalidator function as well, which can either be an unsigned integer (u64) or decimals (f64) (but notAllkeyword). I replaced these with eitherAmount::parseorclap::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 useMAXIMUM_FEEand sometimes useTOKEN_AMOUNT. I ended up fixing this to all useMAXIMUM_FEEinstead 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 parsersclap::value_parser!(TYPE).There are other validator functions like
is_valid_signerandis_valid_pubkeythat we use throughout the code. These will be removed in follow-up PRs.