-
Notifications
You must be signed in to change notification settings - Fork 0
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
QA file-restructure #247
Conversation
const accounts = await config.get() | ||
.then(storedConfig => EntropyAccount.list(storedConfig)) | ||
.catch((err) => { | ||
if (err.message.includes('currently no accounts')) return [] | ||
|
||
throw err | ||
}) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CHANGE: entropy account ls
If I don't have any accounts yet I don't expect an error, I expect []
@@ -35,7 +38,8 @@ function entropyAccountCreate () { | |||
|
|||
cliWrite({ | |||
name: newAccount.name, | |||
address: newAccount.address | |||
address: newAccount.address, | |||
verifyingKeys: [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PATCH:
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
const fullAccount = keyring.getAccount() | ||
// TODO: sdk should create account on constructor | ||
const { admin } = keyring.getAccount() | ||
const data = fixData(fullAccount) |
There was a problem hiding this comment.
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
@@ -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 || [] |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed tests
Checklist
|
@mixmix similar for this PR, need to rebase off of current version of dev. |
@rh0delta DONE! |
PLEASE review + merge #250 first (makes this PR smaller)