Skip to content

Commit

Permalink
fix: always handle payload building for opstack (paradigmxyz#11629)
Browse files Browse the repository at this point in the history
  • Loading branch information
mattsse authored Oct 10, 2024
1 parent 58bfa60 commit eb9565d
Show file tree
Hide file tree
Showing 6 changed files with 56 additions and 6 deletions.
9 changes: 9 additions & 0 deletions crates/chainspec/src/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@ pub trait EthChainSpec: Send + Sync + Unpin + Debug {

/// The bootnodes for the chain, if any.
fn bootnodes(&self) -> Option<Vec<NodeRecord>>;

/// Returns `true` if this chain contains Optimism configuration.
fn is_optimism(&self) -> bool {
self.chain().is_optimism()
}
}

impl EthChainSpec for ChainSpec {
Expand Down Expand Up @@ -92,4 +97,8 @@ impl EthChainSpec for ChainSpec {
fn bootnodes(&self) -> Option<Vec<NodeRecord>> {
self.bootnodes()
}

fn is_optimism(&self) -> bool {
Self::is_optimism(self)
}
}
1 change: 1 addition & 0 deletions crates/engine/service/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ reth-prune.workspace = true
reth-stages-api.workspace = true
reth-tasks.workspace = true
reth-node-types.workspace = true
reth-chainspec.workspace = true

# async
futures.workspace = true
Expand Down
7 changes: 6 additions & 1 deletion crates/engine/service/src/service.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
use futures::{Stream, StreamExt};
use pin_project::pin_project;
use reth_beacon_consensus::{BeaconConsensusEngineEvent, BeaconEngineMessage, EngineNodeTypes};
use reth_chainspec::EthChainSpec;
use reth_consensus::Consensus;
use reth_engine_tree::{
backfill::PipelineSync,
download::BasicBlockDownloader,
engine::{EngineApiRequest, EngineApiRequestHandler, EngineHandler},
engine::{EngineApiKind, EngineApiRequest, EngineApiRequestHandler, EngineHandler},
persistence::PersistenceHandle,
tree::{EngineApiTreeHandler, InvalidBlockHook, TreeConfig},
};
Expand Down Expand Up @@ -79,6 +80,9 @@ where
invalid_block_hook: Box<dyn InvalidBlockHook>,
sync_metrics_tx: MetricEventsSender,
) -> Self {
let engine_kind =
if chain_spec.is_optimism() { EngineApiKind::OpStack } else { EngineApiKind::Ethereum };

let downloader = BasicBlockDownloader::new(client, consensus.clone());

let persistence_handle =
Expand All @@ -97,6 +101,7 @@ where
canonical_in_memory_state,
tree_config,
invalid_block_hook,
engine_kind,
);

let engine_handler = EngineApiRequestHandler::new(to_tree_tx, from_tree);
Expand Down
17 changes: 15 additions & 2 deletions crates/engine/tree/src/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,15 +212,28 @@ where
}
}

/// The type for specifying the kind of engine api
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
/// The type for specifying the kind of engine api.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)]
pub enum EngineApiKind {
/// The chain contains Ethereum configuration.
#[default]
Ethereum,
/// The chain contains Optimism configuration.
OpStack,
}

impl EngineApiKind {
/// Returns true if this is the ethereum variant
pub const fn is_ethereum(&self) -> bool {
matches!(self, Self::Ethereum)
}

/// Returns true if this is the ethereum variant
pub const fn is_opstack(&self) -> bool {
matches!(self, Self::OpStack)
}
}

/// The request variants that the engine API handler can receive.
#[derive(Debug)]
pub enum EngineApiRequest<T: EngineTypes> {
Expand Down
24 changes: 21 additions & 3 deletions crates/engine/tree/src/tree/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,10 @@ pub mod config;
mod invalid_block_hook;
mod metrics;
mod persistence_state;
use crate::{engine::EngineApiRequest, tree::metrics::EngineApiMetrics};
use crate::{
engine::{EngineApiKind, EngineApiRequest},
tree::metrics::EngineApiMetrics,
};
pub use config::TreeConfig;
pub use invalid_block_hook::{InvalidBlockHooks, NoopInvalidBlockHook};
pub use persistence_state::PersistenceState;
Expand Down Expand Up @@ -498,6 +501,8 @@ pub struct EngineApiTreeHandler<P, E, T: EngineTypes, Spec> {
metrics: EngineApiMetrics,
/// An invalid block hook.
invalid_block_hook: Box<dyn InvalidBlockHook>,
/// The engine API variant of this handler
engine_kind: EngineApiKind,
}

impl<P: Debug, E: Debug, T: EngineTypes + Debug, Spec: Debug> std::fmt::Debug
Expand All @@ -519,6 +524,7 @@ impl<P: Debug, E: Debug, T: EngineTypes + Debug, Spec: Debug> std::fmt::Debug
.field("config", &self.config)
.field("metrics", &self.metrics)
.field("invalid_block_hook", &format!("{:p}", self.invalid_block_hook))
.field("engine_kind", &self.engine_kind)
.finish()
}
}
Expand All @@ -545,6 +551,7 @@ where
persistence_state: PersistenceState,
payload_builder: PayloadBuilderHandle<T>,
config: TreeConfig,
engine_kind: EngineApiKind,
) -> Self {
let (incoming_tx, incoming) = std::sync::mpsc::channel();

Expand All @@ -565,6 +572,7 @@ where
metrics: Default::default(),
incoming_tx,
invalid_block_hook: Box::new(NoopInvalidBlockHook),
engine_kind,
}
}

Expand All @@ -589,6 +597,7 @@ where
canonical_in_memory_state: CanonicalInMemoryState,
config: TreeConfig,
invalid_block_hook: Box<dyn InvalidBlockHook>,
kind: EngineApiKind,
) -> (Sender<FromEngine<EngineApiRequest<T>>>, UnboundedReceiver<EngineApiEvent>) {
let best_block_number = provider.best_block_number().unwrap_or(0);
let header = provider.sealed_header(best_block_number).ok().flatten().unwrap_or_default();
Expand Down Expand Up @@ -618,6 +627,7 @@ where
persistence_state,
payload_builder,
config,
kind,
);
task.set_invalid_block_hook(invalid_block_hook);
let incoming = task.incoming_tx.clone();
Expand Down Expand Up @@ -1041,8 +1051,15 @@ where
if let Ok(Some(canonical_header)) = self.find_canonical_header(state.head_block_hash) {
debug!(target: "engine::tree", head = canonical_header.number, "fcu head block is already canonical");

// TODO(mattsse): for optimism we'd need to trigger a build job as well because on
// Optimism, the proposers are allowed to reorg their own chain at will.
// For OpStack the proposers are allowed to reorg their own chain at will, so we need to
// always trigger a new payload job if requested.
if self.engine_kind.is_opstack() {
if let Some(attr) = attrs {
debug!(target: "engine::tree", head = canonical_header.number, "handling payload attributes for canonical head");
let updated = self.process_payload_attributes(attr, &canonical_header, state);
return Ok(TreeOutcome::new(updated))
}
}

// 2. Client software MAY skip an update of the forkchoice state and MUST NOT begin a
// payload build process if `forkchoiceState.headBlockHash` references a `VALID`
Expand Down Expand Up @@ -2680,6 +2697,7 @@ mod tests {
PersistenceState::default(),
payload_builder,
TreeConfig::default(),
EngineApiKind::Ethereum,
);

let block_builder = TestBlockBuilder::default().with_chain_spec((*chain_spec).clone());
Expand Down
4 changes: 4 additions & 0 deletions crates/optimism/chainspec/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,10 @@ impl EthChainSpec for OpChainSpec {
fn bootnodes(&self) -> Option<Vec<NodeRecord>> {
self.inner.bootnodes()
}

fn is_optimism(&self) -> bool {
true
}
}

impl Hardforks for OpChainSpec {
Expand Down

0 comments on commit eb9565d

Please sign in to comment.