From 1cb8b7f01df85c95e5ad3f54d53f910682b0598f Mon Sep 17 00:00:00 2001 From: Edgar Date: Mon, 3 Aug 2026 17:21:11 +0200 Subject: [PATCH 1/8] perf(l1): store jump destinations as a bitmap A sorted `Arc<[u32]>` of JUMPDEST offsets, persisted next to the bytecode, costs ~4x the code size for jumpdest-dense contracts (97,906 B of value for 24,576 B of code) and ~10 ns per entry to rebuild on every code-cache miss: 246 us for a max-size runtime that is almost all JUMPDESTs. A bitmap is len/8 bytes at any density, decodes as a memcpy (0.5 us), and turns jump validation into a bit test rather than a binary search (12.9 -> 0.37 ns). Values written in the older form need no migration: the RLP item header tells a list from a byte string, and for a list the bitmap is rebuilt from the bytecode (26.6 us, still ~9x cheaper than decoding the list). `Code::size()` now counts the bytecode allocation it excluded, so the cache honors its byte budget instead of holding orders of magnitude more than it accounts for; the budget goes from 64 MiB that bounded only the jump tables to 256 MiB of actual bytecode. Re-inserting a cached hash no longer inflates the accounted size. --- crates/common/types/account.rs | 87 +++++--- crates/storage/store.rs | 196 ++++++++++++++++-- .../stack_memory_storage_flow.rs | 23 +- test/tests/common/code_serde_tests.rs | 2 +- test/tests/common/jumpdest_bitmap_tests.rs | 79 +++++++ test/tests/common/mod.rs | 1 + 6 files changed, 317 insertions(+), 71 deletions(-) create mode 100644 test/tests/common/jumpdest_bitmap_tests.rs diff --git a/crates/common/types/account.rs b/crates/common/types/account.rs index a01aca48004..e971e8450ef 100644 --- a/crates/common/types/account.rs +++ b/crates/common/types/account.rs @@ -22,7 +22,7 @@ use crate::constants::{EMPTY_KECCAK_HASH, EMPTY_TRIE_HASH}; /// `JUMPDEST` clone this (a refcount bump) instead of allocating a fresh empty /// `Arc` header each time. This matters because the per-tx `Code::default()` /// placeholder and every EOA / empty-code load would otherwise each allocate. -static EMPTY_JUMP_TARGETS: LazyLock> = LazyLock::new(|| Arc::from(Vec::new())); +static EMPTY_JUMPDESTS: LazyLock> = LazyLock::new(|| Arc::from(Vec::new())); /// Trailing STOP bytes appended to every bytecode so the dispatch loop can read /// the next opcode without a bounds check. 33 is the widest single-opcode advance @@ -42,13 +42,15 @@ pub struct Code { bytecode: Bytes, /// The real bytecode length, needed for some opcodes, `bytecode` is padded with 33 STOPs to avoid checked adds on hot loop. bytecode_len: usize, - // `Arc<[u32]>` so cloning `Code` (hot: every message-call resolves and clones - // the callee's code) is a refcount bump instead of deep-copying the table. + /// One bit per bytecode byte, set when that offset holds a `JUMPDEST` that is not + /// part of a `PUSH` immediate. A bitmap is `len/8` bytes regardless of how dense + /// the jump destinations are, and validating a jump is a bit test rather than a + /// search. + // + // `Arc<[u8]>` so cloning `Code` (hot: every message-call resolves and clones + // the callee's code) is a refcount bump instead of deep-copying the bitmap. // Serializes via serde's `rc` feature (enabled workspace-wide). - // The valid addresses are 32-bit because, despite EIP-3860 restricting initcode size, - // this does not apply to previous forks. This is tested in the EEST tests, which would - // panic in debug mode. - pub jump_targets: Arc<[u32]>, + jumpdests: Arc<[u8]>, } impl Code { @@ -59,26 +61,26 @@ impl Code { // `code` is the logical, unpadded bytecode; `BYTECODE_PADDING` STOP bytes are // appended internally by `from_parts_unchecked`. pub fn from_bytecode_unchecked(code: Bytes, hash: H256) -> Self { - let jump_targets = Self::compute_jump_targets(&code); - Self::from_parts_unchecked(hash, &code, jump_targets) + let jumpdests = Self::compute_jumpdests(&code); + Self::from_parts_unchecked(hash, &code, jumpdests) } /// `code` is the logical, unpadded bytecode; `BYTECODE_PADDING` STOP bytes are /// appended internally by `from_parts_unchecked`. pub fn from_bytecode(code: Bytes, crypto: &dyn Crypto) -> Self { - let jump_targets = Self::compute_jump_targets(&code); + let jumpdests = Self::compute_jumpdests(&code); let hash = H256(crypto.keccak256(code.as_ref())); - Self::from_parts_unchecked(hash, &code, jump_targets) + Self::from_parts_unchecked(hash, &code, jumpdests) } /// Builds a `Code` from precomputed parts. The caller must guarantee `hash` - /// and `jump_targets` correspond to `code`; neither is recomputed or validated. + /// and `jumpdests` correspond to `code`; neither is recomputed or validated. /// /// `code` is the logical, unpadded bytecode: this function appends /// `BYTECODE_PADDING` STOP bytes and records the original length in /// `bytecode_len`. Never pass a pre-padded buffer, or the logical length and /// every `JUMPDEST`/`PUSH` offset derived from it would be wrong. - pub fn from_parts_unchecked(hash: H256, code: &[u8], jump_targets: Arc<[u32]>) -> Self { + pub fn from_parts_unchecked(hash: H256, code: &[u8], jumpdests: Arc<[u8]>) -> Self { let bytecode_len = code.len(); let mut padded_code = Vec::with_capacity(bytecode_len + BYTECODE_PADDING); padded_code.extend_from_slice(code); @@ -87,20 +89,23 @@ impl Code { hash, bytecode: Bytes::from_owner(padded_code), bytecode_len, - jump_targets, + jumpdests, } } - fn compute_jump_targets(code: &[u8]) -> Arc<[u32]> { - debug_assert!(code.len() <= u32::MAX as usize); - let mut targets = Vec::new(); + /// Builds the [`Code::jumpdests`] bitmap: one pass over the bytecode, setting the + /// bit for every `JUMPDEST` while skipping `PUSH` immediates. + pub fn compute_jumpdests(code: &[u8]) -> Arc<[u8]> { + let mut bitmap = vec![0u8; code.len().div_ceil(8)]; + let mut any = false; let mut i = 0; while i < code.len() { // TODO: we don't use the constants from the vm module to avoid a circular dependency match code[i] { // OP_JUMPDEST 0x5B => { - targets.push(i as u32); + bitmap[i / 8] |= 1 << (i % 8); + any = true; } // OP_PUSH1..32 c @ 0x60..0x80 => { @@ -111,15 +116,31 @@ impl Code { } i += 1; } - // Share the single empty table for jumpless bytecode (very common: EOAs, - // tiny contracts) so we don't allocate an `Arc` header for an empty slice. - if targets.is_empty() { - EMPTY_JUMP_TARGETS.clone() + // Share the single empty bitmap for jumpless bytecode (very common: EOAs, + // tiny contracts) so we don't allocate for an all-zero map; `is_valid_jumpdest` + // reads a missing byte as "no jump destination". + if any { + Arc::from(bitmap) } else { - Arc::from(targets) + EMPTY_JUMPDESTS.clone() } } + /// Whether `offset` is a valid jump destination, i.e. it holds a `JUMPDEST` that is + /// not part of a `PUSH` immediate. Offsets past the bytecode are not valid. + #[inline] + pub fn is_valid_jumpdest(&self, offset: usize) -> bool { + self.jumpdests + .get(offset / 8) + .is_some_and(|byte| byte & (1 << (offset % 8)) != 0) + } + + /// The raw [`Code::jumpdests`] bitmap, for persisting it alongside the bytecode. + #[inline] + pub fn jumpdests(&self) -> &[u8] { + &self.jumpdests + } + #[inline] pub fn code(&self) -> &[u8] { self.bytecode.get(..self.bytecode_len).unwrap_or_default() @@ -151,16 +172,18 @@ impl Code { /// Estimates the size of the Code struct in bytes /// (including stack size and heap allocation). /// - /// Note: This is an estimation and may not be exact. + /// Note: This is an estimation and may not be exact. Shared allocations (the + /// empty bitmap, a `Bytes` slice of a larger buffer) are counted in full, so the + /// estimate is an upper bound. /// /// # Returns /// /// usize - Estimated size in bytes pub fn size(&self) -> usize { let hash_size = size_of::(); - let bytes_size = size_of::(); - let vec_size = size_of::>() + self.jump_targets.len() * size_of::(); - hash_size + bytes_size + vec_size + let bytes_size = size_of::() + self.bytecode.len(); + let bitmap_size = size_of::>() + self.jumpdests.len(); + hash_size + bytes_size + bitmap_size } } @@ -174,7 +197,7 @@ impl Code { struct CodeSerde { hash: H256, code: Bytes, - jump_targets: Arc<[u32]>, + jumpdests: Arc<[u8]>, } impl Serialize for Code { @@ -182,7 +205,7 @@ impl Serialize for Code { CodeSerde { hash: self.hash, code: self.code_bytes(), - jump_targets: self.jump_targets.clone(), + jumpdests: self.jumpdests.clone(), } .serialize(serializer) } @@ -193,9 +216,9 @@ impl<'de> Deserialize<'de> for Code { let CodeSerde { hash, code, - jump_targets, + jumpdests, } = CodeSerde::deserialize(deserializer)?; - Ok(Self::from_parts_unchecked(hash, &code, jump_targets)) + Ok(Self::from_parts_unchecked(hash, &code, jumpdests)) } } @@ -263,7 +286,7 @@ impl Default for Code { bytecode: Bytes::from_static(&[0u8; BYTECODE_PADDING]), bytecode_len: 0, hash: *EMPTY_KECCAK_HASH, - jump_targets: EMPTY_JUMP_TARGETS.clone(), + jumpdests: EMPTY_JUMPDESTS.clone(), } } } diff --git a/crates/storage/store.rs b/crates/storage/store.rs index e0288d1267e..5e40105d48d 100644 --- a/crates/storage/store.rs +++ b/crates/storage/store.rs @@ -36,7 +36,7 @@ use ethrex_common::{ }; use ethrex_crypto::{NativeCrypto, keccak::keccak_hash}; use ethrex_rlp::{ - decode::{RLPDecode, decode_bytes}, + decode::{RLPDecode, decode_bytes, decode_rlp_item}, encode::RLPEncode, }; use ethrex_trie::{EMPTY_TRIE_HASH, Nibbles, Trie, TrieLogger, TrieNode, TrieWitness}; @@ -120,8 +120,15 @@ enum FKVGeneratorControlMessage { Continue, } -// 64mb -const CODE_CACHE_MAX_SIZE: u64 = 64 * 1024 * 1024; +/// Byte budget for the in-memory bytecode cache. +/// +/// Bytecode is a small fraction of the working set next to trie nodes and flat +/// key-values (which the RocksDB block cache serves), but a cold code read costs a +/// blob-file fetch, so caching contracts pays for itself: 256 MiB holds ~10k max-size +/// (24 KiB) contracts, or ~100k of typical size. Before [`Code::size`] counted the +/// bytecode, this budget bounded only the jump-destination tables, and the cache could +/// hold multiple GiB of code without evicting. +const CODE_CACHE_MAX_SIZE: u64 = 256 * 1024 * 1024; /// Key used to persist the `flushed_upto` block number in `MISC_VALUES`. const FLUSHED_UPTO_KEY: &[u8] = b"bodies_flushed_upto"; @@ -136,6 +143,7 @@ const MAX_BAD_BLOCKS: usize = 16; struct CodeCache { inner_cache: LruCache, cache_size: u64, + max_size: u64, } impl Default for CodeCache { @@ -143,6 +151,7 @@ impl Default for CodeCache { Self { inner_cache: LruCache::unbounded_with_hasher(FxBuildHasher), cache_size: 0, + max_size: CODE_CACHE_MAX_SIZE, } } } @@ -153,15 +162,17 @@ impl CodeCache { } fn insert(&mut self, code: &Code) -> Result<(), StoreError> { - let code_size = code.size(); - let cache_len = self.inner_cache.len() + 1; - self.cache_size += code_size as u64; - let current_size = self.cache_size; - debug!( - "[ACCOUNT CODE CACHE] cache elements (): {cache_len}, total size: {current_size} bytes" - ); + // A hash already cached must not be added to `cache_size` again, or the counter + // drifts up permanently and evicts entries that fit. `get` also refreshes + // recency, which is what a repeated read should do. + if self.inner_cache.get(&code.hash).is_some() { + return Ok(()); + } + + self.cache_size += code.size() as u64; + self.inner_cache.put(code.hash, code.clone()); - while self.cache_size > CODE_CACHE_MAX_SIZE { + while self.cache_size > self.max_size { if let Some((_, code)) = self.inner_cache.pop_lru() { self.cache_size -= code.size() as u64; } else { @@ -169,7 +180,11 @@ impl CodeCache { } } - self.inner_cache.get_or_insert(code.hash, || code.clone()); + let cache_len = self.inner_cache.len(); + let current_size = self.cache_size; + debug!( + "[ACCOUNT CODE CACHE] cache elements (): {cache_len}, total size: {current_size} bytes" + ); Ok(()) } } @@ -912,11 +927,11 @@ impl Store { else { return Ok(None); }; - let (bytecode_slice, targets) = decode_bytes(&bytes)?; + let (bytecode_slice, jumpdests) = decode_bytes(&bytes)?; let code = Code::from_parts_unchecked( code_hash, bytecode_slice, - >::decode(targets)?.into(), + decode_jumpdests(bytecode_slice, jumpdests)?, ); // insert into cache and evict if needed @@ -4887,15 +4902,27 @@ pub fn receipt_key(block_hash: &BlockHash, index: u64) -> Vec { } fn encode_code(code: &Code) -> Vec { - let mut buf = - Vec::with_capacity(6 + code.len() + std::mem::size_of_val::<[u32]>(&code.jump_targets)); + let jumpdests = code.jumpdests(); + let mut buf = Vec::with_capacity(6 + code.len() + jumpdests.len()); code.code().encode(&mut buf); - // `Arc<[u32]>` (the in-memory share) has no `RLPEncode` impl; encode through an - // owned `Vec` on this cold DB-write path (code is persisted once per hash). - code.jump_targets.to_vec().encode(&mut buf); + jumpdests.encode(&mut buf); buf } +/// Decodes the JUMPDEST bitmap stored after the bytecode in an [`ACCOUNT_CODES`] value. +/// +/// Values written before the bitmap representation hold an RLP *list* of `u32` offsets +/// there instead. The RLP item header distinguishes a list from a byte string, so both +/// forms are readable: for the older one the bitmap is rebuilt from the bytecode, which +/// is cheaper than decoding the list. +fn decode_jumpdests(code: &[u8], encoded: &[u8]) -> Result, StoreError> { + let (is_list, payload, _) = decode_rlp_item(encoded)?; + if is_list { + return Ok(Code::compute_jumpdests(code)); + } + Ok(Arc::from(payload)) +} + #[derive(Debug, Default, Clone)] struct LatestBlockHeaderCache { current: Arc>>, @@ -5069,6 +5096,137 @@ pub fn read_chain_id_from_db(path: &Path) -> Option { } } +#[cfg(test)] +mod account_code_tests { + use super::*; + + const JUMPDEST: u8 = 0x5b; + const PUSH1: u8 = 0x60; + + /// A max-size contract that is almost entirely JUMPDESTs, the shape that makes the + /// jump-destination representation matter: as a list of offsets it is ~4x the size + /// of the bytecode it describes. + fn jumpdest_dense_code() -> Code { + let mut bytecode = vec![JUMPDEST; 24576]; + bytecode[0] = 0x00; + Code::from_bytecode_unchecked(bytecode.into(), H256::zero()) + } + + /// Encodes an `ACCOUNT_CODES` value the way it was written before the bitmap: the + /// bytecode followed by an RLP list of `u32` JUMPDEST offsets. + fn encode_code_legacy(code: &Code) -> Vec { + let offsets: Vec = (0..code.len()) + .filter(|offset| code.is_valid_jumpdest(*offset)) + .map(|offset| offset as u32) + .collect(); + let mut buf = Vec::new(); + code.code().encode(&mut buf); + offsets.encode(&mut buf); + buf + } + + #[test] + fn encoded_value_carries_one_bitmap_bit_per_code_byte() { + let code = jumpdest_dense_code(); + let encoded = encode_code(&code); + + assert_eq!(encoded.len(), 6 + code.len() + code.len().div_ceil(8)); + } + + #[test] + fn round_trip_preserves_the_bitmap() { + let code = jumpdest_dense_code(); + let encoded = encode_code(&code); + + let (bytecode, jumpdests) = decode_bytes(&encoded).unwrap(); + assert_eq!(bytecode, code.code()); + assert_eq!( + decode_jumpdests(bytecode, jumpdests).unwrap().as_ref(), + code.jumpdests() + ); + } + + /// Values written before the bitmap SHALL still decode, with the bitmap rebuilt from + /// the bytecode, so an existing database needs no migration. + #[test] + fn legacy_offset_list_values_decode_to_the_same_bitmap() { + for bytecode in [ + vec![0x00, JUMPDEST, 0x00, JUMPDEST], + vec![PUSH1, JUMPDEST, JUMPDEST], + vec![0x00; 32], + ] { + let code = Code::from_bytecode_unchecked(bytecode.into(), H256::zero()); + let legacy = encode_code_legacy(&code); + + let (decoded_bytecode, jumpdests) = decode_bytes(&legacy).unwrap(); + assert_eq!(decoded_bytecode, code.code()); + assert_eq!( + decode_jumpdests(decoded_bytecode, jumpdests) + .unwrap() + .as_ref(), + code.jumpdests() + ); + } + } + + /// Re-inserting a cached hash SHALL NOT grow the accounted size, or the counter + /// drifts up until the cache evicts entries that fit. + #[test] + fn repeated_inserts_do_not_inflate_the_accounted_size() { + let code = jumpdest_dense_code(); + let mut cache = CodeCache::default(); + + cache.insert(&code).unwrap(); + let after_first = cache.cache_size; + for _ in 0..8 { + cache.insert(&code).unwrap(); + } + + assert_eq!(cache.cache_size, after_first); + assert_eq!(cache.inner_cache.len(), 1); + } + + /// The accounted size SHALL include the bytecode itself, so the cache honors its + /// memory budget instead of holding orders of magnitude more than it accounts for. + #[test] + fn accounted_size_covers_the_bytecode_and_bitmap() { + let code = jumpdest_dense_code(); + let mut cache = CodeCache::default(); + cache.insert(&code).unwrap(); + + assert!(cache.cache_size >= (code.len() + code.jumpdests().len()) as u64); + } + + #[test] + fn cache_evicts_down_to_its_budget() { + let mut cache = CodeCache { + max_size: 128 * 1024, + ..Default::default() + }; + + for i in 0..16u8 { + let mut bytecode = vec![JUMPDEST; 24576]; + bytecode[0] = i; + cache + .insert(&Code::from_bytecode_unchecked( + bytecode.into(), + H256::from_low_u64_be(i.into()), + )) + .unwrap(); + } + + assert!(cache.cache_size <= cache.max_size); + assert!(cache.inner_cache.len() < 16); + } + + /// The default budget SHALL be the one the cache actually enforces, so a change to + /// the constant cannot silently leave the cache unbounded. + #[test] + fn default_cache_uses_the_configured_budget() { + assert_eq!(CodeCache::default().max_size, CODE_CACHE_MAX_SIZE); + } +} + #[cfg(test)] mod state_history_tests { use super::*; diff --git a/crates/vm/levm/src/opcode_handlers/stack_memory_storage_flow.rs b/crates/vm/levm/src/opcode_handlers/stack_memory_storage_flow.rs index 0e05cdb706a..4986a26f26d 100644 --- a/crates/vm/levm/src/opcode_handlers/stack_memory_storage_flow.rs +++ b/crates/vm/levm/src/opcode_handlers/stack_memory_storage_flow.rs @@ -467,25 +467,10 @@ impl OpcodeHandler for OpJumpIHandler { /// JUMPDEST charge), and the JUMPDEST step is pushed directly via /// `synthesize_step` after the gas is charged. fn jump(vm: &mut VM<'_>, target: usize, parent_gas_cost: u64) -> Result<(), VMError> { - // Check target address validity. - // - Target bytecode has to be a JUMPDEST. - // - Target address must not be blacklisted (aka. the JUMPDEST must not be part of a literal). - #[expect(clippy::as_conversions, reason = "safe")] - if vm - .current_call_frame - .bytecode - .dispatch_buf() - .get(target) - .is_some_and(|&value| { - value == Opcode::JUMPDEST as u8 - && vm - .current_call_frame - .bytecode - .jump_targets - .binary_search(&(target as u32)) - .is_ok() - }) - { + // Check target address validity: the target has to be a JUMPDEST that is not part + // of a literal (aka. a PUSH immediate). Both are answered by the bitmap, which only + // has bits set for JUMPDESTs reached by the opcode walk. + if vm.current_call_frame.bytecode.is_valid_jumpdest(target) { if vm.opcode_tracer.active { // Override the parent JUMP/JUMPI's gasCost so the dispatch loop // doesn't roll the upcoming JUMPDEST charge into it. diff --git a/test/tests/common/code_serde_tests.rs b/test/tests/common/code_serde_tests.rs index 1bedb27b413..172f2d7e2b6 100644 --- a/test/tests/common/code_serde_tests.rs +++ b/test/tests/common/code_serde_tests.rs @@ -19,7 +19,7 @@ fn code_serde_roundtrip_preserves_logical_code_and_repads() { assert_eq!(restored.code(), code.code()); assert_eq!(restored.len(), code.len()); assert_eq!(restored.hash, code.hash); - assert_eq!(restored.jump_targets, code.jump_targets); + assert_eq!(restored.jumpdests(), code.jumpdests()); // The dispatch buffer must carry the trailing padding after a round-trip. assert_eq!(restored.dispatch_buf().len(), code.len() + BYTECODE_PADDING); assert_eq!(restored, code); diff --git a/test/tests/common/jumpdest_bitmap_tests.rs b/test/tests/common/jumpdest_bitmap_tests.rs new file mode 100644 index 00000000000..908d38b1fff --- /dev/null +++ b/test/tests/common/jumpdest_bitmap_tests.rs @@ -0,0 +1,79 @@ +//! [`Code`]'s JUMPDEST bitmap: which offsets are valid jump destinations. + +use ethrex_common::H256; +use ethrex_common::types::Code; + +const JUMPDEST: u8 = 0x5b; +const PUSH1: u8 = 0x60; +const PUSH32: u8 = 0x7f; +const STOP: u8 = 0x00; + +fn code_of(bytecode: Vec) -> Code { + Code::from_bytecode_unchecked(bytecode.into(), H256::zero()) +} + +/// A `JUMPDEST` reached by the opcode walk SHALL be a valid destination. +#[test] +fn jumpdests_outside_immediates_are_valid() { + let code = code_of(vec![STOP, JUMPDEST, STOP, JUMPDEST]); + + assert!(code.is_valid_jumpdest(1)); + assert!(code.is_valid_jumpdest(3)); + assert!(!code.is_valid_jumpdest(0)); + assert!(!code.is_valid_jumpdest(2)); +} + +/// A `0x5b` byte that is part of a `PUSH` immediate SHALL NOT be a valid destination: +/// it is data, not an opcode. +#[test] +fn jumpdest_bytes_inside_push_immediates_are_not_valid() { + // PUSH1 0x5b | JUMPDEST | PUSH32 <32 x 0x5b> | JUMPDEST + let mut bytecode = vec![PUSH1, JUMPDEST, JUMPDEST, PUSH32]; + bytecode.extend([JUMPDEST; 32]); + bytecode.push(JUMPDEST); + let code = code_of(bytecode); + + assert!(!code.is_valid_jumpdest(1), "PUSH1 immediate"); + assert!(code.is_valid_jumpdest(2), "opcode between the two PUSHes"); + for offset in 4..36 { + assert!(!code.is_valid_jumpdest(offset), "PUSH32 immediate {offset}"); + } + assert!(code.is_valid_jumpdest(36), "opcode after the immediates"); +} + +/// Offsets past the bytecode SHALL NOT be valid, including offsets inside the trailing +/// [`BYTECODE_PADDING`](ethrex_common::types::BYTECODE_PADDING) and inside the last, +/// partially used bitmap byte. +#[test] +fn offsets_past_the_bytecode_are_not_valid() { + // 9 bytes of code, so the bitmap's second byte covers offsets 8..16 and only its + // lowest bit is meaningful. + let code = code_of(vec![JUMPDEST; 9]); + + assert!(code.is_valid_jumpdest(8)); + for offset in [9, 10, 15, 16, 100, usize::MAX] { + assert!(!code.is_valid_jumpdest(offset)); + } +} + +/// Jumpless bytecode SHALL NOT allocate a bitmap, and empty code SHALL have none. +#[test] +fn jumpless_bytecode_has_an_empty_bitmap() { + assert!(code_of(vec![STOP; 64]).jumpdests().is_empty()); + assert!(code_of(vec![PUSH1, JUMPDEST]).jumpdests().is_empty()); + assert!(Code::default().jumpdests().is_empty()); + assert!(!Code::default().is_valid_jumpdest(0)); +} + +/// The bitmap SHALL be one bit per bytecode byte. +#[test] +fn bitmap_is_one_bit_per_bytecode_byte() { + for len in [1usize, 7, 8, 9, 24576] { + let mut bytecode = vec![STOP; len]; + bytecode[len - 1] = JUMPDEST; + let code = code_of(bytecode); + + assert_eq!(code.jumpdests().len(), len.div_ceil(8), "len {len}"); + assert!(code.is_valid_jumpdest(len - 1), "len {len}"); + } +} diff --git a/test/tests/common/mod.rs b/test/tests/common/mod.rs index edda100bdf9..08239d4a4d7 100644 --- a/test/tests/common/mod.rs +++ b/test/tests/common/mod.rs @@ -6,6 +6,7 @@ mod blobs_bundle_tests; mod code_serde_tests; mod eip7702_authorization_tests; mod frame_tx_validation_tests; +mod jumpdest_bitmap_tests; mod legacy_signature_tests; mod logs_bloom_validation_tests; mod requests_eip8282_tests; From c44a6afc7e07fc19717fcc7f84f078bef3358c52 Mon Sep 17 00:00:00 2001 From: Edgar Date: Mon, 3 Aug 2026 17:21:18 +0200 Subject: [PATCH 2/8] perf(l1): bloom filter and 4KB blocks for the account-code CFs `account_codes` was the only exact-key point-lookup CF on the execution read path without a bloom filter, so a get had to read a data block per candidate level to discover the key was absent; with blob files enabled the SST value is only a blob reference, so its 32KB blocks bought nothing. `account_code_metadata` fell into the default arm for the same reason. Both now match the trie-node and flat-KV CFs: 4KB blocks, 10 bits per key. Write-time option: applies to newly flushed/compacted SSTs, existing SSTs are read as-is. --- crates/storage/backend/rocksdb.rs | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/crates/storage/backend/rocksdb.rs b/crates/storage/backend/rocksdb.rs index 81eeac3249e..dc1c351fe01 100644 --- a/crates/storage/backend/rocksdb.rs +++ b/crates/storage/backend/rocksdb.rs @@ -1,6 +1,6 @@ use crate::api::tables::{ - ACCOUNT_CODES, ACCOUNT_FLATKEYVALUE, ACCOUNT_TRIE_NODES, BLOCK_NUMBERS, BODIES, - CANONICAL_BLOCK_HASHES, FULLSYNC_HEADERS, HEADERS, RECEIPTS_V2, STORAGE_FLATKEYVALUE, + ACCOUNT_CODE_METADATA, ACCOUNT_CODES, ACCOUNT_FLATKEYVALUE, ACCOUNT_TRIE_NODES, BLOCK_NUMBERS, + BODIES, CANONICAL_BLOCK_HASHES, FULLSYNC_HEADERS, HEADERS, RECEIPTS_V2, STORAGE_FLATKEYVALUE, STORAGE_TRIE_NODES, TRANSACTION_LOCATIONS, }; use crate::api::{ @@ -208,18 +208,28 @@ impl RocksDBBackend { configure_block_cache(&mut block_opts); cf_opts.set_block_based_table_factory(&block_opts); } - ACCOUNT_CODES => { + ACCOUNT_CODES | ACCOUNT_CODE_METADATA => { cf_opts.set_write_buffer_size(128 * 1024 * 1024); // 128MB cf_opts.set_max_write_buffer_number(3); cf_opts.set_target_file_size_base(256 * 1024 * 1024); // 256MB - cf_opts.set_enable_blob_files(true); - // Small bytecodes should go inline (mainly for delegation indicators) - cf_opts.set_min_blob_size(32); - cf_opts.set_blob_compression_type(rocksdb::DBCompressionType::Lz4); + if cf_name == ACCOUNT_CODES { + cf_opts.set_enable_blob_files(true); + // Small bytecodes should go inline (mainly for delegation indicators) + cf_opts.set_min_blob_size(32); + cf_opts.set_blob_compression_type(rocksdb::DBCompressionType::Lz4); + } let mut block_opts = BlockBasedOptions::default(); - block_opts.set_block_size(32 * 1024); // 32KB + // Exact-key point lookups on the execution read path, same shape as the + // trie-node and flat-KV CFs above: EXT*/CALL* resolve a code hash to its + // bytecode or length. A 4KB (page-sized) block keeps per-get read + // amplification down — with blob files the SST value is just a blob + // reference, so a large block buys nothing — and the filter prunes the + // levels that cannot hold the hash instead of reading a data block per + // level to find out. + block_opts.set_block_size(4 * 1024); // 4KB + block_opts.set_bloom_filter(10.0, false); // 10 bits per key configure_block_cache(&mut block_opts); cf_opts.set_block_based_table_factory(&block_opts); } From 7ed03d98630c9482e48a4f23009374771f87742e Mon Sep 17 00:00:00 2001 From: Edgar Date: Mon, 3 Aug 2026 17:21:25 +0200 Subject: [PATCH 3/8] perf(l1): answer EXTCODESIZE from the code-length table `get_code_metadata` loaded the whole bytecode to return `.len()`, so a size query materialized up to 24KB out of the blob store; the size-only `ACCOUNT_CODE_METADATA` table was plumbed through every layer but reached only from a mempool check. Read it instead, falling back to code already loaded for another reason. Witness generation stays complete: a metadata read observes the bytecode, so the logger records the code hash and the witness still carries the code, which is what stateless validation recomputes the length from. --- crates/vm/backends/levm/db.rs | 9 +++++++++ crates/vm/levm/src/db/gen_db.rs | 27 ++++++++------------------- 2 files changed, 17 insertions(+), 19 deletions(-) diff --git a/crates/vm/backends/levm/db.rs b/crates/vm/backends/levm/db.rs index 127cfb39099..73ce1bb1e1f 100644 --- a/crates/vm/backends/levm/db.rs +++ b/crates/vm/backends/levm/db.rs @@ -80,6 +80,15 @@ impl LevmDatabase for DatabaseLogger { } fn get_code_metadata(&self, code_hash: CoreH256) -> Result { + // A size-only read still observes the bytecode, so the witness must carry it: + // EIP-8025 stateless validation recomputes the length from the code itself, and + // that is also how `ExecutionWitness::get_code_metadata` answers. + if code_hash != *EMPTY_KECCAK_HASH { + self.code_accessed + .lock() + .map_err(|_| DatabaseError::Custom("Could not lock mutex".to_string()))? + .push(code_hash); + } self.store.get_code_metadata(code_hash) } } diff --git a/crates/vm/levm/src/db/gen_db.rs b/crates/vm/levm/src/db/gen_db.rs index dac556b0880..1bbc04cd7a3 100644 --- a/crates/vm/levm/src/db/gen_db.rs +++ b/crates/vm/levm/src/db/gen_db.rs @@ -492,29 +492,18 @@ impl GeneralizedDatabase { match self.code_metadata.entry(code_hash) { Entry::Occupied(entry) => Ok(entry.into_mut()), Entry::Vacant(entry) => { - // First ensure code is loaded into cache by calling get_code - // This handles witness fallbacks and other code loading logic correctly + // Answer from the size-only store lookup rather than the bytecode: a + // contract's code is up to 24KB and lives out-of-line, so materializing it + // to return a length is the dominant cost of EXTCODESIZE. Code already + // loaded for another reason answers for free. #[expect(clippy::as_conversions, reason = "same sized types (on 64bit)")] - let code_length = { - // Note: `self.get_code(code_hash)` has been inlined due to mutability borrow issues. - // To avoid this inlinement, self.get_code has to be moved into `self.codes` so that it's called - // like this: `self.codes.get(code_hash)`. - let code = match self.codes.entry(code_hash) { - Entry::Occupied(entry) => entry.into_mut(), - Entry::Vacant(entry) => { - entry.insert(self.store.get_account_code(code_hash)?) - } - }; - - code.len() as u64 - }; - - let metadata = CodeMetadata { - length: code_length, + let length = match self.codes.get(&code_hash) { + Some(code) => code.len() as u64, + None => self.store.get_code_metadata(code_hash)?.length, }; // Insert into cache and return reference - Ok(entry.insert(metadata)) + Ok(entry.insert(CodeMetadata { length })) } } } From a01c66792b3acac471a27a3616e47f96feef3b24 Mon Sep 17 00:00:00 2001 From: Edgar Date: Mon, 3 Aug 2026 23:52:05 +0200 Subject: [PATCH 4/8] perf(l1): keep the panic path out of the JUMPDEST scan Indexing the bitmap put a bounds-check panic path in the scan loop, which cost more than the scan itself: 24KB of jumpdest-free initcode took 55.4 us to analyse against 11.2 us for the offset-list version it replaced, and benchmarkoor's test_jumpdest_analysis[00] lost 69-78% throughput. Accumulate a byte's bits in a register and store it once the scan leaves that byte; the monotonically increasing index makes the single store safe. Against the offset-list version, per 24KB of code: dense 49.8 -> 17.0 us, jumpdest-free 11.1 -> 11.8 us, 1-in-32 12.5 -> 12.0 us, PUSH-heavy unchanged. --- crates/common/types/account.rs | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/crates/common/types/account.rs b/crates/common/types/account.rs index e971e8450ef..fddf0db3f3f 100644 --- a/crates/common/types/account.rs +++ b/crates/common/types/account.rs @@ -95,16 +95,31 @@ impl Code { /// Builds the [`Code::jumpdests`] bitmap: one pass over the bytecode, setting the /// bit for every `JUMPDEST` while skipping `PUSH` immediates. + /// + /// The bits of a byte are accumulated in a register and written once the scan leaves + /// that byte, which the monotonic `i` makes safe. Reading the bitmap back inside the + /// loop instead would turn each `JUMPDEST` into a read-modify-write, and indexing it + /// would put a bounds-check panic path in the loop; either costs more than the scan + /// itself for bytecode with few jump destinations. pub fn compute_jumpdests(code: &[u8]) -> Arc<[u8]> { let mut bitmap = vec![0u8; code.len().div_ceil(8)]; let mut any = false; + let mut current_byte = usize::MAX; + let mut bits = 0u8; let mut i = 0; while i < code.len() { // TODO: we don't use the constants from the vm module to avoid a circular dependency match code[i] { // OP_JUMPDEST 0x5B => { - bitmap[i / 8] |= 1 << (i % 8); + if i / 8 != current_byte { + if let Some(byte) = bitmap.get_mut(current_byte) { + *byte = bits; + } + current_byte = i / 8; + bits = 0; + } + bits |= 1 << (i % 8); any = true; } // OP_PUSH1..32 @@ -116,6 +131,9 @@ impl Code { } i += 1; } + if let Some(byte) = bitmap.get_mut(current_byte) { + *byte = bits; + } // Share the single empty bitmap for jumpless bytecode (very common: EOAs, // tiny contracts) so we don't allocate for an all-zero map; `is_valid_jumpdest` // reads a missing byte as "no jump destination". From 8b350e4dcd319d044ff741066268da9ffde140bd Mon Sep 17 00:00:00 2001 From: Edgar Date: Tue, 4 Aug 2026 09:27:54 +0200 Subject: [PATCH 5/8] docs: changelog entry for cold contract-code access --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f8f7d6d28f..7a3b6ccf59e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,10 @@ ## Perf +### 2026-08-04 + +- Cut the cost of a cold contract-code access: store jump destinations as a 1-bit-per-byte bitmap instead of a persisted RLP list of `u32` offsets, count the bytecode in the code cache's byte budget, answer `EXTCODESIZE` from the code-length table instead of materializing the bytecode, and give the account-code column families a bloom filter and 4KB data blocks. `COLD_ACCOUNT_CODE_ACCESS` drops from 7736 to 4652 gas in the EIP-8038 repricing fit, and `COLD_ACCOUNT_CODE_WRITE` from 10415 to 6355 [#7095](https://github.com/lambdaclass/ethrex/pull/7095) + ### 2026-07-22 - Unify full-sync batch import onto the per-block execution pipeline, validating every block's state root and reusing the pipeline's BAL-driven parallel execution instead of the bespoke "execute all, apply once" batch path [#7008](https://github.com/lambdaclass/ethrex/pull/7008) From 6dd6ca8571f529f7cf42c2b1fefe5d4f654b73da Mon Sep 17 00:00:00 2001 From: Edgar Date: Tue, 4 Aug 2026 14:22:03 +0200 Subject: [PATCH 6/8] fix(l1): do not spawn the code-metadata backfill off-runtime EXTCODESIZE now resolves its length through Store::get_code_metadata, whose miss path spawned the metadata backfill with tokio::task::spawn. Execution runs on rayon workers, which are not in a runtime, and that spawn panics there, so a hash with no metadata row would abort the node on the first EXTCODESIZE against it. No migration backfills that column family, so any database written before it existed is in exactly that state. Spawn only when a runtime handle is reachable. Skipping the backfill costs one code read the next time the hash is asked for. --- crates/storage/store.rs | 71 ++++++++++++++++++++++++++++++++--------- 1 file changed, 56 insertions(+), 15 deletions(-) diff --git a/crates/storage/store.rs b/crates/storage/store.rs index 5e40105d48d..8f88e6a1e58 100644 --- a/crates/storage/store.rs +++ b/crates/storage/store.rs @@ -1015,21 +1015,29 @@ impl Store { length: code.len() as u64, }; - // Write metadata for future use (async, fire and forget) - let metadata_buf = metadata.length.to_be_bytes().to_vec(); - let hash_key = code_hash.0.to_vec(); - let backend = self.backend.clone(); - tokio::task::spawn(async move { - if let Err(e) = async { - let mut tx = backend.begin_write()?; - tx.put(ACCOUNT_CODE_METADATA, &hash_key, &metadata_buf)?; - tx.commit() - } - .await - { - tracing::warn!("Failed to write code metadata during auto-migration: {}", e); - } - }); + // Backfill the row for future reads, fire and forget. + // + // Only when a Tokio runtime is reachable: this read is on the execution + // path (`EXTCODESIZE`), which runs on rayon workers, and + // `tokio::task::spawn` panics outside a runtime. Skipping the backfill + // costs one code read the next time that hash is asked for; panicking + // would take the node down. + if let Ok(handle) = tokio::runtime::Handle::try_current() { + let metadata_buf = metadata.length.to_be_bytes().to_vec(); + let hash_key = code_hash.0.to_vec(); + let backend = self.backend.clone(); + handle.spawn(async move { + if let Err(e) = async { + let mut tx = backend.begin_write()?; + tx.put(ACCOUNT_CODE_METADATA, &hash_key, &metadata_buf)?; + tx.commit() + } + .await + { + tracing::warn!("Failed to write code metadata during backfill: {}", e); + } + }); + } metadata }; @@ -5103,6 +5111,39 @@ mod account_code_tests { const JUMPDEST: u8 = 0x5b; const PUSH1: u8 = 0x60; + /// `EXTCODESIZE` resolves a length through [`Store::get_code_metadata`], and + /// execution runs on rayon workers, which are not inside a Tokio runtime. A hash + /// with no metadata row SHALL still answer from the bytecode there rather than + /// panicking on the backfill spawn. This test is deliberately NOT `#[tokio::test]`: + /// the absence of a runtime is the condition under test. + #[test] + fn metadata_falls_back_to_the_bytecode_without_a_tokio_runtime() { + use crate::backend::in_memory::InMemoryBackend; + + let backend: Arc = Arc::new(InMemoryBackend::open().unwrap()); + let dir = tempfile::tempdir().unwrap(); + let store = Store::from_backend( + backend.clone(), + dir.path().to_path_buf(), + 1, + DEFAULT_PERSIST_CHANNEL_CAPACITY, + ) + .unwrap(); + + // Code present, metadata row absent: the shape of any database written before + // ACCOUNT_CODE_METADATA existed, which no migration backfills. + let code = jumpdest_dense_code(); + let mut tx = backend.begin_write().unwrap(); + tx.put(ACCOUNT_CODES, code.hash.as_bytes(), &encode_code(&code)) + .unwrap(); + tx.commit().unwrap(); + + let metadata = store + .get_code_metadata(code.hash) + .expect("metadata read must not fail off-runtime"); + assert_eq!(metadata.map(|m| m.length), Some(code.len() as u64)); + } + /// A max-size contract that is almost entirely JUMPDESTs, the shape that makes the /// jump-destination representation matter: as a list of offsets it is ~4x the size /// of the bytecode it describes. From 36724211a1eeb9271f688c4dae1cf3088d6fd82d Mon Sep 17 00:00:00 2001 From: Edgar Date: Tue, 4 Aug 2026 14:34:11 +0200 Subject: [PATCH 7/8] fix(l1): address review on cold contract-code access Witness: code_accessed records one entry per read, and a contract read for both its bytecode and its length is recorded twice, so the builder embedded the same bytecode more than once. Dedup before embedding. Code metadata: EXTCODESIZE made this the execution read path, where the cache was an unbounded map behind a single mutex that serialized the parallel executor. Bound it to a derived entry count, and answer from resident code in the caching layer so a length read usually never reaches it. Schema: bump the store version with a no-op migration. The value change needs no rewrite forwards, but an older binary cannot decode a bitmap, and the bump makes it warn instead of failing on the first code read. RocksDB: the 4KB block size was justified by blob indirection, which the metadata CF does not have; keep its 16KB and let the bytecode CF skip its last-level filter, whose lookups are almost always positive. Also make the legacy-decode test build its expectation from the offsets the old format names rather than from the function under test. --- CHANGELOG.md | 2 +- crates/blockchain/blockchain.rs | 47 ++++++++++++------ crates/common/types/account.rs | 18 ++++--- crates/storage/backend/rocksdb.rs | 28 ++++++++--- crates/storage/lib.rs | 2 +- crates/storage/migrations.rs | 15 +++++- crates/storage/store.rs | 58 ++++++++++++++++------ crates/vm/levm/src/db/mod.rs | 13 +++-- test/tests/common/jumpdest_bitmap_tests.rs | 2 +- 9 files changed, 132 insertions(+), 53 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b1ff6754556..c0ceda54079 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,7 @@ ### 2026-08-04 -- Cut the cost of a cold contract-code access: store jump destinations as a 1-bit-per-byte bitmap instead of a persisted RLP list of `u32` offsets, count the bytecode in the code cache's byte budget, answer `EXTCODESIZE` from the code-length table instead of materializing the bytecode, and give the account-code column families a bloom filter and 4KB data blocks. `COLD_ACCOUNT_CODE_ACCESS` drops from 7736 to 4652 gas in the EIP-8038 repricing fit, and `COLD_ACCOUNT_CODE_WRITE` from 10415 to 6355 [#7095](https://github.com/lambdaclass/ethrex/pull/7095) +- Cut the cost of a cold contract-code access: store jump destinations as a 1-bit-per-byte bitmap instead of a persisted RLP list of `u32` offsets, count the bytecode in the code cache's byte budget, answer `EXTCODESIZE` from the code-length table instead of materializing the bytecode, and give the account-code column families a bloom filter (4KB data blocks on the blob-backed bytecode CF). Raises the code cache's byte budget from an effective 64 MiB of jump tables to 256 MiB of bytecode, and bumps the store schema version so an older binary warns rather than failing on the new value format. `COLD_ACCOUNT_CODE_ACCESS` drops from 7736 to 4652 gas in the EIP-8038 repricing fit, and `COLD_ACCOUNT_CODE_WRITE` from 10415 to 6355 [#7095](https://github.com/lambdaclass/ethrex/pull/7095) ### 2026-07-22 diff --git a/crates/blockchain/blockchain.rs b/crates/blockchain/blockchain.rs index 67bfd64e9df..9262365692f 100644 --- a/crates/blockchain/blockchain.rs +++ b/crates/blockchain/blockchain.rs @@ -1779,15 +1779,23 @@ impl Blockchain { } } - // Store all the accessed evm bytecodes - for code_hash in logger - .code_accessed - .lock() - .map_err(|_e| { + // Store all the accessed evm bytecodes. `code_accessed` records one entry + // per read, and a contract read for both its bytecode and its length is + // recorded twice, so dedup before embedding: a repeated hash would put the + // same (up to 24KB) bytecode in the witness more than once. + let accessed_codes: Vec = { + let accessed = logger.code_accessed.lock().map_err(|_e| { ChainError::WitnessGeneration("Failed to gather used bytecodes".to_string()) - })? - .iter() - { + })?; + let mut seen = + FxHashSet::with_capacity_and_hasher(accessed.len(), Default::default()); + accessed + .iter() + .copied() + .filter(|h| seen.insert(*h)) + .collect() + }; + for code_hash in &accessed_codes { let code = self .storage .get_account_code(*code_hash) @@ -2041,15 +2049,22 @@ impl Blockchain { } } - // Store all the accessed evm bytecodes - for code_hash in logger - .code_accessed - .lock() - .map_err(|_e| { + // Store all the accessed evm bytecodes. `code_accessed` records one entry + // per read, and a contract read for both its bytecode and its length is + // recorded twice, so dedup before embedding: a repeated hash would put the + // same (up to 24KB) bytecode in the witness more than once. + let accessed_codes: Vec = { + let accessed = logger.code_accessed.lock().map_err(|_e| { ChainError::WitnessGeneration("Failed to gather used bytecodes".to_string()) - })? - .iter() - { + })?; + let mut seen = FxHashSet::with_capacity_and_hasher(accessed.len(), Default::default()); + accessed + .iter() + .copied() + .filter(|h| seen.insert(*h)) + .collect() + }; + for code_hash in &accessed_codes { let code = self .storage .get_account_code(*code_hash) diff --git a/crates/common/types/account.rs b/crates/common/types/account.rs index fddf0db3f3f..97dc74e081f 100644 --- a/crates/common/types/account.rs +++ b/crates/common/types/account.rs @@ -18,7 +18,7 @@ use ethrex_rlp::{ use super::GenesisAccount; use crate::constants::{EMPTY_KECCAK_HASH, EMPTY_TRIE_HASH}; -/// Shared empty jump-target table. `Code::default()` and any bytecode without a +/// Shared empty jumpdest bitmap. `Code::default()` and any bytecode without a /// `JUMPDEST` clone this (a refcount bump) instead of allocating a fresh empty /// `Arc` header each time. This matters because the per-tx `Code::default()` /// placeholder and every EOA / empty-code load would otherwise each allocate. @@ -43,9 +43,13 @@ pub struct Code { /// The real bytecode length, needed for some opcodes, `bytecode` is padded with 33 STOPs to avoid checked adds on hot loop. bytecode_len: usize, /// One bit per bytecode byte, set when that offset holds a `JUMPDEST` that is not - /// part of a `PUSH` immediate. A bitmap is `len/8` bytes regardless of how dense + /// part of a `PUSH` immediate. Costs `ceil(len / 8)` bytes regardless of how dense /// the jump destinations are, and validating a jump is a bit test rather than a /// search. + /// + /// Bytecode with no jump destination stores a zero-length bitmap ([`EMPTY_JUMPDESTS`]) + /// rather than an all-zero one, so this is not always `ceil(len / 8)` bytes long; + /// [`Code::is_valid_jumpdest`] reads a missing byte as "no jump destination". // // `Arc<[u8]>` so cloning `Code` (hot: every message-call resolves and clones // the callee's code) is a refcount bump instead of deep-copying the bitmap. @@ -99,8 +103,8 @@ impl Code { /// The bits of a byte are accumulated in a register and written once the scan leaves /// that byte, which the monotonic `i` makes safe. Reading the bitmap back inside the /// loop instead would turn each `JUMPDEST` into a read-modify-write, and indexing it - /// would put a bounds-check panic path in the loop; either costs more than the scan - /// itself for bytecode with few jump destinations. + /// would put a bounds-check panic path in the loop body, which inhibits optimization + /// of every iteration rather than only the ones that find a destination. pub fn compute_jumpdests(code: &[u8]) -> Arc<[u8]> { let mut bitmap = vec![0u8; code.len().div_ceil(8)]; let mut any = false; @@ -190,9 +194,9 @@ impl Code { /// Estimates the size of the Code struct in bytes /// (including stack size and heap allocation). /// - /// Note: This is an estimation and may not be exact. Shared allocations (the - /// empty bitmap, a `Bytes` slice of a larger buffer) are counted in full, so the - /// estimate is an upper bound. + /// Note: an estimate. It ignores allocator overhead and the `Arc`/`Bytes` control + /// blocks, so it slightly under-counts, and a shared allocation is attributed in + /// full to every entry holding it. /// /// # Returns /// diff --git a/crates/storage/backend/rocksdb.rs b/crates/storage/backend/rocksdb.rs index dc1c351fe01..8dc69201200 100644 --- a/crates/storage/backend/rocksdb.rs +++ b/crates/storage/backend/rocksdb.rs @@ -221,15 +221,27 @@ impl RocksDBBackend { } let mut block_opts = BlockBasedOptions::default(); - // Exact-key point lookups on the execution read path, same shape as the - // trie-node and flat-KV CFs above: EXT*/CALL* resolve a code hash to its - // bytecode or length. A 4KB (page-sized) block keeps per-get read - // amplification down — with blob files the SST value is just a blob - // reference, so a large block buys nothing — and the filter prunes the - // levels that cannot hold the hash instead of reading a data block per - // level to find out. - block_opts.set_block_size(4 * 1024); // 4KB + // Both CFs answer exact-key point lookups on the execution read path: + // EXT*/CALL* resolve a code hash to its bytecode or its length. The + // filter is what pays here, pruning the levels that cannot hold the + // hash instead of reading a data block per level to find out. block_opts.set_bloom_filter(10.0, false); // 10 bits per key + if cf_name == ACCOUNT_CODES { + // With blob files the SST value is only a blob reference, so a + // large block buys nothing; page-sized keeps per-get read + // amplification down. + block_opts.set_block_size(4 * 1024); // 4KB + // Lookups here are almost always positive, since the hash comes + // from an account that references it. Dropping the last level's + // filter is most of the filter memory for no hit-rate loss. + cf_opts.set_optimize_filters_for_hits(true); + } else { + // Metadata rows are 32-byte key + 8-byte length with no blob + // indirection, so ~100 of them share a 4KB block and shrinking + // the block only multiplies index entries. Keep the 16KB this CF + // used before it had an arm of its own. + block_opts.set_block_size(16 * 1024); // 16KB + } configure_block_cache(&mut block_opts); cf_opts.set_block_based_table_factory(&block_opts); } diff --git a/crates/storage/lib.rs b/crates/storage/lib.rs index b6a23e2985a..7f00b2901a9 100644 --- a/crates/storage/lib.rs +++ b/crates/storage/lib.rs @@ -88,7 +88,7 @@ pub use store::{ /// When bumping this version, add a corresponding migration function to /// `migrations::MIGRATIONS`. The migration framework will automatically /// upgrade existing databases instead of requiring a full resync. -pub const STORE_SCHEMA_VERSION: u64 = 3; +pub const STORE_SCHEMA_VERSION: u64 = 4; /// Name of the file storing the metadata about the database. /// diff --git a/crates/storage/migrations.rs b/crates/storage/migrations.rs index f2946bd6a0f..8f27663bae5 100644 --- a/crates/storage/migrations.rs +++ b/crates/storage/migrations.rs @@ -30,7 +30,7 @@ pub type MigrationFn = fn(backend: &dyn StorageBackend) -> Result<(), StoreError /// /// **Invariant**: `MIGRATIONS.len() == (STORE_SCHEMA_VERSION - 1) as usize` /// (empty when `STORE_SCHEMA_VERSION == 1`, one entry when it's 2, etc.) -pub const MIGRATIONS: &[MigrationFn] = &[migrate_1_to_2, migrate_2_to_3]; +pub const MIGRATIONS: &[MigrationFn] = &[migrate_1_to_2, migrate_2_to_3, migrate_3_to_4]; // Compile-time check: the number of migration functions must match the number // of version gaps (i.e. STORE_SCHEMA_VERSION - 1). @@ -44,6 +44,19 @@ fn migration_for_version(version: u64) -> MigrationFn { MIGRATIONS[(version - 1) as usize] } +/// v3 → v4: no data change. +/// +/// `ACCOUNT_CODES` values carry their JUMPDEST positions as a bitmap rather than an RLP +/// list of `u32` offsets. Both forms are readable, so a v3 database needs no rewriting +/// and this migration only moves the version marker. +/// +/// The bump exists for the other direction: a v3 binary cannot decode a bitmap, and +/// `run_pending_migrations` warns that the database is ahead of the binary instead of +/// letting it fail on the first code read. +fn migrate_3_to_4(_backend: &dyn StorageBackend) -> Result<(), StoreError> { + Ok(()) +} + /// Minimum interval between migration progress log lines. const PROGRESS_LOG_INTERVAL: Duration = Duration::from_secs(10); diff --git a/crates/storage/store.rs b/crates/storage/store.rs index 8f88e6a1e58..9d84134ff7c 100644 --- a/crates/storage/store.rs +++ b/crates/storage/store.rs @@ -49,6 +49,7 @@ use std::{ collections::{BTreeMap, HashMap, HashSet, hash_map::Entry}, fmt::Debug, io::Write, + num::NonZeroUsize, path::{Path, PathBuf}, sync::{ Arc, Condvar, Mutex, RwLock, @@ -125,11 +126,16 @@ enum FKVGeneratorControlMessage { /// Bytecode is a small fraction of the working set next to trie nodes and flat /// key-values (which the RocksDB block cache serves), but a cold code read costs a /// blob-file fetch, so caching contracts pays for itself: 256 MiB holds ~10k max-size -/// (24 KiB) contracts, or ~100k of typical size. Before [`Code::size`] counted the -/// bytecode, this budget bounded only the jump-destination tables, and the cache could -/// hold multiple GiB of code without evicting. +/// (24 KiB) contracts, or ~100k of typical size. const CODE_CACHE_MAX_SIZE: u64 = 256 * 1024 * 1024; +/// Entry bound for [`Store::code_metadata_cache`], derived from a 16 MiB ceiling at the +/// ~64 B an `LruCache` entry costs (32 B key + 8 B value + list/table overhead). Sized +/// against the code cache it shadows: at 256 MiB that holds ~10k max-size or ~100k +/// typical contracts, so this keeps a length for every code that could be resident and +/// then some, while bounding what an `EXTCODESIZE` sweep over unique contracts can pin. +const CODE_METADATA_CACHE_MAX_ENTRIES: usize = (16 * 1024 * 1024) / 64; + /// Key used to persist the `flushed_upto` block number in `MISC_VALUES`. const FLUSHED_UPTO_KEY: &[u8] = b"bodies_flushed_upto"; @@ -229,8 +235,11 @@ pub struct Store { account_code_cache: Arc>, /// Cache for code metadata (code length), keyed by the bytecode hash. - /// Uses FxHashMap for efficient lookups, much smaller than code cache. - code_metadata_cache: Arc>>, + /// + /// Bounded: `EXTCODESIZE` reads this on the execution path, so an unbounded map + /// would grow by one entry per distinct contract ever asked for a length and never + /// give the memory back. See [`CODE_METADATA_CACHE_MAX_ENTRIES`]. + code_metadata_cache: Arc>>, /// Serializes concurrent `forkchoice_update` callers so that the cache /// update and the DB write transaction remain mutually ordered. @@ -945,7 +954,7 @@ impl Store { /// Check if account code exists by its hash, without constructing the full `Code` struct. /// More efficient than `get_account_code` for existence checks since it skips - /// RLP decoding and `Code` struct construction (no `jump_targets` deserialization). + /// RLP decoding and `Code` struct construction (no jumpdest-bitmap decoding). /// Note: The underlying `get()` still reads the value from RocksDB (including blob files). pub fn code_exists(&self, code_hash: H256) -> Result { // Code introduced by a not-yet-flushed block lives only in the buffer; check @@ -1046,7 +1055,7 @@ impl Store { self.code_metadata_cache .lock() .map_err(|_| StoreError::LockError)? - .insert(code_hash, metadata); + .put(code_hash, metadata); Ok(Some(metadata)) } @@ -1990,7 +1999,10 @@ impl Store { pending_trie_roots: Arc::new(PendingTrieRoots::default()), last_computed_flatkeyvalue: Arc::new(RwLock::new(last_written)), account_code_cache: Arc::new(Mutex::new(CodeCache::default())), - code_metadata_cache: Arc::new(Mutex::new(rustc_hash::FxHashMap::default())), + code_metadata_cache: Arc::new(Mutex::new(LruCache::with_hasher( + NonZeroUsize::new(CODE_METADATA_CACHE_MAX_ENTRIES).unwrap_or(NonZeroUsize::MIN), + FxBuildHasher, + ))), fcu_lock: Arc::new(tokio::sync::Mutex::new(())), safe_commit_root, journal_pruning_paused: Arc::new(std::sync::atomic::AtomicBool::new(false)), @@ -5188,24 +5200,40 @@ mod account_code_tests { } /// Values written before the bitmap SHALL still decode, with the bitmap rebuilt from - /// the bytecode, so an existing database needs no migration. + /// the bytecode, so an existing database needs no rewriting. + /// + /// The expectation is built from the offsets the legacy value itself names, not from + /// `Code::compute_jumpdests`, so this pins the rebuilt bitmap against the old format + /// rather than against the implementation that produces it. #[test] fn legacy_offset_list_values_decode_to_the_same_bitmap() { - for bytecode in [ - vec![0x00, JUMPDEST, 0x00, JUMPDEST], - vec![PUSH1, JUMPDEST, JUMPDEST], - vec![0x00; 32], + for (bytecode, offsets) in [ + (vec![0x00, JUMPDEST, 0x00, JUMPDEST], vec![1u32, 3]), + // The byte after PUSH1 is its immediate, so only offset 2 is a destination. + (vec![PUSH1, JUMPDEST, JUMPDEST], vec![2u32]), + (vec![0x00; 32], vec![]), ] { - let code = Code::from_bytecode_unchecked(bytecode.into(), H256::zero()); + let code = Code::from_bytecode_unchecked(bytecode.clone().into(), H256::zero()); let legacy = encode_code_legacy(&code); + let mut expected = vec![0u8; bytecode.len().div_ceil(8)]; + for offset in &offsets { + let index = usize::try_from(*offset).expect("offset fits usize"); + expected[index / 8] |= 1 << (index % 8); + } + // A jumpless contract stores no bitmap at all rather than an all-zero one. + if offsets.is_empty() { + expected.clear(); + } + let (decoded_bytecode, jumpdests) = decode_bytes(&legacy).unwrap(); assert_eq!(decoded_bytecode, code.code()); assert_eq!( decode_jumpdests(decoded_bytecode, jumpdests) .unwrap() .as_ref(), - code.jumpdests() + expected.as_slice(), + "rebuilt bitmap disagrees with the legacy offsets for {bytecode:?}" ); } } diff --git a/crates/vm/levm/src/db/mod.rs b/crates/vm/levm/src/db/mod.rs index 470ea856e65..6ba4b3e2e71 100644 --- a/crates/vm/levm/src/db/mod.rs +++ b/crates/vm/levm/src/db/mod.rs @@ -236,9 +236,16 @@ impl Database for CachingDatabase { } fn get_code_metadata(&self, code_hash: H256) -> Result { - // Delegate directly to the underlying database. - // The underlying Store already has its own code_metadata_cache, - // so we don't need to duplicate caching here. + // Answer from resident code when there is any. The BAL warmer loads the block's + // bytecode into this cache, so a length read usually has the answer here under a + // shared read lock. Falling straight through would instead take the store's + // single mutex-guarded metadata cache, serializing every `EXTCODESIZE` across + // the parallel executor's threads. + if let Some(code) = self.read_code()?.get(&code_hash) { + return Ok(CodeMetadata { + length: u64::try_from(code.len()).unwrap_or(u64::MAX), + }); + } self.inner.get_code_metadata(code_hash) } diff --git a/test/tests/common/jumpdest_bitmap_tests.rs b/test/tests/common/jumpdest_bitmap_tests.rs index 908d38b1fff..f17d89de5a9 100644 --- a/test/tests/common/jumpdest_bitmap_tests.rs +++ b/test/tests/common/jumpdest_bitmap_tests.rs @@ -56,7 +56,7 @@ fn offsets_past_the_bytecode_are_not_valid() { } } -/// Jumpless bytecode SHALL NOT allocate a bitmap, and empty code SHALL have none. +/// Jumpless bytecode SHALL have an empty bitmap, as SHALL empty code. #[test] fn jumpless_bytecode_has_an_empty_bitmap() { assert!(code_of(vec![STOP; 64]).jumpdests().is_empty()); From 9a4c1fd841fb565172f409d2772d8fbe4ab79241 Mon Sep 17 00:00:00 2001 From: Edgar Date: Tue, 4 Aug 2026 15:50:52 +0200 Subject: [PATCH 8/8] fix(l1): keep derived jump destinations out of Code's wire format Code's serde form carried the jump destinations, so changing how they are represented changed the wire format of everything embedding a Code. That includes AccountUpdate, which the L2 rollup store persists with bincode and the committer reads back to re-apply state, so rows written by an older binary would not decode. They are a pure function of the bytecode, so drop them from the format and recompute on deserialize. The format is now hash plus bytecode, which cannot be broken again by a representation change, and the payload loses a byte per eight bytes of code. --- crates/common/types/account.rs | 16 +++++++------ test/tests/common/code_serde_tests.rs | 33 +++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 7 deletions(-) diff --git a/crates/common/types/account.rs b/crates/common/types/account.rs index 97dc74e081f..3b287c2a750 100644 --- a/crates/common/types/account.rs +++ b/crates/common/types/account.rs @@ -215,11 +215,14 @@ impl Code { /// `Code` is padded with [`BYTECODE_PADDING`] trailing STOPs) sound regardless of /// where the bytes came from. Deserializing the padded buffer directly would /// otherwise let unpadded input through and cause OOB reads during execution. +/// The jump destinations are deliberately absent: they are a pure function of the +/// bytecode, so carrying them would put a derived value in the wire format of everything +/// that embeds a `Code` (notably `AccountUpdate`, which the L2 rollup store persists with +/// bincode) and couple that format to how they happen to be represented. #[derive(Serialize, Deserialize)] struct CodeSerde { hash: H256, code: Bytes, - jumpdests: Arc<[u8]>, } impl Serialize for Code { @@ -227,7 +230,6 @@ impl Serialize for Code { CodeSerde { hash: self.hash, code: self.code_bytes(), - jumpdests: self.jumpdests.clone(), } .serialize(serializer) } @@ -235,12 +237,12 @@ impl Serialize for Code { impl<'de> Deserialize<'de> for Code { fn deserialize>(deserializer: D) -> Result { - let CodeSerde { + let CodeSerde { hash, code } = CodeSerde::deserialize(deserializer)?; + Ok(Self::from_parts_unchecked( hash, - code, - jumpdests, - } = CodeSerde::deserialize(deserializer)?; - Ok(Self::from_parts_unchecked(hash, &code, jumpdests)) + &code, + Self::compute_jumpdests(&code), + )) } } diff --git a/test/tests/common/code_serde_tests.rs b/test/tests/common/code_serde_tests.rs index 172f2d7e2b6..5579460f95f 100644 --- a/test/tests/common/code_serde_tests.rs +++ b/test/tests/common/code_serde_tests.rs @@ -42,3 +42,36 @@ fn default_code_is_padded() { assert!(code.is_empty()); assert_eq!(code.dispatch_buf().len(), BYTECODE_PADDING); } + +/// The wire format SHALL carry only the hash and the bytecode. Jump destinations are a +/// pure function of the bytecode, and every type embedding a `Code` inherits this format +/// (`AccountUpdate`, which the L2 rollup store persists with bincode), so a derived field +/// here would couple that stored format to how jump destinations are represented. +#[test] +fn code_serde_does_not_persist_derived_jumpdests() { + let bytecode = vec![0x5b /* JUMPDEST */; 512]; + let code = Code::from_bytecode(bytecode.into(), &NativeCrypto); + assert!( + !code.jumpdests().is_empty(), + "fixture must have a non-empty bitmap for this to prove anything" + ); + + let json = serde_json::to_value(&code).expect("serialize"); + // `serde_json` orders its map keys, so compare against a sorted expectation. + let fields: Vec<&str> = json + .as_object() + .expect("Code serializes as a struct") + .keys() + .map(String::as_str) + .collect(); + assert_eq!( + fields, + vec!["code", "hash"], + "unexpected field in the Code wire format" + ); + + // Recomputed on the way back in, so the round trip is still lossless. + let restored: Code = serde_json::from_value(json).expect("deserialize"); + assert_eq!(restored.jumpdests(), code.jumpdests()); + assert_eq!(restored, code); +}