trie/bintrie: fix DeleteAccount no-op#34676
Open
CPerezz wants to merge 1 commit intoethereum:masterfrom
Open
Conversation
BinaryTrie.DeleteAccount was a no-op, silently ignoring the caller's
deletion request and leaving the old BasicData and CodeHash in the trie.
The GetAccount deletion-detection branch (trie.go:219) already expected
a tombstone convention — "BasicData and CodeHash are 32-byte zero blobs
AND a non-nil 32-byte sentinel is present at a reserved offset" — but
nothing was writing that sentinel, so the check was effectively dead
code.
Implement the deletion as an InsertValuesAtStem that:
- writes a 32-byte zero blob to BasicData (offset 0)
- writes a 32-byte zero blob to CodeHash (offset 1)
- writes a 32-byte zero blob to a deletion sentinel offset in the
EIP-7864 reserved range (offset 10, promoted to the named constant
accountDeletedMarkerKey for cross-referencing with GetAccount)
This matches the bintrie's existing "write zeros to delete" convention
seen in DeleteStorage, keeps GetAccount's deletion branch consistent,
and still distinguishes "deleted" from "never existed" (the latter has
all-nil slots so the empty-account check fires first).
Storage slots and code chunks are intentionally left untouched. Wiping
storage on self-destruct is a separate concern handled at the StateDB
level — the bintrie's unified keyspace has no cheap way to enumerate
every slot of a given account, so a blanket wipe is not possible here.
Regression tests cover:
- round-trip: UpdateAccount -> GetAccount -> DeleteAccount -> GetAccount nil
- delete on missing account: no panic, subsequent read still nil
- unrelated accounts at different stems are preserved
- delete + recreate: second read sees the new values, not the old ones
- main storage slots at different stems survive DeleteAccount
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
BinaryTrie.DeleteAccountwas a no-op, silently ignoring the caller's deletion request and leaving the oldBasicDataandCodeHashin the trie. TheGetAccountdeletion-detection branch (aroundtrie.go:219) already expected a tombstone convention — "BasicData and CodeHash are 32-byte zero blobs AND a non-nil 32-byte sentinel is present at a reserved offset" — but nothing was writing that sentinel, so the check was effectively dead code.This PR implements the deletion as an
InsertValuesAtStemthat:BasicData(offset 0)CodeHash(offset 1)accountDeletedMarkerKeyfor cross-referencing withGetAccount)This matches the bintrie's existing "write zeros to delete" convention already used by
DeleteStorage, keepsGetAccount's deletion branch consistent, and still distinguishes deleted from never existed (the latter has all-nil slots so the empty-account check fires first).This becomes critical once any flat-state layer is enabled for the bintrie (in progress in [
state-hasher-iface-2](https://github.com/rjl493456442/go-ethereum/tree/state-hasher-iface-2 as a first abstraction) and the follow-up flat-state PR stack): the flat-state layer records deletions correctly, but the trie keeps the stale values, leading to state-root divergence.