From 652bdaacd3080aeba7b80289fa7fe91fb49276e6 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Sun, 23 Apr 2023 22:21:33 +0200 Subject: [PATCH] feat: add empty payload building support (#2346) --- crates/payload/basic/src/lib.rs | 79 +++++++++++++++++++-- crates/payload/builder/src/service.rs | 14 ++-- crates/payload/builder/src/test_utils.rs | 6 +- crates/payload/builder/src/traits.rs | 9 ++- crates/rpc/rpc-engine-api/src/engine_api.rs | 14 ++-- crates/rpc/rpc-engine-api/src/error.rs | 4 ++ 6 files changed, 106 insertions(+), 20 deletions(-) diff --git a/crates/payload/basic/src/lib.rs b/crates/payload/basic/src/lib.rs index 59a5ca7ba..f10eadab6 100644 --- a/crates/payload/basic/src/lib.rs +++ b/crates/payload/basic/src/lib.rs @@ -15,7 +15,9 @@ use reth_payload_builder::{ }; use reth_primitives::{ bytes::{Bytes, BytesMut}, - constants::{RETH_CLIENT_VERSION, SLOT_DURATION}, + constants::{ + EMPTY_RECEIPTS, EMPTY_TRANSACTIONS, EMPTY_WITHDRAWALS, RETH_CLIENT_VERSION, SLOT_DURATION, + }, proofs, Block, BlockId, BlockNumberOrTag, ChainSpec, Header, IntoRecoveredTransaction, Receipt, SealedBlock, EMPTY_OMMER_ROOT, U256, }; @@ -333,9 +335,17 @@ where Pool: TransactionPool + Unpin + 'static, Tasks: TaskSpawner + Clone + 'static, { - fn best_payload(&self) -> Arc { - // TODO if still not set, initialize empty block - self.best_payload.clone().unwrap() + fn best_payload(&self) -> Result, PayloadBuilderError> { + if let Some(ref payload) = self.best_payload { + 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. + // + // Note: it is assumed that this is unlikely to happen, as the payload job is started right + // away and the first full block should have been built by the time CL is requesting the + // payload. + build_empty_payload(&self.client, self.config.clone()).map(Arc::new) } } @@ -357,6 +367,8 @@ impl Future for PendingPayload { } /// A marker that can be used to cancel a job. +/// +/// If dropped, it will set the `cancelled` flag to true. #[derive(Default, Clone)] struct Cancelled(Arc); @@ -575,6 +587,65 @@ fn build_payload( let _ = to_job.send(try_build(client, pool, config, cancel, best_payload)); } +/// Builds an empty payload without any transactions. +fn build_empty_payload( + client: &Client, + config: PayloadConfig, +) -> Result +where + Client: StateProviderFactory, +{ + // TODO this needs to access the _pending_ state of the parent block hash + let _state = client.latest()?; + + let PayloadConfig { + initialized_block_env, + parent_block, + extra_data, + attributes, + initialized_cfg, + .. + } = config; + + let base_fee = initialized_block_env.basefee.to::(); + let block_gas_limit: u64 = initialized_block_env.gas_limit.try_into().unwrap_or(u64::MAX); + + let mut withdrawals_root = None; + let mut withdrawals = None; + + if initialized_cfg.spec_id >= SpecId::SHANGHAI { + withdrawals_root = Some(EMPTY_WITHDRAWALS); + // set withdrawals + withdrawals = Some(attributes.withdrawals); + } + + let header = Header { + parent_hash: parent_block.hash, + ommers_hash: EMPTY_OMMER_ROOT, + beneficiary: initialized_block_env.coinbase, + // TODO compute state root + state_root: Default::default(), + transactions_root: EMPTY_TRANSACTIONS, + withdrawals_root, + receipts_root: EMPTY_RECEIPTS, + logs_bloom: Default::default(), + timestamp: attributes.timestamp, + mix_hash: attributes.prev_randao, + nonce: 0, + base_fee_per_gas: Some(base_fee), + number: parent_block.number + 1, + gas_limit: block_gas_limit, + difficulty: U256::ZERO, + gas_used: 0, + extra_data: extra_data.into(), + }; + + let block = Block { header, body: vec![], ommers: vec![], withdrawals }; + let sealed_block = block.seal_slow(); + + Ok(BuiltPayload::new(attributes.id, sealed_block, U256::ZERO)) +} + /// Checks if the new payload is better than the current best. /// /// This compares the total fees of the blocks, higher is better. diff --git a/crates/payload/builder/src/service.rs b/crates/payload/builder/src/service.rs index 17e3fcb21..9209260ef 100644 --- a/crates/payload/builder/src/service.rs +++ b/crates/payload/builder/src/service.rs @@ -29,7 +29,10 @@ pub struct PayloadStore { impl PayloadStore { /// Returns the best payload for the given identifier. - pub async fn get_payload(&self, id: PayloadId) -> Option> { + pub async fn get_payload( + &self, + id: PayloadId, + ) -> Option, PayloadBuilderError>> { self.inner.get_payload(id).await } } @@ -53,7 +56,10 @@ pub struct PayloadBuilderHandle { impl PayloadBuilderHandle { /// Returns the best payload for the given identifier. - pub async fn get_payload(&self, id: PayloadId) -> Option> { + pub async fn get_payload( + &self, + id: PayloadId, + ) -> Option, PayloadBuilderError>> { let (tx, rx) = oneshot::channel(); self.to_service.send(PayloadServiceCommand::GetPayload(id, tx)).ok()?; rx.await.ok()? @@ -136,7 +142,7 @@ where } /// Returns the best payload for the given identifier. - fn get_payload(&self, id: PayloadId) -> Option> { + fn get_payload(&self, id: PayloadId) -> Option, PayloadBuilderError>> { self.payload_jobs.iter().find(|(_, job_id)| *job_id == id).map(|(j, _)| j.best_payload()) } } @@ -238,5 +244,5 @@ enum PayloadServiceCommand { oneshot::Sender>, ), /// Get the current payload. - GetPayload(PayloadId, oneshot::Sender>>), + GetPayload(PayloadId, oneshot::Sender, PayloadBuilderError>>>), } diff --git a/crates/payload/builder/src/test_utils.rs b/crates/payload/builder/src/test_utils.rs index 1b458c986..44cd797de 100644 --- a/crates/payload/builder/src/test_utils.rs +++ b/crates/payload/builder/src/test_utils.rs @@ -56,11 +56,11 @@ impl Stream for TestPayloadJob { } impl PayloadJob for TestPayloadJob { - fn best_payload(&self) -> Arc { - Arc::new(BuiltPayload::new( + fn best_payload(&self) -> Result, PayloadBuilderError> { + Ok(Arc::new(BuiltPayload::new( self.attr.payload_id(), Block::default().seal_slow(), U256::ZERO, - )) + ))) } } diff --git a/crates/payload/builder/src/traits.rs b/crates/payload/builder/src/traits.rs index d48c487e7..f64b540b2 100644 --- a/crates/payload/builder/src/traits.rs +++ b/crates/payload/builder/src/traits.rs @@ -9,9 +9,10 @@ use std::sync::Arc; /// /// This type is a Stream that yields better payloads. /// -/// Note: PaylodJob need to be cancel safe. +/// A PayloadJob must always be prepared to return the best payload built so far to make there's a +/// valid payload to deliver to the CL, so it does not miss a slot, even if the payload is empty. /// -/// TODO convert this into a future? +/// Note: A PayloadJob need to be cancel safe because it might be dropped after the CL has requested the payload via `engine_getPayloadV1`, See also pub trait PayloadJob: TryStream, Error = PayloadBuilderError> + Send + Sync { @@ -19,7 +20,7 @@ pub trait PayloadJob: /// /// Note: this is expected to be an empty block without transaction if nothing has been built /// yet. - fn best_payload(&self) -> Arc; + fn best_payload(&self) -> Result, PayloadBuilderError>; } /// A type that knows how to create new jobs for creating payloads. @@ -31,6 +32,8 @@ pub trait PayloadJobGenerator: Send + Sync { /// Creates the initial payload and a new [PayloadJob] that yields better payloads. /// + /// This is called when the CL requests a new payload job via a fork choice update. + /// /// Note: this is expected to build a new (empty) payload without transactions, so it can be /// returned directly. when asked for fn new_payload_job( diff --git a/crates/rpc/rpc-engine-api/src/engine_api.rs b/crates/rpc/rpc-engine-api/src/engine_api.rs index 5200e497c..543e120cb 100644 --- a/crates/rpc/rpc-engine-api/src/engine_api.rs +++ b/crates/rpc/rpc-engine-api/src/engine_api.rs @@ -125,11 +125,12 @@ where /// Note: /// > Client software MAY stop the corresponding build process after serving this call. pub async fn get_payload_v1(&self, payload_id: PayloadId) -> EngineApiResult { - self.payload_store + Ok(self + .payload_store .get_payload(payload_id) .await - .map(|payload| (*payload).clone().into_v1_payload()) - .ok_or(EngineApiError::UnknownPayload) + .ok_or(EngineApiError::UnknownPayload)? + .map(|payload| (*payload).clone().into_v1_payload())?) } /// Returns the most recent version of the payload that is available in the corresponding @@ -143,11 +144,12 @@ where &self, payload_id: PayloadId, ) -> EngineApiResult { - self.payload_store + Ok(self + .payload_store .get_payload(payload_id) .await - .map(|payload| (*payload).clone().into_v2_payload()) - .ok_or(EngineApiError::UnknownPayload) + .ok_or(EngineApiError::UnknownPayload)? + .map(|payload| (*payload).clone().into_v2_payload())?) } /// Called to retrieve execution payload bodies by range. diff --git a/crates/rpc/rpc-engine-api/src/error.rs b/crates/rpc/rpc-engine-api/src/error.rs index 323e40539..1f4fa4b3c 100644 --- a/crates/rpc/rpc-engine-api/src/error.rs +++ b/crates/rpc/rpc-engine-api/src/error.rs @@ -1,5 +1,6 @@ use jsonrpsee_types::error::{INTERNAL_ERROR_CODE, INVALID_PARAMS_CODE}; use reth_beacon_consensus::{BeaconEngineError, BeaconForkChoiceUpdateError}; +use reth_payload_builder::error::PayloadBuilderError; use reth_primitives::{H256, U256}; use thiserror::Error; use tokio::sync::{mpsc, oneshot}; @@ -75,6 +76,9 @@ pub enum EngineApiError { /// Failed to send message due ot closed channel #[error("Closed channel")] ChannelClosed, + /// Fetching the payload failed + #[error(transparent)] + GetPayloadError(#[from] PayloadBuilderError), } impl From> for EngineApiError {