From 26353de167d85b1d830055525938d1b41b14ef99 Mon Sep 17 00:00:00 2001 From: OliverNChalk <11343499+OliverNChalk@users.noreply.github.com> Date: Wed, 19 Jun 2024 22:59:48 -0500 Subject: [PATCH] review: initial feedback and questions (#11) --- .gas-snapshot | 128 ++++++++++----------- README.md | 90 +++++++++++++++ package.json | 2 +- src/ReservoirPriceOracle.sol | 115 ++++++++---------- src/libraries/OracleErrors.sol | 1 + test/large/ReservoirPriceOracleLarge.t.sol | 14 ++- test/unit/ReservoirPriceOracle.t.sol | 25 ++-- 7 files changed, 221 insertions(+), 154 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index ecbc10e..6448500 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -1,77 +1,77 @@ FlagsLibTest:testGetDecimalDifference() (gas: 3974) FlagsLibTest:testIsCompositeRoute() (gas: 4341) FlagsLibTest:testPackSimplePrice(int8,uint256) (runs: 256, μ: 7791, ~: 7555) -QueryProcessorTest:testFindNearestSample_CanFindExactValue(uint32,uint256,uint256,uint256) (runs: 256, μ: 64954402, ~: 72638370) -QueryProcessorTest:testFindNearestSample_CanFindIntermediateValue(uint32,uint256,uint256,uint256) (runs: 256, μ: 65183381, ~: 74353868) +QueryProcessorTest:testFindNearestSample_CanFindExactValue(uint32,uint256,uint256,uint256) (runs: 256, μ: 66028796, ~: 73578593) +QueryProcessorTest:testFindNearestSample_CanFindIntermediateValue(uint32,uint256,uint256,uint256) (runs: 256, μ: 65574840, ~: 74107585) QueryProcessorTest:testFindNearestSample_NotInitialized() (gas: 8937393461068805977) -QueryProcessorTest:testFindNearestSample_OneSample(uint256) (runs: 256, μ: 80330, ~: 80360) +QueryProcessorTest:testFindNearestSample_OneSample(uint256) (runs: 256, μ: 80329, ~: 80360) QueryProcessorTest:testGetInstantValue() (gas: 124248) QueryProcessorTest:testGetInstantValue_NotInitialized(uint256) (runs: 256, μ: 19397, ~: 19397) -QueryProcessorTest:testGetInstantValue_NotInitialized_BeyondBufferSize(uint8,uint16) (runs: 256, μ: 68389670, ~: 68389600) -QueryProcessorTest:testGetPastAccumulator_BufferEmpty(uint8) (runs: 256, μ: 27026, ~: 27087) -QueryProcessorTest:testGetPastAccumulator_ExactMatch(uint32,uint256,uint256,uint16) (runs: 256, μ: 71445048, ~: 79233749) -QueryProcessorTest:testGetPastAccumulator_ExactMatch_LatestAccumulator(uint32,uint256,uint256) (runs: 256, μ: 70305328, ~: 78329546) -QueryProcessorTest:testGetPastAccumulator_ExactMatch_OldestAccumulator(uint32,uint256,uint256) (runs: 256, μ: 70335354, ~: 78361146) -QueryProcessorTest:testGetPastAccumulator_ExtrapolatesBeyondLatest(uint32,uint256,uint256,uint256) (runs: 256, μ: 65156996, ~: 74325443) -QueryProcessorTest:testGetPastAccumulator_InterpolatesBetweenPastAccumulators(uint32,uint256,uint256,uint256) (runs: 256, μ: 65191146, ~: 74359876) -QueryProcessorTest:testGetPastAccumulator_InvalidAgo(uint32,uint256,uint256,uint256) (runs: 256, μ: 65148574, ~: 74317294) -QueryProcessorTest:testGetPastAccumulator_QueryTooOld(uint32,uint256,uint256,uint256) (runs: 256, μ: 65160165, ~: 74326834) -QueryProcessorTest:testGetTimeWeightedAverage(uint32,uint256,uint256,uint256,uint256) (runs: 256, μ: 103403746, ~: 113055688) +QueryProcessorTest:testGetInstantValue_NotInitialized_BeyondBufferSize(uint8,uint16) (runs: 256, μ: 68389651, ~: 68389600) +QueryProcessorTest:testGetPastAccumulator_BufferEmpty(uint8) (runs: 256, μ: 27030, ~: 27087) +QueryProcessorTest:testGetPastAccumulator_ExactMatch(uint32,uint256,uint256,uint16) (runs: 256, μ: 71569162, ~: 78616628) +QueryProcessorTest:testGetPastAccumulator_ExactMatch_LatestAccumulator(uint32,uint256,uint256) (runs: 256, μ: 69854170, ~: 78381550) +QueryProcessorTest:testGetPastAccumulator_ExactMatch_OldestAccumulator(uint32,uint256,uint256) (runs: 256, μ: 69884111, ~: 78413074) +QueryProcessorTest:testGetPastAccumulator_ExtrapolatesBeyondLatest(uint32,uint256,uint256,uint256) (runs: 256, μ: 65548439, ~: 74079107) +QueryProcessorTest:testGetPastAccumulator_InterpolatesBetweenPastAccumulators(uint32,uint256,uint256,uint256) (runs: 256, μ: 65582491, ~: 74113593) +QueryProcessorTest:testGetPastAccumulator_InvalidAgo(uint32,uint256,uint256,uint256) (runs: 256, μ: 65540049, ~: 74070132) +QueryProcessorTest:testGetPastAccumulator_QueryTooOld(uint32,uint256,uint256,uint256) (runs: 256, μ: 65551512, ~: 74080359) +QueryProcessorTest:testGetTimeWeightedAverage(uint32,uint256,uint256,uint256,uint256) (runs: 256, μ: 105204511, ~: 113027711) QueryProcessorTest:testGetTimeWeightedAverage_BadSecs() (gas: 10995) -ReservoirPriceOracleTest:testClearRoute() (gas: 52319) -ReservoirPriceOracleTest:testClearRoute_AllWordsCleared() (gas: 155404) -ReservoirPriceOracleTest:testDesignatePair() (gas: 29102) -ReservoirPriceOracleTest:testDesignatePair_IncorrectPair() (gas: 21222) -ReservoirPriceOracleTest:testDesignatePair_NotOwner() (gas: 17553) -ReservoirPriceOracleTest:testDesignatePair_TokenOrderReversed() (gas: 30740) +ReservoirPriceOracleTest:testClearRoute() (gas: 52160) +ReservoirPriceOracleTest:testClearRoute_AllWordsCleared() (gas: 155162) +ReservoirPriceOracleTest:testDesignatePair() (gas: 29119) +ReservoirPriceOracleTest:testDesignatePair_IncorrectPair() (gas: 21206) +ReservoirPriceOracleTest:testDesignatePair_NotOwner() (gas: 17531) +ReservoirPriceOracleTest:testDesignatePair_TokenOrderReversed() (gas: 30802) ReservoirPriceOracleTest:testGasBountyAvailable(uint256) (runs: 256, μ: 9885, ~: 9881) ReservoirPriceOracleTest:testGasBountyAvailable_Zero() (gas: 8939) -ReservoirPriceOracleTest:testGetLargestSafeQueryWindow() (gas: 8412) -ReservoirPriceOracleTest:testGetLatest(uint32) (runs: 256, μ: 92856, ~: 92787) -ReservoirPriceOracleTest:testGetLatest_Inverted() (gas: 96864) -ReservoirPriceOracleTest:testGetPastAccumulators() (gas: 196417) -ReservoirPriceOracleTest:testGetPastAccumulators_Inverted() (gas: 156850) -ReservoirPriceOracleTest:testGetQuote(uint256,uint256) (runs: 256, μ: 35792, ~: 35904) -ReservoirPriceOracleTest:testGetQuote_AmountInTooLarge() (gas: 12985) -ReservoirPriceOracleTest:testGetQuote_BaseIsVault(uint256) (runs: 256, μ: 445074, ~: 444837) -ReservoirPriceOracleTest:testGetQuote_ComplicatedDecimals() (gas: 10353280) -ReservoirPriceOracleTest:testGetQuote_Inverse(uint256,uint256) (runs: 256, μ: 37978, ~: 38149) -ReservoirPriceOracleTest:testGetQuote_MultipleHops() (gas: 114609) -ReservoirPriceOracleTest:testGetQuote_MultipleHops_Inverse() (gas: 114842) -ReservoirPriceOracleTest:testGetQuote_MultipleHops_PriceZero() (gas: 127427) -ReservoirPriceOracleTest:testGetQuote_NoFallbackOracle() (gas: 16112) -ReservoirPriceOracleTest:testGetQuote_PriceZero() (gas: 16519) -ReservoirPriceOracleTest:testGetQuote_RandomizeAllParam_1HopRoute(uint256,uint256,address,address,uint8,uint8) (runs: 256, μ: 5328074, ~: 5328117) -ReservoirPriceOracleTest:testGetQuote_RandomizeAllParam_2HopRoute(uint256,uint256,uint256,address,address,address,uint8,uint8,uint8) (runs: 256, μ: 10493915, ~: 10493914) -ReservoirPriceOracleTest:testGetQuote_SameBaseQuote(uint256,address) (runs: 256, μ: 8985, ~: 8985) -ReservoirPriceOracleTest:testGetQuote_UseFallback() (gas: 37825) -ReservoirPriceOracleTest:testGetQuote_ZeroIn() (gas: 39322) -ReservoirPriceOracleTest:testGetQuotes(uint256,uint256) (runs: 256, μ: 33348, ~: 33460) -ReservoirPriceOracleTest:testGetTimeWeightedAverage() (gas: 142014) -ReservoirPriceOracleTest:testGetTimeWeightedAverage_Inverted() (gas: 121185) +ReservoirPriceOracleTest:testGetLargestSafeQueryWindow() (gas: 8390) +ReservoirPriceOracleTest:testGetLatest(uint32) (runs: 256, μ: 92808, ~: 92737) +ReservoirPriceOracleTest:testGetLatest_Inverted() (gas: 96792) +ReservoirPriceOracleTest:testGetPastAccumulators() (gas: 196359) +ReservoirPriceOracleTest:testGetPastAccumulators_Inverted() (gas: 156800) +ReservoirPriceOracleTest:testGetQuote(uint256,uint256) (runs: 256, μ: 35815, ~: 35927) +ReservoirPriceOracleTest:testGetQuote_AmountInTooLarge() (gas: 13030) +ReservoirPriceOracleTest:testGetQuote_BaseIsVault(uint256) (runs: 256, μ: 418332, ~: 418079) +ReservoirPriceOracleTest:testGetQuote_ComplicatedDecimals() (gas: 10353271) +ReservoirPriceOracleTest:testGetQuote_Inverse(uint256,uint256) (runs: 256, μ: 37979, ~: 38148) +ReservoirPriceOracleTest:testGetQuote_MultipleHops() (gas: 114566) +ReservoirPriceOracleTest:testGetQuote_MultipleHops_Inverse() (gas: 114799) +ReservoirPriceOracleTest:testGetQuote_MultipleHops_PriceZero() (gas: 127407) +ReservoirPriceOracleTest:testGetQuote_NoFallbackOracle() (gas: 21074) +ReservoirPriceOracleTest:testGetQuote_PriceZero() (gas: 16564) +ReservoirPriceOracleTest:testGetQuote_RandomizeAllParam_1HopRoute(uint256,uint256,address,address,uint8,uint8) (runs: 256, μ: 5328103, ~: 5328201) +ReservoirPriceOracleTest:testGetQuote_RandomizeAllParam_2HopRoute(uint256,uint256,uint256,address,address,address,uint8,uint8,uint8) (runs: 256, μ: 10493966, ~: 10494003) +ReservoirPriceOracleTest:testGetQuote_SameBaseQuote(uint256,address) (runs: 256, μ: 9030, ~: 9030) +ReservoirPriceOracleTest:testGetQuote_UseFallback() (gas: 38776) +ReservoirPriceOracleTest:testGetQuote_ZeroIn() (gas: 39390) +ReservoirPriceOracleTest:testGetQuotes(uint256,uint256) (runs: 256, μ: 33326, ~: 33438) +ReservoirPriceOracleTest:testGetTimeWeightedAverage() (gas: 141789) +ReservoirPriceOracleTest:testGetTimeWeightedAverage_Inverted() (gas: 120964) ReservoirPriceOracleTest:testSetFallbackOracle_NotOwner() (gas: 11003) -ReservoirPriceOracleTest:testSetRoute() (gas: 58936) -ReservoirPriceOracleTest:testSetRoute_InvalidRoute() (gas: 18049) -ReservoirPriceOracleTest:testSetRoute_InvalidRouteLength() (gas: 17655) -ReservoirPriceOracleTest:testSetRoute_MultipleHops() (gas: 196333) -ReservoirPriceOracleTest:testSetRoute_NotSorted() (gas: 12117) -ReservoirPriceOracleTest:testSetRoute_OverwriteExisting() (gas: 162732) -ReservoirPriceOracleTest:testSetRoute_SameToken() (gas: 12070) -ReservoirPriceOracleTest:testUndesignatePair() (gas: 30318) -ReservoirPriceOracleTest:testUndesignatePair_NotOwner() (gas: 15288) -ReservoirPriceOracleTest:testUpdatePriceDeviationThreshold(uint256) (runs: 256, μ: 21390, ~: 21152) -ReservoirPriceOracleTest:testUpdatePrice_BeyondThreshold() (gas: 216785) -ReservoirPriceOracleTest:testUpdatePrice_BeyondThreshold_InsufficientReward(uint256) (runs: 256, μ: 205829, ~: 205771) -ReservoirPriceOracleTest:testUpdatePrice_BeyondThreshold_ZeroRecipient() (gas: 198455) -ReservoirPriceOracleTest:testUpdatePrice_FirstUpdate() (gas: 205996) -ReservoirPriceOracleTest:testUpdatePrice_IntermediateRoutes() (gas: 15872304) -ReservoirPriceOracleTest:testUpdatePrice_PriceOutOfRange() (gas: 5355619) -ReservoirPriceOracleTest:testUpdatePrice_WithinThreshold() (gas: 207028) -ReservoirPriceOracleTest:testUpdateRewardGasAmount() (gas: 19033) -ReservoirPriceOracleTest:testUpdateRewardGasAmount_NotOwner() (gas: 11006) -ReservoirPriceOracleTest:testUpdateTwapPeriod(uint256) (runs: 256, μ: 21815, ~: 21892) -ReservoirPriceOracleTest:testUpdateTwapPeriod_InvalidTwapPeriod(uint256) (runs: 256, μ: 17894, ~: 18208) -ReservoirPriceOracleTest:testWritePriceCache(uint256) (runs: 256, μ: 30058, ~: 29821) +ReservoirPriceOracleTest:testSetRoute() (gas: 58848) +ReservoirPriceOracleTest:testSetRoute_InvalidRoute() (gas: 18005) +ReservoirPriceOracleTest:testSetRoute_InvalidRouteLength() (gas: 17611) +ReservoirPriceOracleTest:testSetRoute_MultipleHops() (gas: 196135) +ReservoirPriceOracleTest:testSetRoute_NotSorted() (gas: 12095) +ReservoirPriceOracleTest:testSetRoute_OverwriteExisting() (gas: 162578) +ReservoirPriceOracleTest:testSetRoute_SameToken() (gas: 12048) +ReservoirPriceOracleTest:testUndesignatePair() (gas: 30357) +ReservoirPriceOracleTest:testUndesignatePair_NotOwner() (gas: 15332) +ReservoirPriceOracleTest:testUpdatePriceDeviationThreshold(uint256) (runs: 256, μ: 21334, ~: 21085) +ReservoirPriceOracleTest:testUpdatePrice_BeyondThreshold() (gas: 215402) +ReservoirPriceOracleTest:testUpdatePrice_BeyondThreshold_InsufficientReward(uint256) (runs: 256, μ: 204437, ~: 204648) +ReservoirPriceOracleTest:testUpdatePrice_BeyondThreshold_ZeroRecipient() (gas: 197028) +ReservoirPriceOracleTest:testUpdatePrice_FirstUpdate() (gas: 204525) +ReservoirPriceOracleTest:testUpdatePrice_IntermediateRoutes() (gas: 15869132) +ReservoirPriceOracleTest:testUpdatePrice_PriceOutOfRange() (gas: 5354102) +ReservoirPriceOracleTest:testUpdatePrice_WithinThreshold() (gas: 205557) +ReservoirPriceOracleTest:testUpdateRewardGasAmount() (gas: 19055) +ReservoirPriceOracleTest:testUpdateRewardGasAmount_NotOwner() (gas: 10984) +ReservoirPriceOracleTest:testUpdateTwapPeriod(uint256) (runs: 256, μ: 21793, ~: 21870) +ReservoirPriceOracleTest:testUpdateTwapPeriod_InvalidTwapPeriod(uint256) (runs: 256, μ: 17858, ~: 18164) +ReservoirPriceOracleTest:testWritePriceCache(uint256) (runs: 256, μ: 30030, ~: 29777) SamplesTest:testAccumulator() (gas: 3959) SamplesTest:testAccumulator_BadVariableRequest() (gas: 3523) SamplesTest:testInstant() (gas: 3909) diff --git a/README.md b/README.md index e69de29..d86ed45 100644 --- a/README.md +++ b/README.md @@ -0,0 +1,90 @@ +# Reservoir Price Oracle + +The Reservoir Price Oracle is designed to work with +[Euler Vault Kit](https://github.com/euler-xyz/euler-vault-kit) by implementing +the `IPriceOracle` interface. + +This oracle provides a geometric mean price between two assets, averaged across +a period. The geometric mean has a useful property whereby we can get the +inverse price by simply taking the reciprocal. Something that arithmetic mean +prices do not provide. + +Powered the built-in on-chain price oracle of Reservoir's [AMM](https://github.com/reservoir-labs/amm-core). + +## Interfaces + +For more information on the `IPriceOracle` interface, refer to Euler's [documentation](https://github.com/euler-xyz/euler-price-oracle?tab=readme-ov-file#ipriceoracle). + +For direct usages of the oracle, refer to +[IReservoirPriceOracle.sol](src/interfaces/IReservoirPriceOracle.sol) for +methods to obtain raw data from the AMM pairs. + +## Usage + +### Install + +To install Price Oracles in a [Foundry](https://github.com/foundry-rs/foundry) project: + +```sh +forge install reservoir-labs/oracle +``` + +### Development + +Clone the repo: + +```sh +git clone https://github.com/reservoir-labs/oracle.git && cd oracle +``` + +Install forge dependencies: + +```sh +forge install +``` + +[Optional] Install Node.js dependencies: + +```sh +npm install +``` + +Compile the contracts: + +```sh +forge build +``` + +### Testing + +The repo contains 3 types of tests: unit, large, and integration. + +To run all tests: + +```sh +npm run test:all +``` + +### Linting + +To run lint on solidity, json, and markdown, run: + +```sh +npm run lint +``` + +Separate `.solhint.json` files exist for `src/` and `test/`. + +## Security vulnerability disclosure + +Please report suspected security vulnerabilities in private to +[security@reservoir.fi](security@reservoir.fi). Please do NOT create publicly +viewable issues for suspected security vulnerabilities. + +## Audits + +These contracts have been audited by TBD and TBD auditing firm. + +## License + +The Euler Price Oracles code is licensed under the [GPL-3.0-or-later](LICENSE) license. diff --git a/package.json b/package.json index baa1cc9..6f3fa5e 100644 --- a/package.json +++ b/package.json @@ -53,7 +53,7 @@ "submodule:check": "cd lib && find . -mindepth 1 -maxdepth 1 -type d -exec bash -c 'cd \"{}\" && pwd && ../../scripts/git-master-diff.sh && echo' \\;", "submodule:reset": "git submodule update --recursive", "test": "npm run test:unit", - "test:all": "npm run test:unit && npm run test:integration", + "test:all": "npm run test:unit && npm run test:unit-large && npm run test:integration", "test:integration": "export FOUNDRY_PROFILE=integration && forge test", "test:unit": "forge test", "test:unit-large": "export FOUNDRY_PROFILE=large-test && forge test" diff --git a/src/ReservoirPriceOracle.sol b/src/ReservoirPriceOracle.sol index fc63e58..ca2ab7a 100644 --- a/src/ReservoirPriceOracle.sol +++ b/src/ReservoirPriceOracle.sol @@ -35,13 +35,13 @@ contract ReservoirPriceOracle is IPriceOracle, IReservoirPriceOracle, Owned(msg. event DesignatePair(address token0, address token1, ReservoirPair pair); event FallbackOracleSet(address fallbackOracle); event PriceDeviationThreshold(uint256 newThreshold); - event ResolvedVaultSet(address vault, address asset); event RewardGasAmount(uint256 newAmount); event Route(address token0, address token1, address[] route); - event Price(address token0, address token1, uint256 price); - event SetPriceType(PriceType priceType); event TwapPeriod(uint256 newPeriod); + /// @notice The type of price queried and stored, possibilities as defined by `PriceType`. + PriceType public immutable PRICE_TYPE; + /////////////////////////////////////////////////////////////////////////////////////////////// // STORAGE // /////////////////////////////////////////////////////////////////////////////////////////////// @@ -50,29 +50,22 @@ contract ReservoirPriceOracle is IPriceOracle, IReservoirPriceOracle, Owned(msg. /// @dev If `address(0)` then there is no fallback. address public fallbackOracle; - /// @dev the following 4 storage variables take up 1 storage slot + // The following 3 storage variables take up 1 storage slot. /// @notice percentage change greater than which, a price update may result in a reward payout of native tokens, /// subject to availability of rewards. /// 1e18 == 100% uint64 public priceDeviationThreshold; - /// @notice multiples of the base fee the contract rewards the caller for updating the price when it goes - /// beyond the `priceDeviationThreshold` + /// @notice This number is multiplied by the base fee to determine the reward for keepers uint64 public rewardGasAmount; /// @notice TWAP period (in seconds) for querying the oracle uint64 public twapPeriod; - /// @notice The type of price queried and stored, possibilities as defined by `PriceType`. - PriceType public priceType; - /// @notice Designated pairs to serve as price feed for a certain token0 and token1 mapping(address token0 => mapping(address token1 => ReservoirPair pair)) public pairs; - /// @notice ERC4626 vaults resolved using internal pricing (`convertToAssets`). - mapping(address vault => address asset) public resolvedVaults; - /////////////////////////////////////////////////////////////////////////////////////////////// // CONSTRUCTOR, FALLBACKS // /////////////////////////////////////////////////////////////////////////////////////////////// @@ -81,7 +74,7 @@ contract ReservoirPriceOracle is IPriceOracle, IReservoirPriceOracle, Owned(msg. updatePriceDeviationThreshold(aThreshold); updateTwapPeriod(aTwapPeriod); updateRewardGasAmount(aMultiplier); - setPriceType(aType); + PRICE_TYPE = aType; } /// @dev contract will hold native tokens to be distributed as gas bounty for updating the prices @@ -148,38 +141,28 @@ contract ReservoirPriceOracle is IPriceOracle, IReservoirPriceOracle, Owned(msg. (address[] memory lRoute,,) = _getRouteDecimalDifferencePrice(lToken0, lToken1); if (lRoute.length == 0) revert OracleErrors.NoPath(); - OracleAverageQuery[] memory lQueries = new OracleAverageQuery[](lRoute.length - 1); - for (uint256 i = 0; i < lRoute.length - 1; ++i) { (lToken0, lToken1) = lRoute[i].sortTokens(lRoute[i + 1]); - lQueries[i] = OracleAverageQuery( - priceType, - lToken0, - lToken1, - twapPeriod, - 0 // now + uint256 lNewPrice = _getTimeWeightedAverageSingle( + OracleAverageQuery( + PRICE_TYPE, + lToken0, + lToken1, + twapPeriod, + 0 // now + ) ); - } - - uint256[] memory lNewPrices = getTimeWeightedAverage(lQueries); - - for (uint256 i = 0; i < lNewPrices.length; ++i) { - address lBase = lQueries[i].base; - address lQuote = lQueries[i].quote; - uint256 lNewPrice = lNewPrices[i]; // assumed to be simple routes and therefore lPrevPrice would only be 0 for the first update // consider an optimization here for simple routes: no need to read the price cache again // as it has been returned by _getRouteDecimalDifferencePrice in the beginning of the function - (uint256 lPrevPrice,) = _priceCache(lBase, lQuote); - + (uint256 lPrevPrice,) = _priceCache(lToken0, lToken1); // determine if price has moved beyond the threshold, and pay out reward if so if (_calcPercentageDiff(lPrevPrice, lNewPrice) >= priceDeviationThreshold) { _rewardUpdater(aRewardRecipient); } - - _writePriceCache(lBase, lQuote, lNewPrice); + _writePriceCache(lToken0, lToken1, lNewPrice); } } @@ -192,16 +175,8 @@ contract ReservoirPriceOracle is IPriceOracle, IReservoirPriceOracle, Owned(msg. returns (uint256[] memory rResults) { rResults = new uint256[](aQueries.length); - - OracleAverageQuery memory lQuery; for (uint256 i = 0; i < aQueries.length; ++i) { - lQuery = aQueries[i]; - ReservoirPair lPair = pairs[lQuery.base][lQuery.quote]; - _validatePair(lPair); - - (,,, uint16 lIndex) = lPair.getReserves(); - uint256 lResult = lPair.getTimeWeightedAverage(lQuery.priceType, lQuery.secs, lQuery.ago, lIndex); - rResults[i] = lResult; + rResults[i] = _getTimeWeightedAverageSingle(aQueries[i]); } } @@ -248,6 +223,14 @@ contract ReservoirPriceOracle is IPriceOracle, IReservoirPriceOracle, Owned(msg. if (address(aPair) == address(0)) revert OracleErrors.NoDesignatedPair(); } + function _getTimeWeightedAverageSingle(OracleAverageQuery memory aQuery) internal view returns (uint256 rResult) { + ReservoirPair lPair = pairs[aQuery.base][aQuery.quote]; + _validatePair(lPair); + + (,,, uint16 lIndex) = lPair.getReserves(); + rResult = lPair.getTimeWeightedAverage(aQuery.priceType, aQuery.secs, aQuery.ago, lIndex); + } + // TODO: replace this with safe, audited lib function function _calcPercentageDiff(uint256 aOriginal, uint256 aNew) internal pure returns (uint256) { if (aOriginal == 0) return 0; @@ -398,15 +381,18 @@ contract ReservoirPriceOracle is IPriceOracle, IReservoirPriceOracle, Owned(msg. (address[] memory lRoute, int256 lDecimalDiff, uint256 lPrice) = _getRouteDecimalDifferencePrice(lToken0, lToken1); - // route does not exist on our oracle, attempt querying the fallback if (lRoute.length == 0) { - address lBaseAsset = resolvedVaults[aBase]; - - if (lBaseAsset != address(0)) { + // There is one case where the behavior is a bit more unexpected, and that is when + // `aBase` is an empty contract, and the revert would not be caught at all, causing + // the entire operation to fail. But this is okay, because if `aBase` is not a contract, trying + // to use the fallbackOracle would not yield any results anyway. + // An alternative would be to use a low level `staticcall`. + try IERC4626(aBase).asset() returns (address rBaseAsset) { uint256 lResolvedAmountIn = IERC4626(aBase).convertToAssets(aAmount); - return _getQuotes(lResolvedAmountIn, lBaseAsset, aQuote, aIsGetQuotes); - } + return _getQuotes(lResolvedAmountIn, rBaseAsset, aQuote, aIsGetQuotes); + } catch { } // solhint-disable-line no-empty-blocks + // route does not exist on our oracle, attempt querying the fallback return _useFallbackOracle(aAmount, aBase, aQuote, aIsGetQuotes); } else if (lRoute.length == 2) { if (lPrice == 0) revert OracleErrors.PriceZero(); @@ -504,13 +490,15 @@ contract ReservoirPriceOracle is IPriceOracle, IReservoirPriceOracle, Owned(msg. emit RewardGasAmount(aNewMultiplier); } - // sets a specific pair to serve as price feed for a certain route - function designatePair(address aToken0, address aToken1, ReservoirPair aPair) external onlyOwner { - (aToken0, aToken1) = aToken0.sortTokens(aToken1); - assert(aToken0 == address(aPair.token0()) && aToken1 == address(aPair.token1())); + /// @notice Sets the pair to serve as price feed for a given route. + function designatePair(address aTokenA, address aTokenB, ReservoirPair aPair) external onlyOwner { + (aTokenA, aTokenB) = aTokenA.sortTokens(aTokenB); + if (aTokenA != address(aPair.token0()) || aTokenB != address(aPair.token1())) { + revert OracleErrors.IncorrectTokensDesignatePair(); + } - pairs[aToken0][aToken1] = aPair; - emit DesignatePair(aToken0, aToken1, aPair); + pairs[aTokenA][aTokenB] = aPair; + emit DesignatePair(aTokenA, aTokenB, aPair); } function undesignatePair(address aToken0, address aToken1) external onlyOwner { @@ -520,17 +508,6 @@ contract ReservoirPriceOracle is IPriceOracle, IReservoirPriceOracle, Owned(msg. emit DesignatePair(aToken0, aToken1, ReservoirPair(address(0))); } - function setPriceType(PriceType aType) public onlyOwner { - priceType = aType; - emit SetPriceType(aType); - } - - function setResolvedVault(address aVault, bool aSet) external onlyOwner { - address lAsset = aSet ? IERC4626(aVault).asset() : address(0); - resolvedVaults[aVault] = lAsset; - emit ResolvedVaultSet(aVault, lAsset); - } - /// @notice Sets the price route between aToken0 and aToken1, and also intermediate routes if previously undefined /// @param aToken0 Address of the lower token /// @param aToken1 Address of the higher token @@ -593,12 +570,14 @@ contract ReservoirPriceOracle is IPriceOracle, IReservoirPriceOracle, Owned(msg. bytes32 lSlot = aToken0.calculateSlot(aToken1); - // clear all storage slots that the route has written to previously + // clear the storage slot that the route has written to previously assembly { sstore(lSlot, 0) } - // routes with length 4 use two words of storage - if (lRoute.length == 4) { + + // routes with length MAX_ROUTE_LENGTH use one more word of storage + // `setRoute` enforces the MAX_ROUTE_LENGTH limit. + if (lRoute.length == Constants.MAX_ROUTE_LENGTH) { assembly { sstore(add(lSlot, 1), 0) } diff --git a/src/libraries/OracleErrors.sol b/src/libraries/OracleErrors.sol index 3d1fa97..3a70260 100644 --- a/src/libraries/OracleErrors.sol +++ b/src/libraries/OracleErrors.sol @@ -4,6 +4,7 @@ pragma solidity ^0.8.0; /// @dev Collection of all oracle related errors. library OracleErrors { // config errors + error IncorrectTokensDesignatePair(); error InvalidRoute(); error InvalidRouteLength(); error InvalidTwapPeriod(); diff --git a/test/large/ReservoirPriceOracleLarge.t.sol b/test/large/ReservoirPriceOracleLarge.t.sol index f30898b..c6a28b7 100644 --- a/test/large/ReservoirPriceOracleLarge.t.sol +++ b/test/large/ReservoirPriceOracleLarge.t.sol @@ -4,6 +4,7 @@ pragma solidity ^0.8.0; import { ReservoirPriceOracleTest, EnumerableSetLib, + FixedPointMathLib, MintableERC20, ReservoirPair, IERC20 @@ -11,6 +12,7 @@ import { contract ReservoirPriceOracleLargeTest is ReservoirPriceOracleTest { using EnumerableSetLib for EnumerableSetLib.AddressSet; + using FixedPointMathLib for uint256; function testGetQuote_RandomizeAllParam_3HopRoute( uint256 aPrice1, @@ -97,14 +99,14 @@ contract ReservoirPriceOracleLargeTest is ReservoirPriceOracleTest { // assert uint256 lExpectedAmtBOut = lTokenA < lTokenB - ? lAmtIn * 10 ** lTokenADecimal * lPrice1 * 10 ** lTokenBDecimal / 10 ** lTokenADecimal / WAD - : lAmtIn * 10 ** lTokenADecimal * WAD * 10 ** lTokenBDecimal / lPrice1 / 10 ** lTokenADecimal; + ? (lAmtIn * 10 ** lTokenADecimal).fullMulDiv(lPrice1 * 10 ** lTokenBDecimal, 10 ** lTokenADecimal * WAD) + : (lAmtIn * 10 ** lTokenADecimal).fullMulDiv(WAD * 10 ** lTokenBDecimal, lPrice1 * 10 ** lTokenADecimal); uint256 lExpectedAmtCOut = lTokenB < lTokenC - ? lExpectedAmtBOut * lPrice2 * 10 ** lTokenCDecimal / 10 ** lTokenBDecimal / WAD - : lExpectedAmtBOut * WAD * 10 ** lTokenCDecimal / lPrice2 / 10 ** lTokenBDecimal; + ? lExpectedAmtBOut.fullMulDiv(lPrice2 * 10 ** lTokenCDecimal, 10 ** lTokenBDecimal * WAD) + : lExpectedAmtBOut.fullMulDiv(WAD * 10 ** lTokenCDecimal, lPrice2 * 10 ** lTokenBDecimal); uint256 lExpectedAmtDOut = lTokenC < lTokenD - ? lExpectedAmtCOut * lPrice3 * 10 ** lTokenDDecimal / 10 ** lTokenCDecimal / WAD - : lExpectedAmtCOut * WAD * 10 ** lTokenDDecimal / lPrice3 / 10 ** lTokenCDecimal; + ? lExpectedAmtCOut.fullMulDiv(lPrice3 * 10 ** lTokenDDecimal, 10 ** lTokenCDecimal * WAD) + : lExpectedAmtCOut.fullMulDiv(WAD * 10 ** lTokenDDecimal, lPrice3 * 10 ** lTokenCDecimal); assertEq(lAmtDOut, lExpectedAmtDOut); } diff --git a/test/unit/ReservoirPriceOracle.t.sol b/test/unit/ReservoirPriceOracle.t.sol index 484bcb5..9b2dced 100644 --- a/test/unit/ReservoirPriceOracle.t.sol +++ b/test/unit/ReservoirPriceOracle.t.sol @@ -6,6 +6,7 @@ import { BaseTest, console2, ReservoirPair, MintableERC20 } from "test/__fixture import { Utils } from "src/libraries/Utils.sol"; import { Buffer, + FixedPointMathLib, PriceType, OracleErrors, OracleLatestQuery, @@ -27,6 +28,7 @@ contract ReservoirPriceOracleTest is BaseTest { using FlagsLib for *; using Bytes32Lib for *; using EnumerableSetLib for EnumerableSetLib.AddressSet; + using FixedPointMathLib for uint256; event DesignatePair(address token0, address token1, ReservoirPair pair); event Oracle(address newOracle); @@ -282,8 +284,8 @@ contract ReservoirPriceOracleTest is BaseTest { // assert uint256 lExpectedAmt = lTokenA < lTokenB - ? lAmtIn * 10 ** lTokenADecimal * lPrice * 10 ** lTokenBDecimal / 10 ** lTokenADecimal / WAD - : lAmtIn * 10 ** lTokenADecimal * WAD * 10 ** lTokenBDecimal / lPrice / 10 ** lTokenADecimal; + ? (lAmtIn * 10 ** lTokenADecimal).fullMulDiv(lPrice * 10 ** lTokenBDecimal, 10 ** lTokenADecimal * WAD) + : (lAmtIn * 10 ** lTokenADecimal).fullMulDiv(WAD * 10 ** lTokenBDecimal, lPrice * 10 ** lTokenADecimal); assertEq(lAmtBOut, lExpectedAmt); } @@ -349,11 +351,11 @@ contract ReservoirPriceOracleTest is BaseTest { // assert uint256 lExpectedAmtBOut = lTokenA < lTokenB - ? lAmtIn * 10 ** lTokenADecimal * lPrice1 * 10 ** lTokenBDecimal / 10 ** lTokenADecimal / WAD - : lAmtIn * 10 ** lTokenADecimal * WAD * 10 ** lTokenBDecimal / lPrice1 / 10 ** lTokenADecimal; + ? (lAmtIn * 10 ** lTokenADecimal).fullMulDiv(lPrice1 * 10 ** lTokenBDecimal, 10 ** lTokenADecimal * WAD) + : (lAmtIn * 10 ** lTokenADecimal).fullMulDiv(WAD * 10 ** lTokenBDecimal, lPrice1 * 10 ** lTokenADecimal); uint256 lExpectedAmtCOut = lTokenB < lTokenC - ? lExpectedAmtBOut * lPrice2 * 10 ** lTokenCDecimal / 10 ** lTokenBDecimal / WAD - : lExpectedAmtBOut * WAD * 10 ** lTokenCDecimal / lPrice2 / 10 ** lTokenBDecimal; + ? (lExpectedAmtBOut).fullMulDiv(lPrice2 * 10 ** lTokenCDecimal, 10 ** lTokenBDecimal * WAD) + : (lExpectedAmtBOut).fullMulDiv(WAD * 10 ** lTokenCDecimal, lPrice2 * 10 ** lTokenBDecimal); assertEq(lAmtCOut, lExpectedAmtCOut); } @@ -413,7 +415,6 @@ contract ReservoirPriceOracleTest is BaseTest { // arrange uint256 lAmtIn = 5e18; StubERC4626 lVault = new StubERC4626(address(_tokenA), lRate); - _oracle.setResolvedVault(address(lVault), true); _writePriceCache(address(_tokenA), address(_tokenB), 1e18); // act @@ -925,7 +926,7 @@ contract ReservoirPriceOracleTest is BaseTest { function testDesignatePair_IncorrectPair() external { // act & assert - vm.expectRevert(); + vm.expectRevert(OracleErrors.IncorrectTokensDesignatePair.selector); _oracle.designatePair(address(_tokenA), address(_tokenC), _pair); } @@ -983,12 +984,6 @@ contract ReservoirPriceOracleTest is BaseTest { _oracle.updatePrice(address(_tokenB), address(_tokenC), address(0)); } - function setPriceType() external { - vm.prank(address(123)); - vm.expectRevert("UNAUTHORIZED"); - _oracle.setPriceType(PriceType.RAW_PRICE); - } - function testSetRoute_SameToken() external { // arrange address lToken0 = address(0x1); @@ -1068,7 +1063,7 @@ contract ReservoirPriceOracleTest is BaseTest { function testGetQuote_NoFallbackOracle() external { // act & assert vm.expectRevert(OracleErrors.NoPath.selector); - _oracle.getQuote(123, address(123), address(456)); + _oracle.getQuote(123, address(_tokenD), address(_tokenA)); } function testGetQuote_PriceZero() external {