significantly reduce gas costs of dispute + fix a bug

src/ClvrHook.sol

@@ -291,9 +291,9 @@ contract ClvrHook is BaseHook, ClvrStake, ClvrSlashing {
     /// @param key The pool key
     /// @return reserve0 The current reserve of the first currency
     /// @return reserve1 The current reserve of the second currency
-    function getCurrentReserves(PoolKey calldata key) view public returns (uint256, uint256) {
+    function getCurrentReserves(PoolKey calldata key) view public returns (uint256, uint256) { // TODO: think about this when decimals of two tokens are different
         uint256 currentPrice = getCurrentPrice(key);
-        uint256 reserve0 = currentPrice * 1e18;
+        uint256 reserve0 = currentPrice;
         uint256 reserve1 = 1e18;
 
         return (reserve0, reserve1);

src/ClvrModel.sol

@@ -35,44 +35,65 @@ contract ClvrModel {
     }
 
     // PUBLIC FUNCTIONS
-
+    
     /// @notice Checks if the candidate ordering is better than the challenged ordering
     /// @notice Candidate ordering is better if the CLVR value is lower at any step
     /// @param p0 The initial price
+    /// @param reserve_x The initial reserve x
+    /// @param reserve_y The initial reserve y
     /// @param challengedOrdering The ordering to be challenged
     /// @param candidateOrdering The ordering to be checked
     /// @return True if the candidate ordering is better, false otherwise
     function isBetterOrdering(
         uint256 p0, 
-        uint256 reserve_x,
         uint256 reserve_y,
+        uint256 reserve_x,
         ClvrHook.SwapParamsExtended[] memory challengedOrdering, 
         ClvrHook.SwapParamsExtended[] memory candidateOrdering
     ) 
         public 
         equalLength(challengedOrdering, candidateOrdering) 
         returns (bool) 
     {
-        set_reserve_x(reserve_x);
         set_reserve_y(reserve_y);
+        set_reserve_x(reserve_x);
 
         challengedOrdering = addMockTrade(challengedOrdering);
         candidateOrdering = addMockTrade(candidateOrdering);
 
-        int256 lnP0 = p0.lnU256().toInt256();
+        int256 ln_p0 = p0.lnU256().toInt256();
+
+        uint256[] memory challengedVolatility = new uint256[](challengedOrdering.length);
+        uint256[] memory candidateVolatility = new uint256[](candidateOrdering.length);
+
+        uint256 cachedY = reserveY;
+        uint256 cachedX = reserveX;
+
         for (uint256 i = 1; i < challengedOrdering.length; i++) {
-            int256 unsquaredChallengedValue = lnP0 - P(challengedOrdering, i).lnU256().toInt256();
-            int256 challengedValue = unsquaredChallengedValue ** 2 / 1e18;
-            // console.log(P(challengedOrdering, i), P(candidateOrdering, i));
-            console.log(challengedOrdering[i].params.zeroForOne ? "buy" : "sell", challengedOrdering[i].params.amountSpecified);
-            console.log(candidateOrdering[i].params.zeroForOne ? "buy" : "sell", candidateOrdering[i].params.amountSpecified);
-
-
-            int256 unsquaredCandidateValue = lnP0 - P(candidateOrdering, i).lnU256().toInt256();
-            int256 candidateValue = unsquaredCandidateValue ** 2 / 1e18;
-
-            // the candidate ordering is better because the value is lower
-            if (candidateValue < challengedValue) {
+            (uint256 p, uint256 cached_y, uint256 cached_x) = P_cached(challengedOrdering, i, cachedY, cachedX);
+            int256 ln_p = p.lnU256().toInt256();
+            int256 diff = ln_p0 - ln_p;
+            challengedVolatility[i] = uint256(diff ** 2 / 1e18);
+
+            cachedY = cached_y;
+            cachedX = cached_x;
+        }
+
+        cachedY = reserveY;
+        cachedX = reserveX;
+
+        for (uint256 i = 1; i < candidateOrdering.length; i++) {
+            (uint256 p, uint256 cached_y, uint256 cached_x) = P_cached(candidateOrdering, i, cachedY, cachedX);
+            int256 ln_p = p.lnU256().toInt256();
+            int256 diff = ln_p0 - ln_p;
+            candidateVolatility[i] = uint256(diff ** 2 / 1e18);
+
+            cachedY = cached_y;
+            cachedX = cached_x;
+        }
+
+        for (uint256 i = 0; i < challengedOrdering.length; i++) {
+            if (candidateVolatility[i] < challengedVolatility[i]) {
                 return true;
             }
         }
@@ -191,8 +212,11 @@ contract ClvrModel {
         revert("Invalid call to X_cached");
     }
 
-    function P_cached(ClvrHook.SwapParamsExtended[] memory o, uint256 i, uint256 cachedY, uint256 cachedX) private view returns (uint256) {
-        return Y_cached(o, i, cachedY, cachedX) * 1e18 / X_cached(o, i, cachedY, cachedX);
+    /// @notice Returns the price, the new y and the new x after the swap
+    function P_cached(ClvrHook.SwapParamsExtended[] memory o, uint256 i, uint256 cachedY, uint256 cachedX) private view returns (uint256, uint256, uint256) {
+        uint256 y_cached_new = Y_cached(o, i, cachedY, cachedX);
+        uint256 x_cached_new = X_cached(o, i, cachedY, cachedX);
+        return (y_cached_new * 1e18 / x_cached_new, y_cached_new, x_cached_new);
     }
 
     function direction(ClvrHook.SwapParamsExtended memory o) private pure returns (Direction) {

src/ClvrSlashing.sol

@@ -5,6 +5,7 @@ import {PoolKey} from "@uniswap/v4-core/src/types/PoolKey.sol";
 import {PoolId} from "@uniswap/v4-core/src/types/PoolId.sol";
 import {ClvrHook} from "./ClvrHook.sol";
 import {ClvrModel} from "./ClvrModel.sol";
+import { console } from "forge-std/console.sol";
 
 /// @title ClvrSlashing
 /// @author Ruslan Akhtariev
@@ -17,11 +18,18 @@ contract ClvrSlashing {
     /// @param disputer The address of the disputer
     event BatchDisputed(PoolId indexed poolId, uint256 batchIndex, address creator, address disputer);
 
+    /// @notice Emitted when a batch is not successfully disputed
+    /// @param poolId The pool ID
+    /// @param batchIndex The index of the batch
+    /// @param creator The address of the creator of the batch
+    /// @param disputer The address of the disputer
+    event BatchNotDisputed(PoolId indexed poolId, uint256 batchIndex, address creator, address disputer);
+
     /// @notice How many batches are simultaneously kept in memory as a graceful period for slashing
     uint256 public constant BATCH_RETENTION_PERIOD = 5;
 
     /// @notice Magic value to return when a batch is disputed successfully
-    bytes4 public constant BATCH_DISPUTED_MAGIC_VALUE = bytes4(keccak256("BATCH_DISPUTED"));
+    bytes4 public constant BATCH_DISPUTED_MAGIC_VALUE = bytes4(keccak256("BATCH_DISPUTED_MAGIC_VALUE"));
 
     /// @notice A struct to store a batch of swaps
     /// @param creator The address of the creator of the batch
@@ -74,7 +82,7 @@ contract ClvrSlashing {
         require(batchIndex < BATCH_RETENTION_PERIOD, "Batch index out of bounds");
         require(!retainedBatches[key.toId()][batchIndex].disputed, "Batch already disputed");
 
-        RetainedBatch storage batch = retainedBatches[key.toId()][batchIndex];
+        RetainedBatch memory batch = retainedBatches[key.toId()][batchIndex];
         uint256 batchSize = batch.swaps.length;
 
         require(betterReordering.length == batchSize, "Invalid reordering length");
@@ -85,13 +93,15 @@ contract ClvrSlashing {
         }
 
         if (model.isBetterOrdering(batch.p0, batch.reserveX, batch.reserveY, batch.swaps, suggestedOrdering)) {
-            batch.disputed = true;
+            retainedBatches[key.toId()][batchIndex].disputed = true;
 
             emit BatchDisputed(key.toId(), batchIndex, batch.creator, msg.sender);
 
             return BATCH_DISPUTED_MAGIC_VALUE;
         }
 
+        emit BatchNotDisputed(key.toId(), batchIndex, batch.creator, msg.sender);
+
         return bytes4(0);
     }
 }

test/ClvrHook.t.sol

@@ -216,7 +216,7 @@ contract ClvrHookTest is Test, Deployers, Fixtures {
 
         uint256[] memory badOrdering = new uint256[](USERS_LENGTH);
 
-        // execute even-indexed swaps first (sell direction), then odd-indexed swaps (buy direction)
+        // bad ordering is all buys, then all sells
         for (uint256 i = 0; i < USERS_LENGTH/2; i++) {
             badOrdering[i] = 2 * i;
         }
@@ -227,12 +227,36 @@ contract ClvrHookTest is Test, Deployers, Fixtures {
 
         executeBatch(abi.encode(badOrdering));
 
-        uint256[] memory betterOrdering = getSwapIds();
+        // better ordering is alternating ordering
+        uint256[] memory betterOrdering = new uint256[](USERS_LENGTH);
+        for (uint256 i = 0; i < USERS_LENGTH/2; i++) {
+            betterOrdering[2*i] = i;
+            betterOrdering[2*i + 1] = USERS_LENGTH/2 + i;
+        }
 
         address disputer = makeAddr("Disputer");
+        uint256 hookBalanceBeforeDispute = address(hook).balance;
+        uint256 batchIndex = hook.BATCH_RETENTION_PERIOD() - 1;
+
+        uint256 gas = gasleft();
 
         vm.startPrank(disputer, disputer);
-        hook.disputeBatch(key, hook.BATCH_RETENTION_PERIOD() - 1, betterOrdering);
+        hook.disputeBatch(key, batchIndex, betterOrdering);
+        vm.stopPrank();
+
+        if (DEBUG) {
+            console.log("Gas used in dispute: ", gas - gasleft(), ", approximately $", gasToDollars(gas - gasleft()));
+        }
+
+        require(address(hook).balance < hookBalanceBeforeDispute, "Hook should pay stake to disputer");
+        require(!hook.isStakedScheduler(key, scheduler), "Scheduler should not be staked after valid dispute");
+        require(address(disputer).balance == 1 ether, "Disputer should receive 1 ether");
+
+        vm.startPrank(disputer, disputer);
+
+        vm.expectRevert(); // should revert because batch is already disputed
+        hook.disputeBatch(key, batchIndex, betterOrdering);
+
         vm.stopPrank();
     }