-
Notifications
You must be signed in to change notification settings - Fork 182
feat: add support for payment channel actor in state decode params API #5996
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
feat: add support for payment channel actor in state decode params API #5996
Conversation
WalkthroughAdds PaymentChannel actor support to StateDecodeParams: new LotusJson param types for payment-channel methods (v8–v16), per-version RPC method registrations for the actor, API compare tests and snapshots for v16 paych decode cases, and a minor changelog cosmetic edit. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Node
participant Registry as MethodRegistry
participant Paych as rpc::registry::actors::payment_channel
participant StateAPI as Filecoin.StateDecodeParams
Note over Node,Registry: Startup
Node->>Registry: init()
Registry->>Registry: detect BuiltinActor::PaymentChannel
Registry->>Paych: register_actor_methods(code_cid, version)
alt version ∈ [8..16]
Paych-->>Registry: register Constructor (typed), UpdateChannelState (typed), Settle (empty), Collect (empty)
else
Paych-->>Registry: no-op
end
Note over Node,StateAPI: Runtime decode request
Node->>StateAPI: StateDecodeParams(actor=PaymentChannel, method, params_bytes)
StateAPI->>Registry: lookup method schema & LotusJson mapping
Registry-->>StateAPI: param schema / (de)serializers
StateAPI-->>Node: decoded params (LotusJson -> internal)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Out-of-scope changes
Possibly related PRs
Suggested reviewers
✨ 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 (
|
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 (11)
src/lotus_json/actor_states/methods/paych_params.rs (3)
137-143: Align schema derivations and annotations across versions (V2/V3 missing JsonSchema and schemars attr for Secret)V4 UpdateChannelStateParams derives JsonSchema and annotates Secret with schemars(with = …), V2/V3 do not. For consistency and API schema generation, mirror V4.
-#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)] +#[derive(Serialize, Deserialize, JsonSchema, Debug, Clone, PartialEq)] #[serde(rename_all = "PascalCase")] pub struct UpdateChannelStateParamsV2LotusJson { pub sv: SignedVoucherV2LotusJson, - #[serde(with = "crate::lotus_json")] + #[schemars(with = "LotusJson<Vec<u8>>")] + #[serde(with = "crate::lotus_json")] pub secret: Vec<u8>, } @@ -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)] +#[derive(Serialize, Deserialize, JsonSchema, Debug, Clone, PartialEq)] #[serde(rename_all = "PascalCase")] pub struct UpdateChannelStateParamsV3LotusJson { pub sv: SignedVoucherV3LotusJson, - #[serde(with = "crate::lotus_json")] + #[schemars(with = "LotusJson<Vec<u8>>")] + #[serde(with = "crate::lotus_json")] pub secret: Vec<u8>, }Also applies to: 145-151
68-70: Comment contradicts behavior and snapshots regarding Merges null vs []The comment states Lotus returns null when no merges; your snapshots and recommended serialization use []. Since from_lotus_json already tolerates None/null, prefer serializing [] for consistency and update the comment accordingly.
Would you like me to update the comment to: “Lotus may return null (not []), but we serialize empty as [], and accept null during deserialization for compatibility.”?
Also applies to: 99-101, 130-132
515-516: Minor: extra space in field initializerDouble space after colon is inconsistent.
- actor: e.actor.into(), + actor: e.actor.into(),src/rpc/registry/methods_reg.rs (1)
268-275: Add PaymentChannel to supported actors test listThe test enumerates actors expected to have methods registered; adding PaymentChannel will guard against regressions in its registration.
let supported_actors = vec![ BuiltinActor::Account, BuiltinActor::Miner, BuiltinActor::EVM, BuiltinActor::Cron, BuiltinActor::DataCap, + BuiltinActor::PaymentChannel, ];I can also add a focused test asserting that PaymentChannel Settle (3) and Collect (4) are registered as “empty” params and behave per the base64/{} convention.
src/rpc/registry/actors/payment_channel.rs (3)
4-5: Avoid macro/function name ambiguity by aliasing or fully-qualifying the macro.This module defines a function named
register_actor_methodswhile also importingregister_actor_methodsfrommethods_reg. Even though macros and functions live in different namespaces, the identical name hurts readability and can mislead IDE tooling. Alias the macro import (or fully-qualify the macro calls) to make intent explicit.Apply this diff to disambiguate:
-use crate::rpc::registry::methods_reg::{MethodRegistry, register_actor_methods}; +use crate::rpc::registry::methods_reg::{MethodRegistry, register_actor_methods as register_methods_macro};And update macro invocations:
- register_actor_methods!( + register_methods_macro!( $registry, $code_cid, [ (Method::Constructor, ConstructorParams), (Method::UpdateChannelState, UpdateChannelStateParams), ] ); @@ - register_actor_methods!( + register_methods_macro!( $registry, $code_cid, [(Method::Settle, empty), (Method::Collect, empty),] );
8-29: Document the version-registration macro.Add a short doc comment explaining that the macro wires PaymentChannel methods for a specific actor version, and that
emptydenotes parameterless methods. This helps future contributors mirror the pattern for other actors.
31-44: Log unsupported versions to aid debugging.Silently ignoring unknown versions makes troubleshooting harder when new actor versions land. A low-level log on the default arm would help detect gaps without impacting behavior.
Example patch:
+use tracing::warn; @@ match version { 8 => register_payment_channel_reg_versions!(registry, cid, fil_actor_paych_state::v8), 9 => register_payment_channel_reg_versions!(registry, cid, fil_actor_paych_state::v9), 10 => register_payment_channel_reg_versions!(registry, cid, fil_actor_paych_state::v10), 11 => register_payment_channel_reg_versions!(registry, cid, fil_actor_paych_state::v11), 12 => register_payment_channel_reg_versions!(registry, cid, fil_actor_paych_state::v12), 13 => register_payment_channel_reg_versions!(registry, cid, fil_actor_paych_state::v13), 14 => register_payment_channel_reg_versions!(registry, cid, fil_actor_paych_state::v14), 15 => register_payment_channel_reg_versions!(registry, cid, fil_actor_paych_state::v15), 16 => register_payment_channel_reg_versions!(registry, cid, fil_actor_paych_state::v16), - _ => {} + v => warn!("PaymentChannel actor: unsupported version {v}, skipping registration"), }src/tool/subcommands/api_cmd/api_compare_tests.rs (4)
1919-1919: Don’t panic in tests: propagate serialization errors instead of unwrap.
to_vec(&constructor_params).unwrap()will panic on serialization errors and is inconsistent with surrounding tests that use?. Prefer?to bubble errors throughanyhow::Result.Apply this diff:
- to_vec(&constructor_params).unwrap(), + to_vec(&constructor_params)?,
1923-1927: Same here: replace unwrap with error propagation.For consistency and better failure reporting, use
?instead ofunwrap().Apply this diff:
- to_vec(&update_channel_state).unwrap(), + to_vec(&update_channel_state)?,
1884-1893: Optional: factor Paych address into a constant to match existing style.You already have
MINER_ADDRESS,ACCOUNT_ADDRESS, andEVM_ADDRESSconstants. Introducing aPAYCH_ADDRESSconstant improves discoverability and reuse across tests.Add near other address constants (outside this hunk):
const PAYCH_ADDRESS: Address = Address::new_id(66116); // payment channel actor t066116Then update here:
- let paych_address = Address::new_id(66116); + let paych_address = PAYCH_ADDRESS;
1884-1941: Consider adding at least one pre-v16 decode case (optional).The registry supports v8–v16. While v16 coverage is great, a single additional case (e.g., v12) can catch cross-version param shape drift early. This is optional if snapshot data or Lotus JSON support isn’t readily available.
Would you like me to draft a minimal v12 decode test using the same scaffolding?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
src/lotus_json/actor_states/methods/mod.rs(1 hunks)src/lotus_json/actor_states/methods/paych_params.rs(1 hunks)src/rpc/registry/actors/mod.rs(1 hunks)src/rpc/registry/actors/payment_channel.rs(1 hunks)src/rpc/registry/methods_reg.rs(2 hunks)src/tool/subcommands/api_cmd/api_compare_tests.rs(1 hunks)src/tool/subcommands/api_cmd/test_snapshots.txt(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/tool/subcommands/api_cmd/api_compare_tests.rs (2)
src/shim/address.rs (1)
new_id(142-144)src/blocks/tipset.rs (2)
key(336-339)key(530-533)
src/rpc/registry/methods_reg.rs (1)
src/rpc/registry/actors/payment_channel.rs (1)
register_actor_methods(31-44)
src/lotus_json/actor_states/methods/paych_params.rs (2)
src/shim/address.rs (1)
new_id(142-144)src/shim/econ.rs (1)
from_atto(106-108)
⏰ 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)
- GitHub Check: Build forest binaries on Linux AMD64
- GitHub Check: All lint checks
- GitHub Check: tests-release
- GitHub Check: tests
- GitHub Check: cargo-publish-dry-run
- GitHub Check: Build MacOS
- GitHub Check: Build Ubuntu
- GitHub Check: Analyze (rust)
- GitHub Check: Analyze (go)
🔇 Additional comments (6)
src/lotus_json/actor_states/methods/paych_params.rs (1)
542-546: Version mapping looks correct (V2: v8–v9, V3: v10–v11, V4: v12–v16)Registration covers V8–V16 as required by the issue, with appropriate param versions per actor version.
src/rpc/registry/actors/mod.rs (1)
11-11: Module wiring addedpayment_channel module is correctly exposed crate-privately alongside other actors.
src/lotus_json/actor_states/methods/mod.rs (1)
16-16: Paych params module inclusionPrivate module inclusion is appropriate; keeps public API surface stable while enabling registry usage.
src/rpc/registry/methods_reg.rs (2)
76-79: Actor imports updated to include payment_channelImport ordering and names are consistent with existing pattern.
100-102: PaymentChannel dispatch integrated into registryActor-specific registration for PaymentChannel is correctly hooked into the central registry.
src/tool/subcommands/api_cmd/api_compare_tests.rs (1)
1879-1879: Nice: paych tests integrated into the state decode suite.Wiring the PaymentChannel decode tests into
state_decode_params_api_testskeeps coverage consistent with other actors.
| #[schemars(with = "LotusJson<Vec<u8>>")] | ||
| #[serde(with = "crate::lotus_json", rename = "SecretHash")] | ||
| pub secret_pre_image: Vec<u8>, |
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.
SecretPreImage field incorrectly renamed to SecretHash (breaks JSON shape and tests)
The SignedVoucher structs override the PascalCase name to "SecretHash". Lotus and your snapshots expect "SecretPreImage"; this mismatch will cause decoding snapshot tests to fail and diverge from Lotus.
Apply this diff to all three voucher versions:
@@
- #[schemars(with = "LotusJson<Vec<u8>>")]
- #[serde(with = "crate::lotus_json", rename = "SecretHash")]
- pub secret_pre_image: Vec<u8>,
+ #[schemars(with = "LotusJson<Vec<u8>>")]
+ #[serde(with = "crate::lotus_json")]
+ pub secret_pre_image: Vec<u8>,Also applies to: 87-90, 118-121
🤖 Prompt for AI Agents
In src/lotus_json/actor_states/methods/paych_params.rs around lines 56-58 (and
also update the same pattern at lines 87-90 and 118-121), the serde rename
attribute incorrectly sets the JSON field to "SecretHash" causing snapshots to
diverge from Lotus; change the serde rename value from "SecretHash" to
"SecretPreImage" for the SecretPreImage field in all three SignedVoucher
versions (keep the schemars and serde(with = "crate::lotus_json") attributes
intact) so the JSON shape matches Lotus and tests.
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.
The field names are derived from the builtin-actors crate and the SecretHash added for lotus compatibility
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.
.
The serde rename attribute is indeed correct for Lotus compatibility. My apologies for the confusion in the original review comment - the field naming you've implemented matches what Lotus expects in its JSON API format.
✏️ Learnings added
Learnt from: akaladarshi
PR: ChainSafe/forest#5996
File: src/lotus_json/actor_states/methods/paych_params.rs:56-58
Timestamp: 2025-09-02T08:44:08.311Z
Learning: In Lotus JSON for payment channel SignedVoucher, the secret_pre_image field should use serde rename to "SecretHash" (not "SecretPreImage") for Lotus API compatibility, even though the internal struct field is named SecretPreimage. This matches the actual JSON format used by Lotus.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: akaladarshi
PR: ChainSafe/forest#5896
File: src/lotus_json/actor_states/methods/verified_reg_actor.rs:133-137
Timestamp: 2025-08-18T12:25:29.183Z
Learning: In Lotus JSON for verified registry actor, the SectorAllocationClaimsLotusJson struct uses "SectorExpiry" as the field name for the expiry field, not "Expiry". The current code with #[serde(rename = "SectorExpiry")] is correct.
| merges: if self.sv.merges.is_empty() { | ||
| None | ||
| } else { | ||
| Some(self.sv.merges.into_iter().map(|m| MergeLotusJson { | ||
| lane: m.lane, | ||
| nonce: m.nonce, | ||
| }).collect()) | ||
| }, | ||
| signature: self.sv.signature, |
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.
Empty merges serialized as null; snapshots (and typical expectations) use []
into_lotus_json currently maps empty merges to None→null. Your snapshots emit "Merges": [], so tests will fail. Also, many clients expect an empty array when no merges are present.
Minimal fix: always serialize an array (possibly empty). Keep from_lotus_json tolerant of null via existing Option handling.
- merges: if self.sv.merges.is_empty() {
- None
- } else {
- Some(self.sv.merges.into_iter().map(|m| MergeLotusJson {
- lane: m.lane,
- nonce: m.nonce,
- }).collect())
- },
+ merges: Some(
+ self.sv
+ .merges
+ .into_iter()
+ .map(|m| MergeLotusJson { lane: m.lane, nonce: m.nonce })
+ .collect()
+ ),Repeat the same change for the V3 and V4 impls in the indicated ranges.
Also applies to: 381-389, 493-501
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.
This is done for the lotus compatibility, Lotus returns null when the merges is empty.
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.
@akaladarshi, thank you for the clarification! You're absolutely right - if Lotus returns null for empty merges, then the current implementation is correct for compatibility reasons.
However, this means the test snapshots in the code should be updated to expect null instead of [] for the Merges field to match the actual Lotus behavior:
"Merges": nullinstead of:
"Merges": []This would ensure the tests accurately reflect the expected Lotus-compatible output.
(\ /)
( .. )
o(")(")
learning added
✏️ Learnings added
Learnt from: akaladarshi
PR: ChainSafe/forest#5996
File: src/lotus_json/actor_states/methods/paych_params.rs:269-277
Timestamp: 2025-09-02T08:46:04.916Z
Learning: In payment channel actor Lotus JSON serialization, empty merges should be serialized as `null` (not an empty array `[]`) to maintain compatibility with Lotus behavior.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
|
no green checkmark, no review! |
…-account-state-decode-params
|
|
||
| pub(crate) fn register_actor_methods(registry: &mut MethodRegistry, cid: Cid, version: u64) { | ||
| match version { | ||
| 8 => register_payment_channel_reg_versions!(registry, cid, fil_actor_paych_state::v8), |
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.
Better not to duplicate version literals here to avoid typos.
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.
Actually we are planning to use enum tracking issue #5937, so we won't have typos.
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.
By typo, I mean the pair of 8 and ::v8, I think it compiles if we write 8 => ::v9 by mistake. My suggested change is in #5996 (comment)
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.
Done
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: 0
🧹 Nitpick comments (2)
src/rpc/registry/methods_reg.rs (1)
266-279: Add PaymentChannel to the “supported actors” smoke-test list.This keeps the invariant that all registered actors have at least their constructor method present, and guards future regressions.
Apply this diff:
let supported_actors = vec![ BuiltinActor::Account, BuiltinActor::Miner, BuiltinActor::EVM, BuiltinActor::Cron, BuiltinActor::DataCap, + BuiltinActor::PaymentChannel, ];src/tool/subcommands/api_cmd/api_compare_tests.rs (1)
1909-1967: Avoid panics: replace unwrap() with error propagation.Two
to_vec(...).unwrap()calls can panic; use?for consistent error handling like other actor tests.Apply this diff:
- to_vec(&constructor_params).unwrap(), + to_vec(&constructor_params)?, ... - to_vec(&update_channel_state).unwrap(), + to_vec(&update_channel_state)?,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
CHANGELOG.md(0 hunks)src/rpc/registry/actors/mod.rs(1 hunks)src/rpc/registry/methods_reg.rs(2 hunks)src/tool/subcommands/api_cmd/api_compare_tests.rs(1 hunks)src/tool/subcommands/api_cmd/test_snapshots.txt(1 hunks)
💤 Files with no reviewable changes (1)
- CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (2)
- src/rpc/registry/actors/mod.rs
- src/tool/subcommands/api_cmd/test_snapshots.txt
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: akaladarshi
PR: ChainSafe/forest#5996
File: src/lotus_json/actor_states/methods/paych_params.rs:269-277
Timestamp: 2025-09-02T08:46:04.916Z
Learning: In payment channel actor Lotus JSON serialization, empty merges should be serialized as `null` (not an empty array `[]`) to maintain compatibility with Lotus behavior.
🧬 Code graph analysis (2)
src/tool/subcommands/api_cmd/api_compare_tests.rs (2)
src/shim/address.rs (1)
new_id(142-144)src/blocks/tipset.rs (2)
key(336-339)key(530-533)
src/rpc/registry/methods_reg.rs (1)
src/rpc/registry/actors/payment_channel.rs (1)
register_actor_methods(31-44)
⏰ 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). (10)
- GitHub Check: All lint checks
- GitHub Check: Check
- GitHub Check: tests-release
- GitHub Check: tests
- GitHub Check: Build Ubuntu
- GitHub Check: cargo-publish-dry-run
- GitHub Check: Build MacOS
- GitHub Check: Build forest binaries on Linux AMD64
- GitHub Check: Analyze (rust)
- GitHub Check: Analyze (go)
🔇 Additional comments (4)
src/rpc/registry/methods_reg.rs (2)
77-79: Good addition: wire PaymentChannel into the registry.Importing
payment_channelalongside the other actors is correct and keeps the registration site cohesive.
101-103: Registration path for PaymentChannel looks correct.Dispatching to
payment_channel::register_actor_methods(self, cid, version)mirrors other actors and should cover v8–v16.src/tool/subcommands/api_cmd/api_compare_tests.rs (2)
1904-1905: Nice: extend StateDecodeParams coverage with Paych.Hooking Paych tests into the main suite is aligned with the PR goal.
1947-1952: Verified: thepaych_paramsserialization maps emptymergestoNone→ JSONnull. TheRpcTest::identitytest won’t catch this at JSON level. Switching toRpcTest::validate_rawis appropriate to lock in the invariant.
Co-authored-by: hanabi1224 <harlowmoo@gmail.com>
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: 0
♻️ Duplicate comments (1)
src/rpc/registry/actors/payment_channel.rs (1)
33-45: Optional: centralize supported versions to avoid driftThe literal list 8..16 is repeated across actor registries. Consider a shared SUPPORTED_PAYCH_VERSIONS (or macro alias) reused here to avoid future typos when bumping versions.
🧹 Nitpick comments (2)
src/tool/subcommands/api_cmd/api_compare_tests.rs (2)
1945-1953: Avoid unwrap in tests; propagate errors insteadReplace unwrap() with ? to keep failure context and avoid panics during test construction.
- to_vec(&constructor_params).unwrap(), + to_vec(&constructor_params)?, ... - to_vec(&update_channel_state).unwrap(), + to_vec(&update_channel_state)?,
1910-1968: Optional: add one legacy-version caseConsider adding a single v8 UpdateChannelState case to detect version-boundary regressions early (since registry supports v8–v16).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
src/lotus_json/actor_states/methods/mod.rs(1 hunks)src/rpc/registry/actors/payment_channel.rs(1 hunks)src/tool/subcommands/api_cmd/api_compare_tests.rs(1 hunks)src/tool/subcommands/api_cmd/test_snapshots.txt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/lotus_json/actor_states/methods/mod.rs
- src/tool/subcommands/api_cmd/test_snapshots.txt
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: akaladarshi
PR: ChainSafe/forest#5996
File: src/lotus_json/actor_states/methods/paych_params.rs:269-277
Timestamp: 2025-09-02T08:46:04.916Z
Learning: In payment channel actor Lotus JSON serialization, empty merges should be serialized as `null` (not an empty array `[]`) to maintain compatibility with Lotus behavior.
🧬 Code graph analysis (1)
src/tool/subcommands/api_cmd/api_compare_tests.rs (2)
src/shim/address.rs (2)
new_id(142-144)default(122-124)src/blocks/tipset.rs (2)
key(336-339)key(530-533)
⏰ 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)
- GitHub Check: Build MacOS
- GitHub Check: cargo-publish-dry-run
- GitHub Check: Build Ubuntu
- GitHub Check: tests
- GitHub Check: tests-release
- GitHub Check: Build forest binaries on Linux AMD64
- GitHub Check: All lint checks
- GitHub Check: Analyze (rust)
- GitHub Check: Analyze (go)
🔇 Additional comments (3)
src/rpc/registry/actors/payment_channel.rs (1)
8-31: Macro-based per-version registration looks goodUsing paste to bind Method/params per version keeps the mapping tight and prevents 8=>v9 style mismatches. LGTM.
src/tool/subcommands/api_cmd/api_compare_tests.rs (2)
1905-1906: Good call adding Paych decode-param tests to the suiteThis wires the new actor into the existing StateDecodeParams tests. LGTM.
1915-1939: Verify empty merges serialize to null (Lotus parity)Per our learning for Paych Lotus JSON, empty merges must serialize as null, not []. Your test uses merges: vec![]; please confirm paych_params.rs implements this mapping (it should around lines 269–277).
Summary of changes
Changes introduced in this pull request:
Reference issue to close (if applicable)
Closes #5992
Other information and links
Change checklist
Summary by CodeRabbit
New Features
Tests