Support batchOrder for futures

[Denton-L]

The Binance futures API allows the placement of multiple orders or the cancellation of multiple orders. We should support this in the library too.

One open question is that I'm not sure how you would shoehorn multiple orders into a fluent API design.

[straddler]

Hi,

I started using this lib a couple of weeks ago and I do miss this future already a bit.

I have looked into code a bit and I don't see easy way for such thing to be implemented. I guess the most basic would be to introduce new service structure that contains array of CreateOrderServices.

Or maybe on today's CreateOrderService introduce new method, idk chain that will return CreateOrderServiceChain. On which you can set new CreateOrderService and the list goes on and on. At the end calling Do on CreateOrderServicesChain will call the multiple orders api.

[OrmEmbaar]

A new method on CreateOrderService is probably not the way to go, in my opinion. If I understand what you're suggesting correctly, it would require a recursive data structure for CreateOrderService that would be somewhat confusing and counterintuitive. As the name implies, a CreateOrderService is for a single order and should stay that way.

I like your first suggestion of a new service structure with a method that either takes either an array of CreateOrderService or can be called multiple times to add new orders. I think that is in keeping with the overall design of the library and should be straightforward to build and test.

[Denton-L]

I definitely agree that we shouldn't be altering CreateOrderService. Perhaps a new CreateBatchOrderService?

Thinking about it some more, here's my proposal: we have CreateBatchOrderService.CreateOrder() which returns a CreateBatchOrderServiceOrder. From there, CreateBatchOrderServiceOrder has the same fluent design with the same interface as CreateOrderService (as in, you can call CreateBatchOrderServiceOrder.Price().SideType()...). Then finally, you call CreateBatchOrderService.Do()` to execute the batch order.

What do you guys think? If my proposal isn't clear, please let me know.

[straddler]

@Denton-L lets say I want to create 2 orders I would do something like:

client.NewCreateBatchOrderService().CreateOrder().Price(***).SideType(***).CreateOrder().Price(***).SideType(**).Do(***)

I am fine with that, sounds reasonable enough to me from the API standpoint. I dislike the naming a bit, this CreateBatchOrderServiceOrder to many orders in there.

Also I think we don't need that additional service at all, CreateBatchOrderService can just reuse inside CreateOrderService. To me it seems that this new SubService(orders orders) is going to have all the same functionalities as already in place CreateOrderService

[OrmEmbaar]

I read Denton's proposal a little differently. CreateOrder would return a CreateBatchOrderServiceOrder, not a CreateBatchOrderService, meaning that it would have no CreateOrder method. You would need to go back to the original object to create another order, like so:

bo := NewCreateBatchOrderService()        // Returns a CreateBatchOrderService
bo.CreateOrder().Price(***).SideType(***) // Returns a CreateBatchOrderServiceOrder
bo.CreateOrder().Price(***).SideType(***) // Must use the original CreateBatchOrderService to call CreateOrder again.
res, err := bo.Do()

Please correct me if I am wrong, Denton. I think something like that would be okay. For the sake of completeness, here is what I propose:

// Create orders as normal.
o1 := cli.NewCreateOrderService().Price(***).SideType(***)
o2 := cli.NewCreateOrderService().Price(***).SideType(***)

// Add those orders to a batch and call Do
res, err := cli.NewCreateBatchOrdersService().AddOrder(o1).AddOrder(o2).Do()

// or 
res, err := cli.NewCreateBatchOrdersService().AddOrders([]CreateOrderService{o1, o2}).Do()

I think this is the cleaner approach. It requires fewer changes to the codebase and has a smaller overall API surface, meaning a lower cognitive burden for the user. Having said that, I am not super attached to it and would be fine with either.

[Denton-L]

Yep, you read it correctly. Between your proposal and mine, I like yours much better. Given the way my code interacts with this library, it is much easier for me to integrate your design than it would be for mine.

[straddler]

@afenrir great proposal, I really like it!

[boskiv]

Hey, folks. What is a status of NewCreateBatchOrdersService? I'm trying to use it but it does not fill my orders

var orders []*futures.CreateOrderService
	log.Println("[exchange][CreateOrder] Creating base order")
	baseOrder := s.client.NewCreateOrderService().
		Symbol(s.pair.BaseName).
		Side(futures.SideType(s.pair.BaseSide)).
		Quantity(s.pair.BaseAmount).
		Type(futures.OrderTypeMarket)
	log.Println("[exchange][CreateOrder] Creating quote order")
	quoteOrder := s.client.NewCreateOrderService().
		Symbol(s.pair.QuoteName).
		Side(futures.SideType(s.pair.QuoteSide)).
		Quantity(s.pair.QuoteAmount).
		Type(futures.OrderTypeMarket)

	orders = append(orders, baseOrder, quoteOrder)
	log.Println("[exchange][CreateOrder] Executing multiple orders")
	res, err := s.client.NewCreateBatchOrdersService().OrderList(orders).Do(context.Background())

	if err != nil {
		log.Println("[exchange][CreateOrder] Error from multiple orders")
		return nil, err
	}

It returns me just a nil in res

P.S. If I change it to single order execution adding Do() after orders, then execution works, but with two requests. P.P.S. client.Debug = true, shows me the truth :)

Binance-golang 2021/11/16 23:28:22 response body: [ {
  "code" : -4003,
  "msg" : "Quantity less than zero."
}, {
  "code" : -4003,
  "msg" : "Quantity less than zero."
} ]
Binance-golang 2021/11/16 23:28:22 response status code: 200
2021/11/16 23:28:22 &{[]}

That was expected error from binance i want to see

But, how i can get this errors from a response, because I need it to report

[boskiv]

Errors are located in `rawMessages array, but it can't be unmarshaled to Order structure and no error about it. :(

[boskiv]

Maybe something like this should help

func StrictUnmarshal(data []byte, v interface{}) error {
	dec := json.NewDecoder(bytes.NewReader(data))
	dec.DisallowUnknownFields()
	return dec.Decode(v)
}

[boskiv]

@adshao

[adshao]

@adshao

sorry, I just found the @ . Yes, since Binance returned status code 200 for the failed request, so it did not try to convert response body to error. We should retry to convert to error if it failed to convert to Order.