feat: impl return value for updatePrice

reservoir-labs · Sep 13, 2024 · e4268af · e4268af
1 parent 42e7c4a
commit e4268af
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 15 deletions.
diff --git 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,24 +189,21 @@ 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 {
             lRewardThresholdWAD = aRewardThreshold * Constants.WAD / Constants.BP_SCALE;
         }
 
         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
@@ -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);