Revert "Change name and return type of HighestSlotBlocksBelow (#10811)" (#10818)

This reverts commit 0e9dfe04a1.

Co-authored-by: Kasey Kirkham <kasey@users.noreply.github.com>
This commit is contained in:
kasey
2022-06-03 13:36:06 -05:00
committed by GitHub
parent 3e36eacd1e
commit a170fd4bd6
8 changed files with 145 additions and 116 deletions

View File

@@ -30,7 +30,7 @@ type ReadOnlyDatabase interface {
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)
HighestBlockBelowSlot(ctx context.Context, slot types.Slot) (interfaces.SignedBeaconBlock, error)
HighestSlotBlocksBelow(ctx context.Context, slot types.Slot) ([]interfaces.SignedBeaconBlock, error)
// State related methods.
State(ctx context.Context, blockRoot [32]byte) (state.BeaconState, error)
StateOrError(ctx context.Context, blockRoot [32]byte) (state.BeaconState, error)

View File

@@ -398,9 +398,9 @@ func (s *Store) SaveBackfillBlockRoot(ctx context.Context, blockRoot [32]byte) e
})
}
// HighestBlockBelowSlot returns the block with the highest slot that's less than the slot argument.
func (s *Store) HighestBlockBelowSlot(ctx context.Context, slot types.Slot) (interfaces.SignedBeaconBlock, error) {
ctx, span := trace.StartSpan(ctx, "BeaconDB.HighestBlockBelowSlot")
// 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")
defer span.End()
var root [32]byte
@@ -457,7 +457,7 @@ func (s *Store) HighestBlockBelowSlot(ctx context.Context, slot types.Slot) (int
return nil, err
}
}
return blk, nil
return []interfaces.SignedBeaconBlock{blk}, nil
}
// FeeRecipientByValidatorID returns the fee recipient for a validator id.

View File

@@ -517,18 +517,18 @@ func TestStore_SaveBlock_CanGetHighestAt(t *testing.T) {
require.NoError(t, db.SaveBlock(ctx, block2))
require.NoError(t, db.SaveBlock(ctx, block3))
highestAt, err := db.HighestBlockBelowSlot(ctx, 2)
highestAt, err := db.HighestSlotBlocksBelow(ctx, 2)
require.NoError(t, err)
assert.NotNil(t, highestAt, "Got empty highest block")
assert.Equal(t, true, proto.Equal(block1.Proto(), highestAt.Proto()), "Wanted: %v, received: %v", block1, highestAt)
highestAt, err = db.HighestBlockBelowSlot(ctx, 11)
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)
require.NoError(t, err)
assert.NotNil(t, highestAt, "Got empty highest block")
assert.Equal(t, true, proto.Equal(block2.Proto(), highestAt.Proto()), "Wanted: %v, received: %v", block2, highestAt)
highestAt, err = db.HighestBlockBelowSlot(ctx, 101)
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)
require.NoError(t, err)
assert.NotNil(t, highestAt, "Got empty highest block")
assert.Equal(t, true, proto.Equal(block3.Proto(), highestAt.Proto()), "Wanted: %v, received: %v", block3, highestAt)
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])
})
}
}
@@ -549,15 +549,15 @@ func TestStore_GenesisBlock_CanGetHighestAt(t *testing.T) {
require.NoError(t, err)
require.NoError(t, db.SaveBlock(ctx, block1))
highestAt, err := db.HighestBlockBelowSlot(ctx, 2)
highestAt, err := db.HighestSlotBlocksBelow(ctx, 2)
require.NoError(t, err)
assert.Equal(t, true, proto.Equal(block1.Proto(), highestAt.Proto()), "Wanted: %v, received: %v", block1, highestAt)
highestAt, err = db.HighestBlockBelowSlot(ctx, 1)
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.NoError(t, err)
assert.Equal(t, true, proto.Equal(genesisBlock.Proto(), highestAt.Proto()), "Wanted: %v, received: %v", genesisBlock, highestAt)
highestAt, err = db.HighestBlockBelowSlot(ctx, 0)
assert.Equal(t, true, proto.Equal(genesisBlock.Proto(), highestAt[0].Proto()), "Wanted: %v, received: %v", genesisBlock, highestAt[0])
highestAt, err = db.HighestSlotBlocksBelow(ctx, 0)
require.NoError(t, err)
assert.Equal(t, true, proto.Equal(genesisBlock.Proto(), highestAt.Proto()), "Wanted: %v, received: %v", genesisBlock, highestAt)
assert.Equal(t, true, proto.Equal(genesisBlock.Proto(), highestAt[0].Proto()), "Wanted: %v, received: %v", genesisBlock, highestAt[0])
})
}
}

View File

