Skip to content

Conversation

@devbugging
Copy link
Contributor

@devbugging devbugging commented Jun 27, 2022

Closes #537

Description

This PR implements the account creation interactive flow for mainnet, testnet and emulator networks. It also covers the encryption of keys for mainnet case. It also provides password prompts when running other commands with the encrypted keys.

The flow is as follows, you choose the network, if the chosen network is testnet then the faucet website is opened, if it's mainnet the flow port is opened, after you complete the required steps in browser to create an account, the CLI listens for new account creation event and stores the account in configuration. In the case of mainnet it offers you an option to encrypt.

Some screens:
Screenshot 2022-06-30 at 16 26 30
Screenshot 2022-06-30 at 16 26 47
Screenshot 2022-06-30 at 16 27 49
Screenshot 2022-06-30 at 16 26 11

DOD:

  • Implemneted
  • Tested
  • Released

For contributor use:

  • Targeted PR against master branch
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work
  • Code follows the standards mentioned here
  • Updated relevant documentation
  • Re-reviewed Files changed in the Github PR explorer
  • Added appropriate labels

@devbugging devbugging self-assigned this Jun 27, 2022
@devbugging devbugging changed the title Feature/account creation Interactive account creation for mainnet, testnet and emulator Jun 27, 2022
@devbugging
Copy link
Contributor Author

@srinjoyc if accounts are created using flow port they don't have enough funds to even send transactions. Shouldn't they be funded?
Also @psiemens during testing I noticed something really weird. The method that creates an account from the network, sometimes (maybe 10% of times) fails the check that the account only has one key. Sometimes that method returns bunch of public keys, the ones added are pretty random, but if you take a look on the network manually it's only one key on the account. I have a suspicion it might be some bug on AN. I plan to investigate.

//
// Ref: https://docs.onflow.org/flow-cli/security/#private-account-configuration-file
Location string
UseAdvanceFormat bool
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
UseAdvanceFormat bool
UseAdvancedFormat bool

"Advance format" isn't quite valid grammar

Comment on lines +351 to +362
func outputList(log *output.StdoutLogger, items []string, numbered bool) {
log.Info(fmt.Sprintf("%s:", items[0]))
items = items[1:]
for n, item := range items {
sep := " -"
if numbered {
sep = fmt.Sprintf(" %d.", n+1)
}
log.Info(fmt.Sprintf("%s %s", sep, item))
}
log.Info("")
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this go in the output package?

Copy link
Contributor Author

@devbugging devbugging Jul 27, 2022

Choose a reason for hiding this comment

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

It could yeah. Wilco. Generally, I was thinking about refactoring this interactive UI into a separate file since more commands will introduce this mode and it could be nicer to have an interactive UI component handling that. Not sure yet exactly how, since I want to wait a bit to see the pattern evolving but I have in mind to refactor this down the line, I don't like the command itself becoming bloated with these interactive responses.

devbugging and others added 5 commits July 27, 2022 10:16
Co-authored-by: Peter Siemens <peterjsiemens@gmail.com>
Co-authored-by: Peter Siemens <peterjsiemens@gmail.com>
Co-authored-by: Peter Siemens <peterjsiemens@gmail.com>
Co-authored-by: Peter Siemens <peterjsiemens@gmail.com>
Co-authored-by: Peter Siemens <peterjsiemens@gmail.com>
Copy link
Contributor

@psiemens psiemens left a comment

Choose a reason for hiding this comment

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

Looks good to me overall.

That public key check bug is strange. It sounds like a data consistency issue coming from the AN… did you get a chance to investigate?

@devbugging
Copy link
Contributor Author

Looks good to me overall.

That public key check bug is strange. It sounds like a data consistency issue coming from the AN… did you get a chance to investigate?

Yep investigated and fixed. It was in fact a bug in CLI as seen here: 71f9fd6
The problem was that since get address was used before only on a transaction that made a new account it didn't matter which address it returned so it did that on all events in that tx, whereas now we monitor more events and it's important that after we get the right event we extract address from that event not on all event range.

@devbugging devbugging merged commit 39fe01a into master Aug 2, 2022
@devbugging devbugging deleted the feature/account-creation branch November 24, 2022 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature A new user feature or a new package API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Interactive creation of accounts

6 participants