Skip to content

Commit bdab34f

Browse files
terencechainterencechain
authored
Use correct head for notifyForkchoiceUpdate (#10485)
* Use correct head for notifyForkchoiceUpdate * Fix existing tests * Update receive_attestation_test.go
1 parent de0143e commit bdab34f

7 files changed

+65
-43
lines changed

‎beacon-chain/blockchain/chain_info_norace_test.go

+15-6
Original file line numberDiff line numberDiff line change
@@ -9,17 +9,20 @@ import (
99
ethpb "github.com/prysmaticlabs/prysm/proto/prysm/v1alpha1"
1010
"github.com/prysmaticlabs/prysm/proto/prysm/v1alpha1/wrapper"
1111
"github.com/prysmaticlabs/prysm/testing/require"
12+
"github.com/prysmaticlabs/prysm/testing/util"
1213
)
1314

1415
func TestHeadSlot_DataRace(t *testing.T) {
1516
beaconDB := testDB.SetupDB(t)
1617
s := &Service{
1718
cfg: &config{BeaconDB: beaconDB},
1819
}
20+
b, err := wrapper.WrappedSignedBeaconBlock(util.NewBeaconBlock())
21+
require.NoError(t, err)
1922
wait := make(chan struct{})
2023
go func() {
2124
defer close(wait)
22-
require.NoError(t, s.saveHead(context.Background(), [32]byte{}))
25+
require.NoError(t, s.saveHead(context.Background(), [32]byte{}, b))
2326
}()
2427
s.HeadSlot()
2528
<-wait
@@ -31,12 +34,14 @@ func TestHeadRoot_DataRace(t *testing.T) {
3134
cfg: &config{BeaconDB: beaconDB, StateGen: stategen.New(beaconDB)},
3235
head: &head{root: [32]byte{'A'}},
3336
}
37+
b, err := wrapper.WrappedSignedBeaconBlock(util.NewBeaconBlock())
38+
require.NoError(t, err)
3439
wait := make(chan struct{})
3540
go func() {
3641
defer close(wait)
37-
require.NoError(t, s.saveHead(context.Background(), [32]byte{}))
42+
require.NoError(t, s.saveHead(context.Background(), [32]byte{}, b))
3843
}()
39-
_, err := s.HeadRoot(context.Background())
44+
_, err = s.HeadRoot(context.Background())
4045
require.NoError(t, err)
4146
<-wait
4247
}
@@ -49,10 +54,12 @@ func TestHeadBlock_DataRace(t *testing.T) {
4954
cfg: &config{BeaconDB: beaconDB, StateGen: stategen.New(beaconDB)},
5055
head: &head{block: wsb},
5156
}
57+
b, err := wrapper.WrappedSignedBeaconBlock(util.NewBeaconBlock())
58+
require.NoError(t, err)
5259
wait := make(chan struct{})
5360
go func() {
5461
defer close(wait)
55-
require.NoError(t, s.saveHead(context.Background(), [32]byte{}))
62+
require.NoError(t, s.saveHead(context.Background(), [32]byte{}, b))
5663
}()
5764
_, err = s.HeadBlock(context.Background())
5865
require.NoError(t, err)
@@ -64,12 +71,14 @@ func TestHeadState_DataRace(t *testing.T) {
6471
s := &Service{
6572
cfg: &config{BeaconDB: beaconDB, StateGen: stategen.New(beaconDB)},
6673
}
74+
b, err := wrapper.WrappedSignedBeaconBlock(util.NewBeaconBlock())
75+
require.NoError(t, err)
6776
wait := make(chan struct{})
6877
go func() {
6978
defer close(wait)
70-
require.NoError(t, s.saveHead(context.Background(), [32]byte{}))
79+
require.NoError(t, s.saveHead(context.Background(), [32]byte{}, b))
7180
}()
72-
_, err := s.HeadState(context.Background())
81+
_, err = s.HeadState(context.Background())
7382
require.NoError(t, err)
7483
<-wait
7584
}

‎beacon-chain/blockchain/head.go

+13-15
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,11 @@ func (s *Service) UpdateAndSaveHeadWithBalances(ctx context.Context) error {
4040
if err != nil {
4141
return errors.Wrap(err, "could not update head")
4242
}
43-
return s.saveHead(ctx, headRoot)
43+
headBlock, err := s.cfg.BeaconDB.Block(ctx, headRoot)
44+
if err != nil {
45+
return err
46+
}
47+
return s.saveHead(ctx, headRoot, headBlock)
4448
}
4549

4650
// This defines the current chain service's view of head.
@@ -97,7 +101,7 @@ func (s *Service) updateHead(ctx context.Context, balances []uint64) ([32]byte,
97101

98102
// This saves head info to the local service cache, it also saves the
99103
// new head root to the DB.
100-
func (s *Service) saveHead(ctx context.Context, headRoot [32]byte) error {
104+
func (s *Service) saveHead(ctx context.Context, headRoot [32]byte, headBlock block.SignedBeaconBlock) error {
101105
ctx, span := trace.StartSpan(ctx, "blockChain.saveHead")
102106
defer span.End()
103107

@@ -109,22 +113,16 @@ func (s *Service) saveHead(ctx context.Context, headRoot [32]byte) error {
109113
if headRoot == bytesutil.ToBytes32(r) {
110114
return nil
111115
}
116+
if err := helpers.BeaconBlockIsNil(headBlock); err != nil {
117+
return err
118+
}
112119

113120
// If the head state is not available, just return nil.
114121
// There's nothing to cache
115122
if !s.cfg.BeaconDB.HasStateSummary(ctx, headRoot) {
116123
return nil
117124
}
118125

119-
// Get the new head block from DB.
120-
newHeadBlock, err := s.cfg.BeaconDB.Block(ctx, headRoot)
121-
if err != nil {
122-
return err
123-
}
124-
if err := helpers.BeaconBlockIsNil(newHeadBlock); err != nil {
125-
return err
126-
}
127-
128126
// Get the new head state from cached state or DB.
129127
newHeadState, err := s.cfg.StateGen.StateByRoot(ctx, headRoot)
130128
if err != nil {
@@ -136,11 +134,11 @@ func (s *Service) saveHead(ctx context.Context, headRoot [32]byte) error {
136134

137135
// A chain re-org occurred, so we fire an event notifying the rest of the services.
138136
headSlot := s.HeadSlot()
139-
newHeadSlot := newHeadBlock.Block().Slot()
137+
newHeadSlot := headBlock.Block().Slot()
140138
oldHeadRoot := s.headRoot()
141139
oldStateRoot := s.headBlock().Block().StateRoot()
142-
newStateRoot := newHeadBlock.Block().StateRoot()
143-
if bytesutil.ToBytes32(newHeadBlock.Block().ParentRoot()) != bytesutil.ToBytes32(r) {
140+
newStateRoot := headBlock.Block().StateRoot()
141+
if bytesutil.ToBytes32(headBlock.Block().ParentRoot()) != bytesutil.ToBytes32(r) {
144142
log.WithFields(logrus.Fields{
145143
"newSlot": fmt.Sprintf("%d", newHeadSlot),
146144
"oldSlot": fmt.Sprintf("%d", headSlot),
@@ -172,7 +170,7 @@ func (s *Service) saveHead(ctx context.Context, headRoot [32]byte) error {
172170
}
173171

174172
// Cache the new head info.
175-
s.setHead(headRoot, newHeadBlock, newHeadState)
173+
s.setHead(headRoot, headBlock, newHeadState)
176174

177175
// Save the new head root to DB.
178176
if err := s.cfg.BeaconDB.SaveHeadBlockRoot(ctx, headRoot); err != nil {

‎beacon-chain/blockchain/head_test.go

+6-5
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,9 @@ func TestSaveHead_Same(t *testing.T) {
2929

3030
r := [32]byte{'A'}
3131
service.head = &head{slot: 0, root: r}
32-
33-
require.NoError(t, service.saveHead(context.Background(), r))
32+
b, err := wrapper.WrappedSignedBeaconBlock(util.NewBeaconBlock())
33+
require.NoError(t, err)
34+
require.NoError(t, service.saveHead(context.Background(), r, b))
3435
assert.Equal(t, types.Slot(0), service.headSlot(), "Head did not stay the same")
3536
assert.Equal(t, r, service.headRoot(), "Head did not stay the same")
3637
}
@@ -68,7 +69,7 @@ func TestSaveHead_Different(t *testing.T) {
6869
require.NoError(t, headState.SetSlot(1))
6970
require.NoError(t, service.cfg.BeaconDB.SaveStateSummary(context.Background(), &ethpb.StateSummary{Slot: 1, Root: newRoot[:]}))
7071
require.NoError(t, service.cfg.BeaconDB.SaveState(context.Background(), headState, newRoot))
71-
require.NoError(t, service.saveHead(context.Background(), newRoot))
72+
require.NoError(t, service.saveHead(context.Background(), newRoot, wsb))
7273

7374
assert.Equal(t, types.Slot(1), service.HeadSlot(), "Head did not change")
7475

@@ -114,7 +115,7 @@ func TestSaveHead_Different_Reorg(t *testing.T) {
114115
require.NoError(t, headState.SetSlot(1))
115116
require.NoError(t, service.cfg.BeaconDB.SaveStateSummary(context.Background(), &ethpb.StateSummary{Slot: 1, Root: newRoot[:]}))
116117
require.NoError(t, service.cfg.BeaconDB.SaveState(context.Background(), headState, newRoot))
117-
require.NoError(t, service.saveHead(context.Background(), newRoot))
118+
require.NoError(t, service.saveHead(context.Background(), newRoot, wsb))
118119

119120
assert.Equal(t, types.Slot(1), service.HeadSlot(), "Head did not change")
120121

@@ -158,7 +159,7 @@ func TestUpdateHead_MissingJustifiedRoot(t *testing.T) {
158159
service.store.SetBestJustifiedCheckpt(&ethpb.Checkpoint{})
159160
headRoot, err := service.updateHead(context.Background(), []uint64{})
160161
require.NoError(t, err)
161-
require.NoError(t, service.saveHead(context.Background(), headRoot))
162+
require.NoError(t, service.saveHead(context.Background(), headRoot, wsb))
162163
}
163164

164165
func Test_notifyNewHeadEvent(t *testing.T) {

‎beacon-chain/blockchain/process_block.go

+6-2
Original file line numberDiff line numberDiff line change
@@ -208,10 +208,14 @@ func (s *Service) onBlock(ctx context.Context, signed block.SignedBeaconBlock, b
208208
if err != nil {
209209
log.WithError(err).Warn("Could not update head")
210210
}
211-
if _, err := s.notifyForkchoiceUpdate(ctx, s.headBlock().Block(), s.headRoot(), bytesutil.ToBytes32(finalized.Root)); err != nil {
211+
headBlock, err := s.cfg.BeaconDB.Block(ctx, headRoot)
212+
if err != nil {
213+
return err
214+
}
215+
if _, err := s.notifyForkchoiceUpdate(ctx, headBlock.Block(), headRoot, bytesutil.ToBytes32(finalized.Root)); err != nil {
212216
return err
213217
}
214-
if err := s.saveHead(ctx, headRoot); err != nil {
218+
if err := s.saveHead(ctx, headRoot, headBlock); err != nil {
215219
return errors.Wrap(err, "could not save head")
216220
}
217221

‎beacon-chain/blockchain/receive_attestation.go

+9-4
Original file line numberDiff line numberDiff line change
@@ -181,15 +181,20 @@ func (s *Service) notifyEngineIfChangedHead(ctx context.Context, newHeadRoot [32
181181
log.WithError(errNilFinalizedInStore).Error("could not get finalized checkpoint")
182182
return
183183
}
184-
_, err := s.notifyForkchoiceUpdate(s.ctx,
185-
s.headBlock().Block(),
186-
s.headRoot(),
184+
newHeadBlock, err := s.cfg.BeaconDB.Block(ctx, newHeadRoot)
185+
if err != nil {
186+
log.WithError(err).Error("Could not get block from db")
187+
return
188+
}
189+
_, err = s.notifyForkchoiceUpdate(s.ctx,
190+
newHeadBlock.Block(),
191+
newHeadRoot,
187192
bytesutil.ToBytes32(finalized.Root),
188193
)
189194
if err != nil {
190195
log.WithError(err).Error("could not notify forkchoice update")
191196
}
192-
if err := s.saveHead(ctx, newHeadRoot); err != nil {
197+
if err := s.saveHead(ctx, newHeadRoot, newHeadBlock); err != nil {
193198
log.WithError(err).Error("could not save head")
194199
}
195200
}

‎beacon-chain/blockchain/receive_attestation_test.go

+10-9
Original file line numberDiff line numberDiff line change
@@ -143,21 +143,22 @@ func TestNotifyEngineIfChangedHead(t *testing.T) {
143143
require.LogsContain(t, hook, finalizedErr)
144144

145145
hook.Reset()
146+
service.head = &head{
147+
root: [32]byte{'a'},
148+
block: nil, /* should not panic if notify head uses correct head */
149+
}
150+
146151
b := util.NewBeaconBlock()
147-
b.Block.Slot = 1
152+
b.Block.Slot = 2
148153
wsb, err := wrapper.WrappedSignedBeaconBlock(b)
149154
require.NoError(t, err)
150155
require.NoError(t, service.cfg.BeaconDB.SaveBlock(ctx, wsb))
151-
r, err := b.Block.HashTreeRoot()
156+
r1, err := b.Block.HashTreeRoot()
152157
require.NoError(t, err)
153-
finalized := &ethpb.Checkpoint{Root: r[:], Epoch: 0}
154-
service.head = &head{
155-
slot: 1,
156-
root: r,
157-
block: wsb,
158-
}
158+
finalized := &ethpb.Checkpoint{Root: r1[:], Epoch: 0}
159+
159160
service.store.SetFinalizedCheckpt(finalized)
160-
service.notifyEngineIfChangedHead(ctx, [32]byte{'b'})
161+
service.notifyEngineIfChangedHead(ctx, r1)
161162
require.LogsDoNotContain(t, hook, finalizedErr)
162163
require.LogsDoNotContain(t, hook, hookErr)
163164
}

‎beacon-chain/blockchain/service_norace_test.go

+6-2
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@ import (
66
"testing"
77

88
testDB "github.com/prysmaticlabs/prysm/beacon-chain/db/testing"
9+
"github.com/prysmaticlabs/prysm/proto/prysm/v1alpha1/wrapper"
910
"github.com/prysmaticlabs/prysm/testing/require"
11+
"github.com/prysmaticlabs/prysm/testing/util"
1012
"github.com/sirupsen/logrus"
1113
)
1214

@@ -20,8 +22,10 @@ func TestChainService_SaveHead_DataRace(t *testing.T) {
2022
s := &Service{
2123
cfg: &config{BeaconDB: beaconDB},
2224
}
25+
b, err := wrapper.WrappedSignedBeaconBlock(util.NewBeaconBlock())
26+
require.NoError(t, err)
2327
go func() {
24-
require.NoError(t, s.saveHead(context.Background(), [32]byte{}))
28+
require.NoError(t, s.saveHead(context.Background(), [32]byte{}, b))
2529
}()
26-
require.NoError(t, s.saveHead(context.Background(), [32]byte{}))
30+
require.NoError(t, s.saveHead(context.Background(), [32]byte{}, b))
2731
}

0 commit comments

Comments
 (0)