H-01 fix - Cancel order refactor (#2328)

0xMithrandir · web-flow · commit 2cb8235b52c6 · 2024-10-21T17:09:20.000-07:00
diff --git a/markets/bfp-market/contracts/modules/OrderModule.sol b/markets/bfp-market/contracts/modules/OrderModule.sol
@@ -76,6 +76,15 @@ contract OrderModule is IOrderModule {
         Position.TradeParams tradeParams;
     }
 
+    struct Runtime_cancelOrder {
+        uint128 accountId;
+        uint128 marketId;
+        bool isStale;
+        bool isReady;
+        bool isMarketSolvent;
+        Order.Data order;
+    }
+
     // --- Helpers --- //
 
     /// @dev Reverts when `fillPrice > limitPrice` when long or `fillPrice < limitPrice` when short.
@@ -472,56 +481,111 @@ contract OrderModule is IOrderModule {
     ) external payable {
         FeatureFlag.ensureAccessToFeature(Flags.CANCEL_ORDER);
         PerpMarket.Data storage market = PerpMarket.exists(marketId);
-        Account.Data storage account = Account.exists(accountId);
-        Order.Data storage order = market.orders[accountId];
+        Runtime_cancelOrder memory runtime;
 
-        // No order available to settle.
-        if (order.sizeDelta == 0) {
+        runtime.accountId = accountId;
+        runtime.marketId = marketId;
+        runtime.order = market.orders[accountId];
+
+        if (runtime.order.sizeDelta == 0) {
             revert ErrorUtil.OrderNotFound();
         }
 
         PerpMarketConfiguration.GlobalData storage globalConfig = PerpMarketConfiguration.load();
 
-        uint64 commitmentTime = order.commitmentTime;
-        (bool isStale, bool isReady) = isOrderStaleOrReady(commitmentTime, globalConfig);
+        (runtime.isStale, runtime.isReady) = isOrderStaleOrReady(
+            runtime.order.commitmentTime,
+            globalConfig
+        );
 
-        if (!isReady) {
+        if (!runtime.isReady) {
             revert ErrorUtil.OrderNotReady();
         }
 
-        // Only do the price divergence check for non stale orders. All stale orders are allowed to be canceled.
-        if (!isStale) {
-            PerpMarketConfiguration.Data storage marketConfig = PerpMarketConfiguration.load(
-                marketId
-            );
+        if (!runtime.isStale) {
+            validateNonStaleOrderCancellation(runtime, priceUpdateData);
+        }
 
-            // Order is within settlement window. Check if price tolerance has exceeded.
-            uint256 pythPrice = PythUtil.parsePythPrice(
-                globalConfig,
-                marketConfig,
-                commitmentTime,
-                priceUpdateData
-            );
-            uint256 fillPrice = Order.getFillPrice(
-                market.skew,
-                marketConfig.skewScale,
-                order.sizeDelta,
-                pythPrice
-            );
+        uint256 keeperFee = chargeKeeperFee(accountId, marketId);
 
-            if (!isPriceToleranceExceeded(order.sizeDelta, fillPrice, order.limitPrice)) {
-                revert ErrorUtil.PriceToleranceNotExceeded(
-                    order.sizeDelta,
-                    fillPrice,
-                    order.limitPrice
-                );
-            }
+        emit OrderCanceled(accountId, marketId, keeperFee, runtime.order.commitmentTime);
+        delete market.orders[accountId];
+    }
+
+    function validateNonStaleOrderCancellation(
+        Runtime_cancelOrder memory runtime,
+        bytes calldata priceUpdateData
+    ) private {
+        PerpMarketConfiguration.Data storage marketConfig = PerpMarketConfiguration.load(
+            runtime.marketId
+        );
+        PerpMarket.Data storage market = PerpMarket.exists(runtime.marketId);
+
+        uint256 pythPrice = PythUtil.parsePythPrice(
+            PerpMarketConfiguration.load(),
+            marketConfig,
+            runtime.order.commitmentTime,
+            priceUpdateData
+        );
+        uint256 fillPrice = Order.getFillPrice(
+            market.skew,
+            marketConfig.skewScale,
+            runtime.order.sizeDelta,
+            pythPrice
+        );
+        AddressRegistry.Data memory addresses = AddressRegistry.Data({
+            synthetix: ISynthetixSystem(SYNTHETIX_CORE),
+            sUsd: SYNTHETIX_SUSD,
+            oracleManager: ORACLE_MANAGER
+        });
+
+        Position.Data storage oldPosition = market.positions[runtime.accountId];
+
+        int128 newPositionSize = oldPosition.size + runtime.order.sizeDelta;
+
+        // lockedCreditDelta is the change in credit that would be locked if the order was filled
+        uint256 newMinCredit = PerpMarket.getMinimumCreditWithPositionSize(
+            market,
+            marketConfig,
+            market.getOraclePrice(addresses),
+            (MathUtil.abs(newPositionSize).toInt() - MathUtil.abs(oldPosition.size).toInt())
+                .to128(),
+            addresses
+        );
+
+        // checks if the market would be solvent with this new credit delta
+        runtime.isMarketSolvent = PerpMarket.isMarketSolventForCredit(
+            market,
+            newMinCredit,
+            market.depositedCollateral[addresses.sUsd],
+            addresses
+        );
+
+        // Allow to cancel if the cancellation is due to market insolvency while not reducing the order
+        // If not, check if fill price exceeded acceptable price
+        if (
+            (runtime.isMarketSolvent ||
+                MathUtil.isSameSideReducing(oldPosition.size, newPositionSize)) &&
+            !isPriceToleranceExceeded(runtime.order.sizeDelta, fillPrice, runtime.order.limitPrice)
+        ) {
+            revert ErrorUtil.PriceToleranceNotExceeded(
+                runtime.order.sizeDelta,
+                fillPrice,
+                runtime.order.limitPrice
+            );
         }
+    }
 
-        // If `isAccountOwner` then 0 else charge cancellation fee.
+    function chargeKeeperFee(
+        uint128 accountId,
+        uint128 marketId
+    ) private returns (uint256 keeperFee) {
+        Account.Data storage account = Account.exists(accountId);
+        PerpMarket.Data storage market = PerpMarket.exists(marketId);
 
-        uint256 keeperFee;
         if (ERC2771Context._msgSender() != account.rbac.owner) {
+            PerpMarketConfiguration.GlobalData storage globalConfig = PerpMarketConfiguration
+                .load();
             uint256 ethPrice = INodeModule(ORACLE_MANAGER)
                 .process(globalConfig.ethOracleNodeId)
                 .price
@@ -538,9 +602,6 @@ contract OrderModule is IOrderModule {
                 keeperFee
             );
         }
-
-        emit OrderCanceled(accountId, marketId, keeperFee, commitmentTime);
-        delete market.orders[accountId];
     }
 
     // --- Views --- //
diff --git a/markets/bfp-market/contracts/storage/PerpMarket.sol b/markets/bfp-market/contracts/storage/PerpMarket.sol
@@ -543,4 +543,18 @@ library PerpMarket {
 
         return totalValueUsd;
     }
+
+    function isMarketSolventForCredit(
+        Data storage self,
+        uint256 newMinCredit,
+        uint256 delegatedSusdValue,
+        AddressRegistry.Data memory addresses
+    ) internal view returns (bool isMarketSolvent) {
+        // establish amount of collateral currently collateralizing outstanding perp markets
+        int256 delegatedCollateralValue = getDelegatedCollateralValueUsd(self, addresses);
+        delegatedCollateralValue += delegatedSusdValue.toInt();
+
+        // Market insolvent delegatedCollateralValue < credit
+        isMarketSolvent = delegatedCollateralValue >= newMinCredit.toInt();
+    }
 }
diff --git a/markets/bfp-market/contracts/utils/MathUtil.sol b/markets/bfp-market/contracts/utils/MathUtil.sol
@@ -42,4 +42,8 @@ library MathUtil {
     function sameSide(int256 a, int256 b) internal pure returns (bool) {
         return (a == 0) || (b == 0) || (a > 0) == (b > 0);
     }
+
+    function isSameSideReducing(int128 a, int128 b) internal pure returns (bool) {
+        return sameSide(a, b) && abs(b) < abs(a);
+    }
 }
diff --git a/markets/bfp-market/storage.dump.json b/markets/bfp-market/storage.dump.json
@@ -227,6 +227,75 @@
           "slot": "6",
           "offset": 0
         }
+      ],
+      "Runtime_cancelOrder": [
+        {
+          "type": "uint128",
+          "name": "accountId",
+          "size": 16,
+          "slot": "0",
+          "offset": 0
+        },
+        {
+          "type": "uint128",
+          "name": "marketId",
+          "size": 16,
+          "slot": "0",
+          "offset": 16
+        },
+        {
+          "type": "bool",
+          "name": "isStale",
+          "size": 1,
+          "slot": "1",
+          "offset": 0
+        },
+        {
+          "type": "bool",
+          "name": "isReady",
+          "size": 1,
+          "slot": "1",
+          "offset": 1
+        },
+        {
+          "type": "bool",
+          "name": "isMarketSolvent",
+          "size": 1,
+          "slot": "1",
+          "offset": 2
+        },
+        {
+          "type": "struct",
+          "name": "order",
+          "members": [
+            {
+              "type": "int128",
+              "name": "sizeDelta"
+            },
+            {
+              "type": "uint64",
+              "name": "commitmentTime"
+            },
+            {
+              "type": "uint256",
+              "name": "limitPrice"
+            },
+            {
+              "type": "uint128",
+              "name": "keeperFeeBufferUsd"
+            },
+            {
+              "type": "array",
+              "name": "hooks",
+              "value": {
+                "type": "address"
+              }
+            }
+          ],
+          "size": 128,
+          "slot": "2",
+          "offset": 0
+        }
       ]
     }
   },
