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.

Sign up for GitHub

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

Fix windowing when no validator is available #2529

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
312e6c1
fixed windowing when no validator is available
abi87 Dec 21, 2023
bc3206f
reintroduced unsigned blocks when possible
abi87 Dec 21, 2023
6280084
Merge branch 'dev' into fix_windowing_for_no_validators_subnets
abi87 Dec 21, 2023
ae38a5f
Merge branch 'dev' into fix_windowing_for_no_validators_subnets
StephenButtolph Dec 22, 2023
473b467
nit
StephenButtolph Dec 22, 2023
7b03343
nit
StephenButtolph Dec 22, 2023
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
84 changes: 44 additions & 40 deletions vms/proposervm/block.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,21 +142,17 @@ 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
}

var delay time.Duration
delay, err = p.verifyPreDurangoBlockDelay(ctx, parentTimestamp, parentPChainHeight, child)
shouldHaveProposer = delay < proposer.MaxVerifyDelay
}
if err != nil {
return err
}

// Verify the signature of the node
if err := child.SignedBlock.Verify(shouldHaveProposer, p.vm.ctx.ChainID); err != nil {
Expand Down Expand Up @@ -205,17 +201,17 @@ func (p *postForkCommonComponents) buildChild(
}

// After Durango, we never allow unsigned blocks.
shouldBuildUnsignedBlock := false
shouldBuildSignedBlock := false
if p.vm.IsDurangoActivated(parentTimestamp) {
err = p.shouldBuildBlockPostDurango(
shouldBuildSignedBlock, err = p.shouldBuildBlockPostDurango(
ctx,
parentID,
parentTimestamp,
parentPChainHeight,
newTimestamp,
)
} else {
shouldBuildUnsignedBlock, err = p.shouldBuildUnsignedBlockPreDurango(
shouldBuildSignedBlock, err = p.shouldBuildUnsignedBlockPreDurango(
ctx,
parentID,
parentTimestamp,
Expand All @@ -241,22 +237,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 @@ -367,7 +363,7 @@ func (p *postForkCommonComponents) verifyPostDurangoBlockDelay(
parentTimestamp time.Time,
parentPChainHeight uint64,
blk *postForkBlock,
) error {
) (bool, error) {
var (
blkTimestamp = blk.Timestamp()
blkHeight = blk.Height()
Expand All @@ -380,19 +376,23 @@ func (p *postForkCommonComponents) verifyPostDurangoBlockDelay(
parentPChainHeight,
proposer.TimeToSlot(parentTimestamp, blkTimestamp),
)
if err != nil {
switch {
case errors.Is(err, proposer.ErrAnyoneCanPropose):
// block should be unsigned
return false, 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 {
return errUnexpectedProposer
return true, err
case expectedProposerID == proposerID:
// block should be signed
return true, nil
default:
return true, errUnexpectedProposer
}

return nil
}

func (p *postForkCommonComponents) shouldBuildBlockPostDurango(
Expand All @@ -401,7 +401,7 @@ func (p *postForkCommonComponents) shouldBuildBlockPostDurango(
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 +410,20 @@ func (p *postForkCommonComponents) shouldBuildBlockPostDurango(
parentPChainHeight,
currentSlot,
)
if err != nil {
switch {
case errors.Is(err, proposer.ErrAnyoneCanPropose):
// build an unsigned block
return false, 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 {
return nil
return true, err
case expectedProposerID == p.vm.ctx.NodeID:
// build an signed block
return true, nil
}

// It's not our turn to propose a block yet. This is likely caused by having
Expand Down Expand Up @@ -450,14 +454,14 @@ func (p *postForkCommonComponents) shouldBuildBlockPostDurango(
zap.Stringer("parentID", parentID),
zap.Error(err),
)
return err
return true, 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 true, errProposerWindowNotStarted
}

func (p *postForkCommonComponents) shouldBuildUnsignedBlockPreDurango(
Expand All @@ -470,7 +474,7 @@ func (p *postForkCommonComponents) shouldBuildUnsignedBlockPreDurango(
delay := newTimestamp.Sub(parentTimestamp)
if delay >= proposer.MaxBuildDelay {
// time for any node to build an unsigned block
return true, nil
return false, nil
}

parentHeight := p.innerBlk.Height()
Expand All @@ -482,13 +486,13 @@ func (p *postForkCommonComponents) shouldBuildUnsignedBlockPreDurango(
zap.Stringer("parentID", parentID),
zap.Error(err),
)
return false, err
return true, err
}

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 All @@ -503,5 +507,5 @@ func (p *postForkCommonComponents) shouldBuildUnsignedBlockPreDurango(
// 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 false, errProposerWindowNotStarted
return true, errProposerWindowNotStarted
}
11 changes: 9 additions & 2 deletions vms/proposervm/proposer/windower.go
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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