review feedback

This commit is contained in:
Guillaume Ballet 2025-08-29 14:14:42 +02:00
parent 59587ee882
commit e99e19a0d9
No known key found for this signature in database
2 changed files with 164 additions and 168 deletions

View file

@ -17,9 +17,13 @@
package stateless package stateless
import ( import (
"maps"
"slices"
"sort"
"strings"
"github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/metrics" "github.com/ethereum/go-ethereum/metrics"
"github.com/ethereum/go-ethereum/rlp"
) )
var ( var (
@ -85,60 +89,21 @@ func NewWitnessStats() *WitnessStats {
} }
} }
// isLeafNode checks if the given RLP-encoded node data represents a leaf node.
// In Ethereum's Modified Merkle Patricia Trie, a leaf node is identified by:
// - Having exactly 2 RLP list elements (for both shortNode and leafNode encodings)
// - The second element being a value (not a hash reference to another node)
func isLeafNode(nodeData []byte) bool {
if len(nodeData) == 0 {
return false
}
// Decode the RLP list
var elems [][]byte
if err := rlp.DecodeBytes(nodeData, &elems); err != nil {
return false
}
// A leaf node in MPT has exactly 2 elements: [key, value]
// An extension node also has 2 elements but the value is a hash (32 bytes)
if len(elems) != 2 {
return false // Branch nodes have 17 elements
}
// If the second element is 32 bytes, it's likely a hash reference (extension node)
// Leaf nodes typically have values that are not exactly 32 bytes
// However, this is not a perfect heuristic as values could be 32 bytes
// A more accurate check would require checking the key's terminator flag
// Check if the key has a terminator (indicates leaf node)
// In compact encoding, the first nibble of the key indicates the node type
if len(elems[0]) > 0 {
// Get the first byte which contains the flags
flags := elems[0][0]
// Check if the terminator flag is set (bit 5)
// Leaf nodes have the terminator flag set (0x20 or 0x30)
return (flags & 0x20) != 0
}
return false
}
// Add records trie access depths from the given node paths. // Add records trie access depths from the given node paths.
// If `owner` is the zero hash, accesses are attributed to the account trie; // If `owner` is the zero hash, accesses are attributed to the account trie;
// otherwise, they are attributed to the storage trie of that account. // otherwise, they are attributed to the storage trie of that account.
func (s *WitnessStats) Add(nodes map[string][]byte, owner common.Hash) { func (s *WitnessStats) Add(nodes map[string][]byte, owner common.Hash) {
if owner == (common.Hash{}) { // Extract paths from the nodes map
for path, nodeData := range nodes { paths := slices.Collect(maps.Keys(nodes))
// Only record depth for leaf nodes sort.Strings(paths)
if isLeafNode(nodeData) {
for i, path := range paths {
// If current path is a prefix of the next path, it's not a leaf.
// The last path is always a leaf.
if i == len(paths)-1 || !strings.HasPrefix(paths[i+1], paths[i]) {
if owner == (common.Hash{}) {
s.accountTrie.add(int64(len(path))) s.accountTrie.add(int64(len(path)))
} } else {
}
} else {
for path, nodeData := range nodes {
// Only record depth for leaf nodes
if isLeafNode(nodeData) {
s.storageTrie.add(int64(len(path))) s.storageTrie.add(int64(len(path)))
} }
} }

View file

@ -20,157 +20,188 @@ import (
"testing" "testing"
"github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/rlp"
) )
func TestIsLeafNode(t *testing.T) { func TestWitnessStatsAdd(t *testing.T) {
tests := []struct { tests := []struct {
name string name string
nodeData []byte nodes map[string][]byte
want bool owner common.Hash
expectedAccountDepth int64
expectedStorageDepth int64
}{ }{
{ {
name: "leaf node with terminator", name: "empty nodes",
// Compact encoding: first byte 0x20 means odd length key with terminator nodes: map[string][]byte{},
// This represents a leaf node owner: common.Hash{},
nodeData: mustEncodeNode(t, [][]byte{ expectedAccountDepth: 0,
{0x20, 0x01, 0x02, 0x03}, // Key with terminator flag expectedStorageDepth: 0,
{0x01, 0x02, 0x03, 0x04}, // Value
}),
want: true,
}, },
{ {
name: "leaf node with even key and terminator", name: "single account trie leaf",
// Compact encoding: first byte 0x30 means even length key with terminator nodes: map[string][]byte{
nodeData: mustEncodeNode(t, [][]byte{ "abc": []byte("data"),
{0x30, 0x01, 0x02}, // Key with terminator flag (even length) },
{0x05, 0x06}, // Value owner: common.Hash{},
}), expectedAccountDepth: 3,
want: true, expectedStorageDepth: 0,
}, },
{ {
name: "extension node (no terminator)", name: "account trie with internal nodes",
// Compact encoding: first byte 0x00 means even length key without terminator nodes: map[string][]byte{
// This represents an extension node "a": []byte("data1"),
nodeData: mustEncodeNode(t, [][]byte{ "ab": []byte("data2"),
{0x00, 0x01, 0x02}, // Key without terminator flag "abc": []byte("data3"),
{0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, // 32-byte hash },
0x0e, 0x0f, 0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18, 0x19, 0x1a, owner: common.Hash{},
0x1b, 0x1c, 0x1d, 0x1e, 0x1f}, expectedAccountDepth: 3, // Only "abc" is a leaf
}), expectedStorageDepth: 0,
want: false,
}, },
{ {
name: "extension node with odd key (no terminator)", name: "multiple account trie branches",
// Compact encoding: first byte 0x10 means odd length key without terminator nodes: map[string][]byte{
nodeData: mustEncodeNode(t, [][]byte{ "a": []byte("data1"),
{0x10, 0x01, 0x02, 0x03}, // Key without terminator flag (odd length) "ab": []byte("data2"),
{0x01, 0x02, 0x03, 0x04}, // Could be hash reference "abc": []byte("data3"),
}), "b": []byte("data4"),
want: false, "bc": []byte("data5"),
"bcd": []byte("data6"),
},
owner: common.Hash{},
expectedAccountDepth: 6, // "abc" (3) + "bcd" (3) = 6
expectedStorageDepth: 0,
}, },
{ {
name: "branch node", name: "siblings are all leaves",
// Branch nodes have 17 elements nodes: map[string][]byte{
nodeData: mustEncodeNode(t, [][]byte{ "aa": []byte("data1"),
{}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, "ab": []byte("data2"),
}), "ac": []byte("data3"),
want: false, },
owner: common.Hash{},
expectedAccountDepth: 6, // 2 + 2 + 2 = 6
expectedStorageDepth: 0,
}, },
{ {
name: "empty data", name: "storage trie leaves",
nodeData: []byte{}, nodes: map[string][]byte{
want: false, "1": []byte("data1"),
"12": []byte("data2"),
"123": []byte("data3"),
"124": []byte("data4"),
},
owner: common.HexToHash("0x1234"),
expectedAccountDepth: 0,
expectedStorageDepth: 6, // "123" (3) + "124" (3) = 6
}, },
{ {
name: "invalid RLP", name: "complex trie structure",
nodeData: []byte{0xff, 0xff, 0xff}, nodes: map[string][]byte{
want: false, "1": []byte("data1"),
"12": []byte("data2"),
"123": []byte("data3"),
"124": []byte("data4"),
"2": []byte("data5"),
"23": []byte("data6"),
"234": []byte("data7"),
"235": []byte("data8"),
"3": []byte("data9"),
},
owner: common.Hash{},
expectedAccountDepth: 13, // "123"(3) + "124"(3) + "234"(3) + "235"(3) + "3"(1) = 13
expectedStorageDepth: 0,
}, },
} }
for _, tt := range tests { for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
got := isLeafNode(tt.nodeData) stats := NewWitnessStats()
if got != tt.want { stats.Add(tt.nodes, tt.owner)
t.Errorf("isLeafNode() = %v, want %v", got, tt.want)
// Check account trie depth
if stats.accountTrie.totalDepth != tt.expectedAccountDepth {
t.Errorf("Account trie total depth = %d, want %d", stats.accountTrie.totalDepth, tt.expectedAccountDepth)
}
// Check storage trie depth
if stats.storageTrie.totalDepth != tt.expectedStorageDepth {
t.Errorf("Storage trie total depth = %d, want %d", stats.storageTrie.totalDepth, tt.expectedStorageDepth)
} }
}) })
} }
} }
func mustEncodeNode(t *testing.T, elems [][]byte) []byte { func TestWitnessStatsMinMax(t *testing.T) {
data, err := rlp.EncodeToBytes(elems)
if err != nil {
t.Fatalf("Failed to encode node: %v", err)
}
return data
}
func TestWitnessStats(t *testing.T) {
// Create a witness stats collector
stats := NewWitnessStats() stats := NewWitnessStats()
// Create witness data with both leaf and non-leaf nodes // Add some account trie nodes with varying depths
witness := map[string][]byte{ stats.Add(map[string][]byte{
// Leaf node at depth 4 (path length 4) "a": []byte("data1"),
"abcd": mustEncodeNode(t, [][]byte{ "ab": []byte("data2"),
{0x20, 0x01, 0x02}, // Key with terminator "abc": []byte("data3"),
{0x01, 0x02}, // Value "abcd": []byte("data4"),
}), "abcde": []byte("data5"),
// Extension node at depth 2 (should not be counted) }, common.Hash{})
"ab": mustEncodeNode(t, [][]byte{
{0x00, 0x01, 0x02}, // Key without terminator // Only "abcde" is a leaf (depth 5)
{0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, if stats.accountTrie.minDepth != 5 {
0x0e, 0x0f, 0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18, 0x19, 0x1a, t.Errorf("Account trie min depth = %d, want %d", stats.accountTrie.minDepth, 5)
0x1b, 0x1c, 0x1d, 0x1e, 0x1f}, // 31-byte hash (simulated) }
}), if stats.accountTrie.maxDepth != 5 {
// Another leaf node at depth 6 t.Errorf("Account trie max depth = %d, want %d", stats.accountTrie.maxDepth, 5)
"abcdef": mustEncodeNode(t, [][]byte{
{0x30, 0x01}, // Key with terminator
{0x03, 0x04}, // Value
}),
// Branch node (should not be counted)
"a": mustEncodeNode(t, [][]byte{
{}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {},
}),
} }
// Add account trie data (zero owner hash) // Add more leaves with different depths
stats.Add(witness, common.Hash{}) stats.Add(map[string][]byte{
"x": []byte("data6"),
"yz": []byte("data7"),
}, common.Hash{})
// Verify only leaf nodes were counted // Now we have leaves at depths 1, 2, and 5
if stats.accountTrie.samples != 2 { if stats.accountTrie.minDepth != 1 {
t.Errorf("Expected 2 leaf nodes in account trie, got %d", stats.accountTrie.samples) t.Errorf("Account trie min depth after update = %d, want %d", stats.accountTrie.minDepth, 1)
} }
if stats.accountTrie.maxDepth != 5 {
// Check the depth statistics t.Errorf("Account trie max depth after update = %d, want %d", stats.accountTrie.maxDepth, 5)
expectedAvg := int64((4 + 6) / 2) // Average of path lengths 4 and 6 }
if stats.accountTrie.totalDepth/stats.accountTrie.samples != expectedAvg { }
t.Errorf("Expected average depth %d, got %d", expectedAvg, stats.accountTrie.totalDepth/stats.accountTrie.samples)
} func TestWitnessStatsAverage(t *testing.T) {
if stats.accountTrie.minDepth != 4 { stats := NewWitnessStats()
t.Errorf("Expected min depth 4, got %d", stats.accountTrie.minDepth)
} // Add nodes that will create leaves at depths 2, 3, and 4
if stats.accountTrie.maxDepth != 6 { stats.Add(map[string][]byte{
t.Errorf("Expected max depth 6, got %d", stats.accountTrie.maxDepth) "aa": []byte("data1"),
} "bb": []byte("data2"),
"ccc": []byte("data3"),
// Test storage trie (non-zero owner hash) "dddd": []byte("data4"),
storageStats := NewWitnessStats() }, common.Hash{})
storageWitness := map[string][]byte{
// Leaf node // All are leaves: 2 + 2 + 3 + 4 = 11 total, 4 samples
"xyz": mustEncodeNode(t, [][]byte{ expectedAvg := int64(11) / int64(4)
{0x20, 0x01}, // Key with terminator actualAvg := stats.accountTrie.totalDepth / stats.accountTrie.samples
{0x05, 0x06}, // Value
}), if actualAvg != expectedAvg {
} t.Errorf("Account trie average depth = %d, want %d", actualAvg, expectedAvg)
storageStats.Add(storageWitness, common.HexToHash("0x1234")) }
}
if storageStats.storageTrie.samples != 1 {
t.Errorf("Expected 1 leaf node in storage trie, got %d", storageStats.storageTrie.samples) func BenchmarkWitnessStatsAdd(b *testing.B) {
} // Create a realistic trie node structure
if storageStats.accountTrie.samples != 0 { nodes := make(map[string][]byte)
t.Errorf("Expected 0 nodes in account trie for storage access, got %d", storageStats.accountTrie.samples) for i := 0; i < 100; i++ {
base := string(rune('a' + i%26))
nodes[base] = []byte("data")
for j := 0; j < 9; j++ {
key := base + string(rune('0'+j))
nodes[key] = []byte("data")
}
}
stats := NewWitnessStats()
b.ResetTimer()
for i := 0; i < b.N; i++ {
stats.Add(nodes, common.Hash{})
} }
} }