fix(rpc): update StateSearchMsg to return null instead of error when not found#6817
fix(rpc): update StateSearchMsg to return null instead of error when not found#6817hanabi1224 merged 3 commits intomainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
WalkthroughState search RPC handlers now return nullable results (Option) when a message is not found; test validators were updated to handle optional lookups; build tooling ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
There was a problem hiding this comment.
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/tool/subcommands/api_cmd/api_compare_tests.rs (1)
1237-1249:⚠️ Potential issue | 🟡 MinorAdd an explicit not-found
StateSearchMsgcase.Every request here is built from sampled on-chain message CIDs, so this change still only verifies the
Some(MessageLookup)path. Please add at least one case where the message exists in the snapshot but falls outside the selected lookback/tipset window, so the newnullbehavior is actually covered.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tool/subcommands/api_cmd/api_compare_tests.rs` around lines 1237 - 1249, Add a test invocation that exercises the "not-found / null" path by creating a StateSearchMsg request whose lookback/tipset window excludes the sampled msg_cid and then validate that the response is null; for example, add a validate_message_lookup(StateSearchMsg::request((None.into(), msg_cid, /* very small lookback or older tipset */ 0, true))?) (or a similar StateSearchMsg::request with parameters that put the message outside the window) alongside the existing validate_message_lookup calls so the code that handles the None/null result for StateSearchMsg is exercised.
🤖 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/rpc/methods/state.rs`:
- Line 1274: The RPC docs still claim a lookup is always returned but the actual
return type is Option<MessageLookup> (type Ok = Option<MessageLookup>), so
update the inline RPC descriptions for the methods that use MessageLookup to
state they return either a MessageLookup or null when the item is missing;
locate the doc comments adjacent to the declaration using MessageLookup (the
block containing "type Ok = Option<MessageLookup>;") and the other similar
occurrence around the second use (near the other Option<MessageLookup>
declaration) and change phrasing to explicitly mention "null on misses" or
equivalent nullable wording.
---
Outside diff comments:
In `@src/tool/subcommands/api_cmd/api_compare_tests.rs`:
- Around line 1237-1249: Add a test invocation that exercises the "not-found /
null" path by creating a StateSearchMsg request whose lookback/tipset window
excludes the sampled msg_cid and then validate that the response is null; for
example, add a validate_message_lookup(StateSearchMsg::request((None.into(),
msg_cid, /* very small lookback or older tipset */ 0, true))?) (or a similar
StateSearchMsg::request with parameters that put the message outside the window)
alongside the existing validate_message_lookup calls so the code that handles
the None/null result for StateSearchMsg is exercised.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 1abfe056-ebc8-4a9e-a695-faee020714ce
⛔ Files ignored due to path filters (2)
src/rpc/snapshots/forest__rpc__tests__rpc__v0.snapis excluded by!**/*.snapsrc/rpc/snapshots/forest__rpc__tests__rpc__v1.snapis excluded by!**/*.snap
📒 Files selected for processing (3)
mise.tomlsrc/rpc/methods/state.rssrc/tool/subcommands/api_cmd/api_compare_tests.rs
7d56c35 to
3c60922
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files
... and 8 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
LesnyRumcajs
left a comment
There was a problem hiding this comment.
Let's add a changelog entry, it's a user-facing change.
@LesnyRumcajs changelog updated |
Summary of changes
To match Lotus logic: filecoin-project/lotus#13562 (comment)
Changes introduced in this pull request:
Reference issue to close (if applicable)
Closes
Other information and links
Change checklist
Outside contributions
Summary by CodeRabbit
Bug Fixes
Tests
Documentation