Use correct pre state to call new payload

beacon-chain/blockchain/optimistic_sync.go

@@ -8,7 +8,6 @@ import (
 	"github.com/prysmaticlabs/prysm/beacon-chain/core/blocks"
 	"github.com/prysmaticlabs/prysm/beacon-chain/core/helpers"
 	v1 "github.com/prysmaticlabs/prysm/beacon-chain/powchain/engine-api-client/v1"
-	"github.com/prysmaticlabs/prysm/beacon-chain/state"
 	"github.com/prysmaticlabs/prysm/config/params"
 	"github.com/prysmaticlabs/prysm/encoding/bytesutil"
 	enginev1 "github.com/prysmaticlabs/prysm/proto/engine/v1"
@@ -87,23 +86,21 @@ func (s *Service) notifyForkchoiceUpdate(ctx context.Context, headBlk block.Beac
 }
 
 // notifyForkchoiceUpdate signals execution engine on a new payload
-func (s *Service) notifyNewPayload(ctx context.Context, preStateVersion int, header *ethpb.ExecutionPayloadHeader, postState state.BeaconState, blk block.SignedBeaconBlock, root [32]byte) error {
+func (s *Service) notifyNewPayload(ctx context.Context, preStateVersion, postStateVersion int,
+	preStateHeader, postStateHeader *ethpb.ExecutionPayloadHeader, blk block.SignedBeaconBlock, root [32]byte) error {
 	ctx, span := trace.StartSpan(ctx, "blockChain.notifyNewPayload")
 	defer span.End()
 
-	if postState == nil {
-		return errors.New("pre and post states must not be nil")
-	}
 	// Execution payload is only supported in Bellatrix and beyond. Pre
 	// merge blocks are never optimistic
-	if isPreBellatrix(postState.Version()) {
+	if isPreBellatrix(postStateVersion) {
 		return s.cfg.ForkChoiceStore.SetOptimisticToValid(ctx, root)
 	}
 	if err := helpers.BeaconBlockIsNil(blk); err != nil {
 		return err
 	}
 	body := blk.Block().Body()
-	enabled, err := blocks.ExecutionEnabled(postState, blk.Block().Body())
+	enabled, err := blocks.IsExecutionEnabledUsingHeader(postStateHeader, body)
 	if err != nil {
 		return errors.Wrap(err, "could not determine if execution is enabled")
 	}
@@ -119,7 +116,7 @@ func (s *Service) notifyNewPayload(ctx context.Context, preStateVersion int, hea
 		switch err {
 		case v1.ErrAcceptedSyncingPayloadStatus:
 			log.WithFields(logrus.Fields{
-				"slot":      postState.Slot(),
+				"slot":      blk.Block().Slot(),
 				"blockHash": fmt.Sprintf("%#x", bytesutil.Trunc(payload.BlockHash)),
 			}).Info("Called new payload with optimistic block")
 			return nil
@@ -138,7 +135,7 @@ func (s *Service) notifyNewPayload(ctx context.Context, preStateVersion int, hea
 		// To reach here, the block must have contained a valid payload.
 		return s.validateMergeBlock(ctx, blk)
 	}
-	atTransition, err := blocks.IsMergeTransitionBlockUsingPayloadHeader(header, body)
+	atTransition, err := blocks.IsMergeTransitionBlockUsingPayloadHeader(preStateHeader, body)
 	if err != nil {
 		return errors.Wrap(err, "could not check if merge block is terminal")
 	}

beacon-chain/blockchain/optimistic_sync_test.go

@@ -218,11 +218,6 @@ func Test_NotifyNewPayload(t *testing.T) {
 			postState: altairState,
 			preState:  altairState,
 		},
-		{
-			name:      "nil post state",
-			preState:  phase0State,
-			errString: "pre and post states must not be nil",
-		},
 		{
 			name:      "nil beacon block",
 			postState: bellatrixState,
@@ -348,7 +343,9 @@ func Test_NotifyNewPayload(t *testing.T) {
 			}
 			root := [32]byte{'a'}
 			require.NoError(t, service.cfg.ForkChoiceStore.InsertOptimisticBlock(ctx, 0, root, root, params.BeaconConfig().ZeroHash, 0, 0))
-			err = service.notifyNewPayload(ctx, tt.preState.Version(), payload, tt.postState, tt.blk, root)
+			postVersion, postHeader, err := getStateVersionAndPayload(tt.postState)
+			require.NoError(t, err)
+			err = service.notifyNewPayload(ctx, tt.preState.Version(), postVersion, payload, postHeader, tt.blk, root)
 			if tt.errString != "" {
 				require.ErrorContains(t, tt.errString, err)
 			} else {
@@ -398,7 +395,9 @@ func Test_NotifyNewPayload_SetOptimisticToValid(t *testing.T) {
 	require.NoError(t, err)
 	root := [32]byte{'c'}
 	require.NoError(t, service.cfg.ForkChoiceStore.InsertOptimisticBlock(ctx, 1, root, [32]byte{'a'}, params.BeaconConfig().ZeroHash, 0, 0))
-	err = service.notifyNewPayload(ctx, bellatrixState.Version(), payload, bellatrixState, bellatrixBlk, root)
+	postVersion, postHeader, err := getStateVersionAndPayload(bellatrixState)
+	require.NoError(t, err)
+	err = service.notifyNewPayload(ctx, bellatrixState.Version(), postVersion, payload, postHeader, bellatrixBlk, root)
 	require.NoError(t, err)
 	optimistic, err := service.IsOptimisticForRoot(ctx, root)
 	require.NoError(t, err)

beacon-chain/blockchain/process_block.go

@@ -110,7 +110,11 @@ func (s *Service) onBlock(ctx context.Context, signed block.SignedBeaconBlock, b
 	if err := s.savePostStateInfo(ctx, blockRoot, signed, postState, false /* reg sync */); err != nil {
 		return err
 	}
-	if err := s.notifyNewPayload(ctx, preStateVersion, preStateHeader, postState, signed, blockRoot); err != nil {
+	postStateVersion, postStateHeader, err := getStateVersionAndPayload(postState)
+	if err != nil {
+		return err
+	}
+	if err := s.notifyNewPayload(ctx, preStateVersion, postStateVersion, preStateHeader, postStateHeader, signed, blockRoot); err != nil {
 		return errors.Wrap(err, "could not verify new payload")
 	}
 
@@ -256,14 +260,17 @@ func (s *Service) onBlock(ctx context.Context, signed block.SignedBeaconBlock, b
 	return s.handleEpochBoundary(ctx, postState)
 }
 
-func getStateVersionAndPayload(preState state.BeaconState) (int, *ethpb.ExecutionPayloadHeader, error) {
+func getStateVersionAndPayload(st state.BeaconState) (int, *ethpb.ExecutionPayloadHeader, error) {
+	if st == nil {
+		return 0, nil, errors.New("nil state")
+	}
 	var preStateHeader *ethpb.ExecutionPayloadHeader
 	var err error
-	preStateVersion := preState.Version()
+	preStateVersion := st.Version()
 	switch preStateVersion {
 	case version.Phase0, version.Altair:
 	default:
-		preStateHeader, err = preState.LatestExecutionPayloadHeader()
+		preStateHeader, err = st.LatestExecutionPayloadHeader()
 		if err != nil {
 			return 0, nil, err
 		}
@@ -308,9 +315,24 @@ func (s *Service) onBlockBatch(ctx context.Context, blks []block.SignedBeaconBlo
 		PublicKeys: []bls.PublicKey{},
 		Messages:   [][32]byte{},
 	}
+	type versionAndHeader struct {
+		version int
+		header  *ethpb.ExecutionPayloadHeader
+	}
+	preVersionAndHeaders := make([]*versionAndHeader, len(blks))
+	postVersionAndHeaders := make([]*versionAndHeader, len(blks))
 	var set *bls.SignatureBatch
 	boundaries := make(map[[32]byte]state.BeaconState)
 	for i, b := range blks {
+		v, h, err := getStateVersionAndPayload(preState)
+		if err != nil {
+			return nil, nil, err
+		}
+		preVersionAndHeaders[i] = &versionAndHeader{
+			version: v,
+			header:  h,
+		}
+
 		set, preState, err = transition.ExecuteStateTransitionNoVerifyAnySig(ctx, preState, b)
 		if err != nil {
 			return nil, nil, err
@@ -321,6 +343,15 @@ func (s *Service) onBlockBatch(ctx context.Context, blks []block.SignedBeaconBlo
 		}
 		jCheckpoints[i] = preState.CurrentJustifiedCheckpoint()
 		fCheckpoints[i] = preState.FinalizedCheckpoint()
+
+		v, h, err = getStateVersionAndPayload(preState)
+		if err != nil {
+			return nil, nil, err
+		}
+		postVersionAndHeaders[i] = &versionAndHeader{
+			version: v,
+			header:  h,
+		}
 		sigSet.Join(set)
 	}
 	verify, err := sigSet.Verify()
@@ -333,22 +364,23 @@ func (s *Service) onBlockBatch(ctx context.Context, blks []block.SignedBeaconBlo
 
 	// blocks have been verified, add them to forkchoice and call the engine
 	for i, b := range blks {
-		preStateVersion, preStateHeader, err := getStateVersionAndPayload(preState)
-		if err != nil {
-			return nil, nil, err
-		}
-
 		s.saveInitSyncBlock(blockRoots[i], b)
 		if err := s.insertBlockToForkChoiceStore(ctx, b.Block(), blockRoots[i], fCheckpoints[i], jCheckpoints[i]); err != nil {
 			return nil, nil, err
 		}
-		if err := s.notifyNewPayload(ctx, preStateVersion, preStateHeader, preState, b, blockRoots[i]); err != nil {
+		if err := s.notifyNewPayload(ctx,
+			preVersionAndHeaders[i].version,
+			postVersionAndHeaders[i].version,
+			preVersionAndHeaders[i].header,
+			postVersionAndHeaders[i].header, b, blockRoots[i]); err != nil {
 			return nil, nil, err
 		}
+
 		if _, err := s.notifyForkchoiceUpdate(ctx, b.Block(), blockRoots[i], bytesutil.ToBytes32(fCheckpoints[i].Root)); err != nil {
 			return nil, nil, err
 		}
 	}
+
 	for r, st := range boundaries {
 		if err := s.cfg.StateGen.SaveState(ctx, r, st); err != nil {
 			return nil, nil, err

beacon-chain/core/blocks/payload.go

@@ -14,6 +14,7 @@ import (
 	enginev1 "github.com/prysmaticlabs/prysm/proto/engine/v1"
 	ethpb "github.com/prysmaticlabs/prysm/proto/prysm/v1alpha1"
 	"github.com/prysmaticlabs/prysm/proto/prysm/v1alpha1/block"
+	"github.com/prysmaticlabs/prysm/runtime/version"
 	"github.com/prysmaticlabs/prysm/time/slots"
 )
 
@@ -85,15 +86,25 @@ func ExecutionBlock(body block.BeaconBlockBody) (bool, error) {
 // Spec code:
 // def is_execution_enabled(state: BeaconState, body: BeaconBlockBody) -> bool:
 //    return is_merge_block(state, body) or is_merge_complete(state)
+// Deprecated: Use `IsExecutionEnabledUsingHeader` instead.
 func ExecutionEnabled(st state.BeaconState, body block.BeaconBlockBody) (bool, error) {
-	mergeBlock, err := MergeTransitionBlock(st, body)
+	if st.Version() == version.Phase0 || st.Version() == version.Altair {
+		return false, nil
+	}
+	header, err := st.LatestExecutionPayloadHeader()
 	if err != nil {
 		return false, err
 	}
-	if mergeBlock {
+	return IsExecutionEnabledUsingHeader(header, body)
+}
+
+// IsExecutionEnabledUsingHeader returns true if the execution is enabled using post processed payload header and block body.
+// This is an optimized version of ExecutionEnabled where beacon state is not required as an argument.
+func IsExecutionEnabledUsingHeader(header *ethpb.ExecutionPayloadHeader, body block.BeaconBlockBody) (bool, error) {
+	if !isEmptyHeader(header) {
 		return true, nil
 	}
-	return MergeTransitionComplete(st)
+	return ExecutionBlock(body)
 }
 
 // ValidatePayloadWhenMergeCompletes validates if payload is valid versus input beacon state.

beacon-chain/core/blocks/payload_test.go

@@ -564,6 +564,82 @@ func Test_IsExecutionBlock(t *testing.T) {
 }
 
 func Test_ExecutionEnabled(t *testing.T) {
+	tests := []struct {
+		name        string
+		payload     *enginev1.ExecutionPayload
+		header      *ethpb.ExecutionPayloadHeader
+		useAltairSt bool
+		want        bool
+	}{
+		{
+			name:        "use older than bellatrix state",
+			payload:     emptyPayload(),
+			header:      emptyPayloadHeader(),
+			useAltairSt: true,
+			want:        false,
+		},
+		{
+			name:    "empty header, empty payload",
+			payload: emptyPayload(),
+			header:  emptyPayloadHeader(),
+			want:    false,
+		},
+		{
+			name:    "non-empty header, empty payload",
+			payload: emptyPayload(),
+			header: func() *ethpb.ExecutionPayloadHeader {
+				h := emptyPayloadHeader()
+				h.ParentHash = bytesutil.PadTo([]byte{'a'}, fieldparams.RootLength)
+				return h
+			}(),
+			want: true,
+		},
+		{
+			name:   "empty header, non-empty payload",
+			header: emptyPayloadHeader(),
+			payload: func() *enginev1.ExecutionPayload {
+				p := emptyPayload()
+				p.Timestamp = 1
+				return p
+			}(),
+			want: true,
+		},
+		{
+			name: "non-empty header, non-empty payload",
+			header: func() *ethpb.ExecutionPayloadHeader {
+				h := emptyPayloadHeader()
+				h.ParentHash = bytesutil.PadTo([]byte{'a'}, fieldparams.RootLength)
+				return h
+			}(),
+			payload: func() *enginev1.ExecutionPayload {
+				p := emptyPayload()
+				p.Timestamp = 1
+				return p
+			}(),
+			want: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			st, _ := util.DeterministicGenesisStateBellatrix(t, 1)
+			require.NoError(t, st.SetLatestExecutionPayloadHeader(tt.header))
+			blk := util.NewBeaconBlockBellatrix()
+			blk.Block.Body.ExecutionPayload = tt.payload
+			body, err := wrapper.WrappedBellatrixBeaconBlockBody(blk.Block.Body)
+			require.NoError(t, err)
+			if tt.useAltairSt {
+				st, _ = util.DeterministicGenesisStateAltair(t, 1)
+			}
+			got, err := blocks.ExecutionEnabled(st, body)
+			require.NoError(t, err)
+			if got != tt.want {
+				t.Errorf("ExecutionEnabled() got = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func Test_IsExecutionEnabledUsingHeader(t *testing.T) {
 	tests := []struct {
 		name    string
 		payload *enginev1.ExecutionPayload
@@ -613,13 +689,11 @@ func Test_ExecutionEnabled(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			st, _ := util.DeterministicGenesisStateBellatrix(t, 1)
-			require.NoError(t, st.SetLatestExecutionPayloadHeader(tt.header))
 			blk := util.NewBeaconBlockBellatrix()
 			blk.Block.Body.ExecutionPayload = tt.payload
 			body, err := wrapper.WrappedBellatrixBeaconBlockBody(blk.Block.Body)
 			require.NoError(t, err)
-			got, err := blocks.ExecutionEnabled(st, body)
+			got, err := blocks.IsExecutionEnabledUsingHeader(tt.header, body)
 			require.NoError(t, err)
 			if got != tt.want {
 				t.Errorf("ExecutionEnabled() got = %v, want %v", got, tt.want)