Skip to content

Commit a374d82

Browse files
committed
fix(pbs): correct submit_block v2 content negotiation and fail loud on v1-only relays
Content negotiation (Builder API): - Missing Accept defaults the response to JSON instead of inheriting the request Content-Type; request and response encodings are independent. - v2 succeeds with an empty 202 and has no body to negotiate, so it skips Accept entirely -- a bad Accept no longer 406s a v2 submission before it reaches a relay. - An unrecognized request Content-Type returns 415 instead of 400. - Always request SSZ from the relay (JSON fallback) rather than mirroring the caller's format: PBS decodes and re-validates the payload and the route re-encodes to the BN's Accept regardless, so SSZ is the fastest wire format on the relay hop. Fail loud on v1-only relays: - Drop the v2->v1 fallback. It returned the relay's v1 payload as a 200 body, but a v2 caller (Lighthouse, Prysm) requires exactly 202 and never reads the body, so the block was silently dropped. PBS cannot publish the payload itself, so a relay that 404s v2 now fails loud (RELAY_V2_UNSUPPORTED) and other relays still get a chance.
1 parent aa6c369 commit a374d82

6 files changed

Lines changed: 149 additions & 77 deletions

File tree

crates/common/src/wire.rs

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -254,9 +254,10 @@ pub fn get_accept_types(
254254
return Err(AcceptedEncodingsError::UnsupportedAcceptType)
255255
}
256256

257-
// No accept header (or only q=0 rejections): fall back to the request
258-
// Content-Type, which mirrors the historical behavior.
259-
Ok(AcceptedEncodings::single(get_content_type(req_headers)))
257+
// No Accept header (or only q=0 rejections): per the Builder API a missing
258+
// Accept means JSON, and request/response encodings are independent — so do
259+
// NOT inherit the request Content-Type (an SSZ request still gets JSON).
260+
Ok(AcceptedEncodings::single(NO_PREFERENCE_DEFAULT))
260261
}
261262

