1
0
mirror of https://github.com/kremalicious/metamask-extension.git synced 2024-12-23 09:52:26 +01:00

Fixed navigation through multiple unapproved transactions for ERC20 tokens (#16822)

This commit is contained in:
Filip Sekulic 2023-01-11 16:01:50 +01:00 committed by GitHub
parent 604bc304e4
commit fc83a1b631
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 153 additions and 118 deletions

View File

@ -29,7 +29,6 @@ describe('Confirm Page Container Container Test', () => {
fromAddress: '0xd8f6a2ffb0fc5952d16c9768b71cfd35b6399aa5', fromAddress: '0xd8f6a2ffb0fc5952d16c9768b71cfd35b6399aa5',
toAddress: '0x7a1A4Ad9cc746a70ee58568466f7996dD0aCE4E8', toAddress: '0x7a1A4Ad9cc746a70ee58568466f7996dD0aCE4E8',
origin: 'testOrigin', // required origin: 'testOrigin', // required
onNextTx: sinon.spy(),
// Footer // Footer
onCancelAll: sinon.spy(), onCancelAll: sinon.spy(),
onCancel: sinon.spy(), onCancel: sinon.spy(),
@ -69,6 +68,104 @@ describe('Confirm Page Container Container Test', () => {
showEdit: true, showEdit: true,
hideSenderToRecipient: false, hideSenderToRecipient: false,
toName: '0x7a1...E4E8', toName: '0x7a1...E4E8',
txData: {
id: 1230035278491151,
time: 1671022500513,
status: 'unapproved',
metamaskNetworkId: '80001',
originalGasEstimate: '0xea60',
userEditedGasLimit: false,
chainId: '0x13881',
loadingDefaults: false,
dappSuggestedGasFees: {
gasPrice: '0x4a817c800',
gas: '0xea60',
},
sendFlowHistory: [],
txParams: {
from: '0xdd34b35ca1de17dfcdc07f79ff1f8f94868c40a1',
to: '0x7a67ff4a59594a56d46e9308a5c6e197fa83a3cf',
value: '0x0',
data: '0x095ea7b30000000000000000000000009bc5baf874d2da8d216ae9f137804184ee5afef40000000000000000000000000000000000000000000000000000000000011170',
gas: '0xea60',
maxFeePerGas: '0x0',
maxPriorityFeePerGas: '0x0',
},
origin: 'https://metamask.github.io',
type: 'simpleSend',
history: [
{
id: 1230035278491151,
time: 1671022500513,
status: 'unapproved',
metamaskNetworkId: '80001',
originalGasEstimate: '0xea60',
userEditedGasLimit: false,
chainId: '0x13881',
loadingDefaults: true,
dappSuggestedGasFees: {
gasPrice: '0x4a817c800',
gas: '0xea60',
},
sendFlowHistory: [],
txParams: {
from: '0xdd34b35ca1de17dfcdc07f79ff1f8f94868c40a1',
to: '0x7a67ff4a59594a56d46e9308a5c6e197fa83a3cf',
value: '0x0',
data: '0x095ea7b30000000000000000000000009bc5baf874d2da8d216ae9f137804184ee5afef40000000000000000000000000000000000000000000000000000000000011170',
gas: '0xea60',
gasPrice: '0x4a817c800',
},
origin: 'https://metamask.github.io',
type: 'simpleSend',
},
[
{
op: 'remove',
path: '/txParams/gasPrice',
note: 'Added new unapproved transaction.',
timestamp: 1671022501288,
},
{
op: 'add',
path: '/txParams/maxFeePerGas',
value: '0x0',
},
{
op: 'add',
path: '/txParams/maxPriorityFeePerGas',
value: '0x0',
},
{
op: 'replace',
path: '/loadingDefaults',
value: false,
},
{
op: 'add',
path: '/userFeeLevel',
value: 'custom',
},
{
op: 'add',
path: '/defaultGasEstimates',
value: {
estimateType: 'custom',
gas: '0xea60',
maxFeePerGas: '0',
maxPriorityFeePerGas: '0',
},
},
],
],
userFeeLevel: 'custom',
defaultGasEstimates: {
estimateType: 'custom',
gas: '0xea60',
maxFeePerGas: '0',
maxPriorityFeePerGas: '0',
},
},
}; };
describe('Render and simulate button clicks', () => { describe('Render and simulate button clicks', () => {
const store = configureMockStore()(mockState); const store = configureMockStore()(mockState);

View File

@ -1,19 +1,50 @@
import React from 'react'; import React, { useContext } from 'react';
import PropTypes from 'prop-types'; import { useDispatch, useSelector } from 'react-redux';
import { useHistory, useParams } from 'react-router-dom';
import {
getCurrentChainId,
getUnapprovedTransactions,
} from '../../../../selectors';
import { transactionMatchesNetwork } from '../../../../../shared/modules/transaction.utils';
import { I18nContext } from '../../../../contexts/i18n';
import { CONFIRM_TRANSACTION_ROUTE } from '../../../../helpers/constants/routes';
import { clearConfirmTransaction } from '../../../../ducks/confirm-transaction/confirm-transaction.duck';
import { hexToDecimal } from '../../../../../shared/lib/metamask-controller-utils';
const ConfirmPageContainerNavigation = (props) => { const ConfirmPageContainerNavigation = () => {
const { const t = useContext(I18nContext);
onNextTx, const dispatch = useDispatch();
totalTx, const history = useHistory();
positionOfCurrentTx, const { id } = useParams();
nextTxId,
prevTxId, const unapprovedTxs = useSelector(getUnapprovedTransactions);
showNavigation, const currentChainId = useSelector(getCurrentChainId);
firstTx, const network = hexToDecimal(currentChainId);
lastTx,
ofText, const currentNetworkUnapprovedTxs = Object.keys(unapprovedTxs)
requestsWaitingText, .filter((key) =>
} = props; transactionMatchesNetwork(unapprovedTxs[key], currentChainId, network),
)
.reduce((acc, key) => ({ ...acc, [key]: unapprovedTxs[key] }), {});
const enumUnapprovedTxs = Object.keys(currentNetworkUnapprovedTxs);
const currentPosition = enumUnapprovedTxs.indexOf(id);
const totalTx = enumUnapprovedTxs.length;
const positionOfCurrentTx = currentPosition + 1;
const nextTxId = enumUnapprovedTxs[currentPosition + 1];
const prevTxId = enumUnapprovedTxs[currentPosition - 1];
const showNavigation = enumUnapprovedTxs.length > 1;
const firstTx = enumUnapprovedTxs[0];
const lastTx = enumUnapprovedTxs[enumUnapprovedTxs.length - 1];
const onNextTx = (txId) => {
if (txId) {
dispatch(clearConfirmTransaction());
history.push(`${CONFIRM_TRANSACTION_ROUTE}/${txId}`);
}
};
return ( return (
<div <div
@ -46,10 +77,10 @@ const ConfirmPageContainerNavigation = (props) => {
</div> </div>
<div className="confirm-page-container-navigation__textcontainer"> <div className="confirm-page-container-navigation__textcontainer">
<div className="confirm-page-container-navigation__navtext"> <div className="confirm-page-container-navigation__navtext">
{positionOfCurrentTx} {ofText} {totalTx} {positionOfCurrentTx} {t('ofTextNofM')} {totalTx}
</div> </div>
<div className="confirm-page-container-navigation__longtext"> <div className="confirm-page-container-navigation__longtext">
{requestsWaitingText} {t('requestsAwaitingAcknowledgement')}
</div> </div>
</div> </div>
<div <div
@ -77,17 +108,4 @@ const ConfirmPageContainerNavigation = (props) => {
); );
}; };
ConfirmPageContainerNavigation.propTypes = {
totalTx: PropTypes.number,
positionOfCurrentTx: PropTypes.number,
onNextTx: PropTypes.func,
nextTxId: PropTypes.string,
prevTxId: PropTypes.string,
showNavigation: PropTypes.bool,
firstTx: PropTypes.string,
lastTx: PropTypes.string,
ofText: PropTypes.string,
requestsWaitingText: PropTypes.string,
};
export default ConfirmPageContainerNavigation; export default ConfirmPageContainerNavigation;

View File

@ -84,17 +84,6 @@ export default class ConfirmPageContainer extends Component {
origin: PropTypes.string.isRequired, origin: PropTypes.string.isRequired,
ethGasPriceWarning: PropTypes.string, ethGasPriceWarning: PropTypes.string,
networkIdentifier: PropTypes.string, networkIdentifier: PropTypes.string,
// Navigation
totalTx: PropTypes.number,
positionOfCurrentTx: PropTypes.number,
nextTxId: PropTypes.string,
prevTxId: PropTypes.string,
showNavigation: PropTypes.bool,
onNextTx: PropTypes.func,
firstTx: PropTypes.string,
lastTx: PropTypes.string,
ofText: PropTypes.string,
requestsWaitingText: PropTypes.string,
// Footer // Footer
onCancelAll: PropTypes.func, onCancelAll: PropTypes.func,
onCancel: PropTypes.func, onCancel: PropTypes.func,
@ -161,16 +150,6 @@ export default class ConfirmPageContainer extends Component {
nonce, nonce,
unapprovedTxCount, unapprovedTxCount,
warning, warning,
totalTx,
positionOfCurrentTx,
nextTxId,
prevTxId,
showNavigation,
onNextTx,
firstTx,
lastTx,
ofText,
requestsWaitingText,
hideSenderToRecipient, hideSenderToRecipient,
showAccountInHeader, showAccountInHeader,
origin, origin,
@ -212,18 +191,7 @@ export default class ConfirmPageContainer extends Component {
return ( return (
<GasFeeContextProvider transaction={currentTransaction}> <GasFeeContextProvider transaction={currentTransaction}>
<div className="page-container" data-testid="page-container"> <div className="page-container" data-testid="page-container">
<ConfirmPageContainerNavigation <ConfirmPageContainerNavigation />
totalTx={totalTx}
positionOfCurrentTx={positionOfCurrentTx}
nextTxId={nextTxId}
prevTxId={prevTxId}
showNavigation={showNavigation}
onNextTx={(txId) => onNextTx(txId)}
firstTx={firstTx}
lastTx={lastTx}
ofText={ofText}
requestsWaitingText={requestsWaitingText}
/>
{assetStandard === ERC20 || {assetStandard === ERC20 ||
assetStandard === ERC721 || assetStandard === ERC721 ||
assetStandard === ERC1155 ? ( assetStandard === ERC1155 ? (

View File

@ -7,10 +7,7 @@ import ConfirmPageContainer from '../../components/app/confirm-page-container';
import TransactionDecoding from '../../components/app/transaction-decoding'; import TransactionDecoding from '../../components/app/transaction-decoding';
import { isBalanceSufficient } from '../send/send.utils'; import { isBalanceSufficient } from '../send/send.utils';
import { addHexes } from '../../helpers/utils/conversions.util'; import { addHexes } from '../../helpers/utils/conversions.util';
import { import { DEFAULT_ROUTE } from '../../helpers/constants/routes';
CONFIRM_TRANSACTION_ROUTE,
DEFAULT_ROUTE,
} from '../../helpers/constants/routes';
import { import {
INSUFFICIENT_FUNDS_ERROR_KEY, INSUFFICIENT_FUNDS_ERROR_KEY,
GAS_LIMIT_TOO_LOW_ERROR_KEY, GAS_LIMIT_TOO_LOW_ERROR_KEY,
@ -116,7 +113,6 @@ export default class ConfirmTransactionBase extends Component {
transactionStatus: PropTypes.string, transactionStatus: PropTypes.string,
txData: PropTypes.object, txData: PropTypes.object,
unapprovedTxCount: PropTypes.number, unapprovedTxCount: PropTypes.number,
currentNetworkUnapprovedTxs: PropTypes.object,
customGas: PropTypes.object, customGas: PropTypes.object,
// Component props // Component props
actionKey: PropTypes.string, actionKey: PropTypes.string,
@ -1015,33 +1011,6 @@ export default class ConfirmTransactionBase extends Component {
); );
} }
handleNextTx(txId) {
const { history, clearConfirmTransaction } = this.props;
if (txId) {
clearConfirmTransaction();
history.push(`${CONFIRM_TRANSACTION_ROUTE}/${txId}`);
}
}
getNavigateTxData() {
const { currentNetworkUnapprovedTxs, txData: { id } = {} } = this.props;
const enumUnapprovedTxs = Object.keys(currentNetworkUnapprovedTxs);
const currentPosition = enumUnapprovedTxs.indexOf(id ? id.toString() : '');
return {
totalTx: enumUnapprovedTxs.length,
positionOfCurrentTx: currentPosition + 1,
nextTxId: enumUnapprovedTxs[currentPosition + 1],
prevTxId: enumUnapprovedTxs[currentPosition - 1],
showNavigation: enumUnapprovedTxs.length > 1,
firstTx: enumUnapprovedTxs[0],
lastTx: enumUnapprovedTxs[enumUnapprovedTxs.length - 1],
ofText: this.context.t('ofTextNofM'),
requestsWaitingText: this.context.t('requestsAwaitingAcknowledgement'),
};
}
_beforeUnloadForGasPolling = () => { _beforeUnloadForGasPolling = () => {
this._isMounted = false; this._isMounted = false;
if (this.state.pollingToken) { if (this.state.pollingToken) {
@ -1150,17 +1119,6 @@ export default class ConfirmTransactionBase extends Component {
const hasSimulationError = Boolean(txData.simulationFails); const hasSimulationError = Boolean(txData.simulationFails);
const renderSimulationFailureWarning = const renderSimulationFailureWarning =
hasSimulationError && !userAcknowledgedGasMissing; hasSimulationError && !userAcknowledgedGasMissing;
const {
totalTx,
positionOfCurrentTx,
nextTxId,
prevTxId,
showNavigation,
firstTx,
lastTx,
ofText,
requestsWaitingText,
} = this.getNavigateTxData();
// This `isTokenApproval` case is added to handle possible rendering of this component from // This `isTokenApproval` case is added to handle possible rendering of this component from
// confirm-approve.js when `assetStandard` is `undefined`. That will happen if the request to // confirm-approve.js when `assetStandard` is `undefined`. That will happen if the request to
@ -1222,16 +1180,6 @@ export default class ConfirmTransactionBase extends Component {
errorKey={errorKey} errorKey={errorKey}
hasSimulationError={hasSimulationError} hasSimulationError={hasSimulationError}
warning={submitWarning} warning={submitWarning}
totalTx={totalTx}
positionOfCurrentTx={positionOfCurrentTx}
nextTxId={nextTxId}
prevTxId={prevTxId}
showNavigation={showNavigation}
onNextTx={(txId) => this.handleNextTx(txId)}
firstTx={firstTx}
lastTx={lastTx}
ofText={ofText}
requestsWaitingText={requestsWaitingText}
disabled={ disabled={
renderSimulationFailureWarning || renderSimulationFailureWarning ||
!valid || !valid ||
@ -1255,6 +1203,7 @@ export default class ConfirmTransactionBase extends Component {
nativeCurrency={nativeCurrency} nativeCurrency={nativeCurrency}
isApprovalOrRejection={isApprovalOrRejection} isApprovalOrRejection={isApprovalOrRejection}
assetStandard={assetStandard} assetStandard={assetStandard}
txData={txData}
/> />
</TransactionModalContextProvider> </TransactionModalContextProvider>
); );

View File

@ -219,7 +219,6 @@ const mapStateToProps = (state, ownProps) => {
nonce, nonce,
unapprovedTxs, unapprovedTxs,
unapprovedTxCount, unapprovedTxCount,
currentNetworkUnapprovedTxs,
customGas: { customGas: {
gasLimit, gasLimit,
gasPrice, gasPrice,

View File

@ -53,6 +53,7 @@ import { setCustomTokenAmount } from '../../ducks/app/app';
import { valuesFor } from '../../helpers/utils/util'; import { valuesFor } from '../../helpers/utils/util';
import { calcTokenAmount } from '../../../shared/lib/transactions-controller-utils'; import { calcTokenAmount } from '../../../shared/lib/transactions-controller-utils';
import { MAX_TOKEN_ALLOWANCE_AMOUNT } from '../../../shared/constants/tokens'; import { MAX_TOKEN_ALLOWANCE_AMOUNT } from '../../../shared/constants/tokens';
import { ConfirmPageContainerNavigation } from '../../components/app/confirm-page-container';
export default function TokenAllowance({ export default function TokenAllowance({
origin, origin,
@ -236,6 +237,9 @@ export default function TokenAllowance({
return ( return (
<Box className="token-allowance-container page-container"> <Box className="token-allowance-container page-container">
<Box>
<ConfirmPageContainerNavigation />
</Box>
<Box <Box
paddingLeft={4} paddingLeft={4}
paddingRight={4} paddingRight={4}