Skip to content

Commit cd339dc

Browse files
authored
fix(submitter): don't halt the node when SetFinal fails during DA backlog (#3473)
* 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. * chore: add changelog entry for #3473 * 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.
1 parent 758708d commit cd339dc

5 files changed

Lines changed: 229 additions & 8 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1212
### Fixed
1313

1414
- 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).
15+
- 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).
1516

1617
## v1.2.3
1718

‎block/internal/submitting/submitter.go‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -353,11 +353,12 @@ func (s *Submitter) processDAInclusionLoop() {
353353
break
354354
}
355355

356-
// Set final height in executor
356+
// Set final height in executor. A failure must not stop the node:
357+
// block production and P2P sync do not depend on finality, so the
358+
// DA included height stays put and the next tick retries.
357359
if err := s.setFinalWithRetry(nextHeight); err != nil {
358-
s.sendCriticalError(fmt.Errorf("failed to set final height: %w", err))
359-
s.logger.Error().Err(err).Uint64("height", nextHeight).Msg("CRITICAL: Failed to set final height after retries - halting DA inclusion processing")
360-
return
360+
s.logger.Error().Err(err).Uint64("height", nextHeight).Msg("failed to set final height after retries, will retry on next tick")
361+
break
361362
}
362363

363364
// Persist DA included height before advancing in-memory state
@@ -379,7 +380,7 @@ func (s *Submitter) processDAInclusionLoop() {
379380
}
380381

