fix(ledger): iterate account indexes in ledgerLiveMode

.changeset/ledger-live-account-indexes.md

@@ -0,0 +1,7 @@
+---
+'@celo/wallet-ledger': patch
+'@celo/viem-account-ledger': patch
+'@celo/celocli': patch
+---
+
+Fix `--ledgerLiveMode` so it iterates BIP-44 hardened account indexes (`m/44'/60'/N'/0/0`) instead of the change index. `LedgerWallet`, `newLedgerWalletWithSetup`, `deriveLedgerAccounts`, and `ledgerToWalletClient` now accept `accountIndexes`.

packages/cli/src/base.test.ts

@@ -195,7 +195,7 @@ testWithAnvilL2('BaseCommand', (provider) => {
     })
 
     describe('with --ledgerLiveMode', () => {
-      it('--ledgerAddresses passes changeIndexes to LedgerWallet', async () => {
+      it('--ledgerAddresses passes accountIndexes to LedgerWallet', async () => {
         await testLocallyWithNode(
           BasicCommand,
           ['--useLedger', '--ledgerLiveMode', '--ledgerAddresses', '5'],
@@ -205,14 +205,16 @@ testWithAnvilL2('BaseCommand', (provider) => {
         expect(WalletLedgerExports.newLedgerWalletWithSetup).toHaveBeenCalledWith(
           expect.anything(),
           expect.objectContaining({
-            changeIndexes: [0, 1, 2, 3, 4],
+            accountIndexes: [0, 1, 2, 3, 4],
+            changeIndexes: [0],
             derivationPathIndexes: [0],
           })
         )
 
         expect(ViemAccountLedgerExports.ledgerToWalletClient).toHaveBeenCalledWith(
           expect.objectContaining({
-            changeIndexes: [0, 1, 2, 3, 4],
+            accountIndexes: [0, 1, 2, 3, 4],
+            changeIndexes: [0],
             derivationPathIndexes: [0],
           })
         )
@@ -244,7 +246,7 @@ testWithAnvilL2('BaseCommand', (provider) => {
         `)
       })
       describe('with --ledgerCustomAddresses', () => {
-        it('passes custom changeIndexes to LedgerWallet', async () => {
+        it('passes custom accountIndexes to LedgerWallet', async () => {
           await testLocallyWithNode(
             BasicCommand,
             ['--useLedger', '--ledgerLiveMode', '--ledgerCustomAddresses', '[1,8,9]'],
@@ -254,14 +256,16 @@ testWithAnvilL2('BaseCommand', (provider) => {
           expect(WalletLedgerExports.newLedgerWalletWithSetup).toHaveBeenCalledWith(
             expect.anything(),
             expect.objectContaining({
-              changeIndexes: [1, 8, 9],
+              accountIndexes: [1, 8, 9],
+              changeIndexes: [0],
               derivationPathIndexes: [0],
             })
           )
 
           expect(ViemAccountLedgerExports.ledgerToWalletClient).toHaveBeenCalledWith(
             expect.objectContaining({
-              changeIndexes: [1, 8, 9],
+              accountIndexes: [1, 8, 9],
+              changeIndexes: [0],
               derivationPathIndexes: [0],
             })
           )

packages/cli/src/base.ts

@@ -1,8 +1,8 @@
 import { type PublicCeloClient, type WalletCeloClient } from '@celo/actions'
 import {
   CELO_DERIVATION_PATH_BASE,
-  ensureLeading0x,
   ETHEREUM_DERIVATION_PATH,
+  ensureLeading0x,
   StrongAddress,
 } from '@celo/base'
 import { type Provider, ReadOnlyWallet } from '@celo/connect'
@@ -95,7 +95,7 @@ export abstract class BaseCommand extends Command {
       dependsOn: ['useLedger'],
       default: false,
       description:
-        'When set, the 4th postion of the derivation path will be iterated over instead of the 5th. This is useful to use same address on you Ledger with celocli as you do on Ledger Live',
+        "When set, the BIP-44 account index is iterated instead of the address index. This matches Ledger Live account paths (m/44'/60'/N'/0/0)",
     }),
     ledgerCustomAddresses: Flags.string({
       dependsOn: ['useLedger'],
@@ -330,7 +330,8 @@ export abstract class BaseCommand extends Command {
       transport: await this.openLedgerTransport(),
       baseDerivationPath: getDefaultDerivationPath(this.config.configDir),
       derivationPathIndexes: isLedgerLiveMode ? [0] : indicesToIterateOver,
-      changeIndexes: isLedgerLiveMode ? indicesToIterateOver : [0],
+      changeIndexes: [0],
+      accountIndexes: isLedgerLiveMode ? indicesToIterateOver : undefined,
       ledgerAddressValidation: ledgerConfirmation,
     }
     return ledgerOptions
@@ -389,7 +390,8 @@ export abstract class BaseCommand extends Command {
         this._wallet = await newLedgerWalletWithSetup(await this.openLedgerTransport(), {
           baseDerivationPath: baseDerivationPath,
           derivationPathIndexes: isLedgerLiveMode ? [0] : indicesToIterateOver,
-          changeIndexes: isLedgerLiveMode ? indicesToIterateOver : [0],
+          changeIndexes: [0],
+          accountIndexes: isLedgerLiveMode ? indicesToIterateOver : undefined,
           ledgerAddressValidation: ledgerConfirmation,
         })
       } catch (err) {

packages/sdk/wallets/wallet-ledger/src/ledger-wallet.test.ts

@@ -6,9 +6,9 @@ import { recoverTransaction, verifyEIP712TypedDataSigner } from '@celo/wallet-ba
 import TransportNodeHid from '@ledgerhq/hw-transport-node-hid'
 import { AddressValidation, CELO_BASE_DERIVATION_PATH, LedgerWallet } from './ledger-wallet'
 import {
+  ACCOUNT_ADDRESS_NEVER,
   ACCOUNT_ADDRESS1,
   ACCOUNT_ADDRESS2,
-  ACCOUNT_ADDRESS_NEVER,
   mockLedgerImplementation,
 } from './test-utils'
 import { tokenInfoByAddressAndChainId } from './tokens'
@@ -221,6 +221,34 @@ describe('LedgerWallet class', () => {
       `)
       expect(wallet.ledger!.getAddress).toHaveBeenCalledTimes(6)
     })
+    it('iterates hardened account indexes for Ledger Live paths', async () => {
+      // Ledger Live accounts are m/44'/60'/N'/0/0. Index ≥1 is required to distinguish
+      // that from iterating the change component (m/44'/60'/0'/N/0).
+      wallet = new LedgerWallet(
+        {},
+        [0],
+        "m/44'/60'/0'",
+        [0],
+        AddressValidation.firstTransactionPerAddress,
+        [0, 1, 2]
+      )
+      mockLedger(wallet, mockForceValidation)
+      await wallet.init()
+      // @ts-expect-error (mock.calls)
+      expect(wallet.ledger!.getAddress.mock.calls).toEqual([
+        ["44'/60'/0'/0/0", false],
+        ["44'/60'/1'/0/0", false],
+        ["44'/60'/2'/0/0", false],
+      ])
+      expect(wallet.ledger!.getAddress).toHaveBeenCalledTimes(3)
+    })
+    it('does not iterate the account component when accountIndexes is omitted', async () => {
+      wallet = new LedgerWallet({}, [0], "m/44'/60'/5'", [0])
+      mockLedger(wallet, mockForceValidation)
+      await wallet.init()
+      // @ts-expect-error (mock.calls)
+      expect(wallet.ledger!.getAddress.mock.calls).toEqual([["44'/60'/5'/0/0", false]])
+    })
     describe('with other ledger apps', () => {
       describe('with the ethereum-recovery app', () => {
         beforeEach(() => {

packages/sdk/wallets/wallet-ledger/src/ledger-wallet.ts

@@ -31,6 +31,7 @@ export enum AddressValidation {
 interface LedgerWalletSetup {
   derivationPathIndexes?: number[]
   changeIndexes?: number[]
+  accountIndexes?: number[]
   baseDerivationPath?: string
   ledgerAddressValidation?: AddressValidation
 }
@@ -42,14 +43,16 @@ export async function newLedgerWalletWithSetup(
     baseDerivationPath,
     ledgerAddressValidation,
     changeIndexes,
+    accountIndexes,
   }: LedgerWalletSetup
 ): Promise<LedgerWallet> {
   const wallet = new LedgerWallet(
     transport,
     derivationPathIndexes,
     baseDerivationPath,
     changeIndexes,
-    ledgerAddressValidation
+    ledgerAddressValidation,
+    accountIndexes
   )
   await wallet.init()
   return wallet
@@ -74,18 +77,25 @@ export class LedgerWallet extends RemoteWallet<LedgerSigner> implements ReadOnly
    * Default: [0].
    * Example: [0, 1] will retrieve the derivation paths of [`${baseDerivationPath}/0/${address_index}`, `${baseDerivationPath}/1/${address_index}`, `${baseDerivationPath}/2/${address_index}`]
    * @param ledgerAddressValidation AddressValidation enum to validate addresses. Default: AddressValidation.firstTransactionPerAddress
+   * @param accountIndexes number array of BIP-44 hardened "account" indexes.
+   * Default: the account component from `baseDerivationPath`.
+   * Example: [0, 1, 2] with change/address 0 yields [`44'/60'/0'/0/0`, `44'/60'/1'/0/0`, `44'/60'/2'/0/0`]
    */
   constructor(
     readonly transport: any = {},
     readonly derivationPathIndexes: number[] = zeroRange(ADDRESS_QTY),
     readonly baseDerivationPath: string = CELO_BASE_DERIVATION_PATH,
     readonly changeIndexes: number[] = [0],
-    readonly ledgerAddressValidation: AddressValidation = AddressValidation.firstTransactionPerAddress
+    readonly ledgerAddressValidation: AddressValidation = AddressValidation.firstTransactionPerAddress,
+    readonly accountIndexes?: number[]
   ) {
     super()
 
     validateIndexes(derivationPathIndexes, 'address index')
     validateIndexes(changeIndexes, 'change index')
+    if (accountIndexes) {
+      validateIndexes(accountIndexes, 'account index')
+    }
     // Remove the 'm/' prefix if it exists since we dont expect it here but that is how derivaiton path is used in the rest of the code
     this.baseDerivationPath = baseDerivationPath.startsWith('m/')
       ? baseDerivationPath.slice(2)
@@ -174,22 +184,27 @@ export class LedgerWallet extends RemoteWallet<LedgerSigner> implements ReadOnly
     const appConfiguration = await this.retrieveAppConfiguration()
     const validationRequired = this.ledgerAddressValidation === AddressValidation.initializationOnly
     // https://trezor.io/learn/a/what-is-bip44
-    const [purpose, coinType, account] = this.baseDerivationPath.split('/')
+    const [purpose, coinType, accountFromPath] = this.baseDerivationPath.split('/')
+    const accounts = this.accountIndexes
+      ? this.accountIndexes.map((index) => `${index}'`)
+      : [accountFromPath]
     // Each address must be retrieved synchronously, (ledger lock)
-    for (const changeIndex of this.changeIndexes) {
-      for (const addressIndex of this.derivationPathIndexes) {
-        const derivationPath = `${purpose}/${coinType}/${account}/${changeIndex}/${addressIndex}`
-        debug(`Fetching address for derivation path ${derivationPath}`)
-        const addressInfo = await this.ledger!.getAddress(derivationPath, validationRequired)
-        addressToSigner.set(
-          addressInfo.address!,
-          new LedgerSigner(
-            this.ledger!,
-            derivationPath,
-            this.ledgerAddressValidation,
-            appConfiguration
+    for (const account of accounts) {
+      for (const changeIndex of this.changeIndexes) {
+        for (const addressIndex of this.derivationPathIndexes) {
+          const derivationPath = `${purpose}/${coinType}/${account}/${changeIndex}/${addressIndex}`
+          debug(`Fetching address for derivation path ${derivationPath}`)
+          const addressInfo = await this.ledger!.getAddress(derivationPath, validationRequired)
+          addressToSigner.set(
+            addressInfo.address!,
+            new LedgerSigner(
+              this.ledger!,
+              derivationPath,
+              this.ledgerAddressValidation,
+              appConfiguration
+            )
           )
-        )
+        }
       }
     }
     return addressToSigner

packages/viem-account-ledger/src/derive-ledger-accounts.test.ts

@@ -1,11 +1,10 @@
+import { beforeEach, describe, expect, it, vi } from 'vitest'
 import { deriveLedgerAccounts } from './derive-ledger-accounts'
 import * as LedgerAccount from './ledger-to-account'
+import { mockLedger } from './test-utils'
 import { AddressValidation } from './types'
 import { generateLedger } from './utils'
 
-import { beforeEach, describe, expect, it, vi } from 'vitest'
-import { mockLedger } from './test-utils'
-
 vi.mock('./utils', () => ({
   generateLedger: vi.fn().mockImplementation(() => {
     return mockLedger()
@@ -85,4 +84,61 @@ describe('deriveLedgerAccounts', () => {
       })
     ).rejects.toThrow(/Invalid change index provided/)
   })
+
+  it('iterates hardened account indexes for Ledger Live paths', async () => {
+    // Index ≥1 distinguishes m/44'/60'/N'/0/0 from iterating change (m/44'/60'/0'/N/0).
+    ledgerToAccount.mockResolvedValue({
+      address: '0x1be31a94361a391bbafb2a4ccd704f57dc04d4bb',
+    })
+    await deriveLedgerAccounts({
+      transport: {} as any,
+      derivationPathIndexes: [0],
+      changeIndexes: [0],
+      accountIndexes: [0, 1, 2],
+      baseDerivationPath: "m/44'/60'/0'",
+    })
+    expect(ledgerToAccount).toHaveBeenCalledTimes(3)
+    expect(
+      ledgerToAccount.mock.calls.map(([{ baseDerivationPath, derivationPathIndex }]) => {
+        return `${baseDerivationPath}/${derivationPathIndex}`
+      })
+    ).toEqual(["44'/60'/0'/0/0", "44'/60'/1'/0/0", "44'/60'/2'/0/0"])
+  })
+
+  it('does not iterate the account component when accountIndexes is omitted', async () => {
+    ledgerToAccount.mockResolvedValue({
+      address: '0x1be31a94361a391bbafb2a4ccd704f57dc04d4bb',
+    })
+    await deriveLedgerAccounts({
+      transport: {} as any,
+      derivationPathIndexes: [0],
+      changeIndexes: [0],
+      baseDerivationPath: "m/44'/60'/5'",
+    })
+    expect(ledgerToAccount).toHaveBeenCalledTimes(1)
+    expect(ledgerToAccount).toHaveBeenCalledWith(
+      expect.objectContaining({
+        derivationPathIndex: 0,
+        baseDerivationPath: "44'/60'/5'/0",
+      })
+    )
+  })
+
+  it('throws if accountIndexes is empty', async () => {
+    await expect(
+      deriveLedgerAccounts({
+        transport: {} as any,
+        accountIndexes: [],
+      })
+    ).rejects.toThrow(/No account index provided/)
+  })
+
+  it('throws if accountIndexes contains invalid value', async () => {
+    await expect(
+      deriveLedgerAccounts({
+        transport: {} as any,
+        accountIndexes: [-1],
+      })
+    ).rejects.toThrow(/Invalid account index provided/)
+  })
 })

packages/viem-account-ledger/src/derive-ledger-accounts.ts

@@ -9,36 +9,46 @@ export async function deriveLedgerAccounts({
   transport,
   derivationPathIndexes = zeroRange(ADDRESS_QTY),
   changeIndexes = [0],
+  accountIndexes,
   baseDerivationPath = DEFAULT_DERIVATION_PATH,
   ledgerAddressValidation,
 }: {
   transport: TransportNodeHid
   derivationPathIndexes?: number[]
   changeIndexes?: number[]
+  accountIndexes?: number[]
   baseDerivationPath?: string
   ledgerAddressValidation?: AddressValidation
 }) {
   const ledger = await generateLedger(transport)
   const accounts: LedgerAccount[] = []
   validateIndexes(derivationPathIndexes, 'address index')
   validateIndexes(changeIndexes, 'change index')
+  if (accountIndexes) {
+    validateIndexes(accountIndexes, 'account index')
+  }
 
   const _baseDerivationPath = baseDerivationPath.startsWith('m/')
     ? baseDerivationPath.slice(2)
     : baseDerivationPath
 
   // https://trezor.io/learn/a/what-is-bip44
-  const [purpose, coinType, accountIndex] = _baseDerivationPath.split('/')
-  for (const changeIndex of changeIndexes) {
-    for (const addressIndex of derivationPathIndexes) {
-      accounts.push(
-        await ledgerToAccount({
-          ledger,
-          derivationPathIndex: addressIndex,
-          baseDerivationPath: `${purpose}/${coinType}/${accountIndex}/${changeIndex}`,
-          ledgerAddressValidation,
-        })
-      )
+  const [purpose, coinType, accountFromPath] = _baseDerivationPath.split('/')
+  const accountsToIterate = accountIndexes
+    ? accountIndexes.map((index) => `${index}'`)
+    : [accountFromPath]
+  for (const account of accountsToIterate) {
+    for (const changeIndex of changeIndexes) {
+      for (const addressIndex of derivationPathIndexes) {
+        accounts.push(
+          await ledgerToAccount({
+            ledger,
+            derivationPathIndex: addressIndex,
+            baseDerivationPath: `${purpose}/${coinType}/${account}/${changeIndex}`,
+            ledgerAddressValidation,
+          })
+        )
+      }
     }
   }
   return accounts