design-doc: SuperchainERC20 Factory for L1 native tokens #50

0xParticle · 2024-07-19T16:24:50Z

Description
This document presents the requirements for a factory that deploys SuperchainERC20 tokens for tokens native to L1.

Co-authored-by: ng <ng@defi.sucks>

tynes · 2024-07-22T16:28:25Z

Are you sure that we need to deploy the L2 tokens via deposits from L1? That will increase the cost of creating these tokens. I believe we can get around this if the salt commits to the L1 token address + the erc20 properties fetched from the OptimismMintableERC20 token

0xParticle · 2024-07-22T16:58:08Z

Are you sure that we need to deploy the L2 tokens via deposits from L1? That will increase the cost of creating these tokens. I believe we can get around this if the salt commits to the L1 token address + the erc20 properties fetched from the OptimismMintableERC20 token

After discussing this point, we will update the design doc to remove the need for L1 deployments. We will relax the metadata enforced condition and make it based on social consensous.

smartcontracts

I added some comments but then realized that the most fundamental question that must be answered is:

Do we really care about the 1:1 mapping between L1 tokens and SuperchainERC20 tokens? I'm skeptical that this is necessary and it seems to limit this design document quite a bit. In fact, this property does not seem to be documented as a design requirement in the problem statement but this property then dominates the design as a whole. Would be great to get a clear answer here which will help direct future review.

smartcontracts · 2024-07-23T11:23:09Z

protocol/superchainerc20/L1-native-factory.md

+
+**Creation method**
+
+The `SuperchainERC20` addresses should be independent of the implementations. `CREATE` was discarded due to nonce dependability, and `CREATE2` was discarded because of its dependence on the creation code. For these reasons, the factory should use `CREATE3`.


A link to the CREATE3 spec would be very helpful here.

protocol/superchainerc20/L1-native-factory.md

Joxess · 2024-07-29T19:48:13Z

protocol/superchainerc20/L1-native-factory.md


-  event SuperchainERC20Deployed(address indexed superchainERC20, address indexed remoteToken, string name, string symbol, uint8 decimals);
+  event OptimismSuperchainERC20Deployed(address indexed superchainERC20, address indexed remoteToken, string name, string symbol, uint8 decimals);


We’re looking for ways to establish common standards regarding deployments. For SuperchainERC20 backed by an L2 native token, we’re considering using chainId as an additional input. Should we do the same for L1 bridged tokens (e.g., Ethereum chainId as 1)?

The same reasoning can be applied to parameters stored for each representation, based on having a remoteToken, chainId, and bridge.

I think we should focus specifically on L1 native tokens for this factory as they are a majority of the liquidity and then return to the problem of making a more generic factory in the future

Follow up context: https://www.notion.so/defi-wonderland/Extending-SuperchainERC20-to-L2-native-tokens-and-custom-e91ca08dd3eb4434834bf74f56237abf

protocol/superchainerc20/L1-native-factory.md

tynes · 2024-07-31T17:31:20Z

protocol/superchainerc20/L1-native-factory.md

+
+**Beacon Contract**
+
+The Beacon Contract holds the implementation address for the BeaconProxies. It will be a predeploy (consistent with the current architecture of the OP stack).


It could be good to detail that the beacon contract has to implement a particular interface defined in erc 1976

https://eips.ethereum.org/EIPS/eip-1967

function implementation() returns (address)

This means that the predeploy would implement this interface and then return an address of the actual implementation. The address of the actual implementation would have to be determined similarly to the upgrade transactions for hardforks.

It could be good to talk about upgradability, so we would likely want the address returned by implementation() to be an immutable and then we can have a hard fork that upgrades the predeploy implementation to include a different immutable

I notice that you cover some of this later in the doc :)

Added links to the EIP and references to the implementation address computation.
Is there a reason to make the implementation address an immutable instead of a constant?

protocol/superchainerc20/L1-native-factory.md

tynes · 2024-08-02T00:06:54Z

This looks ready for a design review meeting, the design at this point generally looks good to me

tynes · 2024-08-05T21:08:37Z

@mds1 Would love to have your eyes on this if you have time

tynes · 2024-08-07T16:56:49Z

Implementation PR for the token itself: ethereum-optimism/optimism#11256

mds1 · 2024-08-07T17:48:20Z

protocol/superchainerc20/L1-native-factory.md

