From 014c25d80f55227a3dbd83541ec30faddab5633f Mon Sep 17 00:00:00 2001 From: AgusDuha <81362284+agusduha@users.noreply.github.com> Date: Tue, 13 Aug 2024 10:40:30 -0300 Subject: [PATCH] feat: modify OptimismMintableERC20Factory for convert (#17) * test: add L2 standard bridge interop unit tests * fix: add tests natspec * fix: add generic factory interface * feat: modify OptimismMintableERC20Factory for convert * fix: use only a public function for create3 * feat: rollback interop factory, modify legacy one * fix: delete local token return variable --- packages/contracts-bedrock/.gas-snapshot | 8 +- packages/contracts-bedrock/semver-lock.json | 4 +- .../abi/OptimismMintableERC20Factory.json | 19 +++ .../OptimismMintableERC20Factory.json | 13 +- .../src/L2/IOptimismERC20Factory.sol | 4 +- .../OptimismMintableERC20Factory.sol | 20 ++- .../OptimismMintableERC20Factory.t.sol | 149 +++++++++++++++--- 7 files changed, 174 insertions(+), 43 deletions(-) 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..ce8add4c9f7a 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": "0x29a49fc387ad84f82199947e49a0d1960902f63492d974c26afd72372e748648", + "sourceCodeHash": "0xbc6cf74368c244bdea8ed64c501129d0b6d41db421dc91d1de051f7b505a4977" }, "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..d3da8e2de735 100644 --- a/packages/contracts-bedrock/snapshots/abi/OptimismMintableERC20Factory.json +++ b/packages/contracts-bedrock/snapshots/abi/OptimismMintableERC20Factory.json @@ -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/L2/IOptimismERC20Factory.sol b/packages/contracts-bedrock/src/L2/IOptimismERC20Factory.sol index 0286a8f7b34e..5e0040aa83cf 100644 --- a/packages/contracts-bedrock/src/L2/IOptimismERC20Factory.sol +++ b/packages/contracts-bedrock/src/L2/IOptimismERC20Factory.sol @@ -6,7 +6,7 @@ pragma solidity ^0.8.0; /// determine if a ERC20 contract is deployed by a factory. interface IOptimismERC20Factory { /// @notice Checks if a ERC20 token is deployed by the factory. - /// @param _token The address of the ERC20 token to check the deployment. + /// @param _localToken The address of the ERC20 token to check the deployment. /// @return _remoteToken The address of the remote token if it is deployed or `address(0)` if not. - function deployments(address _token) external view returns (address _remoteToken); + function deployments(address _localToken) external view returns (address _remoteToken); } diff --git a/packages/contracts-bedrock/src/universal/OptimismMintableERC20Factory.sol b/packages/contracts-bedrock/src/universal/OptimismMintableERC20Factory.sol index 6e7baf31f1fb..799389542abe 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. -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 @@ 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 @@ -43,8 +48,8 @@ contract OptimismMintableERC20Factory is ISemver, Initializable { /// the OptimismMintableERC20 token contract since this contract /// is responsible for deploying OptimismMintableERC20 contracts. /// @notice Semantic version. - /// @custom:semver 1.9.0 - string public constant version = "1.9.0"; + /// @custom:semver 1.10.0 + string public constant version = "1.10.0"; /// @notice Constructs the OptimismMintableERC20Factory contract. constructor() { @@ -117,9 +122,12 @@ contract OptimismMintableERC20Factory is ISemver, Initializable { require(_remoteToken != address(0), "OptimismMintableERC20Factory: must provide remote token address"); bytes32 salt = keccak256(abi.encode(_remoteToken, _name, _symbol, _decimals)); + address 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); 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,