trie/bintrie: fix GetAccount/GetStorage non-membership — verify stem before returning values (#34690)
Some checks are pending
/ Linux Build (push) Waiting to run
/ Linux Build (arm) (push) Waiting to run
/ Keeper Build (push) Waiting to run
/ Windows Build (push) Waiting to run
/ Docker Image (push) Waiting to run

Fix `GetAccount` returning **wrong account data** for non-existent
addresses when the trie root is a `StemNode` (single-account trie) — the
`StemNode` branch returned `r.Values` without verifying the queried
address's stem matches.

Co-authored-by: Guillaume Ballet <3272758+gballet@users.noreply.github.com>
This commit is contained in:
CPerezz 2026-04-10 19:43:48 +02:00 committed by GitHub
parent f71a884e37
commit deda47f6a1
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 208 additions and 2 deletions

View file

@ -37,7 +37,10 @@ type StemNode struct {
// Get retrieves the value for the given key.
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.

View file

@ -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)

View file

@ -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:

View file

@ -620,3 +620,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)
}
}