Allow calls to Options before Verify (#2363)

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)
 }

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,18 +132,14 @@ 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
 	}
 
 	if err := a.commonAccept(b); err != nil {
 		return err
 	}
 
-	parentState, ok := a.blkIDToState[parentID]
-	if !ok {
-		return fmt.Errorf("%w %s", errMissingBlockState, parentID)
-	}
 	if parentState.onDecisionState != nil {
 		if err := parentState.onDecisionState.Apply(a.state); err != nil {
 			return err

vms/platformvm/block/executor/acceptor_test.go

@@ -303,7 +303,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),
@@ -335,7 +335,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),
@@ -413,7 +413,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),
@@ -445,7 +445,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,18 @@ func (b *Block) Timestamp() time.Time {
 }
 
 func (b *Block) Options(context.Context) ([2]snowman.Block, error) {
-	options := options{}
+	options := options{
+		log:                     b.manager.ctx.Log,
+		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

@@ -14,10 +14,9 @@ import (
 )
 
 type proposalBlockState struct {
-	initiallyPreferCommit bool
-	onDecisionState       state.Diff
-	onCommitState         state.Diff
-	onAbortState          state.Diff
+	onDecisionState state.Diff
+	onCommitState   state.Diff
+	onAbortState    state.Diff
 }
 
 // The state of a block.