fix(wallet)_: Handle balance fetching errors & fallback to cached values (#5628)

- Return errors from fetchBalancesForChain and GetBalancesAtByChain instead of silently ignoring them.
 - Use cached balances if fetching new data fails, preventing empty wallets and ensuring data consistency.
 - Fixed unit tests that was expecting GetBalancesAtByChain to always return nil error

Closes #15767

Co-authored-by: belalshehab <belal@status.im>

services/wallet/reader.go

@@ -264,7 +264,10 @@ func (r *Reader) invalidateBalanceCache() {
 func (r *Reader) FetchOrGetCachedWalletBalances(ctx context.Context, clients map[uint64]chain.ClientInterface, addresses []common.Address) (map[common.Address][]token.StorageToken, error) {
 	needFetch := !r.isBalanceCacheValid(addresses) || r.isBalanceUpdateNeededAnyway(clients, addresses)
 	if needFetch {
-		return r.FetchBalances(ctx, clients, addresses)
+		fetchedBalances, err := r.FetchBalances(ctx, clients, addresses)
+		if err == nil {
+			return fetchedBalances, nil
+		}
 	}
 
 	return r.GetCachedBalances(clients, addresses)

services/wallet/reader_test.go

@@ -1040,3 +1040,24 @@ func TestReaderRestart(t *testing.T) {
 	require.NotNil(t, reader.walletEventsWatcher)
 	require.NotEqual(t, previousWalletEventsWatcher, reader.walletEventsWatcher)
 }
+
+func TestFetchOrGetCachedWalletBalances(t *testing.T) {
+	// Test the behavior of FetchOrGetCachedWalletBalances when fetching new balances fails.
+	// This focuses on the error handling path where the function should return the cached balances as a fallback.
+	// We don't explicitly test the contents of fetched or cached balances here, as those
+	// are covered in other dedicated tests. The main goal is to ensure the correct flow of
+	// execution and data retrieval in this specific failure scenario.
+
+	reader, _, tokenPersistence, mockCtrl := setupReader(t)
+	defer mockCtrl.Finish()
+
+	reader.invalidateBalanceCache()
+
+	tokenPersistence.EXPECT().GetTokens().Return(nil, errors.New("error")).AnyTimes()
+
+	clients := map[uint64]chain.ClientInterface{}
+	addresses := []common.Address{}
+
+	_, err := reader.FetchOrGetCachedWalletBalances(context.TODO(), clients, addresses)
+	require.Error(t, err)
+}

services/wallet/token/balancefetcher/balance_fetcher.go

@@ -82,7 +82,7 @@ func (bf *DefaultBalanceFetcher) fetchBalancesForChain(parent context.Context, c
 		group.Add(func(parent context.Context) error {
 			balances, err := bf.FetchChainBalances(parent, accounts, ethScanContract, atBlock)
 			if err != nil {
-				return nil
+				return err
 			}
 
 			updateBalance(balances)
@@ -111,7 +111,7 @@ func (bf *DefaultBalanceFetcher) fetchBalancesForChain(parent context.Context, c
 				}
 
 				if err != nil {
-					return nil
+					return err
 				}
 
 				updateBalance(accTokenBalance)
@@ -298,7 +298,7 @@ func (bf *DefaultBalanceFetcher) GetBalancesAtByChain(parent context.Context, cl
 		group.Add(func(parent context.Context) error {
 			balances, err := bf.fetchBalancesForChain(parent, client, accounts, tokens, atBlocks[client.NetworkID()])
 			if err != nil {
-				return nil
+				return err
 			}
 
 			updateBalance(client.NetworkID(), balances)

services/wallet/token/balancefetcher/balance_fetcher_test.go

@@ -306,11 +306,6 @@ func TestBalanceFetcherGetBalancesAtByChain(t *testing.T) {
 	chainClientOpt.EXPECT().NetworkID().Return(w_common.OptimismMainnet).AnyTimes()
 	chainClientArb := mock_client.NewMockClientInterface(ctrl)
 	chainClientArb.EXPECT().NetworkID().Return(w_common.ArbitrumMainnet).AnyTimes()
-	chainClients := map[uint64]chain.ClientInterface{
-		w_common.EthereumMainnet: chainClient,
-		w_common.OptimismMainnet: chainClientOpt,
-		w_common.ArbitrumMainnet: chainClientArb,
-	}
 
 	expectedEthBalances := map[common.Address]*big.Int{
 		accounts[0]: big.NewInt(100),
@@ -378,9 +373,21 @@ func TestBalanceFetcherGetBalancesAtByChain(t *testing.T) {
 
 	bf := NewDefaultBalanceFetcher(contractMaker)
 
-	// Fetch native balances and token balances using scan contract
+	// Fetch native balances and token balances using scan contract for Ethereum Mainnet and Optimism Mainnet
+	chainClients := map[uint64]chain.ClientInterface{
+		w_common.EthereumMainnet: chainClient,
+		w_common.OptimismMainnet: chainClientOpt,
+	}
 	balances, err := bf.GetBalancesAtByChain(ctx, chainClients, accounts, tokens, atBlocks)
 
 	require.NoError(t, err)
 	require.Equal(t, expectedBalances, balances)
+
+	// Fetch native balances and token balances using scan contract for Arbitrum Mainnet
+	chainClientsArb := map[uint64]chain.ClientInterface{w_common.ArbitrumMainnet: chainClientArb}
+	balancesArb, errArb := bf.GetBalancesAtByChain(ctx, chainClientsArb, accounts, tokens, atBlocks)
+
+	require.Error(t, errArb, "GetBalancesAtByChain should return an error for Arbitrum Mainnet")
+	require.Contains(t, errArb.Error(), "no scan contract", "Incorrect error message for Arbitrum Mainnet")
+	require.Nil(t, balancesArb[w_common.ArbitrumMainnet])
 }