Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: process pending order update for active order book #1060

Merged
merged 3 commits into from
Feb 17, 2023

Conversation

c9s
Copy link
Owner

@c9s c9s commented Feb 17, 2023

No description provided.

@bbgokarma-bot
Copy link

Welcome back! @c9s, This pull request may get 283 BBG.

// the caller can add the created order to the active order book or the order store to collect trades.
// this method is used when you have large amount of orders to be sent and most of the orders might be filled as taker order.
// channel orderC will be closed when all the submit orders are submitted.
func BatchPlaceOrderChan(ctx context.Context, exchange types.Exchange, orderC chan types.Order, submitOrders ...types.SubmitOrder) (types.OrderSlice, []int, error) {
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is unrelated, but we might need this in the future

@bbgokarma-bot
Copy link

Re-estimated karma: this pull request may get 294 BBG

@codecov
Copy link

codecov bot commented Feb 17, 2023

Codecov Report

Merging #1060 (4dc4f73) into main (21cdb7a) will increase coverage by 0.02%.
The diff coverage is 31.37%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1060      +/-   ##
==========================================
+ Coverage   19.06%   19.08%   +0.02%     
==========================================
  Files         434      434              
  Lines       33033    33078      +45     
==========================================
+ Hits         6298     6314      +16     
- Misses      26199    26226      +27     
- Partials      536      538       +2     
Impacted Files Coverage Δ
pkg/bbgo/order_execution.go 3.15% <0.00%> (-0.30%) ⬇️
pkg/types/ordermap.go 0.00% <0.00%> (ø)
pkg/bbgo/activeorderbook.go 11.05% <66.66%> (+7.03%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 21cdb7a...4dc4f73. Read the comment docs.

@bbgokarma-bot
Copy link

Re-estimated karma: this pull request may get 372 BBG

Copy link
Collaborator

@gx578007 gx578007 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Others LGTM

// add the order to the active order book and check the pending order
func (b *ActiveOrderBook) add(order types.Order) {
if pendingOrder, ok := b.pendingOrderUpdates.Get(order.OrderID); ok {
if pendingOrder.UpdateTime.Time().After(order.UpdateTime.Time()) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a rare condition that a pending order and a new coming order are updated at the same timestamp (in millisecond or even in second).
Another way is to compare the filled amount or the state change.

@c9s c9s merged commit 03e25f1 into main Feb 17, 2023
@c9s c9s deleted the fix/active-order-book-pending-order-update branch February 17, 2023 12:18
@bbgokarma-bot
Copy link

Hi @c9s,

Well done! 387 BBG has been sent to your polygon wallet. Please check the following tx:

https://polygonscan.com/tx/0x018fd7e0a34d1983a902a969ee4c7cf39a1bceb323ea8535251ff3ec17b39ffc

Thank you for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants