Skip to content
This repository has been archived by the owner on Jul 1, 2021. It is now read-only.

Add partial support for admin_peers JSON-RPC API #1491

Merged
merged 1 commit into from Mar 23, 2020

Conversation

cburgdorf
Copy link
Contributor

@cburgdorf cburgdorf commented Jan 21, 2020

What was wrong?

As part of the things that we want to ship with our first beta, we want to support the admin_peers RPC endpoint.

The response looks something like this in Geth:

{
    "jsonrpc": "2.0",
    "id": 1,
    "result": [
        {
            "enode": "enode://2ea727082ae93766342646f8ac1c98a20a4d14b1c1ca043e1b3c99dfa5a36767692eec2baf7b66474f6bdaca516322fcb45db58173c08a936f6ebab8928d4562@3.1.27.148:30303",
            "id": "e8191234978143d641f1df9898fde0a7a080ef3feb0f6149d857b1fa7d8a4549",
            "name": "Geth/source/linux/go1.10.4",
            "caps": [
                "eth/62",
                "eth/63"
            ],
            "network": {
                "localAddress": "172.17.0.2:56092",
                "remoteAddress": "3.1.27.148:30303",
                "inbound": false,
                "trusted": false,
                "static": false
            },
            "protocols": {
                "eth": {
                    "version": 63,
                    "difficulty": 729514544367193473735,
                    "head": "0x66f682d761aaabe7a98a1bc1b35303ef9c21b1b6c0a35c61965bce13f5499d28"
                }
            }
        },
        {
            "enode": "enode://c25a1d915f9cc87705690bd59af1776cf0ccc3fb7e8dfd31f5f37ef4a8b6a758a743a379bad6f0793c1413b10cdc26aa9aacb5bc1581c4625e22bb414af5840e@139.199.66.86:20182",
            "id": "e81f6a84023ea00c66bfc87857d14080136e91412afe2421816cd4c3be96e686",
            "name": "Geth/qk_client 1.0.1.202 0x4C1a41639D5bf12b8F5D1A83B2aFcbB6100174Eb/v1.9.11-stable-6a62fe39/windows-amd64/go1.13.8",
            "caps": [
                "eth/63",
                "eth/64",
                "eth/65",
                "les/2",
                "les/3"
            ],
            "network": {
                "localAddress": "172.17.0.2:51214",
                "remoteAddress": "139.199.66.86:20182",
                "inbound": false,
                "trusted": false,
                "static": false
            },
            "protocols": {
                "eth": "handshake"
            }
        }
    ]
}

How was it fixed?

Actual Trintiy API response:

{
    "id": 1,
    "jsonrpc": "2.0",
    "result": [
        {
            "enode": "enode://c25a56161448824a805edda3de2ca2de1e5ff498fa0ff6d6bc05939a075805d7217ff791bc003b8fe7cf05d8f1338ac578e1e68dc8e3358f5d27ff78799bcbb8@47.94.236.241:30303",
            "id": "40d4dbe8-675c-48c2-867f-b6a0ea919a94",
            "name": "Geth/v1.9.9-stable-01744997/linux-amd64/go1.13.5",
            "caps": [
                "eth/63",
                "eth/64",
                "shh/6"
            ],
            "network": {
                "localAddress": "0.0.0.0:30303",
                "remoteAddress": "47.94.236.241:30303"
            }
        }
    ]
}

Eagle eyes might spot that this is missing the protocols part which I aim to add in a different PR. The current implementation is based only on the information that we can obtain from the BasePeerPool.

To-Do

  • Add tests
  • Clean up commit history

Cute Animal Picture

put a cute animal picture link inside the parentheses


async def peers(self) -> Sequence[RpcPeerResponse]:

response = await self.event_bus.request(GetConnectedPeersRequest())
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pipermerriam So, our GetConnectedPeersResponse contains SessionAPIs now but all the interesting stuff that geth prints out is sitting on the peer. The obvious answer would be to change to not return a list of SessionAPI but something that includes more information (a subset of the actual peer) but I know you've been really deep into the p2p stack the last months and formed an idea of how these things could work in the future so I'm reaching out to here your thoughts before I just hack myself through until I have the missing data :)

Copy link
Member

Choose a reason for hiding this comment

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

I think it would be very reasonable to have a new object returned that aggregates information about the session like:

class PeerInfo(NamedTuple):
    session: SessionAPI
    capabilities: Capabilities
    ...

@cburgdorf cburgdorf force-pushed the christoph/feat/admin_peers branch 2 times, most recently from 0d451b5 to 1342ed4 Compare January 21, 2020 16:56
@cburgdorf cburgdorf force-pushed the christoph/feat/admin_peers branch 4 times, most recently from 3ddeb9b to 90e6397 Compare March 19, 2020 15:58
@cburgdorf cburgdorf marked this pull request as ready for review March 19, 2020 16:26
@@ -69,10 +72,28 @@ class PeerLeftEvent(BaseEvent):
session: SessionAPI


class PeerInfo(NamedTuple):
session: SessionAPI
capabilities: Capabilities
Copy link
Member

Choose a reason for hiding this comment

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

Lol, it looks like you implemented pretty much the exact thing I typed up 👍

@cburgdorf cburgdorf merged commit ceb7b07 into ethereum:master Mar 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants