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: [exchange] fix bugs and add integration tests for Coinbase Exchange #1914

Merged
merged 6 commits into from
Feb 26, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions pkg/exchange/coinbase/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ func toGlobalOrder(cbOrder *api.Order) types.Order {
UUID: cbOrder.ID,
OrderID: FNV64a(cbOrder.ID),
OriginalStatus: string(cbOrder.Status),
CreationTime: types.Time(cbOrder.CreatedAt),
CreationTime: cbOrder.CreatedAt,
}
}

Expand Down Expand Up @@ -60,13 +60,15 @@ func FNV64a(text string) uint64 {
return hash.Sum64()
}

func toGlobalKline(symbol string, granity string, candle *api.Candle) types.KLine {
func toGlobalKline(symbol string, interval types.Interval, candle *api.Candle) types.KLine {
startTime := candle.Time.Time()
endTime := startTime.Add(interval.Duration())
kline := types.KLine{
Exchange: types.ExchangeCoinBase,
Symbol: symbol,
StartTime: types.Time(candle.Time),
EndTime: types.Time(time.Time(candle.Time).Add(types.Interval(granity).Duration())),
Interval: types.Interval(granity),
StartTime: types.Time(startTime),
EndTime: types.Time(endTime),
Interval: interval,
Open: candle.Open,
Close: candle.Close,
High: candle.High,
Expand Down
62 changes: 35 additions & 27 deletions pkg/exchange/coinbase/exchage.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,18 +145,20 @@ func (e *Exchange) SubmitOrder(ctx context.Context, order types.SubmitOrder) (cr
req.Price(order.Price)
}

// set time in force
switch order.TimeInForce {
case types.TimeInForceGTC:
req.TimeInForce("GTC")
case types.TimeInForceIOC:
req.TimeInForce("IOC")
case types.TimeInForceFOK:
req.TimeInForce("FOK")
case types.TimeInForceGTT:
req.TimeInForce("GTT")
default:
return nil, fmt.Errorf("unsupported time in force: %v", order.TimeInForce)
// set time in force, using default if not set
if len(order.TimeInForce) > 0 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you set a default timeInForce beforehand?

Copy link
Collaborator Author

@dboyliao dboyliao Feb 26, 2025

Choose a reason for hiding this comment

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

I want to leave it unset if it's not given by to user.
So it will fallback to Coinbase API defaults.
I want it to be consistent with Coinbase API.

switch order.TimeInForce {
case types.TimeInForceGTC:
req.TimeInForce("GTC")
case types.TimeInForceIOC:
req.TimeInForce("IOC")
case types.TimeInForceFOK:
req.TimeInForce("FOK")
case types.TimeInForceGTT:
req.TimeInForce("GTT")
default:
return nil, fmt.Errorf("unsupported time in force: %v", order.TimeInForce)
}
}
// client order id
if len(order.ClientOrderID) > 0 {
Expand Down Expand Up @@ -286,7 +288,7 @@ func (e *Exchange) QueryTickers(ctx context.Context, symbol ...string) (map[stri
for _, s := range symbol {
ticker, err := e.QueryTicker(ctx, s)
if err != nil {
return nil, errors.Wrap(err, "failed to get tickers")
return nil, errors.Wrapf(err, "failed to get ticker for %v", s)
}
tickers[s] = *ticker
}
Expand All @@ -305,8 +307,8 @@ func (e *Exchange) QueryKLines(ctx context.Context, symbol string, interval type
log.Warnf("limit %d is greater than the maximum limit 300, set to 300", options.Limit)
options.Limit = DefaultKLineLimit
}
granity := interval.String()
req := e.client.NewGetCandlesRequest().ProductID(toLocalSymbol(symbol)).Granularity(granity)
granularity := fmt.Sprintf("%d", interval.Seconds())
req := e.client.NewGetCandlesRequest().ProductID(toLocalSymbol(symbol)).Granularity(granularity)
if options.StartTime != nil {
req.Start(*options.StartTime)
}
Expand All @@ -317,17 +319,20 @@ func (e *Exchange) QueryKLines(ctx context.Context, symbol string, interval type
if err != nil {
return nil, errors.Wrapf(err, "failed to get klines(%v): %v", interval, symbol)
}
if len(rawCandles) > options.Limit {
rawCandles = rawCandles[:options.Limit]
}
klines := make([]types.KLine, 0, len(rawCandles))
candles := make([]api.Candle, 0, len(rawCandles))
for _, rawCandle := range rawCandles {
candle, err := rawCandle.Candle()
if err != nil {
log.Warnf("invalid raw candle detected, skipped: %v", rawCandle)
log.Warnf("invalid raw candle detected, skipping: %v", rawCandle)
continue
}
klines = append(klines, toGlobalKline(symbol, granity, candle))
candles = append(candles, *candle)
}

klines := make([]types.KLine, 0, len(candles))
for _, candle := range candles {
kline := toGlobalKline(symbol, interval, &candle)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Bbgo uses go 1.21.6. Before go 1.22, the variable between each for loop iteration is the same variable. That means the &candle always points to the same address unless you use go 1.22. Please ensure this behavior is expected.

https://go.dev/blog/loopvar-preview

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

截圖 2025-02-26 下午2 23 28
Since I do not keep the pointer (&candle) in the body of toGlobalKline(), I think it will do what is expected in go 1.21.6.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

截圖 2025-02-26 下午3 06 05
截圖 2025-02-26 下午3 06 14

I compile and run it with go1.21.6
As shown in the screenshot, there should be no issue where pointers outlive the loop.

klines = append(klines, kline)
}
return klines, nil
}
Expand All @@ -344,7 +349,7 @@ func (e *Exchange) QueryOrder(ctx context.Context, q types.OrderQuery) (*types.O
}

func (e *Exchange) QueryOrderTrades(ctx context.Context, q types.OrderQuery) ([]types.Trade, error) {
cbTrades, err := e.queryOrderTradesByPagination(ctx, q.OrderID)
cbTrades, err := e.queryOrderTradesByPagination(ctx, q)
if err != nil {
return nil, errors.Wrapf(err, "failed to get order trades: %v", q.OrderID)
}
Expand All @@ -355,15 +360,18 @@ func (e *Exchange) QueryOrderTrades(ctx context.Context, q types.OrderQuery) ([]
return trades, nil
}

func (e *Exchange) queryOrderTradesByPagination(ctx context.Context, orderID string) (api.TradeSnapshot, error) {
req := e.client.NewGetOrderTradesRequest().OrderID(orderID).Limit(PaginationLimit)
func (e *Exchange) queryOrderTradesByPagination(ctx context.Context, q types.OrderQuery) (api.TradeSnapshot, error) {
req := e.client.NewGetOrderTradesRequest().Limit(PaginationLimit)
if len(q.OrderID) > 0 {
req.OrderID(q.OrderID)
}
if len(q.Symbol) > 0 {
req.ProductID(toLocalSymbol(q.Symbol))
}
cbTrades, err := req.Do(ctx)
if err != nil {
return nil, err
}
if len(cbTrades) < PaginationLimit {
return cbTrades, nil
}

if len(cbTrades) < PaginationLimit {
return cbTrades, nil
Expand Down
188 changes: 188 additions & 0 deletions pkg/exchange/coinbase/exchange_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,188 @@
package coinbase

import (
"context"
"os"
"strconv"
"testing"
"time"

"github.com/stretchr/testify/assert"

"github.com/c9s/bbgo/pkg/fixedpoint"
"github.com/c9s/bbgo/pkg/testutil"
"github.com/c9s/bbgo/pkg/types"
)

func Test_new(t *testing.T) {
ex := getExchangeOrSkip(t)
assert.Equal(t, ex.Name(), types.ExchangeCoinBase)
t.Log("successfully created coinbase exchange client")
_ = ex.SupportedInterval()
_ = ex.PlatformFeeCurrency()
}

func Test_Symbols(t *testing.T) {
globalSymbol := "NOTEXIST"
localSymbol := toLocalSymbol(globalSymbol)
assert.Equal(t, globalSymbol, toGlobalSymbol(localSymbol))
assert.Equal(t, localSymbol, toLocalSymbol(globalSymbol))

globalSymbol = "ETHUSD"
localSymbol = toLocalSymbol(globalSymbol)
assert.Equal(t, globalSymbol, toGlobalSymbol(localSymbol))
assert.Equal(t, localSymbol, toLocalSymbol(globalSymbol))
}

func Test_OrdersAPI(t *testing.T) {
ex := getExchangeOrSkip(t)
ctx := context.Background()

// should fail on unsupported symbol
order, err := ex.SubmitOrder(
ctx,
types.SubmitOrder{
Market: types.Market{
Symbol: "NOTEXIST",
},
Side: types.SideTypeBuy,
Type: types.OrderTypeLimit,
Price: fixedpoint.MustNewFromString("0.001"),
Quantity: fixedpoint.MustNewFromString("0.001"),
})
assert.Error(t, err)
assert.Empty(t, order)
// should succeed
order, err = ex.SubmitOrder(
ctx,
types.SubmitOrder{
Market: types.Market{
Symbol: "ETHUSD",
},
Side: types.SideTypeBuy,
Type: types.OrderTypeLimit,
Price: fixedpoint.MustNewFromString("0.01"),
Quantity: fixedpoint.MustNewFromString("100"), // min funds is $1
})
assert.NoError(t, err)
assert.NotEmpty(t, order)

// test query open orders
order, err = ex.QueryOrder(ctx, types.OrderQuery{Symbol: "ETHUSD", OrderID: order.UUID, ClientOrderID: order.UUID})
assert.NoError(t, err)

orders, err := ex.QueryOpenOrders(ctx, "ETHUSD")
assert.NoError(t, err)
found := false
for _, o := range orders {
if o.UUID == order.UUID {
found = true
break
}
}
assert.True(t, found)

// test cancel order
err = ex.CancelOrders(ctx, types.Order{
Exchange: types.ExchangeCoinBase,
UUID: order.UUID,
})
assert.NoError(t, err)
}

func Test_QueryAccount(t *testing.T) {
ex := getExchangeOrSkip(t)
ctx := context.Background()
_, err := ex.QueryAccount(ctx)
assert.NoError(t, err)
}

func Test_QueryAccountBalances(t *testing.T) {
ex := getExchangeOrSkip(t)
ctx := context.Background()
_, err := ex.QueryAccountBalances(ctx)
assert.NoError(t, err)
}

func Test_QueryOpenOrders(t *testing.T) {
ex := getExchangeOrSkip(t)
ctx := context.Background()

symbols := []string{"BTCUSD", "ETHUSD", "ETHBTC"}
for _, k := range symbols {
_, err := ex.QueryOpenOrders(ctx, k)
assert.NoError(t, err)
}
}

func Test_QueryMarkets(t *testing.T) {
ex := getExchangeOrSkip(t)
ctx := context.Background()
_, err := ex.QueryMarkets(ctx)
assert.NoError(t, err)
}

func Test_QueryTicker(t *testing.T) {
ex := getExchangeOrSkip(t)
ctx := context.Background()
ticker, err := ex.QueryTicker(ctx, "BTCUSD")
assert.NoError(t, err)
assert.NotNil(t, ticker)
}

func Test_QueryTickers(t *testing.T) {
ex := getExchangeOrSkip(t)
ctx := context.Background()
symbols := []string{"BTCUSD", "ETHUSD", "ETHBTC"}
tickers, err := ex.QueryTickers(ctx, symbols...)
assert.NoError(t, err)
assert.NotNil(t, tickers)
}

func Test_QueryKLines(t *testing.T) {
ex := getExchangeOrSkip(t)
ctx := context.Background()
// should fail on unsupported interval
_, err := ex.QueryKLines(ctx, "BTCUSD", types.Interval12h, types.KLineQueryOptions{})
assert.Error(t, err)

klines, err := ex.QueryKLines(ctx, "BTCUSD", types.Interval1m, types.KLineQueryOptions{})
assert.NoError(t, err)
assert.NotNil(t, klines)

endTime := time.Now()
startTime := endTime.Add(-time.Hour * 5)
klines, err = ex.QueryKLines(
ctx,
"BTCUSD",
types.Interval1m,
types.KLineQueryOptions{
StartTime: &startTime,
EndTime: &endTime,
},
)
assert.NoError(t, err)
assert.NotNil(t, klines)
}

func Test_QueryOrderTrades(t *testing.T) {
ex := getExchangeOrSkip(t)
ctx := context.Background()

trades, err := ex.QueryOrderTrades(ctx, types.OrderQuery{Symbol: "ETHUSD"})
assert.NoError(t, err)
assert.NotNil(t, trades)
}

func getExchangeOrSkip(t *testing.T) *Exchange {
if b, _ := strconv.ParseBool(os.Getenv("CI")); b {
t.Skip("skip test for CI")
}
key, secret, passphrase, ok := testutil.IntegrationTestWithPassphraseConfigured(t, "COINBASE")
if !ok {
t.SkipNow()
return nil
}

return New(key, secret, passphrase, 0)
}
Loading