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 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..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" @@ -584,13 +585,48 @@ 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 (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 { + if err == nil { + return c.setFinal(ctx, blockHash, true) + } + if !isHistoryUnavailable(err) { return fmt.Errorf("failed to get block info: %w", err) } - 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) + } + + 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 +} + +// 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. diff --git a/execution/evm/execution_setfinal_test.go b/execution/evm/execution_setfinal_test.go new file mode 100644 index 0000000000..7382488488 --- /dev/null +++ b/execution/evm/execution_setfinal_test.go @@ -0,0 +1,121 @@ +package evm + +import ( + "context" + "errors" + "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" + "github.com/rs/zerolog" + "github.com/stretchr/testify/require" +) + +func TestSetFinal_UnavailableBlock(t *testing.T) { + prunedErr := &testRPCError{code: prunedHistoryErrorCode, msg: "pruned history unavailable"} + + specs := map[string]struct { + lookupErr error + head *big.Int + headErr error + expectErr bool + }{ + "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_pruned_above_head": { + lookupErr: prunedErr, + head: big.NewInt(50), + expectErr: true, + }, + "error_pruned_head_unavailable": { + lookupErr: prunedErr, + 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{ + lookupErr: spec.lookupErr, + 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.Zero(t, engineRPC.forkchoiceCalls) + }) + } +} + +// mockSetFinalEthRPCClient fails every numbered lookup with lookupErr and +// serves the configured head for latest-block queries. +type mockSetFinalEthRPCClient struct { + lookupErr error + head *big.Int + headErr error +} + +func (m mockSetFinalEthRPCClient) HeaderByNumber(_ context.Context, number *big.Int) (*types.Header, error) { + if number != nil { + return nil, m.lookupErr + } + 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") +}