fix(rpc): bound nEpochs in StateCompute#7374
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)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
🚧 Files skipped from review as they are similar to previous changes (1)
Walkthrough
ChangesState computation limits
Estimated code review effort: 3 (Moderate) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant ForestStateCompute
participant Semaphore
participant TipsetTask
Client->>ForestStateCompute: submit nEpochs
ForestStateCompute->>ForestStateCompute: validate configured maximum
ForestStateCompute->>TipsetTask: spawn per-tipset computation
TipsetTask->>Semaphore: acquire permit
Semaphore-->>TipsetTask: permit
TipsetTask-->>ForestStateCompute: completed tipset result
ForestStateCompute->>Semaphore: acquire processing permit
ForestStateCompute->>Client: return state computation result
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/docs/users/reference/env_variables.md`:
- Line 78: In the FOREST_STATE_COMPUTE_MAX_RANGE documentation entry, correct
the description typo by changing “accpets” to “accepts” without altering the
surrounding table content.
In `@src/rpc/methods/state.rs`:
- Around line 1659-1664: Keep the semaphore permit acquired in the spawned task
alive until the corresponding compute_tipset_state call completes in the parent
loop. Propagate or retain the permit alongside the spawned task’s result, then
release it only after state computation finishes, preserving the shared
concurrency limit across concurrent Forest.StateCompute requests.
- Around line 1620-1628: Update the n_epochs conversion in the state computation
flow to use try_into() instead of an unchecked cast to ChainEpoch. Return a
contextual error if the conversion fails, while preserving the existing
maximum-range validation and successful epoch-range behavior.
🪄 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: 85218c9d-6069-485c-b8c9-d2577e6abdb6
📒 Files selected for processing (2)
docs/docs/users/reference/env_variables.mdsrc/rpc/methods/state.rs
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
filecoin-project/lotus(manual)
LesnyRumcajs
left a comment
LesnyRumcajs
left a comment
There was a problem hiding this comment.
This a breaking change in the unlikely case that someone was using this internal (but exposed) method.
Codecov Report❌ Patch coverage is
Additional details and impacted files
... and 12 files with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/rpc/methods/state.rs`:
- Around line 1627-1628: Update the epoch endpoint calculation in the
surrounding RPC method to use checked addition and subtraction instead of
`from_epoch + n_epochs - 1`. Propagate a contextual error when either operation
overflows, while preserving the existing epoch range behavior for valid inputs.
🪄 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: f129f6b8-a859-4005-869d-c71a8fc5cd54
⛔ 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 (5)
CHANGELOG.mddocs/docs/users/reference/env_variables.mdsrc/cli/subcommands/state_cmd.rssrc/lotus_json/mod.rssrc/rpc/methods/state.rs
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
filecoin-project/lotus(manual)
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/docs/users/reference/env_variables.md
2 participants
Summary of changes
Changes introduced in this pull request:
Reference issue to close (if applicable)
Closes
Other information and links
Change checklist
Outside contributions
Summary by CodeRabbit
nEpochsinForest.StateCompute, defaulting to 2,000.nEpochsexceeding the configured maximum are now rejected early.n_epochs/nEpochsnow useNonZeroU32for both thestate computeCLI andForest.StateComputeRPC.FOREST_STATE_COMPUTE_MAX_RANGE(default 2,000, example 100).