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: Backport minimum commission to v0.45.0 #2

Merged
merged 3 commits into from Mar 2, 2022

Conversation

lydia-pierce
Copy link
Collaborator

@lydia-pierce lydia-pierce commented Mar 1, 2022

backport the minimum commission from #10529 into v0.45.0

tac0turtle and others added 2 commits March 1, 2022 13:03
<!--
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)
@lydia-pierce lydia-pierce changed the title Lydia/backport min commish Backport minimum commission to v0.45.0 Mar 1, 2022
@evan-forbes
Copy link
Collaborator

evan-forbes commented Mar 1, 2022

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

No release type found in pull request title "Backport minimum commission to v0.45.0". Add a prefix to indicate what kind of release this pull request corresponds to (see https://www.conventionalcommits.org/).

@evan-forbes
Copy link
Collaborator

evan-forbes commented Mar 1, 2022

ahh I see

Step 5/18 : COPY db/go.mod db/go.sum /work/db/
COPY failed: file not found in build context or excluded by .dockerignore: stat db/go.mod: file does not exist

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

@evan-forbes evan-forbes removed their assignment Mar 1, 2022
@evan-forbes evan-forbes self-requested a review March 1, 2022 22:33
Copy link
Collaborator

@evan-forbes evan-forbes left a comment

Choose a reason for hiding this comment

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

:shipit:

we should perform further testing to make sure, and would prefer if the linter wasn't failing, but pre-approvingggggg

@lydia-pierce lydia-pierce changed the title Backport minimum commission to v0.45.0 feat: Backport minimum commission to v0.45.0 Mar 2, 2022
@lydia-pierce
Copy link
Collaborator Author

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.

@evan-forbes
Copy link
Collaborator

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.

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!!

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.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants