fix(l1): retry transient chain ID errors instead of shutting down the node by Ehsan-saradar · Pull Request #3771 · NethermindEth/juno

Ehsan-saradar · 2026-06-28T12:29:58Z

What & why

A transient L1 RPC error during the startup chain-ID check would take the whole node down. Per #1385: eth_chainId returns daily request count exceeded, request rate limited, the L1 client's Run returns that error, and since node.StartService runs each service with defer cancel(), any service's Run returning shuts the node down. This also violated the service.Service contract, which asks services to log and retry non-critical errors rather than return them.

What this changes

Retry transient failures. A new verifyChainID (used only by Run) retries transient chain-ID failures — rate limits, timeouts, an unresponsive node — as warnings, reusing the client's existing resubscribe-delay loop (same pattern as subscribeToUpdates and finalisedHeight), until the check passes or ctx is cancelled. Retries are unbounded on purpose: a cap would just return an error and re-trigger the shutdown this fixes.
Keep genuine mismatches fatal. A real L1/L2 network mismatch is a misconfiguration retrying can't fix, so it stays fatal (now a typed chainIDMismatchError, same user-facing message).
checkChainID stays single-attempt — the one-shot CatchUpL1Head migration path relies on it failing fast.

After this change no transient L1 RPC error can shut the node down: catchUpL1HeadUpdates is already best-effort, subscribeToUpdates/finalisedHeight already retry forever, and the only remaining fatal paths are a genuine mismatch and a local DB-write failure in setL1Head — both legitimately critical.

Relationship to #3685

Complementary, not overlapping: #3685 makes StartService tolerate a service returning nil, but an error return still stops the node. Transient chain-ID failures are errors, so the L1 client must stop emitting them regardless. No dependency on #3685.

Tests

go test ./l1/ passes. New TestTransientChainIDErrorDoesNotShutDownNode reproduces the issue's rate-limit scenario and asserts the node retries instead of shutting down (fails on the old code). TestChainIDCheckTimeout / TestChainIDFetchError were re-pointed at the fail-fast CatchUpL1Head path (same error-message assertions); the mismatch tests are unchanged.

Fixes #1385

Copilot

Pull request overview

This PR changes the L1 client’s startup chain-ID verification so transient eth_chainId failures (rate limits/timeouts/unresponsive RPC) are retried instead of returning an error that shuts down the entire node, while keeping genuine L1/L2 network mismatches fatal.

Changes:

Add verifyChainID with an unbounded retry loop for transient chain-ID probe failures and a typed fatal chainIDMismatchError.
Make checkChainID single-attempt and update Run to call verifyChainID (while CatchUpL1Head continues to use checkChainID for fail-fast behavior).
Update/add tests to validate retry-on-transient behavior and keep fail-fast assertions on the CatchUpL1Head path.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
l1/l1.go	Introduces retrying chain-ID verification in `Run`, keeps mismatches fatal via a typed error, and makes `checkChainID` single-shot for migration path.
l1/l1_test.go	Re-targets existing chain-ID failure tests to `CatchUpL1Head` and adds a regression test ensuring transient chain-ID errors don’t shut down the node.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

… node The L1 client ran as a node service, and node.StartService brings the whole node down whenever a service's Run returns. checkChainID returned transient RPC failures (rate limits, timeouts, an unresponsive endpoint) straight out of Run, so a single eth_chainId hiccup on a free-tier provider — e.g. "daily request count exceeded, request rate limited" — killed the node. This also violated the service contract, which asks services to log and retry non-critical errors rather than return them. Add verifyChainID, used only by Run, which retries transient failures with the client's existing resubscribe-delay loop (matching subscribeToUpdates and finalisedHeight), demoting them to warnings until the check succeeds or the context is cancelled. Retries are unbounded by design: a cap would just reintroduce the shutdown. A genuine chain-ID mismatch is a misconfiguration and stays fatal, now modelled as a typed chainIDMismatchError so the retry loop can tell it apart from transient errors. checkChainID stays single-attempt because the one-shot CatchUpL1Head migration path must keep failing fast. Fixes NethermindEth#1385

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

codecov · 2026-06-30T08:15:59Z

Codecov Report

❌ Patch coverage is 93.54839% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.34%. Comparing base (4940509) to head (8efc3d2).

Files with missing lines	Patch %	Lines
l1/l1.go	93.54%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3771      +/-   ##
==========================================
+ Coverage   74.91%   76.34%   +1.42%     
==========================================
  Files         433      433              
  Lines       38815    38656     -159     
==========================================
+ Hits        29079    29510     +431     
+ Misses       7733     7076     -657     
- Partials     2003     2070      +67

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

brbrr

LGTM! Thanks for the contribution!

rodrodros · 2026-06-30T13:04:57Z

+			// Transient: warn and retry, unless we're already shutting down.
+			if ctx.Err() != nil {
+				return nil
+			}


This ctx.Err() is not required since ctx.Done() is checked two instructions after

rodrodros · 2026-06-30T13:05:36Z

+			c.logger.Warn("Failed to verify L1 chain ID; retrying",
+				zap.Duration("tryAgainIn", c.resubscribeDelay),
+			)
+			c.logger.Debug("L1 chain ID verification failed", zap.Error(err))


Why do we need to logs for the same thing? Why doesn't the warn share the error as well?

rodrodros · 2026-06-30T13:08:23Z

+	if err := c.verifyChainID(ctx); err != nil {
 		return err
 	}
+	// verifyChainID returns nil on ctx cancellation; don't start any further
+	// RPC work (which would just fail with context canceled) if we're stopping.
+	if ctx.Err() != nil {
+		return nil
+	}


What is this patch here, why does verifyChainID returns nil on context cancellation rather than returning the context error?

rodrodros · 2026-06-30T13:13:45Z

+// chainIDMismatchError marks an L1/L2 network mismatch: a misconfiguration that
+// retrying cannot fix, so verifyChainID treats it as fatal. (Supporting custom
+// forked Starknet networks would mean warning here instead of erroring.)
+type chainIDMismatchError struct {
+	network string
+}
+
+func (e *chainIDMismatchError) Error() string {
+	return fmt.Sprintf(
+		"mismatched network id between L1 and L2. L2 network is %s; "+
+			"is --eth-node pointing to the right network?",
+		e.network,
+	)
+}


There is no need for a custom error as I can see. It is being used only in one place

rodrodros · 2026-06-30T13:14:26Z

+				zap.Duration("tryAgainIn", c.resubscribeDelay),
+			)
+			c.logger.Debug("L1 chain ID verification failed", zap.Error(err))
+			timer.Reset(c.resubscribeDelay)


Why resubscribe delay is used for separating between requests?

Ehsan-saradar · 2026-07-01T07:15:52Z

@rodrodros thanks, these all make sense. Since the PR is already merged, do you want me to open a follow-up issue/PR for these, or are they minor enough to leave as-is?

Copilot AI review requested due to automatic review settings June 28, 2026 12:29

Ehsan-saradar mentioned this pull request Jun 28, 2026

L1 Verifier shouldn't shutdown the node when it encounters an error #1385

Closed

Copilot started reviewing on behalf of Ehsan-saradar June 28, 2026 12:30 View session

Copilot AI reviewed Jun 28, 2026

View reviewed changes

Comment thread l1/l1.go

Ehsan-saradar force-pushed the fix/1385-l1-no-shutdown-on-error branch from de6ed75 to d0210c0 Compare June 28, 2026 12:42

Ehsan-saradar requested a review from Copilot June 29, 2026 10:14

Copilot started reviewing on behalf of Ehsan-saradar June 29, 2026 10:14 View session

Copilot AI reviewed Jun 29, 2026

View reviewed changes

Comment thread l1/l1.go

fix(l1): skip catch-up when ctx cancelled during chain ID verify

1527478

Ehsan-saradar requested a review from Copilot June 30, 2026 09:12

Copilot started reviewing on behalf of Ehsan-saradar June 30, 2026 09:13 View session

Copilot AI reviewed Jun 30, 2026

View reviewed changes

brbrr approved these changes Jun 30, 2026

View reviewed changes

Merge branch 'main' into fix/1385-l1-no-shutdown-on-error

8efc3d2

brbrr merged commit 0cb6716 into NethermindEth:main Jun 30, 2026
17 checks passed

rodrodros reviewed Jun 30, 2026

View reviewed changes

brbrr mentioned this pull request Jun 30, 2026

fix(l1): address various issues in l1 chainID fetching logic #3776

Merged

Uh oh!

Conversation

Ehsan-saradar commented Jun 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What & why

What this changes

Relationship to #3685

Tests

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

codecov Bot commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

brbrr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rodrodros Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

rodrodros Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

rodrodros Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

rodrodros Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

rodrodros Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

Ehsan-saradar commented Jul 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Ehsan-saradar commented Jun 28, 2026 •

edited

Loading

codecov Bot commented Jun 30, 2026 •

edited

Loading