diff --git a/markets/bfp-market/test/integration/modules/OrderModule.cancel.test.ts b/markets/bfp-market/test/integration/modules/OrderModule.cancel.test.ts
@@ -27,11 +27,12 @@ import {
   isSusdCollateral,
   setBaseFeePerGas,
   setMarketConfiguration,
+  setMarketConfigurationById,
   withExplicitEvmMine,
 } from '../../helpers';
 import { calcKeeperCancellationFee } from '../../calculations';
 
-describe('OrderModule Cancelations', () => {
+describe('OrderModule Cancellations', () => {
   const bs = bootstrap(genBootstrap());
   const { systems, restore, provider, keeper, traders, collateralsWithoutSusd } = bs;
 
@@ -524,4 +525,53 @@ describe('OrderModule Cancelations', () => {
       );
     });
   });
+
+  describe('cancel order due to market insolvency', () => {
+    it('reverts if trying to cancel an order when market is solvent', async () => {
+      const { BfpMarketProxy } = systems();
+
+      const { trader, marketId, market, collateral, collateralDepositAmount } = await depositMargin(
+        bs,
+        genTrader(bs)
+      );
+
+      const order = await genOrder(bs, market, collateral, collateralDepositAmount);
+      await commitOrder(bs, marketId, trader, order);
+
+      // fast forward to settlement
+      const { publishTime, settlementTime } = await getFastForwardTimestamp(bs, marketId, trader);
+      await fastForwardTo(settlementTime, provider());
+
+      const { updateData } = await getPythPriceDataByMarketId(bs, marketId, publishTime);
+
+      await assertRevert(
+        BfpMarketProxy.connect(keeper()).cancelOrder(trader.accountId, marketId, updateData),
+        `PriceToleranceNotExceeded("${order.sizeDelta}", "${order.fillPrice}", "${order.limitPrice}")`
+      );
+    });
+
+    it('allows to cancel an order if the market is insolvent', async () => {
+      const { BfpMarketProxy } = systems();
+      const { trader, marketId, market, collateral, collateralDepositAmount } = await depositMargin(
+        bs,
+        genTrader(bs)
+      );
+
+      const order = await genOrder(bs, market, collateral, collateralDepositAmount, {
+        desiredSize: bn(1),
+      });
+      await commitOrder(bs, marketId, trader, order);
+
+      // fast forward to settlement
+      const { publishTime, settlementTime } = await getFastForwardTimestamp(bs, marketId, trader);
+      await fastForwardTo(settlementTime, provider());
+
+      await setMarketConfigurationById(bs, marketId, {
+        minCreditPercent: bn(1000),
+      });
+
+      const { updateData } = await getPythPriceDataByMarketId(bs, marketId, publishTime);
+      await BfpMarketProxy.connect(keeper()).cancelOrder(trader.accountId, marketId, updateData);
+    });
+  });
 });

Original file line number	Diff line number	Diff line change
`@@ -42,4 +42,8 @@ library MathUtil {`
`42`	`42`	`function sameSide(int256 a, int256 b) internal pure returns (bool) {`
`43`	`43`	`return (a == 0) \|\| (b == 0) \|\| (a > 0) == (b > 0);`
`44`	`44`	`}`
	`45`	`+`
	`46`	`+ function isSameSideReducing(int128 a, int128 b) internal pure returns (bool) {`
	`47`	`+ return sameSide(a, b) && abs(b) < abs(a);`
	`48`	`+ }`
`45`	`49`	`}`