-
Notifications
You must be signed in to change notification settings - Fork 2
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: Backport minimum commission to v0.45.0 #2
Conversation
<!-- 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 --> Added the ability to set a minimum commission rate that all validators cannot set their commission rate below. replaces cosmos#10422 --- *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 *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)
Do you happen to know what the liveness test is failing? I don't think this was failing in the previous version, but github deleted the results of that PR's CI run. However, I feel like I've ran into this issue before, but forgot what we did to fix it. If its something silly then we can ignore it, but we should obvi be careful since we did introduce changes that might not be compatible Not sure if we should bother fixing the linter tho, I'll leave it up to you
|
ahh I see
yeah, we can probably ignore that, but we should definitely be very careful, and still test using this branch in a live chain before we can be certain |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should perform further testing to make sure, and would prefer if the linter wasn't failing, but pre-approvingggggg
I just had to add the "feat" prefix to the name of the PR to fix one of the errors. Two of the failed checks "Tests/Code Coverage/test-rosetta" and "Tests/Code Coverage/liveness-test" also failed in the original PR for this feature, so it doesn't seem like a big concern. The last failed check "Protobuf/breakage" only passed in the original PR because there were no logs available, so I'm not sure if that one's actually a problem for us or not. I'll test the code on my own and find out. |
just for clarity for any future readers, like suggested in #2 (comment) this failure is probably ok because the CI not being designed for a github fork. This is also the reason why is fails in other cosmos-sdk forks and before. Thanks for testing to make sure tho!!
We changed the proto files, so this is expected. The entire point of the test is to make sure that reviewers do not miss this. |
backport the minimum commission from #10529 into v0.45.0