Perf/lazy state reader tries

[ongyimeng]

Summary

• Lazily construct StateReader contract and class tries instead of opening both in NewStateReader.
• Keep trie construction on demand for paths that need it, such as storage proofs and state commitment.
• Avoid two unnecessary trie opens for common state-read RPC paths that never access those tries.

Why

NewStateReader previously opened both the contract trie and class trie for every state reader.

RPC methods such as getStorageAt, getClassHashAt, getNonce, and getClass read directly from contract, class, or storage accessors. They do not use ClassTrie() or ContractTrie().

As a result, every state-read request paid for two unused trie root-node reads/resolves.

Benchmarks

I ran local benchmarks that exercise the public RPC handler paths with Pebble DB.

Benchmark	latency	latency Δ	B/op	B/op Δ	allocs/op	allocs/op Δ
NonceLatest	14.05µs → 9.26µs	-34.12%	4.008Ki → 2.945Ki	-26.51%	60 → 34	-43.33%
ClassHashAtLatest	14.86µs → 9.23µs	-37.87%	4.008Ki → 2.945Ki	-26.51%	60 → 34	-43.33%
StorageAtLatest	14.93µs → 10.06µs	-32.63%	4.173Ki → 3.109Ki	-25.49%	64 → 38	-40.62%
NonceNumber	19.89µs → 11.11µs	-44.14%	4.271Ki → 3.198Ki	-25.11%	69 → 43	-37.68%
NonceHash	17.19µs → 11.03µs	-35.86%	4.271Ki → 3.206Ki	-24.93%	69 → 43	-37.68%

[codecov]

Codecov Report

❌ Patch coverage is 63.63636% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.32%. Comparing base (2095502) to head (2ac1ce0).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
core/state/state_reader.go	63.63%	4 Missing and 4 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3780      +/-   ##
==========================================
- Coverage   75.39%   75.32%   -0.07%     
==========================================
  Files         438      438              
  Lines       39463    39528      +65     
==========================================
+ Hits        29753    29775      +22     
- Misses       7637     7681      +44     
+ Partials     2073     2072       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[github-actions]

Claude finished @ongyimeng's task in 1m 37s —— View job

---

Review: PR #3780 — Lazy StateReader tries

• Read PR diff
• Analyze correctness & concurrency
• Check callers of StateReader for concurrent use
• Post review

This is a clean, well-targeted optimization. Deferring ContractTrie()/ClassTrie() construction until they're actually needed correctly removes two unused trie root reads from the hot getNonce / getClassHashAt / getStorageAt / getClass paths, and Commitment() was properly updated to go through the lazy accessors. No correctness bugs found.

Notes (non-blocking)

Nit — thread safety. The lazy init mutates s.classTrie/s.contractTrie without synchronization, so StateReader is no longer immutable-after-construction. I verified all constructors (HeadState, StateAtBlockNumber, StateAtBlockHash, GetReverseStateDiff, genesis) create a fresh reader per request, and StorageProof accesses the tries sequentially — so there's no concurrent sharing today and this is safe. Worth a doc comment ("not safe for concurrent use") or a sync.Once guard to lock in the previous invariant. (left as an inline comment)

Behavioral note — deferred root validation. Previously NewStateReader opened both tries eagerly, so an unresolvable stateRoot surfaced an error at construction. Now that error is deferred to the first ClassTrie()/ContractTrie()/Commitment() call; pure read paths (getNonce, etc.) no longer validate the root. This is the intended trade-off and none of the current callers depend on early validation, so no action needed — just flagging the semantic change.

Test coverage. Codecov flags the new lazy branches and the Commitment() error paths as uncovered (patch 60%). A small test asserting that ClassTrie()/ContractTrie() construct on demand (and return the same cached instance on a second call), plus one exercising the lazy path in Commitment(), would close the gap.

Summary

• Blocker: none
• Important: none
• Nit: thread-safety documentation/guard; add coverage for lazy branches

[github-actions]

Claude finished @rodrodros's task in 1m 39s —— View job

---

Review: PR #3780 — Lazy StateReader tries

• Read PR diff (git diff origin/main..HEAD)
• Check correctness / nil-deref / error handling
• Verify concurrency safety of lazy init across all callers
• Confirm status of reviewer nits

This is a clean, well-scoped optimization. Deferring ContractTrie()/ClassTrie() construction removes two unused trie root reads from the hot getNonce / getClassHashAt / getStorageAt / getClass paths, and Commitment() correctly routes through the lazy accessors. No correctness bugs found — LGTM.

Reviewer nits (from @rodrodros)

• ✅ Line 26 — TODO for the unused error return: addressed in chore(core): nitpics (// todo(rdr): change this function to not return an error).
• ✅ Line 41 — nil init + comment: addressed in the same commit (contractTrie: nil / classTrie: nil with // Initialized lazily on demand).
• ⬜ Line 109 — invert the guard (nit, optional): not yet applied. Currently ClassTrie()/ContractTrie() use if s.classTrie == nil { … }. Early-return reads slightly cleaner:

  func (s *StateReader) ClassTrie() (core.TrieReader, error) {
      if s.classTrie != nil {
          return s.classTrie, nil
      }
      classTrie, err := s.db.ClassTrie(&s.initRoot)
      if err != nil {
          return nil, err
      }
      s.classTrie = classTrie
      return s.classTrie, nil
  }

  Purely stylistic — safe to leave as-is.

Notes (non-blocking)

• Thread safety. Lazy init now mutates s.classTrie/s.contractTrie without synchronization, so StateReader is no longer immutable-after-construction. I confirmed this is safe today: every constructor (HeadState, StateAtBlockNumber, StateAtBlockHash, GetReverseStateDiff, genesis) builds a fresh reader per request, and the only multi-trie consumer — getStorageProof (rpc/v{8,9,10}/storage.go) — calls ClassTrie()/ContractTrie() sequentially on a single goroutine. A one-line doc comment ("not safe for concurrent use") would lock in the invariant.
• Deferred root validation. Root resolution errors now surface on first ClassTrie()/ContractTrie()/Commitment() call rather than at construction; pure read paths no longer validate the root eagerly. Intended trade-off, no current caller depends on early validation.
• Test coverage. Codecov flags the new lazy branches and Commitment() error paths (patch 60%). A small test asserting on-demand construction + cached-instance reuse, plus one exercising the lazy path in Commitment(), would close the gap.

Summary

• Blocker: none
• Important: none
• Nit: optional guard inversion (line 109); optional // not safe for concurrent use doc; add coverage for lazy branches