Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from
Dec 3, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
108 changes: 108 additions & 0 deletions lib/blocktree/blocktree_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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))

}
}

Expand Down Expand Up @@ -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)

Expand Down
29 changes: 24 additions & 5 deletions lib/blocktree/leaves.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand Down