fix: Remove engine api, add monitor handle and refactor full link monitor logic by sieniven · Pull Request #107 · okx/xlayer-reth

sieniven · 2026-01-21T10:40:09Z

Summary

This PR refactors and resolves the issues mentioned in PR #103.

Remove the custom engine-api handler
Better use of the engine consensus events and built payload stream to receive payload builder triggers / engine state updates
Remove option from the underlying OpEngineApi inner handle, adds the new custom XLayerEngineApiBuilder which explicitly ensures that an instance of OpEngineApi handle is constructed on compile time
Separate out full link custom monitoring logic into the separate xlayer-monitor crate, which contains the XLayerMonitor struct which encapsulates all full link monitoring logic
The XLayerMonitor struct exposes interface that (to be added in the future @LeoGuo621) encapsulates logic for full link monitoring requirements
Add the monitor handle, which is a worker thread for handling new engine events and payload events
Add flashblocks enabled flag

sieniven · 2026-01-22T11:07:20Z

crates/monitor/src/payload.rs

+        let provider = self.provider;

+        // Listen to both Attributes and BuiltPayload events in a single task
        task_executor.spawn_critical(


I think we should reduce the number of tasks being spawned for the monitor logic, into just a single task

Resolved in 3ea6d22

sieniven · 2026-01-22T11:09:12Z

bin/node/src/add_ons.rs

+    async fn launch_add_ons(self, ctx: AddOnsContext<'_, N>) -> eyre::Result<Self::Handle> {
+        // Initialize consensus engine events listener to track block received and canonical chain events
+        let consensus_listener = ConsensusListener::new(self.monitor.clone());
+        consensus_listener.listen(ctx.engine_events.new_listener(), ctx.node.task_executor());


Dont think we should spawn another worker task here, now we are essentially running 2 tasks for not much reason (engine and payload listeners).

Also, running the task here doesnt make sense since this is nested inside an add on wrapper struct? This logic doesnt quite fit launch_add_ons functionality and i suggest we completely do away with this design. we should put this instantiation of a single monitor handle task, that uses tokio runtime to async await both from the engine event and payload receivers.

Resolved in 3ea6d22

sieniven · 2026-01-22T11:09:53Z

bin/node/src/add_ons.rs

I dont really see a point in creating a custom struct for this. We just need the engine event receiver? Why create another nested level of custom add on definition here

We do not need to create a custom XLayerAddOns to retrieve the engine_event by wrapping the launch_add_ons. A more elegant solution is resolved in 3ea6d22, where we can obtain the engine event listener after the node is contructed inside the NodeHandle.add_ons_handle

sieniven · 2026-01-22T15:43:28Z

crates/monitor/src/monitor.rs


    /// Handle transaction commits to the canonical chain.
    pub fn on_tx_commit(&self, _block_info: &BlockInfo, _tx_hash: B256) {
+        info!(target: "xlayer::monitor", "monitor: Transaction committed to canonical chain");


Do not spam the node with so much info level loggings. Setting the target to xlayer::monitor is sufficient, and we should set these to debug level.

Resolved in 62bde76

sieniven · 2026-01-22T15:43:42Z

crates/monitor/src/monitor.rs

+    /// Handle block building start event (when payload attributes are received).
+    /// This is triggered when forkchoiceUpdated contains payload attributes.
+    pub fn on_block_build_start(&self, block_number: u64) {
+        info!(target: "xlayer::monitor", block_number, "monitor: Block building started");


Same for this.

Resolved in 62bde76

crates/monitor/src/monitor.rs

sieniven · 2026-01-22T15:58:04Z

crates/monitor/src/payload.rs

+                if let Ok(payload_events) = payload_builder.subscribe().await {
+                    // Use built_payload_stream to get accurate block numbers
+                    let mut built_payload_stream = payload_events.into_built_payload_stream();
+                    info!(target: "xlayer::monitor", "monitor: Payload events listener started");


Same here. Is there a need to even log this out also actually? Lets change all of these to debug level.

Resolved in 62bde76

sieniven · 2026-01-22T16:01:22Z

crates/monitor/src/consensus.rs

+        task_executor.spawn_critical(
+            "xlayer-monitor-consensus-events",
+            Box::pin(async move {
+                info!(target: "xlayer::monitor", "monitor: Consensus engine events listener started");


Note that you dont need to prefix your logs with a monitor: since we already set the target to xlayer::monitor. thats what the rust log targets help us with, together with allowing us to specify log target levels when running.

Resolved in 62bde76

sieniven · 2026-01-22T16:02:41Z

crates/monitor/src/payload.rs

+                                    Ok(None) => {
+                                        warn!(
+                                            target: "xlayer::monitor",
+                                            payload_id = %payload_id,
+                                            parent_hash = %parent_hash,
+                                            "monitor: Parent block not found for payload attributes"
+                                        );
+                                    }
+                                    Err(e) => {
+                                        warn!(
+                                            target: "xlayer::monitor",
+                                            payload_id = %payload_id,
+                                            parent_hash = %parent_hash,
+                                            error = %e,
+                                            "monitor: Failed to get parent block number"
+                                        );


Both are handled as logging, we can simply handle the match arm with _ =>

In addition I dont think we should spam the node logs here if somehow there is a state provider failure. We should set this to debug level - since if the parent hash cant be found, the engine state tree itself will already report error logs already.

Resolved in 62bde76

sieniven · 2026-01-22T16:05:25Z

crates/monitor/src/payload.rs

+                                        // Call on_block_build_start with the block number
+                                        monitor.on_block_build_start(block_number);
+
+                                        info!(


Why need to log again when monitor.on_block_build_start already adds a log?

Resolved in 62bde76

sieniven · 2026-01-22T16:06:07Z

crates/monitor/src/payload.rs

+                                // Call on_block_send_start with the accurate block number
+                                monitor.on_block_send_start(block_number);
+
+                                info!(


Same for this? on_block_send_start already adds a log?

Resolved in 62bde76

sieniven · 2026-01-22T18:21:26Z

crates/monitor/src/monitor.rs

+pub struct BlockInfo {
+    /// The block number
+    pub block_number: u64,
+    /// The block hash
+    pub block_hash: B256,
+}


Use alloy NumHash instead

Resolved in c31dd25

sieniven · 2026-01-22T18:26:57Z

crates/monitor/src/rpc.rs

+            trace!(
+                target: "xlayer::monitor::rpc",
+                "Transaction submission intercepted: method={}",
+                method_owned
+            );


trace log for transaction interception on the RPC layer is shifted inside the async task

crates/monitor/src/handle.rs

Vui-Chee · 2026-01-23T04:31:07Z

lgtm, I see these improvements:

no need for engine-api crate
simpler event handling logic for different sub mobules
remove generic from middleware rpc

* dev: fix: remove engine api, and simplify monitor logic (#107)

* upgrade to v1.10.0 (#98) * perf(builder): use XLayerPayloadServiceBuilder to simplify main codes (#95) Co-authored-by: Vui-Chee <vuicheesiew@gmail.com> Co-authored-by: Niven <sieniven@gmail.com> * drop unused dep * chore: better rename, fix unused variable warning (#99) * Better refactor, fix unused var warn * Better rename * Remove xlayer suffix * Align cargo chef base image with latest rust toolchain version (#100) * Update builder version (#101) * feat(full trace): Add full trace hook framework (#103) * add engine api hook for full trace * rm comment * fix clippy * add send tx hook * add blockchain tracer * cargo lock * fix review issues * simplify to tracer to avoid nasty type bonanza * renamed `TracerConfig` to `Tracer` * more renames * EngineApiTracer derive clone over manual impl * handle middleware overwrite problem * reorder * drop clone * no box on middleware rpc tracer * impl `EngineApiBuilder` directly to tracer --------- Co-authored-by: Vui-Chee <vuicheesiew@gmail.com> * fix: remove engine api, and simplify monitor logic (#107) * remove unused reth-node-api (#108) * fix: resolve audit issues related to flashblocks subscription (#113) * Fix unnecessary clone * Fix incomplete documentation of subscribe addrs filter * Better desc * chore(flashblocks): update builder version to v0.2.5 (#114) * Update builder version to v0.2.5 with full link monitor updates * Fix wspub init * feat: Add Transaction Trace Monitoring Support (#115) * add trace logic * optimize * fix ut * fix ut * fix * fix * optimize code * optimize code * fix * chore: fix rust formatter (#132) * Fix fmt * Remove unused dep * Fix nested if else clippy check * Update dependencies, use reth v1.10.2 and builder v0.3.0 (#133) * fix: upgrade reth v1.10.2 version with bug fixes (#134) * Fix revm dep * Update reth version, use merge fixes version * Add audit report (#135) * add audit report * add audit report --------- Co-authored-by: chunsheng.wang <chunsheng.wang@okg.com> * feat: add flashblocks enabled RPC method (#136) * feat: add flashblocks enabled RPC method * chore: clippy * chore: add unit tests * chore: reword documentation * remove min gas rpc (#138) --------- Co-authored-by: jimmyshi <417711026@qq.com> Co-authored-by: Niven <sieniven@gmail.com> Co-authored-by: Leo <335209779@qq.com> Co-authored-by: wangchunsheng-ops <13614269216@163.com> Co-authored-by: chunsheng.wang <chunsheng.wang@okg.com> Co-authored-by: lucas <66681646+limyeechern@users.noreply.github.com>

Refactor engine api, separate monitor crate

a4c0d3e

sieniven changed the title ~~Refactor engine api, separate monitor crate~~ fix: Refactor engine api, separate and better encapsulate full link monitor logic Jan 21, 2026

Fix args

0d288bf

sieniven mentioned this pull request Jan 22, 2026

perf(full trace): Use on_block_build_start hook instead of on_fork_choice_updated #106

Closed

JimmyShi22 added 2 commits January 22, 2026 18:32

use event instead of engine api middleware

6a15579

Add more even handle logic

e2d7983

sieniven commented Jan 22, 2026

View reviewed changes

crates/monitor/src/monitor.rs Show resolved Hide resolved

Fix logging, set to debug

62bde76

sieniven commented Jan 22, 2026

View reviewed changes

Fix to single handle task, use tokio select, fix issues

3ea6d22

sieniven force-pushed the niven/refactor-engine-api branch from 20c3e69 to 3ea6d22 Compare January 22, 2026 17:50

sieniven added 3 commits January 23, 2026 02:13

chore: inline refactor

55c1520

Fmt

f2584dd

Remove unnecessary comments

5bac536

sieniven commented Jan 22, 2026

View reviewed changes

Switch to alloy NumHash

c31dd25

sieniven commented Jan 22, 2026

View reviewed changes

sieniven changed the title ~~fix: Refactor engine api, separate and better encapsulate full link monitor logic~~ fix: Remove engine api, add monitor handle and better encapsulate full link monitor logic Jan 22, 2026

sieniven changed the title ~~fix: Remove engine api, add monitor handle and better encapsulate full link monitor logic~~ fix: Remove engine api, add monitor handle and refactor full link monitor logic Jan 22, 2026

sieniven added 2 commits January 23, 2026 10:50

Remove full link monitor arg, add flashblocks enabled flag to monitor

ebbab53

Add back monitor args

3c5d910

Fix trace only on monitor tx interception

bf435e9

LeoGuo621 reviewed Jan 23, 2026

View reviewed changes

crates/monitor/src/handle.rs Outdated Show resolved Hide resolved

Use block numhash on block build start

2051864

LeoGuo621 approved these changes Jan 23, 2026

View reviewed changes

Vui-Chee merged commit 58d4b6f into dev Jan 23, 2026

Vui-Chee added a commit that referenced this pull request Jan 23, 2026

Merge branch 'dev' into vui-chee/xlayer-node-lib

94ac305

* dev: fix: remove engine api, and simplify monitor logic (#107)

Conversation

sieniven commented Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sieniven Jan 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sieniven Jan 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Vui-Chee commented Jan 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sieniven commented Jan 21, 2026 •

edited

Loading

sieniven Jan 22, 2026 •

edited

Loading

sieniven Jan 22, 2026 •

edited

Loading