core: ensure state could be created in ToBlock #22780
Conversation
gballet
requested review from
holiman,
karalabe,
rjl493456442 and
zsfelfoldi
as code owners
| statedb, err := state.New(common.Hash{}, state.NewDatabase(db), nil) | ||
| if err != nil { | ||
| return nil, err | ||
| } |
There was a problem hiding this comment.
I don't get this. How can this possibly error?
There was a problem hiding this comment.
if the root isn't in the database, the underlying trie.OpenTrie will return an error, and if this error isn't caught, you end up with statedb = nil, which will crash later. I spent too much time looking for where the problem was, it would have been immediately obvious had an error been returned here.
There was a problem hiding this comment.
But the root is common.Hash{}, right? So I still don't get it
There was a problem hiding this comment.
Right, and in master, an empty hash will return an empty trie. I got a problem when I modified OpenTrie to support Verkle trees, which don't have the same behavior (it's expecting a translated layer, not an empty tree).
I can certainly do the same thing in Verkle trees: return an empty tree when receiving common.Hash. This PR still makes sense, though, because if any other new database system appears, the same problem will occur.
|
|
fjl
changed the title
Aren't tests supposed to catch errors? I could agree if this was a local function. This one, however, is exported and so it could be used by any client code. |
|
I still don't quite get this. So Now, zooming out a bit from the choice of state representation, whether it be mpt or verkle ... How can it possibly happen that a genesis state cannot be found? I don't mean "wcgw, yolo, let's ignore errors": I'm genuinely wondering why any type of error can be raised here. |
|
Hi |
…um#22780) * Ensure state could be created in ToBlock * Fix rebase errors * use a panic instead
5 participants
While replacing the trie with verkle trees, I ran into this issue that
state.Newwould return an error if it couldn't find the trie in the database, and that error wasn't being checked. This PR returns an error instate.Newfails, so that a problem can be immediately detected by the calling code, and not cause a seemingly unrelated crash further down the road.