Skip to content
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

feat(network): initialize init command #1771

Merged
merged 54 commits into from
Nov 17, 2021
Merged

feat(network): initialize init command #1771

merged 54 commits into from
Nov 17, 2021

Conversation

lumtis
Copy link
Contributor

@lumtis lumtis commented Nov 5, 2021

starport network chain init <launch-id> allows initializing a node from a spn chain launch ID

Opening now for reviews. Open for suggestion for the CLI output format

As mentioned in the specs. The account is added to the genesis with the same value as self-delegation for the gentx. If the user want to start the chain locally with the initialized genesis, it will fail begin the self-delegation tx costs gas and therefore the user account amount must be slightly higher.
Some question arise:

  • Do we really want to add a genesis account in the init command? This is not necessary for generating a gentx and therefore not necessary if we consider the init command if the purpose of the command is solely to generate the gentx. It can still be useful if we want to allow the command to let the user testing the chain locally. Maybe it can also be wise to "simulate" the start of the chain to ensure the generate gentx is valid
  • If we want to provide the genesis account balance, what should it be? If we ask it from the user, we could use the same value by default for the genesis account balance for the join command
  • If we support genesis account in the init command we must handle a edge case: if the account is already present in a custom URL genesis we should not call add-genesis-account or ignore its returned error

There is curious bug with the CLI output if my window is too short for the "do you want to erase?" question spawning 3 times. Does it happen for you?

starport n --local chain init 1 --from alice --key-name coco
? The chain has already been initialized under: /Users/lucas/spn/1. Would you like to overwrite the home directory? [
? The chain has already been initialized under: /Users/lucas/spn/1. Would you like to overwrite the home directory? [
? The chain has already been initialized under: /Users/lucas/spn/1. Would you like to overwrite the home directory? [
The chain has already been initialized under: /Users/lucas/spn/1. Would you like to overwrite the home directory: y
aborted
➜  starport git:(feat/network-init)

One last thing: I used Blockchain() method with SourceOption to initialize the blockchain from a launchID but I'm wondering if we shouldn't implement two separate methods for initialization from source URL and launch ID since both workflow seem to be pretty different

@lumtis lumtis marked this pull request as ready for review November 7, 2021 17:23
starport/cmd/network_chain_init.go Outdated Show resolved Hide resolved
starport/cmd/network_chain_init.go Outdated Show resolved Hide resolved
starport/cmd/network_chain_init.go Outdated Show resolved Hide resolved
starport/cmd/network_chain_init.go Outdated Show resolved Hide resolved
starport/services/network/blockchain.go Outdated Show resolved Hide resolved
starport/services/network/blockchain.go Outdated Show resolved Hide resolved
starport/services/network/network.go Show resolved Hide resolved
@lumtis lumtis requested a review from ilgooz November 10, 2021 15:00
@lumtis lumtis linked an issue Nov 10, 2021 that may be closed by this pull request
Pantani
Pantani previously approved these changes Nov 12, 2021
starport/cmd/account.go Outdated Show resolved Hide resolved
starport/cmd/network.go Outdated Show resolved Hide resolved
starport/cmd/network_chain_publish.go Show resolved Hide resolved
@ilgooz ilgooz merged commit 9aa042f into develop Nov 17, 2021
@ilgooz ilgooz deleted the feat/network-init branch November 17, 2021 13:35
Jchicode pushed a commit to Jchicode/cli that referenced this pull request Aug 9, 2023
* feat(cmd): add network publish cmd

and refactor the network related code.

* re-add network command

* hide the network cmd

* Apply suggestions from code review

Co-authored-by: Danilo Pantani <danpantani@gmail.com>
Co-authored-by: Lucas Bertrand <lucas.bertrand.22@gmail.com>

* consistent event printing

* docs

* Command initialization

* misc modifications

* chain home

* chain fetch

* Lint

* init account

* Add validator description

* Account creation

* Add mnemonic

* overwrite confirmation

* Fix mnemonic typo

* starport/cmd/network_chain_init.go

* Initialize recover

* Small modification

* Network initialization

* Refactor genesis and chain verification

* lint

* Refactor account

* Fix no account

* validator name change

* import account command

* import starport account

* fix account created

* Remove todos

* Update starport/cmd/network_chain_init.go

Co-authored-by: İlker G. Öztürk <ilkergoktugozturk@gmail.com>

* shutdown renaming

* Validator flag

* Fix cli spinner

* comments

* Fix genesis URL

* spinner

* small text change

* comment

* Genesis initialization

* Refactor genesis url init

Co-authored-by: İlker G. Öztürk <ilkergoktugozturk@gmail.com>
Co-authored-by: Danilo Pantani <danpantani@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

starport network chain init
3 participants