Skip to content

feat(discovery): hunt attestation subnet peers with discv5 predicate queries - #732

Draft
MegaRedHand wants to merge 10 commits into
mainfrom
feat/discv5-predicate-queries
Draft

MegaRedHand wants to merge 10 commits into
mainfrom
feat/discv5-predicate-queries

Conversation

@MegaRedHand

Copy link
Copy Markdown
Collaborator

Draft: pins ethrex to 8f847524, the head of ethrex's feat/discv5-predicate-queries. That branch has no ethrex PR yet and is built on the open lambdaclass/ethrex#7217. Not mergeable until both are in an ethrex release and the pin goes back to a tag.

Includes #605. This branch merges feat/ethrex-discovery-contact-table (#605), since predicate queries are built on the contact-table API that PR migrates to. #605 can be closed in favor of this one, or merged first; the diff here then shrinks to the last three commits.

Motivation

The dial loop ranks what discv5's crawl happens to find, which only reorders random draws. About one mainnet record in 32 advertises a given attestation subnet, so a subnet this node subscribes to can stay without peers for as long as the crawl takes to stumble on one. ethrex's new DiscoveryHandle::find_nodes and records_matching let the search be aimed instead, the way Lighthouse uses sigp/discv5's find_node_predicate.

What this does

1. ethrex pin bump

v25.0.0-based #7217 head (4462253e) to 8f847524 (v30.0.1 + the branch). Builds with no API changes: the contact-table API is the same. The lockfile moves only ethrex crates; make cooldown-check passes.

2. Subnet hunt (discovery/subnets.rs)

 dial tick ── wanted subnets ──► watch ──► hunt task (one hunt at a time)
     ▲                                       │ 1. records_matching: what discovery holds
     │                                       │ 2. find_nodes: walk the DHT for the rest
     └──── admitted peers ◄──── mpsc ◄───────┘
