Skip to content

Commit

Permalink
fix: disable multiple send transactions (#25468)
Browse files Browse the repository at this point in the history
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

Multiple transactions break STX logic; we need to prevent these
transactions from being submitted concurrently. While the confirmations
screen covers this, the swap+send flow bypasses this screen altogether,
so we need to add logic to prevent submission via the instant submit
button (i.e. submit for swap+send).

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/25468?quickstart=1)

## **Related issues**

Fixes: #25349 

## **Manual testing steps**

1. Submit a smart transactions (e.g., basic send on mainnet)
2. Go to the send flow and fill out the form for a swap+send
3. Ensure that the submit button is disabled with a tooltip: "A previous
transaction is still being signed or submitted"
4. Ensure that the send flow is not reset to the recipient stage when
the transactions complete
5. Ensure that a transaction can now be submitted as normal

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

See: #25349 

<!-- [screenshots/recordings] -->

### **After**


https://github.com/MetaMask/metamask-extension/assets/44588480/8f2b662c-2e55-433c-8c4a-50aaeb912d22


<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [ ] 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).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] 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
BZahory authored Jun 21, 2024
1 parent 3552a39 commit 2e00b43
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 5 deletions.
19 changes: 17 additions & 2 deletions ui/components/multichain/pages/send/send.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import {
import {
TokenStandard,
AssetType,
SmartTransactionStatus,
} from '../../../../../shared/constants/transaction';
import { MetaMetricsContext } from '../../../../contexts/metametrics';
import { INSUFFICIENT_FUNDS_ERROR } from '../../../../pages/confirmations/send/send.constants';
Expand All @@ -56,6 +57,7 @@ import { getMostRecentOverviewPage } from '../../../../ducks/history/history';
import { AssetPickerAmount } from '../..';
import useUpdateSwapsState from '../../../../hooks/useUpdateSwapsState';
import { getIsDraftSwapAndSend } from '../../../../ducks/send/helpers';
import { smartTransactionsListSelector } from '../../../../selectors';
import {
SendPageAccountPicker,
SendPageRecipientContent,
Expand Down Expand Up @@ -256,13 +258,20 @@ export const SendPage = () => {
const sendErrors = useSelector(getSendErrors);
const isInvalidSendForm = useSelector(isSendFormInvalid);

const smartTransactions = useSelector(smartTransactionsListSelector);

const isSmartTransactionPending = smartTransactions?.find(
({ status }) => status === SmartTransactionStatus.pending,
);

const isGasTooLow =
sendErrors.gasFee === INSUFFICIENT_FUNDS_ERROR &&
sendErrors.amount !== INSUFFICIENT_FUNDS_ERROR;

const submitDisabled =
(isInvalidSendForm && !isGasTooLow) ||
requireContractAddressAcknowledgement;
requireContractAddressAcknowledgement ||
(isSwapAndSend && isSmartTransactionPending);

const isSendFormShown =
draftTransactionExists &&
Expand All @@ -281,7 +290,13 @@ export const SendPage = () => {
[dispatch],
);

const tooltipTitle = isSwapAndSend ? t('sendSwapSubmissionWarning') : '';
let tooltipTitle = '';

if (isSwapAndSend) {
tooltipTitle = isSmartTransactionPending
? t('isSigningOrSubmitting')
: t('sendSwapSubmissionWarning');
}

return (
<Page className="multichain-send-page">
Expand Down
16 changes: 13 additions & 3 deletions ui/store/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ import {
// that does not have an explicit export statement. lets see if it breaks the
// compiler
DraftTransaction,
SEND_STAGES,
} from '../ducks/send';
import { switchedToUnconnectedAccount } from '../ducks/alerts/unconnected-account';
import {
Expand Down Expand Up @@ -1075,9 +1076,13 @@ export function updateAndApproveTx(
unknown,
AnyAction
> {
return (dispatch: MetaMaskReduxDispatch) => {
return (dispatch: MetaMaskReduxDispatch, getState) => {
!dontShowLoadingIndicator &&
dispatch(showLoadingIndication(loadingIndicatorMessage));

const getIsSendActive = () =>
Boolean(getState().send.stage !== SEND_STAGES.INACTIVE);

return new Promise((resolve, reject) => {
const actionId = generateActionId();

Expand All @@ -1086,7 +1091,10 @@ export function updateAndApproveTx(
[String(txMeta.id), { txMeta, actionId }, { waitForResult: true }],
(err) => {
dispatch(updateTransactionParams(txMeta.id, txMeta.txParams));
dispatch(resetSendState());

if (!getIsSendActive()) {
dispatch(resetSendState());
}

if (err) {
dispatch(goHome());
Expand All @@ -1102,7 +1110,9 @@ export function updateAndApproveTx(
.then(() => updateMetamaskStateFromBackground())
.then((newState) => dispatch(updateMetamaskState(newState)))
.then(() => {
dispatch(resetSendState());
if (!getIsSendActive()) {
dispatch(resetSendState());
}
dispatch(completedTx(txMeta.id));
dispatch(hideLoadingIndication());
dispatch(updateCustomNonce(''));
Expand Down

0 comments on commit 2e00b43

Please sign in to comment.