Skip to content

Commit

Permalink
FTX: Fix incorrect partially-cancelled order assignment (thrasher-cor…
Browse files Browse the repository at this point in the history 
…p#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 <[email protected]>
  • Loading branch information
@gloriousCode @MarkDzulko
gloriousCode and MarkDzulko authored Jul 30, 2021
1 parent 431c0fd commit bf5a24b
Show file tree
Hide file tree
Showing 5 changed files with 170 additions and 54 deletions.
12 changes: 12 additions & 0 deletions exchanges/ftx/ftx.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}
)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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():
Expand All @@ -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
Expand Down
185 changes: 142 additions & 43 deletions exchanges/ftx/ftx_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"errors"
"log"
"os"
"reflect"
"testing"
"time"

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
}
fee, err := f.GetFee(feeBuilder)
if err != nil {
t.Error(err)
}
x.FeeType = exchange.OfflineTradeFee
_, 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)
}
x.IsMaker = true
_, err = f.GetFee(&x)
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) {
Expand Down Expand Up @@ -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": {
Expand Down Expand Up @@ -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": {
Expand Down Expand Up @@ -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)
}
}

Expand Down
5 changes: 3 additions & 2 deletions exchanges/ftx/ftx_websocket.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
20 changes: 12 additions & 8 deletions exchanges/ftx/ftx_wrapper.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion exchanges/orderbook/unsafe_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

0 comments on commit bf5a24b

Please sign in to comment.