Add --keystore flag for signing with an encrypted keystore

.changeset/cli-keystore-flag.md

@@ -0,0 +1,24 @@
+---
+'@celo/celocli': minor
+'@celo/keystores': minor
+---
+
+Add a `--keystore` flag for signing with an encrypted keystore file, in the
+spirit of `cast --keystore`.
+
+`--keystore` takes a keystore file, or a directory of them in which case
+`--from` selects the account. The password is read from `--passwordFile` when
+given, and otherwise requested with a hidden prompt. `--keystore` is mutually
+exclusive with `--privateKey` and `--useLedger`.
+
+```bash
+celocli transfer:celo --keystore ~/keystore/UTC--2024-...--8233d802... --to 0x... --value 1
+celocli transfer:celo --keystore ~/keystore --from 0x8233d802... --to 0x... --value 1
+```
+
+`@celo/keystores` now exports its V3 keystore primitives (`decryptV3`,
+`encryptV3`, `v3Filename`) so that they can be used without going through
+`FileKeystore`.
+
+Note that `--keystore` does not yet work with the `bridge:*` commands, which
+construct their signer independently of `BaseCommand`.

packages/cli/package.json

@@ -48,6 +48,7 @@
     "@celo/cryptographic-utils": "^6.0.0",
     "@celo/explorer": "^5.1.1",
     "@celo/governance": "^5.1.11",
+    "@celo/keystores": "^5.0.16",
     "@celo/metadata-claims": "^1.0.4",
     "@celo/utils": "^8.0.3",
     "@celo/viem-account-ledger": "^1.2.3",

packages/cli/src/base.test.ts

@@ -1,16 +1,25 @@
+import { type PublicCeloClient } from '@celo/actions'
 import { Connection } from '@celo/connect'
 import { testWithAnvilL2 } from '@celo/dev-utils/anvil-test'
+import { encryptV3, v3Filename } from '@celo/keystores'
 import * as ViemAccountLedgerExports from '@celo/viem-account-ledger'
 import * as WalletLedgerExports from '@celo/wallet-ledger'
 import { Config, ux } from '@oclif/core'
+import { mkdtempSync, rmSync, writeFileSync } from 'fs'
 import http from 'http'
 import { tmpdir } from 'os'
+import { join } from 'path'
 import { MethodNotFoundRpcError } from 'viem'
 import { privateKeyToAddress } from 'viem/accounts'
+import { celoSepolia } from 'viem/chains'
 import { BaseCommand } from './base'
 import Set from './commands/config/set'
 import CustomHelp from './help'
-import { stripAnsiCodesFromNestedArray, testLocallyWithNode } from './test-utils/cliUtils'
+import {
+  stripAnsiCodesFromNestedArray,
+  testLocallyWithNode,
+  testWithoutChain,
+} from './test-utils/cliUtils'
 import { mockRpcFetch } from './test-utils/mockRpc'
 import { CustomFlags } from './utils/command'
 import * as config from './utils/config'
@@ -99,6 +108,136 @@ describe('flags', () => {
   })
 })
 
+describe('keystore flags', () => {
+  class TestKeystoreCommand extends BaseCommand {
+    static flags = {
+      ...BaseCommand.flags,
+      from: CustomFlags.address({ required: false }),
+    }
+    async run() {
+      // These cases are all rejected while parsing flags, so run() is never reached.
+      throw new Error('flag parsing should have rejected this invocation')
+    }
+  }
+
+  const PASSWORD = 'pw'
+  const keystorePrivateKey = '0x1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef'
+  const keystoreAddress = privateKeyToAddress(keystorePrivateKey)
+  let dir: string
+  let keystoreFile: string
+  let passwordFile: string
+
+  beforeAll(async () => {
+    dir = mkdtempSync(join(tmpdir(), 'celocli-base-keystore-'))
+    const keystore = await encryptV3(keystorePrivateKey, PASSWORD)
+    keystoreFile = join(dir, v3Filename(keystore.address, Date.parse('2024-01-01T00:00:00Z')))
+    writeFileSync(keystoreFile, JSON.stringify(keystore))
+    passwordFile = join(dir, 'password.txt')
+    writeFileSync(passwordFile, PASSWORD)
+  })
+
+  afterAll(() => {
+    rmSync(dir, { recursive: true, force: true })
+  })
+
+  it('rejects --keystore together with --privateKey', async () => {
+    await expect(
+      testWithoutChain(TestKeystoreCommand, [
+        '--keystore',
+        keystoreFile,
+        '--privateKey',
+        '0x1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef',
+      ])
+    ).rejects.toThrow(/cannot also be provided|--privateKey/)
+  })
+
+  it('rejects --keystore together with --useLedger', async () => {
+    await expect(
+      testWithoutChain(TestKeystoreCommand, ['--keystore', keystoreFile, '--useLedger'])
+    ).rejects.toThrow(/cannot also be provided|--useLedger/)
+  })
+
+  it('rejects --passwordFile without --keystore', async () => {
+    await expect(
+      testWithoutChain(TestKeystoreCommand, ['--passwordFile', keystoreFile])
+    ).rejects.toThrow(/keystore/)
+  })
+
+  it('rejects a --keystore path that does not exist', async () => {
+    await expect(
+      testWithoutChain(TestKeystoreCommand, ['--keystore', join(dir, 'nope.json')])
+    ).rejects.toThrow('does not exist')
+  })
+
+  // The wallet client is built without contacting a node, so only the public
+  // client is stubbed out here; the key really is decrypted and its address
+  // really is derived.
+  describe('wiring into the wallet client', () => {
+    let config: Config
+
+    class ExposedKeystoreCommand extends TestKeystoreCommand {
+      async run() {
+        await this.getWalletClient()
+      }
+      public resolveSigningKey() {
+        return this.getSigningPrivateKey()
+      }
+    }
+
+    const commandWith = (argv: string[]) => {
+      const command = new ExposedKeystoreCommand(
+        [...argv, '--node', 'http://localhost:8545'],
+        config
+      )
+      jest
+        .spyOn(BaseCommand.prototype, 'getPublicClient')
+        .mockResolvedValue({ chain: celoSepolia } as unknown as PublicCeloClient)
+      return command
+    }
+
+    beforeAll(async () => {
+      config = await Config.load(require.main?.filename || __dirname)
+    })
+
+    afterEach(() => {
+      jest.restoreAllMocks()
+    })
+
+    it('signs with the address held in the keystore', async () => {
+      const command = commandWith(['--keystore', keystoreFile, '--passwordFile', passwordFile])
+
+      const walletClient = await command.getWalletClient()
+      expect(walletClient.account.address).toBe(keystoreAddress)
+    })
+
+    it('asks for the password once, however many times the key is needed', async () => {
+      const promptSpy = jest.spyOn(ux, 'prompt').mockResolvedValue(PASSWORD)
+      const command = commandWith(['--keystore', keystoreFile])
+
+      await command.resolveSigningKey()
+      await command.resolveSigningKey()
+      await command.getWalletClient()
+
+      expect(promptSpy).toHaveBeenCalledTimes(1)
+    })
+
+    it('fails when --from disagrees with the keystore', async () => {
+      const command = commandWith([
+        '--keystore',
+        keystoreFile,
+        '--passwordFile',
+        passwordFile,
+        '--from',
+        '0x0000000000000000000000000000000000000001',
+      ])
+
+      await expect(command.getWalletClient()).rejects.toThrow(
+        `does not match the address derived from the keystore ${keystoreAddress}`
+      )
+    })
+  })
+})
+
 // Make sure telemetry tests are deterministic, otherwise we'd have to update tests every release
 jest.mock('../package.json', () => ({
   version: '5.2.3',

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'
@@ -35,6 +35,7 @@ import { CustomFlags } from './utils/command'
 import { configExists, getDefaultDerivationPath, getNodeUrl } from './utils/config'
 import { getFeeCurrencyContractWrapper } from './utils/fee-currency'
 import { requireNodeIsSynced } from './utils/helpers'
+import { privateKeyFromKeystore } from './utils/keystore'
 import { reportUsageStatisticsIfTelemetryEnabled } from './utils/telemetry'
 
 export abstract class BaseCommand extends Command {
@@ -43,7 +44,19 @@ export abstract class BaseCommand extends Command {
       char: 'k',
       description: 'Use a private key to sign local transactions with',
       hidden: false,
-      exclusive: ['useLedger', 'useAKV'],
+      exclusive: ['useLedger', 'useAKV', 'keystore'],
+    }),
+    keystore: CustomFlags.path({
+      description:
+        'Path to an encrypted keystore file, or to a directory of them. When a directory is given, --from selects the account. You will be prompted for the password unless --passwordFile is set.',
+      hidden: false,
+      exclusive: ['privateKey', 'useLedger', 'useAKV'],
+    }),
+    passwordFile: CustomFlags.path({
+      dependsOn: ['keystore'],
+      noCacheDefault: true,
+      description: 'Path to a file containing the password for --keystore',
+      hidden: false,
     }),
     node: Flags.string({
       char: 'n',
@@ -81,7 +94,7 @@ export abstract class BaseCommand extends Command {
     useLedger: Flags.boolean({
       default: false,
       hidden: false,
-      exclusive: ['privateKey'],
+      exclusive: ['privateKey', 'keystore'],
       description: 'Set it to use a ledger wallet',
     }),
     ledgerAddresses: Flags.integer({
@@ -107,7 +120,7 @@ export abstract class BaseCommand extends Command {
     }),
     useAKV: Flags.boolean({
       hidden: true,
-      exclusive: ['privateKey', 'useLedger'],
+      exclusive: ['privateKey', 'useLedger', 'keystore'],
       deprecated: true,
       description: 'Set it to use an Azure KeyVault HSM',
     }),
@@ -148,6 +161,7 @@ export abstract class BaseCommand extends Command {
   private walletClient: WalletCeloClient | null = null
   private _parseResult: null | ParserOutput<FlagOutput, FlagOutput> = null
   private ledgerTransport: Awaited<ReturnType<(typeof _TransportNodeHid)['open']>> | null = null
+  private _signingPrivateKey: string | null = null
 
   get _wallet(): ReadOnlyWallet | undefined {
     // the wallet lives on the connection; returning this._wallet would recurse
@@ -158,6 +172,31 @@ export abstract class BaseCommand extends Command {
     this._kit!.connection.wallet = wallet
   }
 
+  /**
+   * The private key to sign with, from --privateKey or by unlocking --keystore.
+   * Memoized so that a keystore is only decrypted, and its password only
+   * requested, once per command.
+   * @returns The private key, or undefined when signing is delegated elsewhere
+   * (a Ledger, or an account the node has unlocked).
+   */
+  protected async getSigningPrivateKey(): Promise<string | undefined> {
+    const res = await this.parse()
+    if (res.flags.privateKey) {
+      return res.flags.privateKey
+    }
+    if (!res.flags.keystore) {
+      return undefined
+    }
+    if (!this._signingPrivateKey) {
+      this._signingPrivateKey = await privateKeyFromKeystore({
+        keystorePath: res.flags.keystore,
+        passwordFile: res.flags.passwordFile,
+        from: res.flags.from,
+      })
+    }
+    return this._signingPrivateKey
+  }
+
   protected async getNodeUrl(): Promise<string> {
     const res = await this.parse()
 
@@ -178,8 +217,11 @@ export abstract class BaseCommand extends Command {
     }
 
     const res = await this.parse()
-    if (res.flags && res.flags.privateKey && !res.flags.useLedger && !res.flags.useAKV) {
-      this._kit.connection.addAccount(res.flags.privateKey)
+    if (res.flags && !res.flags.useLedger && !res.flags.useAKV) {
+      const privateKey = await this.getSigningPrivateKey()
+      if (privateKey) {
+        this._kit.connection.addAccount(privateKey)
+      }
     }
 
     return this._kit
@@ -260,11 +302,14 @@ export abstract class BaseCommand extends Command {
         }
       } else if (res.flags.useAKV) {
         failWith('--useAKV flag is no longer supported')
-      } else if (res.flags.privateKey) {
-        const accountFromPrivateKey = privateKeyToAccount(ensureLeading0x(res.flags.privateKey))
+      } else if (res.flags.privateKey || res.flags.keystore) {
+        const privateKey = (await this.getSigningPrivateKey())!
+        const accountFromPrivateKey = privateKeyToAccount(ensureLeading0x(privateKey))
         if (accountAddress && !isAddressEqual(accountAddress, accountFromPrivateKey.address)) {
           failWith(
-            `The --from address ${accountAddress} does not match the address derived from the provided private key ${accountFromPrivateKey.address}.`
+            `The --from address ${accountAddress} does not match the address derived from the ${
+              res.flags.keystore ? 'keystore' : 'provided private key'
+            } ${accountFromPrivateKey.address}.`
           )
         }
         this.walletClient = createWalletClient({

packages/cli/src/utils/command.ts

@@ -197,6 +197,11 @@ export const CustomFlags = {
     description: 'Hex string',
     helpValue: '0x',
   }),
+  path: Flags.custom({
+    parse: parsePath,
+    description: 'Path to an existing file or directory',
+    helpValue: '/path/to/file',
+  }),
 }
 
 export const CustomArgs = {