diff --git a/script/optimized-deployer-meta b/script/optimized-deployer-meta index 1bd97351..9a93c6c4 100644 --- a/script/optimized-deployer-meta +++ b/script/optimized-deployer-meta @@ -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" } \ No newline at end of file diff --git a/src/Constants.sol b/src/Constants.sol index f84b0f5a..5634d98e 100644 --- a/src/Constants.sol +++ b/src/Constants.sol @@ -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% } diff --git a/src/ReservoirDeployer.sol b/src/ReservoirDeployer.sol index 1a46177b..c12616cd 100644 --- a/src/ReservoirDeployer.sol +++ b/src/ReservoirDeployer.sol @@ -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; diff --git a/src/ReservoirPair.sol b/src/ReservoirPair.sol index 72dd3cbd..98ac3c73 100644 --- a/src/ReservoirPair.sol +++ b/src/ReservoirPair.sol @@ -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()); } } @@ -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; @@ -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( @@ -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); } diff --git a/src/libraries/Bytes32.sol b/src/libraries/Bytes32.sol index 553e2946..b428af10 100644 --- a/src/libraries/Bytes32.sol +++ b/src/libraries/Bytes32.sol @@ -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); } diff --git a/test/unit/OracleWriter.t.sol b/test/unit/OracleWriter.t.sol index 96b81f71..1a3b5f14 100644 --- a/test/unit/OracleWriter.t.sol +++ b/test/unit/OracleWriter.t.sol @@ -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"; @@ -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; @@ -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 { @@ -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)); @@ -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"); + } }