Make PayloadOrAttributes generic over ExecutionData #14666
Conversation
yohkaz
requested review from
mattsse,
Rjected,
fgimenez,
rkrasiuk and
gakonst
as code owners
| @@ -25,6 +23,9 @@ use reth_primitives::{NodePrimitives, RecoveredBlock, SealedBlock}; | |||
| use reth_primitives_traits::Block; | |||
| use serde::{de::DeserializeOwned, Serialize}; | |||
|
|
|||
| // Re-export [`ExecutionPayload`] moved to `reth_payload_primitives` | |||
There was a problem hiding this comment.
The ExecutionPayload trait was moved to the reth_payload_primitives crate.
Importing the trait from the reth_engine_primitives crate to the reth_payload_primitives crate creates a cyclic dependency.
I'm re-exporting it here, to avoid the potential breaking change.
There was a problem hiding this comment.
good thinking, reth-engine-primitives has reth-payload-primitives as dep, so makes more sense to move it into reth-payload-primitives for now
| @@ -25,6 +23,9 @@ use reth_primitives::{NodePrimitives, RecoveredBlock, SealedBlock}; | |||
| use reth_primitives_traits::Block; | |||
| use serde::{de::DeserializeOwned, Serialize}; | |||
|
|
|||
| // Re-export [`ExecutionPayload`] moved to `reth_payload_primitives` | |||
There was a problem hiding this comment.
good thinking, reth-engine-primitives has reth-payload-primitives as dep, so makes more sense to move it into reth-payload-primitives for now
| @@ -302,10 +302,10 @@ impl MessageValidationKind { | |||
| /// either an execution payload, or payload attributes. | |||
| /// | |||
| /// The version is provided by the [`EngineApiMessageVersion`] argument. | |||
| pub fn validate_version_specific_fields<Type, T>( | |||
| pub fn validate_version_specific_fields<Payload: ExecutionPayload, Type, T>( | |||
There was a problem hiding this comment.
better move this trait bound down into where clause on this method
| /// An execution payload. | ||
| pub trait ExecutionPayload: | ||
| Serialize + DeserializeOwned + Debug + Clone + Send + Sync + 'static | ||
| { | ||
| /// Returns the parent hash of the block. | ||
| fn parent_hash(&self) -> B256; | ||
|
|
||
| /// Returns the hash of the block. | ||
| fn block_hash(&self) -> B256; | ||
|
|
||
| /// Returns the number of the block. | ||
| fn block_number(&self) -> u64; | ||
|
|
||
| /// Returns the withdrawals for the payload, if it exists. | ||
| fn withdrawals(&self) -> Option<&Vec<Withdrawal>>; | ||
|
|
||
| /// Return the parent beacon block root for the payload, if it exists. | ||
| fn parent_beacon_block_root(&self) -> Option<B256>; | ||
|
|
||
| /// Returns the timestamp to be used in the payload. | ||
| fn timestamp(&self) -> u64; | ||
| } |
There was a problem hiding this comment.
needs implementation for OpExecutionPayload too, think @mattsse wants an op feature to be added for this reason for now to feature gate the impl of this trait under the impl for ExecutionData
| /// The parent beacon block root | ||
| parent_beacon_block_root: Option<B256>, | ||
| }, | ||
| pub enum PayloadOrAttributes<'a, Payload: ExecutionPayload, Attributes> { |
There was a problem hiding this comment.
| pub enum PayloadOrAttributes<'a, Payload: ExecutionPayload, Attributes> { | |
| pub enum PayloadOrAttributes<'a, Payload, Attributes> { |
no need to add this trait bound on the type definition since none of the fields is an associated type of this trait. for example if you had some field as Payload::SomeAssociatedType then you would need the trait bound here.
Closes #14651