Repository navigation
feat(discovery): report libp2p connections so discv5 paces its lookups - #605
Draft
MegaRedHand wants to merge 7 commits into
Draft
MegaRedHand wants to merge 7 commits into
MegaRedHand wants to merge 7 commits into
Conversation
ethrex used to pace its discv5 lookups off its peer table's connected count, which only RLPx could fill. A libp2p client registered nothing, so the count was permanently 0, completion was permanently 0, and lookups never eased off the 500ms startup rate: roughly 20x the intended steady-state FindNode traffic, for the life of the process. ethrex PR #7217 moves the contact table into the discovery server and gives a consumer `DiscoveryHandle::record_peer_event`, so any consumer can report its own connections whatever transport they run over. Pin the three ethrex crates to that PR's head and report every libp2p connection opened and closed. `--discovery.target-peers` now feeds the pacing it was deliberately kept out of, and `PEER_TABLE_TARGET_PEERS`, the placeholder that only existed to keep the old division off `NaN`, is gone. Reporting needs the discv5 node id behind a libp2p `PeerId`. Both names derive from the same secp256k1 key, and libp2p inlines a key that short into the peer id, so it is recoverable: `node_id` does that round trip, and pins it against `Node::from_enr` so the two halves cannot drift. The peer table is gone from this crate with it. Two consequences: - The filter is what discovery now takes, and `next_dial_candidate` hands back a `Node` rather than a `Contact`, so the ENR the dial loop used to read is no longer in reach. Judging a record is the one moment we hold it, so `LeanFilter` files each admitted peer's multiaddr and `attnets` in `DialTargets` as it goes, and the dial loop looks the node id back up there. Discovery still decides *whether* to dial; we only supply *how*. - `--discovery.advertise-ip` maps onto ethrex's new `nat_extip_set`, so an operator who names an external address keeps it: discv5's PONG voting no longer overwrites what was explicitly asked for. The pin is a commit rather than a branch, for the reason the release tag it replaces was chosen over `main`. It goes back to a tag once #7217 is in a release.
Brings in the Beacon Chain support (#661). Main moved the dial loop's peer-table draws onto a bounded contact-poll task, so that task now asks the discovery handle for nodes and looks their dial targets up instead. The filter takes main's fork id, and #605's node-id recovery from a PeerId reuses the decode main added for custody rather than a second copy of it. Cargo.lock is main's with only the ethrex sources moved to the pinned revision.
This was referenced Oct 29, 2026
Main's fork-digest rollover (#639) needs a handle on the admission policy after discovery has taken its copy, which this branch had made impossible by moving a non-Clone LeanFilter into the discovery server. LeanFilter is Clone again: every clone shares one Policy (what set_fork_id moves) and one DialTargets map (what accepts files into), so discovery runs one copy and the dial loop keeps the other. SpawnedDiscovery carries that clone in place of the bare DialTargets handle, and the self-admission test reads it rather than rebuilding the filter. update_served_enr's doc described ethrex v25.0.0's DiscoveryServer::spawn, which returned (). At this branch's ethrex pin it returns a handle, but none of its messages replaces the record's eth2 entry, so the gap it reports stands for a different reason; the doc now says which.
MegaRedHand
added a commit
that referenced
this pull request
Nov 4, 2026
…queries Brings in main through #605's branch, which now carries it. discovery/{admission,dial,mod}.rs: this branch's side. #605's update resolved main's fork-boundary admission the same way this branch already had, and the doc and test edits it added there are superseded by this branch's own (the live ENR, the update_served_enr text for its ethrex pin). main.rs, p2p lib.rs, docs/rpc.md: /eth/v1/node/identity takes both changes, this branch's live ENR source and main's MetaData (#727), so P2P keeps the LocalEnr handle beside the MetaDataController. Cargo.lock: merged per package against main; main's bumps are kept wherever this branch left a package alone, and this branch's ethrex pin keeps its own picks.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Draft: pins ethrex to the head of an open PR (lambdaclass/ethrex#7217,
4462253ea). Not mergeable until that lands and we can move back to a release tag.Motivation
ethrex paces its discv5 iterative lookups by how far along its consumer is: one lookup every 500ms at startup, easing to one every 10s at the target peer count. That completion figure came from the peer table's connected map, which only an RLPx
PeerConnectioncould fill. ethlambda connects over libp2p and registered nothing, so the count was permanently 0, completion was permanently 0, and the pacing never left the startup rate. A lean node kept looking up every 500ms rather than settling at 10s, roughly 20x the intended steady-stateFindNodetraffic, for the life of the process.It was worse than an oversight: the target had to be lied about.
peer_completiondivided by it, sotarget_peers: 0yielded0/0 = NaN, which saturates to a zero-delay re-fire.docs/discovery.mdcarried the whole thing as a known limitation, andPEER_TABLE_TARGET_PEERSexisted only to keep that division offNaN.ethrex #7217 closes it upstream. Discovery now owns its contacts as plain state and keeps its own connected set, fed by
DiscoveryHandle::record_peer_event(node_id, PeerEvent). Any consumer can report its own connections, whatever transport they run over.What this does
Pins
ethrex-p2p/-rlp/-commonto that PR's head, and reports every libp2p connection opened and closed:--discovery.target-peersnow feeds the pacing it was deliberately kept out of, so the flag means one thing in both places: the dial loop's cutoff, and how hard discovery looks.PEER_TABLE_TARGET_PEERSis gone, and so is the known-limitation section that documented it.Naming the same peer twice
Reporting needs the discv5 node id, and the swarm only ever hands us a libp2p
PeerId. Both derive from one secp256k1 key: the node id is keccak256 of the uncompressed point, the peer id a multihash of the protobuf encoding. libp2p inlines any key encoding short enough, and secp256k1 always is, so the key is recoverable from the peer id.discovery/node_id.rsdoes that round trip in both directions.That derivation covers inbound peers too, which matters: a peer we never discovered still counts towards how hard we should be looking, and undercounting is exactly the bug being fixed. A peer id carrying something other than secp256k1 answers
Noneand is simply not reported.The derivation is pinned against ethrex's own (
Node::from_enr(&record).node_id()) in a test, because a silent disagreement would make every dial-target lookup miss and the dial loop would draw candidates forever and dial none.What the peer table's removal forced
The peer table is gone from this crate, which took two things with it.
The ENR is no longer in reach at dial time.
get_contact_to_initiatereturned aContactcarrying the record;next_dial_candidatereturns aNode, which knows an address and a public key and nothing aboutquicorattnets. Judging a record is now the one moment ethlambda holds it, soLeanFilterfiles each admitted peer's multiaddr and subnets inDialTargetsas it goes, and the dial loop looks the node id back up there.The division of labour is unchanged in substance: discovery decides whether to dial (knows us, already tried, already connected, passed the filter), we supply how. A lookup miss is ordinary rather than a surprise, and means the same thing the old
contact.record.is_none()did: a bootnode known only as a bare endpoint.DialTargetsis capped. Only records passing admission land in it, so on any real network it is the size of the lean population, but minting ENRs costs only a signature and an unbounded map would grow with however many an adversary gossips. The cap matches ethrex's own connection-pool bound. A record re-judged and rejected at a higherseqis dropped from the map, so a peer that leaves our network does not leave a dialable address behind.nat_extip_setis new onDiscoveryConfig, and--discovery.advertise-ipis exactly what it describes: an operator naming the address peers should reach this node on. Wired toadvertise_ip.is_some(), so discv5's PONG-based IP voting no longer overwrites an address that was explicitly asked for. Voting still resolves the address when the flag is unset. This is a deliberate behaviour change, called out indocs/discovery.md.Also required by the newer
spawnsignature, both inert here:shared_local_node, ethrex's live identity mirror, which nothing on this side reads yet; and afork_idwatch channel whose sender is dropped immediately, since lean has no fork schedule and its digest lives ineth2rather thaneth.Testing
make fmt,make lintandmake testclean: 30 test binaries, no failures, clippy at-D warnings.cargo check --workspace --all-targets --lockedpasses, so the lock is what CI will see; the ethrex bump did not reopen the two-libssz split.New tests: the
PeerIdto node id round trip and both of itsNonearms; the node id agreeing withNode::from_enr; and a peer that republishes onto another network losing its filed dial target. That last one was checked against a mutant of its own fix rather than assumed to fail (theforgetcall disabled: it fails, and only it).Not covered by a test: the two
record_peer_eventcasts themselves, which would want a running discovery server and aP2PServertogether. The part that could break silently is the node id derivation, and that is pinned.Follow-ups
shared_local_nodeis a live view of the ENR after IP voting re-signs it.SpawnedDiscovery::local_enris still a startup snapshot, with a doc comment admitting it lags. Wiring the two together is now a small change, deliberately not made here.