Change sirtual parents selection to allow faster branch merges in the network (#1315)

* if more candidates then max, choose half with highest blueWork and half with lowest

* Add a Test GhostDAG sorter

* Add a test for pick virtual parents

* Fix review nits

domain/consensus/processes/consensusstatemanager/pick_virtual_parents.go

@@ -32,11 +32,23 @@ func (csm *consensusStateManager) pickVirtualParents(tips []*externalapi.DomainH
 	log.Debugf("The selected parent of the virtual is: %s", virtualSelectedParent)
 
 	selectedVirtualParents := hashset.NewFromSlice(virtualSelectedParent)
+	candidates := candidatesHeap.ToSlice()
+	// prioritize half the blocks with highest blueWork and half with lowest, so the network will merge splits faster.
+	if len(candidates) >= int(csm.maxBlockParents) {
+		// We already have the selectedParent, so we're left with csm.maxBlockParents-1.
+		maxParents := csm.maxBlockParents - 1
+		end := len(candidates) - 1
+		for i := (maxParents) / 2; i < maxParents; i++ {
+			candidates[i], candidates[end] = candidates[end], candidates[i]
+			end--
+		}
+	}
 
 	mergeSetSize := uint64(1) // starts counting from 1 because selectedParent is already in the mergeSet
 
-	for candidatesHeap.Len() > 0 && uint64(len(selectedVirtualParents)) < uint64(csm.maxBlockParents) {
-		candidate := candidatesHeap.Pop()
+	for len(candidates) > 0 && uint64(len(selectedVirtualParents)) < uint64(csm.maxBlockParents) {
+		candidate := candidates[0]
+		candidates = candidates[1:]
 
 		log.Debugf("Attempting to add %s to the virtual parents", candidate)
 		log.Debugf("The current merge set size is %d", mergeSetSize)

domain/consensus/processes/consensusstatemanager/virtual_parents_test.go

@@ -0,0 +1,110 @@
+package consensusstatemanager_test
+
+import (
+	"github.com/kaspanet/kaspad/domain/consensus"
+	"github.com/kaspanet/kaspad/domain/consensus/model"
+	"github.com/kaspanet/kaspad/domain/consensus/model/externalapi"
+	"github.com/kaspanet/kaspad/domain/consensus/model/testapi"
+	"github.com/kaspanet/kaspad/domain/consensus/utils/consensushashing"
+	"github.com/kaspanet/kaspad/domain/consensus/utils/testutils"
+	"github.com/kaspanet/kaspad/domain/dagconfig"
+	"sort"
+	"testing"
+)
+
+func TestConsensusStateManager_pickVirtualParents(t *testing.T) {
+	testutils.ForAllNets(t, true, func(t *testing.T, params *dagconfig.Params) {
+		tc, teardown, err := consensus.NewFactory().NewTestConsensus(params, "TestConsensusStateManager_pickVirtualParents")
+		if err != nil {
+			t.Fatalf("Error setting up tc: %+v", err)
+		}
+		defer teardown(false)
+
+		getSortedVirtualParents := func(tc testapi.TestConsensus) []*externalapi.DomainHash {
+			virtualRelations, err := tc.BlockRelationStore().BlockRelation(tc.DatabaseContext(), model.VirtualBlockHash)
+			if err != nil {
+				t.Fatalf("Failed getting virtual block virtualRelations: %v", err)
+			}
+
+			block, err := tc.BuildBlock(&externalapi.DomainCoinbaseData{}, nil)
+			if err != nil {
+				t.Fatalf("Consensus failed building a block: %v", err)
+			}
+			blockParents := block.Header.ParentHashes()
+			sort.Sort(consensus.NewTestGhostDAGSorter(virtualRelations.Parents, tc, t))
+			sort.Sort(consensus.NewTestGhostDAGSorter(blockParents, tc, t))
+			if !externalapi.HashesEqual(virtualRelations.Parents, blockParents) {
+				t.Fatalf("Block relations and BuildBlock return different parents for virtual, %s != %s", virtualRelations.Parents, blockParents)
+			}
+			return virtualRelations.Parents
+		}
+
+		// We build 2*params.MaxBlockParents each one with blueWork higher than the other.
+		parents := make([]*externalapi.DomainHash, 0, params.MaxBlockParents)
+		for i := 0; i < 2*int(params.MaxBlockParents); i++ {
+			lastBlock := params.GenesisHash
+			for j := 0; j <= i; j++ {
+				lastBlock, _, err = tc.AddBlock([]*externalapi.DomainHash{lastBlock}, nil, nil)
+				if err != nil {
+					t.Fatalf("Failed Adding block to tc: %v", err)
+				}
+			}
+			parents = append(parents, lastBlock)
+		}
+
+		virtualParents := getSortedVirtualParents(tc)
+		sort.Sort(consensus.NewTestGhostDAGSorter(parents, tc, t))
+
+		// Make sure the first half of the blocks are with highest blueWork
+		// we use (max+1)/2 because the first "half" is rounded up, so `(dividend + (divisor - 1)) / divisor` = `(max + (2-1))/2` = `(max+1)/2`
+		for i := 0; i < int(params.MaxBlockParents+1)/2; i++ {
+			if !virtualParents[i].Equal(parents[i]) {
+				t.Fatalf("Expected block at %d to be equal, instead found %s != %s", i, virtualParents[i], parents[i])
+			}
+		}
+
+		// Make sure the second half is the candidates with lowest blueWork
+		end := len(parents) - int(params.MaxBlockParents)/2
+		for i := (params.MaxBlockParents + 1) / 2; i < params.MaxBlockParents; i++ {
+			if !virtualParents[i].Equal(parents[end]) {
+				t.Fatalf("Expected block at %d to be equal, instead found %s != %s", i, virtualParents[i], parents[end])
+			}
+			end++
+		}
+		if end != len(parents) {
+			t.Fatalf("Expected %d==%d", end, len(parents))
+		}
+
+		// Clear all tips.
+		var virtualSelectedParent *externalapi.DomainHash
+		for {
+			block, err := tc.BuildBlock(&externalapi.DomainCoinbaseData{}, nil)
+			if err != nil {
+				t.Fatalf("Failed building a block: %v", err)
+			}
+			_, err = tc.ValidateAndInsertBlock(block)
+			if err != nil {
+				t.Fatalf("Failed Inserting block to tc: %v", err)
+			}
+			virtualSelectedParent = consensushashing.BlockHash(block)
+			if len(block.Header.ParentHashes()) == 1 {
+				break
+			}
+		}
+		// build exactly params.MaxBlockParents
+		parents = make([]*externalapi.DomainHash, 0, params.MaxBlockParents)
+		for i := 0; i < int(params.MaxBlockParents); i++ {
+			block, _, err := tc.AddBlock([]*externalapi.DomainHash{virtualSelectedParent}, nil, nil)
+			if err != nil {
+				t.Fatalf("Failed Adding block to tc: %v", err)
+			}
+			parents = append(parents, block)
+		}
+
+		sort.Sort(consensus.NewTestGhostDAGSorter(parents, tc, t))
+		virtualParents = getSortedVirtualParents(tc)
+		if !externalapi.HashesEqual(virtualParents, parents) {
+			t.Fatalf("Expected VirtualParents and parents to be equal, instead: %s != %s", virtualParents, parents)
+		}
+	})
+}

domain/consensus/processes/dagtraversalmanager/window_test.go

@@ -1,7 +1,6 @@
 package dagtraversalmanager_test
 
 import (
-	"github.com/kaspanet/kaspad/domain/consensus/model/testapi"
 	"reflect"
 	"sort"
 	"testing"
@@ -345,7 +344,7 @@ func TestBlueBlockWindow(t *testing.T) {
 			if err != nil {
 				t.Fatalf("BlueWindow: %s", err)
 			}
-			sortWindow(t, tc, window)
+			sort.Sort(consensus.NewTestGhostDAGSorter(window, tc, t))
 			if err := checkWindowIDs(window, blockData.expectedWindowWithGenesisPadding, idByBlockMap); err != nil {
 				t.Errorf("Unexpected values for window for block %s: %s", blockData.id, err)
 			}
@@ -353,20 +352,6 @@ func TestBlueBlockWindow(t *testing.T) {
 	})
 }
 
-func sortWindow(t *testing.T, tc testapi.TestConsensus, window []*externalapi.DomainHash) {
-	sort.Slice(window, func(i, j int) bool {
-		ghostdagDataI, err := tc.GHOSTDAGDataStore().Get(tc.DatabaseContext(), window[i])
-		if err != nil {
-			t.Fatalf("Failed getting ghostdag data for %s", err)
-		}
-		ghostdagDataJ, err := tc.GHOSTDAGDataStore().Get(tc.DatabaseContext(), window[j])
-		if err != nil {
-			t.Fatalf("Failed getting ghostdag data for %s", err)
-		}
-		return !tc.GHOSTDAGManager().Less(window[i], ghostdagDataI, window[j], ghostdagDataJ)
-	})
-}
-
 func checkWindowIDs(window []*externalapi.DomainHash, expectedIDs []string, idByBlockMap map[externalapi.DomainHash]string) error {
 	ids := make([]string, len(window))
 	for i, node := range window {

domain/consensus/test_ghostdag_sorter.go

@@ -0,0 +1,43 @@
+package consensus
+
+import (
+	"github.com/kaspanet/kaspad/domain/consensus/model/externalapi"
+	"github.com/kaspanet/kaspad/domain/consensus/model/testapi"
+	"sort"
+	"testing"
+)
+
+type testGhostDAGSorter struct {
+	slice []*externalapi.DomainHash
+	tc    testapi.TestConsensus
+	test  testing.TB
+}
+
+// NewTestGhostDAGSorter returns a sort.Interface over the slice, so you can sort it via GhostDAG ordering
+func NewTestGhostDAGSorter(slice []*externalapi.DomainHash, tc testapi.TestConsensus, t testing.TB) sort.Interface {
+	return testGhostDAGSorter{
+		slice: slice,
+		tc:    tc,
+		test:  t,
+	}
+}
+
+func (sorter testGhostDAGSorter) Len() int {
+	return len(sorter.slice)
+}
+
+func (sorter testGhostDAGSorter) Less(i, j int) bool {
+	ghostdagDataI, err := sorter.tc.GHOSTDAGDataStore().Get(sorter.tc.DatabaseContext(), sorter.slice[i])
+	if err != nil {
+		sorter.test.Fatalf("TestGhostDAGSorter: Failed getting ghostdag data for %s", err)
+	}
+	ghostdagDataJ, err := sorter.tc.GHOSTDAGDataStore().Get(sorter.tc.DatabaseContext(), sorter.slice[j])
+	if err != nil {
+		sorter.test.Fatalf("TestGhostDAGSorter: Failed getting ghostdag data for %s", err)
+	}
+	return !sorter.tc.GHOSTDAGManager().Less(sorter.slice[i], ghostdagDataI, sorter.slice[j], ghostdagDataJ)
+}
+
+func (sorter testGhostDAGSorter) Swap(i, j int) {
+	sorter.slice[i], sorter.slice[j] = sorter.slice[j], sorter.slice[i]
+}