btcm: add modify order functionality, change return to pointer (thras…

…her-corp#940) * btcm: add modify order functionality, change return to pointer * glorious: nits * glorious: nits * btcm: Adjust function name * thrasher: nits * thrasher: nits cont...
ki39 · May 16, 2022 · 16a93b4 · 16a93b4
1 parent ccde38d
commit 16a93b4
Show file tree

Hide file tree

Showing 35 changed files with 241 additions and 114 deletions.
diff --git a/README.md b/README.md
@@ -143,11 +143,11 @@ Binaries will be published once the codebase reaches a stable condition.
 
 |User|Contribution Amount|
 |--|--|
-| [thrasher-](https://github.com/thrasher-) | 664 |
-| [shazbert](https://github.com/shazbert) | 232 |
-| [gloriousCode](https://github.com/gloriousCode) | 194 |
+| [thrasher-](https://github.com/thrasher-) | 666 |
+| [shazbert](https://github.com/shazbert) | 248 |
+| [gloriousCode](https://github.com/gloriousCode) | 195 |
 | [dependabot-preview[bot]](https://github.com/apps/dependabot-preview) | 88 |
-| [dependabot[bot]](https://github.com/apps/dependabot) | 57 |
+| [dependabot[bot]](https://github.com/apps/dependabot) | 73 |
 | [xtda](https://github.com/xtda) | 47 |
 | [lrascao](https://github.com/lrascao) | 27 |
 | [Rots](https://github.com/Rots) | 15 |
@@ -160,7 +160,7 @@ Binaries will be published once the codebase reaches a stable condition.
 | [marcofranssen](https://github.com/marcofranssen) | 8 |
 | [dackroyd](https://github.com/dackroyd) | 5 |
 | [cranktakular](https://github.com/cranktakular) | 5 |
-| [khcchiu](https://github.com/khcchiu) | 4 |
+| [khcchiu](https://github.com/khcchiu) | 5 |
 | [woshidama323](https://github.com/woshidama323) | 3 |
 | [yangrq1018](https://github.com/yangrq1018) | 3 |
 | [TaltaM](https://github.com/TaltaM) | 3 |

diff --git a/cmd/exchange_template/wrapper_file.tmpl b/cmd/exchange_template/wrapper_file.tmpl
@@ -403,11 +403,11 @@ func ({{.Variable}} *{{.CapitalName}}) SubmitOrder(ctx context.Context, s *order
 
 // ModifyOrder will allow of changing orderbook placement and limit to
 // market conversion
-func ({{.Variable}} *{{.CapitalName}}) ModifyOrder(ctx context.Context, action *order.Modify) (order.Modify, error) {
+func ({{.Variable}} *{{.CapitalName}}) ModifyOrder(ctx context.Context, action *order.Modify) (*order.Modify, error) {
 	// if err := action.Validate(); err != nil {
 	// 	return "", err
 	// }
-	return order.Modify{}, common.ErrNotYetImplemented
+	return nil, common.ErrNotYetImplemented
 }
 
 // CancelOrder cancels an order by its corresponding ID number

diff --git a/engine/order_manager.go b/engine/order_manager.go
@@ -401,7 +401,7 @@ func (m *OrderManager) Modify(ctx context.Context, mod *order.Modify) (*order.Mo
 	//
 	// XXX: This comes with a race condition, because [request -> changes] are not
 	// atomic.
-	err = m.orderStore.modifyExisting(mod.ID, &res)
+	err = m.orderStore.modifyExisting(mod.ID, res)
 
 	// Notify observers.
 	var message string

diff --git a/engine/order_manager_test.go b/engine/order_manager_test.go
@@ -84,10 +84,10 @@ func (f omfExchange) GetActiveOrders(ctx context.Context, req *order.GetOrdersRe
 	}}, nil
 }
 
-func (f omfExchange) ModifyOrder(ctx context.Context, action *order.Modify) (order.Modify, error) {
+func (f omfExchange) ModifyOrder(ctx context.Context, action *order.Modify) (*order.Modify, error) {
 	ans := *action
 	ans.ID = "modified_order_id"
-	return ans, nil
+	return &ans, nil
 }
 
 func TestSetupOrderManager(t *testing.T) {

diff --git a/exchanges/binance/binance_wrapper.go b/exchanges/binance/binance_wrapper.go
@@ -1063,8 +1063,8 @@ func (b *Binance) SubmitOrder(ctx context.Context, s *order.Submit) (order.Submi
 
 // ModifyOrder will allow of changing orderbook placement and limit to
 // market conversion
-func (b *Binance) ModifyOrder(ctx context.Context, action *order.Modify) (order.Modify, error) {
-	return order.Modify{}, common.ErrFunctionNotSupported
+func (b *Binance) ModifyOrder(_ context.Context, _ *order.Modify) (*order.Modify, error) {
+	return nil, common.ErrFunctionNotSupported
 }
 
 // CancelOrder cancels an order by its corresponding ID number

diff --git a/exchanges/bitfinex/bitfinex_wrapper.go b/exchanges/bitfinex/bitfinex_wrapper.go
@@ -675,14 +675,14 @@ func (b *Bitfinex) SubmitOrder(ctx context.Context, o *order.Submit) (order.Subm
 
 // ModifyOrder will allow of changing orderbook placement and limit to
 // market conversion
-func (b *Bitfinex) ModifyOrder(ctx context.Context, action *order.Modify) (order.Modify, error) {
+func (b *Bitfinex) ModifyOrder(ctx context.Context, action *order.Modify) (*order.Modify, error) {
 	if err := action.Validate(); err != nil {
-		return order.Modify{}, err
+		return nil, err
 	}
 
 	orderIDInt, err := strconv.ParseInt(action.ID, 10, 64)
 	if err != nil {
-		return order.Modify{ID: action.ID}, err
+		return &order.Modify{ID: action.ID}, err
 	}
 	if b.Websocket.CanUseAuthenticatedWebsocketForWrapper() {
 		request := WsUpdateOrderRequest{
@@ -694,17 +694,16 @@ func (b *Bitfinex) ModifyOrder(ctx context.Context, action *order.Modify) (order
 			request.Amount *= -1
 		}
 		err = b.WsModifyOrder(&request)
-		return order.Modify{
+		return &order.Modify{
 			Exchange:  action.Exchange,
 			AssetType: action.AssetType,
 			Pair:      action.Pair,
 			ID:        action.ID,
-
-			Price:  action.Price,
-			Amount: action.Amount,
+			Price:     action.Price,
+			Amount:    action.Amount,
 		}, err
 	}
-	return order.Modify{}, common.ErrNotYetImplemented
+	return nil, common.ErrNotYetImplemented
 }
 
 // CancelOrder cancels an order by its corresponding ID number

diff --git a/exchanges/bitflyer/bitflyer_wrapper.go b/exchanges/bitflyer/bitflyer_wrapper.go
@@ -395,8 +395,8 @@ func (b *Bitflyer) SubmitOrder(_ context.Context, _ *order.Submit) (order.Submit
 
 // ModifyOrder will allow of changing orderbook placement and limit to
 // market conversion
-func (b *Bitflyer) ModifyOrder(_ context.Context, _ *order.Modify) (order.Modify, error) {
-	return order.Modify{}, common.ErrFunctionNotSupported
+func (b *Bitflyer) ModifyOrder(_ context.Context, _ *order.Modify) (*order.Modify, error) {
+	return nil, common.ErrFunctionNotSupported
 }
 
 // CancelOrder cancels an order by its corresponding ID number

diff --git a/exchanges/bithumb/bithumb_wrapper.go b/exchanges/bithumb/bithumb_wrapper.go
@@ -512,9 +512,9 @@ func (b *Bithumb) SubmitOrder(ctx context.Context, s *order.Submit) (order.Submi
 
 // ModifyOrder will allow of changing orderbook placement and limit to
 // market conversion
-func (b *Bithumb) ModifyOrder(ctx context.Context, action *order.Modify) (order.Modify, error) {
+func (b *Bithumb) ModifyOrder(ctx context.Context, action *order.Modify) (*order.Modify, error) {
 	if err := action.Validate(); err != nil {
-		return order.Modify{}, err
+		return nil, err
 	}
 
 	o, err := b.ModifyTrade(ctx,
@@ -523,20 +523,18 @@ func (b *Bithumb) ModifyOrder(ctx context.Context, action *order.Modify) (order.
 		action.Side.Lower(),
 		action.Amount,
 		int64(action.Price))
-
 	if err != nil {
-		return order.Modify{}, err
+		return nil, err
 	}
 
-	return order.Modify{
+	return &order.Modify{
 		Exchange:  action.Exchange,
 		AssetType: action.AssetType,
 		Pair:      action.Pair,
 		ID:        o.Data[0].ContID,
-
-		Price:  float64(int64(action.Price)),
-		Amount: action.Amount,
-		Side:   action.Side,
+		Price:     float64(int64(action.Price)),
+		Amount:    action.Amount,
+		Side:      action.Side,
 	}, nil
 }
 

diff --git a/exchanges/bitmex/bitmex_wrapper.go b/exchanges/bitmex/bitmex_wrapper.go
@@ -609,15 +609,15 @@ func (b *Bitmex) SubmitOrder(ctx context.Context, s *order.Submit) (order.Submit
 
 // ModifyOrder will allow of changing orderbook placement and limit to
 // market conversion
-func (b *Bitmex) ModifyOrder(ctx context.Context, action *order.Modify) (order.Modify, error) {
+func (b *Bitmex) ModifyOrder(ctx context.Context, action *order.Modify) (*order.Modify, error) {
 	if err := action.Validate(); err != nil {
-		return order.Modify{}, err
+		return nil, err
 	}
 
 	var params OrderAmendParams
 
 	if math.Mod(action.Amount, 1) != 0 {
-		return order.Modify{}, errors.New("contract amount can not have decimals")
+		return nil, errors.New("contract amount can not have decimals")
 	}
 
 	params.OrderID = action.ID
@@ -626,17 +626,16 @@ func (b *Bitmex) ModifyOrder(ctx context.Context, action *order.Modify) (order.M
 
 	o, err := b.AmendOrder(ctx, &params)
 	if err != nil {
-		return order.Modify{}, err
+		return nil, err
 	}
 
-	return order.Modify{
+	return &order.Modify{
 		Exchange:  action.Exchange,
 		AssetType: action.AssetType,
 		Pair:      action.Pair,
 		ID:        o.OrderID,
-
-		Price:  action.Price,
-		Amount: float64(params.OrderQty),
+		Price:     action.Price,
+		Amount:    float64(params.OrderQty),
 	}, nil
 }
 

diff --git a/exchanges/bitstamp/bitstamp_wrapper.go b/exchanges/bitstamp/bitstamp_wrapper.go
@@ -561,8 +561,8 @@ func (b *Bitstamp) SubmitOrder(ctx context.Context, s *order.Submit) (order.Subm
 
 // ModifyOrder will allow of changing orderbook placement and limit to
 // market conversion
-func (b *Bitstamp) ModifyOrder(ctx context.Context, action *order.Modify) (order.Modify, error) {
-	return order.Modify{}, common.ErrFunctionNotSupported
+func (b *Bitstamp) ModifyOrder(_ context.Context, _ *order.Modify) (*order.Modify, error) {
+	return nil, common.ErrFunctionNotSupported
 }
 
 // CancelOrder cancels an order by its corresponding ID number

diff --git a/exchanges/bittrex/bittrex_wrapper.go b/exchanges/bittrex/bittrex_wrapper.go
@@ -587,8 +587,8 @@ func (b *Bittrex) SubmitOrder(ctx context.Context, s *order.Submit) (order.Submi
 
 // ModifyOrder will allow of changing orderbook placement and limit to
 // market conversion
-func (b *Bittrex) ModifyOrder(_ context.Context, _ *order.Modify) (order.Modify, error) {
-	return order.Modify{}, common.ErrFunctionNotSupported
+func (b *Bittrex) ModifyOrder(_ context.Context, _ *order.Modify) (*order.Modify, error) {
+	return nil, common.ErrFunctionNotSupported
 }
 
 // CancelOrder cancels an order by its corresponding ID number

diff --git a/exchanges/btcmarkets/btcmarkets.go b/exchanges/btcmarkets/btcmarkets.go
@@ -21,6 +21,11 @@ import (
 	"github.com/thrasher-corp/gocryptotrader/exchanges/request"
 )
 
+var (
+	errInvalidAmount = errors.New("cannot be less than or equal to zero")
+	errIDRequired    = errors.New("id is required")
+)
+
 const (
 	btcMarketsAPIURL     = "https://api.btcmarkets.net"
 	btcMarketsAPIVersion = "/v3"
@@ -464,8 +469,8 @@ func (b *BTCMarkets) CancelAllOpenOrdersByPairs(ctx context.Context, marketIDs [
 }
 
 // FetchOrder finds order based on the provided id
-func (b *BTCMarkets) FetchOrder(ctx context.Context, id string) (OrderData, error) {
-	var resp OrderData
+func (b *BTCMarkets) FetchOrder(ctx context.Context, id string) (*OrderData, error) {
+	var resp *OrderData
 	return resp, b.SendAuthenticatedRequest(ctx, http.MethodGet,
 		btcMarketsOrders+"/"+id,
 		nil,
@@ -483,6 +488,36 @@ func (b *BTCMarkets) RemoveOrder(ctx context.Context, id string) (CancelOrderRes
 		request.Auth)
 }
 
+// ReplaceOrder cancels an order and then places a new order.
+func (b *BTCMarkets) ReplaceOrder(ctx context.Context, id, clientOrderID string, price, amount float64) (*OrderData, error) {
+	if price <= 0 {
+		return nil, fmt.Errorf("price %w", errInvalidAmount)
+	}
+
+	if amount <= 0 {
+		return nil, fmt.Errorf("amount %w", errInvalidAmount)
+	}
+
+	if id == "" {
+		return nil, errIDRequired
+	}
+
+	req := make(map[string]interface{}, 3)
+	req["price"] = strconv.FormatFloat(price, 'f', -1, 64)
+	req["amount"] = strconv.FormatFloat(amount, 'f', -1, 64)
+	if clientOrderID != "" {
+		req["clientOrderId"] = clientOrderID
+	}
+
+	var resp *OrderData
+	return resp, b.SendAuthenticatedRequest(ctx,
+		http.MethodPut,
+		btcMarketsOrders+"/"+id,
+		req,
+		&resp,
+		request.Auth)
+}
+
 // ListWithdrawals lists the withdrawal history
 func (b *BTCMarkets) ListWithdrawals(ctx context.Context, before, after, limit int64) ([]TransferData, error) {
 	if (before > 0) && (after >= 0) {

diff --git a/exchanges/btcmarkets/btcmarkets_test.go b/exchanges/btcmarkets/btcmarkets_test.go
@@ -1041,6 +1041,60 @@ func TestGetTimeInForce(t *testing.T) {
 	}
 }
 
+func TestReplaceOrder(t *testing.T) {
+	t.Parallel()
+	_, err := b.ReplaceOrder(context.Background(), "", "bro", 0, 0)
+	if !errors.Is(err, errInvalidAmount) {
+		t.Fatalf("received: '%v' but expected: '%v'", err, errInvalidAmount)
+	}
+
+	_, err = b.ReplaceOrder(context.Background(), "", "bro", 1, 0)
+	if !errors.Is(err, errInvalidAmount) {
+		t.Fatalf("received: '%v' but expected: '%v'", err, errInvalidAmount)
+	}
+
+	_, err = b.ReplaceOrder(context.Background(), "", "bro", 1, 1)
+	if !errors.Is(err, errIDRequired) {
+		t.Fatalf("received: '%v' but expected: '%v'", err, errIDRequired)
+	}
+
+	if !areTestAPIKeysSet() || !canManipulateRealOrders {
+		t.Skip("skipping test, either api keys or manipulaterealorders isnt set correctly")
+	}
+
+	_, err = b.ReplaceOrder(context.Background(), "8207096301", "bruh", 100000, 0.001)
+	if !errors.Is(err, nil) {
+		t.Fatalf("received: '%v' but expected: '%v'", err, nil)
+	}
+}
+
+func TestWrapperModifyOrder(t *testing.T) {
+	t.Parallel()
+	_, err := b.ModifyOrder(context.Background(), &order.Modify{})
+	if !errors.Is(err, order.ErrPairIsEmpty) {
+		t.Fatalf("received: '%v' but expected: '%v'", err, order.ErrPairIsEmpty)
+	}
+
+	if !areTestAPIKeysSet() || !canManipulateRealOrders {
+		t.Skip("skipping test, either api keys or manipulaterealorders isnt set correctly")
+	}
+	mo, err := b.ModifyOrder(context.Background(), &order.Modify{
+		Pair:          currency.NewPair(currency.BTC, currency.AUD),
+		AssetType:     asset.Spot,
+		Price:         100000,
+		Amount:        0.001,
+		ID:            "8207123461",
+		ClientOrderID: "bruh3",
+	})
+	if !errors.Is(err, nil) {
+		t.Fatalf("received: '%v' but expected: '%v'", err, nil)
+	}
+
+	if mo == nil {
+		t.Fatal("expected data return")
+	}
+}
+
 func TestUpdateOrderExecutionLimits(t *testing.T) {
 	t.Parallel()
 	err := b.UpdateOrderExecutionLimits(context.Background(), asset.Empty)

diff --git a/exchanges/btcmarkets/btcmarkets_wrapper.go b/exchanges/btcmarkets/btcmarkets_wrapper.go
@@ -90,6 +90,7 @@ func (b *BTCMarkets) SetDefaults() {
 				TradeFee:            true,
 				FiatWithdrawalFee:   true,
 				CryptoWithdrawalFee: true,
+				ModifyOrder:         true,
 			},
 			WebsocketCapabilities: protocol.Features{
 				TickerFetching:         true,
@@ -579,8 +580,41 @@ func (b *BTCMarkets) SubmitOrder(ctx context.Context, s *order.Submit) (order.Su
 
 // ModifyOrder will allow of changing orderbook placement and limit to
 // market conversion
-func (b *BTCMarkets) ModifyOrder(ctx context.Context, action *order.Modify) (order.Modify, error) {
-	return order.Modify{}, common.ErrFunctionNotSupported
+func (b *BTCMarkets) ModifyOrder(ctx context.Context, action *order.Modify) (*order.Modify, error) {
+	if err := action.Validate(); err != nil {
+		return nil, err
+	}
+	resp, err := b.ReplaceOrder(ctx, action.ID, action.ClientOrderID, action.Price, action.Amount)
+	if err != nil {
+		return nil, err
+	}
+	pair, err := currency.NewPairFromString(resp.MarketID)
+	if err != nil {
+		return nil, err
+	}
+	side, err := order.StringToOrderSide(resp.Side)
+	if err != nil {
+		return nil, err
+	}
+	orderT, err := order.StringToOrderType(resp.Type)
+	if err != nil {
+		return nil, err
+	}
+	status, err := order.StringToOrderStatus(resp.Status)
+	if err != nil {
+		return nil, err
+	}
+	return &order.Modify{
+		ID:              resp.OrderID,
+		Pair:            pair,
+		Side:            side,
+		Type:            orderT,
+		Date:            resp.CreationTime,
+		Price:           resp.Price,
+		Amount:          resp.Amount,
+		RemainingAmount: resp.OpenAmount,
+		Status:          status,
+	}, nil
 }
 
 // CancelOrder cancels an order by its corresponding ID number