From c3654398aaf33d0b3546e58368f3d0bf2c29b825 Mon Sep 17 00:00:00 2001 From: Jason Vranek Date: Tue, 21 Jul 2026 17:29:37 -0700 Subject: [PATCH] fix(pbs): return Builder API JSON ErrorMessage instead of plain text PBS returned error responses to the beacon node as plain text, but the Builder API defines errors as the JSON ErrorMessage schema ({code, message}, application/json), so a client (e.g. an SSZ-first beacon node deciding whether to retry as JSON) could not parse them. Encode PbsClientError as JSON with the HTTP status as code; this covers every application-level error from all routes, including getHeader and submitBlock. Errors are always JSON per spec, never SSZ. --- crates/pbs/src/error.rs | 40 ++++++++++++++- tests/tests/pbs_get_header.rs | 71 ++++++++++++++++++++++++-- tests/tests/pbs_post_blinded_blocks.rs | 67 ++++++++++++++++++++++++ 3 files changed, 173 insertions(+), 5 deletions(-) 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,