Merge pull request #5437 from ethereum-optimism/fix/syscfg-min

contracts-bedrock: update systemconfig resource config validation

packages/contracts-bedrock/contracts/L1/SystemConfig.sol

@@ -80,7 +80,7 @@ contract SystemConfig is OwnableUpgradeable, Semver {
     event ConfigUpdate(uint256 indexed version, UpdateType indexed updateType, bytes data);
 
     /**
-     * @custom:semver 1.2.0
+     * @custom:semver 1.3.0
      *
      * @param _owner             Initial owner of the contract.
      * @param _overhead          Initial overhead value.
@@ -98,7 +98,7 @@ contract SystemConfig is OwnableUpgradeable, Semver {
         uint64 _gasLimit,
         address _unsafeBlockSigner,
         ResourceMetering.ResourceConfig memory _config
-    ) Semver(1, 2, 0) {
+    ) Semver(1, 3, 0) {
         initialize({
             _owner: _owner,
             _overhead: _overhead,
@@ -270,7 +270,7 @@ contract SystemConfig is OwnableUpgradeable, Semver {
             "SystemConfig: min base fee must be less than max base"
         );
         // Base fee change denominator must be greater than 0.
-        require(_config.baseFeeMaxChangeDenominator > 0, "SystemConfig: denominator cannot be 0");
+        require(_config.baseFeeMaxChangeDenominator > 1, "SystemConfig: denominator must be larger than 1");
         // Max resource limit plus system tx gas must be less than or equal to the L2 gas limit.
         // The gas limit must be increased before these values can be increased.
         require(

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

@@ -1085,3 +1085,88 @@ contract OptimismPortalUpgradeable_Test is Portal_Initializer {
         assertEq(slot21Expected, slot21After);
     }
 }
+
+/**
+ * @title OptimismPortalResourceFuzz_Test
+ * @dev Test various values of the resource metering config to ensure that deposits cannot be
+ *         broken by changing the config.
+ */
+contract OptimismPortalResourceFuzz_Test is Portal_Initializer {
+
+    /**
+     * @dev The max gas limit observed throughout this test. Setting this too high can cause
+     *      the test to take too long to run.
+     */
+    uint256 constant MAX_GAS_LIMIT = 30_000_000;
+
+    /**
+     * @dev Test that various values of the resource metering config will not break deposits.
+     */
+    function testFuzz_systemConfigDeposit_succeeds(
+        uint32 _maxResourceLimit,
+        uint8 _elasticityMultiplier,
+        uint8 _baseFeeMaxChangeDenominator,
+        uint32 _minimumBaseFee,
+        uint32 _systemTxMaxGas,
+        uint128 _maximumBaseFee,
+        uint64 _gasLimit,
+        uint64 _prevBoughtGas,
+        uint128 _prevBaseFee,
+        uint8 _blockDiff
+    ) external {
+        // Get the set system gas limit
+        uint64 gasLimit = systemConfig.gasLimit();
+        // Bound resource config
+        _maxResourceLimit = uint32(bound(_maxResourceLimit, 21000, MAX_GAS_LIMIT / 8));
+        _gasLimit = uint64(bound( _gasLimit,  21000, _maxResourceLimit));
+        _prevBaseFee = uint128(bound(_prevBaseFee, 0, 5 gwei));
+        // Prevent values that would cause reverts
+        vm.assume(gasLimit >= _gasLimit);
+        vm.assume(_minimumBaseFee < _maximumBaseFee);
+        vm.assume(_baseFeeMaxChangeDenominator > 1);
+        vm.assume(uint256(_maxResourceLimit) + uint256(_systemTxMaxGas) <= gasLimit);
+        vm.assume(_elasticityMultiplier > 0);
+        vm.assume(
+            ((_maxResourceLimit / _elasticityMultiplier) * _elasticityMultiplier) == _maxResourceLimit
+        );
+        _prevBoughtGas = uint64(bound(_prevBoughtGas, 0, _maxResourceLimit - _gasLimit));
+        _blockDiff = uint8(bound(_blockDiff, 0, 3));
+
+        // Create a resource config to mock the call to the system config with
+        ResourceMetering.ResourceConfig memory rcfg = ResourceMetering.ResourceConfig({
+            maxResourceLimit: _maxResourceLimit,
+            elasticityMultiplier: _elasticityMultiplier,
+            baseFeeMaxChangeDenominator: _baseFeeMaxChangeDenominator,
+            minimumBaseFee: _minimumBaseFee,
+            systemTxMaxGas: _systemTxMaxGas,
+            maximumBaseFee: _maximumBaseFee
+        });
+        vm.mockCall(
+            address(systemConfig),
+            abi.encodeWithSelector(systemConfig.resourceConfig.selector),
+            abi.encode(rcfg)
+        );
+
+        // Set the resource params
+        uint256 _prevBlockNum = block.number - _blockDiff;
+        vm.store(
+            address(op),
+            bytes32(uint256(1)),
+            bytes32((_prevBlockNum << 192) | (uint256(_prevBoughtGas) << 128) | _prevBaseFee)
+        );
+        // Ensure that the storage setting is correct
+        (uint128 prevBaseFee, uint64 prevBoughtGas, uint64 prevBlockNum) = op.params();
+        assertEq(prevBaseFee, _prevBaseFee);
+        assertEq(prevBoughtGas, _prevBoughtGas);
+        assertEq(prevBlockNum, _prevBlockNum);
+
+        // Do a deposit, should not revert
+        op.depositTransaction{ gas: MAX_GAS_LIMIT }({
+            _to: address(0x20),
+            _value: 0x40,
+            _gasLimit: _gasLimit,
+            _isCreation: false,
+            _data: hex""
+        });
+    }
+}

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

@@ -110,7 +110,7 @@ contract SystemConfig_Setters_TestFail is SystemConfig_Init {
             maximumBaseFee: 2 gwei
         });
         vm.prank(sysConf.owner());
-        vm.expectRevert("SystemConfig: denominator cannot be 0");
+        vm.expectRevert("SystemConfig: denominator must be larger than 1");
         sysConf.setResourceConfig(config);
     }