Skip to content

fix: UX: Check for unconfirmed transactions on all networks#25459

Closed
darkwing wants to merge 10 commits intodevelopfrom
multichain-all-unapproved
Closed

fix: UX: Check for unconfirmed transactions on all networks#25459
darkwing wants to merge 10 commits intodevelopfrom
multichain-all-unapproved

Conversation

@darkwing
Copy link
Copy Markdown
Contributor

@darkwing darkwing commented Jun 21, 2024

Description

The previously used getUnapprovedTransactions only checked for unapproved transactions on the current chain, not all chains, so if the user has transactions/confirmations on more than 2 simultaneous networks, the middle ones get dropped/forgotten. This PR ensures no dapp-specified transactions get dropped when moving around tabs.

Open in GitHub Codespaces

Related issues

Fixes:

Manual testing steps

  1. Open Tab 1, connect to Uniswap on Ethereum Mainnet
  2. Open Tab 2, connect to PancakeSwap on BNB Chain
  3. Open Tab 3, connect to Test Dapp on Sepolia
  4. Initiate a swap on Tab 1 and Tab 2 BUT DO NOT CONFIRM IT, JUST MOVE ON TO THE NEXT TAB
  5. Initiate a send on Tab 3 BUT DO NOT CONFIRM IT
  6. On the confirmation screen, you should still see the first confirmation from Tab 1 (Uniswap) on Ethereum Mainnet; confirm or reject it. See the confirmation window close
  7. A new confirmation popup should come up with the PancakeSwap/ Tab 2 confirmation on BNB chain; confirm or reject it. See the confirmation window close
  8. See one last confirmation screen pop up for the Tab 3 / Test Dapp send on Sepolia. Confirm or reject it.

Screenshots/Recordings

Before

After

Screen.Recording.2024-06-20.at.8.43.18.PM.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.

@darkwing darkwing requested a review from a team as a code owner June 21, 2024 01:45
@github-actions
Copy link
Copy Markdown
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.

@metamaskbot metamaskbot added the team-core-extension-ux Core Extension UX team label Jun 21, 2024
@darkwing darkwing changed the title fix: UX: Check for incomplete transactions on all networks fix: UX: Check for unconfirmed transactions on all networks Jun 21, 2024
@darkwing darkwing added the needs-assets-ux-review A shared label between the Assets and UX team to flag PRs ready for consolidated team review. label Jun 25, 2024
@darkwing darkwing force-pushed the multichain-all-unapproved branch from f8636fa to 58ad823 Compare June 25, 2024 17:49
@darkwing darkwing force-pushed the multichain-all-unapproved branch from 58ad823 to 03c3bf1 Compare June 26, 2024 01:17
@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [a394768]
Page Load Metrics (148 ± 206 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint60297904924
domContentLoaded9201131
load402022148430206
domInteractive9201131
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 523 Bytes (0.01%)

@codecov
Copy link
Copy Markdown

codecov bot commented Jun 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.66%. Comparing base (bcc8dcb) to head (a394768).
Report is 7 commits behind head on develop.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop   #25459   +/-   ##
========================================
  Coverage    69.65%   69.66%           
========================================
  Files         1347     1347           
  Lines        47808    47814    +6     
  Branches     13187    13188    +1     
========================================
+ Hits         33300    33306    +6     
  Misses       14508    14508           

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

@darkwing
Copy link
Copy Markdown
Contributor Author

Closing as the code change isn't necessary, and it was a result of a bad manual test

@darkwing darkwing closed this Jun 26, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Jun 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

needs-assets-ux-review A shared label between the Assets and UX team to flag PRs ready for consolidated team review. team-core-extension-ux Core Extension UX team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants