diff --git a/src/ReservoirPriceOracle.sol b/src/ReservoirPriceOracle.sol index b6d0a17..a6178b3 100644 --- a/src/ReservoirPriceOracle.sol +++ b/src/ReservoirPriceOracle.sol @@ -160,11 +160,14 @@ 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,, lRewardThreshold) = _priceCache(lToken0, lToken1); } _writePriceCache(lToken0, lToken1, lNewPrice); - rTotalReward += _calculateReward(lPrevPrice, lNewPrice, lRewardThreshold); + // SAFETY: This will not overflow for, and hops are limited by `MAX_ROUTE_LENGTH` + unchecked { + rTotalReward += _calculateReward(lPrevPrice, lNewPrice, lRewardThreshold); + } } _rewardUpdater(aRewardRecipient, rTotalReward); } @@ -224,7 +227,7 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar } function _rewardUpdater(address aRecipient, uint256 aReward) private { - if (aRecipient == address(0)) return; + if (aRecipient == address(0) || aReward == 0) return; // does not revert under any circumstance assembly ("memory-safe") { diff --git a/test/unit/ReservoirPriceOracle.t.sol b/test/unit/ReservoirPriceOracle.t.sol index c1b4918..a9ad44c 100644 --- a/test/unit/ReservoirPriceOracle.t.sol +++ b/test/unit/ReservoirPriceOracle.t.sol @@ -640,7 +640,7 @@ contract ReservoirPriceOracleTest is BaseTest { address lEnd = address(_tokenB); address[] memory lRoute = new address[](4); uint16[] memory lRewardThreshold = new uint16[](3); - lRewardThreshold[0] = lRewardThreshold[1] = lRewardThreshold[2] = Constants.BP_SCALE; + lRewardThreshold[0] = lRewardThreshold[1] = lRewardThreshold[2] = 3; // 3bp lRoute[0] = lStart; lRoute[1] = lIntermediate1; lRoute[2] = lIntermediate2; @@ -672,10 +672,10 @@ contract ReservoirPriceOracleTest is BaseTest { lAC.sync(); lCD.sync(); lBD.sync(); - skip(_oracle.twapPeriod() * 2); + skip(_oracle.twapPeriod()); // act - _oracle.updatePrice(address(_tokenA), address(_tokenB), address(this)); + uint256 lReward = _oracle.updatePrice(address(_tokenA), address(_tokenB), address(this)); // assert (uint256 lPriceAC,,) = _oracle.priceCache(lStart, lIntermediate1); @@ -686,6 +686,26 @@ contract ReservoirPriceOracleTest is BaseTest { assertApproxEqRel(lPriceCD, 2e18, 0.0001e18); assertApproxEqRel(lPriceBD, 2e18, 0.0001e18); assertEq(lPriceAB, 0); // composite price is not stored in the cache + assertEq(lReward, 0); + + // arrange + skip(_oracle.twapPeriod()); + + // act + uint256 lSwapAmt = 1_000_000; + _tokenA.mint(address(lAC), lSwapAmt * 10 ** _tokenA.decimals()); + lAC.swap(int256(lSwapAmt * 10 ** _tokenA.decimals()), true, address(this), ""); + + _tokenC.mint(address(lCD), lSwapAmt * 10 ** _tokenC.decimals()); + lCD.swap(int256(lSwapAmt * 10 ** _tokenC.decimals()), true, address(this), ""); + + _tokenB.mint(address(lBD), lSwapAmt * 10 ** _tokenB.decimals()); + lBD.swap(int256(lSwapAmt * 10 ** _tokenB.decimals()), true, address(this), ""); + + skip(_oracle.twapPeriod()); + + lReward = _oracle.updatePrice(address(_tokenA), address(_tokenB), address(this)); + assertGt(lReward, _oracle.rewardGasAmount() * 3); // ensure that rewards have accumulated } function testSetRoute() public {