diff --git a/crates/pbs/src/error.rs b/crates/pbs/src/error.rs index b02620eb..98f8a2f1 100644 --- a/crates/pbs/src/error.rs +++ b/crates/pbs/src/error.rs @@ -1,10 +1,20 @@ use axum::{ + Json, http::StatusCode, response::{IntoResponse, Response}, }; use cb_common::wire::{AcceptedEncodingsError, BodyDeserializeError}; +use serde::Serialize; use thiserror::Error; +/// The Builder API `ErrorMessage` error body: JSON `{code, message}`, where +/// `code` is the HTTP status. +#[derive(Debug, Serialize)] +struct ErrorResponse { + code: u16, + message: String, +} + #[derive(Debug, Error)] /// Errors that the PbsService returns to client pub enum PbsClientError { @@ -37,7 +47,8 @@ impl PbsClientError { impl IntoResponse for PbsClientError { fn into_response(self) -> Response { - let msg = match &self { + let status = self.status_code(); + let message = match &self { PbsClientError::NoResponse => "no response from relays".to_string(), PbsClientError::NoPayload => "no payload from relays".to_string(), PbsClientError::Internal => "internal server error".to_string(), @@ -45,7 +56,9 @@ impl IntoResponse for PbsClientError { PbsClientError::HeaderError(e) => format!("header error: {e}"), }; - (self.status_code(), msg).into_response() + // Return the spec's JSON `ErrorMessage` rather than plain text so clients + // can parse the error per the Builder API. + (status, Json(ErrorResponse { code: status.as_u16(), message })).into_response() } } @@ -68,4 +81,27 @@ mod test { StatusCode::BAD_REQUEST, ); } + + #[tokio::test] + async fn error_response_is_json_with_code_and_message() { + let err = PbsClientError::NoPayload; + let status = err.status_code(); + let resp = err.into_response(); + + assert_eq!(resp.status(), status); + let content_type = resp + .headers() + .get(axum::http::header::CONTENT_TYPE) + .and_then(|v| v.to_str().ok()) + .unwrap_or_default(); + assert!( + content_type.starts_with("application/json"), + "error content-type must be JSON, got: {content_type}" + ); + + let body = axum::body::to_bytes(resp.into_body(), usize::MAX).await.unwrap(); + let json: serde_json::Value = serde_json::from_slice(&body).unwrap(); + assert_eq!(json["code"], status.as_u16()); + assert_eq!(json["message"], "no payload from relays"); + } } diff --git a/tests/tests/pbs_get_header.rs b/tests/tests/pbs_get_header.rs index 8c0950ce..21ff3e6e 100644 --- a/tests/tests/pbs_get_header.rs +++ b/tests/tests/pbs_get_header.rs @@ -6,8 +6,8 @@ use cb_common::{ signature::sign_builder_root, signer::random_secret, types::{BlsPublicKeyBytes, Chain, KnownChain}, - utils::timestamp_of_slot_start_sec, - wire::{EncodingType, get_consensus_version_header}, + utils::{bls_pubkey_from_hex, timestamp_of_slot_start_sec}, + wire::{CONSENSUS_VERSION_HEADER, EncodingType, get_consensus_version_header}, }; use cb_pbs::{DefaultBuilderApi, PbsService, PbsState}; use cb_tests::{ @@ -20,7 +20,10 @@ use cb_tests::{ use eyre::Result; use lh_eth2::EmptyMetadata; use lh_types::{ForkName, ForkVersionDecode}; -use reqwest::StatusCode; +use reqwest::{ + StatusCode, + header::{ACCEPT, CONTENT_TYPE}, +}; use tracing::info; use tree_hash::TreeHash; use url::Url; @@ -68,6 +71,68 @@ async fn test_get_header_always_requests_ssz_from_relay() -> Result<()> { Ok(()) } +/// Error responses on get_header follow the Builder API `ErrorMessage` schema: +/// JSON `{code, message}` with `application/json`, not plain text. +#[tokio::test] +async fn test_get_header_error_response_is_json() -> Result<()> { + setup_test_env(); + let signer = random_secret(); + let pubkey = signer.public_key(); + let chain = Chain::Hoodi; + let pbs_listener = get_free_listener().await; + let relay_listener = get_free_listener().await; + let pbs_port = pbs_listener.local_addr().unwrap().port(); + let relay_port = relay_listener.local_addr().unwrap().port(); + + let mut mock_state = MockRelayState::new(chain, signer); + mock_state.supported_content_types = + Arc::new(HashSet::from([EncodingType::Ssz, EncodingType::Json])); + let mock_state = Arc::new(mock_state); + let mock_relay = generate_mock_relay(relay_port, pubkey)?; + tokio::spawn(start_mock_relay_service_with_listener(mock_state.clone(), relay_listener)); + + let config = to_pbs_config(chain, get_pbs_config(pbs_port), vec![mock_relay]); + let state = PbsState::new(config, PathBuf::new()); + drop(pbs_listener); + tokio::spawn(PbsService::run::<(), DefaultBuilderApi>(state)); + tokio::time::sleep(Duration::from_millis(100)).await; + + let mock_validator = MockValidator::new(pbs_port)?; + + // Happy path: a normal request succeeds and returns a bid (parses as a bid, + // which an error envelope would not). + let ok = mock_validator.do_get_header(None, vec![EncodingType::Json], ForkName::Fulu).await?; + assert_eq!(ok.status(), StatusCode::OK); + serde_json::from_slice::(&ok.bytes().await?) + .expect("happy response is a bid, not an error object"); + + // Unhappy path: an unsupported Accept must yield a spec JSON error (406). + let bn_pubkey = bls_pubkey_from_hex( + "0xac6e77dfe25ecd6110b8e780608cce0dab71fdd5ebea22a16c0205200f2f8e2e3ad3b71d3499c54ad14d6c21b41a37ae", + )?; + let slot = KnownChain::Hoodi.fulu_fork_slot() + 1; + let url = mock_validator.comm_boost.get_header_url(slot, &B256::ZERO, &bn_pubkey)?; + let err = mock_validator + .comm_boost + .client + .get(url) + .header(CONSENSUS_VERSION_HEADER, ForkName::Fulu.to_string()) + .header(ACCEPT, "application/garbage") + .send() + .await?; + + assert_eq!(err.status(), StatusCode::NOT_ACCEPTABLE); + assert_eq!( + err.headers().get(CONTENT_TYPE).and_then(|v| v.to_str().ok()), + Some("application/json"), + "error body must be JSON per the Builder API", + ); + let body: serde_json::Value = err.json().await?; + assert_eq!(body["code"], 406); + assert!(body["message"].is_string(), "error must carry a message string"); + Ok(()) +} + /// Test requesting JSON when the relay supports JSON #[tokio::test] async fn test_get_header() -> Result<()> { diff --git a/tests/tests/pbs_post_blinded_blocks.rs b/tests/tests/pbs_post_blinded_blocks.rs index 07ca5ada..06e143c2 100644 --- a/tests/tests/pbs_post_blinded_blocks.rs +++ b/tests/tests/pbs_post_blinded_blocks.rs @@ -372,6 +372,73 @@ async fn test_submit_block_v2_ignores_unsupported_accept() -> Result<()> { Ok(()) } +/// Error responses on submit_block follow the Builder API `ErrorMessage` +/// schema: JSON `{code, message}` with `application/json`, not plain text. +#[tokio::test] +async fn test_submit_block_error_response_is_json() -> Result<()> { + setup_test_env(); + let signer = random_secret(); + let pubkey = signer.public_key(); + let chain = Chain::Holesky; + let pbs_listener = get_free_listener().await; + let relay_listener = get_free_listener().await; + let pbs_port = pbs_listener.local_addr().unwrap().port(); + let relay_port = relay_listener.local_addr().unwrap().port(); + + let mut mock_relay_state = MockRelayState::new(chain, signer); + mock_relay_state.supported_content_types = + Arc::new(HashSet::from([EncodingType::Ssz, EncodingType::Json])); + let mock_state = Arc::new(mock_relay_state); + let mock_relay = generate_mock_relay(relay_port, pubkey)?; + tokio::spawn(start_mock_relay_service_with_listener(mock_state.clone(), relay_listener)); + + let config = to_pbs_config(chain, get_pbs_config(pbs_port), vec![mock_relay]); + let state = PbsState::new(config, PathBuf::new()); + drop(pbs_listener); + tokio::spawn(PbsService::run::<(), DefaultBuilderApi>(state)); + tokio::time::sleep(Duration::from_millis(100)).await; + + let mock_validator = MockValidator::new(pbs_port)?; + + // Happy path: a valid v1 submit returns 200 with the payload (parses as the + // payload, which an error envelope would not). + let ok = mock_validator + .do_submit_block_v1( + Some(load_test_signed_blinded_block()), + vec![EncodingType::Json], + EncodingType::Json, + ForkName::Electra, + ) + .await?; + assert_eq!(ok.status(), StatusCode::OK); + serde_json::from_slice::(&ok.bytes().await?) + .expect("happy response is the payload, not an error object"); + + // Unhappy path: an unrecognized request Content-Type must yield a spec JSON + // error (415). + let url = mock_validator.comm_boost.submit_block_url(BuilderApiVersion::V1).unwrap(); + let body = serde_json::to_vec(&load_test_signed_blinded_block()).unwrap(); + let err = mock_validator + .comm_boost + .client + .post(url) + .body(body) + .header(CONTENT_TYPE, "application/garbage") + .send() + .await?; + + assert_eq!(err.status(), StatusCode::UNSUPPORTED_MEDIA_TYPE); + assert_eq!( + err.headers().get(CONTENT_TYPE).and_then(|v| v.to_str().ok()), + Some("application/json"), + "error body must be JSON per the Builder API", + ); + let json: serde_json::Value = err.json().await?; + assert_eq!(json["code"], 415); + assert!(json["message"].is_string(), "error must carry a message string"); + Ok(()) +} + #[allow(clippy::too_many_arguments)] async fn submit_block_impl( api_version: BuilderApiVersion,