JKorf / Binance.Net

A C# .netstandard client library for the Binance REST and Websocket Spot and Futures API focusing on clear usage and models
https://jkorf.github.io/Binance.Net/
MIT License
1.04k stars 427 forks source link

BinanceSocketClient.UsdFuturesApi.SubscribeToTradeUpdatesAsync() Stream has sporadic price Updates which are NOT real #1349

Closed vburwitz3 closed 6 months ago

vburwitz3 commented 7 months ago

Describe the bug I subscribe as example via: BinanceSocketClient.UsdFuturesApi.SubscribeToTradeUpdatesAsync("BTCUSDT") To the BTCUSDT trade stream of the perpetual USDT futures of BTCUSDT (also a problem on ETHUSDT etc). While the price is as example ticking around 57000 I sporadically would get a price far off at 58500 or 55800 or so. Or even a price update at 0. While I can filter out a price of 0 it is not really possible to filter a price 1K above or below current price out. If I then check the candles via klines or check on the Binance website I see at the candle that these prices where indeed outside of the candle for that time period! Also in the price update object CryptoExchange.Net.Sockets.DataEvent the p.Event is "trade" and the p.Symbol is "BTCUSDT". Therefore I dont have a change to filter these wrong ticks out. I dont know what they are, some Mark Price or whatever but they are not a traded price of the USDT Perpetual Future that I subscribed.

To Reproduce Subscribe to the price stream and record the data. If you do something like (for BTCUSDT as example): if (Math.Abs(lastPrice-p.Price)>100) print ... You will see these these ticks, after an hour you should surely have one. Check that it was not a valid price change since it should be outside of the candle from that time period.

Expected behavior These wrong prices should not show up. At least there should be some status field like p.Event that should make it possible to identify these price updates, whatever they are!

Debug logging 27.02. 16:32:06 BTCUSDT p:55853,40 event:trade old:57179,50 PRICEJUMP 27.02. 16:32:06 BTCUSDT p:57179,50 event:trade old:55853,40 PRICEJUMP (Thats European Time in this case) The price 55853,40 was clearly an outlier and also does not show up in the kline data for that time or in the candle at Binance of that time.

JKorf commented 7 months ago

I can confirm I can reproduce this, but I'm not sure what to do about it. Seems like a server issue. You could try asking it on https://dev.binance.vision, but be aware that this stream (individual trade stream for futures) is not officially documented, only the aggregated trades one is. So not sure if support can/will provide you with an answer.

vburwitz3 commented 7 months ago

Thank you for the fast answer. I got some points on that:

  1. I cant see the original JSON messages (or at least I dont know how to get their content over the API). Interesting would be if there is any special flag provided in the corresponding wrong price updates that would make it possible to filter these out. I guess there is a good propability that this is the case, since these "wrong prices" may be some special updated for some purpose.

  2. I need actually all price updates since I build my own "candles" (something similar at least) on the fly and need always the correct realtime high and low. With Aggregated Trades stream I am not sure that I reliably will get all price changes (I dont need every tick but every price update at least for new highs and lows in a time period).

  3. In Spot I dont have any problems like that in the corresponding stream despite what the status might be and a simple price update stream is actually really something essential if not the most essential feature, therefore I wonder that there is such an issue. I would happily make a call to Binance, sadly I am not "known" there which surely doesnt help but before any call I think it would be great to know point 1, meaning is there not maybe anything in the Binance data that shows a special status of some for for these price updates. Would make sense since they are not completely random but seem to have a certain offset to the up and downside to the current price.

JKorf commented 7 months ago
  1. You can enable the 'OutputOriginalData' option in the client, this will fill the OriginalData property on the DataEvent update message, so you can use that to see if there is any way to differentiate between the different messages.

  2. Aggregated trades will only aggregate data if it's the same price, so I think it would still be okay for creating klines. But I can understand you'd prefer to use individual trades.

vburwitz3 commented 7 months ago

I found something interesting:

27.02. 21:23:17 BTCUSDT p:59040,99 e:trade op:57074,50 s:0,002 origdata:"{"stream":"btcusdt@trade","data":{"e":"trade","E":1709065396929,"T":1709065396929,"s":"BTCUSDT","t":4640305685,"p":"59040.99","q":"0.002","X":"INSURANCE_FUND","m":true}}" PRICEJUMP 27.02. 21:23:17 BTCUSDT p:57074,40 e:trade op:59040,99 s:0,017 origdata:"{"stream":"btcusdt@trade","data":{"e":"trade","E":1709065396930,"T":1709065396930,"s":"BTCUSDT","t":4640305686,"p":"57074.40","q":"0.017","X":"MARKET","m":true}}" PRICEJUMP

The outlier had "X":"INSURANCE_FUND" instead of "X":"MARKET" or something that maybe also would seem valid. Filtering by X should do the trick but how could I do that without parsing originaldata?

JKorf commented 7 months ago

I can add the property in next version, but thats probably going to be in a few days. Have you seen any other values? Either way I'll probably just add it as a string value.

vburwitz3 commented 7 months ago

I had my recording now running for 12h and all the pricejumps I filtered where "X":"INSURANCE_FUND". Then I also ran a couple of minutes of price ticks and scrolled through the output and all price updates seem always to be "X":"MARKET" no Limit or any other information was in the data.

I had a thought on that: Maybe a default on BinanceSocketClient.UsdFuturesApi.SubscribeToTradeUpdatesAsync() should be that all Ticks other than "MARKET" should maybe filtered out unless on the constructor an all updates flag is set to true. Maybe make a update which enforces that any given implementation must provide a setting here. Even if I am absolutely not a fan of such things and think APIs should always basically stay the same, here there is a case which could cause severe damage and if anyone wants these "INSURANCE_FUND" updates (and whatever else may exist) they should do a concious decision about that. But it should be clearly doubted that anyone wants since in the current implementation there is really no easy way to actually even detect these special updates (which would require parsing in OriginalData which must even be turned on in the first place) and if people really would want that a feature would have been requested long time ago.

vburwitz3 commented 7 months ago

Also very good would be a extra sentence in the description of BinanceSocketClient.UsdFuturesApi.SubscribeToTradeUpdatesAsync() like "Be aware that price updates will be included that are not from actually traded prices if the setting allUpdates is set to true.". So anyone using the method will see in the auto completion text in Visual Studio this info while in the process of maybe using this function.

JKorf commented 6 months ago

I've added the parameter as suggested and included some comments