Skip to content
This repository has been archived by the owner on Oct 4, 2024. It is now read-only.

Create seed phrase option on add-key #997

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

tituszban
Copy link

The problem:
There is currently no way in the CLI to add a seed phrase to an already existing account. As a result, if an account was generated on the CLI, it is not easy to also add it on the web wallet.

The solution:
If add-key is called not with a public key, but with the --seedPhrase option, the key generated by that seed phrase is added to the account

@MattKetmo
Copy link

MattKetmo commented Jul 11, 2022

You can already add a seed phrase key via the CLI, but in 2 commands:

# First generate the key
near generate-key --seedPhrase 'your seed phrase ... 12 words' key.example

# Get the generated public key
pub_key=$(jq -r .public_key < ~/.near-credentials/testnet/key.example.json)

# Add the key to your current account
near add-key your_account.testnet "$pub_key"

The --seedPhrase option in add-key command directly overwrites the current account.
So it works only as a recovery or import method rather than "adding a mnemonic" to an account.

@tituszban
Copy link
Author

The --seedPhrase option in add-key command directly overwrites the current account.
So it works only as a recovery or import method rather than "adding a mnemonic" to an account.

It doesn't override the current account. It uses the current (already logged in) account to add the key generated by the mnemonic.

As to your example, yes, you can do that. But the discoverability or generating a key to an account which is then not used is quite poor, compared to just adding a new key using the seed phrase to the account directly.

@MattKetmo
Copy link

yes my bad, I confused add-key & generate-key when talking about "overwrites".

A few things about your code:

  • Setting access-key required to false changes the behavior of the command.
    It's no more a 2nd arg but an option --access-key. Which means part 1 in README should be edited too (as well as doc pages)

  • Also, since it's a non-required option, a non-empty check should be added before calling await account.addKey(options.accessKey at line 55 in order to avoid this error

Adding full access key = undefined to example.testnet.
An error occured
TypeError: Cannot read properties of undefined (reading 'borshSerialize')
    at serializeStruct (/private/tmp/near-cli/node_modules/borsh/lib/index.js:302:20)
...
  • I would not console.log the mnemonic for security reasons (or maybe you wanted to write seedPhrasePublicKey at line 44?)

Otherwise yes I think this improve ux. We could even have an option to auto generate the seed phrase.

@tituszban
Copy link
Author

@@ -5,13 +5,13 @@ const { utils } = require('near-api-js');
const checkCredentials = require('../utils/check-credentials');

module.exports = {
command: 'add-key <account-id> <access-key>',
command: 'add-key <account-id>',

Choose a reason for hiding this comment

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

I noticed there is another syntax to possibly use the existing arg as an optional parameter

Suggested change
command: 'add-key <account-id>',
command: 'add-key <account-id> [access-key]',

example in the generate-key command

This would allow to not break the current input of the command (and thus the existing docs). But I let maintainers decide for the best solution :)

Copy link
Author

@tituszban tituszban Aug 1, 2022

Choose a reason for hiding this comment

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

I think that's a pretty good idea. Other thoughts?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants