core icon indicating copy to clipboard operation
core copied to clipboard

feat: reservoir migration

Open sahar-fehri opened this issue 11 months ago • 1 comments

Explanation

Migrates the calls from OpenseaV2 to Reservoir. Removes unused Opensea types and add Reservoir types.

Ideally the types are shared from NFT-API to other consuming clients rather than hardcoding them on core. We will address this in the future.

References

Changelog

@metamask/assets-controllers

  • REMOVED: Removed opensea map functions from assetsUtil.ts like mapOpenSeaNftV2ToV1, mapOpenSeaDetailedNftV2ToV1 and mapOpenSeaContractV2ToV1.
  • ADDED: Added NFT_API_BASE_URL in constants.ts
  • ADDED: Added reservoir types in NftDetectionController.ts
  • REMOVED: Call to openseaV2 to get user nfts in NftDetectionController.ts
  • Added: Call to reservoir to get user nfts in NftDetectionController.ts
  • REMOVED: Removed Opensea types from NftController.ts
  • REMOVED: Removed call Opensea to get NFT information and added call to reservoir in NftController.ts
  • REMOVED: Removed fct getNftContractInformationFromApi that calls OS
  • CHANGED: Changed fct getNftContractInformation to receive nftMetadataFromApi as arg (which contains contract information fetched already from reservoir and aggregate the result with getNftContractInformationFromContract
  • CHANGED: Changed the call to getNftInformation inside addNft function to be before addNftContract, because the nftInformation received from reservoir already includes collection metadata, no need to make another separate call to fetch it.

Checklist

  • [ ] I've updated the test suite for new or updated code as appropriate
  • [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • [ ] I've highlighted breaking changes using the "BREAKING" category above as appropriate

sahar-fehri avatar Mar 07 '24 15:03 sahar-fehri

Generally looks good to me! Love that we can get more information in fewer requests

bergeron avatar Mar 19 '24 20:03 bergeron

This is also non-blocking, but it seems there are a few breaking changes in this PR. Can you note those in Changes? Perhaps we want this:

- **REMOVED**: **BREAKING:** Removed opensea map functions from assetsUtil.ts like `mapOpenSeaNftV2ToV1`,  `mapOpenSeaDetailedNftV2ToV1` and `mapOpenSeaContractV2ToV1`.
- **ADDED**: Added `NFT_API_BASE_URL` in constants.ts
- **ADDED**: Added reservoir types in NftDetectionController.ts
- **REMOVED**: **BREAKING:** Call to openseaV2 to get user nfts in NftDetectionController.ts
- **Added**: Call to reservoir to get user nfts in NftDetectionController.ts
- **REMOVED**: **BREAKING:** Removed Opensea types from NftController.ts
- **REMOVED**: **BREAKING:** Removed call Opensea to get NFT information and added call to reservoir in  NftController.ts
- **REMOVED**: **BREAKING:** Removed fct getNftContractInformationFromApi that calls OS
- **CHANGED**: Changed fct `getNftContractInformation` to receive `nftMetadataFromApi` as arg (which contains contract information fetched already from reservoir and aggregate the result with `getNftContractInformationFromContract`
- **CHANGED**: Changed the call to `getNftInformation` inside `addNft` function to be before  `addNftContract`, because the nftInformation received from reservoir already includes collection metadata, no need to make another separate call to fetch it.

mcmire avatar Mar 20 '24 21:03 mcmire