Conversation
Adds the L1SLOAD precompile at address 0x10001 for reading L1 storage from L2. The precompile takes 84-byte input (address + storage key + block number) and returns the cached storage value. The precompile is a dummy cache: Raiko populates it with verified values before block re-execution during proving. The cache keys include block number to support lookback up to 256 blocks from the anchor block. Changes: - crates/evm/src/precompiles/l1sload.rs: Cache, API, precompile logic - crates/evm/src/precompiles/mod.rs: taiko_precompiles_map() helper - crates/evm/src/factory.rs: Use taiko_precompiles_map in EVM factory - 11 unit tests covering all paths
Promote trace! to info! and add [jmadibekov] tags for: - Precompile entry, input parsing, range checks - Cache hit/miss, RPC fallback, anchor/l1origin context
- Consolidate ~12 per-call info logs in l1sload_run() into 2-3 targeted logs - Cache hits downgraded to trace level; RPC fallback remains at info - Trim verbose doc comments that restate obvious code - Keep all functional logic and tests intact E2E verified: all L1SLOAD scenarios pass on devnet.
- Remove unused L1SLOAD const bundle (address inlined in mod.rs) - Inline clear_anchor_block_id/clear_l1_origin_block_id into clear_l1_storage - Make 4 constants private (not used outside crate) - Unify duplicated output construction in cache-hit/RPC-fallback branches - Downgrade RPC fallback log from info to debug - Remove redundant clear calls in tests
The lookback range is now measured from L1 origin instead of anchor, matching surge-raiko's simplified trust model. L1 origin is the single reference point for both upper and lower bounds.
|
@claude please review |
There was a problem hiding this comment.
Pull request overview
This PR introduces a Taiko-specific L1SLOAD EVM precompile (RIP-7728-inspired) at address 0x10001, enabling L2 contracts to read L1 storage values from a deterministic in-memory cache with an optional RPC fallback for preflight.
Changes:
- Added a new
l1sloadprecompile implementation with cache/context APIs and unit tests. - Wired the Taiko EVM factory to use a custom precompiles map that includes
0x10001. - Added
serial_test(dev dependency) to ensure tests are isolated due to global state.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/evm/src/precompiles/mod.rs | Adds taiko_precompiles_map() to register the L1SLOAD precompile at 0x10001. |
| crates/evm/src/precompiles/l1sload.rs | Implements the L1SLOAD precompile, global cache/context, RPC fallback tracking, and tests. |
| crates/evm/src/lib.rs | Exposes a new precompiles module from the crate root. |
| crates/evm/src/factory.rs | Switches EVM construction to use the Taiko precompiles map (now including L1SLOAD). |
| crates/evm/Cargo.toml | Adds serial_test as a dev-dependency for serializing tests. |
| Cargo.lock | Locks new dev-dependency transitive packages (e.g., serial_test, extra dashmap version). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@claude please review |
|
Claude finished @smartprogrammer93's task in 2m 0s —— View job Code Review: Add L1SLOAD PrecompileTasks:
Good overall structure — the implementation is clean and well-tested. Below are my findings, roughly ordered by severity. Critical / Correctness1. Mutex held across RPC call (deadlock risk) The standard fix is to clone the fetcher under the lock, drop the guard, then invoke: let maybe_fetcher = {
let guard = L1_RPC_FETCHER.lock().expect("...");
guard.as_ref().map(|f| Arc::clone(f)) // fetcher should be Arc<dyn Fn...>
};
if let Some(fetcher) = maybe_fetcher {
let fetched = fetcher(contract_address, storage_key, requested_block)?;
...
}This also shortens the critical section to near-zero during the RPC call. Fix this → 2. Silent fallback: Recommend requiring it explicitly: let l1_origin_block_id = get_l1_origin_block_id().ok_or_else(|| {
warn!("L1SLOAD: L1 origin block ID not set");
PrecompileError::Other("L1 origin block ID not set".into())
})?;3. Mutex poisoning panics the node Design Concerns4. Global mutable state — threading and parallelism hazards
A cleaner long-term approach is to pass this context through the EVM's 5. 6. Code Quality / Lint7. Missing doc comments on private items — clippy will fail 8. Minor9. Unnecessary buffer copy on output // simplify to:
Ok(PrecompileOutput::new(gas_used, Bytes::from(value.0.to_vec())))10. Missing test coverage
Summary
The implementation is functionally solid for the single-threaded sequential case and the test suite is thorough. The most important things to address before merging are the mutex-held-across-RPC-call deadlock risk (#1), the silent l1_origin fallback (#2), and the clippy failures from missing doc comments (#7). |
…origin, add test, format
|
@claude review |
|
Claude finished @smartprogrammer93's task in 1m 50s —— View job Tasks:
Updated Code Review: Add L1SLOAD PrecompileGood progress since the last review — several issues have been addressed. Here's the current state. Fixed Since Last Review
Remaining: Mutex Held Across RPC Call
The author's position ("the callback is a simple RPC call that never re-acquires this lock") is pragmatically reasonable given the current architecture, but there's a secondary consequence worth noting: since the let maybe_value = {
let guard = L1_RPC_FETCHER.lock().expect("...");
guard.as_ref().map(|f| f(contract_address, storage_key, requested_block))
};
if let Some(fetched) = maybe_value {
let fetched = fetched.map_err(|e| PrecompileError::Other(...))?;
// guard is already dropped here
...
}However if this won't be fixed before merge, it's acceptable given the stated constraints. Fix this → Remaining:
|
| Status | Issue |
|---|---|
| ✅ Fixed | Silent l1_origin fallback |
| ✅ Fixed | Missing doc comments (3 functions) |
| ✅ Fixed | Missing tests for l1origin < anchor and missing l1_origin |
| Mutex held across RPC call (acceptable in practice) | |
factory.rs double-construction |
|
| 💡 Minor | Unnecessary output buffer copy |
| ℹ️ Won't Fix | Mutex poisoning, serial_test workspace dep, global state |
The implementation is in good shape. The two remaining open issues (factory.rs double-construction and mutex scope) are both low-risk and the logic is correct. The code is mergeable with those acknowledged.
What is L1SLOAD?
L1SLOAD (https://github.com/ethereum/RIPs/blob/master/RIPS/rip-7728.md) is a precompile at address
0x10001that lets L2 smart contracts read L1 Ethereum storage. For example, an L2 DeFi protocol can check an L1 oracle price or verify L1 token balances without a bridge or messaging system.An L2 transaction calls the precompile with 84 bytes - an L1 contract address (20 bytes), a storage key (32 bytes), and an L1 block number (32 bytes) - and gets back the 32-byte storage value from that slot at that block.
What this PR does
Adds the L1SLOAD precompile to the EVM at address
0x10001. The precompile is cache-based: it looks up pre-populated values from an in-memory cache. This makes it deterministic and suitable for ZK proving.It has an option of providing an L1 RPC endpoint, more on that below.
Precompile behavior
Input: 84 bytes —
[0..20]address,[20..52]storage key,[52..84]block number (uint256 BE).Output: 32-byte storage value.
Gas: 4000 (2000 fixed + 2000 per-load).
Block range validation
The precompile enforces that the requested L1 block falls within a valid range relative to the L1 origin (the L1 block that triggered this L2 batch):
Rules:
requested_block <= l1origin(can't read future L1 state)l1origin - requested_block <= 256(lookback limit measured from L1 origin)Execution flow
RPC fallback for indirect calls
During preflight (when network access is available), the companion prover (Raiko) sets an RPC callback via
set_l1_rpc_fetcher(). This handles cache misses from indirect L1SLOAD calls — cases where a smart contract internally calls the precompile and the call wasn't detected by static transaction scanning.After execution, Raiko reads
take_l1_rpc_served_calls()to get all calls that were served via RPC, then fetches their Merkle proofs for verification.During proving (no network), the callback is not set — cache misses are hard errors, which is correct since all values should have been pre-verified and cached.
Cache API
Unit tests
25 unit tests in
crates/evm/src/precompiles/l1sload.rsHow to run
Test output
Cross references
eth_getProof: https://eips.ethereum.org/EIPS/eip-1186