review feedback

This commit is contained in:
Guillaume Ballet 2026-04-10 18:33:17 +02:00
parent 0835ce4bb5
commit 21f51b7b55
No known key found for this signature in database
3 changed files with 6 additions and 54 deletions

View file

@ -31,13 +31,6 @@ const (
BasicDataBalanceOffset = 16 BasicDataBalanceOffset = 16
) )
// accountDeletedMarkerKey is the stem offset used as a "this account was
// deleted" sentinel. It lives in the EIP-7864 reserved range (offsets 2-63)
// and is written by DeleteAccount and read by GetAccount's deletion-
// detection branch. Both sides reference this constant directly, so there
// is no magic number to keep in sync.
const accountDeletedMarkerKey = 10
var ( var (
zeroInt = uint256.NewInt(0) zeroInt = uint256.NewInt(0)
zeroHash = common.Hash{} zeroHash = common.Hash{}

View file

@ -217,13 +217,10 @@ func (t *BinaryTrie) GetAccount(addr common.Address) (*types.StateAccount, error
} }
// If the account has been deleted, BasicData and CodeHash will both be // If the account has been deleted, BasicData and CodeHash will both be
// 32-byte zero blobs (not nil) and the accountDeletedMarkerKey sentinel // 32-byte zero blobs (not nil). If the account is recreated afterwards,
// will be a non-empty 32-byte zero blob written by DeleteAccount. If the // UpdateAccount overwrites BasicData and CodeHash with non-zero values,
// account has been recreated since, UpdateAccount will have overwritten // so this branch won't activate..
// BasicData and CodeHash with non-zero values, so this branch won't hit.
if bytes.Equal(values[BasicDataLeafKey], zero[:]) && if bytes.Equal(values[BasicDataLeafKey], zero[:]) &&
len(values) > accountDeletedMarkerKey &&
len(values[accountDeletedMarkerKey]) > 0 &&
bytes.Equal(values[CodeHashLeafKey], zero[:]) { bytes.Equal(values[CodeHashLeafKey], zero[:]) {
return nil, nil return nil, nil
} }
@ -299,28 +296,8 @@ func (t *BinaryTrie) UpdateStorage(address common.Address, key, value []byte) er
return nil return nil
} }
// DeleteAccount removes the account metadata (basic data and code hash) for // DeleteAccount erases an account by overwriting the account
// the given address from the trie. // descriptors with 0s.
//
// Binary trie leaves cannot be "physically" removed — there is no delete
// primitive on StemNode. Instead, the bintrie uses a tombstone convention:
// BasicData (offset 0) and CodeHash (offset 1) are overwritten with 32 zero
// bytes, and a non-nil 32-byte sentinel is written at accountDeletedMarkerKey.
// This matches GetAccount's deletion-detection branch which treats "BasicData
// and CodeHash are zero AND the sentinel is present" as an absent account
// while still distinguishing it from "never existed" (all-nil slots).
//
// Storage slots and code chunks are NOT touched. Header storage (slot
// numbers 0-63) shares the same stem as BasicData/CodeHash but lives at
// offsets 64-127; this function writes only BasicDataLeafKey, CodeHashLeafKey,
// and accountDeletedMarkerKey, so header storage survives by relying on
// StemNode.InsertValuesAtStem treating nil entries in the values slice as
// "do not overwrite". Main storage and code chunks live at different stems
// entirely.
//
// If the caller needs to wipe storage on self-destruct, it must walk the
// relevant slots explicitly — bintrie's unified keyspace has no cheap way
// to enumerate every slot of a given account.
func (t *BinaryTrie) DeleteAccount(addr common.Address) error { func (t *BinaryTrie) DeleteAccount(addr common.Address) error {
var ( var (
values = make([][]byte, StemNodeWidth) values = make([][]byte, StemNodeWidth)
@ -329,10 +306,6 @@ func (t *BinaryTrie) DeleteAccount(addr common.Address) error {
// Clear BasicData (nonce, balance, code size) and CodeHash. // Clear BasicData (nonce, balance, code size) and CodeHash.
values[BasicDataLeafKey] = zero[:] values[BasicDataLeafKey] = zero[:]
values[CodeHashLeafKey] = zero[:] values[CodeHashLeafKey] = zero[:]
// Write a non-nil 32-byte sentinel at the deletion marker offset so
// GetAccount can tell "deleted" apart from "never existed" on a
// subsequent read. See GetAccount's deletion-detection branch.
values[accountDeletedMarkerKey] = zero[:]
root, err := t.root.InsertValuesAtStem(stem, values, t.nodeResolver, 0) root, err := t.root.InsertValuesAtStem(stem, values, t.nodeResolver, 0)
if err != nil { if err != nil {

View file

@ -567,16 +567,6 @@ func TestDeleteAccountPreservesHeaderStorage(t *testing.T) {
} }
} }
// TestDeleteAccountHashIsDeterministic verifies that two independent tries
// running the same UpdateAccount → DeleteAccount sequence produce identical
// root hashes. This is the consensus-critical property of the tombstone
// model: deletion may not produce a pristine-empty root (zero blobs hash to
// non-zero leaves), but it MUST be deterministic across independent runs.
//
// A regression that introduced any non-determinism into the tombstone write
// path (e.g. relying on map iteration order, randomized offsets) would
// silently fork the network on the first self-destruct after enabling flat
// state.
func TestDeleteAccountHashIsDeterministic(t *testing.T) { func TestDeleteAccountHashIsDeterministic(t *testing.T) {
addr := common.HexToAddress("0x1234567890abcdef1234567890abcdef12345678") addr := common.HexToAddress("0x1234567890abcdef1234567890abcdef12345678")
codeHash := common.HexToHash("c5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470") codeHash := common.HexToHash("c5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470")
@ -599,13 +589,9 @@ func TestDeleteAccountHashIsDeterministic(t *testing.T) {
t.Fatalf("non-deterministic root after Update+Delete: first=%x second=%x", first, second) t.Fatalf("non-deterministic root after Update+Delete: first=%x second=%x", first, second)
} }
// Sanity check the tombstone model itself: the post-delete root is NOT
// the empty-trie root, because zero blobs hash to non-zero leaves. If
// this assertion ever flips, the tombstone semantics have silently
// changed and the deletion-detection branch in GetAccount needs review.
empty := newEmptyTestTrie(t).Hash() empty := newEmptyTestTrie(t).Hash()
if first == empty { if first == empty {
t.Fatalf("post-delete root unexpectedly equals empty-trie root %x; tombstone semantics changed", empty) t.Fatalf("post-delete root unexpectedly equals empty-trie root %x", empty)
} }
} }