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

[toekn-cli] Replace is_valid_pubkey validator with signer source parser #7447

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

Conversation

samkim-crypto
Copy link
Contributor

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

Problem

In token-cli, there are still some deprecated functions from clap-v2 that should be removed. In particular, we still use input validators, which should be replaced with input parsers.

Summary of Changes

This is a follow-up to #7448. In this PR, I removed all occurrences of the is_valid_pubkey validation function.

Replacing is_valid_pubkey requires two steps.

  1. Replace is_valid_pubkey with either SignerSource::default().allow_all().build() or SignerSource::default().allow_pubkey().build() depending on whether we are only parsing pubkey or any signer source.
  2. In the command logic, use SignerSource::try_get_pubkey(...) to parse the pubkey.

The main commit that applies the steps above is 8d27503. In the previous commits 76f5da8, 58e0441 (I made a mistake in this PR, which I fixed in ae6082d), dc311a7, I did some ground work to update the Config associated functions to use the SignerSource::try_get_pubkey(...).

After this PR, there are still occurrences of is_valid_signer validation function. This will be removed in another follow-up.

@github-actions github-actions bot added the stale [bot only] Added to stale content; will be closed soon label Nov 18, 2024
@samkim-crypto samkim-crypto removed the stale [bot only] Added to stale content; will be closed soon label Nov 19, 2024
@samkim-crypto samkim-crypto force-pushed the token-cli/is-valid-signer branch 2 times, most recently from b6508ff to a696806 Compare November 20, 2024 00:54
@samkim-crypto samkim-crypto marked this pull request as ready for review November 20, 2024 00:54
@samkim-crypto samkim-crypto changed the title [toekn-cli] Replace is_valid_signer validator with signer source parser [toekn-cli] Replace is_valid_pubkey validator with signer source parser Nov 20, 2024
@samkim-crypto samkim-crypto force-pushed the token-cli/is-valid-signer branch from a696806 to 126335b Compare November 20, 2024 00:59
@samkim-crypto samkim-crypto force-pushed the token-cli/is-valid-signer branch 3 times, most recently from 6b17f63 to 76f5da8 Compare November 22, 2024 01:42
@samkim-crypto samkim-crypto force-pushed the token-cli/is-valid-signer branch from 64bd4c9 to ae6082d Compare November 22, 2024 03:27
@github-actions github-actions bot added the stale [bot only] Added to stale content; will be closed soon label Dec 9, 2024
@samkim-crypto samkim-crypto added do-not-close Add this tag to exempt a PR / issue from being closed automatically and removed stale [bot only] Added to stale content; will be closed soon labels Dec 12, 2024
@solana-labs solana-labs deleted a comment from Ownerbusinessjjfm Dec 12, 2024
@2501babe
Copy link
Member

oops sorry i didnt see this in my inbox until this notif, will do it this week!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-close Add this tag to exempt a PR / issue from being closed automatically
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants