From 1c0ff8a6e8c5aad67b83762e4e70b587f822ab21 Mon Sep 17 00:00:00 2001 From: Jyoti Puri Date: Fri, 24 Feb 2023 22:37:26 +0530 Subject: [PATCH] Fix state in confirm transaction (#17838) --- test/e2e/fixture-builder.js | 48 ++++++--- test/e2e/tests/multiple-transactions.spec.js | 98 +++++++++++++++++++ test/e2e/tests/navigate-transactions.spec.js | 1 + .../confirm-transaction-base.component.js | 5 +- .../confirm-transaction.component.js | 29 ++++-- 5 files changed, 155 insertions(+), 26 deletions(-) create mode 100644 test/e2e/tests/multiple-transactions.spec.js diff --git a/test/e2e/fixture-builder.js b/test/e2e/fixture-builder.js index 9c93907b2..7ec760bcf 100644 --- a/test/e2e/fixture-builder.js +++ b/test/e2e/fixture-builder.js @@ -653,14 +653,16 @@ class FixtureBuilder { chainId: CHAIN_IDS.LOCALHOST, dappSuggestedGasFees: { gas: '0x5208', - gasPrice: '0x4a817c800', + maxFeePerGas: '0x59682f0c', + maxPriorityFeePerGas: '0x59682f00', }, history: [ { chainId: CHAIN_IDS.LOCALHOST, dappSuggestedGasFees: { gas: '0x5208', - gasPrice: '0x4a817c800', + maxFeePerGas: '0x59682f0c', + maxPriorityFeePerGas: '0x59682f00', }, id: 7911313280012623, loadingDefaults: true, @@ -671,7 +673,8 @@ class FixtureBuilder { txParams: { from: '0x5cfe73b6021e818b776b421b1c4db2474086a7e1', gas: '0x5208', - gasPrice: '0x4a817c800', + maxFeePerGas: '0x59682f0c', + maxPriorityFeePerGas: '0x59682f00', to: '0x2f318c334780961fb129d2a6c30d0763d9a5c970', value: '0x29a2241af62c0000', }, @@ -696,7 +699,8 @@ class FixtureBuilder { txParams: { from: '0x5cfe73b6021e818b776b421b1c4db2474086a7e1', gas: '0x5208', - gasPrice: '0x4a817c800', + maxFeePerGas: '0x59682f0c', + maxPriorityFeePerGas: '0x59682f00', to: '0x2f318c334780961fb129d2a6c30d0763d9a5c970', value: '0x29a2241af62c0000', }, @@ -706,14 +710,16 @@ class FixtureBuilder { chainId: CHAIN_IDS.LOCALHOST, dappSuggestedGasFees: { gas: '0x5208', - gasPrice: '0x4a817c800', + maxFeePerGas: '0x59682f0c', + maxPriorityFeePerGas: '0x59682f00', }, history: [ { chainId: CHAIN_IDS.LOCALHOST, dappSuggestedGasFees: { gas: '0x5208', - gasPrice: '0x4a817c800', + maxFeePerGas: '0x59682f0c', + maxPriorityFeePerGas: '0x59682f00', }, id: 7911313280012624, loadingDefaults: true, @@ -724,7 +730,8 @@ class FixtureBuilder { txParams: { from: '0x5cfe73b6021e818b776b421b1c4db2474086a7e1', gas: '0x5208', - gasPrice: '0x4a817c800', + maxFeePerGas: '0x59682f0c', + maxPriorityFeePerGas: '0x59682f00', to: '0x2f318c334780961fb129d2a6c30d0763d9a5c970', value: '0x29a2241af62c0000', }, @@ -749,7 +756,8 @@ class FixtureBuilder { txParams: { from: '0x5cfe73b6021e818b776b421b1c4db2474086a7e1', gas: '0x5208', - gasPrice: '0x4a817c800', + maxFeePerGas: '0x59682f0c', + maxPriorityFeePerGas: '0x59682f00', to: '0x2f318c334780961fb129d2a6c30d0763d9a5c970', value: '0x29a2241af62c0000', }, @@ -759,14 +767,16 @@ class FixtureBuilder { chainId: CHAIN_IDS.LOCALHOST, dappSuggestedGasFees: { gas: '0x5208', - gasPrice: '0x4a817c800', + maxFeePerGas: '0x59682f0c', + maxPriorityFeePerGas: '0x59682f00', }, history: [ { chainId: CHAIN_IDS.LOCALHOST, dappSuggestedGasFees: { gas: '0x5208', - gasPrice: '0x4a817c800', + maxFeePerGas: '0x59682f0c', + maxPriorityFeePerGas: '0x59682f00', }, id: 7911313280012625, loadingDefaults: true, @@ -777,7 +787,8 @@ class FixtureBuilder { txParams: { from: '0x5cfe73b6021e818b776b421b1c4db2474086a7e1', gas: '0x5208', - gasPrice: '0x4a817c800', + maxFeePerGas: '0x59682f0c', + maxPriorityFeePerGas: '0x59682f00', to: '0x2f318c334780961fb129d2a6c30d0763d9a5c970', value: '0x29a2241af62c0000', }, @@ -802,7 +813,8 @@ class FixtureBuilder { txParams: { from: '0x5cfe73b6021e818b776b421b1c4db2474086a7e1', gas: '0x5208', - gasPrice: '0x4a817c800', + maxFeePerGas: '0x59682f0c', + maxPriorityFeePerGas: '0x59682f00', to: '0x2f318c334780961fb129d2a6c30d0763d9a5c970', value: '0x29a2241af62c0000', }, @@ -812,14 +824,16 @@ class FixtureBuilder { chainId: CHAIN_IDS.LOCALHOST, dappSuggestedGasFees: { gas: '0x5208', - gasPrice: '0x4a817c800', + maxFeePerGas: '0x59682f0c', + maxPriorityFeePerGas: '0x59682f00', }, history: [ { chainId: CHAIN_IDS.LOCALHOST, dappSuggestedGasFees: { gas: '0x5208', - gasPrice: '0x4a817c800', + maxFeePerGas: '0x59682f0c', + maxPriorityFeePerGas: '0x59682f00', }, id: 7911313280012626, loadingDefaults: true, @@ -830,7 +844,8 @@ class FixtureBuilder { txParams: { from: '0x5cfe73b6021e818b776b421b1c4db2474086a7e1', gas: '0x5208', - gasPrice: '0x4a817c800', + maxFeePerGas: '0x59682f0c', + maxPriorityFeePerGas: '0x59682f00', to: '0x2f318c334780961fb129d2a6c30d0763d9a5c970', value: '0x29a2241af62c0000', }, @@ -855,7 +870,8 @@ class FixtureBuilder { txParams: { from: '0x5cfe73b6021e818b776b421b1c4db2474086a7e1', gas: '0x5208', - gasPrice: '0x4a817c800', + maxFeePerGas: '0x59682f0c', + maxPriorityFeePerGas: '0x59682f00', to: '0x2f318c334780961fb129d2a6c30d0763d9a5c970', value: '0x29a2241af62c0000', }, diff --git a/test/e2e/tests/multiple-transactions.spec.js b/test/e2e/tests/multiple-transactions.spec.js new file mode 100644 index 000000000..bc29c3308 --- /dev/null +++ b/test/e2e/tests/multiple-transactions.spec.js @@ -0,0 +1,98 @@ +const { convertToHexValue, withFixtures } = require('../helpers'); +const FixtureBuilder = require('../fixture-builder'); + +describe('Confirm transactions', function () { + const ganacheOptions = { + hardfork: 'london', + accounts: [ + { + secretKey: + '0x7C9529A67102755B7E6102D6D950AC5D5863C98713805CEC576B945B15B71EAC', + balance: convertToHexValue(25000000000000000000), + }, + ], + }; + it('should be able to confirm multiple transactions', async function () { + await withFixtures( + { + fixtures: new FixtureBuilder() + .withTransactionControllerMultipleTransactions() + .build(), + ganacheOptions, + title: this.test.title, + }, + async ({ driver }) => { + await driver.navigate(); + await driver.fill('#password', 'correct horse battery staple'); + await driver.press('#password', driver.Key.ENTER); + + // confirm multiple transactions + await driver.waitForSelector({ + text: 'Reject 4 transactions', + tag: 'a', + }); + await driver.clickElement({ text: 'Confirm', tag: 'button' }); + await driver.waitForSelector({ + text: 'Reject 3 transactions', + tag: 'a', + }); + await driver.clickElement({ text: 'Confirm', tag: 'button' }); + await driver.waitForSelector({ + text: 'Reject 2 transactions', + tag: 'a', + }); + await driver.clickElement({ text: 'Confirm', tag: 'button' }); + + await driver.waitForElementNotPresent('.loading-overlay__spinner'); + await driver.clickElement({ text: 'Confirm', tag: 'button' }); + + await driver.clickElement('[data-testid="home__activity-tab"]'); + await driver.wait(async () => { + const confirmedTxes = await driver.findElements( + '.transaction-list__completed-transactions .transaction-list-item', + ); + return confirmedTxes.length === 4; + }, 10000); + }, + ); + }); + + it('should be able to reject multiple transactions', async function () { + await withFixtures( + { + fixtures: new FixtureBuilder() + .withTransactionControllerMultipleTransactions() + .build(), + ganacheOptions, + title: this.test.title, + }, + async ({ driver }) => { + await driver.navigate(); + await driver.fill('#password', 'correct horse battery staple'); + await driver.press('#password', driver.Key.ENTER); + + // confirm multiple transactions + await driver.waitForSelector({ + text: 'Reject 4 transactions', + tag: 'a', + }); + await driver.clickElement({ text: 'Reject', tag: 'button' }); + await driver.waitForSelector({ + text: 'Reject 3 transactions', + tag: 'a', + }); + await driver.clickElement({ text: 'Reject', tag: 'button' }); + await driver.waitForSelector({ + text: 'Reject 2 transactions', + tag: 'a', + }); + await driver.clickElement({ text: 'Reject', tag: 'button' }); + + await driver.waitForElementNotPresent('.loading-overlay__spinner'); + await driver.clickElement({ text: 'Reject', tag: 'button' }); + + await driver.waitForSelector('[data-testid="home__activity-tab"]'); + }, + ); + }); +}); diff --git a/test/e2e/tests/navigate-transactions.spec.js b/test/e2e/tests/navigate-transactions.spec.js index d2f964b8e..4d4136732 100644 --- a/test/e2e/tests/navigate-transactions.spec.js +++ b/test/e2e/tests/navigate-transactions.spec.js @@ -4,6 +4,7 @@ const FixtureBuilder = require('../fixture-builder'); describe('Navigate transactions', function () { const ganacheOptions = { + hardfork: 'london', accounts: [ { secretKey: diff --git a/ui/pages/confirm-transaction-base/confirm-transaction-base.component.js b/ui/pages/confirm-transaction-base/confirm-transaction-base.component.js index 28f13294c..fdb8d8fba 100644 --- a/ui/pages/confirm-transaction-base/confirm-transaction-base.component.js +++ b/ui/pages/confirm-transaction-base/confirm-transaction-base.component.js @@ -776,14 +776,12 @@ export default class ConfirmTransactionBase extends Component { cancelTransaction, history, mostRecentOverviewPage, - clearConfirmTransaction, updateCustomNonce, } = this.props; this._removeBeforeUnload(); updateCustomNonce(''); cancelTransaction(txData).then(() => { - clearConfirmTransaction(); history.push(mostRecentOverviewPage); }); } @@ -791,7 +789,6 @@ export default class ConfirmTransactionBase extends Component { handleSubmit() { const { sendTransaction, - clearConfirmTransaction, txData, history, mostRecentOverviewPage, @@ -865,7 +862,6 @@ export default class ConfirmTransactionBase extends Component { sendTransaction(txData) .then(() => { - clearConfirmTransaction(); if (!this._isMounted) { return; } @@ -995,6 +991,7 @@ export default class ConfirmTransactionBase extends Component { componentWillUnmount() { this._beforeUnloadForGasPolling(); this._removeBeforeUnload(); + this.props.clearConfirmTransaction(); } supportsEIP1559 = diff --git a/ui/pages/confirm-transaction/confirm-transaction.component.js b/ui/pages/confirm-transaction/confirm-transaction.component.js index 066fef15b..698a729fe 100644 --- a/ui/pages/confirm-transaction/confirm-transaction.component.js +++ b/ui/pages/confirm-transaction/confirm-transaction.component.js @@ -1,4 +1,4 @@ -import React, { useCallback, useEffect, useMemo, useState } from 'react'; +import React, { useCallback, useEffect, useState } from 'react'; import { useDispatch, useSelector } from 'react-redux'; import { Switch, Route, useHistory, useParams } from 'react-router-dom'; @@ -61,8 +61,7 @@ const ConfirmTransaction = () => { const unconfirmedMessages = useSelector(unconfirmedTransactionsHashSelector); const totalUnapproved = unconfirmedTxs.length || 0; - - const transaction = useMemo(() => { + const getTransaction = useCallback(() => { return totalUnapproved ? unapprovedTxs[paramsTransactionId] || unconfirmedMessages[paramsTransactionId] || @@ -75,10 +74,27 @@ const ConfirmTransaction = () => { unconfirmedMessages, unconfirmedTxs, ]); + const [transaction, setTransaction] = useState(getTransaction); + + useEffect(() => { + const tx = getTransaction(); + setTransaction(tx); + if (tx?.id) { + dispatch(setTransactionToConfirm(tx.id)); + } + }, [ + dispatch, + getTransaction, + paramsTransactionId, + totalUnapproved, + unapprovedTxs, + unconfirmedMessages, + unconfirmedTxs, + ]); const { id, type } = transaction; const transactionId = id && String(id); - const isValidERC20TokenMethod = isTokenMethodAction(type); + const isValidTokenMethod = isTokenMethodAction(type); const isValidTransactionId = transactionId && (!paramsTransactionId || paramsTransactionId === transactionId); @@ -152,7 +168,8 @@ const ConfirmTransaction = () => { } else if ( prevTransactionId && transactionId && - prevTransactionId !== transactionId + prevTransactionId !== transactionId && + paramsTransactionId !== transactionId ) { history.replace(mostRecentOverviewPage); } @@ -168,7 +185,7 @@ const ConfirmTransaction = () => { transactionId, ]); - if (isValidERC20TokenMethod && isValidTransactionId) { + if (isValidTokenMethod && isValidTransactionId) { return ; } // Show routes when state.confirmTransaction has been set and when either the ID in the params