+
+The `SuperchainERC20Factory` will be a proxied predeploy. It will deploy proxied `OptimismSuperchainERC20` tokens using the Beacon pattern. This structure allows upgrading all implementations at once if needed.
+
+Even though this document is specific for L1 native tokens, it should serve as a basis for other cases.


nit, can we rename the file to something like SuperchainERC20-factory.md? The filename sounds like the factory lives on L1, which is not the case since it's a predeploy

My only concern is that this factory is specific for tokens that are native to L1. We can still name it like that and change notation for the others (l2 native, etc)

mds1 · 2024-08-07T17:55:10Z

protocol/superchainerc20/L1-native-factory.md

+
+**Creation method**
+
+The `OptimismSuperchainERC20` addresses should be independent of the implementations. `CREATE` was discarded due to nonce dependability, and `CREATE2` was discarded because of its dependence on the creation code. For these reasons, the factory should use `CREATE3`.


We should consider calling out to CreateX, as it supports CREATE3, and provides some salt-protection mechanisms that may be useful. My suggestion would be to add CreateX as another preinstall, since then we avoid having to reimplement CREATE3 logic in this factory

Disclaimer: I helped write CreateX

I like the idea of having a CreateX preinstall. We where discussing having the CREATE3 one. Let's discuss this in the review meeting

mds1 · 2024-08-07T17:56:05Z

protocol/superchainerc20/L1-native-factory.md

+
+The `OptimismSuperchainERC20` addresses should be independent of the implementations. `CREATE` was discarded due to nonce dependability, and `CREATE2` was discarded because of its dependence on the creation code. For these reasons, the factory should use `CREATE3`.
+
+The salt will be composed of the L1 address and the token metadata. This implies that the same L1 token can have multiple SuperchainERC20 representation, as long as the metadata also changes.


What is the rationale here? Why not just use the L1 address as the salt?

Doing so would imply having a single OptimismSuperchainERC20 per L1 token. As the deployment is permissionless, anyone could "frontrun" and deploy the token with a poor metadata choice. There would be no way of changing this later on.
We initially thought about using just L1 address as the salt but initializing the first deployment from L1 to bring the correct metadata in, but we ended up following the structure the OptimismMintableERC20Factory currently use. Any metadata will be allowed, but social consensous will pick the winner.

mds1 · 2024-08-07T18:00:31Z

protocol/superchainerc20/L1-native-factory.md

