Gas Price Minimum should never be zero (#10909)

celo-org · Jan 30, 2024 · 2e27a3d · 2e27a3d
1 parent b167121
commit 2e27a3d
Show file tree

Hide file tree

Showing 3 changed files with 63 additions and 21 deletions.
diff --git a/packages/protocol/contracts-0.8/common/GasPriceMinimum.sol b/packages/protocol/contracts-0.8/common/GasPriceMinimum.sol
@@ -9,6 +9,7 @@ import "../../contracts/common/interfaces/ICeloVersionedContract.sol";
 import "../../contracts/common/FixidityLib.sol";
 import "./UsingRegistry.sol";
 import "../../contracts/stability/interfaces/ISortedOracles.sol";
+import "@openzeppelin/contracts8/utils/math/Math.sol";
 
 /**
  * @title Stores and provides gas price minimum for various currencies.
@@ -38,6 +39,7 @@ contract GasPriceMinimum is
   FixidityLib.Fraction public adjustmentSpeed;
 
   uint256 public baseFeeOpCodeActivationBlock;
+  uint256 public constant ABSOLUTE_MINIMAL_GAS_PRICE = 1;
 
   /**
    * @notice Sets initialized == true on implementation contracts
@@ -53,7 +55,7 @@ contract GasPriceMinimum is
    * @return Patch version of the contract.
    */
   function getVersionNumber() external pure returns (uint256, uint256, uint256, uint256) {
-    return (1, 2, 0, 0);
+    return (1, 2, 0, 1);
   }
 
   /**
@@ -150,12 +152,7 @@ contract GasPriceMinimum is
     }
   }
 
-  /**
-   * @notice Retrieve the current gas price minimum for a currency.
-   * @param tokenAddress The currency the gas price should be in (defaults to gold).
-   * @return current gas price minimum in the requested currency
-   */
-  function getGasPriceMinimum(address tokenAddress) external view returns (uint256) {
+  function _getGasPriceMinimum(address tokenAddress) private view returns (uint256) {
     if (
       tokenAddress == address(0) ||
       tokenAddress == registry.getAddressForOrDie(GOLD_TOKEN_REGISTRY_ID)
@@ -172,6 +169,21 @@ contract GasPriceMinimum is
     }
   }
 
+  /**
+   * @notice Retrieve the current gas price minimum for a currency.
+   * When caled for 0x0 or Celo address, it returns gasPriceMinimum().
+   * For other addresses it returns gasPriceMinimum() mutiplied by 
+   * the SortedOracles median of the token. It does not check tokenAddress is a valid fee currency.
+   * this function will never returns values less than ABSOLUTE_MINIMAL_GAS_PRICE.
+   * If Oracle rate doesn't exist, it returns ABSOLUTE_MINIMAL_GAS_PRICE.
+   * @dev This functions assumes one unit of token has 18 digits.
+   * @param tokenAddress The currency the gas price should be in (defaults to Celo).
+   * @return current gas price minimum in the requested currency
+   */
+  function getGasPriceMinimum(address tokenAddress) external view returns (uint256) {
+    return Math.max(_getGasPriceMinimum(tokenAddress), ABSOLUTE_MINIMAL_GAS_PRICE);
+  }
+
   /**
    * @notice Adjust the gas price minimum based on governable parameters
    * and block congestion.

diff --git a/packages/protocol/contracts/stability/test/MockSortedOracles.sol b/packages/protocol/contracts/stability/test/MockSortedOracles.sol
@@ -1,4 +1,4 @@
-pragma solidity ^0.5.13;
+pragma solidity >=0.5.13 <0.9.0;
 
 /**
  * @title A mock SortedOracles for testing.
@@ -21,7 +21,7 @@ contract MockSortedOracles {
 
   function setMedianTimestampToNow(address token) external {
     // solhint-disable-next-line not-rely-on-time
-    medianTimestamp[token] = uint128(now);
+    medianTimestamp[token] = uint128(block.timestamp);
   }
 
   function setNumRates(address token, uint256 rate) external {

diff --git a/packages/protocol/test-sol/common/GasPriceMinimum.t.sol b/packages/protocol/test-sol/common/GasPriceMinimum.t.sol
@@ -6,6 +6,8 @@ import "celo-foundry-8/Test.sol";
 import "@celo-contracts/common/FixidityLib.sol";
 
 import "@celo-contracts/common/interfaces/IRegistry.sol";
+import "@celo-contracts/stability/interfaces/ISortedOracles.sol";
+import "@celo-contracts/stability/test/MockSortedOracles.sol";
 
 // Contract to test
 import "@celo-contracts-8/common/GasPriceMinimum.sol";
@@ -15,8 +17,10 @@ contract GasPriceMinimumTest is Test {
 
   IRegistry registry;
   GasPriceMinimum public gasPriceMinimum;
+  MockSortedOracles sortedOracles;
   address owner;
   address nonOwner;
+  address celoToken;
 
   uint256 gasPriceMinimumFloor = 100;
   uint256 initialGasPriceMinimum = gasPriceMinimumFloor;
@@ -35,13 +39,21 @@ contract GasPriceMinimumTest is Test {
   function setUp() public virtual {
     owner = address(this);
     nonOwner = actor("nonOwner");
+    celoToken = actor("CeloToken");
 
     deployCodeTo("Registry.sol", abi.encode(false), registryAddress);
+
+    // deployCodeTo("SortedOracles.sol", abi.encode(true), sortedOracleAddress);
+    // fails with `data did not match any variant of untagged enum Bytecode at line 822 column 3]`
+    sortedOracles = new MockSortedOracles();
+
     gasPriceMinimum = new GasPriceMinimum(true);
 
     registry = IRegistry(registryAddress);
 
     registry.setAddressFor("GasPriceMinimum", address(gasPriceMinimum));
+    registry.setAddressFor("SortedOracles", address(sortedOracles));
+    registry.setAddressFor("GoldToken", celoToken);
 
     gasPriceMinimum.initialize(
       registryAddress,
@@ -53,15 +65,11 @@ contract GasPriceMinimumTest is Test {
   }
 }
 
-contract GasPriceMinimumInitialize is GasPriceMinimumTest {
+contract GasPriceMinimumTest_initialize is GasPriceMinimumTest {
   function test_shouldHaveSetOwner() public {
     assertEq(gasPriceMinimum.owner(), owner);
   }
 
-  function test_shouldSetTheGasPriceMinimum() public {
-    assertEq(gasPriceMinimum.getGasPriceMinimum(address(0)), initialGasPriceMinimum);
-  }
-
   function test_shouldHaveTargetDensity() public {
     assertEq(gasPriceMinimum.targetDensity(), targetDensity);
   }
@@ -86,7 +94,29 @@ contract GasPriceMinimumInitialize is GasPriceMinimumTest {
   }
 }
 
-contract GasPriceMinimumSetAdjustmentSpeed is GasPriceMinimumTest {
+contract GasPriceMinimumTest_getGasPriceMinimum is GasPriceMinimumTest {
+  uint256 public constant ABSOLUTE_MINIMAL_GAS_PRICE = 1;
+  address whateverAddress = address(0x420);
+
+  function test_shouldGetGasPriceMinimumForCelo() public {
+    assertEq(gasPriceMinimum.getGasPriceMinimum(celoToken), initialGasPriceMinimum);
+  }
+  function test_shouldGetGasPriceMinimumFor0x0asCelo() public {
+    assertEq(gasPriceMinimum.getGasPriceMinimum(address(0)), initialGasPriceMinimum);
+  }
+
+  function test_shouldReturnAbsoluteMinInsteadOfZero() public {
+    sortedOracles.setMedianRate(whateverAddress, 1);
+    assertEq(gasPriceMinimum.getGasPriceMinimum(whateverAddress), ABSOLUTE_MINIMAL_GAS_PRICE);
+  }
+
+  function test_shouldReturnTheRightRateForToken() public {
+    sortedOracles.setMedianRate(whateverAddress, 2e24);
+    assertEq(gasPriceMinimum.getGasPriceMinimum(whateverAddress), initialGasPriceMinimum * 2);
+  }
+}
+
+contract GasPriceMinimumTest_setAdjustmentSpeed is GasPriceMinimumTest {
   using FixidityLib for FixidityLib.Fraction;
 
   uint256 newAdjustmentSpeed = FixidityLib.newFixedFraction(1, 3).unwrap();
@@ -97,7 +127,7 @@ contract GasPriceMinimumSetAdjustmentSpeed is GasPriceMinimumTest {
     assertEq(gasPriceMinimum.adjustmentSpeed(), newAdjustmentSpeed);
   }
 
-  function test_shouldEmitAdjustmentSpeedSetEvent() public {
+  function test_Emits_AdjustmentSpeedSetEvent() public {
     vm.expectEmit(true, false, false, false);
     emit AdjustmentSpeedSet(newAdjustmentSpeed);
     gasPriceMinimum.setAdjustmentSpeed(newAdjustmentSpeed);
@@ -115,7 +145,7 @@ contract GasPriceMinimumSetAdjustmentSpeed is GasPriceMinimumTest {
   }
 }
 
-contract GasPriceMinimumSetTargetDensity is GasPriceMinimumTest {
+contract GasPriceMinimumTest_setTargetDensity is GasPriceMinimumTest {
   using FixidityLib for FixidityLib.Fraction;
 
   uint256 newTargetDensity = FixidityLib.newFixedFraction(1, 3).unwrap();
@@ -125,7 +155,7 @@ contract GasPriceMinimumSetTargetDensity is GasPriceMinimumTest {
     assertEq(gasPriceMinimum.targetDensity(), newTargetDensity);
   }
 
-  function test_ShouldEmitTargetDensitySetEvent() public {
+  function test_Emits_TargetDensitySetEvent() public {
     vm.expectEmit(true, true, true, true);
     emit TargetDensitySet(newTargetDensity);
     gasPriceMinimum.setTargetDensity(newTargetDensity);
@@ -143,7 +173,7 @@ contract GasPriceMinimumSetTargetDensity is GasPriceMinimumTest {
   }
 }
 
-contract GasPriceMinimumSetGasPriceMinimumFloor is GasPriceMinimumTest {
+contract GasPriceMinimumTest_setGasPriceMinimumFloor is GasPriceMinimumTest {
   uint256 newGasPriceMinimumFloor = 150;
 
   function test_ShouldSetGasPriceMinimumFloor() public {
@@ -152,7 +182,7 @@ contract GasPriceMinimumSetGasPriceMinimumFloor is GasPriceMinimumTest {
     assertEq(gasPriceMinimum.gasPriceMinimumFloor(), newGasPriceMinimumFloor);
   }
 
-  function test_emitsGasPriceMinimumFloorSet() public {
+  function test_Emits_GasPriceMinimumFloorSet() public {
     vm.expectEmit(true, true, true, true);
     emit GasPriceMinimumFloorSet(newGasPriceMinimumFloor);
     gasPriceMinimum.setGasPriceMinimumFloor(newGasPriceMinimumFloor);
@@ -170,7 +200,7 @@ contract GasPriceMinimumSetGasPriceMinimumFloor is GasPriceMinimumTest {
   }
 }
 
-contract GasPriceMinimumGetUpdatedGasPriceMinimum is GasPriceMinimumTest {
+contract GasPriceMinimumTest_setUpdatedGasPriceMinimum is GasPriceMinimumTest {
   using FixidityLib for FixidityLib.Fraction;
   uint256 nonce = 0;