diff --git a/trie/bintrie/empty.go b/trie/bintrie/empty.go index 252146a4a7..63863e1c4c 100644 --- a/trie/bintrie/empty.go +++ b/trie/bintrie/empty.go @@ -48,8 +48,7 @@ func (e Empty) Hash() common.Hash { } func (e Empty) GetValuesAtStem(_ []byte, _ NodeResolverFn) ([][]byte, error) { - var values [256][]byte - return values[:], nil + return nil, nil } func (e Empty) InsertValuesAtStem(key []byte, values [][]byte, _ NodeResolverFn, depth int) (BinaryNode, error) { diff --git a/trie/bintrie/empty_test.go b/trie/bintrie/empty_test.go index 574ae1830b..526ac8f93c 100644 --- a/trie/bintrie/empty_test.go +++ b/trie/bintrie/empty_test.go @@ -105,7 +105,8 @@ func TestEmptyHash(t *testing.T) { } } -// TestEmptyGetValuesAtStem tests the GetValuesAtStem method +// TestEmptyGetValuesAtStem tests that GetValuesAtStem returns nil for an empty node, +// signaling that no values exist at the queried stem. func TestEmptyGetValuesAtStem(t *testing.T) { node := Empty{} @@ -114,16 +115,8 @@ func TestEmptyGetValuesAtStem(t *testing.T) { if err != nil { t.Fatalf("Unexpected error: %v", err) } - - // Should return an array of 256 nil values - if len(values) != 256 { - t.Errorf("Expected 256 values, got %d", len(values)) - } - - for i, v := range values { - if v != nil { - t.Errorf("Expected nil value at index %d, got %x", i, v) - } + if values != nil { + t.Errorf("Expected nil values from Empty.GetValuesAtStem, got %d entries", len(values)) } } diff --git a/trie/bintrie/stem_node.go b/trie/bintrie/stem_node.go index 3f69261d62..65e7ae3720 100644 --- a/trie/bintrie/stem_node.go +++ b/trie/bintrie/stem_node.go @@ -35,9 +35,13 @@ type StemNode struct { hash common.Hash // cached hash when mustRecompute == false } -// Get retrieves the value for the given key. +// Get retrieves the value for the given key. Returns (nil, nil) if the +// key's stem does not match this node's stem (non-membership). func (bt *StemNode) Get(key []byte, _ NodeResolverFn) ([]byte, error) { - panic("this should not be called directly") + if !bytes.Equal(bt.Stem, key[:StemSize]) { + return nil, nil + } + return bt.Values[key[StemSize]], nil } // Insert inserts a new key-value pair into the node. diff --git a/trie/bintrie/stem_node_test.go b/trie/bintrie/stem_node_test.go index 92c1b49e02..310c553d39 100644 --- a/trie/bintrie/stem_node_test.go +++ b/trie/bintrie/stem_node_test.go @@ -23,6 +23,50 @@ import ( "github.com/ethereum/go-ethereum/common" ) +// TestStemNodeGet tests the Get method for matching stem, non-matching stem, +// and nil-value suffix scenarios. +func TestStemNodeGet(t *testing.T) { + stem := make([]byte, StemSize) + stem[0] = 0xAB + var values [StemNodeWidth][]byte + values[5] = common.HexToHash("0xdeadbeef").Bytes() + + node := &StemNode{Stem: stem, Values: values[:], depth: 0} + + // Matching stem, populated suffix → returns value. + key := make([]byte, HashSize) + copy(key[:StemSize], stem) + key[StemSize] = 5 + got, err := node.Get(key, nil) + if err != nil { + t.Fatalf("Get error: %v", err) + } + if !bytes.Equal(got, values[5]) { + t.Fatalf("Get = %x, want %x", got, values[5]) + } + + // Matching stem, empty suffix → returns nil (slot not set). + key[StemSize] = 99 + got, err = node.Get(key, nil) + if err != nil { + t.Fatalf("Get error: %v", err) + } + if got != nil { + t.Fatalf("Get(empty suffix) = %x, want nil", got) + } + + // Non-matching stem → returns nil, nil. + otherKey := make([]byte, HashSize) + otherKey[0] = 0xFF + got, err = node.Get(otherKey, nil) + if err != nil { + t.Fatalf("Get error: %v", err) + } + if got != nil { + t.Fatalf("Get(wrong stem) = %x, want nil", got) + } +} + // TestStemNodeInsertSameStem tests inserting values with the same stem func TestStemNodeInsertSameStem(t *testing.T) { stem := make([]byte, 31) diff --git a/trie/bintrie/trie.go b/trie/bintrie/trie.go index 6c29239a87..407014d55c 100644 --- a/trie/bintrie/trie.go +++ b/trie/bintrie/trie.go @@ -191,7 +191,7 @@ func (t *BinaryTrie) GetAccount(addr common.Address) (*types.StateAccount, error case *InternalNode: values, err = r.GetValuesAtStem(key[:StemSize], t.nodeResolver) case *StemNode: - values = r.Values + values, err = r.GetValuesAtStem(key[:StemSize], t.nodeResolver) case Empty: return nil, nil default: @@ -202,6 +202,9 @@ func (t *BinaryTrie) GetAccount(addr common.Address) (*types.StateAccount, error if err != nil { return nil, fmt.Errorf("GetAccount (%x) error: %v", addr, err) } + if values == nil { + return nil, nil + } // The following code is required for the MPT->Binary conversion. // An account can be partially migrated, where storage slots were moved to the binary @@ -209,7 +212,7 @@ func (t *BinaryTrie) GetAccount(addr common.Address) (*types.StateAccount, error // are in the binary trie but basic account information must be read in the base tree (MPT). // TODO: we can simplify this logic depending if the conversion is in progress or finished. emptyAccount := true - for i := 0; values != nil && i <= CodeHashLeafKey && emptyAccount; i++ { + for i := 0; i <= CodeHashLeafKey && emptyAccount; i++ { emptyAccount = emptyAccount && values[i] == nil } if emptyAccount { diff --git a/trie/bintrie/trie_test.go b/trie/bintrie/trie_test.go index 256fd218e2..e51437414b 100644 --- a/trie/bintrie/trie_test.go +++ b/trie/bintrie/trie_test.go @@ -292,3 +292,162 @@ func TestBinaryTrieWitness(t *testing.T) { t.Fatal("unexpected witness value for path2") } } + +// testAccount is a helper that creates a BinaryTrie with a tracer and +// inserts a single account, returning the trie. +func testAccount(t *testing.T, addr common.Address, nonce uint64, balance uint64) *BinaryTrie { + t.Helper() + tr := &BinaryTrie{ + root: NewBinaryNode(), + tracer: trie.NewPrevalueTracer(), + } + acc := &types.StateAccount{ + Nonce: nonce, + Balance: uint256.NewInt(balance), + CodeHash: types.EmptyCodeHash[:], + } + if err := tr.UpdateAccount(addr, acc, 0); err != nil { + t.Fatalf("UpdateAccount error: %v", err) + } + return tr +} + +// TestGetAccountNonMembershipStemRoot verifies that querying a non-existent +// address returns nil when the trie root is a StemNode (single-account trie). +// This is a regression test: previously the StemNode branch in GetAccount +// returned the root's values without verifying the stem. +func TestGetAccountNonMembershipStemRoot(t *testing.T) { + addr := common.HexToAddress("0x1111111111111111111111111111111111111111") + tr := testAccount(t, addr, 42, 100) + + // Verify root is a StemNode (single stem inserted). + if _, ok := tr.root.(*StemNode); !ok { + t.Fatalf("expected StemNode root, got %T", tr.root) + } + + // Query a completely different address — must return nil. + other := common.HexToAddress("0x2222222222222222222222222222222222222222") + got, err := tr.GetAccount(other) + if err != nil { + t.Fatalf("GetAccount error: %v", err) + } + if got != nil { + t.Fatalf("expected nil for non-existent account, got nonce=%d balance=%s", got.Nonce, got.Balance) + } + + // Original account must still be retrievable. + got, err = tr.GetAccount(addr) + if err != nil { + t.Fatalf("GetAccount(original) error: %v", err) + } + if got == nil { + t.Fatal("expected original account, got nil") + } + if got.Nonce != 42 { + t.Fatalf("expected nonce=42, got %d", got.Nonce) + } +} + +// TestGetAccountNonMembershipInternalRoot verifies that querying a non-existent +// address returns nil when the trie root is an InternalNode (multi-account trie). +func TestGetAccountNonMembershipInternalRoot(t *testing.T) { + tr := &BinaryTrie{ + root: NewBinaryNode(), + tracer: trie.NewPrevalueTracer(), + } + + // Insert two accounts whose binary tree keys have different first bits + // so the root splits into an InternalNode. + addr1 := common.HexToAddress("0x1111111111111111111111111111111111111111") + addr2 := common.HexToAddress("0x9999999999999999999999999999999999999999") + for _, addr := range []common.Address{addr1, addr2} { + acc := &types.StateAccount{ + Nonce: 1, + Balance: uint256.NewInt(1), + CodeHash: types.EmptyCodeHash[:], + } + if err := tr.UpdateAccount(addr, acc, 0); err != nil { + t.Fatalf("UpdateAccount error: %v", err) + } + } + + // Verify root is an InternalNode. + if _, ok := tr.root.(*InternalNode); !ok { + t.Fatalf("expected InternalNode root, got %T", tr.root) + } + + // Query a non-existent address — must return nil. + other := common.HexToAddress("0x5555555555555555555555555555555555555555") + got, err := tr.GetAccount(other) + if err != nil { + t.Fatalf("GetAccount error: %v", err) + } + if got != nil { + t.Fatalf("expected nil for non-existent account, got nonce=%d", got.Nonce) + } +} + +// TestGetStorageNonMembershipStemRoot verifies that querying storage for a +// non-existent address returns nil when the root is a StemNode. This is a +// regression test: previously StemNode.Get panicked unconditionally. +func TestGetStorageNonMembershipStemRoot(t *testing.T) { + addr := common.HexToAddress("0x1111111111111111111111111111111111111111") + tr := testAccount(t, addr, 1, 100) + + // Verify root is a StemNode. + if _, ok := tr.root.(*StemNode); !ok { + t.Fatalf("expected StemNode root, got %T", tr.root) + } + + // Query storage for a different address — must return nil, not panic. + other := common.HexToAddress("0x2222222222222222222222222222222222222222") + slot := common.HexToHash("0x01") + got, err := tr.GetStorage(other, slot[:]) + if err != nil { + t.Fatalf("GetStorage error: %v", err) + } + if len(got) > 0 && !bytes.Equal(got, zero[:]) { + t.Fatalf("expected nil/zero for non-existent storage, got %x", got) + } +} + +// TestGetStorageNonMembershipInternalRoot verifies that querying storage for a +// non-existent address returns nil when the root is an InternalNode. +func TestGetStorageNonMembershipInternalRoot(t *testing.T) { + tr := &BinaryTrie{ + root: NewBinaryNode(), + tracer: trie.NewPrevalueTracer(), + } + + addr := common.HexToAddress("0x1234567890abcdef1234567890abcdef12345678") + acc := &types.StateAccount{ + Nonce: 1, + Balance: uint256.NewInt(1000), + CodeHash: types.EmptyCodeHash[:], + } + if err := tr.UpdateAccount(addr, acc, 0); err != nil { + t.Fatalf("UpdateAccount error: %v", err) + } + + // Add a storage slot so the root becomes an InternalNode (storage + // slots use a different stem than account data). + slot := common.HexToHash("0xFF") + val := common.TrimLeftZeroes(common.HexToHash("0xdeadbeef").Bytes()) + if err := tr.UpdateStorage(addr, slot[:], val); err != nil { + t.Fatalf("UpdateStorage error: %v", err) + } + + if _, ok := tr.root.(*InternalNode); !ok { + t.Fatalf("expected InternalNode root, got %T", tr.root) + } + + // Query storage for a non-existent address — must return nil. + other := common.HexToAddress("0x9999999999999999999999999999999999999999") + got, err := tr.GetStorage(other, slot[:]) + if err != nil { + t.Fatalf("GetStorage error: %v", err) + } + if len(got) > 0 && !bytes.Equal(got, zero[:]) { + t.Fatalf("expected nil/zero for non-existent storage, got %x", got) + } +}