From 4a1c0c96be26f287f435439679cabe4e98b805b9 Mon Sep 17 00:00:00 2001 From: Alex Wied Date: Mon, 13 Mar 2023 01:40:01 +0000 Subject: [PATCH 1/6] Fix order of arguments to log_count (#4060) See: https://github.com/sigp/lighthouse/pull/4027 ## Proposed Changes The order of the arguments to `log_count` is swapped in `beacon_node/beacon_chain/src/events.rs`. --- beacon_node/beacon_chain/src/events.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/beacon_node/beacon_chain/src/events.rs b/beacon_node/beacon_chain/src/events.rs index b3fa6627f..fed050323 100644 --- a/beacon_node/beacon_chain/src/events.rs +++ b/beacon_node/beacon_chain/src/events.rs @@ -65,43 +65,43 @@ impl ServerSentEventHandler { EventKind::Attestation(_) => self .attestation_tx .send(kind) - .map(|count| log_count(count, "attestation")), + .map(|count| log_count("attestation", count)), EventKind::Block(_) => self .block_tx .send(kind) - .map(|count| log_count(count, "block")), + .map(|count| log_count("block", count)), EventKind::FinalizedCheckpoint(_) => self .finalized_tx .send(kind) - .map(|count| log_count(count, "finalized checkpoint")), + .map(|count| log_count("finalized checkpoint", count)), EventKind::Head(_) => self .head_tx .send(kind) - .map(|count| log_count(count, "head")), + .map(|count| log_count("head", count)), EventKind::VoluntaryExit(_) => self .exit_tx .send(kind) - .map(|count| log_count(count, "exit")), + .map(|count| log_count("exit", count)), EventKind::ChainReorg(_) => self .chain_reorg_tx .send(kind) - .map(|count| log_count(count, "chain reorg")), + .map(|count| log_count("chain reorg", count)), EventKind::ContributionAndProof(_) => self .contribution_tx .send(kind) - .map(|count| log_count(count, "contribution and proof")), + .map(|count| log_count("contribution and proof", count)), EventKind::PayloadAttributes(_) => self .payload_attributes_tx .send(kind) - .map(|count| log_count(count, "payload attributes")), + .map(|count| log_count("payload attributes", count)), EventKind::LateHead(_) => self .late_head .send(kind) - .map(|count| log_count(count, "late head")), + .map(|count| log_count("late head", count)), EventKind::BlockReward(_) => self .block_reward_tx .send(kind) - .map(|count| log_count(count, "block reward")), + .map(|count| log_count("block reward", count)), }; if let Err(SendError(event)) = result { trace!(self.log, "No receivers registered to listen for event"; "event" => ?event); From 90cef1db868378ae7a10189b960b62bbc21b7fe6 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Mon, 13 Mar 2023 01:40:03 +0000 Subject: [PATCH 2/6] Appease Clippy 1.68 and refactor `http_api` (#4068) ## Proposed Changes Two tiny updates to satisfy Clippy 1.68 Plus refactoring of the `http_api` into less complex types so the compiler can chew and digest them more easily. Co-authored-by: Michael Sproul --- beacon_node/beacon_chain/src/lib.rs | 1 - beacon_node/http_api/src/lib.rs | 225 +++++++++--------- beacon_node/http_api/src/version.rs | 6 +- beacon_node/http_api/tests/main.rs | 1 - beacon_node/tests/test.rs | 1 - common/compare_fields_derive/src/lib.rs | 1 - common/warp_utils/src/lib.rs | 1 + common/warp_utils/src/task.rs | 24 +- common/warp_utils/src/uor.rs | 25 ++ consensus/fork_choice/src/fork_choice.rs | 1 - consensus/ssz_derive/src/lib.rs | 1 - consensus/tree_hash_derive/src/lib.rs | 1 - consensus/types/src/lib.rs | 2 - crypto/bls/src/generic_aggregate_signature.rs | 2 +- lighthouse/src/main.rs | 2 - .../execution_engine_integration/src/main.rs | 1 - testing/simulator/src/main.rs | 2 - 17 files changed, 164 insertions(+), 133 deletions(-) create mode 100644 common/warp_utils/src/uor.rs diff --git a/beacon_node/beacon_chain/src/lib.rs b/beacon_node/beacon_chain/src/lib.rs index 0f94c651a..173ce13b4 100644 --- a/beacon_node/beacon_chain/src/lib.rs +++ b/beacon_node/beacon_chain/src/lib.rs @@ -1,4 +1,3 @@ -#![recursion_limit = "128"] // For lazy-static pub mod attestation_rewards; pub mod attestation_verification; mod attester_cache; diff --git a/beacon_node/http_api/src/lib.rs b/beacon_node/http_api/src/lib.rs index 6cca673b8..05fe2fe10 100644 --- a/beacon_node/http_api/src/lib.rs +++ b/beacon_node/http_api/src/lib.rs @@ -1,4 +1,3 @@ -#![recursion_limit = "256"] //! This crate contains a HTTP server which serves the endpoints listed here: //! //! https://github.com/ethereum/beacon-APIs @@ -71,7 +70,8 @@ use warp::Reply; use warp::{http::Response, Filter}; use warp_utils::{ query::multi_key_query, - task::{blocking_json_task, blocking_task}, + task::{blocking_json_task, blocking_response_task}, + uor::UnifyingOrFilter, }; const API_PREFIX: &str = "eth"; @@ -1125,7 +1125,7 @@ pub fn serve( log: Logger| async move { publish_blocks::publish_block(None, block, chain, &network_tx, log) .await - .map(|()| warp::reply()) + .map(|()| warp::reply().into_response()) }, ); @@ -1149,7 +1149,7 @@ pub fn serve( log: Logger| async move { publish_blocks::publish_blinded_block(block, chain, &network_tx, log) .await - .map(|()| warp::reply()) + .map(|()| warp::reply().into_response()) }, ); @@ -1255,7 +1255,7 @@ pub fn serve( |block_id: BlockId, chain: Arc>, accept_header: Option| { - blocking_task(move || { + blocking_response_task(move || { let (block, execution_optimistic) = block_id.blinded_block(&chain)?; let fork_name = block .fork_name(&chain.spec) @@ -1767,7 +1767,7 @@ pub fn serve( .and(eth1_service_filter.clone()) .and_then( |accept_header: Option, eth1_service: eth1::Service| { - blocking_task(move || match accept_header { + blocking_response_task(move || match accept_header { Some(api_types::Accept::Json) | None => { let snapshot = eth1_service.get_deposit_snapshot(); Ok( @@ -1986,7 +1986,7 @@ pub fn serve( state_id: StateId, accept_header: Option, chain: Arc>| { - blocking_task(move || match accept_header { + blocking_response_task(move || match accept_header { Some(api_types::Accept::Ssz) => { // We can ignore the optimistic status for the "fork" since it's a // specification constant that doesn't change across competing heads of the @@ -1999,7 +1999,9 @@ pub fn serve( .status(200) .header("Content-Type", "application/octet-stream") .body(state.as_ssz_bytes().into()) - .map(|resp| add_consensus_version_header(resp, fork_name)) + .map(|resp: warp::reply::Response| { + add_consensus_version_header(resp, fork_name) + }) .map_err(|e| { warp_utils::reject::custom_server_error(format!( "failed to create response: {}", @@ -2162,7 +2164,7 @@ pub fn serve( .and(warp::path::end()) .and(network_globals.clone()) .and_then(|network_globals: Arc>| { - blocking_task(move || match *network_globals.sync_state.read() { + blocking_response_task(move || match *network_globals.sync_state.read() { SyncState::SyncingFinalized { .. } | SyncState::SyncingHead { .. } | SyncState::SyncTransition @@ -2426,7 +2428,7 @@ pub fn serve( .map_err(inconsistent_fork_rejection)?; fork_versioned_response(endpoint_version, fork_name, block) - .map(|response| warp::reply::json(&response)) + .map(|response| warp::reply::json(&response).into_response()) }, ); @@ -2483,7 +2485,7 @@ pub fn serve( // Pose as a V2 endpoint so we return the fork `version`. fork_versioned_response(V2, fork_name, block) - .map(|response| warp::reply::json(&response)) + .map(|response| warp::reply::json(&response).into_response()) }, ); @@ -2856,7 +2858,7 @@ pub fn serve( )) })?; - Ok::<_, warp::reject::Rejection>(warp::reply::json(&())) + Ok::<_, warp::reject::Rejection>(warp::reply::json(&()).into_response()) }, ); @@ -2965,7 +2967,7 @@ pub fn serve( builder .post_builder_validators(&filtered_registration_data) .await - .map(|resp| warp::reply::json(&resp)) + .map(|resp| warp::reply::json(&resp).into_response()) .map_err(|e| { warn!( log, @@ -3227,7 +3229,7 @@ pub fn serve( .and(warp::path::end()) .and(chain_filter.clone()) .and_then(|chain: Arc>| { - blocking_task(move || { + blocking_response_task(move || { Ok::<_, warp::Rejection>(warp::reply::json(&api_types::GenericResponseRef::from( chain .canonical_head @@ -3346,7 +3348,7 @@ pub fn serve( .and(warp::path::end()) .and(chain_filter.clone()) .and_then(|state_id: StateId, chain: Arc>| { - blocking_task(move || { + blocking_response_task(move || { // This debug endpoint provides no indication of optimistic status. let (state, _execution_optimistic) = state_id.state(&chain)?; Response::builder() @@ -3482,9 +3484,10 @@ pub fn serve( .and(chain_filter.clone()) .and_then(|chain: Arc>| async move { let merge_readiness = chain.check_merge_readiness().await; - Ok::<_, warp::reject::Rejection>(warp::reply::json(&api_types::GenericResponse::from( - merge_readiness, - ))) + Ok::<_, warp::reject::Rejection>( + warp::reply::json(&api_types::GenericResponse::from(merge_readiness)) + .into_response(), + ) }); let get_events = eth_v1 @@ -3495,7 +3498,7 @@ pub fn serve( .and_then( |topics_res: Result, chain: Arc>| { - blocking_task(move || { + blocking_response_task(move || { let topics = topics_res?; // for each topic subscribed spawn a new subscription let mut receivers = Vec::with_capacity(topics.topics.len()); @@ -3562,108 +3565,110 @@ pub fn serve( ); // Define the ultimate set of routes that will be provided to the server. + // Use `uor` rather than `or` in order to simplify types (see `UnifyingOrFilter`). let routes = warp::get() .and( get_beacon_genesis - .boxed() - .or(get_beacon_state_root.boxed()) - .or(get_beacon_state_fork.boxed()) - .or(get_beacon_state_finality_checkpoints.boxed()) - .or(get_beacon_state_validator_balances.boxed()) - .or(get_beacon_state_validators_id.boxed()) - .or(get_beacon_state_validators.boxed()) - .or(get_beacon_state_committees.boxed()) - .or(get_beacon_state_sync_committees.boxed()) - .or(get_beacon_state_randao.boxed()) - .or(get_beacon_headers.boxed()) - .or(get_beacon_headers_block_id.boxed()) - .or(get_beacon_block.boxed()) - .or(get_beacon_block_attestations.boxed()) - .or(get_beacon_blinded_block.boxed()) - .or(get_beacon_block_root.boxed()) - .or(get_beacon_pool_attestations.boxed()) - .or(get_beacon_pool_attester_slashings.boxed()) - .or(get_beacon_pool_proposer_slashings.boxed()) - .or(get_beacon_pool_voluntary_exits.boxed()) - .or(get_beacon_pool_bls_to_execution_changes.boxed()) - .or(get_beacon_deposit_snapshot.boxed()) - .or(get_beacon_rewards_blocks.boxed()) - .or(get_config_fork_schedule.boxed()) - .or(get_config_spec.boxed()) - .or(get_config_deposit_contract.boxed()) - .or(get_debug_beacon_states.boxed()) - .or(get_debug_beacon_heads.boxed()) - .or(get_node_identity.boxed()) - .or(get_node_version.boxed()) - .or(get_node_syncing.boxed()) - .or(get_node_health.boxed()) - .or(get_node_peers_by_id.boxed()) - .or(get_node_peers.boxed()) - .or(get_node_peer_count.boxed()) - .or(get_validator_duties_proposer.boxed()) - .or(get_validator_blocks.boxed()) - .or(get_validator_blinded_blocks.boxed()) - .or(get_validator_attestation_data.boxed()) - .or(get_validator_aggregate_attestation.boxed()) - .or(get_validator_sync_committee_contribution.boxed()) - .or(get_lighthouse_health.boxed()) - .or(get_lighthouse_ui_health.boxed()) - .or(get_lighthouse_ui_validator_count.boxed()) - .or(get_lighthouse_syncing.boxed()) - .or(get_lighthouse_nat.boxed()) - .or(get_lighthouse_peers.boxed()) - .or(get_lighthouse_peers_connected.boxed()) - .or(get_lighthouse_proto_array.boxed()) - .or(get_lighthouse_validator_inclusion_global.boxed()) - .or(get_lighthouse_validator_inclusion.boxed()) - .or(get_lighthouse_eth1_syncing.boxed()) - .or(get_lighthouse_eth1_block_cache.boxed()) - .or(get_lighthouse_eth1_deposit_cache.boxed()) - .or(get_lighthouse_beacon_states_ssz.boxed()) - .or(get_lighthouse_staking.boxed()) - .or(get_lighthouse_database_info.boxed()) - .or(get_lighthouse_block_rewards.boxed()) - .or(get_lighthouse_attestation_performance.boxed()) - .or(get_lighthouse_block_packing_efficiency.boxed()) - .or(get_lighthouse_merge_readiness.boxed()) - .or(get_events.boxed()) + .uor(get_beacon_state_root) + .uor(get_beacon_state_fork) + .uor(get_beacon_state_finality_checkpoints) + .uor(get_beacon_state_validator_balances) + .uor(get_beacon_state_validators_id) + .uor(get_beacon_state_validators) + .uor(get_beacon_state_committees) + .uor(get_beacon_state_sync_committees) + .uor(get_beacon_state_randao) + .uor(get_beacon_headers) + .uor(get_beacon_headers_block_id) + .uor(get_beacon_block) + .uor(get_beacon_block_attestations) + .uor(get_beacon_blinded_block) + .uor(get_beacon_block_root) + .uor(get_beacon_pool_attestations) + .uor(get_beacon_pool_attester_slashings) + .uor(get_beacon_pool_proposer_slashings) + .uor(get_beacon_pool_voluntary_exits) + .uor(get_beacon_pool_bls_to_execution_changes) + .uor(get_beacon_deposit_snapshot) + .uor(get_beacon_rewards_blocks) + .uor(get_config_fork_schedule) + .uor(get_config_spec) + .uor(get_config_deposit_contract) + .uor(get_debug_beacon_states) + .uor(get_debug_beacon_heads) + .uor(get_node_identity) + .uor(get_node_version) + .uor(get_node_syncing) + .uor(get_node_health) + .uor(get_node_peers_by_id) + .uor(get_node_peers) + .uor(get_node_peer_count) + .uor(get_validator_duties_proposer) + .uor(get_validator_blocks) + .uor(get_validator_blinded_blocks) + .uor(get_validator_attestation_data) + .uor(get_validator_aggregate_attestation) + .uor(get_validator_sync_committee_contribution) + .uor(get_lighthouse_health) + .uor(get_lighthouse_ui_health) + .uor(get_lighthouse_ui_validator_count) + .uor(get_lighthouse_syncing) + .uor(get_lighthouse_nat) + .uor(get_lighthouse_peers) + .uor(get_lighthouse_peers_connected) + .uor(get_lighthouse_proto_array) + .uor(get_lighthouse_validator_inclusion_global) + .uor(get_lighthouse_validator_inclusion) + .uor(get_lighthouse_eth1_syncing) + .uor(get_lighthouse_eth1_block_cache) + .uor(get_lighthouse_eth1_deposit_cache) + .uor(get_lighthouse_beacon_states_ssz) + .uor(get_lighthouse_staking) + .uor(get_lighthouse_database_info) + .uor(get_lighthouse_block_rewards) + .uor(get_lighthouse_attestation_performance) + .uor(get_lighthouse_block_packing_efficiency) + .uor(get_lighthouse_merge_readiness) + .uor(get_events) .recover(warp_utils::reject::handle_rejection), ) .boxed() - .or(warp::post().and( - post_beacon_blocks - .boxed() - .or(post_beacon_blinded_blocks.boxed()) - .or(post_beacon_pool_attestations.boxed()) - .or(post_beacon_pool_attester_slashings.boxed()) - .or(post_beacon_pool_proposer_slashings.boxed()) - .or(post_beacon_pool_voluntary_exits.boxed()) - .or(post_beacon_pool_sync_committees.boxed()) - .or(post_beacon_pool_bls_to_execution_changes.boxed()) - .or(post_beacon_rewards_attestations.boxed()) - .or(post_beacon_rewards_sync_committee.boxed()) - .or(post_validator_duties_attester.boxed()) - .or(post_validator_duties_sync.boxed()) - .or(post_validator_aggregate_and_proofs.boxed()) - .or(post_validator_contribution_and_proofs.boxed()) - .or(post_validator_beacon_committee_subscriptions.boxed()) - .or(post_validator_sync_committee_subscriptions.boxed()) - .or(post_validator_prepare_beacon_proposer.boxed()) - .or(post_validator_register_validator.boxed()) - .or(post_lighthouse_liveness.boxed()) - .or(post_lighthouse_database_reconstruct.boxed()) - .or(post_lighthouse_database_historical_blocks.boxed()) - .or(post_lighthouse_block_rewards.boxed()) - .or(post_lighthouse_ui_validator_metrics.boxed()) - .or(post_lighthouse_ui_validator_info.boxed()) - .recover(warp_utils::reject::handle_rejection), - )) + .uor( + warp::post().and( + post_beacon_blocks + .uor(post_beacon_blinded_blocks) + .uor(post_beacon_pool_attestations) + .uor(post_beacon_pool_attester_slashings) + .uor(post_beacon_pool_proposer_slashings) + .uor(post_beacon_pool_voluntary_exits) + .uor(post_beacon_pool_sync_committees) + .uor(post_beacon_pool_bls_to_execution_changes) + .uor(post_beacon_rewards_attestations) + .uor(post_beacon_rewards_sync_committee) + .uor(post_validator_duties_attester) + .uor(post_validator_duties_sync) + .uor(post_validator_aggregate_and_proofs) + .uor(post_validator_contribution_and_proofs) + .uor(post_validator_beacon_committee_subscriptions) + .uor(post_validator_sync_committee_subscriptions) + .uor(post_validator_prepare_beacon_proposer) + .uor(post_validator_register_validator) + .uor(post_lighthouse_liveness) + .uor(post_lighthouse_database_reconstruct) + .uor(post_lighthouse_database_historical_blocks) + .uor(post_lighthouse_block_rewards) + .uor(post_lighthouse_ui_validator_metrics) + .uor(post_lighthouse_ui_validator_info) + .recover(warp_utils::reject::handle_rejection), + ), + ) .recover(warp_utils::reject::handle_rejection) .with(slog_logging(log.clone())) .with(prometheus_metrics()) // Add a `Server` header. .map(|reply| warp::reply::with_header(reply, "Server", &version_with_platform())) - .with(cors_builder.build()); + .with(cors_builder.build()) + .boxed(); let http_socket: SocketAddr = SocketAddr::new(config.listen_addr, config.listen_port); let http_server: HttpServer = match config.tls_config { diff --git a/beacon_node/http_api/src/version.rs b/beacon_node/http_api/src/version.rs index 30f475e68..e7fd8910b 100644 --- a/beacon_node/http_api/src/version.rs +++ b/beacon_node/http_api/src/version.rs @@ -4,7 +4,7 @@ use serde::Serialize; use types::{ ExecutionOptimisticForkVersionedResponse, ForkName, ForkVersionedResponse, InconsistentFork, }; -use warp::reply::{self, Reply, WithHeader}; +use warp::reply::{self, Reply, Response}; pub const V1: EndpointVersion = EndpointVersion(1); pub const V2: EndpointVersion = EndpointVersion(2); @@ -48,8 +48,8 @@ pub fn execution_optimistic_fork_versioned_response( } /// Add the `Eth-Consensus-Version` header to a response. -pub fn add_consensus_version_header(reply: T, fork_name: ForkName) -> WithHeader { - reply::with_header(reply, CONSENSUS_VERSION_HEADER, fork_name.to_string()) +pub fn add_consensus_version_header(reply: T, fork_name: ForkName) -> Response { + reply::with_header(reply, CONSENSUS_VERSION_HEADER, fork_name.to_string()).into_response() } pub fn inconsistent_fork_rejection(error: InconsistentFork) -> warp::reject::Rejection { diff --git a/beacon_node/http_api/tests/main.rs b/beacon_node/http_api/tests/main.rs index ca6a27530..88e0032ec 100644 --- a/beacon_node/http_api/tests/main.rs +++ b/beacon_node/http_api/tests/main.rs @@ -1,5 +1,4 @@ #![cfg(not(debug_assertions))] // Tests are too slow in debug. -#![recursion_limit = "256"] pub mod common; pub mod fork_tests; diff --git a/beacon_node/tests/test.rs b/beacon_node/tests/test.rs index 8ccb260d2..bbec70330 100644 --- a/beacon_node/tests/test.rs +++ b/beacon_node/tests/test.rs @@ -1,5 +1,4 @@ #![cfg(test)] -#![recursion_limit = "512"] use beacon_chain::StateSkipConfig; use node_test_rig::{ diff --git a/common/compare_fields_derive/src/lib.rs b/common/compare_fields_derive/src/lib.rs index 752c09ee0..a8b92b3d5 100644 --- a/common/compare_fields_derive/src/lib.rs +++ b/common/compare_fields_derive/src/lib.rs @@ -1,4 +1,3 @@ -#![recursion_limit = "256"] extern crate proc_macro; use proc_macro::TokenStream; diff --git a/common/warp_utils/src/lib.rs b/common/warp_utils/src/lib.rs index 346361b18..77d61251f 100644 --- a/common/warp_utils/src/lib.rs +++ b/common/warp_utils/src/lib.rs @@ -6,3 +6,4 @@ pub mod metrics; pub mod query; pub mod reject; pub mod task; +pub mod uor; diff --git a/common/warp_utils/src/task.rs b/common/warp_utils/src/task.rs index c3b3e86e2..001231f2c 100644 --- a/common/warp_utils/src/task.rs +++ b/common/warp_utils/src/task.rs @@ -1,4 +1,5 @@ use serde::Serialize; +use warp::reply::{Reply, Response}; /// A convenience wrapper around `blocking_task`. pub async fn blocking_task(func: F) -> Result @@ -8,16 +9,29 @@ where { tokio::task::spawn_blocking(func) .await - .unwrap_or_else(|_| Err(warp::reject::reject())) // This should really be a 500 + .unwrap_or_else(|_| Err(warp::reject::reject())) +} + +/// A convenience wrapper around `blocking_task` that returns a `warp::reply::Response`. +/// +/// Using this method consistently makes it possible to simplify types using `.unify()` or `.uor()`. +pub async fn blocking_response_task(func: F) -> Result +where + F: FnOnce() -> Result + Send + 'static, + T: Reply + Send + 'static, +{ + blocking_task(func).await.map(Reply::into_response) } /// A convenience wrapper around `blocking_task` for use with `warp` JSON responses. -pub async fn blocking_json_task(func: F) -> Result +pub async fn blocking_json_task(func: F) -> Result where F: FnOnce() -> Result + Send + 'static, T: Serialize + Send + 'static, { - blocking_task(func) - .await - .map(|resp| warp::reply::json(&resp)) + blocking_response_task(|| { + let response = func()?; + Ok(warp::reply::json(&response)) + }) + .await } diff --git a/common/warp_utils/src/uor.rs b/common/warp_utils/src/uor.rs new file mode 100644 index 000000000..363f1df7d --- /dev/null +++ b/common/warp_utils/src/uor.rs @@ -0,0 +1,25 @@ +use warp::{filters::BoxedFilter, Filter, Rejection}; + +/// Mixin trait for `Filter` providing the unifying-or method. +pub trait UnifyingOrFilter: Filter + Sized + Send + Sync + 'static +where + Self::Extract: Send, +{ + /// Unifying `or`. + /// + /// This is a shorthand for `self.or(other).unify().boxed()`, which is useful because it keeps + /// the filter type simple and prevents type-checker explosions. + fn uor(self, other: F) -> BoxedFilter + where + F: Filter + Clone + Send + Sync + 'static, + { + self.or(other).unify().boxed() + } +} + +impl UnifyingOrFilter for F +where + F: Filter + Sized + Send + Sync + 'static, + F::Extract: Send, +{ +} diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index 566764195..916b1d558 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -1695,7 +1695,6 @@ mod tests { fn get_queued_attestations() -> Vec { (1..4) - .into_iter() .map(|i| QueuedAttestation { slot: Slot::new(i), attesting_indices: vec![], diff --git a/consensus/ssz_derive/src/lib.rs b/consensus/ssz_derive/src/lib.rs index 40d63fd02..53752ba44 100644 --- a/consensus/ssz_derive/src/lib.rs +++ b/consensus/ssz_derive/src/lib.rs @@ -1,4 +1,3 @@ -#![recursion_limit = "256"] //! Provides procedural derive macros for the `Encode` and `Decode` traits of the `eth2_ssz` crate. //! //! ## Attributes diff --git a/consensus/tree_hash_derive/src/lib.rs b/consensus/tree_hash_derive/src/lib.rs index 21ff324d5..85ece80fb 100644 --- a/consensus/tree_hash_derive/src/lib.rs +++ b/consensus/tree_hash_derive/src/lib.rs @@ -1,4 +1,3 @@ -#![recursion_limit = "256"] use darling::FromDeriveInput; use proc_macro::TokenStream; use quote::quote; diff --git a/consensus/types/src/lib.rs b/consensus/types/src/lib.rs index 6a5aef36f..824074244 100644 --- a/consensus/types/src/lib.rs +++ b/consensus/types/src/lib.rs @@ -1,6 +1,4 @@ //! Ethereum 2.0 types -// Required for big type-level numbers -#![recursion_limit = "128"] // Clippy lint set up #![cfg_attr( not(test), diff --git a/crypto/bls/src/generic_aggregate_signature.rs b/crypto/bls/src/generic_aggregate_signature.rs index fdb59626f..a61529af2 100644 --- a/crypto/bls/src/generic_aggregate_signature.rs +++ b/crypto/bls/src/generic_aggregate_signature.rs @@ -266,7 +266,7 @@ where } /// Hashes the `self.serialize()` bytes. -#[allow(clippy::derive_hash_xor_eq)] +#[allow(clippy::derived_hash_with_manual_eq)] impl Hash for GenericAggregateSignature where Sig: TSignature, diff --git a/lighthouse/src/main.rs b/lighthouse/src/main.rs index babe2f8dc..b05e78fe5 100644 --- a/lighthouse/src/main.rs +++ b/lighthouse/src/main.rs @@ -1,5 +1,3 @@ -#![recursion_limit = "256"] - mod metrics; use beacon_node::ProductionBeaconNode; diff --git a/testing/execution_engine_integration/src/main.rs b/testing/execution_engine_integration/src/main.rs index bd3436602..e46bc13c8 100644 --- a/testing/execution_engine_integration/src/main.rs +++ b/testing/execution_engine_integration/src/main.rs @@ -1,4 +1,3 @@ -#![recursion_limit = "1024"] /// This binary runs integration tests between Lighthouse and execution engines. /// /// It will first attempt to build any supported integration clients, then it will run tests. diff --git a/testing/simulator/src/main.rs b/testing/simulator/src/main.rs index 9e05a539c..922149537 100644 --- a/testing/simulator/src/main.rs +++ b/testing/simulator/src/main.rs @@ -1,5 +1,3 @@ -#![recursion_limit = "256"] - //! This crate provides a simluation that creates `n` beacon node and validator clients, each with //! `v` validators. A deposit contract is deployed at the start of the simulation using a local //! `ganache` instance (you must have `ganache` installed and avaliable on your path). All From 373beaf9139d0fd8ff1379508eb2ae6b62f055ab Mon Sep 17 00:00:00 2001 From: Sebastian Richel Date: Mon, 13 Mar 2023 04:08:14 +0000 Subject: [PATCH 3/6] Added warning when new jwt is generated (#4000) ## Issue Addressed #3435 ## Proposed Changes Fire a warning with the path of JWT to be created when the path given by --execution-jwt is not found Currently, the same error is logged if the jwt is found but doesn't match the execution client's jwt, and if no jwt was found at the given path. This makes it very hard to tell if you accidentally typed the wrong path, as a new jwt is created silently that won't match the execution client's jwt. So instead, it will now fire a warning stating that a jwt is being generated at the given path. ## Additional Info In the future, it may be smarter to handle this case by adding an InvalidJWTPath member to the Error enum in lib.rs or auth.rs that can be handled during upcheck() This is my first PR and first project with rust. so thanks to anyone who looks at this for their patience and help! Co-authored-by: Sebastian Richel <47844429+sebastianrich18@users.noreply.github.com> --- beacon_node/execution_layer/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/beacon_node/execution_layer/src/lib.rs b/beacon_node/execution_layer/src/lib.rs index d12f9996d..b7aa4dc05 100644 --- a/beacon_node/execution_layer/src/lib.rs +++ b/beacon_node/execution_layer/src/lib.rs @@ -293,6 +293,7 @@ impl ExecutionLayer { .map_err(Error::InvalidJWTSecret) } else { // Create a new file and write a randomly generated secret to it if file does not exist + warn!(log, "No JWT found on disk. Generating"; "path" => %secret_file.display()); std::fs::File::options() .write(true) .create_new(true) From 06af31a66adb49cd814874f9573108f8a6e3dae5 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Mon, 13 Mar 2023 04:08:15 +0000 Subject: [PATCH 4/6] Correct /lighthouse/nat implementation (#4069) ## Proposed Changes The current `/lighthouse/nat` implementation checks for _zero_ address updated messages, when it should check for a _non-zero_ number. This was spotted while debugging an issue on Discord where a user's ports weren't forwarded but `/lighthouse/nat` was still returning `true`. --- beacon_node/lighthouse_network/src/metrics.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beacon_node/lighthouse_network/src/metrics.rs b/beacon_node/lighthouse_network/src/metrics.rs index 2ee224d5e..aed68e27f 100644 --- a/beacon_node/lighthouse_network/src/metrics.rs +++ b/beacon_node/lighthouse_network/src/metrics.rs @@ -159,7 +159,7 @@ pub fn check_nat() { if NAT_OPEN.as_ref().map(|v| v.get()).unwrap_or(0) != 0 { return; } - if ADDRESS_UPDATE_COUNT.as_ref().map(|v| v.get()).unwrap_or(0) == 0 + if ADDRESS_UPDATE_COUNT.as_ref().map(|v| v.get()).unwrap_or(0) != 0 || NETWORK_INBOUND_PEERS.as_ref().map(|v| v.get()).unwrap_or(0) != 0_i64 { inc_counter(&NAT_OPEN); From e190ebb8a0581c199fbd4089882141dd00f7e26d Mon Sep 17 00:00:00 2001 From: Divma Date: Tue, 14 Mar 2023 01:13:34 +0000 Subject: [PATCH 5/6] Support for Ipv6 (#4046) ## Issue Addressed Add support for ipv6 and dual stack in lighthouse. ## Proposed Changes From an user perspective, now setting an ipv6 address, optionally configuring the ports should feel exactly the same as using an ipv4 address. If listening over both ipv4 and ipv6 then the user needs to: - use the `--listen-address` two times (ipv4 and ipv6 addresses) - `--port6` becomes then required - `--discovery-port6` can now be used to additionally configure the ipv6 udp port ### Rough list of code changes - Discovery: - Table filter and ip mode set to match the listening config. - Ipv6 address, tcp port and udp port set in the ENR builder - Reported addresses now check which tcp port to give to libp2p - LH Network Service: - Can listen over Ipv6, Ipv4, or both. This uses two sockets. Using mapped addresses is disabled from libp2p and it's the most compatible option. - NetworkGlobals: - No longer stores udp port since was not used at all. Instead, stores the Ipv4 and Ipv6 TCP ports. - NetworkConfig: - Update names to make it clear that previous udp and tcp ports in ENR were Ipv4 - Add fields to configure Ipv6 udp and tcp ports in the ENR - Include advertised enr Ipv6 address. - Add type to model Listening address that's either Ipv4, Ipv6 or both. A listening address includes the ip, udp port and tcp port. - UPnP: - Kept only for ipv4 - Cli flags: - `--listen-addresses` now can take up to two values - `--port` will apply to ipv4 or ipv6 if only one listening address is given. If two listening addresses are given it will apply only to Ipv4. - `--port6` New flag required when listening over ipv4 and ipv6 that applies exclusively to Ipv6. - `--discovery-port` will now apply to ipv4 and ipv6 if only one listening address is given. - `--discovery-port6` New flag to configure the individual udp port of ipv6 if listening over both ipv4 and ipv6. - `--enr-udp-port` Updated docs to specify that it only applies to ipv4. This is an old behaviour. - `--enr-udp6-port` Added to configure the enr udp6 field. - `--enr-tcp-port` Updated docs to specify that it only applies to ipv4. This is an old behaviour. - `--enr-tcp6-port` Added to configure the enr tcp6 field. - `--enr-addresses` now can take two values. - `--enr-match` updated behaviour. - Common: - rename `unused_port` functions to specify that they are over ipv4. - add functions to get unused ports over ipv6. - Testing binaries - Updated code to reflect network config changes and unused_port changes. ## Additional Info TODOs: - use two sockets in discovery. I'll get back to this and it's on https://github.com/sigp/discv5/pull/160 - lcli allow listening over two sockets in generate_bootnodes_enr - add at least one smoke flag for ipv6 (I have tested this and works for me) - update the book --- beacon_node/beacon_chain/src/test_utils.rs | 2 +- beacon_node/client/src/lib.rs | 15 +- beacon_node/http_api/tests/common.rs | 6 +- beacon_node/http_api/tests/tests.rs | 2 +- beacon_node/lighthouse_network/src/config.rs | 204 +++++++++- .../lighthouse_network/src/discovery/enr.rs | 37 +- .../lighthouse_network/src/discovery/mod.rs | 49 ++- beacon_node/lighthouse_network/src/lib.rs | 2 + .../lighthouse_network/src/listen_addr.rs | 97 +++++ .../lighthouse_network/src/service/mod.rs | 52 +-- .../lighthouse_network/src/types/globals.rs | 29 +- .../lighthouse_network/tests/common.rs | 12 +- .../network/src/beacon_processor/tests.rs | 5 +- beacon_node/network/src/nat.rs | 12 +- beacon_node/network/src/service.rs | 25 +- beacon_node/network/src/service/tests.rs | 3 +- beacon_node/src/cli.rs | 74 +++- beacon_node/src/config.rs | 368 +++++++++++++----- boot_node/src/cli.rs | 8 + boot_node/src/config.rs | 84 ++-- common/unused_port/src/lib.rs | 39 +- lcli/src/generate_bootnode_enr.rs | 15 +- lighthouse/tests/beacon_node.rs | 350 ++++++++++++++--- lighthouse/tests/boot_node.rs | 6 +- testing/eth1_test_rig/src/ganache.rs | 6 +- .../src/execution_engine.rs | 6 +- .../execution_engine_integration/src/geth.rs | 4 +- .../src/nethermind.rs | 4 +- testing/node_test_rig/src/lib.rs | 5 +- testing/simulator/src/eth1_sim.rs | 4 +- testing/simulator/src/local_network.rs | 22 +- testing/simulator/src/no_eth1_sim.rs | 4 +- testing/simulator/src/sync_sim.rs | 4 +- 33 files changed, 1194 insertions(+), 361 deletions(-) create mode 100644 beacon_node/lighthouse_network/src/listen_addr.rs diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index afb31bba7..3c5d1fd3b 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -457,7 +457,7 @@ where builder_threshold: Option, ) -> Self { // Get a random unused port - let port = unused_port::unused_tcp_port().unwrap(); + let port = unused_port::unused_tcp4_port().unwrap(); let builder_url = SensitiveUrl::parse(format!("http://127.0.0.1:{port}").as_str()).unwrap(); let spec = self.spec.clone().expect("cannot build without spec"); diff --git a/beacon_node/client/src/lib.rs b/beacon_node/client/src/lib.rs index b0184dc0f..584a0d736 100644 --- a/beacon_node/client/src/lib.rs +++ b/beacon_node/client/src/lib.rs @@ -46,9 +46,18 @@ impl Client { self.http_metrics_listen_addr } - /// Returns the port of the client's libp2p stack, if it was started. - pub fn libp2p_listen_port(&self) -> Option { - self.network_globals.as_ref().map(|n| n.listen_port_tcp()) + /// Returns the ipv4 port of the client's libp2p stack, if it was started. + pub fn libp2p_listen_ipv4_port(&self) -> Option { + self.network_globals + .as_ref() + .and_then(|n| n.listen_port_tcp4()) + } + + /// Returns the ipv6 port of the client's libp2p stack, if it was started. + pub fn libp2p_listen_ipv6_port(&self) -> Option { + self.network_globals + .as_ref() + .and_then(|n| n.listen_port_tcp6()) } /// Returns the list of libp2p addresses the client is listening to. diff --git a/beacon_node/http_api/tests/common.rs b/beacon_node/http_api/tests/common.rs index ee0273579..3e34bafe8 100644 --- a/beacon_node/http_api/tests/common.rs +++ b/beacon_node/http_api/tests/common.rs @@ -130,7 +130,7 @@ pub async fn create_api_server( log: Logger, ) -> ApiServer> { // Get a random unused port. - let port = unused_port::unused_tcp_port().unwrap(); + let port = unused_port::unused_tcp4_port().unwrap(); create_api_server_on_port(chain, log, port).await } @@ -151,8 +151,8 @@ pub async fn create_api_server_on_port( let enr = EnrBuilder::new("v4").build(&enr_key).unwrap(); let network_globals = Arc::new(NetworkGlobals::new( enr.clone(), - TCP_PORT, - UDP_PORT, + Some(TCP_PORT), + None, meta_data, vec![], &log, diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index 6424d73eb..977c737fd 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -112,7 +112,7 @@ impl ApiTester { pub async fn new_from_config(config: ApiTesterConfig) -> Self { // Get a random unused port let spec = config.spec; - let port = unused_port::unused_tcp_port().unwrap(); + let port = unused_port::unused_tcp4_port().unwrap(); let beacon_url = SensitiveUrl::parse(format!("http://127.0.0.1:{port}").as_str()).unwrap(); let harness = Arc::new( diff --git a/beacon_node/lighthouse_network/src/config.rs b/beacon_node/lighthouse_network/src/config.rs index 1e3231501..79041f6d9 100644 --- a/beacon_node/lighthouse_network/src/config.rs +++ b/beacon_node/lighthouse_network/src/config.rs @@ -1,3 +1,4 @@ +use crate::listen_addr::{ListenAddr, ListenAddress}; use crate::rpc::config::OutboundRateLimiterConfig; use crate::types::GossipKind; use crate::{Enr, PeerIdSerialized}; @@ -12,6 +13,7 @@ use libp2p::gossipsub::{ use libp2p::Multiaddr; use serde_derive::{Deserialize, Serialize}; use sha2::{Digest, Sha256}; +use std::net::{Ipv4Addr, Ipv6Addr}; use std::path::PathBuf; use std::sync::Arc; use std::time::Duration; @@ -57,24 +59,24 @@ pub struct Config { /// Data directory where node's keyfile is stored pub network_dir: PathBuf, - /// IP address to listen on. - pub listen_address: std::net::IpAddr, - - /// The TCP port that libp2p listens on. - pub libp2p_port: u16, - - /// UDP port that discovery listens on. - pub discovery_port: u16, + /// IP addresses to listen on. + listen_addresses: ListenAddress, /// The address to broadcast to peers about which address we are listening on. None indicates /// that no discovery address has been set in the CLI args. - pub enr_address: Option, + pub enr_address: (Option, Option), - /// The udp port to broadcast to peers in order to reach back for discovery. - pub enr_udp_port: Option, + /// The udp4 port to broadcast to peers in order to reach back for discovery. + pub enr_udp4_port: Option, - /// The tcp port to broadcast to peers in order to reach back for libp2p services. - pub enr_tcp_port: Option, + /// The tcp4 port to broadcast to peers in order to reach back for libp2p services. + pub enr_tcp4_port: Option, + + /// The udp6 port to broadcast to peers in order to reach back for discovery. + pub enr_udp6_port: Option, + + /// The tcp6 port to broadcast to peers in order to reach back for libp2p services. + pub enr_tcp6_port: Option, /// Target number of connected peers. pub target_peers: usize, @@ -139,6 +141,105 @@ pub struct Config { pub outbound_rate_limiter_config: Option, } +impl Config { + /// Sets the listening address to use an ipv4 address. The discv5 ip_mode and table filter are + /// adjusted accordingly to ensure addresses that are present in the enr are globally + /// reachable. + pub fn set_ipv4_listening_address(&mut self, addr: Ipv4Addr, tcp_port: u16, udp_port: u16) { + self.listen_addresses = ListenAddress::V4(ListenAddr { + addr, + udp_port, + tcp_port, + }); + self.discv5_config.ip_mode = discv5::IpMode::Ip4; + self.discv5_config.table_filter = |enr| enr.ip4().as_ref().map_or(false, is_global_ipv4) + } + + /// Sets the listening address to use an ipv6 address. The discv5 ip_mode and table filter is + /// adjusted accordingly to ensure addresses that are present in the enr are globally + /// reachable. + pub fn set_ipv6_listening_address(&mut self, addr: Ipv6Addr, tcp_port: u16, udp_port: u16) { + self.listen_addresses = ListenAddress::V6(ListenAddr { + addr, + udp_port, + tcp_port, + }); + self.discv5_config.ip_mode = discv5::IpMode::Ip6 { + enable_mapped_addresses: false, + }; + self.discv5_config.table_filter = |enr| enr.ip6().as_ref().map_or(false, is_global_ipv6) + } + + /// Sets the listening address to use both an ipv4 and ipv6 address. The discv5 ip_mode and + /// table filter is adjusted accordingly to ensure addresses that are present in the enr are + /// globally reachable. + pub fn set_ipv4_ipv6_listening_addresses( + &mut self, + v4_addr: Ipv4Addr, + tcp4_port: u16, + udp4_port: u16, + v6_addr: Ipv6Addr, + tcp6_port: u16, + udp6_port: u16, + ) { + self.listen_addresses = ListenAddress::DualStack( + ListenAddr { + addr: v4_addr, + udp_port: udp4_port, + tcp_port: tcp4_port, + }, + ListenAddr { + addr: v6_addr, + udp_port: udp6_port, + tcp_port: tcp6_port, + }, + ); + + self.discv5_config.ip_mode = discv5::IpMode::Ip6 { + enable_mapped_addresses: true, + }; + self.discv5_config.table_filter = |enr| match (&enr.ip4(), &enr.ip6()) { + (None, None) => false, + (None, Some(ip6)) => is_global_ipv6(ip6), + (Some(ip4), None) => is_global_ipv4(ip4), + (Some(ip4), Some(ip6)) => is_global_ipv4(ip4) && is_global_ipv6(ip6), + }; + } + + pub fn set_listening_addr(&mut self, listen_addr: ListenAddress) { + match listen_addr { + ListenAddress::V4(ListenAddr { + addr, + udp_port, + tcp_port, + }) => self.set_ipv4_listening_address(addr, tcp_port, udp_port), + ListenAddress::V6(ListenAddr { + addr, + udp_port, + tcp_port, + }) => self.set_ipv6_listening_address(addr, tcp_port, udp_port), + ListenAddress::DualStack( + ListenAddr { + addr: ip4addr, + udp_port: udp4_port, + tcp_port: tcp4_port, + }, + ListenAddr { + addr: ip6addr, + udp_port: udp6_port, + tcp_port: tcp6_port, + }, + ) => self.set_ipv4_ipv6_listening_addresses( + ip4addr, tcp4_port, udp4_port, ip6addr, tcp6_port, udp6_port, + ), + } + } + + pub fn listen_addrs(&self) -> &ListenAddress { + &self.listen_addresses + } +} + impl Default for Config { /// Generate a default network configuration. fn default() -> Self { @@ -183,7 +284,7 @@ impl Default for Config { .filter_rate_limiter(filter_rate_limiter) .filter_max_bans_per_ip(Some(5)) .filter_max_nodes_per_ip(Some(10)) - .table_filter(|enr| enr.ip4().map_or(false, |ip| is_global(&ip))) // Filter non-global IPs + .table_filter(|enr| enr.ip4().map_or(false, |ip| is_global_ipv4(&ip))) // Filter non-global IPs .ban_duration(Some(Duration::from_secs(3600))) .ping_interval(Duration::from_secs(300)) .build(); @@ -191,12 +292,16 @@ impl Default for Config { // NOTE: Some of these get overridden by the corresponding CLI default values. Config { network_dir, - listen_address: "0.0.0.0".parse().expect("valid ip address"), - libp2p_port: 9000, - discovery_port: 9000, - enr_address: None, - enr_udp_port: None, - enr_tcp_port: None, + listen_addresses: ListenAddress::V4(ListenAddr { + addr: Ipv4Addr::UNSPECIFIED, + udp_port: 9000, + tcp_port: 9000, + }), + enr_address: (None, None), + enr_udp4_port: None, + enr_tcp4_port: None, + enr_udp6_port: None, + enr_tcp6_port: None, target_peers: 50, gs_config, discv5_config, @@ -361,7 +466,7 @@ pub fn gossipsub_config(network_load: u8, fork_context: Arc) -> Gos /// Helper function to determine if the IpAddr is a global address or not. The `is_global()` /// function is not yet stable on IpAddr. #[allow(clippy::nonminimal_bool)] -fn is_global(addr: &std::net::Ipv4Addr) -> bool { +fn is_global_ipv4(addr: &Ipv4Addr) -> bool { // check if this address is 192.0.0.9 or 192.0.0.10. These addresses are the only two // globally routable addresses in the 192.0.0.0/24 range. if u32::from_be_bytes(addr.octets()) == 0xc0000009 @@ -382,3 +487,60 @@ fn is_global(addr: &std::net::Ipv4Addr) -> bool { // Make sure the address is not in 0.0.0.0/8 && addr.octets()[0] != 0 } + +/// NOTE: Docs taken from https://doc.rust-lang.org/stable/std/net/struct.Ipv6Addr.html#method.is_global +/// +/// Returns true if the address appears to be globally reachable as specified by the IANA IPv6 +/// Special-Purpose Address Registry. Whether or not an address is practically reachable will +/// depend on your network configuration. +/// +/// Most IPv6 addresses are globally reachable; unless they are specifically defined as not +/// globally reachable. +/// +/// Non-exhaustive list of notable addresses that are not globally reachable: +/// +/// - The unspecified address (is_unspecified) +/// - The loopback address (is_loopback) +/// - IPv4-mapped addresses +/// - Addresses reserved for benchmarking +/// - Addresses reserved for documentation (is_documentation) +/// - Unique local addresses (is_unique_local) +/// - Unicast addresses with link-local scope (is_unicast_link_local) +// TODO: replace with [`Ipv6Addr::is_global`] once +// [Ip](https://github.com/rust-lang/rust/issues/27709) is stable. +pub const fn is_global_ipv6(addr: &Ipv6Addr) -> bool { + const fn is_documentation(addr: &Ipv6Addr) -> bool { + (addr.segments()[0] == 0x2001) && (addr.segments()[1] == 0xdb8) + } + const fn is_unique_local(addr: &Ipv6Addr) -> bool { + (addr.segments()[0] & 0xfe00) == 0xfc00 + } + const fn is_unicast_link_local(addr: &Ipv6Addr) -> bool { + (addr.segments()[0] & 0xffc0) == 0xfe80 + } + !(addr.is_unspecified() + || addr.is_loopback() + // IPv4-mapped Address (`::ffff:0:0/96`) + || matches!(addr.segments(), [0, 0, 0, 0, 0, 0xffff, _, _]) + // IPv4-IPv6 Translat. (`64:ff9b:1::/48`) + || matches!(addr.segments(), [0x64, 0xff9b, 1, _, _, _, _, _]) + // Discard-Only Address Block (`100::/64`) + || matches!(addr.segments(), [0x100, 0, 0, 0, _, _, _, _]) + // IETF Protocol Assignments (`2001::/23`) + || (matches!(addr.segments(), [0x2001, b, _, _, _, _, _, _] if b < 0x200) + && !( + // Port Control Protocol Anycast (`2001:1::1`) + u128::from_be_bytes(addr.octets()) == 0x2001_0001_0000_0000_0000_0000_0000_0001 + // Traversal Using Relays around NAT Anycast (`2001:1::2`) + || u128::from_be_bytes(addr.octets()) == 0x2001_0001_0000_0000_0000_0000_0000_0002 + // AMT (`2001:3::/32`) + || matches!(addr.segments(), [0x2001, 3, _, _, _, _, _, _]) + // AS112-v6 (`2001:4:112::/48`) + || matches!(addr.segments(), [0x2001, 4, 0x112, _, _, _, _, _]) + // ORCHIDv2 (`2001:20::/28`) + || matches!(addr.segments(), [0x2001, b, _, _, _, _, _, _] if b >= 0x20 && b <= 0x2F) + )) + || is_documentation(addr) + || is_unique_local(addr) + || is_unicast_link_local(addr)) +} diff --git a/beacon_node/lighthouse_network/src/discovery/enr.rs b/beacon_node/lighthouse_network/src/discovery/enr.rs index 6b4b87a5f..938e7cfa2 100644 --- a/beacon_node/lighthouse_network/src/discovery/enr.rs +++ b/beacon_node/lighthouse_network/src/discovery/enr.rs @@ -145,16 +145,39 @@ pub fn create_enr_builder_from_config( enable_tcp: bool, ) -> EnrBuilder { let mut builder = EnrBuilder::new("v4"); - if let Some(enr_address) = config.enr_address { - builder.ip(enr_address); + let (maybe_ipv4_address, maybe_ipv6_address) = &config.enr_address; + + if let Some(ip) = maybe_ipv4_address { + builder.ip4(*ip); } - if let Some(udp_port) = config.enr_udp_port { - builder.udp4(udp_port); + + if let Some(ip) = maybe_ipv6_address { + builder.ip6(*ip); } - // we always give it our listening tcp port + + if let Some(udp4_port) = config.enr_udp4_port { + builder.udp4(udp4_port); + } + + if let Some(udp6_port) = config.enr_udp6_port { + builder.udp6(udp6_port); + } + if enable_tcp { - let tcp_port = config.enr_tcp_port.unwrap_or(config.libp2p_port); - builder.tcp4(tcp_port); + // If the ENR port is not set, and we are listening over that ip version, use the listening port instead. + let tcp4_port = config + .enr_tcp4_port + .or_else(|| config.listen_addrs().v4().map(|v4_addr| v4_addr.tcp_port)); + if let Some(tcp4_port) = tcp4_port { + builder.tcp4(tcp4_port); + } + + let tcp6_port = config + .enr_tcp6_port + .or_else(|| config.listen_addrs().v6().map(|v6_addr| v6_addr.tcp_port)); + if let Some(tcp6_port) = tcp6_port { + builder.tcp6(tcp6_port); + } } builder } diff --git a/beacon_node/lighthouse_network/src/discovery/mod.rs b/beacon_node/lighthouse_network/src/discovery/mod.rs index c41844c2c..b9c4e76fe 100644 --- a/beacon_node/lighthouse_network/src/discovery/mod.rs +++ b/beacon_node/lighthouse_network/src/discovery/mod.rs @@ -201,8 +201,13 @@ impl Discovery { info!(log, "ENR Initialised"; "enr" => local_enr.to_base64(), "seq" => local_enr.seq(), "id"=> %local_enr.node_id(), "ip4" => ?local_enr.ip4(), "udp4"=> ?local_enr.udp4(), "tcp4" => ?local_enr.tcp6() ); - - let listen_socket = SocketAddr::new(config.listen_address, config.discovery_port); + let listen_socket = match config.listen_addrs() { + crate::listen_addr::ListenAddress::V4(v4_addr) => v4_addr.udp_socket_addr(), + crate::listen_addr::ListenAddress::V6(v6_addr) => v6_addr.udp_socket_addr(), + crate::listen_addr::ListenAddress::DualStack(_v4_addr, v6_addr) => { + v6_addr.udp_socket_addr() + } + }; // convert the keypair into an ENR key let enr_key: CombinedKey = CombinedKey::from_libp2p(local_key)?; @@ -1015,14 +1020,27 @@ impl NetworkBehaviour for Discovery { *self.network_globals.local_enr.write() = enr; // A new UDP socket has been detected. // Build a multiaddr to report to libp2p - let mut address = Multiaddr::from(socket_addr.ip()); - // NOTE: This doesn't actually track the external TCP port. More sophisticated NAT handling - // should handle this. - address.push(Protocol::Tcp(self.network_globals.listen_port_tcp())); - return Poll::Ready(NBAction::ReportObservedAddr { - address, - score: AddressScore::Finite(1), - }); + let addr = match socket_addr.ip() { + IpAddr::V4(v4_addr) => { + self.network_globals.listen_port_tcp4().map(|tcp4_port| { + Multiaddr::from(v4_addr).with(Protocol::Tcp(tcp4_port)) + }) + } + IpAddr::V6(v6_addr) => { + self.network_globals.listen_port_tcp6().map(|tcp6_port| { + Multiaddr::from(v6_addr).with(Protocol::Tcp(tcp6_port)) + }) + } + }; + + if let Some(address) = addr { + // NOTE: This doesn't actually track the external TCP port. More sophisticated NAT handling + // should handle this. + return Poll::Ready(NBAction::ReportObservedAddr { + address, + score: AddressScore::Finite(1), + }); + } } Discv5Event::EnrAdded { .. } | Discv5Event::TalkRequest(_) @@ -1087,7 +1105,6 @@ mod tests { use enr::EnrBuilder; use slog::{o, Drain}; use types::{BitVector, MinimalEthSpec, SubnetId}; - use unused_port::unused_udp_port; type E = MinimalEthSpec; @@ -1105,17 +1122,15 @@ mod tests { async fn build_discovery() -> Discovery { let keypair = libp2p::identity::Keypair::generate_secp256k1(); - let config = NetworkConfig { - discovery_port: unused_udp_port().unwrap(), - ..Default::default() - }; + let mut config = NetworkConfig::default(); + config.set_listening_addr(crate::ListenAddress::unused_v4_ports()); let enr_key: CombinedKey = CombinedKey::from_libp2p(&keypair).unwrap(); let enr: Enr = build_enr::(&enr_key, &config, &EnrForkId::default()).unwrap(); let log = build_log(slog::Level::Debug, false); let globals = NetworkGlobals::new( enr, - 9000, - 9000, + Some(9000), + None, MetaData::V2(MetaDataV2 { seq_number: 0, attnets: Default::default(), diff --git a/beacon_node/lighthouse_network/src/lib.rs b/beacon_node/lighthouse_network/src/lib.rs index be4da809c..3d539af3b 100644 --- a/beacon_node/lighthouse_network/src/lib.rs +++ b/beacon_node/lighthouse_network/src/lib.rs @@ -10,12 +10,14 @@ pub mod service; #[allow(clippy::mutable_key_type)] // PeerId in hashmaps are no longer permitted by clippy pub mod discovery; +pub mod listen_addr; pub mod metrics; pub mod peer_manager; pub mod rpc; pub mod types; pub use config::gossip_max_size; +pub use listen_addr::*; use serde::{de, Deserialize, Deserializer, Serialize, Serializer}; use std::str::FromStr; diff --git a/beacon_node/lighthouse_network/src/listen_addr.rs b/beacon_node/lighthouse_network/src/listen_addr.rs new file mode 100644 index 000000000..20d87d403 --- /dev/null +++ b/beacon_node/lighthouse_network/src/listen_addr.rs @@ -0,0 +1,97 @@ +use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr}; + +use libp2p::{multiaddr::Protocol, Multiaddr}; +use serde::{Deserialize, Serialize}; + +/// A listening address composed by an Ip, an UDP port and a TCP port. +#[derive(Clone, Debug, Serialize, Deserialize)] +pub struct ListenAddr { + pub addr: Ip, + pub udp_port: u16, + pub tcp_port: u16, +} + +impl + Clone> ListenAddr { + pub fn udp_socket_addr(&self) -> SocketAddr { + (self.addr.clone().into(), self.udp_port).into() + } + + pub fn tcp_socket_addr(&self) -> SocketAddr { + (self.addr.clone().into(), self.tcp_port).into() + } +} + +/// Types of listening addresses Lighthouse can accept. +#[derive(Clone, Debug, Serialize, Deserialize)] +pub enum ListenAddress { + V4(ListenAddr), + V6(ListenAddr), + DualStack(ListenAddr, ListenAddr), +} + +impl ListenAddress { + /// Return the listening address over IpV4 if any. + pub fn v4(&self) -> Option<&ListenAddr> { + match self { + ListenAddress::V4(v4_addr) | ListenAddress::DualStack(v4_addr, _) => Some(v4_addr), + ListenAddress::V6(_) => None, + } + } + + /// Return the listening address over IpV6 if any. + pub fn v6(&self) -> Option<&ListenAddr> { + match self { + ListenAddress::V6(v6_addr) | ListenAddress::DualStack(_, v6_addr) => Some(v6_addr), + ListenAddress::V4(_) => None, + } + } + + /// Returns the TCP addresses. + pub fn tcp_addresses(&self) -> impl Iterator + '_ { + let v4_multiaddr = self + .v4() + .map(|v4_addr| Multiaddr::from(v4_addr.addr).with(Protocol::Tcp(v4_addr.tcp_port))); + let v6_multiaddr = self + .v6() + .map(|v6_addr| Multiaddr::from(v6_addr.addr).with(Protocol::Tcp(v6_addr.tcp_port))); + v4_multiaddr.into_iter().chain(v6_multiaddr) + } + + #[cfg(test)] + pub fn unused_v4_ports() -> Self { + ListenAddress::V4(ListenAddr { + addr: Ipv4Addr::UNSPECIFIED, + udp_port: unused_port::unused_udp4_port().unwrap(), + tcp_port: unused_port::unused_tcp4_port().unwrap(), + }) + } + + #[cfg(test)] + pub fn unused_v6_ports() -> Self { + ListenAddress::V6(ListenAddr { + addr: Ipv6Addr::UNSPECIFIED, + udp_port: unused_port::unused_udp6_port().unwrap(), + tcp_port: unused_port::unused_tcp6_port().unwrap(), + }) + } +} + +impl slog::KV for ListenAddress { + fn serialize( + &self, + _record: &slog::Record, + serializer: &mut dyn slog::Serializer, + ) -> slog::Result { + if let Some(v4_addr) = self.v4() { + serializer.emit_arguments("ip4_address", &format_args!("{}", v4_addr.addr))?; + serializer.emit_u16("udp4_port", v4_addr.udp_port)?; + serializer.emit_u16("tcp4_port", v4_addr.tcp_port)?; + } + if let Some(v6_addr) = self.v6() { + serializer.emit_arguments("ip6_address", &format_args!("{}", v6_addr.addr))?; + serializer.emit_u16("udp6_port", v6_addr.udp_port)?; + serializer.emit_u16("tcp6_port", v6_addr.tcp_port)?; + } + slog::Result::Ok(()) + } +} diff --git a/beacon_node/lighthouse_network/src/service/mod.rs b/beacon_node/lighthouse_network/src/service/mod.rs index e20b86e54..5cdcdeaf8 100644 --- a/beacon_node/lighthouse_network/src/service/mod.rs +++ b/beacon_node/lighthouse_network/src/service/mod.rs @@ -163,8 +163,8 @@ impl Network { let meta_data = utils::load_or_build_metadata(&config.network_dir, &log); let globals = NetworkGlobals::new( enr, - config.libp2p_port, - config.discovery_port, + config.listen_addrs().v4().map(|v4_addr| v4_addr.tcp_port), + config.listen_addrs().v6().map(|v6_addr| v6_addr.tcp_port), meta_data, config .trusted_peers @@ -388,36 +388,26 @@ impl Network { async fn start(&mut self, config: &crate::NetworkConfig) -> error::Result<()> { let enr = self.network_globals.local_enr(); info!(self.log, "Libp2p Starting"; "peer_id" => %enr.peer_id(), "bandwidth_config" => format!("{}-{}", config.network_load, NetworkLoad::from(config.network_load).name)); - let discovery_string = if config.disable_discovery { - "None".into() - } else { - config.discovery_port.to_string() - }; + debug!(self.log, "Attempting to open listening ports"; config.listen_addrs(), "discovery_enabled" => !config.disable_discovery); - debug!(self.log, "Attempting to open listening ports"; "address" => ?config.listen_address, "tcp_port" => config.libp2p_port, "udp_port" => discovery_string); - - let listen_multiaddr = { - let mut m = Multiaddr::from(config.listen_address); - m.push(MProtocol::Tcp(config.libp2p_port)); - m - }; - - match self.swarm.listen_on(listen_multiaddr.clone()) { - Ok(_) => { - let mut log_address = listen_multiaddr; - log_address.push(MProtocol::P2p(enr.peer_id().into())); - info!(self.log, "Listening established"; "address" => %log_address); - } - Err(err) => { - crit!( - self.log, - "Unable to listen on libp2p address"; - "error" => ?err, - "listen_multiaddr" => %listen_multiaddr, - ); - return Err("Libp2p was unable to listen on the given listen address.".into()); - } - }; + for listen_multiaddr in config.listen_addrs().tcp_addresses() { + match self.swarm.listen_on(listen_multiaddr.clone()) { + Ok(_) => { + let mut log_address = listen_multiaddr; + log_address.push(MProtocol::P2p(enr.peer_id().into())); + info!(self.log, "Listening established"; "address" => %log_address); + } + Err(err) => { + crit!( + self.log, + "Unable to listen on libp2p address"; + "error" => ?err, + "listen_multiaddr" => %listen_multiaddr, + ); + return Err("Libp2p was unable to listen on the given listen address.".into()); + } + }; + } // helper closure for dialing peers let mut dial = |mut multiaddr: Multiaddr| { diff --git a/beacon_node/lighthouse_network/src/types/globals.rs b/beacon_node/lighthouse_network/src/types/globals.rs index aadd13a23..ee2b300e2 100644 --- a/beacon_node/lighthouse_network/src/types/globals.rs +++ b/beacon_node/lighthouse_network/src/types/globals.rs @@ -7,7 +7,6 @@ use crate::EnrExt; use crate::{Enr, GossipTopic, Multiaddr, PeerId}; use parking_lot::RwLock; use std::collections::HashSet; -use std::sync::atomic::{AtomicU16, Ordering}; use types::EthSpec; pub struct NetworkGlobals { @@ -17,10 +16,10 @@ pub struct NetworkGlobals { pub peer_id: RwLock, /// Listening multiaddrs. pub listen_multiaddrs: RwLock>, - /// The TCP port that the libp2p service is listening on - pub listen_port_tcp: AtomicU16, - /// The UDP port that the discovery service is listening on - pub listen_port_udp: AtomicU16, + /// The TCP port that the libp2p service is listening on over Ipv4. + listen_port_tcp4: Option, + /// The TCP port that the libp2p service is listening on over Ipv6. + listen_port_tcp6: Option, /// The collection of known peers. pub peers: RwLock>, // The local meta data of our node. @@ -36,8 +35,8 @@ pub struct NetworkGlobals { impl NetworkGlobals { pub fn new( enr: Enr, - tcp_port: u16, - udp_port: u16, + listen_port_tcp4: Option, + listen_port_tcp6: Option, local_metadata: MetaData, trusted_peers: Vec, log: &slog::Logger, @@ -46,8 +45,8 @@ impl NetworkGlobals { local_enr: RwLock::new(enr.clone()), peer_id: RwLock::new(enr.peer_id()), listen_multiaddrs: RwLock::new(Vec::new()), - listen_port_tcp: AtomicU16::new(tcp_port), - listen_port_udp: AtomicU16::new(udp_port), + listen_port_tcp4, + listen_port_tcp6, local_metadata: RwLock::new(local_metadata), peers: RwLock::new(PeerDB::new(trusted_peers, log)), gossipsub_subscriptions: RwLock::new(HashSet::new()), @@ -73,13 +72,13 @@ impl NetworkGlobals { } /// Returns the libp2p TCP port that this node has been configured to listen on. - pub fn listen_port_tcp(&self) -> u16 { - self.listen_port_tcp.load(Ordering::Relaxed) + pub fn listen_port_tcp4(&self) -> Option { + self.listen_port_tcp4 } /// Returns the UDP discovery port that this node has been configured to listen on. - pub fn listen_port_udp(&self) -> u16 { - self.listen_port_udp.load(Ordering::Relaxed) + pub fn listen_port_tcp6(&self) -> Option { + self.listen_port_tcp6 } /// Returns the number of libp2p connected peers. @@ -137,8 +136,8 @@ impl NetworkGlobals { let enr = discv5::enr::EnrBuilder::new("v4").build(&enr_key).unwrap(); NetworkGlobals::new( enr, - 9000, - 9000, + Some(9000), + None, MetaData::V2(MetaDataV2 { seq_number: 0, attnets: Default::default(), diff --git a/beacon_node/lighthouse_network/tests/common.rs b/beacon_node/lighthouse_network/tests/common.rs index dfceb6c4c..d44f20c08 100644 --- a/beacon_node/lighthouse_network/tests/common.rs +++ b/beacon_node/lighthouse_network/tests/common.rs @@ -13,7 +13,7 @@ use tokio::runtime::Runtime; use types::{ ChainSpec, EnrForkId, Epoch, EthSpec, ForkContext, ForkName, Hash256, MinimalEthSpec, Slot, }; -use unused_port::unused_tcp_port; +use unused_port::unused_tcp4_port; type E = MinimalEthSpec; type ReqId = usize; @@ -75,11 +75,9 @@ pub fn build_config(port: u16, mut boot_nodes: Vec) -> NetworkConfig { .tempdir() .unwrap(); - config.libp2p_port = port; // tcp port - config.discovery_port = port; // udp port - config.enr_tcp_port = Some(port); - config.enr_udp_port = Some(port); - config.enr_address = Some("127.0.0.1".parse().unwrap()); + config.set_ipv4_listening_address(std::net::Ipv4Addr::UNSPECIFIED, port, port); + config.enr_udp4_port = Some(port); + config.enr_address = (Some(std::net::Ipv4Addr::LOCALHOST), None); config.boot_nodes_enr.append(&mut boot_nodes); config.network_dir = path.into_path(); // Reduce gossipsub heartbeat parameters @@ -97,7 +95,7 @@ pub async fn build_libp2p_instance( log: slog::Logger, fork_name: ForkName, ) -> Libp2pInstance { - let port = unused_tcp_port().unwrap(); + let port = unused_tcp4_port().unwrap(); let config = build_config(port, boot_nodes); // launch libp2p service diff --git a/beacon_node/network/src/beacon_processor/tests.rs b/beacon_node/network/src/beacon_processor/tests.rs index ea1a59e0d..eb66e434c 100644 --- a/beacon_node/network/src/beacon_processor/tests.rs +++ b/beacon_node/network/src/beacon_processor/tests.rs @@ -36,7 +36,6 @@ const SMALL_CHAIN: u64 = 2; const LONG_CHAIN: u64 = SLOTS_PER_EPOCH * 2; const TCP_PORT: u16 = 42; -const UDP_PORT: u16 = 42; const SEQ_NUMBER: u64 = 0; /// The default time to wait for `BeaconProcessor` events. @@ -177,8 +176,8 @@ impl TestRig { let enr = EnrBuilder::new("v4").build(&enr_key).unwrap(); let network_globals = Arc::new(NetworkGlobals::new( enr, - TCP_PORT, - UDP_PORT, + Some(TCP_PORT), + None, meta_data, vec![], &log, diff --git a/beacon_node/network/src/nat.rs b/beacon_node/network/src/nat.rs index a2fbe5761..9bf123e8d 100644 --- a/beacon_node/network/src/nat.rs +++ b/beacon_node/network/src/nat.rs @@ -20,13 +20,13 @@ pub struct UPnPConfig { disable_discovery: bool, } -impl From<&NetworkConfig> for UPnPConfig { - fn from(config: &NetworkConfig) -> Self { - UPnPConfig { - tcp_port: config.libp2p_port, - udp_port: config.discovery_port, +impl UPnPConfig { + pub fn from_config(config: &NetworkConfig) -> Option { + config.listen_addrs().v4().map(|v4_addr| UPnPConfig { + tcp_port: v4_addr.tcp_port, + udp_port: v4_addr.udp_port, disable_discovery: config.disable_discovery, - } + }) } } diff --git a/beacon_node/network/src/service.rs b/beacon_node/network/src/service.rs index 410461bcd..ab3d15aee 100644 --- a/beacon_node/network/src/service.rs +++ b/beacon_node/network/src/service.rs @@ -228,16 +228,21 @@ impl NetworkService { let (network_senders, network_recievers) = NetworkSenders::new(); // try and construct UPnP port mappings if required. - let upnp_config = crate::nat::UPnPConfig::from(config); - let upnp_log = network_log.new(o!("service" => "UPnP")); - let upnp_network_send = network_senders.network_send(); - if config.upnp_enabled { - executor.spawn_blocking( - move || { - crate::nat::construct_upnp_mappings(upnp_config, upnp_network_send, upnp_log) - }, - "UPnP", - ); + if let Some(upnp_config) = crate::nat::UPnPConfig::from_config(config) { + let upnp_log = network_log.new(o!("service" => "UPnP")); + let upnp_network_send = network_senders.network_send(); + if config.upnp_enabled { + executor.spawn_blocking( + move || { + crate::nat::construct_upnp_mappings( + upnp_config, + upnp_network_send, + upnp_log, + ) + }, + "UPnP", + ); + } } // get a reference to the beacon chain store diff --git a/beacon_node/network/src/service/tests.rs b/beacon_node/network/src/service/tests.rs index f0dd0e75f..83fcc8c9a 100644 --- a/beacon_node/network/src/service/tests.rs +++ b/beacon_node/network/src/service/tests.rs @@ -59,10 +59,9 @@ mod tests { ); let mut config = NetworkConfig::default(); + config.set_ipv4_listening_address(std::net::Ipv4Addr::UNSPECIFIED, 21212, 21212); config.discv5_config.table_filter = |_| true; // Do not ignore local IPs - config.libp2p_port = 21212; config.upnp_enabled = false; - config.discovery_port = 21212; config.boot_nodes_enr = enrs.clone(); runtime.block_on(async move { // Create a new network service which implicitly gets dropped at the diff --git a/beacon_node/src/cli.rs b/beacon_node/src/cli.rs index 792d62534..9ece45741 100644 --- a/beacon_node/src/cli.rs +++ b/beacon_node/src/cli.rs @@ -71,7 +71,16 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { Arg::with_name("listen-address") .long("listen-address") .value_name("ADDRESS") - .help("The address lighthouse will listen for UDP and TCP connections.") + .help("The address lighthouse will listen for UDP and TCP connections. To listen \ + over IpV4 and IpV6 set this flag twice with the different values.\n\ + Examples:\n\ + - --listen-address '0.0.0.0' will listen over Ipv4.\n\ + - --listen-address '::' will listen over Ipv6.\n\ + - --listen-address '0.0.0.0' --listen-address '::' will listen over both \ + Ipv4 and Ipv6. The order of the given addresses is not relevant. However, \ + multiple Ipv4, or multiple Ipv6 addresses will not be accepted.") + .multiple(true) + .max_values(2) .default_value("0.0.0.0") .takes_value(true) ) @@ -79,10 +88,21 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { Arg::with_name("port") .long("port") .value_name("PORT") - .help("The TCP/UDP port to listen on. The UDP port can be modified by the --discovery-port flag.") + .help("The TCP/UDP port to listen on. The UDP port can be modified by the \ + --discovery-port flag. If listening over both Ipv4 and Ipv6 the --port flag \ + will apply to the Ipv4 address and --port6 to the Ipv6 address.") .default_value("9000") .takes_value(true), ) + .arg( + Arg::with_name("port6") + .long("port6") + .value_name("PORT") + .help("The TCP/UDP port to listen on over IpV6 when listening over both Ipv4 and \ + Ipv6. Defaults to 9090 when required.") + .default_value("9090") + .takes_value(true), + ) .arg( Arg::with_name("discovery-port") .long("discovery-port") @@ -90,6 +110,15 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { .help("The UDP port that discovery will listen on. Defaults to `port`") .takes_value(true), ) + .arg( + Arg::with_name("discovery-port6") + .long("discovery-port6") + .value_name("PORT") + .help("The UDP port that discovery will listen on over IpV6 if listening over \ + both Ipv4 and IpV6. Defaults to `port6`") + .hidden(true) // TODO: implement dual stack via two sockets in discv5. + .takes_value(true), + ) .arg( Arg::with_name("target-peers") .long("target-peers") @@ -130,27 +159,49 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { Arg::with_name("enr-udp-port") .long("enr-udp-port") .value_name("PORT") - .help("The UDP port of the local ENR. Set this only if you are sure other nodes can connect to your local node on this port.") + .help("The UDP4 port of the local ENR. Set this only if you are sure other nodes \ + can connect to your local node on this port over IpV4.") + .takes_value(true), + ) + .arg( + Arg::with_name("enr-udp6-port") + .long("enr-udp6-port") + .value_name("PORT") + .help("The UDP6 port of the local ENR. Set this only if you are sure other nodes \ + can connect to your local node on this port over IpV6.") .takes_value(true), ) .arg( Arg::with_name("enr-tcp-port") .long("enr-tcp-port") .value_name("PORT") - .help("The TCP port of the local ENR. Set this only if you are sure other nodes can connect to your local node on this port.\ - The --port flag is used if this is not set.") + .help("The TCP4 port of the local ENR. Set this only if you are sure other nodes \ + can connect to your local node on this port over IpV4. The --port flag is \ + used if this is not set.") + .takes_value(true), + ) + .arg( + Arg::with_name("enr-tcp6-port") + .long("enr-tcp6-port") + .value_name("PORT") + .help("The TCP6 port of the local ENR. Set this only if you are sure other nodes \ + can connect to your local node on this port over IpV6. The --port6 flag is \ + used if this is not set.") .takes_value(true), ) .arg( Arg::with_name("enr-address") .long("enr-address") .value_name("ADDRESS") - .help("The IP address/ DNS address to broadcast to other peers on how to reach this node. \ - If a DNS address is provided, the enr-address is set to the IP address it resolves to and \ - does not auto-update based on PONG responses in discovery. \ - Set this only if you are sure other nodes can connect to your local node on this address. \ - Discovery will automatically find your external address, if possible.") + .help("The IP address/ DNS address to broadcast to other peers on how to reach \ + this node. If a DNS address is provided, the enr-address is set to the IP \ + address it resolves to and does not auto-update based on PONG responses in \ + discovery. Set this only if you are sure other nodes can connect to your \ + local node on this address. This will update the `ip4` or `ip6` ENR fields \ + accordingly. To update both, set this flag twice with the different values.") .requires("enr-udp-port") + .multiple(true) + .max_values(2) .takes_value(true), ) .arg( @@ -158,7 +209,8 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { .short("e") .long("enr-match") .help("Sets the local ENR IP address and port to match those set for lighthouse. \ - Specifically, the IP address will be the value of --listen-address and the UDP port will be --discovery-port.") + Specifically, the IP address will be the value of --listen-address and the \ + UDP port will be --discovery-port.") ) .arg( Arg::with_name("disable-enr-auto-update") diff --git a/beacon_node/src/config.rs b/beacon_node/src/config.rs index 55335081c..93ab1be94 100644 --- a/beacon_node/src/config.rs +++ b/beacon_node/src/config.rs @@ -10,13 +10,13 @@ use environment::RuntimeContext; use execution_layer::DEFAULT_JWT_FILE; use genesis::Eth1Endpoint; use http_api::TlsConfig; +use lighthouse_network::ListenAddress; use lighthouse_network::{multiaddr::Protocol, Enr, Multiaddr, NetworkConfig, PeerIdSerialized}; use sensitive_url::SensitiveUrl; use slog::{info, warn, Logger}; use std::cmp; use std::cmp::max; use std::fmt::Debug; -use std::fmt::Write; use std::fs; use std::net::Ipv6Addr; use std::net::{IpAddr, Ipv4Addr, ToSocketAddrs}; @@ -24,7 +24,6 @@ use std::path::{Path, PathBuf}; use std::str::FromStr; use std::time::Duration; use types::{Checkpoint, Epoch, EthSpec, Hash256, PublicKeyBytes, GRAFFITI_BYTES_LEN}; -use unused_port::{unused_tcp_port, unused_udp_port}; /// Gets the fully-initialized global client. /// @@ -78,13 +77,7 @@ pub fn get_config( let data_dir_ref = client_config.data_dir().clone(); - set_network_config( - &mut client_config.network, - cli_args, - &data_dir_ref, - log, - false, - )?; + set_network_config(&mut client_config.network, cli_args, &data_dir_ref, log)?; /* * Staking flag @@ -404,13 +397,6 @@ pub fn get_config( * Discovery address is set to localhost by default. */ if cli_args.is_present("zero-ports") { - if client_config.network.enr_address == Some(IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0))) { - client_config.network.enr_address = None - } - client_config.network.libp2p_port = - unused_tcp_port().map_err(|e| format!("Failed to get port for libp2p: {}", e))?; - client_config.network.discovery_port = - unused_udp_port().map_err(|e| format!("Failed to get port for discovery: {}", e))?; client_config.http_api.listen_port = 0; client_config.http_metrics.listen_port = 0; } @@ -761,13 +747,177 @@ pub fn get_config( Ok(client_config) } -/// Sets the network config from the command line arguments +/// Gets the listening_addresses for lighthouse based on the cli options. +pub fn parse_listening_addresses( + cli_args: &ArgMatches, + log: &Logger, +) -> Result { + let listen_addresses_str = cli_args + .values_of("listen-address") + .expect("--listen_addresses has a default value"); + + let use_zero_ports = cli_args.is_present("zero-ports"); + + // parse the possible ips + let mut maybe_ipv4 = None; + let mut maybe_ipv6 = None; + for addr_str in listen_addresses_str { + let addr = addr_str.parse::().map_err(|parse_error| { + format!("Failed to parse listen-address ({addr_str}) as an Ip address: {parse_error}") + })?; + + match addr { + IpAddr::V4(v4_addr) => match &maybe_ipv4 { + Some(first_ipv4_addr) => { + return Err(format!( + "When setting the --listen-address option twice, use an IpV4 address and an Ipv6 address. \ + Got two IpV4 addresses {first_ipv4_addr} and {v4_addr}" + )); + } + None => maybe_ipv4 = Some(v4_addr), + }, + IpAddr::V6(v6_addr) => match &maybe_ipv6 { + Some(first_ipv6_addr) => { + return Err(format!( + "When setting the --listen-address option twice, use an IpV4 address and an Ipv6 address. \ + Got two IpV6 addresses {first_ipv6_addr} and {v6_addr}" + )); + } + None => maybe_ipv6 = Some(v6_addr), + }, + } + } + + // parse the possible tcp ports + let port = cli_args + .value_of("port") + .expect("--port has a default value") + .parse::() + .map_err(|parse_error| format!("Failed to parse --port as an integer: {parse_error}"))?; + let port6 = cli_args + .value_of("port6") + .map(str::parse::) + .transpose() + .map_err(|parse_error| format!("Failed to parse --port6 as an integer: {parse_error}"))? + .unwrap_or(9090); + + // parse the possible udp ports + let maybe_udp_port = cli_args + .value_of("discovery-port") + .map(str::parse::) + .transpose() + .map_err(|parse_error| { + format!("Failed to parse --discovery-port as an integer: {parse_error}") + })?; + let maybe_udp6_port = cli_args + .value_of("discovery-port6") + .map(str::parse::) + .transpose() + .map_err(|parse_error| { + format!("Failed to parse --discovery-port6 as an integer: {parse_error}") + })?; + + // Now put everything together + let listening_addresses = match (maybe_ipv4, maybe_ipv6) { + (None, None) => { + // This should never happen unless clap is broken + return Err("No listening addresses provided".into()); + } + (None, Some(ipv6)) => { + // A single ipv6 address was provided. Set the ports + + if cli_args.is_present("port6") { + warn!(log, "When listening only over IpV6, use the --port flag. The value of --port6 will be ignored.") + } + // use zero ports if required. If not, use the given port. + let tcp_port = use_zero_ports + .then(unused_port::unused_tcp6_port) + .transpose()? + .unwrap_or(port); + + if maybe_udp6_port.is_some() { + warn!(log, "When listening only over IpV6, use the --discovery-port flag. The value of --discovery-port6 will be ignored.") + } + // use zero ports if required. If not, use the specific udp port. If none given, use + // the tcp port. + let udp_port = use_zero_ports + .then(unused_port::unused_udp6_port) + .transpose()? + .or(maybe_udp_port) + .unwrap_or(port); + + ListenAddress::V6(lighthouse_network::ListenAddr { + addr: ipv6, + udp_port, + tcp_port, + }) + } + (Some(ipv4), None) => { + // A single ipv4 address was provided. Set the ports + + // use zero ports if required. If not, use the given port. + let tcp_port = use_zero_ports + .then(unused_port::unused_tcp4_port) + .transpose()? + .unwrap_or(port); + // use zero ports if required. If not, use the specific udp port. If none given, use + // the tcp port. + let udp_port = use_zero_ports + .then(unused_port::unused_udp4_port) + .transpose()? + .or(maybe_udp_port) + .unwrap_or(port); + ListenAddress::V4(lighthouse_network::ListenAddr { + addr: ipv4, + udp_port, + tcp_port, + }) + } + (Some(ipv4), Some(ipv6)) => { + let ipv4_tcp_port = use_zero_ports + .then(unused_port::unused_tcp4_port) + .transpose()? + .unwrap_or(port); + let ipv4_udp_port = use_zero_ports + .then(unused_port::unused_udp4_port) + .transpose()? + .or(maybe_udp_port) + .unwrap_or(ipv4_tcp_port); + + // Defaults to 9090 when required + let ipv6_tcp_port = use_zero_ports + .then(unused_port::unused_tcp6_port) + .transpose()? + .unwrap_or(port6); + let ipv6_udp_port = use_zero_ports + .then(unused_port::unused_udp6_port) + .transpose()? + .or(maybe_udp6_port) + .unwrap_or(ipv6_tcp_port); + ListenAddress::DualStack( + lighthouse_network::ListenAddr { + addr: ipv4, + udp_port: ipv4_udp_port, + tcp_port: ipv4_tcp_port, + }, + lighthouse_network::ListenAddr { + addr: ipv6, + udp_port: ipv6_udp_port, + tcp_port: ipv6_tcp_port, + }, + ) + } + }; + + Ok(listening_addresses) +} + +/// Sets the network config from the command line arguments. pub fn set_network_config( config: &mut NetworkConfig, cli_args: &ArgMatches, data_dir: &Path, log: &Logger, - use_listening_port_as_enr_port_by_default: bool, ) -> Result<(), String> { // If a network dir has been specified, override the `datadir` definition. if let Some(dir) = cli_args.value_of("network-dir") { @@ -788,12 +938,7 @@ pub fn set_network_config( config.shutdown_after_sync = true; } - if let Some(listen_address_str) = cli_args.value_of("listen-address") { - let listen_address = listen_address_str - .parse() - .map_err(|_| format!("Invalid listen address: {:?}", listen_address_str))?; - config.listen_address = listen_address; - } + config.set_listening_addr(parse_listening_addresses(cli_args, log)?); if let Some(target_peers_str) = cli_args.value_of("target-peers") { config.target_peers = target_peers_str @@ -801,21 +946,6 @@ pub fn set_network_config( .map_err(|_| format!("Invalid number of target peers: {}", target_peers_str))?; } - if let Some(port_str) = cli_args.value_of("port") { - let port = port_str - .parse::() - .map_err(|_| format!("Invalid port: {}", port_str))?; - config.libp2p_port = port; - config.discovery_port = port; - } - - if let Some(port_str) = cli_args.value_of("discovery-port") { - let port = port_str - .parse::() - .map_err(|_| format!("Invalid port: {}", port_str))?; - config.discovery_port = port; - } - if let Some(value) = cli_args.value_of("network-load") { let network_load = value .parse::() @@ -871,7 +1001,7 @@ pub fn set_network_config( } if let Some(enr_udp_port_str) = cli_args.value_of("enr-udp-port") { - config.enr_udp_port = Some( + config.enr_udp4_port = Some( enr_udp_port_str .parse::() .map_err(|_| format!("Invalid discovery port: {}", enr_udp_port_str))?, @@ -879,7 +1009,23 @@ pub fn set_network_config( } if let Some(enr_tcp_port_str) = cli_args.value_of("enr-tcp-port") { - config.enr_tcp_port = Some( + config.enr_tcp4_port = Some( + enr_tcp_port_str + .parse::() + .map_err(|_| format!("Invalid ENR TCP port: {}", enr_tcp_port_str))?, + ); + } + + if let Some(enr_udp_port_str) = cli_args.value_of("enr-udp6-port") { + config.enr_udp6_port = Some( + enr_udp_port_str + .parse::() + .map_err(|_| format!("Invalid discovery port: {}", enr_udp_port_str))?, + ); + } + + if let Some(enr_tcp_port_str) = cli_args.value_of("enr-tcp6-port") { + config.enr_tcp6_port = Some( enr_tcp_port_str .parse::() .map_err(|_| format!("Invalid ENR TCP port: {}", enr_tcp_port_str))?, @@ -887,58 +1033,106 @@ pub fn set_network_config( } if cli_args.is_present("enr-match") { + // Match the Ip and UDP port in the enr. + // set the enr address to localhost if the address is unspecified - if config.listen_address == IpAddr::V4(Ipv4Addr::UNSPECIFIED) { - config.enr_address = Some(IpAddr::V4(Ipv4Addr::LOCALHOST)); - } else if config.listen_address == IpAddr::V6(Ipv6Addr::UNSPECIFIED) { - config.enr_address = Some(IpAddr::V6(Ipv6Addr::LOCALHOST)); - } else { - config.enr_address = Some(config.listen_address); + if let Some(ipv4_addr) = config.listen_addrs().v4().cloned() { + let ipv4_enr_addr = if ipv4_addr.addr == Ipv4Addr::UNSPECIFIED { + Ipv4Addr::LOCALHOST + } else { + ipv4_addr.addr + }; + config.enr_address.0 = Some(ipv4_enr_addr); + config.enr_udp4_port = Some(ipv4_addr.udp_port); + } + + if let Some(ipv6_addr) = config.listen_addrs().v6().cloned() { + let ipv6_enr_addr = if ipv6_addr.addr == Ipv6Addr::UNSPECIFIED { + Ipv6Addr::LOCALHOST + } else { + ipv6_addr.addr + }; + config.enr_address.1 = Some(ipv6_enr_addr); + config.enr_udp6_port = Some(ipv6_addr.udp_port); } - config.enr_udp_port = Some(config.discovery_port); } - if let Some(enr_address) = cli_args.value_of("enr-address") { - let resolved_addr = match enr_address.parse::() { - Ok(addr) => addr, // // Input is an IpAddr - Err(_) => { - let mut addr = enr_address.to_string(); - // Appending enr-port to the dns hostname to appease `to_socket_addrs()` parsing. - // Since enr-update is disabled with a dns address, not setting the enr-udp-port - // will make the node undiscoverable. - if let Some(enr_udp_port) = - config - .enr_udp_port - .or(if use_listening_port_as_enr_port_by_default { - Some(config.discovery_port) - } else { - None - }) - { - write!(addr, ":{}", enr_udp_port) - .map_err(|e| format!("Failed to write enr address {}", e))?; - } else { - return Err( - "enr-udp-port must be set for node to be discoverable with dns address" - .into(), - ); + if let Some(enr_addresses) = cli_args.values_of("enr-address") { + let mut enr_ip4 = None; + let mut enr_ip6 = None; + let mut resolved_enr_ip4 = None; + let mut resolved_enr_ip6 = None; + + for addr in enr_addresses { + match addr.parse::() { + Ok(IpAddr::V4(v4_addr)) => { + if let Some(used) = enr_ip4.as_ref() { + warn!(log, "More than one Ipv4 ENR address provided"; "used" => %used, "ignored" => %v4_addr) + } else { + enr_ip4 = Some(v4_addr) + } + } + Ok(IpAddr::V6(v6_addr)) => { + if let Some(used) = enr_ip6.as_ref() { + warn!(log, "More than one Ipv6 ENR address provided"; "used" => %used, "ignored" => %v6_addr) + } else { + enr_ip6 = Some(v6_addr) + } + } + Err(_) => { + // Try to resolve the address + + // NOTE: From checking the `to_socket_addrs` code I don't think the port + // actually matters. Just use the udp port. + + let port = match config.listen_addrs() { + ListenAddress::V4(v4_addr) => v4_addr.udp_port, + ListenAddress::V6(v6_addr) => v6_addr.udp_port, + ListenAddress::DualStack(v4_addr, _v6_addr) => { + // NOTE: slight preference for ipv4 that I don't think is of importance. + v4_addr.udp_port + } + }; + + let addr_str = format!("{addr}:{port}"); + match addr_str.to_socket_addrs() { + Err(_e) => { + return Err(format!("Failed to parse or resolve address {addr}.")) + } + Ok(resolved_addresses) => { + for socket_addr in resolved_addresses { + // Use the first ipv4 and first ipv6 addresses present. + + // NOTE: this means that if two dns addresses are provided, we + // might end up using the ipv4 and ipv6 resolved addresses of just + // the first. + match socket_addr.ip() { + IpAddr::V4(v4_addr) => { + if resolved_enr_ip4.is_none() { + resolved_enr_ip4 = Some(v4_addr) + } + } + IpAddr::V6(v6_addr) => { + if resolved_enr_ip6.is_none() { + resolved_enr_ip6 = Some(v6_addr) + } + } + } + } + } + } } - // `to_socket_addr()` does the dns resolution - // Note: `to_socket_addrs()` is a blocking call - let resolved_addr = if let Ok(mut resolved_addrs) = addr.to_socket_addrs() { - // Pick the first ip from the list of resolved addresses - resolved_addrs - .next() - .map(|a| a.ip()) - .ok_or("Resolved dns addr contains no entries")? - } else { - return Err(format!("Failed to parse enr-address: {}", enr_address)); - }; - config.discv5_config.enr_update = false; - resolved_addr } - }; - config.enr_address = Some(resolved_addr); + } + + // The ENR addresses given as ips should take preference over any resolved address + let used_host_resolution = resolved_enr_ip4.is_some() || resolved_enr_ip6.is_some(); + let ip4 = enr_ip4.or(resolved_enr_ip4); + let ip6 = enr_ip6.or(resolved_enr_ip6); + config.enr_address = (ip4, ip6); + if used_host_resolution { + config.discv5_config.enr_update = false; + } } if cli_args.is_present("disable-enr-auto-update") { diff --git a/boot_node/src/cli.rs b/boot_node/src/cli.rs index 9a3732002..c3d7ac48a 100644 --- a/boot_node/src/cli.rs +++ b/boot_node/src/cli.rs @@ -53,6 +53,14 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { .takes_value(true) .conflicts_with("network-dir") ) + .arg( + Arg::with_name("enr-udp6-port") + .long("enr-udp6-port") + .value_name("PORT") + .help("The UDP6 port of the local ENR. Set this only if you are sure other nodes \ + can connect to your local node on this port over IpV6.") + .takes_value(true), + ) .arg( Arg::with_name("enable-enr-auto-update") .short("x") diff --git a/boot_node/src/config.rs b/boot_node/src/config.rs index b7a66cbbd..d3ee58a90 100644 --- a/boot_node/src/config.rs +++ b/boot_node/src/config.rs @@ -1,7 +1,7 @@ use beacon_node::{get_data_dir, set_network_config}; use clap::ArgMatches; use eth2_network_config::Eth2NetworkConfig; -use lighthouse_network::discv5::enr::EnrBuilder; +use lighthouse_network::discovery::create_enr_builder_from_config; use lighthouse_network::discv5::IpMode; use lighthouse_network::discv5::{enr::CombinedKey, Discv5Config, Enr}; use lighthouse_network::{ @@ -57,12 +57,24 @@ impl BootNodeConfig { let logger = slog_scope::logger(); - set_network_config(&mut network_config, matches, &data_dir, &logger, true)?; + set_network_config(&mut network_config, matches, &data_dir, &logger)?; - // Set the enr-udp-port to the default listening port if it was not specified. - if !matches.is_present("enr-udp-port") { - network_config.enr_udp_port = Some(network_config.discovery_port); - } + // Set the Enr UDP ports to the listening ports if not present. + if let Some(listening_addr_v4) = network_config.listen_addrs().v4() { + network_config.enr_udp4_port = Some( + network_config + .enr_udp4_port + .unwrap_or(listening_addr_v4.udp_port), + ) + }; + + if let Some(listening_addr_v6) = network_config.listen_addrs().v6() { + network_config.enr_udp6_port = Some( + network_config + .enr_udp6_port + .unwrap_or(listening_addr_v6.udp_port), + ) + }; // By default this is enabled. If it is not set, revert to false. if !matches.is_present("enable-enr-auto-update") { @@ -70,17 +82,29 @@ impl BootNodeConfig { } // the address to listen on - let listen_socket = - SocketAddr::new(network_config.listen_address, network_config.discovery_port); - if listen_socket.is_ipv6() { - // create ipv6 sockets and enable ipv4 mapped addresses. - network_config.discv5_config.ip_mode = IpMode::Ip6 { - enable_mapped_addresses: true, - }; - } else { - // Set explicitly as ipv4 otherwise - network_config.discv5_config.ip_mode = IpMode::Ip4; - } + let listen_socket = match network_config.listen_addrs().clone() { + lighthouse_network::ListenAddress::V4(v4_addr) => { + // Set explicitly as ipv4 otherwise + network_config.discv5_config.ip_mode = IpMode::Ip4; + v4_addr.udp_socket_addr() + } + lighthouse_network::ListenAddress::V6(v6_addr) => { + // create ipv6 sockets and enable ipv4 mapped addresses. + network_config.discv5_config.ip_mode = IpMode::Ip6 { + enable_mapped_addresses: false, + }; + + v6_addr.udp_socket_addr() + } + lighthouse_network::ListenAddress::DualStack(_v4_addr, v6_addr) => { + // create ipv6 sockets and enable ipv4 mapped addresses. + network_config.discv5_config.ip_mode = IpMode::Ip6 { + enable_mapped_addresses: true, + }; + + v6_addr.udp_socket_addr() + } + }; let private_key = load_private_key(&network_config, &logger); let local_key = CombinedKey::from_libp2p(&private_key)?; @@ -115,30 +139,8 @@ impl BootNodeConfig { // Build the local ENR let mut local_enr = { - let mut builder = EnrBuilder::new("v4"); - // Set the enr address if specified. Set also the port. - // NOTE: if the port is specified but the the address is not, the port won't be - // set since it can't be known if it's an ipv6 or ipv4 udp port. - if let Some(enr_address) = network_config.enr_address { - match enr_address { - std::net::IpAddr::V4(ipv4_addr) => { - builder.ip4(ipv4_addr); - if let Some(port) = network_config.enr_udp_port { - builder.udp4(port); - } - } - std::net::IpAddr::V6(ipv6_addr) => { - builder.ip6(ipv6_addr); - if let Some(port) = network_config.enr_udp_port { - builder.udp6(port); - // We are enabling mapped addresses in the boot node in this case, - // so advertise an udp4 port as well. - builder.udp4(port); - } - } - } - }; - + let enable_tcp = false; + let mut builder = create_enr_builder_from_config(&network_config, enable_tcp); // If we know of the ENR field, add it to the initial construction if let Some(enr_fork_bytes) = enr_fork { builder.add_value("eth2", enr_fork_bytes.as_slice()); diff --git a/common/unused_port/src/lib.rs b/common/unused_port/src/lib.rs index 4a8cf1738..a5d081721 100644 --- a/common/unused_port/src/lib.rs +++ b/common/unused_port/src/lib.rs @@ -6,14 +6,30 @@ pub enum Transport { Udp, } -/// A convenience function for `unused_port(Transport::Tcp)`. -pub fn unused_tcp_port() -> Result { - unused_port(Transport::Tcp) +#[derive(Copy, Clone)] +pub enum IpVersion { + Ipv4, + Ipv6, } -/// A convenience function for `unused_port(Transport::Tcp)`. -pub fn unused_udp_port() -> Result { - unused_port(Transport::Udp) +/// A convenience wrapper over [`zero_port`]. +pub fn unused_tcp4_port() -> Result { + zero_port(Transport::Tcp, IpVersion::Ipv4) +} + +/// A convenience wrapper over [`zero_port`]. +pub fn unused_udp4_port() -> Result { + zero_port(Transport::Udp, IpVersion::Ipv4) +} + +/// A convenience wrapper over [`zero_port`]. +pub fn unused_tcp6_port() -> Result { + zero_port(Transport::Tcp, IpVersion::Ipv6) +} + +/// A convenience wrapper over [`zero_port`]. +pub fn unused_udp6_port() -> Result { + zero_port(Transport::Udp, IpVersion::Ipv6) } /// A bit of hack to find an unused port. @@ -26,10 +42,15 @@ pub fn unused_udp_port() -> Result { /// It is possible that users are unable to bind to the ports returned by this function as the OS /// has a buffer period where it doesn't allow binding to the same port even after the socket is /// closed. We might have to use SO_REUSEADDR socket option from `std::net2` crate in that case. -pub fn unused_port(transport: Transport) -> Result { +pub fn zero_port(transport: Transport, ipv: IpVersion) -> Result { + let localhost = match ipv { + IpVersion::Ipv4 => std::net::Ipv4Addr::LOCALHOST.into(), + IpVersion::Ipv6 => std::net::Ipv6Addr::LOCALHOST.into(), + }; + let socket_addr = std::net::SocketAddr::new(localhost, 0); let local_addr = match transport { Transport::Tcp => { - let listener = TcpListener::bind("127.0.0.1:0").map_err(|e| { + let listener = TcpListener::bind(socket_addr).map_err(|e| { format!("Failed to create TCP listener to find unused port: {:?}", e) })?; listener.local_addr().map_err(|e| { @@ -40,7 +61,7 @@ pub fn unused_port(transport: Transport) -> Result { })? } Transport::Udp => { - let socket = UdpSocket::bind("127.0.0.1:0") + let socket = UdpSocket::bind(socket_addr) .map_err(|e| format!("Failed to create UDP socket to find unused port: {:?}", e))?; socket.local_addr().map_err(|e| { format!( diff --git a/lcli/src/generate_bootnode_enr.rs b/lcli/src/generate_bootnode_enr.rs index 6f39392d1..8662a8047 100644 --- a/lcli/src/generate_bootnode_enr.rs +++ b/lcli/src/generate_bootnode_enr.rs @@ -3,15 +3,14 @@ use lighthouse_network::{ discovery::{build_enr, CombinedKey, CombinedKeyExt, Keypair, ENR_FILENAME}, NetworkConfig, NETWORK_KEY_FILENAME, }; -use std::fs; use std::fs::File; use std::io::Write; -use std::net::IpAddr; use std::path::PathBuf; +use std::{fs, net::Ipv4Addr}; use types::{ChainSpec, EnrForkId, Epoch, EthSpec, Hash256}; pub fn run(matches: &ArgMatches) -> Result<(), String> { - let ip: IpAddr = clap_utils::parse_required(matches, "ip")?; + let ip: Ipv4Addr = clap_utils::parse_required(matches, "ip")?; let udp_port: u16 = clap_utils::parse_required(matches, "udp-port")?; let tcp_port: u16 = clap_utils::parse_required(matches, "tcp-port")?; let output_dir: PathBuf = clap_utils::parse_required(matches, "output-dir")?; @@ -25,12 +24,10 @@ pub fn run(matches: &ArgMatches) -> Result<(), String> { )); } - let config = NetworkConfig { - enr_address: Some(ip), - enr_udp_port: Some(udp_port), - enr_tcp_port: Some(tcp_port), - ..Default::default() - }; + let mut config = NetworkConfig::default(); + config.enr_address = (Some(ip), None); + config.enr_udp4_port = Some(udp_port); + config.enr_tcp6_port = Some(tcp_port); let local_keypair = Keypair::generate_secp256k1(); let enr_key = CombinedKey::from_libp2p(&local_keypair)?; diff --git a/lighthouse/tests/beacon_node.rs b/lighthouse/tests/beacon_node.rs index 7f957b626..63ff6f79b 100644 --- a/lighthouse/tests/beacon_node.rs +++ b/lighthouse/tests/beacon_node.rs @@ -8,7 +8,7 @@ use eth1::Eth1Endpoint; use lighthouse_network::PeerId; use std::fs::File; use std::io::{Read, Write}; -use std::net::IpAddr; +use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; use std::path::PathBuf; use std::process::Command; use std::str::FromStr; @@ -16,7 +16,7 @@ use std::string::ToString; use std::time::Duration; use tempfile::TempDir; use types::{Address, Checkpoint, Epoch, ExecutionBlockHash, ForkName, Hash256, MainnetEthSpec}; -use unused_port::{unused_tcp_port, unused_udp_port}; +use unused_port::{unused_tcp4_port, unused_tcp6_port, unused_udp4_port, unused_udp6_port}; const DEFAULT_ETH1_ENDPOINT: &str = "http://localhost:8545/"; @@ -851,37 +851,188 @@ fn network_shutdown_after_sync_disabled_flag() { .with_config(|config| assert!(!config.network.shutdown_after_sync)); } #[test] -fn network_listen_address_flag() { - let addr = "127.0.0.2".parse::().unwrap(); +fn network_listen_address_flag_v4() { + let addr = "127.0.0.2".parse::().unwrap(); CommandLineTest::new() .flag("listen-address", Some("127.0.0.2")) .run_with_zero_port() - .with_config(|config| assert_eq!(config.network.listen_address, addr)); + .with_config(|config| { + assert_eq!( + config.network.listen_addrs().v4().map(|addr| addr.addr), + Some(addr) + ) + }); } #[test] -fn network_port_flag() { - let port = unused_tcp_port().expect("Unable to find unused port."); +fn network_listen_address_flag_v6() { + const ADDR: &str = "::1"; + let addr = ADDR.parse::().unwrap(); + CommandLineTest::new() + .flag("listen-address", Some(ADDR)) + .run_with_zero_port() + .with_config(|config| { + assert_eq!( + config.network.listen_addrs().v6().map(|addr| addr.addr), + Some(addr) + ) + }); +} +#[test] +fn network_listen_address_flag_dual_stack() { + const V4_ADDR: &str = "127.0.0.1"; + const V6_ADDR: &str = "::1"; + let ipv6_addr = V6_ADDR.parse::().unwrap(); + let ipv4_addr = V4_ADDR.parse::().unwrap(); + CommandLineTest::new() + .flag("listen-address", Some(V6_ADDR)) + .flag("listen-address", Some(V4_ADDR)) + .run_with_zero_port() + .with_config(|config| { + assert_eq!( + config.network.listen_addrs().v6().map(|addr| addr.addr), + Some(ipv6_addr) + ); + assert_eq!( + config.network.listen_addrs().v4().map(|addr| addr.addr), + Some(ipv4_addr) + ) + }); +} +#[test] +#[should_panic] +fn network_listen_address_flag_wrong_double_v4_value_config() { + // It's actually possible to listen over multiple sockets in libp2p over the same ip version. + // However this is not compatible with the single contactable address over each version in ENR. + // Because of this, it's important to test this is disallowed. + const V4_ADDR1: &str = "127.0.0.1"; + const V4_ADDR2: &str = "0.0.0.0"; + CommandLineTest::new() + .flag("listen-address", Some(V4_ADDR1)) + .flag("listen-address", Some(V4_ADDR2)) + .run_with_zero_port(); +} +#[test] +#[should_panic] +fn network_listen_address_flag_wrong_double_v6_value_config() { + // It's actually possible to listen over multiple sockets in libp2p over the same ip version. + // However this is not compatible with the single contactable address over each version in ENR. + // Because of this, it's important to test this is disallowed. + const V6_ADDR1: &str = "::3"; + const V6_ADDR2: &str = "::1"; + CommandLineTest::new() + .flag("listen-address", Some(V6_ADDR1)) + .flag("listen-address", Some(V6_ADDR2)) + .run_with_zero_port(); +} +#[test] +fn network_port_flag_over_ipv4() { + let port = unused_tcp4_port().expect("Unable to find unused port."); CommandLineTest::new() .flag("port", Some(port.to_string().as_str())) .run() .with_config(|config| { - assert_eq!(config.network.libp2p_port, port); - assert_eq!(config.network.discovery_port, port); + assert_eq!( + config + .network + .listen_addrs() + .v4() + .map(|listen_addr| (listen_addr.udp_port, listen_addr.tcp_port)), + Some((port, port)) + ); }); } #[test] -fn network_port_and_discovery_port_flags() { - let port1 = unused_tcp_port().expect("Unable to find unused port."); - let port2 = unused_udp_port().expect("Unable to find unused port."); +fn network_port_flag_over_ipv6() { + let port = unused_tcp6_port().expect("Unable to find unused port."); CommandLineTest::new() - .flag("port", Some(port1.to_string().as_str())) - .flag("discovery-port", Some(port2.to_string().as_str())) + .flag("listen-address", Some("::1")) + .flag("port", Some(port.to_string().as_str())) .run() .with_config(|config| { - assert_eq!(config.network.libp2p_port, port1); - assert_eq!(config.network.discovery_port, port2); + assert_eq!( + config + .network + .listen_addrs() + .v6() + .map(|listen_addr| (listen_addr.udp_port, listen_addr.tcp_port)), + Some((port, port)) + ); }); } +#[test] +fn network_port_and_discovery_port_flags_over_ipv4() { + let tcp4_port = unused_tcp4_port().expect("Unable to find unused port."); + let udp4_port = unused_udp4_port().expect("Unable to find unused port."); + CommandLineTest::new() + .flag("port", Some(tcp4_port.to_string().as_str())) + .flag("discovery-port", Some(udp4_port.to_string().as_str())) + .run() + .with_config(|config| { + assert_eq!( + config + .network + .listen_addrs() + .v4() + .map(|listen_addr| (listen_addr.tcp_port, listen_addr.udp_port)), + Some((tcp4_port, udp4_port)) + ); + }); +} +#[test] +fn network_port_and_discovery_port_flags_over_ipv6() { + let tcp6_port = unused_tcp6_port().expect("Unable to find unused port."); + let udp6_port = unused_udp6_port().expect("Unable to find unused port."); + CommandLineTest::new() + .flag("listen-address", Some("::1")) + .flag("port", Some(tcp6_port.to_string().as_str())) + .flag("discovery-port", Some(udp6_port.to_string().as_str())) + .run() + .with_config(|config| { + assert_eq!( + config + .network + .listen_addrs() + .v6() + .map(|listen_addr| (listen_addr.tcp_port, listen_addr.udp_port)), + Some((tcp6_port, udp6_port)) + ); + }); +} +#[test] +fn network_port_and_discovery_port_flags_over_ipv4_and_ipv6() { + let tcp4_port = unused_tcp4_port().expect("Unable to find unused port."); + let udp4_port = unused_udp4_port().expect("Unable to find unused port."); + let tcp6_port = unused_tcp6_port().expect("Unable to find unused port."); + let udp6_port = unused_udp6_port().expect("Unable to find unused port."); + CommandLineTest::new() + .flag("listen-address", Some("::1")) + .flag("listen-address", Some("127.0.0.1")) + .flag("port", Some(tcp4_port.to_string().as_str())) + .flag("discovery-port", Some(udp4_port.to_string().as_str())) + .flag("port6", Some(tcp6_port.to_string().as_str())) + .flag("discovery-port6", Some(udp6_port.to_string().as_str())) + .run() + .with_config(|config| { + assert_eq!( + config + .network + .listen_addrs() + .v4() + .map(|listen_addr| (listen_addr.tcp_port, listen_addr.udp_port)), + Some((tcp4_port, udp4_port)) + ); + + assert_eq!( + config + .network + .listen_addrs() + .v6() + .map(|listen_addr| (listen_addr.tcp_port, listen_addr.udp_port)), + Some((tcp6_port, udp6_port)) + ); + }); +} + #[test] fn disable_discovery_flag() { CommandLineTest::new() @@ -986,7 +1137,6 @@ fn zero_ports_flag() { CommandLineTest::new() .run_with_zero_port() .with_config(|config| { - assert_eq!(config.network.enr_address, None); assert_eq!(config.http_api.listen_port, 0); assert_eq!(config.http_metrics.listen_port, 0); }); @@ -1003,67 +1153,171 @@ fn network_load_flag() { // Tests for ENR flags. #[test] -fn enr_udp_port_flags() { - let port = unused_udp_port().expect("Unable to find unused port."); +fn enr_udp_port_flag() { + let port = unused_udp4_port().expect("Unable to find unused port."); CommandLineTest::new() .flag("enr-udp-port", Some(port.to_string().as_str())) .run_with_zero_port() - .with_config(|config| assert_eq!(config.network.enr_udp_port, Some(port))); + .with_config(|config| assert_eq!(config.network.enr_udp4_port, Some(port))); } #[test] -fn enr_tcp_port_flags() { - let port = unused_tcp_port().expect("Unable to find unused port."); +fn enr_tcp_port_flag() { + let port = unused_tcp4_port().expect("Unable to find unused port."); CommandLineTest::new() .flag("enr-tcp-port", Some(port.to_string().as_str())) .run_with_zero_port() - .with_config(|config| assert_eq!(config.network.enr_tcp_port, Some(port))); + .with_config(|config| assert_eq!(config.network.enr_tcp4_port, Some(port))); } #[test] -fn enr_match_flag() { - let addr = "127.0.0.2".parse::().unwrap(); - let port1 = unused_udp_port().expect("Unable to find unused port."); - let port2 = unused_udp_port().expect("Unable to find unused port."); +fn enr_udp6_port_flag() { + let port = unused_udp6_port().expect("Unable to find unused port."); + CommandLineTest::new() + .flag("enr-udp6-port", Some(port.to_string().as_str())) + .run_with_zero_port() + .with_config(|config| assert_eq!(config.network.enr_udp6_port, Some(port))); +} +#[test] +fn enr_tcp6_port_flag() { + let port = unused_tcp6_port().expect("Unable to find unused port."); + CommandLineTest::new() + .flag("enr-tcp6-port", Some(port.to_string().as_str())) + .run_with_zero_port() + .with_config(|config| assert_eq!(config.network.enr_tcp6_port, Some(port))); +} +#[test] +fn enr_match_flag_over_ipv4() { + let addr = "127.0.0.2".parse::().unwrap(); + let udp4_port = unused_udp4_port().expect("Unable to find unused port."); + let tcp4_port = unused_tcp4_port().expect("Unable to find unused port."); CommandLineTest::new() .flag("enr-match", None) .flag("listen-address", Some("127.0.0.2")) - .flag("discovery-port", Some(port1.to_string().as_str())) - .flag("port", Some(port2.to_string().as_str())) + .flag("discovery-port", Some(udp4_port.to_string().as_str())) + .flag("port", Some(tcp4_port.to_string().as_str())) .run() .with_config(|config| { - assert_eq!(config.network.listen_address, addr); - assert_eq!(config.network.enr_address, Some(addr)); - assert_eq!(config.network.discovery_port, port1); - assert_eq!(config.network.enr_udp_port, Some(port1)); + assert_eq!( + config.network.listen_addrs().v4().map(|listen_addr| ( + listen_addr.addr, + listen_addr.udp_port, + listen_addr.tcp_port + )), + Some((addr, udp4_port, tcp4_port)) + ); + assert_eq!(config.network.enr_address, (Some(addr), None)); + assert_eq!(config.network.enr_udp4_port, Some(udp4_port)); }); } #[test] -fn enr_address_flag() { - let addr = "192.167.1.1".parse::().unwrap(); - let port = unused_udp_port().expect("Unable to find unused port."); +fn enr_match_flag_over_ipv6() { + const ADDR: &str = "::1"; + let addr = ADDR.parse::().unwrap(); + let udp6_port = unused_udp6_port().expect("Unable to find unused port."); + let tcp6_port = unused_tcp6_port().expect("Unable to find unused port."); + CommandLineTest::new() + .flag("enr-match", None) + .flag("listen-address", Some(ADDR)) + .flag("discovery-port", Some(udp6_port.to_string().as_str())) + .flag("port", Some(tcp6_port.to_string().as_str())) + .run() + .with_config(|config| { + assert_eq!( + config.network.listen_addrs().v6().map(|listen_addr| ( + listen_addr.addr, + listen_addr.udp_port, + listen_addr.tcp_port + )), + Some((addr, udp6_port, tcp6_port)) + ); + assert_eq!(config.network.enr_address, (None, Some(addr))); + assert_eq!(config.network.enr_udp6_port, Some(udp6_port)); + }); +} +#[test] +fn enr_match_flag_over_ipv4_and_ipv6() { + const IPV6_ADDR: &str = "::1"; + let ipv6_addr = IPV6_ADDR.parse::().unwrap(); + let udp6_port = unused_udp6_port().expect("Unable to find unused port."); + let tcp6_port = unused_tcp6_port().expect("Unable to find unused port."); + const IPV4_ADDR: &str = "127.0.0.1"; + let ipv4_addr = IPV4_ADDR.parse::().unwrap(); + let udp4_port = unused_udp4_port().expect("Unable to find unused port."); + let tcp4_port = unused_tcp4_port().expect("Unable to find unused port."); + CommandLineTest::new() + .flag("enr-match", None) + .flag("listen-address", Some(IPV4_ADDR)) + .flag("discovery-port", Some(udp4_port.to_string().as_str())) + .flag("port", Some(tcp4_port.to_string().as_str())) + .flag("listen-address", Some(IPV6_ADDR)) + .flag("discovery-port6", Some(udp6_port.to_string().as_str())) + .flag("port6", Some(tcp6_port.to_string().as_str())) + .run() + .with_config(|config| { + assert_eq!( + config.network.listen_addrs().v6().map(|listen_addr| ( + listen_addr.addr, + listen_addr.udp_port, + listen_addr.tcp_port + )), + Some((ipv6_addr, udp6_port, tcp6_port)) + ); + assert_eq!( + config.network.listen_addrs().v4().map(|listen_addr| ( + listen_addr.addr, + listen_addr.udp_port, + listen_addr.tcp_port + )), + Some((ipv4_addr, udp4_port, tcp4_port)) + ); + assert_eq!( + config.network.enr_address, + (Some(ipv4_addr), Some(ipv6_addr)) + ); + assert_eq!(config.network.enr_udp6_port, Some(udp6_port)); + assert_eq!(config.network.enr_udp4_port, Some(udp4_port)); + }); +} +#[test] +fn enr_address_flag_with_ipv4() { + let addr = "192.167.1.1".parse::().unwrap(); + let port = unused_udp4_port().expect("Unable to find unused port."); CommandLineTest::new() .flag("enr-address", Some("192.167.1.1")) .flag("enr-udp-port", Some(port.to_string().as_str())) .run_with_zero_port() .with_config(|config| { - assert_eq!(config.network.enr_address, Some(addr)); - assert_eq!(config.network.enr_udp_port, Some(port)); + assert_eq!(config.network.enr_address, (Some(addr), None)); + assert_eq!(config.network.enr_udp4_port, Some(port)); + }); +} +#[test] +fn enr_address_flag_with_ipv6() { + let addr = "192.167.1.1".parse::().unwrap(); + let port = unused_udp4_port().expect("Unable to find unused port."); + CommandLineTest::new() + .flag("enr-address", Some("192.167.1.1")) + .flag("enr-udp-port", Some(port.to_string().as_str())) + .run_with_zero_port() + .with_config(|config| { + assert_eq!(config.network.enr_address, (Some(addr), None)); + assert_eq!(config.network.enr_udp4_port, Some(port)); }); } #[test] fn enr_address_dns_flag() { - let addr = "127.0.0.1".parse::().unwrap(); - let ipv6addr = "::1".parse::().unwrap(); - let port = unused_udp_port().expect("Unable to find unused port."); + let addr = Ipv4Addr::LOCALHOST; + let ipv6addr = Ipv6Addr::LOCALHOST; + let port = unused_udp4_port().expect("Unable to find unused port."); CommandLineTest::new() .flag("enr-address", Some("localhost")) .flag("enr-udp-port", Some(port.to_string().as_str())) .run_with_zero_port() .with_config(|config| { assert!( - config.network.enr_address == Some(addr) - || config.network.enr_address == Some(ipv6addr) + config.network.enr_address.0 == Some(addr) + || config.network.enr_address.1 == Some(ipv6addr) ); - assert_eq!(config.network.enr_udp_port, Some(port)); + assert_eq!(config.network.enr_udp4_port, Some(port)); }); } #[test] @@ -1100,8 +1354,8 @@ fn http_address_ipv6_flag() { } #[test] fn http_port_flag() { - let port1 = unused_tcp_port().expect("Unable to find unused port."); - let port2 = unused_tcp_port().expect("Unable to find unused port."); + let port1 = unused_tcp4_port().expect("Unable to find unused port."); + let port2 = unused_tcp4_port().expect("Unable to find unused port."); CommandLineTest::new() .flag("http-port", Some(port1.to_string().as_str())) .flag("port", Some(port2.to_string().as_str())) @@ -1215,8 +1469,8 @@ fn metrics_address_ipv6_flag() { } #[test] fn metrics_port_flag() { - let port1 = unused_tcp_port().expect("Unable to find unused port."); - let port2 = unused_tcp_port().expect("Unable to find unused port."); + let port1 = unused_tcp4_port().expect("Unable to find unused port."); + let port2 = unused_tcp4_port().expect("Unable to find unused port."); CommandLineTest::new() .flag("metrics", None) .flag("metrics-port", Some(port1.to_string().as_str())) diff --git a/lighthouse/tests/boot_node.rs b/lighthouse/tests/boot_node.rs index 8c000bbb3..4dd5ad95d 100644 --- a/lighthouse/tests/boot_node.rs +++ b/lighthouse/tests/boot_node.rs @@ -12,7 +12,7 @@ use std::path::{Path, PathBuf}; use std::process::Command; use std::str::FromStr; use tempfile::TempDir; -use unused_port::unused_udp_port; +use unused_port::unused_udp4_port; const IP_ADDRESS: &str = "192.168.2.108"; @@ -62,7 +62,7 @@ fn enr_address_arg() { #[test] fn port_flag() { - let port = unused_udp_port().unwrap(); + let port = unused_udp4_port().unwrap(); CommandLineTest::new() .flag("port", Some(port.to_string().as_str())) .run_with_ip() @@ -122,7 +122,7 @@ fn boot_nodes_flag() { #[test] fn enr_port_flag() { - let port = unused_udp_port().unwrap(); + let port = unused_udp4_port().unwrap(); CommandLineTest::new() .flag("enr-port", Some(port.to_string().as_str())) .run_with_ip() diff --git a/testing/eth1_test_rig/src/ganache.rs b/testing/eth1_test_rig/src/ganache.rs index d8df3fd8a..898a089ba 100644 --- a/testing/eth1_test_rig/src/ganache.rs +++ b/testing/eth1_test_rig/src/ganache.rs @@ -3,7 +3,7 @@ use std::io::prelude::*; use std::io::BufReader; use std::process::{Child, Command, Stdio}; use std::time::{Duration, Instant}; -use unused_port::unused_tcp_port; +use unused_port::unused_tcp4_port; use web3::{transports::Http, Transport, Web3}; /// How long we will wait for ganache to indicate that it is ready. @@ -65,7 +65,7 @@ impl GanacheInstance { /// Start a new `ganache` process, waiting until it indicates that it is ready to accept /// RPC connections. pub fn new(chain_id: u64) -> Result { - let port = unused_tcp_port()?; + let port = unused_tcp4_port()?; let binary = match cfg!(windows) { true => "ganache.cmd", false => "ganache", @@ -97,7 +97,7 @@ impl GanacheInstance { } pub fn fork(&self) -> Result { - let port = unused_tcp_port()?; + let port = unused_tcp4_port()?; let binary = match cfg!(windows) { true => "ganache.cmd", false => "ganache", diff --git a/testing/execution_engine_integration/src/execution_engine.rs b/testing/execution_engine_integration/src/execution_engine.rs index ad5af5315..61a50b040 100644 --- a/testing/execution_engine_integration/src/execution_engine.rs +++ b/testing/execution_engine_integration/src/execution_engine.rs @@ -4,7 +4,7 @@ use sensitive_url::SensitiveUrl; use std::path::PathBuf; use std::process::Child; use tempfile::TempDir; -use unused_port::unused_tcp_port; +use unused_port::unused_tcp4_port; pub const KEYSTORE_PASSWORD: &str = "testpwd"; pub const ACCOUNT1: &str = "7b8C3a386C0eea54693fFB0DA17373ffC9228139"; @@ -50,8 +50,8 @@ impl ExecutionEngine { pub fn new(engine: E) -> Self { let datadir = E::init_datadir(); let jwt_secret_path = datadir.path().join(DEFAULT_JWT_FILE); - let http_port = unused_tcp_port().unwrap(); - let http_auth_port = unused_tcp_port().unwrap(); + let http_port = unused_tcp4_port().unwrap(); + let http_auth_port = unused_tcp4_port().unwrap(); let child = E::start_client(&datadir, http_port, http_auth_port, jwt_secret_path); let provider = Provider::::try_from(format!("http://localhost:{}", http_port)) .expect("failed to instantiate ethers provider"); diff --git a/testing/execution_engine_integration/src/geth.rs b/testing/execution_engine_integration/src/geth.rs index 1b96fa9f3..5c83a97e2 100644 --- a/testing/execution_engine_integration/src/geth.rs +++ b/testing/execution_engine_integration/src/geth.rs @@ -5,7 +5,7 @@ use std::path::{Path, PathBuf}; use std::process::{Child, Command, Output}; use std::{env, fs::File}; use tempfile::TempDir; -use unused_port::unused_tcp_port; +use unused_port::unused_tcp4_port; const GETH_BRANCH: &str = "master"; const GETH_REPO_URL: &str = "https://github.com/ethereum/go-ethereum"; @@ -83,7 +83,7 @@ impl GenericExecutionEngine for GethEngine { http_auth_port: u16, jwt_secret_path: PathBuf, ) -> Child { - let network_port = unused_tcp_port().unwrap(); + let network_port = unused_tcp4_port().unwrap(); Command::new(Self::binary_path()) .arg("--datadir") diff --git a/testing/execution_engine_integration/src/nethermind.rs b/testing/execution_engine_integration/src/nethermind.rs index 740d87ab8..720a4a73b 100644 --- a/testing/execution_engine_integration/src/nethermind.rs +++ b/testing/execution_engine_integration/src/nethermind.rs @@ -6,7 +6,7 @@ use std::fs::File; use std::path::{Path, PathBuf}; use std::process::{Child, Command, Output}; use tempfile::TempDir; -use unused_port::unused_tcp_port; +use unused_port::unused_tcp4_port; /// We've pinned the Nethermind version since our method of using the `master` branch to /// find the latest tag isn't working. It appears Nethermind don't always tag on `master`. @@ -88,7 +88,7 @@ impl GenericExecutionEngine for NethermindEngine { http_auth_port: u16, jwt_secret_path: PathBuf, ) -> Child { - let network_port = unused_tcp_port().unwrap(); + let network_port = unused_tcp4_port().unwrap(); let genesis_json_path = datadir.path().join("genesis.json"); Command::new(Self::binary_path()) diff --git a/testing/node_test_rig/src/lib.rs b/testing/node_test_rig/src/lib.rs index 82a60cda2..d4fd115be 100644 --- a/testing/node_test_rig/src/lib.rs +++ b/testing/node_test_rig/src/lib.rs @@ -89,8 +89,9 @@ pub fn testing_client_config() -> ClientConfig { let mut client_config = ClientConfig::default(); // Setting ports to `0` means that the OS will choose some available port. - client_config.network.libp2p_port = 0; - client_config.network.discovery_port = 0; + client_config + .network + .set_ipv4_listening_address(std::net::Ipv4Addr::UNSPECIFIED, 0, 0); client_config.network.upnp_enabled = false; client_config.http_api.enabled = true; client_config.http_api.listen_port = 0; diff --git a/testing/simulator/src/eth1_sim.rs b/testing/simulator/src/eth1_sim.rs index 42aefea7a..43e8a5cf4 100644 --- a/testing/simulator/src/eth1_sim.rs +++ b/testing/simulator/src/eth1_sim.rs @@ -13,7 +13,7 @@ use node_test_rig::{ use rayon::prelude::*; use sensitive_url::SensitiveUrl; use std::cmp::max; -use std::net::{IpAddr, Ipv4Addr}; +use std::net::Ipv4Addr; use std::time::Duration; use tokio::time::sleep; use types::{Epoch, EthSpec, MinimalEthSpec}; @@ -149,7 +149,7 @@ pub fn run_eth1_sim(matches: &ArgMatches) -> Result<(), String> { beacon_config.eth1.chain_id = Eth1Id::from(chain_id); beacon_config.network.target_peers = node_count - 1; - beacon_config.network.enr_address = Some(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1))); + beacon_config.network.enr_address = (Some(Ipv4Addr::LOCALHOST), None); if post_merge_sim { let el_config = execution_layer::Config { diff --git a/testing/simulator/src/local_network.rs b/testing/simulator/src/local_network.rs index 8df912ed1..3e481df88 100644 --- a/testing/simulator/src/local_network.rs +++ b/testing/simulator/src/local_network.rs @@ -58,10 +58,13 @@ impl LocalNetwork { context: RuntimeContext, mut beacon_config: ClientConfig, ) -> Result { - beacon_config.network.discovery_port = BOOTNODE_PORT; - beacon_config.network.libp2p_port = BOOTNODE_PORT; - beacon_config.network.enr_udp_port = Some(BOOTNODE_PORT); - beacon_config.network.enr_tcp_port = Some(BOOTNODE_PORT); + beacon_config.network.set_ipv4_listening_address( + std::net::Ipv4Addr::UNSPECIFIED, + BOOTNODE_PORT, + BOOTNODE_PORT, + ); + beacon_config.network.enr_udp4_port = Some(BOOTNODE_PORT); + beacon_config.network.enr_tcp4_port = Some(BOOTNODE_PORT); beacon_config.network.discv5_config.table_filter = |_| true; let execution_node = if let Some(el_config) = &mut beacon_config.execution_layer { @@ -132,10 +135,13 @@ impl LocalNetwork { .enr() .expect("bootnode must have a network"), ); - beacon_config.network.discovery_port = BOOTNODE_PORT + count; - beacon_config.network.libp2p_port = BOOTNODE_PORT + count; - beacon_config.network.enr_udp_port = Some(BOOTNODE_PORT + count); - beacon_config.network.enr_tcp_port = Some(BOOTNODE_PORT + count); + beacon_config.network.set_ipv4_listening_address( + std::net::Ipv4Addr::UNSPECIFIED, + BOOTNODE_PORT + count, + BOOTNODE_PORT + count, + ); + beacon_config.network.enr_udp4_port = Some(BOOTNODE_PORT + count); + beacon_config.network.enr_tcp4_port = Some(BOOTNODE_PORT + count); beacon_config.network.discv5_config.table_filter = |_| true; } if let Some(el_config) = &mut beacon_config.execution_layer { diff --git a/testing/simulator/src/no_eth1_sim.rs b/testing/simulator/src/no_eth1_sim.rs index 1a026ded4..f1f6dc442 100644 --- a/testing/simulator/src/no_eth1_sim.rs +++ b/testing/simulator/src/no_eth1_sim.rs @@ -7,7 +7,7 @@ use node_test_rig::{ }; use rayon::prelude::*; use std::cmp::max; -use std::net::{IpAddr, Ipv4Addr}; +use std::net::Ipv4Addr; use std::time::{Duration, SystemTime, UNIX_EPOCH}; use tokio::time::sleep; use types::{Epoch, EthSpec, MainnetEthSpec}; @@ -91,7 +91,7 @@ pub fn run_no_eth1_sim(matches: &ArgMatches) -> Result<(), String> { beacon_config.dummy_eth1_backend = true; beacon_config.sync_eth1_chain = true; - beacon_config.network.enr_address = Some(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1))); + beacon_config.network.enr_address = (Some(Ipv4Addr::LOCALHOST), None); let main_future = async { let network = LocalNetwork::new(context.clone(), beacon_config.clone()).await?; diff --git a/testing/simulator/src/sync_sim.rs b/testing/simulator/src/sync_sim.rs index 9d759715e..c437457c2 100644 --- a/testing/simulator/src/sync_sim.rs +++ b/testing/simulator/src/sync_sim.rs @@ -8,7 +8,7 @@ use node_test_rig::{ }; use node_test_rig::{testing_validator_config, ClientConfig}; use std::cmp::max; -use std::net::{IpAddr, Ipv4Addr}; +use std::net::Ipv4Addr; use std::time::{Duration, SystemTime, UNIX_EPOCH}; use types::{Epoch, EthSpec}; @@ -95,7 +95,7 @@ fn syncing_sim( beacon_config.http_api.allow_sync_stalled = true; - beacon_config.network.enr_address = Some(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1))); + beacon_config.network.enr_address = (Some(Ipv4Addr::LOCALHOST), None); // Generate the directories and keystores required for the validator clients. let validator_indices = (0..num_validators).collect::>(); From 36e163c042a06a697c18a620acfbf322779cd456 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Tue, 14 Mar 2023 06:26:37 +0000 Subject: [PATCH 6/6] Add parent_block_number to payload SSE (#4053) ## Issue Addressed In #4027 I forgot to add the `parent_block_number` to the payload attributes SSE. ## Proposed Changes Compute the parent block number while computing the pre-payload attributes. Pass it on to the SSE stream. ## Additional Info Not essential for v3.5.1 as I suspect most builders don't need the `parent_block_root`. I would like to use it for my dummy no-op builder however. --- beacon_node/beacon_chain/src/beacon_chain.rs | 19 ++++++++++++++----- .../beacon_chain/src/canonical_head.rs | 11 +++++++++++ beacon_node/execution_layer/src/lib.rs | 2 +- common/eth2/src/types.rs | 3 +++ 4 files changed, 29 insertions(+), 6 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 9ba8f24cb..4c7f314d8 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -197,6 +197,9 @@ pub enum ProduceBlockVerification { pub struct PrePayloadAttributes { pub proposer_index: u64, pub prev_randao: Hash256, + /// The parent block number is not part of the payload attributes sent to the EL, but *is* + /// sent to builders via SSE. + pub parent_block_number: u64, } /// Define whether a forkchoiceUpdate needs to be checked for an override (`Yes`) or has already @@ -3866,16 +3869,21 @@ impl BeaconChain { proposer as u64 }; - // Get the `prev_randao` value. - let prev_randao = if proposer_head == parent_block_root { - cached_head.parent_random() + // Get the `prev_randao` and parent block number. + let head_block_number = cached_head.head_block_number()?; + let (prev_randao, parent_block_number) = if proposer_head == parent_block_root { + ( + cached_head.parent_random()?, + head_block_number.saturating_sub(1), + ) } else { - cached_head.head_random() - }?; + (cached_head.head_random()?, head_block_number) + }; Ok(Some(PrePayloadAttributes { proposer_index, prev_randao, + parent_block_number, })) } @@ -4865,6 +4873,7 @@ impl BeaconChain { proposal_slot: prepare_slot, proposer_index: proposer, parent_block_root: head_root, + parent_block_number: pre_payload_attributes.parent_block_number, parent_block_hash: forkchoice_update_params.head_hash.unwrap_or_default(), payload_attributes: payload_attributes.into(), }, diff --git a/beacon_node/beacon_chain/src/canonical_head.rs b/beacon_node/beacon_chain/src/canonical_head.rs index 19eddf602..24c06680d 100644 --- a/beacon_node/beacon_chain/src/canonical_head.rs +++ b/beacon_node/beacon_chain/src/canonical_head.rs @@ -167,6 +167,17 @@ impl CachedHead { .map(|payload| payload.prev_randao()) } + /// Returns the execution block number of the block at the head of the chain. + /// + /// Returns an error if the chain is prior to Bellatrix. + pub fn head_block_number(&self) -> Result { + self.snapshot + .beacon_block + .message() + .execution_payload() + .map(|payload| payload.block_number()) + } + /// Returns the active validator count for the current epoch of the head state. /// /// Should only return `None` if the caches have not been built on the head state (this should diff --git a/beacon_node/execution_layer/src/lib.rs b/beacon_node/execution_layer/src/lib.rs index b7aa4dc05..60bc6278a 100644 --- a/beacon_node/execution_layer/src/lib.rs +++ b/beacon_node/execution_layer/src/lib.rs @@ -47,7 +47,7 @@ use types::{ mod block_hash; mod engine_api; -mod engines; +pub mod engines; mod keccak; mod metrics; pub mod payload_cache; diff --git a/common/eth2/src/types.rs b/common/eth2/src/types.rs index b4218c361..175c7db78 100644 --- a/common/eth2/src/types.rs +++ b/common/eth2/src/types.rs @@ -921,6 +921,8 @@ pub struct SseExtendedPayloadAttributesGeneric { #[serde(with = "eth2_serde_utils::quoted_u64")] pub proposer_index: u64, pub parent_block_root: Hash256, + #[serde(with = "eth2_serde_utils::quoted_u64")] + pub parent_block_number: u64, pub parent_block_hash: ExecutionBlockHash, pub payload_attributes: T, } @@ -958,6 +960,7 @@ impl ForkVersionDeserialize for SseExtendedPayloadAttributes { proposal_slot: helper.proposal_slot, proposer_index: helper.proposer_index, parent_block_root: helper.parent_block_root, + parent_block_number: helper.parent_block_number, parent_block_hash: helper.parent_block_hash, payload_attributes: SsePayloadAttributes::deserialize_by_fork::( helper.payload_attributes,