From 9f30cf5164271cc45ace78d7a7c2290d827d840a Mon Sep 17 00:00:00 2001 From: Teddy Ding Date: Fri, 10 Nov 2023 10:41:15 -0500 Subject: [PATCH 1/2] [CORE-585] In add-new-market gov e2e test, place order on new market with `0` oracle price (#759) * Try to place order on new market with 0 oracle price * nit --- .../testing/e2e/gov/add_new_market_test.go | 80 ++++++++++++++++--- 1 file changed, 67 insertions(+), 13 deletions(-) diff --git a/protocol/testing/e2e/gov/add_new_market_test.go b/protocol/testing/e2e/gov/add_new_market_test.go index a08ab1431f..38a0021e53 100644 --- a/protocol/testing/e2e/gov/add_new_market_test.go +++ b/protocol/testing/e2e/gov/add_new_market_test.go @@ -1,8 +1,10 @@ package gov_test import ( - "github.com/dydxprotocol/v4-chain/protocol/lib" "testing" + "time" + + "github.com/dydxprotocol/v4-chain/protocol/lib" "github.com/cometbft/cometbft/types" sdk "github.com/cosmos/cosmos-sdk/types" @@ -10,6 +12,7 @@ import ( govtypesv1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1" testapp "github.com/dydxprotocol/v4-chain/protocol/testutil/app" clobtest "github.com/dydxprotocol/v4-chain/protocol/testutil/clob" + "github.com/dydxprotocol/v4-chain/protocol/testutil/constants" "github.com/dydxprotocol/v4-chain/protocol/testutil/encoding" perptest "github.com/dydxprotocol/v4-chain/protocol/testutil/perpetuals" pricestest "github.com/dydxprotocol/v4-chain/protocol/testutil/prices" @@ -20,25 +23,50 @@ import ( "github.com/stretchr/testify/require" ) +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" +) + +var ( + GenesisTime = time.Unix(1690000000, 0) + OrderTemplate_Alice_Num0_Id0_Clob0_Buy_LongTerm = clobtypes.Order{ + OrderId: clobtypes.OrderId{ + SubaccountId: constants.Alice_Num0, + ClientId: 0, + OrderFlags: clobtypes.OrderIdFlags_LongTerm, + ClobPairId: TestMarketId, + }, + Quantums: 1_000_000_000_000, + Subticks: 1_000_000_000, + Side: clobtypes.Order_SIDE_BUY, + GoodTilOneof: &clobtypes.Order_GoodTilBlockTime{ + GoodTilBlockTime: uint32(GenesisTime.Add(1 * time.Hour).Unix()), + }, + } +) + func TestAddNewMarketProposal(t *testing.T) { - testId := uint32(1001) testMarketParam := pricestest.GenerateMarketParamPrice( - pricestest.WithId(testId), + pricestest.WithId(TestMarketId), ) testClobPair := clobtest.GenerateClobPair( - clobtest.WithId(testId), - clobtest.WithPerpetualId(testId), + clobtest.WithId(TestMarketId), + clobtest.WithPerpetualId(TestMarketId), clobtest.WithStatus(clobtypes.ClobPair_STATUS_INITIALIZING), ) testPerpetual := perptest.GeneratePerpetual( - perptest.WithId(testId), - perptest.WithMarketId(testId), + perptest.WithId(TestMarketId), + perptest.WithMarketId(TestMarketId), ) msgUpdateClobPairToActive := &clobtypes.MsgUpdateClobPair{ Authority: delaymsgtypes.ModuleAddress.String(), ClobPair: *clobtest.GenerateClobPair( - clobtest.WithId(testId), - clobtest.WithPerpetualId(testId), + clobtest.WithId(TestMarketId), + clobtest.WithPerpetualId(TestMarketId), clobtest.WithStatus(clobtypes.ClobPair_STATUS_ACTIVE), ), } @@ -46,15 +74,15 @@ func TestAddNewMarketProposal(t *testing.T) { Authority: delaymsgtypes.ModuleAddress.String(), ClobPair: *clobtest.GenerateClobPair( clobtest.WithId(9999), // non existing clob pair - clobtest.WithPerpetualId(testId), + clobtest.WithPerpetualId(TestMarketId), clobtest.WithStatus(clobtypes.ClobPair_STATUS_ACTIVE), ), } msgUpdateClobPairToActive_WrongAuthority := &clobtypes.MsgUpdateClobPair{ Authority: lib.GovModuleAddress.String(), ClobPair: *clobtest.GenerateClobPair( - clobtest.WithId(testId), - clobtest.WithPerpetualId(testId), + clobtest.WithId(TestMarketId), + clobtest.WithPerpetualId(TestMarketId), clobtest.WithStatus(clobtypes.ClobPair_STATUS_ACTIVE), ), } @@ -272,6 +300,7 @@ func TestAddNewMarketProposal(t *testing.T) { genesisState.Params.VotingPeriod = &testapp.TestVotingPeriod }, ) + genesis.GenesisTime = GenesisTime return genesis }).Build() ctx := tApp.InitChain() @@ -342,7 +371,32 @@ func TestAddNewMarketProposal(t *testing.T) { // Check that clob pair is updated. require.Equal(t, msgUpdateClobPairToActive.ClobPair, clobPair) - // TODO(CORE-585): Check that orders cannot be placed if no valid oracle price update has occurred. + + // Advance to some blocks after, and place an order on the market. + ctx = tApp.AdvanceToBlock(uint32(ctx.BlockHeight())+NumBlocksAfterTradingEnabled, testapp.AdvanceToBlockOptions{}) + price, err := tApp.App.PricesKeeper.GetMarketPrice(ctx, testMarketParam.Param.Id) + require.NoError(t, err) + // No oracle price updates were made. + require.Equal(t, uint64(0), price.Price) + + // Place an order on the market which is now ACTIVE with 0 oracle price. + checkTx := testapp.MustMakeCheckTxsWithClobMsg(ctx, tApp.App, *clobtypes.NewMsgPlaceOrder( + OrderTemplate_Alice_Num0_Id0_Clob0_Buy_LongTerm, + )) + resp := tApp.CheckTx(checkTx[0]) + require.Conditionf(t, resp.IsErr, "Expected CheckTx to error. Response: %+v", resp) + require.Contains(t, + resp.Log, + ExpectedPlaceOrderCheckTxResponseLog, + "expected CheckTx response log to contain: %s, got: %s", + ExpectedPlaceOrderCheckTxResponseLog, resp.Log, + ) + + // Advance to the next block and check chain is not halted. + tApp.AdvanceToBlock( + uint32(ctx.BlockHeight())+1, + testapp.AdvanceToBlockOptions{}, + ) default: t.Errorf("unexpected proposal status: %s", tc.expectedProposalStatus) } From 197f530af43edc459d3577d9f030ed9636ec8f94 Mon Sep 17 00:00:00 2001 From: dydxwill <119354122+dydxwill@users.noreply.github.com> Date: Fri, 10 Nov 2023 11:10:11 -0500 Subject: [PATCH 2/2] remove market enum from FillsType (#788) --- .../postgres/__tests__/helpers/constants.ts | 2 +- ...20231110103241_remove_market_fills_type.ts | 19 +++++++++++++++++++ .../packages/postgres/src/types/fill-types.ts | 2 -- .../comlink/public/api-documentation.md | 15 +++++++-------- indexer/services/comlink/public/swagger.json | 1 - .../comlink/public/websocket-documentation.md | 4 +--- .../ender/__tests__/helpers/constants.ts | 2 +- .../__tests__/lib/kafka-publisher.test.ts | 2 +- 8 files changed, 30 insertions(+), 17 deletions(-) create mode 100644 indexer/packages/postgres/src/db/migrations/migration_files/20231110103241_remove_market_fills_type.ts diff --git a/indexer/packages/postgres/__tests__/helpers/constants.ts b/indexer/packages/postgres/__tests__/helpers/constants.ts index b9c5361743..8dad14e10c 100644 --- a/indexer/packages/postgres/__tests__/helpers/constants.ts +++ b/indexer/packages/postgres/__tests__/helpers/constants.ts @@ -355,7 +355,7 @@ export const defaultFill: FillCreateObject = { subaccountId: defaultSubaccountId, side: OrderSide.BUY, liquidity: Liquidity.TAKER, - type: FillType.MARKET, + type: FillType.LIMIT, clobPairId: '1', orderId: defaultOrderId, size: '10', diff --git a/indexer/packages/postgres/src/db/migrations/migration_files/20231110103241_remove_market_fills_type.ts b/indexer/packages/postgres/src/db/migrations/migration_files/20231110103241_remove_market_fills_type.ts new file mode 100644 index 0000000000..8275ba90a9 --- /dev/null +++ b/indexer/packages/postgres/src/db/migrations/migration_files/20231110103241_remove_market_fills_type.ts @@ -0,0 +1,19 @@ +import * as Knex from 'knex'; + +import { formatAlterTableEnumSql } from '../helpers'; + +export async function up(knex: Knex): Promise { + return knex.raw(formatAlterTableEnumSql( + 'fills', + 'type', + ['LIMIT', 'LIQUIDATED', 'LIQUIDATION', 'DELEVERAGED', 'OFFSETTING'], + )); +} + +export async function down(knex: Knex): Promise { + return knex.raw(formatAlterTableEnumSql( + 'fills', + 'type', + ['MARKET', 'LIMIT', 'LIQUIDATED', 'LIQUIDATION', 'DELEVERAGED', 'OFFSETTING'], + )); +} diff --git a/indexer/packages/postgres/src/types/fill-types.ts b/indexer/packages/postgres/src/types/fill-types.ts index a46aa2781d..9055c8e09d 100644 --- a/indexer/packages/postgres/src/types/fill-types.ts +++ b/indexer/packages/postgres/src/types/fill-types.ts @@ -14,8 +14,6 @@ export enum Liquidity { } export enum FillType { - // MARKET is the fill type for a fill with a market taker order. - MARKET = 'MARKET', // LIMIT is the fill type for a fill with a limit taker order. LIMIT = 'LIMIT', // LIQUIDATED is for the taker side of the fill where the subaccount was liquidated. diff --git a/indexer/services/comlink/public/api-documentation.md b/indexer/services/comlink/public/api-documentation.md index 59a4c0395e..459cff2273 100644 --- a/indexer/services/comlink/public/api-documentation.md +++ b/indexer/services/comlink/public/api-documentation.md @@ -618,7 +618,7 @@ fetch('https://indexer.v4testnet.dydx.exchange/v4/fills?address=string&subaccoun "id": "string", "side": "BUY", "liquidity": "TAKER", - "type": "MARKET", + "type": "LIMIT", "market": "string", "marketType": "PERPETUAL", "price": "string", @@ -1639,7 +1639,7 @@ fetch('https://indexer.v4testnet.dydx.exchange/v4/trades/perpetualMarket/{ticker "side": "BUY", "size": "string", "price": "string", - "type": "MARKET", + "type": "LIMIT", "createdAt": "string", "createdAtHeight": "string" } @@ -2270,7 +2270,7 @@ This operation does not require authentication ```json -"MARKET" +"LIMIT" ``` @@ -2284,7 +2284,6 @@ This operation does not require authentication |Property|Value| |---|---| -|*anonymous*|MARKET| |*anonymous*|LIMIT| |*anonymous*|LIQUIDATED| |*anonymous*|LIQUIDATION| @@ -2328,7 +2327,7 @@ This operation does not require authentication "id": "string", "side": "BUY", "liquidity": "TAKER", - "type": "MARKET", + "type": "LIMIT", "market": "string", "marketType": "PERPETUAL", "price": "string", @@ -2374,7 +2373,7 @@ This operation does not require authentication "id": "string", "side": "BUY", "liquidity": "TAKER", - "type": "MARKET", + "type": "LIMIT", "market": "string", "marketType": "PERPETUAL", "price": "string", @@ -3063,7 +3062,7 @@ or "side": "BUY", "size": "string", "price": "string", - "type": "MARKET", + "type": "LIMIT", "createdAt": "string", "createdAtHeight": "string" } @@ -3097,7 +3096,7 @@ or "side": "BUY", "size": "string", "price": "string", - "type": "MARKET", + "type": "LIMIT", "createdAt": "string", "createdAtHeight": "string" } diff --git a/indexer/services/comlink/public/swagger.json b/indexer/services/comlink/public/swagger.json index ac9488555e..6e74f995c9 100644 --- a/indexer/services/comlink/public/swagger.json +++ b/indexer/services/comlink/public/swagger.json @@ -301,7 +301,6 @@ }, "FillType": { "enum": [ - "MARKET", "LIMIT", "LIQUIDATED", "LIQUIDATION", diff --git a/indexer/services/comlink/public/websocket-documentation.md b/indexer/services/comlink/public/websocket-documentation.md index bde62d3c89..b0b5f634b6 100644 --- a/indexer/services/comlink/public/websocket-documentation.md +++ b/indexer/services/comlink/public/websocket-documentation.md @@ -251,8 +251,6 @@ export enum Liquidity { } export enum FillType { - // MARKET is the fill type for a fill with a market taker order. - MARKET = 'MARKET', // LIMIT is the fill type for a fill with a limit taker order. LIMIT = 'LIMIT', // LIQUIDATED is for the taker side of the fill where the subaccount was liquidated. @@ -752,7 +750,7 @@ interface TradeContent { "price": "27837", "side": "SELL", "createdAt": "2023-04-04T00:29:19.353Z", - "type": "MARKET" + "type": "LIMIT" }, { "id": "dd1088b5-5cab-518f-a59c-4d5f735ab861", diff --git a/indexer/services/ender/__tests__/helpers/constants.ts b/indexer/services/ender/__tests__/helpers/constants.ts index 9f69cdd730..4e8dfbeba5 100644 --- a/indexer/services/ender/__tests__/helpers/constants.ts +++ b/indexer/services/ender/__tests__/helpers/constants.ts @@ -325,7 +325,7 @@ export const defaultTradeContent: TradeContent = { price: '10000', side: 'BUY', createdAt: 'createdAt', - type: FillType.MARKET, + type: FillType.LIMIT, }; export const defaultTradeMessage: SingleTradeMessage = contentToSingleTradeMessage( defaultTradeContent, diff --git a/indexer/services/ender/__tests__/lib/kafka-publisher.test.ts b/indexer/services/ender/__tests__/lib/kafka-publisher.test.ts index 6e915a86b0..184ddfd104 100644 --- a/indexer/services/ender/__tests__/lib/kafka-publisher.test.ts +++ b/indexer/services/ender/__tests__/lib/kafka-publisher.test.ts @@ -241,7 +241,7 @@ describe('kafka-publisher', () => { subaccountId: testConstants.defaultSubaccountId, side: OrderSide.BUY, liquidity: Liquidity.TAKER, - type: FillType.MARKET, + type: FillType.LIMIT, clobPairId: '1', orderId: testConstants.defaultOrderId, size: '10',