diff --git a/protocol/testing/e2e/gov/add_new_market_test.go b/protocol/testing/e2e/gov/add_new_market_test.go index 72cdf89c02..8e9451bd5f 100644 --- a/protocol/testing/e2e/gov/add_new_market_test.go +++ b/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 ( diff --git a/protocol/x/clob/keeper/msg_server_place_order_test.go b/protocol/x/clob/keeper/msg_server_place_order_test.go index 39ce10a079..a1b6a41e4c 100644 --- a/protocol/x/clob/keeper/msg_server_place_order_test.go +++ b/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 diff --git a/protocol/x/clob/keeper/orders.go b/protocol/x/clob/keeper/orders.go index e796917c54..a1119e949b 100644 --- a/protocol/x/clob/keeper/orders.go +++ b/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. // diff --git a/protocol/x/clob/keeper/orders_test.go b/protocol/x/clob/keeper/orders_test.go index f3d346fa1d..9646f11257 100644 --- a/protocol/x/clob/keeper/orders_test.go +++ b/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, }, - // - // 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) + // + // 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{}, }, // - // - // 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) + // + // 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})