BugFix: RPCServer cannot retrieve open orders/getOrder due to unset asset type (#634)

* Fixes issue where getorders could not work due to unset asset type in rpcserver.go. Adds test. Also adds start and end date to the cli.

* A few fixes

* lint

* fixes oopsie that affected doopsie

* Ensures dates are set for all open order implementations. Adds new filter to ensure orders without dates are returned rather than filtered. Fixes up Binance OpenOrders implementation. Adds some extra typeconverts for binance

* Add updated time to Binance GetActiveOrders. Update rpcserver.go to only set the time if its not empty. Also addressed bad expected value

* Actually fixes things this time

* Improves recvWindow to process openOrders

* Adds asset type to getOrder as well

* Fixes tests

* Adds missing date fields

* Fixes default time, updates default errors

* Default start to last month, instead of last year

engine/orders.go

@@ -293,7 +293,7 @@ func (o *orderManager) Cancel(cancel *order.Cancel) error {
 // and stores the result in the order manager
 func (o *orderManager) GetOrderInfo(exchangeName, orderID string, cp currency.Pair, a asset.Item) (order.Detail, error) {
 	if orderID == "" {
-		return order.Detail{}, errors.New("order cannot be empty")
+		return order.Detail{}, errOrderCannotBeEmpty
 	}
 
 	exch := Bot.GetExchangeByName(exchangeName)

engine/orders_types.go

@@ -1,12 +1,17 @@
 package engine
 
 import (
+	"errors"
 	"sync"
 
 	"github.com/thrasher-corp/gocryptotrader/currency"
 	"github.com/thrasher-corp/gocryptotrader/exchanges/order"
 )
 
+var (
+	errOrderCannotBeEmpty = errors.New("order cannot be empty")
+)
+
 type orderManagerConfig struct {
 	EnforceLimitConfig     bool
 	AllowMarketOrders      bool

engine/rpcserver.go

@@ -50,18 +50,19 @@ import (
 )
 
 const (
-	errExchangeNameUnset  = "exchange name unset"
-	errCurrencyPairUnset  = "currency pair unset"
-	errStartEndTimesUnset = "invalid start and end times"
-	errAssetTypeUnset     = "asset type unset"
-	errDispatchSystem     = "dispatch system offline"
-	invalidArguments      = "invalid arguments received"
+	errExchangeNameUnset = "exchange name unset"
+	errAssetTypeUnset    = "asset type unset"
+	errDispatchSystem    = "dispatch system offline"
+	invalidArguments     = "invalid arguments received"
 )
 
 var (
 	errExchangeNotLoaded    = errors.New("exchange is not loaded/doesn't exist")
 	errExchangeBaseNotFound = errors.New("cannot get exchange base")
 	errInvalidArguments     = errors.New(invalidArguments)
+	errCurrencyPairUnset    = errors.New("currency pair unset")
+	errCurrencyNotEnabled   = errors.New("currency not enabled")
+	errInvalidStartEndTime  = errors.New("invalid start and end times")
 )
 
 // RPCServer struct
@@ -801,38 +802,115 @@ func (s *RPCServer) GetForexRates(_ context.Context, r *gctrpc.GetForexRatesRequ
 }
 
 // GetOrders returns all open orders, filtered by exchange, currency pair or
-// asset type
+// asset type between optional dates
 func (s *RPCServer) GetOrders(_ context.Context, r *gctrpc.GetOrdersRequest) (*gctrpc.GetOrdersResponse, error) {
+	if r == nil {
+		return nil, errInvalidArguments
+	}
 	exch := s.GetExchangeByName(r.Exchange)
 	if exch == nil {
 		return nil, errExchangeNotLoaded
 	}
+	if r.Pair == nil {
+		return nil, errCurrencyPairUnset
+	}
+	a, err := asset.New(r.AssetType)
+	if err != nil {
+		return nil, err
+	}
+	var start, end time.Time
+	if r.StartDate != "" {
+		start, err = time.Parse(common.SimpleTimeFormat, r.StartDate)
+		if err != nil {
+			return nil, err
+		}
+	}
+	if r.EndDate != "" {
+		end, err = time.Parse(common.SimpleTimeFormat, r.EndDate)
+		if err != nil {
+			return nil, err
+		}
+	}
+	if !start.IsZero() && !end.IsZero() && start.After(end) {
+		return nil, errInvalidStartEndTime
+	}
 
-	resp, err := exch.GetActiveOrders(&order.GetOrdersRequest{
-		Pairs: []currency.Pair{
-			currency.NewPairWithDelimiter(r.Pair.Base,
-				r.Pair.Quote, r.Pair.Delimiter),
-		},
-	})
+	cp := currency.NewPairWithDelimiter(
+		r.Pair.Base,
+		r.Pair.Quote,
+		r.Pair.Delimiter)
+	var pairs currency.Pairs
+	pairs, err = exch.GetEnabledPairs(a)
+	if err != nil {
+		return nil, err
+	}
+	if !pairs.Contains(cp, false) {
+		return nil, fmt.Errorf("%s %w %v, please check your config",
+			exch.GetName(),
+			errCurrencyNotEnabled,
+			cp.String())
+	}
+
+	request := &order.GetOrdersRequest{
+		Pairs:     []currency.Pair{cp},
+		AssetType: a,
+	}
+	if !start.IsZero() {
+		request.StartTime = start
+	}
+	if !end.IsZero() {
+		request.EndTime = end
+	}
+
+	var resp []order.Detail
+	resp, err = exch.GetActiveOrders(request)
 	if err != nil {
 		return nil, err
 	}
 
 	var orders []*gctrpc.OrderDetails
 	for x := range resp {
-		orders = append(orders, &gctrpc.OrderDetails{
+		var trades []*gctrpc.TradeHistory
+		for i := range resp[x].Trades {
+			t := &gctrpc.TradeHistory{
+				Id:        resp[x].Trades[i].TID,
+				Price:     resp[x].Trades[i].Price,
+				Amount:    resp[x].Trades[i].Amount,
+				Exchange:  r.Exchange,
+				AssetType: a.String(),
+				OrderSide: resp[x].Trades[i].Side.String(),
+				Fee:       resp[x].Trades[i].Fee,
+				Total:     resp[x].Trades[i].Total,
+			}
+			if !resp[x].Trades[i].Timestamp.IsZero() {
+				t.CreationTime = resp[x].Trades[i].Timestamp.Unix()
+			}
+			trades = append(trades, t)
+		}
+		o := &gctrpc.OrderDetails{
 			Exchange:      r.Exchange,
 			Id:            resp[x].ID,
+			ClientOrderId: resp[x].ClientOrderID,
 			BaseCurrency:  resp[x].Pair.Base.String(),
 			QuoteCurrency: resp[x].Pair.Quote.String(),
-			AssetType:     asset.Spot.String(),
-			OrderType:     resp[x].Type.String(),
+			AssetType:     resp[x].AssetType.String(),
 			OrderSide:     resp[x].Side.String(),
-			CreationTime:  resp[x].Date.Unix(),
+			OrderType:     resp[x].Type.String(),
 			Status:        resp[x].Status.String(),
 			Price:         resp[x].Price,
 			Amount:        resp[x].Amount,
-		})
+			OpenVolume:    resp[x].Amount - resp[x].ExecutedAmount,
+			Fee:           resp[x].Fee,
+			Cost:          resp[x].Cost,
+			Trades:        trades,
+		}
+		if !resp[x].Date.IsZero() {
+			o.CreationTime = resp[x].Date.Unix()
+		}
+		if !resp[x].LastUpdated.IsZero() {
+			o.UpdateTime = resp[x].LastUpdated.Unix()
+		}
+		orders = append(orders, o)
 	}
 
 	return &gctrpc.GetOrdersResponse{Orders: orders}, nil
@@ -840,10 +918,16 @@ func (s *RPCServer) GetOrders(_ context.Context, r *gctrpc.GetOrdersRequest) (*g
 
 // GetOrder returns order information based on exchange and order ID
 func (s *RPCServer) GetOrder(_ context.Context, r *gctrpc.GetOrderRequest) (*gctrpc.OrderDetails, error) {
+	if r == nil {
+		return nil, errInvalidArguments
+	}
 	exch := s.GetExchangeByName(r.Exchange)
 	if exch == nil {
 		return nil, errExchangeNotLoaded
 	}
+	if r.Pair == nil {
+		return nil, errCurrencyPairUnset
+	}
 
 	pair := currency.Pair{
 		Delimiter: r.Pair.Delimiter,
@@ -851,9 +935,14 @@ func (s *RPCServer) GetOrder(_ context.Context, r *gctrpc.GetOrderRequest) (*gct
 		Quote:     currency.NewCode(r.Pair.Quote),
 	}
 
-	result, err := s.OrderManager.GetOrderInfo(r.Exchange, r.OrderId, pair, "") // assetType will be implemented in the future
+	a, err := asset.New(r.Asset)
 	if err != nil {
-		return nil, fmt.Errorf("error whilst trying to retrieve info for order %s: %s", r.OrderId, err)
+		return nil, err
+	}
+
+	result, err := s.OrderManager.GetOrderInfo(r.Exchange, r.OrderId, pair, a)
+	if err != nil {
+		return nil, fmt.Errorf("error whilst trying to retrieve info for order %s: %w", r.OrderId, err)
 	}
 	var trades []*gctrpc.TradeHistory
 	for i := range result.Trades {
@@ -874,8 +963,8 @@ func (s *RPCServer) GetOrder(_ context.Context, r *gctrpc.GetOrderRequest) (*gct
 	if result.Date.Unix() > 0 {
 		creationTime = result.Date.Unix()
 	}
-	if result.CloseTime.Unix() > 0 {
-		updateTime = result.CloseTime.Unix()
+	if result.LastUpdated.Unix() > 0 {
+		updateTime = result.LastUpdated.Unix()
 	}
 
 	return &gctrpc.OrderDetails{
@@ -1542,7 +1631,7 @@ func (s *RPCServer) GetOrderbookStream(r *gctrpc.GetOrderbookStreamRequest, stre
 	}
 
 	if r.Pair.String() == "" {
-		return errors.New(errCurrencyPairUnset)
+		return errCurrencyPairUnset
 	}
 
 	if r.AssetType == "" {
@@ -1656,7 +1745,7 @@ func (s *RPCServer) GetTickerStream(r *gctrpc.GetTickerStreamRequest, stream gct
 	}
 
 	if r.Pair.String() == "" {
-		return errors.New(errCurrencyPairUnset)
+		return errCurrencyPairUnset
 	}
 
 	if r.AssetType == "" {
@@ -1795,13 +1884,13 @@ func (s *RPCServer) GetAuditEvent(_ context.Context, r *gctrpc.GetAuditEventRequ
 // GetHistoricCandles returns historical candles for a given exchange
 func (s *RPCServer) GetHistoricCandles(_ context.Context, r *gctrpc.GetHistoricCandlesRequest) (*gctrpc.GetHistoricCandlesResponse, error) {
 	if r.Exchange == "" {
-		return nil, errors.New(errExchangeNameUnset)
+		return nil, fmt.Errorf("%w. blank exchange name received", errInvalidArguments)
 	}
 	if r.Pair.String() == "" {
-		return nil, errors.New(errCurrencyPairUnset)
+		return nil, errCurrencyPairUnset
 	}
 	if r.Start == r.End {
-		return nil, errors.New(errStartEndTimesUnset)
+		return nil, errInvalidStartEndTime
 	}
 
 	var klineItem kline.Item

engine/rpcserver_test.go

@@ -7,6 +7,7 @@ import (
 	"os"
 	"path/filepath"
 	"runtime"
+	"strings"
 	"testing"
 	"time"
 
@@ -17,7 +18,7 @@ import (
 	"github.com/thrasher-corp/gocryptotrader/database"
 	"github.com/thrasher-corp/gocryptotrader/database/drivers"
 	"github.com/thrasher-corp/gocryptotrader/database/repository"
-	"github.com/thrasher-corp/gocryptotrader/database/repository/exchange"
+	dbexchange "github.com/thrasher-corp/gocryptotrader/database/repository/exchange"
 	sqltrade "github.com/thrasher-corp/gocryptotrader/database/repository/trade"
 	"github.com/thrasher-corp/gocryptotrader/exchanges/asset"
 	"github.com/thrasher-corp/gocryptotrader/exchanges/kline"
@@ -69,7 +70,7 @@ func RPCTestSetup(t *testing.T) *Engine {
 		t.Fatalf("failed to run migrations %v", err)
 	}
 	uuider, _ := uuid.NewV4()
-	err = exchange.Insert(exchange.Details{Name: testExchange, UUID: uuider})
+	err = dbexchange.Insert(dbexchange.Details{Name: testExchange, UUID: uuider})
 	if err != nil {
 		t.Fatalf("failed to insert exchange %v", err)
 	}
@@ -330,16 +331,16 @@ func TestGetHistoricCandles(t *testing.T) {
 	_, err := s.GetHistoricCandles(context.Background(), &gctrpc.GetHistoricCandlesRequest{
 		Exchange: "",
 	})
-	if err != nil && err.Error() != errExchangeNameUnset {
-		t.Error(err)
+	if !errors.Is(err, errInvalidArguments) {
+		t.Errorf("expected %v, received %v", errInvalidArguments, err)
 	}
 
 	_, err = s.GetHistoricCandles(context.Background(), &gctrpc.GetHistoricCandlesRequest{
 		Exchange: testExchange,
 		Pair:     &gctrpc.CurrencyPair{},
 	})
-	if err != nil && err.Error() != errCurrencyPairUnset {
-		t.Error(err)
+	if !errors.Is(err, errCurrencyPairUnset) {
+		t.Errorf("expected %v, received %v", errCurrencyPairUnset, err)
 	}
 	_, err = s.GetHistoricCandles(context.Background(), &gctrpc.GetHistoricCandlesRequest{
 		Exchange: testExchange,
@@ -348,8 +349,8 @@ func TestGetHistoricCandles(t *testing.T) {
 			Quote: currency.USD.String(),
 		},
 	})
-	if err != nil && err.Error() != errStartEndTimesUnset {
-		t.Error(err)
+	if !errors.Is(err, errInvalidStartEndTime) {
+		t.Errorf("expected %v, received %v", errInvalidStartEndTime, err)
 	}
 	var results *gctrpc.GetHistoricCandlesResponse
 	defaultStart := time.Date(2020, 1, 1, 1, 1, 1, 1, time.UTC)
@@ -818,3 +819,167 @@ func TestUpdateAccountInfo(t *testing.T) {
 		t.Fatalf("TestGetAccountInfo: Unexpected value of the 'TotalValue'")
 	}
 }
+
+func TestGetOrders(t *testing.T) {
+	exchName := "binance"
+	engerino := RPCTestSetup(t)
+	defer CleanRPCTest(t, engerino)
+	s := RPCServer{Engine: engerino}
+
+	_, err := s.GetOrders(context.Background(), nil)
+	if !errors.Is(err, errInvalidArguments) {
+		t.Errorf("expected %v, received %v", errInvalidArguments, err)
+	}
+
+	_, err = s.GetOrders(context.Background(), &gctrpc.GetOrdersRequest{})
+	if !errors.Is(err, errExchangeNotLoaded) {
+		t.Errorf("expected %v, received %v", errExchangeNotLoaded, err)
+	}
+
+	err = engerino.LoadExchange(exchName, false, nil)
+	if err != nil {
+		t.Error(err)
+	}
+
+	_, err = s.GetOrders(context.Background(), &gctrpc.GetOrdersRequest{
+		Exchange: exchName,
+	})
+	if !errors.Is(err, errCurrencyPairUnset) {
+		t.Errorf("expected %v, received %v", errCurrencyPairUnset, err)
+	}
+
+	p := &gctrpc.CurrencyPair{
+		Delimiter: "-",
+		Base:      currency.BTC.String(),
+		Quote:     currency.USDT.String(),
+	}
+	_, err = s.GetOrders(context.Background(), &gctrpc.GetOrdersRequest{
+		Exchange: exchName,
+		Pair:     p,
+	})
+	if !errors.Is(err, asset.ErrNotSupported) {
+		t.Errorf("expected %v, received %v", asset.ErrNotSupported, err)
+	}
+
+	_, err = s.GetOrders(context.Background(), &gctrpc.GetOrdersRequest{
+		Exchange:  exchName,
+		AssetType: asset.Spot.String(),
+		Pair:      p,
+		StartDate: time.Now().Format(common.SimpleTimeFormat),
+		EndDate:   time.Now().Add(-time.Hour).Format(common.SimpleTimeFormat),
+	})
+	if !errors.Is(err, errInvalidStartEndTime) {
+		t.Errorf("expected %v, received %v", errInvalidStartEndTime, err)
+	}
+
+	_, err = s.GetOrders(context.Background(), &gctrpc.GetOrdersRequest{
+		Exchange:  exchName,
+		AssetType: asset.Spot.String(),
+		Pair:      p,
+		StartDate: time.Now().Format(common.SimpleTimeFormat),
+		EndDate:   time.Now().Add(time.Hour).Format(common.SimpleTimeFormat),
+	})
+	if err != nil && !strings.Contains(err.Error(), "not supported due to unset/default API keys") {
+		t.Error(err)
+	}
+	if err == nil {
+		t.Error("expected error")
+	}
+
+	exch := engerino.GetExchangeByName(exchName)
+	if exch == nil {
+		t.Fatal("expected an exchange")
+	}
+	b := exch.GetBase()
+	b.API.Credentials.Key = "test"
+	b.API.Credentials.Secret = "test"
+	b.API.AuthenticatedSupport = true
+
+	_, err = s.GetOrders(context.Background(), &gctrpc.GetOrdersRequest{
+		Exchange:  exchName,
+		AssetType: asset.Spot.String(),
+		Pair:      p,
+	})
+	if err == nil {
+		t.Error("expected error")
+	}
+}
+
+func TestGetOrder(t *testing.T) {
+	exchName := "binance"
+	engerino := RPCTestSetup(t)
+	defer CleanRPCTest(t, engerino)
+	s := RPCServer{Engine: engerino}
+
+	_, err := s.GetOrder(context.Background(), nil)
+	if !errors.Is(err, errInvalidArguments) {
+		t.Errorf("expected %v, received %v", errInvalidArguments, err)
+	}
+
+	_, err = s.GetOrder(context.Background(), &gctrpc.GetOrderRequest{
+		Exchange: exchName,
+		OrderId:  "",
+		Pair:     nil,
+		Asset:    "",
+	})
+	if !errors.Is(err, errExchangeNotLoaded) {
+		t.Errorf("expected %v, received %v", errExchangeNotLoaded, err)
+	}
+
+	err = engerino.LoadExchange(exchName, false, nil)
+	if err != nil {
+		t.Error(err)
+	}
+
+	_, err = s.GetOrder(context.Background(), &gctrpc.GetOrderRequest{
+		Exchange: exchName,
+		OrderId:  "",
+		Pair:     nil,
+		Asset:    "",
+	})
+	if !errors.Is(err, errCurrencyPairUnset) {
+		t.Errorf("expected %v, received %v", errCurrencyPairUnset, err)
+	}
+
+	p := &gctrpc.CurrencyPair{
+		Delimiter: "-",
+		Base:      "BTC",
+		Quote:     "USDT",
+	}
+
+	_, err = s.GetOrder(context.Background(), &gctrpc.GetOrderRequest{
+		Exchange: exchName,
+		OrderId:  "",
+		Pair:     p,
+		Asset:    "",
+	})
+	if !errors.Is(err, asset.ErrNotSupported) {
+		t.Errorf("expected %v, received %v", asset.ErrNotSupported, err)
+	}
+
+	_, err = s.GetOrder(context.Background(), &gctrpc.GetOrderRequest{
+		Exchange: exchName,
+		OrderId:  "",
+		Pair:     p,
+		Asset:    asset.Spot.String(),
+	})
+	if !errors.Is(err, errOrderCannotBeEmpty) {
+		t.Errorf("expected %v, received %v", errOrderCannotBeEmpty, err)
+	}
+	if Bot == nil {
+		Bot = engerino
+	}
+	err = Bot.OrderManager.Start()
+	if err != nil {
+		t.Fatal(err)
+	}
+	_, err = s.GetOrder(context.Background(), &gctrpc.GetOrderRequest{
+		Exchange: exchName,
+		OrderId:  "1234",
+		Pair:     p,
+		Asset:    asset.Spot.String(),
+	})
+	if err == nil {
+		t.Error("expected error")
+	}
+}