From f105ac06449b8a405f8e6dc8c46229d8342c2e49 Mon Sep 17 00:00:00 2001 From: Nick Johnson Date: Tue, 17 Jan 2017 14:47:35 +0000 Subject: [PATCH] core/state, trie: Rewrite NodeIterator as an interface --- core/state/iterator.go | 27 +++++----- trie/iterator.go | 120 ++++++++++++++++++++++++++--------------- trie/iterator_test.go | 4 +- trie/secure_trie.go | 2 +- trie/sync_test.go | 2 +- 5 files changed, 93 insertions(+), 62 deletions(-) diff --git a/core/state/iterator.go b/core/state/iterator.go index 3e82dca099..170aec9833 100644 --- a/core/state/iterator.go +++ b/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 - dataIt *trie.NodeIterator // Secondary iterator for the data trie of a contract + stateIt trie.NodeIterator // Primary iterator for the global state trie + 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 { - return it.dataIt.Error + if it.dataIt.Error() != nil { + 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 { - return it.stateIt.Error + if it.stateIt.Error() != nil { + 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 } diff --git a/trie/iterator.go b/trie/iterator.go index 6610ff368f..cbfab3dd41 100644 --- a/trie/iterator.go +++ b/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 { - it.Key = decodeCompact(it.nodeIt.path) - it.Value = it.nodeIt.LeafBlob + if it.nodeIt.Leaf() { + it.Key = decodeCompact(it.nodeIt.Path()) + 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) - 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 + err error // Failure set in case of an internal error in the iterator - 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 -} diff --git a/trie/iterator_test.go b/trie/iterator_test.go index 89aa8cc15d..adfc30d460 100644 --- a/trie/iterator_test.go +++ b/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{}) { - hashes[it.Hash] = struct{}{} + if it.Hash() != (common.Hash{}) { + hashes[it.Hash()] = struct{}{} } } // Cross check the hashes and the database itself diff --git a/trie/secure_trie.go b/trie/secure_trie.go index 4d9ebe4d37..8b90da02fe 100644 --- a/trie/secure_trie.go +++ b/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) } diff --git a/trie/sync_test.go b/trie/sync_test.go index 547a6938f1..acae039cd5 100644 --- a/trie/sync_test.go +++ b/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.