From fb4691b6582d130b894d21a40b3c9aecff2e27ff Mon Sep 17 00:00:00 2001 From: mixmix Date: Thu, 3 Oct 2024 13:33:08 +1300 Subject: [PATCH 1/3] fix cli opts bug with multiple accountOptions --- src/account/command.ts | 6 +++--- src/cli.ts | 4 +--- src/common/utils-cli.ts | 7 ++++--- src/sign/command.ts | 6 +++--- src/transfer/command.ts | 6 +++--- 5 files changed, 14 insertions(+), 15 deletions(-) diff --git a/src/account/command.ts b/src/account/command.ts index a502a850..8447af16 100644 --- a/src/account/command.ts +++ b/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, passwordOption } from "../common/utils-cli"; +import { accountOption, endpointOption, passwordOption, cliWrite, loadEntropy } from "../common/utils-cli"; export function entropyAccountCommand () { return new Command('account') @@ -84,9 +84,9 @@ function entropyAccountList () { function entropyAccountRegister () { return new Command('register') .description('Register an entropy account with a program') - .addOption(passwordOption()) - .addOption(endpointOption()) .addOption(accountOption()) + .addOption(endpointOption()) + .addOption(passwordOption()) // Removing these options for now until we update the design to accept program configs // .addOption( // new Option( diff --git a/src/cli.ts b/src/cli.ts index b531efa8..cc004d79 100644 --- a/src/cli.ts +++ b/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', diff --git a/src/common/utils-cli.ts b/src/common/utils-cli.ts index 3aec350b..00fc8c34 100644 --- a/src/common/utils-cli.ts +++ b/src/common/utils-cli.ts @@ -20,7 +20,7 @@ function getConfigOrNull () { export function endpointOption () { return new Option( - '-e, --endpoint ', + '-e, --endpoint ', [ '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`.' @@ -44,16 +44,17 @@ export function endpointOption () { export function passwordOption (description?: string) { return new Option( - '-p, --password ', + '-p, --password ', description || 'Password for the account' ) + .hideHelp(true) } export function accountOption () { const storedConfig = getConfigOrNull() return new Option( - '-a, --account ', + '-a, --account ', [ 'Sets the account for the session.', 'Defaults to the last set account (or the first account if one has not been set before).' diff --git a/src/sign/command.ts b/src/sign/command.ts index fc574228..1c67f4d6 100644 --- a/src/sign/command.ts +++ b/src/sign/command.ts @@ -1,14 +1,14 @@ import { Command, /* Option */ } from 'commander' -import { cliWrite, accountOption, endpointOption, loadEntropy, passwordOption } from '../common/utils-cli' +import { accountOption, endpointOption, passwordOption, 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(passwordOption('Password for the source account (if required)')) - .addOption(endpointOption()) .addOption(accountOption()) + .addOption(endpointOption()) + .addOption(passwordOption('Password for the source account (if required)')) // .addOption( // new Option( // '-r, --raw', diff --git a/src/transfer/command.ts b/src/transfer/command.ts index b814e2f9..e8ef3ceb 100644 --- a/src/transfer/command.ts +++ b/src/transfer/command.ts @@ -1,5 +1,5 @@ import { Command } from "commander" -import { accountOption, endpointOption, loadEntropy, passwordOption } from "src/common/utils-cli" +import { accountOption, endpointOption, passwordOption, loadEntropy } from "../common/utils-cli" import { EntropyTransfer } from "./main" export function entropyTransferCommand () { @@ -8,9 +8,9 @@ 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(passwordOption('Password for the source account (if required)')) - .addOption(endpointOption()) .addOption(accountOption()) + .addOption(endpointOption()) + .addOption(passwordOption('Password for the source account (if required)')) .action(async (destination, amount, opts) => { const entropy = await loadEntropy(opts.account, opts.endpoint) const transferService = new EntropyTransfer(entropy, opts.endpoint) From 6eccbba805096e430b0c75645b8b692376ad7b4c Mon Sep 17 00:00:00 2001 From: mixmix Date: Thu, 3 Oct 2024 13:51:23 +1300 Subject: [PATCH 2/3] drop async argParser function! (unsupported) --- src/common/utils-cli.ts | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/src/common/utils-cli.ts b/src/common/utils-cli.ts index 00fc8c34..f8e7be9b 100644 --- a/src/common/utils-cli.ts +++ b/src/common/utils-cli.ts @@ -61,20 +61,29 @@ export function accountOption () { ].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.set({ + ...storedConfig, + selectedAccount: account.name + }) + // 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, password?: string): Promise { From ea2388148d7658dcb01ed455c754c8b99036c391 Mon Sep 17 00:00:00 2001 From: mixmix Date: Thu, 3 Oct 2024 14:01:13 +1300 Subject: [PATCH 3/3] add setSelectedAccount --- src/account/interaction.ts | 5 +---- src/account/utils.ts | 6 +++--- src/common/utils-cli.ts | 5 +---- src/config/index.ts | 20 +++++++++++++++++--- src/config/types.ts | 9 +++++++++ src/tui.ts | 6 +----- tests/account.test.ts | 2 +- 7 files changed, 33 insertions(+), 20 deletions(-) diff --git a/src/account/interaction.ts b/src/account/interaction.ts index bf1c599f..b26103dd 100644 --- a/src/account/interaction.ts +++ b/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 diff --git a/src/account/utils.ts b/src/account/utils.ts index cf51603a..c8efef2e 100644 --- a/src/account/utils.ts +++ b/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 diff --git a/src/common/utils-cli.ts b/src/common/utils-cli.ts index f8e7be9b..96f8bed7 100644 --- a/src/common/utils-cli.ts +++ b/src/common/utils-cli.ts @@ -69,10 +69,7 @@ export function accountOption () { if (!account) return addressOrName // If we find one, we set this account as the future default - config.set({ - ...storedConfig, - selectedAccount: account.name - }) + 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 diff --git a/src/config/index.ts b/src/config/index.ts index 049b6775..a38859a4 100644 --- a/src/config/index.ts +++ b/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 diff --git a/src/config/types.ts b/src/config/types.ts index 7d4deb78..26aa6678 100644 --- a/src/config/types.ts +++ b/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 diff --git a/src/tui.ts b/src/tui.ts index bd7780cf..f02d82da 100644 --- a/src/tui.ts +++ b/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 diff --git a/tests/account.test.ts b/tests/account.test.ts index 9f0288f6..220f101a 100644 --- a/tests/account.test.ts +++ b/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' }