Allow calls to Options before Verify

snow/consensus/snowman/oracle_block.go

@@ -18,6 +18,5 @@ var ErrNotOracle = errors.New("block isn't an oracle")
 type OracleBlock interface {
 	// Options returns the possible children of this block in the order this
 	// validator prefers the blocks.
-	// Options is guaranteed to only be called on a verified block.
 	Options(context.Context) ([2]Block, error)
 }

snow/engine/snowman/transitive.go

@@ -515,22 +515,26 @@ func (t *Transitive) Start(ctx context.Context, startReqID uint32) error {
 	if oracleBlk, ok := lastAccepted.(snowman.OracleBlock); ok {
 		options, err := oracleBlk.Options(ctx)
 		switch {
-		case err == snowman.ErrNotOracle:
-			// if there aren't blocks we need to deliver on startup, we need to set
-			// the preference to the last accepted block
-			if err := t.VM.SetPreference(ctx, lastAcceptedID); err != nil {
-				return err
-			}
-		case err != nil:
-			return err
-		default:
+		case err == nil:
 			issuedMetric := t.metrics.issued.WithLabelValues(builtSource)
 			for _, blk := range options {
 				// note that deliver will set the VM's preference
 				if err := t.deliver(ctx, t.Ctx.NodeID, blk, false, issuedMetric); err != nil {
 					return err
 				}
 			}
+		case err != snowman.ErrNotOracle:
+			t.Ctx.Log.Warn("failed to set preferences for oracle block",
+				zap.Stringer("blkID", lastAcceptedID),
+				zap.Error(err),
+			)
+			fallthrough
+		default:
+			// if there aren't blocks we need to deliver on startup, we need to
+			// set the preference to the last accepted block
+			if err := t.VM.SetPreference(ctx, lastAcceptedID); err != nil {
+				return err
+			}
 		}
 	} else if err := t.VM.SetPreference(ctx, lastAcceptedID); err != nil {
 		return err
@@ -1015,11 +1019,8 @@ func (t *Transitive) deliver(
 	dropped := []snowman.Block{}
 	if blk, ok := blk.(snowman.OracleBlock); ok {
 		options, err := blk.Options(ctx)
-		if err != snowman.ErrNotOracle {
-			if err != nil {
-				return err
-			}
-
+		switch {
+		case err == nil:
 			for _, blk := range options {
 				blkAdded, err := t.addUnverifiedBlockToConsensus(ctx, nodeID, blk, issuedMetric)
 				if err != nil {
@@ -1031,6 +1032,11 @@ func (t *Transitive) deliver(
 					dropped = append(dropped, blk)
 				}
 			}
+		case err != snowman.ErrNotOracle:
+			t.Ctx.Log.Debug("failed to set preferences for oracle block",
+				zap.Stringer("blkID", blkID),
+				zap.Error(err),
+			)
 		}
 	}
 

vms/platformvm/block/executor/acceptor.go

@@ -33,11 +33,11 @@ type acceptor struct {
 }
 
 func (a *acceptor) BanffAbortBlock(b *block.BanffAbortBlock) error {
-	return a.abortBlock(b, "banff abort")
+	return a.optionBlock(b, "banff abort")
 }
 
 func (a *acceptor) BanffCommitBlock(b *block.BanffCommitBlock) error {
-	return a.commitBlock(b, "apricot commit")
+	return a.optionBlock(b, "banff commit")
 }
 
 func (a *acceptor) BanffProposalBlock(b *block.BanffProposalBlock) error {
@@ -50,11 +50,11 @@ func (a *acceptor) BanffStandardBlock(b *block.BanffStandardBlock) error {
 }
 
 func (a *acceptor) ApricotAbortBlock(b *block.ApricotAbortBlock) error {
-	return a.abortBlock(b, "apricot abort")
+	return a.optionBlock(b, "apricot abort")
 }
 
 func (a *acceptor) ApricotCommitBlock(b *block.ApricotCommitBlock) error {
-	return a.commitBlock(b, "apricot commit")
+	return a.optionBlock(b, "apricot commit")
 }
 
 func (a *acceptor) ApricotProposalBlock(b *block.ApricotProposalBlock) error {
@@ -116,46 +116,14 @@ func (a *acceptor) ApricotAtomicBlock(b *block.ApricotAtomicBlock) error {
 	return nil
 }
 
-func (a *acceptor) abortBlock(b block.Block, blockType string) error {
+func (a *acceptor) optionBlock(b block.Block, blockType string) error {
 	parentID := b.Parent()
 	parentState, ok := a.blkIDToState[parentID]
 	if !ok {
 		return fmt.Errorf("%w: %s", state.ErrMissingParentState, parentID)
 	}
 
-	if a.bootstrapped.Get() {
-		if parentState.initiallyPreferCommit {
-			a.metrics.MarkOptionVoteLost()
-		} else {
-			a.metrics.MarkOptionVoteWon()
-		}
-	}
-
-	return a.optionBlock(b, parentState.statelessBlock, blockType)
-}
-
-func (a *acceptor) commitBlock(b block.Block, blockType string) error {
-	parentID := b.Parent()
-	parentState, ok := a.blkIDToState[parentID]
-	if !ok {
-		return fmt.Errorf("%w: %s", state.ErrMissingParentState, parentID)
-	}
-
-	if a.bootstrapped.Get() {
-		if parentState.initiallyPreferCommit {
-			a.metrics.MarkOptionVoteWon()
-		} else {
-			a.metrics.MarkOptionVoteLost()
-		}
-	}
-
-	return a.optionBlock(b, parentState.statelessBlock, blockType)
-}
-
-func (a *acceptor) optionBlock(b, parent block.Block, blockType string) error {
 	blkID := b.ID()
-	parentID := parent.ID()
-
 	defer func() {
 		// Note: we assume this block's sibling doesn't
 		// need the parent's state when it's rejected.
@@ -164,7 +132,7 @@ func (a *acceptor) optionBlock(b, parent block.Block, blockType string) error {
 	}()
 
 	// Note that the parent must be accepted first.
-	if err := a.commonAccept(parent); err != nil {
+	if err := a.commonAccept(parentState.statelessBlock); err != nil {
 		return err
 	}
 

vms/platformvm/block/executor/acceptor_test.go

@@ -295,7 +295,7 @@ func TestAcceptorVisitCommitBlock(t *testing.T) {
 	// Set expected calls on dependencies.
 	// Make sure the parent is accepted first.
 	gomock.InOrder(
-		parentStatelessBlk.EXPECT().ID().Return(parentID).Times(2),
+		parentStatelessBlk.EXPECT().ID().Return(parentID).Times(1),
 		s.EXPECT().SetLastAccepted(parentID).Times(1),
 		parentStatelessBlk.EXPECT().Height().Return(blk.Height()-1).Times(1),
 		s.EXPECT().SetHeight(blk.Height()-1).Times(1),
@@ -319,7 +319,7 @@ func TestAcceptorVisitCommitBlock(t *testing.T) {
 	// Set expected calls on dependencies.
 	// Make sure the parent is accepted first.
 	gomock.InOrder(
-		parentStatelessBlk.EXPECT().ID().Return(parentID).Times(2),
+		parentStatelessBlk.EXPECT().ID().Return(parentID).Times(1),
 		s.EXPECT().SetLastAccepted(parentID).Times(1),
 		parentStatelessBlk.EXPECT().Height().Return(blk.Height()-1).Times(1),
 		s.EXPECT().SetHeight(blk.Height()-1).Times(1),
@@ -386,7 +386,7 @@ func TestAcceptorVisitAbortBlock(t *testing.T) {
 	// Set expected calls on dependencies.
 	// Make sure the parent is accepted first.
 	gomock.InOrder(
-		parentStatelessBlk.EXPECT().ID().Return(parentID).Times(2),
+		parentStatelessBlk.EXPECT().ID().Return(parentID).Times(1),
 		s.EXPECT().SetLastAccepted(parentID).Times(1),
 		parentStatelessBlk.EXPECT().Height().Return(blk.Height()-1).Times(1),
 		s.EXPECT().SetHeight(blk.Height()-1).Times(1),
@@ -411,7 +411,7 @@ func TestAcceptorVisitAbortBlock(t *testing.T) {
 	// Set expected calls on dependencies.
 	// Make sure the parent is accepted first.
 	gomock.InOrder(
-		parentStatelessBlk.EXPECT().ID().Return(parentID).Times(2),
+		parentStatelessBlk.EXPECT().ID().Return(parentID).Times(1),
 		s.EXPECT().SetLastAccepted(parentID).Times(1),
 		parentStatelessBlk.EXPECT().Height().Return(blk.Height()-1).Times(1),
 		s.EXPECT().SetHeight(blk.Height()-1).Times(1),

vms/platformvm/block/executor/block.go

@@ -5,7 +5,6 @@ package executor
 
 import (
 	"context"
-	"fmt"
 	"time"
 
 	"go.uber.org/zap"
@@ -83,22 +82,17 @@ func (b *Block) Timestamp() time.Time {
 }
 
 func (b *Block) Options(context.Context) ([2]snowman.Block, error) {
-	options := options{}
+	options := options{
+		primaryUptimePercentage: b.manager.txExecutorBackend.Config.UptimePercentage,
+		uptimes:                 b.manager.txExecutorBackend.Uptimes,
+		state:                   b.manager.backend.state,
+	}
 	if err := b.Block.Visit(&options); err != nil {
 		return [2]snowman.Block{}, err
 	}
 
-	commitBlock := b.manager.NewBlock(options.commitBlock)
-	abortBlock := b.manager.NewBlock(options.abortBlock)
-
-	blkID := b.ID()
-	blkState, ok := b.manager.blkIDToState[blkID]
-	if !ok {
-		return [2]snowman.Block{}, fmt.Errorf("block %s state not found", blkID)
-	}
-
-	if blkState.initiallyPreferCommit {
-		return [2]snowman.Block{commitBlock, abortBlock}, nil
-	}
-	return [2]snowman.Block{abortBlock, commitBlock}, nil
+	return [2]snowman.Block{
+		b.manager.NewBlock(options.preferredBlock),
+		b.manager.NewBlock(options.alternateBlock),
+	}, nil
 }

vms/platformvm/block/executor/block_state.go

@@ -19,9 +19,8 @@ type standardBlockState struct {
 }
 
 type proposalBlockState struct {
-	initiallyPreferCommit bool
-	onCommitState         state.Diff
-	onAbortState          state.Diff
+	onCommitState state.Diff
+	onAbortState  state.Diff
 }
 
 // The state of a block.