review from Bo-chan + Feli-chan

trie/bintrie/internal_node_test.go

@@ -290,10 +290,7 @@ func TestInternalNodeCollectNodes(t *testing.T) {
 		collectedPaths = append(collectedPaths, pathCopy)
 	}
 
-	err := s.collectNodes(s.root, []byte{1}, flushFn, 8)
+	s.collectNodes(s.root, []byte{1}, flushFn, 8)
-	if err != nil {
-		t.Fatalf("Failed to collect nodes: %v", err)
-	}
 
 	// Should have collected 3 nodes: left stem, right stem, and the internal node itself
 	if len(collectedPaths) != 3 {

trie/bintrie/stem_node_test.go

@@ -320,10 +320,7 @@ func TestStemNodeCollectNodes(t *testing.T) {
 		collectedPaths = append(collectedPaths, pathCopy)
 	}
 
-	err := s.collectNodes(s.root, []byte{0, 1, 0}, flushFn, 8)
+	s.collectNodes(s.root, []byte{0, 1, 0}, flushFn, 8)
-	if err != nil {
-		t.Fatalf("Failed to collect nodes: %v", err)
-	}
 
 	// Should have collected one node (itself)
 	if len(collectedPaths) != 1 {

trie/bintrie/store_commit.go

@@ -340,45 +340,35 @@ func (s *nodeStore) decodeNode(serialized []byte, depth int, hn common.Hash, mus
 // CollectNodes flushes every node that needs flushing via flushfn in post-order.
 // Invariant: any ancestor of a node that needs flushing is itself marked, so a
 // clean root means the whole subtree is clean.
-func (s *nodeStore) collectNodes(ref nodeRef, path []byte, flushfn nodeFlushFn, groupDepth int) error {
+func (s *nodeStore) collectNodes(ref nodeRef, path []byte, flushfn nodeFlushFn, groupDepth int) {
-	if groupDepth < 1 || groupDepth > MaxGroupDepth {
-		return fmt.Errorf("groupDepth %d,  must be between 1 and 8", groupDepth)
-	}
-
 	switch ref.Kind() {
-	case kindEmpty:
-		return nil
 	case kindInternal:
 		node := s.getInternal(ref.Index())
 		if !node.dirty {
-			return nil
+			return
 		}
 		// Only flush at group boundaries (depth % groupDepth == 0)
 		if int(node.depth)%groupDepth == 0 {
 			// We're at a group boundary - first collect any nodes in deeper groups,
 			// then flush this group
-			if err := s.collectChildGroups(node, path, flushfn, groupDepth, groupDepth-1); err != nil {
+			s.collectChildGroups(node, path, flushfn, groupDepth, groupDepth-1)
-				return err
-			}
 			flushfn(path, s.computeHash(ref), s.serializeNode(ref, groupDepth))
 			node.dirty = false
-			return nil
+			return
 		}
 		// Not at a group boundary - this shouldn't happen if we're called correctly from root
 		// but handle it by continuing to traverse
-		return s.collectChildGroups(node, path, flushfn, groupDepth, groupDepth-(int(node.depth)%groupDepth)-1)
+		s.collectChildGroups(node, path, flushfn, groupDepth, groupDepth-(int(node.depth)%groupDepth)-1)
 	case kindStem:
 		sn := s.getStem(ref.Index())
 		if !sn.dirty {
-			return nil
+			return
 		}
 		flushfn(path, s.computeHash(ref), s.serializeNode(ref, groupDepth))
 		sn.dirty = false
-		return nil
+	case kindHashed, kindEmpty:
-	case kindHashed:
-		return nil // Already committed
 	default:
-		return fmt.Errorf("CollectNodes: unexpected kind %d", ref.Kind())
+		panic(fmt.Sprintf("CollectNodes: unexpected kind %d", ref.Kind()))
 	}
 }
 
@@ -389,21 +379,14 @@ func (s *nodeStore) collectChildGroups(node *InternalNode, path []byte, flushfn
 	if remainingLevels == 0 {
 		// Current node is at depth (groupBoundary - 1), its children are at the next group boundary
 		if !node.left.IsEmpty() {
-			if err := s.collectNodes(node.left, appendBit(path, 0), flushfn, groupDepth); err != nil {
+			s.collectNodes(node.left, appendBit(path, 0), flushfn, groupDepth)
-				return err
-			}
 		}
 		if !node.right.IsEmpty() {
-			if err := s.collectNodes(node.right, appendBit(path, 1), flushfn, groupDepth); err != nil {
+			s.collectNodes(node.right, appendBit(path, 1), flushfn, groupDepth)
-				return err
-			}
 		}
 		return nil
 	}
 
-	// Continue traversing within the group.
-	childDepth := node.depth + 1
-
 	if !node.left.IsEmpty() {
 		switch node.left.Kind() {
 		case kindInternal:
@@ -412,10 +395,8 @@ func (s *nodeStore) collectChildGroups(node *InternalNode, path []byte, flushfn
 				return err
 			}
 		default:
-			extPath := s.extendPathToGroupLeaf(appendBit(path, 0), node.left, remainingLevels, int(childDepth))
+			extPath := s.extendPathToGroupLeaf(appendBit(path, 0), node.left, remainingLevels)
-			if err := s.collectNodes(node.left, extPath, flushfn, groupDepth); err != nil {
+			s.collectNodes(node.left, extPath, flushfn, groupDepth)
-				return err
-			}
 		}
 	}
 	if !node.right.IsEmpty() {
@@ -426,10 +407,8 @@ func (s *nodeStore) collectChildGroups(node *InternalNode, path []byte, flushfn
 				return err
 			}
 		default:
-			extPath := s.extendPathToGroupLeaf(appendBit(path, 1), node.right, remainingLevels, int(childDepth))
+			extPath := s.extendPathToGroupLeaf(appendBit(path, 1), node.right, remainingLevels)
-			if err := s.collectNodes(node.right, extPath, flushfn, groupDepth); err != nil {
+			s.collectNodes(node.right, extPath, flushfn, groupDepth)
-				return err
-			}
 		}
 	}
 	return nil
@@ -439,20 +418,20 @@ func (s *nodeStore) collectChildGroups(node *InternalNode, path []byte, flushfn
 // matching the projection done by serializeSubtree. For StemNodes, the path
 // is extended using the stem's key bits (same as serializeSubtree). For other
 // node types, the path is extended with all-zero (left) bits.
-func (s *nodeStore) extendPathToGroupLeaf(path []byte, node nodeRef, remainingLevels int, absoluteDepth int) []byte {
+func (s *nodeStore) extendPathToGroupLeaf(path []byte, node nodeRef, remainingLevels int) []byte {
 	if remainingLevels <= 0 {
 		return path
 	}
 	if node.Kind() == kindStem {
 		sn := s.getStem(node.Index())
-		for d := 0; d < remainingLevels; d++ {
+		for _ = range remainingLevels {
-			bit := sn.Stem[(absoluteDepth+d)/8] >> (7 - ((absoluteDepth + d) % 8)) & 1
+			bit := sn.Stem[len(path)/8] >> (7 - (len(path) % 8)) & 1
 			path = appendBit(path, bit)
 		}
 	} else {
 		// HashedNode or other: all-left extension (matches serializeSubtree's
 		// position << remainingDepth behavior).
-		for d := 0; d < remainingLevels; d++ {
+		for _ = range remainingLevels {
 			path = appendBit(path, 0)
 		}
 	}

trie/bintrie/trie.go

@@ -322,12 +322,9 @@ func (t *BinaryTrie) Commit(_ bool) (common.Hash, *trienode.NodeSet) {
 	// Pre-size the path buffer: collectNodes reuses it in-place via
 	// append/truncate; 32 covers typical binary-trie depth without regrowth.
 	pathBuf := make([]byte, 0, 32)
-	err := t.store.collectNodes(t.store.root, pathBuf, func(path []byte, hash common.Hash, serialized []byte) {
+	t.store.collectNodes(t.store.root, pathBuf, func(path []byte, hash common.Hash, serialized []byte) {
 		nodeset.AddNode(path, trienode.NewNodeWithPrev(hash, serialized, t.tracer.Get(path)))
 	}, t.groupDepth)
-	if err != nil {
-		panic(fmt.Errorf("CollectNodes failed: %v", err))
-	}
 	return t.Hash(), nodeset
 }