Add SSZ to PBS by jclapis · Pull Request #372 · Commit-Boost/commit-boost-client

jclapis · 2025-09-09T20:38:24Z

This is a modernization of #252 since that's been dormant for a while, but was re-raised in #364. Just about everything was ported over cleanly.

NOTE: This now has #397 built in, so that should be merged first.

…-ssz-to-pbs

ltitanb

We should ideally also test this with kurtosis

crates/common/src/utils.rs

ltitanb · 2025-09-12T13:04:39Z

crates/pbs/src/routes/get_header.rs

-
                BEACON_NODE_STATUS.with_label_values(&["200", GET_HEADER_ENDPOINT_TAG]).inc();
-                Ok((StatusCode::OK, axum::Json(max_bid)).into_response())
+                let response = match accept_header {


here we just asssume the relay just support both? probably fine but ok double checking

We do, if we want to add support for relays that only allow JSON for example, we'll have to probably figure that out on startup and flag them accordingly so we don't ping them to negotiate encoding with every request (assuming they never change it down the line). Do we have stats on how many support SSZ and how many don't?

how about catching an error and resubmitting with a different encoding? i assume that's what the BN does already instead of mapping whether a given sidecar supports what

This was a significant rework; so much so that it now lives in its own branch built on top of this one: https://github.com/Commit-Boost/commit-boost-client/tree/ssz-update-v2

ltitanb

in general i think we should try to keep a "charitable" approach, if the request is malformed / some header is missing we should try to pick a reasonable default based on current information (eg current fork, JSON default) and log a warning , instead of returning an error

ltitanb · 2025-11-03T10:48:23Z

crates/common/src/utils.rs

+    accept.media_types().for_each(|mt| match mt.essence().to_string().as_str() {
+        APPLICATION_OCTET_STREAM => ssz_type = true,
+        APPLICATION_JSON | WILDCARD => json_type = true,
+        _ => unsupported_type = true,


would rather default to json here instead just in case?

This is changed somewhat in v2, so now application/octet-stream corresponds to SSZ and application/json, */*, and no header at all correspond to JSON. I don't think it's a good idea to reinterpret anything else as JSON (like application/text or something) because the client went out of its way to tell us that it doesn't accept JSON. Giving it JSON back anyway is going to be messy.

ltitanb · 2025-11-03T10:48:40Z

crates/common/src/utils.rs

+/// defaulting to JSON if missing. Returns an error if malformed or unsupported
+/// types are requested. Supports requests with multiple ACCEPT headers or
+/// headers with multiple media types.
+pub fn get_accept_type(req_headers: &HeaderMap) -> eyre::Result<EncodingType> {


should we add a few unit tests for this function?

Definitely. v2 has lots of them - take a look.

ltitanb · 2025-11-03T10:49:22Z

crates/common/src/utils.rs

+        req_headers
+            .get(CONSENSUS_VERSION_HEADER)
+            .and_then(|value| value.to_str().ok())
+            .unwrap_or(""),


if missing should we default to the current fork instead?

I'm inclined to stick to the spec on this one, which says if the request / response use SSZ, then the header must be present; otherwise it doesn't have to be. Either way, this particular function probably isn't the proper place to backfill a missing header; whatever uses it can decide to do that if this reports a missing header, it really needs to.

ltitanb · 2025-11-03T10:50:02Z

crates/common/src/utils.rs

+impl FromStr for EncodingType {
+    type Err = String;
+    fn from_str(value: &str) -> Result<Self, Self::Err> {
+        match value {


should we match on lowercase string?

I think Hyper will convert them all to lowercase, but I added an explicit conversion just in case in b22eed8.

ltitanb · 2025-11-03T10:52:24Z

crates/pbs/src/mev_boost/register_validator.rs

-                    send_headers.clone(),
-                    state.pbs_config().timeout_register_validator_ms,
-                    state.pbs_config().register_validator_retry_limit,
+            handles.push(


do you why these unrelated changes show up here? they should already be in main

I don't, maybe something related to a merge - there have been a lot of them since this branch started.

ltitanb · 2025-11-03T10:54:23Z

crates/pbs/src/routes/get_header.rs

+    let accept_type = get_accept_type(&req_headers).map_err(|e| {
+        error!(%e, "error parsing accept header");
+        PbsClientError::DecodeError(format!("error parsing accept header: {e}"))
+    });
+    if let Err(e) = accept_type {
+        return Ok((StatusCode::BAD_REQUEST, e).into_response());
+    }


this should be a PbsError where we implement into_response already, ideally we keep all the error mappings there

Good call, check 8ebfbd0 and see if that's what you're looking for.

ltitanb · 2025-11-03T10:57:53Z

crates/pbs/src/routes/get_header.rs

-                Ok((StatusCode::OK, axum::Json(max_bid)).into_response())
+                let response = match accept_type {
+                    EncodingType::Ssz => {
+                        let mut res = max_bid.data.as_ssz_bytes().into_response();


we should encode as SSZ after we checked that we can return it, otherwise we might do the SSZ encoding for nothing

I believe that's what it's doing, no? If ACCEPT has application/octet-stream in it then we proceed, since the request told us it supports SSZ. v2 actually support multiple ACCEPT headers or one header with multiple comma-separated types in it, but same idea.

ltitanb · 2025-11-03T10:58:19Z

crates/pbs/src/routes/get_header.rs

+                            return Ok((StatusCode::OK, axum::Json(max_bid)).into_response());
+                        };
+                        let Ok(content_type_header) =
+                            HeaderValue::from_str(&format!("{}", EncodingType::Ssz))


this could be a static, so we avoid the string allocation

Good call, fixed in 2499bd5.

ltitanb · 2025-11-03T11:57:58Z

crates/pbs/src/routes/submit_block.rs

    api_version: BuilderApiVersion,
 ) -> Result<impl IntoResponse, PbsClientError> {
+    let signed_blinded_block = Arc::new(
+        deserialize_body(&req_headers, raw_request.body_bytes).await.map_err(|e| {


deserialize_body could return either a PbsError or an error that has a #[from] in PbsError, so we avoid the manual error mapping, we can still log it with eg .inspect_err if needed

Nice, I didn't know you could do this - more Rust idiosyncrasies. Take a look at 41d879e, how's that?

ltitanb · 2025-11-03T11:58:25Z

crates/pbs/src/routes/submit_block.rs

+                            ));
+                        };
+                        let Ok(content_type_header) =
+                            HeaderValue::from_str(&EncodingType::Ssz.to_string())


this should be a static

Also fixed in 2499bd5.

eserilev and others added 9 commits February 5, 2025 13:37

Add builder ssz flow

3a2c7d5

Integration tests, fixes, and clippy/linting

5059593

resolve merge conflicts

35b8542

resolve merge conflictds

4acddbe

fix tests

7005422

Started porting #252 back to main

06c207f

Finished porting, successful tests

24fabca

Merge remote-tracking branch 'eserilev/add-builder-ssz-flow' into add…

f0875cd

…-ssz-to-pbs

Cleaned up merge

bad5675

jclapis requested a review from ltitanb September 9, 2025 20:38

jclapis self-assigned this Sep 9, 2025

jclapis added the pbs Pbs module / Builder API label Sep 9, 2025

Fixed clippy

50fccb5

ltitanb requested changes Sep 12, 2025

View reviewed changes

jclapis added 3 commits September 15, 2025 10:19

Swapped to LH's ForkName enum

86fa858

Refactored encoding type support based on feedback

e7335f6

Fixed clippy

8c82b84

ltitanb mentioned this pull request Oct 10, 2025

Support SSZ in proposer api gattaca-com/helix#285

Open

jclapis and others added 5 commits October 14, 2025 12:18

Merge branch 'main' into add-ssz-to-pbs

9a58c07

Merge branch 'main' into add-ssz-to-pbs

01f6b04

Merge branch 'main' into add-ssz-to-pbs

a287e96

spawn

de99bb9

Merge branch 'lt/get-payload-v2-on-all' into add-ssz-to-pbs

06a3092

ltitanb requested changes Nov 3, 2025

View reviewed changes

Merge branch 'main' into add-ssz-to-pbs

a556aa9

jclapis mentioned this pull request Nov 11, 2025

Add improvements to SSZ support #403

Open

Conversation

jclapis commented Sep 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ltitanb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ltitanb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jclapis commented Sep 9, 2025 •

edited

Loading