Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: disable multiple send transactions #25468

Merged
merged 5 commits into from
Jun 21, 2024
Merged

Conversation

BZahory
Copy link
Contributor

@BZahory BZahory commented Jun 21, 2024

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).

Open in GitHub Codespaces

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

Before

See: #25349

After

Screen.Recording.2024-06-21.at.11.13.27.AM.mov

Pre-merge author checklist

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.

@BZahory BZahory requested a review from a team as a code owner June 21, 2024 15:03
Copy link
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@BZahory BZahory changed the title fix: disable multiple fix: disable multiple send transactions Jun 21, 2024
Copy link

codecov bot commented Jun 21, 2024

Codecov Report

Attention: Patch coverage is 35.29412% with 11 lines in your changes missing coverage. Please review.

Project coverage is 64.92%. Comparing base (b8bd980) to head (bd0a7e5).
Report is 20 commits behind head on develop.

Files Patch % Lines
ui/store/actions.ts 0.00% 7 Missing ⚠️
ui/components/multichain/pages/send/send.js 60.00% 4 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #25468      +/-   ##
===========================================
- Coverage    64.93%   64.92%   -0.01%     
===========================================
  Files         1385     1385              
  Lines        54958    54970      +12     
  Branches     14421    14426       +5     
===========================================
+ Hits         35682    35686       +4     
- Misses       19276    19284       +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@metamaskbot
Copy link
Collaborator

Builds ready [bd0a7e5]
Page Load Metrics (137 ± 180 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint6210282105
domContentLoaded9201121
load411773137376180
domInteractive9201121
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 272 Bytes (0.00%)
  • common: 81 Bytes (0.00%)

@sleepytanya
Copy link
Contributor

Able to submit multiple txs:

  1. Start send+swap txs with low gas
  2. Send another (approve) tx from a dapp
  3. Confirmation is stuck on the spinner
  4. Activity tan has two txs - Pending and Unapproved
  5. Unapprovedtxs can't be rejected
  6. If user rejects first tx MM displays an error Cannot destructure property 'balance' of 'Q[z]' as it is undefined.
Screenshot 2024-06-21 at 16 09 26
2.mov
reject.mov

@sleepytanya
Copy link
Contributor

I can't send multiple transactions using send+swap flow:

  • ETH->ETH
  • ETH->non-native token
  • non-native->non-native
  • non-native->ETH
  • send NFT
567.mov

@BZahory BZahory merged commit 2e00b43 into develop Jun 21, 2024
74 checks passed
@BZahory BZahory deleted the swap-send-disable-multiple-tx branch June 21, 2024 21:09
@github-actions github-actions bot locked and limited conversation to collaborators Jun 21, 2024
@metamaskbot metamaskbot added release-12.1.0 Issue or pull request that will be included in release 12.1.0 release-12.0.0 Issue or pull request that will be included in release 12.0.0 and removed release-12.1.0 Issue or pull request that will be included in release 12.1.0 labels Jun 21, 2024
@metamaskbot
Copy link
Collaborator

Missing release label release-12.0.0 on PR. Adding release label release-12.0.0 on PR and removing other release labels(release-12.1.0), as PR was cherry-picked in branch 12.0.0.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release-12.0.0 Issue or pull request that will be included in release 12.0.0 team-bridge
Projects
None yet
4 participants