From 679e6bc54accd2a5ff25f795c6d798189a74c0e1 Mon Sep 17 00:00:00 2001 From: terencechain Date: Mon, 28 Nov 2022 11:42:32 -0800 Subject: [PATCH] Cont FCU if get payload attribute fails (#11693) * Cont FCU if get payload attribute fails * Fix err position Co-authored-by: Sammy Rosso <15244892+saolyn@users.noreply.github.com> --- beacon-chain/blockchain/execution_engine.go | 1 - .../blockchain/execution_engine_test.go | 67 ++++++++++++++++++- 2 files changed, 66 insertions(+), 2 deletions(-) diff --git a/beacon-chain/blockchain/execution_engine.go b/beacon-chain/blockchain/execution_engine.go index 41c99b744..8ff601beb 100644 --- a/beacon-chain/blockchain/execution_engine.go +++ b/beacon-chain/blockchain/execution_engine.go @@ -70,7 +70,6 @@ func (s *Service) notifyForkchoiceUpdate(ctx context.Context, arg *notifyForkcho hasAttr, attr, proposerId, err := s.getPayloadAttribute(ctx, arg.headState, nextSlot) if err != nil { log.WithError(err).Error("Could not get head payload attribute") - return nil, nil } payloadID, lastValidHash, err := s.cfg.ExecutionEngineCaller.ForkchoiceUpdated(ctx, fcs, attr) diff --git a/beacon-chain/blockchain/execution_engine_test.go b/beacon-chain/blockchain/execution_engine_test.go index 519b21b18..31f960ec3 100644 --- a/beacon-chain/blockchain/execution_engine_test.go +++ b/beacon-chain/blockchain/execution_engine_test.go @@ -15,6 +15,7 @@ import ( doublylinkedtree "github.com/prysmaticlabs/prysm/v3/beacon-chain/forkchoice/doubly-linked-tree" forkchoicetypes "github.com/prysmaticlabs/prysm/v3/beacon-chain/forkchoice/types" bstate "github.com/prysmaticlabs/prysm/v3/beacon-chain/state" + state_native "github.com/prysmaticlabs/prysm/v3/beacon-chain/state/state-native" "github.com/prysmaticlabs/prysm/v3/beacon-chain/state/stategen" fieldparams "github.com/prysmaticlabs/prysm/v3/config/fieldparams" "github.com/prysmaticlabs/prysm/v3/config/params" @@ -30,6 +31,69 @@ import ( logTest "github.com/sirupsen/logrus/hooks/test" ) +func Test_NotifyForkchoiceUpdate_GetPayloadAttrErrorCanContinue(t *testing.T) { + ctx := context.Background() + beaconDB := testDB.SetupDB(t) + altairBlk := util.SaveBlock(t, ctx, beaconDB, util.NewBeaconBlockAltair()) + altairBlkRoot, err := altairBlk.Block().HashTreeRoot() + require.NoError(t, err) + bellatrixBlk := util.SaveBlock(t, ctx, beaconDB, util.NewBeaconBlockBellatrix()) + bellatrixBlkRoot, err := bellatrixBlk.Block().HashTreeRoot() + require.NoError(t, err) + fcs := doublylinkedtree.New() + opts := []Option{ + WithDatabase(beaconDB), + WithStateGen(stategen.New(beaconDB, fcs)), + WithForkChoiceStore(fcs), + WithProposerIdsCache(cache.NewProposerPayloadIDsCache()), + } + service, err := NewService(ctx, opts...) + require.NoError(t, err) + st, _ := util.DeterministicGenesisState(t, 10) + service.head = &head{ + state: st, + } + + ojc := ðpb.Checkpoint{Root: params.BeaconConfig().ZeroHash[:]} + ofc := ðpb.Checkpoint{Root: params.BeaconConfig().ZeroHash[:]} + state, blkRoot, err := prepareForkchoiceState(ctx, 0, [32]byte{}, [32]byte{}, params.BeaconConfig().ZeroHash, ojc, ofc) + require.NoError(t, err) + require.NoError(t, fcs.InsertNode(ctx, state, blkRoot)) + state, blkRoot, err = prepareForkchoiceState(ctx, 1, altairBlkRoot, [32]byte{}, params.BeaconConfig().ZeroHash, ojc, ofc) + require.NoError(t, err) + require.NoError(t, fcs.InsertNode(ctx, state, blkRoot)) + state, blkRoot, err = prepareForkchoiceState(ctx, 2, bellatrixBlkRoot, altairBlkRoot, params.BeaconConfig().ZeroHash, ojc, ofc) + require.NoError(t, err) + require.NoError(t, fcs.InsertNode(ctx, state, blkRoot)) + + b, err := consensusblocks.NewBeaconBlock(ðpb.BeaconBlockBellatrix{ + Body: ðpb.BeaconBlockBodyBellatrix{ + ExecutionPayload: &v1.ExecutionPayload{}, + }, + }) + require.NoError(t, err) + + pid := &v1.PayloadIDBytes{1} + service.cfg.ExecutionEngineCaller = &mockExecution.EngineClient{PayloadIDBytes: pid} + st, _ = util.DeterministicGenesisState(t, 1) + require.NoError(t, beaconDB.SaveState(ctx, st, bellatrixBlkRoot)) + require.NoError(t, beaconDB.SaveGenesisBlockRoot(ctx, bellatrixBlkRoot)) + + // Intentionally generate a bad state such that `hash_tree_root` fails during `process_slot` + s, err := state_native.InitializeFromProtoPhase0(ðpb.BeaconState{}) + require.NoError(t, err) + arg := ¬ifyForkchoiceUpdateArg{ + headState: s, + headRoot: [32]byte{}, + headBlock: b, + } + + service.cfg.ProposerSlotIndexCache.SetProposerAndPayloadIDs(1, 0, [8]byte{}, [32]byte{}) + got, err := service.notifyForkchoiceUpdate(ctx, arg) + require.NoError(t, err) + require.DeepEqual(t, got, pid) // We still get a payload ID even though the state is bad. This means it returns until the end. +} + func Test_NotifyForkchoiceUpdate(t *testing.T) { ctx := context.Background() beaconDB := testDB.SetupDB(t) @@ -47,11 +111,12 @@ func Test_NotifyForkchoiceUpdate(t *testing.T) { WithProposerIdsCache(cache.NewProposerPayloadIDsCache()), } service, err := NewService(ctx, opts...) + require.NoError(t, err) st, _ := util.DeterministicGenesisState(t, 10) service.head = &head{ state: st, } - require.NoError(t, err) + ojc := ðpb.Checkpoint{Root: params.BeaconConfig().ZeroHash[:]} ofc := ðpb.Checkpoint{Root: params.BeaconConfig().ZeroHash[:]} state, blkRoot, err := prepareForkchoiceState(ctx, 0, [32]byte{}, [32]byte{}, params.BeaconConfig().ZeroHash, ojc, ofc)