From 9eb510dfba23685b418f291a0627a1c9796d05d5 Mon Sep 17 00:00:00 2001 From: devopsbo3 <69951731+devopsbo3@users.noreply.github.com> Date: Fri, 10 Nov 2023 12:27:53 -0600 Subject: [PATCH] Revert "trie: remove owner and binary marshaling from stacktrie (#28291)" This reverts commit bc238e6ddb4102c791c02e49e969f09f483ae07a. --- core/state/snapshot/conversion.go | 4 +- core/state/statedb.go | 2 +- eth/protocols/snap/sync.go | 29 +++--- tests/fuzzers/stacktrie/trie_fuzzer.go | 9 +- trie/stacktrie.go | 15 +++- trie/stacktrie_marshalling.go | 120 +++++++++++++++++++++++++ trie/stacktrie_test.go | 48 ++++++++++ trie/trie_test.go | 8 +- 8 files changed, 206 insertions(+), 29 deletions(-) create mode 100644 trie/stacktrie_marshalling.go diff --git a/core/state/snapshot/conversion.go b/core/state/snapshot/conversion.go index 321bfbc6a2dc..1e683f76ce04 100644 --- a/core/state/snapshot/conversion.go +++ b/core/state/snapshot/conversion.go @@ -364,11 +364,11 @@ func generateTrieRoot(db ethdb.KeyValueWriter, scheme string, it Iterator, accou func stackTrieGenerate(db ethdb.KeyValueWriter, scheme string, owner common.Hash, in chan trieKV, out chan common.Hash) { var nodeWriter trie.NodeWriteFunc if db != nil { - nodeWriter = func(path []byte, hash common.Hash, blob []byte) { + nodeWriter = func(owner common.Hash, path []byte, hash common.Hash, blob []byte) { rawdb.WriteTrieNode(db, owner, path, hash, blob, scheme) } } - t := trie.NewStackTrie(nodeWriter) + t := trie.NewStackTrieWithOwner(nodeWriter, owner) for leaf := range in { t.Update(leaf.key[:], leaf.value) } diff --git a/core/state/statedb.go b/core/state/statedb.go index d28cd29b309a..a59de16a70fd 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -964,7 +964,7 @@ func (s *StateDB) fastDeleteStorage(addrHash common.Hash, root common.Hash) (boo nodes = trienode.NewNodeSet(addrHash) slots = make(map[common.Hash][]byte) ) - stack := trie.NewStackTrie(func(path []byte, hash common.Hash, blob []byte) { + stack := trie.NewStackTrie(func(owner common.Hash, path []byte, hash common.Hash, blob []byte) { nodes.AddNode(path, trienode.NewDeleted()) size += common.StorageSize(len(path)) }) diff --git a/eth/protocols/snap/sync.go b/eth/protocols/snap/sync.go index df1473e999af..6a2d92c00991 100644 --- a/eth/protocols/snap/sync.go +++ b/eth/protocols/snap/sync.go @@ -738,8 +738,8 @@ func (s *Syncer) loadSyncStatus() { s.accountBytes += common.StorageSize(len(key) + len(value)) }, } - task.genTrie = trie.NewStackTrie(func(path []byte, hash common.Hash, val []byte) { - rawdb.WriteTrieNode(task.genBatch, common.Hash{}, path, hash, val, s.scheme) + task.genTrie = trie.NewStackTrie(func(owner common.Hash, path []byte, hash common.Hash, val []byte) { + rawdb.WriteTrieNode(task.genBatch, owner, path, hash, val, s.scheme) }) for accountHash, subtasks := range task.SubTasks { for _, subtask := range subtasks { @@ -751,10 +751,9 @@ func (s *Syncer) loadSyncStatus() { s.storageBytes += common.StorageSize(len(key) + len(value)) }, } - owner := accountHash // local assignment for stacktrie writer closure - subtask.genTrie = trie.NewStackTrie(func(path []byte, hash common.Hash, val []byte) { + subtask.genTrie = trie.NewStackTrieWithOwner(func(owner common.Hash, path []byte, hash common.Hash, val []byte) { rawdb.WriteTrieNode(subtask.genBatch, owner, path, hash, val, s.scheme) - }) + }, accountHash) } } } @@ -811,8 +810,8 @@ func (s *Syncer) loadSyncStatus() { Last: last, SubTasks: make(map[common.Hash][]*storageTask), genBatch: batch, - genTrie: trie.NewStackTrie(func(path []byte, hash common.Hash, val []byte) { - rawdb.WriteTrieNode(batch, common.Hash{}, path, hash, val, s.scheme) + genTrie: trie.NewStackTrie(func(owner common.Hash, path []byte, hash common.Hash, val []byte) { + rawdb.WriteTrieNode(batch, owner, path, hash, val, s.scheme) }), }) log.Debug("Created account sync task", "from", next, "last", last) @@ -2005,15 +2004,14 @@ func (s *Syncer) processStorageResponse(res *storageResponse) { s.storageBytes += common.StorageSize(len(key) + len(value)) }, } - owner := account // local assignment for stacktrie writer closure tasks = append(tasks, &storageTask{ Next: common.Hash{}, Last: r.End(), root: acc.Root, genBatch: batch, - genTrie: trie.NewStackTrie(func(path []byte, hash common.Hash, val []byte) { + genTrie: trie.NewStackTrieWithOwner(func(owner common.Hash, path []byte, hash common.Hash, val []byte) { rawdb.WriteTrieNode(batch, owner, path, hash, val, s.scheme) - }), + }, account), }) for r.Next() { batch := ethdb.HookedBatch{ @@ -2027,9 +2025,9 @@ func (s *Syncer) processStorageResponse(res *storageResponse) { Last: r.End(), root: acc.Root, genBatch: batch, - genTrie: trie.NewStackTrie(func(path []byte, hash common.Hash, val []byte) { + genTrie: trie.NewStackTrieWithOwner(func(owner common.Hash, path []byte, hash common.Hash, val []byte) { rawdb.WriteTrieNode(batch, owner, path, hash, val, s.scheme) - }), + }, account), }) } for _, task := range tasks { @@ -2074,10 +2072,9 @@ func (s *Syncer) processStorageResponse(res *storageResponse) { slots += len(res.hashes[i]) if i < len(res.hashes)-1 || res.subTask == nil { - // no need to make local reassignment of account: this closure does not outlive the loop - tr := trie.NewStackTrie(func(path []byte, hash common.Hash, val []byte) { - rawdb.WriteTrieNode(batch, account, path, hash, val, s.scheme) - }) + tr := trie.NewStackTrieWithOwner(func(owner common.Hash, path []byte, hash common.Hash, val []byte) { + rawdb.WriteTrieNode(batch, owner, path, hash, val, s.scheme) + }, account) for j := 0; j < len(res.hashes[i]); j++ { tr.Update(res.hashes[i][j][:], res.slots[i][j]) } diff --git a/tests/fuzzers/stacktrie/trie_fuzzer.go b/tests/fuzzers/stacktrie/trie_fuzzer.go index 20b8ca24b3d2..3d6552409578 100644 --- a/tests/fuzzers/stacktrie/trie_fuzzer.go +++ b/tests/fuzzers/stacktrie/trie_fuzzer.go @@ -140,8 +140,8 @@ func (f *fuzzer) fuzz() int { trieA = trie.NewEmpty(dbA) spongeB = &spongeDb{sponge: sha3.NewLegacyKeccak256()} dbB = trie.NewDatabase(rawdb.NewDatabase(spongeB), nil) - trieB = trie.NewStackTrie(func(path []byte, hash common.Hash, blob []byte) { - rawdb.WriteTrieNode(spongeB, common.Hash{}, path, hash, blob, dbB.Scheme()) + trieB = trie.NewStackTrie(func(owner common.Hash, path []byte, hash common.Hash, blob []byte) { + rawdb.WriteTrieNode(spongeB, owner, path, hash, blob, dbB.Scheme()) }) vals []kv useful bool @@ -205,10 +205,13 @@ func (f *fuzzer) fuzz() int { // Ensure all the nodes are persisted correctly var ( nodeset = make(map[string][]byte) // path -> blob - trieC = trie.NewStackTrie(func(path []byte, hash common.Hash, blob []byte) { + trieC = trie.NewStackTrie(func(owner common.Hash, path []byte, hash common.Hash, blob []byte) { if crypto.Keccak256Hash(blob) != hash { panic("invalid node blob") } + if owner != (common.Hash{}) { + panic("invalid node owner") + } nodeset[string(path)] = common.CopyBytes(blob) }) checked int diff --git a/trie/stacktrie.go b/trie/stacktrie.go index 35208e1cb345..781c8429618e 100644 --- a/trie/stacktrie.go +++ b/trie/stacktrie.go @@ -33,12 +33,13 @@ var ( // NodeWriteFunc is used to provide all information of a dirty node for committing // so that callers can flush nodes into database with desired scheme. -type NodeWriteFunc = func(path []byte, hash common.Hash, blob []byte) +type NodeWriteFunc = func(owner common.Hash, path []byte, hash common.Hash, blob []byte) // StackTrie is a trie implementation that expects keys to be inserted // in order. Once it determines that a subtree will no longer be inserted // into, it will hash it and free up the memory it uses. type StackTrie struct { + owner common.Hash // the owner of the trie writeFn NodeWriteFunc // function for committing nodes, can be nil root *stNode h *hasher @@ -53,6 +54,14 @@ func NewStackTrie(writeFn NodeWriteFunc) *StackTrie { } } +// NewStackTrieWithOwner allocates and initializes an empty trie, but with +// the additional owner field. +func NewStackTrieWithOwner(writeFn NodeWriteFunc, owner common.Hash) *StackTrie { + stack := NewStackTrie(writeFn) + stack.owner = owner + return stack +} + // Update inserts a (key, value) pair into the stack trie. func (t *StackTrie) Update(key, value []byte) error { k := keybytesToHex(key) @@ -362,7 +371,7 @@ func (t *StackTrie) hash(st *stNode, path []byte) { // input values st.val = t.h.hashData(encodedNode) if t.writeFn != nil { - t.writeFn(path, common.BytesToHash(st.val), encodedNode) + t.writeFn(t.owner, path, common.BytesToHash(st.val), encodedNode) } } @@ -407,6 +416,6 @@ func (t *StackTrie) Commit() (h common.Hash, err error) { t.h.sha.Write(st.val) t.h.sha.Read(h[:]) - t.writeFn(nil, h, st.val) + t.writeFn(t.owner, nil, h, st.val) return h, nil } diff --git a/trie/stacktrie_marshalling.go b/trie/stacktrie_marshalling.go new file mode 100644 index 000000000000..c0bb07f8685b --- /dev/null +++ b/trie/stacktrie_marshalling.go @@ -0,0 +1,120 @@ +// Copyright 2023 The go-ethereum Authors +// This file is part of the go-ethereum library. +// +// The go-ethereum library is free software: you can redistribute it and/or modify +// it under the terms of the GNU Lesser General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// The go-ethereum library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with the go-ethereum library. If not, see . + +package trie + +import ( + "bufio" + "bytes" + "encoding" + "encoding/gob" +) + +// Compile-time interface checks. +var ( + _ = encoding.BinaryMarshaler((*StackTrie)(nil)) + _ = encoding.BinaryUnmarshaler((*StackTrie)(nil)) +) + +// NewFromBinaryV2 initialises a serialized stacktrie with the given db. +// OBS! Format was changed along with the name of this constructor. +func NewFromBinaryV2(data []byte) (*StackTrie, error) { + stack := NewStackTrie(nil) + if err := stack.UnmarshalBinary(data); err != nil { + return nil, err + } + return stack, nil +} + +// MarshalBinary implements encoding.BinaryMarshaler. +func (t *StackTrie) MarshalBinary() (data []byte, err error) { + var ( + b bytes.Buffer + w = bufio.NewWriter(&b) + ) + if err := gob.NewEncoder(w).Encode(t.owner); err != nil { + return nil, err + } + if err := t.root.marshalInto(w); err != nil { + return nil, err + } + w.Flush() + return b.Bytes(), nil +} + +// UnmarshalBinary implements encoding.BinaryUnmarshaler. +func (t *StackTrie) UnmarshalBinary(data []byte) error { + r := bytes.NewReader(data) + if err := gob.NewDecoder(r).Decode(&t.owner); err != nil { + return err + } + if err := t.root.unmarshalFrom(r); err != nil { + return err + } + return nil +} + +type stackNodeMarshaling struct { + Typ uint8 + Key []byte + Val []byte +} + +func (n *stNode) marshalInto(w *bufio.Writer) (err error) { + enc := stackNodeMarshaling{ + Typ: n.typ, + Key: n.key, + Val: n.val, + } + if err := gob.NewEncoder(w).Encode(enc); err != nil { + return err + } + for _, child := range n.children { + if child == nil { + w.WriteByte(0) + continue + } + w.WriteByte(1) + if err := child.marshalInto(w); err != nil { + return err + } + } + return nil +} + +func (n *stNode) unmarshalFrom(r *bytes.Reader) error { + var dec stackNodeMarshaling + if err := gob.NewDecoder(r).Decode(&dec); err != nil { + return err + } + n.typ = dec.Typ + n.key = dec.Key + n.val = dec.Val + + for i := range n.children { + if b, err := r.ReadByte(); err != nil { + return err + } else if b == 0 { + continue + } + var child stNode + if err := child.unmarshalFrom(r); err != nil { + return err + } + n.children[i] = &child + } + return nil +} diff --git a/trie/stacktrie_test.go b/trie/stacktrie_test.go index 0e52781c62ec..5b86a971e10c 100644 --- a/trie/stacktrie_test.go +++ b/trie/stacktrie_test.go @@ -376,3 +376,51 @@ func TestStacktrieNotModifyValues(t *testing.T) { } } } + +// TestStacktrieSerialization tests that the stacktrie works well if we +// serialize/unserialize it a lot +func TestStacktrieSerialization(t *testing.T) { + var ( + st = NewStackTrieWithOwner(nil, common.Hash{0x12}) + nt = NewEmpty(NewDatabase(rawdb.NewMemoryDatabase(), nil)) + keyB = big.NewInt(1) + keyDelta = big.NewInt(1) + vals [][]byte + keys [][]byte + ) + getValue := func(i int) []byte { + if i%2 == 0 { // large + return crypto.Keccak256(big.NewInt(int64(i)).Bytes()) + } else { //small + return big.NewInt(int64(i)).Bytes() + } + } + for i := 0; i < 10; i++ { + vals = append(vals, getValue(i)) + keys = append(keys, common.BigToHash(keyB).Bytes()) + keyB = keyB.Add(keyB, keyDelta) + keyDelta.Add(keyDelta, common.Big1) + } + for i, k := range keys { + nt.Update(k, common.CopyBytes(vals[i])) + } + + for i, k := range keys { + blob, err := st.MarshalBinary() + if err != nil { + t.Fatal(err) + } + newSt, err := NewFromBinaryV2(blob) + if err != nil { + t.Fatal(err) + } + st = newSt + st.Update(k, common.CopyBytes(vals[i])) + } + if have, want := st.Hash(), nt.Hash(); have != want { + t.Fatalf("have %#x want %#x", have, want) + } + if have, want := st.owner, (common.Hash{0x12}); have != want { + t.Fatalf("have %#x want %#x", have, want) + } +} diff --git a/trie/trie_test.go b/trie/trie_test.go index 2dfe81ef81d2..8078770e7a37 100644 --- a/trie/trie_test.go +++ b/trie/trie_test.go @@ -912,8 +912,8 @@ func TestCommitSequenceStackTrie(t *testing.T) { trie := NewEmpty(db) // Another sponge is used for the stacktrie commits stackTrieSponge := &spongeDb{sponge: sha3.NewLegacyKeccak256(), id: "b"} - stTrie := NewStackTrie(func(path []byte, hash common.Hash, blob []byte) { - rawdb.WriteTrieNode(stackTrieSponge, common.Hash{}, path, hash, blob, db.Scheme()) + stTrie := NewStackTrie(func(owner common.Hash, path []byte, hash common.Hash, blob []byte) { + rawdb.WriteTrieNode(stackTrieSponge, owner, path, hash, blob, db.Scheme()) }) // Fill the trie with elements for i := 0; i < count; i++ { @@ -971,8 +971,8 @@ func TestCommitSequenceSmallRoot(t *testing.T) { trie := NewEmpty(db) // Another sponge is used for the stacktrie commits stackTrieSponge := &spongeDb{sponge: sha3.NewLegacyKeccak256(), id: "b"} - stTrie := NewStackTrie(func(path []byte, hash common.Hash, blob []byte) { - rawdb.WriteTrieNode(stackTrieSponge, common.Hash{}, path, hash, blob, db.Scheme()) + stTrie := NewStackTrie(func(owner common.Hash, path []byte, hash common.Hash, blob []byte) { + rawdb.WriteTrieNode(stackTrieSponge, owner, path, hash, blob, db.Scheme()) }) // Add a single small-element to the trie(s) key := make([]byte, 5)