fix collect tipsets error, add test case (#3250)

Fixes #3027

chain/get_ancestors.go

@@ -51,38 +51,26 @@ func GetRecentAncestors(ctx context.Context, base types.TipSet, provider TipSetP
 		earliestAncestorHeight = types.NewBlockHeight(uint64(0))
 	}
 
-	// Step 1 -- gather all tipsets with a height greater than the earliest
-	// possible proving period start still in scope for the given head.
 	iterator := IterAncestors(ctx, provider, base)
-	provingPeriodAncestors, err := CollectTipSetsOfHeightAtLeast(ctx, iterator, earliestAncestorHeight)
-	if err != nil {
-		return nil, err
-	}
-	firstExtraRandomnessAncestorsCids, err := provingPeriodAncestors[len(provingPeriodAncestors)-1].Parents()
+	// Step 1 -- gather all tipsets up to the first tipset with a height less than
+	// or equal to the earliest possible proving period start
+	provingPeriodAncestors, err := CollectTipSetsPastHeight(ctx, iterator, earliestAncestorHeight)
+
 	if err != nil {
 		return nil, err
 	}
-	// no parents means hit genesis so return the whole chain
-	if firstExtraRandomnessAncestorsCids.Len() == 0 {
-		return provingPeriodAncestors, nil
-	}
 
 	// Step 2 -- gather the lookback tipsets directly preceding provingPeriodAncestors.
-	lookBackTS, err := provider.GetTipSet(firstExtraRandomnessAncestorsCids)
-	if err != nil {
-		return nil, err
-	}
-	iterator = IterAncestors(ctx, provider, lookBackTS)
 	extraRandomnessAncestors, err := CollectAtMostNTipSets(ctx, iterator, lookback)
 	if err != nil {
 		return nil, err
 	}
 	return append(provingPeriodAncestors, extraRandomnessAncestors...), nil
 }
 
