binance-exchange / node-binance-api

Node Binance API is an asynchronous node.js library for the Binance API designed to be easy to use.
MIT License
528 stars 146 forks source link

Getting candlesticks for all Tickers on all intervals is unreliable #166

Closed bramvbilsen closed 5 years ago

bramvbilsen commented 5 years ago

I am currently trying to get all candlestick data via the Stream API. But I am facing a big issue:

The data does not always arrive in time.

So I am trying to get the candlestick data for every symbol on Binance (about 400) for about 5 interval (1m, 5m, 15m, 30m, 1h). I sometimes have to wait over 5 minutes for all data to arrive and of course, by that time, the data is outdated already.
I looked at your implementation and all looks good there, so perhaps it is something with Binance servers. Or perhaps I am not using the correct websocket endpoint. I am really not sure about what the issue is, so perhaps any one could clarify whether it is just me taxing the Binance servers too much or simply using the wrong endpoint.

jaggedsoft commented 5 years ago

hmm, only thing that would make sense to me is processing priority. I have not encountered this problem but I am running on very modern server hardware. I think the websocket code is almost as efficient as it can be, so these would be my suggestions:

  1. Try a faster processor if possible
  2. Try connecting to each websocket endpoint individually (do not pass an array) this will test individual connections instead of a combinedStream
  3. Worst case scenario split your program into several smaller programs, for example one that handles all BTC pairs, one that handles all ETH pairs, etc. I do not know how to perform proper multithreading on node
jaggedsoft commented 5 years ago

I would suggest maybe some other calculations are possibly slowing it down. Are we doing other processing such as technical indicators? If so, this processing can probably be debounced somehow

Are we using the chart function? If so, the bottleneck on this function would be the speed of your memory.

The optimal way to design this type of program is probably to have a daemon that is connected to all the websockets and updates this data immediately into a database. Then you run another program that makes decisions based on the contents of the database. I am sure there is a better way but I can't think of one just yet

jaggedsoft commented 5 years ago

Another suggestion for a trading bot is to do some kind of screener when your program first starts. This is the approach I take: https://github.com/jaggedsoft/node-binance-api/blob/master/examples/subscribe-to-all.js

  1. Download prevDay ticker at program start
  2. Filter out only markets we are interested in. For example, has more than 90 BTC daily volume, or some bullish price action, or only BTC markets, etc. This cuts down your subscribed connections by half or more
  3. ?????
  4. PROFIT!
bramvbilsen commented 5 years ago

Wow, thanks for the extremely fast response! I had already started writing my own websocket implementation of the Binance API and I encountered the exact same thing. So it is certainly not the library.

I am also fairly certain it is nothing computational as I simply display the prices and price changes on the screen without manipulating the data. I thought that perhaps Binance slowed me down because I went over their quotas, but I can't seem to find any on their website or docs so I am guessing that is not the problem either.

But your multithreading suggestion seems like a very reasonable thing to try out!
Again, thanks for the fast response!

jaggedsoft commented 5 years ago

Good luck friend I do believe it is computational, the symptoms are as if it's not able to process all of the data in time so it just snowballs and gets worse (on linux you can type uptime to see the load average, or use top to see if it's using too much CPU)

If you are doing this on a VPS, I would suggest trying it on your own computer instead And vice versa, if you're doing this on your computer, try doing it from a VPS. my digitalocean has a 1000mbit connection and works very well for me

Ciao

jaggedsoft commented 5 years ago

The most ideal option I can think of would consist of you generating the OHLC data yourself This data is generated using only the stream of the latest trades and keeping track of the price change over time (open, high, low, close) i.e, if a minute has passed on a 1m chart, new candle.

It is much more efficient to monitor the trades stream of all symbols, as shown in https://github.com/jaggedsoft/node-binance-api/blob/master/examples/subscribe-to-all.js

jaggedsoft commented 5 years ago

Simplest way I can think of to subscribe to all trade streams (just did this as an experiment)

(async () => {
  const axios = require('axios');
  const WebSocket = require('ws');
  let streams = [];
  let markets = [];
  let prevDay = await axios.get('https://api.binance.com/api/v1/ticker/24hr');
  for ( let obj of prevDay.data ) {
    markets.push(obj.symbol);
    streams.push(obj.symbol.toLowerCase()+'@aggTrade');
  }
  console.log(markets);

  const ws = new WebSocket('wss://stream.binance.com:9443/stream?streams='+streams.join('/'));
  ws.on('message', function(payload) {
    let json = JSON.parse(payload), stream = json.stream, data = json.data;
    if ( data.e == 'aggTrade' ) {
      console.log(data.s+' price: '+data.p);
    } else console.log(stream, data);
  });
})();
bramvbilsen commented 5 years ago

I did something very similar but then with the Candlesticks Stream. I tried it using both your wonderful node JS wrapper and my own websockets implementation but the issue persists. Here is a more exact description of my issue:

I am pulling candlestick data (via websockets) for all symbols, for 5 different intervals and I keep the price of all symbols for every interval in an Object. If I then (5 minutes later) console.log my Object with all the data, I can clearly see that I do not have all data. Several intervals are missing.

I created a separate project just doing that to make sure that there weren't any bottlenecks in the project I am using this for. Same result...
Could you perhaps run this code and see if you are running into the same issue?

const Binance = require("node-binance-api");
const binance = new Binance();

async function getAllTickerSymbols(callback) {
    binance.prices(callback);
}

function startPullingData(symbols, intervals, callback) {
    intervals.forEach(interval => {
        binance.websockets.candlesticks(symbols, interval, callback);
    });
}

async function start() {
    const intervals = ["1m", "5m", "15m", "30m", "1h"];
    const res = {};
    getAllTickerSymbols((error, tickers) => {
        if (error) {
            throw new Error("Aborting, couldn't get all symbols");
        }
        const symbols = Object.keys(tickers);
        symbols.forEach(symbol => {
            res[symbol] = {};
        });
        startPullingData(symbols, intervals, data => {
            res[data.s][data.k.i] = data.k.c;
        });
        setTimeout(() => {
            // Clear the console.
            console.log("\x1Bc");
            console.log(res);
        }, 5 * 60 * 1000);
    });
}

start();

Here is a part of my output in which you can clearly see that some tickers were complete and other weren't at all: screenshot from 2018-11-02 12 43 03

jaggedsoft commented 5 years ago

What I would do in this situation is to run the chart function on 1 minute, but with the limit increased from 500 to 1000. Then I would convert the 1 minute candlestick data into longer timeframes using math. For example this library https://github.com/Algorithmic-Trading/forex.analytics has this function convertOHLC Converts OHLC data set to a larger timeframe (e.g. from 5 minute interval to 30 minute interval)

I just realized the chart function has no way to adjust the limit so it's stuck at the default of 500 results. I will make it possible to increase that to 1000 next release (edit: fixed) How is the chart function different from candlesticks? Chart will download all the historical candles so you have the last 1000 candles + it will be constantly updated with new candles as they become available.

jaggedsoft commented 5 years ago

Available in v0.8.6 https://github.com/jaggedsoft/node-binance-api/commit/3c03495f6231628771f5fda27065843b9e3a48be Converting 1m charts to 30m will give you a maximum of 32 candles I believe, or 16.66 hours of info. 5m might be better if you want longer term

binance.websockets.chart( "BTCUSDT", "1m", ( symbol, interval, chart ) => {
    let tick = binance.last( chart );
    const last = chart[tick].close;
    console.log( chart );
    // Optionally convert 'chart' object to array:
    // let ohlc = binance.ohlc(chart);
    // console.log(symbol, ohlc);
    console.log( symbol + " last price: " + last, "length: " + Object.keys( chart ).length );
}, 1000 ); // limit is the last parameter (max 1000)

The easiest way to subscribe to daily candles is with miniTicker, but this data is only sent once per second

bramvbilsen commented 5 years ago

@jaggedsoft That sounds promising, less demanding for the Binance server and less waiting for me!

It is a shame that Binance only allows getting the last 1000 candles though, it would make much more sense to have 1440 candles as a limit so you can get all candles of all the intervals for the past 24 hours. The 1 minute interval is kind of left out 😞
Anyhow, I am sure I can work around that!

I will try this out this evening and let you know.
Thank you for the incredibly fast update to allow for 1000 candles!

jaggedsoft commented 5 years ago

No worries friend. Unfortunately this method also comes with a caveat .. loading this historical data with 1000 limit uses up a lot of your API quota. So you probably need to delay 350ms between new calls to chart .. which would result in a warm up time of around two to three minutes before your program is correctly streaming all candles.. I just thought about this restriction. Nothing is easy! haha. But I guess that is part of the challenge here.

