Skip to content

Commit

Permalink
fix: use transaction address to get lock for custom nonce (#28272)
Browse files Browse the repository at this point in the history
## **Description**

Use transaction from address to get custom nonce value.

## **Related issues**

Fixes: #28014

## **Manual testing steps**

1. Enable an account connected to the dApp
2. Switch to non-enabled account in the MetaMask
3. Initiate a transaction from the dApp
4. Ensure correct nonce is displayed

## **Screenshots/Recordings**
TODO

## **Pre-merge author checklist**

- [X] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [X] I've completed the PR template to the best of my ability
- [X] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [X] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.
  • Loading branch information
jpuri authored Nov 6, 2024
1 parent 74e163f commit f117f7c
Show file tree
Hide file tree
Showing 12 changed files with 44 additions and 40 deletions.
6 changes: 3 additions & 3 deletions ui/hooks/useMMICustodySendTransaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@ export function useMMICustodySendTransaction() {
const accountType = useSelector(getAccountType);
const mostRecentOverviewPage = useSelector(getMostRecentOverviewPage);

const { currentConfirmation } = useConfirmContext() as unknown as {
currentConfirmation: TransactionMeta | undefined;
};
const { currentConfirmation } = useConfirmContext<
TransactionMeta | undefined
>();
const { from } = getConfirmationSender(currentConfirmation);
const fromChecksumHexAddress = toChecksumHexAddress(from || '');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,8 @@ import { useIsNFT } from '../hooks/use-is-nft';
export const ApproveStaticSimulation = () => {
const t = useI18nContext();

const { currentConfirmation: transactionMeta } = useConfirmContext() as {
currentConfirmation: TransactionMeta;
};
const { currentConfirmation: transactionMeta } =
useConfirmContext<TransactionMeta>();

const { decimals: initialDecimals } = useAssetDetails(
transactionMeta?.txParams?.to,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,8 @@ import { RevokeStaticSimulation } from './revoke-static-simulation/revoke-static
import { SpendingCap } from './spending-cap/spending-cap';

const ApproveInfo = () => {
const { currentConfirmation: transactionMeta } = useConfirmContext() as {
currentConfirmation: TransactionMeta;
};
const { currentConfirmation: transactionMeta } =
useConfirmContext<TransactionMeta>();

const { isNFT } = useIsNFT(transactionMeta);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,8 @@ export const EditSpendingCapModal = ({

const dispatch = useDispatch();

const { currentConfirmation: transactionMeta } = useConfirmContext() as {
currentConfirmation: TransactionMeta;
};
const { currentConfirmation: transactionMeta } =
useConfirmContext<TransactionMeta>();

const { userBalance, tokenSymbol, decimals } = useAssetDetails(
transactionMeta.txParams.to,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,8 @@ import StaticSimulation from '../../shared/static-simulation/static-simulation';
export const RevokeStaticSimulation = () => {
const t = useI18nContext();

const { currentConfirmation: transactionMeta } = useConfirmContext() as {
currentConfirmation: TransactionMeta;
};
const { currentConfirmation: transactionMeta } =
useConfirmContext<TransactionMeta>();

const { chainId } = transactionMeta;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,8 @@ const SpendingCapGroup = ({
}) => {
const t = useI18nContext();

const { currentConfirmation: transactionMeta } = useConfirmContext() as {
currentConfirmation: TransactionMeta;
};
const { currentConfirmation: transactionMeta } =
useConfirmContext<TransactionMeta>();

const { spendingCap, formattedSpendingCap, value } =
useApproveTokenSimulation(transactionMeta, decimals);
Expand Down Expand Up @@ -81,9 +80,8 @@ export const SpendingCap = ({
}) => {
const t = useI18nContext();

const { currentConfirmation: transactionMeta } = useConfirmContext() as {
currentConfirmation: TransactionMeta;
};
const { currentConfirmation: transactionMeta } =
useConfirmContext<TransactionMeta>();

const { userBalance, tokenSymbol, decimals } = useAssetDetails(
transactionMeta.txParams.to,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import React, { useEffect } from 'react';
import { useDispatch, useSelector } from 'react-redux';
import { TransactionMeta } from '@metamask/transaction-controller';

import {
ConfirmInfoRow,
ConfirmInfoRowText,
Expand All @@ -17,15 +19,23 @@ import {
updateCustomNonce,
} from '../../../../../../../store/actions';
import { selectConfirmationAdvancedDetailsOpen } from '../../../../../selectors/preferences';
import { useConfirmContext } from '../../../../../context/confirm';
import { isSignatureTransactionType } from '../../../../../utils';
import { TransactionData } from '../transaction-data/transaction-data';

const NonceDetails = () => {
const { currentConfirmation } = useConfirmContext<TransactionMeta>();
const t = useI18nContext();
const dispatch = useDispatch();

useEffect(() => {
dispatch(getNextNonce());
}, [dispatch]);
if (
currentConfirmation &&
!isSignatureTransactionType(currentConfirmation)
) {
dispatch(getNextNonce(currentConfirmation.txParams.from));
}
}, [currentConfirmation, dispatch]);

const enableCustomNonce = useSelector(getUseNonceField);
const nextNonce = useSelector(getNextSuggestedNonce);
Expand Down
4 changes: 2 additions & 2 deletions ui/pages/confirmations/confirm-approve/confirm-approve.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ export default function ConfirmApprove({
isSetApproveForAll,
}) {
const dispatch = useDispatch();
const { txParams: { data: transactionData } = {} } = transaction;
const { txParams: { data: transactionData, from } = {} } = transaction;

const currentCurrency = useSelector(getCurrentCurrency);
const nativeCurrency = useSelector(getNativeCurrency);
Expand Down Expand Up @@ -266,7 +266,7 @@ export default function ConfirmApprove({
updateCustomNonce={(value) => {
dispatch(updateCustomNonce(value));
}}
getNextNonce={() => dispatch(getNextNonce())}
getNextNonce={() => dispatch(getNextNonce(from))}
showCustomizeNonceModal={({
/* eslint-disable no-shadow */
useNonceField,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,7 @@ export default class ConfirmTransactionBase extends Component {
mostRecentOverviewPage,
txData,
getNextNonce,
fromAddress,
} = this.props;

const {
Expand All @@ -236,7 +237,7 @@ export default class ConfirmTransactionBase extends Component {
transactionStatus === TransactionStatus.confirmed;

if (txData.id !== prevTxData.id) {
getNextNonce();
getNextNonce(fromAddress);
}

if (
Expand Down Expand Up @@ -413,6 +414,7 @@ export default class ConfirmTransactionBase extends Component {
tokenSymbol,
isUsingPaymaster,
isSigningOrSubmitting,
fromAddress,
} = this.props;

const { t } = this.context;
Expand Down Expand Up @@ -447,7 +449,7 @@ export default class ConfirmTransactionBase extends Component {

updateCustomNonce(inputValue);

getNextNonce();
getNextNonce(fromAddress);
};

const renderTotalMaxAmount = ({
Expand Down Expand Up @@ -1012,6 +1014,7 @@ export default class ConfirmTransactionBase extends Component {
this._isMounted = true;
const {
toAddress,
fromAddress,
txData: { origin, chainId: txChainId } = {},
getNextNonce,
tryReverseResolveAddress,
Expand Down Expand Up @@ -1041,7 +1044,7 @@ export default class ConfirmTransactionBase extends Component {
},
});

getNextNonce();
getNextNonce(fromAddress);
if (toAddress) {
tryReverseResolveAddress(toAddress);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,7 @@ export const mapDispatchToProps = (dispatch) => {
fetchSmartTransactionsLiveness: () => {
dispatch(fetchSmartTransactionsLiveness());
},
getNextNonce: () => dispatch(getNextNonce()),
getNextNonce: (address) => dispatch(getNextNonce(address)),
setNextNonce: (val) => dispatch(setNextNonce(val)),
setDefaultHomeActiveTabName: (tabName) =>
dispatch(setDefaultHomeActiveTabName(tabName)),
Expand Down
8 changes: 4 additions & 4 deletions ui/pages/confirmations/token-allowance/token-allowance.js
Original file line number Diff line number Diff line change
Expand Up @@ -347,12 +347,12 @@ export default function TokenAllowance({
};

const handleNextNonce = useCallback(() => {
dispatch(getNextNonce());
}, [getNextNonce, dispatch]);
dispatch(getNextNonce(txData.txParams.from));
}, [dispatch, txData.txParams.from]);

useEffect(() => {
dispatch(getNextNonce());
}, [getNextNonce, dispatch]);
dispatch(getNextNonce(txData.txParams.from));
}, [dispatch, txData.txParams.from]);

const handleUpdateCustomNonce = (value) => {
dispatch(updateCustomNonce(value));
Expand Down
11 changes: 4 additions & 7 deletions ui/store/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4374,16 +4374,13 @@ export function setNextNonce(nextNonce: string): PayloadAction<string> {
* accidental usage of a stale nonce as the call to getNextNonce only works for
* the currently selected address.
*
* @param address - address for which nonce lock shouuld be obtained.
* @returns
*/
export function getNextNonce(): ThunkAction<
Promise<string>,
MetaMaskReduxState,
unknown,
AnyAction
> {
export function getNextNonce(
address,
): ThunkAction<Promise<string>, MetaMaskReduxState, unknown, AnyAction> {
return async (dispatch, getState) => {
const { address } = getSelectedInternalAccount(getState());
const networkClientId = getSelectedNetworkClientId(getState());
let nextNonce;
try {
Expand Down

0 comments on commit f117f7c

Please sign in to comment.