-// CollectTipSetsOfHeightAtLeast collects all tipsets with a height greater
-// than or equal to minHeight from the input tipset.
-func CollectTipSetsOfHeightAtLeast(ctx context.Context, iterator *TipsetIterator, minHeight *types.BlockHeight) ([]types.TipSet, error) {
+// CollectTipSetsPastHeight collects all tipsets up to the first tipset with a height less than
+// or equal to the earliest possible proving period start
+func CollectTipSetsPastHeight(ctx context.Context, iterator *TipsetIterator, minHeight *types.BlockHeight) ([]types.TipSet, error) {
 	var ret []types.TipSet
 	var err error
 	var h uint64
@@ -94,11 +82,16 @@ func CollectTipSetsOfHeightAtLeast(ctx context.Context, iterator *TipsetIterator
 		if err != nil {
 			return nil, err
 		}
-		if types.NewBlockHeight(h).LessThan(minHeight) {
-			return ret, nil
-		}
 		ret = append(ret, iterator.Value())
+		if types.NewBlockHeight(h).LessEqual(minHeight) {
+			err = iterator.Next()
+			if err != nil {
+				return nil, err
+			}
+			break
+		}
 	}
+
 	return ret, nil
 }
 
@@ -116,6 +109,28 @@ func CollectAtMostNTipSets(ctx context.Context, iterator *TipsetIterator, n uint
 	return ret, nil
 }
 
+// CollectTipSetsOfHeightAtLeast collects all tipsets with a height greater
+// than or equal to minHeight from the input tipset.
+func CollectTipSetsOfHeightAtLeast(ctx context.Context, iterator *TipsetIterator, minHeight *types.BlockHeight) ([]types.TipSet, error) {
+	var ret []types.TipSet
+	var err error
+	var h uint64
+	for ; !iterator.Complete(); err = iterator.Next() {
+		if err != nil {
+			return nil, err
+		}
+		h, err = iterator.Value().Height()
+		if err != nil {
+			return nil, err
+		}
+		if types.NewBlockHeight(h).LessThan(minHeight) {
+			return ret, nil
+		}
+		ret = append(ret, iterator.Value())
+	}
+	return ret, nil
+}
+
 // FindCommonAncestor returns the common ancestor of the two tipsets pointed to
 // by the input iterators.  If they share no common ancestor ErrNoCommonAncestor
 // will be returned.

chain/get_ancestors_test.go

@@ -88,6 +88,97 @@ func TestCollectTipSetsOfHeightAtLeastStartingEpochIsNull(t *testing.T) {
 	assert.Equal(t, 20, len(tipsets))
 }
 
+// Happy path
+func TestCollectTipSetsPastHeight(t *testing.T) {
+	tf.UnitTest(t)
+	ctx := context.Background()
+	builder := chain.NewBuilder(t, address.Undef)
+
+	chainLen := 15
+	head := builder.AppendManyOn(chainLen, types.UndefTipSet)
+
+	stopHeight := types.NewBlockHeight(uint64(4))
+	iterator := chain.IterAncestors(ctx, builder, head)
+	tipsets, err := chain.CollectTipSetsPastHeight(ctx, iterator, stopHeight)
+	assert.NoError(t, err)
+	latestHeight, err := tipsets[0].Height()
+	require.NoError(t, err)
+	assert.Equal(t, uint64(14), latestHeight)
+	earliestHeight, err := tipsets[len(tipsets)-1].Height()
+	require.NoError(t, err)
+	assert.Equal(t, uint64(4), earliestHeight)
+	assert.Equal(t, 11, len(tipsets))
+}
+
+// Height at least 0.
+func TestCollectTipSetsPastHeightZero(t *testing.T) {
+	tf.UnitTest(t)
+	ctx := context.Background()
+	builder := chain.NewBuilder(t, address.Undef)
+
+	chainLen := 25
+	head := builder.AppendManyOn(chainLen, types.UndefTipSet)
+
+	stopHeight := types.NewBlockHeight(uint64(0))
+	iterator := chain.IterAncestors(ctx, builder, head)
+	tipsets, err := chain.CollectTipSetsPastHeight(ctx, iterator, stopHeight)
+	assert.NoError(t, err)
+	latestHeight, err := tipsets[0].Height()
+	require.NoError(t, err)
+	assert.Equal(t, uint64(24), latestHeight)
+	earliestHeight, err := tipsets[len(tipsets)-1].Height()
+	require.NoError(t, err)
+	assert.Equal(t, uint64(0), earliestHeight)
+	assert.Equal(t, chainLen, len(tipsets))
+}
+
+// tipsets at least 1.
+func TestCollectTipSetsPastHeightTipsetOne(t *testing.T) {
+	tf.UnitTest(t)
+	ctx := context.Background()
+	builder := chain.NewBuilder(t, address.Undef)
+
+	chainLen := 25
+	head := builder.AppendManyOn(chainLen, types.UndefTipSet)
+
+	stopHeight := types.NewBlockHeight(uint64(25))
+	iterator := chain.IterAncestors(ctx, builder, head)
+	tipsets, err := chain.CollectTipSetsPastHeight(ctx, iterator, stopHeight)
+	assert.NoError(t, err)
+	assert.Equal(t, 1, len(tipsets))
+}
+
+// The starting epoch is a null block.
+func TestCollectTipSetsPastHeightStartingEpochIsNull(t *testing.T) {
+	tf.UnitTest(t)
+	ctx := context.Background()
+	builder := chain.NewBuilder(t, address.Undef)
+	head := builder.NewGenesis()
+
+	// Add 30 tipsets to the head of the chainStore.
+	head = builder.AppendManyOn(30, head)
+
+	// Now add 10 null blocks and 1 tipset.
+	head = builder.BuildOneOn(head, func(b *chain.BlockBuilder) {
+		b.IncHeight(10)
+	})
+
+	// Now add 19 more tipsets.
+	head = builder.AppendManyOn(19, head)
+
+	stopHeight := types.NewBlockHeight(uint64(35))
+	iterator := chain.IterAncestors(ctx, builder, head)
+	tipsets, err := chain.CollectTipSetsPastHeight(ctx, iterator, stopHeight)
+	assert.NoError(t, err)
+	latestHeight, err := tipsets[0].Height()
+	require.NoError(t, err)
+	assert.Equal(t, uint64(60), latestHeight)
+	earliestHeight, err := tipsets[len(tipsets)-1].Height()
+	require.NoError(t, err)
+	assert.Equal(t, uint64(30), earliestHeight)
+	assert.Equal(t, 21, len(tipsets))
+}
+
 func TestCollectAtMostNTipSets(t *testing.T) {
 	tf.UnitTest(t)
 	ctx := context.Background()
@@ -110,6 +201,13 @@ func TestCollectAtMostNTipSets(t *testing.T) {
 		assert.NoError(t, err)
 		assert.Equal(t, 25, len(tipsets))
 	})
+	t.Run("hit zero", func(t *testing.T) {
+		number := uint(0)
+		iterator := chain.IterAncestors(ctx, builder, head)
+		tipsets, err := chain.CollectAtMostNTipSets(ctx, iterator, number)
+		assert.NoError(t, err)
+		assert.Equal(t, 0, len(tipsets))
+	})
 }
 
 // Test the happy path.
@@ -191,11 +289,11 @@ func TestGetRecentAncestorsStartingEpochIsNull(t *testing.T) {
 	ancestors, err := chain.GetRecentAncestors(ctx, head, builder, types.NewBlockHeight(h+uint64(1)), types.NewBlockHeight(epochs), uint(lookback))
 	require.NoError(t, err)
 
-	// We expect to see 20 blocks in the first 28 epochs and an additional 6 for the lookback parameter
-	assert.Equal(t, len2+lookback+1, len(ancestors))
+	// We expect to see 20+1 blocks include less than earliest block in the first 28 epochs and an additional 6 for the lookback parameter
+	assert.Equal(t, len2+lookback+2, len(ancestors))
 	lastBlockHeight, err := ancestors[len(ancestors)-1].Height()
 	require.NoError(t, err)
-	assert.Equal(t, uint64(25), lastBlockHeight)
+	assert.Equal(t, uint64(24), lastBlockHeight)
 }
 
 func TestFindCommonAncestorSameChain(t *testing.T) {