262263
fn essence_encoding(mt: &MediaType) -> Option<EncodingType> {
@@ -468,6 +469,17 @@ mod test {
468469
assert_eq!(result, AcceptedEncodings::single(EncodingType::Json));
469470
}
470471

472+
/// A missing Accept header defaults to JSON even when the request body is
473+
/// SSZ: request and response encodings are independent, so the response
474+
/// encoding MUST NOT inherit the request Content-Type.
475+
#[test]
476+
fn test_missing_accept_header_ignores_ssz_content_type() {
477+
let mut headers = HeaderMap::new();
478+
headers.append(CONTENT_TYPE, HeaderValue::from_str(APPLICATION_OCTET_STREAM).unwrap());
479+
let result = get_accept_types(&headers).unwrap();
480+
assert_eq!(result, AcceptedEncodings::single(EncodingType::Json));
481+
}
482+
471483
/// Test accepting JSON
472484
#[test]
473485
fn test_accept_header_json() {

crates/pbs/src/error.rs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@ impl PbsClientError {
2626
PbsClientError::NoResponse => StatusCode::BAD_GATEWAY,
2727
PbsClientError::NoPayload => StatusCode::BAD_GATEWAY,
2828
PbsClientError::Internal => StatusCode::INTERNAL_SERVER_ERROR,
29+
PbsClientError::DecodeError(BodyDeserializeError::UnsupportedMediaType) => {
30+
StatusCode::UNSUPPORTED_MEDIA_TYPE
31+
}
2932
PbsClientError::DecodeError(_) => StatusCode::BAD_REQUEST,
3033
PbsClientError::HeaderError(_) => StatusCode::NOT_ACCEPTABLE,
3134
}
@@ -45,3 +48,24 @@ impl IntoResponse for PbsClientError {
4548
(self.status_code(), msg).into_response()
4649
}
4750
}
51+
52+
#[cfg(test)]
53+
mod test {
54+
use super::*;
55+
56+
#[test]
57+
fn unsupported_media_type_maps_to_415() {
58+
assert_eq!(
59+
PbsClientError::DecodeError(BodyDeserializeError::UnsupportedMediaType).status_code(),
60+
StatusCode::UNSUPPORTED_MEDIA_TYPE,
61+
);
62+
}
63+
64+
#[test]
65+
fn other_decode_errors_map_to_400() {
66+
assert_eq!(
67+
PbsClientError::DecodeError(BodyDeserializeError::MissingVersionHeader).status_code(),
68+
StatusCode::BAD_REQUEST,
69+
);
70+
}
71+
}

crates/pbs/src/metrics.rs

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -61,12 +61,13 @@ lazy_static! {
6161
PBS_METRICS_REGISTRY
6262
).unwrap();
6363

64-
/// Count of v2 submit_block requests that fell back to the v1 endpoint
65-
/// because the relay returned 404 on v2. A high value indicates the relay
66-
/// fleet has not been upgraded to support submitBlindedBlockV2.
67-
pub static ref V2_FALLBACK_TO_V1: IntCounterVec = register_int_counter_vec_with_registry!(
68-
"pbs_submit_block_v2_fallback_to_v1_total",
69-
"Count of v2 submit_block requests that fell back to v1 because the relay did not support v2",
64+
/// Count of v2 submit_block requests that could not be served because the
65+
/// relay returned 404 on the v2 endpoint. A non-zero value means the relay
66+
/// fleet has not been upgraded to support submitBlindedBlockV2 and those
67+
/// blocks were not submitted.
68+
pub static ref RELAY_V2_UNSUPPORTED: IntCounterVec = register_int_counter_vec_with_registry!(
69+
"pbs_submit_block_v2_unsupported_total",
70+
"Count of v2 submit_block requests a relay could not serve because it does not support v2",
7071
&["relay_id"],
7172
PBS_METRICS_REGISTRY
7273
).unwrap();

crates/pbs/src/mev_boost/submit_block.rs

Lines changed: 32 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,7 @@ use cb_common::{
1515
utils::utcnow_ms,
1616
wire::{
1717
AcceptedEncodings, CONSENSUS_VERSION_HEADER, EncodingType, build_outbound_accept,
18-
get_accept_types, get_user_agent_with_version, parse_response_encoding_and_fork,
19-
read_chunked_body_with_max,
18+
get_user_agent_with_version, parse_response_encoding_and_fork, read_chunked_body_with_max,
2019
},
2120
};
2221
use futures::{FutureExt, future::select_ok};
@@ -25,13 +24,13 @@ use reqwest::{
2524
header::{ACCEPT, CONTENT_TYPE, USER_AGENT},
2625
};
2726
use ssz::Encode;
28-
use tracing::{debug, error, warn};
27+
use tracing::{debug, warn};
2928
use url::Url;
3029

3130
use crate::{
3231
TIMEOUT_ERROR_CODE_STR,
3332
constants::{MAX_SIZE_SUBMIT_BLOCK_RESPONSE, SUBMIT_BLINDED_BLOCK_ENDPOINT_TAG},
34-
metrics::{RELAY_LATENCY, RELAY_STATUS_CODE, V2_FALLBACK_TO_V1},
33+
metrics::{RELAY_LATENCY, RELAY_STATUS_CODE, RELAY_V2_UNSUPPORTED},
3534
state::{BuilderApiState, PbsState},
3635
};
3736

@@ -55,8 +54,8 @@ struct SubmitBlockResponseInfo {
5554
/// ACCEPTED/OK paths where no body is returned.
5655
content_type: Option<EncodingType>,
5756

58-
/// Which fork the response bid is for (if provided as a header, rather than
59-
/// part of the body)
57+
/// Which fork the response payload is for (if provided as a header, rather
58+
/// than part of the body)
6059
fork: Option<ForkName>,
6160

6261
/// The status code of the response, for logging
@@ -82,21 +81,21 @@ pub async fn submit_block<S: BuilderApiState>(
8281
send_headers.insert(HEADER_START_TIME_UNIX_MS, HeaderValue::from(utcnow_ms()));
8382
send_headers.insert(USER_AGENT, get_user_agent_with_version(&req_headers)?);
8483

85-
// Forward the caller's Accept preference to the relay so the relay
86-
// returns data in the format the BN expects, avoiding decode→re-encode.
87-
// Always offer both encodings as fallback so a format-limited relay
88-
// still returns a bid (PBS converts if needed).
89-
let caller_accept = get_accept_types(&req_headers).inspect_err(|err| {
90-
error!(%err, "error parsing accept header");
91-
})?;
92-
let relay_accept = AcceptedEncodings {
93-
primary: caller_accept.primary,
94-
fallback: Some(match caller_accept.primary {
95-
EncodingType::Ssz => EncodingType::Json,
96-
EncodingType::Json => EncodingType::Ssz,
97-
}),
98-
};
99-
send_headers.insert(ACCEPT, build_outbound_accept(relay_accept));
84+
// PBS always decodes and re-validates the relay payload, then the route
85+
// re-encodes it to the BN's Accept. So always request SSZ from the relay
86+
// (smaller on the wire, faster to decode than JSON); JSON is the fallback for
87+
// a relay that can't do SSZ. The BN's own format preference is applied later
88+
// by the route, not here. Skip for v2, whose success is an empty 202 with no
89+
// body to negotiate.
90+
if api_version == BuilderApiVersion::V1 {
91+
send_headers.insert(
92+
ACCEPT,
93+
build_outbound_accept(AcceptedEncodings {
94+
primary: EncodingType::Ssz,
95+
fallback: Some(EncodingType::Json),
96+
}),
97+
);
98+
}
10099

101100
// Send requests to all relays concurrently
102101
let proposal_info =
@@ -130,11 +129,11 @@ async fn submit_block_with_timeout(
130129
relay: RelayClient,
131130
timeout_ms: u64,
132131
) -> Result<Option<SubmitBlindedBlockResponse>, PbsError> {
133-
let mut url = Arc::new(relay.submit_block_url(proposal_info.api_version)?);
132+
let url = Arc::new(relay.submit_block_url(proposal_info.api_version)?);
134133
let mut remaining_timeout_ms = timeout_ms;
135134
let mut retry = 0;
136135
let mut backoff = Duration::from_millis(250);
137-
let mut request_api_version = proposal_info.api_version;
136+
let request_api_version = proposal_info.api_version;
138137

139138
loop {
140139
let start_request = Instant::now();
@@ -148,23 +147,7 @@ async fn submit_block_with_timeout(
148147
)
149148
.await
150149
{
151-
Ok(response) => {
152-
// If the original request was for v2 but we had to fall back to v1, the
153-
// V1 response body (execution payload + blobs bundle) MUST be forwarded
154-
// back to the beacon node so the proposer can broadcast. Returning an
155-
// empty 202 here would cause silent block loss because the BN never
156-
// receives the unblinded payload.
157-
if request_api_version == BuilderApiVersion::V1 &&
158-
proposal_info.api_version != request_api_version
159-
{
160-
warn!(
161-
relay_id = relay.id.as_ref(),
162-
"v2 submit_block fell back to v1; forwarding v1 payload to beacon node"
163-
);
164-
V2_FALLBACK_TO_V1.with_label_values(&[relay.id.as_ref()]).inc();
165-
}
166-
return Ok(response);
167-
}
150+
Ok(response) => return Ok(response),
168151

169152
Err(err) if err.should_retry() => {
170153
tokio::time::sleep(backoff).await;
@@ -178,15 +161,18 @@ async fn submit_block_with_timeout(
178161
}
179162
}
180163

181-
Err(err)
182-
if err.is_not_found() && matches!(request_api_version, BuilderApiVersion::V2) =>
183-
{
164+
// A relay that 404s the v2 endpoint cannot serve a v2 submission. In
165+
// v2 the relay itself publishes the block after an empty 202, so a v1
166+
// payload is useless here: the beacon node expects 202 and will not
167+
// read a 200 body, so forwarding it would silently drop the block.
168+
// Fail loud instead, and let another relay (if any) serve v2.
169+
Err(err) if err.is_not_found() && request_api_version == BuilderApiVersion::V2 => {
184170
warn!(
185171
relay_id = relay.id.as_ref(),
186-
"relay does not support v2 endpoint, retrying with v1"
172+
"relay does not support the v2 submit_block endpoint; cannot serve this submission"
187173
);
188-
url = Arc::new(relay.submit_block_url(BuilderApiVersion::V1)?);
189-
request_api_version = BuilderApiVersion::V1;
174+
RELAY_V2_UNSUPPORTED.with_label_values(&[relay.id.as_ref()]).inc();
175+
return Err(err);
190176
}
191177

192178
Err(err) => return Err(err),

crates/pbs/src/routes/submit_block.rs

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ use cb_common::{
1010
pbs::{BuilderApiVersion, GetPayloadInfo},
1111
utils::{timestamp_of_slot_start_millis, utcnow_ms},
1212
wire::{
13-
AcceptedEncodingsError, CONSENSUS_VERSION_HEADER, EncodingType, deserialize_body,
14-
get_accept_types, get_user_agent,
13+
AcceptedEncodings, AcceptedEncodingsError, CONSENSUS_VERSION_HEADER, EncodingType,
14+
NO_PREFERENCE_DEFAULT, deserialize_body, get_accept_types, get_user_agent,
1515
},
1616
};
1717
use reqwest::{StatusCode, header::CONTENT_TYPE};
@@ -63,9 +63,16 @@ async fn handle_submit_block_impl<S: BuilderApiState, A: BuilderApi<S>>(
6363
let block_hash = signed_blinded_block.block_hash();
6464
let slot_start_ms = timestamp_of_slot_start_millis(slot.into(), state.config.chain);
6565
let ua = get_user_agent(&req_headers);
66-
let accept_types = get_accept_types(&req_headers).inspect_err(|err| {
67-
error!(%err, "error parsing accept header");
68-
})?;
66+
// v1 enforces Accept (a bad one is a 406). v2 succeeds with an empty 202 and
67+
// has no response body to negotiate, so skip Accept for it entirely: a bad
68+
// Accept must not 406 a v2 submission before it reaches a relay.
69+
let accept_types = if api_version == BuilderApiVersion::V1 {
70+
get_accept_types(&req_headers).inspect_err(|err| {
71+
error!(%err, "error parsing accept header");
72+
})?
73+
} else {
74+
AcceptedEncodings::single(NO_PREFERENCE_DEFAULT)
75+
};
6976
// Honor caller q-value preference: pick the highest-priority encoding that
7077
// we can actually produce. Server preference for tiebreaks is SSZ first.
7178
let response_encoding = accept_types.preferred(&[EncodingType::Ssz, EncodingType::Json]);

tests/tests/pbs_post_blinded_blocks.rs

Lines changed: 59 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use cb_common::{
66
},
77
signer::random_secret,
88
types::Chain,
9-
wire::EncodingType,
9+
wire::{CONSENSUS_VERSION_HEADER, EncodingType},
1010
};
1111
use cb_pbs::{DefaultBuilderApi, PbsService, PbsState};
1212
use cb_tests::{
@@ -18,7 +18,10 @@ use cb_tests::{
1818
};
1919
use eyre::Result;
2020
use lh_types::ForkVersionDecode;
21-
use reqwest::{Response, StatusCode};
21+
use reqwest::{
22+
Response, StatusCode,
23+
header::{ACCEPT, CONTENT_TYPE},
24+
};
2225
use tracing::info;
2326

2427
#[tokio::test]
@@ -61,31 +64,24 @@ async fn test_submit_block_v2() -> Result<()> {
6164
Ok(())
6265
}
6366

64-
// Test that when submitting a block using v2 to a relay that does not support
65-
// v2, PBS falls back to v1 and forwards the v1 response body to the beacon
66-
// node (a 200 with the execution payload), rather than swallowing the payload
67-
// and replying 202 with an empty body — which would cause silent block loss.
67+
// A v2 submission to a relay that does not support v2 must fail loud, not fake
68+
// success. v2's contract is an empty 202 after which the relay publishes the
69+
// block; a v1 payload is useless because the beacon node expects 202 and will
70+
// not read a 200 body. So PBS returns an error (no relay could serve v2) rather
71+
// than forwarding a v1 body the caller silently drops.
6872
#[tokio::test]
6973
async fn test_submit_block_v2_without_relay_support() -> Result<()> {
70-
let res = submit_block_impl(
74+
let _res = submit_block_impl(
7175
BuilderApiVersion::V2,
7276
vec![EncodingType::Json],
7377
HashSet::from([EncodingType::Ssz, EncodingType::Json]),
7478
EncodingType::Json,
75-
1,
76-
StatusCode::OK,
79+
0, // relay 404s v2 and there is no v1 fallback, so it never receives a submit
80+
StatusCode::BAD_GATEWAY,
7781
true,
7882
false,
7983
)
8084
.await?;
81-
// Payload must be forwarded so the BN can broadcast.
82-
let signed_blinded_block = load_test_signed_blinded_block();
83-
let response_body = serde_json::from_slice::<SubmitBlindedBlockResponse>(&res.bytes().await?)?;
84-
assert_eq!(
85-
response_body.data.execution_payload.block_hash(),
86-
signed_blinded_block.block_hash().into(),
87-
"v2->v1 fallback must forward the execution payload to the BN"
88-
);
8985
Ok(())
9086
}
9187

@@ -330,6 +326,52 @@ async fn test_submit_block_rejects_fork_mismatch() -> Result<()> {
330326
Ok(())
331327
}
332328

329+
// A v2 submission does no content negotiation (its success is an empty 202), so
330+
// an unsupported Accept header must NOT 406 the request before it reaches a
331+
// relay. Only v1, which returns a body, negotiates on Accept.
332+
#[tokio::test]
333+
async fn test_submit_block_v2_ignores_unsupported_accept() -> Result<()> {
334+
setup_test_env();
335+
let signer = random_secret();
336+
let pubkey = signer.public_key();
337+
let chain = Chain::Holesky;
338+
let pbs_listener = get_free_listener().await;
339+
let relay_listener = get_free_listener().await;
340+
let pbs_port = pbs_listener.local_addr().unwrap().port();
341+
let relay_port = relay_listener.local_addr().unwrap().port();
342+
343+
let mock_relay = generate_mock_relay(relay_port, pubkey)?;
344+
// Default mock supports v2.
345+
let mock_state = Arc::new(MockRelayState::new(chain, signer));
346+
tokio::spawn(start_mock_relay_service_with_listener(mock_state.clone(), relay_listener));
347+
348+
let config = to_pbs_config(chain, get_pbs_config(pbs_port), vec![mock_relay]);
349+
let state = PbsState::new(config, PathBuf::new());
350+
drop(pbs_listener);
351+
tokio::spawn(PbsService::run::<(), DefaultBuilderApi>(state));
352+
tokio::time::sleep(Duration::from_millis(100)).await;
353+
354+
let mock_validator = MockValidator::new(pbs_port)?;
355+
let url = mock_validator.comm_boost.submit_block_url(BuilderApiVersion::V2).unwrap();
356+
let body = serde_json::to_vec(&load_test_signed_blinded_block()).unwrap();
357+
let res = mock_validator
358+
.comm_boost
359+
.client
360+
.post(url)
361+
.body(body)
362+
.header(CONTENT_TYPE, EncodingType::Json.to_string())
363+
.header(CONSENSUS_VERSION_HEADER, ForkName::Electra.to_string())
364+
.header(ACCEPT, "application/garbage")
365+
.send()
366+
.await?;
367+
368+
// Reaches the relay and returns the v2 empty-202 rather than a 406.
369+
assert_ne!(res.status(), StatusCode::NOT_ACCEPTABLE);
370+
assert_eq!(res.status(), StatusCode::ACCEPTED);
371+
assert_eq!(mock_state.received_submit_block(), 1);
372+
Ok(())
373+
}
374+
333375
#[allow(clippy::too_many_arguments)]
334376
async fn submit_block_impl(
335377
api_version: BuilderApiVersion,

0 commit comments

Comments
 (0)