Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 17 commits into from
Mar 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions beacon-chain/blockchain/chain_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
14 changes: 14 additions & 0 deletions beacon-chain/blockchain/chain_info_forkchoice.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
40 changes: 40 additions & 0 deletions beacon-chain/blockchain/chain_info_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{})
Expand Down
6 changes: 5 additions & 1 deletion beacon-chain/blockchain/execution_engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -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[:],
Expand Down
10 changes: 10 additions & 0 deletions beacon-chain/blockchain/testing/mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
11 changes: 11 additions & 0 deletions beacon-chain/forkchoice/doubly-linked-tree/forkchoice.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
17 changes: 17 additions & 0 deletions beacon-chain/forkchoice/doubly-linked-tree/forkchoice_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
1 change: 1 addition & 0 deletions beacon-chain/forkchoice/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
33 changes: 19 additions & 14 deletions beacon-chain/rpc/eth/validator/validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -1280,6 +1281,7 @@ func TestProduceBlockV2(t *testing.T) {
BlockReceiver: mockChainService,
ForkFetcher: mockChainService,
ForkchoiceFetcher: mockChainService,
FinalizationFetcher: mockChainService,
ChainStartFetcher: mockExecutionChain,
Eth1InfoFetcher: mockExecutionChain,
Eth1BlockFetcher: mockExecutionChain,
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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{},
Expand Down Expand Up @@ -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{},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
}
Expand Down Expand Up @@ -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(),
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down