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

Add RPC method for extended information on connected peers #3480

Closed
EdgeDLT opened this issue Jun 6, 2024 · 6 comments · Fixed by #3481
Closed

Add RPC method for extended information on connected peers #3480

EdgeDLT opened this issue Jun 6, 2024 · 6 comments · Fixed by #3481
Labels
feature Completely new functionality I2 Regular impact S4 Routine U4 Nothing urgent
Milestone

Comments

@EdgeDLT
Copy link
Contributor

EdgeDLT commented Jun 6, 2024

Is your feature request related to a problem? Please describe.

I want to be able to quickly query my node for the network state (see last height of peers and their user agents). I've called it getPeerHeights because that's the main thing I want to check but maybe getConnectedPeerInfo or getNetworkState etc is better.

Describe the solution you'd like

Example usage:

{
  "jsonrpc": "2.0",
  "method": "getpeerheights",
  "params": [],
  "id": 1
}

Example response:

{
  "id": 1,
  "jsonrpc": "2.0",
  "result": {
    "peerHeights": [
      {
        "address": "34.16.114.243:20333",
        "userAgent": "/Neo:3.7.4+7f227a3026fadeb9723280e6f961b6f29b0f8a7a/",
        "height": 4104904
      },
      {
        "address": "159.203.28.78:21333",
        "userAgent": "/NEO-GO:0.106.1/",
        "height": 4104904
      }
    ]
  }
}

Describe alternatives you've considered

RPC extension seemed sensible enough.

Additional context

I have already implemented this method to meet my current needs. If consensus is that its useful, I am happy to send a PR for review.

@EdgeDLT EdgeDLT added feature Completely new functionality I2 Regular impact labels Jun 6, 2024
@AnnaShaleva
Copy link
Member

I think this extension is quite a useful one since getpeers doesn’t give us anything except addresses. Vote up from my side for PR.

@roman-khimov
Copy link
Member

How do you get this data? To me it requires P2P ping to every neighbor and thus this RPC can't be safely exposed to everyone.

Refs. neo-project/neo#1844.

@EdgeDLT
Copy link
Contributor Author

EdgeDLT commented Jun 6, 2024

@roman-khimov I use LastBlockIndex() and Version().UserAgent on Peer. Admittedly I don't know how often this is updated, but I was thinking it would be sufficient to detect stuck-but-live nodes.

@ixje
Copy link
Contributor

ixje commented Jun 12, 2024

This sounds like a more useful alternative to the getpeers RPC method. Perhaps including useragent and height for connected nodes is a worthy update to that method instead (then neo-cli users can also benefit from it)

@roman-khimov
Copy link
Member

I had this thought as well, but current getpeers is symmetric for all types of nodes. Maybe it's not a problem though.

Semantically this height is more like lastknownheight I should also add.

@ixje
Copy link
Contributor

ixje commented Jun 13, 2024

I agree if we only add the fields to connected it sacrifices the symmetry. We could use something like useragent: "unknown" and height: -1 for unconnected/bad. I just feel that the current getpeers output is so limited that likely nobody is consuming it. But you might have some stats on that from your nodes.

Semantically this height is more like lastknownheight I should also add.

Sure, depending on how its implemented it's a "best effort". Even what's returned in the VERSION payload during the P2P handshake can be outdated by the time you're connected. I don't think it has to be 100% accurate to be useful though.

@AnnaShaleva AnnaShaleva added this to the v0.106.3 milestone Jun 27, 2024
@AnnaShaleva AnnaShaleva added U4 Nothing urgent S4 Routine labels Jun 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Completely new functionality I2 Regular impact S4 Routine U4 Nothing urgent
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants