From fec23879e357470a671d2e180e4c5cc64d16c15b Mon Sep 17 00:00:00 2001 From: Gareth Kirwan Date: Sat, 23 Nov 2024 16:44:57 +0700 Subject: [PATCH] Assets: Fix False positives with IsValid --- exchanges/asset/asset.go | 37 ++++++++-------- exchanges/asset/asset_test.go | 81 ++++++++++++++--------------------- 2 files changed, 50 insertions(+), 68 deletions(-) diff --git a/exchanges/asset/asset.go b/exchanges/asset/asset.go index aa03ed4e16c..a40a8c49721 100644 --- a/exchanges/asset/asset.go +++ b/exchanges/asset/asset.go @@ -29,7 +29,7 @@ const ( MarginFunding Index Binary - PerpetualContract + PerpetualContract // All Futures must come after this PerpetualSwap Futures DeliveryFutures @@ -38,21 +38,17 @@ const ( CoinMarginedFutures USDTMarginedFutures USDCMarginedFutures - Options - OptionCombo FutureCombo LinearContract // Derivatives with a linear Base (e.g. USDT or USDC) - All // Must come immediately after all valid assets + Options // All Options must come after this + OptionCombo + 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 - spot = "spot" margin = "margin" - crossMargin = "cross_margin" // for Gateio exchange + crossMargin = "cross_margin" marginFunding = "marginfunding" index = "index" binary = "binary" @@ -160,7 +156,18 @@ 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 + // Not 0, and exactly one bit set + return a != 0 && (a&(a-1)) == 0 +} + +// IsFutures checks if the asset type is a futures contract based asset +func (a Item) IsFutures() bool { + return a != 0 && (a&(a-1)) == 0 && a >= PerpetualContract && a < Options +} + +// IsOptions checks if the asset type is options contract based asset +func (a Item) IsOptions() bool { + return a != 0 && (a&(a-1)) == 0 && a >= Options && a < All } // UnmarshalJSON conforms type to the umarshaler interface @@ -242,13 +249,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..14091c3d768 100644 --- a/exchanges/asset/asset_test.go +++ b/exchanges/asset/asset_test.go @@ -3,9 +3,11 @@ package asset import ( "encoding/json" "errors" + "slices" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestString(t *testing.T) { @@ -53,12 +55,39 @@ func TestJoinToString(t *testing.T) { func TestIsValid(t *testing.T) { t.Parallel() - if Item(0).IsValid() { - t.Fatal("TestIsValid returned an unexpected result") + for i := range All { + a := Item(i) + if a.String() == "" { + require.Falsef(t, a.IsValid(), "IsValid must return false with non-asset value %d", i) + } else { + require.Truef(t, a.IsValid(), "IsValid must return true for %s", a) + } } +} - 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 i := range All { + a := Item(i) + 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)", i, a) + } + } +} + +func TestIsOptions(t *testing.T) { + t.Parallel() + valid := []Item{Options, OptionCombo} + for i := range All { + a := Item(i) + 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)", i, a) + } } } @@ -117,50 +146,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))