-
Notifications
You must be signed in to change notification settings - Fork 25
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
chore: [v0.8-develop] alpha.0 deploy prep #111
Conversation
4de9f7b
to
9a02710
Compare
5911793
to
f643f61
Compare
9a02710
to
0a484ef
Compare
f643f61
to
6e5293e
Compare
6e5293e
to
f94a910
Compare
import {UpgradeableModularAccount} from "../src/account/UpgradeableModularAccount.sol"; | ||
import {SingleSignerValidation} from "../src/modules/validation/SingleSignerValidation.sol"; | ||
|
||
contract DeployScript is 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.
Can we update readme for this?
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.
Maybe add .env.example too.
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.
Added both a .env.example
with comments, and a section in the README for running the script.
address addr = Create2.computeAddress( | ||
salt, keccak256(abi.encodePacked(type(UpgradeableModularAccount).creationCode, abi.encode(entryPoint))) | ||
); | ||
if (addr != expected) { |
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.
Not sure if this is necessary in RI. It is gonna make test deployment annoying as you will need to compute all the addresses first and put them in env (account impl, validation, etc.)
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.
It's just to prevent accidental drift between when it's calculated vs deployed, which can happen if using the wrong build profile, for example. For the first time pass, we can just run the script, and the error in console.log
will tell us what address to put in.
Motivation
We want to deploy a non-final build of the RI to testnets.
Solution
Update compiler settings for deterministic builds
Bump up the optimization count while remaining under the 24kb size limit
Add a deploy script and tests for the deploy script.
Change the factory initialization to take in an owner address in the constructor, rather than using
msg.sender
.