From c0b3a4ec50fbe28a1f8c4d29323c2bd46f943c4b Mon Sep 17 00:00:00 2001 From: "Quentin McGaw (desktop)" Date: Fri, 21 Jan 2022 18:05:04 +0000 Subject: [PATCH] Generation test cases and fixes --- lib/trie/trie.go | 15 +- lib/trie/trie_test.go | 372 ++++++++++++++++++++++++++---------------- 2 files changed, 245 insertions(+), 142 deletions(-) diff --git a/lib/trie/trie.go b/lib/trie/trie.go index 66ffc683c9a..3cfbb98e30b 100644 --- a/lib/trie/trie.go +++ b/lib/trie/trie.go @@ -284,6 +284,8 @@ func (t *Trie) tryPut(key, value []byte) { // insert attempts to insert a key with value into the trie func (t *Trie) insert(parent Node, key []byte, value Node) Node { newParent := t.maybeUpdateGeneration(parent) + value.SetGeneration(t.generation) + if newParent == nil { value.SetKey(key) return value @@ -607,12 +609,12 @@ func (t *Trie) clearPrefixLimit(cn Node, prefix []byte, limit *uint32) (Node, bo } func (t *Trie) deleteNodes(cn Node, prefix []byte, limit *uint32) (newNode Node) { - curr := t.maybeUpdateGeneration(cn) - if *limit == 0 { - return curr + return cn } + curr := t.maybeUpdateGeneration(cn) + switch c := curr.(type) { case *node.Leaf: *limit-- @@ -757,8 +759,9 @@ func (t *Trie) delete(parent Node, key []byte) (Node, bool) { // Key exists. Delete it. return nil, true } - // Key doesn't exist. - return p, false + // Key doesn't exist, return parent + // without its generation changed + return parent, false case nil: return nil, false default: @@ -807,6 +810,8 @@ func handleDeletion(p *node.Branch, key []byte) Node { for i, grandchild := range c.Children { if grandchild != nil { br.Children[i] = grandchild + // No need to copy and update the generation + // of the grand children since they are not modified. } } diff --git a/lib/trie/trie_test.go b/lib/trie/trie_test.go index 3af8498e12f..e99059a07c8 100644 --- a/lib/trie/trie_test.go +++ b/lib/trie/trie_test.go @@ -998,21 +998,27 @@ func Test_Trie_insert(t *testing.T) { t.Parallel() testCases := map[string]struct { - trie Trie - parent Node - key []byte - value Node - newNode Node - expectedTrie Trie + trie Trie + parent Node + key []byte + value Node + newNode Node }{ "nil parent": { + trie: Trie{ + generation: 1, + }, key: []byte{1}, value: &node.Leaf{}, newNode: &node.Leaf{ - Key: []byte{1}, + Key: []byte{1}, + Generation: 1, }, }, "branch parent": { + trie: Trie{ + generation: 1, + }, parent: &node.Branch{ Key: []byte{1}, Value: []byte("branch"), @@ -1022,18 +1028,23 @@ func Test_Trie_insert(t *testing.T) { Value: []byte("leaf"), }, newNode: &node.Branch{ - Key: []byte{1}, - Value: []byte("branch"), - Dirty: true, + Key: []byte{1}, + Value: []byte("branch"), + Generation: 1, + Dirty: true, Children: [16]node.Node{ &node.Leaf{ - Key: []byte{}, - Value: []byte("leaf"), + Key: []byte{}, + Value: []byte("leaf"), + Generation: 1, }, }, }, }, "override leaf parent": { + trie: Trie{ + generation: 1, + }, parent: &node.Leaf{ Key: []byte{1}, Value: []byte("original leaf"), @@ -1043,12 +1054,16 @@ func Test_Trie_insert(t *testing.T) { Value: []byte("new leaf"), }, newNode: &node.Leaf{ - Key: []byte{1}, - Value: []byte("new leaf"), - Dirty: true, + Key: []byte{1}, + Value: []byte("new leaf"), + Generation: 1, + Dirty: true, }, }, "write same leaf value as child to parent leaf": { + trie: Trie{ + generation: 1, + }, parent: &node.Leaf{ Key: []byte{1}, Value: []byte("same"), @@ -1058,11 +1073,15 @@ func Test_Trie_insert(t *testing.T) { Value: []byte("same"), }, newNode: &node.Leaf{ - Key: []byte{1}, - Value: []byte("same"), + Key: []byte{1}, + Value: []byte("same"), + Generation: 1, }, }, "write leaf as child to parent leaf": { + trie: Trie{ + generation: 1, + }, parent: &node.Leaf{ Key: []byte{1}, Value: []byte("original leaf"), @@ -1072,18 +1091,23 @@ func Test_Trie_insert(t *testing.T) { Value: []byte("leaf"), }, newNode: &node.Branch{ - Key: []byte{1}, - Value: []byte("original leaf"), - Dirty: true, + Key: []byte{1}, + Value: []byte("original leaf"), + Dirty: true, + Generation: 1, Children: [16]node.Node{ &node.Leaf{ - Key: []byte{}, - Value: []byte("leaf"), + Key: []byte{}, + Value: []byte("leaf"), + Generation: 1, }, }, }, }, "write leaf as divergent child next to parent leaf": { + trie: Trie{ + generation: 1, + }, parent: &node.Leaf{ Key: []byte{1, 2}, Value: []byte("original leaf"), @@ -1093,23 +1117,29 @@ func Test_Trie_insert(t *testing.T) { Value: []byte("leaf"), }, newNode: &node.Branch{ - Key: []byte{}, - Dirty: true, + Key: []byte{}, + Dirty: true, + Generation: 1, Children: [16]node.Node{ nil, &node.Leaf{ - Key: []byte{2}, - Value: []byte("original leaf"), - Dirty: true, + Key: []byte{2}, + Value: []byte("original leaf"), + Dirty: true, + Generation: 1, }, &node.Leaf{ - Key: []byte{3}, - Value: []byte("leaf"), + Key: []byte{3}, + Value: []byte("leaf"), + Generation: 1, }, }, }, }, "write leaf into nil leaf": { + trie: Trie{ + generation: 1, + }, parent: &node.Leaf{ Key: []byte{1}, }, @@ -1118,12 +1148,16 @@ func Test_Trie_insert(t *testing.T) { Value: []byte("leaf"), }, newNode: &node.Branch{ - Key: []byte{1}, - Value: []byte("leaf"), - Dirty: true, + Key: []byte{1}, + Value: []byte("leaf"), + Dirty: true, + Generation: 1, }, }, "write leaf as child to nil value leaf": { + trie: Trie{ + generation: 1, + }, parent: &node.Leaf{ Key: []byte{1, 2}, }, @@ -1132,14 +1166,16 @@ func Test_Trie_insert(t *testing.T) { Value: []byte("leaf"), }, newNode: &node.Branch{ - Key: []byte{1}, - Value: []byte("leaf"), - Dirty: true, + Key: []byte{1}, + Value: []byte("leaf"), + Dirty: true, + Generation: 1, Children: [16]node.Node{ nil, nil, &node.Leaf{ - Key: []byte{}, - Dirty: true, + Key: []byte{}, + Dirty: true, + Generation: 1, }, }, }, @@ -1152,10 +1188,12 @@ func Test_Trie_insert(t *testing.T) { t.Parallel() trie := testCase.trie + expectedTrie := *trie.DeepCopy() + newNode := trie.insert(testCase.parent, testCase.key, testCase.value) assert.Equal(t, testCase.newNode, newNode) - assert.Equal(t, testCase.expectedTrie, trie) + assert.Equal(t, expectedTrie, trie) }) } } @@ -1922,7 +1960,6 @@ func Test_Trie_clearPrefixLimit(t *testing.T) { newParent Node updated bool allDeleted bool - expectedTrie Trie }{ "limit is zero": { allDeleted: true, @@ -1951,6 +1988,9 @@ func Test_Trie_clearPrefixLimit(t *testing.T) { allDeleted: true, }, "leaf parent with key no common prefix": { + trie: Trie{ + generation: 1, + }, parent: &node.Leaf{ Key: []byte{1, 2}, }, @@ -1958,11 +1998,15 @@ func Test_Trie_clearPrefixLimit(t *testing.T) { limit: 1, expectedLimit: 1, newParent: &node.Leaf{ - Key: []byte{1, 2}, + Key: []byte{1, 2}, + Generation: 1, }, allDeleted: true, }, "leaf parent with key smaller than prefix": { + trie: Trie{ + generation: 1, + }, parent: &node.Leaf{ Key: []byte{1}, }, @@ -1970,7 +2014,8 @@ func Test_Trie_clearPrefixLimit(t *testing.T) { limit: 1, expectedLimit: 1, newParent: &node.Leaf{ - Key: []byte{1}, + Key: []byte{1}, + Generation: 1, }, allDeleted: true, }, @@ -1995,6 +2040,9 @@ func Test_Trie_clearPrefixLimit(t *testing.T) { allDeleted: true, }, "branch without value with no common prefix": { + trie: Trie{ + generation: 1, + }, parent: &node.Branch{ Key: []byte{1, 2}, }, @@ -2002,11 +2050,15 @@ func Test_Trie_clearPrefixLimit(t *testing.T) { limit: 1, expectedLimit: 1, newParent: &node.Branch{ - Key: []byte{1, 2}, + Key: []byte{1, 2}, + Generation: 1, }, allDeleted: true, }, "branch without value with key smaller than prefix by more than one": { + trie: Trie{ + generation: 1, + }, parent: &node.Branch{ Key: []byte{1}, }, @@ -2014,11 +2066,15 @@ func Test_Trie_clearPrefixLimit(t *testing.T) { limit: 1, expectedLimit: 1, newParent: &node.Branch{ - Key: []byte{1}, + Key: []byte{1}, + Generation: 1, }, allDeleted: true, }, "branch without value with key smaller than prefix by one": { + trie: Trie{ + generation: 1, + }, parent: &node.Branch{ Key: []byte{1}, }, @@ -2026,7 +2082,8 @@ func Test_Trie_clearPrefixLimit(t *testing.T) { limit: 1, expectedLimit: 1, newParent: &node.Branch{ - Key: []byte{1}, + Key: []byte{1}, + Generation: 1, }, allDeleted: true, }, @@ -2051,6 +2108,9 @@ func Test_Trie_clearPrefixLimit(t *testing.T) { allDeleted: true, }, "branch with value with no common prefix": { + trie: Trie{ + generation: 1, + }, parent: &node.Branch{ Key: []byte{1, 2}, Value: []byte{1}, @@ -2059,12 +2119,16 @@ func Test_Trie_clearPrefixLimit(t *testing.T) { limit: 1, expectedLimit: 1, newParent: &node.Branch{ - Key: []byte{1, 2}, - Value: []byte{1}, + Key: []byte{1, 2}, + Value: []byte{1}, + Generation: 1, }, allDeleted: true, }, "branch with value with key smaller than prefix by more than one": { + trie: Trie{ + generation: 1, + }, parent: &node.Branch{ Key: []byte{1}, Value: []byte{1}, @@ -2073,12 +2137,16 @@ func Test_Trie_clearPrefixLimit(t *testing.T) { limit: 1, expectedLimit: 1, newParent: &node.Branch{ - Key: []byte{1}, - Value: []byte{1}, + Key: []byte{1}, + Value: []byte{1}, + Generation: 1, }, allDeleted: true, }, "branch with value with key smaller than prefix by one": { + trie: Trie{ + generation: 1, + }, parent: &node.Branch{ Key: []byte{1}, Value: []byte{1}, @@ -2087,12 +2155,16 @@ func Test_Trie_clearPrefixLimit(t *testing.T) { limit: 1, expectedLimit: 1, newParent: &node.Branch{ - Key: []byte{1}, - Value: []byte{1}, + Key: []byte{1}, + Value: []byte{1}, + Generation: 1, }, allDeleted: true, }, "delete one child of branch": { + trie: Trie{ + generation: 1, + }, parent: &node.Branch{ Key: []byte{1}, Value: []byte{1}, @@ -2104,9 +2176,10 @@ func Test_Trie_clearPrefixLimit(t *testing.T) { prefix: []byte{1}, limit: 1, newParent: &node.Branch{ - Key: []byte{1}, - Value: []byte{1}, - Dirty: true, + Key: []byte{1}, + Value: []byte{1}, + Dirty: true, + Generation: 1, Children: [16]node.Node{ nil, &node.Leaf{Key: []byte{4}}, @@ -2115,6 +2188,9 @@ func Test_Trie_clearPrefixLimit(t *testing.T) { updated: true, }, "fully delete children of branch with value": { + trie: Trie{ + generation: 1, + }, parent: &node.Branch{ Key: []byte{1}, Value: []byte{1}, @@ -2126,9 +2202,10 @@ func Test_Trie_clearPrefixLimit(t *testing.T) { prefix: []byte{1}, limit: 2, newParent: &node.Leaf{ - Key: []byte{1}, - Value: []byte{1}, - Dirty: true, + Key: []byte{1}, + Value: []byte{1}, + Dirty: true, + Generation: 1, }, updated: true, }, @@ -2146,6 +2223,9 @@ func Test_Trie_clearPrefixLimit(t *testing.T) { allDeleted: true, }, "partially delete child of branch": { + trie: Trie{ + generation: 1, + }, parent: &node.Branch{ Key: []byte{1}, Value: []byte{1}, @@ -2167,23 +2247,29 @@ func Test_Trie_clearPrefixLimit(t *testing.T) { prefix: []byte{1, 0}, limit: 1, newParent: &node.Branch{ - Key: []byte{1}, - Value: []byte{1}, - Dirty: true, + Key: []byte{1}, + Value: []byte{1}, + Dirty: true, + Generation: 1, Children: [16]node.Node{ &node.Leaf{ // full key 1, 0, 3 - Key: []byte{3}, - Value: []byte{1}, - Dirty: true, + Key: []byte{3}, + Value: []byte{1}, + Dirty: true, + Generation: 1, }, &node.Leaf{ Key: []byte{6}, + // Not modified so same generation as before }, }, }, updated: true, }, "update child of branch": { + trie: Trie{ + generation: 1, + }, parent: &node.Branch{ Key: []byte{1}, Value: []byte{1}, @@ -2197,9 +2283,10 @@ func Test_Trie_clearPrefixLimit(t *testing.T) { prefix: []byte{1, 0, 2}, limit: 1, newParent: &node.Leaf{ - Key: []byte{1}, - Value: []byte{1}, - Dirty: true, + Key: []byte{1}, + Value: []byte{1}, + Dirty: true, + Generation: 1, }, updated: true, allDeleted: true, @@ -2212,6 +2299,7 @@ func Test_Trie_clearPrefixLimit(t *testing.T) { t.Parallel() trie := testCase.trie + expectedTrie := *trie.DeepCopy() newParent, updated, allDeleted := trie.clearPrefixLimit(testCase.parent, testCase.prefix, &testCase.limit) @@ -2220,7 +2308,7 @@ func Test_Trie_clearPrefixLimit(t *testing.T) { assert.Equal(t, testCase.expectedLimit, testCase.limit) assert.Equal(t, testCase.updated, updated) assert.Equal(t, testCase.allDeleted, allDeleted) - assert.Equal(t, testCase.expectedTrie, trie) + assert.Equal(t, expectedTrie, trie) }) } } @@ -2229,16 +2317,18 @@ func Test_Trie_deleteNodes(t *testing.T) { t.Parallel() testCases := map[string]struct { - trie Trie - parent Node - prefix []byte - limit uint32 - newLimit uint32 - newNode Node - oneDeletion bool - expectedTrie Trie + trie Trie + parent Node + prefix []byte + limit uint32 + newLimit uint32 + newNode Node + oneDeletion bool }{ "zero limit": { + trie: Trie{ + generation: 1, + }, parent: &node.Leaf{ Key: []byte{1}, }, @@ -2250,30 +2340,6 @@ func Test_Trie_deleteNodes(t *testing.T) { limit: 1, newLimit: 1, }, - "update leaf generation": { - trie: Trie{ - generation: 1, - }, - parent: &node.Leaf{}, - newNode: &node.Leaf{ - Generation: 1, - }, - expectedTrie: Trie{ - generation: 1, - }, - }, - "update branch generation": { - trie: Trie{ - generation: 1, - }, - parent: &node.Branch{}, - newNode: &node.Branch{ - Generation: 1, - }, - expectedTrie: Trie{ - generation: 1, - }, - }, "delete leaf": { parent: &node.Leaf{}, limit: 2, @@ -2326,9 +2392,6 @@ func Test_Trie_deleteNodes(t *testing.T) { &node.Leaf{Key: []byte{2}}, }, }, - expectedTrie: Trie{ - generation: 1, - }, }, "delete branch children only": { trie: Trie{ @@ -2350,9 +2413,6 @@ func Test_Trie_deleteNodes(t *testing.T) { Dirty: true, Generation: 1, }, - expectedTrie: Trie{ - generation: 1, - }, }, "delete branch all children except one": { trie: Trie{ @@ -2377,9 +2437,6 @@ func Test_Trie_deleteNodes(t *testing.T) { Generation: 1, Dirty: true, }, - expectedTrie: Trie{ - generation: 1, - }, }, } @@ -2389,12 +2446,13 @@ func Test_Trie_deleteNodes(t *testing.T) { t.Parallel() trie := testCase.trie + expectedTrie := *trie.DeepCopy() newNode := trie.deleteNodes(testCase.parent, testCase.prefix, &testCase.limit) assert.Equal(t, testCase.limit, testCase.limit) assert.Equal(t, testCase.newNode, newNode) - assert.Equal(t, testCase.expectedTrie, trie) + assert.Equal(t, expectedTrie, trie) }) } } @@ -2474,12 +2532,11 @@ func Test_Trie_clearPrefix(t *testing.T) { t.Parallel() testCases := map[string]struct { - trie Trie - parent Node - prefix []byte - newParent Node - updated bool - expectedTrie Trie + trie Trie + parent Node + prefix []byte + newParent Node + updated bool }{ "nil parent": {}, "leaf parent with common prefix": { @@ -2497,21 +2554,29 @@ func Test_Trie_clearPrefix(t *testing.T) { updated: true, }, "leaf parent with key no common prefix": { + trie: Trie{ + generation: 1, + }, parent: &node.Leaf{ Key: []byte{1, 2}, }, prefix: []byte{1, 3}, newParent: &node.Leaf{ - Key: []byte{1, 2}, + Key: []byte{1, 2}, + Generation: 1, }, }, "leaf parent with key smaller than prefix": { + trie: Trie{ + generation: 1, + }, parent: &node.Leaf{ Key: []byte{1}, }, prefix: []byte{1, 2}, newParent: &node.Leaf{ - Key: []byte{1}, + Key: []byte{1}, + Generation: 1, }, }, "branch parent with common prefix": { @@ -2529,33 +2594,48 @@ func Test_Trie_clearPrefix(t *testing.T) { updated: true, }, "branch with no common prefix": { + trie: Trie{ + generation: 1, + }, parent: &node.Branch{ Key: []byte{1, 2}, }, prefix: []byte{1, 3}, newParent: &node.Branch{ - Key: []byte{1, 2}, + Key: []byte{1, 2}, + Generation: 1, }, }, "branch with key smaller than prefix by more than one": { + trie: Trie{ + generation: 1, + }, parent: &node.Branch{ Key: []byte{1}, }, prefix: []byte{1, 2, 3}, newParent: &node.Branch{ - Key: []byte{1}, + Key: []byte{1}, + Generation: 1, }, }, "branch with key smaller than prefix by one": { + trie: Trie{ + generation: 1, + }, parent: &node.Branch{ Key: []byte{1}, }, prefix: []byte{1, 2}, newParent: &node.Branch{ - Key: []byte{1}, + Key: []byte{1}, + Generation: 1, }, }, "delete one child of branch": { + trie: Trie{ + generation: 1, + }, parent: &node.Branch{ Key: []byte{1}, Value: []byte{1}, @@ -2566,9 +2646,10 @@ func Test_Trie_clearPrefix(t *testing.T) { }, prefix: []byte{1, 0, 3}, newParent: &node.Branch{ - Key: []byte{1}, - Value: []byte{1}, - Dirty: true, + Key: []byte{1}, + Value: []byte{1}, + Dirty: true, + Generation: 1, Children: [16]node.Node{ nil, &node.Leaf{Key: []byte{4}}, @@ -2577,6 +2658,9 @@ func Test_Trie_clearPrefix(t *testing.T) { updated: true, }, "fully delete child of branch": { + trie: Trie{ + generation: 1, + }, parent: &node.Branch{ Key: []byte{1}, Value: []byte{1}, @@ -2586,13 +2670,17 @@ func Test_Trie_clearPrefix(t *testing.T) { }, prefix: []byte{1, 0}, newParent: &node.Leaf{ - Key: []byte{1}, - Value: []byte{1}, - Dirty: true, + Key: []byte{1}, + Value: []byte{1}, + Dirty: true, + Generation: 1, }, updated: true, }, "partially delete child of branch": { + trie: Trie{ + generation: 1, + }, parent: &node.Branch{ Key: []byte{1}, Value: []byte{1}, @@ -2610,14 +2698,16 @@ func Test_Trie_clearPrefix(t *testing.T) { }, prefix: []byte{1, 0, 3, 0}, newParent: &node.Branch{ - Key: []byte{1}, - Value: []byte{1}, - Dirty: true, + Key: []byte{1}, + Value: []byte{1}, + Dirty: true, + Generation: 1, Children: [16]node.Node{ &node.Leaf{ // full key 1, 0, 3 - Key: []byte{3}, - Value: []byte{1}, - Dirty: true, + Key: []byte{3}, + Value: []byte{1}, + Dirty: true, + Generation: 1, }, }, }, @@ -2631,13 +2721,14 @@ func Test_Trie_clearPrefix(t *testing.T) { t.Parallel() trie := testCase.trie + expectedTrie := *trie.DeepCopy() newParent, updated := trie.clearPrefix(testCase.parent, testCase.prefix) assert.Equal(t, testCase.newParent, newParent) assert.Equal(t, testCase.updated, updated) - assert.Equal(t, testCase.expectedTrie, trie) + assert.Equal(t, expectedTrie, trie) }) } } @@ -2665,6 +2756,7 @@ func Test_Trie_Delete(t *testing.T) { }, "delete branch node": { trie: Trie{ + generation: 1, root: &node.Branch{ Key: []byte{1, 2}, Children: [16]node.Node{ @@ -2687,18 +2779,21 @@ func Test_Trie_Delete(t *testing.T) { }, key: []byte{0x12, 0x16}, expectedTrie: Trie{ + generation: 1, root: &node.Branch{ - Key: []byte{1, 2}, - Dirty: true, + Key: []byte{1, 2}, + Dirty: true, + Generation: 1, Children: [16]node.Node{ &node.Leaf{ Key: []byte{5}, Value: []byte{97}, }, &node.Leaf{ // full key in nibbles 1, 2, 1, 6 - Key: []byte{6, 0, 7}, - Value: []byte{99}, - Dirty: true, + Key: []byte{6, 0, 7}, + Value: []byte{99}, + Dirty: true, + Generation: 1, }, }, }, @@ -2760,6 +2855,9 @@ func Test_Trie_delete(t *testing.T) { updated: true, }, "leaf parent mismatches key": { + trie: Trie{ + generation: 1, + }, parent: &node.Leaf{ Key: []byte{1}, },