Improve wallet management UX using Keystore #1869

hbarcelos · 2022-06-07T19:54:30Z

Component

Forge, Cast

Describe the feature you would like

I'm currently migrating from dapp.tools and it had this nice UX to handle keystore files:

Declare the following env vars:
- ETH_KEYSTORE: the path to the keystore directory
- ETH_FROM: the account from which send the tx from (I believe this was used to find the right keystore file inside the keystore directory).
- ETH_PASSWORD: the path to a plain-text password file with the password for the keystore file in the local file system.

Anything requiring wallet signing pickups the above variables automatically:

dapp create src/MyContract.sol:MyContract
seth send $ADDRESS "myFunc(address)" $ARG

I'm finding it a bit difficult to work with Foundry for that purpose. Specifically regarding contract deployment. Main differences are:

ETH_KEYSTORE works, but it expects a keystore file, not a directory.
ETH_FROM is redundant, since the sender wallet is derived from ETH_KEYSTORE
ETH_PASSWORD doesn't work.

Also if I use CAST_PASSWORD env var, forge create does not pick it up, so I need to be explicit when passing it:

forge create --password=${CAST_PASSWORD} src/MyContract.sol:MyContract.sol

Also ideally such configs could be in foundry.toml as well:

[default]
password-file = '~/.eth-password'

# either
keystore-directory = '~/.ethereum/keystore'
from = '0xdeAD00000000000000000000000000000000dEAd'

# or
keystore-file = '~/.ethereum/keystore/UTC--2022-01-01T00-00-00.000000000Z--dead00000000000000000000000000000000dead'

Furthermore, users should have the liberty to overwrite any configs with environment variables. For example, it's very useful to be able to define a keystore directory containing multiple accounts and define the from address with env vars.

FOUNDRY_ETH_FROM=0x0783...122 FOUNDRY_PASSWORD_FILE=~/.eth-password-0783...122 forge deploy ...

Additional context

As a workaround, I have written a small helper script to allow me to use forge the same way I could use dapp.tools. Perhaps this can help guiding the implementation.

#!/bin/bash

# scripts/deploy.sh

set -eo pipefail

function log() {
  echo -e "$@" >&2
}

function die() {
  log "$@"
  log ""
  exit 1
}

function err_msg_keystore_file() {
cat <<MSG
ERROR: could not determine the location of the keystore file.

You should either define:

\t1. The FOUNDRY_ETH_KEYSTORE_FILE env var or;
\t2. Both FOUNDRY_ETH_KEYSTORE_DIR and FOUNDRY_ETH_FROM env vars.
MSG
}

function err_msg_etherscan_api_key() {
cat <<MSG
ERROR: cannot verify contracts without ETHERSCAN_API_KEY being set.

You should either:

\t1. Not use the --verify flag or;
\t2. Define the ETHERSCAN_API_KEY env var.
MSG
}

function usage() {
cat <<MSG
deploy.sh contract_path [--constructor-args ...args]

Examples:

\t# Constructor does not take any arguments
\tdeploy.sh src/MyContract.sol:MyContract

\t# Constructor takes (uint, address) arguments
\tdeploy.sh src/MyContract.sol:MyContract --constructor-args 1 0x0000000000000000000000000000000000000000
MSG
}

function deploy() {
  local ENV_FILE="${BASH_SOURCE%/*}/../.env"
  [ -f "$ENV_FILE" ] && source "$ENV_FILE"

  FOUNDRY_ETH_FROM="${FOUNDRY_ETH_FROM:-$ETH_FROM}"
  FOUNDRY_ETHERSCAN_API_KEY="${FOUNDRY_ETHERSCAN_API_KEY:-$ETHERSCAN_API_KEY}"
  FOUNDRY_ETH_KEYSTORE_DIRECTORY="${FOUNDRY_ETH_KEYSTORE_DIRECTORY:-$ETH_KEYSTORE}"

  if [ -z "$FOUNDRY_ETH_KEYSTORE_FILE" ]; then
    [ -z "$FOUNDRY_ETH_KEYSTORE_DIRECTORY" ] && die "$(err_msg_keystore_file)"
    # Foundy expects the Ethereum Keystore file, not the directory.
    # This step assumes the Keystore file for the deployed wallet includes $ETH_FROM in its name.
    FOUNDRY_ETH_KEYSTORE_FILE="${FOUNDRY_ETH_KEYSTORE_DIRECTORY%/}/$(ls -1 $FOUNDRY_ETH_KEYSTORE_DIRECTORY | \
      # -i: case insensitive
      # #0x: strip the 0x prefix from the the address
      grep -i ${FOUNDRY_ETH_FROM#0x})"
  fi
  [ -z "$FOUNDRY_ETH_KEYSTORE_FILE" ] && die "$(err_msg_keystore_file)"

  # Handle reading from the password file
  local PASSWORD_OPT=''
  if [ -f "$FOUNDRY_ETH_PASSWORD_FILE" ]; then
    PASSWORD_OPT="--password=$(cat "$FOUNDRY_ETH_PASSWORD_FILE")"
  fi

  # Require the Etherscan API Key if --verify option is enabled
  set +e
  if grep -- '--verify' <<< "$@" > /dev/null; then
    [ -z "$FOUNDRY_ETHERSCAN_API_KEY" ] && die "$(err_msg_etherscan_api_key)"
  fi
  set -e

  # Log the command being issued, making sure not to expose the password
  log "forge create --keystore="$FOUNDRY_ETH_KEYSTORE_FILE" $(sed 's/=.*$/=[REDACTED]/' <<<${PASSWORD_OPT}) $@"
  forge create --keystore="$FOUNDRY_ETH_KEYSTORE_FILE" ${PASSWORD_OPT} $@
}

# Executes the function if it's been called as a script.
# This will evaluate to false if this script is sourced by other script.
if [ "$0" = "$BASH_SOURCE" ]; then
  if [ $# -eq 0 ]; then
    die "$(usage)"
  fi

  [ "$1" = '-h' ] || [ "$1" = '--help' ] && {
    log "$(usage)"
    exit 0
  }

  deploy $@
fi

# .env

export FOUNDRY_ETHERSCAN_API_KEY=API_KEY
export FOUNDRY_ETH_FROM=WALLET_ADDRESS
export FOUNDRY_ETH_KEYSTORE_DIRECTORY=PATH_OF_ETHEREUM_KEYSTORE_DIR # i.e.: ${HOME}/.ethereum/keystore/
export FOUNDRY_ETH_PASSWORD=PATH_OF_ETHEREUM_PASSWORD_FILE

The text was updated successfully, but these errors were encountered:

tynes · 2022-06-07T21:02:57Z

Counterparts for seth ls and seth accounts are missing as well. I know that seth uses the same tooling as geth for handling keystores and I do think the idea of ETH_KEYSTORE being a directory is a better idea, I think one problem is that geth uses a non standard keystore json format that includes the address in plaintext. If ETH_KEYSTORE is a directory, then foundry will need to create keystores in the same way to determine which json file is which key

hbarcelos · 2022-06-07T21:43:43Z

I think one problem is that geth uses a non standard keystore json format that includes the address in plaintext. If ETH_KEYSTORE is a directory, then foundry will need to create keystores in the same way to determine which json file is which key

If sticking to the standard keystore json format, you could keep the convention of including the address in the name of the file to easily find it.

gakonst · 2022-06-08T10:47:39Z

all good points, agreed we'd want them in

NiklasKunkel · 2022-07-18T17:41:29Z

Agree with everything here. Transitioning over from dapptools this has been quite frustrating to deal with.

godsflaw · 2022-09-23T19:14:46Z

Adding my vote in support of this request.

gakonst · 2022-09-23T22:43:06Z

Just to set expectations, nobody is working on this rn and we're not prioritizing it. Would happily accept a PR that implements something we agree to

PatrickAlphaC · 2023-04-12T21:51:12Z

Additional context: #3818

PatrickAlphaC · 2023-05-25T18:52:44Z

I will pay someone to implement this.

Import and encrypt a private key to a local keystore file

API:

cast wallet import

The usage/help command would look like this:

Imports and encrypts a private key to a keystore file. 

Usage: cast wallet import <ACCOUNT_NAME>

Commands:
    --keystores The file to store your encrypted keystore (defaults to ~/.foundry/keystores

Which prompts you for a private key that it will obfuscate and a password that it will also obfuscate.

Then ideally...

You could just run:

forge script scripts/<YOUR_SCRIPT> --account <account_name> --broadcast

And it will prompt you for the password to your keystore file.

It would be equivalent to:

forge script scripts/<YOUR_SCRIPT> --keystore ~/.foundry/<account_name>.json --password <password> --broadcast

PatrickAlphaC · 2023-05-25T19:17:20Z

Idk if/how OpenQ works, but YOLO.

https://openq.dev/contract/I_kwDOGBlvNc5LU_e_/0xd558ff76aa038da541202987c11ef61c0b8dacc2?invoiceable=false

Funded this issue here.

Deliverables (IMO):

Fix @hbarcelos's issue
Add my API (if the foundry core team likes it)
Ideally fix Support loading mnemonic + HD path as encrypted keystore #3818 as well

The three is probably worth 1000 MATIC right? (...maybe?)

rickkdev · 2023-06-02T12:22:46Z

Idk if/how OpenQ works, but YOLO.

https://openq.dev/contract/I_kwDOGBlvNc5LU_e_/0xd558ff76aa038da541202987c11ef61c0b8dacc2?invoiceable=false

Funded this issue here.

Deliverables (IMO):

Fix @hbarcelos's issue

Add my API (if the foundry core team likes it)

Ideally fix Support loading mnemonic + HD path as encrypted keystore #3818 as well

The three is probably worth 1000 MATIC right? (...maybe?)

It works :) merge the PR that is connected to this issue and the contributor can login to OpenQ and claim their money.

rplusq · 2023-07-17T18:25:19Z

I wanted to implement a cast wallet import command for saving accounts to keystores and simplifying the UX, this could either be by interactive private key / mnemonic derivation, while also specifying the name. I would use —account ACCOUNT_NAME for this when using cast send / forge scripts. I saw that there's already implementations for comes from the Wallet opts. But then I noticed that Wallet is used as opts in cast wallet, as well as MultiWallet is used as forge script options.

Seems like there are two steps then:

Add to MultiWallet the account param that based on the account name, looks for the keystore
Create the cast wallet import that creates those in the first place

For step 2, I would still leverage a lot of the existing methods from the MultiWallet struct, but not all of them. For example, I would just use interactive pk and mnemonic.

I see that foundry has used clap flatten(Wallet) even where it doesn't leverage all the options (ie: cast wallet address leveraging —from) and not use that later (if you use it, it still doesn't really do it :p)

So for better ux, I think it would be wrong to use flatten(Wallet), as it would show confusing options for the cli that don't really make sense

Should I just repeat code here? or is there a way to leverage the interactive / mnemonic options from Wallet without actually using the rest?

Hope my question makes sense, let me know if you have any other thoughts

Evalir · 2024-02-25T16:53:02Z

Closing, this is already done.

pathakadi · 2024-06-11T08:58:20Z

I will pay someone to implement this.

Import and encrypt a private key to a local keystore file

API:
cast wallet import 
The usage/help command would look like this:
Imports and encrypts a private key to a keystore file. 

Usage: cast wallet import <ACCOUNT_NAME>

Commands:
    --keystores The file to store your encrypted keystore (defaults to ~/.foundry/keystores
Which prompts you for a private key that it will obfuscate and a password that it will also obfuscate.

Then ideally...

You could just run:
forge script scripts/<YOUR_SCRIPT> --account <account_name> --broadcast
And it will prompt you for the password to your keystore file.

It would be equivalent to:
forge script scripts/<YOUR_SCRIPT> --keystore ~/.foundry/<account_name>.json --password <password> --broadcast

This feature is Awesome.

hbarcelos added the T-feature Type: feature label Jun 7, 2022

onbjerg added the C-cast Command: cast label Jun 9, 2022

onbjerg added this to the v1.0.0 milestone Jul 1, 2022

This was referenced Aug 6, 2023

feat: wallet import rplusq/foundry#1

Closed

improvement: cast wallet import / --account option #5551

Merged

Evalir closed this as completed Feb 25, 2024

ericywl mentioned this issue May 20, 2024

feat(forge test): add --account <some_account> flag to access wallet inside tests #7950

Open

bxpana mentioned this issue Aug 8, 2024

.ENV Pledge and keystore management Cyfrin/foundry-full-course-cu#2203

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve wallet management UX using Keystore #1869

Improve wallet management UX using Keystore #1869

hbarcelos commented Jun 7, 2022 •

edited

Loading

tynes commented Jun 7, 2022

hbarcelos commented Jun 7, 2022

gakonst commented Jun 8, 2022

NiklasKunkel commented Jul 18, 2022

godsflaw commented Sep 23, 2022

gakonst commented Sep 23, 2022

PatrickAlphaC commented Apr 12, 2023

PatrickAlphaC commented May 25, 2023 •

edited

Loading

PatrickAlphaC commented May 25, 2023 •

edited

Loading

rickkdev commented Jun 2, 2023 •

edited

Loading

rplusq commented Jul 17, 2023 •

edited

Loading

Evalir commented Feb 25, 2024

pathakadi commented Jun 11, 2024

Then ideally...

Improve wallet management UX using Keystore #1869

Improve wallet management UX using Keystore #1869

Comments

hbarcelos commented Jun 7, 2022 • edited Loading

Component

Describe the feature you would like

Additional context

tynes commented Jun 7, 2022

hbarcelos commented Jun 7, 2022

gakonst commented Jun 8, 2022

NiklasKunkel commented Jul 18, 2022

godsflaw commented Sep 23, 2022

gakonst commented Sep 23, 2022

PatrickAlphaC commented Apr 12, 2023

PatrickAlphaC commented May 25, 2023 • edited Loading

Then ideally...

PatrickAlphaC commented May 25, 2023 • edited Loading

rickkdev commented Jun 2, 2023 • edited Loading

rplusq commented Jul 17, 2023 • edited Loading

Evalir commented Feb 25, 2024

pathakadi commented Jun 11, 2024

Then ideally...

hbarcelos commented Jun 7, 2022 •

edited

Loading

PatrickAlphaC commented May 25, 2023 •

edited

Loading

PatrickAlphaC commented May 25, 2023 •

edited

Loading

rickkdev commented Jun 2, 2023 •

edited

Loading

rplusq commented Jul 17, 2023 •

edited

Loading