refactor path serialization to make it more reusable (#21)

Author: jleni

LICENSE

@@ -186,7 +186,7 @@
       same "printed page" as the copyright notice for easier
       identification within third-party archives.
 
-   Copyright 2022 Zondax AG
+   Copyright 2022-2024 Zondax AG
 
    Licensed under the Apache License, Version 2.0 (the "License");
    you may not use this file except in compliance with the License.

src/app.test.ts

@@ -25,34 +25,9 @@ describe('BaseApp', () => {
     ins: { GET_VERSION: 0x00 as 0 },
     p1Values: { ONLY_RETRIEVE: 0x00 as 0, SHOW_ADDRESS_IN_DEVICE: 0x01 as 1 },
     chunkSize: 255,
-    acceptedPathLengths: [3, 5],
+    requiredPathLengths: [3, 5],
   }
 
-  describe('serializePath', () => {
-    it('should throw an error if path does not start with "m/"', () => {
-      const transport = new MockTransport(Buffer.alloc(0))
-      const app = new BaseApp(transport, params)
-      expect(() => app.serializePath("44'/0'/0'")).toThrow('Path should start with "m/"')
-    })
-
-    it('should throw an error if path length is not accepted', () => {
-      const transport = new MockTransport(Buffer.alloc(0))
-      const app = new BaseApp(transport, params)
-      expect(() => app.serializePath("m/44'/0'")).toThrow('Invalid path.')
-    })
-
-    it('should correctly serialize a valid path', () => {
-      const transport = new MockTransport(Buffer.alloc(0))
-      const app = new BaseApp(transport, params)
-      const path = "m/44'/0'/0'"
-      const buffer = app.serializePath(path)
-      expect(buffer.length).toBe(12)
-      expect(buffer.readUInt32LE(0)).toBe(0x8000002c)
-      expect(buffer.readUInt32LE(4)).toBe(0x80000000)
-      expect(buffer.readUInt32LE(8)).toBe(0x80000000)
-    })
-  })
-
   describe('prepareChunks', () => {
     it('should prepare chunks correctly', () => {
       const transport = new MockTransport(Buffer.alloc(0))

src/app.ts

@@ -15,8 +15,9 @@
  *****************************************************************************/
 import type Transport from '@ledgerhq/hw-transport'
 
+import { serializePath } from './bip32'
 import { processErrorResponse, processResponse } from './common'
-import { HARDENED, LEDGER_DASHBOARD_CLA, LedgerError, PAYLOAD_TYPE } from './consts'
+import { LEDGER_DASHBOARD_CLA, LedgerError, PAYLOAD_TYPE } from './consts'
 import { ResponsePayload } from './payload'
 import { ResponseError } from './responseError'
 import {
@@ -33,7 +34,7 @@ export default class BaseApp {
   readonly CLA: number
   readonly INS: INSGeneric
   readonly P1_VALUES: P1_VALUESGeneric
-  readonly ACCEPTED_PATH_LENGTHS?: number[]
+  readonly REQUIRED_PATH_LENGTHS?: number[]
   readonly CHUNK_SIZE: number
 
   constructor(transport: Transport, params: ConstructorParams) {
@@ -42,56 +43,7 @@ export default class BaseApp {
     this.INS = params.ins
     this.P1_VALUES = params.p1Values
     this.CHUNK_SIZE = params.chunkSize
-    this.ACCEPTED_PATH_LENGTHS = params.acceptedPathLengths
-  }
-
-  /**
-   * Serializes a derivation path into a buffer.
-   * @param path - The derivation path in string format.
-   * @returns A buffer representing the serialized path.
-   * @throws {Error} If the path format is incorrect or invalid.
-   */
-  serializePath(path: string): Buffer {
-    if (typeof path !== 'string') {
-      throw new Error("Path should be a string (e.g \"m/44'/461'/5'/0/3\")")
-    }
-
-    if (!path.startsWith('m/')) {
-      throw new Error('Path should start with "m/" (e.g "m/44\'/5757\'/5\'/0/3")')
-    }
-
-    const pathArray = path.split('/')
-    pathArray.shift() // remove "m"
-
-    if (this.ACCEPTED_PATH_LENGTHS && !this.ACCEPTED_PATH_LENGTHS.includes(pathArray.length)) {
-      throw new Error("Invalid path. (e.g \"m/44'/5757'/5'/0/3\")")
-    }
-
-    const buf = Buffer.alloc(4 * pathArray.length)
-
-    pathArray.forEach((child: string, i: number) => {
-      let value = 0
-
-      if (child.endsWith("'")) {
-        value += HARDENED
-        child = child.slice(0, -1)
-      }
-
-      const numChild = Number(child)
-
-      if (Number.isNaN(numChild)) {
-        throw new Error(`Invalid path : ${child} is not a number. (e.g "m/44'/461'/5'/0/3")`)
-      }
-
-      if (numChild >= HARDENED) {
-        throw new Error('Incorrect child value (bigger or equal to 0x80000000)')
-      }
-
-      value += numChild
-      buf.writeUInt32LE(value, 4 * i)
-    })
-
-    return buf
+    this.REQUIRED_PATH_LENGTHS = params.acceptedPathLengths
   }
 
   /**
@@ -102,7 +54,7 @@ export default class BaseApp {
    */
   prepareChunks(path: string, message: Buffer): Buffer[] {
     const chunks = []
-    const serializedPathBuffer = this.serializePath(path)
+    const serializedPathBuffer = serializePath(path, this.REQUIRED_PATH_LENGTHS)
 
     // First chunk (only path)
     chunks.push(serializedPathBuffer)

src/bip32.test.ts

@@ -0,0 +1,63 @@
+import { serializePath } from './bip32'
+
+test('serializePath - valid path', async () => {
+  const path = "m/44'/461'/0/0/5"
+  const buf = Buffer.alloc(20)
+  buf.writeUInt32LE(0x80000000 + 44, 0)
+  buf.writeUInt32LE(0x80000000 + 461, 4)
+  buf.writeUInt32LE(0, 8)
+  buf.writeUInt32LE(0, 12)
+  buf.writeUInt32LE(5, 16)
+
+  const bufPath = serializePath(path)
+
+  expect(bufPath).toEqual(buf)
+})
+
+test('serializePath - path should be a string', async () => {
+  const path = [44, 461, 0, 2, 3]
+
+  expect(() => {
+    serializePath(path as unknown as string)
+  }).toThrow(/Path should be a string/)
+})
+
+test("serializePath - path should start with 'm'", async () => {
+  const path = "/44'/461'/0/0/5"
+
+  expect(() => {
+    serializePath(path)
+  }).toThrow(/Path should start with "m\/"/)
+})
+
+test('serializePath - path length needs to be 5', async () => {
+  const path = "m/44'/461'/0/0"
+
+  expect(() => {
+    serializePath(path, [5])
+  }).toThrow(/Invalid path/)
+})
+
+test('serializePath - invalid number in path', async () => {
+  const path = "m/44'/461'/0/0/l"
+
+  expect(() => {
+    serializePath(path)
+  }).toThrow(/Invalid path : l is not a number/)
+})
+
+test('serializePath - child value should not be bigger than 0x80000000', async () => {
+  const path = "m/44'/461'/0/0/2147483648"
+
+  expect(() => {
+    serializePath(path)
+  }).toThrow('Incorrect child value (bigger or equal to 0x80000000)')
+})
+
+test('serializePath - child value should not be bigger than 0x80000000', async () => {
+  const path = "m/44'/461'/0/0/2147483649"
+
+  expect(() => {
+    serializePath(path)
+  }).toThrow('Incorrect child value (bigger or equal to 0x80000000)')
+})

src/bip32.ts

@@ -0,0 +1,51 @@
+import { HARDENED, LedgerError } from './consts'
+import { ResponseError } from './responseError'
+
+/**
+ * Serializes a derivation path into a buffer.
+ * @param path - The derivation path in string format.
+ * @returns A buffer representing the serialized path.
+ * @throws {Error} If the path format is incorrect or invalid.
+ */
+export function serializePath(path: string, requiredPathLengths?: number[]): Buffer {
+  if (typeof path !== 'string') {
+    // NOTE: this is probably unnecessary
+    throw new ResponseError(LedgerError.GenericError, "Path should be a string (e.g \"m/44'/461'/5'/0/3\")")
+  }
+
+  if (!path.startsWith('m/')) {
+    throw new ResponseError(LedgerError.GenericError, 'Path should start with "m/" (e.g "m/44\'/461\'/5\'/0/3")')
+  }
+
+  const pathArray = path.split('/')
+  pathArray.shift() // remove "m"
+
+  if (requiredPathLengths && requiredPathLengths.length > 0 && !requiredPathLengths.includes(pathArray.length)) {
+    throw new ResponseError(LedgerError.GenericError, "Invalid path length. (e.g \"m/44'/5757'/5'/0/3\")")
+  }
+
+  const buf = Buffer.alloc(4 * pathArray.length)
+  pathArray.forEach((child: string, i: number) => {
+    let value = 0
+
+    if (child.endsWith("'")) {
+      value += HARDENED
+      child = child.slice(0, -1)
+    }
+
+    const numChild = Number(child)
+
+    if (Number.isNaN(numChild)) {
+      throw new ResponseError(LedgerError.GenericError, `Invalid path : ${child} is not a number. (e.g "m/44'/461'/5'/0/3")`)
+    }
+
+    if (numChild >= HARDENED) {
+      throw new ResponseError(LedgerError.GenericError, 'Incorrect child value (bigger or equal to 0x80000000)')
+    }
+
+    value += numChild
+    buf.writeUInt32LE(value, 4 * i)
+  })
+
+  return buf
+}

src/consts.ts

@@ -87,6 +87,8 @@ export const enum LedgerError {
   FileNotFound = 0x9404,
   UserRefusedOnDevice = 0x5501,
   NotEnoughSpace = 0x5102,
+
+  GenericError = 0xffffffff,
 }
 
 export const ERROR_DESCRIPTION_OVERRIDE: Readonly<Record<LedgerError, string>> = {
@@ -144,4 +146,5 @@ export const ERROR_DESCRIPTION_OVERRIDE: Readonly<Record<LedgerError, string>> =
   [LedgerError.FileNotFound]: 'File Not Found',
   [LedgerError.UserRefusedOnDevice]: 'User Refused on Device',
   [LedgerError.NotEnoughSpace]: 'Not Enough Space',
+  [LedgerError.GenericError]: 'Generic Error',
 }