core/state: fix prefetcher for verkle #29760
Conversation
core/state/statedb.go
Outdated
| // Don't check prefetcher if Verkle Trie has been used. In the context of Verkle, | ||
| // only a single trie is used for state hashing. Replacing a non-nil Verkle tree | ||
| // here could result in losing uncommitted changes from storage. | ||
| if prefetcher != nil && (s.trie == nil || !s.trie.IsVerkle()) { |
There was a problem hiding this comment.
This change does not only affect verkle, but also non-verkle, if prefetcher != nil, s.trie != nil would previously enter this clause, but not after this change.
Please elaborate
There was a problem hiding this comment.
s.trie != nil && !s.trie.IsVerkle() will still entry the clause
only s.trie != nil && s.trie.IsVerkle will be rejected
There was a problem hiding this comment.
I'm talking about non-verkle (so s.trie.IsVerkle() == false).
So, for prefetcher != nil, s.trie != nil
- currently, on
master: we enter the clause - in this PR: we do not enter the clause
There was a problem hiding this comment.
for the case you mentioned, non-verkle, trie is not nil, we will still enter the clause isn't it?
(s.trie == nil || !s.trie.IsVerkle()), the case will satisfy the second condition !s.trie.IsVerkle()
core/state/statedb.go
Outdated
| // Don't check prefetcher if Verkle Trie has been used. In the context of Verkle, | ||
| // only a single trie is used for state hashing. Replacing a non-nil Verkle tree | ||
| // here could result in losing uncommitted changes from storage. | ||
| if prefetcher != nil && (s.trie == nil || !s.trie.IsVerkle()) { |
There was a problem hiding this comment.
I am quite surprised that you need this, because in verkle mode the prefetcher should be nil. But in any case, it doesn't hurt to add that check.
There was a problem hiding this comment.
yeah, i have some changes locally and enable the prefetcher for verkle. Tests are broken immediately. I can imagine prefetcher will enabled for verkle as well.
|
I'd appreciate getting the prefetcher PR in first because I don't want to rebase that for a 1 liner change. |
rjl493456442
force-pushed
the
fix-prefetcher-verkle
branch
from
7600bb9
to
cede7f0
Compare
This pull request fixes a flaw in verkle.
Inside of the statedb, there is a tool named "prefetcher". It will pull the necessary data for state hashing in the background to accelerate the state hashing performance. Unfortunately, it's broken in the verkle context.
The root cause is once verkle is activated, there is only a single trie used for state hashing(instead of two-layer tree structure in merkle). And storage trie commits are schedule before the account trie for performance consideration. If the verkle trie is resolved from the prefetcher after committing the storage changes, the changes caused by storage will be silently dropped.
This pull request disables the prefetching in verkle as a quick fix.