MetaMask / metamask-extension

:globe_with_meridians: :electric_plug: The MetaMask browser extension enables browsing Ethereum blockchain enabled websites
https://metamask.io
Other
11.52k stars 4.7k forks source link

fix: Remove Snaps Methods from 'methodsWithConfirmation' list used to determine whether to enqueue a request #24371

Closed adonesky1 closed 1 week ago

adonesky1 commented 2 weeks ago

Description

Currently Snaps can call RPC requests that do not fully conform to certain expectation built into the QueuedRequestController. wallet_invokeSnap and wallet_snap may some times cause a confirmation and in some cases they may not. Currently our blanket assumption that a method either does or does not involve a confirmation is causing an issue: wallet_invokeSnap can call (as does JSON-RPC Snap on the Test-Snaps Page) another JSON-RPC method upon which it relies for completion. In the queuing system the second request gets queued behind the first (which depends on it for its own execution) leading to a lock up.

We should resolve this issue with a more elegant solution for how these chained RPC requests should be handled by queueing. But for now it's safe to simply circumvent queueing for these Snaps methods: In the cases where there is a confirmation resulting from a wallet_invokeSnap or wallet_snap call these requests will simply not be separated from a batch with another origin, which is fine.

Open in GitHub Codespaces

Related issues

Fixes: See this Slack thread

Manual testing steps

  1. Go to https://metamask.github.io/snaps/test-snaps/2.6.1/
  2. Connect to BIP-32 Snap
  3. Connect to JSON-RPC Snap
  4. Click Invoke Snap on JSON-RPC Snap
  5. You should see a hex value appear below the Invoke Snap button

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

github-actions[bot] commented 2 weeks ago

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 commented 1 week ago
Builds ready [c3d6658]
Page Load Metrics (537 ± 462 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint57132762311
domContentLoaded85714136
load452486537962462
domInteractive85714136
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -34 Bytes (-0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)