refactor: refine msgs_in_tipset cache usage

[hanabi1224]

Summary of changes

Changes introduced in this pull request:

Reference issue to close (if applicable)

Closes

Other information and links

Change checklist

• I have performed a self-review of my own code,
• I have made corresponding changes to the documentation. All new code adheres to the team's documentation standards,
• I have added tests that prove my fix is effective or that my feature works (if possible),
• I have made sure the CHANGELOG is up-to-date. All user-facing changes should be reflected in this document.

Outside contributions

• I have read and agree to the CONTRIBUTING document.
• I have read and agree to the AI Policy document. I understand that failure to comply with the guidelines will lead to rejection of the pull request.

Summary by CodeRabbit

• Refactor

  • Redesigned message handling to use shared references, reducing cloning and improving memory use and performance.
  • Split read/write message APIs for clearer, safer usage patterns.
  • Slimmed RPC/state payloads to reduce per-request memory footprint.
  • Increased per-tipset message cache capacity for better caching and throughput.

• Chores

  • Enabled additional serialization feature to support shared-reference types.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: f6306211-086c-42c7-82e0-e863c4c308db

📥 Commits

Reviewing files that changed from the base of the PR and between d6ffc4d and adbb101.

📒 Files selected for processing (1)

• src/rpc/methods/eth.rs

---

Walkthrough

Split message trait into read-only MessageRead and mutable Message; made ChainMessage variants hold Arc-wrapped messages; moved per-tipset messages cache into ChainStore as MessagesInTipsetCache (Arc<Vec>); removed msgs_in_tipset from RPCState; updated imports and callsites to use MessageRead and Arc-based conversions.

Changes

Cohort / File(s)	Summary
Message trait & ChainMessage refactor src/message/mod.rs, src/message/chain_message.rs, src/message/signed_message.rs	Introduce MessageRead (getters) and make Message only setters; ChainMessage stores Arc<Message>/Arc<SignedMessage>; add From impls, delegate getters, use Arc::make_mut for setters.
ChainStore cache integration src/chain/store/chain_store.rs	Add messages_in_tipset_cache: MessagesInTipsetCache to ChainStore; change messages_for_tipset to return Result<Arc<Vec<ChainMessage>>, Error>; rename/refactor cache to MessagesInTipsetCache and update APIs; remove free helper functions.
RPCState cache removal & callsite updates src/rpc/mod.rs, src/daemon/mod.rs, src/rpc/methods/sync.rs, src/tool/offline_server/server.rs, src/tool/subcommands/api_cmd/generate_test_snapshot.rs, src/tool/subcommands/api_cmd/test_snapshot.rs	Remove msgs_in_tipset field from RPCState and remove its initialization across construction sites.
Import pattern updates to MessageRead src/chain_sync/tipset_syncer.rs, src/cli/subcommands/mpool_cmd.rs, src/eth/transaction.rs, src/interpreter/vm.rs, src/message_pool/msg_chain.rs, src/message_pool/msgpool/..., src/rpc/methods/state/types.rs, src/shim/crypto.rs, src/state_manager/mod.rs, src/tool/subcommands/api_cmd/api_compare_tests.rs, src/tool/subcommands/snapshot_cmd.rs	Replace Message / Message as ... imports with MessageRead as _ or anonymous imports so read-only accessors resolve via MessageRead.
Arc-based conversions & callsite adjustments src/rpc/methods/chain.rs, src/rpc/methods/eth.rs, src/rpc/methods/gas.rs, src/interpreter/vm.rs, src/state_manager/mod.rs, src/message_pool/msgpool/msg_pool.rs	Replace explicit ChainMessage::Signed/Unsigned(...) and clones with .into() conversions; use Arc::unwrap_or_clone and Into::into to handle Arc-wrapped message types at callsites.
Message iteration and borrowing changes src/rpc/methods/state.rs, src/state_manager/mod.rs	Iterate over Arc<Vec<ChainMessage>> by reference (.iter() / .cloned()) to avoid moving cached Arcs.
LRU Cache API change src/utils/cache/lru.rs	Removed public contains<Q>(&self, k: &Q) -> bool from SizeTrackingLruCache.
Workspace dependency feature Cargo.toml	Enable serde workspace feature rc in addition to derive.

