Skip to content

Commit

Permalink
some initial questions
Browse files Browse the repository at this point in the history
  • Loading branch information
OliverNChalk committed Jun 27, 2024
1 parent aa90524 commit f2f3ee9
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 5 deletions.
24 changes: 19 additions & 5 deletions src/ReservoirPriceOracle.sol
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ contract ReservoirPriceOracle is IPriceOracle, IReservoirPriceOracle, Owned(msg.
using LibSort for address[];
using FlagsLib for *;
using QueryProcessor for ReservoirPair;
// REVIEW: It doesn't make sense for calculateSlot to be attached to address
// as it is a function that maps `(addr, addr) -> u256`, not anything that
// operates on a single `address`.
using Utils for *;

///////////////////////////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -249,10 +252,17 @@ contract ReservoirPriceOracle is IPriceOracle, IReservoirPriceOracle, Owned(msg.
if (aRecipient == address(0)) return;

// N.B. Revisit this whenever deployment on a new chain is needed
// we use `block.basefee` instead of `ArbGasInfo::getMinimumGasPrice()` on ARB because the latter will always return
// the demand insensitive base fee, while the former can return real higher fees during times of congestion
// safety: this mul will not overflow even in extreme cases of `block.basefee`
uint256 lPayoutAmt = block.basefee * rewardGasAmount;
//
// we use `block.basefee` instead of `ArbGasInfo::getMinimumGasPrice()`
// on ARB because the latter will always return the demand insensitive
// base fee, while the former can return higher fees during times of
// congestion

// SAFETY: this mul will not overflow even in extreme cases of `block.basefee`.
uint256 lPayoutAmt;
unchecked {
lPayoutAmt = block.basefee * rewardGasAmount;
}

if (lPayoutAmt <= address(this).balance) {
payable(aRecipient).transfer(lPayoutAmt);
Expand Down Expand Up @@ -385,6 +395,7 @@ contract ReservoirPriceOracle is IPriceOracle, IReservoirPriceOracle, Owned(msg.
_getRouteDecimalDifferencePrice(lToken0, lToken1);

if (lRoute.length == 0) {
// REVIEW: Is it possible for `aQuote` to be an IERC4626?
// There is one case where the behavior is a bit more unexpected, and that is when
// `aBase` is an empty contract, and the revert would not be caught at all, causing
// the entire operation to fail. But this is okay, because if `aBase` is not a contract, trying
Expand Down Expand Up @@ -533,7 +544,7 @@ contract ReservoirPriceOracle is IPriceOracle, IReservoirPriceOracle, Owned(msg.

int256 lDiff = int256(lToken1Decimals) - int256(lToken0Decimals);

bytes32 lData = lDiff.packSimplePrice(0);
bytes32 lData = lDiff.packSimplePice(0);
assembly {
// Write data to storage.
sstore(lSlot, lData)
Expand All @@ -545,6 +556,9 @@ contract ReservoirPriceOracle is IPriceOracle, IReservoirPriceOracle, Owned(msg.
address lThirdToken = aRoute[2];

if (lRouteLength == 3) {
// REVIEW: I'm not sure how these 2 or 3 hop routes work? As far
// as I can tell only a single address gets written to storage?
// What am I missing?
bytes32 lData = lSecondToken.pack2HopRoute();
assembly {
sstore(lSlot, lData)
Expand Down
2 changes: 2 additions & 0 deletions src/interfaces/IReservoirPriceOracle.sol
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ interface IReservoirPriceOracle {
*/
function getLatest(OracleLatestQuery calldata priceType) external view returns (uint256);

// REVIEW: Who calls this?
/**
* @dev Returns largest time window that can be safely queried, where 'safely' means the Oracle is guaranteed to be
* able to produce a result and not revert.
Expand All @@ -53,6 +54,7 @@ interface IReservoirPriceOracle {
*/
function getLargestSafeQueryWindow() external view returns (uint256);

// REVIEW: Who calls this?
/**
* @dev Returns the accumulators corresponding to each of `queries`.
*/
Expand Down

0 comments on commit f2f3ee9

Please sign in to comment.