refactor(l1): reuse get_account_state in Store account getters

[ilitteri]

Motivation

Store::get_code_by_account_address and Store::get_nonce_by_account_address each inlined the full account-lookup sequence (canonical block hash → state trie → hash_address_fixed → trie.get → AccountState::decode) that the public Store::get_account_state already implements, and Store::get_account_info_by_hash inlined the get+decode half that get_account_state_from_trie provides.

Three hand-rolled copies of the same walk are a liability: any future change to account lookup (key derivation, decode handling, the missing-trie tri-state) has to be replicated in four places, and a copy that drifts silently diverges in behavior. Deduplicating to the canonical helpers removes 19 net lines of code and leaves the tri-state logic spelled exactly once.

Description

Only the three function bodies in crates/storage/store.rs change; every signature (async-ness included) is byte-identical, so no caller changes.

• get_nonce_by_account_address now delegates to get_account_state(block_number, address) and maps the nonce out.
• get_code_by_account_address now delegates to get_account_state(block_number, address) and, when the account exists, still resolves the code through get_account_code(code_hash) — preserving the account_code_cache fast path and the account-exists-but-code-missing ⇒ Ok(None) (not error) semantics.
• get_account_info_by_hash stays sync: it keeps its own state_trie(block_hash)? call and delegates only to the sync get_account_state_from_trie, mapping the AccountState into AccountInfo (its sync caller get_account_info is untouched).

Invariants preserved and how: the tri-state Ok(None) semantics (missing canonical hash, missing state trie, absent account) come from the same let-else guards now living solely in get_account_state / get_account_state_from_trie, while trie/DB/decode errors still propagate via ? as StoreError; key derivation remains hash_address_fixed because that is what get_account_state_from_trie already uses; get_account_state_by_acc_hash (pre-hashed key, a different helper) is deliberately untouched.

If this change were wrong, one of these would have to be true: get_account_state's lookup sequence differs from the inlined copies (it is verbatim the same five steps), some caller depended on a signature or sync-ness change (workspace clippy compiles all callers unchanged), missing code for an existing account now errors (the get_account_code call is preserved verbatim), or a None case now errors (all three None sources still short-circuit to Ok(None)).

How to test

• cargo fmt — clean (diff is +15/−36 in one file).
• cargo clippy -p ethrex-storage --all-targets -- -D warnings — clean.
• cargo clippy --workspace --all-targets -- -D warnings — clean, proving all RPC/l2 callers compile against the unchanged signatures.
• cargo test -p ethrex-storage — 91 passed, 0 failed (plus doctests).

Checklist

• No Store schema, on-disk format, or RLP change — pure in-memory dedup of function bodies — so STORE_SCHEMA_VERSION (crates/storage/lib.rs) is untouched.

[github-actions]

⚠️ 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.rs → unsync_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]

Lines of code report

Total lines added: 0
Total lines removed: 19
Total lines changed: 19

Detailed view

+--------------------------------+-------+------+
| File                           | Lines | Diff |
+--------------------------------+-------+------+
| ethrex/crates/storage/store.rs | 5088  | -19  |
+--------------------------------+-------+------+

[github-actions]

🤖 Kimi Code Review

This is a clean refactoring PR that eliminates code duplication in account state retrieval. The changes are functionally equivalent and improve maintainability.

crates/storage/store.rs

The refactoring correctly extracts common patterns:

1. Lines 2278-2289: get_account_info_by_address now uses get_account_state_from_trie, removing manual address hashing and RLP decoding duplication.

2. Lines 2319-2326: get_code_by_account_address delegates to get_account_state, eliminating the repeated block_hash → state_trie → address hashing chain.

3. Lines 2330-2338: get_nonce_by_account_address similarly delegates to get_account_state with idiomatic Option::map.

Verdict: Approved. The changes reduce duplication while preserving error handling semantics (? operator propagation) and maintaining the same async/await patterns. The use of Option::map (line 2332) and explicit match (line 2319) are both idiomatic Rust for their respective control flows.

No security or consensus issues introduced—this is purely a code structure improvement.

---

Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt

[github-actions]

🤖 Claude Code Review

This confirms the refactor is a straightforward, behavior-preserving dedup. The delegation matches the original logic exactly (same tri-state Ok(None) short-circuits, same key derivation via hash_address_fixed, same get_account_code fast path preserved).

Review

Correctness: Verified against the current definitions of get_account_state (store.rs:3089) and get_account_state_from_trie (store.rs:3112) — the new bodies are behaviorally identical to what was inlined before:

• get_account_info_by_hash (store.rs:2270-2285) still does its own state_trie(block_hash)? guard, then delegates the get+decode step to get_account_state_from_trie, mapping to AccountInfo. Matches previous behavior.
• get_code_by_account_address (store.rs:2317-2326) delegates to get_account_state, then still routes through get_account_code only when the account exists, preserving the cache fast-path and the "account exists but code missing → Ok(None), not error" semantics.
• get_nonce_by_account_address (store.rs:2328-2337) delegates to get_account_state and maps .nonce out — same as before.

No signature changes, no schema/RLP changes, no new error paths introduced. The tri-state Ok(None) logic (missing canonical hash, missing trie, missing account) is now expressed only in get_account_state/get_account_state_from_trie, eliminating three previously-duplicated copies of that logic — a small but genuine hardening in case that logic ever needs to change.

Performance: No material change — same number of trie lookups/decodes per call.

Security/consensus: Not applicable; this is a pure internal dedup of read-path lookups, no consensus-affecting logic touched.

Style: Idiomatic use of ?, let-else, and .map() over Option. Consistent with existing code in the file (e.g., get_account_info at store.rs:2259 already uses the same match-based delegation pattern).

No issues found — this is a clean, low-risk refactor that does exactly what it claims.

---

Automated review by Claude (Anthropic) · sonnet · custom prompt

[github-actions]

🤖 Codex Code Review

No findings.

The changes in crates/storage/store.rs and crates/storage/store.rs appear behavior-preserving: they reuse the existing get_account_state_from_trie / get_account_state paths, so trie key hashing, AccountState RLP decoding, and None vs error propagation stay consistent with the preexisting implementation at crates/storage/store.rs and crates/storage/store.rs. I do not see consensus, EVM, gas-accounting, or security-sensitive behavior changing here.

Residual risk is limited to coverage: the PR does not add tests around these refactored accessors, and I could not run cargo test in this environment because rustup failed to create temp files under /home/runner/.rustup/tmp on a read-only filesystem.

---

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