FIX: [coinbase] fix stream handlers and order placing logics #1927
Conversation
…ust related constants for clarity
e2e05b0 to
2f1e02b
Compare
2f1e02b to
318d4c5
Compare
pkg/exchange/coinbase/convert.go
Outdated
| if order.FilledSize.Eq(order.Size) { | ||
| return types.OrderStatusFilled | ||
| } else { | ||
| return types.OrderStatusPartiallyFilled |
There was a problem hiding this comment.
return types.OrderStatusCanceled for this line?
There was a problem hiding this comment.
https://docs.cdp.coinbase.com/exchange/docs/websocket-channels#done
It should be filled.
No partially filled for done message.
Will fix it.
pkg/exchange/coinbase/convert.go
Outdated
| case api.OrderStatusRejected: | ||
| return types.OrderStatusRejected | ||
| case api.OrderStatusReceived: | ||
| return types.OrderStatusReceived |
There was a problem hiding this comment.
btw, Is Received = Update?
if this means if an order is working, then you will receive incremental "ExecutedQuantity" ? you might need update the status to PartiallyFilled and add the same check like LINE35-39?
There was a problem hiding this comment.
After discussion with @kbearXD, I think received is more close to new in bbgo.
In that case, a dedicated status for received is not necessary.
I will remove it and map received status on Coinbase to new in bbgo.
pkg/exchange/coinbase/convert.go
Outdated
| case api.OrderStatusRejected: | ||
| return types.OrderStatusRejected | ||
| case api.OrderStatusReceived: | ||
| return types.OrderStatusReceived |
There was a problem hiding this comment.
The status "received" in coinbase means that the order you submitted is valid and it will go into the matching engine. I think it's closed to "NEW" in bbgo.
The status "open" in coinbase means that the order is on the orderbook, so if you submit a order and it will be fully taker order. there will no "open" status. I think we need to check the remaining size to know the status is bbgo's PartiallyFilled or New
062dbce to
5b615ac
Compare
The purpose of this PR is to:
ActiveMessageasActivateMessageto be aligned with Coinbase documentation.These are preliminary works for the upcoming PR #1928.