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: liquidity migration #10

Closed
wants to merge 14 commits into from
Closed
96 changes: 96 additions & 0 deletions packages/contracts-bedrock/src/L2/L2StandardBridgeInterop.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.15;

import { Predeploys } from "src/libraries/Predeploys.sol";
import { L2StandardBridge } from "./L2StandardBridge.sol";
import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import { IERC20Metadata } from "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol";

/// @notice Thrown when the decimals of the tokens are not the same.
error InvalidDecimals();

/// @notice Thrown when the legacy address is not found in the OptimismMintableERC20Factory.
error InvalidLegacyAddress();

/// @notice Thrown when the SuperchainERC20 address is not found in the SuperchainERC20Factory.
error InvalidSuperchainAddress();

/// @notice Thrown when the remote addresses of the tokens are not the same.
error InvalidTokenPair();

// TODO: Use OptimismMintableERC20Factory contract instead of interface
interface IOptimismMintableERC20Factory {
function deployments(address) external view returns (address);
}

Copy link

Choose a reason for hiding this comment

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

We could use an interface that is generic with just the deployments function. We currently do this sometimes, it reduces the number of files when verifying a contract and i think it will reduce compile time if we use interfaces everywhere rather than importing full implementations

// TODO: Move to a separate file
interface ISuperchainERC20Factory {
function deployments(address) external view returns (address);
}

// TODO: Use an existing interface with `mint` and `burn`?
interface MintableAndBurnable is IERC20 {
Copy link

@0xDiscotech 0xDiscotech Aug 13, 2024

Choose a reason for hiding this comment

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

WDYT about this?

Suggested change
interface MintableAndBurnable is IERC20 {
interface Mintable {

function mint(address, uint256) external;
function burn(address, uint256) external;
}

/// @custom:proxied
/// @custom:predeploy 0x4200000000000000000000000000000000000010
/// @title L2StandardBridgeInterop
/// @notice The L2StandardBridgeInterop is an extension of the L2StandardBridge that allows for
/// the conversion of tokens between legacy tokens (OptimismMintableERC20 or StandardL2ERC20)
/// and SuperchainERC20 tokens.
contract L2StandardBridgeInterop is L2StandardBridge {

Choose a reason for hiding this comment

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

Why are extending the standard bridge instead of updating it? did you have this conversation with mark?

Copy link
Member Author

Choose a reason for hiding this comment

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

/// @notice Emitted when a conversion is made.
/// @param from The token being converted from.
/// @param to The token being converted to.
/// @param caller The caller of the conversion.
/// @param amount The amount of tokens being converted.
event Converted(address indexed from, address indexed to, address indexed caller, uint256 amount);

/// @notice Converts `amount` of `from` token to `to` token.
/// @param _from The token being converted from.
/// @param _to The token being converted to.
/// @param _amount The amount of tokens being converted.
function convert(address _from, address _to, uint256 _amount) external {
_validatePair(_from, _to);

MintableAndBurnable(_from).burn(msg.sender, _amount);
MintableAndBurnable(_to).mint(msg.sender, _amount);

emit Converted(_from, _to, msg.sender, _amount);
}

/// @notice Validates the pair of tokens.
/// @param _from The token being converted from.
/// @param _to The token being converted to.
function _validatePair(address _from, address _to) internal view {
// 1. Decimals check
if (IERC20Metadata(_from).decimals() != IERC20Metadata(_to).decimals()) revert InvalidDecimals();

// Order tokens for factory validation
if (_isOptimismMintableERC20(_from)) {
_validateFactories(_from, _to);
} else {
_validateFactories(_to, _from);
}
}

/// @notice Validates that the tokens are deployed by the correct factory.
/// @param _legacyAddr The legacy token address (OptimismMintableERC20 or StandardL2ERC20).
/// @param _superAddr The SuperchainERC20 address.
function _validateFactories(address _legacyAddr, address _superAddr) internal view {
// 2. Valid legacy check
address _legacyRemoteToken =
IOptimismMintableERC20Factory(Predeploys.OPTIMISM_MINTABLE_ERC20_FACTORY).deployments(_legacyAddr);
if (_legacyRemoteToken == address(0)) revert InvalidLegacyAddress();

// 3. Valid SuperchainERC20 check
address _superRemoteToken =
ISuperchainERC20Factory(Predeploys.OPTIMISM_SUPERCHAIN_ERC20_FACTORY).deployments(_superAddr);
if (_superRemoteToken == address(0)) revert InvalidSuperchainAddress();

Choose a reason for hiding this comment

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

Not convinced by the error name. It's understandable in context, but for a first time reader it might be confusing. Maybe InvalidSuperchainERC20Address, even if it sounds ugly. Wdty? I know it also affects the Legacy error, so maybe nay

Choose a reason for hiding this comment

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

Is this check necessary? you are already checking that _legacy !=0 and that _legacy == _super

Choose a reason for hiding this comment

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

I think is just for the revert error. I'd leave it, doesn't harm since we'll be using an OP chain so gas is not an issue

Choose a reason for hiding this comment

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

Yeah, its also a very cheap check. Let's leave it for clarity :)


// 4. Same remote address check
if (_legacyRemoteToken != _superRemoteToken) revert InvalidTokenPair();
}
}
7 changes: 6 additions & 1 deletion packages/contracts-bedrock/src/libraries/Predeploys.sol
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,9 @@ library Predeploys {
/// @notice Address of the ETHLiquidity predeploy.
address internal constant ETH_LIQUIDITY = 0x4200000000000000000000000000000000000025;

/// @notice Address of the OptimismSuperchainERC20Factory predeploy.
address internal constant OPTIMISM_SUPERCHAIN_ERC20_FACTORY = 0x4200000000000000000000000000000000000026;
0xParticle marked this conversation as resolved.
Show resolved Hide resolved

/// @notice Returns the name of the predeploy at the given address.
function getName(address _addr) internal pure returns (string memory out_) {
require(isPredeployNamespace(_addr), "Predeploys: address must be a predeploy");
Expand Down Expand Up @@ -123,6 +126,7 @@ library Predeploys {
if (_addr == L2_TO_L2_CROSS_DOMAIN_MESSENGER) return "L2ToL2CrossDomainMessenger";
if (_addr == SUPERCHAIN_WETH) return "SuperchainWETH";
if (_addr == ETH_LIQUIDITY) return "ETHLiquidity";
if (_addr == OPTIMISM_SUPERCHAIN_ERC20_FACTORY) return "OptimismSuperchainERC20Factory";
revert("Predeploys: unnamed predeploy");
}

Expand All @@ -140,7 +144,8 @@ library Predeploys {
|| _addr == OPTIMISM_MINTABLE_ERC721_FACTORY || _addr == PROXY_ADMIN || _addr == BASE_FEE_VAULT
|| _addr == L1_FEE_VAULT || _addr == SCHEMA_REGISTRY || _addr == EAS || _addr == GOVERNANCE_TOKEN
|| (_useInterop && _addr == CROSS_L2_INBOX) || (_useInterop && _addr == L2_TO_L2_CROSS_DOMAIN_MESSENGER)
|| (_useInterop && _addr == SUPERCHAIN_WETH) || (_useInterop && _addr == ETH_LIQUIDITY);
|| (_useInterop && _addr == SUPERCHAIN_WETH) || (_useInterop && _addr == ETH_LIQUIDITY)
|| (_useInterop && _addr == OPTIMISM_SUPERCHAIN_ERC20_FACTORY);
}

function isPredeployNamespace(address _addr) internal pure returns (bool) {
Expand Down