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.

Sign up for GitHub

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

Jump to bottom

client: add {node,validator}_public_key field to /status response; deprecated node_key #7828

Merged
merged 5 commits into from
Oct 17, 2022
Merged

client: add {node,validator}_public_key field to /status response; deprecated node_key #7828

merged 5 commits into from
Oct 17, 2022

Conversation

mina86
Copy link
Contributor

@mina86 mina86 commented Oct 13, 2022 •
edited
Loading

Currently, response to the /status request returns a node_key
field. However, the field does not include node key but rather
validator key. Deprecated it in favour of two new fields:
node_public_key and validator_public_key.

Fixes: #7672

@mina86
client: add validator_key field and *real* node key to /status response
f3da8b7 
Currently, response to the /status request returns a `node_key` field.
However, the field does not include node key but rather validator key.
Rename this field to `validator_key`.

Furthermore, reuse `node_key` for the actually node key.

The massive problem with this change is of course that it will
silently break any existing code which thinks node_key in the response
is the validator key.

Fixes: near#7672
@mina86 mina86 requested a review from a team as a code owner October 13, 2022 19:25
@mina86 mina86 requested a review from akhi3030 October 13, 2022 19:25
@mina86
Copy link
Contributor Author

mina86 commented Oct 13, 2022

This is really more of an RFC since I’m not convinced myself that this is a correct way to do this.

@akhi3030
Copy link
Collaborator

Tagging some folks from the node team to weight in on the PR then: @nikurt

@nikurt
Copy link
Contributor

nikurt commented Oct 14, 2022

The change looks good.
Given that we don't know anything about dependencies on the current semantics of node_key, let's be prudent and add new fields before removing/repurposing the old one. Something like node_public_key, validator_public_key should be clear enough.

@mina86 mina86 changed the title client: add validator_key field and *real* node key to /status response client: add {node,validator}_public_key field to /status response; deprecated node_key Oct 14, 2022
@mina86
Copy link
Contributor Author

mina86 commented Oct 17, 2022

@akhi3030 or @nikurt, can I haz approval? ;)

@near-bulldozer near-bulldozer bot merged commit ddd1a67 into near:master Oct 17, 2022
@mina86 mina86 deleted the a branch October 17, 2022 11:32
nikurt pushed a commit that referenced this pull request Oct 17, 2022
@mina86 @nikurt
client: add {node,validator}_public_key field to /status response; de…
50e4cea 
…precated node_key (#7828)

Currently, response to the /status request returns a `node_key`
field. However, the field does not include node key but rather
validator key.  Deprecated it in favour of two new fields:
`node_public_key` and `validator_public_key`.

Fixes: #7672
nikurt pushed a commit that referenced this pull request Nov 9, 2022
@mina86 @nikurt
client: add {node,validator}_public_key field to /status response; de…
3bdd2b9 
…precated node_key (#7828)

Currently, response to the /status request returns a `node_key`
field. However, the field does not include node key but rather
validator key.  Deprecated it in favour of two new fields:
`node_public_key` and `validator_public_key`.

Fixes: #7672
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akhi3030 akhi3030 akhi3030 approved these changes

@nikurt nikurt nikurt approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

node_key missing in /status endpoint for non-validator nodes
3 participants
@mina86 @akhi3030 @nikurt