Skip to content

Commit 337e4ba

Browse files
committed
chore(deps): serve the ENR we report, on the ethrex branch tip
Bumps ethrex to the feat/discovery-peer-requirements tip (f30b16d5 -> bf401280, rebased onto main 24.0.0), which reworks `DiscoveryServer::spawn` to take a prepared `NodeRecord` instead of a `Store` it derived one from. That closes the gap docs/discovery.md called "the record ethrex serves is not the record we report": ethrex built its own copy from the local `Node`, so what it answered discv5 queries with carried `ip`, `udp` and `secp256k1` but none of `eth2`, `attnets` or `quic`. A lean peer applying our own admission rules to that record rejected us for the missing `quic` entry, so discovery found peers but could not be found by them. We now hand `spawn` the same record `enr_url` reports, and ethrex edits and re-signs it on IP voting rather than rebuilding, so the consensus entries survive a sequence bump. The empty in-memory ethrex `Store` existed only to satisfy the old signature, so both it and the `ethrex-storage` dependency go, along with the `DiscoveryError::Store` variant that could no longer be constructed.
1 parent 0bf322c commit 337e4ba

5 files changed

Lines changed: 46 additions & 67 deletions

File tree

CLAUDE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ actual_slot = finalized_slot + 1 + relative_index
282282

283283
### Peer Discovery (discv5, opt-in)
284284
- Off by default; `--discovery.enable` plus `--discovery.port` (own UDP socket, must differ from `--gossipsub-port`)
285-
- Reuses ethrex's `DiscoveryServer` + `PeerTable` with discv4 disabled and an empty in-memory ethrex `Store` (`spawn` requires one; lean has no execution chain)
285+
- Reuses ethrex's `DiscoveryServer` + `PeerTable` with discv4 disabled; `spawn` takes the prepared lean ENR, so the record ethrex serves is the one we report
286286
- ENR follows the beacon phase0 spec: `ip`/`udp`/`quic`/`secp256k1`/`eth2`/`attnets`, deliberately **no** `tcp`
287287
- Admission mirrors lighthouse: `eth2.fork_digest` must match, `next_fork_*` may differ, `quic` entry required. Handed to the peer table as `LeanFilter: PeerFilter`, so records are judged on arrival, not at dial time; a reject is re-judged on a higher-`seq` ENR
288288
- Candidates ranked by uncovered attestation subnets. See [`docs/discovery.md`](docs/discovery.md)

Cargo.lock

Lines changed: 33 additions & 34 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/net/p2p/Cargo.toml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,6 @@ rand = "0.8"
4444
ethrex-p2p = { git = "https://github.com/lambdaclass/ethrex", branch = "feat/discovery-peer-requirements" }
4545
ethrex-rlp = { git = "https://github.com/lambdaclass/ethrex", branch = "feat/discovery-peer-requirements" }
4646
ethrex-common = { git = "https://github.com/lambdaclass/ethrex", branch = "feat/discovery-peer-requirements" }
47-
# Only for the empty in-memory Store that DiscoveryServer::spawn requires.
48-
ethrex-storage = { git = "https://github.com/lambdaclass/ethrex", branch = "feat/discovery-peer-requirements" }
4947

5048
# Version pinned to ethrex's workspace: `SecretKey` crosses the API boundary.
5149
secp256k1 = { version = "0.30.0", default-features = false, features = ["global-context"] }

crates/net/p2p/src/discovery/mod.rs

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ use std::time::Duration;
1919
use ethrex_p2p::discovery::{DiscoveryConfig, DiscoveryServer};
2020
use ethrex_p2p::peer_table::{PeerTable, PeerTableServer};
2121
use ethrex_p2p::types::Node;
22-
use ethrex_storage::{EngineType, Store};
2322
use tokio::net::UdpSocket;
2423
use tracing::{info, warn};
2524

@@ -51,8 +50,6 @@ pub enum DiscoveryError {
5150
BuildEnr(ethrex_p2p::types::NodeError),
5251
#[error("failed to encode local ENR: {0}")]
5352
EncodeEnr(ethrex_p2p::types::NodeError),
54-
#[error("failed to create the discovery store: {0}")]
55-
Store(String),
5653
#[error("failed to start discovery server: {0}")]
5754
Server(String),
5855
#[error("node key is not a valid secp256k1 secret key: {0}")]
@@ -151,18 +148,14 @@ pub async fn spawn_discovery(
151148
"Starting discv5 discovery"
152149
);
153150

154-
// `spawn` requires a store but the discv5 path reads it only for
155-
// `get_fork_id`, which errors on a store with no genesis block and makes
156-
// ethrex skip the `eth` entry — what lean wants, having no execution chain.
157-
// That the record ethrex then serves is *not* the complete one we report is
158-
// a known gap; see docs/discovery.md, "The record ethrex serves is not the
159-
// record we report".
160-
let store = Store::new("", EngineType::InMemory)
161-
.map_err(|err| DiscoveryError::Store(err.to_string()))?;
162-
151+
// The record we hand over is the same one `enr_url` above reported, so what
152+
// ethrex answers discv5 queries with carries the consensus entries (`eth2`,
153+
// `attnets`, `quic`) and a lean peer applying our own admission rules to it
154+
// admits us. ethrex re-signs it under `params.signer` whenever IP voting
155+
// bumps the sequence number, keeping the extra entries.
163156
DiscoveryServer::spawn(
164-
store,
165157
local_node,
158+
local_record,
166159
params.signer,
167160
Arc::new(socket),
168161
peer_table.clone(),

docs/discovery.md

Lines changed: 6 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,12 @@ and ethlambda speaks QUIC only.
5858
Read the local ENR from `GET /lean/v0/node/identity`, which reports it as `enr`
5959
(`null` when discovery is disabled). It is also logged once at startup.
6060

61+
This same record is handed to ethrex's `DiscoveryServer`, so it is what answers
62+
discv5 queries: what we report and what peers see are the same bytes. If IP
63+
voting later changes our external address, ethrex edits and re-signs that record
64+
rather than rebuilding one, so the consensus entries survive the bump; only the
65+
sequence number and `ip` move, which the reported ENR then lags.
66+
6167
## Which peers get dialed
6268

6369
A discovered peer is admitted only if:
@@ -131,23 +137,6 @@ another**: two devnets running this code will peer with each other. Closing that
131137
gap requires lean adopting a genesis-derived fork digest, which is a
132138
cross-client change to gossip topic names.
133139

134-
### The record ethrex serves is not the record we report
135-
136-
`GET /lean/v0/node/identity` reports the ENR built by `build_local_enr`, which
137-
carries every entry in the table above. ethrex's `DiscoveryServer` builds its
138-
own copy from the local `Node` and offers no way to seed the consensus entries,
139-
so the record it answers discv5 queries with carries `ip`, `udp` and
140-
`secp256k1` but **not** `eth2`, `attnets` or `quic`.
141-
142-
Discovery is therefore one-sided: we find lean peers and admit them, but a lean
143-
peer applying [the same admission rules](#which-peers-get-dialed) to what ethrex
144-
serves rejects us for a missing `quic` entry. Copying our reported ENR into
145-
another node's bootnode list still works, since that is the complete record.
146-
147-
Closing this needs a way to hand ethrex's `DiscoveryServer::spawn` a prepared
148-
record instead of having it build one. Until then, discovery finds peers but
149-
cannot be found by them.
150-
151140
### A beacon-chain client cannot discover us, and `tcp` is why
152141

153142
Beyond the fork digest never matching a real beacon network, there is a second,

0 commit comments

Comments
 (0)