foundry icon indicating copy to clipboard operation
foundry copied to clipboard

feat(cast): display public_key on wallet creation with "new" and "new-mnemonic" sub-commands

Open mablr opened this issue 6 months ago • 5 comments

Motivation

Resolve #9748

Solution

The method used to obtain public_key with the wallet is taken from "public-key" sub-command.

Update tests to match the new outputs:

  • update redactions for pubkey and to match various spacings
  • use raw data for assertion in "wallet_mnemonic_from_entropy" to avoid conflict with redactions

EDIT:

  • pubkey is displayed when verbosity level is >0

PR Checklist

  • [x] Updated tests
  • [x] ~Added Documentation~
  • [ ] Breaking changes (no as guarded behind -v flag)

mablr avatar May 23 '25 09:05 mablr

I think it is a bit verbose and for most use cases the level of detail seems unnecessary

@mablr would you mind expanding on where you use the public keys?

We already have the cast wallet public-key utility available to derive the public key from a private key in instances a user needs this

zerosnacks avatar Jun 03 '25 14:06 zerosnacks

This feature was initially requested by @coffee-converter.

@zerosnacks I agree with you and I shared the same concerns in comments on the related issue: https://github.com/foundry-rs/foundry/issues/9748#issuecomment-2817232989 & https://github.com/foundry-rs/foundry/issues/9748#issuecomment-2870178362. Some of the information is outdated, but the main idea is in line with your remark.

After receiving feedback from @mattsse https://github.com/foundry-rs/foundry/issues/9748#issuecomment-2886913380, who assigned me to the issue, I finally implemented this feature.

mablr avatar Jun 03 '25 14:06 mablr

Hey @mablr, I see - thanks for the pointer!

To my understanding @mattsse refers to checking if this is verbose or not to indicate we should only display the public key when a user sets a verbosity flag (shell::verbosity() > 0). This is currently not implemented in the current version of the PR.

In my opinion, I don't see a huge benefit to having this be logged, even at higher verbosity.

I would be in favor of closing the ticket as not planned but if others feel strong about this - it being available at a higher verbosity level also works for me. This would also turn it into a non-breaking addition rather than a breaking change.

zerosnacks avatar Jun 03 '25 14:06 zerosnacks

I think you're right, limiting the pubkey display to a higher verbosity level (e.g. >1) is a good deal.

mablr avatar Jun 03 '25 14:06 mablr