fix(wallet) fix crash if to_address is NULL in transfers

Add nill tests for TestGetRecipients, GetOldestTimestamp
Also fix returning duplicate addresses in TestGetRecipients

Updates status-desktop #11170

services/wallet/activity/activity_test.go

@@ -696,7 +696,9 @@ func TestGetActivityEntriesFilterByTokenType(t *testing.T) {
 	for i := range trs {
 		tokenAddr := transfer.TestTokens[i].Address
 		trs[i].ChainID = common.ChainID(transfer.TestTokens[i].ChainID)
-		transfer.InsertTestTransferWithToken(t, deps.db, trs[i].To, &trs[i], tokenAddr)
+		transfer.InsertTestTransferWithOptions(t, deps.db, trs[i].To, &trs[i], &transfer.TestTransferOptions{
+			TokenAddress: tokenAddr,
+		})
 	}
 
 	mockTestAccountsWithAddresses(t, deps.db, append(append(append(fromTds, toTds...), fromTrs...), toTrs...))

services/wallet/activity/filter.go

@@ -89,31 +89,49 @@ type Filter struct {
 	FilterOutCollectibles bool    `json:"filterOutCollectibles"`
 }
 
-// TODO: consider sorting by saved address and contacts to offload the client from doing it at runtime
 func GetRecipients(ctx context.Context, db *sql.DB, offset int, limit int) (addresses []eth.Address, hasMore bool, err error) {
 	rows, err := db.QueryContext(ctx, `
-        SELECT
+		SELECT
-            transfers.tx_to_address as to_address,
+			to_address,
-            transfers.timestamp AS timestamp
+			MIN(timestamp) AS min_timestamp
-        FROM transfers
+		FROM (
-        WHERE transfers.multi_transaction_id = 0
+			SELECT
+				transfers.tx_to_address as to_address,
+				MIN(transfers.timestamp) AS timestamp
+			FROM
+				transfers
+			WHERE
+				transfers.multi_transaction_id = 0 AND transfers.tx_to_address NOT NULL
+			GROUP BY
+				transfers.tx_to_address
 
-        UNION ALL
+			UNION
 
-        SELECT
+			SELECT
-            pending_transactions.to_address AS to_address,
+				pending_transactions.to_address AS to_address,
-            pending_transactions.timestamp AS timestamp
+				MIN(pending_transactions.timestamp) AS timestamp
-        FROM pending_transactions
+			FROM
-        WHERE pending_transactions.multi_transaction_id = 0
+				pending_transactions
+			WHERE
+				pending_transactions.multi_transaction_id = 0 AND pending_transactions.to_address NOT NULL
+			GROUP BY
+				pending_transactions.to_address
 
-        UNION ALL
+			UNION
 
-        SELECT
+			SELECT
-            multi_transactions.to_address AS to_address,
+				multi_transactions.to_address AS to_address,
-            multi_transactions.timestamp AS timestamp
+				MIN(multi_transactions.timestamp) AS timestamp
-        FROM multi_transactions
+			FROM
-        ORDER BY timestamp DESC
+				multi_transactions
-        LIMIT ? OFFSET ?`, limit, offset)
+			GROUP BY
+				multi_transactions.to_address
+		) AS combined_result
+		GROUP BY
+			to_address
+		ORDER BY
+			min_timestamp DESC
+		LIMIT ? OFFSET ?;`, limit, offset)
 	if err != nil {
 		return nil, false, err
 	}

services/wallet/activity/filter_test.go

@@ -7,6 +7,7 @@ import (
 	"testing"
 
 	eth "github.com/ethereum/go-ethereum/common"
+	eth_common "github.com/ethereum/go-ethereum/common"
 
 	"github.com/status-im/status-go/appdatabase"
 	"github.com/status-im/status-go/services/wallet/testutils"
@@ -24,6 +25,50 @@ func setupTestFilterDB(t *testing.T) (db *sql.DB, close func()) {
 	}
 }
 
+// insertTestData inserts 6 extractable activity entries: 2 transfers, 2 pending transactions and 2 multi transactions
+func insertTestData(t *testing.T, db *sql.DB, nullifyToForIndexes []int) (trs []transfer.TestTransfer, toTrs []eth_common.Address, multiTxs []transfer.TestMultiTransaction) {
+	// Add 6 extractable transactions
+	trs, _, toTrs = transfer.GenerateTestTransfers(t, db, 0, 7)
+	multiTxs = []transfer.TestMultiTransaction{
+		transfer.GenerateTestBridgeMultiTransaction(trs[0], trs[1]),
+		transfer.GenerateTestSwapMultiTransaction(trs[2], testutils.SntSymbol, 100),
+	}
+	for j := range nullifyToForIndexes {
+		if nullifyToForIndexes[j] == 1 {
+			multiTxs[0].ToAddress = eth_common.Address{}
+		}
+		if nullifyToForIndexes[j] == 2 {
+			multiTxs[1].ToAddress = eth_common.Address{}
+		}
+	}
+
+	trs[0].MultiTransactionID = transfer.InsertTestMultiTransaction(t, db, &multiTxs[0])
+	trs[1].MultiTransactionID = trs[0].MultiTransactionID
+	trs[2].MultiTransactionID = transfer.InsertTestMultiTransaction(t, db, &multiTxs[1])
+
+	for i := range trs {
+		if i < 5 {
+			var nullifyAddresses []eth_common.Address
+			for j := range nullifyToForIndexes {
+				if i == nullifyToForIndexes[j] {
+					nullifyAddresses = append(nullifyAddresses, trs[i].To)
+				}
+			}
+			transfer.InsertTestTransferWithOptions(t, db, trs[i].To, &trs[i], &transfer.TestTransferOptions{
+				NullifyAddresses: nullifyAddresses,
+			})
+		} else {
+			for j := range nullifyToForIndexes {
+				if i == nullifyToForIndexes[j] {
+					trs[i].To = eth_common.Address{}
+				}
+			}
+			transfer.InsertTestPendingTransaction(t, db, &trs[i])
+		}
+	}
+	return
+}
+
 func TestGetRecipientsEmptyDB(t *testing.T) {
 	db, close := setupTestFilterDB(t)
 	defer close()
@@ -38,11 +83,25 @@ func TestGetRecipients(t *testing.T) {
 	db, close := setupTestFilterDB(t)
 	defer close()
 
-	// Add 6 extractable transactions
+	trs, toTrs, _ := insertTestData(t, db, nil)
-	trs, _, toTrs := transfer.GenerateTestTransfers(t, db, 0, 6)
+
-	for i := range trs {
+	// Generate and insert transactions with the same to address
-		transfer.InsertTestTransfer(t, db, trs[i].To, &trs[i])
+	dupTrs, _, _ := transfer.GenerateTestTransfers(t, db, 8, 4)
+	dupTrs[0].To = trs[1].To
+	dupTrs[2].To = trs[2].To
+	dupMultiTxs := []transfer.TestMultiTransaction{
+		transfer.GenerateTestSendMultiTransaction(dupTrs[0]),
+		transfer.GenerateTestSwapMultiTransaction(dupTrs[2], testutils.SntSymbol, 100),
 	}
+	dupTrs[0].MultiTransactionID = transfer.InsertTestMultiTransaction(t, db, &dupMultiTxs[0])
+	transfer.InsertTestTransfer(t, db, dupTrs[0].To, &dupTrs[0])
+	dupTrs[2].MultiTransactionID = transfer.InsertTestMultiTransaction(t, db, &dupMultiTxs[1])
+	transfer.InsertTestPendingTransaction(t, db, &dupTrs[2])
+
+	dupTrs[1].To = trs[3].To
+	transfer.InsertTestTransfer(t, db, dupTrs[1].To, &dupTrs[1])
+	dupTrs[3].To = trs[5].To
+	transfer.InsertTestPendingTransaction(t, db, &dupTrs[3])
 
 	entries, hasMore, err := GetRecipients(context.Background(), db, 0, 15)
 	require.NoError(t, err)
@@ -59,12 +118,24 @@ func TestGetRecipients(t *testing.T) {
 		require.True(t, found, fmt.Sprintf("recipient %s not found in toTrs", entries[i].Hex()))
 	}
 
-	entries, hasMore, err = GetRecipients(context.Background(), db, 0, 4)
+	entries, hasMore, err = GetRecipients(context.Background(), db, 0, 2)
 	require.NoError(t, err)
-	require.Equal(t, 4, len(entries))
+	require.Equal(t, 2, len(entries))
 	require.True(t, hasMore)
 }
 
+func TestGetRecipients_NullAddresses(t *testing.T) {
+	db, close := setupTestFilterDB(t)
+	defer close()
+
+	insertTestData(t, db, []int{1, 2, 3, 5})
+
+	entries, hasMore, err := GetRecipients(context.Background(), db, 0, 15)
+	require.NoError(t, err)
+	require.False(t, hasMore)
+	require.Equal(t, 3, len(entries))
+}
+
 func TestGetOldestTimestampEmptyDB(t *testing.T) {
 	db, close := setupTestFilterDB(t)
 	defer close()
@@ -78,20 +149,7 @@ func TestGetOldestTimestamp(t *testing.T) {
 	db, close := setupTestFilterDB(t)
 	defer close()
 
-	// Add 6 extractable transactions
+	trs, _, multiTxs := insertTestData(t, db, nil)
-	trs, _, _ := transfer.GenerateTestTransfers(t, db, 0, 7)
-	for i := range trs {
-		if i < 5 {
-			transfer.InsertTestTransfer(t, db, trs[i].To, &trs[i])
-		} else {
-			transfer.InsertTestPendingTransaction(t, db, &trs[i])
-		}
-	}
-
-	multiTxs := []transfer.TestMultiTransaction{
-		transfer.GenerateTestBridgeMultiTransaction(trs[0], trs[1]),
-		transfer.GenerateTestSwapMultiTransaction(trs[2], testutils.SntSymbol, 100),
-	}
 
 	// Extract oldest timestamp, no filter
 	timestamp, err := GetOldestTimestamp(context.Background(), db, []eth.Address{})
@@ -126,3 +184,37 @@ func TestGetOldestTimestamp(t *testing.T) {
 	require.NoError(t, err)
 	require.Equal(t, trs[6].Timestamp, timestamp)
 }
+
+func TestGetOldestTimestamp_NullAddresses(t *testing.T) {
+	db, close := setupTestFilterDB(t)
+	defer close()
+
+	trs, _, _ := transfer.GenerateTestTransfers(t, db, 0, 3)
+	nullifyAddresses := []eth_common.Address{
+		trs[0].To, trs[2].To, trs[1].From,
+	}
+	for i := range trs {
+		transfer.InsertTestTransferWithOptions(t, db, trs[i].To, &trs[i], &transfer.TestTransferOptions{
+			NullifyAddresses: nullifyAddresses,
+		})
+	}
+
+	// Extract oldest timestamp, no filter
+	timestamp, err := GetOldestTimestamp(context.Background(), db, []eth.Address{})
+	require.NoError(t, err)
+	require.Equal(t, trs[0].Timestamp, timestamp)
+
+	// Test to filter
+	timestamp, err = GetOldestTimestamp(context.Background(), db, []eth.Address{
+		trs[1].To, trs[2].To,
+	})
+	require.NoError(t, err)
+	require.Equal(t, trs[1].Timestamp, timestamp)
+
+	// Test from filter
+	timestamp, err = GetOldestTimestamp(context.Background(), db, []eth.Address{
+		trs[1].From,
+	})
+	require.NoError(t, err)
+	require.Equal(t, int64(0), timestamp)
+}

services/wallet/transfer/testutils.go

@@ -207,10 +207,17 @@ var NativeTokenIndices = []int{0, 1, 2}
 
 func InsertTestTransfer(t *testing.T, db *sql.DB, address eth_common.Address, tr *TestTransfer) {
 	token := TestTokens[int(tr.Timestamp)%len(TestTokens)]
-	InsertTestTransferWithToken(t, db, address, tr, token.Address)
+	InsertTestTransferWithOptions(t, db, address, tr, &TestTransferOptions{
+		TokenAddress: token.Address,
+	})
 }
 
-func InsertTestTransferWithToken(t *testing.T, db *sql.DB, address eth_common.Address, tr *TestTransfer, tokenAddress eth_common.Address) {
+type TestTransferOptions struct {
+	TokenAddress     eth_common.Address
+	NullifyAddresses []eth_common.Address
+}
+
+func InsertTestTransferWithOptions(t *testing.T, db *sql.DB, address eth_common.Address, tr *TestTransfer, opt *TestTransferOptions) {
 	var (
 		tx *sql.Tx
 	)
@@ -243,10 +250,22 @@ func InsertTestTransferWithToken(t *testing.T, db *sql.DB, address eth_common.Ad
 	}
 
 	tokenType := "eth"
-	if (tokenAddress != eth_common.Address{}) {
+	if (opt.TokenAddress != eth_common.Address{}) {
 		tokenType = "erc20"
 	}
 
+	// Workaround to simulate writing of NULL values for addresses
+	txTo := &tr.To
+	txFrom := &tr.From
+	for i := 0; i < len(opt.NullifyAddresses); i++ {
+		if opt.NullifyAddresses[i] == tr.To {
+			txTo = nil
+		}
+		if opt.NullifyAddresses[i] == tr.From {
+			txFrom = nil
+		}
+	}
+
 	transfer := transferDBFields{
 		chainID:            uint64(tr.ChainID),
 		id:                 tr.Hash,
@@ -260,9 +279,9 @@ func InsertTestTransferWithToken(t *testing.T, db *sql.DB, address eth_common.Ad
 		baseGasFees:        "0x0",
 		receiptStatus:      &receiptStatus,
 		txValue:            big.NewInt(tr.Value),
-		txFrom:             &tr.From,
+		txFrom:             txFrom,
-		txTo:               &tr.To,
+		txTo:               txTo,
-		tokenAddress:       &tokenAddress,
+		tokenAddress:       &opt.TokenAddress,
 	}
 	err = updateOrInsertTransfersDBFields(tx, []transferDBFields{transfer})
 	require.NoError(t, err)