-
Notifications
You must be signed in to change notification settings - Fork 7
Use obfuscated key in deployment #68
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
Conversation
script/ObfuscateKey.sol
Outdated
|
|
||
| library Obfuscator { | ||
| bytes32 internal constant SHIFT = | ||
| 0x1337133713371337133713371337133713371337133713371337133713371337; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤔 I'm curious, why 0x1377 and not 0xDef1Beef
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Standard placeholder entry (leet or formally 31337, elite), together with 42 (answer to everything). I picked it up from Nick a few years ago and it stuck. 😄
Anyway, this is now cowified in 4d58380. ✔️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hahaha! perfect, now I'm approving!
| To broadcast the deployment onchain, append `--broadcast` to the command above. | ||
| `ETHFLOW_OBFUSCATED_PK` is an obfuscated version of the private key used in the deployment, _not_ a raw public key. | ||
| The purpose of obfuscating the key is making sure the same key isn't used by accident to deploy other contracts, thereby consuming the nonce of the deployer used for deterministic addresses. | ||
| It's not a security mechanism: the key is trivially recovered from the obfuscated version. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes sense, and would prevent someone from having it in their env and deploying something else.
Do you plan to also update in secret manager the key to obfuscate, so we prevent people from importing that into anywere and also making it easier to use only as input of this script?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The secret should contain an entry explicitly named ETHFLOW_OBFUSCATED_PK for ease of copypaste and the actual private key just in case it's ever needed with a warning "do not use."
A very simple mechanism to make sure that the deployer key for the ETH flow isn't used for deploying unrelated contracts.
The ETH-flow contract deployment is notoriously fragile because it relies on CREATE and not CREATE2.
This means that the deployment address only depends on the deployer address and its current nonce.
If the same deployer/nonce combination is used for any transaction other than deploying this contract, there's no chance to ever get the same contract at the same address on that chain.
What's worse, if users by accident send funds to the wrong ETH-flow contract address, the funds are likely to be permanently lost.
The approach of this PR is using a different deployment procedure. Instead of using the private key as a parameter of the script, as customary in most deployment scripts, it uses an obfuscated version of the private key.
The idea is that having a non-standard format for the private key prevents it from being used by accident for other deployments.
This is not intended to be a way to hide the key: the private key is still trivially derivable from the obfuscated version.
This PR:
There are also two tangential changes that I found helpful and decided to include as well:
vm.toStringandVm.Wallet.script/in linting.How to test
First, try to obfuscate a private key you control.
Then, use that obfuscated key to test a deployment on some testnet.
Let me know how to update the readme if something isn't clear.