[NOD-571] Cover ghostdag in tests where possible (#613)

* [NOD-571] Cover reachabilityInterval split methods.

* [NOD-571] Cover reindexInterval.

* [NOD-571] Cover reachability String() methods.

* [NOD-571] Cover blueAnticoneSize.

* [NOD-571] Remove unnecessary error from setTreeNode.

* [NOD-571] Add TestGHOSTDAGErrors.

* [NOD-571] Use PrepareBlockForTest in TestBlueAnticoneSizeErrors.

* [NOD-571] Use PrepareBlockForTest in TestGHOSTDAGErrors.

* [NOD-571] Add substring checks to TestSplitFractionErrors.

* [NOD-571] Add substring checks to TestSplitExactErrors and TestSplitWithExponentialBiasErrors.

* [NOD-571] Add comments to TestReindexIntervalErrors.

* [NOD-571] Add additional info in some error messages.

* [NOD-571] Fix error messages.

blockdag/ghostdag_test.go

@@ -3,9 +3,12 @@ package blockdag
 import (
 	"fmt"
 	"github.com/kaspanet/kaspad/dagconfig"
+	"github.com/kaspanet/kaspad/database"
 	"github.com/kaspanet/kaspad/util"
+	"github.com/kaspanet/kaspad/util/daghash"
 	"reflect"
 	"sort"
+	"strings"
 	"testing"
 )
 
@@ -276,3 +279,92 @@ func checkReds(expectedReds []string, reds map[string]bool) bool {
 	}
 	return true
 }
+
+func TestBlueAnticoneSizeErrors(t *testing.T) {
+	// Create a new database and DAG instance to run tests against.
+	dag, teardownFunc, err := DAGSetup("TestBlueAnticoneSizeErrors", Config{
+		DAGParams: &dagconfig.SimnetParams,
+	})
+	if err != nil {
+		t.Fatalf("TestBlueAnticoneSizeErrors: Failed to setup DAG instance: %s", err)
+	}
+	defer teardownFunc()
+
+	// Prepare a block chain with size K beginning with the genesis block
+	currentBlockA := dag.dagParams.GenesisBlock
+	for i := dagconfig.KType(0); i < dag.dagParams.K; i++ {
+		newBlock := prepareAndProcessBlock(t, dag, currentBlockA)
+		currentBlockA = newBlock
+	}
+
+	// Prepare another block chain with size K beginning with the genesis block
+	currentBlockB := dag.dagParams.GenesisBlock
+	for i := dagconfig.KType(0); i < dag.dagParams.K; i++ {
+		newBlock := prepareAndProcessBlock(t, dag, currentBlockB)
+		currentBlockB = newBlock
+	}
+
+	// Get references to the tips of the two chains
+	blockNodeA := dag.index.LookupNode(currentBlockA.BlockHash())
+	blockNodeB := dag.index.LookupNode(currentBlockB.BlockHash())
+
+	// Try getting the blueAnticoneSize between them. Since the two
+	// blocks are not in the anticones of eachother, this should fail.
+	_, err = dag.blueAnticoneSize(blockNodeA, blockNodeB)
+	if err == nil {
+		t.Fatalf("TestBlueAnticoneSizeErrors: blueAnticoneSize unexpectedly succeeded")
+	}
+	expectedErrSubstring := "is not in blue set of"
+	if !strings.Contains(err.Error(), expectedErrSubstring) {
+		t.Fatalf("TestBlueAnticoneSizeErrors: blueAnticoneSize returned wrong error. "+
+			"Want: %s, got: %s", expectedErrSubstring, err)
+	}
+}
+
+func TestGHOSTDAGErrors(t *testing.T) {
+	// Create a new database and DAG instance to run tests against.
+	dag, teardownFunc, err := DAGSetup("TestGHOSTDAGErrors", Config{
+		DAGParams: &dagconfig.SimnetParams,
+	})
+	if err != nil {
+		t.Fatalf("TestGHOSTDAGErrors: Failed to setup DAG instance: %s", err)
+	}
+	defer teardownFunc()
+
+	// Add two child blocks to the genesis
+	block1 := prepareAndProcessBlock(t, dag, dag.dagParams.GenesisBlock)
+	block2 := prepareAndProcessBlock(t, dag, dag.dagParams.GenesisBlock)
+
+	// Add a child block to the previous two blocks
+	block3 := prepareAndProcessBlock(t, dag, block1, block2)
+
+	// Clear the reachability store
+	dag.reachabilityStore.loaded = map[daghash.Hash]*reachabilityData{}
+	err = dag.db.Update(func(dbTx database.Tx) error {
+		bucket := dbTx.Metadata().Bucket(reachabilityDataBucketName)
+		cursor := bucket.Cursor()
+		for ok := cursor.First(); ok; ok = cursor.Next() {
+			err := bucket.Delete(cursor.Key())
+			if err != nil {
+				return err
+			}
+		}
+		return nil
+	})
+	if err != nil {
+		t.Fatalf("TestGHOSTDAGErrors: db.Update failed: %s", err)
+	}
+
+	// Try to rerun GHOSTDAG on the last block. GHOSTDAG uses
+	// reachability data, so we expect it to fail.
+	blockNode3 := dag.index.LookupNode(block3.BlockHash())
+	_, err = dag.ghostdag(blockNode3)
+	if err == nil {
+		t.Fatalf("TestGHOSTDAGErrors: ghostdag unexpectedly succeeded")
+	}
+	expectedErrSubstring := "Couldn't find reachability data"
+	if !strings.Contains(err.Error(), expectedErrSubstring) {
+		t.Fatalf("TestGHOSTDAGErrors: ghostdag returned wrong error. "+
+			"Want: %s, got: %s", expectedErrSubstring, err)
+	}
+}

blockdag/reachability.go

@@ -383,7 +383,7 @@ func (rtn *reachabilityTreeNode) String() string {
 		var current *reachabilityTreeNode
 		current, queue = queue[0], queue[1:]
 		if len(current.children) == 0 {
-			break
+			continue
 		}
 
 		line := ""
@@ -509,7 +509,8 @@ func (dag *BlockDAG) updateReachability(node *blockNode, selectedParentAnticone
 
 	// If this is the genesis node, simply initialize it and return
 	if node.isGenesis() {
-		return dag.reachabilityStore.setTreeNode(newTreeNode)
+		dag.reachabilityStore.setTreeNode(newTreeNode)
+		return nil
 	}
 
 	// Insert the node into the selected parent's reachability tree
@@ -522,10 +523,7 @@ func (dag *BlockDAG) updateReachability(node *blockNode, selectedParentAnticone
 		return err
 	}
 	for _, modifiedTreeNode := range modifiedTreeNodes {
-		err = dag.reachabilityStore.setTreeNode(modifiedTreeNode)
-		if err != nil {
-			return err
-		}
+		dag.reachabilityStore.setTreeNode(modifiedTreeNode)
 	}
 
 	// Add the block to the futureCoveringSets of all the blocks

blockdag/reachability_test.go

@@ -2,6 +2,7 @@ package blockdag
 
 import (
 	"reflect"
+	"strings"
 	"testing"
 )
 
@@ -322,6 +323,15 @@ func TestSplitWithExponentialBias(t *testing.T) {
 				newReachabilityInterval(41, 10_000),
 			},
 		},
+		{
+			interval: newReachabilityInterval(1, 100_000),
+			sizes:    []uint64{31_000, 31_000, 30_001},
+			expectedIntervals: []*reachabilityInterval{
+				newReachabilityInterval(1, 35_000),
+				newReachabilityInterval(35_001, 69_999),
+				newReachabilityInterval(70_000, 100_000),
+			},
+		},
 	}
 
 	for i, test := range tests {
@@ -473,3 +483,166 @@ func TestInsertBlock(t *testing.T) {
 		}
 	}
 }
