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 all commits
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
90 changes: 43 additions & 47 deletions vms/proposervm/block.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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 {
Expand Down Expand Up @@ -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()
Expand All @@ -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()
Expand All @@ -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) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

flipped the logic to align with verifyPre/PostDurangoBlockDelay. Signed blocks will be the new normal so I chose to signal "should build signed block/should have a proposer" with true

parentHeight := p.innerBlk.Height()
currentSlot := proposer.TimeToSlot(parentTimestamp, newTimestamp)
expectedProposerID, err := p.vm.Windower.ExpectedProposer(
Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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()
Expand All @@ -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
Expand Down
14 changes: 12 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,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,
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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++ {
Expand Down Expand Up @@ -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
}
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
15 changes: 9 additions & 6 deletions vms/proposervm/vm.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Loading