Skip to content

Commit 75126cc

Browse files
conacheMegaRedHand
andauthored
refactor(p2p): move node name registry into P2PServer (lambdaclass#342)
## 🗒️ Description / Motivation The `PeerId -> node_name` lookup lived in a global `RwLock<HashMap<PeerId, &'static str>>` populated once at startup. This PR moves it into `P2PServer` so the actor owns its own state. ## What Changed - **`crates/net/p2p/src/lib.rs`**: the registry is now a field on the P2P actor's state struct, with a small helper to resolve a peer's name. - **`crates/net/p2p/src/metrics.rs`**: no longer owns the registry — its peer-event helpers take the resolved name directly. - **`bin/ethlambda/src/main.rs`**: the loader now returns the map and hands it off to `P2P::spawn` at startup; it also logs how many entries it loaded. ## Correctness / Behavior Guarantees - Metric label values (`lean_connected_peers{client="..."}`) are unchanged for valid configs. - Bad rows in `validator-config.yaml` (unparseable secp256k1 privkeys) used to be silently dropped; they now emit a `warn!` per drop and are still skipped. No new panics, no new failure paths. - No consensus-layer impact (fork choice, attestations, STF, signatures untouched). ## Tests Added / Run - [x] `make fmt` - [x] `make lint` - [x] `make test` ## Related Issues / PRs - Closes lambdaclass#106 --------- Co-authored-by: Tomás Grüner <47506558+MegaRedHand@users.noreply.github.com>
1 parent 80f5f21 commit 75126cc

4 files changed

Lines changed: 88 additions & 68 deletions

File tree

bin/ethlambda/src/main.rs

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ use tokio_util::sync::CancellationToken;
2121
use clap::Parser;
2222
use ethlambda_blockchain::key_manager::ValidatorKeyPair;
2323
use ethlambda_network_api::{InitBlockChain, InitP2P, ToBlockChainToP2PRef, ToP2PToBlockChainRef};
24-
use ethlambda_p2p::{Bootnode, P2P, SwarmConfig, build_swarm, parse_enrs};
24+
use ethlambda_p2p::{Bootnode, P2P, PeerId, SwarmConfig, build_swarm, parse_enrs};
2525
use ethlambda_types::primitives::H256;
2626
use ethlambda_types::{
2727
aggregator::AggregatorController,
@@ -171,7 +171,7 @@ async fn main() -> eyre::Result<()> {
171171
);
172172

173173
let validator_config_file = read_validator_config_file(&validator_config);
174-
populate_name_registry(&validator_config_file);
174+
let node_names = load_node_names(&validator_config_file);
175175

176176
// Resolve attestation_committee_count: CLI flag > validator-config.yaml > 1.
177177
// The CLI path is bounded by clap's `range(1..)`; enforce the same lower
@@ -235,7 +235,7 @@ async fn main() -> eyre::Result<()> {
235235
})
236236
.expect("failed to build swarm");
237237

238-
let p2p = P2P::spawn(built, store.clone());
238+
let p2p = P2P::spawn(built, store.clone(), node_names);
239239

240240
// Wire actors together via protocol refs
241241
blockchain
@@ -336,7 +336,7 @@ async fn run_test_driver(rpc_config: RpcConfig) -> eyre::Result<()> {
336336
///
337337
/// The `config` block is a network-wide settings bag shared across clients;
338338
/// only fields ethlambda actually reads are deserialized. The `validators`
339-
/// list feeds the metrics name registry.
339+
/// list feeds the node-name registry passed to `P2P::spawn`.
340340
#[derive(Debug, Deserialize)]
341341
struct ValidatorConfigFile {
342342
#[serde(default)]
@@ -361,15 +361,14 @@ fn read_validator_config_file(path: impl AsRef<Path>) -> ValidatorConfigFile {
361361
serde_yaml_ng::from_str(&yaml).expect("Failed to parse validator config file")
362362
}
363363

364-
fn populate_name_registry(file: &ValidatorConfigFile) {
364+
fn load_node_names(file: &ValidatorConfigFile) -> HashMap<PeerId, String> {
365365
let names_and_privkeys = file
366366
.validators
367367
.iter()
368368
.map(|v| (v.name.clone(), v.privkey))
369369
.collect();
370370

371-
// Populates a dictionary used for labeling metrics with node names
372-
ethlambda_p2p::metrics::populate_name_registry(names_and_privkeys);
371+
ethlambda_p2p::derive_peer_ids(names_and_privkeys)
373372
}
374373

375374
fn read_bootnodes(bootnodes_path: impl AsRef<Path>) -> Vec<Bootnode> {

crates/net/p2p/src/lib.rs

Lines changed: 60 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,9 @@ use ethrex_p2p::types::NodeRecord;
1717
use ethrex_rlp::decode::RLPDecode;
1818
use futures::StreamExt;
1919
use libp2p::{
20-
Multiaddr, PeerId, StreamProtocol,
20+
Multiaddr, StreamProtocol,
2121
gossipsub::{MessageAuthenticity, ValidationMode},
22-
identity::{PublicKey, secp256k1},
22+
identity::{Keypair, PublicKey, secp256k1},
2323
multiaddr::Protocol,
2424
request_response::{self, OutboundRequestId},
2525
swarm::{NetworkBehaviour, SwarmEvent},
@@ -51,7 +51,7 @@ pub mod metrics;
5151
mod req_resp;
5252
pub(crate) mod swarm_adapter;
5353

54-
pub use metrics::populate_name_registry;
54+
pub use libp2p::PeerId;
5555

5656
// 5ms, 10ms, 20ms, 40ms, 80ms, 160ms, 320ms, 640ms, 1280ms, 2560ms
5757
const MAX_FETCH_RETRIES: u32 = 10;
@@ -282,8 +282,9 @@ pub struct P2P {
282282

283283
impl P2P {
284284
/// Build swarm, start I/O adapter, spawn actor, and wire the swarm event stream.
285-
pub fn spawn(built: BuiltSwarm, store: Store) -> P2P {
286-
let (swarm_stream, swarm_handle) = swarm_adapter::start_swarm_adapter(built.swarm);
285+
pub fn spawn(built: BuiltSwarm, store: Store, node_names: HashMap<PeerId, String>) -> P2P {
286+
let (swarm_stream, swarm_handle) =
287+
swarm_adapter::start_swarm_adapter(built.swarm, node_names.clone());
287288

288289
let server = P2PServer {
289290
swarm_handle,
@@ -297,6 +298,7 @@ impl P2P {
297298
pending_requests: HashMap::new(),
298299
request_id_map: HashMap::new(),
299300
bootnode_addrs: built.bootnode_addrs,
301+
node_names,
300302
};
301303
let handle = server.start();
302304
spawn_listener(handle.context(), swarm_stream.map(WrappedSwarmEvent));
@@ -332,6 +334,16 @@ pub struct P2PServer {
332334
pub(crate) pending_requests: HashMap<H256, PendingRequest>,
333335
pub(crate) request_id_map: HashMap<OutboundRequestId, H256>,
334336
bootnode_addrs: HashMap<PeerId, Multiaddr>,
337+
node_names: HashMap<PeerId, String>,
338+
}
339+
340+
impl P2PServer {
341+
fn resolve_node_name(&self, peer_id: Option<&PeerId>) -> &str {
342+
peer_id
343+
.and_then(|p| self.node_names.get(p))
344+
.map(String::as_str)
345+
.unwrap_or("unknown")
346+
}
335347
}
336348

337349
// Protocol trait for internal messages only (retry scheduling).
@@ -459,7 +471,11 @@ async fn handle_swarm_event(
459471
if num_established.get() == 1 {
460472
server.connected_peers.insert(peer_id);
461473
let peer_count = server.connected_peers.len();
462-
metrics::notify_peer_connected(&Some(peer_id), direction, "success");
474+
metrics::notify_peer_connected(
475+
server.resolve_node_name(Some(&peer_id)),
476+
direction,
477+
"success",
478+
);
463479
// Send status request on first connection to this peer
464480
let our_status = build_status(&server.store);
465481
let our_finalized_slot = our_status.finalized.slot;
@@ -512,7 +528,11 @@ async fn handle_swarm_event(
512528
if num_established == 0 {
513529
server.connected_peers.remove(&peer_id);
514530
let peer_count = server.connected_peers.len();
515-
metrics::notify_peer_disconnected(&Some(peer_id), direction, reason);
531+
metrics::notify_peer_disconnected(
532+
server.resolve_node_name(Some(&peer_id)),
533+
direction,
534+
reason,
535+
);
516536

517537
info!(
518538
%peer_id,
@@ -541,7 +561,11 @@ async fn handle_swarm_event(
541561
} else {
542562
"error"
543563
};
544-
metrics::notify_peer_connected(&peer_id, "outbound", result);
564+
metrics::notify_peer_connected(
565+
server.resolve_node_name(peer_id.as_ref()),
566+
"outbound",
567+
result,
568+
);
545569
warn!(?peer_id, %error, "Outgoing connection error");
546570

547571
// Schedule redial if this was a bootnode
@@ -558,7 +582,11 @@ async fn handle_swarm_event(
558582
}
559583
}
560584
SwarmEvent::IncomingConnectionError { peer_id, error, .. } => {
561-
metrics::notify_peer_connected(&peer_id, "inbound", "error");
585+
metrics::notify_peer_connected(
586+
server.resolve_node_name(peer_id.as_ref()),
587+
"inbound",
588+
"error",
589+
);
562590
warn!(%error, "Incoming connection error");
563591
}
564592
_ => {
@@ -567,6 +595,29 @@ async fn handle_swarm_event(
567595
}
568596
}
569597

598+
// --- Node identity helpers ---
599+
600+
/// Derive each entry's `PeerId` from its secp256k1 private key.
601+
///
602+
/// Drops entries whose key fails to parse, with a `warn!` per drop.
603+
pub fn derive_peer_ids(names_and_privkeys: HashMap<String, H256>) -> HashMap<PeerId, String> {
604+
names_and_privkeys
605+
.into_iter()
606+
.filter_map(|(name, mut privkey)| {
607+
match secp256k1::SecretKey::try_from_bytes(&mut privkey.0) {
608+
Ok(privkey) => {
609+
let pubkey = Keypair::from(secp256k1::Keypair::from(privkey)).public();
610+
Some((PeerId::from_public_key(&pubkey), name))
611+
}
612+
Err(err) => {
613+
warn!(%name, %err, "Skipping node-name registry entry: invalid secp256k1 privkey");
614+
None
615+
}
616+
}
617+
})
618+
.collect()
619+
}
620+
570621
// --- Bootnode parsing ---
571622

572623
pub struct Bootnode {

crates/net/p2p/src/metrics.rs

Lines changed: 17 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1,45 +1,10 @@
11
//! Prometheus metrics for the P2P network layer.
22
3-
use std::{
4-
collections::HashMap,
5-
sync::{LazyLock, RwLock},
6-
};
3+
use std::collections::HashMap;
4+
use std::sync::LazyLock;
75

86
use ethlambda_metrics::*;
9-
use ethlambda_types::primitives::H256;
10-
use libp2p::{
11-
PeerId,
12-
identity::{Keypair, secp256k1},
13-
};
14-
15-
static NODE_NAME_REGISTRY: LazyLock<RwLock<HashMap<PeerId, &'static str>>> =
16-
LazyLock::new(|| RwLock::new(HashMap::new()));
17-
18-
pub fn populate_name_registry(names_and_privkeys: HashMap<String, H256>) {
19-
let mut registry = NODE_NAME_REGISTRY.write().unwrap();
20-
*registry = names_and_privkeys
21-
.into_iter()
22-
.filter_map(|(name, mut privkey)| {
23-
let Ok(privkey) = secp256k1::SecretKey::try_from_bytes(&mut privkey.0) else {
24-
return None;
25-
};
26-
let pubkey = Keypair::from(secp256k1::Keypair::from(privkey)).public();
27-
let peer_id = PeerId::from_public_key(&pubkey);
28-
// NOTE: we leak the name string to get a 'static lifetime.
29-
// In reality, the name registry is not expected to be read, so it should be safe
30-
// to turn these strings to &'static str.
31-
Some((peer_id, &*name.leak()))
32-
})
33-
.collect();
34-
}
35-
36-
fn resolve(peer_id: &Option<PeerId>) -> &'static str {
37-
let registry = NODE_NAME_REGISTRY.read().unwrap();
38-
peer_id
39-
.as_ref()
40-
.and_then(|peer_id| registry.get(peer_id))
41-
.unwrap_or(&"unknown")
42-
}
7+
use libp2p::PeerId;
438

449
static LEAN_CONNECTED_PEERS: LazyLock<IntGaugeVec> = LazyLock::new(|| {
4510
register_int_gauge_vec!(
@@ -237,38 +202,39 @@ pub fn set_attestation_committee_subnet(subnet_id: u64) {
237202
///
238203
/// If `result` is "success", the connected peer count is incremented.
239204
/// The connection event counter is always incremented.
240-
pub fn notify_peer_connected(peer_id: &Option<PeerId>, direction: &str, result: &str) {
205+
pub fn notify_peer_connected(node_name: &str, direction: &str, result: &str) {
241206
LEAN_PEER_CONNECTION_EVENTS_TOTAL
242207
.with_label_values(&[direction, result])
243208
.inc();
244209

245210
if result == "success" {
246-
let name = resolve(peer_id);
247-
LEAN_CONNECTED_PEERS.with_label_values(&[name]).inc();
211+
LEAN_CONNECTED_PEERS.with_label_values(&[node_name]).inc();
248212
}
249213
}
250214

251215
/// Notify that a peer disconnected.
252216
///
253217
/// Decrements the connected peer count and increments the disconnection event counter.
254-
pub fn notify_peer_disconnected(peer_id: &Option<PeerId>, direction: &str, reason: &str) {
218+
pub fn notify_peer_disconnected(node_name: &str, direction: &str, reason: &str) {
255219
LEAN_PEER_DISCONNECTION_EVENTS_TOTAL
256220
.with_label_values(&[direction, reason])
257221
.inc();
258222

259-
let name = resolve(peer_id);
260-
LEAN_CONNECTED_PEERS.with_label_values(&[name]).dec();
223+
LEAN_CONNECTED_PEERS.with_label_values(&[node_name]).dec();
261224
}
262225

263226
/// Refresh the gossipsub mesh peers gauge from the current mesh peer set.
264-
pub fn update_gossip_mesh_peers<'a>(peers: impl Iterator<Item = &'a PeerId>) {
227+
pub fn update_gossip_mesh_peers<'a>(
228+
peers: impl Iterator<Item = &'a PeerId>,
229+
node_names: &HashMap<PeerId, String>,
230+
) {
265231
let mut counts: HashMap<String, i64> = HashMap::new();
266-
{
267-
let registry = NODE_NAME_REGISTRY.read().unwrap();
268-
for peer_id in peers {
269-
let name = registry.get(peer_id).copied().unwrap_or("unknown");
270-
*counts.entry(name.to_string()).or_default() += 1;
271-
}
232+
for peer_id in peers {
233+
let name = node_names
234+
.get(peer_id)
235+
.map(String::as_str)
236+
.unwrap_or("unknown");
237+
*counts.entry(name.to_string()).or_default() += 1;
272238
}
273239
// Seed previously-published labels with 0 so departed clients fall to
274240
// zero in the single set() pass below.

crates/net/p2p/src/swarm_adapter.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use std::collections::HashMap;
12
use std::time::Duration;
23

34
use libp2p::{
@@ -92,14 +93,15 @@ impl SwarmHandle {
9293

9394
pub fn start_swarm_adapter(
9495
swarm: libp2p::Swarm<Behaviour>,
96+
node_names: HashMap<PeerId, String>,
9597
) -> (
9698
impl futures::Stream<Item = SwarmEvent<BehaviourEvent>>,
9799
SwarmHandle,
98100
) {
99101
let (event_tx, event_rx) = mpsc::unbounded_channel();
100102
let (cmd_tx, cmd_rx) = mpsc::unbounded_channel();
101103

102-
tokio::spawn(swarm_loop(swarm, event_tx, cmd_rx));
104+
tokio::spawn(swarm_loop(swarm, event_tx, cmd_rx, node_names));
103105

104106
let stream = tokio_stream::wrappers::UnboundedReceiverStream::new(event_rx);
105107
let handle = SwarmHandle { cmd_tx };
@@ -110,6 +112,7 @@ async fn swarm_loop(
110112
mut swarm: libp2p::Swarm<Behaviour>,
111113
event_tx: mpsc::UnboundedSender<SwarmEvent<BehaviourEvent>>,
112114
mut cmd_rx: mpsc::UnboundedReceiver<SwarmCommand>,
115+
node_names: HashMap<PeerId, String>,
113116
) {
114117
let mut mesh_metric_tick = tokio::time::interval(MESH_METRIC_REFRESH_INTERVAL);
115118
mesh_metric_tick.set_missed_tick_behavior(MissedTickBehavior::Skip);
@@ -126,6 +129,7 @@ async fn swarm_loop(
126129
_ = mesh_metric_tick.tick() => {
127130
metrics::update_gossip_mesh_peers(
128131
swarm.behaviour().gossipsub.all_mesh_peers(),
132+
&node_names,
129133
);
130134
}
131135
}

0 commit comments

Comments
 (0)