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(proto)!: replace pub_key with pub_key_type #2890

Merged
merged 6 commits into from
Apr 29, 2024
Merged

Conversation

melekes
Copy link
Contributor

@melekes melekes commented Apr 25, 2024

and pub_key_bytes in privval.PubKeyResponse, types.Validator and types.SimpleValidator.

Closes #2878
Follow-up to #2843


PR checklist

  • Tests written/updated
  • Changelog entry added in .changelog (we use unclog to manage our changelog)
  • Updated relevant documentation (docs/ or spec/) and code comments
  • Title follows the Conventional Commits spec

@melekes melekes requested review from a team as code owners April 25, 2024 06:15
and `pub_key_bytes` in `privval.PubKeyResponse`, `types.Validator` and
`types.SimpleValidator`.

Closes #2878
@melekes melekes added the backport-to-v1.x Tell Mergify to backport the PR to v1.x label Apr 25, 2024
@melekes melekes self-assigned this Apr 25, 2024
@melekes melekes added the protobuf Protocol buffers-related label Apr 25, 2024
@@ -20,8 +19,11 @@ message PubKeyRequest {

// PubKeyResponse is a response message containing the public key.
message PubKeyResponse {
cometbft.crypto.v1.PublicKey pub_key = 1 [(gogoproto.nullable) = false];
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to discuss how we'll be managing the proto versions from now on... the v1 proto versions match CometBFT v1.0.0, but now these changes (and other we're doing on main) should trigger a bump in the proto version, I think.
TBH, I haven't given it a deep thought... it would probably be worth discussing in our team meeting.
This shouldn't be blocking this PR, BTW.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since we're backporting this to v1, no version bump is needed. That was my thought, at least.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, of course... spot on!

Copy link
Contributor

@sergio-mena sergio-mena left a comment

Choose a reason for hiding this comment

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

Really cool! 🎸

@melekes melekes enabled auto-merge April 29, 2024 05:42
@melekes melekes added this pull request to the merge queue Apr 29, 2024
Merged via the queue into main with commit d342b3a Apr 29, 2024
39 checks passed
@melekes melekes deleted the 2878-remove-pubkey-all branch April 29, 2024 05:53
mergify bot pushed a commit that referenced this pull request Apr 29, 2024
and `pub_key_bytes` in `privval.PubKeyResponse`, `types.Validator` and
`types.SimpleValidator`.

Closes #2878
Follow-up to #2843
<!--

Please add a reference to the issue that this PR addresses and indicate
which
files are most critical to review. If it fully addresses a particular
issue,
please include "Closes #XXX" (where "XXX" is the issue number).

If this PR is non-trivial/large/complex, please ensure that you have
either
created an issue that the team's had a chance to respond to, or had some
discussion with the team prior to submitting substantial pull requests.
The team
can be reached via GitHub Discussions or the Cosmos Network Discord
server in
the #cometbft channel. GitHub Discussions is preferred over Discord as
it
allows us to keep track of conversations topically.
https://github.com/cometbft/cometbft/discussions

If the work in this PR is not aligned with the team's current
priorities, please
be advised that it may take some time before it is merged - especially
if it has
not yet been discussed with the team.

See the project board for the team's current priorities:
https://github.com/orgs/cometbft/projects/1

-->

---

#### PR checklist

- [x] Tests written/updated
- [x] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [ ] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [x] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec

---------

Co-authored-by: Sergio Mena <sergio@informal.systems>
(cherry picked from commit d342b3a)
melekes added a commit that referenced this pull request Apr 29, 2024
…2922)

and `pub_key_bytes` in `privval.PubKeyResponse`, `types.Validator` and
`types.SimpleValidator`.

Closes #2878
Follow-up to #2843


---

#### PR checklist

- [x] Tests written/updated
- [x] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [ ] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [x] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec
<hr>This is an automatic backport of pull request #2890 done by
[Mergify](https://mergify.com).

Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-to-v1.x Tell Mergify to backport the PR to v1.x protobuf Protocol buffers-related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

proto: completely remove cometbft.crypto.v1.PublicKey
2 participants