[LIVE-1174] - Feature: Upgrade NFT Architecture (#1805)

* Fix wrong NFTResource typing * Update NFT types to ProtoNFT * Update NFT Id to contain currency * Update NFT Helpers for new model * Update Eth API metadata call to include chainId * Move nft metadata resolution to bridge * Update NftMetadaProvider logic to use bridge * Update prepare tx of ERC721/1155 w/ new model * Update CLI for new NFT model * Add getNftCapabilities to nft support * Naming + fixing ERC1155 quantity potentially falsy * Make CLI use the LLC branch hash * Add type to nftMetadataResolver param + use of sync metadata * Remove useless import * Remove useless comment in CLI formatters * Add comment to nftsByCollection helper * Add comments + types to NFT metadata call batchers * Fix sync metadata resolution for Eth family * Add return type to nftMetadataResolver + decodeNftId
LedgerHQ · Apr 6, 2022 · cee4a00 · cee4a00
1 parent 25bc993
commit cee4a00
Show file tree

Hide file tree

Showing 21 changed files with 422 additions and 197 deletions.
diff --git a/cli/package.json b/cli/package.json
@@ -34,7 +34,7 @@
     "@ledgerhq/hw-transport-mocker": "6.24.1",
     "@ledgerhq/hw-transport-node-hid": "6.24.1",
     "@ledgerhq/hw-transport-node-speculos": "6.24.1",
-    "@ledgerhq/live-common": "https://github.com/LedgerHQ/ledger-live-common.git#082c946830a332f764f3c95c0eb9c0572b493825",
+    "@ledgerhq/live-common": "https://github.com/LedgerHQ/ledger-live-common.git#31177aae3a559e17f8452ed8dc6e010cd1d6f41e",
     "@ledgerhq/logs": "6.10.0",
     "@walletconnect/client": "^1.7.1",
     "asciichart": "^1.5.25",

diff --git a/cli/src/commands/sync.ts b/cli/src/commands/sync.ts
@@ -1,6 +1,10 @@
 import { map, switchMap } from "rxjs/operators";
-import { accountFormatters } from "@ledgerhq/live-common/lib/account";
-import { metadataCallBatcher } from "@ledgerhq/live-common/lib/nft";
+import {
+  accountFormatters,
+  decodeAccountId,
+} from "@ledgerhq/live-common/lib/account";
+import { getCryptoCurrencyById } from "@ledgerhq/live-common/lib/currencies";
+import { getCurrencyBridge } from "@ledgerhq/live-common/lib/bridge";
 import { scan, scanCommonOpts } from "../scan";
 import type { ScanCommonOpts } from "../scan";
 export default {
@@ -21,23 +25,29 @@ export default {
     }
   ) =>
     scan(opts).pipe(
-      switchMap(async (account) =>
-        account.nfts?.length
+      switchMap(async (account) => {
+        const { currencyId } = decodeAccountId(account.id);
+        const currency = getCryptoCurrencyById(currencyId);
+        const currencyBridge = getCurrencyBridge(currency);
+        const { nftMetadataResolver } = currencyBridge;
+
+        return account.nfts?.length && nftMetadataResolver
           ? {
               ...account,
               nfts: await Promise.all(
                 account.nfts.map(async (nft) => {
-                  const { result: metadata } = await metadataCallBatcher.load({
-                    contract: nft.collection.contract,
+                  const { result: metadata } = await nftMetadataResolver({
+                    contract: nft.contract,
                     tokenId: nft.tokenId,
+                    currencyId: nft.currencyId,
                   });
 
                   return { ...nft, metadata };
                 })
               ).catch(() => account.nfts),
             }
-          : account
-      ),
+          : account;
+      }),
       map((account) =>
         (accountFormatters[opts.format] || accountFormatters.default)(account)
       )

diff --git a/cli/yarn.lock b/cli/yarn.lock
@@ -1087,18 +1087,14 @@
   dependencies:
     bignumber.js "^9.0.1"
     json-rpc-2.0 "^0.2.16"
-
-"@ledgerhq/live-common@https://github.com/LedgerHQ/ledger-live-common.git#082c946830a332f764f3c95c0eb9c0572b493825":
-  version "21.35.0"
-  resolved "https://github.com/LedgerHQ/ledger-live-common.git#082c946830a332f764f3c95c0eb9c0572b493825"
-  dependencies:
-    "@celo/contractkit" "^1.5.2"
-    "@celo/wallet-base" "^1.5.2"
-    "@celo/wallet-ledger" "^1.5.2"
-    "@cosmjs/crypto" "^0.26.5"
-    "@cosmjs/ledger-amino" "^0.26.5"
-    "@cosmjs/proto-signing" "^0.26.5"
-    "@cosmjs/stargate" "^0.26.5"
+
+"@ledgerhq/live-common@https://github.com/LedgerHQ/ledger-live-common.git#31177aae3a559e17f8452ed8dc6e010cd1d6f41e":
+  version "21.32.4"
+  resolved "https://github.com/LedgerHQ/ledger-live-common.git#31177aae3a559e17f8452ed8dc6e010cd1d6f41e"
+  dependencies:
+    "@celo/contractkit" "^1.5.1"
+    "@celo/wallet-base" "^1.5.1"
+    "@celo/wallet-ledger" "^1.5.1"
     "@crypto-com/chain-jslib" "0.0.19"
     "@ethereumjs/common" "^2.6.2"
     "@ethereumjs/tx" "^3.5.0"

diff --git a/src/account/formatters.ts b/src/account/formatters.ts
@@ -6,7 +6,7 @@ import {
   getAccountName,
   getAccountUnit,
 } from ".";
-import type { Account, Operation, Unit } from "../types";
+import type { Account, Operation, ProtoNFT, Unit } from "../types";
 import { getOperationAmountNumberWithInternals } from "../operation";
 import { formatCurrencyUnit } from "../currencies";
 import { getOperationAmountNumber } from "../operation";
@@ -138,13 +138,11 @@ const cliFormat = (account, level?: string) => {
     const NFTCollections = nftsByCollections(nfts);
 
     str += "\n";
-    str += `NFT Collections (${NFTCollections.length}) `;
+    str += `NFT Collections (${Object.keys(NFTCollections).length}) `;
     str += "\n";
 
-    str += NFTCollections.map(
-      // nfts are set to any because there not just NFT, we added a metadata prop on the fly
-      // in the first step of the Rxjs flow to avoid having some async code here
-      ({ contract, nfts }: { contract: string; nfts: any[] }) => {
+    str += Object.entries(NFTCollections)
+      .map(([contract, nfts]: [string, ProtoNFT[]]) => {
         const tokenName = nfts?.[0]?.metadata?.tokenName;
         const { bold, magenta, cyan, reverse } = styling;
 
@@ -162,8 +160,8 @@ const cliFormat = (account, level?: string) => {
             )
             .join()
         );
-      }
-    ).join("\n");
+      })
+      .join("\n");
   }
 
   if (level === "basic") return str;

diff --git a/src/account/serialization.ts b/src/account/serialization.ts
@@ -16,8 +16,8 @@ import type {
   OperationRaw,
   SubAccount,
   SubAccountRaw,
-  NFT,
-  NFTRaw,
+  ProtoNFT,
+  ProtoNFTRaw,
 } from "../types";
 import type { TronResources, TronResourcesRaw } from "../families/tron/types";
 import {
@@ -949,20 +949,42 @@ export function toAccountRaw({
   return res;
 }
 
-export function toNFTRaw({ id, tokenId, amount, collection }: NFT): NFTRaw {
+export function toNFTRaw({
+  id,
+  tokenId,
+  amount,
+  contract,
+  standard,
+  currencyId,
+  metadata,
+}: ProtoNFT): ProtoNFTRaw {
   return {
     id,
     tokenId,
     amount: amount.toFixed(),
-    collection,
+    contract,
+    standard,
+    currencyId,
+    metadata,
   };
 }
 
-export function fromNFTRaw({ id, tokenId, amount, collection }: NFTRaw): NFT {
+export function fromNFTRaw({
+  id,
+  tokenId,
+  amount,
+  contract,
+  standard,
+  currencyId,
+  metadata,
+}: ProtoNFTRaw): ProtoNFT {
   return {
     id,
     tokenId,
     amount: new BigNumber(amount),
-    collection,
+    contract,
+    standard,
+    currencyId,
+    metadata,
   };
 }
diff --git a/src/api/Ethereum.ts b/src/api/Ethereum.ts
@@ -96,7 +96,10 @@ export type API = {
   getAccountNonce: (address: string) => Promise<number>;
   broadcastTransaction: (signedTransaction: string) => Promise<string>;
   getERC20Balances: (input: ERC20BalancesInput) => Promise<ERC20BalanceOutput>;
-  getNFTMetadata: (input: NFTMetadataInput) => Promise<NFTMetadataResponse[]>;
+  getNFTMetadata: (
+    input: NFTMetadataInput,
+    chainId: string
+  ) => Promise<NFTMetadataResponse[]>;
   getAccountBalance: (address: string) => Promise<BigNumber>;
   roughlyEstimateGasLimit: (address: string) => Promise<BigNumber>;
   getERC20ApprovalsPerContract: (
@@ -205,12 +208,12 @@ export const apiForCurrency = (currency: CryptoCurrency): API => {
       return data;
     },
 
-    async getNFTMetadata(input) {
+    async getNFTMetadata(input, chainId) {
       const { data }: { data: NFTMetadataResponse[] } = await network({
         method: "POST",
-        url:
-          getEnv("NFT_ETH_METADATA_SERVICE") +
-          "/v1/ethereum/1/contracts/tokens/infos",
+        url: `${getEnv(
+          "NFT_ETH_METADATA_SERVICE"
+        )}/v1/ethereum/${chainId}/contracts/tokens/infos`,
         data: input,
       });
 

diff --git a/src/bridge/jsHelpers.ts b/src/bridge/jsHelpers.ts
@@ -34,7 +34,7 @@ import type {
   SyncConfig,
   CryptoCurrency,
   DerivationMode,
-  NFT,
+  ProtoNFT,
 } from "../types";
 import type { CurrencyBridge, AccountBridge } from "../types/bridge";
 import getAddress from "../hw/getAddress";
@@ -137,9 +137,12 @@ Operation[] {
   return all;
 }
 
-export const mergeNfts = (oldNfts: NFT[], newNfts: NFT[]): NFT[] => {
+export const mergeNfts = (
+  oldNfts: ProtoNFT[],
+  newNfts: ProtoNFT[]
+): ProtoNFT[] => {
   // Getting a map of id => NFT
-  const newNftsPerId: Record<string, NFT> = {};
+  const newNftsPerId: Record<string, ProtoNFT> = {};
   newNfts.forEach((n) => {
     newNftsPerId[n.id] = n;
   });

diff --git a/src/families/ethereum/bridge/js.ts b/src/families/ethereum/bridge/js.ts
@@ -33,6 +33,7 @@ import { signOperation } from "../signOperation";
 import { modes } from "../modules";
 import postSyncPatch from "../postSyncPatch";
 import { inferDynamicRange } from "../../../range";
+import nftMetadataResolver from "../nftMetadataResolver";
 
 const receive = makeAccountBridgeReceive();
 
@@ -210,6 +211,7 @@ const currencyBridge: CurrencyBridge = {
   preload,
   hydrate,
   scanAccounts,
+  nftMetadataResolver,
 };
 const accountBridge: AccountBridge<Transaction> = {
   createTransaction,

diff --git a/src/families/ethereum/modules/erc1155.ts b/src/families/ethereum/modules/erc1155.ts
@@ -11,10 +11,10 @@ import type { ModeModule, Transaction } from "../types";
 import type { Account } from "../../../types";
 import { prepareTransaction } from "./erc721";
 
-const notOwnedNft = createCustomErrorClass("NotOwnedNft");
-const notEnoughNftOwned = createCustomErrorClass("NotEnoughNftOwned");
-const notTokenIdsProvided = createCustomErrorClass("NotTokenIdsProvided");
-const quantityNeedsToBePositive = createCustomErrorClass(
+const NotOwnedNft = createCustomErrorClass("NotOwnedNft");
+const NotEnoughNftOwned = createCustomErrorClass("NotEnoughNftOwned");
+const NotTokenIdsProvided = createCustomErrorClass("NotTokenIdsProvided");
+const QuantityNeedsToBePositive = createCustomErrorClass(
   "QuantityNeedsToBePositive"
 );
 
@@ -47,8 +47,8 @@ const erc1155Transfer: ModeModule = {
       }
 
       t.quantities?.forEach((quantity) => {
-        if (quantity.isLessThan(1)) {
-          result.errors.amount = new quantityNeedsToBePositive();
+        if (!quantity || quantity.isLessThan(1)) {
+          result.errors.amount = new QuantityNeedsToBePositive();
         }
       });
 
@@ -62,15 +62,15 @@ const erc1155Transfer: ModeModule = {
           const transferQuantity = Number(t.quantities?.[index]);
 
           if (!nft) {
-            return new notOwnedNft();
+            return new NotOwnedNft();
           }
 
           if (transferQuantity && !nft.amount.gte(transferQuantity)) {
-            return new notEnoughNftOwned();
+            return new NotEnoughNftOwned();
           }
 
           return true;
-        }, true as true | Error) || new notTokenIdsProvided();
+        }, true as true | Error) || new NotTokenIdsProvided();
 
       if (!enoughTokensOwned || enoughTokensOwned instanceof Error) {
         result.errors.amount = enoughTokensOwned;

diff --git a/src/families/ethereum/modules/erc721.ts b/src/families/ethereum/modules/erc721.ts
@@ -11,7 +11,7 @@ import type { ModeModule, Transaction } from "../types";
 import type { Account } from "../../../types";
 import { apiForCurrency } from "../../../api/Ethereum";
 
-const notOwnedNft = createCustomErrorClass("NotOwnedNft");
+const NotOwnedNft = createCustomErrorClass("NotOwnedNft");
 
 export type Modes = "erc721.transfer";
 
@@ -23,12 +23,15 @@ export async function prepareTransaction(
   const { collection, collectionName, tokenIds } = transaction;
   if (collection && tokenIds && typeof collectionName === "undefined") {
     const api = apiForCurrency(account.currency);
-    const [{ status, result }] = await api.getNFTMetadata([
-      {
-        contract: collection,
-        tokenId: tokenIds[0],
-      },
-    ]);
+    const [{ status, result }] = await api.getNFTMetadata(
+      [
+        {
+          contract: collection,
+          tokenId: tokenIds[0],
+        },
+      ],
+      account.currency?.ethereumLikeInfo?.chainId?.toString() || "1"
+    );
     let collectionName = ""; // default value fallback if issue
     if (status === 200) {
       collectionName = result?.tokenName || "";
@@ -66,12 +69,10 @@ const erc721Transfer: ModeModule = {
 
       if (
         !a.nfts?.find?.(
-          (n) =>
-            n.tokenId === t.tokenIds?.[0] &&
-            n.collection.contract === t.collection
+          (n) => n.tokenId === t.tokenIds?.[0] && n.contract === t.collection
         )
       ) {
-        result.errors.amount = new notOwnedNft();
+        result.errors.amount = new NotOwnedNft();
       }
     }
   },

diff --git a/src/families/ethereum/nft.merging.unit.test.ts b/src/families/ethereum/nft.merging.unit.test.ts
@@ -1,19 +1,22 @@
 import "../../__tests__/test-helpers/setup";
 import BigNumber from "bignumber.js";
 import { toNFTRaw } from "../../account";
-import type { NFT } from "../../types";
+import type { ProtoNFT } from "../../types";
 import { mergeNfts } from "../../bridge/jsHelpers";
 import { encodeNftId } from "../../nft";
 
 describe("nft merging", () => {
-  const makeNFT = (tokenId: string, contract: string, amount: number): NFT => ({
-    id: encodeNftId("test", contract, tokenId),
+  const makeNFT = (
+    tokenId: string,
+    contract: string,
+    amount: number
+  ): ProtoNFT => ({
+    id: encodeNftId("test", contract, tokenId, "ethereum"),
     tokenId,
     amount: new BigNumber(amount),
-    collection: {
-      contract,
-      standard: "erc721",
-    },
+    contract,
+    standard: "ERC721",
+    currencyId: "ethereum",
   });
   const oldNfts = [
     makeNFT("1", "contract1", 10),