From bf5a24b3d721fe6e3aa9e08ee67fcbd186446b99 Mon Sep 17 00:00:00 2001 From: Scott Date: Fri, 30 Jul 2021 12:17:13 +1000 Subject: [PATCH] FTX: Fix incorrect partially-cancelled order assignment (#730) * Fixed issues that led to incorrect partially-cancelled status * Fixes all compatibleOrderVars args.Adds tests.Allow Offline fee testing Co-authored-by: Mark Dzulko --- exchanges/ftx/ftx.go | 12 ++ exchanges/ftx/ftx_test.go | 187 ++++++++++++++++++++++------- exchanges/ftx/ftx_websocket.go | 5 +- exchanges/ftx/ftx_wrapper.go | 20 +-- exchanges/orderbook/unsafe_test.go | 2 +- 5 files changed, 171 insertions(+), 55 deletions(-) diff --git a/exchanges/ftx/ftx.go b/exchanges/ftx/ftx.go index 9a7b73d4..4b36972f 100644 --- a/exchanges/ftx/ftx.go +++ b/exchanges/ftx/ftx.go @@ -135,6 +135,8 @@ var ( errCoinMustBeSpecified = errors.New("a coin must be specified") errSubaccountTransferSizeGreaterThanZero = errors.New("transfer size must be greater than 0") errSubaccountTransferSourceDestinationMustNotBeEqual = errors.New("subaccount transfer source and destination must not be the same value") + errUnrecognisedOrderStatus = errors.New("unrecognised order status received") + errInvalidOrderAmounts = errors.New("filled amount should not exceed order amount") validResolutionData = []int64{15, 60, 300, 900, 3600, 14400, 86400} ) @@ -1183,6 +1185,9 @@ func (f *FTX) SendAuthHTTPRequest(ep exchange.URL, method, path string, data, re // GetFee returns an estimate of fee based on type of transaction func (f *FTX) GetFee(feeBuilder *exchange.FeeBuilder) (float64, error) { var fee float64 + if !f.GetAuthenticatedAPISupport(exchange.RestAuthentication) { + feeBuilder.FeeType = exchange.OfflineTradeFee + } switch feeBuilder.FeeType { case exchange.OfflineTradeFee: fee = getOfflineTradeFee(feeBuilder) @@ -1213,6 +1218,9 @@ func getOfflineTradeFee(feeBuilder *exchange.FeeBuilder) float64 { } func (f *FTX) compatibleOrderVars(orderSide, orderStatus, orderType string, amount, filledAmount, avgFillPrice float64) (OrderVars, error) { + if filledAmount > amount { + return OrderVars{}, fmt.Errorf("%w, amount: %f filled: %f", errInvalidOrderAmounts, amount, filledAmount) + } var resp OrderVars switch orderSide { case order.Buy.Lower(): @@ -1228,13 +1236,17 @@ func (f *FTX) compatibleOrderVars(orderSide, orderStatus, orderType string, amou case closedStatus: if filledAmount != 0 && filledAmount != amount { resp.Status = order.PartiallyCancelled + break } if filledAmount == 0 { resp.Status = order.Cancelled + break } if filledAmount == amount { resp.Status = order.Filled } + default: + return resp, fmt.Errorf("%w %s", errUnrecognisedOrderStatus, orderStatus) } var feeBuilder exchange.FeeBuilder feeBuilder.PurchasePrice = avgFillPrice diff --git a/exchanges/ftx/ftx_test.go b/exchanges/ftx/ftx_test.go index c314eca1..e7ebcd50 100644 --- a/exchanges/ftx/ftx_test.go +++ b/exchanges/ftx/ftx_test.go @@ -4,7 +4,6 @@ import ( "errors" "log" "os" - "reflect" "testing" "time" @@ -52,11 +51,14 @@ func TestMain(m *testing.M) { log.Fatal(err) } - exchCfg.API.AuthenticatedSupport = true - exchCfg.API.AuthenticatedWebsocketSupport = true exchCfg.API.Credentials.Key = apiKey exchCfg.API.Credentials.Secret = apiSecret exchCfg.API.Credentials.Subaccount = subaccount + if areTestAPIKeysSet() { + // Only set auth to true when keys present as fee online calculation requires authentication + exchCfg.API.AuthenticatedSupport = true + exchCfg.API.AuthenticatedWebsocketSupport = true + } f.Websocket = sharedtestvalues.NewTestWebsocket() err = f.Setup(exchCfg) if err != nil { @@ -987,37 +989,44 @@ func TestFetchAccountInfo(t *testing.T) { func TestGetFee(t *testing.T) { t.Parallel() - var x exchange.FeeBuilder - x.PurchasePrice = 10 - x.Amount = 1 - x.IsMaker = true - var a float64 - var err error - if areTestAPIKeysSet() { - a, err = f.GetFee(&x) - if err != nil { - t.Error(err) - } - if a != 0.0039 { - t.Errorf("incorrect maker fee value") - } + feeBuilder := &exchange.FeeBuilder{ + PurchasePrice: 10, + Amount: 1, + IsMaker: true, } - x.IsMaker = false - if areTestAPIKeysSet() { - if _, err = f.GetFee(&x); err != nil { - t.Error(err) - } - } - x.FeeType = exchange.OfflineTradeFee - _, err = f.GetFee(&x) + fee, err := f.GetFee(feeBuilder) if err != nil { t.Error(err) } - x.IsMaker = true - _, err = f.GetFee(&x) + if fee <= 0 { + t.Errorf("incorrect maker fee value") + } + + feeBuilder.IsMaker = false + if fee, err = f.GetFee(feeBuilder); err != nil { + t.Error(err) + } + if fee <= 0 { + t.Errorf("incorrect maker fee value") + } + + feeBuilder.FeeType = exchange.OfflineTradeFee + fee, err = f.GetFee(feeBuilder) if err != nil { t.Error(err) } + if fee <= 0 { + t.Errorf("incorrect maker fee value") + } + + feeBuilder.IsMaker = true + fee, err = f.GetFee(feeBuilder) + if err != nil { + t.Error(err) + } + if fee <= 0 { + t.Errorf("incorrect maker fee value") + } } func TestGetOfflineTradingFee(t *testing.T) { @@ -1241,9 +1250,6 @@ func TestParsingWSTickerData(t *testing.T) { func TestParsingWSOrdersData(t *testing.T) { t.Parallel() - if !areTestAPIKeysSet() { - t.Skip("API keys required but not set, skipping test") - } data := []byte(`{ "channel": "orders", "data": { @@ -1272,9 +1278,6 @@ func TestParsingWSOrdersData(t *testing.T) { func TestParsingMarketsData(t *testing.T) { t.Parallel() - if !areTestAPIKeysSet() { - t.Skip("API keys required but not set, skipping test") - } data := []byte(`{"channel": "markets", "type": "partial", "data": { @@ -1401,20 +1404,116 @@ func TestTimestampFromFloat64(t *testing.T) { func TestCompatibleOrderVars(t *testing.T) { t.Parallel() - if !areTestAPIKeysSet() { - t.Skip("API keys required but not set, skipping test") - } - a, err := f.compatibleOrderVars("buy", "closed", "limit", 0.5, 0.5, 9500) + orderVars, err := f.compatibleOrderVars( + "buy", + "closed", + "limit", + 0.5, + 0.5, + 9500) if err != nil { t.Error(err) } - var b OrderVars - b.Side = order.Buy - b.OrderType = order.Limit - b.Status = order.Filled - b.Fee = a.Fee // having a preset value will fail because fees are calculated live with getaccount - if !reflect.DeepEqual(a, b) { - t.Errorf("incorrect compatible vars") + if orderVars.Side != order.Buy { + t.Errorf("received %v expected %v", orderVars.Side, order.Buy) + } + if orderVars.OrderType != order.Limit { + t.Errorf("received %v expected %v", orderVars.OrderType, order.Limit) + } + if orderVars.Status != order.Filled { + t.Errorf("received %v expected %v", orderVars.Status, order.Filled) + } + + orderVars, err = f.compatibleOrderVars( + "buy", + "closed", + "limit", + 0, + 0, + 9500) + if !errors.Is(err, nil) { + t.Errorf("received %v expected %v", err, nil) + } + if orderVars.Status != order.Cancelled { + t.Errorf("received %v expected %v", orderVars.Status, order.Cancelled) + } + + orderVars, err = f.compatibleOrderVars( + "buy", + "closed", + "limit", + 0.5, + 0.2, + 9500) + if !errors.Is(err, nil) { + t.Errorf("received %v expected %v", err, nil) + } + if orderVars.Status != order.PartiallyCancelled { + t.Errorf("received %v expected %v", orderVars.Status, order.PartiallyCancelled) + } + + orderVars, err = f.compatibleOrderVars( + "sell", + "closed", + "limit", + 1337, + 1337, + 9500) + if !errors.Is(err, nil) { + t.Errorf("received %v expected %v", err, nil) + } + if orderVars.Status != order.Filled { + t.Errorf("received %v expected %v", orderVars.Status, order.Filled) + } + + orderVars, err = f.compatibleOrderVars( + "buy", + "closed", + "limit", + 0.1, + 0.2, + 9500) + if !errors.Is(err, errInvalidOrderAmounts) { + t.Errorf("received %v expected %v", err, errInvalidOrderAmounts) + } + + orderVars, err = f.compatibleOrderVars( + "buy", + "fake", + "limit", + 0.3, + 0.2, + 9500) + if !errors.Is(err, errUnrecognisedOrderStatus) { + t.Errorf("received %v expected %v", err, errUnrecognisedOrderStatus) + } + + orderVars, err = f.compatibleOrderVars( + "buy", + "new", + "limit", + 0.3, + 0.2, + 9500) + if !errors.Is(err, nil) { + t.Errorf("received %v expected %v", err, nil) + } + if orderVars.Status != order.New { + t.Errorf("received %v expected %v", orderVars.Status, order.New) + } + + orderVars, err = f.compatibleOrderVars( + "buy", + "open", + "limit", + 0.3, + 0.2, + 9500) + if !errors.Is(err, nil) { + t.Errorf("received %v expected %v", err, nil) + } + if orderVars.Status != order.Open { + t.Errorf("received %v expected %v", orderVars.Status, order.Open) } } diff --git a/exchanges/ftx/ftx_websocket.go b/exchanges/ftx/ftx_websocket.go index 93a70189..ec044fc8 100644 --- a/exchanges/ftx/ftx_websocket.go +++ b/exchanges/ftx/ftx_websocket.go @@ -354,11 +354,12 @@ func (f *FTX) wsHandleData(respRaw []byte) error { resp.Pair = pair resp.RemainingAmount = resultData.OrderData.Size - resultData.OrderData.FilledSize var orderVars OrderVars - orderVars, err = f.compatibleOrderVars(resultData.OrderData.Side, + orderVars, err = f.compatibleOrderVars( + resultData.OrderData.Side, resultData.OrderData.Status, resultData.OrderData.OrderType, - resultData.OrderData.FilledSize, resultData.OrderData.Size, + resultData.OrderData.FilledSize, resultData.OrderData.AvgFillPrice) if err != nil { return err diff --git a/exchanges/ftx/ftx_wrapper.go b/exchanges/ftx/ftx_wrapper.go index 4b1cdfb3..82395e57 100644 --- a/exchanges/ftx/ftx_wrapper.go +++ b/exchanges/ftx/ftx_wrapper.go @@ -847,11 +847,12 @@ func (f *FTX) GetActiveOrders(getOrdersRequest *order.GetOrdersRequest) ([]order tempResp.Price = orderData[y].Price tempResp.RemainingAmount = orderData[y].RemainingSize var orderVars OrderVars - orderVars, err = f.compatibleOrderVars(orderData[y].Side, + orderVars, err = f.compatibleOrderVars( + orderData[y].Side, orderData[y].Status, orderData[y].OrderType, - orderData[y].FilledSize, orderData[y].Size, + orderData[y].FilledSize, orderData[y].AvgFillPrice) if err != nil { return resp, err @@ -884,11 +885,12 @@ func (f *FTX) GetActiveOrders(getOrdersRequest *order.GetOrdersRequest) ([]order tempResp.Price = triggerOrderData[z].AvgFillPrice tempResp.RemainingAmount = triggerOrderData[z].Size - triggerOrderData[z].FilledSize tempResp.TriggerPrice = triggerOrderData[z].TriggerPrice - orderVars, err := f.compatibleOrderVars(triggerOrderData[z].Side, + orderVars, err := f.compatibleOrderVars( + triggerOrderData[z].Side, triggerOrderData[z].Status, triggerOrderData[z].OrderType, - triggerOrderData[z].FilledSize, triggerOrderData[z].Size, + triggerOrderData[z].FilledSize, triggerOrderData[z].AvgFillPrice) if err != nil { return resp, err @@ -945,11 +947,12 @@ func (f *FTX) GetOrderHistory(getOrdersRequest *order.GetOrdersRequest) ([]order tempResp.Price = orderData[y].Price tempResp.RemainingAmount = orderData[y].RemainingSize var orderVars OrderVars - orderVars, err = f.compatibleOrderVars(orderData[y].Side, + orderVars, err = f.compatibleOrderVars( + orderData[y].Side, orderData[y].Status, orderData[y].OrderType, - orderData[y].FilledSize, orderData[y].Size, + orderData[y].FilledSize, orderData[y].AvgFillPrice) if err != nil { return resp, err @@ -985,11 +988,12 @@ func (f *FTX) GetOrderHistory(getOrdersRequest *order.GetOrdersRequest) ([]order tempResp.Price = triggerOrderData[z].AvgFillPrice tempResp.RemainingAmount = triggerOrderData[z].Size - triggerOrderData[z].FilledSize tempResp.TriggerPrice = triggerOrderData[z].TriggerPrice - orderVars, err := f.compatibleOrderVars(triggerOrderData[z].Side, + orderVars, err := f.compatibleOrderVars( + triggerOrderData[z].Side, triggerOrderData[z].Status, triggerOrderData[z].OrderType, - triggerOrderData[z].FilledSize, triggerOrderData[z].Size, + triggerOrderData[z].FilledSize, triggerOrderData[z].AvgFillPrice) if err != nil { return resp, err diff --git a/exchanges/orderbook/unsafe_test.go b/exchanges/orderbook/unsafe_test.go index 60abd9f4..3188787b 100644 --- a/exchanges/orderbook/unsafe_test.go +++ b/exchanges/orderbook/unsafe_test.go @@ -22,7 +22,7 @@ func TestUnsafe(t *testing.T) { ob2 := &externalBook{} ob.Lock() - ob.Unlock() // nolint:go-staticcheck // Not needed in test + ob.Unlock() // nolint:staticcheck // Not needed in test ob.LockWith(ob2) ob.UnlockWith(ob2) }