From 1dd91a4bc49124a81695c421085db43949608af5 Mon Sep 17 00:00:00 2001 From: -f Date: Wed, 18 Sep 2024 02:59:48 +0530 Subject: [PATCH 01/10] add PayloadBuilderStack --- crates/payload/basic/src/builder_stack.rs | 69 +++++++++++++++++++++++ crates/payload/basic/src/lib.rs | 20 ++++--- crates/payload/builder/src/error.rs | 3 + 3 files changed, 84 insertions(+), 8 deletions(-) create mode 100644 crates/payload/basic/src/builder_stack.rs diff --git a/crates/payload/basic/src/builder_stack.rs b/crates/payload/basic/src/builder_stack.rs new file mode 100644 index 000000000000..29287bc7f93c --- /dev/null +++ b/crates/payload/basic/src/builder_stack.rs @@ -0,0 +1,69 @@ +use crate::{ + BuildArguments, BuildOutcome, MissingPayloadBehaviour, PayloadBuilder, PayloadBuilderError, + PayloadConfig, +}; + +/// A stack of payload builders that can be used in sequence. +#[derive(Clone, Debug)] +pub struct PayloadBuilderStack(Vec); + +impl PayloadBuilderStack { + /// Create a new payload builder stack. + pub fn new(builders: Vec) -> Self { + Self(builders) + } + + /// Push a builder onto the stack. + pub fn push(&mut self, builder: B) { + self.0.push(builder); + } +} + +impl PayloadBuilder for PayloadBuilderStack +where + B: PayloadBuilder, +{ + type Attributes = B::Attributes; + type BuiltPayload = B::BuiltPayload; + + fn try_build( + &self, + args: BuildArguments, + ) -> Result, PayloadBuilderError> { + let mut args = Some(args); + for builder in &self.0 { + match builder.try_build(args.take().unwrap()) { + Ok(outcome @ BuildOutcome::Better { .. }) => return Ok(outcome), + Ok(_) => continue, + Err(_) => continue, + } + } + Err(PayloadBuilderError::NoBetterPayload) + } + + fn on_missing_payload( + &self, + args: BuildArguments, + ) -> MissingPayloadBehaviour { + let mut args = Some(args); + self.0 + .iter() + .find_map(|builder| match builder.on_missing_payload(args.take().unwrap()) { + MissingPayloadBehaviour::AwaitInProgress => None, + behaviour => Some(behaviour), + }) + .unwrap_or(MissingPayloadBehaviour::AwaitInProgress) + } + + fn build_empty_payload( + &self, + client: &Client, + config: PayloadConfig, + ) -> Result { + let mut config = Some(config); + self.0 + .iter() + .find_map(|builder| builder.build_empty_payload(client, config.take().unwrap()).ok()) + .ok_or(PayloadBuilderError::MissingPayload) + } +} diff --git a/crates/payload/basic/src/lib.rs b/crates/payload/basic/src/lib.rs index b565e37fe39d..0c42c027c790 100644 --- a/crates/payload/basic/src/lib.rs +++ b/crates/payload/basic/src/lib.rs @@ -43,8 +43,12 @@ use tokio::{ }; use tracing::{debug, trace, warn}; +/// The [`PayloadBuilder`] that builds payloads using a stack of builders. +pub mod builder_stack; mod metrics; +pub use builder_stack::PayloadBuilderStack; + /// The [`PayloadJobGenerator`] that creates [`BasicPayloadJob`]s. #[derive(Debug)] pub struct BasicPayloadJobGenerator { @@ -407,7 +411,7 @@ where // check if the deadline is reached if this.deadline.as_mut().poll(cx).is_ready() { trace!(target: "payload_builder", "payload building deadline reached"); - return Poll::Ready(Ok(())) + return Poll::Ready(Ok(())); } // check if the interval is reached @@ -465,7 +469,7 @@ where fn best_payload(&self) -> Result { if let Some(ref payload) = self.best_payload { - return Ok(payload.clone()) + return Ok(payload.clone()); } // No payload has been built yet, but we need to return something that the CL then can // deliver, so we need to return an empty payload. @@ -583,14 +587,14 @@ where this.maybe_better = None; if let Ok(BuildOutcome::Better { payload, .. }) = res { debug!(target: "payload_builder", "resolving better payload"); - return Poll::Ready(Ok(payload)) + return Poll::Ready(Ok(payload)); } } } if let Some(best) = this.best_payload.take() { debug!(target: "payload_builder", "resolving best payload"); - return Poll::Ready(Ok(best)) + return Poll::Ready(Ok(best)); } if let Some(fut) = Pin::new(&mut this.empty_payload).as_pin_mut() { @@ -606,12 +610,12 @@ where Poll::Ready(res) } Err(err) => Poll::Ready(Err(err.into())), - } + }; } } if this.is_empty() { - return Poll::Ready(Err(PayloadBuilderError::MissingPayload)) + return Poll::Ready(Err(PayloadBuilderError::MissingPayload)); } Poll::Pending @@ -918,11 +922,11 @@ pub fn commit_withdrawals>( withdrawals: Withdrawals, ) -> Result { if !chain_spec.is_shanghai_active_at_timestamp(timestamp) { - return Ok(WithdrawalsOutcome::pre_shanghai()) + return Ok(WithdrawalsOutcome::pre_shanghai()); } if withdrawals.is_empty() { - return Ok(WithdrawalsOutcome::empty()) + return Ok(WithdrawalsOutcome::empty()); } let balance_increments = diff --git a/crates/payload/builder/src/error.rs b/crates/payload/builder/src/error.rs index 8c430000f7c3..b9f5fae54e64 100644 --- a/crates/payload/builder/src/error.rs +++ b/crates/payload/builder/src/error.rs @@ -29,6 +29,9 @@ pub enum PayloadBuilderError { /// Thrown if the payload requests withdrawals before Shanghai activation. #[error("withdrawals set before Shanghai activation")] WithdrawalsBeforeShanghai, + /// Thrown when no better payload could be built. + #[error("no better payload could be built")] + NoBetterPayload, /// Any other payload building errors. #[error(transparent)] Other(Box), From eba3e5659f2d47df5b6546d8c4a999530920a8f5 Mon Sep 17 00:00:00 2001 From: -f Date: Thu, 19 Sep 2024 02:28:24 +0530 Subject: [PATCH 02/10] add to BasicPayloadJobGenerator --- crates/payload/basic/src/builder_stack.rs | 9 +++++++++ crates/payload/basic/src/lib.rs | 18 +++++++++--------- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/crates/payload/basic/src/builder_stack.rs b/crates/payload/basic/src/builder_stack.rs index 29287bc7f93c..682317820242 100644 --- a/crates/payload/basic/src/builder_stack.rs +++ b/crates/payload/basic/src/builder_stack.rs @@ -31,13 +31,18 @@ where args: BuildArguments, ) -> Result, PayloadBuilderError> { let mut args = Some(args); + // Iterate through all builders in the stack for builder in &self.0 { match builder.try_build(args.take().unwrap()) { + // If a builder produces a Better outcome, return it immediately Ok(outcome @ BuildOutcome::Better { .. }) => return Ok(outcome), + // If a builder produces a Valid outcome, continue with the next builder Ok(_) => continue, + // If a builder produces an Invalid outcome, continue with the next builder Err(_) => continue, } } + // If no builder produced a Better outcome, return an error Err(PayloadBuilderError::NoBetterPayload) } @@ -46,12 +51,14 @@ where args: BuildArguments, ) -> MissingPayloadBehaviour { let mut args = Some(args); + // Find the first builder that returns a non-AwaitInProgress behaviour self.0 .iter() .find_map(|builder| match builder.on_missing_payload(args.take().unwrap()) { MissingPayloadBehaviour::AwaitInProgress => None, behaviour => Some(behaviour), }) + // If all builders return AwaitInProgress, use that as the default .unwrap_or(MissingPayloadBehaviour::AwaitInProgress) } @@ -61,9 +68,11 @@ where config: PayloadConfig, ) -> Result { let mut config = Some(config); + // Try to build an empty payload using each builder in the stack self.0 .iter() .find_map(|builder| builder.build_empty_payload(client, config.take().unwrap()).ok()) + // If no builder can build an empty payload, return an error .ok_or(PayloadBuilderError::MissingPayload) } } diff --git a/crates/payload/basic/src/lib.rs b/crates/payload/basic/src/lib.rs index 0c42c027c790..2a04fa25f072 100644 --- a/crates/payload/basic/src/lib.rs +++ b/crates/payload/basic/src/lib.rs @@ -67,7 +67,7 @@ pub struct BasicPayloadJobGenerator { /// The type responsible for building payloads. /// /// See [`PayloadBuilder`] - builder: Builder, + builders: PayloadBuilderStack, /// Stored `cached_reads` for new payload jobs. pre_cached: Option, } @@ -83,7 +83,7 @@ impl BasicPayloadJobGenerator, - builder: Builder, + builders: Builder, ) -> Self { Self { client, @@ -92,7 +92,7 @@ impl BasicPayloadJobGenerator, } impl BasicPayloadJob @@ -374,7 +374,7 @@ where let best_payload = self.best_payload.clone(); self.metrics.inc_initiated_payload_builds(); let cached_reads = self.cached_reads.take().unwrap_or_default(); - let builder = self.builder.clone(); + let builder = self.builders.clone(); self.executor.spawn_blocking(Box::pin(async move { // acquire the permit for executing the task let _permit = guard.acquire().await; @@ -478,7 +478,7 @@ where // away and the first full block should have been built by the time CL is requesting the // payload. self.metrics.inc_requested_empty_payload(); - self.builder.build_empty_payload(&self.client, self.config.clone()) + self.builders.build_empty_payload(&self.client, self.config.clone()) } fn payload_attributes(&self) -> Result { @@ -508,7 +508,7 @@ where best_payload: None, }; - match self.builder.on_missing_payload(args) { + match self.builders.on_missing_payload(args) { MissingPayloadBehaviour::AwaitInProgress => { debug!(target: "payload_builder", id=%self.config.payload_id(), "awaiting in progress payload build job"); } @@ -521,7 +521,7 @@ where let (tx, rx) = oneshot::channel(); let client = self.client.clone(); let config = self.config.clone(); - let builder = self.builder.clone(); + let builder = self.builders.clone(); self.executor.spawn_blocking(Box::pin(async move { let res = builder.build_empty_payload(&client, config); let _ = tx.send(res); From ab58fea7225f1e544e9eea839c0a188a7c723b44 Mon Sep 17 00:00:00 2001 From: -f Date: Sun, 22 Sep 2024 02:37:19 +0530 Subject: [PATCH 03/10] chaining payload builders --- crates/payload/basic/src/builder_stack.rs | 88 ++++++++++++----------- crates/payload/basic/src/lib.rs | 22 +++--- 2 files changed, 55 insertions(+), 55 deletions(-) diff --git a/crates/payload/basic/src/builder_stack.rs b/crates/payload/basic/src/builder_stack.rs index 682317820242..9100df8c7cb4 100644 --- a/crates/payload/basic/src/builder_stack.rs +++ b/crates/payload/basic/src/builder_stack.rs @@ -3,76 +3,78 @@ use crate::{ PayloadConfig, }; -/// A stack of payload builders that can be used in sequence. -#[derive(Clone, Debug)] -pub struct PayloadBuilderStack(Vec); - -impl PayloadBuilderStack { - /// Create a new payload builder stack. - pub fn new(builders: Vec) -> Self { - Self(builders) - } +/// A stack of payload builders that allows for flexible composition of payload builders. +/// +/// This structure enables the chaining of multiple `PayloadBuilder` implementations, +/// creating a hierarchical fallback system. It's designed to be nestable, allowing +/// for complex builder arrangements like `Stack, C>`. +#[derive(Debug)] +pub struct PayloadBuilderStack { + left: L, + right: R, +} - /// Push a builder onto the stack. - pub fn push(&mut self, builder: B) { - self.0.push(builder); +impl PayloadBuilderStack { + /// Creates a new `PayloadBuilderStack` with the given left and right builders. + pub const fn new(left: L, right: R) -> Self { + Self { left, right } } } -impl PayloadBuilder for PayloadBuilderStack +impl PayloadBuilder for PayloadBuilderStack where - B: PayloadBuilder, + L: PayloadBuilder, + R: PayloadBuilder, { - type Attributes = B::Attributes; - type BuiltPayload = B::BuiltPayload; + type Attributes = L::Attributes; + type BuiltPayload = L::BuiltPayload; + /// Attempts to build a payload using the left builder first, falling back to the right. fn try_build( &self, args: BuildArguments, ) -> Result, PayloadBuilderError> { let mut args = Some(args); - // Iterate through all builders in the stack - for builder in &self.0 { - match builder.try_build(args.take().unwrap()) { - // If a builder produces a Better outcome, return it immediately - Ok(outcome @ BuildOutcome::Better { .. }) => return Ok(outcome), - // If a builder produces a Valid outcome, continue with the next builder - Ok(_) => continue, - // If a builder produces an Invalid outcome, continue with the next builder - Err(_) => continue, - } + match self.left.try_build(args.take().unwrap()) { + Ok(outcome) => Ok(outcome), + Err(_) => self.right.try_build(args.take().unwrap()), } - // If no builder produced a Better outcome, return an error - Err(PayloadBuilderError::NoBetterPayload) } + /// Handles the case where a payload is missing by delegating to the left builder first, + /// then to the right. fn on_missing_payload( &self, args: BuildArguments, ) -> MissingPayloadBehaviour { let mut args = Some(args); - // Find the first builder that returns a non-AwaitInProgress behaviour - self.0 - .iter() - .find_map(|builder| match builder.on_missing_payload(args.take().unwrap()) { - MissingPayloadBehaviour::AwaitInProgress => None, - behaviour => Some(behaviour), - }) - // If all builders return AwaitInProgress, use that as the default - .unwrap_or(MissingPayloadBehaviour::AwaitInProgress) + match self.left.on_missing_payload(args.take().unwrap()) { + MissingPayloadBehaviour::RaceEmptyPayload => { + self.right.on_missing_payload(args.take().unwrap()) + } + other => other, + } } + /// Builds an empty payload using the left builder, falling back to the right. fn build_empty_payload( &self, client: &Client, config: PayloadConfig, ) -> Result { let mut config = Some(config); - // Try to build an empty payload using each builder in the stack - self.0 - .iter() - .find_map(|builder| builder.build_empty_payload(client, config.take().unwrap()).ok()) - // If no builder can build an empty payload, return an error - .ok_or(PayloadBuilderError::MissingPayload) + self.left + .build_empty_payload(client, config.take().unwrap()) + .or_else(|_| self.right.build_empty_payload(client, config.take().unwrap())) + } +} + +impl Clone for PayloadBuilderStack +where + L: Clone, + R: Clone, +{ + fn clone(&self) -> Self { + Self::new(self.left.clone(), self.right.clone()) } } diff --git a/crates/payload/basic/src/lib.rs b/crates/payload/basic/src/lib.rs index 2a04fa25f072..4a4c89428ac3 100644 --- a/crates/payload/basic/src/lib.rs +++ b/crates/payload/basic/src/lib.rs @@ -47,8 +47,6 @@ use tracing::{debug, trace, warn}; pub mod builder_stack; mod metrics; -pub use builder_stack::PayloadBuilderStack; - /// The [`PayloadJobGenerator`] that creates [`BasicPayloadJob`]s. #[derive(Debug)] pub struct BasicPayloadJobGenerator { @@ -67,7 +65,7 @@ pub struct BasicPayloadJobGenerator { /// The type responsible for building payloads. /// /// See [`PayloadBuilder`] - builders: PayloadBuilderStack, + builder: Builder, /// Stored `cached_reads` for new payload jobs. pre_cached: Option, } @@ -83,7 +81,7 @@ impl BasicPayloadJobGenerator, - builders: Builder, + builder: Builder, ) -> Self { Self { client, @@ -92,7 +90,7 @@ impl BasicPayloadJobGenerator, + builder: Builder, } impl BasicPayloadJob @@ -374,7 +372,7 @@ where let best_payload = self.best_payload.clone(); self.metrics.inc_initiated_payload_builds(); let cached_reads = self.cached_reads.take().unwrap_or_default(); - let builder = self.builders.clone(); + let builder = self.builder.clone(); self.executor.spawn_blocking(Box::pin(async move { // acquire the permit for executing the task let _permit = guard.acquire().await; @@ -411,7 +409,7 @@ where // check if the deadline is reached if this.deadline.as_mut().poll(cx).is_ready() { trace!(target: "payload_builder", "payload building deadline reached"); - return Poll::Ready(Ok(())); + return Poll::Ready(Ok(())) } // check if the interval is reached @@ -478,7 +476,7 @@ where // away and the first full block should have been built by the time CL is requesting the // payload. self.metrics.inc_requested_empty_payload(); - self.builders.build_empty_payload(&self.client, self.config.clone()) + self.builder.build_empty_payload(&self.client, self.config.clone()) } fn payload_attributes(&self) -> Result { @@ -508,7 +506,7 @@ where best_payload: None, }; - match self.builders.on_missing_payload(args) { + match self.builder.on_missing_payload(args) { MissingPayloadBehaviour::AwaitInProgress => { debug!(target: "payload_builder", id=%self.config.payload_id(), "awaiting in progress payload build job"); } @@ -521,7 +519,7 @@ where let (tx, rx) = oneshot::channel(); let client = self.client.clone(); let config = self.config.clone(); - let builder = self.builders.clone(); + let builder = self.builder.clone(); self.executor.spawn_blocking(Box::pin(async move { let res = builder.build_empty_payload(&client, config); let _ = tx.send(res); From 12f2e7a60d4948fb2765e5ec652d884b9d9d4ec5 Mon Sep 17 00:00:00 2001 From: -f Date: Sun, 22 Sep 2024 02:41:00 +0530 Subject: [PATCH 04/10] revert --- crates/payload/basic/src/lib.rs | 19 +++++++++---------- crates/payload/builder/src/error.rs | 6 ++---- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/crates/payload/basic/src/lib.rs b/crates/payload/basic/src/lib.rs index 4a4c89428ac3..6a03c35c1954 100644 --- a/crates/payload/basic/src/lib.rs +++ b/crates/payload/basic/src/lib.rs @@ -9,6 +9,7 @@ #![cfg_attr(docsrs, feature(doc_cfg, doc_auto_cfg))] use crate::metrics::PayloadBuilderMetrics; +use alloy_primitives::{Bytes, B256, U256}; use futures_core::ready; use futures_util::FutureExt; use reth_chainspec::{ChainSpec, EthereumHardforks}; @@ -19,7 +20,7 @@ use reth_payload_builder::{ use reth_payload_primitives::{BuiltPayload, PayloadBuilderAttributes}; use reth_primitives::{ constants::{EMPTY_WITHDRAWALS, RETH_CLIENT_VERSION, SLOT_DURATION}, - proofs, BlockNumberOrTag, Bytes, SealedBlock, Withdrawals, B256, U256, + proofs, BlockNumberOrTag, SealedBlock, Withdrawals, }; use reth_provider::{ BlockReaderIdExt, BlockSource, CanonStateNotification, ProviderError, StateProviderFactory, @@ -43,8 +44,6 @@ use tokio::{ }; use tracing::{debug, trace, warn}; -/// The [`PayloadBuilder`] that builds payloads using a stack of builders. -pub mod builder_stack; mod metrics; /// The [`PayloadJobGenerator`] that creates [`BasicPayloadJob`]s. @@ -467,7 +466,7 @@ where fn best_payload(&self) -> Result { if let Some(ref payload) = self.best_payload { - return Ok(payload.clone()); + return Ok(payload.clone()) } // No payload has been built yet, but we need to return something that the CL then can // deliver, so we need to return an empty payload. @@ -585,14 +584,14 @@ where this.maybe_better = None; if let Ok(BuildOutcome::Better { payload, .. }) = res { debug!(target: "payload_builder", "resolving better payload"); - return Poll::Ready(Ok(payload)); + return Poll::Ready(Ok(payload)) } } } if let Some(best) = this.best_payload.take() { debug!(target: "payload_builder", "resolving best payload"); - return Poll::Ready(Ok(best)); + return Poll::Ready(Ok(best)) } if let Some(fut) = Pin::new(&mut this.empty_payload).as_pin_mut() { @@ -608,12 +607,12 @@ where Poll::Ready(res) } Err(err) => Poll::Ready(Err(err.into())), - }; + } } } if this.is_empty() { - return Poll::Ready(Err(PayloadBuilderError::MissingPayload)); + return Poll::Ready(Err(PayloadBuilderError::MissingPayload)) } Poll::Pending @@ -920,11 +919,11 @@ pub fn commit_withdrawals>( withdrawals: Withdrawals, ) -> Result { if !chain_spec.is_shanghai_active_at_timestamp(timestamp) { - return Ok(WithdrawalsOutcome::pre_shanghai()); + return Ok(WithdrawalsOutcome::pre_shanghai()) } if withdrawals.is_empty() { - return Ok(WithdrawalsOutcome::empty()); + return Ok(WithdrawalsOutcome::empty()) } let balance_increments = diff --git a/crates/payload/builder/src/error.rs b/crates/payload/builder/src/error.rs index b9f5fae54e64..2ad7b287eea8 100644 --- a/crates/payload/builder/src/error.rs +++ b/crates/payload/builder/src/error.rs @@ -1,7 +1,8 @@ //! Error types emitted by types or implementations of this crate. +use alloy_primitives::B256; use reth_errors::{ProviderError, RethError}; -use reth_primitives::{revm_primitives::EVMError, B256}; +use reth_primitives::revm_primitives::EVMError; use reth_transaction_pool::BlobStoreError; use tokio::sync::oneshot; @@ -29,9 +30,6 @@ pub enum PayloadBuilderError { /// Thrown if the payload requests withdrawals before Shanghai activation. #[error("withdrawals set before Shanghai activation")] WithdrawalsBeforeShanghai, - /// Thrown when no better payload could be built. - #[error("no better payload could be built")] - NoBetterPayload, /// Any other payload building errors. #[error(transparent)] Other(Box), From 90114afaf4d801fa292581543cb8250226fee0f2 Mon Sep 17 00:00:00 2001 From: -f Date: Sun, 22 Sep 2024 02:52:46 +0530 Subject: [PATCH 05/10] mod --- crates/payload/basic/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/payload/basic/src/lib.rs b/crates/payload/basic/src/lib.rs index 6a03c35c1954..5f7be31920ba 100644 --- a/crates/payload/basic/src/lib.rs +++ b/crates/payload/basic/src/lib.rs @@ -45,6 +45,7 @@ use tokio::{ use tracing::{debug, trace, warn}; mod metrics; +mod builder_stack; /// The [`PayloadJobGenerator`] that creates [`BasicPayloadJob`]s. #[derive(Debug)] From f2b569849a0324e8a3abac8daaaba21d4c38e7bf Mon Sep 17 00:00:00 2001 From: -f Date: Sun, 29 Sep 2024 01:17:57 +0530 Subject: [PATCH 06/10] either --- crates/payload/basic/src/builder_stack.rs | 320 +++++++++++++++++++--- crates/payload/basic/src/lib.rs | 2 + 2 files changed, 279 insertions(+), 43 deletions(-) diff --git a/crates/payload/basic/src/builder_stack.rs b/crates/payload/basic/src/builder_stack.rs index 9100df8c7cb4..bf1157169aa5 100644 --- a/crates/payload/basic/src/builder_stack.rs +++ b/crates/payload/basic/src/builder_stack.rs @@ -1,13 +1,148 @@ use crate::{ - BuildArguments, BuildOutcome, MissingPayloadBehaviour, PayloadBuilder, PayloadBuilderError, - PayloadConfig, + BuildArguments, BuildOutcome, PayloadBuilder, PayloadBuilderError, + PayloadConfig, PayloadBuilderAttributes }; +use futures_util::future::Either as EitherFuture; -/// A stack of payload builders that allows for flexible composition of payload builders. -/// -/// This structure enables the chaining of multiple `PayloadBuilder` implementations, +use alloy_primitives::B256; +use reth_payload_builder::PayloadId; +use reth_payload_primitives::BuiltPayload; +use reth_primitives::{SealedBlock, U256}; + +use std::fmt; + +/// wrapper for either error +#[derive(Debug)] +pub enum EitherError { + /// left error + Left(L), + /// right error + Right(R), +} + +impl fmt::Display for EitherError +where + L: fmt::Display, + R: fmt::Display, +{ + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + EitherError::Left(l) => write!(f, "Left error: {}", l), + EitherError::Right(r) => write!(f, "Right error: {}", r), + } + } +} + +impl std::error::Error for EitherError +where + L: std::error::Error, + R: std::error::Error, +{} + +/// represents attributes that can be either left or right type. +#[derive(Debug, Clone)] +pub struct EitherAttributes(EitherFuture); + +/// implement PayloadBuilderAttributes for EitherAttributes for use in PayloadBuilderStack +impl PayloadBuilderAttributes for EitherAttributes +where + LAttr: PayloadBuilderAttributes, + RAttr: PayloadBuilderAttributes, +{ + type RpcPayloadAttributes = EitherFuture< + LAttr::RpcPayloadAttributes, + RAttr::RpcPayloadAttributes, + >; + type Error = EitherError; + + fn try_new( + parent: B256, + rpc_payload_attributes: Self::RpcPayloadAttributes, + ) -> Result { + match rpc_payload_attributes { + EitherFuture::Left(attr) => LAttr::try_new(parent, attr) + .map(|attr| EitherAttributes(EitherFuture::Left(attr))) + .map_err(EitherError::Left), + EitherFuture::Right(attr) => RAttr::try_new(parent, attr) + .map(|attr| EitherAttributes(EitherFuture::Right(attr))) + .map_err(EitherError::Right), + } + } + + fn payload_id(&self) -> PayloadId { + match &self.0 { + EitherFuture::Left(attr) => attr.payload_id(), + EitherFuture::Right(attr) => attr.payload_id(), + } + } + + fn parent(&self) -> B256 { + match &self.0 { + EitherFuture::Left(attr) => attr.parent(), + EitherFuture::Right(attr) => attr.parent(), + } + } + + fn timestamp(&self) -> u64 { + match &self.0 { + EitherFuture::Left(attr) => attr.timestamp(), + EitherFuture::Right(attr) => attr.timestamp(), + } + } + + fn parent_beacon_block_root(&self) -> Option { + match &self.0 { + EitherFuture::Left(attr) => attr.parent_beacon_block_root(), + EitherFuture::Right(attr) => attr.parent_beacon_block_root(), + } + } + + fn suggested_fee_recipient(&self) -> alloy_primitives::Address { + match &self.0 { + EitherFuture::Left(attr) => attr.suggested_fee_recipient(), + EitherFuture::Right(attr) => attr.suggested_fee_recipient(), + } + } + + fn prev_randao(&self) -> B256 { + match &self.0 { + EitherFuture::Left(attr) => attr.prev_randao(), + EitherFuture::Right(attr) => attr.prev_randao(), + } + } + + fn withdrawals(&self) -> &reth_primitives::Withdrawals { + match &self.0 { + EitherFuture::Left(attr) => attr.withdrawals(), + EitherFuture::Right(attr) => attr.withdrawals(), + } + } +} + +impl BuiltPayload for EitherAttributes +where + L: BuiltPayload, + R: BuiltPayload, +{ + fn block(&self) -> &SealedBlock { + match &self.0 { + EitherFuture::Left(l) => l.block(), + EitherFuture::Right(r) => r.block(), + } + } + + fn fees(&self) -> U256 { + match &self.0 { + EitherFuture::Left(l) => l.fees(), + EitherFuture::Right(r) => r.fees(), + } + } +} + + +/// this structure enables the chaining of multiple `PayloadBuilder` implementations, /// creating a hierarchical fallback system. It's designed to be nestable, allowing -/// for complex builder arrangements like `Stack, C>`. +/// for complex builder arrangements like `Stack, C>` with different #[derive(Debug)] pub struct PayloadBuilderStack { left: L, @@ -21,60 +156,159 @@ impl PayloadBuilderStack { } } +impl Clone for PayloadBuilderStack +where + L: Clone, + R: Clone, +{ + fn clone(&self) -> Self { + Self::new(self.left.clone(), self.right.clone()) + } +} + +impl BuildOutcome { + fn map_payload(self, f: F) -> BuildOutcome + where + F: FnOnce(B) -> B2, + { + match self { + BuildOutcome::Better { payload, cached_reads } => BuildOutcome::Better { + payload: f(payload), + cached_reads, + }, + BuildOutcome::Aborted { fees, cached_reads } => BuildOutcome::Aborted { fees, cached_reads }, + BuildOutcome::Cancelled => BuildOutcome::Cancelled, + } + } +} + impl PayloadBuilder for PayloadBuilderStack where - L: PayloadBuilder, - R: PayloadBuilder, + L: PayloadBuilder + Unpin + 'static, + R: PayloadBuilder + Unpin + 'static, + Client: Clone, + Pool: Clone, + L::Attributes: Unpin + Clone, // Ensure Attributes can be cloned + R::Attributes: Unpin + Clone, // Ensure Attributes can be cloned + L::BuiltPayload: Unpin + Clone, // Ensure BuiltPayload can be cloned + R::BuiltPayload: Unpin + Clone, // Ensure BuiltPayload can be cloned { - type Attributes = L::Attributes; - type BuiltPayload = L::BuiltPayload; + type Attributes = EitherAttributes; + type BuiltPayload = EitherAttributes; - /// Attempts to build a payload using the left builder first, falling back to the right. + /// attempts to build a payload using the left builder first, falling back to the right fn try_build( &self, args: BuildArguments, ) -> Result, PayloadBuilderError> { - let mut args = Some(args); - match self.left.try_build(args.take().unwrap()) { - Ok(outcome) => Ok(outcome), - Err(_) => self.right.try_build(args.take().unwrap()), + // attempt to build using the left builder + if let EitherFuture::Left(ref left_attr) = args.config.attributes.0 { + let left_args = BuildArguments { + client: args.client.clone(), + pool: args.pool.clone(), + cached_reads: args.cached_reads.clone(), + config: PayloadConfig { + parent_block: args.config.parent_block.clone(), + extra_data: args.config.extra_data.clone(), + attributes: left_attr.clone(), + chain_spec: args.config.chain_spec.clone(), + }, + cancel: args.cancel.clone(), + best_payload: args.best_payload.clone().and_then(|p| { + if let EitherAttributes(EitherFuture::Left(payload)) = p { + Some(payload.clone()) + } else { + None + } + }), + }; + + // try building with the left builder + match self.left.try_build(left_args) { + Ok(BuildOutcome::Better { payload, cached_reads }) => { + return Ok(BuildOutcome::Better { + payload: EitherAttributes(EitherFuture::Left(payload)), + cached_reads, + }); + } + Ok(other) => { + return Ok(other.map_payload(|payload| EitherAttributes(EitherFuture::Left(payload)))); + } + Err(_) => { + // if left builder fails, proceed to the right builder + } + } } - } - /// Handles the case where a payload is missing by delegating to the left builder first, - /// then to the right. - fn on_missing_payload( - &self, - args: BuildArguments, - ) -> MissingPayloadBehaviour { - let mut args = Some(args); - match self.left.on_missing_payload(args.take().unwrap()) { - MissingPayloadBehaviour::RaceEmptyPayload => { - self.right.on_missing_payload(args.take().unwrap()) + // attempt to build using the right builder + if let EitherFuture::Right(ref right_attr) = args.config.attributes.0 { + let right_args = BuildArguments { + client: args.client.clone(), + pool: args.pool.clone(), + cached_reads: args.cached_reads.clone(), + config: PayloadConfig { + parent_block: args.config.parent_block.clone(), + extra_data: args.config.extra_data.clone(), + attributes: right_attr.clone(), + chain_spec: args.config.chain_spec.clone(), + }, + cancel: args.cancel.clone(), + best_payload: args.best_payload.and_then(|p| { + if let EitherAttributes(EitherFuture::Right(payload)) = p { + Some(payload.clone()) + } else { + None + } + }), + }; + + // try building with the right builder + match self.right.try_build(right_args) { + Ok(BuildOutcome::Better { payload, cached_reads }) => { + return Ok(BuildOutcome::Better { + payload: EitherAttributes(EitherFuture::Right(payload)), + cached_reads, + }); + } + Ok(other) => { + return Ok(other.map_payload(|payload| EitherAttributes(EitherFuture::Right(payload)))); + } + Err(err) => { + return Err(err); + } } - other => other, } + + // if attributes don't match Left or Right variants, return an error + Err(PayloadBuilderError::MissingPayload) } - /// Builds an empty payload using the left builder, falling back to the right. fn build_empty_payload( &self, client: &Client, config: PayloadConfig, ) -> Result { - let mut config = Some(config); - self.left - .build_empty_payload(client, config.take().unwrap()) - .or_else(|_| self.right.build_empty_payload(client, config.take().unwrap())) - } -} - -impl Clone for PayloadBuilderStack -where - L: Clone, - R: Clone, -{ - fn clone(&self) -> Self { - Self::new(self.left.clone(), self.right.clone()) + match config.attributes.0 { + EitherFuture::Left(left_attr) => { + let left_config = PayloadConfig { + attributes: left_attr, + parent_block: config.parent_block.clone(), + extra_data: config.extra_data.clone(), + chain_spec: config.chain_spec.clone(), + }; + self.left.build_empty_payload(client, left_config) + .map(|payload| EitherAttributes(EitherFuture::Left(payload))) + }, + EitherFuture::Right(right_attr) => { + let right_config = PayloadConfig { + attributes: right_attr, + parent_block: config.parent_block.clone(), + extra_data: config.extra_data.clone(), + chain_spec: config.chain_spec.clone(), + }; + self.right.build_empty_payload(client, right_config) + .map(|payload| EitherAttributes(EitherFuture::Right(payload))) + }, + } } -} +} \ No newline at end of file diff --git a/crates/payload/basic/src/lib.rs b/crates/payload/basic/src/lib.rs index 44425a67977e..8fb14b435bbf 100644 --- a/crates/payload/basic/src/lib.rs +++ b/crates/payload/basic/src/lib.rs @@ -46,6 +46,8 @@ use tracing::{debug, trace, warn}; mod metrics; mod builder_stack; +pub use builder_stack::{EitherError, EitherAttributes, PayloadBuilderStack}; + /// The [`PayloadJobGenerator`] that creates [`BasicPayloadJob`]s. #[derive(Debug)] pub struct BasicPayloadJobGenerator { From 1f0e8819f31d60efe25f1b5892b34c9cfa3f0697 Mon Sep 17 00:00:00 2001 From: -f Date: Sun, 29 Sep 2024 01:27:25 +0530 Subject: [PATCH 07/10] rm chain_spec --- crates/payload/basic/src/builder_stack.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/crates/payload/basic/src/builder_stack.rs b/crates/payload/basic/src/builder_stack.rs index bf1157169aa5..63f207ceb2d1 100644 --- a/crates/payload/basic/src/builder_stack.rs +++ b/crates/payload/basic/src/builder_stack.rs @@ -211,7 +211,6 @@ where parent_block: args.config.parent_block.clone(), extra_data: args.config.extra_data.clone(), attributes: left_attr.clone(), - chain_spec: args.config.chain_spec.clone(), }, cancel: args.cancel.clone(), best_payload: args.best_payload.clone().and_then(|p| { @@ -250,7 +249,6 @@ where parent_block: args.config.parent_block.clone(), extra_data: args.config.extra_data.clone(), attributes: right_attr.clone(), - chain_spec: args.config.chain_spec.clone(), }, cancel: args.cancel.clone(), best_payload: args.best_payload.and_then(|p| { @@ -294,7 +292,7 @@ where attributes: left_attr, parent_block: config.parent_block.clone(), extra_data: config.extra_data.clone(), - chain_spec: config.chain_spec.clone(), + }; self.left.build_empty_payload(client, left_config) .map(|payload| EitherAttributes(EitherFuture::Left(payload))) @@ -304,7 +302,6 @@ where attributes: right_attr, parent_block: config.parent_block.clone(), extra_data: config.extra_data.clone(), - chain_spec: config.chain_spec.clone(), }; self.right.build_empty_payload(client, right_config) .map(|payload| EitherAttributes(EitherFuture::Right(payload))) From 18cb3155e51a52f924bbba1635e4bd82163ce82d Mon Sep 17 00:00:00 2001 From: -f Date: Sun, 29 Sep 2024 18:56:48 +0530 Subject: [PATCH 08/10] hand-rolled either --- crates/payload/basic/src/builder_stack.rs | 400 +++++++++++----------- crates/payload/basic/src/lib.rs | 2 +- 2 files changed, 206 insertions(+), 196 deletions(-) diff --git a/crates/payload/basic/src/builder_stack.rs b/crates/payload/basic/src/builder_stack.rs index 63f207ceb2d1..4b50b2a14f52 100644 --- a/crates/payload/basic/src/builder_stack.rs +++ b/crates/payload/basic/src/builder_stack.rs @@ -2,143 +2,119 @@ use crate::{ BuildArguments, BuildOutcome, PayloadBuilder, PayloadBuilderError, PayloadConfig, PayloadBuilderAttributes }; -use futures_util::future::Either as EitherFuture; -use alloy_primitives::B256; +use alloy_primitives::{Address, B256}; use reth_payload_builder::PayloadId; use reth_payload_primitives::BuiltPayload; -use reth_primitives::{SealedBlock, U256}; +use reth_primitives::{SealedBlock, Withdrawals, U256}; use std::fmt; +use std::error::Error; -/// wrapper for either error -#[derive(Debug)] -pub enum EitherError { - /// left error +/// hand rolled Either enum to handle two builder types +#[derive(Debug, Clone)] +pub enum Either { + /// left variant Left(L), - /// right error + /// right variant Right(R), } -impl fmt::Display for EitherError +impl fmt::Display for Either where L: fmt::Display, R: fmt::Display, { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - EitherError::Left(l) => write!(f, "Left error: {}", l), - EitherError::Right(r) => write!(f, "Right error: {}", r), + Either::Left(l) => write!(f, "Left: {}", l), + Either::Right(r) => write!(f, "Right: {}", r), } } } -impl std::error::Error for EitherError -where - L: std::error::Error, - R: std::error::Error, -{} - -/// represents attributes that can be either left or right type. -#[derive(Debug, Clone)] -pub struct EitherAttributes(EitherFuture); - -/// implement PayloadBuilderAttributes for EitherAttributes for use in PayloadBuilderStack -impl PayloadBuilderAttributes for EitherAttributes +impl Error for Either where - LAttr: PayloadBuilderAttributes, - RAttr: PayloadBuilderAttributes, + L: Error + 'static, + R: Error + 'static, { - type RpcPayloadAttributes = EitherFuture< - LAttr::RpcPayloadAttributes, - RAttr::RpcPayloadAttributes, - >; - type Error = EitherError; - - fn try_new( - parent: B256, - rpc_payload_attributes: Self::RpcPayloadAttributes, - ) -> Result { - match rpc_payload_attributes { - EitherFuture::Left(attr) => LAttr::try_new(parent, attr) - .map(|attr| EitherAttributes(EitherFuture::Left(attr))) - .map_err(EitherError::Left), - EitherFuture::Right(attr) => RAttr::try_new(parent, attr) - .map(|attr| EitherAttributes(EitherFuture::Right(attr))) - .map_err(EitherError::Right), - } - } - - fn payload_id(&self) -> PayloadId { - match &self.0 { - EitherFuture::Left(attr) => attr.payload_id(), - EitherFuture::Right(attr) => attr.payload_id(), - } - } - - fn parent(&self) -> B256 { - match &self.0 { - EitherFuture::Left(attr) => attr.parent(), - EitherFuture::Right(attr) => attr.parent(), - } - } - - fn timestamp(&self) -> u64 { - match &self.0 { - EitherFuture::Left(attr) => attr.timestamp(), - EitherFuture::Right(attr) => attr.timestamp(), - } - } - - fn parent_beacon_block_root(&self) -> Option { - match &self.0 { - EitherFuture::Left(attr) => attr.parent_beacon_block_root(), - EitherFuture::Right(attr) => attr.parent_beacon_block_root(), + fn source(&self) -> Option<&(dyn Error + 'static)> { + match self { + Either::Left(l) => Some(l), + Either::Right(r) => Some(r), } } +} - fn suggested_fee_recipient(&self) -> alloy_primitives::Address { - match &self.0 { - EitherFuture::Left(attr) => attr.suggested_fee_recipient(), - EitherFuture::Right(attr) => attr.suggested_fee_recipient(), - } - } +impl PayloadBuilderAttributes for Either + where + L: PayloadBuilderAttributes, + R: PayloadBuilderAttributes, + L::Error: Error + 'static, + R::Error: Error + 'static, + { + type RpcPayloadAttributes = Either; + type Error = Either; + + fn try_new( + parent: B256, + rpc_payload_attributes: Self::RpcPayloadAttributes, + ) -> Result { + match rpc_payload_attributes { + Either::Left(attr) => L::try_new(parent, attr).map(Either::Left).map_err(Either::Left), + Either::Right(attr) => R::try_new(parent, attr).map(Either::Right).map_err(Either::Right), + } + } + + fn payload_id(&self) -> PayloadId { + match self { + Either::Left(l) => l.payload_id(), + Either::Right(r) => r.payload_id(), + } + } - fn prev_randao(&self) -> B256 { - match &self.0 { - EitherFuture::Left(attr) => attr.prev_randao(), - EitherFuture::Right(attr) => attr.prev_randao(), - } - } + fn parent(&self) -> B256 { + match self { + Either::Left(l) => l.parent(), + Either::Right(r) => r.parent(), + } + } + + fn timestamp(&self) -> u64 { + match self { + Either::Left(l) => l.timestamp(), + Either::Right(r) => r.timestamp(), + } + } - fn withdrawals(&self) -> &reth_primitives::Withdrawals { - match &self.0 { - EitherFuture::Left(attr) => attr.withdrawals(), - EitherFuture::Right(attr) => attr.withdrawals(), - } - } -} + fn parent_beacon_block_root(&self) -> Option { + match self { + Either::Left(l) => l.parent_beacon_block_root(), + Either::Right(r) => r.parent_beacon_block_root(), + } + } -impl BuiltPayload for EitherAttributes -where - L: BuiltPayload, - R: BuiltPayload, -{ - fn block(&self) -> &SealedBlock { - match &self.0 { - EitherFuture::Left(l) => l.block(), - EitherFuture::Right(r) => r.block(), - } - } + fn suggested_fee_recipient(&self) -> Address { + match self { + Either::Left(l) => l.suggested_fee_recipient(), + Either::Right(r) => r.suggested_fee_recipient(), + } + } - fn fees(&self) -> U256 { - match &self.0 { - EitherFuture::Left(l) => l.fees(), - EitherFuture::Right(r) => r.fees(), - } - } -} + fn prev_randao(&self) -> B256 { + match self { + Either::Left(l) => l.prev_randao(), + Either::Right(r) => r.prev_randao(), + } + } + fn withdrawals(&self) -> &Withdrawals { + match self { + Either::Left(l) => l.withdrawals(), + Either::Right(r) => r.withdrawals(), + } + } + } /// this structure enables the chaining of multiple `PayloadBuilder` implementations, /// creating a hierarchical fallback system. It's designed to be nestable, allowing @@ -166,6 +142,26 @@ where } } +impl BuiltPayload for Either +where + L: BuiltPayload, + R: BuiltPayload, +{ + fn block(&self) -> &SealedBlock { + match self { + Either::Left(l) => l.block(), + Either::Right(r) => r.block(), + } + } + + fn fees(&self) -> U256 { + match self { + Either::Left(l) => l.fees(), + Either::Right(r) => r.fees(), + } + } +} + impl BuildOutcome { fn map_payload(self, f: F) -> BuildOutcome where @@ -184,101 +180,100 @@ impl BuildOutcome { impl PayloadBuilder for PayloadBuilderStack where - L: PayloadBuilder + Unpin + 'static, + L: PayloadBuilder + Unpin + 'static, R: PayloadBuilder + Unpin + 'static, Client: Clone, Pool: Clone, - L::Attributes: Unpin + Clone, // Ensure Attributes can be cloned - R::Attributes: Unpin + Clone, // Ensure Attributes can be cloned - L::BuiltPayload: Unpin + Clone, // Ensure BuiltPayload can be cloned - R::BuiltPayload: Unpin + Clone, // Ensure BuiltPayload can be cloned + L::Attributes: Unpin + Clone, + R::Attributes: Unpin + Clone, + L::BuiltPayload: Unpin + Clone, + R::BuiltPayload: Unpin + Clone, + <>::Attributes as PayloadBuilderAttributes>::Error: 'static, + <>::Attributes as PayloadBuilderAttributes>::Error: 'static, { - type Attributes = EitherAttributes; - type BuiltPayload = EitherAttributes; + type Attributes = Either; + type BuiltPayload = Either; - /// attempts to build a payload using the left builder first, falling back to the right fn try_build( &self, args: BuildArguments, ) -> Result, PayloadBuilderError> { - // attempt to build using the left builder - if let EitherFuture::Left(ref left_attr) = args.config.attributes.0 { - let left_args = BuildArguments { - client: args.client.clone(), - pool: args.pool.clone(), - cached_reads: args.cached_reads.clone(), - config: PayloadConfig { - parent_block: args.config.parent_block.clone(), - extra_data: args.config.extra_data.clone(), - attributes: left_attr.clone(), - }, - cancel: args.cancel.clone(), - best_payload: args.best_payload.clone().and_then(|p| { - if let EitherAttributes(EitherFuture::Left(payload)) = p { - Some(payload.clone()) - } else { - None - } - }), - }; + match args.config.attributes { + Either::Left(ref left_attr) => { + let left_args: BuildArguments = BuildArguments { + client: args.client.clone(), + pool: args.pool.clone(), + cached_reads: args.cached_reads.clone(), + config: PayloadConfig { + parent_block: args.config.parent_block.clone(), + extra_data: args.config.extra_data.clone(), + attributes: left_attr.clone(), + }, + cancel: args.cancel.clone(), + best_payload: args.best_payload.clone().and_then(|payload| { + if let Either::Left(p) = payload { + Some(p) + } else { + None + } + }), + }; - // try building with the left builder - match self.left.try_build(left_args) { - Ok(BuildOutcome::Better { payload, cached_reads }) => { - return Ok(BuildOutcome::Better { - payload: EitherAttributes(EitherFuture::Left(payload)), - cached_reads, - }); - } - Ok(other) => { - return Ok(other.map_payload(|payload| EitherAttributes(EitherFuture::Left(payload)))); - } - Err(_) => { - // if left builder fails, proceed to the right builder + match self.left.try_build(left_args) { + Ok(BuildOutcome::Better { payload, cached_reads }) => { + // Wrap the payload in Either::Left and return + return Ok(BuildOutcome::Better { + payload: Either::Left(payload), + cached_reads, + }) + } + Ok(other) => { + return Ok(other.map_payload(Either::Left)) + } + Err(_) => { + } } } - } + Either::Right(ref right_attr) => { + let right_args = BuildArguments { + client: args.client.clone(), + pool: args.pool.clone(), + cached_reads: args.cached_reads.clone(), + config: PayloadConfig { + parent_block: args.config.parent_block.clone(), + extra_data: args.config.extra_data.clone(), + attributes: right_attr.clone(), + }, + cancel: args.cancel.clone(), + best_payload: args.best_payload.clone().and_then(|payload| { + if let Either::Right(p) = payload { + Some(p) + } else { + None + } + }), + }; - // attempt to build using the right builder - if let EitherFuture::Right(ref right_attr) = args.config.attributes.0 { - let right_args = BuildArguments { - client: args.client.clone(), - pool: args.pool.clone(), - cached_reads: args.cached_reads.clone(), - config: PayloadConfig { - parent_block: args.config.parent_block.clone(), - extra_data: args.config.extra_data.clone(), - attributes: right_attr.clone(), - }, - cancel: args.cancel.clone(), - best_payload: args.best_payload.and_then(|p| { - if let EitherAttributes(EitherFuture::Right(payload)) = p { - Some(payload.clone()) - } else { - None + match self.right.try_build(right_args) { + Ok(BuildOutcome::Better { payload, cached_reads }) => { + return Ok(BuildOutcome::Better { + payload: Either::Right(payload), + cached_reads, + }) + } + Ok(other) => { + return Ok(other.map_payload(Either::Right)) + } + Err(e) => { + return Err(e); } - }), - }; - - // try building with the right builder - match self.right.try_build(right_args) { - Ok(BuildOutcome::Better { payload, cached_reads }) => { - return Ok(BuildOutcome::Better { - payload: EitherAttributes(EitherFuture::Right(payload)), - cached_reads, - }); - } - Ok(other) => { - return Ok(other.map_payload(|payload| EitherAttributes(EitherFuture::Right(payload)))); - } - Err(err) => { - return Err(err); } } } - - // if attributes don't match Left or Right variants, return an error - Err(PayloadBuilderError::MissingPayload) + Err(PayloadBuilderError::Other(Box::new(std::io::Error::new( + std::io::ErrorKind::Other, + "Both left and right builders failed to build the payload" + )))) } fn build_empty_payload( @@ -286,26 +281,41 @@ where client: &Client, config: PayloadConfig, ) -> Result { - match config.attributes.0 { - EitherFuture::Left(left_attr) => { + match config.attributes { + Either::Left(left_attr) => { let left_config = PayloadConfig { attributes: left_attr, parent_block: config.parent_block.clone(), extra_data: config.extra_data.clone(), - }; - self.left.build_empty_payload(client, left_config) - .map(|payload| EitherAttributes(EitherFuture::Left(payload))) + + match self.left.build_empty_payload(client, left_config) { + Ok(payload_left) => { + return Ok(Either::Left(payload_left)) + }, + Err(_) => {} + } }, - EitherFuture::Right(right_attr) => { + Either::Right(right_attr) => { let right_config = PayloadConfig { - attributes: right_attr, parent_block: config.parent_block.clone(), extra_data: config.extra_data.clone(), + attributes: right_attr.clone(), }; - self.right.build_empty_payload(client, right_config) - .map(|payload| EitherAttributes(EitherFuture::Right(payload))) - }, + + match self.right.build_empty_payload(client, right_config) { + Ok(payload_right) => { + return Ok(Either::Right(payload_right)) + }, + Err(e) => { + return Err(e); + } + } + } } + Err(PayloadBuilderError::Other(Box::new(std::io::Error::new( + std::io::ErrorKind::Other, + "Failed to build empty payload with both left and right builders" + )))) } } \ No newline at end of file diff --git a/crates/payload/basic/src/lib.rs b/crates/payload/basic/src/lib.rs index 83756354e3ee..97f0ae669f54 100644 --- a/crates/payload/basic/src/lib.rs +++ b/crates/payload/basic/src/lib.rs @@ -46,7 +46,7 @@ use tracing::{debug, trace, warn}; mod metrics; mod builder_stack; -pub use builder_stack::{EitherError, EitherAttributes, PayloadBuilderStack}; +pub use builder_stack::PayloadBuilderStack; /// The [`PayloadJobGenerator`] that creates [`BasicPayloadJob`]s. #[derive(Debug)] From 8446411d6a4439c6f01763dc1a6f1ac2fd46e09d Mon Sep 17 00:00:00 2001 From: -f Date: Sun, 29 Sep 2024 18:58:14 +0530 Subject: [PATCH 09/10] rm --- crates/payload/basic/src/builder_stack.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/payload/basic/src/builder_stack.rs b/crates/payload/basic/src/builder_stack.rs index 4b50b2a14f52..8229f0489950 100644 --- a/crates/payload/basic/src/builder_stack.rs +++ b/crates/payload/basic/src/builder_stack.rs @@ -221,7 +221,6 @@ where match self.left.try_build(left_args) { Ok(BuildOutcome::Better { payload, cached_reads }) => { - // Wrap the payload in Either::Left and return return Ok(BuildOutcome::Better { payload: Either::Left(payload), cached_reads, From 2c04a630d3c595b8241c068910507a7f031002ec Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Thu, 7 Nov 2024 13:26:34 +0100 Subject: [PATCH 10/10] chore: touchups --- crates/payload/basic/src/builder_stack.rs | 320 ---------------------- crates/payload/basic/src/lib.rs | 19 +- crates/payload/basic/src/stack.rs | 270 ++++++++++++++++++ 3 files changed, 287 insertions(+), 322 deletions(-) delete mode 100644 crates/payload/basic/src/builder_stack.rs create mode 100644 crates/payload/basic/src/stack.rs diff --git a/crates/payload/basic/src/builder_stack.rs b/crates/payload/basic/src/builder_stack.rs deleted file mode 100644 index 8229f0489950..000000000000 --- a/crates/payload/basic/src/builder_stack.rs +++ /dev/null @@ -1,320 +0,0 @@ -use crate::{ - BuildArguments, BuildOutcome, PayloadBuilder, PayloadBuilderError, - PayloadConfig, PayloadBuilderAttributes -}; - -use alloy_primitives::{Address, B256}; -use reth_payload_builder::PayloadId; -use reth_payload_primitives::BuiltPayload; -use reth_primitives::{SealedBlock, Withdrawals, U256}; - -use std::fmt; -use std::error::Error; - -/// hand rolled Either enum to handle two builder types -#[derive(Debug, Clone)] -pub enum Either { - /// left variant - Left(L), - /// right variant - Right(R), -} - -impl fmt::Display for Either -where - L: fmt::Display, - R: fmt::Display, -{ - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Either::Left(l) => write!(f, "Left: {}", l), - Either::Right(r) => write!(f, "Right: {}", r), - } - } -} - -impl Error for Either -where - L: Error + 'static, - R: Error + 'static, -{ - fn source(&self) -> Option<&(dyn Error + 'static)> { - match self { - Either::Left(l) => Some(l), - Either::Right(r) => Some(r), - } - } -} - -impl PayloadBuilderAttributes for Either - where - L: PayloadBuilderAttributes, - R: PayloadBuilderAttributes, - L::Error: Error + 'static, - R::Error: Error + 'static, - { - type RpcPayloadAttributes = Either; - type Error = Either; - - fn try_new( - parent: B256, - rpc_payload_attributes: Self::RpcPayloadAttributes, - ) -> Result { - match rpc_payload_attributes { - Either::Left(attr) => L::try_new(parent, attr).map(Either::Left).map_err(Either::Left), - Either::Right(attr) => R::try_new(parent, attr).map(Either::Right).map_err(Either::Right), - } - } - - fn payload_id(&self) -> PayloadId { - match self { - Either::Left(l) => l.payload_id(), - Either::Right(r) => r.payload_id(), - } - } - - fn parent(&self) -> B256 { - match self { - Either::Left(l) => l.parent(), - Either::Right(r) => r.parent(), - } - } - - fn timestamp(&self) -> u64 { - match self { - Either::Left(l) => l.timestamp(), - Either::Right(r) => r.timestamp(), - } - } - - fn parent_beacon_block_root(&self) -> Option { - match self { - Either::Left(l) => l.parent_beacon_block_root(), - Either::Right(r) => r.parent_beacon_block_root(), - } - } - - fn suggested_fee_recipient(&self) -> Address { - match self { - Either::Left(l) => l.suggested_fee_recipient(), - Either::Right(r) => r.suggested_fee_recipient(), - } - } - - fn prev_randao(&self) -> B256 { - match self { - Either::Left(l) => l.prev_randao(), - Either::Right(r) => r.prev_randao(), - } - } - - fn withdrawals(&self) -> &Withdrawals { - match self { - Either::Left(l) => l.withdrawals(), - Either::Right(r) => r.withdrawals(), - } - } - } - -/// this structure enables the chaining of multiple `PayloadBuilder` implementations, -/// creating a hierarchical fallback system. It's designed to be nestable, allowing -/// for complex builder arrangements like `Stack, C>` with different -#[derive(Debug)] -pub struct PayloadBuilderStack { - left: L, - right: R, -} - -impl PayloadBuilderStack { - /// Creates a new `PayloadBuilderStack` with the given left and right builders. - pub const fn new(left: L, right: R) -> Self { - Self { left, right } - } -} - -impl Clone for PayloadBuilderStack -where - L: Clone, - R: Clone, -{ - fn clone(&self) -> Self { - Self::new(self.left.clone(), self.right.clone()) - } -} - -impl BuiltPayload for Either -where - L: BuiltPayload, - R: BuiltPayload, -{ - fn block(&self) -> &SealedBlock { - match self { - Either::Left(l) => l.block(), - Either::Right(r) => r.block(), - } - } - - fn fees(&self) -> U256 { - match self { - Either::Left(l) => l.fees(), - Either::Right(r) => r.fees(), - } - } -} - -impl BuildOutcome { - fn map_payload(self, f: F) -> BuildOutcome - where - F: FnOnce(B) -> B2, - { - match self { - BuildOutcome::Better { payload, cached_reads } => BuildOutcome::Better { - payload: f(payload), - cached_reads, - }, - BuildOutcome::Aborted { fees, cached_reads } => BuildOutcome::Aborted { fees, cached_reads }, - BuildOutcome::Cancelled => BuildOutcome::Cancelled, - } - } -} - -impl PayloadBuilder for PayloadBuilderStack -where - L: PayloadBuilder + Unpin + 'static, - R: PayloadBuilder + Unpin + 'static, - Client: Clone, - Pool: Clone, - L::Attributes: Unpin + Clone, - R::Attributes: Unpin + Clone, - L::BuiltPayload: Unpin + Clone, - R::BuiltPayload: Unpin + Clone, - <>::Attributes as PayloadBuilderAttributes>::Error: 'static, - <>::Attributes as PayloadBuilderAttributes>::Error: 'static, -{ - type Attributes = Either; - type BuiltPayload = Either; - - fn try_build( - &self, - args: BuildArguments, - ) -> Result, PayloadBuilderError> { - match args.config.attributes { - Either::Left(ref left_attr) => { - let left_args: BuildArguments = BuildArguments { - client: args.client.clone(), - pool: args.pool.clone(), - cached_reads: args.cached_reads.clone(), - config: PayloadConfig { - parent_block: args.config.parent_block.clone(), - extra_data: args.config.extra_data.clone(), - attributes: left_attr.clone(), - }, - cancel: args.cancel.clone(), - best_payload: args.best_payload.clone().and_then(|payload| { - if let Either::Left(p) = payload { - Some(p) - } else { - None - } - }), - }; - - match self.left.try_build(left_args) { - Ok(BuildOutcome::Better { payload, cached_reads }) => { - return Ok(BuildOutcome::Better { - payload: Either::Left(payload), - cached_reads, - }) - } - Ok(other) => { - return Ok(other.map_payload(Either::Left)) - } - Err(_) => { - } - } - } - Either::Right(ref right_attr) => { - let right_args = BuildArguments { - client: args.client.clone(), - pool: args.pool.clone(), - cached_reads: args.cached_reads.clone(), - config: PayloadConfig { - parent_block: args.config.parent_block.clone(), - extra_data: args.config.extra_data.clone(), - attributes: right_attr.clone(), - }, - cancel: args.cancel.clone(), - best_payload: args.best_payload.clone().and_then(|payload| { - if let Either::Right(p) = payload { - Some(p) - } else { - None - } - }), - }; - - match self.right.try_build(right_args) { - Ok(BuildOutcome::Better { payload, cached_reads }) => { - return Ok(BuildOutcome::Better { - payload: Either::Right(payload), - cached_reads, - }) - } - Ok(other) => { - return Ok(other.map_payload(Either::Right)) - } - Err(e) => { - return Err(e); - } - } - } - } - Err(PayloadBuilderError::Other(Box::new(std::io::Error::new( - std::io::ErrorKind::Other, - "Both left and right builders failed to build the payload" - )))) - } - - fn build_empty_payload( - &self, - client: &Client, - config: PayloadConfig, - ) -> Result { - match config.attributes { - Either::Left(left_attr) => { - let left_config = PayloadConfig { - attributes: left_attr, - parent_block: config.parent_block.clone(), - extra_data: config.extra_data.clone(), - }; - - match self.left.build_empty_payload(client, left_config) { - Ok(payload_left) => { - return Ok(Either::Left(payload_left)) - }, - Err(_) => {} - } - }, - Either::Right(right_attr) => { - let right_config = PayloadConfig { - parent_block: config.parent_block.clone(), - extra_data: config.extra_data.clone(), - attributes: right_attr.clone(), - }; - - match self.right.build_empty_payload(client, right_config) { - Ok(payload_right) => { - return Ok(Either::Right(payload_right)) - }, - Err(e) => { - return Err(e); - } - } - } - } - Err(PayloadBuilderError::Other(Box::new(std::io::Error::new( - std::io::ErrorKind::Other, - "Failed to build empty payload with both left and right builders" - )))) - } -} \ No newline at end of file diff --git a/crates/payload/basic/src/lib.rs b/crates/payload/basic/src/lib.rs index 8fa6bfcbbb41..0fc63a5a1495 100644 --- a/crates/payload/basic/src/lib.rs +++ b/crates/payload/basic/src/lib.rs @@ -42,9 +42,9 @@ use tokio::{ use tracing::{debug, trace, warn}; mod metrics; -mod builder_stack; +mod stack; -pub use builder_stack::PayloadBuilderStack; +pub use stack::PayloadBuilderStack; /// The [`PayloadJobGenerator`] that creates [`BasicPayloadJob`]s. #[derive(Debug)] @@ -786,6 +786,21 @@ impl BuildOutcome { pub const fn is_cancelled(&self) -> bool { matches!(self, Self::Cancelled) } + + /// Applies a fn on the current payload. + pub(crate) fn map_payload(self, f: F) -> BuildOutcome

