Skip to content
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

improve(OP): More generic OP chain adapter deployment #714

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

pxrl
Copy link
Contributor

@pxrl pxrl commented Nov 1, 2024

This change permits an incremental step towards a totally generic chain adapter deployment for OP stack chains. It does this be restructuring the address mappings for OP stack chains, such that the L1 contract addresses can be resolved solely with the combination of HubPool and SpokPool chain IDs. It also updates the new OP_Adapter deployment script to support a runtime-defined SpokePool chain ID. This is clunkily supplied via the environment because hardhat deploy doesn't seem to permit arbitrary runtime arguments being supplied, but that aside, this should ease subsequent deployments.

@pxrl
Copy link
Contributor Author

pxrl commented Nov 1, 2024

nb. This only touches constants and deployment scripts, so there should be no audit sensitivity. Contract addresses are being moved around though, so it's important that they're correct. I've checked myself but it'd be good to have a 2nd set of eyes on it.

deploy/consts.ts Show resolved Hide resolved
deploy/consts.ts Show resolved Hide resolved
deploy/consts.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@bmzig bmzig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked also and the bridge addresses check out.

This change permits an incremental step towards a totally generic chain
adapter deployment for OP stack chains. It does this be restructuring
the address mappings for OP stack chains, such that the L1 contract
addresses can be resolved solely with the combination of HubPool and
SpokPool chain IDs. It also updates the new OP_Adapter deployment script
to support a runtime-defined SpokePool chain ID. This is clunkily
supplied via the environment because hardhat deploy doesn't seem to
permit arbitrary runtime arguments being supplied, but that aside, this
should ease subsequent deployments.
@pxrl pxrl changed the base branch from master to reinis-frp/mainnet-fork November 14, 2024 14:00
Base automatically changed from reinis-frp/mainnet-fork to master November 14, 2024 14:12
@pxrl pxrl requested a review from bmzig November 14, 2024 14:17
Copy link
Contributor

@bmzig bmzig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I'm guessing we are just keeping the deploy_[op network]_adapter scripts for the sake of bookkeeping at this point?

@pxrl
Copy link
Contributor Author

pxrl commented Nov 14, 2024

Looks good. I'm guessing we are just keeping the deploy_[op network]_adapter scripts for the sake of bookkeeping at this point?

Kinda, yeah. I do think we could additionally consolidate:

  • Optimism & Base (OP stack, native USDC via CCTP)
  • Zora & Mode (OP stack, bridged USDC)

Blast is a total snowflake of course, and Redstone now seems to have an upgradable USDC deployment here, so in theory we could remove the Redstone Adapter in favour of the OP Adapter (even irrespective of whether we support USDC there).

However...I probably don't expect many (or any) new chain deployments to fit the Optimism/Base pattern or the Zora/Mode pattern, so I think this optimisation for the OP Stack + upgradable USDC combination gives us the most benefit for the least effort.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants