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

node_key missing in /status endpoint for non-validator nodes #7672

Closed
kucharskim opened this issue Sep 23, 2022 · 5 comments · Fixed by #7828
Closed

node_key missing in /status endpoint for non-validator nodes #7672

kucharskim opened this issue Sep 23, 2022 · 5 comments · Fixed by #7828
Labels
Node Node team T-node Team: issues relevant to the node experience team

Comments

@kucharskim
Copy link

Under /status endpoint, .node_key is missing for an RPC node. At present, from my observation, only Validators have .node_key present. I think all nodes should have that json entry, no matter are they validator, or not.

$ curl -s validator:3030/status | jq -r .version  
{
  "version": "1.29.0",
  "build": "1.29.0",
  "rustc_version": "1.62.1"
}

$ curl -s validator:3030/status | jq -r .node_key
ed25519:5Y9hW8cKBb5RnsJBqttHHC5ujz5zcZZ5xnrJPwkCWmGQ
$ curl -s rpc:3030/status | jq -r .version  
{
  "version": "1.29.0",
  "build": "1.29.0",
  "rustc_version": "1.62.1"
}

$ curl -s rpc:3030/status | jq -r .node_key
null

From above output my expectation from operational perspective would be that all nodes would return public part of node key via curl -s rpc:3030/status | jq -r .node_key however non-validators return null.

@matklad matklad added the T-node Team: issues relevant to the node experience team label Sep 23, 2022
@mina86
Copy link
Contributor

mina86 commented Oct 13, 2022

It looks like, node_key is a misnomer. It shows the validator public key not the node key. I wonder if we should rename the field and if we do how many people’s scripts will break.

@kucharskim
Copy link
Author

Shoot. I didn't even notice. I would change this and drop node_key for 2 mainnet releases and scream out loud on Telegram, Discord, email and release notes that node_key is renamed to validator_key (names based on settings in config.json without _file suffix). Then 2 releases later would introduce node_key back as content of the node key. However the question is then, what do expect people to put into public_addrs - the node key or validator key? I thought all this time that node key was added to /status endpoint because of public_addrs content.

@kucharskim
Copy link
Author

This is super confusing and in long run, as painful as it is, I would break people scripts, but scream loud that this is going to happen.

mina86 added a commit to mina86/nearcore that referenced this issue Oct 13, 2022
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
Copy link
Contributor

mina86 commented Oct 13, 2022

I think node key should go into public_addrs but in reality I think it might not matter. The node will connect, notice node key is different and update its bookkeeping. @pompon0, any thoughts on what to do here?

@kucharskim
Copy link
Author

Then why you created public_addrs in the first place? Keep it simple then and drop the req of node key there.

near-bulldozer bot pushed a commit that referenced this issue Oct 17, 2022
…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 issue Oct 17, 2022
…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 issue Nov 9, 2022
…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
@gmilescu gmilescu added the Node Node team label Oct 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Node Node team T-node Team: issues relevant to the node experience team
Projects
None yet
4 participants