[joanestebanr]

🔄 Changes Summary

• The proxy and bridgetracker RPC clients now take the block hash from the RPC response (HashFromJSON = true) instead of recomputing it from the RLP header, as aggkit does by default.
• New [L2RPC] section in the proxy config: mode, retry policy, HashFromJSON and BatchBlockHeaderRetrieval for every L2 client. Each network's URL still comes from the bridge service finder (L2RPC.URL is ignored).
• sources.NewFinderClients now receives the RPC config and builds every dialed client from it (before, it used hardcoded code defaults, so HashFromJSON was false).
• proxy validates [L2RPC] at startup (only basic mode, valid retry policy).

⚠️ Breaking Changes

N/A

📋 Config Updates

• 🧾 Diff/Config snippet:

[L1RPC]
URL = "http://localhost:8545"
Mode = "basic"
RetryMode = "backoff"
MaxRetries = 5
InitialBackoff = "2s"
MaxBackoff = "10s"
BackoffMultiplier = 2.0
HashFromJSON = true
BatchBlockHeaderRetrieval = true

# new: applies to every L2 network resolved by the finder
[L2RPC]
Mode = "basic"
RetryMode = "backoff"
MaxRetries = 5
InitialBackoff = "2s"
MaxBackoff = "10s"
BackoffMultiplier = 2.0
HashFromJSON = true
BatchBlockHeaderRetrieval = true

🔌 API Updates

🔌 Bridge service API

• No changes.

🔌 Proxy API

• No changes.

🔌 Others API

• No changes.

✅ Testing

• 🤖 Automatic: unit tests for the new defaults (proxy/config), validateL2RPC (proxy/cmd) and that FinderClients dials with the provided config (bridgetracker/sources).
• 🖱️ Manual: run the proxy with [L2RPC] HashFromJSON = false/true and check the hash source.

🐞 Issues

• Closes feat: proxy: use eth client etherman/default_eth_client.go and CustomHeaderByNumber #1896

🔗 Related PRs

• feat: hash from rpc instead of calculated v2 1389 #1397

📝 Notes

• [L1RPC] now defaults to HashFromJSON = true: only the way the block hash is obtained changes (from the RPC response instead of RLP hashing), not any behaviour. Set HashFromJSON = false to go back to the previous one.
• L2 clients only support basic mode, like L1: op mode needs per-network extra params.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-08T13:00:34.156053Z	3e30885	PR opened

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3e30885fda

ℹ️ 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".

[chatgpt-codex-connector]

Validate L2 RPC only when the tracker runs

When the binary is invoked with --components proxy, this unconditional check still rejects an invalid or unsupported [L2RPC] section even though only runTracker consumes that configuration and the proxy component never creates L2 clients. This breaks the component isolation already used for tracker-specific validation and can prevent an otherwise valid proxy-only deployment from starting; gate this validation on the tracker component or move it into runTracker.

Useful? React with 👍 / 👎.

[joanestebanr]

Fixed in afba48b: the [L2RPC] validation now lives in runTracker, so it only runs when the tracker component is enabled.

[joanestebanr]

@claude review

[claude]

Claude finished @joanestebanr's task in 31s —— View job

---

Review of PR #1897

The change is small, coherent and well tested. I found no blocking issues, only a few suggestions.

🚀 What's New

• New [L2RPC] section in the proxy config. It holds mode, retry policy, HashFromJSON and BatchBlockHeaderRetrieval, and applies to every L2 client the finder resolves. Each network's URL still comes from the bridge service finder.
• sources.NewFinderClients takes a new rpcCfg parameter and builds every dialed client from it (previously it used NewDefaultRPCClientConfig(), which has HashFromJSON=false).
• proxy validates [L2RPC] at startup via validateL2RPC.
• [L1RPC] and [L2RPC] now default to HashFromJSON = true, so the block hash comes from the RPC response instead of RLP hashing.

🐛 Bug Fixes

None.

📋 Config Updates

