Make use of maxBlocks instead of maxBlueScoreDifference in antiPastHashesBetween (#1772)

* Make use of maxBlocks instead of maxBlueScoreDifference in antiPastHashesBetween

* Make use of maxBlocks instead of maxBlueScoreDifference in antiPastHashesBetween

* Make use of maxBlocks instead of maxBlueScoreDifference in antiPastHashesBetween

* Make use of maxBlocks instead of maxBlueScoreDifference in antiPastHashesBetween

* Make use of maxBlocks instead of maxBlueScoreDifference in antiPastHashesBetween

Co-authored-by: Ori Newman <orinewman1@gmail.com>

app/protocol/flows/blockrelay/handle_request_headers.go

@@ -49,8 +49,9 @@ func (flow *handleRequestHeadersFlow) start() error {
 
 			// GetHashesBetween is a relatively heavy operation so we limit it
 			// in order to avoid locking the consensus for too long
-			const maxBlueScoreDifference = 1 << 10
+			// maxBlocks MUST be >= MergeSetSizeLimit + 1
-			blockHashes, _, err := flow.Domain().Consensus().GetHashesBetween(lowHash, highHash, maxBlueScoreDifference)
+			const maxBlocks = 1 << 10
+			blockHashes, _, err := flow.Domain().Consensus().GetHashesBetween(lowHash, highHash, maxBlocks)
 			if err != nil {
 				return err
 			}

app/protocol/flows/testing/handle_relay_invs_test.go

@@ -206,7 +206,7 @@ func (f *fakeRelayInvsContext) GetBlockAcceptanceData(blockHash *externalapi.Dom
 	panic(errors.Errorf("called unimplemented function from test '%s'", f.testName))
 }
 
-func (f *fakeRelayInvsContext) GetHashesBetween(lowHash, highHash *externalapi.DomainHash, maxBlueScoreDifference uint64) (hashes []*externalapi.DomainHash, actualHighHash *externalapi.DomainHash, err error) {
+func (f *fakeRelayInvsContext) GetHashesBetween(lowHash, highHash *externalapi.DomainHash, maxBlocks uint64) (hashes []*externalapi.DomainHash, actualHighHash *externalapi.DomainHash, err error) {
 	panic(errors.Errorf("called unimplemented function from test '%s'", f.testName))
 }
 

app/rpc/rpchandlers/get_blocks.go

@@ -8,12 +8,6 @@ import (
 	"github.com/kaspanet/kaspad/infrastructure/network/netadapter/router"
 )
 
-const (
-	// maxBlocksInGetBlocksResponse is the max amount of blocks that are
-	// allowed in a GetBlocksResult.
-	maxBlocksInGetBlocksResponse = 1000
-)
-
 // HandleGetBlocks handles the respectively named RPC command
 func HandleGetBlocks(context *rpccontext.Context, _ *router.Router, request appmessage.Message) (appmessage.Message, error) {
 	getBlocksRequest := request.(*appmessage.GetBlocksRequestMessage)
@@ -55,7 +49,11 @@ func HandleGetBlocks(context *rpccontext.Context, _ *router.Router, request appm
 	if err != nil {
 		return nil, err
 	}
-	blockHashes, highHash, err := context.Domain.Consensus().GetHashesBetween(lowHash, virtualSelectedParent, maxBlocksInGetBlocksResponse)
+
+	// We use +1 because lowHash is also returned
+	// maxBlocks MUST be >= MergeSetSizeLimit + 1
+	maxBlocks := context.Config.NetParams().MergeSetSizeLimit + 1
+	blockHashes, highHash, err := context.Domain.Consensus().GetHashesBetween(lowHash, virtualSelectedParent, maxBlocks)
 	if err != nil {
 		return nil, err
 	}
@@ -74,12 +72,6 @@ func HandleGetBlocks(context *rpccontext.Context, _ *router.Router, request appm
 		blockHashes = append(blockHashes, virtualSelectedParentAnticone...)
 	}
 
-	// Both GetHashesBetween and Anticone might return more then the allowed number of blocks, so
-	// trim any extra blocks.
-	if len(blockHashes) > maxBlocksInGetBlocksResponse {
-		blockHashes = blockHashes[:maxBlocksInGetBlocksResponse]
-	}
-
 	// Prepare the response
 	response := appmessage.NewGetBlocksResponseMessage()
 	response.BlockHashes = hashes.ToStrings(blockHashes)

domain/consensus/consensus.go

@@ -226,7 +226,7 @@ func (s *consensus) GetBlockAcceptanceData(blockHash *externalapi.DomainHash) (e
 	return s.acceptanceDataStore.Get(s.databaseContext, stagingArea, blockHash)
 }
 
-func (s *consensus) GetHashesBetween(lowHash, highHash *externalapi.DomainHash, maxBlueScoreDifference uint64) (
+func (s *consensus) GetHashesBetween(lowHash, highHash *externalapi.DomainHash, maxBlocks uint64) (
 	hashes []*externalapi.DomainHash, actualHighHash *externalapi.DomainHash, err error) {
 
 	s.lock.Lock()
@@ -243,7 +243,7 @@ func (s *consensus) GetHashesBetween(lowHash, highHash *externalapi.DomainHash,
 		return nil, nil, err
 	}
 
-	return s.syncManager.GetHashesBetween(stagingArea, lowHash, highHash, maxBlueScoreDifference)
+	return s.syncManager.GetHashesBetween(stagingArea, lowHash, highHash, maxBlocks)
 }
 
 func (s *consensus) GetMissingBlockBodyHashes(highHash *externalapi.DomainHash) ([]*externalapi.DomainHash, error) {

domain/consensus/factory.go

@@ -317,6 +317,7 @@ func (f *factory) NewConsensus(config *Config, db infrastructuredatabase.Databas
 	syncManager := syncmanager.New(
 		dbManager,
 		genesisHash,
+		config.MergeSetSizeLimit,
 		dagTraversalManager,
 		dagTopologyManager,
 		ghostdagManager,

domain/consensus/model/externalapi/consensus.go

@@ -13,7 +13,7 @@ type Consensus interface {
 	GetBlockRelations(blockHash *DomainHash) (parents []*DomainHash, selectedParent *DomainHash, children []*DomainHash, err error)
 	GetBlockAcceptanceData(blockHash *DomainHash) (AcceptanceData, error)
 
-	GetHashesBetween(lowHash, highHash *DomainHash, maxBlueScoreDifference uint64) (hashes []*DomainHash, actualHighHash *DomainHash, err error)
+	GetHashesBetween(lowHash, highHash *DomainHash, maxBlocks uint64) (hashes []*DomainHash, actualHighHash *DomainHash, err error)
 	GetMissingBlockBodyHashes(highHash *DomainHash) ([]*DomainHash, error)
 	GetPruningPointUTXOs(expectedPruningPointHash *DomainHash, fromOutpoint *DomainOutpoint, limit int) ([]*OutpointAndUTXOEntryPair, error)
 	GetVirtualUTXOs(expectedVirtualParents []*DomainHash, fromOutpoint *DomainOutpoint, limit int) ([]*OutpointAndUTXOEntryPair, error)

domain/consensus/model/interface_processes_syncmanager.go

@@ -4,7 +4,7 @@ import "github.com/kaspanet/kaspad/domain/consensus/model/externalapi"
 
 // SyncManager exposes functions to support sync between kaspad nodes
 type SyncManager interface {
-	GetHashesBetween(stagingArea *StagingArea, lowHash, highHash *externalapi.DomainHash, maxBlueScoreDifference uint64) (
+	GetHashesBetween(stagingArea *StagingArea, lowHash, highHash *externalapi.DomainHash, maxBlocks uint64) (
 		hashes []*externalapi.DomainHash, actualHighHash *externalapi.DomainHash, err error)
 	GetMissingBlockBodyHashes(stagingArea *StagingArea, highHash *externalapi.DomainHash) (
 		[]*externalapi.DomainHash, error)

domain/consensus/processes/syncmanager/antipast.go

@@ -7,11 +7,19 @@ import (
 )
 
 // antiPastHashesBetween returns the hashes of the blocks between the
-// lowHash's antiPast and highHash's antiPast, or up to
+// lowHash's antiPast and highHash's antiPast, or up to `maxBlocks`, if non-zero.
-// `maxBlueScoreDifference`, if non-zero.
 // The result excludes lowHash and includes highHash. If lowHash == highHash, returns nothing.
+// If maxBlocks != 0 then maxBlocks MUST be >= MergeSetSizeLimit + 1
+// because it returns blocks with MergeSet granularity,
+// so if MergeSet > maxBlocks, function will return nothing
 func (sm *syncManager) antiPastHashesBetween(stagingArea *model.StagingArea, lowHash, highHash *externalapi.DomainHash,
-	maxBlueScoreDifference uint64) (hashes []*externalapi.DomainHash, actualHighHash *externalapi.DomainHash, err error) {
+	maxBlocks uint64) (hashes []*externalapi.DomainHash, actualHighHash *externalapi.DomainHash, err error) {
+
+	// Sanity check, for debugging only
+	if maxBlocks != 0 && maxBlocks < sm.mergeSetSizeLimit+1 {
+		return nil, nil,
+			errors.Errorf("maxBlocks (%d) MUST be >= MergeSetSizeLimit + 1 (%d)", maxBlocks, sm.mergeSetSizeLimit+1)
+	}
 
 	// If lowHash is not in the selectedParentChain of highHash - SelectedChildIterator will fail.
 	// Therefore, we traverse down lowHash's selectedParentChain until we reach a block that is in
@@ -36,23 +44,6 @@ func (sm *syncManager) antiPastHashesBetween(stagingArea *model.StagingArea, low
 			lowBlockGHOSTDAGData.BlueScore(), highBlockGHOSTDAGData.BlueScore())
 	}
 
-	if maxBlueScoreDifference != 0 {
-		// In order to get no more then maxBlueScoreDifference
-		// blocks from the future of the lowHash (including itself),
-		// we iterate the selected parent chain of the highNode and
-		// stop once we reach
-		// highBlockBlueScore-lowBlockBlueScore+1 <= maxBlueScoreDifference.
-		// That stop point becomes the new highHash.
-		// Using blueScore as an approximation is considered to be
-		// fairly accurate because we presume that most DAG blocks are
-		// blue.
-		highHash, err = sm.findHighHashAccordingToMaxBlueScoreDifference(stagingArea,
-			lowHash, highHash, maxBlueScoreDifference, highBlockGHOSTDAGData, lowBlockGHOSTDAGData)
-		if err != nil {
-			return nil, nil, err
-		}
-	}
-
 	// Collect all hashes by concatenating the merge-sets of all blocks between highHash and lowHash
 	blockHashes := []*externalapi.DomainHash{}
 	iterator, err := sm.dagTraversalManager.SelectedChildIterator(stagingArea, highHash, lowHash)
@@ -76,6 +67,12 @@ func (sm *syncManager) antiPastHashesBetween(stagingArea *model.StagingArea, low
 			return nil, nil, err
 		}
 
+		if maxBlocks != 0 && uint64(len(blockHashes)+len(sortedMergeSet)) > maxBlocks {
+			break
+		}
+
+		highHash = current
+
 		// append to blockHashes all blocks in sortedMergeSet which are not in the past of originalLowHash
 		for _, blockHash := range sortedMergeSet {
 			isInPastOfOriginalLowHash, err := sm.dagTopologyManager.IsAncestorOf(stagingArea, blockHash, originalLowHash)
@@ -97,36 +94,6 @@ func (sm *syncManager) antiPastHashesBetween(stagingArea *model.StagingArea, low
 	return blockHashes, highHash, nil
 }
 
-func (sm *syncManager) findHighHashAccordingToMaxBlueScoreDifference(stagingArea *model.StagingArea, lowHash *externalapi.DomainHash,
-	highHash *externalapi.DomainHash, maxBlueScoreDifference uint64, highBlockGHOSTDAGData *model.BlockGHOSTDAGData,
-	lowBlockGHOSTDAGData *model.BlockGHOSTDAGData) (*externalapi.DomainHash, error) {
-
-	if highBlockGHOSTDAGData.BlueScore()-lowBlockGHOSTDAGData.BlueScore() <= maxBlueScoreDifference {
-		return highHash, nil
-	}
-
-	iterator, err := sm.dagTraversalManager.SelectedChildIterator(stagingArea, highHash, lowHash)
-	if err != nil {
-		return nil, err
-	}
-	defer iterator.Close()
-	for ok := iterator.First(); ok; ok = iterator.Next() {
-		highHashCandidate, err := iterator.Get()
-		if err != nil {
-			return nil, err
-		}
-		highBlockGHOSTDAGData, err = sm.ghostdagDataStore.Get(sm.databaseContext, stagingArea, highHashCandidate)
-		if err != nil {
-			return nil, err
-		}
-		if highBlockGHOSTDAGData.BlueScore()-lowBlockGHOSTDAGData.BlueScore() > maxBlueScoreDifference {
-			break
-		}
-		highHash = highHashCandidate
-	}
-	return highHash, nil
-}
-
 func (sm *syncManager) findLowHashInHighHashSelectedParentChain(stagingArea *model.StagingArea,
 	lowHash *externalapi.DomainHash, highHash *externalapi.DomainHash) (*externalapi.DomainHash, error) {
 	for {

domain/consensus/processes/syncmanager/syncmanager.go

@@ -21,12 +21,15 @@ type syncManager struct {
 	blockStore                model.BlockStore
 	pruningStore              model.PruningStore
 	headersSelectedChainStore model.HeadersSelectedChainStore
+
+	mergeSetSizeLimit uint64
 }
 
 // New instantiates a new SyncManager
 func New(
 	databaseContext model.DBReader,
 	genesisBlockHash *externalapi.DomainHash,
+	mergeSetSizeLimit uint64,
 	dagTraversalManager model.DAGTraversalManager,
 	dagTopologyManager model.DAGTopologyManager,
 	ghostdagManager model.GHOSTDAGManager,
@@ -58,12 +61,12 @@ func New(
 }
 
 func (sm *syncManager) GetHashesBetween(stagingArea *model.StagingArea, lowHash, highHash *externalapi.DomainHash,
-	maxBlueScoreDifference uint64) (hashes []*externalapi.DomainHash, actualHighHash *externalapi.DomainHash, err error) {
+	maxBlocks uint64) (hashes []*externalapi.DomainHash, actualHighHash *externalapi.DomainHash, err error) {
 
 	onEnd := logger.LogAndMeasureExecutionTime(log, "GetHashesBetween")
 	defer onEnd()
 
-	return sm.antiPastHashesBetween(stagingArea, lowHash, highHash, maxBlueScoreDifference)
+	return sm.antiPastHashesBetween(stagingArea, lowHash, highHash, maxBlocks)
 }
 
 func (sm *syncManager) GetMissingBlockBodyHashes(stagingArea *model.StagingArea, highHash *externalapi.DomainHash) ([]*externalapi.DomainHash, error) {