Skip to content

feat(rest): expose peer score and disconnect reason on /eth/v1/node/peers - #8449

Draft
barnabasbusa wants to merge 3 commits into
status-im:unstablefrom
barnabasbusa:bbusa/peer-scores-beacon-api
Draft

feat(rest): expose peer score and disconnect reason on /eth/v1/node/peers#8449
barnabasbusa wants to merge 3 commits into
status-im:unstablefrom
barnabasbusa:bbusa/peer-scores-beacon-api

Conversation

@barnabasbusa

Copy link
Copy Markdown
Contributor

Summary

Extends /eth/v1/node/peers (and /eth/v1/node/peers/{peer_id}) with two optional fields per a simplified beacon-API spec extension currently under discussion:

  • score — from Peer.score: int (Nimbus's application-level reputation score, [0, 1000] with default 300)
  • disconnect_reason — from a new lastDisconnectReason field on Peer, mapped to the spec controlled vocab

agent_version is already exposed in Nimbus's current response (under field name agent).

downscore_reasons is deliberately omitted — Nimbus doesn't currently track per-event reason history. Adding it would require instrumenting the ~30 call sites that adjust Peer.score. Happy to follow up if there's interest.

Spec proposal

ethereum/beacon-APIs#606

What's in this PR

Commit 1: expose score

  • beacon_chain/spec/eth2_apis/rest_types.nim — add score*: Opt[int] to RestNodePeer.
  • beacon_chain/rpc/rest_node_api.nim — populate from peer.score in both the list and single-peer handlers.

Commit 2: expose disconnect_reason

  • beacon_chain/networking/eth2_network.nim — add lastDisconnectReason*: Opt[DisconnectionReason] + lastDisconnectAt*: Opt[chronos.Moment] to the Peer object. Instrument proc disconnect* at the top, so all ~20 disconnect call sites are caught at the single choke point.
  • beacon_chain/spec/eth2_apis/rest_types.nim — add disconnect_reason*: Opt[string] to RestNodePeer.
  • beacon_chain/rpc/rest_node_api.nim — add mapDisconnectReason() translating Nimbus's 4-value DisconnectionReason enum to the spec controlled vocab (ClientShutDown → client_shutdown, IrrelevantNetwork → irrelevant_network, FaultOrError → io_error, PeerScoreLow → bad_score).

Coordinated implementations

Part of a coordinated multi-client effort — see ethereum/beacon-APIs#606 for the other five client PRs.

Status

Draft. nim check on all three patched files: identical error counts vs. pre-patch baseline (only pre-existing version.nim / forks.nim issues, none introduced by this patch).

Adds an optional `score` field to the `RestNodePeer` payload returned by
`GET /eth/v1/node/peers` and `GET /eth/v1/node/peers/{peer_id}`, populated
from `Peer.score` (Nimbus's internal application-level peer score).

This field is not part of the beacon-API specification and is emitted
alongside the existing `agent`/`proto` extensions. It is marked
`Opt[int]` so JSON output remains backwards-compatible (omitted when not
set), in line with `omitOptionalFields = true` for `RestJson`.
Extends the `RestNodePeer` payload returned by `GET /eth/v1/node/peers`
and `GET /eth/v1/node/peers/{peer_id}` with an optional
`disconnect_reason` field. Population is driven by two new fields on
the `Peer` object - `lastDisconnectReason` and `lastDisconnectAt` -
that are written inside `Peer.disconnect()`. Centralising the write at
the disconnect choke point captures every existing call site (~20)
without per-site edits.

`mapDisconnectReason` translates Nimbus's internal `DisconnectionReason`
enum into a small controlled vocabulary aimed at cross-client
consumption:
  ClientShutDown      -> client_shutdown
  IrrelevantNetwork   -> irrelevant_network
  FaultOrError        -> io_error
  PeerScoreLow        -> bad_score

The field is `Opt[string]` so JSON output stays backwards compatible
(omitted while never set) under `omitOptionalFields = true` for
`RestJson`.
Per the proposed beacon-API spec
(ethereum/beacon-APIs#606), `disconnect_reason`
MUST only be populated when the peer's `state` is `disconnected` or
`disconnecting`. Gate the `peer.lastDisconnectReason` lookup in both
the list and single-peer handlers on `peer.connectionState` so the
field is omitted (Opt.none) for connected/connecting peers.
@github-actions

Copy link
Copy Markdown

Unit Test Results

       12 files  ±0    2 840 suites  ±0   1h 11m 38s ⏱️ - 5m 49s
15 986 tests ±0  14 421 ✔️ ±0  1 565 💤 ±0  0 ±0 
76 904 runs  ±0  75 164 ✔️ ±0  1 740 💤 ±0  0 ±0 

Results for commit 416f44e. ± Comparison against base commit ed74c55.

of PeerType.Outgoing:
"outbound"

proc mapDisconnectReason(reason: DisconnectionReason): string =

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

func

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants