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(config.yml): add cointype to the config.yml #1663

Merged
merged 31 commits into from
Oct 12, 2021
Merged

feat(config.yml): add cointype to the config.yml #1663

merged 31 commits into from
Oct 12, 2021

Conversation

fly33499
Copy link
Contributor

@fly33499 fly33499 commented Oct 9, 2021

Hi again.

In the case of a chain with its own hdpath or cointype, it is impossible to test in config.yml.
So, I searched and found some discussion about it.

#811

I love starport and I always want to contribution this project. so I tried some implementation about it.

At first, I want to develop using HdPath but, I found some hint from "keys add --coin-type" command.

--coin-type uint32         coin type number for HD derivation (default 118)

So, my recommendation is below

accounts:
  - name: alice
    coins: ["2000000000000ufct"]
    mnemonic: ozone unfold device pave lemon potato ....
    cointype: 7777777

and if there's no cointype data, the value will set 118 value as a default.

I have confirmed that it is working well in our project, and I think we need your confirmation or opinion.

Thank you!!!

fly33499 and others added 27 commits September 9, 2021 10:05
@fly33499
change tag and commit info when starport build
36211f7
@fly33499
Fix tag sort issue by tagger info
64815ac
@fly33499 @Pantani
Update starport/services/chain/chain.go
bff2910 
Commit suggestion

Co-authored-by: Danilo Pantani <[email protected]>
@ilgooz
chore(services/chain): add unit test for determining app version
7015937
@fly33499
Improve tag versioning and commit info
13ed6f4 
Sort tag between annotated and lightweight tag
@fly33499
Merge pull request #1 from tendermint/chore/version-test
e197c1e 
chore(services/chain): add unit test for determining app version
@fly33499
Merge branch 'tendermint:develop' into develop
4f5cd7e
@fly33499
remove v character on tag
bfa89e6
@fly33499
Merge branch 'develop' of https://github.com/FirmaChain/starport into…
b473c6e 
… develop
@fly33499 @ilgooz
Update starport/services/chain/chain_test.go
95bc59e 
Co-authored-by: İlker G. Öztürk <[email protected]>
@fly33499 @ilgooz
Update starport/services/chain/chain_test.go
a934173 
Co-authored-by: İlker G. Öztürk <[email protected]>
@fly33499 @ilgooz
Update starport/services/chain/chain_test.go
693bc0e 
Co-authored-by: İlker G. Öztürk <[email protected]>
@fly33499
Merge branch 'tendermint:develop' into develop
91bdf6f
@fly33499
change tag output without commit count
4f34a7b
@fly33499
Merge branch 'develop' of https://github.com/FirmaChain/starport into…
f00aa17 
… develop
@fly33499
remove test code
1572d1f
@fly33499
update testcase and untar option
f475038
@fly33499
add repoversion pkg to determine version
b26b07f
@fly33499 @ilgooz
Update starport/pkg/repoversion/repoversion.go
c07e161 
Co-authored-by: İlker G. Öztürk <[email protected]>
@fly33499 @ilgooz
Update starport/services/chain/chain_test.go
e3ebfe9 
Co-authored-by: İlker G. Öztürk <[email protected]>
@fly33499
Merge branch 'tendermint:develop' into develop
cea8ffe
@fly33499
Update README.md
eda2c0d 
Added firmachain info
@fly33499
Merge branch 'tendermint:develop' into develop
c0adaaa
@fly33499
Merge branch 'tendermint:develop' into develop
be69f00
@fly33499
Merge branch 'tendermint:develop' into develop
3033666
@fly33499
Merge branch 'tendermint:develop' into develop
eb90664
@fly33499
add cointype support to config.yml
e098e71
@fly33499 fly33499 requested a review from Pantani as a code owner October 9, 2021 14:17
Pantani
Pantani requested changes Oct 9, 2021
View reviewed changes
Copy link
Collaborator

@Pantani Pantani left a comment

Choose a reason for hiding this comment

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

@fly33499 thanks for your contribution 💪🏽.

Nice feature. I think we can avoid set the default coin type (118) if not specified.

@fly33499
Copy link
Contributor Author

