From c9dadfef1b035ad3fe5302a48aabc542abcd0fb9 Mon Sep 17 00:00:00 2001 From: Ori Newman Date: Sun, 30 Sep 2018 13:30:55 +0300 Subject: [PATCH] [DEV-166] fix endless loop in validate parents (#73) * [DEV-66] fix endless loop in validateParents * [DEV-166] add TestValidateParents * [DEV-166] use generateNode in TestValidateParents to avoid repetition * [DEV-166] use blockDAG.genesis instead of blockDAG.virtual.SelectedTip() --- blockdag/phantom_test.go | 2 +- blockdag/validate.go | 4 ++-- blockdag/validate_test.go | 42 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 45 insertions(+), 3 deletions(-) diff --git a/blockdag/phantom_test.go b/blockdag/phantom_test.go index a2c30c6d9..dca1a7a48 100644 --- a/blockdag/phantom_test.go +++ b/blockdag/phantom_test.go @@ -810,7 +810,7 @@ func TestPhantom(t *testing.T) { netParams.K = test.k // Generate enough synthetic blocks for the rest of the test blockDAG := newTestDAG(&netParams) - genesisNode := blockDAG.virtual.SelectedTip() + genesisNode := blockDAG.genesis blockTime := genesisNode.Header().Timestamp blockByIDMap := make(map[string]*blockNode) idByBlockMap := make(map[*blockNode]string) diff --git a/blockdag/validate.go b/blockdag/validate.go index aada391be..b398160e2 100644 --- a/blockdag/validate.go +++ b/blockdag/validate.go @@ -705,8 +705,8 @@ func validateParents(blockHeader *wire.BlockHeader, parents blockSet) error { if current.height > minHeight { for _, parent := range current.parents { if !visited.contains(parent) { - queue.Push(current) - visited.add(current) + queue.Push(parent) + visited.add(parent) } } } diff --git a/blockdag/validate_test.go b/blockdag/validate_test.go index 554636e72..0bbeb6bb2 100644 --- a/blockdag/validate_test.go +++ b/blockdag/validate_test.go @@ -499,6 +499,48 @@ func TestCheckSerializedHeight(t *testing.T) { } } +func TestValidateParents(t *testing.T) { + blockDAG := newTestDAG(&dagconfig.MainNetParams) + genesisNode := blockDAG.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(setFromSlice(parents...), + blockVersion, + 0, + blockTime, + dagconfig.MainNetParams.K) + } + + a := generateNode(genesisNode) + b := generateNode(a) + c := generateNode(genesisNode) + + fakeBlockHeader := &wire.BlockHeader{} + + // Check direct parents relation + err := validateParents(fakeBlockHeader, setFromSlice(a, b)) + if err == nil { + t.Errorf("validateParents: `a` is a parent of `b`, so an error is expected") + } + + // Check indirect parents relation + err = validateParents(fakeBlockHeader, setFromSlice(genesisNode, b)) + 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, setFromSlice(b, c)) + if err != nil { + t.Errorf("validateParents: unexpected error: %v", err) + } +} + // Block100000 defines block 100,000 of the block chain. It is used to // test Block operations. var Block100000 = wire.MsgBlock{