From 8209d4bc95abb57d9a4f572998c97125bf8cb6cd Mon Sep 17 00:00:00 2001 From: agusduha Date: Tue, 13 Aug 2024 10:07:26 -0300 Subject: [PATCH] feat: rollback interop factory, modify legacy one --- packages/contracts-bedrock/.gas-snapshot | 8 +- packages/contracts-bedrock/semver-lock.json | 4 +- .../abi/OptimismMintableERC20Factory.json | 21 ++- .../OptimismMintableERC20Factory.json | 13 +- .../OptimismMintableERC20Factory.sol | 49 +++--- .../OptimismMintableERC20FactoryInterop.sol | 79 ---------- .../OptimismMintableERC20Factory.t.sol | 149 +++++++++++++++--- 7 files changed, 184 insertions(+), 139 deletions(-) delete mode 100644 packages/contracts-bedrock/src/universal/OptimismMintableERC20FactoryInterop.sol diff --git a/packages/contracts-bedrock/.gas-snapshot b/packages/contracts-bedrock/.gas-snapshot index a97f05678b58..b3ea3b88545e 100644 --- a/packages/contracts-bedrock/.gas-snapshot +++ b/packages/contracts-bedrock/.gas-snapshot @@ -1,7 +1,7 @@ -GasBenchMark_L1CrossDomainMessenger:test_sendMessage_benchmark_0() (gas: 369380) -GasBenchMark_L1CrossDomainMessenger:test_sendMessage_benchmark_1() (gas: 2967520) -GasBenchMark_L1StandardBridge_Deposit:test_depositERC20_benchmark_0() (gas: 561992) -GasBenchMark_L1StandardBridge_Deposit:test_depositERC20_benchmark_1() (gas: 4074035) +GasBenchMark_L1CrossDomainMessenger:test_sendMessage_benchmark_0() (gas: 369356) +GasBenchMark_L1CrossDomainMessenger:test_sendMessage_benchmark_1() (gas: 2967496) +GasBenchMark_L1StandardBridge_Deposit:test_depositERC20_benchmark_0() (gas: 564483) +GasBenchMark_L1StandardBridge_Deposit:test_depositERC20_benchmark_1() (gas: 4076526) GasBenchMark_L1StandardBridge_Deposit:test_depositETH_benchmark_0() (gas: 466947) GasBenchMark_L1StandardBridge_Deposit:test_depositETH_benchmark_1() (gas: 3512629) GasBenchMark_L1StandardBridge_Finalize:test_finalizeETHWithdrawal_benchmark() (gas: 72624) diff --git a/packages/contracts-bedrock/semver-lock.json b/packages/contracts-bedrock/semver-lock.json index dc1f1cc9c4c5..25568e84f2b4 100644 --- a/packages/contracts-bedrock/semver-lock.json +++ b/packages/contracts-bedrock/semver-lock.json @@ -196,8 +196,8 @@ "sourceCodeHash": "0x52737b23e99bf79dd2c23196b3298e80aa41f740efc6adc7916e696833eb546a" }, "src/universal/OptimismMintableERC20Factory.sol": { - "initCodeHash": "0xf6f522681e7ae940cb778db68004f122b25194296a65bba7ad1d792bd593c4a6", - "sourceCodeHash": "0x9b8c73ea139f13028008eedef53a6b07576cd6b08979574e6dde3192656e9268" + "initCodeHash": "0x42291ef554697b24e310c9fcf49b8f13a429a924070cda4244da1d6a64717179", + "sourceCodeHash": "0xfa51d0af52ac01c227e1f291dec74663c81e99fa3594cc052af64641c4337e0f" }, "src/universal/OptimismMintableERC721.sol": { "initCodeHash": "0xb400f430acf4d65bee9635e4935a6e1e3a0284fc50aea40ad8b7818dc826f31c", diff --git a/packages/contracts-bedrock/snapshots/abi/OptimismMintableERC20Factory.json b/packages/contracts-bedrock/snapshots/abi/OptimismMintableERC20Factory.json index 3f2f14fcbe59..5fe1565bc3d9 100644 --- a/packages/contracts-bedrock/snapshots/abi/OptimismMintableERC20Factory.json +++ b/packages/contracts-bedrock/snapshots/abi/OptimismMintableERC20Factory.json @@ -86,7 +86,7 @@ "outputs": [ { "internalType": "address", - "name": "", + "name": "_localToken", "type": "address" } ], @@ -122,6 +122,25 @@ "stateMutability": "nonpayable", "type": "function" }, + { + "inputs": [ + { + "internalType": "address", + "name": "", + "type": "address" + } + ], + "name": "deployments", + "outputs": [ + { + "internalType": "address", + "name": "", + "type": "address" + } + ], + "stateMutability": "view", + "type": "function" + }, { "inputs": [ { diff --git a/packages/contracts-bedrock/snapshots/storageLayout/OptimismMintableERC20Factory.json b/packages/contracts-bedrock/snapshots/storageLayout/OptimismMintableERC20Factory.json index c39454b5e3eb..94c133d105d2 100644 --- a/packages/contracts-bedrock/snapshots/storageLayout/OptimismMintableERC20Factory.json +++ b/packages/contracts-bedrock/snapshots/storageLayout/OptimismMintableERC20Factory.json @@ -28,10 +28,17 @@ "type": "address" }, { - "bytes": "1568", - "label": "__gap", + "bytes": "32", + "label": "deployments", "offset": 0, "slot": "2", - "type": "uint256[49]" + "type": "mapping(address => address)" + }, + { + "bytes": "1536", + "label": "__gap", + "offset": 0, + "slot": "3", + "type": "uint256[48]" } ] \ No newline at end of file diff --git a/packages/contracts-bedrock/src/universal/OptimismMintableERC20Factory.sol b/packages/contracts-bedrock/src/universal/OptimismMintableERC20Factory.sol index 8d62a4a1c4ec..0cfaddc3554d 100644 --- a/packages/contracts-bedrock/src/universal/OptimismMintableERC20Factory.sol +++ b/packages/contracts-bedrock/src/universal/OptimismMintableERC20Factory.sol @@ -4,6 +4,7 @@ pragma solidity 0.8.15; import { OptimismMintableERC20 } from "src/universal/OptimismMintableERC20.sol"; import { ISemver } from "src/universal/ISemver.sol"; import { Initializable } from "@openzeppelin/contracts/proxy/utils/Initializable.sol"; +import { IOptimismERC20Factory } from "src/L2/IOptimismERC20Factory.sol"; /// @custom:proxied /// @custom:predeployed 0x4200000000000000000000000000000000000012 @@ -12,7 +13,7 @@ import { Initializable } from "@openzeppelin/contracts/proxy/utils/Initializable /// contracts on the network it's deployed to. Simplifies the deployment process for users /// who may be less familiar with deploying smart contracts. Designed to be backwards /// compatible with the older StandardL2ERC20Factory contract. -abstract contract OptimismMintableERC20Factory is ISemver, Initializable { +contract OptimismMintableERC20Factory is ISemver, Initializable, IOptimismERC20Factory { /// @custom:spacer OptimismMintableERC20Factory's initializer slot spacing /// @notice Spacer to avoid packing into the initializer slot bytes30 private spacer_0_2_30; @@ -21,10 +22,14 @@ abstract contract OptimismMintableERC20Factory is ISemver, Initializable { /// @custom:network-specific address public bridge; + /// @notice Mapping of local token address to remote token address. + /// This is used to keep track of the token deployments. + mapping(address => address) public deployments; + /// @notice Reserve extra slots in the storage layout for future upgrades. - /// A gap size of 49 was chosen here, so that the first slot used in a child contract - /// would be 1 plus a multiple of 50. - uint256[49] private __gap; + /// A gap size of 48 was chosen here, so that the first slot used in a child contract + /// would be a multiple of 50. + uint256[48] private __gap; /// @custom:legacy /// @notice Emitted whenever a new OptimismMintableERC20 is created. Legacy version of the newer @@ -104,7 +109,7 @@ abstract contract OptimismMintableERC20Factory is ISemver, Initializable { /// @param _name ERC20 name. /// @param _symbol ERC20 symbol. /// @param _decimals ERC20 decimals - /// @return Address of the newly created token. + /// @return _localToken Address of the newly created token. function createOptimismMintableERC20WithDecimals( address _remoteToken, string memory _name, @@ -112,25 +117,21 @@ abstract contract OptimismMintableERC20Factory is ISemver, Initializable { uint8 _decimals ) public - returns (address) + returns (address _localToken) { - return createWithCreate3(_remoteToken, _name, _symbol, _decimals); - } + require(_remoteToken != address(0), "OptimismMintableERC20Factory: must provide remote token address"); - /// @notice Creates an instance of the OptimismMintableERC20 contract, with specified decimals using CREATE3. - /// To be implemented by the child contract. - /// @param _remoteToken Address of the token on the remote chain. - /// @param _name ERC20 name. - /// @param _symbol ERC20 symbol. - /// @param _decimals ERC20 decimals. - /// @return Address of the newly created token. - function createWithCreate3( - address _remoteToken, - string memory _name, - string memory _symbol, - uint8 _decimals - ) - public - virtual - returns (address); + bytes32 salt = keccak256(abi.encode(_remoteToken, _name, _symbol, _decimals)); + + _localToken = address(new OptimismMintableERC20{ salt: salt }(bridge, _remoteToken, _name, _symbol, _decimals)); + + deployments[_localToken] = _remoteToken; + + // Emit the old event too for legacy support. + emit StandardL2TokenCreated(_remoteToken, _localToken); + + // Emit the updated event. The arguments here differ from the legacy event, but + // are consistent with the ordering used in StandardBridge events. + emit OptimismMintableERC20Created(_localToken, _remoteToken, msg.sender); + } } diff --git a/packages/contracts-bedrock/src/universal/OptimismMintableERC20FactoryInterop.sol b/packages/contracts-bedrock/src/universal/OptimismMintableERC20FactoryInterop.sol deleted file mode 100644 index 74751a57e9bb..000000000000 --- a/packages/contracts-bedrock/src/universal/OptimismMintableERC20FactoryInterop.sol +++ /dev/null @@ -1,79 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity 0.8.15; - -import { OptimismMintableERC20Factory, OptimismMintableERC20 } from "src/universal/OptimismMintableERC20Factory.sol"; -import { IOptimismERC20Factory } from "src/L2/IOptimismERC20Factory.sol"; -import { CREATE3 } from "@rari-capital/solmate/src/utils/CREATE3.sol"; - -/// @custom:proxied -/// @custom:predeployed 0x4200000000000000000000000000000000000012 -/// @title OptimismMintableERC20FactoryInterop -/// @notice OptimismMintableERC20FactoryInterop is a factory contract that generates OptimismMintableERC20 -/// contracts on the network it's deployed to, using CREATE3. Simplifies the deployment process for users -/// who may be less familiar with deploying smart contracts. Designed to be backwards -/// compatible with the older OptimismMintableERC20Factory contract. -contract OptimismMintableERC20FactoryInterop is OptimismMintableERC20Factory, IOptimismERC20Factory { - /// @notice Storage slot that the OptimismMintableERC20FactoryInteropStorage struct is stored at. - /// keccak256(abi.encode(uint256(keccak256("optimismMintableERC20FactoryInterop.storage")) - 1)) & - /// ~bytes32(uint256(0xff)); - bytes32 internal constant OPTIMISM_MINTABLE_ERC20_FACTORY_INTEROP_SLOT = - 0xb6fdf24dfda35722597f70f86628ef4ce6db853b20879bdd2d61f0f5d169b100; - - /// @notice Storage struct for the OptimismMintableERC20FactoryInterop storage. - /// @custom:storage-location erc7201:optimismMintableERC20FactoryInterop.storage - struct OptimismMintableERC20FactoryInteropStorage { - /// @notice Mapping of local token address to remote token address. - mapping(address => address) deployments; - } - - /// @notice Returns the storage for the OptimismMintableERC20FactoryInteropStorage. - function _getStorage() private pure returns (OptimismMintableERC20FactoryInteropStorage storage _storage) { - assembly { - _storage.slot := OPTIMISM_MINTABLE_ERC20_FACTORY_INTEROP_SLOT - } - } - - /// @notice Creates an instance of the OptimismMintableERC20 contract, with specified decimals using CREATE3. - /// @param _remoteToken Address of the token on the remote chain. - /// @param _name ERC20 name. - /// @param _symbol ERC20 symbol. - /// @param _decimals ERC20 decimals. - /// @return _localToken Address of the newly created token. - function createWithCreate3( - address _remoteToken, - string memory _name, - string memory _symbol, - uint8 _decimals - ) - public - override - returns (address _localToken) - { - require(_remoteToken != address(0), "OptimismMintableERC20Factory: must provide remote token address"); - - bytes memory creationCode = abi.encodePacked( - type(OptimismMintableERC20).creationCode, abi.encode(bridge, _remoteToken, _name, _symbol, _decimals) - ); - - bytes32 salt = keccak256(abi.encode(_remoteToken, _name, _symbol, _decimals)); - - _localToken = CREATE3.deploy({ salt: salt, creationCode: creationCode, value: 0 }); - - _getStorage().deployments[_localToken] = _remoteToken; - - // Emit the old event too for legacy support. - emit StandardL2TokenCreated(_remoteToken, _localToken); - - // Emit the updated event. The arguments here differ from the legacy event, but - // are consistent with the ordering used in StandardBridge events. - emit OptimismMintableERC20Created(_localToken, _remoteToken, msg.sender); - } - - /// @notice Returns the address of the token on the remote chain if the deployment exists, - /// else returns `address(0)`. - /// @param _localToken Address of the token on the local chain. - /// @return _remoteToken Address of the token on the remote chain. - function deployments(address _localToken) external view override returns (address _remoteToken) { - return _getStorage().deployments[_localToken]; - } -} diff --git a/packages/contracts-bedrock/test/universal/OptimismMintableERC20Factory.t.sol b/packages/contracts-bedrock/test/universal/OptimismMintableERC20Factory.t.sol index d1919a5ff555..e0c18ecf655b 100644 --- a/packages/contracts-bedrock/test/universal/OptimismMintableERC20Factory.t.sol +++ b/packages/contracts-bedrock/test/universal/OptimismMintableERC20Factory.t.sol @@ -17,19 +17,20 @@ contract OptimismMintableTokenFactory_Test is Bridge_Initializer { event StandardL2TokenCreated(address indexed remoteToken, address indexed localToken); event OptimismMintableERC20Created(address indexed localToken, address indexed remoteToken, address deployer); - /// @dev Tests that the constructor is initialized correctly. + /// @notice Tests that the constructor is initialized correctly. function test_constructor_succeeds() external { OptimismMintableERC20Factory impl = new OptimismMintableERC20Factory(); assertEq(address(impl.BRIDGE()), address(0)); assertEq(address(impl.bridge()), address(0)); } - /// @dev Tests that the proxy is initialized correctly. + /// @notice Tests that the proxy is initialized correctly. function test_initialize_succeeds() external view { assertEq(address(l1OptimismMintableERC20Factory.BRIDGE()), address(l1StandardBridge)); assertEq(address(l1OptimismMintableERC20Factory.bridge()), address(l1StandardBridge)); } + /// @notice Tests that the upgrade is successful. function test_upgrading_succeeds() external { Proxy proxy = Proxy(deploy.mustGetAddress("OptimismMintableERC20FactoryProxy")); // Check an unused slot before upgrading. @@ -49,60 +50,156 @@ contract OptimismMintableTokenFactory_Test is Bridge_Initializer { assertEq(slot21Expected, slot21After); } - function test_createStandardL2Token_succeeds() external { - address remote = address(4); + /// @notice Test that calling `createStandardL2Token` with valid parameters succeeds. + function test_createStandardL2Token_succeeds( + address _caller, + address _remoteToken, + string memory _name, + string memory _symbol + ) + external + { + // Assume + vm.assume(_remoteToken != address(0)); + // Arrange // Defaults to 18 decimals - address local = calculateTokenAddress(remote, "Beep", "BOOP", 18); + address local = _calculateTokenAddress(_remoteToken, _name, _symbol, 18); vm.expectEmit(true, true, true, true); - emit StandardL2TokenCreated(remote, local); + emit StandardL2TokenCreated(_remoteToken, local); vm.expectEmit(true, true, true, true); - emit OptimismMintableERC20Created(local, remote, alice); + emit OptimismMintableERC20Created(local, _remoteToken, _caller); - vm.prank(alice); - address addr = l2OptimismMintableERC20Factory.createStandardL2Token(remote, "Beep", "BOOP"); + // Act + vm.prank(_caller); + address addr = l2OptimismMintableERC20Factory.createStandardL2Token(_remoteToken, _name, _symbol); + + // Assert assertTrue(addr == local); assertTrue(OptimismMintableERC20(local).decimals() == 18); + assertEq(l2OptimismMintableERC20Factory.deployments(local), _remoteToken); } - function test_createStandardL2TokenWithDecimals_succeeds() external { - address remote = address(4); - address local = calculateTokenAddress(remote, "Beep", "BOOP", 6); + /// @notice Test that calling `createOptimismMintableERC20WithDecimals` with valid parameters succeeds. + function test_createStandardL2TokenWithDecimals_succeeds( + address _caller, + address _remoteToken, + string memory _name, + string memory _symbol, + uint8 _decimals + ) + external + { + // Assume + vm.assume(_remoteToken != address(0)); + + // Arrange + address local = _calculateTokenAddress(_remoteToken, _name, _symbol, _decimals); vm.expectEmit(true, true, true, true); - emit StandardL2TokenCreated(remote, local); + emit StandardL2TokenCreated(_remoteToken, local); vm.expectEmit(true, true, true, true); - emit OptimismMintableERC20Created(local, remote, alice); + emit OptimismMintableERC20Created(local, _remoteToken, _caller); - vm.prank(alice); - address addr = l2OptimismMintableERC20Factory.createOptimismMintableERC20WithDecimals(remote, "Beep", "BOOP", 6); + // Act + vm.prank(_caller); + address addr = l2OptimismMintableERC20Factory.createOptimismMintableERC20WithDecimals( + _remoteToken, _name, _symbol, _decimals + ); + + // Assert assertTrue(addr == local); + assertTrue(OptimismMintableERC20(local).decimals() == _decimals); + assertEq(l2OptimismMintableERC20Factory.deployments(local), _remoteToken); + } + + /// @notice Test that calling `createStandardL2Token` with the same parameters twice reverts. + function test_createStandardL2Token_sameTwice_reverts( + address _caller, + address _remoteToken, + string memory _name, + string memory _symbol + ) + external + { + // Assume + vm.assume(_remoteToken != address(0)); + + vm.prank(_caller); + l2OptimismMintableERC20Factory.createStandardL2Token(_remoteToken, _name, _symbol); + + // Arrange + vm.expectRevert(bytes("")); - assertTrue(OptimismMintableERC20(local).decimals() == 6); + // Act + vm.prank(_caller); + l2OptimismMintableERC20Factory.createStandardL2Token(_remoteToken, _name, _symbol); } - function test_createStandardL2Token_sameTwice_reverts() external { - address remote = address(4); + /// @notice Test that calling `createStandardL2TokenWithDecimals` with the same parameters twice reverts. + function test_createStandardL2TokenWithDecimals_sameTwice_reverts( + address _caller, + address _remoteToken, + string memory _name, + string memory _symbol, + uint8 _decimals + ) + external + { + // Assume + vm.assume(_remoteToken != address(0)); - vm.prank(alice); - l2OptimismMintableERC20Factory.createStandardL2Token(remote, "Beep", "BOOP"); + vm.prank(_caller); + l2OptimismMintableERC20Factory.createOptimismMintableERC20WithDecimals(_remoteToken, _name, _symbol, _decimals); + // Arrange vm.expectRevert(bytes("")); - vm.prank(alice); - l2OptimismMintableERC20Factory.createStandardL2Token(remote, "Beep", "BOOP"); + // Act + vm.prank(_caller); + l2OptimismMintableERC20Factory.createOptimismMintableERC20WithDecimals(_remoteToken, _name, _symbol, _decimals); } - function test_createStandardL2Token_remoteIsZero_reverts() external { + /// @notice Test that calling `createStandardL2Token` with a zero remote token address reverts. + function test_createStandardL2Token_remoteIsZero_reverts( + address _caller, + string memory _name, + string memory _symbol + ) + external + { + // Arrange address remote = address(0); vm.expectRevert("OptimismMintableERC20Factory: must provide remote token address"); - l2OptimismMintableERC20Factory.createStandardL2Token(remote, "Beep", "BOOP"); + + // Act + vm.prank(_caller); + l2OptimismMintableERC20Factory.createStandardL2Token(remote, _name, _symbol); + } + + /// @notice Test that calling `createStandardL2TokenWithDecimals` with a zero remote token address reverts. + function test_createStandardL2TokenWithDecimals_remoteIsZero_reverts( + address _caller, + string memory _name, + string memory _symbol, + uint8 _decimals + ) + external + { + // Arrange + address remote = address(0); + vm.expectRevert("OptimismMintableERC20Factory: must provide remote token address"); + + // Act + vm.prank(_caller); + l2OptimismMintableERC20Factory.createOptimismMintableERC20WithDecimals(remote, _name, _symbol, _decimals); } - function calculateTokenAddress( + /// @notice Precalculates the address of the token contract. + function _calculateTokenAddress( address _remote, string memory _name, string memory _symbol,