feat(intercept): add bridge intercept in op block builder

[JimmyShi22]

No description provided.

[Vui-Chee]

Code Review

Summary: This PR wires bridge intercept into the non-flashblocks payload builder path by introducing XLayerBlockBuilder (wraps any BlockBuilder to intercept at the per-tx execution level) and XLayerPayloadBuilder (wraps OpPayloadBuilder and drives XLayerBlockBuilder). The design is clean, well-documented, and the intercept crate has solid test coverage.

Verdict: Request Changes — one formatting issue will fail cargo fmt --check in CI, and a couple of minor observability/consistency gaps are worth fixing before merge.

[Vui-Chee]

🟡 Missing builder method for consistency — XLayerPayloadBuilderConfig exposes with_da_config and with_gas_limit_config as chained builder methods, but bridge_config is only settable via direct field mutation (xlayer_config.bridge_config = config in with_bridge_config). Add a builder method to keep the API consistent:

/// Set the bridge intercept config.
pub fn with_bridge_config(mut self, bridge_config: BridgeInterceptConfig) -> Self {
    self.bridge_config = bridge_config;
    self
}

This also lets the field become private in the future without a breaking change.

[Vui-Chee]

🔵 Observability gap — intercepted transactions are silently removed from the pool. Add a log so operators can see which transactions were blocked and why (useful when debugging why a transaction never gets included):

let intercepted = builder.take_intercepted_hashes();
if !intercepted.is_empty() {
    warn!(
        target: "xlayer::payload_builder",
        count = intercepted.len(),
        hashes = ?intercepted,
        "removing intercepted bridge transactions from pool"
    );
    self.inner.pool.remove_transactions_and_descendants(intercepted);
}

[sieniven]

Maybe we can leave to adding the intercept logic just on flashblocks builder for now. We could just cherry-pick the old intercept PR into okx/optimism op-reth crates if needed too in the future.

[JimmyShi22]

Converted to draft now

[Vui-Chee]

Why intercept after execution of tx?

[Vui-Chee]

@claude review the architecture

[Vui-Chee]

@ci

[Vui-Chee]

⏳ Claude is processing your request...

[Vui-Chee]

@ci

[Vui-Chee]

⏳ Claude is processing your request...

[Vui-Chee]

Formatting check FAILED.

---

Results

Detected stacks: Rust

Command run: cargo fmt --all -- --check

Exit code: 1

Formatting issues found in:

1. bin/node/src/payload.rs — multiple formatting diffs:

   • Lines 86–90: ExecutionResult<...> generic should be collapsed to one line
   • Lines 97–120: method call + closure should be reformatted (line-wrapping style)
   • Lines 254–258 and 291–295: BestTxs: PayloadTransactions<...> bound reformatting
   • Line 412: trailing blank line before } should be removed

2. crates/intercept/src/lib.rs — line 1: use tracing::{debug}; should be use tracing::debug;

Stopping as required. No further checks were run.

[Vui-Chee]

@claude pls review again

[XLayer-Bot]

⏳ Robot is processing your request...

[XLayer-Bot]

---

Summary: This PR extends bridge transaction interception to the non-flashblocks (default) payload builder path. Previously, with_bridge_config() silently dropped the config when not using flashblocks. The fix introduces three new types: XLayerBlockBuilder (wraps BlockBuilder to intercept individual txs), XLayerPayloadBuilder (wraps OpPayloadBuilder), and XLayerPayloadBuilderConfig (constructs the wrapped builder via PayloadBuilderBuilder).

---

Issues:

🟡 Warning — xlayer_build_payload / xlayer_build duplicate the upstream OpPayloadBuilder build flow

xlayer_build_payload and xlayer_build (~100 lines) are manually mirrored from upstream's OpPayloadBuilder::build_payload / OpBuilder::build. If upstream changes the build flow (new execution steps, L1_BLOCK_CONTRACT handling, cached-reads strategy, etc.) this code will silently diverge. A comment referencing the upstream source and version would at least make staleness detectable:

// Mirrors OpPayloadBuilder::build_payload / OpBuilder::build from
// reth-optimism v<VERSION>. Keep in sync when upgrading reth.

🟡 Warning — no tests for XLayerBlockBuilder

The xlayer_bridge_intercept crate has thorough tests for the intercept logic itself, but XLayerBlockBuilder::execute_transaction_with_commit_condition — the wiring that connects the intercept to the builder and records hashes — has no coverage. A unit test with a minimal MockBlockBuilder would verify:

• intercepted tx hash is recorded and CommitChanges::No is returned
• non-intercepted tx is transparent (same result as inner builder)
• upstream f returning CommitChanges::No does NOT record the hash (important: line if commit == CommitChanges::No { return CommitChanges::No; } runs before the intercept check; this correct short-circuit has no test)

🟡 Warning — thiserror 1.x → 2.x major version bump in Cargo.lock

-"thiserror 1.0.69",
+"thiserror 2.0.18",

This is a semver-major bump in the xlayer-bridge-intercept crate. The 2.x API is source-compatible for basic #[derive(Error)] usage, but this should be an intentional workspace-level upgrade, not a side effect of this PR. Confirm the workspace thiserror version is already 2.x and this is just reflecting that.

🔵 Suggestion — inconsistent builder API on XLayerPayloadBuilderConfig

XLayerPayloadBuilderConfig has with_da_config() and with_gas_limit_config() builder methods, but bridge_config is set via direct field access from outside:

// in with_bridge_config():
xlayer_config.bridge_config = config;

For consistency, consider adding:

pub fn with_bridge_config(mut self, bridge_config: BridgeInterceptConfig) -> Self {
    self.bridge_config = bridge_config;
    self
}

🔵 Suggestion — trailing blank line before } in impl block (bin/node/src/payload.rs, around line ~400)

    pub fn with_gas_limit_config(mut self, gas_limit_config: OpGasLimitConfig) -> Self {
        self.gas_limit_config = gas_limit_config;
        self
    }
                         // ← extra blank line here
}

Minor, but rustfmt typically removes this; worth running just fix-format before merging.

🔵 Suggestion — sequencer tx intercept hashes are silently discarded when no_tx_pool = true

Sequencer transactions also flow through XLayerBlockBuilder, so if a sequencer tx triggers the bridge intercept, its hash is added to intercepted_hashes. However, take_intercepted_hashes() is only called inside the if !ctx.attributes().no_tx_pool() branch, so those hashes are never drained. This is functionally harmless (sequencer txs aren't in the pool), but worth a comment to prevent future confusion:

// Sequencer txs are not in the mempool, so intercepted hashes from
// execute_sequencer_transactions need not be drained here.

---

Positives:

• The intercepted flag capture pattern is correct — it only sets true when the outer f first returns CommitChanges::Yes and then the bridge check fires, meaning spurious hash recording for upstream-rejected txs is correctly avoided.
• XLayerBlockBuilder cleanly delegates all BlockBuilder trait methods to inner, so evm_mut() (a provided method on the trait calling executor_mut().evm_mut()) works transparently without extra boilerplate.
• The with_bridge_config method now correctly applies the config to both paths via match instead of a silent if let that dropped it for the default path — this was the core bug.
• The intercept crate itself (crates/intercept/src/lib.rs) has excellent test coverage with real mainnet data.

---

Verdict: Request Changes — the duplicate build flow needs a tracking comment, and the XLayerBlockBuilder wiring needs at least basic test coverage before merging.