Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Encrypt deployer PK on .env file (when using hardhat) #1008

Merged
merged 8 commits into from
Dec 11, 2024
Merged

Conversation

carletex
Copy link
Member

@carletex carletex commented Dec 5, 2024

TODO:

  • yarn account:import

Update 1: Check #1008 (comment).

OG comment:

We have been talking about this for a while, and I'm drafting this PR to start the discussion and tinker. I think Hardhat v3 will come with a proper keystore (as foundry) but for now let's see if we can come with an easy solution.

I've been trying a bunch of things, but I've been finding roadblocks (maybe it's a good excuse to do our own simple package :D)

Let's be open about other options too... but I'm here presenting a simpler approach (which is not working yet, lol)

Goal

My goal is to have a simple password encryption system, so I went for the easies option: keep using the same .env file but store the encrypted account.

  • ✔️ yarn generate prompts for a password* (+ repeat to validate), generates a random account as before, and saves the encrypted data** on .env
DEPLOYER_PRIVATE_KEY={"address":"634fe4716de4cdce1b0f0f1538af9385305268ba","id":"a5193ccb-f08e-45ea-a45b-c929519c166c","version":3,"Crypto":{"cipher":"aes-128-ctr","cipherparams":{"iv":"5d6a4a86f7630419c93922332a65157f"},"ciphertext":"32bc2935a5339d62f6adddca1f94c7542b46ac9e779de7ad30922e7de2aea661","kdf":"scrypt","kdfparams":{"salt":"6ba9acac9f3427b404e160ed9b8e91c02594f51dafca357c9964b226003f2f22","n":131072,"dklen":32,"p":1,"r":8},"mac":"4870c238eeda8d5efd2a40e60b66f97df5fcc1af3c36b14d9d4207e596a0c9ca"},"x-ethers":{"client":"ethers/6.13.2","gethFilename":"UTC--2024-12-05T18-03-35.0Z--634fe4716de4cdce1b0f0f1538af9385305268ba","path":"m/44'/60'/0'/0/0","locale":"en","mnemonicCounter":"befb5d31b92467e64f4835cb59934db8","mnemonicCiphertext":"4f3bc210655d7f2b8407c159df9d0b10","version":"0.1"}}

image

  • ✔️ yarn account prompts for a password and shows the account info as before
    image

  • 🔴 hardhat.config.ts needs to be able to prompt for the password (at least when --network is not hardhat) so it's able to parse the PK. This is not working at the moment (so I removed `process.env.DEPLOYER_PRIVATE_KEY for now, so we can test the other commands). I'll continue to tinker, but open to ideas!! (so far, unsuccessful haha)

* I'm using @inquirer/password since it's really small
** I'm using wallet encrypt from ethers since we are already using ethers.

We'd also need a yarn account:import in case you want to import your PK (prompts for your PK and password)

What do you guys think?

@damianmarti
Copy link
Member

We can encrypt the PK using AES from the crypto library and save the encrypted PK. Then, when the user runs a yarn account or yarn deploy, we must ask for the password first, decrypt the PK, and set the DEPLOYER_PRIVATE_KEY env var with the decrypted PK. Am I missing something?

@carletex
Copy link
Member Author

carletex commented Dec 6, 2024

We can encrypt the PK using AES from the crypto library and save the encrypted PK. Then, when the user runs a yarn account or yarn deploy, we must ask for the password first, decrypt the PK, and set the DEPLOYER_PRIVATE_KEY env var with the decrypted PK. Am I missing something?

I think if you read carefully what I wrote and the current code, you'll see that what are you saying is the same thing that I'm trying.

Most of the stuff is already working (password encryption/decryption, yarn generate, yarn account), but there is an issue with how hardhat config works (+ no await in the config). You can check it out if you try it locally. Feel free to suggest any code if you make it work!

I'm currently trying an extra script to be run before any hardhat command to make it work (not the most fancy way) and load the env var.

@damianmarti
Copy link
Member

We can encrypt the PK using AES from the crypto library and save the encrypted PK. Then, when the user runs a yarn account or yarn deploy, we must ask for the password first, decrypt the PK, and set the DEPLOYER_PRIVATE_KEY env var with the decrypted PK. Am I missing something?

I think if you read carefully what I wrote and the current code, you'll see that what are you saying is the same thing that I'm trying.

Yeah! Sorry, you are right, it's the same.

Most of the stuff is already working (password encryption/decryption, yarn generate, yarn account), but there is an issue with how hardhat config works (+ no await in the config). You can check it out if you try it locally. Feel free to suggest any code if you make it work!

Will do!

I'm currently trying an extra script to be run before any hardhat command to make it work (not the most fancy way) and load the env var.

Yes, this was what I was thinking about. I think this is the easier way because it should be transparent to hardhat what we are doing.

@carletex
Copy link
Member Author

carletex commented Dec 6, 2024

Yes, this was what I was thinking about. I think this is the easier way because it should be transparent to hardhat what we are doing.

I think this might be the only way... And maybe that's why https://github.com/smartcontractkit/env-enc/ has a weird workflow (they couldn't find a way around hardhat config)

I'll update in a bit. Thanks Damu!!

@carletex
Copy link
Member Author

carletex commented Dec 6, 2024

Update: pushed 3ae16c4 and it seems to be working.

This is the simplest way that I found to load the decrypted PK in memory and run everything as usual. I think deploy (when a network != hardhat/localhost) is the only place where we actually need the PK. (well maybe for hardhat scripts too).

So everything runs as always, but when you deploy it check if the network is specified. If so, decrypt the PK, load it into the env, spaw a process and run the deploy command as usual.

Added a ToDo on the top PR comment (yarn account:import)

Please test, and let me know if you find a better way or if I'm missing something. Also @Pabl0cks , could you check on Windows?

@rin-st
Copy link
Member

rin-st commented Dec 6, 2024

Tested it, works great to me!

I'll play with it next days but for now I think it's a good solution!

@damianmarti
Copy link
Member

Update: pushed 3ae16c4 and it seems to be working.

This is the simplest way that I found to load the decrypted PK in memory and run everything as usual. I think deploy (when a network != hardhat/localhost) is the only place where we actually need the PK. (well maybe for hardhat scripts too).

So everything runs as always, but when you deploy it check if the network is specified. If so, decrypt the PK, load it into the env, spaw a process and run the deploy command as usual.

Added a ToDo on the top PR comment (yarn account:import)

Please test, and let me know if you find a better way or if I'm missing something. Also @Pabl0cks , could you check on Windows?

Tested and it's working great!!

This will force to use the encrypted PK, right? If I want to use a wallet from a PK I won't be able to do it, right?

@carletex
Copy link
Member Author

carletex commented Dec 6, 2024

This will force to use the encrypted PK, right? If I want to use a wallet from a PK I won't be able to do it, right?

Not yet! But I plan to add a yarn account:importcommand, which will let you add your PK (will ask for PK + password and will encrypt it)

I'll tackle it (+ little tweaks) next week!

Also open to any other feedback. Thanks!!

@damianmarti
Copy link
Member

We'd also need a yarn account:import in case you want to import your PK (prompts for your PK and password)

Yeah, sorry again, it was there on the PR text from the beginning. I think I didn't sleep well last night :-)

package.json Show resolved Hide resolved
@carletex carletex marked this pull request as ready for review December 9, 2024 17:06
@carletex
Copy link
Member Author

carletex commented Dec 9, 2024

Ok, this is ready to review!

Pushed the yarn account:import script + some tweaks on the copy/comments/env var names.

Please test and let me know if I'm missing something. Left some comments in the code too

A couple of things that were on my mind.

  1. Harhdat scripts won't work out of the box (because you have to decrypt the PK before, as we do in runHardhatDeployWithPK. I think it's not a big deal (since people doing hardhat scripts are not beginners)
  2. Give a warning if we detect DEPLOYER_PRIVATE_KEY env var. I think it's not worth it...but was in my mind :D

Copy link
Member

@rin-st rin-st left a comment

Choose a reason for hiding this comment

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

Works great for me!

Added some nipticks/comments, but can be ignored or changed later. Thanks!

Copy link
Member

@damianmarti damianmarti left a comment

Choose a reason for hiding this comment

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

It's working great!! And the code looks good to me.

Copy link
Collaborator

@Pabl0cks Pabl0cks left a comment

Choose a reason for hiding this comment

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

Awesome job @carletex !! This looks great!

On windows, as usual, there is some weird stuff, but I think it's easily fixed!

  • yarn account works as expected. My git bash is a bit weird because it shows the raw password when you type it, but I will try on my laptop (the console on my desktop it's always been a bit buggy). On Windows Powershell the password is always hided 👌
  • yarn deploy throws error, but applied this solution and works great.

The error was:

Error: spawn hardhat ENOENT
    at Process.ChildProcess._handle.onexit (node:internal/child_process:286:19)
    at onErrorNT (node:internal/child_process:484:16)
    at processTicksAndRejections (node:internal/process/task_queues:82:21) {
  errno: -4058,
  code: 'ENOENT',
  syscall: 'spawn hardhat',
  path: 'hardhat',
  spawnargs: [ 'deploy' ]
}

Adding shell to both spawn in runHardhatDeployWithPK.ts when it's windows, fixed it, and hopefully doesn't break the rest of OS behaviours.

stdio: "inherit",
env: process.env,
shell: process.platform === 'win32'

Copy link
Collaborator

@technophile-04 technophile-04 left a comment

Choose a reason for hiding this comment

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

Tysm @carletex this is great! And works nicely, tested with different cases like --tags, import pk + generating and deployments in general everything seems to work smooth!

Hopefully it works nicely on windows and then update the docs 🙌

@carletex
Copy link
Member Author

carletex commented Dec 10, 2024

I knew it was going to fail on Windows! haha

Thanks all for the review!

Pushed c8b08a7. Thanks @Pabl0cks! It works for me. Let's see if it works for the MacOS guys too :)

There are a couple of open things:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants