Fix ipfs address generation

package.json

@@ -61,6 +61,7 @@
     "immer": "^9.0.6",
     "isomorphic-fetch": "^3.0.0",
     "jsonschema": "^1.2.4",
+    "multiformats": "^9.5.2",
     "nanoid": "^3.1.12",
     "punycode": "^2.1.1",
     "single-call-balance-checker-abi": "^1.0.0",

src/assets/CollectiblesController.test.ts

@@ -6,6 +6,7 @@ import {
   NetworkController,
   NetworksChainId,
 } from '../network/NetworkController';
+import { getFormattedIpfsUrl } from '../util';
 import { AssetsContractController } from './AssetsContractController';
 import { CollectiblesController } from './CollectiblesController';
 
@@ -29,8 +30,15 @@ const OPEN_SEA_HOST = 'https://api.opensea.io';
 const OPEN_SEA_PATH = '/api/v1';
 
 const CLOUDFARE_PATH = 'https://cloudflare-ipfs.com/ipfs/';
-const DEPRESSIONIST_IPFS_PATH =
-  '/QmVChNtStZfPyV8JfKpube3eigQh5rUXqYchPgLc91tWLJ';
+
+const DEPRESSIONIST_CID_V1 =
+  'bafybeidf7aw7bmnmewwj4ayq3she2jfk5jrdpp24aaucf6fddzb3cfhrvm';
+
+const DEPRESSIONIST_CLOUDFLARE_IPFS_SUBDOMAIN_PATH = getFormattedIpfsUrl(
+  CLOUDFARE_PATH,
+  `ipfs://${DEPRESSIONIST_CID_V1}`,
+  true,
+);
 
 describe('CollectiblesController', () => {
   let collectiblesController: CollectiblesController;
@@ -167,7 +175,7 @@ describe('CollectiblesController', () => {
         asset_contract: { schema_name: 'ERC1155' },
       });
 
-    nock(CLOUDFARE_PATH).get(DEPRESSIONIST_IPFS_PATH).reply(200, {
+    nock(DEPRESSIONIST_CLOUDFLARE_IPFS_SUBDOMAIN_PATH).get('/').reply(200, {
       name: 'name',
       image: 'image',
       description: 'description',

src/assets/CollectiblesController.ts

@@ -10,7 +10,7 @@ import {
   handleFetch,
   toChecksumHexAddress,
   BNToHex,
-  getIpfsUrlContentIdentifier,
+  getFormattedIpfsUrl,
 } from '../util';
 import {
   MAINNET,
@@ -133,6 +133,7 @@ export interface CollectiblesConfig extends BaseConfig {
   chainId: string;
   ipfsGateway: string;
   openSeaEnabled: boolean;
+  useIPFSSubdomains: boolean;
 }
 
 /**
@@ -246,16 +247,6 @@ export class CollectiblesController extends BaseController<
     return collectibleMetadata;
   }
 
-  private getValidIpfsGatewayFormat() {
-    const { ipfsGateway } = this.config;
-    if (ipfsGateway.endsWith('/ipfs/')) {
-      return ipfsGateway;
-    } else if (ipfsGateway.endsWith('/')) {
-      return `${ipfsGateway}ipfs/`;
-    }
-    return `${ipfsGateway}/ipfs/`;
-  }
-
   /**
    * Request individual collectible information from contracts that follows Metadata Interface.
    *
@@ -267,6 +258,7 @@ export class CollectiblesController extends BaseController<
     contractAddress: string,
     tokenId: string,
   ): Promise<CollectibleMetadata> {
+    const { ipfsGateway, useIPFSSubdomains } = this.config;
     const result = await this.getCollectibleURIAndStandard(
       contractAddress,
       tokenId,
@@ -275,8 +267,7 @@ export class CollectiblesController extends BaseController<
     const standard = result[1];
 
     if (tokenURI.startsWith('ipfs://')) {
-      const contentId = getIpfsUrlContentIdentifier(tokenURI);
-      tokenURI = `${this.getValidIpfsGatewayFormat()}${contentId}`;
+      tokenURI = getFormattedIpfsUrl(ipfsGateway, tokenURI, useIPFSSubdomains);
     }
 
     try {
@@ -852,6 +843,7 @@ export class CollectiblesController extends BaseController<
       chainId: '',
       ipfsGateway: IPFS_DEFAULT_GATEWAY_URL,
       openSeaEnabled: false,
+      useIPFSSubdomains: true,
     };
 
     this.defaultState = {

src/util.test.ts

@@ -14,8 +14,13 @@ const VALID = '4e1fF7229BDdAf0A73DF183a88d9c3a04cc975e0';
 const SOME_API = 'https://someapi.com';
 const SOME_FAILING_API = 'https://somefailingapi.com';
 
-const DEFAULT_IPFS_URL = 'ipfs://0001';
-const ALTERNATIVE_IPFS_URL = 'ipfs://ipfs/0001';
+const DEFAULT_IPFS_URL_FORMAT = 'ipfs://';
+const ALTERNATIVE_IPFS_URL_FORMAT = 'ipfs://ipfs/';
+const IPFS_CID_V0 = 'QmdfTbBqBPQ7VNxZEYEj14VmRuZBkqFbiwReogJgS1zR1n';
+const IPFS_CID_V1 =
+  'bafybeihdwdcefgh4dqkjv67uzcmw7ojee6xedzdetojuzjevtenxquvyku';
+
+const IFPS_GATEWAY = 'dweb.link';
 
 const MAX_FEE_PER_GAS = 'maxFeePerGas';
 const MAX_PRIORITY_FEE_PER_GAS = 'maxPriorityFeePerGas';
@@ -1068,23 +1073,129 @@ describe('util', () => {
     });
   });
 
-  describe('getIpfsUrlContentIdentifier', () => {
-    it('should return content identifier from default ipfs url', () => {
-      expect(util.getIpfsUrlContentIdentifier(DEFAULT_IPFS_URL)).toStrictEqual(
-        '0001',
-      );
+  describe('getFormattedIpfsUrl', () => {
+    it('should return a correctly formatted subdomained ipfs url when passed ipfsGateway without protocol prefix, no path and subdomainSupported argument set to true', () => {
+      expect(
+        util.getFormattedIpfsUrl(
+          IFPS_GATEWAY,
+          `${DEFAULT_IPFS_URL_FORMAT}${IPFS_CID_V1}`,
+          true,
+        ),
+      ).toStrictEqual(`https://${IPFS_CID_V1}.ipfs.${IFPS_GATEWAY}`);
+    });
+
+    it('should return a correctly formatted subdomained ipfs url when passed ipfsGateway with protocol prefix, a cidv0 and no path and subdomainSupported argument set to true', () => {
+      expect(
+        util.getFormattedIpfsUrl(
+          `https://${IFPS_GATEWAY}`,
+          `${DEFAULT_IPFS_URL_FORMAT}${IPFS_CID_V0}`,
+          true,
+        ),
+      ).toStrictEqual(`https://${IPFS_CID_V1}.ipfs.${IFPS_GATEWAY}`);
+    });
+
+    it('should return a correctly formatted subdomained ipfs url when passed ipfsGateway with protocol prefix, a path at the end of the url, and subdomainSupported argument set to true', () => {
+      expect(
+        util.getFormattedIpfsUrl(
+          `https://${IFPS_GATEWAY}`,
+          `${DEFAULT_IPFS_URL_FORMAT}${IPFS_CID_V1}/test`,
+          true,
+        ),
+      ).toStrictEqual(`https://${IPFS_CID_V1}.ipfs.${IFPS_GATEWAY}/test`);
+    });
+
+    it('should return a correctly formatted non-subdomained ipfs url when passed ipfsGateway with no "/ipfs/" appended, a path at the end of the url, and subdomainSupported argument set to false', () => {
+      expect(
+        util.getFormattedIpfsUrl(
+          `https://${IFPS_GATEWAY}`,
+          `${DEFAULT_IPFS_URL_FORMAT}${IPFS_CID_V1}/test`,
+          false,
+        ),
+      ).toStrictEqual(`https://${IFPS_GATEWAY}/ipfs/${IPFS_CID_V1}/test`);
     });
 
-    it('should return content identifier from alternative ipfs url', () => {
+    it('should return a correctly formatted non-subdomained ipfs url when passed an ipfsGateway with "/ipfs/" appended, a path at the end of the url, subdomainSupported argument set to false', () => {
       expect(
-        util.getIpfsUrlContentIdentifier(ALTERNATIVE_IPFS_URL),
-      ).toStrictEqual('0001');
+        util.getFormattedIpfsUrl(
+          `https://${IFPS_GATEWAY}/ipfs/`,
+          `${DEFAULT_IPFS_URL_FORMAT}${IPFS_CID_V1}/test`,
+          false,
+        ),
+      ).toStrictEqual(`https://${IFPS_GATEWAY}/ipfs/${IPFS_CID_V1}/test`);
     });
+  });
 
-    it('should return url if its not a ipfs standard url', () => {
-      expect(util.getIpfsUrlContentIdentifier(SOME_API)).toStrictEqual(
-        SOME_API,
+  describe('removeIpfsProtocolPrefix', () => {
+    it('should return content identifier and path combined string from default ipfs url format', () => {
+      expect(
+        util.removeIpfsProtocolPrefix(
+          `${DEFAULT_IPFS_URL_FORMAT}${IPFS_CID_V0}/test`,
+        ),
+      ).toStrictEqual(`${IPFS_CID_V0}/test`);
+    });
+
+    it('should return content identifier string from default ipfs url format if no path preset', () => {
+      expect(
+        util.removeIpfsProtocolPrefix(
+          `${DEFAULT_IPFS_URL_FORMAT}${IPFS_CID_V0}`,
+        ),
+      ).toStrictEqual(IPFS_CID_V0);
+    });
+
+    it('should return content identifier string from alternate ipfs url format', () => {
+      expect(
+        util.removeIpfsProtocolPrefix(
+          `${ALTERNATIVE_IPFS_URL_FORMAT}${IPFS_CID_V0}`,
+        ),
+      ).toStrictEqual(IPFS_CID_V0);
+    });
+
+    it('should throw error if passed a non ipfs url', () => {
+      expect(() => util.removeIpfsProtocolPrefix(SOME_API)).toThrow(
+        'this method should not be used with non ipfs urls',
+      );
+    });
+  });
+
+  describe('addUrlProtocolPrefix', () => {
+    it('should return a URL with https:// prepended if input URL does not already have it', () => {
+      expect(util.addUrlProtocolPrefix(IFPS_GATEWAY)).toStrictEqual(
+        `https://${IFPS_GATEWAY}`,
       );
     });
+
+    it('should return a URL as is if https:// is already prepended', () => {
+      expect(util.addUrlProtocolPrefix(SOME_API)).toStrictEqual(SOME_API);
+    });
+  });
+
+  describe('getIpfsCIDv1AndPath', () => {
+    it('should return content identifier from default ipfs url format', () => {
+      expect(
+        util.getIpfsCIDv1AndPath(`${DEFAULT_IPFS_URL_FORMAT}${IPFS_CID_V0}`),
+      ).toStrictEqual({ cid: IPFS_CID_V1, path: undefined });
+    });
+
+    it('should return content identifier from alternative ipfs url format', () => {
+      expect(
+        util.getIpfsCIDv1AndPath(
+          `${ALTERNATIVE_IPFS_URL_FORMAT}${IPFS_CID_V0}`,
+        ),
+      ).toStrictEqual({ cid: IPFS_CID_V1, path: undefined });
+    });
+
+    it('should return unchanged content identifier if already v1', () => {
+      expect(
+        util.getIpfsCIDv1AndPath(`${DEFAULT_IPFS_URL_FORMAT}${IPFS_CID_V1}`),
+      ).toStrictEqual({ cid: IPFS_CID_V1, path: undefined });
+    });
+
+    it('should return a path when url contains one', () => {
+      expect(
+        util.getIpfsCIDv1AndPath(
+          `${DEFAULT_IPFS_URL_FORMAT}${IPFS_CID_V1}/test/test/test`,
+        ),
+      ).toStrictEqual({ cid: IPFS_CID_V1, path: '/test/test/test' });
+    });
   });
 });

src/util.ts

@@ -12,6 +12,7 @@ import { ethErrors } from 'eth-rpc-errors';
 import ensNamehash from 'eth-ens-namehash';
 import { TYPED_MESSAGE_SCHEMA, typedSignatureHash } from 'eth-sig-util';
 import { validate } from 'jsonschema';
+import { CID } from 'multiformats/cid';
 import {
   Transaction,
   FetchAllOptions,
@@ -769,19 +770,79 @@ export function validateMinimumIncrease(proposed: string, min: string) {
 }
 
 /**
- * Extracts content identifier from ipfs url.
+ * Removes IPFS protocol prefix from input string.
  *
- * @param url - Ipfs url.
- * @returns Ipfs content identifier as string.
+ * @param ipfsUrl - An IPFS url (e.g. ipfs://{content id})
+ * @returns IPFS content identifier and (possibly) path in a string
+ * @throws Will throw if the url passed is not IPFS.
  */
-export function getIpfsUrlContentIdentifier(url: string): string {
-  if (url.startsWith('ipfs://ipfs/')) {
-    return url.replace('ipfs://ipfs/', '');
-  }
+export function removeIpfsProtocolPrefix(ipfsUrl: string) {
+  if (ipfsUrl.startsWith('ipfs://ipfs/')) {
+    return ipfsUrl.replace('ipfs://ipfs/', '');
+  } else if (ipfsUrl.startsWith('ipfs://')) {
+    return ipfsUrl.replace('ipfs://', '');
+  }
+  // this method should not be used with non-ipfs urls (i.e. startsWith('ipfs://') === true)
+  throw new Error('this method should not be used with non ipfs urls');
+}
 
-  if (url.startsWith('ipfs://')) {
-    return url.replace('ipfs://', '');
+/**
+ * Extracts content identifier and path from an input string.
+ *
+ * @param ipfsUrl - An IPFS URL minus the IPFS protocol prefix
+ * @returns IFPS content identifier (cid) and sub path as string.
+ * @throws Will throw if the url passed is not ipfs.
+ */
+export function getIpfsCIDv1AndPath(
+  ipfsUrl: string,
+): { cid: string; path?: string } {
+  const url = removeIpfsProtocolPrefix(ipfsUrl);
+
+  // check if there is a path
+  // (CID is everything preceding first forward slash, path is everything after)
+  const index = url.indexOf('/');
+  const cid = index !== -1 ? url.substring(0, index) : url;
+  const path = index !== -1 ? url.substring(index) : undefined;
+
+  // We want to ensure that the CID is v1 (https://docs.ipfs.io/concepts/content-addressing/#identifier-formats)
+  // because most cid v0s appear to be incompatible with IPFS subdomains
+  return {
+    cid: CID.parse(cid).toV1().toString(),
+    path,
+  };
+}
+
+/**
+ * Adds URL protocol prefix to input URL string if missing.
+ *
+ * @param urlString - An IPFS URL.
+ * @returns A URL with a https:// prepended.
+ */
+export function addUrlProtocolPrefix(urlString: string): string {
+  if (!urlString.match(/(^http:\/\/)|(^https:\/\/)/u)) {
+    return `https://${urlString}`;
   }
+  return urlString;
+}
 
-  return url;
+/**
+ * Formats URL correctly for use retrieving assets hosted on IPFS.
+ *
+ * @param ipfsGateway - The users preferred IPFS gateway (full URL or just host).
+ * @param ipfsUrl - The IFPS URL pointed at the asset.
+ * @param subdomainSupported - Boolean indicating whether the URL should be formatted with subdomains or not.
+ * @returns A formatted URL, with the user's preferred IPFS gateway and format (subdomain or not), pointing to an asset hosted on IPFS.
+ */
+export function getFormattedIpfsUrl(
+  ipfsGateway: string,
+  ipfsUrl: string,
+  subdomainSupported: boolean,
+): string {
+  const { host, protocol, origin } = new URL(addUrlProtocolPrefix(ipfsGateway));
+  if (subdomainSupported) {
+    const { cid, path } = getIpfsCIDv1AndPath(ipfsUrl);
+    return `${protocol}//${cid}.ipfs.${host}${path ?? ''}`;
+  }
+  const cidAndPath = removeIpfsProtocolPrefix(ipfsUrl);
+  return `${origin}/ipfs/${cidAndPath}`;
 }

yarn.lock

@@ -5730,6 +5730,11 @@ ms@2.1.2, ms@^2.1.1:
   resolved "https://registry.yarnpkg.com/ms/-/ms-2.1.2.tgz#d09d1f357b443f493382a8eb3ccd183872ae6009"
   integrity sha512-sGkPx+VjMtmA6MX27oA4FBFELFCZZ4S4XqeGOXCv68tT+jb3vk/RyaKWP0PTKyWtmLSM0b+adUTEvbs1PEaH2w==
 
+multiformats@^9.5.2:
+  version "9.5.2"
+  resolved "https://registry.yarnpkg.com/multiformats/-/multiformats-9.5.2.tgz#14256e49bac8b6a5ecb558c4d3c347bb94873d65"
+  integrity sha512-nLQ9s7YOVtZdeNOVvCkNyFiZdS3wyq0gvCIvdm7Zy1zw3zBoColJKjMkIPXNdTqT7ruuq+G7HrezIN0cXiAZ0w==
+
 nan@2.13.2:
   version "2.13.2"
   resolved "https://registry.yarnpkg.com/nan/-/nan-2.13.2.tgz#f51dc7ae66ba7d5d55e1e6d4d8092e802c9aefe7"