-
Notifications
You must be signed in to change notification settings - Fork 178
feat: experimental apis for f05 state dump #5907
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes introduce new RPC methods for filtering and dumping Storage Market deals, extend the API to support an "experimental" endpoint, and update dependencies to use specific Git revisions for certain crates. Additional utility methods for iterating deal proposals are added, and the API versioning and client logic are updated to handle the new "experimental" path. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant UrlClient
participant API
Client->>Client: get_or_init_client(ApiPaths::Experimental)
alt Not initialized
Client->>UrlClient: Initialize with base_url + "/rpc/experimental"
UrlClient-->>Client: Return new UrlClient
else Already initialized
Client-->>Client: Return existing UrlClient
end
Client->>API: Make request via experimental UrlClient
sequenceDiagram
participant User
participant RPC
participant MarketState
User->>RPC: Call StateMarketDealsFiltered(filter)
RPC->>MarketState: Load market actor state
MarketState->>MarketState: Iterate deals with for_each_cacheless
MarketState->>RPC: Return filtered deals
RPC-->>User: Return map of deal_id → deal
User->>RPC: Call StateMarketDealsDump(tipset, file)
RPC->>MarketState: Load market actor state
MarketState->>MarketState: Iterate deals with for_each_cacheless
MarketState->>RPC: Serialize and write each deal to NDJSON file
RPC-->>User: Return success
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
✨ Finishing Touches🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
@CodeRabbit full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/rpc/methods/state.rs (1)
555-567
: Consider adding progress reporting for large datasets.For very large market deal datasets, users might benefit from progress reporting to track the dump operation.
You could add periodic progress logging:
+ let mut count = 0u64; da.for_each_cacheless(|deal_id, d| { let s = sa.get(deal_id)?.unwrap_or_else(DealState::empty); let market_deal = ApiMarketDeal { proposal: d?.into(), state: s.into(), }; write!( writer, "{}\n", crate::lotus_json::HasLotusJson::into_lotus_json_string(market_deal)? )?; + count += 1; + if count % 10000 == 0 { + tracing::info!("Dumped {} deals to {}", count, output_file); + } Ok(()) })?; + tracing::info!("Successfully dumped {} deals to {}", count, output_file);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lock
is excluded by!**/*.lock
📒 Files selected for processing (7)
Cargo.toml
(1 hunks)src/rpc/client.rs
(4 hunks)src/rpc/methods/state.rs
(4 hunks)src/rpc/mod.rs
(1 hunks)src/rpc/reflect/mod.rs
(2 hunks)src/rpc/segregation_layer.rs
(1 hunks)src/shim/actors/builtin/market/mod.rs
(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: in the forest codebase, `vec` can be used as an `asyncwrite` implementation in test contexts. th...
Learnt from: hanabi1224
PR: ChainSafe/forest#5835
File: src/chain/tests.rs:58-76
Timestamp: 2025-08-04T13:36:22.993Z
Learning: In the Forest codebase, `Vec<u8>` can be used as an `AsyncWrite` implementation in test contexts. The user confirmed that tests using `&mut Vec<u8>` with `export` and `export_v2` functions compile and pass both locally and on CI.
Applied to files:
src/rpc/reflect/mod.rs
src/rpc/methods/state.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: All lint checks
- GitHub Check: cargo-publish-dry-run
- GitHub Check: Build MacOS
- GitHub Check: Build Ubuntu
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (rust)
🔇 Additional comments (13)
src/rpc/mod.rs (1)
210-211
: LGTM! New RPC methods properly registered.The addition of
StateMarketDealsFiltered
andStateMarketDealsDump
follows the established pattern for registering RPC methods in thefor_each_rpc_method
macro. These methods are correctly placed in the state vertical section.src/rpc/segregation_layer.rs (1)
19-24
: LGTM! Experimental API path properly registered.The addition of
ApiPaths::Experimental
to the version methods mapping correctly extends the segregation layer to support experimental RPC methods. This follows the same pattern as existing API versions and ensures experimental methods will be properly routed and validated.src/rpc/reflect/mod.rs (2)
127-129
: LGTM! Experimental API path properly implemented.The new
Experimental
variant is well-implemented:
- Uses a unique bitflag value (0b10000000) that doesn't conflict with existing variants
- Includes proper case-insensitive string parsing annotation
- Follows the established pattern for API path variants
Note that the
all()
method intentionally excludes the experimental path, which is appropriate for experimental features.
442-444
: LGTM! Test coverage for experimental API path.The test case properly verifies that the URI path
/rpc/experimental
correctly maps to theExperimental
variant, ensuring the string parsing works as expected.src/shim/actors/builtin/market/mod.rs (2)
389-435
: LGTM! Well-implemented cacheless iteration method.The
for_each_cacheless
method is correctly implemented:
- Follows the same pattern as the existing
for_each
method- Properly delegates to the underlying array's
for_each_cacheless
method for all variants (V9-V16)- Maintains consistent error handling and function signature
- Provides a memory-efficient alternative for iteration without caching
451-462
: LGTM! Simple and correct count method.The
count
method is properly implemented:
- Correctly delegates to the underlying array's
count
method for all variants- Follows the established pattern for enum method delegation
- Provides a useful utility for getting the total number of deal proposals
src/rpc/client.rs (4)
40-40
: LGTM! Consistent field addition following established patterns.The
experimental
field follows the same pattern as the existing version fields, usingOnceCell
for proper lazy initialization.
88-88
: LGTM! Proper field initialization.The
experimental
field is correctly initialized using the same pattern as other version fields.
167-167
: LGTM! Consistent pattern matching extension.The match arm correctly handles the
ApiPaths::Experimental
variant and returns the appropriate field reference.
176-176
: LGTM! Appropriate URL path mapping.The experimental API path mapping to "rpc/experimental" follows the established convention and will correctly construct the experimental endpoint URL.
src/rpc/methods/state.rs (3)
13-13
: LGTM!The import additions are appropriate for the new functionality -
LotusJson
for serialization andstd::io::Write
for file operations.Also applies to: 71-71
501-501
: Good optimization for memory efficiency.Using
for_each_cacheless
instead offor_each
is appropriate here as it avoids unnecessary caching when iterating over potentially large deal datasets.
572-584
: LGTM!The filter struct is well-designed with appropriate serialization attributes and optional fields for flexible filtering.
619631a
to
b20ff1c
Compare
Summary of changes
Changes introduced in this pull request:
/v1
but/experimental
Forest.StateMarketDealsDump
which accepts the tipset (ornull
for heaviest) AND the file to dump the JSON to on the host.ForestStateMarketDealsFiltered
which accepts the tipset AND clients/providers to filter on.fvm_ipld_amt 0.7.5
filecoin-project/ref-fvm#2195 is merged and thefvm_ipld_amt 0.7.5
is released (CI will prevent it anyway)Reference issue to close (if applicable)
Closes #5880
Other information and links
Sample usage:
Change checklist
Summary by CodeRabbit
New Features
Enhancements
Chores