From 85cc147c6f5a04c85d74cad117a5aa921cca9acb Mon Sep 17 00:00:00 2001 From: "A.L." Date: Thu, 25 Jul 2024 22:29:03 +0200 Subject: [PATCH 01/17] feat: infra and encoding for bpDiffForMaxReward --- src/ReservoirPriceOracle.sol | 12 +++++++----- src/libraries/RoutesLib.sol | 20 ++++++++++++-------- 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/src/ReservoirPriceOracle.sol b/src/ReservoirPriceOracle.sol index 0201a19..71fd5f2 100644 --- a/src/ReservoirPriceOracle.sol +++ b/src/ReservoirPriceOracle.sol @@ -102,7 +102,7 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar // price update related functions function route(address aToken0, address aToken1) external view returns (address[] memory rRoute) { - (rRoute,,) = _getRouteDecimalDifferencePrice(aToken0, aToken1); + (rRoute,,,) = _getRouteDecimalDifferencePrice(aToken0, aToken1); } /// @notice The latest cached geometric TWAP of token0/token1. @@ -128,7 +128,7 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar function updatePrice(address aTokenA, address aTokenB, address aRewardRecipient) external nonReentrant { (address lToken0, address lToken1) = Utils.sortTokens(aTokenA, aTokenB); - (address[] memory lRoute,, uint256 lPrevPrice) = _getRouteDecimalDifferencePrice(lToken0, lToken1); + (address[] memory lRoute,, uint256 lPrevPrice,) = _getRouteDecimalDifferencePrice(lToken0, lToken1); if (lRoute.length == 0) revert OracleErrors.NoPath(); for (uint256 i = 0; i < lRoute.length - 1; ++i) { @@ -217,10 +217,11 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar /// @return rRoute The route to determine the price between aToken0 and aToken1 /// @return rDecimalDiff The result of token1.decimals() - token0.decimals() if it's a simple route. 0 otherwise /// @return rPrice The price of aToken0/aToken1 if it's a simple route (i.e. rRoute.length == 2). 0 otherwise + /// @return rBpDiffForMaxReward The price difference at and beyond which the maximum amount of gas bounty is applicable. function _getRouteDecimalDifferencePrice(address aToken0, address aToken1) private view - returns (address[] memory rRoute, int256 rDecimalDiff, uint256 rPrice) + returns (address[] memory rRoute, int256 rDecimalDiff, uint256 rPrice, uint256 rBpDiffForMaxReward) { bytes32 lSlot = Utils.calculateSlot(aToken0, aToken1); @@ -236,6 +237,7 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar rRoute[1] = aToken1; rDecimalDiff = lFirstWord.getDecimalDifference(); rPrice = lFirstWord.getPrice(); + rBpDiffForMaxReward = lFirstWord.getBpDiffForMaxReward(); } // composite route else if (lFirstWord.isCompositeRoute()) { @@ -329,7 +331,7 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar if (aAmount > Constants.MAX_AMOUNT_IN) revert OracleErrors.AmountInTooLarge(); (address lToken0, address lToken1) = Utils.sortTokens(aBase, aQuote); - (address[] memory lRoute, int256 lDecimalDiff, uint256 lPrice) = + (address[] memory lRoute, int256 lDecimalDiff, uint256 lPrice,) = _getRouteDecimalDifferencePrice(lToken0, lToken1); if (lRoute.length == 0) { @@ -515,7 +517,7 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar function clearRoute(address aToken0, address aToken1) external onlyOwner { _validateTokens(aToken0, aToken1); - (address[] memory lRoute,,) = _getRouteDecimalDifferencePrice(aToken0, aToken1); + (address[] memory lRoute,,,) = _getRouteDecimalDifferencePrice(aToken0, aToken1); bytes32 lSlot = Utils.calculateSlot(aToken0, aToken1); diff --git a/src/libraries/RoutesLib.sol b/src/libraries/RoutesLib.sol index 72b4a83..0a01b69 100644 --- a/src/libraries/RoutesLib.sol +++ b/src/libraries/RoutesLib.sol @@ -27,13 +27,6 @@ library RoutesLib { return aData[0] == FLAG_3_HOP_ROUTE; } - // Positive value indicates that token1 has a greater number of decimals compared to token2 - // while a negative value indicates otherwise. - // range of values between -18 and 18 - function getDecimalDifference(bytes32 aData) internal pure returns (int256 rDiff) { - rDiff = int8(uint8(aData[1])); - } - // Assumes that aDecimalDifference is between -18 and 18 // Assumes that aPrice is between 1 and 1e36 function packSimplePrice(int256 aDecimalDifference, uint256 aPrice) internal pure returns (bytes32 rPacked) { @@ -55,8 +48,19 @@ library RoutesLib { rSecondWord = bytes20(aThirdToken); } + // Positive value indicates that token1 has a greater number of decimals compared to token2 + // while a negative value indicates otherwise. + // range of values between -18 and 18 + function getDecimalDifference(bytes32 aData) internal pure returns (int256 rDiff) { + rDiff = int8(uint8(aData[1])); + } + function getPrice(bytes32 aData) internal pure returns (uint256 rPrice) { - rPrice = uint256(aData & 0x0000ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff); + rPrice = uint256(aData & 0x00000000ffffffffffffffffffffffffffffffffffffffffffffffffffffffff); + } + + function getBpDiffForMaxReward(bytes32 aData) internal pure returns (uint256 rBpDiffForMaxReward) { + rBpDiffForMaxReward = uint256((aData & 0x0000ffff00000000000000000000000000000000000000000000000000000000) >> 224); } function getTokenFirstWord(bytes32 aData) internal pure returns (address rToken) { From 33d098002be06916fdc667912bc875d10af51656 Mon Sep 17 00:00:00 2001 From: "A.L." Date: Fri, 26 Jul 2024 12:34:27 +0200 Subject: [PATCH 02/17] feat: calculations for payout --- src/ReservoirPriceOracle.sol | 85 ++++++++++++++++------------------ src/libraries/Constants.sol | 1 + src/libraries/OracleErrors.sol | 2 + src/libraries/RoutesLib.sol | 15 ++++-- test/__fixtures/BaseTest.t.sol | 3 +- 5 files changed, 55 insertions(+), 51 deletions(-) diff --git a/src/ReservoirPriceOracle.sol b/src/ReservoirPriceOracle.sol index 71fd5f2..3e45213 100644 --- a/src/ReservoirPriceOracle.sol +++ b/src/ReservoirPriceOracle.sol @@ -26,9 +26,9 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar // EVENTS // /////////////////////////////////////////////////////////////////////////////////////////////// + event BpDiffForMaxReward(address token0, address token1, uint16 bpForMaxReward); event DesignatePair(address token0, address token1, ReservoirPair pair); event FallbackOracleSet(address fallbackOracle); - event PriceDeviationThreshold(uint256 newThreshold); event RewardGasAmount(uint256 newAmount); event Route(address token0, address token1, address[] route); event TwapPeriod(uint256 newPeriod); @@ -44,12 +44,7 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar /// @dev If `address(0)` then there is no fallback. address public fallbackOracle; - // 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; + // The following 2 storage variables take up 1 storage slot. /// @notice This number is multiplied by the base fee to determine the reward for keepers. uint64 public rewardGasAmount; @@ -64,8 +59,7 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar // CONSTRUCTOR, FALLBACKS // /////////////////////////////////////////////////////////////////////////////////////////////// - constructor(uint64 aThreshold, uint64 aTwapPeriod, uint64 aMultiplier, PriceType aType) { - updatePriceDeviationThreshold(aThreshold); + constructor(uint64 aTwapPeriod, uint64 aMultiplier, PriceType aType) { updateTwapPeriod(aTwapPeriod); updateRewardGasAmount(aMultiplier); PRICE_TYPE = aType; @@ -128,7 +122,8 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar function updatePrice(address aTokenA, address aTokenB, address aRewardRecipient) external nonReentrant { (address lToken0, address lToken1) = Utils.sortTokens(aTokenA, aTokenB); - (address[] memory lRoute,, uint256 lPrevPrice,) = _getRouteDecimalDifferencePrice(lToken0, lToken1); + (address[] memory lRoute,, uint256 lPrevPrice, uint256 lBpDiffForMaxReward) = + _getRouteDecimalDifferencePrice(lToken0, lToken1); if (lRoute.length == 0) revert OracleErrors.NoPath(); for (uint256 i = 0; i < lRoute.length - 1; ++i) { @@ -150,11 +145,7 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar } _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); - } + _rewardUpdater(lPrevPrice, lNewPrice, aRewardRecipient, lBpDiffForMaxReward); } } @@ -192,9 +183,17 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar } } - function _rewardUpdater(address aRecipient) private { + function _rewardUpdater(uint256 aPrevPrice, uint256 aNewPrice, address aRecipient, uint256 aBpDiffForMaxReward) + private + { + uint256 lPercentDiff = _calcPercentageDiff(aPrevPrice, aNewPrice); + if (lPercentDiff == 0) return; if (aRecipient == address(0)) return; + // can be unchecked + uint256 lBpForMaxRewardWAD = aBpDiffForMaxReward * Constants.WAD / Constants.BP_SCALE; + uint256 lReward = lPercentDiff > lBpForMaxRewardWAD ? lBpForMaxRewardWAD : lPercentDiff; + // N.B. Revisit this whenever deployment on a new chain is needed // // we use `block.basefee` instead of `ArbGasInfo::getMinimumGasPrice()` @@ -205,7 +204,7 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar // SAFETY: this mul will not overflow even in extreme cases of `block.basefee`. uint256 lPayoutAmt; unchecked { - lPayoutAmt = block.basefee * rewardGasAmount; + lPayoutAmt = block.basefee * rewardGasAmount * lReward / lBpForMaxRewardWAD; } // does not revert under any circumstance @@ -270,7 +269,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 write it as well. - function _checkAndPopulateIntermediateRoute(address aTokenA, address aTokenB) private { + function _checkAndPopulateIntermediateRoute(address aTokenA, address aTokenB, uint16 aBpMaxReward) private { (address lToken0, address lToken1) = Utils.sortTokens(aTokenA, aTokenB); bytes32 lSlot = Utils.calculateSlot(lToken0, lToken1); @@ -282,16 +281,14 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar address[] memory lIntermediateRoute = new address[](2); lIntermediateRoute[0] = lToken0; lIntermediateRoute[1] = lToken1; - setRoute(lToken0, lToken1, lIntermediateRoute); + uint16[] memory asd = new uint16[](1); + asd[0] = aBpMaxReward; + setRoute(lToken0, lToken1, lIntermediateRoute, asd); } } // performs an SLOAD to load 1 word which contains the simple price and decimal difference - function _priceCache(address aToken0, address aToken1) - private - view - returns (uint256 rPrice, int256 rDecimalDiff) - { + function _priceCache(address aToken0, address aToken1) private view returns (uint256 rPrice, int256 rDecimalDiff) { bytes32 lSlot = Utils.calculateSlot(aToken0, aToken1); bytes32 lData; @@ -314,9 +311,7 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar } if (!lData.isSimplePrice()) revert OracleErrors.WriteToNonSimpleRoute(); - int256 lDiff = lData.getDecimalDifference(); - - lData = RoutesLib.packSimplePrice(lDiff, aNewPrice); + lData = RoutesLib.packSimplePrice(lData.getDecimalDifference(), aNewPrice, lData.getBpDiffForMaxReward()); assembly ("memory-safe") { sstore(lSlot, lData) } @@ -421,15 +416,6 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar emit FallbackOracleSet(aFallbackOracle); } - function updatePriceDeviationThreshold(uint64 aNewThreshold) public onlyOwner { - if (aNewThreshold > Constants.MAX_DEVIATION_THRESHOLD) { - revert OracleErrors.PriceDeviationThresholdTooHigh(); - } - - priceDeviationThreshold = aNewThreshold; - emit PriceDeviationThreshold(aNewThreshold); - } - function updateTwapPeriod(uint64 aNewPeriod) public onlyOwner { if (aNewPeriod == 0 || aNewPeriod > Constants.MAX_TWAP_PERIOD) { revert OracleErrors.InvalidTwapPeriod(); @@ -461,16 +447,21 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar emit DesignatePair(aToken0, aToken1, ReservoirPair(address(0))); } - /// @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 - /// @param aRoute Path with which the price between aToken0 and aToken1 should be derived - function setRoute(address aToken0, address aToken1, address[] memory aRoute) public onlyOwner { + /// @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. + /// @param aRoute Path with which the price between aToken0 and aToken1 should be derived. + /// @param aBpDiffForMaxReward Array of basis points at and beyond which the bounty payout for a price update is maximum. + function setRoute(address aToken0, address aToken1, address[] memory aRoute, uint16[] memory aBpDiffForMaxReward) + public + onlyOwner + { uint256 lRouteLength = aRoute.length; _validateTokens(aToken0, aToken1); if (lRouteLength > Constants.MAX_ROUTE_LENGTH || lRouteLength < 2) revert OracleErrors.InvalidRouteLength(); if (aRoute[0] != aToken0 || aRoute[lRouteLength - 1] != aToken1) revert OracleErrors.InvalidRoute(); + if (aBpDiffForMaxReward.length != lRouteLength - 1) revert OracleErrors.InvalidArrayLengthBpForMaxReward(); bytes32 lSlot = Utils.calculateSlot(aToken0, aToken1); @@ -482,11 +473,15 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar int256 lDiff = int256(lToken1Decimals) - int256(lToken0Decimals); - bytes32 lData = RoutesLib.packSimplePrice(lDiff, 0); + if (aBpDiffForMaxReward[0] > Constants.BP_SCALE) revert OracleErrors.InvalidBpForMaxReward(); + + bytes32 lData = RoutesLib.packSimplePrice(lDiff, 0, aBpDiffForMaxReward[0]); assembly ("memory-safe") { // Write data to storage. sstore(lSlot, lData) } + + emit BpDiffForMaxReward(aToken0, aToken1, aBpDiffForMaxReward[0]); } // composite route else { @@ -506,10 +501,10 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar sstore(lSlot, lFirstWord) sstore(add(lSlot, 1), lSecondWord) } - _checkAndPopulateIntermediateRoute(lThirdToken, aToken1); + _checkAndPopulateIntermediateRoute(lThirdToken, aToken1, aBpDiffForMaxReward[2]); } - _checkAndPopulateIntermediateRoute(aToken0, lSecondToken); - _checkAndPopulateIntermediateRoute(lSecondToken, lThirdToken); + _checkAndPopulateIntermediateRoute(aToken0, lSecondToken, aBpDiffForMaxReward[0]); + _checkAndPopulateIntermediateRoute(lSecondToken, lThirdToken, aBpDiffForMaxReward[1]); } emit Route(aToken0, aToken1, aRoute); } diff --git a/src/libraries/Constants.sol b/src/libraries/Constants.sol index e69032b..6ea6af9 100644 --- a/src/libraries/Constants.sol +++ b/src/libraries/Constants.sol @@ -12,4 +12,5 @@ library Constants { uint256 public constant WAD = 1e18; uint256 public constant MAX_SUPPORTED_PRICE = type(uint128).max; uint256 public constant MAX_AMOUNT_IN = type(uint128).max; + uint256 public constant BP_SCALE = 1e4; } diff --git a/src/libraries/OracleErrors.sol b/src/libraries/OracleErrors.sol index 310f313..fcc8713 100644 --- a/src/libraries/OracleErrors.sol +++ b/src/libraries/OracleErrors.sol @@ -5,6 +5,8 @@ pragma solidity ^0.8.0; library OracleErrors { // config errors error IncorrectTokensDesignatePair(); + error InvalidBpForMaxReward(); + error InvalidArrayLengthBpForMaxReward(); error InvalidRoute(); error InvalidRouteLength(); error InvalidTokensProvided(); diff --git a/src/libraries/RoutesLib.sol b/src/libraries/RoutesLib.sol index 0a01b69..1200971 100644 --- a/src/libraries/RoutesLib.sol +++ b/src/libraries/RoutesLib.sol @@ -29,9 +29,15 @@ library RoutesLib { // Assumes that aDecimalDifference is between -18 and 18 // Assumes that aPrice is between 1 and 1e36 - function packSimplePrice(int256 aDecimalDifference, uint256 aPrice) internal pure returns (bytes32 rPacked) { + // Assumes that aBpDiffForMaxReward is <= Constants.BP_SCALE + function packSimplePrice(int256 aDecimalDifference, uint256 aPrice, uint16 aBpDiffForMaxReward) + internal + pure + returns (bytes32 rPacked) + { bytes32 lDecimalDifferenceRaw = bytes1(uint8(int8(aDecimalDifference))); - rPacked = FLAG_SIMPLE_PRICE | lDecimalDifferenceRaw >> 8 | bytes32(aPrice); + bytes32 lBpDiffForMaxReward = bytes2(uint16(aBpDiffForMaxReward)); + rPacked = FLAG_SIMPLE_PRICE | lDecimalDifferenceRaw >> 8 | lBpDiffForMaxReward >> 16 | bytes32(aPrice); } function pack2HopRoute(address aSecondToken) internal pure returns (bytes32 rPacked) { @@ -59,8 +65,9 @@ library RoutesLib { rPrice = uint256(aData & 0x00000000ffffffffffffffffffffffffffffffffffffffffffffffffffffffff); } - function getBpDiffForMaxReward(bytes32 aData) internal pure returns (uint256 rBpDiffForMaxReward) { - rBpDiffForMaxReward = uint256((aData & 0x0000ffff00000000000000000000000000000000000000000000000000000000) >> 224); + function getBpDiffForMaxReward(bytes32 aData) internal pure returns (uint16 rBpDiffForMaxReward) { + rBpDiffForMaxReward = + uint16(uint256((aData & 0x0000ffff00000000000000000000000000000000000000000000000000000000) >> 224)); } function getTokenFirstWord(bytes32 aData) internal pure returns (address rToken) { diff --git a/test/__fixtures/BaseTest.t.sol b/test/__fixtures/BaseTest.t.sol index 39aec87..52f2db0 100644 --- a/test/__fixtures/BaseTest.t.sol +++ b/test/__fixtures/BaseTest.t.sol @@ -19,8 +19,7 @@ contract BaseTest is Test { GenericFactory internal _factory = new GenericFactory(); ReservoirPair internal _pair; - ReservoirPriceOracle internal _oracle = - new ReservoirPriceOracle(0.02e18, 15 minutes, 500_000, PriceType.CLAMPED_PRICE); + ReservoirPriceOracle internal _oracle = new ReservoirPriceOracle(0.15 minutes, 500_000, PriceType.CLAMPED_PRICE); MintableERC20 internal _tokenA = MintableERC20(address(0x100)); MintableERC20 internal _tokenB = MintableERC20(address(0x200)); From cf88644e3596932cf64d3a2565e74fc9bba3c8b5 Mon Sep 17 00:00:00 2001 From: "A.L." Date: Sat, 27 Jul 2024 22:02:09 +0200 Subject: [PATCH 03/17] test: refactor tests to make it work --- src/libraries/Constants.sol | 2 +- test/unit/ReservoirPriceOracle.t.sol | 89 ++++++++++++++++++---------- test/unit/libraries/RoutesLib.t.sol | 4 +- 3 files changed, 62 insertions(+), 33 deletions(-) diff --git a/src/libraries/Constants.sol b/src/libraries/Constants.sol index 6ea6af9..175c3b9 100644 --- a/src/libraries/Constants.sol +++ b/src/libraries/Constants.sol @@ -12,5 +12,5 @@ library Constants { uint256 public constant WAD = 1e18; uint256 public constant MAX_SUPPORTED_PRICE = type(uint128).max; uint256 public constant MAX_AMOUNT_IN = type(uint128).max; - uint256 public constant BP_SCALE = 1e4; + uint16 public constant BP_SCALE = 1e4; } diff --git a/test/unit/ReservoirPriceOracle.t.sol b/test/unit/ReservoirPriceOracle.t.sol index f315c50..9ded8d1 100644 --- a/test/unit/ReservoirPriceOracle.t.sol +++ b/test/unit/ReservoirPriceOracle.t.sol @@ -52,7 +52,8 @@ contract ReservoirPriceOracleTest is BaseTest { require(lAccesses.length == 1, "incorrect number of accesses"); int256 lDecimalDiff = int256(uint256(IERC20(aToken1).decimals())) - int256(uint256(IERC20(aToken0).decimals())); - bytes32 lData = lDecimalDiff.packSimplePrice(aPrice); + uint16 lBpDiffMaxReward = Constants.BP_SCALE; + bytes32 lData = lDecimalDiff.packSimplePrice(aPrice, lBpDiffMaxReward); require(lData.getDecimalDifference() == lDecimalDiff, "decimal diff incorrect"); require(lData.isSimplePrice(), "flag incorrect"); vm.store(address(_oracle), lAccesses[0], lData); @@ -81,11 +82,12 @@ contract ReservoirPriceOracleTest is BaseTest { function setUp() external { // define route address[] memory lRoute = new address[](2); + uint16[] memory lBpDiffForMaxReward = new uint16[](1); lRoute[0] = address(_tokenA); lRoute[1] = address(_tokenB); _oracle.designatePair(address(_tokenB), address(_tokenA), _pair); - _oracle.setRoute(address(_tokenA), address(_tokenB), lRoute); + _oracle.setRoute(address(_tokenA), address(_tokenB), lRoute, lBpDiffForMaxReward); } function testWritePriceCache(uint256 aPrice) external { @@ -144,11 +146,14 @@ contract ReservoirPriceOracleTest is BaseTest { _writePriceCache(address(_tokenC), address(_tokenD), lPriceCD); address[] memory lRoute = new address[](4); + uint16[] memory lBpDiffForMaxReward = new uint16[](3); + lBpDiffForMaxReward[0] = lBpDiffForMaxReward[1] = lBpDiffForMaxReward[2] = Constants.BP_SCALE; + lRoute[0] = address(_tokenA); lRoute[1] = address(_tokenB); lRoute[2] = address(_tokenC); lRoute[3] = address(_tokenD); - _oracle.setRoute(address(_tokenA), address(_tokenD), lRoute); + _oracle.setRoute(address(_tokenA), address(_tokenD), lRoute, lBpDiffForMaxReward); uint256 lAmountIn = 789e6; @@ -171,11 +176,13 @@ contract ReservoirPriceOracleTest is BaseTest { _writePriceCache(address(_tokenC), address(_tokenD), lPriceCD); address[] memory lRoute = new address[](4); + uint16[] memory lBpDiffForMaxReward = new uint16[](3); + lBpDiffForMaxReward[0] = lBpDiffForMaxReward[1] = lBpDiffForMaxReward[2] = Constants.BP_SCALE; lRoute[0] = address(_tokenA); lRoute[1] = address(_tokenB); lRoute[2] = address(_tokenC); lRoute[3] = address(_tokenD); - _oracle.setRoute(address(_tokenA), address(_tokenD), lRoute); + _oracle.setRoute(address(_tokenA), address(_tokenD), lRoute, lBpDiffForMaxReward); uint256 lAmountIn = 789e6; @@ -211,9 +218,13 @@ contract ReservoirPriceOracleTest is BaseTest { lRoute[1] = address(lTokenB); lRoute[2] = address(lTokenC); - _oracle.setRoute(address(lTokenA), address(lTokenC), lRoute); + { + uint16[] memory lBpDiffForMaxReward = new uint16[](2); + lBpDiffForMaxReward[0] = lBpDiffForMaxReward[1] = Constants.BP_SCALE; + _oracle.setRoute(address(lTokenA), address(lTokenC), lRoute, lBpDiffForMaxReward); _writePriceCache(address(lTokenA), address(lTokenB), 1e18); _writePriceCache(address(lTokenC), address(lTokenB), 1e18); + } // act uint256 lAmtCOut = _oracle.getQuote(10 ** lTokenADecimals, address(lTokenA), address(lTokenC)); @@ -252,9 +263,10 @@ contract ReservoirPriceOracleTest is BaseTest { _oracle.designatePair(address(lTokenA), address(lTokenB), lPair); address[] memory lRoute = new address[](2); + uint16[] memory lBpDiffForMaxReward = new uint16[](1); (lRoute[0], lRoute[1]) = lTokenA < lTokenB ? (address(lTokenA), address(lTokenB)) : (address(lTokenB), address(lTokenA)); - _oracle.setRoute(lRoute[0], lRoute[1], lRoute); + _oracle.setRoute(lRoute[0], lRoute[1], lRoute, lBpDiffForMaxReward); _writePriceCache(lRoute[0], lRoute[1], lPrice); // price written could be tokenB/tokenA or tokenA/tokenB depending on the fuzz addresses // act @@ -312,7 +324,8 @@ contract ReservoirPriceOracleTest is BaseTest { lTokenA < lTokenC ? (address(lTokenA), address(lTokenC)) : (address(lTokenC), address(lTokenA)); lRoute[1] = address(lTokenB); - _oracle.setRoute(lRoute[0], lRoute[2], lRoute); + uint16[] memory lBpDiffMaxReward = new uint16[](2); + _oracle.setRoute(lRoute[0], lRoute[2], lRoute, lBpDiffMaxReward); _writePriceCache( address(lTokenA) < address(lTokenB) ? address(lTokenA) : address(lTokenB), address(lTokenA) < address(lTokenB) ? address(lTokenB) : address(lTokenA), @@ -402,17 +415,6 @@ contract ReservoirPriceOracleTest is BaseTest { assertEq(lAmtOut / 1e12, lAmtIn * lRate / 1e18); } - function testUpdatePriceDeviationThreshold(uint256 aNewThreshold) external { - // assume - uint64 lNewThreshold = uint64(bound(aNewThreshold, 0, 0.1e18)); - - // act - _oracle.updatePriceDeviationThreshold(lNewThreshold); - - // assert - assertEq(_oracle.priceDeviationThreshold(), lNewThreshold); - } - function testUpdateTwapPeriod(uint256 aNewPeriod) external { // assume uint64 lNewPeriod = uint64(bound(aNewPeriod, 1, 1 hours)); @@ -554,11 +556,13 @@ contract ReservoirPriceOracleTest is BaseTest { address lIntermediate2 = address(_tokenD); address lEnd = address(_tokenB); address[] memory lRoute = new address[](4); + uint16[] memory lBpDiffForMaxReward = new uint16[](3); + lBpDiffForMaxReward[0] = lBpDiffForMaxReward[1] = lBpDiffForMaxReward[2] = Constants.BP_SCALE; lRoute[0] = lStart; lRoute[1] = lIntermediate1; lRoute[2] = lIntermediate2; lRoute[3] = lEnd; - _oracle.setRoute(lStart, lEnd, lRoute); + _oracle.setRoute(lStart, lEnd, lRoute, lBpDiffForMaxReward); ReservoirPair lAC = ReservoirPair(_createPair(address(_tokenA), address(_tokenC), 0)); ReservoirPair lCD = ReservoirPair(_createPair(address(_tokenC), address(_tokenD), 0)); @@ -606,13 +610,15 @@ contract ReservoirPriceOracleTest is BaseTest { address lToken0 = address(_tokenB); address lToken1 = address(_tokenC); address[] memory lRoute = new address[](2); + uint16[] memory lBpDiffForMaxReward = new uint16[](1); + lBpDiffForMaxReward[0] = Constants.BP_SCALE; lRoute[0] = lToken0; lRoute[1] = lToken1; // act vm.expectEmit(false, false, false, false); emit Route(lToken0, lToken1, lRoute); - _oracle.setRoute(lToken0, lToken1, lRoute); + _oracle.setRoute(lToken0, lToken1, lRoute, lBpDiffForMaxReward); // assert address[] memory lQueriedRoute = _oracle.route(lToken0, lToken1); @@ -628,13 +634,15 @@ contract ReservoirPriceOracleTest is BaseTest { address lToken0 = address(_tokenB); address lToken1 = address(_tokenC); address[] memory lRoute = new address[](4); + uint16[] memory lBpDiffForMaxReward = new uint16[](3); + lBpDiffForMaxReward[0] = lBpDiffForMaxReward[1] = lBpDiffForMaxReward[2] = Constants.BP_SCALE; lRoute[0] = lToken0; lRoute[1] = address(_tokenA); lRoute[2] = address(_tokenD); lRoute[3] = lToken1; // act - _oracle.setRoute(lToken0, lToken1, lRoute); + _oracle.setRoute(lToken0, lToken1, lRoute, lBpDiffForMaxReward); // assert address[] memory lQueriedRoute = _oracle.route(lToken0, lToken1); @@ -667,6 +675,9 @@ contract ReservoirPriceOracleTest is BaseTest { lIntermediateRoute3[0] = lIntermediate2; lIntermediateRoute3[1] = lEnd; + uint16[] memory lBpDiffForMaxReward = new uint16[](3); + lBpDiffForMaxReward[0] = lBpDiffForMaxReward[1] = lBpDiffForMaxReward[2] = Constants.BP_SCALE; + // act vm.expectEmit(false, false, false, true); emit Route(lIntermediate2, lEnd, lIntermediateRoute3); @@ -677,7 +688,8 @@ contract ReservoirPriceOracleTest is BaseTest { emit Route(lIntermediate2, lIntermediate1, lIntermediateRoute2); vm.expectEmit(false, false, false, true); emit Route(lStart, lEnd, lRoute); - _oracle.setRoute(lStart, lEnd, lRoute); + + _oracle.setRoute(lStart, lEnd, lRoute, lBpDiffForMaxReward); // assert assertEq(_oracle.route(lStart, lEnd), lRoute); @@ -691,9 +703,11 @@ contract ReservoirPriceOracleTest is BaseTest { address lToken0 = address(_tokenB); address lToken1 = address(_tokenC); address[] memory lRoute = new address[](2); + uint16[] memory lBpDiffForMaxReward = new uint16[](1); + lBpDiffForMaxReward[0] = Constants.BP_SCALE; lRoute[0] = lToken0; lRoute[1] = lToken1; - _oracle.setRoute(lToken0, lToken1, lRoute); + _oracle.setRoute(lToken0, lToken1, lRoute, lBpDiffForMaxReward); address[] memory lQueriedRoute = _oracle.route(lToken0, lToken1); assertEq(lQueriedRoute, lRoute); _writePriceCache(lToken0, lToken1, 1e18); @@ -713,11 +727,13 @@ contract ReservoirPriceOracleTest is BaseTest { function testClearRoute_AllWordsCleared() external { // arrange address[] memory lRoute = new address[](4); + uint16[] memory lBpDiffForMaxReward = new uint16[](3); + lBpDiffForMaxReward[0] = lBpDiffForMaxReward[1] = lBpDiffForMaxReward[2] = Constants.BP_SCALE; lRoute[0] = address(_tokenA); lRoute[1] = address(_tokenC); lRoute[2] = address(_tokenB); lRoute[3] = address(_tokenD); - _oracle.setRoute(address(_tokenA), address(_tokenD), lRoute); + _oracle.setRoute(address(_tokenA), address(_tokenD), lRoute, lBpDiffForMaxReward); address[] memory lQueriedRoute = _oracle.route(address(_tokenA), address(_tokenD)); assertEq(lQueriedRoute, lRoute); bytes32 lSlot1 = address(_tokenA).calculateSlot(address(_tokenD)); @@ -838,11 +854,13 @@ contract ReservoirPriceOracleTest is BaseTest { lPair.sync(); address[] memory lRoute = new address[](2); + uint16[] memory lBpDiffForMaxReward = new uint16[](1); + lBpDiffForMaxReward[0] = Constants.BP_SCALE; lRoute[0] = address(_tokenB); lRoute[1] = address(_tokenC); _oracle.designatePair(address(_tokenB), address(_tokenC), lPair); - _oracle.setRoute(address(_tokenB), address(_tokenC), lRoute); + _oracle.setRoute(address(_tokenB), address(_tokenC), lRoute, lBpDiffForMaxReward); // act & assert vm.expectRevert( @@ -859,12 +877,14 @@ contract ReservoirPriceOracleTest is BaseTest { address lToken0 = address(0x1); address lToken1 = address(0x1); address[] memory lRoute = new address[](2); + uint16[] memory lBpDiffForMaxReward = new uint16[](1); + lBpDiffForMaxReward[0] = Constants.BP_SCALE; lRoute[0] = lToken0; lRoute[1] = lToken1; // act & assert vm.expectRevert(OracleErrors.InvalidTokensProvided.selector); - _oracle.setRoute(lToken0, lToken1, lRoute); + _oracle.setRoute(lToken0, lToken1, lRoute, lBpDiffForMaxReward); } function testSetRoute_NotSorted() external { @@ -872,12 +892,14 @@ contract ReservoirPriceOracleTest is BaseTest { address lToken0 = address(0x21); address lToken1 = address(0x2); address[] memory lRoute = new address[](2); + uint16[] memory lBpDiffForMaxReward = new uint16[](1); + lBpDiffForMaxReward[0] = Constants.BP_SCALE; lRoute[0] = lToken0; lRoute[1] = lToken1; // act & assert vm.expectRevert(OracleErrors.InvalidTokensProvided.selector); - _oracle.setRoute(lToken0, lToken1, lRoute); + _oracle.setRoute(lToken0, lToken1, lRoute, lBpDiffForMaxReward); } function testSetRoute_InvalidRouteLength() external { @@ -885,6 +907,8 @@ contract ReservoirPriceOracleTest is BaseTest { address lToken0 = address(0x1); address lToken1 = address(0x2); address[] memory lTooLong = new address[](5); + uint16[] memory lBpDiffForMaxReward = new uint16[](1); + lBpDiffForMaxReward[0] = Constants.BP_SCALE; lTooLong[0] = lToken0; lTooLong[1] = address(0); lTooLong[2] = address(0); @@ -895,11 +919,11 @@ contract ReservoirPriceOracleTest is BaseTest { // act & assert vm.expectRevert(OracleErrors.InvalidRouteLength.selector); - _oracle.setRoute(lToken0, lToken1, lTooLong); + _oracle.setRoute(lToken0, lToken1, lTooLong, lBpDiffForMaxReward); // act & assert vm.expectRevert(OracleErrors.InvalidRouteLength.selector); - _oracle.setRoute(lToken0, lToken1, lTooShort); + _oracle.setRoute(lToken0, lToken1, lTooShort, lBpDiffForMaxReward); } function testSetRoute_InvalidRoute() external { @@ -916,11 +940,14 @@ contract ReservoirPriceOracleTest is BaseTest { lInvalidRoute2[1] = address(54); lInvalidRoute2[2] = lToken1; + uint16[] memory lBpDiffForMaxReward = new uint16[](2); + lBpDiffForMaxReward[0] = lBpDiffForMaxReward[1] = Constants.BP_SCALE; + // act & assert vm.expectRevert(OracleErrors.InvalidRoute.selector); - _oracle.setRoute(lToken0, lToken1, lInvalidRoute1); + _oracle.setRoute(lToken0, lToken1, lInvalidRoute1, lBpDiffForMaxReward); vm.expectRevert(OracleErrors.InvalidRoute.selector); - _oracle.setRoute(lToken0, lToken1, lInvalidRoute2); + _oracle.setRoute(lToken0, lToken1, lInvalidRoute2, lBpDiffForMaxReward); } function testUpdateRewardGasAmount_NotOwner() external { diff --git a/test/unit/libraries/RoutesLib.t.sol b/test/unit/libraries/RoutesLib.t.sol index 3b1db8d..3586e23 100644 --- a/test/unit/libraries/RoutesLib.t.sol +++ b/test/unit/libraries/RoutesLib.t.sol @@ -4,6 +4,7 @@ pragma solidity ^0.8.0; import { Test, console2, stdError } from "forge-std/Test.sol"; import { RoutesLib } from "src/libraries/RoutesLib.sol"; +import { Constants } from "src/libraries/Constants.sol"; contract RoutesLibTest is Test { using RoutesLib for bytes32; @@ -40,11 +41,12 @@ contract RoutesLibTest is Test { uint256 lPrice = bound(aPrice, 1, 1e36); // act - bytes32 lResult = int256(aDiff).packSimplePrice(lPrice); + bytes32 lResult = int256(aDiff).packSimplePrice(lPrice, Constants.BP_SCALE); // assert assertEq(lResult[0], RoutesLib.FLAG_SIMPLE_PRICE); assertEq(lResult[1], bytes1(uint8(aDiff))); + assertEq(lResult.getBpDiffForMaxReward(), Constants.BP_SCALE); assertEq(lResult.getPrice(), lPrice); } } From b03603655dd2c48efb4fdabd26b9dc953ad731cf Mon Sep 17 00:00:00 2001 From: "A.L." Date: Sun, 28 Jul 2024 00:25:26 +0200 Subject: [PATCH 04/17] feat: expose bpDiffForMaxReward in public function --- src/ReservoirPriceOracle.sol | 46 ++++++------ src/libraries/Utils.sol | 14 ++++ test/unit/ReservoirPriceOracle.t.sol | 106 ++++++++++++++++++--------- 3 files changed, 107 insertions(+), 59 deletions(-) diff --git a/src/ReservoirPriceOracle.sol b/src/ReservoirPriceOracle.sol index 3e45213..acb13da 100644 --- a/src/ReservoirPriceOracle.sol +++ b/src/ReservoirPriceOracle.sol @@ -20,6 +20,7 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar using FixedPointMathLib for uint256; using LibSort for address[]; using RoutesLib for bytes32; + using Utils for uint256; using QueryProcessor for ReservoirPair; /////////////////////////////////////////////////////////////////////////////////////////////// @@ -107,8 +108,13 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar /// @param aToken1 Address of the higher token. /// @return rPrice The cached price of aToken0/aToken1 for simple routes. Returns 0 for prices of composite routes. /// @return rDecimalDiff The difference in decimals as defined by aToken1.decimals() - aToken0.decimals(). Only valid for simple routes. - function priceCache(address aToken0, address aToken1) external view returns (uint256 rPrice, int256 rDecimalDiff) { - (rPrice, rDecimalDiff) = _priceCache(aToken0, aToken1); + /// @return rBpDiffForMaxReward The number of basis points at and beyond which the bounty payout for a price update is maximum. + function priceCache(address aToken0, address aToken1) + external + view + returns (uint256 rPrice, int256 rDecimalDiff, uint256 rBpDiffForMaxReward) + { + (rPrice, rDecimalDiff, rBpDiffForMaxReward) = _priceCache(aToken0, aToken1); } /// @notice Updates the TWAP price for all simple routes between `aTokenA` and `aTokenB`. Will also update intermediate routes if the route defined between @@ -141,7 +147,7 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar // if it's a simple route, we avoid loading the price again from storage if (lRoute.length != 2) { - (lPrevPrice,) = _priceCache(lToken0, lToken1); + (lPrevPrice,,) = _priceCache(lToken0, lToken1); } _writePriceCache(lToken0, lToken1, lNewPrice); @@ -169,29 +175,18 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar rResult = lPair.getTimeWeightedAverage(aQuery.priceType, aQuery.secs, aQuery.ago, lIndex); } - function _calcPercentageDiff(uint256 aOriginal, uint256 aNew) private pure returns (uint256) { - unchecked { - if (aOriginal == 0) return 0; - - // 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; - } - } - } - function _rewardUpdater(uint256 aPrevPrice, uint256 aNewPrice, address aRecipient, uint256 aBpDiffForMaxReward) private { - uint256 lPercentDiff = _calcPercentageDiff(aPrevPrice, aNewPrice); + uint256 lPercentDiff = aPrevPrice.calcPercentageDiff(aNewPrice); if (lPercentDiff == 0) return; if (aRecipient == address(0)) return; - // can be unchecked - uint256 lBpForMaxRewardWAD = aBpDiffForMaxReward * Constants.WAD / Constants.BP_SCALE; + // SAFETY: this mul will not overflow as `aBpDiffForMaxReward` is capped by `Constants.BP_SCALE` + uint256 lBpForMaxRewardWAD; + unchecked { + lBpForMaxRewardWAD = aBpDiffForMaxReward * Constants.WAD / Constants.BP_SCALE; + } uint256 lReward = lPercentDiff > lBpForMaxRewardWAD ? lBpForMaxRewardWAD : lPercentDiff; // N.B. Revisit this whenever deployment on a new chain is needed @@ -287,8 +282,12 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar } } - // performs an SLOAD to load 1 word which contains the simple price and decimal difference - function _priceCache(address aToken0, address aToken1) private view returns (uint256 rPrice, int256 rDecimalDiff) { + // performs an SLOAD to load 1 word which contains the simple price, decimal difference, and number of basis points for maximum reward + function _priceCache(address aToken0, address aToken1) + private + view + returns (uint256 rPrice, int256 rDecimalDiff, uint256 rBpDiffForMaxReward) + { bytes32 lSlot = Utils.calculateSlot(aToken0, aToken1); bytes32 lData; @@ -298,6 +297,7 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar if (lData.isSimplePrice()) { rPrice = lData.getPrice(); rDecimalDiff = lData.getDecimalDifference(); + rBpDiffForMaxReward = lData.getBpDiffForMaxReward(); } } @@ -357,7 +357,7 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar for (uint256 i = 0; i < lRoute.length - 1; ++i) { (lToken0, lToken1) = Utils.sortTokens(lRoute[i], lRoute[i + 1]); // it is assumed that intermediate routes defined here are simple routes and not composite routes - (lPrice, lDecimalDiff) = _priceCache(lToken0, lToken1); + (lPrice, lDecimalDiff,) = _priceCache(lToken0, lToken1); if (lPrice == 0) revert OracleErrors.PriceZero(); lIntermediateAmount = _calcAmtOut(lIntermediateAmount, lPrice, lDecimalDiff, lRoute[i] != lToken0); diff --git a/src/libraries/Utils.sol b/src/libraries/Utils.sol index 2e8dae8..f7ec76a 100644 --- a/src/libraries/Utils.sol +++ b/src/libraries/Utils.sol @@ -11,4 +11,18 @@ library Utils { function calculateSlot(address aToken0, address aToken1) internal pure returns (bytes32) { return keccak256(abi.encode(aToken0, aToken1)); } + + /// @dev Assumes that `aOriginal` and `aNew` is less than or equal to + /// `Constants.MAX_SUPPORTED_PRICE`. So multiplication by 1e18 will not overflow. + function calcPercentageDiff(uint256 aOriginal, uint256 aNew) internal pure returns (uint256) { + unchecked { + if (aOriginal == 0) return 0; + + if (aOriginal > aNew) { + return (aOriginal - aNew) * 1e18 / aOriginal; + } else { + return (aNew - aOriginal) * 1e18 / aOriginal; + } + } + } } diff --git a/test/unit/ReservoirPriceOracle.t.sol b/test/unit/ReservoirPriceOracle.t.sol index 9ded8d1..c8017e2 100644 --- a/test/unit/ReservoirPriceOracle.t.sol +++ b/test/unit/ReservoirPriceOracle.t.sol @@ -44,7 +44,7 @@ contract ReservoirPriceOracleTest is BaseTest { // writes the cached prices, for easy testing function _writePriceCache(address aToken0, address aToken1, uint256 aPrice) internal { require(aToken0 < aToken1, "tokens unsorted"); - require(bytes32(aPrice) & bytes2(0xffff) == 0, "PRICE WILL OVERLAP FLAG"); + require(aPrice <= Constants.MAX_SUPPORTED_PRICE, "price too large"); vm.record(); _oracle.priceCache(aToken0, aToken1); @@ -98,7 +98,7 @@ contract ReservoirPriceOracleTest is BaseTest { _writePriceCache(address(_tokenB), address(_tokenC), lPrice); // assert - (uint256 lQueriedPrice,) = _oracle.priceCache(address(_tokenB), address(_tokenC)); + (uint256 lQueriedPrice,,) = _oracle.priceCache(address(_tokenB), address(_tokenC)); assertEq(lQueriedPrice, lPrice); } @@ -124,7 +124,7 @@ contract ReservoirPriceOracleTest is BaseTest { // arrange _writePriceCache(address(_tokenA), address(_tokenB), lPrice); - (uint256 lQueriedPrice,) = _oracle.priceCache(address(_tokenA), address(_tokenB)); + (uint256 lQueriedPrice,,) = _oracle.priceCache(address(_tokenA), address(_tokenB)); assertEq(lQueriedPrice, lPrice); // act @@ -219,11 +219,11 @@ contract ReservoirPriceOracleTest is BaseTest { lRoute[2] = address(lTokenC); { - uint16[] memory lBpDiffForMaxReward = new uint16[](2); - lBpDiffForMaxReward[0] = lBpDiffForMaxReward[1] = Constants.BP_SCALE; - _oracle.setRoute(address(lTokenA), address(lTokenC), lRoute, lBpDiffForMaxReward); - _writePriceCache(address(lTokenA), address(lTokenB), 1e18); - _writePriceCache(address(lTokenC), address(lTokenB), 1e18); + uint16[] memory lBpDiffForMaxReward = new uint16[](2); + lBpDiffForMaxReward[0] = lBpDiffForMaxReward[1] = Constants.BP_SCALE; + _oracle.setRoute(address(lTokenA), address(lTokenC), lRoute, lBpDiffForMaxReward); + _writePriceCache(address(lTokenA), address(lTokenB), 1e18); + _writePriceCache(address(lTokenC), address(lTokenB), 1e18); } // act @@ -415,6 +415,17 @@ contract ReservoirPriceOracleTest is BaseTest { assertEq(lAmtOut / 1e12, lAmtIn * lRate / 1e18); } + function testPriceCache_Inverted() external { + // arrange + _writePriceCache(address(_tokenA), address(_tokenB), 1e18); + + // act + (uint256 lPrice,,) = _oracle.priceCache(address(_tokenB), address(_tokenA)); + + // assert + assertEq(lPrice, 0); + } + function testUpdateTwapPeriod(uint256 aNewPeriod) external { // assume uint64 lNewPeriod = uint64(bound(aNewPeriod, 1, 1 hours)); @@ -439,9 +450,9 @@ contract ReservoirPriceOracleTest is BaseTest { assertEq(_oracle.rewardGasAmount(), lNewRewardMultiplier); } - function testUpdatePrice_FirstUpdate() external { + function testUpdatePrice_FirstUpdate() public { // sanity - (uint256 lPrice,) = _oracle.priceCache(address(_tokenA), address(_tokenB)); + (uint256 lPrice,,) = _oracle.priceCache(address(_tokenA), address(_tokenB)); assertEq(lPrice, 0); // arrange @@ -450,23 +461,41 @@ contract ReservoirPriceOracleTest is BaseTest { skip(1); _pair.sync(); skip(_oracle.twapPeriod() * 2); - _tokenA.mint(address(_pair), 2e18); - _pair.swap(2e18, true, address(this), ""); + _pair.sync(); // act _oracle.updatePrice(address(_tokenB), address(_tokenA), address(this)); // assert - (lPrice,) = _oracle.priceCache(address(_tokenA), address(_tokenB)); + (lPrice,,) = _oracle.priceCache(address(_tokenA), address(_tokenB)); assertEq(lPrice, 98_918_868_099_219_913_512); - (lPrice,) = _oracle.priceCache(address(_tokenB), address(_tokenA)); + (lPrice,,) = _oracle.priceCache(address(_tokenB), address(_tokenA)); assertEq(lPrice, 0); assertEq(address(this).balance, 0); // there should be no reward for the first price update } - function testUpdatePrice_WithinThreshold() external { + function testUpdatePrice_NoPriceChange() external { // arrange - _writePriceCache(address(_tokenA), address(_tokenB), 98.9223e18); + testUpdatePrice_FirstUpdate(); + uint256 lExpectedPrice = 98_918_868_099_219_913_512; + (uint256 lPrice,,) = _oracle.priceCache(address(_tokenA), address(_tokenB)); + assertEq(lPrice, lExpectedPrice); // ensure that there is a price to begin with + skip(_oracle.twapPeriod() * 2); + _pair.sync(); + + // act + _oracle.updatePrice(address(_tokenA), address(_tokenB), address(this)); + + // assert + (lPrice,,) = _oracle.priceCache(address(_tokenA), address(_tokenB)); + assertEq(lPrice, lExpectedPrice); + assertEq(address(this).balance, 0); // no reward as the price did not change at all + } + + function testUpdatePrice_BelowMaxReward() external { + // arrange + uint256 lOriginalPrice = 98.9223e18; + _writePriceCache(address(_tokenA), address(_tokenB), lOriginalPrice); deal(address(_oracle), 1 ether); skip(1); @@ -479,14 +508,14 @@ contract ReservoirPriceOracleTest is BaseTest { _oracle.updatePrice(address(_tokenB), address(_tokenA), address(this)); // assert - (uint256 lPrice,) = _oracle.priceCache(address(_tokenA), address(_tokenB)); + (uint256 lPrice,,) = _oracle.priceCache(address(_tokenA), address(_tokenB)); assertEq(lPrice, 98_918_868_099_219_913_512); - (lPrice,) = _oracle.priceCache(address(_tokenB), address(_tokenA)); - assertEq(lPrice, 0); - assertEq(address(this).balance, 0); // no reward since price is within threshold + uint256 lExpectedRewardReceived = + block.basefee * _oracle.rewardGasAmount() * lOriginalPrice.calcPercentageDiff(lPrice) / 1e18; + assertEq(address(this).balance, lExpectedRewardReceived); // some reward received but is less than max possible reward } - function testUpdatePrice_BeyondThreshold() external { + function testUpdatePrice_BeyondMaxReward() external { // arrange _writePriceCache(address(_tokenA), address(_tokenB), 5e18); deal(address(_oracle), 1 ether); @@ -501,15 +530,16 @@ contract ReservoirPriceOracleTest is BaseTest { _oracle.updatePrice(address(_tokenB), address(_tokenA), address(this)); // assert - (uint256 lPrice,) = _oracle.priceCache(address(_tokenA), address(_tokenB)); + (uint256 lPrice,,) = _oracle.priceCache(address(_tokenA), address(_tokenB)); assertEq(lPrice, 98_918_868_099_219_913_512); - (lPrice,) = _oracle.priceCache(address(_tokenB), address(_tokenA)); + (lPrice,,) = _oracle.priceCache(address(_tokenB), address(_tokenA)); assertEq(lPrice, 0); - assertEq(address(this).balance, block.basefee * _oracle.rewardGasAmount()); - assertEq(address(_oracle).balance, 1 ether - block.basefee * _oracle.rewardGasAmount()); + uint256 lExpectedRewardReceived = block.basefee * _oracle.rewardGasAmount(); + assertEq(address(this).balance, lExpectedRewardReceived); + assertEq(address(_oracle).balance, 1 ether - lExpectedRewardReceived); } - function testUpdatePrice_BeyondThreshold_InsufficientReward(uint256 aRewardAvailable) external { + function testUpdatePrice_RewardEligible_InsufficientReward(uint256 aRewardAvailable) external { // assume uint256 lRewardAvailable = bound(aRewardAvailable, 1, block.basefee * _oracle.rewardGasAmount() - 1); @@ -526,11 +556,13 @@ contract ReservoirPriceOracleTest is BaseTest { // act _oracle.updatePrice(address(_tokenA), address(_tokenB), address(this)); - // assert - assertEq(address(this).balance, 0); // no reward as there's insufficient ether in the contract + // assert - no reward as there's insufficient ether in the contract, but price cache updated nonetheless + (uint256 lPrice,,) = _oracle.priceCache(address(_tokenA), address(_tokenB)); + assertNotEq(lPrice, 5e18); + assertEq(address(this).balance, 0); } - function testUpdatePrice_BeyondThreshold_ZeroRecipient() external { + function testUpdatePrice_RewardEligible_ZeroRecipient() external { // arrange uint256 lBalance = 10 ether; deal(address(_oracle), lBalance); @@ -545,7 +577,9 @@ contract ReservoirPriceOracleTest is BaseTest { // act _oracle.updatePrice(address(_tokenA), address(_tokenB), address(0)); - // assert - no change to balance + // assert - no change to balance, but price cache updated nonetheless + (uint256 lPrice,,) = _oracle.priceCache(address(_tokenA), address(_tokenB)); + assertNotEq(lPrice, 5e18); assertEq(address(_oracle).balance, lBalance); } @@ -595,10 +629,10 @@ contract ReservoirPriceOracleTest is BaseTest { _oracle.updatePrice(address(_tokenA), address(_tokenB), address(this)); // assert - (uint256 lPriceAC,) = _oracle.priceCache(lStart, lIntermediate1); - (uint256 lPriceCD,) = _oracle.priceCache(lIntermediate1, lIntermediate2); - (uint256 lPriceBD,) = _oracle.priceCache(lEnd, lIntermediate2); - (uint256 lPriceAB,) = _oracle.priceCache(lStart, lEnd); + (uint256 lPriceAC,,) = _oracle.priceCache(lStart, lIntermediate1); + (uint256 lPriceCD,,) = _oracle.priceCache(lIntermediate1, lIntermediate2); + (uint256 lPriceBD,,) = _oracle.priceCache(lEnd, lIntermediate2); + (uint256 lPriceAB,,) = _oracle.priceCache(lStart, lEnd); assertApproxEqRel(lPriceAC, 0.5e18, 0.0001e18); assertApproxEqRel(lPriceCD, 2e18, 0.0001e18); assertApproxEqRel(lPriceBD, 2e18, 0.0001e18); @@ -623,7 +657,7 @@ contract ReservoirPriceOracleTest is BaseTest { // assert address[] memory lQueriedRoute = _oracle.route(lToken0, lToken1); assertEq(lQueriedRoute, lRoute); - (, int256 lDecimalDiff) = _oracle.priceCache(lToken0, lToken1); + (, int256 lDecimalDiff,) = _oracle.priceCache(lToken0, lToken1); int256 lActualDiff = int256(uint256(IERC20(lToken1).decimals())) - int256(uint256(IERC20(lToken0).decimals())); assertEq(lDecimalDiff, lActualDiff); } @@ -720,7 +754,7 @@ contract ReservoirPriceOracleTest is BaseTest { // assert lQueriedRoute = _oracle.route(lToken0, lToken1); assertEq(lQueriedRoute, new address[](0)); - (uint256 lPrice,) = _oracle.priceCache(lToken0, lToken1); + (uint256 lPrice,,) = _oracle.priceCache(lToken0, lToken1); assertEq(lPrice, 0); } From 2063848f91218aca650a02e0bbd6908ca11c8c16 Mon Sep 17 00:00:00 2001 From: "A.L." Date: Mon, 29 Jul 2024 11:37:24 +0200 Subject: [PATCH 05/17] test: changes for large tests --- test/large/ReservoirPriceOracleLarge.t.sol | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/large/ReservoirPriceOracleLarge.t.sol b/test/large/ReservoirPriceOracleLarge.t.sol index c6a28b7..d5a9c94 100644 --- a/test/large/ReservoirPriceOracleLarge.t.sol +++ b/test/large/ReservoirPriceOracleLarge.t.sol @@ -7,7 +7,8 @@ import { FixedPointMathLib, MintableERC20, ReservoirPair, - IERC20 + IERC20, + Constants } from "test/unit/ReservoirPriceOracle.t.sol"; contract ReservoirPriceOracleLargeTest is ReservoirPriceOracleTest { @@ -77,7 +78,10 @@ contract ReservoirPriceOracleLargeTest is ReservoirPriceOracleTest { lRoute[3] = aTokenAAddress; } - _oracle.setRoute(lRoute[0], lRoute[3], lRoute); + uint16[] memory lBpDiffForMaxReward = new uint16[](3); + lBpDiffForMaxReward[0] = lBpDiffForMaxReward[1] = lBpDiffForMaxReward[2] = Constants.BP_SCALE; + + _oracle.setRoute(lRoute[0], lRoute[3], lRoute, lBpDiffForMaxReward); _writePriceCache( lTokenA < lTokenB ? aTokenAAddress : aTokenBAddress, lTokenA < lTokenB ? aTokenBAddress : aTokenAAddress, From 8deb530bf45e5b9aa184fc43cf9c0670c781872f Mon Sep 17 00:00:00 2001 From: "A.L." Date: Wed, 7 Aug 2024 23:02:42 +0200 Subject: [PATCH 06/17] refactor: rename variable --- src/ReservoirPriceOracle.sol | 55 ++++++++++++++++------------- src/libraries/RoutesLib.sol | 10 +++--- test/unit/libraries/RoutesLib.t.sol | 2 +- 3 files changed, 37 insertions(+), 30 deletions(-) diff --git a/src/ReservoirPriceOracle.sol b/src/ReservoirPriceOracle.sol index acb13da..58b438a 100644 --- a/src/ReservoirPriceOracle.sol +++ b/src/ReservoirPriceOracle.sol @@ -27,13 +27,20 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar // EVENTS // /////////////////////////////////////////////////////////////////////////////////////////////// - event BpDiffForMaxReward(address token0, address token1, uint16 bpForMaxReward); event DesignatePair(address token0, address token1, ReservoirPair pair); event FallbackOracleSet(address fallbackOracle); + event PriceUpdateRewardThreshold(address token0, address token1, uint16 bpForMaxReward); event RewardGasAmount(uint256 newAmount); event Route(address token0, address token1, address[] route); event TwapPeriod(uint256 newPeriod); + /////////////////////////////////////////////////////////////////////////////////////////////// + // CONSTANTS // + /////////////////////////////////////////////////////////////////////////////////////////////// + + /// @notice The maximum multiplier of the gas reward for a price update. + uint256 public constant MAX_REWARD_MULTIPLIER = 4; + /// @notice The type of price queried and stored, possibilities as defined by `PriceType`. PriceType public immutable PRICE_TYPE; @@ -108,13 +115,13 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar /// @param aToken1 Address of the higher token. /// @return rPrice The cached price of aToken0/aToken1 for simple routes. Returns 0 for prices of composite routes. /// @return rDecimalDiff The difference in decimals as defined by aToken1.decimals() - aToken0.decimals(). Only valid for simple routes. - /// @return rBpDiffForMaxReward The number of basis points at and beyond which the bounty payout for a price update is maximum. + /// @return rRewardThreshold The number of basis points at and beyond which the bounty payout for a price update is maximum. function priceCache(address aToken0, address aToken1) external view - returns (uint256 rPrice, int256 rDecimalDiff, uint256 rBpDiffForMaxReward) + returns (uint256 rPrice, int256 rDecimalDiff, uint256 rRewardThreshold) { - (rPrice, rDecimalDiff, rBpDiffForMaxReward) = _priceCache(aToken0, aToken1); + (rPrice, rDecimalDiff, rRewardThreshold) = _priceCache(aToken0, aToken1); } /// @notice Updates the TWAP price for all simple routes between `aTokenA` and `aTokenB`. Will also update intermediate routes if the route defined between @@ -128,7 +135,7 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar function updatePrice(address aTokenA, address aTokenB, address aRewardRecipient) external nonReentrant { (address lToken0, address lToken1) = Utils.sortTokens(aTokenA, aTokenB); - (address[] memory lRoute,, uint256 lPrevPrice, uint256 lBpDiffForMaxReward) = + (address[] memory lRoute,, uint256 lPrevPrice, uint256 lRewardThreshold) = _getRouteDecimalDifferencePrice(lToken0, lToken1); if (lRoute.length == 0) revert OracleErrors.NoPath(); @@ -151,7 +158,7 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar } _writePriceCache(lToken0, lToken1, lNewPrice); - _rewardUpdater(lPrevPrice, lNewPrice, aRewardRecipient, lBpDiffForMaxReward); + _rewardUpdater(lPrevPrice, lNewPrice, aRewardRecipient, lRewardThreshold); } } @@ -175,17 +182,17 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar rResult = lPair.getTimeWeightedAverage(aQuery.priceType, aQuery.secs, aQuery.ago, lIndex); } - function _rewardUpdater(uint256 aPrevPrice, uint256 aNewPrice, address aRecipient, uint256 aBpDiffForMaxReward) + function _rewardUpdater(uint256 aPrevPrice, uint256 aNewPrice, address aRecipient, uint256 aRewardThreshold) private { uint256 lPercentDiff = aPrevPrice.calcPercentageDiff(aNewPrice); if (lPercentDiff == 0) return; if (aRecipient == address(0)) return; - // SAFETY: this mul will not overflow as `aBpDiffForMaxReward` is capped by `Constants.BP_SCALE` + // SAFETY: this mul will not overflow as `aRewardThreshold` is capped by `Constants.BP_SCALE` uint256 lBpForMaxRewardWAD; unchecked { - lBpForMaxRewardWAD = aBpDiffForMaxReward * Constants.WAD / Constants.BP_SCALE; + lBpForMaxRewardWAD = aRewardThreshold * Constants.WAD / Constants.BP_SCALE; } uint256 lReward = lPercentDiff > lBpForMaxRewardWAD ? lBpForMaxRewardWAD : lPercentDiff; @@ -211,11 +218,11 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar /// @return rRoute The route to determine the price between aToken0 and aToken1 /// @return rDecimalDiff The result of token1.decimals() - token0.decimals() if it's a simple route. 0 otherwise /// @return rPrice The price of aToken0/aToken1 if it's a simple route (i.e. rRoute.length == 2). 0 otherwise - /// @return rBpDiffForMaxReward The price difference at and beyond which the maximum amount of gas bounty is applicable. + /// @return rRewardThreshold The price difference at and beyond which the maximum amount of gas bounty is applicable. function _getRouteDecimalDifferencePrice(address aToken0, address aToken1) private view - returns (address[] memory rRoute, int256 rDecimalDiff, uint256 rPrice, uint256 rBpDiffForMaxReward) + returns (address[] memory rRoute, int256 rDecimalDiff, uint256 rPrice, uint256 rRewardThreshold) { bytes32 lSlot = Utils.calculateSlot(aToken0, aToken1); @@ -231,7 +238,7 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar rRoute[1] = aToken1; rDecimalDiff = lFirstWord.getDecimalDifference(); rPrice = lFirstWord.getPrice(); - rBpDiffForMaxReward = lFirstWord.getBpDiffForMaxReward(); + rRewardThreshold = lFirstWord.getRewardThreshold(); } // composite route else if (lFirstWord.isCompositeRoute()) { @@ -286,7 +293,7 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar function _priceCache(address aToken0, address aToken1) private view - returns (uint256 rPrice, int256 rDecimalDiff, uint256 rBpDiffForMaxReward) + returns (uint256 rPrice, int256 rDecimalDiff, uint256 rRewardThreshold) { bytes32 lSlot = Utils.calculateSlot(aToken0, aToken1); @@ -297,7 +304,7 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar if (lData.isSimplePrice()) { rPrice = lData.getPrice(); rDecimalDiff = lData.getDecimalDifference(); - rBpDiffForMaxReward = lData.getBpDiffForMaxReward(); + rRewardThreshold = lData.getRewardThreshold(); } } @@ -311,7 +318,7 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar } if (!lData.isSimplePrice()) revert OracleErrors.WriteToNonSimpleRoute(); - lData = RoutesLib.packSimplePrice(lData.getDecimalDifference(), aNewPrice, lData.getBpDiffForMaxReward()); + lData = RoutesLib.packSimplePrice(lData.getDecimalDifference(), aNewPrice, lData.getRewardThreshold()); assembly ("memory-safe") { sstore(lSlot, lData) } @@ -451,8 +458,8 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar /// @param aToken0 Address of the lower token. /// @param aToken1 Address of the higher token. /// @param aRoute Path with which the price between aToken0 and aToken1 should be derived. - /// @param aBpDiffForMaxReward Array of basis points at and beyond which the bounty payout for a price update is maximum. - function setRoute(address aToken0, address aToken1, address[] memory aRoute, uint16[] memory aBpDiffForMaxReward) + /// @param aRewardThresholds Array of basis points at and beyond which the bounty payout for a price update is maximum. + function setRoute(address aToken0, address aToken1, address[] memory aRoute, uint16[] memory aRewardThresholds) public onlyOwner { @@ -461,7 +468,7 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar _validateTokens(aToken0, aToken1); if (lRouteLength > Constants.MAX_ROUTE_LENGTH || lRouteLength < 2) revert OracleErrors.InvalidRouteLength(); if (aRoute[0] != aToken0 || aRoute[lRouteLength - 1] != aToken1) revert OracleErrors.InvalidRoute(); - if (aBpDiffForMaxReward.length != lRouteLength - 1) revert OracleErrors.InvalidArrayLengthBpForMaxReward(); + if (aRewardThresholds.length != lRouteLength - 1) revert OracleErrors.InvalidArrayLengthBpForMaxReward(); bytes32 lSlot = Utils.calculateSlot(aToken0, aToken1); @@ -473,15 +480,15 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar int256 lDiff = int256(lToken1Decimals) - int256(lToken0Decimals); - if (aBpDiffForMaxReward[0] > Constants.BP_SCALE) revert OracleErrors.InvalidBpForMaxReward(); + if (aRewardThresholds[0] > Constants.BP_SCALE) revert OracleErrors.InvalidBpForMaxReward(); - bytes32 lData = RoutesLib.packSimplePrice(lDiff, 0, aBpDiffForMaxReward[0]); + bytes32 lData = RoutesLib.packSimplePrice(lDiff, 0, aRewardThresholds[0]); assembly ("memory-safe") { // Write data to storage. sstore(lSlot, lData) } - emit BpDiffForMaxReward(aToken0, aToken1, aBpDiffForMaxReward[0]); + emit PriceUpdateRewardThreshold(aToken0, aToken1, aRewardThresholds[0]); } // composite route else { @@ -501,10 +508,10 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar sstore(lSlot, lFirstWord) sstore(add(lSlot, 1), lSecondWord) } - _checkAndPopulateIntermediateRoute(lThirdToken, aToken1, aBpDiffForMaxReward[2]); + _checkAndPopulateIntermediateRoute(lThirdToken, aToken1, aRewardThresholds[2]); } - _checkAndPopulateIntermediateRoute(aToken0, lSecondToken, aBpDiffForMaxReward[0]); - _checkAndPopulateIntermediateRoute(lSecondToken, lThirdToken, aBpDiffForMaxReward[1]); + _checkAndPopulateIntermediateRoute(aToken0, lSecondToken, aRewardThresholds[0]); + _checkAndPopulateIntermediateRoute(lSecondToken, lThirdToken, aRewardThresholds[1]); } emit Route(aToken0, aToken1, aRoute); } diff --git a/src/libraries/RoutesLib.sol b/src/libraries/RoutesLib.sol index 1200971..3f1353c 100644 --- a/src/libraries/RoutesLib.sol +++ b/src/libraries/RoutesLib.sol @@ -29,15 +29,15 @@ library RoutesLib { // Assumes that aDecimalDifference is between -18 and 18 // Assumes that aPrice is between 1 and 1e36 - // Assumes that aBpDiffForMaxReward is <= Constants.BP_SCALE - function packSimplePrice(int256 aDecimalDifference, uint256 aPrice, uint16 aBpDiffForMaxReward) + // Assumes that aRewardThreshold is <= Constants.BP_SCALE + function packSimplePrice(int256 aDecimalDifference, uint256 aPrice, uint16 aRewardThreshold) internal pure returns (bytes32 rPacked) { bytes32 lDecimalDifferenceRaw = bytes1(uint8(int8(aDecimalDifference))); - bytes32 lBpDiffForMaxReward = bytes2(uint16(aBpDiffForMaxReward)); - rPacked = FLAG_SIMPLE_PRICE | lDecimalDifferenceRaw >> 8 | lBpDiffForMaxReward >> 16 | bytes32(aPrice); + bytes32 lRewardThreshold = bytes2(uint16(aRewardThreshold)); + rPacked = FLAG_SIMPLE_PRICE | lDecimalDifferenceRaw >> 8 | lRewardThreshold >> 16 | bytes32(aPrice); } function pack2HopRoute(address aSecondToken) internal pure returns (bytes32 rPacked) { @@ -65,7 +65,7 @@ library RoutesLib { rPrice = uint256(aData & 0x00000000ffffffffffffffffffffffffffffffffffffffffffffffffffffffff); } - function getBpDiffForMaxReward(bytes32 aData) internal pure returns (uint16 rBpDiffForMaxReward) { + function getRewardThreshold(bytes32 aData) internal pure returns (uint16 rBpDiffForMaxReward) { rBpDiffForMaxReward = uint16(uint256((aData & 0x0000ffff00000000000000000000000000000000000000000000000000000000) >> 224)); } diff --git a/test/unit/libraries/RoutesLib.t.sol b/test/unit/libraries/RoutesLib.t.sol index 3586e23..df0540c 100644 --- a/test/unit/libraries/RoutesLib.t.sol +++ b/test/unit/libraries/RoutesLib.t.sol @@ -46,7 +46,7 @@ contract RoutesLibTest is Test { // assert assertEq(lResult[0], RoutesLib.FLAG_SIMPLE_PRICE); assertEq(lResult[1], bytes1(uint8(aDiff))); - assertEq(lResult.getBpDiffForMaxReward(), Constants.BP_SCALE); + assertEq(lResult.getRewardThreshold(), Constants.BP_SCALE); assertEq(lResult.getPrice(), lPrice); } } From d0a04d8b334297365112b30b1797cbbaed362cd0 Mon Sep 17 00:00:00 2001 From: "A.L." Date: Wed, 7 Aug 2024 23:27:02 +0200 Subject: [PATCH 07/17] feat: impl new reward logic --- src/ReservoirPriceOracle.sol | 41 +++++++++++++++++++++------------- src/libraries/OracleErrors.sol | 4 ++-- 2 files changed, 27 insertions(+), 18 deletions(-) diff --git a/src/ReservoirPriceOracle.sol b/src/ReservoirPriceOracle.sol index 58b438a..ea58ff2 100644 --- a/src/ReservoirPriceOracle.sol +++ b/src/ReservoirPriceOracle.sol @@ -29,7 +29,7 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar event DesignatePair(address token0, address token1, ReservoirPair pair); event FallbackOracleSet(address fallbackOracle); - event PriceUpdateRewardThreshold(address token0, address token1, uint16 bpForMaxReward); + event PriceUpdateRewardThreshold(address token0, address token1, uint16 threshold); event RewardGasAmount(uint256 newAmount); event Route(address token0, address token1, address[] route); event TwapPeriod(uint256 newPeriod); @@ -185,28 +185,37 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar function _rewardUpdater(uint256 aPrevPrice, uint256 aNewPrice, address aRecipient, uint256 aRewardThreshold) private { - uint256 lPercentDiff = aPrevPrice.calcPercentageDiff(aNewPrice); - if (lPercentDiff == 0) return; if (aRecipient == address(0)) return; // SAFETY: this mul will not overflow as `aRewardThreshold` is capped by `Constants.BP_SCALE` - uint256 lBpForMaxRewardWAD; + uint256 lRewardThresholdWAD; unchecked { - lBpForMaxRewardWAD = aRewardThreshold * Constants.WAD / Constants.BP_SCALE; + lRewardThresholdWAD = aRewardThreshold * Constants.WAD / Constants.BP_SCALE; } - uint256 lReward = lPercentDiff > lBpForMaxRewardWAD ? lBpForMaxRewardWAD : lPercentDiff; - // N.B. Revisit this whenever deployment on a new chain is needed - // - // we use `block.basefee` instead of `ArbGasInfo::getMinimumGasPrice()` - // on ARB because the latter will always return the demand insensitive - // base fee, while the former can return higher fees during times of - // congestion + uint256 lPercentDiff = aPrevPrice.calcPercentageDiff(aNewPrice); + uint256 lPayoutAmt; // SAFETY: this mul will not overflow even in extreme cases of `block.basefee`. - uint256 lPayoutAmt; unchecked { - lPayoutAmt = block.basefee * rewardGasAmount * lReward / lBpForMaxRewardWAD; + if (lPercentDiff < lRewardThresholdWAD) { + return; + } + // payout max reward + else if (lPercentDiff >= lRewardThresholdWAD * MAX_REWARD_MULTIPLIER) { + // N.B. Revisit this whenever deployment on a new chain is needed + // + // we use `block.basefee` instead of `ArbGasInfo::getMinimumGasPrice()` + // on ARB because the latter will always return the demand insensitive + // base fee, while the former can return higher fees during times of + // congestion + lPayoutAmt = block.basefee * rewardGasAmount * MAX_REWARD_MULTIPLIER; + } else { + assert( + lPercentDiff >= lRewardThresholdWAD && lPercentDiff < lRewardThresholdWAD * MAX_REWARD_MULTIPLIER + ); + lPayoutAmt = block.basefee * rewardGasAmount * lPercentDiff / lRewardThresholdWAD; + } } // does not revert under any circumstance @@ -468,7 +477,7 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar _validateTokens(aToken0, aToken1); if (lRouteLength > Constants.MAX_ROUTE_LENGTH || lRouteLength < 2) revert OracleErrors.InvalidRouteLength(); if (aRoute[0] != aToken0 || aRoute[lRouteLength - 1] != aToken1) revert OracleErrors.InvalidRoute(); - if (aRewardThresholds.length != lRouteLength - 1) revert OracleErrors.InvalidArrayLengthBpForMaxReward(); + if (aRewardThresholds.length != lRouteLength - 1) revert OracleErrors.InvalidArrayLengthRewardThresholds(); bytes32 lSlot = Utils.calculateSlot(aToken0, aToken1); @@ -480,7 +489,7 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar int256 lDiff = int256(lToken1Decimals) - int256(lToken0Decimals); - if (aRewardThresholds[0] > Constants.BP_SCALE) revert OracleErrors.InvalidBpForMaxReward(); + if (aRewardThresholds[0] > Constants.BP_SCALE) revert OracleErrors.InvalidRewardThreshold(); bytes32 lData = RoutesLib.packSimplePrice(lDiff, 0, aRewardThresholds[0]); assembly ("memory-safe") { diff --git a/src/libraries/OracleErrors.sol b/src/libraries/OracleErrors.sol index fcc8713..2a14a8d 100644 --- a/src/libraries/OracleErrors.sol +++ b/src/libraries/OracleErrors.sol @@ -5,8 +5,8 @@ pragma solidity ^0.8.0; library OracleErrors { // config errors error IncorrectTokensDesignatePair(); - error InvalidBpForMaxReward(); - error InvalidArrayLengthBpForMaxReward(); + error InvalidRewardThreshold(); + error InvalidArrayLengthRewardThresholds(); error InvalidRoute(); error InvalidRouteLength(); error InvalidTokensProvided(); From d2f700a26183ca61279caef8d0b6fd6665c7228e Mon Sep 17 00:00:00 2001 From: "A.L." Date: Thu, 8 Aug 2024 22:22:53 +0200 Subject: [PATCH 08/17] test: fuzzing all cases below threshold --- src/libraries/RoutesLib.sol | 4 +- test/__fixtures/BaseTest.t.sol | 5 +- test/unit/ReservoirPriceOracle.t.sol | 152 ++++++++++++++------------- 3 files changed, 85 insertions(+), 76 deletions(-) diff --git a/src/libraries/RoutesLib.sol b/src/libraries/RoutesLib.sol index 3f1353c..fd6b614 100644 --- a/src/libraries/RoutesLib.sol +++ b/src/libraries/RoutesLib.sol @@ -65,8 +65,8 @@ library RoutesLib { rPrice = uint256(aData & 0x00000000ffffffffffffffffffffffffffffffffffffffffffffffffffffffff); } - function getRewardThreshold(bytes32 aData) internal pure returns (uint16 rBpDiffForMaxReward) { - rBpDiffForMaxReward = + function getRewardThreshold(bytes32 aData) internal pure returns (uint16 rRewardThreshold) { + rRewardThreshold = uint16(uint256((aData & 0x0000ffff00000000000000000000000000000000000000000000000000000000) >> 224)); } diff --git a/test/__fixtures/BaseTest.t.sol b/test/__fixtures/BaseTest.t.sol index 52f2db0..735eff5 100644 --- a/test/__fixtures/BaseTest.t.sol +++ b/test/__fixtures/BaseTest.t.sol @@ -16,10 +16,13 @@ import { ReservoirPriceOracle, PriceType, IPriceOracle } from "src/ReservoirPric contract BaseTest is Test { using FactoryStoreLib for GenericFactory; + uint64 internal constant DEFAULT_REWARD_GAS_AMOUNT = 200_000; + GenericFactory internal _factory = new GenericFactory(); ReservoirPair internal _pair; - ReservoirPriceOracle internal _oracle = new ReservoirPriceOracle(0.15 minutes, 500_000, PriceType.CLAMPED_PRICE); + ReservoirPriceOracle internal _oracle = + new ReservoirPriceOracle(0.15 minutes, DEFAULT_REWARD_GAS_AMOUNT, PriceType.CLAMPED_PRICE); MintableERC20 internal _tokenA = MintableERC20(address(0x100)); MintableERC20 internal _tokenB = MintableERC20(address(0x200)); diff --git a/test/unit/ReservoirPriceOracle.t.sol b/test/unit/ReservoirPriceOracle.t.sol index c8017e2..089b828 100644 --- a/test/unit/ReservoirPriceOracle.t.sol +++ b/test/unit/ReservoirPriceOracle.t.sol @@ -47,15 +47,17 @@ contract ReservoirPriceOracleTest is BaseTest { require(aPrice <= Constants.MAX_SUPPORTED_PRICE, "price too large"); vm.record(); - _oracle.priceCache(aToken0, aToken1); + (, int256 lDecimalDiff, uint256 lRewardThreshold) = _oracle.priceCache(aToken0, aToken1); (bytes32[] memory lAccesses,) = vm.accesses(address(_oracle)); require(lAccesses.length == 1, "incorrect number of accesses"); - int256 lDecimalDiff = int256(uint256(IERC20(aToken1).decimals())) - int256(uint256(IERC20(aToken0).decimals())); - uint16 lBpDiffMaxReward = Constants.BP_SCALE; - bytes32 lData = lDecimalDiff.packSimplePrice(aPrice, lBpDiffMaxReward); - require(lData.getDecimalDifference() == lDecimalDiff, "decimal diff incorrect"); + lDecimalDiff = lDecimalDiff == 0 + ? int256(uint256(IERC20(aToken1).decimals())) - int256(uint256(IERC20(aToken0).decimals())) + : lDecimalDiff; + bytes32 lData = lDecimalDiff.packSimplePrice(aPrice, uint16(lRewardThreshold)); require(lData.isSimplePrice(), "flag incorrect"); + require(lData.getDecimalDifference() == lDecimalDiff, "decimal diff incorrect"); + require(lData.getRewardThreshold() == lRewardThreshold, "reward threshold incorrect"); vm.store(address(_oracle), lAccesses[0], lData); } @@ -65,6 +67,7 @@ contract ReservoirPriceOracleTest is BaseTest { // make sure ether balance of test contract is 0 deal(address(this), 0); + deal(address(_oracle), 10 ether); _addressSet.add(address(_tokenA)); _addressSet.add(address(_tokenB)); @@ -82,12 +85,13 @@ contract ReservoirPriceOracleTest is BaseTest { function setUp() external { // define route address[] memory lRoute = new address[](2); - uint16[] memory lBpDiffForMaxReward = new uint16[](1); + uint16[] memory lRewardThreshold = new uint16[](1); lRoute[0] = address(_tokenA); lRoute[1] = address(_tokenB); + lRewardThreshold[0] = 200; // 2% _oracle.designatePair(address(_tokenB), address(_tokenA), _pair); - _oracle.setRoute(address(_tokenA), address(_tokenB), lRoute, lBpDiffForMaxReward); + _oracle.setRoute(address(_tokenA), address(_tokenB), lRoute, lRewardThreshold); } function testWritePriceCache(uint256 aPrice) external { @@ -146,14 +150,14 @@ contract ReservoirPriceOracleTest is BaseTest { _writePriceCache(address(_tokenC), address(_tokenD), lPriceCD); address[] memory lRoute = new address[](4); - uint16[] memory lBpDiffForMaxReward = new uint16[](3); - lBpDiffForMaxReward[0] = lBpDiffForMaxReward[1] = lBpDiffForMaxReward[2] = Constants.BP_SCALE; + uint16[] memory lRewardThreshold = new uint16[](3); + lRewardThreshold[0] = lRewardThreshold[1] = lRewardThreshold[2] = Constants.BP_SCALE; lRoute[0] = address(_tokenA); lRoute[1] = address(_tokenB); lRoute[2] = address(_tokenC); lRoute[3] = address(_tokenD); - _oracle.setRoute(address(_tokenA), address(_tokenD), lRoute, lBpDiffForMaxReward); + _oracle.setRoute(address(_tokenA), address(_tokenD), lRoute, lRewardThreshold); uint256 lAmountIn = 789e6; @@ -176,13 +180,13 @@ contract ReservoirPriceOracleTest is BaseTest { _writePriceCache(address(_tokenC), address(_tokenD), lPriceCD); address[] memory lRoute = new address[](4); - uint16[] memory lBpDiffForMaxReward = new uint16[](3); - lBpDiffForMaxReward[0] = lBpDiffForMaxReward[1] = lBpDiffForMaxReward[2] = Constants.BP_SCALE; + uint16[] memory lRewardThreshold = new uint16[](3); + lRewardThreshold[0] = lRewardThreshold[1] = lRewardThreshold[2] = Constants.BP_SCALE; lRoute[0] = address(_tokenA); lRoute[1] = address(_tokenB); lRoute[2] = address(_tokenC); lRoute[3] = address(_tokenD); - _oracle.setRoute(address(_tokenA), address(_tokenD), lRoute, lBpDiffForMaxReward); + _oracle.setRoute(address(_tokenA), address(_tokenD), lRoute, lRewardThreshold); uint256 lAmountIn = 789e6; @@ -219,9 +223,9 @@ contract ReservoirPriceOracleTest is BaseTest { lRoute[2] = address(lTokenC); { - uint16[] memory lBpDiffForMaxReward = new uint16[](2); - lBpDiffForMaxReward[0] = lBpDiffForMaxReward[1] = Constants.BP_SCALE; - _oracle.setRoute(address(lTokenA), address(lTokenC), lRoute, lBpDiffForMaxReward); + uint16[] memory lRewardThreshold = new uint16[](2); + lRewardThreshold[0] = lRewardThreshold[1] = Constants.BP_SCALE; + _oracle.setRoute(address(lTokenA), address(lTokenC), lRoute, lRewardThreshold); _writePriceCache(address(lTokenA), address(lTokenB), 1e18); _writePriceCache(address(lTokenC), address(lTokenB), 1e18); } @@ -263,10 +267,10 @@ contract ReservoirPriceOracleTest is BaseTest { _oracle.designatePair(address(lTokenA), address(lTokenB), lPair); address[] memory lRoute = new address[](2); - uint16[] memory lBpDiffForMaxReward = new uint16[](1); + uint16[] memory lRewardThreshold = new uint16[](1); (lRoute[0], lRoute[1]) = lTokenA < lTokenB ? (address(lTokenA), address(lTokenB)) : (address(lTokenB), address(lTokenA)); - _oracle.setRoute(lRoute[0], lRoute[1], lRoute, lBpDiffForMaxReward); + _oracle.setRoute(lRoute[0], lRoute[1], lRoute, lRewardThreshold); _writePriceCache(lRoute[0], lRoute[1], lPrice); // price written could be tokenB/tokenA or tokenA/tokenB depending on the fuzz addresses // act @@ -324,8 +328,8 @@ contract ReservoirPriceOracleTest is BaseTest { lTokenA < lTokenC ? (address(lTokenA), address(lTokenC)) : (address(lTokenC), address(lTokenA)); lRoute[1] = address(lTokenB); - uint16[] memory lBpDiffMaxReward = new uint16[](2); - _oracle.setRoute(lRoute[0], lRoute[2], lRoute, lBpDiffMaxReward); + uint16[] memory lRewardThreshold = new uint16[](2); + _oracle.setRoute(lRoute[0], lRoute[2], lRoute, lRewardThreshold); _writePriceCache( address(lTokenA) < address(lTokenB) ? address(lTokenA) : address(lTokenB), address(lTokenA) < address(lTokenB) ? address(lTokenB) : address(lTokenA), @@ -456,8 +460,6 @@ contract ReservoirPriceOracleTest is BaseTest { assertEq(lPrice, 0); // arrange - deal(address(_oracle), 1 ether); - skip(1); _pair.sync(); skip(_oracle.twapPeriod() * 2); @@ -474,29 +476,35 @@ contract ReservoirPriceOracleTest is BaseTest { assertEq(address(this).balance, 0); // there should be no reward for the first price update } - function testUpdatePrice_NoPriceChange() external { - // arrange - testUpdatePrice_FirstUpdate(); - uint256 lExpectedPrice = 98_918_868_099_219_913_512; - (uint256 lPrice,,) = _oracle.priceCache(address(_tokenA), address(_tokenB)); - assertEq(lPrice, lExpectedPrice); // ensure that there is a price to begin with - skip(_oracle.twapPeriod() * 2); + function testUpdatePrice_BelowThreshold(uint256 aPercentDiff) external { + // assume + (,, uint256 lRewardThreshold) = _oracle.priceCache(address(_tokenA), address(_tokenB)); + uint256 lPercentDiff = bound(aPercentDiff, 0, (lRewardThreshold - 1) * WAD / Constants.BP_SCALE); + + // arrange - we fuzz test by varying the starting price instead of the new price + uint256 lCurrentPrice = 98_918_868_099_219_913_512; + uint256 lStartingPrice = lCurrentPrice * WAD / (WAD + lPercentDiff); + _writePriceCache(address(_tokenA), address(_tokenB), lStartingPrice); + + console2.log(lStartingPrice); + skip(1); + _pair.sync(); + skip(_oracle.twapPeriod() + 1); _pair.sync(); // act _oracle.updatePrice(address(_tokenA), address(_tokenB), address(this)); // assert - (lPrice,,) = _oracle.priceCache(address(_tokenA), address(_tokenB)); - assertEq(lPrice, lExpectedPrice); - assertEq(address(this).balance, 0); // no reward as the price did not change at all + (uint256 lPrice,,) = _oracle.priceCache(address(_tokenA), address(_tokenB)); + assertEq(lPrice, lCurrentPrice); + assertEq(address(this).balance, 0); // no reward as the price did not move sufficiently } - function testUpdatePrice_BelowMaxReward() external { + function testUpdatePrice_AboveThresholdBelowMax() external { // arrange uint256 lOriginalPrice = 98.9223e18; _writePriceCache(address(_tokenA), address(_tokenB), lOriginalPrice); - deal(address(_oracle), 1 ether); skip(1); _pair.sync(); @@ -518,7 +526,6 @@ contract ReservoirPriceOracleTest is BaseTest { function testUpdatePrice_BeyondMaxReward() external { // arrange _writePriceCache(address(_tokenA), address(_tokenB), 5e18); - deal(address(_oracle), 1 ether); skip(1); _pair.sync(); @@ -564,8 +571,7 @@ contract ReservoirPriceOracleTest is BaseTest { function testUpdatePrice_RewardEligible_ZeroRecipient() external { // arrange - uint256 lBalance = 10 ether; - deal(address(_oracle), lBalance); + uint256 lOracleBalanceStart = address(_oracle).balance; _writePriceCache(address(_tokenA), address(_tokenB), 5e18); skip(1); @@ -580,7 +586,7 @@ contract ReservoirPriceOracleTest is BaseTest { // assert - no change to balance, but price cache updated nonetheless (uint256 lPrice,,) = _oracle.priceCache(address(_tokenA), address(_tokenB)); assertNotEq(lPrice, 5e18); - assertEq(address(_oracle).balance, lBalance); + assertEq(address(_oracle).balance, lOracleBalanceStart); } function testUpdatePrice_IntermediateRoutes() external { @@ -590,13 +596,13 @@ contract ReservoirPriceOracleTest is BaseTest { address lIntermediate2 = address(_tokenD); address lEnd = address(_tokenB); address[] memory lRoute = new address[](4); - uint16[] memory lBpDiffForMaxReward = new uint16[](3); - lBpDiffForMaxReward[0] = lBpDiffForMaxReward[1] = lBpDiffForMaxReward[2] = Constants.BP_SCALE; + uint16[] memory lRewardThreshold = new uint16[](3); + lRewardThreshold[0] = lRewardThreshold[1] = lRewardThreshold[2] = Constants.BP_SCALE; lRoute[0] = lStart; lRoute[1] = lIntermediate1; lRoute[2] = lIntermediate2; lRoute[3] = lEnd; - _oracle.setRoute(lStart, lEnd, lRoute, lBpDiffForMaxReward); + _oracle.setRoute(lStart, lEnd, lRoute, lRewardThreshold); ReservoirPair lAC = ReservoirPair(_createPair(address(_tokenA), address(_tokenC), 0)); ReservoirPair lCD = ReservoirPair(_createPair(address(_tokenC), address(_tokenD), 0)); @@ -644,15 +650,15 @@ contract ReservoirPriceOracleTest is BaseTest { address lToken0 = address(_tokenB); address lToken1 = address(_tokenC); address[] memory lRoute = new address[](2); - uint16[] memory lBpDiffForMaxReward = new uint16[](1); - lBpDiffForMaxReward[0] = Constants.BP_SCALE; + uint16[] memory lRewardThreshold = new uint16[](1); + lRewardThreshold[0] = Constants.BP_SCALE; lRoute[0] = lToken0; lRoute[1] = lToken1; // act vm.expectEmit(false, false, false, false); emit Route(lToken0, lToken1, lRoute); - _oracle.setRoute(lToken0, lToken1, lRoute, lBpDiffForMaxReward); + _oracle.setRoute(lToken0, lToken1, lRoute, lRewardThreshold); // assert address[] memory lQueriedRoute = _oracle.route(lToken0, lToken1); @@ -668,15 +674,15 @@ contract ReservoirPriceOracleTest is BaseTest { address lToken0 = address(_tokenB); address lToken1 = address(_tokenC); address[] memory lRoute = new address[](4); - uint16[] memory lBpDiffForMaxReward = new uint16[](3); - lBpDiffForMaxReward[0] = lBpDiffForMaxReward[1] = lBpDiffForMaxReward[2] = Constants.BP_SCALE; + uint16[] memory lRewardThreshold = new uint16[](3); + lRewardThreshold[0] = lRewardThreshold[1] = lRewardThreshold[2] = Constants.BP_SCALE; lRoute[0] = lToken0; lRoute[1] = address(_tokenA); lRoute[2] = address(_tokenD); lRoute[3] = lToken1; // act - _oracle.setRoute(lToken0, lToken1, lRoute, lBpDiffForMaxReward); + _oracle.setRoute(lToken0, lToken1, lRoute, lRewardThreshold); // assert address[] memory lQueriedRoute = _oracle.route(lToken0, lToken1); @@ -709,8 +715,8 @@ contract ReservoirPriceOracleTest is BaseTest { lIntermediateRoute3[0] = lIntermediate2; lIntermediateRoute3[1] = lEnd; - uint16[] memory lBpDiffForMaxReward = new uint16[](3); - lBpDiffForMaxReward[0] = lBpDiffForMaxReward[1] = lBpDiffForMaxReward[2] = Constants.BP_SCALE; + uint16[] memory lRewardThreshold = new uint16[](3); + lRewardThreshold[0] = lRewardThreshold[1] = lRewardThreshold[2] = Constants.BP_SCALE; // act vm.expectEmit(false, false, false, true); @@ -723,7 +729,7 @@ contract ReservoirPriceOracleTest is BaseTest { vm.expectEmit(false, false, false, true); emit Route(lStart, lEnd, lRoute); - _oracle.setRoute(lStart, lEnd, lRoute, lBpDiffForMaxReward); + _oracle.setRoute(lStart, lEnd, lRoute, lRewardThreshold); // assert assertEq(_oracle.route(lStart, lEnd), lRoute); @@ -737,11 +743,11 @@ contract ReservoirPriceOracleTest is BaseTest { address lToken0 = address(_tokenB); address lToken1 = address(_tokenC); address[] memory lRoute = new address[](2); - uint16[] memory lBpDiffForMaxReward = new uint16[](1); - lBpDiffForMaxReward[0] = Constants.BP_SCALE; + uint16[] memory lRewardThreshold = new uint16[](1); + lRewardThreshold[0] = Constants.BP_SCALE; lRoute[0] = lToken0; lRoute[1] = lToken1; - _oracle.setRoute(lToken0, lToken1, lRoute, lBpDiffForMaxReward); + _oracle.setRoute(lToken0, lToken1, lRoute, lRewardThreshold); address[] memory lQueriedRoute = _oracle.route(lToken0, lToken1); assertEq(lQueriedRoute, lRoute); _writePriceCache(lToken0, lToken1, 1e18); @@ -761,13 +767,13 @@ contract ReservoirPriceOracleTest is BaseTest { function testClearRoute_AllWordsCleared() external { // arrange address[] memory lRoute = new address[](4); - uint16[] memory lBpDiffForMaxReward = new uint16[](3); - lBpDiffForMaxReward[0] = lBpDiffForMaxReward[1] = lBpDiffForMaxReward[2] = Constants.BP_SCALE; + uint16[] memory lRewardThreshold = new uint16[](3); + lRewardThreshold[0] = lRewardThreshold[1] = lRewardThreshold[2] = Constants.BP_SCALE; lRoute[0] = address(_tokenA); lRoute[1] = address(_tokenC); lRoute[2] = address(_tokenB); lRoute[3] = address(_tokenD); - _oracle.setRoute(address(_tokenA), address(_tokenD), lRoute, lBpDiffForMaxReward); + _oracle.setRoute(address(_tokenA), address(_tokenD), lRoute, lRewardThreshold); address[] memory lQueriedRoute = _oracle.route(address(_tokenA), address(_tokenD)); assertEq(lQueriedRoute, lRoute); bytes32 lSlot1 = address(_tokenA).calculateSlot(address(_tokenD)); @@ -888,13 +894,13 @@ contract ReservoirPriceOracleTest is BaseTest { lPair.sync(); address[] memory lRoute = new address[](2); - uint16[] memory lBpDiffForMaxReward = new uint16[](1); - lBpDiffForMaxReward[0] = Constants.BP_SCALE; + uint16[] memory lRewardThreshold = new uint16[](1); + lRewardThreshold[0] = Constants.BP_SCALE; lRoute[0] = address(_tokenB); lRoute[1] = address(_tokenC); _oracle.designatePair(address(_tokenB), address(_tokenC), lPair); - _oracle.setRoute(address(_tokenB), address(_tokenC), lRoute, lBpDiffForMaxReward); + _oracle.setRoute(address(_tokenB), address(_tokenC), lRoute, lRewardThreshold); // act & assert vm.expectRevert( @@ -911,14 +917,14 @@ contract ReservoirPriceOracleTest is BaseTest { address lToken0 = address(0x1); address lToken1 = address(0x1); address[] memory lRoute = new address[](2); - uint16[] memory lBpDiffForMaxReward = new uint16[](1); - lBpDiffForMaxReward[0] = Constants.BP_SCALE; + uint16[] memory lRewardThreshold = new uint16[](1); + lRewardThreshold[0] = Constants.BP_SCALE; lRoute[0] = lToken0; lRoute[1] = lToken1; // act & assert vm.expectRevert(OracleErrors.InvalidTokensProvided.selector); - _oracle.setRoute(lToken0, lToken1, lRoute, lBpDiffForMaxReward); + _oracle.setRoute(lToken0, lToken1, lRoute, lRewardThreshold); } function testSetRoute_NotSorted() external { @@ -926,14 +932,14 @@ contract ReservoirPriceOracleTest is BaseTest { address lToken0 = address(0x21); address lToken1 = address(0x2); address[] memory lRoute = new address[](2); - uint16[] memory lBpDiffForMaxReward = new uint16[](1); - lBpDiffForMaxReward[0] = Constants.BP_SCALE; + uint16[] memory lRewardThreshold = new uint16[](1); + lRewardThreshold[0] = Constants.BP_SCALE; lRoute[0] = lToken0; lRoute[1] = lToken1; // act & assert vm.expectRevert(OracleErrors.InvalidTokensProvided.selector); - _oracle.setRoute(lToken0, lToken1, lRoute, lBpDiffForMaxReward); + _oracle.setRoute(lToken0, lToken1, lRoute, lRewardThreshold); } function testSetRoute_InvalidRouteLength() external { @@ -941,8 +947,8 @@ contract ReservoirPriceOracleTest is BaseTest { address lToken0 = address(0x1); address lToken1 = address(0x2); address[] memory lTooLong = new address[](5); - uint16[] memory lBpDiffForMaxReward = new uint16[](1); - lBpDiffForMaxReward[0] = Constants.BP_SCALE; + uint16[] memory lRewardThreshold = new uint16[](1); + lRewardThreshold[0] = Constants.BP_SCALE; lTooLong[0] = lToken0; lTooLong[1] = address(0); lTooLong[2] = address(0); @@ -953,11 +959,11 @@ contract ReservoirPriceOracleTest is BaseTest { // act & assert vm.expectRevert(OracleErrors.InvalidRouteLength.selector); - _oracle.setRoute(lToken0, lToken1, lTooLong, lBpDiffForMaxReward); + _oracle.setRoute(lToken0, lToken1, lTooLong, lRewardThreshold); // act & assert vm.expectRevert(OracleErrors.InvalidRouteLength.selector); - _oracle.setRoute(lToken0, lToken1, lTooShort, lBpDiffForMaxReward); + _oracle.setRoute(lToken0, lToken1, lTooShort, lRewardThreshold); } function testSetRoute_InvalidRoute() external { @@ -974,14 +980,14 @@ contract ReservoirPriceOracleTest is BaseTest { lInvalidRoute2[1] = address(54); lInvalidRoute2[2] = lToken1; - uint16[] memory lBpDiffForMaxReward = new uint16[](2); - lBpDiffForMaxReward[0] = lBpDiffForMaxReward[1] = Constants.BP_SCALE; + uint16[] memory lRewardThreshold = new uint16[](2); + lRewardThreshold[0] = lRewardThreshold[1] = Constants.BP_SCALE; // act & assert vm.expectRevert(OracleErrors.InvalidRoute.selector); - _oracle.setRoute(lToken0, lToken1, lInvalidRoute1, lBpDiffForMaxReward); + _oracle.setRoute(lToken0, lToken1, lInvalidRoute1, lRewardThreshold); vm.expectRevert(OracleErrors.InvalidRoute.selector); - _oracle.setRoute(lToken0, lToken1, lInvalidRoute2, lBpDiffForMaxReward); + _oracle.setRoute(lToken0, lToken1, lInvalidRoute2, lRewardThreshold); } function testUpdateRewardGasAmount_NotOwner() external { From 052580a6101020e4b1ce8edda1cee476fd8d1de3 Mon Sep 17 00:00:00 2001 From: "A.L." Date: Thu, 8 Aug 2024 23:43:27 +0200 Subject: [PATCH 09/17] test: fuzz all cases with different scenarios of price changes --- test/unit/ReservoirPriceOracle.t.sol | 85 ++++++++++++++++++++-------- 1 file changed, 60 insertions(+), 25 deletions(-) diff --git a/test/unit/ReservoirPriceOracle.t.sol b/test/unit/ReservoirPriceOracle.t.sol index 089b828..b50a924 100644 --- a/test/unit/ReservoirPriceOracle.t.sol +++ b/test/unit/ReservoirPriceOracle.t.sol @@ -33,7 +33,7 @@ contract ReservoirPriceOracleTest is BaseTest { event Route(address token0, address token1, address[] route); uint256 internal constant WAD = 1e18; - + uint256 internal constant ORACLE_STARTING_BALANCE = 10 ether; address internal constant ADDRESS_THRESHOLD = address(0x1000); // to keep track of addresses to ensure no clash for fuzz tests @@ -67,7 +67,7 @@ contract ReservoirPriceOracleTest is BaseTest { // make sure ether balance of test contract is 0 deal(address(this), 0); - deal(address(_oracle), 10 ether); + deal(address(_oracle), ORACLE_STARTING_BALANCE); _addressSet.add(address(_tokenA)); _addressSet.add(address(_tokenB)); @@ -489,7 +489,7 @@ contract ReservoirPriceOracleTest is BaseTest { console2.log(lStartingPrice); skip(1); _pair.sync(); - skip(_oracle.twapPeriod() + 1); + skip(_oracle.twapPeriod()); _pair.sync(); // act @@ -501,54 +501,72 @@ contract ReservoirPriceOracleTest is BaseTest { assertEq(address(this).balance, 0); // no reward as the price did not move sufficiently } - function testUpdatePrice_AboveThresholdBelowMax() external { + function testUpdatePrice_AboveThresholdBelowMaxReward(uint256 aPercentDiff) external { + // assume + (,, uint256 lRewardThreshold) = _oracle.priceCache(address(_tokenA), address(_tokenB)); + uint256 lRewardThresholdWAD = lRewardThreshold * WAD / Constants.BP_SCALE; + uint256 lPercentDiff = bound( + aPercentDiff, + lRewardThresholdWAD, + _oracle.MAX_REWARD_MULTIPLIER() * lRewardThreshold * WAD / Constants.BP_SCALE + ); + // arrange - uint256 lOriginalPrice = 98.9223e18; - _writePriceCache(address(_tokenA), address(_tokenB), lOriginalPrice); + uint256 lCurrentPrice = 98_918_868_099_219_913_512; + uint256 lStartingPrice = lCurrentPrice * WAD / (WAD + lPercentDiff); + _writePriceCache(address(_tokenA), address(_tokenB), lStartingPrice); skip(1); _pair.sync(); - skip(_oracle.twapPeriod() * 2); - _tokenA.mint(address(_pair), 2e18); - _pair.swap(2e18, true, address(this), ""); + skip(_oracle.twapPeriod()); + _pair.sync(); // act _oracle.updatePrice(address(_tokenB), address(_tokenA), address(this)); // assert (uint256 lPrice,,) = _oracle.priceCache(address(_tokenA), address(_tokenB)); - assertEq(lPrice, 98_918_868_099_219_913_512); + assertEq(lPrice, lCurrentPrice); uint256 lExpectedRewardReceived = - block.basefee * _oracle.rewardGasAmount() * lOriginalPrice.calcPercentageDiff(lPrice) / 1e18; + block.basefee * _oracle.rewardGasAmount() * lPercentDiff / lRewardThresholdWAD; + assertGe(lExpectedRewardReceived, block.basefee * _oracle.rewardGasAmount()); + assertLe(lExpectedRewardReceived, block.basefee * _oracle.rewardGasAmount() * _oracle.MAX_REWARD_MULTIPLIER()); assertEq(address(this).balance, lExpectedRewardReceived); // some reward received but is less than max possible reward } - function testUpdatePrice_BeyondMaxReward() external { + function testUpdatePrice_BeyondMaxReward(uint256 aPercentDiff) external { + // assume + (,, uint256 lRewardThreshold) = _oracle.priceCache(address(_tokenA), address(_tokenB)); + uint256 lPercentDiff = bound( + aPercentDiff, + _oracle.MAX_REWARD_MULTIPLIER() * lRewardThreshold * WAD / Constants.BP_SCALE, + WAD // 100% + ); + // arrange - _writePriceCache(address(_tokenA), address(_tokenB), 5e18); + uint256 lCurrentPrice = 98_918_868_099_219_913_512; + uint256 lStartingPrice = lCurrentPrice * WAD / (WAD + lPercentDiff); + _writePriceCache(address(_tokenA), address(_tokenB), lStartingPrice); skip(1); _pair.sync(); - skip(_oracle.twapPeriod() * 2); - _tokenA.mint(address(_pair), 2e18); - _pair.swap(2e18, true, address(this), ""); + skip(_oracle.twapPeriod()); + _pair.sync(); // act _oracle.updatePrice(address(_tokenB), address(_tokenA), address(this)); // assert (uint256 lPrice,,) = _oracle.priceCache(address(_tokenA), address(_tokenB)); - assertEq(lPrice, 98_918_868_099_219_913_512); - (lPrice,,) = _oracle.priceCache(address(_tokenB), address(_tokenA)); - assertEq(lPrice, 0); - uint256 lExpectedRewardReceived = block.basefee * _oracle.rewardGasAmount(); + assertEq(lPrice, lCurrentPrice); + uint256 lExpectedRewardReceived = block.basefee * _oracle.rewardGasAmount() * _oracle.MAX_REWARD_MULTIPLIER(); assertEq(address(this).balance, lExpectedRewardReceived); - assertEq(address(_oracle).balance, 1 ether - lExpectedRewardReceived); + assertEq(address(_oracle).balance, ORACLE_STARTING_BALANCE - lExpectedRewardReceived); } function testUpdatePrice_RewardEligible_InsufficientReward(uint256 aRewardAvailable) external { // assume - uint256 lRewardAvailable = bound(aRewardAvailable, 1, block.basefee * _oracle.rewardGasAmount() - 1); + uint256 lRewardAvailable = bound(aRewardAvailable, 1, block.basefee * _oracle.rewardGasAmount() * _oracle.MAX_REWARD_MULTIPLIER() - 1); // arrange deal(address(_oracle), lRewardAvailable); @@ -576,9 +594,8 @@ contract ReservoirPriceOracleTest is BaseTest { skip(1); _pair.sync(); - skip(_oracle.twapPeriod() * 2); - _tokenA.mint(address(_pair), 2e18); - _pair.swap(2e18, true, address(this), ""); + skip(_oracle.twapPeriod()); + _pair.sync(); // act _oracle.updatePrice(address(_tokenA), address(_tokenB), address(0)); @@ -589,6 +606,24 @@ contract ReservoirPriceOracleTest is BaseTest { assertEq(address(_oracle).balance, lOracleBalanceStart); } + function testUpdatePrice_RewardEligible_ContractNoReceive() external { + // arrange + _writePriceCache(address(_tokenA), address(_tokenB), 5e18); + + skip(1); + _pair.sync(); + skip(_oracle.twapPeriod()); + _pair.sync(); + + // act + _oracle.updatePrice(address(_tokenA), address(_tokenB), address(_pair)); // recipient set to any contract that doesn't have a `receive` function + + // assert + (uint256 lPrice,,) = _oracle.priceCache(address(_tokenA), address(_tokenB)); + assertNotEq(lPrice, 5e18); // price is updated nonetheless + assertEq(address(_pair).balance, 0); + } + function testUpdatePrice_IntermediateRoutes() external { // arrange address lStart = address(_tokenA); From d9453acb650789d20b0a2770861213611dc92be9 Mon Sep 17 00:00:00 2001 From: "A.L." Date: Thu, 8 Aug 2024 23:48:43 +0200 Subject: [PATCH 10/17] lint: rm console statement --- test/unit/ReservoirPriceOracle.t.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/test/unit/ReservoirPriceOracle.t.sol b/test/unit/ReservoirPriceOracle.t.sol index b50a924..39ac0dc 100644 --- a/test/unit/ReservoirPriceOracle.t.sol +++ b/test/unit/ReservoirPriceOracle.t.sol @@ -486,7 +486,6 @@ contract ReservoirPriceOracleTest is BaseTest { uint256 lStartingPrice = lCurrentPrice * WAD / (WAD + lPercentDiff); _writePriceCache(address(_tokenA), address(_tokenB), lStartingPrice); - console2.log(lStartingPrice); skip(1); _pair.sync(); skip(_oracle.twapPeriod()); From 63f4583481861dd6eb44fa97acddca0e4716b00a Mon Sep 17 00:00:00 2001 From: "A.L." Date: Thu, 8 Aug 2024 23:52:44 +0200 Subject: [PATCH 11/17] test: rename variable --- test/large/ReservoirPriceOracleLarge.t.sol | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/large/ReservoirPriceOracleLarge.t.sol b/test/large/ReservoirPriceOracleLarge.t.sol index d5a9c94..b0dd10f 100644 --- a/test/large/ReservoirPriceOracleLarge.t.sol +++ b/test/large/ReservoirPriceOracleLarge.t.sol @@ -78,10 +78,10 @@ contract ReservoirPriceOracleLargeTest is ReservoirPriceOracleTest { lRoute[3] = aTokenAAddress; } - uint16[] memory lBpDiffForMaxReward = new uint16[](3); - lBpDiffForMaxReward[0] = lBpDiffForMaxReward[1] = lBpDiffForMaxReward[2] = Constants.BP_SCALE; + uint16[] memory lRewardThresholds = new uint16[](3); + lRewardThresholds[0] = lRewardThresholds[1] = lRewardThresholds[2] = Constants.BP_SCALE; - _oracle.setRoute(lRoute[0], lRoute[3], lRoute, lBpDiffForMaxReward); + _oracle.setRoute(lRoute[0], lRoute[3], lRoute, lRewardThresholds); _writePriceCache( lTokenA < lTokenB ? aTokenAAddress : aTokenBAddress, lTokenA < lTokenB ? aTokenBAddress : aTokenAAddress, From e143bbbc2043c4a089a6aa869bb407b32fe6d463 Mon Sep 17 00:00:00 2001 From: "A.L." Date: Thu, 8 Aug 2024 23:58:43 +0200 Subject: [PATCH 12/17] gas: update snapshot --- .gas-snapshot | 118 +++++++++++++++++++++++++------------------------- 1 file changed, 60 insertions(+), 58 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index 22f2a6e..646eca5 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -1,68 +1,70 @@ -QueryProcessorTest:testFindNearestSample_CanFindExactValue(uint32,uint256,uint256,uint256) (runs: 256, μ: 66759099, ~: 75010823) -QueryProcessorTest:testFindNearestSample_CanFindIntermediateValue(uint32,uint256,uint256,uint256) (runs: 256, μ: 66008847, ~: 75083165) +QueryProcessorTest:testFindNearestSample_CanFindExactValue(uint32,uint256,uint256,uint256) (runs: 256, μ: 67082003, ~: 75570862) +QueryProcessorTest:testFindNearestSample_CanFindIntermediateValue(uint32,uint256,uint256,uint256) (runs: 256, μ: 67588655, ~: 77540282) QueryProcessorTest:testFindNearestSample_NotInitialized() (gas: 1056945756) -QueryProcessorTest:testFindNearestSample_OneSample(uint256) (runs: 256, μ: 80327, ~: 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, μ: 27023, ~: 27087) -QueryProcessorTest:testGetPastAccumulator_ExactMatch(uint32,uint256,uint256,uint16) (runs: 256, μ: 71010550, ~: 79636764) -QueryProcessorTest:testGetPastAccumulator_ExactMatch_LatestAccumulator(uint32,uint256,uint256) (runs: 256, μ: 69665266, ~: 79403270) -QueryProcessorTest:testGetPastAccumulator_ExactMatch_OldestAccumulator(uint32,uint256,uint256) (runs: 256, μ: 69695230, ~: 79434870) -QueryProcessorTest:testGetPastAccumulator_ExtrapolatesBeyondLatest(uint32,uint256,uint256,uint256) (runs: 256, μ: 65982456, ~: 75054423) -QueryProcessorTest:testGetPastAccumulator_InterpolatesBetweenPastAccumulators(uint32,uint256,uint256,uint256) (runs: 256, μ: 66016563, ~: 75089173) -QueryProcessorTest:testGetPastAccumulator_InvalidAgo(uint32,uint256,uint256,uint256) (runs: 256, μ: 65974048, ~: 75046134) -QueryProcessorTest:testGetPastAccumulator_QueryTooOld(uint32,uint256,uint256,uint256) (runs: 256, μ: 65985579, ~: 75056017) -QueryProcessorTest:testGetTimeWeightedAverage(uint32,uint256,uint256,uint256,uint256) (runs: 256, μ: 106133347, ~: 115866805) +QueryProcessorTest:testGetInstantValue_NotInitialized_BeyondBufferSize(uint8,uint16) (runs: 256, μ: 68389673, ~: 68389600) +QueryProcessorTest:testGetPastAccumulator_BufferEmpty(uint8) (runs: 256, μ: 27020, ~: 27087) +QueryProcessorTest:testGetPastAccumulator_ExactMatch(uint32,uint256,uint256,uint16) (runs: 256, μ: 70774779, ~: 79026739) +QueryProcessorTest:testGetPastAccumulator_ExactMatch_LatestAccumulator(uint32,uint256,uint256) (runs: 256, μ: 66794318, ~: 77371310) +QueryProcessorTest:testGetPastAccumulator_ExactMatch_OldestAccumulator(uint32,uint256,uint256) (runs: 256, μ: 66824041, ~: 77402910) +QueryProcessorTest:testGetPastAccumulator_ExtrapolatesBeyondLatest(uint32,uint256,uint256,uint256) (runs: 256, μ: 67562190, ~: 77511143) +QueryProcessorTest:testGetPastAccumulator_InterpolatesBetweenPastAccumulators(uint32,uint256,uint256,uint256) (runs: 256, μ: 67596306, ~: 77546290) +QueryProcessorTest:testGetPastAccumulator_InvalidAgo(uint32,uint256,uint256,uint256) (runs: 256, μ: 67553784, ~: 77503540) +QueryProcessorTest:testGetPastAccumulator_QueryTooOld(uint32,uint256,uint256,uint256) (runs: 256, μ: 67565260, ~: 77513080) +QueryProcessorTest:testGetTimeWeightedAverage(uint32,uint256,uint256,uint256,uint256) (runs: 256, μ: 106780956, ~: 116354216) QueryProcessorTest:testGetTimeWeightedAverage_BadSecs() (gas: 10995) -ReservoirPriceOracleTest:testClearRoute() (gas: 50941) -ReservoirPriceOracleTest:testClearRoute_AllWordsCleared() (gas: 151792) -ReservoirPriceOracleTest:testDesignatePair() (gas: 29113) -ReservoirPriceOracleTest:testDesignatePair_IncorrectPair() (gas: 21111) -ReservoirPriceOracleTest:testDesignatePair_NotOwner() (gas: 17531) -ReservoirPriceOracleTest:testDesignatePair_TokenOrderReversed() (gas: 30729) -ReservoirPriceOracleTest:testGetQuote(uint256,uint256) (runs: 256, μ: 35191, ~: 35303) -ReservoirPriceOracleTest:testGetQuote_AmountInTooLarge() (gas: 12941) -ReservoirPriceOracleTest:testGetQuote_BaseIsVault(uint256) (runs: 256, μ: 417472, ~: 417233) -ReservoirPriceOracleTest:testGetQuote_ComplicatedDecimals() (gas: 10350769) -ReservoirPriceOracleTest:testGetQuote_Inverse(uint256,uint256) (runs: 256, μ: 37319, ~: 37435) -ReservoirPriceOracleTest:testGetQuote_MultipleHops() (gas: 113387) -ReservoirPriceOracleTest:testGetQuote_MultipleHops_Inverse() (gas: 113709) -ReservoirPriceOracleTest:testGetQuote_MultipleHops_PriceZero() (gas: 125327) -ReservoirPriceOracleTest:testGetQuote_NoFallbackOracle() (gas: 20831) -ReservoirPriceOracleTest:testGetQuote_PriceZero() (gas: 15946) -ReservoirPriceOracleTest:testGetQuote_RandomizeAllParam_1HopRoute(uint256,uint256,address,address,uint8,uint8) (runs: 256, μ: 5327411, ~: 5327435) -ReservoirPriceOracleTest:testGetQuote_RandomizeAllParam_2HopRoute(uint256,uint256,uint256,address,address,address,uint8,uint8,uint8) (runs: 256, μ: 10492968, ~: 10493010) -ReservoirPriceOracleTest:testGetQuote_SameBaseQuote(uint256,address) (runs: 256, μ: 8941, ~: 8941) -ReservoirPriceOracleTest:testGetQuote_UseFallback() (gas: 38312) -ReservoirPriceOracleTest:testGetQuote_ZeroIn() (gas: 38148) -ReservoirPriceOracleTest:testGetQuotes(uint256,uint256) (runs: 256, μ: 32724, ~: 32836) -ReservoirPriceOracleTest:testSetFallbackOracle_NotOwner() (gas: 11003) -ReservoirPriceOracleTest:testSetRoute() (gas: 58196) -ReservoirPriceOracleTest:testSetRoute_InvalidRoute() (gas: 17964) -ReservoirPriceOracleTest:testSetRoute_InvalidRouteLength() (gas: 17593) -ReservoirPriceOracleTest:testSetRoute_MultipleHops() (gas: 193280) -ReservoirPriceOracleTest:testSetRoute_NotSorted() (gas: 12081) -ReservoirPriceOracleTest:testSetRoute_OverwriteExisting() (gas: 160942) -ReservoirPriceOracleTest:testSetRoute_SameToken() (gas: 12072) -ReservoirPriceOracleTest:testUndesignatePair() (gas: 30279) +ReservoirPriceOracleTest:testClearRoute() (gas: 52419) +ReservoirPriceOracleTest:testClearRoute_AllWordsCleared() (gas: 160380) +ReservoirPriceOracleTest:testDesignatePair() (gas: 29068) +ReservoirPriceOracleTest:testDesignatePair_IncorrectPair() (gas: 21155) +ReservoirPriceOracleTest:testDesignatePair_NotOwner() (gas: 17487) +ReservoirPriceOracleTest:testDesignatePair_TokenOrderReversed() (gas: 30639) +ReservoirPriceOracleTest:testGetQuote(uint256,uint256) (runs: 256, μ: 34030, ~: 34140) +ReservoirPriceOracleTest:testGetQuote_AmountInTooLarge() (gas: 12985) +ReservoirPriceOracleTest:testGetQuote_BaseIsVault(uint256) (runs: 256, μ: 411298, ~: 411040) +ReservoirPriceOracleTest:testGetQuote_ComplicatedDecimals() (gas: 10354355) +ReservoirPriceOracleTest:testGetQuote_Inverse(uint256,uint256) (runs: 256, μ: 36170, ~: 36338) +ReservoirPriceOracleTest:testGetQuote_MultipleHops() (gas: 111841) +ReservoirPriceOracleTest:testGetQuote_MultipleHops_Inverse() (gas: 112117) +ReservoirPriceOracleTest:testGetQuote_MultipleHops_PriceZero() (gas: 122567) +ReservoirPriceOracleTest:testGetQuote_NoFallbackOracle() (gas: 20842) +ReservoirPriceOracleTest:testGetQuote_PriceZero() (gas: 15958) +ReservoirPriceOracleTest:testGetQuote_RandomizeAllParam_1HopRoute(uint256,uint256,address,address,uint8,uint8) (runs: 256, μ: 5329192, ~: 5329188) +ReservoirPriceOracleTest:testGetQuote_RandomizeAllParam_2HopRoute(uint256,uint256,uint256,address,address,address,uint8,uint8,uint8) (runs: 256, μ: 10496453, ~: 10496500) +ReservoirPriceOracleTest:testGetQuote_SameBaseQuote(uint256,address) (runs: 256, μ: 8963, ~: 8963) +ReservoirPriceOracleTest:testGetQuote_UseFallback() (gas: 38334) +ReservoirPriceOracleTest:testGetQuote_ZeroIn() (gas: 36975) +ReservoirPriceOracleTest:testGetQuotes(uint256,uint256) (runs: 256, μ: 26519, ~: 26629) +ReservoirPriceOracleTest:testPriceCache_Inverted() (gas: 22001) +ReservoirPriceOracleTest:testSetFallbackOracle_NotOwner() (gas: 10960) +ReservoirPriceOracleTest:testSetRoute() (gas: 61215) +ReservoirPriceOracleTest:testSetRoute_InvalidRoute() (gas: 20176) +ReservoirPriceOracleTest:testSetRoute_InvalidRouteLength() (gas: 19316) +ReservoirPriceOracleTest:testSetRoute_MultipleHops() (gas: 201832) +ReservoirPriceOracleTest:testSetRoute_NotSorted() (gas: 13028) +ReservoirPriceOracleTest:testSetRoute_OverwriteExisting() (gas: 170189) +ReservoirPriceOracleTest:testSetRoute_SameToken() (gas: 13063) +ReservoirPriceOracleTest:testUndesignatePair() (gas: 30256) ReservoirPriceOracleTest:testUndesignatePair_NotOwner() (gas: 15310) -ReservoirPriceOracleTest:testUpdatePriceDeviationThreshold(uint256) (runs: 256, μ: 21306, ~: 21063) -ReservoirPriceOracleTest:testUpdatePrice_BeyondThreshold() (gas: 213594) -ReservoirPriceOracleTest:testUpdatePrice_BeyondThreshold_InsufficientReward(uint256) (runs: 256, μ: 209575, ~: 209653) -ReservoirPriceOracleTest:testUpdatePrice_BeyondThreshold_ZeroRecipient() (gas: 195472) -ReservoirPriceOracleTest:testUpdatePrice_FirstUpdate() (gas: 203166) -ReservoirPriceOracleTest:testUpdatePrice_IntermediateRoutes() (gas: 15867838) -ReservoirPriceOracleTest:testUpdatePrice_PriceOutOfRange() (gas: 5350485) -ReservoirPriceOracleTest:testUpdatePrice_WithinThreshold() (gas: 204011) -ReservoirPriceOracleTest:testUpdateRewardGasAmount() (gas: 19011) -ReservoirPriceOracleTest:testUpdateRewardGasAmount_NotOwner() (gas: 10962) -ReservoirPriceOracleTest:testUpdateTwapPeriod(uint256) (runs: 256, μ: 21723, ~: 21806) -ReservoirPriceOracleTest:testUpdateTwapPeriod_InvalidTwapPeriod(uint256) (runs: 256, μ: 17861, ~: 18164) -ReservoirPriceOracleTest:testWritePriceCache(uint256) (runs: 256, μ: 30004, ~: 29765) +ReservoirPriceOracleTest:testUpdatePrice_AboveThresholdBelowMaxReward(uint256) (runs: 256, μ: 165410, ~: 165430) +ReservoirPriceOracleTest:testUpdatePrice_BelowThreshold(uint256) (runs: 256, μ: 150274, ~: 149937) +ReservoirPriceOracleTest:testUpdatePrice_BeyondMaxReward(uint256) (runs: 256, μ: 162912, ~: 162937) +ReservoirPriceOracleTest:testUpdatePrice_FirstUpdate() (gas: 153864) +ReservoirPriceOracleTest:testUpdatePrice_IntermediateRoutes() (gas: 15897882) +ReservoirPriceOracleTest:testUpdatePrice_PriceOutOfRange() (gas: 5353642) +ReservoirPriceOracleTest:testUpdatePrice_RewardEligible_ContractNoReceive() (gas: 153424) +ReservoirPriceOracleTest:testUpdatePrice_RewardEligible_InsufficientReward(uint256) (runs: 256, μ: 211617, ~: 211823) +ReservoirPriceOracleTest:testUpdatePrice_RewardEligible_ZeroRecipient() (gas: 144295) +ReservoirPriceOracleTest:testUpdateRewardGasAmount() (gas: 19039) +ReservoirPriceOracleTest:testUpdateRewardGasAmount_NotOwner() (gas: 10940) +ReservoirPriceOracleTest:testUpdateTwapPeriod(uint256) (runs: 256, μ: 21701, ~: 21800) +ReservoirPriceOracleTest:testUpdateTwapPeriod_InvalidTwapPeriod(uint256) (runs: 256, μ: 17846, ~: 18120) +ReservoirPriceOracleTest:testWritePriceCache(uint256) (runs: 256, μ: 30168, ~: 29910) RoutesLibTest:testGetDecimalDifference() (gas: 3974) RoutesLibTest:testIsCompositeRoute() (gas: 4341) -RoutesLibTest:testPackSimplePrice(int8,uint256) (runs: 256, μ: 7786, ~: 7555) +RoutesLibTest:testPackSimplePrice(int8,uint256) (runs: 256, μ: 8203, ~: 7962) SamplesTest:testAccumulator() (gas: 3959) SamplesTest:testAccumulator_BadVariableRequest() (gas: 3523) SamplesTest:testInstant() (gas: 3909) From ed8d0bce21523927dcfffc37fba213270befd494 Mon Sep 17 00:00:00 2001 From: "A.L." Date: Fri, 9 Aug 2024 00:06:30 +0200 Subject: [PATCH 13/17] docs: update comments --- src/ReservoirPriceOracle.sol | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/ReservoirPriceOracle.sol b/src/ReservoirPriceOracle.sol index ea58ff2..b87831d 100644 --- a/src/ReservoirPriceOracle.sol +++ b/src/ReservoirPriceOracle.sol @@ -115,7 +115,7 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar /// @param aToken1 Address of the higher token. /// @return rPrice The cached price of aToken0/aToken1 for simple routes. Returns 0 for prices of composite routes. /// @return rDecimalDiff The difference in decimals as defined by aToken1.decimals() - aToken0.decimals(). Only valid for simple routes. - /// @return rRewardThreshold The number of basis points at and beyond which the bounty payout for a price update is maximum. + /// @return rRewardThreshold The number of basis points of difference in price at and beyond which a reward is applicable for a price update. function priceCache(address aToken0, address aToken1) external view @@ -187,7 +187,7 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar { if (aRecipient == address(0)) return; - // SAFETY: this mul will not overflow as `aRewardThreshold` is capped by `Constants.BP_SCALE` + // 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; @@ -214,7 +214,7 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar assert( lPercentDiff >= lRewardThresholdWAD && lPercentDiff < lRewardThresholdWAD * MAX_REWARD_MULTIPLIER ); - lPayoutAmt = block.basefee * rewardGasAmount * lPercentDiff / lRewardThresholdWAD; + lPayoutAmt = block.basefee * rewardGasAmount * lPercentDiff / lRewardThresholdWAD; // denominator is never 0 } } @@ -227,7 +227,7 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar /// @return rRoute The route to determine the price between aToken0 and aToken1 /// @return rDecimalDiff The result of token1.decimals() - token0.decimals() if it's a simple route. 0 otherwise /// @return rPrice The price of aToken0/aToken1 if it's a simple route (i.e. rRoute.length == 2). 0 otherwise - /// @return rRewardThreshold The price difference at and beyond which the maximum amount of gas bounty is applicable. + /// @return rRewardThreshold The number of basis points of difference in price at and beyond which a reward is applicable for a price update. function _getRouteDecimalDifferencePrice(address aToken0, address aToken1) private view @@ -298,7 +298,7 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar } } - // performs an SLOAD to load 1 word which contains the simple price, decimal difference, and number of basis points for maximum reward + // performs an SLOAD to load 1 word which contains the simple price, decimal difference, and the reward threshold function _priceCache(address aToken0, address aToken1) private view @@ -467,7 +467,7 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar /// @param aToken0 Address of the lower token. /// @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 the bounty payout for a price update is maximum. + /// @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) public onlyOwner @@ -489,15 +489,16 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar int256 lDiff = int256(lToken1Decimals) - int256(lToken0Decimals); - if (aRewardThresholds[0] > Constants.BP_SCALE) revert OracleErrors.InvalidRewardThreshold(); + uint256 lRewardThreshold = aRewardThresholds[0]; + if (lRewardThreshold > Constants.BP_SCALE || lRewardThreshold == 0) revert OracleErrors.InvalidRewardThreshold(); - bytes32 lData = RoutesLib.packSimplePrice(lDiff, 0, aRewardThresholds[0]); + bytes32 lData = RoutesLib.packSimplePrice(lDiff, 0, lRewardThreshold); assembly ("memory-safe") { // Write data to storage. sstore(lSlot, lData) } - emit PriceUpdateRewardThreshold(aToken0, aToken1, aRewardThresholds[0]); + emit PriceUpdateRewardThreshold(aToken0, aToken1, lRewardThreshold); } // composite route else { From fddc674742485a1d58fd42302da8a76be1e6bb9a Mon Sep 17 00:00:00 2001 From: "A.L." Date: Fri, 9 Aug 2024 00:24:13 +0200 Subject: [PATCH 14/17] fix: introduce limits for threshold --- src/ReservoirPriceOracle.sol | 2 +- src/libraries/RoutesLib.sol | 2 +- test/unit/ReservoirPriceOracle.t.sol | 20 ++++++++++++++++++++ 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/ReservoirPriceOracle.sol b/src/ReservoirPriceOracle.sol index b87831d..adfcd59 100644 --- a/src/ReservoirPriceOracle.sol +++ b/src/ReservoirPriceOracle.sol @@ -29,7 +29,7 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar event DesignatePair(address token0, address token1, ReservoirPair pair); event FallbackOracleSet(address fallbackOracle); - event PriceUpdateRewardThreshold(address token0, address token1, uint16 threshold); + event PriceUpdateRewardThreshold(address token0, address token1, uint256 threshold); event RewardGasAmount(uint256 newAmount); event Route(address token0, address token1, address[] route); event TwapPeriod(uint256 newPeriod); diff --git a/src/libraries/RoutesLib.sol b/src/libraries/RoutesLib.sol index fd6b614..294b561 100644 --- a/src/libraries/RoutesLib.sol +++ b/src/libraries/RoutesLib.sol @@ -30,7 +30,7 @@ library RoutesLib { // Assumes that aDecimalDifference is between -18 and 18 // Assumes that aPrice is between 1 and 1e36 // Assumes that aRewardThreshold is <= Constants.BP_SCALE - function packSimplePrice(int256 aDecimalDifference, uint256 aPrice, uint16 aRewardThreshold) + function packSimplePrice(int256 aDecimalDifference, uint256 aPrice, uint256 aRewardThreshold) internal pure returns (bytes32 rPacked) diff --git a/test/unit/ReservoirPriceOracle.t.sol b/test/unit/ReservoirPriceOracle.t.sol index 39ac0dc..3d6ed42 100644 --- a/test/unit/ReservoirPriceOracle.t.sol +++ b/test/unit/ReservoirPriceOracle.t.sol @@ -268,6 +268,7 @@ contract ReservoirPriceOracleTest is BaseTest { address[] memory lRoute = new address[](2); uint16[] memory lRewardThreshold = new uint16[](1); + lRewardThreshold[0] = Constants.BP_SCALE; (lRoute[0], lRoute[1]) = lTokenA < lTokenB ? (address(lTokenA), address(lTokenB)) : (address(lTokenB), address(lTokenA)); _oracle.setRoute(lRoute[0], lRoute[1], lRoute, lRewardThreshold); @@ -329,6 +330,7 @@ contract ReservoirPriceOracleTest is BaseTest { lRoute[1] = address(lTokenB); uint16[] memory lRewardThreshold = new uint16[](2); + lRewardThreshold[0] = lRewardThreshold[1] = Constants.BP_SCALE; _oracle.setRoute(lRoute[0], lRoute[2], lRoute, lRewardThreshold); _writePriceCache( address(lTokenA) < address(lTokenB) ? address(lTokenA) : address(lTokenB), @@ -341,6 +343,7 @@ contract ReservoirPriceOracleTest is BaseTest { lPrice2 ); } + // act uint256 lAmtCOut = _oracle.getQuote(lAmtIn * 10 ** lTokenADecimal, address(lTokenA), address(lTokenC)); @@ -1024,6 +1027,23 @@ contract ReservoirPriceOracleTest is BaseTest { _oracle.setRoute(lToken0, lToken1, lInvalidRoute2, lRewardThreshold); } + function testSetRoute_InvalidRewardThreshold() external { + // arrange + address[] memory lRoute = new address[](2); + uint16[] memory lInvalidRewardThreshold = new uint16[](1); + lInvalidRewardThreshold[0] = Constants.BP_SCALE + 1; + lRoute[0] = address(_tokenC); + lRoute[1] = address(_tokenD); + + // act & assert + vm.expectRevert(OracleErrors.InvalidRewardThreshold.selector); + _oracle.setRoute(lRoute[0], lRoute[1], lRoute, lInvalidRewardThreshold); + + lInvalidRewardThreshold[0] = 0; + vm.expectRevert(OracleErrors.InvalidRewardThreshold.selector); + _oracle.setRoute(lRoute[0], lRoute[1], lRoute, lInvalidRewardThreshold); + } + function testUpdateRewardGasAmount_NotOwner() external { // act & assert vm.prank(address(123)); From bb78e8c8e10d50a5e60cda906d2a009bd8657ba4 Mon Sep 17 00:00:00 2001 From: "A.L." Date: Fri, 9 Aug 2024 00:26:50 +0200 Subject: [PATCH 15/17] gas: update snapshot --- .gas-snapshot | 91 ++++++++++++++++++++++++++------------------------- 1 file changed, 46 insertions(+), 45 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index 646eca5..24f81fd 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -1,70 +1,71 @@ -QueryProcessorTest:testFindNearestSample_CanFindExactValue(uint32,uint256,uint256,uint256) (runs: 256, μ: 67082003, ~: 75570862) -QueryProcessorTest:testFindNearestSample_CanFindIntermediateValue(uint32,uint256,uint256,uint256) (runs: 256, μ: 67588655, ~: 77540282) +QueryProcessorTest:testFindNearestSample_CanFindExactValue(uint32,uint256,uint256,uint256) (runs: 256, μ: 66272877, ~: 74998599) +QueryProcessorTest:testFindNearestSample_CanFindIntermediateValue(uint32,uint256,uint256,uint256) (runs: 256, μ: 65906137, ~: 75387437) QueryProcessorTest:testFindNearestSample_NotInitialized() (gas: 1056945756) -QueryProcessorTest:testFindNearestSample_OneSample(uint256) (runs: 256, μ: 80329, ~: 80360) +QueryProcessorTest:testFindNearestSample_OneSample(uint256) (runs: 256, μ: 80330, ~: 80360) QueryProcessorTest:testGetInstantValue() (gas: 124248) QueryProcessorTest:testGetInstantValue_NotInitialized(uint256) (runs: 256, μ: 19397, ~: 19397) -QueryProcessorTest:testGetInstantValue_NotInitialized_BeyondBufferSize(uint8,uint16) (runs: 256, μ: 68389673, ~: 68389600) -QueryProcessorTest:testGetPastAccumulator_BufferEmpty(uint8) (runs: 256, μ: 27020, ~: 27087) -QueryProcessorTest:testGetPastAccumulator_ExactMatch(uint32,uint256,uint256,uint16) (runs: 256, μ: 70774779, ~: 79026739) -QueryProcessorTest:testGetPastAccumulator_ExactMatch_LatestAccumulator(uint32,uint256,uint256) (runs: 256, μ: 66794318, ~: 77371310) -QueryProcessorTest:testGetPastAccumulator_ExactMatch_OldestAccumulator(uint32,uint256,uint256) (runs: 256, μ: 66824041, ~: 77402910) -QueryProcessorTest:testGetPastAccumulator_ExtrapolatesBeyondLatest(uint32,uint256,uint256,uint256) (runs: 256, μ: 67562190, ~: 77511143) -QueryProcessorTest:testGetPastAccumulator_InterpolatesBetweenPastAccumulators(uint32,uint256,uint256,uint256) (runs: 256, μ: 67596306, ~: 77546290) -QueryProcessorTest:testGetPastAccumulator_InvalidAgo(uint32,uint256,uint256,uint256) (runs: 256, μ: 67553784, ~: 77503540) -QueryProcessorTest:testGetPastAccumulator_QueryTooOld(uint32,uint256,uint256,uint256) (runs: 256, μ: 67565260, ~: 77513080) -QueryProcessorTest:testGetTimeWeightedAverage(uint32,uint256,uint256,uint256,uint256) (runs: 256, μ: 106780956, ~: 116354216) +QueryProcessorTest:testGetInstantValue_NotInitialized_BeyondBufferSize(uint8,uint16) (runs: 256, μ: 68389666, ~: 68389600) +QueryProcessorTest:testGetPastAccumulator_BufferEmpty(uint8) (runs: 256, μ: 27017, ~: 27087) +QueryProcessorTest:testGetPastAccumulator_ExactMatch(uint32,uint256,uint256,uint16) (runs: 256, μ: 72348369, ~: 82308754) +QueryProcessorTest:testGetPastAccumulator_ExactMatch_LatestAccumulator(uint32,uint256,uint256) (runs: 256, μ: 65365980, ~: 75965010) +QueryProcessorTest:testGetPastAccumulator_ExactMatch_OldestAccumulator(uint32,uint256,uint256) (runs: 256, μ: 65395698, ~: 75996610) +QueryProcessorTest:testGetPastAccumulator_ExtrapolatesBeyondLatest(uint32,uint256,uint256,uint256) (runs: 256, μ: 65879780, ~: 75358643) +QueryProcessorTest:testGetPastAccumulator_InterpolatesBetweenPastAccumulators(uint32,uint256,uint256,uint256) (runs: 256, μ: 65913853, ~: 75393445) +QueryProcessorTest:testGetPastAccumulator_InvalidAgo(uint32,uint256,uint256,uint256) (runs: 256, μ: 65871378, ~: 75350354) +QueryProcessorTest:testGetPastAccumulator_QueryTooOld(uint32,uint256,uint256,uint256) (runs: 256, μ: 65882914, ~: 75360237) +QueryProcessorTest:testGetTimeWeightedAverage(uint32,uint256,uint256,uint256,uint256) (runs: 256, μ: 105247590, ~: 115523130) QueryProcessorTest:testGetTimeWeightedAverage_BadSecs() (gas: 10995) -ReservoirPriceOracleTest:testClearRoute() (gas: 52419) -ReservoirPriceOracleTest:testClearRoute_AllWordsCleared() (gas: 160380) +ReservoirPriceOracleTest:testClearRoute() (gas: 52339) +ReservoirPriceOracleTest:testClearRoute_AllWordsCleared() (gas: 159879) ReservoirPriceOracleTest:testDesignatePair() (gas: 29068) ReservoirPriceOracleTest:testDesignatePair_IncorrectPair() (gas: 21155) -ReservoirPriceOracleTest:testDesignatePair_NotOwner() (gas: 17487) +ReservoirPriceOracleTest:testDesignatePair_NotOwner() (gas: 17553) ReservoirPriceOracleTest:testDesignatePair_TokenOrderReversed() (gas: 30639) -ReservoirPriceOracleTest:testGetQuote(uint256,uint256) (runs: 256, μ: 34030, ~: 34140) -ReservoirPriceOracleTest:testGetQuote_AmountInTooLarge() (gas: 12985) -ReservoirPriceOracleTest:testGetQuote_BaseIsVault(uint256) (runs: 256, μ: 411298, ~: 411040) -ReservoirPriceOracleTest:testGetQuote_ComplicatedDecimals() (gas: 10354355) -ReservoirPriceOracleTest:testGetQuote_Inverse(uint256,uint256) (runs: 256, μ: 36170, ~: 36338) +ReservoirPriceOracleTest:testGetQuote(uint256,uint256) (runs: 256, μ: 34006, ~: 34118) +ReservoirPriceOracleTest:testGetQuote_AmountInTooLarge() (gas: 12963) +ReservoirPriceOracleTest:testGetQuote_BaseIsVault(uint256) (runs: 256, μ: 411293, ~: 411040) +ReservoirPriceOracleTest:testGetQuote_ComplicatedDecimals() (gas: 10354021) +ReservoirPriceOracleTest:testGetQuote_Inverse(uint256,uint256) (runs: 256, μ: 36143, ~: 36315) ReservoirPriceOracleTest:testGetQuote_MultipleHops() (gas: 111841) -ReservoirPriceOracleTest:testGetQuote_MultipleHops_Inverse() (gas: 112117) +ReservoirPriceOracleTest:testGetQuote_MultipleHops_Inverse() (gas: 112181) ReservoirPriceOracleTest:testGetQuote_MultipleHops_PriceZero() (gas: 122567) -ReservoirPriceOracleTest:testGetQuote_NoFallbackOracle() (gas: 20842) +ReservoirPriceOracleTest:testGetQuote_NoFallbackOracle() (gas: 20820) ReservoirPriceOracleTest:testGetQuote_PriceZero() (gas: 15958) -ReservoirPriceOracleTest:testGetQuote_RandomizeAllParam_1HopRoute(uint256,uint256,address,address,uint8,uint8) (runs: 256, μ: 5329192, ~: 5329188) -ReservoirPriceOracleTest:testGetQuote_RandomizeAllParam_2HopRoute(uint256,uint256,uint256,address,address,address,uint8,uint8,uint8) (runs: 256, μ: 10496453, ~: 10496500) -ReservoirPriceOracleTest:testGetQuote_SameBaseQuote(uint256,address) (runs: 256, μ: 8963, ~: 8963) +ReservoirPriceOracleTest:testGetQuote_RandomizeAllParam_1HopRoute(uint256,uint256,address,address,uint8,uint8) (runs: 256, μ: 5329109, ~: 5329104) +ReservoirPriceOracleTest:testGetQuote_RandomizeAllParam_2HopRoute(uint256,uint256,uint256,address,address,address,uint8,uint8,uint8) (runs: 256, μ: 10496294, ~: 10496397) +ReservoirPriceOracleTest:testGetQuote_SameBaseQuote(uint256,address) (runs: 256, μ: 8941, ~: 8941) ReservoirPriceOracleTest:testGetQuote_UseFallback() (gas: 38334) ReservoirPriceOracleTest:testGetQuote_ZeroIn() (gas: 36975) -ReservoirPriceOracleTest:testGetQuotes(uint256,uint256) (runs: 256, μ: 26519, ~: 26629) +ReservoirPriceOracleTest:testGetQuotes(uint256,uint256) (runs: 256, μ: 26517, ~: 26629) ReservoirPriceOracleTest:testPriceCache_Inverted() (gas: 22001) -ReservoirPriceOracleTest:testSetFallbackOracle_NotOwner() (gas: 10960) -ReservoirPriceOracleTest:testSetRoute() (gas: 61215) -ReservoirPriceOracleTest:testSetRoute_InvalidRoute() (gas: 20176) -ReservoirPriceOracleTest:testSetRoute_InvalidRouteLength() (gas: 19316) -ReservoirPriceOracleTest:testSetRoute_MultipleHops() (gas: 201832) +ReservoirPriceOracleTest:testSetFallbackOracle_NotOwner() (gas: 10938) +ReservoirPriceOracleTest:testSetRoute() (gas: 61093) +ReservoirPriceOracleTest:testSetRoute_InvalidRewardThreshold() (gas: 37330) +ReservoirPriceOracleTest:testSetRoute_InvalidRoute() (gas: 20154) +ReservoirPriceOracleTest:testSetRoute_InvalidRouteLength() (gas: 19294) +ReservoirPriceOracleTest:testSetRoute_MultipleHops() (gas: 201309) ReservoirPriceOracleTest:testSetRoute_NotSorted() (gas: 13028) -ReservoirPriceOracleTest:testSetRoute_OverwriteExisting() (gas: 170189) -ReservoirPriceOracleTest:testSetRoute_SameToken() (gas: 13063) +ReservoirPriceOracleTest:testSetRoute_OverwriteExisting() (gas: 169666) +ReservoirPriceOracleTest:testSetRoute_SameToken() (gas: 13041) ReservoirPriceOracleTest:testUndesignatePair() (gas: 30256) -ReservoirPriceOracleTest:testUndesignatePair_NotOwner() (gas: 15310) -ReservoirPriceOracleTest:testUpdatePrice_AboveThresholdBelowMaxReward(uint256) (runs: 256, μ: 165410, ~: 165430) -ReservoirPriceOracleTest:testUpdatePrice_BelowThreshold(uint256) (runs: 256, μ: 150274, ~: 149937) -ReservoirPriceOracleTest:testUpdatePrice_BeyondMaxReward(uint256) (runs: 256, μ: 162912, ~: 162937) +ReservoirPriceOracleTest:testUndesignatePair_NotOwner() (gas: 15355) +ReservoirPriceOracleTest:testUpdatePrice_AboveThresholdBelowMaxReward(uint256) (runs: 256, μ: 165388, ~: 165408) +ReservoirPriceOracleTest:testUpdatePrice_BelowThreshold(uint256) (runs: 256, μ: 150249, ~: 149915) +ReservoirPriceOracleTest:testUpdatePrice_BeyondMaxReward(uint256) (runs: 256, μ: 162890, ~: 162915) ReservoirPriceOracleTest:testUpdatePrice_FirstUpdate() (gas: 153864) -ReservoirPriceOracleTest:testUpdatePrice_IntermediateRoutes() (gas: 15897882) -ReservoirPriceOracleTest:testUpdatePrice_PriceOutOfRange() (gas: 5353642) +ReservoirPriceOracleTest:testUpdatePrice_IntermediateRoutes() (gas: 15897381) +ReservoirPriceOracleTest:testUpdatePrice_PriceOutOfRange() (gas: 5353475) ReservoirPriceOracleTest:testUpdatePrice_RewardEligible_ContractNoReceive() (gas: 153424) -ReservoirPriceOracleTest:testUpdatePrice_RewardEligible_InsufficientReward(uint256) (runs: 256, μ: 211617, ~: 211823) +ReservoirPriceOracleTest:testUpdatePrice_RewardEligible_InsufficientReward(uint256) (runs: 256, μ: 211615, ~: 211823) ReservoirPriceOracleTest:testUpdatePrice_RewardEligible_ZeroRecipient() (gas: 144295) ReservoirPriceOracleTest:testUpdateRewardGasAmount() (gas: 19039) ReservoirPriceOracleTest:testUpdateRewardGasAmount_NotOwner() (gas: 10940) -ReservoirPriceOracleTest:testUpdateTwapPeriod(uint256) (runs: 256, μ: 21701, ~: 21800) -ReservoirPriceOracleTest:testUpdateTwapPeriod_InvalidTwapPeriod(uint256) (runs: 256, μ: 17846, ~: 18120) -ReservoirPriceOracleTest:testWritePriceCache(uint256) (runs: 256, μ: 30168, ~: 29910) +ReservoirPriceOracleTest:testUpdateTwapPeriod(uint256) (runs: 256, μ: 21684, ~: 21778) +ReservoirPriceOracleTest:testUpdateTwapPeriod_InvalidTwapPeriod(uint256) (runs: 256, μ: 17838, ~: 18120) +ReservoirPriceOracleTest:testWritePriceCache(uint256) (runs: 256, μ: 30230, ~: 29977) RoutesLibTest:testGetDecimalDifference() (gas: 3974) RoutesLibTest:testIsCompositeRoute() (gas: 4341) -RoutesLibTest:testPackSimplePrice(int8,uint256) (runs: 256, μ: 8203, ~: 7962) +RoutesLibTest:testPackSimplePrice(int8,uint256) (runs: 256, μ: 8200, ~: 7962) SamplesTest:testAccumulator() (gas: 3959) SamplesTest:testAccumulator_BadVariableRequest() (gas: 3523) SamplesTest:testInstant() (gas: 3909) From b0c22f0850d0bbf04ea12965dd4dcc5b0c9e1914 Mon Sep 17 00:00:00 2001 From: "A.L." Date: Fri, 9 Aug 2024 18:53:37 +0200 Subject: [PATCH 16/17] fix: PR comments --- src/libraries/RoutesLib.sol | 5 ++--- test/__fixtures/BaseTest.t.sol | 3 ++- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libraries/RoutesLib.sol b/src/libraries/RoutesLib.sol index 294b561..cc8bbf8 100644 --- a/src/libraries/RoutesLib.sol +++ b/src/libraries/RoutesLib.sol @@ -54,9 +54,8 @@ library RoutesLib { rSecondWord = bytes20(aThirdToken); } - // Positive value indicates that token1 has a greater number of decimals compared to token2 - // while a negative value indicates otherwise. - // range of values between -18 and 18 + // Positive value indicates that token1 has a greater number of decimals compared to token0 + // while a negative value indicates otherwise. Range of values is between -18 and 18 function getDecimalDifference(bytes32 aData) internal pure returns (int256 rDiff) { rDiff = int8(uint8(aData[1])); } diff --git a/test/__fixtures/BaseTest.t.sol b/test/__fixtures/BaseTest.t.sol index 735eff5..79fb3e6 100644 --- a/test/__fixtures/BaseTest.t.sol +++ b/test/__fixtures/BaseTest.t.sol @@ -17,12 +17,13 @@ contract BaseTest is Test { using FactoryStoreLib for GenericFactory; uint64 internal constant DEFAULT_REWARD_GAS_AMOUNT = 200_000; + uint64 internal constant DEFAULT_TWAP_PERIOD = 15 minutes; GenericFactory internal _factory = new GenericFactory(); ReservoirPair internal _pair; ReservoirPriceOracle internal _oracle = - new ReservoirPriceOracle(0.15 minutes, DEFAULT_REWARD_GAS_AMOUNT, PriceType.CLAMPED_PRICE); + new ReservoirPriceOracle(DEFAULT_TWAP_PERIOD, DEFAULT_REWARD_GAS_AMOUNT, PriceType.CLAMPED_PRICE); MintableERC20 internal _tokenA = MintableERC20(address(0x100)); MintableERC20 internal _tokenB = MintableERC20(address(0x200)); From 8d4bc4cd1f3c313337585032e831afc59fb48e58 Mon Sep 17 00:00:00 2001 From: "A.L." Date: Fri, 9 Aug 2024 18:53:58 +0200 Subject: [PATCH 17/17] gas: update snapshot --- .gas-snapshot | 46 +++++++++++++++++++++++----------------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index 24f81fd..935e43a 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -1,19 +1,19 @@ -QueryProcessorTest:testFindNearestSample_CanFindExactValue(uint32,uint256,uint256,uint256) (runs: 256, μ: 66272877, ~: 74998599) -QueryProcessorTest:testFindNearestSample_CanFindIntermediateValue(uint32,uint256,uint256,uint256) (runs: 256, μ: 65906137, ~: 75387437) +QueryProcessorTest:testFindNearestSample_CanFindExactValue(uint32,uint256,uint256,uint256) (runs: 256, μ: 66334752, ~: 75124967) +QueryProcessorTest:testFindNearestSample_CanFindIntermediateValue(uint32,uint256,uint256,uint256) (runs: 256, μ: 65933151, ~: 75772005) QueryProcessorTest:testFindNearestSample_NotInitialized() (gas: 1056945756) -QueryProcessorTest:testFindNearestSample_OneSample(uint256) (runs: 256, μ: 80330, ~: 80360) +QueryProcessorTest:testFindNearestSample_OneSample(uint256) (runs: 256, μ: 80331, ~: 80360) QueryProcessorTest:testGetInstantValue() (gas: 124248) QueryProcessorTest:testGetInstantValue_NotInitialized(uint256) (runs: 256, μ: 19397, ~: 19397) -QueryProcessorTest:testGetInstantValue_NotInitialized_BeyondBufferSize(uint8,uint16) (runs: 256, μ: 68389666, ~: 68389600) -QueryProcessorTest:testGetPastAccumulator_BufferEmpty(uint8) (runs: 256, μ: 27017, ~: 27087) -QueryProcessorTest:testGetPastAccumulator_ExactMatch(uint32,uint256,uint256,uint16) (runs: 256, μ: 72348369, ~: 82308754) -QueryProcessorTest:testGetPastAccumulator_ExactMatch_LatestAccumulator(uint32,uint256,uint256) (runs: 256, μ: 65365980, ~: 75965010) -QueryProcessorTest:testGetPastAccumulator_ExactMatch_OldestAccumulator(uint32,uint256,uint256) (runs: 256, μ: 65395698, ~: 75996610) -QueryProcessorTest:testGetPastAccumulator_ExtrapolatesBeyondLatest(uint32,uint256,uint256,uint256) (runs: 256, μ: 65879780, ~: 75358643) -QueryProcessorTest:testGetPastAccumulator_InterpolatesBetweenPastAccumulators(uint32,uint256,uint256,uint256) (runs: 256, μ: 65913853, ~: 75393445) -QueryProcessorTest:testGetPastAccumulator_InvalidAgo(uint32,uint256,uint256,uint256) (runs: 256, μ: 65871378, ~: 75350354) -QueryProcessorTest:testGetPastAccumulator_QueryTooOld(uint32,uint256,uint256,uint256) (runs: 256, μ: 65882914, ~: 75360237) -QueryProcessorTest:testGetTimeWeightedAverage(uint32,uint256,uint256,uint256,uint256) (runs: 256, μ: 105247590, ~: 115523130) +QueryProcessorTest:testGetInstantValue_NotInitialized_BeyondBufferSize(uint8,uint16) (runs: 256, μ: 68389672, ~: 68389600) +QueryProcessorTest:testGetPastAccumulator_BufferEmpty(uint8) (runs: 256, μ: 27024, ~: 27087) +QueryProcessorTest:testGetPastAccumulator_ExactMatch(uint32,uint256,uint256,uint16) (runs: 256, μ: 71442338, ~: 80821196) +QueryProcessorTest:testGetPastAccumulator_ExactMatch_LatestAccumulator(uint32,uint256,uint256) (runs: 256, μ: 67544657, ~: 77486110) +QueryProcessorTest:testGetPastAccumulator_ExactMatch_OldestAccumulator(uint32,uint256,uint256) (runs: 256, μ: 67574540, ~: 77517710) +QueryProcessorTest:testGetPastAccumulator_ExtrapolatesBeyondLatest(uint32,uint256,uint256,uint256) (runs: 256, μ: 65906778, ~: 75743223) +QueryProcessorTest:testGetPastAccumulator_InterpolatesBetweenPastAccumulators(uint32,uint256,uint256,uint256) (runs: 256, μ: 65940850, ~: 75778013) +QueryProcessorTest:testGetPastAccumulator_InvalidAgo(uint32,uint256,uint256,uint256) (runs: 256, μ: 65898377, ~: 75734934) +QueryProcessorTest:testGetPastAccumulator_QueryTooOld(uint32,uint256,uint256,uint256) (runs: 256, μ: 65909900, ~: 75744817) +QueryProcessorTest:testGetTimeWeightedAverage(uint32,uint256,uint256,uint256,uint256) (runs: 256, μ: 105778530, ~: 115416718) QueryProcessorTest:testGetTimeWeightedAverage_BadSecs() (gas: 10995) ReservoirPriceOracleTest:testClearRoute() (gas: 52339) ReservoirPriceOracleTest:testClearRoute_AllWordsCleared() (gas: 159879) @@ -21,22 +21,22 @@ ReservoirPriceOracleTest:testDesignatePair() (gas: 29068) ReservoirPriceOracleTest:testDesignatePair_IncorrectPair() (gas: 21155) ReservoirPriceOracleTest:testDesignatePair_NotOwner() (gas: 17553) ReservoirPriceOracleTest:testDesignatePair_TokenOrderReversed() (gas: 30639) -ReservoirPriceOracleTest:testGetQuote(uint256,uint256) (runs: 256, μ: 34006, ~: 34118) +ReservoirPriceOracleTest:testGetQuote(uint256,uint256) (runs: 256, μ: 34016, ~: 34118) ReservoirPriceOracleTest:testGetQuote_AmountInTooLarge() (gas: 12963) -ReservoirPriceOracleTest:testGetQuote_BaseIsVault(uint256) (runs: 256, μ: 411293, ~: 411040) +ReservoirPriceOracleTest:testGetQuote_BaseIsVault(uint256) (runs: 256, μ: 411295, ~: 411040) ReservoirPriceOracleTest:testGetQuote_ComplicatedDecimals() (gas: 10354021) -ReservoirPriceOracleTest:testGetQuote_Inverse(uint256,uint256) (runs: 256, μ: 36143, ~: 36315) +ReservoirPriceOracleTest:testGetQuote_Inverse(uint256,uint256) (runs: 256, μ: 36154, ~: 36316) ReservoirPriceOracleTest:testGetQuote_MultipleHops() (gas: 111841) ReservoirPriceOracleTest:testGetQuote_MultipleHops_Inverse() (gas: 112181) ReservoirPriceOracleTest:testGetQuote_MultipleHops_PriceZero() (gas: 122567) ReservoirPriceOracleTest:testGetQuote_NoFallbackOracle() (gas: 20820) ReservoirPriceOracleTest:testGetQuote_PriceZero() (gas: 15958) -ReservoirPriceOracleTest:testGetQuote_RandomizeAllParam_1HopRoute(uint256,uint256,address,address,uint8,uint8) (runs: 256, μ: 5329109, ~: 5329104) -ReservoirPriceOracleTest:testGetQuote_RandomizeAllParam_2HopRoute(uint256,uint256,uint256,address,address,address,uint8,uint8,uint8) (runs: 256, μ: 10496294, ~: 10496397) +ReservoirPriceOracleTest:testGetQuote_RandomizeAllParam_1HopRoute(uint256,uint256,address,address,uint8,uint8) (runs: 256, μ: 5329116, ~: 5329104) +ReservoirPriceOracleTest:testGetQuote_RandomizeAllParam_2HopRoute(uint256,uint256,uint256,address,address,address,uint8,uint8,uint8) (runs: 256, μ: 10496298, ~: 10496410) ReservoirPriceOracleTest:testGetQuote_SameBaseQuote(uint256,address) (runs: 256, μ: 8941, ~: 8941) ReservoirPriceOracleTest:testGetQuote_UseFallback() (gas: 38334) ReservoirPriceOracleTest:testGetQuote_ZeroIn() (gas: 36975) -ReservoirPriceOracleTest:testGetQuotes(uint256,uint256) (runs: 256, μ: 26517, ~: 26629) +ReservoirPriceOracleTest:testGetQuotes(uint256,uint256) (runs: 256, μ: 26527, ~: 26629) ReservoirPriceOracleTest:testPriceCache_Inverted() (gas: 22001) ReservoirPriceOracleTest:testSetFallbackOracle_NotOwner() (gas: 10938) ReservoirPriceOracleTest:testSetRoute() (gas: 61093) @@ -56,13 +56,13 @@ ReservoirPriceOracleTest:testUpdatePrice_FirstUpdate() (gas: 153864) ReservoirPriceOracleTest:testUpdatePrice_IntermediateRoutes() (gas: 15897381) ReservoirPriceOracleTest:testUpdatePrice_PriceOutOfRange() (gas: 5353475) ReservoirPriceOracleTest:testUpdatePrice_RewardEligible_ContractNoReceive() (gas: 153424) -ReservoirPriceOracleTest:testUpdatePrice_RewardEligible_InsufficientReward(uint256) (runs: 256, μ: 211615, ~: 211823) +ReservoirPriceOracleTest:testUpdatePrice_RewardEligible_InsufficientReward(uint256) (runs: 256, μ: 211585, ~: 211793) ReservoirPriceOracleTest:testUpdatePrice_RewardEligible_ZeroRecipient() (gas: 144295) ReservoirPriceOracleTest:testUpdateRewardGasAmount() (gas: 19039) ReservoirPriceOracleTest:testUpdateRewardGasAmount_NotOwner() (gas: 10940) -ReservoirPriceOracleTest:testUpdateTwapPeriod(uint256) (runs: 256, μ: 21684, ~: 21778) -ReservoirPriceOracleTest:testUpdateTwapPeriod_InvalidTwapPeriod(uint256) (runs: 256, μ: 17838, ~: 18120) -ReservoirPriceOracleTest:testWritePriceCache(uint256) (runs: 256, μ: 30230, ~: 29977) +ReservoirPriceOracleTest:testUpdateTwapPeriod(uint256) (runs: 256, μ: 21693, ~: 21778) +ReservoirPriceOracleTest:testUpdateTwapPeriod_InvalidTwapPeriod(uint256) (runs: 256, μ: 17830, ~: 18120) +ReservoirPriceOracleTest:testWritePriceCache(uint256) (runs: 256, μ: 30232, ~: 29977) RoutesLibTest:testGetDecimalDifference() (gas: 3974) RoutesLibTest:testIsCompositeRoute() (gas: 4341) RoutesLibTest:testPackSimplePrice(int8,uint256) (runs: 256, μ: 8200, ~: 7962)