Sequence Diagram(s)

(No sequence diagram generated.)

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~65 minutes

Possibly related PRs

• fix: refine Arc params to reduce pointer cloning #6255: modifies message retrieval/cache APIs in src/chain/store/chain_store.rs and overlaps with the cache relocation and API changes.
• fix: remove unique events logic from the ChainGetEvents API #6286: touches ChainStore fields/constructor changes similar to this PR.
• fix: remove excess vec capacity in caches #5935: related edits to the messages-in-tipset cache implementation and semantics.

Suggested reviewers

• akaladarshi
• sudo-shashank
• LesnyRumcajs

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 48.39% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The pull request title 'refactor: refine msgs_in_tipset cache usage' clearly and accurately describes the main change: refactoring the message-in-tipset cache implementation to be more refined/optimized. The title directly relates to the core modifications throughout the codebase that centralize cache management within ChainStore, wrap messages in Arc, split message traits, and remove redundant cache instantiations.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch hm/refine-msgs_in_tipset-cache

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch hm/refine-msgs_in_tipset-cache

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/chain/store/chain_store.rs (1)

605-608: ⚠️ Potential issue | 🟡 Minor

Update this docstring to the consolidated cache API.

messages_for_tipset_with_cache no longer exists, so this comment now points readers at a dead helper instead of ChainStore::messages_for_tipset.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.
In `@src/chain/store/chain_store.rs` around lines 605 - 608, The doc comment for
the tipset-to-messages cache still references the removed helper
messages_for_tipset_with_cache; update the comment to reference the consolidated
API ChainStore::messages_for_tipset (or the new consolidated cache API name if
different) and explain that this cache is intended to be used alongside
ChainStore::messages_for_tipset to avoid the heavier calls performed by the
canonical implementation. Keep the description concise and accurate, remove the
dead reference to messages_for_tipset_with_cache, and ensure the struct/function
comment mentions ChainStore::messages_for_tipset and the intended usage pattern.

🧹 Nitpick comments (1)

src/chain/store/chain_store.rs (1)

624-636: Cold misses can still stampede here.

get_or_insert_with does a separate lookup and insert, so concurrent misses for the same TipsetKey can all execute the fill closure before any value reaches the LRU. Now that ChainStore::messages_for_tipset routes every fill through this helper, a burst of identical requests can still decode the same tipset multiple times.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.
In `@src/chain/store/chain_store.rs` around lines 624 - 636, get_or_insert_with
currently does separate get then insert (using get and insert) which allows
concurrent cold misses for the same TipsetKey to run the fill closure in
parallel; change it to perform a single atomic "compute-if-absent" operation
(e.g. use the underlying map's entry API or a per-key in-progress placeholder)
so only the first caller executes the provided closure and others await the
result, updating the LRU once; update get_or_insert_with (and ensure
messages_for_tipset still routes fills through it) to use this atomic
entry/placeholder approach to prevent decode stampedes for the same TipsetKey.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/message/chain_message.rs`:
- Around line 17-21: The serde Serialize/Deserialize impls for Arc<T> used by
ChainMessage (which contains Arc<Message> and Arc<SignedMessage>) require
serde's "rc" feature; update the workspace dependency declaration for serde in
Cargo.toml (where serde = { version = "1", default-features = false, features =
["derive"] }) to include "rc" so it becomes features = ["derive", "rc"] so Arc
serialization/deserialization is available for ChainMessage.
---
Outside diff comments:
In `@src/chain/store/chain_store.rs`:
- Around line 605-608: The doc comment for the tipset-to-messages cache still
references the removed helper messages_for_tipset_with_cache; update the comment
to reference the consolidated API ChainStore::messages_for_tipset (or the new
consolidated cache API name if different) and explain that this cache is
intended to be used alongside ChainStore::messages_for_tipset to avoid the
heavier calls performed by the canonical implementation. Keep the description
concise and accurate, remove the dead reference to
messages_for_tipset_with_cache, and ensure the struct/function comment mentions
ChainStore::messages_for_tipset and the intended usage pattern.
---
Nitpick comments:
In `@src/chain/store/chain_store.rs`:
- Around line 624-636: get_or_insert_with currently does separate get then
insert (using get and insert) which allows concurrent cold misses for the same
TipsetKey to run the fill closure in parallel; change it to perform a single
atomic "compute-if-absent" operation (e.g. use the underlying map's entry API or
a per-key in-progress placeholder) so only the first caller executes the
provided closure and others await the result, updating the LRU once; update
get_or_insert_with (and ensure messages_for_tipset still routes fills through
it) to use this atomic entry/placeholder approach to prevent decode stampedes
for the same TipsetKey.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 8cc51baf-0e06-4e44-9484-c290161d43c8

📥 Commits

Reviewing files that changed from the base of the PR and between c2c6d23 and 1f6a651.

📒 Files selected for processing (30)

• src/chain/store/chain_store.rs
• src/chain_sync/tipset_syncer.rs
• src/cli/subcommands/mpool_cmd.rs
• src/daemon/mod.rs
• src/eth/transaction.rs
• src/interpreter/vm.rs
• src/message/chain_message.rs
• src/message/mod.rs
• src/message/signed_message.rs
• src/message_pool/msg_chain.rs
• src/message_pool/msgpool/mod.rs
• src/message_pool/msgpool/msg_pool.rs
• src/message_pool/msgpool/selection.rs
• src/message_pool/msgpool/test_provider.rs
• src/message_pool/msgpool/utils.rs
• src/rpc/methods/chain.rs
• src/rpc/methods/eth.rs
• src/rpc/methods/gas.rs
• src/rpc/methods/state.rs
• src/rpc/methods/state/types.rs
• src/rpc/methods/sync.rs
• src/rpc/mod.rs
• src/shim/crypto.rs
• src/state_manager/mod.rs
• src/tool/offline_server/server.rs
• src/tool/subcommands/api_cmd/api_compare_tests.rs
• src/tool/subcommands/api_cmd/generate_test_snapshot.rs
• src/tool/subcommands/api_cmd/test_snapshot.rs
• src/tool/subcommands/snapshot_cmd.rs
• src/utils/cache/lru.rs

💤 Files with no reviewable changes (7)

• src/tool/offline_server/server.rs
• src/tool/subcommands/api_cmd/generate_test_snapshot.rs
• src/utils/cache/lru.rs
• src/rpc/mod.rs
• src/tool/subcommands/api_cmd/test_snapshot.rs
• src/daemon/mod.rs
• src/rpc/methods/sync.rs

[codecov]

Codecov Report

❌ Patch coverage is 63.24786% with 43 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.60%. Comparing base (c2c6d23) to head (adbb101).
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
src/message/chain_message.rs	53.33%	14 Missing ⚠️
src/rpc/methods/eth.rs	33.33%	10 Missing ⚠️
src/rpc/methods/gas.rs	41.66%	3 Missing and 4 partials ⚠️
src/chain/store/chain_store.rs	90.90%	0 Missing and 3 partials ⚠️
src/message/signed_message.rs	50.00%	3 Missing ⚠️
src/state_manager/mod.rs	50.00%	3 Missing ⚠️
src/message_pool/msgpool/msg_pool.rs	0.00%	0 Missing and 2 partials ⚠️
src/rpc/methods/chain.rs	50.00%	1 Missing ⚠️

Additional details and impacted files

Files with missing lines	Coverage Δ
src/chain_sync/tipset_syncer.rs	63.45% <ø> (ø)
src/cli/subcommands/mpool_cmd.rs	67.41% <ø> (ø)
src/daemon/mod.rs	28.93% <ø> (+0.10%)	⬆️
src/eth/transaction.rs	86.69% <ø> (-0.15%)	⬇️
src/interpreter/vm.rs	81.51% <100.00%> (-0.23%)	⬇️
src/message/mod.rs	90.56% <100.00%> (ø)
src/message_pool/msg_chain.rs	84.69% <ø> (ø)
src/message_pool/msgpool/mod.rs	90.98% <ø> (-0.15%)	⬇️
src/message_pool/msgpool/selection.rs	87.79% <ø> (ø)
src/message_pool/msgpool/test_provider.rs	98.30% <ø> (ø)
... and 19 more

... and 5 files with indirect coverage changes

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c2c6d23...adbb101. Read the comment docs.