@@ -55,11 +55,14 @@ func (c *CanonicalHistory) BlockForSlot(ctx context.Context, target types.Slot)
if ctx.Err() != nil {
return [32]byte{}, nil, errors.Wrap(ctx.Err(), "context canceled during canonicalBlockForSlot")
}
b, err := c.h.HighestBlockBelowSlot(ctx, target+1)
hbs, err := c.h.HighestSlotBlocksBelow(ctx, target+1)
if err != nil {
return [32]byte{}, nil, errors.Wrap(err, fmt.Sprintf("error finding highest block w/ slot <= %d", target))
}
r, err := c.canonicalRoot(ctx, b)
if len(hbs) == 0 {
return [32]byte{}, nil, errors.Wrap(ErrNoBlocksBelowSlot, fmt.Sprintf("slot=%d", target))
}
r, b, err := c.bestForSlot(ctx, hbs)
if err == nil {
// we found a valid, canonical block!
return r, b, nil
@@ -68,10 +71,10 @@ func (c *CanonicalHistory) BlockForSlot(ctx context.Context, target types.Slot)
// 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 b.Block().Slot() == params.BeaconConfig().GenesisSlot {
if hbs[0].Block().Slot() == params.BeaconConfig().GenesisSlot {
break
}
target = b.Block().Slot() - 1
target = hbs[0].Block().Slot() - 1
continue
}
return [32]byte{}, nil, err
@@ -80,34 +83,36 @@ func (c *CanonicalHistory) BlockForSlot(ctx context.Context, target types.Slot)
if err != nil {
return [32]byte{}, nil, errors.Wrap(err, "db error while retrieving genesis block")
}
root, err := c.canonicalRoot(ctx, b)
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
}
// canonicalRoot encapsulates several messy realities of the underlying db code,
// performing nil/validity checks, and using CanonicalChecker to ensure the block is canonical.
func (c *CanonicalHistory) canonicalRoot(ctx context.Context, b interfaces.SignedBeaconBlock) ([32]byte, error) {
if wrapper.BeaconBlockIsNil(b) != nil {
return [32]byte{}, errors.Wrap(ErrNoCanonicalBlockForSlot, "nil block")
// 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)
}
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")
}
if canon {
return root, b, nil
}
}
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{}, errors.Wrap(wrapped, msg)
}
canon, err := c.cc.IsCanonical(ctx, root)
if err != nil {
return [32]byte{}, errors.Wrap(err, "replayer could not check if block is canonical")
}
if canon {
return root, nil
}
return [32]byte{}, errors.Wrap(ErrNoCanonicalBlockForSlot, "no good block for slot")
return [32]byte{}, nil, errors.Wrap(ErrNoCanonicalBlockForSlot, "no good block for slot")
}
// ChainForSlot creates a value that satisfies the Replayer interface via db queries

View File

