Skip to content

various bugfixes found while testing#570

Closed
gballet wants to merge 8 commits intogballet:bintrie-offline-conversionfrom
tellabg:bintrie-conversion-v2
Closed

various bugfixes found while testing#570
gballet wants to merge 8 commits intogballet:bintrie-offline-conversionfrom
tellabg:bintrie-conversion-v2

Conversation

@gballet
Copy link
Copy Markdown
Owner

@gballet gballet commented Mar 4, 2026

No description provided.

tellabg added 8 commits March 1, 2026 14:01
- Add progress percentage (keyspace position 0x00..0xFF)
- Add timing instrumentation: t_hash, t_db, t_gc, t_reload
- Add per-commit details: node count, storage size, hash/db time
- Fix: detect existing binary trie root from previous partial conversions
- Fix: clear snap sync status flag before opening dest pathdb
- Pass initial root to runConversionLoop for correct pathdb layer tracking
GetBinaryTreeKeyStorageSlot had a byte-level manipulation bug: copy(k[1:],
key[:31]) wrote 31 bytes into k[1..31], then k[31] = key[31] overwrote k[31]
(which held key[30]). Two storage slots differing only in byte 30 would
silently map to the same binary trie position, causing data corruption.

Replace the ad-hoc byte manipulation with StorageIndex() which performs
correct uint256 arithmetic: treeIndex = (key >> 8) + (1 << 240), suffix =
key[31]. This also correctly handles header storage slots (key[31] < 64).

Add regression tests:
- TestStorageSlotByte30NotDropped: verifies no collision on byte 30
- TestStorageSlotConsistentWithStorageIndex: verifies consistency
- TestHeaderStorageSlot: verifies header storage path
…rageSlot

GetStorage and DeleteStorage were using GetBinaryTreeKey (no storage offset)
instead of GetBinaryTreeKeyStorageSlot, causing storage slots 0-63 to collide
with account header data in the binary trie.

Also add ResolveTime/ResolveCnt instrumentation to BinaryTrie for conversion
performance profiling.
Add SetAccessEvents method to StateDB for disabling verkle gas accounting
during binary trie replay (where IsVerkle=true for pathdb routing but
EIP-4762 is not active).

Fix nil pointer dereference in AccessEvents merge guards in
state_processor.go: check both statedb.AccessEvents() and evm.AccessEvents
before merging, instead of relying on IsVerkle() which may be true even
when AccessEvents is nil.
Add --bintrie CLI flag that configures geth to use the binary trie as
the state trie backend. When enabled:
- BlockChain opens the binary trie pathdb namespace
- Block validator skips MPT state root verification (binary trie has
  different roots)
- EthConfig propagates the flag through the stack

This requires a prior MPT-to-binary-trie conversion.
…preimages

Add detailed performance instrumentation to bintrie conversion:
- t_iter, t_insert, t_resolve, t_preimage, t_code, t_stoTrie, t_rlp
- Resolve time/count tracked in BinaryTrie struct
- t_other computed as total minus all measured phases

Add --fresh flag to delete existing binary trie data before starting
a new conversion (cleans verkle-prefixed keys, journal, ancient store).

Add --generate-preimages subcommand registration.

Skip missing preimages with a warning instead of crashing, tracking
the count in conversion stats.
- bintrie replay: replays blocks against the binary trie state offline,
  bypassing geth startup state verification. Supports --start, --end,
  --batch, --bintrie-root, and --no-commit flags.
- bintrie verify: spot-checks binary trie state against MPT state.
- bintrie generate-preimages: re-executes blocks to record storage key
  preimages that were missing from a sync without --cache.preimages.
… nodes

InternalNode and StemNode now cache their SHA-256 hash and invalidate
on mutation (Insert, InsertValuesAtStem). This avoids redundant hashing
during conversion commits where most subtrees are unchanged.

Additionally, InternalNode.Hash() parallelizes left/right child hashing
for nodes near the root (depth < 5), using up to 32 goroutines to
exploit multi-core CPUs during the hash phase.

Together these optimizations significantly reduce conversion hash time.
@gballet
Copy link
Copy Markdown
Owner Author

gballet commented Mar 11, 2026

Extracted all the things that I needed from this temporary branch, closing.

@gballet gballet closed this Mar 11, 2026
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