test: Default to enabling fault proofs (ethereum-optimism#12489)

* test: Default to enabling fault proofs * test: replace useFaultProofs with useLegacyContracts Note that this does not modify the deploy config, which still uses the same `useFaultProofs` flag. It simply modifies the state variables in CommonTest * chore: Update gas snapshot * fix: enableLegacy on L2OO invariants * feat: Add comment * fix: enableLegacy on portal invariants * fix: enableLegacy on Initializable.t.sol * fix: enableLegacy on portal tests * feat: follow convention of super.enable...() * fix: Don't double enable
defi-wonderland · Oct 22, 2024 · 76beff3 · 76beff3
1 parent d8012e0
commit 76beff3
Show file tree

Hide file tree

Showing 15 changed files with 31 additions and 28 deletions.
diff --git a/packages/contracts-bedrock/.gas-snapshot b/packages/contracts-bedrock/.gas-snapshot
@@ -4,12 +4,12 @@ GasBenchMark_L1BlockInterop_SetValuesInterop:test_setL1BlockValuesInterop_benchm
 GasBenchMark_L1BlockInterop_SetValuesInterop_Warm:test_setL1BlockValuesInterop_benchmark() (gas: 5099)
 GasBenchMark_L1Block_SetValuesEcotone:test_setL1BlockValuesEcotone_benchmark() (gas: 158531)
 GasBenchMark_L1Block_SetValuesEcotone_Warm:test_setL1BlockValuesEcotone_benchmark() (gas: 7597)
-GasBenchMark_L1CrossDomainMessenger:test_sendMessage_benchmark_0() (gas: 369242)
-GasBenchMark_L1CrossDomainMessenger:test_sendMessage_benchmark_1() (gas: 2967382)
-GasBenchMark_L1StandardBridge_Deposit:test_depositERC20_benchmark_0() (gas: 564356)
-GasBenchMark_L1StandardBridge_Deposit:test_depositERC20_benchmark_1() (gas: 4076571)
-GasBenchMark_L1StandardBridge_Deposit:test_depositETH_benchmark_0() (gas: 467019)
-GasBenchMark_L1StandardBridge_Deposit:test_depositETH_benchmark_1() (gas: 3512723)
+GasBenchMark_L1CrossDomainMessenger:test_sendMessage_benchmark_0() (gas: 369344)
+GasBenchMark_L1CrossDomainMessenger:test_sendMessage_benchmark_1() (gas: 2967484)
+GasBenchMark_L1StandardBridge_Deposit:test_depositERC20_benchmark_0() (gas: 564462)
+GasBenchMark_L1StandardBridge_Deposit:test_depositERC20_benchmark_1() (gas: 4076505)
+GasBenchMark_L1StandardBridge_Deposit:test_depositETH_benchmark_0() (gas: 466947)
+GasBenchMark_L1StandardBridge_Deposit:test_depositETH_benchmark_1() (gas: 3512651)
 GasBenchMark_L1StandardBridge_Finalize:test_finalizeETHWithdrawal_benchmark() (gas: 72621)
 GasBenchMark_L2OutputOracle:test_proposeL2Output_benchmark() (gas: 92973)
 GasBenchMark_OptimismPortal:test_depositTransaction_benchmark() (gas: 68357)

diff --git a/packages/contracts-bedrock/test/L1/L2OutputOracle.t.sol b/packages/contracts-bedrock/test/L1/L2OutputOracle.t.sol
@@ -17,7 +17,12 @@ import { Proxy } from "src/universal/Proxy.sol";
 import { L2OutputOracle } from "src/L1/L2OutputOracle.sol";
 import { IL2OutputOracle } from "src/L1/interfaces/IL2OutputOracle.sol";
 
-contract L2OutputOracle_constructor_Test is CommonTest {
+contract L2OutputOracle_TestBase is CommonTest {
+    function setUp() public override {
+        super.enableLegacyContracts();
+        super.setUp();
+    }
+
     /// @dev Tests that constructor sets the initial values correctly.
     function test_constructor_succeeds() external {
         IL2OutputOracle oracleImpl = IL2OutputOracle(address(new L2OutputOracle()));
@@ -63,7 +68,7 @@ contract L2OutputOracle_constructor_Test is CommonTest {
     }
 }
 
-contract L2OutputOracle_getter_Test is CommonTest {
+contract L2OutputOracle_getter_Test is L2OutputOracle_TestBase {
     bytes32 proposedOutput1 = keccak256(abi.encode(1));
 
     /// @dev Tests that `latestBlockNumber` returns the correct value.
@@ -199,7 +204,7 @@ contract L2OutputOracle_getter_Test is CommonTest {
     }
 }
 
-contract L2OutputOracle_proposeL2Output_Test is CommonTest {
+contract L2OutputOracle_proposeL2Output_Test is L2OutputOracle_TestBase {
     /// @dev Test that `proposeL2Output` succeeds for a valid input
     ///      and when a block hash and number are not specified.
     function test_proposeL2Output_proposeAnotherOutput_succeeds() public {
@@ -291,7 +296,7 @@ contract L2OutputOracle_proposeL2Output_Test is CommonTest {
     }
 }
 
-contract L2OutputOracle_deleteOutputs_Test is CommonTest {
+contract L2OutputOracle_deleteOutputs_Test is L2OutputOracle_TestBase {
     /// @dev Tests that `deleteL2Outputs` succeeds for a single output.
     function test_deleteOutputs_singleOutput_succeeds() external {
         proposeAnotherOutput();
@@ -405,7 +410,7 @@ contract L2OutputOracle_deleteOutputs_Test is CommonTest {
     }
 }
 
-contract L2OutputOracleUpgradeable_Test is CommonTest {
+contract L2OutputOracleUpgradeable_Test is L2OutputOracle_TestBase {
     /// @dev Tests that the proxy can be successfully upgraded.
     function test_upgrading_succeeds() external {
         Proxy proxy = Proxy(deploy.mustGetAddress("L2OutputOracleProxy"));

diff --git a/packages/contracts-bedrock/test/L1/OptimismPortal.t.sol b/packages/contracts-bedrock/test/L1/OptimismPortal.t.sol
@@ -34,6 +34,7 @@ contract OptimismPortal_Test is CommonTest {
     /// @notice Marked virtual to be overridden in
     ///         test/kontrol/deployment/DeploymentSummary.t.sol
     function setUp() public virtual override {
+        super.enableLegacyContracts();
         super.setUp();
         depositor = makeAddr("depositor");
     }
@@ -580,6 +581,7 @@ contract OptimismPortal_FinalizeWithdrawal_Test is CommonTest {
 
     // Use a constructor to set the storage vars above, so as to minimize the number of ffi calls.
     constructor() {
+        super.enableLegacyContracts();
         super.setUp();
         _defaultTx = Types.WithdrawalTransaction({
             nonce: 0,

diff --git a/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol b/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol
@@ -34,7 +34,6 @@ contract OptimismPortal2_Test is CommonTest {
     address depositor;
 
     function setUp() public virtual override {
-        super.enableFaultProofs();
         super.setUp();
 
         // zero out contracts that should not be used
@@ -436,7 +435,6 @@ contract OptimismPortal2_FinalizeWithdrawal_Test is CommonTest {
 
     // Use a constructor to set the storage vars above, so as to minimize the number of ffi calls.
     constructor() {
-        super.enableFaultProofs();
         super.setUp();
 
         _defaultTx = Types.WithdrawalTransaction({
@@ -1397,7 +1395,6 @@ contract OptimismPortal2_FinalizeWithdrawal_Test is CommonTest {
 
 contract OptimismPortal2_Upgradeable_Test is CommonTest {
     function setUp() public override {
-        super.enableFaultProofs();
         super.setUp();
     }
 
@@ -1443,7 +1440,6 @@ contract OptimismPortal2_ResourceFuzz_Test is CommonTest {
     uint256 constant MAX_GAS_LIMIT = 30_000_000;
 
     function setUp() public override {
-        super.enableFaultProofs();
         super.setUp();
     }
 

diff --git a/packages/contracts-bedrock/test/L1/SystemConfig.t.sol b/packages/contracts-bedrock/test/L1/SystemConfig.t.sol
@@ -311,7 +311,7 @@ contract SystemConfig_Init_CustomGasToken is SystemConfig_Init {
     function setUp() public override {
         token = new ERC20("Silly", "SIL");
         super.enableCustomGasToken(address(token));
-        super.enableFaultProofs();
+
         super.setUp();
     }
 

diff --git a/packages/contracts-bedrock/test/dispute/DelayedWETH.t.sol b/packages/contracts-bedrock/test/dispute/DelayedWETH.t.sol
@@ -18,7 +18,6 @@ contract DelayedWETH_Init is CommonTest {
     event Unwrap(address indexed src, uint256 wad);
 
     function setUp() public virtual override {
-        super.enableFaultProofs();
         super.setUp();
 
         // Transfer ownership of delayed WETH to the test contract.

diff --git a/packages/contracts-bedrock/test/dispute/DisputeGameFactory.t.sol b/packages/contracts-bedrock/test/dispute/DisputeGameFactory.t.sol
@@ -24,7 +24,6 @@ contract DisputeGameFactory_Init is CommonTest {
     event InitBondUpdated(GameType indexed gameType, uint256 indexed newBond);
 
     function setUp() public virtual override {
-        super.enableFaultProofs();
         super.setUp();
         fakeClone = new FakeClone();
 

diff --git a/packages/contracts-bedrock/test/invariants/L2OutputOracle.t.sol b/packages/contracts-bedrock/test/invariants/L2OutputOracle.t.sol
@@ -33,6 +33,7 @@ contract L2OutputOracle_MonotonicBlockNumIncrease_Invariant is CommonTest {
     L2OutputOracle_Proposer internal actor;
 
     function setUp() public override {
+        super.enableLegacyContracts();
         super.setUp();
 
         // Create a proposer actor.

diff --git a/packages/contracts-bedrock/test/invariants/OptimismPortal.t.sol b/packages/contracts-bedrock/test/invariants/OptimismPortal.t.sol
@@ -88,6 +88,7 @@ contract OptimismPortal_Invariant_Harness is CommonTest {
     Types.OutputRootProof internal _outputRootProof;
 
     function setUp() public virtual override {
+        super.enableLegacyContracts();
         super.setUp();
 
         _defaultTx = Types.WithdrawalTransaction({

diff --git a/packages/contracts-bedrock/test/invariants/OptimismPortal2.t.sol b/packages/contracts-bedrock/test/invariants/OptimismPortal2.t.sol
@@ -96,7 +96,6 @@ contract OptimismPortal2_Invariant_Harness is CommonTest {
     Types.OutputRootProof internal _outputRootProof;
 
     function setUp() public virtual override {
-        super.enableFaultProofs();
         super.setUp();
 
         _defaultTx = Types.WithdrawalTransaction({

diff --git a/packages/contracts-bedrock/test/safe/DeputyGuardianModule.t.sol b/packages/contracts-bedrock/test/safe/DeputyGuardianModule.t.sol
@@ -32,7 +32,6 @@ contract DeputyGuardianModule_TestInit is CommonTest, SafeTestTools {
 
     /// @dev Sets up the test environment
     function setUp() public virtual override {
-        super.enableFaultProofs();
         super.setUp();
 
         // Create a Safe with 10 owners

diff --git a/packages/contracts-bedrock/test/setup/CommonTest.sol b/packages/contracts-bedrock/test/setup/CommonTest.sol
@@ -19,7 +19,7 @@ contract CommonTest is Test, Setup, Events {
     FFIInterface constant ffi = FFIInterface(address(uint160(uint256(keccak256(abi.encode("optimism.ffi"))))));
 
     bool useAltDAOverride;
-    bool useFaultProofs;
+    bool useLegacyContracts;
     address customGasToken;
     bool useInteropOverride;
 
@@ -35,7 +35,8 @@ contract CommonTest is Test, Setup, Events {
         if (useAltDAOverride) {
             deploy.cfg().setUseAltDA(true);
         }
-        if (useFaultProofs) {
+        // We default to fault proofs unless explicitly disabled by useLegacyContracts
+        if (!useLegacyContracts) {
             deploy.cfg().setUseFaultProofs(true);
         }
         if (customGasToken != address(0)) {
@@ -109,14 +110,14 @@ contract CommonTest is Test, Setup, Events {
         l2OutputOracle.proposeL2Output(proposedOutput2, nextBlockNumber, 0, 0);
     }
 
-    function enableFaultProofs() public {
+    function enableLegacyContracts() public {
         // Check if the system has already been deployed, based off of the heuristic that alice and bob have not been
         // set by the `setUp` function yet.
         if (!(alice == address(0) && bob == address(0))) {
             revert("CommonTest: Cannot enable fault proofs after deployment. Consider overriding `setUp`.");
         }
 
-        useFaultProofs = true;
+        useLegacyContracts = true;
     }
 
     function enableAltDA() public {
@@ -147,7 +148,6 @@ contract CommonTest is Test, Setup, Events {
             revert("CommonTest: Cannot enable interop after deployment. Consider overriding `setUp`.");
         }
 
-        useFaultProofs = true;
         useInteropOverride = true;
     }
 }
diff --git a/packages/contracts-bedrock/test/setup/DeployVariations.t.sol b/packages/contracts-bedrock/test/setup/DeployVariations.t.sol
@@ -26,7 +26,7 @@ contract DeployVariations_Test is CommonTest {
     /// @dev It should be possible to enable Fault Proofs with any mix of CGT and Alt-DA.
     function testFuzz_enableFaultProofs(bool _enableCGT, bool _enableAltDa) public virtual {
         enableAddOns(_enableCGT, _enableAltDa);
-        super.enableFaultProofs();
+
         super.setUp();
     }
 
@@ -35,7 +35,7 @@ contract DeployVariations_Test is CommonTest {
     function test_enableInteropAndFaultProofs(bool _enableCGT, bool _enableAltDa) public virtual {
         enableAddOns(_enableCGT, _enableAltDa);
         super.enableInterop();
-        super.enableFaultProofs();
+
         super.setUp();
     }
 }
diff --git a/packages/contracts-bedrock/test/universal/BenchmarkTest.t.sol b/packages/contracts-bedrock/test/universal/BenchmarkTest.t.sol
@@ -47,6 +47,7 @@ contract GasBenchMark_OptimismPortal is CommonTest {
 
     // Use a constructor to set the storage vars above, so as to minimize the number of ffi calls.
     constructor() {
+        super.enableLegacyContracts();
         super.setUp();
         _defaultTx = Types.WithdrawalTransaction({
             nonce: 0,
@@ -204,6 +205,7 @@ contract GasBenchMark_L2OutputOracle is CommonTest {
     uint256 nextBlockNumber;
 
     function setUp() public override {
+        super.enableLegacyContracts();
         super.setUp();
         nextBlockNumber = l2OutputOracle.nextBlockNumber();
         warpToProposeTime(nextBlockNumber);

diff --git a/packages/contracts-bedrock/test/vendor/Initializable.t.sol b/packages/contracts-bedrock/test/vendor/Initializable.t.sol
@@ -46,7 +46,7 @@ contract Initializer_Test is Bridge_Initializer {
 
     function setUp() public override {
         super.enableAltDA();
-        // Run the `Bridge_Initializer`'s `setUp()` function.
+        super.enableLegacyContracts();
         super.setUp();
 
         // Initialize the `contracts` array with the addresses of the contracts to test, the