better handling of derivationPath flag

celo-org · Dec 6, 2024 · 7b28cd2 · 7b28cd2
1 parent c3e9cd4
commit 7b28cd2
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 21 deletions.
diff --git a/.changeset/nice-fireants-cheat.md b/.changeset/nice-fireants-cheat.md
@@ -0,0 +1,5 @@
+---
+'@celo/celocli': patch
+---
+
+when account:new is called display the full bip44 derivation path in the output. before it would miss the last 2 positions.
diff --git a/docs/command-line-interface/account.md b/docs/command-line-interface/account.md
@@ -986,10 +986,7 @@ FLAGS
       Choose the change index for the derivation path
 
   --derivationPath=<value>
-      Choose a different derivation Path (Celo's default is "m/44'/52752'/0'"). Use "eth"
-      as an alias of the Ethereum derivation path ("m/44'/60'/0'"). Recreating the same
-      account requires knowledge of the mnemonic, passphrase (if any), and the derivation
-      path
+      Derivation path in the format "m/44'/coin_type'/account'" or an alias
 
   --globalHelp
       View all available global flags
@@ -1047,6 +1044,16 @@ FLAG DESCRIPTIONS
     local, localhost => 'http://localhost:8545'
     alfajores => Celo Alfajores Testnet,
     mainnet, celo, forno => Celo Mainnet chain',
+
+
+  --derivationPath=<value>
+
+    Derivation path in the format "m/44'/coin_type'/account'" or an alias
+
+    Choose a different derivation Path (Celo's default is "m/44'/52752'/0'"). Use "eth"
+    as an alias of the Ethereum derivation path ("m/44'/60'/0'"). Recreating the same
+    account requires knowledge of the mnemonic, passphrase (if any), and the derivation
+    path. (use changeIndex, and addressIndex flags to change BIP44 positions 4 and 5)
 ```
 
 _See code: [src/commands/account/new.ts](https://github.com/celo-org/developer-tooling/tree/master/packages/cli/src/commands/account/new.ts)_

diff --git a/packages/cli/src/commands/account/new.test.ts b/packages/cli/src/commands/account/new.test.ts
@@ -39,56 +39,58 @@ testWithAnvilL2('account:new cmd', (web3: Web3) => {
 
     expect(deRandomize(consoleMock.mock.lastCall?.[0])).toMatchInlineSnapshot(`
       "mnemonic: *** *** 
-      derivationPath: m/44'/52752'/0'
+      derivationPath: m/44'/52752'/0'/0/0
       accountAddress: ADDRESS
       privateKey: PUBLIC_KEY
       publicKey: PRIVATE_KEY
       address: ADDRESS"
     `)
   })
-  it("when called with --derivationPath eth flag generates mnemonic using m/44'/60'/0'", async () => {
+
+  it("when called with --derivationPath eth it generates mnemonic using m/44'/60'/0'", async () => {
     await testLocallyWithWeb3Node(NewAccount, ['--derivationPath', 'eth'], web3)
 
     expect(deRandomize(consoleMock.mock.lastCall?.[0])).toMatchInlineSnapshot(`
       "mnemonic: *** *** 
-      derivationPath: m/44'/60'/0'
+      derivationPath: m/44'/60'/0'/0/0
       accountAddress: ADDRESS
       privateKey: PUBLIC_KEY
       publicKey: PRIVATE_KEY
       address: ADDRESS"
     `)
   })
 
-  it(`when called with "--derivationPath celoLegacy" it generates with "m/44'/52752'/0'"`, async () => {
+  it(`when called with --derivationPath celoLegacy it generates with "m/44'/52752'/0'"`, async () => {
     await testLocallyWithWeb3Node(NewAccount, ['--derivationPath', 'celoLegacy'], web3)
 
     expect(deRandomize(consoleMock.mock.lastCall?.[0])).toMatchInlineSnapshot(`
       "mnemonic: *** *** 
-      derivationPath: m/44'/52752'/0'
+      derivationPath: m/44'/52752'/0'/0/0
       accountAddress: ADDRESS
       privateKey: PUBLIC_KEY
       publicKey: PRIVATE_KEY
       address: ADDRESS"
     `)
   })
-  describe('--derivationPath with bad data', () => {
-    it(`called with invalid alias "notARealPath" then throws"`, async () => {
+
+  describe('bad data --derivationPath', () => {
+    it(`with invalid alias "notARealPath"  throws"`, async () => {
       await expect(testLocallyWithWeb3Node(NewAccount, ['--derivationPath', 'notARealPath'], web3))
         .rejects.toThrowErrorMatchingInlineSnapshot(`
         "Parsing --derivationPath 
         	Invalid derivationPath: notARealPath. should be in format  "m / 44' / coin_type' / account'"
         See more help with --help"
       `)
     })
-    it(`called with invalid bip44 then throws"`, async () => {
+    it(`with invalid bip44 throws"`, async () => {
       await expect(testLocallyWithWeb3Node(NewAccount, ['--derivationPath', 'm/44/1/1/2/10'], web3))
         .rejects.toThrowErrorMatchingInlineSnapshot(`
         "Parsing --derivationPath 
         	Invalid derivationPath: m/44/1/1/2/10. should be in format  "m / 44' / coin_type' / account'"
         See more help with --help"
       `)
     })
-    it('call with bip44 with changeIndex 4', async () => {
+    it('with bip44 with changeIndex 4 throws', async () => {
       await expect(
         testLocallyWithWeb3Node(NewAccount, ['--derivationPath', "m/44'/52752'/0/0'"], web3)
       ).rejects.toThrowErrorMatchingInlineSnapshot(`
@@ -97,7 +99,7 @@ testWithAnvilL2('account:new cmd', (web3: Web3) => {
         See more help with --help"
       `)
     })
-    it('call with bip44 with changeIndex 4 and addressIndex 5', async () => {
+    it('with bip44 including changeIndex 4 and addressIndex 5 throws', async () => {
       await expect(
         testLocallyWithWeb3Node(NewAccount, ['--derivationPath', "m/44'/52752'/0/0/0'"], web3)
       ).rejects.toThrowErrorMatchingInlineSnapshot(`
@@ -106,6 +108,18 @@ testWithAnvilL2('account:new cmd', (web3: Web3) => {
         See more help with --help"
       `)
     })
+    it(`with path ending in "/" removes the slash`, async () => {
+      await testLocallyWithWeb3Node(NewAccount, ['--derivationPath', "m/44'/60'/0'/"], web3)
+
+      expect(deRandomize(consoleMock.mock.lastCall?.[0])).toMatchInlineSnapshot(`
+        "mnemonic: *** *** 
+        derivationPath: m/44'/60'/0'/0/0
+        accountAddress: ADDRESS
+        privateKey: PUBLIC_KEY
+        publicKey: PRIVATE_KEY
+        address: ADDRESS"
+      `)
+    })
   })
 
   describe('when called with --mnemonicPath', () => {
@@ -126,7 +140,7 @@ testWithAnvilL2('account:new cmd', (web3: Web3) => {
 
       expect(stripAnsiCodesAndTxHashes(consoleMock.mock.lastCall?.[0])).toMatchInlineSnapshot(`
         "mnemonic: hamster label near volume denial spawn stable orbit trade only crawl learn forest fire test feel bubble found angle also olympic obscure fork venue
-        derivationPath: m/44'/52752'/0'
+        derivationPath: m/44'/52752'/0'/0/0
         accountAddress: 0x0a85BeCD036C86faD4Db5519634904be2021fb7d
         privateKey: 6346d0cd7cfdb7904f08df48e442169d3333643de0351682f8b79cf714395471
         publicKey: 02269b3efc9c4c6b81037d06e73e936078e625fb0f12b9ea1e0fd14d2cd45775f2
@@ -143,7 +157,7 @@ testWithAnvilL2('account:new cmd', (web3: Web3) => {
 
       expect(stripAnsiCodesAndTxHashes(consoleMock.mock.lastCall?.[0])).toMatchInlineSnapshot(`
         "mnemonic: hamster label near volume denial spawn stable orbit trade only crawl learn forest fire test feel bubble found angle also olympic obscure fork venue
-        derivationPath: m/44'/60'/0'
+        derivationPath: m/44'/60'/0'/0/0
         accountAddress: 0x35A4d54B541fc7b2047fb5357cC706191E105cd3
         privateKey: e4816cbb93346760921264ea38a7fc54903f4dd688ae0923fefd89a43c5f58cc
         publicKey: 034b3036d657a6dc2f322db52cca29ae72101a9cf56de4765d17b0507ea1e87b7c
@@ -159,7 +173,7 @@ testWithAnvilL2('account:new cmd', (web3: Web3) => {
 
       expect(stripAnsiCodesAndTxHashes(consoleMock.mock.lastCall?.[0])).toMatchInlineSnapshot(`
         "mnemonic: hamster label near volume denial spawn stable orbit trade only crawl learn forest fire test feel bubble found angle also olympic obscure fork venue
-        derivationPath: m/44'/60'/0'
+        derivationPath: m/44'/60'/0'/2/0
         accountAddress: 0xb3492799c55141e0B3507302F241f1c34c08E1e2
         privateKey: 3abc861ef3e9e31a6a7dc23e5903e41b3fe4a381d4fbb8f9db14e6730abd1c43
         publicKey: 0280df4d09cf8ebcad418327287be3f4ba0054112544ffa290ab3cc0a87949b32a
@@ -175,7 +189,7 @@ testWithAnvilL2('account:new cmd', (web3: Web3) => {
 
       expect(stripAnsiCodesAndTxHashes(consoleMock.mock.lastCall?.[0])).toMatchInlineSnapshot(`
         "mnemonic: hamster label near volume denial spawn stable orbit trade only crawl learn forest fire test feel bubble found angle also olympic obscure fork venue
-        derivationPath: m/44'/60'/0'
+        derivationPath: m/44'/60'/0'/0/3
         accountAddress: 0x336E523118B6091e033F9715257e2E793002964c
         privateKey: aa324b2efd0ebe6387c3bcf03387c78047d82a1d2e8b4e132e9e1b9fb93529d0
         publicKey: 0394cc7cc524079c545aef2067c9ea7e69decb4815004afecf215ea1e6f370ce6c

diff --git a/packages/cli/src/commands/account/new.ts b/packages/cli/src/commands/account/new.ts
@@ -63,7 +63,7 @@ export default class NewAccount extends BaseCommand {
       parse: async (input: string) => {
         return NewAccount.sanitizeDerivationPath(input)
       },
-      summary: 'Derivation path in the format "m/44\'/coin_type\'/account\'" or an alias',
+      summary: "Derivation path in the format \"m/44'/coin_type'/account'\" or an alias",
       description:
         "Choose a different derivation Path (Celo's default is \"m/44'/52752'/0'\"). Use \"eth\" as an alias of the Ethereum derivation path (\"m/44'/60'/0'\"). Recreating the same account requires knowledge of the mnemonic, passphrase (if any), and the derivation path. (use changeIndex, and addressIndex flags to change BIP44 positions 4 and 5)",
     }),
@@ -109,7 +109,9 @@ export default class NewAccount extends BaseCommand {
       return derivationPath
     }
 
-    throw new Error(`Invalid derivationPath: ${derivationPath}. should be in format  "m / 44' / coin_type' / account'"`)
+    throw new Error(
+      `Invalid derivationPath: ${derivationPath}. should be in format  "m / 44' / coin_type' / account'"`
+    )
   }
 
   static readFile(file?: string): string | undefined {
@@ -163,8 +165,9 @@ export default class NewAccount extends BaseCommand {
         )
       )
     }
+    const fullDerivationPath = `${derivationPath}/${res.flags.changeIndex}/${res.flags.addressIndex}`
 
-    printValueMap({ mnemonic, derivationPath, accountAddress, ...keys })
+    printValueMap({ mnemonic, derivationPath: fullDerivationPath, accountAddress, ...keys })
 
     this.log(
       chalk.green.bold(