381382
// setFinalWithRetry sets the final height in executor with retry logic.
382-
// 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.
383+
// NOTE: the function retries the execution client call regardless of the error. Callers decide how to handle exhaustion.
383384
func (s *Submitter) setFinalWithRetry(nextHeight uint64) error {
384385
for attempt := 1; attempt <= common.MaxRetriesBeforeHalt; attempt++ {
385386
if err := s.exec.SetFinal(s.ctx, nextHeight); err != nil {

‎block/internal/submitting/submitter_test.go‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,68 @@ func TestSubmitter_processDAInclusionLoop_advances(t *testing.T) {
316316

317317
}
318318

319+
// TestSubmitter_processDAInclusionLoop_setFinalFailureDoesNotHalt ensures that
320+
// exhausting SetFinal retries holds the DA included height and retries on the
321+
// next tick instead of reporting a critical error that stops the node.
322+
func TestSubmitter_processDAInclusionLoop_setFinalFailureDoesNotHalt(t *testing.T) {
323+
synctest.Test(t, func(t *testing.T) {
324+
cm, st := newTestCacheAndStore(t)
325+
326+
cfg := config.DefaultConfig()
327+
cfg.DA.BlockTime.Duration = 5 * time.Millisecond
328+
329+
exec := testmocks.NewMockExecutor(t)
330+
exec.On("SetFinal", mock.Anything, uint64(1)).Return(errors.New("block pruned")).Times(common.MaxRetriesBeforeHalt)
331+
exec.On("SetFinal", mock.Anything, uint64(1)).Return(nil).Once()
332+
exec.On("SetFinal", mock.Anything, uint64(2)).Return(nil).Once()
333+
334+
errCh := make(chan error, 1)
335+
ctx, cancel := context.WithCancel(t.Context())
336+
s := &Submitter{
337+
store: st,
338+
exec: exec,
339+
cache: cm,
340+
metrics: common.NopMetrics(),
341+
config: cfg,
342+
daIncludedHeight: &atomic.Uint64{},
343+
errorCh: errCh,
344+
logger: zerolog.Nop(),
345+
ctx: ctx,
346+
}
347+
348+
sig := types.Signature([]byte("sig"))
349+
for height := uint64(1); height <= 2; height++ {
350+
h, d := newHeaderAndData("chain", height, true)
351+
batch, err := st.NewBatch(ctx)
352+
require.NoError(t, err)
353+
require.NoError(t, batch.SaveBlockData(h, d, &sig))
354+
require.NoError(t, batch.SetHeight(height))
355+
require.NoError(t, batch.Commit())
356+
cm.SetHeaderDAIncluded(h.Hash().String(), 100+height, height)
357+
cm.SetDataDAIncluded(d.DACommitment().String(), 100+height, height)
358+
}
359+
360+
done := make(chan struct{})
361+
go func() {
362+
s.processDAInclusionLoop()
363+
close(done)
364+
}()
365+
366+
require.Eventually(t, func() bool {
367+
return s.GetDAIncludedHeight() == 2
368+
}, time.Minute, cfg.DA.BlockTime.Duration)
369+
370+
cancel()
371+
<-done
372+
373+
select {
374+
case err := <-errCh:
375+
t.Fatalf("unexpected critical error: %v", err)
376+
default:
377+
}
378+
})
379+
}
380+
319381
// helper to create a minimal header and data for tests
320382
func newHeaderAndData(chainID string, height uint64, nonEmpty bool) (*types.SignedHeader, *types.Data) {
321383
now := time.Now()

‎execution/evm/execution.go‎

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"sync/atomic"
1515
"time"
1616

17+
"github.com/ethereum/go-ethereum"
1718
"github.com/ethereum/go-ethereum/beacon/engine"
1819
"github.com/ethereum/go-ethereum/common"
1920
"github.com/ethereum/go-ethereum/common/hexutil"
@@ -584,13 +585,48 @@ func (c *EngineClient) doForkchoiceUpdate(ctx context.Context, args engine.Forkc
584585
return nil
585586
}
586587

587-
// SetFinal marks the block at the given height as finalized
588+
// SetFinal marks the block at the given height as finalized.
589+
//
590+
// When DA inclusion lags far behind the head, the requested height may no
591+
// longer be served by the execution layer (pruned history). If the lookup
592+
// reports the block as pruned or not found and the EL head is at or above the
593+
// height, the update is skipped: a later height will move the finalized label
594+
// forward. Any other lookup failure (e.g. transient RPC errors), lookups above
595+
// the head, or an unreadable head are returned as errors so the caller retries.
588596
func (c *EngineClient) SetFinal(ctx context.Context, blockHeight uint64) error {
589597
blockHash, _, _, _, err := c.getBlockInfo(ctx, blockHeight)
590-
if err != nil {
598+
if err == nil {
599+
return c.setFinal(ctx, blockHash, true)
600+
}
601+
if !isHistoryUnavailable(err) {
591602
return fmt.Errorf("failed to get block info: %w", err)
592603
}
593-
return c.setFinal(ctx, blockHash, true)
604+
605+
head, headErr := c.ethClient.HeaderByNumber(ctx, nil)
606+
if headErr != nil || head == nil || head.Number == nil || !head.Number.IsUint64() || head.Number.Uint64() < blockHeight {
607+
return fmt.Errorf("failed to get block info: %w", err)
608+
}
609+
610+
c.logger.Warn().
611+
Uint64("height", blockHeight).
612+
Uint64("el_head", head.Number.Uint64()).
613+
Err(err).
614+
Msg("SetFinal: historical block unavailable on execution layer, skipping finalized update")
615+
return nil
616+
}
617+
618+
// prunedHistoryErrorCode is the JSON-RPC error code execution clients (geth,
619+
// reth) return when a request touches pruned history (EIP-4444).
620+
const prunedHistoryErrorCode = 4444
621+
622+
// isHistoryUnavailable reports whether err means the execution layer no longer
623+
// serves the requested block, as opposed to a transient failure.
624+
func isHistoryUnavailable(err error) bool {
625+
if errors.Is(err, ethereum.NotFound) {
626+
return true
627+
}
628+
var rpcErr rpc.Error
629+
return errors.As(err, &rpcErr) && rpcErr.ErrorCode() == prunedHistoryErrorCode
594630
}
595631

596632
// SetSafe explicitly sets the safe block hash.
Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
package evm
2+
3+
import (
4+
"context"
5+
"errors"
6+
"math/big"
7+
"testing"
8+
9+
"github.com/ethereum/go-ethereum"
10+
"github.com/ethereum/go-ethereum/beacon/engine"
11+
"github.com/ethereum/go-ethereum/common"
12+
"github.com/ethereum/go-ethereum/core/types"
13+
"github.com/rs/zerolog"
14+
"github.com/stretchr/testify/require"
15+
)
16+
17+
func TestSetFinal_UnavailableBlock(t *testing.T) {
18+
prunedErr := &testRPCError{code: prunedHistoryErrorCode, msg: "pruned history unavailable"}
19+
20+
specs := map[string]struct {
21+
lookupErr error
22+
head *big.Int
23+
headErr error
24+
expectErr bool
25+
}{
26+
"skip_pruned_below_head": {
27+
lookupErr: prunedErr,
28+
head: big.NewInt(1_000),
29+
},
30+
"skip_not_found_below_head": {
31+
lookupErr: ethereum.NotFound,
32+
head: big.NewInt(1_000),
33+
},
34+
"error_transient_lookup_below_head": {
35+
lookupErr: errors.New("i/o timeout"),
36+
head: big.NewInt(1_000),
37+
expectErr: true,
38+
},
39+
"error_other_rpc_code_below_head": {
40+
lookupErr: &testRPCError{code: -32000, msg: "internal error"},
41+
head: big.NewInt(1_000),
42+
expectErr: true,
43+
},
44+
"error_pruned_above_head": {
45+
lookupErr: prunedErr,
46+
head: big.NewInt(50),
47+
expectErr: true,
48+
},
49+
"error_pruned_head_unavailable": {
50+
lookupErr: prunedErr,
51+
headErr: errors.New("connection refused"),
52+
expectErr: true,
53+
},
54+
}
55+
for name, spec := range specs {
56+
t.Run(name, func(t *testing.T) {
57+
engineRPC := &mockSetFinalEngineRPCClient{}
58+
client := &EngineClient{
59+
engineClient: engineRPC,
60+
ethClient: mockSetFinalEthRPCClient{
61+
lookupErr: spec.lookupErr,
62+
head: spec.head,
63+
headErr: spec.headErr,
64+
},
65+
logger: zerolog.Nop(),
66+
}
67+
68+
err := client.SetFinal(t.Context(), 100)
69+
70+
if spec.expectErr {
71+
require.Error(t, err)
72+
} else {
73+
require.NoError(t, err)
74+
}
75+
require.Zero(t, engineRPC.forkchoiceCalls)
76+
})
77+
}
78+
}
79+
80+
// mockSetFinalEthRPCClient fails every numbered lookup with lookupErr and
81+
// serves the configured head for latest-block queries.
82+
type mockSetFinalEthRPCClient struct {
83+
lookupErr error
84+
head *big.Int
85+
headErr error
86+
}
87+
88+
func (m mockSetFinalEthRPCClient) HeaderByNumber(_ context.Context, number *big.Int) (*types.Header, error) {
89+
if number != nil {
90+
return nil, m.lookupErr
91+
}
92+
if m.headErr != nil {
93+
return nil, m.headErr
94+
}
95+
return &types.Header{Number: m.head}, nil
96+
}
97+
98+
func (mockSetFinalEthRPCClient) GetTxs(_ context.Context) ([]string, error) {
99+
return nil, errors.New("unexpected GetTxs call")
100+
}
101+
102+
func (mockSetFinalEthRPCClient) GetNextProposer(_ context.Context, _ *big.Int) (common.Hash, error) {
103+
return common.Hash{}, errors.New("unexpected GetNextProposer call")
104+
}
105+
106+
type mockSetFinalEngineRPCClient struct {
107+
forkchoiceCalls int
108+
}
109+
110+
func (m *mockSetFinalEngineRPCClient) ForkchoiceUpdated(_ context.Context, _ engine.ForkchoiceStateV1, _ map[string]any) (*engine.ForkChoiceResponse, error) {
111+
m.forkchoiceCalls++
112+
return nil, errors.New("unexpected ForkchoiceUpdated call")
113+
}
114+
115+
func (m *mockSetFinalEngineRPCClient) GetPayload(_ context.Context, _ engine.PayloadID) (*EnginePayloadEnvelope, error) {
116+
return nil, errors.New("unexpected GetPayload call")
117+
}
118+
119+
func (m *mockSetFinalEngineRPCClient) NewPayload(_ context.Context, _ *EnginePayloadEnvelope, _ []string, _ string, _ [][]byte) (*engine.PayloadStatusV1, error) {
120+
return nil, errors.New("unexpected NewPayload call")
121+
}

0 commit comments

Comments
 (0)