diff --git a/beacon-chain/db/iface/interface.go b/beacon-chain/db/iface/interface.go index 95770ef8b6..8023237b90 100644 --- a/beacon-chain/db/iface/interface.go +++ b/beacon-chain/db/iface/interface.go @@ -19,7 +19,7 @@ import ( type ReadOnlyDatabase interface { // Block related methods. Block(ctx context.Context, blockRoot [32]byte) (*eth.SignedBeaconBlock, error) - Blocks(ctx context.Context, f *filters.QueryFilter) ([]*eth.SignedBeaconBlock, error) + Blocks(ctx context.Context, f *filters.QueryFilter) ([]*eth.SignedBeaconBlock, [][32]byte, error) BlockRoots(ctx context.Context, f *filters.QueryFilter) ([][32]byte, error) HasBlock(ctx context.Context, blockRoot [32]byte) bool GenesisBlock(ctx context.Context) (*eth.SignedBeaconBlock, error) diff --git a/beacon-chain/db/kafka/passthrough.go b/beacon-chain/db/kafka/passthrough.go index a60c46a714..288dc077f6 100644 --- a/beacon-chain/db/kafka/passthrough.go +++ b/beacon-chain/db/kafka/passthrough.go @@ -37,7 +37,7 @@ func (e Exporter) HeadBlock(ctx context.Context) (*eth.SignedBeaconBlock, error) } // Blocks -- passthrough. -func (e Exporter) Blocks(ctx context.Context, f *filters.QueryFilter) ([]*eth.SignedBeaconBlock, error) { +func (e Exporter) Blocks(ctx context.Context, f *filters.QueryFilter) ([]*eth.SignedBeaconBlock, [][32]byte, error) { return e.db.Blocks(ctx, f) } diff --git a/beacon-chain/db/kv/blocks.go b/beacon-chain/db/kv/blocks.go index 922ffb1214..1f59b58a5e 100644 --- a/beacon-chain/db/kv/blocks.go +++ b/beacon-chain/db/kv/blocks.go @@ -58,11 +58,13 @@ func (s *Store) HeadBlock(ctx context.Context) (*ethpb.SignedBeaconBlock, error) return headBlock, err } -// Blocks retrieves a list of beacon blocks by filter criteria. -func (s *Store) Blocks(ctx context.Context, f *filters.QueryFilter) ([]*ethpb.SignedBeaconBlock, error) { +// Blocks retrieves a list of beacon blocks and its respective roots by filter criteria. +func (s *Store) Blocks(ctx context.Context, f *filters.QueryFilter) ([]*ethpb.SignedBeaconBlock, [][32]byte, error) { ctx, span := trace.StartSpan(ctx, "BeaconDB.Blocks") defer span.End() blocks := make([]*ethpb.SignedBeaconBlock, 0) + blockRoots := make([][32]byte, 0) + err := s.db.View(func(tx *bolt.Tx) error { bkt := tx.Bucket(blocksBucket) @@ -78,13 +80,18 @@ func (s *Store) Blocks(ctx context.Context, f *filters.QueryFilter) ([]*ethpb.Si return err } blocks = append(blocks, block) + blockRoots = append(blockRoots, bytesutil.ToBytes32(keys[i])) } return nil }) - return blocks, err + return blocks, blockRoots, err } -// BlockRoots retrieves a list of beacon block roots by filter criteria. +// BlockRoots retrieves a list of beacon block roots by filter criteria. If the caller +// requires both the blocks and the block roots for a certain filter they should instead +// use the Blocks function rather than use BlockRoots. During periods of non finality +// there are potential race conditions which leads to differing roots when calling the db +// multiple times for the same filter. func (s *Store) BlockRoots(ctx context.Context, f *filters.QueryFilter) ([][32]byte, error) { ctx, span := trace.StartSpan(ctx, "BeaconDB.BlockRoots") defer span.End() diff --git a/beacon-chain/db/kv/blocks_test.go b/beacon-chain/db/kv/blocks_test.go index 5ae7ae7985..1bedccb4c4 100644 --- a/beacon-chain/db/kv/blocks_test.go +++ b/beacon-chain/db/kv/blocks_test.go @@ -35,7 +35,7 @@ func TestStore_SaveBlock_NoDuplicates(t *testing.T) { require.NoError(t, db.SaveBlock(ctx, block)) } f := filters.NewFilter().SetStartSlot(slot).SetEndSlot(slot) - retrieved, err := db.Blocks(ctx, f) + retrieved, _, err := db.Blocks(ctx, f) require.NoError(t, err) assert.Equal(t, 1, len(retrieved)) // We reset the block cache size. @@ -85,13 +85,13 @@ func TestStore_BlocksBatchDelete(t *testing.T) { } } require.NoError(t, db.SaveBlocks(ctx, totalBlocks)) - retrieved, err := db.Blocks(ctx, filters.NewFilter().SetParentRoot(bytesutil.PadTo([]byte("parent"), 32))) + retrieved, _, err := db.Blocks(ctx, filters.NewFilter().SetParentRoot(bytesutil.PadTo([]byte("parent"), 32))) require.NoError(t, err) assert.Equal(t, numBlocks, len(retrieved), "Unexpected number of blocks received") // We delete all even indexed blocks. require.NoError(t, db.deleteBlocks(ctx, blockRoots)) // When we retrieve the data, only the odd indexed blocks should remain. - retrieved, err = db.Blocks(ctx, filters.NewFilter().SetParentRoot(bytesutil.PadTo([]byte("parent"), 32))) + retrieved, _, err = db.Blocks(ctx, filters.NewFilter().SetParentRoot(bytesutil.PadTo([]byte("parent"), 32))) require.NoError(t, err) sort.Slice(retrieved, func(i, j int) bool { return retrieved[i].Block.Slot < retrieved[j].Block.Slot @@ -213,7 +213,7 @@ func TestStore_Blocks_FiltersCorrectly(t *testing.T) { }, } for _, tt := range tests { - retrievedBlocks, err := db.Blocks(ctx, tt.filter) + retrievedBlocks, _, err := db.Blocks(ctx, tt.filter) require.NoError(t, err) assert.Equal(t, tt.expectedNumBlocks, len(retrievedBlocks), "Unexpected number of blocks") } @@ -252,7 +252,7 @@ func TestStore_Blocks_Retrieve_SlotRange(t *testing.T) { } ctx := context.Background() require.NoError(t, db.SaveBlocks(ctx, totalBlocks)) - retrieved, err := db.Blocks(ctx, filters.NewFilter().SetStartSlot(100).SetEndSlot(399)) + retrieved, _, err := db.Blocks(ctx, filters.NewFilter().SetStartSlot(100).SetEndSlot(399)) require.NoError(t, err) assert.Equal(t, 300, len(retrieved)) } @@ -269,11 +269,11 @@ func TestStore_Blocks_Retrieve_Epoch(t *testing.T) { } ctx := context.Background() require.NoError(t, db.SaveBlocks(ctx, totalBlocks)) - retrieved, err := db.Blocks(ctx, filters.NewFilter().SetStartEpoch(5).SetEndEpoch(6)) + retrieved, _, err := db.Blocks(ctx, filters.NewFilter().SetStartEpoch(5).SetEndEpoch(6)) require.NoError(t, err) want := params.BeaconConfig().SlotsPerEpoch * 2 assert.Equal(t, want, uint64(len(retrieved))) - retrieved, err = db.Blocks(ctx, filters.NewFilter().SetStartEpoch(0).SetEndEpoch(0)) + retrieved, _, err = db.Blocks(ctx, filters.NewFilter().SetStartEpoch(0).SetEndEpoch(0)) require.NoError(t, err) want = params.BeaconConfig().SlotsPerEpoch assert.Equal(t, want, uint64(len(retrieved))) @@ -291,7 +291,7 @@ func TestStore_Blocks_Retrieve_SlotRangeWithStep(t *testing.T) { const step = 2 ctx := context.Background() require.NoError(t, db.SaveBlocks(ctx, totalBlocks)) - retrieved, err := db.Blocks(ctx, filters.NewFilter().SetStartSlot(100).SetEndSlot(399).SetSlotStep(step)) + retrieved, _, err := db.Blocks(ctx, filters.NewFilter().SetStartSlot(100).SetEndSlot(399).SetSlotStep(step)) require.NoError(t, err) assert.Equal(t, 150, len(retrieved)) for _, b := range retrieved { @@ -375,7 +375,33 @@ func TestStore_SaveBlocks_HasCachedBlocks(t *testing.T) { require.NoError(t, db.SaveBlocks(ctx, b)) f := filters.NewFilter().SetStartSlot(0).SetEndSlot(500) - blks, err := db.Blocks(ctx, f) + blks, _, err := db.Blocks(ctx, f) require.NoError(t, err) assert.Equal(t, 500, len(blks), "Did not get wanted blocks") } + +func TestStore_SaveBlocks_HasRootsMatched(t *testing.T) { + db := setupDB(t) + ctx := context.Background() + + b := make([]*ethpb.SignedBeaconBlock, 500) + for i := 0; i < 500; i++ { + blk := testutil.NewBeaconBlock() + blk.Block.ParentRoot = bytesutil.PadTo([]byte("parent"), 32) + blk.Block.Slot = uint64(i) + b[i] = blk + } + + require.NoError(t, db.SaveBlocks(ctx, b)) + f := filters.NewFilter().SetStartSlot(0).SetEndSlot(500) + + blks, roots, err := db.Blocks(ctx, f) + require.NoError(t, err) + assert.Equal(t, 500, len(blks), "Did not get wanted blocks") + + for i, blk := range blks { + rt, err := blk.Block.HashTreeRoot() + require.NoError(t, err) + assert.Equal(t, roots[i], rt, "mismatch of block roots") + } +} diff --git a/beacon-chain/rpc/beacon/attestations.go b/beacon-chain/rpc/beacon/attestations.go index 3c8c6bfe31..130f68a74b 100644 --- a/beacon-chain/rpc/beacon/attestations.go +++ b/beacon-chain/rpc/beacon/attestations.go @@ -64,12 +64,12 @@ func (bs *Server) ListAttestations( var err error switch q := req.QueryFilter.(type) { case *ethpb.ListAttestationsRequest_GenesisEpoch: - blocks, err = bs.BeaconDB.Blocks(ctx, filters.NewFilter().SetStartEpoch(0).SetEndEpoch(0)) + blocks, _, err = bs.BeaconDB.Blocks(ctx, filters.NewFilter().SetStartEpoch(0).SetEndEpoch(0)) if err != nil { return nil, status.Errorf(codes.Internal, "Could not fetch attestations: %v", err) } case *ethpb.ListAttestationsRequest_Epoch: - blocks, err = bs.BeaconDB.Blocks(ctx, filters.NewFilter().SetStartEpoch(q.Epoch).SetEndEpoch(q.Epoch)) + blocks, _, err = bs.BeaconDB.Blocks(ctx, filters.NewFilter().SetStartEpoch(q.Epoch).SetEndEpoch(q.Epoch)) if err != nil { return nil, status.Errorf(codes.Internal, "Could not fetch attestations: %v", err) } @@ -118,12 +118,12 @@ func (bs *Server) ListIndexedAttestations( var err error switch q := req.QueryFilter.(type) { case *ethpb.ListIndexedAttestationsRequest_GenesisEpoch: - blocks, err = bs.BeaconDB.Blocks(ctx, filters.NewFilter().SetStartEpoch(0).SetEndEpoch(0)) + blocks, _, err = bs.BeaconDB.Blocks(ctx, filters.NewFilter().SetStartEpoch(0).SetEndEpoch(0)) if err != nil { return nil, status.Errorf(codes.Internal, "Could not fetch attestations: %v", err) } case *ethpb.ListIndexedAttestationsRequest_Epoch: - blocks, err = bs.BeaconDB.Blocks(ctx, filters.NewFilter().SetStartEpoch(q.Epoch).SetEndEpoch(q.Epoch)) + blocks, _, err = bs.BeaconDB.Blocks(ctx, filters.NewFilter().SetStartEpoch(q.Epoch).SetEndEpoch(q.Epoch)) if err != nil { return nil, status.Errorf(codes.Internal, "Could not fetch attestations: %v", err) } diff --git a/beacon-chain/rpc/beacon/blocks.go b/beacon-chain/rpc/beacon/blocks.go index 0c39d0a5c8..c088b87eec 100644 --- a/beacon-chain/rpc/beacon/blocks.go +++ b/beacon-chain/rpc/beacon/blocks.go @@ -36,7 +36,7 @@ func (bs *Server) ListBlocks( switch q := req.QueryFilter.(type) { case *ethpb.ListBlocksRequest_Epoch: - blks, err := bs.BeaconDB.Blocks(ctx, filters.NewFilter().SetStartEpoch(q.Epoch).SetEndEpoch(q.Epoch)) + blks, _, err := bs.BeaconDB.Blocks(ctx, filters.NewFilter().SetStartEpoch(q.Epoch).SetEndEpoch(q.Epoch)) if err != nil { return nil, status.Errorf(codes.Internal, "Failed to get blocks: %v", err) } @@ -99,7 +99,7 @@ func (bs *Server) ListBlocks( }, nil case *ethpb.ListBlocksRequest_Slot: - blks, err := bs.BeaconDB.Blocks(ctx, filters.NewFilter().SetStartSlot(q.Slot).SetEndSlot(q.Slot)) + blks, _, err := bs.BeaconDB.Blocks(ctx, filters.NewFilter().SetStartSlot(q.Slot).SetEndSlot(q.Slot)) if err != nil { return nil, status.Errorf(codes.Internal, "Could not retrieve blocks for slot %d: %v", q.Slot, err) } diff --git a/beacon-chain/rpc/beaconv1/blocks.go b/beacon-chain/rpc/beaconv1/blocks.go index aad5f9d721..0f721bfbfb 100644 --- a/beacon-chain/rpc/beaconv1/blocks.go +++ b/beacon-chain/rpc/beaconv1/blocks.go @@ -63,13 +63,14 @@ func (bs *Server) GetBlockHeader(ctx context.Context, req *ethpb.BlockRequest) ( func (bs *Server) ListBlockHeaders(ctx context.Context, req *ethpb.BlockHeadersRequest) (*ethpb.BlockHeadersResponse, error) { var err error var blks []*ethpb_alpha.SignedBeaconBlock + var blkRoots [][32]byte if len(req.ParentRoot) == 32 { - blks, err = bs.BeaconDB.Blocks(ctx, filters.NewFilter().SetParentRoot(req.ParentRoot)) + blks, blkRoots, err = bs.BeaconDB.Blocks(ctx, filters.NewFilter().SetParentRoot(req.ParentRoot)) if err != nil { return nil, status.Errorf(codes.Internal, "Could not retrieve blocks: %v", err) } } else { - blks, err = bs.BeaconDB.Blocks(ctx, filters.NewFilter().SetStartSlot(req.Slot).SetEndSlot(req.Slot)) + blks, blkRoots, err = bs.BeaconDB.Blocks(ctx, filters.NewFilter().SetStartSlot(req.Slot).SetEndSlot(req.Slot)) if err != nil { return nil, status.Errorf(codes.Internal, "Could not retrieve blocks for slot %d: %v", req.Slot, err) } @@ -84,11 +85,7 @@ func (bs *Server) ListBlockHeaders(ctx context.Context, req *ethpb.BlockHeadersR if err != nil { return nil, status.Errorf(codes.Internal, "Could not get block header from block: %v", err) } - blkRoot, err := blk.Block.HashTreeRoot() - if err != nil { - return nil, status.Errorf(codes.Internal, "Could not hash block: %v", err) - } - canonical, err := bs.ChainInfoFetcher.IsCanonical(ctx, blkRoot) + canonical, err := bs.ChainInfoFetcher.IsCanonical(ctx, blkRoots[i]) if err != nil { return nil, status.Errorf(codes.Internal, "Could not determine if block root is canonical: %v", err) } @@ -301,7 +298,7 @@ func (bs *Server) blockFromBlockID(ctx context.Context, blockId []byte) (*ethpb_ if err != nil { return nil, errors.Wrap(err, "could not decode block id") } - blks, err := bs.BeaconDB.Blocks(ctx, filters.NewFilter().SetStartSlot(slot).SetEndSlot(slot)) + blks, roots, err := bs.BeaconDB.Blocks(ctx, filters.NewFilter().SetStartSlot(slot).SetEndSlot(slot)) if err != nil { return nil, errors.Wrapf(err, "could not retrieve blocks for slot %d", slot) } @@ -314,12 +311,8 @@ func (bs *Server) blockFromBlockID(ctx context.Context, blockId []byte) (*ethpb_ if numBlks == 1 { break } - for _, block := range blks { - blkRoot, err := block.Block.HashTreeRoot() - if err != nil { - return nil, status.Errorf(codes.Internal, "Could not hash block: %v", err) - } - canonical, err := bs.ChainInfoFetcher.IsCanonical(ctx, blkRoot) + for i, block := range blks { + canonical, err := bs.ChainInfoFetcher.IsCanonical(ctx, roots[i]) if err != nil { return nil, status.Errorf(codes.Internal, "Could not determine if block root is canonical: %v", err) } diff --git a/beacon-chain/rpc/debug/block.go b/beacon-chain/rpc/debug/block.go index a02b6fa58c..e2753e10c6 100644 --- a/beacon-chain/rpc/debug/block.go +++ b/beacon-chain/rpc/debug/block.go @@ -55,7 +55,7 @@ func (ds *Server) GetInclusionSlot(ctx context.Context, req *pbrpc.InclusionSlot endSlot := req.Slot + params.BeaconConfig().SlotsPerEpoch filter := filters.NewFilter().SetStartSlot(startSlot).SetEndSlot(endSlot) - blks, err := ds.BeaconDB.Blocks(ctx, filter) + blks, _, err := ds.BeaconDB.Blocks(ctx, filter) if err != nil { return nil, status.Errorf(codes.Internal, "Could not retrieve blocks: %v", err) } diff --git a/beacon-chain/state/stategen/replay.go b/beacon-chain/state/stategen/replay.go index 9b34fc89ea..3b8179cf9a 100644 --- a/beacon-chain/state/stategen/replay.go +++ b/beacon-chain/state/stategen/replay.go @@ -51,12 +51,7 @@ func (s *State) ReplayBlocks(ctx context.Context, state *stateTrie.BeaconState, // The Blocks are returned in slot-descending order. func (s *State) LoadBlocks(ctx context.Context, startSlot, endSlot uint64, endBlockRoot [32]byte) ([]*ethpb.SignedBeaconBlock, error) { filter := filters.NewFilter().SetStartSlot(startSlot).SetEndSlot(endSlot) - blocks, err := s.beaconDB.Blocks(ctx, filter) - if err != nil { - return nil, err - } - - blockRoots, err := s.beaconDB.BlockRoots(ctx, filter) + blocks, blockRoots, err := s.beaconDB.Blocks(ctx, filter) if err != nil { return nil, err } @@ -252,11 +247,7 @@ func (s *State) genesisRoot(ctx context.Context) ([32]byte, error) { // Since hot states don't have finalized blocks, this should ONLY be used for replaying cold state. func (s *State) loadFinalizedBlocks(ctx context.Context, startSlot, endSlot uint64) ([]*ethpb.SignedBeaconBlock, error) { f := filters.NewFilter().SetStartSlot(startSlot).SetEndSlot(endSlot) - bs, err := s.beaconDB.Blocks(ctx, f) - if err != nil { - return nil, err - } - bRoots, err := s.beaconDB.BlockRoots(ctx, f) + bs, bRoots, err := s.beaconDB.Blocks(ctx, f) if err != nil { return nil, err } diff --git a/beacon-chain/sync/rpc_beacon_blocks_by_range.go b/beacon-chain/sync/rpc_beacon_blocks_by_range.go index 836634761a..a9399734d2 100644 --- a/beacon-chain/sync/rpc_beacon_blocks_by_range.go +++ b/beacon-chain/sync/rpc_beacon_blocks_by_range.go @@ -114,24 +114,13 @@ func (s *Service) writeBlockRangeToStream(ctx context.Context, startSlot, endSlo defer span.End() filter := filters.NewFilter().SetStartSlot(startSlot).SetEndSlot(endSlot).SetSlotStep(step) - blks, err := s.db.Blocks(ctx, filter) + blks, roots, err := s.db.Blocks(ctx, filter) if err != nil { log.WithError(err).Debug("Failed to retrieve blocks") s.writeErrorResponseToStream(responseCodeServerError, genericError, stream) traceutil.AnnotateError(span, err) return err } - roots := make([][32]byte, 0, len(blks)) - for _, b := range blks { - root, err := b.Block.HashTreeRoot() - if err != nil { - log.WithError(err).Debug("Failed to retrieve block root") - s.writeErrorResponseToStream(responseCodeServerError, genericError, stream) - traceutil.AnnotateError(span, err) - return err - } - roots = append(roots, root) - } // handle genesis case if startSlot == 0 { genBlock, genRoot, err := s.retrieveGenesisBlock(ctx) diff --git a/tools/blocktree/main.go b/tools/blocktree/main.go index c9227d7b55..a7ae43db66 100644 --- a/tools/blocktree/main.go +++ b/tools/blocktree/main.go @@ -50,7 +50,7 @@ func main() { startSlot := uint64(*startSlot) endSlot := uint64(*endSlot) filter := filters.NewFilter().SetStartSlot(startSlot).SetEndSlot(endSlot) - blks, err := db.Blocks(context.Background(), filter) + blks, roots, err := db.Blocks(context.Background(), filter) if err != nil { panic(err) } @@ -59,10 +59,7 @@ func main() { m := make(map[[32]byte]*node) for i := 0; i < len(blks); i++ { b := blks[i] - r, err := b.Block.HashTreeRoot() - if err != nil { - panic(err) - } + r := roots[i] m[r] = &node{score: make(map[uint64]bool)} state, err := db.State(context.Background(), r) @@ -74,15 +71,11 @@ func main() { for state == nil { slot-- filter := filters.NewFilter().SetStartSlot(slot).SetEndSlot(slot) - bs, err := db.Blocks(context.Background(), filter) + rts, err := db.BlockRoots(context.Background(), filter) if err != nil { panic(err) } - rs, err := bs[0].Block.HashTreeRoot() - if err != nil { - panic(err) - } - state, err = db.State(context.Background(), rs) + state, err = db.State(context.Background(), rts[0]) if err != nil { panic(err) }