From 5330e0b455ab5f6d1b91212156f79c696cc35985 Mon Sep 17 00:00:00 2001 From: jsign Date: Mon, 21 Sep 2026 16:57:06 -0300 Subject: [PATCH 1/2] feat(l1): add payload witnesses and sender public keys to REST/SSZ Engine API Signed-off-by: jsign --- crates/common/crypto/provider.rs | 33 +++ crates/common/types/transaction.rs | 139 ++++++++++- crates/networking/rpc/Cargo.toml | 2 +- crates/networking/rpc/engine/payload.rs | 109 +++++---- .../rpc/engine_rest/handlers/capabilities.rs | 9 +- .../rpc/engine_rest/handlers/payloads.rs | 119 ++++++++-- crates/networking/rpc/engine_rest/mod.rs | 5 + .../networking/rpc/engine_rest/types/mod.rs | 1 + .../rpc/engine_rest/types/witness.rs | 215 ++++++++++++++++++ test/tests/rpc/engine_rest_tests.rs | 4 +- 10 files changed, 572 insertions(+), 64 deletions(-) create mode 100644 crates/networking/rpc/engine_rest/types/witness.rs diff --git a/crates/common/crypto/provider.rs b/crates/common/crypto/provider.rs index 5eca02782db..8278e6cc456 100644 --- a/crates/common/crypto/provider.rs +++ b/crates/common/crypto/provider.rs @@ -160,6 +160,39 @@ pub trait Crypto: Send + Sync + core::fmt::Debug { Ok(hash) } + /// Recover the signer's **uncompressed** secp256k1 public key (`0x04 || X || Y`). + /// + /// REST payload witness responses include the full key for each transaction. + /// Unlike [`Self::recover_signer`], this returns the key before hashing it + /// to derive the sender address. Both apply the same EIP-2 low-s rejection. + /// + /// Host-only: requires the native `secp256k1` feature. + #[cfg(feature = "secp256k1")] + fn recover_public_key(&self, sig: &[u8; 65], msg: &[u8; 32]) -> Result<[u8; 65], CryptoError> { + // EIP-2: reject high-s signatures (s > secp256k1n/2), matching recover_signer. + const SECP256K1_N_HALF: [u8; 32] = + hex_literal::hex!("7fffffffffffffffffffffffffffffff5d576e7357a4501ddfe92f46681b20a0"); + if sig[32..64] > SECP256K1_N_HALF[..] { + return Err(CryptoError::InvalidSignature); + } + + let recovery_id = secp256k1::ecdsa::RecoveryId::try_from(sig[64] as i32) + .map_err(|_| CryptoError::InvalidRecoveryId)?; + let recoverable_sig = secp256k1::ecdsa::RecoverableSignature::from_compact( + sig[..64] + .try_into() + .map_err(|_| CryptoError::InvalidSignature)?, + recovery_id, + ) + .map_err(|_| CryptoError::InvalidSignature)?; + + let public_key = recoverable_sig + .recover(&secp256k1::Message::from_digest(*msg)) + .map_err(|_| CryptoError::RecoveryFailed)?; + + Ok(public_key.serialize_uncompressed()) + } + /// Recover the signer address from a 65-byte signature (r||s||v) + 32-byte message hash. /// Used by transaction validation (tx.sender()) and EIP-7702 authority recovery. fn recover_signer(&self, sig: &[u8; 65], msg: &[u8; 32]) -> Result { diff --git a/crates/common/types/transaction.rs b/crates/common/types/transaction.rs index adec10daf03..d93ab204b35 100644 --- a/crates/common/types/transaction.rs +++ b/crates/common/types/transaction.rs @@ -36,6 +36,9 @@ pub use serde_impl::{ GenericTransactionError, }; +/// Signing preimage and compact recoverable signature (r || s || parity). +pub type SigningPayload = (Vec, [u8; 65]); + /// The serialized length of a default eip1559 transaction pub const EIP1559_DEFAULT_SERIALIZED_LENGTH: usize = 15; @@ -1247,7 +1250,11 @@ impl Transaction { .copied() } - fn compute_sender(&self, crypto: &dyn Crypto) -> Result { + /// The bytes that were signed, plus the 65-byte `r || s || v` signature. + /// + /// `Ok(None)` for transactions that carry an explicit sender and no signature + /// (privileged L2 and frame), for which there is nothing to recover. + pub fn signing_payload(&self) -> Result, CryptoError> { let (buf, sig) = match self { Transaction::LegacyTransaction(tx) => { let v = u64::try_from(tx.v).map_err(|_| CryptoError::InvalidSignature)?; @@ -1373,8 +1380,10 @@ impl Transaction { sig[64] = tx.signature_y_parity as u8; (buf, sig) } - Transaction::PrivilegedL2Transaction(tx) => return Ok(tx.from), - Transaction::FrameTransaction(tx) => return Ok(tx.sender), + // Explicit sender, no signature: nothing to recover from. + Transaction::PrivilegedL2Transaction(_) | Transaction::FrameTransaction(_) => { + return Ok(None); + } Transaction::FeeTokenTransaction(tx) => { let mut buf = vec![self.tx_type() as u8]; Encoder::new(&mut buf) @@ -1396,10 +1405,37 @@ impl Transaction { (buf, sig) } }; + Ok(Some((buf, sig))) + } + + fn compute_sender(&self, crypto: &dyn Crypto) -> Result { + match self { + Transaction::PrivilegedL2Transaction(tx) => return Ok(tx.from), + Transaction::FrameTransaction(tx) => return Ok(tx.sender), + _ => {} + } + let Some((buf, sig)) = self.signing_payload()? else { + // Unreachable: the two signature-less variants are handled above. + return Err(CryptoError::InvalidSignature); + }; let msg = crypto.keccak256(&buf); crypto.recover_signer(&sig, &msg) } + /// The signer's uncompressed secp256k1 public key (`0x04 || X || Y`). + /// + /// `Ok(None)` for privileged L2 and frame transactions, which carry an explicit + /// sender and no signature. REST payload witness responses include these + /// keys so stateless consumers can verify transaction signatures. + #[cfg(feature = "secp256k1")] + pub fn public_key(&self, crypto: &dyn Crypto) -> Result, CryptoError> { + let Some((buf, sig)) = self.signing_payload()? else { + return Ok(None); + }; + let msg = crypto.keccak256(&buf); + crypto.recover_public_key(&sig, &msg).map(Some) + } + pub fn gas_limit(&self) -> u64 { match self { Transaction::LegacyTransaction(tx) => tx.gas, @@ -5861,3 +5897,100 @@ mod tests { ); } } + +#[cfg(all(test, feature = "secp256k1"))] +mod public_key_tests { + use super::*; + use ethrex_crypto::NativeCrypto; + use secp256k1::{Message, PublicKey, SECP256K1, SecretKey}; + + #[test] + fn public_keys_match_signers_for_every_l1_signature_format() { + let secret = SecretKey::from_byte_array(&[7; 32]).unwrap(); + let expected = PublicKey::from_secret_key(SECP256K1, &secret).serialize_uncompressed(); + let expected_sender = Address::from_slice(&NativeCrypto.keccak256(&expected[1..])[12..]); + for mut tx in [ + Transaction::LegacyTransaction(LegacyTransaction { + v: 27.into(), + ..Default::default() + }), + Transaction::LegacyTransaction(LegacyTransaction { + v: 37.into(), + ..Default::default() + }), + Transaction::EIP2930Transaction(Default::default()), + Transaction::EIP1559Transaction(Default::default()), + Transaction::EIP4844Transaction(Default::default()), + Transaction::EIP7702Transaction(Default::default()), + ] { + let (preimage, _) = tx.signing_payload().unwrap().unwrap(); + let msg = NativeCrypto.keccak256(&preimage); + let (id, sig) = SECP256K1 + .sign_ecdsa_recoverable(&Message::from_digest(msg), &secret) + .serialize_compact(); + let r = U256::from_big_endian(&sig[..32]); + let s = U256::from_big_endian(&sig[32..]); + let parity = i32::from(id) != 0; + macro_rules! typed { + ($t:expr) => {{ + $t.signature_r = r; + $t.signature_s = s; + $t.signature_y_parity = parity; + }}; + } + match &mut tx { + Transaction::LegacyTransaction(t) => { + t.r = r; + t.s = s; + t.v += U256::from(parity as u8); + } + Transaction::EIP2930Transaction(t) => typed!(t), + Transaction::EIP1559Transaction(t) => typed!(t), + Transaction::EIP4844Transaction(t) => typed!(t), + Transaction::EIP7702Transaction(t) => typed!(t), + _ => unreachable!(), + } + assert_eq!(tx.public_key(&NativeCrypto).unwrap(), Some(expected)); + // Warm both sender caches; public-key recovery must still work. + assert_eq!(tx.sender(&NativeCrypto).unwrap(), expected_sender); + assert_eq!(tx.public_key(&NativeCrypto).unwrap(), Some(expected)); + let mut signature = [0; 65]; + signature[..64].copy_from_slice(&sig); + signature[64] = parity as u8; + signature[64] ^= 1; + assert_ne!( + NativeCrypto.recover_public_key(&signature, &msg).unwrap(), + expected + ); + signature[64] = 4; + assert!(NativeCrypto.recover_public_key(&signature, &msg).is_err()); + signature[64] = parity as u8; + signature[32..64].fill(0xff); + assert!(NativeCrypto.recover_public_key(&signature, &msg).is_err()); + } + for v in [0, 1, 26, 29, 34] { + let tx = Transaction::LegacyTransaction(LegacyTransaction { + v: v.into(), + ..Default::default() + }); + assert!(tx.public_key(&NativeCrypto).is_err()); + } + assert!( + Transaction::EIP1559Transaction(Default::default()) + .public_key(&NativeCrypto) + .is_err() + ); + assert_eq!( + Transaction::PrivilegedL2Transaction(Default::default()) + .public_key(&NativeCrypto) + .unwrap(), + None + ); + assert_eq!( + Transaction::FrameTransaction(Default::default()) + .public_key(&NativeCrypto) + .unwrap(), + None + ); + } +} diff --git a/crates/networking/rpc/Cargo.toml b/crates/networking/rpc/Cargo.toml index 94a91a2df9b..9d6bf2986b5 100644 --- a/crates/networking/rpc/Cargo.toml +++ b/crates/networking/rpc/Cargo.toml @@ -24,7 +24,7 @@ tokio = { workspace = true, features = ["full"] } bytes.workspace = true tracing.workspace = true tracing-subscriber.workspace = true -ethrex-common.workspace = true +ethrex-common = { workspace = true, features = ["secp256k1"] } ethrex-storage.workspace = true ethrex-vm.workspace = true ethrex-blockchain.workspace = true diff --git a/crates/networking/rpc/engine/payload.rs b/crates/networking/rpc/engine/payload.rs index 0289e68736a..933fe6ffff7 100644 --- a/crates/networking/rpc/engine/payload.rs +++ b/crates/networking/rpc/engine/payload.rs @@ -53,7 +53,8 @@ impl RpcHandler for NewPayloadV1Request { }; let payload_status = handle_new_payload_v1_v2(self.payload.block_hash, block, context, None, false).await?; - serde_json::to_value(payload_status).map_err(|error| RpcErr::Internal(error.to_string())) + serde_json::to_value(payload_status.into_json_rpc()?) + .map_err(|error| RpcErr::Internal(error.to_string())) } } @@ -86,7 +87,8 @@ impl RpcHandler for NewPayloadV2Request { }; let payload_status = handle_new_payload_v1_v2(self.payload.block_hash, block, context, None, false).await?; - serde_json::to_value(payload_status).map_err(|error| RpcErr::Internal(error.to_string())) + serde_json::to_value(payload_status.into_json_rpc()?) + .map_err(|error| RpcErr::Internal(error.to_string())) } } @@ -153,7 +155,8 @@ impl RpcHandler for NewPayloadV3Request { false, ) .await?; - serde_json::to_value(payload_status).map_err(|error| RpcErr::Internal(error.to_string())) + serde_json::to_value(payload_status.into_json_rpc()?) + .map_err(|error| RpcErr::Internal(error.to_string())) } } @@ -266,7 +269,8 @@ impl RpcHandler for NewPayloadV4Request { false, ) .await?; - serde_json::to_value(payload_status).map_err(|error| RpcErr::Internal(error.to_string())) + serde_json::to_value(payload_status.into_json_rpc()?) + .map_err(|error| RpcErr::Internal(error.to_string())) } } @@ -416,7 +420,8 @@ impl NewPayloadV5Request { make_witness, ) .await?; - serde_json::to_value(payload_status).map_err(|error| RpcErr::Internal(error.to_string())) + serde_json::to_value(payload_status.into_json_rpc()?) + .map_err(|error| RpcErr::Internal(error.to_string())) } } @@ -1052,13 +1057,38 @@ async fn validate_ancestors( Ok(None) } +/// Execution result kept structured until the transport chooses its encoding. +pub(crate) struct PayloadSubmission { + pub status: PayloadStatus, + pub witness: Option, +} + +impl From for PayloadSubmission { + fn from(status: PayloadStatus) -> Self { + Self { + status, + witness: None, + } + } +} + +impl PayloadSubmission { + fn into_json_rpc(mut self) -> Result { + self.status.witness = self + .witness + .map(encode_rpc_witness_for_engine_rpc) + .transpose()?; + Ok(self.status) + } +} + pub(crate) async fn handle_new_payload_v1_v2( expected_block_hash: H256, block: Block, context: RpcApiContext, bal: Option, make_witness: bool, -) -> Result { +) -> Result { let Some(syncer) = &context.syncer else { return Err(RpcErr::Internal( "New payload requested but syncer is not initialized".to_string(), @@ -1066,12 +1096,12 @@ pub(crate) async fn handle_new_payload_v1_v2( }; // Validate block hash if let Err(RpcErr::Internal(error_msg)) = validate_block_hash(expected_block_hash, &block) { - return Ok(PayloadStatus::invalid_with_err(&error_msg)); + return Ok(PayloadStatus::invalid_with_err(&error_msg).into()); } // Check for invalid ancestors if let Some(status) = validate_ancestors(&block, &context).await? { - return Ok(status); + return Ok(status.into()); } // We have validated ancestors, the parent is correct @@ -1079,7 +1109,7 @@ pub(crate) async fn handle_new_payload_v1_v2( if syncer.sync_mode() == SyncMode::Snap { debug!("Snap sync in progress, skipping new payload validation"); - return Ok(PayloadStatus::syncing()); + return Ok(PayloadStatus::syncing().into()); } // All checks passed, execute payload @@ -1095,7 +1125,7 @@ pub(crate) async fn handle_new_payload_v3( expected_blob_versioned_hashes: Option>, bal: Option, make_witness: bool, -) -> Result { +) -> Result { // V3 specific: validate blob hashes (skipped when None, e.g. REST callers) if let Some(expected) = expected_blob_versioned_hashes { let blob_versioned_hashes: Vec = block @@ -1106,9 +1136,7 @@ pub(crate) async fn handle_new_payload_v3( .collect(); if expected != blob_versioned_hashes { - return Ok(PayloadStatus::invalid_with_err( - "Invalid blob_versioned_hashes", - )); + return Ok(PayloadStatus::invalid_with_err("Invalid blob_versioned_hashes").into()); } } @@ -1122,11 +1150,11 @@ pub(crate) async fn handle_new_payload_v4( expected_blob_versioned_hashes: Option>, bal: Option, make_witness: bool, -) -> Result { +) -> Result { if let Some(bal) = &bal && let Err(err) = bal.validate_ordering() { - return Ok(PayloadStatus::invalid_with_err(&err)); + return Ok(PayloadStatus::invalid_with_err(&err).into()); } handle_new_payload_v3( expected_block_hash, @@ -1210,7 +1238,7 @@ async fn try_execute_payload( latest_valid_hash: H256, bal: Option, make_witness: bool, -) -> Result { +) -> Result { let Some(syncer) = &context.syncer else { return Err(RpcErr::Internal( "New payload requested but syncer is not initialized".to_string(), @@ -1226,7 +1254,7 @@ async fn try_execute_payload( // replay reads would then fall through to old-chain disk state. Defer to // SYNCING (the CL retries) for the duration of the pass. if context.blockchain.is_reorg_in_progress() { - return Ok(PayloadStatus::syncing()); + return Ok(PayloadStatus::syncing().into()); } // Fast path: if we already have this block's header AND its state is reachable, // we know it has been fully validated previously and can reply VALID (with a @@ -1302,7 +1330,7 @@ async fn try_execute_payload( "Parent state not materialized; stashing payload as ACCEPTED" ); storage.add_block(block).await?; - return Ok(PayloadStatus::accepted()); + return Ok(PayloadStatus::accepted().into()); } } @@ -1318,7 +1346,7 @@ async fn try_execute_payload( Err(ChainError::ParentNotFound) => { // Start sync syncer.sync_to_head(block_hash); - Ok(PayloadStatus::syncing()) + Ok(PayloadStatus::syncing().into()) } // Parent block is present but its state isn't available yet (e.g. state // regeneration after a restart hasn't reached the CL head). This is a @@ -1327,7 +1355,7 @@ async fn try_execute_payload( Err(ChainError::ParentStateNotFound) => { debug!(%block_hash, "Parent state not found, returning SYNCING and triggering sync"); syncer.sync_to_head(block_hash); - Ok(PayloadStatus::syncing()) + Ok(PayloadStatus::syncing().into()) } Err(ChainError::InvalidBlock(error)) => { warn!(%block_hash, %block_number, "Error executing block: {error}"); @@ -1336,10 +1364,7 @@ async fn try_execute_payload( .set_latest_valid_ancestor(block_hash, latest_valid_hash) .await?; context.storage.add_bad_block(bad_block_candidate).await?; - Ok(PayloadStatus::invalid_with( - latest_valid_hash, - error.to_string(), - )) + Ok(PayloadStatus::invalid_with(latest_valid_hash, error.to_string()).into()) } Err(ChainError::EvmError(error)) => { warn!(%block_hash, %block_number, "Error executing block: {error}"); @@ -1348,10 +1373,7 @@ async fn try_execute_payload( .set_latest_valid_ancestor(block_hash, latest_valid_hash) .await?; context.storage.add_bad_block(bad_block_candidate).await?; - Ok(PayloadStatus::invalid_with( - latest_valid_hash, - error.to_string(), - )) + Ok(PayloadStatus::invalid_with(latest_valid_hash, error.to_string()).into()) } Err(ChainError::StoreError(error)) => { warn!(%block_hash, %block_number, "Error storing block: {error}"); @@ -1363,14 +1385,14 @@ async fn try_execute_payload( } Ok(witness) => { debug!("Block with hash {block_hash} executed and added to storage successfully"); - let mut status = PayloadStatus::valid_with_hash(block_hash); + let mut result = PayloadSubmission::from(PayloadStatus::valid_with_hash(block_hash)); if make_witness { let witness = witness.ok_or_else(|| { RpcErr::Internal("Payload executed without producing a witness".to_string()) })?; - status.witness = Some(encode_witness_for_engine_rpc(witness)?); + result.witness = Some(flatten_witness(witness)?); } - Ok(status) + Ok(result) } } } @@ -1379,21 +1401,21 @@ async fn payload_status_for_existing_block( block: &Block, context: &RpcApiContext, make_witness: bool, -) -> Result { +) -> Result { let block_hash = block.hash(); - let mut status = PayloadStatus::valid_with_hash(block_hash); + let mut result = PayloadSubmission::from(PayloadStatus::valid_with_hash(block_hash)); if make_witness { - status.witness = Some(witness_for_existing_block(block, context).await?); + result.witness = Some(witness_for_existing_block(block, context).await?); } - Ok(status) + Ok(result) } async fn witness_for_existing_block( block: &Block, context: &RpcApiContext, -) -> Result { +) -> Result { let block_hash = block.hash(); if let Some(json_bytes) = context .storage @@ -1402,7 +1424,7 @@ async fn witness_for_existing_block( let rpc_witness = serde_json::from_slice(&json_bytes).map_err(|error| { RpcErr::Internal(format!("Failed to parse cached witness: {error}")) })?; - return encode_rpc_witness_for_engine_rpc(rpc_witness); + return Ok(rpc_witness); } let witness = context @@ -1410,14 +1432,17 @@ async fn witness_for_existing_block( .generate_witness_for_blocks(std::slice::from_ref(block)) .await .map_err(|error| RpcErr::Internal(format!("Failed to build execution witness: {error}")))?; - encode_witness_for_engine_rpc(witness) + flatten_witness(witness) +} + +fn flatten_witness(witness: ExecutionWitness) -> Result { + RpcExecutionWitness::try_from(witness) + .map_err(|error| RpcErr::Internal(format!("Failed to encode execution witness: {error}"))) } +#[cfg(test)] fn encode_witness_for_engine_rpc(witness: ExecutionWitness) -> Result { - let rpc_witness = RpcExecutionWitness::try_from(witness).map_err(|error| { - RpcErr::Internal(format!("Failed to encode execution witness: {error}")) - })?; - encode_rpc_witness_for_engine_rpc(rpc_witness) + encode_rpc_witness_for_engine_rpc(flatten_witness(witness)?) } /// Encodes the witness in geth's opaque `engine_newPayloadWithWitness*` shape. diff --git a/crates/networking/rpc/engine_rest/handlers/capabilities.rs b/crates/networking/rpc/engine_rest/handlers/capabilities.rs index 37dbb009c88..0f51be45628 100644 --- a/crates/networking/rpc/engine_rest/handlers/capabilities.rs +++ b/crates/networking/rpc/engine_rest/handlers/capabilities.rs @@ -6,7 +6,7 @@ //! ```json //! { //! "supported_forks": ["paris", ...], -//! "fork_scoped_endpoints": ["payloads", "forkchoice", "bodies"], +//! "fork_scoped_endpoints": ["payloads", "payloads/witness", "forkchoice", "bodies"], //! "independently_versioned": { "blobs": ["v1", ...] }, //! "unscoped_endpoints": ["capabilities", "identity"], //! "limits": { "bodies.max_count": N, "blobs.max_versioned_hashes": N, "payload.max_bytes": N } @@ -71,7 +71,12 @@ pub fn capabilities() -> Capabilities { "osaka".into(), "amsterdam".into(), ], - fork_scoped_endpoints: vec!["payloads".into(), "forkchoice".into(), "bodies".into()], + fork_scoped_endpoints: vec![ + "payloads".into(), + "payloads/witness".into(), + "forkchoice".into(), + "bodies".into(), + ], independently_versioned: IndependentlyVersioned { blobs: vec!["v1".into(), "v2".into(), "v3".into(), "v4".into()], }, diff --git a/crates/networking/rpc/engine_rest/handlers/payloads.rs b/crates/networking/rpc/engine_rest/handlers/payloads.rs index a9a14a64205..7f44610c4a4 100644 --- a/crates/networking/rpc/engine_rest/handlers/payloads.rs +++ b/crates/networking/rpc/engine_rest/handlers/payloads.rs @@ -1,4 +1,4 @@ -//! Real handlers for `POST /payloads` and `GET /payloads/{id}`. The fork is +//! Handlers for `POST /payloads`, `POST /payloads/witness` and `GET /payloads/{id}`. The fork is //! selected by the `Eth-Execution-Version` request header. use std::str::FromStr; @@ -27,10 +27,12 @@ use crate::engine_rest::types::built_payload::{ BlobsBundleV1, BlobsBundleV2, BuiltPayloadAmsterdam, BuiltPayloadCancun, BuiltPayloadOsaka, BuiltPayloadParis, BuiltPayloadPrague, BuiltPayloadShanghai, MAX_BLOB_COMMITMENTS_PER_BLOCK, }; +use crate::engine_rest::types::common::to_optional; use crate::engine_rest::types::common::{ Bytes20, PayloadId, PayloadStatus as SszPayloadStatus, PayloadStatusCode, }; use crate::engine_rest::types::conversions::{DecodedNewPayload, EngineCall, IntoEngineCall}; +use crate::engine_rest::types::witness::{ExecutionWitness, PayloadStatusWithWitness, PublicKeys}; use crate::engine_rest::types::{amsterdam, cancun, paris, prague, shanghai}; use crate::rpc::RpcApiContext; use crate::types::payload::PayloadValidationStatus; @@ -42,6 +44,19 @@ pub(crate) async fn submit_payload( State(ctx): State, req: Request, ) -> Response { + submit(fork, ctx, req, false).await +} + +pub(crate) async fn submit_payload_with_witness( + ExecutionVersion(fork): ExecutionVersion, + State(ctx): State, + req: Request, +) -> Response { + // Ethrex also supports pre-Amsterdam witnesses using each fork's envelope. + submit(fork, ctx, req, true).await +} + +async fn submit(fork: Fork, ctx: RpcApiContext, req: Request, make_witness: bool) -> Response { if let Err(p) = check_content_type(req.headers()) { return p.into_response(); } @@ -58,16 +73,30 @@ pub(crate) async fn submit_payload( } }; match fork { - Fork::Paris => decode_and_submit::(body, ctx).await, - Fork::Shanghai => decode_and_submit::(body, ctx).await, - Fork::Cancun => decode_and_submit::(body, ctx).await, - Fork::Prague => decode_and_submit::(body, ctx).await, + Fork::Paris => { + decode_and_submit::(body, ctx, fork, make_witness) + .await + } + Fork::Shanghai => { + decode_and_submit::(body, ctx, fork, make_witness) + .await + } + Fork::Cancun => { + decode_and_submit::(body, ctx, fork, make_witness) + .await + } + Fork::Prague => { + decode_and_submit::(body, ctx, fork, make_witness) + .await + } Fork::Osaka => { // Osaka re-exports Prague's envelope (see types/osaka.rs); same shape. - decode_and_submit::(body, ctx).await + decode_and_submit::(body, ctx, fork, make_witness) + .await } Fork::Amsterdam => { - decode_and_submit::(body, ctx).await + decode_and_submit::(body, ctx, fork, make_witness) + .await } // Unreachable: the ExecutionVersion header extractor rejects all // non-spec forks with 400 before the handler runs. @@ -75,7 +104,12 @@ pub(crate) async fn submit_payload( } } -async fn decode_and_submit(body: Bytes, ctx: RpcApiContext) -> Response +async fn decode_and_submit( + body: Bytes, + ctx: RpcApiContext, + fork: Fork, + make_witness: bool, +) -> Response where T: libssz::SszDecode + IntoEngineCall, { @@ -142,7 +176,9 @@ where } EngineCall::V5 { .. } => !chain_config.is_amsterdam_activated(ts), }; - if misrouted { + // Witness requests select a specific REST fork even when Engine versions + // share a handler (Paris/Shanghai and Prague/Osaka). + if misrouted || (make_witness && !fork_covers_timestamp(&chain_config, fork, ts)) { return ProblemJson::unsupported_fork(&format!( "Eth-Execution-Version does not match the payload's fork: {:?}", chain_config.get_fork(ts) @@ -161,17 +197,25 @@ where // binds the CL and EL to the same transactions, so a mismatch still // surfaces as INVALID. The JSON-RPC path keeps the explicit cross-check // (it still receives the param); only this transport drops it. + // Keep transactions only for the witness response; address caches cannot + // reconstruct public keys. Recovery happens after successful validation. + let transactions = if make_witness { + block.body.transactions.clone() + } else { + Vec::new() + }; + let parent_hash = block.header.parent_hash; let result = match call { EngineCall::V1V2 => { - handle_new_payload_v1_v2(expected_block_hash, block, ctx, None, false).await + handle_new_payload_v1_v2(expected_block_hash, block, ctx, None, make_witness).await } EngineCall::V3 => { - handle_new_payload_v3(expected_block_hash, ctx, block, None, None, false).await + handle_new_payload_v3(expected_block_hash, ctx, block, None, None, make_witness).await } // Prague (V4) reuses handle_new_payload_v3 — matches the JSON-RPC // NewPayloadV4Request::handle behavior in engine/payload.rs. EngineCall::V4 => { - handle_new_payload_v3(expected_block_hash, ctx, block, None, None, false).await + handle_new_payload_v3(expected_block_hash, ctx, block, None, None, make_witness).await } EngineCall::V5 { .. } => { // Pass the decoded BAL so handle_new_payload_v4 runs validate_ordering, @@ -182,20 +226,21 @@ where block, None, block_access_list, - false, + make_witness, ) .await } }; // 5. Map internal PayloadStatus → SSZ PayloadStatus. - let internal_status = match result { + let submission = match result { Ok(s) => s, Err(err) => { return ProblemJson::internal(&format!("engine error: {err}")).into_response(); } }; + let internal_status = submission.status; let status_code: u8 = match internal_status.status { PayloadValidationStatus::Valid => PayloadStatusCode::Valid as u8, PayloadValidationStatus::Invalid => PayloadStatusCode::Invalid as u8, @@ -208,7 +253,51 @@ where internal_status.validation_error, ); - SszBody(ssz_status).into_response() + if !make_witness { + return SszBody(ssz_status).into_response(); + } + let response = (|| -> Result { + let (witness, public_keys) = if internal_status.status == PayloadValidationStatus::Valid { + let witness = submission + .witness + .ok_or_else(|| ProblemJson::internal("valid payload has no witness"))?; + let witness = ExecutionWitness::from_rpc(witness, parent_hash)?; + let keys = transactions + .iter() + .map(|tx| { + let key = tx + .public_key(ðrex_crypto::NativeCrypto) + .map_err(|e| { + ProblemJson::internal(&format!("public key recovery failed: {e}")) + })? + .ok_or_else(|| { + ProblemJson::internal( + "valid payload transaction has no sender public key", + ) + })?; + SszVector::::try_from(key.to_vec()) + .map_err(|_| ProblemJson::internal("invalid public key length")) + }) + .collect::, _>>()?; + let keys: PublicKeys = keys + .try_into() + .map_err(|_| ProblemJson::internal("too many transaction public keys"))?; + (to_optional(Some(witness)), keys) + } else { + (to_optional(None), PublicKeys::default()) + }; + let response = PayloadStatusWithWitness { + payload_status: ssz_status, + witness, + public_keys, + }; + response.check_encoded_length()?; + Ok(response) + })(); + match response { + Ok(response) => SszBody(response).into_response(), + Err(problem) => problem.into_response(), + } } // ── get_payload ─────────────────────────────────────────────────────────────── diff --git a/crates/networking/rpc/engine_rest/mod.rs b/crates/networking/rpc/engine_rest/mod.rs index 8c3eae26038..2abb33bda8d 100644 --- a/crates/networking/rpc/engine_rest/mod.rs +++ b/crates/networking/rpc/engine_rest/mod.rs @@ -33,6 +33,7 @@ pub(crate) const CONTENT_TYPE_OCTET_STREAM: &str = "application/octet-stream"; /// GET /identity /// GET /capabilities /// POST /payloads +/// POST /payloads/witness /// GET /payloads/{id} /// POST /forkchoice /// POST /bodies/hash @@ -55,6 +56,10 @@ pub fn router(ctx: RpcApiContext) -> Router { get(handlers::capabilities::get_capabilities), ) .route("/payloads", post(handlers::payloads::submit_payload)) + .route( + "/payloads/witness", + post(handlers::payloads::submit_payload_with_witness), + ) .route("/payloads/{id}", get(handlers::payloads::get_payload)) .route("/forkchoice", post(handlers::forkchoice::forkchoice_update)) .route("/bodies/hash", post(handlers::bodies::bodies_by_hash)) diff --git a/crates/networking/rpc/engine_rest/types/mod.rs b/crates/networking/rpc/engine_rest/types/mod.rs index 8d1fdda5e24..b80862e1ee1 100644 --- a/crates/networking/rpc/engine_rest/types/mod.rs +++ b/crates/networking/rpc/engine_rest/types/mod.rs @@ -16,3 +16,4 @@ pub mod osaka; pub mod paris; pub mod prague; pub mod shanghai; +pub mod witness; diff --git a/crates/networking/rpc/engine_rest/types/witness.rs b/crates/networking/rpc/engine_rest/types/witness.rs new file mode 100644 index 00000000000..89ef909dd19 --- /dev/null +++ b/crates/networking/rpc/engine_rest/types/witness.rs @@ -0,0 +1,215 @@ +//! Payload witness response from execution-apis #885 at e473f58911e49cc619fb4102d3973e4e049322f0. +//! These bounded REST lists deliberately differ from stateless guest containers. + +use ethrex_common::H256; +use ethrex_common::types::{BlockHeader, block_execution_witness::RpcExecutionWitness}; +use ethrex_rlp::decode::RLPDecode; +use libssz::SszEncode; +use libssz_derive::{SszDecode, SszEncode}; +use libssz_types::{SszList, SszVector}; + +use super::common::{MAX_TRANSACTIONS_PER_PAYLOAD, PayloadStatus}; +use crate::engine_rest::error::ProblemJson; + +pub const MAX_WITNESS_ITEMS: usize = 1 << 20; +pub const MAX_WITNESS_ITEM_BYTES: usize = 1 << 20; +pub type WitnessItems = SszList, MAX_WITNESS_ITEMS>; +pub type PublicKeys = SszList, MAX_TRANSACTIONS_PER_PAYLOAD>; + +#[derive(Debug, Clone, PartialEq, Eq, SszEncode, SszDecode)] +pub struct ExecutionWitness { + pub state: WitnessItems, + pub codes: WitnessItems, + pub headers: WitnessItems, +} + +#[derive(Debug, Clone, PartialEq, Eq, SszEncode, SszDecode)] +pub struct PayloadStatusWithWitness { + pub payload_status: PayloadStatus, + pub witness: SszList, + pub public_keys: PublicKeys, +} + +impl ExecutionWitness { + /// Validate the ancestor chain even for cached witnesses. Do not sort a + /// malformed cache entry into apparent validity or omit the parent header. + pub(crate) fn from_rpc( + witness: RpcExecutionWitness, + parent: H256, + ) -> Result { + if !(1..=256).contains(&witness.headers.len()) { + return Err(ProblemJson::internal( + "witness requires 1 to 256 ancestor headers", + )); + } + let mut previous: Option = None; + for bytes in &witness.headers { + let header = BlockHeader::decode(bytes) + .map_err(|e| ProblemJson::internal(&format!("invalid witness header: {e}")))?; + if let Some(prev) = previous + && (header.parent_hash != prev.hash() + || prev.number.checked_add(1) != Some(header.number)) + { + return Err(ProblemJson::internal( + "witness headers are not a contiguous ancestor chain", + )); + } + previous = Some(header); + } + if previous.as_ref().map(BlockHeader::hash) != Some(parent) { + return Err(ProblemJson::internal( + "witness does not end at the payload parent", + )); + } + fn items(values: Vec) -> Result { + if values.len() > MAX_WITNESS_ITEMS { + return Err(ProblemJson::internal( + "witness field exceeds MAX_WITNESS_ITEMS", + )); + } + values + .into_iter() + .map(|bytes| { + bytes.to_vec().try_into().map_err(|_| { + ProblemJson::internal("witness item exceeds MAX_WITNESS_ITEM_BYTES") + }) + }) + .collect::, _>>()? + .try_into() + .map_err(|_| ProblemJson::internal("witness field exceeds MAX_WITNESS_ITEMS")) + } + Ok(Self { + state: items(witness.state)?, + codes: items(witness.codes)?, + headers: items(witness.headers)?, + }) + } +} + +impl PayloadStatusWithWitness { + /// SSZ offsets are uint32 even though the per-field list bounds permit a + /// larger aggregate. Check before the encoder casts offsets to u32. + pub(crate) fn check_encoded_length(&self) -> Result<(), ProblemJson> { + if self.encoded_len() > u32::MAX as usize { + return Err(ProblemJson::internal( + "witness response exceeds SSZ offset range", + )); + } + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::engine_rest::types::common::to_optional; + use ethrex_rlp::encode::RLPEncode; + use libssz::SszDecode; + + #[test] + fn witness_response_matches_independent_wire_bytes() { + let response = PayloadStatusWithWitness { + payload_status: PayloadStatus::new(0, Some([0xaa; 32]), None), + witness: to_optional(Some(ExecutionWitness { + state: vec![vec![0xc0].try_into().unwrap()].try_into().unwrap(), + codes: Default::default(), + headers: vec![vec![0x01, 0x02].try_into().unwrap()] + .try_into() + .unwrap(), + })), + public_keys: vec![ + vec![4; 65].try_into().unwrap(), + vec![5; 65].try_into().unwrap(), + ] + .try_into() + .unwrap(), + }; + // Outer offsets: 12, 12+41, 12+41+4+23. Optional witness has its own offset. + let mut expected = vec![ + 12, 0, 0, 0, 53, 0, 0, 0, 80, 0, 0, 0, 0, 9, 0, 0, 0, 41, 0, 0, 0, + ]; + expected.extend([0xaa; 32]); + expected.extend([ + 4, 0, 0, 0, 12, 0, 0, 0, 17, 0, 0, 0, 17, 0, 0, 0, 4, 0, 0, 0, 0xc0, 4, 0, 0, 0, 1, 2, + ]); + expected.extend([4; 65]); + expected.extend([5; 65]); + assert_eq!(response.to_ssz(), expected); + assert_eq!( + PayloadStatusWithWitness::from_ssz_bytes(&expected).unwrap(), + response + ); + for index in [0, 4, 8, 53] { + let mut malformed = expected.clone(); + malformed[index] = 255; + assert!(PayloadStatusWithWitness::from_ssz_bytes(&malformed).is_err()); + } + expected.pop(); // fixed-size keys cannot be truncated + assert!(PayloadStatusWithWitness::from_ssz_bytes(&expected).is_err()); + } + + #[test] + fn absent_witness_and_keys_have_no_selector_or_padding() { + for status in [1, 2, 3] { + let response = PayloadStatusWithWitness { + payload_status: PayloadStatus::new(status, None, None), + witness: Default::default(), + public_keys: Default::default(), + }; + let expected = vec![ + 12, 0, 0, 0, 21, 0, 0, 0, 21, 0, 0, 0, status, 9, 0, 0, 0, 9, 0, 0, 0, + ]; + assert_eq!(response.to_ssz(), expected); + } + } + + #[test] + fn witness_bounds_reject_oversized_items_and_lists() { + assert!( + SszList::::from_ssz_bytes(&vec![ + 0; + MAX_WITNESS_ITEM_BYTES + 1 + ]) + .is_err() + ); + assert!(WitnessItems::try_from(vec![SszList::default(); MAX_WITNESS_ITEMS + 1]).is_err()); + assert!( + PublicKeys::from_ssz_bytes(&vec![0; 65 * (MAX_TRANSACTIONS_PER_PAYLOAD + 1)]).is_err() + ); + let mut rpc = RpcExecutionWitness::default(); + let parent = BlockHeader::default(); + rpc.headers.push(parent.encode_to_vec().into()); + rpc.codes.push(vec![0; MAX_WITNESS_ITEM_BYTES + 1].into()); + assert!(ExecutionWitness::from_rpc(rpc, parent.hash()).is_err()); + } + + #[test] + fn validates_header_count_order_linkage_and_parent() { + let older = BlockHeader::default(); + let parent = BlockHeader { + number: 1, + parent_hash: older.hash(), + ..Default::default() + }; + let good = vec![older.encode_to_vec().into(), parent.encode_to_vec().into()]; + let make = |headers| RpcExecutionWitness { + headers, + ..Default::default() + }; + assert!(ExecutionWitness::from_rpc(make(good.clone()), parent.hash()).is_ok()); + assert!( + ExecutionWitness::from_rpc(make(vec![parent.encode_to_vec().into()]), parent.hash()) + .is_ok() + ); + for headers in [ + vec![], + vec![older.encode_to_vec().into(); 257], + vec![vec![0xff].into()], + vec![good[1].clone(), good[0].clone()], + vec![good[0].clone(), good[0].clone()], + ] { + assert!(ExecutionWitness::from_rpc(make(headers), parent.hash()).is_err()); + } + assert!(ExecutionWitness::from_rpc(make(good), H256::zero()).is_err()); + } +} diff --git a/test/tests/rpc/engine_rest_tests.rs b/test/tests/rpc/engine_rest_tests.rs index b65da193cf4..4039527ee2d 100644 --- a/test/tests/rpc/engine_rest_tests.rs +++ b/test/tests/rpc/engine_rest_tests.rs @@ -430,7 +430,7 @@ mod capabilities_tests { ); assert_eq!( caps.fork_scoped_endpoints, - vec!["payloads", "forkchoice", "bodies"] + vec!["payloads", "payloads/witness", "forkchoice", "bodies"] ); assert_eq!(caps.unscoped_endpoints, vec!["capabilities", "identity"]); // Flat dot-notation limit keys with scalar values per #793 refactor.md, @@ -543,6 +543,7 @@ mod router_tests { ("GET", "/identity"), ("GET", "/capabilities"), ("POST", "/payloads"), + ("POST", "/payloads/witness"), ("POST", "/blobs/v1"), ] { let app = app.clone(); @@ -2682,6 +2683,7 @@ mod sp3_smoke_tests { ("GET", "/identity"), ("GET", "/capabilities"), ("POST", "/payloads"), + ("POST", "/payloads/witness"), ("GET", "/payloads/0x0102030405060708"), ("POST", "/forkchoice"), ("POST", "/bodies/hash"), From b1fe6f1e852fc59bcbc62b849931d7ccf255da7a Mon Sep 17 00:00:00 2001 From: jsign Date: Fri, 25 Sep 2026 15:50:36 -0300 Subject: [PATCH 2/2] adjust to lastest execution-api changes removing pub_keys field --- crates/common/crypto/provider.rs | 33 ---- crates/common/types/transaction.rs | 139 +---------------- crates/networking/rpc/Cargo.toml | 2 +- .../rpc/engine_rest/handlers/payloads.rs | 36 +---- .../rpc/engine_rest/types/witness.rs | 141 +++++++++++------- 5 files changed, 98 insertions(+), 253 deletions(-) diff --git a/crates/common/crypto/provider.rs b/crates/common/crypto/provider.rs index 8278e6cc456..5eca02782db 100644 --- a/crates/common/crypto/provider.rs +++ b/crates/common/crypto/provider.rs @@ -160,39 +160,6 @@ pub trait Crypto: Send + Sync + core::fmt::Debug { Ok(hash) } - /// Recover the signer's **uncompressed** secp256k1 public key (`0x04 || X || Y`). - /// - /// REST payload witness responses include the full key for each transaction. - /// Unlike [`Self::recover_signer`], this returns the key before hashing it - /// to derive the sender address. Both apply the same EIP-2 low-s rejection. - /// - /// Host-only: requires the native `secp256k1` feature. - #[cfg(feature = "secp256k1")] - fn recover_public_key(&self, sig: &[u8; 65], msg: &[u8; 32]) -> Result<[u8; 65], CryptoError> { - // EIP-2: reject high-s signatures (s > secp256k1n/2), matching recover_signer. - const SECP256K1_N_HALF: [u8; 32] = - hex_literal::hex!("7fffffffffffffffffffffffffffffff5d576e7357a4501ddfe92f46681b20a0"); - if sig[32..64] > SECP256K1_N_HALF[..] { - return Err(CryptoError::InvalidSignature); - } - - let recovery_id = secp256k1::ecdsa::RecoveryId::try_from(sig[64] as i32) - .map_err(|_| CryptoError::InvalidRecoveryId)?; - let recoverable_sig = secp256k1::ecdsa::RecoverableSignature::from_compact( - sig[..64] - .try_into() - .map_err(|_| CryptoError::InvalidSignature)?, - recovery_id, - ) - .map_err(|_| CryptoError::InvalidSignature)?; - - let public_key = recoverable_sig - .recover(&secp256k1::Message::from_digest(*msg)) - .map_err(|_| CryptoError::RecoveryFailed)?; - - Ok(public_key.serialize_uncompressed()) - } - /// Recover the signer address from a 65-byte signature (r||s||v) + 32-byte message hash. /// Used by transaction validation (tx.sender()) and EIP-7702 authority recovery. fn recover_signer(&self, sig: &[u8; 65], msg: &[u8; 32]) -> Result { diff --git a/crates/common/types/transaction.rs b/crates/common/types/transaction.rs index d93ab204b35..adec10daf03 100644 --- a/crates/common/types/transaction.rs +++ b/crates/common/types/transaction.rs @@ -36,9 +36,6 @@ pub use serde_impl::{ GenericTransactionError, }; -/// Signing preimage and compact recoverable signature (r || s || parity). -pub type SigningPayload = (Vec, [u8; 65]); - /// The serialized length of a default eip1559 transaction pub const EIP1559_DEFAULT_SERIALIZED_LENGTH: usize = 15; @@ -1250,11 +1247,7 @@ impl Transaction { .copied() } - /// The bytes that were signed, plus the 65-byte `r || s || v` signature. - /// - /// `Ok(None)` for transactions that carry an explicit sender and no signature - /// (privileged L2 and frame), for which there is nothing to recover. - pub fn signing_payload(&self) -> Result, CryptoError> { + fn compute_sender(&self, crypto: &dyn Crypto) -> Result { let (buf, sig) = match self { Transaction::LegacyTransaction(tx) => { let v = u64::try_from(tx.v).map_err(|_| CryptoError::InvalidSignature)?; @@ -1380,10 +1373,8 @@ impl Transaction { sig[64] = tx.signature_y_parity as u8; (buf, sig) } - // Explicit sender, no signature: nothing to recover from. - Transaction::PrivilegedL2Transaction(_) | Transaction::FrameTransaction(_) => { - return Ok(None); - } + Transaction::PrivilegedL2Transaction(tx) => return Ok(tx.from), + Transaction::FrameTransaction(tx) => return Ok(tx.sender), Transaction::FeeTokenTransaction(tx) => { let mut buf = vec![self.tx_type() as u8]; Encoder::new(&mut buf) @@ -1405,37 +1396,10 @@ impl Transaction { (buf, sig) } }; - Ok(Some((buf, sig))) - } - - fn compute_sender(&self, crypto: &dyn Crypto) -> Result { - match self { - Transaction::PrivilegedL2Transaction(tx) => return Ok(tx.from), - Transaction::FrameTransaction(tx) => return Ok(tx.sender), - _ => {} - } - let Some((buf, sig)) = self.signing_payload()? else { - // Unreachable: the two signature-less variants are handled above. - return Err(CryptoError::InvalidSignature); - }; let msg = crypto.keccak256(&buf); crypto.recover_signer(&sig, &msg) } - /// The signer's uncompressed secp256k1 public key (`0x04 || X || Y`). - /// - /// `Ok(None)` for privileged L2 and frame transactions, which carry an explicit - /// sender and no signature. REST payload witness responses include these - /// keys so stateless consumers can verify transaction signatures. - #[cfg(feature = "secp256k1")] - pub fn public_key(&self, crypto: &dyn Crypto) -> Result, CryptoError> { - let Some((buf, sig)) = self.signing_payload()? else { - return Ok(None); - }; - let msg = crypto.keccak256(&buf); - crypto.recover_public_key(&sig, &msg).map(Some) - } - pub fn gas_limit(&self) -> u64 { match self { Transaction::LegacyTransaction(tx) => tx.gas, @@ -5897,100 +5861,3 @@ mod tests { ); } } - -#[cfg(all(test, feature = "secp256k1"))] -mod public_key_tests { - use super::*; - use ethrex_crypto::NativeCrypto; - use secp256k1::{Message, PublicKey, SECP256K1, SecretKey}; - - #[test] - fn public_keys_match_signers_for_every_l1_signature_format() { - let secret = SecretKey::from_byte_array(&[7; 32]).unwrap(); - let expected = PublicKey::from_secret_key(SECP256K1, &secret).serialize_uncompressed(); - let expected_sender = Address::from_slice(&NativeCrypto.keccak256(&expected[1..])[12..]); - for mut tx in [ - Transaction::LegacyTransaction(LegacyTransaction { - v: 27.into(), - ..Default::default() - }), - Transaction::LegacyTransaction(LegacyTransaction { - v: 37.into(), - ..Default::default() - }), - Transaction::EIP2930Transaction(Default::default()), - Transaction::EIP1559Transaction(Default::default()), - Transaction::EIP4844Transaction(Default::default()), - Transaction::EIP7702Transaction(Default::default()), - ] { - let (preimage, _) = tx.signing_payload().unwrap().unwrap(); - let msg = NativeCrypto.keccak256(&preimage); - let (id, sig) = SECP256K1 - .sign_ecdsa_recoverable(&Message::from_digest(msg), &secret) - .serialize_compact(); - let r = U256::from_big_endian(&sig[..32]); - let s = U256::from_big_endian(&sig[32..]); - let parity = i32::from(id) != 0; - macro_rules! typed { - ($t:expr) => {{ - $t.signature_r = r; - $t.signature_s = s; - $t.signature_y_parity = parity; - }}; - } - match &mut tx { - Transaction::LegacyTransaction(t) => { - t.r = r; - t.s = s; - t.v += U256::from(parity as u8); - } - Transaction::EIP2930Transaction(t) => typed!(t), - Transaction::EIP1559Transaction(t) => typed!(t), - Transaction::EIP4844Transaction(t) => typed!(t), - Transaction::EIP7702Transaction(t) => typed!(t), - _ => unreachable!(), - } - assert_eq!(tx.public_key(&NativeCrypto).unwrap(), Some(expected)); - // Warm both sender caches; public-key recovery must still work. - assert_eq!(tx.sender(&NativeCrypto).unwrap(), expected_sender); - assert_eq!(tx.public_key(&NativeCrypto).unwrap(), Some(expected)); - let mut signature = [0; 65]; - signature[..64].copy_from_slice(&sig); - signature[64] = parity as u8; - signature[64] ^= 1; - assert_ne!( - NativeCrypto.recover_public_key(&signature, &msg).unwrap(), - expected - ); - signature[64] = 4; - assert!(NativeCrypto.recover_public_key(&signature, &msg).is_err()); - signature[64] = parity as u8; - signature[32..64].fill(0xff); - assert!(NativeCrypto.recover_public_key(&signature, &msg).is_err()); - } - for v in [0, 1, 26, 29, 34] { - let tx = Transaction::LegacyTransaction(LegacyTransaction { - v: v.into(), - ..Default::default() - }); - assert!(tx.public_key(&NativeCrypto).is_err()); - } - assert!( - Transaction::EIP1559Transaction(Default::default()) - .public_key(&NativeCrypto) - .is_err() - ); - assert_eq!( - Transaction::PrivilegedL2Transaction(Default::default()) - .public_key(&NativeCrypto) - .unwrap(), - None - ); - assert_eq!( - Transaction::FrameTransaction(Default::default()) - .public_key(&NativeCrypto) - .unwrap(), - None - ); - } -} diff --git a/crates/networking/rpc/Cargo.toml b/crates/networking/rpc/Cargo.toml index 9d6bf2986b5..94a91a2df9b 100644 --- a/crates/networking/rpc/Cargo.toml +++ b/crates/networking/rpc/Cargo.toml @@ -24,7 +24,7 @@ tokio = { workspace = true, features = ["full"] } bytes.workspace = true tracing.workspace = true tracing-subscriber.workspace = true -ethrex-common = { workspace = true, features = ["secp256k1"] } +ethrex-common.workspace = true ethrex-storage.workspace = true ethrex-vm.workspace = true ethrex-blockchain.workspace = true diff --git a/crates/networking/rpc/engine_rest/handlers/payloads.rs b/crates/networking/rpc/engine_rest/handlers/payloads.rs index 7f44610c4a4..07933f5771f 100644 --- a/crates/networking/rpc/engine_rest/handlers/payloads.rs +++ b/crates/networking/rpc/engine_rest/handlers/payloads.rs @@ -32,7 +32,7 @@ use crate::engine_rest::types::common::{ Bytes20, PayloadId, PayloadStatus as SszPayloadStatus, PayloadStatusCode, }; use crate::engine_rest::types::conversions::{DecodedNewPayload, EngineCall, IntoEngineCall}; -use crate::engine_rest::types::witness::{ExecutionWitness, PayloadStatusWithWitness, PublicKeys}; +use crate::engine_rest::types::witness::{ExecutionWitness, PayloadStatusWithWitness}; use crate::engine_rest::types::{amsterdam, cancun, paris, prague, shanghai}; use crate::rpc::RpcApiContext; use crate::types::payload::PayloadValidationStatus; @@ -197,13 +197,6 @@ where // binds the CL and EL to the same transactions, so a mismatch still // surfaces as INVALID. The JSON-RPC path keeps the explicit cross-check // (it still receives the param); only this transport drops it. - // Keep transactions only for the witness response; address caches cannot - // reconstruct public keys. Recovery happens after successful validation. - let transactions = if make_witness { - block.body.transactions.clone() - } else { - Vec::new() - }; let parent_hash = block.header.parent_hash; let result = match call { EngineCall::V1V2 => { @@ -257,39 +250,18 @@ where return SszBody(ssz_status).into_response(); } let response = (|| -> Result { - let (witness, public_keys) = if internal_status.status == PayloadValidationStatus::Valid { + let witness = if internal_status.status == PayloadValidationStatus::Valid { let witness = submission .witness .ok_or_else(|| ProblemJson::internal("valid payload has no witness"))?; let witness = ExecutionWitness::from_rpc(witness, parent_hash)?; - let keys = transactions - .iter() - .map(|tx| { - let key = tx - .public_key(ðrex_crypto::NativeCrypto) - .map_err(|e| { - ProblemJson::internal(&format!("public key recovery failed: {e}")) - })? - .ok_or_else(|| { - ProblemJson::internal( - "valid payload transaction has no sender public key", - ) - })?; - SszVector::::try_from(key.to_vec()) - .map_err(|_| ProblemJson::internal("invalid public key length")) - }) - .collect::, _>>()?; - let keys: PublicKeys = keys - .try_into() - .map_err(|_| ProblemJson::internal("too many transaction public keys"))?; - (to_optional(Some(witness)), keys) + to_optional(Some(witness)) } else { - (to_optional(None), PublicKeys::default()) + to_optional(None) }; let response = PayloadStatusWithWitness { payload_status: ssz_status, witness, - public_keys, }; response.check_encoded_length()?; Ok(response) diff --git a/crates/networking/rpc/engine_rest/types/witness.rs b/crates/networking/rpc/engine_rest/types/witness.rs index 89ef909dd19..c3f10eb3b5c 100644 --- a/crates/networking/rpc/engine_rest/types/witness.rs +++ b/crates/networking/rpc/engine_rest/types/witness.rs @@ -1,4 +1,4 @@ -//! Payload witness response from execution-apis #885 at e473f58911e49cc619fb4102d3973e4e049322f0. +//! Payload witness response from execution-apis #885 at 40924d49a7edecebe4ebc430042d1d6f95a86b8a. //! These bounded REST lists deliberately differ from stateless guest containers. use ethrex_common::H256; @@ -6,28 +6,28 @@ use ethrex_common::types::{BlockHeader, block_execution_witness::RpcExecutionWit use ethrex_rlp::decode::RLPDecode; use libssz::SszEncode; use libssz_derive::{SszDecode, SszEncode}; -use libssz_types::{SszList, SszVector}; +use libssz_types::SszList; -use super::common::{MAX_TRANSACTIONS_PER_PAYLOAD, PayloadStatus}; +use super::common::PayloadStatus; use crate::engine_rest::error::ProblemJson; pub const MAX_WITNESS_ITEMS: usize = 1 << 20; -pub const MAX_WITNESS_ITEM_BYTES: usize = 1 << 20; -pub type WitnessItems = SszList, MAX_WITNESS_ITEMS>; -pub type PublicKeys = SszList, MAX_TRANSACTIONS_PER_PAYLOAD>; +pub const MAX_BYTES_PER_WITNESS_NODE: usize = 1 << 10; +pub const MAX_BYTES_PER_CODE: usize = 1 << 16; +pub const MAX_BYTES_PER_HEADER: usize = 1 << 10; +pub const MAX_WITNESS_HEADERS: usize = 1 << 8; #[derive(Debug, Clone, PartialEq, Eq, SszEncode, SszDecode)] pub struct ExecutionWitness { - pub state: WitnessItems, - pub codes: WitnessItems, - pub headers: WitnessItems, + pub state: SszList, MAX_WITNESS_ITEMS>, + pub codes: SszList, MAX_WITNESS_ITEMS>, + pub headers: SszList, MAX_WITNESS_HEADERS>, } #[derive(Debug, Clone, PartialEq, Eq, SszEncode, SszDecode)] pub struct PayloadStatusWithWitness { pub payload_status: PayloadStatus, pub witness: SszList, - pub public_keys: PublicKeys, } impl ExecutionWitness { @@ -37,7 +37,7 @@ impl ExecutionWitness { witness: RpcExecutionWitness, parent: H256, ) -> Result { - if !(1..=256).contains(&witness.headers.len()) { + if !(1..=MAX_WITNESS_HEADERS).contains(&witness.headers.len()) { return Err(ProblemJson::internal( "witness requires 1 to 256 ancestor headers", )); @@ -61,22 +61,24 @@ impl ExecutionWitness { "witness does not end at the payload parent", )); } - fn items(values: Vec) -> Result { - if values.len() > MAX_WITNESS_ITEMS { + fn items( + values: Vec, + ) -> Result, MAX_ITEMS>, ProblemJson> { + if values.len() > MAX_ITEMS { return Err(ProblemJson::internal( - "witness field exceeds MAX_WITNESS_ITEMS", + "witness field exceeds item count limit", )); } values .into_iter() .map(|bytes| { bytes.to_vec().try_into().map_err(|_| { - ProblemJson::internal("witness item exceeds MAX_WITNESS_ITEM_BYTES") + ProblemJson::internal("witness item exceeds byte length limit") }) }) .collect::, _>>()? .try_into() - .map_err(|_| ProblemJson::internal("witness field exceeds MAX_WITNESS_ITEMS")) + .map_err(|_| ProblemJson::internal("witness field exceeds item count limit")) } Ok(Self { state: items(witness.state)?, @@ -117,70 +119,107 @@ mod tests { .try_into() .unwrap(), })), - public_keys: vec![ - vec![4; 65].try_into().unwrap(), - vec![5; 65].try_into().unwrap(), - ] - .try_into() - .unwrap(), }; - // Outer offsets: 12, 12+41, 12+41+4+23. Optional witness has its own offset. - let mut expected = vec![ - 12, 0, 0, 0, 53, 0, 0, 0, 80, 0, 0, 0, 0, 9, 0, 0, 0, 41, 0, 0, 0, - ]; + // Outer offsets: 8, 8+41. Optional witness has its own offset. + let mut expected = vec![8, 0, 0, 0, 49, 0, 0, 0, 0, 9, 0, 0, 0, 41, 0, 0, 0]; expected.extend([0xaa; 32]); expected.extend([ 4, 0, 0, 0, 12, 0, 0, 0, 17, 0, 0, 0, 17, 0, 0, 0, 4, 0, 0, 0, 0xc0, 4, 0, 0, 0, 1, 2, ]); - expected.extend([4; 65]); - expected.extend([5; 65]); assert_eq!(response.to_ssz(), expected); assert_eq!( PayloadStatusWithWitness::from_ssz_bytes(&expected).unwrap(), response ); - for index in [0, 4, 8, 53] { + for index in [0, 4, 9, 13, 49] { let mut malformed = expected.clone(); malformed[index] = 255; assert!(PayloadStatusWithWitness::from_ssz_bytes(&malformed).is_err()); } - expected.pop(); // fixed-size keys cannot be truncated - assert!(PayloadStatusWithWitness::from_ssz_bytes(&expected).is_err()); } #[test] - fn absent_witness_and_keys_have_no_selector_or_padding() { + fn absent_witness_has_no_selector_or_padding() { for status in [1, 2, 3] { let response = PayloadStatusWithWitness { payload_status: PayloadStatus::new(status, None, None), witness: Default::default(), - public_keys: Default::default(), }; - let expected = vec![ - 12, 0, 0, 0, 21, 0, 0, 0, 21, 0, 0, 0, status, 9, 0, 0, 0, 9, 0, 0, 0, - ]; + let expected = vec![8, 0, 0, 0, 17, 0, 0, 0, status, 9, 0, 0, 0, 9, 0, 0, 0]; assert_eq!(response.to_ssz(), expected); + assert_eq!( + PayloadStatusWithWitness::from_ssz_bytes(&expected).unwrap(), + response + ); } } #[test] fn witness_bounds_reject_oversized_items_and_lists() { - assert!( - SszList::::from_ssz_bytes(&vec![ - 0; - MAX_WITNESS_ITEM_BYTES + 1 - ]) - .is_err() - ); - assert!(WitnessItems::try_from(vec![SszList::default(); MAX_WITNESS_ITEMS + 1]).is_err()); - assert!( - PublicKeys::from_ssz_bytes(&vec![0; 65 * (MAX_TRANSACTIONS_PER_PAYLOAD + 1)]).is_err() - ); - let mut rpc = RpcExecutionWitness::default(); + // Construct the wire bytes independently, including an over-limit field + // that cannot be constructed through SszList's checked API. + let encode = |state: Vec>, codes: Vec>, headers: Vec>| { + let fields = [state.to_ssz(), codes.to_ssz(), headers.to_ssz()]; + let mut bytes = Vec::new(); + let mut offset = 12u32; + for field in &fields { + bytes.extend(offset.to_le_bytes()); + offset += field.len() as u32; + } + for field in fields { + bytes.extend(field); + } + bytes + }; + for extra in [0, 1] { + let state = vec![vec![0; MAX_BYTES_PER_WITNESS_NODE + extra]]; + let codes = vec![vec![0; MAX_BYTES_PER_CODE + extra]]; + let headers = vec![vec![0; MAX_BYTES_PER_HEADER + extra]]; + for bytes in [ + encode(state, vec![], vec![]), + encode(vec![], codes, vec![]), + encode(vec![], vec![], headers), + encode(vec![vec![]; MAX_WITNESS_ITEMS + extra], vec![], vec![]), + encode(vec![], vec![vec![]; MAX_WITNESS_ITEMS + extra], vec![]), + encode(vec![], vec![], vec![vec![]; MAX_WITNESS_HEADERS + extra]), + ] { + assert_eq!(ExecutionWitness::from_ssz_bytes(&bytes).is_ok(), extra == 0); + } + } + } + + #[test] + fn rpc_conversion_enforces_field_byte_limits() { let parent = BlockHeader::default(); - rpc.headers.push(parent.encode_to_vec().into()); - rpc.codes.push(vec![0; MAX_WITNESS_ITEM_BYTES + 1].into()); - assert!(ExecutionWitness::from_rpc(rpc, parent.hash()).is_err()); + for extra in [0, 1] { + for (state, codes) in [ + ( + vec![vec![0; MAX_BYTES_PER_WITNESS_NODE + extra].into()], + vec![], + ), + (vec![], vec![vec![0; MAX_BYTES_PER_CODE + extra].into()]), + ] { + let rpc = RpcExecutionWitness { + state, + codes, + headers: vec![parent.encode_to_vec().into()], + ..Default::default() + }; + assert_eq!( + ExecutionWitness::from_rpc(rpc, parent.hash()).is_ok(), + extra == 0 + ); + } + } + let oversized_parent = BlockHeader { + extra_data: vec![0; MAX_BYTES_PER_HEADER].into(), + ..Default::default() + }; + let rpc = RpcExecutionWitness { + headers: vec![oversized_parent.encode_to_vec().into()], + ..Default::default() + }; + assert!(ExecutionWitness::from_rpc(rpc, oversized_parent.hash()).is_err()); } #[test]