feat: bridgeservicefinder: add IgnoreNetworkIDs to skip dead networks 1809 - #1810
Open
joanestebanr wants to merge 8 commits into
Open
feat: bridgeservicefinder: add IgnoreNetworkIDs to skip dead networks 1809#1810joanestebanr wants to merge 8 commits into
joanestebanr wants to merge 8 commits into
Conversation
Config.IgnoreNetworkIDs lists networkIDs (rollupIDs) that are excluded entirely from on-chain resolution: buildInitialCache skips them during enumeration (no RollupIDToRollupData call, no contract reads, no health probe) and the live listener skips them when a rollup-manager lifecycle event announces them (CreateNewRollup/CreateNewAggchain/AddExistingRollup). This avoids known-dead networks (decommissioned or unreachable test rollups) slowing down proxy startup and event processing with on-chain reads and health-check timeouts that can never succeed. A static Config.BridgeURLs override for a networkID is still served even if that networkID is also listed in IgnoreNetworkIDs. Fixes #1809
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 209b94fb90
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+244
to
+246
| if _, ignored := f.ignoreNetworkIDs[rollupID]; ignored { | ||
| f.logger.Infof("network %d is in IgnoreNetworkIDs, skipping on-chain resolution", rollupID) | ||
| continue |
There was a problem hiding this comment.
When a network appears in both IgnoreNetworkIDs and BridgeURLs, the earlier config-seeding loop still places it in the cache, and Start subsequently calls probeAll, which probes every cached entry. An unreachable ignored override therefore still incurs the configured health-check timeout and, with RequireAllHealthyOnStart=true, makes startup fail with ErrServicesUnhealthyOnStart, contrary to the new option's promise to avoid health probes for ignored networks while continuing to serve their static overrides. The startup probing step needs to exclude ignored IDs.
Useful? React with 👍 / 👎.
Collaborator
Author
There was a problem hiding this comment.
Good catch — fixed in 769721c: probeAll now skips any networkID in IgnoreNetworkIDs, so a config-overridden-but-ignored network's cache entry is served but never health-probed (never counted toward RequireAllHealthyOnStart either). Added TestStart_IgnoreNetworkIDs_SkipsHealthProbe to cover it.
Documents the new bridgeservicefinder.Config.IgnoreNetworkIDs field in the proxy binary's default TOML template. No other doc location lists BridgeServiceFinder fields individually for the proxy binary (only docs/autoclaim.md has a per-field table, already updated for AutoClaim).
…alth probe probeAll iterated every cache entry unconditionally, including one installed by the config-seeding step for a networkID that is both config-overridden and listed in IgnoreNetworkIDs. That defeated the point of ignoring a known-dead network: its /health probe still incurred the configured timeout, and under RequireAllHealthyOnStart=true an unreachable ignored override could still fail Start with ErrServicesUnhealthyOnStart. probeAll now skips any networkID in Config.IgnoreNetworkIDs; its entry is still served by GetURL, with healthy defaulting to false (never probed). Found by review on #1810.
taylanpince
previously approved these changes
Aug 21, 2026
Add the missing BridgeServiceFinder fields (BlockFinality, BlockChunkSize, HealthCheckPath, HealthCheckTimeout, RequireAllHealthyOnStart, IgnoreNetworkIDs) and the empty BridgeURLs/RPCURLs map sections to config/default.go, matching the values already used in proxy/config/default.go and the Default* constants defined in bridgeservicefinder/config.go. Also add the missing BridgeURLs/RPCURLs sections to proxy/config/default.go for the same reason. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…l-clock op-pp's L1 image (arnaubennassar/geth:op-pp) has its chain data baked in at build time and never advances past that snapshot. The op-geth entrypoint was patching the L2 genesis timestamp to date +%s (real wall-clock) on every start, so the gap between the L2 genesis and its L1 origin block grows by one day for every day that passes since the L1 image was built. Once that drift exceeded rollup.json's max_sequencer_drift (600s), op-node's sequencer could never find a valid L1 origin for the first post-genesis block and the L2 chain stalled forever at block 0 -- surfacing as "wait for MintableERC20 deployment: context deadline exceeded" during LoadEnv, since op-pp's L1 snapshot is from Feb 2026 (~6 months of drift by now). Fix: read L1's actual head timestamp and use it to patch the L2 genesis instead of wall-clock time, keeping L2 genesis anchored to L1's frozen origin regardless of what day the test actually runs. Verified locally: op-geth-001/op-node-001 went from stuck at block 0 to actively sequencing new L2 blocks. Note: since L1 never advances, the chain still stalls again once L2's virtual time drifts past max_sequencer_drift from the anchored origin (~1800s of L2 time in local testing) -- well past LoadEnv/MintableERC20 deployment, but a longer-running test could still hit it. Left as a known follow-up rather than widening scope here.
L1's chain data is baked into its image at build time and never advances past block 384. Anchoring L2 genesis to L1's head (previous commit) fixes LoadEnv, but once the sequencer has produced ~600s (max_sequencer_drift) worth of L2 blocks since genesis, op-node's origin-selector needs a newer L1 origin than block 384 to keep going and never finds one, stalling the chain forever mid-test-run. Raise max_sequencer_drift to a week so the sequencer never needs to look for a newer L1 origin within the lifetime of a test run. Found while investigating CI failures on #1810.
…r ~30min" This reverts commit bf18a77.
…d of wall-clock" This reverts commit e01e556.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔄 Changes Summary
Config.IgnoreNetworkIDs []uint32tobridgeservicefinder: networkIDs (rollupIDs) listed here are excluded entirely from on-chain resolution.buildInitialCacheskips them during enumeration (noRollupIDToRollupDatacall, no contract reads, no health probe).CreateNewRollup/CreateNewAggchain/AddExistingRollup) announces them, so they are never added to the watched-address set either.Config.BridgeURLsoverride for a networkID is still served even if that networkID is also listed inIgnoreNetworkIDs— the ignore only skips on-chain inspection, never a config override.proxybinary's default TOML config template.IgnoreNetworkIDsdefaults to an empty list, so existing deployments are unaffected.📋 Config Updates
🧾 New optional field
IgnoreNetworkIDsonBridgeServiceFinder, shared by theautoclaimandproxybinaries since both reuse the samebridgeservicefinderpackage. Defaults to[](nothing ignored). Documented indocs/autoclaim.md's config table and added (empty) to theproxydefault config template.AutoClaim (
AutoClaim.BridgeServiceFindersection):Proxy (
[BridgeServiceFinder]section):IgnoreNetworkIDstakes a list of the dead networks' networkIDs (rollupIDs) — e.g.[5, 12]above skips networks5and12— and can be pasted directly under the existingBridgeServiceFinderblock in either config, replacing the defaultIgnoreNetworkIDs = [].✅ Testing
TestStart_IgnoreNetworkIDs_SkipsEnumerationandTestLiveDiscovery_IgnoredNetworkIsNeverRegisteredinbridgeservicefinder; fullbridgeservicefinderandproxy/configsuites pass;golangci-lintclean;go build ./...passes.🐞 Issues
🔗 Related PRs
📝 Notes
buildInitialCache(which installsBridgeURLs/RPCURLsoverrides) runs independently and is unaffected, so an ignored+config-overridden network is still served.BridgeServiceFinderfields individually (onlydocs/autoclaim.mdhas a per-field table, since AutoClaim reuses the same package), so no further doc update was needed there.