Use justified hash as safe block hash when passing attribute (#12196)

* With attribute, use justified hash as safe block hash

* Use UnrealizedJustifiedPayloadBlockHash

* Cleanups

* Add test to forkchoice

* Block proposal

* Fix mock

* Fix test

* Fix rest of the tests

* Potuz feedback

* Fix test

* Use unrealized justified hash for regular fcu

beacon-chain/blockchain/chain_info.go

@@ -96,6 +96,8 @@ type FinalizationFetcher interface {
 	FinalizedCheckpt() *ethpb.Checkpoint
 	CurrentJustifiedCheckpt() *ethpb.Checkpoint
 	PreviousJustifiedCheckpt() *ethpb.Checkpoint
+	UnrealizedJustifiedPayloadBlockHash() ([32]byte, error)
+	FinalizedBlockHash() [32]byte
 	InForkchoice([32]byte) bool
 	IsFinalized(ctx context.Context, blockRoot [32]byte) bool
 }

beacon-chain/blockchain/chain_info_forkchoice.go

@@ -78,3 +78,17 @@ func (s *Service) ChainHeads() ([][32]byte, []primitives.Slot) {
 	defer s.cfg.ForkChoiceStore.RUnlock()
 	return s.cfg.ForkChoiceStore.Tips()
 }
+
+// UnrealizedJustifiedPayloadBlockHash returns unrealized justified payload block hash from forkchoice.
+func (s *Service) UnrealizedJustifiedPayloadBlockHash() ([32]byte, error) {
+	s.cfg.ForkChoiceStore.RLock()
+	defer s.cfg.ForkChoiceStore.RUnlock()
+	return s.cfg.ForkChoiceStore.UnrealizedJustifiedPayloadBlockHash()
+}
+
+// FinalizedBlockHash returns finalized payload block hash from forkchoice.
+func (s *Service) FinalizedBlockHash() [32]byte {
+	s.cfg.ForkChoiceStore.RLock()
+	defer s.cfg.ForkChoiceStore.RUnlock()
+	return s.cfg.ForkChoiceStore.FinalizedPayloadBlockHash()
+}

beacon-chain/blockchain/chain_info_test.go

@@ -119,6 +119,46 @@ func TestCurrentJustifiedCheckpt_CanRetrieve(t *testing.T) {
 	require.Equal(t, cp.Root, bytesutil.ToBytes32(jp.Root))
 }
 
+func TestFinalizedBlockHash(t *testing.T) {
+	ctx := context.Background()
+	beaconDB := testDB.SetupDB(t)
+	fcs := doublylinkedtree.New()
+	opts := []Option{
+		WithDatabase(beaconDB),
+		WithForkChoiceStore(fcs),
+		WithStateGen(stategen.New(beaconDB, fcs)),
+	}
+	service, err := NewService(ctx, opts...)
+	require.NoError(t, err)
+
+	r := [32]byte{'f'}
+	cp := &forkchoicetypes.Checkpoint{Epoch: 6, Root: r}
+	bState, _ := util.DeterministicGenesisState(t, 10)
+	require.NoError(t, beaconDB.SaveState(ctx, bState, r))
+
+	require.NoError(t, fcs.UpdateFinalizedCheckpoint(cp))
+	h := service.FinalizedBlockHash()
+	require.Equal(t, params.BeaconConfig().ZeroHash, h)
+	require.Equal(t, r, fcs.FinalizedCheckpoint().Root)
+}
+
+func TestUnrealizedJustifiedBlockHash(t *testing.T) {
+	ctx := context.Background()
+	service := &Service{cfg: &config{ForkChoiceStore: doublylinkedtree.New()}}
+	ojc := &ethpb.Checkpoint{Root: []byte{'j'}}
+	ofc := &ethpb.Checkpoint{Root: []byte{'f'}}
+	st, blkRoot, err := prepareForkchoiceState(ctx, 0, [32]byte{}, [32]byte{}, params.BeaconConfig().ZeroHash, ojc, ofc)
+	require.NoError(t, err)
+	require.NoError(t, service.cfg.ForkChoiceStore.InsertNode(ctx, st, blkRoot))
+	service.cfg.ForkChoiceStore.SetBalancesByRooter(func(_ context.Context, _ [32]byte) ([]uint64, error) { return []uint64{}, nil })
+	require.NoError(t, service.cfg.ForkChoiceStore.UpdateJustifiedCheckpoint(ctx, &forkchoicetypes.Checkpoint{Epoch: 6, Root: [32]byte{'j'}}))
+
+	h, err := service.UnrealizedJustifiedPayloadBlockHash()
+	require.NoError(t, err)
+	require.Equal(t, params.BeaconConfig().ZeroHash, h)
+	require.Equal(t, [32]byte{'j'}, service.cfg.ForkChoiceStore.JustifiedCheckpoint().Root)
+}
+
 func TestHeadSlot_CanRetrieve(t *testing.T) {
 	c := &Service{}
 	s, err := state_native.InitializeFromProtoPhase0(&ethpb.BeaconState{})

beacon-chain/blockchain/execution_engine.go

@@ -61,7 +61,11 @@ func (s *Service) notifyForkchoiceUpdate(ctx context.Context, arg *notifyForkcho
 		return nil, nil
 	}
 	finalizedHash := s.cfg.ForkChoiceStore.FinalizedPayloadBlockHash()
-	justifiedHash := s.cfg.ForkChoiceStore.JustifiedPayloadBlockHash()
+	justifiedHash, err := s.cfg.ForkChoiceStore.UnrealizedJustifiedPayloadBlockHash()
+	if err != nil {
+		log.WithError(err).Error("Could not get unrealized justified payload block hash")
+		justifiedHash = finalizedHash
+	}
 	fcs := &enginev1.ForkchoiceState{
 		HeadBlockHash:      headPayload.BlockHash(),
 		SafeBlockHash:      justifiedHash[:],

beacon-chain/blockchain/testing/mock.go

@@ -587,3 +587,13 @@ func (s *ChainService) ProposerBoost() [32]byte {
 	}
 	return [32]byte{}
 }
+
+// FinalizedBlockHash mocks the same method in the chain service
+func (s *ChainService) FinalizedBlockHash() [32]byte {
+	return [32]byte{}
+}
+
+// UnrealizedJustifiedPayloadBlockHash mocks the same method in the chain service
+func (s *ChainService) UnrealizedJustifiedPayloadBlockHash() ([32]byte, error) {
+	return [32]byte{}, nil
+}

beacon-chain/forkchoice/doubly-linked-tree/forkchoice.go

@@ -508,6 +508,17 @@ func (f *ForkChoice) JustifiedPayloadBlockHash() [32]byte {
 	return node.payloadHash
 }
 
+// UnrealizedJustifiedPayloadBlockHash returns the hash of the payload at the unrealized justified checkpoint
+func (f *ForkChoice) UnrealizedJustifiedPayloadBlockHash() ([32]byte, error) {
+	root := f.store.unrealizedJustifiedCheckpoint.Root
+	node, ok := f.store.nodeByRoot[root]
+	if !ok || node == nil {
+		// This should not happen
+		return [32]byte{}, ErrNilNode
+	}
+	return node.payloadHash, nil
+}
+
 // ForkChoiceDump returns a full dump of forkchoice.
 func (f *ForkChoice) ForkChoiceDump(ctx context.Context) (*v1.ForkChoiceDump, error) {
 	jc := &v1.Checkpoint{

beacon-chain/forkchoice/doubly-linked-tree/forkchoice_test.go

@@ -740,5 +740,22 @@ func TestForkchoice_UpdateJustifiedBalances(t *testing.T) {
 	require.Equal(t, uint64(7), f.numActiveValidators)
 	require.Equal(t, uint64(430)/32, f.store.committeeWeight)
 	require.DeepEqual(t, balances, f.justifiedBalances)
-
+}
+
+func TestForkChoice_UnrealizedJustifiedPayloadBlockHash(t *testing.T) {
+	ctx := context.Background()
+	f := setup(0, 0)
+
+	st, blkRoot, err := prepareForkchoiceState(ctx, 0, [32]byte{'a'}, params.BeaconConfig().ZeroHash, [32]byte{'A'}, 1, 1)
+	require.NoError(t, err)
+	require.NoError(t, f.InsertNode(ctx, st, blkRoot))
+
+	f.store.unrealizedJustifiedCheckpoint.Root = [32]byte{'a'}
+	got, err := f.UnrealizedJustifiedPayloadBlockHash()
+	require.NoError(t, err)
+	require.Equal(t, [32]byte{'A'}, got)
+
+	f.store.unrealizedJustifiedCheckpoint.Root = [32]byte{'b'}
+	_, err = f.UnrealizedJustifiedPayloadBlockHash()
+	require.ErrorIs(t, err, ErrNilNode)
 }

beacon-chain/forkchoice/interfaces.go

@@ -57,6 +57,7 @@ type Getter interface {
 	JustifiedCheckpoint() *forkchoicetypes.Checkpoint
 	PreviousJustifiedCheckpoint() *forkchoicetypes.Checkpoint
 	JustifiedPayloadBlockHash() [32]byte
+	UnrealizedJustifiedPayloadBlockHash() ([32]byte, error)
 	NodeCount() int
 	HighestReceivedBlockSlot() primitives.Slot
 	ReceivedBlocksLastEpoch() (uint64, error)

beacon-chain/rpc/eth/validator/validator_test.go

@@ -1033,6 +1033,7 @@ func TestProduceBlockV2(t *testing.T) {
 			TimeFetcher:            mockChainService,
 			HeadFetcher:            mockChainService,
 			OptimisticModeFetcher:  mockChainService,
+			FinalizationFetcher:    mockChainService,
 			SyncChecker:            &mockSync.Sync{IsSyncing: false},
 			BlockReceiver:          mockChainService,
 			ForkFetcher:            mockChainService,
@@ -1280,6 +1281,7 @@ func TestProduceBlockV2(t *testing.T) {
 			BlockReceiver:          mockChainService,
 			ForkFetcher:            mockChainService,
 			ForkchoiceFetcher:      mockChainService,
+			FinalizationFetcher:    mockChainService,
 			ChainStartFetcher:      mockExecutionChain,
 			Eth1InfoFetcher:        mockExecutionChain,
 			Eth1BlockFetcher:       mockExecutionChain,
@@ -1413,20 +1415,21 @@ func TestProduceBlockV2SSZ(t *testing.T) {
 
 		mockChainService := &mockChain.ChainService{State: bs, Root: parentRoot[:], ForkChoiceStore: doublylinkedtree.New()}
 		v1Alpha1Server := &v1alpha1validator.Server{
-			HeadFetcher:       mockChainService,
-			SyncChecker:       &mockSync.Sync{IsSyncing: false},
-			BlockReceiver:     mockChainService,
-			TimeFetcher:       mockChainService,
-			ForkFetcher:       mockChainService,
-			ForkchoiceFetcher: mockChainService,
-			ChainStartFetcher: &mockExecution.Chain{},
-			Eth1InfoFetcher:   &mockExecution.Chain{},
-			Eth1BlockFetcher:  &mockExecution.Chain{},
-			MockEth1Votes:     true,
-			AttPool:           attestations.NewPool(),
-			SlashingsPool:     slashings.NewPool(),
-			ExitPool:          voluntaryexits.NewPool(),
-			StateGen:          stategen.New(db, doublylinkedtree.New()),
+			HeadFetcher:         mockChainService,
+			SyncChecker:         &mockSync.Sync{IsSyncing: false},
+			BlockReceiver:       mockChainService,
+			TimeFetcher:         mockChainService,
+			ForkFetcher:         mockChainService,
+			ForkchoiceFetcher:   mockChainService,
+			FinalizationFetcher: mockChainService,
+			ChainStartFetcher:   &mockExecution.Chain{},
+			Eth1InfoFetcher:     &mockExecution.Chain{},
+			Eth1BlockFetcher:    &mockExecution.Chain{},
+			MockEth1Votes:       true,
+			AttPool:             attestations.NewPool(),
+			SlashingsPool:       slashings.NewPool(),
+			ExitPool:            voluntaryexits.NewPool(),
+			StateGen:            stategen.New(db, doublylinkedtree.New()),
 		}
 
 		proposerSlashings := make([]*ethpbalpha.ProposerSlashing, 1)
@@ -2033,6 +2036,7 @@ func TestProduceBlockV2SSZ(t *testing.T) {
 			BlockReceiver:          mockChainService,
 			ForkFetcher:            mockChainService,
 			ForkchoiceFetcher:      mockChainService,
+			FinalizationFetcher:    mockChainService,
 			ChainStartFetcher:      &mockExecution.Chain{},
 			Eth1InfoFetcher:        &mockExecution.Chain{},
 			Eth1BlockFetcher:       &mockExecution.Chain{},
@@ -3545,6 +3549,7 @@ func TestProduceBlindedBlockSSZ(t *testing.T) {
 			BlockReceiver:          mockChainService,
 			ForkFetcher:            mockChainService,
 			ForkchoiceFetcher:      mockChainService,
+			FinalizationFetcher:    mockChainService,
 			ChainStartFetcher:      &mockExecution.Chain{},
 			Eth1InfoFetcher:        &mockExecution.Chain{},
 			Eth1BlockFetcher:       &mockExecution.Chain{},

beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix_test.go

@@ -63,6 +63,7 @@ func TestServer_setExecutionData(t *testing.T) {
 	vs := &Server{
 		ExecutionEngineCaller:  &powtesting.EngineClient{PayloadIDBytes: id, ExecutionPayloadCapella: &v1.ExecutionPayloadCapella{BlockNumber: 1, Withdrawals: withdrawals}, BlockValue: big.NewInt(0)},
 		HeadFetcher:            &blockchainTest.ChainService{State: capellaTransitionState},
+		FinalizationFetcher:    &blockchainTest.ChainService{},
 		BeaconDB:               beaconDB,
 		ProposerSlotIndexCache: cache.NewProposerPayloadIDsCache(),
 	}

beacon-chain/rpc/prysm/v1alpha1/validator/proposer_execution_payload.go

@@ -115,31 +115,23 @@ func (vs *Server) getExecutionPayload(ctx context.Context, slot primitives.Slot,
 	if err != nil {
 		return nil, err
 	}
-	finalizedBlockHash := params.BeaconConfig().ZeroHash[:]
-	finalizedRoot := bytesutil.ToBytes32(st.FinalizedCheckpoint().Root)
-	if finalizedRoot != [32]byte{} { // finalized root could be zeros before the first finalized block.
-		finalizedBlock, err := vs.BeaconDB.Block(ctx, bytesutil.ToBytes32(st.FinalizedCheckpoint().Root))
+
+	finalizedBlockHash := [32]byte{}
+	justifiedBlockHash := [32]byte{}
+	// Blocks before Bellatrix don't have execution payloads. Use zeros as the hash.
+	if st.Version() >= version.Altair {
+		finalizedBlockHash = vs.FinalizationFetcher.FinalizedBlockHash()
+		justifiedBlockHash, err = vs.FinalizationFetcher.UnrealizedJustifiedPayloadBlockHash()
 		if err != nil {
-			return nil, err
-		}
-		if err := consensusblocks.BeaconBlockIsNil(finalizedBlock); err != nil {
-			return nil, err
-		}
-		switch finalizedBlock.Version() {
-		case version.Phase0, version.Altair: // Blocks before Bellatrix don't have execution payloads. Use zeros as the hash.
-		default:
-			finalizedPayload, err := finalizedBlock.Block().Body().Execution()
-			if err != nil {
-				return nil, err
-			}
-			finalizedBlockHash = finalizedPayload.BlockHash()
+			log.WithError(err).Error("Could not get unrealized justified payload block hash")
+			justifiedBlockHash = finalizedBlockHash // Don't fail block proposal if we can't get the justified block hash.
 		}
 	}
 
 	f := &enginev1.ForkchoiceState{
 		HeadBlockHash:      parentHash,
-		SafeBlockHash:      finalizedBlockHash,
-		FinalizedBlockHash: finalizedBlockHash,
+		SafeBlockHash:      justifiedBlockHash[:],
+		FinalizedBlockHash: finalizedBlockHash[:],
 	}
 	var attr payloadattribute.Attributer
 	switch st.Version() {

beacon-chain/rpc/prysm/v1alpha1/validator/proposer_execution_payload_test.go

@@ -139,6 +139,7 @@ func TestServer_getExecutionPayload(t *testing.T) {
 			vs := &Server{
 				ExecutionEngineCaller:  &powtesting.EngineClient{PayloadIDBytes: tt.payloadID, ErrForkchoiceUpdated: tt.forkchoiceErr, ExecutionPayload: &pb.ExecutionPayload{}},
 				HeadFetcher:            &chainMock.ChainService{State: tt.st},
+				FinalizationFetcher:    &chainMock.ChainService{},
 				BeaconDB:               beaconDB,
 				ProposerSlotIndexCache: cache.NewProposerPayloadIDsCache(),
 			}
@@ -221,6 +222,7 @@ func TestServer_getExecutionPayload_UnexpectedFeeRecipient(t *testing.T) {
 			ExecutionPayload: payload,
 		},
 		HeadFetcher:            &chainMock.ChainService{State: transitionSt},
+		FinalizationFetcher:    &chainMock.ChainService{},
 		BeaconDB:               beaconDB,
 		ProposerSlotIndexCache: cache.NewProposerPayloadIDsCache(),
 	}

beacon-chain/rpc/prysm/v1alpha1/validator/proposer_test.go

@@ -465,6 +465,7 @@ func getProposerServer(db db.HeadAccessDatabase, headState state.BeaconState, he
 		ChainStartFetcher:     &mockExecution.Chain{},
 		Eth1InfoFetcher:       &mockExecution.Chain{},
 		Eth1BlockFetcher:      &mockExecution.Chain{},
+		FinalizationFetcher:   mockChainService,
 		ForkFetcher:           mockChainService,
 		ForkchoiceFetcher:     mockChainService,
 		MockEth1Votes:         true,