diff --git a/Cargo.lock b/Cargo.lock index 140c33039..4c56db71a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1971,6 +1971,26 @@ dependencies = [ "uuid", ] +[[package]] +name = "cb-km" +version = "0.11.0" +dependencies = [ + "alloy", + "axum 0.8.9", + "cb-common", + "clap", + "eyre", + "reqwest 0.13.5", + "serde", + "serde_json", + "tempfile", + "tokio", + "toml", + "tracing", + "tracing-subscriber", + "url", +] + [[package]] name = "cb-metrics" version = "0.11.0" @@ -2007,11 +2027,13 @@ dependencies = [ "rustls", "serde", "serde_json", + "tempfile", "thiserror 2.0.20", "tokio", "tokio-tungstenite", "tower-http", "tracing", + "tracing-test", "tree_hash", "types", "url", @@ -2275,8 +2297,10 @@ name = "commit-boost" version = "0.11.0" dependencies = [ "assert_cmd", + "axum 0.8.9", "cb-cli", "cb-common", + "cb-km", "cb-metrics", "cb-pbs", "cb-signer", @@ -2284,6 +2308,7 @@ dependencies = [ "color-eyre", "eyre", "predicates", + "serde_json", "serde_yaml", "tempfile", "tokio", diff --git a/Cargo.toml b/Cargo.toml index b0281eae9..e46fbfddc 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,6 +32,7 @@ bytes = "1.10.1" criterion = { version = "0.5", features = ["html_reports"] } cb-cli = { path = "crates/cli" } cb-common = { path = "crates/common" } +cb-km = { path = "crates/km" } cb-metrics = { path = "crates/metrics" } cb-pbs = { path = "crates/pbs" } cb-signer = { path = "crates/signer" } diff --git a/README.md b/README.md index 4faa4a375..9ccf9a9f2 100644 --- a/README.md +++ b/README.md @@ -21,7 +21,7 @@ Commit-Boost is a modular sidecar that allows Ethereum validators to opt-in to d - Support for hard-forks and new protocol requirements ## Get started -- [Node operators](https://commit-boost.github.io/commit-boost-client/category/get-started) +- [Node operators](https://commit-boost.github.io/commit-boost-client/category/get-started). From the Gloas fork, a validator key gets no bids through Commit-Boost until its builder config points at Commit-Boost, which [`commit-boost builder-config`](https://commit-boost.github.io/commit-boost-client/get_started/epbs#builder-config-command) writes - [Developers](https://commit-boost.github.io/commit-boost-client/category/developing). Check out also the [examples](/examples) ## Audit diff --git a/benches/pbs/src/main.rs b/benches/pbs/src/main.rs index 4a0007652..c9ee367e4 100644 --- a/benches/pbs/src/main.rs +++ b/benches/pbs/src/main.rs @@ -162,6 +162,7 @@ fn get_mock_validator(bench: BenchConfig) -> RelayClient { target_first_request_ms: None, frequency_get_header_ms: None, validator_registration_batch_size: None, + max_execution_payment_gwei: None, }; RelayClient::new(config).unwrap() diff --git a/bin/Cargo.toml b/bin/Cargo.toml index e7a250913..e201efd97 100644 --- a/bin/Cargo.toml +++ b/bin/Cargo.toml @@ -7,6 +7,7 @@ version.workspace = true [dependencies] cb-cli.workspace = true cb-common.workspace = true +cb-km.workspace = true cb-metrics.workspace = true cb-pbs.workspace = true cb-signer.workspace = true @@ -20,7 +21,9 @@ tree_hash_derive.workspace = true [dev-dependencies] assert_cmd.workspace = true +axum.workspace = true predicates.workspace = true +serde_json.workspace = true serde_yaml.workspace = true tempfile.workspace = true diff --git a/bin/commit-boost.rs b/bin/commit-boost.rs index e424d1444..036494045 100644 --- a/bin/commit-boost.rs +++ b/bin/commit-boost.rs @@ -1,4 +1,4 @@ -use std::path::PathBuf; +use std::{path::PathBuf, process::ExitCode}; use cb_cli::docker_init::handle_docker_init; use cb_common::{ @@ -42,12 +42,14 @@ enum Commands { #[arg(short, long("output"), default_value = "./")] output_path: PathBuf, }, + + /// Print or write each validator key's ePBS builder config, for its + /// validator client's keymanager API + BuilderConfig(cb_km::cli::BuilderConfigArgs), } #[tokio::main] -async fn main() -> Result<()> { - // Parse the CLI arguments (currently only used for version info, more can be - // added later) +async fn main() -> Result { let cli = Cli::parse(); color_eyre::install()?; @@ -56,9 +58,10 @@ async fn main() -> Result<()> { Commands::Pbs => run_pbs_service().await?, Commands::Signer => run_signer_service().await?, Commands::Init { config_path, output_path } => run_init(config_path, output_path).await?, + Commands::BuilderConfig(args) => return Ok(cb_km::cli::run(args).await), } - Ok(()) + Ok(ExitCode::SUCCESS) } /// Run the PBS service diff --git a/bin/tests/builder_config.rs b/bin/tests/builder_config.rs new file mode 100644 index 000000000..34eb023ab --- /dev/null +++ b/bin/tests/builder_config.rs @@ -0,0 +1,1393 @@ +//! `commit-boost builder-config` against a mock keymanager API (axum). + +use std::{ + collections::HashMap, + sync::{Arc, Mutex}, +}; + +use axum::{ + Router, + extract::{Path, State}, + http::{HeaderMap, StatusCode}, + response::IntoResponse, + routing::get, +}; +use cb_common::config::CommitBoostConfig; +use cb_km::{ + apply::{ApplyOptions, ApplyReport, run_apply}, + project::{Projection, mux_keys, parse_config, project}, + targets::Targets, +}; + +const RELAY_PK_A: &str = "0xa1cec75a3f0661e99299274182938151e8433c61a19222347ea1313d839229cb4ce4e3e5aa2bdeb71c8fcf1b084963c2"; +const RELAY_PK_B: &str = "0xa119589bb33ef52acbb8116832bec2b58fca590fe5c85eac5d3230b44d5bc09fe73ccd21f88eab31d6de16194d17782e"; +const TOKEN: &str = "test-token"; + +#[derive(Clone, Default)] +struct MockVc { + /// keys this VC holds (lowercase hex) + keystores: Vec, + /// keys this VC holds through a remote signer; None: no remotekeys route + remotekeys: Option>, + /// whether the VC serves the #88 builder_config route + supports_builder_config: bool, + /// stored docs returned on GET + stored: HashMap, + /// POST status override per key, answered with an ErrorResponse body + /// (default: 202 when held, else 404) + post_status: HashMap, + /// GET builder_config status override for every key + get_status: Option, + /// GET builder_config status override per key + get_status_for: HashMap, + /// remotekeys status override + remotekeys_status: Option, + /// answers 202 to a write for a key it does not hold, as some clients do + accepts_any_key: bool, + /// refuses a write with a cap above 0, as Lodestar does without its flag + lodestar_cap_refusal: bool, + /// keys whose builder_config GET or POST it drops the connection on, + /// leaving the request unanswered + hangs_up_on: Vec, + /// hangs up on the writes in `hangs_up_on` only, answering their reads + answers_reads: bool, + /// (pubkey, raw body) of every builder_config POST + posts: Arc>>, +} + +impl MockVc { + fn holding(keys: &[String]) -> Self { + Self { keystores: keys.to_vec(), supports_builder_config: true, ..Default::default() } + } + + fn holds(&self, key: &String) -> bool { + self.keystores.contains(key) || self.remotekeys.iter().flatten().any(|k| k == key) + } + + fn posts(&self) -> Vec<(String, String)> { + self.posts.lock().unwrap().clone() + } +} + +/// A key as a client may list it: the spec allows uppercase hex +fn listed(pk: &str) -> String { + format!("0x{}", pk[2..].to_uppercase()) +} + +fn authed(headers: &HeaderMap) -> bool { + headers + .get("authorization") + .and_then(|v| v.to_str().ok()) + .is_some_and(|v| v == format!("Bearer {TOKEN}")) +} + +async fn keystores(State(vc): State, headers: HeaderMap) -> impl IntoResponse { + if !authed(&headers) { + return (StatusCode::UNAUTHORIZED, "unauthorized").into_response(); + } + let data: Vec<_> = vc + .keystores + .iter() + .map(|pk| serde_json::json!({ "validating_pubkey": listed(pk) })) + .collect(); + axum::Json(serde_json::json!({ "data": data })).into_response() +} + +async fn remotekeys(State(vc): State, headers: HeaderMap) -> impl IntoResponse { + if !authed(&headers) { + return (StatusCode::UNAUTHORIZED, "unauthorized").into_response(); + } + if let Some(status) = vc.remotekeys_status { + return (StatusCode::from_u16(status).unwrap(), "overridden").into_response(); + } + let Some(keys) = &vc.remotekeys else { + return (StatusCode::NOT_FOUND, "not found").into_response(); + }; + let data: Vec<_> = keys + .iter() + .map(|pk| serde_json::json!({ "pubkey": listed(pk), "url": "https://signer.example.com", "readonly": false })) + .collect(); + axum::Json(serde_json::json!({ "data": data })).into_response() +} + +async fn get_config( + State(vc): State, + Path(pubkey): Path, + headers: HeaderMap, +) -> impl IntoResponse { + if !authed(&headers) { + return (StatusCode::UNAUTHORIZED, "unauthorized").into_response(); + } + // A panic ends the connection's task, closing it without a response + if !vc.answers_reads && vc.hangs_up_on.contains(&pubkey) { + panic!("hanging up on the read"); + } + if let Some(status) = vc.get_status.or(vc.get_status_for.get(&pubkey).copied()) { + return (StatusCode::from_u16(status).unwrap(), "overridden").into_response(); + } + if !vc.supports_builder_config || !vc.holds(&pubkey) { + return (StatusCode::NOT_FOUND, "not found").into_response(); + } + let doc = vc.stored.get(&pubkey).cloned().unwrap_or(serde_json::json!({})); + axum::Json(serde_json::json!({ "data": doc })).into_response() +} + +async fn post_config( + State(vc): State, + Path(pubkey): Path, + headers: HeaderMap, + body: String, +) -> impl IntoResponse { + if !authed(&headers) { + return (StatusCode::UNAUTHORIZED, "unauthorized").into_response(); + } + let doc: serde_json::Value = serde_json::from_str(&body).unwrap_or_default(); + let capped = doc["builders"] + .as_array() + .into_iter() + .flatten() + .any(|entry| entry["max_execution_payment"] != "0"); + vc.posts.lock().unwrap().push((pubkey.clone(), body)); + if vc.hangs_up_on.contains(&pubkey) { + panic!("hanging up on the write"); + } + if !vc.supports_builder_config { + return (StatusCode::NOT_FOUND, "not found").into_response(); + } + if vc.lodestar_cap_refusal && capped { + let message = "Configuring a builder max execution payment above 0 requires \ + --allowDangerousTrustedPayments"; + let body = serde_json::json!({ "code": 400, "message": message }); + return (StatusCode::BAD_REQUEST, axum::Json(body)).into_response(); + } + if let Some(&status) = vc.post_status.get(&pubkey) { + let message = serde_json::json!({ "message": format!("refused with {status}") }); + return (StatusCode::from_u16(status).unwrap(), axum::Json(message)).into_response(); + } + if vc.holds(&pubkey) || vc.accepts_any_key { + StatusCode::ACCEPTED + } else { + StatusCode::NOT_FOUND + } + .into_response() +} + +/// Serves a mock VC on an ephemeral port, returning its base URL. +async fn serve(vc: MockVc) -> String { + let app = Router::new() + .route("/eth/v1/keystores", get(keystores)) + .route("/eth/v1/remotekeys", get(remotekeys)) + .route("/eth/v1/validator/{pubkey}/builder_config", get(get_config).post(post_config)) + .with_state(vc); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + tokio::spawn(async move { axum::serve(listener, app).await.unwrap() }); + format!("http://{addr}") +} + +/// A keymanager URL nothing listens on. The socket keeps the port bound while +/// it lives, so another test's server cannot take it +fn down_url() -> (String, tokio::net::TcpSocket) { + let socket = tokio::net::TcpSocket::new_v4().unwrap(); + socket.bind("127.0.0.1:0".parse().unwrap()).unwrap(); + (format!("http://{}", socket.local_addr().unwrap()), socket) +} + +fn random_key() -> String { + cb_common::types::BlsSecretKey::random().public_key().as_hex_string() +} + +struct TestEnv { + cfg: CommitBoostConfig, + targets: Targets, + token_file: tempfile::NamedTempFile, + /// config.toml, for the binary + dir: tempfile::TempDir, +} + +impl TestEnv { + /// A mux of `keys` behind two relays, applied to the VCs at `vc_urls` + fn new(keys: &[String], vc_urls: &[String]) -> Self { + Self::build(keys, vc_urls, "") + } + + /// `new`, with a `[[relays]]` entry for the keys outside the mux + fn with_relays(keys: &[String], vc_urls: &[String]) -> Self { + let relays = + format!("[[relays]]\nurl = \"https://{RELAY_PK_B}@default-relay.example.com\"\n"); + Self::build(keys, vc_urls, &relays) + } + + /// `new`, with `top` after `[pbs]`'s `min_bid_eth`, for more `[pbs]` lines + /// and `[[relays]]` + fn build(keys: &[String], vc_urls: &[String], top: &str) -> Self { + let keys = keys.iter().map(|k| format!("\"{k}\"")).collect::>().join(", "); + let cfg_text = format!( + r#" +chain = "Holesky" + +[pbs] +min_bid_eth = 0.5 +{top} +[[mux]] +id = "mux1" +validator_pubkeys = [{keys}] + +[[mux.relays]] +url = "https://{RELAY_PK_A}@relay-a.example.com" + +[[mux.relays]] +url = "https://{RELAY_PK_B}@relay-b.example.com" +"# + ); + let cfg = parse_config(&cfg_text).unwrap(); + + let token_file = tempfile::NamedTempFile::new().unwrap(); + std::fs::write(token_file.path(), format!("{TOKEN}\n")).unwrap(); + let vcs = vc_urls + .iter() + .map(|url| format!("{url}={}", token_file.path().display()).parse().unwrap()) + .collect(); + let targets = Targets::new("https://cb.example.com".to_string(), vcs).unwrap(); + let dir = tempfile::tempdir().unwrap(); + std::fs::write(dir.path().join("config.toml"), cfg_text).unwrap(); + Self { cfg, targets, token_file, dir } + } + + /// Runs `builder-config apply` on this config and these targets + async fn apply_cli(&self, args: &[&str]) -> std::process::Output { + self.command(args).output().await.unwrap() + } + + /// `builder-config apply` on these targets, not yet run, from this config + /// unless `args` name another source + fn command(&self, args: &[&str]) -> tokio::process::Command { + let mut cmd = builder_config("apply"); + if !args.contains(&"--from") { + cmd.arg("--config").arg(self.dir.path().join("config.toml")); + } + cmd.arg("--advertised-url").arg(&self.targets.advertised_url); + for vc in &self.targets.vcs { + cmd.arg("--vc").arg(format!("{}={}", vc.url, vc.token_path.display())); + } + cmd.args(args); + cmd + } + + /// Runs `builder-config print` on this config + async fn print(&self) -> std::process::Output { + let mut cmd = builder_config("print"); + cmd.arg("--config").arg(self.dir.path().join("config.toml")); + cmd.arg("--advertised-url").arg(&self.targets.advertised_url); + cmd.output().await.unwrap() + } + + async fn projection(&self) -> Projection { + let mux_keys = mux_keys(&self.cfg).await.unwrap(); + project(&self.cfg, &mux_keys, &self.targets.advertised_url).unwrap() + } + + async fn apply(&self, opts: ApplyOptions) -> ApplyReport { + run_apply(&self.projection().await, &self.targets, &opts).await.unwrap() + } + + /// The builder config apply writes for `key`, serialized as it POSTs it + async fn projected(&self, key: &str) -> String { + serde_json::to_string(self.projection().await.doc_for(key).unwrap()).unwrap() + } +} + +/// `commit-boost builder-config `, with no CB_CONFIG inherited +fn builder_config(subcommand: &str) -> tokio::process::Command { + let mut cmd = tokio::process::Command::new(env!("CARGO_BIN_EXE_commit-boost")); + cmd.env_remove("CB_CONFIG").arg("builder-config").arg(subcommand); + cmd +} + +fn third_party_entry(url: &str, auth_hex: &str) -> serde_json::Value { + serde_json::json!({ "url": url, "auth_data": auth_hex, "builder_pubkeys": [RELAY_PK_B] }) +} + +// Each key goes to the client that holds it, and the POST body is the +// projection, without the entries the client stored before +#[tokio::test] +async fn apply_partitioned_keys_accepted_once_each() { + let (k1, k2) = (random_key(), random_key()); + let mut vc1 = MockVc::holding(std::slice::from_ref(&k1)); + vc1.stored.insert( + k1.clone(), + serde_json::json!({ "builders": [third_party_entry("https://third-party.example.com", "0xc0ffee")] }), + ); + let vc2 = MockVc::holding(std::slice::from_ref(&k2)); + let (url1, url2) = (serve(vc1.clone()).await, serve(vc2.clone()).await); + + let env = TestEnv::new(&[k1.clone(), k2.clone()], &[url1, url2]); + let report = env.apply(ApplyOptions::default()).await; + + assert!(report.errors.is_empty(), "{:?}", report.errors); + assert_eq!(report.accepted.get(&k1).map(Vec::len), Some(1)); + assert_eq!(report.accepted.get(&k2).map(Vec::len), Some(1)); + let accepted: Vec<_> = vc1.posts().into_iter().filter(|(pk, _)| pk == &k1).collect(); + assert_eq!(accepted, [(k1.clone(), env.projected(&k1).await)]); +} + +// A client whose setup, key listing or probe fails is skipped with that +// failure's own error; only a probe 404 reads as missing builder_config support +#[tokio::test] +async fn preflight_failures_skip_the_vc() { + for (token, supports_builder_config, get_status, expected) in [ + (None, true, None, "unable to read token file"), + (Some("wrong-token"), true, None, "key listing failed: keystores: 401 Unauthorized"), + ( + Some(TOKEN), + false, + None, + "no builder_config support (keymanager-APIs #88); none of its 1 keys", + ), + ( + Some(TOKEN), + true, + Some(500), + "probe failed: 500 Internal Server Error; none of its 1 keys written", + ), + (Some(TOKEN), true, Some(200), "invalid builder_config response: error decoding"), + ] { + let key = random_key(); + let mut vc = MockVc::holding(std::slice::from_ref(&key)); + vc.supports_builder_config = supports_builder_config; + vc.get_status = get_status; + let url = serve(vc.clone()).await; + + let env = TestEnv::new(std::slice::from_ref(&key), &[url]); + match token { + Some(token) => std::fs::write(env.token_file.path(), token).unwrap(), + None => std::fs::remove_file(env.token_file.path()).unwrap(), + } + let report = env.apply(ApplyOptions::default()).await; + assert!(report.errors.iter().any(|err| err.contains(expected)), "{:?}", report.errors); + assert!(vc.posts().is_empty(), "{expected}"); + } +} + +// A key is written only to the clients that list it: a client that accepts a +// write for any key would otherwise count as a second holder +#[tokio::test] +async fn apply_writes_only_to_clients_that_list_the_key() { + let (key, other) = (random_key(), random_key()); + let empty_vc = MockVc::holding(&[]); + let any_key_vc = + MockVc { accepts_any_key: true, ..MockVc::holding(std::slice::from_ref(&other)) }; + let holder_vc = MockVc::holding(std::slice::from_ref(&key)); + let urls = + [serve(empty_vc.clone()).await, serve(any_key_vc.clone()).await, serve(holder_vc).await]; + + let env = TestEnv::new(std::slice::from_ref(&key), &urls); + let report = env.apply(ApplyOptions::default()).await; + assert!(report.errors.is_empty(), "{:?}", report.errors); + assert_eq!(report.accepted.get(&key), Some(&vec![urls[2].clone() + "/"])); + assert!(empty_vc.posts().is_empty() && any_key_vc.posts().is_empty()); + assert!( + report.warnings.iter().any(|w| w.contains("lists no validator keys")), + "{:?}", + report.warnings + ); +} + +// A client may list a remote-signer key as a keystore too; it holds it once +#[tokio::test] +async fn a_key_listed_twice_by_one_client_is_held_once() { + let key = random_key(); + let vc = MockVc { + remotekeys: Some(vec![key.clone()]), + ..MockVc::holding(std::slice::from_ref(&key)) + }; + let url = serve(vc.clone()).await; + + let env = TestEnv::new(std::slice::from_ref(&key), &[url]); + let report = env.apply(ApplyOptions::default()).await; + assert!(report.errors.is_empty(), "{:?}", report.errors); + assert_eq!(vc.posts().len(), 1); +} + +#[tokio::test] +async fn a_key_no_client_lists_is_an_error() { + let (projected, other) = (random_key(), random_key()); + // the VC lists a different key, so nothing is written + let vc = MockVc::holding(std::slice::from_ref(&other)); + let url = serve(vc.clone()).await; + + let env = TestEnv::new(std::slice::from_ref(&projected), &[url]); + let report = env.apply(ApplyOptions::default()).await; + let expected = format!("keys in a mux that no validator client lists: 1\n {projected}"); + assert_eq!(report.errors, [expected]); + assert!( + report.warnings.iter().any(|w| w.ends_with("no [[relays]] to write for them: 1")), + "{:?}", + report.warnings + ); + assert!(vc.posts().is_empty()); +} + +// A key only a URL or registry loader lists, such as an exited one, is counted +// in one warning, while a key the config names is an error +#[tokio::test] +async fn an_unheld_fetched_key_is_a_warning() { + let (held, fetched) = (random_key(), random_key()); + let url = serve(MockVc::holding(std::slice::from_ref(&held))).await; + + let env = TestEnv::new(&[held.clone(), fetched.clone()], &[url]); + let mut projection = env.projection().await; + // A fetched key a client holds is not counted + projection.fetched_keys.extend([held, fetched]); + let report = run_apply(&projection, &env.targets, &ApplyOptions::default()).await.unwrap(); + assert!(report.errors.is_empty(), "{:?}", report.errors); + let expected = "keys a URL or registry loader lists that no validator client holds: 1"; + assert_eq!(report.warnings, [expected]); +} + +// An unlisted client may hold the keys no other client lists, so the error for +// unheld mux keys and the loader warning both name it +#[tokio::test] +async fn an_unlisted_client_may_hold_the_unheld_keys() { + let (held, unheld, fetched) = (random_key(), random_key(), random_key()); + let (down, _bound) = down_url(); + let up = serve(MockVc::holding(std::slice::from_ref(&held))).await; + + let env = TestEnv::new(&[held, unheld.clone(), fetched.clone()], &[down.clone(), up]); + let mut projection = env.projection().await; + projection.fetched_keys.insert(fetched); + let report = run_apply(&projection, &env.targets, &ApplyOptions::default()).await.unwrap(); + let may_hold = format!("; {down}/ could not be listed and may hold some"); + assert_eq!(report.errors.len(), 2, "{:?}", report.errors); + assert!(report.errors[0].starts_with(&format!("{down}/: key listing failed"))); + let expected = format!("keys in a mux that no validator client lists: 1{may_hold}\n {unheld}"); + assert_eq!(report.errors[1], expected); + let expected = + format!("keys a URL or registry loader lists that no validator client holds: 1{may_hold}"); + assert_eq!(report.warnings, [expected]); + + // With no client listed, its own failure says why nothing was written + let (down, _bound) = down_url(); + let env = TestEnv::new(&[random_key()], &[down]); + let report = env.apply(ApplyOptions::default()).await; + assert!(!report.errors.iter().any(|err| err == "no validator client lists a key")); +} + +// Keys two clients share are one alarm that names both clients and lists the +// keys, whether or not the clients list the same keys: two clients with +// identical keys are a migration or failover left running +#[tokio::test] +async fn the_slashing_alarm_groups_keys_by_their_clients() { + let mut shared = [random_key(), random_key()]; + shared.sort(); + for identical in [false, true] { + let mut second = shared.to_vec(); + if !identical { + second.push(random_key()); + } + let (url1, url2) = + (serve(MockVc::holding(&shared)).await, serve(MockVc::holding(&second)).await); + + let env = TestEnv::with_relays(&shared, &[url1.clone(), url2.clone()]); + let report = env.apply(ApplyOptions::default()).await; + let expected = format!( + "keys held by more than one validator client ({url1}/, {url2}/): 2; each is a \ + slashing risk, so keep it on one\n {}\n {}", + shared[0], shared[1] + ); + assert_eq!(report.errors, [expected]); + } +} + +// After 3 requests in a row get no answer, writes or --preserve-entries reads, +// apply stops writing to that client, once, and counts the keys it left with a +// config to write: the last key is in no mux, so it has none +#[tokio::test] +async fn three_unanswered_writes_stop_the_client() { + let mut keys: Vec = (0..7).map(|_| random_key()).collect(); + keys.sort(); + // An answered read before an unanswered write leaves the write counted + for (preserve_entries, answers_reads, posts) in + [(false, false, 4), (true, false, 1), (true, true, 4)] + { + // The probe, of the lowest key, is answered + let vc = + MockVc { hangs_up_on: keys[1..].to_vec(), answers_reads, ..MockVc::holding(&keys) }; + let url = serve(vc.clone()).await; + + let env = TestEnv::new(&keys[..6], std::slice::from_ref(&url)); + let report = env.apply(ApplyOptions { preserve_entries, ..Default::default() }).await; + assert_eq!(vc.posts().len(), posts, "{preserve_entries} {answers_reads}"); + let stop = format!( + "{url}/: 3 writes in a row got no answer, so its other 2 keys were not written" + ); + let stops: Vec<_> = report.errors.iter().filter(|err| err.contains("in a row")).collect(); + assert_eq!(stops, [&stop], "{preserve_entries}"); + } +} + +// The keys a stopped client is left owing leave out the capped keys its cap +// refusal already skips: keys[0] and keys[4] are in no mux, so they get the +// capped [[relays]] config, which a Lodestar without its flag refuses +#[tokio::test] +async fn an_unanswered_stop_counts_only_the_keys_left_to_write() { + let mut keys: Vec = (0..6).map(|_| random_key()).collect(); + keys.sort(); + let mux_keys = [&keys[1..4], &keys[5..]].concat(); + let vc = MockVc { + lodestar_cap_refusal: true, + hangs_up_on: keys[1..4].to_vec(), + ..MockVc::holding(&keys) + }; + let url = serve(vc).await; + let top = format!( + "max_execution_payment_gwei = 0\n[[relays]]\n\ + url = \"https://{RELAY_PK_B}@default-relay.example.com\"\n\ + max_execution_payment_gwei = \"unclamped\"\n" + ); + let env = TestEnv::build(&mux_keys, std::slice::from_ref(&url), &top); + let report = env.apply(ApplyOptions::default()).await; + let stop = + format!("{url}/: 3 writes in a row got no answer, so its other 1 keys were not written"); + assert!(report.errors.contains(&stop), "{:?}", report.errors); +} + +// Any answer, a refusal included, resets the count, so two requests left +// unanswered on each side of it do not stop the writes. The middle key alone +// gets the capped [[relays]] config, which a Lodestar without its flag refuses +#[tokio::test] +async fn an_answer_resets_the_unanswered_count() { + let mut keys: Vec = (0..7).map(|_| random_key()).collect(); + keys.sort(); + let middle = keys[3].clone(); + let mux_keys: Vec = keys.iter().filter(|key| **key != middle).cloned().collect(); + let hangs_up_on = vec![keys[1].clone(), keys[2].clone(), keys[4].clone(), keys[5].clone()]; + let vc = MockVc { hangs_up_on, ..MockVc::holding(&keys) }; + let refusing = + |status| MockVc { post_status: HashMap::from([(middle.clone(), status)]), ..vc.clone() }; + let unreadable = + MockVc { get_status_for: HashMap::from([(middle.clone(), 500)]), ..vc.clone() }; + let top = format!( + "max_execution_payment_gwei = 0\n[[relays]]\n\ + url = \"https://{RELAY_PK_B}@default-relay.example.com\"\n\ + max_execution_payment_gwei = \"unclamped\"\n" + ); + for (vc, preserve_entries) in [ + (vc.clone(), false), + (refusing(404), false), + (refusing(400), false), + (MockVc { lodestar_cap_refusal: true, ..vc.clone() }, false), + (unreadable, true), + ] { + let env = TestEnv::build(&mux_keys, &[serve(vc).await], &top); + let report = env.apply(ApplyOptions { preserve_entries, ..Default::default() }).await; + assert!(report.accepted.contains_key(&keys[6]), "{:?}", report.errors); + } +} + +// Keys are grouped by the exact clients that list them, named in --vc order +#[tokio::test] +async fn the_slashing_alarm_keeps_distinct_holder_sets_apart() { + let (pair_key, trio_key) = (random_key(), random_key()); + let both = [pair_key.clone(), trio_key.clone()]; + let mut pair = [serve(MockVc::holding(&both)).await, serve(MockVc::holding(&both)).await]; + // Against URL order, which must not reorder them + pair.sort_by(|a, b| b.cmp(a)); + let [first, second] = pair; + let third = serve(MockVc::holding(std::slice::from_ref(&trio_key))).await; + + let env = TestEnv::new(&both, &[first.clone(), second.clone(), third.clone()]); + let report = env.apply(ApplyOptions::default()).await; + let alarm = |clients: String, key: &str| { + format!( + "keys held by more than one validator client ({clients}): 1; each is a slashing \ + risk, so keep it on one\n {key}" + ) + }; + assert_eq!(report.errors, [ + alarm(format!("{first}/, {second}/"), &pair_key), + alarm(format!("{first}/, {second}/, {third}/"), &trio_key), + ]); +} + +#[tokio::test] +async fn no_client_listing_a_key_is_an_error() { + let url = serve(MockVc::holding(&[])).await; + let env = TestEnv::with_relays(&[random_key()], &[url]); + let report = env.apply(ApplyOptions::default()).await; + let expected = "no validator client lists a key".to_string(); + assert!(report.errors.contains(&expected), "{:?}", report.errors); +} + +// The alarm covers a key outside every mux, which here gets no write +#[tokio::test] +async fn a_key_outside_every_mux_two_clients_list_raises_the_slashing_alarm() { + let (mux_key, outside) = (random_key(), random_key()); + let vc1 = MockVc::holding(&[mux_key.clone(), outside.clone()]); + let vc2 = MockVc::holding(std::slice::from_ref(&outside)); + let (url1, url2) = (serve(vc1).await, serve(vc2).await); + + let env = TestEnv::new(std::slice::from_ref(&mux_key), &[url1, url2]); + let report = env.apply(ApplyOptions::default()).await; + assert!( + report + .errors + .iter() + .any(|err| err.contains("held by more than one validator client") && + err.contains(&outside)), + "{:?}", + report.errors + ); +} + +// A key held by two clients, here as a keystore on one and through a remote +// signer on the other, is a slashing risk. The remote-signer key is listed, so +// that client is probed rather than written to blind +#[tokio::test] +async fn a_key_two_clients_list_raises_the_slashing_alarm() { + let key = random_key(); + let vc1 = MockVc::holding(std::slice::from_ref(&key)); + let vc2 = MockVc { remotekeys: Some(vec![key.clone()]), ..MockVc::holding(&[]) }; + let (url1, url2) = (serve(vc1).await, serve(vc2).await); + + let env = TestEnv::new(std::slice::from_ref(&key), &[url1.clone(), url2.clone()]); + let report = env.apply(ApplyOptions::default()).await; + assert!(report.warnings.is_empty(), "{:?}", report.warnings); + // The alarm names both clients + let alarm = format!("held by more than one validator client ({url1}/, {url2}/)"); + assert!(report.errors.iter().any(|err| err.contains(&alarm)), "{:?}", report.errors); +} + +// A second client that lists the key but refuses the write still holds it +#[tokio::test] +async fn apply_duplicate_holder_that_refuses_raises_slashing_alarm() { + for status in [403, 400] { + let key = random_key(); + let vc1 = MockVc::holding(std::slice::from_ref(&key)); + let mut vc2 = MockVc::holding(std::slice::from_ref(&key)); + vc2.post_status.insert(key.clone(), status); + let (url1, url2) = (serve(vc1).await, serve(vc2).await); + + let env = TestEnv::new(std::slice::from_ref(&key), &[url1, url2]); + let report = env.apply(ApplyOptions::default()).await; + assert!( + report.errors.iter().any(|err| err.contains("a slashing risk")), + "{status}: {:?}", + report.errors + ); + } +} + +// A refused write is one error, quoting the keymanager's message, such as +// Lodestar's refusal of a cap +#[tokio::test] +async fn apply_post_refusals() { + for (status, expected) in [ + (400, r#"400 Bad Request: "refused with 400""#), + (403, r#"403 Forbidden: "refused with 403""#), + (404, "but answered 404 to its write"), + ] { + let key = random_key(); + let mut vc = MockVc::holding(std::slice::from_ref(&key)); + vc.post_status.insert(key.clone(), status); + let url = serve(vc).await; + + let env = TestEnv::new(std::slice::from_ref(&key), &[url]); + let report = env.apply(ApplyOptions::default()).await; + assert!( + report.errors.iter().any(|msg| msg.contains(expected)), + "{status}: {:?}", + report.errors + ); + assert_eq!(report.errors.len(), 1, "{status}: {:?}", report.errors); + } +} + +// A 403 before any write lands, as Lodestar answers every write under +// --proposerSettingsFile, is one error and stops writes to that client; after +// a write lands, a 403 is that key's error +#[tokio::test] +async fn a_403_before_any_write_stops_the_client() { + let mut keys = [random_key(), random_key(), random_key()]; + keys.sort(); + // From `first`, the keys are in the mux; keys[0] outside it has no config, + // so a 403 on keys[1] still comes before any write lands + for (first, refused, posts, stopped) in [(0, 0, 1, true), (0, 1, 3, false), (1, 1, 1, true)] { + let mut vc = MockVc::holding(&keys); + vc.post_status.insert(keys[refused].clone(), 403); + let env = TestEnv::new(&keys[first..], &[serve(vc.clone()).await]); + let report = env.apply(ApplyOptions::default()).await; + assert_eq!(vc.posts().len(), posts, "{refused}"); + let stop = report.errors.iter().any(|err| err.contains("answered 403 before any write")); + assert_eq!(stop, stopped, "{:?}", report.errors); + assert_eq!(report.errors.len(), 1, "{:?}", report.errors); + assert!(report.errors[0].contains("refused with 403"), "{:?}", report.errors); + assert_eq!(report.unwritten, if stopped { 3 - first } else { 1 }); + } +} + +// --preserve-entries writes at most the 64 entries a builder config holds; the +// projection here has two +#[tokio::test] +async fn preserve_entries_refuses_more_than_64_entries() { + for (kept, refused) in [(62, false), (63, true)] { + let key = random_key(); + let mut vc = MockVc::holding(std::slice::from_ref(&key)); + let others: Vec<_> = (0..kept) + .map(|i| third_party_entry(&format!("https://builder-{i}.example.com"), "0xbb")) + .collect(); + vc.stored.insert(key.clone(), serde_json::json!({ "builders": others })); + let url = serve(vc.clone()).await; + + let env = TestEnv::new(std::slice::from_ref(&key), &[url]); + let report = env.apply(ApplyOptions { preserve_entries: true, ..Default::default() }).await; + let errored = report.errors.iter().any(|err| err.contains("would keep 65 builder entries")); + assert_eq!(errored, refused, "{kept}: {:?}", report.errors); + assert_eq!(vc.posts().is_empty(), refused, "{kept}"); + } +} + +// --preserve-entries keeps stored entries at other URLs, after the projected +// ones. One at the advertised URL, here with the trailing slash a client may +// add, is replaced, and the key-level values stay the projection's +#[tokio::test] +async fn preserve_entries_keeps_only_entries_at_other_urls() { + let key = random_key(); + let mut vc = MockVc::holding(std::slice::from_ref(&key)); + let env = TestEnv::new(std::slice::from_ref(&key), &[]); + let projected: serde_json::Value = serde_json::from_str(&env.projected(&key).await).unwrap(); + let mut stored = projected.clone(); + stored["min_bid"] = serde_json::json!("1"); + let third_party = third_party_entry("https://third-party.example.com", "0xc0ffee"); + let builders = stored["builders"].as_array_mut().unwrap(); + builders.push(third_party.clone()); + // "cb.example.com" + builders.push(third_party_entry("https://cb.example.com/", "0x63622e6578616d706c652e636f6d")); + vc.stored.insert(key.clone(), stored); + let url = serve(vc.clone()).await; + + let env = TestEnv::new(std::slice::from_ref(&key), &[url]); + let report = env.apply(ApplyOptions { preserve_entries: true, ..Default::default() }).await; + assert!(report.errors.is_empty(), "{:?}", report.errors); + + let mut expected = projected; + expected["builders"].as_array_mut().unwrap().push(third_party); + let posts = vc.posts(); + let posted: serde_json::Value = serde_json::from_str(&posts[0].1).unwrap(); + assert_eq!(posted, expected); +} + +// Under --preserve-entries, a key whose GET answers 404 has nothing to keep, so +// it gets the projection rather than being skipped +#[tokio::test] +async fn preserve_entries_writes_a_key_whose_get_is_404() { + let mut keys = [random_key(), random_key()]; + keys.sort(); + let [probed, missing] = keys; + let mut vc = MockVc::holding(&[probed.clone(), missing.clone()]); + vc.get_status_for.insert(missing.clone(), 404); + let url = serve(vc.clone()).await; + + let env = TestEnv::new(&[probed, missing.clone()], &[url]); + let report = env.apply(ApplyOptions { preserve_entries: true, ..Default::default() }).await; + assert!(report.errors.is_empty(), "{:?}", report.errors); + assert!( + vc.posts().contains(&(missing.clone(), env.projected(&missing).await)), + "{:?}", + vc.posts() + ); +} + +// A key outside every mux gets the `[[relays]]` builder config, a mux key its +// mux's +#[tokio::test] +async fn a_key_outside_every_mux_gets_the_relays_config() { + let (in_mux, outside) = (random_key(), random_key()); + let vc = MockVc::holding(&[in_mux.clone(), outside.clone()]); + let url = serve(vc.clone()).await; + + let env = TestEnv::with_relays(std::slice::from_ref(&in_mux), &[url]); + let report = env.apply(ApplyOptions::default()).await; + assert!(report.errors.is_empty(), "{:?}", report.errors); + assert!(report.warnings.is_empty(), "{:?}", report.warnings); + // auth data is each relay's hex hostname: default-relay and relay-a + let (default_relay, relay_a) = ( + "0x64656661756c742d72656c61792e6578616d706c652e636f6d", + "0x72656c61792d612e6578616d706c652e636f6d", + ); + let posts = vc.posts(); + let body = |key: &str| &posts.iter().find(|(pk, _)| pk == key).unwrap().1; + assert!(body(&outside).contains(default_relay) && !body(&outside).contains(relay_a)); + assert!(body(&in_mux).contains(relay_a) && !body(&in_mux).contains(default_relay)); +} + +// Lodestar refuses a cap above 0 without its flag, for every key alike, so +// apply reports it once, names the fix and writes no further capped key there. +// A key with cap 0 is still written +#[tokio::test] +async fn a_lodestar_cap_refusal_stops_capped_writes_to_that_client() { + // the capped keys sort around the uncapped mux key + let mut keys = [random_key(), random_key(), random_key()]; + keys.sort(); + let [capped, mux_key, capped_later] = keys.clone(); + let vc = MockVc { lodestar_cap_refusal: true, ..MockVc::holding(&keys) }; + let url = serve(vc.clone()).await; + + let top = format!( + "max_execution_payment_gwei = 0\n[[relays]]\n\ + url = \"https://{RELAY_PK_B}@default-relay.example.com\"\n\ + max_execution_payment_gwei = \"unclamped\"\n" + ); + let env = TestEnv::build(std::slice::from_ref(&mux_key), &[url], &top); + let report = env.apply(ApplyOptions::default()).await; + let refusals: Vec<_> = + report.errors.iter().filter(|err| err.contains("restart it with that flag")).collect(); + assert_eq!(refusals.len(), 1, "{:?}", report.errors); + let posted: Vec<_> = vc.posts().into_iter().map(|(pk, _)| pk).collect(); + assert_eq!(posted, [capped, mux_key.clone()]); + assert!(!posted.contains(&capped_later)); + assert_eq!(report.accepted.into_keys().collect::>(), [mux_key]); + assert_eq!(report.unwritten, 2); +} + +// A key whose stored config cannot be read is not written: --preserve-entries +// would erase the entries it keeps +#[tokio::test] +async fn unreadable_config_is_not_written() { + // apply probes the lowest key, so the unreadable one sorts after it + let mut keys = [random_key(), random_key()]; + keys.sort(); + let unreadable = keys[1].clone(); + let mut vc = MockVc::holding(&keys); + vc.get_status_for.insert(unreadable.clone(), 500); + let url = serve(vc.clone()).await; + + let env = TestEnv::new(&keys, &[url]); + let report = env.apply(ApplyOptions { preserve_entries: true, ..Default::default() }).await; + let expected = "preserve-entries GET"; + assert!(report.errors.iter().any(|err| err.contains(expected)), "{:?}", report.errors); + assert!(vc.posts().iter().all(|(pk, _)| pk != &unreadable)); +} + +// A remote-key listing that fails, other than with the 404 of a client without +// remote signing, skips the client rather than dropping its remote keys +#[tokio::test] +async fn remotekeys_failure_skips_the_vc() { + for (status, expected) in [ + (500, "key listing failed: remotekeys: 500"), + (200, "key listing failed: invalid remotekeys response: error decoding"), + ] { + let key = random_key(); + let vc = MockVc { + remotekeys_status: Some(status), + ..MockVc::holding(std::slice::from_ref(&key)) + }; + let url = serve(vc.clone()).await; + + let env = TestEnv::new(std::slice::from_ref(&key), &[url]); + let report = env.apply(ApplyOptions::default()).await; + assert!(report.errors.iter().any(|err| err.contains(expected)), "{:?}", report.errors); + assert!(vc.posts().is_empty()); + } +} + +// A failing client does not stop the run, and the keys it lists still count as +// held, so a key it shares with the next client raises the slashing alarm +#[tokio::test] +async fn a_failing_client_does_not_stop_the_others() { + for (supports_builder_config, get_status, expected) in [ + (false, None, "no builder_config support"), + (true, Some(500), "builder_config probe failed"), + ] { + let key = random_key(); + let broken = MockVc { + supports_builder_config, + get_status, + ..MockVc::holding(std::slice::from_ref(&key)) + }; + let holder = MockVc::holding(std::slice::from_ref(&key)); + let (broken_url, holder_url) = (serve(broken).await, serve(holder).await); + + let env = TestEnv::new(std::slice::from_ref(&key), &[broken_url, holder_url]); + let report = env.apply(ApplyOptions::default()).await; + assert_eq!(report.accepted.get(&key).map(Vec::len), Some(1), "{expected}"); + assert_eq!(report.unwritten, 1, "{expected}"); + assert!(report.errors.iter().any(|err| err.contains(expected)), "{:?}", report.errors); + assert!( + report.errors.iter().any(|err| err.contains("a slashing risk")), + "{:?}", + report.errors + ); + } +} + +// The bearer token goes in cleartext to a client that is neither HTTPS nor +// loopback, so apply warns +#[tokio::test] +async fn warns_on_a_cleartext_token_to_a_remote_client() { + for (url, warns) in [ + ("http://vc.example.com:5062", true), + ("http://10.0.0.5:5062", true), + ("https://vc.example.com:5062", false), + ("http://localhost:5062", false), + ("http://[::ffff:127.0.0.1]:5062", false), + ("http://[::1]:5062", false), + ] { + let env = TestEnv::new(&[random_key()], &[url.to_string()]); + // without a token file, setup fails before any request goes out + std::fs::remove_file(env.token_file.path()).unwrap(); + let report = env.apply(ApplyOptions::default()).await; + let warned = report.warnings.iter().any(|w| w.contains("cleartext")); + assert_eq!(warned, warns, "{url}: {:?}", report.warnings); + } +} + +// print writes only the document to stdout: each mux's config once with its +// keys, and the `[[relays]]` config as the default. Its counts and the +// Lodestar note go to stderr, and it contacts no client +#[tokio::test] +async fn cli_print_contacts_no_client() { + let key = random_key(); + let vc = MockVc::holding(std::slice::from_ref(&key)); + let url = serve(vc.clone()).await; + + let env = TestEnv::with_relays(std::slice::from_ref(&key), &[url]); + let out = env.print().await; + let stderr = String::from_utf8_lossy(&out.stderr); + assert!(out.status.success(), "{stderr}"); + let printed: serde_json::Value = serde_json::from_slice(&out.stdout).unwrap(); + let mux = &printed["muxes"]["mux1"]; + let json = |text: String| serde_json::from_str::(&text).unwrap(); + assert_eq!(mux["config"], json(env.projected(&key).await)); + assert_eq!(mux["keys"], serde_json::json!([key])); + assert_eq!(mux["fetched_keys"], serde_json::json!([])); + assert_eq!(printed["default"], json(env.projected(&random_key()).await)); + assert_eq!(printed["advertised_url"], env.targets.advertised_url); + assert_eq!(printed["version"], 1); + assert!(stderr.contains("mux mux1: 1 keys"), "{stderr}"); + assert!(stderr.contains("NOTE: the builder config sets a max_execution_payment above 0")); + assert!(out.stdout.ends_with(b"}\n")); + assert!(vc.posts().is_empty()); + + // Without [[relays]] the default is null, so other keys are left alone, and + // without a cap above 0 there is no note + let env = TestEnv::build(std::slice::from_ref(&key), &[], "max_execution_payment_gwei = 0\n"); + let out = env.print().await; + let stderr = String::from_utf8_lossy(&out.stderr); + let printed: serde_json::Value = serde_json::from_slice(&out.stdout).unwrap(); + assert!(printed["default"].is_null(), "{printed}"); + assert!(!stderr.contains("NOTE:"), "{stderr}"); +} + +// print exits 2 on an --advertised-url apply would refuse, and 1 when it +// cannot write the document, such as to a closed pipe +#[tokio::test] +async fn cli_print_exit_codes() { + let env = TestEnv::with_relays(&[random_key()], &[]); + let print = |advertised_url: &str| { + let mut cmd = builder_config("print"); + cmd.arg("--config").arg(env.dir.path().join("config.toml")); + cmd.arg("--advertised-url").arg(advertised_url); + cmd + }; + let out = print("localhost:18550").output().await.unwrap(); + let stderr = String::from_utf8_lossy(&out.stderr); + assert_eq!(out.status.code(), Some(2), "{stderr}"); + assert!(stderr.contains("is not an http(s) URL"), "{stderr}"); + + let (reader, writer) = std::io::pipe().unwrap(); + drop(reader); + let mut cmd = print(&env.targets.advertised_url); + cmd.stdout(writer).stderr(std::process::Stdio::piped()); + let out = cmd.spawn().unwrap().wait_with_output().await.unwrap(); + let stderr = String::from_utf8_lossy(&out.stderr); + assert_eq!(out.status.code(), Some(1), "{stderr}"); + assert!(stderr.contains("ERROR: could not write the document"), "{stderr}"); +} + +// A key only a loader lists is printed under fetched_keys, and apply --from +// counts one no client holds in a warning, as apply from the config does +#[tokio::test] +async fn print_and_apply_from_keep_fetched_keys_apart() { + let (held, unheld) = (random_key(), random_key()); + let body = format!(r#"["{held}", "{unheld}"]"#); + let app = Router::new().route("/keys", get(move || async move { body })); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + tokio::spawn(async move { axum::serve(listener, app).await.unwrap() }); + + let vc = MockVc::holding(std::slice::from_ref(&held)); + let env = TestEnv::new(&[random_key()], &[serve(vc.clone()).await]); + let cfg = format!( + "chain = \"Holesky\"\n[pbs]\n[[mux]]\nid = \"m\"\nloader = {{ url = \"http://{addr}/keys\" }}\n\ + [[mux.relays]]\nurl = \"https://{RELAY_PK_A}@relay-a.example.com\"\n" + ); + std::fs::write(env.dir.path().join("config.toml"), cfg).unwrap(); + let printed = env.print().await.stdout; + let mux = &serde_json::from_slice::(&printed).unwrap()["muxes"]["m"]; + let mut fetched = [held.clone(), unheld]; + fetched.sort(); + assert_eq!(mux["keys"], serde_json::json!([])); + assert_eq!(mux["fetched_keys"], serde_json::json!(fetched)); + + let path = env.dir.path().join("printed.json"); + std::fs::write(&path, &printed).unwrap(); + let out = env.apply_cli(&["--from", path.to_str().unwrap()]).await; + let stdout = String::from_utf8_lossy(&out.stdout); + assert_eq!(out.status.code(), Some(0), "{stdout}"); + let warning = "WARN: keys a URL or registry loader lists that no validator client holds: 1"; + assert!(stdout.contains(warning), "{stdout}"); + assert_eq!(vc.posts().into_iter().map(|(key, _)| key).collect::>(), [held]); +} + +// Applying the printed document writes what applying the config writes, from +// a file or from stdin +#[tokio::test] +async fn cli_apply_from_a_printed_document_writes_what_apply_config_writes() { + let (mux_key, other) = (random_key(), random_key()); + for stdin in [false, true] { + let from_config = MockVc::holding(&[mux_key.clone(), other.clone()]); + let from_print = MockVc::holding(&[mux_key.clone(), other.clone()]); + let mut env = + TestEnv::with_relays(std::slice::from_ref(&mux_key), &[ + serve(from_config.clone()).await + ]); + assert!(env.apply_cli(&[]).await.status.success()); + + let printed = env.print().await.stdout; + let path = env.dir.path().join("printed.json"); + std::fs::write(&path, &printed).unwrap(); + env.targets.vcs[0].url = serve(from_print.clone()).await.parse().unwrap(); + let out = if stdin { + let mut cmd = env.command(&["--from", "-"]); + cmd.stdin(std::process::Stdio::piped()).stdout(std::process::Stdio::piped()); + let mut child = cmd.spawn().unwrap(); + let mut input = child.stdin.take().unwrap(); + tokio::io::AsyncWriteExt::write_all(&mut input, &printed).await.unwrap(); + drop(input); + child.wait_with_output().await.unwrap() + } else { + env.apply_cli(&["--from", path.to_str().unwrap()]).await + }; + assert!(out.status.success(), "{}", String::from_utf8_lossy(&out.stderr)); + let sorted = |mut posts: Vec<(String, String)>| { + posts.sort(); + posts + }; + assert_eq!(sorted(from_print.posts()), sorted(from_config.posts()), "stdin {stdin}"); + } +} + +// apply --from refuses, before contacting a client, a document for another +// URL or version, with an entry elsewhere, auth data that is not hex, a field +// print does not write, or a key in two muxes or one that is not a validator +// key +#[tokio::test] +async fn cli_apply_from_refuses_a_mismatched_document() { + let key = random_key(); + let vc = MockVc::holding(std::slice::from_ref(&key)); + let env = TestEnv::with_relays(std::slice::from_ref(&key), &[serve(vc.clone()).await]); + let printed: serde_json::Value = serde_json::from_slice(&env.print().await.stdout).unwrap(); + let edited = |edit: &dyn Fn(&mut serde_json::Value)| { + let mut printed = printed.clone(); + edit(&mut printed); + printed + }; + for (document, expected) in [ + (edited(&|p| p["advertised_url"] = "http://other:18550".into()), "not --advertised-url"), + ( + edited(&|p| { + let url = format!("{}/", p["advertised_url"].as_str().unwrap()); + p["advertised_url"] = url.into(); + }), + "not --advertised-url", + ), + ( + edited(&|p| p["muxes"]["mux1"]["config"]["builders"][0]["auth_data"] = "cb".into()), + "not 0x-prefixed hex", + ), + ( + edited(&|p| { + p["muxes"]["mux1"]["config"]["builders"][0]["max_execution_paymnet"] = "0".into() + }), + "a field `print` does not write", + ), + (edited(&|p| p["version"] = 2.into()), "is version 2"), + (edited(&|p| p["version"] = 0.into()), "is version 0"), + // The second entry, so every entry is checked + ( + edited(&|p| p["muxes"]["mux1"]["config"]["builders"][1]["url"] = "http://evil".into()), + "has an entry at http://evil", + ), + ( + edited(&|p| p["default"]["builders"][0]["url"] = "http://evil".into()), + "default's config has an entry at http://evil", + ), + // Written as is, so a URL that only parses the same is refused too + ( + edited(&|p| { + let url = format!("{}/", p["advertised_url"].as_str().unwrap()); + p["muxes"]["mux1"]["config"]["builders"][0]["url"] = url.into(); + }), + "has an entry at", + ), + ( + edited(&|p| { + p["muxes"]["mux2"] = p["muxes"]["mux1"].clone(); + p["muxes"]["mux2"]["keys"] = + serde_json::json!([key.to_uppercase().replace("0X", "0x")]); + }), + "is in both mux mux1 and mux mux2", + ), + ( + edited(&|p| p["muxes"]["mux1"]["keys"] = serde_json::json!(["0x1234"])), + "is not a validator key", + ), + (edited(&|p| p["extra"] = 1.into()), "is not a printed document"), + ] { + let path = env.dir.path().join("printed.json"); + std::fs::write(&path, document.to_string()).unwrap(); + let out = env.apply_cli(&["--from", path.to_str().unwrap()]).await; + let stderr = String::from_utf8_lossy(&out.stderr); + assert_eq!(out.status.code(), Some(2), "{expected}: {stderr}"); + assert!(stderr.contains(expected), "{expected}: {stderr}"); + } + assert!(vc.posts().is_empty()); +} + +// --config defaults to CB_CONFIG, and --from wins over it +#[tokio::test] +async fn cli_apply_reads_cb_config_and_from_wins_over_it() { + let key = random_key(); + let vc = MockVc::holding(std::slice::from_ref(&key)); + let env = TestEnv::new(std::slice::from_ref(&key), &[serve(vc.clone()).await]); + let mut cmd = builder_config("apply"); + cmd.env("CB_CONFIG", env.dir.path().join("config.toml")); + cmd.arg("--advertised-url").arg(&env.targets.advertised_url); + for vc in &env.targets.vcs { + cmd.arg("--vc").arg(format!("{}={}", vc.url, vc.token_path.display())); + } + assert!(cmd.output().await.unwrap().status.success()); + + let path = env.dir.path().join("printed.json"); + std::fs::write(&path, env.print().await.stdout).unwrap(); + let mut cmd = env.command(&["--from", path.to_str().unwrap()]); + cmd.env("CB_CONFIG", "/nonexistent.toml"); + let out = cmd.output().await.unwrap(); + assert!(out.status.success(), "{}", String::from_utf8_lossy(&out.stderr)); + assert_eq!(vc.posts().len(), 2); +} + +// The loaders' own warnings reach stderr, here for a keys URL over HTTP, even +// under a RUST_LOG meant for another program, and count in the tally; each +// mux's key count is printed +#[tokio::test] +async fn cli_shows_the_loaders_warnings() { + let key = random_key(); + let body = format!(r#"["{key}"]"#); + let app = Router::new().route("/keys", get(move || async move { body })); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + tokio::spawn(async move { axum::serve(listener, app).await.unwrap() }); + + let env = TestEnv::new(&[random_key()], &[serve(MockVc::holding(&[key])).await]); + let cfg = format!( + "chain = \"Holesky\"\n[pbs]\n[[mux]]\nid = \"m\"\nloader = {{ url = \"http://{addr}/keys\" }}\n\ + [[mux.relays]]\nurl = \"https://{RELAY_PK_A}@relay-a.example.com\"\n" + ); + std::fs::write(env.dir.path().join("config.toml"), cfg).unwrap(); + for rust_log in + [None, Some(""), Some("lighthouse=debug"), Some("info"), Some("error"), Some("off")] + { + let mut cmd = env.command(&[]); + match rust_log { + Some(rust_log) => cmd.env("RUST_LOG", rust_log), + None => cmd.env_remove("RUST_LOG"), + }; + let out = cmd.output().await.unwrap(); + let (stdout, stderr) = + (String::from_utf8_lossy(&out.stdout), String::from_utf8_lossy(&out.stderr)); + assert!(out.status.success(), "{rust_log:?}: {stderr}"); + assert!(stderr.contains("is insecure"), "{rust_log:?}: {stderr}"); + assert!(!stderr.contains('\u{1b}'), "ANSI codes on a piped stderr: {stderr:?}"); + assert!(stdout.contains("mux m: 1 keys"), "{stdout}"); + assert!(stdout.contains("; 0 errors, 1 warnings"), "{stdout}"); + } +} + +// A sidecar can run before its client has keys: with --partial that is a +// warning, not a failure +#[tokio::test] +async fn partial_allows_a_client_with_no_keys() { + let env = TestEnv::with_relays(&[random_key()], &[serve(MockVc::holding(&[])).await]); + for (args, code) in [(&[][..], 1), (&["--partial"][..], 0)] { + let out = env.apply_cli(args).await; + let stdout = String::from_utf8_lossy(&out.stdout); + assert_eq!(out.status.code(), Some(code), "{args:?}: {stdout}"); + } + let out = env.apply_cli(&["--partial"]).await; + let stdout = String::from_utf8_lossy(&out.stdout); + assert!(stdout.contains("WARN: no validator client given lists a key"), "{stdout}"); +} + +// A reader that stops early, such as `| head -1`, closes stdout, and the +// writes still finish +#[tokio::test] +async fn a_closed_stdout_does_not_stop_the_writes() { + use tokio::io::AsyncBufReadExt; + + let keys: Vec = (0..50).map(|_| random_key()).collect(); + let vc = MockVc::holding(&keys); + let env = TestEnv::new(&keys, &[serve(vc.clone()).await]); + let mut cmd = env.command(&[]); + cmd.stdout(std::process::Stdio::piped()).stderr(std::process::Stdio::null()); + let mut child = cmd.spawn().unwrap(); + let mut stdout = tokio::io::BufReader::new(child.stdout.take().unwrap()); + let mut first = String::new(); + stdout.read_line(&mut first).await.unwrap(); + assert!(first.starts_with("mux mux1: 50 keys"), "{first}"); + drop(stdout); + assert_eq!(child.wait().await.unwrap().code(), Some(0)); + assert_eq!(vc.posts().len(), 50); +} + +// Nor does a closed stderr, which the errors go to; the tally counts the keys +// of a client that could not take them +#[tokio::test] +async fn a_closed_stderr_does_not_stop_the_writes() { + let keys: Vec = (0..5).map(|_| random_key()).collect(); + let vc = MockVc::holding(&keys[..3]); + let unsupported = MockVc { supports_builder_config: false, ..MockVc::holding(&keys[3..]) }; + let env = TestEnv::new(&keys, &[serve(unsupported).await, serve(vc.clone()).await]); + let closed_stderr = || { + let (reader, writer) = std::io::pipe().unwrap(); + drop(reader); + writer + }; + let mut cmd = env.command(&[]); + cmd.stdout(std::process::Stdio::piped()).stderr(closed_stderr()); + let out = cmd.spawn().unwrap().wait_with_output().await.unwrap(); + let stdout = String::from_utf8_lossy(&out.stdout); + assert_eq!(out.status.code(), Some(1), "{stdout}"); + assert_eq!(vc.posts().len(), 3); + assert!(stdout.contains("done: 3 keys written, 2 not written, on 1 of 2"), "{stdout}"); + + // A run that stops before contacting a client still exits 2 + let mut cmd = TestEnv::new(&keys, &[]).command(&[]); + cmd.stdout(std::process::Stdio::null()).stderr(closed_stderr()); + assert_eq!(cmd.status().await.unwrap().code(), Some(2)); +} + +#[tokio::test] +async fn cli_refuses_a_config_with_nothing_to_write() { + let env = TestEnv::new(&[random_key()], &[serve(MockVc::holding(&[])).await]); + std::fs::write(env.dir.path().join("config.toml"), "chain = \"Holesky\"\n[pbs]\n").unwrap(); + let out = env.print().await; + let stderr = String::from_utf8_lossy(&out.stderr); + assert_eq!(out.status.code(), Some(2), "{stderr}"); + assert!(stderr.contains("ERROR: nothing to write"), "{stderr}"); +} + +// Bad flags and unreadable token files stop the run with exit 2 before any +// client is contacted +#[tokio::test] +async fn cli_refuses_bad_targets() { + let env = TestEnv::new(&[random_key()], &[]); + let vc = format!("http://127.0.0.1:1={}", env.token_file.path().display()); + let empty = tempfile::NamedTempFile::new().unwrap(); + let empty = format!("http://127.0.0.1:2={}", empty.path().display()); + let cb = "http://cb.example.com"; + for (args, expected) in [ + (vec!["--advertised-url", cb], "no validator client given"), + (vec!["--advertised-url", "localhost:18550", "--vc", &vc], "is not an http(s) URL"), + (vec!["--advertised-url", cb, "--vc", &vc, "--vc", &vc], "given twice"), + (vec!["--advertised-url", cb, "--vc", "localhost:1=/t"], "not an http(s) URL"), + (vec!["--advertised-url", cb, "--vc", "http://127.0.0.1:1=~/t"], "use $HOME"), + // A token file after the first is read too + (vec!["--advertised-url", cb, "--vc", &vc, "--vc", &empty], "is empty"), + ( + vec![ + "--advertised-url", + cb, + "--vc", + "http://localhost:1=/t", + "--vc", + "http://127.0.0.1:1=/t", + ], + "one validator client given twice", + ), + ] { + let out = builder_config("apply") + .arg("--config") + .arg(env.dir.path().join("config.toml")) + .args(&args) + .output() + .await + .unwrap(); + let stderr = String::from_utf8_lossy(&out.stderr); + assert_eq!(out.status.code(), Some(2), "{args:?}: {stderr}"); + assert!(stderr.contains(expected), "{args:?}: {stderr}"); + } +} + +// apply exits 1 on any error, such as a mux key no client lists, which +// --partial turns into a warning. It prints where each key's config came +// from, a line per client and a closing tally +#[tokio::test] +async fn cli_exit_code() { + for (held, args, code) in + [(true, &[][..], 0), (false, &[][..], 1), (false, &["--partial"][..], 0)] + { + let key = random_key(); + let other = random_key(); + let url = serve(MockVc::holding(&[if held { key.clone() } else { other.clone() }])).await; + + let env = TestEnv::with_relays(std::slice::from_ref(&key), std::slice::from_ref(&url)); + let out = env.apply_cli(args).await; + let (stdout, stderr) = + (String::from_utf8_lossy(&out.stdout), String::from_utf8_lossy(&out.stderr)); + assert_eq!(out.status.code(), Some(code), "{args:?}: {stderr}"); + let unheld = "keys in a mux that no validator client lists"; + assert_eq!(stderr.contains(unheld), code == 1, "{stderr}"); + let partial = "WARN: keys in a mux that no validator client given holds: 1"; + assert_eq!(stdout.contains(partial), !args.is_empty(), "{stdout}"); + let (accepted, source) = if held { (&key, "mux mux1") } else { (&other, "[[relays]]") }; + assert!(stdout.contains(&format!("accepted: {accepted} on {url}/ ({source})")), "{stdout}"); + assert!(stdout.contains(&format!("{url}/: 1 keys listed, 1 written")), "{stdout}"); + let errors = usize::from(code == 1); + assert!( + stdout.contains(&format!( + "done: 1 keys written, 0 not written, on 1 of 1 validator clients; {errors} errors" + )), + "{stdout}" + ); + } +} + +// A client written to gets a line counting only the keys with a config to +// write, here not the key in no mux. The tally counts each write, so a key on +// two clients (a slashing error) counts twice, and counts the clients written +// to out of every client given. The Lodestar note is for print only +#[tokio::test] +async fn cli_prints_a_line_per_client_and_a_tally() { + let mut keys = [random_key(), random_key(), random_key()]; + keys.sort(); + let [a, b, c] = keys.clone(); + let (vc1, vc2) = (MockVc::holding(&[a, c.clone(), random_key()]), MockVc::holding(&[b, c])); + let urls = [serve(vc1).await, serve(vc2).await, serve(MockVc::holding(&[])).await]; + + let env = TestEnv::new(&keys, &urls); + let out = env.apply_cli(&[]).await; + let stdout = String::from_utf8_lossy(&out.stdout); + assert_eq!(out.status.code(), Some(1), "{stdout}"); + for line in [ + format!("{}/: 3 keys listed, 2 written\n", urls[0]), + format!("{}/: 2 keys listed, 2 written\n", urls[1]), + format!("WARN: {}/: lists no validator keys\n", urls[2]), + "done: 4 keys written, 0 not written, on 2 of 3 validator clients; 1 errors, 2 warnings\n" + .to_string(), + ] { + assert!(stdout.contains(&line), "{line}{stdout}"); + } + assert!(!stdout.contains("NOTE:"), "{stdout}"); +} diff --git a/config.example.toml b/config.example.toml index 1c6354e60..235fa7383 100644 --- a/config.example.toml +++ b/config.example.toml @@ -34,6 +34,15 @@ 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 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 +# (unreleased, from v0.12.0-rc1) Read only by `commit-boost builder-config`: the cap, in Gwei, on the execution payment a validator counts toward a bid, or "unclamped" to count all of it (https://commit-boost.github.io/commit-boost-client/get_started/epbs#builder-config-command) +# OPTIONAL, DEFAULT: "unclamped" +# max_execution_payment_gwei = 0 +# (unreleased, from v0.12.0-rc1) Read only by `commit-boost builder-config`: the min_bid it writes for bids received over p2p (https://commit-boost.github.io/commit-boost-client/get_started/epbs#builder-config-command) +# OPTIONAL, DEFAULT: the entries' min_bid +# min_bid_p2p_eth = 0.0 +# (unreleased, from v0.12.0-rc1) Read only by `commit-boost builder-config`: the builder_boost_factor it writes for bids received over p2p (https://commit-boost.github.io/commit-boost-client/get_started/epbs#builder-config-command) +# OPTIONAL, DEFAULT: the entries' builder_boost_factor +# builder_boost_factor_p2p = 100 # Timeout in milliseconds for the `submit_blinded_block` call to relays. # OPTIONAL, DEFAULT: 4000 timeout_get_payload_ms = 4000 @@ -138,6 +147,9 @@ target_first_request_ms = 200 # Frequency in ms to send get_header requests # OPTIONAL frequency_get_header_ms = 300 +# (unreleased, from v0.12.0-rc1) Read only by `commit-boost builder-config`: this relay's cap, instead of the [pbs] one (https://commit-boost.github.io/commit-boost-client/get_started/epbs#builder-config-command) +# OPTIONAL, DEFAULT: the [pbs] cap +# max_execution_payment_gwei = 0 # Configuration for the PBS multiplexers, which enable different configs to be used for get header requests, depending on validator pubkey # Note that: @@ -184,12 +196,21 @@ loader = "./tests/data/mux_keys.example.json" # loader = { registry = "stader", node_operator_id = 8, stader_pool = "permissionless", enable_refreshing = false } late_in_slot_time_ms = 1500 timeout_get_header_ms = 900 +# (unreleased, from v0.12.0-rc1) Read only by `commit-boost builder-config`, not by the PBS service: the min_bid it writes for this mux's keys, for p2p bids too unless [pbs] min_bid_p2p_eth is set (https://commit-boost.github.io/commit-boost-client/get_started/epbs#builder-config-command) +# OPTIONAL, DEFAULT: the [pbs] min_bid_eth +# min_bid_eth = 0.0 +# (unreleased, from v0.12.0-rc1) Read only by `commit-boost builder-config`: the builder_boost_factor it writes for this mux's keys, for p2p bids too unless [pbs] builder_boost_factor_p2p is set (https://commit-boost.github.io/commit-boost-client/get_started/epbs#builder-config-command) +# OPTIONAL, DEFAULT: 100 +# builder_boost_factor = 100 # For each mux, one or more [[mux.relays]] can be defined, which will be used for the matching validator pubkeys # Only the relays defined here will be used, and the relays defined in the main [[relays]] config will be ignored # The fields specified here are the same as in [[relays]] (headers, get_params, enable_timing_games, target_first_request_ms, frequency_get_header_ms) [[mux.relays]] id = "mux-relay-1" url = "http://0xa119589bb33ef52acbb8116832bec2b58fca590fe5c85eac5d3230b44d5bc09fe73ccd21f88eab31d6de16194d17782e@def.xyz" +# (unreleased, from v0.12.0-rc1) Read only by `commit-boost builder-config`: this relay's cap, instead of the [pbs] one (https://commit-boost.github.io/commit-boost-client/get_started/epbs#builder-config-command) +# OPTIONAL, DEFAULT: the [pbs] cap +# max_execution_payment_gwei = 0 # Configuration for the Signer service, only required if any `commit` module is present, or if `pbs.with_signer = true` # Currently two types of Signer service are supported (only one can be used at a time): diff --git a/crates/common/src/config/mod.rs b/crates/common/src/config/mod.rs index 378c57649..bf47b7682 100644 --- a/crates/common/src/config/mod.rs +++ b/crates/common/src/config/mod.rs @@ -3,7 +3,7 @@ use std::{ path::{Path, PathBuf}, }; -use eyre::{Result, bail}; +use eyre::{Context, Result, bail}; use serde::{Deserialize, Serialize}; use crate::types::{Chain, ChainLoader, ForkVersion, load_chain_from_file}; @@ -59,14 +59,14 @@ impl CommitBoostConfig { } pub fn from_file(path: &PathBuf) -> Result { - let (config, _): (Self, _) = load_from_file(path)?; - Ok(config) + load_checked(path) } // When loading the config from the environment, it's important that every path // is replaced with the correct value if the config is loaded inside a container pub fn from_env_path() -> Result<(Self, PathBuf)> { - let (helper_config, config_path): (HelperConfig, PathBuf) = load_file_from_env(CONFIG_ENV)?; + let config_path = PathBuf::from(load_env_var(CONFIG_ENV)?); + let helper_config: HelperConfig = load_checked(&config_path)?; let chain = match helper_config.chain { ChainLoader::Path { path, genesis_time_secs } => { @@ -184,6 +184,17 @@ impl CommitBoostConfig { } } +/// Parses the config twice from one read: as a `toml::Value` to check each +/// `[[mux]]` alone, then as `T` from the text, so a type error keeps its line +fn load_checked + std::fmt::Debug, T: serde::de::DeserializeOwned>( + path: P, +) -> Result { + let text = + std::fs::read_to_string(&path).wrap_err(format!("Unable to find config file: {path:?}"))?; + check_mux_tables(&toml::from_str(&text).wrap_err("could not deserialize toml from string")?)?; + toml::from_str(&text).wrap_err("could not deserialize toml from string") +} + /// Helper struct to load the chain spec file #[derive(Deserialize)] struct ChainConfig { diff --git a/crates/common/src/config/mux.rs b/crates/common/src/config/mux.rs index 1b1f0c38e..6c0628199 100644 --- a/crates/common/src/config/mux.rs +++ b/crates/common/src/config/mux.rs @@ -24,7 +24,7 @@ use crate::{ interop::{lido::utils::*, ssv::utils::*, stader::utils::*}, pbs::RelayClient, types::{BlsPublicKey, Chain, StaderPool}, - utils::default_bool, + utils::{as_opt_eth_str, default_bool}, wire::safe_read_http_response, }; @@ -153,6 +153,11 @@ pub struct MuxConfig { pub loader: Option, pub timeout_get_header_ms: Option, pub late_in_slot_time_ms: Option, + // The fields below are read only by `commit-boost builder-config`, which + // writes them into the validators' builder config + pub builder_boost_factor: Option, + #[serde(rename = "min_bid_eth", with = "as_opt_eth_str", default)] + pub min_bid_wei: Option, } impl MuxConfig { @@ -306,6 +311,22 @@ impl MuxKeysLoader { } } +/// The flattened `Option` reads a `[[mux]]` that fails to parse as +/// no muxes at all, so parse each one alone to surface its error +pub fn check_mux_tables(raw: &toml::Value) -> eyre::Result<()> { + let Some(muxes) = raw.get("mux") else { return Ok(()) }; + let Some(muxes) = muxes.as_array() else { + eyre::bail!("`mux` is not an array of tables: write [[mux]], not [mux]") + }; + for mux in muxes { + let id = mux.get("id").and_then(toml::Value::as_str).unwrap_or_default(); + mux.clone() + .try_into::() + .wrap_err_with(|| format!("could not parse [[mux]] {id}"))?; + } + Ok(()) +} + fn load_file + std::fmt::Debug>(path: P) -> eyre::Result { std::fs::read_to_string(&path).wrap_err(format!("Unable to find mux keys file: {path:?}")) } @@ -547,3 +568,75 @@ async fn fetch_ssv_pubkeys_from_public_api( Ok(pubkeys) } + +#[cfg(test)] +mod tests { + use super::*; + use crate::{ + config::{ + CONFIG_ENV, CommitBoostConfig, + test_env::{RELAY_URL, with_env}, + }, + types::BlsSecretKey, + }; + + // Both loaders refuse a mux that fails to parse, rather than run without muxes + #[test] + fn a_mux_that_fails_to_parse_is_an_error() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("cb-config.toml"); + let key = BlsSecretKey::random().public_key().as_hex_string(); + let key2 = BlsSecretKey::random().public_key().as_hex_string(); + for (line, ok) in [("", true), ("builder_boost_factor = 1.5", false)] { + std::fs::write( + &path, + format!( + "chain = \"Holesky\"\n[pbs]\n[[relays]]\nurl = \"{RELAY_URL}\"\n\ + [[mux]]\nid = \"m1\"\nvalidator_pubkeys = [\"{key}\"]\n\ + [[mux.relays]]\nurl = \"{RELAY_URL}\"\n\ + [[mux]]\nid = \"m2\"\nvalidator_pubkeys = [\"{key2}\"]\n{line}\n\ + [[mux.relays]]\nurl = \"{RELAY_URL}\"\n" + ), + ) + .unwrap(); + let from_env = + with_env(&[(CONFIG_ENV, path.to_str())], CommitBoostConfig::from_env_path); + for (loader, muxes) in [ + ("from_file", CommitBoostConfig::from_file(&path).map(|cfg| cfg.muxes)), + ("from_env_path", from_env.map(|(cfg, _)| cfg.muxes)), + ] { + match muxes { + Ok(muxes) => { + assert!(ok && muxes.is_some_and(|m| m.muxes.len() == 2), "{loader}: {line}") + } + Err(err) => assert!( + !ok && format!("{err:#}").contains("could not parse [[mux]] m2"), + "{loader}: {line}: {err:#}" + ), + } + } + } + } + + // `[mux]` for `[[mux]]` would otherwise also read as no muxes + #[test] + fn a_mux_that_is_not_an_array_of_tables_is_an_error() { + for text in ["[mux]\nid = \"m\"", "mux = \"m\""] { + let raw: toml::Value = toml::from_str(text).unwrap(); + assert!(check_mux_tables(&raw).is_err(), "{text}"); + } + } + + #[test] + fn a_config_error_names_its_line() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("cb-config.toml"); + let text = + format!("chain = \"Holesky\"\n[pbs]\n[[relays]]\nurl = \"{RELAY_URL}\"\nfoo = 1\n"); + std::fs::write(&path, text).unwrap(); + let from_env = with_env(&[(CONFIG_ENV, path.to_str())], CommitBoostConfig::from_env_path); + for err in [CommitBoostConfig::from_file(&path).unwrap_err(), from_env.unwrap_err()] { + assert!(format!("{err:#}").contains("line 5"), "{err:#}"); + } + } +} diff --git a/crates/common/src/config/pbs.rs b/crates/common/src/config/pbs.rs index 294c5b535..c1cc7efa2 100644 --- a/crates/common/src/config/pbs.rs +++ b/crates/common/src/config/pbs.rs @@ -8,6 +8,7 @@ use std::{ net::{Ipv4Addr, SocketAddr}, path::{Path, PathBuf}, sync::Arc, + time::Duration, }; use alloy::{ @@ -36,8 +37,8 @@ use crate::{ }, types::{BlsPublicKey, Chain, Jwt, ModuleId}, utils::{ - WEI_PER_ETH, as_eth_str, default_bool, default_host, default_u16, default_u32, default_u64, - default_u256, + WEI_PER_ETH, as_eth_str, as_opt_eth_str, default_bool, default_host, default_u16, + default_u32, default_u64, default_u256, }, }; @@ -145,6 +146,59 @@ pub struct RelayConfig { /// request #[serde(deserialize_with = "empty_string_as_none", default)] pub validator_registration_batch_size: Option, + /// Overrides the `[pbs]` cap for this relay; read only by + /// `commit-boost builder-config` + pub max_execution_payment_gwei: Option, +} + +/// The cap on the execution payment a validator counts toward a bid +#[derive(Debug, Clone, Copy, Deserialize, Serialize)] +#[serde(try_from = "CapValue", into = "CapValue")] +pub enum ExecutionPaymentCap { + Gwei(u64), + /// `"unclamped"`: the whole execution payment counts + Unclamped, +} + +impl ExecutionPaymentCap { + /// The keymanager `max_execution_payment`, where the largest Uint64 + /// leaves the payment unclamped + pub fn gwei(self) -> u64 { + match self { + Self::Gwei(gwei) => gwei, + Self::Unclamped => u64::MAX, + } + } +} + +#[derive(Deserialize, Serialize)] +#[serde(untagged)] +enum CapValue { + Gwei(u64), + Keyword(String), +} + +impl TryFrom for ExecutionPaymentCap { + type Error = String; + + fn try_from(value: CapValue) -> Result { + match value { + CapValue::Gwei(gwei) => Ok(Self::Gwei(gwei)), + CapValue::Keyword(word) if word == "unclamped" => Ok(Self::Unclamped), + CapValue::Keyword(word) => { + Err(format!("expected a Gwei amount or \"unclamped\", got {word:?}")) + } + } + } +} + +impl From for CapValue { + fn from(cap: ExecutionPaymentCap) -> Self { + match cap { + ExecutionPaymentCap::Gwei(gwei) => Self::Gwei(gwei), + ExecutionPaymentCap::Unclamped => Self::Keyword("unclamped".to_string()), + } + } } fn empty_string_as_none<'de, D>(deserializer: D) -> Result, D::Error> @@ -236,6 +290,14 @@ pub struct PbsConfig { /// from the registry, in seconds #[serde(default = "default_u64::<{ DEFAULT_REGISTRY_REFRESH_SECONDS }>")] pub mux_registry_refresh_interval_seconds: u64, + // The fields below are read only by `commit-boost builder-config`, which + // writes them into the validators' builder config + pub max_execution_payment_gwei: Option, + /// Key-level value, for p2p bids + #[serde(rename = "min_bid_p2p_eth", with = "as_opt_eth_str", default)] + pub min_bid_p2p_wei: Option, + /// Key-level value, for p2p bids + pub builder_boost_factor_p2p: Option, } impl PbsConfig { @@ -280,8 +342,17 @@ impl PbsConfig { } if let Some(rpc_url) = &self.rpc_url { + ensure!( + self.http_timeout_seconds > 0, + "http_timeout_seconds must be greater than 0 to check rpc_url" + ); let provider = ProviderBuilder::new().connect_http(rpc_url.clone()); - let chain_id = provider.get_chain_id().await?; + let timeout = Duration::from_secs(self.http_timeout_seconds); + let chain_id = + tokio::time::timeout(timeout, provider.get_chain_id()).await.map_err(|_| { + // The URL is left out: it often carries an API key + eyre::eyre!("rpc_url did not answer eth_chainId within {timeout:?}") + })??; let chain_id_big = U256::from(chain_id); ensure!( chain_id_big == chain.id(), @@ -563,6 +634,33 @@ mod tests { ) } + // An RPC that accepts the connection and never answers + #[tokio::test] + async fn test_stalled_rpc_url_fails_validation() { + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + tokio::spawn(async move { + let mut held = Vec::new(); + while let Ok((socket, _)) = listener.accept().await { + held.push(socket); + } + }); + let config: CommitBoostConfig = toml::from_str(&format!( + "chain = \"Holesky\"\n[pbs]\nrpc_url = \"http://{addr}\"\nhttp_timeout_seconds = 1\n\ + [[relays]]\nurl = \"{RELAY_URL}\"\n" + )) + .unwrap(); + let validate = tokio::time::timeout(Duration::from_secs(10), config.validate()); + let err = validate.await.expect("validate hung").expect_err("a stalled rpc_url validated"); + assert!(format!("{err:#}").contains("did not answer eth_chainId within 1s"), "{err:#}"); + + // A zero timeout could never let the check pass + let mut config = config; + config.pbs.pbs_config.http_timeout_seconds = 0; + let err = config.validate().await.expect_err("a zero http_timeout_seconds validated"); + assert!(format!("{err:#}").contains("http_timeout_seconds must be greater than 0")); + } + #[tokio::test] async fn test_deadline_buffer_range() { for (buffer, accepted) in [(0, false), (1, true), (11999, true), (12000, false)] { diff --git a/crates/common/src/config/signer.rs b/crates/common/src/config/signer.rs index 6d948adc2..9e104f9de 100644 --- a/crates/common/src/config/signer.rs +++ b/crates/common/src/config/signer.rs @@ -483,6 +483,9 @@ mod tests { register_validator_retry_limit: 3, validator_registration_batch_size: None, mux_registry_refresh_interval_seconds: 5, + max_execution_payment_gwei: None, + min_bid_p2p_wei: None, + builder_boost_factor_p2p: None, ssv_node_api_url: Url::parse("https://example.net").unwrap(), ssv_public_api_url: Url::parse("https://example.net").unwrap(), }, diff --git a/crates/common/src/utils.rs b/crates/common/src/utils.rs index c167c33db..ffceca5ac 100644 --- a/crates/common/src/utils.rs +++ b/crates/common/src/utils.rs @@ -84,12 +84,10 @@ pub fn test_encode_decode(d: &str) -> T { pub mod as_eth_str { use alloy::primitives::{ U256, - utils::{format_ether, parse_ether}, + utils::{ParseUnits, Unit, format_ether}, }; use serde::Deserialize; - use super::eth_to_wei; - pub fn serialize(data: &U256, serializer: S) -> Result where S: serde::Serializer, @@ -109,15 +107,42 @@ pub mod as_eth_str { F64(f64), } - let value = StringOrF64::deserialize(deserializer)?; - let wei = match value { - StringOrF64::Str(s) => { - parse_ether(&s).map_err(|_| serde::de::Error::custom("invalid eth amount"))? - } - StringOrF64::F64(f) => eth_to_wei(f), + let value = match StringOrF64::deserialize(deserializer)? { + StringOrF64::Str(s) => s, + // Its shortest decimal form, so `0.009` is exact + StringOrF64::F64(f) => f.to_string(), }; + // A negative amount parses as a signed value, which is refused + match ParseUnits::parse_units(&value, Unit::ETHER) { + Ok(ParseUnits::U256(wei)) => Ok(wei), + _ => Err(serde::de::Error::custom(format!("invalid eth amount: {value}"))), + } + } +} + +/// `as_eth_str` for an optional field +pub mod as_opt_eth_str { + use alloy::primitives::U256; + use serde::{Deserialize, Deserializer, Serializer}; - Ok(wei) + pub fn serialize(data: &Option, serializer: S) -> Result + where + S: Serializer, + { + match data { + Some(wei) => super::as_eth_str::serialize(wei, serializer), + None => serializer.serialize_none(), + } + } + + pub fn deserialize<'de, D>(deserializer: D) -> Result, D::Error> + where + D: Deserializer<'de>, + { + #[derive(Deserialize)] + struct Eth(#[serde(with = "super::as_eth_str")] U256); + + Ok(Option::::deserialize(deserializer)?.map(|Eth(wei)| wei)) } } @@ -465,7 +490,7 @@ pub fn bls_pubkey_from_hex_unchecked(hex: &str) -> BlsPublicKey { #[cfg(test)] mod test { - use alloy::primitives::keccak256; + use alloy::primitives::{U256, keccak256}; use super::{ create_admin_jwt, create_jwt, decode_admin_jwt, decode_jwt, ms_into_slot, @@ -691,4 +716,26 @@ mod test { // Two calls should produce distinct values with overwhelming probability. assert_ne!(secret, random_jwt_secret()); } + + #[test] + fn eth_amounts_parse_exactly_and_refuse_non_finite_or_negative() { + #[derive(serde::Deserialize)] + struct Amount { + #[serde(with = "super::as_eth_str")] + eth: U256, + } + let parse = |toml: &str| toml::from_str::(toml).map(|amount| amount.eth); + for (toml, wei) in [ + ("eth = 0.009", 9_000_000_000_000_000u64), + ("eth = \"0.009\"", 9_000_000_000_000_000), + ("eth = 0.0157", 15_700_000_000_000_000), + ("eth = 0.0021", 2_100_000_000_000_000), + ("eth = 1", 1_000_000_000_000_000_000), + ] { + assert_eq!(parse(toml).unwrap(), U256::from(wei), "{toml}"); + } + for toml in ["eth = -0.1", "eth = \"-0.1\"", "eth = nan", "eth = inf"] { + assert!(parse(toml).is_err(), "{toml}"); + } + } } diff --git a/crates/km/Cargo.toml b/crates/km/Cargo.toml new file mode 100644 index 000000000..803f98b34 --- /dev/null +++ b/crates/km/Cargo.toml @@ -0,0 +1,24 @@ +[package] +edition.workspace = true +name = "cb-km" +publish = false +rust-version.workspace = true +version.workspace = true + +[dependencies] +alloy.workspace = true +cb-common.workspace = true +clap.workspace = true +eyre.workspace = true +reqwest.workspace = true +serde.workspace = true +serde_json.workspace = true +tokio.workspace = true +toml.workspace = true +tracing.workspace = true +tracing-subscriber.workspace = true +url.workspace = true + +[dev-dependencies] +axum.workspace = true +tempfile.workspace = true diff --git a/crates/km/src/apply.rs b/crates/km/src/apply.rs new file mode 100644 index 000000000..c6df64b9a --- /dev/null +++ b/crates/km/src/apply.rs @@ -0,0 +1,355 @@ +//! `builder-config apply`: writes the builder config of every key a validator +//! client holds, a mux key's from its mux and any other key's from +//! `[[relays]]`. + +use std::{ + collections::{BTreeMap, BTreeSet}, + fmt::Display, +}; + +use eyre::{Result, ensure}; +use url::Url; + +use crate::{ + client::{KmClient, SetOutcome}, + doc::{BuilderConfig, MAX_BUILDER_ENTRIES}, + output, + project::Projection, + targets::{Targets, VcConfig, is_loopback}, +}; + +/// Lodestar's refusal of a nonzero `max_execution_payment` names this flag +pub const LODESTAR_CAP_FLAG: &str = "--allowDangerousTrustedPayments"; + +/// Writes in a row a client may leave unanswered before apply stops writing to +/// it, since each waits for the request timeout. A `--preserve-entries` read +/// that gets no answer counts as its key's write +const MAX_UNANSWERED: usize = 3; + +#[derive(Default)] +pub struct ApplyOptions { + pub preserve_entries: bool, + /// The clients given are not all that hold the muxes' keys, so a key none + /// of them holds is counted rather than an error + pub partial: bool, + /// Print each error, warning and write as it is recorded, and each client's + /// line + pub print: bool, +} + +#[derive(Debug, Default)] +pub struct ApplyReport { + pub errors: Vec, + pub warnings: Vec, + /// Key -> the clients that accepted it + pub accepted: BTreeMap>, + /// Writes a listed client was due but did not get + pub unwritten: usize, + print: bool, +} + +impl ApplyReport { + fn error(&mut self, msg: impl Into) { + let msg = msg.into(); + if self.print { + output::err(format_args!("ERROR: {msg}")); + } + self.errors.push(msg); + } + + fn warn(&mut self, msg: impl Into) { + let msg = msg.into(); + if self.print { + output::out(format_args!("WARN: {msg}")); + } + self.warnings.push(msg); + } + + fn accept(&mut self, key: &str, vc: &str, mux: Option<&String>) { + if self.print { + match mux { + Some(id) => output::out(format_args!("accepted: {key} on {vc} (mux {id})")), + None => output::out(format_args!("accepted: {key} on {vc} ([[relays]])")), + } + } + self.accepted.entry(key.to_string()).or_default().push(vc.to_string()); + } +} + +/// Each of `keys` on its own indented line, to append to a message +fn key_lines(keys: impl IntoIterator) -> String { + keys.into_iter().map(|key| format!("\n {key}")).collect() +} + +/// Fails only before contacting a validator client; every later failure is on +/// the report +pub async fn run_apply( + projection: &Projection, + targets: &Targets, + opts: &ApplyOptions, +) -> Result { + ensure!( + !targets.vcs.is_empty(), + "no validator client given: pass --vc =" + ); + let mut report = ApplyReport { print: opts.print, ..Default::default() }; + + let advertised = Url::parse(&targets.advertised_url)?; + // client -> the keys it lists + let mut listings: BTreeMap<&str, BTreeSet> = BTreeMap::new(); + for vc in &targets.vcs { + if let Some(keys) = apply_to_vc(vc, projection, &advertised, opts, &mut report).await { + listings.insert(vc.url.as_str(), keys); + } + } + // key -> the clients that list it, in `--vc` order + let mut listed: BTreeMap<&str, Vec<&str>> = BTreeMap::new(); + let mut unlisted = Vec::new(); + for vc in &targets.vcs { + match listings.get(vc.url.as_str()) { + Some(keys) => { + for key in keys { + listed.entry(key).or_default().push(vc.url.as_str()); + } + } + None => unlisted.push(vc.url.as_str()), + } + } + // A client that could not be listed may hold any key no other client lists + let may_hold = if unlisted.is_empty() { + String::new() + } else { + format!("; {} could not be listed and may hold some", unlisted.join(", ")) + }; + + if listed.is_empty() && unlisted.is_empty() { + // A sidecar can run before its client has keys + if opts.partial { + report.warn("no validator client given lists a key"); + } else { + report.error("no validator client lists a key"); + } + } + if projection.relays_doc.is_none() { + let outside = listed.keys().filter(|key| !projection.mux_docs.contains_key(**key)).count(); + if outside > 0 { + report.warn(format!("keys in no mux, with no [[relays]] to write for them: {outside}")); + } + } + // A loader lists keys no client of this operator holds, such as exited ones + let (fetched, named): (Vec<&String>, Vec<&String>) = projection + .mux_docs + .keys() + .filter(|key| !listed.contains_key(key.as_str())) + .partition(|key| projection.fetched_keys.contains(*key)); + if !named.is_empty() { + let count = named.len(); + if opts.partial { + report.warn(format!("keys in a mux that no validator client given holds: {count}")); + } else { + report.error(format!( + "keys in a mux that no validator client lists: {count}{may_hold}{}", + key_lines(named) + )); + } + } + if !fetched.is_empty() { + report.warn(format!( + "keys a URL or registry loader lists that no validator client holds: {}{may_hold}", + fetched.len() + )); + } + // holders -> the keys they all list + let mut shared: BTreeMap<&[&str], Vec<&str>> = BTreeMap::new(); + for (key, holders) in listed.iter().filter(|(_, holders)| holders.len() > 1) { + shared.entry(holders.as_slice()).or_default().push(key); + } + for (holders, keys) in shared { + report.error(format!( + "keys held by more than one validator client ({}): {}; each is a slashing risk, so \ + keep it on one{}", + holders.join(", "), + keys.len(), + key_lines(keys) + )); + } + + Ok(report) +} + +/// A request that got no answer, rather than a refusal +fn is_unanswered(err: &eyre::Report) -> bool { + err.downcast_ref::().is_some_and(|err| err.is_timeout() || err.is_request()) +} + +/// `run` after a key's last request: one more if it got no answer, else 0 +fn next_unanswered(run: usize, err: Option<&eyre::Report>) -> usize { + if err.is_some_and(is_unanswered) { run + 1 } else { 0 } +} + +/// Writes the projection to one client and returns the keys it lists, or `None` +/// when they could not be listed. A failure is recorded on the report and stops +/// only this client. +async fn apply_to_vc( + vc: &VcConfig, + projection: &Projection, + advertised: &Url, + opts: &ApplyOptions, + report: &mut ApplyReport, +) -> Option> { + let name = vc.url.as_str(); + if vc.url.scheme() != "https" && !is_loopback(&vc.url) { + report.warn(format!( + "{name}: not HTTPS and not loopback, so the bearer token is sent in cleartext" + )); + } + let client = match KmClient::from_token_file(vc.url.clone(), &vc.token_path) { + Ok(client) => client, + Err(err) => { + report.error(format!("{name}: setup failed: {err:#}")); + return None; + } + }; + let listed = match client.list_keys().await { + Ok(keys) => keys, + Err(err) => { + report.error(format!("{name}: key listing failed: {err:#}")); + return None; + } + }; + let to_write = listed.iter().filter(|key| projection.doc_for(key).is_some()).count(); + + // A 404 for a key the client lists means the route is missing + let Some(probe) = listed.first() else { + report.warn(format!("{name}: lists no validator keys")); + return Some(listed); + }; + let failure = match client.get_builder_config(probe).await { + Ok(Some(_)) => None, + Ok(None) => Some("no builder_config support (keymanager-APIs #88)".to_string()), + Err(err) => Some(format!("builder_config probe failed: {err:#}")), + }; + if let Some(failure) = failure { + report.error(format!("{name}: {failure}; none of its {to_write} keys written")); + report.unwritten += to_write; + return Some(listed); + } + + // Set once the client refuses a cap above 0, as Lodestar does without its flag + let mut cap_refused = false; + let (mut written, mut unanswered) = (0, 0); + // Only the keys the client lists: some clients accept a write for any key + for (i, key) in listed.iter().enumerate() { + if unanswered == MAX_UNANSWERED { + let left = listed + .iter() + .skip(i) + .filter(|key| { + projection + .doc_for(key) + .is_some_and(|doc| !(cap_refused && doc.has_nonzero_cap())) + }) + .count(); + if left > 0 { + report.error(format!( + "{name}: {MAX_UNANSWERED} writes in a row got no answer, so its other {left} \ + keys were not written" + )); + } + break; + } + let Some(doc) = projection.doc_for(key) else { continue }; + let merged; + let doc = if opts.preserve_entries { + match client.get_builder_config(key).await { + Ok(Some(stored)) => { + merged = merge_preserved_entries(doc, &stored, advertised); + let entries = merged.builders.as_ref().map_or(0, Vec::len); + if entries > MAX_BUILDER_ENTRIES { + report.error(format!( + "{name}: {key} would keep {entries} builder entries, over the \ + {MAX_BUILDER_ENTRIES} a builder config holds" + )); + continue; + } + &merged + } + Ok(None) => doc, + Err(err) => { + unanswered = next_unanswered(unanswered, Some(&err)); + report.error(format!("{name}: preserve-entries GET for {key} failed: {err:#}")); + continue; + } + } + } else { + doc + }; + if cap_refused && doc.has_nonzero_cap() { + continue; + } + + let result = client.set_builder_config(key, doc).await; + unanswered = next_unanswered(unanswered, result.as_ref().err()); + match result { + Ok(SetOutcome::Accepted) => { + report.accept(key, name, projection.mux_ids.get(key)); + written += 1; + } + Ok(SetOutcome::KeyNotFound) => { + report.error(format!("{name}: lists {key} but answered 404 to its write")) + } + // A 403 before any write lands refuses every write, as Lodestar does + // under --proposerSettingsFile + Ok(SetOutcome::Forbidden(message)) if written == 0 => { + report.error(format!( + "{name}: answered 403 before any write landed ({message}), as under a \ + proposer settings file; no further key was written to it" + )); + break; + } + Ok(SetOutcome::Forbidden(message)) => { + report.error(format!("{name}: POST {key} refused: {message}")) + } + // Lodestar refuses every capped write alike, so one error says it all + Err(err) if format!("{err:#}").contains(LODESTAR_CAP_FLAG) => { + report.error(format!( + "{name}: refuses a max_execution_payment above 0 unless the validator client \ + runs with {LODESTAR_CAP_FLAG}; restart it with that flag. No further key \ + with a cap above 0 was written to it: {err:#}" + )); + cap_refused = true; + } + Err(err) => report.error(format!("{name}: POST {key} failed: {err:#}")), + } + } + + report.unwritten += to_write - written; + if opts.print { + let unwritten = to_write - written; + let unwritten = + if unwritten > 0 { format!(", {unwritten} not written") } else { String::new() }; + output::out(format_args!( + "{name}: {} keys listed, {written} written{unwritten}", + listed.len() + )); + } + Some(listed) +} + +/// The projection plus the stored entries at other URLs. Stored entries at the +/// advertised URL, this tool's or global builders the GET resolved, are dropped +fn merge_preserved_entries( + projected: &BuilderConfig, + stored: &BuilderConfig, + advertised: &Url, +) -> BuilderConfig { + let mut merged = projected.clone(); + let others = stored.builders.iter().flatten().filter(|entry| !same_url(&entry.url, advertised)); + merged.builders.get_or_insert_default().extend(others.cloned()); + merged +} + +/// Equal once parsed, so a trailing slash does not hide a match +fn same_url(url: &str, other: &Url) -> bool { + Url::parse(url).is_ok_and(|url| url == *other) +} diff --git a/crates/km/src/cli.rs b/crates/km/src/cli.rs new file mode 100644 index 000000000..298578538 --- /dev/null +++ b/crates/km/src/cli.rs @@ -0,0 +1,238 @@ +//! `commit-boost builder-config`: prints each validator key's keymanager +//! builder config, or writes it to validator clients. + +use std::{ + collections::{BTreeMap, BTreeSet}, + io::{IsTerminal, Read, Write}, + path::{Path, PathBuf}, + process::ExitCode, + sync::atomic::{AtomicUsize, Ordering}, +}; + +use clap::{Args, Subcommand}; +use eyre::{Context, Result, ensure}; +use tracing::{Event, Level, Subscriber}; +use tracing_subscriber::{ + EnvFilter, Layer, + filter::LevelFilter, + layer::{Context as LayerContext, SubscriberExt}, + util::SubscriberInitExt, +}; + +use crate::{ + apply::{ApplyOptions, LODESTAR_CAP_FLAG, run_apply}, + output, + printed::Printed, + project::{Projection, mux_keys, parse_config, project}, + targets::{Targets, VcConfig, check_advertised_url}, +}; + +#[derive(Args, Debug)] +pub struct BuilderConfigArgs { + #[command(subcommand)] + command: Command, +} + +#[derive(Subcommand, Debug)] +enum Command { + /// Print each key's builder config as JSON, contacting no validator client + Print { + /// Commit-Boost config TOML + #[arg(long, env = "CB_CONFIG")] + config: PathBuf, + /// Commit-Boost's URL as the beacon nodes reach it, written into + /// every entry + #[arg(long)] + advertised_url: String, + }, + /// Write the builder config of every key the validator clients hold + Apply { + /// Commit-Boost config TOML + #[arg(long, env = "CB_CONFIG")] + config: Option, + /// A document `print` wrote, or `-` for stdin, in place of the + /// Commit-Boost config + #[arg(long, value_name = "FILE")] + from: Option, + /// Commit-Boost's URL as the beacon nodes reach it, written into + /// every entry + #[arg(long)] + advertised_url: String, + /// A validator client to write to, as `=`; repeat for each + #[arg(long = "vc", value_name = "URL=TOKEN_FILE")] + vcs: Vec, + /// Keep each stored builder entry at a URL other than the advertised + /// one, which a write otherwise erases + #[arg(long)] + preserve_entries: bool, + /// The validator clients given are only some of those holding the + /// muxes' keys, such as one pod's: count a mux key none of them holds + /// instead of failing on it + #[arg(long)] + partial: bool, + }, +} + +fn read(path: &Path) -> Result { + std::fs::read_to_string(path).wrap_err_with(|| format!("unable to read {path:?}")) +} + +/// Warnings the loaders log, such as a fallback to the SSV public API, for the +/// closing tally +static LOADER_WARNINGS: AtomicUsize = AtomicUsize::new(0); + +struct CountWarnings; + +impl Layer for CountWarnings { + fn on_event(&self, event: &Event<'_>, _: LayerContext<'_, S>) { + if *event.metadata().level() <= Level::WARN { + LOADER_WARNINGS.fetch_add(1, Ordering::Relaxed); + } + } +} + +/// The loaders' own warnings go to stderr. `RUST_LOG` adds directives but +/// cannot hide these, which one set for another program would +fn init_logging() { + // A bare level replaces `warn` rather than adding to it, so one below it, + // such as `error` or `off`, is dropped + let rust_log = std::env::var("RUST_LOG").unwrap_or_default(); + let quieter = |directive: &str| { + directive.trim().parse::().is_ok_and(|level| level < LevelFilter::WARN) + }; + let directives: Vec<&str> = + rust_log.split(',').filter(|directive| !quieter(directive)).collect(); + let filter = format!("warn,{}", directives.join(",")); + tracing_subscriber::registry() + .with(EnvFilter::builder().parse_lossy(filter)) + .with( + tracing_subscriber::fmt::layer() + .with_writer(std::io::stderr) + .with_ansi(std::io::stderr().is_terminal()) + .with_target(false) + .without_time(), + ) + .with(CountWarnings) + .init(); +} + +/// Runs `builder-config`: 0 ok, 1 an error once the config is read, 2 stopped +/// before contacting any validator client +pub async fn run(args: BuilderConfigArgs) -> ExitCode { + init_logging(); + let result = match args.command { + Command::Print { config, advertised_url } => print(&config, &advertised_url).await, + Command::Apply { config, from, advertised_url, vcs, preserve_entries, partial } => { + apply(config, from, advertised_url, vcs, ApplyOptions { + preserve_entries, + partial, + print: true, + }) + .await + } + }; + match result { + Ok(code) => code, + // Both fail only before a validator client is contacted + Err(err) => { + output::err(format_args!("ERROR: {err:#}")); + ExitCode::from(2) + } + } +} + +async fn resolve(config: &Path, advertised_url: &str) -> Result { + let cfg = parse_config(&read(config)?)?; + let projection = project(&cfg, &mux_keys(&cfg).await?, advertised_url)?; + ensure!( + !projection.mux_docs.is_empty() || projection.relays_doc.is_some(), + "nothing to write: the config has no mux keys and no [[relays]]" + ); + Ok(projection) +} + +/// Each mux's key count, to show a stale or wrong keys file +fn mux_counts(projection: &Projection) -> BTreeMap<&str, usize> { + let mut counts = BTreeMap::new(); + for id in projection.mux_ids.values() { + *counts.entry(id.as_str()).or_default() += 1; + } + counts +} + +/// stdout carries only the document, so its counts and notes go to stderr +async fn print(config: &Path, advertised_url: &str) -> Result { + check_advertised_url(advertised_url)?; + let projection = resolve(config, advertised_url).await?; + for (id, count) in mux_counts(&projection) { + output::err(format_args!("mux {id}: {count} keys")); + } + if projection.has_nonzero_cap() { + output::err(format_args!( + "NOTE: the builder config sets a max_execution_payment above 0, which a Lodestar \ + validator client refuses unless it runs with {LODESTAR_CAP_FLAG}" + )); + } + let printed = Printed::new(&projection, advertised_url); + let mut stdout = std::io::stdout().lock(); + let written = serde_json::to_writer_pretty(&mut stdout, &printed) + .map_err(std::io::Error::from) + .and_then(|()| writeln!(stdout)) + .and_then(|()| stdout.flush()); + if let Err(err) = written { + output::err(format_args!("ERROR: could not write the document: {err}")); + return Ok(ExitCode::from(1)); + } + Ok(ExitCode::SUCCESS) +} + +/// The printed document at `from`, or on stdin for `-` +fn read_printed(from: &Path) -> Result { + let text = if from == Path::new("-") { + ensure!( + !std::io::stdin().is_terminal(), + "--from -: stdin is a terminal; pipe in the document `print` wrote" + ); + let mut text = String::new(); + std::io::stdin().read_to_string(&mut text).wrap_err("unable to read stdin")?; + text + } else { + read(from)? + }; + Printed::parse(&text).wrap_err_with(|| format!("{from:?} is not a printed document")) +} + +async fn apply( + config: Option, + from: Option, + advertised_url: String, + vcs: Vec, + opts: ApplyOptions, +) -> Result { + ensure!(!vcs.is_empty(), "no validator client given: pass --vc ="); + let targets = Targets::new(advertised_url, vcs)?; + targets.check_token_files()?; + // `--config` may come from CB_CONFIG, so `--from` wins over it + let projection = match (from, config) { + (Some(from), _) => read_printed(&from)?.into_projection(&targets.advertised_url)?, + (None, Some(config)) => resolve(&config, &targets.advertised_url).await?, + (None, None) => eyre::bail!("pass --config, set CB_CONFIG, or pass --from"), + }; + for (id, count) in mux_counts(&projection) { + output::out(format_args!("mux {id}: {count} keys")); + } + + let report = run_apply(&projection, &targets, &opts).await?; + let written: usize = report.accepted.values().map(Vec::len).sum(); + let clients = report.accepted.values().flatten().collect::>().len(); + output::out(format_args!( + "done: {written} keys written, {} not written, on {clients} of {} validator clients; {} \ + errors, {} warnings", + report.unwritten, + targets.vcs.len(), + report.errors.len(), + report.warnings.len() + LOADER_WARNINGS.load(Ordering::Relaxed) + )); + Ok(if report.errors.is_empty() { ExitCode::SUCCESS } else { ExitCode::from(1) }) +} diff --git a/crates/km/src/client.rs b/crates/km/src/client.rs new file mode 100644 index 000000000..54291a011 --- /dev/null +++ b/crates/km/src/client.rs @@ -0,0 +1,173 @@ +//! Keymanager API client: key listing and the builder_config endpoints +//! (keymanager-APIs #88), with bearer-token auth. + +use std::{collections::BTreeSet, path::Path, time::Duration}; + +use eyre::{Context, Result, bail, ensure}; +use serde::Deserialize; +use url::Url; + +use crate::doc::BuilderConfig; + +const HTTP_TIMEOUT: Duration = Duration::from_secs(30); + +#[derive(Deserialize)] +struct Data { + data: T, +} + +#[derive(Deserialize)] +struct KeystoreEntry { + validating_pubkey: String, +} + +#[derive(Deserialize)] +struct SignerDefinition { + pubkey: String, +} + +#[derive(Deserialize)] +struct ErrorResponse { + message: String, +} + +/// A failed response's status, with the keymanager error `message` when the +/// body has one +async fn failure(resp: reqwest::Response) -> String { + let status = resp.status(); + resp.json::() + .await + .map_or_else(|_| status.to_string(), |body| format!("{status}: {:?}", body.message)) +} + +#[derive(Debug)] +pub enum SetOutcome { + Accepted, + KeyNotFound, + Forbidden(String), +} + +pub struct KmClient { + http: reqwest::Client, + base: Url, + token: String, +} + +/// A keymanager API token, read from its file +pub fn read_token(path: &Path) -> Result { + // The shell expands `~` only at the start of a word, not after `--vc URL=` + let hint = if path.starts_with("~") { " (use $HOME, not ~, after =)" } else { "" }; + let token = std::fs::read_to_string(path) + .wrap_err_with(|| format!("unable to read token file {path:?}{hint}"))?; + let token = token.trim(); + ensure!(!token.is_empty(), "token file {path:?} is empty"); + // An older Prysm token file holds a JWT secret, then the token + ensure!(!token.contains('\n'), "token file {path:?} has more than one line: regenerate it"); + Ok(token.to_string()) +} + +/// `Url::join` with an absolute path would drop a base path prefix +fn endpoint(base: &Url, path: &str) -> Result { + let base = base.as_str().trim_end_matches('/'); + Url::parse(&format!("{base}{path}")).wrap_err_with(|| format!("invalid endpoint {path}")) +} + +impl KmClient { + pub fn from_token_file(base: Url, token_path: &Path) -> Result { + let token = read_token(token_path)?; + let http = reqwest::Client::builder() + .timeout(HTTP_TIMEOUT) + .redirect(reqwest::redirect::Policy::none()) + .build()?; + Ok(Self { http, base, token }) + } + + fn endpoint(&self, path: &str) -> Result { + endpoint(&self.base, path) + } + + /// The client's validator keys, lowercased and each once: its keystores, + /// plus its remote-signer keys where it serves that route. A client may + /// list a remote-signer key in both + pub async fn list_keys(&self) -> Result> { + let url = self.endpoint("/eth/v1/keystores")?; + let resp = self.http.get(url).bearer_auth(&self.token).send().await?; + if !resp.status().is_success() { + bail!("keystores: {}", failure(resp).await); + } + let keystores: Data> = + resp.json().await.wrap_err("invalid keystores response")?; + let mut keys: BTreeSet = + keystores.data.into_iter().map(|k| k.validating_pubkey.to_lowercase()).collect(); + + let url = self.endpoint("/eth/v1/remotekeys")?; + let resp = self.http.get(url).bearer_auth(&self.token).send().await?; + match resp.status().as_u16() { + 200 => { + let remote: Data> = + resp.json().await.wrap_err("invalid remotekeys response")?; + keys.extend(remote.data.into_iter().map(|k| k.pubkey.to_lowercase())); + } + // a client without remote signing does not serve the route + 404 => {} + _ => bail!("remotekeys: {}", failure(resp).await), + } + Ok(keys) + } + + /// The key's builder config, or `None` on a 404 + pub async fn get_builder_config(&self, pubkey: &str) -> Result> { + let url = self.endpoint(&format!("/eth/v1/validator/{pubkey}/builder_config"))?; + let resp = self.http.get(url).bearer_auth(&self.token).send().await?; + match resp.status().as_u16() { + 200 => { + let body: Data = + resp.json().await.wrap_err("invalid builder_config response")?; + Ok(Some(body.data)) + } + 404 => Ok(None), + _ => bail!("{}", failure(resp).await), + } + } + + pub async fn set_builder_config( + &self, + pubkey: &str, + doc: &BuilderConfig, + ) -> Result { + let url = self.endpoint(&format!("/eth/v1/validator/{pubkey}/builder_config"))?; + let resp = self.http.post(url).bearer_auth(&self.token).json(doc).send().await?; + match resp.status().as_u16() { + 202 => Ok(SetOutcome::Accepted), + 404 => Ok(SetOutcome::KeyNotFound), + 403 => Ok(SetOutcome::Forbidden(failure(resp).await)), + _ => bail!("{}", failure(resp).await), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn endpoint_keeps_the_base_path() { + for base in ["https://vc.example/prefix", "https://vc.example/prefix/"] { + let url = endpoint(&Url::parse(base).unwrap(), "/eth/v1/keystores").unwrap(); + assert_eq!(url.as_str(), "https://vc.example/prefix/eth/v1/keystores"); + } + } + + // Only a path starting with `~`, which the shell leaves as is after + // `--vc URL=`, gets the $HOME hint; a token file of whitespace is empty + #[test] + fn read_token_errors() { + let err = |path: &Path| format!("{:#}", read_token(path).unwrap_err()); + assert!(!err(Path::new("/nonexistent/token")).contains("$HOME")); + let blank = tempfile::NamedTempFile::new().unwrap(); + std::fs::write(blank.path(), " \n").unwrap(); + assert!(err(blank.path()).contains("is empty")); + std::fs::write(blank.path(), "abcd\nef01\n").unwrap(); + assert!(err(blank.path()).contains("has more than one line")); + } +} diff --git a/crates/km/src/doc.rs b/crates/km/src/doc.rs new file mode 100644 index 000000000..445b6ca11 --- /dev/null +++ b/crates/km/src/doc.rs @@ -0,0 +1,44 @@ +//! The keymanager builder config document (keymanager-APIs +//! `types/builder_entry.yaml`): Uint64s are JSON strings and `auth_data` is +//! 0x-prefixed hex. + +use serde::{Deserialize, Serialize}; + +/// The most entries a key's builder config can hold +pub const MAX_BUILDER_ENTRIES: usize = 64; + +#[derive(Debug, Clone, Default, Serialize, Deserialize)] +pub struct BuilderConfig { + #[serde(skip_serializing_if = "Option::is_none")] + pub min_bid: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub builder_boost_factor: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub builders: Option>, +} + +impl BuilderConfig { + /// Whether any entry has a cap above 0, which Lodestar refuses without a + /// flag + pub fn has_nonzero_cap(&self) -> bool { + self.builders + .iter() + .flatten() + .any(|entry| entry.max_execution_payment.as_deref() != Some("0")) + } +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct BuilderEntry { + pub url: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub auth_data: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub builder_pubkeys: Option>, + #[serde(skip_serializing_if = "Option::is_none")] + pub max_execution_payment: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub min_bid: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub builder_boost_factor: Option, +} diff --git a/crates/km/src/lib.rs b/crates/km/src/lib.rs new file mode 100644 index 000000000..ec02f020b --- /dev/null +++ b/crates/km/src/lib.rs @@ -0,0 +1,12 @@ +//! `commit-boost builder-config`: validators' keymanager builder config +//! (keymanager-APIs #88) from a Commit-Boost config. + +pub mod apply; +pub mod cli; +mod client; +pub mod doc; +pub mod output; +pub mod printed; +pub mod project; +pub mod targets; +mod unknown_keys; diff --git a/crates/km/src/output.rs b/crates/km/src/output.rs new file mode 100644 index 000000000..387feb0f7 --- /dev/null +++ b/crates/km/src/output.rs @@ -0,0 +1,12 @@ +//! Output that a closed pipe cannot abort: `println!` panics on a write error, +//! which would stop a run between two keys + +use std::{fmt::Display, io::Write}; + +pub fn out(line: impl Display) { + let _ = writeln!(std::io::stdout().lock(), "{line}"); +} + +pub fn err(line: impl Display) { + let _ = writeln!(std::io::stderr().lock(), "{line}"); +} diff --git a/crates/km/src/printed.rs b/crates/km/src/printed.rs new file mode 100644 index 000000000..96dd8ff6b --- /dev/null +++ b/crates/km/src/printed.rs @@ -0,0 +1,132 @@ +//! The document `builder-config print` writes and `apply --from` reads: each +//! mux's builder config once, with its keys, and the `[[relays]]` config for +//! every other key. Every config is a keymanager POST body as is. + +use std::collections::{BTreeMap, BTreeSet}; + +use alloy::primitives::hex; +use cb_common::utils::bls_pubkey_from_hex; +use eyre::{Result, bail, ensure}; +use serde::{Deserialize, Serialize}; + +use crate::{doc::BuilderConfig, project::Projection}; + +pub const VERSION: u32 = 1; + +#[derive(Debug, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct Printed { + pub version: u32, + pub advertised_url: String, + /// The config of every key in no mux; `null` with no `[[relays]]`, so other + /// keys are left alone + pub default: Option, + pub muxes: BTreeMap, +} + +#[derive(Debug, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct PrintedMux { + pub config: BuilderConfig, + /// Keys the Commit-Boost config names + pub keys: Vec, + /// Keys only a URL or registry loader lists, which can change after the + /// print + pub fetched_keys: Vec, +} + +impl Printed { + pub fn new(projection: &Projection, advertised_url: &str) -> Self { + let mut muxes: BTreeMap = BTreeMap::new(); + for (key, id) in &projection.mux_ids { + let mux = muxes.entry(id.clone()).or_insert_with(|| PrintedMux { + config: projection.mux_docs[key].clone(), + keys: vec![], + fetched_keys: vec![], + }); + if projection.fetched_keys.contains(key) { + mux.fetched_keys.push(key.clone()); + } else { + mux.keys.push(key.clone()); + } + } + Self { + version: VERSION, + advertised_url: advertised_url.to_string(), + default: projection.relays_doc.clone(), + muxes, + } + } + + /// Reads a document as `print` writes it. A field `print` does not write, + /// such as a misspelled cap, would be dropped from each POST and the + /// client's default applied + pub fn parse(text: &str) -> Result { + let value: serde_json::Value = serde_json::from_str(text)?; + let printed: Self = serde_json::from_value(value.clone())?; + ensure!( + serde_json::to_value(&printed)? == value, + "it holds a field `print` does not write" + ); + Ok(printed) + } + + /// The projection `apply` writes, once the document is checked against the + /// advertised URL apply was given + pub fn into_projection(self, advertised_url: &str) -> Result { + ensure!( + self.version == VERSION, + "the printed document is version {}, and this commit-boost reads version {VERSION}", + self.version + ); + // Each entry's url is written as is, so it must be the one checked + ensure!( + self.advertised_url == advertised_url, + "the printed document is for {}, not --advertised-url {advertised_url}", + self.advertised_url + ); + for (name, config) in self + .muxes + .iter() + .map(|(id, mux)| (id.as_str(), &mux.config)) + .chain(self.default.as_ref().map(|config| ("default", config))) + { + for entry in config.builders.iter().flatten() { + ensure!( + entry.url == advertised_url, + "{name}'s config has an entry at {}, not {advertised_url}", + entry.url + ); + if let Some(auth_data) = &entry.auth_data { + ensure!( + auth_data.starts_with("0x") && hex::decode(auth_data).is_ok(), + "{name}'s config has auth_data {auth_data}, not 0x-prefixed hex" + ); + } + } + } + + let mut projection = Projection { + mux_docs: BTreeMap::new(), + fetched_keys: BTreeSet::new(), + relays_doc: self.default, + mux_ids: BTreeMap::new(), + }; + for (id, mux) in self.muxes { + let keys = mux.keys.iter().map(|key| (key, false)); + for (key, fetched) in keys.chain(mux.fetched_keys.iter().map(|key| (key, true))) { + let key = bls_pubkey_from_hex(key) + .map_err(|err| eyre::eyre!("mux {id}: {key} is not a validator key: {err}"))? + .as_hex_string(); + if let Some(other) = projection.mux_ids.insert(key.clone(), id.clone()) { + bail!("{key} is in both mux {other} and mux {id}"); + } + if fetched { + projection.fetched_keys.insert(key.clone()); + } + projection.mux_docs.insert(key, mux.config.clone()); + } + } + Ok(projection) + } +} diff --git a/crates/km/src/project.rs b/crates/km/src/project.rs new file mode 100644 index 000000000..438029a6d --- /dev/null +++ b/crates/km/src/project.rs @@ -0,0 +1,600 @@ +//! Projects a Commit-Boost config into each validator key's builder config, +//! routing a key as Commit-Boost does: a mux key to its mux's relays, any other +//! key to `[[relays]]`. +//! +//! Each relay hostname gets one entry, at Commit-Boost's URL, with the hostname +//! as its auth data, so relays on one host share an entry. `builder_pubkeys` is +//! empty, accepting any builder: the pubkey in a relay URL is the relay's, not +//! the builder's bid-signing key. + +use std::collections::{BTreeMap, BTreeSet}; + +use alloy::primitives::{U256, hex, utils::Unit}; +use cb_common::{ + config::{ + CommitBoostConfig, ExecutionPaymentCap, MuxKeysLoader, PbsConfig, RelayConfig, + check_mux_tables, + }, + types::BlsPublicKey, +}; +use eyre::{Context, Result, ensure}; + +use crate::{ + doc::{BuilderConfig, BuilderEntry, MAX_BUILDER_ENTRIES}, + unknown_keys::unknown_keys, +}; + +/// Unset, a bid counts at its value against the local block's +const BUILDER_BOOST_FACTOR: u64 = 100; + +pub fn parse_config(text: &str) -> Result { + let raw: toml::Value = toml::from_str(text).wrap_err("could not parse Commit-Boost config")?; + check_mux_tables(&raw)?; + let unknown = unknown_keys(&raw); + ensure!(unknown.is_empty(), "unknown keys in the Commit-Boost config: {}", unknown.join(", ")); + toml::from_str(text).wrap_err("could not parse Commit-Boost config") +} + +/// Each mux's keys, resolved as Commit-Boost resolves them at startup, file, +/// URL and registry loaders included +pub async fn mux_keys(cfg: &CommitBoostConfig) -> Result>> { + let Some(mut muxes) = cfg.muxes.clone() else { return Ok(BTreeMap::new()) }; + let mut ids = BTreeSet::new(); + for mux in &mut muxes.muxes { + // Keys are grouped by mux id below, so two muxes with one id would merge + ensure!(ids.insert(mux.id.clone()), "mux id {} names more than one [[mux]]", mux.id); + // builder-config never contacts a relay, so it needs none of their header + // secrets + for relay in &mut mux.relays { + relay.headers = None; + } + } + // builder-config reads no bids, so it needs rpc_url only for a registry loader + let mut pbs = cfg.pbs.pbs_config.clone(); + if !muxes.muxes.iter().any(|mux| matches!(mux.loader, Some(MuxKeysLoader::Registry { .. }))) { + pbs.rpc_url = None; + pbs.extra_validation_enabled = false; + } + let (lookup, _) = muxes + .validate_and_fill(cfg.chain, &pbs) + .await + .wrap_err("could not resolve the mux keys as Commit-Boost does at startup")?; + let mut keys: BTreeMap> = BTreeMap::new(); + for (key, mux) in lookup { + keys.entry(mux.id).or_default().push(key); + } + Ok(keys) +} + +#[derive(Debug)] +pub struct Projection { + /// Builder config by mux key, in 0x-hex + pub mux_docs: BTreeMap, + /// Mux keys only a URL or registry loader lists, which the config does not + /// name + pub fetched_keys: BTreeSet, + /// The builder config of any other key, from `[[relays]]`; none when it is + /// empty + pub relays_doc: Option, + /// Mux key -> its mux's id + pub mux_ids: BTreeMap, +} + +impl Projection { + /// The builder config apply writes for `key` + pub fn doc_for(&self, key: &str) -> Option<&BuilderConfig> { + self.mux_docs.get(key).or(self.relays_doc.as_ref()) + } + + /// Whether any entry has a cap above 0, which Lodestar refuses without a + /// flag + pub fn has_nonzero_cap(&self) -> bool { + self.mux_docs.values().chain(&self.relays_doc).any(BuilderConfig::has_nonzero_cap) + } +} + +pub fn project( + cfg: &CommitBoostConfig, + mux_keys: &BTreeMap>, + advertised_url: &str, +) -> Result { + let pbs = &cfg.pbs.pbs_config; + let mut mux_docs = BTreeMap::new(); + let mut fetched_keys = BTreeSet::new(); + let mut mux_ids = BTreeMap::new(); + for mux in cfg.muxes.iter().flat_map(|muxes| &muxes.muxes) { + let doc = project_relays( + pbs, + &mux.relays, + mux.min_bid_wei, + mux.builder_boost_factor, + advertised_url, + ) + .wrap_err_with(|| format!("mux {}", mux.id))?; + let fetches = + matches!(mux.loader, Some(MuxKeysLoader::HTTP { .. } | MuxKeysLoader::Registry { .. })); + for key in mux_keys.get(&mux.id).into_iter().flatten() { + let hex = key.as_hex_string(); + if fetches && !mux.validator_pubkeys.contains(key) { + fetched_keys.insert(hex.clone()); + } + mux_docs.insert(hex.clone(), doc.clone()); + mux_ids.insert(hex, mux.id.clone()); + } + } + let relays_doc = match cfg.relays.as_slice() { + [] => None, + relays => { + Some(project_relays(pbs, relays, None, None, advertised_url).wrap_err("[[relays]]")?) + } + }; + Ok(Projection { mux_docs, fetched_keys, relays_doc, mux_ids }) +} + +/// One key's builder config for `relays`, with a mux's `min_bid_wei` and +/// `builder_boost_factor` where it sets them +fn project_relays( + pbs: &PbsConfig, + relays: &[RelayConfig], + min_bid_wei: Option, + builder_boost_factor: Option, + advertised_url: &str, +) -> Result { + // hostname -> (its first relay, that relay's cap) + let mut hosts: BTreeMap<&str, (&str, u64)> = BTreeMap::new(); + for relay in relays { + let host = relay.entry.url.host_str().unwrap_or_default(); + let cap = relay.max_execution_payment_gwei.or(pbs.max_execution_payment_gwei); + let cap = cap.unwrap_or(ExecutionPaymentCap::Unclamped).gwei(); + let (first, first_cap) = *hosts.entry(host).or_insert((relay.id(), cap)); + ensure!( + first_cap == cap, + "relays {first} and {} share a hostname, so they share one entry and need the same \ + max_execution_payment_gwei", + relay.id() + ); + } + ensure!( + hosts.len() <= MAX_BUILDER_ENTRIES, + "relays on {} hostnames, over the {MAX_BUILDER_ENTRIES} entries a builder config holds", + hosts.len() + ); + + let min_bid = to_gwei(min_bid_wei.unwrap_or(pbs.min_bid_wei)).wrap_err("min_bid_eth")?; + let key_min_bid = pbs.min_bid_p2p_wei.map(to_gwei).transpose().wrap_err("min_bid_p2p_eth")?; + let boost = builder_boost_factor.unwrap_or(BUILDER_BOOST_FACTOR); + let key_boost = pbs.builder_boost_factor_p2p.unwrap_or(boost); + + let builders = hosts + .into_iter() + .map(|(host, (_, cap))| BuilderEntry { + url: advertised_url.to_string(), + auth_data: Some(hex::encode_prefixed(host)), + builder_pubkeys: Some(vec![]), + max_execution_payment: Some(cap.to_string()), + min_bid: Some(min_bid.clone()), + builder_boost_factor: Some(boost.to_string()), + }) + .collect(); + + Ok(BuilderConfig { + min_bid: Some(key_min_bid.unwrap_or(min_bid)), + builder_boost_factor: Some(key_boost.to_string()), + builders: Some(builders), + }) +} + +/// Whole Gwei, rounded down +fn to_gwei(wei: U256) -> Result { + let gwei = u64::try_from(wei / Unit::GWEI.wei()).wrap_err("over the largest Uint64 in Gwei")?; + Ok(gwei.to_string()) +} + +#[cfg(test)] +mod tests { + use super::*; + + // relay pubkeys from config.example.toml (valid BLS points) + const RELAY_PK_A: &str = "0xa1cec75a3f0661e99299274182938151e8433c61a19222347ea1313d839229cb4ce4e3e5aa2bdeb71c8fcf1b084963c2"; + const RELAY_PK_B: &str = "0xa119589bb33ef52acbb8116832bec2b58fca590fe5c85eac5d3230b44d5bc09fe73ccd21f88eab31d6de16194d17782e"; + const ADVERTISED_URL: &str = "https://cb.example.com"; + const UNCLAMPED: &str = "18446744073709551615"; + + fn random_key_hex() -> String { + cb_common::types::BlsSecretKey::random().public_key().as_hex_string() + } + + /// One mux holding a fresh key, with extra `[pbs]` and `[[mux]]` lines and + /// relays given as (URL host, extra relay lines) + fn one_mux(pbs: &str, mux: &str, relays: &[(&str, &str)]) -> Result { + let relays: String = relays + .iter() + .enumerate() + .map(|(i, (host, extra))| { + let pk = [RELAY_PK_A, RELAY_PK_B][i % 2]; + format!("[[mux.relays]]\nid = \"r{i}\"\nurl = \"https://{pk}@{host}\"\n{extra}\n") + }) + .collect(); + parse_config(&format!( + "chain = \"Holesky\"\n[pbs]\n{pbs}\n[[mux]]\nid = \"m\"\nvalidator_pubkeys = [\"{}\"]\n{mux}\n{relays}", + random_key_hex() + )) + } + + async fn projected(cfg: &CommitBoostConfig) -> Result { + project(cfg, &mux_keys(cfg).await?, ADVERTISED_URL) + } + + /// The one mux key's builder config + fn doc(projection: &Projection) -> &BuilderConfig { + projection.mux_docs.values().next().unwrap() + } + + // A mux key gets its mux's relays; `[[relays]]` is the config of any other key + #[tokio::test] + async fn projects_literal_json_doc() { + let key = random_key_hex(); + let cfg = parse_config(&format!( + r#" +chain = "Holesky" + +[pbs] +min_bid_eth = 0.5 + +[[relays]] +url = "https://{RELAY_PK_B}@default-relay.example.com" + +[[mux]] +id = "mux1" +validator_pubkeys = ["{key}"] + +[[mux.relays]] +url = "https://{RELAY_PK_A}@relay-a.example.com" + +[[mux.relays]] +url = "https://{RELAY_PK_B}@relay-b.example.com" +"# + )) + .unwrap(); + let projection = projected(&cfg).await.unwrap(); + + assert_eq!(projection.mux_docs.keys().collect::>(), [&key]); + let entry = |host_hex: &str| { + format!( + r#"{{"url":"https://cb.example.com","auth_data":"{host_hex}","builder_pubkeys":[],"max_execution_payment":"18446744073709551615","min_bid":"500000000","builder_boost_factor":"100"}}"# + ) + }; + let config = |entries: &[String]| { + format!( + r#"{{"min_bid":"500000000","builder_boost_factor":"100","builders":[{}]}}"#, + entries.join(",") + ) + }; + let mux_entries = [ + entry("0x72656c61792d612e6578616d706c652e636f6d"), + entry("0x72656c61792d622e6578616d706c652e636f6d"), + ]; + assert_eq!(serde_json::to_string(doc(&projection)).unwrap(), config(&mux_entries)); + // "default-relay.example.com" + let relays_entries = [entry("0x64656661756c742d72656c61792e6578616d706c652e636f6d")]; + let relays_doc = projection.relays_doc.as_ref().unwrap(); + assert_eq!(serde_json::to_string(relays_doc).unwrap(), config(&relays_entries)); + } + + #[tokio::test] + async fn entry_cap_is_the_relays_else_the_global_one_else_unclamped() { + let cases = [ + ("", "max_execution_payment_gwei = 250000000", "", [ + Some("250000000"), + Some(UNCLAMPED), + ]), + ( + "max_execution_payment_gwei = 500000000", + "max_execution_payment_gwei = 250000000", + "", + [Some("250000000"), Some("500000000")], + ), + ( + r#"max_execution_payment_gwei = "unclamped""#, + "", + "max_execution_payment_gwei = 0", + [Some(UNCLAMPED), Some("0")], + ), + ( + "max_execution_payment_gwei = 5", + r#"max_execution_payment_gwei = "unclamped""#, + "", + [Some(UNCLAMPED), Some("5")], + ), + ]; + for (pbs, cap_a, cap_b, expected) in cases { + let relays = [("relay-a.example.com", cap_a), ("relay-b.example.com", cap_b)]; + let projection = projected(&one_mux(pbs, "", &relays).unwrap()).await.unwrap(); + let caps: Vec<_> = doc(&projection) + .builders + .as_ref() + .unwrap() + .iter() + .map(|entry| entry.max_execution_payment.as_deref()) + .collect(); + assert_eq!(caps, expected, "{pbs} / {cap_a} / {cap_b}"); + } + } + + #[tokio::test] + async fn one_host_is_one_entry_with_one_cap() { + let unclamped = r#"max_execution_payment_gwei = "unclamped""#; + let cases = [ + ("", "max_execution_payment_gwei = 100", "max_execution_payment_gwei = 200", None), + ("", "max_execution_payment_gwei = 100", "", None), + ( + "max_execution_payment_gwei = 500", + "max_execution_payment_gwei = 500", + "", + Some("500"), + ), + (unclamped, "", unclamped, Some(UNCLAMPED)), + ]; + for (pbs, cap_a, cap_b, expected) in cases { + let relays = [("relay-a.example.com", cap_a), ("relay-a.example.com:8443/b", cap_b)]; + match (projected(&one_mux(pbs, "", &relays).unwrap()).await, expected) { + (Ok(projection), Some(cap)) => { + let entries = doc(&projection).builders.clone().unwrap(); + assert_eq!(entries.len(), 1); + assert_eq!(entries[0].max_execution_payment.as_deref(), Some(cap)); + } + (Err(err), None) => { + assert!(format!("{err:#}").contains("share a hostname"), "{err:#}") + } + (result, _) => panic!("{pbs} / {cap_a} / {cap_b}: {result:?}"), + } + } + } + + // builder-config stops on a key Commit-Boost would ignore, naming it and its + // table, so a typo cannot project a default + #[test] + fn refuses_unknown_keys_and_bad_caps() { + let cases: [(&str, &str, &str, &[&str]); 3] = [ + ("min_bid_p2p_eht = \"0.2\"", "bulder_boost_factor = 100", "", &[ + "`min_bid_p2p_eht` in [pbs]", + "`bulder_boost_factor` in [[mux]] m", + ]), + ("", "", "max_execution_payment = 1", &[ + "could not parse [[mux]] m", + "unknown field `max_execution_payment`", + ]), + (r#"max_execution_payment_gwei = "unlimited""#, "", "", &[ + r#"expected a Gwei amount or "unclamped", got "unlimited""#, + ]), + ]; + for (pbs, mux, relay, expected) in cases { + let Err(err) = one_mux(pbs, mux, &[("relay-a.example.com", relay)]) else { + panic!("accepted: {pbs} / {mux} / {relay}"); + }; + for part in expected { + assert!(format!("{err:#}").contains(part), "{err:#}"); + } + } + } + + #[tokio::test] + async fn key_level_takes_the_p2p_values() { + // (extra [pbs], extra [[mux]], key level, entry level); an unset boost is 100 + let cases = [ + ("", "", ("0", Some("100")), ("0", Some("100"))), + ("builder_boost_factor_p2p = 50", "", ("0", Some("50")), ("0", Some("100"))), + ( + "min_bid_p2p_eth = \"0.2\"\nbuilder_boost_factor_p2p = 0", + "min_bid_eth = \"0.0000010015\"\nbuilder_boost_factor = 100", + ("200000000", Some("0")), + ("1001", Some("100")), + ), + ( + "min_bid_p2p_eth = \"0.2\"", + "builder_boost_factor = 120", + ("200000000", Some("120")), + ("0", Some("120")), + ), + ( + "builder_boost_factor_p2p = 50\nmin_bid_eth = 0.5", + "min_bid_eth = \"0.7\"\nbuilder_boost_factor = 100", + ("700000000", Some("50")), + ("700000000", Some("100")), + ), + ]; + for (pbs, mux, key_level, entry_level) in cases { + let cfg = one_mux(pbs, mux, &[("relay-a.example.com", "")]).unwrap(); + let projection = projected(&cfg).await.unwrap(); + let doc = doc(&projection); + let entry = &doc.builders.as_ref().unwrap()[0]; + assert_eq!( + (doc.min_bid.as_deref().unwrap(), doc.builder_boost_factor.as_deref()), + key_level, + "{pbs} / {mux}" + ); + assert_eq!( + (entry.min_bid.as_deref().unwrap(), entry.builder_boost_factor.as_deref()), + entry_level, + "{pbs} / {mux}" + ); + } + } + + // Commit-Boost refuses to start with either + #[tokio::test] + async fn duplicate_key_errors() { + let key = random_key_hex(); + let relay = + |pk: &str, host: &str| format!("[[mux.relays]]\nurl = \"https://{pk}@{host}\"\n"); + let (relay_a, relay_b) = + (relay(RELAY_PK_A, "relay-a.example.com"), relay(RELAY_PK_B, "relay-b.example.com")); + let across = format!( + "[[mux]]\nid = \"m1\"\nvalidator_pubkeys = [\"{key}\"]\n{relay_a}\ + [[mux]]\nid = \"m2\"\nvalidator_pubkeys = [\"{key}\"]\n{relay_b}" + ); + let within = + format!("[[mux]]\nid = \"m1\"\nvalidator_pubkeys = [\"{key}\", \"{key}\"]\n{relay_a}"); + for muxes in [across, within] { + let cfg = parse_config(&format!("chain = \"Holesky\"\n[pbs]\n{muxes}")).unwrap(); + let err = format!("{:#}", projected(&cfg).await.unwrap_err()); + assert!(err.contains("duplicate validator pubkey"), "{muxes}: {err}"); + } + } + + // (extra [pbs] lines, extra [[mux]] lines, relay hostnames, the error or None) + #[tokio::test] + async fn projection_errors() { + let cases = [ + ("", "relays = []", 0, Some("must have at least one relay")), + ("", "min_bid_eth = 1e20", 1, Some("min_bid_eth: over the largest Uint64")), + ("min_bid_p2p_eth = 1e20", "", 1, Some("min_bid_p2p_eth: over the largest Uint64")), + ("", "", 64, None), + ("", "", 65, Some("relays on 65 hostnames, over the 64 entries")), + ]; + let hosts: Vec = (0..65).map(|i| format!("relay-{i}.example.com")).collect(); + for (pbs, mux, n_hosts, expected) in cases { + let relays: Vec<_> = hosts[..n_hosts].iter().map(|host| (host.as_str(), "")).collect(); + let cfg = one_mux(pbs, mux, &relays).unwrap(); + match (projected(&cfg).await, expected) { + (Ok(projection), None) => { + assert_eq!(doc(&projection).builders.as_ref().map(Vec::len), Some(n_hosts)) + } + (Err(err), Some(expected)) => { + assert!(format!("{err:#}").contains(expected), "{pbs} / {mux}: {err:#}") + } + (result, _) => panic!("{pbs} / {mux} / {n_hosts}: {result:?}"), + } + } + } + + #[test] + fn a_config_error_names_its_line() { + let text = format!( + "chain = \"Holesky\"\n[pbs]\n[[relays]]\nurl = \"https://{RELAY_PK_A}@relay.example.com\"\nfoo = 1\n" + ); + let err = parse_config(&text).unwrap_err(); + assert!(format!("{err:#}").contains("line 5"), "{err:#}"); + } + + // A `[[relays]]` entry's own cap applies to the keys outside every mux, and + // with no `[[relays]]` those keys get nothing + #[tokio::test] + async fn relays_config_is_for_keys_outside_muxes() { + let relays = format!( + "[[relays]]\nurl = \"https://{RELAY_PK_A}@relay-a.example.com\"\nmax_execution_payment_gwei = 7\n" + ); + let cfg = parse_config(&format!("chain = \"Holesky\"\n[pbs]\n{relays}")).unwrap(); + let projection = projected(&cfg).await.unwrap(); + assert!(projection.mux_docs.is_empty()); + let key = random_key_hex(); + let entries = projection.doc_for(&key).unwrap().builders.as_ref().unwrap(); + assert_eq!(entries[0].max_execution_payment.as_deref(), Some("7")); + + let cfg = parse_config("chain = \"Holesky\"\n[pbs]\n").unwrap(); + assert!(projected(&cfg).await.unwrap().doc_for(&key).is_none()); + } + + // Lodestar refuses a cap above 0 without a flag, so builder-config warns when + // it writes one + #[tokio::test] + async fn nonzero_cap_is_detected() { + let relays = format!( + "[[relays]]\nurl = \"https://{RELAY_PK_A}@relay-a.example.com\"\n\ + max_execution_payment_gwei = \"unclamped\"\n" + ); + let zero_with_relays = format!("max_execution_payment_gwei = 0\n{relays}"); + for (pbs, expected) in [ + ("", true), + ("max_execution_payment_gwei = 5", true), + ("max_execution_payment_gwei = 0", false), + (zero_with_relays.as_str(), true), + ] { + let cfg = one_mux(pbs, "", &[("relay-a.example.com", "")]).unwrap(); + assert_eq!(projected(&cfg).await.unwrap().has_nonzero_cap(), expected, "{pbs}"); + } + } + + // A URL loader's keys are fetched, as Commit-Boost fetches them at startup + #[tokio::test] + async fn url_loader_resolves_keys() { + let key = random_key_hex(); + let body = format!(r#"["{key}"]"#); + let app = + axum::Router::new().route("/keys", axum::routing::get(move || async move { body })); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + tokio::spawn(async move { axum::serve(listener, app).await }); + + let mux = format!(r#"loader = {{ url = "http://{addr}/keys" }}"#); + let cfg = one_mux("", &mux, &[("relay-a.example.com", "")]).unwrap(); + let projection = projected(&cfg).await.unwrap(); + assert!(projection.mux_docs.contains_key(&key), "{:?}", projection.mux_docs.keys()); + // The mux's own validator_pubkeys key is named, the fetched one is not + assert_eq!(projection.fetched_keys.into_iter().collect::>(), [key]); + } + + // Commit-Boost routes each of two muxes with one id to its own relays, which + // builder-config cannot tell apart by id + #[tokio::test] + async fn two_muxes_with_one_id_are_refused() { + let mux = |host: &str| { + format!( + "[[mux]]\nid = \"m\"\nvalidator_pubkeys = [\"{}\"]\n\ + [[mux.relays]]\nurl = \"https://{RELAY_PK_A}@{host}\"\n", + random_key_hex() + ) + }; + let cfg = parse_config(&format!( + "chain = \"Holesky\"\n[pbs]\n{}{}", + mux("relay-a.example.com"), + mux("relay-b.example.com") + )) + .unwrap(); + let err = projected(&cfg).await.unwrap_err(); + assert!(format!("{err:#}").contains("mux id m names more than one [[mux]]"), "{err:#}"); + } + + // builder-config reads no relay header secrets, and checks rpc_url only for a + // registry loader, which reads its keys through it + #[tokio::test] + async fn resolves_keys_without_commit_boosts_runtime_secrets() { + let pbs = r#"rpc_url = "http://127.0.0.1:1""#; + let header = r#"headers = { X-Api-Key = { env = "CB_KM_TEST_UNSET_HEADER" } }"#; + let cfg = one_mux(pbs, "", &[("relay-a.example.com", header)]).unwrap(); + assert_eq!(projected(&cfg).await.unwrap().mux_docs.len(), 1); + + let lido = r#"loader = { registry = "lido", node_operator_id = 1 }"#; + let cfg = one_mux(pbs, lido, &[("relay-a.example.com", "")]).unwrap(); + let err = projected(&cfg).await.unwrap_err(); + assert!(!format!("{err:#}").contains("requires RPC URL"), "{err:#}"); + } + + #[tokio::test] + async fn file_loader_resolves_keys() { + let key_a = random_key_hex(); + let key_b = random_key_hex(); + let dir = tempfile::tempdir().unwrap(); + let keys_path = dir.path().join("keys.json"); + // A file listing a key twice is deduped, as Commit-Boost does + std::fs::write(&keys_path, format!(r#"["{key_b}", "{key_b}"]"#)).unwrap(); + let cfg = parse_config(&format!( + r#" +chain = "Holesky" +[pbs] +[[mux]] +id = "filemux" +validator_pubkeys = ["{key_a}"] +loader = "{}" +[[mux.relays]] +url = "https://{RELAY_PK_A}@relay-a.example.com" +"#, + keys_path.display() + )) + .unwrap(); + let projection = projected(&cfg).await.unwrap(); + let mut expected = vec![key_a, key_b]; + expected.sort(); + assert_eq!(projection.mux_docs.into_keys().collect::>(), expected); + } +} diff --git a/crates/km/src/targets.rs b/crates/km/src/targets.rs new file mode 100644 index 000000000..acc872e21 --- /dev/null +++ b/crates/km/src/targets.rs @@ -0,0 +1,192 @@ +//! Where builder-config writes: Commit-Boost's URL and the validator clients. + +use std::{ + collections::BTreeSet, + net::{Ipv4Addr, Ipv6Addr}, + path::PathBuf, + str::FromStr, +}; + +use eyre::{Result, bail, ensure}; +use url::{Host, Url}; + +use crate::client::read_token; + +#[derive(Debug)] +pub struct Targets { + /// Commit-Boost's URL as the beacon nodes reach it. Written as given, + /// since a parsed `Url` gains a trailing slash + pub advertised_url: String, + pub vcs: Vec, +} + +#[derive(Debug, Clone)] +pub struct VcConfig { + /// The validator client's keymanager API + pub url: Url, + pub token_path: PathBuf, +} + +/// `=`, split at the first `=`: a keymanager URL +/// has none, a file path might +impl FromStr for VcConfig { + type Err = String; + + fn from_str(s: &str) -> Result { + let Some((url, token_path)) = s.split_once('=') else { + return Err(format!("{s}: not =")); + }; + // `localhost:5062` would parse with the scheme `localhost` + if !url.starts_with("http://") && !url.starts_with("https://") { + return Err(format!("{url}: not an http(s) URL")); + } + let url = Url::parse(url).map_err(|err| format!("{url}: {err}"))?; + if token_path.is_empty() { + return Err(format!("{s}: no token file after =")); + } + Ok(Self { url, token_path: token_path.into() }) + } +} + +/// Written as typed, so only a form the beacon node reads the same way: +/// `localhost:18550` parses with the scheme `localhost`, and +/// `https:cb.example.com` only once the parser repairs it +pub fn check_advertised_url(url: &str) -> Result<()> { + ensure!( + (url.starts_with("http://") || url.starts_with("https://")) && + url.trim() == url && + Url::parse(url).is_ok(), + "--advertised-url is not an http(s) URL: {url}" + ); + Ok(()) +} + +impl Targets { + pub fn new(advertised_url: String, vcs: Vec) -> Result { + check_advertised_url(&advertised_url)?; + let mut seen = BTreeSet::new(); + if let Some(vc) = vcs.iter().find(|vc| !seen.insert(&vc.url)) { + bail!("{} is given twice with --vc", vc.url); + } + for (i, vc) in vcs.iter().enumerate() { + if let Some(alias) = + vcs[i + 1..].iter().find(|other| same_loopback(&vc.url, &other.url)) + { + bail!( + "{} and {} are one validator client given twice with --vc", + vc.url, + alias.url + ); + } + } + Ok(Self { advertised_url, vcs }) + } + + /// Reads every token file, so a bad path stops the run before any client + /// is contacted + pub fn check_token_files(&self) -> Result<()> { + for vc in &self.vcs { + read_token(&vc.token_path)?; + } + Ok(()) + } +} + +pub(crate) fn is_loopback(url: &Url) -> bool { + match url.host() { + Some(Host::Ipv4(ip)) => ip.is_loopback(), + Some(Host::Ipv6(ip)) => ip.to_canonical().is_loopback(), + Some(Host::Domain(host)) => host == "localhost", + None => false, + } +} + +/// `localhost`, `127.0.0.1` or `[::1]`, the names of one listener; other +/// loopback addresses, such as `127.0.0.2`, are separate sockets +fn localhost_name(url: &Url) -> bool { + match url.host() { + Some(Host::Ipv4(ip)) => ip == Ipv4Addr::LOCALHOST, + Some(Host::Ipv6(ip)) => ip == Ipv6Addr::LOCALHOST, + Some(Host::Domain(host)) => host == "localhost", + None => false, + } +} + +/// Two localhost names for one port, such as `localhost` and `127.0.0.1`: one +/// client, whose keys would read as held by two +fn same_loopback(a: &Url, b: &Url) -> bool { + localhost_name(a) && + localhost_name(b) && + a.scheme() == b.scheme() && + a.port_or_known_default() == b.port_or_known_default() && + a.path() == b.path() +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn advertised_url_must_be_http() { + for (url, ok) in [ + ("http://cb.example.com:18550", true), + ("https://cb.example.com", true), + ("not a url", false), + ("localhost:18550", false), + ("ftp://cb.example.com", false), + ("https:cb.example.com", false), + (" http://cb.example.com", false), + ("http://cb.example.com ", false), + ("http://", false), + ] { + assert_eq!(Targets::new(url.to_string(), vec![]).is_ok(), ok, "{url}"); + } + } + + // One client twice would read as a key held by two + #[test] + fn refuses_a_validator_client_given_twice() { + for (a, b, refused) in [ + ("http://127.0.0.1:7500", "http://127.0.0.1:7500", true), + ("http://localhost:7500", "http://127.0.0.1:7500", true), + ("http://[::1]:7500", "http://127.0.0.1:7500/", true), + ("http://localhost:7500", "http://127.0.0.1:7501", false), + ("http://localhost:7500", "https://127.0.0.1:7500", false), + ("http://10.0.0.1:7500", "http://10.0.0.2:7500", false), + ("http://10.0.0.1:7500", "http://127.0.0.1:7500", false), + ("http://127.0.0.1:7500", "http://10.0.0.1:7500", false), + // A proxy on one port can route two paths to two clients + ("http://localhost:7500/a", "http://127.0.0.1:7500/b", false), + // Clients bound to their own loopback addresses + ("http://127.0.0.1:7500", "http://127.0.0.2:7500", false), + ] { + let vc = |url: &str| format!("{url}=/t").parse::().unwrap(); + let targets = Targets::new("http://cb:18550".to_string(), vec![vc(a), vc(b)]); + match targets { + Ok(_) => assert!(!refused, "{a} {b}"), + Err(err) => { + assert!(refused, "{a} {b}"); + assert!(format!("{err:#}").contains("given twice"), "{err:#}"); + } + } + } + } + + #[test] + fn vc_flag_parses_url_and_token_file() { + let vc: VcConfig = "http://127.0.0.1:7500=/run/secrets/a=b".parse().unwrap(); + assert_eq!( + (vc.url.as_str(), vc.token_path.to_str()), + ("http://127.0.0.1:7500/", Some("/run/secrets/a=b")) + ); + for bad in [ + "http://127.0.0.1:7500", + "http://127.0.0.1:7500=", + "not a url=/t", + "localhost:7500=/t", + "127.0.0.1:7500=/t", + ] { + assert!(bad.parse::().is_err(), "{bad}"); + } + } +} diff --git a/crates/km/src/unknown_keys.rs b/crates/km/src/unknown_keys.rs new file mode 100644 index 000000000..cea337483 --- /dev/null +++ b/crates/km/src/unknown_keys.rs @@ -0,0 +1,134 @@ +//! Serde ignores unknown `[pbs]` and `[[mux]]` keys, so a typo such as +//! `min_bid_p2p_eht` would project the default; builder-config refuses the +//! config instead. Relay entries already reject unknown keys. + +/// `StaticPbsConfig` and its flattened `PbsConfig`, as serde names them +const PBS_KEYS: &[&str] = &[ + "docker_image", + "with_signer", + "host", + "port", + "relay_check", + "wait_all_registrations", + "timeout_get_header_ms", + "timeout_get_payload_ms", + "timeout_register_validator_ms", + "skip_sigverify", + "min_bid_eth", + "late_in_slot_time_ms", + "proposer_deadline_buffer_ms", + "extra_validation_enabled", + "rpc_url", + "ssv_node_api_url", + "ssv_public_api_url", + "http_timeout_seconds", + "register_validator_retry_limit", + "validator_registration_batch_size", + "mux_registry_refresh_interval_seconds", + "max_execution_payment_gwei", + "min_bid_p2p_eth", + "builder_boost_factor_p2p", +]; + +/// `MuxConfig`, as serde names it +const MUX_KEYS: &[&str] = &[ + "id", + "relays", + "validator_pubkeys", + "loader", + "timeout_get_header_ms", + "late_in_slot_time_ms", + "builder_boost_factor", + "min_bid_eth", +]; + +/// Every `[pbs]` and `[[mux]]` key Commit-Boost does not read, naming its table +pub fn unknown_keys(raw: &toml::Value) -> Vec { + let mut unknown = Vec::new(); + if let Some(pbs) = raw.get("pbs").and_then(toml::Value::as_table) { + for key in pbs.keys().filter(|key| !PBS_KEYS.contains(&key.as_str())) { + unknown.push(format!("`{key}` in [pbs]")); + } + } + let muxes = raw.get("mux").and_then(toml::Value::as_array).into_iter().flatten(); + for table in muxes.filter_map(toml::Value::as_table) { + let id = table.get("id").and_then(toml::Value::as_str).unwrap_or_default(); + for key in table.keys().filter(|key| !MUX_KEYS.contains(&key.as_str())) { + unknown.push(format!("`{key}` in [[mux]] {id}")); + } + } + unknown +} + +#[cfg(test)] +mod tests { + use std::{collections::BTreeSet, net::Ipv4Addr}; + + use alloy::primitives::U256; + use cb_common::config::{ + ExecutionPaymentCap, MuxConfig, MuxKeysLoader, PbsConfig, StaticPbsConfig, + }; + use url::Url; + + use super::*; + + fn keys(value: &toml::Value) -> BTreeSet { + value.as_table().unwrap().keys().cloned().collect() + } + + fn set(list: &[&str]) -> BTreeSet { + list.iter().map(|key| key.to_string()).collect() + } + + // Every Option is set, since toml drops a None, and a new field does not + // compile until it is set here + #[test] + fn lists_match_the_keys_commit_boost_reads() { + let url = |s: &str| Url::parse(s).unwrap(); + let pbs = StaticPbsConfig { + docker_image: "ghcr.io/commit-boost/pbs:latest".into(), + pbs_config: PbsConfig { + host: Ipv4Addr::LOCALHOST, + port: 18550, + relay_check: true, + wait_all_registrations: true, + timeout_get_header_ms: 950, + timeout_get_payload_ms: 4000, + timeout_register_validator_ms: 3000, + skip_sigverify: false, + min_bid_wei: U256::from(1), + late_in_slot_time_ms: 2000, + proposer_deadline_buffer_ms: 50, + extra_validation_enabled: false, + rpc_url: Some(url("https://rpc.example.com")), + ssv_node_api_url: url("https://ssv.example.com"), + ssv_public_api_url: url("https://ssv-public.example.com"), + http_timeout_seconds: 30, + register_validator_retry_limit: 3, + validator_registration_batch_size: Some(10), + mux_registry_refresh_interval_seconds: 384, + max_execution_payment_gwei: Some(ExecutionPaymentCap::Unclamped), + min_bid_p2p_wei: Some(U256::from(2)), + builder_boost_factor_p2p: Some(90), + }, + with_signer: false, + }; + let mux = MuxConfig { + id: "m".into(), + relays: vec![], + validator_pubkeys: vec![], + loader: Some(MuxKeysLoader::File("./keys.json".into())), + timeout_get_header_ms: Some(900), + late_in_slot_time_ms: Some(1500), + builder_boost_factor: Some(100), + min_bid_wei: Some(U256::from(3)), + }; + + let pbs = toml::Value::try_from(&pbs).unwrap(); + assert_eq!(keys(&pbs), set(PBS_KEYS)); + pbs.try_into::().unwrap(); + let mux = toml::Value::try_from(&mux).unwrap(); + assert_eq!(keys(&mux), set(MUX_KEYS)); + mux.try_into::().unwrap(); + } +} diff --git a/crates/pbs/Cargo.toml b/crates/pbs/Cargo.toml index 7037a566f..d25303eb1 100644 --- a/crates/pbs/Cargo.toml +++ b/crates/pbs/Cargo.toml @@ -40,4 +40,6 @@ webpki-roots.workspace = true thiserror.workspace = true [dev-dependencies] +tempfile.workspace = true tokio = { workspace = true, features = ["test-util"] } +tracing-test.workspace = true diff --git a/crates/pbs/src/config_miss.rs b/crates/pbs/src/config_miss.rs new file mode 100644 index 000000000..ff5001164 --- /dev/null +++ b/crates/pbs/src/config_miss.rs @@ -0,0 +1,155 @@ +//! Keys whose bid or preferences requests name none of their relays, because +//! their builder config is missing or stale. Each epoch names the first few +//! keys and counts the rest, so these warnings stay a few lines an epoch and +//! their memory stays bounded. + +use std::{ + collections::HashSet, + sync::{LazyLock, Mutex, PoisonError}, + time::Duration, +}; + +use cb_common::types::BlsPublicKey; +use tracing::warn; + +/// Keys named in the log each epoch +const NAMED: usize = 5; +/// Keys remembered each epoch, so each is counted once +const REMEMBERED: usize = 1024; +/// Auth data bytes logged, since a request can send up to 4096 +const AUTH_DATA_SHOWN: usize = 64; + +/// Auth data naming none of the key's relays that is not a builder outside the +/// config +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum Miss { + /// It names Commit-Boost itself: no builder config, or an entry without + /// auth data + NoConfig, + /// It names another of Commit-Boost's relays + Stale, +} + +#[derive(Default)] +struct Epoch { + keys: HashSet, + /// Keys past the first few + unnamed: usize, + /// Whether keys past `REMEMBERED` went uncounted + full: bool, +} + +static EPOCH: LazyLock> = LazyLock::new(Default::default); + +/// Whether to name `pubkey`: each epoch names its first few keys and counts +/// the rest, and a key already seen is neither +fn admit(epoch: &mut Epoch, pubkey: &BlsPublicKey) -> bool { + if epoch.keys.contains(pubkey) { + return false; + } + if epoch.keys.len() == REMEMBERED { + epoch.full = true; + return false; + } + epoch.keys.insert(pubkey.clone()); + let named = epoch.keys.len() <= NAMED; + if !named { + epoch.unnamed += 1; + } + named +} + +/// Logs how many keys the epoch left unnamed, and starts the next +fn roll(epoch: &mut Epoch) { + if epoch.unnamed > 0 { + warn!( + keys = epoch.unnamed, + at_least = epoch.full, + "more keys than logged named none of their relays last epoch; write their builder \ + config with `commit-boost builder-config apply` or your tooling" + ); + } + *epoch = Epoch::default(); +} + +pub(crate) fn record(pubkey: &BlsPublicKey, mux_id: Option<&str>, auth_data: &[u8], miss: Miss) { + if !admit(&mut EPOCH.lock().unwrap_or_else(PoisonError::into_inner), pubkey) { + return; + } + let mux = mux_id.unwrap_or("[[relays]]"); + let auth_data = String::from_utf8_lossy(&auth_data[..auth_data.len().min(AUTH_DATA_SHOWN)]); + match miss { + Miss::NoConfig => warn!( + %pubkey, mux, ?auth_data, + "auth data names Commit-Boost itself: the key has no builder config, or an entry \ + without auth_data; write it with `commit-boost builder-config apply` or your tooling" + ), + Miss::Stale => warn!( + %pubkey, mux, ?auth_data, + "auth data names another of Commit-Boost's relays: the key's builder config is \ + stale; write it again with `commit-boost builder-config apply` or your tooling" + ), + } +} + +/// Ends an epoch every `epoch_secs`, counted from the service's start +pub(crate) fn start(epoch_secs: u64) { + tokio::spawn(async move { + let mut tick = tokio::time::interval(Duration::from_secs(epoch_secs.max(1))); + loop { + tick.tick().await; + roll(&mut EPOCH.lock().unwrap_or_else(PoisonError::into_inner)); + } + }); +} + +#[cfg(test)] +mod tests { + use cb_common::types::BlsSecretKey; + + use super::*; + + // A key is counted once, the first few are named, and memory stops growing + // at the cap + #[test] + fn an_epoch_names_a_few_and_counts_each_key_once() { + let mut epoch = Epoch::default(); + let keys: Vec<_> = + (0..REMEMBERED + 3).map(|_| BlsSecretKey::random().public_key()).collect(); + let named = keys.iter().chain(&keys[..2]).filter(|key| admit(&mut epoch, key)).count(); + assert_eq!(named, NAMED); + assert_eq!(epoch.keys.len(), REMEMBERED); + assert_eq!((epoch.unnamed, epoch.full), (REMEMBERED - NAMED, true)); + roll(&mut epoch); + assert_eq!((epoch.keys.len(), epoch.unnamed, epoch.full), (0, 0, false)); + } + + // An epoch that ends with keys it did not name logs how many + #[test] + #[tracing_test::traced_test] + fn an_epoch_ending_counts_its_unnamed_keys() { + roll(&mut Epoch { unnamed: 3, ..Default::default() }); + assert!(logs_contain("keys=3")); + assert!(logs_contain("more keys than logged named none of their relays last epoch")); + } + + // A request whose auth data names Commit-Boost itself is recorded for the + // epoch's log + #[tokio::test] + async fn a_miss_is_recorded() { + let pubkey = BlsSecretKey::random().public_key(); + let addressed = crate::utils::Addressed { + endpoint: "a_miss_is_recorded", + pubkey: &pubkey, + mux_id: None, + all_relays: &[], + }; + let mut headers = reqwest::header::HeaderMap::new(); + let host = reqwest::header::HeaderValue::from_static("cb.example.com"); + headers.insert(reqwest::header::HOST, host); + let resolved = + crate::utils::resolve_addressed_relay(&[], b"cb.example.com", &headers, 0, &addressed); + assert!(resolved.await.is_err()); + assert!(EPOCH.lock().unwrap_or_else(PoisonError::into_inner).keys.contains(&pubkey)); + } +} diff --git a/crates/pbs/src/config_watch.rs b/crates/pbs/src/config_watch.rs new file mode 100644 index 000000000..3c7f386fe --- /dev/null +++ b/crates/pbs/src/config_watch.rs @@ -0,0 +1,242 @@ +//! Watches the PBS config file for a change to its contents. + +use std::path::Path; + +use notify::{Event, RecommendedWatcher, RecursiveMode, Watcher}; +use tracing::warn; + +/// Calls `on_change` each time the contents of `path` differ from the last +/// ones it accepted, so a failed reload is retried at the next event. Watches +/// the directory too, since replacing the file ends the watch on it +pub(crate) fn watch( + path: &Path, + on_change: impl Fn() -> bool + Send + 'static, +) -> notify::Result { + let file = path.to_path_buf(); + let mut accepted = std::fs::read(&file).ok(); + let mut watcher = RecommendedWatcher::new( + move |result: notify::Result| match result { + Ok(event) => handle(&event, &file, &mut accepted, &on_change), + Err(err) => warn!(%err, "error watching PBS config file for changes"), + }, + notify::Config::default(), + )?; + // A write to a bind-mounted file shows only on the file itself + watcher.watch(path, RecursiveMode::NonRecursive)?; + // A bare file name has an empty parent + let dir = path.parent().filter(|dir| !dir.as_os_str().is_empty()).unwrap_or(Path::new(".")); + if let Err(err) = watcher.watch(dir, RecursiveMode::NonRecursive) { + warn!(%err, ?dir, "cannot watch the PBS config file's directory, so a replaced file is not seen"); + } + Ok(watcher) +} + +/// Calls `on_change` when `event` can have changed the file and it reads +/// differently from `accepted`, which a successful reload updates +fn handle( + event: &Event, + file: &Path, + accepted: &mut Option>, + on_change: &dyn Fn() -> bool, +) { + // Reading the file opens it, so acting on an open or read would loop + if event.kind.is_access() { + return; + } + // Missing mid-replace; the event that completes the replace reads it + let Ok(contents) = std::fs::read(file) else { return }; + if accepted.as_ref() != Some(&contents) && on_change() { + *accepted = Some(contents); + } +} + +#[cfg(test)] +mod tests { + use std::{ + fs, + path::PathBuf, + sync::{ + Arc, Mutex, + atomic::{AtomicUsize, Ordering}, + }, + thread, + time::{Duration, Instant}, + }; + + use notify::event::{AccessKind, AccessMode, EventKind, ModifyKind}; + + use super::*; + + fn counted(path: &Path) -> (RecommendedWatcher, Arc) { + let count = Arc::new(AtomicUsize::new(0)); + let seen = count.clone(); + let watcher = watch(path, move || { + seen.fetch_add(1, Ordering::SeqCst); + true + }) + .unwrap(); + (watcher, count) + } + + /// Waits up to 5 s for `count` to reach `want`, then 300 ms for any extra + /// call + fn settled(count: &AtomicUsize, want: usize) -> usize { + let start = Instant::now(); + while count.load(Ordering::SeqCst) < want && start.elapsed() < Duration::from_secs(5) { + thread::sleep(Duration::from_millis(20)); + } + thread::sleep(Duration::from_millis(300)); + count.load(Ordering::SeqCst) + } + + // A ConfigMap mount: the file is a symlink through `..data`, which an + // update points at a new directory + #[cfg(unix)] + #[test] + fn every_configmap_update_is_seen() { + use std::os::unix::fs::symlink; + + let dir = tempfile::tempdir().unwrap(); + let version = |name: &str, contents: &str| -> PathBuf { + let path = dir.path().join(name); + fs::create_dir(&path).unwrap(); + fs::write(path.join("config.toml"), contents).unwrap(); + path + }; + version("..v1", "a = 1"); + symlink("..v1", dir.path().join("..data")).unwrap(); + symlink("..data/config.toml", dir.path().join("config.toml")).unwrap(); + let (_watcher, count) = counted(&dir.path().join("config.toml")); + + for (n, (name, old)) in [("..v2", "..v1"), ("..v3", "..v2")].into_iter().enumerate() { + version(name, &format!("a = {}", n + 2)); + symlink(name, dir.path().join("..data_tmp")).unwrap(); + fs::rename(dir.path().join("..data_tmp"), dir.path().join("..data")).unwrap(); + fs::remove_dir_all(dir.path().join(old)).unwrap(); + assert_eq!(settled(&count, n + 1), n + 1, "update {}", n + 1); + } + } + + #[test] + fn every_replace_by_rename_is_seen() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("config.toml"); + fs::write(&path, "a = 1").unwrap(); + let (_watcher, count) = counted(&path); + + for n in 1..=2 { + let tmp = dir.path().join("config.toml.tmp"); + fs::write(&tmp, format!("a = {}", n + 1)).unwrap(); + fs::rename(&tmp, &path).unwrap(); + assert_eq!(settled(&count, n), n, "replace {n}"); + } + } + + #[test] + fn a_write_in_place_is_seen_and_a_chmod_is_not() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("config.toml"); + fs::write(&path, "a = 1").unwrap(); + let seen = Arc::new(Mutex::new(Vec::new())); + let (file, reads) = (path.clone(), seen.clone()); + let _watcher = watch(&path, move || { + reads.lock().unwrap().push(fs::read_to_string(&file).unwrap_or_default()); + true + }) + .unwrap(); + let last = || seen.lock().unwrap().last().cloned(); + + fs::write(&path, "a = 2").unwrap(); + // A read can land mid-write, so wait for the final contents + let start = Instant::now(); + while last().as_deref() != Some("a = 2") && start.elapsed() < Duration::from_secs(5) { + thread::sleep(Duration::from_millis(20)); + } + assert_eq!(last().as_deref(), Some("a = 2")); + + // The write's own events can still be arriving + thread::sleep(Duration::from_millis(500)); + let calls = seen.lock().unwrap().len(); + let mut permissions = fs::metadata(&path).unwrap().permissions(); + permissions.set_readonly(true); + fs::set_permissions(&path, permissions).unwrap(); + thread::sleep(Duration::from_secs(1)); + assert_eq!(seen.lock().unwrap().len(), calls); + } + + // A write through another link to the file reaches only the file's own + // watch, as a host write to a bind-mounted file does + #[cfg(unix)] + #[test] + fn a_write_through_another_link_is_seen() { + let dir = tempfile::tempdir().unwrap(); + fs::create_dir(dir.path().join("a")).unwrap(); + fs::create_dir(dir.path().join("b")).unwrap(); + let path = dir.path().join("a/config.toml"); + fs::write(&path, "a = 1").unwrap(); + fs::hard_link(&path, dir.path().join("b/config.toml")).unwrap(); + let (_watcher, count) = counted(&path); + + fs::write(dir.path().join("b/config.toml"), "a = 2").unwrap(); + assert!(settled(&count, 1) >= 1); + } + + // An open or a read is not a change, since reading the file is one; a + // write is + #[test] + fn only_a_change_to_the_file_is_handled() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("config.toml"); + fs::write(&path, "a = 2").unwrap(); + let mut accepted = Some(b"a = 1".to_vec()); + let calls = AtomicUsize::new(0); + let on_change = || { + calls.fetch_add(1, Ordering::SeqCst); + true + }; + for kind in [ + AccessKind::Open(AccessMode::Any), + AccessKind::Read, + AccessKind::Close(AccessMode::Read), + ] { + handle(&Event::new(EventKind::Access(kind)), &path, &mut accepted, &on_change); + } + assert_eq!(calls.load(Ordering::SeqCst), 0); + handle(&Event::new(EventKind::Modify(ModifyKind::Any)), &path, &mut accepted, &on_change); + assert_eq!(calls.load(Ordering::SeqCst), 1); + } + + // A reload that fails leaves the contents unaccepted, so the next event + // retries it, and one that succeeds is not repeated + #[test] + fn a_failed_reload_is_retried() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("config.toml"); + fs::write(&path, "a = 2").unwrap(); + let mut accepted = Some(b"a = 1".to_vec()); + let calls = AtomicUsize::new(0); + let on_change = || calls.fetch_add(1, Ordering::SeqCst) > 0; + let modified = Event::new(EventKind::Modify(ModifyKind::Any)); + for _ in 0..3 { + handle(&modified, &path, &mut accepted, &on_change); + } + assert_eq!(calls.load(Ordering::SeqCst), 2); + assert_eq!(accepted.as_deref(), Some(&b"a = 2"[..])); + } + + // A directory the service can enter but not list still lets it watch the + // file + #[cfg(unix)] + #[test] + fn an_unlistable_directory_still_watches_the_file() { + use std::os::unix::fs::PermissionsExt; + + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("config.toml"); + fs::write(&path, "a = 1").unwrap(); + fs::set_permissions(dir.path(), fs::Permissions::from_mode(0o311)).unwrap(); + let watched = watch(&path, || true); + fs::set_permissions(dir.path(), fs::Permissions::from_mode(0o755)).unwrap(); + assert!(watched.is_ok(), "{:?}", watched.err()); + } +} diff --git a/crates/pbs/src/dial.rs b/crates/pbs/src/dial.rs index d5a75a007..fb61f5303 100644 --- a/crates/pbs/src/dial.rs +++ b/crates/pbs/src/dial.rs @@ -179,6 +179,7 @@ fn dial_config(url: Url) -> RelayConfig { target_first_request_ms: None, frequency_get_header_ms: None, validator_registration_batch_size: None, + max_execution_payment_gwei: None, } } diff --git a/crates/pbs/src/error.rs b/crates/pbs/src/error.rs index 29f8dc4ca..0a2305935 100644 --- a/crates/pbs/src/error.rs +++ b/crates/pbs/src/error.rs @@ -26,6 +26,8 @@ pub enum PbsClientError { NoBuilderResponse, #[error("auth data does not match a configured builder")] AuthDataMismatch, + #[error("auth data names Commit-Boost itself, not a builder")] + NoBuilderConfig, #[error("dial target does not resolve or resolves to a disallowed address")] DialTargetBlocked, #[error("missing or invalid timing headers")] @@ -52,6 +54,7 @@ impl PbsClientError { PbsClientError::NoResponse => StatusCode::BAD_GATEWAY, PbsClientError::NoBuilderResponse => StatusCode::INTERNAL_SERVER_ERROR, PbsClientError::AuthDataMismatch => StatusCode::BAD_REQUEST, + PbsClientError::NoBuilderConfig => StatusCode::BAD_REQUEST, PbsClientError::DialTargetBlocked => StatusCode::BAD_REQUEST, PbsClientError::MissingTimingHeader => StatusCode::BAD_REQUEST, PbsClientError::AuthSlotMismatch => StatusCode::BAD_REQUEST, @@ -76,6 +79,9 @@ impl IntoResponse for PbsClientError { PbsClientError::AuthDataMismatch => { "Invalid SignedBuilderRequestAuth: auth.message.data does not match any configured builder".to_string() } + PbsClientError::NoBuilderConfig => { + "Invalid SignedBuilderRequestAuth: auth.message.data names Commit-Boost itself; the key's builder config names no relay".to_string() + } PbsClientError::DialTargetBlocked => { "Invalid SignedBuilderRequestAuth: the addressed builder's host does not resolve or resolves to a disallowed address".to_string() } diff --git a/crates/pbs/src/lib.rs b/crates/pbs/src/lib.rs index 9c04224a8..504d32178 100644 --- a/crates/pbs/src/lib.rs +++ b/crates/pbs/src/lib.rs @@ -1,4 +1,6 @@ mod api; +mod config_miss; +mod config_watch; mod constants; mod dial; mod error; diff --git a/crates/pbs/src/metrics.rs b/crates/pbs/src/metrics.rs index 51846f946..163ceb10d 100644 --- a/crates/pbs/src/metrics.rs +++ b/crates/pbs/src/metrics.rs @@ -97,6 +97,15 @@ lazy_static! { .unwrap(); // TO BEACON NODE + /// ePBS auth data routing outcome, by endpoint + pub static ref AUTH_DATA_ROUTE: IntCounterVec = register_int_counter_vec_with_registry!( + "auth_data_route_total", + "How ePBS requests' auth data routed", + &["endpoint", "outcome"], + PBS_METRICS_REGISTRY + ) + .unwrap(); + /// Status code returned to beacon node by endpoint pub static ref BEACON_NODE_STATUS: IntCounterVec = register_int_counter_vec_with_registry!( "beacon_node_status_code_total", diff --git a/crates/pbs/src/routes/builder_preferences.rs b/crates/pbs/src/routes/builder_preferences.rs index cba99d0ef..35d57b27f 100644 --- a/crates/pbs/src/routes/builder_preferences.rs +++ b/crates/pbs/src/routes/builder_preferences.rs @@ -18,8 +18,9 @@ use crate::{ error::PbsClientError, state::{BuilderApiState, PbsState}, utils::{ - builder_rejection, epbs_base_send_headers, log_mux_selection, post_ssz_expect_accepted, - record_beacon_status, record_request_failure, resolve_addressed_relay, + Addressed, builder_rejection, epbs_base_send_headers, log_mux_selection, + post_ssz_expect_accepted, record_beacon_status, record_request_failure, + resolve_addressed_relay, }, }; @@ -67,11 +68,18 @@ pub async fn submit_builder_preferences( // Preferences are submitted an epoch ahead, so they share the registration // timeout rather than the block-production one + let addressed = Addressed { + endpoint: SUBMIT_BUILDER_PREFERENCES_ENDPOINT_TAG, + pubkey: ¶ms.proposer_pubkey, + mux_id: maybe_mux_id, + all_relays: state.all_relays(), + }; let (relay, timeout_ms) = resolve_addressed_relay( relays, request.auth.message.data.as_ref(), &req_headers, pbs_config.timeout_register_validator_ms, + &addressed, ) .await?; diff --git a/crates/pbs/src/routes/execution_payload_bid.rs b/crates/pbs/src/routes/execution_payload_bid.rs index affdfcf10..23cdd1927 100644 --- a/crates/pbs/src/routes/execution_payload_bid.rs +++ b/crates/pbs/src/routes/execution_payload_bid.rs @@ -35,8 +35,9 @@ use crate::{ metrics::{RELAY_HEADER_VALUE, RELAY_LAST_SLOT}, state::{BuilderApiState, PbsState}, utils::{ - builder_rejection, epbs_base_send_headers, format_gwei_as_eth, log_mux_selection, - record_beacon_status, record_request_failure, resolve_addressed_relay, send_to_relay, + Addressed, builder_rejection, epbs_base_send_headers, format_gwei_as_eth, + log_mux_selection, record_beacon_status, record_request_failure, resolve_addressed_relay, + send_to_relay, }, }; @@ -144,11 +145,18 @@ pub async fn get_execution_payload_bid( return Ok(None); } + let addressed = Addressed { + endpoint: GET_EXECUTION_PAYLOAD_BID_ENDPOINT_TAG, + pubkey: ¶ms.proposer_pubkey, + mux_id: maybe_mux_id, + all_relays: state.all_relays(), + }; let (relay, max_timeout_ms) = match resolve_addressed_relay( relays, auth.message.data.as_ref(), &req_headers, max_timeout_ms, + &addressed, ) .await { diff --git a/crates/pbs/src/service.rs b/crates/pbs/src/service.rs index ed3d179c2..3f59d8e18 100644 --- a/crates/pbs/src/service.rs +++ b/crates/pbs/src/service.rs @@ -12,7 +12,6 @@ use cb_common::{ }; use cb_metrics::provider::MetricsProvider; use eyre::{Context, Result, bail}; -use notify::{Error, Event, RecommendedWatcher, RecursiveMode, Watcher}; use parking_lot::RwLock; use prometheus::core::Collector; use tokio::net::TcpListener; @@ -21,9 +20,11 @@ use url::Url; use crate::{ api::BuilderApi, + config_miss, config_watch, metrics::PBS_METRICS_REGISTRY, routes::create_app_router, state::{BuilderApiState, PbsState, PbsStateGuard}, + utils::init_auth_data_route_metric, }; pub struct PbsService; @@ -51,6 +52,9 @@ impl PbsService { }) }); + config_miss::start(state.config.chain.slot_time_sec().saturating_mul(32)); + init_auth_data_route_metric(); + let config_path = state.config_path.clone(); let state: Arc>> = RwLock::new(state).into(); let app = create_app_router::(state.clone()); @@ -71,43 +75,30 @@ impl PbsService { } // Set up the filesystem watcher for the config file - let mut watcher: RecommendedWatcher; + let _watcher; if config_path.to_str() != Some("") { let state_for_watcher = state.clone(); let config_path_for_watcher = config_path.clone(); - watcher = RecommendedWatcher::new( - move |result: Result| { - match result { - Err(err) => { - warn!(%err, "error watching PBS config file for changes"); - return; - } - Ok(event) => { - if !event.kind.is_modify() { - return; - } - } + // The watcher calls back on its own thread, and loading the config + // needs this runtime: its loaders and the rpc_url check are async + let runtime = tokio::runtime::Handle::current(); + _watcher = config_watch::watch(&config_path, move || { + info!("detected change in PBS config file, reloading configuration"); + let result = + runtime.block_on(load_pbs_config(Some(config_path_for_watcher.to_path_buf()))); + match result { + Ok((new_config, _)) => { + let mut state = state_for_watcher.write(); + state.config = Arc::new(new_config); + info!("configuration reloaded from file after update"); + true } - - // Reload the configuration when the file is modified - info!("detected change in PBS config file, reloading configuration"); - let result = futures::executor::block_on(load_pbs_config(Some( - config_path_for_watcher.to_path_buf(), - ))); - match result { - Ok((new_config, _)) => { - let mut state = state_for_watcher.write(); - state.config = Arc::new(new_config); - info!("configuration reloaded from file after update"); - } - Err(err) => { - warn!(%err, "failed to reload configuration from file after update"); - } + Err(err) => { + warn!(%err, "failed to reload configuration from file after update"); + false } - }, - notify::Config::default(), - )?; - watcher.watch(config_path.as_path(), RecursiveMode::Recursive)?; + } + })?; info!("watching PBS config file for changes: {:?}", config_path); } diff --git a/crates/pbs/src/state.rs b/crates/pbs/src/state.rs index bd683e5f4..87162c4a4 100644 --- a/crates/pbs/src/state.rs +++ b/crates/pbs/src/state.rs @@ -44,9 +44,9 @@ where &self.config.pbs_config } - /// Returns all the relays (including those in muxes) - /// DO NOT use this through the PBS module, use - /// [`PbsState::mux_config_and_relays`] instead + /// Every relay, muxes' included. Route with + /// [`PbsState::mux_config_and_relays`]; this one only tells a stale builder + /// config apart pub fn all_relays(&self) -> &[RelayClient] { &self.config.all_relays } diff --git a/crates/pbs/src/utils.rs b/crates/pbs/src/utils.rs index 1ec49bc91..cc7e85e3d 100644 --- a/crates/pbs/src/utils.rs +++ b/crates/pbs/src/utils.rs @@ -4,7 +4,7 @@ use std::{ }; use alloy::primitives::utils::{ParseUnits, Unit}; -use axum::body::Bytes; +use axum::{body::Bytes, http::uri::Authority}; use cb_common::{ pbs::{HEADER_VERSION_KEY, RelayClient, decode_auth_data_url, error::PbsError}, types::BlsPublicKey, @@ -16,16 +16,20 @@ use cb_common::{ use futures::future::join_all; use reqwest::{ StatusCode, - header::{CONTENT_TYPE, HeaderMap, USER_AGENT}, + header::{CONTENT_TYPE, HOST, HeaderMap, USER_AGENT}, }; use tracing::{Instrument, debug, error, warn}; use url::Url; use crate::{ - constants::{MAX_SIZE_DEFAULT, TIMEOUT_ERROR_CODE_STR}, + config_miss::{self, Miss}, + constants::{ + GET_EXECUTION_PAYLOAD_BID_ENDPOINT_TAG, MAX_SIZE_DEFAULT, + SUBMIT_BUILDER_PREFERENCES_ENDPOINT_TAG, TIMEOUT_ERROR_CODE_STR, + }, dial::{auth_data_address, dial_relay}, error::PbsClientError, - metrics::{BEACON_NODE_STATUS, RELAY_LATENCY, RELAY_STATUS_CODE}, + metrics::{AUTH_DATA_ROUTE, BEACON_NODE_STATUS, RELAY_LATENCY, RELAY_STATUS_CODE}, }; /// Sends one already-built relay request, recording the per-relay metrics @@ -69,6 +73,9 @@ pub(crate) fn record_request_failure( let err = err.into(); if err.status_code().is_server_error() { error!(%err, "{endpoint} failed"); + } else if matches!(err, PbsClientError::NoBuilderConfig) { + // config_miss warned about the key, a few times an epoch + debug!(%err, "{endpoint} failed"); } else { warn!(%err, "{endpoint} failed"); } @@ -187,32 +194,90 @@ pub fn check_gas_limit(gas_limit: u64, parent_gas_limit: u64) -> bool { true } +/// The key a bid or preferences request is for, and every relay Commit-Boost +/// has, to tell a missing or stale builder config from a dial +pub(crate) struct Addressed<'a> { + pub endpoint: &'static str, + pub pubkey: &'a BlsPublicKey, + pub mux_id: Option<&'a str>, + pub all_relays: &'a [RelayClient], +} + +/// The relay `address` names, by hostname or, for a URL, by origin +fn relay_named<'a>( + relays: &'a [RelayClient], + address: &[u8], + data_url: Option<&Url>, +) -> Option<&'a RelayClient> { + relays.iter().find(|relay| { + let url = &relay.config.entry.url; + match data_url { + Some(data_url) => { + url.scheme() == data_url.scheme() && + url.host() == data_url.host() && + url.port_or_known_default() == data_url.port_or_known_default() + } + None => url.host_str().is_some_and(|host| host.as_bytes() == address), + } + }) +} + +/// Whether auth data naming none of the key's relays names another of +/// Commit-Boost's relays, or Commit-Boost itself by the request's `Host` +fn classify_miss( + address: &[u8], + data_url: Option<&Url>, + req_headers: &HeaderMap, + all_relays: &[RelayClient], +) -> Option { + if relay_named(all_relays, address, data_url).is_some() { + return Some(Miss::Stale); + } + let own: Authority = req_headers.get(HOST)?.to_str().ok()?.parse().ok()?; + let is_own = match data_url { + // A URL also names a port, so a builder on Commit-Boost's host at another + // port is still dialed + Some(data_url) => { + let default = if data_url.scheme() == "https" { 443 } else { 80 }; + data_url.host_str()?.eq_ignore_ascii_case(own.host()) && + data_url.port_or_known_default() == Some(own.port_u16().unwrap_or(default)) + } + None => address.eq_ignore_ascii_case(own.host().as_bytes()), + }; + is_own.then_some(Miss::NoConfig) +} + +/// The outcomes `resolve_addressed_relay` counts +const AUTH_DATA_OUTCOMES: [&str; 4] = ["relay", "no_config", "stale", "dial"]; + +/// Creates every auth data route series at 0, so an alert sees the first miss +/// as an increase +pub(crate) fn init_auth_data_route_metric() { + for endpoint in + [GET_EXECUTION_PAYLOAD_BID_ENDPOINT_TAG, SUBMIT_BUILDER_PREFERENCES_ENDPOINT_TAG] + { + for outcome in AUTH_DATA_OUTCOMES { + AUTH_DATA_ROUTE.with_label_values(&[endpoint, outcome]); + } + } +} + /// 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. +/// hostname or, for a URL, by origin; otherwise a relay that dials it, unless +/// it names Commit-Boost itself. Also returns what `timeout_ms` leaves after +/// setting up that dial. pub(crate) async fn resolve_addressed_relay( relays: &[RelayClient], auth_data: &[u8], req_headers: &HeaderMap, timeout_ms: u64, + addressed: &Addressed<'_>, ) -> Result<(RelayClient, u64), PbsClientError> { + let count = |outcome| AUTH_DATA_ROUTE.with_label_values(&[addressed.endpoint, outcome]).inc(); 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) - }); - 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 { + if let Some(relay) = relay_named(relays, address, data_url.as_ref()) { + count("relay"); return Ok((relay.clone(), timeout_ms)); } // Every Commit-Boost dial carries this header, so a request dialed back into @@ -224,6 +289,18 @@ pub(crate) async fn resolve_addressed_relay( ); return Err(PbsClientError::AuthDataMismatch); } + match classify_miss(address, data_url.as_ref(), req_headers, addressed.all_relays) { + Some(Miss::NoConfig) => { + config_miss::record(addressed.pubkey, addressed.mux_id, auth_data, Miss::NoConfig); + count("no_config"); + return Err(PbsClientError::NoBuilderConfig); + } + Some(Miss::Stale) => { + config_miss::record(addressed.pubkey, addressed.mux_id, auth_data, Miss::Stale); + count("stale"); + } + None => count("dial"), + } 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); @@ -260,10 +337,49 @@ mod tests { target_first_request_ms: None, frequency_get_header_ms: None, validator_registration_batch_size: None, + max_execution_payment_gwei: None, }; RelayClient::new(config).unwrap() } + // A miss naming Commit-Boost's own host, as a hostname or Prysm's whole URL, + // is a key with no builder config; one naming another of its relays is + // stale, even on Commit-Boost's host; anything else is a dial + #[test] + fn classify_auth_data_misses() { + let all = + vec![test_relay("https://other.example.com"), test_relay("http://127.0.0.1:9000")]; + let with_host = |host: &str| { + let mut headers = HeaderMap::new(); + headers.insert(HOST, host.parse().unwrap()); + headers + }; + for (data, host, expected) in [ + ("cb.example.com", "cb.example.com:18550", Some(Miss::NoConfig)), + ("CB.example.com", "cb.example.com", Some(Miss::NoConfig)), + ("http://cb.example.com:18550", "cb.example.com:18550", Some(Miss::NoConfig)), + ("http://cb.example.com", "cb.example.com", Some(Miss::NoConfig)), + ("http://cb.example.com:9000", "cb.example.com:18550", None), + ("http://cb.example.com:18550", "CB.example.com:18550", Some(Miss::NoConfig)), + ("https://cb.example.com", "cb.example.com", Some(Miss::NoConfig)), + ("[::1]", "[::1]:18550", Some(Miss::NoConfig)), + // A Host that is not an authority names nothing + ("cb.example.com", "cb example.com", None), + ("other.example.com", "cb.example.com:18550", Some(Miss::Stale)), + ("https://other.example.com", "cb.example.com:18550", Some(Miss::Stale)), + ("127.0.0.1", "127.0.0.1:18550", Some(Miss::Stale)), + ("builder.example.com", "cb.example.com:18550", None), + ("http://builder.example.com", "cb.example.com:18550", None), + ] { + let data_url = decode_auth_data_url(data.as_bytes()); + let miss = classify_miss(data.as_bytes(), data_url.as_ref(), &with_host(host), &all); + assert_eq!(miss, expected, "{data} at {host}"); + } + // Without a Host header Commit-Boost cannot tell its own name + let none = classify_miss(b"cb.example.com", None, &HeaderMap::new(), &all); + assert_eq!(none, None); + } + #[tokio::test] async fn resolve_relay_by_auth_data() { let relays = vec![ @@ -275,7 +391,10 @@ mod tests { 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) + let pubkey = BlsSecretKey::random().public_key(); + let addressed = + Addressed { endpoint: "test", pubkey: &pubkey, mux_id: None, all_relays: &relays }; + resolve_addressed_relay(&relays, data, &from_cb, 0, &addressed) .await .ok() .map(|(relay, _)| relay.config.entry.url.host_str().unwrap().to_string()) @@ -291,6 +410,7 @@ mod tests { Some("builder-a.example.com") ); assert!(host(b"http://builder-a.example.com").await.is_none()); + assert!(host(b"http://builder-a.example.com:443").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!( @@ -298,4 +418,72 @@ mod tests { Some("builder-a.example.com") ); } + + // Each endpoint has every outcome's series before any request + #[test] + fn auth_data_route_series_start_at_zero() { + use prometheus::core::Collector; + + init_auth_data_route_metric(); + let family = &AUTH_DATA_ROUTE.collect()[0]; + let present = |endpoint: &str, outcome: &str| { + family.get_metric().iter().any(|metric| { + let labels: Vec<_> = + metric.get_label().iter().map(|label| label.get_value()).collect(); + labels.contains(&endpoint) && labels.contains(&outcome) + }) + }; + for endpoint in + [GET_EXECUTION_PAYLOAD_BID_ENDPOINT_TAG, SUBMIT_BUILDER_PREFERENCES_ENDPOINT_TAG] + { + for outcome in AUTH_DATA_OUTCOMES { + assert!(present(endpoint, outcome), "{endpoint} {outcome}"); + } + } + } + + // Each route counts its outcome: one of the key's relays, Commit-Boost + // itself (refused), another of its relays (stale, still dialed) and any + // other builder (dialed). A request from a Commit-Boost goes no further and + // is not counted. The dials here are to addresses the dial check refuses + #[tokio::test] + async fn resolve_counts_each_outcome() { + let relays = vec![test_relay("https://builder-a.example.com")]; + let all = vec![relays[0].clone(), test_relay("http://127.0.0.1:9000")]; + let endpoint = "resolve_counts_each_outcome"; + let pubkey = BlsSecretKey::random().public_key(); + let addressed = Addressed { endpoint, pubkey: &pubkey, mux_id: None, all_relays: &all }; + let mut to_cb = HeaderMap::new(); + to_cb.insert(HOST, reqwest::header::HeaderValue::from_static("cb.example.com:18550")); + let mut from_cb = to_cb.clone(); + from_cb.insert(HEADER_VERSION_KEY, reqwest::header::HeaderValue::from_static("test")); + let outcomes = AUTH_DATA_OUTCOMES; + let counts = || { + outcomes.map(|outcome| AUTH_DATA_ROUTE.with_label_values(&[endpoint, outcome]).get()) + }; + for (data, headers, outcome, expected) in [ + ("builder-a.example.com", &to_cb, Some("relay"), "routed"), + ("cb.example.com", &to_cb, Some("no_config"), "no builder config"), + ("127.0.0.1", &to_cb, Some("stale"), "dialed"), + ("10.0.0.1", &to_cb, Some("dial"), "dialed"), + ("cb.example.com", &from_cb, None, "not dialed on"), + ] { + let before = counts(); + let result = + resolve_addressed_relay(&relays, data.as_bytes(), headers, 1000, &addressed); + let result = match result.await { + Ok(_) => "routed", + Err(PbsClientError::NoBuilderConfig) => "no builder config", + Err(PbsClientError::DialTargetBlocked) => "dialed", + Err(PbsClientError::AuthDataMismatch) => "not dialed on", + Err(err) => panic!("{data}: {err}"), + }; + assert_eq!(result, expected, "{data}"); + let after = counts(); + for (i, name) in outcomes.iter().enumerate() { + let counted = u64::from(Some(*name) == outcome); + assert_eq!(after[i] - before[i], counted, "{data}: {name}"); + } + } + } } diff --git a/docs/docs/get_started/configuration.md b/docs/docs/get_started/configuration.md index 075c61e79..1d358ba39 100644 --- a/docs/docs/get_started/configuration.md +++ b/docs/docs/get_started/configuration.md @@ -703,7 +703,9 @@ Passing the minted token on the command line is acceptable only because it expir ### Automatic reload (PBS only) -In addition to the manual `/reload` endpoint, the PBS service watches the config file for changes and automatically reloads the configuration whenever the file is modified, with no restart or API call needed. If a reload fails (e.g. because of a misconfigured option), the previous configuration is kept: the watcher logs a warning and the `/reload` endpoint returns a 500 error. +In addition to the manual `/reload` endpoint, the PBS service watches the config file and automatically reloads the configuration whenever its contents change, with no restart or API call needed. That includes a file replaced by a rename in its directory, as editors and Ansible do, and a Kubernetes ConfigMap or Secret update; a Kubernetes `subPath` mount is never updated. A change that leaves the contents as they were, such as `touch`, does not reload, and neither does a change to a file the config names, such as a mux keys file: send `POST /reload` for those. If a reload fails (e.g. because of a misconfigured option), the previous configuration is kept: the watcher logs a warning and tries again at the next change, and the `/reload` endpoint returns a 500 error. + +From the Gloas fork, a reload does not reach your validator clients' builder config: after changing `[[relays]]`, `[[mux]]` or a value [`commit-boost builder-config`](./epbs.md#builder-config-command) reads, run it again. :::caution Custom PBS binaries Custom PBS binaries only get the file watcher if they pass the real config path to `PbsState::new`; see [Extending PBS](../developing/extending-pbs.md#entry-point). `POST /reload` works either way. diff --git a/docs/docs/get_started/epbs.md b/docs/docs/get_started/epbs.md index 1c14de6c8..a0fd5c3ed 100644 --- a/docs/docs/get_started/epbs.md +++ b/docs/docs/get_started/epbs.md @@ -24,9 +24,11 @@ With ePBS the beacon node validates each bid and weighs it against your builder The ePBS bid endpoint does not use these PBS options, which will be deprecated after the hard fork: -- `skip_sigverify`, `min_bid_eth` and `extra_validation_enabled`: the beacon node checks the bid against the on-chain builder registry and applies the `min_bid` from its builder config. +- `skip_sigverify` and `extra_validation_enabled`: the beacon node checks the bid against the on-chain builder registry. - `timeout_get_header_ms`, `late_in_slot_time_ms`, their `[[mux]]` overrides and the timing-games options: the beacon node's deadline bounds the request (see [Timing](#timing)). +Commit-Boost does not apply `min_bid_eth` to ePBS bids either. The beacon node applies the `min_bid` in each key's builder config, which [`commit-boost builder-config`](#builder-config-command) writes from `min_bid_eth`. + A `get_header = "stream"` relay is asked for ePBS bids over plain HTTP. ## How a bid request reaches a builder @@ -43,14 +45,14 @@ List each builder as a relay entry, as for PBS: in `[[relays]]`, or in the `[[mu ### 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 gets no bids through Commit-Boost ([builders outside your config](#builders-outside-your-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 hostname in the builder's relay entry `url`. For a relay, that is the relay's own host, not a block builder behind it. 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`: +[`commit-boost builder-config`](#builder-config-command) writes this config for you from the Commit-Boost config. To write it by hand, 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 file at `$TOKEN_FILE` and the validator key in `$PUBKEY`: ```bash -curl -X POST "$KEYMANAGER_URL/eth/v1/validator/$PUBKEY/builder_config" \ - -H "Authorization: Bearer $TOKEN" \ - -H "Content-Type: application/json" \ +printf 'Authorization: Bearer %s' "$(cat "$TOKEN_FILE")" | \ + curl -X POST "$KEYMANAGER_URL/eth/v1/validator/$PUBKEY/builder_config" \ + -H @- -H "Content-Type: application/json" \ -d '{ "builders": [ { @@ -62,7 +64,7 @@ curl -X POST "$KEYMANAGER_URL/eth/v1/validator/$PUBKEY/builder_config" \ }' ``` -`echo 0x$(printf builder-a.example.com | xxd -p | tr -d '\n')` prints the `auth_data` hex. +The token goes to curl on stdin (`-H @-`, curl 7.55 or newer), so it never appears in a command line other users can read. `echo 0x$(printf builder-a.example.com | xxd -p | tr -d '\n')` prints the `auth_data` hex. 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: @@ -70,6 +72,229 @@ The call is `POST /eth/v1/validator/{pubkey}/builder_config`, authenticated with - `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. +:::warning Proposer settings files +A validator client that loads a proposer settings file overrides or refuses keymanager writes. Prysm started with `--proposer-settings-file` or `--proposer-settings-url` reloads the file at every start, and a file with a `proposer_config` section replaces every key's per-key config, so builder config written through the API is lost at the next restart. Lodestar started with `--proposerSettingsFile` refuses every per-key write with `403`. Before writing builder config through the API, move off the file's per-key settings: + +- Prysm: remove the flag, restart, check that your fee recipients survived, then write the builder config. +- Lodestar: save each key's fee recipient with `GET /eth/v1/validator//feerecipient`, which works with the file set. Stop the validator client and replace `--proposerSettingsFile` with flags for the file's `default_config`, such as `--suggestedFeeRecipient`, `--graffiti`, `--defaultGasLimit` and `--builder.selection`: without them every key falls back to Lodestar's defaults, a zero-address fee recipient among them. Start it, POST each key's own settings from `proposer_config`, such as `{"ethaddress": "0x..."}` to `/eth/v1/validator//feerecipient`, and read them back. Once a key is written through the API, Lodestar refuses to start with `--proposerSettingsFile` again. +::: + +#### With `commit-boost builder-config` {#builder-config-command} + +`commit-boost builder-config` works out each key's builder config from the Commit-Boost config, routed as Commit-Boost routes it: a key in a `[[mux]]` gets that mux's relays, and any other key gets `[[relays]]`. It finds each mux's keys as Commit-Boost does at startup, from `validator_pubkeys` and the mux's loader, so a URL or registry loader needs the network, and keys a registry adds later need another run. With a registry loader it also checks `rpc_url` as Commit-Boost does. It has two subcommands: + +- `apply` writes the config to the validator clients you give it, through their keymanager APIs, so it runs where it can reach them. +- `print` writes the config as JSON and contacts no validator client, for your own tooling to send, or for `apply --from` to write where the validator client runs. + +Which to use: + +| Setup | Use | +|---|---| +| Commit-Boost and the validator client run as binaries on one host | [`apply`](#builder-config-apply) | +| `commit-boost init` Docker compose, with the validator client on the host | `apply` in [`docker run`](#builder-config-with-docker) | +| The validator client in another container or a packaged stack, such as eth-docker or Dappnode | [`print` piped into `apply --from -`](#builder-config-with-docker) in the client's network | +| Kubernetes | [`apply` in a sidecar](#builder-config-on-kubernetes) | +| Your own tooling already writes per-key settings, such as fee recipients, to locked-down validator clients | [`print`](#builder-config-print), and your tooling sends it | + +:::tip From MEV-Boost +Your MEV-Boost relays are the builders on this page: each `-relays` URL becomes a `[[relays]]` `url`. `-min-bid` becomes `[pbs] min_bid_eth`, which `builder-config` writes as every key's `min_bid`, so it floors p2p bids too; set `[pbs] min_bid_p2p_eth` to give p2p bids their own floor. +::: + +1. From v0.12.0-rc1, `builder-config` is a subcommand of `commit-boost`, in the binary and the Docker image. + +2. In the Commit-Boost config, set the values `builder-config` writes, such as a mux's `min_bid_eth` and `builder_boost_factor`. Commit-Boost itself does not act on them, so a mux's `min_bid_eth` does not floor that mux's PBS bids; `[pbs] min_bid_eth` still does. The table at the end of this section lists where each value comes from: + + ```toml + [[mux]] + id = "epbs" + validator_pubkeys = [ + "0x80c7f782b2467c5898c5516a8b6595d75623960b4afc4f71ee07d40985d20e117ba35e7cd352a3e75fb85a8668a3b745", + ] + # Drops bids below 0.01 ETH for these keys, p2p bids included + min_bid_eth = 0.01 + builder_boost_factor = 100 + + [[mux.relays]] + id = "builder-a" + url = "https://0xa1cec75a3f0661e99299274182938151e8433c61a19222347ea1313d839229cb4ce4e3e5aa2bdeb71c8fcf1b084963c2@builder-a.example.com" + + [[mux.relays]] + id = "builder-b" + url = "https://0xa119589bb33ef52acbb8116832bec2b58fca590fe5c85eac5d3230b44d5bc09fe73ccd21f88eab31d6de16194d17782e@builder-b.example.com" + ``` + + On Lodestar, start its validator client with `--allowDangerousTrustedPayments`: it refuses any cap above `0` without that flag, and the default cap is unclamped. `[pbs] max_execution_payment_gwei = 0` avoids the flag, but then the beacon node counts none of a builder's execution payment. `print` notes this on stderr. In `apply`, a client that refuses gets one error naming the flag, and no further key with a cap above `0` is written to it. + +3. Preview the config with `print`, then write it with `apply`, or send it with your own tooling. `--advertised-url` is Commit-Boost's URL as your beacon nodes reach it, starting with `http://` or `https://`. `--config` defaults to `CB_CONFIG`, as set inside Commit-Boost's container. + +##### `print` {#builder-config-print} + +```bash +commit-boost builder-config print --config cb-config.toml --advertised-url http://cb.example.com:18550 +``` + +stdout carries only the JSON document; the mux key counts, the Lodestar note and the loaders' warnings go to stderr. It exits `0`, `1` if the document could not be written, or `2` on a config or loader error. For the config above it prints this (abridged): + +```json +{ + "version": 1, + "advertised_url": "http://cb.example.com:18550", + "default": null, + "muxes": { + "epbs": { + "config": { + "min_bid": "10000000", + "builder_boost_factor": "100", + "builders": [ + { + "url": "http://cb.example.com:18550", + "auth_data": "0x6275696c6465722d612e6578616d706c652e636f6d", + "builder_pubkeys": [], + "max_execution_payment": "18446744073709551615", + "min_bid": "10000000", + "builder_boost_factor": "100" + }, + ... + ] + }, + "keys": ["0x80c7f782b2467c5898c5516a8b6595d75623960b4afc4f71ee07d40985d20e117ba35e7cd352a3e75fb85a8668a3b745"], + "fetched_keys": [] + } + } +} +``` + +Each mux's `config` is the POST body for every key in its `keys`, which the Commit-Boost config names, and its `fetched_keys`, which only a URL or registry loader lists and which can change after the print. `default` is the `[[relays]]` body for every other key, or `null` without `[[relays]]`. Keys are lowercase 0x hex. `version` is `1`; refuse any other. To send the document with your own tooling: + +- List the validator client's keys (`GET /eth/v1/keystores` and `GET /eth/v1/remotekeys`, where `404` means no remote keys), lowercase them, and write only those: some validator clients answer `202` for a key they do not hold. +- List every validator client before writing, then check each mux's `keys`: a key no client lists means a client is missing from your inventory, and a key two clients list is a slashing risk. A `fetched_keys` key no client lists is normal, such as an exited validator's. `apply` makes both checks. +- Give each key its mux's `config`, else `default`. With `default` `null`, leave other keys alone. Never `DELETE` a key's builder config: the key then falls back to the validator client's own builders. To take a key off Commit-Boost, POST `{"builders": []}` or its other builders. +- POST each body to `/eth/v1/validator//builder_config` with `Content-Type: application/json`, and require `202`. +- On `404` for a key the client lists, stop: the client has no builder config endpoint. On `403`, stop: a [proposer settings file](#validator-builder-config) or a wrong token. On `400` naming `--allowDangerousTrustedPayments`, restart Lodestar with that flag. On `501` or `503`, retry later: Prysm answers these before Gloas is scheduled and while it is not ready. +- A POST replaces the key's whole config. To keep entries another tool wrote, GET it first and keep each entry whose `url`, parsed, differs from `advertised_url`, so that `http://cb:18550` and `http://cb:18550/` count as one; write at most 64 entries in all. +- Run one writer at a time per validator client, print again after a mux's loader gains keys, and read back a key after writing it. +- `print` exits `0` when a loader falls back, such as to the SSV public API, and says so only in a warning on stderr, so fail your job on any stderr line containing `WARN`. +- Add a builder to Commit-Boost's config before writing builder configs that name it, and remove it from the builder configs before removing it from Commit-Boost; otherwise Commit-Boost reports those keys as stale or dials them as builders outside your config. +- Pass the token on stdin or from a file only its owner can read, never on the command line. + +`jq -r '.muxes[] | .config as $c | (.keys + .fetched_keys)[] | [., ($c | tojson)] | @tsv'` turns the document into one line per mux key: the key, a tab and its POST body. + +##### `apply` {#builder-config-apply} + +```bash +commit-boost builder-config apply --config cb-config.toml --advertised-url http://cb.example.com:18550 \ + --vc http://127.0.0.1:5062=$HOME/.lighthouse/hoodi/validators/api-token.txt +``` + +Each `--vc` is a validator client to write to: its keymanager API, starting with `http://` or `https://`, then `=` and the file holding its API token. Repeat `--vc` for each client. The shell does not expand `~` after `=`, so write `$HOME`. With `--from `, or `--from -` for stdin, `apply` writes a document `print` wrote in place of the Commit-Boost config. The document must be as `print` wrote it, with `advertised_url` exactly `--advertised-url`, and `--from` wins over `--config` and `CB_CONFIG`. + +Each validator client serves its keymanager API only when started with the flag below. By default: + +| Client | Keymanager API | Token file | +|---|---|---| +| Lighthouse | `--http`, at `http://127.0.0.1:5062` | `validators/api-token.txt` in its data directory, such as `~/.lighthouse/hoodi/validators/api-token.txt`; `--http-token-path` moves it | +| Lodestar | `--keymanager`, at `http://127.0.0.1:5062` | `validator-db/api-token.txt` in its data directory; `--keymanager.tokenFile` moves it. Lodestar rewrites the file at every start, keeping the token | +| Prysm | `--rpc`, at `http://127.0.0.1:7500` | `~/.eth2validators/prysm-wallet-v2/auth-token` on Linux; `--keymanager-token-file` moves it. `apply` refuses a token file of more than one line, so regenerate an older two-line one with `validator web generate-auth-token` | + +The token file is readable only by its owner, so run `apply` as the validator client's user, such as with `sudo -u `, and write the token path in full, since `$HOME` expands to yours. + +As of Nimbus 26.10.0, Nimbus stores builder config, but its beacon node does not request bids with it; its keys are written like any other's, so the output does not show this. Teku 26.9.1 and Grandine 3.0.0-rc.0 have no builder config endpoint. + +`apply` asks each validator client given with `--vc` for the keys it holds, its keystores and remote-signer keys, and writes each key only to the client that holds it. It prints: + +- `mux : keys` for each mux, before contacting a client; +- `accepted: on (mux )` for each write, or `([[relays]])` for a key in no mux; +- `: keys listed, written` for each client it wrote to, ending `, not written` when some were not; +- a closing `done:` tally: keys written, keys a listed client did not get, errors and warnings, the loaders' warnings included. + +`WARN:` lines go to stdout. `ERROR:` lines and the loaders' own warnings, such as a fallback to the SSV public API, go to stderr; `RUST_LOG` raises the loaders' log level, such as `RUST_LOG=info`. A failure on one client does not stop the others. A client that leaves 3 writes in a row unanswered, or answers `403` before any write succeeds, gets no further writes. + +It exits: + +- `0` when it prints no `ERROR:` line. +- `1` when it contacted the validator clients and printed an `ERROR:` line, such as for mux keys that no validator client given with `--vc` holds, or keys that two hold (a slashing risk). Those two errors list the keys. +- `2` when it stopped before contacting any validator client: a bad flag, which clap reports as `error:` and `apply` as `ERROR:`, an unreadable, empty or multi-line token file, a config error, a failed mux loader, or a `--from` document that does not match. + +Keys that only a URL or registry loader lists, such as exited validators', are counted in one warning instead, since no client may hold them. When a client could not be listed, that warning and the error for mux keys no client lists both name it, since it may hold them. + +A run over only some of the clients that hold a mux's keys, such as a sidecar in one validator client's pod or a canary on one client, fails on the keys the others hold. Pass `--partial` to such a run, and mux keys that no client given holds are counted in one warning, as is a client with no keys yet. A run's slashing check covers only the clients it is given, so also run `apply` over every client without `--partial`, from somewhere that reaches each keymanager API. + +Each run rewrites every key's builder config in full, so running it again is safe, and a run over 500 keys takes under a second. Run it from a timer, such as every epoch, and after each validator client restart, so a key you import later is written without your noticing it was missing. Run it again after fixing an error, whenever you change the muxes, their keys, their builders or the values it reads, and whenever a validator client gains a key, which gets no bids through Commit-Boost until it is written. A Commit-Boost config reload does not reach the validator clients. A key you take out of a mux gets the `[[relays]]` config on the next run. Run one `apply` at a time: each run writes every key and the last write wins, so two overlapping runs with different configs can leave a mix of both while both exit `0`. + +Run it from the directory you run Commit-Boost from, so a relative file `loader` path names the same file; a `mux : keys` count you do not expect means it read a different or stale keys file. It never contacts a relay, so it needs none of the relays' `headers` secrets. It sends each validator client's token with every request, so use an `https://` keymanager URL unless it reaches the client over loopback, as from the same host or a sidecar in its pod. For a keymanager API whose certificate comes from a private certificate authority, set `SSL_CERT_FILE` to a bundle that holds it and the public roots your URL and registry loaders need. + +Each write replaces the key's whole builder config. To keep entries another tool wrote, add `--preserve-entries`: it keeps every stored entry whose `url` is not the advertised URL. After changing `--advertised-url`, the entries at the old URL stay. To remove them, run once without `--preserve-entries`, which also removes the other tools' entries, then write those again. A kept entry is written back with the values the validator client filled in for it, so it does not follow later changes to the key's top-level values or the client's settings. A key with no builder config of its own gets a copy of the client's global builders, the ones it uses for every key without its own config. + +To read back what a client stored for a key: + +```bash +printf 'Authorization: Bearer %s' "$(cat )" | \ + curl -H @- /eth/v1/validator//builder_config +``` + +The written config sets every value, so your validator client's own builder defaults never apply to these keys. Left unset, each defaults to the most profitable choice: + +- `builder_boost_factor` is `100`, so a builder's bid is weighed against the local block at its full value. This overrides a validator client default that favours the local block. +- `min_bid` is `0`, so the beacon node takes any bid that beats its local block. +- The cap is unclamped, `18446744073709551615`, so the beacon node counts a builder's whole execution payment toward its bid. That trusts each builder to pay what it promised, since the protocol does not enforce that payment. Set a cap in Gwei in `[pbs]` or on a relay entry to limit it. + +Where each builder config value comes from: + +| Builder config field | Source | +|---|---| +| entry `url` | `--advertised-url` | +| entry `auth_data` | The hostname of the relay's `url`. Builders on one host share one entry, so they need the same cap | +| entry `max_execution_payment` | The relay entry's `max_execution_payment_gwei`, in `[[mux.relays]]` or `[[relays]]`, else the `[pbs]` one, else unclamped. `"unclamped"` sets that explicitly | +| entry `min_bid` | For a mux key, the mux `min_bid_eth`; else `[pbs] min_bid_eth`, else `0`; rounded down to whole Gwei | +| entry `builder_boost_factor` | For a mux key, the mux `builder_boost_factor`; else `100` | +| top-level `min_bid`, `builder_boost_factor` | `[pbs]` `min_bid_p2p_eth` and `builder_boost_factor_p2p`, one setting for every key, else the entry values. They govern bids received over p2p | +| entry `builder_pubkeys` | Always empty, so any builder's bid is accepted | + +#### With Docker {#builder-config-with-docker} + +`builder-config` runs from the Commit-Boost image, `ghcr.io/commit-boost/commit-boost:`, whose entrypoint is `commit-boost`. Use the tag `cb_pbs` runs, which `docker inspect -f '{{.Config.Image}}' cb_pbs` prints. + +With the validator client on the host, run `apply` from the directory that holds `cb.docker-compose.yml`, so the config and any keys file are the ones Commit-Boost uses: + +```bash +docker run --rm --network host --user "$(sudo stat -c %u:%g )" \ + --mount type=bind,src="$PWD",dst=/cb,readonly -w /cb \ + --mount type=bind,src=,dst=/token,readonly \ + ghcr.io/commit-boost/commit-boost: builder-config apply --config cb-config.toml \ + --advertised-url http://127.0.0.1:18550 --vc http://127.0.0.1:5062=/token +``` + +- `--network host` lets it reach a validator client listening on the host's `127.0.0.1`; in a bridge network, `127.0.0.1` is the container itself, and `docker compose run` cannot change that. +- `--user` runs it as the token file's owner, the only user who can read it. `sudo` lets `stat` see the file inside the validator client's private data directory; without it `--user` is empty and `apply` cannot read the token. +- `--mount` refuses a source that does not exist, where `-v` would create an empty directory in its place. +- `-w /cb` makes the config's relative paths resolve in the mounted directory. An absolute `loader` path must exist inside the container too. +- `--advertised-url` is the URL your beacon node's builder flag uses: `http://127.0.0.1:` for a beacon node on the host and the Commit-Boost that `commit-boost init` sets up. `http://cb_pbs:18550` works only inside the compose network. + +With the validator client in another container, such as a packaged stack's, print inside `cb_pbs`, where the config and keys files already are, and apply in a container that joins the client's network: + +```bash +docker exec cb_pbs commit-boost builder-config print --advertised-url \ + | docker run -i --rm --network container: --user "$(docker exec id -u)" \ + --mount type=volume,src=,dst=/vc,readonly \ + ghcr.io/commit-boost/commit-boost: builder-config apply --from - \ + --advertised-url --vc http://127.0.0.1:=/vc/ +``` + +Mount only the volume that holds the token, not the client's keystores. A packaged stack may name its Commit-Boost container something other than `cb_pbs`. + +#### On Kubernetes {#builder-config-on-kubernetes} + +A Kubernetes deployment usually serves a validator client's keymanager API only inside its pod, and the client often writes its own token. Run `apply` as a sidecar in the validator client's pod, from the Commit-Boost image: + +- Point `--vc` at `http://127.0.0.1:` and at the token file on the client's data volume, mounted read-only. The file is readable only by its owner, so set the sidecar's `securityContext.runAsUser` to the client's user. A token you supply from a Secret works too, but not mounted read-only at Lodestar's `--keymanager.tokenFile`, which Lodestar rewrites at every start. +- Do not make it an init container the validator client waits for: `apply` exits at once when the keymanager API is not listening, so the pod never starts. A native sidecar, an init container with `restartPolicy: Always`, works as long as it has no `startupProbe`, which would hold the client back the same way. +- The image's entrypoint is `commit-boost` and its default argument `pbs`. Give the sidecar a `command` that runs `commit-boost builder-config apply` in a loop and stays up between runs: with `args` alone it exits after one run and restarts in CrashLoopBackOff. The API can come up later than the sidecar, so retry until it exits `0`. Run it again after any failed run, when the Commit-Boost config or a keys file changes, and on a timer, such as every epoch, which also covers keys the client gains and a client that restarts with an empty store. A pass over 500 keys takes under a second. A shell loop running as PID 1 must trap `SIGTERM` and sleep with `sleep & wait $!`, since the shell runs a trap only once its foreground command ends; otherwise the pod stays in Terminating until its grace period ends. +- A sidecar sees one validator client, so pass `--partial`. Its slashing check then covers only that client, so also run `apply` over every client without `--partial`, from somewhere that reaches each keymanager API, such as through `kubectl port-forward` with each pod's token. +- Mount the Commit-Boost config, and any keys file a mux loader names, from the same source as Commit-Boost's, at the paths Commit-Boost uses, and set any `CB_MUX_PATH_` Commit-Boost's container sets on the sidecar too. The image works in `/`, so write `loader` paths as absolute paths. A ConfigMap cannot be mounted from another namespace, and a copy that drifts writes builders Commit-Boost does not route. Set `CB_CONFIG` on the sidecar, or pass `--config`. A URL or registry loader runs in the sidecar, so the pod needs egress to its URL or `rpc_url`. +- Or render with `print` in CI into a ConfigMap and run `apply --from --partial` in the sidecar, so the pod needs no Commit-Boost config, keys file or loader access. Render again on every config change, and on a schedule for a registry loader's keys. + +`--advertised-url` goes into every key's builder config as given, so use the address your beacon nodes resolve: a Service name such as `http://commit-boost..svc:18550`, or `http://localhost:18550` when Commit-Boost runs in each beacon node's pod. Every beacon node a validator client fails over to must resolve it. Commit-Boost replicas behind one Service share its URL, since Commit-Boost keeps no state between a bid request and its block. With `--preserve-entries`, `apply` keeps every stored entry whose parsed URL differs from `--advertised-url`, so keep one spelling of the Service name. If some beacon nodes reach Commit-Boost at a different address, run `apply` once for each address, with `--partial` and the validator clients whose beacon nodes use it. After the address changes, run once without `--preserve-entries`, or each key keeps an entry at the old address. + ## 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. @@ -82,7 +307,15 @@ Either form may end in `?` and form-encoded parameters for the builder, such as 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. +Commit-Boost never dials for a request that came from another Commit-Boost, so chained Commit-Boosts route only through relay entries. It answers `400` to auth data that names Commit-Boost itself ([missing builder config](#missing-builder-config)). + +## Keys with a missing or stale builder config {#missing-builder-config} + +A key with no builder config sends Commit-Boost's hostname as its `auth_data`, and Prysm sends Commit-Boost's whole URL for an entry without `auth_data`. Commit-Boost tells these apart by the request's `Host` header, a hostname at any port or a URL at its host and port, answers `400` at once, and logs a warning naming the key. A key whose `auth_data` names one of Commit-Boost's relays, but not one of the key's own relays, has a stale config: Commit-Boost logs a warning and dials that relay as a builder outside your config, without its `headers`. Both mean the key needs its builder config written again with [`commit-boost builder-config`](#builder-config-command) or your tooling. + +Commit-Boost names up to 5 such keys an epoch (32 slots, counted from its start), with their mux and `auth_data`, and logs how many more there were when the epoch ends. Preferences arrive about an epoch before the slot, so the warning comes only minutes before the proposal; to catch it sooner, read a key back after writing it. A key whose validator client names no builder at all never reaches Commit-Boost, and Nimbus does not send its keys' builder config, so neither shows here. + +Neither warning catches every miss. A key moved to a mux whose relays have the same hostnames routes as before and is not reported, while its validator client keeps the old mux's values. A key whose `auth_data` names a relay you removed counts as `dial` and is dialed as a builder outside your config. Reading every key back on a timer and comparing it with its body in `print`'s document is the only check that finds these, and a validator client that lost its builder config. ## Timing @@ -100,17 +333,40 @@ With [metrics](./running/metrics.md) enabled, the ePBS endpoints use the `endpoi |---|---| | 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 | +| Do keys' builder configs name their relays? | `cb_pbs_auth_data_route_total`, by `endpoint` and `outcome`: `relay` for one of the key's relays, `dial` for auth data Commit-Boost dials, including dials it then refuses, and `no_config` and `stale` for a [missing or stale builder config](#missing-builder-config). | | 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 | +Every `cb_pbs_auth_data_route_total` series starts at `0`, so this alerts on the first key with a missing or stale builder config: + +```promql +increase(cb_pbs_auth_data_route_total{outcome=~"no_config|stale"}[1h]) > 0 +``` + +If you add no builders outside your config, alert on `outcome="dial"` too: a key whose config names a relay you removed is dialed, not reported as stale. + ## Troubleshooting | Symptom | Cause | Fix | |---|---|---| | 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) | +| Bid or preferences requests get `400` "auth.message.data names Commit-Boost itself", and Commit-Boost warns "auth data names Commit-Boost itself" | The key has no builder config, or an entry without `auth_data` | Write the key's builder config with [`commit-boost builder-config`](#builder-config-command) or your tooling | +| Commit-Boost warns "auth data names another of Commit-Boost's relays" | The key's builder config is stale: it moved to a mux with other relays, or got the `[[relays]]` config while in a mux | Write the key's builder config again | +| 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) | 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` | +| `builder-config` stops with "unknown keys in the Commit-Boost config" | A misspelled `[pbs]` or `[[mux]]` key, or a `[pbs]` key that a custom PBS module reads | Fix each misspelled key. For a custom module's keys, run `builder-config` on a copy of the config without them | +| `builder-config` exits `2` | It stopped before contacting a validator client. Its `ERROR:` line, or clap's `error:` line for a flag clap rejects, names the flag, token file, config or mux loader | Fix what the line names | +| `builder-config apply` fails with "key listing failed" and `Connection refused` | The validator client's keymanager API is off, or listens at another address | Start the client with its keymanager API flag ([the client table](#builder-config-apply)) and check the `--vc` URL | +| `builder-config apply` fails with "key listing failed" and `401` or `403` | The token does not match the validator client's | Point `--vc` at the client's current token file | +| `builder-config apply` fails with "no validator client lists a key" | Every validator client given with `--vc` was listed, and none holds a key | Point `--vc` at the keymanager APIs of the validator clients that hold your keys. With `--partial`, this is a warning | +| `builder-config apply` fails with "keys in a mux that no validator client lists" | No validator client given with `--vc` holds the keys the error lists, which are in a mux's `validator_pubkeys` or keys file. The error also names any client that could not be listed and may hold them | Fix that client's error, add a `--vc` for the client that holds the keys, pass `--partial` if another run covers those clients, or take the keys out of the mux | +| `builder-config apply` fails with "a slashing risk" | More than one validator client given with `--vc` holds the keys the error lists, and it names the clients; `apply` has still written each key's builder config to each that accepted it. Two clients with the same keys are usually a migration or a failover left running | Stop the duplicate validator client, or remove the keys from every client but one, then run `apply` again. `apply` refuses one client given twice, such as `localhost` and `127.0.0.1` on one port | +| `builder-config apply` reports "builder_config probe failed: 501" | Prysm answers `501` until Gloas is scheduled, and while it is not ready | Run `apply` again once the fork is scheduled | +| `builder-config apply` reports "no builder_config support" | The validator client has no builder config endpoint: an older release, or Teku or Grandine | Upgrade it, or leave it out of `--vc` | +| `builder-config apply` reports "writes in a row got no answer" | The validator client hung or dropped the connections, so `apply` stopped writing to it | Check the validator client, then run `apply` again | +| `builder-config apply` reports that a validator client "answered 403 before any write landed" | Lodestar runs with `--proposerSettingsFile`, which takes over its builder config | Move off the settings file ([proposer settings files](#validator-builder-config)), or leave the client out of `--vc` | +| `builder-config apply` reports that a validator client "refuses a max_execution_payment above 0" | Lodestar's validator client refuses a cap above 0 unless it runs with `--allowDangerousTrustedPayments`, and the default cap is unclamped. `apply` writes no further key with a cap above 0 to that client. A refused key keeps its old builder config; one that had none gets no bids through Commit-Boost | Restart the validator client with `--allowDangerousTrustedPayments`, then run `apply` again | diff --git a/docs/docs/get_started/running/binary.md b/docs/docs/get_started/running/binary.md index 006e14b0b..521517775 100644 --- a/docs/docs/get_started/running/binary.md +++ b/docs/docs/get_started/running/binary.md @@ -71,4 +71,8 @@ Or for the Signer service: CB_CONFIG=./cb-config.toml CB_JWTS="MY_MODULE=" CB_SIGNER_ADMIN_JWT="" commit-boost signer ``` +:::warning ePBS +From the Gloas fork, a validator key gets no bids through Commit-Boost until its builder config points at Commit-Boost. [`commit-boost builder-config`](../epbs.md#builder-config-command) writes it; run it again after changing the relays or muxes, since a config reload does not reach the validator clients. +::: + For a worked signer startup, see [Verifying the Signer Service](../building.md#verifying-the-signer-service). diff --git a/docs/docs/get_started/running/docker.md b/docs/docs/get_started/running/docker.md index f297d6494..a4f842dc1 100644 --- a/docs/docs/get_started/running/docker.md +++ b/docs/docs/get_started/running/docker.md @@ -32,6 +32,10 @@ This will run all the configured services, including PBS, signer and commit modu The MEV-Boost server will be exposed at `pbs.port` from the config, `18550` in our example. You'll need to point your CL/Validator client to this port to be able to source blocks from the builder market. +:::warning ePBS +From the Gloas fork, a validator key gets no bids through Commit-Boost until its builder config points at Commit-Boost. [`commit-boost builder-config`](../epbs.md#builder-config-command) writes it; run it again after changing the relays or muxes, since a config reload does not reach the validator clients. +::: + ## Logs To check the logs, run: ```bash @@ -111,7 +115,7 @@ This will run the PBS service in a container named `cb_pbs`. The program creates a read-only volume binding for the config file, which the PBS service needs to run. The Docker compose file that it creates with the `init` command, `cb.docker-compose.yml`, will be placed into your current working directory when you run the program. The volume source will be specified as a *relative path* to that working directory, so it's ideal if the config file is directly within your working directory (or a subdirectory). If you need to specify an absolute path for the config file, you can adjust the `volumes` entry within the Docker compose file manually after its creation. -Since this is a volume, the PBS service sees changes to the file: the stock PBS image watches the config file and [automatically reloads the configuration](../configuration.md#automatic-reload-pbs-only) whenever the file is modified, without needing a restart. That means you can change the file any time after the Docker compose file is created to tweak PBS's parameters, but it also means the config file must stay in the same location; if you move it, the PBS container won't be able to mount it anymore and will fail to start unless you manually adjust the volume's source location. +Since this is a volume, the PBS service sees changes to the file: the stock PBS image watches the config file and [automatically reloads the configuration](../configuration.md#automatic-reload-pbs-only) whenever its contents change, without needing a restart. The compose file mounts the file itself, so edit it in place: a tool that replaces the file by a rename, such as `sed -i`, leaves the container on the old file until it is recreated. That means you can change the file any time after the Docker compose file is created to tweak PBS's parameters, but it also means the config file must stay in the same location; if you move it, the PBS container won't be able to mount it anymore and will fail to start unless you manually adjust the volume's source location. Custom PBS images may not auto-reload; see [Extending PBS](../../developing/extending-pbs.md#entry-point). diff --git a/docs/docs/get_started/running/k8s.md b/docs/docs/get_started/running/k8s.md index c71201d3e..07a8861ec 100644 --- a/docs/docs/get_started/running/k8s.md +++ b/docs/docs/get_started/running/k8s.md @@ -12,6 +12,10 @@ Commit-Boost can be deployed on Kubernetes using the [Helm chart](https://helm.s The current Helm chart supports only the **PBS Service**. It does **not** support the Signer Service or custom commit modules. If you need Signer or module support, please use the [Docker](./docker.md) or [Binary](./binary.md) deployment methods instead. ::: +:::warning ePBS +From the Gloas fork, a validator key gets no bids through Commit-Boost until its builder config points at Commit-Boost. [`commit-boost builder-config`](../epbs.md#builder-config-on-kubernetes) writes it, from a sidecar in each validator client's pod. +::: + ## Prerequisites - A Kubernetes cluster diff --git a/docs/docs/get_started/running/metrics-catalog.md b/docs/docs/get_started/running/metrics-catalog.md index 4db35ea41..ba3d2bd97 100644 --- a/docs/docs/get_started/running/metrics-catalog.md +++ b/docs/docs/get_started/running/metrics-catalog.md @@ -22,6 +22,7 @@ PBS metrics use a custom Prometheus registry with namespace prefix `cb_pbs_`. Th | `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_auth_data_route_total` | Counter | `endpoint`, `outcome` | (unreleased, from v0.12.0-rc1) How each ePBS bid or preferences request's auth data routed. Incremented once per request routed by its auth data. Outcomes: `relay` (one of the key's relays), `dial` (auth data Commit-Boost dials, including dials it then refuses), `stale` (another of Commit-Boost's relays, which is then dialed) and `no_config` (Commit-Boost itself, answered `400`). Endpoint values: `get_execution_payload_bid`, `submit_builder_preferences`. | | `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/src/utils.rs b/tests/src/utils.rs index 1839b3c9c..589264d03 100644 --- a/tests/src/utils.rs +++ b/tests/src/utils.rs @@ -76,6 +76,7 @@ fn mock_relay_config(port: u16, pubkey: BlsPublicKey) -> Result { target_first_request_ms: None, frequency_get_header_ms: None, validator_registration_batch_size: None, + max_execution_payment_gwei: None, }) } @@ -148,6 +149,9 @@ pub fn get_pbs_config(port: u16) -> PbsConfig { register_validator_retry_limit: u32::MAX, validator_registration_batch_size: None, mux_registry_refresh_interval_seconds: 5, + max_execution_payment_gwei: None, + min_bid_p2p_wei: None, + builder_boost_factor_p2p: None, } } diff --git a/tests/tests/pbs_cfg_file_update.rs b/tests/tests/pbs_cfg_file_update.rs index c4c02ce17..38faa51ca 100644 --- a/tests/tests/pbs_cfg_file_update.rs +++ b/tests/tests/pbs_cfg_file_update.rs @@ -8,7 +8,7 @@ use cb_common::{ }, pbs::RelayEntry, signer::random_secret, - types::Chain, + types::{BlsPublicKey, Chain}, }; use cb_pbs::{DefaultBuilderApi, PbsService, PbsState}; use cb_tests::{ @@ -75,6 +75,9 @@ async fn test_cfg_file_update() -> Result<()> { register_validator_retry_limit: 3, validator_registration_batch_size: None, mux_registry_refresh_interval_seconds: 384, + max_execution_payment_gwei: None, + min_bid_p2p_wei: None, + builder_boost_factor_p2p: None, }; let cb_config = CommitBoostConfig { chain, @@ -97,6 +100,7 @@ async fn test_cfg_file_update() -> Result<()> { headers: None, target_first_request_ms: None, validator_registration_batch_size: None, + max_execution_payment_gwei: None, entry: RelayEntry { id: relay1.id.to_string(), url: Url::parse(&format!("http://localhost:{relay1_port}"))?, @@ -150,6 +154,7 @@ async fn test_cfg_file_update() -> Result<()> { headers: None, target_first_request_ms: None, validator_registration_batch_size: None, + max_execution_payment_gwei: None, entry: RelayEntry { id: relay2_id, url: Url::parse(&format!("http://{pubkey}@localhost:{relay2_port}"))?, @@ -173,3 +178,120 @@ async fn test_cfg_file_update() -> Result<()> { Ok(()) } + +/// The relay entry a config file names for the mock relay on `port` +fn relay_config(port: u16, pubkey: &BlsPublicKey) -> Result { + let id = format!("mock_{port}"); + Ok(RelayConfig { + id: Some(id.clone()), + enable_timing_games: false, + frequency_get_header_ms: None, + get_params: None, + get_header: GetHeaderTransport::Http, + headers: None, + target_first_request_ms: None, + validator_registration_batch_size: None, + max_execution_payment_gwei: None, + entry: RelayEntry { + id, + url: Url::parse(&format!("http://{pubkey}@localhost:{port}"))?, + pubkey: pubkey.clone(), + }, + }) +} + +/// A Kubernetes ConfigMap update swaps the `..data` symlink the config file +/// points through. Every update reloads the configuration, not only the first, +/// and one that fails to load, as an unreachable rpc_url does, leaves the next +#[cfg(unix)] +#[tokio::test] +async fn test_cfg_configmap_updates() -> Result<()> { + use std::os::unix::fs::symlink; + + setup_test_env(); + let signer = random_secret(); + let pubkey = signer.public_key(); + let chain = Chain::Hoodi; + let pbs_listener = get_free_listener().await; + let pbs_port = pbs_listener.local_addr()?.port(); + + let mut relays = Vec::new(); + for _ in 0..2 { + let listener = get_free_listener().await; + let port = listener.local_addr()?.port(); + let state = Arc::new(MockRelayState::new(chain, signer.clone())); + tokio::spawn(start_mock_relay_service_with_listener(state.clone(), listener)); + relays.push((port, state)); + } + + // An rpc_url nothing listens on, which the config's validation checks + let closed = get_free_listener().await; + let rpc_url = Url::parse(&format!("http://{}", closed.local_addr()?))?; + drop(closed); + + // Writes a config naming one relay to `..v{version}` and points `..data` at it + let dir = tempfile::tempdir()?; + let write_version = |version: usize, relay: usize, rpc_url: Option| -> Result<()> { + let cb_config = CommitBoostConfig { + chain, + pbs: StaticPbsConfig { + docker_image: "cb-fake-repo/cb-fake-image:latest".to_string(), + // TOML integers stop at i64::MAX + pbs_config: PbsConfig { + timeout_get_header_ms: 950, + timeout_get_payload_ms: 4000, + timeout_register_validator_ms: 3000, + late_in_slot_time_ms: u64::MAX / 2, + rpc_url, + http_timeout_seconds: 1, + ..get_pbs_config(pbs_port) + }, + with_signer: false, + }, + muxes: None, + modules: None, + signer: None, + logs: LogsSettings::default(), + metrics: None, + relays: vec![relay_config(relays[relay].0, &pubkey)?], + }; + let path = dir.path().join(format!("..v{version}")); + std::fs::create_dir(&path)?; + std::fs::write(path.join("cb-config.toml"), toml::to_string_pretty(&cb_config)?)?; + let tmp = dir.path().join("..data_tmp"); + symlink(format!("..v{version}"), &tmp)?; + std::fs::rename(&tmp, dir.path().join("..data"))?; + Ok(()) + }; + write_version(0, 0, None)?; + let config_path = dir.path().join("cb-config.toml"); + symlink("..data/cb-config.toml", &config_path)?; + + let first = generate_mock_relay(relays[0].0, pubkey.clone())?; + let config = to_pbs_config(chain, get_pbs_config(pbs_port), vec![first]); + let state = PbsState::new(config, config_path); + tokio::spawn(PbsService::run_with_listener::<(), DefaultBuilderApi>(state, pbs_listener)); + tokio::time::sleep(Duration::from_millis(1000)).await; + + let mock_validator = MockValidator::new(pbs_port)?; + let received = + || relays.iter().map(|(_, state)| state.received_get_header()).collect::>(); + let res = mock_validator.do_get_header(None, Vec::new(), ForkName::Fulu).await?; + assert_eq!(res.status(), StatusCode::OK); + assert_eq!(received(), [1, 0]); + + // Relay 2; relay 1 with an rpc_url that fails, so relay 2 stays; relay 1. + // A watch on the file itself misses the second update + for (version, relay, rpc_url, expected) in + [(1, 1, None, [1, 1]), (2, 0, Some(rpc_url.clone()), [1, 2]), (3, 0, None, [2, 2])] + { + write_version(version, relay, rpc_url)?; + std::fs::remove_dir_all(dir.path().join(format!("..v{}", version - 1)))?; + tokio::time::sleep(Duration::from_millis(1000)).await; + let res = mock_validator.do_get_header(None, Vec::new(), ForkName::Fulu).await?; + assert_eq!(res.status(), StatusCode::OK); + assert_eq!(received(), expected, "update {version}"); + } + + Ok(()) +} diff --git a/tests/tests/pbs_get_execution_payload_bid.rs b/tests/tests/pbs_get_execution_payload_bid.rs index 1d58cf52f..cdb77e5eb 100644 --- a/tests/tests/pbs_get_execution_payload_bid.rs +++ b/tests/tests/pbs_get_execution_payload_bid.rs @@ -569,8 +569,10 @@ async fn test_get_execution_payload_bid_relay_timing_headers() -> Result<()> { 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 +/// Auth data naming Commit-Boost as the request reached it is a key with no +/// builder config: 400, with nothing dialed. Under another name it 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 @@ -578,12 +580,23 @@ async fn test_get_execution_payload_bid_dial_self_400() -> Result<()> { 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"); + let port = own.port().unwrap(); + for (host, message) in [ + ( + own.host_str().unwrap(), + "Invalid SignedBuilderRequestAuth: auth.message.data names Commit-Boost itself; the key's builder config names no relay", + ), + // Commit-Boost listens on 0.0.0.0, which 127.0.0.1 reaches under a name + // other than the request's Host + ("127.0.0.1", "The addressed builder rejected the request with status 400"), + ] { + let auth_data = format!("http://{host}:{port}/"); + let res = + get_json_bid(&mock_validator, &opaque_auth(auth_data.as_bytes(), TEST_SLOT)).await?; + assert_eq!(res.status(), StatusCode::BAD_REQUEST, "{host}"); + let body: serde_json::Value = serde_json::from_slice(&res.bytes().await?)?; + assert_eq!(body["message"], message, "{host}"); + } assert_eq!(cfg_state.received_execution_payload_bid(), 0); Ok(()) } diff --git a/tests/tests/pbs_mux.rs b/tests/tests/pbs_mux.rs index 6f01f502d..36ddf5102 100644 --- a/tests/tests/pbs_mux.rs +++ b/tests/tests/pbs_mux.rs @@ -384,6 +384,8 @@ async fn test_ssv_multi_with_node() -> Result<()> { relays: vec![(*relay.config).clone()], timeout_get_header_ms: Some(u64::MAX - 1), validator_pubkeys: vec![], + builder_boost_factor: None, + min_bid_wei: None, }], }; @@ -491,6 +493,8 @@ async fn test_ssv_multi_with_public() -> Result<()> { relays: vec![(*relay.config).clone()], timeout_get_header_ms: Some(u64::MAX - 1), validator_pubkeys: vec![], + builder_boost_factor: None, + min_bid_wei: None, }], }; diff --git a/tests/tests/pbs_mux_refresh.rs b/tests/tests/pbs_mux_refresh.rs index 10a9f1c87..e1f240684 100644 --- a/tests/tests/pbs_mux_refresh.rs +++ b/tests/tests/pbs_mux_refresh.rs @@ -93,6 +93,8 @@ async fn test_auto_refresh() -> Result<()> { relays: vec![(*mux_relay.config).clone()], timeout_get_header_ms: Some(u64::MAX - 1), validator_pubkeys: vec![], + builder_boost_factor: None, + min_bid_wei: None, }], };