Apply fixes to block slots methods in DB (#5194)

* Apply fixes
* Typo
* Merge refs/heads/master into fix-slots-saved-for-blocks

Author: terencechain

beacon-chain/db/kv/blocks.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"context"
 	"fmt"
+	"math"
 	"strconv"
 
 	"github.com/pkg/errors"
@@ -302,9 +303,9 @@ func (k *Store) SaveGenesisBlockRoot(ctx context.Context, blockRoot [32]byte) er
 	})
 }
 
-// HighestSlotBlock returns the block with the highest slot from the db.
-func (k *Store) HighestSlotBlock(ctx context.Context) (*ethpb.SignedBeaconBlock, error) {
-	ctx, span := trace.StartSpan(ctx, "BeaconDB.HighestSlotBlock")
+// HighestSlotBlocks returns the blocks with the highest slot from the db.
+func (k *Store) HighestSlotBlocks(ctx context.Context) ([]*ethpb.SignedBeaconBlock, error) {
+	ctx, span := trace.StartSpan(ctx, "BeaconDB.HighestSlotBlocks")
 	defer span.End()
 
 	blocks := make([]*ethpb.SignedBeaconBlock, 0)
@@ -316,6 +317,7 @@ func (k *Store) HighestSlotBlock(ctx context.Context) (*ethpb.SignedBeaconBlock,
 			return err
 		}
 		highestSlot := highestIndex - 1
+		highestSlot = int(math.Max(0, float64(highestSlot)))
 		f := filters.NewFilter().SetStartSlot(uint64(highestSlot)).SetEndSlot(uint64(highestSlot))
 
 		keys, err := getBlockRootsByFilter(ctx, tx, f)
@@ -335,11 +337,7 @@ func (k *Store) HighestSlotBlock(ctx context.Context) (*ethpb.SignedBeaconBlock,
 		return nil
 	})
 
-	if len(blocks) != 1 {
-		return nil, errors.New("no highest slot block saved")
-	}
-
-	return blocks[0], err
+	return blocks, err
 }
 
 // setBlockSlotBitField sets the block slot bit in DB.
@@ -348,10 +346,17 @@ func (k *Store) setBlockSlotBitField(ctx context.Context, tx *bolt.Tx, slot uint
 	ctx, span := trace.StartSpan(ctx, "BeaconDB.setBlockSlotBitField")
 	defer span.End()
 
+	k.blockSlotBitLock.Lock()
+	defer k.blockSlotBitLock.Unlock()
+
 	bucket := tx.Bucket(slotsHasObjectBucket)
 	slotBitfields := bucket.Get(savedBlockSlotsKey)
+
+	// Copy is needed to avoid unsafe pointer conversions.
+	// See: https://github.com/etcd-io/bbolt/pull/201
 	tmp := make([]byte, len(slotBitfields))
 	copy(tmp, slotBitfields)
+
 	slotBitfields = bytesutil.SetBit(tmp, int(slot))
 	return bucket.Put(savedBlockSlotsKey, slotBitfields)
 }
@@ -362,10 +367,17 @@ func (k *Store) clearBlockSlotBitField(ctx context.Context, tx *bolt.Tx, slot ui
 	ctx, span := trace.StartSpan(ctx, "BeaconDB.clearBlockSlotBitField")
 	defer span.End()
 
+	k.blockSlotBitLock.Lock()
+	defer k.blockSlotBitLock.Unlock()
+
 	bucket := tx.Bucket(slotsHasObjectBucket)
 	slotBitfields := bucket.Get(savedBlockSlotsKey)
+
+	// Copy is needed to avoid unsafe pointer conversions.
+	// See: https://github.com/etcd-io/bbolt/pull/201
 	tmp := make([]byte, len(slotBitfields))
 	copy(tmp, slotBitfields)
+
 	slotBitfields = bytesutil.ClearBit(tmp, int(slot))
 	return bucket.Put(savedBlockSlotsKey, slotBitfields)
 }

beacon-chain/db/kv/blocks_test.go

