[MegaRedHand]

Motivation

Running other clients' validator clients against ethlambda beacon on a fulu->gloas Kurtosis devnet (lighthouse v8.3.0-rc.0, teku 26.9.1, nimbus v26.10.0, prysm v7.2.0, lodestar v1.49.0, each driving 77 keys on its own ethlambda node) showed four ways our node turned them away:

VC	Symptom	Cause
lighthouse	every validator "inactive", proposals refused ("Proposer index does not match")	no GET /eth/v1/beacon/states/{state_id}/validators/{validator_id}, which it resolves indices with
nimbus	node marked incompatible, no duty at all	no GET /eth/v1/config/fork_schedule
prysm	aggregates and payload votes 400 "invalid request body"	it posts them as SSZ
teku	gloas envelopes 415, so teku-proposed gloas blocks never got their payload	it posts the envelope as JSON

Changes

• GET /eth/v1/beacon/states/{state_id}/validators/{validator_id}: one entry, by index or pubkey; 404 when unknown.
• GET /eth/v1/config/fork_schedule: every scheduled fork as a linked {previous_version, current_version, epoch} list (nimbus checks the linking).
• Request bodies chosen by Content-Type (none means JSON, anything else is a 415, which prysm answers by retrying with JSON):
  • JSON or SSZ lists on POST /eth/v2/beacon/pool/attestations, POST /eth/v2/validator/aggregate_and_proofs, POST /eth/v1/beacon/pool/payload_attestations;
  • JSON or SSZ on POST /eth/v2/beacon/blocks (fulu SignedBlockContents, gloas SignedBeaconBlock) and POST /eth/v1/beacon/execution_payload_envelopes (envelope or envelope contents per Eth-Blob-Data-Included).
• Deserialize for every container a fulu/gloas block or envelope carries, with the inverse of each serde adapter (oversized lists are refused, not truncated).

Content types per VC, read from their tagged sources:

VC	Blocks	Envelope	Envelope contents
lighthouse	SSZ	SSZ	SSZ
prysm	SSZ, JSON on 415	SSZ, JSON on 415	same
teku	SSZ, then JSON	JSON	JSON
nimbus	JSON	JSON	SSZ
lodestar	JSON	JSON	JSON

Validation

