Skip to content

Commit

Permalink
feat: introduce max change per trade
Browse files Browse the repository at this point in the history
* feat: introduce max change per trade
* feat: implementing per trade limited price
* feat: insert assert statement
* test: fix expected emit log
* fix: underflow problem for price reduction case
* test: add tests for different clamp scenarios
* test: rm blank line
* docs: update comment
* fix: update impl to clamp the rate of change
* test: more cases
* test: show how long it takes for a pair to recover from malicious mint
* lint: forge fmt
* test: update case with diff parameters
  • Loading branch information
xenide authored Nov 18, 2024
1 parent 4100d42 commit 33218e2
Show file tree
Hide file tree
Showing 6 changed files with 201 additions and 40 deletions.
8 changes: 4 additions & 4 deletions script/optimized-deployer-meta
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"constant_product_hash": "0x356ae496acb4119a46481b3326102cb38281aad285e579d14598f6c5bb76f5e2",
"factory_hash": "0xae59efa1baf1113957e748f189c8eb9271c4a0d919c8af03d26ae4fbddb51e81",
"oracle_caller_hash": "0xf130f5cb7eebcf57bac9f5d273fd8cc18dfb1b3f48d114a5e6a8230440d50e0f",
"stable_hash": "0xa4bb872f2e22f611d0fd1ff88d960edc58283528282cb20aab4fbe14da557300"
"constant_product_hash": "0xbd78fb88c65e9a8804773c3a0e0350627ae244d6daf312a549fc1e08ef9bf56e",
"factory_hash": "0x84e232ec0f6ea2ec4c9e4cfa7e5469dab805cb65ffaab4a7ee9c7c602f91345a",
"oracle_caller_hash": "0xed0f7d96dcba353321d583022005b03c09088f7de3800703dc8324b1da6c77a6",
"stable_hash": "0x24174b50e2a4c46e25d5367496b6a2ca38bf33dceb07ef3aba32e2c1266a6bf1"
}
3 changes: 2 additions & 1 deletion src/Constants.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,6 @@ library Constants {
uint256 public constant DEFAULT_SWAP_FEE_SP = 100; // 0.01%
uint256 public constant DEFAULT_PLATFORM_FEE = 250_000; // 25%
uint256 public constant DEFAULT_AMP_COEFF = 1000;
uint256 public constant DEFAULT_MAX_CHANGE_RATE = 0.0005e18;
uint128 public constant DEFAULT_MAX_CHANGE_RATE = 0.0005e18;
uint128 public constant DEFAULT_MAX_CHANGE_PER_TRADE = 0.03e18; // 3%
}
1 change: 1 addition & 0 deletions src/ReservoirDeployer.sol
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ contract ReservoirDeployer {
factory.write("Shared::platformFeeTo", address(this));
factory.write("Shared::recoverer", address(this));
factory.write("Shared::maxChangeRate", Constants.DEFAULT_MAX_CHANGE_RATE);
factory.write("Shared::maxChangePerTrade", Constants.DEFAULT_MAX_CHANGE_PER_TRADE);

// Step complete.
step += 1;
Expand Down
40 changes: 29 additions & 11 deletions src/ReservoirPair.sol
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ abstract contract ReservoirPair is IAssetManagedPair, ReservoirERC20 {
updateSwapFee();
updatePlatformFee();
updateOracleCaller();
setMaxChangeRate(factory.read(MAX_CHANGE_RATE_NAME).toUint256());
setClampParams(factory.read(MAX_CHANGE_RATE_NAME).toUint128(), factory.read(MAX_CHANGE_PER_TRADE_NAME).toUint128());
}
}

Expand Down Expand Up @@ -516,18 +516,23 @@ abstract contract ReservoirPair is IAssetManagedPair, ReservoirERC20 {
//////////////////////////////////////////////////////////////////////////*/

event OracleCallerUpdated(address oldCaller, address newCaller);
event MaxChangeRateUpdated(uint256 oldMaxChangePerSecond, uint256 newMaxChangePerSecond);
event ClampParamsUpdated(uint128 newMaxChangeRatePerSecond, uint128 newMaxChangePerTrade);

// 100 basis points per second which is 60% per minute
uint256 internal constant MAX_CHANGE_PER_SEC = 0.01e18;
// 10%
uint256 internal constant MAX_CHANGE_PER_TRADE = 0.1e18;
string internal constant MAX_CHANGE_RATE_NAME = "Shared::maxChangeRate";
string internal constant MAX_CHANGE_PER_TRADE_NAME = "Shared::maxChangePerTrade";
string internal constant ORACLE_CALLER_NAME = "Shared::oracleCaller";

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. 1e18 == 100%
uint256 public maxChangeRate;
uint128 public maxChangeRate;
// how much the clamped price can move within one trade. 1e18 == 100%
uint128 public maxChangePerTrade;

address public oracleCaller;

Expand All @@ -544,10 +549,13 @@ abstract contract ReservoirPair is IAssetManagedPair, ReservoirERC20 {
}
}

function setMaxChangeRate(uint256 aMaxChangeRate) public onlyFactory {
function setClampParams(uint128 aMaxChangeRate, uint128 aMaxChangePerTrade) public onlyFactory {
require(0 < aMaxChangeRate && aMaxChangeRate <= MAX_CHANGE_PER_SEC, "RP: INVALID_CHANGE_PER_SECOND");
emit MaxChangeRateUpdated(maxChangeRate, aMaxChangeRate);
require(0 < aMaxChangePerTrade && aMaxChangePerTrade <= MAX_CHANGE_PER_TRADE, "RP: INVALID_CHANGE_PER_TRADE");

emit ClampParamsUpdated(aMaxChangeRate, aMaxChangePerTrade);
maxChangeRate = aMaxChangeRate;
maxChangePerTrade = aMaxChangePerTrade;
}

function _calcClampedPrice(
Expand All @@ -559,19 +567,29 @@ abstract contract ReservoirPair is IAssetManagedPair, ReservoirERC20 {
) internal virtual returns (uint256 rClampedPrice, int256 rClampedLogPrice) {
// call to `percentDelta` will revert if the difference between aCurrRawPrice and aPrevClampedPrice is
// greater than uint196 (1e59). It is extremely unlikely that one trade can change the price by 1e59
// if aPreviousTimestamp is 0, this is the first calculation of clamped price, and so should be set to the raw price
if (aPreviousTimestamp == 0 || aCurrRawPrice.percentDelta(aPrevClampedPrice) <= maxChangeRate * aTimeElapsed) {
if (
(
aCurrRawPrice.percentDelta(aPrevClampedPrice) <= maxChangeRate * aTimeElapsed
&& aCurrRawPrice.percentDelta(aPrevClampedPrice) <= maxChangePerTrade
)
// this is the first ever calculation of clamped price, and so should be set to the raw price
|| aPreviousTimestamp == 0
) {
(rClampedPrice, rClampedLogPrice) = (aCurrRawPrice, aCurrLogRawPrice);
} else {
// clamp the price
// multiplication of maxChangeRate and aTimeElapsed would not overflow as
// multiplication of maxChangeRate and aTimeElapsed will not overflow as
// maxChangeRate <= 0.01e18 (50 bits)
// aTimeElapsed <= 32 bits
uint256 lLowerRateOfChange = (maxChangeRate * aTimeElapsed).min(maxChangePerTrade);
if (aCurrRawPrice > aPrevClampedPrice) {
rClampedPrice = aPrevClampedPrice.fullMulDiv(1e18 + maxChangeRate * aTimeElapsed, 1e18);
rClampedPrice = aPrevClampedPrice.fullMulDiv(1e18 + lLowerRateOfChange, 1e18);
assert(rClampedPrice < aCurrRawPrice);
} else {
assert(aPrevClampedPrice > aCurrRawPrice);
rClampedPrice = aPrevClampedPrice.fullMulDiv(1e18 - maxChangeRate * aTimeElapsed, 1e18);
// subtraction will not underflow as it is limited by the max possible value of maxChangePerTrade
// which is MAX_CHANGE_PER_TRADE
rClampedPrice = aPrevClampedPrice.fullMulDiv(1e18 - lLowerRateOfChange, 1e18);
assert(rClampedPrice > aCurrRawPrice);
}
rClampedLogPrice = LogCompression.toLowResLog(rClampedPrice);
}
Expand Down
4 changes: 4 additions & 0 deletions src/libraries/Bytes32.sol
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ library Bytes32Lib {
return uint64(uint256(aValue));
}

function toUint128(bytes32 aValue) internal pure returns (uint128) {
return uint128(uint256(aValue));
}

function toUint256(bytes32 aValue) internal pure returns (uint256) {
return uint256(aValue);
}
Expand Down
185 changes: 161 additions & 24 deletions test/unit/OracleWriter.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ pragma solidity ^0.8.0;

import "test/__fixtures/BaseTest.sol";

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

import { ReservoirPair, Observation } from "src/ReservoirPair.sol";
import { LogCompression } from "src/libraries/LogCompression.sol";
import { FactoryStoreLib } from "src/libraries/FactoryStore.sol";
Expand All @@ -13,9 +15,10 @@ import { Constants } from "src/Constants.sol";

contract OracleWriterTest is BaseTest {
using FactoryStoreLib for GenericFactory;
using FixedPointMathLib for uint256;

event OracleCallerUpdated(address oldCaller, address newCaller);
event MaxChangeRateUpdated(uint256 oldMaxChangeRate, uint256 newMaxChangeRate);
event ClampParamsUpdated(uint128 newMaxChangeRatePerSecond, uint128 newMaxChangePerTrade);

ReservoirPair[] internal _pairs;
ReservoirPair internal _pair;
Expand Down Expand Up @@ -136,33 +139,33 @@ contract OracleWriterTest is BaseTest {
assertEq(_pair.maxChangeRate(), Constants.DEFAULT_MAX_CHANGE_RATE);
}

function testSetMaxChangeRate_OnlyFactory() external allPairs {
function testSetClampParams_OnlyFactory() external allPairs {
// act & assert
vm.expectRevert();
_pair.setMaxChangeRate(1);
_pair.setClampParams(1, 1);

vm.prank(address(_factory));
vm.expectEmit(false, false, false, true);
emit MaxChangeRateUpdated(Constants.DEFAULT_MAX_CHANGE_RATE, 1);
_pair.setMaxChangeRate(1);
vm.expectEmit(false, false, false, false);
emit ClampParamsUpdated(1, 1);
_pair.setClampParams(1, 1);
assertEq(_pair.maxChangeRate(), 1);
}

function testSetMaxChangeRate_TooLow() external allPairs {
function testSetClampParams_TooLow() external allPairs {
// act & assert
vm.prank(address(_factory));
vm.expectRevert("RP: INVALID_CHANGE_PER_SECOND");
_pair.setMaxChangeRate(0);
_pair.setClampParams(0, 0);
}

function testSetMaxChangeRate_TooHigh(uint256 aMaxChangeRate) external allPairs {
function testSetClampParams_TooHigh(uint256 aMaxChangeRate) external allPairs {
// assume
uint256 lMaxChangeRate = bound(aMaxChangeRate, 0.01e18 + 1, type(uint256).max);
uint128 lMaxChangeRate = uint128(bound(aMaxChangeRate, 0.01e18 + 1, type(uint128).max));

// act & assert
vm.prank(address(_factory));
vm.expectRevert("RP: INVALID_CHANGE_PER_SECOND");
_pair.setMaxChangeRate(lMaxChangeRate);
_pair.setClampParams(lMaxChangeRate, 1);
}

function testOracle_NoWriteInSameTimestamp() public allPairs {
Expand Down Expand Up @@ -293,6 +296,100 @@ contract OracleWriterTest is BaseTest {
assertEq(lObs.timestamp, lStartingTimestamp + lJumpAhead);
}

function testUpdateOracle_DecreasePrice_LongElapsedTime() external allPairs {
// arrange - assume that no trade has taken place in a long time
uint256 lOriginalPrice = 1e18;
uint256 lFastForward = type(uint24).max; // 16777216
_stepTime(lFastForward);

// act
_tokenA.mint(address(_pair), 5000e18);
_pair.swap(5000e18, true, address(this), "");

// assert - since the max change rate is useless price given the long time elapsed
// the clamped price should only be limited by the max change per trade
(,,, uint16 lIndex) = _pair.getReserves();
Observation memory lObs = _oracleCaller.observation(_pair, lIndex);
uint256 lMaxChangePerTrade = _pair.maxChangePerTrade();
assertApproxEqRel(
LogCompression.fromLowResLog(lObs.logInstantClampedPrice),
lOriginalPrice.fullMulDiv(1e18 - lMaxChangePerTrade, 1e18),
0.0001e18
);
assertLt(lObs.logInstantRawPrice, lObs.logInstantClampedPrice); // the log of the raw price should be more negative than the log of the clamped price
}

function testUpdateOracle_DecreasePrice_ExceedMaxChangeRate() external allPairs {
// arrange - set maxChangeRate to a very small number
uint256 lFastForward = 10;
uint256 lOriginalPrice = 1e18;
_stepTime(lFastForward);
vm.prank(address(_factory));
_pair.setClampParams(0.00001e18, 0.1e18);

// act
_tokenA.mint(address(_pair), 50e18);
_pair.swap(50e18, true, address(this), "");

// assert
(,,, uint16 lIndex) = _pair.getReserves();
Observation memory lObs = _oracleCaller.observation(_pair, lIndex);
uint256 lMaxChangeRate = _pair.maxChangeRate();
assertApproxEqRel(
LogCompression.fromLowResLog(lObs.logInstantClampedPrice),
lOriginalPrice.fullMulDiv(1e18 - lMaxChangeRate * lFastForward, 1e18),
0.0001e18
);
}

function testUpdateOracle_DecreasePrice_ExceedMaxChangePerTrade() external allPairs {
// arrange - set maxChangePerTrade to a very small number
uint256 lFastForward = 10;
uint256 lOriginalPrice = 1e18;
_stepTime(lFastForward);
vm.prank(address(_factory));
_pair.setClampParams(0.001e18, 0.000001e18);

// act
_tokenA.mint(address(_pair), 50e18);
_pair.swap(50e18, true, address(this), "");

// assert
(,,, uint16 lIndex) = _pair.getReserves();
Observation memory lObs = _oracleCaller.observation(_pair, lIndex);
uint256 lMaxChangePerTrade = _pair.maxChangePerTrade();
assertApproxEqRel(
LogCompression.fromLowResLog(lObs.logInstantClampedPrice),
lOriginalPrice.fullMulDiv(1e18 - lMaxChangePerTrade, 1e18),
0.0001e18
);
}

function testUpdateOracle_DecreasePrice_ExceedBothMaxChangeRateAndMaxChangePerTrade() external allPairs {
// arrange
uint256 lFastForward = 100 days;
uint256 lOriginalPrice = 1e18;
_stepTime(lFastForward);

// act
uint256 lAmtToSwap = type(uint104).max / 2;
_tokenA.mint(address(_pair), lAmtToSwap);
_pair.swap(int256(lAmtToSwap), true, address(this), "");

// assert
(,,, uint16 lIndex) = _pair.getReserves();
Observation memory lObs = _oracleCaller.observation(_pair, lIndex);
uint256 lMaxChangeRate = _pair.maxChangeRate();
uint256 lMaxChangePerTrade = _pair.maxChangePerTrade();

uint256 lLowerRateOfChange = lMaxChangePerTrade.min(lMaxChangeRate * lFastForward);
assertApproxEqRel(
LogCompression.fromLowResLog(lObs.logInstantClampedPrice),
lOriginalPrice.fullMulDiv(1e18 - lLowerRateOfChange, 1e18),
0.0001e18
);
}

function testOracle_SameReservesDiffPrice(uint32 aNewStartTime) external randomizeStartTime(aNewStartTime) {
// arrange
ConstantProductPair lCP = ConstantProductPair(_createPair(address(_tokenB), address(_tokenC), 0));
Expand Down Expand Up @@ -415,28 +512,68 @@ contract OracleWriterTest is BaseTest {
}

function testOracle_OverflowAccPrice(uint32 aNewStartTime) public randomizeStartTime(aNewStartTime) allPairs {
// assume
vm.assume(aNewStartTime >= 1);

// arrange - make the last observation close to overflowing
(,,, uint16 lIndex) = _stablePair.getReserves();
_writeObservation(
_stablePair, lIndex, 1e3, 1e3, type(int88).max, type(int88).max, uint32(block.timestamp % 2 ** 31)
);
Observation memory lPrevObs = _oracleCaller.observation(_stablePair, lIndex);
(,,, uint16 lIndex) = _pair.getReserves();
_writeObservation(_pair, lIndex, 1e3, 1e3, type(int88).max, type(int88).max, uint32(block.timestamp % 2 ** 31));
Observation memory lPrevObs = _oracleCaller.observation(_pair, lIndex);

// act
uint256 lAmountToSwap = 5e18;
_tokenB.mint(address(_stablePair), lAmountToSwap);
_stablePair.swap(-int256(lAmountToSwap), true, address(this), "");
_tokenB.mint(address(_pair), lAmountToSwap);
_pair.swap(-int256(lAmountToSwap), true, address(this), "");

_stepTime(5);
_stablePair.sync();
_pair.sync();

// assert - when it overflows it goes from a very positive number to a very negative number
(,,, lIndex) = _stablePair.getReserves();
Observation memory lCurrObs = _oracleCaller.observation(_stablePair, lIndex);
(,,, lIndex) = _pair.getReserves();
Observation memory lCurrObs = _oracleCaller.observation(_pair, lIndex);
assertLt(lCurrObs.logAccRawPrice, lPrevObs.logAccRawPrice);
}

function testOracle_MintWrongPriceThenConverge() public {
// arrange - suppose that token C is ETH and token D is USDC
ReservoirPair lCP = ReservoirPair(_createPair(address(_tokenC), address(_tokenD), 0));
// initialize the pair with a price of 3M USD / ETH
_tokenC.mint(address(lCP), 0.000001e18);
_tokenD.mint(address(lCP), 3e6);
lCP.mint(address(this));
vm.startPrank(address(_factory));
lCP.setCustomSwapFee(0);
// set to 0.25 bp/s and 2% per trade
lCP.setClampParams(0.000025e18, 0.02e18);
vm.stopPrank();

// sanity - instant price is 3M
(,,, uint16 lIndex) = lCP.getReserves();
Observation memory lObs = _oracleCaller.observation(lCP, lIndex);
assertApproxEqRel(LogCompression.fromLowResLog(lObs.logInstantClampedPrice), 3_000_000e18, 0.0001e18);

// act - arbitrage happens that make the price go to around 3500 USD / ETH in one trade
_stepTime(10);
_tokenC.mint(address(lCP), 0.0000283e18);
lCP.swap(
address(lCP.token0()) == address(_tokenC) ? int256(0.0000283e18) : -int256(0.0000283e18),
true,
address(this),
""
);

// the instant raw price now is at 3494 USD
(,,, lIndex) = lCP.getReserves();
lObs = _oracleCaller.observation(lCP, lIndex);
assertApproxEqRel(LogCompression.fromLowResLog(lObs.logInstantRawPrice), 3494e18, 0.01e18);
// but clamped price is at 2.98M
assertApproxEqRel(LogCompression.fromLowResLog(lObs.logInstantClampedPrice), 2_984_969e18, 0.01e18);

uint256 lTimeStart = block.timestamp;
while (LogCompression.fromLowResLog(lObs.logInstantClampedPrice) > 3495e18) {
_stepTime(30);
lCP.sync();
(,,, lIndex) = lCP.getReserves();
lObs = _oracleCaller.observation(lCP, lIndex);
console.log(LogCompression.fromLowResLog(lObs.logInstantClampedPrice));
}
console.log("it took", block.timestamp - lTimeStart, "secs to get the clamped price to the true price");
}
}

0 comments on commit 33218e2

Please sign in to comment.