GasPriceOracle: remove saturating math from getOperatorFee (jovian) (#17913)

* GasPriceOracle: remove saturating math fro getOperatorFee (isthmus and jovian)

Extend test coverage to edge cases

* restore Isthmys formula

* fix test

* use library

* lint

* semver bump

* modify semver and regenerate lock file

Author: geoknee

packages/contracts-bedrock/snapshots/semver-lock.json

@@ -60,8 +60,8 @@
     "sourceCodeHash": "0x6d137fef431d75a8bf818444915fc39c8b1d93434a9af9971d96fb3170bc72b7"
   },
   "src/L2/GasPriceOracle.sol:GasPriceOracle": {
-    "initCodeHash": "0xedc721584d43025c515186d4d5879d2b8e4abaf3a69181b99b6bf90c684df442",
-    "sourceCodeHash": "0x0074761fc0f893a2418b4e1197c7f29ee59f407df31b99c420bf2fd82d14d583"
+    "initCodeHash": "0xf72c23d9c3775afd7b645fde429d09800622d329116feb5ff9829634655123ca",
+    "sourceCodeHash": "0xb4d1bf3669ba87bbeaf4373145c7e1490478c4a05ba4838a524aa6f0ce7348a6"
   },
   "src/L2/L1Block.sol:L1Block": {
     "initCodeHash": "0x1f054ff228ecad7f51772dd25084469192f7a33c522b87cd46ec5558d3c46aec",

packages/contracts-bedrock/src/L2/GasPriceOracle.sol

@@ -30,8 +30,8 @@ contract GasPriceOracle is ISemver {
     uint256 public constant DECIMALS = 6;
 
     /// @notice Semantic version.
-    /// @custom:semver 1.5.0
-    string public constant version = "1.5.0";
+    /// @custom:semver 1.6.0
+    string public constant version = "1.6.0";
 
     /// @notice This is the intercept value for the linear regression used to estimate the final size of the
     ///         compressed transaction.
@@ -224,7 +224,7 @@ contract GasPriceOracle is ISemver {
         uint256 operatorConstant = IL1Block(Predeploys.L1_BLOCK_ATTRIBUTES).operatorFeeConstant();
 
         if (isJovian) {
-            return Arithmetic.saturatingAdd(Arithmetic.saturatingMul(_gasUsed, operatorScalar) * 100, operatorConstant);
+            return _gasUsed * operatorScalar * 100 + operatorConstant;
         } else {
             return Arithmetic.saturatingAdd(Arithmetic.saturatingMul(_gasUsed, operatorScalar) / 1e6, operatorConstant);
         }

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

@@ -7,6 +7,7 @@ import { Fork } from "scripts/libraries/Config.sol";
 
 // Libraries
 import { Encoding } from "src/libraries/Encoding.sol";
+import { stdError } from "forge-std/Test.sol";
 
 contract GasPriceOracle_Test is CommonTest {
     address depositor;
@@ -26,6 +27,10 @@ contract GasPriceOracle_Test is CommonTest {
     uint32 constant operatorFeeScalar = 4_000_000;
     uint64 constant operatorFeeConstant = 300;
 
+    uint256 constant MAX_UINT256 = type(uint256).max;
+    uint64 constant MAX_UINT64 = type(uint64).max;
+    uint32 constant MAX_UINT32 = type(uint32).max;
+
     /// @dev Sets up the test suite.
     function setUp() public virtual override {
         super.setUp();
@@ -447,6 +452,26 @@ contract GasPriceOracleJovian_Test is GasPriceOracle_Test {
         assertEq(gasPriceOracle.isJovian(), true, "Jovian activation failed");
     }
 
+    function _setOperatorFeeParams(uint32 _operatorFeeScalar, uint64 _operatorFeeConstant) internal {
+        vm.prank(depositor);
+        (bool success,) = address(l1Block).call(
+            Encoding.encodeSetL1BlockValuesIsthmus(
+                baseFeeScalar,
+                blobBaseFeeScalar,
+                sequenceNumber,
+                timestamp,
+                number,
+                baseFee,
+                blobBaseFee,
+                hash,
+                batcherHash,
+                _operatorFeeScalar,
+                _operatorFeeConstant
+            )
+        );
+        require(success, "GasPriceOracleJovian_Test: L1Block setup failed");
+    }
+
     /// @dev Tests that `operatorFee` is set correctly using the new Jovian formula (multiply by 100).
     function test_getOperatorFee_succeeds() external {
         _activateJovian();
@@ -468,19 +493,67 @@ contract GasPriceOracleJovian_Test is GasPriceOracle_Test {
     }
 
     /// @dev Tests the transition from Isthmus formula to Jovian formula.
-    function test_formulaTransition_succeeds() external {
-        // Check Isthmus formula (divide by 1e6)
+    function test_formulaTransition_edgeCases_works() external {
+        // Check Isthmus formula with a low gasUsed value (divide by 1e6)
+        _setOperatorFeeParams(operatorFeeScalar, operatorFeeConstant);
         uint256 isthmusFee = gasPriceOracle.getOperatorFee(10);
-        assertEq(isthmusFee, 10 * operatorFeeScalar / 1e6 + operatorFeeConstant);
+        assertEq(
+            isthmusFee,
+            uint256(10) * operatorFeeScalar / 1e6 + operatorFeeConstant,
+            "Isthmus operator fee incorrect with 10 gas used"
+        );
+
+        // Use maximum values permitted by data types for scalars.
+        // Use maximum value for gasUsed according to client implementations.
+        // Assert that the fee is as expected (no overflow).
+        _setOperatorFeeParams(MAX_UINT32, MAX_UINT64);
+        isthmusFee = gasPriceOracle.getOperatorFee(MAX_UINT64);
+        assertEq(
+            isthmusFee,
+            uint256(MAX_UINT64) * MAX_UINT32 / 1e6 + MAX_UINT64,
+            "Isthmus operator fee incorrect with max uint64 gas used"
+        );
+
+        // Show that the math saturates if the maximum
+        // value for gasUsed (according to data type) is used.
+        _setOperatorFeeParams(1e6, 1);
+        uint256 saturatedIsthmusFee = gasPriceOracle.getOperatorFee(MAX_UINT256);
+        assertEq(
+            saturatedIsthmusFee,
+            115792089237316195423570985008687907853269984665640564039457584007913130,
+            "Incorrect value for fee under Isthmus (saturating arithmetic triggered)"
+        );
 
         // Activate Jovian
         _activateJovian();
 
-        // Check Jovian formula (multiply by 100)
+        // Check Jovian formula with a low gasUsed value (multiply by 100)
+        _setOperatorFeeParams(operatorFeeScalar, operatorFeeConstant);
         uint256 jovianFee = gasPriceOracle.getOperatorFee(10);
-        assertEq(jovianFee, 10 * operatorFeeScalar * 100 + operatorFeeConstant);
+        assertEq(
+            jovianFee,
+            uint256(10) * operatorFeeScalar * 100 + operatorFeeConstant,
+            "Jovian operator fee incorrect with 10 gas used"
+        );
+
+        // Use maximum values permitted by data types for scalars.
+        // Use maximum value for gasUsed according to client implementations.
+        // Assert that the fee is as expected (no overflow).
+        _setOperatorFeeParams(MAX_UINT32, MAX_UINT64);
+        jovianFee = gasPriceOracle.getOperatorFee(MAX_UINT64);
+        assertEq(
+            jovianFee,
+            uint256(MAX_UINT64) * MAX_UINT32 * 100 + MAX_UINT64,
+            "Jovian operator fee incorrect with max uint64 gas used"
+        );
+
+        // Show that a revert is possible if the maximum
+        // value for gasUsed (according to data type) is used.
+        _setOperatorFeeParams(1, 1);
+        vm.expectRevert(stdError.arithmeticError);
+        gasPriceOracle.getOperatorFee(MAX_UINT256);
 
         // Verify the fee increased significantly
-        assertGt(jovianFee, isthmusFee);
+        assertGt(jovianFee, isthmusFee, "Jovian formula fee should be greater than Isthmus formula fee");
     }
 }