Merge pull request #7988 from ethereum-optimism/ctb/more-modularize-test-mocks

ctb: more modularize test mocks

packages/contracts-bedrock/.gas-snapshot

@@ -171,11 +171,11 @@ L1BlockTest:test_timestamp_succeeds() (gas: 7640)
 L1BlockTest:test_updateValues_succeeds() (gas: 60482)
 L1CrossDomainMessenger_Test:test_messageVersion_succeeds() (gas: 24781)
 L1CrossDomainMessenger_Test:test_relayMessage_legacyOldReplay_reverts() (gas: 49384)
-L1CrossDomainMessenger_Test:test_relayMessage_legacyRetryAfterFailureThenSuccess_reverts() (gas: 212329)
-L1CrossDomainMessenger_Test:test_relayMessage_legacyRetryAfterFailure_succeeds() (gas: 206087)
+L1CrossDomainMessenger_Test:test_relayMessage_legacyRetryAfterFailureThenSuccess_reverts() (gas: 239657)
+L1CrossDomainMessenger_Test:test_relayMessage_legacyRetryAfterFailure_succeeds() (gas: 233416)
 L1CrossDomainMessenger_Test:test_relayMessage_legacyRetryAfterSuccess_reverts() (gas: 126344)
 L1CrossDomainMessenger_Test:test_relayMessage_legacy_succeeds() (gas: 79599)
-L1CrossDomainMessenger_Test:test_relayMessage_retryAfterFailure_succeeds() (gas: 200127)
+L1CrossDomainMessenger_Test:test_relayMessage_retryAfterFailure_succeeds() (gas: 227456)
 L1CrossDomainMessenger_Test:test_relayMessage_succeeds() (gas: 76590)
 L1CrossDomainMessenger_Test:test_relayMessage_toSystemContract_reverts() (gas: 60779)
 L1CrossDomainMessenger_Test:test_relayMessage_v2_reverts() (gas: 12343)
@@ -219,7 +219,7 @@ L1StandardBridge_Getter_Test:test_getters_succeeds() (gas: 28383)
 L1StandardBridge_Initialize_Test:test_initialize_succeeds() (gas: 27239)
 L1StandardBridge_Receive_Test:test_receive_succeeds() (gas: 615130)
 L2CrossDomainMessenger_Test:test_messageVersion_succeeds() (gas: 8477)
-L2CrossDomainMessenger_Test:test_relayMessage_retry_succeeds() (gas: 163749)
+L2CrossDomainMessenger_Test:test_relayMessage_retry_succeeds() (gas: 191078)
 L2CrossDomainMessenger_Test:test_relayMessage_succeeds() (gas: 48913)
 L2CrossDomainMessenger_Test:test_relayMessage_toSystemContract_reverts() (gas: 31136)
 L2CrossDomainMessenger_Test:test_relayMessage_v2_reverts() (gas: 11689)
@@ -673,7 +673,7 @@ SafeCall_Test:test_callWithMinGas_noLeakageLow_succeeds() (gas: 1095190688)
 Semver_Test:test_behindProxy_succeeds() (gas: 507558)
 Semver_Test:test_version_succeeds() (gas: 9418)
 SequencerFeeVault_L2Withdrawal_Test:test_withdraw_toL2_succeeds() (gas: 78333)
-SequencerFeeVault_L2Withdrawal_Test:test_withdraw_toL2recipientReverts_fails() (gas: 46486)
+SequencerFeeVault_L2Withdrawal_Test:test_withdraw_toL2recipientReverts_fails() (gas: 46791)
 SequencerFeeVault_Test:test_constructor_succeeds() (gas: 5526)
 SequencerFeeVault_Test:test_minWithdrawalAmount_succeeds() (gas: 5464)
 SequencerFeeVault_Test:test_receive_succeeds() (gas: 17373)

packages/contracts-bedrock/test/CommonTest.t.sol

@@ -731,99 +731,3 @@ contract FFIInterface is Test {
         return (memRoot, proof);
     }
 }
