core/state, trie: Rewrite NodeIterator as an interface

core/state/iterator.go

@@ -31,15 +31,14 @@ import (
 type NodeIterator struct {
 	state *StateDB // State being iterated
 
-	stateIt *trie.NodeIterator // Primary iterator for the global state trie
+	stateIt trie.NodeIterator // Primary iterator for the global state trie
-	dataIt  *trie.NodeIterator // Secondary iterator for the data trie of a contract
+	dataIt  trie.NodeIterator // Secondary iterator for the data trie of a contract
 
 	accountHash common.Hash // Hash of the node containing the account
 	codeHash    common.Hash // Hash of the contract source code
 	code        []byte      // Source code associated with a contract
 
 	Hash   common.Hash // Hash of the current entry being iterated (nil if not standalone)
-	Entry  interface{} // Current state entry being iterated (internal representation)
 	Parent common.Hash // Hash of the first full ancestor node (nil if current is the root)
 
 	Error error // Failure set in case of an internal error in the iterator
@@ -81,8 +80,8 @@ func (it *NodeIterator) step() error {
 	// If we had data nodes previously, we surely have at least state nodes
 	if it.dataIt != nil {
 		if cont := it.dataIt.Next(true); !cont {
-			if it.dataIt.Error != nil {
+			if it.dataIt.Error() != nil {
-				return it.dataIt.Error
+				return it.dataIt.Error()
 			}
 			it.dataIt = nil
 		}
@@ -95,14 +94,14 @@ func (it *NodeIterator) step() error {
 	}
 	// Step to the next state trie node, terminating if we're out of nodes
 	if cont := it.stateIt.Next(true); !cont {
-		if it.stateIt.Error != nil {
+		if it.stateIt.Error() != nil {
-			return it.stateIt.Error
+			return it.stateIt.Error()
 		}
 		it.state, it.stateIt = nil, nil
 		return nil
 	}
 	// If the state trie node is an internal entry, leave as is
-	if !it.stateIt.Leaf {
+	if !it.stateIt.Leaf() {
 		return nil
 	}
 	// Otherwise we've reached an account node, initiate data iteration
@@ -112,7 +111,7 @@ func (it *NodeIterator) step() error {
 		Root     common.Hash
 		CodeHash []byte
 	}
-	if err := rlp.Decode(bytes.NewReader(it.stateIt.LeafBlob), &account); err != nil {
+	if err := rlp.Decode(bytes.NewReader(it.stateIt.LeafBlob()), &account); err != nil {
 		return err
 	}
 	dataTrie, err := trie.New(account.Root, it.state.db)
@@ -130,7 +129,7 @@ func (it *NodeIterator) step() error {
 			return fmt.Errorf("code %x: %v", account.CodeHash, err)
 		}
 	}
-	it.accountHash = it.stateIt.Parent
+	it.accountHash = it.stateIt.Parent()
 	return nil
 }
 
@@ -138,7 +137,7 @@ func (it *NodeIterator) step() error {
 // The method returns whether there are any more data left for inspection.
 func (it *NodeIterator) retrieve() bool {
 	// Clear out any previously set values
-	it.Hash, it.Entry = common.Hash{}, nil
+	it.Hash = common.Hash{}
 
 	// If the iteration's done, return no available data
 	if it.state == nil {
@@ -147,14 +146,14 @@ func (it *NodeIterator) retrieve() bool {
 	// Otherwise retrieve the current entry
 	switch {
 	case it.dataIt != nil:
-		it.Hash, it.Entry, it.Parent = it.dataIt.Hash, it.dataIt.Node, it.dataIt.Parent
+		it.Hash, it.Parent = it.dataIt.Hash(), it.dataIt.Parent()
 		if it.Parent == (common.Hash{}) {
 			it.Parent = it.accountHash
 		}
 	case it.code != nil:
-		it.Hash, it.Entry, it.Parent = it.codeHash, it.code, it.accountHash
+		it.Hash, it.Parent = it.codeHash, it.accountHash
 	case it.stateIt != nil:
-		it.Hash, it.Entry, it.Parent = it.stateIt.Hash, it.stateIt.Node, it.stateIt.Parent
+		it.Hash, it.Parent = it.stateIt.Hash(), it.stateIt.Parent()
 	}
 	return true
 }

trie/iterator.go

@@ -21,7 +21,7 @@ import "github.com/ethereum/go-ethereum/common"
 // Iterator is a key-value trie iterator that traverses a Trie.
 type Iterator struct {
 	trie   *Trie
-	nodeIt *NodeIterator
+	nodeIt NodeIterator
 
 	Key   []byte // Current data key on which the iterator is positioned on
 	Value []byte // Current data value on which the iterator is positioned on
@@ -39,9 +39,9 @@ func NewIterator(trie *Trie) *Iterator {
 // Next moves the iterator forward one key-value entry.
 func (it *Iterator) Next() bool {
 	for it.nodeIt.Next(true) {
-		if it.nodeIt.Leaf {
+		if it.nodeIt.Leaf() {
-			it.Key = decodeCompact(it.nodeIt.path)
+			it.Key = decodeCompact(it.nodeIt.Path())
-			it.Value = it.nodeIt.LeafBlob
+			it.Value = it.nodeIt.LeafBlob()
 			return true
 		}
 	}
@@ -50,6 +50,17 @@ func (it *Iterator) Next() bool {
 	return false
 }
 
+// NodeIterator is an iterator to traverse the trie pre-order.
+type NodeIterator interface {
+	Hash() common.Hash
+	Parent() common.Hash
+	Leaf() bool
+	LeafBlob() []byte
+	Path() []byte
+	Next(bool) bool
+	Error() error
+}
+
 // nodeIteratorState represents the iteration state at one particular node of the
 // trie, which can be resumed at a later invocation.
 type nodeIteratorState struct {
@@ -60,48 +71,92 @@ type nodeIteratorState struct {
 	pathlen int         // Length of the path to this node
 }
 
-// NodeIterator is an iterator to traverse the trie post-order.
+type nodeIterator struct {
-type NodeIterator struct {
 	trie  *Trie                // Trie being iterated
 	stack []*nodeIteratorState // Hierarchy of trie nodes persisting the iteration state
 
-	Hash     common.Hash // Hash of the current node being iterated (nil if not standalone)
+	err error // Failure set in case of an internal error in the iterator
-	Node     node        // Current node being iterated (internal representation)
-	Parent   common.Hash // Hash of the first full ancestor node (nil if current is the root)
-	Leaf     bool        // Flag whether the current node is a value (data) node
-	LeafBlob []byte      // Data blob contained within a leaf (otherwise nil)
-	path     []byte      // Path to the current node
 
-	Error error // Failure set in case of an internal error in the iterator
+	path []byte // Path to the current node
 }
 
 // NewNodeIterator creates an post-order trie iterator.
-func NewNodeIterator(trie *Trie) *NodeIterator {
+func NewNodeIterator(trie *Trie) NodeIterator {
 	if trie.Hash() == emptyState {
-		return new(NodeIterator)
+		return new(nodeIterator)
 	}
-	return &NodeIterator{trie: trie}
+	return &nodeIterator{trie: trie}
+}
+
+// Hash returns the hash of the current node
+func (it *nodeIterator) Hash() common.Hash {
+	if it.trie == nil {
+		return common.Hash{}
+	}
+
+	return it.stack[len(it.stack)-1].hash
+}
+
+// Parent returns the hash of the parent node
+func (it *nodeIterator) Parent() common.Hash {
+	if it.trie == nil {
+		return common.Hash{}
+	}
+
+	return it.stack[len(it.stack)-1].parent
+}
+
+// Leaf returns true if the current node is a leaf
+func (it *nodeIterator) Leaf() bool {
+	if it.trie == nil {
+		return false
+	}
+
+	_, ok := it.stack[len(it.stack)-1].node.(valueNode)
+	return ok
+}
+
+// LeafBlob returns the data for the current node, if it is a leaf
+func (it *nodeIterator) LeafBlob() []byte {
+	if it.trie == nil {
+		return nil
+	}
+
+	if node, ok := it.stack[len(it.stack)-1].node.(valueNode); ok {
+		return []byte(node)
+	}
+	return nil
+}
+
+// Path returns the hex-encoded path to the current node
+func (it *nodeIterator) Path() []byte {
+	return it.path
+}
+
+// Error returns the error set in case of an internal error in the iterator
+func (it *nodeIterator) Error() error {
+	return it.err
 }
 
 // Next moves the iterator to the next node, returning whether there are any
 // further nodes. In case of an internal error this method returns false and
 // sets the Error field to the encountered failure. If `children` is false,
 // skips iterating over any subnodes of the current node.
-func (it *NodeIterator) Next(children bool) bool {
+func (it *nodeIterator) Next(children bool) bool {
 	// If the iterator failed previously, don't do anything
-	if it.Error != nil {
+	if it.err != nil {
 		return false
 	}
 	// Otherwise step forward with the iterator and report any errors
 	if err := it.step(children); err != nil {
-		it.Error = err
+		it.err = err
 		return false
 	}
-	return it.retrieve()
+	return it.trie != nil
 }
 
 // step moves the iterator to the next node of the trie.
-func (it *NodeIterator) step(children bool) error {
+func (it *nodeIterator) step(children bool) error {
 	if it.trie == nil {
 		// Abort if we reached the end of the iteration
 		return nil
@@ -189,26 +244,3 @@ func (it *NodeIterator) step(children bool) error {
 	}
 	return nil
 }
-
-// retrieve pulls and caches the current trie node the iterator is traversing.
-// In case of a value node, the additional leaf blob is also populated with the
-// data contents for external interpretation.
-//
-// The method returns whether there are any more data left for inspection.
-func (it *NodeIterator) retrieve() bool {
-	// Clear out any previously set values
-	it.Hash, it.Node, it.Parent, it.Leaf, it.LeafBlob = common.Hash{}, nil, common.Hash{}, false, nil
-
-	// If the iteration's done, return no available data
-	if it.trie == nil {
-		return false
-	}
-	// Otherwise retrieve the current node and resolve leaf accessors
-	state := it.stack[len(it.stack)-1]
-
-	it.Hash, it.Node, it.Parent = state.hash, state.node, state.parent
-	if value, ok := it.Node.(valueNode); ok {
-		it.Leaf, it.LeafBlob = true, []byte(value)
-	}
-	return true
-}

trie/iterator_test.go

@@ -100,8 +100,8 @@ func TestNodeIteratorCoverage(t *testing.T) {
 	// Gather all the node hashes found by the iterator
 	hashes := make(map[common.Hash]struct{})
 	for it := NewNodeIterator(trie); it.Next(true); {
-		if it.Hash != (common.Hash{}) {
+		if it.Hash() != (common.Hash{}) {
-			hashes[it.Hash] = struct{}{}
+			hashes[it.Hash()] = struct{}{}
 		}
 	}
 	// Cross check the hashes and the database itself

trie/secure_trie.go

@@ -159,7 +159,7 @@ func (t *SecureTrie) Iterator() *Iterator {
 	return t.trie.Iterator()
 }
 
-func (t *SecureTrie) NodeIterator() *NodeIterator {
+func (t *SecureTrie) NodeIterator() NodeIterator {
 	return NewNodeIterator(&t.trie)
 }
 

trie/sync_test.go

@@ -83,7 +83,7 @@ func checkTrieConsistency(db Database, root common.Hash) error {
 	it := NewNodeIterator(trie)
 	for it.Next(true) {
 	}
-	return it.Error
+	return it.Error()
 }
 
 // Tests that an empty trie is not scheduled for syncing.