fix: collapse validation logic

reservoir-labs · Jul 7, 2024 · c58587e · c58587e
1 parent 821c0fa
commit c58587e
Show file tree

Hide file tree

Showing 5 changed files with 22 additions and 26 deletions.
diff --git a/src/ReservoirPriceOracle.sol b/src/ReservoirPriceOracle.sol
@@ -18,12 +18,12 @@ import { ReentrancyGuard } from "lib/amm-core/lib/solmate/src/utils/ReentrancyGu
 import { FixedPointMathLib } from "lib/amm-core/lib/solady/src/utils/FixedPointMathLib.sol";
 import { LibSort } from "lib/solady/src/utils/LibSort.sol";
 import { Constants } from "src/libraries/Constants.sol";
-import { FlagsLib } from "src/libraries/FlagsLib.sol";
+import { RoutesLib } from "src/libraries/RoutesLib.sol";
 
 contract ReservoirPriceOracle is IPriceOracle, IReservoirPriceOracle, Owned(msg.sender), ReentrancyGuard {
     using FixedPointMathLib for uint256;
     using LibSort for address[];
-    using FlagsLib for *;
+    using RoutesLib for bytes32;
     using QueryProcessor for ReservoirPair;
 
     ///////////////////////////////////////////////////////////////////////////////////////////////
@@ -197,9 +197,7 @@ contract ReservoirPriceOracle is IPriceOracle, IReservoirPriceOracle, Owned(msg.
     }
 
     function _validateTokens(address aToken0, address aToken1) private pure {
-        // REVIEW: Can be collapsed into `if (aToken1 <= aToken1) revert`.
-        if (aToken0 == aToken1) revert OracleErrors.SameToken();
-        if (aToken1 < aToken0) revert OracleErrors.TokensUnsorted();
+        if (aToken1 <= aToken0) revert OracleErrors.InvalidTokensProvided();
     }
 
     function _getTimeWeightedAverageSingle(OracleAverageQuery memory aQuery) internal view returns (uint256 rResult) {
@@ -347,7 +345,7 @@ contract ReservoirPriceOracle is IPriceOracle, IReservoirPriceOracle, Owned(msg.
 
         int256 lDiff = lData.getDecimalDifference();
 
-        lData = FlagsLib.packSimplePrice(lDiff, aNewPrice);
+        lData = RoutesLib.packSimplePrice(lDiff, aNewPrice);
         assembly {
             sstore(lSlot, lData)
         }
@@ -513,7 +511,7 @@ contract ReservoirPriceOracle is IPriceOracle, IReservoirPriceOracle, Owned(msg.
 
             int256 lDiff = int256(lToken1Decimals) - int256(lToken0Decimals);
 
-            bytes32 lData = FlagsLib.packSimplePrice(lDiff, 0);
+            bytes32 lData = RoutesLib.packSimplePrice(lDiff, 0);
             assembly {
                 // Write data to storage.
                 sstore(lSlot, lData)
@@ -525,12 +523,12 @@ contract ReservoirPriceOracle is IPriceOracle, IReservoirPriceOracle, Owned(msg.
             address lThirdToken = aRoute[2];
 
             if (lRouteLength == 3) {
-                bytes32 lData = FlagsLib.pack2HopRoute(lSecondToken);
+                bytes32 lData = RoutesLib.pack2HopRoute(lSecondToken);
                 assembly {
                     sstore(lSlot, lData)
                 }
             } else if (lRouteLength == 4) {
-                (bytes32 lFirstWord, bytes32 lSecondWord) = FlagsLib.pack3HopRoute(lSecondToken, lThirdToken);
+                (bytes32 lFirstWord, bytes32 lSecondWord) = RoutesLib.pack3HopRoute(lSecondToken, lThirdToken);
 
                 // Write two words to storage.
                 assembly {

diff --git a/src/libraries/OracleErrors.sol b/src/libraries/OracleErrors.sol
@@ -7,11 +7,10 @@ library OracleErrors {
     error IncorrectTokensDesignatePair();
     error InvalidRoute();
     error InvalidRouteLength();
+    error InvalidTokensProvided();
     error InvalidTwapPeriod();
     error NoDesignatedPair();
     error PriceDeviationThresholdTooHigh();
-    error SameToken();
-    error TokensUnsorted();
     error UnsupportedTokenDecimals();
 
     // query errors

diff --git a/src/libraries/FlagsLib.sol → src/libraries/RoutesLib.sol b/src/libraries/FlagsLib.sol → src/libraries/RoutesLib.sol
@@ -1,8 +1,7 @@
 // SPDX-License-Identifier: GPL-3.0-or-later
 pragma solidity ^0.8.0;
 
-// REVIEW: Rename `FlagsLib` -> `RoutesLib` as all operations are routes related?
-library FlagsLib {
+library RoutesLib {
     bytes32 public constant FLAG_UNINITIALIZED = bytes32(hex"00");
     bytes32 public constant FLAG_SIMPLE_PRICE = bytes32(hex"01");
     bytes32 public constant FLAG_2_HOP_ROUTE = bytes32(hex"02");

diff --git a/test/unit/ReservoirPriceOracle.t.sol b/test/unit/ReservoirPriceOracle.t.sol
@@ -13,7 +13,7 @@ import {
     ReservoirPriceOracle,
     IERC20,
     IPriceOracle,
-    FlagsLib
+    RoutesLib
 } from "src/ReservoirPriceOracle.sol";
 import { Bytes32Lib } from "amm-core/libraries/Bytes32.sol";
 import { EnumerableSetLib } from "lib/solady/src/utils/EnumerableSetLib.sol";
@@ -23,7 +23,7 @@ import { StubERC4626 } from "test/mock/StubERC4626.sol";
 
 contract ReservoirPriceOracleTest is BaseTest {
     using Utils for *;
-    using FlagsLib for *;
+    using RoutesLib for *;
     using Bytes32Lib for *;
     using EnumerableSetLib for EnumerableSetLib.AddressSet;
     using FixedPointMathLib for uint256;
@@ -941,7 +941,7 @@ contract ReservoirPriceOracleTest is BaseTest {
         lRoute[1] = lToken1;
 
         // act & assert
-        vm.expectRevert(OracleErrors.SameToken.selector);
+        vm.expectRevert(OracleErrors.InvalidTokensProvided.selector);
         _oracle.setRoute(lToken0, lToken1, lRoute);
     }
 
@@ -954,7 +954,7 @@ contract ReservoirPriceOracleTest is BaseTest {
         lRoute[1] = lToken1;
 
         // act & assert
-        vm.expectRevert(OracleErrors.TokensUnsorted.selector);
+        vm.expectRevert(OracleErrors.InvalidTokensProvided.selector);
         _oracle.setRoute(lToken0, lToken1, lRoute);
     }
 

diff --git a/test/unit/libraries/FlagsLib.t.sol b/test/unit/libraries/FlagsLib.t.sol
@@ -3,18 +3,18 @@ pragma solidity ^0.8.0;
 
 import { Test, console2, stdError } from "forge-std/Test.sol";
 
-import { FlagsLib } from "src/libraries/FlagsLib.sol";
+import { RoutesLib } from "src/libraries/RoutesLib.sol";
 
-contract FlagsLibTest is Test {
-    using FlagsLib for bytes32;
-    using FlagsLib for int256;
+contract RoutesLibTest is Test {
+    using RoutesLib for bytes32;
+    using RoutesLib for int256;
 
     function testIsCompositeRoute() external pure {
         // arrange
-        bytes32 lUninitialized = FlagsLib.FLAG_UNINITIALIZED;
-        bytes32 l1HopRoute = FlagsLib.FLAG_SIMPLE_PRICE;
-        bytes32 l2HopRoute = FlagsLib.FLAG_2_HOP_ROUTE;
-        bytes32 l3HopRoute = FlagsLib.FLAG_3_HOP_ROUTE;
+        bytes32 lUninitialized = RoutesLib.FLAG_UNINITIALIZED;
+        bytes32 l1HopRoute = RoutesLib.FLAG_SIMPLE_PRICE;
+        bytes32 l2HopRoute = RoutesLib.FLAG_2_HOP_ROUTE;
+        bytes32 l3HopRoute = RoutesLib.FLAG_3_HOP_ROUTE;
 
         // act & assert
         assertTrue(l2HopRoute.isCompositeRoute());
@@ -43,7 +43,7 @@ contract FlagsLibTest is Test {
         bytes32 lResult = int256(aDiff).packSimplePrice(lPrice);
 
         // assert
-        assertEq(lResult[0], FlagsLib.FLAG_SIMPLE_PRICE);
+        assertEq(lResult[0], RoutesLib.FLAG_SIMPLE_PRICE);
         assertEq(lResult[1], bytes1(uint8(aDiff)));
         assertEq(lResult.getPrice(), lPrice);
     }