@@ -429,35 +429,35 @@ func TestStore_SaveBlock_CanGetHighest(t *testing.T) {
 	if err := db.SaveBlock(ctx, block); err != nil {
 		t.Fatal(err)
 	}
-	highestSavedBlock, err := db.HighestSlotBlock(ctx)
+	highestSavedBlock, err := db.HighestSlotBlocks(ctx)
 	if err != nil {
 		t.Fatal(err)
 	}
-	if !proto.Equal(block, highestSavedBlock) {
+	if !proto.Equal(block, highestSavedBlock[0]) {
 		t.Errorf("Wanted %v, received %v", block, highestSavedBlock)
 	}
 
 	block = &ethpb.SignedBeaconBlock{Block: &ethpb.BeaconBlock{Slot: 999}}
 	if err := db.SaveBlock(ctx, block); err != nil {
 		t.Fatal(err)
 	}
-	highestSavedBlock, err = db.HighestSlotBlock(ctx)
+	highestSavedBlock, err = db.HighestSlotBlocks(ctx)
 	if err != nil {
 		t.Fatal(err)
 	}
-	if !proto.Equal(block, highestSavedBlock) {
+	if !proto.Equal(block, highestSavedBlock[0]) {
 		t.Errorf("Wanted %v, received %v", block, highestSavedBlock)
 	}
 
 	block = &ethpb.SignedBeaconBlock{Block: &ethpb.BeaconBlock{Slot: 300000000}} // 100 years.
 	if err := db.SaveBlock(ctx, block); err != nil {
 		t.Fatal(err)
 	}
-	highestSavedBlock, err = db.HighestSlotBlock(ctx)
+	highestSavedBlock, err = db.HighestSlotBlocks(ctx)
 	if err != nil {
 		t.Fatal(err)
 	}
-	if !proto.Equal(block, highestSavedBlock) {
+	if !proto.Equal(block, highestSavedBlock[0]) {
 		t.Errorf("Wanted %v, received %v", block, highestSavedBlock)
 	}
 }
@@ -480,11 +480,11 @@ func TestStore_SaveBlocks_CanGetHighest(t *testing.T) {
 	if err := db.SaveBlocks(ctx, b); err != nil {
 		t.Fatal(err)
 	}
-	highestSavedBlock, err := db.HighestSlotBlock(ctx)
+	highestSavedBlock, err := db.HighestSlotBlocks(ctx)
 	if err != nil {
 		t.Fatal(err)
 	}
-	if !proto.Equal(b[len(b)-1], highestSavedBlock) {
+	if !proto.Equal(b[len(b)-1], highestSavedBlock[0]) {
 		t.Errorf("Wanted %v, received %v", b[len(b)-1], highestSavedBlock)
 	}
 }
@@ -498,11 +498,11 @@ func TestStore_DeleteBlock_CanGetHighest(t *testing.T) {
 	if err := db.SaveBlock(ctx, b50); err != nil {
 		t.Fatal(err)
 	}
-	highestSavedBlock, err := db.HighestSlotBlock(ctx)
+	highestSavedBlock, err := db.HighestSlotBlocks(ctx)
 	if err != nil {
 		t.Fatal(err)
 	}
-	if !proto.Equal(b50, highestSavedBlock) {
+	if !proto.Equal(b50, highestSavedBlock[0]) {
 		t.Errorf("Wanted %v, received %v", b50, highestSavedBlock)
 	}
 
@@ -512,22 +512,22 @@ func TestStore_DeleteBlock_CanGetHighest(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	highestSavedBlock, err = db.HighestSlotBlock(ctx)
+	highestSavedBlock, err = db.HighestSlotBlocks(ctx)
 	if err != nil {
 		t.Fatal(err)
 	}
-	if !proto.Equal(b51, highestSavedBlock) {
+	if !proto.Equal(b51, highestSavedBlock[0]) {
 		t.Errorf("Wanted %v, received %v", b51, highestSavedBlock)
 	}
 
 	if err := db.DeleteBlock(ctx, r51); err != nil {
 		t.Fatal(err)
 	}
-	highestSavedBlock, err = db.HighestSlotBlock(ctx)
+	highestSavedBlock, err = db.HighestSlotBlocks(ctx)
 	if err != nil {
 		t.Fatal(err)
 	}
-	if !proto.Equal(b50, highestSavedBlock) {
+	if !proto.Equal(b50, highestSavedBlock[0]) {
 		t.Errorf("Wanted %v, received %v", b50, highestSavedBlock)
 	}
 }
@@ -555,11 +555,11 @@ func TestStore_DeleteBlocks_CanGetHighest(t *testing.T) {
 	if err := db.DeleteBlocks(ctx, [][32]byte{r[99], r[98], r[97]}); err != nil {
 		t.Fatal(err)
 	}
-	highestSavedBlock, err := db.HighestSlotBlock(ctx)
+	highestSavedBlock, err := db.HighestSlotBlocks(ctx)
 	if err != nil {
 		t.Fatal(err)
 	}
-	if !proto.Equal(b[96], highestSavedBlock) {
+	if !proto.Equal(b[96], highestSavedBlock[0]) {
 		t.Errorf("Wanted %v, received %v", b[len(b)-1], highestSavedBlock)
 	}
 }

beacon-chain/db/kv/kv.go

@@ -9,7 +9,7 @@ import (
 	"github.com/dgraph-io/ristretto"
 	"github.com/pkg/errors"
 	"github.com/prometheus/client_golang/prometheus"
-	"github.com/prysmaticlabs/prombbolt"
+	prombolt "github.com/prysmaticlabs/prombbolt"
 	"github.com/prysmaticlabs/prysm/beacon-chain/db/iface"
 	bolt "go.etcd.io/bbolt"
 )
@@ -37,6 +37,7 @@ type Store struct {
 	blockCache          *ristretto.Cache
 	validatorIndexCache *ristretto.Cache
 	stateSlotBitLock    sync.Mutex
+	blockSlotBitLock    sync.Mutex
 }
 
 // NewKVStore initializes a new boltDB key-value store at the directory