Fix ancestor to return most recent root in the case of skip slot (#6242)

* Revert "remove excess copies (#6142)"

This reverts commit c956905cf0.
* Update ancestor function to handle skip slot
* Test
* Revert "Revert "remove excess copies (#6142)""

This reverts commit f1222b5e77f9356dd370182808d9c1e5fc3f5e12.
* Merge refs/heads/master into fix-ancestor
This commit is contained in:
terence tsao 2020-06-13 23:58:36 -07:00 committed by GitHub
parent 0067e52b1e
commit 87ba5a5993
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 60 additions and 12 deletions

View File

@ -380,14 +380,14 @@ func (s *Service) filterBlockRoots(ctx context.Context, roots [][32]byte) ([][32
// ancestor returns the block root of an ancestry block from the input block root.
//
// Spec pseudocode definition:
// def get_ancestor(store: Store, root: Hash, slot: Slot) -> Hash:
// def get_ancestor(store: Store, root: Root, slot: Slot) -> Root:
// block = store.blocks[root]
// if block.slot > slot:
// return get_ancestor(store, block.parent_root, slot)
// elif block.slot == slot:
// return root
// else:
// # root is older than queried slot, thus a skip slot. Return most recent root prior to slot.
// # root is older than queried slot, thus a skip slot. Return most recent root prior to slot
// return root
func (s *Service) ancestor(ctx context.Context, root []byte, slot uint64) ([]byte, error) {
ctx, span := trace.StartSpan(ctx, "forkchoice.ancestor")
@ -412,13 +412,7 @@ func (s *Service) ancestor(ctx context.Context, root []byte, slot uint64) ([]byt
}
b := signed.Block
// If we dont have the ancestor in the DB, simply return nil so rest of fork choice
// operation can proceed. This is not an error condition.
if b == nil || b.Slot < slot {
return nil, nil
}
if b.Slot == slot {
if b.Slot == slot || b.Slot < slot {
return root, nil
}

View File

@ -779,3 +779,57 @@ func TestCurrentSlot_HandlesOverflow(t *testing.T) {
t.Fatalf("Expected slot to be 0, got %d", slot)
}
}
func TestAncestor_HandleSkipSlot(t *testing.T) {
ctx := context.Background()
db := testDB.SetupDB(t)
cfg := &Config{BeaconDB: db}
service, err := NewService(ctx, cfg)
if err != nil {
t.Fatal(err)
}
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)
}
b200 := &ethpb.BeaconBlock{Slot: 200, ParentRoot: r100[:]}
r200, err := ssz.HashTreeRoot(b200)
if err != nil {
t.Fatal(err)
}
for _, b := range []*ethpb.BeaconBlock{b1, b100, b200} {
beaconBlock := testutil.NewBeaconBlock()
beaconBlock.Block.Slot = b.Slot
beaconBlock.Block.ParentRoot = bytesutil.PadTo(b.ParentRoot, 32)
beaconBlock.Block.Body = &ethpb.BeaconBlockBody{}
if err := db.SaveBlock(context.Background(), beaconBlock); err != nil {
t.Fatal(err)
}
}
// Slots 100 to 200 are skip slots. Requesting root at 150 will yield root at 100. The last physical block.
r, err := service.ancestor(context.Background(), r200[:], 150)
if err != nil {
t.Fatal(err)
}
if bytesutil.ToBytes32(r) != r100 {
t.Error("Did not get correct root")
}
// Slots 1 to 100 are skip slots. Requesting root at 50 will yield root at 1. The last physical block.
r, err = service.ancestor(context.Background(), r200[:], 50)
if err != nil {
t.Fatal(err)
}
if bytesutil.ToBytes32(r) != r1 {
t.Error("Did not get correct root")
}
}