Skip to content

feat(p2p): add opt-in discv5 peer discovery - #579

Draft
MegaRedHand wants to merge 1 commit into
mainfrom
feat/discv5-discovery
Draft

feat(p2p): add opt-in discv5 peer discovery#579
MegaRedHand wants to merge 1 commit into
mainfrom
feat/discv5-discovery

Conversation

@MegaRedHand

Copy link
Copy Markdown
Collaborator

What

Adds opt-in discv5 peer discovery, so a lean node can find peers instead of
being handed them. Off by default; --discovery.enable turns it on and
--discovery.port gives it its own UDP socket (it must differ from
--gossipsub-port, and the node refuses to start otherwise rather than
failing later with an opaque EADDRINUSE). Static bootnode dialing is
untouched.

Built on ethrex's discovery stack: DiscoveryServer runs discv5-only and
writes what it finds into a PeerTable, which P2PServer polls, filters and
dials over libp2p QUIC.

How peers are judged

Admission follows the beacon phase0 p2p spec, mirroring lighthouse's
eth2_fork_predicate:

Check Rule
eth2 entry must be present and decode
fork_digest must equal ours
next_fork_version / next_fork_epoch may differ (the spec's MAY)
quic port required, and non-zero
secp256k1, ip/ip6 required to derive a dialable target

These live in a LeanFilter handed to the peer table as its PeerFilter, so
each record is judged the moment it arrives rather than at dial time. No
rejection is final: the peer table re-runs the filter as soon as the peer
publishes a higher-seq ENR, so a node that adds a quic entry or gains an
address through discv5's IP voting is reconsidered without a restart.

Admitted peers are ranked by how many attestation subnets they advertise that
no connected peer covers, so discovery fills coverage gaps first. attnets is
self-reported and unauthenticated, so subnet ids at or beyond the local
committee count are dropped before ranking sees them: otherwise an ENR padding
its bitfield with a few hundred bytes of 0xFF would outrank every honest peer
forever.

Also here

  • --discovery.advertise-ip separates the bound address from the advertised
    one, for a node behind NAT or on a host whose public IP is not what it binds.
  • GET /lean/v0/node/identity reports the local ENR alongside the peer id,
    grouped into a NodeIdentity struct.
  • A bootnode entry no longer needs a quic port: one with only a udp entry
    is kept as a discv5 seed even though it cannot be dialed over libp2p.
  • docs/discovery.md covers the ENR layout, the admission rules, the operator
    flags and the known limitations.

Dependency

ethrex-p2p is pinned to the unmerged feat/discovery-peer-requirements
branch, which carries the unified DiscoveryServer, the peer table and the
PeerFilter seam. Cargo.lock pins the exact commit, so builds are
reproducible. This should be repointed at a main revision before merge.

Note that ethrex still uses libssz 0.2.2 while ethlambda is on 0.3.0, so the
dependency graph now carries both. Nothing SSZ-typed crosses the boundary
(lean's EnrForkId is its own type), but it is worth knowing.

Known limitation: discovery is one-sided

DiscoveryServer::spawn builds its own local record from the ethrex Node and
offers no way to seed the consensus entries, so the ENR ethrex answers queries
with carries ip, udp and secp256k1 but not eth2, attnets or
quic. The ENR this node reports is complete; the one it serves is not.

We therefore find and admit lean peers, but a lean peer applying these same
rules to what ethrex serves would refuse us for a missing quic entry. Closing
this needs a way to hand spawn a prepared record.

Separately, lean's fork_digest is the hardcoded cross-client dummy
0x12345678, so the eth2 check separates lean from non-lean but not one
lean devnet from another
. Two devnets running this code will peer with each
other.

Testing

  • make lint clean.
  • cargo test --workspace --profile release-fast --no-fail-fast: 601 passed,
    0 failed, including the forkchoice, signature, STF and SSZ spec tests.
  • Unit coverage for the ENR round trip, every admission rejection reason, the
    oversized-attnets ranking attack, subnet ranking, and spawn_discovery
    binding a real socket (including --discovery.advertise-ip and a busy port).

Draft because the ethrex dependency is still an unmerged branch.

Lean nodes could only meet through a static bootnode list, so every new
node needed an operator to hand it peers. This wires ethrex's discv5
stack in behind `--discovery.enable`: the node builds and signs its own
ENR, joins the DHT on its own UDP socket, and dials what it finds over
libp2p QUIC. Static bootnode dialing is untouched and discovery is off by
default, so nothing changes for an operator who does not ask for it.

Admission follows the beacon phase0 p2p spec, mirroring lighthouse's
`eth2_fork_predicate`: the `eth2` fork digest must match, a differing
`next_fork_version`/`next_fork_epoch` is explicitly tolerated, and the
peer must advertise a `quic` port. The checks live in a `LeanFilter` that
ethrex's peer table runs as each ENR arrives, so a record is judged where
it lands rather than at dial time, and is judged afresh whenever the peer
publishes a higher-`seq` record. Survivors are ranked by how many
attestation subnets they cover that no connected peer does, so discovery
fills subnet gaps first. A peer's `attnets` is self-reported, so subnet
ids at or beyond the local committee count are dropped before ranking
sees them.

`ethrex-p2p` is pinned to the unmerged `feat/discovery-peer-requirements`
branch, which carries the unified `DiscoveryServer`, the peer table, and
the `PeerFilter` seam. Repoint it at a main revision once that merges.

Known gap: `DiscoveryServer::spawn` builds its own local record and
offers no way to seed the consensus entries, so the ENR ethrex answers
queries with carries `ip`/`udp`/`secp256k1` but not `eth2`, `attnets` or
`quic`. Discovery is one-sided until `spawn` can take a prepared record:
we find and admit lean peers, but a lean peer applying these same rules
to what ethrex serves would refuse us. See `docs/discovery.md`.
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.

1 participant