diff --git a/beacon-chain/blockchain/error.go b/beacon-chain/blockchain/error.go index 40b1d2e8be..98bbecb808 100644 --- a/beacon-chain/blockchain/error.go +++ b/beacon-chain/blockchain/error.go @@ -13,4 +13,7 @@ var ( errInvalidNilSummary = errors.New("nil summary returned from the DB") // errNilParentInDB is returned when a nil parent block is returned from the DB. errNilParentInDB = errors.New("nil parent block in DB") + // errWrongBlockCound is returned when the wrong number of blocks or + // block roots is used + errWrongBlockCount = errors.New("wrong number of blocks or block roots") ) diff --git a/beacon-chain/blockchain/optimistic_sync.go b/beacon-chain/blockchain/optimistic_sync.go index 990494a6df..4fd2e95c98 100644 --- a/beacon-chain/blockchain/optimistic_sync.go +++ b/beacon-chain/blockchain/optimistic_sync.go @@ -83,13 +83,14 @@ 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) error { +func (s *Service) notifyNewPayload(ctx context.Context, preStateVersion int, header *ethpb.ExecutionPayloadHeader, postState state.BeaconState, blk block.SignedBeaconBlock, root [32]byte) error { if postState == nil { return errors.New("pre and post states must not be nil") } - // Execution payload is only supported in Bellatrix and beyond. + // Execution payload is only supported in Bellatrix and beyond. Pre + // merge blocks are never optimistic if isPreBellatrix(postState.Version()) { - return nil + return s.cfg.ForkChoiceStore.SetOptimisticToValid(ctx, root) } if err := helpers.BeaconBlockIsNil(blk); err != nil { return err @@ -100,7 +101,7 @@ func (s *Service) notifyNewPayload(ctx context.Context, preStateVersion int, hea return errors.Wrap(err, "could not determine if execution is enabled") } if !enabled { - return nil + return s.cfg.ForkChoiceStore.SetOptimisticToValid(ctx, root) } payload, err := body.ExecutionPayload() if err != nil { @@ -120,6 +121,10 @@ func (s *Service) notifyNewPayload(ctx context.Context, preStateVersion int, hea } } + if err := s.cfg.ForkChoiceStore.SetOptimisticToValid(ctx, root); err != nil { + return errors.Wrap(err, "could not set optimistic status") + } + // During the transition event, the transition block should be verified for sanity. if isPreBellatrix(preStateVersion) { // Handle case where pre-state is Altair but block contains payload. diff --git a/beacon-chain/blockchain/optimistic_sync_test.go b/beacon-chain/blockchain/optimistic_sync_test.go index 57e3eab8f9..208da82054 100644 --- a/beacon-chain/blockchain/optimistic_sync_test.go +++ b/beacon-chain/blockchain/optimistic_sync_test.go @@ -346,7 +346,9 @@ func Test_NotifyNewPayload(t *testing.T) { payload, err = tt.preState.LatestExecutionPayloadHeader() require.NoError(t, err) } - err := service.notifyNewPayload(ctx, tt.preState.Version(), payload, tt.postState, tt.blk) + 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) if tt.errString != "" { require.ErrorContains(t, tt.errString, err) } else { @@ -356,6 +358,53 @@ func Test_NotifyNewPayload(t *testing.T) { } } +func Test_NotifyNewPayload_SetOptimisticToValid(t *testing.T) { + cfg := params.BeaconConfig() + cfg.TerminalTotalDifficulty = "2" + params.OverrideBeaconConfig(cfg) + ctx := context.Background() + beaconDB := testDB.SetupDB(t) + fcs := protoarray.New(0, 0, [32]byte{'a'}) + opts := []Option{ + WithDatabase(beaconDB), + WithStateGen(stategen.New(beaconDB)), + WithForkChoiceStore(fcs), + } + bellatrixState, _ := util.DeterministicGenesisStateBellatrix(t, 2) + blk := ðpb.SignedBeaconBlockBellatrix{ + Block: ðpb.BeaconBlockBellatrix{ + Body: ðpb.BeaconBlockBodyBellatrix{ + ExecutionPayload: &v1.ExecutionPayload{ + ParentHash: bytesutil.PadTo([]byte{'a'}, fieldparams.RootLength), + }, + }, + }, + } + bellatrixBlk, err := wrapper.WrappedSignedBeaconBlock(blk) + require.NoError(t, err) + service, err := NewService(ctx, opts...) + require.NoError(t, err) + engine := &mockEngineService{blks: map[[32]byte]*v1.ExecutionBlock{}} + engine.blks[[32]byte{'a'}] = &v1.ExecutionBlock{ + ParentHash: bytesutil.PadTo([]byte{'b'}, fieldparams.RootLength), + TotalDifficulty: "0x2", + } + engine.blks[[32]byte{'b'}] = &v1.ExecutionBlock{ + ParentHash: bytesutil.PadTo([]byte{'3'}, fieldparams.RootLength), + TotalDifficulty: "0x1", + } + service.cfg.ExecutionEngineCaller = engine + payload, err := bellatrixState.LatestExecutionPayloadHeader() + 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) + require.NoError(t, err) + optimistic, err := service.IsOptimisticForRoot(ctx, root) + require.NoError(t, err) + require.Equal(t, false, optimistic) +} + func Test_IsOptimisticCandidateBlock(t *testing.T) { params.SetupTestConfigCleanup(t) params.OverrideBeaconConfig(params.MainnetConfig()) diff --git a/beacon-chain/blockchain/process_block.go b/beacon-chain/blockchain/process_block.go index 7e83d14ba4..da578f7d0b 100644 --- a/beacon-chain/blockchain/process_block.go +++ b/beacon-chain/blockchain/process_block.go @@ -107,13 +107,12 @@ func (s *Service) onBlock(ctx context.Context, signed block.SignedBeaconBlock, b if err != nil { return err } - if err := s.notifyNewPayload(ctx, preStateVersion, preStateHeader, postState, signed); err != nil { - return errors.Wrap(err, "could not verify new payload") - } - 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 { + return errors.Wrap(err, "could not verify new payload") + } // We add a proposer score boost to fork choice for the block root if applicable, right after // running a successful state transition for the block. @@ -280,6 +279,11 @@ func (s *Service) onBlockBatch(ctx context.Context, blks []block.SignedBeaconBlo if len(blks) == 0 || len(blockRoots) == 0 { return nil, nil, errors.New("no blocks provided") } + + if len(blks) != len(blockRoots) { + return nil, nil, errWrongBlockCount + } + if err := helpers.BeaconBlockIsNil(blks[0]); err != nil { return nil, nil, err } @@ -307,17 +311,10 @@ func (s *Service) onBlockBatch(ctx context.Context, blks []block.SignedBeaconBlo var set *bls.SignatureBatch boundaries := make(map[[32]byte]state.BeaconState) for i, b := range blks { - preStateVersion, preStateHeader, err := getStateVersionAndPayload(preState) - if err != nil { - return nil, nil, err - } set, preState, err = transition.ExecuteStateTransitionNoVerifyAnySig(ctx, preState, b) if err != nil { return nil, nil, err } - if err := s.notifyNewPayload(ctx, preStateVersion, preStateHeader, preState, b); err != nil { - return nil, nil, err - } // Save potential boundary states. if slots.IsEpochStart(preState.Slot()) { boundaries[blockRoots[i]] = preState.Copy() @@ -336,6 +333,25 @@ func (s *Service) onBlockBatch(ctx context.Context, blks []block.SignedBeaconBlo if !verify { return nil, nil, errors.New("batch block signature verification failed") } + + // 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 { + 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 @@ -357,15 +373,6 @@ func (s *Service) onBlockBatch(ctx context.Context, blks []block.SignedBeaconBlo // their state summaries and split them off to relative hot/cold storage. func (s *Service) handleBlockAfterBatchVerify(ctx context.Context, signed block.SignedBeaconBlock, blockRoot [32]byte, fCheckpoint, jCheckpoint *ethpb.Checkpoint) error { - b := signed.Block() - - s.saveInitSyncBlock(blockRoot, signed) - if err := s.insertBlockToForkChoiceStore(ctx, b, blockRoot, fCheckpoint, jCheckpoint); err != nil { - return err - } - if _, err := s.notifyForkchoiceUpdate(ctx, b, blockRoot, bytesutil.ToBytes32(fCheckpoint.Root)); err != nil { - return err - } if err := s.cfg.BeaconDB.SaveStateSummary(ctx, ðpb.StateSummary{ Slot: signed.Block().Slot(), diff --git a/beacon-chain/blockchain/process_block_test.go b/beacon-chain/blockchain/process_block_test.go index 27a6d51668..7c5e906633 100644 --- a/beacon-chain/blockchain/process_block_test.go +++ b/beacon-chain/blockchain/process_block_test.go @@ -295,6 +295,8 @@ func TestStore_OnBlockBatch_ProtoArray(t *testing.T) { rBlock.Block.ParentRoot = gRoot[:] require.NoError(t, beaconDB.SaveBlock(context.Background(), blks[0])) require.NoError(t, service.cfg.StateGen.SaveState(ctx, blkRoots[0], firstState)) + _, _, err = service.onBlockBatch(ctx, blks, blkRoots[1:]) + require.ErrorIs(t, errWrongBlockCount, err) _, _, err = service.onBlockBatch(ctx, blks[1:], blkRoots[1:]) require.NoError(t, err) } @@ -347,6 +349,8 @@ func TestStore_OnBlockBatch_DoublyLinkedTree(t *testing.T) { rBlock.Block.ParentRoot = gRoot[:] require.NoError(t, beaconDB.SaveBlock(context.Background(), blks[0])) require.NoError(t, service.cfg.StateGen.SaveState(ctx, blkRoots[0], firstState)) + _, _, err = service.onBlockBatch(ctx, blks, blkRoots[1:]) + require.ErrorIs(t, errWrongBlockCount, err) _, _, err = service.onBlockBatch(ctx, blks[1:], blkRoots[1:]) require.NoError(t, err) }