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

chore: Add error handling for setCorrectChain #28740

Merged
merged 21 commits into from
Nov 27, 2024

Conversation

gambinish
Copy link
Contributor

@gambinish gambinish commented Nov 26, 2024

Description

Adds error handling to setCurrentNetwork and additional check to make sure that the user is never redirected to an incorrect network that is incompatible with the token they are trying to send/swap.

This is an edge case, and should be revisited post-launch for a more elegant/user friendly approach. We are now checking to make sure that the network and token chainIds match before redirecting. Very rarely, this will result in the user needing to click the send/swap button twice before they get redirected to the send/swap flow.

In our opinion, this was preferable to potentially redirecting the user to an incorrect network, where they could transact with a token incompatible with the network they are on.

Open in GitHub Codespaces

Related issues

Fixes:

Manual testing steps

Screenshots/Recordings

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.

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.

@gambinish gambinish added the portfolio-view Used for PRs and issues related to Q4 2024 portfolio view label Nov 26, 2024
@gambinish gambinish marked this pull request as ready for review November 26, 2024 19:37
@gambinish gambinish requested a review from a team as a code owner November 26, 2024 19:37
@metamaskbot
Copy link
Collaborator

Builds ready [e829c99]
Page Load Metrics (1727 ± 75 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint15172100173316278
domContentLoaded14792003169915273
load15162100172715675
domInteractive2496422311
backgroundConnect97230199
firstReactRender1674322110
getState56219199
initialActions00000
loadScripts10801552125313967
setupStore659222110
uiStartup17322406192418086
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 334 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

salimtb
salimtb previously approved these changes Nov 27, 2024
@metamaskbot
Copy link
Collaborator

Builds ready [da5872e]
Page Load Metrics (1678 ± 42 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint41918551614288138
domContentLoaded1508184216538842
load1523185716788842
domInteractive206834157
backgroundConnect97226178
firstReactRender16103462713
getState55615168
initialActions01000
loadScripts1113139412208139
setupStore66212157
uiStartup17222497188816077
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 640 Bytes (0.01%)
  • common: 0 Bytes (0.00%)

@metamaskbot
Copy link
Collaborator

Builds ready [8d8272d]
Page Load Metrics (1711 ± 44 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1533184617129144
domContentLoaded1515183516829144
load1531184117119244
domInteractive2394432110
backgroundConnect9109292412
firstReactRender1675392412
getState521842
initialActions01000
loadScripts1101139412488440
setupStore658192010
uiStartup17512128193611756
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 286 Bytes (0.00%)
  • common: 309 Bytes (0.00%)

@metamaskbot
Copy link
Collaborator

Builds ready [954652b]
Page Load Metrics (1923 ± 55 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint17422239192511957
domContentLoaded17312170189211053
load17412184192311555
domInteractive257639168
backgroundConnect10104352813
firstReactRender155726136
getState46122832110
initialActions01000
loadScripts13221745148010148
setupStore610811
uiStartup19622531219115675
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 286 Bytes (0.00%)
  • common: 309 Bytes (0.00%)

@gambinish gambinish changed the title chore: Add error handling and unit test for setCorrectChain chore: Add error handling for setCorrectChain Nov 27, 2024
@metamaskbot
Copy link
Collaborator

Builds ready [8a8f65c]
Page Load Metrics (1752 ± 48 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint15822040175010048
domContentLoaded1571201417319646
load15842042175210048
domInteractive246434126
backgroundConnect75422147
firstReactRender14511984
getState4210679209
initialActions01000
loadScripts1160158513119445
setupStore6431084
uiStartup17822347197314268
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 286 Bytes (0.00%)
  • common: 309 Bytes (0.00%)

@gambinish gambinish added this pull request to the merge queue Nov 27, 2024
Merged via the queue into develop with commit 781267a Nov 27, 2024
78 checks passed
@gambinish gambinish deleted the chore/portfolio-view-swap-check branch November 27, 2024 23:20
@github-actions github-actions bot locked and limited conversation to collaborators Nov 27, 2024
@metamaskbot metamaskbot added the release-12.9.0 Issue or pull request that will be included in release 12.9.0 label Nov 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
portfolio-view Used for PRs and issues related to Q4 2024 portfolio view release-12.9.0 Issue or pull request that will be included in release 12.9.0 team-assets
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants