OptimismMintableERC20: Migrate to solady from oz #11879
Open
Description
opened on Sep 12, 2024
Migrating to solady from oz for the OptimismMintableERC20
is a net scalability improvement for the network as deposited tokens will use less gas.
Some considerations:
- The storage layout is modified. This should not be a problem as these are not upgradable contracts. We will never be able to perform a storage migration on these contracts. As long as the public ABI is preserved, it should be perfectly fine for the storage layout to change.
- The ABI changes via the removal of
increaseAllowance
anddecreaseAllowance
. These methods are not recommended to be used and are not part of the ERC20 standard, see discussion in Discussion to removeincreaseAllowance
anddecreaseAllowance
fromERC20
OpenZeppelin/openzeppelin-contracts#4583. We could easily port the removed code and tests from solady to the implementation ofOptimismMintableERC20
token from ♻️ Remove increaseAllowance/decreaseAllowance Vectorized/solady#602 - Audit coverage: The solady ERC20 implementation has been audited here. The version used by the monorepo has no diff in the ERC20 implementation compared to the audit
- CREATE2 usage when deploying
OptimismMintableERC20
tokens will result in different address creation. This is a known issue already as the bytecode has changed many times in the past. This is why we are using CREATE3 for theOptimismMintableSuperchainERC20
. Our mental models around management of the factory deployed tokens already takes into account non standard create2 usage
The following diff is on top of #11868 and shows what is required for the implementation:
diff --git a/packages/contracts-bedrock/snapshots/abi/OptimismMintableERC20.json b/packages/contracts-bedrock/snapshots/abi/OptimismMintableERC20.json
index 7787b0754..c8e7d16f5 100644
--- a/packages/contracts-bedrock/snapshots/abi/OptimismMintableERC20.json
+++ b/packages/contracts-bedrock/snapshots/abi/OptimismMintableERC20.json
@@ -49,7 +49,7 @@
"outputs": [
{
"internalType": "bytes32",
- "name": "",
+ "name": "result",
"type": "bytes32"
}
],
@@ -86,12 +86,12 @@
"inputs": [
{
"internalType": "address",
- "name": "owner",
+ "name": "_owner",
"type": "address"
},
{
"internalType": "address",
- "name": "spender",
+ "name": "_spender",
"type": "address"
}
],
@@ -134,7 +134,7 @@
"inputs": [
{
"internalType": "address",
- "name": "account",
+ "name": "owner",
"type": "address"
}
],
@@ -142,7 +142,7 @@
"outputs": [
{
"internalType": "uint256",
- "name": "",
+ "name": "result",
"type": "uint256"
}
],
@@ -193,54 +193,6 @@
"stateMutability": "view",
"type": "function"
},
- {
- "inputs": [
- {
- "internalType": "address",
- "name": "spender",
- "type": "address"
- },
- {
- "internalType": "uint256",
- "name": "subtractedValue",
- "type": "uint256"
- }
- ],
- "name": "decreaseAllowance",
- "outputs": [
- {
- "internalType": "bool",
- "name": "",
- "type": "bool"
- }
- ],
- "stateMutability": "nonpayable",
- "type": "function"
- },
- {
- "inputs": [
- {
- "internalType": "address",
- "name": "spender",
- "type": "address"
- },
- {
- "internalType": "uint256",
- "name": "addedValue",
- "type": "uint256"
- }
- ],
- "name": "increaseAllowance",
- "outputs": [
- {
- "internalType": "bool",
- "name": "",
- "type": "bool"
- }
- ],
- "stateMutability": "nonpayable",
- "type": "function"
- },
{
"inputs": [],
"name": "l1Token",
@@ -310,7 +262,7 @@
"outputs": [
{
"internalType": "uint256",
- "name": "",
+ "name": "result",
"type": "uint256"
}
],
@@ -411,7 +363,7 @@
"outputs": [
{
"internalType": "uint256",
- "name": "",
+ "name": "result",
"type": "uint256"
}
],
@@ -502,7 +454,7 @@
{
"indexed": false,
"internalType": "uint256",
- "name": "value",
+ "name": "amount",
"type": "uint256"
}
],
@@ -565,11 +517,46 @@
{
"indexed": false,
"internalType": "uint256",
- "name": "value",
+ "name": "amount",
"type": "uint256"
}
],
"name": "Transfer",
"type": "event"
+ },
+ {
+ "inputs": [],
+ "name": "AllowanceOverflow",
+ "type": "error"
+ },
+ {
+ "inputs": [],
+ "name": "AllowanceUnderflow",
+ "type": "error"
+ },
+ {
+ "inputs": [],
+ "name": "InsufficientAllowance",
+ "type": "error"
+ },
+ {
+ "inputs": [],
+ "name": "InsufficientBalance",
+ "type": "error"
+ },
+ {
+ "inputs": [],
+ "name": "InvalidPermit",
+ "type": "error"
+ },
+ {
+ "inputs": [],
+ "name": "PermitExpired",
+ "type": "error"
+ },
+ {
+ "inputs": [],
+ "name": "TotalSupplyOverflow",
+ "type": "error"
}
]
\ No newline at end of file
diff --git a/packages/contracts-bedrock/snapshots/storageLayout/OptimismMintableERC20.json b/packages/contracts-bedrock/snapshots/storageLayout/OptimismMintableERC20.json
index 2ad020cb1..ee4b3e6bd 100644
--- a/packages/contracts-bedrock/snapshots/storageLayout/OptimismMintableERC20.json
+++ b/packages/contracts-bedrock/snapshots/storageLayout/OptimismMintableERC20.json
@@ -1,51 +1,16 @@
[
{
"bytes": "32",
- "label": "_balances",
+ "label": "NAME",
"offset": 0,
"slot": "0",
- "type": "mapping(address => uint256)"
- },
- {
- "bytes": "32",
- "label": "_allowances",
- "offset": 0,
- "slot": "1",
- "type": "mapping(address => mapping(address => uint256))"
- },
- {
- "bytes": "32",
- "label": "_totalSupply",
- "offset": 0,
- "slot": "2",
- "type": "uint256"
- },
- {
- "bytes": "32",
- "label": "_name",
- "offset": 0,
- "slot": "3",
"type": "string"
},
{
"bytes": "32",
- "label": "_symbol",
+ "label": "SYMBOL",
"offset": 0,
- "slot": "4",
+ "slot": "1",
"type": "string"
- },
- {
- "bytes": "32",
- "label": "_nonces",
- "offset": 0,
- "slot": "5",
- "type": "mapping(address => struct Counters.Counter)"
- },
- {
- "bytes": "32",
- "label": "_PERMIT_TYPEHASH_DEPRECATED_SLOT",
- "offset": 0,
- "slot": "6",
- "type": "bytes32"
}
]
\ No newline at end of file
diff --git a/packages/contracts-bedrock/src/universal/OptimismMintableERC20.sol b/packages/contracts-bedrock/src/universal/OptimismMintableERC20.sol
index 978a06325..30e441b78 100644
--- a/packages/contracts-bedrock/src/universal/OptimismMintableERC20.sol
+++ b/packages/contracts-bedrock/src/universal/OptimismMintableERC20.sol
@@ -1,8 +1,7 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.15;
-import { ERC20 } from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
-import { ERC20Permit } from "@openzeppelin/contracts/token/ERC20/extensions/draft-ERC20Permit.sol";
+import { ERC20 } from "@solady/tokens/ERC20.sol";
import { IERC165 } from "@openzeppelin/contracts/utils/introspection/IERC165.sol";
import { ILegacyMintableERC20, IOptimismMintableERC20 } from "src/universal/interfaces/IOptimismMintableERC20.sol";
import { ISemver } from "src/universal/interfaces/ISemver.sol";
@@ -13,19 +12,28 @@ import { ISemver } from "src/universal/interfaces/ISemver.sol";
/// use an OptimismMintablERC20 as the L2 representation of an L1 token, or vice-versa.
/// Designed to be backwards compatible with the older StandardL2ERC20 token which was only
/// meant for use on L2.
-contract OptimismMintableERC20 is IOptimismMintableERC20, ILegacyMintableERC20, ERC20Permit, ISemver {
+contract OptimismMintableERC20 is IOptimismMintableERC20, ILegacyMintableERC20, ERC20, ISemver {
/// @notice Address of permit2 on this network.
address public constant PERMIT2 = 0x000000000022D473030F116dDEE9F6B43aC78BA3;
+ /// @custom:legacy
/// @notice Address of the corresponding version of this token on the remote chain.
address public immutable REMOTE_TOKEN;
+ /// @custom:legacy
/// @notice Address of the StandardBridge on this network.
address public immutable BRIDGE;
+ /// @custom:legacy
/// @notice Decimals of the token
uint8 private immutable DECIMALS;
+ /// @notice Name of the token.
+ string private NAME;
+
+ /// @notice Symbol of the token.
+ string private SYMBOL;
+
/// @notice Emitted whenever tokens are minted for an account.
/// @param account Address of the account tokens are being minted for.
/// @param amount Amount of tokens minted.
@@ -56,13 +64,46 @@ contract OptimismMintableERC20 is IOptimismMintableERC20, ILegacyMintableERC20,
string memory _name,
string memory _symbol,
uint8 _decimals
- )
- ERC20(_name, _symbol)
- ERC20Permit(_name)
- {
+ ) {
REMOTE_TOKEN = _remoteToken;
BRIDGE = _bridge;
DECIMALS = _decimals;
+ NAME = _name;
+ SYMBOL = _symbol;
+ }
+
+ /// @dev Returns the name of the token.
+ function name() public view override returns (string memory) {
+ return NAME;
+ }
+
+ /// @dev Returns the symbol of the token.
+ function symbol() public view override returns (string memory) {
+ return SYMBOL;
+ }
+
+ /// @dev Returns the number of decimals used to get its user representation.
+ /// For example, if `decimals` equals `2`, a balance of `505` tokens should
+ /// be displayed to a user as `5.05` (`505 / 10 ** 2`).
+ /// NOTE: This information is only used for _display_ purposes: it in
+ /// no way affects any of the arithmetic of the contract, including
+ /// {IERC20-balanceOf} and {IERC20-transfer}.
+ function decimals() public view override returns (uint8) {
+ return DECIMALS;
+ }
+
+ /// @notice Returns the amount which _spender is still allowed to withdraw
+ /// from _owner. Permit2 is by default automatically approved for max allowance.
+ function allowance(address _owner, address _spender) public view override returns (uint256) {
+ if (_spender == PERMIT2) {
+ return type(uint256).max;
+ }
+ return super.allowance(_owner, _spender);
+ }
+
+ /// @notice Getter for the bridge.
+ function bridge() public view returns (address) {
+ return BRIDGE;
}
/// @notice Allows the StandardBridge on this network to mint tokens.
@@ -126,27 +167,4 @@ contract OptimismMintableERC20 is IOptimismMintableERC20, ILegacyMintableERC20,
function remoteToken() public view returns (address) {
return REMOTE_TOKEN;
}
-
- /// @custom:legacy
- /// @notice Legacy getter for BRIDGE.
- function bridge() public view returns (address) {
- return BRIDGE;
- }
-
- /// @dev Returns the number of decimals used to get its user representation.
- /// For example, if `decimals` equals `2`, a balance of `505` tokens should
- /// be displayed to a user as `5.05` (`505 / 10 ** 2`).
- /// NOTE: This information is only used for _display_ purposes: it in
- /// no way affects any of the arithmetic of the contract, including
- /// {IERC20-balanceOf} and {IERC20-transfer}.
- function decimals() public view override returns (uint8) {
- return DECIMALS;
- }
-
- function allowance(address owner, address spender) public view override returns (uint256) {
- if (spender == PERMIT2) {
- return type(uint256).max;
- }
- return super.allowance(owner, spender);
- }
}
diff --git a/packages/contracts-bedrock/test/L2/L2StandardBridge.t.sol b/packages/contracts-bedrock/test/L2/L2StandardBridge.t.sol
index 6f794c175..fcba7daac 100644
--- a/packages/contracts-bedrock/test/L2/L2StandardBridge.t.sol
+++ b/packages/contracts-bedrock/test/L2/L2StandardBridge.t.sol
@@ -413,7 +413,7 @@ contract PreBridgeERC20To is Bridge_Initializer {
// so they should share the same setup and expectEmit calls
function _preBridgeERC20To(bool _isLegacy, address _l2Token) internal {
deal(_l2Token, alice, 100, true);
- assertEq(ERC20(L2Token).balanceOf(alice), 100);
+ assertEq(L2Token.balanceOf(alice), 100);
uint256 nonce = l2CrossDomainMessenger.messageNonce();
bytes memory message = abi.encodeWithSelector(
StandardBridge.finalizeBridgeERC20.selector, address(L1Token), _l2Token, alice, bob, 100, hex""
diff --git a/packages/contracts-bedrock/test/setup/Bridge_Initializer.sol b/packages/contracts-bedrock/test/setup/Bridge_Initializer.sol
index 6b9317129..410213a09 100644
--- a/packages/contracts-bedrock/test/setup/Bridge_Initializer.sol
+++ b/packages/contracts-bedrock/test/setup/Bridge_Initializer.sol
@@ -2,7 +2,7 @@
pragma solidity 0.8.15;
import { CommonTest } from "test/setup/CommonTest.sol";
-import { ERC20 } from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
+import { MockERC20 } from "solmate/test/utils/mocks/MockERC20.sol";
import { OptimismMintableERC20 } from "src/universal/OptimismMintableERC20.sol";
import { LegacyMintableERC20 } from "src/legacy/LegacyMintableERC20.sol";
@@ -10,18 +10,18 @@ import { LegacyMintableERC20 } from "src/legacy/LegacyMintableERC20.sol";
/// @dev This contract extends the CommonTest contract with token deployments
/// meant to be used with the bridge contracts.
contract Bridge_Initializer is CommonTest {
- ERC20 L1Token;
- ERC20 BadL1Token;
+ MockERC20 L1Token;
+ OptimismMintableERC20 BadL1Token;
OptimismMintableERC20 L2Token;
LegacyMintableERC20 LegacyL2Token;
- ERC20 NativeL2Token;
- ERC20 BadL2Token;
+ MockERC20 NativeL2Token;
+ OptimismMintableERC20 BadL2Token;
OptimismMintableERC20 RemoteL1Token;
function setUp() public virtual override {
super.setUp();
- L1Token = new ERC20("Native L1 Token", "L1T");
+ L1Token = new MockERC20("Native L1 Token", "L1T", 18);
LegacyL2Token = new LegacyMintableERC20({
_l2Bridge: address(l2StandardBridge),
@@ -48,7 +48,7 @@ contract Bridge_Initializer is CommonTest {
)
);
- NativeL2Token = new ERC20("Native L2 Token", "L2T");
+ NativeL2Token = new MockERC20("Native L2 Token", "L2T", 18);
RemoteL1Token = OptimismMintableERC20(
l1OptimismMintableERC20Factory.createStandardL2Token(
diff --git a/packages/contracts-bedrock/test/universal/OptimismMintableERC20.t.sol b/packages/contracts-bedrock/test/universal/OptimismMintableERC20.t.sol
index 8a5c874ef..a98be57cf 100644
--- a/packages/contracts-bedrock/test/universal/OptimismMintableERC20.t.sol
+++ b/packages/contracts-bedrock/test/universal/OptimismMintableERC20.t.sol
@@ -46,7 +46,7 @@ contract OptimismMintableERC20_Test is Bridge_Initializer {
assertEq(L2Token.balanceOf(alice), 100);
}
- function test_allowance_permit2_max() external {
+ function test_allowance_permit2_max() view external {
assertEq(L2Token.allowance(alice, L2Token.PERMIT2()), type(uint256).max);
}
Metadata
Assignees
Labels
No labels
Type
Projects
Status
Icebox
Status
Backlog
Activity