trie: nits and polishes

trie/bytepool.go

@@ -1,20 +1,39 @@
+// Copyright 2024 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
 
-type bytepool struct {
+// bytesPool is a pool for byteslices. It is safe for concurrent use.
+type bytesPool struct {
 	c chan []byte
 	w int
-	h int
 }
 
-func newByteslicepool(sliceCap, nitems int) *bytepool {
-	b := &bytepool{
+// newBytesPool creates a new bytesPool. The sliceCap sets the capacity of
+// newly allocated slices, and the nitems determines how many items the pool
+// will hold, at maximum.
+func newBytesPool(sliceCap, nitems int) *bytesPool {
+	return &bytesPool{
 		c: make(chan []byte, nitems),
 		w: sliceCap,
 	}
-	return b
 }
 
-func (bp *bytepool) Get() []byte {
+// Get returns a slice. Safe for concurrent use.
+func (bp *bytesPool) Get() []byte {
 	select {
 	case b := <-bp.c:
 		return b
@@ -23,13 +42,10 @@ func (bp *bytepool) Get() []byte {
 	}
 }
 
-func (bp *bytepool) Put(b []byte) {
-	// Ignore too small slices
-	if cap(b) < bp.w {
-		return
-	}
-	// Don't retain too large slices either
-	if cap(b) > 3*bp.w {
+// Put returns a slice to the pool. Safe for concurrent use. This method
+// will ignore slices that are too small or too large (>3x the cap)
+func (bp *bytesPool) Put(b []byte) {
+	if c := cap(b); c < bp.w || c > 3*bp.w {
 		return
 	}
 	select {

trie/hasher.go

@@ -188,12 +188,12 @@ func (h *hasher) hashData(data []byte) hashNode {
 	return n
 }
 
-// hashDataTo hashes the provided data to the dest buffer (must be at least
-// 32 byte large)
-func (h *hasher) hashDataTo(data []byte, dest []byte) {
+// hashDataTo hashes the provided data to the given destination buffer. The caller
+// must ensure that the dst buffer is of appropriate size.
+func (h *hasher) hashDataTo(dst, data []byte) {
 	h.sha.Reset()
 	h.sha.Write(data)
-	h.sha.Read(dest)
+	h.sha.Read(dst)
 }
 
 // proofHash is used to construct trie proofs, and returns the 'collapsed'

trie/node.go

@@ -135,19 +135,6 @@ func (n valueNode) fstring(ind string) string {
 	return fmt.Sprintf("%x ", []byte(n))
 }
 
-// rawNode is a simple binary blob used to differentiate between collapsed trie
-// nodes and already encoded RLP binary blobs (while at the same time store them
-// in the same cache fields).
-type rawNode []byte
-
-func (n rawNode) cache() (hashNode, bool)   { panic("this should never end up in a live trie") }
-func (n rawNode) fstring(ind string) string { panic("this should never end up in a live trie") }
-
-func (n rawNode) EncodeRLP(w io.Writer) error {
-	_, err := w.Write(n)
-	return err
-}
-
 // mustDecodeNode is a wrapper of decodeNode and panic if any error is encountered.
 func mustDecodeNode(hash, buf []byte) node {
 	n, err := decodeNode(hash, buf)

trie/node_enc.go

@@ -86,7 +86,3 @@ func (n hashNode) encode(w rlp.EncoderBuffer) {
 func (n valueNode) encode(w rlp.EncoderBuffer) {
 	w.WriteBytes(n)
 }
-
-func (n rawNode) encode(w rlp.EncoderBuffer) {
-	w.Write(n)
-}

trie/stacktrie.go

@@ -27,7 +27,7 @@ import (
 
 var (
 	stPool = sync.Pool{New: func() any { return new(stNode) }}
-	bPool  = newByteslicepool(32, 100)
+	bPool  = newBytesPool(32, 100)
 	_      = types.TrieHasher((*StackTrie)(nil))
 )
 
@@ -48,9 +48,8 @@ type StackTrie struct {
 	h          *hasher
 	last       []byte
 	onTrieNode OnTrieNode
-
-	keyScratch  []byte
-	pathScratch []byte
+	kBuf       []byte // buf space used for hex-key during insertions
+	pBuf       []byte // buf space used for path during insertions
 }
 
 // NewStackTrie allocates and initializes an empty trie. The committed nodes
@@ -60,8 +59,8 @@ func NewStackTrie(onTrieNode OnTrieNode) *StackTrie {
 		root:       stPool.Get().(*stNode),
 		h:          newHasher(false),
 		onTrieNode: onTrieNode,
-		keyScratch:  make([]byte, 0, 32),
-		pathScratch: make([]byte, 0, 32),
+		kBuf:       make([]byte, 0, 64),
+		pBuf:       make([]byte, 0, 32),
 	}
 }
 
@@ -71,16 +70,14 @@ func (t *StackTrie) Update(key, value []byte) error {
 		return errors.New("trying to insert empty (deletion)")
 	}
 	var k []byte
-	{
-		// We can reuse the key scratch area, but only if the insert-method
-		// never holds on to it.
-		if cap(t.keyScratch) < 2*len(key) { // realloc to ensure sufficient cap
-			t.keyScratch = make([]byte, 2*len(key), 2*len(key))
+	{ // Need to expand the 'key' into hex-form. We use the dedicated buf for that.
+		if cap(t.kBuf) < 2*len(key) { // realloc to ensure sufficient cap
+			t.kBuf = make([]byte, 2*len(key))
 		}
 		// resize to ensure correct size
-		t.keyScratch = t.keyScratch[:2*len(key)]
-		writeHexKey(t.keyScratch, key)
-		k = t.keyScratch
+		t.kBuf = t.kBuf[:2*len(key)]
+		writeHexKey(t.kBuf, key)
+		k = t.kBuf
 	}
 	if bytes.Compare(t.last, k) >= 0 {
 		return errors.New("non-ascending key order")
@@ -90,7 +87,7 @@ func (t *StackTrie) Update(key, value []byte) error {
 	} else {
 		t.last = append(t.last[:0], k...) // reuse key slice
 	}
-	t.insert(t.root, k, value, t.pathScratch[:0])
+	t.insert(t.root, k, value, t.pBuf[:0])
 	return nil
 }
 
@@ -173,9 +170,11 @@ func (n *stNode) getDiffIndex(key []byte) int {
 	return len(n.key)
 }
 
-// Helper function to that inserts a (key, value) pair into
-// the trie.
-// The key is not retained by this method, but always copied if needed.
+// Helper function to that inserts a (key, value) pair into the trie.
+//   - The key is not retained by this method, but always copied if needed.
+//   - The value is retained by this method, as long as the leaf that it represents
+//     remains unhashed. However: it is never modified.
+//   - The path is not retained by this method.
 func (t *StackTrie) insert(st *stNode, key, value []byte, path []byte) {
 	switch st.typ {
 	case branchNode: /* Branch */
@@ -408,7 +407,7 @@ func (t *StackTrie) hash(st *stNode, path []byte) {
 	// input values.
 	val := bPool.Get()
 	val = val[:32]
-	t.h.hashDataTo(blob, val)
+	t.h.hashDataTo(val, blob)
 	st.val = val
 
 	// Invoke the callback it's provided. Notably, the path and blob slices are