Skip to content

Commit

Permalink
fix: rm prevClampedPrice storage variable
Browse files Browse the repository at this point in the history
  • Loading branch information
@xenide
xenide committed Mar 5, 2024
1 parent 4e06950 commit a0d012e
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 37 deletions.
14 changes: 9 additions & 5 deletions src/ReservoirPair.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,10 @@ abstract contract ReservoirPair is IAssetManagedPair, ReservoirERC20 {
lTimeElapsed = lBlockTimestamp - aBlockTimestampLast;
}
if (lTimeElapsed > 0 && aReserve0 != 0 && aReserve1 != 0) {
_updateOracle(aReserve0, aReserve1, lTimeElapsed, lBlockTimestamp);

(uint256 lInstantPrice, int256 lLogInstantRawPrice) = _calcInstantPrice(aBalance0, aBalance1);

Check warning on line 160 in src/ReservoirPair.sol

View workflow job for this annotation

GitHub Actions / lint 

Variable "lInstantPrice" is unused

_updateOracle(lLogInstantRawPrice, aReserve0, aReserve1, lTimeElapsed, lBlockTimestamp);
}

// update reserves to match latest balances
Expand Down Expand Up @@ -489,10 +492,9 @@ abstract contract ReservoirPair is IAssetManagedPair, ReservoirERC20 {

mapping(uint256 => Observation) internal _observations;

// maximum allowed rate of change of price per second
// to mitigate oracle manipulation attacks in the face of post-merge ETH
// maximum allowed rate of change of price per second to mitigate oracle manipulation attacks in the face of
// post-merge ETH. 1e18 == 100%
uint256 public maxChangeRate;
uint256 public prevClampedPrice;

address public oracleCaller;

Expand Down Expand Up @@ -544,7 +546,9 @@ abstract contract ReservoirPair is IAssetManagedPair, ReservoirERC20 {
}
}

function _updateOracle(uint256 aReserve0, uint256 aReserve1, uint32 aTimeElapsed, uint32 aCurrentTimestamp)
function _updateOracle(int256 aLogInstantRawPrice, uint256 aReserve0, uint256 aReserve1, uint32 aTimeElapsed, uint32 aCurrentTimestamp)
internal
virtual;

function _calcInstantPrice(uint256 aBalance0, uint256 aBalance1) internal virtual returns (uint256, int112);
}
26 changes: 15 additions & 11 deletions src/curve/constant-product/ConstantProductPair.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { ConstantProductOracleMath } from "src/libraries/ConstantProductOracleMa
import { IReservoirCallee } from "src/interfaces/IReservoirCallee.sol";
import { IGenericFactory, IERC20 } from "src/interfaces/IGenericFactory.sol";

import { ReservoirPair, Slot0, Observation } from "src/ReservoirPair.sol";
import { ReservoirPair, Slot0, Observation, SafeCast, LogCompression } from "src/ReservoirPair.sol";

