diff --git a/config.example.toml b/config.example.toml index 4ce750cb..1c6354e6 100644 --- a/config.example.toml +++ b/config.example.toml @@ -31,7 +31,7 @@ wait_all_registrations = true # this should be lower than that, leaving some margin for overhead # OPTIONAL, DEFAULT: 950 timeout_get_header_ms = 950 -# (unreleased, from v0.12.0-rc1) ePBS bids only: ms kept back from the beacon node's X-Timeout-Ms deadline, must be under one slot, 12000 on mainnet (https://commit-boost.github.io/commit-boost-client/get_started/epbs#timing) +# (unreleased, from v0.12.0-rc1) ePBS bids only: ms kept back from the beacon node's X-Timeout-Ms deadline, must be above 0 and under one slot, 12000 on mainnet (https://commit-boost.github.io/commit-boost-client/get_started/epbs#timing) # OPTIONAL, DEFAULT: 50 # proposer_deadline_buffer_ms = 50 # Timeout in milliseconds for the `submit_blinded_block` call to relays. diff --git a/crates/common/src/config/pbs.rs b/crates/common/src/config/pbs.rs index 22f7a1ed..294c5b53 100644 --- a/crates/common/src/config/pbs.rs +++ b/crates/common/src/config/pbs.rs @@ -250,13 +250,12 @@ impl PbsConfig { ); ensure!(self.late_in_slot_time_ms > 0, "late_in_slot_time_ms must be greater than 0"); - // The buffer comes out of the proposer's deadline, which is clamped to - // one slot, so a buffer of a slot or more leaves no time for the bid - // request. 0 is allowed (no reserve). + // 0 leaves the bid no time to get back to the beacon node, and a slot or + // more leaves the builder none, as the proposer's deadline is clamped to a slot let slot_time_ms = chain.slot_time_sec().saturating_mul(1000); ensure!( - self.proposer_deadline_buffer_ms < slot_time_ms, - "proposer_deadline_buffer_ms must be less than one slot ({slot_time_ms} ms)" + (1..slot_time_ms).contains(&self.proposer_deadline_buffer_ms), + "proposer_deadline_buffer_ms must be greater than 0 and less than one slot ({slot_time_ms} ms)" ); ensure!( @@ -557,6 +556,42 @@ mod tests { use super::*; use crate::config::test_env::{RELAY_URL, with_env}; + fn config_with_buffer(buffer: u64) -> String { + format!( + "chain = \"Holesky\"\n[pbs]\nproposer_deadline_buffer_ms = {buffer}\n\ + [[relays]]\nurl = \"{RELAY_URL}\"\n" + ) + } + + #[tokio::test] + async fn test_deadline_buffer_range() { + for (buffer, accepted) in [(0, false), (1, true), (11999, true), (12000, false)] { + let config: CommitBoostConfig = toml::from_str(&config_with_buffer(buffer)).unwrap(); + assert_eq!(config.validate().await.is_ok(), accepted, "{buffer}"); + } + } + + // The service and a custom PBS module load [pbs] through different functions + #[test] + fn test_loaders_reject_zero_deadline_buffer() { + #[derive(Deserialize)] + struct NoExtra {} + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("cb-config.toml"); + std::fs::write(&path, config_with_buffer(0)).unwrap(); + let runtime = tokio::runtime::Builder::new_current_thread().enable_all().build().unwrap(); + let service = runtime.block_on(load_pbs_config(Some(path.clone()))).map(|_| ()); + let custom = with_env(&[(CONFIG_ENV, path.to_str())], || { + runtime.block_on(load_pbs_custom_config::()).map(|_| ()) + }); + for result in [service, custom] { + let err = result.expect_err("a zero deadline buffer loaded"); + let expected = + "proposer_deadline_buffer_ms must be greater than 0 and less than one slot"; + assert!(format!("{err:#}").contains(expected), "{err:#}"); + } + } + fn relay_with_headers(headers: &str) -> Result { toml::from_str(&format!("url = \"{RELAY_URL}\"\nheaders = {headers}\n")) } diff --git a/crates/common/src/pbs/relay.rs b/crates/common/src/pbs/relay.rs index ae7f4b9a..b877d858 100644 --- a/crates/common/src/pbs/relay.rs +++ b/crates/common/src/pbs/relay.rs @@ -97,6 +97,14 @@ pub struct RelayClient { impl RelayClient { pub fn new(config: RelayConfig) -> eyre::Result { + Self::with_client_builder(config, reqwest::Client::builder()) + } + + /// [`Self::new`] on a client builder that already holds extra settings + pub fn with_client_builder( + config: RelayConfig, + builder: reqwest::ClientBuilder, + ) -> eyre::Result { let stream_url = match config.get_header { GetHeaderTransport::Http => None, GetHeaderTransport::Stream => Some(stream_url(&config.entry.url)?), @@ -131,10 +139,8 @@ impl RelayClient { } } - let client = reqwest::Client::builder() - .default_headers(headers.clone()) - .timeout(DEFAULT_REQUEST_TIMEOUT) - .build()?; + let client = + builder.default_headers(headers.clone()).timeout(DEFAULT_REQUEST_TIMEOUT).build()?; Ok(Self { id: Arc::new(config.id().to_owned()), @@ -255,6 +261,13 @@ impl RelayClient { } } +/// The builder URL in URL-form auth data. Only `http` and `https` count, since +/// opaque data such as `builder-a:prod` parses as a URL too. +pub fn decode_auth_data_url(data: &[u8]) -> Option { + let url = Url::parse(std::str::from_utf8(data).ok()?).ok()?; + matches!(url.scheme(), "http" | "https").then_some(url) +} + /// First 4 bytes of the value's SHA-256: enough to see a rotation /// across reloads, too short to identify a value on its own fn value_fingerprint(value: &str) -> String { @@ -268,12 +281,30 @@ mod tests { use alloy::primitives::B256; - use super::{GetHeaderRequest, RelayClient, RelayEntry, value_fingerprint}; + use super::{ + GetHeaderRequest, RelayClient, RelayEntry, decode_auth_data_url, value_fingerprint, + }; use crate::{ config::{GetHeaderTransport, RelayConfig, test_env::RELAY_URL}, utils::bls_pubkey_from_hex_unchecked, }; + #[test] + fn decode_auth_data_url_accepts_only_http_urls() { + let cases: [(&[u8], Option<&str>); 5] = [ + (b"https://Builder-A.example.com:443/eth", Some("https://builder-a.example.com/eth")), + (b"http://10.0.0.1:18550", Some("http://10.0.0.1:18550/")), + // Opaque data and host:port parse as URLs with their own scheme + (b"builder-a:prod", None), + (b"localhost:18550", None), + (b"builder-a.example.com", None), + ]; + for (data, expected) in cases { + let decoded = decode_auth_data_url(data).map(|url| url.to_string()); + assert_eq!(decoded.as_deref(), expected, "{}", String::from_utf8_lossy(data)); + } + } + #[test] fn test_relay_entry() { let pubkey = bls_pubkey_from_hex_unchecked( diff --git a/crates/pbs/Cargo.toml b/crates/pbs/Cargo.toml index 7497aa93..7037a566 100644 --- a/crates/pbs/Cargo.toml +++ b/crates/pbs/Cargo.toml @@ -5,6 +5,9 @@ publish = false rust-version.workspace = true version.workspace = true +[features] +testing-flags = [] + [dependencies] alloy.workspace = true async-trait.workspace = true @@ -35,3 +38,6 @@ url.workspace = true uuid.workspace = true webpki-roots.workspace = true thiserror.workspace = true + +[dev-dependencies] +tokio = { workspace = true, features = ["test-util"] } diff --git a/crates/pbs/src/dial.rs b/crates/pbs/src/dial.rs new file mode 100644 index 00000000..d5a75a00 --- /dev/null +++ b/crates/pbs/src/dial.rs @@ -0,0 +1,450 @@ +use std::{ + io, + net::{IpAddr, Ipv4Addr, SocketAddr, ToSocketAddrs}, + sync::{Arc, LazyLock, OnceLock}, + time::Duration, +}; + +use cb_common::{ + config::{GetHeaderTransport, RelayConfig}, + pbs::{RelayClient, RelayEntry}, + types::BlsPublicKey, + utils::bls_pubkey_from_hex, +}; +use reqwest::dns::{Addrs, Name, Resolve, Resolving}; +use tokio::{sync::Semaphore, task::JoinHandle}; +use tracing::{info, warn}; +use url::Url; + +use crate::error::PbsClientError; + +/// The `relay_id` of every dial: the target comes from request data, so a +/// per-host metric label would be unbounded +const DIAL_RELAY_ID: &str = "dial"; + +/// The BLS12-381 G1 generator, standing in for the pubkey `RelayEntry` requires +/// but only PBS reads. A workaround to leave `RelayClient` unchanged until PBS +/// is deprecated, when the pubkey can go. +static DIAL_PUBKEY: LazyLock = LazyLock::new(|| { + bls_pubkey_from_hex( + "0x97f1d3a73197d7942695638c4fa9ac0fc3688c4f9774b905a14e3a3f171bac586c55e83ff97a1aeffb3af00adb22c6bb", + ) + .expect("the G1 generator is a valid pubkey") +}); + +/// The address in auth data, before the first `?`. Parameters after it are +/// for the builder, which gets them inside the signed auth +pub(crate) fn auth_data_address(data: &[u8]) -> &[u8] { + data.iter().position(|&byte| byte == b'?').map_or(data, |end| &data[..end]) +} + +/// A relay for the builder an unmatched address names, checked before the dial +/// and, for a hostname, again on each new connection +pub(crate) async fn dial_relay( + data_url: Option, + address: &[u8], + lookup_timeout: Duration, +) -> Result { + let url = dial_target(data_url, address)?; + let origin = url.origin().ascii_serialization(); + let addrs = resolve_dial_target(&url, &origin, lookup_timeout).await?; + info!(%origin, ?addrs, "auth data names a builder outside the config, dialing it"); + dial_client(url) +} + +/// `data_url`, else `https:///` for the builder-specs default auth +/// data, a bare hostname +fn dial_target(data_url: Option, address: &[u8]) -> Result { + data_url + .or_else(|| { + let url = + Url::parse(&format!("https://{}/", std::str::from_utf8(address).ok()?)).ok()?; + (url.host_str()?.as_bytes() == address).then_some(url) + }) + .ok_or_else(|| { + warn!( + address = ?String::from_utf8_lossy(address), + "auth data matches no configured relay and is neither a hostname nor a URL" + ); + PbsClientError::AuthDataMismatch + }) +} + +/// A lookup that times out keeps its blocking thread until the resolver +/// returns, and relays' own lookups share that pool, so few run at once +static DIAL_LOOKUPS: Semaphore = Semaphore::const_new(32); + +async fn resolve_dial_target( + url: &Url, + origin: &str, + lookup_timeout: Duration, +) -> Result, PbsClientError> { + let host_port = format!( + "{}:{}", + url.host_str().unwrap_or_default(), + url.port_or_known_default().unwrap_or_default() + ); + // An IP literal needs no lookup + if let Ok(addr) = host_port.parse::() { + vet_dial_addrs(origin, &[addr])?; + return Ok(vec![addr]); + } + let Some(lookup) = spawn_lookup(host_port) else { + warn!(%origin, "too many dial lookups in flight, not dialing"); + return Err(PbsClientError::NoBuilderResponse); + }; + let addrs = match tokio::time::timeout(lookup_timeout, lookup).await { + Ok(Ok(Ok(addrs))) => addrs, + Ok(Ok(Err(err))) => { + warn!(%origin, %err, "dial target lookup failed, not dialing"); + return Err(PbsClientError::DialTargetBlocked); + } + Ok(Err(_)) => return Err(PbsClientError::Internal), + // Transient, unlike a failed lookup: the builder counts as not answering + Err(_) => { + warn!(%origin, ?lookup_timeout, "dial target lookup timed out, not dialing"); + return Err(PbsClientError::NoBuilderResponse); + } + }; + vet_dial_addrs(origin, &addrs)?; + Ok(addrs) +} + +/// `None` when every lookup slot is taken +fn spawn_lookup(host_port: String) -> Option>>> { + let permit = DIAL_LOOKUPS.try_acquire().ok()?; + Some(tokio::task::spawn_blocking(move || { + // Held until the lookup returns, which a timeout does not stop + let _permit = permit; + host_port.to_socket_addrs().map(Iterator::collect::>) + })) +} + +/// Every address is checked, since the dial may connect to any of them +fn vet_dial_addrs(origin: &str, addrs: &[SocketAddr]) -> Result<(), PbsClientError> { + if dial_target_check_enabled() && addrs.iter().any(|addr| ip_is_disallowed(addr.ip())) { + warn!(%origin, "dial target resolves to a disallowed address, not dialing"); + return Err(PbsClientError::DialTargetBlocked); + } + Ok(()) +} + +/// Dial hosts are chosen by the request, so an idle connection is reused for at +/// most one slot. The pool sweeps at this interval, so it closes within two. +const DIAL_IDLE_TIMEOUT: Duration = Duration::from_secs(12); + +/// Lets a builder dialed again reuse its connection. A failed build is not +/// kept, so the next dial tries again. +static DIAL_CLIENT: OnceLock = OnceLock::new(); + +/// No proxy, which would resolve the host itself, and no redirects, since one +/// to an IP address would skip the resolver +fn build_dial_client() -> eyre::Result { + RelayClient::with_client_builder( + dial_config(Url::parse("https://dial.invalid/").expect("a valid URL")), + reqwest::Client::builder() + .no_proxy() + .redirect(reqwest::redirect::Policy::none()) + .dns_resolver(DialResolver) + .pool_idle_timeout(DIAL_IDLE_TIMEOUT) + .pool_max_idle_per_host(1), + ) +} + +/// The shared dial client, addressed to `url` +fn dial_client(url: Url) -> Result { + let shared = match DIAL_CLIENT.get() { + Some(shared) => shared, + None => { + let built = build_dial_client().map_err(|err| { + warn!(%err, "failed to build the dial client"); + PbsClientError::Internal + })?; + DIAL_CLIENT.get_or_init(|| built) + } + }; + let mut relay = shared.clone(); + relay.config = Arc::new(dial_config(url)); + Ok(relay) +} + +fn dial_config(url: Url) -> RelayConfig { + RelayConfig { + entry: RelayEntry { id: DIAL_RELAY_ID.to_string(), pubkey: DIAL_PUBKEY.clone(), url }, + id: None, + headers: None, + get_params: None, + get_header: GetHeaderTransport::Http, + enable_timing_games: false, + target_first_request_ms: None, + frequency_get_header_ms: None, + validator_registration_batch_size: None, + } +} + +/// Run for each new connection to a hostname (an IP address skips it): refuses +/// it while every lookup slot is taken or if any address is disallowed +struct DialResolver; + +impl Resolve for DialResolver { + fn resolve(&self, name: Name) -> Resolving { + Box::pin(async move { + let lookup = spawn_lookup(format!("{}:0", name.as_str())) + .ok_or("too many dial lookups in flight")?; + let addrs = lookup.await??; + vet_dial_addrs(name.as_str(), &addrs)?; + Ok(Box::new(addrs.into_iter()) as Addrs) + }) + } +} + +#[cfg(feature = "testing-flags")] +thread_local! { + static SKIP_DIAL_TARGET_CHECK: std::cell::Cell = const { std::cell::Cell::new(false) }; +} + +/// Test-only: skips the dial target's address check on this thread, so a +/// test can dial a mock builder on loopback +#[cfg(feature = "testing-flags")] +pub fn set_skip_dial_target_check(skip: bool) { + SKIP_DIAL_TARGET_CHECK.with(|flag| flag.set(skip)); +} + +fn dial_target_check_enabled() -> bool { + #[cfg(feature = "testing-flags")] + { + !SKIP_DIAL_TARGET_CHECK.with(|flag| flag.get()) + } + #[cfg(not(feature = "testing-flags"))] + { + true + } +} + +/// Anything but public unicast: the special-purpose IPv4 ranges, also as the +/// IPv4 inside a v4-mapped, v4-compatible or NAT64 address, and IPv6 outside +/// 2000::/3 or in its documentation, Teredo and 6to4 ranges +fn ip_is_disallowed(ip: IpAddr) -> bool { + match ip { + IpAddr::V4(v4) => { + let [a, b, c, _] = v4.octets(); + v4.is_private() || + v4.is_loopback() || + v4.is_link_local() || + v4.is_multicast() || + v4.is_documentation() || + // std checks these only on nightly: 0.0.0.0/8, 100.64.0.0/10, + // 192.0.0.0/24, 198.18.0.0/15 and 240.0.0.0/4 + a == 0 || + (a == 100 && b & 0xc0 == 0x40) || + (a == 192 && b == 0 && c == 0) || + (a == 198 && b & 0xfe == 18) || + a >= 240 + } + IpAddr::V6(v6) => { + let s = v6.segments(); + let nat64 = s[..6] == [0x64, 0xff9b, 0, 0, 0, 0]; + let embedded = + if nat64 { Some(Ipv4Addr::from_bits(v6.to_bits() as u32)) } else { v6.to_ipv4() }; + match embedded { + // :: and ::1 land here as 0.0.0.0 and 0.0.0.1 + Some(v4) => ip_is_disallowed(IpAddr::V4(v4)), + None => { + s[0] & 0xe000 != 0x2000 || + (s[0] == 0x2001 && (s[1] == 0 || s[1] == 0x0db8)) || + s[0] == 0x2002 + } + } + } + } +} + +#[cfg(test)] +mod tests { + use std::sync::atomic::{AtomicUsize, Ordering}; + + use axum::http::StatusCode; + use cb_common::pbs::decode_auth_data_url; + + use super::*; + + // Literal IPs keep the lookup off the network + #[tokio::test] + async fn dial_relay_targets() { + const MISMATCH: &str = "auth data does not match a configured builder"; + const BLOCKED: &str = "dial target does not resolve or resolves to a disallowed address"; + let cases: [(&[u8], Result<&str, &str>); 8] = [ + (b"http://1.1.1.1:8551", Ok("http://1.1.1.1:8551/")), + (b"1.1.1.1", Ok("https://1.1.1.1/")), + (b"1.1.1.1?ofac=1", Ok("https://1.1.1.1/")), + (b"[2606:4700:4700::1111]", Ok("https://[2606:4700:4700::1111]/")), + (b"10.0.0.1", Err(BLOCKED)), + (b"1.1.1.1:8551", Err(MISMATCH)), + (b"ftp://1.1.1.1", Err(MISMATCH)), + (&[0xde, 0xad], Err(MISMATCH)), + ]; + for (data, expected) in cases { + let address = auth_data_address(data); + let res = + dial_relay(decode_auth_data_url(address), address, Duration::from_secs(5)).await; + let case = String::from_utf8_lossy(data); + match (&res, expected) { + (Ok(relay), Ok(url)) => { + assert_eq!(relay.config.entry.url.as_str(), url, "{case}"); + assert_eq!(relay.id.as_str(), "dial", "{case}"); + } + (Err(err), Err(message)) => assert_eq!(err.to_string(), message, "{case}"), + _ => panic!("{case}: expected {expected:?}, got {:?}", res.map(|_| ())), + } + } + } + + /// Taken by every test that holds or takes a lookup slot, since the slots + /// are shared by the whole test binary + static LOOKUP_SLOTS: tokio::sync::Mutex<()> = tokio::sync::Mutex::const_new(()); + + #[tokio::test] + async fn lookup_cap_refuses_hostnames_not_ip_literals() { + let _slots = LOOKUP_SLOTS.lock().await; + let _held = DIAL_LOOKUPS.try_acquire_many(32).unwrap(); + let dial = |address: &'static [u8]| dial_relay(None, address, Duration::from_secs(1)); + assert!(dial(b"1.1.1.1").await.is_ok()); + assert!(matches!(dial(b"builder.invalid").await, Err(PbsClientError::NoBuilderResponse))); + } + + #[test] + fn vet_dial_addrs_checks_every_address() { + let addr = |s: &str| s.parse::().unwrap(); + assert!(matches!( + vet_dial_addrs("https://builder.example.com", &[ + addr("1.1.1.1:443"), + addr("10.0.0.1:443") + ]), + Err(PbsClientError::DialTargetBlocked) + )); + } + + /// A builder on loopback whose `/bid` redirects to `/internal` and whose + /// `/slow` answers after 50 ms, and its count of accepted connections + async fn mock_builder() -> eyre::Result<(u16, Arc)> { + use axum::{Router, http::header::LOCATION, routing::post, serve::ListenerExt}; + + let connections = Arc::new(AtomicUsize::new(0)); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await?; + let port = listener.local_addr()?.port(); + let accepted = connections.clone(); + let listener = listener.tap_io(move |_| { + accepted.fetch_add(1, Ordering::Relaxed); + }); + let builder = Router::new() + .route( + "/bid", + post(|| async { (StatusCode::TEMPORARY_REDIRECT, [(LOCATION, "/internal")]) }), + ) + .route("/internal", post(|| async { StatusCode::OK })) + .route( + "/slow", + post(|| async { + tokio::time::sleep(Duration::from_millis(50)).await; + StatusCode::OK + }), + ); + tokio::spawn(async move { axum::serve(listener, builder).await }); + Ok((port, connections)) + } + + // An IP literal skips the resolver, which would refuse loopback + #[tokio::test] + async fn dials_share_connections_and_refuse_redirects() -> eyre::Result<()> { + let (port, connections) = mock_builder().await?; + let url = Url::parse(&format!("http://127.0.0.1:{port}/bid"))?; + for _ in 0..2 { + let res = dial_client(url.clone())?.client.post(url.clone()).send().await?; + assert_eq!(res.status(), StatusCode::TEMPORARY_REDIRECT); + } + assert_eq!(connections.load(Ordering::Relaxed), 1); + + tokio::time::pause(); + tokio::time::advance(DIAL_IDLE_TIMEOUT + Duration::from_secs(1)).await; + tokio::time::resume(); + dial_client(url.clone())?.client.post(url).send().await?; + assert_eq!(connections.load(Ordering::Relaxed), 2); + Ok(()) + } + + // Two requests at once need two connections, and only one stays idle + #[tokio::test] + async fn dial_client_keeps_one_idle_connection_per_host() -> eyre::Result<()> { + let (port, connections) = mock_builder().await?; + let url = Url::parse(&format!("http://127.0.0.1:{port}/slow"))?; + let client = dial_client(url.clone())?.client; + for _ in 0..2 { + tokio::try_join!(client.post(url.clone()).send(), client.post(url.clone()).send())?; + } + assert_eq!(connections.load(Ordering::Relaxed), 3); + Ok(()) + } + + // No check runs before these dials, so only the resolver refuses + // `localhost`, which resolves to loopback + #[cfg(feature = "testing-flags")] + #[tokio::test] + async fn dial_client_checks_the_addresses_it_resolves() -> eyre::Result<()> { + let _slots = LOOKUP_SLOTS.lock().await; + let (port, _) = mock_builder().await?; + let url = Url::parse(&format!("http://localhost:{port}/bid"))?; + let dial = || dial_client(url.clone()).map(|relay| relay.client.post(url.clone()).send()); + assert!(dial()?.await.is_err()); + set_skip_dial_target_check(true); + let held = DIAL_LOOKUPS.try_acquire_many(32)?; + assert!(dial()?.await.is_err()); + drop(held); + let res = dial()?.await; + set_skip_dial_target_check(false); + assert_eq!(res?.status(), StatusCode::TEMPORARY_REDIRECT); + Ok(()) + } + + #[test] + fn ip_is_disallowed_table() { + for (ip, disallowed) in [ + ("127.0.0.1", true), + ("0.0.0.0", true), + ("::1", true), + ("10.0.0.1", true), + ("169.254.1.1", true), + ("255.255.255.255", true), + ("100.64.0.1", true), + ("100.127.255.254", true), + ("100.63.0.1", false), + ("100.128.0.1", false), + ("fd12:3456::1", true), + ("fe80::1", true), + ("::ffff:127.0.0.1", true), + ("::10.0.0.1", true), + ("64:ff9b::a00:5", true), + ("::1.1.1.1", false), + ("64:ff9b::1.1.1.1", false), + ("::ffff:1.1.1.1", false), + ("0.1.2.3", true), + ("192.0.1.1", false), + ("198.19.255.255", true), + ("198.20.0.1", false), + ("224.0.0.1", true), + ("240.0.0.1", true), + ("192.0.0.192", true), + ("198.18.0.1", true), + ("203.0.113.1", true), + ("fec0::1", true), + ("ff02::1", true), + ("2001:db8::1", true), + ("64:ff9b:1::a00:1", true), + ("2002:a00:1::1", true), + ("2001::1", true), + ("1.1.1.1", false), + ("2606:4700:4700::1111", false), + ] { + assert_eq!(ip_is_disallowed(ip.parse().unwrap()), disallowed, "{ip}"); + } + } +} diff --git a/crates/pbs/src/error.rs b/crates/pbs/src/error.rs index 9b9aaf07..29f8dc4c 100644 --- a/crates/pbs/src/error.rs +++ b/crates/pbs/src/error.rs @@ -20,12 +20,14 @@ struct ErrorResponse { pub enum PbsClientError { #[error("no response from relays")] NoResponse, - /// 500, not 502: 502 is in neither endpoint's builder-specs response set. - /// Legacy routes keep `NoResponse` -> 502. + /// 500, not 502: 502 is not in the preferences endpoint's builder-specs + /// response set. Legacy routes keep `NoResponse` -> 502. #[error("no builder accepted the submission")] NoBuilderResponse, #[error("auth data does not match a configured builder")] AuthDataMismatch, + #[error("dial target does not resolve or resolves to a disallowed address")] + DialTargetBlocked, #[error("missing or invalid timing headers")] MissingTimingHeader, #[error("auth slot does not match the request path")] @@ -50,6 +52,7 @@ impl PbsClientError { PbsClientError::NoResponse => StatusCode::BAD_GATEWAY, PbsClientError::NoBuilderResponse => StatusCode::INTERNAL_SERVER_ERROR, PbsClientError::AuthDataMismatch => StatusCode::BAD_REQUEST, + PbsClientError::DialTargetBlocked => StatusCode::BAD_REQUEST, PbsClientError::MissingTimingHeader => StatusCode::BAD_REQUEST, PbsClientError::AuthSlotMismatch => StatusCode::BAD_REQUEST, PbsClientError::BuilderRejected(code) => *code, @@ -73,6 +76,9 @@ impl IntoResponse for PbsClientError { PbsClientError::AuthDataMismatch => { "Invalid SignedBuilderRequestAuth: auth.message.data does not match any configured builder".to_string() } + PbsClientError::DialTargetBlocked => { + "Invalid SignedBuilderRequestAuth: the addressed builder's host does not resolve or resolves to a disallowed address".to_string() + } PbsClientError::MissingTimingHeader => { "Invalid request: Date-Milliseconds and X-Timeout-Ms headers are required".to_string() } diff --git a/crates/pbs/src/lib.rs b/crates/pbs/src/lib.rs index 8b4afdcf..9c04224a 100644 --- a/crates/pbs/src/lib.rs +++ b/crates/pbs/src/lib.rs @@ -1,5 +1,6 @@ mod api; mod constants; +mod dial; mod error; mod metrics; mod mev_boost; @@ -10,6 +11,8 @@ mod utils; pub use api::*; pub use constants::*; +#[cfg(feature = "testing-flags")] +pub use dial::set_skip_dial_target_check; pub use mev_boost::*; pub use service::PbsService; pub use state::{BuilderApiState, PbsState, PbsStateGuard}; diff --git a/crates/pbs/src/routes/builder_preferences.rs b/crates/pbs/src/routes/builder_preferences.rs index dc8596e7..cba99d0e 100644 --- a/crates/pbs/src/routes/builder_preferences.rs +++ b/crates/pbs/src/routes/builder_preferences.rs @@ -65,17 +65,21 @@ pub async fn submit_builder_preferences( log_mux_selection(maybe_mux_id, relays.len(), ¶ms.proposer_pubkey); - let relay = resolve_addressed_relay(relays, request.auth.message.data.as_ref())?; + // Preferences are submitted an epoch ahead, so they share the registration + // timeout rather than the block-production one + let (relay, timeout_ms) = resolve_addressed_relay( + relays, + request.auth.message.data.as_ref(), + &req_headers, + pbs_config.timeout_register_validator_ms, + ) + .await?; let send_headers = epbs_base_send_headers(&req_headers)?; // SSZ on the relay hop let body = Bytes::from(request.as_ssz_bytes()); - // Preferences are submitted an epoch ahead, so they share the registration - // timeout rather than the block-production one - let timeout_ms = pbs_config.timeout_register_validator_ms; - let relay_id = relay.id.as_ref(); let sent = match relay.submit_builder_preferences_url(¶ms.proposer_pubkey) { Ok(url) => { diff --git a/crates/pbs/src/routes/execution_payload_bid.rs b/crates/pbs/src/routes/execution_payload_bid.rs index 9e1b9e36..affdfcf1 100644 --- a/crates/pbs/src/routes/execution_payload_bid.rs +++ b/crates/pbs/src/routes/execution_payload_bid.rs @@ -16,7 +16,7 @@ use cb_common::{ wire::{ CONSENSUS_VERSION_HEADER, EncodingType, GLOAS_CONSENSUS_VERSION, OUTBOUND_ACCEPT_SSZ_FIRST, decode_versioned_request_body, get_accept_types, get_user_agent, - parse_response_encoding_and_fork, safe_read_http_response, + parse_response_encoding_and_fork, read_chunked_body_with_max, safe_read_http_response, }, }; use reqwest::{ @@ -28,7 +28,9 @@ use tracing::{debug, error, info, warn}; use crate::{ PbsStateGuard, - constants::{GET_EXECUTION_PAYLOAD_BID_ENDPOINT_TAG, MAX_SIZE_GET_HEADER_RESPONSE}, + constants::{ + GET_EXECUTION_PAYLOAD_BID_ENDPOINT_TAG, MAX_SIZE_DEFAULT, MAX_SIZE_GET_HEADER_RESPONSE, + }, error::PbsClientError, metrics::{RELAY_HEADER_VALUE, RELAY_LAST_SLOT}, state::{BuilderApiState, PbsState}, @@ -125,8 +127,6 @@ pub async fn get_execution_payload_bid( return Err(PbsClientError::AuthSlotMismatch); } - let relay = resolve_addressed_relay(relays, auth.message.data.as_ref())?; - // The beacon node's deadline, not timeout_get_header_ms, bounds the request let slot_ms = state.config.chain.slot_time_sec().saturating_mul(1000); let budget_ms = request_budget_ms(&req_headers, utcnow_ms(), slot_ms)?; @@ -144,6 +144,20 @@ pub async fn get_execution_payload_bid( return Ok(None); } + let (relay, max_timeout_ms) = match resolve_addressed_relay( + relays, + auth.message.data.as_ref(), + &req_headers, + max_timeout_ms, + ) + .await + { + // A dial with no time left, or refused for too many lookups, is a builder + // that did not answer + Err(PbsClientError::NoBuilderResponse) => return Ok(None), + resolved => resolved?, + }; + let mut send_headers = epbs_base_send_headers(&req_headers)?; // SSZ is smaller and reaches a beacon node that asks for SSZ unchanged @@ -169,7 +183,7 @@ pub async fn get_execution_payload_bid( Ok(None) } Err(err) => { - error!(%err, %relay_id); + error!(err = ?err, %relay_id); builder_rejection(&err).map_or(Ok(None), Err) } } @@ -229,17 +243,18 @@ async fn send_get_execution_payload_bid( let (res, request_latency) = send_to_relay(request, &relay, GET_EXECUTION_PAYLOAD_BID_ENDPOINT_TAG).await?; let code = res.status(); + if !code.is_success() { + // Read after the status check, so a body over the cap still reports the + // builder's status + let url = res.url().to_string(); + let error_msg = match read_chunked_body_with_max(res, MAX_SIZE_DEFAULT, &url).await { + Ok(body) => String::from_utf8_lossy(&body).into_owned(), + Err(err) => err.to_string(), + }; + return Err(PbsError::RelayResponse { error_msg, code: code.as_u16() }); + } - // Parse the negotiated Content-Type (and optional fork) before the body is - // consumed. Only successful responses carry a meaningful encoding; on - // non-success we fall through to safe_read_http_response's NonSuccess error, - // so these values are never consumed. - let (content_type, fork) = if code.is_success() { - parse_response_encoding_and_fork(res.headers(), code.as_u16())? - } else { - (EncodingType::Json, None) - }; - + let (content_type, fork) = parse_response_encoding_and_fork(res.headers(), code.as_u16())?; let response_bytes = safe_read_http_response(res, MAX_SIZE_GET_HEADER_RESPONSE).await?; if code == StatusCode::NO_CONTENT { debug!( @@ -253,9 +268,15 @@ async fn send_get_execution_payload_bid( } let bid = match content_type { - EncodingType::Json => serde_json::from_slice(&response_bytes).map_err(|err| { - PbsError::JsonDecode { err, raw: String::from_utf8_lossy(&response_bytes).into_owned() } - })?, + EncodingType::Json => { + serde_json::from_slice(&response_bytes).map_err(|err| PbsError::JsonDecode { + err, + raw: String::from_utf8_lossy( + &response_bytes[..response_bytes.len().min(MAX_SIZE_DEFAULT)], + ) + .into_owned(), + })? + } EncodingType::Ssz => { // SSZ requires the fork from Eth-Consensus-Version; its absence is a // relay protocol violation. diff --git a/crates/pbs/src/routes/submit_signed_beacon_block.rs b/crates/pbs/src/routes/submit_signed_beacon_block.rs index 75bb3554..f8471bad 100644 --- a/crates/pbs/src/routes/submit_signed_beacon_block.rs +++ b/crates/pbs/src/routes/submit_signed_beacon_block.rs @@ -4,7 +4,7 @@ use cb_common::wire::{ require_consensus_version_header, }; use reqwest::StatusCode; -use tracing::{debug, info}; +use tracing::{debug, info, warn}; use crate::{ PbsStateGuard, @@ -35,7 +35,8 @@ pub async fn handle_submit_signed_beacon_block( /// Implements https://ethereum.github.io/builder-specs/?urls.primaryName=dev#/Builder/submitSignedBeaconBlock /// Forwards the block bytes, undecoded, to every configured builder, since CB -/// keeps no auction state to know which bid won. Returns 202 if one accepts +/// keeps no auction state to know which bid won. Returns 202 whatever they +/// answer: the beacon node gossips the block anyway pub async fn submit_signed_beacon_block( body: Bytes, req_headers: HeaderMap, @@ -86,11 +87,10 @@ pub async fn submit_signed_beacon_block( } }) .count(); - - // Only the winner accepts, so one 202 across the broadcast is success if accepted == 0 { - return Err(PbsClientError::NoBuilderResponse); + warn!(addressed = relays.len(), "no builder accepted the signed beacon block"); + } else { + info!(accepted, addressed = relays.len(), "signed beacon block accepted"); } - info!(accepted, addressed = relays.len(), "signed beacon block submitted"); Ok(()) } diff --git a/crates/pbs/src/utils.rs b/crates/pbs/src/utils.rs index 023dc6bb..1ec49bc9 100644 --- a/crates/pbs/src/utils.rs +++ b/crates/pbs/src/utils.rs @@ -6,7 +6,7 @@ use std::{ use alloy::primitives::utils::{ParseUnits, Unit}; use axum::body::Bytes; use cb_common::{ - pbs::{RelayClient, error::PbsError}, + pbs::{HEADER_VERSION_KEY, RelayClient, decode_auth_data_url, error::PbsError}, types::BlsPublicKey, wire::{ CONSENSUS_VERSION_HEADER, EncodingType, GLOAS_CONSENSUS_VERSION, @@ -23,6 +23,7 @@ use url::Url; use crate::{ constants::{MAX_SIZE_DEFAULT, TIMEOUT_ERROR_CODE_STR}, + dial::{auth_data_address, dial_relay}, error::PbsClientError, metrics::{BEACON_NODE_STATUS, RELAY_LATENCY, RELAY_STATUS_CODE}, }; @@ -186,25 +187,51 @@ pub fn check_gas_limit(gas_limit: u64, parent_gas_limit: u64) -> bool { true } -/// The relay an ePBS request addresses: the first whose URL hostname equals -/// `auth_data`. No match is a 400. -pub(crate) fn resolve_addressed_relay( +/// The first configured relay that the address in `auth_data` names, by +/// hostname or, for a URL, by origin; otherwise a relay that dials it. Also +/// returns what `timeout_ms` leaves after setting up that dial. +pub(crate) async fn resolve_addressed_relay( relays: &[RelayClient], auth_data: &[u8], -) -> Result { - let addressed = relays.iter().find(|relay| { - relay.config.entry.url.host_str().is_some_and(|host| host.as_bytes() == auth_data) + req_headers: &HeaderMap, + timeout_ms: u64, +) -> Result<(RelayClient, u64), PbsClientError> { + let address = auth_data_address(auth_data); + let by_host = relays.iter().find(|relay| { + relay.config.entry.url.host_str().is_some_and(|host| host.as_bytes() == address) }); - match addressed { - Some(relay) => Ok(relay.clone()), - None => { - warn!( - auth_data = %String::from_utf8_lossy(auth_data), - "auth data matches no configured relay" - ); - Err(PbsClientError::AuthDataMismatch) - } + if let Some(relay) = by_host { + return Ok((relay.clone(), timeout_ms)); + } + let data_url = decode_auth_data_url(address); + let by_origin = data_url.as_ref().and_then(|data_url| { + relays.iter().find(|relay| { + let url = &relay.config.entry.url; + url.scheme() == data_url.scheme() && + url.host() == data_url.host() && + url.port_or_known_default() == data_url.port_or_known_default() + }) + }); + if let Some(relay) = by_origin { + return Ok((relay.clone(), timeout_ms)); + } + // Every Commit-Boost dial carries this header, so a request dialed back into + // a Commit-Boost, this one included, goes no further + if req_headers.contains_key(HEADER_VERSION_KEY) { + warn!( + auth_data = ?String::from_utf8_lossy(auth_data), + "auth data matches no configured relay and the request came from a Commit-Boost, not dialing on" + ); + return Err(PbsClientError::AuthDataMismatch); + } + let started = Instant::now(); + let relay = dial_relay(data_url, address, Duration::from_millis(timeout_ms)).await?; + let left_ms = timeout_ms.saturating_sub(started.elapsed().as_millis() as u64); + // A request with no time left would still reach the builder + if left_ms == 0 { + return Err(PbsClientError::NoBuilderResponse); } + Ok((relay, left_ms)) } #[cfg(test)] @@ -237,24 +264,38 @@ mod tests { RelayClient::new(config).unwrap() } - #[test] - fn resolve_relay_by_hostname() { + #[tokio::test] + async fn resolve_relay_by_auth_data() { let relays = vec![ test_relay("https://0xdeadbeef@builder-a.example.com"), test_relay("https://builder-b.example.com:8443/eth"), test_relay("http://[::1]:18550"), ]; - let host = |data: &[u8]| -> Option { - resolve_addressed_relay(&relays, data) + // From a Commit-Boost, so a miss is an error, not a dial + let mut from_cb = HeaderMap::new(); + from_cb.insert(HEADER_VERSION_KEY, reqwest::header::HeaderValue::from_static("test")); + let host = async |data: &[u8]| -> Option { + resolve_addressed_relay(&relays, data, &from_cb, 0) + .await .ok() - .map(|relay| relay.config.entry.url.host_str().unwrap().to_string()) + .map(|(relay, _)| relay.config.entry.url.host_str().unwrap().to_string()) }; - assert_eq!(host(b"builder-a.example.com").as_deref(), Some("builder-a.example.com")); - assert_eq!(host(b"builder-b.example.com").as_deref(), Some("builder-b.example.com")); - assert_eq!(host(b"[::1]").as_deref(), Some("[::1]")); - assert!(host(b"Builder-A.example.com").is_none()); - assert!(host(b"builder-a.example.com:443").is_none()); - // a URL is not a hostname - assert!(host(b"https://builder-a.example.com").is_none()); + assert_eq!(host(b"builder-a.example.com").await.as_deref(), Some("builder-a.example.com")); + assert_eq!(host(b"builder-b.example.com").await.as_deref(), Some("builder-b.example.com")); + assert_eq!(host(b"[::1]").await.as_deref(), Some("[::1]")); + assert!(host(b"Builder-A.example.com").await.is_none()); + assert!(host(b"builder-a.example.com:443").await.is_none()); + // A URL matches on scheme, host and port + assert_eq!( + host(b"https://builder-a.example.com:443/eth").await.as_deref(), + Some("builder-a.example.com") + ); + assert!(host(b"http://builder-a.example.com").await.is_none()); + assert!(host(b"https://builder-b.example.com").await.is_none()); + // Parameters after `?` are for the builder and do not affect routing + assert_eq!( + host(b"builder-a.example.com?ofac=1").await.as_deref(), + Some("builder-a.example.com") + ); } } diff --git a/docs/docs/get_started/epbs.md b/docs/docs/get_started/epbs.md index 27cc0f75..1c14de6c 100644 --- a/docs/docs/get_started/epbs.md +++ b/docs/docs/get_started/epbs.md @@ -33,17 +33,17 @@ A `get_header = "stream"` relay is asked for ePBS bids over plain HTTP. From the fork, each validator key has a builder config: a list of entries, each a `url` and an `auth_data`. For every entry the beacon node sends a bid request to the entry's `url`, carrying its `auth_data` in a `SignedBuilderRequestAuth` that the validator signs. `auth_data` tells a builder that a request was meant for it, so a request signed for one builder is rejected by another. Unless the proposer and builder agree on another value, it is the builder's hostname. -To go through Commit-Boost, every entry's `url` is Commit-Boost's own URL, and its `auth_data` is the hostname of the builder it stands for. Commit-Boost reads the `auth_data` of each request, finds the relay entry with that hostname and forwards the request there. Builder preferences are routed the same way. The signed block goes to every builder, since only the one whose bid won accepts it. +To go through Commit-Boost, every entry's `url` is Commit-Boost's own URL, and its `auth_data` is the hostname of the builder it stands for. Commit-Boost reads the `auth_data` of each request, finds the relay entry with that hostname and forwards the request there. If no relay entry has it, Commit-Boost dials the builder the `auth_data` names ([builders outside your config](#builders-outside-your-config)). Builder preferences are routed the same way. The signed block goes to every builder in your config, since only the one whose bid won accepts it. ## Setup ### 1. Add the builders to Commit-Boost -List each builder as a relay entry, as for PBS: in `[[relays]]`, or in the `[[mux.relays]]` of the mux that lists the proposer's key. ePBS adds one option, `[pbs] proposer_deadline_buffer_ms` (default `50`): the time kept back from the beacon node's deadline (see [Timing](#timing)). It must be under one slot, `12000` on mainnet. +List each builder as a relay entry, as for PBS: in `[[relays]]`, or in the `[[mux.relays]]` of the mux that lists the proposer's key. ePBS adds one option, `[pbs] proposer_deadline_buffer_ms` (default `50`): the time kept back from the beacon node's deadline (see [Timing](#timing)). It must be above `0` and under one slot, `12000` on mainnet. ### 2. Point each validator key at Commit-Boost {#validator-builder-config} -Write each key's builder config through its validator client's keymanager API, which must implement the builder config endpoint ([keymanager-APIs #88](https://github.com/ethereum/keymanager-APIs/pull/88)). Add one entry per builder: `url` is Commit-Boost's URL, and `auth_data` is the hex of the builder's hostname. A key without builder config sends the default auth data of Commit-Boost's own URL, which matches no builder, so every bid request gets `400`. +Write each key's builder config through its validator client's keymanager API, which must implement the builder config endpoint ([keymanager-APIs #88](https://github.com/ethereum/keymanager-APIs/pull/88)). Add one entry per builder: `url` is Commit-Boost's URL, and `auth_data` is the hex of the builder's hostname. A key without builder config gets no bids through Commit-Boost ([builders outside your config](#builders-outside-your-config)). With the relay entry `url = "https://0xa1ce...@builder-a.example.com"`, Commit-Boost listening at `http://cb.example.com:18550`, the keymanager API at `$KEYMANAGER_URL`, its token in `$TOKEN` and the validator key in `$PUBKEY`: @@ -66,19 +66,29 @@ curl -X POST "$KEYMANAGER_URL/eth/v1/validator/$PUBKEY/builder_config" \ The call is `POST /eth/v1/validator/{pubkey}/builder_config`, authenticated with the keymanager API's bearer token. The body replaces the key's config in full, and the validator client answers `202` once it is stored. Besides `url` and `auth_data`, an entry or the top level can set: -- `min_bid` and `builder_boost_factor` are optional. At the top level they apply to every entry, and to p2p bids, unless an entry sets its own. A top-level `min_bid` also floors the bids that come through Commit-Boost, so a value above your builders' bids sends every proposal to p2p bids or a local block. -- `max_execution_payment`, when an entry omits it, gets the validator client's default, which differs between clients (Lodestar stores `0`, Nimbus the maximum). Lodestar accepts a nonzero value only when started with `--allowDangerousTrustedPayments`. +- `min_bid` and `builder_boost_factor` are optional. At the top level they apply to p2p bids and to every entry that does not set its own, so a top-level `min_bid` above your builders' bids sends every proposal to a p2p bid or a local block, unless an entry sets a lower one of its own. +- `max_execution_payment`, when an entry omits it, gets the validator client's default, which differs between clients and can be `0`, so no execution payment counts. Lodestar accepts a nonzero value only when started with `--allowDangerousTrustedPayments`. - `builder_pubkeys` limits which builder keys' bids the beacon node accepts; leave it empty to accept any. Don't copy the pubkey from the relay URL: that is the relay's key, not the builder's bid-signing key. ## Routing by auth data -Commit-Boost sends a bid or preferences request to the first builder serving the proposer's key (the relays of its `[[mux]]`, otherwise `[[relays]]`) whose URL hostname equals the request's `auth_data`, byte for byte. That is the [builder specs](https://github.com/ethereum/builder-specs/blob/main/specs/gloas/validator.md#default-auth-data) default auth data: the lowercase hostname, without scheme, port or path, with an IPv6 address in brackets, such as `[::1]`. Builders on one host share it, so only the first is asked. Commit-Boost routes by hostname only, so a value agreed with a builder works through it only if it is that hostname. When no builder matches, the request gets `400`. +Commit-Boost sends a bid or preferences request to the first builder serving the proposer's key (the relays of its `[[mux]]`, otherwise `[[relays]]`) whose URL hostname equals the request's `auth_data`, byte for byte. That is the [builder specs](https://github.com/ethereum/builder-specs/blob/main/specs/gloas/validator.md#default-auth-data) default auth data: the lowercase hostname, without scheme, port or path, with an IPv6 address in brackets, such as `[::1]`. Builders on one host share it, so only the first is asked. + +`auth_data` can also be an `http` or `https` URL, a form some builders agree out of band. It then matches the first builder whose `url` has the same scheme, host and port; the path and the pubkey in the relay URL do not count. Any other value agreed with a builder does not route through Commit-Boost. When no builder matches, Commit-Boost [dials the builder](#builders-outside-your-config) the `auth_data` names, and answers `400` when the `auth_data` is neither a hostname nor an `http(s)` URL. + +Either form may end in `?` and form-encoded parameters for the builder, such as `builder-a.example.com?ofac=1`. Commit-Boost routes on the part before `?`. The parameters reach the builder inside the signed auth, so set them only for a builder that accepts them; one that does not answers `400`. + +## Builders outside your config {#builders-outside-your-config} + +A key's builder config may name a builder you have not added as a relay entry. Commit-Boost dials that builder anyway: a hostname at `https://` on the default port, and a URL at its scheme, host and port. It answers `400` instead when the host does not resolve, or resolves to any address that is not public unicast, such as a loopback, private, link-local or `100.64.0.0/10` address, so a builder on your own network has to be a relay entry. These requests connect directly, ignoring `HTTP_PROXY`, `HTTPS_PROXY` and `ALL_PROXY`, follow no redirects and do not carry your relay `headers`. A builder reached this way gets the signed block over gossip, not from Commit-Boost. Neither `[[relays]]` nor a mux's relays limit which builders a key can reach this way, and anyone who can reach Commit-Boost's port can make it dial a public host, so keep that port private. Commit-Boost looks up at most 32 of these hosts at once and answers further requests as if the builder had not responded. + +A key without builder config sends Commit-Boost's own hostname as its `auth_data`, so it gets no bids. Commit-Boost answers `400` when that hostname resolves to a loopback or private address, as it usually does, and otherwise dials `https://` on port 443, where Commit-Boost itself does not listen. Commit-Boost never dials for a request that came from another Commit-Boost, so chained Commit-Boosts route only through relay entries. ## Timing The builder specs require each bid request to carry `Date-Milliseconds` and `X-Timeout-Ms`, which together say how long the beacon node will wait for a bid. Commit-Boost subtracts your `proposer_deadline_buffer_ms` from the time remaining and sends the result to the builder as its `X-Timeout-Ms`, so the builder knows how long it has to answer. -Think of `proposer_deadline_buffer_ms` as the slack you keep from the beacon node's remaining time: enough for the bid to get back from Commit-Boost to the beacon node and for the beacon node to process it. If no time is left, Commit-Boost does not ask the builder. +Think of `proposer_deadline_buffer_ms` as the slack you keep from the beacon node's remaining time: enough for the bid to get back from Commit-Boost to the beacon node and for the beacon node to process it. If no time is left, Commit-Boost does not ask the builder. With `0`, Commit-Boost refuses to start or reload: the builder gets the whole deadline, so a bid it sends at the deadline reaches the beacon node late. Builder preferences use `timeout_register_validator_ms`, and the signed block `timeout_get_payload_ms`. @@ -88,8 +98,8 @@ With [metrics](./running/metrics.md) enabled, the ePBS endpoints use the `endpoi | Question | Series | |---|---| -| What did Commit-Boost answer the beacon node? | `cb_pbs_beacon_node_status_code_total`: `200` or `204` for a bid request, `202` for preferences and the signed block, `4xx` for a rejected request, `500` when no builder accepted preferences or the signed block | -| What did each builder answer? | `cb_pbs_relay_status_code_total`, by `relay_id`: the builder's HTTP status, or `555` when no response arrived (timeout, DNS or connection failure) | +| What did Commit-Boost answer the beacon node? | `cb_pbs_beacon_node_status_code_total`: `200` or `204` for a bid request, `202` for preferences, `202` for the signed block whatever the builders answer, `4xx` for a rejected request, `500` when no builder accepted the preferences | +| What did each builder answer? | `cb_pbs_relay_status_code_total`, by `relay_id`: the builder's HTTP status, or `555` when no response arrived (timeout, DNS or connection failure). Requests to builders outside your config count under `relay_id="dial"`, except one Commit-Boost [refuses to dial](#builders-outside-your-config), which gets no request | | How fast are builders? | `cb_pbs_relay_latency`, by `relay_id` | | What are builders bidding? | `cb_pbs_relay_header_value` (the bid's `value` in Gwei, without the execution payment) and `cb_pbs_relay_last_slot`, by `relay_id`, from each bid a builder serves | @@ -97,7 +107,10 @@ With [metrics](./running/metrics.md) enabled, the ePBS endpoints use the `endpoi | Symptom | Cause | Fix | |---|---|---| -| Bid requests get `400` "auth.message.data does not match any configured builder" | The auth data matches no builder serving the key. Usually the key has no builder config, so its auth data is Commit-Boost's own hostname | Write the key's [builder config](#validator-builder-config), or add the builder it names as a relay entry for the key | -| A builder answers bid requests with `400` (`cb_pbs_relay_status_code_total{endpoint="get_execution_payload_bid",http_status_code="400"}`) | The builder compares auth data byte for byte and expects something other than its hostname | Have the builder accept its hostname, the only auth data Commit-Boost [routes by](#routing-by-auth-data) | +| Bid requests get `400` "auth.message.data does not match any configured builder" | The auth data is neither a hostname nor an `http(s)` URL, or the request came from another Commit-Boost and matches none of this one's relay entries | Correct the `auth_data` in the key's [builder config](#validator-builder-config), or add the builder as a relay entry on the Commit-Boost the request reaches | +| Bid requests get `400` "the addressed builder's host does not resolve or resolves to a disallowed address" | The auth data names a builder outside your config whose host does not resolve or resolves to an [internal address](#builders-outside-your-config). A key without builder config sends Commit-Boost's own hostname, which usually does | Add the builder as a relay entry for the key, or write or correct the key's [builder config](#validator-builder-config) | +| A builder answers bid requests with `400` (`cb_pbs_relay_status_code_total{endpoint="get_execution_payload_bid",http_status_code="400"}`) | The builder compares auth data byte for byte and expects something other than what the key sends, such as its hostname without `?` parameters | Send the auth data the builder expects: its hostname or its URL, the forms Commit-Boost [routes by](#routing-by-auth-data), with `?` parameters only if it accepts them | +| Commit-Boost does not start or reload, or `commit-boost init` fails, with "proposer_deadline_buffer_ms must be greater than 0 and less than one slot" | `proposer_deadline_buffer_ms` is `0`, or one slot or more | Set it above `0` and under one slot, or remove it to use the default `50` | | The signed block gets `415` | The beacon node sends it as JSON | Configure the beacon node to send SSZ | +| Commit-Boost logs "no builder accepted the signed beacon block" | The winning bid came from a builder outside your config, which gets the block over gossip, or your builders did not accept it. The beacon node gossips the block either way | If the bid came from a configured builder, check its answer in `cb_pbs_relay_status_code_total{endpoint="submit_signed_beacon_block"}` | | Commit-Boost returns `200` but the beacon node builds locally | The beacon node rejected the bid, or valued its local block higher after `builder_boost_factor` | Compare the bid with the key's `min_bid` and `builder_boost_factor` | diff --git a/docs/docs/get_started/running/metrics-catalog.md b/docs/docs/get_started/running/metrics-catalog.md index fe44cc3f..4db35ea4 100644 --- a/docs/docs/get_started/running/metrics-catalog.md +++ b/docs/docs/get_started/running/metrics-catalog.md @@ -10,19 +10,19 @@ Every metric emitted by the Commit-Boost PBS and Signer services, together with ## PBS metrics -PBS metrics use a custom Prometheus registry with namespace prefix `cb_pbs_`. The registry is created via `Registry::new_custom(Some("cb_pbs"), None)` in `crates/pbs/src/metrics.rs`. All wire names shown below include this prefix. +PBS metrics use a custom Prometheus registry with namespace prefix `cb_pbs_`. The registry is created via `Registry::new_custom(Some("cb_pbs"), None)` in `crates/pbs/src/metrics.rs`. All wire names shown below include this prefix. (unreleased, from v0.12.0-rc1) ePBS requests to a builder outside your config are labeled `relay_id="dial"`. | Metric name (wire) | Type | Labels | Description | |---|---|---|---| -| `cb_pbs_relay_status_code_total` | Counter | `http_status_code`, `endpoint`, `relay_id` | HTTP status code received by relay. Incremented once per relay response. Two synthetic codes stand in for outcomes that never reach a status line: `"555"` when the request hit its deadline, and `"556"` for a bid-stream transport failure (connect failed, the stream broke mid-window, or the handshake was answered with a success code instead of the upgrade). Endpoint values: `get_header`, `get_header_stream`, `register_validator`, `submit_blinded_block`, `status`. | -| `cb_pbs_relay_latency` | Histogram | `endpoint`, `relay_id` | HTTP latency (duration in seconds) by relay. Records the duration of relay HTTP requests. Endpoint values: `get_header`, `get_header_stream`, `register_validator`, `submit_blinded_block`, `status`. Under `get_header_stream` the observation is the time from opening the websocket to the first bid update, not a round trip, and a window that received no bid records nothing. | -| `cb_pbs_relay_last_slot` | Gauge | `relay_id` | Latest slot for which a relay delivered a header. Only updated in the `get_header` handler. Set to the current slot on each successful header from that relay. | -| `cb_pbs_relay_header_value` | Gauge | `relay_id` | Header value in gwei delivered by a relay. Converted from raw wei (÷ 1e9) in the `get_header` handler. | +| `cb_pbs_relay_status_code_total` | Counter | `http_status_code`, `endpoint`, `relay_id` | HTTP status code received by relay. Incremented once per relay response. Two synthetic codes stand in for outcomes that never reach a status line: `"555"` when no response arrived (timeout, DNS or connection failure), and `"556"` for a bid-stream transport failure (connect failed, the stream broke mid-window, or the handshake was answered with a success code instead of the upgrade). Endpoint values: `get_header`, `get_header_stream`, `register_validator`, `submit_blinded_block`, `status`, and (unreleased, from v0.12.0-rc1) `get_execution_payload_bid`, `submit_builder_preferences`, `submit_signed_beacon_block`. | +| `cb_pbs_relay_latency` | Histogram | `endpoint`, `relay_id` | HTTP latency (duration in seconds) by relay. Records the duration of relay HTTP requests. Endpoint values: `get_header`, `get_header_stream`, `register_validator`, `submit_blinded_block`, `status`, and (unreleased, from v0.12.0-rc1) `get_execution_payload_bid`, `submit_builder_preferences`, `submit_signed_beacon_block`. Under `get_header_stream` the observation is the time from opening the websocket to the first bid update, not a round trip, and a window that received no bid records nothing. | +| `cb_pbs_relay_last_slot` | Gauge | `relay_id` | Latest slot for which a relay delivered a bid. Set to the current slot on each successful `get_header` bid and (unreleased, from v0.12.0-rc1) each ePBS bid served from that relay. | +| `cb_pbs_relay_header_value` | Gauge | `relay_id` | Value in gwei of the bid a relay delivered: for `get_header`, the bid's value converted from wei (÷ 1e9). (unreleased, from v0.12.0-rc1) For ePBS, the returned bid's `value`, without its execution payment. | | `cb_pbs_relay_stream_connect_latency` | Histogram | `relay_id` | Websocket handshake latency in seconds, for a relay with `get_header = "stream"`. Observed once per successful handshake; a handshake that fails or runs out the bid window records nothing here. Custom buckets from 5 ms to 1 s. | | `cb_pbs_relay_stream_updates` | Histogram | `relay_id` | Bid updates received per stream window. Observed once per `get_header` served by the stream, including windows that received nothing. Buckets: `0, 1, 2, 3, 5, 10, 20, 50, 100`. | | `cb_pbs_relay_stream_invalid_frames_total` | Counter | `relay_id` | Websocket frames that could not be parsed as a bid. Incremented only for windows that saw at least one, so the series is absent while zero. | | `cb_pbs_relay_stream_fallback_total` | Counter | `relay_id` | Stream attempts that fell back to a plain HTTP `get_header`. Only a handshake failure with bid window left to retry counts here; one that consumed the window shows on the status series alone. The fallback's own result is recorded under `endpoint="get_header"`. | -| `cb_pbs_beacon_node_status_code_total` | Counter | `http_status_code`, `endpoint` | HTTP status code returned to the beacon node. Tracks what status codes the PBS returns for beacon node-facing requests. Endpoint values: `get_header`, `register_validator`, `submit_blinded_block`, `status`, `reload`. Error status codes (`502` for `NoResponse`/`NoPayload`, `500` for `Internal`) are set via `PbsClientError`. The handlers also record `406` directly when the request's `Accept` header offers no supported encoding, `204` when no bid is available on `get_header`, and `202` for accepted v2 `submit_blinded_block` requests. | +| `cb_pbs_beacon_node_status_code_total` | Counter | `http_status_code`, `endpoint` | HTTP status code returned to the beacon node. Tracks what status codes the PBS returns for beacon node-facing requests. Endpoint values: `get_header`, `register_validator`, `submit_blinded_block`, `status`, `reload`, and (unreleased, from v0.12.0-rc1) `get_execution_payload_bid`, `submit_builder_preferences`, `submit_signed_beacon_block`. Error status codes (`502` for `NoResponse`/`NoPayload`, `500` for `Internal`) are set via `PbsClientError`. The handlers also record `406` directly when the request's `Accept` header offers no supported encoding, `204` when no bid is available on `get_header`, and `202` for accepted v2 `submit_blinded_block` requests. (unreleased, from v0.12.0-rc1) The ePBS endpoints record `200` for a bid, `204` for none, `202` for accepted preferences and for every signed block it forwards, `400` for a request Commit-Boost refuses, a builder's own `400` or `401`, `406` for a bid request whose `Accept` offers no supported encoding, `415` for an unsupported `Content-Type`, and `500` when no builder accepts the preferences. | | `cb_pbs_pbs_submit_block_v2_unsupported_total` | Counter | `relay_id` | Count of v2 `submit_blinded_block` requests a relay could not serve because it returned 404 on the v2 endpoint. A non-zero value means the relay does not support `submitBlindedBlockV2` and those blocks were not submitted via that relay. The double `pbs` in the wire name comes from the registry prefix plus the metric name `pbs_submit_block_v2_unsupported_total`. | --- diff --git a/tests/Cargo.toml b/tests/Cargo.toml index c8503378..77206686 100644 --- a/tests/Cargo.toml +++ b/tests/Cargo.toml @@ -8,7 +8,7 @@ version.workspace = true alloy.workspace = true axum.workspace = true cb-common.workspace = true -cb-pbs.workspace = true +cb-pbs = { workspace = true, features = ["testing-flags"] } cb-signer.workspace = true eyre.workspace = true ethereum_ssz.workspace = true diff --git a/tests/src/mock_relay.rs b/tests/src/mock_relay.rs index 5826a1d8..c1ad4b81 100644 --- a/tests/src/mock_relay.rs +++ b/tests/src/mock_relay.rs @@ -129,6 +129,11 @@ pub struct MockRelayState { trustless_bid_gwei: u64, // default 10 /// When true, an SSZ bid is served without `Eth-Consensus-Version` epbs_omit_consensus_version: bool, + /// When true, a JSON bid is pretty-printed, so it differs from the compact + /// form a re-encode produces + pretty_json_bid: bool, + /// The body of the last bid served + served_bid: RwLock>, } impl MockRelayState { @@ -189,6 +194,9 @@ impl MockRelayState { pub fn received_auth_data(&self) -> Option> { self.received_auth.read().unwrap().as_ref().map(|a| a.message.data.to_vec()) } + pub fn served_bid(&self) -> Option { + self.served_bid.read().unwrap().clone() + } pub fn large_body(&self) -> bool { self.large_body } @@ -252,6 +260,8 @@ impl MockRelayState { api_keys_seen: RwLock::new(HashMap::new()), trustless_bid_gwei: 10, epbs_omit_consensus_version: false, + pretty_json_bid: false, + served_bid: RwLock::new(None), supported_content_types: Arc::new( [EncodingType::Json, EncodingType::Ssz].iter().cloned().collect(), ), @@ -329,6 +339,10 @@ impl MockRelayState { pub fn with_epbs_omit_consensus_version(self) -> Self { Self { epbs_omit_consensus_version: true, ..self } } + + pub fn with_pretty_json_bid(self) -> Self { + Self { pretty_json_bid: true, ..self } + } } pub fn mock_relay_app_router(state: Arc) -> Router { @@ -427,6 +441,10 @@ async fn handle_get_execution_payload_bid( // fail on the bid endpoint (its bid is then dropped by PBS). The request was // already counted above. if let Some(status) = *state.response_override.read().unwrap() { + // An error body over PBS's 1 KiB read cap + if state.large_body() { + return (status, "x".repeat(2048)).into_response(); + } return status.into_response(); } @@ -485,9 +503,15 @@ async fn handle_get_execution_payload_bid( data, metadata: Default::default(), }; - serde_json::to_vec(&versioned).unwrap() + if state.pretty_json_bid { + serde_json::to_vec_pretty(&versioned).unwrap() + } else { + serde_json::to_vec(&versioned).unwrap() + } } }; + let response_body = axum::body::Bytes::from(response_body); + *state.served_bid.write().unwrap() = Some(response_body.clone()); let mut response = (StatusCode::OK, response_body).into_response(); // A real builder tags the 200 with the fork so a client can decode the diff --git a/tests/src/mock_validator.rs b/tests/src/mock_validator.rs index 2059a245..c433d322 100644 --- a/tests/src/mock_validator.rs +++ b/tests/src/mock_validator.rs @@ -30,7 +30,10 @@ impl MockValidator { let pubkey = bls_pubkey_from_hex( "0xac6e77dfe25ecd6110b8e780608cce0dab71fdd5ebea22a16c0205200f2f8e2e3ad3b71d3499c54ad14d6c21b41a37ae", )?; - Ok(Self { comm_boost: generate_mock_relay(port, pubkey)? }) + let mut comm_boost = generate_mock_relay(port, pubkey)?; + // Like a beacon node, without X-CommitBoost-Version, which stops a dial + comm_boost.client = reqwest::Client::new(); + Ok(Self { comm_boost }) } pub async fn do_get_header( diff --git a/tests/src/utils.rs b/tests/src/utils.rs index d3c92416..1839b3c9 100644 --- a/tests/src/utils.rs +++ b/tests/src/utils.rs @@ -138,7 +138,7 @@ pub fn get_pbs_config(port: u16) -> PbsConfig { skip_sigverify: false, min_bid_wei: U256::ZERO, late_in_slot_time_ms: u64::MAX, - proposer_deadline_buffer_ms: 0, + proposer_deadline_buffer_ms: 50, extra_validation_enabled: false, ssv_node_api_url: Url::parse("http://localhost:0").unwrap(), diff --git a/tests/tests/pbs_cfg_file_update.rs b/tests/tests/pbs_cfg_file_update.rs index 23d59e94..c4c02ce1 100644 --- a/tests/tests/pbs_cfg_file_update.rs +++ b/tests/tests/pbs_cfg_file_update.rs @@ -66,7 +66,7 @@ async fn test_cfg_file_update() -> Result<()> { min_bid_wei: U256::ZERO, late_in_slot_time_ms: u64::MAX / 2, /* serde gets very upset about serializing u64::MAX * or anything close to it */ - proposer_deadline_buffer_ms: 0, + proposer_deadline_buffer_ms: 50, extra_validation_enabled: false, rpc_url: None, ssv_node_api_url: Url::parse("http://example.com").unwrap(), diff --git a/tests/tests/pbs_get_execution_payload_bid.rs b/tests/tests/pbs_get_execution_payload_bid.rs index 3eca13e2..1d58cf52 100644 --- a/tests/tests/pbs_get_execution_payload_bid.rs +++ b/tests/tests/pbs_get_execution_payload_bid.rs @@ -4,7 +4,7 @@ use alloy::primitives::{B256, U256}; use cb_common::{ config::RuntimeMuxConfig, pbs::{ - GetExecutionPayloadBidResponse, HEADER_START_TIME_UNIX_MS, HEADER_TIMEOUT_MS, + GetExecutionPayloadBidResponse, HEADER_START_TIME_UNIX_MS, HEADER_TIMEOUT_MS, RelayClient, SignedBuilderRequestAuth, SignedExecutionPayloadBid, }, signer::random_secret, @@ -121,9 +121,8 @@ async fn test_get_execution_payload_bid_shared_auth_data_asks_the_first() -> Res } /// The spec default auth data, the builder's hostname, routes to the relay on -/// that host, and the auth reaches the relay unchanged. A hostname no relay -/// has is a 400 with the builder's data-mismatch message, and no relay is -/// contacted. +/// that host, and the auth reaches the relay unchanged. An unconfigured +/// `localhost` resolves to loopback, so it is refused with 400 and not dialed. #[tokio::test] async fn test_get_execution_payload_bid_demux_by_hostname() -> Result<()> { let chain = Chain::Hoodi; @@ -148,11 +147,40 @@ async fn test_get_execution_payload_bid_demux_by_hostname() -> Result<()> { assert_eq!(body["code"], 400); assert_eq!( body["message"], - "Invalid SignedBuilderRequestAuth: auth.message.data does not match any configured builder" + "Invalid SignedBuilderRequestAuth: the addressed builder's host does not resolve or resolves to a disallowed address" ); Ok(()) } +/// Auth data that matches no relay entry is dialed, and the builder gets the +/// auth, `?` parameters included, unchanged. A request from a Commit-Boost is +/// not dialed, though a configured relay still serves it. +#[tokio::test] +async fn test_get_execution_payload_bid_dial() -> Result<()> { + // The mock builder listens on an address the dial check refuses + cb_pbs::set_skip_dial_target_check(true); + let chain = Chain::Hoodi; + let (mut mock_validator, cfg_state) = setup_relay(chain, |_| {}, generate_mock_relay).await?; + let (dial_state, dial_port) = + spawn_mock_relay(MockRelayState::new(chain, random_secret())).await?; + let dial_auth_data = format!("http://0.0.0.0:{dial_port}/?ofac=1").into_bytes(); + + let res = get_json_bid(&mock_validator, &opaque_auth(&dial_auth_data, TEST_SLOT)).await?; + assert_eq!(res.status(), StatusCode::OK); + assert_eq!(dial_state.received_auth_data(), Some(dial_auth_data.clone())); + + // RelayClient::new's client sends the Commit-Boost version header + mock_validator.comm_boost = + RelayClient::new(mock_validator.comm_boost.config.as_ref().clone())?; + let res = get_json_bid(&mock_validator, &opaque_auth(&dial_auth_data, TEST_SLOT)).await?; + assert_eq!(res.status(), StatusCode::BAD_REQUEST); + let res = get_json_bid(&mock_validator, &opaque_auth(TEST_AUTH_DATA, TEST_SLOT)).await?; + assert_eq!(res.status(), StatusCode::OK); + assert_eq!(dial_state.received_execution_payload_bid(), 1); + assert_eq!(cfg_state.received_execution_payload_bid(), 1); + Ok(()) +} + /// A key listed in a `[[mux]]` is served by that mux's relays: its bid request /// reaches the mux relay and not the default `[[relays]]` one. #[tokio::test] @@ -274,7 +302,8 @@ async fn test_get_execution_payload_bid_rejected_before_relays() -> Result<()> { } /// A deadline that has already passed means there is no time to serve the -/// request: CB returns 204 rather than calling a relay it cannot beat. +/// request: CB returns 204 rather than calling a relay it cannot beat, or +/// looking up a dial target (`localhost` would be refused with 400). #[tokio::test] async fn test_get_execution_payload_bid_expired_deadline_204() -> Result<()> { let (mock_validator, mock_state) = @@ -285,18 +314,20 @@ async fn test_get_execution_payload_bid_expired_deadline_204() -> Result<()> { &B256::ZERO, &random_secret().public_key(), )?; - let res = mock_validator - .comm_boost - .client - .post(url) - .header(HEADER_START_TIME_UNIX_MS, utcnow_ms() - 5_000) - .header(HEADER_TIMEOUT_MS, 1_000u64) - .header("Eth-Consensus-Version", "gloas") - .header(CONTENT_TYPE, EncodingType::Ssz.content_type_header().clone()) - .body(opaque_auth(TEST_AUTH_DATA, TEST_SLOT).as_ssz_bytes()) - .send() - .await?; - assert_eq!(res.status(), StatusCode::NO_CONTENT); + for auth_data in [TEST_AUTH_DATA, b"localhost"] { + let res = mock_validator + .comm_boost + .client + .post(url.clone()) + .header(HEADER_START_TIME_UNIX_MS, utcnow_ms() - 5_000) + .header(HEADER_TIMEOUT_MS, 1_000u64) + .header("Eth-Consensus-Version", "gloas") + .header(CONTENT_TYPE, EncodingType::Ssz.content_type_header().clone()) + .body(opaque_auth(auth_data, TEST_SLOT).as_ssz_bytes()) + .send() + .await?; + assert_eq!(res.status(), StatusCode::NO_CONTENT); + } assert_eq!(mock_state.received_execution_payload_bid(), 0, "no relay call past the deadline"); Ok(()) } @@ -408,6 +439,22 @@ async fn test_get_execution_payload_bid_response_encoding() -> Result<()> { Ok(()) } +/// A beacon node that asks for the relay's encoding gets the relay's body byte +/// for byte; a re-encode would drop the pretty-printing +#[tokio::test] +async fn test_get_execution_payload_bid_json_passthrough() -> Result<()> { + let chain = Chain::Hoodi; + let relay = MockRelayState::new(chain, random_secret()) + .with_json_only_response() + .with_pretty_json_bid(); + let (mock_validator, states) = setup_relays(chain, vec![relay]).await?; + + let res = get_json_bid(&mock_validator, &opaque_auth(TEST_AUTH_DATA, TEST_SLOT)).await?; + assert_eq!(res.status(), StatusCode::OK); + assert_eq!(Some(res.bytes().await?), states[0].served_bid()); + Ok(()) +} + /// An SSZ bid without `Eth-Consensus-Version` cannot be forwarded, since CB's /// 200 must name the fork: that relay contributes no bid, so the request is a /// 204. The relay is still contacted, so the 204 proves the drop. @@ -445,6 +492,21 @@ async fn test_get_execution_payload_bid_relay_status() -> Result<()> { Ok(()) } +/// An error body over PBS's 1 KiB read cap still reports the builder's status. +#[tokio::test] +async fn test_get_execution_payload_bid_large_error_body() -> Result<()> { + let chain = Chain::Hoodi; + let (mock_validator, states) = + setup_relays(chain, vec![MockRelayState::new(chain, random_secret()).with_large_body()]) + .await?; + for status in [StatusCode::BAD_REQUEST, StatusCode::UNAUTHORIZED] { + states[0].set_response_override(status); + let res = get_json_bid(&mock_validator, &opaque_auth(TEST_AUTH_DATA, TEST_SLOT)).await?; + assert_eq!(res.status(), status); + } + Ok(()) +} + /// A relay slower than the proposer's `X-Timeout-Ms` is dropped, so the request /// degrades to 204 rather than waiting the relay out. #[tokio::test] @@ -506,3 +568,22 @@ async fn test_get_execution_payload_bid_relay_timing_headers() -> Result<()> { assert!(0 < timeout_ms && timeout_ms <= 11_900, "relay saw X-Timeout-Ms {timeout_ms}"); Ok(()) } + +/// Auth data naming Commit-Boost's own URL is dialed once, and that hop, a +/// request from a Commit-Boost, is not dialed on: the builder's 400 +#[tokio::test] +async fn test_get_execution_payload_bid_dial_self_400() -> Result<()> { + // As if Commit-Boost's own address were public + cb_pbs::set_skip_dial_target_check(true); + let (mock_validator, cfg_state) = + setup_relay(Chain::Hoodi, |_| {}, generate_mock_relay).await?; + let own = &mock_validator.comm_boost.config.entry.url; + let own = format!("http://{}:{}/", own.host_str().unwrap(), own.port().unwrap()); + + let res = get_json_bid(&mock_validator, &opaque_auth(own.as_bytes(), TEST_SLOT)).await?; + assert_eq!(res.status(), StatusCode::BAD_REQUEST); + let body: serde_json::Value = serde_json::from_slice(&res.bytes().await?)?; + assert_eq!(body["message"], "The addressed builder rejected the request with status 400"); + assert_eq!(cfg_state.received_execution_payload_bid(), 0); + Ok(()) +} diff --git a/tests/tests/pbs_submit_builder_preferences.rs b/tests/tests/pbs_submit_builder_preferences.rs index b1079e7e..302fece0 100644 --- a/tests/tests/pbs_submit_builder_preferences.rs +++ b/tests/tests/pbs_submit_builder_preferences.rs @@ -9,7 +9,7 @@ use cb_tests::{ mock_relay::MockRelayState, utils::{ TEST_AUTH_DATA, generate_mock_relay, opaque_auth, setup_relay, setup_relays, - setup_relays_on_hosts, + setup_relays_on_hosts, spawn_mock_relay, }, }; use eyre::Result; @@ -173,10 +173,9 @@ async fn test_submit_builder_preferences_past_slot_forwarded() -> Result<()> { Ok(()) } -/// Auth data matching no relay's hostname is a 400 with the builder's -/// data-mismatch message: unmatched means CB has no builder to proxy to, and -/// proposer-private preferences must never broadcast to builders the proposer -/// did not address. +/// Auth data that is neither a hostname nor a URL is a 400 with the builder's +/// data-mismatch message: CB has no builder to proxy to, and proposer-private +/// preferences must never broadcast to builders the proposer did not address. #[tokio::test] async fn test_submit_builder_preferences_unmatched_auth_data_400() -> Result<()> { let chain = Chain::Hoodi; @@ -270,3 +269,23 @@ async fn test_submit_builder_preferences_two_relays_addressed_one_only() -> Resu assert_eq!(states[1].received_builder_preferences(), 1, "the addressed builder is asked"); Ok(()) } + +/// Preferences whose auth data names a builder outside the config are sent +/// to it by a dial and accepted with its 202 +#[tokio::test] +async fn test_submit_builder_preferences_dial() -> Result<()> { + // The mock builder listens on an address the dial check refuses + cb_pbs::set_skip_dial_target_check(true); + let chain = Chain::Hoodi; + let (mock_validator, _) = setup_relay(chain, |_| {}, generate_mock_relay).await?; + let (dial_state, dial_port) = + spawn_mock_relay(MockRelayState::new(chain, random_secret())).await?; + + let auth = opaque_auth(format!("http://0.0.0.0:{dial_port}/").as_bytes(), TEST_SLOT); + let request = preferences(auth, TEST_MAX_EXECUTION_PAYMENT); + let res = + mock_validator.do_submit_builder_preferences(None, &request, EncodingType::Ssz).await?; + assert_eq!(res.status(), StatusCode::ACCEPTED); + assert_eq!(dial_state.received_builder_preferences(), 1); + Ok(()) +} diff --git a/tests/tests/pbs_submit_signed_beacon_block.rs b/tests/tests/pbs_submit_signed_beacon_block.rs index a5e474a1..2e05c2e3 100644 --- a/tests/tests/pbs_submit_signed_beacon_block.rs +++ b/tests/tests/pbs_submit_signed_beacon_block.rs @@ -56,18 +56,16 @@ async fn test_submit_signed_beacon_block_broadcasts_to_all_relays() -> Result<() Ok(()) } -/// The block is broadcast; if every builder rejects it, PBS maps the all-reject -/// outcome to a 500 (no builder accepted). +/// Every builder rejecting the block is still a 202: the beacon node gossips +/// the block anyway, so a builder's answer is not a failure. Each is asked. #[tokio::test] -async fn test_submit_signed_beacon_block_broadcast_all_reject_500() -> Result<()> { +async fn test_submit_signed_beacon_block_broadcast_all_reject_202() -> Result<()> { let chain = Chain::Hoodi; let (mock_validator, states) = setup_relays(chain, vec![ MockRelayState::new(chain, random_secret()), MockRelayState::new(chain, random_secret()), ]) .await?; - - // Make every builder reject the broadcast for state in &states { state.set_response_override(StatusCode::INTERNAL_SERVER_ERROR); } @@ -75,36 +73,7 @@ async fn test_submit_signed_beacon_block_broadcast_all_reject_500() -> Result<() let block = gloas_block(TEST_SLOT); let res = mock_validator.do_submit_signed_beacon_block(&block, EncodingType::Ssz).await?; - assert_eq!(res.status(), StatusCode::INTERNAL_SERVER_ERROR); - assert_eq!(states[0].received_signed_beacon_block(), 1, "every builder is asked"); - assert_eq!(states[1].received_signed_beacon_block(), 1, "every builder is asked"); - Ok(()) -} - -/// A broadcast where one builder accepts and the other rejects is still a 202: -/// one acceptance across the broadcast is success. This is the core of the -/// stateless broadcast model, distinct from the all-accept and all-reject -/// extremes the other tests cover. -#[tokio::test] -async fn test_submit_signed_beacon_block_broadcast_one_accepts_202() -> Result<()> { - let chain = Chain::Hoodi; - let (mock_validator, states) = setup_relays(chain, vec![ - MockRelayState::new(chain, random_secret()), - MockRelayState::new(chain, random_secret()), - ]) - .await?; - - // Only the second builder accepts; the first rejects. PBS must still 202. - states[0].set_response_override(StatusCode::INTERNAL_SERVER_ERROR); - - let block = gloas_block(TEST_SLOT); - let res = mock_validator.do_submit_signed_beacon_block(&block, EncodingType::Ssz).await?; - - assert_eq!( - res.status(), - StatusCode::ACCEPTED, - "one accepting builder makes the broadcast a success" - ); + assert_eq!(res.status(), StatusCode::ACCEPTED); assert_eq!(states[0].received_signed_beacon_block(), 1, "every builder is asked"); assert_eq!(states[1].received_signed_beacon_block(), 1, "every builder is asked"); Ok(())