[L1RPC]   # existing; now also sets the backoff and the two flags below explicitly
InitialBackoff = "2s"
MaxBackoff = "10s"
BackoffMultiplier = 2.0
HashFromJSON = true
BatchBlockHeaderRetrieval = true

[L2RPC]   # new; URL is ignored
Mode = "basic"
RetryMode = "backoff"
MaxRetries = 5
InitialBackoff = "2s"
MaxBackoff = "10s"
BackoffMultiplier = 2.0
HashFromJSON = true
BatchBlockHeaderRetrieval = true

⚠️ Breaking Changes

• No config breakage.
• The internal Go API sources.NewFinderClients has a new rpcCfg parameter. I found one non-test caller, proxy/cmd/run.go:245, and it is updated.

---

Findings

🟡 Suggested: validate [L2RPC] only when the tracker runs (proxy/cmd/run.go:51)
I agree with the Codex comment. Only runTracker uses cfg.L2RPC (run.go:245). With --components proxy, a bad or unsupported [L2RPC] (for example Mode = "op") still aborts startup, even though the proxy component never dials L2 clients. Gate the check on the tracker, for example:

if slices.Contains(components, proxyconfig.TRACKER) {
    if err := validateL2RPC(cfg.L2RPC); err != nil { ... }
}

The L1 check is correctly unconditional, because the finder always needs L1.

🟡 Suggested: L2 retry backoff defaults differ from the previous behaviour (proxy/config/default.go:24-30)
Before this PR the L2 clients used NewDefaultRPCClientConfig(), which has InitialBackoff=5s and MaxBackoff=60s. The new [L2RPC] default is 2s / 10s, so retries are more aggressive against L2 nodes. This may be intended, but the PR description says "only the way the block hash is obtained changes". Please either keep the previous values or state the change in the Notes. The same applies to [L1RPC] if its effective backoff changes. I did not check how [L1RPC] was filled before, since only MaxRetries was in the TOML.

🟡 Suggested: validateL2RPC mutates a copy through a magic placeholder (proxy/cmd/run.go:393-398)
It works, because cfg is passed by value. The "placeholder" string only exists to get past ErrMissingRPCURL. It would be clearer to call cfg.RetryPolicyGenericConfig.Validate() directly and keep the mode check, which avoids coupling to Validate()'s URL rule. Alternatively, keep the current approach and add a one-line comment explaining why the URL is set.

🟢 Optional: redundant test case (proxy/cmd/run_test.go:23-28)
In the "invalid mode is rejected" case, cfg.Mode = ethermanconfig.RPCModeBasic is immediately overwritten by "bogus", so the first assignment is dead. That case also fails on the "only basic" check, not on Validate(). Two things would make the test better:

• Drop the dead assignment.
• Add a case for an invalid retry policy (for example MaxRetries = -1, or an unknown RetryMode). The PR description says retry-policy validation is covered, but no test exercises it.

🟢 Optional: HashFromJSON test depends on the concrete client type (bridgetracker/sources/sources_test.go:1014-1018)
client.(*etherman.DefaultEthClient) plus the exported HashFromJSON field is fine today. It will break if NewRPCClient is ever wrapped, for example by the OP decorator. That is acceptable here, since only basic mode is allowed. The dial to 127.0.0.1:1 is lazy, so the test should not hit the network, but it is worth confirming it is stable in CI.

🟢 Optional: docs (proxy/config/config.go:51-54)
L2RPC.URL is silently ignored. A user who sets it gets no warning. Consider logging a warning at startup if cfg.L2RPC.URL != "".

Security / Performance

• No security concerns. No secrets or new external inputs, and the URL still comes from the finder.
• HashFromJSON = true avoids RLP-hashing every header, which is a small CPU saving. Trusting the node-supplied hash is the intended trade-off, as with L1. The tracker's rpcClientsBlockHashVerifier still verifies persisted rows on reload.
• BatchBlockHeaderRetrieval = true is the existing default. Nodes without JSON-RPC batch support need false, and the per-L2 config now makes that possible. Note that it applies to all L2 networks at once.

I did not run the build, lint or tests in this review. My conclusions come from reading the diff and the surrounding code.