[LIVE-3722][LIVE-3744][LIVE-3722] - Bugfix: Fix currencies inferences (…

…#1304)

* Add missing currencies to CLI

* Update wrong inferTransactions not returning txs in CLI

* Update findCryptoCurrencyByKeyword looking for managerAppName first which will fail with currencies with the same nano app

* improve typing

* Add "keywords" key to currency and update the findByKeyword method

* Add keywords to TOP 30 in marketcap currencies

* Update currencies tests

* Make findCryptoCurrencyByKeyword tests customisable

* Reorg currencies as a temp fix to collisions

* Add polyfill test to check ethereum is inferred correctly

* Update doc

* changesets

* Fix sortByMarketcap inferring by ticker (collisions)

* Add a notion of "priority" to currency sorting in registerCryptoCurrency to handle ticker collision

* Update cryptoassets doc

* Fix currencies list in accounts mock

* Update snapshot

Co-authored-by: Gaëtan Renaudeau <[email protected]>

.changeset/old-lemons-smile.md

@@ -0,0 +1,6 @@
+---
+"@ledgerhq/live-cli": patch
+"@ledgerhq/live-common": patch
+---
+
+Fix wrong inferences for currencies + add tests to make sure we catch the issue next time

.changeset/tidy-items-remain.md

@@ -0,0 +1,6 @@
+---
+"@ledgerhq/cryptoassets": patch
+"@ledgerhq/types-cryptoassets": patch
+---
+
+Fixing currency inferences in cryptoassets, and reorder currencies as a temporary fix for the ticker/managerAppName collisions' side effects

apps/cli/src/live-common-setup-base.ts

@@ -51,6 +51,9 @@ setSupportedCurrencies([
   "osmosis",
   "fantom",
   "moonbeam",
+  "cronos",
+  "songbird",
+  "flare"
 ]);
 
 for (const k in process.env) setEnvUnsafe(k as EnvName, process.env[k]);

apps/cli/src/transaction.ts

@@ -5,12 +5,11 @@ import uniqBy from "lodash/uniqBy";
 import shuffle from "lodash/shuffle";
 import flatMap from "lodash/flatMap";
 import { BigNumber } from "bignumber.js";
-import type {
-  Transaction,
-} from "@ledgerhq/live-common/generated/types";
+import type { Transaction } from "@ledgerhq/live-common/generated/types";
 import type {
   Account,
   AccountLike,
+  AccountLikeArray,
   TransactionStatusCommon,
 } from "@ledgerhq/types-live";
 import perFamily from "@ledgerhq/live-common/lib/generated/cli-transaction";
@@ -106,20 +105,34 @@ export async function inferTransactions(
   const bridge = getAccountBridge(mainAccount, null);
   const specific = perFamily[mainAccount.currency.family];
 
-  const inferAccounts =
+  const inferAccounts: (
+    account: Account,
+    opts: Record<string, any>
+  ) => AccountLikeArray =
     (specific && specific.inferAccounts) || ((account, _opts) => [account]);
 
-  const inferTransactions =
+  const inferTransactions: (
+    transactions: Array<{
+      account: AccountLike;
+      transaction: Transaction;
+    }>,
+    opts: Record<string, any>,
+    { inferAmount }: any
+  ) => Transaction[] =
     (specific && specific.inferTransactions) ||
-    ((transactions, _opts, _r) => transactions);
+    ((inferred, _opts, _r) => inferred.map(({ transaction }) => transaction));
 
-  let all: any[] = await Promise.all(
+  let all: Array<{
+    account: AccountLike;
+    transaction: Transaction;
+    mainAccount: Account;
+  }> = await Promise.all(
     product(
       inferAccounts(mainAccount, opts),
       opts.recipient || [
         opts["self-transaction"] ? mainAccount.freshAddress : "",
       ]
-    ).map(async ([account, recipient]) => {
+    ).map(async ([account, recipient]: [AccountLike, string]) => {
       const transaction = bridge.createTransaction(mainAccount);
       transaction.recipient = recipient;
       transaction.useAllAmount = !!opts["use-all-amount"];
@@ -148,21 +161,22 @@ export async function inferTransactions(
     all = shuffle(all);
   }
 
-  const transactions: [Transaction, TransactionStatusCommon][] = await Promise.all(
-    inferTransactions(all, opts, {
-      inferAmount,
-    }).map(async (transaction) => {
-      const tx = await bridge.prepareTransaction(mainAccount, transaction);
-      const status = await bridge.getTransactionStatus(mainAccount, tx);
-      const errorKeys = Object.keys(status.errors);
-
-      if (errorKeys.length) {
-        throw status.errors[errorKeys[0]];
-      }
-
-      return [tx, status];
-    })
-  );
+  const transactions: [Transaction, TransactionStatusCommon][] =
+    await Promise.all(
+      inferTransactions(all, opts, {
+        inferAmount,
+      }).map(async (transaction) => {
+        const tx = await bridge.prepareTransaction(mainAccount, transaction);
+        const status = await bridge.getTransactionStatus(mainAccount, tx);
+        const errorKeys = Object.keys(status.errors);
+
+        if (errorKeys.length) {
+          throw status.errors[errorKeys[0]];
+        }
+
+        return [tx, status];
+      })
+    );
 
   return transactions;
 }

libs/ledger-live-common/src/__tests__/__snapshots__/portfolio.v2.ts.snap

@@ -28,6 +28,10 @@ Object {
         ],
         "family": "bitcoin",
         "id": "bitcoin",
+        "keywords": Array [
+          "btc",
+          "bitcoin",
+        ],
         "managerAppName": "Bitcoin",
         "name": "Bitcoin",
         "scheme": "bitcoin",
@@ -1800,6 +1804,10 @@ Object {
         ],
         "family": "bitcoin",
         "id": "bitcoin",
+        "keywords": Array [
+          "btc",
+          "bitcoin",
+        ],
         "managerAppName": "Bitcoin",
         "name": "Bitcoin",
         "scheme": "bitcoin",
@@ -4137,6 +4145,10 @@ Object {
         ],
         "family": "bitcoin",
         "id": "bitcoin",
+        "keywords": Array [
+          "btc",
+          "bitcoin",
+        ],
         "managerAppName": "Bitcoin",
         "name": "Bitcoin",
         "scheme": "bitcoin",
@@ -4773,6 +4785,10 @@ Object {
         ],
         "family": "bitcoin",
         "id": "bitcoin",
+        "keywords": Array [
+          "btc",
+          "bitcoin",
+        ],
         "managerAppName": "Bitcoin",
         "name": "Bitcoin",
         "scheme": "bitcoin",

libs/ledger-live-common/src/apps/polyfill.test.ts

@@ -0,0 +1,24 @@
+import { Application } from "@ledgerhq/types-live";
+import { polyfillApplication } from "./polyfill";
+
+test("polyfillApplication set ethereum currency for ethereum app", () => {
+  const app: Application = {
+    id: 1,
+    name: "Ethereum",
+    description: undefined,
+    application_versions: [],
+    providers: [],
+    category: 0,
+    date_creation: "",
+    date_last_modified: "",
+    publisher: undefined,
+    currencyId: undefined,
+    authorName: undefined,
+    supportURL: undefined,
+    contactURL: undefined,
+    sourceURL: undefined,
+    compatibleWalletsJSON: undefined,
+  };
+  const polyfilled = polyfillApplication(app);
+  expect(polyfilled.currencyId).toEqual("ethereum");
+});

libs/ledger-live-common/src/currencies/__snapshots__/sortByMarketcap.test.ts.snap

@@ -1354,26 +1354,26 @@ Array [
   "ethereum/erc20/holotoken",
   "ethereum/erc20/etoro_euro",
   "near",
-  "arbitrum",
   "avalanche",
   "banano",
   "celo",
-  "cronos",
   "dexon",
   "elrond",
   "fic",
-  "flare",
   "flow",
   "icp",
   "LBRY",
-  "moonbeam",
   "moonriver",
   "musicoin",
-  "songbird",
   "reosc",
   "resistance",
   "osmo",
   "shyft",
+  "arbitrum",
+  "cronos",
+  "flare",
+  "songbird",
+  "moonbeam",
   "ethereum/erc20/$aapl",
   "ethereum/erc20/$based",
   "ethereum/erc20/$die",

libs/ledger-live-common/src/currencies/sortByMarketcap.ts

@@ -20,7 +20,12 @@ function lazyLoadTickerMap() {
     }
   });
   listCryptoCurrencies().forEach((c) => {
-    if (!c.disableCountervalue) {
+    if (
+      !c.disableCountervalue &&
+      // FIXME Avoid duplicates & override if the already set currency is a token
+      // E.g. 'Binance-Peg Ethereum Token' has an 'ETH' ticker
+      (!m.has(c.ticker) || m.get(c.ticker)?.type !== "CryptoCurrency")
+    ) {
       m.set(c.ticker, c);
     }
   });

libs/ledger-live-common/src/mock/account.ts

@@ -183,25 +183,22 @@ const hardcodedMarketcap = [
   "algorand/asa/163650",
 ];
 // for the mock generation we need to adjust to the actual market price of things, we want to avoid having things < 0.01 EUR
-const tickerApproxMarketPrice = {
-  BTC: 0.0073059,
-  ETH: 5.7033e-14,
-  ETC: 1.4857e-15,
-  BCH: 0.0011739,
-  BTG: 0.00005004,
-  LTC: 0.00011728,
-  XRP: 0.000057633,
-  DOGE: 4.9e-9,
-  DASH: 0.0003367,
-  PPC: 0.000226,
-  ZEC: 0.000205798,
+const currencyIdApproxMarketPrice = {
+  bitcoin: 0.0073059,
+  ethereum: 5.7033e-14,
+  ethereum_classic: 1.4857e-15,
+  bitcoin_cash: 0.0011739,
+  bitcoin_gold: 0.00005004,
+  litecoin: 0.00011728,
+  ripple: 0.000057633,
+  dogecoin: 4.9e-9,
+  dash: 0.0003367,
+  peercoin: 0.000226,
+  zcash: 0.000205798,
 };
 // mock only use subset of cryptocurrencies to not affect tests when adding coins
 const currencies = listCryptoCurrencies().filter(
-  (c) =>
-    tickerApproxMarketPrice[c.ticker] &&
-    // take only "official" currencies, otherwise you might end up with ticker collisions
-    c.name === c.managerAppName
+  (c) => currencyIdApproxMarketPrice[c.id]
 );
 // TODO fix the mock to never generate negative balance...
 
@@ -225,11 +222,12 @@ export function genOperation(
   );
   const address = genAddress(superAccount.currency, rng);
   const type = rng.next() < 0.3 ? "OUT" : "IN";
+  const divider =
+    (account.type === "Account" &&
+      currencyIdApproxMarketPrice[account.currency.id]) ||
+    currencyIdApproxMarketPrice.bitcoin;
   const value = new BigNumber(
-    Math.floor(
-      rng.nextInt(0, 100000 * rng.next() * rng.next()) /
-        (tickerApproxMarketPrice[ticker] || tickerApproxMarketPrice.BTC)
-    )
+    Math.floor(rng.nextInt(0, 100000 * rng.next() * rng.next()) / divider)
   );
 
   if (Number.isNaN(value)) {

libs/ledgerjs/packages/cryptoassets/README.md

@@ -85,6 +85,8 @@ Returns **[string](https://developer.mozilla.org/docs/Web/JavaScript/Reference/G
 *   `id` **[string](https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/String)** 
 *   `currency` **CryptoCurrency** 
 
+Returns **void** 
+
 ### listCryptoCurrencies
 
 #### Parameters
@@ -131,6 +133,7 @@ Returns **(CryptoCurrency | null | [undefined](https://developer.mozilla.org/doc
 #### Parameters
 
 *   `keyword` **any** 
+*   `tests`   (optional, default `["keywords","name","id","ticker","manager"]`)
 
 Returns **(CryptoCurrency | null | [undefined](https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/undefined))** 
 

libs/ledgerjs/packages/cryptoassets/src/currencies.test.ts

@@ -19,6 +19,7 @@ import {
   findCryptoCurrencyByTicker,
   findCryptoCurrencyByKeyword,
   registerCryptoCurrency,
+  cryptocurrenciesById,
 } from "./currencies";
 
 test("can get currency by coin type", () => {
@@ -37,24 +38,69 @@ test("can get currency by coin type", () => {
   expect(() => getCryptoCurrencyById("_")).toThrow();
 });
 
-test("can find currency", () => {
+// Unique list in case of dup in the currency list
+const currencies = Array.from(new Set(Object.values(cryptocurrenciesById)));
+const duplicatedTickers: Set<string> = new Set();
+currencies.reduce((acc, curr) => {
+  if (acc.includes(curr.ticker)) {
+    duplicatedTickers.add(curr.ticker);
+  }
+  acc.push(curr.ticker);
+  return acc;
+}, [] as string[]);
+
+currencies.forEach((c) => {
+  test(`should find ${c.name} by id`, () => {
+    expect(findCryptoCurrencyById(c.id)).toEqual(c);
+  });
+
+  test(`should find ${c.name} by ticker`, () => {
+    try {
+      expect(findCryptoCurrencyByTicker(c.ticker)).toEqual(c);
+    } catch (e) {
+      // Should throw only if the ticker is not duplicated or if it's not a testnet (see conditions in `cryptocurrenciesByTicker`)
+      if (!duplicatedTickers.has(c.ticker) && !c.isTestnetFor) {
+        throw e;
+      }
+    }
+  });
+
+  test(`should find ${c.name} by scheme`, () => {
+    expect(findCryptoCurrencyByScheme(c.scheme)).toEqual(c);
+  });
+
+  c.keywords?.forEach((k) => {
+    test(`should find ${c.name} with keyword ${k}`, () => {
+      expect(findCryptoCurrencyByKeyword(k)).toEqual(c);
+    });
+  });
+});
+
+test("[LEGACY TEST] can find currency", () => {
   const bitcoinMatch = {
     id: "bitcoin",
     name: "Bitcoin",
   };
+  const ethereumMatch = {
+    id: "ethereum",
+    name: "Ethereum",
+  };
+
   expect(findCryptoCurrency((c) => c.name === "Bitcoin")).toMatchObject(
     bitcoinMatch
   );
   expect(findCryptoCurrencyById("bitcoin")).toMatchObject(bitcoinMatch);
   expect(findCryptoCurrencyByKeyword("btc")).toMatchObject(bitcoinMatch);
-  expect(findCryptoCurrencyByKeyword("btc")).toMatchObject(bitcoinMatch);
-  expect(findCryptoCurrencyByKeyword("btc")).toMatchObject(bitcoinMatch);
   expect(findCryptoCurrencyByTicker("BTC")).toMatchObject(bitcoinMatch);
   expect(findCryptoCurrencyByScheme("bitcoin")).toMatchObject(bitcoinMatch);
+
+  expect(findCryptoCurrencyById("ethereum")).toMatchObject(ethereumMatch);
+  expect(findCryptoCurrencyByKeyword("eth")).toMatchObject(ethereumMatch);
+  expect(findCryptoCurrencyByTicker("ETH")).toMatchObject(ethereumMatch);
+  expect(findCryptoCurrencyByScheme("ethereum")).toMatchObject(ethereumMatch);
+
   expect(findCryptoCurrencyById("_")).toBe(undefined);
   expect(findCryptoCurrencyByKeyword("_")).toBe(undefined);
-  expect(findCryptoCurrencyByKeyword("_")).toBe(undefined);
-  expect(findCryptoCurrencyByKeyword("_")).toBe(undefined);
   expect(findCryptoCurrencyByTicker("_")).toBe(undefined);
   expect(findCryptoCurrencyByScheme("_")).toBe(undefined);
 });