solana icon indicating copy to clipboard operation
solana copied to clipboard

[clap-v3-utils] Replace `pubkeys_sigs_of` with `try_pubkeys_sigs_of`

Open samkim-crypto opened this issue 1 year ago • 1 comments

Problem

Due to the differences in the way value_of and is_present functions behave in clap-v2 and clap-v3, the try_ versions of different key parsers were added in https://github.com/solana-labs/solana/pull/33184. However, that PR missed a point in the logic in signer_from_path_with_config where pubkeys_sigs_of is still used instead of try_pubkeys_sigs_of. The panicking behavior of pubkeys_sigs_of when called on a non pre-specified argument, it is blocking the token-cli from being upgraded to clap-v3.

Summary of Changes

I replaced the pubkeys_sigs_of with try_pubkeys_sigs_of when parsing SignerSourceKind::Pubkey.

Technically, to be consistent with the rest of the module, we should call try_pubkeys_sigs_of(matches, SIGNER_ARG.name)?. However, this changes the behavior of the function. In clap-v2, if pubkeys_sigs_of(matches, SIGNER_ARG.name) is called when SIGNER_ARG.name is not pre-specified as a clap argument, then it returns None. Therefore, instead of returning error, I invoked .ok().flatten() on the resulting result value.

There is a bigger PR in the works https://github.com/solana-labs/solana/pull/34678 to clean up parsing of signer source. However, I just wanted to make sure that this change goes into 1.18, hence a smaller PR here.

Fixes #

samkim-crypto avatar Jan 17 '24 03:01 samkim-crypto

Codecov Report

Attention: 12 lines in your changes are missing coverage. Please review.

Comparison is base (8f9d915) 81.8% compared to head (032e2bc) 81.7%. Report is 27 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##           master   #34801     +/-   ##
=========================================
- Coverage    81.8%    81.7%   -0.1%     
=========================================
  Files         825      825             
  Lines      223269   223274      +5     
=========================================
- Hits       182635   182588     -47     
- Misses      40634    40686     +52     

codecov[bot] avatar Jan 17 '24 09:01 codecov[bot]