Skip to content

perf(l1): allocation-free leaf and extension node hashing - #7155

Open
diegokingston wants to merge 4 commits into
mainfrom
perf/trie-allocation-free-node-encoding
Open

perf(l1): allocation-free leaf and extension node hashing#7155
diegokingston wants to merge 4 commits into
mainfrom
perf/trie-allocation-free-node-encoding

Conversation

@diegokingston

Copy link
Copy Markdown
Contributor

Motivation

Branch nodes already build their RLP in place (#7104); leaf and extension nodes still went through Encoder, which accumulates a node's payload in a heap Vec so it can prepend the list header, then copies the whole payload across. Together with the Vec that encode_compact returns, hashing one leaf cost about five allocations and two passes over its bytes.

This is aimed at the zkVM guest. ZisK's guest allocator is a bump pointer whose dealloc is a no-op and whose realloc always allocates a fresh block and memcpys (ziskos/entrypoint/src/alloc/bump.rs), so allocation churn there is permanently consumed heap and unavoidable copying — a growable, reclaiming allocator hides most of this cost on the host, and the guest has neither.

Description

Every node's payload length is knowable before any of it is written — for a leaf, the compact path length plus the value's RLP length; for an extension, the compact path length plus the child hash variant's width. So the header can go down first and the node can be built in place, which is the shape BranchNode::encode_into_vec already uses.

  • Nibbles::encode_compact_len derives the hex-prefix length from the nibble count alone, and encode_compact_into appends into a caller buffer. encode_compact keeps its signature for the p2p call sites and becomes a thin wrapper.
  • LeafNode::encode_into_vec / ExtensionNode::encode_into_vec take a concrete &mut Vec<u8>, mirroring the branch writer.
  • BranchNode::encode_into_vec still routed each of its 16 child hashes through hash.0.encode(&mut *buf), which takes a trait object — so the monomorphic wrapper added in perf(l1): give branch-node hashing a monomorphic RLP encoder #7104 avoided the outer dispatch and then paid 16 inner ones. Those are now direct pushes.
  • NodeRef::memoize_hashes memoized the subtrie and then called Node::compute_hash_no_alloc, whose first act is to memoize the subtrie again. Not exponential — the second pass finds every OnceLock already set — but every branch re-walked all 16 children to learn nothing. Split out hash_memoized for callers that have already done the walk.

RLPEncode::encode is deliberately left untouched, both because other callers depend on it and because it then serves as a differential oracle for the new writers.

Guest RISC-V cycles

Measured with ziskemu on a ZisK guest ELF that builds a 2,000-account trie, memoizes every hash (as get_embedded_root_committed does when a witness is decoded), dirties the paths a block would touch, and re-hashes. Keccak runs through ZisK's accelerator.

workload steps total cost
200 of 2,000 touched 11,971,068 → 11,260,324 (-5.94%) 1,664,380,558 → 1,601,452,041 (-3.78%)
2,000 of 2,000 touched 20,272,208 → 19,007,229 (-6.24%) 2,592,549,675 → 2,481,068,843 (-4.30%)

Both figures are for a program that also builds the trie from scratch and spends 16-17% of its cost in keccak, neither of which this touches — so the saving on the hashing itself is larger than the totals suggest.

The keccak opcode count is identical across the two builds (3,646 and 5,986 respectively): the same hashing work is done, and what disappears is encoding overhead. dma_memcpy drops 75,895 → 64,850, which is the bump allocator's realloc copies going away.

Host

hash_no_commit over a 500k-account trie arriving fully memoized, with only the dirtied paths re-encoded:

accounts touched allocations heap consumed realloc copied wall
200 1,011 → 2 52.6 KB → 1.5 KB 20.7 KB → 512 B 470 → 433 µs (-8%)
2,000 10,104 → 2 512 KB → 1.5 KB 203 KB → 512 B 3.53 → 3.09 ms (-12%)
20,000 101,184 → 2 5.11 MB → 1.5 KB 2.02 MB → 512 B 29.3 → 25.3 ms (-14%)

Correctness

  • Guest output byte-identical on RISC-V under ziskemu for both workloads above.
  • The full l1::execution_program run natively over a real block witness (hoodi 1,265,656, fixtures/cache/rpc_prover) reproduces initial_state_hash, final_state_hash and last_block_hash exactly.
  • Root hash identical at every host size in the table above.
  • 49,675 differential checks against the untouched RLPEncode implementations: every path length 0..=64 for leaf and extension, every single-byte value across the 0x80 boundary, every RLP header boundary (55/56/57, 255/256), and every NodeHash variant including the empty inline case. 0 failures.
  • ethrex-trie (61) and ethrex-common (171) suites pass; no_std builds; dependent crates build.

One behaviour is preserved deliberately: an extension node whose child is an empty inline hash contributes nothing to the payload, which is what the Encoder path produces today (NodeHash::encode writes encode_raw of an empty slice) even though RLPEncode::length reports 1 for that case. It only arises from a malformed trie. extension_child_len and put_extension_child are derived from the same match so they cannot drift apart.

Checklist

  • Updated STORE_SCHEMA_VERSION (crates/storage/lib.rs) if the PR includes breaking changes to the Store requiring a re-sync. — n/a, no Store changes; trie encodings are byte-identical.

Branch nodes already build their RLP in place (#7104); leaf and extension still
went through `Encoder`, which accumulates the payload in a heap `Vec` so it can
prepend the list header, then copies the whole payload across. Together with the
`Vec` that `encode_compact` returns, hashing one leaf cost about five
allocations and two passes over its bytes.

Every node's payload length is knowable before any of it is written -- for a
leaf, the compact path length plus the value's RLP length; for an extension, the
compact path length plus the child hash variant's width. So the header can go
down first and the node can be built in place, the same shape branch already
uses.

  * `Nibbles::encode_compact_len` derives the hex-prefix length from the nibble
    count alone, and `encode_compact_into` appends into a caller buffer.
    `encode_compact` keeps its signature for the p2p call sites and is now a
    thin wrapper.
  * `LeafNode::encode_into_vec` / `ExtensionNode::encode_into_vec` take a
    concrete `&mut Vec<u8>`, mirroring `BranchNode::encode_into_vec`.
  * `BranchNode::encode_into_vec` still routed each of its 16 child hashes
    through `hash.0.encode(&mut *buf)`, which takes a trait object -- so the
    monomorphic wrapper avoided the outer dispatch and paid 16 inner ones. Those
    are now direct pushes.
  * `NodeRef::memoize_hashes` memoized the subtrie and then called
    `Node::compute_hash_no_alloc`, whose first act is to memoize the subtrie
    again. Not exponential -- the second pass finds every `OnceLock` set -- but
    every branch re-walked all 16 children to learn nothing. Split out
    `hash_memoized` for callers that have already done the walk.

`RLPEncode::encode` is deliberately left untouched, both because other callers
depend on it and because it then serves as the differential oracle below.

This matters most in the zkVM guest, whose bump allocator's `dealloc` is a no-op
and whose `realloc` always allocates fresh and memcpys (ziskos
entrypoint/src/alloc/bump.rs), so allocation churn there is permanently consumed
heap rather than reused blocks.

Measured on the shape the guest actually runs: a 500k-account trie arrives fully
memoized (`get_embedded_root_committed` seeds every `OnceLock`), a block dirties
the paths it touched, and `hash_no_commit` re-encodes only those.

  touched   allocations      heap consumed   realloc copied   wall
      200   1,011 ->     2   52.6 KB -> 1.5K  20.7 KB -> 512B  470 -> 433 us
    2,000  10,104 ->     2    512 KB -> 1.5K   203 KB -> 512B  3.53 -> 3.09 ms
   20,000 101,184 ->     2   5.11 MB -> 1.5K  2.02 MB -> 512B  29.3 -> 25.3 ms

Host wall-clock understates the guest case: the system allocator reuses freed
blocks and can grow in place, and ZisK does neither, so the eliminated heap and
memcpy columns are worth more there than they are here.

Correctness. Root hash identical at every size above. The full L1 guest program
run natively over a real block witness (hoodi 1265656, fixtures/cache/rpc_prover)
reproduces initial_state_hash, final_state_hash and last_block_hash exactly.
49,675 differential checks against the untouched `RLPEncode` implementations --
every path length 0..=64 for leaf and extension, every single-byte value across
the 0x80 boundary, every RLP header boundary (55/56/57, 255/256), and every
`NodeHash` variant including the empty inline case -- 0 failures. `ethrex-trie`
(61) and `ethrex-common` (171) suites pass, `no_std` builds, dependent crates
build.

One behavioural subtlety preserved deliberately: an extension node whose child is
an empty inline hash contributes nothing to the payload, which is what the
`Encoder` path produces today (`NodeHash::encode` writes `encode_raw` of an empty
slice), even though `RLPEncode::length` reports 1 for that case. It only arises
from a malformed trie, and `extension_child_len` and `put_extension_child` are
derived from the same match so they cannot drift apart.
@diegokingston
diegokingston requested a review from a team as a code owner August 14, 2026 20:54
@github-actions

github-actions Bot commented Aug 14, 2026

Copy link
Copy Markdown

⚠️ Known Issues — intentionally skipped tests

Source: docs/known_issues.md

rpc-compat log-bearing cases excluded

Where: KNOWN_EXCLUDED_TESTS in .github/scripts/check-hive-results.sh counts out
eight hive rpc-compat cases — the four eth_getLogs cases, eth_getBlockReceipts/get-block-receipts-latest,
and three eth_getTransactionReceipt cases. They are exactly the cases whose recorded
response contains at least one log object; every case with an empty log array still runs.
Note this leaves eth_getLogs with no rpc-compat coverage at all, since all four of its
cases are in the set.

Why: ethrex populates blockTimestamp on log objects, as geth, besu, nethermind, reth
and erigon all do. hive's rpc-compat compares responses byte-exactly (jsondiff.FullMatch;
the lenient checkJSONStructure path applies only to cases upstream marks speconly), and
the corpus is pinned to execution-apis d08382ae (2025-02-10), whose recordings predate the
field — it entered the schema in execution-apis#639 and the fixtures in #846 (2026-07-22).
So the extra key cannot match, and this is a property of the pin rather than of the response.

The pin cannot move, and this is not temporary. The pin sits one commit before
execution-apis#627, which moved the test chain to a pre-merge genesis: the current corpus has
~36 proof-of-work blocks before its terminal total difficulty. ethrex does not support
pre-merge chains and will not, so importing that chain.rlp fails at block 1 —
validate_block_header has no pre-London base-fee path. Every revision carrying
blockTimestamp in its fixtures also carries that chain, so there is no revision that
satisfies both. Nor can the corpus be patched locally: rpc-compat's Dockerfile clones
ethereum/execution-apis by hard-coded URL, so the branch buildarg cannot point at a fork.

Coverage: the field itself is pinned by
block_timestamp_is_on_the_log_and_not_on_the_receipt in
crates/networking/rpc/types/receipt.rs, which asserts it is present on each log and absent
from the receipt level.

Removal: delete the entries if ethrex ever gains pre-merge chain import, or if upstream
marks these cases speconly so they are type-checked instead of compared byte-for-byte.


The stateless schema id does not identify the encoding

Where: STATELESS_INPUT_SCHEMA_ID in crates/common/types/stateless_ssz.rs.

Upstream keeps the stateless input schema id at 0x1501
(fork_index 0x15 << 8 | revision 0x01) across incompatible body changes. Three
encodings have now shipped under it: tests-zkevm@v0.6.2, then #3248 + #3278,
then #3356, which moved state, codes and public_keys from SszList to
ProgressiveList. ethrex speaks the last one.

The consequence is that the 2-byte prefix cannot be used to detect a stale or
mismatched bundle. A wrong-dialect input is accepted by the id check and then
fails later — in SSZ decode, or on a root that does not match — rather than being
rejected up front for what it is. only_amsterdam_schema_id_decodes therefore
proves less than its name suggests.

Worth raising upstream: a revision field that does not move across a body change
provides no version negotiation at all.


ZisK guest program hash changes with the unsync_cell gate

Where: crates/common/types/block.rs, transaction.rs.

The gate on the single-threaded unsync_cell::OnceCell moved from
all(feature = "eip-8025", target_arch = "riscv64") to
all(feature = "zisk", target_arch = "riscv64") when the eip-8025 feature was removed.

The guest ELFs were previously built --features "<zkvm>-build-elf,ci", which never enabled
eip-8025, so they compiled the atomic once_cell variant. bin/zisk/Cargo.toml does enable
ethrex-common/zisk, so the ZisK guest now compiles the unsafe impl Sync cell instead.
That changes the ELF bytes and therefore the program hash and verification key.

This is intended (the guest is single-threaded, so the unsync cell is sound and cheaper), but it
is a VK change rather than a no-op refactor, and the diffstat presents it as a file rename
(eip8025_cell.rsunsync_cell.rs). Anyone pinning a ZisK VK across this change must
re-register it. The stateless-validator crate now forwards ethrex-common/zisk from its own
zisk feature so the two ZisK guests do not disagree on the cell type.


Release signing key is an unprotected repository secret

Where: .github/workflows/tag_release.yaml.

MINISIGN_SECRET_KEY is a plain repository secret. There is no environment: on
finalize-release or dry-run-release-assets, and gh api repos/lambdaclass/ethrex/rulesets
shows only branch-targeted rulesets, so the github.ref_type == 'tag' condition is a workflow
check rather than an enforced boundary: anyone who can push a tag can reach the signing key.

This is a repository-settings change, not a code change, so it is recorded here rather than
fixed in the tree. Recommended:

  1. Move MINISIGN_SECRET_KEY / MINISIGN_PASSWORD into a GitHub Environment with required
    reviewers, and add environment: to the two jobs that sign.
  2. Add a ruleset targeting refs/tags/v* restricting who may create release tags.

Until then, the compromise of that key is silent and durable: signatures would still verify
against the committed .github/minisign.pub.

@github-actions github-actions Bot added L1 Ethereum client performance Block execution throughput and performance in general labels Aug 14, 2026
@github-actions

Copy link
Copy Markdown

🤖 Codex Code Review

No blocking findings in the diff.

  • Low: Please add a byte-for-byte regression test around crates/common/trie/rlp.rs, crates/common/trie/rlp.rs, and crates/common/trie/nibbles.rs. These new allocation-free helpers now produce consensus-critical trie/RLP bytes directly instead of going through Encoder, so boundary cases like compact_len == 1, odd/even nibble counts, empty values, and inline-vs-hashed child refs deserve explicit equivalence coverage against the existing RLPEncode::encode_to_vec() path.

Static review-wise, the hash memoization split in crates/common/trie/node.rs looks sound and the buffer reuse changes in branch/leaf/extension hashing preserve the previous control flow.

I could not run cargo test in this environment because rustup/cargo need write access outside the workspace and dependency fetches are blocked.


Automated review by OpenAI Codex · gpt-5.4 · custom prompt

@github-actions

Copy link
Copy Markdown

Lines of code report

Total lines added: 93
Total lines removed: 6
Total lines changed: 99

Detailed view
+---------------------------------------------+-------+------+
| File                                        | Lines | Diff |
+---------------------------------------------+-------+------+
| ethrex/crates/common/trie/nibbles.rs        | 614   | +9   |
+---------------------------------------------+-------+------+
| ethrex/crates/common/trie/node.rs           | 467   | +3   |
+---------------------------------------------+-------+------+
| ethrex/crates/common/trie/node/branch.rs    | 584   | -2   |
+---------------------------------------------+-------+------+
| ethrex/crates/common/trie/node/extension.rs | 545   | -2   |
+---------------------------------------------+-------+------+
| ethrex/crates/common/trie/node/leaf.rs      | 304   | -2   |
+---------------------------------------------+-------+------+
| ethrex/crates/common/trie/rlp.rs            | 243   | +81  |
+---------------------------------------------+-------+------+

@github-actions

github-actions Bot commented Aug 19, 2026

Copy link
Copy Markdown

Benchmark Block Execution Results Comparison Against Main

Command Mean [s] Min [s] Max [s] Relative
base 86.515 ± 0.881 84.785 87.299 1.00 ± 0.01
head 86.448 ± 0.762 85.447 87.656 1.00

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

L1 Ethereum client performance Block execution throughput and performance in general

Projects

Status: No status
Status: Todo

Development

Successfully merging this pull request may close these issues.

1 participant