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.

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

QA file-restructure #247

Open
wants to merge 5 commits into
base: naynay/file-restructure
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@
"commander": "^12.0.0",
"env-paths": "^3.0.0",
"inquirer": "8.0.0",
"lodash.clonedeep": "^4.5.0",
"mkdirp": "^3.0.1",
"winston": "^3.13.0",
"x25519": "^0.1.0"
Expand Down
23 changes: 17 additions & 6 deletions src/account/command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ export function entropyAccountCommand () {
.addCommand(entropyAccountImport())
.addCommand(entropyAccountList())
.addCommand(entropyAccountRegister())
// .addCommand(entropyAccountAlias())
// IDEA: support aliases for remote accounts (those we don't have seeds for)
// this would make transfers safer/ easier from CLI
}

function entropyAccountCreate () {
Expand All @@ -35,7 +38,8 @@ function entropyAccountCreate () {

cliWrite({
name: newAccount.name,
address: newAccount.address
address: newAccount.address,
verifyingKeys: []
Copy link
Contributor Author

Choose a reason for hiding this comment

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

entropy account create
entropy account create

Have them output the same format as entropy account ls
✔️ consistency
✔️ set up expectations that there is something interesting/ important meant to go in here

})
process.exit(0)
})
Expand All @@ -61,7 +65,8 @@ function entropyAccountImport () {

cliWrite({
name: newAccount.name,
address: newAccount.address
address: newAccount.address,
verifyingKeys: []
})
process.exit(0)
})
Expand All @@ -73,8 +78,14 @@ function entropyAccountList () {
.description('List all accounts. Output is JSON of form [{ name, address, verifyingKeys }]')
.action(async () => {
// TODO: test if it's an encrypted account, if no password provided, throw because later on there's no protection from a prompt coming up
const storedConfig = await config.get()
const accounts = EntropyAccount.list(storedConfig)
const accounts = await config.get()
.then(storedConfig => EntropyAccount.list(storedConfig))
.catch((err) => {
if (err.message.includes('currently no accounts')) return []

throw err
})

Comment on lines +81 to +88
Copy link
Contributor Author

Choose a reason for hiding this comment

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

entropy account ls
If I don't have any accounts yet I don't expect an error, I expect []

cliWrite(accounts)
process.exit(0)
})
Expand All @@ -84,9 +95,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(
Expand Down
5 changes: 1 addition & 4 deletions src/account/interaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
49 changes: 37 additions & 12 deletions src/account/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { FLOW_CONTEXT } from "./constants";
import { AccountCreateParams, AccountImportParams, AccountRegisterParams } from "./types";

import { EntropyBase } from "../common/entropy-base";
import { EntropyAccountConfig } from "../config/types";
import { EntropyAccountConfig, EntropyAccountConfigFormatted } from "../config/types";

export class EntropyAccount extends EntropyBase {
constructor (entropy: Entropy, endpoint: string) {
Expand All @@ -19,28 +19,28 @@ export class EntropyAccount extends EntropyBase {
return EntropyAccount.import({ name, seed, path })
}

// WARNING: #create depends on #import => be careful modifying this function
static async import ({ name, seed, path }: AccountImportParams ): Promise<EntropyAccountConfig> {
// WARNING: #create currently depends on this => be careful modifying this function

await wasmGlobalsReady()
const keyring = new Keyring({ seed, path, debug: true })

const fullAccount = keyring.getAccount()
// TODO: sdk should create account on constructor
const { admin } = keyring.getAccount()
const data = fixData(fullAccount)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is a bug in the SDK where you are "cloning" with `JSON.parse(JSON.stringify(obj)) and it fucks up the Uint8Array, which then fucks up config, logging etc.

This is a quick hack

const maybeEncryptedData = data
// const maybeEncryptedData = password ? passwordFlow.encrypt(data, password) : data

const data = fullAccount
const { admin } = keyring.getAccount()
delete admin.pair
// const encryptedData = password ? passwordFlow.encrypt(data, password) : data


return {
name,
address: admin.address,
data
// data: encryptedData // TODO: replace once password input is added back
data: maybeEncryptedData,
}
}

static list ({ accounts }: { accounts: EntropyAccountConfig[] }) {
static list ({ accounts }: { accounts: EntropyAccountConfig[] }): EntropyAccountConfigFormatted[] {
if (!accounts.length)
throw new Error(
'AccountsError: There are currently no accounts available, please create or import a new account using the Manage Accounts feature'
Expand All @@ -49,7 +49,7 @@ export class EntropyAccount extends EntropyBase {
return accounts.map((account: EntropyAccountConfig) => ({
name: account.name,
address: account.address,
verifyingKeys: account?.data?.admin?.verifyingKeys
verifyingKeys: account?.data?.registration?.verifyingKeys || []
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?
I assume verifyingKeys were meant to go under registration and not admin right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

AH tests say no. I don't know what the difference is between `admin + registration.
I changed where vks are being pushed into elsewhere in code too

}))
}

Expand Down Expand Up @@ -90,7 +90,7 @@ export class EntropyAccount extends EntropyBase {
dispatchError.asModule
)
const { docs, name, section } = decoded

msg = `${section}.${name}: ${docs.join(' ')}`
} else {
// Other, CannotLookup, BadOrigin, no extra info
Expand All @@ -107,3 +107,28 @@ export class EntropyAccount extends EntropyBase {
})
}
}

// TODO: there is a bug in SDK that is munting this data
function fixData (data) {
if (data.admin) {
data.admin.pair.addressRaw = objToUint8Array(data.admin.pair.addressRaw)
data.admin.pair.secretKey = objToUint8Array(data.admin.pair.secretKey)
data.admin.pair.publicKey = objToUint8Array(data.admin.pair.publicKey)
}

if (data.registration) {
data.registration.pair.addressRaw = objToUint8Array(data.registration.pair.addressRaw)
data.registration.pair.secretKey = objToUint8Array(data.registration.pair.secretKey)
data.registration.pair.publicKey = objToUint8Array(data.registration.pair.publicKey)
}

return data
}

function objToUint8Array (obj) {
const values: any = Object.entries(obj)
.sort((a, b) => Number(a[0]) - Number(b[0])) // sort entries by keys
.map(entry => entry[1])

return new Uint8Array(values)
}
8 changes: 5 additions & 3 deletions src/account/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,21 +19,23 @@ export async function selectAndPersistNewAccount (newAccount: EntropyAccountConf
accounts.push(newAccount)
await config.set({
...storedConfig,
selectedAccount: newAccount.address
selectedAccount: newAccount.name
})
}

export async function addVerifyingKeyToAccountAndSelect (verifyingKey: string, accountNameOrAddress: string) {
const storedConfig = await config.get()
const account = findAccountByAddressOrName(storedConfig.accounts, accountNameOrAddress)
const { accounts } = storedConfig

const account = findAccountByAddressOrName(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
await config.set({
...storedConfig,
selectedAccount: account.address
setSelectedAccount: account.name
})
}

Expand Down
19 changes: 13 additions & 6 deletions src/balance/command.ts
Original file line number Diff line number Diff line change
@@ -1,22 +1,29 @@
import { Command } from "commander";
import Entropy from "@entropyxyz/sdk";
import { cliWrite, endpointOption, loadEntropy, passwordOption } from "src/common/utils-cli";

import { EntropyBalance } from "./main";
import { cliWrite, endpointOption, loadEntropy, passwordOption } from "../common/utils-cli";
import { findAccountByAddressOrName } from "../common/utils";
import * as config from "../config";

export function entropyBalanceCommand () {
const balanceCommand = new Command('balance')
balanceCommand
.description('Command to retrieive the balance of an account on the Entropy Network')
.argument('address', 'Account address whose balance you want to query')
.addOption(passwordOption())
.argument('account <address|name>', 'Account address whose balance you want to query')
.addOption(endpointOption())
.action(async (address, opts) => {
const entropy: Entropy = await loadEntropy(address, opts.endpoint)
.addOption(passwordOption())
.action(async (account, opts) => {
const entropy: Entropy = await loadEntropy(account, opts.endpoint)
const BalanceService = new EntropyBalance(entropy, opts.endpoint)

const { accounts } = await config.get()
const address = findAccountByAddressOrName(accounts, account)?.address

const balance = await BalanceService.getBalance(address)
cliWrite(`${balance.toLocaleString('en-US')} BITS`)
process.exit(0)
})

return balanceCommand
}
51 changes: 26 additions & 25 deletions src/common/masking.ts
Original file line number Diff line number Diff line change
@@ -1,37 +1,38 @@
import cloneDeep from 'lodash.clonedeep'

const DEFAULT_MASKED_FIELDS = [
const PREFIX = 'data:application/UI8A;base64,'
const DEFAULT_MASKED_FIELDS = new Set([
'seed',
'secretKey',
'addressRaw',
];
]);

export function maskPayload (payload: any): any {
if (typeof payload === 'string') return payload
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I ended up in here cos of the masking of the badly handled UInt8Arrays... and reading it I saw there was not test and it was pretty confusing, so I wrote tests and wrote a simpler function with less dependencies.

I think it's an improvement ... I mean it needed to check the DEFAULT_MASKED_FIELDS earlier cos the test for "is this a Uint8Array or a munted Uint8Array (object like { "0": 34, "1": 23, ... }) ... yeah that broke when I fixed the bad keys coming from SDK...

There's a chance I introduced more edge cases...but look at the tests I guess!


const clonedPayload = cloneDeep(payload);
const maskedPayload = {}
if (
typeof payload === 'string' ||
typeof payload === 'boolean' ||
payload === null
) return payload

if (!clonedPayload) {
return clonedPayload;
}
const maskedPayload = Array.isArray(payload)
? []
: {}

// maskJSONFields doesn't handle nested objects very well so we'll
// need to recursively walk to object and mask them one by one
for (const [property, value] of Object.entries(clonedPayload)) {
if (value && typeof value === 'object') {
if (Object.keys(clonedPayload[property]).filter(key => isNaN(parseInt(key))).length === 0) {
const reconstructedUintArr: number[] = Object.values(clonedPayload[property])
maskedPayload[property] = "base64:" + Buffer.from(reconstructedUintArr).toString("base64");
} else {
maskedPayload[property] = maskPayload(value);
}
} else if (value && typeof value === 'string' && DEFAULT_MASKED_FIELDS.includes(property)) {
maskedPayload[property] = "*".repeat(clonedPayload[property].length)
} else {
maskedPayload[property] = value
return Object.entries(payload).reduce((acc, [property, value]) => {
if (DEFAULT_MASKED_FIELDS.has(property)) {
// @ts-expect-error .length does not exist on type "unknown"
acc[property] = "*".repeat(value?.length || 32)
}
else if (value instanceof Uint8Array) {
acc[property] = PREFIX + Buffer.from(value).toString('base64')
}
else if (typeof value === 'object') {
acc[property] = maskPayload(value);
}
else {
acc[property] = value
}
}

return maskedPayload;
return acc
}, maskedPayload)
}
37 changes: 22 additions & 15 deletions src/common/utils-cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ function getConfigOrNull () {

export function endpointOption () {
return new Option(
'-e, --endpoint <endpoint>',
'-e, --endpoint <url>',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

reading the docs the pattern is -shortcode --longcode <input-type>

[
'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`.'
Expand All @@ -39,41 +39,48 @@ export function endpointOption () {
return endpoint
})
.default('ws://testnet.entropy.xyz:9944/')
// NOTE: argParser is only run IF an option is provided, so this cannot be 'test-net'
// NOTE: argParser only runs IF the -e/--endpoint option called, so this default cannot be 'test-net'
}

export function passwordOption (description?: string) {
return new Option(
'-p, --password <password>',
'-p, --password',
description || 'Password for the account'
)
.hideHelp() // TEMP
}

export function accountOption () {
const storedConfig = getConfigOrNull()

return new Option(
'-a, --account <accountAddressOrName>',
'-a, --account <address|name>',
[
'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 => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

🔥 🔥 NASTY bug

this thing don't handle async!

See comments below

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I caught this cos I tried to register naynay in my QA and it was registering faucet!

// 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, password?: string): Promise<Entropy> {
Expand Down
11 changes: 8 additions & 3 deletions src/config/encoding.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
const PREFIX = 'data:application/UI8A;base64,'
// was a UInt8Array, but is stored as base64

export function serialize (config) {
export function serialize (config: object) {
return JSON.stringify(config, replacer, 2)
}

export function deserialize (config) {
return JSON.parse(config, reviver)
export function deserialize (config: string) {
try {
return JSON.parse(config, reviver)
} catch (err) {
console.log('broken config:', config)
throw err
}
}

function replacer (_key: string, value: any) {
Expand Down
Loading
Loading