diff --git a/.changeset/nice-bobcats-drop.md b/.changeset/nice-bobcats-drop.md new file mode 100644 index 000000000000..1a95a6d6f90d --- /dev/null +++ b/.changeset/nice-bobcats-drop.md @@ -0,0 +1,5 @@ +--- +"@ledgerhq/hw-app-btc": minor +--- + +make Qtum work with old and new nano app interfaces diff --git a/libs/ledgerjs/packages/hw-app-btc/src/Btc.ts b/libs/ledgerjs/packages/hw-app-btc/src/Btc.ts index f673affbb0ef..dac245c5764c 100644 --- a/libs/ledgerjs/packages/hw-app-btc/src/Btc.ts +++ b/libs/ledgerjs/packages/hw-app-btc/src/Btc.ts @@ -1,3 +1,4 @@ +import semver from "semver"; import type Transport from "@ledgerhq/hw-transport"; import BtcNew from "./BtcNew"; import BtcOld from "./BtcOld"; @@ -49,12 +50,19 @@ export default class Btc { ], scrambleKey, ); - // new APDU (nano app API) for bitcoin and old APDU for altcoin - if (currency === "bitcoin" || currency === "bitcoin_testnet") { - this._impl = new BtcNew(new AppClient(this._transport)); - } else { - this._impl = new BtcOld(this._transport); - } + + this._impl = (() => { + switch (currency) { + case "bitcoin": + case "bitcoin_testnet": + case "qtum": + // new APDU (nano app API) for currencies using app-bitcoin-new implementation + return new BtcNew(new AppClient(this._transport)); + default: + // old APDU (legacy API) for currencies using legacy bitcoin app implementation + return new BtcOld(this._transport); + } + })(); } /** @@ -263,25 +271,31 @@ export default class Btc { // if BtcOld was instantiated, stick with it if (this._impl instanceof BtcOld) return this._impl; - const appAndVersion = await getAppAndVersion(this._transport); - let isBtcLegacy = true; // default for all altcoins + const { name, version } = await getAppAndVersion(this._transport); - if (appAndVersion.name === "Bitcoin" || appAndVersion.name === "Bitcoin Test") { - const [major, minor] = appAndVersion.version.split("."); - // we use the legacy protocol for versions below 2.1.0 of the Bitcoin app. - isBtcLegacy = parseInt(major) <= 1 || (parseInt(major) == 2 && parseInt(minor) == 0); - } else if ( - appAndVersion.name === "Bitcoin Legacy" || - appAndVersion.name === "Bitcoin Test Legacy" - ) { - // the "Bitcoin Legacy" and "Bitcoin Testnet Legacy" app use the legacy protocol, regardless of the version - isBtcLegacy = true; - } else if (appAndVersion.name === "Exchange") { - // We can't query the version of the Bitcoin app if we're coming from Exchange; - // therefore, we use a workaround to distinguish legacy and new versions. - // This can be removed once Ledger Live enforces minimum bitcoin version >= 2.1.0. - isBtcLegacy = await checkIsBtcLegacy(this._transport); - } + const isBtcLegacy = await (async () => { + switch (name) { + case "Bitcoin": + case "Bitcoin Test": { + // we use the legacy protocol for versions below 2.1.0 of the Bitcoin app. + return semver.lt(version, "2.1.0"); + } + case "Bitcoin Legacy": + case "Bitcoin Test Legacy": + // the "Bitcoin Legacy" and "Bitcoin Testnet Legacy" app use the legacy protocol, regardless of the version + return true; + case "Exchange": + // We can't query the version of the Bitcoin app if we're coming from Exchange; + // therefore, we use a workaround to distinguish legacy and new versions. + // This can be removed once Ledger Live enforces minimum bitcoin version >= 2.1.0. + return await checkIsBtcLegacy(this._transport); + case "Qtum": + // we use the legacy protocol for versions below 3.0.0 of the Qtum app. + return semver.lt(version, "3.0.0"); + default: + return true; + } + })(); if (isBtcLegacy) { this._impl = new BtcOld(this._transport); diff --git a/libs/ledgerjs/packages/hw-app-btc/src/BtcNew.ts b/libs/ledgerjs/packages/hw-app-btc/src/BtcNew.ts index b6eb351eec21..7dc4b676230d 100644 --- a/libs/ledgerjs/packages/hw-app-btc/src/BtcNew.ts +++ b/libs/ledgerjs/packages/hw-app-btc/src/BtcNew.ts @@ -445,11 +445,11 @@ function accountTypeFromArg( /* The new protocol only allows standard path. Standard paths are (currently): - M/44'/(1|0)'/X' - M/49'/(1|0)'/X' - M/84'/(1|0)'/X' - M/86'/(1|0)'/X' - M/48'/(1|0)'/X'/Y' + M/44'/(1|0|88)'/X' + M/49'/(1|0|88)'/X' + M/84'/(1|0|88)'/X' + M/86'/(1|0|88)'/X' + M/48'/(1|0|88)'/X'/Y' followed by "", "(0|1)", or "(0|1)/b", where a and b are non-hardened. For example, the following paths are standard M/48'/1'/99'/7' @@ -460,32 +460,60 @@ function accountTypeFromArg( M/48'/0'/99'/7'/1/17/2 // Too many non-hardened derivation steps M/199'/0'/1'/0/88 // Not a known purpose 199 M/86'/1'/99'/2 // Change path item must be 0 or 1 + + Useful resource on derivation paths: https://learnmeabitcoin.com/technical/derivation-paths */ -function isPathNormal(path: string): boolean { - //path is not deepest hardened node of a standard path or deeper, use BtcOld - const h = 0x80000000; //HARDENED from bip32 - const pathElems = pathStringToArray(path); - const hard = (n: number) => n >= h; - const soft = (n: number | undefined) => n === undefined || n < h; - const change = (n: number | undefined) => n === undefined || n === 0 || n === 1; +//path is not deepest hardened node of a standard path or deeper, use BtcOld +const H = 0x80000000; //HARDENED from bip32 + +const VALID_COIN_TYPES = [ + 0, // Bitcoin + 1, // Bitcoin (Testnet) + 88, // Qtum +]; + +const VALID_SINGLE_SIG_PURPOSES = [ + 44, // BIP44 - https://github.com/bitcoin/bips/blob/master/bip-0044.mediawiki + 49, // BIP49 - https://github.com/bitcoin/bips/blob/master/bip-0049.mediawiki + 84, // BIP84 - https://github.com/bitcoin/bips/blob/master/bip-0084.mediawiki + 86, // BIP86 - https://github.com/bitcoin/bips/blob/master/bip-0086.mediawiki +]; + +const VALID_MULTISIG_PURPOSES = [ + 48, // BIP48 - https://github.com/bitcoin/bips/blob/master/bip-0048.mediawiki +]; +const hard = (n: number) => n >= H; +const soft = (n: number | undefined) => n === undefined || n < H; +const change = (n: number | undefined) => n === undefined || n === 0 || n === 1; + +const validCoinPathPartsSet = new Set(VALID_COIN_TYPES.map(t => t + H)); +const validSingleSigPurposePathPartsSet = new Set(VALID_SINGLE_SIG_PURPOSES.map(t => t + H)); +const validMultiSigPurposePathPartsSet = new Set(VALID_MULTISIG_PURPOSES.map(t => t + H)); + +export function isPathNormal(path: string): boolean { + const pathElems = pathStringToArray(path); + + // Single sig if ( pathElems.length >= 3 && pathElems.length <= 5 && - [44 + h, 49 + h, 84 + h, 86 + h].some(v => v == pathElems[0]) && - [0 + h, 1 + h].some(v => v == pathElems[1]) && + validSingleSigPurposePathPartsSet.has(pathElems[0]) && + validCoinPathPartsSet.has(pathElems[1]) && hard(pathElems[2]) && change(pathElems[3]) && soft(pathElems[4]) ) { return true; } + + // Multi sig if ( pathElems.length >= 4 && pathElems.length <= 6 && - 48 + h == pathElems[0] && - [0 + h, 1 + h].some(v => v == pathElems[1]) && + validMultiSigPurposePathPartsSet.has(pathElems[0]) && + validCoinPathPartsSet.has(pathElems[1]) && hard(pathElems[2]) && hard(pathElems[3]) && change(pathElems[4]) && diff --git a/libs/ledgerjs/packages/hw-app-btc/tests/newops/isPathNormal.test.ts b/libs/ledgerjs/packages/hw-app-btc/tests/newops/isPathNormal.test.ts new file mode 100644 index 000000000000..5987f2d9cfab --- /dev/null +++ b/libs/ledgerjs/packages/hw-app-btc/tests/newops/isPathNormal.test.ts @@ -0,0 +1,30 @@ +import { isPathNormal } from "../../src/BtcNew"; + +describe("isPathNormal", () => { + describe("should return true for normal and supported paths", () => { + test.each([ + "44'/0'/0'", + "44'/0'/0'/0/0", + "84'/0'/0'", + "84'/0'/0'/0/0", + "86'/0'/0'", + "86'/0'/0'/0/0", + "49'/0'/0'", + "49'/0'/0'/0/0", + "48'/1'/99'/7'", + "86'/1'/99'/0", + "48'/0'/99'/7'/1/17", + ])("%s", path => { + expect(isPathNormal(path)).toEqual(true); + }); + }); + + describe("should return false for non-normal or non supported paths", () => { + test.each(["48'/0'/99'", "48'/0'/99'/7'/1/17/2", "199'/0'/1'/0/88", "86'/1'/99'/2"])( + "%s", + path => { + expect(isPathNormal(path)).toEqual(false); + }, + ); + }); +}); diff --git a/libs/ledgerjs/packages/hw-app-btc/tests/semver.test.ts b/libs/ledgerjs/packages/hw-app-btc/tests/semver.test.ts new file mode 100644 index 000000000000..b8357bde55b3 --- /dev/null +++ b/libs/ledgerjs/packages/hw-app-btc/tests/semver.test.ts @@ -0,0 +1,37 @@ +import semver from "semver"; + +/** + * To detect any semver regression that could impact the behavior of the package + * implementation when working with versions having prefix. + * One might say this is not necessary since it should already be covered by the + * semver package own testsuite, but better be safe than sorry. + * + * Following this comment: + * Are you 100% sure this does the same as the current implem? + * Just being paranoid, because app versions can be different from a clean + * major.minor.patch + * e.g they can have -rc0 -next0 or such additional stuff at the end, + * and it happened in the past that semver doesn't behave as we think it does. + * cf. https://github.com/LedgerHQ/ledger-live/pull/5675/files#r1417174514 + */ + +describe("semver", () => { + describe("should be able to properly compare version with prefix", () => { + it("2.0.0-rc0 should be lower than 2.1.0", () => { + const version = "2.0.0-rc1"; + expect(semver.lt(version, "2.1.0")); + }); + it("2.0.0-next0 should be lower than 2.1.0", () => { + const version = "2.0.0-next0"; + expect(semver.lt(version, "2.1.0")); + }); + it("2.1.0-rc0 should be greater than 2.1.0", () => { + const version = "2.1.0-rc0"; + expect(semver.gt(version, "2.1.0")); + }); + it("2.1.0-next0 should be greater than 2.1.0", () => { + const version = "2.1.0-next0"; + expect(semver.gt(version, "2.1.0")); + }); + }); +});