+
+func TestSplitFractionErrors(t *testing.T) {
+	interval := newReachabilityInterval(100, 200)
+
+	// Negative fraction
+	_, _, err := interval.splitFraction(-0.5)
+	if err == nil {
+		t.Fatalf("TestSplitFractionErrors: splitFraction unexpectedly " +
+			"didn't return an error for a negative fraction")
+	}
+	expectedErrSubstring := "fraction must be between 0 and 1"
+	if !strings.Contains(err.Error(), expectedErrSubstring) {
+		t.Fatalf("TestSplitFractionErrors: splitFraction returned wrong error "+
+			"for a negative fraction. "+
+			"Want: %s, got: %s", expectedErrSubstring, err)
+	}
+
+	// Fraction > 1
+	_, _, err = interval.splitFraction(1.5)
+	if err == nil {
+		t.Fatalf("TestSplitFractionErrors: splitFraction unexpectedly " +
+			"didn't return an error for a fraction greater than 1")
+	}
+	expectedErrSubstring = "fraction must be between 0 and 1"
+	if !strings.Contains(err.Error(), expectedErrSubstring) {
+		t.Fatalf("TestSplitFractionErrors: splitFraction returned wrong error "+
+			"for a fraction greater than 1. "+
+			"Want: %s, got: %s", expectedErrSubstring, err)
+	}
+
+	// Splitting an empty interval
+	emptyInterval := newReachabilityInterval(1, 0)
+	_, _, err = emptyInterval.splitFraction(0.5)
+	if err == nil {
+		t.Fatalf("TestSplitFractionErrors: splitFraction unexpectedly " +
+			"didn't return an error for an empty interval")
+	}
+	expectedErrSubstring = "cannot split an empty interval"
+	if !strings.Contains(err.Error(), expectedErrSubstring) {
+		t.Fatalf("TestSplitFractionErrors: splitFraction returned wrong error "+
+			"for an empty interval. "+
+			"Want: %s, got: %s", expectedErrSubstring, err)
+	}
+}
+
+func TestSplitExactErrors(t *testing.T) {
+	interval := newReachabilityInterval(100, 199)
+
+	// Sum of sizes greater than the size of the interval
+	sizes := []uint64{50, 51}
+	_, err := interval.splitExact(sizes)
+	if err == nil {
+		t.Fatalf("TestSplitExactErrors: splitExact unexpectedly " +
+			"didn't return an error for (sum of sizes) > (size of interval)")
+	}
+	expectedErrSubstring := "sum of sizes must be equal to the interval's size"
+	if !strings.Contains(err.Error(), expectedErrSubstring) {
+		t.Fatalf("TestSplitExactErrors: splitExact returned wrong error "+
+			"for (sum of sizes) > (size of interval). "+
+			"Want: %s, got: %s", expectedErrSubstring, err)
+	}
+
+	// Sum of sizes smaller than the size of the interval
+	sizes = []uint64{50, 49}
+	_, err = interval.splitExact(sizes)
+	if err == nil {
+		t.Fatalf("TestSplitExactErrors: splitExact unexpectedly " +
+			"didn't return an error for (sum of sizes) < (size of interval)")
+	}
+	expectedErrSubstring = "sum of sizes must be equal to the interval's size"
+	if !strings.Contains(err.Error(), expectedErrSubstring) {
+		t.Fatalf("TestSplitExactErrors: splitExact returned wrong error "+
+			"for (sum of sizes) < (size of interval). "+
+			"Want: %s, got: %s", expectedErrSubstring, err)
+	}
+}
+
+func TestSplitWithExponentialBiasErrors(t *testing.T) {
+	interval := newReachabilityInterval(100, 199)
+
+	// Sum of sizes greater than the size of the interval
+	sizes := []uint64{50, 51}
+	_, err := interval.splitWithExponentialBias(sizes)
+	if err == nil {
+		t.Fatalf("TestSplitWithExponentialBiasErrors: splitWithExponentialBias " +
+			"unexpectedly didn't return an error")
+	}
+	expectedErrSubstring := "sum of sizes must be less than or equal to the interval's size"
+	if !strings.Contains(err.Error(), expectedErrSubstring) {
+		t.Fatalf("TestSplitWithExponentialBiasErrors: splitWithExponentialBias "+
+			"returned wrong error. Want: %s, got: %s", expectedErrSubstring, err)
+	}
+}
+
+func TestReindexIntervalErrors(t *testing.T) {
+	// Create a treeNode and give it size = 100
+	treeNode := newReachabilityTreeNode(&blockNode{})
+	treeNode.setInterval(newReachabilityInterval(0, 99))
+
+	// Add a chain of 100 child treeNodes to treeNode
+	var err error
+	currentTreeNode := treeNode
+	for i := 0; i < 100; i++ {
+		childTreeNode := newReachabilityTreeNode(&blockNode{})
+		_, err = currentTreeNode.addChild(childTreeNode)
+		if err != nil {
+			break
+		}
+		currentTreeNode = childTreeNode
+	}
+
+	// At the 100th addChild we expect a reindex. This reindex should
+	// fail because our initial treeNode only has size = 100, and the
+	// reindex requires size > 100.
+	// This simulates the case when (somehow) there's more than 2^64
+	// blocks in the DAG, since the genesis block has size = 2^64.
+	if err == nil {
+		t.Fatalf("TestReindexIntervalErrors: reindexIntervals " +
+			"unexpectedly didn't return an error")
+	}
+	if !strings.Contains(err.Error(), "missing tree parent during reindexing") {
+		t.Fatalf("TestReindexIntervalErrors: reindexIntervals "+
+			"returned an expected error: %s", err)
+	}
+}
+
+func TestFutureCoveringBlockSetString(t *testing.T) {
+	treeNodeA := newReachabilityTreeNode(&blockNode{})
+	treeNodeA.setInterval(newReachabilityInterval(123, 456))
+	treeNodeB := newReachabilityTreeNode(&blockNode{})
+	treeNodeB.setInterval(newReachabilityInterval(457, 789))
+	futureCoveringSet := futureCoveringBlockSet{
+		&futureCoveringBlock{treeNode: treeNodeA},
+		&futureCoveringBlock{treeNode: treeNodeB},
+	}
+
+	str := futureCoveringSet.String()
+	expectedStr := "[123,456][457,789]"
+	if str != expectedStr {
+		t.Fatalf("TestFutureCoveringBlockSetString: unexpected "+
+			"string. Want: %s, got: %s", expectedStr, str)
+	}
+}
+
+func TestReachabilityTreeNodeString(t *testing.T) {
+	treeNodeA := newReachabilityTreeNode(&blockNode{})
+	treeNodeA.setInterval(newReachabilityInterval(100, 199))
+	treeNodeB1 := newReachabilityTreeNode(&blockNode{})
+	treeNodeB1.setInterval(newReachabilityInterval(100, 150))
+	treeNodeB2 := newReachabilityTreeNode(&blockNode{})
+	treeNodeB2.setInterval(newReachabilityInterval(150, 199))
+	treeNodeC := newReachabilityTreeNode(&blockNode{})
+	treeNodeC.setInterval(newReachabilityInterval(100, 149))
+	treeNodeA.children = []*reachabilityTreeNode{treeNodeB1, treeNodeB2}
+	treeNodeB2.children = []*reachabilityTreeNode{treeNodeC}
+
+	str := treeNodeA.String()
+	expectedStr := "[100,149]\n[100,150][150,199]\n[100,199]"
+	if str != expectedStr {
+		t.Fatalf("TestReachabilityTreeNodeString: unexpected "+
+			"string. Want: %s, got: %s", expectedStr, str)
+	}
+}

blockdag/reachabilitystore.go

@@ -28,7 +28,7 @@ func newReachabilityStore(dag *BlockDAG) *reachabilityStore {
 	}
 }
 
-func (store *reachabilityStore) setTreeNode(treeNode *reachabilityTreeNode) error {
+func (store *reachabilityStore) setTreeNode(treeNode *reachabilityTreeNode) {
 	// load the reachability data from DB to store.loaded
 	node := treeNode.blockNode
 	_, exists := store.reachabilityDataByHash(node.hash)
@@ -38,7 +38,6 @@ func (store *reachabilityStore) setTreeNode(treeNode *reachabilityTreeNode) erro
 
 	store.loaded[*node.hash].treeNode = treeNode
 	store.setBlockAsDirty(node.hash)
-	return nil
 }
 
 func (store *reachabilityStore) setFutureCoveringSet(node *blockNode, futureCoveringSet futureCoveringBlockSet) error {