From 37517d53445082fd265b621ed09620491ee24ed0 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Fri, 14 Oct 2022 12:31:54 +0200 Subject: [PATCH 1/6] Reduce the amount of calls to insight snaps --- ui/hooks/flask/useTransactionInsightSnap.js | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/ui/hooks/flask/useTransactionInsightSnap.js b/ui/hooks/flask/useTransactionInsightSnap.js index 6206bc30da90..81cc6366541f 100644 --- a/ui/hooks/flask/useTransactionInsightSnap.js +++ b/ui/hooks/flask/useTransactionInsightSnap.js @@ -1,4 +1,5 @@ -import { useEffect, useState } from 'react'; +import deepEqual from 'fast-deep-equal'; +import { useEffect, useState, useRef } from 'react'; import { useSelector } from 'react-redux'; import { handleSnapRequest } from '../../store/actions'; import { getPermissionSubjects } from '../../selectors'; @@ -13,25 +14,33 @@ export function useTransactionInsightSnap({ transaction, chainId, snapId }) { ); } const [data, setData] = useState(undefined); + const transactionRef = useRef(transaction); + + // The transaction object reference is often changed even though the contents arent. + // This is a way of only updating our effect once an actual value changes in the object + if (!deepEqual(transactionRef.current, transaction)) { + transactionRef.current = transaction; + } useEffect(() => { async function fetchInsight() { const d = await handleSnapRequest({ snapId, - origin: 'test', + origin: '', handler: 'onTransaction', request: { jsonrpc: '2.0', method: ' ', - params: { transaction, chainId }, + params: { transaction: transactionRef.current, chainId }, }, }); setData(d); } - if (transaction) { + if (transactionRef.current) { fetchInsight(); } - }, [snapId, transaction, chainId]); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [snapId, transactionRef.current, chainId]); return data; } From 13fcf6cbec60854655c715de522eeada59881904 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Fri, 14 Oct 2022 12:33:09 +0200 Subject: [PATCH 2/6] Add dep --- package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/package.json b/package.json index 756de20282f9..01bbbe88ec09 100644 --- a/package.json +++ b/package.json @@ -184,6 +184,7 @@ "ethjs-contract": "^0.2.3", "ethjs-query": "^0.3.4", "extension-port-stream": "^2.0.0", + "fast-deep-equal": "^3.1.3", "fast-json-patch": "^2.2.1", "fuse.js": "^3.2.0", "globalthis": "^1.0.1", From 7469c94e0e7c52eb877766a96005cc559c9d8d20 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Fri, 14 Oct 2022 12:54:26 +0200 Subject: [PATCH 3/6] Update LavaMoat policies --- lavamoat/browserify/beta/policy.json | 4 ++-- lavamoat/browserify/flask/policy.json | 12 ++++++------ lavamoat/browserify/main/policy.json | 4 ++-- lavamoat/build-system/policy.json | 4 ++-- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/lavamoat/browserify/beta/policy.json b/lavamoat/browserify/beta/policy.json index 1ab56a362b4f..2e9a2bc431b5 100644 --- a/lavamoat/browserify/beta/policy.json +++ b/lavamoat/browserify/beta/policy.json @@ -2693,7 +2693,6 @@ "browserify>buffer": true, "browserify>events": true, "deep-freeze-strict": true, - "eslint>fast-deep-equal": true, "eth-ens-namehash": true, "eth-keyring-controller": true, "eth-query": true, @@ -2702,6 +2701,7 @@ "ethereumjs-util": true, "ethers": true, "ethjs>ethjs-unit": true, + "fast-deep-equal": true, "immer": true, "json-rpc-engine": true, "jsonschema": true, @@ -4435,7 +4435,7 @@ }, "packages": { "@metamask/snap-utils>superstruct": true, - "eslint>fast-deep-equal": true, + "fast-deep-equal": true, "nock>debug": true } }, diff --git a/lavamoat/browserify/flask/policy.json b/lavamoat/browserify/flask/policy.json index cd23cc3b57af..1d8631808017 100644 --- a/lavamoat/browserify/flask/policy.json +++ b/lavamoat/browserify/flask/policy.json @@ -2854,7 +2854,6 @@ "browserify>buffer": true, "browserify>events": true, "deep-freeze-strict": true, - "eslint>fast-deep-equal": true, "eth-ens-namehash": true, "eth-keyring-controller": true, "eth-query": true, @@ -2863,6 +2862,7 @@ "ethereumjs-util": true, "ethers": true, "ethjs>ethjs-unit": true, + "fast-deep-equal": true, "immer": true, "json-rpc-engine": true, "jsonschema": true, @@ -3592,7 +3592,6 @@ "browserify>buffer": true, "browserify>events": true, "deep-freeze-strict": true, - "eslint>fast-deep-equal": true, "eth-ens-namehash": true, "eth-keyring-controller": true, "eth-query": true, @@ -3600,6 +3599,7 @@ "eth-sig-util": true, "ethereumjs-util": true, "ethjs>ethjs-unit": true, + "fast-deep-equal": true, "immer": true, "json-rpc-engine": true, "jsonschema": true, @@ -3913,7 +3913,6 @@ "browserify>buffer": true, "browserify>events": true, "deep-freeze-strict": true, - "eslint>fast-deep-equal": true, "eth-ens-namehash": true, "eth-keyring-controller": true, "eth-query": true, @@ -3921,6 +3920,7 @@ "eth-sig-util": true, "ethereumjs-util": true, "ethjs>ethjs-unit": true, + "fast-deep-equal": true, "immer": true, "json-rpc-engine": true, "jsonschema": true, @@ -4031,7 +4031,7 @@ }, "@metamask/snap-controllers>@metamask/post-message-stream>@metamask/utils": { "packages": { - "eslint>fast-deep-equal": true + "fast-deep-equal": true } }, "@metamask/snap-controllers>@metamask/post-message-stream>readable-stream": { @@ -4263,8 +4263,8 @@ "browserify>crypto-browserify": true, "browserify>events": true, "browserify>path-browserify": true, - "eslint>fast-deep-equal": true, "eth-block-tracker>@metamask/utils": true, + "fast-deep-equal": true, "semver": true } }, @@ -5270,7 +5270,7 @@ }, "packages": { "@metamask/snap-utils>superstruct": true, - "eslint>fast-deep-equal": true, + "fast-deep-equal": true, "nock>debug": true } }, diff --git a/lavamoat/browserify/main/policy.json b/lavamoat/browserify/main/policy.json index 1ab56a362b4f..2e9a2bc431b5 100644 --- a/lavamoat/browserify/main/policy.json +++ b/lavamoat/browserify/main/policy.json @@ -2693,7 +2693,6 @@ "browserify>buffer": true, "browserify>events": true, "deep-freeze-strict": true, - "eslint>fast-deep-equal": true, "eth-ens-namehash": true, "eth-keyring-controller": true, "eth-query": true, @@ -2702,6 +2701,7 @@ "ethereumjs-util": true, "ethers": true, "ethjs>ethjs-unit": true, + "fast-deep-equal": true, "immer": true, "json-rpc-engine": true, "jsonschema": true, @@ -4435,7 +4435,7 @@ }, "packages": { "@metamask/snap-utils>superstruct": true, - "eslint>fast-deep-equal": true, + "fast-deep-equal": true, "nock>debug": true } }, diff --git a/lavamoat/build-system/policy.json b/lavamoat/build-system/policy.json index 600dd2b65905..62d9c7b8cf84 100644 --- a/lavamoat/build-system/policy.json +++ b/lavamoat/build-system/policy.json @@ -2240,7 +2240,6 @@ "eslint>espree": true, "eslint>esquery": true, "eslint>esutils": true, - "eslint>fast-deep-equal": true, "eslint>file-entry-cache": true, "eslint>functional-red-black-tree": true, "eslint>glob-parent": true, @@ -2253,6 +2252,7 @@ "eslint>minimatch": true, "eslint>natural-compare": true, "eslint>regexpp": true, + "fast-deep-equal": true, "globby>ignore": true, "nock>debug": true } @@ -2796,7 +2796,7 @@ "eslint>ajv>fast-json-stable-stringify": true, "eslint>ajv>json-schema-traverse": true, "eslint>ajv>uri-js": true, - "eslint>fast-deep-equal": true + "fast-deep-equal": true } }, "eslint>ajv>uri-js": { From 023f1b9ff41e94fe0b57d8727071eef01bc3e13f Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Fri, 14 Oct 2022 15:14:10 +0200 Subject: [PATCH 4/6] Fix selectors and revert changes to hook --- ui/hooks/flask/useTransactionInsightSnap.js | 17 ++---- .../confirm-transaction-base.container.js | 23 ++------ ui/selectors/selectors.js | 54 +++++++++++++++++-- 3 files changed, 59 insertions(+), 35 deletions(-) diff --git a/ui/hooks/flask/useTransactionInsightSnap.js b/ui/hooks/flask/useTransactionInsightSnap.js index 81cc6366541f..ad2102837c10 100644 --- a/ui/hooks/flask/useTransactionInsightSnap.js +++ b/ui/hooks/flask/useTransactionInsightSnap.js @@ -1,5 +1,4 @@ -import deepEqual from 'fast-deep-equal'; -import { useEffect, useState, useRef } from 'react'; +import { useEffect, useState } from 'react'; import { useSelector } from 'react-redux'; import { handleSnapRequest } from '../../store/actions'; import { getPermissionSubjects } from '../../selectors'; @@ -14,13 +13,6 @@ export function useTransactionInsightSnap({ transaction, chainId, snapId }) { ); } const [data, setData] = useState(undefined); - const transactionRef = useRef(transaction); - - // The transaction object reference is often changed even though the contents arent. - // This is a way of only updating our effect once an actual value changes in the object - if (!deepEqual(transactionRef.current, transaction)) { - transactionRef.current = transaction; - } useEffect(() => { async function fetchInsight() { @@ -31,16 +23,15 @@ export function useTransactionInsightSnap({ transaction, chainId, snapId }) { request: { jsonrpc: '2.0', method: ' ', - params: { transaction: transactionRef.current, chainId }, + params: { transaction, chainId }, }, }); setData(d); } - if (transactionRef.current) { + if (transaction) { fetchInsight(); } - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [snapId, transactionRef.current, chainId]); + }, [snapId, transaction, chainId]); return data; } diff --git a/ui/pages/confirm-transaction-base/confirm-transaction-base.container.js b/ui/pages/confirm-transaction-base/confirm-transaction-base.container.js index 6df5f6f3d8ff..cecc4af26ce0 100644 --- a/ui/pages/confirm-transaction-base/confirm-transaction-base.container.js +++ b/ui/pages/confirm-transaction-base/confirm-transaction-base.container.js @@ -35,6 +35,8 @@ import { getEIP1559V2Enabled, getIsBuyableChain, getEnsResolutionByAddress, + getUnapprovedTransaction, + getFullTxData, ///: BEGIN:ONLY_INCLUDE_IN(flask) getInsightSnaps, ///: END:ONLY_INCLUDE_IN @@ -98,10 +100,8 @@ const mapStateToProps = (state, ownProps) => { } = metamask; const { tokenData, txData, tokenProps, nonce } = confirmTransaction; const { txParams = {}, id: transactionId, type } = txData; - const transaction = - Object.values(unapprovedTxs).find( - ({ id }) => id === (transactionId || Number(paramsTransactionId)), - ) || {}; + const txId = transactionId || Number(paramsTransactionId); + const transaction = getUnapprovedTransaction(state, txId); const { from: fromAddress, to: txParamsToAddress, @@ -148,10 +148,6 @@ const mapStateToProps = (state, ownProps) => { gasEstimationObject, } = transactionFeeSelector(state, transaction); - if (transaction && transaction.simulationFails) { - txData.simulationFails = transaction.simulationFails; - } - const currentNetworkUnapprovedTxs = Object.keys(unapprovedTxs) .filter((key) => transactionMatchesNetwork(unapprovedTxs[key], chainId, network), @@ -168,16 +164,7 @@ const mapStateToProps = (state, ownProps) => { const methodData = getKnownMethodData(state, data) || {}; - let fullTxData = { ...txData, ...transaction }; - if (customTxParamsData) { - fullTxData = { - ...fullTxData, - txParams: { - ...fullTxData.txParams, - data: customTxParamsData, - }, - }; - } + const fullTxData = getFullTxData(state, txId, customTxParamsData); const isCollectibleTransfer = Boolean( allCollectibleContracts?.[selectedAddress]?.[chainId]?.find((contract) => { diff --git a/ui/selectors/selectors.js b/ui/selectors/selectors.js index 5b08d39fcfb9..5b79b744ece5 100644 --- a/ui/selectors/selectors.js +++ b/ui/selectors/selectors.js @@ -1,7 +1,14 @@ -import { createSelector } from 'reselect'; -///: BEGIN:ONLY_INCLUDE_IN(flask) -import { memoize } from 'lodash'; -///: END:ONLY_INCLUDE_IN +import { + createSelector, + createSelectorCreator, + defaultMemoize, +} from 'reselect'; +import { + ///: BEGIN:ONLY_INCLUDE_IN(flask) + memoize, + ///: END:ONLY_INCLUDE_IN + isEqual, +} from 'lodash'; import { addHexPrefix } from '../../app/scripts/lib/util'; import { TEST_CHAINS, @@ -771,6 +778,45 @@ export function getShowWhatsNewPopup(state) { return state.appState.showWhatsNewPopup; } +const createDeepEqualSelector = createSelectorCreator(defaultMemoize, isEqual); + +export const getUnapprovedTransactions = (state) => + state.metamask.unapprovedTxs; + +export const getTxData = (state) => state.confirmTransaction.txData; + +export const getUnapprovedTransaction = createDeepEqualSelector( + getUnapprovedTransactions, + (_, transactionId) => transactionId, + (unapprovedTxs, transactionId) => { + return ( + Object.values(unapprovedTxs).find(({ id }) => id === transactionId) || {} + ); + }, +); + +export const getFullTxData = createDeepEqualSelector( + getTxData, + (state, transactionId) => getUnapprovedTransaction(state, transactionId), + (_state, _transactionId, customTxParamsData) => customTxParamsData, + (txData, transaction, customTxParamsData) => { + let fullTxData = { ...txData, ...transaction }; + if (transaction && transaction.simulationFails) { + txData.simulationFails = transaction.simulationFails; + } + if (customTxParamsData) { + fullTxData = { + ...fullTxData, + txParams: { + ...fullTxData.txParams, + data: customTxParamsData, + }, + }; + } + return fullTxData; + }, +); + ///: BEGIN:ONLY_INCLUDE_IN(flask) export function getSnaps(state) { return state.metamask.snaps; From 1d0bfd9c03321e26dc01d7926f91d6d36bc31f27 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Fri, 14 Oct 2022 15:14:57 +0200 Subject: [PATCH 5/6] Remove dep --- package.json | 1 - 1 file changed, 1 deletion(-) diff --git a/package.json b/package.json index 01bbbe88ec09..756de20282f9 100644 --- a/package.json +++ b/package.json @@ -184,7 +184,6 @@ "ethjs-contract": "^0.2.3", "ethjs-query": "^0.3.4", "extension-port-stream": "^2.0.0", - "fast-deep-equal": "^3.1.3", "fast-json-patch": "^2.2.1", "fuse.js": "^3.2.0", "globalthis": "^1.0.1", From 84e9586eadacfa3e9dec1e725939441c2911524f Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Fri, 14 Oct 2022 15:15:04 +0200 Subject: [PATCH 6/6] Revert "Update LavaMoat policies" This reverts commit 7469c94e0e7c52eb877766a96005cc559c9d8d20. --- lavamoat/browserify/beta/policy.json | 4 ++-- lavamoat/browserify/flask/policy.json | 12 ++++++------ lavamoat/browserify/main/policy.json | 4 ++-- lavamoat/build-system/policy.json | 4 ++-- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/lavamoat/browserify/beta/policy.json b/lavamoat/browserify/beta/policy.json index 2e9a2bc431b5..1ab56a362b4f 100644 --- a/lavamoat/browserify/beta/policy.json +++ b/lavamoat/browserify/beta/policy.json @@ -2693,6 +2693,7 @@ "browserify>buffer": true, "browserify>events": true, "deep-freeze-strict": true, + "eslint>fast-deep-equal": true, "eth-ens-namehash": true, "eth-keyring-controller": true, "eth-query": true, @@ -2701,7 +2702,6 @@ "ethereumjs-util": true, "ethers": true, "ethjs>ethjs-unit": true, - "fast-deep-equal": true, "immer": true, "json-rpc-engine": true, "jsonschema": true, @@ -4435,7 +4435,7 @@ }, "packages": { "@metamask/snap-utils>superstruct": true, - "fast-deep-equal": true, + "eslint>fast-deep-equal": true, "nock>debug": true } }, diff --git a/lavamoat/browserify/flask/policy.json b/lavamoat/browserify/flask/policy.json index 1d8631808017..cd23cc3b57af 100644 --- a/lavamoat/browserify/flask/policy.json +++ b/lavamoat/browserify/flask/policy.json @@ -2854,6 +2854,7 @@ "browserify>buffer": true, "browserify>events": true, "deep-freeze-strict": true, + "eslint>fast-deep-equal": true, "eth-ens-namehash": true, "eth-keyring-controller": true, "eth-query": true, @@ -2862,7 +2863,6 @@ "ethereumjs-util": true, "ethers": true, "ethjs>ethjs-unit": true, - "fast-deep-equal": true, "immer": true, "json-rpc-engine": true, "jsonschema": true, @@ -3592,6 +3592,7 @@ "browserify>buffer": true, "browserify>events": true, "deep-freeze-strict": true, + "eslint>fast-deep-equal": true, "eth-ens-namehash": true, "eth-keyring-controller": true, "eth-query": true, @@ -3599,7 +3600,6 @@ "eth-sig-util": true, "ethereumjs-util": true, "ethjs>ethjs-unit": true, - "fast-deep-equal": true, "immer": true, "json-rpc-engine": true, "jsonschema": true, @@ -3913,6 +3913,7 @@ "browserify>buffer": true, "browserify>events": true, "deep-freeze-strict": true, + "eslint>fast-deep-equal": true, "eth-ens-namehash": true, "eth-keyring-controller": true, "eth-query": true, @@ -3920,7 +3921,6 @@ "eth-sig-util": true, "ethereumjs-util": true, "ethjs>ethjs-unit": true, - "fast-deep-equal": true, "immer": true, "json-rpc-engine": true, "jsonschema": true, @@ -4031,7 +4031,7 @@ }, "@metamask/snap-controllers>@metamask/post-message-stream>@metamask/utils": { "packages": { - "fast-deep-equal": true + "eslint>fast-deep-equal": true } }, "@metamask/snap-controllers>@metamask/post-message-stream>readable-stream": { @@ -4263,8 +4263,8 @@ "browserify>crypto-browserify": true, "browserify>events": true, "browserify>path-browserify": true, + "eslint>fast-deep-equal": true, "eth-block-tracker>@metamask/utils": true, - "fast-deep-equal": true, "semver": true } }, @@ -5270,7 +5270,7 @@ }, "packages": { "@metamask/snap-utils>superstruct": true, - "fast-deep-equal": true, + "eslint>fast-deep-equal": true, "nock>debug": true } }, diff --git a/lavamoat/browserify/main/policy.json b/lavamoat/browserify/main/policy.json index 2e9a2bc431b5..1ab56a362b4f 100644 --- a/lavamoat/browserify/main/policy.json +++ b/lavamoat/browserify/main/policy.json @@ -2693,6 +2693,7 @@ "browserify>buffer": true, "browserify>events": true, "deep-freeze-strict": true, + "eslint>fast-deep-equal": true, "eth-ens-namehash": true, "eth-keyring-controller": true, "eth-query": true, @@ -2701,7 +2702,6 @@ "ethereumjs-util": true, "ethers": true, "ethjs>ethjs-unit": true, - "fast-deep-equal": true, "immer": true, "json-rpc-engine": true, "jsonschema": true, @@ -4435,7 +4435,7 @@ }, "packages": { "@metamask/snap-utils>superstruct": true, - "fast-deep-equal": true, + "eslint>fast-deep-equal": true, "nock>debug": true } }, diff --git a/lavamoat/build-system/policy.json b/lavamoat/build-system/policy.json index 62d9c7b8cf84..600dd2b65905 100644 --- a/lavamoat/build-system/policy.json +++ b/lavamoat/build-system/policy.json @@ -2240,6 +2240,7 @@ "eslint>espree": true, "eslint>esquery": true, "eslint>esutils": true, + "eslint>fast-deep-equal": true, "eslint>file-entry-cache": true, "eslint>functional-red-black-tree": true, "eslint>glob-parent": true, @@ -2252,7 +2253,6 @@ "eslint>minimatch": true, "eslint>natural-compare": true, "eslint>regexpp": true, - "fast-deep-equal": true, "globby>ignore": true, "nock>debug": true } @@ -2796,7 +2796,7 @@ "eslint>ajv>fast-json-stable-stringify": true, "eslint>ajv>json-schema-traverse": true, "eslint>ajv>uri-js": true, - "fast-deep-equal": true + "eslint>fast-deep-equal": true } }, "eslint>ajv>uri-js": {