-
Notifications
You must be signed in to change notification settings - Fork 5
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
feat: deploy Rollup contracts #76
Conversation
6900e3e
to
afbbde1
Compare
do we need cd for this? |
2505a57
to
c0eeace
Compare
vm.startBroadcast(); | ||
|
||
// deploy system contracts | ||
rollupPassage = address(new RollupPassage{salt: "zenith.rollupPassage"}(PERMIT2)); |
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.
do we want to do salt-mining to find low addresses?
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.
hmm. well, we don't technically have to. we could handpick better addrs than we could reasonably mine by simply directly editing the allocs
file. though, that would mean generating the allocs would be less reproducible.
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.
yeah, we'd also have to do some find-and-replace shenangians to update immutables and storage vars. which would be annoying. but it would be awful nice to have them have short addresses 🤔
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.
depends on how dearly we wish for the script to produce the exact final allocs
if we want efficient addrs for all contracts (&& fully deterministic script -> allocs) we might want to consider forking the USDC deploy script and making them CREATE2 deploys, because rn they're just CREATE and can't be easily mined
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.
yeah, we'd also have to do some find-and-replace shenangians to update immutables and storage vars. which would be annoying. but it would be awful nice to have them have short addresses 🤔
well, actually, out of the following contracts
- permit2
- RollupPassage
- RollupOrders
- WBTC
- USDT
- SafeProxyFactory
- SafeL2
- CompatibilityFallbackHandler
- the USDC admin Safe instance
- WETH9
we could handpick the addrs without find-and-replacing anything in code/storage:
permit2
is stored as an immutable in RollupPassage and RollupOrdersSafeL2
andCompatibilityFallbackHandler
are stored in the storage slots of the USDC admin Safe instance- USDC admin Safe instance is stored in various storage slots in the USDC protocol
BUT, for all three of these cases, the addrs that are stored in storage/immutables are just params configured in the deploys of the subsequent contracts. we could just handpick the addrs and reconfigure the deploys to point to those addrs and the immutables/storage slots would be produced correctly by the deploy scripts.
The only ones that would be more complex to fiddle with (and therefore something I'm reticent to do) is:
- the entire USDC protocol
which has a lot of internal dependencies on itself that is stored in immutables/storage, so we'd have to get into the guts to mess with the addresses
script/ZenithL2.s.sol
Outdated
// create2 address for Permit2 | ||
address constant PERMIT2 = 0x000000000022D473030F116dDEE9F6B43aC78BA3; | ||
// address of Minter with permission to mint ERC20 tokens | ||
address constant MINTER = 0x9999999999999999999999999999999999999999; |
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 you document this further? not 100% sure what's going on. is this a canary that is used later to do a find/replace on the minter address?
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.
is this a canary that is used later to do a find/replace on the minter address?
correct. the intention was actually to replace the real minter address here once it's chosen (rather than find/replace in the alloc json)
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.
similar for OWNER_ONE
and OWNER_TWO
, which really should be updated in code because the storage slots that Gnosis stores owners at depend on the list of owners themselves
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.
kind-of a nit, but this canary value is ambiguous if preceded by a 9 nibble. can we use something like 0x9988...
(only ambiguous if preceded by 0x9988
)
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.
how bout this (improved docs too) f5e85bd
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.
my understanding is that the DeployGnosisSafe.s.sol
and ZenithL2.s.sol
are not intended to be run via foundry targeting a live RPC. is that right? can we move them to a separate folder with a small readme or some other note?
hmm. well, their purpose is to be run in order to generate the they could be moved to script/predeploy/ or some such (and add a README) if that feels clearer |
1cd2987
to
f60638a
Compare
f60638a
to
f5e85bd
Compare
System
Safe
The Safe repo lacks Forge scripts to deploy their setup to a new chain, so we have implemented them here.
Other External
For the following pre-deployed contracts, we will be using the Forge deploy scripts written by the teams.