diff --git a/trie/bintrie/trie.go b/trie/bintrie/trie.go index 6c29239a87..a1f5b57a4c 100644 --- a/trie/bintrie/trie.go +++ b/trie/bintrie/trie.go @@ -216,10 +216,15 @@ func (t *BinaryTrie) GetAccount(addr common.Address) (*types.StateAccount, error return nil, nil } - // If the account has been deleted, then values[10] will be 0 and not nil. If it has - // been recreated after that, then its code keccak will NOT be 0. So return `nil` if - // the nonce, and values[10], and code keccak is 0. - if bytes.Equal(values[BasicDataLeafKey], zero[:]) && len(values) > 10 && len(values[10]) > 0 && bytes.Equal(values[CodeHashLeafKey], zero[:]) { + // If the account has been deleted, BasicData and CodeHash will both be + // 32-byte zero blobs (not nil) and the accountDeletedMarkerKey sentinel + // will be a non-empty 32-byte zero blob written by DeleteAccount. If the + // account has been recreated since, UpdateAccount will have overwritten + // BasicData and CodeHash with non-zero values, so this branch won't hit. + if bytes.Equal(values[BasicDataLeafKey], zero[:]) && + len(values) > accountDeletedMarkerKey && + len(values[accountDeletedMarkerKey]) > 0 && + bytes.Equal(values[CodeHashLeafKey], zero[:]) { return nil, nil } @@ -294,11 +299,47 @@ func (t *BinaryTrie) UpdateStorage(address common.Address, key, value []byte) er return nil } -// DeleteAccount is a no-op as it is disabled in stateless. +// DeleteAccount removes the account metadata (basic data and code hash) for +// the given address from the trie. +// +// 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 the deletion marker +// offset that GetAccount consults. This matches the detection logic at +// GetAccount around trie.go:219 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 by this function. 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 { - return nil + var ( + err error + zeroBlob [HashSize]byte + values = make([][]byte, StemNodeWidth) + stem = GetBinaryTreeKey(addr, zero[:]) + ) + // Clear BasicData (nonce, balance, code size) and CodeHash. + values[BasicDataLeafKey] = zeroBlob[:] + values[CodeHashLeafKey] = zeroBlob[:] + // 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 branch around trie.go:219. + values[accountDeletedMarkerKey] = zeroBlob[:] + + t.root, err = t.root.InsertValuesAtStem(stem, values, t.nodeResolver, 0) + return err } +// 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 consulted only by GetAccount's deletion-detection branch. Keep this +// in sync with the check in GetAccount (trie.go:219). +const accountDeletedMarkerKey = 10 + // DeleteStorage removes any existing value for key from the trie. If a node was not // found in the database, a trie.MissingNodeError is returned. func (t *BinaryTrie) DeleteStorage(addr common.Address, key []byte) error { diff --git a/trie/bintrie/trie_test.go b/trie/bintrie/trie_test.go index 256fd218e2..2857fbbb4b 100644 --- a/trie/bintrie/trie_test.go +++ b/trie/bintrie/trie_test.go @@ -267,6 +267,225 @@ func TestStorageRoundTrip(t *testing.T) { } } +// newTestTrie creates a fresh BinaryTrie with an empty root and a default +// prevalue tracer. Used by tests that need a realistic trie instance. +func newTestTrie(t *testing.T) *BinaryTrie { + t.Helper() + return &BinaryTrie{ + root: NewBinaryNode(), + tracer: trie.NewPrevalueTracer(), + } +} + +// makeAccount constructs a StateAccount with the given fields. The Root is +// zeroed out because the bintrie has no per-account storage root. +func makeAccount(nonce uint64, balance uint64, codeHash common.Hash) *types.StateAccount { + return &types.StateAccount{ + Nonce: nonce, + Balance: uint256.NewInt(balance), + CodeHash: codeHash.Bytes(), + } +} + +// TestDeleteAccountRoundTrip verifies the basic delete path: create an +// account, read it back, delete it, confirm subsequent reads return nil. +// Regression test for the no-op DeleteAccount bug where the deletion was +// silently ignored and the old values remained in the trie. +func TestDeleteAccountRoundTrip(t *testing.T) { + tr := newTestTrie(t) + addr := common.HexToAddress("0x1234567890abcdef1234567890abcdef12345678") + codeHash := common.HexToHash("c5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470") + + // Create: write account, verify round-trip. + acc := makeAccount(42, 1000, codeHash) + if err := tr.UpdateAccount(addr, acc, 0); err != nil { + t.Fatalf("UpdateAccount: %v", err) + } + got, err := tr.GetAccount(addr) + if err != nil { + t.Fatalf("GetAccount: %v", err) + } + if got == nil { + t.Fatal("GetAccount returned nil after UpdateAccount") + } + if got.Nonce != 42 { + t.Fatalf("Nonce: got %d, want 42", got.Nonce) + } + if got.Balance.Uint64() != 1000 { + t.Fatalf("Balance: got %s, want 1000", got.Balance) + } + if !bytes.Equal(got.CodeHash, codeHash[:]) { + t.Fatalf("CodeHash: got %x, want %x", got.CodeHash, codeHash) + } + + // Delete: verify GetAccount returns nil afterwards. + if err := tr.DeleteAccount(addr); err != nil { + t.Fatalf("DeleteAccount: %v", err) + } + got, err = tr.GetAccount(addr) + if err != nil { + t.Fatalf("GetAccount after delete: %v", err) + } + if got != nil { + t.Fatalf("GetAccount after delete: got %+v, want nil", got) + } +} + +// TestDeleteAccountOnMissingAccount verifies that deleting an account that +// was never created does not error and subsequent reads still return nil. +func TestDeleteAccountOnMissingAccount(t *testing.T) { + tr := newTestTrie(t) + addr := common.HexToAddress("0x1234567890abcdef1234567890abcdef12345678") + + // Delete without any prior create. Should not panic or error on an + // empty root, and GetAccount should still return nil. + if err := tr.DeleteAccount(addr); err != nil { + t.Fatalf("DeleteAccount on empty trie: %v", err) + } + got, err := tr.GetAccount(addr) + if err != nil { + t.Fatalf("GetAccount after delete on empty trie: %v", err) + } + if got != nil { + t.Fatalf("GetAccount on deleted missing account: got %+v, want nil", got) + } +} + +// TestDeleteAccountPreservesOtherAccounts verifies that deleting one account +// does not affect accounts at different stems. +func TestDeleteAccountPreservesOtherAccounts(t *testing.T) { + tr := newTestTrie(t) + addrA := common.HexToAddress("0x1234567890abcdef1234567890abcdef12345678") + addrB := common.HexToAddress("0xabcdef1234567890abcdef1234567890abcdef12") + codeHashA := common.HexToHash("c5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470") + codeHashB := common.HexToHash("f0f1f2f3f4f5f6f7f8f9fafbfcfdfeff0102030405060708090a0b0c0d0e0f10") + + // Create two distinct accounts. + if err := tr.UpdateAccount(addrA, makeAccount(1, 100, codeHashA), 0); err != nil { + t.Fatalf("UpdateAccount(A): %v", err) + } + if err := tr.UpdateAccount(addrB, makeAccount(2, 200, codeHashB), 0); err != nil { + t.Fatalf("UpdateAccount(B): %v", err) + } + + // Delete A. + if err := tr.DeleteAccount(addrA); err != nil { + t.Fatalf("DeleteAccount(A): %v", err) + } + + // A should be gone. + if got, err := tr.GetAccount(addrA); err != nil { + t.Fatalf("GetAccount(A): %v", err) + } else if got != nil { + t.Fatalf("GetAccount(A) after delete: got %+v, want nil", got) + } + + // B should still be readable with its original values. + got, err := tr.GetAccount(addrB) + if err != nil { + t.Fatalf("GetAccount(B): %v", err) + } + if got == nil { + t.Fatal("GetAccount(B) returned nil after unrelated delete") + } + if got.Nonce != 2 { + t.Fatalf("Account B Nonce: got %d, want 2", got.Nonce) + } + if got.Balance.Uint64() != 200 { + t.Fatalf("Account B Balance: got %s, want 200", got.Balance) + } + if !bytes.Equal(got.CodeHash, codeHashB[:]) { + t.Fatalf("Account B CodeHash: got %x, want %x", got.CodeHash, codeHashB) + } +} + +// TestDeleteAccountThenRecreate verifies that an account can be deleted and +// then recreated with different values; the second read must return the new +// values, not the stale ones from before deletion. +func TestDeleteAccountThenRecreate(t *testing.T) { + tr := newTestTrie(t) + addr := common.HexToAddress("0x1234567890abcdef1234567890abcdef12345678") + codeHash1 := common.HexToHash("1111111111111111111111111111111111111111111111111111111111111111") + codeHash2 := common.HexToHash("2222222222222222222222222222222222222222222222222222222222222222") + + // Create. + if err := tr.UpdateAccount(addr, makeAccount(1, 100, codeHash1), 0); err != nil { + t.Fatalf("UpdateAccount #1: %v", err) + } + // Delete. + if err := tr.DeleteAccount(addr); err != nil { + t.Fatalf("DeleteAccount: %v", err) + } + // Recreate with new values. + if err := tr.UpdateAccount(addr, makeAccount(7, 9999, codeHash2), 0); err != nil { + t.Fatalf("UpdateAccount #2: %v", err) + } + // Read: must observe the new values, not the originals. + got, err := tr.GetAccount(addr) + if err != nil { + t.Fatalf("GetAccount: %v", err) + } + if got == nil { + t.Fatal("GetAccount returned nil after recreate") + } + if got.Nonce != 7 { + t.Fatalf("Nonce: got %d, want 7", got.Nonce) + } + if got.Balance.Uint64() != 9999 { + t.Fatalf("Balance: got %s, want 9999", got.Balance) + } + if !bytes.Equal(got.CodeHash, codeHash2[:]) { + t.Fatalf("CodeHash: got %x, want %x", got.CodeHash, codeHash2) + } +} + +// TestDeleteAccountDoesNotAffectMainStorage verifies that DeleteAccount only +// clears the account's BasicData and CodeHash, leaving main storage slots +// (which live at different stems) untouched. Wiping storage on self-destruct +// is a separate concern handled at the StateDB level. +func TestDeleteAccountDoesNotAffectMainStorage(t *testing.T) { + tr := newTestTrie(t) + addr := common.HexToAddress("0x1234567890abcdef1234567890abcdef12345678") + codeHash := common.HexToHash("c5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470") + + // Create account. + if err := tr.UpdateAccount(addr, makeAccount(1, 100, codeHash), 0); err != nil { + t.Fatalf("UpdateAccount: %v", err) + } + // Write a main storage slot — i.e. key[31] >= 64 or key[:31] != 0 — so + // it lives at a different stem from the account header. + slot := common.HexToHash("0000000000000000000000000000000000000000000000000000000000000080") + value := common.TrimLeftZeroes(common.HexToHash("00000000000000000000000000000000000000000000000000000000deadbeef").Bytes()) + if err := tr.UpdateStorage(addr, slot[:], value); err != nil { + t.Fatalf("UpdateStorage: %v", err) + } + + // Delete the account. + if err := tr.DeleteAccount(addr); err != nil { + t.Fatalf("DeleteAccount: %v", err) + } + + // Account should be absent. + if got, _ := tr.GetAccount(addr); got != nil { + t.Fatalf("GetAccount after delete: got %+v, want nil", got) + } + + // Main storage slot should still be readable — DeleteAccount must not + // have touched it. + got, err := tr.GetStorage(addr, slot[:]) + if err != nil { + t.Fatalf("GetStorage after DeleteAccount: %v", err) + } + if len(got) == 0 { + t.Fatal("main storage slot was wiped by DeleteAccount, expected it to survive") + } + var expected [HashSize]byte + copy(expected[HashSize-len(value):], value) + if !bytes.Equal(got, expected[:]) { + t.Fatalf("main storage slot: got %x, want %x", got, expected) + } +} + func TestBinaryTrieWitness(t *testing.T) { tracer := trie.NewPrevalueTracer()