diff --git a/crates/builder/src/validation/server.rs b/crates/builder/src/validation/server.rs index aed6ca6cb..56fa0a0d1 100644 --- a/crates/builder/src/validation/server.rs +++ b/crates/builder/src/validation/server.rs @@ -15,7 +15,7 @@ use helix_common::{ decoder::{DecoderError, SubmissionDecoder, SubmissionDecoderParams}, simulator::{SszMergedValidationRequest, SszValidationRequest}, }; -use helix_types::Submission; +use helix_types::{ForkName, Submission}; use ssz::Decode; use tokio::{net::TcpListener, sync::Semaphore, time}; use tracing::{error, info, warn}; @@ -55,6 +55,21 @@ pub async fn run(validator: BlockValidator, addr: SocketAddr, max_concurrent: us } } +/// Forks this validator understands. A submission from any other fork is +/// refused rather than validated under the wrong rules. +fn supported_fork(params: &Option) -> bool { + // No params means raw Fulu-shaped SSZ bytes. + params.as_ref().is_none_or(|params| matches!(params.fork_name, ForkName::Fulu)) +} + +/// 501, not 400: the relay maps a 400 body to `BlockValidationFailed`, which +/// demotes the builder. This is helix's own limitation, not the builder's. +fn unsupported_fork(params: &Option) -> Response { + let fork = params.as_ref().map(|params| params.fork_name); + warn!(?fork, "refusing a submission from an unsupported fork"); + (StatusCode::NOT_IMPLEMENTED, format!("unsupported fork: {fork:?}")).into_response() +} + /// A dehydrated submission needs transactions this simulator does not cache. /// The relay answers a 424 by retrying with full SSZ bytes. fn decode_submission( @@ -79,6 +94,9 @@ async fn validate(State(state): State, body: axum::body::Bytes) -> Ok(request) => request, Err(err) => return bad_request(format!("{err:?}")), }; + if !supported_fork(&request.decoder_params) { + return unsupported_fork(&request.decoder_params); + } let submission = match decode_submission(request.decoder_params, &request.signed_bid_submission) { Ok(Some(submission)) => submission, @@ -104,6 +122,9 @@ async fn validate_merged(State(state): State, body: axum::body::Byt Ok(request) => request, Err(err) => return bad_request(format!("{err:?}")), }; + if !supported_fork(&request.decoder_params) { + return unsupported_fork(&request.decoder_params); + } let submission = match decode_submission(request.decoder_params, &request.signed_bid_submission) { Ok(Some(submission)) => submission, diff --git a/crates/builder/src/validation/server_tests.rs b/crates/builder/src/validation/server_tests.rs index 8ab6aad06..65ce09b37 100644 --- a/crates/builder/src/validation/server_tests.rs +++ b/crates/builder/src/validation/server_tests.rs @@ -204,3 +204,75 @@ fn an_unchanged_list_reports_no_new_digest() { "an unchanged list must report nothing" ); } + +/// Decoder params naming a fork, with everything else at its default. +fn params_for(fork: helix_types::ForkName) -> helix_common::decoder::SubmissionDecoderParams { + helix_common::decoder::SubmissionDecoderParams { + compression: Default::default(), + encoding: helix_common::decoder::Encoding::Ssz, + merge_type: Default::default(), + is_dehydrated: false, + with_mergeable_data: false, + with_adjustments: false, + mark_all_txs_mergeable: false, + fork_name: fork, + } +} + +#[tokio::test] +async fn a_gloas_validation_request_is_refused() { + let fixture = Fixture::new().await; + let built = fixture.build_on(fixture.genesis_hash, fixture.genesis_timestamp + 12, 0); + let mut request = fixture.ssz_request(&built, true); + request.decoder_params = Some(params_for(helix_types::ForkName::Gloas)); + + let (status, body) = post(&fixture, "/validate", request.as_ssz_bytes()).await; + + assert_eq!( + status, + StatusCode::NOT_IMPLEMENTED, + "a Gloas block must not be validated under Fulu rules: {body}", + ); +} + +#[tokio::test] +async fn a_merged_gloas_request_is_refused() { + let fixture = Fixture::new().await; + let built = fixture.build_on(fixture.genesis_hash, fixture.genesis_timestamp + 12, 0); + let base = fixture.ssz_request(&built, true); + let request = SszMergedValidationRequest { + apply_blacklist: base.apply_blacklist, + registered_gas_limit: base.registered_gas_limit, + parent_beacon_block_root: base.parent_beacon_block_root, + inclusion_list: base.inclusion_list, + decoder_params: Some(params_for(helix_types::ForkName::Gloas)), + signed_bid_submission: base.signed_bid_submission, + base_payment_tx_index: 0, + }; + + let (status, body) = post(&fixture, "/validate_merged", request.as_ssz_bytes()).await; + + assert_eq!(status, StatusCode::NOT_IMPLEMENTED, "the merged route has the same hole: {body}"); +} + +#[tokio::test] +async fn a_fulu_request_is_still_validated() { + let fixture = Fixture::new().await; + let built = fixture.build_on(fixture.genesis_hash, fixture.genesis_timestamp + 12, 0); + let mut request = fixture.ssz_request(&built, true); + request.decoder_params = Some(params_for(helix_types::ForkName::Fulu)); + + let (status, body) = post(&fixture, "/validate", request.as_ssz_bytes()).await; + + assert_eq!(status, StatusCode::OK, "{body}"); +} + +#[test] +fn refusing_an_unsupported_fork_cannot_demote_a_builder() { + // `SimulatorClient::ssz_request` maps 400 to `BlockValidationFailed`, which + // demotes, and everything else to `RpcError`, which does not. Refusing a + // fork is helix's own limitation, so it must not cost a builder its + // optimistic status. + assert_ne!(StatusCode::NOT_IMPLEMENTED, StatusCode::BAD_REQUEST); + assert!(!helix_common::simulator::BlockSimError::RpcError.is_demotable()); +} diff --git a/crates/relay/src/simulator/client.rs b/crates/relay/src/simulator/client.rs index d2275175e..793f86390 100644 --- a/crates/relay/src/simulator/client.rs +++ b/crates/relay/src/simulator/client.rs @@ -54,10 +54,20 @@ impl SimulatorClient { &self.config.url } - pub fn ssz_request_builder(&self) -> Option { + /// `None` for a fork the SSZ validator cannot handle, mirroring + /// [`Self::sim_request_builder`]. The two dispatch kinds have separate fork + /// lists: an SSZ validator is a different implementation from the JSON one. + pub fn ssz_request_builder(&self, fork: ForkName) -> Option { + if !Self::ssz_supports(fork) { + return None; + } self.ssz_url.as_ref().map(|url| self.client.post(format!("{url}/validate"))) } + fn ssz_supports(fork: ForkName) -> bool { + matches!(fork, ForkName::Fulu) + } + /// Relay-internal merged-block SSZ route; see `sim_method_merged_v5`. pub fn ssz_merged_request_builder(&self) -> Option { self.ssz_url.as_ref().map(|url| self.client.post(format!("{url}/validate_merged"))) @@ -232,6 +242,28 @@ mod test { assert!(sim_client().sim_request_builder(ForkName::Gloas).is_none()); } + fn ssz_client() -> super::SimulatorClient { + super::SimulatorClient::new(reqwest::Client::new(), SimulatorConfig { + url: "http://localhost:8545".into(), + namespace: "relay".into(), + max_concurrent_tasks: 1, + ssz_url: Some("http://localhost:8552".into()), + }) + } + + #[test] + fn ssz_request_builder_routes_fulu() { + assert!(ssz_client().ssz_request_builder(ForkName::Fulu).is_some()); + } + + #[test] + fn ssz_request_builder_refuses_gloas() { + assert!( + ssz_client().ssz_request_builder(ForkName::Gloas).is_none(), + "the SSZ path short-circuited above the fork gate #517 added", + ); + } + #[tokio::test] async fn balance_request() { let sim_client = super::SimulatorClient::new(reqwest::Client::new(), SimulatorConfig { diff --git a/crates/relay/src/simulator/tile.rs b/crates/relay/src/simulator/tile.rs index 0b6d3844a..0a31db425 100644 --- a/crates/relay/src/simulator/tile.rs +++ b/crates/relay/src/simulator/tile.rs @@ -360,35 +360,39 @@ impl SimulatorTile { let submission_ref = decoded_data.submission_data.submission_ref; let sim = &mut self.simulators[id]; - let dispatch = if let Some(url) = &sim.client.ssz_url { - SimDispatch::Ssz { - to_send: sim.client.client.post(format!("{url}/validate")), + // Both dispatch kinds gate on the fork. The SSZ and JSON validators are + // different implementations, so they have separate fork lists. + let fork = submission.fork_name(); + let dispatch = match &sim.client.ssz_url { + Some(url) => sim.client.ssz_request_builder(fork).map(|to_send| SimDispatch::Ssz { + to_send, ssz_url: url.clone(), http: sim.client.client.clone(), - } - } else { - let fork = submission.fork_name(); - let Some((builder, method)) = sim.client.sim_request_builder(fork) else { - warn!(%fork, "no validation RPC method for fork, dropping submission"); - sim.pending += 1; - let result_ix = self.sim_results.push(SimResult::Validate(( - id, - Some(SimulationResultInner { - submission_ref: req.submission_ref, - optimistic_version: req.optimistic_version(), - bid: None, - result: Err(BlockSimError::UnsupportedFork(fork)), - }), - ))); - let _ = self.task_tx.try_send(SimTileInternalEvent::TaskDone { - id, - paused_until: None, - result_ix, - elapsed: None, - }); - return; - }; - SimDispatch::Json { to_send: builder, method: method.to_owned() } + }), + None => sim + .client + .sim_request_builder(fork) + .map(|(to_send, method)| SimDispatch::Json { to_send, method: method.to_owned() }), + }; + let Some(dispatch) = dispatch else { + warn!(%fork, "no validation method for fork, dropping submission"); + sim.pending += 1; + let result_ix = self.sim_results.push(SimResult::Validate(( + id, + Some(SimulationResultInner { + submission_ref: req.submission_ref, + optimistic_version: req.optimistic_version(), + bid: None, + result: Err(BlockSimError::UnsupportedFork(fork)), + }), + ))); + let _ = self.task_tx.try_send(SimTileInternalEvent::TaskDone { + id, + paused_until: None, + result_ix, + elapsed: None, + }); + return; }; sim.pending += 1;