From fa9549d1425021c7524dbfdaebbd6690c8233539 Mon Sep 17 00:00:00 2001 From: Gareth Kirwan Date: Mon, 25 Nov 2024 05:35:25 +0100 Subject: [PATCH] Assets: Fix LinearContracts support (#1724) * Assets: Fix LinearContracts support * Assets: Fix False positives with IsValid supportedFlag&a is true for every even number, making the test pretty close to meaningless. [This fix](https://github.com/gbjk/gocryptotrader/commit/5b33bc5324e94a03de69c739ba8507e22199742c) was a viable fix maintaining bit shifted iota, but there's no benefit to it at all. Simplifying. --- exchanges/asset/asset.go | 50 ++++++----- exchanges/asset/asset_test.go | 160 +++++++++------------------------- exchanges/order/order_test.go | 2 +- 3 files changed, 68 insertions(+), 144 deletions(-) diff --git a/exchanges/asset/asset.go b/exchanges/asset/asset.go index e229dffeab6..8e20e59e6dd 100644 --- a/exchanges/asset/asset.go +++ b/exchanges/asset/asset.go @@ -20,37 +20,38 @@ type Item uint32 // Items stores a list of assets types type Items []Item -// Const vars for asset package +// Supported Assets const ( - Empty Item = 0 - Spot Item = 1 << iota + Empty Item = iota + Spot Margin CrossMargin MarginFunding Index Binary + // Futures asset consts must come below this comment for method `IsFutures` + Futures PerpetualContract PerpetualSwap - Futures DeliveryFutures UpsideProfitContract DownsideProfitContract CoinMarginedFutures USDTMarginedFutures USDCMarginedFutures + FutureCombo + LinearContract + // Options asset consts must come below this comment for method `IsOptions` Options OptionCombo - FutureCombo - LinearContract // Added to represent a USDT and USDC based linear derivatives(futures/perpetual) assets in Bybit V5 + // All asset const must come immediately after all valid assets for method `IsValid` All +) - optionsFlag = OptionCombo | Options - futuresFlag = PerpetualContract | PerpetualSwap | Futures | DeliveryFutures | UpsideProfitContract | DownsideProfitContract | CoinMarginedFutures | USDTMarginedFutures | USDCMarginedFutures | LinearContract | FutureCombo - supportedFlag = Spot | Margin | CrossMargin | MarginFunding | Index | Binary | PerpetualContract | PerpetualSwap | Futures | DeliveryFutures | UpsideProfitContract | DownsideProfitContract | CoinMarginedFutures | USDTMarginedFutures | USDCMarginedFutures | Options | LinearContract | OptionCombo | FutureCombo - +const ( spot = "spot" margin = "margin" - crossMargin = "cross_margin" // for Gateio exchange + crossMargin = "cross_margin" marginFunding = "marginfunding" index = "index" binary = "binary" @@ -67,6 +68,7 @@ const ( options = "options" optionCombo = "option_combo" futureCombo = "future_combo" + linearContract = "linearcontract" all = "all" ) @@ -118,6 +120,8 @@ func (a Item) String() string { return optionCombo case FutureCombo: return futureCombo + case LinearContract: + return linearContract case All: return all default: @@ -155,7 +159,17 @@ func (a Items) JoinToString(separator string) string { // IsValid returns whether or not the supplied asset type is valid or not func (a Item) IsValid() bool { - return a != Empty && supportedFlag&a == a + return a > Empty && a < All +} + +// IsFutures checks if the asset type is a futures contract based asset +func (a Item) IsFutures() bool { + return a >= Futures && a < Options +} + +// IsOptions checks if the asset type is options contract based asset +func (a Item) IsOptions() bool { + return a >= Options && a < All } // UnmarshalJSON conforms type to the umarshaler interface @@ -224,6 +238,8 @@ func New(input string) (Item, error) { return OptionCombo, nil case futureCombo: return FutureCombo, nil + case linearContract: + return LinearContract, nil case all: return All, nil default: @@ -235,13 +251,3 @@ func New(input string) (Item, error) { func UseDefault() Item { return Spot } - -// IsFutures checks if the asset type is a futures contract based asset -func (a Item) IsFutures() bool { - return a != Empty && futuresFlag&a == a -} - -// IsOptions checks if the asset type is options contract based asset -func (a Item) IsOptions() bool { - return a != Empty && optionsFlag&a == a -} diff --git a/exchanges/asset/asset_test.go b/exchanges/asset/asset_test.go index 2e8ba1a0577..cef070415d3 100644 --- a/exchanges/asset/asset_test.go +++ b/exchanges/asset/asset_test.go @@ -3,21 +3,21 @@ package asset import ( "encoding/json" "errors" + "slices" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestString(t *testing.T) { t.Parallel() - a := Spot - if a.String() != "spot" { - t.Fatal("TestString returned an unexpected result") - } - - a = 0 - if a.String() != "" { - t.Fatal("TestString returned an unexpected result") + for a := range All { + if a == 0 { + assert.Empty(t, a.String(), "Empty.String should return empty") + } else { + assert.NotEmptyf(t, a.String(), "%s.String should return empty", a) + } } } @@ -58,12 +58,37 @@ func TestJoinToString(t *testing.T) { func TestIsValid(t *testing.T) { t.Parallel() - if Item(0).IsValid() { - t.Fatal("TestIsValid returned an unexpected result") + for a := range All { + if a.String() == "" { + require.Falsef(t, a.IsValid(), "IsValid must return false with non-asset value %d", a) + } else { + require.Truef(t, a.IsValid(), "IsValid must return true for %s", a) + } + } + require.False(t, All.IsValid(), "IsValid must return false for All") +} + +func TestIsFutures(t *testing.T) { + t.Parallel() + valid := []Item{PerpetualContract, PerpetualSwap, Futures, DeliveryFutures, UpsideProfitContract, DownsideProfitContract, CoinMarginedFutures, USDTMarginedFutures, USDCMarginedFutures, FutureCombo, LinearContract} + for a := range All { + if slices.Contains(valid, a) { + require.Truef(t, a.IsFutures(), "IsFutures must return true for %s", a) + } else { + require.Falsef(t, a.IsFutures(), "IsFutures must return false for non-asset value %d (%s)", a, a) + } } +} - if !Spot.IsValid() { - t.Fatal("TestIsValid returned an unexpected result") +func TestIsOptions(t *testing.T) { + t.Parallel() + valid := []Item{Options, OptionCombo} + for a := range All { + if slices.Contains(valid, a) { + require.Truef(t, a.IsOptions(), "IsOptions must return true for %s", a) + } else { + require.Falsef(t, a.IsOptions(), "IsOptions must return false for non-asset value %d (%s)", a, a) + } } } @@ -90,8 +115,9 @@ func TestNew(t *testing.T) { {Input: "Options", Expected: Options}, {Input: "Option", Expected: Options}, {Input: "Future", Error: ErrNotSupported}, - {Input: "future_combo", Expected: FutureCombo}, {Input: "option_combo", Expected: OptionCombo}, + {Input: "future_combo", Expected: FutureCombo}, + {Input: "linearContract", Expected: LinearContract}, } for _, tt := range cases { @@ -121,114 +147,6 @@ func TestSupported(t *testing.T) { } } -func TestIsFutures(t *testing.T) { - t.Parallel() - type scenario struct { - item Item - isFutures bool - } - scenarios := []scenario{ - { - item: Spot, - isFutures: false, - }, - { - item: Margin, - isFutures: false, - }, - { - item: MarginFunding, - isFutures: false, - }, - { - item: Index, - isFutures: false, - }, - { - item: Binary, - isFutures: false, - }, - { - item: PerpetualContract, - isFutures: true, - }, - { - item: PerpetualSwap, - isFutures: true, - }, - { - item: Futures, - isFutures: true, - }, - { - item: UpsideProfitContract, - isFutures: true, - }, - { - item: DownsideProfitContract, - isFutures: true, - }, - { - item: CoinMarginedFutures, - isFutures: true, - }, - { - item: USDTMarginedFutures, - isFutures: true, - }, - { - item: USDCMarginedFutures, - isFutures: true, - }, { - item: FutureCombo, - isFutures: true, - }, - } - for _, s := range scenarios { - testScenario := s - t.Run(testScenario.item.String(), func(t *testing.T) { - t.Parallel() - if testScenario.item.IsFutures() != testScenario.isFutures { - t.Errorf("expected %v isFutures to be %v", testScenario.item, testScenario.isFutures) - } - }) - } -} - -func TestIsOptions(t *testing.T) { - t.Parallel() - type scenario struct { - item Item - isOptions bool - } - scenarios := []scenario{ - { - item: Options, - isOptions: true, - }, { - item: OptionCombo, - isOptions: true, - }, - { - item: Futures, - isOptions: false, - }, - { - item: Empty, - isOptions: false, - }, - } - for _, s := range scenarios { - testScenario := s - t.Run(testScenario.item.String(), func(t *testing.T) { - t.Parallel() - if testScenario.item.IsOptions() != testScenario.isOptions { - t.Errorf("expected %v isOptions to be %v", testScenario.item, testScenario.isOptions) - } - }) - } -} - func TestUnmarshalMarshal(t *testing.T) { t.Parallel() data, err := json.Marshal(Item(0)) diff --git a/exchanges/order/order_test.go b/exchanges/order/order_test.go index 04c0258d44a..2f2b8190595 100644 --- a/exchanges/order/order_test.go +++ b/exchanges/order/order_test.go @@ -55,7 +55,7 @@ func TestSubmit_Validate(t *testing.T) { Submit: &Submit{ Exchange: "test", Pair: testPair, - AssetType: 255, + AssetType: asset.All, }, }, // valid pair but invalid asset {