feat: adding limited space mirror factory #37

0xng · 2024-06-20T18:50:34Z

Description

This document describes a possible design for a new Factory predeploy capable of deploying Mirror contracts for OptimismMintableERC20Tokens.

Additional Context
The space for this design is limited to only OptimismMintableERC20Tokens.

tynes · 2024-06-24T18:31:47Z

protocol/superc20/mirror-factory-limited.md

+
+We can track versioning through numbers or the addresses of the implementations used.
+
+This design allows the `StandardBridge` or any contract that needs to check the legitimacy of a `Mirror` to call `MIRROR_FACTORY.getMirrorAddress()` and validate whether the calling user is a mirror that corresponds to a given `token`. As an illustrative example of a potential function of the `StandardBridge`:


We should be clear that this is only for the L2StandardBridge since the superchain erc20 standard is only for L2 to L2

tynes · 2024-06-24T18:35:26Z

protocol/superc20/mirror-factory-limited.md

+		require(_isOptimismMintableERC20(_token), "not OptimismMintableERC20Token");
+
+		// Note: There's no _token slot in this proxy
+		Proxy _mirror = address(new L2ChugSplashProxy{salt: keccak256(abi.encode(_token))}(address(this), _token));


Perhaps we could just use Proxy rather than chugsplash proxy. There is always a balance between pseudocode and details, in practice we don't want to use chugsplash here. We should consider a 1967 beacon for this

tynes · 2024-06-24T18:35:43Z

protocol/superc20/mirror-factory-limited.md

