Orders can have fee and averagePrice == null

[amjad1210]

Currently when you query orders the fee is always null. It seems the only time you can get the fee from Binance is when you create a new order with the parameter newOrderRespType set to FULL and the order fills immediately. https://github.com/binance-exchange/binance-official-api-docs/blob/master/rest-api.md#new-order--trade

But if you query the order using the GET request the fills are missing so there's no way to get the fee. https://github.com/binance-exchange/binance-official-api-docs/blob/master/rest-api.md#query-order-user_data

The only workaround I can see is requesting data from https://github.com/binance-exchange/binance-official-api-docs/blob/master/rest-api.md#account-trade-list-user_data and filtering the orderId to match an order.

I really need the total fee taken from the exchange for the order.

@walec51 Any input on this?

Will create a PR for review.

[walec51]

This is a more general problem.

Orders from Binance don't provide fees because Binances API does not provide it on the orders level.

On other exchanges orders can even have the averagePrice == null because this information is not present on the orders level there.

For both fields we can choose one of the following solutions:

1. Add documentation to those fields that they can be null if the underlining exchange does not provide them on an order level. To remedy this situation we can also add a new method to TradeService called <T extends Order> T assureTradeResultsFilled(T order) which would make the additional effort fetch trades and calculate those fields if they are missing in the order. One would have to explicitly call it to indicated that he is prepared to make that effort.

2. Calculate those fields for every order if the underlining exchange does not provide them. This may be a very unoptimised solution for people that don't needed them on the order because they always download all their trades and base their decisions on that. This problem is escalated by the fact that exchanges limit call rates aggressively.

So this is a case of ease of use vs optimisation. I would like to hear from more people which direction is preferable.

[amjad1210]

@walec51 I like the proposal of adding a new method which does extra checks to calculate missing values only when requested. The current methods will stay the same with no extra overhead unless the user explicitly call it to indicate tradeResultsFilled == true. Which would do any extra checks for orders to populate missing data. This sounds like the best solution moving forward across all exchanges and still ensures ease of use.

[amjad1210]

@walec51 I've implemented, tested and pushed a commit to solution 1. Please review the changes.

With this approach users can continue to use the previous functionality with no drawbacks. I've added a new method called getOrderAssureResultsFilled(...) which concats the results of getOrder(...) and assureTradeResultsFilled(...).

[walec51]

@timmolter what do you think about adding these methods to TradeService:

• Collection<Order> assureTradeResultsFilled(Collection<Order> orders) throws IOException
• Collection<Order> getOrderWithTradeResults(OrderQueryParams... orderQueryParams)
• Collection<Order> getOrderWithTradeResults(String... orderIds) throws IOException

?

[timmolter]

It's a tough call. On the one hand we don't want to have to make extra multiple round trip calls to fill in missing fields that may not even be necessary for some users. Each API endpoint should really just be making one remote call ideally. However, there is a need to have those missing fields, which requires making those extra queries and pieces this together. I originally intended the client to do that piecing together with the individual api calls as needed rather than adding more and more generic API methods and complexity. So I'm not really sure at the moment. The method names that you're proposing are confusing for sure.

[walec51]

If only one additional call was required to add the missing data then I would be in favour of doing it under the current API. But getting those two missing fields may require not just a second call but many calls.

If the underlining exchange does not allow you to filter getTrades via orderId than one will have to call getTrades multiple times using windowing via fromTradeId o fromTimestamp to get to the trades of a given order.

Multiple windowing calls to getTrades my also be required it a given order was being executed for a long time and has more corresponding trades then getTrades can return in one go. The current code in the PR does not take this into account and requires more work.

[amjad1210]

Currently i'm focusing on CoinbasePro and Binance so i'm not 100% sure on what other exchanges are missing or require to populate missing data. For now I'll get the fee data for Binance on the client side until I'm sure this solution is needed on a wider level for multiple exchanges.

If you're happy with the above can you please close my PR but keep this issue open for a later date.

[jessethouin]

Was this ever resolved or revisited? I can't find anything related to newOrderRespType, commission, or commissionAsset in any XChange code.

edit: I found commissionAsset in ExecutionReportBinanceUserTransaction, but it isn't really representative of the FULL response from new orders.

{ "symbol": "BTCUSDT", "orderId": 28, "orderListId": -1, //Unless OCO, value will be -1 "clientOrderId": "6gCrw2kRUAF9CvJDGP16IP", "transactTime": 1507725176595, "price": "0.00000000", "origQty": "10.00000000", "executedQty": "10.00000000", "cummulativeQuoteQty": "10.00000000", "status": "FILLED", "timeInForce": "GTC", "type": "MARKET", "side": "SELL", "fills": [ { "price": "4000.00000000", "qty": "1.00000000", "commission": "4.00000000", "commissionAsset": "USDT" }, { "price": "3999.00000000", "qty": "5.00000000", "commission": "19.99500000", "commissionAsset": "USDT" }, { "price": "3998.00000000", "qty": "2.00000000", "commission": "7.99600000", "commissionAsset": "USDT" }, { "price": "3997.00000000", "qty": "1.00000000", "commission": "3.99700000", "commissionAsset": "USDT" }, { "price": "3995.00000000", "qty": "1.00000000", "commission": "3.99500000", "commissionAsset": "USDT" } ] }

Found at https://github.com/binance/binance-spot-api-docs/blob/master/rest-api.md#new-order--trade