From ed8086c8977a0e0eb7f061bf4dc7484b28cf611d Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Wed, 12 Feb 2025 02:13:05 +0200 Subject: [PATCH 1/3] Ensure `GET v2/validator/aggregate_attestation` is backwards compatible (#6984) Closes #6983 `GET v2/validator/aggregate_attestation` is not backwards compatible. It only works for post electra attestations. This PR adds backwards compatibility and additional test coverage. We should include this in the upcoming 7.0 beta release if possible --- .../http_api/src/aggregate_attestation.rs | 37 ++++---- beacon_node/http_api/tests/tests.rs | 94 +++++++++++-------- 2 files changed, 77 insertions(+), 54 deletions(-) diff --git a/beacon_node/http_api/src/aggregate_attestation.rs b/beacon_node/http_api/src/aggregate_attestation.rs index 94b6acd2e6..23af5b0cb5 100644 --- a/beacon_node/http_api/src/aggregate_attestation.rs +++ b/beacon_node/http_api/src/aggregate_attestation.rs @@ -18,13 +18,14 @@ pub fn get_aggregate_attestation( endpoint_version: EndpointVersion, chain: Arc>, ) -> Result, warp::reject::Rejection> { - if endpoint_version == V2 { + let fork_name = chain.spec.fork_name_at_slot::(slot); + let aggregate_attestation = if fork_name.electra_enabled() { let Some(committee_index) = committee_index else { return Err(warp_utils::reject::custom_bad_request( "missing committee index".to_string(), )); }; - let aggregate_attestation = chain + chain .get_aggregated_attestation_electra(slot, attestation_data_root, committee_index) .map_err(|e| { warp_utils::reject::custom_bad_request(format!( @@ -34,8 +35,22 @@ pub fn get_aggregate_attestation( })? .ok_or_else(|| { warp_utils::reject::custom_not_found("no matching aggregate found".to_string()) - })?; - let fork_name = chain.spec.fork_name_at_slot::(slot); + })? + } else { + chain + .get_pre_electra_aggregated_attestation_by_slot_and_root(slot, attestation_data_root) + .map_err(|e| { + warp_utils::reject::custom_bad_request(format!( + "unable to fetch aggregate: {:?}", + e + )) + })? + .ok_or_else(|| { + warp_utils::reject::custom_not_found("no matching aggregate found".to_string()) + })? + }; + + if endpoint_version == V2 { let fork_versioned_response = ForkVersionedResponse { version: Some(fork_name), metadata: EmptyMetadata {}, @@ -46,19 +61,7 @@ pub fn get_aggregate_attestation( fork_name, )) } else if endpoint_version == V1 { - let aggregate_attestation = chain - .get_pre_electra_aggregated_attestation_by_slot_and_root(slot, attestation_data_root) - .map_err(|e| { - warp_utils::reject::custom_bad_request(format!( - "unable to fetch aggregate: {:?}", - e - )) - })? - .map(GenericResponse::from) - .ok_or_else(|| { - warp_utils::reject::custom_not_found("no matching aggregate found".to_string()) - })?; - Ok(warp::reply::json(&aggregate_attestation).into_response()) + Ok(warp::reply::json(&GenericResponse::from(aggregate_attestation)).into_response()) } else { return Err(unsupported_version_rejection(endpoint_version)); } diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index bc3159e074..556ddebad3 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -3555,44 +3555,48 @@ impl ApiTester { } #[allow(clippy::await_holding_lock)] // This is a test, so it should be fine. - pub async fn test_get_validator_aggregate_attestation(self) -> Self { - if self + pub async fn test_get_validator_aggregate_attestation_v1(self) -> Self { + let attestation = self .chain - .spec - .fork_name_at_slot::(self.chain.slot().unwrap()) - .electra_enabled() - { - for attestation in self.chain.naive_aggregation_pool.read().iter() { - let result = self - .client - .get_validator_aggregate_attestation_v2( - attestation.data().slot, - attestation.data().tree_hash_root(), - attestation.committee_index().expect("committee index"), - ) - .await - .unwrap() - .unwrap() - .data; - let expected = attestation; + .head_beacon_block() + .message() + .body() + .attestations() + .next() + .unwrap() + .clone_as_attestation(); + let result = self + .client + .get_validator_aggregate_attestation_v1( + attestation.data().slot, + attestation.data().tree_hash_root(), + ) + .await + .unwrap() + .unwrap() + .data; + let expected = attestation; - assert_eq!(&result, expected); - } - } else { - let attestation = self - .chain - .head_beacon_block() - .message() - .body() - .attestations() - .next() - .unwrap() - .clone_as_attestation(); + assert_eq!(result, expected); + + self + } + + pub async fn test_get_validator_aggregate_attestation_v2(self) -> Self { + let attestations = self + .chain + .naive_aggregation_pool + .read() + .iter() + .cloned() + .collect::>(); + for attestation in attestations { let result = self .client - .get_validator_aggregate_attestation_v1( + .get_validator_aggregate_attestation_v2( attestation.data().slot, attestation.data().tree_hash_root(), + attestation.committee_index().expect("committee index"), ) .await .unwrap() @@ -3602,7 +3606,6 @@ impl ApiTester { assert_eq!(result, expected); } - self } @@ -6775,19 +6778,36 @@ async fn get_validator_attestation_data_with_skip_slots() { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn get_validator_aggregate_attestation() { +async fn get_validator_aggregate_attestation_v1() { ApiTester::new() .await - .test_get_validator_aggregate_attestation() + .test_get_validator_aggregate_attestation_v1() .await; } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn get_validator_aggregate_attestation_with_skip_slots() { +async fn get_validator_aggregate_attestation_v2() { + ApiTester::new() + .await + .test_get_validator_aggregate_attestation_v2() + .await; +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn get_validator_aggregate_attestation_with_skip_slots_v1() { ApiTester::new() .await .skip_slots(E::slots_per_epoch() * 2) - .test_get_validator_aggregate_attestation() + .test_get_validator_aggregate_attestation_v1() + .await; +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn get_validator_aggregate_attestation_with_skip_slots_v2() { + ApiTester::new() + .await + .skip_slots(E::slots_per_epoch() * 2) + .test_get_validator_aggregate_attestation_v2() .await; } From 25f804a111c8c2fd127863c8f3db6b12d0d668f8 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Thu, 13 Feb 2025 11:50:17 +1100 Subject: [PATCH 2/3] Fix light client plumbing in beacon processor (#6993) Our Holesky nodes running with the light client enabled were logging messages about full queues: > Feb 12 22:09:28.949 ERRO Work queue is full queue: unknown_light_client_optimistic_update, queue_len: 128, msg: the system has insufficient resources for load, service: bproc I thought this might be genuine overload, but it turns out this queue was never being read from! - [x] Rename light-client related queues in the beacon processor for clarity. - [x] Ensure all light-client related queues are being popped from. --- beacon_node/beacon_processor/src/lib.rs | 73 +++++++++++++++---------- 1 file changed, 43 insertions(+), 30 deletions(-) diff --git a/beacon_node/beacon_processor/src/lib.rs b/beacon_node/beacon_processor/src/lib.rs index 2743f93bb3..a8960d47c9 100644 --- a/beacon_node/beacon_processor/src/lib.rs +++ b/beacon_node/beacon_processor/src/lib.rs @@ -109,8 +109,6 @@ pub struct BeaconProcessorQueueLengths { gossip_voluntary_exit_queue: usize, gossip_proposer_slashing_queue: usize, gossip_attester_slashing_queue: usize, - finality_update_queue: usize, - optimistic_update_queue: usize, unknown_light_client_update_queue: usize, unknown_block_sampling_request_queue: usize, rpc_block_queue: usize, @@ -132,9 +130,11 @@ pub struct BeaconProcessorQueueLengths { dcbroots_queue: usize, dcbrange_queue: usize, gossip_bls_to_execution_change_queue: usize, + lc_gossip_finality_update_queue: usize, + lc_gossip_optimistic_update_queue: usize, lc_bootstrap_queue: usize, - lc_optimistic_update_queue: usize, - lc_finality_update_queue: usize, + lc_rpc_optimistic_update_queue: usize, + lc_rpc_finality_update_queue: usize, lc_update_range_queue: usize, api_request_p0_queue: usize, api_request_p1_queue: usize, @@ -175,15 +175,13 @@ impl BeaconProcessorQueueLengths { gossip_voluntary_exit_queue: 4096, gossip_proposer_slashing_queue: 4096, gossip_attester_slashing_queue: 4096, - finality_update_queue: 1024, - optimistic_update_queue: 1024, - unknown_block_sampling_request_queue: 16384, unknown_light_client_update_queue: 128, rpc_block_queue: 1024, rpc_blob_queue: 1024, // TODO(das): Placeholder values rpc_custody_column_queue: 1000, rpc_verify_data_column_queue: 1000, + unknown_block_sampling_request_queue: 16384, sampling_result_queue: 1000, chain_segment_queue: 64, backfill_chain_segment: 64, @@ -200,9 +198,11 @@ impl BeaconProcessorQueueLengths { dcbroots_queue: 1024, dcbrange_queue: 1024, gossip_bls_to_execution_change_queue: 16384, + lc_gossip_finality_update_queue: 1024, + lc_gossip_optimistic_update_queue: 1024, lc_bootstrap_queue: 1024, - lc_optimistic_update_queue: 512, - lc_finality_update_queue: 512, + lc_rpc_optimistic_update_queue: 512, + lc_rpc_finality_update_queue: 512, lc_update_range_queue: 512, api_request_p0_queue: 1024, api_request_p1_queue: 1024, @@ -884,21 +884,16 @@ impl BeaconProcessor { let mut gossip_attester_slashing_queue = FifoQueue::new(queue_lengths.gossip_attester_slashing_queue); - // Using a FIFO queue for light client updates to maintain sequence order. - let mut finality_update_queue = FifoQueue::new(queue_lengths.finality_update_queue); - let mut optimistic_update_queue = FifoQueue::new(queue_lengths.optimistic_update_queue); - let mut unknown_light_client_update_queue = - FifoQueue::new(queue_lengths.unknown_light_client_update_queue); - let mut unknown_block_sampling_request_queue = - FifoQueue::new(queue_lengths.unknown_block_sampling_request_queue); - // Using a FIFO queue since blocks need to be imported sequentially. let mut rpc_block_queue = FifoQueue::new(queue_lengths.rpc_block_queue); let mut rpc_blob_queue = FifoQueue::new(queue_lengths.rpc_blob_queue); let mut rpc_custody_column_queue = FifoQueue::new(queue_lengths.rpc_custody_column_queue); let mut rpc_verify_data_column_queue = FifoQueue::new(queue_lengths.rpc_verify_data_column_queue); + // TODO(das): the sampling_request_queue is never read let mut sampling_result_queue = FifoQueue::new(queue_lengths.sampling_result_queue); + let mut unknown_block_sampling_request_queue = + FifoQueue::new(queue_lengths.unknown_block_sampling_request_queue); let mut chain_segment_queue = FifoQueue::new(queue_lengths.chain_segment_queue); let mut backfill_chain_segment = FifoQueue::new(queue_lengths.backfill_chain_segment); let mut gossip_block_queue = FifoQueue::new(queue_lengths.gossip_block_queue); @@ -917,10 +912,18 @@ impl BeaconProcessor { let mut gossip_bls_to_execution_change_queue = FifoQueue::new(queue_lengths.gossip_bls_to_execution_change_queue); + // Using FIFO queues for light client updates to maintain sequence order. + let mut lc_gossip_finality_update_queue = + FifoQueue::new(queue_lengths.lc_gossip_finality_update_queue); + let mut lc_gossip_optimistic_update_queue = + FifoQueue::new(queue_lengths.lc_gossip_optimistic_update_queue); + let mut unknown_light_client_update_queue = + FifoQueue::new(queue_lengths.unknown_light_client_update_queue); let mut lc_bootstrap_queue = FifoQueue::new(queue_lengths.lc_bootstrap_queue); - let mut lc_optimistic_update_queue = - FifoQueue::new(queue_lengths.lc_optimistic_update_queue); - let mut lc_finality_update_queue = FifoQueue::new(queue_lengths.lc_finality_update_queue); + let mut lc_rpc_optimistic_update_queue = + FifoQueue::new(queue_lengths.lc_rpc_optimistic_update_queue); + let mut lc_rpc_finality_update_queue = + FifoQueue::new(queue_lengths.lc_rpc_finality_update_queue); let mut lc_update_range_queue = FifoQueue::new(queue_lengths.lc_update_range_queue); let mut api_request_p0_queue = FifoQueue::new(queue_lengths.api_request_p0_queue); @@ -1254,11 +1257,19 @@ impl BeaconProcessor { } else if let Some(item) = backfill_chain_segment.pop() { Some(item) // Handle light client requests. + } else if let Some(item) = lc_gossip_finality_update_queue.pop() { + Some(item) + } else if let Some(item) = lc_gossip_optimistic_update_queue.pop() { + Some(item) + } else if let Some(item) = unknown_light_client_update_queue.pop() { + Some(item) } else if let Some(item) = lc_bootstrap_queue.pop() { Some(item) - } else if let Some(item) = lc_optimistic_update_queue.pop() { + } else if let Some(item) = lc_rpc_optimistic_update_queue.pop() { Some(item) - } else if let Some(item) = lc_finality_update_queue.pop() { + } else if let Some(item) = lc_rpc_finality_update_queue.pop() { + Some(item) + } else if let Some(item) = lc_update_range_queue.pop() { Some(item) // This statement should always be the final else statement. } else { @@ -1362,10 +1373,10 @@ impl BeaconProcessor { sync_contribution_queue.push(work) } Work::GossipLightClientFinalityUpdate { .. } => { - finality_update_queue.push(work, work_id, &self.log) + lc_gossip_finality_update_queue.push(work, work_id, &self.log) } Work::GossipLightClientOptimisticUpdate { .. } => { - optimistic_update_queue.push(work, work_id, &self.log) + lc_gossip_optimistic_update_queue.push(work, work_id, &self.log) } Work::RpcBlock { .. } | Work::IgnoredRpcBlock { .. } => { rpc_block_queue.push(work, work_id, &self.log) @@ -1400,10 +1411,10 @@ impl BeaconProcessor { lc_bootstrap_queue.push(work, work_id, &self.log) } Work::LightClientOptimisticUpdateRequest { .. } => { - lc_optimistic_update_queue.push(work, work_id, &self.log) + lc_rpc_optimistic_update_queue.push(work, work_id, &self.log) } Work::LightClientFinalityUpdateRequest { .. } => { - lc_finality_update_queue.push(work, work_id, &self.log) + lc_rpc_finality_update_queue.push(work, work_id, &self.log) } Work::LightClientUpdatesByRangeRequest { .. } => { lc_update_range_queue.push(work, work_id, &self.log) @@ -1472,9 +1483,11 @@ impl BeaconProcessor { WorkType::GossipAttesterSlashing => gossip_attester_slashing_queue.len(), WorkType::GossipSyncSignature => sync_message_queue.len(), WorkType::GossipSyncContribution => sync_contribution_queue.len(), - WorkType::GossipLightClientFinalityUpdate => finality_update_queue.len(), + WorkType::GossipLightClientFinalityUpdate => { + lc_gossip_finality_update_queue.len() + } WorkType::GossipLightClientOptimisticUpdate => { - optimistic_update_queue.len() + lc_gossip_optimistic_update_queue.len() } WorkType::RpcBlock => rpc_block_queue.len(), WorkType::RpcBlobs | WorkType::IgnoredRpcBlock => rpc_blob_queue.len(), @@ -1495,10 +1508,10 @@ impl BeaconProcessor { } WorkType::LightClientBootstrapRequest => lc_bootstrap_queue.len(), WorkType::LightClientOptimisticUpdateRequest => { - lc_optimistic_update_queue.len() + lc_rpc_optimistic_update_queue.len() } WorkType::LightClientFinalityUpdateRequest => { - lc_finality_update_queue.len() + lc_rpc_finality_update_queue.len() } WorkType::LightClientUpdatesByRangeRequest => lc_update_range_queue.len(), WorkType::ApiRequestP0 => api_request_p0_queue.len(), From 1888be554cb365c419aac993528bc6697728f793 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Thu, 13 Feb 2025 14:06:20 +1100 Subject: [PATCH 3/3] Release v7.0.0-beta.0 (#6962) New release for Electra on Holesky and Sepolia. Includes PRs: - https://github.com/sigp/lighthouse/pull/6808 - https://github.com/sigp/lighthouse/pull/6914 - https://github.com/sigp/lighthouse/pull/6949 - https://github.com/sigp/lighthouse/pull/6950 - https://github.com/sigp/lighthouse/pull/6958 --- Cargo.lock | 8 ++++---- beacon_node/Cargo.toml | 2 +- boot_node/Cargo.toml | 2 +- common/lighthouse_version/src/lib.rs | 13 +++++++------ lcli/Cargo.toml | 2 +- lighthouse/Cargo.toml | 2 +- 6 files changed, 15 insertions(+), 14 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bc68957333..e41616d7dd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -860,7 +860,7 @@ dependencies = [ [[package]] name = "beacon_node" -version = "6.0.1" +version = "7.0.0-beta.0" dependencies = [ "account_utils", "beacon_chain", @@ -1108,7 +1108,7 @@ dependencies = [ [[package]] name = "boot_node" -version = "6.0.1" +version = "7.0.0-beta.0" dependencies = [ "beacon_node", "bytes", @@ -4811,7 +4811,7 @@ checksum = "830d08ce1d1d941e6b30645f1a0eb5643013d835ce3779a5fc208261dbe10f55" [[package]] name = "lcli" -version = "6.0.1" +version = "7.0.0-beta.0" dependencies = [ "account_utils", "beacon_chain", @@ -5366,7 +5366,7 @@ dependencies = [ [[package]] name = "lighthouse" -version = "6.0.1" +version = "7.0.0-beta.0" dependencies = [ "account_manager", "account_utils", diff --git a/beacon_node/Cargo.toml b/beacon_node/Cargo.toml index 7da65ad742..f6948e8743 100644 --- a/beacon_node/Cargo.toml +++ b/beacon_node/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "beacon_node" -version = "6.0.1" +version = "7.0.0-beta.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 a35b8c42c1..cfffdbbb09 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/v6.0.1-", - fallback = "Lighthouse/v6.0.1" + prefix = "Lighthouse/v7.0.0-beta.0-", + fallback = "Lighthouse/v7.0.0-beta.0" ); /// Returns the first eight characters of the latest commit hash for this build. @@ -54,7 +54,7 @@ pub fn version_with_platform() -> String { /// /// `1.5.1` pub fn version() -> &'static str { - "6.0.1" + "7.0.0-beta.0" } /// Returns the name of the current client running. @@ -71,9 +71,10 @@ mod test { #[test] fn version_formatting() { - let re = - Regex::new(r"^Lighthouse/v[0-9]+\.[0-9]+\.[0-9]+(-rc.[0-9])?(-[[:xdigit:]]{7})?\+?$") - .unwrap(); + let re = Regex::new( + r"^Lighthouse/v[0-9]+\.[0-9]+\.[0-9]+(-(rc|beta).[0-9])?(-[[:xdigit:]]{7})?\+?$", + ) + .unwrap(); assert!( re.is_match(VERSION), "version doesn't match regex: {}", diff --git a/lcli/Cargo.toml b/lcli/Cargo.toml index 72be77a70b..74b7ddcb2a 100644 --- a/lcli/Cargo.toml +++ b/lcli/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "lcli" description = "Lighthouse CLI (modeled after zcli)" -version = "6.0.1" +version = "7.0.0-beta.0" authors = ["Paul Hauner "] edition = { workspace = true } diff --git a/lighthouse/Cargo.toml b/lighthouse/Cargo.toml index c95735d41c..fc73a2cb93 100644 --- a/lighthouse/Cargo.toml +++ b/lighthouse/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "lighthouse" -version = "6.0.1" +version = "7.0.0-beta.0" authors = ["Sigma Prime "] edition = { workspace = true } autotests = false