fly33499 commented Oct 9, 2021

@fly33499 thanks for your contribution 💪🏽.

Nice feature. I think we can avoid set the default coin type (118) if not specified.

Thank you for great comment. I've learned a lot!!

@barriebyron
Copy link
Contributor

@fly33499 did you know about October Codefest? Complete at least one valid pull request between Oct 1 to Oct 31 and complete the Contributor Details form to get your swag.

@fly33499
Copy link
Contributor Author

@fly33499 did you know about October Codefest? Complete at least one valid pull request between Oct 1 to Oct 31 and complete the Contributor Details form to get your swag.

wow!! it's great! After PR finished, I'm gonna write it soon!
Thanks a lot!

@fly33499 fly33499 requested a review from Pantani October 11, 2021 01:42
@fadeev
Copy link
Contributor

fadeev commented Oct 11, 2021

If cointype is set on a per-account basis (which it looks like it does), then I think this is a good addition.

Pantani
Pantani approved these changes Oct 11, 2021
View reviewed changes
Copy link
Collaborator

@Pantani Pantani left a comment •
edited
Loading

Choose a reason for hiding this comment

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

👌🏽

@fly33499, can you also provide some test commands for all reviewers can test the implementation?

ilgooz
ilgooz approved these changes Oct 12, 2021
View reviewed changes
Copy link
Member

@ilgooz ilgooz left a comment

Choose a reason for hiding this comment

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

Thank you @fly33499 and @youngjoon-lee!

@ilgooz ilgooz merged commit 00adc1e into ignite:develop Oct 12, 2021
@barriebyron
Copy link
Contributor

congratulations @fly33499 you can complete the Contributor Details form to get your October Codefest swag

@barriebyron
Copy link
Contributor

hi @youngjoon-lee I didn't see commits from you, but please do join the October Codefest fun https://blog.cosmos.network/october-codefest-call-for-contributors-42ade37f4913

Jchicode pushed a commit to Jchicode/cli that referenced this pull request Aug 9, 2023
@fly33499 @Pantani @ilgooz
feat(config.yml): add cointype to the config.yml (ignite#1663)
9e17a75 
* change tag and commit info when starport build

* Fix tag sort issue by tagger info

* Update starport/services/chain/chain.go

Commit suggestion

Co-authored-by: Danilo Pantani <[email protected]>

* chore(services/chain): add unit test for determining app version

* Improve tag versioning and commit info
Sort tag between annotated and lightweight tag

* remove v character on tag

* Update starport/services/chain/chain_test.go

Co-authored-by: İlker G. Öztürk <[email protected]>

* Update starport/services/chain/chain_test.go

Co-authored-by: İlker G. Öztürk <[email protected]>

* Update starport/services/chain/chain_test.go

Co-authored-by: İlker G. Öztürk <[email protected]>

* change tag output without commit count

* remove test code

* update testcase and untar option

* add repoversion pkg to determine version

* Update starport/pkg/repoversion/repoversion.go

Co-authored-by: İlker G. Öztürk <[email protected]>

* Update starport/services/chain/chain_test.go

Co-authored-by: İlker G. Öztürk <[email protected]>

* Update README.md

Added firmachain info

* add cointype support to config.yml

* Update starport/chainconfig/config_test.go

Co-authored-by: Danilo Pantani <[email protected]>

* Update starport/pkg/chaincmd/chaincmd.go

Co-authored-by: Danilo Pantani <[email protected]>

* Update starport/pkg/chaincmd/chaincmd.go

Co-authored-by: Danilo Pantani <[email protected]>

* Update starport/pkg/cosmosfaucet/cosmosfaucet.go

Co-authored-by: Danilo Pantani <[email protected]>

Co-authored-by: Danilo Pantani <[email protected]>
Co-authored-by: İlker G. Öztürk <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Pantani Pantani Pantani approved these changes

@ilgooz ilgooz ilgooz approved these changes

@fadeev fadeev Awaiting requested review from fadeev

@lumtis lumtis Awaiting requested review from lumtis

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@fly33499 @barriebyron @fadeev @Pantani @ilgooz