FEATURE: [depth]support binance futures orderbook depth buffer #1902
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1902 +/- ##
==========================================
- Coverage 23.04% 22.71% -0.33%
==========================================
Files 664 667 +3
Lines 50301 51185 +884
==========================================
+ Hits 11590 11626 +36
- Misses 37846 38692 +846
- Partials 865 867 +2
... and 10 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
pkg/depth/buffer.go
Outdated
| // skip old events | ||
| if u.FinalUpdateID <= finalUpdateID { | ||
| continue | ||
| if b.isFutures { |
There was a problem hiding this comment.
what's the difference between futures and non-futures?
There was a problem hiding this comment.
For futures, packet loss detection is stricter: it requires pu == previous u. If this condition isn't met, it means data might be missing, so you need to reinitialize the order book.
https://developers.binance.com/docs/derivatives/usds-margined-futures/websocket-market-streams/How-to-manage-a-local-order-book-correctly
For spot, as long as U <= lastUpdateId <= u, the order book update continues. But if U > local order book update ID, you have to rebuild it from scratch.
pkg/depth/buffer.go
Outdated
| } | ||
| if u.FirstUpdateID <= finalUpdateID && u.FinalUpdateID >= finalUpdateID { | ||
| pushUpdates = append(pushUpdates, u) | ||
| } |
There was a problem hiding this comment.
I think this logic is the same as non-futures? it's just reversed ?
There was a problem hiding this comment.
And the field PreviousUpdateID is not used here?
There was a problem hiding this comment.
I see, it's indeed the same logic. I'll merge it.
anywhy
force-pushed
the
futures_depth_buffer
branch
from
2691d3d to
6b709e5
Compare
pkg/depth/buffer.go
Outdated
| b.logger.Info("try fetching the snapshot due to missing update") | ||
| go b.tryFetch() | ||
| }) | ||
| b.EmitReset() |
There was a problem hiding this comment.
EmitReset needs to be called after the mutex Unlock in order to avoid nested locking.
In your case, when calling this method, the mutex is still in the locked mode, did you verify this behavior? does it cause other effect?
pkg/depth/buffer.go
Outdated
| @@ -236,7 +265,7 @@ func (b *Buffer) fetchAndPush() error { | |||
| continue | |||
| } | |||
|
|
|||
| if u.FirstUpdateID > finalUpdateID+1 { | |||
| if u.FirstUpdateID > finalUpdateID+1 || (b.isFutures && idx > 0 && u.PreviousUpdateID != b.buffer[idx-1].FinalUpdateID) { | |||
There was a problem hiding this comment.
could you please explain the reason in the comment we need the isFutures flag for the previous id check?
There was a problem hiding this comment.
I was originally planning to validate that the event in the buffer needs to satisfy U <= lastUpdateId <= u, but it seems unnecessary.
pkg/depth/buffer.go
Outdated
| @@ -253,6 +282,10 @@ func (b *Buffer) fetchAndPush() error { | |||
| b.buffer = nil | |||
|
|
|||
| // set the final update ID so that we will know if there is an update missing | |||
| if b.isFutures && len(pushUpdates) > 0 { | |||
| // reset finalUpdateId to first update event | |||
| finalUpdateID = pushUpdates[0].FinalUpdateID | |||
There was a problem hiding this comment.
sorry, why do we need to reset the finalUpdateId here? I don't quiet understand
There was a problem hiding this comment.
It seems I misunderstood the code logic here; this is completely unnecessary.
anywhy
force-pushed
the
futures_depth_buffer
branch
2 times, most recently
from
5a1422d to
7dfb507
Compare
anywhy
force-pushed
the
futures_depth_buffer
branch
from
7dfb507 to
6242fa6
Compare
anywhy
changed the title
No description provided.