-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Invalid gentx causes hard-to-diagnose errors #9304
Comments
Hmmm looks like there isn't a call to |
Note that calling ValidateBasic will only catch some errors. The validate-genesis call is stateless and can't catch everything. |
This is my thought as well. We should call |
validate-genesis
Then would my other suggestion be useful?
It is such a pain when in a rush to produce a genesis to have BOTH AND running Can we address the second part? Why does |
May be we can improve the panic message and include gentx metadata as an easy fix. |
yeah this has been a long standing issue. During the Cosmos Hub launch, Chorus One wrote a github got to validate the gentxs. I don't think they open sourced it. |
We are using a GitHub action recently to validate gentxs on PR submission. Here's the CI job we used for regen mainnet gentx validation recently: https://github.com/regen-network/mainnet/blob/main/.github/workflows/validate-gentx.yml May be we can provide a simple shell script on the SDK to validate all the gentxs from a dir. |
Thanks for the pointer! OMG, I was hoping there was a cleaner way to do it, but looks like there isn't. Your script is just a refinement of the basic idea I used under pressure (try starting a chain with just one gentx at a time and see if it dies right away).
That would be useful! I may be able to help do this, starting from your script and making it work for Agoric as well. I don't have time immediately, but it is on the critical path for our next testnet phase, so I'll be looking into it before then. |
Hi @michaelfig - could you confirm if you are working on a PR? do you need support? |
Thanks for asking. I won't be able to work on this for another few weeks. |
<!-- The default pull request template is for types feat, fix, or refactor. For other templates, add one of the following parameters to the url: - template=docs.md - template=other.md --> ## Description ref: #9304 <!-- Add a description of the changes that this PR introduces and the files that are the most critical to review. --> --- ### Author Checklist *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [x] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [x] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [x] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [x] updated the relevant documentation or specification - [x] reviewed "Files changed" and left comments if necessary - [x] confirmed all CI checks have passed ### Reviewers Checklist *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable)
@anilcse wrote a PR here #9486 which adds a bash script to validate gentx. It's on master now. It was proposed as an alternative solution to look into using Go code to validate gentxs in the |
@anilcse any news on this? It would be nice to have the Go code do the validation, or at least produce understandable errors on chain start. Ideally, we could leverage the |
@anilcse are you still interested in putting these validations into Go code, or should we keep your script? |
## Description Closes: #9304 --- ### Author Checklist *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [ ] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed ### Reviewers Checklist *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable)
Summary of Bug
When launching a testnet chain with collected gentxs, I found that some of them had invalid parameters. However
validate-genesis
did not detect these errors. They were only detected when launching a node based on the incorrect genesis.json.It was also very inconvenient to narrow-down on the specific transactions that failed; none of their metadata was present in the node's error message.
These look like systemic problems, and likely a solution so that either errors are not fatal or are reported verbosely with
validate-genesis
would solve all of them.Version
v0.42.4
Steps to Reproduce
ag-cosmos-helper
is myappd
.ag-cosmos-helper --home=/tmp/t init my-moniker
ag-cosmos-helper --home=/tmp/t add-genesis-account foo 1000000ustake
a. either: use a different chain-id than the initialized one:
ag-cosmos-helper --home=/tmp/t gentx foo 1000000ustake --chain-id=something-else
b. or: use commission of 1.0:
ag-cosmos-helper --home=/tmp/t gentx foo 1000000ustake --chain-id=test-chain-Rvhym1 --commission-rate=1.0
ag-cosmos-helper --home=/tmp/t collect-gentxs
ag-cosmos-helper --home=/tmp/t validate-genesis
ag-cosmos-helper --home=/tmp/t start
Panic with 3.a.
Panic with 3.b.
Thanks!
For Admin Use
The text was updated successfully, but these errors were encountered: