-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
feat: Open Sea to Reservoir migration #9032
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
Conversation
|
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. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9032 +/- ##
==========================================
+ Coverage 45.50% 45.59% +0.08%
==========================================
Files 1275 1276 +1
Lines 31202 31311 +109
Branches 3184 3203 +19
==========================================
+ Hits 14199 14276 +77
- Misses 16163 16189 +26
- Partials 840 846 +6 ☔ View full report in Codecov by Sentry. |
|
|
NicolasMassart
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
I reviewed the patch outside of github as it's too big to display.
I was able to confirm the change to new service and no conneciton to opensea
# HTTP request for nft.api.cx.metamask.io/tokens (ID: c2fbdaaa-3de8-4376-8bad-34b718d11279)
## Request
HTTP GET https://nft.api.cx.metamask.io/tokens?chainIds=1&tokens=...&includeTopBid=true&includeAttributes=true&includeLastSale=true
Accept-Encoding: gzip
Connection: Keep-Alive
Cookie: ...
Host: nft.api.cx.metamask.io
User-Agent: okhttp/4.9.2
version: 1
## Response
HTTP 200 undefined
| -exports.OPENSEA_PROXY_URL = 'https://proxy.metafi.codefi.network/opensea/v1/api/v1'; | ||
| -exports.OPENSEA_API_URL = 'https://api.opensea.io/api/v1'; | ||
| -exports.OPENSEA_TEST_API_URL = 'https://testnets-api.opensea.io/api/v1'; | ||
| +exports.OPENSEA_PROXY_URL = 'https://proxy.metafi.codefi.network/opensea/v1/api/v2'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps these could be made configurable, instead of harcoded constants, in the upstream package? Would simplify this, remove the need for a patch, and these could be treated like any build config.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @legobeat !
Are there examples of how we can make the url configurable? also do you think it can be done on a separate PR?
I will close this PR and create a new one because the assets-controller version on mobile has been upgraded and i will be using a different core branch to create the patch 🙏
|
Closing this one in favor of this #9547 |



Description
This PR adds patches to mobile to enable Open Sea (OS) migration.
It also fixes a small bug where this component app/components/UI/CollectibleContracts/index.js when on mainnet keeps making calls to OS while the NFTs are already detected.
Related issues
MMASSETS-156. (Restricted access Jira issue)
Main core PR: MetaMask/core#4030
Core Compare link used to create the assets patch: https://github.com/MetaMask/core/compare/patch/mobile-assets-controllers-v-12-0-0...patch/mobile-assets-controllers-v-12-0-0-res-migration?expand=1
Context
Problem: We want to roll out Reservoir to Extension and Mobile similar to how we have done in Portfolio.
Requirements:
Define a way to compare OpenSea’s spam filtering and Reservoir’s spam filtering. Ideally the spam filtering is similar.
Create Core PR to migrate calls from OS to Reservoir.
Create PR on mobile to migrate from OS to Reservoir.
Create Extension PR to migrate from OS to Reservoir
Define roll out plan for Extension and Mobile
Roll out Reservoir to Extension and Mobile
Today, to get the NFTs core is calling opensea V2 api to fetch user nfts and to add new nfts (get NFT metadata)
In this ticket we wanted to call Reservoir API instead of calling openseaV2.
We are not changing any of the functional behaviors of the app, we are just swapping third party providers.
So the app should behave the exact same regarding any NFT related functionality:
Like showing NFT details when clicking on the NFT
And being able to remove/add the NFT
Mark it as favorite
Sending NFts
Manual testing steps
Screenshots/Recordings
The NFT tab should behave the same. You should be able to see the same NFTs you had.
Only thing noticed while testing is for some collections you might see either a different image or the mobile default image (if the new third party provider was not able to return the image)
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist