fix: ensure extradata is 32 bytes or less (#2775)

bin/reth/src/args/network_args.rs

@@ -1,6 +1,6 @@
 //! clap [Args](clap::Args) for network related arguments.
 
-use crate::version::P2P_VERSION;
+use crate::version::P2P_CLIENT_VERSION;
 use clap::Args;
 use reth_net_nat::NatResolver;
 use reth_network::{HelloMessage, NetworkConfigBuilder};
@@ -38,7 +38,7 @@ pub struct NetworkArgs {
     pub peers_file: Option<PathBuf>,
 
     /// Custom node identity
-    #[arg(long, value_name = "IDENTITY", default_value = P2P_VERSION)]
+    #[arg(long, value_name = "IDENTITY", default_value = P2P_CLIENT_VERSION)]
     pub identity: String,
 
     /// Secret key to use for this node.

bin/reth/src/args/payload_builder_args.rs

@@ -1,12 +1,16 @@
-use crate::{utils::parse_duration_from_secs, version::P2P_VERSION};
-use clap::{builder::RangedU64ValueParser, Args};
-use std::time::Duration;
+use crate::{utils::parse_duration_from_secs, version::default_extradata};
+use clap::{
+    builder::{RangedU64ValueParser, TypedValueParser},
+    Arg, Args, Command,
+};
+use reth_primitives::constants::MAXIMUM_EXTRA_DATA_SIZE;
+use std::{ffi::OsStr, time::Duration};
 
 /// Parameters for configuring the Payload Builder
 #[derive(Debug, Args, PartialEq, Default)]
 pub struct PayloadBuilderArgs {
-    /// Extra block data set by the builder.
-    #[arg(long = "builder.extradata", help_heading = "Builder", default_value = P2P_VERSION)]
+    /// Block extra data set by the payload builder.
+    #[arg(long = "builder.extradata", help_heading = "Builder", value_parser=ExtradataValueParser::default(),  default_value_t = default_extradata())]
     pub extradata: String,
 
     /// Target gas ceiling for built blocks.
@@ -31,9 +35,35 @@ pub struct PayloadBuilderArgs {
     pub max_payload_tasks: usize,
 }
 
+#[derive(Clone, Debug, Default)]
+#[non_exhaustive]
+struct ExtradataValueParser;
+
+impl TypedValueParser for ExtradataValueParser {
+    type Value = String;
+
+    fn parse_ref(
+        &self,
+        _cmd: &Command,
+        _arg: Option<&Arg>,
+        value: &OsStr,
+    ) -> Result<Self::Value, clap::Error> {
+        let val =
+            value.to_str().ok_or_else(|| clap::Error::new(clap::error::ErrorKind::InvalidUtf8))?;
+        if val.as_bytes().len() > MAXIMUM_EXTRA_DATA_SIZE {
+            return Err(clap::Error::raw(
+                clap::error::ErrorKind::InvalidValue,
+                format!(
+                    "Payload builder extradata size exceeds {MAXIMUM_EXTRA_DATA_SIZE}bytes limit"
+                ),
+            ))
+        }
+        Ok(val.to_string())
+    }
+}
+
 #[cfg(test)]
 mod tests {
-
     use super::*;
     use clap::{Args, Parser};
 
@@ -61,4 +91,27 @@ mod tests {
         ])
         .is_err());
     }
+
+    #[test]
+    fn test_default_extradata() {
+        let extradata = default_extradata();
+        let args = CommandParser::<PayloadBuilderArgs>::parse_from([
+            "reth",
+            "--builder.extradata",
+            extradata.as_str(),
+        ])
+        .args;
+        assert_eq!(args.extradata, extradata);
+    }
+
+    #[test]
+    fn test_invalid_extradata() {
+        let extradata = "x".repeat(MAXIMUM_EXTRA_DATA_SIZE + 1);
+        let args = CommandParser::<PayloadBuilderArgs>::try_parse_from([
+            "reth",
+            "--builder.extradata",
+            extradata.as_str(),
+        ]);
+        assert!(args.is_err());
+    }
 }

bin/reth/src/version.rs

@@ -52,6 +52,33 @@ pub(crate) const LONG_VERSION: &str = concat!(
 /// ```text
 /// reth/v{major}.{minor}.{patch}/{target}
 /// ```
-#[allow(dead_code)]
-pub(crate) const P2P_VERSION: &str =
+pub(crate) const P2P_CLIENT_VERSION: &str =
     concat!("reth/v", env!("CARGO_PKG_VERSION"), "/", env!("VERGEN_CARGO_TARGET_TRIPLE"));
+
+/// The default extradata used for payload building.
+///
+/// - The latest version from Cargo.toml
+/// - The OS identifier
+///
+/// # Example
+///
+/// ```text
+/// reth/v{major}.{minor}.{patch}/{OS}
+/// ```
+pub fn default_extradata() -> String {
+    format!("reth/v{}/{}", env!("CARGO_PKG_VERSION"), std::env::consts::OS)
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn assert_extradata_less_32bytes() {
+        let extradata = default_extradata();
+        assert!(
+            extradata.as_bytes().len() <= 32,
+            "extradata must be less than 32 bytes: {extradata}"
+        )
+    }
+}

crates/consensus/beacon/src/beacon_consensus.rs

@@ -2,7 +2,8 @@
 use reth_consensus_common::validation;
 use reth_interfaces::consensus::{Consensus, ConsensusError};
 use reth_primitives::{
-    Chain, ChainSpec, Hardfork, Header, SealedBlock, SealedHeader, EMPTY_OMMER_ROOT, U256,
+    constants::MAXIMUM_EXTRA_DATA_SIZE, Chain, ChainSpec, Hardfork, Header, SealedBlock,
+    SealedHeader, EMPTY_OMMER_ROOT, U256,
 };
 use std::sync::Arc;
 
@@ -89,7 +90,7 @@ impl Consensus for BeaconConsensus {
 /// From yellow paper: extraData: An arbitrary byte array containing data relevant to this block.
 /// This must be 32 bytes or fewer; formally Hx.
 fn validate_header_extradata(header: &Header) -> Result<(), ConsensusError> {
-    if header.extra_data.len() > 32 {
+    if header.extra_data.len() > MAXIMUM_EXTRA_DATA_SIZE {
         Err(ConsensusError::ExtraDataExceedsMax { len: header.extra_data.len() })
     } else {
         Ok(())

crates/primitives/src/constants.rs

@@ -10,6 +10,9 @@ pub const RETH_CLIENT_VERSION: &str = concat!("reth/v", env!("CARGO_PKG_VERSION"
 /// The first four bytes of the call data for a function call specifies the function to be called.
 pub const SELECTOR_LEN: usize = 4;
 
+/// Maximum extra data size in a block after genesis
+pub const MAXIMUM_EXTRA_DATA_SIZE: usize = 32;
+
 /// The duration of a slot in seconds.
 ///
 /// This is the time period of 12 seconds in which a randomly chosen validator has time to propose a

crates/rpc/rpc-types/src/eth/engine/payload.rs

@@ -1,5 +1,5 @@
 use reth_primitives::{
-    constants::MIN_PROTOCOL_BASE_FEE_U256,
+    constants::{MAXIMUM_EXTRA_DATA_SIZE, MIN_PROTOCOL_BASE_FEE_U256},
     proofs::{self, EMPTY_LIST_HASH},
     Address, Block, Bloom, Bytes, Header, SealedBlock, TransactionSigned, UintTryTo, Withdrawal,
     H256, H64, U256, U64,
@@ -126,7 +126,7 @@ impl TryFrom<ExecutionPayload> for SealedBlock {
     type Error = PayloadError;
 
     fn try_from(payload: ExecutionPayload) -> Result<Self, Self::Error> {
-        if payload.extra_data.len() > 32 {
+        if payload.extra_data.len() > MAXIMUM_EXTRA_DATA_SIZE {
             return Err(PayloadError::ExtraData(payload.extra_data))
         }