[NOD-603] Update validateParents to use reachability (#640)

* [NOD-603] Update validateParents to use reachability

* [NOD-603] Break a long line

* [NOD-721] Remove redundant check if block parent is a tip

blockdag/validate.go

@@ -598,41 +598,32 @@ func (dag *BlockDAG) validateDifficulty(header *wire.BlockHeader, bluestParent *
 }
 
 // validateParents validates that no parent is an ancestor of another parent, and no parent is finalized
-func validateParents(blockHeader *wire.BlockHeader, parents blockSet) error {
-	minBlueScore := uint64(math.MaxUint64)
-	queue := newDownHeap()
-	visited := newBlockSet()
-	for parent := range parents {
+func (dag *BlockDAG) validateParents(blockHeader *wire.BlockHeader, parents blockSet) error {
+	for parentA := range parents {
 		// isFinalized might be false-negative because node finality status is
 		// updated in a separate goroutine. This is why later the block is
 		// checked more thoroughly on the finality rules in dag.checkFinalityRules.
-		if parent.isFinalized {
-			return ruleError(ErrFinality, fmt.Sprintf("block %s is a finalized parent of block %s", parent.hash, blockHeader.BlockHash()))
+		if parentA.isFinalized {
+			return ruleError(ErrFinality, fmt.Sprintf("block %s is a finalized "+
+				"parent of block %s", parentA.hash, blockHeader.BlockHash()))
 		}
-		if parent.blueScore < minBlueScore {
-			minBlueScore = parent.blueScore
-		}
-		for grandParent := range parent.parents {
-			if !visited.contains(grandParent) {
-				queue.Push(grandParent)
-				visited.add(grandParent)
+
+		for parentB := range parents {
+			if parentA == parentB {
+				continue
 			}
-		}
-	}
-	for queue.Len() > 0 {
-		current := queue.pop()
-		if parents.contains(current) {
-			return ruleError(ErrInvalidParentsRelation, fmt.Sprintf("block %s is both a parent of %s and an"+
-				" ancestor of another parent",
-				current.hash,
-				blockHeader.BlockHash()))
-		}
-		if current.blueScore > minBlueScore {
-			for parent := range current.parents {
-				if !visited.contains(parent) {
-					queue.Push(parent)
-					visited.add(parent)
-				}
+
+			isAncestorOf, err := dag.isAncestorOf(parentA, parentB)
+			if err != nil {
+				return err
+			}
+			if isAncestorOf {
+				return ruleError(ErrInvalidParentsRelation, fmt.Sprintf("block %s is both a parent of %s and an"+
+					" ancestor of another parent %s",
+					parentA.hash,
+					blockHeader.BlockHash(),
+					parentB.hash,
+				))
 			}
 		}
 	}
@@ -654,7 +645,7 @@ func (dag *BlockDAG) checkBlockContext(block *util.Block, parents blockSet, flag
 	bluestParent := parents.bluest()
 	fastAdd := flags&BFFastAdd == BFFastAdd
 
-	err := validateParents(&block.MsgBlock().Header, parents)
+	err := dag.validateParents(&block.MsgBlock().Header, parents)
 	if err != nil {
 		return err
 	}

blockdag/validate_test.go

@@ -558,24 +558,23 @@ func TestPastMedianTime(t *testing.T) {
 }
 
 func TestValidateParents(t *testing.T) {
-	dag := newTestDAG(&dagconfig.SimnetParams)
-	genesisNode := dag.genesis
-	blockVersion := int32(0x10000000)
-
-	blockTime := genesisNode.Header().Timestamp
-
-	generateNode := func(parents ...*blockNode) *blockNode {
-		// The timestamp of each block is changed to prevent a situation where two blocks share the same hash
-		blockTime = blockTime.Add(time.Second)
-		return newTestNode(dag, blockSetFromSlice(parents...),
-			blockVersion,
-			0,
-			blockTime)
+	// Create a new database and dag instance to run tests against.
+	dag, teardownFunc, err := DAGSetup("TestCheckBlockSanity", Config{
+		DAGParams: &dagconfig.SimnetParams,
+	})
+	if err != nil {
+		t.Errorf("Failed to setup dag instance: %v", err)
+		return
 	}
+	defer teardownFunc()
 
-	a := generateNode(genesisNode)
-	b := generateNode(a)
-	c := generateNode(genesisNode)
+	a := prepareAndProcessBlock(t, dag, dag.dagParams.GenesisBlock)
+	b := prepareAndProcessBlock(t, dag, a)
+	c := prepareAndProcessBlock(t, dag, dag.dagParams.GenesisBlock)
+
+	aNode := nodeByMsgBlock(t, dag, a)
+	bNode := nodeByMsgBlock(t, dag, b)
+	cNode := nodeByMsgBlock(t, dag, c)
 
 	fakeBlockHeader := &wire.BlockHeader{
 		HashMerkleRoot:       &daghash.ZeroHash,
@@ -584,19 +583,19 @@ func TestValidateParents(t *testing.T) {
 	}
 
 	// Check direct parents relation
-	err := validateParents(fakeBlockHeader, blockSetFromSlice(a, b))
+	err = dag.validateParents(fakeBlockHeader, blockSetFromSlice(aNode, bNode))
 	if err == nil {
 		t.Errorf("validateParents: `a` is a parent of `b`, so an error is expected")
 	}
 
 	// Check indirect parents relation
-	err = validateParents(fakeBlockHeader, blockSetFromSlice(genesisNode, b))
+	err = dag.validateParents(fakeBlockHeader, blockSetFromSlice(dag.genesis, bNode))
 	if err == nil {
 		t.Errorf("validateParents: `genesis` and `b` are indirectly related, so an error is expected")
 	}
 
 	// Check parents with no relation
-	err = validateParents(fakeBlockHeader, blockSetFromSlice(b, c))
+	err = dag.validateParents(fakeBlockHeader, blockSetFromSlice(bNode, cNode))
 	if err != nil {
 		t.Errorf("validateParents: unexpected error: %v", err)
 	}