(Exchanges) Introduce validation method and small updates (#565)

* Remove pointer reference

* Fix portfolio withdraw tests

* Add nil protection in validator method to reduce prospective panics and for future outbound checking

* Updated tests

* ch order var to not ref package

* rm comparison

* Add order ID validation check

* Add exchange name validation check

* Add in test details

* fix tests

* fix linter issues

* linter issues strikes again

* linter rabbit hole

* Addr nitterinos

* Add validation variadic interface to define sets of functionality check POC

* didn't want to add an amount other than 0, didn't want to add address to exchange withdraw, didn't want to whitlist, can change if need be

* add coverage

* Add validation method options for exchange wrappers and abstracted validation into its own package

* Add validation code for structs in exchange template generation

* remove extra validation call as this is done in wrapper

* fix niterinos for examplerinos

* Add template to documentation tool and regenerated documentation

* Addr niticles

* Fix tests due to validation update

* Add more validation checks for modify/submit orders

* update tests

* fix more tests

* Add asset type to submit variable in tests and rpc call. Regen funcs.

* Add field to modify struct in tests

* applied field asset to cancel struct across project

* fix woopsy

engine/orders.go

@@ -72,19 +72,19 @@ func (o *orderStore) GetByInternalOrderID(internalOrderID string) (*order.Detail
 	return nil, ErrOrderNotFound
 }
 
-func (o *orderStore) exists(order *order.Detail) bool {
-	if order == nil {
+func (o *orderStore) exists(det *order.Detail) bool {
+	if det == nil {
 		return false
 	}
 	o.m.RLock()
 	defer o.m.RUnlock()
-	r, ok := o.Orders[order.Exchange]
+	r, ok := o.Orders[det.Exchange]
 	if !ok {
 		return false
 	}
 
 	for x := range r {
-		if r[x].ID == order.ID {
+		if r[x].ID == det.ID {
 			return true
 		}
 	}
@@ -92,33 +92,33 @@ func (o *orderStore) exists(order *order.Detail) bool {
 }
 
 // Add Adds an order to the orderStore for tracking the lifecycle
-func (o *orderStore) Add(order *order.Detail) error {
-	if order == nil {
+func (o *orderStore) Add(det *order.Detail) error {
+	if det == nil {
 		return errors.New("order store: Order is nil")
 	}
-	exch := Bot.GetExchangeByName(order.Exchange)
+	exch := Bot.GetExchangeByName(det.Exchange)
 	if exch == nil {
 		return ErrExchangeNotFound
 	}
-	if o.exists(order) {
+	if o.exists(det) {
 		return ErrOrdersAlreadyExists
 	}
 	// Untracked websocket orders will not have internalIDs yet
-	if order.InternalOrderID == "" {
+	if det.InternalOrderID == "" {
 		id, err := uuid.NewV4()
 		if err != nil {
 			log.Warnf(log.OrderMgr,
 				"Order manager: Unable to generate UUID. Err: %s",
 				err)
 		} else {
-			order.InternalOrderID = id.String()
+			det.InternalOrderID = id.String()
 		}
 	}
 	o.m.Lock()
 	defer o.m.Unlock()
-	orders := o.Orders[order.Exchange]
-	orders = append(orders, order)
-	o.Orders[order.Exchange] = orders
+	orders := o.Orders[det.Exchange]
+	orders = append(orders, det)
+	o.Orders[det.Exchange] = orders
 
 	return nil
 }
@@ -214,6 +214,7 @@ func (o *orderManager) CancelAllOrders(exchangeNames []string) {
 				Type:          v[y].Type,
 				Side:          v[y].Side,
 				Pair:          v[y].Pair,
+				AssetType:     v[y].AssetType,
 			})
 			if err != nil {
 				log.Error(log.OrderMgr, err)

engine/rpcserver.go

@@ -842,14 +842,20 @@ func (s *RPCServer) SubmitOrder(_ context.Context, r *gctrpc.SubmitOrderRequest)
 		return nil, err
 	}
 
+	a := asset.Item(r.AssetType)
+	if !asset.IsValid(a) {
+		return nil, fmt.Errorf("asset type: %s is invalid", a)
+	}
+
 	submission := &order.Submit{
-		Pair:     p,
-		Side:     order.Side(r.Side),
-		Type:     order.Type(r.OrderType),
-		Amount:   r.Amount,
-		Price:    r.Price,
-		ClientID: r.ClientId,
-		Exchange: r.Exchange,
+		Pair:      p,
+		Side:      order.Side(r.Side),
+		Type:      order.Type(r.OrderType),
+		Amount:    r.Amount,
+		Price:     r.Price,
+		ClientID:  r.ClientId,
+		Exchange:  r.Exchange,
+		AssetType: a,
 	}
 
 	resp, err := exch.SubmitOrder(submission)
@@ -958,12 +964,18 @@ func (s *RPCServer) CancelOrder(_ context.Context, r *gctrpc.CancelOrderRequest)
 		return nil, err
 	}
 
+	a := asset.Item(r.AssetType)
+	if !asset.IsValid(a) {
+		return nil, fmt.Errorf("asset type: %s is invalid", a)
+	}
+
 	err = exch.CancelOrder(&order.Cancel{
 		AccountID:     r.AccountId,
 		ID:            r.OrderId,
 		Side:          order.Side(r.Side),
 		WalletAddress: r.WalletAddress,
 		Pair:          p,
+		AssetType:     a,
 	})
 	if err != nil {
 		return nil, err
@@ -1045,18 +1057,19 @@ func (s *RPCServer) WithdrawCryptocurrencyFunds(_ context.Context, r *gctrpc.Wit
 	}
 
 	request := &withdraw.Request{
+		Exchange:    r.Exchange,
 		Amount:      r.Amount,
 		Currency:    currency.NewCode(strings.ToUpper(r.Currency)),
 		Type:        withdraw.Crypto,
 		Description: r.Description,
-		Crypto: &withdraw.CryptoRequest{
+		Crypto: withdraw.CryptoRequest{
 			Address:    r.Address,
 			AddressTag: r.AddressTag,
 			FeeAmount:  r.Fee,
 		},
 	}
 
-	resp, err := SubmitWithdrawal(r.Exchange, request)
+	resp, err := SubmitWithdrawal(request)
 	if err != nil {
 		return nil, err
 	}
@@ -1088,15 +1101,17 @@ func (s *RPCServer) WithdrawFiatFunds(_ context.Context, r *gctrpc.WithdrawFiatR
 	}
 
 	request := &withdraw.Request{
+		Exchange:    r.Exchange,
 		Amount:      r.Amount,
 		Currency:    currency.NewCode(strings.ToUpper(r.Currency)),
 		Type:        withdraw.Fiat,
 		Description: r.Description,
-		Fiat: &withdraw.FiatRequest{
-			Bank: bankAccount,
+		Fiat: withdraw.FiatRequest{
+			Bank: *bankAccount,
 		},
 	}
-	resp, err := SubmitWithdrawal(r.Exchange, request)
+
+	resp, err := SubmitWithdrawal(request)
 	if err != nil {
 		return nil, err
 	}
@@ -1153,7 +1168,7 @@ func (s *RPCServer) WithdrawalEventByID(_ context.Context, r *gctrpc.WithdrawalE
 			Fee:        v.RequestDetails.Crypto.FeeAmount,
 		}
 	} else if v.RequestDetails.Type == withdraw.Fiat {
-		if v.RequestDetails.Fiat != nil {
+		if v.RequestDetails.Fiat != (withdraw.FiatRequest{}) {
 			resp.Event.Request.Fiat = new(gctrpc.FiatWithdrawalEvent)
 			resp.Event.Request.Fiat = &gctrpc.FiatWithdrawalEvent{
 				BankName:      v.RequestDetails.Fiat.Bank.BankName,

engine/withdraw.go

@@ -1,7 +1,6 @@
 package engine
 
 import (
-	"errors"
 	"fmt"
 	"time"
 
@@ -21,41 +20,33 @@ const (
 	StatusError = "error"
 )
 
-// SubmitWithdrawal preforms validation and submits a new withdraw request to exchange
-func SubmitWithdrawal(exchName string, req *withdraw.Request) (*withdraw.Response, error) {
+// SubmitWithdrawal performs validation and submits a new withdraw request to
+// exchange
+func SubmitWithdrawal(req *withdraw.Request) (*withdraw.Response, error) {
 	if req == nil {
-		return nil, errors.New(ErrRequestCannotbeNil)
+		return nil, withdraw.ErrRequestCannotBeNil
 	}
 
-	var err error
-	var ret *withdraw.ExchangeResponse
-	if req.Exchange == "" {
-		req.Exchange = exchName
-	}
-
-	err = withdraw.Validate(req)
-	if err != nil {
-		return nil, err
-	}
-
-	exch := Bot.GetExchangeByName(exchName)
+	exch := Bot.GetExchangeByName(req.Exchange)
 	if exch == nil {
 		return nil, ErrExchangeNotFound
 	}
 
 	resp := &withdraw.Response{
-		Exchange: &withdraw.ExchangeResponse{
-			Name: exchName,
+		Exchange: withdraw.ExchangeResponse{
+			Name: req.Exchange,
 		},
-		RequestDetails: req,
+		RequestDetails: *req,
 	}
 
+	var err error
 	if Bot.Settings.EnableDryRun {
 		log.Warnln(log.Global, "Dry run enabled, no withdrawal request will be submitted or have an event created")
 		resp.ID = withdraw.DryRunID
 		resp.Exchange.Status = "dryrun"
 		resp.Exchange.ID = withdraw.DryRunID.String()
 	} else {
+		var ret *withdraw.ExchangeResponse
 		if req.Type == withdraw.Fiat {
 			ret, err = exch.WithdrawFiatFunds(req)
 			if err != nil {
@@ -151,7 +142,7 @@ func parseMultipleEvents(ret []*withdraw.Response) *gctrpc.WithdrawalEventsByExc
 				Fee:        ret[x].RequestDetails.Crypto.FeeAmount,
 			}
 		} else if ret[x].RequestDetails.Type == withdraw.Fiat {
-			if ret[x].RequestDetails.Fiat != nil {
+			if ret[x].RequestDetails.Fiat != (withdraw.FiatRequest{}) {
 				tempEvent.Request.Fiat = new(gctrpc.FiatWithdrawalEvent)
 				tempEvent.Request.Fiat = &gctrpc.FiatWithdrawalEvent{
 					BankName:      ret[x].RequestDetails.Fiat.Bank.BankName,
@@ -203,7 +194,7 @@ func parseSingleEvents(ret *withdraw.Response) *gctrpc.WithdrawalEventsByExchang
 			Fee:        ret.RequestDetails.Crypto.FeeAmount,
 		}
 	} else if ret.RequestDetails.Type == withdraw.Fiat {
-		if ret.RequestDetails.Fiat != nil {
+		if ret.RequestDetails.Fiat != (withdraw.FiatRequest{}) {
 			tempEvent.Request.Fiat = new(gctrpc.FiatWithdrawalEvent)
 			tempEvent.Request.Fiat = &gctrpc.FiatWithdrawalEvent{
 				BankName:      ret.RequestDetails.Fiat.Bank.BankName,

engine/withdraw_test.go

@@ -67,17 +67,17 @@ func TestSubmitWithdrawal(t *testing.T) {
 		Description: testExchange,
 		Amount:      1.0,
 		Type:        1,
-		Fiat: &withdraw.FiatRequest{
-			Bank: bank,
+		Fiat: withdraw.FiatRequest{
+			Bank: *bank,
 		},
 	}
 
-	_, err = SubmitWithdrawal(testExchange, req)
+	_, err = SubmitWithdrawal(req)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	_, err = SubmitWithdrawal(testExchange, nil)
+	_, err = SubmitWithdrawal(nil)
 	if err != nil {
 		if err.Error() != withdraw.ErrRequestCannotBeNil.Error() {
 			t.Fatal(err)
@@ -135,12 +135,12 @@ func TestParseEvents(t *testing.T) {
 		test := fmt.Sprintf("test-%v", x)
 		resp := &withdraw.Response{
 			ID: withdraw.DryRunID,
-			Exchange: &withdraw.ExchangeResponse{
+			Exchange: withdraw.ExchangeResponse{
 				Name:   test,
 				ID:     test,
 				Status: test,
 			},
-			RequestDetails: &withdraw.Request{
+			RequestDetails: withdraw.Request{
 				Exchange:    test,
 				Description: test,
 				Amount:      1.0,
@@ -149,12 +149,21 @@ func TestParseEvents(t *testing.T) {
 		if x%2 == 0 {
 			resp.RequestDetails.Currency = currency.AUD
 			resp.RequestDetails.Type = 1
-			resp.RequestDetails.Fiat = new(withdraw.FiatRequest)
-			resp.RequestDetails.Fiat.Bank = new(banking.Account)
+			resp.RequestDetails.Fiat = withdraw.FiatRequest{
+				Bank: banking.Account{
+					Enabled:             false,
+					ID:                  fmt.Sprintf("test-%v", x),
+					BankName:            fmt.Sprintf("test-%v-bank", x),
+					AccountName:         "hello",
+					AccountNumber:       fmt.Sprintf("test-%v", x),
+					BSBNumber:           "123456",
+					SupportedCurrencies: "BTC-AUD",
+					SupportedExchanges:  testExchange,
+				},
+			}
 		} else {
 			resp.RequestDetails.Currency = currency.BTC
 			resp.RequestDetails.Type = 0
-			resp.RequestDetails.Crypto = new(withdraw.CryptoRequest)
 			resp.RequestDetails.Crypto.Address = test
 			resp.RequestDetails.Crypto.FeeAmount = 0
 			resp.RequestDetails.Crypto.AddressTag = test