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

feat: introduce OptimismSuperchainERC20Factory #11617

Merged

Conversation

agusduha
Copy link
Contributor

@agusduha agusduha commented Aug 27, 2024

Description

Implement the factory for deploying OptimismSuperchainERC20 Beacon proxies.

Spec: ethereum-optimism/specs#332

Tests

Added OptimismSuperchainERC20Factory tests
Test the OptimismSuperchainERC20 Beacon in its tests

Additional context

Q: Should the OptimismSuperchainERC20Beacon predeploy be proxied?

TODO: Set the correct address of the OptimismSuperchainERC20 implementation in the OptimismSuperchainERC20Beacon contract.

Metadata

@agusduha
Copy link
Contributor Author

@tynes Do you have an usual way to generate an address in the L2Genesis script? It is the thing missing for the OptimismSuperchainERC20 implementation

@tynes
Copy link
Contributor

tynes commented Aug 29, 2024

@tynes Do you have an usual way to generate an address in the L2Genesis script? It is the thing missing for the OptimismSuperchainERC20 implementation

An example of how addresses are generated can be found here: https://github.com/ethereum-optimism/specs/blob/9342867e3fd6dcd47038200b21629d25cb7fe566/specs/protocol/fjord/derivation.md#network-upgrade-automation-transactions

The idea is that the hardfork includes special deposit transactions at the hardfork block and these deposit transactions can deploy contracts, initialize state, modify implementations in proxies. We will need to compute the address at which the implementation will be deployed at. There isn't really any special reason as to why a particular from is picked, usually a fresh one is used with nonce 0. There needs to be enough gas to do the execution.

I recommend just creating an entry in the predeploys library for an arbitrary address for now, and we can think about the hardfork later. For now we just care about new networks

@gotzenx gotzenx marked this pull request as ready for review September 2, 2024 18:38
@gotzenx gotzenx requested a review from a team as a code owner September 2, 2024 18:38
@gotzenx gotzenx requested a review from Inphi September 2, 2024 18:38
Copy link
Contributor

@tynes tynes left a comment

Choose a reason for hiding this comment

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

One small fix to fix CI but the rest of the PR looks good to me

Copy link

codecov bot commented Sep 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.78%. Comparing base (d887cfa) to head (3e23981).
Report is 2 commits behind head on develop.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop   #11617   +/-   ##
========================================
  Coverage    78.78%   78.78%           
========================================
  Files           41       41           
  Lines         3262     3262           
========================================
  Hits          2570     2570           
  Misses         529      529           
  Partials       163      163           
Flag Coverage Δ
cannon-go-tests 78.78% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@tynes
Copy link
Contributor

tynes commented Sep 12, 2024

It looks like this may need to be rebased one more time to get the latest CI to run with it, some new required CI was added since this was last committed to. The code looks good to me, great work!

agusduha and others added 4 commits September 12, 2024 08:44
* test: add L2 standard bridge interop unit tests

* fix: add tests natspec

* fix: unit tests fixes

* fix: super to legacy tests failing

* fix: mock and expect mint and burn
* test: add L2 standard bridge interop unit tests

* fix: add tests natspec

* fix: add generic factory interface
* test: add L2 standard bridge interop unit tests

* fix: add tests natspec

* fix: add generic factory interface

* feat: modify OptimismMintableERC20Factory for convert

* fix: use only a public function for create3

* feat: rollback interop factory, modify legacy one

* fix: delete local token return variable
@0xng 0xng force-pushed the sc/superchain-erc20-factory branch from 1a46cd7 to 3e23981 Compare September 12, 2024 11:57
@tynes tynes added this pull request to the merge queue Sep 12, 2024
Merged via the queue into ethereum-optimism:develop with commit 232c54d Sep 12, 2024
59 checks passed
@skeletor-spaceman skeletor-spaceman deleted the sc/superchain-erc20-factory branch September 12, 2024 23:35
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.

L2StandardBridgeInterop tiny TODO Removal SuperchainERC20: Factory Implementation
5 participants