fix(lib/blocktree): removes the inconsistency to choose a deepest leaf (

ChainSafe#2094)

lib/blocktree/blocktree_test.go

@@ -58,6 +58,7 @@ func createTestBlockTree(t *testing.T, header *types.Header, number int) (*Block
 		hash := header.Hash()
 		err := bt.AddBlock(header, time.Unix(0, at))
 		require.NoError(t, err)
+
 		previousHash = hash
 
 		isBranch := r.Intn(2)
@@ -88,8 +89,10 @@ func createTestBlockTree(t *testing.T, header *types.Header, number int) (*Block
 			hash := header.Hash()
 			err := bt.AddBlock(header, time.Unix(0, at))
 			require.NoError(t, err)
+
 			previousHash = hash
 			at += int64(r.Intn(8))
+
 		}
 	}
 
@@ -474,6 +477,111 @@ func TestBlockTree_GetHashByNumber(t *testing.T) {
 	require.Error(t, err)
 }
 
+func TestBlockTree_AllLeavesHasSameNumberAndArrivalTime_DeepestBlockHash_ShouldHasConsistentOutput(t *testing.T) {
+	bt := NewBlockTreeFromRoot(testHeader)
+	previousHash := testHeader.Hash()
+
+	branches := []testBranch{}
+
+	const fixedArrivalTime = 99
+	const deep = 8
+
+	// create a base tree with a fixed amount of blocks
+	// and all block with the same arrival time
+
+	/**
+	base tree and nodes representation, all with the same arrival time and all
+	the leaves has the same number (8) the numbers in the right represents the order
+	the nodes are inserted into the blocktree.
+
+	a -> b -> c -> d -> e -> f -> g -> h (1)
+		|    |    |    |    |    |> h (7)
+		|    |    |    |    |> g -> h (6)
+		|    |    |    |> f -> g -> h (5)
+		|    |    |> e -> f -> g -> h (4)
+		|    |> d -> e -> f -> g -> h (3)
+		|> c -> d -> e -> f -> g -> h (2)
+	**/
+
+	for i := 1; i <= deep; i++ {
+		header := &types.Header{
+			ParentHash: previousHash,
+			Number:     big.NewInt(int64(i)),
+			Digest:     types.NewDigest(),
+		}
+
+		hash := header.Hash()
+
+		err := bt.AddBlock(header, time.Unix(0, fixedArrivalTime))
+		require.NoError(t, err)
+
+		previousHash = hash
+
+		// the last block on the base tree should not generates a branch
+		if i < deep {
+			branches = append(branches, testBranch{
+				hash:   hash,
+				number: bt.getNode(hash).number,
+			})
+		}
+	}
+
+	// create all the branch nodes with the same arrival time
+	for _, branch := range branches {
+		previousHash = branch.hash
+
+		for i := int(branch.number.Uint64()); i < deep; i++ {
+			header := &types.Header{
+				ParentHash: previousHash,
+				Number:     big.NewInt(int64(i) + 1),
+				StateRoot:  common.Hash{0x1},
+				Digest:     types.NewDigest(),
+			}
+
+			hash := header.Hash()
+			err := bt.AddBlock(header, time.Unix(0, fixedArrivalTime))
+			require.NoError(t, err)
+
+			previousHash = hash
+		}
+	}
+
+	// check all leaves has the same number and timestamps
+	leaves := bt.leaves.nodes()
+	for idx := 0; idx < len(leaves)-2; idx++ {
+		curr := leaves[idx]
+		next := leaves[idx+1]
+
+		require.Equal(t, curr.number, next.number)
+		require.Equal(t, curr.arrivalTime, next.arrivalTime)
+	}
+
+	require.Len(t, leaves, deep)
+
+	// expects currentDeepestLeaf nil till call deepestLeaf() function
+	require.Nil(t, bt.leaves.currentDeepestLeaf)
+	deepestLeaf := bt.deepestLeaf()
+
+	require.Equal(t, deepestLeaf, bt.leaves.currentDeepestLeaf)
+	require.Contains(t, leaves, deepestLeaf)
+
+	// adding a new node with a greater number, should update the currentDeepestLeaf
+	header := &types.Header{
+		ParentHash: previousHash,
+		Number:     big.NewInt(int64(deepestLeaf.number.Uint64() + 1)),
+		StateRoot:  common.Hash{0x1},
+		Digest:     types.NewDigest(),
+	}
+
+	hash := header.Hash()
+	err := bt.AddBlock(header, time.Unix(0, fixedArrivalTime))
+	require.NoError(t, err)
+
+	deepestLeaf = bt.deepestLeaf()
+	require.Equal(t, hash, deepestLeaf.hash)
+	require.Equal(t, hash, bt.leaves.currentDeepestLeaf.hash)
+}
+
 func TestBlockTree_DeepCopy(t *testing.T) {
 	bt, _ := createFlatTree(t, 8)
 

lib/blocktree/leaves.go

@@ -13,6 +13,7 @@ import (
 
 // leafMap provides quick lookup for existing leaves
 type leafMap struct {
+	currentDeepestLeaf *node
 	sync.RWMutex
 	smap *sync.Map // map[common.Hash]*node
 }
@@ -63,7 +64,7 @@ func (lm *leafMap) deepestLeaf() *node {
 
 	max := big.NewInt(-1)
 
-	var dLeaf *node
+	var deepest *node
 	lm.smap.Range(func(h, n interface{}) bool {
 		if n == nil {
 			return true
@@ -73,15 +74,33 @@ func (lm *leafMap) deepestLeaf() *node {
 
 		if max.Cmp(node.number) < 0 {
 			max = node.number
-			dLeaf = node
-		} else if max.Cmp(node.number) == 0 && node.arrivalTime.Before(dLeaf.arrivalTime) {
-			dLeaf = node
+			deepest = node
+		} else if max.Cmp(node.number) == 0 && node.arrivalTime.Before(deepest.arrivalTime) {
+			deepest = node
 		}
 
 		return true
 	})
 
-	return dLeaf
+	if lm.currentDeepestLeaf != nil {
+		if lm.currentDeepestLeaf.hash == deepest.hash {
+			return lm.currentDeepestLeaf
+		}
+
+		// update the current deepest leaf if the found deepest has a greater number or
+		// if the current and the found deepest has the same number however the current
+		// arrived later then the found deepest
+		if deepest.number.Cmp(lm.currentDeepestLeaf.number) == 1 {
+			lm.currentDeepestLeaf = deepest
+		} else if deepest.number.Cmp(lm.currentDeepestLeaf.number) == 0 &&
+			deepest.arrivalTime.Before(lm.currentDeepestLeaf.arrivalTime) {
+			lm.currentDeepestLeaf = deepest
+		}
+	} else {
+		lm.currentDeepestLeaf = deepest
+	}
+
+	return lm.currentDeepestLeaf
 }
 
 func (lm *leafMap) toMap() map[common.Hash]*node {