diff --git a/src/ReservoirPriceOracle.sol b/src/ReservoirPriceOracle.sol index adfcd59..b6d0a17 100644 --- a/src/ReservoirPriceOracle.sol +++ b/src/ReservoirPriceOracle.sol @@ -132,7 +132,13 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar /// @param aTokenA Address of one of the tokens for the price update. Does not have to be less than address of aTokenB /// @param aTokenB Address of one of the tokens for the price update. Does not have to be greater than address of aTokenA /// @param aRewardRecipient The beneficiary of the reward. Must be able to receive ether. Set to address(0) if not seeking a reward - function updatePrice(address aTokenA, address aTokenB, address aRewardRecipient) external nonReentrant { + /// @return rTotalReward The total amount of ETH reward if the price was updated in the same call. Mainly used by keepers and MEV bots to simulate offchain if a price update is worth doing. + /// Does not take into account if there is sufficient ETH for rewards in the contract. Oracle could have insufficient ETH resulting in no rewards even if called. + function updatePrice(address aTokenA, address aTokenB, address aRewardRecipient) + external + nonReentrant + returns (uint256 rTotalReward) + { (address lToken0, address lToken1) = Utils.sortTokens(aTokenA, aTokenB); (address[] memory lRoute,, uint256 lPrevPrice, uint256 lRewardThreshold) = @@ -158,8 +164,9 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar } _writePriceCache(lToken0, lToken1, lNewPrice); - _rewardUpdater(lPrevPrice, lNewPrice, aRewardRecipient, lRewardThreshold); + rTotalReward += _calculateReward(lPrevPrice, lNewPrice, lRewardThreshold); } + _rewardUpdater(aRewardRecipient, rTotalReward); } /////////////////////////////////////////////////////////////////////////////////////////////// @@ -182,11 +189,9 @@ 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 aRewardThreshold) - private + function _calculateReward(uint256 aPrevPrice, uint256 aNewPrice, uint256 aRewardThreshold) + private returns (uint256 rReward) { - if (aRecipient == address(0)) return; - // SAFETY: this mul will not overflow as 0 < `aRewardThreshold` <= `Constants.BP_SCALE`, as checked by `setRoute` uint256 lRewardThresholdWAD; unchecked { @@ -194,12 +199,11 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar } uint256 lPercentDiff = aPrevPrice.calcPercentageDiff(aNewPrice); - uint256 lPayoutAmt; // SAFETY: this mul will not overflow even in extreme cases of `block.basefee`. unchecked { if (lPercentDiff < lRewardThresholdWAD) { - return; + return 0; } // payout max reward else if (lPercentDiff >= lRewardThresholdWAD * MAX_REWARD_MULTIPLIER) { @@ -209,18 +213,22 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar // 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; + rReward = block.basefee * rewardGasAmount * MAX_REWARD_MULTIPLIER; } else { assert( lPercentDiff >= lRewardThresholdWAD && lPercentDiff < lRewardThresholdWAD * MAX_REWARD_MULTIPLIER ); - lPayoutAmt = block.basefee * rewardGasAmount * lPercentDiff / lRewardThresholdWAD; // denominator is never 0 + rReward = block.basefee * rewardGasAmount * lPercentDiff / lRewardThresholdWAD; // denominator is never 0 } } + } + + function _rewardUpdater(address aRecipient, uint256 aReward) private { + if (aRecipient == address(0)) return; // does not revert under any circumstance assembly ("memory-safe") { - pop(call(gas(), aRecipient, lPayoutAmt, codesize(), 0x00, codesize(), 0x00)) + pop(call(gas(), aRecipient, aReward, codesize(), 0x00, codesize(), 0x00)) } } @@ -490,7 +498,9 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar int256 lDiff = int256(lToken1Decimals) - int256(lToken0Decimals); uint256 lRewardThreshold = aRewardThresholds[0]; - if (lRewardThreshold > Constants.BP_SCALE || lRewardThreshold == 0) revert OracleErrors.InvalidRewardThreshold(); + if (lRewardThreshold > Constants.BP_SCALE || lRewardThreshold == 0) { + revert OracleErrors.InvalidRewardThreshold(); + } bytes32 lData = RoutesLib.packSimplePrice(lDiff, 0, lRewardThreshold); assembly ("memory-safe") { diff --git a/test/unit/ReservoirPriceOracle.t.sol b/test/unit/ReservoirPriceOracle.t.sol index 3d6ed42..7395a22 100644 --- a/test/unit/ReservoirPriceOracle.t.sol +++ b/test/unit/ReservoirPriceOracle.t.sol @@ -529,8 +529,7 @@ contract ReservoirPriceOracleTest is BaseTest { // assert (uint256 lPrice,,) = _oracle.priceCache(address(_tokenA), address(_tokenB)); assertEq(lPrice, lCurrentPrice); - uint256 lExpectedRewardReceived = - block.basefee * _oracle.rewardGasAmount() * lPercentDiff / lRewardThresholdWAD; + uint256 lExpectedRewardReceived = 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 @@ -568,7 +567,8 @@ contract ReservoirPriceOracleTest is BaseTest { function testUpdatePrice_RewardEligible_InsufficientReward(uint256 aRewardAvailable) external { // assume - uint256 lRewardAvailable = bound(aRewardAvailable, 1, block.basefee * _oracle.rewardGasAmount() * _oracle.MAX_REWARD_MULTIPLIER() - 1); + uint256 lRewardAvailable = + bound(aRewardAvailable, 1, block.basefee * _oracle.rewardGasAmount() * _oracle.MAX_REWARD_MULTIPLIER() - 1); // arrange deal(address(_oracle), lRewardAvailable);