FEATURE: Implement Coinbase API client and order management features #1903
Conversation
…d cancel order functionalities
|
dboy seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
c353e97 to
43a6476
Compare
…ation for API client
| // ClientOID must be uuid | ||
| ClientOID string `json:"client_oid" db:"client_oid"` | ||
| Stop string `json:"stop" db:"stop"` | ||
| StopPrice json.Number `json:"stop_price" db:"stop_price"` |
| import "context" | ||
|
|
||
| func (client *RestAPIClient) GetOrderTrades(ctx context.Context, orderID string, limit int, before *string) (TradeSnapshot, error) { | ||
| req := GetOrderTradesRequest{ |
| @echo "Generate symbols.go" && go run ./generate_symbol_map.go | ||
|
|
||
| go-generate: | ||
| @echo "Running \`go generate\`" && go generate ./... |
There was a problem hiding this comment.
you don't need the makefile actually? you can embed them in the go:generate comment
There was a problem hiding this comment.
I just want a way to document how I generate the symbols.go
pkg/exchange/coinbase/convert.go
Outdated
| QuoteQuantity: cbTrade.Size.Mul(cbTrade.Price), | ||
| Symbol: cbTrade.ProductID, | ||
| Side: cbTrade.Side.GlobalSideType(), | ||
| IsBuyer: cbTrade.Liquidity == "T", |
There was a problem hiding this comment.
What do T and M represent?
There was a problem hiding this comment.
"T"aker and "M"aker
There was a problem hiding this comment.
Perhaps you can use cb Trade.Liquidity == api.Liquidity Maker
There was a problem hiding this comment.
Good idea. Will do.
pkg/exchange/coinbase/exchage.go
Outdated
| cur := strings.ToUpper(b.Currency) | ||
| balances[cur] = types.Balance{ | ||
| Currency: cur, | ||
| Available: b.Available, |
There was a problem hiding this comment.
I think you missed the Lock field?
There was a problem hiding this comment.
It's not well documented in the CoinBase API which is the right value for Locked:
https://docs.cdp.coinbase.com/exchange/reference/exchangerestapi_getaccounts
After searching around on the web, I think the reasonable choice would be balance - available from the CoinBase side.
Let's use balance - available for Locked for now.
I'll update the NetAsset as well.
pkg/exchange/coinbase/convert.go
Outdated
| QuoteQuantity: cbTrade.Size.Mul(cbTrade.Price), | ||
| Symbol: cbTrade.ProductID, | ||
| Side: cbTrade.Side.GlobalSideType(), | ||
| IsBuyer: cbTrade.Liquidity == "T", |
There was a problem hiding this comment.
Perhaps you can use cb Trade.Liquidity == api.Liquidity Maker
pkg/exchange/coinbase/exchage.go
Outdated
| done := false | ||
| if len(cbOrders) < 1000 || len(cbOrders) == 0 { | ||
| done = true | ||
| } |
There was a problem hiding this comment.
It can be simplify to
if len(cbOrders) < paginationLimit {
return cbOrders,nil)
}
done := false
pkg/exchange/coinbase/exchage.go
Outdated
| if len(cbTrades) < paginationLimit || len(cbTrades) == 0 { | ||
| done = true | ||
| } |
There was a problem hiding this comment.
In order to get all trades from Coinbase Exchange API, I believe applying pagination is a must.
See:
4c395e2 to
8cf8461
Compare
| } | ||
| done := false | ||
| for { | ||
| if done { |
There was a problem hiding this comment.
you need to handle the ctx.Done() in the loop.
|
|
||
| done := false | ||
| for { | ||
| if done { |
There was a problem hiding this comment.
you need to handle the ctx.Done() in the loop.
pkg/exchange/coinbase/convert.go
Outdated
| return &ticker | ||
| } | ||
|
|
||
| func toGlobalBalance(cur string, cbBalance *api.Balance) *types.Balance { |
There was a problem hiding this comment.
for this case, you can just return the value struct
pkg/exchange/coinbase/exchage.go
Outdated
| balances := make(types.BalanceMap) | ||
| for _, cbBalance := range accounts { | ||
| cur := strings.ToUpper(cbBalance.Currency) | ||
| balances[cur] = *toGlobalBalance(cur, &cbBalance) |
There was a problem hiding this comment.
using dereferencing the return pointer could be dangerous, so for this function, you can just return value (you always have a value to return)
pkg/exchange/coinbase/exchage.go
Outdated
| } | ||
| return &types.Order{ | ||
| Exchange: types.ExchangeCoinBase, | ||
| UUID: res.ID, |
There was a problem hiding this comment.
you need to assign SubmitOrder into the SubmitOrder field,
and you might need to query the order details, take a look at bybit or bitget's SubmitOrder() implementation
pkg/exchange/coinbase/exchage.go
Outdated
| } | ||
| orders := make([]types.Order, 0, len(cbOrders)) | ||
| for _, cbOrder := range cbOrders { | ||
| orders = append(orders, *toGlobalOrder(&cbOrder)) |
There was a problem hiding this comment.
ditto, avoid de-referencing the value directly from the function call since it's a plain data object.
| return orders, nil | ||
| } | ||
|
|
||
| func (e *Exchange) queryOrdersByPagination(ctx context.Context, symbol string, status []string) ([]api.Order, error) { |
There was a problem hiding this comment.
you might need to apply the rate limiter here, or from the request itself. requestgen supports rate limiter's Wait() method call.
pkg/exchange/coinbase/exchage.go
Outdated
| } | ||
|
|
||
| func (e *Exchange) CancelOrders(ctx context.Context, orders ...types.Order) error { | ||
| failedOrderIDs := make([]string, 0) |
There was a problem hiding this comment.
this is minor, simple form is:
var failedOrderIDs []string
they are equivalent.
pkg/exchange/coinbase/exchage.go
Outdated
| log.WithError(err).Errorf("failed to cancel order: %v", order.UUID) | ||
| failedOrderIDs = append(failedOrderIDs, order.UUID) | ||
| } | ||
| log.Infof("order %v has been cancelled", res) |
There was a problem hiding this comment.
you need an "else" for this Infof log?
|
|
||
| // ExchangeMarketDataService | ||
| func (e *Exchange) NewStream() types.Stream { | ||
| return nil |
pkg/exchange/coinbase/exchage.go
Outdated
| } | ||
| marketMap := make(types.MarketMap) | ||
| for _, m := range markets { | ||
| marketMap[toGlobalSymbol(m.ID)] = *toGlobalMarket(&m) |
pkg/exchange/coinbase/exchage.go
Outdated
| } | ||
| // default limit is 300, which is the maximum limit of the Coinbase Exchange API | ||
| if options.Limit == 0 { | ||
| options.Limit = 300 |
There was a problem hiding this comment.
pull these numbers out to constants?
There was a problem hiding this comment.
Done.
It has been renamed as DefaultKLineLimit.
pkg/exchange/coinbase/exchage.go
Outdated
| } | ||
| if options.Limit > 300 { | ||
| log.Warnf("limit %d is greater than the maximum limit 300, set to 300", options.Limit) | ||
| options.Limit = 300 |
5215b79 to
ed7d2c1
Compare
pkg/exchange/coinbase/exchage.go
Outdated
| if done { | ||
| break | ||
| } |
There was a problem hiding this comment.
My bad. Silly rebase error.
pkg/exchange/coinbase/exchage.go
Outdated
| if done { | ||
| break | ||
| } |
There was a problem hiding this comment.
To implement rate limits on each API, refer to the code snippet found in the following link: Cancel Order Request in Bybit API.
…ne, function return pointers)
ed7d2c1 to
d1ad710
Compare
Implement following exchange interfaces:
ExchangeMinimalExchangeTradeServiceExchangeOrderQueryServiceExchangeMarketDataServiceis WIP (Impl. forNewStreamwill be in another PR)TODO: unit tests