feat: introduce max change per trade

script/optimized-deployer-meta

@@ -1,6 +1,6 @@
 {
-  "constant_product_hash": "0x7b0f16c5cf2f4e78c4f697bfefb1365008f8865aaaa942f46ced4e0dea45c42a",
-  "factory_hash": "0xab6a26d7fb7b6df7e81d9f9354f8b767b5ac11b732f1b3f2e9d6e5b69a7d0754",
-  "oracle_caller_hash": "0x0d71d4a05b676bfbccd00e8422375c347a056fb93cc502cc962d5e6912213b04",
-  "stable_hash": "0xc5b9d3031f577510c547b9465241780111ad9d25a80dbe2292d1dc4290d9cd5a"
+  "constant_product_hash": "0x6bb54a3a2e0eef36a4ee1866eee21dc514fac57496141ce405f9af4e37f34941",
+  "factory_hash": "0x5d635090575f999ee44232af44309f6053dc93b7844bfdd90ee7a7919a4d363c",
+  "oracle_caller_hash": "0x529da7083c2ffd40e327dd40a4ae6f796de1561a0191b323d0cbfab827fab3b7",
+  "stable_hash": "0x51e93a48df3616a3e56c131684d126d7d3ddcd8911d5b807a773aca3a576439e"
 }

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%
 }

src/ReservoirDeployer.sol

@@ -17,12 +17,12 @@ contract ReservoirDeployer {
     uint256 public step = 0;
 
     // Bytecode hashes.
-    bytes32 public constant FACTORY_HASH = bytes32(0xab6a26d7fb7b6df7e81d9f9354f8b767b5ac11b732f1b3f2e9d6e5b69a7d0754);
+    bytes32 public constant FACTORY_HASH = bytes32(0x5d635090575f999ee44232af44309f6053dc93b7844bfdd90ee7a7919a4d363c);
     bytes32 public constant CONSTANT_PRODUCT_HASH =
-        bytes32(0x7b0f16c5cf2f4e78c4f697bfefb1365008f8865aaaa942f46ced4e0dea45c42a);
-    bytes32 public constant STABLE_HASH = bytes32(0xc5b9d3031f577510c547b9465241780111ad9d25a80dbe2292d1dc4290d9cd5a);
+        bytes32(0x6bb54a3a2e0eef36a4ee1866eee21dc514fac57496141ce405f9af4e37f34941);
+    bytes32 public constant STABLE_HASH = bytes32(0x51e93a48df3616a3e56c131684d126d7d3ddcd8911d5b807a773aca3a576439e);
     bytes32 public constant ORACLE_CALLER_HASH =
-        bytes32(0x0d71d4a05b676bfbccd00e8422375c347a056fb93cc502cc962d5e6912213b04);
+        bytes32(0x529da7083c2ffd40e327dd40a4ae6f796de1561a0191b323d0cbfab827fab3b7);
 
     // Deployment addresses.
     GenericFactory public factory;
@@ -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;

src/ReservoirPair.sol

@@ -49,7 +49,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());
         }
     }
 
@@ -514,18 +514,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;
 
@@ -542,10 +547,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(
@@ -557,19 +565,37 @@ 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
             // maxChangeRate <= 0.01e18 (50 bits)
             // aTimeElapsed <= 32 bits
             if (aCurrRawPrice > aPrevClampedPrice) {
-                rClampedPrice = aPrevClampedPrice.fullMulDiv(1e18 + maxChangeRate * aTimeElapsed, 1e18);
+                uint256 lRateLimitedPrice = aPrevClampedPrice.fullMulDiv(1e18 + maxChangeRate * aTimeElapsed, 1e18);
+                uint256 lPerTradeLimitedPrice = aPrevClampedPrice.fullMulDiv(1e18 + maxChangePerTrade, 1e18);
+                rClampedPrice = lRateLimitedPrice.min(lPerTradeLimitedPrice);
+                assert(rClampedPrice < aCurrRawPrice);
             } else {
-                assert(aPrevClampedPrice > aCurrRawPrice);
-                rClampedPrice = aPrevClampedPrice.fullMulDiv(1e18 - maxChangeRate * aTimeElapsed, 1e18);
+                // make sure that the time elapsed is not too long, else the subtraction from 1e18 will underflow
+                // if the time elapsed is too long, then we only depend on the per trade limited price
+                uint256 lChangeElapsed = maxChangeRate * aTimeElapsed;
+                if (lChangeElapsed > 1e18) {
+                    lChangeElapsed = 1e18;
+                }
+                uint256 lRateLimitedPrice = aPrevClampedPrice.fullMulDiv(1e18 - lChangeElapsed, 1e18);
+                // subtraction will not underflow as maxChangePerTrade is limited by MAX_CHANGE_PER_TRADE
+                uint256 lPerTradeLimitedPrice = aPrevClampedPrice.fullMulDiv(1e18 - maxChangePerTrade, 1e18);
+                rClampedPrice = lRateLimitedPrice.max(lPerTradeLimitedPrice);
+                assert(rClampedPrice > aCurrRawPrice);
             }
             rClampedLogPrice = LogCompression.toLowResLog(rClampedPrice);
         }

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);
     }

test/unit/OracleWriter.t.sol

@@ -2,6 +2,8 @@
 
 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";
@@ -12,9 +14,10 @@
 
 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;
@@ -135,33 +138,33 @@
         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 {
@@ -292,6 +295,35 @@
         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 raw price should be more negative than the clamped price
+    }
+
+    function testUpdateOracle_DecreasePrice_ExceedMaxChangeRate() external allPairs { }
+
+    function testUpdateOracle_DecreasePrice_ExceedMaxChangePerTrade() external allPairs { }
+
+    function testUpdateOracle_DecreasePrice_ExceedBothMaxChangeRateAndMaxChangePerTrade() external allPairs { }
+
     function testOracle_SameReservesDiffPrice(uint32 aNewStartTime) external randomizeStartTime(aNewStartTime) {
         // arrange
         ConstantProductPair lCP = ConstantProductPair(_createPair(address(_tokenB), address(_tokenC), 0));
@@ -414,28 +446,22 @@
     }
 
     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);
     }
-
 }