From 6baf1f78aa3ae60038fa599cdf3c9e1e3f4b5a3d Mon Sep 17 00:00:00 2001 From: dlipicar Date: Mon, 21 Oct 2024 08:57:28 -0300 Subject: [PATCH] fix(wallet)_: limit max parameter length in cryptocompare price fetches (#5957) * fix(wallet)_: limit max parameter length in cryptocompare price fetches * chore(wallet)_: separate package for network-dependant unit tests --- Makefile | 27 ++++-- .../wallet/thirdparty/cryptocompare/client.go | 22 ++++- services/wallet/thirdparty/utils/symbols.go | 55 +++++++----- .../wallet/thirdparty/utils/symbols_test.go | 61 ++++++++++++++ tests-unit-network/README.MD | 12 +++ .../cryptocompare/cryptocompare_test.go | 84 +++++++++++++++++++ 6 files changed, 231 insertions(+), 30 deletions(-) create mode 100644 services/wallet/thirdparty/utils/symbols_test.go create mode 100644 tests-unit-network/README.MD create mode 100644 tests-unit-network/cryptocompare/cryptocompare_test.go diff --git a/Makefile b/Makefile index 9ebd422ee..42ce54518 100644 --- a/Makefile +++ b/Makefile @@ -362,23 +362,32 @@ docker-test: ##@tests Run tests in a docker container with golang. test: test-unit ##@tests Run basic, short tests during development -test-unit: generate -test-unit: export BUILD_TAGS ?= -test-unit: export UNIT_TEST_DRY_RUN ?= false -test-unit: export UNIT_TEST_COUNT ?= 1 -test-unit: export UNIT_TEST_FAILFAST ?= true +test-unit-prep: generate +test-unit-prep: export BUILD_TAGS ?= +test-unit-prep: export UNIT_TEST_DRY_RUN ?= false +test-unit-prep: export UNIT_TEST_COUNT ?= 1 +test-unit-prep: export UNIT_TEST_FAILFAST ?= true +test-unit-prep: export UNIT_TEST_USE_DEVELOPMENT_LOGGER ?= true +test-unit-prep: export UNIT_TEST_REPORT_CODECLIMATE ?= false +test-unit-prep: export UNIT_TEST_REPORT_CODECOV ?= false + +test-unit: test-unit-prep test-unit: export UNIT_TEST_RERUN_FAILS ?= true -test-unit: export UNIT_TEST_USE_DEVELOPMENT_LOGGER ?= true -test-unit: export UNIT_TEST_REPORT_CODECLIMATE ?= false -test-unit: export UNIT_TEST_REPORT_CODECOV ?= false test-unit: export UNIT_TEST_PACKAGES ?= $(call sh, go list ./... | \ grep -v /vendor | \ grep -v /t/e2e | \ grep -v /t/benchmarks | \ - grep -v /transactions/fake) + grep -v /transactions/fake | \ + grep -v /tests-unit-network) test-unit: ##@tests Run unit and integration tests ./_assets/scripts/run_unit_tests.sh +test-unit-network: test-unit-prep +test-unit-network: export UNIT_TEST_RERUN_FAILS ?= false +test-unit-network: export UNIT_TEST_PACKAGES ?= $(call sh, go list ./tests-unit-network/...) +test-unit-network: ##@tests Run unit and integration tests with network access + ./_assets/scripts/run_unit_tests.sh + test-unit-race: export GOTEST_EXTRAFLAGS=-race test-unit-race: test-unit ##@tests Run unit and integration tests with -race flag diff --git a/services/wallet/thirdparty/cryptocompare/client.go b/services/wallet/thirdparty/cryptocompare/client.go index 892a9bc1e..80163de33 100644 --- a/services/wallet/thirdparty/cryptocompare/client.go +++ b/services/wallet/thirdparty/cryptocompare/client.go @@ -73,7 +73,15 @@ func NewClientWithParams(params Params) *Client { } func (c *Client) FetchPrices(symbols []string, currencies []string) (map[string]map[string]float64, error) { - chunks := utils.ChunkSymbols(symbols, 60) + const maxFsymsLength = 300 + chunkSymbolParams := utils.ChunkSymbolsParams{ + MaxCharsPerChunk: maxFsymsLength, + ExtraCharsPerSymbol: 1, // joined with a comma + } + chunks, err := utils.ChunkSymbols(symbols, chunkSymbolParams) + if err != nil { + return nil, err + } result := make(map[string]map[string]float64) realCurrencies := utils.RenameSymbols(currencies) for _, smbls := range chunks { @@ -82,6 +90,7 @@ func (c *Client) FetchPrices(symbols []string, currencies []string) (map[string] params := url.Values{} params.Add("fsyms", strings.Join(realSymbols, ",")) params.Add("tsyms", strings.Join(realCurrencies, ",")) + params.Add("relaxedValidation", "true") params.Add("extraParams", extraParamStatus) url := fmt.Sprintf("%s/data/pricemulti", c.baseURL) @@ -129,7 +138,15 @@ func (c *Client) FetchTokenDetails(symbols []string) (map[string]thirdparty.Toke } func (c *Client) FetchTokenMarketValues(symbols []string, currency string) (map[string]thirdparty.TokenMarketValues, error) { - chunks := utils.ChunkSymbols(symbols) + const maxFsymsLength = 300 + chunkSymbolParams := utils.ChunkSymbolsParams{ + MaxCharsPerChunk: maxFsymsLength, + ExtraCharsPerSymbol: 1, // joined with a comma + } + chunks, err := utils.ChunkSymbols(symbols, chunkSymbolParams) + if err != nil { + return nil, err + } realCurrency := utils.GetRealSymbol(currency) item := map[string]thirdparty.TokenMarketValues{} for _, smbls := range chunks { @@ -138,6 +155,7 @@ func (c *Client) FetchTokenMarketValues(symbols []string, currency string) (map[ params := url.Values{} params.Add("fsyms", strings.Join(realSymbols, ",")) params.Add("tsyms", realCurrency) + params.Add("relaxedValidation", "true") params.Add("extraParams", extraParamStatus) url := fmt.Sprintf("%s/data/pricemultifull", c.baseURL) diff --git a/services/wallet/thirdparty/utils/symbols.go b/services/wallet/thirdparty/utils/symbols.go index d97fd1482..f82625f9b 100644 --- a/services/wallet/thirdparty/utils/symbols.go +++ b/services/wallet/thirdparty/utils/symbols.go @@ -1,6 +1,9 @@ package utils -import "strings" +import ( + "errors" + "strings" +) var renameMapping = map[string]string{ "STT": "SNT", @@ -32,22 +35,36 @@ func GetRealSymbol(symbol string) string { return strings.ToUpper(symbol) } -func ChunkSymbols(symbols []string, chunkSizeOptional ...int) [][]string { - var chunks [][]string - chunkSize := 100 - if len(chunkSizeOptional) > 0 { - chunkSize = chunkSizeOptional[0] - } - - for i := 0; i < len(symbols); i += chunkSize { - end := i + chunkSize - - if end > len(symbols) { - end = len(symbols) - } - - chunks = append(chunks, symbols[i:end]) - } - - return chunks +type ChunkSymbolsParams struct { + MaxSymbolsPerChunk int + MaxCharsPerChunk int + ExtraCharsPerSymbol int +} + +func ChunkSymbols(symbols []string, params ChunkSymbolsParams) ([][]string, error) { + var chunks [][]string + if len(symbols) == 0 { + return chunks, nil + } + + chunk := make([]string, 0, 100) + chunkChars := 0 + for _, symbol := range symbols { + symbolChars := len(symbol) + params.ExtraCharsPerSymbol + if params.MaxCharsPerChunk > 0 && symbolChars > params.MaxCharsPerChunk { + return nil, errors.New("chunk cannot fit symbol: " + symbol) + } + if (params.MaxCharsPerChunk > 0 && chunkChars+symbolChars > params.MaxCharsPerChunk) || + (params.MaxSymbolsPerChunk > 0 && len(chunk) >= params.MaxSymbolsPerChunk) { + // Max chars/symbols reached, store chunk and start a new one + chunks = append(chunks, chunk) + chunk = make([]string, 0, 100) + chunkChars = 0 + } + chunk = append(chunk, symbol) + chunkChars += symbolChars + } + chunks = append(chunks, chunk) + + return chunks, nil } diff --git a/services/wallet/thirdparty/utils/symbols_test.go b/services/wallet/thirdparty/utils/symbols_test.go new file mode 100644 index 000000000..ef67613b1 --- /dev/null +++ b/services/wallet/thirdparty/utils/symbols_test.go @@ -0,0 +1,61 @@ +package utils + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func Test_RenameSymbols(t *testing.T) { + symbols := []string{"STT", "ETH", "BTC"} + renames := RenameSymbols(symbols) + require.Equal(t, []string{"SNT", "ETH", "BTC"}, renames) +} + +func Test_RemoveDuplicates(t *testing.T) { + strings := []string{"STT", "ETH", "BTC", "ETH", "BTC"} + uniqueStrings := RemoveDuplicates(strings) + require.Equal(t, []string{"STT", "ETH", "BTC"}, uniqueStrings) +} + +func Test_GetRealSymbol(t *testing.T) { + require.Equal(t, "SNT", GetRealSymbol("STT")) + require.Equal(t, "ETH", GetRealSymbol("ETH")) +} + +func Test_ChunkSymbols(t *testing.T) { + symbols := []string{"STT", "ETH", "BTC"} + params := ChunkSymbolsParams{MaxCharsPerChunk: 10, ExtraCharsPerSymbol: 1} + chunks, err := ChunkSymbols(symbols, params) + require.NoError(t, err) + require.Equal(t, [][]string{{"STT", "ETH"}, {"BTC"}}, chunks) + + params = ChunkSymbolsParams{MaxCharsPerChunk: 10, ExtraCharsPerSymbol: 2} + chunks, err = ChunkSymbols(symbols, params) + require.NoError(t, err) + require.Equal(t, [][]string{{"STT", "ETH"}, {"BTC"}}, chunks) + + params = ChunkSymbolsParams{MaxCharsPerChunk: 4, ExtraCharsPerSymbol: 1} + chunks, err = ChunkSymbols(symbols, params) + require.NoError(t, err) + require.Equal(t, [][]string{{"STT"}, {"ETH"}, {"BTC"}}, chunks) + + params = ChunkSymbolsParams{MaxSymbolsPerChunk: 1, MaxCharsPerChunk: 10, ExtraCharsPerSymbol: 2} + chunks, err = ChunkSymbols(symbols, params) + require.NoError(t, err) + require.Equal(t, [][]string{{"STT"}, {"ETH"}, {"BTC"}}, chunks) + + params = ChunkSymbolsParams{MaxCharsPerChunk: 9, ExtraCharsPerSymbol: 2} + chunks, err = ChunkSymbols(symbols, params) + require.NoError(t, err) + require.Equal(t, [][]string{{"STT"}, {"ETH"}, {"BTC"}}, chunks) + + params = ChunkSymbolsParams{MaxCharsPerChunk: 2, ExtraCharsPerSymbol: 1} + chunks, err = ChunkSymbols([]string{}, params) + require.NoError(t, err) + require.Len(t, chunks, 0) + + params = ChunkSymbolsParams{MaxCharsPerChunk: 2, ExtraCharsPerSymbol: 1} + _, err = ChunkSymbols(symbols, params) + require.Error(t, err) +} diff --git a/tests-unit-network/README.MD b/tests-unit-network/README.MD new file mode 100644 index 000000000..84d151228 --- /dev/null +++ b/tests-unit-network/README.MD @@ -0,0 +1,12 @@ +## Overview + +Tests for status-go that require access to the network (for example, chain/market/collectibles providers). + +## Table of Contents + +- [Overview](#overview) +- [How to Run](#how-to-run) + +## How to Run + +`make test-unit-network` \ No newline at end of file diff --git a/tests-unit-network/cryptocompare/cryptocompare_test.go b/tests-unit-network/cryptocompare/cryptocompare_test.go new file mode 100644 index 000000000..3c55ab882 --- /dev/null +++ b/tests-unit-network/cryptocompare/cryptocompare_test.go @@ -0,0 +1,84 @@ +package cryptocompare_tests + +import ( + "testing" + + "github.com/status-im/status-go/appdatabase" + "github.com/status-im/status-go/params" + mock_network "github.com/status-im/status-go/rpc/network/mock" + w_common "github.com/status-im/status-go/services/wallet/common" + "github.com/status-im/status-go/services/wallet/thirdparty/cryptocompare" + "github.com/status-im/status-go/services/wallet/token" + "github.com/status-im/status-go/t/helpers" + "github.com/status-im/status-go/walletdatabase" + + "github.com/stretchr/testify/require" + + "go.uber.org/mock/gomock" +) + +func getTokenSymbols(t *testing.T) []string { + appDB, err := helpers.SetupTestMemorySQLDB(appdatabase.DbInitializer{}) + require.NoError(t, err) + + walletDB, err := helpers.SetupTestMemorySQLDB(walletdatabase.DbInitializer{}) + require.NoError(t, err) + + networksList := []params.Network{ + { + ChainID: w_common.EthereumMainnet, + }, + { + ChainID: w_common.OptimismMainnet, + }, + { + ChainID: w_common.ArbitrumMainnet, + }, + } + + ptrNetworkList := make([]*params.Network, 0, len(networksList)) + for i := range networksList { + ptrNetworkList = append(ptrNetworkList, &networksList[i]) + } + + ctrl := gomock.NewController(t) + defer ctrl.Finish() + networkManager := mock_network.NewMockManagerInterface(ctrl) + networkManager.EXPECT().Get(gomock.Any()).Return(ptrNetworkList, nil).AnyTimes() + networkManager.EXPECT().GetAll().Return(ptrNetworkList, nil).AnyTimes() + networkManager.EXPECT().GetConfiguredNetworks().Return(networksList).AnyTimes() + + // Skeleton token store to get full list of tokens + tm := token.NewTokenManager(walletDB, nil, nil, networkManager, appDB, nil, nil, nil, nil, nil) + + tokens, err := tm.GetAllTokens() + require.NoError(t, err) + + symbolsMap := make(map[string]bool) + for _, token := range tokens { + symbolsMap[token.Symbol] = true + } + + symbols := make([]string, 0, len(symbolsMap)) + for symbol := range symbolsMap { + symbols = append(symbols, symbol) + } + + return symbols +} + +func TestFetchPrices(t *testing.T) { + symbols := getTokenSymbols(t) + + stdClient := cryptocompare.NewClient() + _, err := stdClient.FetchPrices(symbols, []string{"USD"}) + require.NoError(t, err) +} + +func TestFetchTokenMarketValues(t *testing.T) { + symbols := getTokenSymbols(t) + + stdClient := cryptocompare.NewClient() + _, err := stdClient.FetchTokenMarketValues(symbols, "USD") + require.NoError(t, err) +}