trie: remove owner and binary marshaling from stacktrie (#28291)

This change
  - Removes the owner-notion from a stacktrie; the owner is only ever needed for comitting to the database, but the commit-function, the `writeFn` is provided by the caller, so the caller can just set the owner into the `writeFn` instead of having it passed through the stacktrie.
  - Removes the `encoding.BinaryMarshaler`/`encoding.BinaryUnmarshaler` interface from stacktrie. We're not using it, and it is doubtful whether anyone downstream is either.

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(owner common.Hash, path []byte, hash common.Hash, blob []byte) {
+		nodeWriter = func(path []byte, hash common.Hash, blob []byte) {
 			rawdb.WriteTrieNode(db, owner, path, hash, blob, scheme)
 		}
 	}
-	t := trie.NewStackTrieWithOwner(nodeWriter, owner)
+	t := trie.NewStackTrie(nodeWriter)
 	for leaf := range in {
 		t.Update(leaf.key[:], leaf.value)
 	}

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(owner common.Hash, path []byte, hash common.Hash, blob []byte) {
+	stack := trie.NewStackTrie(func(path []byte, hash common.Hash, blob []byte) {
 		nodes.AddNode(path, trienode.NewDeleted())
 		size += common.StorageSize(len(path))
 	})

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(owner common.Hash, path []byte, hash common.Hash, val []byte) {
+				task.genTrie = trie.NewStackTrie(func(path []byte, hash common.Hash, val []byte) {
-					rawdb.WriteTrieNode(task.genBatch, owner, path, hash, val, s.scheme)
+					rawdb.WriteTrieNode(task.genBatch, common.Hash{}, path, hash, val, s.scheme)
 				})
 				for accountHash, subtasks := range task.SubTasks {
 					for _, subtask := range subtasks {
@@ -751,9 +751,10 @@ func (s *Syncer) loadSyncStatus() {
 								s.storageBytes += common.StorageSize(len(key) + len(value))
 							},
 						}
-						subtask.genTrie = trie.NewStackTrieWithOwner(func(owner common.Hash, path []byte, hash common.Hash, val []byte) {
+						owner := accountHash // local assignment for stacktrie writer closure
+						subtask.genTrie = trie.NewStackTrie(func(path []byte, hash common.Hash, val []byte) {
 							rawdb.WriteTrieNode(subtask.genBatch, owner, path, hash, val, s.scheme)
-						}, accountHash)
+						})
 					}
 				}
 			}
@@ -810,8 +811,8 @@ func (s *Syncer) loadSyncStatus() {
 			Last:     last,
 			SubTasks: make(map[common.Hash][]*storageTask),
 			genBatch: batch,
-			genTrie: trie.NewStackTrie(func(owner common.Hash, path []byte, hash common.Hash, val []byte) {
+			genTrie: trie.NewStackTrie(func(path []byte, hash common.Hash, val []byte) {
-				rawdb.WriteTrieNode(batch, owner, path, hash, val, s.scheme)
+				rawdb.WriteTrieNode(batch, common.Hash{}, path, hash, val, s.scheme)
 			}),
 		})
 		log.Debug("Created account sync task", "from", next, "last", last)
@@ -2004,14 +2005,15 @@ 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.NewStackTrieWithOwner(func(owner common.Hash, path []byte, hash common.Hash, val []byte) {
+						genTrie: trie.NewStackTrie(func(path []byte, hash common.Hash, val []byte) {
 							rawdb.WriteTrieNode(batch, owner, path, hash, val, s.scheme)
-						}, account),
+						}),
 					})
 					for r.Next() {
 						batch := ethdb.HookedBatch{
@@ -2025,9 +2027,9 @@ func (s *Syncer) processStorageResponse(res *storageResponse) {
 							Last:     r.End(),
 							root:     acc.Root,
 							genBatch: batch,
-							genTrie: trie.NewStackTrieWithOwner(func(owner common.Hash, path []byte, hash common.Hash, val []byte) {
+							genTrie: trie.NewStackTrie(func(path []byte, hash common.Hash, val []byte) {
 								rawdb.WriteTrieNode(batch, owner, path, hash, val, s.scheme)
-							}, account),
+							}),
 						})
 					}
 					for _, task := range tasks {
@@ -2072,9 +2074,10 @@ func (s *Syncer) processStorageResponse(res *storageResponse) {
 		slots += len(res.hashes[i])
 
 		if i < len(res.hashes)-1 || res.subTask == nil {
-			tr := trie.NewStackTrieWithOwner(func(owner common.Hash, path []byte, hash common.Hash, val []byte) {
+			// no need to make local reassignment of account: this closure does not outlive the loop
-				rawdb.WriteTrieNode(batch, owner, path, hash, val, s.scheme)
+			tr := trie.NewStackTrie(func(path []byte, hash common.Hash, val []byte) {
-			}, account)
+				rawdb.WriteTrieNode(batch, account, path, hash, val, s.scheme)
+			})
 			for j := 0; j < len(res.hashes[i]); j++ {
 				tr.Update(res.hashes[i][j][:], res.slots[i][j])
 			}

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(owner common.Hash, path []byte, hash common.Hash, blob []byte) {
+		trieB   = trie.NewStackTrie(func(path []byte, hash common.Hash, blob []byte) {
-			rawdb.WriteTrieNode(spongeB, owner, path, hash, blob, dbB.Scheme())
+			rawdb.WriteTrieNode(spongeB, common.Hash{}, path, hash, blob, dbB.Scheme())
 		})
 		vals        []kv
 		useful      bool
@@ -205,13 +205,10 @@ func (f *fuzzer) fuzz() int {
 	// Ensure all the nodes are persisted correctly
 	var (
 		nodeset = make(map[string][]byte) // path -> blob
-		trieC   = trie.NewStackTrie(func(owner common.Hash, path []byte, hash common.Hash, blob []byte) {
+		trieC   = trie.NewStackTrie(func(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

trie/stacktrie.go

@@ -33,13 +33,12 @@ 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(owner common.Hash, path []byte, hash common.Hash, blob []byte)
+type NodeWriteFunc = func(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
@@ -54,14 +53,6 @@ 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)
@@ -371,7 +362,7 @@ func (t *StackTrie) hash(st *stNode, path []byte) {
 	// input values
 	st.val = t.h.hashData(encodedNode)
 	if t.writeFn != nil {
-		t.writeFn(t.owner, path, common.BytesToHash(st.val), encodedNode)
+		t.writeFn(path, common.BytesToHash(st.val), encodedNode)
 	}
 }
 
@@ -416,6 +407,6 @@ func (t *StackTrie) Commit() (h common.Hash, err error) {
 	t.h.sha.Write(st.val)
 	t.h.sha.Read(h[:])
 
-	t.writeFn(t.owner, nil, h, st.val)
+	t.writeFn(nil, h, st.val)
 	return h, nil
 }

trie/stacktrie_marshalling.go

@@ -1,120 +0,0 @@
-// 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 <http://www.gnu.org/licenses/>.
-
-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
-}

trie/stacktrie_test.go

@@ -376,51 +376,3 @@ 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)
-	}
-}

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(owner common.Hash, path []byte, hash common.Hash, blob []byte) {
+		stTrie := NewStackTrie(func(path []byte, hash common.Hash, blob []byte) {
-			rawdb.WriteTrieNode(stackTrieSponge, owner, path, hash, blob, db.Scheme())
+			rawdb.WriteTrieNode(stackTrieSponge, common.Hash{}, 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(owner common.Hash, path []byte, hash common.Hash, blob []byte) {
+	stTrie := NewStackTrie(func(path []byte, hash common.Hash, blob []byte) {
-		rawdb.WriteTrieNode(stackTrieSponge, owner, path, hash, blob, db.Scheme())
+		rawdb.WriteTrieNode(stackTrieSponge, common.Hash{}, path, hash, blob, db.Scheme())
 	})
 	// Add a single small-element to the trie(s)
 	key := make([]byte, 5)