• Unit and router tests for each endpoint and body type (JSON and SSZ give identical results; 415 for other types); JSON round trips of the block, envelope and request containers.
• Devnet on ethlambda-5, image built from this branch merged into tmp/bci-626-63-64-633-636-638-gloas-live (with fix(rpc): bound duties by the wall clock, advance the head for later epochs #663): fulu genesis, gloas at epoch 5, 1536 validators, ethlambda nodes holding 50% (half of that driven by the five foreign VCs above).
  • Epochs 2 to 12: every validator in every group earned source, target and head rewards.
  • Gloas slots 160 to 470: no missed slot; every block's envelope landed, including the 73 proposed by foreign VCs through our nodes.
• Left: sync-committee endpoints (separate work), and proposer_preferences / register_validator (builder path), which lighthouse and prysm call and get a 404.

[github-actions]

🤖 Codex Code Review

Overall this PR looks solid: the JSON deserialization coverage for Beacon API containers is useful, the new fork_schedule endpoint matches client expectations, and the request-body content negotiation changes are mostly clean.

• crates/net/rpc/src/beacon/config.rs:77
fork_schedule() does not enforce monotonic epochs. If a custom network config accidentally sets a later fork at an earlier epoch than a previous scheduled fork, the endpoint will emit an out-of-order linked list that validator clients may treat as invalid or misleading. Since this endpoint is used for compatibility checks, I’d either sort by epoch/version explicitly or reject non-monotonic schedules when building the response.

• crates/net/rpc/src/beacon/states.rs:214
ValidatorId::parse() only accepts lowercase 0x prefixes. Beacon API callers sometimes normalize hex more loosely; rejecting 0X... is probably not intended and is inconsistent with many other hex parsers. Low severity, but easy compatibility footgun.

• crates/net/rpc/src/beacon/proposal.rs:163
  For fulu publishBlockV2, signature verification advances from the current head state instead of the block’s parent state, while the gloas path correctly uses block.parent_root(). That means a valid block extending a non-head parent can be rejected, and worse, proposer selection/signature checks become dependent on local head rather than the declared parent. For consensus code, I’d strongly prefer using the parent state here as well, like post_gloas_block() does.

• crates/net/rpc/src/beacon/proposal.rs:153
  Related to the above: block.slot() <= head_state.slot() rejects any block not strictly after the local head. That may be intentional API policy, but it’s stricter than “parent is held and block is valid” and can reject valid side-chain/reorg blocks from a validator client. If the goal is gossip+import correctness rather than only local-head publication, this deserves another look.

• crates/net/rpc/src/beacon/gloas_proposal.rs:808
publishExecutionPayloadEnvelope waits for the block by root, then validates envelope contents against that imported block. That’s good, but it means the endpoint’s correctness depends on import winning a race within BLOCK_WAIT. If callers can legitimately publish immediately after publishBlockV2, short waits can create flaky 400 unknown block responses under load. Not a correctness bug in the validation itself, but worth checking operationally.

• crates/common/types/src/beacon/containers/json_tests.rs:9
  The round-trip tests are a good start, but they only prove self-consistency of your serializers/deserializers. They won’t catch schema drift versus the Beacon API JSON shape. A few fixture-based tests with exact expected field names/encodings for the newly deserializable request types would increase confidence, especially around bitfields and quoted integers.

If you want, I can turn Point 3 into a concrete patch recommendation.

---

Automated review by OpenAI Codex · gpt-5.4 · custom prompt

[github-actions]

🤖 Kimi Code Review

I'll review this PR which adds JSON deserialization support for beacon containers, SSZ body encoding negotiation, and related Beacon API endpoints. Let me analyze the changes systematically.

Overview

The PR makes three main changes:

1. Adds serde::Deserialize to many beacon containers and fixes serde field attributes
2. Implements content-type negotiation (JSON/SSZ) for validator submission endpoints
3. Adds /eth/v1/config/fork_schedule and /eth/v1/beacon/states/{id}/validators/{validator_id} endpoints

Code Review

Item 1: U256 Deserialization - Missing 0x prefix handling

File: crates/common/types/src/beacon/primitives.rs, lines 345-347

let text = <String as serde::Deserialize>::deserialize(deserializer)?;
U256::from_dec_str(text.trim()).map_err(serde::de::Error::custom)

The U256 serialize implementation at line 325-335 writes a decimal string. However, the from_dec_str call doesn't handle the 0x prefix that many Ethereum tools use for uint256 values. More critically, this is inconsistent with how Uint256 might be received from other clients.

Question: Is U256::from_dec_str strict enough? If a client sends "0x1a2b3c..." (hex), this will fail. The spec doesn't mandate a format for uint256 in JSON, but execution layer APIs typically use hex for large integers.

Suggestion: Consider whether hex strings should be accepted, or document that only decimal is supported.

Item 2: ssz_hex Deserializer - Missing validation of 0x prefix handling

File: crates/common/types/src/beacon/serde_helpers.rs, lines 383-396 (in ssz_hex_seq::deserialize)

let digits = text.trim();
let bytes = hex::decode(digits.strip_prefix("0x").unwrap_or(digits))
    .map_err(serde::de::Error::custom)?;

The strip_prefix("0x") is correct, but note that hex::decode accepts both "0xab" and "ab" — this is fine. However, compare with impl_hex_deserialize! in primitives.rs which also strips 0x.

Potential inconsistency: The ssz_hex module's serialize function (line 255-265) adds 0x prefix. The deserializer strips it. This is correct and symmetric.

Item 3: seq::deserialize - Panic on conversion failure exposes debug format

File: crates/common/types/src/beacon/serde_helpers.rs, lines 427-432

C::try_from(values)
    .map_err(|err| serde::de::Error::custom(format!("invalid sequence: {err:?}")))

Using {:?} for error messages leaks internal debug representations to API clients. This occurs in multiple deserializers (seq, ssz_hex_seq, quoted_u64_seq).

Suggestion: Use Display if available, or a generic message:

.map_err(|_| serde::de::Error::custom("invalid sequence: length or conversion failed"))

Item 4: Missing serde::Deserialize on SyncAggregate in electra and later forks

File: crates/common/types/src/beacon/containers/altair.rs, lines 73-85

The SyncAggregate gets serde::Deserialize in altair, but check if electra's SyncAggregate (which reuses or redefines it) also needs this. Looking at the diff, electra.rs doesn't redefine SyncAggregate — it uses altair::SyncAggregate. Good.

However, bellatrix.rs and capella.rs have SyncAggregate fields that reference altair::SyncAggregate. The derive is only on the altair definition. This is correct since it's the same type.

Item 5: gloas.rs - BeaconBlock and SignedBeaconBlock get Default derive with serde::Deserialize

File: crates/common/types/src/beacon/containers/gloas.rs, lines 808-820

#[derive(
    Debug,
    Clone,
    Default,  // <-- here
    PartialEq,
    Eq,
    serde::Serialize,
    serde::Deserialize,
    ...
)]
pub struct BeaconBlock {

Adding Default to BeaconBlock is concerning. A default block with slot: 0, proposer_index: 0, zeroed roots, and an empty body is not a valid block. However, this may be needed for SSZ progressive containers or test fixtures.

Check: Does Default make sense semantically? The BeaconBlockBody also has Default in gloas. This seems intentional for the progressive container feature (#[ssz(progressive_container)]), but verify that a default block cannot accidentally be processed as valid.

Item 6: json_tests.rs - Test uses unwrap() extensively

File: crates/common/types/src/beacon/containers/json_tests.rs

The test file uses .unwrap() ~40 times. For a test file this is acceptable, but the round_trip function panics on failure without context:

fn round_trip<T>(value: &T)
where
    T: serde::Serialize + serde::de::DeserializeOwned + PartialEq + std::fmt::Debug,
{
    let json = serde_json::to_string(value).unwrap();
    let back: T = serde_json::from_str(&json).unwrap();
    assert_eq!(&back, value, "{json}");
}

The assert_eq! with "{json}" format is good for debugging. However, serde_json::from_str failure won't show the JSON that failed.

Suggestion: Consider capturing the JSON in a variable for error reporting, or use expect with the JSON included.

Item 7: fork_schedule - Epoch serialization as string vs number

File: crates/net/rpc/src/beacon/config.rs, lines 55-80

let fork = |previous: [u8; 4], current: [u8; 4], epoch: u64| {
    serde_json::json!({
        "previous_version": hex_string(previous),
        "current_version": hex_string(current),
        "epoch": epoch.to_string(),  // <-- quoted as string
    })
};

The epoch is serialized as a string (epoch.to_string()), but the standard Beacon API typically returns integers as numbers. However, looking at the existing get_spec implementation, it also uses .to_string() for integers (line 120: json!(value.to_string())).

This is intentional and consistent with the existing codebase's approach to avoid JavaScript number precision issues. The test at line 480 confirms: assert_eq!(data[0]["epoch"], "0");

Item 8: BodyEncoding::decode - Silent failure on decode errors

File: crates/net/rpc/src/beacon/mod.rs, lines 134-142

pub(crate) fn decode<T>(self, body: &[u8]) -> Option<T>
where
    T: serde::de::DeserializeOwned + libssz::SszDecode,
{
    match self {
        Self::Json => serde_json::from_slice(body).ok(),
        Self::Ssz => T::from_ssz_bytes(body).ok(),
    }
}

This silently swallows deserialization errors, returning None which becomes a generic 400 Bad Request. This loses diagnostic information.

Security/UX concern: A client sending a slightly malformed body gets the same error as completely wrong data. Consider logging the actual error for debugging:

Self::Json => serde_json::from_slice(body).map_err(|e| {
    tracing::debug!("JSON decode error: {e}");
    e
}).ok(),

Item 9: decode_list - SSZ list decoding doesn't use BodyEncoding::decode

File: crates/net/rpc/src/beacon/mod.rs, lines 148-162

pub(crate) fn decode_list<T>(
    headers: &axum::http::HeaderMap,
    body: &[u8],
) -> Result<Vec<T>, ApiError>
where
    T: serde::de::DeserializeOwned + libssz::SszDecode,
{
    let invalid = || ApiError::BadRequest("invalid request body");
    match BodyEncoding::from_headers(headers)? {
        BodyEncoding::Json => serde_json::from_slice(body).map_err(|_| invalid()),
        BodyEncoding::Ssz => {
            <Vec<T> as libssz::SszDecode>::from_ssz_bytes(body).map_err(|_| invalid())
        }
    }
}

Note that decode_list for SSZ uses <Vec<T> as SszDecode>::from_ssz_bytes, which is the SSZ List[T, N] encoding with offsets. But BodyEncoding::decode for single items uses T::from_ssz_bytes.

Question: Is <Vec<T> as SszDecode> the correct SSZ encoding for a list? In SSZ, a List[T, N] is variable-length with a 4-byte length prefix. The Vec<T> implementation in libssz should handle this. This appears correct but verify that Vec<T>'s SszDecode expects the list encoding, not a fixed-length vector.

Item 10: post_pool_attestations - Missing version header check for SSZ

File: crates/net/rpc/src/beacon/pool.rs, lines 89-105

The post_pool_attestations function uses decode_list which checks Content-Type, but doesn't check Eth-Consensus-Version for SSZ submissions. Looking more carefully, the function does:

let header_fork = match header_fork(&headers, &store) {
    Ok(fork) => fork,
    Err(err) => return err.into_response(),
};

This is called before decode_list. Good — the fork check happens first.

Item 11: gloas_proposal.rs - GloasSignedEnvelopeContents field attributes

File: crates/net/rpc/src/beacon/gloas_proposal.rs, lines 155-162

pub(crate) struct GloasSignedEnvelopeContents {
    pub(crate) signed_execution_payload_envelope: SignedExecutionPayloadEnvelope,
    #[serde(with = "ethlambda_types::beacon::serde_helpers::seq")]
    pub(crate) kzg_proofs: CellKzgProofs,
    #[serde(with = "ethlambda_types::beacon::serde_helpers::ssz_hex_seq")]
    pub(crate) blobs: Blobs,
}

Wait — kzg_proofs uses seq (not ssz_hex_seq), but KzgProof is a 48-byte fixed array that should be hex-encoded. Let me check...

Looking at primitives.rs line 521: impl_hex_deserialize!(BlsPubkey, BlsSignature, H160, KzgCommitment, KzgProof); — so KzgProof has its own Serialize/Deserialize impls that use hex.

But seq calls T::deserialize for each element, which for KzgProof would use its Deserialize impl. Let me verify: CellKzgProofs is SszList<KzgProof, ...>. The seq serializer calls serialize on each element, which for KzgProof uses the hex impl from impl_hex_serialize!.

Wait — there's an asymmetry. Looking at the original FuluBlockContents:

#[serde(with = "ethlambda_types::beacon::serde_helpers::seq")]
pub(crate) kzg_proofs: CellKzgProofs,

This was seq before the PR too. And KzgProof has impl_hex_serialize! and impl_hex_deserialize!. So seq works because it delegates to each element's Serialize/Deserialize. This is correct.

But blobs uses ssz_hex_seq which hex-encodes the SSZ bytes of each element. For Blob (which is SszVector<u8, BYTES_PER_BLOB>), this is correct because Blob's own Serialize might not be hex.

Actually, let me re-check. Looking at the diff for FuluBlockContents:

-#[derive(Debug, Clone, PartialEq, SszEncode, SszDecode, serde::Serialize)]
+#[derive(Debug, Clone, PartialEq, SszEncode, SszDecode, serde::Serialize, serde::Deserialize)]
 pub(crate) struct FuluBlockContents {
     pub(crate) block: BeaconBlock,
-    #[serde(serialize_with = "ethlambda_types::beacon::serde_helpers::seq::serialize")]
+    #[serde(with = "ethlambda_types::beacon::serde_helpers::seq")]
     pub(crate) kzg_proofs: CellKzgProofs,
-    #[serde(serialize_with = "ethlambda_types::beacon::serde_helpers::ssz_hex_seq::serialize")]
+    #[serde(with = "ethlambda_types::beacon::serde_helpers::ssz_hex_seq")]
     pub(crate) blobs: Blobs,
 }

The blobs field was ssz_hex_seq::serialize and is now with = "ssz_hex_seq" (both serialize and deserialize). This is correct.

Item 12: Critical — ssz_hex_seq::deserialize doesn't validate length bounds properly

File: crates/common/types/src/beacon/serde_helpers.rs, lines 370-396

pub fn deserialize<'de, D, C, T>(deserializer: D) -> Result<C, D::Error>
where
    D: serde::Deserializer<'de>,
    C: TryFrom<Vec<T>>,
    C::Error: std::fmt::Debug,
    T: libssz::SszDecode,
{
    let texts = <Vec<String> as serde::Deserialize>::deserialize(deserializer)?;
    let values = texts
        .iter()
        .map(|text| {
            let digits = text.trim();
            let bytes = hex::decode(digits.strip_prefix("0x").unwrap_or(digits))
                .map_err(serde::de::Error::custom)?;
            T::from_ssz_bytes(&bytes).map_err(|err| {
                serde::de::Error::custom(format!("invalid SSZ encoding: {err:?}"))
            })
        })
        .collect::<Result<Vec<T>, _>>()?;
    C::try_from(values)
        .map_err(|err| serde::de::Error::custom(format!("invalid sequence: {err:?}")))
}

The T::from_ssz_bytes for an SszList or SszVector will validate its own bounds. But for Blob which is SszVector<u8, BYTES_PER_BLOB>, from_ssz_bytes checks the length. Good.

However, for Transaction (in ssz_hex_seq usage in ExecutionPayload), Transaction is SszList<u8, MAX_BYTES_PER_TRANSACTION>. The from_ssz_bytes will check this bound. Good.

Item 13: get_validator — Missing state_id validation edge case

File: crates/net/rpc/src/beacon/states.rs, lines 268-296

async fn get_validator(
    Path((state_id, validator_id)): Path<(String, String)>,
    State(store): State<Store>,
) -> Response {

The function uses load(&store, &state_id) which can fail with ApiError::BadRequest for invalid state IDs. Good.

But looking at the ValidatorId::Pubkey branch:

ValidatorId::Pubkey(pubkey) => state
    .iter_validators()
    .zip(state.iter_balances())
    .enumerate()
    .find(|(_, (validator, _))| validator.pubkey == pubkey)
    .map(|(index, found)| (index as ValidatorIndex, found)),

This does a linear scan O(n) over all validators. For a state with 1M validators, this is expensive. However, this matches the spec behavior and the batch post_validators also does linear scans. Acceptable for now, but consider indexing by pubkey in the future.

Item 14: an_oversized_list_is_refused_rather_than_truncated test — Good security test

File: crates/common/types/src/beacon/containers/json_tests.rs, lines 398-405

#[test]
fn an_oversized_list_is_refused_rather_than_truncated() {
    let mut json = serde_json::to_value(electra_block()).unwrap();
    let commitment = json["message"]["body"]["blob_kzg_commitments"][0].clone();
    json["message"]["body"]["blob_kzg_commitments"] = serde_json::Value::Array(vec![
        commitment;
        crate::beacon::preset::MAX_BLOB_COMMITMENTS_PER_BLOCK + 1
    ]);
    assert!(serde_json::from_value::<electra::SignedBeaconBlock>(json).is_err());
}

This is an excellent security test verifying that the TryFrom<Vec<T>> bound check in seq::deserialize rejects oversized lists rather than silently truncating.

Item 15: post_envelope_waiting — JSON envelope with blob_data_included: false uses wrong type

File: crates/net/rpc/src/beacon/gloas_proposal.rs, lines 793-802

} else {
    match encoding.decode::<SignedExecutionPayloadEnvelope>(body) {
        Some(signed) => (signed, None),
        None => {
            return ApiError::BadRequest(
                "the body is not a gloas SignedExecutionPayloadEnvelope",
            )
            .into_response();
        }
    }
}

When blob_data_included is false, the body should be a bare SignedExecutionPayloadEnvelope. With JSON encoding, this decodes correctly. With SSZ, it also decodes correctly. Good.

But wait — the GloasSignedEnvelopeContents struct has kzg_proofs and blobs fields that are not optional. When blob_data_included is true, the JSON must include these fields. The serde::Deserialize derive will require them. This is correct.

Item 16: PayloadCache — Not shown in diff, but insert and get need review

Not in diff, but the PayloadCache is used with GloasBlockContents which now implements Deserialize. Verify that the cache doesn't accidentally deserialize untrusted data.

Summary of Issues

Priority	Item	Location	Issue
Medium	3	serde_helpers.rs	`{:

---

Automated review by Kimi (Moonshot AI) · kimi-k2.6 · custom prompt

[github-actions]

🤖 Claude Code Review

Say in a few words what I'm doing: I've read the whole diff and I'm writing up the review now.

Review of PR 667: JSON request bodies for the Beacon API, plus fork_schedule

Overall: The change is sound and consistent. The serialize_with to with swap is mechanical. BodyEncoding and decode_list route JSON and SSZ through the same typed decode, and the tests check that both encodings give the same outcome. I found nothing consensus-critical. I read the diff but did not build or run the tests. The points below are minor.

Findings

1. Doc comments claim bare integers are accepted, but the code takes strings only. This is in serde_helpers.rs (the quoted_u64_seq::deserialize comment) and primitives.rs (the impl Deserialize for U256 comment).

   • Both deserialize through Vec<String> or String, so [1, 2] or 123 is rejected. quoted_or_bare does accept both.
   • Beacon API JSON always quotes integers, so this is harmless in practice.
   • Either fix the comments, or reuse the quoted_or_bare logic so bare integers work too.

2. The JSON body can use much more memory than the request size. The new publish routes accept JSON up to MAX_PUBLISH_BODY_BYTES, which is 64 MiB and was sized in beacon/mod.rs for SSZ.

   • JSON hex doubles the size of blobs and payload transactions, so a maximal-but-legitimate JSON body could exceed the limit and get a 413. That is unlikely in practice, but the doc comment should say the limit is SSZ-sized.
   • The decode builds a Vec<String> and then a Vec<T> before C::try_from checks the length bound (ssz_hex_seq, seq, quoted_u64_seq). Peak memory per request is therefore several times the body size.
   • This is bounded and the endpoints are operator-facing, so it is not a blocker. A note, or a limit sized per encoding, would help.

3. Missing Content-Type now means JSON on the block and envelope endpoints. Before, a missing header got a 415 there.

   • This matches the Beacon API and the behavior of the pool endpoints, so it is fine.
   • Matching is now exact on application/json and application/octet-stream, where blocks previously used starts_with.
   • from_headers compares case-sensitively, but MIME types are case-insensitive, so Application/JSON gets a 415. This is rare, but eq_ignore_ascii_case is cheap.

4. fork_schedule is correct.

   • Phase0 is its own predecessor, unscheduled forks are skipped, and previous_version chains from the last scheduled fork.
   • Both the all-scheduled and the skipped-fork cases are tested.
   • It assumes epochs increase with fork order and does not enforce it. Config validation at startup presumably guarantees that. If it does not, a misordered config would return a non-monotonic schedule.

5. Tests are good.

   • They cover SSZ and JSON parity for blocks and envelopes, forged signatures, tampered blobs, and a 415 versus 400 split.
   • The json_tests.rs round-trip file and the nimbus spec-key test lock in the contract.
   • One gap: no test sends a bare-integer or malformed quoted value to the new deserializers. Such a test would have caught Item 1.

---

Automated review by Claude (Anthropic) · sonnet · custom prompt