From 2e27a3dc4ab7cf20723ec18f6215512769866558 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mart=C3=ADn=20Volpe?= Date: Tue, 30 Jan 2024 09:27:03 -0300 Subject: [PATCH] Gas Price Minimum should never be zero (#10909) --- .../contracts-0.8/common/GasPriceMinimum.sol | 26 ++++++--- .../stability/test/MockSortedOracles.sol | 4 +- .../test-sol/common/GasPriceMinimum.t.sol | 54 ++++++++++++++----- 3 files changed, 63 insertions(+), 21 deletions(-) diff --git a/packages/protocol/contracts-0.8/common/GasPriceMinimum.sol b/packages/protocol/contracts-0.8/common/GasPriceMinimum.sol index f09dc6f8859..bc08d26c9d6 100644 --- 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 index 6b35d2939bc..f3faeb0e55e 100644 --- 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 index ec3c67f6526..425f77008f8 100644 --- 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;