-
-library EIP1967Helper {
-    Vm internal constant vm = Vm(0x7109709ECfa91a80626fF3989D68f67F5b1DD12D);
-
-    function getAdmin(address _proxy) internal view returns (address) {
-        return address(uint160(uint256(vm.load(address(_proxy), Constants.PROXY_OWNER_ADDRESS))));
-    }
-
-    function getImplementation(address _proxy) internal view returns (address) {
-        return address(uint160(uint256(vm.load(address(_proxy), Constants.PROXY_IMPLEMENTATION_ADDRESS))));
-    }
-}
-
-// Used for testing a future upgrade beyond the current implementations.
-// We include some variables so that we can sanity check accessing storage values after an upgrade.
-contract NextImpl is Initializable {
-    // Initializable occupies the zero-th slot.
-    bytes32 slot1;
-    bytes32[19] __gap;
-    bytes32 slot21;
-    bytes32 public constant slot21Init = bytes32(hex"1337");
-
-    function initialize(uint8 _init) public reinitializer(_init) {
-        // Slot21 is unused by an of our upgradeable contracts.
-        // This is used to verify that we can access this value after an upgrade.
-        slot21 = slot21Init;
-    }
-}
-
-contract Reverter {
-    fallback() external {
-        revert();
-    }
-}
-
-// Useful for testing reentrancy guards
-contract CallerCaller {
-    event WhatHappened(bool success, bytes returndata);
-
-    fallback() external {
-        (bool success, bytes memory returndata) = msg.sender.call(msg.data);
-        emit WhatHappened(success, returndata);
-        assembly {
-            switch success
-            case 0 { revert(add(returndata, 0x20), mload(returndata)) }
-            default { return(add(returndata, 0x20), mload(returndata)) }
-        }
-    }
-}
-
-// Used for testing the `CrossDomainMessenger`'s per-message reentrancy guard.
-contract ConfigurableCaller {
-    bool doRevert = true;
-    address target;
-    bytes payload;
-
-    event WhatHappened(bool success, bytes returndata);
-
-    /// @notice Call the configured target with the configured payload OR revert.
-    function call() external {
-        if (doRevert) {
-            revert("ConfigurableCaller: revert");
-        } else {
-            (bool success, bytes memory returndata) = address(target).call(payload);
-            emit WhatHappened(success, returndata);
-            assembly {
-                switch success
-                case 0 { revert(add(returndata, 0x20), mload(returndata)) }
-                default { return(add(returndata, 0x20), mload(returndata)) }
-            }
-        }
-    }
-
-    /// @notice Set whether or not to have `call` revert.
-    function setDoRevert(bool _doRevert) external {
-        doRevert = _doRevert;
-    }
-
-    /// @notice Set the target for the call made in `call`.
-    function setTarget(address _target) external {
-        target = _target;
-    }
-
-    /// @notice Set the payload for the call made in `call`.
-    function setPayload(bytes calldata _payload) external {
-        payload = _payload;
-    }
-
-    /// @notice Fallback function that reverts if `doRevert` is true.
-    ///        Otherwise, it does nothing.
-    fallback() external {
-        if (doRevert) {
-            revert("ConfigurableCaller: revert");
-        }
-    }
-}

packages/contracts-bedrock/test/CrossDomainMessenger.t.sol

@@ -2,13 +2,16 @@
 pragma solidity 0.8.15;
 
 // Testing utilities
-import { Messenger_Initializer, Reverter, CallerCaller, CommonTest } from "test/CommonTest.t.sol";
+import { Test } from "forge-std/Test.sol";
+import { Messenger_Initializer, Test } from "test/CommonTest.t.sol";
 import { L1CrossDomainMessenger } from "src/L1/L1CrossDomainMessenger.sol";
 
+import { Reverter, CallerCaller } from "test/mocks/Callers.sol";
+
 // Libraries
-import { Predeploys } from "../src/libraries/Predeploys.sol";
-import { Hashing } from "../src/libraries/Hashing.sol";
-import { Encoding } from "../src/libraries/Encoding.sol";
+import { Predeploys } from "src/libraries/Predeploys.sol";
+import { Hashing } from "src/libraries/Hashing.sol";
+import { Encoding } from "src/libraries/Encoding.sol";
 
 // CrossDomainMessenger_Test is for testing functionality which is common to both the L1 and L2
 // CrossDomainMessenger contracts. For simplicity, we use the L1 Messenger as the test contract.
