fix(engine): scope engine-API retry budget to consensus phase

Author: fridrik01

beacon/blockchain/execution_engine.go

@@ -63,7 +63,7 @@ func (s *Service) sendPostBlockFCU(
 		fcuData,
 		s.chainSpec.ActiveForkVersionForTimestamp(lph.GetTimestamp()),
 	)
-	if _, err = s.executionEngine.NotifyForkchoiceUpdate(ctx, req); err != nil {
+	if _, err = s.executionEngine.NotifyForkchoiceUpdate(ctx, req, engineprimitives.PhaseFinalize); err != nil {
 		return fmt.Errorf("failed forkchoice update, head %s: %w",
 			lph.GetBlockHash().String(),
 			err,

beacon/blockchain/finalize_block.go

@@ -29,6 +29,7 @@ import (
 	ctypes "github.com/berachain/beacon-kit/consensus-types/types"
 	"github.com/berachain/beacon-kit/consensus/types"
 	datypes "github.com/berachain/beacon-kit/da/types"
+	engineprimitives "github.com/berachain/beacon-kit/engine-primitives/engine-primitives"
 	"github.com/berachain/beacon-kit/primitives/crypto"
 	"github.com/berachain/beacon-kit/primitives/math"
 	"github.com/berachain/beacon-kit/primitives/transition"
@@ -214,6 +215,7 @@ func (s *Service) executeStateTransition(
 		ctx,
 		blk.GetConsensusTime(),
 		blk.GetProposerAddress(),
+		engineprimitives.PhaseFinalize,
 	).
 		WithVerifyPayload(true).
 		WithVerifyRandao(false).

beacon/blockchain/interfaces.go

@@ -51,13 +51,14 @@ type ExecutionEngine interface {
 	NotifyNewPayload(
 		ctx context.Context,
 		req ctypes.NewPayloadRequest,
-		retryOnSyncingStatus bool,
+		phase engineprimitives.EnginePhase,
 	) error
 	// NotifyForkchoiceUpdate notifies the execution client of a forkchoice
 	// update.
 	NotifyForkchoiceUpdate(
 		ctx context.Context,
 		req *ctypes.ForkchoiceUpdateRequest,
+		phase engineprimitives.EnginePhase,
 	) (*engineprimitives.PayloadID, error)
 }
 

beacon/blockchain/payload.go

@@ -27,8 +27,6 @@ import (
 	payloadtime "github.com/berachain/beacon-kit/beacon/payload-time"
 	ctypes "github.com/berachain/beacon-kit/consensus-types/types"
 	engineprimitives "github.com/berachain/beacon-kit/engine-primitives/engine-primitives"
-	engineerrors "github.com/berachain/beacon-kit/engine-primitives/errors"
-	"github.com/berachain/beacon-kit/errors"
 	"github.com/berachain/beacon-kit/payload/builder"
 	"github.com/berachain/beacon-kit/primitives/crypto"
 	"github.com/berachain/beacon-kit/primitives/math"
@@ -66,7 +64,7 @@ func (s *Service) forceSyncUponProcess(
 		},
 		s.chainSpec.ActiveForkVersionForTimestamp(lph.GetTimestamp()),
 	)
-	if _, err = s.executionEngine.NotifyForkchoiceUpdate(ctx, req); err != nil {
+	if _, err = s.executionEngine.NotifyForkchoiceUpdate(ctx, req, engineprimitives.PhaseStartup); err != nil {
 		s.logger.Error(
 			"failed to send force head FCU",
 			"error", err,
@@ -93,9 +91,10 @@ func (s *Service) forceSyncUponFinalize(
 		return err
 	}
 
-	// We set retryOnSyncingStatus to false here. We can ignore SYNCING status and proceed
-	// to the FCU.
-	err = s.executionEngine.NotifyNewPayload(ctx, payloadReq, false)
+	// EL may return SYNCING (payload accepted but not yet validated — it's
+	// still catching up). Fine at startup: the block is already finalized
+	// in our state, and the FCU below kicks off the EL sync.
+	err = s.executionEngine.NotifyNewPayload(ctx, payloadReq, engineprimitives.PhaseStartup)
 	if err != nil {
 		return fmt.Errorf("startSyncUponFinalize NotifyNewPayload failed: %w", err)
 	}
@@ -111,22 +110,10 @@ func (s *Service) forceSyncUponFinalize(
 		s.chainSpec.ActiveForkVersionForTimestamp(executionPayload.GetTimestamp()),
 	)
 
-	switch _, err = s.executionEngine.NotifyForkchoiceUpdate(ctx, req); {
-	case err == nil:
-		return nil
-
-	case errors.IsAny(err,
-		engineerrors.ErrSyncingPayloadStatus,
-		engineerrors.ErrAcceptedPayloadStatus):
-		s.logger.Warn(
-			//nolint:lll // long message on one line for readability.
-			`Your execution client is syncing. It should be downloading eth blocks from its peers. Restart the beacon node once the execution client is caught up.`,
-		)
-		return err
-
-	default:
+	if _, err = s.executionEngine.NotifyForkchoiceUpdate(ctx, req, engineprimitives.PhaseStartup); err != nil {
 		return fmt.Errorf("force startup NotifyForkchoiceUpdate failed: %w", err)
 	}
+	return nil
 }
 
 // Once you provide the right state, we really need to carry out the very same operations

beacon/blockchain/payload_test.go

@@ -85,7 +85,7 @@ func TestOptimisticBlockBuildingRejectedBlockStateChecks(t *testing.T) {
 	// Since this is the first block called post genesis
 	// forceSyncUponProcess will be called.
 	dummyPayloadID := &engineprimitives.PayloadID{1, 2, 3}
-	eng.EXPECT().NotifyForkchoiceUpdate(mock.Anything, mock.Anything).Return(dummyPayloadID, nil)
+	eng.EXPECT().NotifyForkchoiceUpdate(mock.Anything, mock.Anything, mock.Anything).Return(dummyPayloadID, nil)
 
 	// we set just enough data in invalid block to let it pass
 	// the first validations in chain before state processor is invoked
@@ -175,7 +175,7 @@ func TestOptimisticBlockBuildingVerifiedBlockStateChecks(t *testing.T) {
 	// Since this is the first block called post genesis
 	// forceSyncUponProcess will be called.
 	dummyPayloadID := &engineprimitives.PayloadID{1, 2, 3}
-	eng.EXPECT().NotifyForkchoiceUpdate(mock.Anything, mock.Anything).Return(dummyPayloadID, nil)
+	eng.EXPECT().NotifyForkchoiceUpdate(mock.Anything, mock.Anything, mock.Anything).Return(dummyPayloadID, nil)
 
 	// BUILD A VALID BLOCK (without polluting state st)
 	sdkCtx := sdk.NewContext(cms.CacheMultiStore(), true, log.NewNopLogger())
@@ -403,6 +403,7 @@ func computeStateRoot(
 		ctx,
 		consensusTime,
 		proposerAddress,
+		engineprimitives.PhaseBuild,
 	).
 		WithVerifyPayload(false).
 		WithVerifyRandao(false).

beacon/blockchain/process_proposal.go

@@ -403,6 +403,7 @@ func (s *Service) verifyStateRoot(
 		ctx,
 		blk.GetConsensusTime(),
 		blk.GetProposerAddress(),
+		engineprimitives.PhaseValidate,
 	).
 		WithVerifyPayload(true).
 		WithVerifyRandao(true).

beacon/validator/block_builder.go

@@ -418,6 +418,7 @@ func (s *Service) computeStateRoot(
 		ctx,
 		consensusTime,
 		proposerAddress,
+		engineprimitives.PhaseBuild,
 	).
 		WithVerifyPayload(false).
 		WithVerifyRandao(false).

engine-primitives/engine-primitives/phase.go

@@ -0,0 +1,68 @@
+// SPDX-License-Identifier: BUSL-1.1
+//
+// Copyright (C) 2025, Berachain Foundation. All rights reserved.
+// Use of this software is governed by the Business Source License included
+// in the LICENSE file of this repository and at www.mariadb.com/bsl11.
+//
+// ANY USE OF THE LICENSED WORK IN VIOLATION OF THIS LICENSE WILL AUTOMATICALLY
+// TERMINATE YOUR RIGHTS UNDER THIS LICENSE FOR THE CURRENT AND ALL OTHER
+// VERSIONS OF THE LICENSED WORK.
+//
+// THIS LICENSE DOES NOT GRANT YOU ANY RIGHT IN ANY TRADEMARK OR LOGO OF
+// LICENSOR OR ITS AFFILIATES (PROVIDED THAT YOU MAY USE A TRADEMARK OR LOGO OF
+// LICENSOR AS EXPRESSLY REQUIRED BY THIS LICENSE).
+//
+// TO THE EXTENT PERMITTED BY APPLICABLE LAW, THE LICENSED WORK IS PROVIDED ON
+// AN "AS IS" BASIS. LICENSOR HEREBY DISCLAIMS ALL WARRANTIES AND CONDITIONS,
+// EXPRESS OR IMPLIED, INCLUDING (WITHOUT LIMITATION) WARRANTIES OF
+// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE, NON-INFRINGEMENT, AND
+// TITLE.
+
+package engineprimitives
+
+// EnginePhase tags an engine API call with the consensus phase that issued it,
+// so the retry wrapper can pick a budget that fits the phase's safety/liveness
+// requirements:
+//
+//   - PhaseBuild and PhaseValidate are bounded: a stuck call must return so
+//     CometBFT can move to a new round/proposer. This is what closes the
+//     malicious-payload retry-loop class of bugs.
+//   - PhaseFinalize is unbounded: the block is already agreed by >=2/3 of
+//     validators, so the node must eventually apply it or fall out of
+//     consensus. A brief EL outage is absorbed by retrying; the loop logs
+//     loudly so operator alerting can catch a persistently stuck node.
+//   - PhaseStartup has an unbounded time budget so a slow cold-starting EL
+//     (re-importing chain state, snap-syncing) is acceptable. Fatal errors
+//     still propagate immediately — a misconfigured boot (wrong JWT, wrong
+//     chain ID) fails fast rather than hot-looping before serving consensus.
+type EnginePhase int
+
+const (
+	// PhaseBuild is used from PrepareProposal when this node is the proposer
+	// and is asking the EL to start building a payload.
+	PhaseBuild EnginePhase = iota
+	// PhaseValidate is used from ProcessProposal when validating a block
+	// proposed by another validator.
+	PhaseValidate
+	// PhaseFinalize is used from FinalizeBlock and the post-block FCU when
+	// applying a block that consensus has already agreed on.
+	PhaseFinalize
+	// PhaseStartup is used from one-shot startup paths (forceSyncUponProcess
+	// / forceSyncUponFinalize).
+	PhaseStartup
+)
+
+func (p EnginePhase) String() string {
+	switch p {
+	case PhaseBuild:
+		return "build"
+	case PhaseValidate:
+		return "validate"
+	case PhaseFinalize:
+		return "finalize"
+	case PhaseStartup:
+		return "startup"
+	default:
+		return "unknown"
+	}
+}

execution/client/errors.go

@@ -21,6 +21,8 @@
 package client
 
 import (
+	nethttp "net/http"
+
 	engineerrors "github.com/berachain/beacon-kit/engine-primitives/errors"
 	"github.com/berachain/beacon-kit/errors"
 	"github.com/berachain/beacon-kit/execution/client/ethclient/rpc"
@@ -76,7 +78,14 @@ func (s *EngineClient) handleRPCError(
 		var httpErr *rpc.HTTPStatusError
 		if errors.As(err, &httpErr) && httpErr != nil &&
 			httpErr.StatusCode >= 400 && httpErr.StatusCode < 500 {
-			// HTTP 4xx is a request-level rejection; retrying with the same payload will never
+			// 408/425/429 are RFC-defined retryable signals — a reverse proxy,
+			// rate limiter, or WAF in front of the EL must not turn brief
+			// backpressure into missed slots. Treat as transport-level transient.
+			switch httpErr.StatusCode {
+			case nethttp.StatusRequestTimeout, nethttp.StatusTooEarly, nethttp.StatusTooManyRequests:
+				return errors.Join(ErrBadConnection, err)
+			}
+			// Other 4xx is a request-level rejection; retrying with the same payload will never
 			// succeed. Fatal so the proposer skips the slot instead of looping.
 			return errors.Join(ErrHTTPClientError, err)
 		}

execution/client/errors_test.go

@@ -68,20 +68,53 @@ func TestHandleRPCError_Classification(t *testing.T) {
 			wantRetry: false,
 		},
 		{
-			name:      "HTTP 413 (PoC) → ErrHTTPClientError (fatal)",
-			in:        &rpc.HTTPStatusError{StatusCode: 413, Body: `{"code":-32007,"message":"Request is too big"}`},
+			name:      "HTTP 400 → ErrHTTPClientError (fatal, malformed request)",
+			in:        &rpc.HTTPStatusError{StatusCode: 400, Body: ""},
 			wantIs:    ErrHTTPClientError,
 			wantFatal: true,
 		},
 		{
-			name:      "HTTP 499 → ErrHTTPClientError (fatal)",
-			in:        &rpc.HTTPStatusError{StatusCode: 499, Body: ""},
+			name:      "HTTP 401 → ErrHTTPClientError (fatal, JWT misconfig)",
+			in:        &rpc.HTTPStatusError{StatusCode: 401, Body: ""},
 			wantIs:    ErrHTTPClientError,
 			wantFatal: true,
 		},
 		{
-			name:      "HTTP 401 → ErrHTTPClientError (fatal)",
-			in:        &rpc.HTTPStatusError{StatusCode: 401, Body: ""},
+			name:      "HTTP 404 → ErrHTTPClientError (fatal, wrong RPC path)",
+			in:        &rpc.HTTPStatusError{StatusCode: 404, Body: ""},
+			wantIs:    ErrHTTPClientError,
+			wantFatal: true,
+		},
+		{
+			name:      "HTTP 408 → ErrBadConnection (retryable, idle-conn cleanup from proxy)",
+			in:        &rpc.HTTPStatusError{StatusCode: 408, Body: ""},
+			wantIs:    ErrBadConnection,
+			wantFatal: false,
+			wantRetry: true,
+		},
+		{
+			name:      "HTTP 413 (PoC) → ErrHTTPClientError (fatal)",
+			in:        &rpc.HTTPStatusError{StatusCode: 413, Body: `{"code":-32007,"message":"Request is too big"}`},
+			wantIs:    ErrHTTPClientError,
+			wantFatal: true,
+		},
+		{
+			name:      "HTTP 425 → ErrBadConnection (retryable, TLS 0-RTT replay)",
+			in:        &rpc.HTTPStatusError{StatusCode: 425, Body: ""},
+			wantIs:    ErrBadConnection,
+			wantFatal: false,
+			wantRetry: true,
+		},
+		{
+			name:      "HTTP 429 → ErrBadConnection (retryable, rate limited by proxy/WAF)",
+			in:        &rpc.HTTPStatusError{StatusCode: 429, Body: ""},
+			wantIs:    ErrBadConnection,
+			wantFatal: false,
+			wantRetry: true,
+		},
+		{
+			name:      "HTTP 499 → ErrHTTPClientError (fatal, non-retryable 4xx catch-all)",
+			in:        &rpc.HTTPStatusError{StatusCode: 499, Body: ""},
 			wantIs:    ErrHTTPClientError,
 			wantFatal: true,
 		},