-
-
Notifications
You must be signed in to change notification settings - Fork 289
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: Fix duplicate orders caused by position risk control #1327
Conversation
Welcome back! @narumiruna, This pull request may get 227 BBG. |
Codecov Report
@@ Coverage Diff @@
## main #1327 +/- ##
==========================================
- Coverage 20.78% 20.74% -0.04%
==========================================
Files 558 558
Lines 39790 39800 +10
==========================================
- Hits 8269 8257 -12
- Misses 30921 30944 +23
+ Partials 600 599 -1
Continue to review full report in Codecov by Sentry.
|
a86e95c
to
597645d
Compare
Re-estimated karma: this pull request may get 247 BBG |
pkg/risk/riskcontrol/position.go
Outdated
p.activeOrderBook.BindStream(session.UserDataStream) | ||
|
||
p.OnReleasePosition(func(quantity fixedpoint.Value, side types.SideType) { | ||
if err := p.orderExecutor.CancelOrders(ctx, p.activeOrderBook.Orders()...); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think p.activeOrderBook also provides the cancel api
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
597645d
to
ac08b03
Compare
Re-estimated karma: this pull request may get 272 BBG |
@@ -45,27 +72,16 @@ func NewPositionRiskControl(orderExecutor bbgo.OrderExecutorExtended, hardLimit, | |||
} | |||
|
|||
log.Infof("RiskControl: position limit exceeded, submitting order to reduce position: %+v", submitOrder) | |||
createdOrders, err := orderExecutor.SubmitOrders(context.Background(), submitOrder) | |||
createdOrders, err := p.orderExecutor.SubmitOrders(ctx, submitOrder) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need to be careful here, the trade collector might get stuck
would better to run a back-test to test this change~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will run a back-test later
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the trade collector didn't get stuck
ac08b03
to
4a6f6f7
Compare
Re-estimated karma: this pull request may get 344 BBG |
Hi @narumiruna, Well done! 349 BBG has been sent to your polygon wallet. Please check the following tx: https://polygonscan.com/tx/0xccce679d8e1eee717e0ec0904a92b66a08d0ab3038774e7a5f365d5ed3e67ae4 Thank you for your contribution! |
cancel previous order before submitting position release order to prevent duplicate orders