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

Add near generate-key command #178

Merged
merged 6 commits into from
Nov 5, 2019
Merged

Add near generate-key command #178

merged 6 commits into from
Nov 5, 2019

Conversation

vgrichina
Copy link
Contributor

Fixes #177

console.log(`Generated key pair with ${keyPair.publicKey} public key`);
}
})
};
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we have this function individually not with other functions? and this makes command and function together? why not follow what we already have?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@icerove didn't want to do too many chnages at once to pull every function, so for now only pulled new functions like this

The reason to do this is that it's more convenient to work with such structure if there are concurrent modifications that needs to be merged (e.g. we both add some commands).
Also current structure avoids extra step (you just define a command at once, without having to define a function first and then wrap it into command).

Copy link
Contributor

@bowenwang1996 bowenwang1996 left a comment

Choose a reason for hiding this comment

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

Looks like this will override the old key. Is that the best option?

@vgrichina
Copy link
Contributor Author

Looks like this will override the old key. Is that the best option?

@bowenwang1996 do you think we should ask the user whether to overwrite old key? Or just no-op with warning if key does exists?

@bowenwang1996
Copy link
Contributor

bowenwang1996 commented Nov 4, 2019

I think we should ask user before overriding the key file because once done, the private key would be gone. Normally that is fine because it's the same account, but if they actually want to use that key for a different network, they will lose their old key and that actually matters.
EDIT: actually it is not fine even if it's same account because for this key that you generate, it is not added to your account yet. So if you override the old key, it could be that you now lose any access to your account.

@vgrichina
Copy link
Contributor Author

@bowenwang1996 the problem you described is solved by doing no-op with warning too though. Benefit over asking is that it's easier to put in a script (does not require user input).

BTW we also can add separate network ID for stake wars.

@bowenwang1996
Copy link
Contributor

Sorry I don't understand. If you override your old key and that is your only key to your account, it seems that you will lose your account. Did I miss something?

@icerove
Copy link
Contributor

icerove commented Nov 4, 2019

How about only generate new public keys to that account and do not change account private key? not generate key pair but only public one. Since multi public keys are accepted.

@vgrichina
Copy link
Contributor Author

How about only generate new public keys to that account and do not change account private key? not generate key pair but only public one. Since multi public keys are accepted.

@icerove hmm, do you mean deriving multiple different key pairs from one private key? Sounds like a bit out of scope.

@bowenwang1996

Sorry I don't understand. If you override your old key and that is your only key to your account, it seems that you will lose your account. Did I miss something?

Yes, that I am suggesting to solve this problem in a different way: make near-shell not do anything when there is existing key for account, but just show a warning message. This also requires changes to PR, just a different change.

@bowenwang1996
Copy link
Contributor

Oh I see. Sure that works.

@vgrichina vgrichina force-pushed the generate-key branch 2 times, most recently from cc56768 to 67e5754 Compare November 5, 2019 06:12
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.

Generate account keys
3 participants