For an InternalNode at a group-boundary depth, hashInternal previously
computed pure SHA256(left, right) recursion over the natural-depth
in-memory tree built by UpdateStem. But serializeSubtree extends stems
to the group's bottom layer via key-bit extension, so the on-disk blob
encodes an extended-depth structure. When a fresh reader deserializes
that blob, hashInternal walks the extended-depth in-memory tree and
produces a different value.
The result was that for any subtree with multiple stems sharing a
prefix shorter than groupDepth, the parent's stored child-hash (computed
from the natural-depth in-memory tree at commit time) did not equal the
child blob's read-back hash. Geth's own write-read cycle was internally
inconsistent: state-actor's groundtruth test, which feeds the same
stems through state-actor's streaming builder and geth's UpdateStem +
Commit and diffs the resulting on-disk node sets, fails at n=4 with a
mismatched slot hash in the root group blob.
At a group boundary, recompute the hash via serializeSubtree +
groupedRecursiveHash so that the parent stores the same value the
reader will compute when it deserializes the child blob.
The fix is gated on groupDepth > 0, so nodeStore tests that construct
the store directly without going through NewBinaryTrie retain the
existing pure-SHA256 recursion semantics.
Verification:
- All existing trie/bintrie tests pass unchanged.
- state-actor/generator's TestStreamingMatchesGethCommit (which compares
state-actor's streaming builder output to geth's Commit output
byte-for-byte at n=2,4,8,32,128) now passes.
The compact LSB-aligned encoding via ActiveBytes packed paths into
ceil(len/8) bytes without recording the bit-length. Two distinct paths
whose bit-lengths fell in the same byte-bucket and whose integer values
matched produced identical bytes — e.g. the 1-bit path "1" and the 8-bit
path "00000001" both encoded to [0x01], so two stems sitting at depths 1
and 8 on different branches could clobber each other in nodeset.AddNode.
Replace ActiveBytes/PutActiveBytes with KeyBytes/PutKeyBytes, which
append a uint8 bit-length byte after the active bytes. Postpend (rather
than prepend) so nodes along a single root-to-leaf descent share leading
path bytes, improving LSM block locality during traversal.
The empty path is encoded as no bytes (not [0x00]): byteCount=0 is
unique to len=0 so no disambiguation byte is needed. This keeps the
root's DB key empty, matching the resolver's existing nil-path convention.
This PR addresses one of the biggest performance issue with binary
tries: storing each internal node individually bloats the index, the
disk, and triggers a lot of write amplifications. To fix this issue,
this PR serializes groups of nodes together.
Because we are still looking for the ideal group size, the "depth" of
the group tree is made a parameter, but that will be removed in the
future, once the perfect size is known.
This is a rebase of #33658
---------
Co-authored-by: Copilot <copilot@github.com>
## Summary
Replace the `BinaryNode` interface with `NodeRef uint32` indices into
typed arena pools, eliminating GC-scanned pointers from binary trie
nodes.
Inspired by [fjl's
observation](https://github.com/ethereum/go-ethereum/pull/34034#issuecomment-4075176446):
> *"if the binary trie produces such a large graph, it should probably
be changed so that the trie node type does not contain pointers. The
runtime does not scan objects that do not contain pointers, so it can
really help with the performance to build it this way."*
### The problem
CPU profiling of the binary trie (EIP-7864) showed **44% of CPU time in
garbage collection**. Each `InternalNode` held two `BinaryNode`
interface values (2 pointer-words each), and the GC scanned every one.
With ~25K `InternalNode`s in memory during block processing, this
created enormous GC pressure.
### The solution
`NodeRef` is a compact `uint32` (2-bit kind tag + 30-bit pool index).
`NodeStore` manages chunked typed pools per node kind:
- **InternalNode pool**: ZERO Go pointers (children are `NodeRef`, hash
is `[32]byte`) → noscan spans
- **HashedNode pool**: ZERO Go pointers → noscan spans
- **StemNode pool**: retains `Values [][]byte` (matching existing
format)
The serialization format is unchanged — flat InternalNode
`[type][leftHash][rightHash]` = 65 bytes.
## Benchmark: Apple M4 Pro (`--benchtime=10s --count=3`, on top of
#34021)
| Metric | Baseline | Arena | Delta |
|--------|----------|-------|-------|
| Approve (Mgas/s) | 374 | 382 | **+2.1%** |
| BalanceOf (Mgas/s) | 885 | 901 | **+1.8%** |
| Approve allocs/op | 775K | **607K** | **-21.7%** |
| BalanceOf allocs/op | 265K | **228K** | **-14.0%** |
## Benchmark: AMD EPYC 48-core (50GB state, execution-specs ERC-20, on
top of #34021 + #34032)
| Benchmark | Baseline | Arena | Delta |
|-----------|----------|-------|-------|
| erc20_approve (write) | 22.4 Mgas/s | **27.0 Mgas/s** | **+20.5%** |
| mixed_sload_sstore | 62.9 Mgas/s | **97.3 Mgas/s** | **+54.7%** |
| erc20_balanceof (read) | 180.8 Mgas/s | 167.6 Mgas/s | -7.3% (cold
cache variance) |
The arena benefit scales with heap size — the EPYC (larger heap, more GC
pressure) shows much larger gains than the M4 Pro (efficient unified
memory). The mixed workload baseline was unstable (62.9 vs 16.3 Mgas/s
between runs due to GC-induced throughput collapse); the arena
eliminates this entirely (95-97 Mgas/s, stable).
## Dependencies
Benchmarked with #34021 (H01 N+1 fix) + #34032 (R14 parallel hashing).
No code dependency — applies independently to master.
All test suites pass (`trie/bintrie` with `-race`, `core/state`,
`triedb/pathdb`, `cmd/geth`).
---------
Co-authored-by: Guillaume Ballet <3272758+gballet@users.noreply.github.com>
This Pr implements some prerequisite changes for #34004 : split the
`CachingDB` into a `MerkleDB` and a `UBTDB`, so that very different
behaviors don't clash as much.
The transition isn't handled by this PR, but after talking to Gary we
agreed that `UBTDB` should receive another `triedb`, which will only be
loaded if the `Ended` flag is set to false in the conversion contract.
If this is too hard to achieve, it makes sense to load it regardless,
and then loading can be prevented at a later stage by adding a
`UBTTransitionFinalizationTime` in `ChainConfig`.
---------
Co-authored-by: Gary Rong <garyrong0905@gmail.com>
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>
`BinaryTrie.DeleteAccount` was a no-op, silently ignoring the caller's
deletion request and leaving the old `BasicData` and `CodeHash` in the
trie.
Co-authored-by: Guillaume Ballet <3272758+gballet@users.noreply.github.com>
This is an optimization that existed for verkle and the MPT, but that
got dropped during the rebase.
Mark the nodes that were modified as needing recomputation, and skip the
hash computation if this is not needed. Otherwise, the whole tree is
hashed, which kills performance.
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.
The `Witness` method was not implemented for the binary tree, which
caused `debug_excutionWitness` to panic. This PR fixes that.
Note that the `TransitionTrie` version isn't implemented, and that's on
purpose: more thought must be given to what should go in the global
witness.
In order to reduce the amount of code that is embedded into the keeper
binary, I am removing all the verkle code that uses go-verkle and
go-ipa. This will be followed by further PRs that are more like stubs to
replace code when the keeper build is detected.
I'm keeping the binary tree of course. This means that you will still
see `isVerkle` variables all over the codebase, but they will be renamed
when code is touched (i.e. this is not an invitation for 30+ AI slop
PRs).
---------
Co-authored-by: Gary Rong <garyrong0905@gmail.com>
This is broken off of #31730 to only focus on testing networks that
start with verkle at genesis.
The PR has seen a lot of work since its creation, and it now targets
creating and re-executing tests for a binary tree testnet without the
transition (so it starts at genesis). The transition tree has been moved
to its own package. It also replaces verkle with the binary tree for
this specific application.
---------
Co-authored-by: Gary Rong <garyrong0905@gmail.com>
Implement the binary tree as specified in [eip-7864](https://eips.ethereum.org/EIPS/eip-7864).
This will gradually replace verkle trees in the codebase. This is only
running the tests and will not be executed in production, but will help
me rebase some of my work, so that it doesn't bitrot as much.
---------
Signed-off-by: Guillaume Ballet
Co-authored-by: Parithosh Jayanthi <parithosh.jayanthi@ethereum.org>
Co-authored-by: rjl493456442 <garyrong0905@gmail.com>
