fix(wallet): cleanup multi_transactions table on account removed

Updates #4937

services/wallet/history/service_test.go

@@ -418,13 +418,14 @@ func Test_removeBalanceHistoryOnEventAccountRemoved(t *testing.T) {
 			Accounts: []common.Address{address},
 		})
 
-		require.NoError(t, utils.Eventually(func() error {
+		err := utils.Eventually(func() error {
 			entries, err := database.getNewerThan(&assetIdentity{1, []common.Address{address}, "ETH"}, 0)
 			if err == nil && len(entries) == 0 {
 				return nil
 			}
 			return errors.New("data is not removed")
-		}, 100*time.Millisecond, 10*time.Millisecond))
+		}, 100*time.Millisecond, 10*time.Millisecond)
+		require.NoError(t, err)
 	}()
 
 	group.Wait()

services/wallet/keycard_pairings_test.go

@@ -6,6 +6,8 @@ import (
 
 	"github.com/stretchr/testify/require"
 
+	"github.com/ethereum/go-ethereum/event"
+
 	"github.com/status-im/status-go/appdatabase"
 	"github.com/status-im/status-go/multiaccounts/accounts"
 	"github.com/status-im/status-go/params"
@@ -25,7 +27,9 @@ func TestKeycardPairingsFile(t *testing.T) {
 	db, err := helpers.SetupTestMemorySQLDB(walletdatabase.DbInitializer{})
 	require.NoError(t, err)
 
-	service := NewService(db, accountsDb, appDB, &rpc.Client{NetworkManager: network.NewManager(db)}, nil, nil, nil, nil, &params.NodeConfig{}, nil, nil, nil, nil, nil)
+	accountFeed := &event.Feed{}
+
+	service := NewService(db, accountsDb, appDB, &rpc.Client{NetworkManager: network.NewManager(db)}, accountFeed, nil, nil, nil, &params.NodeConfig{}, nil, nil, nil, nil, nil)
 
 	data, err := service.KeycardPairings().GetPairingsJSONFileContent()
 	require.NoError(t, err)

services/wallet/transfer/controller.go

@@ -266,6 +266,11 @@ func (c *Controller) cleanUpRemovedAccount(address common.Address) {
 	if err != nil {
 		log.Error("Failed to delete blocks ranges sequential", "error", err)
 	}
+
+	err = c.transactionManager.removeMultiTransactionByAddress(address)
+	if err != nil {
+		log.Error("Failed to delete multitransactions", "error", err)
+	}
 }
 
 func (c *Controller) cleanupAccountsLeftovers() error {

services/wallet/transfer/controller_test.go

@@ -1,19 +1,23 @@
 package transfer
 
 import (
+	"context"
 	"math/big"
+	"sync"
 	"testing"
 	"time"
 
 	"github.com/stretchr/testify/require"
 
 	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/common/hexutil"
 	"github.com/ethereum/go-ethereum/event"
 	"github.com/status-im/status-go/appdatabase"
 	"github.com/status-im/status-go/eth-node/types"
 	"github.com/status-im/status-go/multiaccounts/accounts"
 	"github.com/status-im/status-go/services/accounts/accountsevent"
 	"github.com/status-im/status-go/services/wallet/blockchainstate"
+	wallet_common "github.com/status-im/status-go/services/wallet/common"
 	"github.com/status-im/status-go/t/helpers"
 	"github.com/status-im/status-go/walletdatabase"
 )
@@ -31,16 +35,17 @@ func TestController_watchAccountsChanges(t *testing.T) {
 	accountFeed := &event.Feed{}
 
 	bcstate := blockchainstate.NewBlockChainState()
+	transactionManager := NewTransactionManager(walletDB, nil, nil, nil, accountsDB, nil, nil)
 	c := NewTransferController(
 		walletDB,
 		accountsDB,
 		nil, // rpcClient
 		accountFeed,
-		nil, // transferFeed
-		nil, // transactionManager
-		nil, // pendingTxManager
-		nil, // tokenManager
-		nil, // balanceCacher
+		nil,                // transferFeed
+		transactionManager, // transactionManager
+		nil,                // pendingTxManager
+		nil,                // tokenManager
+		nil,                // balanceCacher
 		bcstate,
 	)
 
@@ -80,11 +85,73 @@ func TestController_watchAccountsChanges(t *testing.T) {
 	require.NoError(t, err)
 	require.NotNil(t, ranges)
 
+	// Insert multitransactions
+	// Save address to accounts DB which transactions we want to preserve
+	counterparty := common.Address{0x1}
+	err = accountsDB.SaveOrUpdateAccounts([]*accounts.Account{
+		{Address: types.Address(counterparty), Chat: false, Wallet: true},
+	}, false)
+	require.NoError(t, err)
+
+	// Self multi transaction
+	midSelf, err := transactionManager.InsertMultiTransaction(&MultiTransaction{
+		FromAddress: address,
+		ToAddress:   address,
+		FromAsset:   "ETH",
+		ToAsset:     "DAI",
+		FromAmount:  &hexutil.Big{},
+		ToAmount:    &hexutil.Big{},
+		Timestamp:   1,
+	})
+
+	require.NoError(t, err)
+	mtxs, err := transactionManager.GetMultiTransactions(context.Background(), []wallet_common.MultiTransactionIDType{midSelf})
+	require.NoError(t, err)
+	require.Len(t, mtxs, 1)
+
+	// Send multi transaction
+	mt := &MultiTransaction{
+		FromAddress: address,
+		ToAddress:   counterparty,
+		FromAsset:   "ETH",
+		ToAsset:     "DAI",
+		FromAmount:  &hexutil.Big{},
+		ToAmount:    &hexutil.Big{},
+		Timestamp:   2,
+	}
+	mid, err := transactionManager.InsertMultiTransaction(mt)
+
+	require.NoError(t, err)
+	mtxs, err = transactionManager.GetMultiTransactions(context.Background(), []wallet_common.MultiTransactionIDType{midSelf, mid})
+	require.NoError(t, err)
+	require.Len(t, mtxs, 2)
+
+	// Another Send multi-transaction where sender and receiver are inverted (both accounts are in accounts DB)
+	midReverse, err := transactionManager.InsertMultiTransaction(&MultiTransaction{
+		FromAddress: mt.ToAddress,
+		ToAddress:   mt.FromAddress,
+		FromAsset:   mt.FromAsset,
+		ToAsset:     mt.ToAsset,
+		FromAmount:  mt.FromAmount,
+		ToAmount:    mt.ToAmount,
+		Timestamp:   mt.Timestamp + 1,
+	})
+
+	require.NoError(t, err)
+	mtxs, err = transactionManager.GetMultiTransactions(context.Background(), []wallet_common.MultiTransactionIDType{midSelf, mid, midReverse})
+	require.NoError(t, err)
+	require.Len(t, mtxs, 3)
+
+	// Start watching accounts
+	wg := sync.WaitGroup{}
+	wg.Add(1)
 	c.accWatcher = accountsevent.NewWatcher(c.accountsDB, c.accountFeed, func(changedAddresses []common.Address, eventType accountsevent.EventType, currentAddresses []common.Address) {
 		c.onAccountsChanged(changedAddresses, eventType, currentAddresses, []uint64{chainID})
 
 		// Quit channel event handler before  destroying the channel
 		go func() {
+			defer wg.Done()
+
 			time.Sleep(1 * time.Millisecond)
 			// Wait for DB to be cleaned up
 			c.accWatcher.Stop()
@@ -107,6 +174,11 @@ func TestController_watchAccountsChanges(t *testing.T) {
 			require.Nil(t, ranges.tokens.FirstKnown)
 			require.Nil(t, ranges.tokens.LastKnown)
 			require.Nil(t, ranges.tokens.Start)
+
+			mtxs, err := transactionManager.GetMultiTransactions(context.Background(), []wallet_common.MultiTransactionIDType{mid, midSelf, midReverse})
+			require.NoError(t, err)
+			require.Len(t, mtxs, 1)
+			require.Equal(t, midReverse, mtxs[0].ID)
 		}()
 	})
 	c.startAccountWatcher([]uint64{chainID})
@@ -122,6 +194,8 @@ func TestController_watchAccountsChanges(t *testing.T) {
 			Accounts: []common.Address{address},
 		})
 	}()
+
+	wg.Wait()
 }
 
 func TestController_cleanupAccountLeftovers(t *testing.T) {
@@ -146,17 +220,18 @@ func TestController_cleanupAccountLeftovers(t *testing.T) {
 	require.NoError(t, err)
 	require.Len(t, storedAccs, 1)
 
+	transactionManager := NewTransactionManager(walletDB, nil, nil, nil, accountsDB, nil, nil)
 	bcstate := blockchainstate.NewBlockChainState()
 	c := NewTransferController(
 		walletDB,
 		accountsDB,
-		nil, // rpcClient
-		nil, // accountFeed
-		nil, // transferFeed
-		nil, // transactionManager
-		nil, // pendingTxManager
-		nil, // tokenManager
-		nil, // balanceCacher
+		nil,                // rpcClient
+		nil,                // accountFeed
+		nil,                // transferFeed
+		transactionManager, // transactionManager
+		nil,                // pendingTxManager
+		nil,                // tokenManager
+		nil,                // balanceCacher
 		bcstate,
 	)
 	chainID := uint64(777)

services/wallet/transfer/transaction_manager_multitransaction.go

@@ -458,3 +458,67 @@ func (tm *TransactionManager) getVerifiedWalletAccount(address, password string)
 		AccountKey: key,
 	}, nil
 }
+
+func (tm *TransactionManager) removeMultiTransactionByAddress(address common.Address) error {
+	// We must not remove those transactions, where from_address and to_address are different and both are stored in accounts DB
+	// and one of them is equal to the address, as we want to keep the records for the other address
+	// That is why we don't use cascade delete here with references to transfers table, as we might have 2 records in multi_transactions
+	// for the same transaction, one for each address
+
+	stmt, err := tm.db.Prepare(`SELECT rowid, from_address, to_address
+								FROM multi_transactions
+								WHERE from_address=? OR to_address=?`)
+	if err != nil {
+		return err
+	}
+
+	rows, err := stmt.Query(address, address)
+	if err != nil {
+		return err
+	}
+	defer rows.Close()
+
+	rowIDs := make([]int, 0)
+	rowID, fromAddress, toAddress := 0, common.Address{}, common.Address{}
+	for rows.Next() {
+		err = rows.Scan(&rowID, &fromAddress, &toAddress)
+		if err != nil {
+			log.Error("Failed to scan row", "error", err)
+			continue
+		}
+
+		// Remove self transactions as well, leave only those where we have the counterparty in accounts DB
+		if fromAddress != toAddress {
+			// If both addresses are stored in accounts DB, we don't remove the record
+			var addressToCheck common.Address
+			if fromAddress == address {
+				addressToCheck = toAddress
+			} else {
+				addressToCheck = fromAddress
+			}
+			counterpartyExists, err := tm.accountsDB.AddressExists(types.Address(addressToCheck))
+			if err != nil {
+				log.Error("Failed to query accounts db for a given address", "address", address, "error", err)
+				continue
+			}
+
+			// Skip removal if counterparty is in accounts DB and removed address is not sender
+			if counterpartyExists && address != fromAddress {
+				continue
+			}
+		}
+
+		rowIDs = append(rowIDs, rowID)
+	}
+
+	if len(rowIDs) > 0 {
+		for _, rowID := range rowIDs {
+			_, err = tm.db.Exec(`DELETE FROM multi_transactions WHERE rowid=?`, rowID)
+			if err != nil {
+				log.Error("Failed to remove multitransaction", "rowid", rowID, "error", err)
+			}
+		}
+	}
+
+	return err
+}