fix: addresses #7

reservoir-labs · Dec 7, 2024 · 74defc0 · 74defc0
1 parent adbe551
commit 74defc0
Show file tree

Hide file tree

Showing 8 changed files with 110 additions and 72 deletions.
diff --git a/src/ReservoirPriceOracle.sol b/src/ReservoirPriceOracle.sol
@@ -207,21 +207,15 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar
         view
         returns (uint256 rReward)
     {
-        // SAFETY: this mul will not overflow as 0 < `aRewardThreshold` <= `Constants.BP_SCALE`, as checked by `setRoute`
-        uint256 lRewardThresholdWAD;
-        unchecked {
-            lRewardThresholdWAD = aRewardThreshold * Constants.WAD / Constants.BP_SCALE;
-        }
-
         uint256 lPercentDiff = aPrevPrice.calcPercentageDiff(aNewPrice);
 
         // SAFETY: this mul will not overflow even in extreme cases of `block.basefee`.
         unchecked {
-            if (lPercentDiff < lRewardThresholdWAD) {
+            if (lPercentDiff < aRewardThreshold) {
                 return 0;
             }
             // payout max reward
-            else if (lPercentDiff >= lRewardThresholdWAD * MAX_REWARD_MULTIPLIER) {
+            else if (lPercentDiff >= aRewardThreshold * MAX_REWARD_MULTIPLIER) {
                 // N.B. Revisit this whenever deployment on a new chain is needed
                 //
                 // we use `block.basefee` instead of `ArbGasInfo::getMinimumGasPrice()`
@@ -230,7 +224,8 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar
                 // congestion
                 rReward = block.basefee * rewardGasAmount * MAX_REWARD_MULTIPLIER;
             } else {
-                rReward = block.basefee * rewardGasAmount * lPercentDiff / lRewardThresholdWAD; // denominator is never 0
+                // denominator is never 0 as checked by `setRoute`
+                rReward = block.basefee * rewardGasAmount * lPercentDiff / aRewardThreshold;
             }
         }
     }
@@ -300,7 +295,7 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar
 
     // Calculate the storage slot for this intermediate segment and read it to see if there is an existing
     // route. If there isn't an existing route, we create one as well.
-    function _checkAndPopulateIntermediateRoute(address aTokenA, address aTokenB, uint16 aBpMaxReward) private {
+    function _checkAndPopulateIntermediateRoute(address aTokenA, address aTokenB, uint64 aBpMaxReward) private {
         (address lToken0, address lToken1) = Utils.sortTokens(aTokenA, aTokenB);
 
         bytes32 lSlot = Utils.calculateSlot(lToken0, lToken1);
@@ -312,9 +307,9 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar
             address[] memory lIntermediateRoute = new address[](2);
             lIntermediateRoute[0] = lToken0;
             lIntermediateRoute[1] = lToken1;
-            uint16[] memory asd = new uint16[](1);
-            asd[0] = aBpMaxReward;
-            setRoute(lToken0, lToken1, lIntermediateRoute, asd);
+            uint64[] memory lRewardThreshold = new uint64[](1);
+            lRewardThreshold[0] = aBpMaxReward;
+            setRoute(lToken0, lToken1, lIntermediateRoute, lRewardThreshold);
         }
     }
 
@@ -488,7 +483,7 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar
     /// @param aToken1 Address of the higher token.
     /// @param aRoute Path with which the price between aToken0 and aToken1 should be derived.
     /// @param aRewardThresholds Array of basis points at and beyond which a reward is applicable for a price update.
-    function setRoute(address aToken0, address aToken1, address[] memory aRoute, uint16[] memory aRewardThresholds)
+    function setRoute(address aToken0, address aToken1, address[] memory aRoute, uint64[] memory aRewardThresholds)
         public
         onlyOwner
     {
@@ -511,7 +506,7 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar
 
             uint256 lRewardThreshold = aRewardThresholds[0];
             require(
-                lRewardThreshold <= Constants.BP_SCALE && lRewardThreshold != 0, OracleErrors.InvalidRewardThreshold()
+                lRewardThreshold <= Constants.WAD && lRewardThreshold != 0, OracleErrors.InvalidRewardThreshold()
             );
 
             bytes32 lData = RoutesLib.packSimplePrice(lDiff, 0, lRewardThreshold);

diff --git a/src/libraries/Constants.sol b/src/libraries/Constants.sol
@@ -6,10 +6,9 @@ library Constants {
     //                                       CONSTANTS                                           //
     ///////////////////////////////////////////////////////////////////////////////////////////////
 
+    uint64 public constant WAD = 1e18;
     uint256 public constant MAX_TWAP_PERIOD = 1 hours;
     uint256 public constant MAX_ROUTE_LENGTH = 4;
-    uint256 public constant WAD = 1e18;
     uint256 public constant MAX_SUPPORTED_PRICE = type(uint128).max;
     uint256 public constant MAX_AMOUNT_IN = type(uint128).max;
-    uint16 public constant BP_SCALE = 1e4;
 }
diff --git a/src/libraries/RoutesLib.sol b/src/libraries/RoutesLib.sol
@@ -29,14 +29,14 @@ library RoutesLib {
 
     // Assumes that aDecimalDifference is between -18 and 18
     // Assumes that aPrice is between 1 and `Constants.MAX_SUPPORTED_PRICE`
-    // Assumes that aRewardThreshold is <= `Constants.BP_SCALE`
+    // Assumes that aRewardThreshold is between 1 and `Constants.WAD`
     function packSimplePrice(int256 aDecimalDifference, uint256 aPrice, uint256 aRewardThreshold)
         internal
         pure
         returns (bytes32 rPacked)
     {
         bytes32 lDecimalDifferenceRaw = bytes1(uint8(int8(aDecimalDifference)));
-        bytes32 lRewardThreshold = bytes2(uint16(aRewardThreshold));
+        bytes32 lRewardThreshold = bytes8(uint64(aRewardThreshold));
         rPacked = FLAG_SIMPLE_PRICE | lDecimalDifferenceRaw >> 8 | lRewardThreshold >> 16 | bytes32(aPrice);
     }
 
@@ -61,12 +61,12 @@ library RoutesLib {
     }
 
     function getPrice(bytes32 aData) internal pure returns (uint256 rPrice) {
-        rPrice = uint256(aData & 0x00000000ffffffffffffffffffffffffffffffffffffffffffffffffffffffff);
+        rPrice = uint256(aData & 0x00000000000000000000ffffffffffffffffffffffffffffffffffffffffffff);
     }
 
-    function getRewardThreshold(bytes32 aData) internal pure returns (uint16 rRewardThreshold) {
+    function getRewardThreshold(bytes32 aData) internal pure returns (uint64 rRewardThreshold) {
         rRewardThreshold =
-            uint16(uint256((aData & 0x0000ffff00000000000000000000000000000000000000000000000000000000) >> 224));
+            uint64(uint256((aData & 0x0000ffffffffffffffff00000000000000000000000000000000000000000000) >> 176));
     }
 
     function getTokenFirstWord(bytes32 aData) internal pure returns (address rToken) {

diff --git a/src/libraries/Utils.sol b/src/libraries/Utils.sol
@@ -14,6 +14,7 @@ library Utils {
 
     /// @dev Assumes that `aOriginal` and `aNew` is less than or equal to
     /// `Constants.MAX_SUPPORTED_PRICE`. So multiplication by 1e18 will not overflow.
+    /// 1e18 indicates a 100% difference. i.e. a doubling in price
     function calcPercentageDiff(uint256 aOriginal, uint256 aNew) internal pure returns (uint256) {
         unchecked {
             if (aOriginal == 0) return 0;

diff --git a/test/large/ReservoirPriceOracleLarge.t.sol b/test/large/ReservoirPriceOracleLarge.t.sol
@@ -79,7 +79,7 @@ contract ReservoirPriceOracleLargeTest is ReservoirPriceOracleTest {
         }
 
         uint16[] memory lRewardThresholds = new uint16[](3);
-        lRewardThresholds[0] = lRewardThresholds[1] = lRewardThresholds[2] = Constants.BP_SCALE;
+        lRewardThresholds[0] = lRewardThresholds[1] = lRewardThresholds[2] = Constants.WAD;
 
         _oracle.setRoute(lRoute[0], lRoute[3], lRoute, lRewardThresholds);
         _writePriceCache(