From 321c85dd8760fef00c9247d84f84db6e731a280c Mon Sep 17 00:00:00 2001 From: andrepimenta Date: Mon, 15 Feb 2021 16:14:51 +0000 Subject: [PATCH 1/6] Optimize adding multiple tokens at the same time --- src/assets/AssetsController.ts | 37 +++++++++++++++++++++++++ src/assets/AssetsDetectionController.ts | 14 ++++++---- 2 files changed, 46 insertions(+), 5 deletions(-) diff --git a/src/assets/AssetsController.ts b/src/assets/AssetsController.ts index 61a504b1981..0c4c98e62bd 100644 --- a/src/assets/AssetsController.ts +++ b/src/assets/AssetsController.ts @@ -586,6 +586,43 @@ export class AssetsController extends BaseController } } + /** + * Adds a batch of tokens to the stored token list + * + * @param tokens - Array of Tokens to be added or updated + * @returns - Current token list + */ + async addTokens(tokensToAdd: Token[]): Promise { + const releaseLock = await this.mutex.acquire(); + const { allTokens, tokens } = this.state; + const { networkType, selectedAddress } = this.config; + + try { + tokensToAdd.forEach((tokenToAdd) => { + const { address, symbol, decimals, image } = tokenToAdd; + const checksumAddress = toChecksumAddress(address); + + const newEntry: Token = { address: checksumAddress, symbol, decimals, image }; + const previousEntry = tokens.find((token) => token.address === checksumAddress); + if (previousEntry) { + const previousIndex = tokens.indexOf(previousEntry); + tokens[previousIndex] = newEntry; + } else { + tokens.push(newEntry); + } + }); + + const addressTokens = allTokens[selectedAddress]; + const newAddressTokens = { ...addressTokens, ...{ [networkType]: tokens } }; + const newAllTokens = { ...allTokens, ...{ [selectedAddress]: newAddressTokens } }; + const newTokens = [...tokens]; + this.update({ allTokens: newAllTokens, tokens: newTokens }); + return newTokens; + } finally { + releaseLock(); + } + } + /** * Adds a new suggestedAsset to state. Parameters will be validated according to * asset type being watched. A `:pending` hub event will be emitted once added. diff --git a/src/assets/AssetsDetectionController.ts b/src/assets/AssetsDetectionController.ts index 200ebb1a29f..9375b0fcfa3 100644 --- a/src/assets/AssetsDetectionController.ts +++ b/src/assets/AssetsDetectionController.ts @@ -173,6 +173,7 @@ export class AssetsDetectionController extends BaseController token.address === toChecksumAddress(tokenAddress)); } if (!ignored) { - await assetsController.addToken( - tokenAddress, - contractMap[tokenAddress].symbol, - contractMap[tokenAddress].decimals, - ); + tokensToAdd.push({ + address: tokenAddress, + decimals: contractMap[tokenAddress].decimals, + symbol: contractMap[tokenAddress].symbol, + }); } } + if (tokensToAdd.length) { + await assetsController.addTokens(tokensToAdd); + } }); } From 486671f787d2f0032402795adb89e44e21009ad1 Mon Sep 17 00:00:00 2001 From: andrepimenta Date: Mon, 15 Feb 2021 16:15:48 +0000 Subject: [PATCH 2/6] Add chainId to transactions --- src/transaction/TransactionController.ts | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/src/transaction/TransactionController.ts b/src/transaction/TransactionController.ts index 243dd1cc04d..a1e2f0b201e 100644 --- a/src/transaction/TransactionController.ts +++ b/src/transaction/TransactionController.ts @@ -100,6 +100,7 @@ export interface TransactionMeta { }; id: string; networkID?: string; + chainId?: string; origin?: string; rawTransaction?: string; status: string; @@ -255,7 +256,7 @@ export class TransactionController extends BaseController { + normalizeTxFromAlehio = (txMeta: AlethioTransactionMeta, currentNetworkID: string, currentChainId: string): TransactionMeta => { const { attributes: { symbol, blockCreationTime, decimals, transactionGasLimit, transactionGasPrice, value }, relationships: { to, from, transaction, token }, @@ -296,6 +298,7 @@ export class TransactionController extends BaseController Promise.all( transactions.map(async (meta, index) => { - if (meta.status === 'submitted' && meta.networkID === currentNetworkID) { + if (meta.status === 'submitted' && (meta.networkID === currentChainId || meta.chainId === currentChainId)) { const txObj = await query(this.ethQuery, 'getTransactionByHash', [meta.transactionHash]); /* istanbul ignore else */ if (txObj && txObj.blockNumber) { @@ -721,8 +726,8 @@ export class TransactionController extends BaseController networkID !== currentNetworkID); + const currentChainId = network.state.provider.chainId; + const newTransactions = this.state.transactions.filter(({ networkID, chainId }) => ((networkID !== undefined && networkID !== currentChainId) || (chainId !== undefined && chainId !== currentChainId))); this.update({ transactions: newTransactions }); } @@ -739,6 +744,7 @@ export class TransactionController extends BaseController { /* istanbul ignore next */ if (!remoteTxList[tx.hash]) { - remoteTxs.push(this.normalizeTxFromEtherscan(tx, currentNetworkID)); + remoteTxs.push(this.normalizeTxFromEtherscan(tx, currentNetworkID, currentChainId)); remoteTxList[tx.hash] = 1; } }); alethioResponse.data.forEach((tx: AlethioTransactionMeta) => { - const cleanTx = this.normalizeTxFromAlehio(tx, currentNetworkID); + const cleanTx = this.normalizeTxFromAlehio(tx, currentNetworkID, currentChainId); remoteTxs.push(cleanTx); /* istanbul ignore next */ remoteTxList[cleanTx.transactionHash || ''] = 1; From c405b2175599cb51010f575196019e317a50d2ea Mon Sep 17 00:00:00 2001 From: andrepimenta Date: Thu, 18 Feb 2021 14:21:25 +0000 Subject: [PATCH 3/6] Prefer chainId over networkID --- src/transaction/TransactionController.ts | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/transaction/TransactionController.ts b/src/transaction/TransactionController.ts index a1e2f0b201e..813515b97fd 100644 --- a/src/transaction/TransactionController.ts +++ b/src/transaction/TransactionController.ts @@ -254,6 +254,7 @@ export class TransactionController extends BaseController { @@ -410,7 +412,7 @@ export class TransactionController extends BaseController Promise.all( transactions.map(async (meta, index) => { - if (meta.status === 'submitted' && (meta.networkID === currentChainId || meta.chainId === currentChainId)) { + if (meta.status === 'submitted' && (meta.chainId === currentChainId || (!meta.chainId && meta.networkID === currentNetworkID))) { const txObj = await query(this.ethQuery, 'getTransactionByHash', [meta.transactionHash]); /* istanbul ignore else */ if (txObj && txObj.blockNumber) { @@ -727,7 +730,8 @@ export class TransactionController extends BaseController ((networkID !== undefined && networkID !== currentChainId) || (chainId !== undefined && chainId !== currentChainId))); + const currentNetworkID = network.state.network; + const newTransactions = this.state.transactions.filter(({ networkID, chainId }) => ((chainId !== currentChainId) || (!chainId && networkID !== currentNetworkID))); this.update({ transactions: newTransactions }); } @@ -744,8 +748,7 @@ export class TransactionController extends BaseController { /* istanbul ignore next */ if ( - tx.networkID === currentNetworkID && + (tx.chainId === currentChainId || (!tx.chainId && tx.networkID === currentNetworkID)) && tx.transaction.to && tx.transaction.to.toLowerCase() === address.toLowerCase() ) { From d3c71b19d18e8fa644f33deb58bcf9a35d5fa6ef Mon Sep 17 00:00:00 2001 From: andrepimenta Date: Thu, 18 Feb 2021 14:37:55 +0000 Subject: [PATCH 4/6] Add test for addTokens --- tests/AssetsController.test.ts | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/tests/AssetsController.test.ts b/tests/AssetsController.test.ts index c224945dfce..417e4c76985 100644 --- a/tests/AssetsController.test.ts +++ b/tests/AssetsController.test.ts @@ -110,6 +110,37 @@ describe('AssetsController', () => { }); }); + it('should add tokens', async () => { + await assetsController.addTokens([ + { address: 'addressA', symbol: 'barA', decimals: 2 }, + { address: 'addressB', symbol: 'barB', decimals: 2 }, + ]); + expect(assetsController.state.tokens[0]).toEqual({ + address: '0xAdDRessA', + decimals: 2, + symbol: 'barA', + }); + expect(assetsController.state.tokens[1]).toEqual({ + address: '0xAddReSSB', + decimals: 2, + symbol: 'barB', + }); + await assetsController.addTokens([ + { address: 'addressA', symbol: 'bazA', decimals: 2 }, + { address: 'addressB', symbol: 'bazB', decimals: 2 }, + ]); + expect(assetsController.state.tokens[0]).toEqual({ + address: '0xAdDRessA', + decimals: 2, + symbol: 'bazA', + }); + expect(assetsController.state.tokens[1]).toEqual({ + address: '0xAddReSSB', + decimals: 2, + symbol: 'bazB', + }); + }); + it('should add token by selected address', async () => { const firstAddress = '0x123'; const secondAddress = '0x321'; From 31751af9226ed2b7ccd5f2c97cb4d19fd5fceb41 Mon Sep 17 00:00:00 2001 From: andrepimenta Date: Fri, 19 Feb 2021 11:47:20 +0000 Subject: [PATCH 5/6] Add tests for testing fallback to networkID --- src/transaction/TransactionController.ts | 18 +++++++-- tests/TransactionController.test.ts | 49 ++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 4 deletions(-) diff --git a/src/transaction/TransactionController.ts b/src/transaction/TransactionController.ts index 813515b97fd..497f2a5403e 100644 --- a/src/transaction/TransactionController.ts +++ b/src/transaction/TransactionController.ts @@ -411,9 +411,12 @@ export class TransactionController extends BaseController Promise.all( transactions.map(async (meta, index) => { + // Using fallback to networkID only when there is no chainId present. Should be removed when networkID is completely removed. if (meta.status === 'submitted' && (meta.chainId === currentChainId || (!meta.chainId && meta.networkID === currentNetworkID))) { const txObj = await query(this.ethQuery, 'getTransactionByHash', [meta.transactionHash]); /* istanbul ignore else */ @@ -731,7 +735,12 @@ export class TransactionController extends BaseController ((chainId !== currentChainId) || (!chainId && networkID !== currentNetworkID))); + const newTransactions = this.state.transactions.filter(({ networkID, chainId }) => { + // 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)); + return !isCurrentNetwork; + }); + this.update({ transactions: newTransactions }); } @@ -789,6 +798,7 @@ export class TransactionController extends BaseController { /* istanbul ignore next */ if ( + // Using fallback to networkID only when there is no chainId present. Should be removed when networkID is completely removed. (tx.chainId === currentChainId || (!tx.chainId && tx.networkID === currentNetworkID)) && tx.transaction.to && tx.transaction.to.toLowerCase() === address.toLowerCase() diff --git a/tests/TransactionController.test.ts b/tests/TransactionController.test.ts index 5f4e81bcc52..da8a6fdd44a 100644 --- a/tests/TransactionController.test.ts +++ b/tests/TransactionController.test.ts @@ -442,6 +442,7 @@ describe('TransactionController', () => { }); expect(controller.state.transactions[0].transaction.from).toBe(from); expect(controller.state.transactions[0].networkID).toBe(MOCK_NETWORK.state.network); + expect(controller.state.transactions[0].chainId).toBe(MOCK_NETWORK.state.provider.chainId); expect(controller.state.transactions[0].status).toBe('unapproved'); }); @@ -486,6 +487,25 @@ describe('TransactionController', () => { expect(controller.state.transactions).toHaveLength(0); }); + // This tests the fallback to networkID only when there is no chainId present. Should be removed when networkID is completely removed. + it('should wipe transactions using networkID when there is no chainId', async () => { + const controller = new TransactionController({ provider: PROVIDER }); + controller.wipeTransactions(); + controller.context = { + NetworkController: MOCK_NETWORK, + } as any; + controller.onComposed(); + controller.state.transactions.push({ + from: MOCK_PRFERENCES.state.selectedAddress, + id: 'foo', + networkID: '3', + status: 'submitted', + transactionHash: '1337', + } as any); + controller.wipeTransactions(); + expect(controller.state.transactions).toHaveLength(0); + }); + it('should fail to approve an invalid transaction', () => { return new Promise(async (resolve) => { const controller = new TransactionController({ @@ -612,6 +632,35 @@ describe('TransactionController', () => { }); it('should query transaction statuses', () => { + return new Promise((resolve) => { + const controller = new TransactionController({ + provider: PROVIDER, + sign: async (transaction: any) => transaction, + }); + controller.context = { + NetworkController: MOCK_NETWORK, + } as any; + controller.onComposed(); + controller.state.transactions.push({ + from: MOCK_PRFERENCES.state.selectedAddress, + id: 'foo', + networkID: '3', + chainId: '3', + status: 'submitted', + transactionHash: '1337', + } as any); + controller.state.transactions.push({} as any); + + controller.hub.once(`${controller.state.transactions[0].id}:confirmed`, () => { + expect(controller.state.transactions[0].status).toBe('confirmed'); + resolve(''); + }); + controller.queryTransactionStatuses(); + }); + }); + + // This tests the fallback to networkID only when there is no chainId present. Should be removed when networkID is completely removed. + it('should query transaction statuses with networkID only when there is no chainId', () => { return new Promise((resolve) => { const controller = new TransactionController({ provider: PROVIDER, From 5aa193746680f7c331f3762050d699a25c69b87b Mon Sep 17 00:00:00 2001 From: andrepimenta Date: Fri, 19 Feb 2021 11:50:41 +0000 Subject: [PATCH 6/6] Run prettier --- tests/TransactionController.test.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/TransactionController.test.ts b/tests/TransactionController.test.ts index da8a6fdd44a..ed160774df0 100644 --- a/tests/TransactionController.test.ts +++ b/tests/TransactionController.test.ts @@ -496,12 +496,12 @@ describe('TransactionController', () => { } as any; controller.onComposed(); controller.state.transactions.push({ - from: MOCK_PRFERENCES.state.selectedAddress, - id: 'foo', - networkID: '3', - status: 'submitted', - transactionHash: '1337', - } as any); + from: MOCK_PRFERENCES.state.selectedAddress, + id: 'foo', + networkID: '3', + status: 'submitted', + transactionHash: '1337', + } as any); controller.wipeTransactions(); expect(controller.state.transactions).toHaveLength(0); });