Fix windowing when no validator is available (#2529)

Co-authored-by: Stephen Buttolph <[email protected]>

vms/proposervm/block.go

@@ -142,20 +142,14 @@ func (p *postForkCommonComponents) Verify(
 			)
 		}
 
-		// After Durango, we never allow unsigned blocks.
-		shouldHaveProposer := true
+		var shouldHaveProposer bool
 		if p.vm.IsDurangoActivated(parentTimestamp) {
-			err := p.verifyPostDurangoBlockDelay(ctx, parentTimestamp, parentPChainHeight, child)
-			if err != nil {
-				return err
-			}
+			shouldHaveProposer, err = p.verifyPostDurangoBlockDelay(ctx, parentTimestamp, parentPChainHeight, child)
 		} else {
-			delay, err := p.verifyPreDurangoBlockDelay(ctx, parentTimestamp, parentPChainHeight, child)
-			if err != nil {
-				return err
-			}
-
-			shouldHaveProposer = delay < proposer.MaxVerifyDelay
+			shouldHaveProposer, err = p.verifyPreDurangoBlockDelay(ctx, parentTimestamp, parentPChainHeight, child)
+		}
+		if err != nil {
+			return err
 		}
 
 		// Verify the signature of the node
@@ -204,18 +198,17 @@ func (p *postForkCommonComponents) buildChild(
 		return nil, err
 	}
 
