From aa4e16f8cb3b4171d1ab47f48094caf93e7cf2a9 Mon Sep 17 00:00:00 2001 From: Jason Vranek Date: Tue, 4 Aug 2026 17:24:22 -0700 Subject: [PATCH] fix(pbs): derive a proposal's fork from its slot, not the JSON decode --- config.example.toml | 2 +- crates/common/src/types.rs | 46 +++++++++++++- crates/common/src/wire.rs | 78 +++++++++++++++++++++++- crates/pbs/src/mev_boost/submit_block.rs | 31 +++++++--- 4 files changed, 144 insertions(+), 13 deletions(-) diff --git a/config.example.toml b/config.example.toml index 0777a22a..b67bf1b6 100644 --- a/config.example.toml +++ b/config.example.toml @@ -4,7 +4,7 @@ # Chain spec ID. Supported values: # A network ID. Supported values: Mainnet, Holesky, Sepolia, Hoodi. Lower case values e.g. "mainnet" are also accepted # A custom object, e.g., chain = { genesis_time_secs = 1695902400, path = "/path/to/spec.json" }, with a path to a chain spec file, either in .json format (e.g., as returned by the beacon endpoint /eth/v1/config/spec), or in .yml format (see examples in tests/data). -# A custom object, e.g., chain = { genesis_time_secs = 1695902400, slot_time_secs = 12, genesis_fork_version = "0x01017000", chain_id = 17000 }. +# A custom object, e.g., chain = { genesis_time_secs = 1695902400, slot_time_secs = 12, genesis_fork_version = "0x01017000", fulu_fork_slot = 5283840, chain_id = 17000 }. chain = "Holesky" # Configuration for the PBS module diff --git a/crates/common/src/types.rs b/crates/common/src/types.rs index 748e2595..3fbbbde8 100644 --- a/crates/common/src/types.rs +++ b/crates/common/src/types.rs @@ -181,10 +181,11 @@ impl Chain { Chain::Holesky => KnownChain::Holesky.fulu_fork_slot(), Chain::Sepolia => KnownChain::Sepolia.fulu_fork_slot(), Chain::Hoodi => KnownChain::Hoodi.fulu_fork_slot(), - Chain::Custom { slot_time_secs, .. } => *slot_time_secs, + Chain::Custom { fulu_fork_slot, .. } => *fulu_fork_slot, } } + /// TODO(gloas): this resolves to Electra or Fulu only pub fn fork_by_slot(&self, slot: u64) -> ForkName { if slot >= self.fulu_fork_slot() { ForkName::Fulu } else { ForkName::Electra } } @@ -490,6 +491,49 @@ mod tests { }) } + /// A custom chain must report its CONFIGURED fulu fork slot. + #[test] + fn custom_chain_reports_its_configured_fulu_fork_slot() { + let chain = Chain::Custom { + genesis_time_secs: 1, + slot_time_secs: 12, + genesis_fork_version: [1, 0, 0, 0], + fulu_fork_slot: 8192, + chain_id: U256::from(123), + }; + assert_eq!(chain.fulu_fork_slot(), 8192, "must not return slot_time_secs"); + } + + /// `fork_by_slot` is the authoritative fork source for a proposal, so pin + /// its boundary exactly: the fork slot itself is already Fulu. + #[test] + fn fork_by_slot_switches_at_the_fork_boundary() { + let chain = Chain::Custom { + genesis_time_secs: 1, + slot_time_secs: 12, + genesis_fork_version: [1, 0, 0, 0], + fulu_fork_slot: 100, + chain_id: U256::from(123), + }; + assert_eq!(chain.fork_by_slot(99), ForkName::Electra); + assert_eq!(chain.fork_by_slot(100), ForkName::Fulu); + assert_eq!(chain.fork_by_slot(101), ForkName::Fulu); + } + + /// A chain that is Fulu from genesis (the devnet shape) reports Fulu at + /// every slot, including slot 0. + #[test] + fn fork_by_slot_handles_fulu_from_genesis() { + let chain = Chain::Custom { + genesis_time_secs: 1, + slot_time_secs: 12, + genesis_fork_version: [1, 0, 0, 0], + fulu_fork_slot: 0, + chain_id: U256::from(123), + }; + assert_eq!(chain.fork_by_slot(0), ForkName::Fulu); + } + #[test] fn test_spec_mainnet_data_json() { let a = env!("CARGO_MANIFEST_DIR"); diff --git a/crates/common/src/wire.rs b/crates/common/src/wire.rs index 0773dddb..4f426398 100644 --- a/crates/common/src/wire.rs +++ b/crates/common/src/wire.rs @@ -6,7 +6,7 @@ use axum::http::HeaderValue; use bytes::Bytes; use futures::StreamExt; use headers_accept::Accept; -use lh_types::{BeaconBlock, ForkName}; +use lh_types::{BeaconBlock, ForkName, SignedBeaconBlock, map_fork_name}; use mediatype::{MediaType, ReadParams, names}; use reqwest::{ Response, @@ -403,8 +403,22 @@ pub fn deserialize_body( }; match encoding { - EncodingType::Json => serde_json::from_slice::(&body) - .map_err(BodyDeserializeError::SerdeJsonError), + EncodingType::Json => match get_consensus_version_header(headers) { + // `SignedBlindedBeaconBlock` is untagged and Electra and Fulu are + // field-identical, so a plain `from_slice` stops at Electra and + // reports the wrong fork for every Fulu block. + Some(version) => Ok(map_fork_name!( + version, + SignedBeaconBlock, + serde_json::from_slice(&body).map_err(BodyDeserializeError::SerdeJsonError)? + )), + // builder-specs doesn't require the header for JSON bodies. + // A request without it still has to decode and an untagged decode would silently pick + // Electra. Assume Fulu to be conservative until ePBS warrants the refactor + None => Ok(SignedBeaconBlock::Fulu( + serde_json::from_slice(&body).map_err(BodyDeserializeError::SerdeJsonError)?, + )), + }, EncodingType::Ssz => match get_consensus_version_header(headers) { Some(version) => SignedBlindedBeaconBlock::from_ssz_bytes_with(&body, |bytes| { BeaconBlock::from_ssz_bytes_for_fork(bytes, version) @@ -930,4 +944,62 @@ mod test { let err = deserialize_body(&headers, body).unwrap_err(); assert!(matches!(err, BodyDeserializeError::MissingVersionHeader)); } + + /// A blinded block body, encoded as JSON, with the given consensus version. + /// + /// The Electra fixture is deliberately reused for the Fulu case: Electra + /// and Fulu have *identical* `BeaconBlockBody` fields, and their + /// `ExecutionPayloadHeader`s are identical too, so one byte string is a + /// valid encoding of BOTH forks. That ambiguity is the whole point - the + /// header is the only thing that can tell them apart. + fn blinded_block_json(version: Option<&'static str>) -> (HeaderMap, Bytes) { + let body = Bytes::from_static(include_bytes!( + "pbs/types/testdata/signed-blinded-beacon-block-electra.json" + )); + let mut headers = HeaderMap::new(); + headers.insert(CONTENT_TYPE, HeaderValue::from_static(APPLICATION_JSON)); + if let Some(version) = version { + headers.insert( + HeaderName::try_from(CONSENSUS_VERSION_HEADER).unwrap(), + HeaderValue::from_static(version), + ); + } + (headers, body) + } + + /// A JSON body labelled `fulu` must decode AS Fulu. + #[test] + fn test_deserialize_body_json_decodes_fulu_as_fulu() { + let (headers, body) = blinded_block_json(Some("fulu")); + let block = deserialize_body(&headers, body).expect("fulu body decodes"); + assert_eq!( + block.fork_name_unchecked(), + ForkName::Fulu, + "decoded fork must follow Eth-Consensus-Version, not the untagged variant scan" + ); + } + + /// The same bytes labelled `electra` must still decode as Electra + /// The header is honoured in both directions + #[test] + fn test_deserialize_body_json_decodes_electra_as_electra() { + let (headers, body) = blinded_block_json(Some("electra")); + let block = deserialize_body(&headers, body).expect("electra body decodes"); + assert_eq!(block.fork_name_unchecked(), ForkName::Electra); + } + + /// The header is `required: false` for JSON requests in builder-specs + /// ("Required if request is SSZ encoded"), so a body without it must still + /// decode. It is assumed to be Fulu rather than left to the untagged scan, + /// which would silently pick Electra. + #[test] + fn test_deserialize_body_json_without_version_header_assumes_fulu() { + let (headers, body) = blinded_block_json(None); + let block = deserialize_body(&headers, body).expect("decodes without the header"); + assert_eq!( + block.fork_name_unchecked(), + ForkName::Fulu, + "a headerless JSON body must not fall back to the untagged Electra match" + ); + } } diff --git a/crates/pbs/src/mev_boost/submit_block.rs b/crates/pbs/src/mev_boost/submit_block.rs index 7ccc5ce7..57cb4100 100644 --- a/crates/pbs/src/mev_boost/submit_block.rs +++ b/crates/pbs/src/mev_boost/submit_block.rs @@ -44,6 +44,17 @@ struct ProposalInfo { /// The version of the submit_block route being used api_version: BuilderApiVersion, + + /// Fork of this proposal, derived from its SLOT via the chain's fork + /// schedule. + /// + /// Deliberately not `signed_blinded_block.fork_name_unchecked()`: + /// `SignedBlindedBeaconBlock` is an untagged enum, so a JSON body decodes + /// into the first variant that parses, and field-identical forks (Electra + /// and Fulu) cannot be told apart by shape. The slot can tell them apart, + /// and unlike `Eth-Consensus-Version` it does not depend on the proposer + /// sending a header that builder-specs makes optional for JSON. + fork: ForkName, } struct SubmitBlockResponseInfo { @@ -92,8 +103,9 @@ pub async fn submit_block( } // Send requests to all relays concurrently + let fork = state.config.chain.fork_by_slot(signed_blinded_block.slot().as_u64()); let proposal_info = - Arc::new(ProposalInfo { signed_blinded_block, headers: send_headers, api_version }); + Arc::new(ProposalInfo { signed_blinded_block, headers: send_headers, api_version, fork }); let mut handles = Vec::with_capacity(state.all_relays().len()); for relay in state.all_relays().iter() { handles.push( @@ -201,12 +213,9 @@ async fn send_submit_block( // Extract the info needed for validation let got_block_hash = response.data.execution_payload.block_hash().0; - // Reject if response's fork mismatches BlindedBeaconBlock's fork - let expected_fork = match &proposal_info.signed_blinded_block.message() { - BlindedBeaconBlock::Electra(_) => ForkName::Electra, - BlindedBeaconBlock::Fulu(_) => ForkName::Fulu, - _ => return Err(PbsError::Validation(ValidationError::UnsupportedFork)), - }; + // Reject if the response's fork mismatches the proposal's. Unsupported + // forks are rejected by the variant match below. + let expected_fork = proposal_info.fork; if response.version != expected_fork { return Err(PbsError::Validation(ValidationError::ForkMismatch { expected: expected_fork, @@ -265,6 +274,7 @@ async fn send_submit_block_full( timeout_ms, &proposal_info.headers, &proposal_info.signed_blinded_block, + proposal_info.fork, retry, api_version, ) @@ -320,12 +330,14 @@ fn decode_by_encoding( /// Sends the actual HTTP request to the relay's submit_block endpoint, /// returning the response (if applicable), the round-trip time, and the /// encoding type used for the body (if any). Used by send_submit_block. +#[allow(clippy::too_many_arguments)] async fn send_submit_block_impl( relay: &RelayClient, url: Arc, timeout_ms: u64, headers: &HeaderMap, signed_blinded_block: &SignedBlindedBeaconBlock, + fork: ForkName, retry: u32, api_version: BuilderApiVersion, ) -> Result { @@ -339,7 +351,7 @@ async fn send_submit_block_impl( .headers(headers.clone()) .body(signed_blinded_block.as_ssz_bytes()) .header(CONTENT_TYPE, EncodingType::Ssz.content_type_header()) - .header(CONSENSUS_VERSION_HEADER, signed_blinded_block.fork_name_unchecked().to_string()) + .header(CONSENSUS_VERSION_HEADER, fork.to_string()) .send() .await { @@ -380,6 +392,9 @@ async fn send_submit_block_impl( .headers(headers.clone()) .body(json_body) .header(CONTENT_TYPE, EncodingType::Json.content_type_header()) + // The SSZ attempt labels the fork; without it here the retry is the + // ambiguous one, since Electra and Fulu are identical in JSON. + .header(CONSENSUS_VERSION_HEADER, fork.to_string()) .send() .await {