From 6dabf38d0eb773882adc3d7cb2082a3e3e30b96b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Wed, 28 Oct 2026 19:19:50 -0300 Subject: [PATCH 1/4] docs(readme): give docker stop time to flush, explain the version string A clean shutdown during catch-up took 14.3 s, past Docker's default 10 s stop timeout, so `docker stop` killed the node mid-flush and lost the state `--stop-signal SIGINT` was there to save. Both `docker run` lines now pass `--stop-timeout 60`. A release binary reports `-rc.N-` in `--version` while the promoted image reports `-stable-`, and only RELEASE.md said why. The release notes link this README, so the explanation goes here too. The `is_syncing` caveat understated the problem: during catch-up it can also flip back and forth, and `/eth/v1/node/health` can answer 200 while the node is far behind. --- README.md | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 2b3457da7..116373eaf 100644 --- a/README.md +++ b/README.md @@ -72,6 +72,10 @@ with `Ctrl+C`; it shuts down gracefully on SIGINT only, so a systemd unit needs `xattr -d com.apple.quarantine ` lifts; a `curl` download is not. - [`RELEASE.md`](./RELEASE.md#verifying-a-release) shows how to verify an ethlambda binary's provenance attestation. +- A release binary is the tested release candidate's build, so `--version` + names that candidate (`vX.Y.Z-rc.N-`), while the same release's Docker + image reports `vX.Y.Z-stable-`. Both run the same code; see + [`RELEASE.md`](./RELEASE.md#version-string). To skip payload validation, run the node without ethrex: drop it, the JWT secret and the two `--execution-*` flags. The node then follows the chain and @@ -96,8 +100,9 @@ curl -s localhost:5052/eth/v1/node/syncing ``` The node has caught up once `sync_distance` (the chain's current slot minus -`head_slot`) is near 0. Don't rely on `is_syncing` yet: it currently reads -`false` during catch-up as well. +`head_slot`) is near 0. Don't rely on `is_syncing` or `/eth/v1/node/health` +yet: during catch-up `is_syncing` can read `false` or switch back and forth, +and the health endpoint can answer `200` rather than `206`. For Sepolia or Hoodi, change `--network mainnet` to `--network sepolia` or `--network hoodi` in both the ethrex and the beacon node arguments, and point @@ -150,8 +155,10 @@ docker run -d --name ethrex --network ethereum \ --authrpc.addr 0.0.0.0 \ --authrpc.jwtsecret /jwt.hex -# The node shuts down gracefully on SIGINT only, so this makes `docker stop` flush its state -docker run -d --name ethlambda-beacon --network ethereum --stop-signal SIGINT \ +# The node shuts down gracefully on SIGINT only, so `--stop-signal` makes `docker stop` send it. +# Flushing its state can outlast Docker's default 10 s, after which Docker kills it and the +# unflushed state is lost, so `--stop-timeout` gives it a minute. +docker run -d --name ethlambda-beacon --network ethereum --stop-signal SIGINT --stop-timeout 60 \ -p 9000:9000/udp -p 9001:9001/udp -p 9001:9001/tcp \ -p 127.0.0.1:5052:5052 \ -v "$PWD/beacon-data:/data" \ @@ -182,8 +189,10 @@ openssl rand -hex 32 > beacon-data/node-key # This host's public address, published in the node's ENR (see above) PUBLIC_IP=$(curl -s https://ifconfig.me) -# The node shuts down gracefully on SIGINT only, so this makes `docker stop` flush its state -docker run -d --name ethlambda-beacon --stop-signal SIGINT \ +# The node shuts down gracefully on SIGINT only, so `--stop-signal` makes `docker stop` send it. +# Flushing its state can outlast Docker's default 10 s, after which Docker kills it and the +# unflushed state is lost, so `--stop-timeout` gives it a minute. +docker run -d --name ethlambda-beacon --stop-signal SIGINT --stop-timeout 60 \ -p 9000:9000/udp -p 9001:9001/udp -p 9001:9001/tcp \ -p 127.0.0.1:5052:5052 \ -v "$PWD/beacon-data:/data" \ From 153dc2f32f7374bcf3addf911c9c74272774bf02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Wed, 28 Oct 2026 19:19:55 -0300 Subject: [PATCH 2/4] fix(p2p): stop logging a graceful shutdown as a P2P failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every Ctrl+C logged `ERROR Swarm adapter loop exited — P2P networking is no longer functional`, which reads as a crash to an operator. The swarm loop ends when the P2P actor drops its end of either channel, which is how a shutdown reaches it, so that exit is now logged at info. An actor that panicked instead looks the same from the loop, but spawned-concurrency already logs the panic as an error before stopping the actor. Only the swarm's own event stream ending, which never happens while the swarm lives, stays an error. --- crates/net/p2p/src/swarm_adapter.rs | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/crates/net/p2p/src/swarm_adapter.rs b/crates/net/p2p/src/swarm_adapter.rs index 245827325..83094c4bb 100644 --- a/crates/net/p2p/src/swarm_adapter.rs +++ b/crates/net/p2p/src/swarm_adapter.rs @@ -269,14 +269,14 @@ async fn swarm_loop( ) { let mut mesh_metric_tick = tokio::time::interval(MESH_METRIC_REFRESH_INTERVAL); mesh_metric_tick.set_missed_tick_behavior(MissedTickBehavior::Skip); - loop { + let exit = loop { tokio::select! { event = swarm.next() => { - let Some(event) = event else { break }; - if event_tx.send(event).is_err() { break } + let Some(event) = event else { break LoopExit::SwarmEnded }; + if event_tx.send(event).is_err() { break LoopExit::ActorStopped } } cmd = cmd_rx.recv() => { - let Some(cmd) = cmd else { break }; + let Some(cmd) = cmd else { break LoopExit::ActorStopped }; execute_command(&mut swarm, cmd); } _ = mesh_metric_tick.tick() => { @@ -301,8 +301,27 @@ async fn swarm_loop( disconnect_graylisted_peers(&mut swarm); } } + }; + match exit { + LoopExit::ActorStopped => info!("Swarm adapter loop stopped: the P2P actor shut down"), + LoopExit::SwarmEnded => { + error!( + "Swarm adapter loop exited: the swarm stopped, so P2P networking is no longer functional" + ) + } } - error!("Swarm adapter loop exited — P2P networking is no longer functional"); +} + +/// Why [`swarm_loop`] returned. +/// +/// The P2P actor dropping its end of either channel is how a graceful +/// shutdown reaches this task, so that exit is routine. An actor that panicked +/// instead of stopping looks the same from here, but `spawned-concurrency` +/// already logs that panic as an error. The swarm's own event stream never +/// ends while the swarm lives, so its end is the one exit worth an error. +enum LoopExit { + ActorStopped, + SwarmEnded, } /// Disconnect every peer gossipsub scores below the graylist, and publish how From 476d4ff466d19e0960e940e78258c6654a491c3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Wed, 28 Oct 2026 19:20:04 -0300 Subject: [PATCH 3/4] fix(rpc): report the node's live MetaData in /eth/v1/node/identity `metadata` was a hard-coded placeholder (seq 0, no subnets), so it disagreed with the ENR's `attnets` and with what peers get over req/resp. The p2p layer now publishes its MetaData (`seq_number`, `attnets`, `syncnets`) to a shared `MetaDataController` at startup and on every sequence-number bump, all of which go through `BeaconWire::bump_metadata_seq_number`. The endpoint reads it per request, since `syncnets` and the sequence number change when a validator client joins sync committee subnets. `seq_number` stays the MetaData counter, which the p2p specification keeps separate from the ENR's `seq`. --- bin/ethlambda/src/main.rs | 18 ++++--- crates/net/api/src/lib.rs | 45 ++++++++++++++++ crates/net/p2p/src/beacon/handler.rs | 13 +++++ crates/net/p2p/src/beacon/mod.rs | 24 ++++++++- crates/net/p2p/src/beacon/sync_committee.rs | 28 ++++++++-- crates/net/p2p/src/beacon/transition.rs | 2 +- crates/net/p2p/src/lib.rs | 25 +++++++-- crates/net/rpc/src/beacon/node.rs | 58 ++++++++++++++++++--- crates/net/rpc/src/lib.rs | 4 ++ docs/rpc.md | 5 +- 10 files changed, 198 insertions(+), 24 deletions(-) diff --git a/bin/ethlambda/src/main.rs b/bin/ethlambda/src/main.rs index 6ea55328f..3d2eed361 100644 --- a/bin/ethlambda/src/main.rs +++ b/bin/ethlambda/src/main.rs @@ -145,6 +145,7 @@ fn beacon_identity( format!("/{family}/{ip}/tcp/{gossipsub_port}/p2p/{peer_id}"), ], discovery_addresses: vec![format!("/{family}/{ip}/udp/{discovery_port}/p2p/{peer_id}")], + metadata: None, } } @@ -768,13 +769,16 @@ async fn run_node(options: Options) -> eyre::Result<()> { } => ethlambda_rpc::CustodyColumns(custody_columns.clone()), ChainActor::Lean(..) => ethlambda_rpc::CustodyColumns::default(), }; - let rpc_identity = beacon_identity( - p2p.local_enr().unwrap_or_default(), - common.discovery.advertise_ip, - common.gossipsub_port, - common.discovery.port, - &local_peer_id, - ); + let rpc_identity = ethlambda_rpc::BeaconIdentity { + metadata: p2p.local_metadata(), + ..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 { diff --git a/crates/net/api/src/lib.rs b/crates/net/api/src/lib.rs index 29297ce9a..a0aa347d2 100644 --- a/crates/net/api/src/lib.rs +++ b/crates/net/api/src/lib.rs @@ -1,3 +1,4 @@ +use std::sync::{Arc, RwLock}; use std::time::Instant; use ethlambda_types::{ @@ -470,6 +471,50 @@ pub trait RpcToP2P: Send + Sync { ) -> Result<(), ActorError>; } +// --- Shared state: P2P -> RPC --- + +/// The beacon `MetaData` this node answers its peers with. +/// +/// The two bitfields are kept as their SSZ bytes, which is also what the +/// Beacon API's `/eth/v1/node/identity` reports (hex-encoded), so a reader +/// needs neither the subnet counts nor the bitvector types. +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub struct AdvertisedMetaData { + pub seq_number: u64, + /// `attnets`: the attestation subnets this node serves. + pub attnets: Vec, + /// `syncnets`: the sync committee subnets this node serves. + pub syncnets: Vec, +} + +/// Shared, runtime-readable copy of the node's [`AdvertisedMetaData`]. +/// +/// The p2p layer is the sole writer: it publishes whenever the `MetaData` it +/// answers peers with changes, which is also every time it bumps +/// `seq_number`. Read paths outside it, the Beacon API's +/// `/eth/v1/node/identity` in particular, clone the handle and read the +/// current value, so that endpoint reports what peers are told rather than a +/// copy taken at startup. +#[derive(Clone, Debug, Default)] +pub struct MetaDataController { + metadata: Arc>, +} + +impl MetaDataController { + /// Publish the current `MetaData`. + pub fn set(&self, metadata: AdvertisedMetaData) { + *self.metadata.write().expect("metadata lock poisoned") = metadata; + } + + /// Read the current `MetaData`. + pub fn get(&self) -> AdvertisedMetaData { + self.metadata + .read() + .expect("metadata lock poisoned") + .clone() + } +} + // --- Init messages --- // Used to wire actors together after spawn. diff --git a/crates/net/p2p/src/beacon/handler.rs b/crates/net/p2p/src/beacon/handler.rs index 8fa0692e8..2e566f2dc 100644 --- a/crates/net/p2p/src/beacon/handler.rs +++ b/crates/net/p2p/src/beacon/handler.rs @@ -7,9 +7,11 @@ //! [`build_status`] and [`build_metadata`] for the bodies they cannot construct //! themselves. +use ethlambda_network_api::AdvertisedMetaData; use ethlambda_storage::Store; use ethlambda_types::beacon::primitives::Root; use libp2p::PeerId; +use libssz::SszEncode; use tracing::debug; use super::messages::{ @@ -148,6 +150,16 @@ pub fn build_metadata(wire: &BeaconWire, protocol: &str) -> Option AdvertisedMetaData { + AdvertisedMetaData { + seq_number: wire.metadata_seq_number, + attnets: attnets(wire).to_ssz(), + syncnets: syncnets(wire).to_ssz(), + } +} + /// The `attnets` bitfield for this node's backbone subscription. /// /// A subnet id past the bitfield's width is dropped rather than wrapped: the @@ -254,6 +266,7 @@ mod tests { genesis_time: 1_606_824_023, genesis_validators_root: Root::ZERO, metadata_seq_number: 0, + metadata: Default::default(), custody_columns: Vec::new(), attestation_subnets: Vec::new(), sync_committee_subnets: Default::default(), diff --git a/crates/net/p2p/src/beacon/mod.rs b/crates/net/p2p/src/beacon/mod.rs index 70d7680c3..33944b4f7 100644 --- a/crates/net/p2p/src/beacon/mod.rs +++ b/crates/net/p2p/src/beacon/mod.rs @@ -32,6 +32,7 @@ pub mod verdict; use std::collections::BTreeMap; +use ethlambda_network_api::MetaDataController; use ethlambda_types::beacon::config::Config; use ethlambda_types::beacon::fork::ForkName; use ethlambda_types::beacon::primitives::{Epoch, ForkDigest, Root, Slot}; @@ -69,8 +70,9 @@ pub struct BeaconWire { /// epoch, so serving history means computing digests this node never runs /// on. See [`encoding`]'s module docs for the rule. pub genesis_validators_root: Root, - /// Advertised in `Ping` responses and in `MetaData`. Bumped when the - /// digest switches, the one advertised value that changes at runtime. + /// Advertised in `Ping` responses and in `MetaData`. Bumped through + /// [`Self::bump_metadata_seq_number`] when the digest switches and when + /// `syncnets` changes. pub metadata_seq_number: u64, /// The columns this node custodies, carried alongside `topics` so the /// gossip handler and the request handlers can check what this node @@ -88,9 +90,27 @@ pub struct BeaconWire { /// `build_metadata` derives `syncnets` from its keys. Never in the ENR, /// which cannot be replaced at runtime. pub sync_committee_subnets: BTreeMap, + /// The `MetaData` above as the Beacon API's `/eth/v1/node/identity` reads + /// it, republished by [`Self::bump_metadata_seq_number`]. + pub metadata: MetaDataController, } impl BeaconWire { + /// Bump `metadata_seq_number` after a change to what `MetaData` + /// advertises, and republish it for the Beacon API. + /// + /// The one way the sequence number moves, so the copy the Beacon API + /// reports cannot fall behind what peers are told. + pub fn bump_metadata_seq_number(&mut self) { + self.metadata_seq_number += 1; + self.publish_metadata(); + } + + /// Publish the current `MetaData` to [`Self::metadata`]'s readers. + pub fn publish_metadata(&self) { + self.metadata.set(handler::advertised_metadata(self)); + } + /// Every topic set this node is subscribed to: the current digest's, then /// the window's. pub fn held_topics(&self) -> impl Iterator { diff --git a/crates/net/p2p/src/beacon/sync_committee.rs b/crates/net/p2p/src/beacon/sync_committee.rs index a668d24a0..bfba3355a 100644 --- a/crates/net/p2p/src/beacon/sync_committee.rs +++ b/crates/net/p2p/src/beacon/sync_committee.rs @@ -153,7 +153,7 @@ fn join(server: &mut P2PServer, subnets: Vec<(u64, Epoch)>) -> Vec { } if !joined.is_empty() { // The advertised `syncnets` changed. - wire.metadata_seq_number += 1; + wire.bump_metadata_seq_number(); } subscribed } @@ -197,7 +197,7 @@ fn leave_expired(server: &mut P2PServer, epoch: Epoch) -> Vec { } } if !expired.is_empty() { - wire.metadata_seq_number += 1; + wire.bump_metadata_seq_number(); } unsubscribed } @@ -227,7 +227,7 @@ mod tests { use ethlambda_types::beacon::primitives::Root; use super::*; - use crate::beacon::handler::build_metadata; + use crate::beacon::handler::{advertised_metadata, build_metadata}; use crate::beacon::messages::BeaconMetaData; use crate::beacon::protocols; use crate::beacon::transition::apply; @@ -352,6 +352,28 @@ mod tests { assert_eq!(seq(&server), start + 3); } + /// The copy the Beacon API reports (`/eth/v1/node/identity`) is + /// republished with every change, so it never lags what a peer's + /// `MetaData` request is answered with. + #[tokio::test] + async fn the_beacon_api_copy_follows_every_change() { + let mut server = unconnected_beacon_server(Config::mainnet(), 0).await; + let in_step = |server: &P2PServer| { + let wire = server.wire.beacon().expect("beacon wire"); + wire.metadata.get() == advertised_metadata(wire) + }; + assert!(in_step(&server), "published at startup"); + + join(&mut server, vec![(1, 100)]); + assert!(in_step(&server)); + let published = server.wire.beacon().expect("beacon wire").metadata.get(); + assert_eq!(published.syncnets, vec![0b0000_0010]); + assert_eq!(published.seq_number, seq(&server)); + + leave_expired(&mut server, 100); + assert!(in_step(&server)); + } + #[tokio::test] async fn publishing_a_message_marks_each_subnet_seen() { let mut server = unconnected_beacon_server(Config::mainnet(), 0).await; diff --git a/crates/net/p2p/src/beacon/transition.rs b/crates/net/p2p/src/beacon/transition.rs index 9bc6f378f..38c11e599 100644 --- a/crates/net/p2p/src/beacon/transition.rs +++ b/crates/net/p2p/src/beacon/transition.rs @@ -225,7 +225,7 @@ pub(crate) fn apply(server: &mut P2PServer, epoch: Epoch) -> Changes { let previous = wire.fork_digest; wire.fork_digest = current.digest; wire.fork = current.fork; - wire.metadata_seq_number += 1; + wire.bump_metadata_seq_number(); let digest_hex = hex::encode(current.digest); metrics::set_beacon_fork_digest(&digest_hex); info!( diff --git a/crates/net/p2p/src/lib.rs b/crates/net/p2p/src/lib.rs index dc2f1f1fa..da435368e 100644 --- a/crates/net/p2p/src/lib.rs +++ b/crates/net/p2p/src/lib.rs @@ -38,7 +38,7 @@ use std::{ use either::Either; use ethlambda_network_api::{ - FetchRequest, InitBlockChain, P2PToBlockChainRef, + FetchRequest, InitBlockChain, MetaDataController, P2PToBlockChainRef, block_chain_to_p2p::{ CheckDataColumnSidecars, FetchBlock, PublishAggregatedAttestation, PublishAttestation, PublishBlock, @@ -1099,7 +1099,7 @@ pub fn build_swarm(config: SwarmConfig) -> Result { &beacon.config, beacon.genesis_validators_root, ); - Wire::Beacon(Box::new(beacon::BeaconWire { + let wire = beacon::BeaconWire { fork_digest: beacon.fork_digest, fork: beacon.fork, schedule, @@ -1112,7 +1112,10 @@ pub fn build_swarm(config: SwarmConfig) -> Result { custody_columns: beacon.custody_columns, attestation_subnets: beacon.attestation_subnets, sync_committee_subnets: std::collections::BTreeMap::new(), - })) + metadata: MetaDataController::default(), + }; + wire.publish_metadata(); + Wire::Beacon(Box::new(wire)) } }; @@ -1132,6 +1135,9 @@ pub struct P2P { /// 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, + /// The beacon `MetaData` this node answers peers with, for the same + /// endpoint. `None` on a lean wire, which has no `MetaData`. + local_metadata: Option, } impl P2P { @@ -1189,6 +1195,7 @@ impl P2P { .wire .beacon() .map(|beacon| beacon::scoring::PeerScoring::new(&beacon.config, beacon.fork_digest)); + let local_metadata = built.wire.beacon().map(|beacon| beacon.metadata.clone()); let server = P2PServer { swarm_handle, @@ -1276,7 +1283,11 @@ impl P2P { .send(p2p_protocol::AdvanceForkSchedule) .inspect_err(|err| warn!(%err, "Could not schedule the fork schedule")); spawn_listener(handle.context(), swarm_stream.map(WrappedSwarmEvent)); - Ok(P2P { handle, local_enr }) + Ok(P2P { + handle, + local_enr, + local_metadata, + }) } pub fn actor_ref(&self) -> &ActorRef { @@ -1290,6 +1301,12 @@ impl P2P { pub fn local_enr(&self) -> Option<&str> { self.local_enr.as_deref() } + + /// A live view of the beacon `MetaData` this node answers peers with, + /// updated whenever it changes. `None` on a lean wire. + pub fn local_metadata(&self) -> Option { + self.local_metadata.clone() + } } /// 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 aa2c0cabf..0f8081dec 100644 --- a/crates/net/rpc/src/beacon/node.rs +++ b/crates/net/rpc/src/beacon/node.rs @@ -102,20 +102,35 @@ async fn get_version(version: &'static str) -> Response { /// /// 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. +/// the ENR carries no IP. +/// +/// `metadata` is the `MetaData` peers get over req/resp, read at request time. +/// Its `seq_number` versions that `MetaData` alone, so it is not the ENR's +/// `seq`: the p2p specification counts the two separately. async fn get_identity(peer_id: String, identity: Option) -> Response { let identity = identity.unwrap_or_default(); + let metadata = match identity.metadata { + Some(metadata) => { + let metadata = metadata.get(); + serde_json::json!({ + "seq_number": metadata.seq_number.to_string(), + "attnets": format!("0x{}", hex::encode(&metadata.attnets)), + "syncnets": format!("0x{}", hex::encode(&metadata.syncnets)), + }) + } + None => serde_json::json!({ + "seq_number": "0", + "attnets": "0x0000000000000000", + "syncnets": "0x00", + }), + }; crate::json_response(serde_json::json!({ "data": { "peer_id": peer_id, "enr": identity.enr, "p2p_addresses": identity.p2p_addresses, "discovery_addresses": identity.discovery_addresses, - "metadata": { - "seq_number": "0", - "attnets": "0x0000000000000000", - "syncnets": "0x00", - }, + "metadata": metadata, } })) } @@ -242,6 +257,7 @@ mod tests { 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()], + metadata: None, }; let app = routes("ethlambda/test", "test-peer".into()) .with_state(fixture.store) @@ -261,4 +277,34 @@ mod tests { "/ip4/10.0.0.1/udp/9000/p2p/test-peer" ); } + + /// `metadata` is read from the p2p layer's handle on each request, so a + /// change published after the router was built is what the next answer + /// reports. + #[tokio::test] + async fn identity_reports_the_metadata_peers_are_told() { + let fixture = beacon_fixture(ANCHOR_SLOT); + let metadata = ethlambda_network_api::MetaDataController::default(); + let identity = crate::BeaconIdentity { + metadata: Some(metadata.clone()), + ..Default::default() + }; + let app = routes("ethlambda/test", "test-peer".into()) + .with_state(fixture.store) + .layer(Extension(identity)); + + metadata.set(ethlambda_network_api::AdvertisedMetaData { + seq_number: 3, + attnets: vec![0x00, 0x10, 0, 0, 0, 0, 0, 0x01], + syncnets: vec![0x04], + }); + 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"]["metadata"]["seq_number"], "3"); + assert_eq!(json["data"]["metadata"]["attnets"], "0x0010000000000001"); + assert_eq!(json["data"]["metadata"]["syncnets"], "0x04"); + } } diff --git a/crates/net/rpc/src/lib.rs b/crates/net/rpc/src/lib.rs index 9173f0ac4..ec0397a63 100644 --- a/crates/net/rpc/src/lib.rs +++ b/crates/net/rpc/src/lib.rs @@ -202,6 +202,10 @@ pub struct BeaconIdentity { pub p2p_addresses: Vec, /// discv5 multiaddrs, likewise. pub discovery_addresses: Vec, + /// The `MetaData` the p2p layer answers peers with, read per request + /// since its sequence number and `syncnets` change at runtime. `None` + /// without a beacon p2p layer, which leaves `metadata` all-zero. + pub metadata: Option, } /// What the Beacon API's endpoints reach beyond the store. diff --git a/docs/rpc.md b/docs/rpc.md index 04d725892..bdcc260ec 100644 --- a/docs/rpc.md +++ b/docs/rpc.md @@ -843,7 +843,10 @@ state. A slot the store holds nothing at is still a `404`. 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. +no IP. `metadata` is the `MetaData` the node answers peers with over req/resp, +read at request time, so `syncnets` and `seq_number` follow the sync committee +subnets a validator client asks for. That `seq_number` versions `MetaData` +alone: the p2p specification counts it separately from the ENR's `seq`. ### `GET /eth/v1/events` From 9b41158590f60b9ad247a1ab7bcf39870bb4cf3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Wed, 28 Oct 2026 19:20:08 -0300 Subject: [PATCH 4/4] fix(storage): index the beacon anchor's slot in BlockRoots `/eth/v1/beacon/headers/head` answered `canonical: false` while the head was still the checkpoint anchor. `update_checkpoints` keeps `BlockRoots` by diffing the old head against the new one, which never writes the block both start from, and at bootstrap that is the anchor. Lean's `init_store` writes the anchor's row itself; the beacon bootstrap did not. `get_forkchoice_store` now calls `Store::index_beacon_anchor`, which keys the row by the anchor block's slot rather than the anchor state's: a checkpoint state sits at an epoch's first slot, after its latest block whenever that slot was empty. BlocksByRange now serves the anchor block too, which BlocksByRoot already did. Directories bootstrapped before this change still have no row for their anchor, and keep relying on `anchored_root_at_slot` for slot lookups. --- .../src/beacon/fork_choice.rs | 5 ++ crates/net/rpc/src/beacon/headers.rs | 49 ++++++++---- crates/net/rpc/src/lib.rs | 12 ++- crates/net/rpc/src/shared/block_id.rs | 24 +++--- crates/storage/src/store.rs | 76 ++++++++++++++++++- docs/data_storage.md | 11 ++- 6 files changed, 143 insertions(+), 34 deletions(-) diff --git a/crates/blockchain/state_transition/src/beacon/fork_choice.rs b/crates/blockchain/state_transition/src/beacon/fork_choice.rs index 068cd60a7..02c959376 100644 --- a/crates/blockchain/state_transition/src/beacon/fork_choice.rs +++ b/crates/blockchain/state_transition/src/beacon/fork_choice.rs @@ -1282,6 +1282,11 @@ pub fn get_forkchoice_store( store .insert_signed_block(anchor_root, anchor_block) .expect("insert"); + // The anchor heads the canonical chain, but the head-diff that maintains + // `BlockRoots` never writes the block it starts from. + store + .index_beacon_anchor(anchor_root) + .expect("index anchor"); // Only the hash, not a verdict: the specification's `is_execution_block` // reads the block body, and the anchor is the one block that never goes // through `on_block`, which is what fills this cache. Without it the first diff --git a/crates/net/rpc/src/beacon/headers.rs b/crates/net/rpc/src/beacon/headers.rs index de3a178f6..8f68dc3dd 100644 --- a/crates/net/rpc/src/beacon/headers.rs +++ b/crates/net/rpc/src/beacon/headers.rs @@ -82,11 +82,12 @@ async fn get_header(Path(block_id): Path, State(store): State) -> #[cfg(test)] mod tests { use super::*; - use crate::test_utils::beacon_fixture; + use crate::test_utils::{beacon_fixture, phase0_beacon_block}; use axum::{ body::Body, http::{Request, StatusCode}, }; + use ethlambda_types::beacon::containers::SignedBeaconBlock; use http_body_util::BodyExt as _; use tower::ServiceExt as _; @@ -141,26 +142,48 @@ mod tests { ); } - /// `canonical` is read off the index rather than assumed. The anchor is - /// reachable by root but is not in `BlockRoots`, so it answers `false` - /// while the head answers `true`. + /// `canonical` is read off the index rather than assumed. The anchor's + /// slot is indexed at bootstrap (`Store::index_beacon_anchor`), so it + /// answers `true` like the head does, and a root the index does not name + /// at its slot answers `false`. #[tokio::test] async fn canonical_is_checked_against_the_index() { let head = body_json(get("/eth/v1/beacon/headers/head".to_string()).await).await; assert_eq!(head["data"]["canonical"], true); let fixture = beacon_fixture(ANCHOR_SLOT); - let uri = format!("/eth/v1/beacon/headers/0x{:x}", fixture.anchor_root); - let app = routes().with_state(fixture.store); - let response = app - .oneshot(Request::builder().uri(uri).body(Body::empty()).unwrap()) - .await - .unwrap(); - let anchor = body_json(response).await; + let mut store = fixture.store; + // A sibling of the head: stored, but not on the branch the head ends. + let SignedBeaconBlock::Phase0(mut sibling) = + phase0_beacon_block(fixture.head_slot, fixture.anchor_root) + else { + unreachable!("the helper builds a phase0 block"); + }; + sibling.message.proposer_index = 1; + let sibling = SignedBeaconBlock::Phase0(sibling); + let sibling_root = sibling.message_hash_tree_root(); + store + .insert_signed_block(sibling_root, sibling) + .expect("insert sibling"); + + let app = routes().with_state(store); + let canonical = |root: H256| { + let app = app.clone(); + async move { + let uri = format!("/eth/v1/beacon/headers/0x{root:x}"); + let response = app + .oneshot(Request::builder().uri(uri).body(Body::empty()).unwrap()) + .await + .unwrap(); + body_json(response).await["data"]["canonical"].clone() + } + }; assert_eq!( - anchor["data"]["canonical"], false, - "the anchor's slot is not in BlockRoots, so the index cannot vouch for it" + canonical(fixture.anchor_root).await, + true, + "the anchor heads the canonical chain" ); + assert_eq!(canonical(sibling_root).await, false); } #[tokio::test] diff --git a/crates/net/rpc/src/lib.rs b/crates/net/rpc/src/lib.rs index ec0397a63..4870c2781 100644 --- a/crates/net/rpc/src/lib.rs +++ b/crates/net/rpc/src/lib.rs @@ -857,10 +857,11 @@ pub(crate) mod test_utils { /// Build a beacon store anchored at `anchor_slot`, with a real child block /// at `anchor_slot + 1` whose import moves the head for real. /// - /// `Table::BlockRoots` is written only by `Store::update_checkpoints` - /// (see `BlockId::resolve_beacon`'s doc), so a fixture that wants that - /// table populated has to move the head through the real API rather than - /// poke the backend directly. That is what distinguishes this from + /// `Table::BlockRoots` is written only by `Store::index_beacon_anchor` + /// and `Store::update_checkpoints`, so a fixture that wants that table + /// populated has to index the anchor and move the head through the real + /// API, the way `get_forkchoice_store` and an import do, rather than poke + /// the backend directly. That is what distinguishes this from /// `beacon_test_state`/`beacon_test_block` in `ethlambda_storage::store`'s /// own tests, which this otherwise mirrors. pub(crate) fn beacon_fixture(anchor_slot: u64) -> BeaconFixture { @@ -882,6 +883,9 @@ pub(crate) mod test_utils { store .insert_signed_block(anchor_root, anchor_block) .expect("insert anchor block"); + store + .index_beacon_anchor(anchor_root) + .expect("index anchor block"); store .insert_state(anchor_root, BeaconState::Phase0(anchor_state)) .expect("insert anchor state"); diff --git a/crates/net/rpc/src/shared/block_id.rs b/crates/net/rpc/src/shared/block_id.rs index 04f2ed420..a17b2c63e 100644 --- a/crates/net/rpc/src/shared/block_id.rs +++ b/crates/net/rpc/src/shared/block_id.rs @@ -13,12 +13,12 @@ use ethlambda_types::primitives::H256; /// The block at `slot` among the roots this store can name, or `None`. /// -/// `Table::BlockRoots` is the slot index, and `Store::update_checkpoints` is -/// its only writer. That writer diffs the old head against the new one and -/// writes nothing when they are the same root, which is the situation at -/// bootstrap: `Store::init_beacon` seeds `KEY_HEAD` with the anchor. So the -/// anchor's own slot is missing from the index even though its block is on -/// disk, and `canonical_root_at_slot` alone cannot find it. +/// `Table::BlockRoots` is the slot index. `Store::update_checkpoints` keeps +/// it by diffing the old head against the new one, which never writes the +/// block they start from, so the anchor's own slot is indexed separately, by +/// `Store::index_beacon_anchor` at bootstrap. A directory bootstrapped before +/// that existed has no row for its anchor even though the block is on disk, +/// and `canonical_root_at_slot` alone cannot find it there. /// /// That slot is not a curiosity. `bin/ethlambda/src/checkpoint_sync.rs` reads /// a peer's finalized state, takes the anchor slot from it, and asks that peer @@ -116,14 +116,10 @@ impl BlockId { .latest_justified() .map_err(|_| IdError::NotFound)? .root), - // A beacon store cannot answer `genesis`. `BlockRoots` indexes the - // canonical branch above the store's anchor, and the anchor's own - // slot is never written to it: `update_checkpoints` is that - // index's only writer and it walks from the old head to the new - // one, which at bootstrap are the same root. A checkpoint-synced - // directory has no genesis block to return either way. Refusing - // is better than resolving it to the anchor and calling that - // genesis. + // A beacon store does not answer `genesis`: `BlockRoots` starts at + // the store's anchor, and a checkpoint-synced directory has no + // genesis block to return. Refusing is better than resolving it to + // the anchor and calling that genesis. BlockId::Genesis => Err(IdError::NotFound), BlockId::Slot(slot) => { if let Some(root) = store diff --git a/crates/storage/src/store.rs b/crates/storage/src/store.rs index 5827e047c..14ac9116f 100644 --- a/crates/storage/src/store.rs +++ b/crates/storage/src/store.rs @@ -3129,8 +3129,10 @@ impl Store { /// /// A `None` covers two cases the index cannot tell apart: a slot the /// canonical chain skipped, and a slot below the anchor this store was - /// bootstrapped from. Callers that use this to *reject* something must treat - /// `None` as "unknown" rather than "not canonical". + /// bootstrapped from (or, on a beacon directory bootstrapped before + /// [`index_beacon_anchor`](Self::index_beacon_anchor) existed, the + /// anchor's own slot). Callers that use this to *reject* something must + /// treat `None` as "unknown" rather than "not canonical". pub fn canonical_root_at_slot(&self, slot: u64) -> Result, Error> { let view = self.backend.begin_read().expect("read view"); Ok(view @@ -3140,6 +3142,33 @@ impl Store { .expect("read block root")) } + /// Write a beacon anchor's own slot into the canonical `BlockRoots` index. + /// + /// [`update_checkpoints`](Self::update_checkpoints), the index's other + /// writer, records the blocks between the old head and the new one but + /// never their common ancestor, and at bootstrap the head *is* the anchor. + /// Without this row the anchor would be the one canonical block the index + /// never names, so `/eth/v1/beacon/headers/head` answered `canonical: false` + /// until the first import moved the head. Lean's `init_store` writes the + /// same row. + /// + /// Keyed by the block's own slot, read back from the block row the caller + /// has already written, rather than by the anchor state's: a checkpoint + /// state sits at an epoch's first slot, which is after its latest block + /// whenever that slot was empty. + pub fn index_beacon_anchor(&mut self, root: H256) -> Result<(), Error> { + let (slot, _parent_root) = self + .block_entry(&root) + .ok_or(Error::UnexpectedMissingBlockHeader(root))?; + let entries = vec![(encode_block_root_key(slot), root.to_ssz())]; + let mut batch = self.backend.begin_write().expect("write batch"); + batch + .put_batch(Table::BlockRoots, entries) + .expect("put anchor block root"); + batch.commit().expect("commit"); + Ok(()) + } + /// Return canonical signed blocks for the slot range `[start_slot, end_slot]`. /// /// Missing slots or blocks are skipped, which is what both chains' @@ -6210,6 +6239,49 @@ mod tests { assert_eq!(resumed.head().expect("head"), r1); } + /// The anchor's `BlockRoots` row is keyed by its block's slot. A checkpoint + /// state at an epoch's first slot sits above its latest block whenever + /// that slot was empty, and the state's slot would index the anchor at a + /// slot it was not proposed in. + #[test] + fn index_beacon_anchor_keys_the_row_by_the_blocks_slot() { + let anchor_root = H256::repeat_byte(0xaa); + let (block_slot, state_slot) = (30, 32); + let mut store = Store::init_beacon( + Arc::new(InMemoryBackend::new()), + 0, + Config::mainnet(), + anchor_root, + Checkpoint { + root: anchor_root, + slot: state_slot, + }, + state_slot, + ); + let anchor_block = beacon_test_block(block_slot, H256::repeat_byte(0xbb)); + store + .insert_signed_block(anchor_root, anchor_block) + .expect("insert anchor block"); + assert_eq!( + store.canonical_root_at_slot(block_slot).expect("read"), + None, + "inserting a block does not index it" + ); + + store + .index_beacon_anchor(anchor_root) + .expect("index anchor"); + + assert_eq!( + store.canonical_root_at_slot(block_slot).expect("read"), + Some(anchor_root) + ); + assert_eq!( + store.canonical_root_at_slot(state_slot).expect("read"), + None + ); + } + /// The common case: nothing for `repair_head` to do when the recorded /// head already has a persisted state. Same head back, and the slot-1 /// `BlockRoots` entry survives untouched, which is what proves no rewind diff --git a/docs/data_storage.md b/docs/data_storage.md index a88050a8b..0f8d62db8 100644 --- a/docs/data_storage.md +++ b/docs/data_storage.md @@ -233,6 +233,13 @@ that leave the canonical chain and writing the ones that join it. A reorg therefore touches only the affected slot range, not the whole table. Never pruned. +That walk never writes the common ancestor itself, and at bootstrap the head +is the anchor, so the anchor's own slot is written separately: by `init_store` +on a lean directory, and by `index_beacon_anchor` (called from the beacon +`get_forkchoice_store`) on a beacon one, keyed by the anchor block's slot +rather than the anchor state's. A beacon directory bootstrapped before +`index_beacon_anchor` existed has no row for its anchor. + Backs `get_signed_blocks_by_slot_range`, which serves BlocksByRange requests over req/resp (`crates/net/p2p/src/req_resp/handlers.rs`). It does **not** back the RPC `GET /lean/v0/blocks/:slot` endpoint: that handler resolves a @@ -876,7 +883,9 @@ entries are written by its caller instead, through the same `insert_state` and `insert_signed_block` an ordinary block import uses, followed by the anchor's own `BeaconUnrealizedJustifications` row (`set_unrealized_justification`), the same order every later block's import follows too — see -[BeaconUnrealizedJustifications](#beaconunrealizedjustifications) above. +[BeaconUnrealizedJustifications](#beaconunrealizedjustifications) above. The +one row an import does not write is the anchor's `BlockRoots` entry, which +`index_beacon_anchor` adds right after the block (see [BlockRoots](#blockroots)). `from_db_state` is the restore path: it reads `db_version`, `preset`, `chain`, `config` and `anchor_slot` from `Metadata`, returning `None` for an empty DB. A format