Use keystores in Scaffold-Eth

[jrcarlos2000]

Description

Adds keystore for yarn generate and also for all scripts run inside script/deploy.s.sol. keystore is saved to '~/.foundry/keystores/scaffold-eth-default'

yarn account:generate // generates a random wallet and saves to keystore

yarn account:import // uses prompt to ask for private key and password

Additional Information

• [x] I have read the contributing docs (if this is your first contribution)
• [x] This is not a duplicate of any existing pull request

Related Issues

[rin-st]

Hey @jrcarlos2000 ! Overall working great!

But I noticed some things we possibly need to change:

• [ ] We need to add information to readme that after yarn:deploy we need to enter empty password when working locally or password we set for other chains
• [x] I think it's better to use another keystore for account:generate. Let's say I want to deploy my contract to sepolia. I run account:generate and then deploy it. Then I want to change contract, so I work locally again, and when yarn chain my generated sepolia account is gone with all my sepolia eth. So I need to save private key of sepolia account somewhere or generate a new one. But as I understand if we will use another keystore this problem disappears.
• [x] yarn account doesn't work
• [x] I'm getting this warning all the time on deploy, even when deploying to sepolia

  
  Warning (9302): Return value of low-level calls not used.
  --> script/DeployHelpers.s.sol:33:9:
  |
  33 |         deployer.call{ value: SCAFFOLD_BASE_BALANCE }("");
  |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[⠆] Compiling...

Can we get rid of it?
- [x] `yarn deploy:verify --network sepolia` returns error

Submitting verification for [lib/forge-std/src/Base.sol:CommonBase] 0xB3763733d9C88b1C9Db929319a6763F761D1661E. Encountered an error verifying this contract: Response: NOTOK Details: Invalid constructor arguments provided. Please verify that they are in ABI-encoded format

[jrcarlos2000]

thanks @rin-st , yes still testing out what would be the best Dev Ex here before coming up with an updated README.

I think it's better to use another keystore for account:generate. Let's say I want to deploy my contract to sepolia. I run account:generate and then deploy it. Then I want to change contract, so I work locally again, and when yarn chain my generated sepolia account is gone with all my sepolia eth. So I need to save private key of sepolia account somewhere or generate a new one. But as I understand if we will use another keystore this problem disappears.

so for this, we can do a pre-check, only generates a wallet after yarn chain, when there is no existing keystore.

I'm getting this warning all the time on deploy, even when deploying to sepolia

yes, will get rid of this

yarn deploy:verify --network sepolia returns error

i think you need to foundryup

[jrcarlos2000]

update : i have made a change:

when you do yarn account:generate or yarn account:import your keystore will be named scaffold-eth-custom, users will be able to set this on .env , please check the .env.example. so they can either used the anvil account ( scaffold-eth-default) or the imported / generated account ( at scaffold-eth-custom )

we also allow them to give it a name if provided.

yarn account:generate --name KEYSTORE_NAME , will need to update on .env

yarn account:import --name KEYSTORE_NAME , will need to update on .env

KEYSTORE_NAME defaults to scaffold-eth-custom

yarn account works and will use the provided keystore name on .env it will require you to unlock this account with password

[rin-st]

Thanks for the quick fixes! 🔥

Regarding yarn account. Could you take address from scaffold-eth-custom keystore, not as parameter? It worked without parameters before and also we don't need to save address before passing to yarn account

[jrcarlos2000]

Thanks for the quick fixes! 🔥

Regarding yarn account. Could you take address from scaffold-eth-custom keystore, not as parameter? It worked without parameters before and also we don't need to save address before passing to yarn account

yes this will be taken from the .env , it will be set to scaffold-eth-default first and scaffold-eth-custom when user has generated a wallet. However you still need to unlock it. we still need the password to decrypt the keystore.

cc: @technophile-04

[technophile-04]

After running yarn chain and then yarn deploy it gives:

Also after doing yarn account:generate and then yarn account (to view the generated account details) I get :

[technophile-04]

Tysm Carlos for this!!

As I understand ETH_KEYSTORE_ACCOUNT basically denotes which keystore to use.

SE-2 basically will be interacting with two keystore named:

1. scaffold-eth-default => This keystore uses anvil's 0th account private key, password is set to empty string ''
2. scaffold-eth-custom => This will be generated when you run yarn account:generate, users sets the password.

I kinda like how we are getting account of keystore from .env. This means if I have created an keystore account "my-account", I could just add it in .env#ETH_KEYSTORE_ACCOUNT=my-account and everything should work in context of my-account address as deployer / sending transaction and also yarn account will list details of my-account address.

^ So if above is true(it seems to be, on my testing) then this makes yarn account:import kind of redundant? If people want to use their preferred account with PK then they can just update .env#ETH_KEYSTORE_ACCOUNT with their account?

Difference b/w current SE-2 flow and keystore flow is:

1. Whenever you run yarn deploy(on localhost) you need to hit "Enter"
2. After you run yarn account:generate, you have to manually update .env#ETH_KEYSTORE_ACCOUNT with scaffold-eth-custom to use account generated by you to deploy contract.
3. If you do yarn deploy(on localhost) after pt.2, user has to enter scaffold-eth-custom password instead of hitting enter since now we will be using scaffold-eth-custom account always.

Some DX improvements:

1. Considering point 2. and 3, I think we should show the user whats the current account/address being used at that time when he is entering the password(Because I was kind of confused initally, should I hit "Enter" or use my generated account password without looking at .env)

2. (optional): Umm thinking out loud, with keystore it feels like we should have single(testnet/mainnet) deploy account for all SE-2 instance. Currently(in this branch) it removes the previous instance scaffold-eth-custom from keystore and generates a new one.

   • Imagninary flow:
     1. Users run yarn generate => we check if scaffold-eth-custom is already present in keystore
     2. If not, we create scaffold-eth-cusom with random address.
     3. If its present, then in CLI we directly show the "Using address/account 0x23423423", and then "Enter the password"
     4. There could be an another script like yarn account:dangerous-re-generate (this will remove scaffold-eth-custom) and generate new one

But yeah would love to know others thought as well!

[jrcarlos2000]

Hey @technophile-04 thanks for the comments :

1. the flow is correct
2. the yarn account:import is a helper function for a non trivial operation cast wallet import ${1:-scaffold-eth-custom} --interactive // here is worth mentioning that the user can select the name if they pass --name {ACCOUNT_NAME} and use it later on the .env if i want to put my current pk into a keystore i would need to use this command anyways ( or something similar ) this is interactive which is good!
3. what we can add is setting the custom or "named" account to be the one that .env points to, after the user calls yarn generate or yarn account:import . I wouldnt prefer this imo

[technophile-04]

Stressing on the point, in my thinking keystore kind of changes how we have been preaching to deploy contracts to testnet / mainnet.

Let's suppose I have 2 SE-2 instance (DAO-1 and NFT-proj-2).

Previously each of the project had their respective deployer key independent of each other. Example running yarn generate in NFT-proj-2 won't affect DAO-1 project's pk at all.

With keystore since we have only one global namespace(sacffold-eth-custom), running yarn generate in NFT-proj-2 will override the pk of DAO-1 project(loosing the deployer account of DAO-1 project)

So, Ideally I think yarn generate should tell the user do you want to override previous account or not?(if its second time). If its very first time we create an account.

Similarly to here, we could mention if you want to use your personal keystore account, you could update the .env#ETH_KEYSTORE_ACCOUNT with your preffered account name.

2. the yarn account:import is a helper function for a non trivial operation cast wallet import ${1:-scaffold-eth-custom} --interactive // here is worth mentioning that the user can select the name if they pass --name {ACCOUNT_NAME} and use it later on the .env if i want to put my current pk into a keystore i would need to use this command anyways ( or something similar ) this is interactive which is good!

Umm I kind of confused and like 51% not to add yarn account:import and 49% to add it.

The reason :

1. It add volume to scripts in package.json, if we didn't have it then the options could be just yarn generate (to generate an account), yarn acount to list account details (same as current SE-2)
2. Its just an abstractions over cast wallet import ACCOUNT_NAME --interactive, also foundry people seems to prefer running their own commands (for beginners we could have a line about in the docs)
3. We don't have to do any magic, like removing scaffold-eth-custm etc.

3. what we can add is setting the custom or "named" account to be the one that .env points to, after the user calls yarn generate or yarn account:import . I wouldnt prefer this imo

Lol confused on this too. Both option makes sense to me but yeah telling people to manually update .env with scaffold-eth-custom/ their prefered account.

Also @jrcarlos2000 can we achieve point 1. of "Some DX improvements" from https://github.com/scaffold-eth/scaffold-eth-2/pull/878#issuecomment-2220397549, I think it should be just adding echo 'using $ETH_KEYSTORE_ACCOUNT' before everything?

[jrcarlos2000]

Its just an abstractions over cast wallet import ACCOUNT_NAME --interactive, also foundry people seems to prefer running their own commands (for beginners we could have a line about in the docs)

I agree with you @technophile-04 , for DX 1, 2 we can achieve it. keeping only one single scaffold-eth keystore account

Lets see other thoughts !

[rin-st]

Some DX improvements:

1. Considering point 2. and 3, I think we should show the user whats the current account/address being used at that time when he is entering the password(Because I was kind of confused initally, should I hit "Enter" or use my generated account password without looking at .env)

2. (optional): Umm thinking out loud, with keystore it feels like we should have single(testnet/mainnet) deploy account for all SE-2 instance. Currently(in this branch) it removes the previous instance scaffold-eth-custom from keystore and generates a new one.

   • Imagninary flow:

     1. Users run yarn generate => we check if scaffold-eth-custom is already present in keystore
     2. If not, we create scaffold-eth-cusom with random address.
     3. If its present, then in CLI we directly show the "Using address/account 0x23423423", and then "Enter the password"
     4. There could be an another script like yarn account:dangerous-re-generate (this will remove scaffold-eth-custom) and generate new one

But yeah would love to know others thought as well!

Agree, good points!

what we can add is setting the custom or "named" account to be the one that .env points to, after the user calls yarn generate or yarn account:import . I wouldnt prefer this IMO

I wouldn't prefer this too

With keystore since we have only one global namespace

hm, I didn't take it into account!

So, Ideally I think yarn generate should tell the user do you want to override previous account or not?(if its second time). If its very first time we create an account. Similarly to here, we could mention if you want to use your personal keystore account, you could update the .env#ETH_KEYSTORE_ACCOUNT with your preffered account name.

+++

Umm I kind of confused and like 51% not to add yarn account:import and 49% to add it.

2. Its just an abstractions over cast wallet import ACCOUNT_NAME --interactive, also foundry people seems to prefer running their own commands (for beginners we could have a line about in the docs)

Since I'm not a foundry pro, I'd prefer to leave it. It's easier to check package.json than find information in the docs. We can always remove it if not needed

3. We don't have to do any magic, like removing scaffold-eth-custm etc.

As an option we can make a name required for account:import, so it won't rewrite scaffold-eth-custom by default