Skip to content

feat(proto)!: replace PubKey in abci.ValidatorUpdate with PubKeyType and PubKeyBytes#2843

Merged
melekes merged 26 commits intomainfrom
marko/val_param
Apr 23, 2024
Merged

feat(proto)!: replace PubKey in abci.ValidatorUpdate with PubKeyType and PubKeyBytes#2843
melekes merged 26 commits intomainfrom
marko/val_param

Conversation

@melekes
Copy link
Collaborator

@melekes melekes commented Apr 18, 2024

This PR deprecates PubKey in abci.ValidatorUpdate and adds two new fields:

  • string PubKeyType
  • bytes PubKeyBytes

Co-authored by: @tac0turtle


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 a review from a team as a code owner April 18, 2024 08:17
@melekes melekes requested a review from a team April 18, 2024 08:17
@melekes melekes self-assigned this Apr 18, 2024
@hvanz
Copy link
Collaborator

hvanz commented Apr 18, 2024

The spec needs to be updated. For instance, here https://github.com/cometbft/cometbft/blob/main/spec/abci/abci%2B%2B_methods.md#validatorupdate

@adizere adizere added this to the 2024-Q2 milestone Apr 18, 2024
@melekes melekes requested a review from a team as a code owner April 18, 2024 08:45
@melekes melekes marked this pull request as draft April 18, 2024 08:46
Copy link
Collaborator

@andynog andynog left a comment

Choose a reason for hiding this comment

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

lgtm, I don't have deep expertise in the pubkey usage but made a couple of comments on things that jump into my head while understanding the logic a bit better. Feel free to disregard if it doesn't make sense.

Copy link
Collaborator

@andynog andynog left a comment

Choose a reason for hiding this comment

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

lgtm, thanks for adding the additional logic @melekes

Copy link

@cason cason left a comment

Choose a reason for hiding this comment

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

Not a part of code I know, but no restrictions.

@adizere adizere linked an issue Apr 22, 2024 that may be closed by this pull request
@adizere adizere removed this from the 2024-Q2 milestone Apr 22, 2024
@adizere adizere added this to the 2024-Q2 milestone Apr 22, 2024
Copy link
Collaborator

@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.

🚀

@melekes melekes added this pull request to the merge queue Apr 23, 2024
Merged via the queue into main with commit 71cd087 Apr 23, 2024
@melekes melekes deleted the marko/val_param branch April 23, 2024 16:41
mergify bot pushed a commit that referenced this pull request Apr 23, 2024
…Type` and `PubKeyBytes` (#2843)

This PR deprecates `PubKey` in `abci.ValidatorUpdate` and adds two new
fields:

- string `PubKeyType`
- bytes `PubKeyBytes`

Co-authored by: @tac0turtle

---

#### PR checklist

- [x] Tests written/updated
- [x] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [x] 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: Marko Baricevic <markobaricevic3778@gmail.com>
Co-authored-by: Andy Nogueira <me@andynogueira.dev>
(cherry picked from commit 71cd087)

# Conflicts:
#	abci/example/kvstore/kvstore.go
melekes added a commit that referenced this pull request Apr 24, 2024
…Type` and `PubKeyBytes` (backport #2843) (#2879)

This PR deprecates `PubKey` in `abci.ValidatorUpdate` and adds two new
fields:

- string `PubKeyType`
- bytes `PubKeyBytes`

Co-authored by: @tac0turtle 

---

#### PR checklist

- [x] Tests written/updated
- [x] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [x] 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 #2843 done by
[Mergify](https://mergify.com).

---------

Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>
github-merge-queue 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>
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

None yet

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

7 participants