From 9ed311dd72b37a7185c6378b73eaa30b9a412929 Mon Sep 17 00:00:00 2001 From: 0xDiscotech <131301107+0xDiscotech@users.noreply.github.com> Date: Tue, 24 Sep 2024 18:49:55 -0300 Subject: [PATCH 1/5] refactor: use oz upgradeable erc20 as dependency * chore: update interfaces * fix: tests based on changes --- packages/contracts-bedrock/foundry.toml | 2 + .../src/L2/OptimismSuperchainERC20.sol | 12 +- .../src/L2/SuperchainERC20.sol | 4 +- .../interfaces/IOptimismSuperchainERC20.sol | 17 +- .../src/L2/interfaces/ISuperchainERC20.sol | 37 ++--- .../test/L2/OptimismSuperchainERC20.t.sol | 133 ---------------- .../OptimismSuperchainERC20/PROPERTIES.md | 38 +++-- .../fuzz/Protocol.guided.t.sol | 146 ------------------ .../fuzz/Protocol.unguided.t.sol | 72 --------- 9 files changed, 50 insertions(+), 411 deletions(-) diff --git a/packages/contracts-bedrock/foundry.toml b/packages/contracts-bedrock/foundry.toml index cef9f85bbaeb..fc5cc2360b3a 100644 --- a/packages/contracts-bedrock/foundry.toml +++ b/packages/contracts-bedrock/foundry.toml @@ -12,6 +12,8 @@ optimizer = true optimizer_runs = 999999 remappings = [ '@openzeppelin/contracts-upgradeable/=lib/openzeppelin-contracts-upgradeable/contracts', + '@openzeppelin/contracts-upgradeable-v5/=lib/openzeppelin-contracts-upgradeable-v5/contracts', + 'openzeppelin-contracts-upgradeable-v5/@openzeppelin/contracts/=lib/openzeppelin-contracts-v5/contracts', # Package-specific '@openzeppelin/contracts/=lib/openzeppelin-contracts/contracts', '@openzeppelin/contracts-v5/=lib/openzeppelin-contracts-v5/contracts', '@rari-capital/solmate/=lib/solmate', diff --git a/packages/contracts-bedrock/src/L2/OptimismSuperchainERC20.sol b/packages/contracts-bedrock/src/L2/OptimismSuperchainERC20.sol index 81cef632bfbe..b89390922172 100644 --- a/packages/contracts-bedrock/src/L2/OptimismSuperchainERC20.sol +++ b/packages/contracts-bedrock/src/L2/OptimismSuperchainERC20.sol @@ -5,10 +5,8 @@ import { IOptimismSuperchainERC20Extension } from "src/L2/interfaces/IOptimismSu import { IL2ToL2CrossDomainMessenger } from "src/L2/interfaces/IL2ToL2CrossDomainMessenger.sol"; import { ISemver } from "src/universal/interfaces/ISemver.sol"; import { Predeploys } from "src/libraries/Predeploys.sol"; -import { ERC20 } from "@solady/tokens/ERC20.sol"; -import { SuperchainERC20 } from "src/L2/SuperchainERC20.sol"; -import { Initializable } from "@openzeppelin/contracts-v5/proxy/utils/Initializable.sol"; import { ERC165 } from "@openzeppelin/contracts-v5/utils/introspection/ERC165.sol"; +import { ERC20Upgradeable } from "@openzeppelin/contracts-upgradeable-v5/token/ERC20/ERC20Upgradeable.sol"; /// @notice Thrown when attempting to mint or burn tokens and the function caller is not the StandardBridge. error OnlyBridge(); @@ -21,13 +19,7 @@ error OnlyBridge(); /// token, turning it fungible and interoperable across the superchain. Likewise, it also enables the inverse /// conversion path. /// Moreover, it builds on top of the L2ToL2CrossDomainMessenger for both replay protection and domain binding. -contract OptimismSuperchainERC20 is - IOptimismSuperchainERC20Extension, - SuperchainERC20, - ISemver, - Initializable, - ERC165 -{ +contract OptimismSuperchainERC20 is ERC20Upgradeable, ERC165, IOptimismSuperchainERC20Extension, ISemver { /// @notice Address of the StandardBridge Predeploy. address internal constant BRIDGE = Predeploys.L2_STANDARD_BRIDGE; diff --git a/packages/contracts-bedrock/src/L2/SuperchainERC20.sol b/packages/contracts-bedrock/src/L2/SuperchainERC20.sol index e20b375ff891..6f90e452e327 100644 --- a/packages/contracts-bedrock/src/L2/SuperchainERC20.sol +++ b/packages/contracts-bedrock/src/L2/SuperchainERC20.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.25; -import { ISuperchainERC20Extensions, ISuperchainERC20Errors } from "src/L2/interfaces/ISuperchainERC20.sol"; +import { ISuperchainERC20Extensions } from "src/L2/interfaces/ISuperchainERC20.sol"; import { ERC20 } from "@solady/tokens/ERC20.sol"; import { IL2ToL2CrossDomainMessenger } from "src/L2/interfaces/IL2ToL2CrossDomainMessenger.sol"; import { Predeploys } from "src/libraries/Predeploys.sol"; @@ -10,7 +10,7 @@ import { Predeploys } from "src/libraries/Predeploys.sol"; /// @notice SuperchainERC20 is a standard extension of the base ERC20 token contract that unifies ERC20 token /// bridging to make it fungible across the Superchain. It builds on top of the L2ToL2CrossDomainMessenger for /// both replay protection and domain binding. -abstract contract SuperchainERC20 is ISuperchainERC20Extensions, ISuperchainERC20Errors, ERC20 { +abstract contract SuperchainERC20 is ERC20, ISuperchainERC20Extensions { /// @notice Address of the L2ToL2CrossDomainMessenger Predeploy. address internal constant MESSENGER = Predeploys.L2_TO_L2_CROSS_DOMAIN_MESSENGER; diff --git a/packages/contracts-bedrock/src/L2/interfaces/IOptimismSuperchainERC20.sol b/packages/contracts-bedrock/src/L2/interfaces/IOptimismSuperchainERC20.sol index 5f537c1f51ec..d7b493e59bb7 100644 --- a/packages/contracts-bedrock/src/L2/interfaces/IOptimismSuperchainERC20.sol +++ b/packages/contracts-bedrock/src/L2/interfaces/IOptimismSuperchainERC20.sol @@ -1,15 +1,18 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.0; +pragma solidity 0.8.25; -// Interfaces -import { IERC20Solady } from "src/vendor/interfaces/IERC20Solady.sol"; -import { ISuperchainERC20Extensions, ISuperchainERC20Errors } from "src/L2/interfaces/ISuperchainERC20.sol"; +/// @title IOptimismSuperchainERC20Errors +/// @notice Interface containing the errors added in the OptimismSuperchainERC20 implementation. +interface IOptimismSuperchainERC20Errors { + /// @notice Thrown when attempting to perform an operation and the account is the zero address. + error ZeroAddress(); +} /// @title IOptimismSuperchainERC20Extension /// @notice This interface is available on the OptimismSuperchainERC20 contract. /// We declare it as a separate interface so that it can be used in /// custom implementations of SuperchainERC20. -interface IOptimismSuperchainERC20Extension is ISuperchainERC20Extensions, ISuperchainERC20Errors { +interface IOptimismSuperchainERC20Extension is IOptimismSuperchainERC20Errors { /// @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. @@ -33,7 +36,3 @@ interface IOptimismSuperchainERC20Extension is ISuperchainERC20Extensions, ISupe /// @notice Returns the address of the corresponding version of this token on the remote chain. function remoteToken() external view returns (address); } - -/// @title IOptimismSuperchainERC20 -/// @notice Combines Solady's ERC20 interface with the OptimismSuperchainERC20Extension interface. -interface IOptimismSuperchainERC20 is IERC20Solady, IOptimismSuperchainERC20Extension { } diff --git a/packages/contracts-bedrock/src/L2/interfaces/ISuperchainERC20.sol b/packages/contracts-bedrock/src/L2/interfaces/ISuperchainERC20.sol index fee6a2c2f7bd..700281467818 100644 --- a/packages/contracts-bedrock/src/L2/interfaces/ISuperchainERC20.sol +++ b/packages/contracts-bedrock/src/L2/interfaces/ISuperchainERC20.sol @@ -1,14 +1,26 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.0; -// Interfaces -import { IERC20Solady } from "src/vendor/interfaces/IERC20Solady.sol"; +/// @title ISuperchainERC20Errors +/// @notice Interface containing the errors added in the SuperchainERC20 implementation. +interface ISuperchainERC20Errors { + /// @notice Thrown when attempting to relay a message and the function caller (msg.sender) is not + /// L2ToL2CrossDomainMessenger. + error CallerNotL2ToL2CrossDomainMessenger(); + + /// @notice Thrown when attempting to relay a message and the cross domain message sender is not this + /// SuperchainERC20. + error InvalidCrossDomainSender(); + + /// @notice Thrown when attempting to perform an operation and the account is the zero address. + error ZeroAddress(); +} /// @title ISuperchainERC20Extensions /// @notice Interface for the extensions to the ERC20 standard that are used by SuperchainERC20. /// Exists in case developers are already importing the ERC20 interface separately and /// importing the full SuperchainERC20 interface would cause conflicting imports. -interface ISuperchainERC20Extensions { +interface ISuperchainERC20Extensions is ISuperchainERC20Errors { /// @notice Emitted when tokens are sent from one chain to another. /// @param from Address of the sender. /// @param to Address of the recipient. @@ -35,22 +47,3 @@ interface ISuperchainERC20Extensions { /// @param _amount Amount of tokens to relay. function relayERC20(address _from, address _to, uint256 _amount) external; } - -/// @title ISuperchainERC20Errors -/// @notice Interface containing the errors added in the SuperchainERC20 implementation. -interface ISuperchainERC20Errors { - /// @notice Thrown when attempting to relay a message and the function caller (msg.sender) is not - /// L2ToL2CrossDomainMessenger. - error CallerNotL2ToL2CrossDomainMessenger(); - - /// @notice Thrown when attempting to relay a message and the cross domain message sender is not this - /// SuperchainERC20. - error InvalidCrossDomainSender(); - - /// @notice Thrown when attempting to perform an operation and the account is the zero address. - error ZeroAddress(); -} - -/// @title ISuperchainERC20 -/// @notice Combines Solady's ERC20 interface with the SuperchainERC20Extensions interface. -interface ISuperchainERC20 is IERC20Solady, ISuperchainERC20Extensions, ISuperchainERC20Errors { } diff --git a/packages/contracts-bedrock/test/L2/OptimismSuperchainERC20.t.sol b/packages/contracts-bedrock/test/L2/OptimismSuperchainERC20.t.sol index b8de1468421b..3007ffaaa7e9 100644 --- a/packages/contracts-bedrock/test/L2/OptimismSuperchainERC20.t.sol +++ b/packages/contracts-bedrock/test/L2/OptimismSuperchainERC20.t.sol @@ -218,139 +218,6 @@ contract OptimismSuperchainERC20Test is Test { assertEq(superchainERC20.balanceOf(_from), _fromBalanceBefore - _amount); } - /// @notice Tests the `sendERC20` function reverts when the `_to` address is the zero address. - function testFuzz_sendERC20_zeroAddressTo_reverts(uint256 _amount, uint256 _chainId) public { - // Expect the revert with `ZeroAddress` selector - vm.expectRevert(ISuperchainERC20Errors.ZeroAddress.selector); - - // Call the `sendERC20` function with the zero address - vm.prank(BRIDGE); - superchainERC20.sendERC20({ _to: ZERO_ADDRESS, _amount: _amount, _chainId: _chainId }); - } - - /// @notice Tests the `sendERC20` function burns the sender tokens, sends the message, and emits the `SendERC20` - /// event. - function testFuzz_sendERC20_succeeds(address _sender, address _to, uint256 _amount, uint256 _chainId) external { - // Ensure `_sender` is not the zero address - vm.assume(_sender != ZERO_ADDRESS); - vm.assume(_to != ZERO_ADDRESS); - - // Mint some tokens to the sender so then they can be sent - vm.prank(BRIDGE); - superchainERC20.mint(_sender, _amount); - - // Get the total supply and balance of `_sender` before the send to compare later on the assertions - uint256 _totalSupplyBefore = superchainERC20.totalSupply(); - uint256 _senderBalanceBefore = superchainERC20.balanceOf(_sender); - - // Look for the emit of the `Transfer` event - vm.expectEmit(address(superchainERC20)); - emit IERC20.Transfer(_sender, ZERO_ADDRESS, _amount); - - // Look for the emit of the `SendERC20` event - vm.expectEmit(address(superchainERC20)); - emit ISuperchainERC20Extensions.SendERC20(_sender, _to, _amount, _chainId); - - // Mock the call over the `sendMessage` function and expect it to be called properly - bytes memory _message = abi.encodeCall(superchainERC20.relayERC20, (_sender, _to, _amount)); - _mockAndExpect( - MESSENGER, - abi.encodeWithSelector( - IL2ToL2CrossDomainMessenger.sendMessage.selector, _chainId, address(superchainERC20), _message - ), - abi.encode("") - ); - - // Call the `sendERC20` function - vm.prank(_sender); - superchainERC20.sendERC20(_to, _amount, _chainId); - - // Check the total supply and balance of `_sender` after the send were updated correctly - assertEq(superchainERC20.totalSupply(), _totalSupplyBefore - _amount); - assertEq(superchainERC20.balanceOf(_sender), _senderBalanceBefore - _amount); - } - - /// @notice Tests the `relayERC20` function reverts when the caller is not the L2ToL2CrossDomainMessenger. - function testFuzz_relayERC20_notMessenger_reverts(address _caller, address _to, uint256 _amount) public { - // Ensure the caller is not the messenger - vm.assume(_caller != MESSENGER); - vm.assume(_to != ZERO_ADDRESS); - - // Expect the revert with `CallerNotL2ToL2CrossDomainMessenger` selector - vm.expectRevert(ISuperchainERC20Errors.CallerNotL2ToL2CrossDomainMessenger.selector); - - // Call the `relayERC20` function with the non-messenger caller - vm.prank(_caller); - superchainERC20.relayERC20(_caller, _to, _amount); - } - - /// @notice Tests the `relayERC20` function reverts when the `crossDomainMessageSender` that sent the message is not - /// the same SuperchainERC20 address. - function testFuzz_relayERC20_notCrossDomainSender_reverts( - address _crossDomainMessageSender, - address _to, - uint256 _amount - ) - public - { - vm.assume(_to != ZERO_ADDRESS); - vm.assume(_crossDomainMessageSender != address(superchainERC20)); - - // Mock the call over the `crossDomainMessageSender` function setting a wrong sender - vm.mockCall( - MESSENGER, - abi.encodeWithSelector(IL2ToL2CrossDomainMessenger.crossDomainMessageSender.selector), - abi.encode(_crossDomainMessageSender) - ); - - // Expect the revert with `InvalidCrossDomainSender` selector - vm.expectRevert(ISuperchainERC20Errors.InvalidCrossDomainSender.selector); - - // Call the `relayERC20` function with the sender caller - vm.prank(MESSENGER); - superchainERC20.relayERC20(_crossDomainMessageSender, _to, _amount); - } - - /// @notice Tests the `relayERC20` mints the proper amount and emits the `RelayERC20` event. - function testFuzz_relayERC20_succeeds(address _from, address _to, uint256 _amount, uint256 _source) public { - vm.assume(_from != ZERO_ADDRESS); - vm.assume(_to != ZERO_ADDRESS); - - // Mock the call over the `crossDomainMessageSender` function setting the same address as value - _mockAndExpect( - MESSENGER, - abi.encodeWithSelector(IL2ToL2CrossDomainMessenger.crossDomainMessageSender.selector), - abi.encode(address(superchainERC20)) - ); - - // Mock the call over the `crossDomainMessageSource` function setting the source chain ID as value - _mockAndExpect( - MESSENGER, - abi.encodeWithSelector(IL2ToL2CrossDomainMessenger.crossDomainMessageSource.selector), - abi.encode(_source) - ); - - // Get the total supply and balance of `_to` before the relay to compare later on the assertions - uint256 _totalSupplyBefore = superchainERC20.totalSupply(); - uint256 _toBalanceBefore = superchainERC20.balanceOf(_to); - - // Look for the emit of the `Transfer` event - vm.expectEmit(address(superchainERC20)); - emit IERC20.Transfer(ZERO_ADDRESS, _to, _amount); - - // Look for the emit of the `RelayERC20` event - vm.expectEmit(address(superchainERC20)); - emit ISuperchainERC20Extensions.RelayERC20(_from, _to, _amount, _source); - - // Call the `relayERC20` function with the messenger caller - vm.prank(MESSENGER); - superchainERC20.relayERC20(_from, _to, _amount); - - // Check the total supply and balance of `_to` after the relay were updated correctly - assertEq(superchainERC20.totalSupply(), _totalSupplyBefore + _amount); - assertEq(superchainERC20.balanceOf(_to), _toBalanceBefore + _amount); - } - /// @notice Tests the `decimals` function always returns the correct value. function testFuzz_decimals_succeeds(uint8 _decimals) public { OptimismSuperchainERC20 _newSuperchainERC20 = _deploySuperchainERC20Proxy(REMOTE_TOKEN, NAME, SYMBOL, _decimals); diff --git a/packages/contracts-bedrock/test/invariants/OptimismSuperchainERC20/PROPERTIES.md b/packages/contracts-bedrock/test/invariants/OptimismSuperchainERC20/PROPERTIES.md index 18970855ca46..1c8c90bb0301 100644 --- a/packages/contracts-bedrock/test/invariants/OptimismSuperchainERC20/PROPERTIES.md +++ b/packages/contracts-bedrock/test/invariants/OptimismSuperchainERC20/PROPERTIES.md @@ -1,5 +1,9 @@ # Supertoken advanced testing +## Note + +This campaign will need to be updated the redesign `OptimismSuperchainERC20` redesign. Please delete this comment once the update is done. + ## Milestones The supertoken ecosystem consists of not just the supertoken contract, but the required changes to other contracts for liquidity to reach the former. @@ -12,8 +16,8 @@ Considering only the supertoken contract is merged into the `develop` branch, an ## Definitions -- *legacy token:* an OptimismMintableERC20 or L2StandardERC20 token on the suprechain that has either been deployed by the factory after the liquidity migration upgrade to the latter, or has been deployed before it **but** added to factory’s `deployments` mapping as part of the upgrade. This testing campaign is not concerned with tokens on L1 or not listed in the factory’s `deployments` mapping. -- *supertoken:* a SuperchainERC20 contract deployed by the `OptimismSuperchainERC20Factory` +- _legacy token:_ an OptimismMintableERC20 or L2StandardERC20 token on the suprechain that has either been deployed by the factory after the liquidity migration upgrade to the latter, or has been deployed before it **but** added to factory’s `deployments` mapping as part of the upgrade. This testing campaign is not concerned with tokens on L1 or not listed in the factory’s `deployments` mapping. +- _supertoken:_ a SuperchainERC20 contract deployed by the `OptimismSuperchainERC20Factory` # Ecosystem properties @@ -28,7 +32,7 @@ legend: ## Unit test | id | milestone | description | tested | -| --- | --- | --- | --- | +| --- | ------------------- | ------------------------------------------------------------------------------------------ | ------ | | 0 | Factories | supertoken token address does not depend on the executing chain’s chainID | [ ] | | 1 | Factories | supertoken token address depends on remote token, name, symbol and decimals | [ ] | | 2 | Liquidity Migration | convert() should only allow converting legacy tokens to supertoken and viceversa | [ ] | @@ -40,18 +44,18 @@ legend: ## Valid state | id | milestone | description | tested | -| --- | --- | --- | --- | -| 6 | SupERC20 | calls to sendERC20 succeed as long as caller has enough balance | [x] | -| 7 | SupERC20 | calls to relayERC20 always succeed as long as the cross-domain caller is valid | [~] | +| --- | --------- | ------------------------------------------------------------------------------ | ------ | +| 6 | SupERC20 | calls to sendERC20 succeed as long as caller has enough balance | [] | +| 7 | SupERC20 | calls to relayERC20 always succeed as long as the cross-domain caller is valid | [] | ## Variable transition | id | milestone | description | tested | -| --- | --- | --- | --- | -| 8 | SupERC20 | sendERC20 with a value of zero does not modify accounting | [x] | -| 9 | SupERC20 | relayERC20 with a value of zero does not modify accounting | [x] | -| 10 | SupERC20 | sendERC20 decreases the token's totalSupply in the source chain exactly by the input amount | [x] | -| 26 | SupERC20 | sendERC20 decreases the sender's balance in the source chain exactly by the input amount | [x] | +| --- | ------------------- | ------------------------------------------------------------------------------------------------- | ------ | +| 8 | SupERC20 | sendERC20 with a value of zero does not modify accounting | [] | +| 9 | SupERC20 | relayERC20 with a value of zero does not modify accounting | [] | +| 10 | SupERC20 | sendERC20 decreases the token's totalSupply in the source chain exactly by the input amount | [] | +| 26 | SupERC20 | sendERC20 decreases the sender's balance in the source chain exactly by the input amount | [] | | 27 | SupERC20 | relayERC20 increases sender's balance in the destination chain exactly by the input amount | [x] | | 11 | SupERC20 | relayERC20 increases the token's totalSupply in the destination chain exactly by the input amount | [ ] | | 12 | Liquidity Migration | supertoken total supply only increases on calls to mint() by the L2toL2StandardBridge | [~] | @@ -63,9 +67,9 @@ legend: ## High level | id | milestone | description | tested | -| --- | --- | --- | --- | -| 17 | Liquidity Migration | only calls to convert(legacy, super) can increase a supertoken’s total supply across chains | [ ] | -| 18 | Liquidity Migration | only calls to convert(super, legacy) can decrease a supertoken’s total supply across chains | [ ] | +| --- | ------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------ | +| 17 | Liquidity Migration | only calls to convert(legacy, super) can increase a supertoken’s total supply across chains | [ ] | +| 18 | Liquidity Migration | only calls to convert(super, legacy) can decrease a supertoken’s total supply across chains | [ ] | | 19 | Liquidity Migration | sum of supertoken total supply across all chains is always <= to convert(legacy, super)- convert(super, legacy) | [~] | | 20 | SupERC20 | tokens sendERC20-ed on a source chain to a destination chain can be relayERC20-ed on it as long as the source chain is in the dependency set of the destination chain | [ ] | | 21 | Liquidity Migration | sum of supertoken total supply across all chains is = to convert(legacy, super)- convert(super, legacy) when all cross-chain messages are processed | [~] | @@ -76,7 +80,7 @@ As another layer of defense, the following properties are defined which assume b It’s worth noting that these properties will not hold for a live system | id | milestone | description | tested | -| --- | --- | --- | --- | -| 22 | SupERC20 | sendERC20 decreases sender balance in source chain and increases receiver balance in destination chain exactly by the input amount | [x] | -| 23 | SupERC20 | sendERC20 decreases total supply in source chain and increases it in destination chain exactly by the input amount | [x] | +| --- | ------------------- | ---------------------------------------------------------------------------------------------------------------------------------- | ------ | +| 22 | SupERC20 | sendERC20 decreases sender balance in source chain and increases receiver balance in destination chain exactly by the input amount | [] | +| 23 | SupERC20 | sendERC20 decreases total supply in source chain and increases it in destination chain exactly by the input amount | [] | | 24 | Liquidity Migration | sum of supertoken total supply across all chains is always equal to convert(legacy, super)- convert(super, legacy) | [~] | diff --git a/packages/contracts-bedrock/test/invariants/OptimismSuperchainERC20/fuzz/Protocol.guided.t.sol b/packages/contracts-bedrock/test/invariants/OptimismSuperchainERC20/fuzz/Protocol.guided.t.sol index 536a4ea7025a..b1f2d8451240 100644 --- a/packages/contracts-bedrock/test/invariants/OptimismSuperchainERC20/fuzz/Protocol.guided.t.sol +++ b/packages/contracts-bedrock/test/invariants/OptimismSuperchainERC20/fuzz/Protocol.guided.t.sol @@ -32,100 +32,6 @@ contract ProtocolGuided is ProtocolHandler, CompatibleAssert { compatibleAssert(supertoken.totalSupply() == 0); } - /// @custom:property-id 6 - /// @custom:property calls to sendERC20 succeed as long as caller has enough balance - /// @custom:property-id 22 - /// @custom:property sendERC20 decreases sender balance in source chain and increases receiver balance in - /// destination chain exactly by the input amount - /// @custom:property-id 23 - /// @custom:property sendERC20 decreases total supply in source chain and increases it in destination chain exactly - /// by the input amount - function fuzz_bridgeSupertokenAtomic( - uint256 fromIndex, - uint256 recipientIndex, - uint256 destinationChainId, - uint256 amount - ) - public - withActor(msg.sender) - { - destinationChainId = bound(destinationChainId, 0, MAX_CHAINS - 1); - fromIndex = bound(fromIndex, 0, allSuperTokens.length - 1); - address recipient = getActorByRawIndex(recipientIndex); - OptimismSuperchainERC20 sourceToken = OptimismSuperchainERC20(allSuperTokens[fromIndex]); - OptimismSuperchainERC20 destinationToken = - MESSENGER.crossChainMessageReceiver(address(sourceToken), destinationChainId); - uint256 sourceBalanceBefore = sourceToken.balanceOf(currentActor()); - uint256 sourceSupplyBefore = sourceToken.totalSupply(); - uint256 destinationBalanceBefore = destinationToken.balanceOf(recipient); - uint256 destinationSupplyBefore = destinationToken.totalSupply(); - - MESSENGER.setAtomic(true); - vm.prank(currentActor()); - try sourceToken.sendERC20(recipient, amount, destinationChainId) { - MESSENGER.setAtomic(false); - uint256 sourceBalanceAfter = sourceToken.balanceOf(currentActor()); - uint256 destinationBalanceAfter = destinationToken.balanceOf(recipient); - // no free mint - compatibleAssert( - sourceBalanceBefore + destinationBalanceBefore == sourceBalanceAfter + destinationBalanceAfter - ); - // 22 - compatibleAssert(sourceBalanceBefore - amount == sourceBalanceAfter); - compatibleAssert(destinationBalanceBefore + amount == destinationBalanceAfter); - uint256 sourceSupplyAfter = sourceToken.totalSupply(); - uint256 destinationSupplyAfter = destinationToken.totalSupply(); - // 23 - compatibleAssert(sourceSupplyBefore - amount == sourceSupplyAfter); - compatibleAssert(destinationSupplyBefore + amount == destinationSupplyAfter); - } catch { - MESSENGER.setAtomic(false); - // 6 - compatibleAssert(address(destinationToken) == address(sourceToken) || sourceBalanceBefore < amount); - } - } - - /// @custom:property-id 6 - /// @custom:property calls to sendERC20 succeed as long as caller has enough balance - /// @custom:property-id 26 - /// @custom:property sendERC20 decreases sender balance in source chain exactly by the input amount - /// @custom:property-id 10 - /// @custom:property sendERC20 decreases total supply in source chain exactly by the input amount - function fuzz_sendERC20( - uint256 fromIndex, - uint256 recipientIndex, - uint256 destinationChainId, - uint256 amount - ) - public - withActor(msg.sender) - { - destinationChainId = bound(destinationChainId, 0, MAX_CHAINS - 1); - fromIndex = bound(fromIndex, 0, allSuperTokens.length - 1); - address recipient = getActorByRawIndex(recipientIndex); - OptimismSuperchainERC20 sourceToken = OptimismSuperchainERC20(allSuperTokens[fromIndex]); - OptimismSuperchainERC20 destinationToken = - MESSENGER.crossChainMessageReceiver(address(sourceToken), destinationChainId); - bytes32 deploySalt = MESSENGER.superTokenInitDeploySalts(address(sourceToken)); - uint256 sourceBalanceBefore = sourceToken.balanceOf(currentActor()); - uint256 sourceSupplyBefore = sourceToken.totalSupply(); - - vm.prank(currentActor()); - try sourceToken.sendERC20(recipient, amount, destinationChainId) { - (, uint256 currentlyInTransit) = ghost_tokensInTransit.tryGet(deploySalt); - ghost_tokensInTransit.set(deploySalt, currentlyInTransit + amount); - // 26 - uint256 sourceBalanceAfter = sourceToken.balanceOf(currentActor()); - compatibleAssert(sourceBalanceBefore - amount == sourceBalanceAfter); - // 10 - uint256 sourceSupplyAfter = sourceToken.totalSupply(); - compatibleAssert(sourceSupplyBefore - amount == sourceSupplyAfter); - } catch { - // 6 - compatibleAssert(address(destinationToken) == address(sourceToken) || sourceBalanceBefore < amount); - } - } - /// @custom:property-id 11 /// @custom:property relayERC20 increases the token's totalSupply in the destination chain exactly by the input /// amount @@ -156,56 +62,4 @@ contract ProtocolGuided is ProtocolHandler, CompatibleAssert { compatibleAssert(false); } } - - /// @custom:property-id 8 - /// @custom:property calls to sendERC20 with a value of zero dont modify accounting - // @notice is a subset of fuzz_sendERC20, so we'll just call it - // instead of re-implementing it. Keeping the function for visibility of the property. - function fuzz_sendZeroDoesNotModifyAccounting( - uint256 fromIndex, - uint256 recipientIndex, - uint256 destinationChainId - ) - external - { - fuzz_sendERC20(fromIndex, recipientIndex, destinationChainId, 0); - } - - /// @custom:property-id 9 - /// @custom:property calls to relayERC20 with a value of zero dont modify accounting - /// @custom:property-id 7 - /// @custom:property calls to relayERC20 always succeed as long as the cross-domain caller is valid - /// @notice cant call fuzz_RelayERC20 internally since that pops a - /// random message, which we cannot guarantee has a value of zero - function fuzz_relayZeroDoesNotModifyAccounting( - uint256 fromIndex, - uint256 recipientIndex - ) - external - withActor(msg.sender) - { - fromIndex = bound(fromIndex, 0, allSuperTokens.length - 1); - address recipient = getActorByRawIndex(recipientIndex); - OptimismSuperchainERC20 token = OptimismSuperchainERC20(allSuperTokens[fromIndex]); - uint256 balanceSenderBefore = token.balanceOf(currentActor()); - uint256 balanceRecipientBefore = token.balanceOf(recipient); - uint256 supplyBefore = token.totalSupply(); - - MESSENGER.setCrossDomainMessageSender(address(token)); - vm.prank(address(MESSENGER)); - try token.relayERC20(currentActor(), recipient, 0) { - MESSENGER.setCrossDomainMessageSender(address(0)); - } catch { - // should not revert because of 7, and if it *does* revert, I want the test suite - // to discard the sequence instead of potentially getting another - // error due to the crossDomainMessageSender being manually set - compatibleAssert(false); - } - uint256 balanceSenderAfter = token.balanceOf(currentActor()); - uint256 balanceRecipeintAfter = token.balanceOf(recipient); - uint256 supplyAfter = token.totalSupply(); - compatibleAssert(balanceSenderBefore == balanceSenderAfter); - compatibleAssert(balanceRecipientBefore == balanceRecipeintAfter); - compatibleAssert(supplyBefore == supplyAfter); - } } diff --git a/packages/contracts-bedrock/test/invariants/OptimismSuperchainERC20/fuzz/Protocol.unguided.t.sol b/packages/contracts-bedrock/test/invariants/OptimismSuperchainERC20/fuzz/Protocol.unguided.t.sol index 90cad38baa99..ccd9b91d9994 100644 --- a/packages/contracts-bedrock/test/invariants/OptimismSuperchainERC20/fuzz/Protocol.unguided.t.sol +++ b/packages/contracts-bedrock/test/invariants/OptimismSuperchainERC20/fuzz/Protocol.unguided.t.sol @@ -10,78 +10,6 @@ import { CompatibleAssert } from "../helpers/CompatibleAssert.t.sol"; contract ProtocolUnguided is ProtocolHandler, CompatibleAssert { using EnumerableMap for EnumerableMap.Bytes32ToUintMap; - /// @custom:property-id 7 - /// @custom:property calls to relayERC20 always succeed as long as the cross-domain caller is valid - /// @notice this ensures actors cant simply call relayERC20 and get tokens, no matter the system state - /// but there's still some possible work on how hard we can bork the system state with handlers calling - /// the L2ToL2CrossDomainMessenger or bridge directly (pending on non-atomic bridging) - function fuzz_relayERC20( - uint256 tokenIndex, - address sender, - address crossDomainMessageSender, - address recipient, - uint256 amount - ) - external - { - MESSENGER.setCrossDomainMessageSender(crossDomainMessageSender); - address token = allSuperTokens[bound(tokenIndex, 0, allSuperTokens.length)]; - vm.prank(sender); - try OptimismSuperchainERC20(token).relayERC20(sender, recipient, amount) { - MESSENGER.setCrossDomainMessageSender(address(0)); - compatibleAssert(sender == address(MESSENGER)); - compatibleAssert(crossDomainMessageSender == token); - // this increases the supply across chains without a call to - // `mint` by the MESSENGER, so it kind of breaks an invariant, but - // let's walk around that: - bytes32 salt = MESSENGER.superTokenInitDeploySalts(token); - (, uint256 currentValue) = ghost_totalSupplyAcrossChains.tryGet(salt); - ghost_totalSupplyAcrossChains.set(salt, currentValue + amount); - } catch { - compatibleAssert(sender != address(MESSENGER) || crossDomainMessageSender != token); - MESSENGER.setCrossDomainMessageSender(address(0)); - } - } - - /// @custom:property-id 6 - /// @custom:property calls to sendERC20 succeed as long as caller has enough balance - /// @custom:property-id 26 - /// @custom:property sendERC20 decreases sender balance in source chain exactly by the input amount - /// @custom:property-id 10 - /// @custom:property sendERC20 decreases total supply in source chain exactly by the input amount - function fuzz_sendERC20( - address sender, - address recipient, - uint256 fromIndex, - uint256 destinationChainId, - uint256 amount - ) - public - { - destinationChainId = bound(destinationChainId, 0, MAX_CHAINS - 1); - OptimismSuperchainERC20 sourceToken = OptimismSuperchainERC20(allSuperTokens[fromIndex]); - OptimismSuperchainERC20 destinationToken = - MESSENGER.crossChainMessageReceiver(address(sourceToken), destinationChainId); - bytes32 deploySalt = MESSENGER.superTokenInitDeploySalts(address(sourceToken)); - uint256 sourceBalanceBefore = sourceToken.balanceOf(sender); - uint256 sourceSupplyBefore = sourceToken.totalSupply(); - - vm.prank(sender); - try sourceToken.sendERC20(recipient, amount, destinationChainId) { - (, uint256 currentlyInTransit) = ghost_tokensInTransit.tryGet(deploySalt); - ghost_tokensInTransit.set(deploySalt, currentlyInTransit + amount); - // 26 - uint256 sourceBalanceAfter = sourceToken.balanceOf(sender); - compatibleAssert(sourceBalanceBefore - amount == sourceBalanceAfter); - // 10 - uint256 sourceSupplyAfter = sourceToken.totalSupply(); - compatibleAssert(sourceSupplyBefore - amount == sourceSupplyAfter); - } catch { - // 6 - compatibleAssert(address(destinationToken) == address(sourceToken) || sourceBalanceBefore < amount); - } - } - /// @custom:property-id 12 /// @custom:property supertoken total supply only increases on calls to mint() by the L2toL2StandardBridge function fuzz_mint(uint256 tokenIndex, address to, address sender, uint256 amount) external { From ad4691443c283d2f32b60e06a0c8a6c300751606 Mon Sep 17 00:00:00 2001 From: 0xDiscotech <131301107+0xDiscotech@users.noreply.github.com> Date: Thu, 26 Sep 2024 13:06:13 -0300 Subject: [PATCH 2/5] refactor: remove op as dependency * feat: add check for supererc20 bridge on modifier * chore: update tests and interfaces --- packages/contracts-bedrock/.gitmodules | 0 packages/contracts-bedrock/foundry.toml | 2 - packages/contracts-bedrock/semver-lock.json | 4 +- .../abi/OptimismSuperchainERC20.json | 120 +----------------- .../snapshots/abi/SuperchainWETH.json | 15 +++ .../src/L2/OptimismSuperchainERC20.sol | 25 ++-- .../interfaces/IOptimismSuperchainERC20.sol | 13 +- .../src/L2/interfaces/ISuperchainERC20.sol | 7 + .../test/L2/OptimismSuperchainERC20.t.sol | 58 +++++---- 9 files changed, 82 insertions(+), 162 deletions(-) create mode 100644 packages/contracts-bedrock/.gitmodules diff --git a/packages/contracts-bedrock/.gitmodules b/packages/contracts-bedrock/.gitmodules new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/packages/contracts-bedrock/foundry.toml b/packages/contracts-bedrock/foundry.toml index fc5cc2360b3a..cef9f85bbaeb 100644 --- a/packages/contracts-bedrock/foundry.toml +++ b/packages/contracts-bedrock/foundry.toml @@ -12,8 +12,6 @@ optimizer = true optimizer_runs = 999999 remappings = [ '@openzeppelin/contracts-upgradeable/=lib/openzeppelin-contracts-upgradeable/contracts', - '@openzeppelin/contracts-upgradeable-v5/=lib/openzeppelin-contracts-upgradeable-v5/contracts', - 'openzeppelin-contracts-upgradeable-v5/@openzeppelin/contracts/=lib/openzeppelin-contracts-v5/contracts', # Package-specific '@openzeppelin/contracts/=lib/openzeppelin-contracts/contracts', '@openzeppelin/contracts-v5/=lib/openzeppelin-contracts-v5/contracts', '@rari-capital/solmate/=lib/solmate', diff --git a/packages/contracts-bedrock/semver-lock.json b/packages/contracts-bedrock/semver-lock.json index 5a2dd78f72e6..fd8a13bc6f3c 100644 --- a/packages/contracts-bedrock/semver-lock.json +++ b/packages/contracts-bedrock/semver-lock.json @@ -116,8 +116,8 @@ "sourceCodeHash": "0x4b806cc85cead74c8df34ab08f4b6c6a95a1a387a335ec8a7cb2de4ea4e1cf41" }, "src/L2/OptimismSuperchainERC20.sol": { - "initCodeHash": "0x4fd71b5352b78d51d39625b6defa77a75be53067b32f3cba86bd17a46917adf9", - "sourceCodeHash": "0xad3934ea533544b3c130c80be26201354af85f9166cb2ce54d96e5e383ebb5c1" + "initCodeHash": "0x9b48e5b8271d9b5d4407bf1a7327842844b4bd858123ffa6da0c55ad9d3b9d8e", + "sourceCodeHash": "0xaf5702655736e9c4b480a0bb4f0f2220f09834b2292f8e5419b5b245e0a20300" }, "src/L2/OptimismSuperchainERC20Beacon.sol": { "initCodeHash": "0x99ce8095b23c124850d866cbc144fee6cee05dbc6bb5d83acadfe00b90cf42c7", diff --git a/packages/contracts-bedrock/snapshots/abi/OptimismSuperchainERC20.json b/packages/contracts-bedrock/snapshots/abi/OptimismSuperchainERC20.json index 6eb57764a8cb..62dfe3ea6191 100644 --- a/packages/contracts-bedrock/snapshots/abi/OptimismSuperchainERC20.json +++ b/packages/contracts-bedrock/snapshots/abi/OptimismSuperchainERC20.json @@ -236,29 +236,6 @@ "stateMutability": "nonpayable", "type": "function" }, - { - "inputs": [ - { - "internalType": "address", - "name": "_from", - "type": "address" - }, - { - "internalType": "address", - "name": "_to", - "type": "address" - }, - { - "internalType": "uint256", - "name": "_amount", - "type": "uint256" - } - ], - "name": "relayERC20", - "outputs": [], - "stateMutability": "nonpayable", - "type": "function" - }, { "inputs": [], "name": "remoteToken", @@ -272,29 +249,6 @@ "stateMutability": "view", "type": "function" }, - { - "inputs": [ - { - "internalType": "address", - "name": "_to", - "type": "address" - }, - { - "internalType": "uint256", - "name": "_amount", - "type": "uint256" - }, - { - "internalType": "uint256", - "name": "_chainId", - "type": "uint256" - } - ], - "name": "sendERC20", - "outputs": [], - "stateMutability": "nonpayable", - "type": "function" - }, { "inputs": [ { @@ -482,68 +436,6 @@ "name": "Mint", "type": "event" }, - { - "anonymous": false, - "inputs": [ - { - "indexed": true, - "internalType": "address", - "name": "from", - "type": "address" - }, - { - "indexed": true, - "internalType": "address", - "name": "to", - "type": "address" - }, - { - "indexed": false, - "internalType": "uint256", - "name": "amount", - "type": "uint256" - }, - { - "indexed": false, - "internalType": "uint256", - "name": "source", - "type": "uint256" - } - ], - "name": "RelayERC20", - "type": "event" - }, - { - "anonymous": false, - "inputs": [ - { - "indexed": true, - "internalType": "address", - "name": "from", - "type": "address" - }, - { - "indexed": true, - "internalType": "address", - "name": "to", - "type": "address" - }, - { - "indexed": false, - "internalType": "uint256", - "name": "amount", - "type": "uint256" - }, - { - "indexed": false, - "internalType": "uint256", - "name": "destination", - "type": "uint256" - } - ], - "name": "SendERC20", - "type": "event" - }, { "anonymous": false, "inputs": [ @@ -579,11 +471,6 @@ "name": "AllowanceUnderflow", "type": "error" }, - { - "inputs": [], - "name": "CallerNotL2ToL2CrossDomainMessenger", - "type": "error" - }, { "inputs": [], "name": "InsufficientAllowance", @@ -594,11 +481,6 @@ "name": "InsufficientBalance", "type": "error" }, - { - "inputs": [], - "name": "InvalidCrossDomainSender", - "type": "error" - }, { "inputs": [], "name": "InvalidInitialization", @@ -616,7 +498,7 @@ }, { "inputs": [], - "name": "OnlyBridge", + "name": "OnlyAuthorizedBridge", "type": "error" }, { diff --git a/packages/contracts-bedrock/snapshots/abi/SuperchainWETH.json b/packages/contracts-bedrock/snapshots/abi/SuperchainWETH.json index 600e0e6b64f7..36d1aa36f8f8 100644 --- a/packages/contracts-bedrock/snapshots/abi/SuperchainWETH.json +++ b/packages/contracts-bedrock/snapshots/abi/SuperchainWETH.json @@ -408,6 +408,16 @@ "name": "Withdrawal", "type": "event" }, + { + "inputs": [], + "name": "CallerNotL2ToL2CrossDomainMessenger", + "type": "error" + }, + { + "inputs": [], + "name": "InvalidCrossDomainSender", + "type": "error" + }, { "inputs": [], "name": "NotCustomGasToken", @@ -417,5 +427,10 @@ "inputs": [], "name": "Unauthorized", "type": "error" + }, + { + "inputs": [], + "name": "ZeroAddress", + "type": "error" } ] \ No newline at end of file diff --git a/packages/contracts-bedrock/src/L2/OptimismSuperchainERC20.sol b/packages/contracts-bedrock/src/L2/OptimismSuperchainERC20.sol index b89390922172..5ba5059abb42 100644 --- a/packages/contracts-bedrock/src/L2/OptimismSuperchainERC20.sol +++ b/packages/contracts-bedrock/src/L2/OptimismSuperchainERC20.sol @@ -6,10 +6,8 @@ import { IL2ToL2CrossDomainMessenger } from "src/L2/interfaces/IL2ToL2CrossDomai import { ISemver } from "src/universal/interfaces/ISemver.sol"; import { Predeploys } from "src/libraries/Predeploys.sol"; import { ERC165 } from "@openzeppelin/contracts-v5/utils/introspection/ERC165.sol"; -import { ERC20Upgradeable } from "@openzeppelin/contracts-upgradeable-v5/token/ERC20/ERC20Upgradeable.sol"; - -/// @notice Thrown when attempting to mint or burn tokens and the function caller is not the StandardBridge. -error OnlyBridge(); +import { ERC20 } from "@solady/tokens/ERC20.sol"; +import { Initializable } from "@openzeppelin/contracts-v5/proxy/utils/Initializable.sol"; /// @custom:proxied true /// @title OptimismSuperchainERC20 @@ -19,10 +17,7 @@ error OnlyBridge(); /// token, turning it fungible and interoperable across the superchain. Likewise, it also enables the inverse /// conversion path. /// Moreover, it builds on top of the L2ToL2CrossDomainMessenger for both replay protection and domain binding. -contract OptimismSuperchainERC20 is ERC20Upgradeable, ERC165, IOptimismSuperchainERC20Extension, ISemver { - /// @notice Address of the StandardBridge Predeploy. - address internal constant BRIDGE = Predeploys.L2_STANDARD_BRIDGE; - +contract OptimismSuperchainERC20 is ERC20, Initializable, ERC165, IOptimismSuperchainERC20Extension, ISemver { /// @notice Storage slot that the OptimismSuperchainERC20Metadata struct is stored at. /// keccak256(abi.encode(uint256(keccak256("optimismSuperchainERC20.metadata")) - 1)) & ~bytes32(uint256(0xff)); bytes32 internal constant OPTIMISM_SUPERCHAIN_ERC20_METADATA_SLOT = @@ -49,14 +44,16 @@ contract OptimismSuperchainERC20 is ERC20Upgradeable, ERC165, IOptimismSuperchai } /// @notice A modifier that only allows the bridge to call - modifier onlyBridge() { - if (msg.sender != BRIDGE) revert OnlyBridge(); + modifier onlyAuthorizedBridge() { + if (msg.sender != Predeploys.L2_STANDARD_BRIDGE && msg.sender != Predeploys.SUPERCHAIN_ERC20_BRIDGE) { + revert OnlyAuthorizedBridge(); + } _; } /// @notice Semantic version. - /// @custom:semver 1.0.0-beta.2 - string public constant version = "1.0.0-beta.2"; + /// @custom:semver 1.0.0-beta.3 + string public constant version = "1.0.0-beta.3"; /// @notice Constructs the OptimismSuperchainERC20 contract. constructor() { @@ -87,7 +84,7 @@ contract OptimismSuperchainERC20 is ERC20Upgradeable, ERC165, IOptimismSuperchai /// @notice Allows the L2StandardBridge to mint tokens. /// @param _to Address to mint tokens to. /// @param _amount Amount of tokens to mint. - function mint(address _to, uint256 _amount) external virtual onlyBridge { + function mint(address _to, uint256 _amount) external virtual onlyAuthorizedBridge { if (_to == address(0)) revert ZeroAddress(); _mint(_to, _amount); @@ -98,7 +95,7 @@ contract OptimismSuperchainERC20 is ERC20Upgradeable, ERC165, IOptimismSuperchai /// @notice Allows the L2StandardBridge to burn tokens. /// @param _from Address to burn tokens from. /// @param _amount Amount of tokens to burn. - function burn(address _from, uint256 _amount) external virtual onlyBridge { + function burn(address _from, uint256 _amount) external virtual onlyAuthorizedBridge { if (_from == address(0)) revert ZeroAddress(); _burn(_from, _amount); diff --git a/packages/contracts-bedrock/src/L2/interfaces/IOptimismSuperchainERC20.sol b/packages/contracts-bedrock/src/L2/interfaces/IOptimismSuperchainERC20.sol index d7b493e59bb7..2417794d0886 100644 --- a/packages/contracts-bedrock/src/L2/interfaces/IOptimismSuperchainERC20.sol +++ b/packages/contracts-bedrock/src/L2/interfaces/IOptimismSuperchainERC20.sol @@ -1,11 +1,18 @@ // SPDX-License-Identifier: MIT -pragma solidity 0.8.25; +pragma solidity ^0.8.0; + +// Interfaces +import { IERC20Solady } from "src/vendor/interfaces/IERC20Solady.sol"; /// @title IOptimismSuperchainERC20Errors /// @notice Interface containing the errors added in the OptimismSuperchainERC20 implementation. interface IOptimismSuperchainERC20Errors { /// @notice Thrown when attempting to perform an operation and the account is the zero address. error ZeroAddress(); + + /// @notice Thrown when attempting to mint or burn tokens and the function caller is not the StandardBridge or the + /// SuperchainERC20Bridge. + error OnlyAuthorizedBridge(); } /// @title IOptimismSuperchainERC20Extension @@ -36,3 +43,7 @@ interface IOptimismSuperchainERC20Extension is IOptimismSuperchainERC20Errors { /// @notice Returns the address of the corresponding version of this token on the remote chain. function remoteToken() external view returns (address); } + +/// @title IOptimismSuperchainERC20 +/// @notice Combines Solady's ERC20 interface with the OptimismSuperchainERC20Extension interface. +interface IOptimismSuperchainERC20 is IERC20Solady, IOptimismSuperchainERC20Extension { } diff --git a/packages/contracts-bedrock/src/L2/interfaces/ISuperchainERC20.sol b/packages/contracts-bedrock/src/L2/interfaces/ISuperchainERC20.sol index 700281467818..a4396e88affa 100644 --- a/packages/contracts-bedrock/src/L2/interfaces/ISuperchainERC20.sol +++ b/packages/contracts-bedrock/src/L2/interfaces/ISuperchainERC20.sol @@ -1,6 +1,9 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.0; +// Interfaces +import { IERC20Solady } from "src/vendor/interfaces/IERC20Solady.sol"; + /// @title ISuperchainERC20Errors /// @notice Interface containing the errors added in the SuperchainERC20 implementation. interface ISuperchainERC20Errors { @@ -47,3 +50,7 @@ interface ISuperchainERC20Extensions is ISuperchainERC20Errors { /// @param _amount Amount of tokens to relay. function relayERC20(address _from, address _to, uint256 _amount) external; } + +/// @title ISuperchainERC20 +/// @notice Combines Solady's ERC20 interface with the SuperchainERC20Extensions interface. +interface ISuperchainERC20 is IERC20Solady, ISuperchainERC20Extensions { } diff --git a/packages/contracts-bedrock/test/L2/OptimismSuperchainERC20.t.sol b/packages/contracts-bedrock/test/L2/OptimismSuperchainERC20.t.sol index 3007ffaaa7e9..2ef390e213f9 100644 --- a/packages/contracts-bedrock/test/L2/OptimismSuperchainERC20.t.sol +++ b/packages/contracts-bedrock/test/L2/OptimismSuperchainERC20.t.sol @@ -16,12 +16,8 @@ import { IBeacon } from "@openzeppelin/contracts-v5/proxy/beacon/IBeacon.sol"; import { BeaconProxy } from "@openzeppelin/contracts-v5/proxy/beacon/BeaconProxy.sol"; // Target contract -import { - OptimismSuperchainERC20, IOptimismSuperchainERC20Extension, OnlyBridge -} from "src/L2/OptimismSuperchainERC20.sol"; - -// SuperchainERC20 Interfaces -import { ISuperchainERC20Extensions, ISuperchainERC20Errors } from "src/L2/interfaces/ISuperchainERC20.sol"; +import { OptimismSuperchainERC20, IOptimismSuperchainERC20Extension } from "src/L2/OptimismSuperchainERC20.sol"; +import { IOptimismSuperchainERC20Errors } from "src/L2/interfaces/IOptimismSuperchainERC20.sol"; /// @title OptimismSuperchainERC20Test /// @notice Contract for testing the OptimismSuperchainERC20 contract. @@ -31,7 +27,8 @@ contract OptimismSuperchainERC20Test is Test { string internal constant NAME = "OptimismSuperchainERC20"; string internal constant SYMBOL = "SCE"; uint8 internal constant DECIMALS = 18; - address internal constant BRIDGE = Predeploys.L2_STANDARD_BRIDGE; + address internal constant L2_BRIDGE = Predeploys.L2_STANDARD_BRIDGE; + address internal constant SUPERCHAIN_ERC20_BRIDGE = Predeploys.SUPERCHAIN_ERC20_BRIDGE; address internal constant MESSENGER = Predeploys.L2_TO_L2_CROSS_DOMAIN_MESSENGER; OptimismSuperchainERC20 public superchainERC20Impl; @@ -86,6 +83,12 @@ contract OptimismSuperchainERC20Test is Test { ); } + /// @notice Helper function to fuzz the bridge address to performs the calls with. + /// @dev Needed to cover both possible branches of the authorized callers on `mint` and `burn` functions. + function _getBridge(bool _returnL2StandardBridge) internal pure returns (address _bridge) { + _bridge = _returnL2StandardBridge ? L2_BRIDGE : SUPERCHAIN_ERC20_BRIDGE; + } + /// @notice Helper function to setup a mock and expect a call to it. function _mockAndExpect(address _receiver, bytes memory _calldata, bytes memory _returned) internal { vm.mockCall(_receiver, _calldata, _returned); @@ -119,10 +122,11 @@ contract OptimismSuperchainERC20Test is Test { /// @notice Tests the `mint` function reverts when the caller is not the bridge. function testFuzz_mint_callerNotBridge_reverts(address _caller, address _to, uint256 _amount) public { // Ensure the caller is not the bridge - vm.assume(_caller != BRIDGE); + vm.assume(_caller != L2_BRIDGE); + vm.assume(_caller != SUPERCHAIN_ERC20_BRIDGE); - // Expect the revert with `OnlyBridge` selector - vm.expectRevert(OnlyBridge.selector); + // Expect the revert with `OnlyAuthorizedBridge` selector + vm.expectRevert(IOptimismSuperchainERC20Errors.OnlyAuthorizedBridge.selector); // Call the `mint` function with the non-bridge caller vm.prank(_caller); @@ -130,17 +134,18 @@ contract OptimismSuperchainERC20Test is Test { } /// @notice Tests the `mint` function reverts when the amount is zero. - function testFuzz_mint_zeroAddressTo_reverts(uint256 _amount) public { + function testFuzz_mint_zeroAddressTo_reverts(uint256 _amount, bool _returnL2StandardBridge) public { // Expect the revert with `ZeroAddress` selector - vm.expectRevert(ISuperchainERC20Errors.ZeroAddress.selector); + vm.expectRevert(IOptimismSuperchainERC20Errors.ZeroAddress.selector); // Call the `mint` function with the zero address - vm.prank(BRIDGE); + address _bridge = _getBridge(_returnL2StandardBridge); + vm.prank(_bridge); superchainERC20.mint({ _to: ZERO_ADDRESS, _amount: _amount }); } /// @notice Tests the `mint` succeeds and emits the `Mint` event. - function testFuzz_mint_succeeds(address _to, uint256 _amount) public { + function testFuzz_mint_succeeds(address _to, uint256 _amount, bool _returnL2StandardBridge) public { // Ensure `_to` is not the zero address vm.assume(_to != ZERO_ADDRESS); @@ -157,7 +162,8 @@ contract OptimismSuperchainERC20Test is Test { emit IOptimismSuperchainERC20Extension.Mint(_to, _amount); // Call the `mint` function with the bridge caller - vm.prank(BRIDGE); + address _bridge = _getBridge(_returnL2StandardBridge); + vm.prank(_bridge); superchainERC20.mint(_to, _amount); // Check the total supply and balance of `_to` after the mint were updated correctly @@ -168,10 +174,11 @@ contract OptimismSuperchainERC20Test is Test { /// @notice Tests the `burn` function reverts when the caller is not the bridge. function testFuzz_burn_callerNotBridge_reverts(address _caller, address _from, uint256 _amount) public { // Ensure the caller is not the bridge - vm.assume(_caller != BRIDGE); + vm.assume(_caller != L2_BRIDGE); + vm.assume(_caller != SUPERCHAIN_ERC20_BRIDGE); - // Expect the revert with `OnlyBridge` selector - vm.expectRevert(OnlyBridge.selector); + // Expect the revert with `OnlyAuthorizedBridge` selector + vm.expectRevert(IOptimismSuperchainERC20Errors.OnlyAuthorizedBridge.selector); // Call the `burn` function with the non-bridge caller vm.prank(_caller); @@ -179,22 +186,24 @@ contract OptimismSuperchainERC20Test is Test { } /// @notice Tests the `burn` function reverts when the amount is zero. - function testFuzz_burn_zeroAddressFrom_reverts(uint256 _amount) public { + function testFuzz_burn_zeroAddressFrom_reverts(uint256 _amount, bool _returnL2StandardBridge) public { // Expect the revert with `ZeroAddress` selector - vm.expectRevert(ISuperchainERC20Errors.ZeroAddress.selector); + vm.expectRevert(IOptimismSuperchainERC20Errors.ZeroAddress.selector); // Call the `burn` function with the zero address - vm.prank(BRIDGE); + address _bridge = _getBridge(_returnL2StandardBridge); + vm.prank(_bridge); superchainERC20.burn({ _from: ZERO_ADDRESS, _amount: _amount }); } /// @notice Tests the `burn` burns the amount and emits the `Burn` event. - function testFuzz_burn_succeeds(address _from, uint256 _amount) public { + function testFuzz_burn_succeeds(address _from, uint256 _amount, bool _returnL2StandardBridge) public { // Ensure `_from` is not the zero address vm.assume(_from != ZERO_ADDRESS); // Mint some tokens to `_from` so then they can be burned - vm.prank(BRIDGE); + address _bridge = _getBridge(_returnL2StandardBridge); + vm.prank(_bridge); superchainERC20.mint(_from, _amount); // Get the total supply and balance of `_from` before the burn to compare later on the assertions @@ -210,7 +219,8 @@ contract OptimismSuperchainERC20Test is Test { emit IOptimismSuperchainERC20Extension.Burn(_from, _amount); // Call the `burn` function with the bridge caller - vm.prank(BRIDGE); + _bridge = _getBridge(_returnL2StandardBridge); + vm.prank(_bridge); superchainERC20.burn(_from, _amount); // Check the total supply and balance of `_from` after the burn were updated correctly From aa4c505936f545270e23f6cddcaa279404bba97f Mon Sep 17 00:00:00 2001 From: 0xDiscotech <131301107+0xDiscotech@users.noreply.github.com> Date: Thu, 26 Sep 2024 13:28:20 -0300 Subject: [PATCH 3/5] chore: update stack vars name on test --- .../test/L2/OptimismSuperchainERC20.t.sol | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/packages/contracts-bedrock/test/L2/OptimismSuperchainERC20.t.sol b/packages/contracts-bedrock/test/L2/OptimismSuperchainERC20.t.sol index 2ef390e213f9..1931328bce4c 100644 --- a/packages/contracts-bedrock/test/L2/OptimismSuperchainERC20.t.sol +++ b/packages/contracts-bedrock/test/L2/OptimismSuperchainERC20.t.sol @@ -85,8 +85,8 @@ contract OptimismSuperchainERC20Test is Test { /// @notice Helper function to fuzz the bridge address to performs the calls with. /// @dev Needed to cover both possible branches of the authorized callers on `mint` and `burn` functions. - function _getBridge(bool _returnL2StandardBridge) internal pure returns (address _bridge) { - _bridge = _returnL2StandardBridge ? L2_BRIDGE : SUPERCHAIN_ERC20_BRIDGE; + function _getBridge(bool _returnL2StandardBridge) internal pure returns (address bridge) { + bridge = _returnL2StandardBridge ? L2_BRIDGE : SUPERCHAIN_ERC20_BRIDGE; } /// @notice Helper function to setup a mock and expect a call to it. @@ -139,8 +139,8 @@ contract OptimismSuperchainERC20Test is Test { vm.expectRevert(IOptimismSuperchainERC20Errors.ZeroAddress.selector); // Call the `mint` function with the zero address - address _bridge = _getBridge(_returnL2StandardBridge); - vm.prank(_bridge); + address bridge = _getBridge(_returnL2StandardBridge); + vm.prank(bridge); superchainERC20.mint({ _to: ZERO_ADDRESS, _amount: _amount }); } @@ -162,8 +162,8 @@ contract OptimismSuperchainERC20Test is Test { emit IOptimismSuperchainERC20Extension.Mint(_to, _amount); // Call the `mint` function with the bridge caller - address _bridge = _getBridge(_returnL2StandardBridge); - vm.prank(_bridge); + address bridge = _getBridge(_returnL2StandardBridge); + vm.prank(bridge); superchainERC20.mint(_to, _amount); // Check the total supply and balance of `_to` after the mint were updated correctly @@ -191,8 +191,8 @@ contract OptimismSuperchainERC20Test is Test { vm.expectRevert(IOptimismSuperchainERC20Errors.ZeroAddress.selector); // Call the `burn` function with the zero address - address _bridge = _getBridge(_returnL2StandardBridge); - vm.prank(_bridge); + address bridge = _getBridge(_returnL2StandardBridge); + vm.prank(bridge); superchainERC20.burn({ _from: ZERO_ADDRESS, _amount: _amount }); } @@ -202,8 +202,8 @@ contract OptimismSuperchainERC20Test is Test { vm.assume(_from != ZERO_ADDRESS); // Mint some tokens to `_from` so then they can be burned - address _bridge = _getBridge(_returnL2StandardBridge); - vm.prank(_bridge); + address bridge = _getBridge(_returnL2StandardBridge); + vm.prank(bridge); superchainERC20.mint(_from, _amount); // Get the total supply and balance of `_from` before the burn to compare later on the assertions @@ -219,8 +219,8 @@ contract OptimismSuperchainERC20Test is Test { emit IOptimismSuperchainERC20Extension.Burn(_from, _amount); // Call the `burn` function with the bridge caller - _bridge = _getBridge(_returnL2StandardBridge); - vm.prank(_bridge); + bridge = _getBridge(_returnL2StandardBridge); + vm.prank(bridge); superchainERC20.burn(_from, _amount); // Check the total supply and balance of `_from` after the burn were updated correctly From 431436920d8224fb7544b55dee46dc1f93fbc96a Mon Sep 17 00:00:00 2001 From: 0xDiscotech <131301107+0xDiscotech@users.noreply.github.com> Date: Thu, 26 Sep 2024 13:45:54 -0300 Subject: [PATCH 4/5] chore: remove empty gitmodules file --- packages/contracts-bedrock/.gitmodules | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 packages/contracts-bedrock/.gitmodules diff --git a/packages/contracts-bedrock/.gitmodules b/packages/contracts-bedrock/.gitmodules deleted file mode 100644 index e69de29bb2d1..000000000000 From 9ef170d78a3dad03c0e3f7997f4e1599a0d00792 Mon Sep 17 00:00:00 2001 From: 0xDiscotech <131301107+0xDiscotech@users.noreply.github.com> Date: Thu, 26 Sep 2024 14:39:08 -0300 Subject: [PATCH 5/5] chore: update superchain weth errors --- packages/contracts-bedrock/semver-lock.json | 4 ++-- .../contracts-bedrock/snapshots/abi/SuperchainWETH.json | 5 ----- packages/contracts-bedrock/src/L2/SuperchainWETH.sol | 8 ++++---- .../src/L2/interfaces/ISuperchainERC20.sol | 3 +-- 4 files changed, 7 insertions(+), 13 deletions(-) diff --git a/packages/contracts-bedrock/semver-lock.json b/packages/contracts-bedrock/semver-lock.json index fd8a13bc6f3c..c5698dfb798b 100644 --- a/packages/contracts-bedrock/semver-lock.json +++ b/packages/contracts-bedrock/semver-lock.json @@ -136,8 +136,8 @@ "sourceCodeHash": "0xb11ce94fd6165d8ca86eebafc7235e0875380d1a5d4e8b267ff0c6477083b21c" }, "src/L2/SuperchainWETH.sol": { - "initCodeHash": "0xd8766c7ab41d34d935febf5b48289f947804634bde38f8e346075b9f2d867275", - "sourceCodeHash": "0x6c1691c0fb5c86f1fd67e23495725c2cd86567556602e8cc0f28104ad6114bf4" + "initCodeHash": "0x702ff6dc90e7e02085e95e3510590cce9bf44a7ea06bfbb8f7a47e203a8809b2", + "sourceCodeHash": "0x823ded4da0dc1f44bc87b5e46d0a1c90c76f76e0f36c294c5410c4755886c925" }, "src/L2/WETH.sol": { "initCodeHash": "0xfb253765520690623f177941c2cd9eba23e4c6d15063bccdd5e98081329d8956", diff --git a/packages/contracts-bedrock/snapshots/abi/SuperchainWETH.json b/packages/contracts-bedrock/snapshots/abi/SuperchainWETH.json index 36d1aa36f8f8..08364ff80514 100644 --- a/packages/contracts-bedrock/snapshots/abi/SuperchainWETH.json +++ b/packages/contracts-bedrock/snapshots/abi/SuperchainWETH.json @@ -423,11 +423,6 @@ "name": "NotCustomGasToken", "type": "error" }, - { - "inputs": [], - "name": "Unauthorized", - "type": "error" - }, { "inputs": [], "name": "ZeroAddress", diff --git a/packages/contracts-bedrock/src/L2/SuperchainWETH.sol b/packages/contracts-bedrock/src/L2/SuperchainWETH.sol index a672918ffea1..af39fa53951c 100644 --- a/packages/contracts-bedrock/src/L2/SuperchainWETH.sol +++ b/packages/contracts-bedrock/src/L2/SuperchainWETH.sol @@ -21,8 +21,8 @@ import { IETHLiquidity } from "src/L2/interfaces/IETHLiquidity.sol"; /// do not use a custom gas token. contract SuperchainWETH is WETH98, ISuperchainERC20Extensions, ISemver { /// @notice Semantic version. - /// @custom:semver 1.0.0-beta.4 - string public constant version = "1.0.0-beta.4"; + /// @custom:semver 1.0.0-beta.5 + string public constant version = "1.0.0-beta.5"; /// @inheritdoc WETH98 function deposit() public payable override { @@ -61,8 +61,8 @@ contract SuperchainWETH is WETH98, ISuperchainERC20Extensions, ISemver { function relayERC20(address from, address dst, uint256 wad) external { // Receive message from other chain. IL2ToL2CrossDomainMessenger messenger = IL2ToL2CrossDomainMessenger(Predeploys.L2_TO_L2_CROSS_DOMAIN_MESSENGER); - if (msg.sender != address(messenger)) revert Unauthorized(); - if (messenger.crossDomainMessageSender() != address(this)) revert Unauthorized(); + if (msg.sender != address(messenger)) revert CallerNotL2ToL2CrossDomainMessenger(); + if (messenger.crossDomainMessageSender() != address(this)) revert InvalidCrossDomainSender(); // Mint from ETHLiquidity contract. if (!IL1Block(Predeploys.L1_BLOCK_ATTRIBUTES).isCustomGasToken()) { diff --git a/packages/contracts-bedrock/src/L2/interfaces/ISuperchainERC20.sol b/packages/contracts-bedrock/src/L2/interfaces/ISuperchainERC20.sol index a4396e88affa..737cfa0a53cd 100644 --- a/packages/contracts-bedrock/src/L2/interfaces/ISuperchainERC20.sol +++ b/packages/contracts-bedrock/src/L2/interfaces/ISuperchainERC20.sol @@ -11,8 +11,7 @@ interface ISuperchainERC20Errors { /// L2ToL2CrossDomainMessenger. error CallerNotL2ToL2CrossDomainMessenger(); - /// @notice Thrown when attempting to relay a message and the cross domain message sender is not this - /// SuperchainERC20. + /// @notice Thrown when attempting to relay a message and the cross domain message sender is not `address(this)` error InvalidCrossDomainSender(); /// @notice Thrown when attempting to perform an operation and the account is the zero address.