diff --git a/Cargo.lock b/Cargo.lock index fc79d370..fcf2acfc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2007,6 +2007,7 @@ dependencies = [ "postcard", "thiserror 2.0.18", "tracing", + "zk_alloc", ] [[package]] @@ -2024,7 +2025,6 @@ dependencies = [ "thiserror 2.0.18", "tokio", "tracing", - "zk_alloc", ] [[package]] diff --git a/bin/ethlambda/src/main.rs b/bin/ethlambda/src/main.rs index 01774b51..990a8c79 100644 --- a/bin/ethlambda/src/main.rs +++ b/bin/ethlambda/src/main.rs @@ -109,6 +109,36 @@ fn main() -> eyre::Result<()> { } } +/// What `/eth/v1/node/identity` reports beyond the peer id: the ENR discv5 +/// publishes, and the multiaddrs peers reach this node on. The addresses need +/// the node's externally reachable IP, which only `--discovery.advertise-ip` +/// supplies (a node bound to `0.0.0.0` does not know it), so without the flag +/// they are left empty rather than guessed. +fn beacon_identity( + enr: &str, + advertise_ip: Option, + gossipsub_port: u16, + discovery_port: u16, + peer_id: &str, +) -> ethlambda_rpc::BeaconIdentity { + let Some(ip) = advertise_ip else { + return ethlambda_rpc::BeaconIdentity { + enr: enr.to_string(), + ..Default::default() + }; + }; + let family = if ip.is_ipv4() { "ip4" } else { "ip6" }; + ethlambda_rpc::BeaconIdentity { + enr: enr.to_string(), + // QUIC first, matching the order this node dials in. + p2p_addresses: vec![ + format!("/{family}/{ip}/udp/{gossipsub_port}/quic-v1/p2p/{peer_id}"), + format!("/{family}/{ip}/tcp/{gossipsub_port}/p2p/{peer_id}"), + ], + discovery_addresses: vec![format!("/{family}/{ip}/udp/{discovery_port}/p2p/{peer_id}")], + } +} + /// Node logging: INFO and above, on stdout. fn init_node_logging() -> eyre::Result<()> { let filter = EnvFilter::builder() @@ -727,6 +757,13 @@ async fn run_node(options: Options) -> eyre::Result<()> { let rpc_sync_status = sync_status.clone(); let rpc_events = events.clone(); let rpc_p2p = p2p.actor_ref().to_rpc_to_p2p_ref(); + let rpc_identity = beacon_identity( + p2p.local_enr().unwrap_or_default(), + common.discovery.advertise_ip, + common.gossipsub_port, + common.discovery.port, + &local_peer_id, + ); // Block production builds its payloads with the same execution client the // chain actor validates them with. let rpc_engine = match &setup.chain { @@ -751,6 +788,7 @@ async fn run_node(options: Options) -> eyre::Result<()> { p2p: rpc_p2p, attestation_pool: attestation_pool.clone(), engine: rpc_engine, + identity: rpc_identity, }, local_peer_id, rpc_shutdown, diff --git a/bin/ethlambda/src/validator.rs b/bin/ethlambda/src/validator.rs index 15285c90..df486c2b 100644 --- a/bin/ethlambda/src/validator.rs +++ b/bin/ethlambda/src/validator.rs @@ -27,6 +27,9 @@ pub(crate) struct ValidatorOptions { pub(crate) beacon_nodes: Vec, /// Directory holding the EIP-2335 keystores and `validator_definitions.yml`. + /// Without that file, keystores in the Lighthouse layout + /// (`<0xpubkey>/voting-keystore.json`, password in + /// `--secrets-dir/<0xpubkey>`) are discovered and the file is written. #[arg(long)] pub(crate) validators_dir: PathBuf, diff --git a/crates/net/p2p/src/discovery/mod.rs b/crates/net/p2p/src/discovery/mod.rs index 6a6e3a09..3e24cae0 100644 --- a/crates/net/p2p/src/discovery/mod.rs +++ b/crates/net/p2p/src/discovery/mod.rs @@ -191,7 +191,15 @@ pub async fn spawn_discovery( }; let local_node = params.local_node(); let local_record = build_local_enr(¶ms)?; - let local_enr = local_record.enr_url().map_err(DiscoveryError::EncodeEnr)?; + // EIP-778's text form is URL-safe base64 *without* padding. ethrex's + // `enr_url` pads, and lighthouse refuses a padded record outright ("Not + // valid as ENR nor Multiaddr"), which is the form this string is handed to + // peers in: `/eth/v1/node/identity`, and through it devnet bootnode lists. + let local_enr = local_record + .enr_url() + .map_err(DiscoveryError::EncodeEnr)? + .trim_end_matches('=') + .to_string(); // `spawn` rather than `spawn_with_filter` would install ethrex's own filter, // which wants an EIP-2124 `eth` entry compatible with an execution chain lean @@ -304,6 +312,9 @@ mod tests { .expect("discovery spawns"); assert!(handle.local_enr.starts_with("enr:")); + // EIP-778's text form carries no base64 padding, and lighthouse + // refuses a record that does. + assert!(!handle.local_enr.contains('='), "{}", handle.local_enr); let record = decode_enr(&handle.local_enr); // With discovery_port: 0 the OS picks the real port, and the published diff --git a/crates/net/p2p/src/lib.rs b/crates/net/p2p/src/lib.rs index 1a0a7c84..b172fb17 100644 --- a/crates/net/p2p/src/lib.rs +++ b/crates/net/p2p/src/lib.rs @@ -1020,6 +1020,9 @@ pub fn build_swarm(config: SwarmConfig) -> Result { /// Public handle to the P2P actor. pub struct P2P { handle: ActorRef, + /// This node's ENR as published at startup, for the Beacon API's + /// `/eth/v1/node/identity`. `None` when discovery is disabled. + local_enr: Option, } impl P2P { @@ -1044,11 +1047,15 @@ impl P2P { discovery: Option, attestation_pool: SharedAttestationPool, ) -> Result { - let discovery = match discovery { - Some(config) => Some(spawn_discovery(config).await?), + let (discovery, local_enr) = match discovery { + Some(config) => { + let discovery = spawn_discovery(config).await?; + let local_enr = discovery.local_enr.clone(); + (Some(discovery), Some(local_enr)) + } None => { info!("discv5 discovery disabled; peering from the static bootnode list only"); - None + (None, None) } }; let (swarm_stream, swarm_handle) = @@ -1118,12 +1125,20 @@ impl P2P { ); } spawn_listener(handle.context(), swarm_stream.map(WrappedSwarmEvent)); - Ok(P2P { handle }) + Ok(P2P { handle, local_enr }) } pub fn actor_ref(&self) -> &ActorRef { &self.handle } + + /// This node's ENR, `enr:`-prefixed, as published at startup. discv5 may + /// re-sign it later with a higher sequence number if IP voting changes the + /// external address; this is the startup record. `None` when discovery is + /// disabled, since then no ENR is published. + pub fn local_enr(&self) -> Option<&str> { + self.local_enr.as_deref() + } } /// Message wrapper for swarm events. Not part of the protocol because diff --git a/crates/net/rpc/src/beacon/node.rs b/crates/net/rpc/src/beacon/node.rs index 564d23d1..a9199698 100644 --- a/crates/net/rpc/src/beacon/node.rs +++ b/crates/net/rpc/src/beacon/node.rs @@ -17,7 +17,12 @@ pub(crate) fn routes(version: &'static str, peer_id: String) -> Router { .route("/eth/v1/node/version", get(move || get_version(version))) .route( "/eth/v1/node/identity", - get(move || get_identity(peer_id.clone())), + get(move |identity: Option>| { + get_identity( + peer_id.clone(), + identity.map(|Extension(identity)| identity), + ) + }), ) } @@ -69,18 +74,21 @@ async fn get_version(version: &'static str) -> Response { crate::json_response(serde_json::json!({ "data": { "version": version } })) } -async fn get_identity(peer_id: String) -> Response { - // `enr` and the two address lists are empty, which is not spec-valid: the - // ENR is built for discv5 and owned by the P2P actor, and `BuiltSwarm` - // hands `run_node` only a `local_peer_id`. Serving the record means - // widening the `ethlambda-p2p` surface and threading it through startup, - // which is a change of its own. Recorded in docs/spec_deviations.md. +/// `GET /eth/v1/node/identity`: the peer id, the node's ENR, and the +/// multiaddrs it listens on, which is how a peer that reads this endpoint (a +/// devnet orchestrator wiring bootnodes, say) dials it. +/// +/// The addresses are only as good as `--discovery.advertise-ip`: without it the +/// node does not know the address peers reach it on, so the lists are empty and +/// the ENR carries no IP. `metadata` is still a placeholder. +async fn get_identity(peer_id: String, identity: Option) -> Response { + let identity = identity.unwrap_or_default(); crate::json_response(serde_json::json!({ "data": { "peer_id": peer_id, - "enr": "", - "p2p_addresses": [], - "discovery_addresses": [], + "enr": identity.enr, + "p2p_addresses": identity.p2p_addresses, + "discovery_addresses": identity.discovery_addresses, "metadata": { "seq_number": "0", "attnets": "0x0000000000000000", @@ -158,13 +166,38 @@ mod tests { } #[tokio::test] - async fn identity_carries_the_peer_id_and_empty_network_fields() { + async fn identity_without_an_identity_carries_the_peer_id_and_empty_network_fields() { let json = body_json(get("/eth/v1/node/identity").await).await; assert_eq!(json["data"]["peer_id"], "test-peer"); - // Deliberately empty, and not spec-valid; see docs/spec_deviations.md. assert_eq!(json["data"]["enr"], ""); assert_eq!(json["data"]["p2p_addresses"], serde_json::json!([])); - assert_eq!(json["data"]["discovery_addresses"], serde_json::json!([])); assert_eq!(json["data"]["metadata"]["seq_number"], "0"); } + + #[tokio::test] + async fn identity_reports_the_enr_and_listen_addresses() { + let fixture = beacon_fixture(ANCHOR_SLOT); + let identity = crate::BeaconIdentity { + enr: "enr:-abc".to_string(), + p2p_addresses: vec!["/ip4/10.0.0.1/tcp/9001/p2p/test-peer".to_string()], + discovery_addresses: vec!["/ip4/10.0.0.1/udp/9000/p2p/test-peer".to_string()], + }; + let app = routes("ethlambda/test", "test-peer".into()) + .with_state(fixture.store) + .layer(Extension(identity)); + let request = Request::builder() + .uri("/eth/v1/node/identity") + .body(Body::empty()) + .unwrap(); + let json = body_json(app.oneshot(request).await.unwrap()).await; + assert_eq!(json["data"]["enr"], "enr:-abc"); + assert_eq!( + json["data"]["p2p_addresses"][0], + "/ip4/10.0.0.1/tcp/9001/p2p/test-peer" + ); + assert_eq!( + json["data"]["discovery_addresses"][0], + "/ip4/10.0.0.1/udp/9000/p2p/test-peer" + ); + } } diff --git a/crates/net/rpc/src/lib.rs b/crates/net/rpc/src/lib.rs index a2144f7c..e80ceb74 100644 --- a/crates/net/rpc/src/lib.rs +++ b/crates/net/rpc/src/lib.rs @@ -184,6 +184,17 @@ pub fn build_beacon_api_router(store: Store, version: &'static str, peer_id: Str .with_state(store) } +/// How peers reach this node, as `/eth/v1/node/identity` reports it. +#[derive(Debug, Clone, Default)] +pub struct BeaconIdentity { + /// The node's ENR, `enr:`-prefixed. + pub enr: String, + /// libp2p multiaddrs the node listens on, each ending in `/p2p/`. + pub p2p_addresses: Vec, + /// discv5 multiaddrs, likewise. + pub discovery_addresses: Vec, +} + /// What the Beacon API's validator endpoints reach beyond the store. pub struct BeaconApiHandles { /// Through which the pool, aggregate and block endpoints gossip what a @@ -195,6 +206,8 @@ pub struct BeaconApiHandles { /// The execution client block production builds payloads with; `None` /// makes it answer 503. pub engine: Option, + /// What `/eth/v1/node/identity` reports beyond the peer id. + pub identity: BeaconIdentity, } /// Start the HTTP servers for a beacon node. @@ -216,7 +229,8 @@ pub async fn start_beacon_rpc_server( .layer(Extension(handles.p2p)) .layer(Extension(handles.attestation_pool)) .layer(Extension(beacon::validator::FeeRecipients::default())) - .layer(Extension(handles.engine)); + .layer(Extension(handles.engine)) + .layer(Extension(handles.identity)); start_http_servers(config, Some(api_router), shutdown).await } diff --git a/crates/validator/src/keys/definitions.rs b/crates/validator/src/keys/definitions.rs index f1c99080..ab48a8db 100644 --- a/crates/validator/src/keys/definitions.rs +++ b/crates/validator/src/keys/definitions.rs @@ -79,6 +79,66 @@ impl ValidatorDefinitions { } /// The entries that should be loaded and signed with. + /// Write a definitions file for a validators directory that has none, by + /// discovering the keystores laid out the way Lighthouse lays them out: + /// `/<0xpubkey>/voting-keystore.json`, each with its + /// password in `/<0xpubkey>`. This is the layout + /// `eth2-val-tools`, `staking-deposit-cli` imports and ethereum-package + /// produce, and what Lighthouse's own client discovers without a + /// definitions file. + /// + /// Does nothing when the file already exists: it is the operator's (and the + /// keymanager's) record of which validators run, and rescanning would + /// resurrect a key the keymanager deleted. The public key is read from each + /// keystore; `ValidatorStore::load` then checks it against the decrypted + /// secret as it does for every definition. Returns how many were written. + pub fn discover_if_absent(validators_dir: &Path, secrets_dir: &Path) -> Result { + if validators_dir.join(DEFINITIONS_FILE).exists() { + return Ok(0); + } + let entries = std::fs::read_dir(validators_dir).map_err(|source| Error::Io { + path: validators_dir.display().to_string(), + source, + })?; + let mut definitions = Vec::new(); + for entry in entries { + let entry = entry.map_err(|source| Error::Io { + path: validators_dir.display().to_string(), + source, + })?; + let keystore_path = entry.path().join("voting-keystore.json"); + if !keystore_path.is_file() { + continue; + } + let json = std::fs::read_to_string(&keystore_path).map_err(|source| Error::Io { + path: keystore_path.display().to_string(), + source, + })?; + let pubkey = crate::keys::keystore::Keystore::from_json(&json) + .ok() + .and_then(|keystore| keystore.pubkey) + .ok_or_else(|| Error::Keystore { + path: keystore_path.display().to_string(), + reason: "a discovered keystore must name its public key".to_string(), + })?; + let pubkey = format!("0x{}", pubkey.trim_start_matches("0x")); + definitions.push(ValidatorDefinition { + enabled: true, + voting_keystore_password_path: secrets_dir.join(&pubkey), + voting_public_key: pubkey, + voting_keystore_path: keystore_path, + }); + } + // Directory order is not stable across filesystems; sorting makes the + // written file reproducible. + definitions.sort_by(|a, b| a.voting_public_key.cmp(&b.voting_public_key)); + let count = definitions.len(); + if count > 0 { + Self(definitions).save(validators_dir)?; + } + Ok(count) + } + pub fn enabled(&self) -> impl Iterator { self.0.iter().filter(|definition| definition.enabled) } @@ -88,6 +148,64 @@ impl ValidatorDefinitions { mod tests { use super::*; + /// A keystore file naming `pubkey`, in the minimal shape discovery reads. + fn write_keystore(dir: &Path, pubkey: &str) { + let key_dir = dir.join(format!("0x{pubkey}")); + std::fs::create_dir_all(&key_dir).unwrap(); + let json = serde_json::json!({ + "crypto": { + "kdf": { "function": "pbkdf2", "params": { "dklen": 32, "c": 1, "prf": "hmac-sha256", "salt": "00" }, "message": "" }, + "checksum": { "function": "sha256", "params": {}, "message": "00" }, + "cipher": { "function": "aes-128-ctr", "params": { "iv": "00" }, "message": "00" } + }, + "pubkey": pubkey, + "path": "", + "uuid": "00000000-0000-0000-0000-000000000000", + "version": 4 + }); + std::fs::write(key_dir.join("voting-keystore.json"), json.to_string()).unwrap(); + } + + #[test] + fn keystores_in_the_lighthouse_layout_are_discovered_when_no_file_exists() { + let validators = tempfile::tempdir().unwrap(); + let secrets = tempfile::tempdir().unwrap(); + write_keystore(validators.path(), "bb"); + write_keystore(validators.path(), "aa"); + std::fs::create_dir(validators.path().join("not-a-key")).unwrap(); + + let written = + ValidatorDefinitions::discover_if_absent(validators.path(), secrets.path()).unwrap(); + assert_eq!(written, 2); + let definitions = ValidatorDefinitions::open(validators.path()).unwrap(); + assert_eq!(definitions.0[0].voting_public_key, "0xaa"); + assert_eq!( + definitions.0[0].voting_keystore_password_path, + secrets.path().join("0xaa") + ); + assert!(definitions.0.iter().all(|definition| definition.enabled)); + } + + #[test] + fn an_existing_definitions_file_is_never_rescanned() { + let validators = tempfile::tempdir().unwrap(); + let secrets = tempfile::tempdir().unwrap(); + write_keystore(validators.path(), "aa"); + ValidatorDefinitions(Vec::new()) + .save(validators.path()) + .unwrap(); + + let written = + ValidatorDefinitions::discover_if_absent(validators.path(), secrets.path()).unwrap(); + assert_eq!(written, 0); + assert!( + ValidatorDefinitions::open(validators.path()) + .unwrap() + .0 + .is_empty() + ); + } + fn definition(pubkey: &str, enabled: bool) -> ValidatorDefinition { ValidatorDefinition { enabled, diff --git a/crates/validator/src/lib.rs b/crates/validator/src/lib.rs index 9eca256e..c5dd2350 100644 --- a/crates/validator/src/lib.rs +++ b/crates/validator/src/lib.rs @@ -122,6 +122,17 @@ pub async fn run(config: ValidatorConfig) -> Result<()> { ); } + let discovered = keys::definitions::ValidatorDefinitions::discover_if_absent( + &config.validators_dir, + &config.secrets_dir, + )?; + if discovered > 0 { + info!( + count = discovered, + "No validator definitions file; wrote one from the keystores found in the validators \ + directory" + ); + } let store = ValidatorStore::load(&config.validators_dir)?; if store.is_empty() { warn!("No validators loaded; the client will idle"); diff --git a/docs/cli.md b/docs/cli.md index 4b665e16..5e157049 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -277,7 +277,7 @@ implementation's, so none of the [common flags](#common-flags) apply to it. | Flag | Default | Meaning | |---|---|---| | `--beacon-nodes` | required | Base URLs of the beacon nodes to use, comma-separated or repeated. Tried in list order; the first that answers serves the request, so the order is a preference, not load balancing | -| `--validators-dir` | required | Directory holding the EIP-2335 keystores and `validator_definitions.yml` | +| `--validators-dir` | required | Directory holding the EIP-2335 keystores and `validator_definitions.yml`. Without that file, keystores in the Lighthouse layout (`<0xpubkey>/voting-keystore.json`, password in `--secrets-dir/<0xpubkey>`) are discovered and the file is written | | `--secrets-dir` | required | Directory holding one password file per keystore, named after the validator's public key | | `--http-address` | `127.0.0.1` | Bind address for the metrics and keymanager servers | | `--metrics-port` | `5064` | Prometheus metrics port | diff --git a/docs/rpc.md b/docs/rpc.md index cd0f2259..8b677eb1 100644 --- a/docs/rpc.md +++ b/docs/rpc.md @@ -235,7 +235,7 @@ surface rather than sitting beside it; a `/lean/v0` path on a beacon node is a | `GET` | `/eth/v1/node/syncing` | JSON | Head slot, sync distance, optimistic flag | | `GET` | `/eth/v1/node/health` | *(status only)* | `200` caught up, `206` syncing | | `GET` | `/eth/v1/node/version` | JSON | Client version string | -| `GET` | `/eth/v1/node/identity` | JSON | Peer ID and metadata only (see below) | +| `GET` | `/eth/v1/node/identity` | JSON | Peer ID, ENR and listen multiaddrs (see below) | | `GET`, `POST` | `/eth/v1/beacon/states/{state_id}/validators` | JSON | Registry entries by index or pubkey, with status | | `GET` | `/eth/v1/validator/duties/proposer/{epoch}` | JSON | Proposers for the head's epoch or the next | | `POST` | `/eth/v1/validator/duties/attester/{epoch}` | JSON | Committee assignments for the given indices | @@ -385,8 +385,11 @@ the block under it really sits at that slot. This matters because it is the slot a checkpoint-syncing peer asks for right after reading the finalized state. A slot the store holds nothing at is still a `404`. -`/eth/v1/node/identity` reports `peer_id` and `metadata`; `enr`, -`p2p_addresses` and `discovery_addresses` are empty. +`/eth/v1/node/identity` reports the peer id, the ENR discv5 publishes, and the +node's QUIC, TCP and discv5 multiaddrs. The multiaddrs need +`--discovery.advertise-ip`, since a node bound to `0.0.0.0` does not know the +address peers reach it on; without the flag they are empty and the ENR carries +no IP. `metadata` is still a placeholder. ## Metrics & Debug Server (`:5054`) diff --git a/docs/spec_deviations.md b/docs/spec_deviations.md index 7ff2d24e..45b68361 100644 --- a/docs/spec_deviations.md +++ b/docs/spec_deviations.md @@ -82,23 +82,6 @@ same keys, already signed. runs, and treat a restart as an event that needs the same care a manual key move would. The client warns about this at startup on every run. -## `/eth/v1/node/identity` reports no ENR - -The endpoint's `enr`, `p2p_addresses` and `discovery_addresses` are empty -rather than populated, which is not spec-valid. - -- **ethlambda:** `get_identity` (`crates/net/rpc/src/beacon/node.rs`) reports - `peer_id` and a placeholder `metadata` block and nothing else. The ENR is - built for discv5 and owned by the P2P actor; `BuiltSwarm` hands `run_node` - only a `local_peer_id`, so serving the record means widening the - `ethlambda-p2p` surface and threading it through startup. -- **Beacon API:** `enr` is the node's base64 ENR and the two lists are its - libp2p multiaddrs. -- **Consequence:** a consumer reading `enr` to dial this node gets an empty - string rather than a record, so peer discovery through this endpoint does not - work. Everything that reads `peer_id` is unaffected. Out of scope for the - change that added the Beacon API surface; a follow-up exposes the record. - ## `block_id` cannot name `genesis`, and `state_id` cannot be a state root Two id forms the Beacon API defines return `404` here.