Skip to content

core/state: fix state iterator#19127

Merged
karalabe merged 2 commits intoethereum:masterfrom
rjl493456442:fix-state-iterator
Apr 5, 2019
Merged

core/state: fix state iterator#19127
karalabe merged 2 commits intoethereum:masterfrom
rjl493456442:fix-state-iterator

Conversation

@rjl493456442
Copy link
Copy Markdown
Member

@rjl493456442 rjl493456442 commented Feb 19, 2019

This PR introduces a fix to state.ForEachStorage function.

Since before inserting data into trie, there is an additional RLP encode operation.
While in the state.ForEachStorage function, it relies on the trie iterator to iterate all leaf nodes(aka state data), but the rlp decode operation is missing there.

Comment thread core/state/statedb.go
cb(key, common.BytesToHash(it.Value))

if len(it.Value) > 0 {
_, content, _, err := rlp.Split(it.Value)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need to post-process the value returned by the iterator? Shouldn't the underlying iterator do this?

Copy link
Copy Markdown
Member Author

@rjl493456442 rjl493456442 Feb 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it is quite weird since before putting the value into the trie, we have an additional rlp encode process https://github.com/ethereum/go-ethereum/blob/master/core/state/state_object.go#L234
And for the normal GetState operation, there is a corresponding rlp decode process https://github.com/ethereum/go-ethereum/blob/master/core/state/state_object.go#L186

While the iterator only iterate the trie node and miss the rlp decode operation.

Comment thread core/state/statedb.go Outdated
if len(it.Value) > 0 {
_, content, _, err := rlp.Split(it.Value)
if err != nil {
continue
Copy link
Copy Markdown
Member

@karalabe karalabe Feb 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a very bad idea. When we iterate a trie, no error should ever happen. Here not only are we expecting an error, but also silently discarding it. This would end up haunting us a lot. What's happening here?

@rjl493456442
Copy link
Copy Markdown
Member Author

@karalabe I return the error explicitly. PTAL

@karalabe karalabe added this to the 1.9.0 milestone Apr 5, 2019
Copy link
Copy Markdown
Member

@karalabe karalabe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@karalabe karalabe merged commit 36f8111 into ethereum:master Apr 5, 2019
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Mar 27, 2025
gzliudan added a commit to XinFinOrg/XDPoSChain that referenced this pull request Mar 28, 2025
gzliudan added a commit to XinFinOrg/XDPoSChain that referenced this pull request Apr 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants