Order book resync (after dropped message) appears broken

[tensleep]

When the OrderbookSync class encounters an unexpected sequence number, it attempts to resync by requesting the entire order book snapshot from the REST API.

The response is processed by calling Orderbook.state(). Instead of replacing the existing orderbook data, it adds the new orders without removing any old ones. This is confirmed with a test snippet that calls .state() twice:

const book = new Gdax.Orderbook();

book.state({bids: [ [ 100, 2, "oldX" ] ], asks: [ [ 101, 3, "oldY" ] ] });
console.log('first state: %s', JSON.stringify(book.state()));

book.state({bids: [ [ 101, 4, "newA" ] ], asks: [ [ 102, 1, "newB" ] ] });
console.log('second state: %s', JSON.stringify(book.state()));

Result:

first state: {"asks":[{"id":"oldY","side":"sell","price":"101","size":"3"}],"bids":[{"id":"oldX","side":"buy","price":"100","size":"2"}]}
second state: {"asks":[{"id":"oldY","side":"sell","price":"101","size":"3"},{"id":"newB","side":"sell","price":"102","size":"1"}],"bids":[{"id":"newA","side":"buy","price":"101","size":"4"},{"id":"oldX","side":"buy","price":"100","size":"2"}]}

The orderbook now contains both old and new orders and is nonsensical. It seems the same thing would happen when OrderbookSync finds a lost message and resyncs?

[BGrades]

I agree with @tensleep. This is cause for people reporting that their orderbook gets out of sync at random time after start. Thank you tensleep for finding it.

[sdclibbery]

I'm afraid I haven't raised a PR, but the following horrible monkey patch seems to fix the desyncs quite reliably:

const oldOrderBookStateMethod = orderBook.state
orderBook.state = function (book) {
  if (book) {
    this._asks.clear()
    this._bids.clear()
  }
  return oldOrderBookStateMethod.call(this, book)
}

where orderBook is an orderbook owned by your OrderBookSync; eg const orderBook = orderbookSync.books['BTC-USD']

Using this, I get rock solid order book syncing, so my order book is in sync with the web site at all times.

Edit: updated to include missing 'return' statement as found in the conversation below.

[johank16]

Hey sdclibbery, could you elaborate on how you did this? I tried the following but am getting errors. Not sure exactly what your snippet is doing, but I think overwriting the default state function to clear the asks and bids first, and then do what it normally does?

let openCoinStream = function(coin) {
    console.log('subscribing to coinbase coin ' + coin)
    coinbaseProCoinStream[coin] = new coinbaseProAPI.OrderbookSync(coin)

    coinbaseProCoinStream[coin].on('open', function() {
        setTimeout(function () 
            {
                const oldOrderBookStateMethod = coinbaseProCoinStream[coin].books[coin].state 
                coinbaseProCoinStream[coin].books[coin].state = function (book) 
                {
                    if(book) {
                        console.log('clearing asks and bids out of books object')
                        this._asks.clear() 
                        this._bids.clear() 
                    }
                    oldOrderBookStateMethod.call(this, book);
                }
            }, 500);
    })

    coinbaseProCoinStream[coin].on('error', err => {
        console.log('error with ' + coin + ' coinbasepro stream: ' + err.stack)
        console.log('trying to redo connection in 5 seconds')
        setTimeout(function() { openCoinStream(coin) }, 5000)
    })
    coinbaseProCoinStream[coin].on('close', err => {
        console.log('coinbasepro order book websocket closed for ' + coin + ', reconnecting in 5 seconds')
        setTimeout(function() { openCoinStream(coin) }, 5000)
    })
} 

let bids = {} 
let asks = {} 

if(coinbaseProCoinStream[coinName] == null)
    openCoinStream(coinName)
else
{
    if(coinbaseProCoinStream[coinName].books[coinName] != null)
    {
        let orderBook = coinbaseProCoinStream[coinName].books[coinName].state()

        for(let i in orderBook.bids)
        {
            if(bids[orderBook.bids[i].price] == null)
                bids[orderBook.bids[i].price] = 0
            bids[orderBook.bids[i].price] += Number(orderBook.bids[i].size)
        }

        for(let i in orderBook.asks)
        {
            if(asks[orderBook.asks[i].price] == null)
                asks[orderBook.asks[i].price] = 0
            asks[orderBook.asks[i].price] += Number(orderBook.asks[i].size)
        }
    }
}

let bidPrices = Object.keys(bids).sort(function(a,b){return(b-a)})
let askPrices = Object.keys(asks).sort()

if(askPrices[0] < bidPrices[0])
    console.log('coinbasepro order book is out of sync for ' + coinName)

[sdclibbery]

The only difference I can see from my code is that mine runs asynchronously a little while after I create the OrderBookSync. Not immediately sure why that would make a difference though. What errors do you get?

[johank16]

Thanks for the quick reply! I edited my code above so that it waits to overwrite the 'state' function until after the connection has been opened, and also threw in a setTimeout for 500 ms later just to be safe. But I'm still getting the same error. I also tried doing just the setTimeout outside of the .on('open' part, but same thing.

The error I'm getting is at the part that tries to read the bids "for(let i in orderBook.bids)" about 21 lines up in the code from my previous comment.

TypeError: Cannot read property bids of undefined 
    at Object.apiQueryCoinbasePro (/... path to the line i mentioned above) 
    at Object.apiQuery (/... my function that routes general API calls to exchange-specific methods )

I put some console logging at the very beginning of the loadOrderbook and state functions and it seems like the code is: First time function is run: 1) opens the coin stream 2) runs loadOrderbook 3) clears the asks and bids out of the books object 4) runs state, where the "book" object passed in looks like the full initial order book

Subsequent calls (skips initialization): 1) calls state, where the "book" object passed in is undefined 2) State returns an undefined object, which later causes it to throw an error that it can't read bids of undefined

One question: am I supposed to be passing in an object when calling state in the second half of my code let orderBook = coinbaseProCoinStream[coinName].books[coinName].state() ? I tried let orderBook = coinbaseProCoinStream[coinName].books[coinName]; orderBook.state(orderBook); but that lead to a Cannot read property 'forEach' of undefined error...

[johank16]

I think the problem with the above is actually that the last line in your snippet oldOrderBookStateMethod.call(this, book) should have a "return" right in front of it?

That seems to be giving me a working order book. Going to run it for a while now to see if it stays synced

[sdclibbery]

Good spot! I'm not acrtually calling state() in my code, so I never noticed that.

[johank16]

So after a couple weeks of testing, I'm still getting out of sync errors. Albeit it takes a little longer to show up. Looking through the code, one thing looks weird to me on orderbook.js line 38:

      this._bids.reach(bid => book.bids.push(...bid.orders));
      this._asks.each(ask => book.asks.push(...ask.orders)); 

is this actually supposed to be ".reach"? or is this a typo for ".each"? tried googling and I'm not finding any documentation for a .reach function, and not seeing it defined in any of the other files..

[drewrothstein]

Hi, we are closing out PRs + Issues as this project is being archived.