From 70374b4b5e17eb6422e7d5be0a293c9d998e6839 Mon Sep 17 00:00:00 2001 From: "A.L" Date: Thu, 27 Jun 2024 21:49:12 +0200 Subject: [PATCH] fix: calc percentage diff * fix: use unchecked for arithmetic * gas: update snapshot --- .gas-snapshot | 62 ++++++++++++++++++------------------ src/ReservoirPriceOracle.sol | 19 +++++++---- 2 files changed, 43 insertions(+), 38 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index 6448500..6e6cca9 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -1,22 +1,22 @@ 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, μ: 66028796, ~: 73578593) -QueryProcessorTest:testFindNearestSample_CanFindIntermediateValue(uint32,uint256,uint256,uint256) (runs: 256, μ: 65574840, ~: 74107585) +QueryProcessorTest:testFindNearestSample_CanFindExactValue(uint32,uint256,uint256,uint256) (runs: 256, μ: 64950967, ~: 73522026) +QueryProcessorTest:testFindNearestSample_CanFindIntermediateValue(uint32,uint256,uint256,uint256) (runs: 256, μ: 65276558, ~: 74273370) QueryProcessorTest:testFindNearestSample_NotInitialized() (gas: 8937393461068805977) -QueryProcessorTest:testFindNearestSample_OneSample(uint256) (runs: 256, μ: 80329, ~: 80360) +QueryProcessorTest:testFindNearestSample_OneSample(uint256) (runs: 256, μ: 80327, ~: 80360) QueryProcessorTest:testGetInstantValue() (gas: 124248) QueryProcessorTest:testGetInstantValue_NotInitialized(uint256) (runs: 256, μ: 19397, ~: 19397) -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:testGetInstantValue_NotInitialized_BeyondBufferSize(uint8,uint16) (runs: 256, μ: 68389668, ~: 68389600) +QueryProcessorTest:testGetPastAccumulator_BufferEmpty(uint8) (runs: 256, μ: 27027, ~: 27087) +QueryProcessorTest:testGetPastAccumulator_ExactMatch(uint32,uint256,uint256,uint16) (runs: 256, μ: 71195969, ~: 79321025) +QueryProcessorTest:testGetPastAccumulator_ExactMatch_LatestAccumulator(uint32,uint256,uint256) (runs: 256, μ: 68275149, ~: 77600848) +QueryProcessorTest:testGetPastAccumulator_ExactMatch_OldestAccumulator(uint32,uint256,uint256) (runs: 256, μ: 68304930, ~: 77632448) +QueryProcessorTest:testGetPastAccumulator_ExtrapolatesBeyondLatest(uint32,uint256,uint256,uint256) (runs: 256, μ: 65250265, ~: 74245223) +QueryProcessorTest:testGetPastAccumulator_InterpolatesBetweenPastAccumulators(uint32,uint256,uint256,uint256) (runs: 256, μ: 65284307, ~: 74279378) +QueryProcessorTest:testGetPastAccumulator_InvalidAgo(uint32,uint256,uint256,uint256) (runs: 256, μ: 65241836, ~: 74236389) +QueryProcessorTest:testGetPastAccumulator_QueryTooOld(uint32,uint256,uint256,uint256) (runs: 256, μ: 65253417, ~: 74246272) +QueryProcessorTest:testGetTimeWeightedAverage(uint32,uint256,uint256,uint256,uint256) (runs: 256, μ: 107071945, ~: 116076010) QueryProcessorTest:testGetTimeWeightedAverage_BadSecs() (gas: 10995) ReservoirPriceOracleTest:testClearRoute() (gas: 52160) ReservoirPriceOracleTest:testClearRoute_AllWordsCleared() (gas: 155162) @@ -27,26 +27,26 @@ ReservoirPriceOracleTest:testDesignatePair_TokenOrderReversed() (gas: 30802) ReservoirPriceOracleTest:testGasBountyAvailable(uint256) (runs: 256, μ: 9885, ~: 9881) ReservoirPriceOracleTest:testGasBountyAvailable_Zero() (gas: 8939) ReservoirPriceOracleTest:testGetLargestSafeQueryWindow() (gas: 8390) -ReservoirPriceOracleTest:testGetLatest(uint32) (runs: 256, μ: 92808, ~: 92737) +ReservoirPriceOracleTest:testGetLatest(uint32) (runs: 256, μ: 92802, ~: 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(uint256,uint256) (runs: 256, μ: 35828, ~: 35927) ReservoirPriceOracleTest:testGetQuote_AmountInTooLarge() (gas: 13030) -ReservoirPriceOracleTest:testGetQuote_BaseIsVault(uint256) (runs: 256, μ: 418332, ~: 418079) +ReservoirPriceOracleTest:testGetQuote_BaseIsVault(uint256) (runs: 256, μ: 418316, ~: 418079) ReservoirPriceOracleTest:testGetQuote_ComplicatedDecimals() (gas: 10353271) -ReservoirPriceOracleTest:testGetQuote_Inverse(uint256,uint256) (runs: 256, μ: 37979, ~: 38148) +ReservoirPriceOracleTest:testGetQuote_Inverse(uint256,uint256) (runs: 256, μ: 37990, ~: 38150) 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_RandomizeAllParam_1HopRoute(uint256,uint256,address,address,uint8,uint8) (runs: 256, μ: 5328095, ~: 5328121) +ReservoirPriceOracleTest:testGetQuote_RandomizeAllParam_2HopRoute(uint256,uint256,uint256,address,address,address,uint8,uint8,uint8) (runs: 256, μ: 10493971, ~: 10493982) 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:testGetQuotes(uint256,uint256) (runs: 256, μ: 33339, ~: 33438) ReservoirPriceOracleTest:testGetTimeWeightedAverage() (gas: 141789) ReservoirPriceOracleTest:testGetTimeWeightedAverage_Inverted() (gas: 120964) ReservoirPriceOracleTest:testSetFallbackOracle_NotOwner() (gas: 11003) @@ -59,19 +59,19 @@ 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:testUpdatePriceDeviationThreshold(uint256) (runs: 256, μ: 21325, ~: 21085) +ReservoirPriceOracleTest:testUpdatePrice_BeyondThreshold() (gas: 215212) +ReservoirPriceOracleTest:testUpdatePrice_BeyondThreshold_InsufficientReward(uint256) (runs: 256, μ: 204236, ~: 204176) +ReservoirPriceOracleTest:testUpdatePrice_BeyondThreshold_ZeroRecipient() (gas: 196838) +ReservoirPriceOracleTest:testUpdatePrice_FirstUpdate() (gas: 204522) +ReservoirPriceOracleTest:testUpdatePrice_IntermediateRoutes() (gas: 15869123) +ReservoirPriceOracleTest:testUpdatePrice_PriceOutOfRange() (gas: 5351880) +ReservoirPriceOracleTest:testUpdatePrice_WithinThreshold() (gas: 205367) 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) +ReservoirPriceOracleTest:testUpdateTwapPeriod(uint256) (runs: 256, μ: 21787, ~: 21870) +ReservoirPriceOracleTest:testUpdateTwapPeriod_InvalidTwapPeriod(uint256) (runs: 256, μ: 17855, ~: 18164) +ReservoirPriceOracleTest:testWritePriceCache(uint256) (runs: 256, μ: 30014, ~: 29777) SamplesTest:testAccumulator() (gas: 3959) SamplesTest:testAccumulator_BadVariableRequest() (gas: 3523) SamplesTest:testInstant() (gas: 3909) diff --git a/src/ReservoirPriceOracle.sol b/src/ReservoirPriceOracle.sol index ca2ab7a..5b6db0f 100644 --- a/src/ReservoirPriceOracle.sol +++ b/src/ReservoirPriceOracle.sol @@ -158,11 +158,13 @@ contract ReservoirPriceOracle is IPriceOracle, IReservoirPriceOracle, Owned(msg. // 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(lToken0, lToken1); + + _writePriceCache(lToken0, lToken1, lNewPrice); + // determine if price has moved beyond the threshold, and pay out reward if so if (_calcPercentageDiff(lPrevPrice, lNewPrice) >= priceDeviationThreshold) { _rewardUpdater(aRewardRecipient); } - _writePriceCache(lToken0, lToken1, lNewPrice); } } @@ -231,14 +233,17 @@ contract ReservoirPriceOracle is IPriceOracle, IReservoirPriceOracle, Owned(msg. 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; + unchecked { + if (aOriginal == 0) return 0; - if (aOriginal > aNew) { - return (aOriginal - aNew) * 1e18 / aOriginal; - } else { - return (aNew - aOriginal) * 1e18 / aOriginal; + // multiplication does not overflow as `aOriginal` and `aNew` is always less than or + // equal to `Constants.MAX_SUPPORTED_PRICE`, as checked in `_writePriceCache` + if (aOriginal > aNew) { + return (aOriginal - aNew) * 1e18 / aOriginal; + } else { + return (aNew - aOriginal) * 1e18 / aOriginal; + } } }