Skip to content

Commit b036036

Browse files
xenidexenide
committed
feat: expose bpDiffForMaxReward in public function
1 parent cf88644 commit b036036

File tree

3 files changed

+107
-59
lines changed

3 files changed

+107
-59
lines changed

‎src/ReservoirPriceOracle.sol

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar
2020
using FixedPointMathLib for uint256;
2121
using LibSort for address[];
2222
using RoutesLib for bytes32;
23+
using Utils for uint256;
2324
using QueryProcessor for ReservoirPair;
2425

2526
///////////////////////////////////////////////////////////////////////////////////////////////
@@ -107,8 +108,13 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar
107108
/// @param aToken1 Address of the higher token.
108109
/// @return rPrice The cached price of aToken0/aToken1 for simple routes. Returns 0 for prices of composite routes.
109110
/// @return rDecimalDiff The difference in decimals as defined by aToken1.decimals() - aToken0.decimals(). Only valid for simple routes.
110-
function priceCache(address aToken0, address aToken1) external view returns (uint256 rPrice, int256 rDecimalDiff) {
111-
(rPrice, rDecimalDiff) = _priceCache(aToken0, aToken1);
111+
/// @return rBpDiffForMaxReward The number of basis points at and beyond which the bounty payout for a price update is maximum.
112+
function priceCache(address aToken0, address aToken1)
113+
external
114+
view
115+
returns (uint256 rPrice, int256 rDecimalDiff, uint256 rBpDiffForMaxReward)
116+
{
117+
(rPrice, rDecimalDiff, rBpDiffForMaxReward) = _priceCache(aToken0, aToken1);
112118
}
113119

114120
/// @notice Updates the TWAP price for all simple routes between `aTokenA` and `aTokenB`. Will also update intermediate routes if the route defined between
@@ -141,7 +147,7 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar
141147

142148
// if it's a simple route, we avoid loading the price again from storage
143149
if (lRoute.length != 2) {
144-
(lPrevPrice,) = _priceCache(lToken0, lToken1);
150+
(lPrevPrice,,) = _priceCache(lToken0, lToken1);
145151
}
146152

147153
_writePriceCache(lToken0, lToken1, lNewPrice);
@@ -169,29 +175,18 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar
169175
rResult = lPair.getTimeWeightedAverage(aQuery.priceType, aQuery.secs, aQuery.ago, lIndex);
170176
}
171177

172-
function _calcPercentageDiff(uint256 aOriginal, uint256 aNew) private pure returns (uint256) {
173-
unchecked {
174-
if (aOriginal == 0) return 0;
175-
176-
// multiplication does not overflow as `aOriginal` and `aNew` is always less than or
177-
// equal to `Constants.MAX_SUPPORTED_PRICE`, as checked in `_writePriceCache`
178-
if (aOriginal > aNew) {
179-
return (aOriginal - aNew) * 1e18 / aOriginal;
180-
} else {
181-
return (aNew - aOriginal) * 1e18 / aOriginal;
182-
}
183-
}
184-
}
185-
186178
function _rewardUpdater(uint256 aPrevPrice, uint256 aNewPrice, address aRecipient, uint256 aBpDiffForMaxReward)
187179
private
188180
{
189-
uint256 lPercentDiff = _calcPercentageDiff(aPrevPrice, aNewPrice);
181+
uint256 lPercentDiff = aPrevPrice.calcPercentageDiff(aNewPrice);
190182
if (lPercentDiff == 0) return;
191183
if (aRecipient == address(0)) return;
192184

193-
// can be unchecked
194-
uint256 lBpForMaxRewardWAD = aBpDiffForMaxReward * Constants.WAD / Constants.BP_SCALE;
185+
// SAFETY: this mul will not overflow as `aBpDiffForMaxReward` is capped by `Constants.BP_SCALE`
186+
uint256 lBpForMaxRewardWAD;
187+
unchecked {
188+
lBpForMaxRewardWAD = aBpDiffForMaxReward * Constants.WAD / Constants.BP_SCALE;
189+
}
195190
uint256 lReward = lPercentDiff > lBpForMaxRewardWAD ? lBpForMaxRewardWAD : lPercentDiff;
196191

197192
// N.B. Revisit this whenever deployment on a new chain is needed
@@ -287,8 +282,12 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar
287282
}
288283
}
289284

290-
// performs an SLOAD to load 1 word which contains the simple price and decimal difference
291-
function _priceCache(address aToken0, address aToken1) private view returns (uint256 rPrice, int256 rDecimalDiff) {
285+
// performs an SLOAD to load 1 word which contains the simple price, decimal difference, and number of basis points for maximum reward
286+
function _priceCache(address aToken0, address aToken1)
287+
private
288+
view
289+
returns (uint256 rPrice, int256 rDecimalDiff, uint256 rBpDiffForMaxReward)
290+
{
292291
bytes32 lSlot = Utils.calculateSlot(aToken0, aToken1);
293292

294293
bytes32 lData;
@@ -298,6 +297,7 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar
298297
if (lData.isSimplePrice()) {
299298
rPrice = lData.getPrice();
300299
rDecimalDiff = lData.getDecimalDifference();
300+
rBpDiffForMaxReward = lData.getBpDiffForMaxReward();
301301
}
302302
}
303303

@@ -357,7 +357,7 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar
357357
for (uint256 i = 0; i < lRoute.length - 1; ++i) {
358358
(lToken0, lToken1) = Utils.sortTokens(lRoute[i], lRoute[i + 1]);
359359
// it is assumed that intermediate routes defined here are simple routes and not composite routes
360-
(lPrice, lDecimalDiff) = _priceCache(lToken0, lToken1);
360+
(lPrice, lDecimalDiff,) = _priceCache(lToken0, lToken1);
361361

362362
if (lPrice == 0) revert OracleErrors.PriceZero();
363363
lIntermediateAmount = _calcAmtOut(lIntermediateAmount, lPrice, lDecimalDiff, lRoute[i] != lToken0);

‎src/libraries/Utils.sol

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,4 +11,18 @@ library Utils {
1111
function calculateSlot(address aToken0, address aToken1) internal pure returns (bytes32) {
1212
return keccak256(abi.encode(aToken0, aToken1));
1313
}
14+
15+
/// @dev Assumes that `aOriginal` and `aNew` is less than or equal to
16+
/// `Constants.MAX_SUPPORTED_PRICE`. So multiplication by 1e18 will not overflow.
17+
function calcPercentageDiff(uint256 aOriginal, uint256 aNew) internal pure returns (uint256) {
18+
unchecked {
19+
if (aOriginal == 0) return 0;
20+
21+
if (aOriginal > aNew) {
22+
return (aOriginal - aNew) * 1e18 / aOriginal;
23+
} else {
24+
return (aNew - aOriginal) * 1e18 / aOriginal;
25+
}
26+
}
27+
}
1428
}

‎test/unit/ReservoirPriceOracle.t.sol

Lines changed: 70 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ contract ReservoirPriceOracleTest is BaseTest {
4444
// writes the cached prices, for easy testing
4545
function _writePriceCache(address aToken0, address aToken1, uint256 aPrice) internal {
4646
require(aToken0 < aToken1, "tokens unsorted");
47-
require(bytes32(aPrice) & bytes2(0xffff) == 0, "PRICE WILL OVERLAP FLAG");
47+
require(aPrice <= Constants.MAX_SUPPORTED_PRICE, "price too large");
4848

4949
vm.record();
5050
_oracle.priceCache(aToken0, aToken1);
@@ -98,7 +98,7 @@ contract ReservoirPriceOracleTest is BaseTest {
9898
_writePriceCache(address(_tokenB), address(_tokenC), lPrice);
9999

100100
// assert
101-
(uint256 lQueriedPrice,) = _oracle.priceCache(address(_tokenB), address(_tokenC));
101+
(uint256 lQueriedPrice,,) = _oracle.priceCache(address(_tokenB), address(_tokenC));
102102
assertEq(lQueriedPrice, lPrice);
103103
}
104104

@@ -124,7 +124,7 @@ contract ReservoirPriceOracleTest is BaseTest {
124124

125125
// arrange
126126
_writePriceCache(address(_tokenA), address(_tokenB), lPrice);
127-
(uint256 lQueriedPrice,) = _oracle.priceCache(address(_tokenA), address(_tokenB));
127+
(uint256 lQueriedPrice,,) = _oracle.priceCache(address(_tokenA), address(_tokenB));
128128
assertEq(lQueriedPrice, lPrice);
129129

130130
// act
@@ -219,11 +219,11 @@ contract ReservoirPriceOracleTest is BaseTest {
219219
lRoute[2] = address(lTokenC);
220220

221221
{
222-
uint16[] memory lBpDiffForMaxReward = new uint16[](2);
223-
lBpDiffForMaxReward[0] = lBpDiffForMaxReward[1] = Constants.BP_SCALE;
224-
_oracle.setRoute(address(lTokenA), address(lTokenC), lRoute, lBpDiffForMaxReward);
225-
_writePriceCache(address(lTokenA), address(lTokenB), 1e18);
226-
_writePriceCache(address(lTokenC), address(lTokenB), 1e18);
222+
uint16[] memory lBpDiffForMaxReward = new uint16[](2);
223+
lBpDiffForMaxReward[0] = lBpDiffForMaxReward[1] = Constants.BP_SCALE;
224+
_oracle.setRoute(address(lTokenA), address(lTokenC), lRoute, lBpDiffForMaxReward);
225+
_writePriceCache(address(lTokenA), address(lTokenB), 1e18);
226+
_writePriceCache(address(lTokenC), address(lTokenB), 1e18);
227227
}
228228

229229
// act
@@ -415,6 +415,17 @@ contract ReservoirPriceOracleTest is BaseTest {
415415
assertEq(lAmtOut / 1e12, lAmtIn * lRate / 1e18);
416416
}
417417

418+
function testPriceCache_Inverted() external {
419+
// arrange
420+
_writePriceCache(address(_tokenA), address(_tokenB), 1e18);
421+
422+
// act
423+
(uint256 lPrice,,) = _oracle.priceCache(address(_tokenB), address(_tokenA));
424+
425+
// assert
426+
assertEq(lPrice, 0);
427+
}
428+
418429
function testUpdateTwapPeriod(uint256 aNewPeriod) external {
419430
// assume
420431
uint64 lNewPeriod = uint64(bound(aNewPeriod, 1, 1 hours));
@@ -439,9 +450,9 @@ contract ReservoirPriceOracleTest is BaseTest {
439450
assertEq(_oracle.rewardGasAmount(), lNewRewardMultiplier);
440451
}
441452

442-
function testUpdatePrice_FirstUpdate() external {
453+
function testUpdatePrice_FirstUpdate() public {
443454
// sanity
444-
(uint256 lPrice,) = _oracle.priceCache(address(_tokenA), address(_tokenB));
455+
(uint256 lPrice,,) = _oracle.priceCache(address(_tokenA), address(_tokenB));
445456
assertEq(lPrice, 0);
446457

447458
// arrange
@@ -450,23 +461,41 @@ contract ReservoirPriceOracleTest is BaseTest {
450461
skip(1);
451462
_pair.sync();
452463
skip(_oracle.twapPeriod() * 2);
453-
_tokenA.mint(address(_pair), 2e18);
454-
_pair.swap(2e18, true, address(this), "");
464+
_pair.sync();
455465

456466
// act
457467
_oracle.updatePrice(address(_tokenB), address(_tokenA), address(this));
458468

459469
// assert
460-
(lPrice,) = _oracle.priceCache(address(_tokenA), address(_tokenB));
470+
(lPrice,,) = _oracle.priceCache(address(_tokenA), address(_tokenB));
461471
assertEq(lPrice, 98_918_868_099_219_913_512);
462-
(lPrice,) = _oracle.priceCache(address(_tokenB), address(_tokenA));
472+
(lPrice,,) = _oracle.priceCache(address(_tokenB), address(_tokenA));
463473
assertEq(lPrice, 0);
464474
assertEq(address(this).balance, 0); // there should be no reward for the first price update
465475
}
466476

467-
function testUpdatePrice_WithinThreshold() external {
477+
function testUpdatePrice_NoPriceChange() external {
468478
// arrange
469-
_writePriceCache(address(_tokenA), address(_tokenB), 98.9223e18);
479+
testUpdatePrice_FirstUpdate();
480+
uint256 lExpectedPrice = 98_918_868_099_219_913_512;
481+
(uint256 lPrice,,) = _oracle.priceCache(address(_tokenA), address(_tokenB));
482+
assertEq(lPrice, lExpectedPrice); // ensure that there is a price to begin with
483+
skip(_oracle.twapPeriod() * 2);
484+
_pair.sync();
485+
486+
// act
487+
_oracle.updatePrice(address(_tokenA), address(_tokenB), address(this));
488+
489+
// assert
490+
(lPrice,,) = _oracle.priceCache(address(_tokenA), address(_tokenB));
491+
assertEq(lPrice, lExpectedPrice);
492+
assertEq(address(this).balance, 0); // no reward as the price did not change at all
493+
}
494+
495+
function testUpdatePrice_BelowMaxReward() external {
496+
// arrange
497+
uint256 lOriginalPrice = 98.9223e18;
498+
_writePriceCache(address(_tokenA), address(_tokenB), lOriginalPrice);
470499
deal(address(_oracle), 1 ether);
471500

472501
skip(1);
@@ -479,14 +508,14 @@ contract ReservoirPriceOracleTest is BaseTest {
479508
_oracle.updatePrice(address(_tokenB), address(_tokenA), address(this));
480509

481510
// assert
482-
(uint256 lPrice,) = _oracle.priceCache(address(_tokenA), address(_tokenB));
511+
(uint256 lPrice,,) = _oracle.priceCache(address(_tokenA), address(_tokenB));
483512
assertEq(lPrice, 98_918_868_099_219_913_512);
484-
(lPrice,) = _oracle.priceCache(address(_tokenB), address(_tokenA));
485-
assertEq(lPrice, 0);
486-
assertEq(address(this).balance, 0); // no reward since price is within threshold
513+
uint256 lExpectedRewardReceived =
514+
block.basefee * _oracle.rewardGasAmount() * lOriginalPrice.calcPercentageDiff(lPrice) / 1e18;
515+
assertEq(address(this).balance, lExpectedRewardReceived); // some reward received but is less than max possible reward
487516
}
488517

489-
function testUpdatePrice_BeyondThreshold() external {
518+
function testUpdatePrice_BeyondMaxReward() external {
490519
// arrange
491520
_writePriceCache(address(_tokenA), address(_tokenB), 5e18);
492521
deal(address(_oracle), 1 ether);
@@ -501,15 +530,16 @@ contract ReservoirPriceOracleTest is BaseTest {
501530
_oracle.updatePrice(address(_tokenB), address(_tokenA), address(this));
502531

503532
// assert
504-
(uint256 lPrice,) = _oracle.priceCache(address(_tokenA), address(_tokenB));
533+
(uint256 lPrice,,) = _oracle.priceCache(address(_tokenA), address(_tokenB));
505534
assertEq(lPrice, 98_918_868_099_219_913_512);
506-
(lPrice,) = _oracle.priceCache(address(_tokenB), address(_tokenA));
535+
(lPrice,,) = _oracle.priceCache(address(_tokenB), address(_tokenA));
507536
assertEq(lPrice, 0);
508-
assertEq(address(this).balance, block.basefee * _oracle.rewardGasAmount());
509-
assertEq(address(_oracle).balance, 1 ether - block.basefee * _oracle.rewardGasAmount());
537+
uint256 lExpectedRewardReceived = block.basefee * _oracle.rewardGasAmount();
538+
assertEq(address(this).balance, lExpectedRewardReceived);
539+
assertEq(address(_oracle).balance, 1 ether - lExpectedRewardReceived);
510540
}
511541

512-
function testUpdatePrice_BeyondThreshold_InsufficientReward(uint256 aRewardAvailable) external {
542+
function testUpdatePrice_RewardEligible_InsufficientReward(uint256 aRewardAvailable) external {
513543
// assume
514544
uint256 lRewardAvailable = bound(aRewardAvailable, 1, block.basefee * _oracle.rewardGasAmount() - 1);
515545

@@ -526,11 +556,13 @@ contract ReservoirPriceOracleTest is BaseTest {
526556
// act
527557
_oracle.updatePrice(address(_tokenA), address(_tokenB), address(this));
528558

529-
// assert
530-
assertEq(address(this).balance, 0); // no reward as there's insufficient ether in the contract
559+
// assert - no reward as there's insufficient ether in the contract, but price cache updated nonetheless
560+
(uint256 lPrice,,) = _oracle.priceCache(address(_tokenA), address(_tokenB));
561+
assertNotEq(lPrice, 5e18);
562+
assertEq(address(this).balance, 0);
531563
}
532564

533-
function testUpdatePrice_BeyondThreshold_ZeroRecipient() external {
565+
function testUpdatePrice_RewardEligible_ZeroRecipient() external {
534566
// arrange
535567
uint256 lBalance = 10 ether;
536568
deal(address(_oracle), lBalance);
@@ -545,7 +577,9 @@ contract ReservoirPriceOracleTest is BaseTest {
545577
// act
546578
_oracle.updatePrice(address(_tokenA), address(_tokenB), address(0));
547579

548-
// assert - no change to balance
580+
// assert - no change to balance, but price cache updated nonetheless
581+
(uint256 lPrice,,) = _oracle.priceCache(address(_tokenA), address(_tokenB));
582+
assertNotEq(lPrice, 5e18);
549583
assertEq(address(_oracle).balance, lBalance);
550584
}
551585

@@ -595,10 +629,10 @@ contract ReservoirPriceOracleTest is BaseTest {
595629
_oracle.updatePrice(address(_tokenA), address(_tokenB), address(this));
596630

597631
// assert
598-
(uint256 lPriceAC,) = _oracle.priceCache(lStart, lIntermediate1);
599-
(uint256 lPriceCD,) = _oracle.priceCache(lIntermediate1, lIntermediate2);
600-
(uint256 lPriceBD,) = _oracle.priceCache(lEnd, lIntermediate2);
601-
(uint256 lPriceAB,) = _oracle.priceCache(lStart, lEnd);
632+
(uint256 lPriceAC,,) = _oracle.priceCache(lStart, lIntermediate1);
633+
(uint256 lPriceCD,,) = _oracle.priceCache(lIntermediate1, lIntermediate2);
634+
(uint256 lPriceBD,,) = _oracle.priceCache(lEnd, lIntermediate2);
635+
(uint256 lPriceAB,,) = _oracle.priceCache(lStart, lEnd);
602636
assertApproxEqRel(lPriceAC, 0.5e18, 0.0001e18);
603637
assertApproxEqRel(lPriceCD, 2e18, 0.0001e18);
604638
assertApproxEqRel(lPriceBD, 2e18, 0.0001e18);
@@ -623,7 +657,7 @@ contract ReservoirPriceOracleTest is BaseTest {
623657
// assert
624658
address[] memory lQueriedRoute = _oracle.route(lToken0, lToken1);
625659
assertEq(lQueriedRoute, lRoute);
626-
(, int256 lDecimalDiff) = _oracle.priceCache(lToken0, lToken1);
660+
(, int256 lDecimalDiff,) = _oracle.priceCache(lToken0, lToken1);
627661
int256 lActualDiff = int256(uint256(IERC20(lToken1).decimals())) - int256(uint256(IERC20(lToken0).decimals()));
628662
assertEq(lDecimalDiff, lActualDiff);
629663
}
@@ -720,7 +754,7 @@ contract ReservoirPriceOracleTest is BaseTest {
720754
// assert
721755
lQueriedRoute = _oracle.route(lToken0, lToken1);
722756
assertEq(lQueriedRoute, new address[](0));
723-
(uint256 lPrice,) = _oracle.priceCache(lToken0, lToken1);
757+
(uint256 lPrice,,) = _oracle.priceCache(lToken0, lToken1);
724758
assertEq(lPrice, 0);
725759
}
726760

0 commit comments

Comments
 (0)