diff --git a/markets/perps-market/contracts/interfaces/IAsyncOrderModule.sol b/markets/perps-market/contracts/interfaces/IAsyncOrderModule.sol index a3478f0d8f..2b7cbd7ff7 100644 --- a/markets/perps-market/contracts/interfaces/IAsyncOrderModule.sol +++ b/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). diff --git a/markets/perps-market/contracts/modules/AsyncOrderModule.sol b/markets/perps-market/contracts/modules/AsyncOrderModule.sol index 8111d75cda..6941542fe7 100644 --- a/markets/perps-market/contracts/modules/AsyncOrderModule.sol +++ b/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,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 @@ -44,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); @@ -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); } } diff --git a/markets/perps-market/contracts/modules/AsyncOrderSettlementModule.sol b/markets/perps-market/contracts/modules/AsyncOrderSettlementModule.sol index c1e83527f0..e9aad91201 100644 --- a/markets/perps-market/contracts/modules/AsyncOrderSettlementModule.sol +++ b/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); diff --git a/markets/perps-market/contracts/modules/PerpsAccountModule.sol b/markets/perps-market/contracts/modules/PerpsAccountModule.sol index 2f04d7e763..989faaa267 100644 --- a/markets/perps-market/contracts/modules/PerpsAccountModule.sol +++ b/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)); diff --git a/markets/perps-market/contracts/storage/AsyncOrder.sol b/markets/perps-market/contracts/storage/AsyncOrder.sol index 5697824b86..e570925650 100644 --- a/markets/perps-market/contracts/storage/AsyncOrder.sol +++ b/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); diff --git a/markets/perps-market/contracts/storage/PerpsAccount.sol b/markets/perps-market/contracts/storage/PerpsAccount.sol index 7833f6a771..c71462edf3 100644 --- a/markets/perps-market/contracts/storage/PerpsAccount.sol +++ b/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, 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/storage.dump.sol b/markets/perps-market/storage.dump.sol index 263bcb9c0d..66c6e48d88 100644 --- a/markets/perps-market/storage.dump.sol +++ b/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 { diff --git a/markets/perps-market/test/integration/Orders/OffchainAsyncOrder.pending.test.ts b/markets/perps-market/test/integration/Orders/OffchainAsyncOrder.pending.test.ts new file mode 100644 index 0000000000..1421f549c7 --- /dev/null +++ b/markets/perps-market/test/integration/Orders/OffchainAsyncOrder.pending.test.ts @@ -0,0 +1,184 @@ +import { ethers } from 'ethers'; +import { DEFAULT_SETTLEMENT_STRATEGY, bn, bootstrapMarkets } from '../bootstrap'; +import { fastForwardTo } from '@synthetixio/core-utils/utils/hardhat/rpc'; +import { snapshotCheckpoint } from '@synthetixio/core-utils/utils/mocha/snapshot'; +import { depositCollateral, settleOrder } from '../helpers'; +import assertBn from '@synthetixio/core-utils/utils/assertions/assert-bignumber'; +import assertRevert from '@synthetixio/core-utils/utils/assertions/assert-revert'; +import { getTxTime } from '@synthetixio/core-utils/src/utils/hardhat/rpc'; + +describe('Offchain Async Order - Prevent updates with pending order test', () => { + const { systems, perpsMarkets, provider, trader1, keeper } = bootstrapMarkets({ + synthMarkets: [], + perpsMarkets: [ + { + name: 'Ether', + token: 'snxETH', + price: bn(1000), + fundingParams: { skewScale: bn(100_000), maxFundingVelocity: bn(10) }, + }, + { + name: 'Bitcoin', + token: 'snxBTC', + price: bn(10_000), + fundingParams: { skewScale: bn(100_000), maxFundingVelocity: bn(10) }, + }, + ], + traderAccountIds: [2, 3], + }); + let ethMarketId: ethers.BigNumber; + let btcMarketId: ethers.BigNumber; + + before('identify actors', async () => { + ethMarketId = perpsMarkets()[0].marketId(); + btcMarketId = perpsMarkets()[1].marketId(); + }); + + describe('With a pending order', () => { + let startTime: number; + before('add collateral', async () => { + await depositCollateral({ + systems, + trader: trader1, + accountId: () => 2, + collaterals: [ + { + snxUSDAmount: () => bn(10_000), + }, + ], + }); + }); + + before('commit the order', async () => { + const tx = await systems() + .PerpsMarket.connect(trader1()) + .commitOrder({ + marketId: ethMarketId, + accountId: 2, + sizeDelta: bn(1), + settlementStrategyId: 0, + acceptablePrice: bn(1050), // 5% slippage + trackingCode: ethers.constants.HashZero, + }); + startTime = await getTxTime(provider(), tx); + }); + + const restoreToCommit = snapshotCheckpoint(provider); + + describe('failures', () => { + before(restoreToCommit); + + it('reverts if attempt to update collateral', async () => { + await assertRevert( + systems().PerpsMarket.connect(trader1()).modifyCollateral(2, 0, bn(10)), + 'PendingOrderExist()', + systems().PerpsMarket + ); + }); + + it('reverts if attempt to commit another order', async () => { + await assertRevert( + systems() + .PerpsMarket.connect(trader1()) + .commitOrder({ + marketId: btcMarketId, + accountId: 2, + sizeDelta: bn(0.01), + settlementStrategyId: 0, + acceptablePrice: bn(10050), // 5% slippage + trackingCode: ethers.constants.HashZero, + }), + 'PendingOrderExist()', + systems().PerpsMarket + ); + }); + }); + + describe('after settle the pending order', () => { + before(restoreToCommit); + + before('settle the order', async () => { + const settlementTime = startTime + DEFAULT_SETTLEMENT_STRATEGY.settlementDelay + 1; + await fastForwardTo(settlementTime, provider()); + await settleOrder({ + systems, + keeper: keeper(), + marketId: ethMarketId, + accountId: 2, + feedId: DEFAULT_SETTLEMENT_STRATEGY.feedId, + settlementTime, + offChainPrice: 1000, + }); + }); + + it('can update the collateral', async () => { + const collateralBalancBefore = await systems().PerpsMarket.getCollateralAmount(2, 0); + + await systems().PerpsMarket.connect(trader1()).modifyCollateral(2, 0, bn(10)); + + const collateralBalancAfter = await systems().PerpsMarket.getCollateralAmount(2, 0); + assertBn.equal(collateralBalancAfter, collateralBalancBefore.add(bn(10))); + }); + + it('can commit another order', async () => { + await systems() + .PerpsMarket.connect(trader1()) + .commitOrder({ + marketId: btcMarketId, + accountId: 2, + sizeDelta: bn(1), + settlementStrategyId: 0, + acceptablePrice: bn(10050), // 5% slippage + trackingCode: ethers.constants.HashZero, + }); + + const order = await systems().PerpsMarket.getOrder(btcMarketId, 2); + assertBn.equal(order.accountId, 2); + assertBn.equal(order.marketId, btcMarketId); + assertBn.equal(order.sizeDelta, bn(1)); + }); + }); + + describe('after cancel the pending order', () => { + before(restoreToCommit); + + before('cancel the order', async () => { + await fastForwardTo( + startTime + + DEFAULT_SETTLEMENT_STRATEGY.settlementDelay + + DEFAULT_SETTLEMENT_STRATEGY.settlementWindowDuration + + 1, + provider() + ); + await systems().PerpsMarket.cancelOrder(ethMarketId, 2); + }); + + it('can update the collateral', async () => { + const collateralBalancBefore = await systems().PerpsMarket.getCollateralAmount(2, 0); + + await systems().PerpsMarket.connect(trader1()).modifyCollateral(2, 0, bn(10)); + + const collateralBalancAfter = await systems().PerpsMarket.getCollateralAmount(2, 0); + assertBn.equal(collateralBalancAfter, collateralBalancBefore.add(bn(10))); + }); + + it('can commit another order', async () => { + await systems() + .PerpsMarket.connect(trader1()) + .commitOrder({ + marketId: btcMarketId, + accountId: 2, + sizeDelta: bn(1), + settlementStrategyId: 0, + acceptablePrice: bn(10050), // 5% slippage + trackingCode: ethers.constants.HashZero, + }); + + const order = await systems().PerpsMarket.getOrder(btcMarketId, 2); + assertBn.equal(order.accountId, 2); + assertBn.equal(order.marketId, btcMarketId); + assertBn.equal(order.sizeDelta, bn(1)); + }); + }); + }); +}); 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()' ); });