From ae691d22e75f751ee39b4d8cca587945e8063906 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Mon, 20 May 2024 11:54:03 -0400 Subject: [PATCH] Skip equity tier limit check in PlaceShortTermOrder (backport #1318) (#1549) * Skip equity tier limit check in PlaceShortTermOrder (#1318) * Skip equity tier limit check in PlaceShortTermOrder * remove tests * Add comment (cherry picked from commit 24790ff5c664c63330a9d85879304063b6c92928) * Fix lint --------- Co-authored-by: roy-dydx <133032749+roy-dydx@users.noreply.github.com> Co-authored-by: Roy Li --- protocol/x/clob/e2e/equity_tier_limit_test.go | 351 +----------------- protocol/x/clob/keeper/equity_tier_limit.go | 3 + protocol/x/clob/keeper/orders.go | 7 - protocol/x/clob/keeper/orders_test.go | 26 -- 4 files changed, 6 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 b3189357db..ebd9360062 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{ - testapp.MustScaleOrder( - constants.Order_Alice_Num0_Id0_Clob0_Buy5_Price10_GTB20, - testapp.DefaultGenesis(), - ), - }, - limitedOrder: testapp.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{ - testapp.MustScaleOrder( - constants.Order_Alice_Num0_Id0_Clob0_Buy5_Price10_GTB20, - testapp.DefaultGenesis(), - ), - testapp.MustScaleOrder( - constants.Order_Alice_Num0_Id0_Clob1_Buy5_Price10_GTB15, - testapp.DefaultGenesis(), - ), - }, - limitedOrder: testapp.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{ testapp.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{ - testapp.MustScaleOrder( - constants.Order_Alice_Num0_Id0_Clob0_Buy5_Price10_GTB20, - testapp.DefaultGenesis(), - ), - }, - limitedOrder: testapp.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{ testapp.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{ - testapp.MustScaleOrder( - constants.Order_Alice_Num0_Id0_Clob0_Buy5_Price10_GTB20, - testapp.DefaultGenesis(), - ), - }, - limitedOrder: testapp.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{ testapp.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{ - testapp.MustScaleOrder( - constants.Order_Alice_Num0_Id0_Clob0_Buy5_Price10_GTB20, - testapp.DefaultGenesis(), - ), - }, - limitedOrder: testapp.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{ testapp.MustScaleOrder( @@ -737,65 +577,6 @@ func TestPlaceOrder_EquityTierLimit_OrderExpiry(t *testing.T) { expectError bool crashingAppCheckTxNonDeterminsmChecksDisabled bool }{ - "Short-term order has not expired": { - firstOrder: testapp.MustScaleOrder( - constants.Order_Alice_Num0_Id0_Clob1_Buy5_Price10_GTB15, - testapp.DefaultGenesis(), - ), - secondOrder: testapp.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: testapp.MustScaleOrder( - constants.Order_Alice_Num0_Id0_Clob1_Buy5_Price10_GTB15, - testapp.DefaultGenesis(), - ), - secondOrder: testapp.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: testapp.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: testapp.MustScaleOrder( - constants.Order_Bob_Num0_Id8_Clob0_Sell20_Price10_GTB22, - testapp.DefaultGenesis(), - ), - takerOrder: testapp.MustScaleOrder( - constants.Order_Alice_Num0_Id0_Clob0_Buy5_Price10_GTB20, - testapp.DefaultGenesis(), - ), - extraOrder: testapp.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: testapp.MustScaleOrder( - constants.Order_Bob_Num0_Id8_Clob0_Sell20_Price10_GTB22, - testapp.DefaultGenesis(), - ), - takerOrder: testapp.MustScaleOrder( - constants.Order_Alice_Num0_Id0_Clob0_Buy35_Price10_GTB20, - testapp.DefaultGenesis(), - ), - extraOrder: testapp.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: testapp.MustScaleOrder( - constants.Order_Bob_Num0_Id8_Clob0_Sell20_Price10_GTB22, - testapp.DefaultGenesis(), - ), - takerOrder: testapp.MustScaleOrder( - constants.Order_Alice_Num0_Id0_Clob0_Buy5_Price10_GTB20, - testapp.DefaultGenesis(), - ), - extraOrder: testapp.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: testapp.MustScaleOrder( - constants.Order_Bob_Num0_Id8_Clob0_Sell20_Price10_GTB22, - testapp.DefaultGenesis(), - ), - takerOrder: testapp.MustScaleOrder( - constants.Order_Alice_Num0_Id0_Clob0_Buy35_Price10_GTB20, - testapp.DefaultGenesis(), - ), - extraOrder: testapp.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: testapp.MustScaleOrder( constants.LongTermOrder_Bob_Num0_Id0_Clob0_Sell5_Price5_GTBT10, diff --git a/protocol/x/clob/keeper/equity_tier_limit.go b/protocol/x/clob/keeper/equity_tier_limit.go index be4df49446..15ff5cb6ee 100644 --- a/protocol/x/clob/keeper/equity_tier_limit.go +++ b/protocol/x/clob/keeper/equity_tier_limit.go @@ -95,6 +95,9 @@ func (k Keeper) getEquityTierLimitForSubaccount( return equityTierLimit, nil, nil } +// Deprecated: Equity tier limits were removed for short term orders. +// See https://github.com/dydxprotocol/v4-chain/pull/1318. +// // ValidateSubaccountEquityTierLimitForShortTermOrder returns an error if adding the order would exceed the equity // tier limit on how many short term open orders a subaccount can have. Short-term fill-or-kill and immediate-or-cancel // orders never rest on the book and will always be allowed as they do not apply to the number of open orders that diff --git a/protocol/x/clob/keeper/orders.go b/protocol/x/clob/keeper/orders.go index aaeca84de0..9986580930 100644 --- a/protocol/x/clob/keeper/orders.go +++ b/protocol/x/clob/keeper/orders.go @@ -167,7 +167,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. @@ -208,12 +207,6 @@ func (k Keeper) PlaceShortTermOrder( return 0, 0, err } - // Validate that adding the order wouldn't exceed subaccount equity tier limits. - err = k.ValidateSubaccountEquityTierLimitForShortTermOrder(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 da10a4b948..c01b27aaba 100644 --- a/protocol/x/clob/keeper/orders_test.go +++ b/protocol/x/clob/keeper/orders_test.go @@ -578,32 +578,6 @@ func TestPlaceShortTermOrder(t *testing.T) { constants.EthUsd_20PercentInitial_10PercentMaintenance.Params.Id: big.NewInt(2_000_000_000), }, }, - `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)