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

feat(ramps): introduces dynamic support for rampable networks #24041

Merged
merged 93 commits into from
Jun 27, 2024

Conversation

georgeweiler
Copy link
Contributor

@georgeweiler georgeweiler commented Apr 16, 2024

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

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

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.

@georgeweiler georgeweiler requested review from a team as code owners April 16, 2024 00:49
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.

@metamaskbot
Copy link
Collaborator

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%)

Copy link

codecov bot commented Apr 16, 2024

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 ⚠️
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     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@metamaskbot
Copy link
Collaborator

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
Copy link
Collaborator

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%)

@bkirb
Copy link

bkirb commented Apr 18, 2024

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

builds.yml Outdated Show resolved Hide resolved
ui/ducks/ramps/types.ts Outdated Show resolved Hide resolved
ui/ducks/ramps/ramps.ts Outdated Show resolved Hide resolved
},
});
// @ts-expect-error mocked API has mockReset method
RampAPI.getNetworks.mockReset();
Copy link
Member

Choose a reason for hiding this comment

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

request for change: Use jest.Mocked instead of suppressing TS error

gantunesr
gantunesr previously approved these changes Jun 26, 2024
Copy link
Member

@gantunesr gantunesr left a comment

Choose a reason for hiding this comment

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

LGTM. Reviewed mainly the code relevant for the BTC project

@gantunesr
Copy link
Member

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

matthewwalsh0
matthewwalsh0 previously approved these changes Jun 27, 2024
@georgeweiler georgeweiler dismissed stale reviews from matthewwalsh0 and gantunesr via ba2eefa June 27, 2024 12:04
danjm
danjm previously approved these changes Jun 27, 2024
Copy link
Contributor

@danjm danjm left a comment

Choose a reason for hiding this comment

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

LGTM. Approving the privacy snapshot changes

gantunesr
gantunesr previously approved these changes Jun 27, 2024
@georgeweiler georgeweiler dismissed stale reviews from gantunesr and danjm via 7ee6250 June 27, 2024 13:55
@metamaskbot
Copy link
Collaborator

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%)

@georgeweiler georgeweiler merged commit a166147 into develop Jun 27, 2024
70 checks passed
@georgeweiler georgeweiler deleted the ramps-buyable-networks branch June 27, 2024 16:16
@github-actions github-actions bot locked and limited conversation to collaborators Jun 27, 2024
@metamaskbot metamaskbot added the release-12.1.0 Issue or pull request that will be included in release 12.1.0 label Jun 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release-12.1.0 Issue or pull request that will be included in release 12.1.0 team-ramp
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

10 participants