From c87313f7d5157a18ac4ee1fc361a71724331f313 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ecl=C3=A9sio=20J=C3=BAnior?= Date: Wed, 1 Dec 2021 13:02:07 -0400 Subject: [PATCH 1/5] fix: removes the inconsistency to choose a deepest leaf --- lib/blocktree/blocktree_test.go | 13 +++++++++++++ lib/blocktree/leaves.go | 29 ++++++++++++++++++++++++----- 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/lib/blocktree/blocktree_test.go b/lib/blocktree/blocktree_test.go index f35b8462f4..d259616543 100644 --- a/lib/blocktree/blocktree_test.go +++ b/lib/blocktree/blocktree_test.go @@ -38,6 +38,7 @@ func newBlockTreeFromNode(root *node) *BlockTree { } func createTestBlockTree(t *testing.T, header *types.Header, number int) (*BlockTree, []testBranch) { + fmt.Printf("Building the block tree\n") bt := NewBlockTreeFromRoot(header) previousHash := header.Hash() @@ -58,6 +59,9 @@ 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) + + fmt.Printf("BUILDING #%v (%v) => %s\n", i, at, hash) + previousHash = hash isBranch := r.Intn(2) @@ -88,8 +92,12 @@ 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) + + fmt.Printf("BRANCHING #%v (%v) => %s\n", branch.number.Uint64(), at, hash) + previousHash = hash at += int64(r.Intn(8)) + } } @@ -458,11 +466,16 @@ func TestBlockTree_GetHashByNumber(t *testing.T) { best := bt.DeepestBlockHash() bn := bt.getNode(best) + fmt.Printf("Assert the descendents\n") for i := int64(0); i < bn.number.Int64(); i++ { + deepest := bt.leaves.deepestLeaf() + hash, err := bt.GetHashByNumber(big.NewInt(i)) require.NoError(t, err) require.Equal(t, big.NewInt(i), bt.getNode(hash).number) desc, err := bt.IsDescendantOf(hash, best) + fmt.Printf("%s -> %s\n", hash, deepest.hash) + require.NoError(t, err) require.True(t, desc, fmt.Sprintf("index %d failed, got hash=%s", i, hash)) } diff --git a/lib/blocktree/leaves.go b/lib/blocktree/leaves.go index 13d20cfa3f..1922a78f83 100644 --- a/lib/blocktree/leaves.go +++ b/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 foundDeepest *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 + foundDeepest = node + } else if max.Cmp(node.number) == 0 && node.arrivalTime.Before(foundDeepest.arrivalTime) { + foundDeepest = node } return true }) - return dLeaf + if lm.currentDeepestLeaf != nil { + if lm.currentDeepestLeaf.hash == foundDeepest.hash { + return lm.currentDeepestLeaf + } + + // update the current deepest leaf if the foundDeepest 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 foundDeepest.number.Cmp(lm.currentDeepestLeaf.number) == 1 { + lm.currentDeepestLeaf = foundDeepest + } else if foundDeepest.number.Cmp(lm.currentDeepestLeaf.number) == 0 && + foundDeepest.arrivalTime.Before(lm.currentDeepestLeaf.arrivalTime) { + lm.currentDeepestLeaf = foundDeepest + } + } else { + lm.currentDeepestLeaf = foundDeepest + } + + return lm.currentDeepestLeaf } func (lm *leafMap) toMap() map[common.Hash]*node { From 1fe67404580b96f1524a01a204711a8a54551e57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ecl=C3=A9sio=20J=C3=BAnior?= Date: Wed, 1 Dec 2021 16:32:21 -0400 Subject: [PATCH 2/5] chore: add test to currentDeepestNode --- lib/blocktree/blocktree_test.go | 105 ++++++++++++++++++++++++++++++++ lib/blocktree/leaves.go | 24 ++++---- 2 files changed, 117 insertions(+), 12 deletions(-) diff --git a/lib/blocktree/blocktree_test.go b/lib/blocktree/blocktree_test.go index d259616543..2817de416b 100644 --- a/lib/blocktree/blocktree_test.go +++ b/lib/blocktree/blocktree_test.go @@ -487,6 +487,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 arraival 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 arraival 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) diff --git a/lib/blocktree/leaves.go b/lib/blocktree/leaves.go index 1922a78f83..f7c1f4ecc0 100644 --- a/lib/blocktree/leaves.go +++ b/lib/blocktree/leaves.go @@ -64,7 +64,7 @@ func (lm *leafMap) deepestLeaf() *node { max := big.NewInt(-1) - var foundDeepest *node + var deepest *node lm.smap.Range(func(h, n interface{}) bool { if n == nil { return true @@ -74,30 +74,30 @@ func (lm *leafMap) deepestLeaf() *node { if max.Cmp(node.number) < 0 { max = node.number - foundDeepest = node - } else if max.Cmp(node.number) == 0 && node.arrivalTime.Before(foundDeepest.arrivalTime) { - foundDeepest = node + deepest = node + } else if max.Cmp(node.number) == 0 && node.arrivalTime.Before(deepest.arrivalTime) { + deepest = node } return true }) if lm.currentDeepestLeaf != nil { - if lm.currentDeepestLeaf.hash == foundDeepest.hash { + if lm.currentDeepestLeaf.hash == deepest.hash { return lm.currentDeepestLeaf } - // update the current deepest leaf if the foundDeepest has a greater number or + // 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 foundDeepest.number.Cmp(lm.currentDeepestLeaf.number) == 1 { - lm.currentDeepestLeaf = foundDeepest - } else if foundDeepest.number.Cmp(lm.currentDeepestLeaf.number) == 0 && - foundDeepest.arrivalTime.Before(lm.currentDeepestLeaf.arrivalTime) { - lm.currentDeepestLeaf = foundDeepest + 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 = foundDeepest + lm.currentDeepestLeaf = deepest } return lm.currentDeepestLeaf From de90ca3c063c528cf129730fe6c2103c222a16ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ecl=C3=A9sio=20J=C3=BAnior?= Date: Wed, 1 Dec 2021 16:34:16 -0400 Subject: [PATCH 3/5] chore: removing prints --- lib/blocktree/blocktree_test.go | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/lib/blocktree/blocktree_test.go b/lib/blocktree/blocktree_test.go index 2817de416b..68f17e51a8 100644 --- a/lib/blocktree/blocktree_test.go +++ b/lib/blocktree/blocktree_test.go @@ -38,7 +38,6 @@ func newBlockTreeFromNode(root *node) *BlockTree { } func createTestBlockTree(t *testing.T, header *types.Header, number int) (*BlockTree, []testBranch) { - fmt.Printf("Building the block tree\n") bt := NewBlockTreeFromRoot(header) previousHash := header.Hash() @@ -60,8 +59,6 @@ func createTestBlockTree(t *testing.T, header *types.Header, number int) (*Block err := bt.AddBlock(header, time.Unix(0, at)) require.NoError(t, err) - fmt.Printf("BUILDING #%v (%v) => %s\n", i, at, hash) - previousHash = hash isBranch := r.Intn(2) @@ -93,8 +90,6 @@ func createTestBlockTree(t *testing.T, header *types.Header, number int) (*Block err := bt.AddBlock(header, time.Unix(0, at)) require.NoError(t, err) - fmt.Printf("BRANCHING #%v (%v) => %s\n", branch.number.Uint64(), at, hash) - previousHash = hash at += int64(r.Intn(8)) @@ -466,16 +461,11 @@ func TestBlockTree_GetHashByNumber(t *testing.T) { best := bt.DeepestBlockHash() bn := bt.getNode(best) - fmt.Printf("Assert the descendents\n") for i := int64(0); i < bn.number.Int64(); i++ { - deepest := bt.leaves.deepestLeaf() - hash, err := bt.GetHashByNumber(big.NewInt(i)) require.NoError(t, err) require.Equal(t, big.NewInt(i), bt.getNode(hash).number) desc, err := bt.IsDescendantOf(hash, best) - fmt.Printf("%s -> %s\n", hash, deepest.hash) - require.NoError(t, err) require.True(t, desc, fmt.Sprintf("index %d failed, got hash=%s", i, hash)) } From 41146d6471dcd3e97f9772cc5ebe2c3e7ef57965 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ecl=C3=A9sio=20J=C3=BAnior?= Date: Wed, 1 Dec 2021 16:36:07 -0400 Subject: [PATCH 4/5] chore: right identation in the comment block --- lib/blocktree/blocktree_test.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/blocktree/blocktree_test.go b/lib/blocktree/blocktree_test.go index 68f17e51a8..63797f2dcc 100644 --- a/lib/blocktree/blocktree_test.go +++ b/lib/blocktree/blocktree_test.go @@ -494,13 +494,13 @@ func TestBlockTree_AllLeavesHasSameNumberAndArrivalTime_DeepestBlockHash_ShouldH 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) + 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++ { From 2d94bc49d198b91d9ef26fe97c52b0f13dd19e0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ecl=C3=A9sio=20J=C3=BAnior?= Date: Wed, 1 Dec 2021 17:34:07 -0400 Subject: [PATCH 5/5] chore: fix typo --- lib/blocktree/blocktree_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/blocktree/blocktree_test.go b/lib/blocktree/blocktree_test.go index 63797f2dcc..b7d2bc1639 100644 --- a/lib/blocktree/blocktree_test.go +++ b/lib/blocktree/blocktree_test.go @@ -490,7 +490,7 @@ func TestBlockTree_AllLeavesHasSameNumberAndArrivalTime_DeepestBlockHash_ShouldH // and all block with the same arrival time /** - base tree and nodes representation, all with the same arraival time and all + 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. @@ -526,7 +526,7 @@ func TestBlockTree_AllLeavesHasSameNumberAndArrivalTime_DeepestBlockHash_ShouldH } } - // create all the branch nodes with the same arraival time + // create all the branch nodes with the same arrival time for _, branch := range branches { previousHash = branch.hash