From c58587e0da55aedaf1087f524264a359740fb042 Mon Sep 17 00:00:00 2001 From: "A.L." Date: Sun, 7 Jul 2024 22:45:30 +0200 Subject: [PATCH] fix: collapse validation logic --- src/ReservoirPriceOracle.sol | 16 +++++++--------- src/libraries/OracleErrors.sol | 3 +-- src/libraries/{FlagsLib.sol => RoutesLib.sol} | 3 +-- test/unit/ReservoirPriceOracle.t.sol | 8 ++++---- test/unit/libraries/FlagsLib.t.sol | 18 +++++++++--------- 5 files changed, 22 insertions(+), 26 deletions(-) rename src/libraries/{FlagsLib.sol => RoutesLib.sol} (96%) diff --git a/src/ReservoirPriceOracle.sol b/src/ReservoirPriceOracle.sol index 0d76121..81a3549 100644 --- 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 index 3a70260..310f313 100644 --- 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 b/src/libraries/RoutesLib.sol similarity index 96% rename from src/libraries/FlagsLib.sol rename to src/libraries/RoutesLib.sol index 63f3419..0bc207c 100644 --- a/src/libraries/FlagsLib.sol +++ b/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 index 00fa54d..6082106 100644 --- 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 index e2f1021..3b1db8d 100644 --- 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); }