@@ -39,7 +42,7 @@ contract CrossDomainMessenger_BaseGas_Test is Messenger_Initializer {
 /// @title ExternalRelay
 /// @notice A mock external contract called via the SafeCall inside
 ///         the CrossDomainMessenger's `relayMessage` function.
-contract ExternalRelay is CommonTest {
+contract ExternalRelay is Test {
     address internal op;
     address internal fuzzedSender;
     L1CrossDomainMessenger internal L1Messenger;

packages/contracts-bedrock/test/L1CrossDomainMessenger.t.sol

@@ -2,8 +2,9 @@
 pragma solidity 0.8.15;
 
 // Testing utilities
-import { Messenger_Initializer, Reverter, ConfigurableCaller } from "test/CommonTest.t.sol";
+import { Messenger_Initializer } from "test/CommonTest.t.sol";
 import { L2OutputOracle_Initializer } from "test/L2OutputOracle.t.sol";
+import { Reverter, ConfigurableCaller } from "test/mocks/Callers.sol";
 
 // Libraries
 import { AddressAliasHelper } from "src/vendor/AddressAliasHelper.sol";
@@ -12,12 +13,8 @@ import { Hashing } from "src/libraries/Hashing.sol";
 import { Encoding } from "src/libraries/Encoding.sol";
 
 // Target contract dependencies
-import { L2OutputOracle } from "src/L1/L2OutputOracle.sol";
 import { OptimismPortal } from "src/L1/OptimismPortal.sol";
 
-// Target contract
-import { L1CrossDomainMessenger } from "src/L1/L1CrossDomainMessenger.sol";
-
 contract L1CrossDomainMessenger_Test is Messenger_Initializer {
     /// @dev The receiver address
     address recipient = address(0xabbaacdc);

packages/contracts-bedrock/test/L2CrossDomainMessenger.t.sol

@@ -2,7 +2,8 @@
 pragma solidity 0.8.15;
 
 // Testing utilities
-import { Messenger_Initializer, Reverter, ConfigurableCaller } from "test/CommonTest.t.sol";
+import { Messenger_Initializer } from "test/CommonTest.t.sol";
+import { Reverter, ConfigurableCaller } from "test/mocks/Callers.sol";
 
 // Libraries
 import { Hashing } from "src/libraries/Hashing.sol";
@@ -12,10 +13,6 @@ import { Types } from "src/libraries/Types.sol";
 // Target contract dependencies
 import { L2ToL1MessagePasser } from "src/L2/L2ToL1MessagePasser.sol";
 import { AddressAliasHelper } from "src/vendor/AddressAliasHelper.sol";
-import { L1CrossDomainMessenger } from "src/L1/L1CrossDomainMessenger.sol";
-
-// Target contract
-import { L2CrossDomainMessenger } from "src/L2/L2CrossDomainMessenger.sol";
 
 contract L2CrossDomainMessenger_Test is Messenger_Initializer {
     /// @dev Receiver address for testing

packages/contracts-bedrock/test/L2OutputOracle.t.sol

@@ -3,7 +3,9 @@ pragma solidity 0.8.15;
 
 // Testing utilities
 import { stdError } from "forge-std/Test.sol";
-import { L2OutputOracle_Initializer, NextImpl, EIP1967Helper } from "test/CommonTest.t.sol";
+import { L2OutputOracle_Initializer } from "test/CommonTest.t.sol";
+import { NextImpl } from "test/mocks/NextImpl.sol";
+import { EIP1967Helper } from "test/mocks/EIP1967Helper.sol";
 
 // Libraries
 import { Types } from "src/libraries/Types.sol";

packages/contracts-bedrock/test/OptimismPortal.t.sol

@@ -3,7 +3,8 @@ pragma solidity 0.8.15;
 
 // Testing utilities
 import { stdError } from "forge-std/Test.sol";
-import { Portal_Initializer, CommonTest, NextImpl } from "test/CommonTest.t.sol";
+import { Portal_Initializer } from "test/CommonTest.t.sol";
+import { NextImpl } from "test/mocks/NextImpl.sol";
 
 // Libraries
 import { Types } from "src/libraries/Types.sol";

packages/contracts-bedrock/test/SequencerFeeVault.t.sol

@@ -2,7 +2,8 @@
 pragma solidity 0.8.15;
 
 // Testing utilities
-import { FeeVault_Initializer, Reverter } from "test/CommonTest.t.sol";
+import { FeeVault_Initializer } from "test/CommonTest.t.sol";
+import { Reverter } from "test/mocks/Callers.sol";
 import { StandardBridge } from "src/universal/StandardBridge.sol";
 
 // Libraries

packages/contracts-bedrock/test/invariants/OptimismPortal.t.sol

@@ -12,7 +12,7 @@ import { ResourceMetering } from "src/L1/ResourceMetering.sol";
 import { Constants } from "src/libraries/Constants.sol";
 
 import { Portal_Initializer } from "test/CommonTest.t.sol";
-import { EIP1967Helper } from "test/CommonTest.t.sol";
+import { EIP1967Helper } from "test/mocks/EIP1967Helper.sol";
 import { Types } from "src/libraries/Types.sol";
 
 contract OptimismPortal_Depositor is StdUtils, ResourceMetering {

packages/contracts-bedrock/test/mocks/Callers.sol

@@ -19,8 +19,75 @@ contract CallRecorder {
     }
 }
 
+/// @dev Useful for testing reentrancy guards
+contract CallerCaller {
+    event WhatHappened(bool success, bytes returndata);
+
+    fallback() external {
+        (bool success, bytes memory returndata) = msg.sender.call(msg.data);
+        emit WhatHappened(success, returndata);
+        assembly {
+            switch success
+            case 0 { revert(add(returndata, 0x20), mload(returndata)) }
+            default { return(add(returndata, 0x20), mload(returndata)) }
+        }
+    }
+}
+
+/// @dev Used for testing the `CrossDomainMessenger`'s per-message reentrancy guard.
+contract ConfigurableCaller {
+    bool doRevert = true;
+    address target;
+    bytes payload;
+
+    event WhatHappened(bool success, bytes returndata);
+
+    /// @notice Call the configured target with the configured payload OR revert.
+    function call() external {
+        if (doRevert) {
+            revert("ConfigurableCaller: revert");
+        } else {
+            (bool success, bytes memory returndata) = address(target).call(payload);
+            emit WhatHappened(success, returndata);
+            assembly {
+                switch success
+                case 0 { revert(add(returndata, 0x20), mload(returndata)) }
+                default { return(add(returndata, 0x20), mload(returndata)) }
+            }
+        }
+    }
+
+    /// @notice Set whether or not to have `call` revert.
+    function setDoRevert(bool _doRevert) external {
+        doRevert = _doRevert;
+    }
+
+    /// @notice Set the target for the call made in `call`.
+    function setTarget(address _target) external {
+        target = _target;
+    }
+
+    /// @notice Set the payload for the call made in `call`.
+    function setPayload(bytes calldata _payload) external {
+        payload = _payload;
+    }
+
+    /// @notice Fallback function that reverts if `doRevert` is true.
+    ///        Otherwise, it does nothing.
+    fallback() external {
+        if (doRevert) {
+            revert("ConfigurableCaller: revert");
+        }
+    }
+}
+
+/// @dev Any call will revert
 contract Reverter {
     function doRevert() public pure {
         revert("Reverter reverted");
     }
+
+    fallback() external {
+        revert();
+    }
 }

packages/contracts-bedrock/test/mocks/EIP1967Helper.sol

+// SPDX-License-Identifier: MIT
+pragma solidity 0.8.15;
+
+import { Vm } from "forge-std/Vm.sol";
+import { Constants } from "src/libraries/Constants.sol";
+
+/// @title EIP1967Helper
+/// @dev Testing library to help with reading EIP 1967 variables from state
+library EIP1967Helper {
+    Vm internal constant vm = Vm(0x7109709ECfa91a80626fF3989D68f67F5b1DD12D);
+
+    function getAdmin(address _proxy) internal view returns (address) {
+        return address(uint160(uint256(vm.load(address(_proxy), Constants.PROXY_OWNER_ADDRESS))));
+    }
+
+    function getImplementation(address _proxy) internal view returns (address) {
+        return address(uint160(uint256(vm.load(address(_proxy), Constants.PROXY_IMPLEMENTATION_ADDRESS))));
+    }
+}

packages/contracts-bedrock/test/mocks/NextImpl.sol

+// SPDX-License-Identifier: MIT
+pragma solidity 0.8.15;
+
+import { Initializable } from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
+
+/// @title NextImpl
+/// @dev Used for testing a future upgrade beyond the current implementations.
+//       We include some variables so that we can sanity check accessing storage values after an upgrade.
+contract NextImpl is Initializable {
+    // Initializable occupies the zero-th slot.
+    bytes32 slot1;
+    bytes32[19] __gap;
+    bytes32 slot21;
+    bytes32 public constant slot21Init = bytes32(hex"1337");
+
+    function initialize(uint8 _init) public reinitializer(_init) {
+        // Slot21 is unused by an of our upgradeable contracts.
+        // This is used to verify that we can access this value after an upgrade.
+        slot21 = slot21Init;
+    }
+}