Prevent Collateral updates with pending orders

markets/perps-market/contracts/interfaces/IAsyncOrderModule.sol

@@ -47,11 +47,6 @@ interface IAsyncOrderModule {
         uint256 acceptablePrice
     );
 
-    /**
-     * @notice Gets thrown when commit order is called when a pending order already exists.
-     */
-    error OrderAlreadyCommitted(uint128 marketId, uint128 accountId);
-
     /**
      * @notice Commit an async order via this function
      * @param commitment Order commitment data (see AsyncOrder.OrderCommitmentRequest struct).

markets/perps-market/contracts/modules/AsyncOrderModule.sol

@@ -6,6 +6,7 @@ import {DecimalMath} from "@synthetixio/core-contracts/contracts/utils/DecimalMa
 import {Account} from "@synthetixio/main/contracts/storage/Account.sol";
 import {AccountRBAC} from "@synthetixio/main/contracts/storage/AccountRBAC.sol";
 import {IPythVerifier} from "../interfaces/external/IPythVerifier.sol";
+import {IAccountModule} from "../interfaces/IAccountModule.sol";
 import {IAsyncOrderModule} from "../interfaces/IAsyncOrderModule.sol";
 import {PerpsAccount, SNX_USD_MARKET_ID} from "../storage/PerpsAccount.sol";
 import {MathUtil} from "../utils/MathUtil.sol";
@@ -37,14 +38,15 @@ contract AsyncOrderModule is IAsyncOrderModule {
     using Position for Position.Data;
     using SafeCastU256 for uint256;
     using SafeCastI256 for int256;
+    using PerpsAccount for PerpsAccount.Data;
 
     /**
      * @inheritdoc IAsyncOrderModule
      */
     function commitOrder(
         AsyncOrder.OrderCommitmentRequest memory commitment
     ) external override returns (AsyncOrder.Data memory retOrder, uint fees) {
-        PerpsMarket.Data storage market = PerpsMarket.loadValid(commitment.marketId);
+        PerpsMarket.loadValid(commitment.marketId);
 
         // Check if commitment.accountId is valid
         Account.exists(commitment.accountId);
@@ -57,11 +59,10 @@ contract AsyncOrderModule is IAsyncOrderModule {
 
         GlobalPerpsMarket.load().checkLiquidation(commitment.accountId);
 
-        AsyncOrder.Data storage order = market.asyncOrders[commitment.accountId];
-
-        if (order.sizeDelta != 0) {
-            revert OrderAlreadyCommitted(commitment.marketId, commitment.accountId);
-        }
+        AsyncOrder.Data storage order = AsyncOrder.createValid(
+            commitment.accountId,
+            commitment.marketId
+        );
 
         SettlementStrategy.Data storage strategy = PerpsMarketConfiguration
             .load(commitment.marketId)
@@ -97,21 +98,26 @@ contract AsyncOrderModule is IAsyncOrderModule {
     function getOrder(
         uint128 marketId,
         uint128 accountId
-    ) public view override returns (AsyncOrder.Data memory) {
-        return PerpsMarket.loadValid(marketId).asyncOrders[accountId];
+    ) external view override returns (AsyncOrder.Data memory order) {
+        order = AsyncOrder.load(accountId);
+        if (order.marketId != marketId) {
+            // return emtpy order if marketId does not match
+            order = AsyncOrder.Data(0, 0, 0, 0, 0, 0, 0);
+        }
     }
 
     /**
      * @inheritdoc IAsyncOrderModule
      */
     function cancelOrder(uint128 marketId, uint128 accountId) external override {
-        AsyncOrder.Data storage order = PerpsMarket.loadValid(marketId).asyncOrders[accountId];
-        order.checkValidity();
+        AsyncOrder.Data storage order = AsyncOrder.loadValid(accountId, marketId);
+
         SettlementStrategy.Data storage settlementStrategy = PerpsMarketConfiguration
             .load(marketId)
             .settlementStrategies[order.settlementStrategyId];
         order.checkCancellationEligibility(settlementStrategy);
         order.reset();
+
         emit OrderCanceled(marketId, accountId, order.settlementTime, order.acceptablePrice);
     }
 }

markets/perps-market/contracts/modules/AsyncOrderSettlementModule.sol

@@ -203,12 +203,12 @@ contract AsyncOrderSettlementModule is IAsyncOrderSettlementModule, IMarketEvent
         uint128 marketId,
         uint128 accountId
     ) private view returns (AsyncOrder.Data storage, SettlementStrategy.Data storage) {
-        AsyncOrder.Data storage order = PerpsMarket.loadValid(marketId).asyncOrders[accountId];
+        AsyncOrder.Data storage order = AsyncOrder.loadValid(accountId, marketId);
+
         SettlementStrategy.Data storage settlementStrategy = PerpsMarketConfiguration
             .load(marketId)
             .settlementStrategies[order.settlementStrategyId];
 
-        order.checkValidity();
         order.checkWithinSettlementWindow(settlementStrategy);
 
         return (order, settlementStrategy);

markets/perps-market/contracts/modules/PerpsAccountModule.sol

@@ -54,6 +54,8 @@ contract PerpsAccountModule is IAccountModule {
             account.id = accountId;
         }
 
+        account.checkPendingOrder();
+
         ITokenModule synth = synthMarketId == 0
             ? perpsMarketFactory.usdToken
             : ITokenModule(perpsMarketFactory.spotMarket.getSynth(synthMarketId));

markets/perps-market/contracts/storage/AsyncOrder.sol

@@ -37,9 +37,19 @@ library AsyncOrder {
         uint256 settlementExpiration
     );
 
+    error AcceptablePriceExceeded(uint256 acceptablePrice, uint256 fillPrice);
+
     error OrderNotValid();
 
-    error AcceptablePriceExceeded(uint256 acceptablePrice, uint256 fillPrice);
+    /**
+     * @notice Gets thrown when commit order is called when a pending order already exists.
+     */
+    error OrderAlreadyCommitted(uint128 marketId, uint128 accountId);
+
+    /**
+     * @notice Gets thrown when pending orders exist and attempts to modify collateral.
+     */
+    error PendingOrderExist();
 
     struct Data {
         uint128 accountId;
@@ -60,6 +70,46 @@ library AsyncOrder {
         bytes32 trackingCode;
     }
 
+    function load(uint128 accountId) internal pure returns (Data storage order) {
+        bytes32 s = keccak256(abi.encode("io.synthetix.perps-market.AsyncOrder", accountId));
+
+        assembly {
+            order.slot := s
+        }
+    }
+
+    /**
+     * @dev Reverts if the order does not belongs to the market or not exists. Otherwise, returns the order.
+     * @dev non-existent order is considered an order with sizeDelta == 0.
+     */
+    function loadValid(
+        uint128 accountId,
+        uint128 marketId
+    ) internal view returns (Data storage order) {
+        order = load(accountId);
+        if (order.marketId != marketId || order.sizeDelta == 0) {
+            revert OrderNotValid();
+        }
+    }
+
+    /**
+     * @dev Reverts if the order does not belongs to the market or not exists. Otherwise, returns the order.
+     * @dev non-existent order is considered an order with sizeDelta == 0.
+     */
+    function createValid(
+        uint128 accountId,
+        uint128 marketId
+    ) internal view returns (Data storage order) {
+        order = load(accountId);
+        if (order.sizeDelta != 0 && order.marketId == marketId) {
+            revert OrderAlreadyCommitted(marketId, accountId);
+        }
+
+        if (order.sizeDelta != 0) {
+            revert PendingOrderExist();
+        }
+    }
+
     function update(
         Data storage self,
         OrderCommitmentRequest memory commitment,
@@ -110,12 +160,6 @@ library AsyncOrder {
         }
     }
 
-    function checkValidity(Data storage self) internal view {
-        if (self.sizeDelta == 0) {
-            revert OrderNotValid();
-        }
-    }
-
     error ZeroSizeOrder();
     error InsufficientMargin(int availableMargin, uint minMargin);
 

markets/perps-market/contracts/storage/PerpsAccount.sol

@@ -9,6 +9,7 @@ import {Position} from "./Position.sol";
 import {PerpsMarket} from "./PerpsMarket.sol";
 import {MathUtil} from "../utils/MathUtil.sol";
 import {PerpsPrice} from "./PerpsPrice.sol";
+import {AsyncOrder} from "./AsyncOrder.sol";
 import {PerpsMarketFactory} from "./PerpsMarketFactory.sol";
 import {GlobalPerpsMarket} from "./GlobalPerpsMarket.sol";
 import {GlobalPerpsMarketConfiguration} from "./GlobalPerpsMarketConfiguration.sol";
@@ -34,10 +35,13 @@ library PerpsAccount {
     using DecimalMath for uint256;
 
     struct Data {
-        // synth marketId => amount
+        // @dev synth marketId => amount
         mapping(uint128 => uint256) collateralAmounts;
+        // @dev account Id
         uint128 id;
+        // @dev set of active collateral types. By active we mean collateral types that have a non-zero amount
         SetUtil.UintSet activeCollateralTypes;
+        // @dev set of open position market ids
         SetUtil.UintSet openPositionMarketIds;
     }
 
@@ -89,6 +93,14 @@ library PerpsAccount {
         }
     }
 
+    function checkPendingOrder(Data storage self) internal {
+        // Check if there are pending orders
+        AsyncOrder.Data memory asyncOrder = AsyncOrder.load(self.id);
+        if (asyncOrder.sizeDelta != 0) {
+            revert AsyncOrder.PendingOrderExist();
+        }
+    }
+
     function addCollateralAmount(
         Data storage self,
         uint128 synthMarketId,

markets/perps-market/contracts/storage/PerpsMarket.sol

@@ -45,8 +45,6 @@ library PerpsMarket {
         // liquidation data
         uint128 lastTimeLiquidationCapacityUpdated;
         uint128 lastUtilizedLiquidationCapacity;
-        // accountId => asyncOrder
-        mapping(uint => AsyncOrder.Data) asyncOrders;
         // accountId => position
         mapping(uint => Position.Data) positions;
     }

markets/perps-market/storage.dump.sol

@@ -500,6 +500,12 @@ library AsyncOrder {
         Position.Data newPosition;
         bytes32 trackingCode;
     }
+    function load(uint128 accountId) internal pure returns (Data storage order) {
+        bytes32 s = keccak256(abi.encode("io.synthetix.perps-market.AsyncOrder", accountId));
+        assembly {
+            order.slot := s
+        }
+    }
 }
 
 // @custom:artifact contracts/storage/GlobalPerpsMarket.sol:GlobalPerpsMarket
@@ -586,7 +592,6 @@ library PerpsMarket {
         uint256 lastFundingTime;
         uint128 lastTimeLiquidationCapacityUpdated;
         uint128 lastUtilizedLiquidationCapacity;
-        mapping(uint => AsyncOrder.Data) asyncOrders;
         mapping(uint => Position.Data) positions;
     }
     struct MarketUpdateData {