Skip to content

op-rbuilder: build the pool from OpNode::standard_pool_builder - #22118

Open
op-will wants to merge 1 commit into
developfrom
refactor/op-rbuilder-standard-pool-builder
Open

op-rbuilder: build the pool from OpNode::standard_pool_builder#22118
op-will wants to merge 1 commit into
developfrom
refactor/op-rbuilder-standard-pool-builder

Conversation

@op-will

@op-will op-will commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

What

Both op-rbuilder pool construction sites now start from OpNode::standard_pool_builder instead of hand-copying the base pool configuration.

Why

standard_pool_builder was extracted (#21688) precisely so downstream pool builders could compose the base configuration — tx-conditional support, interop endpoints and quorum, the shared failsafe gate — rather than duplicate it. op-rbuilder still duplicates it in two places:

  • launcher.rs (production)
  • tests/framework/instance.rs (pool_component)

Hand-copying means a field added to the base upstream is silently dropped here: the pool keeps compiling, just with the old configuration. That is the failure mode the extraction exists to prevent, and op-rbuilder is currently the only in-repo instance of it.

How

Start from standard_pool_builder and override only what genuinely differs:

  • enable_tx_conditional — op-rbuilder additionally enables it for revert protection, which reuses the same pool bookkeeping.
  • interop_failsafe — load-bearing, not cosmetic. BuilderConfig mints its own handle rather than adopting the node's, and the payload builder reads the failsafe through BuilderConfig. So the pool has to be pointed back at that handle; taking the node's (as the base does) would leave the filter client writing one flag while the builder reads another. A comment records this at both sites.

with_interop no longer needs restating — the base already sets it from the same rollup_args.

No behavior change: every value the previous code set is still set.

Verification

  • cargo clippy -p op-rbuilder --all-targets — clean; zero warnings in either changed file. (The workspace has 20 pre-existing lints in unrelated test files; this PR neither adds nor fixes them.)
  • cargo +nightly fmt --check clean.
  • Test targets compile, which covers the pool_component signature change. The devnet-backed tests need Docker and are left to CI.

Follow-up worth considering (not in this PR)

That BuilderConfig mints its own interop_failsafe instead of adopting the node's is a latent footgun — it is why the override above is required, and why a naive swap to standard_pool_builder would silently misconfigure the pool. Having BuilderConfig adopt the node's handle would remove the special case, but that is a behavior change and does not belong in a no-op refactor.

🤖 Generated with Claude Code

Both pool construction sites hand-copied the base pool configuration
(tx-conditional, interop endpoints and quorum) instead of starting from
`OpNode::standard_pool_builder`, which exists to be composed for exactly
this reason. Hand-copying means a base field added upstream is silently
dropped here — the pool keeps compiling with the old configuration.

Start from `standard_pool_builder` and override only what differs:
`enable_tx_conditional`, which additionally honors revert protection, and
the interop failsafe.

The failsafe override is load-bearing, not cosmetic. `BuilderConfig`
mints its own handle rather than adopting the node's, and the payload
builder reads the failsafe through `BuilderConfig` — so the pool must be
pointed back at that handle or the filter client and the builder would
observe different flags. A comment records this at both sites.

No behavior change: every value the previous code set is still set.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@op-will op-will changed the title refactor(op-rbuilder): build the pool from OpNode::standard_pool_builder op-rbuilder: build the pool from OpNode::standard_pool_builder Jul 29, 2026
@op-will
op-will force-pushed the refactor/op-rbuilder-standard-pool-builder branch from 61f3377 to 66772ad Compare July 29, 2026 22:37
@op-will
op-will marked this pull request as ready for review July 30, 2026 15:32
@op-will
op-will requested a review from a team as a code owner July 30, 2026 15:32
@op-will op-will self-assigned this Jul 30, 2026
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.

1 participant