From 7cd411844d19822f47b8179601d4d3d895cd93ae Mon Sep 17 00:00:00 2001 From: Kishan Sagathiya Date: Tue, 22 Mar 2022 07:42:26 +0530 Subject: [PATCH] fix(lib/trie): Make sure writing and reading a trie to disk gives the same trie and cover more store/load child trie related test cases (#2302) * test TestGetStorageChildAndGetStorageFromChild with non-empty trie * test store and load of a trie that hash child tries * tackle the case when encoding and hash is same, i.e., encoding is less than 32 bits * don't put childTrie again inside the trie --- dot/state/storage.go | 2 +- dot/state/storage_test.go | 4 +- internal/trie/node/leaf.go | 2 +- lib/trie/child_storage.go | 10 ++--- lib/trie/database.go | 16 ++++---- lib/trie/database_test.go | 73 ++++++++++++++++++++++++++++++---- lib/trie/helpers_test.go | 2 +- lib/trie/trie_endtoend_test.go | 42 +++++++++---------- 8 files changed, 106 insertions(+), 45 deletions(-) diff --git a/dot/state/storage.go b/dot/state/storage.go index 7326d34992..b4781b204c 100644 --- a/dot/state/storage.go +++ b/dot/state/storage.go @@ -167,7 +167,7 @@ func (s *StorageState) loadTrie(root *common.Hash) (*trie.Trie, error) { tr, err := s.LoadFromDB(*root) if err != nil { - return nil, errTrieDoesNotExist(*root) + return nil, fmt.Errorf("trie does not exist at root %s: %w", *root, err) } return tr, nil diff --git a/dot/state/storage_test.go b/dot/state/storage_test.go index ffbaf86c5f..b060cf2c3e 100644 --- a/dot/state/storage_test.go +++ b/dot/state/storage_test.go @@ -10,6 +10,7 @@ import ( "github.com/ChainSafe/gossamer/dot/state/pruner" "github.com/ChainSafe/gossamer/dot/telemetry" "github.com/ChainSafe/gossamer/dot/types" + "github.com/ChainSafe/gossamer/internal/trie/node" "github.com/ChainSafe/gossamer/lib/common" "github.com/ChainSafe/gossamer/lib/genesis" runtime "github.com/ChainSafe/gossamer/lib/runtime/storage" @@ -179,7 +180,8 @@ func TestGetStorageChildAndGetStorageFromChild(t *testing.T) { "0", )) - testChildTrie := trie.NewEmptyTrie() + testChildTrie := trie.NewTrie(node.NewLeaf([]byte{1, 2}, []byte{3, 4}, true, 0)) + testChildTrie.Put([]byte("keyInsidechild"), []byte("voila")) err = genTrie.PutChild([]byte("keyToChild"), testChildTrie) diff --git a/internal/trie/node/leaf.go b/internal/trie/node/leaf.go index f3087c14ff..129ebce3c1 100644 --- a/internal/trie/node/leaf.go +++ b/internal/trie/node/leaf.go @@ -16,7 +16,7 @@ type Leaf struct { // Partial key bytes in nibbles (0 to f in hexadecimal) Key []byte Value []byte - // Dirty is true when the branch differs + // Dirty is true when the leaf differs // from the node stored in the database. Dirty bool HashDigest []byte diff --git a/lib/trie/child_storage.go b/lib/trie/child_storage.go index 41307ded29..9b49f95a4d 100644 --- a/lib/trie/child_storage.go +++ b/lib/trie/child_storage.go @@ -16,16 +16,16 @@ var ChildStorageKeyPrefix = []byte(":child_storage:default:") var ErrChildTrieDoesNotExist = errors.New("child trie does not exist") // PutChild inserts a child trie into the main trie at key :child_storage:[keyToChild] +// A child trie is added as a node (K, V) in the main trie. K is the child storage key +// associated to the child trie, and V is the root hash of the child trie. func (t *Trie) PutChild(keyToChild []byte, child *Trie) error { childHash, err := child.Hash() if err != nil { return err } - key := append(ChildStorageKeyPrefix, keyToChild...) - value := [32]byte(childHash) - t.Put(key, value[:]) + t.Put(key, childHash.ToBytes()) t.childTries[childHash] = child return nil } @@ -38,9 +38,7 @@ func (t *Trie) GetChild(keyToChild []byte) (*Trie, error) { return nil, fmt.Errorf("%w at key 0x%x%x", ErrChildTrieDoesNotExist, ChildStorageKeyPrefix, keyToChild) } - hash := [32]byte{} - copy(hash[:], childHash) - return t.childTries[common.Hash(hash)], nil + return t.childTries[common.BytesToHash(childHash)], nil } // PutIntoChild puts a key-value pair into the child trie located in the main trie at key :child_storage:[keyToChild] diff --git a/lib/trie/database.go b/lib/trie/database.go index d60e7aafb6..b9822dc47a 100644 --- a/lib/trie/database.go +++ b/lib/trie/database.go @@ -150,8 +150,7 @@ func (t *Trie) Load(db chaindb.Database, rootHash common.Hash) error { t.root = nil return nil } - - rootHashBytes := rootHash[:] + rootHashBytes := rootHash.ToBytes() encodedNode, err := db.Get(rootHashBytes) if err != nil { @@ -163,6 +162,7 @@ func (t *Trie) Load(db chaindb.Database, rootHash common.Hash) error { if err != nil { return fmt.Errorf("cannot decode root node: %w", err) } + t.root = root t.root.SetDirty(false) t.root.SetEncodingAndHash(encodedNode, rootHashBytes) @@ -185,6 +185,7 @@ func (t *Trie) load(db chaindb.Database, n Node) error { } hash := child.GetHash() + encodedNode, err := db.Get(hash) if err != nil { return fmt.Errorf("cannot find child node key 0x%x in database: %w", hash, err) @@ -209,16 +210,17 @@ func (t *Trie) load(db chaindb.Database, n Node) error { for _, key := range t.GetKeysWithPrefix(ChildStorageKeyPrefix) { childTrie := NewEmptyTrie() value := t.Get(key) - err := childTrie.Load(db, common.NewHash(value)) + rootHash := common.BytesToHash(value) + err := childTrie.Load(db, rootHash) if err != nil { - return fmt.Errorf("failed to load child trie with root hash=0x%x: %w", value, err) + return fmt.Errorf("failed to load child trie with root hash=%s: %w", rootHash, err) } - err = t.PutChild(value, childTrie) + hash, err := childTrie.Hash() if err != nil { - return fmt.Errorf("failed to insert child trie with root hash=0x%x into main trie: %w", - childTrie.root.GetHash(), err) + return fmt.Errorf("cannot hash chilld trie at key 0x%x: %w", key, err) } + t.childTries[hash] = childTrie } return nil diff --git a/lib/trie/database_test.go b/lib/trie/database_test.go index f121573d41..07a041090b 100644 --- a/lib/trie/database_test.go +++ b/lib/trie/database_test.go @@ -9,6 +9,7 @@ import ( "testing" "github.com/ChainSafe/chaindb" + "github.com/ChainSafe/gossamer/internal/trie/node" "github.com/ChainSafe/gossamer/lib/utils" "github.com/stretchr/testify/require" ) @@ -21,7 +22,7 @@ func newTestDB(t *testing.T) chaindb.Database { } func TestTrie_DatabaseStoreAndLoad(t *testing.T) { - cases := [][]Test{ + cases := [][]keyValues{ { {key: []byte{0x01, 0x35}, value: []byte("pen")}, {key: []byte{0x01, 0x35, 0x79}, value: []byte("penguin")}, @@ -65,7 +66,7 @@ func TestTrie_DatabaseStoreAndLoad(t *testing.T) { res := NewEmptyTrie() err = res.Load(db, trie.MustHash()) require.NoError(t, err) - require.Equal(t, trie.MustHash(), res.MustHash()) + require.Equal(t, trie.String(), res.String()) for _, test := range testCase { val, err := GetFromDB(db, trie.MustHash(), test.key) @@ -76,7 +77,7 @@ func TestTrie_DatabaseStoreAndLoad(t *testing.T) { } func TestTrie_WriteDirty_Put(t *testing.T) { - cases := [][]Test{ + cases := [][]keyValues{ { {key: []byte{0x01, 0x35}, value: []byte("pen")}, {key: []byte{0x01, 0x35, 0x79}, value: []byte("penguin")}, @@ -154,7 +155,7 @@ func TestTrie_WriteDirty_Put(t *testing.T) { } func TestTrie_WriteDirty_PutReplace(t *testing.T) { - cases := [][]Test{ + cases := [][]keyValues{ { {key: []byte{0x01, 0x35}, value: []byte("pen")}, {key: []byte{0x01, 0x35, 0x79}, value: []byte("penguin")}, @@ -217,7 +218,7 @@ func TestTrie_WriteDirty_PutReplace(t *testing.T) { } func TestTrie_WriteDirty_Delete(t *testing.T) { - cases := [][]Test{ + cases := [][]keyValues{ { {key: []byte{0x01, 0x35}, value: []byte("pen")}, {key: []byte{0x01, 0x35, 0x79}, value: []byte("penguin")}, @@ -284,7 +285,7 @@ func TestTrie_WriteDirty_Delete(t *testing.T) { } func TestTrie_WriteDirty_ClearPrefix(t *testing.T) { - cases := [][]Test{ + cases := [][]keyValues{ { {key: []byte{0x01, 0x35}, value: []byte("pen")}, {key: []byte{0x01, 0x35, 0x79}, value: []byte("penguin")}, @@ -337,7 +338,7 @@ func TestTrie_WriteDirty_ClearPrefix(t *testing.T) { } func TestTrie_GetFromDB(t *testing.T) { - cases := [][]Test{ + cases := [][]keyValues{ { {key: []byte{0x01, 0x35}, value: []byte("pen")}, {key: []byte{0x01, 0x35, 0x79}, value: []byte("penguin")}, @@ -387,3 +388,61 @@ func TestTrie_GetFromDB(t *testing.T) { } } } + +func TestStoreAndLoadWithChildTries(t *testing.T) { + keyValue := []keyValues{ + {key: []byte{0xf2, 0x3}, value: []byte("f")}, + {key: []byte{0x09, 0xd3}, value: []byte("noot")}, + {key: []byte{0x07}, value: []byte("ramen")}, + {key: []byte{0}, value: nil}, + { + key: []byte("The boxed moved. That was a problem."), + value: []byte("The question now was whether or not Peter was going to open it up and look inside to see why it had moved."), // nolint + }, + } + + key := []byte{1, 2} + value := []byte{3, 4} + const dirty = true + const generation = 0 + + t.Run("happy path, tries being loaded are same as trie being read", func(t *testing.T) { + t.Parallel() + + // hash could be different for keys smaller than 32 and larger than 32 bits. + // thus, testing with keys of different sizes. + keysToTest := [][]byte{ + []byte("This handout will help you understand how paragraphs are formed, how to develop stronger paragraphs."), + []byte("This handout"), + []byte("test"), + } + + for _, keyToChild := range keysToTest { + trie := NewEmptyTrie() + + for _, test := range keyValue { + trie.Put(test.key, test.value) + } + + db := newTestDB(t) + + sampleChildTrie := NewTrie(node.NewLeaf(key, value, dirty, generation)) + + err := trie.PutChild(keyToChild, sampleChildTrie) + require.NoError(t, err) + + err = trie.Store(db) + require.NoError(t, err) + + res := NewEmptyTrie() + + err = res.Load(db, trie.MustHash()) + require.NoError(t, err) + + require.Equal(t, trie.childTries, res.childTries) + require.Equal(t, trie.String(), res.String()) + } + + }) + +} diff --git a/lib/trie/helpers_test.go b/lib/trie/helpers_test.go index 834415f4d9..e0853c6efe 100644 --- a/lib/trie/helpers_test.go +++ b/lib/trie/helpers_test.go @@ -20,7 +20,7 @@ type writeCall struct { var errTest = errors.New("test error") -type Test struct { +type keyValues struct { key []byte value []byte op int diff --git a/lib/trie/trie_endtoend_test.go b/lib/trie/trie_endtoend_test.go index 7baf467c7a..cdb0c69945 100644 --- a/lib/trie/trie_endtoend_test.go +++ b/lib/trie/trie_endtoend_test.go @@ -60,7 +60,7 @@ func writeFailedData(t *testing.T, kv map[string][]byte, path string) { func buildSmallTrie() *Trie { trie := NewEmptyTrie() - tests := []Test{ + tests := []keyValues{ {key: []byte{0x01, 0x35}, value: []byte("pen")}, {key: []byte{0x01, 0x35, 0x79}, value: []byte("penguin")}, {key: []byte{0xf2}, value: []byte("feather")}, @@ -76,7 +76,7 @@ func buildSmallTrie() *Trie { return trie } -func runTests(t *testing.T, trie *Trie, tests []Test) { +func runTests(t *testing.T, trie *Trie, tests []keyValues) { for _, test := range tests { switch test.op { case put: @@ -96,7 +96,7 @@ func runTests(t *testing.T, trie *Trie, tests []Test) { func TestPutAndGetBranch(t *testing.T) { trie := NewEmptyTrie() - tests := []Test{ + tests := []keyValues{ {key: []byte{0x01, 0x35}, value: []byte("spaghetti"), op: put}, {key: []byte{0x01, 0x35, 0x79}, value: []byte("gnocchi"), op: put}, {key: []byte{0x07}, value: []byte("ramen"), op: put}, @@ -115,7 +115,7 @@ func TestPutAndGetBranch(t *testing.T) { func TestPutAndGetOddKeyLengths(t *testing.T) { trie := NewEmptyTrie() - tests := []Test{ + tests := []keyValues{ {key: []byte{0x43, 0xc1}, value: []byte("noot"), op: put}, {key: []byte{0x49, 0x29}, value: []byte("nootagain"), op: put}, {key: []byte{0x43, 0x0c}, value: []byte("odd"), op: put}, @@ -210,7 +210,7 @@ func Test_Trie_PutAndGet_FailedData(t *testing.T) { func TestGetPartialKey(t *testing.T) { trie := NewEmptyTrie() - tests := []Test{ + tests := []keyValues{ {key: []byte{0x01, 0x35}, value: []byte("pen"), op: put}, {key: []byte{0x01, 0x35, 0x79}, value: []byte("penguin"), op: put}, {key: []byte{0x01, 0x35, 0x07}, value: []byte("odd"), op: put}, @@ -235,7 +235,7 @@ func TestGetPartialKey(t *testing.T) { func TestDeleteSmall(t *testing.T) { trie := buildSmallTrie() - tests := []Test{ + tests := []keyValues{ {key: []byte{}, value: []byte("floof"), op: del}, {key: []byte{}, value: nil, op: get}, {key: []byte{}, value: []byte("floof"), op: put}, @@ -279,7 +279,7 @@ func TestDeleteSmall(t *testing.T) { func TestDeleteCombineBranch(t *testing.T) { trie := buildSmallTrie() - tests := []Test{ + tests := []keyValues{ {key: []byte{0x01, 0x35, 0x46}, value: []byte("raccoon"), op: put}, {key: []byte{0x01, 0x35, 0x46, 0x77}, value: []byte("rat"), op: put}, {key: []byte{0x09, 0xd3}, value: []byte("noot"), op: del}, @@ -292,7 +292,7 @@ func TestDeleteCombineBranch(t *testing.T) { func TestDeleteFromBranch(t *testing.T) { trie := NewEmptyTrie() - tests := []Test{ + tests := []keyValues{ {key: []byte{0x06, 0x15, 0xfc}, value: []byte("noot"), op: put}, {key: []byte{0x06, 0x2b, 0xa9}, value: []byte("nootagain"), op: put}, {key: []byte{0x06, 0xaf, 0xb1}, value: []byte("odd"), op: put}, @@ -317,7 +317,7 @@ func TestDeleteFromBranch(t *testing.T) { func TestDeleteOddKeyLengths(t *testing.T) { trie := NewEmptyTrie() - tests := []Test{ + tests := []keyValues{ {key: []byte{0x43, 0xc1}, value: []byte("noot"), op: put}, {key: []byte{0x43, 0xc1}, value: []byte("noot"), op: get}, {key: []byte{0x49, 0x29}, value: []byte("nootagain"), op: put}, @@ -360,7 +360,7 @@ func TestTrieDiff(t *testing.T) { var testKey = []byte("testKey") - tests := []Test{ + tests := []keyValues{ {key: testKey, value: testKey}, {key: []byte("testKey1"), value: []byte("testKey1")}, {key: []byte("testKey2"), value: []byte("testKey2")}, @@ -374,7 +374,7 @@ func TestTrieDiff(t *testing.T) { err = trie.Store(storageDB) require.NoError(t, err) - tests = []Test{ + tests = []keyValues{ {key: testKey, value: []byte("newTestKey2")}, {key: []byte("testKey2"), value: []byte("newKey")}, {key: []byte("testKey3"), value: []byte("testKey3")}, @@ -462,7 +462,7 @@ func TestDelete(t *testing.T) { } func TestClearPrefix(t *testing.T) { - tests := []Test{ + tests := []keyValues{ {key: []byte{0x01, 0x35}, value: []byte("spaghetti"), op: put}, {key: []byte{0x01, 0x35, 0x79}, value: []byte("gnocchi"), op: put}, {key: []byte{0x01, 0x35, 0x79, 0xab}, value: []byte("spaghetti"), op: put}, @@ -627,7 +627,7 @@ func TestTrie_ClearPrefixVsDelete(t *testing.T) { []byte("a"), } - cases := [][]Test{ + cases := [][]keyValues{ { {key: []byte{0x01, 0x35}, value: []byte("pen")}, {key: []byte{0x01, 0x35, 0x79}, value: []byte("penguin")}, @@ -681,7 +681,7 @@ func TestTrie_ClearPrefixVsDelete(t *testing.T) { } func TestSnapshot(t *testing.T) { - tests := []Test{ + tests := []keyValues{ {key: []byte{0x01, 0x35}, value: []byte("spaghetti"), op: put}, {key: []byte{0x01, 0x35, 0x79}, value: []byte("gnocchi"), op: put}, {key: []byte{0x01, 0x35, 0x79, 0xab}, value: []byte("spaghetti"), op: put}, @@ -784,11 +784,11 @@ func TestTrie_ConcurrentSnapshotWrites(t *testing.T) { const size = 1000 const workers = 4 - testCases := make([][]Test, workers) + testCases := make([][]keyValues, workers) expectedTries := make([]*Trie, workers) for i := 0; i < workers; i++ { - testCases[i] = make([]Test, size) + testCases[i] = make([]keyValues, size) expectedTries[i] = buildSmallTrie() for j := 0; j < size; j++ { k := make([]byte, 2) @@ -805,7 +805,7 @@ func TestTrie_ConcurrentSnapshotWrites(t *testing.T) { expectedTries[i].ClearPrefix(k) } - testCases[i][j] = Test{ + testCases[i][j] = keyValues{ key: k, op: op, } @@ -821,7 +821,7 @@ func TestTrie_ConcurrentSnapshotWrites(t *testing.T) { for i := 0; i < workers; i++ { snapshotedTries[i] = buildSmallTrie().Snapshot() - go func(trie *Trie, operations []Test, + go func(trie *Trie, operations []keyValues, startWg, finishWg *sync.WaitGroup) { defer finishWg.Done() startWg.Done() @@ -864,7 +864,7 @@ func TestTrie_ClearPrefixLimit(t *testing.T) { {0x09}, } - cases := [][]Test{ + cases := [][]keyValues{ { {key: []byte{0x01, 0x35}, value: []byte("pen")}, {key: []byte{0x01, 0x36}, value: []byte("pencil")}, @@ -900,7 +900,7 @@ func TestTrie_ClearPrefixLimit(t *testing.T) { }, } - testFn := func(t *testing.T, testCase []Test, prefix []byte) { + testFn := func(t *testing.T, testCase []keyValues, prefix []byte) { prefixNibbles := codec.KeyLEToNibbles(prefix) if len(prefixNibbles) > 0 && prefixNibbles[len(prefixNibbles)-1] == 0 { prefixNibbles = prefixNibbles[:len(prefixNibbles)-1] @@ -965,7 +965,7 @@ func TestTrie_ClearPrefixLimitSnapshot(t *testing.T) { {0x09}, } - cases := [][]Test{ + cases := [][]keyValues{ { {key: []byte{0x01}, value: []byte("feather")}, },