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

[DEPLOY-295]: Refactors L2EP Contracts #11758

Closed
wants to merge 65 commits into from

Conversation

chris-de-leon-cll
Copy link
Contributor

@chris-de-leon-cll chris-de-leon-cll commented Jan 12, 2024

Background

L2EP currently requires 4 Solidity smart contracts to be developed and deployed on the target chain. These contracts are:

  • CrossDomainForwarder
  • CrossDomainGovernor
  • SequencerUptimeFeed
  • Validator

Each of these contracts share an almost identical implementation per chain. In fact, the code between Scroll and Optimism is nearly identical and by extension contains a lot of duplication. This current trajectory will be unsustainable to maintain as we expand L2EP to other chains.

Overview

This PR refactors the L2EP Solidity contracts such that:

  1. The amount of duplicate code between chains is reduced
  2. The amount of code needed to implement L2EP for a new chain is minimized
  3. All contracts/interfaces/helpers/etc. related to L2EP comply with the style guide

Methodology

Instead of each chain having to define separate Validator, Governor, Forwarder, and Sequencer contracts with nearly identical code, this PR adds an abstract class for each of them. These abstract classes contain the functionality that is shared/duplicated for each chain. The result is that all shared code now exists in a centralized location making it easier to reuse and debug while cutting down on the size of existing contracts. Each chain will still need its own Validator, Governor, etc. contracts, but unlike before, they can now inherit from their corresponding abstract contracts and override methods as needed to account for any chain-specific differences. The effect: code that is specific to a particular chain now lives in the folder belonging to that chain.

This PR maintains full backwards compatibility with the existing L2EP design. Additionally, these new changes do not introduce any extra deployment steps. However, gas consumption is not the same. Some methods have experienced an increase in gas usage whereas others have experienced a decline. The gas snapshot included in the PR can be used to view the differences.

Summary of Notable Changes

  • 5 new abstract contracts were introduced:
    • CrossDomainForwader.sol
    • CrossDomainGovernor.sol
    • SequencerUptimeFeed.sol
    • GasLimitValidator.sol (needed for Optimism and Scroll, but not Arbitrum)
    • Validator.sol
  • All L2EP contracts now comply with the style guide:
    • Each now uses a specific pragma (0.8.19)
    • All /**/ comments have been replaced with ///
    • Variables have been renamed to follow the prefixing style i_ / s_
    • Imports have been rearranged
    • And more!
  • The chain specific interfaces for the sequencer uptime feed have been removed
    • The abstract SequencerUptimeFeed contract can now be used in place of these

Additional Note(s)

  • This PR does not simplify the Foundry tests
    • I decided to keep this separate for now to keep this PR slim and experiment with a few different testing designs
    • There is a draft PR in progress for this here
    • There's potential to simplify the Scroll and Optimism tests greatly - Arbitrum on the other hand may not be able to benefit from the simplifications

…ses, and refactors test cases by using a L2EPTest helper
…erUptimeFeed test cases, adds ScrollCrossDomainForwarder skeleton
…o ScrollCrossDomainForwarder helper function
Copy link
Contributor

I see that you haven't updated any CHANGELOG files. Would it make sense to do so?

Copy link
Contributor

I see you updated files related to contracts. Please run pnpm changeset in the contracts directory to add a changeset.

@cl-sonarqube-production
Copy link

@chainchad
Copy link
Collaborator

Closing this as it appears to be stale. Feel free to re-open if it's still needed.

@chainchad chainchad closed this Nov 8, 2024
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.

4 participants