contract ConstantProductPair is ReservoirPair {
using FactoryStoreLib for IGenericFactory;
Expand Down Expand Up @@ -225,35 +225,39 @@ contract ConstantProductPair is ReservoirPair {
ORACLE METHODS
//////////////////////////////////////////////////////////////////////////*/

function _updateOracle(uint256 aReserve0, uint256 aReserve1, uint32 aTimeElapsed, uint32 aCurrentTimestamp)
function _updateOracle(int256 aLogInstantRawPrice, uint256 aReserve0, uint256 aReserve1, uint32 aTimeElapsed, uint32 aCurrentTimestamp)

Check warning on line 228 in src/curve/constant-product/ConstantProductPair.sol

View workflow job for this annotation

GitHub Actions / lint 

Variable "aReserve0" is unused

Check warning on line 228 in src/curve/constant-product/ConstantProductPair.sol

View workflow job for this annotation

GitHub Actions / lint 

Variable "aReserve1" is unused
internal
override
{
Observation storage previous = _observations[_slot0.index];

(uint256 instantRawPrice, int112 logInstantRawPrice) = ConstantProductOracleMath.calcLogPrice(
aReserve0 * token0PrecisionMultiplier(), aReserve1 * token1PrecisionMultiplier()
);
(uint256 instantClampedPrice, int112 logInstantClampedPrice) =

Check warning on line 234 in src/curve/constant-product/ConstantProductPair.sol

View workflow job for this annotation

GitHub Actions / lint 

Variable "instantClampedPrice" is unused
_calcClampedPrice(instantRawPrice, prevClampedPrice, aTimeElapsed);
prevClampedPrice = instantClampedPrice;
_calcClampedPrice(
LogCompression.fromLowResLog(previous.logInstantRawPrice),
LogCompression.fromLowResLog(previous.logInstantClampedPrice),
aTimeElapsed
);

// overflow is desired here as the consumer of the oracle will be reading the difference in those
// accumulated log values
// when the index overflows it will overwrite the oldest observation and then forms a loop
unchecked {
int112 logAccRawPrice = previous.logAccRawPrice + logInstantRawPrice * int112(int256(uint256(aTimeElapsed)));
int112 logAccRawPrice = previous.logAccRawPrice + previous.logInstantRawPrice * int112(int256(uint256(aTimeElapsed)));
int56 logAccClampedPrice =
previous.logAccClampedPrice + int56(logInstantClampedPrice) * int56(int256(uint256(aTimeElapsed)));
_slot0.index += 1;
_observations[_slot0.index] = Observation(
// TODO: prove that these values are guaranteed <=int56 to remove these safe casts
SafeCastLib.toInt56(logInstantRawPrice),
SafeCastLib.toInt56(logInstantClampedPrice),
SafeCastLib.toInt56(logAccRawPrice),
SafeCast.toInt56(aLogInstantRawPrice),
SafeCast.toInt56(logInstantClampedPrice),
SafeCast.toInt56(logAccRawPrice),
logAccClampedPrice,
aCurrentTimestamp
);
}
}

function _calcInstantPrice(uint256 aBalance0, uint256 aBalance1) internal override returns (uint256, int112) {
return ConstantProductOracleMath.calcLogPrice(aBalance0 * token0PrecisionMultiplier(), aBalance1 * token1PrecisionMultiplier());
}
}
28 changes: 15 additions & 13 deletions src/curve/stable/StablePair.sol
View file
Original file line number Diff line number Diff line change
@@ -1,15 +1,13 @@
// SPDX-License-Identifier: GPL-3.0-or-later
pragma solidity ^0.8.0;

import { SafeCastLib } from "solady/utils/SafeCastLib.sol";

import { IReservoirCallee } from "src/interfaces/IReservoirCallee.sol";
import { IGenericFactory } from "src/interfaces/IGenericFactory.sol";

import { Bytes32Lib } from "src/libraries/Bytes32.sol";
import { FactoryStoreLib } from "src/libraries/FactoryStore.sol";

import { ReservoirPair, Slot0, Observation, IERC20 } from "src/ReservoirPair.sol";
import { ReservoirPair, Slot0, Observation, IERC20, SafeCast, LogCompression } from "src/ReservoirPair.sol";
import { StableMath } from "src/libraries/StableMath.sol";
import { StableOracleMath } from "src/libraries/StableOracleMath.sol";
import { ConstantProductOracleMath } from "src/libraries/ConstantProductOracleMath.sol";
Expand Down Expand Up @@ -281,34 +279,38 @@ contract StablePair is ReservoirPair {
ORACLE METHODS
//////////////////////////////////////////////////////////////////////////*/

function _updateOracle(uint256 aReserve0, uint256 aReserve1, uint32 aTimeElapsed, uint32 aCurrentTimestamp)
function _updateOracle(int256 aLogInstantRawPrice, uint256 aReserve0, uint256 aReserve1, uint32 aTimeElapsed, uint32 aCurrentTimestamp)

Check warning on line 282 in src/curve/stable/StablePair.sol

View workflow job for this annotation

GitHub Actions / lint 

Variable "aReserve0" is unused

Check warning on line 282 in src/curve/stable/StablePair.sol

View workflow job for this annotation

GitHub Actions / lint 

Variable "aReserve1" is unused
internal
override
{
Observation storage previous = _observations[_slot0.index];

(uint256 instantRawPrice, int112 logInstantRawPrice) = StableOracleMath.calcLogPrice(
_getCurrentAPrecise(), aReserve0 * token0PrecisionMultiplier(), aReserve1 * token1PrecisionMultiplier()
);
(uint256 instantClampedPrice, int112 logInstantClampedPrice) =

Check warning on line 288 in src/curve/stable/StablePair.sol

View workflow job for this annotation

GitHub Actions / lint 

Variable "instantClampedPrice" is unused
_calcClampedPrice(instantRawPrice, prevClampedPrice, aTimeElapsed);
prevClampedPrice = instantClampedPrice;
_calcClampedPrice(
LogCompression.fromLowResLog(previous.logInstantRawPrice),
LogCompression.fromLowResLog(previous.logInstantClampedPrice),
aTimeElapsed
);

// overflow is desired here as the consumer of the oracle will be reading the difference in those accumulated log values
// when the index overflows it will overwrite the oldest observation to form a loop
unchecked {
int112 logAccRawPrice = previous.logAccRawPrice + logInstantRawPrice * int112(int256(uint256(aTimeElapsed)));
int112 logAccRawPrice = previous.logAccRawPrice + previous.logInstantRawPrice * int112(int256(uint256(aTimeElapsed)));
int56 logAccClampedPrice =
previous.logAccClampedPrice + int56(logInstantClampedPrice) * int56(int256(uint256(aTimeElapsed)));
_slot0.index += 1;
_observations[_slot0.index] = Observation(
// TODO: prove that these values are guaranteed <=int56 to remove these safe casts
SafeCastLib.toInt56(logInstantRawPrice),
SafeCastLib.toInt56(logInstantClampedPrice),
SafeCastLib.toInt56(logAccRawPrice),
SafeCast.toInt56(aLogInstantRawPrice),
SafeCast.toInt56(logInstantClampedPrice),
SafeCast.toInt56(logAccRawPrice),
logAccClampedPrice,
aCurrentTimestamp
);
}
}

function _calcInstantPrice(uint256 aBalance0, uint256 aBalance1) internal override returns (uint256, int112) {

Check warning on line 313 in src/curve/stable/StablePair.sol

View workflow job for this annotation

GitHub Actions / lint 

Code contains empty blocks

}
}
14 changes: 8 additions & 6 deletions test/unit/ConstantProductPair.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -553,7 +553,8 @@ contract ConstantProductPairTest is BaseTest, IReservoirCallee {
_constantProductPair.swap(-int256(lSwapAmt), true, address(this), bytes(""));

// sanity
assertEq(_constantProductPair.prevClampedPrice(), 1e18);
// TODO: change to read instant clamped price from latest oracle observation
// assertEq(_constantProductPair.prevClampedPrice(), 1e18);

// act
_stepTime(5);
Expand All @@ -563,7 +564,8 @@ contract ConstantProductPairTest is BaseTest, IReservoirCallee {
Observation memory lObs1 = _oracleCaller.observation(_constantProductPair, 1);
// no diff between raw and clamped prices
assertEq(lObs1.logAccClampedPrice, lObs1.logAccRawPrice);
assertLt(_constantProductPair.prevClampedPrice(), 1.0025e18);

// assertLt(_constantProductPair.prevClampedPrice(), 1.0025e18);
}

function testOracle_ClampedPrice_AtLimit() external {
Expand All @@ -575,7 +577,7 @@ contract ConstantProductPairTest is BaseTest, IReservoirCallee {
_constantProductPair.swap(-int256(lSwapAmt), true, address(this), bytes(""));

// sanity
assertEq(_constantProductPair.prevClampedPrice(), 1e18);
// assertEq(_constantProductPair.prevClampedPrice(), 1e18);

// act
_stepTime(5);
Expand All @@ -585,7 +587,7 @@ contract ConstantProductPairTest is BaseTest, IReservoirCallee {
Observation memory lObs1 = _oracleCaller.observation(_constantProductPair, 1);
// no diff between raw and clamped prices
assertEq(lObs1.logAccClampedPrice, lObs1.logAccRawPrice);
assertLt(_constantProductPair.prevClampedPrice(), 1.0025e18);
// assertLt(_constantProductPair.prevClampedPrice(), 1.0025e18);
}

function testOracle_ClampedPrice_OverLimit() external {
Expand All @@ -597,7 +599,7 @@ contract ConstantProductPairTest is BaseTest, IReservoirCallee {
_constantProductPair.swap(-int256(lSwapAmt), true, address(this), bytes(""));

// sanity
assertEq(_constantProductPair.prevClampedPrice(), 1e18);
// assertEq(_constantProductPair.prevClampedPrice(), 1e18);

// act
_stepTime(5);
Expand All @@ -606,7 +608,7 @@ contract ConstantProductPairTest is BaseTest, IReservoirCallee {
// assert
Observation memory lObs1 = _oracleCaller.observation(_constantProductPair, 1);
assertGt(lObs1.logAccRawPrice, lObs1.logAccClampedPrice);
assertEq(_constantProductPair.prevClampedPrice(), 1.0025e18);
// assertEq(_constantProductPair.prevClampedPrice(), 1.0025e18);
}

function testPlatformFee_Disable() external {
Expand Down
4 changes: 2 additions & 2 deletions test/unit/StablePair.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1754,7 +1754,7 @@ contract StablePairTest is BaseTest {
_stablePair.swap(-int256(lSwapAmt), true, address(this), bytes(""));

// sanity
assertEq(_stablePair.prevClampedPrice(), 1e18);
// assertEq(_stablePair.prevClampedPrice(), 1e18);

// act
_stepTime(5);
Expand All @@ -1764,6 +1764,6 @@ contract StablePairTest is BaseTest {
Observation memory lObs1 = _oracleCaller.observation(_stablePair, 1);
// no diff between raw and clamped prices
assertEq(lObs1.logAccClampedPrice, lObs1.logAccRawPrice);
assertLt(_stablePair.prevClampedPrice(), 1.0025e18);
// assertLt(_stablePair.prevClampedPrice(), 1.0025e18);
}
}

0 comments on commit a0d012e

Please sign in to comment.