metamask-extension icon indicating copy to clipboard operation
metamask-extension copied to clipboard

Published 20 hours ago • verified publisher icon MetaMask

feat(ramps): introduces dynamic support for rampable networks

Open georgeweiler opened this issue 10 months ago • 20 comments

Description

This PR introduces a new reducer for Ramps to store an array of "buyable" networks. A "buyable chain" is one that the native token has onramp support for. This BUYABLE_CHAINS_MAP list is currently hard-coded, and this PR fetches a dynamic network list from the Ramps API instead. The list of supported networks by the MetMask onramp team is dynamic and is based on provider support among other things.

There are several fallback protections in place. Buyable chains will default to the current hard-coded list before loading and will default to that same list if there are any errors. There is no need for loading or error states.

The ramps base API url has been added as a new environment variable, defaulted to production. example: METAMASK_RAMP_API_BASE_URL=https://on-ramp-content.metaswap.codefi.network

Here's screenshot to show the issue this PR will fix. Base network is supported as a buyable network. but because the hard-coded array of networks does not include base we do not enable the buy CTAs:

image

Open in GitHub Codespaces

Related issues

Fixes:

Manual testing steps

  1. Go to this page...

Screenshots/Recordings

Video Description of the changes in the PR (5 min) https://www.loom.com/share/973960816e7e497aae51ed1cdc3cebf5

Before

After

Pre-merge author checklist

  • [x] I’ve followed MetaMask Coding Standards.
  • [x] I've completed the PR template to the best of my ability
  • [x] I’ve included tests if applicable
  • [x] I’ve documented my code using JSDoc format if applicable
  • [x] I’ve applied the right labels on the PR (see labeling guidelines). Not required for external contributors.

