Fixes a deep bug in the resolve virtual process

domain/consensus/processes/consensusstatemanager/resolve.go

@@ -5,17 +5,15 @@ import (
 	"github.com/kaspanet/kaspad/domain/consensus/model/externalapi"
 	"github.com/kaspanet/kaspad/infrastructure/logger"
 	"github.com/kaspanet/kaspad/util/staging"
+	"github.com/pkg/errors"
 	"sort"
 )
 
-func (csm *consensusStateManager) ResolveVirtual(maxBlocksToResolve uint64) (*externalapi.VirtualChangeSet, bool, error) {
+func (csm *consensusStateManager) findNextPendingTip() (*externalapi.DomainHash, externalapi.BlockStatus, error) {
-	onEnd := logger.LogAndMeasureExecutionTime(log, "csm.ResolveVirtual")
-	defer onEnd()
-
 	readStagingArea := model.NewStagingArea()
 	tips, err := csm.consensusStateStore.Tips(readStagingArea, csm.databaseContext)
 	if err != nil {
-		return nil, false, err
+		return nil, externalapi.StatusInvalid, err
 	}
 
 	var sortErr error
@@ -29,16 +27,14 @@ func (csm *consensusStateManager) ResolveVirtual(maxBlocksToResolve uint64) (*ex
 		return selectedParent.Equal(tips[i])
 	})
 	if sortErr != nil {
-		return nil, false, sortErr
+		return nil, externalapi.StatusInvalid, sortErr
 	}
 
-	var selectedTip *externalapi.DomainHash
-	isCompletelyResolved := true
 	for _, tip := range tips {
 		log.Debugf("Resolving tip %s", tip)
 		isViolatingFinality, shouldNotify, err := csm.isViolatingFinality(readStagingArea, tip)
 		if err != nil {
-			return nil, false, err
+			return nil, externalapi.StatusInvalid, err
 		}
 
 		if isViolatingFinality {
@@ -49,55 +45,125 @@ func (csm *consensusStateManager) ResolveVirtual(maxBlocksToResolve uint64) (*ex
 			continue
 		}
 
+		status, err := csm.blockStatusStore.Get(csm.databaseContext, readStagingArea, tip)
+		if err != nil {
+			return nil, externalapi.StatusInvalid, err
+		}
+		if status == externalapi.StatusUTXOValid || status == externalapi.StatusUTXOPendingVerification {
+			return tip, status, nil
+		}
+	}
+
+	return nil, externalapi.StatusInvalid, nil
+}
+
+func (csm *consensusStateManager) ResolveVirtual(maxBlocksToResolve uint64) (*externalapi.VirtualChangeSet, bool, error) {
+	onEnd := logger.LogAndMeasureExecutionTime(log, "csm.ResolveVirtual")
+	defer onEnd()
+
+	readStagingArea := model.NewStagingArea()
+
+	/*
+		Algo (begin resolve):
+			Go over tips by GHOSTDAG select parent order ignoring UTXO disqualified blocks and finality violating blocks
+			Set pending tip to the first tip
+			if this tip is already UTXO valid, finalize virtual state and return
+			if the tip is UTXO pending, find the earliest UTXO pending block in its chain and set it as position
+			set the tip as resolving virtual pending
+			try resolving a chunk up the chain from position to pending tip
+
+		Algo (continue resolve):
+			Start from position and try to continue resolving another chunk up the chain to pending tip
+			If we encounter a UTXO disqualified block, we should
+				mark the whole chain up to pending tip as disqualified
+				set position and pending tip to the next candidate chain
+				return and let the next call continue the processing
+			If we reach the tip, and it is valid, only then set virtual parents to DAG tips, and clear resolving state
+	*/
+
+	pendingTip, pendingTipStatus, err := csm.findNextPendingTip()
+	if err != nil {
+		return nil, false, err
+	}
+
+	if pendingTip == nil {
+		log.Warnf("Non of the DAG tips are valid")
+		return &externalapi.VirtualChangeSet{}, true, nil
+	}
+
+	prevVirtualSelectedParent, err := csm.virtualSelectedParent(readStagingArea)
+	if err != nil {
+		return nil, false, err
+	}
+
+	if pendingTipStatus == externalapi.StatusUTXOValid && prevVirtualSelectedParent.Equal(pendingTip) {
+		return &externalapi.VirtualChangeSet{}, true, nil
+	}
+
+	// Resolve a chunk from the pending chain
 	resolveStagingArea := model.NewStagingArea()
-		unverifiedBlocks, err := csm.getUnverifiedChainBlocks(resolveStagingArea, tip)
+	unverifiedBlocks, err := csm.getUnverifiedChainBlocks(resolveStagingArea, pendingTip)
 	if err != nil {
 		return nil, false, err
 	}
 
-		resolveTip := tip
+	intermediateTip := pendingTip
-		hasMoreUnverifiedThanMax := maxBlocksToResolve != 0 && uint64(len(unverifiedBlocks)) > maxBlocksToResolve
-		if hasMoreUnverifiedThanMax {
-			resolveTip = unverifiedBlocks[uint64(len(unverifiedBlocks))-maxBlocksToResolve]
-			log.Debugf("Has more than %d blocks to resolve. Changing the resolve tip to %s", maxBlocksToResolve, resolveTip)
-		}
 
-		blockStatus, reversalData, err := csm.resolveBlockStatus(resolveStagingArea, resolveTip, true)
+	// Too many blocks to verify, so we only process a chunk and return
+	if maxBlocksToResolve != 0 && uint64(len(unverifiedBlocks)) > maxBlocksToResolve {
+
+		intermediateTipIndex := uint64(len(unverifiedBlocks)) - maxBlocksToResolve
+		intermediateTip = unverifiedBlocks[intermediateTipIndex]
+		isNewVirtualSelectedParent, err := csm.isNewSelectedTip(readStagingArea, intermediateTip, prevVirtualSelectedParent)
 		if err != nil {
 			return nil, false, err
 		}
 
-		if blockStatus == externalapi.StatusUTXOValid {
+		// We must find an intermediate tip which wins previous virtual selected parent
-			selectedTip = resolveTip
+		// even if we process more than `maxBlocksToResolve` for that.
-			isCompletelyResolved = !hasMoreUnverifiedThanMax
+		// Otherwise, internal UTXO diff logic gets all messed up
+		for !isNewVirtualSelectedParent {
+			if intermediateTipIndex == 0 {
+				return nil, false, errors.Errorf(
+					"Expecting the pending tip %s to overcome the previous selected parent %s", pendingTip, prevVirtualSelectedParent)
+			}
+			intermediateTipIndex--
+			intermediateTip = unverifiedBlocks[intermediateTipIndex]
+			isNewVirtualSelectedParent, err = csm.isNewSelectedTip(readStagingArea, intermediateTip, prevVirtualSelectedParent)
+			if err != nil {
+				return nil, false, err
+			}
+		}
+		log.Debugf("Has more than %d blocks to resolve. Changing the resolve tip to %s", maxBlocksToResolve, intermediateTip)
+	}
 
+	intermediateTipStatus, reversalData, err := csm.resolveBlockStatus(
+		resolveStagingArea, intermediateTip, true)
+	if err != nil {
+		return nil, false, err
+	}
+
+	if intermediateTipStatus == externalapi.StatusUTXOValid {
 		err = staging.CommitAllChanges(csm.databaseContext, resolveStagingArea)
 		if err != nil {
 			return nil, false, err
 		}
 
 		if reversalData != nil {
-				err = csm.ReverseUTXODiffs(resolveTip, reversalData)
+			err = csm.ReverseUTXODiffs(intermediateTip, reversalData)
 			if err != nil {
 				return nil, false, err
 			}
 		}
-			break
-		}
 	}
 
-	if selectedTip == nil {
+	isActualTip := intermediateTip.Equal(pendingTip)
-		log.Warnf("Non of the DAG tips are valid")
+	isCompletelyResolved := isActualTip && intermediateTipStatus == externalapi.StatusUTXOValid
-		return &externalapi.VirtualChangeSet{}, true, nil
-	}
-
-	oldVirtualGHOSTDAGData, err := csm.ghostdagDataStore.Get(csm.databaseContext, readStagingArea, model.VirtualBlockHash, false)
-	if err != nil {
-		return nil, false, err
-	}
 
 	updateVirtualStagingArea := model.NewStagingArea()
-	virtualUTXODiff, err := csm.updateVirtualWithParents(updateVirtualStagingArea, []*externalapi.DomainHash{selectedTip})
+
+	// TODO: if `isCompletelyResolved`, set virtual correctly with all tips which have less blue work than pending
+	virtualUTXODiff, err := csm.updateVirtualWithParents(updateVirtualStagingArea, []*externalapi.DomainHash{intermediateTip})
 	if err != nil {
 		return nil, false, err
 	}
@@ -107,13 +173,14 @@ func (csm *consensusStateManager) ResolveVirtual(maxBlocksToResolve uint64) (*ex
 		return nil, false, err
 	}
 
+	// TODO: why was `readStagingArea` used here ?
 	selectedParentChainChanges, err := csm.dagTraversalManager.
-		CalculateChainPath(readStagingArea, oldVirtualGHOSTDAGData.SelectedParent(), selectedTip)
+		CalculateChainPath(updateVirtualStagingArea, prevVirtualSelectedParent, pendingTip)
 	if err != nil {
 		return nil, false, err
 	}
 
-	virtualParents, err := csm.dagTopologyManager.Parents(readStagingArea, model.VirtualBlockHash)
+	virtualParents, err := csm.dagTopologyManager.Parents(updateVirtualStagingArea, model.VirtualBlockHash)
 	if err != nil {
 		return nil, false, err
 	}

domain/consensus/processes/consensusstatemanager/resolve_block_status.go

@@ -233,7 +233,7 @@ func (csm *consensusStateManager) resolveSingleBlockStatus(stagingArea *model.St
 		return externalapi.StatusUTXOValid, nil, nil
 	}
 
-	oldSelectedTip, err := csm.selectedTip(stagingArea)
+	oldSelectedTip, err := csm.virtualSelectedParent(stagingArea)
 	if err != nil {
 		return 0, nil, err
 	}
@@ -298,7 +298,7 @@ func (csm *consensusStateManager) isNewSelectedTip(stagingArea *model.StagingAre
 	return blockHash.Equal(newSelectedTip), nil
 }
 
-func (csm *consensusStateManager) selectedTip(stagingArea *model.StagingArea) (*externalapi.DomainHash, error) {
+func (csm *consensusStateManager) virtualSelectedParent(stagingArea *model.StagingArea) (*externalapi.DomainHash, error) {
 	virtualGHOSTDAGData, err := csm.ghostdagDataStore.Get(csm.databaseContext, stagingArea, model.VirtualBlockHash, false)
 	if err != nil {
 		return nil, err

domain/consensus/processes/consensusstatemanager/update_virtual.go

@@ -110,7 +110,7 @@ func (csm *consensusStateManager) updateSelectedTipUTXODiff(
 	onEnd := logger.LogAndMeasureExecutionTime(log, "updateSelectedTipUTXODiff")
 	defer onEnd()
 
-	selectedTip, err := csm.selectedTip(stagingArea)
+	selectedTip, err := csm.virtualSelectedParent(stagingArea)
 	if err != nil {
 		return err
 	}