[DEV-107] Disallow situation where a parent is also an ancestor of another parent

* [DEV-107] Disallow situation where a parent is also an ancestor of another parent

* [DEV-107] Add comment for validateParents

* [DEV-107] move validateParents to checkBlockContext

* [DEV-107] break validateParents error to 2 lines

* [DEV-107] remove TestProcessBlock

* [DEV-107] fix comment that explains block 3c test

* [DEV-107] remove blk_3C from TestCheckConnectBlockTemplate

blockdag/accept.go

@@ -34,7 +34,7 @@ func (dag *BlockDAG) maybeAcceptBlock(block *util.Block, flags BehaviorFlags) er
 
 	// The block must pass all of the validation rules which depend on the
 	// position of the block within the block DAG.
-	err = dag.checkBlockContext(block, selectedParent, flags)
+	err = dag.checkBlockContext(block, parents, selectedParent, flags)
 	if err != nil {
 		return err
 	}

blockdag/dag_test.go

@@ -13,8 +13,8 @@ import (
 
 	"github.com/daglabs/btcd/dagconfig"
 	"github.com/daglabs/btcd/dagconfig/daghash"
-	"github.com/daglabs/btcd/wire"
 	"github.com/daglabs/btcd/util"
+	"github.com/daglabs/btcd/wire"
 )
 
 // TestHaveBlock tests the HaveBlock API to ensure proper functionality.
@@ -63,8 +63,33 @@ func TestHaveBlock(t *testing.T) {
 		}
 	}
 
+	testFiles = []string{
+		"blk_3C.dat",
+	}
+
+	for _, file := range testFiles {
+		blockTmp, err := loadBlocks(file)
+		if err != nil {
+			t.Errorf("Error loading file: %v\n", err)
+			return
+		}
+		blocks = append(blocks, blockTmp...)
+	}
+	isOrphan, err := chain.ProcessBlock(blocks[6], BFNone)
+
+	// Block 3c should fail to connect since its parents are related. (It points to 1 and 2, and 1 is the parent of 2)
+	if err == nil {
+		t.Errorf("ProcessBlock for block 3c has no error when expected to have an error\n")
+		return
+	}
+	if isOrphan {
+		t.Errorf("ProcessBlock incorrectly returned block 3c " +
+			"is an orphan\n")
+		return
+	}
+
 	// Insert an orphan block.
-	isOrphan, err := chain.ProcessBlock(util.NewBlock(&Block100000),
+	isOrphan, err = chain.ProcessBlock(util.NewBlock(&Block100000),
 		BFNone)
 	if err != nil {
 		t.Errorf("Unable to process block: %v", err)
@@ -84,7 +109,7 @@ func TestHaveBlock(t *testing.T) {
 		{hash: dagconfig.MainNetParams.GenesisHash.String(), want: true},
 
 		// Block 3b should be present (as a second child of Block 2).
-		{hash: "000000bce70562ed076f269c5c4e39c590abb29428c573c02ab970e17931f8a4", want: true},
+		{hash: "00000093c8f2ab3444502da0754fc8149d738701aef9b2e0f32f32c078039295", want: true},
 
 		// Block 100000 should be present (as an orphan).
 		{hash: "000000a805b083e0ef1f516b1153828724c235d6e6f0fabb47b869f6d054ac3f", want: true},

blockdag/validate.go

@@ -15,8 +15,8 @@ import (
 	"github.com/daglabs/btcd/dagconfig"
 	"github.com/daglabs/btcd/dagconfig/daghash"
 	"github.com/daglabs/btcd/txscript"
-	"github.com/daglabs/btcd/wire"
 	"github.com/daglabs/btcd/util"
+	"github.com/daglabs/btcd/wire"
 )
 
 const (
@@ -699,6 +699,42 @@ func (dag *BlockDAG) checkBlockHeaderContext(header *wire.BlockHeader, selectedP
 	return nil
 }
 
+// validateParents validates that no parent is an ancestor of another parent
+func validateParents(blockHeader *wire.BlockHeader, parents blockSet) error {
+	minHeight := int32(math.MaxInt32)
+	queue := NewHeap()
+	visited := newSet()
+	for _, parent := range parents {
+		if parent.height < minHeight {
+			minHeight = parent.height
+		}
+		for _, grandParent := range parent.parents {
+			if !visited.contains(grandParent) {
+				queue.Push(grandParent)
+				visited.add(grandParent)
+			}
+		}
+	}
+	for queue.Len() > 0 {
+		current := queue.Pop()
+		if parents.contains(current) {
+			return fmt.Errorf("Block %s is both a parent of %s and an"+
+				" ancestor of another parent",
+				current.hash,
+				blockHeader.BlockHash())
+		}
+		if current.height > minHeight {
+			for _, parent := range current.parents {
+				if !visited.contains(parent) {
+					queue.Push(current)
+					visited.add(current)
+				}
+			}
+		}
+	}
+	return nil
+}
+
 // checkBlockContext peforms several validation checks on the block which depend
 // on its position within the block chain.
 //
@@ -710,10 +746,14 @@ func (dag *BlockDAG) checkBlockHeaderContext(header *wire.BlockHeader, selectedP
 // for how the flags modify its behavior.
 //
 // This function MUST be called with the chain state lock held (for writes).
-func (dag *BlockDAG) checkBlockContext(block *util.Block, selectedParent *blockNode, flags BehaviorFlags) error {
+func (dag *BlockDAG) checkBlockContext(block *util.Block, parents blockSet, selectedParent *blockNode, flags BehaviorFlags) error {
+	err := validateParents(&block.MsgBlock().Header, parents)
+	if err != nil {
+		return err
+	}
 	// Perform all block header related validation checks.
 	header := &block.MsgBlock().Header
-	err := dag.checkBlockHeaderContext(header, selectedParent, flags)
+	err = dag.checkBlockHeaderContext(header, selectedParent, flags)
 	if err != nil {
 		return err
 	}
@@ -1130,7 +1170,12 @@ func (dag *BlockDAG) CheckConnectBlockTemplate(block *util.Block) error {
 		return err
 	}
 
-	err = dag.checkBlockContext(block, dag.virtual.SelectedTip(), flags)
+	parents, err := lookupPreviousNodes(block, dag)
+	if err != nil {
+		return err
+	}
+
+	err = dag.checkBlockContext(block, parents, dag.virtual.SelectedTip(), flags)
 	if err != nil {
 		return err
 	}

blockdag/validate_test.go

@@ -12,8 +12,8 @@ import (
 
 	"github.com/daglabs/btcd/dagconfig"
 	"github.com/daglabs/btcd/dagconfig/daghash"
-	"github.com/daglabs/btcd/wire"
 	"github.com/daglabs/btcd/util"
+	"github.com/daglabs/btcd/wire"
 )
 
 // TestSequenceLocksActive tests the SequenceLockActive function to ensure it