+In an implementation update, the upgraded chains must perform a hardfork to change the constant value in the Beacon Contract representing the implementation address. 
+The address of the implementation will be determined similarly to [other upgrade transactions](https://github.com/ethereum-optimism/specs/blob/1f8ace7f21e44ff028951965ab552c81b892f199/specs/fjord/derivation.md#gaspriceoracle-deployment) .


What is the rationale for this? One consideration is incident response: if a buggy implementation is deployed, it is much harder to coordinate a hard fork than to send L1 deposit transactions that change a storage slot on each chain

Yes, I think the correct word here would be "Protocol Upgrade", which could have a hardfork as a particular case. I will change the sentence 👍

mds1 · 2024-08-07T18:02:20Z

protocol/superchainerc20/L1-native-factory.md

+
+### Factory Upgradability
+
+The factory will be a simple Proxy contract pointing to the current implementation. In case of an upgrade to the factory, the upgrading chains will need to deploy the new implementation and perform a hardfork to update the implementation address.


Same comment regarding incident response. I'm ok with hardfork being the default happy case path, but we should consider also supporting upgrades via deposit transaction to support quicker changes when required. The way this would work is similar to the current setup: The L2ProxyAdmin Owner is set to the aliased address of the L1ProxyAdmin owner. This way the L1ProxyAdmin owner can send a deposit transaction to upgrade predeploys.

Agree, will make this more generic

mds1 · 2024-08-07T18:05:20Z

protocol/superchainerc20/L1-native-factory.md

+### Deployment history
+
+The factory will store a `deployments` mapping that the `L2StandardBridge` will check for access control when converting between the SuperchainERC20 and their corresponding legacy representations (see the [liquidity migration specs](https://github.com/ethereum-optimism/specs/pull/294) for more details).
+
+```solidity
+mapping(address superc20 => address remoteToken) public deployments;
+```
+
+that maps each deployed `OptimismSuperchainERC20` address to the corresponding L1 address (remote address).


I need a refresher on the liquidity migration spec so I'm not totally following this section. Why does the L2StandardBridge need a mapping from an L2's SuperchainERC20 address to the RemoteToken address? Is an alternate design for the SuperchainERC20 contract itself expose a getter of it's L1 token address?

Would be great if we can expand this section and clarify it. In general, diagrams showing the various flows and conversions would be useful (edit: I do see a nice deployment diagram below, it doesn't include the L2StandardBridge though)

I will add a short refresher here. The TLDR is that the L2StandardBridge needs a way to check a certain superchain token address is valid, and the best way to do so is asking to the factory if that address was ever deployed. That check could also be done with a mapping from address to bool, but having the remoteToken here is very useful for the L2StandardBridge as well, as it needs to verify both tokens being converted correspond to the same remoteToken

mds1 · 2024-08-07T18:08:46Z

protocol/superchainerc20/L1-native-factory.md

+
+The Token should follow a simple BeaconProxy implementation, like Openzeppelin:
+
+https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/proxy/beacon/BeaconProxy.sol


nit, let's use a permalink to to a specific OZ version so this link doesn't break in the future if things get moved around

mds1 · 2024-08-07T18:09:59Z

protocol/superchainerc20/L1-native-factory.md

+- An `initialize` function that takes `(address _remoteToken, string _name, string _symbol, uint8 decimals)` and stores these in the storage of the BeaconProxy.
+- A function to reinitialize after proxy deployments or upgrades.
+
+## User Stories


It sounds like we also need a section here explaining the interactions with the L2StandardBridge

mds1 · 2024-08-07T18:11:58Z

protocol/superchainerc20/L1-native-factory.md

+2. The factory stores the `_remoteToken` address for each deployed SuperchainERC20 address.
+
+```mermaid
+sequenceDiagram


Since we're deploying an OptimismSuperchainERC20 BeaconProxy, should this diagram also have an OptimismSuperchainERC20BeaconProxy.initialize() call? Or is there nothing that needs to be initialized?

design-doc: L1 native Factory

1f40f44

Co-authored-by: ng <ng@defi.sucks>

0xParticle changed the title ~~design-doc: L1 native Factory~~ design-doc: SuperchainERC20 Factory for L1 native tokens Jul 19, 2024

0xng added 3 commits July 19, 2024 16:10

polish: polish factory code

d9c30a2

fix: using spaces over tabs

6f397c3

fix: rename variable and event params order

831fb3f

smartcontracts reviewed Jul 23, 2024

View reviewed changes

This was referenced Jul 25, 2024

SuperchainERC20: Factory Implementation ethereum-optimism/optimism#10872

Closed

SuperchainERC20: Factory Spec ethereum-optimism/optimism#10873

Closed

0xParticle added 2 commits July 29, 2024 15:03

feat: removed first time l1 deployment requirement

7db5a5d

feat: minor fixes

afed89b

Joxess reviewed Jul 29, 2024

View reviewed changes

feat: implementation details

58f554f

tynes reviewed Jul 31, 2024

View reviewed changes

protocol/superchainerc20/L1-native-factory.md Outdated Show resolved Hide resolved

tynes reviewed Jul 31, 2024

View reviewed changes

protocol/superchainerc20/L1-native-factory.md Outdated Show resolved Hide resolved

tynes reviewed Jul 31, 2024

View reviewed changes

protocol/superchainerc20/L1-native-factory.md Outdated Show resolved Hide resolved

tynes reviewed Jul 31, 2024

View reviewed changes

protocol/superchainerc20/L1-native-factory.md Outdated Show resolved Hide resolved

tynes reviewed Jul 31, 2024

View reviewed changes

protocol/superchainerc20/L1-native-factory.md Outdated Show resolved Hide resolved

tynes reviewed Jul 31, 2024

View reviewed changes

protocol/superchainerc20/L1-native-factory.md Outdated Show resolved Hide resolved

tynes reviewed Jul 31, 2024

View reviewed changes

protocol/superchainerc20/L1-native-factory.md Outdated Show resolved Hide resolved

This was referenced Jul 31, 2024

feat: introduce OptimismSuperchainERC20 ethereum-optimism/optimism#11256

Merged

feat: adding limited space mirror factory #37

Closed

feat: feedback addressed

33c0f9a

tynes mentioned this pull request Aug 5, 2024

Design Review: 8/7/2024 #56

Closed

mds1 reviewed Aug 7, 2024

View reviewed changes

jakim929 mentioned this pull request Aug 8, 2024

overview of interop ERC20 support ethereum-optimism/supersim#88

Closed

feat: changed create3 for createX and a convert diagram

530c606

tynes approved these changes Aug 9, 2024

View reviewed changes

tynes merged commit 5ef09f0 into main Aug 9, 2024

tynes deleted the feat/l1NativeFactory branch August 9, 2024 23:10

0xParticle mentioned this pull request Aug 13, 2024

interop: add OptimismSuperchainERC20Factory to predeploys ethereum-optimism/specs#332

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

design-doc: SuperchainERC20 Factory for L1 native tokens #50

design-doc: SuperchainERC20 Factory for L1 native tokens #50

0xParticle commented Jul 19, 2024

tynes commented Jul 22, 2024

0xParticle commented Jul 22, 2024

smartcontracts left a comment

smartcontracts Jul 23, 2024

Joxess Jul 29, 2024

tynes Jul 31, 2024

tynes Aug 7, 2024

tynes Jul 31, 2024

tynes Jul 31, 2024

0xParticle Aug 1, 2024

tynes commented Aug 2, 2024

tynes commented Aug 5, 2024

tynes commented Aug 7, 2024

mds1 Aug 7, 2024

0xParticle Aug 7, 2024

mds1 Aug 7, 2024

0xParticle Aug 7, 2024

mds1 Aug 7, 2024

0xParticle Aug 7, 2024

mds1 Aug 7, 2024

0xParticle Aug 7, 2024

mds1 Aug 7, 2024

0xParticle Aug 7, 2024

mds1 Aug 7, 2024

0xParticle Aug 7, 2024

mds1 Aug 7, 2024

mds1 Aug 7, 2024

mds1 Aug 7, 2024

0xParticle Aug 8, 2024


		Creation method

		The `SuperchainERC20` addresses should be independent of the implementations. `CREATE` was discarded due to nonce dependability, and `CREATE2` was discarded because of its dependence on the creation code. For these reasons, the factory should use `CREATE3`.


		event SuperchainERC20Deployed(address indexed superchainERC20, address indexed remoteToken, string name, string symbol, uint8 decimals);
		event OptimismSuperchainERC20Deployed(address indexed superchainERC20, address indexed remoteToken, string name, string symbol, uint8 decimals);


		Beacon Contract

		The Beacon Contract holds the implementation address for the BeaconProxies. It will be a predeploy (consistent with the current architecture of the OP stack).


		The `SuperchainERC20Factory` will be a proxied predeploy. It will deploy proxied `OptimismSuperchainERC20` tokens using the Beacon pattern. This structure allows upgrading all implementations at once if needed.

		Even though this document is specific for L1 native tokens, it should serve as a basis for other cases.


		Creation method

		The `OptimismSuperchainERC20` addresses should be independent of the implementations. `CREATE` was discarded due to nonce dependability, and `CREATE2` was discarded because of its dependence on the creation code. For these reasons, the factory should use `CREATE3`.


		The `OptimismSuperchainERC20` addresses should be independent of the implementations. `CREATE` was discarded due to nonce dependability, and `CREATE2` was discarded because of its dependence on the creation code. For these reasons, the factory should use `CREATE3`.

		The salt will be composed of the L1 address and the token metadata. This implies that the same L1 token can have multiple SuperchainERC20 representation, as long as the metadata also changes.

		In an implementation update, the upgraded chains must perform a hardfork to change the constant value in the Beacon Contract representing the implementation address.
		The address of the implementation will be determined similarly to [other upgrade transactions](https://github.com/ethereum-optimism/specs/blob/1f8ace7f21e44ff028951965ab552c81b892f199/specs/fjord/derivation.md#gaspriceoracle-deployment) .


		### Factory Upgradability

		The factory will be a simple Proxy contract pointing to the current implementation. In case of an upgrade to the factory, the upgrading chains will need to deploy the new implementation and perform a hardfork to update the implementation address.


		The Token should follow a simple BeaconProxy implementation, like Openzeppelin:

		https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/proxy/beacon/BeaconProxy.sol

design-doc: SuperchainERC20 Factory for L1 native tokens #50

design-doc: SuperchainERC20 Factory for L1 native tokens #50

Conversation

0xParticle commented Jul 19, 2024

tynes commented Jul 22, 2024

0xParticle commented Jul 22, 2024

smartcontracts left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tynes commented Aug 2, 2024

tynes commented Aug 5, 2024

tynes commented Aug 7, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment