Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor entropy register so CLI/TUI are similar #242

Merged
merged 2 commits into from
Sep 27, 2024
Merged

refactor entropy register so CLI/TUI are similar #242

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cc63c72
refactor entropy register so CLI/TUI are similar
mixmix Sep 26, 2024
4c86915
add detailed error message on failure to persist verifyingKey
mixmix Sep 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 6 additions & 22 deletions src/account/command.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
import Entropy from "@entropyxyz/sdk"
import { Command, Option } from 'commander'
import { EntropyAccount } from "./main";
import { selectAndPersistNewAccount } from "./utils";
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 { findAccountByAddressOrName } from "src/common/utils";

export function entropyAccountCommand () {
return new Command('account')
Expand Down Expand Up @@ -102,28 +101,13 @@ function entropyAccountRegister () {
// )
// )
.action(async (opts) => {
const { account, endpoint, /* password */ } = opts
const storedConfig = await config.get()
const { accounts } = storedConfig
const accountToRegister = findAccountByAddressOrName(accounts, account)
if (!accountToRegister) {
throw new Error('AccountError: Unable to register non-existent account')
}

const entropy: Entropy = await loadEntropy(accountToRegister.address, endpoint)
const accountService = new EntropyAccount(entropy, endpoint)
const updatedAccount = await accountService.registerAccount(accountToRegister)
// NOTE: loadEntropy throws if it can't find opts.account
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// NOTE: loadEntropy throws if it can't find opts.account
// NOTE: loadEntropy throws if it can't find opts.account
// This is important because we don't want to register if we don't have a place to persist later

const entropy: Entropy = await loadEntropy(opts.account, opts.endpoint)
const accountService = new EntropyAccount(entropy, opts.endpoint)

const arrIdx = accounts.indexOf(accountToRegister)
accounts.splice(arrIdx, 1, updatedAccount)
await config.set({
...storedConfig,
accounts,
selectedAccount: updatedAccount.address
})
const verifyingKey = await accountService.register()
await addVerifyingKeyToAccountAndSelect(verifyingKey, opts.account)

const verifyingKeys = updatedAccount?.data?.registration?.verifyingKeys
const verifyingKey = verifyingKeys[verifyingKeys.length - 1]
cliWrite(verifyingKey)
process.exit(0)
})
Expand Down
19 changes: 9 additions & 10 deletions src/account/interaction.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import inquirer from "inquirer";
import Entropy from "@entropyxyz/sdk";

import { EntropyAccount } from './main'
import { selectAndPersistNewAccount } from "./utils";
import { selectAndPersistNewAccount, addVerifyingKeyToAccountAndSelect } from "./utils";
import { findAccountByAddressOrName, print } from "../common/utils"
import { EntropyConfig } from "../config/types";
import * as config from "../config";
Expand Down Expand Up @@ -77,16 +77,15 @@ export async function entropyRegister (entropy: Entropy, endpoint: string, store
const accountService = new EntropyAccount(entropy, endpoint)

const { accounts, selectedAccount } = storedConfig
const currentAccount = findAccountByAddressOrName(accounts, selectedAccount)
if (!currentAccount) {
const account = findAccountByAddressOrName(accounts, selectedAccount)
if (!account) {
print("No account selected to register")
return;
return
}
print("Attempting to register the address:", currentAccount.address)
const updatedAccount = await accountService.registerAccount(currentAccount)
const arrIdx = accounts.indexOf(currentAccount)
accounts.splice(arrIdx, 1, updatedAccount)
print("Your address", updatedAccount.address, "has been successfully registered.")

return { accounts, selectedAccount }
print("Attempting to register the address:", account.address)
const verifyingKey = await accountService.register()
await addVerifyingKeyToAccountAndSelect(verifyingKey, account.address)

print("Your address", account.address, "has been successfully registered.")
}
23 changes: 1 addition & 22 deletions src/account/main.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ export class EntropyAccount extends EntropyBase {
}
: undefined

this.logger.debug(`registering with params: ${registerParams}`, 'REGISTER')
return this.entropy.register(registerParams)
// NOTE: if "register" fails for any reason, core currently leaves the chain in a "polluted"
// state. To fix this we manually "prune" the dirty registration transaction.
Expand All @@ -75,28 +76,6 @@ export class EntropyAccount extends EntropyBase {
})
}

// WATCH: should this be extracted to interaction.ts?
async registerAccount (account: EntropyAccountConfig, registerParams?: AccountRegisterParams): Promise<EntropyAccountConfig> {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All this function is is registerAndPushKeysIntoAccount

I have pulled this out into the verbosely named util which

  • pushes into account
  • persists config

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's also the lgger correct?

this.logger.debug(
[
`registering account: ${account.address}`,
// @ts-expect-error Type export of ChildKey still not available from SDK
`to keyring: ${this.entropy.keyring.getLazyLoadAccountProxy('registration').pair.address}`
].join(', '),
'REGISTER'
)
// Register params to be defined from user input (arguments/options or inquirer prompts)
try {
const verifyingKey = await this.register(registerParams)
// NOTE: this mutation triggers events in Keyring
account.data.registration.verifyingKeys.push(verifyingKey)
return account
} catch (error) {
this.logger.error('There was a problem registering', error)
throw error
}
}

/* PRIVATE */

private async pruneRegistration () {
Expand Down
21 changes: 17 additions & 4 deletions src/account/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { ACCOUNTS_CONTENT } from './constants';
import { EntropyAccountConfig } from "../config/types";
import * as config from "../config";
import { ACCOUNTS_CONTENT } from './constants';
import { generateAccountChoices } from '../common/utils';
import { generateAccountChoices, findAccountByAddressOrName } from '../common/utils';

export async function selectAndPersistNewAccount (newAccount) {
export async function selectAndPersistNewAccount (newAccount: EntropyAccountConfig) {
const storedConfig = await config.get()
const { accounts } = storedConfig

Expand All @@ -19,11 +19,24 @@ export async function selectAndPersistNewAccount (newAccount) {
accounts.push(newAccount)
await config.set({
...storedConfig,
accounts,
selectedAccount: newAccount.address
})
}

export async function addVerifyingKeyToAccountAndSelect (verifyingKey: string, accountNameOrAddress: string) {
const storedConfig = await config.get()
const account = findAccountByAddressOrName(storedConfig.accounts, accountNameOrAddress)
if (!account) throw Error(`Unable to persist verifyingKey "${verifyingKey}" to unknown account "${accountNameOrAddress}"`)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very unlikely because we have earlier checked the account exists...but we put a nice message out anyway 🤷

Alternative.. make a function with an uglier signature (storeConfig, verifyingKey, account) ? 😢 this could still fail if account is not within storedConfig.accounts tho ...


account.data.registration.verifyingKeys.push(verifyingKey)

// persist to config, set selectedAccount
await config.set({
...storedConfig,
selectedAccount: account.address
})
Comment on lines +34 to +37
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check my working.

We don't need to any of the fancy slicing or inserting accounts here because:

  • on line 28 : we grab the ref for target account in storedConfig.accounts
  • on line 31 : we push a key into that account's data (mutating it)
  • on line 35 : we say "persist that mutated storedConfig"

}

function validateSeedInput (seed) {
if (seed.includes('#debug')) return true
if (seed.length === 66 && seed.startsWith('0x')) return true
Expand Down
Loading