From 5662a8bc287d90ae1403e693a2d66ec09b2e0178 Mon Sep 17 00:00:00 2001 From: Jonathan Fung <121899091+jonfung-dydx@users.noreply.github.com> Date: Fri, 24 May 2024 14:54:00 -0400 Subject: [PATCH] Remove uncommitted stateful orders from equity tier check (#1569) --- protocol/x/clob/e2e/equity_tier_limit_test.go | 124 ------------------ protocol/x/clob/keeper/equity_tier_limit.go | 20 --- 2 files changed, 144 deletions(-) diff --git a/protocol/x/clob/e2e/equity_tier_limit_test.go b/protocol/x/clob/e2e/equity_tier_limit_test.go index ebd9360062..b83a95c008 100644 --- a/protocol/x/clob/e2e/equity_tier_limit_test.go +++ b/protocol/x/clob/e2e/equity_tier_limit_test.go @@ -26,130 +26,6 @@ func TestPlaceOrder_EquityTierLimit(t *testing.T) { expectError bool crashingAppCheckTxNonDeterminsmChecksDisabled bool }{ - "Long-term order would exceed max open stateful orders in same block": { - allowedOrders: []clobtypes.Order{ - testapp.MustScaleOrder( - constants.ConditionalOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTBT15_StopLoss20, - testapp.DefaultGenesis(), - ), - }, - limitedOrder: testapp.MustScaleOrder( - constants.LongTermOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTBT15, - testapp.DefaultGenesis(), - ), - equityTierLimitConfiguration: clobtypes.EquityTierLimitConfiguration{ - StatefulOrderEquityTiers: []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, - }, - "Long-term order would exceed max open stateful orders in same block with multiple orders": { - allowedOrders: []clobtypes.Order{ - testapp.MustScaleOrder( - constants.ConditionalOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTBT15_StopLoss20, - testapp.DefaultGenesis(), - ), - testapp.MustScaleOrder( - constants.ConditionalOrder_Alice_Num0_Id0_Clob1_Buy5_Price10_GTBT15_StopLoss20, - testapp.DefaultGenesis(), - ), - }, - limitedOrder: testapp.MustScaleOrder( - constants.LongTermOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTBT15, - testapp.DefaultGenesis(), - ), - equityTierLimitConfiguration: clobtypes.EquityTierLimitConfiguration{ - StatefulOrderEquityTiers: []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, - }, - "Conditional order would exceed max open stateful orders in same block": { - allowedOrders: []clobtypes.Order{ - testapp.MustScaleOrder( - constants.LongTermOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTBT15, - testapp.DefaultGenesis(), - ), - }, - limitedOrder: testapp.MustScaleOrder( - constants.ConditionalOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTBT15_StopLoss20, - testapp.DefaultGenesis(), - ), - equityTierLimitConfiguration: clobtypes.EquityTierLimitConfiguration{ - StatefulOrderEquityTiers: []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, - }, - "Conditional order would exceed max open stateful orders in same block with multiple orders": { - allowedOrders: []clobtypes.Order{ - testapp.MustScaleOrder( - constants.LongTermOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTBT15, - testapp.DefaultGenesis(), - ), - testapp.MustScaleOrder( - constants.LongTermOrder_Alice_Num0_Id1_Clob1_Sell65_Price15_GTBT25, - testapp.DefaultGenesis(), - ), - }, - limitedOrder: testapp.MustScaleOrder( - constants.ConditionalOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTBT15_StopLoss20, - testapp.DefaultGenesis(), - ), - equityTierLimitConfiguration: clobtypes.EquityTierLimitConfiguration{ - StatefulOrderEquityTiers: []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 across blocks": { allowedOrders: []clobtypes.Order{ testapp.MustScaleOrder( diff --git a/protocol/x/clob/keeper/equity_tier_limit.go b/protocol/x/clob/keeper/equity_tier_limit.go index 15ff5cb6ee..528b061f61 100644 --- a/protocol/x/clob/keeper/equity_tier_limit.go +++ b/protocol/x/clob/keeper/equity_tier_limit.go @@ -1,7 +1,6 @@ package keeper import ( - "fmt" "math/big" errorsmod "cosmossdk.io/errors" @@ -145,7 +144,6 @@ func (k Keeper) ValidateSubaccountEquityTierLimitForShortTermOrder(ctx sdk.Conte // stateful orders. For `deliverState`, we sum: // - the number of long term orders. // - the number of conditional orders. -// - the number of uncommitted stateful orders in the mempool (checkState only) func (k Keeper) ValidateSubaccountEquityTierLimitForStatefulOrder(ctx sdk.Context, order types.Order) error { equityTierLimits := k.GetEquityTierLimitConfiguration(ctx).StatefulOrderEquityTiers if len(equityTierLimits) == 0 { @@ -162,24 +160,6 @@ func (k Keeper) ValidateSubaccountEquityTierLimitForStatefulOrder(ctx sdk.Contex } equityTierCount := k.GetStatefulOrderCount(ctx, order.OrderId.SubaccountId) - // If this is `CheckTx` then we must also add the number of uncommitted stateful orders that this validator - // is aware of (orders that are part of the mempool but have yet to proposed in a block). - if !lib.IsDeliverTxMode(ctx) { - equityTierCountMaybeNegative := k.GetUncommittedStatefulOrderCount(ctx, order.OrderId) + int32(equityTierCount) - if equityTierCountMaybeNegative < 0 { - panic( - fmt.Errorf( - "Expected ValidateSubaccountEquityTierLimitForStatefulOrder for new order %+v to be >= 0. "+ - "equityTierCount %d, statefulOrderCount %d, uncommittedStatefulOrderCount %d.", - order, - equityTierCountMaybeNegative, - k.GetStatefulOrderCount(ctx, order.OrderId.SubaccountId), - k.GetUncommittedStatefulOrderCount(ctx, order.OrderId), - ), - ) - } - equityTierCount = uint32(equityTierCountMaybeNegative) - } // Verify that opening this order would not exceed the maximum amount of orders for the equity tier. if lib.MustConvertIntegerToUint32(equityTierCount) >= equityTierLimit.Limit {