From 9209776159e14efee3900a4d99fc121ab2698a6e Mon Sep 17 00:00:00 2001 From: corey Date: Fri, 28 Aug 2026 14:14:14 +0800 Subject: [PATCH 1/3] fix(node): make RPC retries and derivation shutdown respect context cancellation RetryableClient shared one BackOff across all methods and passed it to backoff.Retry unbound, so a canceled context could not stop a retry loop; context errors were also classified retryable. A shutdown during an L2 outage therefore held Derivation.Stop for the full 30-minute budget. Bind the backoff to the caller's context (fresh instance per call, since BackOff is stateful and these methods run concurrently), treat context errors as permanent, and bound Stop's wait so shutdown cannot hang on a poll that is mid-RPC. Co-authored-by: Cursor --- node/derivation/derivation.go | 21 +++++++++++--- node/types/retryable_client.go | 44 ++++++++++++++++++----------- node/types/retryable_client_test.go | 29 +++++++++++++++++++ 3 files changed, 74 insertions(+), 20 deletions(-) diff --git a/node/derivation/derivation.go b/node/derivation/derivation.go index 00d9972aa..7d1b3fdf0 100644 --- a/node/derivation/derivation.go +++ b/node/derivation/derivation.go @@ -34,6 +34,10 @@ var ( RollupEventTopicHash = crypto.Keccak256Hash([]byte(RollupEventTopic)) ) +// stopTimeout bounds how long Stop waits for the main loop to unwind after +// the context is canceled. +const stopTimeout = 30 * time.Second + type Derivation struct { ctx context.Context node *tmnode.Node @@ -229,8 +233,17 @@ func (d *Derivation) Stop() { if d.cancel != nil { d.cancel() } - <-d.stop - d.logger.Info("derivation service is stopped") + // The main loop only observes ctx.Done() between polls, so a poll that + // is mid-RPC still has to unwind. Bound the wait rather than block the + // process's shutdown sequence indefinitely: nothing here needs a clean + // hand-off, since the L1 cursor is only persisted after a fully + // successful poll and any partial derivation is redone on restart. + select { + case <-d.stop: + d.logger.Info("derivation service is stopped") + case <-time.After(stopTimeout): + d.logger.Error("derivation service did not stop within timeout; abandoning wait", "timeout", stopTimeout) + } } func (d *Derivation) derivationBlock(ctx context.Context) { @@ -546,7 +559,7 @@ func (d *Derivation) fetchRollupLog(ctx context.Context, from, to uint64) ([]eth } func (d *Derivation) fetchRollupDataByTxHash(txHash common.Hash, blockNumber uint64) (*BatchInfo, error) { - tx, pending, err := d.l1Client.TransactionByHash(context.Background(), txHash) + tx, pending, err := d.l1Client.TransactionByHash(d.ctx, txHash) if err != nil { return nil, err } @@ -762,7 +775,7 @@ func (d *Derivation) getL1Message(l1MessagePopped, l1MsgNum uint64) ([]types.L1M func (d *Derivation) derive(rollupData *BatchInfo) (*eth.Header, error) { var lastHeader *eth.Header for _, blockData := range rollupData.blockContexts { - latestBlockNumber, err := d.l2Client.BlockNumber(context.Background()) + latestBlockNumber, err := d.l2Client.BlockNumber(d.ctx) if err != nil { return nil, fmt.Errorf("get derivation geth block number error:%v", err) } diff --git a/node/types/retryable_client.go b/node/types/retryable_client.go index ffe9f7f3f..ae175d994 100644 --- a/node/types/retryable_client.go +++ b/node/types/retryable_client.go @@ -46,22 +46,28 @@ const ( type RetryableClient struct { authClient *authclient.Client ethClient *ethclient.Client - b backoff.BackOff logger tmlog.Logger } func NewRetryableClient(authClient *authclient.Client, ethClient *ethclient.Client, logger tmlog.Logger) *RetryableClient { logger = logger.With("module", "retryClient") - bo := backoff.NewExponentialBackOff() - bo.MaxElapsedTime = GethRetryMaxElapsedTime return &RetryableClient{ authClient: authClient, ethClient: ethClient, - b: bo, logger: logger, } } +// retryPolicy builds the backoff for a single call. It is bound to ctx so a +// canceled caller (process shutdown) aborts the retry loop instead of +// spinning until GethRetryMaxElapsedTime. A per-call instance is required: +// backoff.BackOff carries attempt state and these methods run concurrently. +func retryPolicy(ctx context.Context) backoff.BackOffContext { + bo := backoff.NewExponentialBackOff() + bo.MaxElapsedTime = GethRetryMaxElapsedTime + return backoff.WithContext(bo, ctx) +} + func (rc *RetryableClient) AssembleL2Block(ctx context.Context, number *big.Int, transactions eth.Transactions) (ret *catalyst.ExecutableL2Data, err error) { timestamp := uint64(time.Now().Unix()) if retryErr := backoff.Retry(func() error { @@ -75,7 +81,7 @@ func (rc *RetryableClient) AssembleL2Block(ctx context.Context, number *big.Int, } ret = resp return nil - }, rc.b); retryErr != nil { + }, retryPolicy(ctx)); retryErr != nil { return nil, retryErr } return @@ -93,7 +99,7 @@ func (rc *RetryableClient) ValidateL2Block(ctx context.Context, executableL2Data } ret = resp return nil - }, rc.b); retryErr != nil { + }, retryPolicy(ctx)); retryErr != nil { return false, retryErr } return @@ -112,7 +118,7 @@ func (rc *RetryableClient) NewL2Block(ctx context.Context, executableL2Data *cat err = respErr } return nil - }, rc.b); retryErr != nil { + }, retryPolicy(ctx)); retryErr != nil { return retryErr } return @@ -133,7 +139,7 @@ func (rc *RetryableClient) NewL2BlockV2(ctx context.Context, executableL2Data *c } header = respHeader return nil - }, rc.b); retryErr != nil { + }, retryPolicy(ctx)); retryErr != nil { return nil, retryErr } return @@ -151,7 +157,7 @@ func (rc *RetryableClient) NewSafeL2Block(ctx context.Context, safeL2Data *catal } ret = resp return nil - }, rc.b); retryErr != nil { + }, retryPolicy(ctx)); retryErr != nil { return nil, retryErr } return @@ -169,7 +175,7 @@ func (rc *RetryableClient) BlockNumber(ctx context.Context) (ret uint64, err err } ret = resp return nil - }, rc.b); retryErr != nil { + }, retryPolicy(ctx)); retryErr != nil { return 0, retryErr } return @@ -188,7 +194,7 @@ func (rc *RetryableClient) HeaderByNumber(ctx context.Context, blockNumber *big. } ret = resp return nil - }, rc.b); retryErr != nil { + }, retryPolicy(ctx)); retryErr != nil { return nil, retryErr } return @@ -207,7 +213,7 @@ func (rc *RetryableClient) BlockByNumber(ctx context.Context, blockNumber *big.I } ret = resp return nil - }, rc.b); retryErr != nil { + }, retryPolicy(ctx)); retryErr != nil { return nil, retryErr } return @@ -225,7 +231,7 @@ func (rc *RetryableClient) CallContract(ctx context.Context, call ethereum.CallM } ret = resp return nil - }, rc.b); retryErr != nil { + }, retryPolicy(ctx)); retryErr != nil { return nil, retryErr } return @@ -243,7 +249,7 @@ func (rc *RetryableClient) CodeAt(ctx context.Context, contract common.Address, } ret = resp return nil - }, rc.b); retryErr != nil { + }, retryPolicy(ctx)); retryErr != nil { return nil, retryErr } return @@ -260,7 +266,7 @@ func (rc *RetryableClient) SetBlockTags(ctx context.Context, safeBlockHash commo err = respErr } return nil - }, rc.b); retryErr != nil { + }, retryPolicy(ctx)); retryErr != nil { return retryErr } return @@ -282,6 +288,9 @@ func (rc *RetryableClient) SetBlockTags(ctx context.Context, safeBlockHash commo // block, derivation logs an Error, and the next poll re-evaluates. // - DiscontinuousBlockError: structurally invalid input that no amount // of retry will fix. +// - context.Canceled / context.DeadlineExceeded: the caller is gone (or +// the process is shutting down). Retrying cannot succeed and would +// hold the caller for the full backoff budget. // // retryableError returns true for transient errors that should be retried. // Permanent logic errors (wrong block number, missing parent) and block @@ -291,6 +300,9 @@ func retryableError(err error) bool { if errors.Is(err, ethereum.NotFound) { return false } + if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { + return false + } msg := err.Error() return !strings.Contains(msg, DiscontinuousBlockError) && !strings.Contains(msg, WrongBlockNumberError) && @@ -317,7 +329,7 @@ func (rc *RetryableClient) AssembleL2BlockV2(ctx context.Context, parentHash com } ret = resp return nil - }, rc.b); retryErr != nil { + }, retryPolicy(ctx)); retryErr != nil { return nil, retryErr } return diff --git a/node/types/retryable_client_test.go b/node/types/retryable_client_test.go index 673780fd6..9e91d9173 100644 --- a/node/types/retryable_client_test.go +++ b/node/types/retryable_client_test.go @@ -1,10 +1,13 @@ package types import ( + "context" "errors" "fmt" "testing" + "time" + "github.com/cenkalti/backoff/v4" "github.com/morph-l2/go-ethereum" "github.com/stretchr/testify/require" ) @@ -76,6 +79,32 @@ func TestRetryableError_NotFoundIsPermanent(t *testing.T) { } } +// A canceled or expired context must be permanent, and retryPolicy must be +// bound to the caller's context. Together these bound how long an in-flight +// call holds the caller during shutdown; without them a canceled caller +// still waits out the full GethRetryMaxElapsedTime budget. +func TestRetryableError_ContextErrorsArePermanent(t *testing.T) { + require.False(t, retryableError(context.Canceled)) + require.False(t, retryableError(context.DeadlineExceeded)) + require.False(t, retryableError(fmt.Errorf("BlockNumber: %w", context.Canceled))) +} + +func TestRetryPolicy_StopsOnCanceledContext(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + attempts := 0 + start := time.Now() + err := backoff.Retry(func() error { + attempts++ + return errors.New("connection refused") + }, retryPolicy(ctx)) + + require.Error(t, err) + require.Equal(t, 1, attempts, "canceled context must not be retried") + require.Less(t, time.Since(start), time.Second) +} + func TestRetryableError_DiscontinuousBlockIsPermanent(t *testing.T) { err := errors.New("discontinuous block number: ...") if retryableError(err) { From 7870b3342caf938ec5fe2d87f5c4a5d155f65e71 Mon Sep 17 00:00:00 2001 From: corey Date: Fri, 28 Aug 2026 14:19:23 +0800 Subject: [PATCH 2/3] refactor(node): drop the context-bound backoff refactor Classifying context errors as permanent is enough to unblock a canceled caller: retryableError returning false makes the backoff operation return nil, so the call exits after one attempt. Binding the backoff to ctx was a separate robustness fix and is not needed here. Co-authored-by: Cursor --- node/types/retryable_client.go | 45 +++++++++++++---------------- node/types/retryable_client_test.go | 24 ++------------- 2 files changed, 22 insertions(+), 47 deletions(-) diff --git a/node/types/retryable_client.go b/node/types/retryable_client.go index ae175d994..6195e8e6b 100644 --- a/node/types/retryable_client.go +++ b/node/types/retryable_client.go @@ -46,28 +46,22 @@ const ( type RetryableClient struct { authClient *authclient.Client ethClient *ethclient.Client + b backoff.BackOff logger tmlog.Logger } func NewRetryableClient(authClient *authclient.Client, ethClient *ethclient.Client, logger tmlog.Logger) *RetryableClient { logger = logger.With("module", "retryClient") + bo := backoff.NewExponentialBackOff() + bo.MaxElapsedTime = GethRetryMaxElapsedTime return &RetryableClient{ authClient: authClient, ethClient: ethClient, + b: bo, logger: logger, } } -// retryPolicy builds the backoff for a single call. It is bound to ctx so a -// canceled caller (process shutdown) aborts the retry loop instead of -// spinning until GethRetryMaxElapsedTime. A per-call instance is required: -// backoff.BackOff carries attempt state and these methods run concurrently. -func retryPolicy(ctx context.Context) backoff.BackOffContext { - bo := backoff.NewExponentialBackOff() - bo.MaxElapsedTime = GethRetryMaxElapsedTime - return backoff.WithContext(bo, ctx) -} - func (rc *RetryableClient) AssembleL2Block(ctx context.Context, number *big.Int, transactions eth.Transactions) (ret *catalyst.ExecutableL2Data, err error) { timestamp := uint64(time.Now().Unix()) if retryErr := backoff.Retry(func() error { @@ -81,7 +75,7 @@ func (rc *RetryableClient) AssembleL2Block(ctx context.Context, number *big.Int, } ret = resp return nil - }, retryPolicy(ctx)); retryErr != nil { + }, rc.b); retryErr != nil { return nil, retryErr } return @@ -99,7 +93,7 @@ func (rc *RetryableClient) ValidateL2Block(ctx context.Context, executableL2Data } ret = resp return nil - }, retryPolicy(ctx)); retryErr != nil { + }, rc.b); retryErr != nil { return false, retryErr } return @@ -118,7 +112,7 @@ func (rc *RetryableClient) NewL2Block(ctx context.Context, executableL2Data *cat err = respErr } return nil - }, retryPolicy(ctx)); retryErr != nil { + }, rc.b); retryErr != nil { return retryErr } return @@ -139,7 +133,7 @@ func (rc *RetryableClient) NewL2BlockV2(ctx context.Context, executableL2Data *c } header = respHeader return nil - }, retryPolicy(ctx)); retryErr != nil { + }, rc.b); retryErr != nil { return nil, retryErr } return @@ -157,7 +151,7 @@ func (rc *RetryableClient) NewSafeL2Block(ctx context.Context, safeL2Data *catal } ret = resp return nil - }, retryPolicy(ctx)); retryErr != nil { + }, rc.b); retryErr != nil { return nil, retryErr } return @@ -175,7 +169,7 @@ func (rc *RetryableClient) BlockNumber(ctx context.Context) (ret uint64, err err } ret = resp return nil - }, retryPolicy(ctx)); retryErr != nil { + }, rc.b); retryErr != nil { return 0, retryErr } return @@ -194,7 +188,7 @@ func (rc *RetryableClient) HeaderByNumber(ctx context.Context, blockNumber *big. } ret = resp return nil - }, retryPolicy(ctx)); retryErr != nil { + }, rc.b); retryErr != nil { return nil, retryErr } return @@ -213,7 +207,7 @@ func (rc *RetryableClient) BlockByNumber(ctx context.Context, blockNumber *big.I } ret = resp return nil - }, retryPolicy(ctx)); retryErr != nil { + }, rc.b); retryErr != nil { return nil, retryErr } return @@ -231,7 +225,7 @@ func (rc *RetryableClient) CallContract(ctx context.Context, call ethereum.CallM } ret = resp return nil - }, retryPolicy(ctx)); retryErr != nil { + }, rc.b); retryErr != nil { return nil, retryErr } return @@ -249,7 +243,7 @@ func (rc *RetryableClient) CodeAt(ctx context.Context, contract common.Address, } ret = resp return nil - }, retryPolicy(ctx)); retryErr != nil { + }, rc.b); retryErr != nil { return nil, retryErr } return @@ -266,7 +260,7 @@ func (rc *RetryableClient) SetBlockTags(ctx context.Context, safeBlockHash commo err = respErr } return nil - }, retryPolicy(ctx)); retryErr != nil { + }, rc.b); retryErr != nil { return retryErr } return @@ -288,9 +282,10 @@ func (rc *RetryableClient) SetBlockTags(ctx context.Context, safeBlockHash commo // block, derivation logs an Error, and the next poll re-evaluates. // - DiscontinuousBlockError: structurally invalid input that no amount // of retry will fix. -// - context.Canceled / context.DeadlineExceeded: the caller is gone (or -// the process is shutting down). Retrying cannot succeed and would -// hold the caller for the full backoff budget. +// - context.Canceled / context.DeadlineExceeded: the caller's context is +// already done (typically process shutdown), so no retry can succeed. +// The backoff loop is not context-bound, so without this the caller is +// held for the full 30-minute budget after cancellation. // // retryableError returns true for transient errors that should be retried. // Permanent logic errors (wrong block number, missing parent) and block @@ -329,7 +324,7 @@ func (rc *RetryableClient) AssembleL2BlockV2(ctx context.Context, parentHash com } ret = resp return nil - }, retryPolicy(ctx)); retryErr != nil { + }, rc.b); retryErr != nil { return nil, retryErr } return diff --git a/node/types/retryable_client_test.go b/node/types/retryable_client_test.go index 9e91d9173..5fcbb74a3 100644 --- a/node/types/retryable_client_test.go +++ b/node/types/retryable_client_test.go @@ -5,9 +5,7 @@ import ( "errors" "fmt" "testing" - "time" - "github.com/cenkalti/backoff/v4" "github.com/morph-l2/go-ethereum" "github.com/stretchr/testify/require" ) @@ -79,32 +77,14 @@ func TestRetryableError_NotFoundIsPermanent(t *testing.T) { } } -// A canceled or expired context must be permanent, and retryPolicy must be -// bound to the caller's context. Together these bound how long an in-flight -// call holds the caller during shutdown; without them a canceled caller -// still waits out the full GethRetryMaxElapsedTime budget. +// The backoff loop is not context-bound, so a canceled caller only unblocks +// if the context error itself is classified permanent. func TestRetryableError_ContextErrorsArePermanent(t *testing.T) { require.False(t, retryableError(context.Canceled)) require.False(t, retryableError(context.DeadlineExceeded)) require.False(t, retryableError(fmt.Errorf("BlockNumber: %w", context.Canceled))) } -func TestRetryPolicy_StopsOnCanceledContext(t *testing.T) { - ctx, cancel := context.WithCancel(context.Background()) - cancel() - - attempts := 0 - start := time.Now() - err := backoff.Retry(func() error { - attempts++ - return errors.New("connection refused") - }, retryPolicy(ctx)) - - require.Error(t, err) - require.Equal(t, 1, attempts, "canceled context must not be retried") - require.Less(t, time.Since(start), time.Second) -} - func TestRetryableError_DiscontinuousBlockIsPermanent(t *testing.T) { err := errors.New("discontinuous block number: ...") if retryableError(err) { From 077f82dac005242b7cb440b51f482782a3307af7 Mon Sep 17 00:00:00 2001 From: corey Date: Fri, 28 Aug 2026 14:23:08 +0800 Subject: [PATCH 3/3] refactor(node): keep the shutdown fix inside derivation Revert the retryableError change; bounding Stop's wait is enough to keep shutdown from hanging, since the process exits once Stop returns. Document why derive's BlockNumber call must stay on context.Background(). Co-authored-by: Cursor --- node/derivation/derivation.go | 6 +++++- node/types/retryable_client.go | 7 ------- node/types/retryable_client_test.go | 9 --------- 3 files changed, 5 insertions(+), 17 deletions(-) diff --git a/node/derivation/derivation.go b/node/derivation/derivation.go index 7d1b3fdf0..e574c2311 100644 --- a/node/derivation/derivation.go +++ b/node/derivation/derivation.go @@ -775,7 +775,11 @@ func (d *Derivation) getL1Message(l1MessagePopped, l1MsgNum uint64) ([]types.L1M func (d *Derivation) derive(rollupData *BatchInfo) (*eth.Header, error) { var lastHeader *eth.Header for _, blockData := range rollupData.blockContexts { - latestBlockNumber, err := d.l2Client.BlockNumber(d.ctx) + // Deliberately not d.ctx: RetryableClient's backoff loop is not + // context-bound and treats a canceled context as retryable, so + // passing d.ctx here would spin until the 30-minute retry budget + // expires instead of returning. + latestBlockNumber, err := d.l2Client.BlockNumber(context.Background()) if err != nil { return nil, fmt.Errorf("get derivation geth block number error:%v", err) } diff --git a/node/types/retryable_client.go b/node/types/retryable_client.go index 6195e8e6b..ffe9f7f3f 100644 --- a/node/types/retryable_client.go +++ b/node/types/retryable_client.go @@ -282,10 +282,6 @@ func (rc *RetryableClient) SetBlockTags(ctx context.Context, safeBlockHash commo // block, derivation logs an Error, and the next poll re-evaluates. // - DiscontinuousBlockError: structurally invalid input that no amount // of retry will fix. -// - context.Canceled / context.DeadlineExceeded: the caller's context is -// already done (typically process shutdown), so no retry can succeed. -// The backoff loop is not context-bound, so without this the caller is -// held for the full 30-minute budget after cancellation. // // retryableError returns true for transient errors that should be retried. // Permanent logic errors (wrong block number, missing parent) and block @@ -295,9 +291,6 @@ func retryableError(err error) bool { if errors.Is(err, ethereum.NotFound) { return false } - if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { - return false - } msg := err.Error() return !strings.Contains(msg, DiscontinuousBlockError) && !strings.Contains(msg, WrongBlockNumberError) && diff --git a/node/types/retryable_client_test.go b/node/types/retryable_client_test.go index 5fcbb74a3..673780fd6 100644 --- a/node/types/retryable_client_test.go +++ b/node/types/retryable_client_test.go @@ -1,7 +1,6 @@ package types import ( - "context" "errors" "fmt" "testing" @@ -77,14 +76,6 @@ func TestRetryableError_NotFoundIsPermanent(t *testing.T) { } } -// The backoff loop is not context-bound, so a canceled caller only unblocks -// if the context error itself is classified permanent. -func TestRetryableError_ContextErrorsArePermanent(t *testing.T) { - require.False(t, retryableError(context.Canceled)) - require.False(t, retryableError(context.DeadlineExceeded)) - require.False(t, retryableError(fmt.Errorf("BlockNumber: %w", context.Canceled))) -} - func TestRetryableError_DiscontinuousBlockIsPermanent(t *testing.T) { err := errors.New("discontinuous block number: ...") if retryableError(err) {