+```
+
+Example code:
+


small nit: using tabs here makes it a little harder to read

tynes · 2024-06-24T18:43:31Z

protocol/superc20/mirror-factory-limited.md

+		_mirror.setImplementation(MIRROR_IMPL);
+	}
+
+	function upgradeMirror(address _mirror) external {


We should remove this function and think about a 1967 beacon based design for simplicity.

We don't yet have a predeploy system for 1967 proxies, but thinking of making a predeploy that looks like this:

contract Mirror1967Beacon { function implementation() { return address(0x..); } }

The above contract would be a predeploy and it would hardcode in an address that it returns code of. As part of a hardfork, we deploy smart contracts. We would deploy the implementation and we would be able to know the address ahead of time, see here for previous work on how we can know the address ahead of time. We would be able to install the bytecode into the genesis for new chains

What this would allow us to do is to hardfork to update the Mirror1967Beacon predeploy and modify the hardcoded address that it returns

Perfect, this makes a lot of sense. Thanks for the suggestion, will update this.

Another thought, we could do something like this:

contract MirrorFactory { address implementation public; constructor() { implementation = new MirrorSuperchainERC20(); } function deploy() { proxy = new Proxy({ beacon: address(this) }); } }

Then upgrading the implementation would be updating the factory

Agreed with @tynes, my read of this design doc makes it seem like we want to use a beacon pattern here

tynes · 2024-06-24T18:53:52Z

protocol/superc20/mirror-factory-limited.md

+            || ERC165Checker.supportsInterface(_token, type(IOptimismMintableERC20).interfaceId);
+    }
+
+	function getMirrorAddress(address _token) external view returns (address _mirror) {


Can we use a mapping that is populated rather than computing the address on chain? Otherwise we will need to forever pin the specific version of solc

tynes · 2024-06-24T18:54:47Z

protocol/superc20/mirror-factory-limited.md

+We can track versioning through numbers or the addresses of the implementations used.
+
+This design allows the `StandardBridge` or any contract that needs to check the legitimacy of a `Mirror` to call `MIRROR_FACTORY.getMirrorAddress()` and validate whether the calling user is a mirror that corresponds to a given `token`. As an illustrative example of a potential function of the `StandardBridge`:
+


Can you describe exactly how this is secure? In terms of invariants. Also hypothetically if we modified the bytecode used for the create2 deployment, it should still be secure in theory but allow for fragmentation?

I can add invariants, but you raise a great point. If modifying the init code used for create2 is a possibility, many issues appear:

Theoretically--if there wasn't a check--one could deploy two mirrors for the same token, as the resulting address of the create2 would be different.

The getMirrorAddress function would break for tokens with the old init code.

If adding a chain later on that has different/upgraded init code, and we deploy a Mirror for a token deployed with the previous init code on another chain, the interoperable features would break as the address of these tokens would differ.

It seems to me that I need to rewrite this with the CREATE3 approach to remove the init code dependency and rely solely on the salt. I think this will be the most robust approach accompanying with the beacon and the populated mapping as you suggested.

mds1 · 2024-06-25T15:55:45Z

protocol/superc20/mirror-factory-limited.md

+
+*The work present below is part of the [Interoperability project](https://github.com/ethereum-optimism/optimism/issues/10899).*
+
+A factory to spin up `ISuperchainERC20` Mirrors of `OptimismMintableERC20Tokens` is required. This document aims to provide a design for such a factory. The Mirrors need to be proxied to address any future addition of features. The factory will deploy these proxies. Therefore, from now on, when we say `Mirror` we refer to the Proxy and not the implementation.


I would really appreciate an overall architecture/glossary section in one of these design docs, there are a lot of new terms and I want to make sure the overall design is unambiguous:

OptimismMintableERC20Factory is a proxy, with a corresponding implementation

OptimismMintableERC20Factory deploys OptimismMintableERC20 contracts, and the OptimismMintableERC20s are not proxied

MirrorERC20Factory is a proxy, with a corresponding implementation

It deploys MirrorERC20 tokens, which are proxied using the beacon pattern, so we can upgrade all implementations if needed

(Something like that, I don't know if that's exactly correct)

Perfect, agreed. Will add it.

mds1 · 2024-06-25T15:57:48Z

protocol/superc20/mirror-factory-limited.md

+
+- `Salt`: If we use the `token` address, we can link the `Mirror` to this `token`. This will allow other contracts to know whether a caller is a legitimate `Mirror` or not.
+- `Sender Address`: In this case, it will be the Factory address itself, or the address of the factory's proxy. This works well, as it the factory can be deployed at the same address accross different chains. For chains in the superchain, it will be a predeploy. This predeploy, however, won't have a pretty address. Instead, it has to be deployed at the address where the L1 Factory is deployed so the address is maintained for all chains.
+- `Init Code`: The same proxy's init code must be used accross chains.


If we change the solidity version, new chains deployed after that version change will have proxies with different initcode, losing the "same address" benefit?

Correct, I missed this. I have pushed an alternative approach with CREATE3 and the beacon approach here: #39

mds1 · 2024-06-25T16:00:10Z

protocol/superc20/mirror-factory-limited.md

+- `Sender Address`: In this case, it will be the Factory address itself, or the address of the factory's proxy. This works well, as it the factory can be deployed at the same address accross different chains. For chains in the superchain, it will be a predeploy. This predeploy, however, won't have a pretty address. Instead, it has to be deployed at the address where the L1 Factory is deployed so the address is maintained for all chains.
+- `Init Code`: The same proxy's init code must be used accross chains.
+
+The salt doesn't need to be more than the `OptimismMintableERC20Token` address. The `name` and `symbol`, and `decimals` can be fetched with the `token` address alone in the implementation. The factory will be the `Mirror`'s admin, and the proxy will have to either call `setStorage` immediately with the `token` at the time of deployment, or add a `TOKEN_SLOT` and update it on the constructor.


and the proxy will have to either call setStorage immediately with the token at the time of deployment, or add a TOKEN_SLOT and update it on the constructor.

I'm not following what this sentence means, I don't see setStorage referenced anywhere else in this document

mds1 · 2024-06-25T16:01:17Z

protocol/superc20/mirror-factory-limited.md

+
+The salt doesn't need to be more than the `OptimismMintableERC20Token` address. The `name` and `symbol`, and `decimals` can be fetched with the `token` address alone in the implementation. The factory will be the `Mirror`'s admin, and the proxy will have to either call `setStorage` immediately with the `token` at the time of deployment, or add a `TOKEN_SLOT` and update it on the constructor.
+
+The factory should be upgradable or have setters to allow the modification of the implementations for the current `Mirrors`. The deployment and update of already-deployed `Mirrors` to new implementations should be open to anyone.


This sounds like implementations can be changed permissionlessly so I think I may be misunderstanding something here

mds1 · 2024-06-25T16:02:32Z

protocol/superc20/mirror-factory-limited.md

+
+The factory should be upgradable or have setters to allow the modification of the implementations for the current `Mirrors`. The deployment and update of already-deployed `Mirrors` to new implementations should be open to anyone.
+
+We can track versioning through numbers or the addresses of the implementations used.


For contract versioning, wouldn't we just use the standard semver versioning scheme we use for other contracts? https://github.com/ethereum-optimism/optimism/blob/develop/packages/contracts-bedrock/VERSIONING.md

mds1 · 2024-06-25T16:05:42Z

protocol/superc20/mirror-factory-limited.md

+            ))
+        )))))
+	} 
+}


Similar to #36 (comment), we should focus less on specific code here and more on high level design and flows

mds1 · 2024-06-25T16:07:47Z

protocol/superc20/mirror-factory-limited.md

+
+# Risks & Uncertainties
+
+This only accounts for `OptimismMintableERC20Tokens`. The design space is limited for simplicity. Legacy tokens are not considered. New interoperable tokens using the `Superc20` standard without Mirrors, are not taken into account for this factory.


This is a user story I'd like to see fleshed out more, as the high level paths are unclear to me:

Token A has existing OptimismMintableERC20 contract -> describe the path to create a SuperERC20 version

Token B has no existing OptimismMintableERC20 contract -> describe the path to create a SuperERC20 version

Token B now has SuperERC20 version but no OptimismMintableERC20 version -> can someone create the OptimismMintableERC20 now? If yes, what happens? If no, what prevents it?

tynes · 2024-07-31T23:23:29Z

Closing this in favor of #50

0xng added 2 commits June 20, 2024 15:45

feat: adding limited space mirror factory

23d96d9

fix: adding bulb to highlight note

00a5d74

0xng mentioned this pull request Jun 20, 2024

design-doc: mirror standard #36

Closed

fix: adding missing argument types in example snippet

7c475d6

This was referenced Jun 20, 2024

Interop: Migrated Liquidity ethereum-optimism/optimism#10899

Closed

Design Review: 6/25/2024 #38

Closed

polish: polishing text and fixing code example

27031fa

tynes reviewed Jun 24, 2024

View reviewed changes

0xng mentioned this pull request Jun 25, 2024

feat: Adding Mirror Factory with Create3 #39

Closed

mds1 reviewed Jun 25, 2024

View reviewed changes

tynes closed this Jul 31, 2024

tynes deleted the feat/limited-mirror-factory branch July 31, 2024 23:23


		We can track versioning through numbers or the addresses of the implementations used.

		This design allows the `StandardBridge` or any contract that needs to check the legitimacy of a `Mirror` to call `MIRROR_FACTORY.getMirrorAddress()` and validate whether the calling user is a mirror that corresponds to a given `token`. As an illustrative example of a potential function of the `StandardBridge`:


		The work present below is part of the [Interoperability project](https://github.com/ethereum-optimism/optimism/issues/10899).

		A factory to spin up `ISuperchainERC20` Mirrors of `OptimismMintableERC20Tokens` is required. This document aims to provide a design for such a factory. The Mirrors need to be proxied to address any future addition of features. The factory will deploy these proxies. Therefore, from now on, when we say `Mirror` we refer to the Proxy and not the implementation.


		The salt doesn't need to be more than the `OptimismMintableERC20Token` address. The `name` and `symbol`, and `decimals` can be fetched with the `token` address alone in the implementation. The factory will be the `Mirror`'s admin, and the proxy will have to either call `setStorage` immediately with the `token` at the time of deployment, or add a `TOKEN_SLOT` and update it on the constructor.

		The factory should be upgradable or have setters to allow the modification of the implementations for the current `Mirrors`. The deployment and update of already-deployed `Mirrors` to new implementations should be open to anyone.


		# Risks & Uncertainties

		This only accounts for `OptimismMintableERC20Tokens`. The design space is limited for simplicity. Legacy tokens are not considered. New interoperable tokens using the `Superc20` standard without Mirrors, are not taken into account for this factory.

		```

		Example code:

feat: adding limited space mirror factory #37

feat: adding limited space mirror factory #37

Uh oh!

Conversation

0xng commented Jun 20, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tynes Jun 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tynes Jun 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tynes commented Jul 31, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

tynes Jun 24, 2024 •

edited

Loading

tynes Jun 25, 2024 •

edited

Loading