fix cli opts bug with multiple accountOptions (#250)

* fix cli opts bug with multiple accountOptions

* drop async argParser function! (unsupported)

* add setSelectedAccount

src/account/command.ts

@@ -4,7 +4,7 @@ import { EntropyAccount } from "./main";
 import { selectAndPersistNewAccount, addVerifyingKeyToAccountAndSelect } from "./utils";
 import { ACCOUNTS_CONTENT } from './constants'
 import * as config from '../config'
-import { cliWrite, accountOption, endpointOption, loadEntropy } from "../common/utils-cli";
+import { accountOption, endpointOption, cliWrite, loadEntropy } from "../common/utils-cli";
 
 export function entropyAccountCommand () {
   return new Command('account')
@@ -81,8 +81,8 @@ function entropyAccountList () {
 function entropyAccountRegister () {
   return new Command('register')
     .description('Register an entropy account with a program')
-    .addOption(endpointOption())
     .addOption(accountOption())
+    .addOption(endpointOption())
     // Removing these options for now until we update the design to accept program configs
     // .addOption(
     //   new Option(

src/account/interaction.ts

@@ -45,10 +45,7 @@ export async function entropyAccount (endpoint: string, storedConfig: EntropyCon
       return
     }
     const { selectedAccount } = await inquirer.prompt(accountSelectQuestions(accounts))
-    await config.set({
-      ...storedConfig,
-      selectedAccount: selectedAccount.address
-    })
+    await config.setSelectedAccount(selectedAccount)
 
     print('Current selected account is ' + selectedAccount)
     return

src/account/utils.ts

@@ -16,7 +16,8 @@ export async function selectAndPersistNewAccount (newAccount: EntropyAccountConf
     throw Error(`An account with address "${newAccount.address}" already exists.`)
   }
 
-  accounts.push(newAccount) 
+  // persist to config, set selectedAccount
+  accounts.push(newAccount)
   await config.set({
     ...storedConfig,
     selectedAccount: newAccount.address
@@ -28,9 +29,8 @@ export async function addVerifyingKeyToAccountAndSelect (verifyingKey: string, a
   const account = findAccountByAddressOrName(storedConfig.accounts, accountNameOrAddress)
   if (!account) throw Error(`Unable to persist verifyingKey "${verifyingKey}" to unknown account "${accountNameOrAddress}"`)
 
-  account.data.registration.verifyingKeys.push(verifyingKey)
-
   // persist to config, set selectedAccount
+  account.data.registration.verifyingKeys.push(verifyingKey)
   await config.set({
     ...storedConfig,
     selectedAccount: account.address

src/cli.ts

@@ -4,7 +4,7 @@
 import { Command, Option } from 'commander'
 
 import { EntropyTuiOptions } from './types'
-import { accountOption, endpointOption, loadEntropy } from './common/utils-cli'
+import { loadEntropy } from './common/utils-cli'
 import * as config from './config'
 
 import launchTui from './tui'
@@ -20,8 +20,6 @@ const program = new Command()
 program
   .name('entropy')
   .description('CLI interface for interacting with entropy.xyz. Running this binary without any commands or arguments starts a text-based interface.')
-  .addOption(accountOption())
-  .addOption(endpointOption())
   .addOption(
     new Option(
       '-d, --dev',

src/common/utils-cli.ts

@@ -20,7 +20,7 @@ function getConfigOrNull () {
 
 export function endpointOption () {
   return new Option(
-    '-e, --endpoint <endpoint>',
+    '-e, --endpoint <url>',
     [
       'Runs entropy with the given endpoint and ignores network endpoints in config.',
       'Can also be given a stored endpoint name from config eg: `entropy --endpoint test-net`.'
@@ -46,27 +46,33 @@ export function accountOption () {
   const storedConfig = getConfigOrNull()
 
   return new Option(
-    '-a, --account <accountAddressOrName>',
+    '-a, --account <name|address>',
     [
       'Sets the account for the session.',
       'Defaults to the last set account (or the first account if one has not been set before).'
     ].join(' ')
   )
     .env('ENTROPY_ACCOUNT')
-    .argParser(async (account) => {
-      if (storedConfig && storedConfig.selectedAccount !== account) {
-        // Updated selected account in config with new address from this option
-        await config.set({
-          ...storedConfig,
-          selectedAccount: account
-        })
-      }
+    .argParser(addressOrName => {
+      // We try to map addressOrName to an account we have stored
+      if (!storedConfig) return addressOrName
 
-      return account
+      const account = findAccountByAddressOrName(storedConfig.accounts, addressOrName)
+      if (!account) return addressOrName
+
+      // If we find one, we set this account as the future default
+      config.setSelectedAccount(account)
+      // NOTE: argParser cannot be an async function, so we cannot await this call
+      // WARNING: this will lead to a race-condition if functions are called in quick succession
+      // and assume the selectedAccount has been persisted
+      //
+      // RISK: doesn't seem likely as most of our functions will await at slow other steps....
+      // SOLUTION: write a scynchronous version?
+
+      // We finally return the account name to be as consistent as possible (using name, not address)
+      return account.name
     })
     .default(storedConfig?.selectedAccount)
-    // TODO: display the *name* not address
-    // TODO: standardise whether selectedAccount is name or address.
 }
 
 export async function loadEntropy (addressOrName: string, endpoint: string): Promise<Entropy> {

src/config/index.ts

@@ -6,7 +6,7 @@ import envPaths from 'env-paths'
 
 import allMigrations from './migrations'
 import { serialize, deserialize } from './encoding'
-import { EntropyConfig } from './types'
+import { EntropyConfig, EntropyAccountConfig } from './types'
 
 const paths = envPaths('entropy-cryptography', { suffix: '' })
 const CONFIG_PATH = join(paths.config, 'entropy-cli.json')
@@ -73,14 +73,28 @@ export async function set (config: EntropyConfig, configPath = CONFIG_PATH) {
   await writeFile(configPath, serialize(config))
 }
 
+export async function setSelectedAccount (account: EntropyAccountConfig, configPath = CONFIG_PATH) {
+  const storedConfig = await get(configPath)
+
+  if (storedConfig.selectedAccount === account.name) return storedConfig
+  // no need for update
+
+  const newConfig = {
+    ...storedConfig,
+    selectedAccount: account.name
+  }
+  await set(newConfig, configPath)
+  return newConfig
+}
+
 /* util */
 function noop () {}
-function assertConfigPath (configPath) {
+function assertConfigPath (configPath: string) {
   if (!configPath.endsWith('.json')) {
     throw Error(`configPath must be of form *.json, got ${configPath}`)
   }
 }
-export function isDangerousReadError (err) {
+export function isDangerousReadError (err: any) {
   // file not found:
   if (err.code === 'ENOENT') return false
 

src/config/types.ts

@@ -4,6 +4,7 @@ export interface EntropyConfig {
     dev: string;
     'test-net': string
   }
+  // selectedAccount is account.name (alias) for the account
   selectedAccount: string
   'migration-version': string
 }
@@ -14,6 +15,14 @@ export interface EntropyAccountConfig {
   data: EntropyAccountData
 }
 
+// Safe output format
+export interface EntropyAccountConfigFormatted {
+  name: string
+  address: string
+  verifyingKeys: string[]
+}
+
+// TODO: document this whole thing
 export interface EntropyAccountData {
   debug?: boolean
   seed: string

src/sign/command.ts

@@ -1,13 +1,13 @@
 import { Command, /* Option */ } from 'commander'
-import { cliWrite, accountOption, endpointOption, loadEntropy } from '../common/utils-cli'
+import { accountOption, endpointOption, cliWrite, loadEntropy } from '../common/utils-cli'
 import { EntropySign } from './main'
 
 export function entropySignCommand () {
   const signCommand = new Command('sign')
     .description('Sign a message using the Entropy network. Output is a JSON { verifyingKey, signature }')
     .argument('msg', 'Message you would like to sign (string)')
-    .addOption(endpointOption())
     .addOption(accountOption())
+    .addOption(endpointOption())
     // .addOption(
     //   new Option(
     //     '-r, --raw',

src/transfer/command.ts

@@ -8,8 +8,8 @@ export function entropyTransferCommand () {
     .description('Transfer funds between two Entropy accounts.') // TODO: name the output
     .argument('destination', 'Account address funds will be sent to')
     .argument('amount', 'Amount of funds to be moved')
-    .addOption(endpointOption())
     .addOption(accountOption())
+    .addOption(endpointOption())
     .action(async (destination, amount, opts) => {
       const entropy = await loadEntropy(opts.account, opts.endpoint)
       const transferService = new EntropyTransfer(entropy, opts.endpoint)

src/tui.ts

@@ -19,11 +19,7 @@ async function setupConfig () {
 
   // set selectedAccount if we can
   if (!storedConfig.selectedAccount && storedConfig.accounts.length) {
-    await config.set({
-      ...storedConfig,
-      selectedAccount: storedConfig.accounts[0].address
-    })
-    storedConfig = await config.get()
+    storedConfig = await config.setSelectedAccount(storedConfig.accounts[0])
   }
 
   return storedConfig

tests/account.test.ts

@@ -32,7 +32,7 @@ test('Account - list', async t => {
       dev: 'ws://127.0.0.1:9944',
       'test-net': 'wss://testnet.entropy.xyz',
     },
-    selectedAccount: account.address,
+    selectedAccount: account.name,
     'migration-version': '0'
   }