Pre-merge reviewer checklist

  • [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • [x] 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.

georgeweiler avatar Apr 16 '24 00:04 georgeweiler

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.

github-actions[bot] avatar Apr 16 '24 00:04 github-actions[bot]

Builds ready [7c4ba77]
Page Load Metrics (879 ± 575 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint861591152512
domContentLoaded127629189
load7231068791198575
domInteractive127629189
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: 5.45 KiB (0.09%)
  • common: -1014 Bytes (-0.02%)

metamaskbot avatar Apr 16 '24 15:04 metamaskbot

Codecov Report

Attention: Patch coverage is 98.33333% with 1 line in your changes missing coverage. Please review.

Project coverage is 69.72%. Comparing base (d403213) to head (7ee6250).

Files Patch % Lines
...ages/confirmations/send/gas-display/gas-display.js 0.00% 1 Missing :warning:
Additional details and impacted files 
@@             Coverage Diff             @@
##           develop   #24041      +/-   ##
===========================================
+ Coverage    69.69%   69.72%   +0.03%     
===========================================
  Files         1350     1353       +3     
  Lines        47865    47910      +45     
  Branches     13199    13210      +11     
===========================================
+ Hits         33355    33402      +47     
+ Misses       14510    14508       -2

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Apr 16 '24 15:04 codecov[bot]

Builds ready [7fe1f84]
Page Load Metrics (352 ± 384 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint641851073517
domContentLoaded96726189
load483032352799384
domInteractive96726189
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: 5.45 KiB (0.09%)
  • common: -1014 Bytes (-0.02%)

metamaskbot avatar Apr 16 '24 15:04 metamaskbot

Builds ready [91521fc]
Page Load Metrics (491 ± 459 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint673621348641
domContentLoaded107231209
load543055491955459
domInteractive107231209
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: 5.62 KiB (0.09%)
  • common: -1014 Bytes (-0.02%)

metamaskbot avatar Apr 17 '24 19:04 metamaskbot

LGTM for QA, I tested with Base and other networks ✅ Screenshot 2024-04-18 at 6 11 18 AM Screenshot 2024-04-18 at 6 12 50 AM

bkirb avatar Apr 18 '24 13:04 bkirb

Builds ready [48fe591]
Page Load Metrics (1298 ± 712 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint712561264220
domContentLoaded1091282211
load57389612981482712
domInteractive991282211
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: 6.33 KiB (0.10%)
  • common: -1021 Bytes (-0.02%)

metamaskbot avatar Apr 19 '24 19:04 metamaskbot

Builds ready [debd520]
Page Load Metrics (1075 ± 647 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint612451223818
domContentLoaded108829188
load49337510751347647
domInteractive108729188
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: 6.58 KiB (0.10%)
  • common: -1021 Bytes (-0.02%)

metamaskbot avatar Apr 22 '24 18:04 metamaskbot

Builds ready [75f0806]
Page Load Metrics (199 ± 294 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint559974115
domContentLoaded813911
load442866199612294
domInteractive813911
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: 6.58 KiB (0.10%)
  • common: -1007 Bytes (-0.02%)

metamaskbot avatar May 10 '24 19:05 metamaskbot

Builds ready [142d6ad]
Page Load Metrics (1192 ± 660 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint67182992914
domContentLoaded107618189
load53303311921374660
domInteractive97618189
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: 6.58 KiB (0.10%)
  • common: -1007 Bytes (-0.02%)

metamaskbot avatar May 14 '24 16:05 metamaskbot

hey @georgeweiler , i was asked to take a look at this PR. This LGTM and i agree with your take on the selectors, maybe it would be great if someone from the extension team can confirm the full selector inclusion approach as i am not familiar with this

nikoferro avatar May 16 '24 08:05 nikoferro

Builds ready [b2f55c3]
Page Load Metrics (640 ± 472 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint63136942612
domContentLoaded96919189
load512592640984472
domInteractive96919189
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: 6.58 KiB (0.10%)
  • common: -1007 Bytes (-0.02%)

metamaskbot avatar May 16 '24 09:05 metamaskbot

I’m okay with the approach of requiring all selectors and mocking only some of them, but there must be a reason why originally this was not done this way, if no one from extension team raises a concern, then we can just merge.

wachunei avatar May 16 '24 14:05 wachunei

@georgeweiler @pedronfigueiredo @nikoferro I am okay with the use of "requireActual" as done in this PR. We do already use it in some other places. The reasons for not using it would be to have greater control, and visibility into that control, of the state and related dependencies. If we mock only what we need, and everything that we need, it becomes clear that the component unit test is only testing the component. But I don't think it is a strict requirement to do that.

Maybe we should establish clearer and stricter guidelines, but as we don't have them, I think what is in this PR is fine.

With that said, I don't think the usage of requireActual in question in this PR is necessary. Here is a PR I created against this branch that shows the unit tests passing without the instances of requireActual https://github.com/MetaMask/metamask-extension/pull/24589

I would say, at least, that you can remove the instances in the routes component test, which seems to be unnecessary

danjm avatar May 17 '24 13:05 danjm

Builds ready [53606d7]
Page Load Metrics (829 ± 501 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint62166922512
domContentLoaded96416147
load5125388291043501
domInteractive96416147
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: 6.58 KiB (0.10%)
  • common: -1007 Bytes (-0.02%)

metamaskbot avatar May 20 '24 21:05 metamaskbot

Failing test-deps-audit job addressed in #24664.

legobeat avatar May 21 '24 01:05 legobeat

Builds ready [4cb8b13]
Page Load Metrics (165 ± 217 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint651057484
domContentLoaded9391163
load532139165453217
domInteractive9391163
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: 6.58 KiB (0.10%)
  • common: -1007 Bytes (-0.02%)

metamaskbot avatar May 21 '24 01:05 metamaskbot

Builds ready [a9cfbf2]
Page Load Metrics (399 ± 366 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint62176962814
domContentLoaded1065242110
load512287399763366
domInteractive1065242110
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: 6.58 KiB (0.10%)
  • common: -1007 Bytes (-0.02%)

metamaskbot avatar May 21 '24 17:05 metamaskbot

Hey @MetaMask/confirmations-system-team, your review is the last one missing to merge this PR please 🙏

wachunei avatar May 23 '24 18:05 wachunei

Builds ready [d75e639]
Page Load Metrics (180 ± 221 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint59244914120
domContentLoaded9150193115
load472184180461221
domInteractive9150193115
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: 6.58 KiB (0.10%)
  • common: -1007 Bytes (-0.02%)

metamaskbot avatar May 24 '24 15:05 metamaskbot

Hey @georgeweiler, is the test-e2e-chrome-mmi passing for you locally? I have ran the CI 7 times and all failed on the same suite

gantunesr avatar Jun 26 '24 23:06 gantunesr

Builds ready [7ee6250]
Page Load Metrics (158 ± 160 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint742541113919
domContentLoaded10180383718
load411602158334160
domInteractive10180383718
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: 6.66 KiB (0.10%)
  • common: -1021 Bytes (-0.02%)

metamaskbot avatar Jun 27 '24 14:06 metamaskbot