diff --git a/Cargo.lock b/Cargo.lock index 8d514cf2a96..873e5dd518a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9887,6 +9887,7 @@ dependencies = [ "axum_utils", "beacon_node_fallback", "bls", + "builder_store", "deposit_contract", "directory", "dirs", diff --git a/book/src/gloas_builder_config.md b/book/src/gloas_builder_config.md index 4709885b7e1..605ffdf1f73 100644 --- a/book/src/gloas_builder_config.md +++ b/book/src/gloas_builder_config.md @@ -5,12 +5,14 @@ The validator client reads its external-builder settings from a YAML file named `builder_definitions.yml` in the validator directory -(`/validators/builder_definitions.yml`). The file holds two things: +(`/validators/builder_definitions.yml`). The file contains: - **A global bid policy** — `min_bid` and `builder_boost_factor`, applied to bids received over p2p (gossip) and used as the default for any builder that does not set its own. - **A list of builders** to request bids from directly, each with optional per-builder overrides of the global policy. +- **Per-validator configurations** under `validator_configs`, managed through the standard keymanager + API. Each map key is a validator public key. ## Example @@ -35,11 +37,16 @@ builders: builder_pubkeys: # optional — reject a bid not signed by one of these keys - "0xa1b2c3d4..." # auth_data: "0x68747470..." # optional — defaults to the UTF-8 bytes of `url` + +# Optional per-validator configuration. +# validator_configs: +# "0x": +# min_bid: 500000000 +# builders: [] # explicitly disable direct builders for this validator ``` -> **Comments are not preserved.** The validator client rewrites this file when builders are added or -> removed (for example via the keymanager API), which strips YAML comments. Keep an annotated copy -> elsewhere if you rely on inline notes. +> **Comments are not preserved.** The validator client rewrites this file when builder settings +> change through the keymanager API. Keep an annotated copy elsewhere if you rely on inline notes. ## Fields @@ -50,6 +57,7 @@ builders: | `min_bid` | no | `0` | Minimum total payment, in gwei, for a p2p bid. A bid below the floor is ranked behind any floor-clearing candidate (including the local block) and only wins when nothing else is viable. Also the default `min_bid` for any builder that omits it. | | `builder_boost_factor` | no | `100` | Percentage multiplier applied to p2p bids when comparing against the local block. Also the default for any builder that omits it. | | `builders` | no | `[]` | The list of builders to request bids from directly. | +| `validator_configs` | no | `{}` | Builder settings for individual validators. Omitted fields use global values. An empty `builders` list uses no direct builders. | ### Per builder (each entry under `builders`) diff --git a/common/eth2/src/lighthouse_vc/http_client.rs b/common/eth2/src/lighthouse_vc/http_client.rs index 3c850fcb052..297635c2f75 100644 --- a/common/eth2/src/lighthouse_vc/http_client.rs +++ b/common/eth2/src/lighthouse_vc/http_client.rs @@ -494,6 +494,18 @@ impl ValidatorClientHttpClient { Ok(url) } + fn make_builder_config_url(&self, pubkey: &PublicKeyBytes) -> Result { + let mut url = self.server.expose_full().clone(); + url.path_segments_mut() + .map_err(|()| Error::InvalidUrl(self.server.clone()))? + .push("eth") + .push("v1") + .push("validator") + .push(&pubkey.to_string()) + .push("builder_config"); + Ok(url) + } + fn make_graffiti_url(&self, pubkey: &PublicKeyBytes) -> Result { let mut url = self.server.expose_full().clone(); url.path_segments_mut() @@ -603,6 +615,33 @@ impl ValidatorClientHttpClient { self.delete_with_raw_response(url, &()).await } + /// `GET /eth/v1/validator/{pubkey}/builder_config` + pub async fn get_builder_config( + &self, + pubkey: &PublicKeyBytes, + ) -> Result { + let url = self.make_builder_config_url(pubkey)?; + self.get(url) + .await + .map(|generic: GenericResponse| generic.data) + } + + /// `POST /eth/v1/validator/{pubkey}/builder_config` + pub async fn post_builder_config( + &self, + pubkey: &PublicKeyBytes, + request: &BuilderConfig, + ) -> Result { + let url = self.make_builder_config_url(pubkey)?; + self.post_with_raw_response(url, request).await + } + + /// `DELETE /eth/v1/validator/{pubkey}/builder_config` + pub async fn delete_builder_config(&self, pubkey: &PublicKeyBytes) -> Result { + let url = self.make_builder_config_url(pubkey)?; + self.delete_with_raw_response(url, &()).await + } + /// `GET /eth/v1/validator/{pubkey}/gas_limit` pub async fn get_gas_limit( &self, diff --git a/common/eth2/src/lighthouse_vc/std_types.rs b/common/eth2/src/lighthouse_vc/std_types.rs index c54252b9e33..987556760d3 100644 --- a/common/eth2/src/lighthouse_vc/std_types.rs +++ b/common/eth2/src/lighthouse_vc/std_types.rs @@ -1,9 +1,39 @@ use bls::PublicKeyBytes; +pub use builder_types::{BuilderUrl, RequestAuthData}; use eth2_keystore::Keystore; -use serde::{Deserialize, Serialize}; +use serde::{Deserialize, Deserializer, Serialize, de}; +pub use serde_utils::quoted_u64::Quoted; use types::{Address, Graffiti}; use zeroize::Zeroizing; +fn deserialize_present<'de, D, T>(deserializer: D) -> Result, D::Error> +where + D: Deserializer<'de>, + T: Deserialize<'de>, +{ + Option::::deserialize(deserializer)? + .map(Some) + .ok_or_else(|| de::Error::custom("null is not allowed")) +} + +fn deserialize_keymanager_u64<'de, D>(deserializer: D) -> Result>, D::Error> +where + D: Deserializer<'de>, +{ + let value = String::deserialize(deserializer)?; + let is_canonical = value == "0" + || (value.len() <= 20 + && value.as_bytes().first().is_some_and(|byte| *byte >= b'1') + && value.as_bytes().iter().all(u8::is_ascii_digit)); + if !is_canonical { + return Err(de::Error::custom("invalid quoted uint64")); + } + value + .parse() + .map(|value| Some(Quoted { value })) + .map_err(de::Error::custom) +} + pub use eip_3076::Interchange; #[derive(Debug, Deserialize, Serialize, PartialEq)] @@ -20,6 +50,139 @@ pub struct GetGasLimitResponse { pub gas_limit: u64, } +/// Per-validator external-builder configuration from the standard keymanager API. +/// +/// A missing field inherits the validator client's global configuration. The GET endpoint returns +/// all fields resolved, while POST accepts an omitted `builders` field and an explicitly empty +/// list as distinct values. +#[derive(Debug, Clone, Default, PartialEq, Deserialize, Serialize)] +pub struct BuilderConfig { + #[serde( + default, + skip_serializing_if = "Option::is_none", + deserialize_with = "deserialize_keymanager_u64" + )] + pub min_bid: Option>, + #[serde( + default, + skip_serializing_if = "Option::is_none", + deserialize_with = "deserialize_keymanager_u64" + )] + pub builder_boost_factor: Option>, + #[serde( + default, + skip_serializing_if = "Option::is_none", + deserialize_with = "deserialize_present" + )] + pub builders: Option>, +} + +/// Request authentication data encoded as `0x`-prefixed hex in the keymanager API. +#[derive(Debug, Clone, PartialEq, Deserialize, Serialize)] +#[serde(transparent)] +pub struct HexRequestAuthData( + #[serde(with = "ssz_types::serde_utils::hex_var_list")] pub RequestAuthData, +); + +/// An external-builder entry from the standard keymanager API. +#[derive(Debug, Clone, PartialEq, Deserialize, Serialize)] +pub struct BuilderEntry { + pub url: BuilderUrl, + #[serde( + default, + skip_serializing_if = "Option::is_none", + deserialize_with = "deserialize_present" + )] + pub auth_data: Option, + #[serde( + default, + skip_serializing_if = "Option::is_none", + deserialize_with = "deserialize_present" + )] + pub builder_pubkeys: Option>, + #[serde( + default, + skip_serializing_if = "Option::is_none", + deserialize_with = "deserialize_keymanager_u64" + )] + pub max_execution_payment: Option>, + #[serde( + default, + skip_serializing_if = "Option::is_none", + deserialize_with = "deserialize_keymanager_u64" + )] + pub min_bid: Option>, + #[serde( + default, + skip_serializing_if = "Option::is_none", + deserialize_with = "deserialize_keymanager_u64" + )] + pub builder_boost_factor: Option>, +} + +#[cfg(test)] +mod builder_config_tests { + use super::*; + + #[test] + fn builder_config_uses_keymanager_json_encoding() { + let config = BuilderConfig { + min_bid: Some(Quoted { value: 3 }), + builder_boost_factor: Some(Quoted { value: 110 }), + builders: Some(vec![BuilderEntry { + url: "https://builder.example".parse().unwrap(), + auth_data: Some(HexRequestAuthData( + RequestAuthData::new(vec![1, 2]).unwrap(), + )), + builder_pubkeys: Some(vec![]), + max_execution_payment: Some(Quoted { value: 8 }), + min_bid: None, + builder_boost_factor: None, + }]), + }; + + let json = serde_json::to_value(&config).unwrap(); + assert_eq!(json["min_bid"], "3"); + assert_eq!(json["builder_boost_factor"], "110"); + assert_eq!(json["builders"][0]["auth_data"], "0x0102"); + assert_eq!(json["builders"][0]["max_execution_payment"], "8"); + assert_eq!( + json["builders"][0]["builder_pubkeys"], + serde_json::json!([]) + ); + assert_eq!( + serde_json::from_value::(json).unwrap(), + config + ); + assert_eq!( + serde_json::to_value(BuilderConfig::default()).unwrap(), + serde_json::json!({}) + ); + assert!( + serde_json::from_value::(serde_json::json!({"min_bid": 3})).is_err() + ); + for value in ["01", "+1"] { + assert!( + serde_json::from_value::(serde_json::json!({"min_bid": value})) + .is_err() + ); + } + + for field in ["builders", "min_bid", "builder_boost_factor"] { + let json = serde_json::json!({(field): null}); + assert!( + serde_json::from_value::(json).is_err(), + "field {field} unexpectedly accepts null" + ); + } + + let json = serde_json::json!({ + "builders": [{"url": "https://builder.example", "builder_pubkeys": null}] + }); + assert!(serde_json::from_value::(json).is_err()); + } +} + #[derive(Debug, Deserialize, Serialize, PartialEq)] pub struct AuthResponse { pub token_path: String, diff --git a/common/warp_utils/src/reject.rs b/common/warp_utils/src/reject.rs index b88fd79b23f..408551bb9b2 100644 --- a/common/warp_utils/src/reject.rs +++ b/common/warp_utils/src/reject.rs @@ -65,6 +65,15 @@ pub fn custom_bad_request(msg: String) -> warp::reject::Rejection { warp::reject::custom(CustomBadRequest(msg)) } +#[derive(Debug)] +pub struct CustomForbidden(pub String); + +impl Reject for CustomForbidden {} + +pub fn custom_forbidden(msg: String) -> warp::reject::Rejection { + warp::reject::custom(CustomForbidden(msg)) +} + #[derive(Debug)] pub struct CustomDeserializeError(pub String); @@ -194,6 +203,9 @@ pub async fn handle_rejection(err: warp::Rejection) -> Result() { code = StatusCode::BAD_REQUEST; message = format!("BAD_REQUEST: {}", e.0); + } else if let Some(e) = err.find::() { + code = StatusCode::FORBIDDEN; + message = format!("FORBIDDEN: {}", e.0); } else if let Some(e) = err.find::() { code = StatusCode::INTERNAL_SERVER_ERROR; message = format!("INTERNAL_SERVER_ERROR: {}", e.0); diff --git a/validator_client/builder_store/src/builder_definitions.rs b/validator_client/builder_store/src/builder_definitions.rs index 928f6d3ec14..4976a976cb5 100644 --- a/validator_client/builder_store/src/builder_definitions.rs +++ b/validator_client/builder_store/src/builder_definitions.rs @@ -1,11 +1,12 @@ use account_utils::write_file_via_temporary; use bls::PublicKeyBytes; -use builder_types::{BuilderUrl, MAX_BUILDER_ENTRIES, RequestAuthData}; -use serde::{Deserialize, Serialize}; -use std::collections::HashSet; +use builder_types::{BuilderPubkeys, BuilderUrl, MAX_BUILDER_ENTRIES, RequestAuthData}; +use serde::{Deserialize, Deserializer, Serialize, Serializer, de}; +use std::collections::{BTreeMap, HashSet}; use std::fs::{File, create_dir_all}; use std::io; use std::path::{Path, PathBuf}; +use std::str::FromStr; /// The file name for the serialized `BuilderConfigFile` struct. pub const BUILDERS_FILENAME: &str = "builder_definitions.yml"; @@ -35,6 +36,10 @@ pub enum Error { /// More than `MAX_BUILDER_ENTRIES` builders are enabled, exceeding what fits in a /// `BuilderConfig`. TooManyEnabledBuilders { enabled: usize, max: usize }, + /// A builder entry contains more public keys than fits in a `BuilderEntry`. + TooManyBuilderPubkeys(BuilderUrl), + /// A builder entry contains an explicitly empty authentication value. + EmptyAuthData(BuilderUrl), } /// A single builder in the config file: a direct bid request, with optional per-builder overrides @@ -104,6 +109,84 @@ mod serde_option_auth_data { } } +/// A per-validator builder configuration as submitted through the keymanager API. +/// +/// Every field is optional so that an omitted value can inherit from the global configuration. +/// `builders: Some(vec![])` is intentionally different from `builders: None`: the former disables +/// direct builder requests for this validator, while the latter follows the global builder list. +#[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize)] +pub struct ValidatorBuilderConfig { + #[serde(default, skip_serializing_if = "Option::is_none")] + pub min_bid: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub builder_boost_factor: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub builders: Option>, +} + +/// A builder entry in a per-validator configuration. +/// +/// Unlike [`BuilderDefinition`], `max_execution_payment` is optional because the keymanager API +/// allows it to inherit from the validator client's matching global builder definition. +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct ValidatorBuilderDefinition { + pub url: BuilderUrl, + #[serde( + default, + skip_serializing_if = "Option::is_none", + with = "serde_option_auth_data" + )] + pub auth_data: Option, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub builder_pubkeys: Vec, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub max_execution_payment: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub min_bid: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub builder_boost_factor: Option, +} + +/// A fully resolved builder configuration used by the validator client and the HTTP API. +#[derive(Debug, Clone, PartialEq)] +pub struct ResolvedBuilderConfig { + pub min_bid: u64, + pub builder_boost_factor: u64, + pub builders: Vec, +} + +impl ValidatorBuilderConfig { + pub(crate) fn validate(&self) -> Result<(), Error> { + let Some(builders) = &self.builders else { + return Ok(()); + }; + + if builders.len() > MAX_BUILDER_ENTRIES { + return Err(Error::TooManyEnabledBuilders { + enabled: builders.len(), + max: MAX_BUILDER_ENTRIES, + }); + } + + let mut seen_auth_urls = HashSet::new(); + for builder in builders { + validate_builder_definition(&builder.url, &builder.auth_data, &mut seen_auth_urls)?; + if BuilderPubkeys::new(builder.builder_pubkeys.clone()).is_err() { + return Err(Error::TooManyBuilderPubkeys(builder.url.clone())); + } + if builder + .auth_data + .as_ref() + .is_some_and(|data| data.is_empty()) + { + return Err(Error::EmptyAuthData(builder.url.clone())); + } + } + + Ok(()) + } +} + /// The validator client's builder configuration file. /// /// Holds the global bid-policy defaults plus the list of builders to request bids from directly. It @@ -122,6 +205,38 @@ pub struct BuilderConfigFile { /// The builders to request bids from directly. #[serde(default)] pub builders: Vec, + /// Per-validator overrides. The key is the compressed validator public key in hex form. + #[serde( + default, + skip_serializing_if = "BTreeMap::is_empty", + with = "serde_validator_configs" + )] + pub validator_configs: BTreeMap, +} + +mod serde_validator_configs { + use super::*; + + pub fn serialize( + configs: &BTreeMap, + serializer: S, + ) -> Result { + configs.serialize(serializer) + } + + pub fn deserialize<'de, D: Deserializer<'de>>( + deserializer: D, + ) -> Result, D::Error> { + let configs = BTreeMap::::deserialize(deserializer)?; + let mut canonical = BTreeMap::new(); + for (key, config) in configs { + let public_key = PublicKeyBytes::from_str(&key).map_err(de::Error::custom)?; + if canonical.insert(public_key.to_string(), config).is_some() { + return Err(de::Error::custom("duplicate validator public key")); + } + } + Ok(canonical) + } } impl Default for BuilderConfigFile { @@ -130,6 +245,7 @@ impl Default for BuilderConfigFile { min_bid: 0, builder_boost_factor: default_builder_boost_factor(), builders: Vec::new(), + validator_configs: BTreeMap::new(), } } } @@ -205,28 +321,128 @@ impl BuilderConfigFile { continue; } let url = &definition.url; - // Reject malformed or non-http(s) builder URLs here, at config load, rather than - // silently skipping them during block proposal. - let sensitive_url = url - .to_sensitive_url() - .map_err(|_| Error::InvalidBuilderUrl(url.clone()))?; - if !matches!(sensitive_url.expose_full().scheme(), "http" | "https") { - return Err(Error::UnsupportedUrlScheme(url.clone())); - } + validate_builder_definition(url, &definition.auth_data, &mut seen_auth_urls)?; + } - let auth = definition - .auth_data - .clone() - .unwrap_or_else(|| url.to_default_auth_data()); - // two entries cannot contain the same url and auth data - let key = (url.clone(), auth); - if !seen_auth_urls.insert(key) { - return Err(Error::DuplicateBuilderAuth(url.clone())); - } + for config in self.validator_configs.values() { + config.validate()?; } Ok(()) } + + /// Resolve the configuration that applies to `validator_pubkey`. + pub fn resolved_for(&self, validator_pubkey: &PublicKeyBytes) -> ResolvedBuilderConfig { + let validator_config = self.validator_configs.get(&validator_pubkey.to_string()); + let min_bid = validator_config + .and_then(|config| config.min_bid) + .unwrap_or(self.min_bid); + let builder_boost_factor = validator_config + .and_then(|config| config.builder_boost_factor) + .unwrap_or(self.builder_boost_factor); + + let builders = match validator_config.and_then(|config| config.builders.as_ref()) { + Some(builders) => builders + .iter() + .map(|builder| { + self.resolve_validator_builder(builder, min_bid, builder_boost_factor) + }) + .collect(), + None => self + .builders + .iter() + .filter(|builder| { + builder.enabled + && BuilderPubkeys::new(builder.builder_pubkeys.clone()).is_ok() + && !builder + .auth_data + .as_ref() + .is_some_and(|auth_data| auth_data.is_empty()) + }) + .map(|builder| { + let mut builder = builder.clone(); + builder.min_bid = Some(builder.min_bid.unwrap_or(min_bid)); + builder.builder_boost_factor = + Some(builder.builder_boost_factor.unwrap_or(builder_boost_factor)); + builder + }) + .collect(), + }; + + ResolvedBuilderConfig { + min_bid, + builder_boost_factor, + builders, + } + } + + fn resolve_validator_builder( + &self, + builder: &ValidatorBuilderDefinition, + min_bid: u64, + builder_boost_factor: u64, + ) -> BuilderDefinition { + let max_execution_payment = builder + .max_execution_payment + .or_else(|| self.global_max_execution_payment(builder)) + .unwrap_or_default(); + + BuilderDefinition { + enabled: true, + url: builder.url.clone(), + auth_data: builder.auth_data.clone(), + builder_pubkeys: builder.builder_pubkeys.clone(), + max_execution_payment, + min_bid: Some(builder.min_bid.unwrap_or(min_bid)), + builder_boost_factor: Some( + builder.builder_boost_factor.unwrap_or(builder_boost_factor), + ), + } + } + + fn global_max_execution_payment(&self, builder: &ValidatorBuilderDefinition) -> Option { + let auth_data = builder + .auth_data + .clone() + .unwrap_or_else(|| builder.url.to_default_auth_data()); + self.builders + .iter() + .filter(|global| global.enabled) + .find(|global| { + global.url == builder.url + && global + .auth_data + .clone() + .unwrap_or_else(|| global.url.to_default_auth_data()) + == auth_data + }) + .map(|global| global.max_execution_payment) + } +} + +fn validate_builder_definition( + url: &BuilderUrl, + auth_data: &Option, + seen_auth_urls: &mut HashSet<(BuilderUrl, RequestAuthData)>, +) -> Result<(), Error> { + // Reject malformed or non-http(s) builder URLs here, at config load, rather than silently + // skipping them during block proposal. + let sensitive_url = url + .to_sensitive_url() + .map_err(|_| Error::InvalidBuilderUrl(url.clone()))?; + if !matches!(sensitive_url.expose_full().scheme(), "http" | "https") { + return Err(Error::UnsupportedUrlScheme(url.clone())); + } + + let auth = auth_data + .clone() + .unwrap_or_else(|| url.to_default_auth_data()); + // Two entries cannot contain the same URL and auth data. + if !seen_auth_urls.insert((url.clone(), auth)) { + return Err(Error::DuplicateBuilderAuth(url.clone())); + } + + Ok(()) } impl<'a> IntoIterator for &'a BuilderConfigFile { @@ -289,4 +505,18 @@ mod tests { ); } } + + #[test] + fn validator_config_keys_are_canonicalized() { + let public_key = bls::Keypair::random().pk.compress(); + let encoded = public_key.to_string(); + let uppercase = format!("0x{}", encoded[2..].to_uppercase()); + let yaml = format!("validator_configs:\n {uppercase}: {{}}\n"); + + let config: BuilderConfigFile = yaml_serde::from_str(&yaml).unwrap(); + assert!(config.validator_configs.contains_key(&encoded)); + + let invalid = "validator_configs:\n not-a-public-key: {}\n"; + assert!(yaml_serde::from_str::(invalid).is_err()); + } } diff --git a/validator_client/builder_store/src/lib.rs b/validator_client/builder_store/src/lib.rs index d17c3c73260..2001e0ddb23 100644 --- a/validator_client/builder_store/src/lib.rs +++ b/validator_client/builder_store/src/lib.rs @@ -1,10 +1,13 @@ mod builder_definitions; use builder_definitions::BuilderConfigFile; -pub use builder_definitions::{BuilderDefinition, Error}; +pub use builder_definitions::{ + BuilderDefinition, Error, ResolvedBuilderConfig, ValidatorBuilderConfig, + ValidatorBuilderDefinition, +}; use builder_types::{ BuilderConfig, BuilderEntry, BuilderPubkeys, RequestAuthData, SignedRequestAuth, }; -use parking_lot::RwLock; +use parking_lot::{Mutex, RwLock}; use ssz_types::VariableList; use std::future::Future; use std::path::{Path, PathBuf}; @@ -14,6 +17,7 @@ use tracing::error; #[derive(Clone)] pub struct BuilderStore { config: Arc>, + update_lock: Arc>, validators_dir: PathBuf, } @@ -25,6 +29,7 @@ impl BuilderStore { config: Arc::new(RwLock::new(BuilderConfigFile::open_or_create( &validators_dir, )?)), + update_lock: Arc::new(Mutex::new(())), validators_dir, }) } @@ -39,25 +44,29 @@ impl BuilderStore { /// /// Signing is per-builder: a builder whose auth `sign` fails to produce is logged (with the /// returned error) and omitted, so one unsignable builder cannot drop the rest. The returned - /// config always carries the global policy; its `builders` list holds only the successfully - /// signed builders, and is empty when no builders are enabled or every one failed to sign. - pub async fn builder_config(&self, sign: F) -> BuilderConfig + /// config always carries the validator's resolved policy; its `builders` list holds only the + /// successfully signed builders, and is empty when no builders are enabled or every one + /// failed to sign. + pub async fn builder_config( + &self, + validator_pubkey: &bls::PublicKeyBytes, + sign: F, + ) -> BuilderConfig where F: Fn(RequestAuthData) -> Fut, Fut: Future>, E: std::fmt::Debug, { - // Snapshot the enabled builders and the global policy under the lock, then sign outside it, - // so the lock is never held across an `.await`. + // Snapshot the validator's resolved builders and policy under the lock, then sign outside + // it, so the lock is never held across an `.await`. let (definitions, min_bid, builder_boost_factor) = { let config = self.config.read(); - let definitions: Vec = config - .as_slice() - .iter() - .filter(|d| d.enabled) - .cloned() - .collect(); - (definitions, config.min_bid, config.builder_boost_factor) + let resolved = config.resolved_for(validator_pubkey); + ( + resolved.builders, + resolved.min_bid, + resolved.builder_boost_factor, + ) }; // Sign every builder's request auth concurrently. With a remote signer each `sign` is a @@ -127,6 +136,7 @@ impl BuilderStore { } pub fn insert(&self, builder: BuilderDefinition) -> Result<(), Error> { + let _update_guard = self.update_lock.lock(); let mut config = self.config.write(); // Validate a candidate copy before committing, so a bad insert leaves the config unchanged // (and the global bid-policy defaults are preserved). @@ -137,4 +147,270 @@ impl BuilderStore { *config = candidate; config.save(&self.validators_dir) } + + /// Return the fully resolved configuration for a validator without signing builder auth data. + pub fn validator_config( + &self, + validator_pubkey: &bls::PublicKeyBytes, + ) -> ResolvedBuilderConfig { + self.config.read().resolved_for(validator_pubkey) + } + + /// Replace the per-validator configuration and persist it atomically. + pub fn set_validator_config( + &self, + validator_pubkey: &bls::PublicKeyBytes, + validator_config: ValidatorBuilderConfig, + ) -> Result<(), Error> { + validator_config.validate()?; + + let _update_guard = self.update_lock.lock(); + let mut candidate = self.config.read().clone(); + candidate + .validator_configs + .insert(validator_pubkey.to_string(), validator_config); + candidate.save(&self.validators_dir)?; + *self.config.write() = candidate; + Ok(()) + } + + /// Remove a validator's override and persist the inherited global configuration atomically. + pub fn delete_validator_config( + &self, + validator_pubkey: &bls::PublicKeyBytes, + ) -> Result<(), Error> { + let _update_guard = self.update_lock.lock(); + let mut candidate = self.config.read().clone(); + if candidate + .validator_configs + .remove(&validator_pubkey.to_string()) + .is_none() + { + return Ok(()); + } + candidate.save(&self.validators_dir)?; + *self.config.write() = candidate; + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use bls::Keypair; + use builder_types::RequestAuth; + use tempfile::tempdir; + use types::Slot; + + fn global_builder(url: &str, max_execution_payment: u64) -> BuilderDefinition { + BuilderDefinition { + enabled: true, + url: url.parse().unwrap(), + auth_data: None, + builder_pubkeys: vec![], + max_execution_payment, + min_bid: None, + builder_boost_factor: None, + } + } + + fn validator_builder(url: &str) -> ValidatorBuilderDefinition { + ValidatorBuilderDefinition { + url: url.parse().unwrap(), + auth_data: None, + builder_pubkeys: vec![], + max_execution_payment: None, + min_bid: None, + builder_boost_factor: None, + } + } + + fn signed_auth(data: RequestAuthData) -> SignedRequestAuth { + SignedRequestAuth { + message: RequestAuth { + data, + slot: Slot::new(0), + }, + signature: bls::Signature::empty(), + } + } + + #[test] + fn validator_config_inherits_global_and_distinguishes_empty_builders() { + let directory = tempdir().unwrap(); + let store = BuilderStore::open_or_create(directory.path()).unwrap(); + let validator = Keypair::random().pk.compress(); + + store + .insert(global_builder("https://global-builder.example", 7)) + .unwrap(); + let mut empty_auth = global_builder("https://empty-auth.example", 7); + empty_auth.auth_data = Some(RequestAuthData::default()); + store.insert(empty_auth).unwrap(); + let mut excessive_pubkeys = global_builder("https://too-many-pubkeys.example", 7); + excessive_pubkeys.builder_pubkeys = + (0..65).map(|_| Keypair::random().pk.compress()).collect(); + store.insert(excessive_pubkeys).unwrap(); + store + .set_validator_config( + &validator, + ValidatorBuilderConfig { + min_bid: Some(5), + builder_boost_factor: Some(125), + builders: None, + }, + ) + .unwrap(); + + let inherited = store.validator_config(&validator); + assert_eq!(inherited.min_bid, 5); + assert_eq!(inherited.builder_boost_factor, 125); + assert_eq!(inherited.builders.len(), 1); + assert_eq!(inherited.builders[0].max_execution_payment, 7); + assert_eq!(inherited.builders[0].min_bid, Some(5)); + assert_eq!(inherited.builders[0].builder_boost_factor, Some(125)); + + store + .set_validator_config( + &validator, + ValidatorBuilderConfig { + min_bid: None, + builder_boost_factor: None, + builders: Some(vec![]), + }, + ) + .unwrap(); + assert!(store.validator_config(&validator).builders.is_empty()); + + store.delete_validator_config(&validator).unwrap(); + let restored = store.validator_config(&validator); + assert_eq!(restored.min_bid, 0); + assert_eq!(restored.builder_boost_factor, 100); + assert_eq!(restored.builders.len(), 1); + } + + #[test] + fn empty_validator_config_persists_across_store_restart() { + let directory = tempdir().unwrap(); + let store = BuilderStore::open_or_create(directory.path()).unwrap(); + let validator = Keypair::random().pk.compress(); + + store + .set_validator_config(&validator, ValidatorBuilderConfig::default()) + .unwrap(); + + let file = builder_definitions::BuilderConfigFile::open(directory.path()).unwrap(); + assert!(file.validator_configs.contains_key(&validator.to_string())); + + let restarted = BuilderStore::open_or_create(directory.path()).unwrap(); + assert_eq!( + restarted.validator_config(&validator), + store.validator_config(&validator) + ); + + restarted.delete_validator_config(&validator).unwrap(); + let file = builder_definitions::BuilderConfigFile::open(directory.path()).unwrap(); + assert!(!file.validator_configs.contains_key(&validator.to_string())); + } + + #[test] + fn validator_updates_are_serialized_without_losing_each_other() { + let directory = tempdir().unwrap(); + let store = Arc::new(BuilderStore::open_or_create(directory.path()).unwrap()); + let first = Keypair::random().pk.compress(); + let second = Keypair::random().pk.compress(); + + let handles = [(first, 11), (second, 22)].map(|(validator, min_bid)| { + let store = store.clone(); + std::thread::spawn(move || { + store.set_validator_config( + &validator, + ValidatorBuilderConfig { + min_bid: Some(min_bid), + ..Default::default() + }, + ) + }) + }); + for handle in handles { + handle.join().unwrap().unwrap(); + } + assert_eq!(store.validator_config(&first).min_bid, 11); + assert_eq!(store.validator_config(&second).min_bid, 22); + + let restarted = BuilderStore::open_or_create(directory.path()).unwrap(); + assert_eq!(restarted.validator_config(&first).min_bid, 11); + assert_eq!(restarted.validator_config(&second).min_bid, 22); + } + + #[test] + fn builder_consumer_observes_runtime_updates_immediately() { + let directory = tempdir().unwrap(); + let store = BuilderStore::open_or_create(directory.path()).unwrap(); + let validator = Keypair::random().pk.compress(); + store + .insert(global_builder("https://global-builder.example", 7)) + .unwrap(); + + store + .set_validator_config( + &validator, + ValidatorBuilderConfig { + builders: Some(vec![]), + ..Default::default() + }, + ) + .unwrap(); + let empty = + futures::executor::block_on(store.builder_config(&validator, |data| async move { + Ok::<_, ()>(signed_auth(data)) + })); + assert!(empty.builders.is_empty()); + + store.delete_validator_config(&validator).unwrap(); + let inherited = + futures::executor::block_on(store.builder_config(&validator, |data| async move { + Ok::<_, ()>(signed_auth(data)) + })); + assert_eq!(inherited.builders.len(), 1); + assert_eq!(inherited.builders[0].max_execution_payment, 7); + } + + #[test] + fn custom_builders_resolve_payment_limits_from_enabled_globals() { + let directory = tempdir().unwrap(); + let store = BuilderStore::open_or_create(directory.path()).unwrap(); + let validator = Keypair::random().pk.compress(); + store + .insert(global_builder("https://global-builder.example", 9)) + .unwrap(); + store + .insert(BuilderDefinition { + enabled: false, + url: "https://disabled-builder.example".parse().unwrap(), + auth_data: None, + builder_pubkeys: vec![], + max_execution_payment: 9, + min_bid: None, + builder_boost_factor: None, + }) + .unwrap(); + + store + .set_validator_config( + &validator, + ValidatorBuilderConfig { + builders: Some(vec![ + validator_builder("https://global-builder.example"), + validator_builder("https://disabled-builder.example"), + ]), + ..Default::default() + }, + ) + .unwrap(); + + let resolved = store.validator_config(&validator); + assert_eq!(resolved.builders[0].max_execution_payment, 9); + assert_eq!(resolved.builders[1].max_execution_payment, 0); + } } diff --git a/validator_client/http_api/Cargo.toml b/validator_client/http_api/Cargo.toml index e03ce2f9ebc..3148ff1f10d 100644 --- a/validator_client/http_api/Cargo.toml +++ b/validator_client/http_api/Cargo.toml @@ -17,6 +17,7 @@ axum = { workspace = true } axum_utils = { workspace = true } beacon_node_fallback = { workspace = true } bls = { workspace = true } +builder_store = { workspace = true } deposit_contract = { workspace = true, optional = true } directory = { workspace = true } dirs = { workspace = true } diff --git a/validator_client/http_api/src/lib.rs b/validator_client/http_api/src/lib.rs index 8543a246cba..520c082b5ed 100644 --- a/validator_client/http_api/src/lib.rs +++ b/validator_client/http_api/src/lib.rs @@ -19,6 +19,7 @@ use axum::Router; use axum_utils::server::Server; use beacon_node_fallback::CandidateInfo; use bls::{PublicKey, PublicKeyBytes}; +use builder_store::{ResolvedBuilderConfig, ValidatorBuilderConfig, ValidatorBuilderDefinition}; use core::convert::Infallible; use create_signed_voluntary_exit::create_signed_voluntary_exit; use create_validator::{ @@ -59,7 +60,7 @@ use validator_services::block_service::BlockService; use validator_store::ValidatorStore; use warp::{Filter, reply::Response, sse::Event}; use warp_utils::reject::convert_rejection; -use warp_utils::task::blocking_json_task; +use warp_utils::task::{blocking_json_task, blocking_response_task}; #[derive(Debug, thiserror::Error)] pub enum Error { @@ -79,6 +80,85 @@ impl From for Error { } } +fn into_store_builder_config(config: api_types::BuilderConfig) -> ValidatorBuilderConfig { + ValidatorBuilderConfig { + min_bid: config.min_bid.map(|value| value.value), + builder_boost_factor: config.builder_boost_factor.map(|value| value.value), + builders: config.builders.map(|builders| { + builders + .into_iter() + .map(|builder| ValidatorBuilderDefinition { + url: builder.url, + auth_data: builder.auth_data.map(|data| data.0), + builder_pubkeys: builder.builder_pubkeys.unwrap_or_default(), + max_execution_payment: builder.max_execution_payment.map(|value| value.value), + min_bid: builder.min_bid.map(|value| value.value), + builder_boost_factor: builder.builder_boost_factor.map(|value| value.value), + }) + .collect() + }), + } +} + +fn into_api_builder_config(config: ResolvedBuilderConfig) -> api_types::BuilderConfig { + let ResolvedBuilderConfig { + min_bid, + builder_boost_factor, + builders, + } = config; + let builders = builders + .into_iter() + .map(|builder| { + let auth_data = Some(api_types::HexRequestAuthData( + builder + .auth_data + .unwrap_or_else(|| builder.url.to_default_auth_data()), + )); + api_types::BuilderEntry { + url: builder.url, + auth_data, + builder_pubkeys: Some(builder.builder_pubkeys), + max_execution_payment: Some(api_types::Quoted { + value: builder.max_execution_payment, + }), + min_bid: Some(api_types::Quoted { + value: builder.min_bid.unwrap_or(min_bid), + }), + builder_boost_factor: Some(api_types::Quoted { + value: builder.builder_boost_factor.unwrap_or(builder_boost_factor), + }), + } + }) + .collect(); + + api_types::BuilderConfig { + min_bid: Some(api_types::Quoted { value: min_bid }), + builder_boost_factor: Some(api_types::Quoted { + value: builder_boost_factor, + }), + builders: Some(builders), + } +} + +fn builder_store_rejection(error: builder_store::Error) -> warp::Rejection { + let message = format!("builder configuration error: {error:?}"); + match error { + builder_store::Error::DuplicateBuilderAuth(_) + | builder_store::Error::InvalidBuilderUrl(_) + | builder_store::Error::UnsupportedUrlScheme(_) + | builder_store::Error::TooManyEnabledBuilders { .. } + | builder_store::Error::TooManyBuilderPubkeys(_) + | builder_store::Error::EmptyAuthData(_) => warp_utils::reject::custom_bad_request(message), + _ => warp_utils::reject::custom_server_error(message), + } +} + +fn builder_store_delete_rejection(error: builder_store::Error) -> warp::Rejection { + warp_utils::reject::custom_forbidden(format!( + "builder configuration could not be removed: {error:?}" + )) +} + /// A wrapper around all the items required to spawn the HTTP server. /// /// The server will gracefully handle the case where any fields are `None`. @@ -88,6 +168,7 @@ pub struct Context { pub block_service: Option, T>>, pub validator_store: Option>>, pub validator_dir: Option, + pub configured_builders: builder_store::BuilderStore, pub secrets_dir: Option, pub graffiti_file: Option, pub graffiti_flag: Option, @@ -213,6 +294,9 @@ pub async fn serve( }) }); + let inner_configured_builders = ctx.configured_builders.clone(); + let configured_builders_filter = warp::any().map(move || inner_configured_builders.clone()); + let inner_task_executor = ctx.task_executor.clone(); let task_executor_filter = warp::any().map(move || inner_task_executor.clone()); @@ -1018,6 +1102,120 @@ pub async fn serve( ) .map(|reply| warp::reply::with_status(reply, warp::http::StatusCode::NO_CONTENT)); + // GET /eth/v1/validator/{pubkey}/builder_config + let get_builder_config = eth_v1 + .and(warp::path("validator")) + .and(warp::path::param::()) + .and(warp::path("builder_config")) + .and(warp::path::end()) + .and(validator_store_filter.clone()) + .and(configured_builders_filter.clone()) + .then( + |validator_pubkey: PublicKey, + validator_store: Arc>, + configured_builders: builder_store::BuilderStore| { + blocking_json_task(move || { + if validator_store + .initialized_validators() + .read() + .is_enabled(&validator_pubkey) + .is_none() + { + return Err(warp_utils::reject::custom_not_found(format!( + "no validator found with pubkey {:?}", + validator_pubkey + ))); + } + + Ok(GenericResponse::from(into_api_builder_config( + configured_builders + .validator_config(&PublicKeyBytes::from(&validator_pubkey)), + ))) + }) + }, + ); + + // POST /eth/v1/validator/{pubkey}/builder_config + let post_builder_config = eth_v1 + .and(warp::path("validator")) + .and(warp::path::param::()) + .and(warp::path("builder_config")) + .and(warp::body::json()) + .and(warp::path::end()) + .and(validator_store_filter.clone()) + .and(configured_builders_filter.clone()) + .and_then( + |validator_pubkey: PublicKey, + request: api_types::BuilderConfig, + validator_store: Arc>, + configured_builders: builder_store::BuilderStore| { + blocking_response_task(move || { + if validator_store + .initialized_validators() + .read() + .is_enabled(&validator_pubkey) + .is_none() + { + return Err(warp_utils::reject::custom_not_found(format!( + "no validator found with pubkey {:?}", + validator_pubkey + ))); + } + + configured_builders + .set_validator_config( + &PublicKeyBytes::from(&validator_pubkey), + into_store_builder_config(request), + ) + .map(|_| { + warp::reply::with_status( + warp::reply(), + warp::http::StatusCode::ACCEPTED, + ) + }) + .map_err(builder_store_rejection) + }) + }, + ); + + // DELETE /eth/v1/validator/{pubkey}/builder_config + let delete_builder_config = eth_v1 + .and(warp::path("validator")) + .and(warp::path::param::()) + .and(warp::path("builder_config")) + .and(warp::path::end()) + .and(validator_store_filter.clone()) + .and(configured_builders_filter.clone()) + .and_then( + |validator_pubkey: PublicKey, + validator_store: Arc>, + configured_builders: builder_store::BuilderStore| { + blocking_response_task(move || { + if validator_store + .initialized_validators() + .read() + .is_enabled(&validator_pubkey) + .is_none() + { + return Err(warp_utils::reject::custom_not_found(format!( + "no validator found with pubkey {:?}", + validator_pubkey + ))); + } + + configured_builders + .delete_validator_config(&PublicKeyBytes::from(&validator_pubkey)) + .map(|_| { + warp::reply::with_status( + warp::reply(), + warp::http::StatusCode::NO_CONTENT, + ) + }) + .map_err(builder_store_delete_rejection) + }) + }, + ); + // GET /eth/v1/validator/{pubkey}/gas_limit let get_gas_limit = eth_v1 .and(warp::path("validator")) @@ -1360,6 +1558,7 @@ pub async fn serve( .or(get_lighthouse_ui_graffiti) .or(get_lighthouse_beacon_health) .or(get_fee_recipient) + .or(get_builder_config) .or(get_gas_limit) .or(get_graffiti) .or(get_std_keystores) @@ -1373,6 +1572,7 @@ pub async fn serve( .or(post_validators_web3signer) .or(post_validators_voluntary_exits) .or(post_fee_recipient) + .or(post_builder_config) .or(post_gas_limit) .or(post_std_keystores) .or(post_std_remotekeys) @@ -1385,6 +1585,7 @@ pub async fn serve( .or(warp::delete().and( delete_lighthouse_keystores .or(delete_fee_recipient) + .or(delete_builder_config) .or(delete_gas_limit) .or(delete_std_keystores) .or(delete_std_remotekeys) diff --git a/validator_client/http_api/src/test_utils.rs b/validator_client/http_api/src/test_utils.rs index 2c9bf79895a..2f7db82f150 100644 --- a/validator_client/http_api/src/test_utils.rs +++ b/validator_client/http_api/src/test_utils.rs @@ -5,6 +5,7 @@ use account_utils::{ eth2_wallet::WalletBuilder, mnemonic_from_phrase, random_mnemonic, random_password, }; use bls::Keypair; +use builder_store::BuilderStore; use deposit_contract::decode_eth1_tx_data; use doppelganger_service::DoppelgangerService; use eth2::{ @@ -88,6 +89,7 @@ impl ApiTester { let validator_dir = tempdir().unwrap(); let secrets_dir = tempdir().unwrap(); let token_path = tempdir().unwrap().path().join(PK_FILENAME); + let configured_builders = BuilderStore::open_or_create(validator_dir.path()).unwrap(); let validator_defs = ValidatorDefinitions::open_or_create(validator_dir.path()).unwrap(); @@ -134,6 +136,7 @@ impl ApiTester { api_secret, block_service: None::, _>>, validator_dir: Some(validator_dir.path().into()), + configured_builders: configured_builders.clone(), secrets_dir: Some(secrets_dir.path().into()), validator_store: Some(validator_store.clone()), graffiti_file: None, diff --git a/validator_client/http_api/src/tests.rs b/validator_client/http_api/src/tests.rs index 723d2175ee5..985b8710ea5 100644 --- a/validator_client/http_api/src/tests.rs +++ b/validator_client/http_api/src/tests.rs @@ -12,10 +12,14 @@ use account_utils::{ random_password_string, validator_definitions::ValidatorDefinitions, }; use bls::{Keypair, PublicKeyBytes}; +use builder_store::{BuilderDefinition, BuilderStore}; use deposit_contract::decode_eth1_tx_data; use eth2::{ Error as ApiError, - lighthouse_vc::{http_client::ValidatorClientHttpClient, types::*}, + lighthouse_vc::{ + http_client::{StatusCode, ValidatorClientHttpClient}, + types::*, + }, types::ErrorMessage as ApiErrorMessage, }; use eth2_keystore::KeystoreBuilder; @@ -44,6 +48,7 @@ struct ApiTester { client: ValidatorClientHttpClient, initialized_validators: Arc>, validator_store: Arc>, + configured_builders: BuilderStore, url: SensitiveUrl, slot_clock: TestingSlotClock, spec: Arc, @@ -65,6 +70,7 @@ impl ApiTester { let validator_dir = tempdir().unwrap(); let secrets_dir = tempdir().unwrap(); let token_path = tempdir().unwrap().path().join("api-token.txt"); + let configured_builders = BuilderStore::open_or_create(validator_dir.path()).unwrap(); let validator_defs = ValidatorDefinitions::open_or_create(validator_dir.path()).unwrap(); @@ -115,6 +121,7 @@ impl ApiTester { api_secret, block_service: None, validator_dir: Some(validator_dir.path().into()), + configured_builders: configured_builders.clone(), secrets_dir: Some(secrets_dir.path().into()), validator_store: Some(validator_store.clone()), graffiti_file: None, @@ -154,6 +161,7 @@ impl ApiTester { client, initialized_validators, validator_store, + configured_builders, url, slot_clock, spec, @@ -941,6 +949,20 @@ async fn routes_with_invalid_auth() { .await }) .await + .test_with_invalid_auth(|client| async move { + client.get_builder_config(&PublicKeyBytes::empty()).await + }) + .await + .test_with_invalid_auth(|client| async move { + client + .post_builder_config(&PublicKeyBytes::empty(), &BuilderConfig::default()) + .await + }) + .await + .test_with_invalid_auth(|client| async move { + client.delete_builder_config(&PublicKeyBytes::empty()).await + }) + .await .test_with_invalid_auth(|client| async move { client.get_keystores().await }) .await .test_with_invalid_auth(|client| async move { @@ -985,6 +1007,235 @@ async fn routes_with_invalid_auth() { .await; } +async fn builder_configuration_tester() -> (ApiTester, PublicKeyBytes) { + let tester = ApiTester::new() + .await + .create_hd_validators(HdValidatorScenario { + count: 1, + specify_mnemonic: false, + key_derivation_path_offset: 0, + disabled: vec![], + }) + .await; + let validator = tester + .client + .get_lighthouse_validators() + .await + .unwrap() + .data[0] + .voting_pubkey; + (tester, validator) +} + +#[tokio::test] +async fn validator_builder_configuration_endpoints() { + let (tester, validator) = builder_configuration_tester().await; + tester + .configured_builders + .insert(BuilderDefinition { + enabled: true, + url: "https://global-builder.example".parse().unwrap(), + auth_data: None, + builder_pubkeys: vec![], + max_execution_payment: 7, + min_bid: None, + builder_boost_factor: None, + }) + .unwrap(); + + let inherited = tester.client.get_builder_config(&validator).await.unwrap(); + assert_eq!(inherited.min_bid.unwrap().value, 0); + assert_eq!(inherited.builder_boost_factor.unwrap().value, 100); + assert_eq!(inherited.builders.as_ref().unwrap().len(), 1); + assert_eq!( + inherited.builders.as_ref().unwrap()[0] + .max_execution_payment + .unwrap() + .value, + 7 + ); + + let empty_post = tester + .client + .post_builder_config(&validator, &BuilderConfig::default()) + .await + .unwrap(); + assert_eq!(empty_post.status(), StatusCode::ACCEPTED); + assert_eq!( + tester.client.get_builder_config(&validator).await.unwrap(), + inherited + ); + + let custom = BuilderConfig { + min_bid: Some(Quoted { value: 3 }), + builder_boost_factor: Some(Quoted { value: 115 }), + builders: Some(vec![BuilderEntry { + url: "https://builder.example".parse().unwrap(), + auth_data: Some(HexRequestAuthData( + RequestAuthData::new(b"validator-auth".to_vec()).unwrap(), + )), + builder_pubkeys: Some(vec![]), + max_execution_payment: Some(Quoted { value: 8 }), + min_bid: None, + builder_boost_factor: None, + }]), + }; + tester + .client + .post_builder_config(&validator, &custom) + .await + .unwrap(); + assert_eq!( + tester.client.get_builder_config(&validator).await.unwrap(), + BuilderConfig { + min_bid: Some(Quoted { value: 3 }), + builder_boost_factor: Some(Quoted { value: 115 }), + builders: Some(vec![BuilderEntry { + url: "https://builder.example".parse().unwrap(), + auth_data: Some(HexRequestAuthData( + RequestAuthData::new(b"validator-auth".to_vec()).unwrap(), + )), + builder_pubkeys: Some(vec![]), + max_execution_payment: Some(Quoted { value: 8 }), + min_bid: Some(Quoted { value: 3 }), + builder_boost_factor: Some(Quoted { value: 115 }), + }]), + } + ); + + let empty_list = BuilderConfig { + builders: Some(vec![]), + ..Default::default() + }; + tester + .client + .post_builder_config(&validator, &empty_list) + .await + .unwrap(); + assert_eq!( + tester.client.get_builder_config(&validator).await.unwrap(), + BuilderConfig { + min_bid: Some(Quoted { value: 0 }), + builder_boost_factor: Some(Quoted { value: 100 }), + builders: Some(vec![]), + } + ); + + let delete = tester + .client + .delete_builder_config(&validator) + .await + .unwrap(); + assert_eq!(delete.status(), StatusCode::NO_CONTENT); + assert_eq!( + tester.client.get_builder_config(&validator).await.unwrap(), + inherited + ); +} + +#[tokio::test] +async fn builder_configuration_unknown_validator() { + let tester = ApiTester::new().await; + let unknown = Keypair::random().pk.compress(); + assert_server_error_code(tester.client.get_builder_config(&unknown).await, 404); + assert_server_error_code( + tester + .client + .post_builder_config(&unknown, &BuilderConfig::default()) + .await, + 404, + ); + assert_server_error_code(tester.client.delete_builder_config(&unknown).await, 404); +} + +#[tokio::test] +async fn builder_configuration_rejects_invalid_input() { + let (tester, validator) = builder_configuration_tester().await; + let invalid = BuilderConfig { + builders: Some(vec![BuilderEntry { + url: "ftp://builder.example".parse().unwrap(), + auth_data: None, + builder_pubkeys: Some(vec![]), + max_execution_payment: Some(Quoted { value: 1 }), + min_bid: None, + builder_boost_factor: None, + }]), + ..Default::default() + }; + assert_server_error_code( + tester + .client + .post_builder_config(&validator, &invalid) + .await, + 400, + ); + + let empty_auth = BuilderConfig { + builders: Some(vec![BuilderEntry { + url: "https://builder.example".parse().unwrap(), + auth_data: Some(HexRequestAuthData(RequestAuthData::default())), + builder_pubkeys: Some(vec![]), + max_execution_payment: Some(Quoted { value: 1 }), + min_bid: None, + builder_boost_factor: None, + }]), + ..Default::default() + }; + assert_server_error_code( + tester + .client + .post_builder_config(&validator, &empty_auth) + .await, + 400, + ); + + let before = tester.client.get_builder_config(&validator).await.unwrap(); + let duplicate = BuilderEntry { + url: "https://duplicate-builder.example".parse().unwrap(), + auth_data: None, + builder_pubkeys: None, + max_execution_payment: None, + min_bid: None, + builder_boost_factor: None, + }; + assert_server_error_code( + tester + .client + .post_builder_config( + &validator, + &BuilderConfig { + builders: Some(vec![duplicate.clone(), duplicate]), + ..Default::default() + }, + ) + .await, + 400, + ); + assert_eq!( + tester.client.get_builder_config(&validator).await.unwrap(), + before + ); +} + +#[test] +fn builder_delete_errors_are_forbidden() { + let rejection = crate::builder_store_delete_rejection(builder_store::Error::UnableToOpenFile( + std::io::Error::other("test"), + )); + assert!( + rejection + .find::() + .is_some() + ); +} + +fn assert_server_error_code(result: Result, expected: u16) { + match result { + Err(ApiError::ServerMessage(ApiErrorMessage { code, .. })) if code == expected => (), + other => panic!("expected HTTP {expected}, got {other:?}"), + } +} + #[tokio::test] async fn simple_getters() { ApiTester::new() diff --git a/validator_client/src/lib.rs b/validator_client/src/lib.rs index 7697a08c45b..6621adbcdf7 100644 --- a/validator_client/src/lib.rs +++ b/validator_client/src/lib.rs @@ -94,6 +94,7 @@ pub struct ProductionValidatorClient { doppelganger_service: Option>, preparation_service: PreparationService, SystemTimeSlotClock>, validator_store: Arc>, + configured_builders: BuilderStore, builder_preferences_service: BuilderPreferencesService, SystemTimeSlotClock>, slot_clock: SystemTimeSlotClock, http_api_listen_addr: Option, @@ -609,6 +610,7 @@ impl ProductionValidatorClient { doppelganger_service, preparation_service, validator_store, + configured_builders: configured_builders.clone(), builder_preferences_service, config, slot_clock, @@ -633,6 +635,7 @@ impl ProductionValidatorClient { block_service: Some(self.block_service.clone()), validator_store: Some(self.validator_store.clone()), validator_dir: Some(self.config.validator_dir.clone()), + configured_builders: self.configured_builders.clone(), secrets_dir: Some(self.config.secrets_dir.clone()), graffiti_file: self.config.graffiti_file.clone(), graffiti_flag: self.config.graffiti, diff --git a/validator_client/validator_services/src/block_service.rs b/validator_client/validator_services/src/block_service.rs index b2cfa138e88..1e157b3ae95 100644 --- a/validator_client/validator_services/src/block_service.rs +++ b/validator_client/validator_services/src/block_service.rs @@ -501,7 +501,7 @@ impl BlockService { // inside `builder_config`, so this never fails the proposal. let builder_config = self_ref .configured_builders - .builder_config(|auth_data| { + .builder_config(&validator_pubkey, |auth_data| { self_ref.request_auth_cache.get_or_sign( slot, validator_pubkey, diff --git a/validator_client/validator_services/src/builder_preferences_service.rs b/validator_client/validator_services/src/builder_preferences_service.rs index bb1be8b0089..431d6995db4 100644 --- a/validator_client/validator_services/src/builder_preferences_service.rs +++ b/validator_client/validator_services/src/builder_preferences_service.rs @@ -222,7 +222,7 @@ impl BuilderPreferencesServ let config = self .inner .configured_builders - .builder_config(|auth_data| { + .builder_config(&pubkey, |auth_data| { self.inner.request_auth_cache.get_or_sign( slot, pubkey,