Skip to content

commitment: eliminate CellGetter callback — hashRow + pure EncodeBranch#20306

Merged
awskii merged 5 commits into
mainfrom
awskii/branch-encoding
Apr 9, 2026
Merged

commitment: eliminate CellGetter callback — hashRow + pure EncodeBranch#20306
awskii merged 5 commits into
mainfrom
awskii/branch-encoding

Conversation

@awskii

@awskii awskii commented Apr 3, 2026

Copy link
Copy Markdown
Member

Summary

Replaces the readCell func(nibble int, skip bool) callback pattern in BranchEncoder.CollectUpdate / EncodeBranch with direct data passing via a new hashRow method on HexPatriciaHashed.

Problem

createCellGetter created a closure capturing row, depth, b, updateKey. Because the closure body contains fmt.Printf(..., nibble, row, depth) in the trace path, Go's escape analysis moved all captured vars to the heap — even when hph.trace == false.

Solution

// New: hph.hashRow feeds keccak2 AND extracts cellEncodeData in one pass
func (hph *HexPatriciaHashed) hashRow(row int, depth int16, bitmap, afterMap uint16) ([16]cellEncodeData, error)

// Changed: pure serializer, no side effects
func (be *BranchEncoder) EncodeBranch(bitmap, touchMap, afterMap uint16, cells *[16]cellEncodeData) (BranchData, int, error)

// Changed: takes *[16]cellEncodeData instead of readCell func
func (be *BranchEncoder) CollectUpdate(ctx PatriciaContext, prefix []byte, bitmap, touchMap, afterMap uint16, cells *[16]cellEncodeData) (int, error)

Removed: createCellGetter, RetrieveCellNoop, readCell func(nibble int, skip bool) callback type.

Delete/propagate call-sites pass nil instead of RetrieveCellNoop.

Benchmark

Benchmark_HexPatriciaHashed_Process (10s, DO-Premium-AMD):

Before After Δ
ns/op 96208 ~99154 +3% (noise)
B/op 11121 ~10921 -200 (-1.8%)
allocs/op 113 ~110 -3 (-2.3%)

Pure code quality refactor — allocation reduction is a side effect of removing the closure.

Related

awskii and others added 2 commits April 3, 2026 11:51
Replace the readCell func(nibble int, skip bool) callback pattern with
direct data passing:

- New hph.hashRow(row, depth) feeds keccak2 and extracts [16]cellEncodeData
  in a single pass over the afterMap — no closure, no captured vars, no
  heap escapes from fmt.Printf in trace paths.
- EncodeBranch(bitmap, touchMap, afterMap, *[16]cellEncodeData) is now a
  pure serializer with no side effects.
- CollectUpdate takes *[16]cellEncodeData; nil/zero for delete cases,
  eliminating RetrieveCellNoop.
- createCellGetter and the readCell callback type are removed.

Benchmark (Benchmark_HexPatriciaHashed_Process, 10s):
  Before: 96208 ns/op  11121 B/op  113 allocs/op
  After:  99154 ns/op  10921 B/op  110 allocs/op
  Delta:  +3% ns (noise), -200 B/op (-1.8%), -3 allocs (-2.3%)

Co-authored-by: shuo <shuo@erigon.dev>
@awskii awskii mentioned this pull request Apr 7, 2026
15 tasks
@awskii awskii marked this pull request as ready for review April 8, 2026 12:47
@awskii awskii requested a review from mh0lt April 8, 2026 12:49
@awskii awskii enabled auto-merge April 9, 2026 13:30
…ce check

When USE_STATE_CACHE is on, SharedDomains.GetLatest serves reads from
stateCache without re-verifying against the backing tx. Under suspected
cache invalidation bugs, a stale cache entry silently propagates into
gas accounting (observed: tx #27 in Sepolia block 10619150 under-charged
by 13680 gas).

Add an ASSERT_STATE_CACHE env flag that, on every stateCache hit, fetches
the authoritative value from the backing tx and panics on any divergence.
This makes the bug fail loudly at the exact offending (domain, key) instead
of silently flowing through execution.

Set ASSERT_STATE_CACHE=true alongside USE_STATE_CACHE=true to trip the
panic on first divergence; the panic message identifies the domain, key,
cached value, DB value and txNum.
@awskii awskii added this pull request to the merge queue Apr 9, 2026
Merged via the queue into main with commit 9233287 Apr 9, 2026
34 of 36 checks passed
@awskii awskii deleted the awskii/branch-encoding branch April 9, 2026 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants