From 8ce30a83752c6ff874fd5db0a037f78a1a8dde12 Mon Sep 17 00:00:00 2001 From: Gareth Kirwan Date: Thu, 14 Dec 2023 05:53:39 +0100 Subject: [PATCH] Okx: Fix GetBlockTrades and enhance test coverage (#1416) * Okx: Remove UpdateTradablePairs from TestMain When running individual tests we do not want to be forced to update trading pairs. It also does not seem to be required, since disabling UpdateTradingPairs entirely doesn't stop any tests from passing. If it is required we can enact this pattern: ``` func TestUpdateTradablePairs(t *testing.T) { t.Parallel() updatePairsOnce(t) } var updatePairsGuard sync.Once func updatePairsOnce(tb testing.TB) { tb.Helper() updatePairsGuard.Do(func() { err := ok.UpdateTradablePairs(context.Background(), true) assert.NoError(tb, err, "UpdateTradablePairs should not error") }) ``` * Okx: Fix GetBlockTrades url Endpoint was wrong Not convinced we should prefix with market vs public, but not committing a driveby * Okx: Expand test coverage for GetBlockTrades * Okx: Fix GetPublicBlockTrades Was marked as an authenticated API call, and it's not. Also renames it because without the context of "Block" (or RFQ) it's misleading. Adds tests. * Okx: Fix GetPublicBlockTrades to be public Expand coverage of GetBlockTrade and GetPublicBlockTrades --- exchanges/okx/okx.go | 12 +++--- exchanges/okx/okx_test.go | 88 ++++++++++++++++++++++++++++++++------ exchanges/okx/okx_types.go | 63 ++++++++++++++------------- 3 files changed, 113 insertions(+), 50 deletions(-) diff --git a/exchanges/okx/okx.go b/exchanges/okx/okx.go index 7d7fafd7..04710781 100644 --- a/exchanges/okx/okx.go +++ b/exchanges/okx/okx.go @@ -109,7 +109,6 @@ const ( marketIndexComponents = "market/index-components" marketBlockTickers = "market/block-tickers" marketBlockTicker = "market/block-ticker" - marketBlockTrades = "market/block-trades" // Public endpoints publicInstruments = "public/instruments" @@ -130,6 +129,7 @@ const ( publicUnderlyings = "public/underlying" publicInsuranceFunds = "public/insurance-fund" publicCurrencyConvertContract = "public/convert-contract-coin" + publicBlockTrades = "public/block-trades" // Trading Endpoints tradingDataSupportedCoins = "rubik/stat/trading-data/support-coin" @@ -1361,8 +1361,8 @@ func (ok *Okx) GetRfqTrades(ctx context.Context, arg *RfqTradesRequestParams) ([ return resp, ok.SendHTTPRequest(ctx, exchange.RestSpot, getTradesEPL, http.MethodGet, common.EncodeURLValues(rfqTrades, params), nil, &resp, true) } -// GetPublicTrades retrieves the recent executed block trades. -func (ok *Okx) GetPublicTrades(ctx context.Context, beginID, endID string, limit int64) ([]PublicTradesResponse, error) { +// GetPublicBlockTrades retrieves the recent executed block trades. +func (ok *Okx) GetPublicBlockTrades(ctx context.Context, beginID, endID string, limit int64) ([]PublicBlockTradesResponse, error) { params := url.Values{} if beginID != "" { params.Set("beginId", beginID) @@ -1373,8 +1373,8 @@ func (ok *Okx) GetPublicTrades(ctx context.Context, beginID, endID string, limit if limit > 0 { params.Set("limit", strconv.FormatInt(limit, 10)) } - var resp []PublicTradesResponse - return resp, ok.SendHTTPRequest(ctx, exchange.RestSpot, getPublicTradesEPL, http.MethodGet, common.EncodeURLValues(rfqPublicTrades, params), nil, &resp, true) + var resp []PublicBlockTradesResponse + return resp, ok.SendHTTPRequest(ctx, exchange.RestSpot, getPublicTradesEPL, http.MethodGet, common.EncodeURLValues(rfqPublicTrades, params), nil, &resp, false) } /*************************************** Funding Tradings ********************************/ @@ -3348,7 +3348,7 @@ func (ok *Okx) GetBlockTrades(ctx context.Context, instrumentID string) ([]Block } params.Set("instId", instrumentID) var resp []BlockTrade - return resp, ok.SendHTTPRequest(ctx, exchange.RestSpot, getBlockTradesEPL, http.MethodGet, common.EncodeURLValues(marketBlockTrades, params), nil, &resp, false) + return resp, ok.SendHTTPRequest(ctx, exchange.RestSpot, getBlockTradesEPL, http.MethodGet, common.EncodeURLValues(publicBlockTrades, params), nil, &resp, false) } /************************************ Public Data Endpoinst *************************************************/ diff --git a/exchanges/okx/okx_test.go b/exchanges/okx/okx_test.go index bacb1187..a07a2181 100644 --- a/exchanges/okx/okx_test.go +++ b/exchanges/okx/okx_test.go @@ -73,10 +73,6 @@ func TestMain(m *testing.M) { ok.Websocket.TrafficAlert = sharedtestvalues.GetWebsocketStructChannelOverride() setupWS() } - err = ok.UpdateTradablePairs(contextGenerate(), true) - if err != nil { - log.Fatal(err) - } os.Exit(m.Run()) } @@ -214,9 +210,55 @@ func TestGetBlockTicker(t *testing.T) { func TestGetBlockTrade(t *testing.T) { t.Parallel() - if _, err := ok.GetBlockTrades(contextGenerate(), "BTC-USDT"); err != nil { - t.Error("Okx GetBlockTrades() error", err) + trades, err := ok.GetBlockTrades(contextGenerate(), "BTC-USDT") + assert.NoError(t, err, "GetBlockTrades should not error") + if assert.NotEmpty(t, trades, "Should get some block trades") { + trade := trades[0] + assert.Equal(t, "BTC-USDT", trade.InstrumentID, "InstrumentID should have correct value") + assert.NotEmpty(t, trade.TradeID, "TradeID should not be empty") + assert.Positive(t, trade.Price, "Price should have a positive value") + assert.Positive(t, trade.Size, "Size should have a positive value") + assert.Contains(t, []order.Side{order.Buy, order.Sell}, trade.Side, "Side should be a side") + assert.WithinRange(t, trade.Timestamp.Time(), time.Now().Add(time.Hour*-24*7), time.Now(), "Timestamp should be within last 7 days") } + + updatePairsOnce(t) + + pairs, err := ok.GetAvailablePairs(asset.Options) + assert.NoError(t, err, "GetAvailablePairs should not error") + assert.NotEmpty(t, pairs, "Should get some Option pairs") + + publicTrades, err := ok.GetPublicBlockTrades(contextGenerate(), "", "", 100) + assert.NoError(t, err, "GetPublicBlockTrades should not error") + + tested := false +LOOP: + for _, trade := range publicTrades { + for _, leg := range trade.Legs { + p, err := ok.MatchSymbolWithAvailablePairs(leg.InstrumentID, asset.Options, true) + if err != nil { + continue + } + + trades, err = ok.GetBlockTrades(contextGenerate(), p.String()) + assert.NoError(t, err, "GetBlockTrades should not error on Options") + for _, trade := range trades { + assert.Equal(t, p.String(), trade.InstrumentID, "InstrumentID should have correct value") + assert.NotEmpty(t, trade.TradeID, "TradeID should not be empty") + assert.Positive(t, trade.Price, "Price should have a positive value") + assert.Positive(t, trade.Size, "Size should have a positive value") + assert.Contains(t, []order.Side{order.Buy, order.Sell}, trade.Side, "Side should be a side") + assert.Positive(t, trade.FillVolatility, "FillVolatility should have a positive value") + assert.Positive(t, trade.ForwardPrice, "ForwardPrice should have a positive value") + assert.Positive(t, trade.IndexPrice, "IndexPrice should have a positive value") + assert.Positive(t, trade.MarkPrice, "MarkPrice should have a positive value") + assert.NotEmpty(t, trade.Timestamp, "Timestamp should not be empty") + tested = true + break LOOP + } + } + } + assert.True(t, tested, "Should find at least one BlockTrade somewhere") } func TestGetInstrument(t *testing.T) { @@ -999,12 +1041,22 @@ func TestGetRfqTrades(t *testing.T) { } } -func TestGetPublicTrades(t *testing.T) { +func TestGetPublicBlockTrades(t *testing.T) { t.Parallel() - sharedtestvalues.SkipTestIfCredentialsUnset(t, ok) - - if _, err := ok.GetPublicTrades(contextGenerate(), "", "", 3); err != nil { - t.Error("Okx GetPublicTrades() error", err) + trades, err := ok.GetPublicBlockTrades(contextGenerate(), "", "", 3) + assert.NoError(t, err, "GetPublicBlockTrades should not error") + assert.NotEmpty(t, trades, "Should get some block trades back") + for _, trade := range trades { + assert.NotEmpty(t, trade.CreationTime, "CreationTime shound not be empty") + assert.NotEmpty(t, trade.BlockTradeID, "BlockTradeID shound not be empty") + if assert.NotEmpty(t, trade.Legs, "Should get some trades") { + leg := trade.Legs[0] + assert.NotEmpty(t, leg.InstrumentID, "InstrumentID should have correct value") + assert.NotEmpty(t, leg.TradeID, "TradeID should not be empty") + assert.Positive(t, leg.Price, "Price should have a positive value") + assert.Positive(t, leg.Size, "Size should have a positive value") + assert.Contains(t, []order.Side{order.Buy, order.Sell}, leg.Side, "Side should be a side") + } } } @@ -1883,9 +1935,17 @@ func TestFetchTradablePairs(t *testing.T) { func TestUpdateTradablePairs(t *testing.T) { t.Parallel() - if err := ok.UpdateTradablePairs(contextGenerate(), true); err != nil { - t.Error("Okx UpdateTradablePairs() error", err) - } + updatePairsOnce(t) +} + +var updatePairsGuard sync.Once + +func updatePairsOnce(tb testing.TB) { + tb.Helper() + updatePairsGuard.Do(func() { + err := ok.UpdateTradablePairs(context.Background(), true) + assert.NoError(tb, err, "UpdateTradablePairs should not error") + }) } func TestUpdateOrderExecutionLimits(t *testing.T) { diff --git a/exchanges/okx/okx_types.go b/exchanges/okx/okx_types.go index e02bc1f3..2dcef163 100644 --- a/exchanges/okx/okx_types.go +++ b/exchanges/okx/okx_types.go @@ -1843,34 +1843,33 @@ type RfqTradesRequestParams struct { // RfqTradeResponse Rfq trade response type RfqTradeResponse struct { - RfqID string `json:"rfqId"` - ClientRfqID string `json:"clRfqId"` - QuoteID string `json:"quoteId"` - ClientQuoteID string `json:"clQuoteId"` - BlockTradeID string `json:"blockTdId"` - Legs []RfqTradeLeg `json:"legs"` - CreationTime time.Time `json:"cTime"` - TakerTraderCode string `json:"tTraderCode"` - MakerTraderCode string `json:"mTraderCode"` + RfqID string `json:"rfqId"` + ClientRfqID string `json:"clRfqId"` + QuoteID string `json:"quoteId"` + ClientQuoteID string `json:"clQuoteId"` + BlockTradeID string `json:"blockTdId"` + Legs []BlockTradeLeg `json:"legs"` + CreationTime time.Time `json:"cTime"` + TakerTraderCode string `json:"tTraderCode"` + MakerTraderCode string `json:"mTraderCode"` } -// RfqTradeLeg Rfq trade response leg. -type RfqTradeLeg struct { - InstrumentID string `json:"instId"` - Side string `json:"side"` - Size string `json:"sz"` - Price float64 `json:"px,string"` - TradeID string `json:"tradeId"` - - Fee float64 `json:"fee,string,omitempty"` - FeeCurrency string `json:"feeCcy,omitempty"` +// BlockTradeLeg Rfq trade response leg. +type BlockTradeLeg struct { + TradeID string `json:"tradeId"` + InstrumentID string `json:"instId"` + Side order.Side `json:"side"` + Size convert.StringToFloat64 `json:"sz"` + Price convert.StringToFloat64 `json:"px"` + Fee convert.StringToFloat64 `json:"fee,omitempty"` + FeeCurrency string `json:"feeCcy,omitempty"` } -// PublicTradesResponse represents data will be pushed whenever there is a block trade. -type PublicTradesResponse struct { +// PublicBlockTradesResponse represents data will be pushed whenever there is a block trade. +type PublicBlockTradesResponse struct { BlockTradeID string `json:"blockTdId"` CreationTime okxUnixMilliTime `json:"cTime"` - Legs []RfqTradeLeg `json:"legs"` + Legs []BlockTradeLeg `json:"legs"` } // SubaccountInfo represents subaccount information detail. @@ -2169,12 +2168,16 @@ type BlockTicker struct { // BlockTrade represents a block trade. type BlockTrade struct { - InstrumentID string `json:"instId"` - TradeID string `json:"tradeId"` - Price float64 `json:"px,string"` - Size float64 `json:"sz,string"` - Side order.Side `json:"side"` - Timestamp okxUnixMilliTime `json:"ts"` + InstrumentID string `json:"instId"` + TradeID string `json:"tradeId"` + Price convert.StringToFloat64 `json:"px"` + Size convert.StringToFloat64 `json:"sz"` + Side order.Side `json:"side"` + FillVolatility convert.StringToFloat64 `json:"fillVol"` + ForwardPrice convert.StringToFloat64 `json:"fwdPx"` + IndexPrice convert.StringToFloat64 `json:"idxPx"` + MarkPrice convert.StringToFloat64 `json:"markPx"` + Timestamp convert.ExchangeTime `json:"ts"` } // UnitConvertResponse unit convert response. @@ -2915,8 +2918,8 @@ type WsSystemStatusResponse struct { // WsPublicTradesResponse represents websocket push data of structured block trades as a result of subscription to "public-struc-block-trades" type WsPublicTradesResponse struct { - Argument SubscriptionInfo `json:"arg"` - Data []PublicTradesResponse `json:"data"` + Argument SubscriptionInfo `json:"arg"` + Data []PublicBlockTradesResponse `json:"data"` } // WsBlockTicker represents websocket push data as a result of subscription to channel "block-tickers".