ethereum / go-verkle

A go implementation of Verkle trees
The Unlicense
209 stars 63 forks source link

refactor(tree): use const and type declaration #421

Closed weiihann closed 8 months ago

weiihann commented 9 months ago

This PR does the following:

  1. Use Stem []byte instead of []byte, as well as KeyToStem(key []byte) instead of key[:31] in most occurrences that require stem. This enhances code readability, without affecting the underlying logic.
  2. Replaces 32 with constants LeafValueSize or KeySize for code maintainability.

BREAKING CHANGE: The GetProofItems function in the VerkleNode interface now returns []Stem instead of [][]byte, which affects geth's usage of this library

Benchmark results:

                                        old allocs/op   new allocs/op  delta               
CommitLeaves/insert/leaves/1000-8         16.82k ± ∞ ¹   16.96k ± ∞ ¹  +0.87% (p=0.008 n=5)
CommitLeaves/insert/leaves/10000-8        155.4k ± ∞ ¹   155.3k ± ∞ ¹  -0.11% (p=0.008 n=5)
CommitFullNode-8                          3.795k ± ∞ ¹   3.795k ± ∞ ¹       ~ (p=1.000 n=5) ²
ModifyLeaves-8                            164.4k ± ∞ ¹   164.3k ± ∞ ¹       ~ (p=1.000 n=5)
weiihann commented 9 months ago

Thanks for the comments! I'm personally more keen to do something like ExtStem { stem: key[:31] } and KeyToStem(...) so it's safer and for better code readability. Will push the latest code changes ASAP.

weiihann commented 9 months ago

@gballet @jsign I've pushed a completely new set of changes and changed the PR title and description, please review again. I've decided to go with the ExtStem []byte structure instead of ExtStem { stem: key[:31] } structure. For code readability, I think it's better comparatively and has less code footprint.