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. |
dboyliao
force-pushed
the
dboy/coinbase
branch
2 times, most recently
from
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:
dboyliao
force-pushed
the
dboy/coinbase
branch
from
4c395e2 to
8cf8461
Compare
…numeric fields for precision
…creation, by setting request parameters via accessors, for better maintainability.
dboyliao
force-pushed
the
dboy/coinbase
branch
from
21327f5 to
aeebe4a
Compare
…d clarity and maintainability
…nversion and clarity in account balance queries
dboyliao
force-pushed
the
dboy/coinbase
branch
from
bdf7c9d to
674a0d6
Compare
| productID string `param:"product_id,required"` | ||
| granularity *string `param:"granularity" validValues:"60,300,900,3600,21600,86400"` | ||
| start *string `param:"start"` | ||
| end *string `param:"end"` |
There was a problem hiding this comment.
check this file for the timestamp parsing usage:
| orderID string `param:"order_id,required"` | ||
| limit int `param:"limit"` | ||
| before *string `param:"before"` | ||
| after *string `param:"after"` |
There was a problem hiding this comment.
you can use *time.Time here?
| startDate *time.Time `param:"start_date" timeFormat:"RFC3339"` | ||
| endDate *time.Time `param:"end_date" timeFormat:"RFC3339"` | ||
| before *time.Time `param:"before" timeFormat:"RFC3339"` | ||
| after *time.Time `param:"after" timeFormat:"RFC3339"` |
There was a problem hiding this comment.
pagination, it's a number, not a date string?
https://docs.cdp.coinbase.com/exchange/docs/rest-pagination
There was a problem hiding this comment.
The pagination id used for GET /orders are before and after which are timestamp string in RFC3339.
See:
https://docs.cdp.coinbase.com/exchange/reference/exchangerestapi_getorders
…consistency in bbgo
Implement following exchange interfaces:
ExchangeMinimalExchangeTradeServiceExchangeOrderQueryServiceExchangeMarketDataServiceis WIP (Impl. forNewStreamwill be in another PR)TODO: unit tests