tardis-dev / serum-vial

Real-time WebSocket market data API for Serum
Mozilla Public License 2.0
173 stars 60 forks source link

`open`/`change` messages should *not always* be processed before `fill`/`done` #30

Closed chris-rr closed 2 years ago

chris-rr commented 3 years ago

Hi @thaaddeus,

Have noticed during examination of data recorded via this excellent software a scenario which implies that the logic for ordering messages published to the level3 stream appears to require some modification.

In short, there are several instances where an order is opened on a particular update, and on the same update a fill message is generated for a different order which is already on the book. In a number of cases the newly placed order is for a better price than the existing one which is traded; this implies immediately that in fact the fill message should be handled before the open, since if the new order were already open when the fill occurred, it would have traded against the incoming order instead of the existing.

For example, I see progressions like the following:

1. 2021-11-03T17:48:55.477Z : Sell limit order #4147160109163233996678936 *open* for 2996.7 SOL @ 224.818 USDT 
2. 2021-11-03T17:55:29.956Z : Sell limit order #4146828067769907224750968 *open* for 130.5 SOL @ 224.800 USDT 
3. 2021-11-03T17:55:29.956Z : Sell limit order #4147160109163233996678936 *traded* for 0.6 SOL @ 224.818 USDT

If analysing this, it would be expected that the order open at 2. should be traded against any incoming order, as it is for a better price - clearly the trade at 3. then actually occurred before the open at 2. The message ordering from the level3 stream should reflect this.

It can also occur that all three messages are generated from the same update, and have the same timestamp, implying that the first order was immediately partially filled when it was received - the same principle applies here.

1. 2021-11-03T18:47:06.482Z : Buy limit order #4323732343436781781789141 *open* for 300.0 SOL @ 234.389 USDT 
2. 2021-11-03T18:47:06.482Z : Buy limit order #4327292565043007725251028 *open* for 22.0 SOL @ 234.582 USDT 
3. 2021-11-03T18:47:06.482Z : Buy limit order #4323732343436781781789141 *traded* for 198.2 SOL @ 234.389 USDT

Have not investigated yet if there is any way to confirm the ordering via the update data received from RPC node; if not then would perhaps have to develop logic which compares the price of opens and fills on the same update in order to deduce an ordering which is correct at least in relation to the traded prices.

Kindly let me know if you agree with this assessment, and if needing any further information or input from my side.

Thanks!

thaaddeus commented 3 years ago

I'll look into this, sure. You can also recognize if the messages are coming from single 'block' by comparing 'slot' field (same slot -> single solana block)

thaaddeus commented 3 years ago

Hi, please try v1.2.8 it has updated order of l3 messages, but still not sure if it's correct, it's rather tricky to do it correctly.

chris-rr commented 3 years ago

Thanks @thaaddeus for the quick response, will check this out and let you know 👍

thaaddeus commented 3 years ago

actually please use v1.2.9 as previous had a bug.

chris-rr commented 3 years ago

Hi @thaaddeus, have checked data recorded over the weekend, not seeing any further occurrences of this ordering issue - the method you had applied appears fairly solid so don't expect this to be an issue any longer, but will post a comment here if encountering any other cases.

Thanks again!

chris-rr commented 2 years ago

Hi @thaaddeus, found another unexpected scenario among the data; seems possibly related so attaching to this issue, but not able to confidently decipher what happened so may actually be a new issue.

An instance occurred where an open message is generated for an order, followed by a fill for the complete size of that order, then another open for the same order with a different size, and then a fill for that "extra" size, before the order is finally done. These events all occur on the same update:

{
      "type": "open",
      "market": "SOL/USDC",
      "timestamp": "2021-11-28T18:44:38.298Z",
      "slot": 109152064,
      "version": 3,
      "orderId": "3386453277051599512988827",
      "clientId": "646587",
      "side": "sell",
      "price": "183.580",
      "size": "164.7",
      "account": "EpAdzaqV13Es3x4dukfjFoCrKVXnZ7y9Y76whgMHo5qx",
      "accountSlot": 2,
      "feeTier": 0
  }
  {
    "type": "fill",
    "market": "SOL/USDC",
    "timestamp": "2021-11-28T18:44:38.298Z",
    "slot": 109152064,
    "version": 3,
    "orderId": "3386453277051599512988827",
    "clientId": "646587",
    "side": "sell",
    "price": "183.580",
    "size": "164.7",
    "maker": true,
    "feeCost": -9.070687,
    "account": "EpAdzaqV13Es3x4dukfjFoCrKVXnZ7y9Y76whgMHo5qx",
    "accountSlot": 2,
    "feeTier": 0
}
{
    "type": "open",
    "market": "SOL/USDC",
    "timestamp": "2021-11-28T18:44:38.298Z",
    "slot": 109152064,
    "version": 3,
    "orderId": "3386453277051599512988827",
    "clientId": "646587",
    "side": "sell",
    "price": "183.580",
    "size": "0.9",
    "account": "EpAdzaqV13Es3x4dukfjFoCrKVXnZ7y9Y76whgMHo5qx",
    "accountSlot": 2,
    "feeTier": 0
}
{
    "type": "fill",
    "market": "SOL/USDC",
    "timestamp": "2021-11-28T18:44:38.298Z",
    "slot": 109152064,
    "version": 3,
    "orderId": "3386453277051599512988827",
    "clientId": "646587",
    "side": "sell",
    "price": "183.580",
    "size": "0.9",
    "maker": true,
    "feeCost": -0.049566,
    "account": "EpAdzaqV13Es3x4dukfjFoCrKVXnZ7y9Y76whgMHo5qx",
    "accountSlot": 2,
    "feeTier": 0
}
thaaddeus commented 2 years ago

Hi @chris-rr I think I know what that could be but to be frank I'm considering removing the logic that artificially adds 'open' orders for maker:true fills (https://github.com/tardis-dev/serum-vial/issues/31) - currently it checks for fills with maker:true flag if those have corresponding open orders and if not adds one, it can only happen for orders that were added to the order book and matched in single slot. Issue that happens now is that single order was added to there order book and matched fully in the same slot but by two taker orders resulting in two fill messages - so two open orders were added, and only one should have been. I could fix that, but still does it cover all edge cases? Perhaps it's simpler to assume that there can be fills with maker:true flag without corresponding open orders for cases when those are matched in single update/slot?

chris-rr commented 2 years ago

Hmm, I think it is too inconsistent with the expected message progression to have maker fills without corresponding order opens, we should know the size of the opened order before it starts to filled. I would lean towards modifying this logic to check all fills in an update and deduce the total size of the order to be opened, and use this to generate a single open message for the full size followed by two fills - this would seem to be the correct logic for this open+fill on a single update case.

thaaddeus commented 2 years ago

v1.3.8 should have it fixed.