From b28929060e2db8de7eb31af46e159fe84ddb3fd5 Mon Sep 17 00:00:00 2001 From: Goktug Poyraz Date: Thu, 3 Aug 2023 11:46:11 +0200 Subject: [PATCH 1/5] Adds a support of wiping transactions with specific address on current chain --- .../src/TransactionController.test.ts | 55 +++++++++++++++++++ .../src/TransactionController.ts | 10 +++- 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index ca3fe9c4059..a80cfdeef8f 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -1049,6 +1049,61 @@ describe('TransactionController', () => { expect(controller.state.transactions).toHaveLength(0); }); + + it('removes only txs from given address on current network', async () => { + const controller = newController(); + + controller.wipeTransactions(); + + const mockFromAccount1 = '0x1bf137f335ea1b8f193b8f6ea92561a60d23a207'; + const mockFromAccount2 = '0x2bf137f335ea1b8f193b8f6ea92561a60d23a207'; + const mockDifferentChainId = toHex(1); + const mockCurrentChainId = toHex(5); + + controller.state.transactions.push({ + id: '1', + chainId: mockCurrentChainId, + transaction: { + from: mockFromAccount1, + }, + } as any); + + controller.state.transactions.push({ + id: '2', + chainId: mockCurrentChainId, + transaction: { + from: mockFromAccount2, + }, + } as any); + + controller.state.transactions.push({ + id: '3', + chainId: mockCurrentChainId, + transaction: { + from: mockFromAccount2, + }, + } as any); + + controller.state.transactions.push({ + id: '4', + chainId: mockDifferentChainId, + transaction: { + from: mockFromAccount2, + }, + } as any); + + controller.wipeTransactions(false, mockFromAccount2); + + const remainingTxsOnCurrentChain = controller.state.transactions.filter( + (tx) => tx.chainId === mockCurrentChainId, + ); + const remainingTxsOnDifferentChain = controller.state.transactions.filter( + (tx) => tx.chainId === mockDifferentChainId, + ); + + expect(remainingTxsOnCurrentChain).toHaveLength(1); + expect(remainingTxsOnDifferentChain).toHaveLength(1); + }); }); describe('queryTransactionStatus', () => { diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index 22ad692a323..80a8904891d 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -789,8 +789,10 @@ export class TransactionController extends BaseController< * * @param ignoreNetwork - Determines whether to wipe all transactions, or just those on the * current network. If `true`, all transactions are wiped. + * @param address - If specified, only transactions originating from this address will be + * wiped on current network. */ - wipeTransactions(ignoreNetwork?: boolean) { + wipeTransactions(ignoreNetwork?: boolean, address?: string) { /* istanbul ignore next */ if (ignoreNetwork) { this.update({ transactions: [] }); @@ -800,11 +802,15 @@ export class TransactionController extends BaseController< this.getNetworkState(); const { chainId: currentChainId } = providerConfig; const newTransactions = this.state.transactions.filter( - ({ networkID, chainId }) => { + ({ networkID, chainId, transaction }) => { // Using fallback to networkID only when there is no chainId present. Should be removed when networkID is completely removed. const isCurrentNetwork = chainId === currentChainId || (!chainId && networkID === currentNetworkID); + if (address) { + const isFromAddress = transaction.from === address; + return !(isCurrentNetwork && isFromAddress); + } return !isCurrentNetwork; }, ); From 0cb0b595653e67f5263eb358751c3b727265788b Mon Sep 17 00:00:00 2001 From: Goktug Poyraz Date: Thu, 3 Aug 2023 11:52:47 +0200 Subject: [PATCH 2/5] typo --- .../src/TransactionController.test.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index a80cfdeef8f..1b940eab68e 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -220,7 +220,7 @@ function waitForTransactionFinished( }); } -const MOCK_PRFERENCES = { state: { selectedAddress: 'foo' } }; +const MOCK_PREFERENCES = { state: { selectedAddress: 'foo' } }; const INFURA_PROJECT_ID = '341eacb578dd44a1a049cbc5f6fd4035'; const GOERLI_PROVIDER = new HttpProvider( `https://goerli.infura.io/v3/${INFURA_PROJECT_ID}`, @@ -1038,7 +1038,7 @@ describe('TransactionController', () => { controller.wipeTransactions(); controller.state.transactions.push({ - from: MOCK_PRFERENCES.state.selectedAddress, + from: MOCK_PREFERENCES.state.selectedAddress, id: 'foo', networkID: '5', status: TransactionStatus.submitted, @@ -1050,7 +1050,7 @@ describe('TransactionController', () => { expect(controller.state.transactions).toHaveLength(0); }); - it('removes only txs from given address on current network', async () => { + it('removes only txs with given address on current network', async () => { const controller = newController(); controller.wipeTransactions(); @@ -1111,7 +1111,7 @@ describe('TransactionController', () => { const controller = newController(); controller.state.transactions.push({ - from: MOCK_PRFERENCES.state.selectedAddress, + from: MOCK_PREFERENCES.state.selectedAddress, id: 'foo', networkID: '5', chainId: toHex(5), @@ -1137,7 +1137,7 @@ describe('TransactionController', () => { const controller = newController(); controller.state.transactions.push({ - from: MOCK_PRFERENCES.state.selectedAddress, + from: MOCK_PREFERENCES.state.selectedAddress, id: 'foo', networkID: '5', status: TransactionStatus.submitted, @@ -1160,7 +1160,7 @@ describe('TransactionController', () => { const controller = newController(); controller.state.transactions.push({ - from: MOCK_PRFERENCES.state.selectedAddress, + from: MOCK_PREFERENCES.state.selectedAddress, id: 'foo', networkID: '5', status: TransactionStatus.submitted, @@ -1177,7 +1177,7 @@ describe('TransactionController', () => { const controller = newController(); controller.state.transactions.push({ - from: MOCK_PRFERENCES.state.selectedAddress, + from: MOCK_PREFERENCES.state.selectedAddress, id: 'foo', networkID: '5', chainId: toHex(5), From 3bc6f0492df2b409f0e4cabbb005b0e81588c3cd Mon Sep 17 00:00:00 2001 From: Goktug Poyraz Date: Thu, 3 Aug 2023 13:25:17 +0200 Subject: [PATCH 3/5] Add toLowerCase before comparing addresses --- packages/transaction-controller/src/TransactionController.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index 80a8904891d..0a1f0371aff 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -794,7 +794,7 @@ export class TransactionController extends BaseController< */ wipeTransactions(ignoreNetwork?: boolean, address?: string) { /* istanbul ignore next */ - if (ignoreNetwork) { + if (ignoreNetwork && !address) { this.update({ transactions: [] }); return; } @@ -808,7 +808,7 @@ export class TransactionController extends BaseController< chainId === currentChainId || (!chainId && networkID === currentNetworkID); if (address) { - const isFromAddress = transaction.from === address; + const isFromAddress = transaction.from?.toLowerCase() === address.toLowerCase(); return !(isCurrentNetwork && isFromAddress); } return !isCurrentNetwork; From c0494727b9c330c8af695ddb55047fad8fe556ec Mon Sep 17 00:00:00 2001 From: Goktug Poyraz Date: Thu, 3 Aug 2023 13:39:58 +0200 Subject: [PATCH 4/5] Fix comments --- .../src/TransactionController.test.ts | 37 +++++++++++-------- .../src/TransactionController.ts | 15 +++++--- 2 files changed, 32 insertions(+), 20 deletions(-) diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index 1b940eab68e..c9d619c0be8 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -1050,14 +1050,13 @@ describe('TransactionController', () => { expect(controller.state.transactions).toHaveLength(0); }); - it('removes only txs with given address on current network', async () => { + it('removes only txs with given address', async () => { const controller = newController(); controller.wipeTransactions(); const mockFromAccount1 = '0x1bf137f335ea1b8f193b8f6ea92561a60d23a207'; const mockFromAccount2 = '0x2bf137f335ea1b8f193b8f6ea92561a60d23a207'; - const mockDifferentChainId = toHex(1); const mockCurrentChainId = toHex(5); controller.state.transactions.push({ @@ -1076,11 +1075,26 @@ describe('TransactionController', () => { }, } as any); + controller.wipeTransactions(false, mockFromAccount2); + + expect(controller.state.transactions).toHaveLength(1); + expect(controller.state.transactions[0].id).toBe('1'); + }); + + it('removes only txs with given address only on current network', async () => { + const controller = newController(); + + controller.wipeTransactions(); + + const mockFromAccount1 = '0x1bf137f335ea1b8f193b8f6ea92561a60d23a207'; + const mockDifferentChainId = toHex(1); + const mockCurrentChainId = toHex(5); + controller.state.transactions.push({ - id: '3', + id: '1', chainId: mockCurrentChainId, transaction: { - from: mockFromAccount2, + from: mockFromAccount1, }, } as any); @@ -1088,21 +1102,14 @@ describe('TransactionController', () => { id: '4', chainId: mockDifferentChainId, transaction: { - from: mockFromAccount2, + from: mockFromAccount1, }, } as any); - controller.wipeTransactions(false, mockFromAccount2); - - const remainingTxsOnCurrentChain = controller.state.transactions.filter( - (tx) => tx.chainId === mockCurrentChainId, - ); - const remainingTxsOnDifferentChain = controller.state.transactions.filter( - (tx) => tx.chainId === mockDifferentChainId, - ); + controller.wipeTransactions(false, mockFromAccount1); - expect(remainingTxsOnCurrentChain).toHaveLength(1); - expect(remainingTxsOnDifferentChain).toHaveLength(1); + expect(controller.state.transactions).toHaveLength(1); + expect(controller.state.transactions[0].id).toBe('4'); }); }); diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index 0a1f0371aff..c20a993625c 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -804,14 +804,19 @@ export class TransactionController extends BaseController< const newTransactions = this.state.transactions.filter( ({ networkID, chainId, transaction }) => { // Using fallback to networkID only when there is no chainId present. Should be removed when networkID is completely removed. - const isCurrentNetwork = + const isMatchingNetwork = + ignoreNetwork || chainId === currentChainId || (!chainId && networkID === currentNetworkID); - if (address) { - const isFromAddress = transaction.from?.toLowerCase() === address.toLowerCase(); - return !(isCurrentNetwork && isFromAddress); + + if (!isMatchingNetwork) { + return true; } - return !isCurrentNetwork; + + const isMatchingAddress = + !address || transaction.from?.toLowerCase() === address.toLowerCase(); + + return !isMatchingAddress; }, ); From 6fc6c38d8823d3fdebdaf147c310d6af13ba4ca9 Mon Sep 17 00:00:00 2001 From: Goktug Poyraz Date: Mon, 14 Aug 2023 09:16:54 +0200 Subject: [PATCH 5/5] fix test case --- .../transaction-controller/src/TransactionController.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index c9d619c0be8..c244d0e33a0 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -1075,7 +1075,7 @@ describe('TransactionController', () => { }, } as any); - controller.wipeTransactions(false, mockFromAccount2); + controller.wipeTransactions(true, mockFromAccount2); expect(controller.state.transactions).toHaveLength(1); expect(controller.state.transactions[0].id).toBe('1');