diff --git a/Makefile b/Makefile index 66d0b68268..475d3aac8a 100644 --- a/Makefile +++ b/Makefile @@ -193,11 +193,11 @@ test-beacon-chain: $(patsubst %,test-beacon-chain-%,$(FORKS)) test-beacon-chain-%: env FORK_NAME=$* cargo nextest run --release --features "fork_from_env,slasher/lmdb,$(TEST_FEATURES)" -p beacon_chain -# Run the tests in the `beacon_chain` crate for all known forks. -test-http-api: $(patsubst %,test-beacon-chain-%,$(RECENT_FORKS)) +# Run the tests in the `http_api` crate for recent forks. +test-http-api: $(patsubst %,test-http-api-%,$(RECENT_FORKS)) test-http-api-%: - env FORK_NAME=$* cargo nextest run --release --features "fork_from_env,slasher/lmdb,$(TEST_FEATURES)" -p http_api + env FORK_NAME=$* cargo nextest run --release --features "beacon_chain/fork_from_env" -p http_api # Run the tests in the `operation_pool` crate for all known forks. diff --git a/beacon_node/beacon_chain/src/blob_verification.rs b/beacon_node/beacon_chain/src/blob_verification.rs index 2ba20d5a82..53676c0b24 100644 --- a/beacon_node/beacon_chain/src/blob_verification.rs +++ b/beacon_node/beacon_chain/src/blob_verification.rs @@ -96,7 +96,7 @@ pub enum GossipBlobError { /// ## Peer scoring /// /// We cannot process the blob without validating its parent, the peer isn't necessarily faulty. - BlobParentUnknown { parent_root: Hash256 }, + ParentUnknown { parent_root: Hash256 }, /// Invalid kzg commitment inclusion proof /// ## Peer scoring @@ -474,7 +474,7 @@ pub fn validate_blob_sidecar_for_gossip impl futures::Future + 'static { + strict_registrations: bool, + apply_operations: bool, + broadcast_to_bn: bool, + ) -> impl futures::Future + use { let mock_el = self .mock_execution_layer .as_ref() @@ -727,6 +730,9 @@ where let (mock_builder, (addr, mock_builder_server)) = MockBuilder::new_for_testing( mock_el_url, beacon_url, + strict_registrations, + apply_operations, + broadcast_to_bn, self.spec.clone(), self.runtime.task_executor.clone(), ); @@ -903,8 +909,65 @@ where state: BeaconState, slot: Slot, ) -> (SignedBlindedBeaconBlock, BeaconState) { - let (unblinded, new_state) = self.make_block(state, slot).await; - ((*unblinded.0).clone().into(), new_state) + self.make_blinded_block_with_modifier(state, slot, |_| {}) + .await + } + + pub async fn make_blinded_block_with_modifier( + &self, + mut state: BeaconState, + slot: Slot, + block_modifier: impl FnOnce(&mut BlindedBeaconBlock), + ) -> (SignedBlindedBeaconBlock, BeaconState) { + assert_ne!(slot, 0, "can't produce a block at slot 0"); + assert!(slot >= state.slot()); + + complete_state_advance(&mut state, None, slot, &self.spec) + .expect("should be able to advance state to slot"); + + state.build_caches(&self.spec).expect("should build caches"); + + let proposer_index = state.get_beacon_proposer_index(slot, &self.spec).unwrap(); + + // If we produce two blocks for the same slot, they hash up to the same value and + // BeaconChain errors out with `DuplicateFullyImported`. Vary the graffiti so that we produce + // different blocks each time. + let graffiti = Graffiti::from(self.rng.lock().random::<[u8; 32]>()); + + let randao_reveal = self.sign_randao_reveal(&state, proposer_index, slot); + + // Always use the builder, so that we produce a "real" blinded payload. + let builder_boost_factor = Some(u64::MAX); + + let BeaconBlockResponseWrapper::Blinded(block_response) = self + .chain + .produce_block_on_state( + state, + None, + slot, + randao_reveal, + Some(graffiti), + ProduceBlockVerification::VerifyRandao, + builder_boost_factor, + BlockProductionVersion::V3, + ) + .await + .unwrap() + else { + panic!("Should always be a blinded payload response"); + }; + + let mut block = block_response.block; + block_modifier(&mut block); + + let signed_block = block.sign( + &self.validator_keypairs[proposer_index].sk, + &block_response.state.fork(), + block_response.state.genesis_validators_root(), + &self.spec, + ); + + (signed_block, block_response.state) } /// Returns a newly created block, signed by the proposer for the given slot. diff --git a/beacon_node/builder_client/src/lib.rs b/beacon_node/builder_client/src/lib.rs index 0c3fdca907..2c83e34755 100644 --- a/beacon_node/builder_client/src/lib.rs +++ b/beacon_node/builder_client/src/lib.rs @@ -8,7 +8,7 @@ use eth2::types::{ use eth2::types::{FullPayloadContents, SignedBlindedBeaconBlock}; use eth2::{ CONSENSUS_VERSION_HEADER, CONTENT_TYPE_HEADER, JSON_CONTENT_TYPE_HEADER, - SSZ_CONTENT_TYPE_HEADER, StatusCode, ok_or_error, + SSZ_CONTENT_TYPE_HEADER, StatusCode, ok_or_error, success_or_error, }; use reqwest::header::{ACCEPT, HeaderMap, HeaderValue}; use reqwest::{IntoUrl, Response}; @@ -249,7 +249,7 @@ impl BuilderHttpClient { .send() .await .map_err(Error::from)?; - ok_or_error(response).await + success_or_error(response).await } async fn post_with_raw_response( @@ -270,7 +270,7 @@ impl BuilderHttpClient { .send() .await .map_err(Error::from)?; - ok_or_error(response).await + success_or_error(response).await } /// `POST /eth/v1/builder/validators` diff --git a/beacon_node/execution_layer/src/lib.rs b/beacon_node/execution_layer/src/lib.rs index 5b48b81aa6..401646f367 100644 --- a/beacon_node/execution_layer/src/lib.rs +++ b/beacon_node/execution_layer/src/lib.rs @@ -2032,7 +2032,9 @@ impl ExecutionLayer { relay_response_ms = duration.as_millis(), ?block_root, "Successfully submitted blinded block to the builder" - ) + ); + + Ok(()) } Err(e) => { metrics::inc_counter_vec( @@ -2045,11 +2047,10 @@ impl ExecutionLayer { relay_response_ms = duration.as_millis(), ?block_root, "Failed to submit blinded block to the builder" - ) + ); + Err(e) } } - - Ok(()) } else { Err(Error::NoPayloadBuilder) } diff --git a/beacon_node/execution_layer/src/test_utils/mock_builder.rs b/beacon_node/execution_layer/src/test_utils/mock_builder.rs index 516662b1d6..6b63881d85 100644 --- a/beacon_node/execution_layer/src/test_utils/mock_builder.rs +++ b/beacon_node/execution_layer/src/test_utils/mock_builder.rs @@ -3,8 +3,8 @@ use crate::{Config, ExecutionLayer, PayloadAttributes, PayloadParameters}; use bytes::Bytes; use eth2::types::PublishBlockRequest; use eth2::types::{ - BlobsBundle, BlockId, BroadcastValidation, EventKind, EventTopic, FullPayloadContents, - ProposerData, StateId, ValidatorId, + BlobsBundle, BlockId, BroadcastValidation, EndpointVersion, EventKind, EventTopic, + FullPayloadContents, ProposerData, StateId, ValidatorId, }; use eth2::{ BeaconNodeHttpClient, CONSENSUS_VERSION_HEADER, CONTENT_TYPE_HEADER, SSZ_CONTENT_TYPE_HEADER, @@ -332,6 +332,10 @@ pub struct MockBuilder { payload_id_cache: Arc>>, /// If set to `true`, sets the bid returned by `get_header` to Uint256::MAX max_bid: bool, + /// Broadcast the full block with payload to the attached beacon node (simulating the relay). + /// + /// Turning this off is useful for testing. + broadcast_to_bn: bool, /// A cache that stores the proposers index for a given epoch proposers_cache: Arc>>>, } @@ -340,6 +344,9 @@ impl MockBuilder { pub fn new_for_testing( mock_el_url: SensitiveUrl, beacon_url: SensitiveUrl, + validate_pubkey: bool, + apply_operations: bool, + broadcast_to_bn: bool, spec: Arc, executor: TaskExecutor, ) -> (Self, (SocketAddr, impl Future)) { @@ -357,12 +364,15 @@ impl MockBuilder { let el = ExecutionLayer::from_config(config, executor.clone()).unwrap(); + let max_bid = false; + let builder = MockBuilder::new( el, BeaconNodeHttpClient::new(beacon_url, Timeouts::set_all(Duration::from_secs(1))), - true, - true, - false, + validate_pubkey, + apply_operations, + broadcast_to_bn, + max_bid, spec, None, ); @@ -378,6 +388,7 @@ impl MockBuilder { beacon_client: BeaconNodeHttpClient, validate_pubkey: bool, apply_operations: bool, + broadcast_to_bn: bool, max_bid: bool, spec: Arc, sk: Option<&[u8]>, @@ -407,6 +418,7 @@ impl MockBuilder { proposers_cache: Arc::new(RwLock::new(HashMap::new())), apply_operations, max_bid, + broadcast_to_bn, genesis_time: None, } } @@ -486,14 +498,20 @@ impl MockBuilder { block.message.body.execution_payload.tree_hash_root() } }; + let block_hash = block + .message() + .body() + .execution_payload() + .unwrap() + .block_hash(); info!( - block_hash = %root, + execution_payload_root = %root, + ?block_hash, "Submitting blinded beacon block to builder" ); - let payload = self - .el - .get_payload_by_root(&root) - .ok_or_else(|| "missing payload for tx root".to_string())?; + let payload = self.el.get_payload_by_root(&root).ok_or_else(|| { + format!("missing payload for root: {root:?}, block_hash: {block_hash:?}",) + })?; let (payload, blobs) = payload.deconstruct(); let full_block = block @@ -502,16 +520,28 @@ impl MockBuilder { debug!( txs_count = payload.transactions().len(), blob_count = blobs.as_ref().map(|b| b.commitments.len()), - "Got full payload, sending to local beacon node for propagation" + "Got full payload" ); - let publish_block_request = PublishBlockRequest::new( - Arc::new(full_block), - blobs.clone().map(|b| (b.proofs, b.blobs)), - ); - self.beacon_client - .post_beacon_blocks_v2(&publish_block_request, Some(BroadcastValidation::Gossip)) - .await - .map_err(|e| format!("Failed to post blinded block {:?}", e))?; + if self.broadcast_to_bn { + debug!( + block_hash = ?payload.block_hash(), + "Broadcasting builder block to BN" + ); + let publish_block_request = PublishBlockRequest::new( + Arc::new(full_block), + blobs.clone().map(|b| (b.proofs, b.blobs)), + ); + self.beacon_client + .post_beacon_blocks_v2( + &publish_block_request, + Some(BroadcastValidation::ConsensusAndEquivocation), + ) + .await + .map_err(|e| { + // XXX: this should really be a 400 but warp makes that annoyingly difficult + format!("Failed to post blinded block {e:?}") + })?; + } Ok(FullPayloadContents::new(payload, blobs)) } @@ -542,16 +572,29 @@ impl MockBuilder { info!("Got payload params"); let fork = self.fork_name_at_slot(slot); + let payload_response_type = self .el - .get_full_payload_caching(PayloadParameters { - parent_hash: payload_parameters.parent_hash, - parent_gas_limit: payload_parameters.parent_gas_limit, - proposer_gas_limit: payload_parameters.proposer_gas_limit, - payload_attributes: &payload_parameters.payload_attributes, - forkchoice_update_params: &payload_parameters.forkchoice_update_params, - current_fork: payload_parameters.current_fork, - }) + .get_full_payload_with( + PayloadParameters { + parent_hash: payload_parameters.parent_hash, + parent_gas_limit: payload_parameters.parent_gas_limit, + proposer_gas_limit: payload_parameters.proposer_gas_limit, + payload_attributes: &payload_parameters.payload_attributes, + forkchoice_update_params: &payload_parameters.forkchoice_update_params, + current_fork: payload_parameters.current_fork, + }, + // If apply_operations is set, do NOT cache the payload at this point, we are about + // to mutate it and it would be incorrect to cache the unmutated payload. + // + // This is a flaw in apply_operations generally, if you want the mock builder to + // actually return payloads then this option should be turned off. + if self.apply_operations { + |_, _| None + } else { + ExecutionLayer::cache_payload + }, + ) .await .map_err(|e| format!("couldn't get payload {:?}", e))?; @@ -958,11 +1001,21 @@ pub fn serve( let inner_ctx = builder.clone(); let ctx_filter = warp::any().map(move || inner_ctx.clone()); - let prefix = warp::path("eth") + let prefix_v1 = warp::path("eth") .and(warp::path("v1")) .and(warp::path("builder")); - let validators = prefix + let prefix_either = warp::path("eth") + .and( + warp::path::param::().or_else(|_| async move { + Err(warp::reject::custom(Custom( + "Invalid EndpointVersion".to_string(), + ))) + }), + ) + .and(warp::path("builder")); + + let validators = prefix_v1 .and(warp::path("validators")) .and(warp::body::json()) .and(warp::path::end()) @@ -974,61 +1027,89 @@ pub fn serve( .register_validators(registrations) .await .map_err(|e| warp::reject::custom(Custom(e)))?; - Ok::<_, Rejection>(warp::reply()) - }, - ) - .boxed(); - - let blinded_block_ssz = prefix - .and(warp::path("blinded_blocks")) - .and(warp::body::bytes()) - .and(warp::header::header::(CONSENSUS_VERSION_HEADER)) - .and(warp::path::end()) - .and(ctx_filter.clone()) - .and_then( - |block_bytes: Bytes, fork_name: ForkName, builder: MockBuilder| async move { - let block = - SignedBlindedBeaconBlock::::from_ssz_bytes_by_fork(&block_bytes, fork_name) - .map_err(|e| warp::reject::custom(Custom(format!("{:?}", e))))?; - let payload = builder - .submit_blinded_block(block) - .await - .map_err(|e| warp::reject::custom(Custom(e)))?; - - Ok::<_, warp::reject::Rejection>( - warp::http::Response::builder() - .status(200) - .body(payload.as_ssz_bytes()) - .map(add_ssz_content_type_header) - .map(|res| add_consensus_version_header(res, fork_name)) - .unwrap(), - ) + Ok::<_, Rejection>(warp::reply().into_response()) }, ); - let blinded_block = - prefix + let blinded_block_ssz = + prefix_either .and(warp::path("blinded_blocks")) - .and(warp::body::json()) + .and(warp::body::bytes()) .and(warp::header::header::(CONSENSUS_VERSION_HEADER)) .and(warp::path::end()) .and(ctx_filter.clone()) .and_then( - |block: SignedBlindedBeaconBlock, + |endpoint_version, + block_bytes: Bytes, fork_name: ForkName, builder: MockBuilder| async move { + if endpoint_version != EndpointVersion(1) + && endpoint_version != EndpointVersion(2) + { + return Err(warp::reject::custom(Custom(format!( + "Unsupported version: {endpoint_version}" + )))); + } + let block = SignedBlindedBeaconBlock::::from_ssz_bytes_by_fork( + &block_bytes, + fork_name, + ) + .map_err(|e| warp::reject::custom(Custom(format!("{:?}", e))))?; let payload = builder .submit_blinded_block(block) .await .map_err(|e| warp::reject::custom(Custom(e)))?; - let resp: ForkVersionedResponse<_> = ForkVersionedResponse { - version: fork_name, - metadata: Default::default(), - data: payload, - }; - let json_payload = serde_json::to_string(&resp) - .map_err(|_| reject("coudn't serialize response"))?; + if endpoint_version == EndpointVersion(1) { + Ok::<_, warp::reject::Rejection>( + warp::http::Response::builder() + .status(200) + .body(payload.as_ssz_bytes()) + .map(add_ssz_content_type_header) + .map(|res| add_consensus_version_header(res, fork_name)) + .unwrap(), + ) + } else { + Ok(warp::http::Response::builder() + .status(202) + .body(&[] as &'static [u8]) + .map(|res| add_consensus_version_header(res, fork_name)) + .unwrap()) + } + }, + ); + + let blinded_block = prefix_either + .and(warp::path("blinded_blocks")) + .and(warp::body::json()) + .and(warp::header::header::(CONSENSUS_VERSION_HEADER)) + .and(warp::path::end()) + .and(ctx_filter.clone()) + .and_then( + |endpoint_version, + block: SignedBlindedBeaconBlock, + fork_name: ForkName, + builder: MockBuilder| async move { + if endpoint_version != EndpointVersion(1) && endpoint_version != EndpointVersion(2) + { + return Err(warp::reject::custom(Custom(format!( + "Unsupported version: {endpoint_version}" + )))); + } + let payload = builder + .submit_blinded_block(block) + .await + .map_err(|e| warp::reject::custom(Custom(e)))?; + let resp: ForkVersionedResponse<_> = ForkVersionedResponse { + version: fork_name, + metadata: Default::default(), + data: payload, + }; + + let json_payload = serde_json::to_string(&resp) + .map_err(|_| reject("coudn't serialize response"))?; + + if endpoint_version == EndpointVersion(1) { Ok::<_, warp::reject::Rejection>( warp::http::Response::builder() .status(200) @@ -1036,16 +1117,24 @@ pub fn serve( serde_json::to_string(&json_payload) .map_err(|_| reject("invalid JSON"))?, ) + .map(|res| add_consensus_version_header(res, fork_name)) .unwrap(), ) - }, - ); + } else { + Ok(warp::http::Response::builder() + .status(202) + .body("".to_string()) + .map(|res| add_consensus_version_header(res, fork_name)) + .unwrap()) + } + }, + ); - let status = prefix + let status = prefix_v1 .and(warp::path("status")) - .then(|| async { warp::reply() }); + .then(|| async { warp::reply().into_response() }); - let header = prefix + let header = prefix_v1 .and(warp::path("header")) .and(warp::path::param::().or_else(|_| async { Err(reject("Invalid slot")) })) .and( diff --git a/beacon_node/http_api/src/lib.rs b/beacon_node/http_api/src/lib.rs index 515c262b19..bfe0bd4d38 100644 --- a/beacon_node/http_api/src/lib.rs +++ b/beacon_node/http_api/src/lib.rs @@ -1640,16 +1640,27 @@ pub fn serve( .and(warp::query::()) .and(warp::path::end()) .and(warp_utils::json::json()) + .and(consensus_version_header_filter) .and(task_spawner_filter.clone()) .and(chain_filter.clone()) .and(network_tx_filter.clone()) .then( move |validation_level: api_types::BroadcastValidationQuery, - blinded_block: Arc>, + blinded_block_json: serde_json::Value, + consensus_version: ForkName, task_spawner: TaskSpawner, chain: Arc>, network_tx: UnboundedSender>| { task_spawner.spawn_async_with_rejection(Priority::P0, async move { + let blinded_block = + SignedBlindedBeaconBlock::::context_deserialize( + &blinded_block_json, + consensus_version, + ) + .map(Arc::new) + .map_err(|e| { + warp_utils::reject::custom_bad_request(format!("invalid JSON: {e:?}")) + })?; publish_blocks::publish_blinded_block( blinded_block, chain, diff --git a/beacon_node/http_api/src/test_utils.rs b/beacon_node/http_api/src/test_utils.rs index 90f2fd2d95..28eed26276 100644 --- a/beacon_node/http_api/src/test_utils.rs +++ b/beacon_node/http_api/src/test_utils.rs @@ -60,8 +60,15 @@ type Mutator = BoxedMutator, MemoryStore>; impl InteractiveTester { pub async fn new(spec: Option, validator_count: usize) -> Self { - Self::new_with_initializer_and_mutator(spec, validator_count, None, None, Config::default()) - .await + Self::new_with_initializer_and_mutator( + spec, + validator_count, + None, + None, + Config::default(), + true, + ) + .await } pub async fn new_with_initializer_and_mutator( @@ -70,6 +77,7 @@ impl InteractiveTester { initializer: Option>, mutator: Option>, config: Config, + use_mock_builder: bool, ) -> Self { let mut harness_builder = BeaconChainHarness::builder(E::default()) .spec_or_default(spec.map(Arc::new)) @@ -91,7 +99,7 @@ impl InteractiveTester { harness_builder = harness_builder.initial_mutator(mutator); } - let harness = harness_builder.build(); + let mut harness = harness_builder.build(); let ApiServer { ctx, @@ -103,6 +111,40 @@ impl InteractiveTester { tokio::spawn(server); + // Late-initalize the mock builder now that the mock execution node and beacon API ports + // have been allocated. + let beacon_api_ip = listening_socket.ip(); + let beacon_api_port = listening_socket.port(); + let beacon_url = + SensitiveUrl::parse(format!("http://{beacon_api_ip}:{beacon_api_port}").as_str()) + .unwrap(); + + // We disable apply_operations because it breaks the mock builder's ability to return + // payloads. + let apply_operations = false; + + // We disable strict registration checks too, because it makes HTTP tests less fiddly to + // write. + let strict_registrations = false; + + // Broadcast to the BN only if Fulu is scheduled. In the broadcast validation tests we want + // to infer things from the builder return code, and pre-Fulu it's simpler to let the BN + // handle broadcast and return detailed codes. Post-Fulu the builder doesn't return the + // block at all, so we *need* the builder to do the broadcast and return a 400 if the block + // is invalid. + let broadcast_to_bn = ctx.chain.as_ref().unwrap().spec.is_fulu_scheduled(); + + if use_mock_builder { + let mock_builder_server = harness.set_mock_builder( + beacon_url.clone(), + strict_registrations, + apply_operations, + broadcast_to_bn, + ); + + tokio::spawn(mock_builder_server); + } + // Override the default timeout to 2s to timeouts on CI, as CI seems to require longer // to process. The 1s timeouts for other tasks have been working for a long time, so we'll // keep it as it is, as it may help identify a performance regression. @@ -110,15 +152,7 @@ impl InteractiveTester { default: Duration::from_secs(2), ..Timeouts::set_all(Duration::from_secs(1)) }; - let client = BeaconNodeHttpClient::new( - SensitiveUrl::parse(&format!( - "http://{}:{}", - listening_socket.ip(), - listening_socket.port() - )) - .unwrap(), - timeouts, - ); + let client = BeaconNodeHttpClient::new(beacon_url.clone(), timeouts); Self { ctx, diff --git a/beacon_node/http_api/tests/broadcast_validation_tests.rs b/beacon_node/http_api/tests/broadcast_validation_tests.rs index c125ae035b..d9ddbf9892 100644 --- a/beacon_node/http_api/tests/broadcast_validation_tests.rs +++ b/beacon_node/http_api/tests/broadcast_validation_tests.rs @@ -1,9 +1,9 @@ use beacon_chain::test_utils::test_spec; use beacon_chain::{ - GossipVerifiedBlock, IntoGossipVerifiedBlock, + GossipVerifiedBlock, IntoGossipVerifiedBlock, WhenSlotSkipped, test_utils::{AttestationStrategy, BlockStrategy}, }; -use eth2::reqwest::StatusCode; +use eth2::reqwest::{Response, StatusCode}; use eth2::types::{BroadcastValidation, PublishBlockRequest}; use http_api::test_utils::InteractiveTester; use http_api::{Config, ProvenancedBlock, publish_blinded_block, publish_block, reconstruct_block}; @@ -74,7 +74,7 @@ pub async fn gossip_invalid() { }) .await; - let response: Result<(), eth2::Error> = tester + let response: Result = tester .client .post_beacon_blocks_v2_ssz(&PublishBlockRequest::new(block, blobs), validation_level) .await; @@ -85,7 +85,13 @@ pub async fn gossip_invalid() { /* mandated by Beacon API spec */ assert_eq!(error_response.status(), Some(StatusCode::BAD_REQUEST)); - assert_server_message_error(error_response, "BAD_REQUEST: NotFinalizedDescendant { block_parent_root: 0x0000000000000000000000000000000000000000000000000000000000000000 }".to_string()); + // Since Deneb, the invalidity of the blobs will be detected prior to the invalidity of the + // block. + let pre_finalized_block_root = Hash256::zero(); + assert_server_message_error( + error_response, + format!("BAD_REQUEST: ParentUnknown {{ parent_root: {pre_finalized_block_root:?} }}"), + ); } /// This test checks that a block that is valid from a gossip perspective is accepted when using `broadcast_validation=gossip`. @@ -123,15 +129,11 @@ pub async fn gossip_partial_pass() { }) .await; - let response: Result<(), eth2::Error> = tester + let response: Result = tester .client .post_beacon_blocks_v2_ssz(&PublishBlockRequest::new(block, blobs), validation_level) .await; - assert!(response.is_err()); - - let error_response = response.unwrap_err(); - - assert_eq!(error_response.status(), Some(StatusCode::ACCEPTED)); + assert_eq!(response.unwrap().status(), StatusCode::ACCEPTED); } // This test checks that a block that is valid from both a gossip and consensus perspective is accepted when using `broadcast_validation=gossip`. @@ -164,7 +166,7 @@ pub async fn gossip_full_pass() { let state_a = tester.harness.get_current_state(); let ((block, blobs), _) = tester.harness.make_block(state_a, slot_b).await; - let response: Result<(), eth2::Error> = tester + let response: Result = tester .client .post_beacon_blocks_v2_ssz( &PublishBlockRequest::new(block.clone(), blobs), @@ -215,7 +217,7 @@ pub async fn gossip_full_pass_ssz() { let (block_contents_tuple, _) = tester.harness.make_block(state_a, slot_b).await; let block_contents = block_contents_tuple.into(); - let response: Result<(), eth2::Error> = tester + let response: Result = tester .client .post_beacon_blocks_v2_ssz(&block_contents, validation_level) .await; @@ -264,7 +266,7 @@ pub async fn consensus_invalid() { }) .await; - let response: Result<(), eth2::Error> = tester + let response: Result = tester .client .post_beacon_blocks_v2_ssz(&PublishBlockRequest::new(block, blobs), validation_level) .await; @@ -274,7 +276,13 @@ pub async fn consensus_invalid() { /* mandated by Beacon API spec */ assert_eq!(error_response.status(), Some(StatusCode::BAD_REQUEST)); - assert_server_message_error(error_response, "BAD_REQUEST: NotFinalizedDescendant { block_parent_root: 0x0000000000000000000000000000000000000000000000000000000000000000 }".to_string()); + // Since Deneb, the invalidity of the blobs will be detected prior to the invalidity of the + // block. + let pre_finalized_block_root = Hash256::zero(); + assert_server_message_error( + error_response, + format!("BAD_REQUEST: ParentUnknown {{ parent_root: {pre_finalized_block_root:?} }}"), + ); } /// This test checks that a block that is only valid from a gossip perspective is rejected when using `broadcast_validation=consensus`. @@ -304,13 +312,17 @@ pub async fn consensus_gossip() { let slot_a = Slot::new(num_initial); let slot_b = slot_a + 1; + let mut correct_state_root = Hash256::ZERO; let state_a = tester.harness.get_current_state(); let ((block, blobs), _) = tester .harness - .make_block_with_modifier(state_a, slot_b, |b| *b.state_root_mut() = Hash256::zero()) + .make_block_with_modifier(state_a, slot_b, |b| { + *correct_state_root = *b.state_root(); + *b.state_root_mut() = Hash256::zero() + }) .await; - let response: Result<(), eth2::Error> = tester + let response: Result = tester .client .post_beacon_blocks_v2_ssz(&PublishBlockRequest::new(block, blobs), validation_level) .await; @@ -320,7 +332,14 @@ pub async fn consensus_gossip() { /* mandated by Beacon API spec */ assert_eq!(error_response.status(), Some(StatusCode::BAD_REQUEST)); - assert_server_message_error(error_response, "BAD_REQUEST: Invalid block: StateRootMismatch { block: 0x0000000000000000000000000000000000000000000000000000000000000000, local: 0x253405be9aa159bce7b276b8e1d3849c743e673118dfafe8c7d07c203ae0d80d }".to_string()); + assert_server_message_error( + error_response, + format!( + "BAD_REQUEST: Invalid block: StateRootMismatch {{ block: {}, \ + local: {correct_state_root:?} }}", + Hash256::ZERO + ), + ); } /// This test checks that a block that is valid from both a gossip and consensus perspective, but nonetheless equivocates, is accepted when using `broadcast_validation=consensus`. @@ -424,7 +443,7 @@ pub async fn consensus_full_pass() { let state_a = tester.harness.get_current_state(); let ((block, blobs), _) = tester.harness.make_block(state_a, slot_b).await; - let response: Result<(), eth2::Error> = tester + let response: Result = tester .client .post_beacon_blocks_v2_ssz( &PublishBlockRequest::new(block.clone(), blobs), @@ -478,7 +497,7 @@ pub async fn equivocation_invalid() { }) .await; - let response: Result<(), eth2::Error> = tester + let response: Result = tester .client .post_beacon_blocks_v2_ssz(&PublishBlockRequest::new(block, blobs), validation_level) .await; @@ -488,7 +507,13 @@ pub async fn equivocation_invalid() { /* mandated by Beacon API spec */ assert_eq!(error_response.status(), Some(StatusCode::BAD_REQUEST)); - assert_server_message_error(error_response, "BAD_REQUEST: NotFinalizedDescendant { block_parent_root: 0x0000000000000000000000000000000000000000000000000000000000000000 }".to_string()); + // Since Deneb, the invalidity of the blobs will be detected prior to the invalidity of the + // block. + let pre_finalized_block_root = Hash256::zero(); + assert_server_message_error( + error_response, + format!("BAD_REQUEST: ParentUnknown {{ parent_root: {pre_finalized_block_root:?} }}"), + ); } /// This test checks that a block that is valid from both a gossip and consensus perspective is rejected when using `broadcast_validation=consensus_and_equivocation`. @@ -554,7 +579,7 @@ pub async fn equivocation_consensus_early_equivocation() { ); /* submit `block_b` which should induce equivocation */ - let response: Result<(), eth2::Error> = tester + let response: Result = tester .client .post_beacon_blocks_v2_ssz( &PublishBlockRequest::new(block_b.clone(), blobs_b), @@ -597,14 +622,18 @@ pub async fn equivocation_gossip() { let slot_a = Slot::new(num_initial); let slot_b = slot_a + 1; + let mut correct_state_root = Hash256::zero(); let state_a = tester.harness.get_current_state(); let ((block, blobs), _) = tester .harness - .make_block_with_modifier(state_a, slot_b, |b| *b.state_root_mut() = Hash256::zero()) + .make_block_with_modifier(state_a, slot_b, |b| { + *correct_state_root = *b.state_root(); + *b.state_root_mut() = Hash256::zero() + }) .await; - let response: Result<(), eth2::Error> = tester + let response: Result = tester .client .post_beacon_blocks_v2_ssz(&PublishBlockRequest::new(block, blobs), validation_level) .await; @@ -614,7 +643,13 @@ pub async fn equivocation_gossip() { /* mandated by Beacon API spec */ assert_eq!(error_response.status(), Some(StatusCode::BAD_REQUEST)); - assert_server_message_error(error_response, "BAD_REQUEST: Invalid block: StateRootMismatch { block: 0x0000000000000000000000000000000000000000000000000000000000000000, local: 0x253405be9aa159bce7b276b8e1d3849c743e673118dfafe8c7d07c203ae0d80d }".to_string()); + assert_server_message_error( + error_response, + format!( + "BAD_REQUEST: Invalid block: StateRootMismatch {{ block: {}, local: {correct_state_root} }}", + Hash256::zero() + ), + ); } /// This test checks that a block that is valid from both a gossip and consensus perspective but @@ -725,7 +760,7 @@ pub async fn equivocation_full_pass() { let state_a = tester.harness.get_current_state(); let ((block, blobs), _) = tester.harness.make_block(state_a, slot_b).await; - let response: Result<(), eth2::Error> = tester + let response: Result = tester .client .post_beacon_blocks_v2_ssz( &PublishBlockRequest::new(block.clone(), blobs), @@ -770,28 +805,43 @@ pub async fn blinded_gossip_invalid() { tester.harness.advance_slot(); - let (block_contents_tuple, _) = tester + let (blinded_block, _) = tester .harness - .make_block_with_modifier(chain_state_before, slot, |b| { + .make_blinded_block_with_modifier(chain_state_before, slot, |b| { *b.state_root_mut() = Hash256::zero(); *b.parent_root_mut() = Hash256::zero(); }) .await; - let response: Result<(), eth2::Error> = tester + let response: Result = tester .client - .post_beacon_blinded_blocks_v2(&block_contents_tuple.0.clone_as_blinded(), validation_level) + .post_beacon_blinded_blocks_v2(&blinded_block, validation_level) .await; assert!(response.is_err()); let error_response: eth2::Error = response.err().unwrap(); - + let pre_finalized_block_root = Hash256::zero(); /* mandated by Beacon API spec */ - assert_eq!(error_response.status(), Some(StatusCode::BAD_REQUEST)); - assert_server_message_error(error_response, "BAD_REQUEST: NotFinalizedDescendant { block_parent_root: 0x0000000000000000000000000000000000000000000000000000000000000000 }".to_string()); + if tester.harness.spec.is_fulu_scheduled() { + // XXX: this should be a 400 but is a 500 due to the mock-builder being janky + assert_eq!( + error_response.status(), + Some(StatusCode::INTERNAL_SERVER_ERROR) + ); + } else { + assert_eq!(error_response.status(), Some(StatusCode::BAD_REQUEST)); + assert_server_message_error( + error_response, + format!("BAD_REQUEST: ParentUnknown {{ parent_root: {pre_finalized_block_root:?} }}"), + ); + } } -/// This test checks that a block that is valid from a gossip perspective is accepted when using `broadcast_validation=gossip`. +/// Process a blinded block that is invalid, but valid on gossip. +/// +/// Due to the checks conducted by the "relay" (mock-builder) when `broadcast_to_bn` is set (post +/// Fulu), we can't always assert that we get a 202 status for this block -- post Fulu the relay +/// detects it as invalid and the BN returns an error. #[tokio::test(flavor = "multi_thread", worker_threads = 2)] pub async fn blinded_gossip_partial_pass() { /* this test targets gossip-level validation */ @@ -819,22 +869,27 @@ pub async fn blinded_gossip_partial_pass() { tester.harness.advance_slot(); - let (block_contents_tuple, _) = tester + let (blinded_block, _) = tester .harness - .make_block_with_modifier(chain_state_before, slot, |b| { + .make_blinded_block_with_modifier(chain_state_before, slot, |b| { *b.state_root_mut() = Hash256::zero() }) .await; - let response: Result<(), eth2::Error> = tester + let response: Result = tester .client - .post_beacon_blinded_blocks_v2(&block_contents_tuple.0.clone_as_blinded(), validation_level) + .post_beacon_blinded_blocks_v2(&blinded_block, validation_level) .await; - assert!(response.is_err()); - - let error_response = response.unwrap_err(); - - assert_eq!(error_response.status(), Some(StatusCode::ACCEPTED)); + if tester.harness.spec.is_fulu_scheduled() { + let error_response = response.unwrap_err(); + // XXX: this should be a 400 but is a 500 due to the mock-builder being janky + assert_eq!( + error_response.status(), + Some(StatusCode::INTERNAL_SERVER_ERROR) + ); + } else { + assert_eq!(response.unwrap().status(), StatusCode::ACCEPTED); + } } // This test checks that a block that is valid from both a gossip and consensus perspective is accepted when using `broadcast_validation=gossip`. @@ -866,12 +921,13 @@ pub async fn blinded_gossip_full_pass() { let state_a = tester.harness.get_current_state(); let (blinded_block, _) = tester.harness.make_blinded_block(state_a, slot_b).await; - let response: Result<(), eth2::Error> = tester + let response: Result = tester .client .post_beacon_blinded_blocks_v2(&blinded_block, validation_level) .await; assert!(response.is_ok()); + assert_eq!(response.unwrap().status(), StatusCode::OK); assert!( tester .harness @@ -910,12 +966,13 @@ pub async fn blinded_gossip_full_pass_ssz() { let state_a = tester.harness.get_current_state(); let (blinded_block, _) = tester.harness.make_blinded_block(state_a, slot_b).await; - let response: Result<(), eth2::Error> = tester + let response: Result = tester .client .post_beacon_blinded_blocks_v2_ssz(&blinded_block, validation_level) .await; assert!(response.is_ok()); + assert_eq!(response.unwrap().status(), StatusCode::OK); assert!( tester .harness @@ -933,7 +990,7 @@ pub async fn blinded_consensus_invalid() { // Validator count needs to be at least 32 or proposer boost gets set to 0 when computing // `validator_count // 32`. let validator_count = 64; - let num_initial: u64 = 31; + let num_initial: u64 = 256; let tester = InteractiveTester::::new(None, validator_count).await; // Create some chain depth. @@ -952,25 +1009,48 @@ pub async fn blinded_consensus_invalid() { tester.harness.advance_slot(); - let (block_contents_tuple, _) = tester + let finalized_slot = chain_state_before + .finalized_checkpoint() + .epoch + .start_slot(E::slots_per_epoch()); + assert_ne!(finalized_slot, 0); + let pre_finalized_block_root = tester .harness - .make_block_with_modifier(chain_state_before, slot, |b| { + .chain + .block_root_at_slot(finalized_slot - 1, WhenSlotSkipped::Prev) + .unwrap() + .unwrap(); + + let (blinded_block, _) = tester + .harness + .make_blinded_block_with_modifier(chain_state_before, slot, |b| { *b.state_root_mut() = Hash256::zero(); - *b.parent_root_mut() = Hash256::zero(); + *b.parent_root_mut() = pre_finalized_block_root; }) .await; - let response: Result<(), eth2::Error> = tester + let response: Result = tester .client - .post_beacon_blinded_blocks_v2(&block_contents_tuple.0.clone_as_blinded(), validation_level) + .post_beacon_blinded_blocks_v2(&blinded_block, validation_level) .await; assert!(response.is_err()); let error_response: eth2::Error = response.err().unwrap(); /* mandated by Beacon API spec */ - assert_eq!(error_response.status(), Some(StatusCode::BAD_REQUEST)); - assert_server_message_error(error_response, "BAD_REQUEST: NotFinalizedDescendant { block_parent_root: 0x0000000000000000000000000000000000000000000000000000000000000000 }".to_string()); + if tester.harness.spec.is_fulu_scheduled() { + // XXX: this should be a 400 but is a 500 due to the mock-builder being janky + assert_eq!( + error_response.status(), + Some(StatusCode::INTERNAL_SERVER_ERROR) + ); + } else { + assert_eq!(error_response.status(), Some(StatusCode::BAD_REQUEST)); + assert_server_message_error( + error_response, + format!("BAD_REQUEST: ParentUnknown {{ parent_root: {pre_finalized_block_root:?} }}"), + ); + } } /// This test checks that a block that is only valid from a gossip perspective is rejected when using `broadcast_validation=consensus`. @@ -1000,23 +1080,44 @@ pub async fn blinded_consensus_gossip() { let slot_a = Slot::new(num_initial); let slot_b = slot_a + 1; + let mut correct_state_root = Hash256::zero(); + let state_a = tester.harness.get_current_state(); - let (block_contents_tuple, _) = tester + let (blinded_block, _) = tester .harness - .make_block_with_modifier(state_a, slot_b, |b| *b.state_root_mut() = Hash256::zero()) + .make_blinded_block_with_modifier(state_a, slot_b, |b| { + *correct_state_root = *b.state_root(); + *b.state_root_mut() = Hash256::zero() + }) .await; - let response: Result<(), eth2::Error> = tester + let response: Result = tester .client - .post_beacon_blinded_blocks_v2(&block_contents_tuple.0.clone_as_blinded(), validation_level) + .post_beacon_blinded_blocks_v2(&blinded_block, validation_level) .await; + assert!(response.is_err()); let error_response: eth2::Error = response.err().unwrap(); /* mandated by Beacon API spec */ - assert_eq!(error_response.status(), Some(StatusCode::BAD_REQUEST)); - assert_server_message_error(error_response, "BAD_REQUEST: Invalid block: StateRootMismatch { block: 0x0000000000000000000000000000000000000000000000000000000000000000, local: 0x253405be9aa159bce7b276b8e1d3849c743e673118dfafe8c7d07c203ae0d80d }".to_string()); + if tester.harness.spec.is_fulu_scheduled() { + // XXX: this should be a 400 but is a 500 due to the mock-builder being janky + assert_eq!( + error_response.status(), + Some(StatusCode::INTERNAL_SERVER_ERROR) + ); + } else { + assert_eq!(error_response.status(), Some(StatusCode::BAD_REQUEST)); + assert_server_message_error( + error_response, + format!( + "BAD_REQUEST: Invalid block: StateRootMismatch {{ block: {}, \ + local: {correct_state_root} }}", + Hash256::ZERO + ), + ); + } } /// This test checks that a block that is valid from both a gossip and consensus perspective is accepted when using `broadcast_validation=consensus`. @@ -1049,7 +1150,7 @@ pub async fn blinded_consensus_full_pass() { let state_a = tester.harness.get_current_state(); let (blinded_block, _) = tester.harness.make_blinded_block(state_a, slot_b).await; - let response: Result<(), eth2::Error> = tester + let response: Result = tester .client .post_beacon_blinded_blocks_v2(&blinded_block, validation_level) .await; @@ -1073,7 +1174,7 @@ pub async fn blinded_equivocation_invalid() { // Validator count needs to be at least 32 or proposer boost gets set to 0 when computing // `validator_count // 32`. let validator_count = 64; - let num_initial: u64 = 31; + let num_initial: u64 = 256; let tester = InteractiveTester::::new(None, validator_count).await; // Create some chain depth. @@ -1092,25 +1193,47 @@ pub async fn blinded_equivocation_invalid() { tester.harness.advance_slot(); - let (block_contents_tuple, _) = tester + let finalized_slot = chain_state_before + .finalized_checkpoint() + .epoch + .start_slot(E::slots_per_epoch()); + assert_ne!(finalized_slot, 0); + let pre_finalized_block_root = tester .harness - .make_block_with_modifier(chain_state_before, slot, |b| { + .chain + .block_root_at_slot(finalized_slot - 1, WhenSlotSkipped::Prev) + .unwrap() + .unwrap(); + + let (blinded_block, _) = tester + .harness + .make_blinded_block_with_modifier(chain_state_before, slot, |b| { *b.state_root_mut() = Hash256::zero(); - *b.parent_root_mut() = Hash256::zero(); + *b.parent_root_mut() = pre_finalized_block_root; }) .await; - let response: Result<(), eth2::Error> = tester + let response: Result = tester .client - .post_beacon_blinded_blocks_v2(&block_contents_tuple.0.clone_as_blinded(), validation_level) + .post_beacon_blinded_blocks_v2(&blinded_block, validation_level) .await; assert!(response.is_err()); let error_response: eth2::Error = response.err().unwrap(); /* mandated by Beacon API spec */ - assert_eq!(error_response.status(), Some(StatusCode::BAD_REQUEST)); - assert_server_message_error(error_response, "BAD_REQUEST: NotFinalizedDescendant { block_parent_root: 0x0000000000000000000000000000000000000000000000000000000000000000 }".to_string()); + if tester.harness.spec.is_fulu_scheduled() { + assert_eq!( + error_response.status(), + Some(StatusCode::INTERNAL_SERVER_ERROR) + ); + } else { + assert_eq!(error_response.status(), Some(StatusCode::BAD_REQUEST)); + assert_server_message_error( + error_response, + format!("BAD_REQUEST: ParentUnknown {{ parent_root: {pre_finalized_block_root:?} }}"), + ); + } } /// This test checks that a block that is valid from both a gossip and consensus perspective is rejected when using `broadcast_validation=consensus_and_equivocation`. @@ -1160,13 +1283,11 @@ pub async fn blinded_equivocation_consensus_early_equivocation() { assert_ne!(block_a.state_root(), block_b.state_root()); /* submit `block_a` as valid */ - assert!( - tester - .client - .post_beacon_blinded_blocks_v2(&block_a, validation_level) - .await - .is_ok() - ); + tester + .client + .post_beacon_blinded_blocks_v2(&block_a, validation_level) + .await + .unwrap(); assert!( tester .harness @@ -1175,7 +1296,7 @@ pub async fn blinded_equivocation_consensus_early_equivocation() { ); /* submit `block_b` which should induce equivocation */ - let response: Result<(), eth2::Error> = tester + let response: Result = tester .client .post_beacon_blinded_blocks_v2(&block_b, validation_level) .await; @@ -1183,8 +1304,15 @@ pub async fn blinded_equivocation_consensus_early_equivocation() { let error_response: eth2::Error = response.err().unwrap(); - assert_eq!(error_response.status(), Some(StatusCode::BAD_REQUEST)); - assert_server_message_error(error_response, "BAD_REQUEST: Slashable".to_string()); + if tester.harness.spec.is_fulu_scheduled() { + assert_eq!( + error_response.status(), + Some(StatusCode::INTERNAL_SERVER_ERROR) + ); + } else { + assert_eq!(error_response.status(), Some(StatusCode::BAD_REQUEST)); + assert_server_message_error(error_response, "BAD_REQUEST: Slashable".to_string()); + } } /// This test checks that a block that is only valid from a gossip perspective is rejected when using `broadcast_validation=consensus_and_equivocation`. @@ -1215,24 +1343,41 @@ pub async fn blinded_equivocation_gossip() { let slot_a = Slot::new(num_initial); let slot_b = slot_a + 1; + let mut correct_state_root = Hash256::zero(); let state_a = tester.harness.get_current_state(); - let (block_contents_tuple, _) = tester + let (blinded_block, _) = tester .harness - .make_block_with_modifier(state_a, slot_b, |b| *b.state_root_mut() = Hash256::zero()) + .make_blinded_block_with_modifier(state_a, slot_b, |b| { + *correct_state_root = *b.state_root(); + *b.state_root_mut() = Hash256::zero() + }) .await; - let response: Result<(), eth2::Error> = tester + let response: Result = tester .client - .post_beacon_blinded_blocks_v2(&block_contents_tuple.0.clone_as_blinded(), validation_level) + .post_beacon_blinded_blocks_v2(&blinded_block, validation_level) .await; - assert!(response.is_err()); + assert!(response.is_err()); let error_response: eth2::Error = response.err().unwrap(); /* mandated by Beacon API spec */ - assert_eq!(error_response.status(), Some(StatusCode::BAD_REQUEST)); - - assert_server_message_error(error_response, "BAD_REQUEST: Invalid block: StateRootMismatch { block: 0x0000000000000000000000000000000000000000000000000000000000000000, local: 0x253405be9aa159bce7b276b8e1d3849c743e673118dfafe8c7d07c203ae0d80d }".to_string()); + if tester.harness.spec.is_fulu_scheduled() { + // XXX: this should be a 400 but is a 500 due to the mock-builder being janky + assert_eq!( + error_response.status(), + Some(StatusCode::INTERNAL_SERVER_ERROR) + ); + } else { + assert_eq!(error_response.status(), Some(StatusCode::BAD_REQUEST)); + assert_server_message_error( + error_response, + format!( + "BAD_REQUEST: Invalid block: StateRootMismatch {{ block: {}, local: {correct_state_root} }}", + Hash256::zero() + ), + ); + } } /// This test checks that a block that is valid from both a gossip and @@ -1287,54 +1432,58 @@ pub async fn blinded_equivocation_consensus_late_equivocation() { ); assert_ne!(block_a.state_root(), block_b.state_root()); - let unblinded_block_a = reconstruct_block( - tester.harness.chain.clone(), - block_a.canonical_root(), - Arc::new(block_a), - ) - .await - .expect("failed to reconstruct block") - .expect("block expected"); + // From fulu builders never send back a full payload, hence further checks in this test + // are not possible + if !tester.harness.spec.is_fulu_scheduled() { + let unblinded_block_a = reconstruct_block( + tester.harness.chain.clone(), + block_a.canonical_root(), + Arc::new(block_a), + ) + .await + .expect("failed to reconstruct block") + .expect("block expected"); - let unblinded_block_b = reconstruct_block( - tester.harness.chain.clone(), - block_b.canonical_root(), - block_b.clone(), - ) - .await - .expect("failed to reconstruct block") - .expect("block expected"); + let unblinded_block_b = reconstruct_block( + tester.harness.chain.clone(), + block_b.canonical_root(), + block_b.clone(), + ) + .await + .expect("failed to reconstruct block") + .expect("block expected"); - let inner_block_a = match unblinded_block_a { - ProvenancedBlock::Local(a, _, _) => a, - ProvenancedBlock::Builder(a, _, _) => a, - }; - let inner_block_b = match unblinded_block_b { - ProvenancedBlock::Local(b, _, _) => b, - ProvenancedBlock::Builder(b, _, _) => b, - }; + let inner_block_a = match unblinded_block_a { + ProvenancedBlock::Local(a, _, _) => a, + ProvenancedBlock::Builder(a, _, _) => a, + }; + let inner_block_b = match unblinded_block_b { + ProvenancedBlock::Local(b, _, _) => b, + ProvenancedBlock::Builder(b, _, _) => b, + }; - let gossip_block_b = GossipVerifiedBlock::new(inner_block_b, &tester.harness.chain); - assert!(gossip_block_b.is_ok()); - let gossip_block_a = GossipVerifiedBlock::new(inner_block_a, &tester.harness.chain); - assert!(gossip_block_a.is_err()); + let gossip_block_b = GossipVerifiedBlock::new(inner_block_b, &tester.harness.chain); + assert!(gossip_block_b.is_ok()); + let gossip_block_a = GossipVerifiedBlock::new(inner_block_a, &tester.harness.chain); + assert!(gossip_block_a.is_err()); - let channel = tokio::sync::mpsc::unbounded_channel(); + let channel = tokio::sync::mpsc::unbounded_channel(); - let publication_result = publish_blinded_block( - block_b, - tester.harness.chain, - &channel.0, - validation_level, - StatusCode::ACCEPTED, - ) - .await; + let publication_result = publish_blinded_block( + block_b, + tester.harness.chain, + &channel.0, + validation_level, + StatusCode::ACCEPTED, + ) + .await; - assert!(publication_result.is_err()); + assert!(publication_result.is_err()); - let publication_error: Rejection = publication_result.unwrap_err(); + let publication_error: Rejection = publication_result.unwrap_err(); - assert!(publication_error.find::().is_some()); + assert!(publication_error.find::().is_some()); + } } /// This test checks that a block that is valid from both a gossip and consensus perspective (and does not equivocate) is accepted when using `broadcast_validation=consensus_and_equivocation`. @@ -1368,7 +1517,7 @@ pub async fn blinded_equivocation_full_pass() { let state_a = tester.harness.get_current_state(); let (block, _) = tester.harness.make_blinded_block(state_a, slot_b).await; - let response: Result<(), eth2::Error> = tester + let response: Result = tester .client .post_beacon_blinded_blocks_v2(&block, validation_level) .await; @@ -1434,7 +1583,7 @@ pub async fn block_seen_on_gossip_without_blobs_or_columns() { ); // Post the block *and* blobs to the HTTP API. - let response: Result<(), eth2::Error> = tester + let response: Result = tester .client .post_beacon_blocks_v2_ssz( &PublishBlockRequest::new(block.clone(), Some(blobs)), @@ -1522,7 +1671,7 @@ pub async fn block_seen_on_gossip_with_some_blobs_or_columns() { ); // Post the block *and* all blobs to the HTTP API. - let response: Result<(), eth2::Error> = tester + let response: Result = tester .client .post_beacon_blocks_v2_ssz( &PublishBlockRequest::new(block.clone(), Some(blobs)), @@ -1597,7 +1746,7 @@ pub async fn blobs_or_columns_seen_on_gossip_without_block() { ); // Post the block *and* all blobs to the HTTP API. - let response: Result<(), eth2::Error> = tester + let response: Result = tester .client .post_beacon_blocks_v2_ssz( &PublishBlockRequest::new(block.clone(), Some((kzg_proofs, blobs))), @@ -1672,7 +1821,7 @@ async fn blobs_or_columns_seen_on_gossip_without_block_and_no_http_blobs_or_colu ); // Post just the block to the HTTP API (blob lists are empty). - let response: Result<(), eth2::Error> = tester + let response: Result = tester .client .post_beacon_blocks_v2_ssz( &PublishBlockRequest::new( @@ -1750,7 +1899,7 @@ async fn slashable_blobs_or_columns_seen_on_gossip_cause_failure() { ); // Post block A *and* all its blobs to the HTTP API. - let response: Result<(), eth2::Error> = tester + let response: Result = tester .client .post_beacon_blocks_v2_ssz( &PublishBlockRequest::new(block_a.clone(), Some((kzg_proofs_a, blobs_a))), @@ -1788,6 +1937,7 @@ pub async fn duplicate_block_status_code() { duplicate_block_status_code, ..Config::default() }, + true, ) .await; @@ -1812,7 +1962,7 @@ pub async fn duplicate_block_status_code() { // Post the block blobs to the HTTP API once. let block_request = PublishBlockRequest::new(block.clone(), Some((kzg_proofs, blobs))); - let response: Result<(), eth2::Error> = tester + let response: Result = tester .client .post_beacon_blocks_v2_ssz(&block_request, validation_level) .await; @@ -1827,7 +1977,7 @@ pub async fn duplicate_block_status_code() { ); // Post again. - let duplicate_response: Result<(), eth2::Error> = tester + let duplicate_response: Result = tester .client .post_beacon_blocks_v2_ssz(&block_request, validation_level) .await; diff --git a/beacon_node/http_api/tests/fork_tests.rs b/beacon_node/http_api/tests/fork_tests.rs index 880e206777..62a3461276 100644 --- a/beacon_node/http_api/tests/fork_tests.rs +++ b/beacon_node/http_api/tests/fork_tests.rs @@ -425,6 +425,7 @@ async fn bls_to_execution_changes_update_all_around_capella_fork() { })), None, Default::default(), + true, ) .await; let harness = &tester.harness; diff --git a/beacon_node/http_api/tests/interactive_tests.rs b/beacon_node/http_api/tests/interactive_tests.rs index 1e55bfb7b3..1398d8c72f 100644 --- a/beacon_node/http_api/tests/interactive_tests.rs +++ b/beacon_node/http_api/tests/interactive_tests.rs @@ -73,6 +73,7 @@ async fn state_by_root_pruned_from_fork_choice() { })), None, Default::default(), + false, ) .await; @@ -429,6 +430,7 @@ pub async fn proposer_boost_re_org_test( ) })), Default::default(), + false, ) .await; let harness = &tester.harness; @@ -666,6 +668,7 @@ pub async fn proposer_boost_re_org_test( // Check the fork choice updates that were sent. let forkchoice_updates = forkchoice_updates.lock(); + let block_a_exec_hash = block_a .0 .message() diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index 92abbd84c7..91f8666381 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -1,14 +1,16 @@ use beacon_chain::test_utils::RelativeSyncCommittee; use beacon_chain::{ BeaconChain, ChainConfig, StateSkipConfig, WhenSlotSkipped, - test_utils::{AttestationStrategy, BeaconChainHarness, BlockStrategy, EphemeralHarnessType}, + test_utils::{ + AttestationStrategy, BeaconChainHarness, BlockStrategy, EphemeralHarnessType, test_spec, + }, }; use eth2::{ BeaconNodeHttpClient, Error, Error::ServerMessage, StatusCode, Timeouts, mixin::{RequestAccept, ResponseForkName, ResponseOptional}, - reqwest::RequestBuilder, + reqwest::{RequestBuilder, Response}, types::{ BlockId as CoreBlockId, ForkChoiceNode, ProduceBlockV3Response, StateId as CoreStateId, *, }, @@ -113,15 +115,11 @@ impl ApiTester { Self::new_from_config(ApiTesterConfig::default()).await } - pub async fn new_with_hard_forks(altair: bool, bellatrix: bool) -> Self { - let mut config = ApiTesterConfig::default(); - // Set whether the chain has undergone each hard fork. - if altair { - config.spec.altair_fork_epoch = Some(Epoch::new(0)); - } - if bellatrix { - config.spec.bellatrix_fork_epoch = Some(Epoch::new(0)); - } + pub async fn new_with_hard_forks() -> Self { + let config = ApiTesterConfig { + spec: test_spec::(), + ..Default::default() + }; Self::new_from_config(config).await } @@ -291,7 +289,19 @@ impl ApiTester { let beacon_api_port = listening_socket.port(); let beacon_url = SensitiveUrl::parse(format!("http://127.0.0.1:{beacon_api_port}").as_str()).unwrap(); - let mock_builder_server = harness.set_mock_builder(beacon_url.clone()); + + // Be strict with validator registrations, but don't bother applying operations, that flag + // is only used by mock-builder tests. + let strict_registrations = true; + let apply_operations = true; + let broadcast_to_bn = true; + + let mock_builder_server = harness.set_mock_builder( + beacon_url.clone(), + strict_registrations, + apply_operations, + broadcast_to_bn, + ); // Start the mock builder service prior to building the chain out. harness @@ -334,6 +344,7 @@ impl ApiTester { .deterministic_keypairs(VALIDATOR_COUNT) .deterministic_withdrawal_keypairs(VALIDATOR_COUNT) .fresh_ephemeral_store() + .mock_execution_layer() .build(), ); @@ -419,7 +430,7 @@ impl ApiTester { } pub async fn new_mev_tester() -> Self { - let tester = Self::new_with_hard_forks(true, true) + let tester = Self::new_with_hard_forks() .await .test_post_validator_register_validator() .await; @@ -1539,7 +1550,10 @@ impl ApiTester { pub async fn test_post_beacon_blocks_valid(mut self) -> Self { let next_block = self.next_block.clone(); - self.client.post_beacon_blocks(&next_block).await.unwrap(); + self.client + .post_beacon_blocks_v2(&next_block, None) + .await + .unwrap(); assert!( self.network_rx.network_recv.recv().await.is_some(), @@ -1553,7 +1567,7 @@ impl ApiTester { let next_block = &self.next_block; self.client - .post_beacon_blocks_ssz(next_block) + .post_beacon_blocks_v2_ssz(next_block, None) .await .unwrap(); @@ -1578,12 +1592,14 @@ impl ApiTester { .await .0; - assert!( - self.client - .post_beacon_blocks(&PublishBlockRequest::from(block)) - .await - .is_err() - ); + let response: Result = self + .client + .post_beacon_blocks_v2(&PublishBlockRequest::from(block), None) + .await; + + assert!(response.is_ok()); + + assert_eq!(response.unwrap().status(), StatusCode::ACCEPTED); assert!( self.network_rx.network_recv.recv().await.is_some(), @@ -1606,13 +1622,13 @@ impl ApiTester { .await .0; - assert!( - self.client - .post_beacon_blocks_ssz(&PublishBlockRequest::from(block)) - .await - .is_err() - ); + let response: Result = self + .client + .post_beacon_blocks_v2(&PublishBlockRequest::from(block), None) + .await; + assert!(response.is_ok()); + assert_eq!(response.unwrap().status(), StatusCode::ACCEPTED); assert!( self.network_rx.network_recv.recv().await.is_some(), "gossip valid blocks should be sent to network" @@ -1634,7 +1650,7 @@ impl ApiTester { assert!( self.client - .post_beacon_blocks(&block_contents) + .post_beacon_blocks_v2(&block_contents, None) .await .is_ok() ); @@ -1644,45 +1660,25 @@ impl ApiTester { // Test all the POST methods in sequence, they should all behave the same. let responses = vec![ - self.client - .post_beacon_blocks(&block_contents) - .await - .unwrap_err(), self.client .post_beacon_blocks_v2(&block_contents, None) .await - .unwrap_err(), - self.client - .post_beacon_blocks_ssz(&block_contents) - .await - .unwrap_err(), + .unwrap(), self.client .post_beacon_blocks_v2_ssz(&block_contents, None) .await - .unwrap_err(), - self.client - .post_beacon_blinded_blocks(&blinded_block_contents) - .await - .unwrap_err(), + .unwrap(), self.client .post_beacon_blinded_blocks_v2(&blinded_block_contents, None) .await - .unwrap_err(), - self.client - .post_beacon_blinded_blocks_ssz(&blinded_block_contents) - .await - .unwrap_err(), + .unwrap(), self.client .post_beacon_blinded_blocks_v2_ssz(&blinded_block_contents, None) .await - .unwrap_err(), + .unwrap(), ]; for (i, response) in responses.into_iter().enumerate() { - assert_eq!( - response.status().unwrap(), - StatusCode::ACCEPTED, - "response {i}" - ); + assert_eq!(response.status(), StatusCode::ACCEPTED, "response {i}"); } self @@ -3405,7 +3401,7 @@ impl ApiTester { PublishBlockRequest::try_from(Arc::new(signed_block.clone())).unwrap(); self.client - .post_beacon_blocks(&signed_block_contents) + .post_beacon_blocks_v2(&signed_block_contents, None) .await .unwrap(); @@ -3470,7 +3466,7 @@ impl ApiTester { block_contents.sign(&sk, &fork, genesis_validators_root, &self.chain.spec); self.client - .post_beacon_blocks_ssz(&signed_block_contents) + .post_beacon_blocks_v2_ssz(&signed_block_contents, None) .await .unwrap(); @@ -3588,7 +3584,7 @@ impl ApiTester { block_contents.sign(&sk, &fork, genesis_validators_root, &self.chain.spec); self.client - .post_beacon_blocks_ssz(&signed_block_contents) + .post_beacon_blocks_v2_ssz(&signed_block_contents, None) .await .unwrap(); @@ -6394,7 +6390,7 @@ impl ApiTester { }); self.client - .post_beacon_blocks(&self.next_block) + .post_beacon_blocks_v2(&self.next_block, None) .await .unwrap(); @@ -6439,7 +6435,7 @@ impl ApiTester { self.harness.advance_slot(); self.client - .post_beacon_blocks(&self.reorg_block) + .post_beacon_blocks_v2(&self.reorg_block, None) .await .unwrap(); @@ -6661,7 +6657,7 @@ impl ApiTester { }); self.client - .post_beacon_blocks(&self.next_block) + .post_beacon_blocks_v2(&self.next_block, None) .await .unwrap(); @@ -7829,7 +7825,7 @@ async fn lighthouse_endpoints() { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn optimistic_responses() { - ApiTester::new_with_hard_forks(true, true) + ApiTester::new_with_hard_forks() .await .test_check_optimistic_responses() .await; diff --git a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs index a53e76402e..cb6d63fe91 100644 --- a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs @@ -840,7 +840,7 @@ impl NetworkBeaconProcessor { } Err(err) => { match err { - GossipBlobError::BlobParentUnknown { parent_root } => { + GossipBlobError::ParentUnknown { parent_root } => { debug!( action = "requesting parent", block_root = %root, diff --git a/common/eth2/src/lib.rs b/common/eth2/src/lib.rs index bbc38e31d6..3368569d59 100644 --- a/common/eth2/src/lib.rs +++ b/common/eth2/src/lib.rs @@ -498,7 +498,7 @@ impl BeaconNodeHttpClient { .post(url) .timeout(timeout.unwrap_or(self.timeouts.default)); let response = builder.json(body).send().await?; - ok_or_error(response).await + success_or_error(response).await } /// Generic POST function supporting arbitrary responses and timeouts. @@ -518,7 +518,7 @@ impl BeaconNodeHttpClient { .json(body) .send() .await?; - ok_or_error(response).await + success_or_error(response).await } /// Generic POST function that includes octet-stream content type header. @@ -535,7 +535,7 @@ impl BeaconNodeHttpClient { HeaderValue::from_static("application/octet-stream"), ); let response = builder.headers(headers).json(body).send().await?; - ok_or_error(response).await + success_or_error(response).await } /// Generic POST function supporting arbitrary responses and timeouts. @@ -560,7 +560,7 @@ impl BeaconNodeHttpClient { HeaderValue::from_static("application/octet-stream"), ); let response = builder.headers(headers).body(body).send().await?; - ok_or_error(response).await + success_or_error(response).await } /// `GET beacon/genesis` @@ -1257,16 +1257,17 @@ impl BeaconNodeHttpClient { &self, block_contents: &PublishBlockRequest, validation_level: Option, - ) -> Result<(), Error> { - self.post_generic_with_consensus_version( - self.post_beacon_blocks_v2_path(validation_level)?, - block_contents, - Some(self.timeouts.proposal), - block_contents.signed_block().message().body().fork_name(), - ) - .await?; + ) -> Result { + let response = self + .post_generic_with_consensus_version( + self.post_beacon_blocks_v2_path(validation_level)?, + block_contents, + Some(self.timeouts.proposal), + block_contents.signed_block().message().body().fork_name(), + ) + .await?; - Ok(()) + Ok(response) } /// `POST v2/beacon/blocks` @@ -1274,16 +1275,17 @@ impl BeaconNodeHttpClient { &self, block_contents: &PublishBlockRequest, validation_level: Option, - ) -> Result<(), Error> { - self.post_generic_with_consensus_version_and_ssz_body( - self.post_beacon_blocks_v2_path(validation_level)?, - block_contents.as_ssz_bytes(), - Some(self.timeouts.proposal), - block_contents.signed_block().message().body().fork_name(), - ) - .await?; + ) -> Result { + let response = self + .post_generic_with_consensus_version_and_ssz_body( + self.post_beacon_blocks_v2_path(validation_level)?, + block_contents.as_ssz_bytes(), + Some(self.timeouts.proposal), + block_contents.signed_block().message().body().fork_name(), + ) + .await?; - Ok(()) + Ok(response) } /// `POST v2/beacon/blinded_blocks` @@ -1291,16 +1293,17 @@ impl BeaconNodeHttpClient { &self, signed_block: &SignedBlindedBeaconBlock, validation_level: Option, - ) -> Result<(), Error> { - self.post_generic_with_consensus_version( - self.post_beacon_blinded_blocks_v2_path(validation_level)?, - signed_block, - Some(self.timeouts.proposal), - signed_block.message().body().fork_name(), - ) - .await?; + ) -> Result { + let response = self + .post_generic_with_consensus_version( + self.post_beacon_blinded_blocks_v2_path(validation_level)?, + signed_block, + Some(self.timeouts.proposal), + signed_block.message().body().fork_name(), + ) + .await?; - Ok(()) + Ok(response) } /// `POST v2/beacon/blinded_blocks` @@ -1308,16 +1311,17 @@ impl BeaconNodeHttpClient { &self, signed_block: &SignedBlindedBeaconBlock, validation_level: Option, - ) -> Result<(), Error> { - self.post_generic_with_consensus_version_and_ssz_body( - self.post_beacon_blinded_blocks_v2_path(validation_level)?, - signed_block.as_ssz_bytes(), - Some(self.timeouts.proposal), - signed_block.message().body().fork_name(), - ) - .await?; + ) -> Result { + let response = self + .post_generic_with_consensus_version_and_ssz_body( + self.post_beacon_blinded_blocks_v2_path(validation_level)?, + signed_block.as_ssz_bytes(), + Some(self.timeouts.proposal), + signed_block.message().body().fork_name(), + ) + .await?; - Ok(()) + Ok(response) } /// Path for `v2/beacon/blocks` @@ -2903,3 +2907,20 @@ pub async fn ok_or_error(response: Response) -> Result { Err(Error::StatusCode(status)) } } + +/// Returns `Ok(response)` if the response is a success (2xx) response. Otherwise, creates an +/// appropriate error message. +pub async fn success_or_error(response: Response) -> Result { + let status = response.status(); + + if status.is_success() { + Ok(response) + } else if let Ok(message) = response.json().await { + match message { + ResponseError::Message(message) => Err(Error::ServerMessage(message)), + ResponseError::Indexed(indexed) => Err(Error::ServerIndexedMessage(indexed)), + } + } else { + Err(Error::StatusCode(status)) + } +} diff --git a/validator_client/validator_services/src/block_service.rs b/validator_client/validator_services/src/block_service.rs index 834df67e8a..c111b1f22e 100644 --- a/validator_client/validator_services/src/block_service.rs +++ b/validator_client/validator_services/src/block_service.rs @@ -497,6 +497,7 @@ impl BlockService { beacon_node .post_beacon_blocks_v2_ssz(signed_block, None) .await + .map(|_| ()) .or_else(|e| { handle_block_post_error(e, signed_block.signed_block().message().slot()) })? @@ -506,10 +507,12 @@ impl BlockService { &validator_metrics::BLOCK_SERVICE_TIMES, &[validator_metrics::BLINDED_BEACON_BLOCK_HTTP_POST], ); + beacon_node .post_beacon_blinded_blocks_v2_ssz(signed_block, None) .await - .or_else(|e| handle_block_post_error(e, signed_block.message().slot()))? + .map(|_| ()) + .or_else(|e| handle_block_post_error(e, signed_block.message().slot()))?; } } Ok::<_, BlockError>(())