From 057b53ac4a1ea3292bb69ff4ade7b763ea3941f1 Mon Sep 17 00:00:00 2001 From: roy-dydx <133032749+roy-dydx@users.noreply.github.com> Date: Tue, 2 Apr 2024 23:13:22 -0400 Subject: [PATCH] Skip equity tier limit check in PlaceShortTermOrder (#1318) * Skip equity tier limit check in PlaceShortTermOrder * remove tests * Add comment --- protocol/x/clob/e2e/equity_tier_limit_test.go | 351 +----------------- protocol/x/clob/keeper/orders.go | 7 - protocol/x/clob/keeper/orders_test.go | 26 -- 3 files changed, 3 insertions(+), 381 deletions(-) diff --git a/protocol/x/clob/e2e/equity_tier_limit_test.go b/protocol/x/clob/e2e/equity_tier_limit_test.go index 0fafedbcc7..098915b784 100644 --- a/protocol/x/clob/e2e/equity_tier_limit_test.go +++ b/protocol/x/clob/e2e/equity_tier_limit_test.go @@ -2,6 +2,9 @@ package clob_test import ( "fmt" + "testing" + "time" + "github.com/cometbft/cometbft/types" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/dydxprotocol/v4-chain/protocol/dtypes" @@ -11,8 +14,6 @@ import ( clobtypes "github.com/dydxprotocol/v4-chain/protocol/x/clob/types" satypes "github.com/dydxprotocol/v4-chain/protocol/x/subaccounts/types" "github.com/stretchr/testify/require" - "testing" - "time" ) func TestPlaceOrder_EquityTierLimit(t *testing.T) { @@ -25,68 +26,6 @@ func TestPlaceOrder_EquityTierLimit(t *testing.T) { expectError bool crashingAppCheckTxNonDeterminsmChecksDisabled bool }{ - "Short-term order would exceed max open short-term orders in same block": { - allowedOrders: []clobtypes.Order{ - MustScaleOrder( - constants.Order_Alice_Num0_Id0_Clob0_Buy5_Price10_GTB20, - testapp.DefaultGenesis(), - ), - }, - limitedOrder: MustScaleOrder( - constants.Order_Alice_Num0_Id0_Clob1_Buy5_Price10_GTB15, - testapp.DefaultGenesis(), - ), - equityTierLimitConfiguration: clobtypes.EquityTierLimitConfiguration{ - ShortTermOrderEquityTiers: []clobtypes.EquityTierLimit{ - { - UsdTncRequired: dtypes.NewInt(0), - Limit: 0, - }, - { - UsdTncRequired: dtypes.NewInt(5_000_000_000), // $5,000 - Limit: 1, - }, - { - UsdTncRequired: dtypes.NewInt(70_000_000_000), // $70,000 - Limit: 100, - }, - }, - }, - expectError: true, - }, - "Short-term order would exceed max open short-term orders in same block with multiple orders": { - allowedOrders: []clobtypes.Order{ - MustScaleOrder( - constants.Order_Alice_Num0_Id0_Clob0_Buy5_Price10_GTB20, - testapp.DefaultGenesis(), - ), - MustScaleOrder( - constants.Order_Alice_Num0_Id0_Clob1_Buy5_Price10_GTB15, - testapp.DefaultGenesis(), - ), - }, - limitedOrder: MustScaleOrder( - constants.Order_Alice_Num0_Id0_Clob0_Buy6_Price10_GTB20, - testapp.DefaultGenesis(), - ), - equityTierLimitConfiguration: clobtypes.EquityTierLimitConfiguration{ - ShortTermOrderEquityTiers: []clobtypes.EquityTierLimit{ - { - UsdTncRequired: dtypes.NewInt(0), - Limit: 0, - }, - { - UsdTncRequired: dtypes.NewInt(5_000_000_000), // $5,000 - Limit: 2, - }, - { - UsdTncRequired: dtypes.NewInt(70_000_000_000), // $70,000 - Limit: 100, - }, - }, - }, - expectError: true, - }, "Long-term order would exceed max open stateful orders in same block": { allowedOrders: []clobtypes.Order{ MustScaleOrder( @@ -211,38 +150,6 @@ func TestPlaceOrder_EquityTierLimit(t *testing.T) { }, expectError: true, }, - "Short-term order would exceed max open short-term orders across blocks": { - allowedOrders: []clobtypes.Order{ - MustScaleOrder( - constants.Order_Alice_Num0_Id0_Clob0_Buy5_Price10_GTB20, - testapp.DefaultGenesis(), - ), - }, - limitedOrder: MustScaleOrder( - constants.Order_Alice_Num0_Id0_Clob1_Buy5_Price10_GTB15, - testapp.DefaultGenesis(), - ), - equityTierLimitConfiguration: clobtypes.EquityTierLimitConfiguration{ - ShortTermOrderEquityTiers: []clobtypes.EquityTierLimit{ - { - UsdTncRequired: dtypes.NewInt(0), - Limit: 0, - }, - { - UsdTncRequired: dtypes.NewInt(5_000_000_000), // $5,000 - Limit: 1, - }, - { - UsdTncRequired: dtypes.NewInt(70_000_000_000), // $70,000 - Limit: 100, - }, - }, - }, - advanceBlock: true, - expectError: true, - // The short-term order will be forgotten when restarting the app. - crashingAppCheckTxNonDeterminsmChecksDisabled: true, - }, "Long-term order would exceed max open stateful orders across blocks": { allowedOrders: []clobtypes.Order{ MustScaleOrder( @@ -393,38 +300,6 @@ func TestPlaceOrder_EquityTierLimit(t *testing.T) { advanceBlock: true, expectError: true, }, - "Order cancellation prevents exceeding max open short-term orders for short-term order in same block": { - allowedOrders: []clobtypes.Order{ - MustScaleOrder( - constants.Order_Alice_Num0_Id0_Clob0_Buy5_Price10_GTB20, - testapp.DefaultGenesis(), - ), - }, - limitedOrder: MustScaleOrder( - constants.Order_Alice_Num0_Id0_Clob1_Buy5_Price10_GTB15, - testapp.DefaultGenesis(), - ), - cancellation: clobtypes.NewMsgCancelOrderShortTerm( - constants.Order_Alice_Num0_Id0_Clob0_Buy5_Price10_GTB20.OrderId, - constants.Order_Alice_Num0_Id0_Clob0_Buy5_Price10_GTB20.GetGoodTilBlock(), - ), - equityTierLimitConfiguration: clobtypes.EquityTierLimitConfiguration{ - ShortTermOrderEquityTiers: []clobtypes.EquityTierLimit{ - { - UsdTncRequired: dtypes.NewInt(0), - Limit: 0, - }, - { - UsdTncRequired: dtypes.NewInt(5_000_000_000), // $5,000 - Limit: 1, - }, - { - UsdTncRequired: dtypes.NewInt(70_000_000_000), // $70,000 - Limit: 100, - }, - }, - }, - }, "Order cancellation prevents exceeding max open stateful orders for long-term order in same block": { allowedOrders: []clobtypes.Order{ MustScaleOrder( @@ -522,41 +397,6 @@ func TestPlaceOrder_EquityTierLimit(t *testing.T) { }, }, }, - "Order cancellation prevents exceeding max open short-term orders for short-term order across blocks": { - allowedOrders: []clobtypes.Order{ - MustScaleOrder( - constants.Order_Alice_Num0_Id0_Clob0_Buy5_Price10_GTB20, - testapp.DefaultGenesis(), - ), - }, - limitedOrder: MustScaleOrder( - constants.Order_Alice_Num0_Id0_Clob1_Buy5_Price10_GTB15, - testapp.DefaultGenesis(), - ), - cancellation: clobtypes.NewMsgCancelOrderShortTerm( - constants.Order_Alice_Num0_Id0_Clob0_Buy5_Price10_GTB20.OrderId, - constants.Order_Alice_Num0_Id0_Clob0_Buy5_Price10_GTB20.GetGoodTilBlock(), - ), - equityTierLimitConfiguration: clobtypes.EquityTierLimitConfiguration{ - ShortTermOrderEquityTiers: []clobtypes.EquityTierLimit{ - { - UsdTncRequired: dtypes.NewInt(0), - Limit: 0, - }, - { - UsdTncRequired: dtypes.NewInt(5_000_000_000), // $5,000 - Limit: 1, - }, - { - UsdTncRequired: dtypes.NewInt(70_000_000_000), // $70,000 - Limit: 100, - }, - }, - }, - advanceBlock: true, - // The short-term order & cancel will be forgotten when restarting the app. - crashingAppCheckTxNonDeterminsmChecksDisabled: true, - }, "Order cancellation prevents exceeding max open stateful orders for long-term order across blocks": { allowedOrders: []clobtypes.Order{ MustScaleOrder( @@ -737,65 +577,6 @@ func TestPlaceOrder_EquityTierLimit_OrderExpiry(t *testing.T) { expectError bool crashingAppCheckTxNonDeterminsmChecksDisabled bool }{ - "Short-term order has not expired": { - firstOrder: MustScaleOrder( - constants.Order_Alice_Num0_Id0_Clob1_Buy5_Price10_GTB15, - testapp.DefaultGenesis(), - ), - secondOrder: MustScaleOrder( - constants.Order_Alice_Num0_Id0_Clob0_Buy5_Price10_GTB20, - testapp.DefaultGenesis(), - ), - equityTierLimitConfiguration: clobtypes.EquityTierLimitConfiguration{ - ShortTermOrderEquityTiers: []clobtypes.EquityTierLimit{ - { - UsdTncRequired: dtypes.NewInt(0), - Limit: 0, - }, - { - UsdTncRequired: dtypes.NewInt(5_000_000_000), // $5,000 - Limit: 1, - }, - { - UsdTncRequired: dtypes.NewInt(70_000_000_000), // $70,000 - Limit: 100, - }, - }, - }, - advanceToBlockAndTime: 14, - expectError: true, - // Short term order will be forgotten on app restart. - crashingAppCheckTxNonDeterminsmChecksDisabled: true, - }, - "Short-term order has expired": { - firstOrder: MustScaleOrder( - constants.Order_Alice_Num0_Id0_Clob1_Buy5_Price10_GTB15, - testapp.DefaultGenesis(), - ), - secondOrder: MustScaleOrder( - constants.Order_Alice_Num0_Id0_Clob0_Buy5_Price10_GTB20, - testapp.DefaultGenesis(), - ), - equityTierLimitConfiguration: clobtypes.EquityTierLimitConfiguration{ - ShortTermOrderEquityTiers: []clobtypes.EquityTierLimit{ - { - UsdTncRequired: dtypes.NewInt(0), - Limit: 0, - }, - { - UsdTncRequired: dtypes.NewInt(5_000_000_000), // $5,000 - Limit: 1, - }, - { - UsdTncRequired: dtypes.NewInt(70_000_000_000), // $70,000 - Limit: 100, - }, - }, - }, - advanceToBlockAndTime: 15, - // Short term order will be forgotten on app restart. - crashingAppCheckTxNonDeterminsmChecksDisabled: true, - }, "Stateful order has not expired": { firstOrder: MustScaleOrder( constants.LongTermOrder_Alice_Num0_Id0_Clob1_Buy5_Price10_GTBT5, @@ -911,132 +692,6 @@ func TestPlaceOrder_EquityTierLimit_OrderFill(t *testing.T) { expectError bool crashingAppCheckTxNonDeterminsmChecksDisabled bool }{ - "Fully filled order prevents exceeding max open short-term orders for short-term order in same block": { - makerOrder: MustScaleOrder( - constants.Order_Bob_Num0_Id8_Clob0_Sell20_Price10_GTB22, - testapp.DefaultGenesis(), - ), - takerOrder: MustScaleOrder( - constants.Order_Alice_Num0_Id0_Clob0_Buy5_Price10_GTB20, - testapp.DefaultGenesis(), - ), - extraOrder: MustScaleOrder( - constants.Order_Alice_Num0_Id0_Clob1_Buy5_Price10_GTB15, - testapp.DefaultGenesis(), - ), - equityTierLimitConfiguration: clobtypes.EquityTierLimitConfiguration{ - ShortTermOrderEquityTiers: []clobtypes.EquityTierLimit{ - { - UsdTncRequired: dtypes.NewInt(0), - Limit: 0, - }, - { - UsdTncRequired: dtypes.NewInt(5_000_000_000), // $5,000 - Limit: 1, - }, - { - UsdTncRequired: dtypes.NewInt(70_000_000_000), // $70,000 - Limit: 100, - }, - }, - }, - }, - "Partially filled order causes new short-term order to exceed max open short-term orders in same block": { - makerOrder: MustScaleOrder( - constants.Order_Bob_Num0_Id8_Clob0_Sell20_Price10_GTB22, - testapp.DefaultGenesis(), - ), - takerOrder: MustScaleOrder( - constants.Order_Alice_Num0_Id0_Clob0_Buy35_Price10_GTB20, - testapp.DefaultGenesis(), - ), - extraOrder: MustScaleOrder( - constants.Order_Alice_Num0_Id0_Clob1_Buy5_Price10_GTB15, - testapp.DefaultGenesis(), - ), - equityTierLimitConfiguration: clobtypes.EquityTierLimitConfiguration{ - ShortTermOrderEquityTiers: []clobtypes.EquityTierLimit{ - { - UsdTncRequired: dtypes.NewInt(0), - Limit: 0, - }, - { - UsdTncRequired: dtypes.NewInt(5_000_000_000), // $5,000 - Limit: 1, - }, - { - UsdTncRequired: dtypes.NewInt(70_000_000_000), // $70,000 - Limit: 100, - }, - }, - }, - expectError: true, - }, - "Fully filled order prevents exceeding max open short-term orders for short-term order across blocks": { - makerOrder: MustScaleOrder( - constants.Order_Bob_Num0_Id8_Clob0_Sell20_Price10_GTB22, - testapp.DefaultGenesis(), - ), - takerOrder: MustScaleOrder( - constants.Order_Alice_Num0_Id0_Clob0_Buy5_Price10_GTB20, - testapp.DefaultGenesis(), - ), - extraOrder: MustScaleOrder( - constants.Order_Alice_Num0_Id0_Clob1_Buy5_Price10_GTB15, - testapp.DefaultGenesis(), - ), - equityTierLimitConfiguration: clobtypes.EquityTierLimitConfiguration{ - ShortTermOrderEquityTiers: []clobtypes.EquityTierLimit{ - { - UsdTncRequired: dtypes.NewInt(0), - Limit: 0, - }, - { - UsdTncRequired: dtypes.NewInt(5_000_000_000), // $5,000 - Limit: 1, - }, - { - UsdTncRequired: dtypes.NewInt(70_000_000_000), // $70,000 - Limit: 100, - }, - }, - }, - advanceBlock: true, - }, - "Partially filled order causes new short-term order to exceed max open short-term orders across blocks": { - makerOrder: MustScaleOrder( - constants.Order_Bob_Num0_Id8_Clob0_Sell20_Price10_GTB22, - testapp.DefaultGenesis(), - ), - takerOrder: MustScaleOrder( - constants.Order_Alice_Num0_Id0_Clob0_Buy35_Price10_GTB20, - testapp.DefaultGenesis(), - ), - extraOrder: MustScaleOrder( - constants.Order_Alice_Num0_Id0_Clob1_Buy5_Price10_GTB15, - testapp.DefaultGenesis(), - ), - equityTierLimitConfiguration: clobtypes.EquityTierLimitConfiguration{ - ShortTermOrderEquityTiers: []clobtypes.EquityTierLimit{ - { - UsdTncRequired: dtypes.NewInt(0), - Limit: 0, - }, - { - UsdTncRequired: dtypes.NewInt(5_000_000_000), // $5,000 - Limit: 1, - }, - { - UsdTncRequired: dtypes.NewInt(70_000_000_000), // $70,000 - Limit: 100, - }, - }, - }, - advanceBlock: true, - expectError: true, - // The short-term order will be forgotten when restarting the app. - crashingAppCheckTxNonDeterminsmChecksDisabled: true, - }, "Order fully filled prevents exceeding max open stateful orders for conditional order across blocks": { makerOrder: MustScaleOrder( constants.LongTermOrder_Bob_Num0_Id0_Clob0_Sell5_Price5_GTBT10, diff --git a/protocol/x/clob/keeper/orders.go b/protocol/x/clob/keeper/orders.go index b78cd01901..84abb5a58d 100644 --- a/protocol/x/clob/keeper/orders.go +++ b/protocol/x/clob/keeper/orders.go @@ -98,7 +98,6 @@ func (k Keeper) CancelShortTermOrder( // // An error will be returned if any of the following conditions are true: // - Standard stateful validation fails. -// - The subaccount's equity tier limit is exceeded. // - Placing the short term order on the memclob returns an error. // // This method will panic if the provided order is not a Short-Term order. @@ -139,12 +138,6 @@ func (k Keeper) PlaceShortTermOrder( return 0, 0, err } - // Validate that adding the order wouldn't exceed subaccount equity tier limits. - err = k.ValidateSubaccountEquityTierLimitForNewOrder(ctx, order) - if err != nil { - return 0, 0, err - } - // Place the order on the memclob and return the result. orderSizeOptimisticallyFilledFromMatchingQuantums, orderStatus, offchainUpdates, err := k.MemClob.PlaceOrder( ctx, diff --git a/protocol/x/clob/keeper/orders_test.go b/protocol/x/clob/keeper/orders_test.go index 750ad1440c..cb9f2a301c 100644 --- a/protocol/x/clob/keeper/orders_test.go +++ b/protocol/x/clob/keeper/orders_test.go @@ -508,32 +508,6 @@ func TestPlaceShortTermOrder(t *testing.T) { order: constants.Order_Carl_Num0_Id4_Clob1_Buy01ETH_Price3000, expectedOrderStatus: types.Success, }, - `Subaccount cannot place maker buy order for 1 BTC at 5 subticks with 0 collateral`: { - perpetuals: []perptypes.Perpetual{ - constants.BtcUsd_50PercentInitial_40PercentMaintenance, - }, - subaccounts: []satypes.Subaccount{constants.Carl_Num0_0USD}, - clobs: []types.ClobPair{ - constants.ClobPair_Btc, - }, - existingOrders: []types.Order{}, - feeParams: constants.PerpetualFeeParamsNoFee, - order: constants.Order_Carl_Num0_Id0_Clob0_Buy1BTC_Price5subticks_GTB10, - expectedErr: types.ErrOrderWouldExceedMaxOpenOrdersEquityTierLimit, - }, - `Subaccount cannot place maker sell order for 1 BTC at 500,000 with 0 collateral`: { - perpetuals: []perptypes.Perpetual{ - constants.BtcUsd_50PercentInitial_40PercentMaintenance, - }, - subaccounts: []satypes.Subaccount{constants.Carl_Num0_0USD}, - clobs: []types.ClobPair{ - constants.ClobPair_Btc, - }, - existingOrders: []types.Order{}, - feeParams: constants.PerpetualFeeParamsNoFee, - order: constants.Order_Carl_Num0_Id0_Clob0_Sell1BTC_Price500000_GTB10, - expectedErr: types.ErrOrderWouldExceedMaxOpenOrdersEquityTierLimit, - }, // // The following 3 tests are a group to test the deprecation of pessimistic collateralization check. // 1. The first should have a buy price well below the oracle price of 50,000. (success)