Skip to content

feat(intercept): add bridge intercept in flashblocks#177

Merged
louisliu2048 merged 9 commits intomainfrom
jimmyshi/bridge-intercept
Mar 17, 2026
Merged

feat(intercept): add bridge intercept in flashblocks#177
louisliu2048 merged 9 commits intomainfrom
jimmyshi/bridge-intercept

Conversation

@JimmyShi22
Copy link
Copy Markdown
Contributor

@JimmyShi22 JimmyShi22 commented Mar 10, 2026

Bridge Intercept Configuration Guide

The bridge intercept feature allows op-reth nodes to intercept and hold transactions that trigger BridgeEvent logs from a specified bridge contract. This is useful for controlling bridge operations on the L2 chain.

CLI Flags

Flag Required Description
--xlayer.intercept.enabled Yes Enables the intercept feature
--xlayer.intercept.bridge-contract <ADDRESS> Yes Address of the bridge contract to monitor
--xlayer.intercept.target-token <ADDRESS|*> Yes Token address to intercept, or * for all tokens

Interception Modes

Specific-token mode

Intercepts only when both the bridge contract and the token match:

--xlayer.intercept.enabled
--xlayer.intercept.bridge-contract  0x1234...abcd
--xlayer.intercept.target-token     0x5678...ef01

Wildcard mode

Intercepts all BridgeEvent logs from the bridge contract, regardless of token:

--xlayer.intercept.enabled
--xlayer.intercept.bridge-contract  0x1234...abcd
--xlayer.intercept.target-token     *

Interception Logic

A transaction is intercepted (held from inclusion in blocks) when all of the following are true:

  1. The intercept feature is enabled (--xlayer.intercept.enabled)
  2. A BridgeEvent(uint8,uint32,address,uint32,address,uint256,bytes,uint32) log is emitted by the configured bridge contract address
  3. The targetToken field (3rd parameter) in the event matches the configured --xlayer.intercept.target-token, or the target-token is set to *

Transactions that do not meet all three conditions are processed normally.

This pull request introduces a new bridge transaction interception feature to the flashblocks builder, allowing transactions that trigger certain bridge events to be blocked based on configurable rules. It adds a new internal crate, integrates its configuration and logic throughout the builder workflow, and provides comprehensive unit tests for the interception logic.

Bridge transaction interception integration:

  • Added the new xlayer-bridge-intercept internal crate to the workspace and as a dependency for the builder crate, with a new BridgeInterceptConfig struct for configuration. (Cargo.toml, crates/builder/Cargo.toml, crates/intercept/Cargo.toml) [1] [2] [3] [4]
  • Extended the FlashblocksBuilder, FlashblocksBuilderCtx, and related contexts to include bridge_intercept_config, ensuring the configuration is passed through all relevant construction paths. (crates/builder/src/flashblocks/builder.rs, crates/builder/src/flashblocks/context.rs, crates/builder/src/flashblocks/handler_ctx.rs) [1] [2] [3] [4] [5]

Builder logic changes:

  • Integrated bridge transaction interception into the transaction execution loop: transactions that trigger a blocked bridge event are now skipped and marked for removal from the pool. (crates/builder/src/flashblocks/context.rs)

Configuration and service updates:

  • Refactored FlashblocksServiceBuilder to accept and propagate bridge_intercept configuration, with a builder method for setting it, and updated all internal usage to the new config structure. (crates/builder/src/flashblocks/service.rs, crates/builder/src/tests/framework/instance.rs) [1] [2] [3] [4] [5] [6] [7] [8] [9] [10]

Testing:

  • Added comprehensive unit tests for bridge interception logic, covering wildcard and token-specific blocking, non-target tokens, and disabled config scenarios. (crates/builder/src/flashblocks/context.rs)

@JimmyShi22 JimmyShi22 requested a review from sieniven March 10, 2026 10:35
@okg-xqliu
Copy link
Copy Markdown

/claude

Copy link
Copy Markdown
Contributor

@sieniven sieniven left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review comments on struct design and validation duplication.

Comment thread crates/builder/src/flashblocks/service.rs Outdated
Comment thread bin/node/src/args.rs
`to_bridge_intercept_config()` now calls `validate()` first so all
validation lives in one place; `.expect("validated")` replaces the
redundant address-parse error handling after validation passes.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Comment thread bin/node/src/args.rs
Comment on lines +48 to +58
let bridge_contract_address = self
.bridge_contract
.as_deref()
.map(|a| a.parse::<Address>().expect("validated"))
.unwrap_or(Address::ZERO);

let (target_token_address, wildcard) = if let Some(token) = &self.target_token {
if token.is_empty() || token == "*" {
(Address::ZERO, true)
} else {
(token.parse::<Address>().expect("validated"), false)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 When enabled is false, validate() early-returns Ok(()) (line 84), skipping all address parsing checks. However, to_bridge_intercept_config() still calls .parse::<Address>().expect("validated") on lines 51 and 58, which will panic if an invalid address string was provided. Passing --xlayer.intercept.bridge-contract not-an-address without --xlayer.intercept.enabled crashes the node at startup. Fix: either validate address format regardless of the enabled flag, or replace .expect("validated") with fallible parsing (e.g., .ok() / ?).

Extended reasoning...

Bug Analysis

XLayerInterceptArgs::validate() (line 83) short-circuits when enabled is false — it returns Ok(()) at line 84-85 without checking whether bridge_contract or target_token contain valid Ethereum addresses. This is the only validation gate before to_bridge_intercept_config() attempts to parse those strings.

Code Path

to_bridge_intercept_config() calls self.validate()? at line 44, then unconditionally proceeds to parse self.bridge_contract at line 51 with .parse::<Address>().expect("validated") and self.target_token at line 58 with .parse::<Address>().expect("validated"). The .expect("validated") comment assumes that validate() already verified the format, but when enabled is false, validate() verified nothing.

Step-by-step proof

  1. User runs: xlayer-reth-node --xlayer.intercept.bridge-contract not-an-address (without --xlayer.intercept.enabled)
  2. enabled defaults to false, bridge_contract is Some("not-an-address")
  3. In main.rs, args.xlayer_args.intercept.to_bridge_intercept_config() is called unconditionally at startup
  4. to_bridge_intercept_config() calls self.validate()!self.enabled is true → returns Ok(())
  5. Execution reaches line 51: "not-an-address".parse::<Address>().expect("validated")panic
  6. The node crashes before it even starts

The same issue applies to target_token at line 58 — passing --xlayer.intercept.target-token garbage without enabling interception also panics.

Why existing tests miss this

The test test_parse_xlayer_intercept_disabled_with_params uses a valid address (0x2a3d...), so the parse succeeds even though validation was skipped. The test test_xlayer_intercept_to_config_disabled uses Default (all None), so there is nothing to parse. Neither test covers the case of an invalid address when disabled.

Impact

Any user who accidentally provides a malformed address to --xlayer.intercept.bridge-contract or --xlayer.intercept.target-token without also passing --xlayer.intercept.enabled will get an unrecoverable panic at startup. Since to_bridge_intercept_config() returns Result<_, String>, callers rightfully expect graceful error handling — a panic here violates the function contract.

Fix

Either (a) have validate() check address format even when enabled is false, or (b) replace .expect("validated") with .map_err(...) / ? so the function returns an Err instead of panicking. Option (b) is more defensive and aligns with the function already returning Result.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@claude is it a bug or nit?

Copy link
Copy Markdown

@claude claude Bot Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude finished @JimmyShi22's task in 43s —— View job


Analysis: Bug or Nit?

It's a real bug (though low-severity given typical usage).

Why it's a bug, not a nit

The issue is a panic on user-provided invalid input. Here's the exact path:

  1. User runs: --xlayer.intercept.bridge-contract not-an-address (without --xlayer.intercept.enabled)
  2. validate() hits line 84 → !self.enabled is true → returns Ok(()) immediately, skipping all address validation
  3. to_bridge_intercept_config() proceeds to line 51: "not-an-address".parse::<Address>().expect("validated")panic

The comment // After validation we know: if enabled, bridge_contract is Some and parseable is only true when enabled = true. When enabled = false, validation verified nothing, so the .expect("validated") assumption is wrong.

Why it matters

  • A panic at startup with user input is worse than returning an error — the function already returns Result<_, String> so callers expect graceful failures
  • This violates the function contract implied by its return type
  • The .expect("validated") comment actively misleads future maintainers

Severity

Low-medium — it only triggers when someone passes a malformed address (e.g., a typo in --xlayer.intercept.bridge-contract) without also passing --xlayer.intercept.enabled. Users who provide valid addresses or no addresses at all are unaffected.

Fix

Replace .expect("validated") with proper error handling:

let bridge_contract_address = self
    .bridge_contract
    .as_deref()
    .map(|a| a.parse::<Address>().map_err(|e| format!("Invalid bridge contract address: {e}")))
    .transpose()?
    .unwrap_or(Address::ZERO);

And similarly for target_token at line 58:

let addr = token.parse::<Address>()
    .map_err(|e| format!("Invalid target token address: {e}"))?;
(addr, false)

This way the function returns Err(...) instead of panicking, consistent with its return type. Fix this →

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@claude you are right, This function is only used for ut. Are there some ways that can define this functions only for ut? And move it to the place close to the tests

Comment thread bin/node/src/payload.rs
@Vui-Chee
Copy link
Copy Markdown
Contributor

@review

@JimmyShi22

This comment was marked as off-topic.

@claude

This comment was marked as off-topic.

Comment thread crates/builder/src/flashblocks/context.rs
@JimmyShi22 JimmyShi22 force-pushed the jimmyshi/bridge-intercept branch from 51d783a to 4dcae77 Compare March 17, 2026 07:13
@JimmyShi22
Copy link
Copy Markdown
Contributor Author

@review

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 17, 2026

Claude finished @JimmyShi22's task in 3m 12s —— View job


PR Review: feat(intercept): add bridge intercept in flashblocks

  • Read CLAUDE.md for repo guidelines
  • Gather context (read changed files)
  • Analyze code for bugs, security issues, performance problems
  • Post full review summary

Summary: This PR adds a new xlayer-bridge-intercept crate and integrates it into the flashblocks builder to block transactions that emit matching BridgeEvent logs from a configurable bridge contract. The interception runs post-EVM-execution by inspecting logs, supporting both wildcard (all tokens) and specific-token modes.


Issues

🔴 Bug — Panic on invalid address when interception is disabled (args.rs:51, 58)

validate() short-circuits at line 84 when enabled = false, skipping all address-format checks. However, to_bridge_intercept_config() then proceeds to call .expect("validated") on lines 51 and 58:

let bridge_contract_address = self
    .bridge_contract
    .as_deref()
    .map(|a| a.parse::<Address>().expect("validated"))  // ← panics if addr is garbage
    .unwrap_or(Address::ZERO);

Reproduction: xlayer-reth-node --xlayer.intercept.bridge-contract not-an-address (no --xlayer.intercept.enabled) → panic at startup.

to_bridge_intercept_config() is called unconditionally in main.rs:108–112, so this is a real startup-crash on user input. Was flagged by prior review comments but not yet fixed. Fix this →


🟡 Warning — with_bridge_config() silently drops config when flashblocks are disabled (payload.rs:66–71)

pub fn with_bridge_config(mut self, config: BridgeInterceptConfig) -> Self {
    if let XLayerPayloadServiceBuilderInner::Flashblocks(ref mut fb) = self.builder {
        fb.with_bridge_intercept(config);
    }
    // ← no else branch; config silently dropped if not flashblocks
    self
}

A user who enables --xlayer.intercept.enabled without --xlayer.builder.flashblocks.enabled will believe bridge interception is active when it is not. A previous claude commit (51d783a) claimed to have added a warning here, but that commit is not present in the current branch. The else if config.enabled { tracing::warn!(...) } branch needs to be re-applied. Fix this →


🟡 Warning — FlashblockHandlerContext silently bypasses bridge interception (handler_ctx.rs:92)

into_flashblocks_builder_ctx() hardcodes bridge_intercept_config: Default::default() (disabled), so any transaction execution that goes through the handler path (used by FlashblocksPayloadHandler) will not apply bridge interception.

If the handler only relays/verifies already-built payloads and never calls execute_best_transactions, this is fine — but it should be documented with a comment explaining why the default is intentional. As written it looks like an oversight.


🟡 Warning — bridge_intercept_config set via field mutation after construction (service.rs:147)

let mut payload_builder = FlashblocksBuilder::new(...);
payload_builder.bridge_intercept_config = self.bridge_intercept.clone();  // ← set after new()

FlashblocksBuilder::new() already initialises bridge_intercept_config: Default::default() at line 245, then it is immediately overwritten. This is fragile: adding a second constructor path without this line would silently produce a disabled config. Prefer passing the config through new() or using an explicit builder method. Fix this →


🔵 Suggestion — Duplicated test constants and data

crates/builder/src/flashblocks/context.rs (lines 694–712) duplicates the BRIDGE, TOKEN, OTHER, SENDER, DATA1 constants and make_bridge_log helper verbatim from crates/intercept/src/lib.rs tests. Consider exporting a #[cfg(test)] test-fixtures module from the intercept crate, or at minimum a shared test_utils module inside the builder crate.


🔵 Suggestion — Builder pattern inconsistency

FlashblocksServiceBuilder::with_bridge_intercept() is &mut self → &mut Self (mutating), while XLayerPayloadServiceBuilder::with_bridge_config() is self → Self (consuming). Pick one builder pattern to use consistently across this feature.


Positives

  • Clean crate design: xlayer-bridge-intercept has minimal deps (alloy-primitives, thiserror, tracing) and zero runtime overhead when disabled (early return Ok(()) guard).
  • Manual ABI decoding is documented with a slot-by-slot comment and validated against real mainnet data — this is a reliable approach for a fixed event signature.
  • Good test coverage: 14 unit tests covering wildcard, specific-token, wrong-contract, disabled, empty-logs, invalid-signature, and multi-log cases.
  • BridgeInterceptError uses thiserror with informative messages that include the token address, amount, and sender — great for diagnosing issues in logs.
  • Interception happens after EVM execution on the result logs, which correctly handles the indirect-call case (another contract calls the bridge) without needing to trace call trees.

Verdict: Request Changes — the panic-on-invalid-address bug and the missing warning log for the flashblocks-disabled case should be fixed before merge.

@github-actions
Copy link
Copy Markdown
Contributor

Code Review: feat(intercept): add bridge intercept in flashblocks

Summary: This PR adds a new xlayer-bridge-intercept crate and integrates it into the flashblocks builder to block transactions that emit matching BridgeEvent logs from a configurable bridge contract. The interception runs post-EVM-execution by inspecting logs, supporting both wildcard (all tokens) and specific-token modes.


Issues

🔴 Bug — Panic on invalid address when interception is disabled (args.rs:51, 58)

validate() short-circuits at line 84 when enabled = false, skipping all address-format checks. However, to_bridge_intercept_config() then calls .expect("validated") on lines 51 and 58:

let bridge_contract_address = self
    .bridge_contract
    .as_deref()
    .map(|a| a.parse::<Address>().expect("validated"))  // ← panics if addr is garbage
    .unwrap_or(Address::ZERO);

Reproduction: xlayer-reth-node --xlayer.intercept.bridge-contract not-an-address (no --xlayer.intercept.enabled) → panic at startup.

to_bridge_intercept_config() is called unconditionally in main.rs:108–112, so this is a real startup-crash on user input. Was flagged by prior review comments but the fix was not applied.


🟡 Warning — with_bridge_config() silently drops config when flashblocks are disabled (payload.rs:66–71)

pub fn with_bridge_config(mut self, config: BridgeInterceptConfig) -> Self {
    if let XLayerPayloadServiceBuilderInner::Flashblocks(ref mut fb) = self.builder {
        fb.with_bridge_intercept(config);
    }
    // ← no else branch; config silently dropped if not flashblocks
    self
}

A user who enables --xlayer.intercept.enabled without --xlayer.builder.flashblocks.enabled will believe bridge interception is active when it is not. A previous claude commit (51d783a) claimed to have added a warning here, but that commit is not present in the current branch. The else if config.enabled { tracing::warn!(...) } branch needs to be re-applied.


🟡 Warning — FlashblockHandlerContext silently bypasses bridge interception (handler_ctx.rs:92)

into_flashblocks_builder_ctx() hardcodes bridge_intercept_config: Default::default() (disabled), so any transaction execution through the handler path will not apply bridge interception. If this is intentional (handler only relays/verifies already-built payloads), it should be documented with a comment.


🟡 Warning — bridge_intercept_config set via field mutation after construction (service.rs:147)

let mut payload_builder = FlashblocksBuilder::new(...);
payload_builder.bridge_intercept_config = self.bridge_intercept.clone();  // ← set after new()

new() already initialises to Default::default() then it's immediately overwritten. Adding a second constructor path without this line would silently produce a disabled config. Prefer passing the config through new() directly.


🔵 Suggestion — Duplicated test constants

context.rs duplicates BRIDGE, TOKEN, OTHER, SENDER, DATA1, and make_bridge_log verbatim from intercept/src/lib.rs tests. Consider a shared test-fixtures module.


🔵 Suggestion — Builder pattern inconsistency

FlashblocksServiceBuilder::with_bridge_intercept() is &mut self → &mut Self (mutating), while XLayerPayloadServiceBuilder::with_bridge_config() is self → Self (consuming). Pick one pattern.


Positives

  • Clean crate design: minimal deps and zero overhead when disabled.
  • Manual ABI decoding documented with slot-by-slot comments and validated against real mainnet data.
  • Good test coverage: 14 unit tests covering wildcard, specific-token, wrong-contract, disabled, empty-logs, invalid-signature, and multi-log cases.
  • BridgeInterceptError includes token, amount, and sender for easy debugging.
  • Interception after EVM execution correctly handles indirect-call scenarios.

Verdict: Request Changes — the panic-on-invalid-address bug and missing flashblocks-disabled warning should be fixed before merge.

@louisliu2048 louisliu2048 merged commit f9f64e4 into main Mar 17, 2026
Vui-Chee added a commit that referenced this pull request Mar 17, 2026
* main:
  fix(claude): dup comments @review and unwarranted auth step trigger (#202)
  feat(intercept): add bridge intercept in flashblocks (#177)
  add github token to claude step (#201)
  restore id-token (#200)
  fix: resolve Node compile issue, use nightly formatting, pin Rust to 1.93, and fix Claude 401s (#198)
  feat: integrate legacy migrate tool into bin/tools crate (#196)
  refactor(node): clean up main.rs (#187)
  feat(ci): restrict to PR author as triggerman (#195)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants