Skip to content

Commit

Permalink
Fork choice avoids redundant call to get_ancestor (#6282)
Browse files Browse the repository at this point in the history 
* Add helper to prevent zero hashes

* Test

* Add cacheJustifiedStateBalances

* Cache justifed balances

* Use cached justified balances

* Remove unncessary justification checks during initial syncing

* Test

* Proper locking

* Use getter

* Add CheckPointIsEqual helper

* Test CheckPointIsEqual helper

* Update finalizedImpliesNewJustified

* Better tests for finalizedImpliesNewJustified

Co-authored-by: prylabs-bulldozer[bot] <58059840+prylabs-bulldozer[bot]@users.noreply.github.com>
  • Loading branch information
@terencechain @prylabs-bulldozer
terencechain and prylabs-bulldozer[bot] committed Jun 17, 2020
1 parent 4740f7e commit af3122a
Show file tree
Hide file tree
Showing 4 changed files with 181 additions and 21 deletions.
53 changes: 32 additions & 21 deletions beacon-chain/blockchain/process_block_helpers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/prysmaticlabs/prysm/beacon-chain/db/filters"
stateTrie "github.com/prysmaticlabs/prysm/beacon-chain/state"
"github.com/prysmaticlabs/prysm/beacon-chain/state/stateutil"
"github.com/prysmaticlabs/prysm/shared/attestationutil"
"github.com/prysmaticlabs/prysm/shared/bytesutil"
"github.com/prysmaticlabs/prysm/shared/featureconfig"
"github.com/prysmaticlabs/prysm/shared/params"
Expand Down Expand Up @@ -425,32 +426,42 @@ func (s *Service) ancestor(ctx context.Context, root []byte, slot uint64) ([]byt
// the store's justified is not in chain with finalized check point.
//
// Spec definition:
// if (
// state.current_justified_checkpoint.epoch > store.justified_checkpoint.epoch
// or get_ancestor(store, store.justified_checkpoint.root, finalized_slot) != store.finalized_checkpoint.root
// ):
// store.justified_checkpoint = state.current_justified_checkpoint
// # Potentially update justified if different from store
// if store.justified_checkpoint != state.current_justified_checkpoint:
// # Update justified if new justified is later than store justified
// if state.current_justified_checkpoint.epoch > store.justified_checkpoint.epoch:
// store.justified_checkpoint = state.current_justified_checkpoint
// return
// # Update justified if store justified is not in chain with finalized checkpoint
// finalized_slot = compute_start_slot_at_epoch(store.finalized_checkpoint.epoch)
// ancestor_at_finalized_slot = get_ancestor(store, store.justified_checkpoint.root, finalized_slot)
// if ancestor_at_finalized_slot != store.finalized_checkpoint.root:
// store.justified_checkpoint = state.current_justified_checkpoint
func (s *Service) finalizedImpliesNewJustified(ctx context.Context, state *stateTrie.BeaconState) error {
finalizedBlkSigned, err := s.beaconDB.Block(ctx, bytesutil.ToBytes32(s.finalizedCheckpt.Root))
if err != nil || finalizedBlkSigned == nil || finalizedBlkSigned.Block == nil {
return errors.Wrap(err, "could not get finalized block")
}
finalizedBlk := finalizedBlkSigned.Block

justifiedRoot := s.ensureRootNotZeros(bytesutil.ToBytes32(s.justifiedCheckpt.Root))
anc, err := s.ancestor(ctx, justifiedRoot[:], finalizedBlk.Slot)
if err != nil {
return err
}
// Update justified if it's different than the one cached in the store.
if !attestationutil.CheckPointIsEqual(s.justifiedCheckpt, state.CurrentJustifiedCheckpoint()) {
if state.CurrentJustifiedCheckpoint().Epoch > s.justifiedCheckpt.Epoch {
s.justifiedCheckpt = state.CurrentJustifiedCheckpoint()
if err := s.cacheJustifiedStateBalances(ctx, bytesutil.ToBytes32(s.justifiedCheckpt.Root)); err != nil {
return err
}
return nil
}

// Either the new justified is later than stored justified or not in chain with finalized check pint.
if cpt := state.CurrentJustifiedCheckpoint(); cpt != nil && cpt.Epoch > s.justifiedCheckpt.Epoch || !bytes.Equal(anc, s.finalizedCheckpt.Root) {
s.justifiedCheckpt = state.CurrentJustifiedCheckpoint()
if err := s.cacheJustifiedStateBalances(ctx, bytesutil.ToBytes32(s.justifiedCheckpt.Root)); err != nil {
// Update justified if store justified is not in chain with finalized check point.
finalizedSlot := helpers.StartSlot(s.finalizedCheckpt.Epoch)
justifiedRoot := s.ensureRootNotZeros(bytesutil.ToBytes32(s.justifiedCheckpt.Root))
anc, err := s.ancestor(ctx, justifiedRoot[:], finalizedSlot)
if err != nil {
return err
}
if !bytes.Equal(anc, s.finalizedCheckpt.Root) {
s.justifiedCheckpt = state.CurrentJustifiedCheckpoint()
if err := s.cacheJustifiedStateBalances(ctx, bytesutil.ToBytes32(s.justifiedCheckpt.Root)); err != nil {
return err
}
}
}

return nil
}

Expand Down
83 changes: 83 additions & 0 deletions beacon-chain/blockchain/process_block_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"github.com/prysmaticlabs/prysm/beacon-chain/state/stategen"
"github.com/prysmaticlabs/prysm/beacon-chain/state/stateutil"
pb "github.com/prysmaticlabs/prysm/proto/beacon/p2p/v1"
"github.com/prysmaticlabs/prysm/shared/attestationutil"
"github.com/prysmaticlabs/prysm/shared/bytesutil"
"github.com/prysmaticlabs/prysm/shared/featureconfig"
"github.com/prysmaticlabs/prysm/shared/params"
Expand Down Expand Up @@ -853,3 +854,85 @@ func TestEnsureRootNotZeroHashes(t *testing.T) {
t.Error("Did not get wanted justified root")
}
}

func TestFinalizedImpliesNewJustified(t *testing.T) {
db := testDB.SetupDB(t)
ctx := context.Background()
type args struct {
cachedCheckPoint *ethpb.Checkpoint
stateCheckPoint *ethpb.Checkpoint
diffFinalizedCheckPoint bool
}
tests := []struct {
name string
args args
want *ethpb.Checkpoint
}{
{
name: "Same justified, do nothing",
args: args{
cachedCheckPoint: &ethpb.Checkpoint{Epoch: 1, Root: []byte{'a'}},
stateCheckPoint: &ethpb.Checkpoint{Epoch: 1, Root: []byte{'a'}},
},
want: &ethpb.Checkpoint{Epoch: 1, Root: []byte{'a'}},
},
{
name: "Different justified, higher epoch, cache new justified",
args: args{
cachedCheckPoint: &ethpb.Checkpoint{Epoch: 1, Root: []byte{'a'}},
stateCheckPoint: &ethpb.Checkpoint{Epoch: 2, Root: []byte{'b'}},
},
want: &ethpb.Checkpoint{Epoch: 2, Root: []byte{'b'}},
},
{
name: "finalized has different justified, cache new justified",
args: args{
cachedCheckPoint: &ethpb.Checkpoint{Epoch: 1, Root: []byte{'a'}},
stateCheckPoint: &ethpb.Checkpoint{Epoch: 1, Root: []byte{'b'}},
diffFinalizedCheckPoint: true,
},
want: &ethpb.Checkpoint{Epoch: 1, Root: []byte{'b'}},
},
}
for _, test := range tests {
beaconState := testutil.NewBeaconState()
if err := beaconState.SetCurrentJustifiedCheckpoint(test.args.stateCheckPoint); err != nil {
t.Fatal(err)
}
service, err := NewService(ctx, &Config{BeaconDB: db})
if err != nil {
t.Fatal(err)
}
service.justifiedCheckpt = test.args.cachedCheckPoint

if test.args.diffFinalizedCheckPoint {
b1 := &ethpb.BeaconBlock{Slot: 1, ParentRoot: []byte{'a'}}
r1, err := ssz.HashTreeRoot(b1)
if err != nil {
t.Fatal(err)
}
b100 := &ethpb.BeaconBlock{Slot: 100, ParentRoot: r1[:]}
r100, err := ssz.HashTreeRoot(b100)
if err != nil {
t.Fatal(err)
}
for _, b := range []*ethpb.BeaconBlock{b1, b100} {
beaconBlock := testutil.NewBeaconBlock()
beaconBlock.Block.Slot = b.Slot
beaconBlock.Block.ParentRoot = bytesutil.PadTo(b.ParentRoot, 32)
if err := service.beaconDB.SaveBlock(context.Background(), beaconBlock); err != nil {
t.Fatal(err)
}
}
service.finalizedCheckpt = &ethpb.Checkpoint{Root: []byte{'c'}, Epoch: 1}
service.justifiedCheckpt.Root = r100[:]
}

if err := service.finalizedImpliesNewJustified(ctx, beaconState); err != nil {
t.Fatal(err)
}
if !attestationutil.CheckPointIsEqual(test.want, service.justifiedCheckpt) {
t.Error("Did not get wanted check point")
}
}
}
11 changes: 11 additions & 0 deletions shared/attestationutil/attestation_utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,3 +197,14 @@ func AttDataIsEqual(attData1 *ethpb.AttestationData, attData2 *ethpb.Attestation
}
return true
}

// CheckPointIsEqual performs an equality check between 2 check points, returns false if unequal.
func CheckPointIsEqual(checkPt1 *ethpb.Checkpoint, checkPt2 *ethpb.Checkpoint) bool {
if checkPt1.Epoch != checkPt2.Epoch {
return false
}
if !bytes.Equal(checkPt1.Root, checkPt2.Root) {
return false
}
return true
}
55 changes: 55 additions & 0 deletions shared/attestationutil/attestation_utils_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,61 @@ func TestAttDataIsEqual(t *testing.T) {
}
}

func TestCheckPtIsEqual(t *testing.T) {
type test struct {
name string
checkPt1 *eth.Checkpoint
checkPt2 *eth.Checkpoint
equal bool
}
tests := []test{
{
name: "same",
checkPt1: &eth.Checkpoint{
Epoch: 4,
Root: []byte("good source"),
},
checkPt2: &eth.Checkpoint{
Epoch: 4,
Root: []byte("good source"),
},
equal: true,
},
{
name: "diff epoch",
checkPt1: &eth.Checkpoint{
Epoch: 4,
Root: []byte("good source"),
},
checkPt2: &eth.Checkpoint{
Epoch: 5,
Root: []byte("good source"),
},
equal: false,
},
{
name: "diff root",
checkPt1: &eth.Checkpoint{
Epoch: 4,
Root: []byte("good source"),
},
checkPt2: &eth.Checkpoint{
Epoch: 4,
Root: []byte("bad source"),
},
equal: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
areEqual := attestationutil.CheckPointIsEqual(tt.checkPt1, tt.checkPt2)
if areEqual != tt.equal {
t.Errorf("Expected %t, received %t", tt.equal, areEqual)
}
})
}
}

func BenchmarkAttDataIsEqual(b *testing.B) {
attData1 := &eth.AttestationData{
Slot: 5,
Expand Down

0 comments on commit af3122a

Please sign in to comment.