From 30656d714edaf4756729bad0672cde5790128a8f Mon Sep 17 00:00:00 2001
From: phrwlk <phrwlk7@gmail.com>
Date: Wed, 11 Feb 2026 12:42:17 +0200
Subject: [PATCH] trie/bintrie: use correct key mapping in GetStorage and
 DeleteStorage (#33807)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

GetStorage and DeleteStorage used GetBinaryTreeKey to compute the tree
key, while UpdateStorage used GetBinaryTreeKeyStorageSlot. The latter
applies storage slot remapping (header offset for slots <64, main
storage prefix for the rest), so reads and deletes were targeting
different tree locations than writes.

Replace GetBinaryTreeKey with GetBinaryTreeKeyStorageSlot in both
GetStorage and DeleteStorage to match UpdateStorage. Add a regression
test that verifies the write→read→delete→read round-trip for main
storage slots.
---
 trie/bintrie/trie.go      |  4 +--
 trie/bintrie/trie_test.go | 70 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 72 insertions(+), 2 deletions(-)

diff --git a/trie/bintrie/trie.go b/trie/bintrie/trie.go
index 966d236c08..ecdd002331 100644
--- a/trie/bintrie/trie.go
+++ b/trie/bintrie/trie.go
@@ -236,7 +236,7 @@ func (t *BinaryTrie) GetAccount(addr common.Address) (*types.StateAccount, error
 // not be modified by the caller. If a node was not found in the database, a
 // trie.MissingNodeError is returned.
 func (t *BinaryTrie) GetStorage(addr common.Address, key []byte) ([]byte, error) {
-	return t.root.Get(GetBinaryTreeKey(addr, key), t.nodeResolver)
+	return t.root.Get(GetBinaryTreeKeyStorageSlot(addr, key), t.nodeResolver)
 }
 
 // UpdateAccount updates the account information for the given address.
@@ -302,7 +302,7 @@ func (t *BinaryTrie) DeleteAccount(addr common.Address) error {
 // 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 {
-	k := GetBinaryTreeKey(addr, key)
+	k := GetBinaryTreeKeyStorageSlot(addr, key)
 	var zero [HashSize]byte
 	root, err := t.root.Insert(k, zero[:], t.nodeResolver, 0)
 	if err != nil {
diff --git a/trie/bintrie/trie_test.go b/trie/bintrie/trie_test.go
index 050cc8d940..256fd218e2 100644
--- a/trie/bintrie/trie_test.go
+++ b/trie/bintrie/trie_test.go
@@ -22,7 +22,9 @@ import (
 	"testing"
 
 	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/trie"
+	"github.com/holiman/uint256"
 )
 
 var (
@@ -197,6 +199,74 @@ func TestMerkleizeMultipleEntries(t *testing.T) {
 	}
 }
 
+// TestStorageRoundTrip verifies that GetStorage and DeleteStorage use the same
+// key mapping as UpdateStorage (GetBinaryTreeKeyStorageSlot). This is a regression
+// test: previously GetStorage and DeleteStorage used GetBinaryTreeKey directly,
+// which produced different tree keys and broke the read/delete path.
+func TestStorageRoundTrip(t *testing.T) {
+	tracer := trie.NewPrevalueTracer()
+	tr := &BinaryTrie{
+		root:   NewBinaryNode(),
+		tracer: tracer,
+	}
+	addr := common.HexToAddress("0x1234567890abcdef1234567890abcdef12345678")
+
+	// Create an account first so the root becomes an InternalNode,
+	// which is the realistic state when storage operations happen.
+	acc := &types.StateAccount{
+		Nonce:    1,
+		Balance:  uint256.NewInt(1000),
+		CodeHash: common.HexToHash("c5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470").Bytes(),
+	}
+	if err := tr.UpdateAccount(addr, acc, 0); err != nil {
+		t.Fatalf("UpdateAccount error: %v", err)
+	}
+
+	// Test main storage slots (key[31] >= 64 or key[:31] != 0).
+	// These produce a different stem than the account data, so after
+	// UpdateAccount + UpdateStorage the root is an InternalNode.
+	// Note: header slots (key[31] < 64, key[:31] == 0) share the same
+	// stem as account data and are covered by GetAccount/UpdateAccount path.
+	slots := []common.Hash{
+		common.HexToHash("00000000000000000000000000000000000000000000000000000000000000FF"), // main storage (slot 255)
+		common.HexToHash("0100000000000000000000000000000000000000000000000000000000000001"), // main storage (non-zero prefix)
+	}
+	val := common.TrimLeftZeroes(common.HexToHash("00000000000000000000000000000000000000000000000000000000deadbeef").Bytes())
+
+	for _, slot := range slots {
+		// Write
+		if err := tr.UpdateStorage(addr, slot[:], val); err != nil {
+			t.Fatalf("UpdateStorage(%x) error: %v", slot, err)
+		}
+		// Read back
+		got, err := tr.GetStorage(addr, slot[:])
+		if err != nil {
+			t.Fatalf("GetStorage(%x) error: %v", slot, err)
+		}
+		if len(got) == 0 {
+			t.Fatalf("GetStorage(%x) returned empty, expected value", slot)
+		}
+		// Verify value (right-justified in 32 bytes)
+		var expected [HashSize]byte
+		copy(expected[HashSize-len(val):], val)
+		if !bytes.Equal(got, expected[:]) {
+			t.Fatalf("GetStorage(%x) = %x, want %x", slot, got, expected)
+		}
+		// Delete
+		if err := tr.DeleteStorage(addr, slot[:]); err != nil {
+			t.Fatalf("DeleteStorage(%x) error: %v", slot, err)
+		}
+		// Verify deleted (should read as zero, not the old value)
+		got, err = tr.GetStorage(addr, slot[:])
+		if err != nil {
+			t.Fatalf("GetStorage(%x) after delete error: %v", slot, err)
+		}
+		if len(got) > 0 && !bytes.Equal(got, zero[:]) {
+			t.Fatalf("GetStorage(%x) after delete = %x, expected zero", slot, got)
+		}
+	}
+}
+
 func TestBinaryTrieWitness(t *testing.T) {
 	tracer := trie.NewPrevalueTracer()