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.

Sign up for GitHub

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

Jump to bottom

Feat: Init can Pull existing Network #18

Merged
merged 9 commits into from
Jul 28, 2023
Merged

Feat: Init can Pull existing Network #18

merged 9 commits into from
Jul 28, 2023

Conversation

gluax
Copy link
Contributor

@gluax gluax commented Jul 27, 2023 •
edited
Loading

Motivation

The ability to grab the correct genesis.json and gentx files from a repo.

Explanation of Changes

I winded up splitting the init command into two further sub-commands(these both still take a moniker):

  • init new: This one is mostly the logic from the original init command. Granted I hard configured some changes, i.e. to have the denom be seda(although it will still need to be set to aseda manually in certain locations). My rationale for leaving this ability in is because eventually we will need to create other networks like mainnet and having a clean start genesis.json would be preferable for this.
  • init network - where by default it does testnet. However, you can specify another network and if that network exists it will pull it for you. NOTE: since I couldn't open source the repo yet this does use an ENV variable for a github personal access token. This is something you can use from your personal account.

They both do similar functionality of outputting the genesis.json to stdout. For that, I just copy pasted the code the original init command had.

Beyond that, I pulled out the other config file inits into a separate file, and put those defaults in there.

The exception to that are some of the const variables in the init.go file since those were used in that file. I could pull these all out to another file if people prefer.

Testing

I tested this locally and that it works with our new seda-networks repo.

I would like to write some unit tests... but I'd have to abstract out the http and file io operations into some interface and then figure out how to mock them. This is doable, but is a layer of abstraction over something we are only going to use, presumably, in this one place so I wasn't sure this was worth it?

Related PRs and Issues

Closes #16

@gluax gluax self-assigned this Jul 27, 2023
Thomasvdam
Thomasvdam approved these changes Jul 28, 2023
View reviewed changes
Copy link
Member

@Thomasvdam Thomasvdam left a comment

Choose a reason for hiding this comment

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

The export that's currently in the seda-networks repo is a little old (pre me destroying the network 😛). Perhaps we want a more recent one (would also reduce the size since there's less WASM code stored).

And I'm not sure whether we should call this testnet. I think it could help to reserve that term for a network that is more stable and public and name this devnet. How does the rest feel about this?

cmd/seda-chaind/utils/git_download.go
if err != nil {
return err
}
req.Header.Add("Authorization", "token "+Token)
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to merge it like this and clean up when the repos are open or do we wait and remove it before we merge?

mariocao
mariocao approved these changes Jul 28, 2023
View reviewed changes
Copy link
Member

@mariocao mariocao left a comment

Choose a reason for hiding this comment

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

I kind of like the devnet tag.

@gluax
Copy link
Contributor Author

gluax commented Jul 28, 2023

I'll put a more recent one and rename it to devnet before merging!

@gluax gluax merged commit 06cfbce into main Jul 28, 2023
1 check passed
@gluax gluax deleted the feat/init-config branch July 28, 2023 18:07
JeancarloBarrios added a commit that referenced this pull request Oct 18, 2023
@JeancarloBarrios
# This is a combination of 22 commits.
4559114 
# This is the 1st commit message:

fix: fix dockerbuild for arm arch

# The commit message #2 will be skipped:

# fix: fix failing Dockerfile for arm architecture

# The commit message #3 will be skipped:

# fix: add build setup for validator script

# The commit message #4 will be skipped:

# feat: add testnet deployment scripts

# The commit message #5 will be skipped:

# feat: node setup script

# The commit message #6 will be skipped:

# fix: use devnet ips

# The commit message #7 will be skipped:

# fix: jq fix

# The commit message #8 will be skipped:

# fix: replace use of deprecated flag for jq

# The commit message #9 will be skipped:

# fix: support linux environment and address comments

# The commit message #10 will be skipped:

# chore: use config example to hide pvt info

# The commit message #11 will be skipped:

# chore: fix gitignore

# The commit message #12 will be skipped:

# chore: add clarifying comments

# The commit message #13 will be skipped:

# fix: add build setup for validator script

# The commit message #14 will be skipped:

# fix: add build setup for validator script

# The commit message #15 will be skipped:

# fix: add build setup for validator script

# The commit message #16 will be skipped:

# refactor: improve init command

# The commit message #17 will be skipped:

# fix: init cmds bug fixes and refactor

# The commit message #18 will be skipped:

# chore: go mod tidy

# The commit message #19 will be skipped:

# fix: improve condition check in init join

# The commit message #20 will be skipped:

# fix: enable initializing node from given validator key file

# The commit message #21 will be skipped:

# fix: add build setup for validator script

# The commit message #22 will be skipped:

# fix: add build setup for validator script
JeancarloBarrios added a commit that referenced this pull request Oct 18, 2023
@JeancarloBarrios
# This is a combination of 22 commits.
ef8ac5a 
# This is the 1st commit message:

fix: fix dockerbuild for arm arch

# The commit message #2 will be skipped:

# fix: fix failing Dockerfile for arm architecture

# The commit message #3 will be skipped:

# fix: add build setup for validator script

# The commit message #4 will be skipped:

# feat: add testnet deployment scripts

# The commit message #5 will be skipped:

# feat: node setup script

# The commit message #6 will be skipped:

# fix: use devnet ips

# The commit message #7 will be skipped:

# fix: jq fix

# The commit message #8 will be skipped:

# fix: replace use of deprecated flag for jq

# The commit message #9 will be skipped:

# fix: support linux environment and address comments

# The commit message #10 will be skipped:

# chore: use config example to hide pvt info

# The commit message #11 will be skipped:

# chore: fix gitignore

# The commit message #12 will be skipped:

# chore: add clarifying comments

# The commit message #13 will be skipped:

# fix: add build setup for validator script

# The commit message #14 will be skipped:

# fix: add build setup for validator script

# The commit message #15 will be skipped:

# fix: add build setup for validator script

# The commit message #16 will be skipped:

# refactor: improve init command

# The commit message #17 will be skipped:

# fix: init cmds bug fixes and refactor

# The commit message #18 will be skipped:

# chore: go mod tidy

# The commit message #19 will be skipped:

# fix: improve condition check in init join

# The commit message #20 will be skipped:

# fix: enable initializing node from given validator key file

# The commit message #21 will be skipped:

# fix: add build setup for validator script

# The commit message #22 will be skipped:

# fix: add build setup for validator script
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jamesondh jamesondh jamesondh approved these changes

@Thomasvdam Thomasvdam Thomasvdam approved these changes

@mariocao mariocao mariocao approved these changes

@FranklinWaller FranklinWaller Awaiting requested review from FranklinWaller

@mennataboelnaga mennataboelnaga Awaiting requested review from mennataboelnaga

Assignees

@gluax gluax

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

✨ Add Genesis.json Pulling to Init Command
4 participants
@gluax @jamesondh @Thomasvdam @mariocao