feat(cli): generate account key if not specified #3587
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## development #3587 +/- ##
===============================================
+ Coverage 50.51% 51.75% +1.24%
===============================================
Files 230 232 +2
Lines 29006 23793 -5213
===============================================
- Hits 14653 12315 -2338
+ Misses 12856 9972 -2884
- Partials 1497 1506 +9 |
| case "default": | ||
| if keyStore.Type() == "ed25519" { | ||
| kp, err := ed25519.GenerateKeypair() | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| return keyStore.Insert(kp) | ||
| } else { | ||
| kp, err := sr25519.GenerateKeypair() | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| return keyStore.Insert(kp) | ||
| } |
There was a problem hiding this comment.
We currently only have the test key flow in the CLI. Check https://github.com/1garo/gossamer/blob/feat/generate-account-if-not-specified/cmd/gossamer/commands/root.go#L577
I feel we should separate out the flow for the keystore setup in the cmd package instead of this change. The rust implementation has --dev flag; maybe we could add the same!
There was a problem hiding this comment.
you mean something like subkey?
There was a problem hiding this comment.
Maybe account generate already do that, I don't know if I really understood what you meant
There was a problem hiding this comment.
I spent some more time going through the code. Thank you for your patience and time!
I believe we don't have the need to generate an account if not specified since we're still in development and only need development accounts.
If we want to support the use of auto-generated or external accounts, we'll have to
- Add a new flag
devto enable the clear separation for the development runs - Clear separation for the development config
- Store the auto generated account in disk
- Ability to load the account from disk
I think this isn't a priority for us at the moment. I'll update the issue.
I managed to make the error go away adding keyRing.Default() and reusing alice's grandpa priv key on |
|
@kanishkatn regards this I think we don't need to introduce the The following command should generate an account for me But this following command should not generate an account but use alice test account instead I agree with these 2 specifications:
|
|
@EclesioMeloJunior I agree. Thank you! |
|
@1garo do you want to continue working on this or shall we take this over? |
q9f
added
A-stale
|
I definitely can continue working on it! |
|
Hey @1garo, how are the things going? Are you planning to continue working on this PR or can we close it as stale? Let us know if you need any help Thks! |
Sorry for the delayed response, I will not be able to work on the PR. Feel free to close or if someone wants to get the issue it's okay for me. |
Thanks @1garo I'll close it for now. |
Changes
Tests
Issues
#3255
Primary Reviewer
@kanishkatn