Implement fixes from MCP L1 audit (1) (#9476)

* contracts-bedrock: add check for latestBlockNumber in L2OutputOracle.t.sol

* contracts-bedrock: fix incorrect implementation initialization testing in L2CrossDomainMessenger.t.sol, L2StandardBridge.t.sol

* contracts-bedrock: add resource metering params check in OptimismPortal.t.sol

* op-chain-ops: check resource config as part of l1.go

* op-chain-ops: fix error messages in l1.go

* contracts-bedrock: fix minor typo in L2OutputOracle.t.sol

* op-chain-ops: drop if config != nil in upgrade script

op-chain-ops/upgrades/l1.go

@@ -115,11 +115,11 @@ func L1CrossDomainMessenger(batch *safe.Batch, implementations superchain.Implem
 	}
 
 	if optimismPortal != common.Address(list.OptimismPortalProxy) {
-		return fmt.Errorf("upgrading L1CrossDomainMessenger: Portal address doesn't match config")
+		return fmt.Errorf("Portal address doesn't match config")
 	}
 
 	if otherMessenger != predeploys.L2CrossDomainMessengerAddr {
-		return fmt.Errorf("upgrading L1CrossDomainMessenger: OtherMessenger address doesn't match config")
+		return fmt.Errorf("OtherMessenger address doesn't match config")
 	}
 
 	calldata, err := l1CrossDomainMessengerABI.Pack("initialize", superchainConfigProxy, optimismPortal)
@@ -197,11 +197,11 @@ func L1ERC721Bridge(batch *safe.Batch, implementations superchain.Implementation
 	}
 
 	if messenger != common.Address(list.L1CrossDomainMessengerProxy) {
-		return fmt.Errorf("upgrading L1ERC721Bridge: Messenger address doesn't match config")
+		return fmt.Errorf("Messenger address doesn't match config")
 	}
 
 	if otherBridge != predeploys.L2ERC721BridgeAddr {
-		return fmt.Errorf("upgrading L1ERC721Bridge: OtherBridge address doesn't match config")
+		return fmt.Errorf("OtherBridge address doesn't match config")
 	}
 
 	calldata, err := l1ERC721BridgeABI.Pack("initialize", messenger, superchainConfigProxy)
@@ -281,11 +281,11 @@ func L1StandardBridge(batch *safe.Batch, implementations superchain.Implementati
 	}
 
 	if messenger != common.Address(list.L1CrossDomainMessengerProxy) {
-		return fmt.Errorf("upgrading L1StandardBridge: Messenger address doesn't match config")
+		return fmt.Errorf("Messenger address doesn't match config")
 	}
 
 	if otherBridge != predeploys.L2StandardBridgeAddr {
-		return fmt.Errorf("upgrading L1StandardBridge: OtherBridge address doesn't match config")
+		return fmt.Errorf("OtherBridge address doesn't match config")
 	}
 
 	calldata, err := l1StandardBridgeABI.Pack("initialize", messenger, superchainConfigProxy)
@@ -391,15 +391,15 @@ func L2OutputOracle(batch *safe.Batch, implementations superchain.Implementation
 
 	if config != nil {
 		if l2OutputOracleSubmissionInterval.Uint64() != config.L2OutputOracleSubmissionInterval {
-			return fmt.Errorf("upgrading L2OutputOracle: L2OutputOracleSubmissionInterval address doesn't match config")
+			return fmt.Errorf("L2OutputOracleSubmissionInterval address doesn't match config")
 		}
 
 		if l2BlockTime.Uint64() != config.L2BlockTime {
-			return fmt.Errorf("upgrading L2OutputOracle: L2BlockTime address doesn't match config")
+			return fmt.Errorf("L2BlockTime address doesn't match config")
 		}
 
 		if l2OutputOracleStartingBlockNumber.Uint64() != config.L2OutputOracleStartingBlockNumber {
-			return fmt.Errorf("upgrading L2OutputOracle: L2OutputOracleStartingBlockNumber address doesn't match config")
+			return fmt.Errorf("L2OutputOracleStartingBlockNumber address doesn't match config")
 		}
 
 		if config.L2OutputOracleStartingTimestamp < 0 {
@@ -407,19 +407,19 @@ func L2OutputOracle(batch *safe.Batch, implementations superchain.Implementation
 		}
 
 		if int(l2OutputOracleStartingTimestamp.Int64()) != config.L2OutputOracleStartingTimestamp {
-			return fmt.Errorf("upgrading L2OutputOracle: L2OutputOracleStartingTimestamp address doesn't match config")
+			return fmt.Errorf("L2OutputOracleStartingTimestamp address doesn't match config")
 		}
 
 		if l2OutputOracleProposer != config.L2OutputOracleProposer {
-			return fmt.Errorf("upgrading L2OutputOracle: L2OutputOracleProposer address doesn't match config")
+			return fmt.Errorf("L2OutputOracleProposer address doesn't match config")
 		}
 
 		if l2OutputOracleChallenger != config.L2OutputOracleChallenger {
-			return fmt.Errorf("upgrading L2OutputOracle: L2OutputOracleChallenger address doesn't match config")
+			return fmt.Errorf("L2OutputOracleChallenger address doesn't match config")
 		}
 
 		if finalizationPeriodSeconds.Uint64() != config.FinalizationPeriodSeconds {
-			return fmt.Errorf("upgrading L2OutputOracle: FinalizationPeriodSeconds address doesn't match config")
+			return fmt.Errorf("FinalizationPeriodSeconds address doesn't match config")
 		}
 	}
 
@@ -504,7 +504,7 @@ func OptimismMintableERC20Factory(batch *safe.Batch, implementations superchain.
 	}
 
 	if bridge != common.Address(list.L1StandardBridgeProxy) {
-		return fmt.Errorf("upgrading OptimismMintableERC20Factory: Bridge address doesn't match config")
+		return fmt.Errorf("Bridge address doesn't match config")
 	}
 
 	calldata, err := optimismMintableERC20FactoryABI.Pack("initialize", bridge)
@@ -582,11 +582,11 @@ func OptimismPortal(batch *safe.Batch, implementations superchain.Implementation
 	}
 
 	if l2OutputOracle != common.HexToAddress(list.L2OutputOracleProxy.String()) {
-		return fmt.Errorf("upgrading OptimismPortal: L2OutputOracle address doesn't match config")
+		return fmt.Errorf("L2OutputOracle address doesn't match config")
 	}
 
 	if systemConfig != common.HexToAddress(chainConfig.SystemConfigAddr.String()) {
-		return fmt.Errorf("upgrading OptimismPortal: SystemConfig address doesn't match config")
+		return fmt.Errorf("SystemConfig address doesn't match config")
 	}
 
 	calldata, err := optimismPortalABI.Pack("initialize", l2OutputOracle, systemConfig, superchainConfigProxy)
@@ -696,25 +696,47 @@ func SystemConfig(batch *safe.Batch, implementations superchain.ImplementationLi
 		return err
 	}
 
-	if config != nil {
-		if gasPriceOracleOverhead.Uint64() != config.GasPriceOracleOverhead {
-			return fmt.Errorf("upgrading SystemConfig: GasPriceOracleOverhead address doesn't match config")
-		}
-		if gasPriceOracleScalar.Uint64() != config.GasPriceOracleScalar {
-			return fmt.Errorf("upgrading SystemConfig: GasPriceOracleScalar address doesn't match config")
-		}
-		if batcherHash != common.BytesToHash(config.BatchSenderAddress.Bytes()) {
-			return fmt.Errorf("upgrading SystemConfig: BatchSenderAddress address doesn't match config")
-		}
-		if l2GenesisBlockGasLimit != uint64(config.L2GenesisBlockGasLimit) {
-			return fmt.Errorf("upgrading SystemConfig: L2GenesisBlockGasLimit address doesn't match config")
-		}
-		if p2pSequencerAddress != config.P2PSequencerAddress {
-			return fmt.Errorf("upgrading SystemConfig: P2PSequencerAddress address doesn't match config")
-		}
-		if finalSystemOwner != config.FinalSystemOwner {
-			return fmt.Errorf("upgrading SystemConfig: FinalSystemOwner address doesn't match config")
-		}
+	if gasPriceOracleOverhead.Uint64() != config.GasPriceOracleOverhead {
+		return fmt.Errorf("GasPriceOracleOverhead address doesn't match config")
+	}
+	if gasPriceOracleScalar.Uint64() != config.GasPriceOracleScalar {
+		return fmt.Errorf("GasPriceOracleScalar address doesn't match config")
+	}
+	if batcherHash != common.BytesToHash(config.BatchSenderAddress.Bytes()) {
+		return fmt.Errorf("BatchSenderAddress address doesn't match config")
+	}
+	if l2GenesisBlockGasLimit != uint64(config.L2GenesisBlockGasLimit) {
+		return fmt.Errorf("L2GenesisBlockGasLimit address doesn't match config")
+	}
+	if p2pSequencerAddress != config.P2PSequencerAddress {
+		return fmt.Errorf("P2PSequencerAddress address doesn't match config")
+	}
+	if finalSystemOwner != config.FinalSystemOwner {
+		return fmt.Errorf("FinalSystemOwner address doesn't match config")
+	}
+
+	resourceConfig, err := systemConfig.ResourceConfig(&bind.CallOpts{})
+	if err != nil {
+		return err
+	}
+
+	if resourceConfig.MaxResourceLimit != genesis.DefaultResourceConfig.MaxResourceLimit {
+		return fmt.Errorf("DefaultResourceConfig MaxResourceLimit doesn't match contract MaxResourceLimit")
+	}
+	if resourceConfig.ElasticityMultiplier != genesis.DefaultResourceConfig.ElasticityMultiplier {
+		return fmt.Errorf("DefaultResourceConfig ElasticityMultiplier doesn't match contract ElasticityMultiplier")
+	}
+	if resourceConfig.BaseFeeMaxChangeDenominator != genesis.DefaultResourceConfig.BaseFeeMaxChangeDenominator {
+		return fmt.Errorf("DefaultResourceConfig BaseFeeMaxChangeDenominator doesn't match contract BaseFeeMaxChangeDenominator")
+	}
+	if resourceConfig.MinimumBaseFee != genesis.DefaultResourceConfig.MinimumBaseFee {
+		return fmt.Errorf("DefaultResourceConfig MinimumBaseFee doesn't match contract MinimumBaseFee")
+	}
+	if resourceConfig.SystemTxMaxGas != genesis.DefaultResourceConfig.SystemTxMaxGas {
+		return fmt.Errorf("DefaultResourceConfig SystemTxMaxGas doesn't match contract SystemTxMaxGas")
+	}
+	if resourceConfig.MaximumBaseFee.Cmp(genesis.DefaultResourceConfig.MaximumBaseFee) != 0 {
+		return fmt.Errorf("DefaultResourceConfig MaximumBaseFee doesn't match contract MaximumBaseFee")
 	}
 
 	calldata, err := systemConfigABI.Pack(

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

@@ -51,6 +51,7 @@ contract L2OutputOracle_constructor_Test is CommonTest {
         assertEq(l2OutputOracle.submissionInterval(), submissionInterval);
         assertEq(l2OutputOracle.L2_BLOCK_TIME(), l2BlockTime);
         assertEq(l2OutputOracle.l2BlockTime(), l2BlockTime);
+        assertEq(l2OutputOracle.latestBlockNumber(), startingBlockNumber);
         assertEq(l2OutputOracle.startingBlockNumber(), startingBlockNumber);
         assertEq(l2OutputOracle.startingTimestamp(), startingTimestamp);
         assertEq(l2OutputOracle.finalizationPeriodSeconds(), finalizationPeriodSeconds);
@@ -451,7 +452,7 @@ contract L2OutputOracleUpgradeable_Test is CommonTest {
     }
 
     /// @dev Tests that initialize reverts if the l2BlockTime is invalid.
-    function test_initalize_l2BlockTimeZero_reverts() external {
+    function test_initialize_l2BlockTimeZero_reverts() external {
         // Reset the initialized field in the 0th storage slot
         // so that initialize can be called again.
         vm.store(address(l2OutputOracle), bytes32(uint256(0)), bytes32(uint256(0)));

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

@@ -43,6 +43,10 @@ contract OptimismPortal_Test is CommonTest {
         assertEq(address(opImpl.systemConfig()), address(0));
         assertEq(address(opImpl.superchainConfig()), address(0));
         assertEq(opImpl.l2Sender(), Constants.DEFAULT_L2_SENDER);
+        (uint128 prevBaseFee, uint64 prevBoughtGas, uint64 prevBlockNum) = opImpl.params();
+        assertEq(prevBaseFee, 1 gwei);
+        assertEq(prevBoughtGas, 0);
+        assertEq(prevBlockNum, uint64(block.number));
     }
 
     /// @dev Tests that the initializer sets the correct values.
@@ -59,6 +63,10 @@ contract OptimismPortal_Test is CommonTest {
         assertEq(address(optimismPortal.superchainConfig()), address(superchainConfig));
         assertEq(optimismPortal.l2Sender(), Constants.DEFAULT_L2_SENDER);
         assertEq(optimismPortal.paused(), false);
+        (uint128 prevBaseFee, uint64 prevBoughtGas, uint64 prevBlockNum) = optimismPortal.params();
+        assertEq(prevBaseFee, 1 gwei);
+        assertEq(prevBoughtGas, 0);
+        assertEq(prevBlockNum, uint64(block.number));
     }
 
     /// @dev Tests that `pause` successfully pauses

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

@@ -4,6 +4,7 @@ pragma solidity 0.8.15;
 // Testing utilities
 import { Bridge_Initializer } from "test/setup/Bridge_Initializer.sol";
 import { Reverter, ConfigurableCaller } from "test/mocks/Callers.sol";
+import { EIP1967Helper } from "test/mocks/EIP1967Helper.sol";
 
 // Libraries
 import { Hashing } from "src/libraries/Hashing.sol";
@@ -21,10 +22,11 @@ contract L2CrossDomainMessenger_Test is Bridge_Initializer {
 
     /// @dev Tests that the implementation is initialized correctly.
     function test_constructor_succeeds() external {
-        L2CrossDomainMessenger impl = L2CrossDomainMessenger(deploy.mustGetAddress("L2CrossDomainMessenger"));
-        assertEq(address(impl.OTHER_MESSENGER()), address(l1CrossDomainMessenger));
-        assertEq(address(impl.otherMessenger()), address(l1CrossDomainMessenger));
-        assertEq(address(impl.l1CrossDomainMessenger()), address(l1CrossDomainMessenger));
+        L2CrossDomainMessenger impl =
+            L2CrossDomainMessenger(EIP1967Helper.getImplementation(deploy.mustGetAddress("L2CrossDomainMessenger")));
+        assertEq(address(impl.OTHER_MESSENGER()), address(0));
+        assertEq(address(impl.otherMessenger()), address(0));
+        assertEq(address(impl.l1CrossDomainMessenger()), address(0));
     }
 
     /// @dev Tests that the proxy is initialized correctly.

packages/contracts-bedrock/test/L2/L2StandardBridge.t.sol

@@ -2,6 +2,8 @@
 pragma solidity 0.8.15;
 
 // Testing utilities
+import { EIP1967Helper } from "test/mocks/EIP1967Helper.sol";
+
 // Target contract is imported by the `Bridge_Initializer`
 import { Bridge_Initializer } from "test/setup/Bridge_Initializer.sol";
 import { stdStorage, StdStorage } from "forge-std/Test.sol";
@@ -24,12 +26,12 @@ contract L2StandardBridge_Test is Bridge_Initializer {
 
     /// @dev Test that the bridge's constructor sets the correct values.
     function test_constructor_succeeds() external {
-        L2StandardBridge impl = L2StandardBridge(deploy.mustGetAddress("L2StandardBridge"));
-        assertEq(address(impl.MESSENGER()), address(l2CrossDomainMessenger));
-        assertEq(address(impl.messenger()), address(l2CrossDomainMessenger));
-        assertEq(l1StandardBridge.l2TokenBridge(), address(impl));
-        assertEq(address(impl.OTHER_BRIDGE()), address(l1StandardBridge));
-        assertEq(address(impl.otherBridge()), address(l1StandardBridge));
+        L2StandardBridge impl =
+            L2StandardBridge(payable(EIP1967Helper.getImplementation(deploy.mustGetAddress("L2StandardBridge"))));
+        assertEq(address(impl.MESSENGER()), address(0));
+        assertEq(address(impl.messenger()), address(0));
+        assertEq(address(impl.OTHER_BRIDGE()), address(0));
+        assertEq(address(impl.otherBridge()), address(0));
     }
 
     /// @dev Tests that the bridge is initialized correctly.