Piece Behavior
What is short A subscribed subnet (what our attnets advertises) with fewer than TARGET_SUBNET_PEERS = 3 connected peers advertising it (Lighthouse's number)
Predicate Admission (LeanFilter) plus at least one wanted attnets bit; the bit test runs first so most records are turned away before anything is decoded
Cadence One hunt at a time on its own task: cached matches first, then a walk of up to 10 s, at most 6 peers per hunt, 30 s between hunts
Gating Nothing is wanted while the dial loop has no budget, so no walk runs for peers nobody would dial
Re-offers A peer a hunt handed over is left out of hunts for 10 minutes: a mainnet peer at its inbound cap connects and says goodbye, and discovery keeps offering it
Where finds go The existing candidate queue. rank_candidates gains a tier: custody gaps, then wanted subnets, then any uncovered subnet
Metric lean_discovery_subnet_hunt_peers_total

3. ENR sync between discv5 and the API

Direction Status
discv5 → API Done. ethrex writes every record it re-signs (discv5 IP voting) into the SharedLocalNode it is spawned with. discovery::LocalEnr reads that mirror, and /eth/v1/node/identity reads it per request through ethlambda_rpc::EnrSource (a closure, since the RPC crate does not depend on ethlambda-p2p).
ethlambda → discv5 Not supported by ethrex yet. DiscoveryServerProtocol has no message that edits the local record. Its only consumer-driven edit, the fork_id watch, writes the EIP-2124 eth entry, not eth2. Writing into the mirror does nothing: discovery serves its own copy and overwrites the mirror on its next edit. So the served eth2 still keeps the startup digest across a fork boundary (update_served_enr still only warns; its doc now says this). What ethrex would need: a protocol message that applies an edit through NodeRecord::edit (one seq bump, re-signed) and mirrors the result, as refresh_fork_id already does for eth.

Merge resolution with #605

Main had made LeanFilter Clone with a shared fork-boundary Policy; #605 had made it non-Clone because discovery takes it by value. Both hold now: clones share the Policy and the DialTargets map, and SpawnedDiscovery carries a clone, so set_fork_id still reaches discovery's copy. #605's behavior change is inherited: --discovery.advertise-ip sets nat_extip_set, so IP voting no longer overwrites an address the operator named.

Assumptions

  • "Subnets we're missing" means subscribed subnets below 3 connected peers, not only subnets with zero.
  • Duty subnets from a validator client (beacon_committee_subscriptions) are not fed to the hunt; that stays a separate gap.
  • Counts come from the attnets of peers dialed from discovery. Inbound peers never show us an ENR, so they count for nothing: the hunt errs toward hunting.

Testing

  • make lint, cargo check --workspace --all-targets --locked, make cooldown-check
  • cargo test --workspace --profile release-fast --lib --bins: all pass. leanSpec spec suites not run.
  • New unit tests: the hunt predicate (wanted bit, other network, already offered), offer's limit and dedup, short_subnets, the no-budget gate, and the new ranking tier.
  • Not covered: an end-to-end hunt across live discovery servers. ethrex's own discv5_server_tests cover find_nodes; this side tests what it feeds it and what it does with the answer.

Follow-ups

  • Return the pin to an ethrex release tag once #7217 and the predicate-query branch land.
  • Feed VC duty subnets to the hunt (with expiry), as Lighthouse and Grandine do.
  • Ask ethrex for a local-record edit message, then refresh eth2 at fork boundaries.
  • records_matching does not exclude ethrex's tried set, so a hunt can re-offer a peer the contact poll already handed out within the same 10 minutes; the dial loop absorbs it (AlreadyInProgress, skip if connected).

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.
…dicate-queries

Folds #605's move onto ethrex's contact-table discovery API into this
branch, since ethrex's predicate queries are built on that API.

Conflicts resolved in discovery/admission.rs and discovery/dial.rs: main
had made LeanFilter cloneable with a shared fork-boundary Policy, and #605
had made it non-Clone because discovery takes it by value. Both hold now:
clones share the Policy and the DialTargets map, and SpawnedDiscovery
carries a clone, so set_fork_id still reaches the copy discovery runs.
The branch adds DiscoveryHandle::find_nodes and records_matching, discv5
predicate queries: discovery runs a consumer's test on every record a walk
sees and answers with the matches whose nodes replied. That is what lets
the dial loop look for peers on a given attestation subnet instead of
waiting for the crawl to stumble on one.

It is built on ethrex #7217, which the previous pin was, so the API this
crate uses is unchanged and it builds as is. The lockfile moves only the
ethrex crates (v25.0.0 -> v30.0.1 at 8f847524); make cooldown-check passes.

A commit rather than the branch, for the same reason as before: what this
crate builds against must not move without the manifest saying so.
…live ENR

The dial loop ranks what the crawl happened to find, which only reorders
random draws. About one mainnet record in 32 advertises a given subnet, so
a subscribed subnet nobody connected covers could stay uncovered for as
long as the crawl took to stumble on a peer for it.

The subnet hunt (discovery/subnets.rs) searches for them instead. Each
dial tick publishes the subscribed subnets with fewer than 3 connected
peers advertising them (Lighthouse's TARGET_SUBNET_PEERS). A task off the
P2P actor takes the matches discovery already holds, then walks discv5
with find_nodes for the rest, one hunt at a time, at most 6 peers each,
30 s apart. The predicate is admission plus one wanted attnets bit, as in
Lighthouse's subnet predicate. Finds join the dial loop's candidate queue
and rank after custody gaps, ahead of other subnets. Nothing is hunted
while the loop has no budget to dial, and a peer handed over is left out
of hunts for 10 minutes, since discovery keeps offering a mainnet peer
that connected and said goodbye.

/eth/v1/node/identity now reports the ENR discv5 serves rather than the
one built at startup. ethrex mirrors every record it re-signs (on discv5
IP voting) into the SharedLocalNode it was spawned with; LocalEnr reads
that mirror, and the RPC crate gets it as an EnrSource closure, since it
does not depend on ethlambda-p2p.

The other direction stays unsupported, now documented against the new
API: ethrex has no message that edits its local record. Its one
consumer-driven edit writes the EIP-2124 `eth` entry, not `eth2`, and a
write into the mirror is overwritten by discovery's next edit. So the
served eth2 still keeps the startup digest across a fork boundary.
The endpoint reads discovery's own copy of the record since the previous
commit, so after IP voting it reports the re-signed record, IP included,
rather than the startup one the paragraph described.

This branch has not been deployed

No deployments
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