bramvbilsen commented 5 years ago

Challenge accepted, just have to keep my server running 24/7 😁

bramvbilsen commented 5 years ago

So I tried what you suggested, but got some unexpected results when getting the historical data. I'll add my code here, it is a pretty big chunk, but the problem resides in the callback of the binance.candlesticks method (I added all the code for completeness sake and it is easier if you'd like to test it yourself):

async function getCandlesForLast24Hours(
    tradingPairs: string[]
) {
    const binance = new Binance();
    const currentTime = new Date().getTime();
    let promises: Promise<ISimplifiedCandle[]>[] = [];
    tradingPairs.forEach((tradingPair: string) => {
        for (let i = 0; i < 2; i++) {
            let startTime: number;
            let endTime: number;
            if (i === 0) {
                startTime = currentTime - 24 * 60 * 60 * 1000; // 24h ago
                endTime = currentTime - 12 * 60 * 60 * 1000; // 12h ago
            } else {
                startTime = currentTime - 12 * 60 * 60 * 1000; //12h ago
                endTime = currentTime;
            }
            promises.push(
                new Promise((resolve, reject) => {
                    binance.candlesticks(
                        tradingPair,
                        "1m",
                        (error, ticks: Array<Array<any>>, symbol) => { // !!! This callback does not always give the expected outcome
                            if (error) {
                                reject(error);
                            }
                            // !!!! Problem here. See output bellow !!!!
                            if (!ticks.length) {
                                console.log(symbol);
                                console.log("-----------------");
                                console.log(ticks);
                            }
                            let candles: ISimplifiedCandle[] = [];
                            ticks.forEach(tick => {
                                candles.push({
                                    openTime: parseInt(tick[0]),
                                    open: parseFloat(tick[1]),
                                    high: parseFloat(tick[2]),
                                    low: parseFloat(tick[3]),
                                    close: parseFloat(tick[4]),
                                    baseAssetVolume: parseFloat(tick[5]),
                                    closeTime: parseInt(tick[6]),
                                    quoteAssetVolume: parseFloat(tick[7]),
                                    symbol: symbol
                                });
                            });
                            resolve(candles);
                        },
                        { limit: 720, startTime, endTime }
                    );
                })
            );
        }
    });
    // e.g. ISimplifiedCandle[0] will contain all candles for the first 12h of trading pair X
    //          ISimplifiedCandle[1] will contain all candles for the last 12h of trading pair X
    // e.g. ISimplifiedCandle[2] will contain all candles for the first 12h of trading pair Y
    //          ISimplifiedCandle[3] will contain all candles for the last 12h of trading pair Y
    return Promise.all(promises)
        .then((allCandles: ISimplifiedCandle[][]) => {
            const candlesPerSymbol: {
                [symbol: string]: ISimplifiedCandle[];
            } = {};
            tradingPairs.forEach((tradingPair: string) => {
                candlesPerSymbol[tradingPair] = [];
            });

            allCandles.forEach((candles: ISimplifiedCandle[]) => {
                candlesPerSymbol[candles[0].symbol] = [
                    ...candlesPerSymbol[candles[0].symbol],
                    ...candles
                ];
            });
            return candlesPerSymbol;
        })
        .catch(e => console.log(`\n\n${e}\n\n`));
}

The problem is that the ticks that the callback of the binance.candlesticks function does not always return an array (as per spec with the docs). It sometimes simply returns an empty object. Here is the output that I am seeing:

LRCETH
-----------------
{}

(followed by an Error of course because I can't take the length of an object)
Now I am not certain whether this is something that is wrong with my code, whether something is wrong with the library or whether something is wrong with the API itself.
Could you shine some light on this mysterious result?

bramvbilsen commented 5 years ago

It does BTW not only happen with LRCETH, I just got the same issue with ZRXBTC for example and have seen it with other trading pairs as well.

jaggedsoft commented 5 years ago

I would suggest we extend the error handler to handle empty responses

                            if (error) {
                                reject(error);
                            }

And if this happens put it in a queue to retry Only thing I can think of

jaggedsoft commented 5 years ago

I haven't checked out this guys code in awhile https://github.com/binance-exchange/binance-api-node But he might have a solution I haven't thought of

bramvbilsen commented 5 years ago

Alright, I am going to try the fork you just mentioned. It seems to also have types for typescript which is great!