Repository navigation
helix: rebase the ow/balance-floor commits onto main (v31) - #1
Closed
adrianchifor wants to merge 12 commits into
Closed
adrianchifor wants to merge 12 commits into
adrianchifor wants to merge 12 commits into
Conversation
A block builder that merges onto a fixed parent produces many payloads from one base. Each currently re-derives the whole state root, even though the base's account and storage updates are identical across them -- measured at ~93% of the walk on mainnet traffic. Two seams make that avoidable without duplicating finalisation: - `finalize_payload_with_state_root` takes a root the caller already holds and fills every other header field exactly as `finalize_payload` does. It cannot be written outside this crate: `burned_fees` reads `PayloadBuildContext::base_fee_per_blob_gas`, which is private. - `merkleize_updates` exposes the existing parallel per-account walk. The only public route today is the serial `Store::apply_account_updates_from_trie_batch`. Both delegate to the existing implementations; no behaviour changes.
… backups on L1 on request; cache the blob base fee
Branch nodes already build their RLP in place (lambdaclass#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.
`Trie::compute_hash_from_unsorted_iter` builds the transactions, receipts and withdrawals roots. Every leaf of the first two holds a whole encoded transaction or receipt, so hashing the leaves is most of their cost: on a 263-transaction mainnet block, 70% of the transactions root is keccak over the transactions' own bytes. The subtries under the top four branch levels are now hashed on all cores before the root hash, which then only combines cached child hashes. Four levels, because index keys are skewed: every index from 128 to 255 sits under the path 8, 1, so one subtrie two levels down holds half the leaves of a typical block. Builds without std, such as the zkVM guest, keep the sequential hashing. A test checks the root matches sequential hashing across the index boundaries where key length changes.
clippy's items_after_test_module rejected the helper and its constant, which sat after the `tests` module; lint-l2 checks all targets, so it failed there.
…l pool compute_hash_from_unsorted_iter fanned the top subtries out on the global pool, and it runs on async request paths: the engine API builds a block's header, transactions root included, on a tokio worker, and snap sync checks receipts roots from async code. A caller outside a rayon pool blocks until a worker takes the job, so while the global pool was busy for tens of minutes (the post-snap-sync storage trie validation keeps every worker busy with par_bridge work) each newPayload pinned another tokio worker. With 16 workers that took about 3 minutes, after which the node stopped answering RPC and the multisync monitor failed mainnet and sepolia during validation. The fan-out now runs on a dedicated pool built on first use, so a root waits only for itself; if that pool cannot be built the root is hashed sequentially. A test occupies every global-pool worker and requires the root within 5 s.
Squashed from the zkevm autoresearch branch perf/state-access (5 commits), measured on the zisk guest benchmark: - journaled Substate (revm-style undo log): O(1) commit_backup and O(1) warmth checks, replacing the linked list of per-frame sets - identity/truncation hashers for the warmth maps (accessed_addresses, accessed_storage_slots) instead of fxhash's 20/32-byte lbu chains - LevmAccount.storage FxHashMap -> SlotMap (SlotHasher) for the per-SLOAD value lookup - VM::get_account_mut does one load_account instead of two - CacheDB accounts_state maps -> TruncMap (identity hasher on the hottest map) Excludes the server-local build artifacts committed on the source branch. Fixups on top of it: rustfmt; hashers use first_chunk/last_chunk so they pass levm's arithmetic_side_effects and indexing_slicing lints; revert_backup drains the journal in reverse instead of popping under an expect_used allowance; drop the imports the CacheDB signature change left unused; scope the lint allowances for the journal's randomized reference model; and update destroyed_refault_tests plus main's bal_validate_tx_execution_tests for the CacheDB/SlotMap types (the source branch left the root test crate uncompilable, which CI's Lint job skips). Rebased onto v31.0.0 without the hasher changes: upstream (lambdaclass#7408) seeds the warmth, storage and account-cache maps because contracts choose their keys, and the unkeyed truncating hashers would let a block force long collision chains. The journaled substate and the single-load get_account_mut are kept.
…he coinbase address Rebased onto v31.0.0 with TxReads keyed by SeededHashMap, matching the upstream rule (lambdaclass#7408) for maps keyed by contract-chosen values.
…p to 64 bytes Rebased onto v31.0.0: v31 dispatches keccak through a backend table (scalar assembly or AVX-512VL); the scalar entry now names the prefixed symbols. The memo sits in front of v31's one-shot hash_with, which also computes the hash on a miss.
…ores for wrapped tries
…t another balance A transaction that only moves value through an account depends on its balance only through the checks that guard each debit. For each passed check, TxReads.floors keeps the least first-read balance with which it still passes; a failed check or SELFDESTRUCT counts as inspecting the balance.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
helix-builder pins this fork for the hooks its merge builder and simulation role need. Until now it pinned
ow/balance-floor: v29.0.1 plus 14 commits. This branch rebases those commits onto currentmain(upstream as of Nov 5, which contains everything in v31.0.0), and gattaca-com/helix#714 pins helix to it. It is opened for review of the rebase; helix pins the commit, so the branch does not need to merge.12 of the 14 commits carry over. Eight applied without changes: the two merkleizer hooks, the precomputed receipts root and logs bloom in finalize, the precomputed transactions root and trie node stores, the balance floors, and the four ordered-trie hashing commits from Diego and Iñaki. These commits needed review:
perf(levm): journaled substate and faster state maps. Besides the journal, this commit replaced the hashers on the EVM's warm-address and warm-slot sets, account storage and the account cache with unkeyed truncating hashers tuned for the ZisK zkVM. Upstream fix(l1): seed the hashers of every map keyed by contract- or block-chosen values lambdaclass/ethrex#7408 seeds those same maps, because contracts choose their keys: with an unkeyed hasher a block can pick thousands of colliding keys, and every lookup then walks one long chain. helix-builder validates blocks from untrusted builders, so this branch keeps upstream's seeded maps and drops the hasher change andhashers.rs. The journaled substate and the single-loadget_account_mutare kept. Thatget_account_mutmatches upstream'sget_accountplusget_account_mut, done with one load. The commit's differential test against the layer reference model passes.fix(levm): CI lint fixes for new state mapsandfix(levm): drop the eip-8025 cfg qualifier main removedare dropped. They only adjusted imports for the removed hashers.TxReads.accountsandTxReads.slotsare keyed by addresses and slots the transaction chooses, so they now useSeededHashMapfor the same reason.ethrex_-prefixed assembly symbols. The memo for inputs up to 64 bytes sits in front of upstream's one-shothash_with, which also computes the hash on a miss. One hex constant in the memo is regrouped for clippy; its value is unchanged.merkleize_updates. It takes aSeededHashMap, following upstream's map type. The original commit had placed it betweenhandle_merkleization_bal_from_updatesand that function's doc comment and#[instrument]attribute, which then applied to the wrong function. It now sits above them.Every commit is
cargo fmtclean, and clippy is clean on the crates they touch. The ethrex-levm, ethrex-trie and ethrex-crypto unit tests pass. The original authors are kept. The journal, first-reads and keccak commits each note their rebase changes in the commit message.