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

feat(engine): validate execution requests #13685

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions crates/payload/primitives/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -31,5 +31,8 @@ serde.workspace = true
thiserror.workspace = true
tokio = { workspace = true, default-features = false, features = ["sync"] }

[dev-dependencies]
assert_matches.workspace = true

[features]
op = ["dep:op-alloy-rpc-types-engine"]
77 changes: 76 additions & 1 deletion crates/payload/primitives/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@
#![cfg_attr(not(test), warn(unused_crate_dependencies))]
#![cfg_attr(docsrs, feature(doc_cfg, doc_auto_cfg))]

use alloy_primitives::Bytes;
use reth_chainspec::EthereumHardforks;

mod error;
pub use error::{
EngineObjectValidationError, InvalidPayloadAttributesError, PayloadBuilderError,
Expand All @@ -24,7 +27,6 @@ pub use traits::{
mod payload;
pub use payload::PayloadOrAttributes;

use reth_chainspec::EthereumHardforks;
/// The types that are used by the engine API.
pub trait PayloadTypes: Send + Sync + Unpin + core::fmt::Debug + Clone + 'static {
/// The built payload type.
Expand Down Expand Up @@ -363,12 +365,85 @@ pub enum PayloadKind {
WaitForPending,
}

/// Validates that execution requests are valid according to Engine API specification.
///
/// `executionRequests`: `Array of DATA` - List of execution layer triggered requests. Each list
/// element is a `requests` byte array as defined by [EIP-7685](https://eips.ethereum.org/EIPS/eip-7685).
/// The first byte of each element is the `request_type` and the remaining bytes are the
/// `request_data`. Elements of the list **MUST** be ordered by `request_type` in ascending order.
/// Elements with empty `request_data` **MUST** be excluded from the list. If any element is out of
/// order or has a length of 1-byte or shorter, client software **MUST** return `-32602: Invalid
/// params` error.
pub fn validate_execution_requests(requests: &[Bytes]) -> Result<(), EngineObjectValidationError> {
let mut last_request_type = None;
for request in requests {
if request.len() <= 1 {
return Err(EngineObjectValidationError::InvalidParams(
"empty execution request".to_string().into(),
))
}

let request_type = request[0];
if Some(request_type) < last_request_type {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this allows multiple entries, spec isn't clear on this. so this seems okay

return Err(EngineObjectValidationError::InvalidParams(
"execution requests out of order".to_string().into(),
))
}

last_request_type = Some(request_type);
}
Ok(())
}

#[cfg(test)]
mod tests {
use super::*;
use assert_matches::assert_matches;

#[test]
fn version_ord() {
assert!(EngineApiMessageVersion::V4 > EngineApiMessageVersion::V3);
}

#[test]
fn execution_requests_validation() {
assert_matches!(validate_execution_requests(&[]), Ok(()));

let valid_requests = [
Bytes::from_iter([1, 2]),
Bytes::from_iter([2, 3]),
Bytes::from_iter([3, 4]),
Bytes::from_iter([4, 5]),
];
assert_matches!(validate_execution_requests(&valid_requests), Ok(()));

let requests_with_empty = [
Bytes::from_iter([1, 2]),
Bytes::from_iter([2, 3]),
Bytes::new(),
Bytes::from_iter([3, 4]),
];
assert_matches!(
validate_execution_requests(&requests_with_empty),
Err(EngineObjectValidationError::InvalidParams(_))
);

let mut requests_valid_reversed = valid_requests;
requests_valid_reversed.reverse();
assert_matches!(
validate_execution_requests(&requests_with_empty),
Err(EngineObjectValidationError::InvalidParams(_))
);

let requests_out_of_order = [
Bytes::from_iter([1, 2]),
Bytes::from_iter([2, 3]),
Bytes::from_iter([4, 5]),
Bytes::from_iter([3, 4]),
];
assert_matches!(
validate_execution_requests(&requests_out_of_order),
Err(EngineObjectValidationError::InvalidParams(_))
);
}
}
6 changes: 4 additions & 2 deletions crates/rpc/rpc-engine-api/src/engine_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ use reth_chainspec::{EthereumHardforks, Hardforks};
use reth_engine_primitives::{EngineTypes, EngineValidator};
use reth_payload_builder::PayloadStore;
use reth_payload_primitives::{
validate_payload_timestamp, EngineApiMessageVersion, PayloadBuilderAttributes,
PayloadOrAttributes,
validate_execution_requests, validate_payload_timestamp, EngineApiMessageVersion,
PayloadBuilderAttributes, PayloadOrAttributes,
};
use reth_primitives::EthereumHardfork;
use reth_rpc_api::EngineApiServer;
Expand Down Expand Up @@ -270,6 +270,8 @@ where
.validator
.validate_version_specific_fields(EngineApiMessageVersion::V4, payload_or_attrs)?;

validate_execution_requests(&execution_requests)?;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems okay here because these rules are for the request object specifically, unclear how this will look like for OP V4


Ok(self
.inner
.beacon_consensus
Expand Down
Loading