Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix windowing when no validator is available #2529

Merged
merged 6 commits into from
Dec 22, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 12 additions & 8 deletions vms/proposervm/block.go
Original file line number Diff line number Diff line change
Expand Up @@ -380,19 +380,21 @@ func (p *postForkCommonComponents) verifyPostDurangoBlockDelay(
parentPChainHeight,
proposer.TimeToSlot(parentTimestamp, blkTimestamp),
)
if err != nil {
switch {
case errors.Is(err, proposer.ErrAnyoneCanPropose):
return nil
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 {
case expectedProposerID == proposerID:
return nil
default:
return errUnexpectedProposer
}

return nil
}

func (p *postForkCommonComponents) shouldBuildBlockPostDurango(
Expand All @@ -410,15 +412,17 @@ func (p *postForkCommonComponents) shouldBuildBlockPostDurango(
parentPChainHeight,
currentSlot,
)
if err != nil {
switch {
case errors.Is(err, proposer.ErrAnyoneCanPropose):
return nil
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 {
case expectedProposerID == p.vm.ctx.NodeID:
return nil
}

Expand Down
11 changes: 9 additions & 2 deletions vms/proposervm/proposer/windower.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ const (
var (
_ Windower = (*windower)(nil)

ErrNoProposersAvailable = errors.New("no proposers available")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this becomes an internal error that does not require a special error. Instead we should singla the case where no validators are available to be able to handle it appropriately

ErrAnyoneCanPropose = errors.New("anyone can propose")
)

type Windower interface {
Expand Down Expand Up @@ -68,6 +68,7 @@ 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,
Expand Down Expand Up @@ -171,6 +172,9 @@ func (w *windower) ExpectedProposer(
if err != nil {
return ids.EmptyNodeID, err
}
if len(validators) == 0 {
return ids.EmptyNodeID, ErrAnyoneCanPropose
}

return w.expectedProposer(
validators,
Expand All @@ -193,6 +197,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++ {
Expand Down Expand Up @@ -263,7 +270,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
}
Expand Down
6 changes: 5 additions & 1 deletion vms/proposervm/proposer/windower_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
24 changes: 14 additions & 10 deletions vms/proposervm/vm.go
Original file line number Diff line number Diff line change
Expand Up @@ -411,18 +411,22 @@ func (vm *VM) getPostDurangoSlotTime(
vm.ctx.NodeID,
slot,
)
if err != nil {
switch {
case err == nil:
// 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
case errors.Is(err, proposer.ErrAnyoneCanPropose):
delay = math.Max(time.Duration(0), vm.MinBlkDelay)
return parentTimestamp.Add(delay), err
default:
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
}

func (vm *VM) LastAccepted(ctx context.Context) (ids.ID, error) {
Expand Down
Loading