From 00af017582b2b53ca2f18aace93559c4d41d8453 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Mon, 15 Jan 2024 11:19:47 +1100 Subject: [PATCH 01/25] Fix asset paths in release CI upload job (#5056) --- .github/workflows/release.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 26ef781b68..30ac52b6ef 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -282,6 +282,6 @@ jobs: | | Docker | [${{ env.VERSION }}](https://hub.docker.com/r/${{ env.IMAGE_NAME }}/tags?page=1&ordering=last_updated&name=${{ env.VERSION }}) | [${{ env.IMAGE_NAME }}](https://hub.docker.com/r/${{ env.IMAGE_NAME }}) | ENDBODY ) - assets=(./lighthouse-*.tar.gz*) + assets=(./lighthouse-*.tar.gz*/lighthouse-*.tar.gz*) tag_name="${{ env.VERSION }}" echo "$body" | gh release create --draft -F "-" "$tag_name" "${assets[@]}" From 72bcf47dd004432b9af58133035ea0a96beefafc Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Mon, 15 Jan 2024 07:23:39 +0200 Subject: [PATCH 02/25] add content-type octet stream helper fn (#5062) --- beacon_node/http_api/src/lib.rs | 33 ++++++++++++----------- beacon_node/http_api/src/produce_block.rs | 9 +++---- beacon_node/http_api/src/version.rs | 9 +++++-- common/eth2/src/lib.rs | 3 +++ 4 files changed, 32 insertions(+), 22 deletions(-) diff --git a/beacon_node/http_api/src/lib.rs b/beacon_node/http_api/src/lib.rs index 1b4f1d35e1..f1d7769e89 100644 --- a/beacon_node/http_api/src/lib.rs +++ b/beacon_node/http_api/src/lib.rs @@ -45,6 +45,7 @@ use eth2::types::{ PublishBlockRequest, ValidatorBalancesRequestBody, ValidatorId, ValidatorStatus, ValidatorsRequestBody, }; +use eth2::{CONTENT_TYPE_HEADER, SSZ_CONTENT_TYPE_HEADER}; use lighthouse_network::{types::SyncState, EnrExt, NetworkGlobals, PeerId, PubsubMessage}; use lighthouse_version::version_with_platform; use logging::SSELoggingComponents; @@ -86,10 +87,12 @@ use types::{ }; use validator::pubkey_to_validator_index; use version::{ - add_consensus_version_header, execution_optimistic_finalized_fork_versioned_response, - inconsistent_fork_rejection, unsupported_version_rejection, V1, V2, V3, + add_consensus_version_header, add_ssz_content_type_header, + execution_optimistic_finalized_fork_versioned_response, inconsistent_fork_rejection, + unsupported_version_rejection, V1, V2, V3, }; use warp::http::StatusCode; +use warp::hyper::Body; use warp::sse::Event; use warp::Reply; use warp::{http::Response, Filter}; @@ -1567,8 +1570,8 @@ pub fn serve( match accept_header { Some(api_types::Accept::Ssz) => Response::builder() .status(200) - .header("Content-Type", "application/octet-stream") .body(block.as_ssz_bytes().into()) + .map(|res: Response| add_ssz_content_type_header(res)) .map_err(|e| { warp_utils::reject::custom_server_error(format!( "failed to create response: {}", @@ -1659,8 +1662,8 @@ pub fn serve( match accept_header { Some(api_types::Accept::Ssz) => Response::builder() .status(200) - .header("Content-Type", "application/octet-stream") .body(block.as_ssz_bytes().into()) + .map(|res: Response| add_ssz_content_type_header(res)) .map_err(|e| { warp_utils::reject::custom_server_error(format!( "failed to create response: {}", @@ -1710,8 +1713,8 @@ pub fn serve( match accept_header { Some(api_types::Accept::Ssz) => Response::builder() .status(200) - .header("Content-Type", "application/octet-stream") .body(blob_sidecar_list_filtered.as_ssz_bytes().into()) + .map(|res: Response| add_ssz_content_type_header(res)) .map_err(|e| { warp_utils::reject::custom_server_error(format!( "failed to create response: {}", @@ -2239,8 +2242,8 @@ pub fn serve( .map(|snapshot| { Response::builder() .status(200) - .header("Content-Type", "application/octet-stream") .body(snapshot.as_ssz_bytes().into()) + .map(|res: Response| add_ssz_content_type_header(res)) .map_err(|e| { warp_utils::reject::custom_server_error(format!( "failed to create response: {}", @@ -2251,8 +2254,8 @@ pub fn serve( .unwrap_or_else(|| { Response::builder() .status(503) - .header("Content-Type", "application/octet-stream") .body(Vec::new().into()) + .map(|res: Response| add_ssz_content_type_header(res)) .map_err(|e| { warp_utils::reject::custom_server_error(format!( "failed to create response: {}", @@ -2323,8 +2326,8 @@ pub fn serve( match accept_header { Some(api_types::Accept::Ssz) => Response::builder() .status(200) - .header("Content-Type", "application/octet-stream") .body(withdrawals.as_ssz_bytes().into()) + .map(|res: Response| add_ssz_content_type_header(res)) .map_err(|e| { warp_utils::reject::custom_server_error(format!( "failed to create response: {}", @@ -2389,8 +2392,8 @@ pub fn serve( match accept_header { Some(api_types::Accept::Ssz) => Response::builder() .status(200) - .header("Content-Type", "application/octet-stream") .body(bootstrap.as_ssz_bytes().into()) + .map(|res: Response| add_ssz_content_type_header(res)) .map_err(|e| { warp_utils::reject::custom_server_error(format!( "failed to create response: {}", @@ -2437,8 +2440,8 @@ pub fn serve( match accept_header { Some(api_types::Accept::Ssz) => Response::builder() .status(200) - .header("Content-Type", "application/octet-stream") .body(update.as_ssz_bytes().into()) + .map(|res: Response| add_ssz_content_type_header(res)) .map_err(|e| { warp_utils::reject::custom_server_error(format!( "failed to create response: {}", @@ -2485,8 +2488,8 @@ pub fn serve( match accept_header { Some(api_types::Accept::Ssz) => Response::builder() .status(200) - .header("Content-Type", "application/octet-stream") .body(update.as_ssz_bytes().into()) + .map(|res: Response| add_ssz_content_type_header(res)) .map_err(|e| { warp_utils::reject::custom_server_error(format!( "failed to create response: {}", @@ -2687,8 +2690,8 @@ pub fn serve( .map_err(inconsistent_fork_rejection)?; Response::builder() .status(200) - .header("Content-Type", "application/octet-stream") .body(state.as_ssz_bytes().into()) + .map(|res: Response| add_ssz_content_type_header(res)) .map(|resp: warp::reply::Response| { add_consensus_version_header(resp, fork_name) }) @@ -4273,8 +4276,8 @@ pub fn serve( let (state, _execution_optimistic, _finalized) = state_id.state(&chain)?; Response::builder() .status(200) - .header("Content-Type", "application/ssz") - .body(state.as_ssz_bytes()) + .body(state.as_ssz_bytes().into()) + .map(|res: Response| add_ssz_content_type_header(res)) .map_err(|e| { warp_utils::reject::custom_server_error(format!( "failed to create response: {}", @@ -4646,7 +4649,7 @@ pub fn serve( .boxed() .uor( warp::post().and( - warp::header::exact("Content-Type", "application/octet-stream") + warp::header::exact(CONTENT_TYPE_HEADER, SSZ_CONTENT_TYPE_HEADER) // Routes which expect `application/octet-stream` go within this `and`. .and( post_beacon_blocks_ssz diff --git a/beacon_node/http_api/src/produce_block.rs b/beacon_node/http_api/src/produce_block.rs index 6b8a1bb1c3..0da3bdc7aa 100644 --- a/beacon_node/http_api/src/produce_block.rs +++ b/beacon_node/http_api/src/produce_block.rs @@ -3,13 +3,12 @@ use crate::{ version::{ add_consensus_block_value_header, add_consensus_version_header, add_execution_payload_blinded_header, add_execution_payload_value_header, - fork_versioned_response, inconsistent_fork_rejection, + add_ssz_content_type_header, fork_versioned_response, inconsistent_fork_rejection, }, }; use beacon_chain::{ BeaconBlockResponseWrapper, BeaconChain, BeaconChainTypes, ProduceBlockVerification, }; -use bytes::Bytes; use eth2::types::{ self as api_types, EndpointVersion, ProduceBlockV3Metadata, SkipRandaoVerification, }; @@ -95,8 +94,8 @@ pub fn build_response_v3( match accept_header { Some(api_types::Accept::Ssz) => Response::builder() .status(200) - .header("Content-Type", "application/ssz") .body(block_contents.as_ssz_bytes().into()) + .map(|res: Response| add_ssz_content_type_header(res)) .map(|res: Response| add_consensus_version_header(res, fork_name)) .map(|res| add_execution_payload_blinded_header(res, execution_payload_blinded)) .map(|res: Response| { @@ -196,9 +195,9 @@ pub fn build_response_v2( match accept_header { Some(api_types::Accept::Ssz) => Response::builder() .status(200) - .header("Content-Type", "application/octet-stream") .body(block_contents.as_ssz_bytes().into()) - .map(|res: Response| add_consensus_version_header(res, fork_name)) + .map(|res: Response| add_ssz_content_type_header(res)) + .map(|res: Response| add_consensus_version_header(res, fork_name)) .map_err(|e| { warp_utils::reject::custom_server_error(format!("failed to create response: {}", e)) }), diff --git a/beacon_node/http_api/src/version.rs b/beacon_node/http_api/src/version.rs index 7cd5e6700a..59816cb897 100644 --- a/beacon_node/http_api/src/version.rs +++ b/beacon_node/http_api/src/version.rs @@ -1,7 +1,7 @@ use crate::api_types::EndpointVersion; use eth2::{ - CONSENSUS_BLOCK_VALUE_HEADER, CONSENSUS_VERSION_HEADER, EXECUTION_PAYLOAD_BLINDED_HEADER, - EXECUTION_PAYLOAD_VALUE_HEADER, + CONSENSUS_BLOCK_VALUE_HEADER, CONSENSUS_VERSION_HEADER, CONTENT_TYPE_HEADER, + EXECUTION_PAYLOAD_BLINDED_HEADER, EXECUTION_PAYLOAD_VALUE_HEADER, SSZ_CONTENT_TYPE_HEADER, }; use serde::Serialize; use types::{ @@ -59,6 +59,11 @@ pub fn execution_optimistic_finalized_fork_versioned_response( }) } +/// Add the 'Content-Type application/octet-stream` header to a response. +pub fn add_ssz_content_type_header(reply: T) -> Response { + reply::with_header(reply, CONTENT_TYPE_HEADER, SSZ_CONTENT_TYPE_HEADER).into_response() +} + /// Add the `Eth-Consensus-Version` header to a response. 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() diff --git a/common/eth2/src/lib.rs b/common/eth2/src/lib.rs index c633a6c6c6..9c8edc4bdb 100644 --- a/common/eth2/src/lib.rs +++ b/common/eth2/src/lib.rs @@ -46,6 +46,9 @@ pub const EXECUTION_PAYLOAD_BLINDED_HEADER: &str = "Eth-Execution-Payload-Blinde pub const EXECUTION_PAYLOAD_VALUE_HEADER: &str = "Eth-Execution-Payload-Value"; pub const CONSENSUS_BLOCK_VALUE_HEADER: &str = "Eth-Consensus-Block-Value"; +pub const CONTENT_TYPE_HEADER: &str = "Content-Type"; +pub const SSZ_CONTENT_TYPE_HEADER: &str = "application/octet-stream"; + #[derive(Debug)] pub enum Error { /// The `reqwest` client raised an error. From 31b797ed83395bff203786e42fdff2d954b3548e Mon Sep 17 00:00:00 2001 From: Enrico Del Fante Date: Tue, 16 Jan 2024 02:18:06 +0100 Subject: [PATCH 03/25] Update teku's bootnodes (#5052) --- .../built_in_network_configs/mainnet/boot_enr.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/eth2_network_config/built_in_network_configs/mainnet/boot_enr.yaml b/common/eth2_network_config/built_in_network_configs/mainnet/boot_enr.yaml index 3c6e1bad8a..1ae519387a 100644 --- a/common/eth2_network_config/built_in_network_configs/mainnet/boot_enr.yaml +++ b/common/eth2_network_config/built_in_network_configs/mainnet/boot_enr.yaml @@ -9,7 +9,7 @@ - enr:-Ku4QPn5eVhcoF1opaFEvg1b6JNFD2rqVkHQ8HApOKK61OIcIXD127bKWgAtbwI7pnxx6cDyk_nI88TrZKQaGMZj0q0Bh2F0dG5ldHOIAAAAAAAAAACEZXRoMpC1MD8qAAAAAP__________gmlkgnY0gmlwhDayLMaJc2VjcDI1NmsxoQK2sBOLGcUb4AwuYzFuAVCaNHA-dy24UuEKkeFNgCVCsIN1ZHCCIyg - enr:-Ku4QEWzdnVtXc2Q0ZVigfCGggOVB2Vc1ZCPEc6j21NIFLODSJbvNaef1g4PxhPwl_3kax86YPheFUSLXPRs98vvYsoBh2F0dG5ldHOIAAAAAAAAAACEZXRoMpC1MD8qAAAAAP__________gmlkgnY0gmlwhDZBrP2Jc2VjcDI1NmsxoQM6jr8Rb1ktLEsVcKAPa08wCsKUmvoQ8khiOl_SLozf9IN1ZHCCIyg # Teku team (Consensys) -- enr:-KG4QMOEswP62yzDjSwWS4YEjtTZ5PO6r65CPqYBkgTTkrpaedQ8uEUo1uMALtJIvb2w_WWEVmg5yt1UAuK1ftxUU7QDhGV0aDKQu6TalgMAAAD__________4JpZIJ2NIJpcIQEnfA2iXNlY3AyNTZrMaEDfol8oLr6XJ7FsdAYE7lpJhKMls4G_v6qQOGKJUWGb_uDdGNwgiMog3VkcIIjKA +- enr:-KG4QNTx85fjxABbSq_Rta9wy56nQ1fHK0PewJbGjLm1M4bMGx5-3Qq4ZX2-iFJ0pys_O90sVXNNOxp2E7afBsGsBrgDhGV0aDKQu6TalgMAAAD__________4JpZIJ2NIJpcIQEnfA2iXNlY3AyNTZrMaECGXWQ-rQ2KZKRH1aOW4IlPDBkY4XDphxg9pxKytFCkayDdGNwgiMog3VkcIIjKA - enr:-KG4QF4B5WrlFcRhUU6dZETwY5ZzAXnA0vGC__L1Kdw602nDZwXSTs5RFXFIFUnbQJmhNGVU6OIX7KVrCSTODsz1tK4DhGV0aDKQu6TalgMAAAD__________4JpZIJ2NIJpcIQExNYEiXNlY3AyNTZrMaECQmM9vp7KhaXhI-nqL_R0ovULLCFSFTa9CPPSdb1zPX6DdGNwgiMog3VkcIIjKA # Prysm team (Prysmatic Labs) - enr:-Ku4QImhMc1z8yCiNJ1TyUxdcfNucje3BGwEHzodEZUan8PherEo4sF7pPHPSIB1NNuSg5fZy7qFsjmUKs2ea1Whi0EBh2F0dG5ldHOIAAAAAAAAAACEZXRoMpD1pf1CAAAAAP__________gmlkgnY0gmlwhBLf22SJc2VjcDI1NmsxoQOVphkDqal4QzPMksc5wnpuC3gvSC8AfbFOnZY_On34wIN1ZHCCIyg From 0613eb7a2189cb72f7bb4f706fc53b1edaca35dd Mon Sep 17 00:00:00 2001 From: vuittont60 <81072379+vuittont60@users.noreply.github.com> Date: Tue, 16 Jan 2024 09:18:54 +0800 Subject: [PATCH 04/25] docs: fix typos (#5059) * docs: fix typos * docs: fix typo * docs: update by 'make cli' --- Makefile | 2 +- beacon_node/src/cli.rs | 2 +- beacon_node/src/lib.rs | 2 +- book/src/help_bn.md | 2 +- book/src/redundancy.md | 2 +- book/src/slasher.md | 2 +- book/src/validator-manager-create.md | 2 +- common/malloc_utils/src/jemalloc.rs | 2 +- 8 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Makefile b/Makefile index 564e32843f..8392d00170 100644 --- a/Makefile +++ b/Makefile @@ -14,7 +14,7 @@ BUILD_PATH_AARCH64 = "target/$(AARCH64_TAG)/release" PINNED_NIGHTLY ?= nightly CLIPPY_PINNED_NIGHTLY=nightly-2022-05-19 -# List of features to use when building natively. Can be overriden via the environment. +# List of features to use when building natively. Can be overridden via the environment. # No jemalloc on Windows ifeq ($(OS),Windows_NT) FEATURES?= diff --git a/beacon_node/src/cli.rs b/beacon_node/src/cli.rs index 73ca5c6231..002bb344a3 100644 --- a/beacon_node/src/cli.rs +++ b/beacon_node/src/cli.rs @@ -1164,7 +1164,7 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { .arg( Arg::with_name("disable-deposit-contract-sync") .long("disable-deposit-contract-sync") - .help("Explictly disables syncing of deposit logs from the execution node. \ + .help("Explicitly disables syncing of deposit logs from the execution node. \ This overrides any previous option that depends on it. \ Useful if you intend to run a non-validating beacon node.") .takes_value(false) diff --git a/beacon_node/src/lib.rs b/beacon_node/src/lib.rs index cf6d627c30..ee782c650e 100644 --- a/beacon_node/src/lib.rs +++ b/beacon_node/src/lib.rs @@ -99,7 +99,7 @@ impl ProductionBeaconNode { DatabaseBackendOverride::Success(old_backend) => { info!( log, - "Slasher backend overriden"; + "Slasher backend overridden"; "reason" => "database exists", "configured_backend" => %old_backend, "override_backend" => %slasher_config.backend, diff --git a/book/src/help_bn.md b/book/src/help_bn.md index bb4d7a10c6..dff2ab6876 100644 --- a/book/src/help_bn.md +++ b/book/src/help_bn.md @@ -29,7 +29,7 @@ FLAGS: contention which degrades staking performance. Stakers should generally choose to avoid this flag since backfill sync is not required for staking. - --disable-deposit-contract-sync Explictly disables syncing of deposit logs from the execution node. This + --disable-deposit-contract-sync Explicitly disables syncing of deposit logs from the execution node. This overrides any previous option that depends on it. Useful if you intend to run a non-validating beacon node. --disable-duplicate-warn-logs Disable warning logs for duplicate gossip messages. The WARN level log is diff --git a/book/src/redundancy.md b/book/src/redundancy.md index 8318aea21e..11b9845658 100644 --- a/book/src/redundancy.md +++ b/book/src/redundancy.md @@ -97,7 +97,7 @@ from this list: setup this is the first option we would recommend enabling. - `sync-committee`: Send sync committee signatures & aggregates to all beacon nodes. This can improve propagation of sync committee messages with similar tradeoffs to broadcasting - attestations, although occuring less often due to the infrequency of sync committee duties. + attestations, although occurring less often due to the infrequency of sync committee duties. - `none`: Disable all broadcasting. This option only has an effect when provided alone, otherwise it is ignored. Not recommended except for expert tweakers. diff --git a/book/src/slasher.md b/book/src/slasher.md index 41bc3baf7e..c8506922b4 100644 --- a/book/src/slasher.md +++ b/book/src/slasher.md @@ -71,7 +71,7 @@ If an MDBX database is already found on disk, then Lighthouse will try to use it in a log at start-up: ``` -INFO Slasher backend overriden reason: database exists, configured_backend: lmdb, overriden_backend: mdbx +INFO Slasher backend overridden reason: database exists, configured_backend: lmdb, overridden_backend: mdbx ``` If the running Lighthouse binary doesn't have the MDBX backend enabled but an existing database is diff --git a/book/src/validator-manager-create.md b/book/src/validator-manager-create.md index 0cec150dab..6ba894a43c 100644 --- a/book/src/validator-manager-create.md +++ b/book/src/validator-manager-create.md @@ -139,7 +139,7 @@ In order to import the validators, the location of the VC `api-token.txt` file must be known. The location of the file varies, but it is located in the "validator directory" of your data directory. For example: `~/.lighthouse/mainnet/validators/api-token.txt`. We will use `` -to subsitute this value. If you are unsure of the `api-token.txt` path, you can run `curl http://localhost:5062/lighthouse/auth` which will show the path. +to substitute this value. If you are unsure of the `api-token.txt` path, you can run `curl http://localhost:5062/lighthouse/auth` which will show the path. Once the VC is running, use the `import` command to import the validators to the VC: diff --git a/common/malloc_utils/src/jemalloc.rs b/common/malloc_utils/src/jemalloc.rs index c796ea39a1..92533048c5 100644 --- a/common/malloc_utils/src/jemalloc.rs +++ b/common/malloc_utils/src/jemalloc.rs @@ -3,7 +3,7 @@ //! Due to `jemalloc` requiring configuration at compile time or immediately upon runtime //! initialisation it is configured via a Cargo config file in `.cargo/config.toml`. //! -//! The `jemalloc` tuning can be overriden by: +//! The `jemalloc` tuning can be overridden by: //! //! A) `JEMALLOC_SYS_WITH_MALLOC_CONF` at compile-time. //! B) `_RJEM_MALLOC_CONF` at runtime. From e10e4b781100f673e1f4a8fc6508af74e762f214 Mon Sep 17 00:00:00 2001 From: Akihito Nakano Date: Tue, 16 Jan 2024 10:19:49 +0900 Subject: [PATCH 05/25] Fix zero port error (#5021) * Fix zero port error and add tests * Tweak indentation * assign 0 to quic_port if tcp_port == 0 * Remove unnecessary deps --- beacon_node/src/config.rs | 20 ++++--- lighthouse/tests/beacon_node.rs | 95 +++++++++++++++++++++++++++++++++ 2 files changed, 109 insertions(+), 6 deletions(-) diff --git a/beacon_node/src/config.rs b/beacon_node/src/config.rs index 6cb2c76af2..2f67161ecf 100644 --- a/beacon_node/src/config.rs +++ b/beacon_node/src/config.rs @@ -973,13 +973,13 @@ pub fn parse_listening_addresses( .then(unused_port::unused_udp6_port) .transpose()? .or(maybe_disc_port) - .unwrap_or(port); + .unwrap_or(tcp_port); let quic_port = use_zero_ports .then(unused_port::unused_udp6_port) .transpose()? .or(maybe_quic_port) - .unwrap_or(port + 1); + .unwrap_or(if tcp_port == 0 { 0 } else { tcp_port + 1 }); ListenAddress::V6(lighthouse_network::ListenAddr { addr: ipv6, @@ -1002,14 +1002,14 @@ pub fn parse_listening_addresses( .then(unused_port::unused_udp4_port) .transpose()? .or(maybe_disc_port) - .unwrap_or(port); + .unwrap_or(tcp_port); // use zero ports if required. If not, use the specific quic port. If none given, use // the tcp port + 1. let quic_port = use_zero_ports .then(unused_port::unused_udp4_port) .transpose()? .or(maybe_quic_port) - .unwrap_or(port + 1); + .unwrap_or(if tcp_port == 0 { 0 } else { tcp_port + 1 }); ListenAddress::V4(lighthouse_network::ListenAddr { addr: ipv4, @@ -1032,7 +1032,11 @@ pub fn parse_listening_addresses( .then(unused_port::unused_udp4_port) .transpose()? .or(maybe_quic_port) - .unwrap_or(port + 1); + .unwrap_or(if ipv4_tcp_port == 0 { + 0 + } else { + ipv4_tcp_port + 1 + }); // Defaults to 9090 when required let ipv6_tcp_port = use_zero_ports @@ -1048,7 +1052,11 @@ pub fn parse_listening_addresses( .then(unused_port::unused_udp6_port) .transpose()? .or(maybe_quic6_port) - .unwrap_or(ipv6_tcp_port + 1); + .unwrap_or(if ipv6_tcp_port == 0 { + 0 + } else { + ipv6_tcp_port + 1 + }); ListenAddress::DualStack( lighthouse_network::ListenAddr { diff --git a/lighthouse/tests/beacon_node.rs b/lighthouse/tests/beacon_node.rs index e17018539f..2a88770cdd 100644 --- a/lighthouse/tests/beacon_node.rs +++ b/lighthouse/tests/beacon_node.rs @@ -859,6 +859,23 @@ fn network_port_flag_over_ipv4() { listen_addr.quic_port, listen_addr.tcp_port )), + // quic_port should be 0 if tcp_port is given as 0. + Some((port, 0, port)) + ); + }); + + let port = 9000; + CommandLineTest::new() + .flag("port", Some(port.to_string().as_str())) + .run() + .with_config(|config| { + assert_eq!( + config.network.listen_addrs().v4().map(|listen_addr| ( + listen_addr.disc_port, + listen_addr.quic_port, + listen_addr.tcp_port + )), + // quic_port should be (tcp_port + 1) if tcp_port is given as non-zero. Some((port, port + 1, port)) ); }); @@ -877,11 +894,89 @@ fn network_port_flag_over_ipv6() { listen_addr.quic_port, listen_addr.tcp_port )), + // quic_port should be 0 if tcp_port is given as 0. + Some((port, 0, port)) + ); + }); + + let port = 9000; + CommandLineTest::new() + .flag("listen-address", Some("::1")) + .flag("port", Some(port.to_string().as_str())) + .run() + .with_config(|config| { + assert_eq!( + config.network.listen_addrs().v6().map(|listen_addr| ( + listen_addr.disc_port, + listen_addr.quic_port, + listen_addr.tcp_port + )), + // quic_port should be (tcp_port + 1) if tcp_port is given as non-zero. Some((port, port + 1, port)) ); }); } #[test] +fn network_port_flag_over_ipv4_and_ipv6() { + let port = 0; + let port6 = 0; + CommandLineTest::new() + .flag("listen-address", Some("127.0.0.1")) + .flag("listen-address", Some("::1")) + .flag("port", Some(port.to_string().as_str())) + .flag("port6", Some(port6.to_string().as_str())) + .run() + .with_config(|config| { + assert_eq!( + config.network.listen_addrs().v4().map(|listen_addr| ( + listen_addr.disc_port, + listen_addr.quic_port, + listen_addr.tcp_port + )), + // quic_port should be 0 if tcp_port is given as 0. + Some((port, 0, port)) + ); + assert_eq!( + config.network.listen_addrs().v6().map(|listen_addr| ( + listen_addr.disc_port, + listen_addr.quic_port, + listen_addr.tcp_port + )), + // quic_port should be 0 if tcp_port is given as 0. + Some((port6, 0, port6)) + ); + }); + + let port = 19000; + let port6 = 29000; + CommandLineTest::new() + .flag("listen-address", Some("127.0.0.1")) + .flag("listen-address", Some("::1")) + .flag("port", Some(port.to_string().as_str())) + .flag("port6", Some(port6.to_string().as_str())) + .run() + .with_config(|config| { + assert_eq!( + config.network.listen_addrs().v4().map(|listen_addr| ( + listen_addr.disc_port, + listen_addr.quic_port, + listen_addr.tcp_port + )), + // quic_port should be (tcp_port + 1) if tcp_port is given as non-zero. + Some((port, port + 1, port)) + ); + assert_eq!( + config.network.listen_addrs().v6().map(|listen_addr| ( + listen_addr.disc_port, + listen_addr.quic_port, + listen_addr.tcp_port + )), + // quic_port should be (tcp_port + 1) if tcp_port is given as non-zero. + Some((port6, port6 + 1, port6)) + ); + }); +} +#[test] fn network_port_and_discovery_port_flags_over_ipv4() { let tcp4_port = 0; let disc4_port = 0; From a68b7018071713a7aba699697e45431b453d91ed Mon Sep 17 00:00:00 2001 From: Divma <26765164+divagant-martian@users.noreply.github.com> Date: Mon, 15 Jan 2024 20:22:09 -0500 Subject: [PATCH 06/25] add tracing metrics layer for dependency logging (#4979) * add metrics layer * add metrics * simplify getting the target * make clippy happy * fix typos * unify deps under workspace * make import statement shorter, fix typos * enable warn by default, mark flag as deprecated * do not exit on error when initializing logging fails * revert exit on error * adjust bootnode logging * use target as is by default * make libp2p events register correctly * adjust repilcated cli help * turn on debug logs by default, remove deprecation warning * suppress output (#5) --------- Co-authored-by: Age Manning --- Cargo.lock | 179 +++++++++++++------- Cargo.toml | 3 + book/src/help_general.md | 4 +- boot_node/src/lib.rs | 5 +- common/logging/Cargo.toml | 19 ++- common/logging/src/lib.rs | 3 + common/logging/src/tracing_metrics_layer.rs | 63 +++++++ lighthouse/Cargo.toml | 3 +- lighthouse/src/main.rs | 34 +++- 9 files changed, 234 insertions(+), 79 deletions(-) create mode 100644 common/logging/src/tracing_metrics_layer.rs diff --git a/Cargo.lock b/Cargo.lock index b22b601d6c..532388cf4c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -310,7 +310,7 @@ dependencies = [ "futures-lite", "parking", "polling", - "rustix 0.38.28", + "rustix 0.38.30", "slab", "tracing", "windows-sys 0.52.0", @@ -318,9 +318,9 @@ dependencies = [ [[package]] name = "async-lock" -version = "3.2.0" +version = "3.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7125e42787d53db9dd54261812ef17e937c95a51e4d291373b670342fa44310c" +checksum = "d034b430882f8381900d3fe6f0aaa3ad94f2cb4ac519b429692a1bc2dda4ae7b" dependencies = [ "event-listener 4.0.3", "event-listener-strategy", @@ -416,9 +416,9 @@ checksum = "d468802bab17cbc0cc575e9b053f41e72aa36bfa6b7f55e3529ffa43161b97fa" [[package]] name = "axum" -version = "0.7.3" +version = "0.7.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d09dbe0e490df5da9d69b36dca48a76635288a82f92eca90024883a56202026d" +checksum = "1236b4b292f6c4d6dc34604bb5120d85c3fe1d1aa596bd5cc52ca054d13e7b9e" dependencies = [ "async-trait", "axum-core", @@ -450,9 +450,9 @@ dependencies = [ [[package]] name = "axum-core" -version = "0.4.2" +version = "0.4.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e87c8503f93e6d144ee5690907ba22db7ba79ab001a932ab99034f0fe836b3df" +checksum = "a15c63fd72d41492dc4f497196f5da1fb04fb7529e631d73630d1b491e47a2e3" dependencies = [ "async-trait", "bytes", @@ -510,9 +510,9 @@ checksum = "9e1b586273c5702936fe7b7d6896644d8be71e6314cfe09d3167c95f712589e8" [[package]] name = "base64" -version = "0.21.6" +version = "0.21.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c79fed4cdb43e993fcdadc7e58a09fd0e3e649c4436fa11da71c9f1f3ee7feb9" +checksum = "9d297deb1925b89f2ccc13d7635fa0714f12c87adce1c75356b39ca9b7178567" [[package]] name = "base64ct" @@ -1972,7 +1972,7 @@ version = "0.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2a3d8dc56e02f954cac8eb489772c552c473346fc34f67412bb6244fd647f7e4" dependencies = [ - "base64 0.21.6", + "base64 0.21.7", "bytes", "ed25519-dalek", "hex", @@ -3084,9 +3084,9 @@ dependencies = [ [[package]] name = "h2" -version = "0.3.22" +version = "0.3.23" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4d6250322ef6e60f93f9a2162799302cd6f68f79f6e5d85c8c16f14d1d958178" +checksum = "b553656127a00601c8ae5590fcfdc118e4083a7924b6cf4ffc1ea4b99dc429d7" dependencies = [ "bytes", "fnv", @@ -3181,7 +3181,7 @@ version = "0.3.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "06683b93020a07e3dbcf5f8c0f6d40080d725bea7936fc01ad345c01b97dc270" dependencies = [ - "base64 0.21.6", + "base64 0.21.7", "bytes", "headers-core", "http 0.2.11", @@ -3499,7 +3499,7 @@ dependencies = [ "futures-channel", "futures-core", "futures-util", - "h2 0.3.22", + "h2 0.3.23", "http 0.2.11", "http-body 0.4.6", "httparse", @@ -3691,9 +3691,9 @@ dependencies = [ [[package]] name = "igd-next" -version = "0.14.2" +version = "0.14.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "57e065e90a518ab5fedf79aa1e4b784e10f8e484a834f6bda85c42633a2cb7af" +checksum = "064d90fec10d541084e7b39ead8875a5a80d9114a2b18791565253bae25f49e4" dependencies = [ "async-trait", "attohttpc 0.24.1", @@ -3915,9 +3915,9 @@ dependencies = [ [[package]] name = "js-sys" -version = "0.3.66" +version = "0.3.67" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cee9c64da59eae3b50095c18d3e74f8b73c0b86d2792824ff01bbce68ba229ca" +checksum = "9a1d36f1235bc969acba30b7f5990b864423a6068a10f7c90ae8f0112e3a59d1" dependencies = [ "wasm-bindgen", ] @@ -3928,7 +3928,7 @@ version = "8.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6971da4d9c3aa03c3d8f3ff0f4155b534aad021292003895a469716b2a230378" dependencies = [ - "base64 0.21.6", + "base64 0.21.7", "pem 1.1.1", "ring 0.16.20", "serde", @@ -3965,9 +3965,9 @@ dependencies = [ [[package]] name = "keccak" -version = "0.1.4" +version = "0.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8f6d5ed8676d904364de097082f4e7d240b571b67989ced0240f08b7f966f940" +checksum = "ecc2af9a1119c51f12a14607e783cb977bde58bc069ff0c3da1095e635d70654" dependencies = [ "cpufeatures", ] @@ -4237,7 +4237,7 @@ source = "git+https://github.com/sigp/rust-libp2p/?rev=b96b90894faab0a1eed78e1c8 dependencies = [ "async-channel", "asynchronous-codec", - "base64 0.21.6", + "base64 0.21.7", "byteorder", "bytes", "either", @@ -4615,7 +4615,6 @@ dependencies = [ "clap_utils", "database_manager", "directory", - "env_logger 0.9.3", "environment", "eth1", "eth2", @@ -4626,6 +4625,7 @@ dependencies = [ "lighthouse_metrics", "lighthouse_network", "lighthouse_version", + "logging", "malloc_utils", "sensitive_url", "serde", @@ -4637,6 +4637,7 @@ dependencies = [ "sloggers", "task_executor", "tempfile", + "tracing-subscriber", "types", "unused_port", "validator_client", @@ -4792,6 +4793,9 @@ dependencies = [ "sloggers", "take_mut", "tokio", + "tracing-core", + "tracing-log", + "tracing-subscriber", ] [[package]] @@ -4888,9 +4892,9 @@ dependencies = [ [[package]] name = "mediatype" -version = "0.19.16" +version = "0.19.17" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bf0bc9784973713e4a90d515a4302991ca125a7c4516951cb607f2298cb757e5" +checksum = "83a018c36a54f4e12c30464bbc59311f85d3f6f4d6c1b4fa4ea9db2b174ddefc" [[package]] name = "memchr" @@ -5298,6 +5302,16 @@ dependencies = [ "winapi", ] +[[package]] +name = "nu-ansi-term" +version = "0.46.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "77a8165726e8236064dbb45459242600304b42a5ea24ee2948e18e023bf7ba84" +dependencies = [ + "overload", + "winapi", +] + [[package]] name = "num-bigint" version = "0.4.4" @@ -5521,6 +5535,12 @@ dependencies = [ "types", ] +[[package]] +name = "overload" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b15813163c1d831bf4a13c3610c05c0d03b39feb07f7e09fa234dac9b15aaf39" + [[package]] name = "p256" version = "0.13.2" @@ -5698,7 +5718,7 @@ version = "3.0.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1b8fcc794035347fb64beda2d3b462595dd2753e3f268d89c5aae77e8cf2c310" dependencies = [ - "base64 0.21.6", + "base64 0.21.7", "serde", ] @@ -5845,14 +5865,14 @@ dependencies = [ [[package]] name = "polling" -version = "3.3.1" +version = "3.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cf63fa624ab313c11656b4cda960bfc46c410187ad493c41f6ba2d8c1e991c9e" +checksum = "545c980a3880efd47b2e262f6a4bb6daad6555cf3367aa9c4e52895f69537a41" dependencies = [ "cfg-if", "concurrent-queue", "pin-project-lite", - "rustix 0.38.28", + "rustix 0.38.30", "tracing", "windows-sys 0.52.0", ] @@ -5898,7 +5918,7 @@ version = "0.6.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "49b6c5ef183cd3ab4ba005f1ca64c21e8bd97ce4699cfea9e8d9a2c4958ca520" dependencies = [ - "base64 0.21.6", + "base64 0.21.7", "byteorder", "bytes", "fallible-iterator", @@ -6413,12 +6433,12 @@ version = "0.11.23" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "37b1ae8d9ac08420c66222fb9096fc5de435c3c48542bc5336c51892cffafb41" dependencies = [ - "base64 0.21.6", + "base64 0.21.7", "bytes", "encoding_rs", "futures-core", "futures-util", - "h2 0.3.22", + "h2 0.3.23", "http 0.2.11", "http-body 0.4.6", "hyper 0.14.28", @@ -6630,9 +6650,9 @@ dependencies = [ [[package]] name = "rustix" -version = "0.38.28" +version = "0.38.30" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "72e572a5e8ca657d7366229cdde4bd14c4eb5499a9573d4d366fe1b599daa316" +checksum = "322394588aaf33c24007e8bb3238ee3e4c5c09c084ab32bc73890b99ff326bca" dependencies = [ "bitflags 2.4.1", "errno", @@ -6673,7 +6693,7 @@ version = "1.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1c74cae0a4cf6ccbbf5f359f08efdf8ee7e1dc532573bf0db71968cb56b1448c" dependencies = [ - "base64 0.21.6", + "base64 0.21.7", ] [[package]] @@ -6682,7 +6702,7 @@ version = "2.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "35e4980fa29e4c4b212ffb3db068a564cbf560e51d3944b7c88bd8bf5bec64f4" dependencies = [ - "base64 0.21.6", + "base64 0.21.7", "rustls-pki-types", ] @@ -7092,6 +7112,15 @@ dependencies = [ "keccak", ] +[[package]] +name = "sharded-slab" +version = "0.1.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f40ca3c46823713e0d4209592e8d6e826aa57e928f09752619fc696c499637f6" +dependencies = [ + "lazy_static", +] + [[package]] name = "shlex" version = "1.2.0" @@ -7353,9 +7382,9 @@ dependencies = [ [[package]] name = "smallvec" -version = "1.11.2" +version = "1.12.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4dccd0940a2dcdf68d092b8cbab7dc0ad8fa938bf95787e1b916b0e3d0e8e970" +checksum = "2593d31f82ead8df961d8bd23a64c2ccf2eb5dd34b0a34bfb4dd54011c72009e" [[package]] name = "snap" @@ -7728,7 +7757,7 @@ dependencies = [ "cfg-if", "fastrand", "redox_syscall 0.4.1", - "rustix 0.38.28", + "rustix 0.38.30", "windows-sys 0.52.0", ] @@ -7745,9 +7774,9 @@ dependencies = [ [[package]] name = "termcolor" -version = "1.4.0" +version = "1.4.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ff1bc3d3f05aff0403e8ac0d92ced918ec05b666a43f83297ccef5bea8a3d449" +checksum = "06794f8f6c5c898b3275aebefa6b8a1cb24cd2c6c79397ab15774837a0bc5755" dependencies = [ "winapi-util", ] @@ -8179,6 +8208,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c06d3da6113f116aaee68e4d601191614c9053067f9ab7f6edbcb161237daa54" dependencies = [ "once_cell", + "valuable", ] [[package]] @@ -8191,6 +8221,31 @@ dependencies = [ "tracing", ] +[[package]] +name = "tracing-log" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ee855f1f400bd0e5c02d150ae5de3840039a3f54b025156404e34c23c03f47c3" +dependencies = [ + "log", + "once_cell", + "tracing-core", +] + +[[package]] +name = "tracing-subscriber" +version = "0.3.18" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ad0f048c97dbd9faa9b7df56362b8ebcaa52adb06b498c050d2f4e32f90a7a8b" +dependencies = [ + "nu-ansi-term", + "sharded-slab", + "smallvec", + "thread_local", + "tracing-core", + "tracing-log", +] + [[package]] name = "trackable" version = "1.3.0" @@ -8560,6 +8615,12 @@ dependencies = [ "validator_client", ] +[[package]] +name = "valuable" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "830b7e5d4d90034032940e4ace0d9a9a057e7a45cd94e6c007832e39edb82f6d" + [[package]] name = "vcpkg" version = "0.2.15" @@ -8606,7 +8667,7 @@ dependencies = [ [[package]] name = "warp" version = "0.3.6" -source = "git+https://github.com/seanmonstar/warp.git#2c3581e8387e29bab2ac1aa5f9ae9602fe62339f" +source = "git+https://github.com/seanmonstar/warp.git#724e767173132de7c435b323e12d6bec68038de2" dependencies = [ "bytes", "futures-channel", @@ -8657,9 +8718,9 @@ checksum = "9c8d87e72b64a3b4db28d11ce29237c246188f4f51057d65a7eab63b7987e423" [[package]] name = "wasm-bindgen" -version = "0.2.89" +version = "0.2.90" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0ed0d4f68a3015cc185aff4db9506a015f4b96f95303897bfa23f846db54064e" +checksum = "b1223296a201415c7fad14792dbefaace9bd52b62d33453ade1c5b5f07555406" dependencies = [ "cfg-if", "wasm-bindgen-macro", @@ -8667,9 +8728,9 @@ dependencies = [ [[package]] name = "wasm-bindgen-backend" -version = "0.2.89" +version = "0.2.90" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1b56f625e64f3a1084ded111c4d5f477df9f8c92df113852fa5a374dbda78826" +checksum = "fcdc935b63408d58a32f8cc9738a0bffd8f05cc7c002086c6ef20b7312ad9dcd" dependencies = [ "bumpalo", "log", @@ -8682,9 +8743,9 @@ dependencies = [ [[package]] name = "wasm-bindgen-futures" -version = "0.4.39" +version = "0.4.40" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ac36a15a220124ac510204aec1c3e5db8a22ab06fd6706d881dc6149f8ed9a12" +checksum = "bde2032aeb86bdfaecc8b261eef3cba735cc426c1f3a3416d1e0791be95fc461" dependencies = [ "cfg-if", "js-sys", @@ -8694,9 +8755,9 @@ dependencies = [ [[package]] name = "wasm-bindgen-macro" -version = "0.2.89" +version = "0.2.90" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0162dbf37223cd2afce98f3d0785506dcb8d266223983e4b5b525859e6e182b2" +checksum = "3e4c238561b2d428924c49815533a8b9121c664599558a5d9ec51f8a1740a999" dependencies = [ "quote", "wasm-bindgen-macro-support", @@ -8704,9 +8765,9 @@ dependencies = [ [[package]] name = "wasm-bindgen-macro-support" -version = "0.2.89" +version = "0.2.90" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f0eb82fcb7930ae6219a7ecfd55b217f5f0893484b7a13022ebb2b2bf20b5283" +checksum = "bae1abb6806dc1ad9e560ed242107c0f6c84335f1749dd4e8ddb012ebd5e25a7" dependencies = [ "proc-macro2", "quote", @@ -8717,9 +8778,9 @@ dependencies = [ [[package]] name = "wasm-bindgen-shared" -version = "0.2.89" +version = "0.2.90" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7ab9b36309365056cd639da3134bf87fa8f3d86008abf99e612384a6eecd459f" +checksum = "4d91413b1c31d7539ba5ef2451af3f0b833a005eb27a631cec32bc0635a8602b" [[package]] name = "wasm-streams" @@ -8786,9 +8847,9 @@ dependencies = [ [[package]] name = "web-sys" -version = "0.3.66" +version = "0.3.67" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "50c24a44ec86bb68fbecd1b3efed7e85ea5621b39b35ef2766b66cd984f8010f" +checksum = "58cd2333b6e0be7a39605f0e255892fd7418a682d8da8fe042fe25128794d2ed" dependencies = [ "js-sys", "wasm-bindgen", @@ -8835,7 +8896,7 @@ dependencies = [ "either", "home", "once_cell", - "rustix 0.38.28", + "rustix 0.38.30", ] [[package]] @@ -9137,9 +9198,9 @@ checksum = "dff9641d1cd4be8d1a070daf9e3773c5f67e78b4d9d42263020c057706765c04" [[package]] name = "winnow" -version = "0.5.33" +version = "0.5.34" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b7520bbdec7211caa7c4e682eb1fbe07abe20cee6756b6e00f537c82c11816aa" +checksum = "b7cf47b659b318dccbd69cc4797a39ae128f533dce7902a1096044d1967b9c16" dependencies = [ "memchr", ] diff --git a/Cargo.toml b/Cargo.toml index e059fa9f67..f43d375f0d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -160,6 +160,9 @@ tempfile = "3" tokio = { version = "1", features = ["rt-multi-thread", "sync", "signal"] } tokio-stream = { version = "0.1", features = ["sync"] } tokio-util = { version = "0.6", features = ["codec", "compat", "time"] } +tracing-core = "0.1" +tracing-log = "0.2" +tracing-subscriber = "0.3" tree_hash = "0.5" tree_hash_derive = "0.5" url = "2" diff --git a/book/src/help_general.md b/book/src/help_general.md index 7169884cdc..fbe05693e7 100644 --- a/book/src/help_general.md +++ b/book/src/help_general.md @@ -13,8 +13,8 @@ FLAGS: --disable-malloc-tuning If present, do not configure the system allocator. Providing this flag will generally increase memory usage, it should only be provided when debugging specific memory allocation issues. - -l Enables environment logging giving access to sub-protocol logs such as discv5 - and libp2p + -l DEPRECATED Enables environment logging giving access to sub-protocol logs such + as discv5 and libp2p -h, --help Prints help information --log-color Force outputting colors when emitting logs to the terminal. --logfile-compress If present, compress old log files. This can help reduce the space needed to diff --git a/boot_node/src/lib.rs b/boot_node/src/lib.rs index d76e7906b2..0421ce2684 100644 --- a/boot_node/src/lib.rs +++ b/boot_node/src/lib.rs @@ -48,11 +48,8 @@ pub fn run( log::Level::Error => drain.filter_level(Level::Error), }; - let logger = Logger::root(drain.fuse(), o!()); - let _scope_guard = slog_scope::set_global_logger(logger); - slog_stdlog::init_with_level(debug_level).unwrap(); + let log = Logger::root(drain.fuse(), o!()); - let log = slog_scope::logger(); // Run the main function emitting any errors if let Err(e) = match eth_spec_id { EthSpecId::Minimal => { diff --git a/common/logging/Cargo.toml b/common/logging/Cargo.toml index 9c5321591b..6975711651 100644 --- a/common/logging/Cargo.toml +++ b/common/logging/Cargo.toml @@ -8,15 +8,18 @@ edition = { workspace = true } test_logger = [] # Print log output to stderr when running tests instead of dropping it [dependencies] -slog = { workspace = true } -slog-term = { workspace = true } -tokio = { workspace = true } -lighthouse_metrics = { workspace = true } +chrono = { version = "0.4", default-features = false, features = ["clock", "std"] } lazy_static = { workspace = true } -sloggers = { workspace = true } -slog-async = { workspace = true } -take_mut = "0.2.2" +lighthouse_metrics = { workspace = true } parking_lot = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } -chrono = { version = "0.4", default-features = false, features = ["clock", "std"] } +slog = { workspace = true } +slog-async = { workspace = true } +slog-term = { workspace = true } +sloggers = { workspace = true } +take_mut = "0.2.2" +tokio = { workspace = true } +tracing-core = { workspace = true } +tracing-log = { workspace = true } +tracing-subscriber = { workspace = true } diff --git a/common/logging/src/lib.rs b/common/logging/src/lib.rs index a9ad25f3f3..80fc00a00f 100644 --- a/common/logging/src/lib.rs +++ b/common/logging/src/lib.rs @@ -13,7 +13,10 @@ pub const MAX_MESSAGE_WIDTH: usize = 40; pub mod async_record; mod sse_logging_components; +mod tracing_metrics_layer; + pub use sse_logging_components::SSELoggingComponents; +pub use tracing_metrics_layer::MetricsLayer; /// The minimum interval between log messages indicating that a queue is full. const LOG_DEBOUNCE_INTERVAL: Duration = Duration::from_secs(30); diff --git a/common/logging/src/tracing_metrics_layer.rs b/common/logging/src/tracing_metrics_layer.rs new file mode 100644 index 0000000000..08c323ee89 --- /dev/null +++ b/common/logging/src/tracing_metrics_layer.rs @@ -0,0 +1,63 @@ +//! Exposes [`MetricsLayer`]: A tracing layer that registers metrics of logging events. + +use lighthouse_metrics as metrics; +use tracing_log::NormalizeEvent; + +lazy_static! { + /// Count of `INFO` logs registered per enabled dependency. + pub static ref DEP_INFOS_TOTAL: metrics::Result = + metrics::try_create_int_counter_vec( + "dep_info_total", + "Count of infos logged per enabled dependency", + &["target"] + ); + /// Count of `WARN` logs registered per enabled dependency. + pub static ref DEP_WARNS_TOTAL: metrics::Result = + metrics::try_create_int_counter_vec( + "dep_warn_total", + "Count of warns logged per enabled dependency", + &["target"] + ); + /// Count of `ERROR` logs registered per enabled dependency. + pub static ref DEP_ERRORS_TOTAL: metrics::Result = + metrics::try_create_int_counter_vec( + "dep_error_total", + "Count of errors logged per enabled dependency", + &["target"] + ); +} + +/// Layer that registers Prometheus metrics for `INFO`, `WARN` and `ERROR` logs emitted per dependency. +/// Dependencies are enabled via the `RUST_LOG` env flag. +pub struct MetricsLayer; + +impl tracing_subscriber::layer::Layer for MetricsLayer { + fn on_event( + &self, + event: &tracing_core::Event<'_>, + _ctx: tracing_subscriber::layer::Context<'_, S>, + ) { + // get the event's normalized metadata + // this is necessary to get the correct module path for libp2p events + let normalized_meta = event.normalized_metadata(); + let meta = normalized_meta.as_ref().unwrap_or_else(|| event.metadata()); + + if !meta.is_event() { + // ignore tracing span events + return; + } + + let full_target = meta.module_path().unwrap_or_else(|| meta.target()); + let target = full_target + .split_once("::") + .map(|(name, _rest)| name) + .unwrap_or(full_target); + let target = &[target]; + match *meta.level() { + tracing_core::Level::INFO => metrics::inc_counter_vec(&DEP_INFOS_TOTAL, target), + tracing_core::Level::WARN => metrics::inc_counter_vec(&DEP_WARNS_TOTAL, target), + tracing_core::Level::ERROR => metrics::inc_counter_vec(&DEP_ERRORS_TOTAL, target), + _ => {} + } + } +} diff --git a/lighthouse/Cargo.toml b/lighthouse/Cargo.toml index 8766865650..c402e5b932 100644 --- a/lighthouse/Cargo.toml +++ b/lighthouse/Cargo.toml @@ -35,7 +35,6 @@ types = { workspace = true } bls = { workspace = true } ethereum_hashing = { workspace = true } clap = { workspace = true } -env_logger = { workspace = true } environment = { workspace = true } boot_node = { path = "../boot_node" } futures = { workspace = true } @@ -57,6 +56,8 @@ unused_port = { workspace = true } database_manager = { path = "../database_manager" } slasher = { workspace = true } validator_manager = { path = "../validator_manager" } +tracing-subscriber = { workspace = true } +logging = { workspace = true } [dev-dependencies] tempfile = { workspace = true } diff --git a/lighthouse/src/main.rs b/lighthouse/src/main.rs index edf661abab..ccdfa60f11 100644 --- a/lighthouse/src/main.rs +++ b/lighthouse/src/main.rs @@ -4,7 +4,6 @@ use beacon_node::ProductionBeaconNode; use clap::{App, Arg, ArgMatches}; use clap_utils::{flags::DISABLE_MALLOC_TUNING_FLAG, get_eth2_network_config}; use directory::{parse_path_or_default, DEFAULT_BEACON_NODE_DIR, DEFAULT_VALIDATOR_DIR}; -use env_logger::{Builder, Env}; use environment::{EnvironmentBuilder, LoggerConfig}; use eth2_network_config::{Eth2NetworkConfig, DEFAULT_HARDCODED_NETWORK, HARDCODED_NET_NAMES}; use ethereum_hashing::have_sha_extensions; @@ -15,6 +14,7 @@ use slog::{crit, info}; use std::path::PathBuf; use std::process::exit; use task_executor::ShutdownReason; +use tracing_subscriber::{layer::SubscriberExt, util::SubscriberInitExt}; use types::{EthSpec, EthSpecId}; use validator_client::ProductionValidatorClient; @@ -84,7 +84,8 @@ fn main() { .arg( Arg::with_name("env_log") .short("l") - .help("Enables environment logging giving access to sub-protocol logs such as discv5 and libp2p", + .help( + "DEPRECATED Enables environment logging giving access to sub-protocol logs such as discv5 and libp2p", ) .takes_value(false), ) @@ -364,9 +365,32 @@ fn main() { } } - // Debugging output for libp2p and external crates. - if matches.is_present("env_log") { - Builder::from_env(Env::default()).init(); + // read the `RUST_LOG` statement + let filter_layer = match tracing_subscriber::EnvFilter::try_from_default_env() + .or_else(|_| tracing_subscriber::EnvFilter::try_new("debug")) + { + Ok(filter) => filter, + Err(e) => { + eprintln!("Failed to initialize dependency logging {e}"); + exit(1) + } + }; + + let turn_on_terminal_logs = matches.is_present("env_log"); + + if let Err(e) = tracing_subscriber::fmt() + .with_env_filter(filter_layer) + .with_writer(move || { + tracing_subscriber::fmt::writer::OptionalWriter::::from( + turn_on_terminal_logs.then(std::io::stdout), + ) + }) + .finish() + .with(logging::MetricsLayer) + .try_init() + { + eprintln!("Failed to initialize dependency logging {e}"); + exit(1) } let result = get_eth2_network_config(&matches).and_then(|eth2_network_config| { From f22e5b0f85ac0a667b1fab712abb8112d1dbc465 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Tue, 16 Jan 2024 10:44:26 +0200 Subject: [PATCH 07/25] Critical dependency logging (#4988) * add metrics layer * add metrics * simplify getting the target * make clippy happy * fix typos * unify deps under workspace * make import statement shorter, fix typos * enable warn by default, mark flag as deprecated * do not exit on error when initializing logging fails * revert exit on error * adjust bootnode logging * add logging layer * non blocking file writer * non blocking file writer * add tracing visitor * use target as is by default * make libp2p events register correctly * adjust repilcated cli help * refactor tracing layer * linting * filesize * log gossipsub, dont filter by log level * turn on debug logs by default, remove deprecation warning * truncate file, add timestamp, add unit test * suppress output (#5) * use tracing appender * cleanup * Add a task to remove old log files and upgrade to warn level * Add the time feature for tokio * Udeps and fmt --------- Co-authored-by: Diva M Co-authored-by: Divma <26765164+divagant-martian@users.noreply.github.com> Co-authored-by: Age Manning --- Cargo.lock | 48 +++++++- Cargo.toml | 3 +- common/directory/src/lib.rs | 1 + common/logging/Cargo.toml | 4 +- common/logging/src/lib.rs | 48 ++++++++ common/logging/src/tracing_logging_layer.rs | 115 ++++++++++++++++++++ lighthouse/src/main.rs | 54 ++++----- 7 files changed, 238 insertions(+), 35 deletions(-) create mode 100644 common/logging/src/tracing_logging_layer.rs diff --git a/Cargo.lock b/Cargo.lock index 532388cf4c..306580e430 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4793,6 +4793,8 @@ dependencies = [ "sloggers", "take_mut", "tokio", + "tracing", + "tracing-appender", "tracing-core", "tracing-log", "tracing-subscriber", @@ -4857,6 +4859,15 @@ version = "0.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ffbee8634e0d45d258acb448e7eaab3fce7a0a467395d4d9f228e3c1f01fb2e4" +[[package]] +name = "matchers" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8263075bb86c5a1b1427b5ae862e8889656f126e9f77c484496e8b47cf5c5558" +dependencies = [ + "regex-automata 0.1.10", +] + [[package]] name = "matches" version = "0.1.10" @@ -6406,8 +6417,17 @@ checksum = "380b951a9c5e80ddfd6136919eef32310721aa4aacd4889a8d39124b026ab343" dependencies = [ "aho-corasick", "memchr", - "regex-automata", - "regex-syntax", + "regex-automata 0.4.3", + "regex-syntax 0.8.2", +] + +[[package]] +name = "regex-automata" +version = "0.1.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6c230d73fb8d8c1b9c0b3135c5142a8acee3a0558fb8db5cf1cb65f8d7862132" +dependencies = [ + "regex-syntax 0.6.29", ] [[package]] @@ -6418,9 +6438,15 @@ checksum = "5f804c7828047e88b2d32e2d7fe5a105da8ee3264f01902f796c8e067dc2483f" dependencies = [ "aho-corasick", "memchr", - "regex-syntax", + "regex-syntax 0.8.2", ] +[[package]] +name = "regex-syntax" +version = "0.6.29" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f162c6dd7b008981e4d40210aca20b4bd0f9b60ca9271061b07f78537722f2e1" + [[package]] name = "regex-syntax" version = "0.8.2" @@ -8190,6 +8216,18 @@ dependencies = [ "tracing-core", ] +[[package]] +name = "tracing-appender" +version = "0.2.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3566e8ce28cc0a3fe42519fc80e6b4c943cc4c8cef275620eb8dac2d3d4e06cf" +dependencies = [ + "crossbeam-channel", + "thiserror", + "time", + "tracing-subscriber", +] + [[package]] name = "tracing-attributes" version = "0.1.27" @@ -8238,10 +8276,14 @@ version = "0.3.18" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ad0f048c97dbd9faa9b7df56362b8ebcaa52adb06b498c050d2f4e32f90a7a8b" dependencies = [ + "matchers", "nu-ansi-term", + "once_cell", + "regex", "sharded-slab", "smallvec", "thread_local", + "tracing", "tracing-core", "tracing-log", ] diff --git a/Cargo.toml b/Cargo.toml index f43d375f0d..a3e1430f3f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -160,9 +160,10 @@ tempfile = "3" tokio = { version = "1", features = ["rt-multi-thread", "sync", "signal"] } tokio-stream = { version = "0.1", features = ["sync"] } tokio-util = { version = "0.6", features = ["codec", "compat", "time"] } +tracing-appender = "0.2" tracing-core = "0.1" tracing-log = "0.2" -tracing-subscriber = "0.3" +tracing-subscriber = { version = "0.3", features = ["env-filter"] } tree_hash = "0.5" tree_hash_derive = "0.5" url = "2" diff --git a/common/directory/src/lib.rs b/common/directory/src/lib.rs index 62b98aab94..e8585c504a 100644 --- a/common/directory/src/lib.rs +++ b/common/directory/src/lib.rs @@ -10,6 +10,7 @@ pub const DEFAULT_NETWORK_DIR: &str = "network"; pub const DEFAULT_VALIDATOR_DIR: &str = "validators"; pub const DEFAULT_SECRET_DIR: &str = "secrets"; pub const DEFAULT_WALLET_DIR: &str = "wallets"; +pub const DEFAULT_TRACING_DIR: &str = "tracing"; /// Base directory name for unnamed testnets passed through the --testnet-dir flag pub const CUSTOM_TESTNET_DIR: &str = "custom"; diff --git a/common/logging/Cargo.toml b/common/logging/Cargo.toml index 6975711651..1fad56d475 100644 --- a/common/logging/Cargo.toml +++ b/common/logging/Cargo.toml @@ -19,7 +19,9 @@ slog-async = { workspace = true } slog-term = { workspace = true } sloggers = { workspace = true } take_mut = "0.2.2" -tokio = { workspace = true } +tokio = { workspace = true, features = [ "time" ] } +tracing = "0.1" tracing-core = { workspace = true } tracing-log = { workspace = true } tracing-subscriber = { workspace = true } +tracing-appender = { workspace = true } diff --git a/common/logging/src/lib.rs b/common/logging/src/lib.rs index 80fc00a00f..5217d541cf 100644 --- a/common/logging/src/lib.rs +++ b/common/logging/src/lib.rs @@ -7,15 +7,21 @@ use lighthouse_metrics::{ use slog::Logger; use slog_term::Decorator; use std::io::{Result, Write}; +use std::path::PathBuf; use std::time::{Duration, Instant}; +use tracing_appender::non_blocking::NonBlocking; +use tracing_logging_layer::LoggingLayer; +use tracing_subscriber::{layer::SubscriberExt, util::SubscriberInitExt}; pub const MAX_MESSAGE_WIDTH: usize = 40; pub mod async_record; mod sse_logging_components; +mod tracing_logging_layer; mod tracing_metrics_layer; pub use sse_logging_components::SSELoggingComponents; +pub use tracing_logging_layer::cleanup_logging_task; pub use tracing_metrics_layer::MetricsLayer; /// The minimum interval between log messages indicating that a queue is full. @@ -217,6 +223,48 @@ impl TimeLatch { } } +pub fn create_tracing_layer(base_tracing_log_path: PathBuf, turn_on_terminal_logs: bool) { + let filter_layer = match tracing_subscriber::EnvFilter::try_from_default_env() + .or_else(|_| tracing_subscriber::EnvFilter::try_new("warn")) + { + Ok(filter) => filter, + Err(e) => { + eprintln!("Failed to initialize dependency logging {e}"); + return; + } + }; + + let libp2p_writer = + tracing_appender::rolling::daily(base_tracing_log_path.clone(), "libp2p.log"); + let discv5_writer = + tracing_appender::rolling::daily(base_tracing_log_path.clone(), "discv5.log"); + + let (libp2p_non_blocking_writer, libp2p_guard) = NonBlocking::new(libp2p_writer); + let (discv5_non_blocking_writer, discv5_guard) = NonBlocking::new(discv5_writer); + + let custom_layer = LoggingLayer { + libp2p_non_blocking_writer, + libp2p_guard, + discv5_non_blocking_writer, + discv5_guard, + }; + + if let Err(e) = tracing_subscriber::fmt() + .with_env_filter(filter_layer) + .with_writer(move || { + tracing_subscriber::fmt::writer::OptionalWriter::::from( + turn_on_terminal_logs.then(std::io::stdout), + ) + }) + .finish() + .with(MetricsLayer) + .with(custom_layer) + .try_init() + { + eprintln!("Failed to initialize dependency logging {e}"); + } +} + /// Return a logger suitable for test usage. /// /// By default no logs will be printed, but they can be enabled via diff --git a/common/logging/src/tracing_logging_layer.rs b/common/logging/src/tracing_logging_layer.rs new file mode 100644 index 0000000000..a74e24bdb6 --- /dev/null +++ b/common/logging/src/tracing_logging_layer.rs @@ -0,0 +1,115 @@ +use chrono::{naive::Days, prelude::*}; +use slog::{debug, warn}; +use std::io::Write; +use tracing::Subscriber; +use tracing_appender::non_blocking::{NonBlocking, WorkerGuard}; +use tracing_subscriber::layer::Context; +use tracing_subscriber::Layer; + +pub struct LoggingLayer { + pub libp2p_non_blocking_writer: NonBlocking, + pub libp2p_guard: WorkerGuard, + pub discv5_non_blocking_writer: NonBlocking, + pub discv5_guard: WorkerGuard, +} + +impl Layer for LoggingLayer +where + S: Subscriber, +{ + fn on_event(&self, event: &tracing::Event<'_>, _ctx: Context) { + let meta = event.metadata(); + let log_level = meta.level(); + let timestamp = Local::now().format("%Y-%m-%d %H:%M:%S").to_string(); + + let target = match meta.target().split_once("::") { + Some((crate_name, _)) => crate_name, + None => "unknown", + }; + + let mut writer = match target { + "libp2p_gossipsub" => self.libp2p_non_blocking_writer.clone(), + "discv5" => self.discv5_non_blocking_writer.clone(), + _ => return, + }; + + let mut visitor = LogMessageExtractor { + message: String::default(), + }; + + event.record(&mut visitor); + let message = format!("{} {} {}\n", timestamp, log_level, visitor.message); + + if let Err(e) = writer.write_all(message.as_bytes()) { + eprintln!("Failed to write log: {}", e); + } + } +} + +struct LogMessageExtractor { + message: String, +} + +impl tracing_core::field::Visit for LogMessageExtractor { + fn record_debug(&mut self, _: &tracing_core::Field, value: &dyn std::fmt::Debug) { + self.message = format!("{} {:?}", self.message, value); + } +} + +/// Creates a long lived async task that routinely deletes old tracing log files +pub async fn cleanup_logging_task(path: std::path::PathBuf, log: slog::Logger) { + loop { + // Delay for 1 day and then prune old logs + tokio::time::sleep(std::time::Duration::from_secs(60 * 60 * 24)).await; + + let Some(yesterday_date) = chrono::prelude::Local::now() + .naive_local() + .checked_sub_days(Days::new(1)) + else { + warn!(log, "Could not calculate the current date"); + return; + }; + + // Search for old log files + let dir = path.as_path(); + + if dir.is_dir() { + let Ok(files) = std::fs::read_dir(dir) else { + warn!(log, "Could not read log directory contents"; "path" => ?dir); + break; + }; + + for file in files { + let Ok(dir_entry) = file else { + warn!(log, "Could not read file"); + continue; + }; + + let Ok(file_name) = dir_entry.file_name().into_string() else { + warn!(log, "Could not read file"; "file" => ?dir_entry); + continue; + }; + + if file_name.starts_with("libp2p.log") | file_name.starts_with("discv5.log") { + let log_file_date = file_name.split('.').collect::>(); + if log_file_date.len() == 3 { + let Ok(log_file_date_type) = + NaiveDate::parse_from_str(log_file_date[2], "%Y-%m-%d") + else { + warn!(log, "Could not parse log file date"; "file" => file_name); + continue; + }; + + if log_file_date_type < yesterday_date.into() { + // Delete the file, its too old + debug!(log, "Removing old log file"; "file" => &file_name); + if let Err(e) = std::fs::remove_file(dir_entry.path()) { + warn!(log, "Failed to remove log file"; "file" => file_name, "error" => %e); + } + } + } + } + } + } + } +} diff --git a/lighthouse/src/main.rs b/lighthouse/src/main.rs index ccdfa60f11..b8cedfde0d 100644 --- a/lighthouse/src/main.rs +++ b/lighthouse/src/main.rs @@ -14,7 +14,6 @@ use slog::{crit, info}; use std::path::PathBuf; use std::process::exit; use task_executor::ShutdownReason; -use tracing_subscriber::{layer::SubscriberExt, util::SubscriberInitExt}; use types::{EthSpec, EthSpecId}; use validator_client::ProductionValidatorClient; @@ -365,34 +364,6 @@ fn main() { } } - // read the `RUST_LOG` statement - let filter_layer = match tracing_subscriber::EnvFilter::try_from_default_env() - .or_else(|_| tracing_subscriber::EnvFilter::try_new("debug")) - { - Ok(filter) => filter, - Err(e) => { - eprintln!("Failed to initialize dependency logging {e}"); - exit(1) - } - }; - - let turn_on_terminal_logs = matches.is_present("env_log"); - - if let Err(e) = tracing_subscriber::fmt() - .with_env_filter(filter_layer) - .with_writer(move || { - tracing_subscriber::fmt::writer::OptionalWriter::::from( - turn_on_terminal_logs.then(std::io::stdout), - ) - }) - .finish() - .with(logging::MetricsLayer) - .try_init() - { - eprintln!("Failed to initialize dependency logging {e}"); - exit(1) - } - let result = get_eth2_network_config(&matches).and_then(|eth2_network_config| { let eth_spec_id = eth2_network_config.eth_spec_id()?; @@ -534,7 +505,7 @@ fn run( }; let logger_config = LoggerConfig { - path: log_path, + path: log_path.clone(), debug_level: String::from(debug_level), logfile_debug_level: String::from(logfile_debug_level), log_format: log_format.map(String::from), @@ -557,6 +528,29 @@ fn run( let log = environment.core_context().log().clone(); + let mut tracing_log_path: Option = clap_utils::parse_optional(matches, "logfile")?; + + if tracing_log_path.is_none() { + tracing_log_path = Some( + parse_path_or_default(matches, "datadir")? + .join(DEFAULT_BEACON_NODE_DIR) + .join("logs"), + ) + } + + let path = tracing_log_path.clone().unwrap(); + + let turn_on_terminal_logs = matches.is_present("env_log"); + + // Run a task to clean up old tracing logs. + let log_cleaner_context = environment.service_context("log_cleaner".to_string()); + log_cleaner_context.executor.spawn( + logging::cleanup_logging_task(path.clone(), log.clone()), + "log_cleaner", + ); + + logging::create_tracing_layer(path, turn_on_terminal_logs); + // Allow Prometheus to export the time at which the process was started. metrics::expose_process_start_time(&log); From a0b407c15dbcfb9c5c201b53021fbdc924411e9e Mon Sep 17 00:00:00 2001 From: Mac L Date: Fri, 19 Jan 2024 07:21:38 +1100 Subject: [PATCH 08/25] Add Deneb readiness logging (#5074) --- .../beacon_chain/src/capella_readiness.rs | 3 +- .../beacon_chain/src/deneb_readiness.rs | 121 ++++++++++++++++++ beacon_node/beacon_chain/src/lib.rs | 1 + beacon_node/client/src/notifier.rs | 78 ++++++++++- consensus/types/src/payload.rs | 59 ++++++++- 5 files changed, 252 insertions(+), 10 deletions(-) create mode 100644 beacon_node/beacon_chain/src/deneb_readiness.rs diff --git a/beacon_node/beacon_chain/src/capella_readiness.rs b/beacon_node/beacon_chain/src/capella_readiness.rs index bb729d8999..cde71d462d 100644 --- a/beacon_node/beacon_chain/src/capella_readiness.rs +++ b/beacon_node/beacon_chain/src/capella_readiness.rs @@ -1,5 +1,4 @@ -//! Provides tools for checking if a node is ready for the Capella upgrade and following merge -//! transition. +//! Provides tools for checking if a node is ready for the Capella upgrade. use crate::{BeaconChain, BeaconChainTypes}; use execution_layer::http::{ diff --git a/beacon_node/beacon_chain/src/deneb_readiness.rs b/beacon_node/beacon_chain/src/deneb_readiness.rs new file mode 100644 index 0000000000..1ba6fe3ea6 --- /dev/null +++ b/beacon_node/beacon_chain/src/deneb_readiness.rs @@ -0,0 +1,121 @@ +//! Provides tools for checking if a node is ready for the Deneb upgrade. + +use crate::{BeaconChain, BeaconChainTypes}; +use execution_layer::http::{ + ENGINE_FORKCHOICE_UPDATED_V3, ENGINE_GET_PAYLOAD_V3, ENGINE_NEW_PAYLOAD_V3, +}; +use serde::{Deserialize, Serialize}; +use std::fmt; +use std::time::Duration; +use types::*; + +/// The time before the Deneb fork when we will start issuing warnings about preparation. +use super::merge_readiness::SECONDS_IN_A_WEEK; +pub const DENEB_READINESS_PREPARATION_SECONDS: u64 = SECONDS_IN_A_WEEK * 2; +pub const ENGINE_CAPABILITIES_REFRESH_INTERVAL: u64 = 300; + +#[derive(Debug, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +#[serde(tag = "type")] +pub enum DenebReadiness { + /// The execution engine is deneb-enabled (as far as we can tell) + Ready, + /// We are connected to an execution engine which doesn't support the V3 engine api methods + V3MethodsNotSupported { error: String }, + /// The transition configuration with the EL failed, there might be a problem with + /// connectivity, authentication or a difference in configuration. + ExchangeCapabilitiesFailed { error: String }, + /// The user has not configured an execution endpoint + NoExecutionEndpoint, +} + +impl fmt::Display for DenebReadiness { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + DenebReadiness::Ready => { + write!(f, "This node appears ready for Deneb.") + } + DenebReadiness::ExchangeCapabilitiesFailed { error } => write!( + f, + "Could not exchange capabilities with the \ + execution endpoint: {}", + error + ), + DenebReadiness::NoExecutionEndpoint => write!( + f, + "The --execution-endpoint flag is not specified, this is a \ + requirement post-merge" + ), + DenebReadiness::V3MethodsNotSupported { error } => write!( + f, + "Execution endpoint does not support Deneb methods: {}", + error + ), + } + } +} + +impl BeaconChain { + /// Returns `true` if deneb epoch is set and Deneb fork has occurred or will + /// occur within `DENEB_READINESS_PREPARATION_SECONDS` + pub fn is_time_to_prepare_for_deneb(&self, current_slot: Slot) -> bool { + if let Some(deneb_epoch) = self.spec.deneb_fork_epoch { + let deneb_slot = deneb_epoch.start_slot(T::EthSpec::slots_per_epoch()); + let deneb_readiness_preparation_slots = + DENEB_READINESS_PREPARATION_SECONDS / self.spec.seconds_per_slot; + // Return `true` if Deneb has happened or is within the preparation time. + current_slot + deneb_readiness_preparation_slots > deneb_slot + } else { + // The Deneb fork epoch has not been defined yet, no need to prepare. + false + } + } + + /// Attempts to connect to the EL and confirm that it is ready for capella. + pub async fn check_deneb_readiness(&self) -> DenebReadiness { + if let Some(el) = self.execution_layer.as_ref() { + match el + .get_engine_capabilities(Some(Duration::from_secs( + ENGINE_CAPABILITIES_REFRESH_INTERVAL, + ))) + .await + { + Err(e) => { + // The EL was either unreachable or responded with an error + DenebReadiness::ExchangeCapabilitiesFailed { + error: format!("{:?}", e), + } + } + Ok(capabilities) => { + let mut missing_methods = String::from("Required Methods Unsupported:"); + let mut all_good = true; + if !capabilities.get_payload_v3 { + missing_methods.push(' '); + missing_methods.push_str(ENGINE_GET_PAYLOAD_V3); + all_good = false; + } + if !capabilities.forkchoice_updated_v3 { + missing_methods.push(' '); + missing_methods.push_str(ENGINE_FORKCHOICE_UPDATED_V3); + all_good = false; + } + if !capabilities.new_payload_v3 { + missing_methods.push(' '); + missing_methods.push_str(ENGINE_NEW_PAYLOAD_V3); + all_good = false; + } + + if all_good { + DenebReadiness::Ready + } else { + DenebReadiness::V3MethodsNotSupported { + error: missing_methods, + } + } + } + } + } else { + DenebReadiness::NoExecutionEndpoint + } + } +} diff --git a/beacon_node/beacon_chain/src/lib.rs b/beacon_node/beacon_chain/src/lib.rs index 40fd5e63d1..ce841b106c 100644 --- a/beacon_node/beacon_chain/src/lib.rs +++ b/beacon_node/beacon_chain/src/lib.rs @@ -18,6 +18,7 @@ pub mod canonical_head; pub mod capella_readiness; pub mod chain_config; pub mod data_availability_checker; +pub mod deneb_readiness; mod early_attester_cache; mod errors; pub mod eth1_chain; diff --git a/beacon_node/client/src/notifier.rs b/beacon_node/client/src/notifier.rs index 2c7738e8fa..8a0e5ce223 100644 --- a/beacon_node/client/src/notifier.rs +++ b/beacon_node/client/src/notifier.rs @@ -1,6 +1,7 @@ use crate::metrics; use beacon_chain::{ capella_readiness::CapellaReadiness, + deneb_readiness::DenebReadiness, merge_readiness::{GenesisExecutionPayloadStatus, MergeConfig, MergeReadiness}, BeaconChain, BeaconChainTypes, ExecutionStatus, }; @@ -319,6 +320,7 @@ pub fn spawn_notifier( eth1_logging(&beacon_chain, &log); merge_readiness_logging(current_slot, &beacon_chain, &log).await; capella_readiness_logging(current_slot, &beacon_chain, &log).await; + deneb_readiness_logging(current_slot, &beacon_chain, &log).await; } }; @@ -356,8 +358,8 @@ async fn merge_readiness_logging( } if merge_completed && !has_execution_layer { + // Logging of the EE being offline is handled in the other readiness logging functions. if !beacon_chain.is_time_to_prepare_for_capella(current_slot) { - // logging of the EE being offline is handled in `capella_readiness_logging()` error!( log, "Execution endpoint required"; @@ -445,12 +447,15 @@ async fn capella_readiness_logging( } if capella_completed && !has_execution_layer { - error!( - log, - "Execution endpoint required"; - "info" => "you need a Capella enabled execution engine to validate blocks, see: \ - https://lighthouse-book.sigmaprime.io/merge-migration.html" - ); + // Logging of the EE being offline is handled in the other readiness logging functions. + if !beacon_chain.is_time_to_prepare_for_deneb(current_slot) { + error!( + log, + "Execution endpoint required"; + "info" => "you need a Capella enabled execution engine to validate blocks, see: \ + https://lighthouse-book.sigmaprime.io/merge-migration.html" + ); + } return; } @@ -479,6 +484,65 @@ async fn capella_readiness_logging( } } +/// Provides some helpful logging to users to indicate if their node is ready for Deneb +async fn deneb_readiness_logging( + current_slot: Slot, + beacon_chain: &BeaconChain, + log: &Logger, +) { + let deneb_completed = beacon_chain + .canonical_head + .cached_head() + .snapshot + .beacon_block + .message() + .body() + .execution_payload() + .map_or(false, |payload| payload.blob_gas_used().is_ok()); + + let has_execution_layer = beacon_chain.execution_layer.is_some(); + + if deneb_completed && has_execution_layer + || !beacon_chain.is_time_to_prepare_for_deneb(current_slot) + { + return; + } + + if deneb_completed && !has_execution_layer { + error!( + log, + "Execution endpoint required"; + "info" => "you need a Deneb enabled execution engine to validate blocks, see: \ + https://lighthouse-book.sigmaprime.io/merge-migration.html" + ); + return; + } + + match beacon_chain.check_deneb_readiness().await { + DenebReadiness::Ready => { + info!( + log, + "Ready for Deneb"; + "info" => "ensure the execution endpoint is updated to the latest Deneb/Cancun release" + ) + } + readiness @ DenebReadiness::ExchangeCapabilitiesFailed { error: _ } => { + error!( + log, + "Not ready for Deneb"; + "hint" => "the execution endpoint may be offline", + "info" => %readiness, + ) + } + readiness => warn!( + log, + "Not ready for Deneb"; + "hint" => "try updating the execution endpoint", + "info" => %readiness, + ), + } +} + async fn genesis_execution_payload_logging( beacon_chain: &BeaconChain, log: &Logger, diff --git a/consensus/types/src/payload.rs b/consensus/types/src/payload.rs index fa7745ad97..2f7975161c 100644 --- a/consensus/types/src/payload.rs +++ b/consensus/types/src/payload.rs @@ -39,6 +39,7 @@ pub trait ExecPayload: Debug + Clone + PartialEq + Hash + TreeHash + fn transactions(&self) -> Option<&Transactions>; /// fork-specific fields fn withdrawals_root(&self) -> Result; + fn blob_gas_used(&self) -> Result; /// Is this a default payload with 0x0 roots for transactions and withdrawals? fn is_default_with_zero_roots(&self) -> bool; @@ -254,6 +255,13 @@ impl ExecPayload for FullPayload { } } + fn blob_gas_used(&self) -> Result { + match self { + FullPayload::Merge(_) | FullPayload::Capella(_) => Err(Error::IncorrectStateVariant), + FullPayload::Deneb(ref inner) => Ok(inner.execution_payload.blob_gas_used), + } + } + fn is_default_with_zero_roots<'a>(&'a self) -> bool { map_full_payload_ref!(&'a _, self.to_ref(), move |payload, cons| { cons(payload); @@ -372,6 +380,15 @@ impl<'b, T: EthSpec> ExecPayload for FullPayloadRef<'b, T> { } } + fn blob_gas_used(&self) -> Result { + match self { + FullPayloadRef::Merge(_) | FullPayloadRef::Capella(_) => { + Err(Error::IncorrectStateVariant) + } + FullPayloadRef::Deneb(inner) => Ok(inner.execution_payload.blob_gas_used), + } + } + fn is_default_with_zero_roots<'a>(&'a self) -> bool { map_full_payload_ref!(&'a _, self, move |payload, cons| { cons(payload); @@ -533,6 +550,15 @@ impl ExecPayload for BlindedPayload { } } + fn blob_gas_used(&self) -> Result { + match self { + BlindedPayload::Merge(_) | BlindedPayload::Capella(_) => { + Err(Error::IncorrectStateVariant) + } + BlindedPayload::Deneb(ref inner) => Ok(inner.execution_payload_header.blob_gas_used), + } + } + fn is_default_with_zero_roots(&self) -> bool { self.to_ref().is_default_with_zero_roots() } @@ -621,6 +647,15 @@ impl<'b, T: EthSpec> ExecPayload for BlindedPayloadRef<'b, T> { } } + fn blob_gas_used(&self) -> Result { + match self { + BlindedPayloadRef::Merge(_) | BlindedPayloadRef::Capella(_) => { + Err(Error::IncorrectStateVariant) + } + BlindedPayloadRef::Deneb(inner) => Ok(inner.execution_payload_header.blob_gas_used), + } + } + fn is_default_with_zero_roots<'a>(&'a self) -> bool { map_blinded_payload_ref!(&'b _, self, move |payload, cons| { cons(payload); @@ -646,7 +681,8 @@ macro_rules! impl_exec_payload_common { $block_type_variant:ident, // Blinded | Full $is_default_with_empty_roots:block, $f:block, - $g:block) => { + $g:block, + $h:block) => { impl ExecPayload for $wrapper_type { fn block_type() -> BlockType { BlockType::$block_type_variant @@ -704,6 +740,11 @@ macro_rules! impl_exec_payload_common { let g = $g; g(self) } + + fn blob_gas_used(&self) -> Result { + let h = $h; + h(self) + } } impl From<$wrapped_type> for $wrapper_type { @@ -741,6 +782,14 @@ macro_rules! impl_exec_payload_for_fork { wrapper_ref_type.withdrawals_root() }; c + }, + { + let c: for<'a> fn(&'a $wrapper_type_header) -> Result = + |payload: &$wrapper_type_header| { + let wrapper_ref_type = BlindedPayloadRef::$fork_variant(&payload); + wrapper_ref_type.blob_gas_used() + }; + c } ); @@ -820,6 +869,14 @@ macro_rules! impl_exec_payload_for_fork { wrapper_ref_type.withdrawals_root() }; c + }, + { + let c: for<'a> fn(&'a $wrapper_type_full) -> Result = + |payload: &$wrapper_type_full| { + let wrapper_ref_type = FullPayloadRef::$fork_variant(&payload); + wrapper_ref_type.blob_gas_used() + }; + c } ); From 47b28c4935a40d9869214904f060dda4aa1678d3 Mon Sep 17 00:00:00 2001 From: Mac L Date: Fri, 19 Jan 2024 07:59:08 +1100 Subject: [PATCH 09/25] Remove blobs_db when `purge-db` (#5081) --- beacon_node/src/config.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/beacon_node/src/config.rs b/beacon_node/src/config.rs index 2f67161ecf..c940049c50 100644 --- a/beacon_node/src/config.rs +++ b/beacon_node/src/config.rs @@ -62,6 +62,13 @@ pub fn get_config( fs::remove_dir_all(freezer_db) .map_err(|err| format!("Failed to remove freezer_db: {}", err))?; } + + // Remove the blobs db. + let blobs_db = client_config.get_blobs_db_path(); + if blobs_db.exists() { + fs::remove_dir_all(blobs_db) + .map_err(|err| format!("Failed to remove blobs_db: {}", err))?; + } } // Create `datadir` and any non-existing parent directories. From 2a161cef73e4d6e94c6ec920aebad5f94fde628b Mon Sep 17 00:00:00 2001 From: realbigsean Date: Thu, 18 Jan 2024 20:41:28 -0500 Subject: [PATCH 10/25] Bump h2 (#5085) * bump h2 * Empty commit for CI * bump the other h2 version --- Cargo.lock | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 306580e430..7874e1ec3d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3084,9 +3084,9 @@ dependencies = [ [[package]] name = "h2" -version = "0.3.23" +version = "0.3.24" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b553656127a00601c8ae5590fcfdc118e4083a7924b6cf4ffc1ea4b99dc429d7" +checksum = "bb2c4422095b67ee78da96fbb51a4cc413b3b25883c7717ff7ca1ab31022c9c9" dependencies = [ "bytes", "fnv", @@ -3103,9 +3103,9 @@ dependencies = [ [[package]] name = "h2" -version = "0.4.1" +version = "0.4.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "991910e35c615d8cab86b5ab04be67e6ad24d2bf5f4f11fdbbed26da999bbeab" +checksum = "31d030e59af851932b72ceebadf4a2b5986dba4c3b99dd2493f8273a0f151943" dependencies = [ "bytes", "fnv", @@ -3499,14 +3499,14 @@ dependencies = [ "futures-channel", "futures-core", "futures-util", - "h2 0.3.23", + "h2 0.3.24", "http 0.2.11", "http-body 0.4.6", "httparse", "httpdate", "itoa", "pin-project-lite", - "socket2 0.5.5", + "socket2 0.4.10", "tokio", "tower-service", "tracing", @@ -3522,7 +3522,7 @@ dependencies = [ "bytes", "futures-channel", "futures-util", - "h2 0.4.1", + "h2 0.4.2", "http 1.0.0", "http-body 1.0.0", "httparse", @@ -6464,7 +6464,7 @@ dependencies = [ "encoding_rs", "futures-core", "futures-util", - "h2 0.3.23", + "h2 0.3.24", "http 0.2.11", "http-body 0.4.6", "hyper 0.14.28", From 185646acb2ddcab08d0c2896e675e9bf49be1314 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Fri, 19 Jan 2024 12:56:30 +1100 Subject: [PATCH 11/25] Fix PublishBlockRequest SSZ decoding (#5078) * Fix PublishBlockRequest SSZ decoding * Send header for block v1 ssz client * Delete unused function --- beacon_node/http_api/src/lib.rs | 12 +++++-- .../tests/broadcast_validation_tests.rs | 10 +++--- common/eth2/src/lib.rs | 31 +++++------------ common/eth2/src/types.rs | 34 +++++++------------ consensus/types/src/signed_beacon_block.rs | 10 ++++++ 5 files changed, 47 insertions(+), 50 deletions(-) diff --git a/beacon_node/http_api/src/lib.rs b/beacon_node/http_api/src/lib.rs index f1d7769e89..02ea9518c6 100644 --- a/beacon_node/http_api/src/lib.rs +++ b/beacon_node/http_api/src/lib.rs @@ -45,7 +45,7 @@ use eth2::types::{ PublishBlockRequest, ValidatorBalancesRequestBody, ValidatorId, ValidatorStatus, ValidatorsRequestBody, }; -use eth2::{CONTENT_TYPE_HEADER, SSZ_CONTENT_TYPE_HEADER}; +use eth2::{CONSENSUS_VERSION_HEADER, CONTENT_TYPE_HEADER, SSZ_CONTENT_TYPE_HEADER}; use lighthouse_network::{types::SyncState, EnrExt, NetworkGlobals, PeerId, PubsubMessage}; use lighthouse_version::version_with_platform; use logging::SSELoggingComponents; @@ -1249,6 +1249,8 @@ pub fn serve( /* * beacon/blocks */ + let consensus_version_header_filter = + warp::header::header::(CONSENSUS_VERSION_HEADER); // POST beacon/blocks let post_beacon_blocks = eth_v1 @@ -1286,12 +1288,14 @@ pub fn serve( .and(warp::path("blocks")) .and(warp::path::end()) .and(warp::body::bytes()) + .and(consensus_version_header_filter) .and(task_spawner_filter.clone()) .and(chain_filter.clone()) .and(network_tx_filter.clone()) .and(log_filter.clone()) .then( move |block_bytes: Bytes, + consensus_version: ForkName, task_spawner: TaskSpawner, chain: Arc>, network_tx: UnboundedSender>, @@ -1299,7 +1303,7 @@ pub fn serve( task_spawner.spawn_async_with_rejection(Priority::P0, async move { let block_contents = PublishBlockRequest::::from_ssz_bytes( &block_bytes, - &chain.spec, + consensus_version, ) .map_err(|e| { warp_utils::reject::custom_bad_request(format!("invalid SSZ: {e:?}")) @@ -1356,6 +1360,7 @@ pub fn serve( .and(warp::query::()) .and(warp::path::end()) .and(warp::body::bytes()) + .and(consensus_version_header_filter) .and(task_spawner_filter.clone()) .and(chain_filter.clone()) .and(network_tx_filter.clone()) @@ -1363,6 +1368,7 @@ pub fn serve( .then( move |validation_level: api_types::BroadcastValidationQuery, block_bytes: Bytes, + consensus_version: ForkName, task_spawner: TaskSpawner, chain: Arc>, network_tx: UnboundedSender>, @@ -1370,7 +1376,7 @@ pub fn serve( task_spawner.spawn_async_with_rejection(Priority::P0, async move { let block_contents = PublishBlockRequest::::from_ssz_bytes( &block_bytes, - &chain.spec, + consensus_version, ) .map_err(|e| { warp_utils::reject::custom_bad_request(format!("invalid SSZ: {e:?}")) diff --git a/beacon_node/http_api/tests/broadcast_validation_tests.rs b/beacon_node/http_api/tests/broadcast_validation_tests.rs index eb39bdd115..2d42371a7c 100644 --- a/beacon_node/http_api/tests/broadcast_validation_tests.rs +++ b/beacon_node/http_api/tests/broadcast_validation_tests.rs @@ -2,17 +2,16 @@ use beacon_chain::{ test_utils::{AttestationStrategy, BlockStrategy}, GossipVerifiedBlock, IntoGossipVerifiedBlockContents, }; +use eth2::reqwest::StatusCode; use eth2::types::{BroadcastValidation, PublishBlockRequest, SignedBeaconBlock}; use http_api::test_utils::InteractiveTester; use http_api::{publish_blinded_block, publish_block, reconstruct_block, ProvenancedBlock}; use std::sync::Arc; use tree_hash::TreeHash; -use types::{Hash256, MainnetEthSpec, Slot}; +use types::{Epoch, EthSpec, ForkName, Hash256, MainnetEthSpec, Slot}; use warp::Rejection; use warp_utils::reject::CustomBadRequest; -use eth2::reqwest::StatusCode; - type E = MainnetEthSpec; /* @@ -190,7 +189,10 @@ pub async fn gossip_full_pass_ssz() { // `validator_count // 32`. let validator_count = 64; let num_initial: u64 = 31; - let tester = InteractiveTester::::new(None, validator_count).await; + // Deneb epoch set ahead of block slot, to test fork-based decoding + let mut spec = ForkName::Capella.make_genesis_spec(MainnetEthSpec::default_spec()); + spec.deneb_fork_epoch = Some(Epoch::new(4)); + let tester = InteractiveTester::::new(Some(spec), validator_count).await; // Create some chain depth. tester.harness.advance_slot(); diff --git a/common/eth2/src/lib.rs b/common/eth2/src/lib.rs index 9c8edc4bdb..bed5044b4b 100644 --- a/common/eth2/src/lib.rs +++ b/common/eth2/src/lib.rs @@ -377,25 +377,6 @@ impl BeaconNodeHttpClient { ok_or_error(response).await } - /// Generic POST function supporting arbitrary responses and timeouts. - async fn post_generic_with_ssz_body, U: IntoUrl>( - &self, - url: U, - body: T, - timeout: Option, - ) -> Result { - let mut builder = self.client.post(url); - if let Some(timeout) = timeout { - builder = builder.timeout(timeout); - } - let response = builder - .header("Content-Type", "application/octet-stream") - .body(body) - .send() - .await?; - ok_or_error(response).await - } - /// Generic POST function supporting arbitrary responses and timeouts. async fn post_generic_with_consensus_version( &self, @@ -866,10 +847,11 @@ impl BeaconNodeHttpClient { .push("beacon") .push("blocks"); - self.post_generic_with_ssz_body( + self.post_generic_with_consensus_version_and_ssz_body( path, block_contents.as_ssz_bytes(), Some(self.timeouts.proposal), + block_contents.signed_block().fork_name_unchecked(), ) .await?; @@ -910,8 +892,13 @@ impl BeaconNodeHttpClient { .push("beacon") .push("blinded_blocks"); - self.post_generic_with_ssz_body(path, block.as_ssz_bytes(), Some(self.timeouts.proposal)) - .await?; + self.post_generic_with_consensus_version_and_ssz_body( + path, + block.as_ssz_bytes(), + Some(self.timeouts.proposal), + block.fork_name_unchecked(), + ) + .await?; Ok(()) } diff --git a/common/eth2/src/types.rs b/common/eth2/src/types.rs index 98074615b8..3e8d0c6079 100644 --- a/common/eth2/src/types.rs +++ b/common/eth2/src/types.rs @@ -1487,7 +1487,7 @@ mod tests { .expect("should convert into signed block contents"); let decoded: PublishBlockRequest = - PublishBlockRequest::from_ssz_bytes(&block.as_ssz_bytes(), &spec) + PublishBlockRequest::from_ssz_bytes(&block.as_ssz_bytes(), ForkName::Capella) .expect("should decode Block"); assert!(matches!(decoded, PublishBlockRequest::Block(_))); } @@ -1505,9 +1505,11 @@ mod tests { let kzg_proofs = KzgProofs::::from(vec![KzgProof::empty()]); let signed_block_contents = PublishBlockRequest::new(block, Some((kzg_proofs, blobs))); - let decoded: PublishBlockRequest = - PublishBlockRequest::from_ssz_bytes(&signed_block_contents.as_ssz_bytes(), &spec) - .expect("should decode BlockAndBlobSidecars"); + let decoded: PublishBlockRequest = PublishBlockRequest::from_ssz_bytes( + &signed_block_contents.as_ssz_bytes(), + ForkName::Deneb, + ) + .expect("should decode BlockAndBlobSidecars"); assert!(matches!(decoded, PublishBlockRequest::BlockContents(_))); } } @@ -1746,22 +1748,11 @@ impl PublishBlockRequest { } } - /// SSZ decode with fork variant determined by slot. - pub fn from_ssz_bytes(bytes: &[u8], spec: &ChainSpec) -> Result { - let slot_len = ::ssz_fixed_len(); - let slot_bytes = bytes - .get(0..slot_len) - .ok_or(DecodeError::InvalidByteLength { - len: bytes.len(), - expected: slot_len, - })?; - - let slot = Slot::from_ssz_bytes(slot_bytes)?; - let fork_at_slot = spec.fork_name_at_slot::(slot); - - match fork_at_slot { + /// SSZ decode with fork variant determined by `fork_name`. + pub fn from_ssz_bytes(bytes: &[u8], fork_name: ForkName) -> Result { + match fork_name { ForkName::Base | ForkName::Altair | ForkName::Merge | ForkName::Capella => { - SignedBeaconBlock::from_ssz_bytes(bytes, spec) + SignedBeaconBlock::from_ssz_bytes_for_fork(bytes, fork_name) .map(|block| PublishBlockRequest::Block(block)) } ForkName::Deneb => { @@ -1771,8 +1762,9 @@ impl PublishBlockRequest { builder.register_type::>()?; let mut decoder = builder.build()?; - let block = decoder - .decode_next_with(|bytes| SignedBeaconBlock::from_ssz_bytes(bytes, spec))?; + let block = decoder.decode_next_with(|bytes| { + SignedBeaconBlock::from_ssz_bytes_for_fork(bytes, fork_name) + })?; let kzg_proofs = decoder.decode_next()?; let blobs = decoder.decode_next()?; Ok(PublishBlockRequest::new(block, Some((kzg_proofs, blobs)))) diff --git a/consensus/types/src/signed_beacon_block.rs b/consensus/types/src/signed_beacon_block.rs index 24b5e27c8c..37304de1f1 100644 --- a/consensus/types/src/signed_beacon_block.rs +++ b/consensus/types/src/signed_beacon_block.rs @@ -104,6 +104,16 @@ impl> SignedBeaconBlock Self::from_ssz_bytes_with(bytes, |bytes| BeaconBlock::from_ssz_bytes(bytes, spec)) } + /// SSZ decode with explicit fork variant. + pub fn from_ssz_bytes_for_fork( + bytes: &[u8], + fork_name: ForkName, + ) -> Result { + Self::from_ssz_bytes_with(bytes, |bytes| { + BeaconBlock::from_ssz_bytes_for_fork(bytes, fork_name) + }) + } + /// SSZ decode which attempts to decode all variants (slow). pub fn any_from_ssz_bytes(bytes: &[u8]) -> Result { Self::from_ssz_bytes_with(bytes, BeaconBlock::any_from_ssz_bytes) From 585124fb2f1a169d31de1413e47c22dcd229d234 Mon Sep 17 00:00:00 2001 From: Lion - dapplion <35266934+dapplion@users.noreply.github.com> Date: Mon, 22 Jan 2024 12:14:11 +0800 Subject: [PATCH 12/25] Hold HeadTracker lock until persisting to disk (#5084) * Fix head tracker drop order on un-ordered shutdown * lint --------- Co-authored-by: Michael Sproul --- beacon_node/beacon_chain/src/beacon_chain.rs | 27 ++++++++++++++------ beacon_node/beacon_chain/src/builder.rs | 4 ++- beacon_node/beacon_chain/src/head_tracker.rs | 9 ++++++- 3 files changed, 30 insertions(+), 10 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index bf41805479..fcd7be791d 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -30,7 +30,7 @@ use crate::eth1_finalization_cache::{Eth1FinalizationCache, Eth1FinalizationData use crate::events::ServerSentEventHandler; use crate::execution_payload::{get_execution_payload, NotifyExecutionLayer, PreparePayloadHandle}; use crate::fork_choice_signal::{ForkChoiceSignalRx, ForkChoiceSignalTx, ForkChoiceWaitResult}; -use crate::head_tracker::HeadTracker; +use crate::head_tracker::{HeadTracker, HeadTrackerReader, SszHeadTracker}; use crate::historical_blocks::HistoricalBlockError; use crate::light_client_finality_update_verification::{ Error as LightClientFinalityUpdateError, VerifiedLightClientFinalityUpdate, @@ -610,12 +610,19 @@ impl BeaconChain { let mut batch = vec![]; let _head_timer = metrics::start_timer(&metrics::PERSIST_HEAD); - batch.push(self.persist_head_in_batch()); + + // Hold a lock to head_tracker until it has been persisted to disk. Otherwise there's a race + // condition with the pruning thread which can result in a block present in the head tracker + // but absent in the DB. This inconsistency halts pruning and dramastically increases disk + // size. Ref: https://github.com/sigp/lighthouse/issues/4773 + let head_tracker = self.head_tracker.0.read(); + batch.push(self.persist_head_in_batch(&head_tracker)); let _fork_choice_timer = metrics::start_timer(&metrics::PERSIST_FORK_CHOICE); batch.push(self.persist_fork_choice_in_batch()); self.store.hot_db.do_atomically(batch)?; + drop(head_tracker); Ok(()) } @@ -623,25 +630,28 @@ impl BeaconChain { /// Return a `PersistedBeaconChain` without reference to a `BeaconChain`. pub fn make_persisted_head( genesis_block_root: Hash256, - head_tracker: &HeadTracker, + head_tracker_reader: &HeadTrackerReader, ) -> PersistedBeaconChain { PersistedBeaconChain { _canonical_head_block_root: DUMMY_CANONICAL_HEAD_BLOCK_ROOT, genesis_block_root, - ssz_head_tracker: head_tracker.to_ssz_container(), + ssz_head_tracker: SszHeadTracker::from_map(head_tracker_reader), } } /// Return a database operation for writing the beacon chain head to disk. - pub fn persist_head_in_batch(&self) -> KeyValueStoreOp { - Self::persist_head_in_batch_standalone(self.genesis_block_root, &self.head_tracker) + pub fn persist_head_in_batch( + &self, + head_tracker_reader: &HeadTrackerReader, + ) -> KeyValueStoreOp { + Self::persist_head_in_batch_standalone(self.genesis_block_root, head_tracker_reader) } pub fn persist_head_in_batch_standalone( genesis_block_root: Hash256, - head_tracker: &HeadTracker, + head_tracker_reader: &HeadTrackerReader, ) -> KeyValueStoreOp { - Self::make_persisted_head(genesis_block_root, head_tracker) + Self::make_persisted_head(genesis_block_root, head_tracker_reader) .as_kv_store_op(BEACON_CHAIN_DB_KEY) } @@ -1341,6 +1351,7 @@ impl BeaconChain { self.head_tracker.heads() } + /// Only used in tests. pub fn knows_head(&self, block_hash: &SignedBeaconBlockHash) -> bool { self.head_tracker.contains_head((*block_hash).into()) } diff --git a/beacon_node/beacon_chain/src/builder.rs b/beacon_node/beacon_chain/src/builder.rs index d36d996f01..330036d43c 100644 --- a/beacon_node/beacon_chain/src/builder.rs +++ b/beacon_node/beacon_chain/src/builder.rs @@ -805,10 +805,11 @@ where // // This *must* be stored before constructing the `BeaconChain`, so that its `Drop` instance // doesn't write a `PersistedBeaconChain` without the rest of the batch. + let head_tracker_reader = head_tracker.0.read(); self.pending_io_batch.push(BeaconChain::< Witness, >::persist_head_in_batch_standalone( - genesis_block_root, &head_tracker + genesis_block_root, &head_tracker_reader )); self.pending_io_batch.push(BeaconChain::< Witness, @@ -819,6 +820,7 @@ where .hot_db .do_atomically(self.pending_io_batch) .map_err(|e| format!("Error writing chain & metadata to disk: {:?}", e))?; + drop(head_tracker_reader); let genesis_validators_root = head_snapshot.beacon_state.genesis_validators_root(); let genesis_time = head_snapshot.beacon_state.genesis_time(); diff --git a/beacon_node/beacon_chain/src/head_tracker.rs b/beacon_node/beacon_chain/src/head_tracker.rs index 3fa577ff93..71e2473cdc 100644 --- a/beacon_node/beacon_chain/src/head_tracker.rs +++ b/beacon_node/beacon_chain/src/head_tracker.rs @@ -1,4 +1,4 @@ -use parking_lot::RwLock; +use parking_lot::{RwLock, RwLockReadGuard}; use ssz_derive::{Decode, Encode}; use std::collections::HashMap; use types::{Hash256, Slot}; @@ -16,6 +16,8 @@ pub enum Error { #[derive(Default, Debug)] pub struct HeadTracker(pub RwLock>); +pub type HeadTrackerReader<'a> = RwLockReadGuard<'a, HashMap>; + impl HeadTracker { /// Register a block with `Self`, so it may or may not be included in a `Self::heads` call. /// @@ -44,6 +46,11 @@ impl HeadTracker { /// Returns a `SszHeadTracker`, which contains all necessary information to restore the state /// of `Self` at some later point. + /// + /// Should ONLY be used for tests, due to the potential for database races. + /// + /// See + #[cfg(test)] pub fn to_ssz_container(&self) -> SszHeadTracker { SszHeadTracker::from_map(&self.0.read()) } From 70b0528f36d0c0cb8f5333b8472d416a7028a7a0 Mon Sep 17 00:00:00 2001 From: realbigsean Date: Sun, 21 Jan 2024 23:14:26 -0500 Subject: [PATCH 13/25] set deneb fork on testnets (#5089) * set deneb fork on testnets * re-order fields in holesky --- .../chiado/config.yaml | 247 ++++++++---------- .../holesky/config.yaml | 8 +- .../sepolia/config.yaml | 42 ++- 3 files changed, 151 insertions(+), 146 deletions(-) diff --git a/common/eth2_network_config/built_in_network_configs/chiado/config.yaml b/common/eth2_network_config/built_in_network_configs/chiado/config.yaml index fcebaa9bd8..8064ea5556 100644 --- a/common/eth2_network_config/built_in_network_configs/chiado/config.yaml +++ b/common/eth2_network_config/built_in_network_configs/chiado/config.yaml @@ -1,145 +1,38 @@ -# Extends the mainnet preset -PRESET_BASE: gnosis -# needs to exist because of Prysm. Otherwise it conflicts with mainnet genesis -CONFIG_NAME: chiado +PRESET_BASE: 'gnosis' + +# Free-form short name of the network that this configuration applies to - known +# canonical network names include: +# * 'mainnet' - there can be only one +# * 'prater' - testnet +# Must match the regex: [a-z0-9\-] +CONFIG_NAME: 'chiado' + +# Transition +# --------------------------------------------------------------- +# Projected time: 2022-11-04T15:00:00.000Z, block: 680928 +TERMINAL_TOTAL_DIFFICULTY: 231707791542740786049188744689299064356246512 +# By default, don't use these params +TERMINAL_BLOCK_HASH: 0x0000000000000000000000000000000000000000000000000000000000000000 +TERMINAL_BLOCK_HASH_ACTIVATION_EPOCH: 18446744073709551615 + # Genesis +# --------------------------------------------------------------- +# *CUSTOM MIN_GENESIS_ACTIVE_VALIDATOR_COUNT: 6000 # 10 October 2022 10:00:00 GMT+0000 MIN_GENESIS_TIME: 1665396000 -GENESIS_DELAY: 300 - -# Projected time: 2022-11-04T15:00:00.000Z, block: 680928 -TERMINAL_TOTAL_DIFFICULTY: 231707791542740786049188744689299064356246512 - -# Deposit contract -# --------------------------------------------------------------- -# NOTE: Don't use a value too high, or Teku rejects it (4294906129 NOK) -DEPOSIT_CHAIN_ID: 10200 -DEPOSIT_NETWORK_ID: 10200 -DEPOSIT_CONTRACT_ADDRESS: 0xb97036A26259B7147018913bD58a774cf91acf25 - -# Misc -# --------------------------------------------------------------- -# 2**6 (= 64) -MAX_COMMITTEES_PER_SLOT: 64 -# 2**7 (= 128) -TARGET_COMMITTEE_SIZE: 128 -# 2**11 (= 2,048) -MAX_VALIDATORS_PER_COMMITTEE: 2048 -# 2**2 (= 4) -MIN_PER_EPOCH_CHURN_LIMIT: 4 -# 2**3 (= 8) -MAX_PER_EPOCH_ACTIVATION_CHURN_LIMIT: 8 -# 2**12 (= 4096) -CHURN_LIMIT_QUOTIENT: 4096 -# See issue 563 -SHUFFLE_ROUND_COUNT: 90 -# 4 -HYSTERESIS_QUOTIENT: 4 -# 1 (minus 0.25) -HYSTERESIS_DOWNWARD_MULTIPLIER: 1 -# 5 (plus 1.25) -HYSTERESIS_UPWARD_MULTIPLIER: 5 -# Validator -# --------------------------------------------------------------- -# 2**10 (= 1024) ~1.4 hour -ETH1_FOLLOW_DISTANCE: 1024 -# 2**4 (= 16) -TARGET_AGGREGATORS_PER_COMMITTEE: 16 -# 2**0 (= 1) -RANDOM_SUBNETS_PER_VALIDATOR: 1 -# 2**8 (= 256) -EPOCHS_PER_RANDOM_SUBNET_SUBSCRIPTION: 256 -# 6 (estimate from xDai mainnet) -SECONDS_PER_ETH1_BLOCK: 6 - -# Gwei values -# --------------------------------------------------------------- -# 2**0 * 10**9 (= 1,000,000,000) Gwei -MIN_DEPOSIT_AMOUNT: 1000000000 -# 2**5 * 10**9 (= 32,000,000,000) Gwei -MAX_EFFECTIVE_BALANCE: 32000000000 -# 2**4 * 10**9 (= 16,000,000,000) Gwei -EJECTION_BALANCE: 16000000000 -# 2**0 * 10**9 (= 1,000,000,000) Gwei -EFFECTIVE_BALANCE_INCREMENT: 1000000000 -# Initial values -# --------------------------------------------------------------- # GBC area code GENESIS_FORK_VERSION: 0x0000006f -BLS_WITHDRAWAL_PREFIX: 0x00 -# Time parameters -# --------------------------------------------------------------- -# 5 seconds -SECONDS_PER_SLOT: 5 -# 2**0 (= 1) slots 12 seconds -MIN_ATTESTATION_INCLUSION_DELAY: 1 -# 2**4 (= 16) slots 1.87 minutes -SLOTS_PER_EPOCH: 16 -# 2**0 (= 1) epochs 1.87 minutes -MIN_SEED_LOOKAHEAD: 1 -# 2**2 (= 4) epochs 7.47 minutes -MAX_SEED_LOOKAHEAD: 4 -# 2**6 (= 64) epochs ~2 hours -EPOCHS_PER_ETH1_VOTING_PERIOD: 64 -# 2**13 (= 8,192) slots ~15.9 hours -SLOTS_PER_HISTORICAL_ROOT: 8192 -# 2**8 (= 256) epochs ~8 hours -MIN_VALIDATOR_WITHDRAWABILITY_DELAY: 256 -# 2**8 (= 256) epochs ~8 hours -SHARD_COMMITTEE_PERIOD: 256 -# 2**2 (= 4) epochs 7.47 minutes -MIN_EPOCHS_TO_INACTIVITY_PENALTY: 4 +# *CUSTOM +GENESIS_DELAY: 300 -# State vector lengths + +# Forking # --------------------------------------------------------------- -# 2**16 (= 65,536) epochs ~85 days -EPOCHS_PER_HISTORICAL_VECTOR: 65536 -# 2**13 (= 8,192) epochs ~10.6 days -EPOCHS_PER_SLASHINGS_VECTOR: 8192 -# 2**24 (= 16,777,216) historical roots, ~15,243 years -HISTORICAL_ROOTS_LIMIT: 16777216 -# 2**40 (= 1,099,511,627,776) validator spots -VALIDATOR_REGISTRY_LIMIT: 1099511627776 -# Reward and penalty quotients -# --------------------------------------------------------------- -# 25 -BASE_REWARD_FACTOR: 25 -# 2**9 (= 512) -WHISTLEBLOWER_REWARD_QUOTIENT: 512 -# 2**3 (= 8) -PROPOSER_REWARD_QUOTIENT: 8 -# 2**26 (= 67,108,864) -INACTIVITY_PENALTY_QUOTIENT: 67108864 -# 2**7 (= 128) (lower safety margin at Phase 0 genesis) -MIN_SLASHING_PENALTY_QUOTIENT: 128 -# 1 (lower safety margin at Phase 0 genesis) -PROPORTIONAL_SLASHING_MULTIPLIER: 1 -# Max operations per block -# --------------------------------------------------------------- -# 2**4 (= 16) -MAX_PROPOSER_SLASHINGS: 16 -# 2**1 (= 2) -MAX_ATTESTER_SLASHINGS: 2 -# 2**7 (= 128) -MAX_ATTESTATIONS: 128 -# 2**4 (= 16) -MAX_DEPOSITS: 16 -# 2**4 (= 16) -MAX_VOLUNTARY_EXITS: 16 -# Signature domains -# --------------------------------------------------------------- -DOMAIN_BEACON_PROPOSER: 0x00000000 -DOMAIN_BEACON_ATTESTER: 0x01000000 -DOMAIN_RANDAO: 0x02000000 -DOMAIN_DEPOSIT: 0x03000000 -DOMAIN_VOLUNTARY_EXIT: 0x04000000 -DOMAIN_SELECTION_PROOF: 0x05000000 -DOMAIN_AGGREGATE_AND_PROOF: 0x06000000 -DOMAIN_SYNC_COMMITTEE: 0x07000000 -DOMAIN_SYNC_COMMITTEE_SELECTION_PROOF: 0x08000000 -DOMAIN_CONTRIBUTION_AND_PROOF: 0x09000000 +# Some forks are disabled for now: +# - These may be re-assigned to another fork-version later +# - Temporarily set to max uint64 value: 2**64 - 1 # Altair ALTAIR_FORK_VERSION: 0x0100006f @@ -150,7 +43,95 @@ BELLATRIX_FORK_EPOCH: 180 # Mon Oct 10 2022 14:00:00 GMT+0000 # Capella CAPELLA_FORK_VERSION: 0x0300006f CAPELLA_FORK_EPOCH: 244224 # Wed May 24 2023 13:12:00 GMT+0000 +# Deneb +DENEB_FORK_VERSION: 0x0400006f +DENEB_FORK_EPOCH: 516608 # Wed Jan 31 2024 18:15:40 GMT+0000 + +# Time parameters +# --------------------------------------------------------------- +# 5 seconds +SECONDS_PER_SLOT: 5 +# 6 (estimate from xDai mainnet) +SECONDS_PER_ETH1_BLOCK: 6 +# 2**8 (= 256) epochs ~5.7 hours +MIN_VALIDATOR_WITHDRAWABILITY_DELAY: 256 +# 2**8 (= 256) epochs ~5.7 hours +SHARD_COMMITTEE_PERIOD: 256 +# 2**10 (= 1024) ~1.4 hour +ETH1_FOLLOW_DISTANCE: 1024 + + +# Validator cycle +# --------------------------------------------------------------- +# 2**2 (= 4) INACTIVITY_SCORE_BIAS: 4 # 2**4 (= 16) INACTIVITY_SCORE_RECOVERY_RATE: 16 +# 2**4 * 10**9 (= 16,000,000,000) Gwei +EJECTION_BALANCE: 16000000000 +# 2**2 (= 4) +MIN_PER_EPOCH_CHURN_LIMIT: 4 +# 2**12 (= 4096) +CHURN_LIMIT_QUOTIENT: 4096 +# [New in Deneb:EIP7514] 2* +MAX_PER_EPOCH_ACTIVATION_CHURN_LIMIT: 2 + +# Fork choice +# --------------------------------------------------------------- +# 40% +PROPOSER_SCORE_BOOST: 40 +# 20% +REORG_HEAD_WEIGHT_THRESHOLD: 20 +# 160% +REORG_PARENT_WEIGHT_THRESHOLD: 160 +# `2` epochs +REORG_MAX_EPOCHS_SINCE_FINALIZATION: 2 + + +# Deposit contract +# --------------------------------------------------------------- +# xDai Mainnet +DEPOSIT_CHAIN_ID: 10200 +DEPOSIT_NETWORK_ID: 10200 +DEPOSIT_CONTRACT_ADDRESS: 0xb97036A26259B7147018913bD58a774cf91acf25 + +# Networking +# --------------------------------------------------------------- +# `10 * 2**20` (= 10485760, 10 MiB) +GOSSIP_MAX_SIZE: 10485760 +# `2**10` (= 1024) +MAX_REQUEST_BLOCKS: 1024 +# `2**8` (= 256) +EPOCHS_PER_SUBNET_SUBSCRIPTION: 256 +# 33024, ~31 days +MIN_EPOCHS_FOR_BLOCK_REQUESTS: 33024 +# `10 * 2**20` (=10485760, 10 MiB) +MAX_CHUNK_SIZE: 10485760 +# 5s +TTFB_TIMEOUT: 5 +# 10s +RESP_TIMEOUT: 10 +ATTESTATION_PROPAGATION_SLOT_RANGE: 32 +# 500ms +MAXIMUM_GOSSIP_CLOCK_DISPARITY: 500 +MESSAGE_DOMAIN_INVALID_SNAPPY: 0x00000000 +MESSAGE_DOMAIN_VALID_SNAPPY: 0x01000000 +# 2 subnets per node +SUBNETS_PER_NODE: 2 +# 2**8 (= 64) +ATTESTATION_SUBNET_COUNT: 64 +ATTESTATION_SUBNET_EXTRA_BITS: 0 +# ceillog2(ATTESTATION_SUBNET_COUNT) + ATTESTATION_SUBNET_EXTRA_BITS +ATTESTATION_SUBNET_PREFIX_BITS: 6 + +# Deneb +# `2**7` (=128) +MAX_REQUEST_BLOCKS_DENEB: 128 +# MAX_REQUEST_BLOCKS_DENEB * MAX_BLOBS_PER_BLOCK +MAX_REQUEST_BLOB_SIDECARS: 768 +# `2**14` (= 16384 epochs, ~15 days) +MIN_EPOCHS_FOR_BLOB_SIDECARS_REQUESTS: 16384 +# `6` +BLOB_SIDECAR_SUBNET_COUNT: 6 + diff --git a/common/eth2_network_config/built_in_network_configs/holesky/config.yaml b/common/eth2_network_config/built_in_network_configs/holesky/config.yaml index d699b9a39f..0bb72ebd88 100644 --- a/common/eth2_network_config/built_in_network_configs/holesky/config.yaml +++ b/common/eth2_network_config/built_in_network_configs/holesky/config.yaml @@ -33,6 +33,10 @@ TERMINAL_BLOCK_HASH_ACTIVATION_EPOCH: 18446744073709551615 CAPELLA_FORK_VERSION: 0x04017000 CAPELLA_FORK_EPOCH: 256 +# Deneb +DENEB_FORK_VERSION: 0x05017000 +DENEB_FORK_EPOCH: 29696 + # Time parameters # --------------------------------------------------------------- # 12 seconds @@ -57,10 +61,10 @@ INACTIVITY_SCORE_RECOVERY_RATE: 16 EJECTION_BALANCE: 28000000000 # 2**2 (= 4) MIN_PER_EPOCH_CHURN_LIMIT: 4 -# 2**3 (= 8) -MAX_PER_EPOCH_ACTIVATION_CHURN_LIMIT: 8 # 2**16 (= 65,536) CHURN_LIMIT_QUOTIENT: 65536 +# [New in Deneb:EIP7514] 2**3 (= 8) +MAX_PER_EPOCH_ACTIVATION_CHURN_LIMIT: 8 # Fork choice # --------------------------------------------------------------- diff --git a/common/eth2_network_config/built_in_network_configs/sepolia/config.yaml b/common/eth2_network_config/built_in_network_configs/sepolia/config.yaml index b3dbb8a115..33a5ccb3fe 100644 --- a/common/eth2_network_config/built_in_network_configs/sepolia/config.yaml +++ b/common/eth2_network_config/built_in_network_configs/sepolia/config.yaml @@ -33,12 +33,8 @@ CAPELLA_FORK_VERSION: 0x90000072 CAPELLA_FORK_EPOCH: 56832 # Deneb -DENEB_FORK_VERSION: 0x03001020 -DENEB_FORK_EPOCH: 18446744073709551615 - -# Sharding -SHARDING_FORK_VERSION: 0x04001020 -SHARDING_FORK_EPOCH: 18446744073709551615 +DENEB_FORK_VERSION: 0x90000073 +DENEB_FORK_EPOCH: 132608 # Time parameters # --------------------------------------------------------------- @@ -64,11 +60,10 @@ INACTIVITY_SCORE_RECOVERY_RATE: 16 EJECTION_BALANCE: 16000000000 # 2**2 (= 4) MIN_PER_EPOCH_CHURN_LIMIT: 4 -# 2**3 (= 8) -MAX_PER_EPOCH_ACTIVATION_CHURN_LIMIT: 8 # 2**16 (= 65,536) CHURN_LIMIT_QUOTIENT: 65536 - +# [New in Deneb:EIP7514] 2**3 (= 8) +MAX_PER_EPOCH_ACTIVATION_CHURN_LIMIT: 8 # Fork choice # --------------------------------------------------------------- @@ -81,16 +76,41 @@ DEPOSIT_CHAIN_ID: 11155111 DEPOSIT_NETWORK_ID: 11155111 DEPOSIT_CONTRACT_ADDRESS: 0x7f02C3E3c98b133055B8B348B2Ac625669Ed295D -# Network +# Networking # --------------------------------------------------------------- -SUBNETS_PER_NODE: 2 +# `10 * 2**20` (= 10485760, 10 MiB) GOSSIP_MAX_SIZE: 10485760 +# `2**10` (= 1024) +MAX_REQUEST_BLOCKS: 1024 +# `2**8` (= 256) +EPOCHS_PER_SUBNET_SUBSCRIPTION: 256 +# `MIN_VALIDATOR_WITHDRAWABILITY_DELAY + CHURN_LIMIT_QUOTIENT // 2` (= 33024, ~5 months) MIN_EPOCHS_FOR_BLOCK_REQUESTS: 33024 +# `10 * 2**20` (=10485760, 10 MiB) MAX_CHUNK_SIZE: 10485760 +# 5s TTFB_TIMEOUT: 5 +# 10s RESP_TIMEOUT: 10 +ATTESTATION_PROPAGATION_SLOT_RANGE: 32 +# 500ms +MAXIMUM_GOSSIP_CLOCK_DISPARITY: 500 MESSAGE_DOMAIN_INVALID_SNAPPY: 0x00000000 MESSAGE_DOMAIN_VALID_SNAPPY: 0x01000000 +# 2 subnets per node +SUBNETS_PER_NODE: 2 +# 2**8 (= 64) ATTESTATION_SUBNET_COUNT: 64 ATTESTATION_SUBNET_EXTRA_BITS: 0 +# ceillog2(ATTESTATION_SUBNET_COUNT) + ATTESTATION_SUBNET_EXTRA_BITS ATTESTATION_SUBNET_PREFIX_BITS: 6 + +# Deneb +# `2**7` (=128) +MAX_REQUEST_BLOCKS_DENEB: 128 +# MAX_REQUEST_BLOCKS_DENEB * MAX_BLOBS_PER_BLOCK +MAX_REQUEST_BLOB_SIDECARS: 768 +# `2**12` (= 4096 epochs, ~18 days) +MIN_EPOCHS_FOR_BLOB_SIDECARS_REQUESTS: 4096 +# `6` +BLOB_SIDECAR_SUBNET_COUNT: 6 From 9a630e4dbbd3f9d235b89e01d6310799885bd8f2 Mon Sep 17 00:00:00 2001 From: ethDreamer <37123614+ethDreamer@users.noreply.github.com> Date: Mon, 22 Jan 2024 05:35:06 +0100 Subject: [PATCH 14/25] Stop Penalizing Peers in Parent SingleBlobLookup (#5096) * Stop Penalizing Peers in Parent SingleBlobLookup * Add test for parent lookup bug (#13) --------- Co-authored-by: realbigsean --- .../sync/block_lookups/single_block_lookup.rs | 4 + .../network/src/sync/block_lookups/tests.rs | 202 ++++++++++++------ 2 files changed, 137 insertions(+), 69 deletions(-) diff --git a/beacon_node/network/src/sync/block_lookups/single_block_lookup.rs b/beacon_node/network/src/sync/block_lookups/single_block_lookup.rs index 4e29816294..59931fadd7 100644 --- a/beacon_node/network/src/sync/block_lookups/single_block_lookup.rs +++ b/beacon_node/network/src/sync/block_lookups/single_block_lookup.rs @@ -92,6 +92,10 @@ impl SingleBlockLookup { self.block_request_state.requested_block_root = block_root; self.block_request_state.state.state = State::AwaitingDownload; self.blob_request_state.state.state = State::AwaitingDownload; + self.block_request_state.state.component_downloaded = false; + self.blob_request_state.state.component_downloaded = false; + self.block_request_state.state.component_processed = false; + self.blob_request_state.state.component_processed = false; self.child_components = Some(ChildComponents::empty(block_root)); } diff --git a/beacon_node/network/src/sync/block_lookups/tests.rs b/beacon_node/network/src/sync/block_lookups/tests.rs index 83f0b26156..c506696b9d 100644 --- a/beacon_node/network/src/sync/block_lookups/tests.rs +++ b/beacon_node/network/src/sync/block_lookups/tests.rs @@ -1157,6 +1157,7 @@ mod deneb_only { use crate::sync::block_lookups::common::ResponseType; use beacon_chain::data_availability_checker::AvailabilityCheckError; use beacon_chain::test_utils::NumBlobs; + use ssz_types::VariableList; use std::ops::IndexMut; use std::str::FromStr; @@ -1164,10 +1165,12 @@ mod deneb_only { bl: BlockLookups, cx: SyncNetworkContext, rig: TestRig, - block: Option>>, + block: Arc>, blobs: Vec>>, - parent_block: Option>>, - parent_blobs: Vec>>, + parent_block: VecDeque>>, + parent_blobs: VecDeque>>>, + unknown_parent_block: Option>>, + unknown_parent_blobs: Option>>>, peer_id: PeerId, block_req_id: Option, parent_block_req_id: Option, @@ -1179,8 +1182,18 @@ mod deneb_only { enum RequestTrigger { AttestationUnknownBlock, - GossipUnknownParentBlock, - GossipUnknownParentBlob, + GossipUnknownParentBlock { num_parents: usize }, + GossipUnknownParentBlob { num_parents: usize }, + } + + impl RequestTrigger { + fn num_parents(&self) -> usize { + match self { + RequestTrigger::AttestationUnknownBlock => 0, + RequestTrigger::GossipUnknownParentBlock { num_parents } => *num_parents, + RequestTrigger::GossipUnknownParentBlob { num_parents } => *num_parents, + } + } } impl DenebTester { @@ -1194,14 +1207,33 @@ mod deneb_only { E::slots_per_epoch() * rig.harness.spec.deneb_fork_epoch.unwrap().as_u64(), ); let (block, blobs) = rig.rand_block_and_blobs(fork_name, NumBlobs::Random); - let block = Arc::new(block); - let slot = block.slot(); - let mut block_root = block.canonical_root(); - let mut block = Some(block); + let mut block = Arc::new(block); let mut blobs = blobs.into_iter().map(Arc::new).collect::>(); + let slot = block.slot(); - let mut parent_block = None; - let mut parent_blobs = vec![]; + let num_parents = request_trigger.num_parents(); + let mut parent_block_chain = VecDeque::with_capacity(num_parents); + let mut parent_blobs_chain = VecDeque::with_capacity(num_parents); + for _ in 0..num_parents { + // Set the current block as the parent. + let parent_root = block.canonical_root(); + let parent_block = block.clone(); + let parent_blobs = blobs.clone(); + parent_block_chain.push_front(parent_block); + parent_blobs_chain.push_front(parent_blobs); + + // Create the next block. + let (child_block, child_blobs) = + rig.block_with_parent_and_blobs(parent_root, get_fork_name(), NumBlobs::Random); + let mut child_block = Arc::new(child_block); + let mut child_blobs = child_blobs.into_iter().map(Arc::new).collect::>(); + + // Update the new block to the current block. + std::mem::swap(&mut child_block, &mut block); + std::mem::swap(&mut child_blobs, &mut blobs); + } + let block_root = block.canonical_root(); + let parent_root = block.parent_root(); let peer_id = PeerId::random(); @@ -1214,31 +1246,17 @@ mod deneb_only { let blob_req_id = rig.expect_lookup_request(ResponseType::Blob); (Some(block_req_id), Some(blob_req_id), None, None) } - RequestTrigger::GossipUnknownParentBlock => { - let (child_block, child_blobs) = rig.block_with_parent_and_blobs( - block_root, - get_fork_name(), - NumBlobs::Random, - ); - parent_block = Some(Arc::new(child_block)); - parent_blobs = child_blobs.into_iter().map(Arc::new).collect::>(); - std::mem::swap(&mut parent_block, &mut block); - std::mem::swap(&mut parent_blobs, &mut blobs); - - let child_block = block.as_ref().expect("block").clone(); - let child_root = child_block.canonical_root(); - let parent_root = block_root; - block_root = child_root; + RequestTrigger::GossipUnknownParentBlock { .. } => { bl.search_child_block( - child_root, - ChildComponents::new(child_root, Some(child_block), None), + block_root, + ChildComponents::new(block_root, Some(block.clone()), None), &[peer_id], &mut cx, ); let blob_req_id = rig.expect_lookup_request(ResponseType::Blob); rig.expect_empty_network(); // expect no block request - bl.search_parent(slot, child_root, parent_root, peer_id, &mut cx); + bl.search_parent(slot, block_root, parent_root, peer_id, &mut cx); let parent_block_req_id = rig.expect_parent_request(ResponseType::Block); let parent_blob_req_id = rig.expect_parent_request(ResponseType::Blob); ( @@ -1248,28 +1266,15 @@ mod deneb_only { Some(parent_blob_req_id), ) } - RequestTrigger::GossipUnknownParentBlob => { - let (child_block, child_blobs) = rig.block_with_parent_and_blobs( - block_root, - get_fork_name(), - NumBlobs::Random, - ); + RequestTrigger::GossipUnknownParentBlob { .. } => { + let single_blob = blobs.first().cloned().unwrap(); + let child_root = single_blob.block_root(); - parent_block = Some(Arc::new(child_block)); - parent_blobs = child_blobs.into_iter().map(Arc::new).collect::>(); - std::mem::swap(&mut parent_block, &mut block); - std::mem::swap(&mut parent_blobs, &mut blobs); - - let child_blob = blobs.first().cloned().unwrap(); - let parent_root = block_root; - let child_root = child_blob.block_root(); - block_root = child_root; - - let mut blobs = FixedBlobSidecarList::default(); - *blobs.index_mut(0) = Some(child_blob); + let mut lookup_blobs = FixedBlobSidecarList::default(); + *lookup_blobs.index_mut(0) = Some(single_blob); bl.search_child_block( child_root, - ChildComponents::new(child_root, None, Some(blobs)), + ChildComponents::new(child_root, None, Some(lookup_blobs)), &[peer_id], &mut cx, ); @@ -1295,8 +1300,10 @@ mod deneb_only { rig, block, blobs, - parent_block, - parent_blobs, + parent_block: parent_block_chain, + parent_blobs: parent_blobs_chain, + unknown_parent_block: None, + unknown_parent_blobs: None, peer_id, block_req_id, parent_block_req_id, @@ -1309,10 +1316,12 @@ mod deneb_only { fn parent_block_response(mut self) -> Self { self.rig.expect_empty_network(); + let block = self.parent_block.pop_front().unwrap().clone(); + let _ = self.unknown_parent_block.insert(block.clone()); self.bl.parent_lookup_response::>( self.parent_block_req_id.expect("parent request id"), self.peer_id, - self.parent_block.clone(), + Some(block), D, &self.cx, ); @@ -1322,7 +1331,9 @@ mod deneb_only { } fn parent_blob_response(mut self) -> Self { - for blob in &self.parent_blobs { + let blobs = self.parent_blobs.pop_front().unwrap(); + let _ = self.unknown_parent_blobs.insert(blobs.clone()); + for blob in &blobs { self.bl .parent_lookup_response::>( self.parent_blob_req_id.expect("parent blob request id"), @@ -1361,7 +1372,7 @@ mod deneb_only { .single_lookup_response::>( self.block_req_id.expect("block request id"), self.peer_id, - self.block.clone(), + Some(self.block.clone()), D, &self.cx, ); @@ -1483,12 +1494,16 @@ mod deneb_only { } fn parent_block_unknown_parent(mut self) -> Self { + let block = self.unknown_parent_block.take().unwrap(); + let block = RpcBlock::new( + Some(block.canonical_root()), + block, + self.unknown_parent_blobs.take().map(VariableList::from), + ) + .unwrap(); self.bl.parent_block_processed( self.block_root, - BlockProcessingResult::Err(BlockError::ParentUnknown(RpcBlock::new_without_blobs( - Some(self.block_root), - self.parent_block.clone().expect("parent block"), - ))), + BlockProcessingResult::Err(BlockError::ParentUnknown(block)), &mut self.cx, ); assert_eq!(self.bl.parent_lookups.len(), 1); @@ -1805,7 +1820,9 @@ mod deneb_only { #[test] fn parent_block_unknown_parent() { - let Some(tester) = DenebTester::new(RequestTrigger::GossipUnknownParentBlock) else { + let Some(tester) = + DenebTester::new(RequestTrigger::GossipUnknownParentBlock { num_parents: 1 }) + else { return; }; @@ -1823,7 +1840,9 @@ mod deneb_only { #[test] fn parent_block_invalid_parent() { - let Some(tester) = DenebTester::new(RequestTrigger::GossipUnknownParentBlock) else { + let Some(tester) = + DenebTester::new(RequestTrigger::GossipUnknownParentBlock { num_parents: 1 }) + else { return; }; @@ -1842,7 +1861,9 @@ mod deneb_only { #[test] fn parent_block_and_blob_lookup_parent_returned_first() { - let Some(tester) = DenebTester::new(RequestTrigger::GossipUnknownParentBlock) else { + let Some(tester) = + DenebTester::new(RequestTrigger::GossipUnknownParentBlock { num_parents: 1 }) + else { return; }; @@ -1857,7 +1878,9 @@ mod deneb_only { #[test] fn parent_block_and_blob_lookup_child_returned_first() { - let Some(tester) = DenebTester::new(RequestTrigger::GossipUnknownParentBlock) else { + let Some(tester) = + DenebTester::new(RequestTrigger::GossipUnknownParentBlock { num_parents: 1 }) + else { return; }; @@ -1875,7 +1898,9 @@ mod deneb_only { #[test] fn empty_parent_block_then_parent_blob() { - let Some(tester) = DenebTester::new(RequestTrigger::GossipUnknownParentBlock) else { + let Some(tester) = + DenebTester::new(RequestTrigger::GossipUnknownParentBlock { num_parents: 1 }) + else { return; }; @@ -1895,7 +1920,9 @@ mod deneb_only { #[test] fn empty_parent_blobs_then_parent_block() { - let Some(tester) = DenebTester::new(RequestTrigger::GossipUnknownParentBlock) else { + let Some(tester) = + DenebTester::new(RequestTrigger::GossipUnknownParentBlock { num_parents: 1 }) + else { return; }; @@ -1916,7 +1943,9 @@ mod deneb_only { #[test] fn parent_blob_unknown_parent() { - let Some(tester) = DenebTester::new(RequestTrigger::GossipUnknownParentBlob) else { + let Some(tester) = + DenebTester::new(RequestTrigger::GossipUnknownParentBlob { num_parents: 1 }) + else { return; }; @@ -1934,7 +1963,9 @@ mod deneb_only { #[test] fn parent_blob_invalid_parent() { - let Some(tester) = DenebTester::new(RequestTrigger::GossipUnknownParentBlob) else { + let Some(tester) = + DenebTester::new(RequestTrigger::GossipUnknownParentBlob { num_parents: 1 }) + else { return; }; @@ -1953,7 +1984,9 @@ mod deneb_only { #[test] fn parent_block_and_blob_lookup_parent_returned_first_blob_trigger() { - let Some(tester) = DenebTester::new(RequestTrigger::GossipUnknownParentBlob) else { + let Some(tester) = + DenebTester::new(RequestTrigger::GossipUnknownParentBlob { num_parents: 1 }) + else { return; }; @@ -1968,7 +2001,9 @@ mod deneb_only { #[test] fn parent_block_and_blob_lookup_child_returned_first_blob_trigger() { - let Some(tester) = DenebTester::new(RequestTrigger::GossipUnknownParentBlob) else { + let Some(tester) = + DenebTester::new(RequestTrigger::GossipUnknownParentBlob { num_parents: 1 }) + else { return; }; @@ -1986,7 +2021,9 @@ mod deneb_only { #[test] fn empty_parent_block_then_parent_blob_blob_trigger() { - let Some(tester) = DenebTester::new(RequestTrigger::GossipUnknownParentBlob) else { + let Some(tester) = + DenebTester::new(RequestTrigger::GossipUnknownParentBlob { num_parents: 1 }) + else { return; }; @@ -2006,7 +2043,9 @@ mod deneb_only { #[test] fn empty_parent_blobs_then_parent_block_blob_trigger() { - let Some(tester) = DenebTester::new(RequestTrigger::GossipUnknownParentBlob) else { + let Some(tester) = + DenebTester::new(RequestTrigger::GossipUnknownParentBlob { num_parents: 1 }) + else { return; }; @@ -2024,4 +2063,29 @@ mod deneb_only { .parent_block_imported() .expect_parent_chain_process(); } + + #[test] + fn parent_blob_unknown_parent_chain() { + let Some(tester) = + DenebTester::new(RequestTrigger::GossipUnknownParentBlob { num_parents: 2 }) + else { + return; + }; + + tester + .block_response() + .expect_empty_beacon_processor() + .parent_block_response() + .parent_blob_response() + .expect_no_penalty() + .expect_block_process() + .parent_block_unknown_parent() + .expect_parent_block_request() + .expect_parent_blobs_request() + .expect_empty_beacon_processor() + .parent_block_response() + .parent_blob_response() + .expect_no_penalty() + .expect_block_process(); + } } From 712f5aba73a135a8d2bde605222cca332bfd8688 Mon Sep 17 00:00:00 2001 From: ethDreamer <37123614+ethDreamer@users.noreply.github.com> Date: Tue, 23 Jan 2024 11:16:36 +0800 Subject: [PATCH 15/25] Upgrade shlex to 1.3.0 (#5108) --- Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7874e1ec3d..25766f86ab 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7149,9 +7149,9 @@ dependencies = [ [[package]] name = "shlex" -version = "1.2.0" +version = "1.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a7cee0529a6d40f580e7a5e6c495c8fbfe21b7b52795ed4bb5e62cdf92bc6380" +checksum = "0fda2ff0d084019ba4d7c6f371c95d8fd75ce3524c3cb8fb653a3023f6323e64" [[package]] name = "signal-hook-registry" From 02d1f360908701945ae4f861761434f56642c277 Mon Sep 17 00:00:00 2001 From: ethDreamer <37123614+ethDreamer@users.noreply.github.com> Date: Tue, 23 Jan 2024 12:08:57 +0800 Subject: [PATCH 16/25] Small Readability Improvement in Networking Code (#5098) --- .../network/src/sync/block_lookups/common.rs | 3 +-- .../sync/block_lookups/single_block_lookup.rs | 17 ++++++++--------- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/beacon_node/network/src/sync/block_lookups/common.rs b/beacon_node/network/src/sync/block_lookups/common.rs index 78b10473df..d989fbb336 100644 --- a/beacon_node/network/src/sync/block_lookups/common.rs +++ b/beacon_node/network/src/sync/block_lookups/common.rs @@ -101,11 +101,10 @@ pub trait RequestState { fn build_request_and_send( &mut self, id: Id, - already_downloaded: bool, cx: &SyncNetworkContext, ) -> Result<(), LookupRequestError> { // Check if request is necessary. - if already_downloaded || !matches!(self.get_state().state, State::AwaitingDownload) { + if !matches!(self.get_state().state, State::AwaitingDownload) { return Ok(()); } diff --git a/beacon_node/network/src/sync/block_lookups/single_block_lookup.rs b/beacon_node/network/src/sync/block_lookups/single_block_lookup.rs index 59931fadd7..8c60621f1c 100644 --- a/beacon_node/network/src/sync/block_lookups/single_block_lookup.rs +++ b/beacon_node/network/src/sync/block_lookups/single_block_lookup.rs @@ -114,19 +114,18 @@ impl SingleBlockLookup { &mut self, cx: &SyncNetworkContext, ) -> Result<(), LookupRequestError> { - let block_root = self.block_root(); let block_already_downloaded = self.block_already_downloaded(); let blobs_already_downloaded = self.blobs_already_downloaded(); - if block_already_downloaded && blobs_already_downloaded { - trace!(cx.log, "Lookup request already completed"; "block_root"=> ?block_root); - return Ok(()); + if !block_already_downloaded { + self.block_request_state + .build_request_and_send(self.id, cx)?; } - let id = self.id; - self.block_request_state - .build_request_and_send(id, block_already_downloaded, cx)?; - self.blob_request_state - .build_request_and_send(id, blobs_already_downloaded, cx) + if !blobs_already_downloaded { + self.blob_request_state + .build_request_and_send(self.id, cx)?; + } + Ok(()) } /// Returns a `CachedChild`, which is a wrapper around a `RpcBlock` that is either: From a403138ed02f981143f41d53c24e2cae0af07baa Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Tue, 23 Jan 2024 15:32:07 +1100 Subject: [PATCH 17/25] Reduce size of futures in HTTP API to prevent stack overflows (#5104) * Box::pin a few big futures * Arc the blocks early in publication * Fix more tests --- .../beacon_chain/src/block_verification.rs | 2 +- .../overflow_lru_cache.rs | 2 +- beacon_node/beacon_chain/src/test_utils.rs | 26 +++++----- .../beacon_chain/tests/block_verification.rs | 20 ++------ .../tests/payload_invalidation.rs | 11 ++--- beacon_node/beacon_chain/tests/store_tests.rs | 10 ++-- beacon_node/http_api/src/lib.rs | 6 ++- beacon_node/http_api/src/publish_blocks.rs | 25 +++++----- .../tests/broadcast_validation_tests.rs | 48 ++++++++----------- .../http_api/tests/interactive_tests.rs | 2 +- beacon_node/http_api/tests/tests.rs | 12 ++--- .../src/network_beacon_processor/tests.rs | 2 +- common/eth2/src/types.rs | 42 +++++++++------- consensus/fork_choice/tests/tests.rs | 18 +++---- .../src/per_block_processing/tests.rs | 6 +-- testing/state_transition_vectors/src/exit.rs | 2 +- validator_client/src/block_service.rs | 5 +- 17 files changed, 116 insertions(+), 123 deletions(-) diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index 0cdf5b9fe9..e8df5b811e 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -722,7 +722,7 @@ impl IntoGossipVerifiedBlockContents for PublishBlockReq Ok::<_, BlockContentsError>(gossip_verified_blobs) }) .transpose()?; - let gossip_verified_block = GossipVerifiedBlock::new(Arc::new(block), chain)?; + let gossip_verified_block = GossipVerifiedBlock::new(block, chain)?; Ok((gossip_verified_block, gossip_verified_blobs)) } diff --git a/beacon_node/beacon_chain/src/data_availability_checker/overflow_lru_cache.rs b/beacon_node/beacon_chain/src/data_availability_checker/overflow_lru_cache.rs index 8b8368022a..34c9bc76f6 100644 --- a/beacon_node/beacon_chain/src/data_availability_checker/overflow_lru_cache.rs +++ b/beacon_node/beacon_chain/src/data_availability_checker/overflow_lru_cache.rs @@ -952,7 +952,7 @@ mod test { }; let availability_pending_block = AvailabilityPendingExecutedBlock { - block: Arc::new(block), + block, import_data, payload_verification_outcome, }; diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index 7a66a6bdb4..6b85c8e493 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -822,7 +822,7 @@ where slot: Slot, ) -> (SignedBlindedBeaconBlock, BeaconState) { let (unblinded, new_state) = self.make_block(state, slot).await; - (unblinded.0.into(), new_state) + ((*unblinded.0).clone().into(), new_state) } /// Returns a newly created block, signed by the proposer for the given slot. @@ -866,14 +866,14 @@ where panic!("Should always be a full payload response"); }; - let signed_block = block_response.block.sign( + let signed_block = Arc::new(block_response.block.sign( &self.validator_keypairs[proposer_index].sk, &block_response.state.fork(), block_response.state.genesis_validators_root(), &self.spec, - ); + )); - let block_contents: SignedBlockContentsTuple = match &signed_block { + let block_contents: SignedBlockContentsTuple = match *signed_block { SignedBeaconBlock::Base(_) | SignedBeaconBlock::Altair(_) | SignedBeaconBlock::Merge(_) @@ -928,14 +928,14 @@ where panic!("Should always be a full payload response"); }; - let signed_block = block_response.block.sign( + let signed_block = Arc::new(block_response.block.sign( &self.validator_keypairs[proposer_index].sk, &block_response.state.fork(), block_response.state.genesis_validators_root(), &self.spec, - ); + )); - let block_contents: SignedBlockContentsTuple = match &signed_block { + let block_contents: SignedBlockContentsTuple = match *signed_block { SignedBeaconBlock::Base(_) | SignedBeaconBlock::Altair(_) | SignedBeaconBlock::Merge(_) @@ -1742,7 +1742,7 @@ where let ((block, blobs), state) = self.make_block_return_pre_state(state, slot).await; - let (mut block, _) = block.deconstruct(); + let (mut block, _) = (*block).clone().deconstruct(); block_modifier(&mut block); @@ -1754,7 +1754,7 @@ where state.genesis_validators_root(), &self.spec, ); - ((signed_block, blobs), state) + ((Arc::new(signed_block), blobs), state) } pub async fn make_blob_with_modifier( @@ -1768,7 +1768,7 @@ where let ((block, mut blobs), state) = self.make_block_return_pre_state(state, slot).await; - let (block, _) = block.deconstruct(); + let (block, _) = (*block).clone().deconstruct(); blob_modifier(&mut blobs.as_mut().unwrap().1); @@ -1780,7 +1780,7 @@ where state.genesis_validators_root(), &self.spec, ); - ((signed_block, blobs), state) + ((Arc::new(signed_block), blobs), state) } pub fn make_deposits<'a>( @@ -1873,7 +1873,7 @@ where .chain .process_block( block_root, - RpcBlock::new(Some(block_root), Arc::new(block), sidecars).unwrap(), + RpcBlock::new(Some(block_root), block, sidecars).unwrap(), NotifyExecutionLayer::Yes, || Ok(()), ) @@ -1899,7 +1899,7 @@ where .chain .process_block( block_root, - RpcBlock::new(Some(block_root), Arc::new(block), sidecars).unwrap(), + RpcBlock::new(Some(block_root), block, sidecars).unwrap(), NotifyExecutionLayer::Yes, || Ok(()), ) diff --git a/beacon_node/beacon_chain/tests/block_verification.rs b/beacon_node/beacon_chain/tests/block_verification.rs index 4344013b3c..541e97436d 100644 --- a/beacon_node/beacon_chain/tests/block_verification.rs +++ b/beacon_node/beacon_chain/tests/block_verification.rs @@ -1142,11 +1142,7 @@ async fn verify_block_for_gossip_slashing_detection() { let ((block1, blobs1), _) = harness.make_block(state.clone(), Slot::new(1)).await; let ((block2, _blobs2), _) = harness.make_block(state, Slot::new(1)).await; - let verified_block = harness - .chain - .verify_block_for_gossip(Arc::new(block1)) - .await - .unwrap(); + let verified_block = harness.chain.verify_block_for_gossip(block1).await.unwrap(); if let Some((kzg_proofs, blobs)) = blobs1 { let sidecars = @@ -1174,12 +1170,7 @@ async fn verify_block_for_gossip_slashing_detection() { ) .await .unwrap(); - unwrap_err( - harness - .chain - .verify_block_for_gossip(Arc::new(block2)) - .await, - ); + unwrap_err(harness.chain.verify_block_for_gossip(block2).await); // Slasher should have been handed the two conflicting blocks and crafted a slashing. slasher.process_queued(Epoch::new(0)).unwrap(); @@ -1198,11 +1189,7 @@ async fn verify_block_for_gossip_doppelganger_detection() { let state = harness.get_current_state(); let ((block, _), _) = harness.make_block(state.clone(), Slot::new(1)).await; - let verified_block = harness - .chain - .verify_block_for_gossip(Arc::new(block)) - .await - .unwrap(); + let verified_block = harness.chain.verify_block_for_gossip(block).await.unwrap(); let attestations = verified_block.block.message().body().attestations().clone(); harness .chain @@ -1564,7 +1551,6 @@ async fn import_duplicate_block_unrealized_justification() { let slot = harness.get_current_slot(); let (block_contents, _) = harness.make_block(state.clone(), slot).await; let (block, _) = block_contents; - let block = Arc::new(block); let block_root = block.canonical_root(); // Create two verified variants of the block, representing the same block being processed in diff --git a/beacon_node/beacon_chain/tests/payload_invalidation.rs b/beacon_node/beacon_chain/tests/payload_invalidation.rs index 5076a6c1a9..a0b7fbd365 100644 --- a/beacon_node/beacon_chain/tests/payload_invalidation.rs +++ b/beacon_node/beacon_chain/tests/payload_invalidation.rs @@ -319,7 +319,7 @@ impl InvalidPayloadRig { .get_full_block(&block_root) .unwrap() .unwrap(), - block, + *block, "block from db must match block imported" ); } @@ -700,7 +700,7 @@ async fn invalidates_all_descendants() { .chain .process_block( fork_block.canonical_root(), - Arc::new(fork_block), + fork_block, NotifyExecutionLayer::Yes, || Ok(()), ) @@ -800,7 +800,7 @@ async fn switches_heads() { .chain .process_block( fork_block.canonical_root(), - Arc::new(fork_block), + fork_block, NotifyExecutionLayer::Yes, || Ok(()), ) @@ -1044,8 +1044,7 @@ async fn invalid_parent() { // Produce another block atop the parent, but don't import yet. let slot = parent_block.slot() + 1; rig.harness.set_current_slot(slot); - let (block_tuple, state) = rig.harness.make_block(parent_state, slot).await; - let block = Arc::new(block_tuple.0); + let ((block, _), state) = rig.harness.make_block(parent_state, slot).await; let block_root = block.canonical_root(); assert_eq!(block.parent_root(), parent_root); @@ -1865,7 +1864,7 @@ impl InvalidHeadSetup { .state_at_slot(slot - 1, StateSkipConfig::WithStateRoots) .unwrap(); let (fork_block_tuple, _) = rig.harness.make_block(parent_state, slot).await; - opt_fork_block = Some(Arc::new(fork_block_tuple.0)); + opt_fork_block = Some(fork_block_tuple.0); } else { // Skipped slot. }; diff --git a/beacon_node/beacon_chain/tests/store_tests.rs b/beacon_node/beacon_chain/tests/store_tests.rs index 28770b93e2..9b832bd764 100644 --- a/beacon_node/beacon_chain/tests/store_tests.rs +++ b/beacon_node/beacon_chain/tests/store_tests.rs @@ -2268,17 +2268,17 @@ async fn garbage_collect_temp_states_from_failed_block() { let block_slot = Slot::new(2 * slots_per_epoch); let ((signed_block, _), state) = harness.make_block(genesis_state, block_slot).await; - let (mut block, _) = signed_block.deconstruct(); + let (mut block, _) = (*signed_block).clone().deconstruct(); // Mutate the block to make it invalid, and re-sign it. *block.state_root_mut() = Hash256::repeat_byte(0xff); let proposer_index = block.proposer_index() as usize; - let block = block.sign( + let block = Arc::new(block.sign( &harness.validator_keypairs[proposer_index].sk, &state.fork(), state.genesis_validators_root(), &harness.spec, - ); + )); // The block should be rejected, but should store a bunch of temporary states. harness.set_current_slot(block_slot); @@ -2677,7 +2677,7 @@ async fn process_blocks_and_attestations_for_unaligned_checkpoint() { .chain .process_block( invalid_fork_block.canonical_root(), - Arc::new(invalid_fork_block.clone()), + invalid_fork_block.clone(), NotifyExecutionLayer::Yes, || Ok(()), ) @@ -2690,7 +2690,7 @@ async fn process_blocks_and_attestations_for_unaligned_checkpoint() { .chain .process_block( valid_fork_block.canonical_root(), - Arc::new(valid_fork_block.clone()), + valid_fork_block.clone(), NotifyExecutionLayer::Yes, || Ok(()), ) diff --git a/beacon_node/http_api/src/lib.rs b/beacon_node/http_api/src/lib.rs index 02ea9518c6..3777e61420 100644 --- a/beacon_node/http_api/src/lib.rs +++ b/beacon_node/http_api/src/lib.rs @@ -1410,7 +1410,7 @@ pub fn serve( .and(network_tx_filter.clone()) .and(log_filter.clone()) .then( - move |block_contents: SignedBlindedBeaconBlock, + move |block_contents: Arc>, task_spawner: TaskSpawner, chain: Arc>, network_tx: UnboundedSender>, @@ -1450,6 +1450,7 @@ pub fn serve( &block_bytes, &chain.spec, ) + .map(Arc::new) .map_err(|e| { warp_utils::reject::custom_bad_request(format!("invalid SSZ: {e:?}")) })?; @@ -1478,7 +1479,7 @@ pub fn serve( .and(log_filter.clone()) .then( move |validation_level: api_types::BroadcastValidationQuery, - blinded_block: SignedBlindedBeaconBlock, + blinded_block: Arc>, task_spawner: TaskSpawner, chain: Arc>, network_tx: UnboundedSender>, @@ -1519,6 +1520,7 @@ pub fn serve( &block_bytes, &chain.spec, ) + .map(Arc::new) .map_err(|e| { warp_utils::reject::custom_bad_request(format!("invalid SSZ: {e:?}")) })?; diff --git a/beacon_node/http_api/src/publish_blocks.rs b/beacon_node/http_api/src/publish_blocks.rs index 8b03771540..8b85c2ac95 100644 --- a/beacon_node/http_api/src/publish_blocks.rs +++ b/beacon_node/http_api/src/publish_blocks.rs @@ -194,7 +194,7 @@ pub async fn publish_block { info!( @@ -291,7 +290,7 @@ pub async fn publish_block( - blinded_block: SignedBlindedBeaconBlock, + blinded_block: Arc>, chain: Arc>, network_tx: &UnboundedSender>, log: Logger, @@ -319,7 +318,7 @@ pub async fn publish_blinded_block( pub async fn reconstruct_block( chain: Arc>, block_root: Hash256, - block: SignedBlindedBeaconBlock, + block: Arc>, log: Logger, ) -> Result>, Rejection> { let full_payload_opt = if let Ok(payload_header) = block.message().body().execution_payload() { @@ -380,6 +379,10 @@ pub async fn reconstruct_block( None }; + // Perf: cloning the block here to unblind it is a little sub-optimal. This is considered an + // acceptable tradeoff to avoid passing blocks around on the stack (unarced), which blows up + // the size of futures. + let block = (*block).clone(); match full_payload_opt { // A block without a payload is pre-merge and we consider it locally // built. diff --git a/beacon_node/http_api/tests/broadcast_validation_tests.rs b/beacon_node/http_api/tests/broadcast_validation_tests.rs index 2d42371a7c..6a3f7947e6 100644 --- a/beacon_node/http_api/tests/broadcast_validation_tests.rs +++ b/beacon_node/http_api/tests/broadcast_validation_tests.rs @@ -3,7 +3,7 @@ use beacon_chain::{ GossipVerifiedBlock, IntoGossipVerifiedBlockContents, }; use eth2::reqwest::StatusCode; -use eth2::types::{BroadcastValidation, PublishBlockRequest, SignedBeaconBlock}; +use eth2::types::{BroadcastValidation, PublishBlockRequest}; use http_api::test_utils::InteractiveTester; use http_api::{publish_blinded_block, publish_block, reconstruct_block, ProvenancedBlock}; use std::sync::Arc; @@ -63,7 +63,7 @@ pub async fn gossip_invalid() { tester.harness.advance_slot(); - let ((block, blobs), _): ((SignedBeaconBlock, _), _) = tester + let ((block, blobs), _) = tester .harness .make_block_with_modifier(chain_state_before, slot, |b| { *b.state_root_mut() = Hash256::zero(); @@ -115,7 +115,7 @@ pub async fn gossip_partial_pass() { tester.harness.advance_slot(); - let ((block, blobs), _): ((SignedBeaconBlock, _), _) = tester + let ((block, blobs), _) = tester .harness .make_block_with_modifier(chain_state_before, slot, |b| { *b.state_root_mut() = Hash256::random() @@ -161,8 +161,7 @@ pub async fn gossip_full_pass() { let slot_b = slot_a + 1; let state_a = tester.harness.get_current_state(); - let ((block, blobs), _): ((SignedBeaconBlock, _), _) = - tester.harness.make_block(state_a, slot_b).await; + let ((block, blobs), _) = tester.harness.make_block(state_a, slot_b).await; let response: Result<(), eth2::Error> = tester .client @@ -252,7 +251,7 @@ pub async fn consensus_invalid() { tester.harness.advance_slot(); - let ((block, blobs), _): ((SignedBeaconBlock, _), _) = tester + let ((block, blobs), _) = tester .harness .make_block_with_modifier(chain_state_before, slot, |b| { *b.state_root_mut() = Hash256::zero(); @@ -304,7 +303,7 @@ pub async fn consensus_gossip() { let slot_b = slot_a + 1; let state_a = tester.harness.get_current_state(); - let ((block, blobs), _): ((SignedBeaconBlock, _), _) = tester + let ((block, blobs), _) = tester .harness .make_block_with_modifier(state_a, slot_b, |b| *b.state_root_mut() = Hash256::zero()) .await; @@ -418,8 +417,7 @@ pub async fn consensus_full_pass() { let slot_b = slot_a + 1; let state_a = tester.harness.get_current_state(); - let ((block, blobs), _): ((SignedBeaconBlock, _), _) = - tester.harness.make_block(state_a, slot_b).await; + let ((block, blobs), _) = tester.harness.make_block(state_a, slot_b).await; let response: Result<(), eth2::Error> = tester .client @@ -465,7 +463,7 @@ pub async fn equivocation_invalid() { tester.harness.advance_slot(); - let ((block, blobs), _): ((SignedBeaconBlock, _), _) = tester + let ((block, blobs), _) = tester .harness .make_block_with_modifier(chain_state_before, slot, |b| { *b.state_root_mut() = Hash256::zero(); @@ -518,10 +516,9 @@ pub async fn equivocation_consensus_early_equivocation() { let slot_b = slot_a + 1; let state_a = tester.harness.get_current_state(); - let ((block_a, blobs_a), state_after_a): ((SignedBeaconBlock, _), _) = + let ((block_a, blobs_a), state_after_a) = tester.harness.make_block(state_a.clone(), slot_b).await; - let ((block_b, blobs_b), state_after_b): ((SignedBeaconBlock, _), _) = - tester.harness.make_block(state_a, slot_b).await; + let ((block_b, blobs_b), state_after_b) = tester.harness.make_block(state_a, slot_b).await; /* check for `make_block` curios */ assert_eq!(block_a.state_root(), state_after_a.tree_hash_root()); @@ -590,7 +587,7 @@ pub async fn equivocation_gossip() { let slot_b = slot_a + 1; let state_a = tester.harness.get_current_state(); - let ((block, blobs), _): ((SignedBeaconBlock, _), _) = tester + let ((block, blobs), _) = tester .harness .make_block_with_modifier(state_a, slot_b, |b| *b.state_root_mut() = Hash256::zero()) .await; @@ -645,10 +642,9 @@ pub async fn equivocation_consensus_late_equivocation() { let slot_b = slot_a + 1; let state_a = tester.harness.get_current_state(); - let ((block_a, blobs_a), state_after_a): ((SignedBeaconBlock, _), _) = + let ((block_a, blobs_a), state_after_a) = tester.harness.make_block(state_a.clone(), slot_b).await; - let ((block_b, blobs_b), state_after_b): ((SignedBeaconBlock, _), _) = - tester.harness.make_block(state_a, slot_b).await; + let ((block_b, blobs_b), state_after_b) = tester.harness.make_block(state_a, slot_b).await; /* check for `make_block` curios */ assert_eq!(block_a.state_root(), state_after_a.tree_hash_root()); @@ -716,8 +712,7 @@ pub async fn equivocation_full_pass() { let slot_b = slot_a + 1; let state_a = tester.harness.get_current_state(); - let ((block, blobs), _): ((SignedBeaconBlock, _), _) = - tester.harness.make_block(state_a, slot_b).await; + let ((block, blobs), _) = tester.harness.make_block(state_a, slot_b).await; let response: Result<(), eth2::Error> = tester .client @@ -1269,6 +1264,7 @@ pub async fn blinded_equivocation_consensus_late_equivocation() { .make_blinded_block(state_a.clone(), slot_b) .await; let (block_b, state_after_b) = tester.harness.make_blinded_block(state_a, slot_b).await; + let block_b = Arc::new(block_b); /* check for `make_blinded_block` curios */ assert_eq!(block_a.state_root(), state_after_a.tree_hash_root()); @@ -1278,7 +1274,7 @@ pub async fn blinded_equivocation_consensus_late_equivocation() { let unblinded_block_a = reconstruct_block( tester.harness.chain.clone(), block_a.canonical_root(), - block_a, + Arc::new(block_a), test_logger.clone(), ) .await @@ -1301,15 +1297,11 @@ pub async fn blinded_equivocation_consensus_late_equivocation() { ProvenancedBlock::Builder(b, _) => b, }; - let gossip_block_b = GossipVerifiedBlock::new( - Arc::new(inner_block_b.clone().deconstruct().0), - &tester.harness.chain, - ); + let gossip_block_b = + GossipVerifiedBlock::new(inner_block_b.clone().deconstruct().0, &tester.harness.chain); assert!(gossip_block_b.is_ok()); - let gossip_block_a = GossipVerifiedBlock::new( - Arc::new(inner_block_a.clone().deconstruct().0), - &tester.harness.chain, - ); + let gossip_block_a = + GossipVerifiedBlock::new(inner_block_a.clone().deconstruct().0, &tester.harness.chain); assert!(gossip_block_a.is_err()); let channel = tokio::sync::mpsc::unbounded_channel(); diff --git a/beacon_node/http_api/tests/interactive_tests.rs b/beacon_node/http_api/tests/interactive_tests.rs index 210c4d2550..6fb197b41a 100644 --- a/beacon_node/http_api/tests/interactive_tests.rs +++ b/beacon_node/http_api/tests/interactive_tests.rs @@ -632,7 +632,7 @@ pub async fn proposer_boost_re_org_test( panic!("Should not be a blinded block"); } }; - let block_c = harness.sign_beacon_block(unsigned_block_c, &state_b); + let block_c = Arc::new(harness.sign_beacon_block(unsigned_block_c, &state_b)); if should_re_org { // Block C should build on A. diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index 300c5a1060..4e0b4d761e 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -2597,7 +2597,7 @@ impl ApiTester { let signed_block = block.sign(&sk, &fork, genesis_validators_root, &self.chain.spec); let signed_block_contents = - PublishBlockRequest::try_from(signed_block.clone()).unwrap(); + PublishBlockRequest::try_from(Arc::new(signed_block.clone())).unwrap(); self.client .post_beacon_blocks(&signed_block_contents) @@ -2670,8 +2670,8 @@ impl ApiTester { .unwrap(); assert_eq!( - self.chain.head_beacon_block().as_ref(), - signed_block_contents.signed_block() + self.chain.head_beacon_block(), + *signed_block_contents.signed_block() ); self.chain.slot_clock.set_slot(slot.as_u64() + 1); @@ -2763,8 +2763,8 @@ impl ApiTester { .unwrap(); assert_eq!( - self.chain.head_beacon_block().as_ref(), - signed_block_contents.signed_block() + self.chain.head_beacon_block(), + *signed_block_contents.signed_block() ); self.chain.slot_clock.set_slot(slot.as_u64() + 1); @@ -2994,7 +2994,7 @@ impl ApiTester { .data; let signed_block = signed_block_contents.signed_block(); - assert_eq!(&head_block, signed_block); + assert_eq!(head_block, **signed_block); self.chain.slot_clock.set_slot(slot.as_u64() + 1); } diff --git a/beacon_node/network/src/network_beacon_processor/tests.rs b/beacon_node/network/src/network_beacon_processor/tests.rs index 48c5334357..dd58eb8355 100644 --- a/beacon_node/network/src/network_beacon_processor/tests.rs +++ b/beacon_node/network/src/network_beacon_processor/tests.rs @@ -250,7 +250,7 @@ impl TestRig { }; Self { chain, - next_block: Arc::new(block), + next_block: block, next_blobs: blob_sidecars, attestations, next_block_attestations, diff --git a/common/eth2/src/types.rs b/common/eth2/src/types.rs index 3e8d0c6079..a301055f34 100644 --- a/common/eth2/src/types.rs +++ b/common/eth2/src/types.rs @@ -15,6 +15,7 @@ use ssz_derive::{Decode, Encode}; use std::convert::TryFrom; use std::fmt::{self, Display}; use std::str::{from_utf8, FromStr}; +use std::sync::Arc; use std::time::Duration; use types::beacon_block_body::KzgCommitments; pub use types::*; @@ -1479,10 +1480,10 @@ mod tests { type E = MainnetEthSpec; let spec = ForkName::Capella.make_genesis_spec(E::default_spec()); - let block: PublishBlockRequest = SignedBeaconBlock::from_block( + let block: PublishBlockRequest = Arc::new(SignedBeaconBlock::from_block( BeaconBlock::::Capella(BeaconBlockCapella::empty(&spec)), Signature::empty(), - ) + )) .try_into() .expect("should convert into signed block contents"); @@ -1503,7 +1504,8 @@ mod tests { ); let blobs = BlobsList::::from(vec![Blob::::default()]); let kzg_proofs = KzgProofs::::from(vec![KzgProof::empty()]); - let signed_block_contents = PublishBlockRequest::new(block, Some((kzg_proofs, blobs))); + let signed_block_contents = + PublishBlockRequest::new(Arc::new(block), Some((kzg_proofs, blobs))); let decoded: PublishBlockRequest = PublishBlockRequest::from_ssz_bytes( &signed_block_contents.as_ssz_bytes(), @@ -1644,7 +1646,7 @@ impl FullBlockContents { ) -> PublishBlockRequest { let (block, maybe_blobs) = self.deconstruct(); let signed_block = block.sign(secret_key, fork, genesis_validators_root, spec); - PublishBlockRequest::new(signed_block, maybe_blobs) + PublishBlockRequest::new(Arc::new(signed_block), maybe_blobs) } } @@ -1675,7 +1677,10 @@ impl Into> for FullBlockContents { } } -pub type SignedBlockContentsTuple = (SignedBeaconBlock, Option<(KzgProofs, BlobsList)>); +pub type SignedBlockContentsTuple = ( + Arc>, + Option<(KzgProofs, BlobsList)>, +); fn parse_required_header( headers: &HeaderMap, @@ -1730,12 +1735,12 @@ impl TryFrom<&HeaderMap> for ProduceBlockV3Metadata { #[ssz(enum_behaviour = "transparent")] pub enum PublishBlockRequest { BlockContents(SignedBlockContents), - Block(SignedBeaconBlock), + Block(Arc>), } impl PublishBlockRequest { pub fn new( - block: SignedBeaconBlock, + block: Arc>, blob_items: Option<(KzgProofs, BlobsList)>, ) -> Self { match blob_items { @@ -1753,7 +1758,7 @@ impl PublishBlockRequest { match fork_name { ForkName::Base | ForkName::Altair | ForkName::Merge | ForkName::Capella => { SignedBeaconBlock::from_ssz_bytes_for_fork(bytes, fork_name) - .map(|block| PublishBlockRequest::Block(block)) + .map(|block| PublishBlockRequest::Block(Arc::new(block))) } ForkName::Deneb => { let mut builder = ssz::SszDecoderBuilder::new(bytes); @@ -1767,12 +1772,15 @@ impl PublishBlockRequest { })?; let kzg_proofs = decoder.decode_next()?; let blobs = decoder.decode_next()?; - Ok(PublishBlockRequest::new(block, Some((kzg_proofs, blobs)))) + Ok(PublishBlockRequest::new( + Arc::new(block), + Some((kzg_proofs, blobs)), + )) } } } - pub fn signed_block(&self) -> &SignedBeaconBlock { + pub fn signed_block(&self) -> &Arc> { match self { PublishBlockRequest::BlockContents(block_and_sidecars) => { &block_and_sidecars.signed_block @@ -1802,14 +1810,14 @@ pub fn into_full_block_and_blobs( let signed_block = blinded_block .try_into_full_block(None) .ok_or("Failed to build full block with payload".to_string())?; - Ok(PublishBlockRequest::new(signed_block, None)) + Ok(PublishBlockRequest::new(Arc::new(signed_block), None)) } // This variant implies a pre-deneb block Some(FullPayloadContents::Payload(execution_payload)) => { let signed_block = blinded_block .try_into_full_block(Some(execution_payload)) .ok_or("Failed to build full block with payload".to_string())?; - Ok(PublishBlockRequest::new(signed_block, None)) + Ok(PublishBlockRequest::new(Arc::new(signed_block), None)) } // This variant implies a post-deneb block Some(FullPayloadContents::PayloadAndBlobs(payload_and_blobs)) => { @@ -1818,7 +1826,7 @@ pub fn into_full_block_and_blobs( .ok_or("Failed to build full block with payload".to_string())?; Ok(PublishBlockRequest::new( - signed_block, + Arc::new(signed_block), Some(( payload_and_blobs.blobs_bundle.proofs, payload_and_blobs.blobs_bundle.blobs, @@ -1828,10 +1836,10 @@ pub fn into_full_block_and_blobs( } } -impl TryFrom> for PublishBlockRequest { +impl TryFrom>> for PublishBlockRequest { type Error = &'static str; - fn try_from(block: SignedBeaconBlock) -> Result { - match block { + fn try_from(block: Arc>) -> Result { + match *block { SignedBeaconBlock::Base(_) | SignedBeaconBlock::Altair(_) | SignedBeaconBlock::Merge(_) @@ -1852,7 +1860,7 @@ impl From> for PublishBlockRequest { #[derive(Debug, Clone, Serialize, Deserialize, Encode)] #[serde(bound = "T: EthSpec")] pub struct SignedBlockContents { - pub signed_block: SignedBeaconBlock, + pub signed_block: Arc>, pub kzg_proofs: KzgProofs, #[serde(with = "ssz_types::serde_utils::list_of_hex_fixed_vec")] pub blobs: BlobsList, diff --git a/consensus/fork_choice/tests/tests.rs b/consensus/fork_choice/tests/tests.rs index c21b8ed567..649fbcc555 100644 --- a/consensus/fork_choice/tests/tests.rs +++ b/consensus/fork_choice/tests/tests.rs @@ -323,8 +323,9 @@ impl ForkChoiceTest { ) .unwrap(); let slot = self.harness.get_current_slot(); - let (mut block_tuple, mut state) = self.harness.make_block(state, slot).await; - func(&mut block_tuple.0, &mut state); + let ((block_arc, _block_blobs), mut state) = self.harness.make_block(state, slot).await; + let mut block = (*block_arc).clone(); + func(&mut block, &mut state); let current_slot = self.harness.get_current_slot(); self.harness .chain @@ -332,8 +333,8 @@ impl ForkChoiceTest { .fork_choice_write_lock() .on_block( current_slot, - block_tuple.0.message(), - block_tuple.0.canonical_root(), + block.message(), + block.canonical_root(), Duration::from_secs(0), &state, PayloadVerificationStatus::Verified, @@ -366,8 +367,9 @@ impl ForkChoiceTest { ) .unwrap(); let slot = self.harness.get_current_slot(); - let (mut block_tuple, mut state) = self.harness.make_block(state, slot).await; - mutation_func(&mut block_tuple.0, &mut state); + let ((block_arc, _block_blobs), mut state) = self.harness.make_block(state, slot).await; + let mut block = (*block_arc).clone(); + mutation_func(&mut block, &mut state); let current_slot = self.harness.get_current_slot(); let err = self .harness @@ -376,8 +378,8 @@ impl ForkChoiceTest { .fork_choice_write_lock() .on_block( current_slot, - block_tuple.0.message(), - block_tuple.0.canonical_root(), + block.message(), + block.canonical_root(), Duration::from_secs(0), &state, PayloadVerificationStatus::Verified, diff --git a/consensus/state_processing/src/per_block_processing/tests.rs b/consensus/state_processing/src/per_block_processing/tests.rs index 75eb438967..83fd0f232c 100644 --- a/consensus/state_processing/src/per_block_processing/tests.rs +++ b/consensus/state_processing/src/per_block_processing/tests.rs @@ -90,7 +90,7 @@ async fn invalid_block_header_state_slot() { let slot = state.slot() + Slot::new(1); let ((signed_block, _), mut state) = harness.make_block_return_pre_state(state, slot).await; - let (mut block, signature) = signed_block.deconstruct(); + let (mut block, signature) = (*signed_block).clone().deconstruct(); *block.slot_mut() = slot + Slot::new(1); let mut ctxt = ConsensusContext::new(block.slot()); @@ -123,7 +123,7 @@ async fn invalid_parent_block_root() { let ((signed_block, _), mut state) = harness .make_block_return_pre_state(state, slot + Slot::new(1)) .await; - let (mut block, signature) = signed_block.deconstruct(); + let (mut block, signature) = (*signed_block).clone().deconstruct(); *block.parent_root_mut() = Hash256::from([0xAA; 32]); let mut ctxt = ConsensusContext::new(block.slot()); @@ -158,7 +158,7 @@ async fn invalid_block_signature() { let ((signed_block, _), mut state) = harness .make_block_return_pre_state(state, slot + Slot::new(1)) .await; - let (block, _) = signed_block.deconstruct(); + let (block, _) = (*signed_block).clone().deconstruct(); let mut ctxt = ConsensusContext::new(block.slot()); let result = per_block_processing( diff --git a/testing/state_transition_vectors/src/exit.rs b/testing/state_transition_vectors/src/exit.rs index 29f5c015e3..50b98d3066 100644 --- a/testing/state_transition_vectors/src/exit.rs +++ b/testing/state_transition_vectors/src/exit.rs @@ -57,7 +57,7 @@ impl ExitTest { block_modifier(&harness, block); }) .await; - (signed_block.0, state) + ((*signed_block.0).clone(), state) } fn process( diff --git a/validator_client/src/block_service.rs b/validator_client/src/block_service.rs index b65e301de8..e0c98660e2 100644 --- a/validator_client/src/block_service.rs +++ b/validator_client/src/block_service.rs @@ -450,12 +450,13 @@ impl BlockService { self.validator_store .sign_block(*validator_pubkey, block, slot) .await - .map(|b| SignedBlock::Full(PublishBlockRequest::new(b, maybe_blobs))) + .map(|b| SignedBlock::Full(PublishBlockRequest::new(Arc::new(b), maybe_blobs))) } UnsignedBlock::Blinded(block) => self .validator_store .sign_block(*validator_pubkey, block, slot) .await + .map(Arc::new) .map(SignedBlock::Blinded), }; @@ -870,7 +871,7 @@ impl UnsignedBlock { pub enum SignedBlock { Full(PublishBlockRequest), - Blinded(SignedBlindedBeaconBlock), + Blinded(Arc>), } impl SignedBlock { From b7c9b2787267b96e36978b1d221e46c832222289 Mon Sep 17 00:00:00 2001 From: Age Manning Date: Tue, 23 Jan 2024 16:38:00 +1100 Subject: [PATCH 18/25] Gossipsub fanout correction (#5110) * Correct fanout in gossipsub * Upgrade discv5 to pin new libp2p version * Update cargo.lock --- Cargo.lock | 134 +++++++++++----------- Cargo.toml | 2 +- beacon_node/lighthouse_network/Cargo.toml | 4 +- 3 files changed, 70 insertions(+), 70 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 25766f86ab..4c830105de 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -299,9 +299,9 @@ dependencies = [ [[package]] name = "async-io" -version = "2.2.2" +version = "2.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6afaa937395a620e33dc6a742c593c01aced20aa376ffb0f628121198578ccc7" +checksum = "fb41eb19024a91746eba0773aa5e16036045bbf45733766661099e182ea6a744" dependencies = [ "async-lock", "cfg-if", @@ -676,7 +676,7 @@ version = "0.66.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f2b84e06fc203107bfbad243f4aba2af864eb7db3b1cf46ea0a023b0b433d2a7" dependencies = [ - "bitflags 2.4.1", + "bitflags 2.4.2", "cexpr", "clang-sys", "lazy_static", @@ -701,9 +701,9 @@ checksum = "bef38d45163c2f1dde094a7dfd33ccf595c92905c8f8f4fdc18d06fb1037718a" [[package]] name = "bitflags" -version = "2.4.1" +version = "2.4.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "327762f6e5a765692301e5bb513e0d9fef63be86bbc14528052b1cd3e6f03e07" +checksum = "ed570934406eb16438a4e976b1b4500774099c13b8cb96eec99f620f05090ddf" [[package]] name = "bitvec" @@ -1021,14 +1021,14 @@ dependencies = [ [[package]] name = "chrono" -version = "0.4.31" +version = "0.4.32" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7f2c685bad3eb3d45a01354cedb7d5faa66194d1d58ba6e267a8de788f79db38" +checksum = "41daef31d7a747c5c847246f36de49ced6f7403b4cdabc807a97b5cc184cda7a" dependencies = [ "android-tzdata", "iana-time-zone", "num-traits", - "windows-targets 0.48.5", + "windows-targets 0.52.0", ] [[package]] @@ -1662,7 +1662,7 @@ version = "2.1.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "62c6fcf842f17f8c78ecf7c81d75c5ce84436b41ee07e03f490fbb5f5a8731d8" dependencies = [ - "bitflags 2.4.1", + "bitflags 2.4.2", "byteorder", "diesel_derives", "itoa", @@ -1776,7 +1776,7 @@ dependencies = [ [[package]] name = "discv5" version = "0.4.0" -source = "git+https://github.com/sigp/discv5?rev=dbb4a718cd32eaed8127c3c8241bfd0fde9eb908#dbb4a718cd32eaed8127c3c8241bfd0fde9eb908" +source = "git+https://github.com/sigp/discv5?rev=e30a2c31b7ac0c57876458b971164654dfa4513b#e30a2c31b7ac0c57876458b971164654dfa4513b" dependencies = [ "aes 0.7.5", "aes-gcm 0.9.2", @@ -2836,7 +2836,7 @@ dependencies = [ [[package]] name = "futures-bounded" version = "0.2.3" -source = "git+https://github.com/sigp/rust-libp2p/?rev=b96b90894faab0a1eed78e1c82c6452138a3538a#b96b90894faab0a1eed78e1c82c6452138a3538a" +source = "git+https://github.com/sigp/rust-libp2p/?rev=cfa3275ca17e502799ed56e555b6c0611752e369#cfa3275ca17e502799ed56e555b6c0611752e369" dependencies = [ "futures-timer", "futures-util", @@ -3216,9 +3216,9 @@ dependencies = [ [[package]] name = "hermit-abi" -version = "0.3.3" +version = "0.3.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d77f7ec81a6d05a3abb01ab6eb7590f6083d08449fe5a1c8b1e620283546ccb7" +checksum = "5d3d0e0f38255e7fa3cf31335b3a56f05febd18025f4db5ef7a0cfb4f8da651f" [[package]] name = "hex" @@ -3506,7 +3506,7 @@ dependencies = [ "httpdate", "itoa", "pin-project-lite", - "socket2 0.4.10", + "socket2 0.5.5", "tokio", "tower-service", "tracing", @@ -3835,7 +3835,7 @@ version = "1.0.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "eae7b9aee968036d54dce06cebaefd919e4472e753296daccd6d344e3e2df0c2" dependencies = [ - "hermit-abi 0.3.3", + "hermit-abi 0.3.4", "libc", "windows-sys 0.48.0", ] @@ -4136,7 +4136,7 @@ dependencies = [ [[package]] name = "libp2p" version = "0.54.0" -source = "git+https://github.com/sigp/rust-libp2p/?rev=b96b90894faab0a1eed78e1c82c6452138a3538a#b96b90894faab0a1eed78e1c82c6452138a3538a" +source = "git+https://github.com/sigp/rust-libp2p/?rev=cfa3275ca17e502799ed56e555b6c0611752e369#cfa3275ca17e502799ed56e555b6c0611752e369" dependencies = [ "bytes", "either", @@ -4169,7 +4169,7 @@ dependencies = [ [[package]] name = "libp2p-allow-block-list" version = "0.3.0" -source = "git+https://github.com/sigp/rust-libp2p/?rev=b96b90894faab0a1eed78e1c82c6452138a3538a#b96b90894faab0a1eed78e1c82c6452138a3538a" +source = "git+https://github.com/sigp/rust-libp2p/?rev=cfa3275ca17e502799ed56e555b6c0611752e369#cfa3275ca17e502799ed56e555b6c0611752e369" dependencies = [ "libp2p-core", "libp2p-identity", @@ -4180,7 +4180,7 @@ dependencies = [ [[package]] name = "libp2p-connection-limits" version = "0.3.1" -source = "git+https://github.com/sigp/rust-libp2p/?rev=b96b90894faab0a1eed78e1c82c6452138a3538a#b96b90894faab0a1eed78e1c82c6452138a3538a" +source = "git+https://github.com/sigp/rust-libp2p/?rev=cfa3275ca17e502799ed56e555b6c0611752e369#cfa3275ca17e502799ed56e555b6c0611752e369" dependencies = [ "libp2p-core", "libp2p-identity", @@ -4191,7 +4191,7 @@ dependencies = [ [[package]] name = "libp2p-core" version = "0.41.2" -source = "git+https://github.com/sigp/rust-libp2p/?rev=b96b90894faab0a1eed78e1c82c6452138a3538a#b96b90894faab0a1eed78e1c82c6452138a3538a" +source = "git+https://github.com/sigp/rust-libp2p/?rev=cfa3275ca17e502799ed56e555b6c0611752e369#cfa3275ca17e502799ed56e555b6c0611752e369" dependencies = [ "either", "fnv", @@ -4218,7 +4218,7 @@ dependencies = [ [[package]] name = "libp2p-dns" version = "0.41.1" -source = "git+https://github.com/sigp/rust-libp2p/?rev=b96b90894faab0a1eed78e1c82c6452138a3538a#b96b90894faab0a1eed78e1c82c6452138a3538a" +source = "git+https://github.com/sigp/rust-libp2p/?rev=cfa3275ca17e502799ed56e555b6c0611752e369#cfa3275ca17e502799ed56e555b6c0611752e369" dependencies = [ "async-trait", "futures", @@ -4233,7 +4233,7 @@ dependencies = [ [[package]] name = "libp2p-gossipsub" version = "0.46.1" -source = "git+https://github.com/sigp/rust-libp2p/?rev=b96b90894faab0a1eed78e1c82c6452138a3538a#b96b90894faab0a1eed78e1c82c6452138a3538a" +source = "git+https://github.com/sigp/rust-libp2p/?rev=cfa3275ca17e502799ed56e555b6c0611752e369#cfa3275ca17e502799ed56e555b6c0611752e369" dependencies = [ "async-channel", "asynchronous-codec", @@ -4265,7 +4265,7 @@ dependencies = [ [[package]] name = "libp2p-identify" version = "0.44.1" -source = "git+https://github.com/sigp/rust-libp2p/?rev=b96b90894faab0a1eed78e1c82c6452138a3538a#b96b90894faab0a1eed78e1c82c6452138a3538a" +source = "git+https://github.com/sigp/rust-libp2p/?rev=cfa3275ca17e502799ed56e555b6c0611752e369#cfa3275ca17e502799ed56e555b6c0611752e369" dependencies = [ "asynchronous-codec", "either", @@ -4310,7 +4310,7 @@ dependencies = [ [[package]] name = "libp2p-mdns" version = "0.45.1" -source = "git+https://github.com/sigp/rust-libp2p/?rev=b96b90894faab0a1eed78e1c82c6452138a3538a#b96b90894faab0a1eed78e1c82c6452138a3538a" +source = "git+https://github.com/sigp/rust-libp2p/?rev=cfa3275ca17e502799ed56e555b6c0611752e369#cfa3275ca17e502799ed56e555b6c0611752e369" dependencies = [ "data-encoding", "futures", @@ -4330,7 +4330,7 @@ dependencies = [ [[package]] name = "libp2p-metrics" version = "0.14.1" -source = "git+https://github.com/sigp/rust-libp2p/?rev=b96b90894faab0a1eed78e1c82c6452138a3538a#b96b90894faab0a1eed78e1c82c6452138a3538a" +source = "git+https://github.com/sigp/rust-libp2p/?rev=cfa3275ca17e502799ed56e555b6c0611752e369#cfa3275ca17e502799ed56e555b6c0611752e369" dependencies = [ "futures", "instant", @@ -4346,7 +4346,7 @@ dependencies = [ [[package]] name = "libp2p-mplex" version = "0.41.0" -source = "git+https://github.com/sigp/rust-libp2p/?rev=b96b90894faab0a1eed78e1c82c6452138a3538a#b96b90894faab0a1eed78e1c82c6452138a3538a" +source = "git+https://github.com/sigp/rust-libp2p/?rev=cfa3275ca17e502799ed56e555b6c0611752e369#cfa3275ca17e502799ed56e555b6c0611752e369" dependencies = [ "asynchronous-codec", "bytes", @@ -4364,7 +4364,7 @@ dependencies = [ [[package]] name = "libp2p-noise" version = "0.44.0" -source = "git+https://github.com/sigp/rust-libp2p/?rev=b96b90894faab0a1eed78e1c82c6452138a3538a#b96b90894faab0a1eed78e1c82c6452138a3538a" +source = "git+https://github.com/sigp/rust-libp2p/?rev=cfa3275ca17e502799ed56e555b6c0611752e369#cfa3275ca17e502799ed56e555b6c0611752e369" dependencies = [ "asynchronous-codec", "bytes", @@ -4389,7 +4389,7 @@ dependencies = [ [[package]] name = "libp2p-plaintext" version = "0.41.0" -source = "git+https://github.com/sigp/rust-libp2p/?rev=b96b90894faab0a1eed78e1c82c6452138a3538a#b96b90894faab0a1eed78e1c82c6452138a3538a" +source = "git+https://github.com/sigp/rust-libp2p/?rev=cfa3275ca17e502799ed56e555b6c0611752e369#cfa3275ca17e502799ed56e555b6c0611752e369" dependencies = [ "asynchronous-codec", "bytes", @@ -4404,7 +4404,7 @@ dependencies = [ [[package]] name = "libp2p-quic" version = "0.10.2" -source = "git+https://github.com/sigp/rust-libp2p/?rev=b96b90894faab0a1eed78e1c82c6452138a3538a#b96b90894faab0a1eed78e1c82c6452138a3538a" +source = "git+https://github.com/sigp/rust-libp2p/?rev=cfa3275ca17e502799ed56e555b6c0611752e369#cfa3275ca17e502799ed56e555b6c0611752e369" dependencies = [ "bytes", "futures", @@ -4427,7 +4427,7 @@ dependencies = [ [[package]] name = "libp2p-swarm" version = "0.45.0" -source = "git+https://github.com/sigp/rust-libp2p/?rev=b96b90894faab0a1eed78e1c82c6452138a3538a#b96b90894faab0a1eed78e1c82c6452138a3538a" +source = "git+https://github.com/sigp/rust-libp2p/?rev=cfa3275ca17e502799ed56e555b6c0611752e369#cfa3275ca17e502799ed56e555b6c0611752e369" dependencies = [ "either", "fnv", @@ -4449,7 +4449,7 @@ dependencies = [ [[package]] name = "libp2p-swarm-derive" version = "0.34.1" -source = "git+https://github.com/sigp/rust-libp2p/?rev=b96b90894faab0a1eed78e1c82c6452138a3538a#b96b90894faab0a1eed78e1c82c6452138a3538a" +source = "git+https://github.com/sigp/rust-libp2p/?rev=cfa3275ca17e502799ed56e555b6c0611752e369#cfa3275ca17e502799ed56e555b6c0611752e369" dependencies = [ "heck", "proc-macro2", @@ -4460,7 +4460,7 @@ dependencies = [ [[package]] name = "libp2p-tcp" version = "0.41.0" -source = "git+https://github.com/sigp/rust-libp2p/?rev=b96b90894faab0a1eed78e1c82c6452138a3538a#b96b90894faab0a1eed78e1c82c6452138a3538a" +source = "git+https://github.com/sigp/rust-libp2p/?rev=cfa3275ca17e502799ed56e555b6c0611752e369#cfa3275ca17e502799ed56e555b6c0611752e369" dependencies = [ "futures", "futures-timer", @@ -4476,7 +4476,7 @@ dependencies = [ [[package]] name = "libp2p-tls" version = "0.3.0" -source = "git+https://github.com/sigp/rust-libp2p/?rev=b96b90894faab0a1eed78e1c82c6452138a3538a#b96b90894faab0a1eed78e1c82c6452138a3538a" +source = "git+https://github.com/sigp/rust-libp2p/?rev=cfa3275ca17e502799ed56e555b6c0611752e369#cfa3275ca17e502799ed56e555b6c0611752e369" dependencies = [ "futures", "futures-rustls", @@ -4494,7 +4494,7 @@ dependencies = [ [[package]] name = "libp2p-upnp" version = "0.2.0" -source = "git+https://github.com/sigp/rust-libp2p/?rev=b96b90894faab0a1eed78e1c82c6452138a3538a#b96b90894faab0a1eed78e1c82c6452138a3538a" +source = "git+https://github.com/sigp/rust-libp2p/?rev=cfa3275ca17e502799ed56e555b6c0611752e369#cfa3275ca17e502799ed56e555b6c0611752e369" dependencies = [ "futures", "futures-timer", @@ -4509,7 +4509,7 @@ dependencies = [ [[package]] name = "libp2p-yamux" version = "0.45.1" -source = "git+https://github.com/sigp/rust-libp2p/?rev=b96b90894faab0a1eed78e1c82c6452138a3538a#b96b90894faab0a1eed78e1c82c6452138a3538a" +source = "git+https://github.com/sigp/rust-libp2p/?rev=cfa3275ca17e502799ed56e555b6c0611752e369#cfa3275ca17e502799ed56e555b6c0611752e369" dependencies = [ "either", "futures", @@ -4526,7 +4526,7 @@ version = "0.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "85c833ca1e66078851dba29046874e38f08b2c883700aa29a03ddd3b23814ee8" dependencies = [ - "bitflags 2.4.1", + "bitflags 2.4.2", "libc", "redox_syscall 0.4.1", ] @@ -4728,9 +4728,9 @@ checksum = "f051f77a7c8e6957c0696eac88f26b0117e54f52d3fc682ab19397a8812846a4" [[package]] name = "linux-raw-sys" -version = "0.4.12" +version = "0.4.13" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c4cd1a83af159aa67994778be9070f0ae1bd732942279cabb14f86f986a21456" +checksum = "01cda141df6706de531b6c46c3a33ecca755538219bd484262fa09410c13539c" [[package]] name = "lmdb-rkv" @@ -5106,7 +5106,7 @@ dependencies = [ [[package]] name = "multistream-select" version = "0.13.0" -source = "git+https://github.com/sigp/rust-libp2p/?rev=b96b90894faab0a1eed78e1c82c6452138a3538a#b96b90894faab0a1eed78e1c82c6452138a3538a" +source = "git+https://github.com/sigp/rust-libp2p/?rev=cfa3275ca17e502799ed56e555b6c0611752e369#cfa3275ca17e502799ed56e555b6c0611752e369" dependencies = [ "bytes", "futures", @@ -5267,7 +5267,7 @@ version = "0.27.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2eb04e9c688eff1c89d72b407f168cf79bb9e867a9d3323ed6c01519eb9cc053" dependencies = [ - "bitflags 2.4.1", + "bitflags 2.4.2", "cfg-if", "libc", ] @@ -5388,7 +5388,7 @@ version = "1.16.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4161fcb6d602d4d2081af7c3a45852d875a03dd337a6bfdd6e06407b61342a43" dependencies = [ - "hermit-abi 0.3.3", + "hermit-abi 0.3.4", "libc", ] @@ -5471,11 +5471,11 @@ dependencies = [ [[package]] name = "openssl" -version = "0.10.62" +version = "0.10.63" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8cde4d2d9200ad5909f8dac647e29482e07c3a35de8a13fce7c9c7747ad9f671" +checksum = "15c9d69dd87a29568d4d017cfe8ec518706046a05184e5aea92d0af890b803c8" dependencies = [ - "bitflags 2.4.1", + "bitflags 2.4.2", "cfg-if", "foreign-types", "libc", @@ -5512,9 +5512,9 @@ dependencies = [ [[package]] name = "openssl-sys" -version = "0.9.98" +version = "0.9.99" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c1665caf8ab2dc9aef43d1c0023bd904633a6a05cb30b0ad59bec2ae986e57a7" +checksum = "22e1bf214306098e4832460f797824c05d25aacdf896f64a985fb0fd992454ae" dependencies = [ "cc", "libc", @@ -5830,9 +5830,9 @@ dependencies = [ [[package]] name = "pkg-config" -version = "0.3.28" +version = "0.3.29" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "69d3587f8a9e599cc7ec2c00e331f71c4e69a5f9a4b8a6efd5b07466b9736f9a" +checksum = "2900ede94e305130c13ddd391e0ab7cbaeb783945ae07a279c268cb05109c6cb" [[package]] name = "platforms" @@ -6072,9 +6072,9 @@ dependencies = [ [[package]] name = "proc-macro2" -version = "1.0.76" +version = "1.0.78" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "95fc56cda0b5c3325f5fbbd7ff9fda9e02bb00bb3dac51252d2f1bfa1cb8cc8c" +checksum = "e2422ad645d89c99f8f3e6b88a9fdeca7fabeac836b1002371c4367c8f984aae" dependencies = [ "unicode-ident", ] @@ -6188,7 +6188,7 @@ dependencies = [ [[package]] name = "quick-protobuf-codec" version = "0.3.1" -source = "git+https://github.com/sigp/rust-libp2p/?rev=b96b90894faab0a1eed78e1c82c6452138a3538a#b96b90894faab0a1eed78e1c82c6452138a3538a" +source = "git+https://github.com/sigp/rust-libp2p/?rev=cfa3275ca17e502799ed56e555b6c0611752e369#cfa3275ca17e502799ed56e555b6c0611752e369" dependencies = [ "asynchronous-codec", "bytes", @@ -6350,9 +6350,9 @@ dependencies = [ [[package]] name = "rayon" -version = "1.8.0" +version = "1.8.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9c27db03db7734835b3f53954b534c91069375ce6ccaa2e065441e07d9b6cdb1" +checksum = "fa7237101a77a10773db45d62004a272517633fbcc3df19d96455ede1122e051" dependencies = [ "either", "rayon-core", @@ -6360,9 +6360,9 @@ dependencies = [ [[package]] name = "rayon-core" -version = "1.12.0" +version = "1.12.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5ce3fb6ad83f861aac485e76e1985cd109d9a3713802152be56c3b1f0e0658ed" +checksum = "1465873a3dfdaa8ae7cb14b4383657caab0b3e8a0aa9ae8e04b044854c8dfce2" dependencies = [ "crossbeam-deque", "crossbeam-utils", @@ -6411,13 +6411,13 @@ dependencies = [ [[package]] name = "regex" -version = "1.10.2" +version = "1.10.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "380b951a9c5e80ddfd6136919eef32310721aa4aacd4889a8d39124b026ab343" +checksum = "b62dbe01f0b06f9d8dc7d49e05a0785f153b00b2c227856282f671e0318c9b15" dependencies = [ "aho-corasick", "memchr", - "regex-automata 0.4.3", + "regex-automata 0.4.4", "regex-syntax 0.8.2", ] @@ -6432,9 +6432,9 @@ dependencies = [ [[package]] name = "regex-automata" -version = "0.4.3" +version = "0.4.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5f804c7828047e88b2d32e2d7fe5a105da8ee3264f01902f796c8e067dc2483f" +checksum = "3b7fa1134405e2ec9353fd416b17f8dacd46c473d7d3fd1cf202706a14eb792a" dependencies = [ "aho-corasick", "memchr", @@ -6680,10 +6680,10 @@ version = "0.38.30" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "322394588aaf33c24007e8bb3238ee3e4c5c09c084ab32bc73890b99ff326bca" dependencies = [ - "bitflags 2.4.1", + "bitflags 2.4.2", "errno", "libc", - "linux-raw-sys 0.4.12", + "linux-raw-sys 0.4.13", "windows-sys 0.52.0", ] @@ -6768,7 +6768,7 @@ checksum = "7ffc183a10b4478d04cbbbfc96d0873219d962dd5accaff2ffbd4ceb7df837f4" [[package]] name = "rw-stream-sink" version = "0.4.0" -source = "git+https://github.com/sigp/rust-libp2p/?rev=b96b90894faab0a1eed78e1c82c6452138a3538a#b96b90894faab0a1eed78e1c82c6452138a3538a" +source = "git+https://github.com/sigp/rust-libp2p/?rev=cfa3275ca17e502799ed56e555b6c0611752e369#cfa3275ca17e502799ed56e555b6c0611752e369" dependencies = [ "futures", "pin-project", @@ -7408,9 +7408,9 @@ dependencies = [ [[package]] name = "smallvec" -version = "1.12.0" +version = "1.13.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2593d31f82ead8df961d8bd23a64c2ccf2eb5dd34b0a34bfb4dd54011c72009e" +checksum = "e6ecd384b10a64542d77071bd64bd7b231f4ed5940fba55e98c3de13824cf3d7" [[package]] name = "snap" @@ -8433,9 +8433,9 @@ dependencies = [ [[package]] name = "unicode-bidi" -version = "0.3.14" +version = "0.3.15" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6f2528f27a9eb2b21e69c95319b30bd0efd85d09c379741b0f78ea1d86be2416" +checksum = "08f95100a766bf4f8f28f90d77e0a5461bbdb219042e7679bebe79004fed8d75" [[package]] name = "unicode-ident" @@ -8709,7 +8709,7 @@ dependencies = [ [[package]] name = "warp" version = "0.3.6" -source = "git+https://github.com/seanmonstar/warp.git#724e767173132de7c435b323e12d6bec68038de2" +source = "git+https://github.com/seanmonstar/warp.git#7b07043cee0ca24e912155db4e8f6d9ab7c049ed" dependencies = [ "bytes", "futures-channel", diff --git a/Cargo.toml b/Cargo.toml index a3e1430f3f..ca55d00d4b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -105,7 +105,7 @@ criterion = "0.3" delay_map = "0.3" derivative = "2" dirs = "3" -discv5 = { git="https://github.com/sigp/discv5", rev="dbb4a718cd32eaed8127c3c8241bfd0fde9eb908", features = ["libp2p"] } +discv5 = { git="https://github.com/sigp/discv5", rev="e30a2c31b7ac0c57876458b971164654dfa4513b", features = ["libp2p"] } env_logger = "0.9" error-chain = "0.12" ethereum-types = "0.14" diff --git a/beacon_node/lighthouse_network/Cargo.toml b/beacon_node/lighthouse_network/Cargo.toml index e1ae62be65..46acdeaded 100644 --- a/beacon_node/lighthouse_network/Cargo.toml +++ b/beacon_node/lighthouse_network/Cargo.toml @@ -43,11 +43,11 @@ prometheus-client = "0.22.0" unused_port = { workspace = true } delay_map = { workspace = true } void = "1" -libp2p-mplex = { git = "https://github.com/sigp/rust-libp2p/", rev = "b96b90894faab0a1eed78e1c82c6452138a3538a" } +libp2p-mplex = { git = "https://github.com/sigp/rust-libp2p/", rev = "cfa3275ca17e502799ed56e555b6c0611752e369" } [dependencies.libp2p] git = "https://github.com/sigp/rust-libp2p/" -rev = "b96b90894faab0a1eed78e1c82c6452138a3538a" +rev = "cfa3275ca17e502799ed56e555b6c0611752e369" default-features = false features = ["identify", "yamux", "noise", "gossipsub", "dns", "tcp", "tokio", "plaintext", "secp256k1", "macros", "ecdsa", "metrics", "quic"] From 5851027bfde24bc372c5143221c5e4d695197f82 Mon Sep 17 00:00:00 2001 From: Age Manning Date: Tue, 23 Jan 2024 16:58:24 +1100 Subject: [PATCH 19/25] Correct discovery logic (#5111) --- beacon_node/lighthouse_network/src/peer_manager/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beacon_node/lighthouse_network/src/peer_manager/mod.rs b/beacon_node/lighthouse_network/src/peer_manager/mod.rs index 3459548ec3..30b22dd303 100644 --- a/beacon_node/lighthouse_network/src/peer_manager/mod.rs +++ b/beacon_node/lighthouse_network/src/peer_manager/mod.rs @@ -916,7 +916,7 @@ impl PeerManager { { self.max_outbound_dialing_peers() .saturating_sub(dialing_peers) - - peer_count + .saturating_sub(peer_count) } else { 0 }; From 1cebf41452b63ec622e9ca3a4eb2e38f5b9ddbc7 Mon Sep 17 00:00:00 2001 From: realbigsean Date: Tue, 23 Jan 2024 17:35:02 -0500 Subject: [PATCH 20/25] Backfill blob storage fix (#5119) * store blobs in the correct db in backfill * add database migration * add migration file * remove log info suggesting deneb isn't schedule * add batching in blob migration --- .../beacon_chain/src/historical_blocks.rs | 6 +- beacon_node/beacon_chain/src/schema_change.rs | 9 +++ .../src/schema_change/migration_schema_v19.rs | 65 +++++++++++++++++++ beacon_node/store/src/metadata.rs | 2 +- book/src/database-migrations.md | 6 +- 5 files changed, 83 insertions(+), 5 deletions(-) create mode 100644 beacon_node/beacon_chain/src/schema_change/migration_schema_v19.rs diff --git a/beacon_node/beacon_chain/src/historical_blocks.rs b/beacon_node/beacon_chain/src/historical_blocks.rs index b5e58e7ffa..b5b42fcfcd 100644 --- a/beacon_node/beacon_chain/src/historical_blocks.rs +++ b/beacon_node/beacon_chain/src/historical_blocks.rs @@ -101,8 +101,9 @@ impl BeaconChain { ChunkWriter::::new(&self.store.cold_db, prev_block_slot.as_usize())?; let mut new_oldest_blob_slot = blob_info.oldest_blob_slot; + let mut blob_batch = Vec::with_capacity(n_blobs_lists_to_import); let mut cold_batch = Vec::with_capacity(blocks_to_import.len()); - let mut hot_batch = Vec::with_capacity(blocks_to_import.len() + n_blobs_lists_to_import); + let mut hot_batch = Vec::with_capacity(blocks_to_import.len()); let mut signed_blocks = Vec::with_capacity(blocks_to_import.len()); for available_block in blocks_to_import.into_iter().rev() { @@ -124,7 +125,7 @@ impl BeaconChain { if let Some(blobs) = maybe_blobs { new_oldest_blob_slot = Some(block.slot()); self.store - .blobs_as_kv_store_ops(&block_root, blobs, &mut hot_batch); + .blobs_as_kv_store_ops(&block_root, blobs, &mut blob_batch); } // Store block roots, including at all skip slots in the freezer DB. @@ -199,6 +200,7 @@ impl BeaconChain { // Write the I/O batches to disk, writing the blocks themselves first, as it's better // for the hot DB to contain extra blocks than for the cold DB to point to blocks that // do not exist. + self.store.blobs_db.do_atomically(blob_batch)?; self.store.hot_db.do_atomically(hot_batch)?; self.store.cold_db.do_atomically(cold_batch)?; diff --git a/beacon_node/beacon_chain/src/schema_change.rs b/beacon_node/beacon_chain/src/schema_change.rs index e42ee20c48..63eb72c43a 100644 --- a/beacon_node/beacon_chain/src/schema_change.rs +++ b/beacon_node/beacon_chain/src/schema_change.rs @@ -1,6 +1,7 @@ //! Utilities for managing database schema changes. mod migration_schema_v17; mod migration_schema_v18; +mod migration_schema_v19; use crate::beacon_chain::BeaconChainTypes; use crate::types::ChainSpec; @@ -69,6 +70,14 @@ pub fn migrate_schema( let ops = migration_schema_v18::downgrade_from_v18::(db.clone(), log)?; db.store_schema_version_atomically(to, ops) } + (SchemaVersion(18), SchemaVersion(19)) => { + let ops = migration_schema_v19::upgrade_to_v19::(db.clone(), log)?; + db.store_schema_version_atomically(to, ops) + } + (SchemaVersion(19), SchemaVersion(18)) => { + let ops = migration_schema_v19::downgrade_from_v19::(db.clone(), log)?; + db.store_schema_version_atomically(to, ops) + } // Anything else is an error. (_, _) => Err(HotColdDBError::UnsupportedSchemaVersion { target_version: to, diff --git a/beacon_node/beacon_chain/src/schema_change/migration_schema_v19.rs b/beacon_node/beacon_chain/src/schema_change/migration_schema_v19.rs new file mode 100644 index 0000000000..578e9bad31 --- /dev/null +++ b/beacon_node/beacon_chain/src/schema_change/migration_schema_v19.rs @@ -0,0 +1,65 @@ +use crate::beacon_chain::BeaconChainTypes; +use slog::{debug, info, Logger}; +use std::sync::Arc; +use store::{get_key_for_col, DBColumn, Error, HotColdDB, KeyValueStore, KeyValueStoreOp}; + +pub fn upgrade_to_v19( + db: Arc>, + log: Logger, +) -> Result, Error> { + let mut hot_delete_ops = vec![]; + let mut blob_keys = vec![]; + let column = DBColumn::BeaconBlob; + + debug!(log, "Migrating from v18 to v19"); + // Iterate through the blobs on disk. + for res in db.hot_db.iter_column_keys::>(column) { + let key = res?; + let key_col = get_key_for_col(column.as_str(), &key); + hot_delete_ops.push(KeyValueStoreOp::DeleteKey(key_col)); + blob_keys.push(key); + } + + let num_blobs = blob_keys.len(); + debug!(log, "Collected {} blob lists to migrate", num_blobs); + + let batch_size = 500; + let mut batch = Vec::with_capacity(batch_size); + + for key in blob_keys { + let next_blob = db.hot_db.get_bytes(column.as_str(), &key)?; + if let Some(next_blob) = next_blob { + let key_col = get_key_for_col(column.as_str(), &key); + batch.push(KeyValueStoreOp::PutKeyValue(key_col, next_blob)); + + if batch.len() >= batch_size { + db.blobs_db.do_atomically(batch.clone())?; + batch.clear(); + } + } + } + + // Process the remaining batch if it's not empty + if !batch.is_empty() { + db.blobs_db.do_atomically(batch)?; + } + + debug!(log, "Wrote {} blobs to the blobs db", num_blobs); + + // Delete all the blobs + info!(log, "Upgrading to v19 schema"); + Ok(hot_delete_ops) +} + +pub fn downgrade_from_v19( + _db: Arc>, + log: Logger, +) -> Result, Error> { + // No-op + info!( + log, + "Downgrading to v18 schema"; + ); + + Ok(vec![]) +} diff --git a/beacon_node/store/src/metadata.rs b/beacon_node/store/src/metadata.rs index 6fef74d7ff..1675051bd8 100644 --- a/beacon_node/store/src/metadata.rs +++ b/beacon_node/store/src/metadata.rs @@ -4,7 +4,7 @@ use ssz::{Decode, Encode}; use ssz_derive::{Decode, Encode}; use types::{Checkpoint, Hash256, Slot}; -pub const CURRENT_SCHEMA_VERSION: SchemaVersion = SchemaVersion(18); +pub const CURRENT_SCHEMA_VERSION: SchemaVersion = SchemaVersion(19); // All the keys that get stored under the `BeaconMeta` column. // diff --git a/book/src/database-migrations.md b/book/src/database-migrations.md index 5b7b4d4937..a4d28452d4 100644 --- a/book/src/database-migrations.md +++ b/book/src/database-migrations.md @@ -16,7 +16,8 @@ validator client or the slasher**. | Lighthouse version | Release date | Schema version | Downgrade available? | |--------------------|--------------|----------------|----------------------| -| v4.6.0 | Dec 2023 | v18 | yes before Deneb | +| v4.6.0 | Dec 2023 | v19 | yes before Deneb | +| v4.6.0-rc.0 | Dec 2023 | v18 | yes before Deneb | | v4.5.0 | Sep 2023 | v17 | yes | | v4.4.0 | Aug 2023 | v17 | yes | | v4.3.0 | Jul 2023 | v17 | yes | @@ -192,7 +193,8 @@ Here are the steps to prune historic states: | Lighthouse version | Release date | Schema version | Downgrade available? | |--------------------|--------------|----------------|-------------------------------------| -| v4.6.0 | Dec 2023 | v18 | yes before Deneb | +| v4.6.0 | Dec 2023 | v19 | yes before Deneb | +| v4.6.0-rc.0 | Dec 2023 | v18 | yes before Deneb | | v4.5.0 | Sep 2023 | v17 | yes | | v4.4.0 | Aug 2023 | v17 | yes | | v4.3.0 | Jul 2023 | v17 | yes | From b55b58b3c6d71eab13805ed736c5579d7b68d9da Mon Sep 17 00:00:00 2001 From: Pawan Dhananjay Date: Wed, 24 Jan 2024 04:05:24 +0530 Subject: [PATCH 21/25] Fix indices filter in blobs_sidecar http endpoint (#5118) * add get blobs unit test * Use a multi_key_query for blob_sidecar indices * Fix test * Remove env_logger --------- Co-authored-by: realbigsean --- beacon_node/http_api/src/lib.rs | 5 +-- beacon_node/http_api/tests/tests.rs | 54 +++++++++++++++++++++++++++++ common/eth2/src/lib.rs | 12 ++++++- 3 files changed, 68 insertions(+), 3 deletions(-) diff --git a/beacon_node/http_api/src/lib.rs b/beacon_node/http_api/src/lib.rs index 3777e61420..1594668e5c 100644 --- a/beacon_node/http_api/src/lib.rs +++ b/beacon_node/http_api/src/lib.rs @@ -1704,18 +1704,19 @@ pub fn serve( .and(warp::path("beacon")) .and(warp::path("blob_sidecars")) .and(block_id_or_err) - .and(warp::query::()) .and(warp::path::end()) + .and(multi_key_query::()) .and(task_spawner_filter.clone()) .and(chain_filter.clone()) .and(warp::header::optional::("accept")) .then( |block_id: BlockId, - indices: api_types::BlobIndicesQuery, + indices_res: Result, task_spawner: TaskSpawner, chain: Arc>, accept_header: Option| { task_spawner.blocking_response_task(Priority::P1, move || { + let indices = indices_res?; let blob_sidecar_list_filtered = block_id.blob_sidecar_list_filtered(indices, &chain)?; match accept_header { diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index 4e0b4d761e..933f98661c 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -1587,6 +1587,39 @@ impl ApiTester { self } + pub async fn test_get_blob_sidecars(self, use_indices: bool) -> Self { + let block_id = BlockId(CoreBlockId::Finalized); + let (block_root, _, _) = block_id.root(&self.chain).unwrap(); + let (block, _, _) = block_id.full_block(&self.chain).await.unwrap(); + let num_blobs = block.num_expected_blobs(); + let blob_indices = if use_indices { + Some( + (0..num_blobs.saturating_sub(1) as u64) + .into_iter() + .collect::>(), + ) + } else { + None + }; + let result = match self + .client + .get_blobs::(CoreBlockId::Root(block_root), blob_indices.as_deref()) + .await + { + Ok(result) => result.unwrap().data, + Err(e) => panic!("query failed incorrectly: {e:?}"), + }; + + assert_eq!( + result.len(), + blob_indices.map_or(num_blobs, |indices| indices.len()) + ); + let expected = block.slot(); + assert_eq!(result.get(0).unwrap().slot(), expected); + + self + } + pub async fn test_beacon_blocks_attestations(self) -> Self { for block_id in self.interesting_block_ids() { let result = self @@ -6291,6 +6324,27 @@ async fn builder_works_post_deneb() { .await; } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn get_blob_sidecars() { + let mut config = ApiTesterConfig { + retain_historic_states: false, + spec: E::default_spec(), + }; + config.spec.altair_fork_epoch = Some(Epoch::new(0)); + config.spec.bellatrix_fork_epoch = Some(Epoch::new(0)); + config.spec.capella_fork_epoch = Some(Epoch::new(0)); + config.spec.deneb_fork_epoch = Some(Epoch::new(0)); + + ApiTester::new_from_config(config) + .await + .test_post_beacon_blocks_valid() + .await + .test_get_blob_sidecars(false) + .await + .test_get_blob_sidecars(true) + .await; +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn post_validator_liveness_epoch() { ApiTester::new() diff --git a/common/eth2/src/lib.rs b/common/eth2/src/lib.rs index bed5044b4b..16801be8e2 100644 --- a/common/eth2/src/lib.rs +++ b/common/eth2/src/lib.rs @@ -1064,8 +1064,18 @@ impl BeaconNodeHttpClient { pub async fn get_blobs( &self, block_id: BlockId, + indices: Option<&[u64]>, ) -> Result>>, Error> { - let path = self.get_blobs_path(block_id)?; + let mut path = self.get_blobs_path(block_id)?; + if let Some(indices) = indices { + let indices_string = indices + .iter() + .map(|i| i.to_string()) + .collect::>() + .join(","); + path.query_pairs_mut() + .append_pair("indices", &indices_string); + } let Some(response) = self.get_response(path, |b| b).await.optional()? else { return Ok(None); }; From a36a12a8d2fc34dc9e8af7c0585ad29b2e16529e Mon Sep 17 00:00:00 2001 From: Age Manning Date: Wed, 24 Jan 2024 10:10:05 +1100 Subject: [PATCH 22/25] Correct multiple dial bug (#5113) * Dialing the same peer-id error fix * Improve dialing logging * Update beacon_node/lighthouse_network/src/peer_manager/mod.rs Co-authored-by: Lion - dapplion <35266934+dapplion@users.noreply.github.com> --------- Co-authored-by: Lion - dapplion <35266934+dapplion@users.noreply.github.com> --- .../src/peer_manager/mod.rs | 20 +++++++++++++------ .../lighthouse_network/src/service/mod.rs | 6 ++++-- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/beacon_node/lighthouse_network/src/peer_manager/mod.rs b/beacon_node/lighthouse_network/src/peer_manager/mod.rs index 30b22dd303..4316c0d07e 100644 --- a/beacon_node/lighthouse_network/src/peer_manager/mod.rs +++ b/beacon_node/lighthouse_network/src/peer_manager/mod.rs @@ -326,8 +326,10 @@ impl PeerManager { // considered a priority. We have pre-allocated some extra priority slots for these // peers as specified by PRIORITY_PEER_EXCESS. Therefore we dial these peers, even // if we are already at our max_peer limit. - if min_ttl.is_some() && connected_or_dialing + to_dial_peers < self.max_priority_peers() - || connected_or_dialing + to_dial_peers < self.max_peers() + if !self.peers_to_dial.contains(&enr) + && ((min_ttl.is_some() + && connected_or_dialing + to_dial_peers < self.max_priority_peers()) + || connected_or_dialing + to_dial_peers < self.max_peers()) { // This should be updated with the peer dialing. In fact created once the peer is // dialed @@ -337,9 +339,11 @@ impl PeerManager { .write() .update_min_ttl(&enr.peer_id(), min_ttl); } - debug!(self.log, "Dialing discovered peer"; "peer_id" => %enr.peer_id()); - self.dial_peer(enr); - to_dial_peers += 1; + let peer_id = enr.peer_id(); + if self.dial_peer(enr) { + debug!(self.log, "Dialing discovered peer"; "peer_id" => %peer_id); + to_dial_peers += 1; + } } } @@ -401,7 +405,8 @@ impl PeerManager { /* Notifications from the Swarm */ /// A peer is being dialed. - pub fn dial_peer(&mut self, peer: Enr) { + /// Returns true, if this peer will be dialed. + pub fn dial_peer(&mut self, peer: Enr) -> bool { if self .network_globals .peers @@ -409,6 +414,9 @@ impl PeerManager { .should_dial(&peer.peer_id()) { self.peers_to_dial.push(peer); + true + } else { + false } } diff --git a/beacon_node/lighthouse_network/src/service/mod.rs b/beacon_node/lighthouse_network/src/service/mod.rs index e85cf75fd8..2b20c76cf4 100644 --- a/beacon_node/lighthouse_network/src/service/mod.rs +++ b/beacon_node/lighthouse_network/src/service/mod.rs @@ -1161,9 +1161,11 @@ impl Network { // Remove the ENR from the cache to prevent continual re-dialing on disconnects for enr in peers_to_dial { - debug!(self.log, "Dialing cached ENR peer"; "peer_id" => %enr.peer_id()); self.discovery_mut().remove_cached_enr(&enr.peer_id()); - self.peer_manager_mut().dial_peer(enr); + let peer_id = enr.peer_id(); + if self.peer_manager_mut().dial_peer(enr) { + debug!(self.log, "Dialing cached ENR peer"; "peer_id" => %peer_id); + } } } From 612eaf2d41b9ca98d7e47b327f4564d41ff24ac9 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Wed, 24 Jan 2024 01:12:48 +0200 Subject: [PATCH 23/25] Prevent rolling file appender panic (#5117) * rolling file appender panic removal and max log file count * max log file --- common/logging/src/lib.rs | 27 +++++++-- common/logging/src/tracing_logging_layer.rs | 61 +-------------------- lighthouse/src/main.rs | 7 --- 3 files changed, 23 insertions(+), 72 deletions(-) diff --git a/common/logging/src/lib.rs b/common/logging/src/lib.rs index 5217d541cf..caf3e1d2fb 100644 --- a/common/logging/src/lib.rs +++ b/common/logging/src/lib.rs @@ -10,6 +10,7 @@ use std::io::{Result, Write}; use std::path::PathBuf; use std::time::{Duration, Instant}; use tracing_appender::non_blocking::NonBlocking; +use tracing_appender::rolling::{RollingFileAppender, Rotation}; use tracing_logging_layer::LoggingLayer; use tracing_subscriber::{layer::SubscriberExt, util::SubscriberInitExt}; @@ -21,7 +22,6 @@ mod tracing_logging_layer; mod tracing_metrics_layer; pub use sse_logging_components::SSELoggingComponents; -pub use tracing_logging_layer::cleanup_logging_task; pub use tracing_metrics_layer::MetricsLayer; /// The minimum interval between log messages indicating that a queue is full. @@ -234,10 +234,27 @@ pub fn create_tracing_layer(base_tracing_log_path: PathBuf, turn_on_terminal_log } }; - let libp2p_writer = - tracing_appender::rolling::daily(base_tracing_log_path.clone(), "libp2p.log"); - let discv5_writer = - tracing_appender::rolling::daily(base_tracing_log_path.clone(), "discv5.log"); + let Ok(libp2p_writer) = RollingFileAppender::builder() + .rotation(Rotation::DAILY) + .max_log_files(2) + .filename_prefix("libp2p") + .filename_suffix("log") + .build(base_tracing_log_path.clone()) + else { + eprintln!("Failed to initialize libp2p rolling file appender"); + return; + }; + + let Ok(discv5_writer) = RollingFileAppender::builder() + .rotation(Rotation::DAILY) + .max_log_files(2) + .filename_prefix("discv5") + .filename_suffix("log") + .build(base_tracing_log_path.clone()) + else { + eprintln!("Failed to initialize discv5 rolling file appender"); + return; + }; let (libp2p_non_blocking_writer, libp2p_guard) = NonBlocking::new(libp2p_writer); let (discv5_non_blocking_writer, discv5_guard) = NonBlocking::new(discv5_writer); diff --git a/common/logging/src/tracing_logging_layer.rs b/common/logging/src/tracing_logging_layer.rs index a74e24bdb6..e7d9109beb 100644 --- a/common/logging/src/tracing_logging_layer.rs +++ b/common/logging/src/tracing_logging_layer.rs @@ -1,5 +1,4 @@ -use chrono::{naive::Days, prelude::*}; -use slog::{debug, warn}; +use chrono::prelude::*; use std::io::Write; use tracing::Subscriber; use tracing_appender::non_blocking::{NonBlocking, WorkerGuard}; @@ -55,61 +54,3 @@ impl tracing_core::field::Visit for LogMessageExtractor { self.message = format!("{} {:?}", self.message, value); } } - -/// Creates a long lived async task that routinely deletes old tracing log files -pub async fn cleanup_logging_task(path: std::path::PathBuf, log: slog::Logger) { - loop { - // Delay for 1 day and then prune old logs - tokio::time::sleep(std::time::Duration::from_secs(60 * 60 * 24)).await; - - let Some(yesterday_date) = chrono::prelude::Local::now() - .naive_local() - .checked_sub_days(Days::new(1)) - else { - warn!(log, "Could not calculate the current date"); - return; - }; - - // Search for old log files - let dir = path.as_path(); - - if dir.is_dir() { - let Ok(files) = std::fs::read_dir(dir) else { - warn!(log, "Could not read log directory contents"; "path" => ?dir); - break; - }; - - for file in files { - let Ok(dir_entry) = file else { - warn!(log, "Could not read file"); - continue; - }; - - let Ok(file_name) = dir_entry.file_name().into_string() else { - warn!(log, "Could not read file"; "file" => ?dir_entry); - continue; - }; - - if file_name.starts_with("libp2p.log") | file_name.starts_with("discv5.log") { - let log_file_date = file_name.split('.').collect::>(); - if log_file_date.len() == 3 { - let Ok(log_file_date_type) = - NaiveDate::parse_from_str(log_file_date[2], "%Y-%m-%d") - else { - warn!(log, "Could not parse log file date"; "file" => file_name); - continue; - }; - - if log_file_date_type < yesterday_date.into() { - // Delete the file, its too old - debug!(log, "Removing old log file"; "file" => &file_name); - if let Err(e) = std::fs::remove_file(dir_entry.path()) { - warn!(log, "Failed to remove log file"; "file" => file_name, "error" => %e); - } - } - } - } - } - } - } -} diff --git a/lighthouse/src/main.rs b/lighthouse/src/main.rs index b8cedfde0d..06eb06fc08 100644 --- a/lighthouse/src/main.rs +++ b/lighthouse/src/main.rs @@ -542,13 +542,6 @@ fn run( let turn_on_terminal_logs = matches.is_present("env_log"); - // Run a task to clean up old tracing logs. - let log_cleaner_context = environment.service_context("log_cleaner".to_string()); - log_cleaner_context.executor.spawn( - logging::cleanup_logging_task(path.clone(), log.clone()), - "log_cleaner", - ); - logging::create_tracing_layer(path, turn_on_terminal_logs); // Allow Prometheus to export the time at which the process was started. From f9e36c94edf69c87fafb912f81c124714855a84f Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Thu, 25 Jan 2024 00:09:47 +0200 Subject: [PATCH 24/25] Expose additional builder booster related flags in the vc (#5086) * expose builder booster flags in vc, enable options in validator endpoints, update tests * resolve failing test * fix issues related to CreateConfig and MoveConfig * remove unneeded val, change how boost factor flag logic in the vc, add some additional documentation * fix typos * fix typos * assume builder-proosals flag if one of other two vc builder flags are present * fmt * typo * typo * Fix CLI help text * Prioritise per validator builder boost configurations over CLI flags. * Add http test for builder boost factor with process defaults. * Fix issue with PATCH request * Add prefer builder proposals * Add more builder boost factor tests. --------- Co-authored-by: Mac L Co-authored-by: Jimmy Chen Co-authored-by: Paul Hauner --- account_manager/src/validator/import.rs | 2 + book/src/api-vc-endpoints.md | 2 +- book/src/builders.md | 18 +- book/src/help_vc.md | 6 + book/src/help_vm_create.md | 10 +- book/src/help_vm_move.md | 8 +- .../src/validator_definitions.rs | 13 + common/eth2/src/lighthouse_vc/http_client.rs | 5 + common/eth2/src/lighthouse_vc/types.rs | 24 ++ lighthouse/tests/account_manager.rs | 8 + lighthouse/tests/validator_client.rs | 26 ++ lighthouse/tests/validator_manager.rs | 47 ++++ testing/web3signer_tests/src/lib.rs | 4 + validator_client/src/block_service.rs | 39 ++- validator_client/src/cli.rs | 18 ++ validator_client/src/config.rs | 12 + .../src/http_api/create_validator.rs | 2 + validator_client/src/http_api/keystores.rs | 2 + validator_client/src/http_api/mod.rs | 12 + validator_client/src/http_api/remotekeys.rs | 2 + validator_client/src/http_api/test_utils.rs | 22 +- validator_client/src/http_api/tests.rs | 232 +++++++++++++++++- .../src/http_api/tests/keystores.rs | 4 +- .../src/initialized_validators.rs | 43 ++++ validator_client/src/validator_store.rs | 89 ++++++- validator_manager/src/common.rs | 6 + validator_manager/src/create_validators.rs | 39 +++ validator_manager/src/move_validators.rs | 41 +++- 28 files changed, 715 insertions(+), 21 deletions(-) diff --git a/account_manager/src/validator/import.rs b/account_manager/src/validator/import.rs index 339d9a2914..bf000385f3 100644 --- a/account_manager/src/validator/import.rs +++ b/account_manager/src/validator/import.rs @@ -284,6 +284,8 @@ pub fn cli_run(matches: &ArgMatches, validator_dir: PathBuf) -> Result<(), Strin suggested_fee_recipient, None, None, + None, + None, ) .map_err(|e| format!("Unable to create new validator definition: {:?}", e))?; diff --git a/book/src/api-vc-endpoints.md b/book/src/api-vc-endpoints.md index f41625ad88..d3b2edafe9 100644 --- a/book/src/api-vc-endpoints.md +++ b/book/src/api-vc-endpoints.md @@ -427,7 +427,7 @@ Example Response Body ## `PATCH /lighthouse/validators/:voting_pubkey` -Update some values for the validator with `voting_pubkey`. Possible fields: `enabled`, `gas_limit`, `builder_proposals`, +Update some values for the validator with `voting_pubkey`. Possible fields: `enabled`, `gas_limit`, `builder_proposals`, `builder_boost_factor`, `prefer_builder_proposals` and `graffiti`. The following example updates a validator from `enabled: true` to `enabled: false`. ### HTTP Specification diff --git a/book/src/builders.md b/book/src/builders.md index e48cc0a884..014e432117 100644 --- a/book/src/builders.md +++ b/book/src/builders.md @@ -31,6 +31,18 @@ blinded blocks, you should use the following flag: lighthouse vc --builder-proposals ``` With the `--builder-proposals` flag, the validator client will ask for blinded blocks for all validators it manages. + +``` +lighthouse vc --prefer-builder-proposals +``` +With the `--prefer-builder-proposals` flag, the validator client will always prefer blinded blocks, regardless of the payload value, for all validators it manages. + +``` +lighthouse vc --builder-boost-factor +``` +With the `--builder-boost-factor` flag, a percentage multiplier is applied to the builder's payload value when choosing between a +builder payload header and payload from the paired execution node. + In order to configure whether a validator queries for blinded blocks check out [this section.](#validator-client-configuration) ## Multiple builders @@ -46,9 +58,9 @@ relays, run one of the following services and configure lighthouse to use it wit In the validator client you can configure gas limit and fee recipient on a per-validator basis. If no gas limit is configured, Lighthouse will use a default gas limit of 30,000,000, which is the current default value used in execution engines. You can also enable or disable use of external builders on a per-validator basis rather than using -`--builder-proposals`, which enables external builders for all validators. In order to manage these configurations -per-validator, you can either make updates to the `validator_definitions.yml` file or you can use the HTTP requests -described below. +`--builder-proposals`, `--builder-boost-factor` or `--prefer-builder-proposals`, which apply builder related preferences for all validators. +In order to manage these configurations per-validator, you can either make updates to the `validator_definitions.yml` file +or you can use the HTTP requests described below. Both the gas limit and fee recipient will be passed along as suggestions to connected builders. If there is a discrepancy in either, it will *not* keep you from proposing a block with the builder. This is because the bounds on gas limit are diff --git a/book/src/help_vc.md b/book/src/help_vc.md index 62b64efd41..bc6deec1e9 100644 --- a/book/src/help_vc.md +++ b/book/src/help_vc.md @@ -56,6 +56,9 @@ FLAGS: machine. Note that logs can often contain sensitive information about your validator and so this flag should be used with caution. For Windows users, the log file permissions will be inherited from the parent folder. --metrics Enable the Prometheus metrics HTTP server. Disabled by default. + --prefer-builder-proposals + If this flag is set, Lighthouse will always prefer blocks constructed by builders, regardless of payload + value. --produce-block-v3 Enable block production via the block v3 endpoint for this validator client. This should only be enabled when paired with a beacon node that has this endpoint implemented. This flag will be enabled by default in @@ -80,6 +83,9 @@ OPTIONS: Comma-separated list of beacon API topics to broadcast to all beacon nodes. Possible values are: none, attestations, blocks, subscriptions, sync-committee. Default (when flag is omitted) is to broadcast subscriptions only. + --builder-boost-factor + Defines the boost factor, a percentage multiplier to apply to the builder's payload value when choosing + between a builder payload header and payload from the local execution node. --builder-registration-timestamp-override This flag takes a unix timestamp value that will be used to override the timestamp used in the builder api registration diff --git a/book/src/help_vm_create.md b/book/src/help_vm_create.md index 505ea8638f..71db3cc599 100644 --- a/book/src/help_vm_create.md +++ b/book/src/help_vm_create.md @@ -42,6 +42,9 @@ OPTIONS: A HTTP(S) address of a beacon node using the beacon-API. If this value is provided, an error will be raised if any validator key here is already known as a validator by that beacon node. This helps prevent the same validator being created twice and therefore slashable conditions. + --builder-boost-factor + Defines the boost factor, a percentage multiplier to apply to the builder's payload value when choosing + between a builder payload header and payload from the local execution node. --builder-proposals When provided, all created validators will attempt to create blocks via builder rather than the local EL. [possible values: true, false] @@ -93,13 +96,18 @@ OPTIONS: --logfile-max-size The maximum size (in MB) each log file can grow to before rotating. If set to 0, background file logging is disabled. [default: 200] - --mnemonic-path If present, the mnemonic will be read in from this file. + --mnemonic-path + If present, the mnemonic will be read in from this file. + --network Name of the Eth2 chain Lighthouse will sync and follow. [possible values: mainnet, prater, goerli, gnosis, chiado, sepolia, holesky] --output-path The path to a directory where the validator and (optionally) deposits files will be created. The directory will be created if it does not exist. + --prefer-builder-proposals + If this flag is set, Lighthouse will always prefer blocks constructed by builders, regardless of payload + value. [possible values: true, false] --safe-slots-to-import-optimistically Used to coordinate manual overrides of the SAFE_SLOTS_TO_IMPORT_OPTIMISTICALLY parameter. This flag should only be used if the user has a clear understanding that the broad Ethereum community has elected to override diff --git a/book/src/help_vm_move.md b/book/src/help_vm_move.md index dea440dca9..a89af437a9 100644 --- a/book/src/help_vm_move.md +++ b/book/src/help_vm_move.md @@ -26,10 +26,13 @@ FLAGS: -V, --version Prints version information OPTIONS: + --builder-boost-factor + Defines the boost factor, a percentage multiplier to apply to the builder's payload value when choosing + between a builder payload header and payload from the local execution node. --builder-proposals When provided, all created validators will attempt to create blocks via builder rather than the local EL. [possible values: true, false] - --count The number of validators to move. + --count The number of validators to move. -d, --datadir Used to specify a custom root data directory for lighthouse keys and databases. Defaults to $HOME/.lighthouse/{network} where network is the value of the `network` flag Note: Users should specify @@ -75,6 +78,9 @@ OPTIONS: --network Name of the Eth2 chain Lighthouse will sync and follow. [possible values: mainnet, prater, goerli, gnosis, chiado, sepolia, holesky] + --prefer-builder-proposals + If this flag is set, Lighthouse will always prefer blocks constructed by builders, regardless of payload + value. [possible values: true, false] --safe-slots-to-import-optimistically Used to coordinate manual overrides of the SAFE_SLOTS_TO_IMPORT_OPTIMISTICALLY parameter. This flag should only be used if the user has a clear understanding that the broad Ethereum community has elected to override diff --git a/common/account_utils/src/validator_definitions.rs b/common/account_utils/src/validator_definitions.rs index 61c65a29ba..f228ce5fdf 100644 --- a/common/account_utils/src/validator_definitions.rs +++ b/common/account_utils/src/validator_definitions.rs @@ -157,6 +157,12 @@ pub struct ValidatorDefinition { #[serde(skip_serializing_if = "Option::is_none")] pub builder_proposals: Option, #[serde(default)] + #[serde(skip_serializing_if = "Option::is_none")] + pub builder_boost_factor: Option, + #[serde(default)] + #[serde(skip_serializing_if = "Option::is_none")] + pub prefer_builder_proposals: Option, + #[serde(default)] pub description: String, #[serde(flatten)] pub signing_definition: SigningDefinition, @@ -169,6 +175,7 @@ impl ValidatorDefinition { /// ## Notes /// /// This function does not check the password against the keystore. + #[allow(clippy::too_many_arguments)] pub fn new_keystore_with_password>( voting_keystore_path: P, voting_keystore_password_storage: PasswordStorage, @@ -176,6 +183,8 @@ impl ValidatorDefinition { suggested_fee_recipient: Option
, gas_limit: Option, builder_proposals: Option, + builder_boost_factor: Option, + prefer_builder_proposals: Option, ) -> Result { let voting_keystore_path = voting_keystore_path.as_ref().into(); let keystore = @@ -196,6 +205,8 @@ impl ValidatorDefinition { suggested_fee_recipient, gas_limit, builder_proposals, + builder_boost_factor, + prefer_builder_proposals, signing_definition: SigningDefinition::LocalKeystore { voting_keystore_path, voting_keystore_password_path, @@ -344,6 +355,8 @@ impl ValidatorDefinitions { suggested_fee_recipient: None, gas_limit: None, builder_proposals: None, + builder_boost_factor: None, + prefer_builder_proposals: None, signing_definition: SigningDefinition::LocalKeystore { voting_keystore_path, voting_keystore_password_path, diff --git a/common/eth2/src/lighthouse_vc/http_client.rs b/common/eth2/src/lighthouse_vc/http_client.rs index 2e6756c63e..83aeea4bfc 100644 --- a/common/eth2/src/lighthouse_vc/http_client.rs +++ b/common/eth2/src/lighthouse_vc/http_client.rs @@ -483,12 +483,15 @@ impl ValidatorClientHttpClient { } /// `PATCH lighthouse/validators/{validator_pubkey}` + #[allow(clippy::too_many_arguments)] pub async fn patch_lighthouse_validators( &self, voting_pubkey: &PublicKeyBytes, enabled: Option, gas_limit: Option, builder_proposals: Option, + builder_boost_factor: Option, + prefer_builder_proposals: Option, graffiti: Option, ) -> Result<(), Error> { let mut path = self.server.full.clone(); @@ -505,6 +508,8 @@ impl ValidatorClientHttpClient { enabled, gas_limit, builder_proposals, + builder_boost_factor, + prefer_builder_proposals, graffiti, }, ) diff --git a/common/eth2/src/lighthouse_vc/types.rs b/common/eth2/src/lighthouse_vc/types.rs index 230293f1b8..d903d7b73d 100644 --- a/common/eth2/src/lighthouse_vc/types.rs +++ b/common/eth2/src/lighthouse_vc/types.rs @@ -32,6 +32,12 @@ pub struct ValidatorRequest { #[serde(default)] #[serde(skip_serializing_if = "Option::is_none")] pub builder_proposals: Option, + #[serde(default)] + #[serde(skip_serializing_if = "Option::is_none")] + pub builder_boost_factor: Option, + #[serde(default)] + #[serde(skip_serializing_if = "Option::is_none")] + pub prefer_builder_proposals: Option, #[serde(with = "serde_utils::quoted_u64")] pub deposit_gwei: u64, } @@ -86,6 +92,12 @@ pub struct ValidatorPatchRequest { #[serde(default)] #[serde(skip_serializing_if = "Option::is_none")] pub graffiti: Option, + #[serde(default)] + #[serde(skip_serializing_if = "Option::is_none")] + pub builder_boost_factor: Option, + #[serde(default)] + #[serde(skip_serializing_if = "Option::is_none")] + pub prefer_builder_proposals: Option, } #[derive(Clone, PartialEq, Serialize, Deserialize)] @@ -105,6 +117,12 @@ pub struct KeystoreValidatorsPostRequest { #[serde(default)] #[serde(skip_serializing_if = "Option::is_none")] pub builder_proposals: Option, + #[serde(default)] + #[serde(skip_serializing_if = "Option::is_none")] + pub builder_boost_factor: Option, + #[serde(default)] + #[serde(skip_serializing_if = "Option::is_none")] + pub prefer_builder_proposals: Option, } #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] @@ -135,6 +153,12 @@ pub struct Web3SignerValidatorRequest { pub client_identity_path: Option, #[serde(skip_serializing_if = "Option::is_none")] pub client_identity_password: Option, + #[serde(default)] + #[serde(skip_serializing_if = "Option::is_none")] + pub builder_boost_factor: Option, + #[serde(default)] + #[serde(skip_serializing_if = "Option::is_none")] + pub prefer_builder_proposals: Option, } #[derive(Debug, Clone, PartialEq, Deserialize, Serialize)] diff --git a/lighthouse/tests/account_manager.rs b/lighthouse/tests/account_manager.rs index 63d79fceb2..f82e3ec713 100644 --- a/lighthouse/tests/account_manager.rs +++ b/lighthouse/tests/account_manager.rs @@ -492,6 +492,8 @@ fn validator_import_launchpad() { suggested_fee_recipient: None, gas_limit: None, builder_proposals: None, + builder_boost_factor: None, + prefer_builder_proposals: None, voting_public_key: keystore.public_key().unwrap(), signing_definition: SigningDefinition::LocalKeystore { voting_keystore_path, @@ -614,6 +616,8 @@ fn validator_import_launchpad_no_password_then_add_password() { suggested_fee_recipient: None, gas_limit: None, builder_proposals: None, + builder_boost_factor: None, + prefer_builder_proposals: None, voting_public_key: keystore.public_key().unwrap(), signing_definition: SigningDefinition::LocalKeystore { voting_keystore_path, @@ -640,6 +644,8 @@ fn validator_import_launchpad_no_password_then_add_password() { suggested_fee_recipient: None, gas_limit: None, builder_proposals: None, + builder_boost_factor: None, + prefer_builder_proposals: None, voting_public_key: keystore.public_key().unwrap(), signing_definition: SigningDefinition::LocalKeystore { voting_keystore_path: dst_keystore_dir.join(KEYSTORE_NAME), @@ -742,6 +748,8 @@ fn validator_import_launchpad_password_file() { suggested_fee_recipient: None, gas_limit: None, builder_proposals: None, + builder_boost_factor: None, + prefer_builder_proposals: None, signing_definition: SigningDefinition::LocalKeystore { voting_keystore_path, voting_keystore_password_path: None, diff --git a/lighthouse/tests/validator_client.rs b/lighthouse/tests/validator_client.rs index 025188fed4..701aed07ed 100644 --- a/lighthouse/tests/validator_client.rs +++ b/lighthouse/tests/validator_client.rs @@ -464,6 +464,32 @@ fn builder_proposals_flag() { .with_config(|config| assert!(config.builder_proposals)); } #[test] +fn builder_boost_factor_flag() { + CommandLineTest::new() + .flag("builder-boost-factor", Some("150")) + .run() + .with_config(|config| assert_eq!(config.builder_boost_factor, Some(150))); +} +#[test] +fn no_builder_boost_factor_flag() { + CommandLineTest::new() + .run() + .with_config(|config| assert_eq!(config.builder_boost_factor, None)); +} +#[test] +fn prefer_builder_proposals_flag() { + CommandLineTest::new() + .flag("prefer-builder-proposals", None) + .run() + .with_config(|config| assert!(config.prefer_builder_proposals)); +} +#[test] +fn no_prefer_builder_proposals_flag() { + CommandLineTest::new() + .run() + .with_config(|config| assert!(!config.prefer_builder_proposals)); +} +#[test] fn no_builder_registration_timestamp_override_flag() { CommandLineTest::new() .run() diff --git a/lighthouse/tests/validator_manager.rs b/lighthouse/tests/validator_manager.rs index e0a1e92d6a..fab1cfebf4 100644 --- a/lighthouse/tests/validator_manager.rs +++ b/lighthouse/tests/validator_manager.rs @@ -122,6 +122,8 @@ pub fn validator_create_defaults() { specify_voting_keystore_password: false, eth1_withdrawal_address: None, builder_proposals: None, + builder_boost_factor: None, + prefer_builder_proposals: None, fee_recipient: None, gas_limit: None, bn_url: None, @@ -143,6 +145,8 @@ pub fn validator_create_misc_flags() { .flag("--specify-voting-keystore-password", None) .flag("--eth1-withdrawal-address", Some(EXAMPLE_ETH1_ADDRESS)) .flag("--builder-proposals", Some("true")) + .flag("--prefer-builder-proposals", Some("true")) + .flag("--builder-boost-factor", Some("150")) .flag("--suggested-fee-recipient", Some(EXAMPLE_ETH1_ADDRESS)) .flag("--gas-limit", Some("1337")) .flag("--beacon-node", Some("http://localhost:1001")) @@ -159,6 +163,8 @@ pub fn validator_create_misc_flags() { specify_voting_keystore_password: true, eth1_withdrawal_address: Some(Address::from_str(EXAMPLE_ETH1_ADDRESS).unwrap()), builder_proposals: Some(true), + builder_boost_factor: Some(150), + prefer_builder_proposals: Some(true), fee_recipient: Some(Address::from_str(EXAMPLE_ETH1_ADDRESS).unwrap()), gas_limit: Some(1337), bn_url: Some(SensitiveUrl::parse("http://localhost:1001").unwrap()), @@ -244,6 +250,8 @@ pub fn validator_move_defaults() { dest_vc_token_path: PathBuf::from("./2.json"), validators: Validators::All, builder_proposals: None, + builder_boost_factor: None, + prefer_builder_proposals: None, fee_recipient: None, gas_limit: None, password_source: PasswordSource::Interactive { @@ -280,6 +288,8 @@ pub fn validator_move_misc_flags_0() { PublicKeyBytes::from_str(EXAMPLE_PUBKEY_1).unwrap(), ]), builder_proposals: Some(true), + builder_boost_factor: None, + prefer_builder_proposals: None, fee_recipient: Some(Address::from_str(EXAMPLE_ETH1_ADDRESS).unwrap()), gas_limit: Some(1337), password_source: PasswordSource::Interactive { stdin_inputs: true }, @@ -297,6 +307,7 @@ pub fn validator_move_misc_flags_1() { .flag("--dest-vc-token", Some("./2.json")) .flag("--validators", Some(&format!("{}", EXAMPLE_PUBKEY_0))) .flag("--builder-proposals", Some("false")) + .flag("--prefer-builder-proposals", Some("false")) .assert_success(|config| { let expected = MoveConfig { src_vc_url: SensitiveUrl::parse("http://localhost:1").unwrap(), @@ -307,6 +318,40 @@ pub fn validator_move_misc_flags_1() { PublicKeyBytes::from_str(EXAMPLE_PUBKEY_0).unwrap() ]), builder_proposals: Some(false), + builder_boost_factor: None, + prefer_builder_proposals: Some(false), + fee_recipient: None, + gas_limit: None, + password_source: PasswordSource::Interactive { + stdin_inputs: cfg!(windows) || false, + }, + }; + assert_eq!(expected, config); + }); +} + +#[test] +pub fn validator_move_misc_flags_2() { + CommandLineTest::validators_move() + .flag("--src-vc-url", Some("http://localhost:1")) + .flag("--src-vc-token", Some("./1.json")) + .flag("--dest-vc-url", Some("http://localhost:2")) + .flag("--dest-vc-token", Some("./2.json")) + .flag("--validators", Some(&format!("{}", EXAMPLE_PUBKEY_0))) + .flag("--builder-proposals", Some("false")) + .flag("--builder-boost-factor", Some("100")) + .assert_success(|config| { + let expected = MoveConfig { + src_vc_url: SensitiveUrl::parse("http://localhost:1").unwrap(), + src_vc_token_path: PathBuf::from("./1.json"), + dest_vc_url: SensitiveUrl::parse("http://localhost:2").unwrap(), + dest_vc_token_path: PathBuf::from("./2.json"), + validators: Validators::Specific(vec![ + PublicKeyBytes::from_str(EXAMPLE_PUBKEY_0).unwrap() + ]), + builder_proposals: Some(false), + builder_boost_factor: Some(100), + prefer_builder_proposals: None, fee_recipient: None, gas_limit: None, password_source: PasswordSource::Interactive { @@ -333,6 +378,8 @@ pub fn validator_move_count() { dest_vc_token_path: PathBuf::from("./2.json"), validators: Validators::Count(42), builder_proposals: None, + builder_boost_factor: None, + prefer_builder_proposals: None, fee_recipient: None, gas_limit: None, password_source: PasswordSource::Interactive { diff --git a/testing/web3signer_tests/src/lib.rs b/testing/web3signer_tests/src/lib.rs index 463de0c8b3..6f3536fe46 100644 --- a/testing/web3signer_tests/src/lib.rs +++ b/testing/web3signer_tests/src/lib.rs @@ -391,6 +391,8 @@ mod tests { suggested_fee_recipient: None, gas_limit: None, builder_proposals: None, + builder_boost_factor: None, + prefer_builder_proposals: None, description: String::default(), signing_definition: SigningDefinition::LocalKeystore { voting_keystore_path: signer_rig.keystore_path.clone(), @@ -409,6 +411,8 @@ mod tests { suggested_fee_recipient: None, gas_limit: None, builder_proposals: None, + builder_boost_factor: None, + prefer_builder_proposals: None, description: String::default(), signing_definition: SigningDefinition::Web3Signer(Web3SignerDefinition { url: signer_rig.url.to_string(), diff --git a/validator_client/src/block_service.rs b/validator_client/src/block_service.rs index e0c98660e2..445d4f1a5d 100644 --- a/validator_client/src/block_service.rs +++ b/validator_client/src/block_service.rs @@ -325,14 +325,7 @@ impl BlockService { if self.validator_store.produce_block_v3() { for validator_pubkey in proposers { - let builder_proposals = self - .validator_store - .get_builder_proposals(&validator_pubkey); - // Translate `builder_proposals` to a boost factor. Builder proposals set to `true` - // requires no boost factor, it just means "use a builder proposal if the BN returns - // one". On the contrary, `builder_proposals: false` indicates a preference for - // local payloads, so we set the builder boost factor to 0. - let builder_boost_factor = if !builder_proposals { Some(0) } else { None }; + let builder_boost_factor = self.get_builder_boost_factor(&validator_pubkey); let service = self.clone(); let log = log.clone(); self.inner.context.executor.spawn( @@ -853,6 +846,36 @@ impl BlockService { Ok::<_, BlockError>(unsigned_block) } + + /// Returns the builder boost factor of the given public key. + /// The priority order for fetching this value is: + /// + /// 1. validator_definitions.yml + /// 2. process level flag + fn get_builder_boost_factor(&self, validator_pubkey: &PublicKeyBytes) -> Option { + // Apply per validator configuration first. + let validator_builder_boost_factor = self + .validator_store + .determine_validator_builder_boost_factor(validator_pubkey); + + // Fallback to process-wide configuration if needed. + let maybe_builder_boost_factor = validator_builder_boost_factor.or_else(|| { + self.validator_store + .determine_default_builder_boost_factor() + }); + + if let Some(builder_boost_factor) = maybe_builder_boost_factor { + // if builder boost factor is set to 100 it should be treated + // as None to prevent unnecessary calculations that could + // lead to loss of information. + if builder_boost_factor == 100 { + return None; + } + return Some(builder_boost_factor); + } + + None + } } pub enum UnsignedBlock { diff --git a/validator_client/src/cli.rs b/validator_client/src/cli.rs index cd3ad494da..e6d19bc898 100644 --- a/validator_client/src/cli.rs +++ b/validator_client/src/cli.rs @@ -349,4 +349,22 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { .default_value("500") .takes_value(true), ) + .arg( + Arg::with_name("builder-boost-factor") + .long("builder-boost-factor") + .value_name("UINT64") + .help("Defines the boost factor, \ + a percentage multiplier to apply to the builder's payload value \ + when choosing between a builder payload header and payload from \ + the local execution node.") + .conflicts_with("prefer-builder-proposals") + .takes_value(true), + ) + .arg( + Arg::with_name("prefer-builder-proposals") + .long("prefer-builder-proposals") + .help("If this flag is set, Lighthouse will always prefer blocks \ + constructed by builders, regardless of payload value.") + .takes_value(false), + ) } diff --git a/validator_client/src/config.rs b/validator_client/src/config.rs index 4b7da76428..a919afb018 100644 --- a/validator_client/src/config.rs +++ b/validator_client/src/config.rs @@ -77,6 +77,10 @@ pub struct Config { pub validator_registration_batch_size: usize, /// Enables block production via the block v3 endpoint. This configuration option can be removed post deneb. pub produce_block_v3: bool, + /// Specifies the boost factor, a percentage multiplier to apply to the builder's payload value. + pub builder_boost_factor: Option, + /// If true, Lighthouse will prefer builder proposals, if available. + pub prefer_builder_proposals: bool, } impl Default for Config { @@ -118,6 +122,8 @@ impl Default for Config { enable_latency_measurement_service: true, validator_registration_batch_size: 500, produce_block_v3: false, + builder_boost_factor: None, + prefer_builder_proposals: false, } } } @@ -346,6 +352,10 @@ impl Config { config.produce_block_v3 = true; } + if cli_args.is_present("prefer-builder-proposals") { + config.prefer_builder_proposals = true; + } + config.gas_limit = cli_args .value_of("gas-limit") .map(|gas_limit| { @@ -365,6 +375,8 @@ impl Config { ); } + config.builder_boost_factor = parse_optional(cli_args, "builder-boost-factor")?; + config.enable_latency_measurement_service = parse_optional(cli_args, "latency-measurement-service")?.unwrap_or(true); diff --git a/validator_client/src/http_api/create_validator.rs b/validator_client/src/http_api/create_validator.rs index 52336afa59..afa5d4fed1 100644 --- a/validator_client/src/http_api/create_validator.rs +++ b/validator_client/src/http_api/create_validator.rs @@ -148,6 +148,8 @@ pub async fn create_validators_mnemonic, T: 'static + SlotClock, request.suggested_fee_recipient, request.gas_limit, request.builder_proposals, + request.builder_boost_factor, + request.prefer_builder_proposals, ) .await .map_err(|e| { diff --git a/validator_client/src/http_api/keystores.rs b/validator_client/src/http_api/keystores.rs index c2d9b4d67f..074c578347 100644 --- a/validator_client/src/http_api/keystores.rs +++ b/validator_client/src/http_api/keystores.rs @@ -224,6 +224,8 @@ fn import_single_keystore( None, None, None, + None, + None, )) .map_err(|e| format!("failed to initialize validator: {:?}", e))?; diff --git a/validator_client/src/http_api/mod.rs b/validator_client/src/http_api/mod.rs index c65beb7390..dcf66d2fbc 100644 --- a/validator_client/src/http_api/mod.rs +++ b/validator_client/src/http_api/mod.rs @@ -565,6 +565,8 @@ pub fn serve( let suggested_fee_recipient = body.suggested_fee_recipient; let gas_limit = body.gas_limit; let builder_proposals = body.builder_proposals; + let builder_boost_factor = body.builder_boost_factor; + let prefer_builder_proposals = body.prefer_builder_proposals; let validator_def = { if let Some(handle) = task_executor.handle() { @@ -577,6 +579,8 @@ pub fn serve( suggested_fee_recipient, gas_limit, builder_proposals, + builder_boost_factor, + prefer_builder_proposals, )) .map_err(|e| { warp_utils::reject::custom_server_error(format!( @@ -625,6 +629,8 @@ pub fn serve( suggested_fee_recipient: web3signer.suggested_fee_recipient, gas_limit: web3signer.gas_limit, builder_proposals: web3signer.builder_proposals, + builder_boost_factor: web3signer.builder_boost_factor, + prefer_builder_proposals: web3signer.prefer_builder_proposals, description: web3signer.description, signing_definition: SigningDefinition::Web3Signer( Web3SignerDefinition { @@ -691,8 +697,12 @@ pub fn serve( (Some(is_enabled), Some(initialized_validator)) if Some(is_enabled) == body.enabled && initialized_validator.get_gas_limit() == body.gas_limit + && initialized_validator.get_builder_boost_factor() + == body.builder_boost_factor && initialized_validator.get_builder_proposals() == body.builder_proposals + && initialized_validator.get_prefer_builder_proposals() + == body.prefer_builder_proposals && initialized_validator.get_graffiti() == maybe_graffiti => { Ok(()) @@ -706,6 +716,8 @@ pub fn serve( body.enabled, body.gas_limit, body.builder_proposals, + body.builder_boost_factor, + body.prefer_builder_proposals, body.graffiti, ), ) diff --git a/validator_client/src/http_api/remotekeys.rs b/validator_client/src/http_api/remotekeys.rs index 991dfb8bf7..053bbcb4b2 100644 --- a/validator_client/src/http_api/remotekeys.rs +++ b/validator_client/src/http_api/remotekeys.rs @@ -125,6 +125,8 @@ fn import_single_remotekey( suggested_fee_recipient: None, gas_limit: None, builder_proposals: None, + builder_boost_factor: None, + prefer_builder_proposals: None, description: String::from("Added by remotekey API"), signing_definition: SigningDefinition::Web3Signer(Web3SignerDefinition { url, diff --git a/validator_client/src/http_api/test_utils.rs b/validator_client/src/http_api/test_utils.rs index 916c098cdd..7b0cb51ec1 100644 --- a/validator_client/src/http_api/test_utils.rs +++ b/validator_client/src/http_api/test_utils.rs @@ -315,6 +315,8 @@ impl ApiTester { suggested_fee_recipient: None, gas_limit: None, builder_proposals: None, + builder_boost_factor: None, + prefer_builder_proposals: None, deposit_gwei: E::default_spec().max_effective_balance, }) .collect::>(); @@ -447,6 +449,8 @@ impl ApiTester { suggested_fee_recipient: None, gas_limit: None, builder_proposals: None, + builder_boost_factor: None, + prefer_builder_proposals: None, }; self.client @@ -467,6 +471,8 @@ impl ApiTester { suggested_fee_recipient: None, gas_limit: None, builder_proposals: None, + builder_boost_factor: None, + prefer_builder_proposals: None, }; let response = self @@ -511,6 +517,8 @@ impl ApiTester { request_timeout_ms: None, client_identity_path: None, client_identity_password: None, + builder_boost_factor: None, + prefer_builder_proposals: None, } }) .collect(); @@ -534,7 +542,15 @@ impl ApiTester { let validator = &self.client.get_lighthouse_validators().await.unwrap().data[index]; self.client - .patch_lighthouse_validators(&validator.voting_pubkey, Some(enabled), None, None, None) + .patch_lighthouse_validators( + &validator.voting_pubkey, + Some(enabled), + None, + None, + None, + None, + None, + ) .await .unwrap(); @@ -582,6 +598,8 @@ impl ApiTester { Some(gas_limit), None, None, + None, + None, ) .await .unwrap(); @@ -610,6 +628,8 @@ impl ApiTester { None, Some(builder_proposals), None, + None, + None, ) .await .unwrap(); diff --git a/validator_client/src/http_api/tests.rs b/validator_client/src/http_api/tests.rs index 7de3cea21f..f7db76e4ad 100644 --- a/validator_client/src/http_api/tests.rs +++ b/validator_client/src/http_api/tests.rs @@ -52,6 +52,12 @@ struct ApiTester { impl ApiTester { pub async fn new() -> Self { + let mut config = Config::default(); + config.fee_recipient = Some(TEST_DEFAULT_FEE_RECIPIENT); + Self::new_with_config(config).await + } + + pub async fn new_with_config(mut config: Config) -> Self { let log = test_logger(); let validator_dir = tempdir().unwrap(); @@ -70,10 +76,8 @@ impl ApiTester { let api_secret = ApiSecret::create_or_open(validator_dir.path()).unwrap(); let api_pubkey = api_secret.api_token(); - let mut config = Config::default(); config.validator_dir = validator_dir.path().into(); config.secrets_dir = secrets_dir.path().into(); - config.fee_recipient = Some(TEST_DEFAULT_FEE_RECIPIENT); let spec = E::default_spec(); @@ -271,6 +275,8 @@ impl ApiTester { suggested_fee_recipient: None, gas_limit: None, builder_proposals: None, + builder_boost_factor: None, + prefer_builder_proposals: None, deposit_gwei: E::default_spec().max_effective_balance, }) .collect::>(); @@ -404,6 +410,8 @@ impl ApiTester { suggested_fee_recipient: None, gas_limit: None, builder_proposals: None, + builder_boost_factor: None, + prefer_builder_proposals: None, }; self.client @@ -424,6 +432,8 @@ impl ApiTester { suggested_fee_recipient: None, gas_limit: None, builder_proposals: None, + builder_boost_factor: None, + prefer_builder_proposals: None, }; let response = self @@ -462,6 +472,8 @@ impl ApiTester { suggested_fee_recipient: None, gas_limit: None, builder_proposals: None, + builder_boost_factor: None, + prefer_builder_proposals: None, voting_public_key: kp.pk, url: format!("http://signer_{}.com/", i), root_certificate_path: None, @@ -518,7 +530,15 @@ impl ApiTester { let validator = &self.client.get_lighthouse_validators().await.unwrap().data[index]; self.client - .patch_lighthouse_validators(&validator.voting_pubkey, Some(enabled), None, None, None) + .patch_lighthouse_validators( + &validator.voting_pubkey, + Some(enabled), + None, + None, + None, + None, + None, + ) .await .unwrap(); @@ -566,6 +586,8 @@ impl ApiTester { Some(gas_limit), None, None, + None, + None, ) .await .unwrap(); @@ -594,6 +616,50 @@ impl ApiTester { None, Some(builder_proposals), None, + None, + None, + ) + .await + .unwrap(); + + self + } + + pub async fn set_builder_boost_factor(self, index: usize, builder_boost_factor: u64) -> Self { + let validator = &self.client.get_lighthouse_validators().await.unwrap().data[index]; + + self.client + .patch_lighthouse_validators( + &validator.voting_pubkey, + None, + None, + None, + Some(builder_boost_factor), + None, + None, + ) + .await + .unwrap(); + + self + } + + pub async fn set_prefer_builder_proposals( + self, + index: usize, + prefer_builder_proposals: bool, + ) -> Self { + let validator = &self.client.get_lighthouse_validators().await.unwrap().data[index]; + + self.client + .patch_lighthouse_validators( + &validator.voting_pubkey, + None, + None, + None, + None, + Some(prefer_builder_proposals), + None, ) .await .unwrap(); @@ -613,6 +679,64 @@ impl ApiTester { self } + pub async fn assert_builder_boost_factor( + self, + index: usize, + builder_boost_factor: Option, + ) -> Self { + let validator = &self.client.get_lighthouse_validators().await.unwrap().data[index]; + + assert_eq!( + self.validator_store + .get_builder_boost_factor(&validator.voting_pubkey), + builder_boost_factor + ); + + self + } + + pub async fn assert_validator_derived_builder_boost_factor( + self, + index: usize, + builder_boost_factor: Option, + ) -> Self { + let validator = &self.client.get_lighthouse_validators().await.unwrap().data[index]; + + assert_eq!( + self.validator_store + .determine_validator_builder_boost_factor(&validator.voting_pubkey), + builder_boost_factor + ); + + self + } + + pub fn assert_default_builder_boost_factor(self, builder_boost_factor: Option) -> Self { + assert_eq!( + self.validator_store + .determine_default_builder_boost_factor(), + builder_boost_factor + ); + + self + } + + pub async fn assert_prefer_builder_proposals( + self, + index: usize, + prefer_builder_proposals: bool, + ) -> Self { + let validator = &self.client.get_lighthouse_validators().await.unwrap().data[index]; + + assert_eq!( + self.validator_store + .get_prefer_builder_proposals(&validator.voting_pubkey), + prefer_builder_proposals + ); + + self + } + pub async fn set_graffiti(self, index: usize, graffiti: &str) -> Self { let validator = &self.client.get_lighthouse_validators().await.unwrap().data[index]; let graffiti_str = GraffitiString::from_str(graffiti).unwrap(); @@ -622,6 +746,8 @@ impl ApiTester { None, None, None, + None, + None, Some(graffiti_str), ) .await @@ -741,6 +867,8 @@ async fn routes_with_invalid_auth() { gas_limit: <_>::default(), builder_proposals: <_>::default(), deposit_gwei: <_>::default(), + builder_boost_factor: <_>::default(), + prefer_builder_proposals: <_>::default(), }]) .await }) @@ -771,6 +899,8 @@ async fn routes_with_invalid_auth() { suggested_fee_recipient: <_>::default(), gas_limit: <_>::default(), builder_proposals: <_>::default(), + builder_boost_factor: <_>::default(), + prefer_builder_proposals: <_>::default(), }) .await }) @@ -783,6 +913,8 @@ async fn routes_with_invalid_auth() { None, None, None, + None, + None, ) .await }) @@ -980,6 +1112,100 @@ async fn validator_builder_proposals() { .await; } +#[tokio::test] +async fn validator_builder_boost_factor() { + ApiTester::new() + .await + .create_hd_validators(HdValidatorScenario { + count: 2, + specify_mnemonic: false, + key_derivation_path_offset: 0, + disabled: vec![], + }) + .await + .assert_enabled_validators_count(2) + .assert_validators_count(2) + .set_builder_boost_factor(0, 120) + .await + // Test setting builder proposals while the validator is disabled + .set_validator_enabled(0, false) + .await + .assert_enabled_validators_count(1) + .assert_validators_count(2) + .set_builder_boost_factor(0, 80) + .await + .set_validator_enabled(0, true) + .await + .assert_enabled_validators_count(2) + .assert_builder_boost_factor(0, Some(80)) + .await; +} + +/// Verifies the builder boost factors translated from the `builder_proposals`, +/// `prefer_builder_proposals` and `builder_boost_factor` values. +#[tokio::test] +async fn validator_derived_builder_boost_factor_with_process_defaults() { + let config = Config { + builder_proposals: true, + prefer_builder_proposals: false, + builder_boost_factor: Some(80), + ..Config::default() + }; + ApiTester::new_with_config(config) + .await + .create_hd_validators(HdValidatorScenario { + count: 3, + specify_mnemonic: false, + key_derivation_path_offset: 0, + disabled: vec![], + }) + .await + .assert_default_builder_boost_factor(Some(80)) + .assert_validator_derived_builder_boost_factor(0, None) + .await + .set_builder_proposals(0, false) + .await + .assert_validator_derived_builder_boost_factor(0, Some(0)) + .await + .set_builder_boost_factor(1, 120) + .await + .assert_validator_derived_builder_boost_factor(1, Some(120)) + .await + .set_prefer_builder_proposals(2, true) + .await + .assert_validator_derived_builder_boost_factor(2, Some(u64::MAX)) + .await; +} + +#[tokio::test] +async fn prefer_builder_proposals_validator() { + ApiTester::new() + .await + .create_hd_validators(HdValidatorScenario { + count: 2, + specify_mnemonic: false, + key_derivation_path_offset: 0, + disabled: vec![], + }) + .await + .assert_enabled_validators_count(2) + .assert_validators_count(2) + .set_prefer_builder_proposals(0, false) + .await + // Test setting builder proposals while the validator is disabled + .set_validator_enabled(0, false) + .await + .assert_enabled_validators_count(1) + .assert_validators_count(2) + .set_prefer_builder_proposals(0, true) + .await + .set_validator_enabled(0, true) + .await + .assert_enabled_validators_count(2) + .assert_prefer_builder_proposals(0, true) + .await; +} + #[tokio::test] async fn validator_graffiti() { ApiTester::new() diff --git a/validator_client/src/http_api/tests/keystores.rs b/validator_client/src/http_api/tests/keystores.rs index f301af1c21..fe58393bb8 100644 --- a/validator_client/src/http_api/tests/keystores.rs +++ b/validator_client/src/http_api/tests/keystores.rs @@ -43,6 +43,8 @@ fn web3signer_validator_with_pubkey(pubkey: PublicKey) -> Web3SignerValidatorReq suggested_fee_recipient: None, gas_limit: None, builder_proposals: None, + builder_boost_factor: None, + prefer_builder_proposals: None, voting_public_key: pubkey, url: web3_signer_url(), root_certificate_path: None, @@ -468,7 +470,7 @@ async fn import_and_delete_conflicting_web3_signer_keystores() { for pubkey in &pubkeys { tester .client - .patch_lighthouse_validators(pubkey, Some(false), None, None, None) + .patch_lighthouse_validators(pubkey, Some(false), None, None, None, None, None) .await .unwrap(); } diff --git a/validator_client/src/initialized_validators.rs b/validator_client/src/initialized_validators.rs index b65dad4c47..7e4331dc85 100644 --- a/validator_client/src/initialized_validators.rs +++ b/validator_client/src/initialized_validators.rs @@ -131,6 +131,8 @@ pub struct InitializedValidator { suggested_fee_recipient: Option
, gas_limit: Option, builder_proposals: Option, + builder_boost_factor: Option, + prefer_builder_proposals: Option, /// The validators index in `state.validators`, to be updated by an external service. index: Option, } @@ -159,6 +161,14 @@ impl InitializedValidator { self.gas_limit } + pub fn get_builder_boost_factor(&self) -> Option { + self.builder_boost_factor + } + + pub fn get_prefer_builder_proposals(&self) -> Option { + self.prefer_builder_proposals + } + pub fn get_builder_proposals(&self) -> Option { self.builder_proposals } @@ -335,6 +345,8 @@ impl InitializedValidator { suggested_fee_recipient: def.suggested_fee_recipient, gas_limit: def.gas_limit, builder_proposals: def.builder_proposals, + builder_boost_factor: def.builder_boost_factor, + prefer_builder_proposals: def.prefer_builder_proposals, index: None, }) } @@ -815,6 +827,22 @@ impl InitializedValidators { .and_then(|v| v.builder_proposals) } + /// Returns the `builder_boost_factor` for a given public key specified in the + /// `ValidatorDefinitions`. + pub fn builder_boost_factor(&self, public_key: &PublicKeyBytes) -> Option { + self.validators + .get(public_key) + .and_then(|v| v.builder_boost_factor) + } + + /// Returns the `prefer_builder_proposals` for a given public key specified in the + /// `ValidatorDefinitions`. + pub fn prefer_builder_proposals(&self, public_key: &PublicKeyBytes) -> Option { + self.validators + .get(public_key) + .and_then(|v| v.prefer_builder_proposals) + } + /// Returns an `Option` of a reference to an `InitializedValidator` for a given public key specified in the /// `ValidatorDefinitions`. pub fn validator(&self, public_key: &PublicKeyBytes) -> Option<&InitializedValidator> { @@ -835,12 +863,15 @@ impl InitializedValidators { /// or `InitializedValidator`. The same logic applies to `builder_proposals` and `graffiti`. /// /// Saves the `ValidatorDefinitions` to file, even if no definitions were changed. + #[allow(clippy::too_many_arguments)] pub async fn set_validator_definition_fields( &mut self, voting_public_key: &PublicKey, enabled: Option, gas_limit: Option, builder_proposals: Option, + builder_boost_factor: Option, + prefer_builder_proposals: Option, graffiti: Option, ) -> Result<(), Error> { if let Some(def) = self @@ -862,6 +893,12 @@ impl InitializedValidators { if let Some(graffiti) = graffiti.clone() { def.graffiti = Some(graffiti); } + if let Some(builder_boost_factor) = builder_boost_factor { + def.builder_boost_factor = Some(builder_boost_factor); + } + if let Some(prefer_builder_proposals) = prefer_builder_proposals { + def.prefer_builder_proposals = Some(prefer_builder_proposals); + } } self.update_validators().await?; @@ -880,6 +917,12 @@ impl InitializedValidators { if let Some(graffiti) = graffiti { val.graffiti = Some(graffiti.into()); } + if let Some(builder_boost_factor) = builder_boost_factor { + val.builder_boost_factor = Some(builder_boost_factor); + } + if let Some(prefer_builder_proposals) = prefer_builder_proposals { + val.prefer_builder_proposals = Some(prefer_builder_proposals); + } } self.definitions diff --git a/validator_client/src/validator_store.rs b/validator_client/src/validator_store.rs index 19726c2aec..c913b99060 100644 --- a/validator_client/src/validator_store.rs +++ b/validator_client/src/validator_store.rs @@ -98,6 +98,8 @@ pub struct ValidatorStore { gas_limit: Option, builder_proposals: bool, produce_block_v3: bool, + prefer_builder_proposals: bool, + builder_boost_factor: Option, task_executor: TaskExecutor, _phantom: PhantomData, } @@ -130,6 +132,8 @@ impl ValidatorStore { gas_limit: config.gas_limit, builder_proposals: config.builder_proposals, produce_block_v3: config.produce_block_v3, + prefer_builder_proposals: config.prefer_builder_proposals, + builder_boost_factor: config.builder_boost_factor, task_executor, _phantom: PhantomData, } @@ -178,6 +182,8 @@ impl ValidatorStore { suggested_fee_recipient: Option
, gas_limit: Option, builder_proposals: Option, + builder_boost_factor: Option, + prefer_builder_proposals: Option, ) -> Result { let mut validator_def = ValidatorDefinition::new_keystore_with_password( voting_keystore_path, @@ -186,6 +192,8 @@ impl ValidatorStore { suggested_fee_recipient, gas_limit, builder_proposals, + builder_boost_factor, + prefer_builder_proposals, ) .map_err(|e| format!("failed to create validator definitions: {:?}", e))?; @@ -474,7 +482,7 @@ impl ValidatorStore { .unwrap_or(DEFAULT_GAS_LIMIT) } - /// Returns a `bool` for the given public key that denotes whther this validator should use the + /// Returns a `bool` for the given public key that denotes whether this validator should use the /// builder API. The priority order for fetching this value is: /// /// 1. validator_definitions.yml @@ -487,12 +495,91 @@ impl ValidatorStore { ) } + /// Returns a `u64` for the given public key that denotes the builder boost factor. The priority order for fetching this value is: + /// + /// 1. validator_definitions.yml + /// 2. process level flag + pub fn get_builder_boost_factor(&self, validator_pubkey: &PublicKeyBytes) -> Option { + self.validators + .read() + .builder_boost_factor(validator_pubkey) + .or(self.builder_boost_factor) + } + + /// Returns a `bool` for the given public key that denotes whether this validator should prefer a + /// builder payload. The priority order for fetching this value is: + /// + /// 1. validator_definitions.yml + /// 2. process level flag + pub fn get_prefer_builder_proposals(&self, validator_pubkey: &PublicKeyBytes) -> bool { + self.validators + .read() + .prefer_builder_proposals(validator_pubkey) + .unwrap_or(self.prefer_builder_proposals) + } + fn get_builder_proposals_defaulting(&self, builder_proposals: Option) -> bool { builder_proposals // If there's nothing in the file, try the process-level default value. .unwrap_or(self.builder_proposals) } + /// Translate the per validator `builder_proposals`, `builder_boost_factor` and + /// `prefer_builder_proposals` to a boost factor, if available. + /// - If `prefer_builder_proposals` is true, set boost factor to `u64::MAX` to indicate a + /// preference for builder payloads. + /// - If `builder_boost_factor` is a value other than None, return its value as the boost factor. + /// - If `builder_proposals` is set to false, set boost factor to 0 to indicate a preference for + /// local payloads. + /// - Else return `None` to indicate no preference between builder and local payloads. + pub fn determine_validator_builder_boost_factor( + &self, + validator_pubkey: &PublicKeyBytes, + ) -> Option { + let validator_prefer_builder_proposals = self + .validators + .read() + .prefer_builder_proposals(validator_pubkey); + + if matches!(validator_prefer_builder_proposals, Some(true)) { + return Some(u64::MAX); + } + + self.validators + .read() + .builder_boost_factor(validator_pubkey) + .or_else(|| { + if matches!( + self.validators.read().builder_proposals(validator_pubkey), + Some(false) + ) { + return Some(0); + } + None + }) + } + + /// Translate the process-wide `builder_proposals`, `builder_boost_factor` and + /// `prefer_builder_proposals` configurations to a boost factor. + /// - If `prefer_builder_proposals` is true, set boost factor to `u64::MAX` to indicate a + /// preference for builder payloads. + /// - If `builder_boost_factor` is a value other than None, return its value as the boost factor. + /// - If `builder_proposals` is set to false, set boost factor to 0 to indicate a preference for + /// local payloads. + /// - Else return `None` to indicate no preference between builder and local payloads. + pub fn determine_default_builder_boost_factor(&self) -> Option { + if self.prefer_builder_proposals { + return Some(u64::MAX); + } + self.builder_boost_factor.or({ + if self.builder_proposals { + Some(0) + } else { + None + } + }) + } + pub async fn sign_block>( &self, validator_pubkey: PublicKeyBytes, diff --git a/validator_manager/src/common.rs b/validator_manager/src/common.rs index 6a3f93a3f7..871c536203 100644 --- a/validator_manager/src/common.rs +++ b/validator_manager/src/common.rs @@ -46,6 +46,8 @@ pub struct ValidatorSpecification { pub fee_recipient: Option
, pub gas_limit: Option, pub builder_proposals: Option, + pub builder_boost_factor: Option, + pub prefer_builder_proposals: Option, pub enabled: Option, } @@ -64,6 +66,8 @@ impl ValidatorSpecification { gas_limit, builder_proposals, enabled, + builder_boost_factor, + prefer_builder_proposals, } = self; let voting_public_key = voting_keystore @@ -136,6 +140,8 @@ impl ValidatorSpecification { enabled, gas_limit, builder_proposals, + builder_boost_factor, + prefer_builder_proposals, None, // Grafitti field is not maintained between validator moves. ) .await diff --git a/validator_manager/src/create_validators.rs b/validator_manager/src/create_validators.rs index 8ea740ff5b..8ab3303d36 100644 --- a/validator_manager/src/create_validators.rs +++ b/validator_manager/src/create_validators.rs @@ -25,6 +25,8 @@ pub const ETH1_WITHDRAWAL_ADDRESS_FLAG: &str = "eth1-withdrawal-address"; pub const GAS_LIMIT_FLAG: &str = "gas-limit"; pub const FEE_RECIPIENT_FLAG: &str = "suggested-fee-recipient"; pub const BUILDER_PROPOSALS_FLAG: &str = "builder-proposals"; +pub const BUILDER_BOOST_FACTOR_FLAG: &str = "builder-boost-factor"; +pub const PREFER_BUILDER_PROPOSALS_FLAG: &str = "prefer-builder-proposals"; pub const BEACON_NODE_FLAG: &str = "beacon-node"; pub const FORCE_BLS_WITHDRAWAL_CREDENTIALS: &str = "force-bls-withdrawal-credentials"; @@ -183,6 +185,30 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { address. This is not recommended.", ), ) + .arg( + Arg::with_name(BUILDER_BOOST_FACTOR_FLAG) + .long(BUILDER_BOOST_FACTOR_FLAG) + .takes_value(true) + .value_name("UINT64") + .required(false) + .help( + "Defines the boost factor, \ + a percentage multiplier to apply to the builder's payload value \ + when choosing between a builder payload header and payload from \ + the local execution node.", + ), + ) + .arg( + Arg::with_name(PREFER_BUILDER_PROPOSALS_FLAG) + .long(PREFER_BUILDER_PROPOSALS_FLAG) + .help( + "If this flag is set, Lighthouse will always prefer blocks \ + constructed by builders, regardless of payload value.", + ) + .required(false) + .possible_values(&["true", "false"]) + .takes_value(true), + ) } /// The CLI arguments are parsed into this struct before running the application. This step of @@ -199,6 +225,8 @@ pub struct CreateConfig { pub specify_voting_keystore_password: bool, pub eth1_withdrawal_address: Option
, pub builder_proposals: Option, + pub builder_boost_factor: Option, + pub prefer_builder_proposals: Option, pub fee_recipient: Option
, pub gas_limit: Option, pub bn_url: Option, @@ -223,6 +251,11 @@ impl CreateConfig { ETH1_WITHDRAWAL_ADDRESS_FLAG, )?, builder_proposals: clap_utils::parse_optional(matches, BUILDER_PROPOSALS_FLAG)?, + builder_boost_factor: clap_utils::parse_optional(matches, BUILDER_BOOST_FACTOR_FLAG)?, + prefer_builder_proposals: clap_utils::parse_optional( + matches, + PREFER_BUILDER_PROPOSALS_FLAG, + )?, fee_recipient: clap_utils::parse_optional(matches, FEE_RECIPIENT_FLAG)?, gas_limit: clap_utils::parse_optional(matches, GAS_LIMIT_FLAG)?, bn_url: clap_utils::parse_optional(matches, BEACON_NODE_FLAG)?, @@ -254,6 +287,8 @@ impl ValidatorsAndDeposits { gas_limit, bn_url, force_bls_withdrawal_credentials, + builder_boost_factor, + prefer_builder_proposals, } = config; // Since Capella, it really doesn't make much sense to use BLS @@ -456,6 +491,8 @@ impl ValidatorsAndDeposits { fee_recipient, gas_limit, builder_proposals, + builder_boost_factor, + prefer_builder_proposals, // Allow the VC to choose a default "enabled" state. Since "enabled" is not part of // the standard API, leaving this as `None` means we are not forced to use the // non-standard API. @@ -585,6 +622,8 @@ pub mod tests { specify_voting_keystore_password: false, eth1_withdrawal_address: junk_execution_address(), builder_proposals: None, + builder_boost_factor: None, + prefer_builder_proposals: None, fee_recipient: None, gas_limit: None, bn_url: None, diff --git a/validator_manager/src/move_validators.rs b/validator_manager/src/move_validators.rs index fa886e8f94..5826f2756b 100644 --- a/validator_manager/src/move_validators.rs +++ b/validator_manager/src/move_validators.rs @@ -32,6 +32,8 @@ pub const VALIDATORS_FLAG: &str = "validators"; pub const GAS_LIMIT_FLAG: &str = "gas-limit"; pub const FEE_RECIPIENT_FLAG: &str = "suggested-fee-recipient"; pub const BUILDER_PROPOSALS_FLAG: &str = "builder-proposals"; +pub const BUILDER_BOOST_FACTOR_FLAG: &str = "builder-boost-factor"; +pub const PREFER_BUILDER_PROPOSALS_FLAG: &str = "prefer-builder-proposals"; const NO_VALIDATORS_MSG: &str = "No validators present on source validator client"; @@ -170,6 +172,30 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { .long(STDIN_INPUTS_FLAG) .help("If present, read all user inputs from stdin instead of tty."), ) + .arg( + Arg::with_name(BUILDER_BOOST_FACTOR_FLAG) + .long(BUILDER_BOOST_FACTOR_FLAG) + .takes_value(true) + .value_name("UINT64") + .required(false) + .help( + "Defines the boost factor, \ + a percentage multiplier to apply to the builder's payload value \ + when choosing between a builder payload header and payload from \ + the local execution node.", + ), + ) + .arg( + Arg::with_name(PREFER_BUILDER_PROPOSALS_FLAG) + .long(PREFER_BUILDER_PROPOSALS_FLAG) + .help( + "If this flag is set, Lighthouse will always prefer blocks \ + constructed by builders, regardless of payload value.", + ) + .required(false) + .possible_values(&["true", "false"]) + .takes_value(true), + ) } #[derive(Clone, PartialEq, Debug, Serialize, Deserialize)] @@ -187,6 +213,8 @@ pub struct MoveConfig { pub dest_vc_token_path: PathBuf, pub validators: Validators, pub builder_proposals: Option, + pub builder_boost_factor: Option, + pub prefer_builder_proposals: Option, pub fee_recipient: Option
, pub gas_limit: Option, pub password_source: PasswordSource, @@ -221,6 +249,11 @@ impl MoveConfig { dest_vc_token_path: clap_utils::parse_required(matches, DEST_VC_TOKEN_FLAG)?, validators, builder_proposals: clap_utils::parse_optional(matches, BUILDER_PROPOSALS_FLAG)?, + builder_boost_factor: clap_utils::parse_optional(matches, BUILDER_BOOST_FACTOR_FLAG)?, + prefer_builder_proposals: clap_utils::parse_optional( + matches, + PREFER_BUILDER_PROPOSALS_FLAG, + )?, fee_recipient: clap_utils::parse_optional(matches, FEE_RECIPIENT_FLAG)?, gas_limit: clap_utils::parse_optional(matches, GAS_LIMIT_FLAG)?, password_source: PasswordSource::Interactive { @@ -253,6 +286,8 @@ async fn run<'a>(config: MoveConfig) -> Result<(), String> { fee_recipient, gas_limit, mut password_source, + builder_boost_factor, + prefer_builder_proposals, } = config; // Moving validators between the same VC is unlikely to be useful and probably indicates a user @@ -488,13 +523,15 @@ async fn run<'a>(config: MoveConfig) -> Result<(), String> { let keystore_derivation_path = voting_keystore.0.path(); - let validator_specification = ValidatorSpecification { + let validator_specification: ValidatorSpecification = ValidatorSpecification { voting_keystore, voting_keystore_password, slashing_protection: Some(InterchangeJsonStr(slashing_protection)), fee_recipient, gas_limit, builder_proposals, + builder_boost_factor, + prefer_builder_proposals, // Allow the VC to choose a default "enabled" state. Since "enabled" is not part of // the standard API, leaving this as `None` means we are not forced to use the // non-standard API. @@ -758,6 +795,8 @@ mod test { dest_vc_token_path: dest_vc_token_path.clone(), validators: validators.clone(), builder_proposals: None, + builder_boost_factor: None, + prefer_builder_proposals: None, fee_recipient: None, gas_limit: None, password_source: PasswordSource::Testing(self.passwords.clone()), From 1be5253610dc8fee3bf4b7a8dc1d01254bc5b57d Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Thu, 25 Jan 2024 10:02:00 +1100 Subject: [PATCH 25/25] Bump versions (#5123) --- Cargo.lock | 8 ++++---- beacon_node/Cargo.toml | 2 +- boot_node/Cargo.toml | 2 +- common/lighthouse_version/src/lib.rs | 4 ++-- lcli/Cargo.toml | 2 +- lighthouse/Cargo.toml | 2 +- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4c830105de..1d3dc30706 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -583,7 +583,7 @@ dependencies = [ [[package]] name = "beacon_node" -version = "4.6.0-rc.0" +version = "4.6.0" dependencies = [ "beacon_chain", "clap", @@ -805,7 +805,7 @@ dependencies = [ [[package]] name = "boot_node" -version = "4.6.0-rc.0" +version = "4.6.0" dependencies = [ "beacon_node", "clap", @@ -4015,7 +4015,7 @@ checksum = "830d08ce1d1d941e6b30645f1a0eb5643013d835ce3779a5fc208261dbe10f55" [[package]] name = "lcli" -version = "4.6.0-rc.0" +version = "4.6.0" dependencies = [ "account_utils", "beacon_chain", @@ -4603,7 +4603,7 @@ dependencies = [ [[package]] name = "lighthouse" -version = "4.6.0-rc.0" +version = "4.6.0" dependencies = [ "account_manager", "account_utils", diff --git a/beacon_node/Cargo.toml b/beacon_node/Cargo.toml index d6d7de8b8d..8428a30a3b 100644 --- a/beacon_node/Cargo.toml +++ b/beacon_node/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "beacon_node" -version = "4.6.0-rc.0" +version = "4.6.0" authors = [ "Paul Hauner ", "Age Manning "] edition = { workspace = true } diff --git a/common/lighthouse_version/src/lib.rs b/common/lighthouse_version/src/lib.rs index db31826eca..af11723487 100644 --- a/common/lighthouse_version/src/lib.rs +++ b/common/lighthouse_version/src/lib.rs @@ -17,8 +17,8 @@ pub const VERSION: &str = git_version!( // NOTE: using --match instead of --exclude for compatibility with old Git "--match=thiswillnevermatchlol" ], - prefix = "Lighthouse/v4.6.0-rc.0-", - fallback = "Lighthouse/v4.6.0-rc.0" + prefix = "Lighthouse/v4.6.0-", + fallback = "Lighthouse/v4.6.0" ); /// Returns `VERSION`, but with platform information appended to the end. diff --git a/lcli/Cargo.toml b/lcli/Cargo.toml index fb980ac7f2..1bba8242c5 100644 --- a/lcli/Cargo.toml +++ b/lcli/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "lcli" description = "Lighthouse CLI (modeled after zcli)" -version = "4.6.0-rc.0" +version = "4.6.0" authors = ["Paul Hauner "] edition = { workspace = true } diff --git a/lighthouse/Cargo.toml b/lighthouse/Cargo.toml index c402e5b932..8517c66c38 100644 --- a/lighthouse/Cargo.toml +++ b/lighthouse/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "lighthouse" -version = "4.6.0-rc.0" +version = "4.6.0" authors = ["Sigma Prime "] edition = { workspace = true } autotests = false