Skip to content

Commit

Permalink
feat: expose bpDiffForMaxReward in public function
Browse files Browse the repository at this point in the history
  • Loading branch information
@xenide
xenide committed Jul 27, 2024
1 parent cf88644 commit b036036
Show file tree
Hide file tree
Showing 3 changed files with 107 additions and 59 deletions.
46 changes: 23 additions & 23 deletions src/ReservoirPriceOracle.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

///////////////////////////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -298,6 +297,7 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar
if (lData.isSimplePrice()) {
rPrice = lData.getPrice();
rDecimalDiff = lData.getDecimalDifference();
rBpDiffForMaxReward = lData.getBpDiffForMaxReward();
}
}

Expand Down Expand Up @@ -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);
Expand Down
14 changes: 14 additions & 0 deletions src/libraries/Utils.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
}
}
106 changes: 70 additions & 36 deletions test/unit/ReservoirPriceOracle.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}

Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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));
Expand All @@ -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
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);

Expand All @@ -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);
Expand All @@ -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);
}

Expand Down Expand Up @@ -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);
Expand All @@ -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);
}
Expand Down Expand Up @@ -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);
}

Expand Down

0 comments on commit b036036

Please sign in to comment.