From 859c4512fb06d654a31304dee82581a1ef83feff Mon Sep 17 00:00:00 2001 From: Bea <103050835+Beadko@users.noreply.github.com> Date: Fri, 13 Oct 2023 11:34:21 +0700 Subject: [PATCH] Bitstamp: Add new trading fees API endpoint; refine fee calcuations and test coverage (#1289) * Bitstamp: WIP fixing trading fees * Bitstamp/amended the TestGetFee test and currected the getTradingFee function * Bitstamp: TestGetFee implemented for maker and taker * Bitfinex: added a wrapped error * Bitstamp: GetTradingFee and test updated, fetched from the API Progresses #1271 * Bitstamp: minor changes- whitespace removal and comment added * Bitstamp: fixed lint issues in TestGetBalance and TestGetTransactions * Bitstamp:Typo in types TradingFee comment * Bitstamp: GetAccountTradingFees at view the fees on all pairs modified: exchanges/bitstamp/bitstamp.go * Bitstamp: returning the TradingFee info instead of just MakerTakerFees * Bitstamp:TestGetAccountTradingFees + TestGetAccountTradingFees added,data structure amended to match the outcome * Bitstamp:error and a test for empty pair added in GetAccountTradingFee * Bitstamp: RM the whitespace linter error --- exchanges/bitstamp/bitstamp.go | 77 ++++++++++++----------- exchanges/bitstamp/bitstamp_live_test.go | 12 +++- exchanges/bitstamp/bitstamp_test.go | 74 ++++++++++++++-------- exchanges/bitstamp/bitstamp_types.go | 15 ++++- testdata/http_mock/bitstamp/bitstamp.json | 50 +++++++++++++++ 5 files changed, 159 insertions(+), 69 deletions(-) diff --git a/exchanges/bitstamp/bitstamp.go b/exchanges/bitstamp/bitstamp.go index e30f98e3..c279bed0 100644 --- a/exchanges/bitstamp/bitstamp.go +++ b/exchanges/bitstamp/bitstamp.go @@ -29,6 +29,7 @@ const ( bitstampAPIOrderbook = "order_book" bitstampAPITransactions = "transactions" bitstampAPIEURUSD = "eur_usd" + bitstampAPITradingFees = "fees/trading" bitstampAPIBalance = "balance" bitstampAPIUserTransactions = "user_transactions" bitstampAPIOHLC = "ohlc" @@ -67,15 +68,11 @@ func (b *Bitstamp) GetFee(ctx context.Context, feeBuilder *exchange.FeeBuilder) switch feeBuilder.FeeType { case exchange.CryptocurrencyTradeFee: - balance, err := b.GetBalance(ctx) + tradingFee, err := b.getTradingFee(ctx, feeBuilder) if err != nil { - return 0, err + return 0, fmt.Errorf("error getting trading fee: %w", err) } - fee = b.CalculateTradingFee(feeBuilder.Pair.Base, - feeBuilder.Pair.Quote, - feeBuilder.PurchasePrice, - feeBuilder.Amount, - balance) + fee = tradingFee case exchange.CryptocurrencyDepositFee: fee = 0 case exchange.InternationalBankDepositFee: @@ -91,6 +88,41 @@ func (b *Bitstamp) GetFee(ctx context.Context, feeBuilder *exchange.FeeBuilder) return fee, nil } +// GetTradingFee returns a trading fee based on a currency +func (b *Bitstamp) getTradingFee(ctx context.Context, feeBuilder *exchange.FeeBuilder) (float64, error) { + tradingFees, err := b.GetAccountTradingFee(ctx, feeBuilder.Pair) + if err != nil { + return 0, err + } + fees := tradingFees.Fees + fee := fees.Taker + if feeBuilder.IsMaker { + fee = fees.Maker + } + return fee / 100 * feeBuilder.PurchasePrice * feeBuilder.Amount, nil +} + +// GetAccountTradingFee returns a TradingFee for a pair +func (b *Bitstamp) GetAccountTradingFee(ctx context.Context, pair currency.Pair) (TradingFees, error) { + path := bitstampAPITradingFees + "/" + strings.ToLower(pair.String()) + + var resp TradingFees + if pair.IsEmpty() { + return resp, currency.ErrCurrencyPairEmpty + } + err := b.SendAuthenticatedHTTPRequest(ctx, exchange.RestSpot, path, true, nil, &resp) + + return resp, err +} + +// GetAccountTradingFees returns a slice of TradingFee +func (b *Bitstamp) GetAccountTradingFees(ctx context.Context) ([]TradingFees, error) { + path := bitstampAPITradingFees + var resp []TradingFees + err := b.SendAuthenticatedHTTPRequest(ctx, exchange.RestSpot, path, true, nil, &resp) + return resp, err +} + // getOfflineTradeFee calculates the worst case-scenario trading fee func getOfflineTradeFee(price, amount float64) float64 { return 0.0025 * price * amount @@ -119,22 +151,6 @@ func getInternationalBankDepositFee(amount float64) float64 { return fee } -// CalculateTradingFee returns fee on a currency pair -func (b *Bitstamp) CalculateTradingFee(base, quote currency.Code, purchasePrice, amount float64, balances Balances) float64 { - var fee float64 - if v, ok := balances[base.String()]; ok { - switch quote { - case currency.BTC: - fee = v.BTCFee - case currency.USD: - fee = v.USDFee - case currency.EUR: - fee = v.EURFee - } - } - return fee * purchasePrice * amount -} - // GetTicker returns ticker information func (b *Bitstamp) GetTicker(ctx context.Context, currency string, hourly bool) (*Ticker, error) { response := Ticker{} @@ -251,21 +267,6 @@ func (b *Bitstamp) GetBalance(ctx context.Context) (Balances, error) { Reserved: reserved, WithdrawalFee: withdrawalFee, } - switch strings.ToUpper(curr) { - case currency.USD.String(): - eurFee, _ := strconv.ParseFloat(balance[curr+"eur_fee"], 64) - currBalance.EURFee = eurFee - case currency.EUR.String(): - usdFee, _ := strconv.ParseFloat(balance[curr+"usd_fee"], 64) - currBalance.USDFee = usdFee - default: - btcFee, _ := strconv.ParseFloat(balance[curr+"btc_fee"], 64) - currBalance.BTCFee = btcFee - eurFee, _ := strconv.ParseFloat(balance[curr+"eur_fee"], 64) - currBalance.EURFee = eurFee - usdFee, _ := strconv.ParseFloat(balance[curr+"usd_fee"], 64) - currBalance.USDFee = usdFee - } balances[strings.ToUpper(curr)] = currBalance } return balances, nil diff --git a/exchanges/bitstamp/bitstamp_live_test.go b/exchanges/bitstamp/bitstamp_live_test.go index b13a2b34..1623c82a 100644 --- a/exchanges/bitstamp/bitstamp_live_test.go +++ b/exchanges/bitstamp/bitstamp_live_test.go @@ -26,9 +26,15 @@ func TestMain(m *testing.M) { log.Fatal("Bitstamp Setup() init error", err) } bitstampConfig.API.AuthenticatedSupport = true - bitstampConfig.API.Credentials.Key = apiKey - bitstampConfig.API.Credentials.Secret = apiSecret - bitstampConfig.API.Credentials.ClientID = customerID + if apiKey != "" { + bitstampConfig.API.Credentials.Key = apiKey + } + if apiSecret != "" { + bitstampConfig.API.Credentials.Secret = apiSecret + } + if customerID != "" { + bitstampConfig.API.Credentials.ClientID = customerID + } b.SetDefaults() b.Websocket = sharedtestvalues.NewTestWebsocket() err = b.Setup(bitstampConfig) diff --git a/exchanges/bitstamp/bitstamp_test.go b/exchanges/bitstamp/bitstamp_test.go index 70e717bb..6234d53c 100644 --- a/exchanges/bitstamp/bitstamp_test.go +++ b/exchanges/bitstamp/bitstamp_test.go @@ -33,10 +33,10 @@ var b = &Bitstamp{} func setFeeBuilder() *exchange.FeeBuilder { return &exchange.FeeBuilder{ - Amount: 1, + Amount: 5, FeeType: exchange.CryptocurrencyTradeFee, - Pair: currency.NewPair(currency.BTC, currency.LTC), - PurchasePrice: 1, + Pair: currency.NewPair(currency.LTC, currency.BTC), + PurchasePrice: 1800, } } @@ -99,8 +99,21 @@ func TestGetFee(t *testing.T) { // CryptocurrencyTradeFee IsMaker feeBuilder = setFeeBuilder() feeBuilder.IsMaker = true - if _, err := b.GetFee(context.Background(), feeBuilder); err != nil { + fee, err := b.GetFee(context.Background(), feeBuilder) + if err != nil { t.Error(err) + } else if expected := 0.003 * feeBuilder.PurchasePrice * feeBuilder.Amount; fee != expected { + t.Errorf("Bitstamp GetFee wrong Maker fee; Pair: %s Expected: %v Got: %v", feeBuilder.Pair, expected, fee) + } + + // CryptocurrencyTradeFee IsTaker + feeBuilder = setFeeBuilder() + feeBuilder.IsMaker = false + fee, err = b.GetFee(context.Background(), feeBuilder) + if err != nil { + t.Error(err) + } else if expected := 0.002 * feeBuilder.PurchasePrice * feeBuilder.Amount; fee != expected { + t.Errorf("Bitstamp GetFee wrong Taker fee; Pair: %s Expected: %v Got: %v", feeBuilder.Pair, expected, fee) } // CryptocurrencyTradeFee Negative purchase price @@ -141,28 +154,44 @@ func TestGetFee(t *testing.T) { } } -func TestCalculateTradingFee(t *testing.T) { +func TestGetAccountTradingFee(t *testing.T) { t.Parallel() - newBalance := make(Balances) - newBalance["BTC"] = Balance{ - USDFee: 1, - EURFee: 0, + if !mockTests { + sharedtestvalues.SkipTestIfCredentialsUnset(t, b) } - if resp := b.CalculateTradingFee(currency.BTC, currency.USD, 0, 0, newBalance); resp != 0 { - t.Error("GetFee() error") - } - if resp := b.CalculateTradingFee(currency.BTC, currency.USD, 2, 2, newBalance); resp != float64(4) { - t.Errorf("GetFee() error. Expected: %f, Received: %f", float64(4), resp) - } - if resp := b.CalculateTradingFee(currency.BTC, currency.EUR, 2, 2, newBalance); resp != float64(0) { - t.Errorf("GetFee() error. Expected: %f, Received: %f", float64(0), resp) + fee, err := b.GetAccountTradingFee(context.Background(), currency.NewPair(currency.LTC, currency.BTC)) + if assert.NoError(t, err, "GetAccountTradingFee should not error") { + if mockTests { + assert.Positive(t, fee.Fees.Maker, "Maker should be positive") + assert.Positive(t, fee.Fees.Taker, "Taker should be positive") + } + assert.NotEmpty(t, fee.Symbol, "Symbol should not be empty") + assert.Equal(t, fee.Symbol, "ltcbtc", "Symbol should be correct") } - dummy1, dummy2 := currency.NewCode(""), currency.NewCode("") - if resp := b.CalculateTradingFee(dummy1, dummy2, 0, 0, newBalance); resp != 0 { - t.Error("GetFee() error") + _, err = b.GetAccountTradingFee(context.Background(), currency.EMPTYPAIR) + assert.ErrorIs(t, err, currency.ErrCurrencyPairEmpty, "Should get back the right error") +} + +func TestGetAccountTradingFees(t *testing.T) { + t.Parallel() + + if !mockTests { + sharedtestvalues.SkipTestIfCredentialsUnset(t, b) + } + + fees, err := b.GetAccountTradingFees(context.Background()) + if assert.NoError(t, err, "GetAccountTradingFee should not error") { + if assert.Positive(t, len(fees), "Should get back multiple fees") { + fee := fees[0] + assert.NotEmpty(t, fee.Symbol, "Should get back a symbol") + if mockTests { + assert.Positive(t, fee.Fees.Maker, "Maker should be positive") + assert.Positive(t, fee.Fees.Taker, "Taker should be positive") + } + } } } @@ -287,14 +316,12 @@ func TestGetBalance(t *testing.T) { Balance: 1337.42, Reserved: 1295.00, WithdrawalFee: 5.0, - USDFee: 0, }, "BTC": { Available: 9.1, Balance: 11.2, Reserved: 2.1, WithdrawalFee: 0.00050000, - USDFee: 0.25, }, } { if got, ok := bal[k]; !ok { @@ -312,9 +339,6 @@ func TestGetBalance(t *testing.T) { if got.WithdrawalFee != e.WithdrawalFee { t.Errorf("Incorrect WithdrawalFee for %s; Expected: %v Got: %v", k, e.WithdrawalFee, got.WithdrawalFee) } - if got.USDFee != e.USDFee { - t.Errorf("Incorrect USDFee for %s; Expected: %v Got: %v", k, e.USDFee, got.USDFee) - } } } } diff --git a/exchanges/bitstamp/bitstamp_types.go b/exchanges/bitstamp/bitstamp_types.go index a544e9e6..723c0394 100644 --- a/exchanges/bitstamp/bitstamp_types.go +++ b/exchanges/bitstamp/bitstamp_types.go @@ -75,15 +75,24 @@ type EURUSDConversionRate struct { Sell float64 `json:"sell,string"` } +// TradingFees holds trading fee information +type TradingFees struct { + Symbol string `json:"currency_pair"` + Fees MakerTakerFees `json:"fees"` +} + +// MakerTakerFees holds maker and taker fee information +type MakerTakerFees struct { + Maker float64 `json:"maker,string"` + Taker float64 `json:"taker,string"` +} + // Balance stores the balance info type Balance struct { Available float64 Balance float64 Reserved float64 WithdrawalFee float64 - BTCFee float64 // for cryptocurrency pairs - USDFee float64 - EURFee float64 } // Balances holds full balance information with the supplied APIKEYS diff --git a/testdata/http_mock/bitstamp/bitstamp.json b/testdata/http_mock/bitstamp/bitstamp.json index 373a21bd..86f8879f 100644 --- a/testdata/http_mock/bitstamp/bitstamp.json +++ b/testdata/http_mock/bitstamp/bitstamp.json @@ -148,6 +148,56 @@ } ] }, + "/api/v2/fees/trading/ltcbtc/": { + "POST": [ + { + "data": { + "currency_pair": "ltcbtc", + "fees": + { + "maker": "0.3", + "taker": "0.2" + } + }, + "queryString": "", + "bodyParams": "key=\u0026nonce=1690610275327495000\u0026signature=B619E1AEF317B95D3BB28025F36EEF94CABCDE425CE3E13E8A1861C0A9777F2A", + "headers": { + "Content-Type": [ + "application/x-www-form-urlencoded" + ] + } + } + ] +}, +"/api/v2/fees/trading/": { + "POST": [ + { + "data": [ + { + "currency_pair":"ltcbtc", + "fees": { + "maker": "0.3", + "taker": "0.2" + } + }, + { + "currency_pair":"btcusd", + "fees": { + "maker": "0.3", + "taker": "0.2" + } + } + ], + "queryString": "", + "bodyParams": "key=&nonce=1696325404980771000&signature=85B2B7E60567E352241BD98BDD08C0A435C2A77CB8F386739DD471BFFFA6C660", + "headers": { + "Content-Type": [ + "application/x-www-form-urlencoded" + ] + } + } + ] + }, "/api/v2/buy/market/btcusd/": { "POST": [ {