Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix builder API headers #7009

Merged
merged 9 commits into from
Feb 24, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions beacon_node/beacon_chain/src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -779,6 +779,7 @@ where
SensitiveUrl::parse(format!("http://127.0.0.1:{port}").as_str()).unwrap(),
None,
None,
false,
)
.unwrap();

Expand Down
106 changes: 81 additions & 25 deletions beacon_node/builder_client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ pub const DEFAULT_GET_HEADER_TIMEOUT_MILLIS: u64 = 1000;
/// Default user agent for HTTP requests.
pub const DEFAULT_USER_AGENT: &str = lighthouse_version::VERSION;

/// The value we set on the `ACCEPT` http header to indicate a preference for ssz response.
pub const PREFERENCE_ACCEPT_VALUE: &str = "application/octet-stream;q=1.0,application/json;q=0.9";
/// Only accept json responses.
pub const JSON_ACCEPT_VALUE: &str = "application/json";

#[derive(Clone)]
pub struct Timeouts {
get_header: Duration,
Expand Down Expand Up @@ -57,14 +62,20 @@ pub struct BuilderHttpClient {
server: SensitiveUrl,
timeouts: Timeouts,
user_agent: String,
ssz_enabled: Arc<AtomicBool>,
/// Only use json for all requests/responses types.
disable_ssz: bool,
/// Indicates that the `get_header` response had content-type ssz
/// so we can set content-type header to ssz to make the `submit_blinded_blocks`
/// request.
ssz_available: Arc<AtomicBool>,
}

impl BuilderHttpClient {
pub fn new(
server: SensitiveUrl,
user_agent: Option<String>,
builder_header_timeout: Option<Duration>,
disable_ssz: bool,
) -> Result<Self, Error> {
let user_agent = user_agent.unwrap_or(DEFAULT_USER_AGENT.to_string());
let client = reqwest::Client::builder().user_agent(&user_agent).build()?;
Expand All @@ -73,7 +84,8 @@ impl BuilderHttpClient {
server,
timeouts: Timeouts::new(builder_header_timeout),
user_agent,
ssz_enabled: Arc::new(false.into()),
disable_ssz,
ssz_available: Arc::new(false.into()),
})
}

Expand Down Expand Up @@ -124,15 +136,15 @@ impl BuilderHttpClient {

let Ok(Some(fork_name)) = self.fork_name_from_header(&headers) else {
// if no fork version specified, attempt to fallback to JSON
self.ssz_enabled.store(false, Ordering::SeqCst);
self.ssz_available.store(false, Ordering::SeqCst);
return serde_json::from_slice(&response_bytes).map_err(Error::InvalidJson);
};

let content_type = self.content_type_from_header(&headers);

match content_type {
ContentType::Ssz => {
self.ssz_enabled.store(true, Ordering::SeqCst);
self.ssz_available.store(true, Ordering::SeqCst);
T::from_ssz_bytes_by_fork(&response_bytes, fork_name)
.map(|data| ForkVersionedResponse {
version: Some(fork_name),
Expand All @@ -142,15 +154,17 @@ impl BuilderHttpClient {
.map_err(Error::InvalidSsz)
}
ContentType::Json => {
self.ssz_enabled.store(false, Ordering::SeqCst);
self.ssz_available.store(false, Ordering::SeqCst);
serde_json::from_slice(&response_bytes).map_err(Error::InvalidJson)
}
}
}

/// Return `true` if the most recently received response from the builder had SSZ Content-Type.
pub fn is_ssz_enabled(&self) -> bool {
self.ssz_enabled.load(Ordering::SeqCst)
/// Return `false` otherwise.
/// Also returns `false` if we have explicitly disabled ssz.
pub fn is_ssz_available(&self) -> bool {
!self.disable_ssz && self.ssz_available.load(Ordering::SeqCst)
}

async fn get_with_timeout<T: DeserializeOwned, U: IntoUrl>(
Expand Down Expand Up @@ -213,19 +227,14 @@ impl BuilderHttpClient {
&self,
url: U,
ssz_body: Vec<u8>,
mut headers: HeaderMap,
headers: HeaderMap,
timeout: Option<Duration>,
) -> Result<Response, Error> {
let mut builder = self.client.post(url);
if let Some(timeout) = timeout {
builder = builder.timeout(timeout);
}

headers.insert(
CONTENT_TYPE_HEADER,
HeaderValue::from_static(SSZ_CONTENT_TYPE_HEADER),
);

let response = builder
.headers(headers)
.body(ssz_body)
Expand Down Expand Up @@ -292,9 +301,21 @@ impl BuilderHttpClient {
.push("blinded_blocks");

let mut headers = HeaderMap::new();
if let Ok(value) = HeaderValue::from_str(&blinded_block.fork_name_unchecked().to_string()) {
headers.insert(CONSENSUS_VERSION_HEADER, value);
}
headers.insert(
CONSENSUS_VERSION_HEADER,
HeaderValue::from_str(&blinded_block.fork_name_unchecked().to_string())
.map_err(|e| Error::InvalidHeaders(format!("{}", e)))?,
);
headers.insert(
CONTENT_TYPE_HEADER,
HeaderValue::from_str(SSZ_CONTENT_TYPE_HEADER)
.map_err(|e| Error::InvalidHeaders(format!("{}", e)))?,
);
headers.insert(
ACCEPT,
HeaderValue::from_str(PREFERENCE_ACCEPT_VALUE)
.map_err(|e| Error::InvalidHeaders(format!("{}", e)))?,
);

let result = self
.post_ssz_with_raw_response(
Expand Down Expand Up @@ -326,9 +347,21 @@ impl BuilderHttpClient {
.push("blinded_blocks");

let mut headers = HeaderMap::new();
if let Ok(value) = HeaderValue::from_str(&blinded_block.fork_name_unchecked().to_string()) {
headers.insert(CONSENSUS_VERSION_HEADER, value);
}
headers.insert(
CONSENSUS_VERSION_HEADER,
HeaderValue::from_str(&blinded_block.fork_name_unchecked().to_string())
.map_err(|e| Error::InvalidHeaders(format!("{}", e)))?,
);
headers.insert(
CONTENT_TYPE_HEADER,
HeaderValue::from_str(JSON_CONTENT_TYPE_HEADER)
.map_err(|e| Error::InvalidHeaders(format!("{}", e)))?,
);
headers.insert(
ACCEPT,
HeaderValue::from_str(JSON_ACCEPT_VALUE)
.map_err(|e| Error::InvalidHeaders(format!("{}", e)))?,
);

Ok(self
.post_with_raw_response(
Expand Down Expand Up @@ -362,12 +395,20 @@ impl BuilderHttpClient {
.push(pubkey.as_hex_string().as_str());

let mut headers = HeaderMap::new();
if let Ok(ssz_content_type_header) = HeaderValue::from_str(&format!(
"{}; q=1.0,{}; q=0.9",
SSZ_CONTENT_TYPE_HEADER, JSON_CONTENT_TYPE_HEADER
)) {
headers.insert(ACCEPT, ssz_content_type_header);
};
if self.disable_ssz {
headers.insert(
ACCEPT,
HeaderValue::from_str(JSON_CONTENT_TYPE_HEADER)
.map_err(|e| Error::InvalidHeaders(format!("{}", e)))?,
);
} else {
// Indicate preference for ssz response in the accept header
headers.insert(
ACCEPT,
HeaderValue::from_str(PREFERENCE_ACCEPT_VALUE)
.map_err(|e| Error::InvalidHeaders(format!("{}", e)))?,
);
}

let resp = self
.get_with_header(path, self.timeouts.get_header, headers)
Expand Down Expand Up @@ -395,3 +436,18 @@ impl BuilderHttpClient {
.await
}
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn test_headers_no_panic() {
for fork in ForkName::list_all() {
assert!(HeaderValue::from_str(&fork.to_string()).is_ok());
}
assert!(HeaderValue::from_str(PREFERENCE_ACCEPT_VALUE).is_ok());
assert!(HeaderValue::from_str(JSON_ACCEPT_VALUE).is_ok());
assert!(HeaderValue::from_str(JSON_CONTENT_TYPE_HEADER).is_ok());
}
}
22 changes: 20 additions & 2 deletions beacon_node/execution_layer/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,8 @@ pub struct Config {
pub builder_header_timeout: Option<Duration>,
/// User agent to send with requests to the builder API.
pub builder_user_agent: Option<String>,
/// Disable ssz requests on builder. Only use json.
pub disable_builder_ssz_requests: bool,
/// JWT secret for the above endpoint running the engine api.
pub secret_file: Option<PathBuf>,
/// The default fee recipient to use on the beacon node if none if provided from
Expand Down Expand Up @@ -470,6 +472,7 @@ impl<E: EthSpec> ExecutionLayer<E> {
builder_url,
builder_user_agent,
builder_header_timeout,
disable_builder_ssz_requests,
secret_file,
suggested_fee_recipient,
jwt_id,
Expand Down Expand Up @@ -539,7 +542,12 @@ impl<E: EthSpec> ExecutionLayer<E> {
};

if let Some(builder_url) = builder_url {
el.set_builder_url(builder_url, builder_user_agent, builder_header_timeout)?;
el.set_builder_url(
builder_url,
builder_user_agent,
builder_header_timeout,
disable_builder_ssz_requests,
)?;
}

Ok(el)
Expand All @@ -562,18 +570,21 @@ impl<E: EthSpec> ExecutionLayer<E> {
builder_url: SensitiveUrl,
builder_user_agent: Option<String>,
builder_header_timeout: Option<Duration>,
disable_ssz: bool,
) -> Result<(), Error> {
let builder_client = BuilderHttpClient::new(
builder_url.clone(),
builder_user_agent,
builder_header_timeout,
disable_ssz,
)
.map_err(Error::Builder)?;
info!(
self.log(),
"Using external block builder";
"builder_url" => ?builder_url,
"local_user_agent" => builder_client.get_user_agent(),
"ssz_disabled" => disable_ssz
);
self.inner.builder.swap(Some(Arc::new(builder_client)));
Ok(())
Expand Down Expand Up @@ -1901,7 +1912,14 @@ impl<E: EthSpec> ExecutionLayer<E> {
if let Some(builder) = self.builder() {
let (payload_result, duration) =
timed_future(metrics::POST_BLINDED_PAYLOAD_BUILDER, async {
if builder.is_ssz_enabled() {
let ssz_enabled = builder.is_ssz_available();
debug!(
self.log(),
"Calling submit_blinded_block on builder";
"block_root" => ?block_root,
"ssz" => ssz_enabled
);
if ssz_enabled {
builder
.post_builder_blinded_blocks_ssz(block)
.await
Expand Down
9 changes: 9 additions & 0 deletions beacon_node/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1460,6 +1460,15 @@ pub fn cli_app() -> Command {
.action(ArgAction::Set)
.display_order(0)
)
.arg(
Arg::new("builder-disable-ssz")
.long("builder-disable-ssz")
.value_name("BOOLEAN")
.help("Disables sending requests using SSZ over the builder API.")
.requires("builder")
.action(ArgAction::SetTrue)
.display_order(0)
)
.arg(
Arg::new("reset-payload-statuses")
.long("reset-payload-statuses")
Expand Down
2 changes: 2 additions & 0 deletions beacon_node/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,8 @@ pub fn get_config<E: EthSpec>(
el_config.builder_header_timeout =
clap_utils::parse_optional(cli_args, "builder-header-timeout")?
.map(Duration::from_millis);

el_config.disable_builder_ssz_requests = cli_args.get_flag("builder-disable-ssz");
}

// Set config values from parse values.
Expand Down
2 changes: 2 additions & 0 deletions book/src/help_bn.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ Options:
network. Multiaddr is also supported.
--builder <builder>
The URL of a service compatible with the MEV-boost API.
--builder-disable-ssz
Disables sending requests using SSZ over the builder API.
--builder-fallback-epochs-since-finalization <builder-fallback-epochs-since-finalization>
If this node is proposing a block and the chain has not finalized
within this number of epochs, it will NOT query any connected
Expand Down
34 changes: 34 additions & 0 deletions lighthouse/tests/beacon_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -720,6 +720,40 @@ fn builder_user_agent() {
);
}

#[test]
fn test_builder_disable_ssz_flag() {
run_payload_builder_flag_test_with_config(
"builder",
"http://meow.cats",
None,
None,
|config| {
assert!(
!config
.execution_layer
.as_ref()
.unwrap()
.disable_builder_ssz_requests,
);
},
);
run_payload_builder_flag_test_with_config(
"builder",
"http://meow.cats",
Some("builder-disable-ssz"),
None,
|config| {
assert!(
config
.execution_layer
.as_ref()
.unwrap()
.disable_builder_ssz_requests,
);
},
);
}

fn run_jwt_optional_flags_test(jwt_flag: &str, jwt_id_flag: &str, jwt_version_flag: &str) {
use sensitive_url::SensitiveUrl;

Expand Down