fix(eth2api): decode Transaction as a bare SSZ byte list

crates/core/src/validatorapi/component.rs

@@ -529,7 +529,7 @@ impl Component {
         self.dutydb
             .await_proposal(slot)
             .await
-            .map_err(map_dutydb_error)
+            .map_err(|err| map_dutydb_error("proposal", err))
     }
 
     /// Resolves the validator index for a VC-submitted attestation.
@@ -1067,7 +1067,7 @@ impl Handler for Component {
                 "attestation data not available before deadline",
             )
         })?
-        .map_err(map_dutydb_error)?;
+        .map_err(|err| map_dutydb_error("attestation", err))?;
 
         Ok(AttestationDataResponse { data })
     }
@@ -2045,20 +2045,27 @@ fn upstream_unexpected<R: std::fmt::Debug>(endpoint: &'static str, response: R)
 }
 
 /// Maps a [`crate::dutydb::Error`] into the `ApiError` returned to the client
-/// when an `attestation_data` await fails. `Shutdown` propagates as 503 so the
-/// VC can retry; `AwaitDutyExpired` propagates as 408 — same as a timeout —
-/// since the duty is gone and the data will never arrive. Anything else is a
-/// programming error here and becomes 500.
-fn map_dutydb_error(err: DutyDbError) -> ApiError {
+/// when a duty-data await fails. `Shutdown` propagates as 503 so the VC can
+/// retry; `AwaitDutyExpired` propagates as 408 — same as a timeout — since the
+/// duty is gone and the data will never arrive. Anything else is a programming
+/// error here and becomes 500.
+///
+/// `duty` names the duty being awaited (e.g. `"attestation"`, `"proposal"`) so
+/// the client-visible message matches the request that produced it; this mapper
+/// is shared by more than one endpoint.
+fn map_dutydb_error(duty: &'static str, err: DutyDbError) -> ApiError {
     let (status, message) = match err {
-        DutyDbError::Shutdown => (StatusCode::SERVICE_UNAVAILABLE, "dutydb is shutting down"),
+        DutyDbError::Shutdown => (
+            StatusCode::SERVICE_UNAVAILABLE,
+            "dutydb is shutting down".to_string(),
+        ),
         DutyDbError::AwaitDutyExpired => (
             StatusCode::REQUEST_TIMEOUT,
-            "attestation duty expired before data was stored",
+            format!("{duty} duty expired before data was stored"),
         ),
         _ => (
             StatusCode::INTERNAL_SERVER_ERROR,
-            "await attestation failed",
+            format!("await {duty} failed"),
         ),
     };
     ApiError::new(status, message).with_source(err)
@@ -3169,19 +3176,35 @@ mod tests {
     #[test]
     fn map_dutydb_error_status_codes() {
         assert_eq!(
-            map_dutydb_error(DutyDbError::Shutdown).status_code,
+            map_dutydb_error("attestation", DutyDbError::Shutdown).status_code,
             StatusCode::SERVICE_UNAVAILABLE
         );
         assert_eq!(
-            map_dutydb_error(DutyDbError::AwaitDutyExpired).status_code,
+            map_dutydb_error("attestation", DutyDbError::AwaitDutyExpired).status_code,
             StatusCode::REQUEST_TIMEOUT
         );
         assert_eq!(
-            map_dutydb_error(DutyDbError::UnsupportedDutyType).status_code,
+            map_dutydb_error("attestation", DutyDbError::UnsupportedDutyType).status_code,
             StatusCode::INTERNAL_SERVER_ERROR
         );
     }
 
+    /// The expiry message names the duty that was awaited: the mapper is shared
+    /// by `attestation_data` and `await_proposal`, and previously reported
+    /// every timeout — including a missed block proposal — as an
+    /// attestation.
+    #[test]
+    fn map_dutydb_error_message_names_the_duty() {
+        assert_eq!(
+            map_dutydb_error("proposal", DutyDbError::AwaitDutyExpired).message,
+            "proposal duty expired before data was stored"
+        );
+        assert_eq!(
+            map_dutydb_error("attestation", DutyDbError::AwaitDutyExpired).message,
+            "attestation duty expired before data was stored"
+        );
+    }
+
     /// `upstream_status_error` keeps the upstream response body out of the
     /// client-visible message but preserves it on `source()` so it lands in
     /// the debug log.

crates/core/src/validatorapi/error.rs

@@ -100,6 +100,28 @@ struct ErrorBody {
 
 impl IntoResponse for ApiError {
     fn into_response(self) -> Response {
+        // The `source` never reaches the client (it can carry internal detail),
+        // but it is the only place the underlying cause is recorded — e.g. which
+        // field an SSZ/JSON body failed to decode. Log it here, on the single
+        // path every error response takes, otherwise it is silently dropped.
+        if let Some(source) = &self.source {
+            if self.status_code.is_server_error() {
+                tracing::error!(
+                    status = self.status_code.as_u16(),
+                    message = %self.message,
+                    source = %DisplayChain(source.as_ref()),
+                    "validator api error"
+                );
+            } else {
+                tracing::debug!(
+                    status = self.status_code.as_u16(),
+                    message = %self.message,
+                    source = %DisplayChain(source.as_ref()),
+                    "validator api error"
+                );
+            }
+        }
+
         let body = ErrorBody {
             code: self.status_code.as_u16(),
             message: self.message,
@@ -108,3 +130,20 @@ impl IntoResponse for ApiError {
         (self.status_code, Json(body)).into_response()
     }
 }
+
+/// Renders an error together with its `source()` chain, so a wrapped cause
+/// (such as the inner `ssz::DecodeError` behind a decode failure) is not
+/// truncated to just the outermost message.
+struct DisplayChain<'a>(&'a (dyn std::error::Error + 'static));
+
+impl fmt::Display for DisplayChain<'_> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "{}", self.0)?;
+        let mut current = self.0.source();
+        while let Some(err) = current {
+            write!(f, ": {err}")?;
+            current = err.source();
+        }
+        Ok(())
+    }
+}

crates/eth2api/src/spec/bellatrix.rs

@@ -22,9 +22,22 @@ pub const MAX_BYTES_PER_TRANSACTION: usize = 1_073_741_824;
 pub type BaseFeePerGas = U256;
 
 /// Raw execution transaction bytes.
+///
+/// Spec: `Transaction = ByteList[MAX_BYTES_PER_TRANSACTION]` — a bare SSZ list,
+/// not a container. `struct_behaviour = "transparent"` is therefore required:
+/// without it `ssz_derive` frames every transaction as a single-field container
+/// (a spurious 4-byte offset prefix), which makes any block carrying at least
+/// one transaction fail to decode. Charon models the same type as a plain
+/// `type Transaction []byte`.
+///
+/// `TreeHash` intentionally keeps the derived container behaviour: merkleizing
+/// a one-field container yields the single leaf unchanged, so the root already
+/// equals the bare list's root (locked in by the `*_beacon_block_body_root`
+/// spec-vector tests, whose fixtures carry transactions).
 #[serde_as]
 #[derive(Debug, Clone, PartialEq, Eq, Encode, Decode, TreeHash, Serialize, Deserialize)]
 #[serde(transparent)]
+#[ssz(struct_behaviour = "transparent")]
 pub struct Transaction {
     /// Transaction bytes.
     #[serde_as(as = "pluto_ssz::serde_utils::Hex0x")]
@@ -358,4 +371,53 @@ mod tests {
     fn json_matches_vector(actual: serde_json::Value, expected_json: &'static str) {
         test_fixtures::assert_json_eq(actual, expected_json);
     }
+
+    /// `Transaction` is `ByteList[MAX_BYTES_PER_TRANSACTION]`, so its SSZ form
+    /// is the raw transaction bytes with no framing. Without
+    /// `struct_behaviour = "transparent"` the derive emitted a leading 4-byte
+    /// offset, which made every block carrying a transaction undecodable.
+    #[test]
+    fn transaction_ssz_is_the_bare_byte_list() {
+        use ssz::{Decode, Encode};
+
+        // A realistic type-3 (blob) transaction prefix: the first four bytes are
+        // not a valid container offset, which is exactly why container framing
+        // broke decoding.
+        let raw = vec![0x03, 0xf8, 0xb9, 0x83, 0xde, 0xad, 0xbe, 0xef];
+        let tx = super::Transaction::from(raw.clone());
+
+        assert_eq!(tx.as_ssz_bytes(), raw, "encoding must not add framing");
+        assert_eq!(tx.ssz_bytes_len(), raw.len());
+        assert_eq!(
+            super::Transaction::from_ssz_bytes(&raw).expect("raw bytes decode"),
+            tx
+        );
+        // Empty transactions are a valid zero-length list.
+        assert!(super::Transaction::from_ssz_bytes(&[]).is_ok());
+    }
+
+    /// An execution payload carrying transactions must survive an SSZ round
+    /// trip. Previously the fixtures only exercised `TreeHash` and JSON, so
+    /// the broken SSZ framing went unnoticed.
+    #[test]
+    fn execution_payload_with_transactions_ssz_round_trips() {
+        use ssz::{Decode, Encode};
+
+        let payload = test_fixtures::bellatrix_execution_payload_fixture();
+        assert!(
+            !payload.transactions.0.is_empty(),
+            "fixture must carry transactions for this to be meaningful"
+        );
+
+        let bytes = payload.as_ssz_bytes();
+        let decoded =
+            super::ExecutionPayload::from_ssz_bytes(&bytes).expect("payload round trips via ssz");
+        assert_eq!(decoded, payload);
+
+        // The transactions list is `List[Transaction, N]`: a 4-byte offset per
+        // element followed by each element's bare bytes.
+        let txs = &payload.transactions.0;
+        let expected_len = txs.len() * 4 + txs.iter().map(|tx| tx.bytes.0.len()).sum::<usize>();
+        assert_eq!(payload.transactions.as_ssz_bytes().len(), expected_len);
+    }
 }