Commit b8c10302 authored by Mark Tyneway's avatar Mark Tyneway

contracts-bedrock: fix standard bridge token pair checking

This ensures that the legacy interface is only used for
legacy tokens and that the modern interface can be used
on the OptimismMintableERC20 token.

Note that this is not the gas efficient way to implement this,
but its ok for now as we are focused on minimal diff changes.

In the future we could refactor this code to ensure that
the ERC165Checker calls a minimal amount of times.
parent b0f4c978
......@@ -174,16 +174,16 @@ L2OutputOracleUpgradeable_Test:test_initValuesOnProxy_succeeds() (gas: 26208)
L2OutputOracleUpgradeable_Test:test_initializeImpl_alreadyInitialized_reverts() (gas: 15149)
L2OutputOracleUpgradeable_Test:test_initializeProxy_alreadyInitialized_reverts() (gas: 20175)
L2OutputOracleUpgradeable_Test:test_upgrading_succeeds() (gas: 180481)
L2StandardBridge_BridgeERC20To_Test:test_bridgeERC20To_succeeds() (gas: 387833)
L2StandardBridge_BridgeERC20To_Test:test_withdrawTo_withdrawingERC20_succeeds() (gas: 388064)
L2StandardBridge_BridgeERC20_Test:test_bridgeERC20_succeeds() (gas: 383536)
L2StandardBridge_BridgeERC20To_Test:test_bridgeERC20To_succeeds() (gas: 390084)
L2StandardBridge_BridgeERC20To_Test:test_withdrawTo_withdrawingERC20_succeeds() (gas: 390315)
L2StandardBridge_BridgeERC20_Test:test_bridgeERC20_succeeds() (gas: 385788)
L2StandardBridge_BridgeERC20_Test:test_withdraw_notEOA_reverts() (gas: 251758)
L2StandardBridge_BridgeERC20_Test:test_withdraw_withdrawingERC20_succeeds() (gas: 383722)
L2StandardBridge_BridgeERC20_Test:test_withdraw_withdrawingERC20_succeeds() (gas: 385973)
L2StandardBridge_Bridge_Test:test_finalizeBridgeETH_incorrectValue_reverts() (gas: 23843)
L2StandardBridge_Bridge_Test:test_finalizeBridgeETH_sendToMessenger_reverts() (gas: 23982)
L2StandardBridge_Bridge_Test:test_finalizeBridgeETH_sendToSelf_reverts() (gas: 23870)
L2StandardBridge_Bridge_Test:test_finalizeDeposit_depositingERC20_succeeds() (gas: 91013)
L2StandardBridge_Bridge_Test:test_finalizeDeposit_depositingETH_succeeds() (gas: 89889)
L2StandardBridge_Bridge_Test:test_finalizeDeposit_depositingERC20_succeeds() (gas: 93824)
L2StandardBridge_Bridge_Test:test_finalizeDeposit_depositingETH_succeeds() (gas: 92700)
L2StandardBridge_FinalizeBridgeETH_Test:test_finalizeBridgeETH_succeeds() (gas: 43155)
L2StandardBridge_Test:test_initialize_succeeds() (gas: 24292)
L2StandardBridge_Test:test_receive_succeeds() (gas: 174011)
......@@ -412,6 +412,8 @@ SequencerFeeVault_Test:test_minWithdrawalAmount_succeeds() (gas: 5442)
SequencerFeeVault_Test:test_receive_succeeds() (gas: 17373)
SequencerFeeVault_Test:test_withdraw_notEnough_reverts() (gas: 9331)
SequencerFeeVault_Test:test_withdraw_succeeds() (gas: 163228)
StandardBridge_Stateless_Test:test_isCorrectTokenPair_succeeds() (gas: 49936)
StandardBridge_Stateless_Test:test_isOptimismMintableERC20_succeeds() (gas: 33072)
SystemConfig_Initialize_TestFail:test_initialize_lowGasLimit_reverts() (gas: 62012)
SystemConfig_Setters_TestFail:test_setBatcherHash_notOwner_reverts() (gas: 10612)
SystemConfig_Setters_TestFail:test_setGasConfig_notOwner_reverts() (gas: 10555)
......
// SPDX-License-Identifier: MIT
pragma solidity 0.8.15;
import { StandardBridge } from "../universal/StandardBridge.sol";
import { CommonTest } from "./CommonTest.t.sol";
import { OptimismMintableERC20, ILegacyMintableERC20 } from "../universal/OptimismMintableERC20.sol";
import { ERC20 } from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
/**
* @title StandardBridgeTester
* @notice Simple wrapper around the StandardBridge contract that exposes
* internal functions so they can be more easily tested directly.
*/
contract StandardBridgeTester is StandardBridge {
constructor(address payable _messenger, address payable _otherBridge)
StandardBridge(_messenger, _otherBridge) {}
function isOptimismMintableERC20(address _token) external view returns (bool) {
return _isOptimismMintableERC20(_token);
}
function isCorrectTokenPair(address _mintableToken, address _otherToken) external view returns (bool) {
return _isCorrectTokenPair(_mintableToken, _otherToken);
}
receive() external override payable {}
}
/**
* @title LegacyMintable
* @notice Simple implementation of the legacy OptimismMintableERC20.
*/
contract LegacyMintable is ERC20, ILegacyMintableERC20 {
constructor(string memory _name, string memory _ticker) ERC20(_name, _ticker) {}
function l1Token() external view returns (address) {
return address(0);
}
function mint(address _to, uint256 _amount) external {}
function burn(address _from, uint256 _amount) external {}
/**
* @notice Implements ERC165. This implementation should not be changed as
* it is how the actual legacy optimism mintable token does the
* check. Allows for testing against code that is has been deployed,
* assuming different compiler version is no problem.
*/
function supportsInterface(bytes4 _interfaceId) external view returns (bool) {
bytes4 firstSupportedInterface = bytes4(keccak256("supportsInterface(bytes4)")); // ERC165
bytes4 secondSupportedInterface = ILegacyMintableERC20.l1Token.selector ^
ILegacyMintableERC20.mint.selector ^
ILegacyMintableERC20.burn.selector;
return _interfaceId == firstSupportedInterface || _interfaceId == secondSupportedInterface;
}
}
/**
* @title StandardBridge_Stateless_Test
* @notice Tests internal functions that require no existing state or contract
* interactions with the messenger.
*/
contract StandardBridge_Stateless_Test is CommonTest {
StandardBridgeTester internal bridge;
OptimismMintableERC20 internal mintable;
ERC20 internal erc20;
LegacyMintable internal legacy;
function setUp() public override {
super.setUp();
bridge = new StandardBridgeTester({
_messenger: payable(address(0)),
_otherBridge: payable(address(0))
});
mintable = new OptimismMintableERC20({
_bridge: address(0),
_remoteToken: address(0),
_name: "Stonks",
_symbol: "STONK"
});
erc20 = new ERC20("Altcoin", "ALT");
legacy = new LegacyMintable("Legacy", "LEG");
}
/**
* @notice Test coverage for identifying OptimismMintableERC20 tokens.
* This function should return true for both modern and legacy
* OptimismMintableERC20 tokens and false for any accounts that
* do not implement the interface.
*/
function test_isOptimismMintableERC20_succeeds() external {
// Both the modern and legacy mintable tokens should return true
assertTrue(bridge.isOptimismMintableERC20(address(mintable)));
assertTrue(bridge.isOptimismMintableERC20(address(legacy)));
// A regular ERC20 should return false
assertFalse(bridge.isOptimismMintableERC20(address(erc20)));
// Non existent contracts should return false and not revert
assertEq(address(0x20).code.length, 0);
assertFalse(bridge.isOptimismMintableERC20(address(0x20)));
}
/**
* @notice Test coverage of isCorrectTokenPair under different types of
* tokens.
*/
function test_isCorrectTokenPair_succeeds() external {
// Modern + known to be correct remote token
assertTrue(
bridge.isCorrectTokenPair(address(mintable), mintable.remoteToken())
);
// Modern + known to be correct l1Token (legacy interface)
assertTrue(
bridge.isCorrectTokenPair(address(mintable), mintable.l1Token())
);
// Modern + known to be incorrect remote token
assertTrue(mintable.remoteToken() != address(0x20));
assertFalse(
bridge.isCorrectTokenPair(address(mintable), address(0x20))
);
// Legacy + known to be correct l1Token
assertTrue(
bridge.isCorrectTokenPair(address(legacy), legacy.l1Token())
);
// Legacy + known to be incorrect l1Token
assertTrue(legacy.l1Token() != address(0x20));
assertFalse(
bridge.isCorrectTokenPair(address(legacy), address(0x20))
);
// A token that doesn't support either modern or legacy interface
// will revert
vm.expectRevert();
bridge.isCorrectTokenPair(address(erc20), address(1));
}
}
......@@ -10,7 +10,7 @@ import { IERC165 } from "@openzeppelin/contracts/utils/introspection/IERC165.sol
* OptimismMintableERC20.
*/
interface IOptimismMintableERC20 is IERC165 {
function remoteToken() external returns (address);
function remoteToken() external view returns (address);
function bridge() external returns (address);
......@@ -26,7 +26,7 @@ interface IOptimismMintableERC20 is IERC165 {
* on the OptimismMintableERC20 contract for backwards compatibility.
*/
interface ILegacyMintableERC20 is IERC165 {
function l1Token() external returns (address);
function l1Token() external view returns (address);
function mint(address _to, uint256 _amount) external;
......
......@@ -458,6 +458,8 @@ abstract contract StandardBridge {
/**
* @notice Checks if the "other token" is the correct pair token for the OptimismMintableERC20.
* Calls can be saved in the future by combining this logic with
* `_isOptimismMintableERC20`.
*
* @param _mintableToken OptimismMintableERC20 to check against.
* @param _otherToken Pair token to check.
......@@ -469,7 +471,11 @@ abstract contract StandardBridge {
view
returns (bool)
{
return _otherToken == OptimismMintableERC20(_mintableToken).l1Token();
if (ERC165Checker.supportsInterface(_mintableToken, type(ILegacyMintableERC20).interfaceId)) {
return _otherToken == ILegacyMintableERC20(_mintableToken).l1Token();
} else {
return _otherToken == IOptimismMintableERC20(_mintableToken).remoteToken();
}
}
/** @notice Emits the ETHBridgeInitiated event and if necessary the appropriate legacy event
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment