feat: Factory Deployment Script #318
Conversation
Zer0dot
force-pushed
the
zer0dot/deploy-factory
branch
from
43d5021 to
db63642
Compare
Zer0dot
force-pushed
the
zer0dot/deploy-factory
branch
from
e3fa152 to
065b98c
Compare
|
Note that the factory address prediction reads addresses from the env for dependencies, rather than using the predicted addresses in the same script, as those may differ from the predicted addresses. |
script/DeployFactory.s.sol
Outdated
| expectedFactoryAddr = vm.envOr("FACTORY", address(0)); | ||
| factorySalt = vm.envOr("FACTORY_SALT", uint256(0)); |
There was a problem hiding this comment.
I think the current var used in the .env.example file is ACCOUNT_FACTORY and ACCOUNT_FACTORY_SALT. I'm fine with using either that or the one here, but could you make it consistent across the locations?
There was a problem hiding this comment.
Will do! Good catch, in my infinite wisdom I didn't check .env.example :)
script/GetInitcodeHash.s.sol
Outdated
| } else { | ||
| console.log("Using user-defined EntryPoint at: %x", address(entryPoint)); | ||
| } | ||
| IEntryPoint entryPoint = _getEntryPoint(); //IEntryPoint(payable(vm.envOr("ENTRYPOINT", address(0)))); |
| } | ||
|
|
||
| function _getSemiModularAccountBytecodeImpl() internal view returns (SemiModularAccountBytecode) { | ||
| return SemiModularAccountBytecode(payable(vm.envOr("SEMI_MODULAR_ACCOUNT_BYTECODE_IMPL", address(0)))); |
There was a problem hiding this comment.
I think there's a different format already set up in .env.example without the _IMPL ending, could you make that consistent here too?
There was a problem hiding this comment.
Oops-- actually the one in .env.example has _IMPL, I'd say we're good to keep it, it ensures the developer knows we're looking for an implementation, not an actual account.
test/script/DeployFactory.s.t.sol
Outdated
| entryPoint = IEntryPoint(address(6)); | ||
| modularAccountImpl = ModularAccount(payable(address(1))); | ||
| semiModularAccountBytecodeImpl = SemiModularAccountBytecode(payable(address(2))); | ||
| singleSignerValidationModule = address(3); | ||
| webAuthnValidationModule = address(4); |
There was a problem hiding this comment.
Why not deploy real copies of them? The EP can use the deploy script in OptimizedTest.
There was a problem hiding this comment.
Honestly, could go either way, I just didn't really want to test anything related to deploying anything else-- but it's a small change, I'll address this!
test/script/DeployFactory.s.t.sol
Outdated
| semiModularAccountBytecodeImpl = SemiModularAccountBytecode(payable(address(2))); | ||
| singleSignerValidationModule = address(3); | ||
| webAuthnValidationModule = address(4); | ||
| factoryOwner = address(5); |
There was a problem hiding this comment.
nit: prefer makeAddr over incrementing numbers due to appearing as precompiles in the test traces.
Motivation
We need scripts to predict the factory address and to deploy it.
Solution
Implement the aforementioned scripts, as well as some minor refactors to reduce resulting code duplication.
TODO