diff --git a/beacon_node/execution_layer/src/engine_api/json_structures.rs b/beacon_node/execution_layer/src/engine_api/json_structures.rs index 4071245166..d3be56f17a 100644 --- a/beacon_node/execution_layer/src/engine_api/json_structures.rs +++ b/beacon_node/execution_layer/src/engine_api/json_structures.rs @@ -487,8 +487,7 @@ pub struct JsonExecutionRequests(pub Vec); impl From> for JsonExecutionRequests { fn from(requests: ExecutionRequests) -> Self { - // Each element is a `RequestType`-prefixed, SSZ-encoded request list (EIP-7685). - // The Gloas variant additionally emits builder deposit/exit requests. + // Each element is a `RequestType`-prefixed, SSZ-encoded request list. let result = requests .get_execution_requests_list() .into_iter() @@ -498,123 +497,124 @@ impl From> for JsonExecutionRequests { } } -impl TryFrom for ExecutionRequests { - type Error = RequestsError; +/// Parse an EIP-7685 `JsonExecutionRequests` list into its component request lists. +/// +/// Returns the deposit, withdrawal, consolidation, builder deposit and builder exit lists. +/// Builder lists are empty pre-gloas or post-gloas when no builder requests are present. +#[allow(clippy::type_complexity)] +fn parse_execution_requests( + value: JsonExecutionRequests, +) -> Result< + ( + DepositRequests, + WithdrawalRequests, + ConsolidationRequests, + BuilderDepositRequests, + BuilderExitRequests, + ), + RequestsError, +> { + let mut deposits = DepositRequests::::default(); + let mut withdrawals = WithdrawalRequests::::default(); + let mut consolidations = ConsolidationRequests::::default(); + let mut builder_deposits = BuilderDepositRequests::::default(); + let mut builder_exits = BuilderExitRequests::::default(); + let mut prev_prefix: Option = None; + for (i, request) in value.0.into_iter().enumerate() { + // hex string + let decoded_bytes = hex::decode(request.strip_prefix("0x").unwrap_or(&request)) + .map_err(RequestsError::InvalidHex)?; - fn try_from(value: JsonExecutionRequests) -> Result { - let mut deposits = DepositRequests::::default(); - let mut withdrawals = WithdrawalRequests::::default(); - let mut consolidations = ConsolidationRequests::::default(); - let mut builder_deposits = BuilderDepositRequests::::default(); - let mut builder_exits = BuilderExitRequests::::default(); - // [New in Gloas:EIP8282] The presence of builder requests determines the variant: the - // EIP-7685 list is fork-agnostic, so we only know it is Gloas-shaped once a builder - // request type appears. - let mut has_builder_requests = false; - let mut prev_prefix: Option = None; - for (i, request) in value.0.into_iter().enumerate() { - // hex string - let decoded_bytes = hex::decode(request.strip_prefix("0x").unwrap_or(&request)) - .map_err(RequestsError::InvalidHex)?; + // The first byte of each element is the `request_type` and the remaining bytes are the `request_data`. + // Elements with empty `request_data` **MUST** be excluded from the list. + let Some((prefix_byte, request_bytes)) = decoded_bytes.split_first() else { + return Err(RequestsError::EmptyRequest(i)); + }; + if request_bytes.is_empty() { + return Err(RequestsError::EmptyRequest(i)); + } + // Elements of the list **MUST** be ordered by `request_type` in ascending order + let current_prefix = + RequestType::from_u8(*prefix_byte).ok_or(RequestsError::InvalidPrefix(*prefix_byte))?; + if let Some(prev) = prev_prefix + && prev.to_u8() >= current_prefix.to_u8() + { + return Err(RequestsError::InvalidOrdering); + } + prev_prefix = Some(current_prefix); - // The first byte of each element is the `request_type` and the remaining bytes are the `request_data`. - // Elements with empty `request_data` **MUST** be excluded from the list. - let Some((prefix_byte, request_bytes)) = decoded_bytes.split_first() else { - return Err(RequestsError::EmptyRequest(i)); - }; - if request_bytes.is_empty() { - return Err(RequestsError::EmptyRequest(i)); + match current_prefix { + RequestType::Deposit => { + deposits = DepositRequests::::from_ssz_bytes(request_bytes).map_err(|e| { + RequestsError::DecodeError(format!( + "Failed to decode DepositRequest from EL: {:?}", + e + )) + })?; } - // Elements of the list **MUST** be ordered by `request_type` in ascending order - let current_prefix = RequestType::from_u8(*prefix_byte) - .ok_or(RequestsError::InvalidPrefix(*prefix_byte))?; - if let Some(prev) = prev_prefix - && prev.to_u8() >= current_prefix.to_u8() - { - return Err(RequestsError::InvalidOrdering); + RequestType::Withdrawal => { + withdrawals = + WithdrawalRequests::::from_ssz_bytes(request_bytes).map_err(|e| { + RequestsError::DecodeError(format!( + "Failed to decode WithdrawalRequest from EL: {:?}", + e + )) + })?; } - prev_prefix = Some(current_prefix); - - match current_prefix { - RequestType::Deposit => { - deposits = - DepositRequests::::from_ssz_bytes(request_bytes).map_err(|e| { - RequestsError::DecodeError(format!( - "Failed to decode DepositRequest from EL: {:?}", - e - )) - })?; - } - RequestType::Withdrawal => { - withdrawals = - WithdrawalRequests::::from_ssz_bytes(request_bytes).map_err(|e| { - RequestsError::DecodeError(format!( - "Failed to decode WithdrawalRequest from EL: {:?}", - e - )) - })?; - } - RequestType::Consolidation => { - consolidations = ConsolidationRequests::::from_ssz_bytes(request_bytes) - .map_err(|e| { - RequestsError::DecodeError(format!( - "Failed to decode ConsolidationRequest from EL: {:?}", - e - )) - })?; - } - RequestType::BuilderDeposit => { - builder_deposits = BuilderDepositRequests::::from_ssz_bytes(request_bytes) - .map_err(|e| { + RequestType::Consolidation => { + consolidations = ConsolidationRequests::::from_ssz_bytes(request_bytes) + .map_err(|e| { + RequestsError::DecodeError(format!( + "Failed to decode ConsolidationRequest from EL: {:?}", + e + )) + })?; + } + RequestType::BuilderDeposit => { + builder_deposits = BuilderDepositRequests::::from_ssz_bytes(request_bytes) + .map_err(|e| { RequestsError::DecodeError(format!( "Failed to decode BuilderDepositRequest from EL: {:?}", e )) })?; - has_builder_requests = true; - } - RequestType::BuilderExit => { - builder_exits = BuilderExitRequests::::from_ssz_bytes(request_bytes) - .map_err(|e| { - RequestsError::DecodeError(format!( - "Failed to decode BuilderExitRequest from EL: {:?}", - e - )) - })?; - has_builder_requests = true; - } + } + RequestType::BuilderExit => { + builder_exits = + BuilderExitRequests::::from_ssz_bytes(request_bytes).map_err(|e| { + RequestsError::DecodeError(format!( + "Failed to decode BuilderExitRequest from EL: {:?}", + e + )) + })?; } } - - // Without any builder requests the list is indistinguishable from a pre-Gloas one, so we - // produce the Electra-shaped variant. Consumers that require the Gloas variant lift it - // (carrying empty builder lists) at their boundary. - if has_builder_requests { - Ok(ExecutionRequests::Gloas(ExecutionRequestsGloas { - deposits, - withdrawals, - consolidations, - builder_deposits, - builder_exits, - })) - } else { - Ok(ExecutionRequests::Electra(ExecutionRequestsElectra { - deposits, - withdrawals, - consolidations, - })) - } } + + Ok(( + deposits, + withdrawals, + consolidations, + builder_deposits, + builder_exits, + )) } impl TryFrom for ExecutionRequestsElectra { type Error = RequestsError; fn try_from(value: JsonExecutionRequests) -> Result { - match ExecutionRequests::::try_from(value)? { - ExecutionRequests::Electra(requests) => Ok(requests), - ExecutionRequests::Gloas(_) => Err(RequestsError::VariantMismatch), + let (deposits, withdrawals, consolidations, builder_deposits, builder_exits) = + parse_execution_requests::(value)?; + // Builder requests are not valid pre-Gloas. + if !builder_deposits.is_empty() || !builder_exits.is_empty() { + return Err(RequestsError::VariantMismatch); } + Ok(ExecutionRequestsElectra { + deposits, + withdrawals, + consolidations, + }) } } @@ -622,10 +622,15 @@ impl TryFrom for ExecutionRequestsGloas { type Error = RequestsError; fn try_from(value: JsonExecutionRequests) -> Result { - match ExecutionRequests::::try_from(value)? { - ExecutionRequests::Gloas(requests) => Ok(requests), - ExecutionRequests::Electra(_) => Err(RequestsError::VariantMismatch), - } + let (deposits, withdrawals, consolidations, builder_deposits, builder_exits) = + parse_execution_requests::(value)?; + Ok(ExecutionRequestsGloas { + deposits, + withdrawals, + consolidations, + builder_deposits, + builder_exits, + }) } } @@ -1224,7 +1229,8 @@ mod tests { use bls::{PublicKeyBytes, SignatureBytes}; use ssz::Encode; use types::{ - ConsolidationRequest, DepositRequest, MainnetEthSpec, RequestType, WithdrawalRequest, + BuilderDepositRequest, BuilderExitRequest, ConsolidationRequest, DepositRequest, + MainnetEthSpec, RequestType, WithdrawalRequest, }; use super::*; @@ -1269,7 +1275,7 @@ mod tests { // First check a valid request with all requests assert!( - ExecutionRequests::::try_from(JsonExecutionRequests(vec![ + ExecutionRequestsElectra::::try_from(JsonExecutionRequests(vec![ create_request_string(RequestType::Deposit.to_u8(), &deposit_request), create_request_string(RequestType::Withdrawal.to_u8(), &withdrawal_request), create_request_string(RequestType::Consolidation.to_u8(), &consolidation_request), @@ -1279,21 +1285,21 @@ mod tests { // Single requests assert!( - ExecutionRequests::::try_from(JsonExecutionRequests(vec![ + ExecutionRequestsElectra::::try_from(JsonExecutionRequests(vec![ create_request_string(RequestType::Deposit.to_u8(), &deposit_request), ])) .is_ok() ); assert!( - ExecutionRequests::::try_from(JsonExecutionRequests(vec![ + ExecutionRequestsElectra::::try_from(JsonExecutionRequests(vec![ create_request_string(RequestType::Withdrawal.to_u8(), &withdrawal_request), ])) .is_ok() ); assert!( - ExecutionRequests::::try_from(JsonExecutionRequests(vec![ + ExecutionRequestsElectra::::try_from(JsonExecutionRequests(vec![ create_request_string(RequestType::Consolidation.to_u8(), &consolidation_request), ])) .is_ok() @@ -1301,7 +1307,7 @@ mod tests { // Out of order assert!(matches!( - ExecutionRequests::::try_from(JsonExecutionRequests(vec![ + ExecutionRequestsElectra::::try_from(JsonExecutionRequests(vec![ create_request_string(RequestType::Withdrawal.to_u8(), &withdrawal_request), create_request_string(RequestType::Deposit.to_u8(), &deposit_request), ])) @@ -1310,7 +1316,7 @@ mod tests { )); assert!(matches!( - ExecutionRequests::::try_from(JsonExecutionRequests(vec![ + ExecutionRequestsElectra::::try_from(JsonExecutionRequests(vec![ create_request_string(RequestType::Consolidation.to_u8(), &consolidation_request), create_request_string(RequestType::Withdrawal.to_u8(), &withdrawal_request), ])) @@ -1319,7 +1325,7 @@ mod tests { )); assert!(matches!( - ExecutionRequests::::try_from(JsonExecutionRequests(vec![ + ExecutionRequestsElectra::::try_from(JsonExecutionRequests(vec![ create_request_string(RequestType::Consolidation.to_u8(), &consolidation_request), create_request_string(RequestType::Deposit.to_u8(), &deposit_request), ])) @@ -1329,7 +1335,7 @@ mod tests { // Multiple requests of same type assert!(matches!( - ExecutionRequests::::try_from(JsonExecutionRequests(vec![ + ExecutionRequestsElectra::::try_from(JsonExecutionRequests(vec![ create_request_string(RequestType::Deposit.to_u8(), &deposit_request), create_request_string(RequestType::Deposit.to_u8(), &deposit_request), ])) @@ -1339,7 +1345,7 @@ mod tests { // Invalid prefix assert!(matches!( - ExecutionRequests::::try_from(JsonExecutionRequests(vec![ + ExecutionRequestsElectra::::try_from(JsonExecutionRequests(vec![ create_request_string(42, &deposit_request), ])) .unwrap_err(), @@ -1348,7 +1354,7 @@ mod tests { // Prefix followed by no data assert!(matches!( - ExecutionRequests::::try_from(JsonExecutionRequests(vec![ + ExecutionRequestsElectra::::try_from(JsonExecutionRequests(vec![ create_request_string(RequestType::Deposit.to_u8(), &deposit_request), create_request_string( RequestType::Consolidation.to_u8(), @@ -1360,12 +1366,141 @@ mod tests { )); // Empty request assert!(matches!( - ExecutionRequests::::try_from(JsonExecutionRequests(vec![ + ExecutionRequestsElectra::::try_from(JsonExecutionRequests(vec![ create_request_string(RequestType::Deposit.to_u8(), &deposit_request), "0x".to_string() ])) .unwrap_err(), RequestsError::EmptyRequest(1) )); + + // Builder requests are not valid pre-gloas. + let builder_deposit_request = BuilderDepositRequest { + pubkey: PublicKeyBytes::empty(), + withdrawal_credentials: Hash256::random(), + amount: 32, + signature: SignatureBytes::empty(), + }; + assert!(matches!( + ExecutionRequestsElectra::::try_from(JsonExecutionRequests(vec![ + create_request_string( + RequestType::BuilderDeposit.to_u8(), + &builder_deposit_request + ), + ])) + .unwrap_err(), + RequestsError::VariantMismatch + )); + } + + #[test] + fn test_gloas_execution_requests() { + let deposit_request = DepositRequest { + pubkey: PublicKeyBytes::empty(), + withdrawal_credentials: Hash256::random(), + amount: 32, + signature: SignatureBytes::empty(), + index: 0, + }; + + let withdrawal_request = WithdrawalRequest { + amount: 32, + source_address: Address::random(), + validator_pubkey: PublicKeyBytes::empty(), + }; + + let consolidation_request = ConsolidationRequest { + source_address: Address::random(), + source_pubkey: PublicKeyBytes::empty(), + target_pubkey: PublicKeyBytes::empty(), + }; + + let builder_deposit_request = BuilderDepositRequest { + pubkey: PublicKeyBytes::empty(), + withdrawal_credentials: Hash256::random(), + amount: 32, + signature: SignatureBytes::empty(), + }; + + let builder_exit_request = BuilderExitRequest { + source_address: Address::random(), + pubkey: PublicKeyBytes::empty(), + }; + + // Valid request with all five request types, in ascending prefix order. + assert!( + ExecutionRequestsGloas::::try_from(JsonExecutionRequests(vec![ + create_request_string(RequestType::Deposit.to_u8(), &deposit_request), + create_request_string(RequestType::Withdrawal.to_u8(), &withdrawal_request), + create_request_string(RequestType::Consolidation.to_u8(), &consolidation_request), + create_request_string( + RequestType::BuilderDeposit.to_u8(), + &builder_deposit_request + ), + create_request_string(RequestType::BuilderExit.to_u8(), &builder_exit_request), + ])) + .is_ok() + ); + + // A builder-less list is a valid Gloas value (builder lists are simply empty). + assert!( + ExecutionRequestsGloas::::try_from(JsonExecutionRequests(vec![ + create_request_string(RequestType::Deposit.to_u8(), &deposit_request), + ])) + .is_ok() + ); + + // Only builder requests. + assert!( + ExecutionRequestsGloas::::try_from(JsonExecutionRequests(vec![ + create_request_string( + RequestType::BuilderDeposit.to_u8(), + &builder_deposit_request + ), + create_request_string(RequestType::BuilderExit.to_u8(), &builder_exit_request), + ])) + .is_ok() + ); + + // Out of order: builder exit must come after a builder deposit. + assert!(matches!( + ExecutionRequestsGloas::::try_from(JsonExecutionRequests(vec![ + create_request_string(RequestType::BuilderExit.to_u8(), &builder_exit_request), + create_request_string( + RequestType::BuilderDeposit.to_u8(), + &builder_deposit_request + ), + ])) + .unwrap_err(), + RequestsError::InvalidOrdering + )); + + // Duplicate builder request type. + assert!(matches!( + ExecutionRequestsGloas::::try_from(JsonExecutionRequests(vec![ + create_request_string( + RequestType::BuilderDeposit.to_u8(), + &builder_deposit_request + ), + create_request_string( + RequestType::BuilderDeposit.to_u8(), + &builder_deposit_request + ), + ])) + .unwrap_err(), + RequestsError::InvalidOrdering + )); + + // Empty builder request data. + assert!(matches!( + ExecutionRequestsGloas::::try_from(JsonExecutionRequests(vec![ + create_request_string( + RequestType::BuilderDeposit.to_u8(), + &Vec::::new() + ), + ])) + .unwrap_err(), + RequestsError::EmptyRequest(0) + )); } }