-	// After Durango, we never allow unsigned blocks.
-	shouldBuildUnsignedBlock := false
+	var shouldBuildSignedBlock bool
 	if p.vm.IsDurangoActivated(parentTimestamp) {
-		err = p.shouldBuildBlockPostDurango(
+		shouldBuildSignedBlock, err = p.shouldBuildSignedBlockPostDurango(
 			ctx,
 			parentID,
 			parentTimestamp,
 			parentPChainHeight,
 			newTimestamp,
 		)
 	} else {
-		shouldBuildUnsignedBlock, err = p.shouldBuildUnsignedBlockPreDurango(
+		shouldBuildSignedBlock, err = p.shouldBuildSignedBlockPreDurango(
 			ctx,
 			parentID,
 			parentTimestamp,
@@ -241,22 +234,22 @@ func (p *postForkCommonComponents) buildChild(
 
 	// Build the child
 	var statelessChild block.SignedBlock
-	if shouldBuildUnsignedBlock {
-		statelessChild, err = block.BuildUnsigned(
+	if shouldBuildSignedBlock {
+		statelessChild, err = block.Build(
 			parentID,
 			newTimestamp,
 			pChainHeight,
+			p.vm.StakingCertLeaf,
 			innerBlock.Bytes(),
+			p.vm.ctx.ChainID,
+			p.vm.StakingLeafSigner,
 		)
 	} else {
-		statelessChild, err = block.Build(
+		statelessChild, err = block.BuildUnsigned(
 			parentID,
 			newTimestamp,
 			pChainHeight,
-			p.vm.StakingCertLeaf,
 			innerBlock.Bytes(),
-			p.vm.ctx.ChainID,
-			p.vm.StakingLeafSigner,
 		)
 	}
 	if err != nil {
@@ -332,7 +325,7 @@ func (p *postForkCommonComponents) verifyPreDurangoBlockDelay(
 	parentTimestamp time.Time,
 	parentPChainHeight uint64,
 	blk *postForkBlock,
-) (time.Duration, error) {
+) (bool, error) {
 	var (
 		blkTimestamp = blk.Timestamp()
 		childHeight  = blk.Height()
@@ -351,23 +344,23 @@ func (p *postForkCommonComponents) verifyPreDurangoBlockDelay(
 			zap.Stringer("blkID", blk.ID()),
 			zap.Error(err),
 		)
-		return 0, err
+		return false, err
 	}
 
 	delay := blkTimestamp.Sub(parentTimestamp)
 	if delay < minDelay {
-		return 0, errProposerWindowNotStarted
+		return false, errProposerWindowNotStarted
 	}
 
-	return delay, nil
+	return delay < proposer.MaxVerifyDelay, nil
 }
 
 func (p *postForkCommonComponents) verifyPostDurangoBlockDelay(
 	ctx context.Context,
 	parentTimestamp time.Time,
 	parentPChainHeight uint64,
 	blk *postForkBlock,
-) error {
+) (bool, error) {
 	var (
 		blkTimestamp = blk.Timestamp()
 		blkHeight    = blk.Height()
@@ -380,28 +373,30 @@ func (p *postForkCommonComponents) verifyPostDurangoBlockDelay(
 		parentPChainHeight,
 		proposer.TimeToSlot(parentTimestamp, blkTimestamp),
 	)
-	if err != nil {
+	switch {
+	case errors.Is(err, proposer.ErrAnyoneCanPropose):
+		return false, nil // block should be unsigned
+	case err != nil:
 		p.vm.ctx.Log.Error("unexpected block verification failure",
 			zap.String("reason", "failed to calculate expected proposer"),
 			zap.Stringer("blkID", blk.ID()),
 			zap.Error(err),
 		)
-		return err
-	}
-	if expectedProposerID != proposerID {
-		return errUnexpectedProposer
+		return false, err
+	case expectedProposerID == proposerID:
+		return true, nil // block should be signed
+	default:
+		return false, errUnexpectedProposer
 	}
-
-	return nil
 }
 
-func (p *postForkCommonComponents) shouldBuildBlockPostDurango(
+func (p *postForkCommonComponents) shouldBuildSignedBlockPostDurango(
 	ctx context.Context,
 	parentID ids.ID,
 	parentTimestamp time.Time,
 	parentPChainHeight uint64,
 	newTimestamp time.Time,
-) error {
+) (bool, error) {
 	parentHeight := p.innerBlk.Height()
 	currentSlot := proposer.TimeToSlot(parentTimestamp, newTimestamp)
 	expectedProposerID, err := p.vm.Windower.ExpectedProposer(
@@ -410,16 +405,18 @@ func (p *postForkCommonComponents) shouldBuildBlockPostDurango(
 		parentPChainHeight,
 		currentSlot,
 	)
-	if err != nil {
+	switch {
+	case errors.Is(err, proposer.ErrAnyoneCanPropose):
+		return false, nil // build an unsigned block
+	case err != nil:
 		p.vm.ctx.Log.Error("unexpected build block failure",
 			zap.String("reason", "failed to calculate expected proposer"),
 			zap.Stringer("parentID", parentID),
 			zap.Error(err),
 		)
-		return err
-	}
-	if expectedProposerID == p.vm.ctx.NodeID {
-		return nil
+		return false, err
+	case expectedProposerID == p.vm.ctx.NodeID:
+		return true, nil // build a signed block
 	}
 
 	// It's not our turn to propose a block yet. This is likely caused by having
@@ -450,17 +447,17 @@ func (p *postForkCommonComponents) shouldBuildBlockPostDurango(
 			zap.Stringer("parentID", parentID),
 			zap.Error(err),
 		)
-		return err
+		return false, err
 	}
 	p.vm.Scheduler.SetBuildBlockTime(nextStartTime)
 
 	// In case the inner VM only issued one pendingTxs message, we should
 	// attempt to re-handle that once it is our turn to build the block.
 	p.vm.notifyInnerBlockReady()
-	return errProposerWindowNotStarted
+	return false, errProposerWindowNotStarted
 }
 
-func (p *postForkCommonComponents) shouldBuildUnsignedBlockPreDurango(
+func (p *postForkCommonComponents) shouldBuildSignedBlockPreDurango(
 	ctx context.Context,
 	parentID ids.ID,
 	parentTimestamp time.Time,
@@ -469,8 +466,7 @@ func (p *postForkCommonComponents) shouldBuildUnsignedBlockPreDurango(
 ) (bool, error) {
 	delay := newTimestamp.Sub(parentTimestamp)
 	if delay >= proposer.MaxBuildDelay {
-		// time for any node to build an unsigned block
-		return true, nil
+		return false, nil // time for any node to build an unsigned block
 	}
 
 	parentHeight := p.innerBlk.Height()
@@ -488,7 +484,7 @@ func (p *postForkCommonComponents) shouldBuildUnsignedBlockPreDurango(
 	if delay >= minDelay {
 		// it's time for this node to propose a block. It'll be signed or
 		// unsigned depending on the delay
-		return delay >= proposer.MaxVerifyDelay, nil
+		return delay < proposer.MaxVerifyDelay, nil
 	}
 
 	// It's not our turn to propose a block yet. This is likely caused by having

vms/proposervm/proposer/windower.go

@@ -37,7 +37,7 @@ const (
 var (
 	_ Windower = (*windower)(nil)
 
-	ErrNoProposersAvailable = errors.New("no proposers available")
+	ErrAnyoneCanPropose = errors.New("anyone can propose")
 )
 
 type Windower interface {
@@ -68,6 +68,8 @@ type Windower interface {
 	// able to propose from a given time on as it happens Pre-Durango).
 	// [ExpectedProposer] calculates which nodeID is scheduled to propose a
 	// block of height [blockHeight] at [slot].
+	// If no validators are currently available, [ErrAnyoneCanPropose] is
+	// returned.
 	ExpectedProposer(
 		ctx context.Context,
 		blockHeight,
@@ -82,6 +84,8 @@ type Windower interface {
 	// slot to start. Delay is specified as starting from slot zero start.
 	// (which is parent timestamp). For efficiency reasons, we cap the slot
 	// search to [MaxLookAheadSlots].
+	// If no validators are currently available, [ErrAnyoneCanPropose] is
+	// returned.
 	MinDelayForProposer(
 		ctx context.Context,
 		blockHeight,
@@ -171,6 +175,9 @@ func (w *windower) ExpectedProposer(
 	if err != nil {
 		return ids.EmptyNodeID, err
 	}
+	if len(validators) == 0 {
+		return ids.EmptyNodeID, ErrAnyoneCanPropose
+	}
 
 	return w.expectedProposer(
 		validators,
@@ -193,6 +200,9 @@ func (w *windower) MinDelayForProposer(
 	if err != nil {
 		return 0, err
 	}
+	if len(validators) == 0 {
+		return 0, ErrAnyoneCanPropose
+	}
 
 	maxSlot := startSlot + MaxLookAheadSlots
 	for slot := startSlot; slot < maxSlot; slot++ {
@@ -263,7 +273,7 @@ func (w *windower) expectedProposer(
 	source.Seed(w.chainSource ^ blockHeight ^ bits.Reverse64(slot))
 	indices, err := sampler.Sample(1)
 	if err != nil {
-		return ids.EmptyNodeID, fmt.Errorf("%w, %w", err, ErrNoProposersAvailable)
+		return ids.EmptyNodeID, fmt.Errorf("failed sampling proposers: %w", err)
 	}
 	return validators[indices[0]].id, nil
 }

vms/proposervm/proposer/windower_test.go

@@ -41,8 +41,12 @@ func TestWindowerNoValidators(t *testing.T) {
 	require.Zero(delay)
 
 	expectedProposer, err := w.ExpectedProposer(context.Background(), chainHeight, pChainHeight, slot)
-	require.ErrorIs(err, ErrNoProposersAvailable)
+	require.ErrorIs(err, ErrAnyoneCanPropose)
 	require.Equal(ids.EmptyNodeID, expectedProposer)
+
+	delay, err = w.MinDelayForProposer(context.Background(), chainHeight, pChainHeight, nodeID, slot)
+	require.ErrorIs(err, ErrAnyoneCanPropose)
+	require.Zero(delay)
 }
 
 func TestWindowerRepeatedValidator(t *testing.T) {

vms/proposervm/vm.go

@@ -411,18 +411,21 @@ func (vm *VM) getPostDurangoSlotTime(
 		vm.ctx.NodeID,
 		slot,
 	)
-	if err != nil {
-		return time.Time{}, err
-	}
-
 	// Note: The P-chain does not currently try to target any block time. It
 	// notifies the consensus engine as soon as a new block may be built. To
 	// avoid fast runs of blocks there is an additional minimum delay that
 	// validators can specify. This delay may be an issue for high performance,
 	// custom VMs. Until the P-chain is modified to target a specific block
 	// time, ProposerMinBlockDelay can be configured in the subnet config.
-	delay = math.Max(delay, vm.MinBlkDelay)
-	return parentTimestamp.Add(delay), err
+	switch {
+	case err == nil:
+		delay = math.Max(delay, vm.MinBlkDelay)
+		return parentTimestamp.Add(delay), err
+	case errors.Is(err, proposer.ErrAnyoneCanPropose):
+		return parentTimestamp.Add(vm.MinBlkDelay), err
+	default:
+		return time.Time{}, err
+	}
 }
 
 func (vm *VM) LastAccepted(ctx context.Context) (ids.ID, error) {