Skip to content
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 90 additions & 0 deletions protocol/superc20/mirror-factory-limited.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
# Summary

*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.
Copy link
Contributor

Choose a reason for hiding this comment

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

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)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perfect, agreed. Will add it.


# Problem Statement + Context

A vast amount of `OptimismMintableERC20Tokens` currently exist in the ecosystem. These tokens do not have interoperable properties. Therefore, they require a migration to become interoperable. The migration will be performed through Mirrors, as specified by [Mirror Standard](https://github.com/ethereum-optimism/design-docs/pull/36). These Mirrors must be easily deployed through a predefined factory to ensure all Mirrors share the same functionality and address across chains and also to verify their legitimacy.

The factory must facilitate the creation of a single legitimate Mirror per `OptimistismMintableERC20Token` to avoid liquidity fragmentation and unnecessary complexity.

# Alternatives Considered
- Using `CREATE`. Discarded due to nonce dependability.
- Using `CREATE3`. Viable, yet not preferred due to increased complexity.

# Proposed Solution

Limiting the scope to `OptimismMintableERC20Tokens` allows using `CREATE2` instead of `CREATE3`, which is more expensive and complex. This is because the variables CREATE2 uses to define the address of the deployed contract provides all the properties we need to maintain the same address accross chains and easily verify whether a mirror was deployed by the official factory or not:

- `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.
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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


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.
Copy link
Contributor

Choose a reason for hiding this comment

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

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


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.
Copy link
Contributor

Choose a reason for hiding this comment

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

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


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

Choose a reason for hiding this comment

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

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


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`:
Copy link
Contributor

Choose a reason for hiding this comment

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

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


Copy link
Contributor

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

```solidity
function mirrorMint(address _token, uint256 _amount) external {
address _mirror = MIRROR_FACTORY.getMirrorAddress(_token);
if (_mirror != msg.sender) revert();
///...mint logic
}

```

Example code:

Copy link
Contributor

Choose a reason for hiding this comment

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

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

```solidity
contract MirrorFactory {
string public constant MIRROR_VERSION = "1.0.0";
Mirror public constant MIRROR_IMPL = <PLACEHOLDER_ADDRESS>;

function deployMirror(address _token) external returns (address) {
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));
Copy link
Contributor

Choose a reason for hiding this comment

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

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


// Note: This function does not exist on L2ChugSplashProxy.
_mirror.setImplementation(MIRROR_IMPL);
}

function upgradeMirror(address _mirror) external {
Copy link
Contributor

@tynes tynes Jun 24, 2024

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor

@tynes tynes Jun 25, 2024

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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

// Note: This function does not exist on L2ChugSplashProxy.
_mirror.setImplementation(MIRROR_IMPL);
}

function _isOptimismMintableERC20(address _token) internal view returns (bool) {
return ERC165Checker.supportsInterface(_token, type(ILegacyMintableERC20).interfaceId)
|| ERC165Checker.supportsInterface(_token, type(IOptimismMintableERC20).interfaceId);
}

function getMirrorAddress(address _token) external view returns (address _mirror) {
Copy link
Contributor

Choose a reason for hiding this comment

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

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

_mirror = address(uint160(uint(keccak256(abi.encodePacked(
bytes1(0xff),
address(this),
keccak256(abi.encode(_token)),
keccak256(abi.encodePacked(
type(L2ChugSplashProxy).creationCode,
abi.encode(_token)
))
)))))
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

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


```

# 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.
Copy link
Contributor

Choose a reason for hiding this comment

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

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?


If the design space were to be extended `Create3` and extra predeploys would be needed.

Current `OptimismMintableERC20Tokens` deployed by OP and Ethereum mainnet factories have different versions and, therefore, different bytecodes. This should not be a problem as long as these tokens grant `burn` and `mint` permissions to the `StandardBridge` and conform to the `ERC20` interface.