feat: introduce max change per trade

reservoir-labs · Mar 16, 2024 · 77ea2ab · 77ea2ab
1 parent 5a606bc
commit 77ea2ab
Show file tree

Hide file tree

Showing 5 changed files with 35 additions and 18 deletions.
diff --git 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
@@ -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
@@ -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,21 @@ 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;
     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 +545,12 @@ 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);
+
+        emit ClampParamsUpdated(aMaxChangeRate, aMaxChangePerTrade);
         maxChangeRate = aMaxChangeRate;
+        maxChangePerTrade = aMaxChangePerTrade;
     }
 
     function _calcClampedPrice(
@@ -557,8 +562,14 @@ 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

diff --git 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
@@ -14,7 +14,7 @@ contract OracleWriterTest is BaseTest {
     using FactoryStoreLib for GenericFactory;
 
     event OracleCallerUpdated(address oldCaller, address newCaller);
-    event MaxChangeRateUpdated(uint256 oldMaxChangeRate, uint256 newMaxChangeRate);
+    event ClampParamsUpdated(uint256 oldMaxChangeRate, uint256 newMaxChangeRate);
 
     ReservoirPair[] internal _pairs;
     ReservoirPair internal _pair;
@@ -135,33 +135,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);
+        emit ClampParamsUpdated(Constants.DEFAULT_MAX_CHANGE_RATE, 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 {