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

Fully remove CGT code from L1 contracts (OPCM - Upgrade 13) #13671

Closed
Tracked by #13069
maurelian opened this issue Jan 9, 2025 · 2 comments · Fixed by #13686
Closed
Tracked by #13069

Fully remove CGT code from L1 contracts (OPCM - Upgrade 13) #13671

maurelian opened this issue Jan 9, 2025 · 2 comments · Fixed by #13686
Assignees

Comments

@maurelian
Copy link
Contributor

maurelian commented Jan 9, 2025

Description

Build on the work begun in #13589 to completely remove cgt code from L1 code paths. No further work should be done to L2 code paths.

The full set of ABI changes from CGT can be seen here: https://github.com/ethereum-optimism/optimism/pull/10143/files#diff-97745aae7f1af56cec3629873b886e53b94123dedbae6b3db26babb1ed50354b

We should remove all newly added functions (including getters) on L1 contracts. However we should minimize changes to the initialize() functions (meaning it is fine to keep the systemConfig address as a new input to the bridge contracts, and as a state var, but it should become internal).

Specifically:

  • In L1CrossDomainMessenger:
    • make the systemConfig variable internal
    • keep the _systemConfig argument
  • In L1StandardBridge:
    • make the systemConfig variable internal
    • keep the _systemConfig argument
  • In SystemConfig
    • remove gasPayingToken()
    • remove isCustomGasToken()
    • remove gasPayingTokenName()
    • remove gasPayingTokenSymbol()
    • remove _setGasPayingToken()
    • remove gasPayingToken from Addresses struct
  • In OptimismPortal:
    • remove balance()
    • remove depositERC20Transaction()
    • remove setGasPayingToken()

Note that the SystemConfig should be the only contract with changes to the initializer.

In the above contracts, unused branching code, errors and events should also be removed.

Where state vars are removed a spacer should be added.

@maurelian maurelian changed the title Fully remove CGT code Fully remove CGT code from L1 contracts Jan 9, 2025
@mds1
Copy link
Contributor

mds1 commented Jan 9, 2025

Where state vars are removed a spacer should be added.

@AmadiMichael In case you aren't familiar, info on our spacing naming convention here: https://github.com/ethereum-optimism/optimism/blob/develop/packages/contracts-bedrock/meta/STYLE_GUIDE.md#spacers

@zhiqiangxu
Copy link
Contributor

When will CGT be added back again?

@BlocksOnAChain BlocksOnAChain changed the title Fully remove CGT code from L1 contracts Fully remove CGT code from L1 contracts (Upgrade 12) Jan 10, 2025
@maurelian maurelian moved this from Todo to In Progress in OP Stack Upgrades Jan 10, 2025
@github-project-automation github-project-automation bot moved this from In Progress to Done in OP Stack Upgrades Jan 13, 2025
@maurelian maurelian changed the title Fully remove CGT code from L1 contracts (Upgrade 12) Fully remove CGT code from L1 contracts (Upgrade 13) Jan 30, 2025
@maurelian maurelian changed the title Fully remove CGT code from L1 contracts (Upgrade 13) Fully remove CGT code from L1 contracts (OPCM - Upgrade 13) Feb 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants