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

Review/oliver 5 #16

Merged
merged 9 commits into from
Jul 8, 2024
Merged

Review/oliver 5 #16

Changes from 2 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
b485238
make functions private instead of internal where possible
OliverNChalk Jul 8, 2024
aae7af3
make functions external instead of public where possible
OliverNChalk Jul 8, 2024
03db2f4
fix: rm redundant function to query bounty available
xenide Jul 8, 2024
5359ac8
fix: rename section from internal functions to private functions
xenide Jul 8, 2024
08b30ee
test: rm gas bounty tests
xenide Jul 8, 2024
5316d5c
fix: variable re-declaration
xenide Jul 8, 2024
84b1cc5
gas: update snapshot
xenide Jul 8, 2024
cd6f68b
fix: rm `IReservoirPriceOracle` and associated functions
xenide Jul 8, 2024
60564f0
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
37 changes: 20 additions & 17 deletions src/ReservoirPriceOracle.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,13 @@ contract ReservoirPriceOracle is IPriceOracle, IReservoirPriceOracle, Owned(msg.
/// 1e18 == 100%
uint64 public priceDeviationThreshold;

/// @notice This number is multiplied by the base fee to determine the reward for keepers
/// @notice This number is multiplied by the base fee to determine the reward for keepers.
uint64 public rewardGasAmount;

/// @notice TWAP period (in seconds) for querying the oracle
/// @notice TWAP period (in seconds) for querying the oracle.
uint64 public twapPeriod;

/// @notice Designated pairs to serve as price feed for a certain token0 and token1
/// @notice Designated pairs to serve as price feed for a certain token0 and token1.
mapping(address token0 => mapping(address token1 => ReservoirPair pair)) public pairs;

///////////////////////////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -101,6 +101,7 @@ contract ReservoirPriceOracle is IPriceOracle, IReservoirPriceOracle, Owned(msg.

// price update related functions

// REVIEW: While this is nice in terms of self documentation, I think any MEV bot can just do `address(reservoirOracle).balance` right?
OliverNChalk marked this conversation as resolved.
Show resolved Hide resolved
function gasBountyAvailable() external view returns (uint256) {
return address(this).balance;
}
Expand Down Expand Up @@ -129,7 +130,7 @@ contract ReservoirPriceOracle is IPriceOracle, IReservoirPriceOracle, Owned(msg.
/// @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) public nonReentrant {
function updatePrice(address aTokenA, address aTokenB, address aRewardRecipient) external nonReentrant {
(address lToken0, address lToken1) = Utils.sortTokens(aTokenA, aTokenB);

(address[] memory lRoute,, uint256 lPrevPrice) = _getRouteDecimalDifferencePrice(lToken0, lToken1);
Expand Down Expand Up @@ -164,9 +165,10 @@ contract ReservoirPriceOracle is IPriceOracle, IReservoirPriceOracle, Owned(msg.

// IReservoirPriceOracle

// REVIEW: This let's anyone read the Reservoir oracle without paying right?
xenide marked this conversation as resolved.
Show resolved Hide resolved
/// @inheritdoc IReservoirPriceOracle
function getTimeWeightedAverage(OracleAverageQuery[] memory aQueries)
public
external
view
returns (uint256[] memory rResults)
{
Expand All @@ -176,6 +178,7 @@ contract ReservoirPriceOracle is IPriceOracle, IReservoirPriceOracle, Owned(msg.
}
}

// REVIEW: What value is there in this?
/// @inheritdoc IReservoirPriceOracle
function getLatest(OracleLatestQuery calldata aQuery) external view returns (uint256) {
ReservoirPair lPair = pairs[aQuery.base][aQuery.quote];
Expand All @@ -198,15 +201,15 @@ contract ReservoirPriceOracle is IPriceOracle, IReservoirPriceOracle, Owned(msg.
if (aToken1 <= aToken0) revert OracleErrors.InvalidTokensProvided();
}

function _getTimeWeightedAverageSingle(OracleAverageQuery memory aQuery) internal view returns (uint256 rResult) {
function _getTimeWeightedAverageSingle(OracleAverageQuery memory aQuery) private view returns (uint256 rResult) {
ReservoirPair lPair = pairs[aQuery.base][aQuery.quote];
_validatePair(lPair);

(,,, uint16 lIndex) = lPair.getReserves();
rResult = lPair.getTimeWeightedAverage(aQuery.priceType, aQuery.secs, aQuery.ago, lIndex);
}

function _calcPercentageDiff(uint256 aOriginal, uint256 aNew) internal pure returns (uint256) {
function _calcPercentageDiff(uint256 aOriginal, uint256 aNew) private pure returns (uint256) {
unchecked {
if (aOriginal == 0) return 0;

Expand All @@ -220,7 +223,7 @@ contract ReservoirPriceOracle is IPriceOracle, IReservoirPriceOracle, Owned(msg.
}
}

function _rewardUpdater(address aRecipient) internal {
function _rewardUpdater(address aRecipient) private {
if (aRecipient == address(0)) return;

// N.B. Revisit this whenever deployment on a new chain is needed
Expand All @@ -246,7 +249,7 @@ contract ReservoirPriceOracle is IPriceOracle, IReservoirPriceOracle, Owned(msg.
/// @return rDecimalDiff The result of token1.decimals() - token0.decimals() if it's a simple route. 0 otherwise
/// @return rPrice The price of aToken0/aToken1 if it's a simple route (i.e. rRoute.length == 2). 0 otherwise
function _getRouteDecimalDifferencePrice(address aToken0, address aToken1)
internal
private
view
returns (address[] memory rRoute, int256 rDecimalDiff, uint256 rPrice)
{
Expand Down Expand Up @@ -314,7 +317,7 @@ contract ReservoirPriceOracle is IPriceOracle, IReservoirPriceOracle, Owned(msg.

// performs an SLOAD to load 1 word which contains the simple price and decimal difference
function _priceCache(address aToken0, address aToken1)
internal
private
view
returns (uint256 rPrice, int256 rDecimalDiff)
{
Expand All @@ -330,7 +333,7 @@ contract ReservoirPriceOracle is IPriceOracle, IReservoirPriceOracle, Owned(msg.
}
}

function _writePriceCache(address aToken0, address aToken1, uint256 aNewPrice) internal {
function _writePriceCache(address aToken0, address aToken1, uint256 aNewPrice) private {
if (aNewPrice == 0 || aNewPrice > Constants.MAX_SUPPORTED_PRICE) revert OracleErrors.PriceOutOfRange(aNewPrice);

bytes32 lSlot = Utils.calculateSlot(aToken0, aToken1);
Expand All @@ -349,7 +352,7 @@ contract ReservoirPriceOracle is IPriceOracle, IReservoirPriceOracle, Owned(msg.
}

function _getQuotes(uint256 aAmount, address aBase, address aQuote, bool aIsGetQuotes)
internal
private
view
returns (uint256 rBidOut, uint256 rAskOut)
{
Expand Down Expand Up @@ -386,20 +389,20 @@ contract ReservoirPriceOracle is IPriceOracle, IReservoirPriceOracle, Owned(msg.
assert(lRoute[0] == aBase);

for (uint256 i = 0; i < lRoute.length - 1; ++i) {
(address lLowerToken, address lHigherToken) = Utils.sortTokens(lRoute[i], lRoute[i + 1]);
(address lToken0, address 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(lLowerToken, lHigherToken);
(lPrice, lDecimalDiff) = _priceCache(lToken0, lToken1);

if (lPrice == 0) revert OracleErrors.PriceZero();
lIntermediateAmount = _calcAmtOut(lIntermediateAmount, lPrice, lDecimalDiff, lRoute[i] != lLowerToken);
lIntermediateAmount = _calcAmtOut(lIntermediateAmount, lPrice, lDecimalDiff, lRoute[i] != lToken0);
}
rBidOut = rAskOut = lIntermediateAmount;
}
}

/// @dev aPrice assumed to be > 0, as checked by _getQuote
function _calcAmtOut(uint256 aAmountIn, uint256 aPrice, int256 aDecimalDiff, bool aInverse)
internal
private
pure
returns (uint256 rOut)
{
Expand Down Expand Up @@ -427,7 +430,7 @@ contract ReservoirPriceOracle is IPriceOracle, IReservoirPriceOracle, Owned(msg.
}

function _useFallbackOracle(uint256 aAmount, address aBase, address aQuote, bool aIsGetQuotes)
internal
private
view
returns (uint256 rBidOut, uint256 rAskOut)
{
Expand Down
Loading