Improve dynamic state sync transition comments (#1873)

Signed-off-by: aaronbuchwald <[email protected]>

snow/block.go

@@ -137,7 +137,7 @@ func (b *StatefulBlock[I, O, A]) Verify(ctx context.Context) error {
 		b.vm.metrics.blockVerify.Observe(float64(time.Since(start)))
 	}()
 
-	ready := b.vm.ready.Load()
+	ready := b.vm.ready
 	ctx, span := b.vm.tracer.Start(
 		ctx, "StatefulBlock.Verify",
 		trace.WithAttributes(
@@ -260,7 +260,7 @@ func (b *StatefulBlock[I, O, A]) Accept(ctx context.Context) error {
 	defer b.vm.log.Info("accepting block", zap.Stringer("block", b))
 
 	// If I'm not ready yet, mark myself as accepted, and return early.
-	isReady := b.vm.ready.Load()
+	isReady := b.vm.ready
 	if !isReady {
 		return b.markAccepted(ctx, nil)
 	}

snow/chain_index.go

@@ -45,13 +45,13 @@ func (v *VM[I, O, A]) makeConsensusIndex(
 
 	var lastAcceptedBlock *StatefulBlock[I, O, A]
 	if stateReady {
-		v.ready.Store(true)
+		v.ready = true
 		lastAcceptedBlock, err = v.reprocessFromOutputToInput(ctx, inputBlock, outputBlock, acceptedBlock)
 		if err != nil {
 			return err
 		}
 	} else {
-		v.ready.Store(false)
+		v.ready = false
 		lastAcceptedBlock = NewInputBlock(v, inputBlock)
 	}
 	v.setLastAccepted(lastAcceptedBlock)
@@ -121,6 +121,18 @@ func (c *ConsensusIndex[I, O, A]) GetBlock(ctx context.Context, blkID ids.ID) (I
 	return blk.Input, nil
 }
 
+// GetPreferredBlock returns the output block of the current preference.
+//
+// Prior to dynamic state sync, GetPreferredBlock will return an error because the preference
+// will not have been verified.
+// After completing dynamic state sync, all outstanding processing blocks will be verified.
+// However, there's an edge case where the node may have vacuously verified an invalid block
+// during dynamic state sync, such that the preferred block is invalid and its output is
+// empty.
+// Consensus should guarantee that we do not accept such a block even if it's preferred as
+// long as a majority of validators are correct.
+// After outstanding processing blocks have been Accepted/Rejected, the preferred block
+// will be verified and the output will be available.
 func (c *ConsensusIndex[I, O, A]) GetPreferredBlock(ctx context.Context) (O, error) {
 	c.vm.metaLock.Lock()
 	preference := c.vm.preferredBlkID
@@ -136,10 +148,15 @@ func (c *ConsensusIndex[I, O, A]) GetPreferredBlock(ctx context.Context) (O, err
 	return blk.Output, nil
 }
 
+// GetLastAccepted returns the last accepted block of the chain.
+//
+// If the chain is mid dynamic state sync, GetLastAccepted will return an error
+// because the last accepted block will not be populated.
 func (c *ConsensusIndex[I, O, A]) GetLastAccepted(context.Context) (A, error) {
 	c.vm.metaLock.Lock()
+	defer c.vm.metaLock.Unlock()
+
 	lastAccepted := c.vm.lastAcceptedBlock
-	c.vm.metaLock.Unlock()
 
 	if !lastAccepted.accepted {
 		return utils.Zero[A](), fmt.Errorf("last accepted block %s has not been populated", lastAccepted)

snow/statesync.go

@@ -26,7 +26,7 @@ func (v *VM[I, O, A]) StartStateSync(ctx context.Context, block I) error {
 	if err := v.inputChainIndex.UpdateLastAccepted(ctx, block); err != nil {
 		return err
 	}
-	v.ready.Store(false)
+	v.ready = false
 	v.setLastAccepted(NewInputBlock(v, block))
 	return nil
 }
@@ -38,7 +38,7 @@ func (v *VM[I, O, A]) FinishStateSync(ctx context.Context, input I, output O, ac
 	defer v.chainLock.Unlock()
 
 	// Cannot call FinishStateSync if already marked as ready and in normal operation
-	if v.ready.Load() {
+	if v.ready {
 		return fmt.Errorf("can't finish dynamic state sync from normal operation: %s", input)
 	}
 
@@ -68,7 +68,7 @@ func (v *VM[I, O, A]) FinishStateSync(ctx context.Context, input I, output O, ac
 		return err
 	}
 
-	v.ready.Store(true)
+	v.ready = true
 	return nil
 }
 

snow/vm.go

@@ -9,7 +9,6 @@ import (
 	"fmt"
 	"net/http"
 	"sync"
-	"sync/atomic"
 	"time"
 
 	"github.com/ava-labs/avalanchego/api/health"
@@ -104,9 +103,17 @@ type VM[I Block, O Block, A Block] struct {
 	preReadyAcceptedSubs []event.Subscription[I]
 	version              string
 
-	// chainLock must be held to process a block with chain (Build/Verify/Accept).
-	chainLock       sync.Mutex
-	chain           Chain[I, O, A]
+	// chainLock provides a synchronization point between state sync and normal operation.
+	// To complete dynamic state sync, we must:
+	// 1. Accept a sequence of blocks from the final state sync target to the last accepted block
+	// 2. Re-process all outstanding processing blocks
+	// 3. Mark the VM as ready for normal operation
+	//
+	// During this time, we must not allow any new blocks to be verified/accepted.
+	chainLock sync.Mutex
+	chain     Chain[I, O, A]
+	ready     bool
+
 	inputChainIndex ChainIndex[I]
 	consensusIndex  *ConsensusIndex[I, O, A]
 
@@ -131,8 +138,6 @@ type VM[I Block, O Block, A Block] struct {
 	lastAcceptedBlock *StatefulBlock[I, O, A]
 	preferredBlkID    ids.ID
 
-	ready atomic.Bool
-
 	metrics *Metrics
 	log     logging.Logger
 	tracer  trace.Tracer
@@ -283,10 +288,6 @@ func (v *VM[I, O, A]) GetBlock(ctx context.Context, blkID ids.ID) (*StatefulBloc
 	}
 	v.verifiedL.RUnlock()
 
-	// Check if last accepted block or recently accepted
-	if v.lastAcceptedBlock.ID() == blkID {
-		return v.lastAcceptedBlock, nil
-	}
 	if blk, ok := v.acceptedBlocksByID.Get(blkID); ok {
 		return blk, nil
 	}