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: replace web3-stream-provider with StreamProvider from @metamask/providers #29650

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

cryptodev-2s
Copy link
Contributor

@cryptodev-2s cryptodev-2s commented Jan 12, 2025

Description

We use global.ethereumProvider in various places in the UI as an Ethereum provider, but it doesn't support EIP-1193 (it is missing events and the request method). We should replace the existing legacy provider with one that is compatible with EIP-1193.

This PR replaces web3-stream-provider with StreamProvider from @metamask/providers which is EIP-1193 compatible (only).

Open in GitHub Codespaces

Related issues

Fixes: #28774

Manual testing steps

Screenshots/Recordings

Before

After

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.

@cryptodev-2s cryptodev-2s self-assigned this Jan 12, 2025
@cryptodev-2s cryptodev-2s marked this pull request as draft January 12, 2025 13:37
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.

@cryptodev-2s
Copy link
Contributor Author

@metamaskbot update-policies

Copy link

socket-security bot commented Jan 12, 2025

Updated and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/@metamask/[email protected] 🔁 npm/@metamask/[email protected] None 0 35.7 kB metamaskbot

🚮 Removed packages: npm/[email protected]

View full report↗︎

@metamaskbot
Copy link
Collaborator

Policies updated.
👀 Please review the diff for suspicious new powers.

🧠 Learn how: https://lavamoat.github.io/guides/policy-diff/#what-to-look-for-when-reviewing-a-policy-diff

@cryptodev-2s
Copy link
Contributor Author

@metamaskbot update-policies

@metamaskbot
Copy link
Collaborator

Policies updated.
👀 Please review the diff for suspicious new powers.

🧠 Learn how: https://lavamoat.github.io/guides/policy-diff/#what-to-look-for-when-reviewing-a-policy-diff

@cryptodev-2s cryptodev-2s force-pushed the cryptodev2s/remove-eth-query-package branch from 6f611fc to 7066fe8 Compare January 13, 2025 12:09
@cryptodev-2s cryptodev-2s force-pushed the cryptodev2s/remove-web3-stream-provider-2 branch from 2bb3610 to db8cbbf Compare January 13, 2025 13:25
@cryptodev-2s
Copy link
Contributor Author

@metamaskbot update-policies

@metamaskbot
Copy link
Collaborator

Policies updated.
👀 Please review the diff for suspicious new powers.

🧠 Learn how: https://lavamoat.github.io/guides/policy-diff/#what-to-look-for-when-reviewing-a-policy-diff

@cryptodev-2s cryptodev-2s force-pushed the cryptodev2s/remove-web3-stream-provider-2 branch from f544f7f to 3ed696d Compare January 13, 2025 14:10
@cryptodev-2s cryptodev-2s force-pushed the cryptodev2s/remove-eth-query-package branch from 3acfcd6 to 0e2c767 Compare January 14, 2025 18:51
Base automatically changed from cryptodev2s/remove-eth-query-package to main January 15, 2025 13:23
@cryptodev-2s cryptodev-2s force-pushed the cryptodev2s/remove-web3-stream-provider-2 branch from 3ed696d to 52f8812 Compare January 15, 2025 13:43
@cryptodev-2s
Copy link
Contributor Author

@metamaskbot update-policies

@metamaskbot
Copy link
Collaborator

Policies updated.
👀 Please review the diff for suspicious new powers.

🧠 Learn how: https://lavamoat.github.io/guides/policy-diff/#what-to-look-for-when-reviewing-a-policy-diff

@cryptodev-2s cryptodev-2s force-pushed the cryptodev2s/remove-web3-stream-provider-2 branch 3 times, most recently from e45f12d to f60316b Compare January 21, 2025 15:16
@cryptodev-2s
Copy link
Contributor Author

@metamaskbot update-policies

@metamaskbot
Copy link
Collaborator

Policies updated.
👀 Please review the diff for suspicious new powers.

🧠 Learn how: https://lavamoat.github.io/guides/policy-diff/#what-to-look-for-when-reviewing-a-policy-diff

@metamaskbot
Copy link
Collaborator

