Skip to content
This repository was archived by the owner on Mar 11, 2025. It is now read-only.

[transfer-hook] Remove deprecated functions from cli #6525

Merged

Conversation

tonton-sol
Copy link
Contributor

Removed the deprecated functions from the transfer-hook cli as per #6307

@mergify mergify bot added the community Community contribution label Mar 31, 2024
@joncinque joncinque requested a review from samkim-crypto April 1, 2024 11:25
Copy link
Contributor

@samkim-crypto samkim-crypto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution! Everything looks good to me, but I did make a mistake in the deprecation message for is_valid_signer in a previous PR. SignerSourceParserBuilder::default().build() does not actually accept any signer source kind. is_valid_signer should instead be replaced with SignerSourceParserBuilder::default().allow_all().build() instead. I made a PR to fix this anza-xyz/agave#531. Let's make the changes below and then I will approve the PR (I think is_valid_pubkey -> allow_pubkey().allow_file_path().build() is fine here). Thanks!

@@ -209,7 +205,7 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
Arg::new("fee_payer")
.long("fee-payer")
.value_name("KEYPAIR")
.validator(|s| is_valid_signer(s))
.value_parser(SignerSourceParserBuilder::default().build())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.value_parser(SignerSourceParserBuilder::default().build())
.value_parser(SignerSourceParserBuilder::default().allow_all().build())

@samkim-crypto
Copy link
Contributor

Oh and it would be great to update #6526 to change is_valid_signer with SignerSourceParserBuilder::default().allow_all().build(). Sorry to make you do extra work due to my mistake 🙏 .

@tonton-sol tonton-sol requested a review from samkim-crypto April 2, 2024 13:37
@samkim-crypto samkim-crypto merged commit d209ddf into solana-labs:master Apr 3, 2024
31 checks passed
@tonton-sol tonton-sol deleted the transfer_hook_cli_clap_v3 branch April 3, 2024 01:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
community Community contribution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants