bybit: Add protected subtype for account checking to reduce outbound requests (#1739)

* bybit: Add protected sub type for account checking to reduce outbound requests

* add type and string method plus fix linter with comment

* linter: fix

* whoops

* Update exchanges/bybit/bybit.go

Co-authored-by: Scott <gloriousCode@users.noreply.github.com>

* Update exchanges/bybit/bybit.go

Co-authored-by: Scott <gloriousCode@users.noreply.github.com>

* glorious: nits

---------

Co-authored-by: Ryan O'Hara-Reid <ryan.oharareid@thrasher.io>
Co-authored-by: Scott <gloriousCode@users.noreply.github.com>

exchanges/bybit/bybit.go

@@ -27,10 +27,7 @@ import (
 // Bybit is the overarching type across this package
 type Bybit struct {
 	exchange.Base
-
-	// AccountType holds information about whether the account to which the api key belongs is a unified margin account or not.
-	// 0: unified, and 1: for normal account
-	AccountType int64
+	account accountTypeHolder
 }
 
 const (
@@ -45,8 +42,8 @@ const (
 
 	cSpot, cLinear, cOption, cInverse = "spot", "linear", "option", "inverse"
 
-	accountTypeNormal  = 0 // 0: regular account
-	accountTypeUnified = 1 // 1: unified trade account
+	accountTypeNormal  AccountType = 1
+	accountTypeUnified AccountType = 2
 
 	longDatedFormat = "02Jan06"
 )
@@ -1176,8 +1173,9 @@ func (by *Bybit) GetPreUpgradeOrderHistory(ctx context.Context, category, symbol
 	if err != nil {
 		return nil, err
 	}
-	if by.AccountType == accountTypeNormal {
-		return nil, errAPIKeyIsNotUnified
+	err = by.RequiresUnifiedAccount(ctx)
+	if err != nil {
+		return nil, err
 	}
 	var resp *TradeOrders
 	return resp, by.SendAuthHTTPRequestV5(ctx, exchange.RestSpot, http.MethodGet, "/v5/pre-upgrade/order/history", params, nil, &resp, defaultEPL)
@@ -1189,8 +1187,9 @@ func (by *Bybit) GetPreUpgradeTradeHistory(ctx context.Context, category, symbol
 	if err != nil {
 		return nil, err
 	}
-	if by.AccountType == accountTypeNormal {
-		return nil, errAPIKeyIsNotUnified
+	err = by.RequiresUnifiedAccount(ctx)
+	if err != nil {
+		return nil, err
 	}
 	if executionType != "" {
 		params.Set("executionType", executionType)
@@ -1205,8 +1204,9 @@ func (by *Bybit) GetPreUpgradeClosedPnL(ctx context.Context, category, symbol, c
 	if err != nil {
 		return nil, err
 	}
-	if by.AccountType == accountTypeNormal {
-		return nil, errAPIKeyIsNotUnified
+	err = by.RequiresUnifiedAccount(ctx)
+	if err != nil {
+		return nil, err
 	}
 	var resp *ClosedProfitAndLossResponse
 	return resp, by.SendAuthHTTPRequestV5(ctx, exchange.RestSpot, http.MethodGet, "/v5/pre-upgrade/position/closed-pnl", params, nil, &resp, defaultEPL)
@@ -1218,8 +1218,9 @@ func (by *Bybit) GetPreUpgradeTransactionLog(ctx context.Context, category, base
 	if err != nil {
 		return nil, err
 	}
-	if by.AccountType == accountTypeNormal {
-		return nil, errAPIKeyIsNotUnified
+	err = by.RequiresUnifiedAccount(ctx)
+	if err != nil {
+		return nil, err
 	}
 	if transactionType != "" {
 		params.Set("type", transactionType)
@@ -1234,8 +1235,9 @@ func (by *Bybit) GetPreUpgradeOptionDeliveryRecord(ctx context.Context, category
 	if err != nil {
 		return nil, err
 	}
-	if by.AccountType == accountTypeNormal {
-		return nil, errAPIKeyIsNotUnified
+	err = by.RequiresUnifiedAccount(ctx)
+	if err != nil {
+		return nil, err
 	}
 	if !expiryDate.IsZero() {
 		params.Set("expData", expiryDate.Format(longDatedFormat))
@@ -1250,8 +1252,9 @@ func (by *Bybit) GetPreUpgradeUSDCSessionSettlement(ctx context.Context, categor
 	if err != nil {
 		return nil, err
 	}
-	if by.AccountType == accountTypeNormal {
-		return nil, errAPIKeyIsNotUnified
+	err = by.RequiresUnifiedAccount(ctx)
+	if err != nil {
+		return nil, err
 	}
 	var resp *SettlementSession
 	return resp, by.SendAuthHTTPRequestV5(ctx, exchange.RestSpot, http.MethodGet, "/v5/pre-upgrade/asset/settlement-record", params, nil, &resp, defaultEPL)
@@ -2699,13 +2702,31 @@ func getSign(sign, secret string) (string, error) {
 	return crypto.HexEncodeToString(hmacSigned), nil
 }
 
-// RetrieveAndSetAccountType retrieve to set account type information
-func (by *Bybit) RetrieveAndSetAccountType(ctx context.Context) error {
-	accInfo, err := by.GetAPIKeyInformation(ctx)
-	if err != nil {
-		return err
+// FetchAccountType if not set fetches the account type from the API, stores it and returns it. Else returns the stored account type.
+func (by *Bybit) FetchAccountType(ctx context.Context) (AccountType, error) {
+	by.account.m.Lock()
+	defer by.account.m.Unlock()
+	if by.account.accountType == 0 {
+		accInfo, err := by.GetAPIKeyInformation(ctx)
+		if err != nil {
+			return 0, err
+		}
+		// From endpoint 0：regular account; 1：unified trade account
+		// + 1 to make it 1 and 2 so that a zero value can be used to check if the account type has been set or not.
+		by.account.accountType = AccountType(accInfo.IsUnifiedTradeAccount + 1)
+	}
+	return by.account.accountType, nil
+}
+
+// RequiresUnifiedAccount checks account type and returns error if not unified
+func (by *Bybit) RequiresUnifiedAccount(ctx context.Context) error {
+	at, err := by.FetchAccountType(ctx)
+	if err != nil {
+		return nil //nolint:nilerr // if we can't get the account type, we can't check if it's unified or not, fail on call
+	}
+	if at != accountTypeUnified {
+		return fmt.Errorf("%w, account type: %s", errAPIKeyIsNotUnified, at)
 	}
-	by.AccountType = accInfo.IsUnifiedTradeAccount // 0：regular account; 1：unified trade account
 	return nil
 }
 

exchanges/bybit/bybit_live_test.go

@@ -31,8 +31,8 @@ func TestMain(m *testing.M) {
 	}
 
 	if b.API.AuthenticatedSupport {
-		if err := b.RetrieveAndSetAccountType(context.Background()); err != nil {
-			log.Printf("%s unable to RetrieveAndSetAccountType: %v", b.Name, err)
+		if _, err := b.FetchAccountType(context.Background()); err != nil {
+			log.Printf("%s unable to FetchAccountType: %v", b.Name, err)
 		}
 	}
 

exchanges/bybit/bybit_test.go

@@ -3548,12 +3548,35 @@ func TestStringToOrderStatus(t *testing.T) {
 	}
 }
 
-func TestRetrieveAndSetAccountType(t *testing.T) {
-	sharedtestvalues.SkipTestIfCredentialsUnset(t, b, canManipulateRealOrders)
-	err := b.RetrieveAndSetAccountType(context.Background())
-	if err != nil {
-		t.Fatal(err)
+func TestFetchAccountType(t *testing.T) {
+	t.Parallel()
+	if !mockTests {
+		sharedtestvalues.SkipTestIfCredentialsUnset(t, b)
 	}
+	val, err := b.FetchAccountType(context.Background())
+	require.NoError(t, err)
+	require.NotZero(t, val)
+}
+
+func TestAccountTypeString(t *testing.T) {
+	t.Parallel()
+	require.Equal(t, "unset", AccountType(0).String())
+	require.Equal(t, "unified", accountTypeUnified.String())
+	require.Equal(t, "normal", accountTypeNormal.String())
+	require.Equal(t, "unknown", AccountType(3).String())
+}
+
+func TestRequiresUnifiedAccount(t *testing.T) {
+	t.Parallel()
+	if !mockTests {
+		sharedtestvalues.SkipTestIfCredentialsUnset(t, b)
+	}
+	err := b.RequiresUnifiedAccount(context.Background())
+	require.NoError(t, err)
+	b := &Bybit{} //nolint:govet // Intentional shadow to avoid future copy/paste mistakes. Also stops race below.
+	b.account.accountType = accountTypeNormal
+	err = b.RequiresUnifiedAccount(context.Background())
+	require.ErrorIs(t, err, errAPIKeyIsNotUnified)
 }
 
 func TestGetLatestFundingRates(t *testing.T) {

exchanges/bybit/bybit_types.go

@@ -2,6 +2,7 @@ package bybit
 
 import (
 	"encoding/json"
+	"sync"
 	"time"
 
 	"github.com/gofrs/uuid"
@@ -2035,3 +2036,26 @@ type Error struct {
 	ExtCode         string `json:"ext_code"`
 	ExtMsg          string `json:"ext_info"`
 }
+
+// accountTypeHolder holds the account type associated with the loaded API key.
+type accountTypeHolder struct {
+	accountType AccountType
+	m           sync.Mutex
+}
+
+// AccountType constants
+type AccountType uint8
+
+// String returns the account type as a string
+func (a AccountType) String() string {
+	switch a {
+	case 0:
+		return "unset"
+	case accountTypeNormal:
+		return "normal"
+	case accountTypeUnified:
+		return "unified"
+	default:
+		return "unknown"
+	}
+}

exchanges/bybit/bybit_wrapper.go

@@ -562,7 +562,7 @@ func (by *Bybit) UpdateAccountInfo(ctx context.Context, assetType asset.Item) (a
 	var acc account.SubAccount
 	var accountType string
 	info.Exchange = by.Name
-	err := by.RetrieveAndSetAccountType(ctx)
+	at, err := by.FetchAccountType(ctx)
 	if err != nil {
 		return info, err
 	}
@@ -570,7 +570,7 @@ func (by *Bybit) UpdateAccountInfo(ctx context.Context, assetType asset.Item) (a
 	case asset.Spot, asset.Options,
 		asset.USDCMarginedFutures,
 		asset.USDTMarginedFutures:
-		switch by.AccountType {
+		switch at {
 		case accountTypeUnified:
 			accountType = "UNIFIED"
 		case accountTypeNormal: