-
Notifications
You must be signed in to change notification settings - Fork 0
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 driver #12
Conversation
|
||
/// The payload attributes for block building tailored for Scroll. | ||
#[derive(Debug, Clone, Default, Serialize, Deserialize)] | ||
pub struct ScrollPayloadAttributes { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will move this to scroll-alloy-rpc-types-engine
once the alloy
PR is merge on Reth.
crates/engine/src/payload.rs
Outdated
/// - all transactions match | ||
/// - timestamps are equal | ||
/// - `prev_randaos` are equal | ||
/// - TODO: should we also compare the `fee_recipient` with the `suggested_fee_recipient`? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this header.coinbase
? It is currently used by Clique.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Added a few comments inline.
crates/engine/src/engine.rs
Outdated
safe_head: BlockInfo, | ||
finalized_head: BlockInfo, | ||
) -> Self { | ||
let fcu = ForkchoiceState { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let fcu = ForkchoiceState { | |
let fcs = ForkchoiceState { |
crates/engine/src/engine.rs
Outdated
loop { | ||
match client.fork_choice_updated_v1(fcu, None).await { | ||
Err(err) => { | ||
debug!(target: "engine::driver", ?err, "waiting on engine client"); | ||
tokio::time::sleep(tokio::time::Duration::from_secs(1)).await; | ||
} | ||
Ok(status) => { | ||
info!(target: "engine::driver", payload_status = ?status.payload_status.status, "engine ready"); | ||
break | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
loop { | |
match client.fork_choice_updated_v1(fcu, None).await { | |
Err(err) => { | |
debug!(target: "engine::driver", ?err, "waiting on engine client"); | |
tokio::time::sleep(tokio::time::Duration::from_secs(1)).await; | |
} | |
Ok(status) => { | |
info!(target: "engine::driver", payload_status = ?status.payload_status.status, "engine ready"); | |
break | |
} | |
} | |
} | |
// wait on engine | |
while let Err(err) = client.fork_choice_updated_v1(fcu, None).await { | |
debug!(target: "engine::driver", ?err, "waiting on engine client"); | |
tokio::time::sleep(tokio::time::Duration::from_secs(1)).await; | |
} | |
info!(target: "engine::driver", "engine ready"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to keep Ok(status)
to log it
/// The finalized L2 block info. | ||
finalized_block_info: BlockInfo, | ||
/// Marker | ||
_types: std::marker::PhantomData<ET>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just curious about why this variable need a _
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PhantomData
is a zero-sized type that is only there to give a hint about ET
to the compiler. So this field will never be accessed. The _
prefix just tells the compiler that this field is not used and prevents warnings (see here).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Left a single comment inline. Let me know what you think.
Add the engine driver component to the rollup node. The Engine driver is the interface to the Engine API of the EN and handles communicating new safe and unsafe payloads to the EN.
Resolves #5