OpenSea V2 -> V1 compatibility

[bergeron]

Explanation

Moves to using OpenSea V2 API, but modifies the responses to keep compatibility with the existing controller state schema.

References

Changelog

@metamask/controller-utils

• BREAKING: OPENSEA_PROXY_URL now points to OpenSea's v2 API. OPENSEA_API_URL + OPENSEA_TEST_API_URL have been removed.

@metamask/assets-controllers

• BREAKING:
  • NftDetectionController constructor now requires the NftController.getNftApi function.
  • NFT controllers will no longer return last_sale information for NFTs fetched after the OpenSea V2 update

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

[mcmire]

I think there was just one field missing from mapOpenSeaNftV2ToV1 that I noticed, but this looks good to me so far otherwise! A lot of the comments below are just about improving types.

[bergeron]

I will strongly type the v2 responses. It could be convenient to generate these from swagger / use the OpenSea typescript SDK. But I've noticed at least 3 places where the spec is wrong, and for quickness i'll just add some types since its only a couple models.

[bergeron]

In all these examples I found, OS falls back to the original, non-CDN URL when you fetch the NFT individually. V1 also did this when listing, but in a separate field image_original_url, which is how those used to render.

Alternatively we could fetch metadata_url and grab the image field, but was more anonying to handle ipfs, arweave. The timeout was more for when I was doing that approach and may not be needed.

[Gudahtt]

Good call on not fetching the metadata. That would have had complicated privacy implications (for metadata URLs that aren't IPFS, at least)

[Gudahtt]

Hmm, exposing this for another controller to use is typically something we'd avoid. Lately we've been trying to extract network calls from controllers altogether, moving them to separate "API service" classes.

But this seems OK for now, we can refactor this later.

[mcmire]

This looks great! Thanks so much for taking care of this.

[Gudahtt]

LGTM!

[Gudahtt]

Hmm, this could mean a lot more network requests.

I wonder if we could cache these somehow to prevent this.

[bergeron]

Yeah. Although it's not the common case. Saw no occurrences in most wallets, and only on a few tokens when it does occur. I could tally more stats on the %. Also the && metadata_url avoids doing it some cases where it wouldn't return anything useful.

[Gudahtt]

Good point! OK, maybe we highlight it to the mobile and extension teams as a risk, and we can add caching later if necessary

[bergeron]

Was testing occurrences with a script like this:

let next = '', account = '';
do {
  const nfts = await(
    await fetch(
      `https://api.opensea.io/api/v2/chain/ethereum/account/${account}/nfts?limit=200&next=${next}`,
      { headers: { 'x-api-key': '' } }
    )).json();

  for (nft of nfts.nfts) {
    if (!nft.image_url && nft.metadata_url) {
      console.log(nft);
    }
  }
  next = nfts.next;
} while (next);