+ where + F: FnOnce(Payload) -> P, + { + match self { + Self::Better { payload, cached_reads } => { + BuildOutcome::Better { payload: f(payload), cached_reads } + } + Self::Aborted { fees, cached_reads } => BuildOutcome::Aborted { fees, cached_reads }, + Self::Cancelled => BuildOutcome::Cancelled, + Self::Freeze(payload) => BuildOutcome::Freeze(f(payload)), + } + } } /// The possible outcomes of a payload building attempt without reused [`CachedReads`] diff --git a/crates/payload/basic/src/stack.rs b/crates/payload/basic/src/stack.rs new file mode 100644 index 000000000000..722399ab2781 --- /dev/null +++ b/crates/payload/basic/src/stack.rs @@ -0,0 +1,270 @@ +use crate::{ + BuildArguments, BuildOutcome, PayloadBuilder, PayloadBuilderAttributes, PayloadBuilderError, + PayloadConfig, +}; + +use alloy_primitives::{Address, B256, U256}; +use reth_payload_builder::PayloadId; +use reth_payload_primitives::BuiltPayload; +use reth_primitives::{SealedBlock, Withdrawals}; + +use alloy_eips::eip7685::Requests; +use std::{error::Error, fmt}; + +/// hand rolled Either enum to handle two builder types +#[derive(Debug, Clone)] +pub enum Either { + /// left variant + Left(L), + /// right variant + Right(R), +} + +impl fmt::Display for Either +where + L: fmt::Display, + R: fmt::Display, +{ + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Left(l) => write!(f, "Left: {}", l), + Self::Right(r) => write!(f, "Right: {}", r), + } + } +} + +impl Error for Either +where + L: Error + 'static, + R: Error + 'static, +{ + fn source(&self) -> Option<&(dyn Error + 'static)> { + match self { + Self::Left(l) => Some(l), + Self::Right(r) => Some(r), + } + } +} + +impl PayloadBuilderAttributes for Either +where + L: PayloadBuilderAttributes, + R: PayloadBuilderAttributes, + L::Error: Error + 'static, + R::Error: Error + 'static, +{ + type RpcPayloadAttributes = Either; + type Error = Either; + + fn try_new( + parent: B256, + rpc_payload_attributes: Self::RpcPayloadAttributes, + version: u8, + ) -> Result { + match rpc_payload_attributes { + Either::Left(attr) => { + L::try_new(parent, attr, version).map(Either::Left).map_err(Either::Left) + } + Either::Right(attr) => { + R::try_new(parent, attr, version).map(Either::Right).map_err(Either::Right) + } + } + } + + fn payload_id(&self) -> PayloadId { + match self { + Self::Left(l) => l.payload_id(), + Self::Right(r) => r.payload_id(), + } + } + + fn parent(&self) -> B256 { + match self { + Self::Left(l) => l.parent(), + Self::Right(r) => r.parent(), + } + } + + fn timestamp(&self) -> u64 { + match self { + Self::Left(l) => l.timestamp(), + Self::Right(r) => r.timestamp(), + } + } + + fn parent_beacon_block_root(&self) -> Option { + match self { + Self::Left(l) => l.parent_beacon_block_root(), + Self::Right(r) => r.parent_beacon_block_root(), + } + } + + fn suggested_fee_recipient(&self) -> Address { + match self { + Self::Left(l) => l.suggested_fee_recipient(), + Self::Right(r) => r.suggested_fee_recipient(), + } + } + + fn prev_randao(&self) -> B256 { + match self { + Self::Left(l) => l.prev_randao(), + Self::Right(r) => r.prev_randao(), + } + } + + fn withdrawals(&self) -> &Withdrawals { + match self { + Self::Left(l) => l.withdrawals(), + Self::Right(r) => r.withdrawals(), + } + } +} + +/// this structure enables the chaining of multiple `PayloadBuilder` implementations, +/// creating a hierarchical fallback system. It's designed to be nestable, allowing +/// for complex builder arrangements like `Stack, C>` with different +#[derive(Debug)] +pub struct PayloadBuilderStack { + left: L, + right: R, +} + +impl PayloadBuilderStack { + /// Creates a new `PayloadBuilderStack` with the given left and right builders. + pub const fn new(left: L, right: R) -> Self { + Self { left, right } + } +} + +impl Clone for PayloadBuilderStack +where + L: Clone, + R: Clone, +{ + fn clone(&self) -> Self { + Self::new(self.left.clone(), self.right.clone()) + } +} + +impl BuiltPayload for Either +where + L: BuiltPayload, + R: BuiltPayload, +{ + fn block(&self) -> &SealedBlock { + match self { + Self::Left(l) => l.block(), + Self::Right(r) => r.block(), + } + } + + fn fees(&self) -> U256 { + match self { + Self::Left(l) => l.fees(), + Self::Right(r) => r.fees(), + } + } + + fn requests(&self) -> Option { + match self { + Self::Left(l) => l.requests(), + Self::Right(r) => r.requests(), + } + } +} + +impl PayloadBuilder for PayloadBuilderStack +where + L: PayloadBuilder + Unpin + 'static, + R: PayloadBuilder + Unpin + 'static, + Client: Clone, + Pool: Clone, + L::Attributes: Unpin + Clone, + R::Attributes: Unpin + Clone, + L::BuiltPayload: Unpin + Clone, + R::BuiltPayload: Unpin + Clone, + <>::Attributes as PayloadBuilderAttributes>::Error: 'static, + <>::Attributes as PayloadBuilderAttributes>::Error: 'static, +{ + type Attributes = Either; + type BuiltPayload = Either; + + fn try_build( + &self, + args: BuildArguments, + ) -> Result, PayloadBuilderError> { + match args.config.attributes { + Either::Left(ref left_attr) => { + let left_args: BuildArguments = + BuildArguments { + client: args.client.clone(), + pool: args.pool.clone(), + cached_reads: args.cached_reads.clone(), + config: PayloadConfig { + parent_header: args.config.parent_header.clone(), + extra_data: args.config.extra_data.clone(), + attributes: left_attr.clone(), + }, + cancel: args.cancel.clone(), + best_payload: args.best_payload.clone().and_then(|payload| { + if let Either::Left(p) = payload { + Some(p) + } else { + None + } + }), + }; + + self.left.try_build(left_args).map(|out| out.map_payload(Either::Left)) + } + Either::Right(ref right_attr) => { + let right_args = BuildArguments { + client: args.client.clone(), + pool: args.pool.clone(), + cached_reads: args.cached_reads.clone(), + config: PayloadConfig { + parent_header: args.config.parent_header.clone(), + extra_data: args.config.extra_data.clone(), + attributes: right_attr.clone(), + }, + cancel: args.cancel.clone(), + best_payload: args.best_payload.clone().and_then(|payload| { + if let Either::Right(p) = payload { + Some(p) + } else { + None + } + }), + }; + + self.right.try_build(right_args).map(|out| out.map_payload(Either::Right)) + } + } + } + + fn build_empty_payload( + &self, + client: &Client, + config: PayloadConfig, + ) -> Result { + match config.attributes { + Either::Left(left_attr) => { + let left_config = PayloadConfig { + attributes: left_attr, + parent_header: config.parent_header.clone(), + extra_data: config.extra_data.clone(), + }; + self.left.build_empty_payload(client, left_config).map(Either::Left) + } + Either::Right(right_attr) => { + let right_config = PayloadConfig { + parent_header: config.parent_header.clone(), + extra_data: config.extra_data.clone(), + attributes: right_attr, + }; + self.right.build_empty_payload(client, right_config).map(Either::Right) + } + } + } +}