diff --git a/beacon-chain/blockchain/error.go b/beacon-chain/blockchain/error.go index 8b085e79d491..64338cecec5a 100644 --- a/beacon-chain/blockchain/error.go +++ b/beacon-chain/blockchain/error.go @@ -27,6 +27,8 @@ var ( errWSBlockNotFound = errors.New("weak subjectivity root not found in db") // errWSBlockNotFoundInEpoch is returned when a block is not found in the WS cache or DB within epoch. errWSBlockNotFoundInEpoch = errors.New("weak subjectivity root not found in db within epoch") + // errNotDescendantOfFinalized is returned when a block is not a descendant of the finalized checkpoint + errNotDescendantOfFinalized = invalidBlock{errors.New("not descendant of finalized checkpoint")} ) // An invalid block is the block that fails state transition based on the core protocol rules. diff --git a/beacon-chain/blockchain/process_block_helpers.go b/beacon-chain/blockchain/process_block_helpers.go index cd071bbb38a1..e66fa89ca598 100644 --- a/beacon-chain/blockchain/process_block_helpers.go +++ b/beacon-chain/blockchain/process_block_helpers.go @@ -349,8 +349,6 @@ func (s *Service) fillInForkChoiceMissingBlocks(ctx context.Context, blk interfa pendingNodes := make([]interfaces.BeaconBlock, 0) pendingRoots := make([][32]byte, 0) - parentRoot := bytesutil.ToBytes32(blk.ParentRoot()) - slot := blk.Slot() // Fork choice only matters from last finalized slot. finalized, err := s.store.FinalizedCheckpt() if err != nil { @@ -360,20 +358,23 @@ func (s *Service) fillInForkChoiceMissingBlocks(ctx context.Context, blk interfa if err != nil { return err } - higherThanFinalized := slot > fSlot // As long as parent node is not in fork choice store, and parent node is in DB. - for !s.cfg.ForkChoiceStore.HasNode(parentRoot) && s.cfg.BeaconDB.HasBlock(ctx, parentRoot) && higherThanFinalized { - b, err := s.getBlock(ctx, parentRoot) + root := bytesutil.ToBytes32(blk.ParentRoot()) + for !s.cfg.ForkChoiceStore.HasNode(root) && s.cfg.BeaconDB.HasBlock(ctx, root) { + b, err := s.getBlock(ctx, root) if err != nil { return err } - + if b.Block().Slot() <= fSlot { + break + } pendingNodes = append(pendingNodes, b.Block()) - copiedRoot := parentRoot + copiedRoot := root pendingRoots = append(pendingRoots, copiedRoot) - parentRoot = bytesutil.ToBytes32(b.Block().ParentRoot()) - slot = b.Block().Slot() - higherThanFinalized = slot > fSlot + root = bytesutil.ToBytes32(b.Block().ParentRoot()) + } + if len(pendingRoots) > 0 && root != bytesutil.ToBytes32(finalized.Root) { + return errNotDescendantOfFinalized } // Insert parent nodes to fork choice store in reverse order. diff --git a/beacon-chain/blockchain/process_block_test.go b/beacon-chain/blockchain/process_block_test.go index 48772093afd1..0bb58397162d 100644 --- a/beacon-chain/blockchain/process_block_test.go +++ b/beacon-chain/blockchain/process_block_test.go @@ -730,7 +730,6 @@ func TestFillForkChoiceMissingBlocks_CanSave_ProtoArray(t *testing.T) { service, err := NewService(ctx, opts...) require.NoError(t, err) service.cfg.ForkChoiceStore = protoarray.New(0, 0) - service.store.SetFinalizedCheckptAndPayloadHash(ðpb.Checkpoint{Root: make([]byte, 32)}, [32]byte{}) genesisStateRoot := [32]byte{} genesis := blocks.NewGenesisBlock(genesisStateRoot[:]) @@ -753,12 +752,14 @@ func TestFillForkChoiceMissingBlocks_CanSave_ProtoArray(t *testing.T) { wsb, err = wrapper.WrappedSignedBeaconBlock(blk) require.NoError(t, err) + service.store.SetFinalizedCheckptAndPayloadHash(ðpb.Checkpoint{Root: roots[0]}, [32]byte{}) err = service.fillInForkChoiceMissingBlocks( context.Background(), wsb.Block(), beaconState.FinalizedCheckpoint(), beaconState.CurrentJustifiedCheckpoint()) require.NoError(t, err) - // 5 nodes from the block tree 1. B0 - B3 - B4 - B6 - B8 - assert.Equal(t, 5, service.cfg.ForkChoiceStore.NodeCount(), "Miss match nodes") + // 4 nodes from the block tree 1. B3 - B4 - B6 - B8 + assert.Equal(t, 4, service.cfg.ForkChoiceStore.NodeCount(), "Miss match nodes") + assert.Equal(t, true, service.cfg.ForkChoiceStore.HasNode(bytesutil.ToBytes32(roots[3])), "Didn't save node") assert.Equal(t, true, service.cfg.ForkChoiceStore.HasNode(bytesutil.ToBytes32(roots[4])), "Didn't save node") assert.Equal(t, true, service.cfg.ForkChoiceStore.HasNode(bytesutil.ToBytes32(roots[6])), "Didn't save node") assert.Equal(t, true, service.cfg.ForkChoiceStore.HasNode(bytesutil.ToBytes32(roots[8])), "Didn't save node") @@ -775,7 +776,6 @@ func TestFillForkChoiceMissingBlocks_CanSave_DoublyLinkedTree(t *testing.T) { service, err := NewService(ctx, opts...) require.NoError(t, err) service.cfg.ForkChoiceStore = doublylinkedtree.New(0, 0) - service.store.SetFinalizedCheckptAndPayloadHash(ðpb.Checkpoint{Root: make([]byte, 32)}, [32]byte{}) genesisStateRoot := [32]byte{} genesis := blocks.NewGenesisBlock(genesisStateRoot[:]) @@ -799,12 +799,14 @@ func TestFillForkChoiceMissingBlocks_CanSave_DoublyLinkedTree(t *testing.T) { wsb, err = wrapper.WrappedSignedBeaconBlock(blk) require.NoError(t, err) + service.store.SetFinalizedCheckptAndPayloadHash(ðpb.Checkpoint{Root: roots[0]}, [32]byte{}) err = service.fillInForkChoiceMissingBlocks( context.Background(), wsb.Block(), beaconState.FinalizedCheckpoint(), beaconState.CurrentJustifiedCheckpoint()) require.NoError(t, err) // 5 nodes from the block tree 1. B0 - B3 - B4 - B6 - B8 - assert.Equal(t, 5, service.cfg.ForkChoiceStore.NodeCount(), "Miss match nodes") + assert.Equal(t, 4, service.cfg.ForkChoiceStore.NodeCount(), "Miss match nodes") + assert.Equal(t, true, service.cfg.ForkChoiceStore.HasNode(bytesutil.ToBytes32(roots[3])), "Didn't save node") assert.Equal(t, true, service.cfg.ForkChoiceStore.HasNode(bytesutil.ToBytes32(roots[4])), "Didn't save node") assert.Equal(t, true, service.cfg.ForkChoiceStore.HasNode(bytesutil.ToBytes32(roots[6])), "Didn't save node") assert.Equal(t, true, service.cfg.ForkChoiceStore.HasNode(bytesutil.ToBytes32(roots[8])), "Didn't save node") @@ -821,7 +823,6 @@ func TestFillForkChoiceMissingBlocks_RootsMatch_ProtoArray(t *testing.T) { service, err := NewService(ctx, opts...) require.NoError(t, err) service.cfg.ForkChoiceStore = protoarray.New(0, 0) - service.store.SetFinalizedCheckptAndPayloadHash(ðpb.Checkpoint{Root: make([]byte, 32)}, [32]byte{}) genesisStateRoot := [32]byte{} genesis := blocks.NewGenesisBlock(genesisStateRoot[:]) @@ -845,14 +846,15 @@ func TestFillForkChoiceMissingBlocks_RootsMatch_ProtoArray(t *testing.T) { wsb, err = wrapper.WrappedSignedBeaconBlock(blk) require.NoError(t, err) + service.store.SetFinalizedCheckptAndPayloadHash(ðpb.Checkpoint{Root: roots[0]}, [32]byte{}) err = service.fillInForkChoiceMissingBlocks( context.Background(), wsb.Block(), beaconState.FinalizedCheckpoint(), beaconState.CurrentJustifiedCheckpoint()) require.NoError(t, err) - // 5 nodes from the block tree 1. B0 - B3 - B4 - B6 - B8 - assert.Equal(t, 5, service.cfg.ForkChoiceStore.NodeCount(), "Miss match nodes") + // 4 nodes from the block tree 1. B3 - B4 - B6 - B8 + assert.Equal(t, 4, service.cfg.ForkChoiceStore.NodeCount(), "Miss match nodes") // Ensure all roots and their respective blocks exist. - wantedRoots := [][]byte{roots[0], roots[3], roots[4], roots[6], roots[8]} + wantedRoots := [][]byte{roots[3], roots[4], roots[6], roots[8]} for i, rt := range wantedRoots { assert.Equal(t, true, service.cfg.ForkChoiceStore.HasNode(bytesutil.ToBytes32(rt)), fmt.Sprintf("Didn't save node: %d", i)) assert.Equal(t, true, service.cfg.BeaconDB.HasBlock(context.Background(), bytesutil.ToBytes32(rt))) @@ -870,7 +872,6 @@ func TestFillForkChoiceMissingBlocks_RootsMatch_DoublyLinkedTree(t *testing.T) { service, err := NewService(ctx, opts...) require.NoError(t, err) service.cfg.ForkChoiceStore = doublylinkedtree.New(0, 0) - service.store.SetFinalizedCheckptAndPayloadHash(ðpb.Checkpoint{Root: make([]byte, 32)}, [32]byte{}) genesisStateRoot := [32]byte{} genesis := blocks.NewGenesisBlock(genesisStateRoot[:]) @@ -894,14 +895,15 @@ func TestFillForkChoiceMissingBlocks_RootsMatch_DoublyLinkedTree(t *testing.T) { wsb, err = wrapper.WrappedSignedBeaconBlock(blk) require.NoError(t, err) + service.store.SetFinalizedCheckptAndPayloadHash(ðpb.Checkpoint{Root: roots[0]}, [32]byte{}) err = service.fillInForkChoiceMissingBlocks( context.Background(), wsb.Block(), beaconState.FinalizedCheckpoint(), beaconState.CurrentJustifiedCheckpoint()) require.NoError(t, err) - // 5 nodes from the block tree 1. B0 - B3 - B4 - B6 - B8 - assert.Equal(t, 5, service.cfg.ForkChoiceStore.NodeCount(), "Miss match nodes") + // 5 nodes from the block tree 1. B3 - B4 - B6 - B8 + assert.Equal(t, 4, service.cfg.ForkChoiceStore.NodeCount(), "Miss match nodes") // Ensure all roots and their respective blocks exist. - wantedRoots := [][]byte{roots[0], roots[3], roots[4], roots[6], roots[8]} + wantedRoots := [][]byte{roots[3], roots[4], roots[6], roots[8]} for i, rt := range wantedRoots { assert.Equal(t, true, service.cfg.ForkChoiceStore.HasNode(bytesutil.ToBytes32(rt)), fmt.Sprintf("Didn't save node: %d", i)) assert.Equal(t, true, service.cfg.BeaconDB.HasBlock(context.Background(), bytesutil.ToBytes32(rt))) @@ -919,8 +921,6 @@ func TestFillForkChoiceMissingBlocks_FilterFinalized_ProtoArray(t *testing.T) { service, err := NewService(ctx, opts...) require.NoError(t, err) service.cfg.ForkChoiceStore = protoarray.New(0, 0) - // Set finalized epoch to 1. - service.store.SetFinalizedCheckptAndPayloadHash(ðpb.Checkpoint{Epoch: 1}, [32]byte{}) genesisStateRoot := [32]byte{} genesis := blocks.NewGenesisBlock(genesisStateRoot[:]) @@ -934,7 +934,7 @@ func TestFillForkChoiceMissingBlocks_FilterFinalized_ProtoArray(t *testing.T) { require.NoError(t, service.cfg.BeaconDB.SaveState(ctx, st.Copy(), validGenesisRoot)) - // Define a tree branch, slot 63 <- 64 <- 65 + // Define a tree branch, slot 63 <- 64 <- 65 <- 66 b63 := util.NewBeaconBlock() b63.Block.Slot = 63 wsb, err = wrapper.WrappedSignedBeaconBlock(b63) @@ -953,20 +953,28 @@ func TestFillForkChoiceMissingBlocks_FilterFinalized_ProtoArray(t *testing.T) { b65 := util.NewBeaconBlock() b65.Block.Slot = 65 b65.Block.ParentRoot = r64[:] + r65, err := b65.Block.HashTreeRoot() + require.NoError(t, err) wsb, err = wrapper.WrappedSignedBeaconBlock(b65) require.NoError(t, err) require.NoError(t, service.cfg.BeaconDB.SaveBlock(ctx, wsb)) + b66 := util.NewBeaconBlock() + b66.Block.Slot = 66 + b66.Block.ParentRoot = r65[:] + wsb, err = wrapper.WrappedSignedBeaconBlock(b66) + require.NoError(t, err) + require.NoError(t, service.cfg.BeaconDB.SaveBlock(ctx, wsb)) beaconState, _ := util.DeterministicGenesisState(t, 32) + // Set finalized epoch to 2. + service.store.SetFinalizedCheckptAndPayloadHash(ðpb.Checkpoint{Epoch: 2, Root: r64[:]}, [32]byte{}) err = service.fillInForkChoiceMissingBlocks( context.Background(), wsb.Block(), beaconState.FinalizedCheckpoint(), beaconState.CurrentJustifiedCheckpoint()) require.NoError(t, err) - // There should be 2 nodes, block 65 and block 64. - assert.Equal(t, 2, service.cfg.ForkChoiceStore.NodeCount(), "Miss match nodes") - - // Block with slot 63 should be in fork choice because it's less than finalized epoch 1. - assert.Equal(t, true, service.cfg.ForkChoiceStore.HasNode(r63), "Didn't save node") + // We should have saved 1 node: block 65 + assert.Equal(t, 1, service.cfg.ForkChoiceStore.NodeCount(), "Miss match nodes") + assert.Equal(t, true, service.cfg.ForkChoiceStore.HasNode(r65), "Didn't save node") } func TestFillForkChoiceMissingBlocks_FilterFinalized_DoublyLinkedTree(t *testing.T) { @@ -980,8 +988,6 @@ func TestFillForkChoiceMissingBlocks_FilterFinalized_DoublyLinkedTree(t *testing service, err := NewService(ctx, opts...) require.NoError(t, err) service.cfg.ForkChoiceStore = doublylinkedtree.New(0, 0) - // Set finalized epoch to 1. - service.store.SetFinalizedCheckptAndPayloadHash(ðpb.Checkpoint{Epoch: 1}, [32]byte{}) genesisStateRoot := [32]byte{} genesis := blocks.NewGenesisBlock(genesisStateRoot[:]) @@ -1014,27 +1020,75 @@ func TestFillForkChoiceMissingBlocks_FilterFinalized_DoublyLinkedTree(t *testing b65 := util.NewBeaconBlock() b65.Block.Slot = 65 b65.Block.ParentRoot = r64[:] + r65, err := b65.Block.HashTreeRoot() + require.NoError(t, err) wsb, err = wrapper.WrappedSignedBeaconBlock(b65) require.NoError(t, err) require.NoError(t, service.cfg.BeaconDB.SaveBlock(ctx, wsb)) + b66 := util.NewBeaconBlock() + b66.Block.Slot = 66 + b66.Block.ParentRoot = r65[:] + wsb, err = wrapper.WrappedSignedBeaconBlock(b66) + require.NoError(t, err) + require.NoError(t, service.cfg.BeaconDB.SaveBlock(ctx, wsb)) beaconState, _ := util.DeterministicGenesisState(t, 32) + + // Set finalized epoch to 1. + service.store.SetFinalizedCheckptAndPayloadHash(ðpb.Checkpoint{Epoch: 2, Root: r64[:]}, [32]byte{}) err = service.fillInForkChoiceMissingBlocks( context.Background(), wsb.Block(), beaconState.FinalizedCheckpoint(), beaconState.CurrentJustifiedCheckpoint()) require.NoError(t, err) - // There should be 2 nodes, block 65 and block 64. - assert.Equal(t, 2, service.cfg.ForkChoiceStore.NodeCount(), "Miss match nodes") + // There should be 1 node: block 65 + assert.Equal(t, 1, service.cfg.ForkChoiceStore.NodeCount(), "Miss match nodes") + assert.Equal(t, true, service.cfg.ForkChoiceStore.HasNode(r65), "Didn't save node") +} - // Block with slot 63 should be in fork choice because it's less than finalized epoch 1. - assert.Equal(t, true, service.cfg.ForkChoiceStore.HasNode(r63), "Didn't save node") +func TestFillForkChoiceMissingBlocks_FinalizedSibling_DoublyLinkedTree(t *testing.T) { + ctx := context.Background() + beaconDB := testDB.SetupDB(t) + + opts := []Option{ + WithDatabase(beaconDB), + WithStateGen(stategen.New(beaconDB)), + } + service, err := NewService(ctx, opts...) + require.NoError(t, err) + service.cfg.ForkChoiceStore = doublylinkedtree.New(0, 0) + + genesisStateRoot := [32]byte{} + genesis := blocks.NewGenesisBlock(genesisStateRoot[:]) + wsb, err := wrapper.WrappedSignedBeaconBlock(genesis) + require.NoError(t, err) + require.NoError(t, beaconDB.SaveBlock(ctx, wsb)) + validGenesisRoot, err := genesis.Block.HashTreeRoot() + require.NoError(t, err) + st, err := util.NewBeaconState() + require.NoError(t, err) + + require.NoError(t, service.cfg.BeaconDB.SaveState(ctx, st.Copy(), validGenesisRoot)) + roots, err := blockTree1(t, beaconDB, validGenesisRoot[:]) + require.NoError(t, err) + + beaconState, _ := util.DeterministicGenesisState(t, 32) + blk := util.NewBeaconBlock() + blk.Block.Slot = 9 + blk.Block.ParentRoot = roots[8] + + wsb, err = wrapper.WrappedSignedBeaconBlock(blk) + require.NoError(t, err) + + service.store.SetFinalizedCheckptAndPayloadHash(ðpb.Checkpoint{Root: roots[1]}, [32]byte{}) + err = service.fillInForkChoiceMissingBlocks( + context.Background(), wsb.Block(), beaconState.FinalizedCheckpoint(), beaconState.CurrentJustifiedCheckpoint()) + require.ErrorIs(t, errNotDescendantOfFinalized, err) } // blockTree1 constructs the following tree: // /- B1 // B0 /- B5 - B7 // \- B3 - B4 - B6 - B8 -// (B1, and B3 are all from the same slots) func blockTree1(t *testing.T, beaconDB db.Database, genesisRoot []byte) ([][]byte, error) { genesisRoot = bytesutil.PadTo(genesisRoot, 32) b0 := util.NewBeaconBlock()