From 70341f85a59f75f4d947f5379db22c12a2adffde Mon Sep 17 00:00:00 2001 From: Stefan Date: Wed, 6 Sep 2023 20:15:07 +0100 Subject: [PATCH] fix(wallet) send/receive for duplicate transactions Brings consistency in case when sender and receiver are both in the filter address list. This fixes the case of sender and receiver in addresses and filters out duplicate entries. Also - refactor tests to provide support for owners - adapt TestGetActivityEntriesWithSameTransactionForSenderAndReceiverInDB to the use of owner instead of from --- services/wallet/activity/activity.go | 1 + services/wallet/activity/activity_test.go | 275 +++++++++++----------- services/wallet/activity/filter.sql | 60 +++-- services/wallet/transfer/commands.go | 88 ++++--- services/wallet/transfer/database.go | 10 + services/wallet/transfer/iterative.go | 2 +- transactions/pendingtxtracker.go | 11 +- 7 files changed, 247 insertions(+), 200 deletions(-) diff --git a/services/wallet/activity/activity.go b/services/wallet/activity/activity.go index ba5613b36..deb45c030 100644 --- a/services/wallet/activity/activity.go +++ b/services/wallet/activity/activity.go @@ -421,6 +421,7 @@ func getActivityEntries(ctx context.Context, deps FilterDependencies, addresses startFilterDisabled, filter.Period.StartTimestamp, endFilterDisabled, filter.Period.EndTimestamp, filterActivityTypeAll, sliceContains(filter.Types, SendAT), sliceContains(filter.Types, ReceiveAT), sliceContains(filter.Types, ContractDeploymentAT), sliceContains(filter.Types, MintAT), + transfer.MultiTransactionSend, fromTrType, toTrType, filterAllAddresses, filterAllToAddresses, includeAllStatuses, filterStatusCompleted, filterStatusFailed, filterStatusFinalized, filterStatusPending, diff --git a/services/wallet/activity/activity_test.go b/services/wallet/activity/activity_test.go index 0e489e771..d1d2ad82b 100644 --- a/services/wallet/activity/activity_test.go +++ b/services/wallet/activity/activity_test.go @@ -219,18 +219,18 @@ func TestGetActivityEntriesAll(t *testing.T) { transaction: &transfer.TransactionIdentity{ChainID: td.tr1.ChainID, Hash: td.tr1.Hash, Address: td.tr1.To}, id: td.tr1.MultiTransactionID, timestamp: td.tr1.Timestamp, - activityType: SendAT, + activityType: ReceiveAT, activityStatus: CompleteAS, - amountOut: (*hexutil.Big)(big.NewInt(td.tr1.Value)), - amountIn: (*hexutil.Big)(big.NewInt(0)), - tokenOut: TTrToToken(t, &td.tr1.TestTransaction), - tokenIn: nil, - symbolOut: common.NewAndSet("ETH"), - symbolIn: nil, + amountOut: (*hexutil.Big)(big.NewInt(0)), + amountIn: (*hexutil.Big)(big.NewInt(td.tr1.Value)), + tokenOut: nil, + tokenIn: TTrToToken(t, &td.tr1.TestTransaction), + symbolOut: nil, + symbolIn: common.NewAndSet("ETH"), sender: &td.tr1.From, recipient: &td.tr1.To, - chainIDOut: &td.tr1.ChainID, - chainIDIn: nil, + chainIDOut: nil, + chainIDIn: &td.tr1.ChainID, transferType: expectedTokenType(td.tr1.Token.Address), }, entries[3]) require.Equal(t, Entry{ @@ -286,8 +286,10 @@ func TestGetActivityEntriesAll(t *testing.T) { }, entries[0]) } -// TestGetActivityEntriesWithSenderFilter covers the issue with returning the same transaction -// twice when the sender and receiver have entries in the transfers table +// TestGetActivityEntriesWithSenderFilter covers the corner-case of having both sender and receiver in the filter. +// In this specific case we expect that there will be two transactions (one probably backed by a multi-transaction) +// In case of both sender and receiver are included we validate we receive both entries otherwise only the "owned" +// transactions should be retrieved by the filter func TestGetActivityEntriesWithSameTransactionForSenderAndReceiverInDB(t *testing.T) { deps, close := setupTestActivityDB(t) defer close() @@ -297,39 +299,28 @@ func TestGetActivityEntriesWithSameTransactionForSenderAndReceiverInDB(t *testin mockTestAccountsWithAddresses(t, deps.accountsDb, append(fromAddresses, toAddresses...)) - // Add another transaction with sender and receiver reversed - receiverTr := td.tr1 - prevTo := receiverTr.To - receiverTr.To = td.tr1.From - receiverTr.From = prevTo - - // TODO: test also when there is a transaction in the other direction - - // Ensure they are the oldest transactions (last in the list) and we have a consistent order - receiverTr.Timestamp-- - transfer.InsertTestTransfer(t, deps.db, receiverTr.To, &receiverTr) + // Add another transaction with owner reversed + senderTr := td.tr1 + // Ensure we have a consistent order + senderTr.Timestamp++ + // add sender as owner, fillTestData adds receiver as owner + transfer.InsertTestTransfer(t, deps.db, senderTr.From, &senderTr) var filter Filter - entries, err := getActivityEntries(context.Background(), deps, []eth.Address{td.tr1.From, receiverTr.From}, []common.ChainID{}, filter, 0, 10) + entries, err := getActivityEntries(context.Background(), deps, []eth.Address{td.tr1.To, senderTr.From}, []common.ChainID{}, filter, 0, 10) require.NoError(t, err) require.Equal(t, 2, len(entries)) // Check that the transaction are labeled alternatively as send and receive - require.Equal(t, ReceiveAT, entries[1].activityType) - require.NotEqual(t, eth.Address{}, entries[1].transaction.Address) - require.Equal(t, receiverTr.To, entries[1].transaction.Address) - require.Equal(t, SendAT, entries[0].activityType) - require.NotEqual(t, eth.Address{}, entries[0].transaction.Address) - require.Equal(t, td.tr1.To, *entries[0].recipient) + require.Equal(t, senderTr.From, entries[0].transaction.Address) + require.Equal(t, senderTr.From, *entries[0].sender) + require.Equal(t, senderTr.To, *entries[0].recipient) - entries, err = getActivityEntries(context.Background(), deps, []eth.Address{}, []common.ChainID{}, filter, 0, 10) - require.NoError(t, err) - require.Equal(t, 5, len(entries)) - - // Check that the transaction are labeled alternatively as send and receive - require.Equal(t, ReceiveAT, entries[4].activityType) - require.Equal(t, SendAT, entries[3].activityType) + require.Equal(t, ReceiveAT, entries[1].activityType) + require.Equal(t, td.tr1.To, *entries[1].recipient) + require.Equal(t, td.tr1.From, *entries[1].sender) + require.Equal(t, td.tr1.To, *entries[1].recipient) } func TestGetActivityEntriesFilterByTime(t *testing.T) { @@ -360,18 +351,18 @@ func TestGetActivityEntriesFilterByTime(t *testing.T) { transaction: &transfer.TransactionIdentity{ChainID: trs[5].ChainID, Hash: trs[5].Hash, Address: trs[5].To}, id: 0, timestamp: trs[5].Timestamp, - activityType: SendAT, + activityType: ReceiveAT, activityStatus: CompleteAS, - amountOut: (*hexutil.Big)(big.NewInt(trs[5].Value)), - amountIn: (*hexutil.Big)(big.NewInt(0)), - tokenOut: TTrToToken(t, &trs[5].TestTransaction), - tokenIn: nil, - symbolOut: common.NewAndSet("USDC"), - symbolIn: nil, + amountOut: (*hexutil.Big)(big.NewInt(0)), + amountIn: (*hexutil.Big)(big.NewInt(trs[5].Value)), + tokenOut: nil, + tokenIn: TTrToToken(t, &trs[5].TestTransaction), + symbolOut: nil, + symbolIn: common.NewAndSet("USDC"), sender: &trs[5].From, recipient: &trs[5].To, - chainIDOut: &trs[5].ChainID, - chainIDIn: nil, + chainIDOut: nil, + chainIDIn: &trs[5].ChainID, transferType: expectedTokenType(trs[5].Token.Address), }, entries[0]) require.Equal(t, Entry{ @@ -406,18 +397,18 @@ func TestGetActivityEntriesFilterByTime(t *testing.T) { transaction: &transfer.TransactionIdentity{ChainID: trs[2].ChainID, Hash: trs[2].Hash, Address: trs[2].To}, id: 0, timestamp: trs[2].Timestamp, - activityType: SendAT, + activityType: ReceiveAT, activityStatus: CompleteAS, - amountOut: (*hexutil.Big)(big.NewInt(trs[2].Value)), - amountIn: (*hexutil.Big)(big.NewInt(0)), - tokenOut: TTrToToken(t, &trs[2].TestTransaction), - tokenIn: nil, - symbolOut: common.NewAndSet("ETH"), - symbolIn: nil, + amountOut: (*hexutil.Big)(big.NewInt(0)), + amountIn: (*hexutil.Big)(big.NewInt(trs[2].Value)), + tokenOut: nil, + tokenIn: TTrToToken(t, &trs[2].TestTransaction), + symbolOut: nil, + symbolIn: common.NewAndSet("ETH"), sender: &trs[2].From, recipient: &trs[2].To, - chainIDOut: &trs[2].ChainID, - chainIDIn: nil, + chainIDOut: nil, + chainIDIn: &trs[2].ChainID, transferType: expectedTokenType(trs[2].Token.Address), }, entries[0]) require.Equal(t, Entry{ @@ -451,18 +442,18 @@ func TestGetActivityEntriesFilterByTime(t *testing.T) { transaction: &transfer.TransactionIdentity{ChainID: trs[2].ChainID, Hash: trs[2].Hash, Address: trs[2].To}, id: 0, timestamp: trs[2].Timestamp, - activityType: SendAT, + activityType: ReceiveAT, activityStatus: CompleteAS, - amountOut: (*hexutil.Big)(big.NewInt(trs[2].Value)), - amountIn: (*hexutil.Big)(big.NewInt(0)), - tokenOut: TTrToToken(t, &trs[2].TestTransaction), - tokenIn: nil, - symbolOut: common.NewAndSet("ETH"), - symbolIn: nil, + amountOut: (*hexutil.Big)(big.NewInt(0)), + amountIn: (*hexutil.Big)(big.NewInt(trs[2].Value)), + tokenOut: nil, + tokenIn: TTrToToken(t, &trs[2].TestTransaction), + symbolOut: nil, + symbolIn: common.NewAndSet("ETH"), sender: &trs[2].From, recipient: &trs[2].To, - chainIDOut: &trs[2].ChainID, - chainIDIn: nil, + chainIDOut: nil, + chainIDIn: &trs[2].ChainID, transferType: expectedTokenType(trs[2].Token.Address), }, entries[0]) require.Equal(t, Entry{ @@ -470,18 +461,18 @@ func TestGetActivityEntriesFilterByTime(t *testing.T) { transaction: &transfer.TransactionIdentity{ChainID: td.tr1.ChainID, Hash: td.tr1.Hash, Address: td.tr1.To}, id: 0, timestamp: td.tr1.Timestamp, - activityType: SendAT, + activityType: ReceiveAT, activityStatus: CompleteAS, - amountOut: (*hexutil.Big)(big.NewInt(td.tr1.Value)), - amountIn: (*hexutil.Big)(big.NewInt(0)), - tokenOut: TTrToToken(t, &td.tr1.TestTransaction), - tokenIn: nil, - symbolOut: common.NewAndSet("ETH"), - symbolIn: nil, + amountOut: (*hexutil.Big)(big.NewInt(0)), + amountIn: (*hexutil.Big)(big.NewInt(td.tr1.Value)), + tokenOut: nil, + tokenIn: TTrToToken(t, &td.tr1.TestTransaction), + symbolOut: nil, + symbolIn: common.NewAndSet("ETH"), sender: &td.tr1.From, recipient: &td.tr1.To, - chainIDOut: &td.tr1.ChainID, - chainIDIn: nil, + chainIDOut: nil, + chainIDIn: &td.tr1.ChainID, transferType: expectedTokenType(td.tr1.Token.Address), }, entries[6]) } @@ -516,18 +507,18 @@ func TestGetActivityEntriesCheckOffsetAndLimit(t *testing.T) { transaction: &transfer.TransactionIdentity{ChainID: trs[8].ChainID, Hash: trs[8].Hash, Address: trs[8].To}, id: 0, timestamp: trs[8].Timestamp, - activityType: SendAT, + activityType: ReceiveAT, activityStatus: CompleteAS, - amountOut: (*hexutil.Big)(big.NewInt(trs[8].Value)), - amountIn: (*hexutil.Big)(big.NewInt(0)), - tokenOut: TTrToToken(t, &trs[8].TestTransaction), - tokenIn: nil, - symbolOut: common.NewAndSet("ETH"), - symbolIn: nil, + amountOut: (*hexutil.Big)(big.NewInt(0)), + amountIn: (*hexutil.Big)(big.NewInt(trs[8].Value)), + tokenOut: nil, + tokenIn: TTrToToken(t, &trs[8].TestTransaction), + symbolOut: nil, + symbolIn: common.NewAndSet("ETH"), sender: &trs[8].From, recipient: &trs[8].To, - chainIDOut: &trs[8].ChainID, - chainIDIn: nil, + chainIDOut: nil, + chainIDIn: &trs[8].ChainID, transferType: expectedTokenType(trs[8].Token.Address), }, entries[0]) require.Equal(t, Entry{ @@ -535,18 +526,18 @@ func TestGetActivityEntriesCheckOffsetAndLimit(t *testing.T) { transaction: &transfer.TransactionIdentity{ChainID: trs[6].ChainID, Hash: trs[6].Hash, Address: trs[6].To}, id: 0, timestamp: trs[6].Timestamp, - activityType: SendAT, + activityType: ReceiveAT, activityStatus: CompleteAS, - amountOut: (*hexutil.Big)(big.NewInt(trs[6].Value)), - amountIn: (*hexutil.Big)(big.NewInt(0)), - tokenOut: TTrToToken(t, &trs[6].TestTransaction), - tokenIn: nil, - symbolOut: common.NewAndSet("DAI"), - symbolIn: nil, + amountOut: (*hexutil.Big)(big.NewInt(0)), + amountIn: (*hexutil.Big)(big.NewInt(trs[6].Value)), + tokenOut: nil, + tokenIn: TTrToToken(t, &trs[6].TestTransaction), + symbolOut: nil, + symbolIn: common.NewAndSet("DAI"), sender: &trs[6].From, recipient: &trs[6].To, - chainIDOut: &trs[6].ChainID, - chainIDIn: nil, + chainIDOut: nil, + chainIDIn: &trs[6].ChainID, transferType: expectedTokenType(trs[6].Token.Address), }, entries[2]) @@ -560,18 +551,18 @@ func TestGetActivityEntriesCheckOffsetAndLimit(t *testing.T) { transaction: &transfer.TransactionIdentity{ChainID: trs[6].ChainID, Hash: trs[6].Hash, Address: trs[6].To}, id: 0, timestamp: trs[6].Timestamp, - activityType: SendAT, + activityType: ReceiveAT, activityStatus: CompleteAS, - amountOut: (*hexutil.Big)(big.NewInt(trs[6].Value)), - amountIn: (*hexutil.Big)(big.NewInt(0)), - tokenOut: TTrToToken(t, &trs[6].TestTransaction), - tokenIn: nil, - symbolOut: common.NewAndSet("DAI"), - symbolIn: nil, + amountOut: (*hexutil.Big)(big.NewInt(0)), + amountIn: (*hexutil.Big)(big.NewInt(trs[6].Value)), + tokenOut: nil, + tokenIn: TTrToToken(t, &trs[6].TestTransaction), + symbolOut: nil, + symbolIn: common.NewAndSet("DAI"), sender: &trs[6].From, recipient: &trs[6].To, - chainIDOut: &trs[6].ChainID, - chainIDIn: nil, + chainIDOut: nil, + chainIDIn: &trs[6].ChainID, transferType: expectedTokenType(trs[6].Token.Address), }, entries[0]) require.Equal(t, Entry{ @@ -579,18 +570,18 @@ func TestGetActivityEntriesCheckOffsetAndLimit(t *testing.T) { transaction: &transfer.TransactionIdentity{ChainID: trs[4].ChainID, Hash: trs[4].Hash, Address: trs[4].To}, id: 0, timestamp: trs[4].Timestamp, - activityType: SendAT, + activityType: ReceiveAT, activityStatus: CompleteAS, - amountOut: (*hexutil.Big)(big.NewInt(trs[4].Value)), - amountIn: (*hexutil.Big)(big.NewInt(0)), - tokenOut: TTrToToken(t, &trs[4].TestTransaction), - tokenIn: nil, - symbolOut: common.NewAndSet("USDC"), - symbolIn: nil, + amountOut: (*hexutil.Big)(big.NewInt(0)), + amountIn: (*hexutil.Big)(big.NewInt(trs[4].Value)), + tokenOut: nil, + tokenIn: TTrToToken(t, &trs[4].TestTransaction), + symbolOut: nil, + symbolIn: common.NewAndSet("USDC"), sender: &trs[4].From, recipient: &trs[4].To, - chainIDOut: &trs[4].ChainID, - chainIDIn: nil, + chainIDOut: nil, + chainIDIn: &trs[4].ChainID, transferType: expectedTokenType(trs[4].Token.Address), }, entries[2]) @@ -604,18 +595,18 @@ func TestGetActivityEntriesCheckOffsetAndLimit(t *testing.T) { transaction: &transfer.TransactionIdentity{ChainID: trs[2].ChainID, Hash: trs[2].Hash, Address: trs[2].To}, id: 0, timestamp: trs[2].Timestamp, - activityType: SendAT, + activityType: ReceiveAT, activityStatus: CompleteAS, - amountOut: (*hexutil.Big)(big.NewInt(trs[2].Value)), - amountIn: (*hexutil.Big)(big.NewInt(0)), - tokenOut: TTrToToken(t, &trs[2].TestTransaction), - tokenIn: nil, - symbolOut: common.NewAndSet("USDC"), - symbolIn: nil, + amountOut: (*hexutil.Big)(big.NewInt(0)), + amountIn: (*hexutil.Big)(big.NewInt(trs[2].Value)), + tokenOut: nil, + tokenIn: TTrToToken(t, &trs[2].TestTransaction), + symbolOut: nil, + symbolIn: common.NewAndSet("USDC"), sender: &trs[2].From, recipient: &trs[2].To, - chainIDOut: &trs[2].ChainID, - chainIDIn: nil, + chainIDOut: nil, + chainIDIn: &trs[2].ChainID, transferType: expectedTokenType(trs[2].Token.Address), }, entries[0]) } @@ -753,7 +744,7 @@ func TestGetActivityEntriesFilterByAddresses(t *testing.T) { td, fromTds, toTds := fillTestData(t, deps.db) trs, fromTrs, toTrs := transfer.GenerateTestTransfers(t, deps.db, td.nextIndex, 6) for i := range trs { - transfer.InsertTestTransfer(t, deps.db, trs[i].To, &trs[i]) + transfer.InsertTestTransfer(t, deps.db, trs[i].From, &trs[i]) } mockTestAccountsWithAddresses(t, deps.accountsDb, append(append(append(fromTds, toTds...), fromTrs...), toTrs...)) @@ -765,32 +756,33 @@ func TestGetActivityEntriesFilterByAddresses(t *testing.T) { require.NoError(t, err) require.Equal(t, 10, len(entries)) - addressesFilter = []eth.Address{td.multiTx2.ToAddress, trs[1].From, trs[4].To} + addressesFilter = []eth.Address{td.multiTx1.ToAddress, td.multiTx2.FromAddress, trs[1].From, trs[4].From, trs[3].To} + // The td.multiTx1.ToAddress and trs[3].To are missing not having them as owner address entries, err = getActivityEntries(context.Background(), deps, addressesFilter, []common.ChainID{}, filter, 0, 15) require.NoError(t, err) require.Equal(t, 3, len(entries)) require.Equal(t, Entry{ payloadType: SimpleTransactionPT, - transaction: &transfer.TransactionIdentity{ChainID: trs[4].ChainID, Hash: trs[4].Hash, Address: trs[4].To}, + transaction: &transfer.TransactionIdentity{ChainID: trs[4].ChainID, Hash: trs[4].Hash, Address: trs[4].From}, id: 0, timestamp: trs[4].Timestamp, - activityType: ReceiveAT, + activityType: SendAT, activityStatus: CompleteAS, - amountOut: (*hexutil.Big)(big.NewInt(0)), - amountIn: (*hexutil.Big)(big.NewInt(trs[4].Value)), - tokenOut: nil, - tokenIn: TTrToToken(t, &trs[4].TestTransaction), - symbolOut: nil, - symbolIn: common.NewAndSet("USDC"), + amountOut: (*hexutil.Big)(big.NewInt(trs[4].Value)), + amountIn: (*hexutil.Big)(big.NewInt(0)), + tokenOut: TTrToToken(t, &trs[4].TestTransaction), + tokenIn: nil, + symbolOut: common.NewAndSet("USDC"), + symbolIn: nil, sender: &trs[4].From, recipient: &trs[4].To, - chainIDOut: nil, - chainIDIn: &trs[4].ChainID, + chainIDOut: &trs[4].ChainID, + chainIDIn: nil, transferType: expectedTokenType(trs[4].Token.Address), }, entries[0]) require.Equal(t, Entry{ payloadType: SimpleTransactionPT, - transaction: &transfer.TransactionIdentity{ChainID: trs[1].ChainID, Hash: trs[1].Hash, Address: trs[1].To}, + transaction: &transfer.TransactionIdentity{ChainID: trs[1].ChainID, Hash: trs[1].Hash, Address: trs[1].From}, id: 0, timestamp: trs[1].Timestamp, activityType: SendAT, @@ -937,9 +929,9 @@ func TestGetActivityEntriesFilterByTokenType(t *testing.T) { require.NoError(t, err) // Two MT for which ChainID is ignored and one transfer on the main net and the Goerli is ignored require.Equal(t, 3, len(entries)) - require.Equal(t, Erc20, entries[0].tokenOut.TokenType) - require.Equal(t, transfer.UsdcMainnet.Address, entries[0].tokenOut.Address) - require.Nil(t, entries[0].tokenIn) + require.Equal(t, Erc20, entries[0].tokenIn.TokenType) + require.Equal(t, transfer.UsdcMainnet.Address, entries[0].tokenIn.Address) + require.Nil(t, entries[0].tokenOut) // MT has only symbol, the first token is lookup by symbol for both entries require.Equal(t, Erc20, entries[1].tokenOut.TokenType) require.Equal(t, transfer.UsdcMainnet.Address, entries[1].tokenOut.Address) @@ -963,9 +955,9 @@ func TestGetActivityEntriesFilterByTokenType(t *testing.T) { require.NoError(t, err) // Two MT for which ChainID is ignored and two transfers on the main net and Goerli require.Equal(t, 4, len(entries)) - require.Equal(t, Erc20, entries[0].tokenOut.TokenType) - require.Equal(t, transfer.UsdcGoerli.Address, entries[0].tokenOut.Address) - require.Nil(t, entries[0].tokenIn) + require.Equal(t, Erc20, entries[0].tokenIn.TokenType) + require.Equal(t, transfer.UsdcGoerli.Address, entries[0].tokenIn.Address) + require.Nil(t, entries[0].tokenOut) } func TestGetActivityEntriesFilterByToAddresses(t *testing.T) { @@ -1138,19 +1130,19 @@ func TestGetActivityEntriesCheckToAndFrom(t *testing.T) { transfer.InsertTestTransfer(t, deps.db, trs[0].To, &trs[0]) transfer.InsertTestPendingTransaction(t, deps.db, &trs[1]) - addresses := []eth.Address{td.tr1.From, td.pendingTr.From, - td.multiTx1.FromAddress, td.multiTx2.ToAddress, trs[0].To, trs[1].To} + addresses := []eth.Address{td.tr1.To, td.pendingTr.To, + td.multiTx1.FromAddress, td.multiTx2.FromAddress, trs[0].To, trs[1].To} var filter Filter entries, err := getActivityEntries(context.Background(), deps, addresses, []common.ChainID{}, filter, 0, 15) require.NoError(t, err) require.Equal(t, 6, len(entries)) - require.Equal(t, SendAT, entries[5].activityType) // td.tr1 + require.Equal(t, ReceiveAT, entries[5].activityType) // td.tr1 require.NotEqual(t, eth.Address{}, entries[5].transaction.Address) // td.tr1 require.Equal(t, td.tr1.To, *entries[5].recipient) // td.tr1 - require.Equal(t, SendAT, entries[4].activityType) // td.pendingTr + require.Equal(t, ReceiveAT, entries[4].activityType) // td.pendingTr // Multi-transactions are always considered as SendAT require.Equal(t, SendAT, entries[3].activityType) // td.multiTx1 @@ -1161,9 +1153,6 @@ func TestGetActivityEntriesCheckToAndFrom(t *testing.T) { require.Equal(t, trs[0].To, entries[1].transaction.Address) // trs[0] require.Equal(t, ReceiveAT, entries[0].activityType) // trs[1] (pending) - - // TODO: add accounts to DB for proper detection of sender/receiver - // TODO: Test with all addresses } func TestGetActivityEntriesCheckContextCancellation(t *testing.T) { diff --git a/services/wallet/activity/filter.sql b/services/wallet/activity/filter.sql index 33ca516a9..f62a85e6d 100644 --- a/services/wallet/activity/filter.sql +++ b/services/wallet/activity/filter.sql @@ -27,6 +27,7 @@ WITH filter_conditions AS ( ? AS filterActivityTypeReceive, ? AS filterActivityTypeContractDeployment, ? AS filterActivityTypeMint, + ? AS mTTypeSend, ? AS fromTrType, ? AS toTrType, ? AS filterAllAddresses, @@ -184,7 +185,7 @@ SELECT FROM transfers, filter_conditions - LEFT JOIN filter_addresses from_join ON HEX(transfers.tx_from_address) = from_join.address + LEFT JOIN filter_addresses from_join ON HEX(transfers.address) = from_join.address LEFT JOIN filter_addresses to_join ON HEX(transfers.tx_to_address) = to_join.address WHERE transfers.loaded == 1 @@ -206,10 +207,17 @@ WHERE filterActivityTypeSend AND ( filterAllAddresses - OR (HEX(transfers.tx_from_address) IN filter_addresses) + OR ( + HEX(transfers.tx_from_address) IN filter_addresses + ) + ) + AND NOT ( + tr_type = fromTrType + and transfers.tx_to_address IS NULL + AND transfers.type = "eth" + AND transfers.contract_address IS NOT NULL + AND HEX(transfers.contract_address) != zeroAddress ) - AND NOT (tr_type = fromTrType and transfers.tx_to_address IS NULL AND transfers.type = "eth" - AND transfers.contract_address IS NOT NULL AND HEX(transfers.contract_address) != zeroAddress) ) OR ( filterActivityTypeReceive @@ -217,14 +225,22 @@ WHERE filterAllAddresses OR (HEX(transfers.tx_to_address) IN filter_addresses) ) - AND NOT (tr_type = toTrType AND transfers.type = "erc721" AND ( - transfers.tx_from_address IS NULL - OR HEX(transfers.tx_from_address) = zeroAddress)) + AND NOT ( + tr_type = toTrType + AND transfers.type = "erc721" + AND ( + transfers.tx_from_address IS NULL + OR HEX(transfers.tx_from_address) = zeroAddress + ) + ) ) OR ( filterActivityTypeContractDeployment - AND tr_type = fromTrType AND transfers.tx_to_address IS NULL AND transfers.type = "eth" - AND transfers.contract_address IS NOT NULL AND HEX(transfers.contract_address) != zeroAddress + AND tr_type = fromTrType + AND transfers.tx_to_address IS NULL + AND transfers.type = "eth" + AND transfers.contract_address IS NOT NULL + AND HEX(transfers.contract_address) != zeroAddress AND ( filterAllAddresses OR (HEX(transfers.address) IN filter_addresses) @@ -232,10 +248,13 @@ WHERE ) OR ( filterActivityTypeMint - AND tr_type = toTrType AND transfers.type = "erc721" AND ( - transfers.tx_from_address IS NULL + AND tr_type = toTrType + AND transfers.type = "erc721" + AND ( + transfers.tx_from_address IS NULL OR HEX(transfers.tx_from_address) = zeroAddress - ) AND ( + ) + AND ( filterAllAddresses OR (HEX(transfers.address) IN filter_addresses) ) @@ -243,10 +262,7 @@ WHERE ) AND ( filterAllAddresses - OR ( - HEX(transfers.tx_from_address) IN filter_addresses - ) - OR (HEX(transfers.tx_to_address) IN filter_addresses) + OR (HEX(owner_address) IN filter_addresses) ) AND ( filterAllToAddresses @@ -391,7 +407,7 @@ SELECT NULL as tr_type, multi_transactions.from_address AS from_address, multi_transactions.to_address AS to_address, - NULL AS owner_address, + multi_transactions.from_address AS owner_address, NULL AS tr_amount, multi_transactions.from_amount AS mt_from_amount, multi_transactions.to_amount AS mt_to_amount, @@ -434,10 +450,16 @@ WHERE AND ( filterAllAddresses OR ( - HEX(multi_transactions.from_address) IN filter_addresses + -- Send multi-transaction types are exclusively for outbound transfers. The receiving end will have a corresponding entry as "owner_address" in the transfers table. + mt_type = mTTypeSend + AND HEX(owner_address) IN filter_addresses ) OR ( - HEX(multi_transactions.to_address) IN filter_addresses + mt_type != mTTypeSend + AND ( + HEX(multi_transactions.from_address) IN filter_addresses + OR HEX(multi_transactions.to_address) IN filter_addresses + ) ) ) AND ( diff --git a/services/wallet/transfer/commands.go b/services/wallet/transfer/commands.go index c0a1355ee..f7653d9c1 100644 --- a/services/wallet/transfer/commands.go +++ b/services/wallet/transfer/commands.go @@ -2,7 +2,7 @@ package transfer import ( "context" - "fmt" + "database/sql" "math/big" "strings" "time" @@ -452,24 +452,63 @@ func (c *transfersCommand) Run(ctx context.Context) (err error) { return nil } +// saveAndConfirmPending ensures only the transaction that has owner (Address) as a sender is matched to the +// corresponding multi-transaction (by multi-transaction ID). This way we ensure that if receiver is in the list +// of accounts filter will discard the proper one func (c *transfersCommand) saveAndConfirmPending(allTransfers []Transfer, blockNum *big.Int) error { - tx, err := c.db.client.Begin() - if err != nil { - return err + tx, resErr := c.db.client.Begin() + if resErr != nil { + return resErr } - notifyFunctions := make([]func(), 0) - // Confirm all pending transactions that are included in this block - for i, transfer := range allTransfers { - txType, MTID, err := transactions.GetTransferData(tx, w_common.ChainID(transfer.NetworkID), transfer.Receipt.TxHash) - if err != nil { - log.Error("GetTransferData error", "error", err) + defer func() { + if resErr == nil { + commitErr := tx.Commit() + if commitErr != nil { + log.Error("failed to commit", "error", commitErr) + } + for _, notify := range notifyFunctions { + notify() + } + } else { + rollbackErr := tx.Rollback() + if rollbackErr != nil { + log.Error("failed to rollback", "error", rollbackErr) + } } - if MTID != nil { - allTransfers[i].MultiTransactionID = MultiTransactionIDType(*MTID) + }() + + // Confirm all pending transactions that are included in this block + for i, tr := range allTransfers { + chainID := w_common.ChainID(tr.NetworkID) + txHash := tr.Receipt.TxHash + txType, mTID, err := transactions.GetOwnedPendingStatus(tx, chainID, txHash, tr.Address) + if err == sql.ErrNoRows { + if tr.MultiTransactionID > 0 { + continue + } else { + // Outside transaction, already confirmed by another duplicate or not yet downloaded + existingMTID, err := GetOwnedMultiTransactionID(tx, chainID, tr.ID, tr.Address) + if err == sql.ErrNoRows || existingMTID == 0 { + // Outside transaction, ignore it + continue + } else if err != nil { + log.Warn("GetOwnedMultiTransactionID", "error", err) + continue + } + mTID = w_common.NewAndSet(existingMTID) + + } + } else if err != nil { + log.Warn("GetOwnedPendingStatus", "error", err) + continue + } + + if mTID != nil { + allTransfers[i].MultiTransactionID = MultiTransactionIDType(*mTID) } if txType != nil && *txType == transactions.WalletTransfer { - notify, err := c.pendingTxManager.DeleteBySQLTx(tx, w_common.ChainID(transfer.NetworkID), transfer.Receipt.TxHash) + notify, err := c.pendingTxManager.DeleteBySQLTx(tx, chainID, txHash) if err != nil && err != transactions.ErrStillPending { log.Error("DeleteBySqlTx error", "error", err) } @@ -477,27 +516,12 @@ func (c *transfersCommand) saveAndConfirmPending(allTransfers []Transfer, blockN } } - err = saveTransfersMarkBlocksLoaded(tx, c.chainClient.ChainID, c.address, allTransfers, []*big.Int{blockNum}) - if err != nil { - log.Error("SaveTransfers error", "error", err) - return err + resErr = saveTransfersMarkBlocksLoaded(tx, c.chainClient.ChainID, c.address, allTransfers, []*big.Int{blockNum}) + if resErr != nil { + log.Error("SaveTransfers error", "error", resErr) } - if err == nil { - err = tx.Commit() - if err != nil { - return err - } - for _, notify := range notifyFunctions { - notify() - } - } else { - err = tx.Rollback() - if err != nil { - return fmt.Errorf("failed to rollback: %w", err) - } - } - return nil + return resErr } // Mark all subTxs of a given Tx with the same multiTxID diff --git a/services/wallet/transfer/database.go b/services/wallet/transfer/database.go index f85350776..5090ed14c 100644 --- a/services/wallet/transfer/database.go +++ b/services/wallet/transfer/database.go @@ -573,3 +573,13 @@ func markBlocksAsLoaded(chainID uint64, creator statementCreator, address common } return nil } + +// GetOwnedMultiTransactionID returns sql.ErrNoRows if no transaction is found for the given identity +func GetOwnedMultiTransactionID(tx *sql.Tx, chainID w_common.ChainID, id common.Hash, address common.Address) (mTID int64, err error) { + row := tx.QueryRow(`SELECT COALESCE(multi_transaction_id, 0) FROM transfers WHERE network_id = ? AND hash = ? AND address = ?`, chainID, id, address) + err = row.Scan(&mTID) + if err != nil { + return 0, err + } + return mTID, nil +} diff --git a/services/wallet/transfer/iterative.go b/services/wallet/transfer/iterative.go index 468bf9304..2fd54c76b 100644 --- a/services/wallet/transfer/iterative.go +++ b/services/wallet/transfer/iterative.go @@ -67,7 +67,7 @@ func (d *IterativeDownloader) Next(parent context.Context) ([]*DBHeader, *big.In headers, err := d.downloader.GetHeadersInRange(parent, from, to) log.Info("load erc20 transfers in range", "from", from, "to", to, "batchSize", d.batchSize) if err != nil { - log.Error("failed to get transfer in between two bloks", "from", from, "to", to, "error", err) + log.Error("failed to get transfer in between two blocks", "from", from, "to", to, "error", err) return nil, nil, nil, err } diff --git a/transactions/pendingtxtracker.go b/transactions/pendingtxtracker.go index 890e3b9dd..bde7b279a 100644 --- a/transactions/pendingtxtracker.go +++ b/transactions/pendingtxtracker.go @@ -561,15 +561,16 @@ func (tm *PendingTxTracker) DeleteBySQLTx(tx *sql.Tx, chainID common.ChainID, ha }, err } -func GetTransferData(tx *sql.Tx, chainID common.ChainID, hash eth.Hash) (txType *PendingTrxType, MTID *int64, err error) { - row := tx.QueryRow(`SELECT type, multi_transaction_id FROM pending_transactions WHERE network_id = ? AND hash = ?`, chainID, hash, txType) +// GetOwnedPendingStatus returns sql.ErrNoRows if no pending transaction is found for the given identity +func GetOwnedPendingStatus(tx *sql.Tx, chainID common.ChainID, hash eth.Hash, ownerAddress eth.Address) (txType *PendingTrxType, mTID *int64, err error) { + row := tx.QueryRow(`SELECT type, multi_transaction_id FROM pending_transactions WHERE network_id = ? AND hash = ? AND from_address = ?`, chainID, hash, ownerAddress) txType = new(PendingTrxType) - MTID = new(int64) - err = row.Scan(txType, MTID) + mTID = new(int64) + err = row.Scan(txType, mTID) if err != nil { return nil, nil, err } - return txType, MTID, nil + return txType, mTID, nil } // Watch returns sql.ErrNoRows if no pending transaction is found for the given identity