Builds ready [b35e0c7]
Page Load Metrics (1660 ± 74 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint39419961598319153
domContentLoaded13681928162515273
load14331977166015574
domInteractive23131432713
backgroundConnect691322411
firstReactRender1595322713
getState488132010
initialActions01000
loadScripts9371453119213866
setupStore76915189
uiStartup15712217190519192
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -41.98 KiB (-0.71%)
  • ui: 80.36 KiB (1.01%)
  • common: 41.7 KiB (0.47%)

@cryptodev-2s cryptodev-2s force-pushed the cryptodev2s/remove-web3-stream-provider-2 branch from b35e0c7 to c16a7bc Compare January 21, 2025 21:58
@cryptodev-2s cryptodev-2s force-pushed the cryptodev2s/remove-web3-stream-provider-2 branch from c16a7bc to c83882d Compare January 22, 2025 00:14
@cryptodev-2s cryptodev-2s requested a review from Gudahtt January 22, 2025 00:16
@metamaskbot
Copy link
Collaborator

Builds ready [c83882d]
Page Load Metrics (1447 ± 36 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1361171814508038
domContentLoaded1353167014296933
load1363170914477536
domInteractive236334136
backgroundConnect64419126
firstReactRender1495372412
getState45913167
initialActions01000
loadScripts972126910497335
setupStore6391073
uiStartup15541948167011053
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -41.98 KiB (-0.71%)
  • ui: 80.25 KiB (1.01%)
  • common: 41.94 KiB (0.48%)

@metamaskbot
Copy link
Collaborator

Builds ready [a486419]
Page Load Metrics (1788 ± 74 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint42422211721331159
domContentLoaded14392194175814369
load14472222178815474
domInteractive26121432412
backgroundConnect77727199
firstReactRender1699372612
getState567252110
initialActions0483105
loadScripts1072144812958139
setupStore871272311
uiStartup16872604210820096
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: -41.98 KiB (-0.71%)
  • ui: 80.23 KiB (1.01%)
  • common: 41.99 KiB (0.48%)

@mcmire
Copy link
Contributor

mcmire commented Jan 22, 2025

@cryptodev-2s Can you explain the problem that this solves in the PR description? I know that this is explained in the original ticket, but putting it in the commit message would save a step if we need to look this up later.

@cryptodev-2s
Copy link
Contributor Author

@metamaskbot update-policies

@cryptodev-2s cryptodev-2s force-pushed the cryptodev2s/remove-web3-stream-provider-2 branch from 5ba1eeb to 84767fa Compare January 22, 2025 16:56
@metamaskbot
Copy link
Collaborator

Policies updated.
👀 Please review the diff for suspicious new powers.

🧠 Learn how: https://lavamoat.github.io/guides/policy-diff/#what-to-look-for-when-reviewing-a-policy-diff

@cryptodev-2s
Copy link
Contributor Author

@metamaskbot update-policies

@metamaskbot
Copy link
Collaborator

No policy changes

@metamaskbot
Copy link
Collaborator

Builds ready [78283c9]
Page Load Metrics (1652 ± 81 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint64719991576250120
domContentLoaded14452083162915876
load14542126165216881
domInteractive24563494
backgroundConnect77523199
firstReactRender1598373014
getState560202110
initialActions01000
loadScripts10201639118714771
setupStore87219199
uiStartup165824071988225108
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -80.83 KiB (-1.36%)
  • ui: 120.77 KiB (1.52%)
  • common: 41.99 KiB (0.48%)

Copy link
Contributor

@mcmire mcmire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I admit I do not totally understand how we are using streams, so forgive my perhaps dumb questions.

app/scripts/lib/stream-utils.js Outdated Show resolved Hide resolved
app/scripts/lib/stream-utils.js Outdated Show resolved Hide resolved
app/scripts/ui.js Outdated Show resolved Hide resolved
@metamaskbot
Copy link
Collaborator

Builds ready [1c55067]
Page Load Metrics (1806 ± 99 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint17423621478642308
domContentLoaded15072299178820096
load15172359180620799
domInteractive248650168
backgroundConnect75920168
firstReactRender16109543014
getState45916178
initialActions01000
loadScripts11161784132216077
setupStore860212010
uiStartup173127572137290139
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -81.27 KiB (-1.37%)
  • ui: 121.1 KiB (1.52%)
  • common: 42.33 KiB (0.48%)

Copy link

socket-security bot commented Jan 22, 2025

👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

View full report↗︎

@cryptodev-2s cryptodev-2s requested a review from mcmire January 22, 2025 22:36
@metamaskbot
Copy link
Collaborator

Builds ready [a75ce9e]
Page Load Metrics (1715 ± 77 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint14662121172215775
domContentLoaded14562037169314972
load14652127171516077
domInteractive25112402311
backgroundConnect787242211
firstReactRender1576372412
getState546994
initialActions01000
loadScripts10771549127513163
setupStore884282512
uiStartup16912452199618790
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -81.27 KiB (-1.37%)
  • ui: 121.1 KiB (1.52%)
  • common: 42.32 KiB (0.48%)

mcmire
mcmire previously approved these changes Jan 22, 2025
Copy link
Contributor

@mcmire mcmire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After reviewing the differences between StreamProvider from web3-stream-provider and @metamask/providers and the discussion in this PR, I have enough to wrap my head around this PR. Looks good to me, although it would be nice to have a second opinion.

@metamaskbot
Copy link
Collaborator

Builds ready [fd71f0a]
Page Load Metrics (1812 ± 111 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint155723601815226108
domContentLoaded152322001772214103
load152823671812230111
domInteractive2697432110
backgroundConnect8171433718
firstReactRender1595442914
getState598252412
initialActions01000
loadScripts10781656131817082
setupStore75712115
uiStartup174630672104330158
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -41.98 KiB (-0.71%)
  • ui: 121.33 KiB (1.52%)
  • common: 41.7 KiB (0.46%)

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

Successfully merging this pull request may close these issues.

Replace global.ethereumProvider with EIP-1193-compatible provider
3 participants