Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

questions #15

Merged
merged 10 commits into from
Jul 8, 2024
Merged

questions #15

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
8f60414
questions
OliverNChalk Jul 7, 2024
2a020c9
fix: make sure ETH transfer call never reverts
xenide Jul 7, 2024
2377530
fix: all assembly blocks memory safe
xenide Jul 7, 2024
f13724a
docs: update comment
xenide Jul 7, 2024
042fe39
docs: update comment
xenide Jul 7, 2024
95bad55
fix: use pop in after low level call to clear stack
xenide Jul 8, 2024
c6f4277
lint: add solhint disable
xenide Jul 8, 2024
7548258
fix: rm review comment
xenide Jul 8, 2024
12b0e73
fix: rm test case that wastes all gas
xenide Jul 8, 2024
b880ee8
gas: update snapshot
xenide Jul 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 42 additions & 41 deletions .gas-snapshot
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,71 +1,72 @@
QueryProcessorTest:testFindNearestSample_CanFindExactValue(uint32,uint256,uint256,uint256) (runs: 256, μ: 67585746, ~: 76007063)
QueryProcessorTest:testFindNearestSample_CanFindIntermediateValue(uint32,uint256,uint256,uint256) (runs: 256, μ: 67051023, ~: 76087612)
QueryProcessorTest:testFindNearestSample_CanFindExactValue(uint32,uint256,uint256,uint256) (runs: 256, μ: 66457845, ~: 74935508)
QueryProcessorTest:testFindNearestSample_CanFindIntermediateValue(uint32,uint256,uint256,uint256) (runs: 256, μ: 64579864, ~: 74273370)
QueryProcessorTest:testFindNearestSample_NotInitialized() (gas: 1056945756)
QueryProcessorTest:testFindNearestSample_OneSample(uint256) (runs: 256, μ: 80325, ~: 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, μ: 68389647, ~: 68389600)
QueryProcessorTest:testGetPastAccumulator_BufferEmpty(uint8) (runs: 256, μ: 27017, ~: 27087)
QueryProcessorTest:testGetPastAccumulator_ExactMatch(uint32,uint256,uint256,uint16) (runs: 256, μ: 69289018, ~: 79307245)
QueryProcessorTest:testGetPastAccumulator_ExactMatch_LatestAccumulator(uint32,uint256,uint256) (runs: 256, μ: 70900090, ~: 79224986)
QueryProcessorTest:testGetPastAccumulator_ExactMatch_OldestAccumulator(uint32,uint256,uint256) (runs: 256, μ: 70930212, ~: 79256586)
QueryProcessorTest:testGetPastAccumulator_ExtrapolatesBeyondLatest(uint32,uint256,uint256,uint256) (runs: 256, μ: 67024575, ~: 76058923)
QueryProcessorTest:testGetPastAccumulator_InterpolatesBetweenPastAccumulators(uint32,uint256,uint256,uint256) (runs: 256, μ: 67058609, ~: 76093620)
QueryProcessorTest:testGetPastAccumulator_InvalidAgo(uint32,uint256,uint256,uint256) (runs: 256, μ: 67016184, ~: 76050634)
QueryProcessorTest:testGetPastAccumulator_QueryTooOld(uint32,uint256,uint256,uint256) (runs: 256, μ: 67027596, ~: 76060517)
QueryProcessorTest:testGetTimeWeightedAverage(uint32,uint256,uint256,uint256,uint256) (runs: 256, μ: 101761121, ~: 108597954)
QueryProcessorTest:testGetInstantValue_NotInitialized_BeyondBufferSize(uint8,uint16) (runs: 256, μ: 68389666, ~: 68389600)
QueryProcessorTest:testGetPastAccumulator_BufferEmpty(uint8) (runs: 256, μ: 27024, ~: 27087)
QueryProcessorTest:testGetPastAccumulator_ExactMatch(uint32,uint256,uint256,uint16) (runs: 256, μ: 71545320, ~: 80987339)
QueryProcessorTest:testGetPastAccumulator_ExactMatch_LatestAccumulator(uint32,uint256,uint256) (runs: 256, μ: 67835200, ~: 79351610)
QueryProcessorTest:testGetPastAccumulator_ExactMatch_OldestAccumulator(uint32,uint256,uint256) (runs: 256, μ: 67865042, ~: 79383210)
QueryProcessorTest:testGetPastAccumulator_ExtrapolatesBeyondLatest(uint32,uint256,uint256,uint256) (runs: 256, μ: 64553572, ~: 74245223)
QueryProcessorTest:testGetPastAccumulator_InterpolatesBetweenPastAccumulators(uint32,uint256,uint256,uint256) (runs: 256, μ: 64587596, ~: 74279378)
QueryProcessorTest:testGetPastAccumulator_InvalidAgo(uint32,uint256,uint256,uint256) (runs: 256, μ: 64545179, ~: 74236389)
QueryProcessorTest:testGetPastAccumulator_QueryTooOld(uint32,uint256,uint256,uint256) (runs: 256, μ: 64556733, ~: 74246272)
QueryProcessorTest:testGetTimeWeightedAverage(uint32,uint256,uint256,uint256,uint256) (runs: 256, μ: 106580968, ~: 116745041)
QueryProcessorTest:testGetTimeWeightedAverage_BadSecs() (gas: 10995)
ReservoirPriceOracleTest:testClearRoute() (gas: 50974)
ReservoirPriceOracleTest:testClearRoute() (gas: 50956)
ReservoirPriceOracleTest:testClearRoute_AllWordsCleared() (gas: 151907)
ReservoirPriceOracleTest:testDesignatePair() (gas: 29091)
ReservoirPriceOracleTest:testDesignatePair_IncorrectPair() (gas: 21111)
ReservoirPriceOracleTest:testDesignatePair_NotOwner() (gas: 17531)
ReservoirPriceOracleTest:testDesignatePair_IncorrectPair() (gas: 21178)
ReservoirPriceOracleTest:testDesignatePair_NotOwner() (gas: 17509)
ReservoirPriceOracleTest:testDesignatePair_TokenOrderReversed() (gas: 30729)
ReservoirPriceOracleTest:testGasBountyAvailable(uint256) (runs: 256, μ: 9929, ~: 9925)
ReservoirPriceOracleTest:testGasBountyAvailable_Zero() (gas: 8961)
ReservoirPriceOracleTest:testGetLatest(uint32) (runs: 256, μ: 92685, ~: 92614)
ReservoirPriceOracleTest:testGetLatest_Inverted() (gas: 96786)
ReservoirPriceOracleTest:testGetQuote(uint256,uint256) (runs: 256, μ: 35198, ~: 35302)
ReservoirPriceOracleTest:testGetQuote_AmountInTooLarge() (gas: 12963)
ReservoirPriceOracleTest:testGetQuote_BaseIsVault(uint256) (runs: 256, μ: 417430, ~: 417188)
ReservoirPriceOracleTest:testGasBountyAvailable_Zero() (gas: 8939)
ReservoirPriceOracleTest:testGetLatest(uint32) (runs: 256, μ: 92749, ~: 92680)
ReservoirPriceOracleTest:testGetLatest_Inverted() (gas: 96764)
ReservoirPriceOracleTest:testGetQuote(uint256,uint256) (runs: 256, μ: 35165, ~: 35280)
ReservoirPriceOracleTest:testGetQuote_AmountInTooLarge() (gas: 13030)
ReservoirPriceOracleTest:testGetQuote_BaseIsVault(uint256) (runs: 256, μ: 417433, ~: 417188)
ReservoirPriceOracleTest:testGetQuote_ComplicatedDecimals() (gas: 10350840)
ReservoirPriceOracleTest:testGetQuote_Inverse(uint256,uint256) (runs: 256, μ: 37311, ~: 37474)
ReservoirPriceOracleTest:testGetQuote_Inverse(uint256,uint256) (runs: 256, μ: 37278, ~: 37451)
ReservoirPriceOracleTest:testGetQuote_MultipleHops() (gas: 113391)
ReservoirPriceOracleTest:testGetQuote_MultipleHops_Inverse() (gas: 113646)
ReservoirPriceOracleTest:testGetQuote_MultipleHops_PriceZero() (gas: 125259)
ReservoirPriceOracleTest:testGetQuote_NoFallbackOracle() (gas: 20875)
ReservoirPriceOracleTest:testGetQuote_PriceZero() (gas: 15902)
ReservoirPriceOracleTest:testGetQuote_RandomizeAllParam_1HopRoute(uint256,uint256,address,address,uint8,uint8) (runs: 256, μ: 5327381, ~: 5327411)
ReservoirPriceOracleTest:testGetQuote_RandomizeAllParam_2HopRoute(uint256,uint256,uint256,address,address,address,uint8,uint8,uint8) (runs: 256, μ: 10492723, ~: 10492862)
ReservoirPriceOracleTest:testGetQuote_SameBaseQuote(uint256,address) (runs: 256, μ: 8963, ~: 8963)
ReservoirPriceOracleTest:testGetQuote_PriceZero() (gas: 15968)
ReservoirPriceOracleTest:testGetQuote_RandomizeAllParam_1HopRoute(uint256,uint256,address,address,uint8,uint8) (runs: 256, μ: 5327383, ~: 5327411)
ReservoirPriceOracleTest:testGetQuote_RandomizeAllParam_2HopRoute(uint256,uint256,uint256,address,address,address,uint8,uint8,uint8) (runs: 256, μ: 10492872, ~: 10492874)
ReservoirPriceOracleTest:testGetQuote_SameBaseQuote(uint256,address) (runs: 256, μ: 9030, ~: 9030)
ReservoirPriceOracleTest:testGetQuote_UseFallback() (gas: 38312)
ReservoirPriceOracleTest:testGetQuote_ZeroIn() (gas: 38147)
ReservoirPriceOracleTest:testGetQuotes(uint256,uint256) (runs: 256, μ: 32687, ~: 32791)
ReservoirPriceOracleTest:testGetQuotes(uint256,uint256) (runs: 256, μ: 32676, ~: 32791)
ReservoirPriceOracleTest:testGetTimeWeightedAverage() (gas: 141765)
ReservoirPriceOracleTest:testGetTimeWeightedAverage_Inverted() (gas: 120958)
ReservoirPriceOracleTest:testSetFallbackOracle_NotOwner() (gas: 10938)
ReservoirPriceOracleTest:testSetRoute() (gas: 58216)
ReservoirPriceOracleTest:testSetRoute_InvalidRoute() (gas: 17990)
ReservoirPriceOracleTest:testSetRoute_InvalidRouteLength() (gas: 17619)
ReservoirPriceOracleTest:testSetRoute_MultipleHops() (gas: 193395)
ReservoirPriceOracleTest:testSetFallbackOracle_NotOwner() (gas: 11003)
ReservoirPriceOracleTest:testSetRoute() (gas: 58194)
ReservoirPriceOracleTest:testSetRoute_InvalidRoute() (gas: 17968)
ReservoirPriceOracleTest:testSetRoute_InvalidRouteLength() (gas: 17597)
ReservoirPriceOracleTest:testSetRoute_MultipleHops() (gas: 193373)
ReservoirPriceOracleTest:testSetRoute_NotSorted() (gas: 12102)
ReservoirPriceOracleTest:testSetRoute_OverwriteExisting() (gas: 160967)
ReservoirPriceOracleTest:testSetRoute_SameToken() (gas: 12115)
ReservoirPriceOracleTest:testSetRoute_OverwriteExisting() (gas: 160945)
ReservoirPriceOracleTest:testSetRoute_SameToken() (gas: 12093)
ReservoirPriceOracleTest:testUndesignatePair() (gas: 30257)
ReservoirPriceOracleTest:testUndesignatePair_NotOwner() (gas: 15288)
ReservoirPriceOracleTest:testUpdatePriceDeviationThreshold(uint256) (runs: 256, μ: 21328, ~: 21085)
ReservoirPriceOracleTest:testUpdatePrice_BeyondThreshold() (gas: 213770)
ReservoirPriceOracleTest:testUpdatePrice_BeyondThreshold_InsufficientReward(uint256) (runs: 256, μ: 202989, ~: 203067)
ReservoirPriceOracleTest:testUpdatePriceDeviationThreshold(uint256) (runs: 256, μ: 21331, ~: 21085)
ReservoirPriceOracleTest:testUpdatePrice_BeyondThreshold() (gas: 213736)
ReservoirPriceOracleTest:testUpdatePrice_BeyondThreshold_InsufficientReward(uint256) (runs: 256, μ: 209788, ~: 210004)
ReservoirPriceOracleTest:testUpdatePrice_BeyondThreshold_ZeroRecipient() (gas: 195593)
ReservoirPriceOracleTest:testUpdatePrice_FirstUpdate() (gas: 203220)
ReservoirPriceOracleTest:testUpdatePrice_IntermediateRoutes() (gas: 15867847)
ReservoirPriceOracleTest:testUpdatePrice_PriceOutOfRange() (gas: 5350717)
ReservoirPriceOracleTest:testUpdatePrice_WithinThreshold() (gas: 204109)
ReservoirPriceOracleTest:testUpdatePrice_RecipientOutOfGas() (gas: 1040441212)
ReservoirPriceOracleTest:testUpdatePrice_WithinThreshold() (gas: 204087)
ReservoirPriceOracleTest:testUpdateRewardGasAmount() (gas: 19033)
ReservoirPriceOracleTest:testUpdateRewardGasAmount_NotOwner() (gas: 10984)
ReservoirPriceOracleTest:testUpdateTwapPeriod(uint256) (runs: 256, μ: 21745, ~: 21828)
ReservoirPriceOracleTest:testUpdateTwapPeriod(uint256) (runs: 256, μ: 21748, ~: 21828)
ReservoirPriceOracleTest:testUpdateTwapPeriod_InvalidTwapPeriod(uint256) (runs: 256, μ: 17861, ~: 18164)
ReservoirPriceOracleTest:testWritePriceCache(uint256) (runs: 256, μ: 29939, ~: 29697)
ReservoirPriceOracleTest:testWritePriceCache(uint256) (runs: 256, μ: 29920, ~: 29675)
RoutesLibTest:testGetDecimalDifference() (gas: 3974)
RoutesLibTest:testIsCompositeRoute() (gas: 4341)
RoutesLibTest:testPackSimplePrice(int8,uint256) (runs: 256, μ: 7786, ~: 7555)
Expand Down
39 changes: 19 additions & 20 deletions src/ReservoirPriceOracle.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,7 @@
import { IERC4626 } from "forge-std/interfaces/IERC4626.sol";

import { OracleErrors } from "src/libraries/OracleErrors.sol";
import {
IReservoirPriceOracle,
OracleAverageQuery,
OracleLatestQuery
} from "src/interfaces/IReservoirPriceOracle.sol";
import { IReservoirPriceOracle, OracleAverageQuery, OracleLatestQuery } from "src/interfaces/IReservoirPriceOracle.sol";
import { IPriceOracle } from "src/interfaces/IPriceOracle.sol";
import { QueryProcessor, ReservoirPair, PriceType } from "src/libraries/QueryProcessor.sol";
import { Utils } from "src/libraries/Utils.sol";
Expand Down Expand Up @@ -113,6 +109,7 @@
(rRoute,,) = _getRouteDecimalDifferencePrice(aToken0, aToken1);
}

// REVIEW: Is this intended primarily for debugging? Because this won't work on composite routes right?
xenide marked this conversation as resolved.
Show resolved Hide resolved
/// @notice The latest cached geometric TWAP of token0/token1.
/// Stored in the form of a 18 decimal fixed point number.
/// Supported price range: 1wei to `Constants.MAX_SUPPORTED_PRICE`.
Expand Down Expand Up @@ -153,7 +150,9 @@
);

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

_writePriceCache(lToken0, lToken1, lNewPrice);

Expand Down Expand Up @@ -238,10 +237,10 @@
lPayoutAmt = block.basefee * rewardGasAmount;
}

if (lPayoutAmt <= address(this).balance) {
payable(aRecipient).transfer(lPayoutAmt);
// does not revert under any circumstance
assembly ("memory-safe") {
let result := call(gas(), aRecipient, lPayoutAmt, codesize(), 0x00, codesize(), 0x00)

Check warning on line 242 in src/ReservoirPriceOracle.sol

View workflow job for this annotation

GitHub Actions / lint 

Variable "result" is unused
OliverNChalk marked this conversation as resolved.
Show resolved Hide resolved
}
// do nothing if lPayoutAmt is greater than the balance
}

/// @return rRoute The route to determine the price between aToken0 and aToken1
Expand All @@ -255,7 +254,7 @@
bytes32 lSlot = Utils.calculateSlot(aToken0, aToken1);

bytes32 lFirstWord;
assembly {
assembly ("memory-safe") {
lFirstWord := sload(lSlot)
}

Expand All @@ -277,7 +276,7 @@
} else {
assert(lFirstWord.is3HopRoute());
bytes32 lSecondWord;
assembly {
assembly ("memory-safe") {
lSecondWord := sload(add(lSlot, 1))
}
address lThirdToken = lSecondWord.getThirdToken();
Expand All @@ -303,7 +302,7 @@

bytes32 lSlot = Utils.calculateSlot(lToken0, lToken1);
bytes32 lData;
assembly {
assembly ("memory-safe") {
lData := sload(lSlot)
}
if (lData == bytes32(0)) {
Expand All @@ -323,7 +322,7 @@
bytes32 lSlot = Utils.calculateSlot(aToken0, aToken1);

bytes32 lData;
assembly {
assembly ("memory-safe") {
lData := sload(lSlot)
}
if (lData.isSimplePrice()) {
Expand All @@ -337,15 +336,15 @@

bytes32 lSlot = Utils.calculateSlot(aToken0, aToken1);
bytes32 lData;
assembly {
assembly ("memory-safe") {
lData := sload(lSlot)
}
if (!lData.isSimplePrice()) revert OracleErrors.WriteToNonSimpleRoute();

int256 lDiff = lData.getDecimalDifference();

lData = RoutesLib.packSimplePrice(lDiff, aNewPrice);
assembly {
assembly ("memory-safe") {
sstore(lSlot, lData)
}
}
Expand Down Expand Up @@ -511,7 +510,7 @@
int256 lDiff = int256(lToken1Decimals) - int256(lToken0Decimals);

bytes32 lData = RoutesLib.packSimplePrice(lDiff, 0);
assembly {
assembly ("memory-safe") {
// Write data to storage.
sstore(lSlot, lData)
}
Expand All @@ -523,14 +522,14 @@

if (lRouteLength == 3) {
bytes32 lData = RoutesLib.pack2HopRoute(lSecondToken);
assembly {
assembly ("memory-safe") {
sstore(lSlot, lData)
}
} else if (lRouteLength == 4) {
(bytes32 lFirstWord, bytes32 lSecondWord) = RoutesLib.pack3HopRoute(lSecondToken, lThirdToken);

// Write two words to storage.
assembly {
assembly ("memory-safe") {
sstore(lSlot, lFirstWord)
sstore(add(lSlot, 1), lSecondWord)
}
Expand All @@ -550,14 +549,14 @@
bytes32 lSlot = Utils.calculateSlot(aToken0, aToken1);

// clear the storage slot that the route has written to previously
assembly {
assembly ("memory-safe") {
sstore(lSlot, 0)
}

// routes with length MAX_ROUTE_LENGTH use one more word of storage
// `setRoute` enforces the MAX_ROUTE_LENGTH limit.
if (lRoute.length == Constants.MAX_ROUTE_LENGTH) {
assembly {
assembly ("memory-safe") {
sstore(add(lSlot, 1), 0)
}
}
Expand Down
11 changes: 11 additions & 0 deletions test/mock/GasBuster.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.0;

contract GasBuster {
// Allow the contract to receive ETH
receive() external payable {
while (true) {

Check warning on line 7 in test/mock/GasBuster.sol

View workflow job for this annotation

GitHub Actions / lint 

Code contains empty blocks
// This loop will continue until all gas is consumed
}
}
}
26 changes: 26 additions & 0 deletions test/unit/ReservoirPriceOracle.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { EnumerableSetLib } from "lib/solady/src/utils/EnumerableSetLib.sol";
import { Constants } from "src/libraries/Constants.sol";
import { MockFallbackOracle } from "test/mock/MockFallbackOracle.sol";
import { StubERC4626 } from "test/mock/StubERC4626.sol";
import { GasBuster } from "test/mock/GasBuster.sol";

contract ReservoirPriceOracleTest is BaseTest {
using Utils for *;
Expand Down Expand Up @@ -621,6 +622,31 @@ contract ReservoirPriceOracleTest is BaseTest {
assertEq(lPriceAB, 0); // composite price is not stored in the cache
}

function testUpdatePrice_RecipientOutOfGas() external {
// arrange
GasBuster lGasBuster = new GasBuster();

_writePriceCache(address(_tokenA), address(_tokenB), 5e18);
deal(address(_oracle), 1 ether);

skip(1);
_pair.sync();
skip(_oracle.twapPeriod() * 2);
_tokenA.mint(address(_pair), 2e18);
_pair.swap(2e18, true, address(this), "");

// act
_oracle.updatePrice(address(_tokenB), address(_tokenA), address(lGasBuster));

// assert
(uint256 lPrice,) = _oracle.priceCache(address(_tokenA), address(_tokenB));
assertEq(lPrice, 98_918_868_099_219_913_512);
(lPrice,) = _oracle.priceCache(address(_tokenB), address(_tokenA));
xenide marked this conversation as resolved.
Show resolved Hide resolved
assertEq(lPrice, 0);
assertEq(address(lGasBuster).balance, 0);
assertEq(address(_oracle).balance, 1 ether);
}

function testSetRoute() public {
// arrange
address lToken0 = address(_tokenB);
Expand Down
Loading