feat: add support for reward actor in state decode params API #5888
Conversation
WalkthroughSupport for the Reward actor has been added to the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API
participant Registry
participant RewardActor
Client->>API: StateDecodeParams (Reward, MethodNum, Params)
API->>Registry: Lookup RewardActor method/version
Registry->>RewardActor: Deserialize Params via HasLotusJson
RewardActor-->>Registry: Decoded Params
Registry-->>API: Decoded Params
API-->>Client: Decoded Params Result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Suggested labels
Suggested reviewers
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
⏰ 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). (9)
✨ 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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
c13302a to
4c1c7bb
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/rpc/registry/actors/reward.rs (1)
30-45: Consider renaming the macro for clarity.The macro name
register_reward_version_8_to_9is misleading since it's also used for version 10 (line 56). Consider renaming it toregister_reward_version_8_to_10to better reflect its usage.-macro_rules! register_reward_version_8_to_9 { +macro_rules! register_reward_version_8_to_10 {Also update the usage sites accordingly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/lotus_json/actor_states/methods/mod.rs(1 hunks)src/lotus_json/actor_states/methods/reward_methods.rs(1 hunks)src/lotus_json/big_int.rs(1 hunks)src/rpc/registry/actors/mod.rs(1 hunks)src/rpc/registry/actors/reward.rs(1 hunks)src/rpc/registry/methods_reg.rs(2 hunks)src/tool/subcommands/api_cmd/api_compare_tests.rs(3 hunks)src/tool/subcommands/api_cmd/test_snapshots.txt(1 hunks)
⏰ 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). (18)
- GitHub Check: tests
- GitHub Check: tests-release
- GitHub Check: Build forest binaries on Linux AMD64
- GitHub Check: Devnet checks
- GitHub Check: Snapshot export checks
- GitHub Check: Calibnet kademlia checks
- GitHub Check: Bootstrap checks - Forest
- GitHub Check: Bootstrap checks - Lotus
- GitHub Check: Calibnet eth mapping check
- GitHub Check: Calibnet no discovery checks
- GitHub Check: db-migration-checks
- GitHub Check: Wallet tests
- GitHub Check: State migrations
- GitHub Check: Calibnet check
- GitHub Check: Forest CLI checks
- GitHub Check: Calibnet stateless mode check
- GitHub Check: Analyze (rust)
- GitHub Check: Analyze (go)
🔇 Additional comments (14)
src/lotus_json/big_int.rs (1)
33-61: LGTM! Well-structured macro for DRY implementation.The macro effectively generates consistent
HasLotusJsonimplementations across multipleBigIntDewrapper types from differentfvm_sharedversions. The implementation correctly delegates to the innerBigIntfield and maintains consistency with the baseBigIntimplementation pattern.src/tool/subcommands/api_cmd/test_snapshots.txt (1)
137-139: LGTM! New test snapshots for reward actor methods.The snapshot files follow the established naming convention and support the new reward actor method parameter decoding tests mentioned in the PR objectives.
src/lotus_json/actor_states/methods/mod.rs (1)
12-12: LGTM! Proper module integration.The module declaration follows the established pattern and correctly integrates the new reward methods functionality.
src/rpc/registry/actors/mod.rs (1)
8-8: LGTM! Consistent actor module declaration.The module declaration follows the established pattern and correctly integrates the reward actor into the registry system.
src/rpc/registry/methods_reg.rs (2)
77-77: LGTM! Proper import addition.The reward module is correctly added to the import statement, following the established pattern.
86-86: LGTM! Version-aware reward actor registration.The reward actor case is properly integrated into the match statement. The inclusion of the
_versionparameter is appropriate given the PR objective to support reward actor versions 8-16, making this implementation version-aware unlike other actors.src/rpc/registry/actors/reward.rs (2)
1-28: LGTM!The imports are appropriate and the macro implementation for versions 11-16 correctly handles method registration with proper parameter types. The separation of parameterized and parameterless methods is well-structured.
47-66: LGTM!The main registration function correctly handles all required reward actor versions (8-16) as specified in the PR objectives. The version-based dispatch to appropriate macros is well-structured and the default case appropriately handles unsupported versions.
src/tool/subcommands/api_cmd/api_compare_tests.rs (2)
55-55: LGTM!The
num_bigint::BigIntimport is necessary for the new reward actor parameter definitions and follows the established import patterns.
1920-1937: LGTM!The new reward actor test cases follow the established patterns and correctly use the reward actor methods and address. The test structure will properly validate parameter decoding for reward actor versions.
src/lotus_json/actor_states/methods/reward_methods.rs (4)
1-41: LGTM!The JSON wrapper structs are well-designed with proper Serde annotations. The use of
#[serde(transparent)]for simple wrappers and#[serde(rename_all = "PascalCase")]for structured data follows Lotus JSON conventions correctly.
44-82: LGTM!The macro implementation for constructor parameters is well-structured with proper version handling, comprehensive test snapshots, and correct type conversions between BigIntDe wrappers and BigInt types.
85-133: LGTM!The award block reward parameters macro is correctly implemented with realistic test data and proper field-by-field conversions between native types and their JSON representations.
136-181: LGTM!The update network KPI parameters macro maintains consistency with the constructor parameters approach. The macro invocations correctly map versions to their corresponding fvm_shared types, with appropriate version ranges for each parameter type based on when features were introduced or structure changes occurred.
Summary of changes
Changes introduced in this pull request:
StateDecodeParamsReference issue to close (if applicable)
Closes #5719
Other information and links
Change checklist
Summary by CodeRabbit