[CT-619] deprecate pessimistic add-to-book collateralization check

protocol/testing/e2e/gov/add_new_market_test.go

@@ -27,8 +27,8 @@ const (
 	NumBlocksAfterTradingEnabled = 50
 	TestMarketId                 = 1001
 	// Expected response log when a order is submitted but oracle price is zero.
-	ExpectedPlaceOrderCheckTxResponseLog = "recovered: clob pair ID = (1001), perpetual ID = (1001), " +
-		"market ID = (1001): Oracle price must be > 0"
+	// https://github.com/dydxprotocol/v4-chain/blob/5ee11ed/protocol/x/perpetuals/keeper/perpetual.go#L1514-L1517
+	ExpectedPlaceOrderCheckTxResponseLog = "recovered: type: perpetual, id: 1001: product position is not updatable"
 )
 
 var (

protocol/x/clob/keeper/msg_server_place_order_test.go

@@ -6,6 +6,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/dydxprotocol/v4-chain/protocol/dtypes"
 	indexerevents "github.com/dydxprotocol/v4-chain/protocol/indexer/events"
 	"github.com/dydxprotocol/v4-chain/protocol/indexer/indexer_manager"
 	"github.com/dydxprotocol/v4-chain/protocol/lib"
@@ -48,9 +49,14 @@ func TestPlaceOrder_Error(t *testing.T) {
 			ExpectedError:          types.ErrTimeExceedsGoodTilBlockTime,
 		},
 		"Returns an error when collateralization check fails": {
-			StatefulOrderPlacement: constants.LongTermOrder_Bob_Num0_Id2_Clob0_Buy15_Price5_GTBT10,
+			StatefulOrderPlacement: constants.LongTermOrder_Alice_Num0_Id1_Clob0_Buy1BTC_Price50000_GTBT15,
 			ExpectedError:          types.ErrStatefulOrderCollateralizationCheckFailed,
 		},
+		"Returns an error when equity tier check fails": {
+			// Bob has TNC of $0.
+			StatefulOrderPlacement: constants.LongTermOrder_Bob_Num0_Id2_Clob0_Buy15_Price5_GTBT10,
+			ExpectedError:          types.ErrOrderWouldExceedMaxOpenOrdersEquityTierLimit,
+		},
 		"Returns an error when order replacement is attempted": {
 			StatefulOrders: []types.Order{
 				constants.LongTermOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTBT15,
@@ -128,6 +134,27 @@ func TestPlaceOrder_Error(t *testing.T) {
 			)
 			require.NoError(t, err)
 
+			ks.SubaccountsKeeper.SetSubaccount(ks.Ctx, constants.Alice_Num0_10_000USD)
+
+			err = ks.ClobKeeper.InitializeEquityTierLimit(
+				ks.Ctx,
+				types.EquityTierLimitConfiguration{
+					ShortTermOrderEquityTiers: []types.EquityTierLimit{
+						{
+							UsdTncRequired: dtypes.NewInt(20_000_000),
+							Limit:          5,
+						},
+					},
+					StatefulOrderEquityTiers: []types.EquityTierLimit{
+						{
+							UsdTncRequired: dtypes.NewInt(20_000_000),
+							Limit:          5,
+						},
+					},
+				},
+			)
+			require.NoError(t, err)
+
 			// Create ClobPair.
 			clobPair := constants.ClobPair_Btc
 			// PerpetualMarketCreateEvents are emitted when initializing the genesis state, so we need to mock

protocol/x/clob/keeper/orders.go

@@ -2,7 +2,6 @@ package keeper
 
 import (
 	"fmt"
-	"math"
 	"math/big"
 	"time"
 
@@ -961,12 +960,8 @@ func (k Keeper) AddOrderToOrderbookCollatCheck(
 	pendingUpdates := types.NewPendingUpdates()
 
 	// Retrieve the associated `PerpetualId` for the `ClobPair`.
-	oraclePriceSubticksRat := k.GetOraclePriceSubticksRat(ctx, clobPair)
 	perpetualId := clobPair.MustGetPerpetualId()
 
-	// TODO(DEC-1713): Complete as many calculations from getPessimisticCollateralCheckPrice as possible here
-	// so we aren't recalculating the same thing within the loop.
-
 	iterateOverOpenOrdersStart := time.Now()
 	for subaccountId, openOrders := range subaccountOpenOrders {
 		telemetry.SetGauge(
@@ -983,37 +978,7 @@ func (k Keeper) AddOrderToOrderbookCollatCheck(
 				panic(fmt.Sprintf("Order `ClobPairId` must equal `clobPairId` for order %+v", openOrder))
 			}
 
-			// We don't want to allow users to place orders to improve their collateralization, so we choose between the
-			// order price (user-input) or the oracle price (sane default) and select the price that results in the
-			// most pessimistic collateral-check outcome.
-			collatCheckPriceSubticks, err := getPessimisticCollateralCheckPrice(
-				oraclePriceSubticksRat,
-				openOrder.Subticks,
-				openOrder.IsBuy,
-			)
-			if satypes.ErrIntegerOverflow.Is(err) {
-				// TODO(DEC-1701): Determine best action to take if the oracle price overflows max uint64
-				log.ErrorLog(
-					ctx,
-					fmt.Sprintf(
-						"Integer overflow: oracle price (subticks) exceeded uint64 max. "+
-							"perpetual ID = (%d), oracle price = (%+v), is buy = (%t)",
-						perpetualId,
-						oraclePriceSubticksRat,
-						openOrder.IsBuy,
-					),
-				)
-			} else if err != nil {
-				panic(
-					errorsmod.Wrapf(
-						err,
-						"perpetual id = (%d), oracle price = (%+v), is buy = (%t)",
-						perpetualId,
-						oraclePriceSubticksRat,
-						openOrder.IsBuy,
-					),
-				)
-			}
+			collatCheckPriceSubticks := openOrder.Subticks
 
 			bigFillQuoteQuantums, err := getFillQuoteQuantums(
 				clobPair,
@@ -1277,42 +1242,6 @@ func (k Keeper) SendOffchainMessages(
 	}
 }
 
-// getPessimisticCollateralCheckPrice returns the price in subticks we should use for collateralization checks.
-// It pessimistically rounds oraclePriceSubticksRat (up for buys, down for sells) and then pessimistically
-// chooses the subticks value to return: the highest for buys, the lowest for sells.
-//
-// Returns an error if:
-// - the oraclePriceSubticksRat, after being rounded pessimistically, overflows a uint64
-func getPessimisticCollateralCheckPrice(
-	oraclePriceSubticksRat *big.Rat,
-	makerOrderSubticks types.Subticks,
-	isBuy bool,
-) (price types.Subticks, err error) {
-	// TODO(DEC-1713): Move this rounding to before the PendingOpenOrders loop.
-	// The oracle price is guaranteed to be >= 0. Since we are using this value for a collateralization check,
-	// we want to round pessimistically (up for buys, down for sells).
-	oraclePriceSubticksInt := lib.BigRatRound(oraclePriceSubticksRat, isBuy)
-
-	// TODO(DEC-1701): Determine best action to take if the oracle price overflows max uint64
-	var oraclePriceSubticks types.Subticks
-	if oraclePriceSubticksInt.IsUint64() {
-		oraclePriceSubticks = types.Subticks(lib.Max(1, oraclePriceSubticksInt.Uint64()))
-	} else {
-		// Clamping the oracle price here should be fine because there are 2 outcomes:
-		// 1. This is a sell order, in which case we choose the lowest value, so we choose the maker sell price which
-		// would be identical to the old logic.
-		// 2. This is a buy order, and we select uint64 max as the price, which will fail the collateral check in any
-		// real-world example.
-		oraclePriceSubticks = types.Subticks(math.MaxUint64)
-		err = satypes.ErrIntegerOverflow
-	}
-
-	if isBuy {
-		return lib.Max(makerOrderSubticks, oraclePriceSubticks), err
-	}
-	return lib.Min(makerOrderSubticks, oraclePriceSubticks), err
-}
-
 // getFillQuoteQuantums returns the total fillAmount price in quote quantums based on the maker subticks.
 // This value is always positive.
 //

protocol/x/clob/keeper/orders_test.go

@@ -6,6 +6,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/dydxprotocol/v4-chain/protocol/dtypes"
 	indexerevents "github.com/dydxprotocol/v4-chain/protocol/indexer/events"
 
 	cmt "github.com/cometbft/cometbft/types"
@@ -515,12 +516,10 @@ func TestPlaceShortTermOrder(t *testing.T) {
 			clobs: []types.ClobPair{
 				constants.ClobPair_Btc,
 			},
-			existingOrders:           []types.Order{},
-			feeParams:                constants.PerpetualFeeParamsNoFee,
-			order:                    constants.Order_Carl_Num0_Id0_Clob0_Buy1BTC_Price5subticks_GTB10,
-			expectedOrderStatus:      types.Undercollateralized,
-			expectedFilledSize:       0,
-			expectedMultiStoreWrites: []string{},
+			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{
@@ -530,20 +529,18 @@ func TestPlaceShortTermOrder(t *testing.T) {
 			clobs: []types.ClobPair{
 				constants.ClobPair_Btc,
 			},
-			existingOrders:           []types.Order{},
-			feeParams:                constants.PerpetualFeeParamsNoFee,
-			order:                    constants.Order_Carl_Num0_Id0_Clob0_Sell1BTC_Price500000_GTB10,
-			expectedOrderStatus:      types.Undercollateralized,
-			expectedFilledSize:       0,
-			expectedMultiStoreWrites: []string{},
+			existingOrders: []types.Order{},
+			feeParams:      constants.PerpetualFeeParamsNoFee,
+			order:          constants.Order_Carl_Num0_Id0_Clob0_Sell1BTC_Price500000_GTB10,
+			expectedErr:    types.ErrOrderWouldExceedMaxOpenOrdersEquityTierLimit,
 		},
-		// <grouped tests: pessimistic value collateralization check -- BUY>
-		// The following 3 tests are a group to test the pessimistic value used for the collateralization check.
-		// 1. The first should have a lower asset value in its subaccount. (undercollateralized)
+		// <grouped tests: deprecating pessimistic value collateralization check -- BUY>
+		// 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)
 		// 2. The second should have a buy price above the oracle price of 50,000. (undercollateralized)
 		// 3. The third should have the order in common with #1 and the subaccount in common with #2 and should succeed.
-		`Subaccount cannot place buy order due to a failed collateralization check with the oracle price but would
-				pass if using the maker price`: {
+		`Subaccount can now place buy order that would have failed the 
+				deprecated pessimistic collateralization check with the oracle price`: {
 			perpetuals: []perptypes.Perpetual{
 				constants.BtcUsd_50PercentInitial_40PercentMaintenance,
 			},
@@ -554,7 +551,7 @@ func TestPlaceShortTermOrder(t *testing.T) {
 			existingOrders:           []types.Order{},
 			feeParams:                constants.PerpetualFeeParamsNoFee,
 			order:                    constants.Order_Carl_Num0_Id0_Clob0_Buy1BTC_Price5subticks_GTB10,
-			expectedOrderStatus:      types.Undercollateralized,
+			expectedOrderStatus:      types.Success,
 			expectedFilledSize:       0,
 			expectedMultiStoreWrites: []string{},
 		},
@@ -574,7 +571,7 @@ func TestPlaceShortTermOrder(t *testing.T) {
 			expectedFilledSize:       0,
 			expectedMultiStoreWrites: []string{},
 		},
-		`Subaccount placed buy order passes collateralization check when using the oracle price`: {
+		`Subaccount placed buy order passes collateralization check when using the maker price`: {
 			perpetuals: []perptypes.Perpetual{
 				constants.BtcUsd_50PercentInitial_40PercentMaintenance,
 			},
@@ -590,13 +587,13 @@ func TestPlaceShortTermOrder(t *testing.T) {
 			expectedMultiStoreWrites: []string{},
 		},
 		// <end of grouped tests: pessimistic value collateralization check -- BUY>
-		// <grouped tests: pessimistic value collateralization check -- SELL>
-		// The following 3 tests are a group to test the pessimistic value used for the collateralization check.
-		// 1. The first should have a lower asset value in its subaccount. (undercollateralized)
+		// <grouped tests: deprecating pessimistic value collateralization check -- SELL>
+		// The following 3 tests are a group to test the deprecation of pessimistic collateralization check.
+		// 1. The first should have a sell price well above the oracle price of 50,000. (success)
 		// 2. The second should have a sell price below the oracle price of 50,000 subticks. (undercollateralized)
 		// 3. The third should have the order in common with #1 and the subaccount in common with #2 and should succeed.
-		`Subaccount cannot place sell order due to a failed collateralization check with the oracle price but would
-				pass if using the maker price`: {
+		`Subaccount can now place sell order that would have failed the 
+				deprecated pessimistic collateralization check with the oracle price`: {
 			perpetuals: []perptypes.Perpetual{
 				constants.BtcUsd_50PercentInitial_40PercentMaintenance,
 			},
@@ -607,7 +604,7 @@ func TestPlaceShortTermOrder(t *testing.T) {
 			existingOrders:           []types.Order{},
 			feeParams:                constants.PerpetualFeeParamsNoFee,
 			order:                    constants.Order_Carl_Num0_Id0_Clob0_Sell1BTC_Price500000_GTB10,
-			expectedOrderStatus:      types.Undercollateralized,
+			expectedOrderStatus:      types.Success,
 			expectedFilledSize:       0,
 			expectedMultiStoreWrites: []string{},
 		},
@@ -627,7 +624,7 @@ func TestPlaceShortTermOrder(t *testing.T) {
 			expectedFilledSize:       0,
 			expectedMultiStoreWrites: []string{},
 		},
-		`Subaccount placed sell order passes collateralization check when using the oracle price`: {
+		`Subaccount placed sell order passes collateralization check when using the maker price`: {
 			perpetuals: []perptypes.Perpetual{
 				constants.BtcUsd_50PercentInitial_40PercentMaintenance,
 			},
@@ -705,6 +702,25 @@ func TestPlaceShortTermOrder(t *testing.T) {
 				require.NoError(t, err)
 			}
 
+			err = ks.ClobKeeper.InitializeEquityTierLimit(
+				ctx,
+				types.EquityTierLimitConfiguration{
+					ShortTermOrderEquityTiers: []types.EquityTierLimit{
+						{
+							UsdTncRequired: dtypes.NewInt(20_000_000),
+							Limit:          5,
+						},
+					},
+					StatefulOrderEquityTiers: []types.EquityTierLimit{
+						{
+							UsdTncRequired: dtypes.NewInt(20_000_000),
+							Limit:          5,
+						},
+					},
+				},
+			)
+			require.NoError(t, err)
+
 			// Create all existing orders.
 			for _, order := range tc.existingOrders {
 				_, _, err := ks.ClobKeeper.PlaceShortTermOrder(ctx, &types.MsgPlaceOrder{Order: order})