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: modify OptimismMintableERC20Factory for convert #17

Merged
merged 8 commits into from
Aug 13, 2024
Merged
Show file tree
Hide file tree
Changes from 7 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
8 changes: 4 additions & 4 deletions packages/contracts-bedrock/.gas-snapshot
Original file line number Diff line number Diff line change
@@ -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)
Expand Down
4 changes: 2 additions & 2 deletions packages/contracts-bedrock/semver-lock.json
Original file line number Diff line number Diff line change
Expand Up @@ -196,8 +196,8 @@
"sourceCodeHash": "0x52737b23e99bf79dd2c23196b3298e80aa41f740efc6adc7916e696833eb546a"
},
"src/universal/OptimismMintableERC20Factory.sol": {
"initCodeHash": "0xf6f522681e7ae940cb778db68004f122b25194296a65bba7ad1d792bd593c4a6",
"sourceCodeHash": "0x9b8c73ea139f13028008eedef53a6b07576cd6b08979574e6dde3192656e9268"
"initCodeHash": "0x42291ef554697b24e310c9fcf49b8f13a429a924070cda4244da1d6a64717179",
"sourceCodeHash": "0xfa51d0af52ac01c227e1f291dec74663c81e99fa3594cc052af64641c4337e0f"
},
"src/universal/OptimismMintableERC721.sol": {
"initCodeHash": "0xb400f430acf4d65bee9635e4935a6e1e3a0284fc50aea40ad8b7818dc826f31c",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@
"outputs": [
{
"internalType": "address",
"name": "",
"name": "_localToken",
"type": "address"
}
],
Expand Down Expand Up @@ -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": [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]"
}
]
4 changes: 2 additions & 2 deletions packages/contracts-bedrock/src/L2/IOptimismERC20Factory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Choose a reason for hiding this comment

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

Same question here with the returned arg

Suggested change
function deployments(address _localToken) external view returns (address _remoteToken);
function deployments(address _localToken) external view returns (address remoteToken_);

Copy link
Member Author

Choose a reason for hiding this comment

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

Newer contracts like L2ToL2CrossDomainMessenger have it like that, so not fixing for now unless it is asked for

}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
Expand All @@ -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
Expand All @@ -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() {
Expand Down Expand Up @@ -104,29 +109,29 @@ 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.

Choose a reason for hiding this comment

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

not sure if there is a convention, but they dont use underscore for localToken in their current version.

function createOptimismMintableERC20WithDecimals(
address _remoteToken,
string memory _name,
string memory _symbol,
uint8 _decimals
)
public
returns (address)
returns (address _localToken)

Choose a reason for hiding this comment

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

They use returns (address) instead of declaring the variable here

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok we can rollback it to make the less changes possible

{
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));

_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 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);

return localToken;
emit OptimismMintableERC20Created(_localToken, _remoteToken, msg.sender);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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,
Expand Down
Loading