@@ -33,7 +33,7 @@ func TestChainForSlotFuture(t *testing.T) {
require.ErrorIs(t, err, ErrFutureSlotRequested)
}
func TestCanonicalRoot(t *testing.T) {
func TestBestForSlot(t *testing.T) {
nilBlock, err := wrapper.WrappedSignedBeaconBlock(&ethpb.SignedBeaconBlock{})
require.NoError(t, err)
nilBody, err := wrapper.WrappedSignedBeaconBlock(&ethpb.SignedBeaconBlock{Block: &ethpb.BeaconBlock{}})
@@ -48,61 +48,70 @@ func TestCanonicalRoot(t *testing.T) {
better := &mock.SignedBeaconBlock{BeaconBlock: &mock.BeaconBlock{BeaconBlockBody: &mock.BeaconBlockBody{}, Htr: betterHTR}}
cases := []struct {
name string
err error
block interfaces.SignedBeaconBlock
root [32]byte
cc CanonicalChecker
name string
err error
blocks []interfaces.SignedBeaconBlock
best interfaces.SignedBeaconBlock
root [32]byte
cc CanonicalChecker
}{
{
name: "empty SignedBeaconBlock",
err: ErrNoCanonicalBlockForSlot,
block: nil,
name: "empty list",
err: ErrNoCanonicalBlockForSlot,
blocks: []interfaces.SignedBeaconBlock{},
},
{
name: "empty BeaconBlock",
err: ErrNoCanonicalBlockForSlot,
block: nilBlock,
name: "empty SignedBeaconBlock",
err: ErrNoCanonicalBlockForSlot,
blocks: []interfaces.SignedBeaconBlock{nil},
},
{
name: "empty BeaconBlockBody",
err: ErrNoCanonicalBlockForSlot,
block: nilBody,
name: "empty BeaconBlock",
err: ErrNoCanonicalBlockForSlot,
blocks: []interfaces.SignedBeaconBlock{nilBlock},
},
{
name: "bad HTR",
err: ErrInvalidDBBlock,
block: badHTR,
name: "empty BeaconBlockBody",
err: ErrNoCanonicalBlockForSlot,
blocks: []interfaces.SignedBeaconBlock{nilBody},
},
{
name: "IsCanonical fail",
block: good,
cc: &mockCanonicalChecker{is: true, err: derp},
err: derp,
name: "bad HTR",
err: ErrInvalidDBBlock,
blocks: []interfaces.SignedBeaconBlock{badHTR},
},
{
name: "all non-canonical",
err: ErrNoCanonicalBlockForSlot,
block: good,
cc: &mockCanonicalChecker{is: false},
name: "IsCanonical fail",
blocks: []interfaces.SignedBeaconBlock{good, better},
cc: &mockCanonicalChecker{is: true, err: derp},
err: derp,
},
{
name: "one canonical",
block: good,
cc: &mockCanonicalChecker{is: true},
root: goodHTR,
name: "all non-canonical",
err: ErrNoCanonicalBlockForSlot,
blocks: []interfaces.SignedBeaconBlock{good, better},
cc: &mockCanonicalChecker{is: false},
},
{
name: "all canonical",
block: better,
cc: &mockCanonicalChecker{is: true},
root: betterHTR,
name: "one canonical",
blocks: []interfaces.SignedBeaconBlock{good},
cc: &mockCanonicalChecker{is: true},
root: goodHTR,
best: good,
},
{
name: "first wins",
block: good,
cc: &mockCanonicalChecker{is: true},
root: goodHTR,
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,
},
}
for _, c := range cases {
@@ -112,9 +121,10 @@ func TestCanonicalRoot(t *testing.T) {
chk = c.cc
}
ch := &CanonicalHistory{cc: chk}
r, err := ch.canonicalRoot(context.Background(), c.block)
r, b, err := ch.bestForSlot(context.Background(), c.blocks)
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)
@@ -154,9 +164,10 @@ func TestCanonicalBlockForSlotHappy(t *testing.T) {
}
for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
b, err := hist.HighestBlockBelowSlot(ctx, c.slot+1)
bs, err := hist.HighestSlotBlocksBelow(ctx, c.slot+1)
require.NoError(t, err)
r, err := b.Block().HashTreeRoot()
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)
@@ -177,35 +188,43 @@ func TestCanonicalBlockForSlotNonHappy(t *testing.T) {
hist := newMockHistory(t, specs, end+1)
slotOrderObserved := make([]types.Slot, 0)
derp := errors.New("HighestBlockBelowSlot don't work")
derp := errors.New("HighestSlotBlocksBelow 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) ([]interfaces.SignedBeaconBlock, error)
slotOrderExpected []types.Slot
err error
root [32]byte
}{
{
name: "HighestBlockBelowSlot not called for genesis",
overrideHighest: func(_ context.Context, _ types.Slot) (interfaces.SignedBeaconBlock, error) {
name: "HighestSlotBlocksBelow not called for genesis",
overrideHighest: func(_ context.Context, _ types.Slot) ([]interfaces.SignedBeaconBlock, error) {
return nil, derp
},
root: hist.slotMap[0],
},
{
name: "wrapped error from HighestBlockBelowSlot returned",
name: "wrapped error from HighestSlotBlocksBelow returned",
err: derp,
overrideHighest: func(_ context.Context, _ types.Slot) (interfaces.SignedBeaconBlock, error) {
overrideHighest: func(_ context.Context, _ types.Slot) ([]interfaces.SignedBeaconBlock, error) {
return nil, derp
},
slot: end,
},
{
name: "HighestBlockBelowSlot no canonical",
name: "HighestSlotBlocksBelow empty list",
err: ErrNoBlocksBelowSlot,
overrideHighest: func(_ context.Context, _ types.Slot) ([]interfaces.SignedBeaconBlock, error) {
return []interfaces.SignedBeaconBlock{}, nil
},
slot: end,
},
{
name: "HighestSlotBlocksBelow no canonical",
err: ErrNoCanonicalBlockForSlot,
canon: &mockCanonicalChecker{is: false},
slot: end,
@@ -218,9 +237,9 @@ 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) ([]interfaces.SignedBeaconBlock, error) {
slotOrderObserved = append(slotOrderObserved, s)
// this allows the mock HighestBlockBelowSlot to continue to execute now that we've recorded
// this allows the mock HighestSlotBlocksBelow to continue to execute now that we've recorded
// the slot in our channel
return nil, errFallThroughOverride
},
@@ -236,9 +255,9 @@ 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) ([]interfaces.SignedBeaconBlock, error) {
slotOrderObserved = append(slotOrderObserved, s)
// this allows the mock HighestBlockBelowSlot to continue to execute now that we've recorded
// this allows the mock HighestSlotBlocksBelow to continue to execute now that we've recorded
// the slot in our channel
return nil, errFallThroughOverride
},
@@ -254,7 +273,7 @@ func TestCanonicalBlockForSlotNonHappy(t *testing.T) {
canon = c.canon
}
ch := &CanonicalHistory{h: hist, cc: canon, cs: hist}
hist.overrideHighestBlockBelowSlot = c.overrideHighest
hist.overrideHighestSlotBlocksBelow = c.overrideHighest
r, _, err := ch.BlockForSlot(ctx, c.slot)
if c.err == nil {
require.NoError(t, err)
@@ -262,7 +281,7 @@ func TestCanonicalBlockForSlotNonHappy(t *testing.T) {
require.ErrorIs(t, err, c.err)
}
if len(c.slotOrderExpected) > 0 {
require.Equal(t, len(c.slotOrderExpected), len(slotOrderObserved), "HighestBlockBelowSlot not called the expected number of times")
require.Equal(t, len(c.slotOrderExpected), len(slotOrderObserved), "HighestSlotBlocksBelow not called the expected number of times")
for i := range c.slotOrderExpected {
require.Equal(t, c.slotOrderExpected[i], slotOrderObserved[i])
}

View File

@@ -55,11 +55,16 @@ func (s *State) MigrateToCold(ctx context.Context, fRoot [32]byte) error {
aRoot = cached.root
aState = cached.state
} else {
blk, err := s.beaconDB.HighestBlockBelowSlot(ctx, slot)
blks, err := s.beaconDB.HighestSlotBlocksBelow(ctx, slot)
if err != nil {
return err
}
missingRoot, err := blk.Block().HashTreeRoot()
// 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 {
return errUnknownBlock
}
missingRoot, err := blks[0].Block().HashTreeRoot()
if err != nil {
return err
}

View File

@@ -74,14 +74,14 @@ type mockHistorySpec struct {
}
type mockHistory struct {
blocks map[[32]byte]interfaces.SignedBeaconBlock
slotMap map[types.Slot][32]byte
slotIndex slotList
canonical map[[32]byte]bool
states map[[32]byte]state.BeaconState
hiddenStates map[[32]byte]state.BeaconState
current types.Slot
overrideHighestBlockBelowSlot func(context.Context, types.Slot) (interfaces.SignedBeaconBlock, error)
blocks map[[32]byte]interfaces.SignedBeaconBlock
slotMap map[types.Slot][32]byte
slotIndex slotList
canonical map[[32]byte]bool
states map[[32]byte]state.BeaconState
hiddenStates map[[32]byte]state.BeaconState
current types.Slot
overrideHighestSlotBlocksBelow func(context.Context, types.Slot) ([]interfaces.SignedBeaconBlock, error)
}
type slotList []types.Slot
@@ -98,11 +98,11 @@ 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 HighestBlockBelowSlot")
var errFallThroughOverride = errors.New("override yielding control back to real HighestSlotBlocksBelow")
func (m *mockHistory) HighestBlockBelowSlot(_ context.Context, slot types.Slot) (interfaces.SignedBeaconBlock, error) {
if m.overrideHighestBlockBelowSlot != nil {
s, err := m.overrideHighestBlockBelowSlot(context.Background(), slot)
func (m *mockHistory) HighestSlotBlocksBelow(_ context.Context, slot types.Slot) ([]interfaces.SignedBeaconBlock, error) {
if m.overrideHighestSlotBlocksBelow != nil {
s, err := m.overrideHighestSlotBlocksBelow(context.Background(), slot)
if !errors.Is(err, errFallThroughOverride) {
return s, err
}
@@ -115,10 +115,10 @@ func (m *mockHistory) HighestBlockBelowSlot(_ context.Context, slot types.Slot)
}
for _, s := range m.slotIndex {
if s < slot {
return m.blocks[m.slotMap[s]], nil
return []interfaces.SignedBeaconBlock{m.blocks[m.slotMap[s]]}, nil
}
}
return nil, nil
return []interfaces.SignedBeaconBlock{}, nil
}
var errGenesisBlockNotFound = errors.New("canonical genesis block not found in db")

View File

@@ -27,7 +27,7 @@ const (
// HistoryAccessor describes the minimum set of database methods needed to support the ReplayerBuilder.
type HistoryAccessor interface {
HighestBlockBelowSlot(ctx context.Context, slot types.Slot) (interfaces.SignedBeaconBlock, error)
HighestSlotBlocksBelow(ctx context.Context, slot types.Slot) ([]interfaces.SignedBeaconBlock, error)
GenesisBlock(ctx context.Context) (interfaces.SignedBeaconBlock, error)
Block(ctx context.Context, blockRoot [32]byte) (interfaces.SignedBeaconBlock, error)
StateOrError(ctx context.Context, blockRoot [32]byte) (state.BeaconState, error)