feat(ramps): introduces dynamic support for rampable networks #24041
+795
−202
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
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. |
Builds ready [7c4ba77]
Page Load Metrics (879 ± 575 ms)
Bundle size diffs [🚀 Bundle size reduced!]
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #24041 +/- ##
===========================================
+ Coverage 67.40% 67.43% +0.04%
===========================================
Files 1289 1292 +3
Lines 50248 50269 +21
Branches 13011 13019 +8
===========================================
+ Hits 33865 33897 +32
+ Misses 16383 16372 -11 ☔ View full report in Codecov by Sentry. |
Builds ready [7fe1f84]
Page Load Metrics (352 ± 384 ms)
Bundle size diffs [🚀 Bundle size reduced!]
|
…metamask-extension into ramps-buyable-networks
Builds ready [91521fc]
Page Load Metrics (491 ± 459 ms)
Bundle size diffs [🚀 Bundle size reduced!]
|
|
LGTM for QA, I tested with Base and other networks ✅ |
wachunei
reviewed
Apr 18, 2024
wachunei
reviewed
Apr 18, 2024
wachunei
reviewed
Apr 18, 2024
wachunei
reviewed
Apr 18, 2024
wachunei
reviewed
Apr 18, 2024
wachunei
reviewed
Apr 18, 2024
wachunei
reviewed
Apr 18, 2024
wachunei
reviewed
Apr 18, 2024
wachunei
reviewed
Apr 18, 2024
wachunei
reviewed
Apr 18, 2024
wachunei
previously approved these changes
May 16, 2024
nikoferro
previously approved these changes
May 16, 2024
|
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. |
|
@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 #24589 I would say, at least, that you can remove the instances in the routes component test, which seems to be unnecessary |
…metamask-extension into ramps-buyable-networks
wachunei
approved these changes
May 17, 2024
nikoferro
approved these changes
May 20, 2024
Builds ready [53606d7]
Page Load Metrics (829 ± 501 ms)
Bundle size diffs [🚀 Bundle size reduced!]
|
danjm
approved these changes
May 20, 2024
This comment was marked as resolved.
This comment was marked as resolved.
Builds ready [4cb8b13]
Page Load Metrics (165 ± 217 ms)
Bundle size diffs [🚀 Bundle size reduced!]
|
Closed
Builds ready [a9cfbf2]
Page Load Metrics (399 ± 366 ms)
Bundle size diffs [🚀 Bundle size reduced!]
|
jpuri
approved these changes
May 22, 2024
|
Hey @MetaMask/confirmations-system-team, your review is the last one missing to merge this PR please 🙏 |
Builds ready [d75e639]
Page Load Metrics (180 ± 221 ms)
Bundle size diffs [🚀 Bundle size reduced!]
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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:
Related issues
Fixes:
Manual testing steps
Screenshots/Recordings
Video Description of the changes in the PR (5 min)
https://www.loom.com/share/973960816e7e497aae51ed1cdc3cebf5
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist