Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check empty EL reqs for OpEngineValidator #13908

Open
emhane opened this issue Jan 21, 2025 · 3 comments
Open

Check empty EL reqs for OpEngineValidator #13908

emhane opened this issue Jan 21, 2025 · 3 comments
Assignees
Labels
A-consensus Related to the consensus engine A-op-reth Related to Optimism and op-reth

Comments

@emhane
Copy link
Member

emhane commented Jan 21, 2025

Describe the feature

Check that the param Requests are always empty in impl of EngineValidator for OpEngineValidator

Blocked by #13849

Additional context

Ref ethereum-optimism/specs#506

@emhane emhane added A-consensus Related to the consensus engine A-op-reth Related to Optimism and op-reth S-blocked This cannot more forward until something else changes labels Jan 21, 2025
@emhane emhane removed the S-blocked This cannot more forward until something else changes label Jan 23, 2025
@emhane
Copy link
Member Author

emhane commented Jan 23, 2025

@meyer9 would you like to impl this? would wrap up eip7685 impl on op-reth end

@emhane
Copy link
Member Author

emhane commented Jan 23, 2025

requires moving the default impl of EngineValidator::validate_execution_requests into an impl on EthereumEngineValidator

impl<Types> EngineValidator<Types> for EthereumEngineValidator
where
Types: EngineTypes<PayloadAttributes = EthPayloadAttributes>,
{
fn validate_version_specific_fields(
&self,
version: EngineApiMessageVersion,
payload_or_attrs: PayloadOrAttributes<'_, EthPayloadAttributes>,
) -> Result<(), EngineObjectValidationError> {
validate_version_specific_fields(self.chain_spec(), version, payload_or_attrs)
}
fn ensure_well_formed_attributes(
&self,
version: EngineApiMessageVersion,
attributes: &EthPayloadAttributes,
) -> Result<(), EngineObjectValidationError> {
validate_version_specific_fields(self.chain_spec(), version, attributes.into())
}
}

and then custom impl on OpEngineValidator checking for empty requests
impl<Types> EngineValidator<Types> for OpEngineValidator
where
Types: EngineTypes<PayloadAttributes = OpPayloadAttributes>,
{
fn validate_version_specific_fields(
&self,
version: EngineApiMessageVersion,
payload_or_attrs: PayloadOrAttributes<'_, OpPayloadAttributes>,
) -> Result<(), EngineObjectValidationError> {
validate_withdrawals_presence(
self.chain_spec(),
version,
payload_or_attrs.message_validation_kind(),
payload_or_attrs.timestamp(),
payload_or_attrs.withdrawals().is_some(),
)?;
validate_parent_beacon_block_root_presence(
self.chain_spec(),
version,
payload_or_attrs.message_validation_kind(),
payload_or_attrs.timestamp(),
payload_or_attrs.parent_beacon_block_root().is_some(),
)
}
fn ensure_well_formed_attributes(
&self,
version: EngineApiMessageVersion,
attributes: &OpPayloadAttributes,
) -> Result<(), EngineObjectValidationError> {
validate_version_specific_fields(self.chain_spec(), version, attributes.into())?;
if attributes.gas_limit.is_none() {
return Err(EngineObjectValidationError::InvalidParams(
"MissingGasLimitInPayloadAttributes".to_string().into(),
))
}
if self
.chain_spec()
.is_holocene_active_at_timestamp(attributes.payload_attributes.timestamp)
{
let (elasticity, denominator) =
attributes.decode_eip_1559_params().ok_or_else(|| {
EngineObjectValidationError::InvalidParams(
"MissingEip1559ParamsInPayloadAttributes".to_string().into(),
)
})?;
if elasticity != 0 && denominator == 0 {
return Err(EngineObjectValidationError::InvalidParams(
"Eip1559ParamsDenominatorZero".to_string().into(),
))
}
}
Ok(())
}
}

@meyer9
Copy link
Contributor

meyer9 commented Jan 23, 2025

yup, I can take this one!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-consensus Related to the consensus engine A-op-reth Related to Optimism and op-reth
Projects
Status: Todo
Development

No branches or pull requests

2 participants