-
Notifications
You must be signed in to change notification settings - Fork 140
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
Add UpgradeExecutor to token bridge creation flow #38
Conversation
// transfer ownership to rollup's owner | ||
ProxyAdmin(proxyAdmin).transferOwnership(rollupOwner); | ||
// deploy multicall | ||
Create2.deploy(0, _getL2Salt(OrbitSalts.L2_MULTICALL), _creationCodeFor(l2Code.multicall)); |
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.
Should we use a proxy in case the multicall implementation changes? Or has it been stable for a long time?
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 good q, not sure if it is worth it. Arb multicall hasn't been changed last 2 year as far as I can see. @gzeoneth what do you think?
…design Rollup and rollup proxy admin are owned by L1 upgrade executor, on which the rollupOwner has EXECUTOR role. When creating token bridge for rollup, now rollupOwner has to be provided as well. L1 creator will check that rollupOwner has EXECUTOR role on the upgrade executor (which is fetched via .owner()). Aliased L1 upgrade executor and rollup owner are then made executors on L2 upgrade executor.
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.
LGTM
On L1 side of token bridge: use existing rollup's proxy admin (fetch it from inbox).
On L2 side of token bridge: create new proxy admin for L2 contracts. Also create new UpgradeExecutor. Executor is the owner of proxy admin. Rollup owner and aliased L1 executor have EXECUTOR role.
Note: 'Test token bridge deployment' test which targets nitro test node is failing because test node is rn not fully adapted to work with latest contract changes.