From 990cdb115d6567e2d957bd50a12808ec513cccd7 Mon Sep 17 00:00:00 2001 From: tac0turtle Date: Mon, 12 Oct 2026 20:06:12 +0200 Subject: [PATCH 1/3] fix(submitter): don't halt the node when SetFinal fails during DA backlog When the DA-included height lags far behind the head, the DA inclusion loop finalizes historical heights one by one. EVM SetFinal resolves each height through HeaderByNumber; once Reth has pruned that history, every retry fails and the submitter reports a critical error that shuts down an otherwise healthy node. - EVM SetFinal: if the block lookup fails but the EL head is at or above the requested height, the block is historical and unavailable, so skip the finalized update with a warning. Lookups above the head, or when the head cannot be read, still return an error. - Submitter: on exhausted SetFinal retries, log and retry on the next tick instead of sending a critical error. The DA-included height does not advance past a height that failed to finalize. --- block/internal/submitting/submitter.go | 11 ++- block/internal/submitting/submitter_test.go | 62 ++++++++++++ execution/evm/execution.go | 24 ++++- execution/evm/execution_setfinal_test.go | 101 ++++++++++++++++++++ 4 files changed, 190 insertions(+), 8 deletions(-) create mode 100644 execution/evm/execution_setfinal_test.go diff --git a/block/internal/submitting/submitter.go b/block/internal/submitting/submitter.go index c90000f2e1..6841fc425a 100644 --- a/block/internal/submitting/submitter.go +++ b/block/internal/submitting/submitter.go @@ -353,11 +353,12 @@ func (s *Submitter) processDAInclusionLoop() { break } - // Set final height in executor + // Set final height in executor. A failure must not stop the node: + // block production and P2P sync do not depend on finality, so the + // DA included height stays put and the next tick retries. if err := s.setFinalWithRetry(nextHeight); err != nil { - s.sendCriticalError(fmt.Errorf("failed to set final height: %w", err)) - s.logger.Error().Err(err).Uint64("height", nextHeight).Msg("CRITICAL: Failed to set final height after retries - halting DA inclusion processing") - return + s.logger.Error().Err(err).Uint64("height", nextHeight).Msg("failed to set final height after retries, will retry on next tick") + break } // Persist DA included height before advancing in-memory state @@ -379,7 +380,7 @@ func (s *Submitter) processDAInclusionLoop() { } // setFinalWithRetry sets the final height in executor with retry logic. -// NOTE: the function retries the execution client call regardless of the error. Some execution client errors are irrecoverable, and will eventually halt the node, as expected. +// NOTE: the function retries the execution client call regardless of the error. Callers decide how to handle exhaustion. func (s *Submitter) setFinalWithRetry(nextHeight uint64) error { for attempt := 1; attempt <= common.MaxRetriesBeforeHalt; attempt++ { if err := s.exec.SetFinal(s.ctx, nextHeight); err != nil { diff --git a/block/internal/submitting/submitter_test.go b/block/internal/submitting/submitter_test.go index ee5340a7f8..67e1773a14 100644 --- a/block/internal/submitting/submitter_test.go +++ b/block/internal/submitting/submitter_test.go @@ -316,6 +316,68 @@ func TestSubmitter_processDAInclusionLoop_advances(t *testing.T) { } +// TestSubmitter_processDAInclusionLoop_setFinalFailureDoesNotHalt ensures that +// exhausting SetFinal retries holds the DA included height and retries on the +// next tick instead of reporting a critical error that stops the node. +func TestSubmitter_processDAInclusionLoop_setFinalFailureDoesNotHalt(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + cm, st := newTestCacheAndStore(t) + + cfg := config.DefaultConfig() + cfg.DA.BlockTime.Duration = 5 * time.Millisecond + + exec := testmocks.NewMockExecutor(t) + exec.On("SetFinal", mock.Anything, uint64(1)).Return(errors.New("block pruned")).Times(common.MaxRetriesBeforeHalt) + exec.On("SetFinal", mock.Anything, uint64(1)).Return(nil).Once() + exec.On("SetFinal", mock.Anything, uint64(2)).Return(nil).Once() + + errCh := make(chan error, 1) + ctx, cancel := context.WithCancel(t.Context()) + s := &Submitter{ + store: st, + exec: exec, + cache: cm, + metrics: common.NopMetrics(), + config: cfg, + daIncludedHeight: &atomic.Uint64{}, + errorCh: errCh, + logger: zerolog.Nop(), + ctx: ctx, + } + + sig := types.Signature([]byte("sig")) + for height := uint64(1); height <= 2; height++ { + h, d := newHeaderAndData("chain", height, true) + batch, err := st.NewBatch(ctx) + require.NoError(t, err) + require.NoError(t, batch.SaveBlockData(h, d, &sig)) + require.NoError(t, batch.SetHeight(height)) + require.NoError(t, batch.Commit()) + cm.SetHeaderDAIncluded(h.Hash().String(), 100+height, height) + cm.SetDataDAIncluded(d.DACommitment().String(), 100+height, height) + } + + done := make(chan struct{}) + go func() { + s.processDAInclusionLoop() + close(done) + }() + + require.Eventually(t, func() bool { + return s.GetDAIncludedHeight() == 2 + }, time.Minute, cfg.DA.BlockTime.Duration) + + cancel() + <-done + + select { + case err := <-errCh: + t.Fatalf("unexpected critical error: %v", err) + default: + } + }) +} + // helper to create a minimal header and data for tests func newHeaderAndData(chainID string, height uint64, nonEmpty bool) (*types.SignedHeader, *types.Data) { now := time.Now() diff --git a/execution/evm/execution.go b/execution/evm/execution.go index 0b64e91e72..d12a9feba1 100644 --- a/execution/evm/execution.go +++ b/execution/evm/execution.go @@ -584,13 +584,31 @@ func (c *EngineClient) doForkchoiceUpdate(ctx context.Context, args engine.Forkc return nil } -// SetFinal marks the block at the given height as finalized +// SetFinal marks the block at the given height as finalized. +// +// When DA inclusion lags far behind the head, the requested height may no +// longer be served by the execution layer (e.g. pruned history). If the +// lookup fails but the EL head is at or above the height, the block exists +// but is unavailable, so the update is skipped: a later height will move the +// finalized label forward. Lookup failures for heights above the head, or +// when the head itself cannot be read, are returned as errors. func (c *EngineClient) SetFinal(ctx context.Context, blockHeight uint64) error { blockHash, _, _, _, err := c.getBlockInfo(ctx, blockHeight) - if err != nil { + if err == nil { + return c.setFinal(ctx, blockHash, true) + } + + head, headErr := c.ethClient.HeaderByNumber(ctx, nil) + if headErr != nil || head == nil || head.Number == nil || !head.Number.IsUint64() || head.Number.Uint64() < blockHeight { return fmt.Errorf("failed to get block info: %w", err) } - return c.setFinal(ctx, blockHash, true) + + c.logger.Warn(). + Uint64("height", blockHeight). + Uint64("el_head", head.Number.Uint64()). + Err(err). + Msg("SetFinal: historical block unavailable on execution layer, skipping finalized update") + return nil } // SetSafe explicitly sets the safe block hash. diff --git a/execution/evm/execution_setfinal_test.go b/execution/evm/execution_setfinal_test.go new file mode 100644 index 0000000000..2b4b20d055 --- /dev/null +++ b/execution/evm/execution_setfinal_test.go @@ -0,0 +1,101 @@ +package evm + +import ( + "context" + "errors" + "math/big" + "testing" + + "github.com/ethereum/go-ethereum/beacon/engine" + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/core/types" + "github.com/rs/zerolog" + "github.com/stretchr/testify/require" +) + +func TestSetFinal_UnavailableBlock(t *testing.T) { + specs := map[string]struct { + head *big.Int + headErr error + expectErr bool + expectFCUpd bool + }{ + "skip_when_below_head": { + head: big.NewInt(1_000), + expectErr: false, + expectFCUpd: false, + }, + "error_when_above_head": { + head: big.NewInt(50), + expectErr: true, + }, + "error_when_head_unavailable": { + headErr: errors.New("connection refused"), + expectErr: true, + }, + } + for name, spec := range specs { + t.Run(name, func(t *testing.T) { + engineRPC := &mockSetFinalEngineRPCClient{} + client := &EngineClient{ + engineClient: engineRPC, + ethClient: mockSetFinalEthRPCClient{ + head: spec.head, + headErr: spec.headErr, + }, + logger: zerolog.Nop(), + } + + err := client.SetFinal(t.Context(), 100) + + if spec.expectErr { + require.Error(t, err) + } else { + require.NoError(t, err) + } + require.Equal(t, spec.expectFCUpd, engineRPC.forkchoiceCalls > 0) + }) + } +} + +// mockSetFinalEthRPCClient fails every historical lookup, as if the EL had +// pruned it, and serves the configured head for latest-block queries. +type mockSetFinalEthRPCClient struct { + head *big.Int + headErr error +} + +func (m mockSetFinalEthRPCClient) HeaderByNumber(_ context.Context, number *big.Int) (*types.Header, error) { + if number != nil { + return nil, errors.New("block pruned") + } + if m.headErr != nil { + return nil, m.headErr + } + return &types.Header{Number: m.head}, nil +} + +func (mockSetFinalEthRPCClient) GetTxs(_ context.Context) ([]string, error) { + return nil, errors.New("unexpected GetTxs call") +} + +func (mockSetFinalEthRPCClient) GetNextProposer(_ context.Context, _ *big.Int) (common.Hash, error) { + return common.Hash{}, errors.New("unexpected GetNextProposer call") +} + +type mockSetFinalEngineRPCClient struct { + forkchoiceCalls int +} + +func (m *mockSetFinalEngineRPCClient) ForkchoiceUpdated(_ context.Context, _ engine.ForkchoiceStateV1, _ map[string]any) (*engine.ForkChoiceResponse, error) { + m.forkchoiceCalls++ + return nil, errors.New("unexpected ForkchoiceUpdated call") +} + +func (m *mockSetFinalEngineRPCClient) GetPayload(_ context.Context, _ engine.PayloadID) (*EnginePayloadEnvelope, error) { + return nil, errors.New("unexpected GetPayload call") +} + +func (m *mockSetFinalEngineRPCClient) NewPayload(_ context.Context, _ *EnginePayloadEnvelope, _ []string, _ string, _ [][]byte) (*engine.PayloadStatusV1, error) { + return nil, errors.New("unexpected NewPayload call") +} From cace6d3a846b39808e463e58d22705fe0ef4a202 Mon Sep 17 00:00:00 2001 From: tac0turtle Date: Mon, 12 Oct 2026 20:06:31 +0200 Subject: [PATCH 2/3] chore: add changelog entry for #3473 --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index bad7904823..d3db5ce02f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Prune per-height state snapshots in `all` pruning mode, keep the state needed to replay the oldest retained block, and stop pruning from falling behind when more than 10k blocks are produced per interval [#3469](https://github.com/evstack/ev-node/pull/3469). +- Do not halt the node when `SetFinal` fails while the DA-included height lags the head; the EVM client skips finalizing historical blocks the execution layer no longer serves, and the submitter retries on the next tick [#3473](https://github.com/evstack/ev-node/pull/3473). ## v1.2.3 From 1ee8358b61706b4031b6f12f862fa085d5f1f847 Mon Sep 17 00:00:00 2001 From: tac0turtle Date: Mon, 12 Oct 2026 20:14:42 +0200 Subject: [PATCH 3/3] fix(evm): only skip SetFinal for pruned or missing history Restrict the historical-block skip in SetFinal to lookup errors that mean the execution layer no longer serves the block: JSON-RPC code 4444 (pruned history, EIP-4444, used by geth and reth) or ethereum.NotFound. Transient lookup failures are returned so the submitter retries the height instead of advancing the DA-included height past it. --- execution/evm/execution.go | 28 +++++++++--- execution/evm/execution_setfinal_test.go | 56 ++++++++++++++++-------- 2 files changed, 61 insertions(+), 23 deletions(-) diff --git a/execution/evm/execution.go b/execution/evm/execution.go index d12a9feba1..b72646b27f 100644 --- a/execution/evm/execution.go +++ b/execution/evm/execution.go @@ -14,6 +14,7 @@ import ( "sync/atomic" "time" + "github.com/ethereum/go-ethereum" "github.com/ethereum/go-ethereum/beacon/engine" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/hexutil" @@ -587,16 +588,19 @@ func (c *EngineClient) doForkchoiceUpdate(ctx context.Context, args engine.Forkc // SetFinal marks the block at the given height as finalized. // // When DA inclusion lags far behind the head, the requested height may no -// longer be served by the execution layer (e.g. pruned history). If the -// lookup fails but the EL head is at or above the height, the block exists -// but is unavailable, so the update is skipped: a later height will move the -// finalized label forward. Lookup failures for heights above the head, or -// when the head itself cannot be read, are returned as errors. +// longer be served by the execution layer (pruned history). If the lookup +// reports the block as pruned or not found and the EL head is at or above the +// height, the update is skipped: a later height will move the finalized label +// forward. Any other lookup failure (e.g. transient RPC errors), lookups above +// the head, or an unreadable head are returned as errors so the caller retries. func (c *EngineClient) SetFinal(ctx context.Context, blockHeight uint64) error { blockHash, _, _, _, err := c.getBlockInfo(ctx, blockHeight) if err == nil { return c.setFinal(ctx, blockHash, true) } + if !isHistoryUnavailable(err) { + return fmt.Errorf("failed to get block info: %w", err) + } head, headErr := c.ethClient.HeaderByNumber(ctx, nil) if headErr != nil || head == nil || head.Number == nil || !head.Number.IsUint64() || head.Number.Uint64() < blockHeight { @@ -611,6 +615,20 @@ func (c *EngineClient) SetFinal(ctx context.Context, blockHeight uint64) error { return nil } +// prunedHistoryErrorCode is the JSON-RPC error code execution clients (geth, +// reth) return when a request touches pruned history (EIP-4444). +const prunedHistoryErrorCode = 4444 + +// isHistoryUnavailable reports whether err means the execution layer no longer +// serves the requested block, as opposed to a transient failure. +func isHistoryUnavailable(err error) bool { + if errors.Is(err, ethereum.NotFound) { + return true + } + var rpcErr rpc.Error + return errors.As(err, &rpcErr) && rpcErr.ErrorCode() == prunedHistoryErrorCode +} + // SetSafe explicitly sets the safe block hash. // This allows the derivation layer to advance the safe block independently of head. // Safe indicates a block that is unlikely to be reorged (e.g., confirmed by DA). diff --git a/execution/evm/execution_setfinal_test.go b/execution/evm/execution_setfinal_test.go index 2b4b20d055..7382488488 100644 --- a/execution/evm/execution_setfinal_test.go +++ b/execution/evm/execution_setfinal_test.go @@ -6,6 +6,7 @@ import ( "math/big" "testing" + "github.com/ethereum/go-ethereum" "github.com/ethereum/go-ethereum/beacon/engine" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core/types" @@ -14,22 +15,39 @@ import ( ) func TestSetFinal_UnavailableBlock(t *testing.T) { + prunedErr := &testRPCError{code: prunedHistoryErrorCode, msg: "pruned history unavailable"} + specs := map[string]struct { - head *big.Int - headErr error - expectErr bool - expectFCUpd bool + lookupErr error + head *big.Int + headErr error + expectErr bool }{ - "skip_when_below_head": { - head: big.NewInt(1_000), - expectErr: false, - expectFCUpd: false, + "skip_pruned_below_head": { + lookupErr: prunedErr, + head: big.NewInt(1_000), + }, + "skip_not_found_below_head": { + lookupErr: ethereum.NotFound, + head: big.NewInt(1_000), + }, + "error_transient_lookup_below_head": { + lookupErr: errors.New("i/o timeout"), + head: big.NewInt(1_000), + expectErr: true, + }, + "error_other_rpc_code_below_head": { + lookupErr: &testRPCError{code: -32000, msg: "internal error"}, + head: big.NewInt(1_000), + expectErr: true, }, - "error_when_above_head": { + "error_pruned_above_head": { + lookupErr: prunedErr, head: big.NewInt(50), expectErr: true, }, - "error_when_head_unavailable": { + "error_pruned_head_unavailable": { + lookupErr: prunedErr, headErr: errors.New("connection refused"), expectErr: true, }, @@ -40,8 +58,9 @@ func TestSetFinal_UnavailableBlock(t *testing.T) { client := &EngineClient{ engineClient: engineRPC, ethClient: mockSetFinalEthRPCClient{ - head: spec.head, - headErr: spec.headErr, + lookupErr: spec.lookupErr, + head: spec.head, + headErr: spec.headErr, }, logger: zerolog.Nop(), } @@ -53,21 +72,22 @@ func TestSetFinal_UnavailableBlock(t *testing.T) { } else { require.NoError(t, err) } - require.Equal(t, spec.expectFCUpd, engineRPC.forkchoiceCalls > 0) + require.Zero(t, engineRPC.forkchoiceCalls) }) } } -// mockSetFinalEthRPCClient fails every historical lookup, as if the EL had -// pruned it, and serves the configured head for latest-block queries. +// mockSetFinalEthRPCClient fails every numbered lookup with lookupErr and +// serves the configured head for latest-block queries. type mockSetFinalEthRPCClient struct { - head *big.Int - headErr error + lookupErr error + head *big.Int + headErr error } func (m mockSetFinalEthRPCClient) HeaderByNumber(_ context.Context, number *big.Int) (*types.Header, error) { if number != nil { - return nil, errors.New("block pruned") + return nil, m.lookupErr } if m.headErr != nil { return nil, m.headErr