all: implement state history v2 #30107
Conversation
rjl493456442
requested review from
karalabe,
holiman,
MariusVanDerWijden and
lightclient
as code owners
rjl493456442
force-pushed
the
state-history-v2
branch
7 times, most recently
from
c014ee1 to
7def7a7
Compare
rjl493456442
force-pushed
the
state-history-v2
branch
2 times, most recently
from
7036366 to
22d2359
Compare
How does this PR work with chugging through older chain-segments, which potentially contain selfdestructs with storage-traversal needed? |
The state history of old chain segments (from the cancun fork) will be stored with initial version, the state history of post-cancun fork will be stored in new version. |
| @@ -1148,6 +1148,9 @@ func (s *StateDB) handleDestruction() (map[common.Hash]*accountDelete, []*trieno | |||
| if prev.Root == types.EmptyRootHash { | |||
| continue | |||
| } | |||
| if noStorageWiping { | |||
| return nil, nil, fmt.Errorf("unexpected storage wiping, %x", addr) | |||
There was a problem hiding this comment.
The case here is that the caller is saying that we're operating in "no selfdestruct-mode", but still tells us to destruct something (which has storage), is that correct?
| func (s *StateDB) Commit(block uint64, deleteEmptyObjects bool) (common.Hash, error) { | ||
| ret, err := s.commitAndFlush(block, deleteEmptyObjects) | ||
| // | ||
| // noStorageWiping is a flag indicating whether storage wiping is permitted. |
There was a problem hiding this comment.
I find it a bit unintuitive with reverse bools, "no XXX". Couldn't you flip it to wipeStorage bool?
There was a problem hiding this comment.
Can't we set that flag when we construct the state? We have acces to the ruleset then (block number etfc), don't we? If so a lot of the diffs in this PR goes away (?)
zsfelfoldi
force-pushed
the
state-history-v2
branch
from
22d2359 to
a408131
Compare
zsfelfoldi
force-pushed
the
state-history-v2
branch
from
a408131 to
d6c8fc4
Compare
This pull request delivers the new version of the state history, where the raw storage key is used instead of the hash.
Before the cancun fork, it's supported by protocol to destruct a specific account and therefore, all the storage slot owned by it should be wiped in the same transition.
Technically, storage wiping should be performed through storage iteration, and only the storage key hash will be available for traversal if the state snapshot is not available. Therefore, the storage key hash is chosen as the identifier in the old version state history.
Fortunately, account self-destruction has been deprecated by the protocol since the Cancun fork, and there are no empty accounts eligible for deletion under EIP-158. Therefore, we can conclude that no storage wiping should occur after the Cancun fork. In this case, it makes no sense to keep using hash.
Besides, another big reason for making this change is the current format state history is unusable if verkle is activated. Verkle tree has a different key derivation scheme (merkle uses keccak256), the preimage of key hash must be provided in order to make verkle rollback functional. This pull request is a prerequisite for landing verkle.
Additionally, the raw storage key is more human-friendly for those who want to manually check the history, even though Solidity already performs some hashing to derive the storage location.
This pull request doesn't bump the database version, as I believe the database should still be compatible if users degrade from the new geth version to old one, the only side effect is the persistent new version state history will be unusable.