-
Notifications
You must be signed in to change notification settings - Fork 66
Add engine_api v4 support #159
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
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
Closes #156 |
src/server.rs
Outdated
debug!(message = "execution mode is disabled, skipping FCU call to builder", "head_block_hash" = %fork_choice_state.head_block_hash); | ||
} else if should_send_to_builder { | ||
let builder_client = self.builder_client.clone(); | ||
println!("print payload attributes: {:?}", payload_attributes); |
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.
debugging log?
Can we update the |
|
||
let l2_req = HttpRequest::from_parts(parts, HttpBody::from(body_bytes)); | ||
info!(target: "proxy::call", message = "proxying request to rollup-boost server", ?method); | ||
info!(target: "proxy::call", message = "forward request to rollup-boost server", ?method); |
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 think it would make sense to prefix our targets with rollup_boost
since our OTLP filter is on rollup_boost
targets.
Line 112 in eca9266
.with_target(&filter_name, LevelFilter::TRACE); |
Not really relevant to this PR, but I think it would make sense to set rollup_boost
as the target on all traces
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.
src/integration/service_reth.rs
Outdated
.arg("never") | ||
.arg("--ipcdisable"); | ||
.arg("--ipcdisable") | ||
.arg("-vvvvv"); |
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.
do we want to enable tracing here on reth? seems it would be too verbose
mint: None, | ||
value: U256::default(), | ||
gas_limit: 210000, | ||
is_system_transaction: true, |
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.
does this deposit tx work? had to set is_system_transaction: false for it to work in op-rbuilder
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.
Our integration tests are running in an old (two weeks) version of Reth
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.
LGTM, nice work on this!
Main scaffolding is done, tests blocked by paradigmxyz/reth#15435
Builder-playground test
Build local docker image:
Clone builder-playground at run the op-stack with the new image:
Testing
We cannot do integration tests yet because there is not a nightly release available yet for
op-reth
. But, in the Kurtosis setup I am enabling theIshtmus
andPrague
fork after 2 blocks.