diff --git a/beacon-chain/db/iface/interface.go b/beacon-chain/db/iface/interface.go index e3848db597..fcfdeaffba 100644 --- a/beacon-chain/db/iface/interface.go +++ b/beacon-chain/db/iface/interface.go @@ -23,14 +23,14 @@ type ReadOnlyDatabase interface { Block(ctx context.Context, blockRoot [32]byte) (interfaces.SignedBeaconBlock, error) Blocks(ctx context.Context, f *filters.QueryFilter) ([]interfaces.SignedBeaconBlock, [][32]byte, error) BlockRoots(ctx context.Context, f *filters.QueryFilter) ([][32]byte, error) - BlocksBySlot(ctx context.Context, slot types.Slot) (bool, []interfaces.SignedBeaconBlock, error) + BlocksBySlot(ctx context.Context, slot types.Slot) ([]interfaces.SignedBeaconBlock, error) BlockRootsBySlot(ctx context.Context, slot types.Slot) (bool, [][32]byte, error) HasBlock(ctx context.Context, blockRoot [32]byte) bool GenesisBlock(ctx context.Context) (interfaces.SignedBeaconBlock, error) GenesisBlockRoot(ctx context.Context) ([32]byte, error) IsFinalizedBlock(ctx context.Context, blockRoot [32]byte) bool FinalizedChildBlock(ctx context.Context, blockRoot [32]byte) (interfaces.SignedBeaconBlock, error) - HighestSlotBlocksBelow(ctx context.Context, slot types.Slot) ([]interfaces.SignedBeaconBlock, error) + HighestRootsBelowSlot(ctx context.Context, slot types.Slot) (types.Slot, [][32]byte, error) // State related methods. State(ctx context.Context, blockRoot [32]byte) (state.BeaconState, error) StateOrError(ctx context.Context, blockRoot [32]byte) (state.BeaconState, error) diff --git a/beacon-chain/db/kv/blocks.go b/beacon-chain/db/kv/blocks.go index 07d8bdb9e7..5dba268c7c 100644 --- a/beacon-chain/db/kv/blocks.go +++ b/beacon-chain/db/kv/blocks.go @@ -185,17 +185,19 @@ func (s *Store) HasBlock(ctx context.Context, blockRoot [32]byte) bool { } // BlocksBySlot retrieves a list of beacon blocks and its respective roots by slot. -func (s *Store) BlocksBySlot(ctx context.Context, slot types.Slot) (bool, []interfaces.SignedBeaconBlock, error) { +func (s *Store) BlocksBySlot(ctx context.Context, slot types.Slot) ([]interfaces.SignedBeaconBlock, error) { ctx, span := trace.StartSpan(ctx, "BeaconDB.BlocksBySlot") defer span.End() - blocks := make([]interfaces.SignedBeaconBlock, 0) + blocks := make([]interfaces.SignedBeaconBlock, 0) err := s.db.View(func(tx *bolt.Tx) error { bkt := tx.Bucket(blocksBucket) - - keys := blockRootsBySlot(ctx, tx, slot) - for i := 0; i < len(keys); i++ { - encoded := bkt.Get(keys[i]) + roots, err := blockRootsBySlot(ctx, tx, slot) + if err != nil { + return errors.Wrap(err, "could not retrieve blocks by slot") + } + for _, r := range roots { + encoded := bkt.Get(r[:]) blk, err := unmarshalBlock(ctx, encoded) if err != nil { return err @@ -204,7 +206,7 @@ func (s *Store) BlocksBySlot(ctx context.Context, slot types.Slot) (bool, []inte } return nil }) - return len(blocks) > 0, blocks, err + return blocks, err } // BlockRootsBySlot retrieves a list of beacon block roots by slot @@ -213,11 +215,9 @@ func (s *Store) BlockRootsBySlot(ctx context.Context, slot types.Slot) (bool, [] defer span.End() blockRoots := make([][32]byte, 0) err := s.db.View(func(tx *bolt.Tx) error { - keys := blockRootsBySlot(ctx, tx, slot) - for i := 0; i < len(keys); i++ { - blockRoots = append(blockRoots, bytesutil.ToBytes32(keys[i])) - } - return nil + var err error + blockRoots, err = blockRootsBySlot(ctx, tx, slot) + return err }) if err != nil { return false, nil, errors.Wrap(err, "could not retrieve block roots by slot") @@ -398,14 +398,17 @@ func (s *Store) SaveBackfillBlockRoot(ctx context.Context, blockRoot [32]byte) e }) } -// HighestSlotBlocksBelow returns the block with the highest slot below the input slot from the db. -func (s *Store) HighestSlotBlocksBelow(ctx context.Context, slot types.Slot) ([]interfaces.SignedBeaconBlock, error) { - ctx, span := trace.StartSpan(ctx, "BeaconDB.HighestSlotBlocksBelow") +// HighestRootsBelowSlot returns roots from the database slot index from the highest slot below the input slot. +// The slot value at the beginning of the return list is the slot where the roots were found. This is helpful so that +// calling code can make decisions based on the slot without resolving the blocks to discover their slot (for instance +// checking which root is canonical in fork choice, which operates purely on roots, +// then if no canonical block is found, continuing to search through lower slots). +func (s *Store) HighestRootsBelowSlot(ctx context.Context, slot types.Slot) (fs types.Slot, roots [][32]byte, err error) { + ctx, span := trace.StartSpan(ctx, "BeaconDB.HighestRootsBelowSlot") defer span.End() - var root [32]byte sk := bytesutil.Uint64ToBytesBigEndian(uint64(slot)) - err := s.db.View(func(tx *bolt.Tx) error { + err = s.db.View(func(tx *bolt.Tx) error { bkt := tx.Bucket(blockSlotIndicesBucket) c := bkt.Cursor() // The documentation for Seek says: @@ -430,34 +433,28 @@ func (s *Store) HighestSlotBlocksBelow(ctx context.Context, slot types.Slot) ([] if r == nil { continue } - bs := bytesutil.BytesToSlotBigEndian(sl) + fs = bytesutil.BytesToSlotBigEndian(sl) // Iterating through the index using .Prev will move from higher to lower, so the first key we find behind // the requested slot must be the highest block below that slot. - if slot > bs { - root = bytesutil.ToBytes32(r) - break + if slot > fs { + roots, err = splitRoots(r) + if err != nil { + return errors.Wrapf(err, "error parsing packed roots %#x", r) + } + return nil } } return nil }) if err != nil { - return nil, err + return 0, nil, err + } + if len(roots) == 0 || (len(roots) == 1 && roots[0] == params.BeaconConfig().ZeroHash) { + gr, err := s.GenesisBlockRoot(ctx) + return 0, [][32]byte{gr}, err } - var blk interfaces.SignedBeaconBlock - if root != params.BeaconConfig().ZeroHash { - blk, err = s.Block(ctx, root) - if err != nil { - return nil, err - } - } - if blk == nil || blk.IsNil() { - blk, err = s.GenesisBlock(ctx) - if err != nil { - return nil, err - } - } - return []interfaces.SignedBeaconBlock{blk}, nil + return fs, roots, nil } // FeeRecipientByValidatorID returns the fee recipient for a validator id. @@ -681,21 +678,22 @@ func blockRootsBySlotRange( } // blockRootsBySlot retrieves the block roots by slot -func blockRootsBySlot(ctx context.Context, tx *bolt.Tx, slot types.Slot) [][]byte { +func blockRootsBySlot(ctx context.Context, tx *bolt.Tx, slot types.Slot) ([][32]byte, error) { _, span := trace.StartSpan(ctx, "BeaconDB.blockRootsBySlot") defer span.End() - roots := make([][]byte, 0) bkt := tx.Bucket(blockSlotIndicesBucket) key := bytesutil.SlotToBytesBigEndian(slot) c := bkt.Cursor() k, v := c.Seek(key) if k != nil && bytes.Equal(k, key) { - for i := 0; i < len(v); i += 32 { - roots = append(roots, v[i:i+32]) + r, err := splitRoots(v) + if err != nil { + return nil, errors.Wrapf(err, "corrupt value in block slot index for slot=%d", slot) } + return r, nil } - return roots + return [][32]byte{}, nil } // createBlockIndicesFromBlock takes in a beacon block and returns diff --git a/beacon-chain/db/kv/blocks_test.go b/beacon-chain/db/kv/blocks_test.go index 160fc3bb09..5785609eeb 100644 --- a/beacon-chain/db/kv/blocks_test.go +++ b/beacon-chain/db/kv/blocks_test.go @@ -517,18 +517,32 @@ func TestStore_SaveBlock_CanGetHighestAt(t *testing.T) { require.NoError(t, db.SaveBlock(ctx, block2)) require.NoError(t, db.SaveBlock(ctx, block3)) - highestAt, err := db.HighestSlotBlocksBelow(ctx, 2) + _, roots, err := db.HighestRootsBelowSlot(ctx, 2) require.NoError(t, err) - assert.Equal(t, false, len(highestAt) <= 0, "Got empty highest at slice") - assert.Equal(t, true, proto.Equal(block1.Proto(), highestAt[0].Proto()), "Wanted: %v, received: %v", block1, highestAt[0]) - highestAt, err = db.HighestSlotBlocksBelow(ctx, 11) + assert.Equal(t, false, len(roots) <= 0, "Got empty highest at slice") + require.Equal(t, 1, len(roots)) + root := roots[0] + b, err := db.Block(ctx, root) require.NoError(t, err) - assert.Equal(t, false, len(highestAt) <= 0, "Got empty highest at slice") - assert.Equal(t, true, proto.Equal(block2.Proto(), highestAt[0].Proto()), "Wanted: %v, received: %v", block2, highestAt[0]) - highestAt, err = db.HighestSlotBlocksBelow(ctx, 101) + assert.Equal(t, true, proto.Equal(block1.Proto(), b.Proto()), "Wanted: %v, received: %v", block1, b) + + _, roots, err = db.HighestRootsBelowSlot(ctx, 11) require.NoError(t, err) - assert.Equal(t, false, len(highestAt) <= 0, "Got empty highest at slice") - assert.Equal(t, true, proto.Equal(block3.Proto(), highestAt[0].Proto()), "Wanted: %v, received: %v", block3, highestAt[0]) + assert.Equal(t, false, len(roots) <= 0, "Got empty highest at slice") + require.Equal(t, 1, len(roots)) + root = roots[0] + b, err = db.Block(ctx, root) + require.NoError(t, err) + assert.Equal(t, true, proto.Equal(block2.Proto(), b.Proto()), "Wanted: %v, received: %v", block2, b) + + _, roots, err = db.HighestRootsBelowSlot(ctx, 101) + require.NoError(t, err) + assert.Equal(t, false, len(roots) <= 0, "Got empty highest at slice") + require.Equal(t, 1, len(roots)) + root = roots[0] + b, err = db.Block(ctx, root) + require.NoError(t, err) + assert.Equal(t, true, proto.Equal(block3.Proto(), b.Proto()), "Wanted: %v, received: %v", block3, b) }) } } @@ -549,15 +563,29 @@ func TestStore_GenesisBlock_CanGetHighestAt(t *testing.T) { require.NoError(t, err) require.NoError(t, db.SaveBlock(ctx, block1)) - highestAt, err := db.HighestSlotBlocksBelow(ctx, 2) + _, roots, err := db.HighestRootsBelowSlot(ctx, 2) require.NoError(t, err) - assert.Equal(t, true, proto.Equal(block1.Proto(), highestAt[0].Proto()), "Wanted: %v, received: %v", block1, highestAt[0]) - highestAt, err = db.HighestSlotBlocksBelow(ctx, 1) + require.Equal(t, 1, len(roots)) + root := roots[0] + b, err := db.Block(ctx, root) require.NoError(t, err) - assert.Equal(t, true, proto.Equal(genesisBlock.Proto(), highestAt[0].Proto()), "Wanted: %v, received: %v", genesisBlock, highestAt[0]) - highestAt, err = db.HighestSlotBlocksBelow(ctx, 0) + assert.Equal(t, true, proto.Equal(block1.Proto(), b.Proto()), "Wanted: %v, received: %v", block1, b) + + _, roots, err = db.HighestRootsBelowSlot(ctx, 1) require.NoError(t, err) - assert.Equal(t, true, proto.Equal(genesisBlock.Proto(), highestAt[0].Proto()), "Wanted: %v, received: %v", genesisBlock, highestAt[0]) + require.Equal(t, 1, len(roots)) + root = roots[0] + b, err = db.Block(ctx, root) + require.NoError(t, err) + assert.Equal(t, true, proto.Equal(genesisBlock.Proto(), b.Proto()), "Wanted: %v, received: %v", genesisBlock, b) + + _, roots, err = db.HighestRootsBelowSlot(ctx, 0) + require.NoError(t, err) + require.Equal(t, 1, len(roots)) + root = roots[0] + b, err = db.Block(ctx, root) + require.NoError(t, err) + assert.Equal(t, true, proto.Equal(genesisBlock.Proto(), b.Proto()), "Wanted: %v, received: %v", genesisBlock, b) }) } } @@ -638,22 +666,21 @@ func TestStore_BlocksBySlot_BlockRootsBySlot(t *testing.T) { r3, err := b3.Block().HashTreeRoot() require.NoError(t, err) - hasBlocks, retrievedBlocks, err := db.BlocksBySlot(ctx, 1) + retrievedBlocks, err := db.BlocksBySlot(ctx, 1) require.NoError(t, err) assert.Equal(t, 0, len(retrievedBlocks), "Unexpected number of blocks received, expected none") - assert.Equal(t, false, hasBlocks, "Expected no blocks") - hasBlocks, retrievedBlocks, err = db.BlocksBySlot(ctx, 20) + retrievedBlocks, err = db.BlocksBySlot(ctx, 20) require.NoError(t, err) assert.Equal(t, true, proto.Equal(b1.Proto(), retrievedBlocks[0].Proto()), "Wanted: %v, received: %v", b1, retrievedBlocks[0]) - assert.Equal(t, true, hasBlocks, "Expected to have blocks") - hasBlocks, retrievedBlocks, err = db.BlocksBySlot(ctx, 100) + assert.Equal(t, true, len(retrievedBlocks) > 0, "Expected to have blocks") + retrievedBlocks, err = db.BlocksBySlot(ctx, 100) require.NoError(t, err) if len(retrievedBlocks) != 2 { t.Fatalf("Expected 2 blocks, received %d blocks", len(retrievedBlocks)) } assert.Equal(t, true, proto.Equal(b2.Proto(), retrievedBlocks[0].Proto()), "Wanted: %v, received: %v", b2, retrievedBlocks[0]) assert.Equal(t, true, proto.Equal(b3.Proto(), retrievedBlocks[1].Proto()), "Wanted: %v, received: %v", b3, retrievedBlocks[1]) - assert.Equal(t, true, hasBlocks, "Expected to have blocks") + assert.Equal(t, true, len(retrievedBlocks) > 0, "Expected to have blocks") hasBlockRoots, retrievedBlockRoots, err := db.BlockRootsBySlot(ctx, 1) require.NoError(t, err) diff --git a/beacon-chain/db/kv/utils.go b/beacon-chain/db/kv/utils.go index dde90f0952..88d2e98d45 100644 --- a/beacon-chain/db/kv/utils.go +++ b/beacon-chain/db/kv/utils.go @@ -4,6 +4,8 @@ import ( "bytes" "context" + "github.com/pkg/errors" + "github.com/prysmaticlabs/prysm/encoding/bytesutil" bolt "go.etcd.io/bbolt" "go.opencensus.io/trace" ) @@ -99,3 +101,16 @@ func deleteValueForIndices(ctx context.Context, indicesByBucket map[string][]byt } return nil } + +var errMisalignedRootList = errors.New("incorrectly packed root list, length is not a multiple of 32") + +func splitRoots(b []byte) ([][32]byte, error) { + rl := make([][32]byte, 0) + if len(b)%32 != 0 { + return nil, errors.Wrapf(errMisalignedRootList, "root list len=%d", len(b)) + } + for s, f := 0, 32; f <= len(b); s, f = f, f+32 { + rl = append(rl, bytesutil.ToBytes32(b[s:f])) + } + return rl, nil +} diff --git a/beacon-chain/db/kv/utils_test.go b/beacon-chain/db/kv/utils_test.go index 56f62dcd82..7690e4a12d 100644 --- a/beacon-chain/db/kv/utils_test.go +++ b/beacon-chain/db/kv/utils_test.go @@ -138,3 +138,60 @@ func Test_deleteValueForIndices(t *testing.T) { }) } } + +func testPack(bs [][32]byte) []byte { + r := make([]byte, 0) + for _, b := range bs { + r = append(r, b[:]...) + } + return r +} + +func TestSplitRoots(t *testing.T) { + bt := make([][32]byte, 0) + for _, x := range []byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9} { + var b [32]byte + for i := 0; i < 32; i++ { + b[i] = x + } + bt = append(bt, b) + } + cases := []struct { + name string + b []byte + expect [][32]byte + err error + }{ + { + name: "misaligned", + b: make([]byte, 61), + err: errMisalignedRootList, + }, + { + name: "happy", + b: testPack(bt[0:5]), + expect: bt[0:5], + }, + { + name: "single", + b: testPack([][32]byte{bt[0]}), + expect: [][32]byte{bt[0]}, + }, + { + name: "empty", + b: []byte{}, + expect: [][32]byte{}, + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + r, err := splitRoots(c.b) + if c.err != nil { + require.ErrorIs(t, err, c.err) + return + } + require.NoError(t, err) + require.DeepEqual(t, c.expect, r) + }) + } +} diff --git a/beacon-chain/rpc/eth/beacon/blocks.go b/beacon-chain/rpc/eth/beacon/blocks.go index 30d198d41b..34a9e97a1e 100644 --- a/beacon-chain/rpc/eth/beacon/blocks.go +++ b/beacon-chain/rpc/eth/beacon/blocks.go @@ -70,10 +70,14 @@ func (bs *Server) GetWeakSubjectivity(ctx context.Context, _ *empty.Empty) (*eth if err != nil { return nil, status.Errorf(codes.Internal, "could not get weak subjectivity slot: %v", err) } - cbr, cb, err := bs.CanonicalHistory.BlockForSlot(ctx, wsSlot) + cbr, err := bs.CanonicalHistory.BlockRootForSlot(ctx, wsSlot) if err != nil { return nil, status.Errorf(codes.Internal, fmt.Sprintf("could not find highest block below slot %d", wsSlot)) } + cb, err := bs.BeaconDB.Block(ctx, cbr) + if err != nil { + return nil, status.Errorf(codes.Internal, fmt.Sprintf("block with root %#x from slot index %d not found in db", cbr, wsSlot)) + } stateRoot := bytesutil.ToBytes32(cb.Block().StateRoot()) log.Printf("weak subjectivity checkpoint reported as epoch=%d, block root=%#x, state root=%#x", wsEpoch, cbr, stateRoot) return ðpbv1.WeakSubjectivityResponse{ @@ -150,7 +154,7 @@ func (bs *Server) ListBlockHeaders(ctx context.Context, req *ethpbv1.BlockHeader if req.Slot != nil { slot = *req.Slot } - _, blks, err = bs.BeaconDB.BlocksBySlot(ctx, slot) + blks, err = bs.BeaconDB.BlocksBySlot(ctx, slot) if err != nil { return nil, status.Errorf(codes.Internal, "Could not retrieve blocks for slot %d: %v", req.Slot, err) } @@ -754,7 +758,7 @@ func (bs *Server) blockFromBlockID(ctx context.Context, blockId []byte) (interfa e := newBlockIdParseError(err) return nil, &e } - _, blks, err := bs.BeaconDB.BlocksBySlot(ctx, types.Slot(slot)) + blks, err := bs.BeaconDB.BlocksBySlot(ctx, types.Slot(slot)) if err != nil { return nil, errors.Wrapf(err, "could not retrieve blocks for slot %d", slot) } diff --git a/beacon-chain/rpc/eth/beacon/blocks_test.go b/beacon-chain/rpc/eth/beacon/blocks_test.go index dacb0eddc7..10a64eced0 100644 --- a/beacon-chain/rpc/eth/beacon/blocks_test.go +++ b/beacon-chain/rpc/eth/beacon/blocks_test.go @@ -1527,8 +1527,8 @@ func TestServer_GetBlockSSZ(t *testing.T) { }, } - ok, blocks, err := beaconDB.BlocksBySlot(ctx, 30) - require.Equal(t, true, ok) + blocks, err := beaconDB.BlocksBySlot(ctx, 30) + require.Equal(t, true, len(blocks) > 0) require.NoError(t, err) sszBlock, err := blocks[0].MarshalSSZ() require.NoError(t, err) @@ -1567,8 +1567,8 @@ func TestServer_GetBlockSSZV2(t *testing.T) { }, } - ok, blocks, err := beaconDB.BlocksBySlot(ctx, 30) - require.Equal(t, true, ok) + blocks, err := beaconDB.BlocksBySlot(ctx, 30) + require.Equal(t, true, len(blocks) > 0) require.NoError(t, err) sszBlock, err := blocks[0].MarshalSSZ() require.NoError(t, err) @@ -1606,8 +1606,8 @@ func TestServer_GetBlockSSZV2(t *testing.T) { }, } - ok, blocks, err := beaconDB.BlocksBySlot(ctx, 30) - require.Equal(t, true, ok) + blocks, err := beaconDB.BlocksBySlot(ctx, 30) + require.Equal(t, true, len(blocks) > 0) require.NoError(t, err) sszBlock, err := blocks[0].MarshalSSZ() require.NoError(t, err) @@ -1645,8 +1645,8 @@ func TestServer_GetBlockSSZV2(t *testing.T) { }, } - ok, blocks, err := beaconDB.BlocksBySlot(ctx, 30) - require.Equal(t, true, ok) + blocks, err := beaconDB.BlocksBySlot(ctx, 30) + require.Equal(t, true, len(blocks) > 0) require.NoError(t, err) sszBlock, err := blocks[0].MarshalSSZ() require.NoError(t, err) diff --git a/beacon-chain/rpc/prysm/v1alpha1/beacon/blocks.go b/beacon-chain/rpc/prysm/v1alpha1/beacon/blocks.go index 0959bde31e..ca38d0a080 100644 --- a/beacon-chain/rpc/prysm/v1alpha1/beacon/blocks.go +++ b/beacon-chain/rpc/prysm/v1alpha1/beacon/blocks.go @@ -209,11 +209,11 @@ func (bs *Server) listBlocksForRoot(ctx context.Context, _ *ethpb.ListBlocksRequ // listBlocksForSlot retrieves all blocks for the provided slot. func (bs *Server) listBlocksForSlot(ctx context.Context, req *ethpb.ListBlocksRequest, q *ethpb.ListBlocksRequest_Slot) ([]blockContainer, int, string, error) { - hasBlocks, blks, err := bs.BeaconDB.BlocksBySlot(ctx, q.Slot) + blks, err := bs.BeaconDB.BlocksBySlot(ctx, q.Slot) if err != nil { return nil, 0, strconv.Itoa(0), status.Errorf(codes.Internal, "Could not retrieve blocks for slot %d: %v", q.Slot, err) } - if !hasBlocks { + if len(blks) == 0 { return []blockContainer{}, 0, strconv.Itoa(0), nil } diff --git a/beacon-chain/rpc/statefetcher/fetcher.go b/beacon-chain/rpc/statefetcher/fetcher.go index 831f9b20fc..7e29520068 100644 --- a/beacon-chain/rpc/statefetcher/fetcher.go +++ b/beacon-chain/rpc/statefetcher/fetcher.go @@ -292,11 +292,11 @@ func (p *StateProvider) stateRootBySlot(ctx context.Context, slot types.Slot) ([ if slot > currentSlot { return nil, errors.New("slot cannot be in the future") } - found, blks, err := p.BeaconDB.BlocksBySlot(ctx, slot) + blks, err := p.BeaconDB.BlocksBySlot(ctx, slot) if err != nil { return nil, errors.Wrap(err, "could not get blocks") } - if !found { + if len(blks) == 0 { return nil, errors.New("no block exists") } if len(blks) != 1 { diff --git a/beacon-chain/state/stategen/history.go b/beacon-chain/state/stategen/history.go index 1af93a6936..bb65c765d7 100644 --- a/beacon-chain/state/stategen/history.go +++ b/beacon-chain/state/stategen/history.go @@ -46,73 +46,58 @@ func (c *CanonicalHistory) ReplayerForSlot(target types.Slot) Replayer { return &stateReplayer{chainer: c, method: forSlot, target: target} } -func (c *CanonicalHistory) BlockForSlot(ctx context.Context, target types.Slot) ([32]byte, interfaces.SignedBeaconBlock, error) { - currentSlot := c.cs.CurrentSlot() - if target > currentSlot { - return [32]byte{}, nil, errors.Wrap(ErrFutureSlotRequested, fmt.Sprintf("requested=%d, current=%d", target, currentSlot)) +func (c *CanonicalHistory) BlockRootForSlot(ctx context.Context, target types.Slot) ([32]byte, error) { + if currentSlot := c.cs.CurrentSlot(); target > currentSlot { + return [32]byte{}, errors.Wrap(ErrFutureSlotRequested, fmt.Sprintf("requested=%d, current=%d", target, currentSlot)) } - for target > 0 { + + slotAbove := target + 1 + // don't bother searching for candidate roots when we know the target slot is genesis + for slotAbove > 1 { if ctx.Err() != nil { - return [32]byte{}, nil, errors.Wrap(ctx.Err(), "context canceled during canonicalBlockForSlot") + return [32]byte{}, errors.Wrap(ctx.Err(), "context canceled during canonicalBlockForSlot") } - hbs, err := c.h.HighestSlotBlocksBelow(ctx, target+1) + slot, roots, err := c.h.HighestRootsBelowSlot(ctx, slotAbove) if err != nil { - return [32]byte{}, nil, errors.Wrap(err, fmt.Sprintf("error finding highest block w/ slot <= %d", target)) + return [32]byte{}, errors.Wrap(err, fmt.Sprintf("error finding highest block w/ slot < %d", slotAbove)) } - if len(hbs) == 0 { - return [32]byte{}, nil, errors.Wrap(ErrNoBlocksBelowSlot, fmt.Sprintf("slot=%d", target)) + if len(roots) == 0 { + return [32]byte{}, errors.Wrap(ErrNoBlocksBelowSlot, fmt.Sprintf("slot=%d", slotAbove)) } - r, b, err := c.bestForSlot(ctx, hbs) + r, err := c.bestForSlot(ctx, roots) if err == nil { // we found a valid, canonical block! - return r, b, nil + return r, nil } // we found a block, but it wasn't considered canonical - keep looking if errors.Is(err, ErrNoCanonicalBlockForSlot) { // break once we've seen slot 0 (and prevent underflow) - if hbs[0].Block().Slot() == params.BeaconConfig().GenesisSlot { + if slot == params.BeaconConfig().GenesisSlot { break } - target = hbs[0].Block().Slot() - 1 + slotAbove = slot continue } - return [32]byte{}, nil, err + return [32]byte{}, err } - b, err := c.h.GenesisBlock(ctx) - if err != nil { - return [32]byte{}, nil, errors.Wrap(err, "db error while retrieving genesis block") - } - root, _, err := c.bestForSlot(ctx, []interfaces.SignedBeaconBlock{b}) - if err != nil { - return [32]byte{}, nil, errors.Wrap(err, "problem retrieving genesis block") - } - return root, b, nil + + return c.h.GenesisBlockRoot(ctx) } // bestForSlot encapsulates several messy realities of the underlying db code, looping through multiple blocks, // performing null/validity checks, and using CanonicalChecker to only pick canonical blocks. -func (c *CanonicalHistory) bestForSlot(ctx context.Context, hbs []interfaces.SignedBeaconBlock) ([32]byte, interfaces.SignedBeaconBlock, error) { - for _, b := range hbs { - if wrapper.BeaconBlockIsNil(b) != nil { - continue - } - root, err := b.Block().HashTreeRoot() - if err != nil { - // use this error message to wrap a sentinel error for error type matching - wrapped := errors.Wrap(ErrInvalidDBBlock, err.Error()) - msg := fmt.Sprintf("could not compute hash_tree_root for block at slot=%d", b.Block().Slot()) - return [32]byte{}, nil, errors.Wrap(wrapped, msg) - } +func (c *CanonicalHistory) bestForSlot(ctx context.Context, roots [][32]byte) ([32]byte, error) { + for _, root := range roots { canon, err := c.cc.IsCanonical(ctx, root) if err != nil { - return [32]byte{}, nil, errors.Wrap(err, "replayer could not check if block is canonical") + return [32]byte{}, errors.Wrap(err, "replayer could not check if block is canonical") } if canon { - return root, b, nil + return root, nil } } - return [32]byte{}, nil, errors.Wrap(ErrNoCanonicalBlockForSlot, "no good block for slot") + return [32]byte{}, errors.Wrap(ErrNoCanonicalBlockForSlot, "no good block for slot") } // ChainForSlot creates a value that satisfies the Replayer interface via db queries @@ -122,9 +107,13 @@ func (c *CanonicalHistory) bestForSlot(ctx context.Context, hbs []interfaces.Sig func (c *CanonicalHistory) chainForSlot(ctx context.Context, target types.Slot) (state.BeaconState, []interfaces.SignedBeaconBlock, error) { ctx, span := trace.StartSpan(ctx, "canonicalChainer.chainForSlot") defer span.End() - _, b, err := c.BlockForSlot(ctx, target) + r, err := c.BlockRootForSlot(ctx, target) if err != nil { - return nil, nil, errors.Wrap(err, fmt.Sprintf("unable to find replay data for slot=%d", target)) + return nil, nil, errors.Wrapf(err, "no canonical block root found below slot=%d", target) + } + b, err := c.h.Block(ctx, r) + if err != nil { + return nil, nil, errors.Wrapf(err, "unable to retrieve canonical block for slot, root=%#x", r) } s, descendants, err := c.ancestorChain(ctx, b) if err != nil { diff --git a/beacon-chain/state/stategen/history_test.go b/beacon-chain/state/stategen/history_test.go index 4c7314887c..dda1c657cb 100644 --- a/beacon-chain/state/stategen/history_test.go +++ b/beacon-chain/state/stategen/history_test.go @@ -12,8 +12,6 @@ import ( types "github.com/prysmaticlabs/prysm/consensus-types/primitives" "github.com/pkg/errors" - "github.com/prysmaticlabs/prysm/consensus-types/wrapper" - ethpb "github.com/prysmaticlabs/prysm/proto/prysm/v1alpha1" "github.com/prysmaticlabs/prysm/testing/require" ) @@ -21,7 +19,7 @@ func TestBlockForSlotFuture(t *testing.T) { ch := &CanonicalHistory{ cs: &mockCurrentSlotter{Slot: 0}, } - _, _, err := ch.BlockForSlot(context.Background(), 1) + _, err := ch.BlockRootForSlot(context.Background(), 1) require.ErrorIs(t, err, ErrFutureSlotRequested) } @@ -34,84 +32,54 @@ func TestChainForSlotFuture(t *testing.T) { } func TestBestForSlot(t *testing.T) { - nilBlock, err := wrapper.WrappedSignedBeaconBlock(ðpb.SignedBeaconBlock{}) - require.NoError(t, err) - nilBody, err := wrapper.WrappedSignedBeaconBlock(ðpb.SignedBeaconBlock{Block: ðpb.BeaconBlock{}}) - require.NoError(t, err) derp := errors.New("fake hash tree root method no hash good") - badHTR := &mock.SignedBeaconBlock{BeaconBlock: &mock.BeaconBlock{HtrErr: derp, BeaconBlockBody: &mock.BeaconBlockBody{}}} var goodHTR [32]byte copy(goodHTR[:], []byte{23}) var betterHTR [32]byte copy(betterHTR[:], []byte{42}) - good := &mock.SignedBeaconBlock{BeaconBlock: &mock.BeaconBlock{BeaconBlockBody: &mock.BeaconBlockBody{}, Htr: goodHTR}} - better := &mock.SignedBeaconBlock{BeaconBlock: &mock.BeaconBlock{BeaconBlockBody: &mock.BeaconBlockBody{}, Htr: betterHTR}} cases := []struct { name string err error blocks []interfaces.SignedBeaconBlock - best interfaces.SignedBeaconBlock + roots [][32]byte root [32]byte cc CanonicalChecker }{ { - name: "empty list", - err: ErrNoCanonicalBlockForSlot, - blocks: []interfaces.SignedBeaconBlock{}, + name: "empty list", + err: ErrNoCanonicalBlockForSlot, + roots: [][32]byte{}, }, { - name: "empty SignedBeaconBlock", - err: ErrNoCanonicalBlockForSlot, - blocks: []interfaces.SignedBeaconBlock{nil}, + name: "IsCanonical fail", + roots: [][32]byte{goodHTR, betterHTR}, + cc: &mockCanonicalChecker{is: true, err: derp}, + err: derp, }, { - name: "empty BeaconBlock", - err: ErrNoCanonicalBlockForSlot, - blocks: []interfaces.SignedBeaconBlock{nilBlock}, + name: "all non-canonical", + err: ErrNoCanonicalBlockForSlot, + roots: [][32]byte{goodHTR, betterHTR}, + cc: &mockCanonicalChecker{is: false}, }, { - name: "empty BeaconBlockBody", - err: ErrNoCanonicalBlockForSlot, - blocks: []interfaces.SignedBeaconBlock{nilBody}, + name: "one canonical", + cc: &mockCanonicalChecker{is: true}, + root: goodHTR, + roots: [][32]byte{goodHTR}, }, { - name: "bad HTR", - err: ErrInvalidDBBlock, - blocks: []interfaces.SignedBeaconBlock{badHTR}, + name: "all canonical", + cc: &mockCanonicalChecker{is: true}, + root: betterHTR, + roots: [][32]byte{betterHTR, goodHTR}, }, { - name: "IsCanonical fail", - blocks: []interfaces.SignedBeaconBlock{good, better}, - cc: &mockCanonicalChecker{is: true, err: derp}, - err: derp, - }, - { - name: "all non-canonical", - err: ErrNoCanonicalBlockForSlot, - blocks: []interfaces.SignedBeaconBlock{good, better}, - cc: &mockCanonicalChecker{is: false}, - }, - { - name: "one canonical", - blocks: []interfaces.SignedBeaconBlock{good}, - cc: &mockCanonicalChecker{is: true}, - root: goodHTR, - best: good, - }, - { - name: "all canonical", - blocks: []interfaces.SignedBeaconBlock{better, good}, - cc: &mockCanonicalChecker{is: true}, - root: betterHTR, - best: better, - }, - { - name: "first wins", - blocks: []interfaces.SignedBeaconBlock{good, better}, - cc: &mockCanonicalChecker{is: true}, - root: goodHTR, - best: good, + name: "first wins", + cc: &mockCanonicalChecker{is: true}, + root: goodHTR, + roots: [][32]byte{goodHTR, betterHTR}, }, } for _, c := range cases { @@ -121,10 +89,9 @@ func TestBestForSlot(t *testing.T) { chk = c.cc } ch := &CanonicalHistory{cc: chk} - r, b, err := ch.bestForSlot(context.Background(), c.blocks) + r, err := ch.bestForSlot(context.Background(), c.roots) if c.err == nil { require.NoError(t, err) - require.DeepEqual(t, c.best, b) require.Equal(t, c.root, r) } else { require.ErrorIs(t, err, c.err) @@ -164,13 +131,11 @@ func TestCanonicalBlockForSlotHappy(t *testing.T) { } for _, c := range cases { t.Run(c.name, func(t *testing.T) { - bs, err := hist.HighestSlotBlocksBelow(ctx, c.slot+1) + _, rs, err := hist.HighestRootsBelowSlot(ctx, c.slot+1) require.NoError(t, err) - require.Equal(t, len(bs), 1) - r, err := bs[0].Block().HashTreeRoot() - require.NoError(t, err) - require.Equal(t, hist.slotMap[c.highest], r) - cr, _, err := ch.BlockForSlot(ctx, c.slot) + require.Equal(t, len(rs), 1) + require.Equal(t, hist.slotMap[c.highest], rs[0]) + cr, err := ch.BlockRootForSlot(ctx, c.slot) require.NoError(t, err) require.Equal(t, hist.slotMap[c.canon], cr) }) @@ -187,47 +152,49 @@ func TestCanonicalBlockForSlotNonHappy(t *testing.T) { } hist := newMockHistory(t, specs, end+1) + genesis, err := hist.GenesisBlockRoot(ctx) + require.NoError(t, err) slotOrderObserved := make([]types.Slot, 0) - derp := errors.New("HighestSlotBlocksBelow don't work") + derp := errors.New("HighestRootsBelowSlot don't work") // since only the end block and genesis are canonical, once the slot drops below // end, we should always get genesis cases := []struct { name string slot types.Slot canon CanonicalChecker - overrideHighest func(context.Context, types.Slot) ([]interfaces.SignedBeaconBlock, error) + overrideHighest func(context.Context, types.Slot) (types.Slot, [][32]byte, error) slotOrderExpected []types.Slot err error root [32]byte }{ { - name: "HighestSlotBlocksBelow not called for genesis", - overrideHighest: func(_ context.Context, _ types.Slot) ([]interfaces.SignedBeaconBlock, error) { - return nil, derp + name: "HighestRootsBelowSlot not called for genesis", + overrideHighest: func(_ context.Context, _ types.Slot) (types.Slot, [][32]byte, error) { + return 0, [][32]byte{}, derp }, root: hist.slotMap[0], }, { - name: "wrapped error from HighestSlotBlocksBelow returned", + name: "wrapped error from HighestRootsBelowSlot returned", err: derp, - overrideHighest: func(_ context.Context, _ types.Slot) ([]interfaces.SignedBeaconBlock, error) { - return nil, derp + overrideHighest: func(_ context.Context, _ types.Slot) (types.Slot, [][32]byte, error) { + return 0, [][32]byte{}, derp }, slot: end, }, { - name: "HighestSlotBlocksBelow empty list", + name: "HighestRootsBelowSlot empty list", err: ErrNoBlocksBelowSlot, - overrideHighest: func(_ context.Context, _ types.Slot) ([]interfaces.SignedBeaconBlock, error) { - return []interfaces.SignedBeaconBlock{}, nil + overrideHighest: func(_ context.Context, _ types.Slot) (types.Slot, [][32]byte, error) { + return 0, [][32]byte{}, nil }, slot: end, }, { - name: "HighestSlotBlocksBelow no canonical", - err: ErrNoCanonicalBlockForSlot, + name: "HighestRootsBelowSlot no canonical", canon: &mockCanonicalChecker{is: false}, slot: end, + root: genesis, }, { name: "slot ordering correct - only genesis canonical", @@ -237,11 +204,11 @@ func TestCanonicalBlockForSlotNonHappy(t *testing.T) { } return false, nil }}, - overrideHighest: func(_ context.Context, s types.Slot) ([]interfaces.SignedBeaconBlock, error) { + overrideHighest: func(_ context.Context, s types.Slot) (types.Slot, [][32]byte, error) { slotOrderObserved = append(slotOrderObserved, s) - // this allows the mock HighestSlotBlocksBelow to continue to execute now that we've recorded + // this allows the mock HighestRootsBelowSlot to continue to execute now that we've recorded // the slot in our channel - return nil, errFallThroughOverride + return 0, nil, errFallThroughOverride }, slotOrderExpected: []types.Slot{156, 155, 150, 100}, slot: end, @@ -255,11 +222,11 @@ func TestCanonicalBlockForSlotNonHappy(t *testing.T) { } return false, nil }}, - overrideHighest: func(_ context.Context, s types.Slot) ([]interfaces.SignedBeaconBlock, error) { + overrideHighest: func(_ context.Context, s types.Slot) (types.Slot, [][32]byte, error) { slotOrderObserved = append(slotOrderObserved, s) - // this allows the mock HighestSlotBlocksBelow to continue to execute now that we've recorded + // this allows the mock HighestRootsBelowSlot to continue to execute now that we've recorded // the slot in our channel - return nil, errFallThroughOverride + return 0, nil, errFallThroughOverride }, slotOrderExpected: []types.Slot{156, 155, 150}, slot: end, @@ -274,14 +241,14 @@ func TestCanonicalBlockForSlotNonHappy(t *testing.T) { } ch := &CanonicalHistory{h: hist, cc: canon, cs: hist} hist.overrideHighestSlotBlocksBelow = c.overrideHighest - r, _, err := ch.BlockForSlot(ctx, c.slot) + r, err := ch.BlockRootForSlot(ctx, c.slot) if c.err == nil { require.NoError(t, err) } else { require.ErrorIs(t, err, c.err) } if len(c.slotOrderExpected) > 0 { - require.Equal(t, len(c.slotOrderExpected), len(slotOrderObserved), "HighestSlotBlocksBelow not called the expected number of times") + require.Equal(t, len(c.slotOrderExpected), len(slotOrderObserved), "HighestRootsBelowSlot not called the expected number of times") for i := range c.slotOrderExpected { require.Equal(t, c.slotOrderExpected[i], slotOrderObserved[i]) } diff --git a/beacon-chain/state/stategen/migrate.go b/beacon-chain/state/stategen/migrate.go index ca444e5833..dc5e85bf2c 100644 --- a/beacon-chain/state/stategen/migrate.go +++ b/beacon-chain/state/stategen/migrate.go @@ -55,24 +55,20 @@ func (s *State) MigrateToCold(ctx context.Context, fRoot [32]byte) error { aRoot = cached.root aState = cached.state } else { - blks, err := s.beaconDB.HighestSlotBlocksBelow(ctx, slot) + _, roots, err := s.beaconDB.HighestRootsBelowSlot(ctx, slot) if err != nil { return err } // Given the block has been finalized, the db should not have more than one block in a given slot. // We should error out when this happens. - if len(blks) != 1 { + if len(roots) != 1 { return errUnknownBlock } - missingRoot, err := blks[0].Block().HashTreeRoot() - if err != nil { - return err - } - aRoot = missingRoot + aRoot = roots[0] // There's no need to generate the state if the state already exists in the DB. // We can skip saving the state. if !s.beaconDB.HasState(ctx, aRoot) { - aState, err = s.StateByRoot(ctx, missingRoot) + aState, err = s.StateByRoot(ctx, aRoot) if err != nil { return err } diff --git a/beacon-chain/state/stategen/mock_test.go b/beacon-chain/state/stategen/mock_test.go index 73711fb1fe..10dc2d2af1 100644 --- a/beacon-chain/state/stategen/mock_test.go +++ b/beacon-chain/state/stategen/mock_test.go @@ -81,7 +81,7 @@ type mockHistory struct { states map[[32]byte]state.BeaconState hiddenStates map[[32]byte]state.BeaconState current types.Slot - overrideHighestSlotBlocksBelow func(context.Context, types.Slot) ([]interfaces.SignedBeaconBlock, error) + overrideHighestSlotBlocksBelow func(context.Context, types.Slot) (types.Slot, [][32]byte, error) } type slotList []types.Slot @@ -98,13 +98,13 @@ func (m slotList) Swap(i, j int) { m[i], m[j] = m[j], m[i] } -var errFallThroughOverride = errors.New("override yielding control back to real HighestSlotBlocksBelow") +var errFallThroughOverride = errors.New("override yielding control back to real HighestRootsBelowSlot") -func (m *mockHistory) HighestSlotBlocksBelow(_ context.Context, slot types.Slot) ([]interfaces.SignedBeaconBlock, error) { +func (m *mockHistory) HighestRootsBelowSlot(_ context.Context, slot types.Slot) (types.Slot, [][32]byte, error) { if m.overrideHighestSlotBlocksBelow != nil { - s, err := m.overrideHighestSlotBlocksBelow(context.Background(), slot) + s, r, err := m.overrideHighestSlotBlocksBelow(context.Background(), slot) if !errors.Is(err, errFallThroughOverride) { - return s, err + return s, r, err } } if len(m.slotIndex) == 0 && len(m.slotMap) > 0 { @@ -115,20 +115,20 @@ func (m *mockHistory) HighestSlotBlocksBelow(_ context.Context, slot types.Slot) } for _, s := range m.slotIndex { if s < slot { - return []interfaces.SignedBeaconBlock{m.blocks[m.slotMap[s]]}, nil + return s, [][32]byte{m.slotMap[s]}, nil } } - return []interfaces.SignedBeaconBlock{}, nil + return 0, [][32]byte{}, nil } var errGenesisBlockNotFound = errors.New("canonical genesis block not found in db") -func (m *mockHistory) GenesisBlock(_ context.Context) (interfaces.SignedBeaconBlock, error) { +func (m *mockHistory) GenesisBlockRoot(_ context.Context) ([32]byte, error) { genesisRoot, ok := m.slotMap[0] if !ok { - return nil, errGenesisBlockNotFound + return [32]byte{}, errGenesisBlockNotFound } - return m.blocks[genesisRoot], nil + return genesisRoot, nil } func (m *mockHistory) Block(_ context.Context, blockRoot [32]byte) (interfaces.SignedBeaconBlock, error) { diff --git a/beacon-chain/state/stategen/replayer.go b/beacon-chain/state/stategen/replayer.go index 04f2e78b9f..b40bf7db60 100644 --- a/beacon-chain/state/stategen/replayer.go +++ b/beacon-chain/state/stategen/replayer.go @@ -27,8 +27,8 @@ const ( // HistoryAccessor describes the minimum set of database methods needed to support the ReplayerBuilder. type HistoryAccessor interface { - HighestSlotBlocksBelow(ctx context.Context, slot types.Slot) ([]interfaces.SignedBeaconBlock, error) - GenesisBlock(ctx context.Context) (interfaces.SignedBeaconBlock, error) + HighestRootsBelowSlot(ctx context.Context, slot types.Slot) (types.Slot, [][32]byte, error) + GenesisBlockRoot(ctx context.Context) ([32]byte, error) Block(ctx context.Context, blockRoot [32]byte) (interfaces.SignedBeaconBlock, error) StateOrError(ctx context.Context, blockRoot [32]byte) (state.BeaconState, error) }