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: remove methods from array used to determine which requests should be enqueued because they can be safely passed through #27315

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

adonesky1
Copy link
Contributor

@adonesky1 adonesky1 commented Sep 20, 2024

Description

Fix issues that arise when a STX is initated in a dapp and subsequent method calls were being unnecessarily queued until the STX was complete.

The following methods can be safely removed from the list of methods we use to determine whether a request should be queued or executed immediately:

  • 'wallet_addEthereumChain'
  • 'wallet_requestPermissions',
  • 'wallet_requestSnaps',
  • 'eth_decrypt',
  • 'eth_requestAccounts',
  • 'eth_getEncryptionPublicKey',

Open in GitHub Codespaces

Related issues

Fixes: #27098

Manual testing steps

  1. Make sure you have STX enabled from settings
  2. Navigate to https://docs.metamask.io/wallet/reference/eth_sendtransaction/
  3. Connect the wallet and switch networks to Sepolia
  4. Trigger a TX (call run request)
  5. Confirm the transaction and see the STX pending screen
  6. Go to test dapp
  7. Click Connect --> this needs to happen while STX is pending
  8. See that you are able to connect

Screenshots/Recordings

Before

Before-wallet_requestSnaps.mov

After

wallet_requestPermissions:

Screen.Recording.2024-09-20.at.1.11.57.PM.mov

wallet_requestSnaps

Screen.Recording.2024-09-20.at.1.41.04.PM.mov

eth_requestAccounts

Screen.Recording.2024-09-20.at.1.14.44.PM.mov

wallet_addEthereumChain

Screen.Recording.2024-09-20.at.1.01.45.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.

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.

@adonesky1 adonesky1 changed the title Remove methods from array used to determine which requests should be enqueued because they can be safely passed through Fix: remove methods from array used to determine which requests should be enqueued because they can be safely passed through Sep 20, 2024
@adonesky1 adonesky1 changed the title Fix: remove methods from array used to determine which requests should be enqueued because they can be safely passed through fix: remove methods from array used to determine which requests should be enqueued because they can be safely passed through Sep 20, 2024
@jiexi
Copy link
Contributor

jiexi commented Sep 20, 2024

LGTM after the eth_accounts special handling is removed from shouldEnqueueRequest

…enqueued because they can be safely passed through
@adonesky1 adonesky1 marked this pull request as ready for review September 20, 2024 19:38
@adonesky1 adonesky1 requested a review from a team as a code owner September 20, 2024 19:38
Copy link

sonarcloud bot commented Sep 20, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants