From 6f955c562b02a5582823f5a45e78cb75618e8d95 Mon Sep 17 00:00:00 2001 From: Gareth Kirwan Date: Fri, 22 Nov 2024 15:13:30 +0700 Subject: [PATCH 1/2] Assets: Fix LinearContracts support --- exchanges/asset/asset.go | 15 ++++-- exchanges/asset/asset_test.go | 86 ++++------------------------------- 2 files changed, 20 insertions(+), 81 deletions(-) diff --git a/exchanges/asset/asset.go b/exchanges/asset/asset.go index e229dffeab6..aa03ed4e16c 100644 --- a/exchanges/asset/asset.go +++ b/exchanges/asset/asset.go @@ -20,10 +20,10 @@ 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 + Spot Item = 1 << (iota - 1) Margin CrossMargin MarginFunding @@ -41,9 +41,11 @@ const ( Options OptionCombo FutureCombo - LinearContract // Added to represent a USDT and USDC based linear derivatives(futures/perpetual) assets in Bybit V5 - All + LinearContract // Derivatives with a linear Base (e.g. USDT or USDC) + All // Must come immediately after all valid assets +) +const ( 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 @@ -67,6 +69,7 @@ const ( options = "options" optionCombo = "option_combo" futureCombo = "future_combo" + linearContract = "linearcontract" all = "all" ) @@ -118,6 +121,8 @@ func (a Item) String() string { return optionCombo case FutureCombo: return futureCombo + case LinearContract: + return linearContract case All: return all default: @@ -224,6 +229,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: diff --git a/exchanges/asset/asset_test.go b/exchanges/asset/asset_test.go index 2e8ba1a0577..f82129fa4d1 100644 --- a/exchanges/asset/asset_test.go +++ b/exchanges/asset/asset_test.go @@ -10,15 +10,10 @@ import ( 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 := Item(1); a <= All; a <<= 1 { + assert.NotEmptyf(t, a.String(), "%s.String should return non-empty", a) } + assert.Empty(t, Empty.String(), "Empty.String should return empty") } func TestStrings(t *testing.T) { @@ -90,8 +85,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 { @@ -123,75 +119,11 @@ 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 _, a := range []Item{Spot, Margin, MarginFunding, Index, Binary} { + assert.Falsef(t, a.IsFutures(), "%s should return correctly for IsFutures", a) } - 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) - } - }) + for _, a := range []Item{PerpetualContract, PerpetualSwap, Futures, UpsideProfitContract, DownsideProfitContract, CoinMarginedFutures, USDTMarginedFutures, USDCMarginedFutures, FutureCombo} { + assert.Truef(t, a.IsFutures(), "%s should return correctly for IsFutures", a) } } From c6f39af3df86f43b924c9ddbe1dd47396eada6c1 Mon Sep 17 00:00:00 2001 From: Gareth Kirwan Date: Sat, 23 Nov 2024 16:44:57 +0700 Subject: [PATCH 2/2] 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 | 43 +++++++++-------- exchanges/asset/asset_test.go | 88 +++++++++++++++-------------------- exchanges/order/order_test.go | 2 +- 3 files changed, 59 insertions(+), 74 deletions(-) diff --git a/exchanges/asset/asset.go b/exchanges/asset/asset.go index aa03ed4e16c..8e20e59e6dd 100644 --- a/exchanges/asset/asset.go +++ b/exchanges/asset/asset.go @@ -22,37 +22,36 @@ type Items []Item // Supported Assets const ( - Empty Item = 0 - Spot Item = 1 << (iota - 1) + 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 // Derivatives with a linear Base (e.g. USDT or USDC) - All // Must come immediately after all valid assets + // All asset const must come immediately after all valid assets for method `IsValid` + All ) const ( - 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 - spot = "spot" margin = "margin" - crossMargin = "cross_margin" // for Gateio exchange + crossMargin = "cross_margin" marginFunding = "marginfunding" index = "index" binary = "binary" @@ -160,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 @@ -242,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 f82129fa4d1..cef070415d3 100644 --- a/exchanges/asset/asset_test.go +++ b/exchanges/asset/asset_test.go @@ -3,17 +3,22 @@ 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() - for a := Item(1); a <= All; a <<= 1 { - assert.NotEmptyf(t, a.String(), "%s.String should return non-empty", a) + 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) + } } - assert.Empty(t, Empty.String(), "Empty.String should return empty") } func TestStrings(t *testing.T) { @@ -53,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") +} - if !Spot.IsValid() { - t.Fatal("TestIsValid returned an unexpected result") +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) + } + } +} + +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) + } } } @@ -117,50 +147,6 @@ func TestSupported(t *testing.T) { } } -func TestIsFutures(t *testing.T) { - t.Parallel() - for _, a := range []Item{Spot, Margin, MarginFunding, Index, Binary} { - assert.Falsef(t, a.IsFutures(), "%s should return correctly for IsFutures", a) - } - for _, a := range []Item{PerpetualContract, PerpetualSwap, Futures, UpsideProfitContract, DownsideProfitContract, CoinMarginedFutures, USDTMarginedFutures, USDCMarginedFutures, FutureCombo} { - assert.Truef(t, a.IsFutures(), "%s should return correctly for IsFutures", a) - } -} - -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 {