diff --git a/markets/perps-market/contracts/modules/AsyncOrderModule.sol b/markets/perps-market/contracts/modules/AsyncOrderModule.sol index 8b66eabc99..ed64dacb1a 100644 --- a/markets/perps-market/contracts/modules/AsyncOrderModule.sol +++ b/markets/perps-market/contracts/modules/AsyncOrderModule.sol @@ -38,6 +38,7 @@ contract AsyncOrderModule is IAsyncOrderModule { using Position for Position.Data; using SafeCastU256 for uint256; using SafeCastI256 for int256; + using PerpsAccount for PerpsAccount.Data; /** * @inheritdoc IAsyncOrderModule @@ -45,7 +46,7 @@ contract AsyncOrderModule is 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); @@ -58,12 +59,16 @@ contract AsyncOrderModule is IAsyncOrderModule { GlobalPerpsMarket.load().checkLiquidation(commitment.accountId); - AsyncOrder.Data storage order = market.asyncOrders[commitment.accountId]; + AsyncOrder.Data storage order = PerpsAccount.load(commitment.accountId).asyncOrder[0]; - if (order.sizeDelta != 0) { + if (order.sizeDelta != 0 && order.marketId == commitment.marketId) { revert OrderAlreadyCommitted(commitment.marketId, commitment.accountId); } + if (order.sizeDelta != 0) { + revert IAccountModule.PendingOrdersExist(); + } + SettlementStrategy.Data storage strategy = PerpsMarketConfiguration .load(commitment.marketId) .loadValidSettlementStrategy(commitment.settlementStrategyId); @@ -76,11 +81,6 @@ contract AsyncOrderModule is IAsyncOrderModule { PerpsPrice.getCurrentPrice(commitment.marketId) ); - // Check if there is a pending orders - if (PerpsAccount.load(commitment.accountId).hasPendingOrders) { - revert IAccountModule.PendingOrdersExist(); - } - PerpsAccount.load(commitment.accountId).addPendingOrder(); // TODO include fees in event @@ -105,16 +105,21 @@ 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) { + AsyncOrder.Data memory order = PerpsAccount.load(accountId).asyncOrder[0]; + if (order.marketId != marketId) { + // return emtpy order if marketId does not match + return AsyncOrder.Data(0, 0, 0, 0, 0, 0, 0); + } + return order; } /** * @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 = PerpsAccount.load(accountId).getValidOrder(marketId); + SettlementStrategy.Data storage settlementStrategy = PerpsMarketConfiguration .load(marketId) .settlementStrategies[order.settlementStrategyId]; diff --git a/markets/perps-market/contracts/modules/AsyncOrderSettlementModule.sol b/markets/perps-market/contracts/modules/AsyncOrderSettlementModule.sol index a3ec05046e..57c78b5ab9 100644 --- a/markets/perps-market/contracts/modules/AsyncOrderSettlementModule.sol +++ b/markets/perps-market/contracts/modules/AsyncOrderSettlementModule.sol @@ -205,12 +205,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 = PerpsAccount.load(accountId).getValidOrder(marketId); + SettlementStrategy.Data storage settlementStrategy = PerpsMarketConfiguration .load(marketId) .settlementStrategies[order.settlementStrategyId]; - order.checkValidity(); order.checkWithinSettlementWindow(settlementStrategy); return (order, settlementStrategy); diff --git a/markets/perps-market/contracts/storage/AsyncOrder.sol b/markets/perps-market/contracts/storage/AsyncOrder.sol index 5697824b86..18c902bab2 100644 --- a/markets/perps-market/contracts/storage/AsyncOrder.sol +++ b/markets/perps-market/contracts/storage/AsyncOrder.sol @@ -37,8 +37,6 @@ library AsyncOrder { uint256 settlementExpiration ); - error OrderNotValid(); - error AcceptablePriceExceeded(uint256 acceptablePrice, uint256 fillPrice); struct Data { @@ -110,12 +108,6 @@ library AsyncOrder { } } - function checkValidity(Data storage self) internal view { - if (self.sizeDelta == 0) { - revert OrderNotValid(); - } - } - error ZeroSizeOrder(); error InsufficientMargin(int availableMargin, uint minMargin); diff --git a/markets/perps-market/contracts/storage/PerpsAccount.sol b/markets/perps-market/contracts/storage/PerpsAccount.sol index 1c36caf4f3..217649d33c 100644 --- a/markets/perps-market/contracts/storage/PerpsAccount.sol +++ b/markets/perps-market/contracts/storage/PerpsAccount.sol @@ -6,6 +6,7 @@ import {SafeCastI256, SafeCastU256, SafeCastU128} from "@synthetixio/core-contra import {SetUtil} from "@synthetixio/core-contracts/contracts/utils/SetUtil.sol"; import {ISpotMarketSystem} from "../interfaces/external/ISpotMarketSystem.sol"; import {Position} from "./Position.sol"; +import {AsyncOrder} from "./AsyncOrder.sol"; import {PerpsMarket} from "./PerpsMarket.sol"; import {MathUtil} from "../utils/MathUtil.sol"; import {PerpsPrice} from "./PerpsPrice.sol"; @@ -40,6 +41,8 @@ library PerpsAccount { bool hasPendingOrders; SetUtil.UintSet activeCollateralTypes; SetUtil.UintSet openPositionMarketIds; + // @dev using a mapping to isolate storage layout from the rest of the struct. Id is 0 always + mapping(uint128 => AsyncOrder.Data) asyncOrder; } error InsufficientCollateralAvailableForWithdraw(uint available, uint required); @@ -48,6 +51,8 @@ library PerpsAccount { error AccountLiquidatable(uint128 accountId); + error OrderNotValid(); + function load(uint128 id) internal pure returns (Data storage account) { bytes32 s = keccak256(abi.encode("io.synthetix.perps-market.Account", id)); @@ -56,6 +61,17 @@ library PerpsAccount { } } + function getValidOrder( + Data storage self, + uint marketId + ) internal view returns (AsyncOrder.Data storage) { + AsyncOrder.Data storage order = self.asyncOrder[0]; + if (order.marketId != marketId || order.sizeDelta == 0) { + revert OrderNotValid(); + } + return order; + } + function isEligibleForLiquidation( Data storage self ) diff --git a/markets/perps-market/contracts/storage/PerpsMarket.sol b/markets/perps-market/contracts/storage/PerpsMarket.sol index 559d8003db..7a16f15ebe 100644 --- a/markets/perps-market/contracts/storage/PerpsMarket.sol +++ b/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; } diff --git a/markets/perps-market/test/integration/Orders/OffchainAsyncOrder.settle.test.ts b/markets/perps-market/test/integration/Orders/OffchainAsyncOrder.settle.test.ts index e628bc8a96..9c4f110f1a 100644 --- a/markets/perps-market/test/integration/Orders/OffchainAsyncOrder.settle.test.ts +++ b/markets/perps-market/test/integration/Orders/OffchainAsyncOrder.settle.test.ts @@ -45,7 +45,7 @@ describe('Settle Offchain Async Order test', () => { it('reverts if market id is incorrect', async () => { await assertRevert( systems().PerpsMarket.connect(trader1()).settle(1337, 2), - 'InvalidMarket("1337")' + 'OrderNotValid()' ); }); @@ -88,7 +88,7 @@ describe('Settle Offchain Async Order test', () => { systems() .PerpsMarket.connect(keeper()) .settlePythOrder(pythPriceData, extraData, { value: updateFee }), - 'InvalidMarket("1337")' + 'OrderNotValid()' ); });