Skip to content

Commit

Permalink
feat: introduce return value for updatePrice to facilitate upkeep (#18
Browse files Browse the repository at this point in the history 
)

* feat: impl return value for `updatePrice`

* test: modify existing tests to test for new property

* test: modify existing tests to test for new property

* doc: update safety comment

* gas: update snapshot

* fix: add empty line

Co-authored-by: OliverNChalk <11343499+OliverNChalk@users.noreply.github.com>

* gas: update snapshot

---------

Co-authored-by: OliverNChalk <11343499+OliverNChalk@users.noreply.github.com>
  • Loading branch information
@xenide @OliverNChalk
xenide and OliverNChalk authored Sep 18, 2024
1 parent 42e7c4a commit b946c0c
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 58 deletions.
66 changes: 33 additions & 33 deletions .gas-snapshot
View file
Original file line number Diff line number Diff line change
@@ -1,42 +1,42 @@
QueryProcessorTest:testFindNearestSample_CanFindExactValue(uint32,uint256,uint256,uint256) (runs: 256, μ: 66334752, ~: 75124967)
QueryProcessorTest:testFindNearestSample_CanFindIntermediateValue(uint32,uint256,uint256,uint256) (runs: 256, μ: 65933151, ~: 75772005)
QueryProcessorTest:testFindNearestSample_CanFindExactValue(uint32,uint256,uint256,uint256) (runs: 256, μ: 65090324, ~: 74975370)
QueryProcessorTest:testFindNearestSample_CanFindIntermediateValue(uint32,uint256,uint256,uint256) (runs: 256, μ: 64771162, ~: 74583507)
QueryProcessorTest:testFindNearestSample_NotInitialized() (gas: 1056945756)
QueryProcessorTest:testFindNearestSample_OneSample(uint256) (runs: 256, μ: 80331, ~: 80360)
QueryProcessorTest:testFindNearestSample_OneSample(uint256) (runs: 256, μ: 80327, ~: 80360)
QueryProcessorTest:testGetInstantValue() (gas: 124248)
QueryProcessorTest:testGetInstantValue_NotInitialized(uint256) (runs: 256, μ: 19397, ~: 19397)
QueryProcessorTest:testGetInstantValue_NotInitialized_BeyondBufferSize(uint8,uint16) (runs: 256, μ: 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:testGetInstantValue_NotInitialized_BeyondBufferSize(uint8,uint16) (runs: 256, μ: 68389670, ~: 68389600)
QueryProcessorTest:testGetPastAccumulator_BufferEmpty(uint8) (runs: 256, μ: 27016, ~: 27087)
QueryProcessorTest:testGetPastAccumulator_ExactMatch(uint32,uint256,uint256,uint16) (runs: 256, μ: 70591497, ~: 80271384)
QueryProcessorTest:testGetPastAccumulator_ExactMatch_LatestAccumulator(uint32,uint256,uint256) (runs: 256, μ: 69164823, ~: 77928090)
QueryProcessorTest:testGetPastAccumulator_ExactMatch_OldestAccumulator(uint32,uint256,uint256) (runs: 256, μ: 69194817, ~: 77959690)
QueryProcessorTest:testGetPastAccumulator_ExtrapolatesBeyondLatest(uint32,uint256,uint256,uint256) (runs: 256, μ: 64744868, ~: 74555386)
QueryProcessorTest:testGetPastAccumulator_InterpolatesBetweenPastAccumulators(uint32,uint256,uint256,uint256) (runs: 256, μ: 64778894, ~: 74589515)
QueryProcessorTest:testGetPastAccumulator_InvalidAgo(uint32,uint256,uint256,uint256) (runs: 256, μ: 64736471, ~: 74546552)
QueryProcessorTest:testGetPastAccumulator_QueryTooOld(uint32,uint256,uint256,uint256) (runs: 256, μ: 64748015, ~: 74556435)
QueryProcessorTest:testGetTimeWeightedAverage(uint32,uint256,uint256,uint256,uint256) (runs: 256, μ: 102669133, ~: 112352230)
QueryProcessorTest:testGetTimeWeightedAverage_BadSecs() (gas: 10995)
ReservoirPriceOracleTest:testClearRoute() (gas: 52339)
ReservoirPriceOracleTest:testClearRoute_AllWordsCleared() (gas: 159879)
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, μ: 34016, ~: 34118)
ReservoirPriceOracleTest:testGetQuote(uint256,uint256) (runs: 256, μ: 34001, ~: 34118)
ReservoirPriceOracleTest:testGetQuote_AmountInTooLarge() (gas: 12963)
ReservoirPriceOracleTest:testGetQuote_BaseIsVault(uint256) (runs: 256, μ: 411295, ~: 411040)
ReservoirPriceOracleTest:testGetQuote_ComplicatedDecimals() (gas: 10354021)
ReservoirPriceOracleTest:testGetQuote_Inverse(uint256,uint256) (runs: 256, μ: 36154, ~: 36316)
ReservoirPriceOracleTest:testGetQuote_BaseIsVault(uint256) (runs: 256, μ: 411282, ~: 411040)
ReservoirPriceOracleTest:testGetQuote_ComplicatedDecimals() (gas: 10354017)
ReservoirPriceOracleTest:testGetQuote_Inverse(uint256,uint256) (runs: 256, μ: 36138, ~: 36254)
ReservoirPriceOracleTest:testGetQuote_MultipleHops() (gas: 111841)
ReservoirPriceOracleTest:testGetQuote_MultipleHops_Inverse() (gas: 112181)
ReservoirPriceOracleTest:testGetQuote_MultipleHops_Inverse() (gas: 112163)
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, μ: 5329116, ~: 5329104)
ReservoirPriceOracleTest:testGetQuote_RandomizeAllParam_2HopRoute(uint256,uint256,uint256,address,address,address,uint8,uint8,uint8) (runs: 256, μ: 10496298, ~: 10496410)
ReservoirPriceOracleTest:testGetQuote_RandomizeAllParam_1HopRoute(uint256,uint256,address,address,uint8,uint8) (runs: 256, μ: 5329100, ~: 5329104)
ReservoirPriceOracleTest:testGetQuote_RandomizeAllParam_2HopRoute(uint256,uint256,uint256,address,address,address,uint8,uint8,uint8) (runs: 256, μ: 10496290, ~: 10496408)
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, μ: 26527, ~: 26629)
ReservoirPriceOracleTest:testGetQuotes(uint256,uint256) (runs: 256, μ: 26512, ~: 26629)
ReservoirPriceOracleTest:testPriceCache_Inverted() (gas: 22001)
ReservoirPriceOracleTest:testSetFallbackOracle_NotOwner() (gas: 10938)
ReservoirPriceOracleTest:testSetRoute() (gas: 61093)
Expand All @@ -49,20 +49,20 @@ ReservoirPriceOracleTest:testSetRoute_OverwriteExisting() (gas: 169666)
ReservoirPriceOracleTest:testSetRoute_SameToken() (gas: 13041)
ReservoirPriceOracleTest:testUndesignatePair() (gas: 30256)
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: 15897381)
ReservoirPriceOracleTest:testUpdatePrice_PriceOutOfRange() (gas: 5353475)
ReservoirPriceOracleTest:testUpdatePrice_RewardEligible_ContractNoReceive() (gas: 153424)
ReservoirPriceOracleTest:testUpdatePrice_RewardEligible_InsufficientReward(uint256) (runs: 256, μ: 211585, ~: 211793)
ReservoirPriceOracleTest:testUpdatePrice_RewardEligible_ZeroRecipient() (gas: 144295)
ReservoirPriceOracleTest:testUpdatePrice_AboveThresholdBelowMaxReward(uint256) (runs: 256, μ: 165939, ~: 165959)
ReservoirPriceOracleTest:testUpdatePrice_BelowThreshold(uint256) (runs: 256, μ: 150797, ~: 150473)
ReservoirPriceOracleTest:testUpdatePrice_BeyondMaxReward(uint256) (runs: 256, μ: 163444, ~: 163469)
ReservoirPriceOracleTest:testUpdatePrice_FirstUpdate() (gas: 154418)
ReservoirPriceOracleTest:testUpdatePrice_IntermediateRoutes() (gas: 16111065)
ReservoirPriceOracleTest:testUpdatePrice_PriceOutOfRange() (gas: 5353482)
ReservoirPriceOracleTest:testUpdatePrice_RewardEligible_ContractNoReceive() (gas: 153582)
ReservoirPriceOracleTest:testUpdatePrice_RewardEligible_InsufficientReward(uint256) (runs: 256, μ: 212147, ~: 212363)
ReservoirPriceOracleTest:testUpdatePrice_RewardEligible_ZeroRecipient() (gas: 147250)
ReservoirPriceOracleTest:testUpdateRewardGasAmount() (gas: 19039)
ReservoirPriceOracleTest:testUpdateRewardGasAmount_NotOwner() (gas: 10940)
ReservoirPriceOracleTest:testUpdateTwapPeriod(uint256) (runs: 256, μ: 21693, ~: 21778)
ReservoirPriceOracleTest:testUpdateTwapPeriod_InvalidTwapPeriod(uint256) (runs: 256, μ: 17830, ~: 18120)
ReservoirPriceOracleTest:testWritePriceCache(uint256) (runs: 256, μ: 30232, ~: 29977)
ReservoirPriceOracleTest:testUpdateTwapPeriod(uint256) (runs: 256, μ: 21687, ~: 21778)
ReservoirPriceOracleTest:testUpdateTwapPeriod_InvalidTwapPeriod(uint256) (runs: 256, μ: 17825, ~: 18120)
ReservoirPriceOracleTest:testWritePriceCache(uint256) (runs: 256, μ: 30219, ~: 29977)
RoutesLibTest:testGetDecimalDifference() (gas: 3974)
RoutesLibTest:testIsCompositeRoute() (gas: 4341)
RoutesLibTest:testPackSimplePrice(int8,uint256) (runs: 256, μ: 8200, ~: 7962)
Expand Down
41 changes: 28 additions & 13 deletions src/ReservoirPriceOracle.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) =
Expand All @@ -154,12 +160,18 @@ 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);
_rewardUpdater(lPrevPrice, lNewPrice, aRewardRecipient, lRewardThreshold);
// SAFETY: This will not overflow even if reward gas amount is set to the block gas limit (30M at time if writing),
// and hops are limited by `MAX_ROUTE_LENGTH`.
unchecked {
rTotalReward += _calculateReward(lPrevPrice, lNewPrice, lRewardThreshold);
}
}

_rewardUpdater(aRewardRecipient, rTotalReward);
}

///////////////////////////////////////////////////////////////////////////////////////////////
Expand All @@ -182,24 +194,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) {
Expand All @@ -209,18 +218,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) || aReward == 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))
}
}

Expand Down Expand Up @@ -490,7 +503,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") {
Expand Down
Loading

0 comments on commit b946c0c

Please sign in to comment.