src/rpc/registry/actors/evm.rs (1)

29-31: Pending: Add GetStorageAt when implementing it

You’ve registered GetBytecode and GetBytecodeHash (empty params). Once GetStorageAt lands, remember to register it here with its correct params type.

I can draft the registration snippet and tests when you pick up GetStorageAt. Want me to open a follow-up issue/PR checklist for the remaining EVM methods?

src/lotus_json/actor_states/methods/evm_actor_params.rs (1)

7-11: Prefer serde::Serialize import for clarity and consistency

Avoid importing Serialize from jsonrpsee. Derive macros expand to serde::Serialize anyway; importing from serde is clearer and consistent with Deserialize.

Apply:

-use jsonrpsee::core::Serialize;
-use paste::paste;
-use schemars::JsonSchema;
-use serde::Deserialize;
+use paste::paste;
+use schemars::JsonSchema;
+use serde::{Deserialize, Serialize};

src/tool/subcommands/api_cmd/api_compare_tests.rs (2)

2182-2219: DRY: Reuse parsed EVM address to avoid repeated parsing

Minor cleanup to avoid multiple Address::from_str calls.

Apply:

 fn evm_actor_state_decode_params_tests(tipset: &Tipset) -> anyhow::Result<Vec<RpcTest>> {
+    let evm_address = Address::from_str(EVM_ADDRESS).unwrap();
@@
-        RpcTest::identity(StateDecodeParams::request((
-            Address::from_str(EVM_ADDRESS).unwrap(),
+        RpcTest::identity(StateDecodeParams::request((
+            evm_address,
             fil_actor_evm_state::v16::Method::Constructor as u64,
             to_vec(&evm_constructor_params)?,
             tipset.key().into(),
         ))?),
         RpcTest::identity(StateDecodeParams::request((
-            Address::from_str(EVM_ADDRESS).unwrap(),
+            evm_address,
             fil_actor_evm_state::v16::Method::Resurrect as u64,
             to_vec(&evm_constructor_params)?,
             tipset.key().into(),
         ))?),
         RpcTest::identity(StateDecodeParams::request((
-            Address::from_str(EVM_ADDRESS).unwrap(),
+            evm_address,
             fil_actor_evm_state::v16::Method::GetBytecode as u64,
             vec![],
             tipset.key().into(),
         ))?),
         RpcTest::identity(StateDecodeParams::request((
-            Address::from_str(EVM_ADDRESS).unwrap(),
+            evm_address,
             fil_actor_evm_state::v16::Method::GetBytecodeHash as u64,
             vec![],
             tipset.key().into(),
         ))?),
         RpcTest::identity(StateDecodeParams::request((
-            Address::from_str(EVM_ADDRESS).unwrap(),
+            evm_address,
             fil_actor_evm_state::v16::Method::InvokeContract as u64,
             to_vec(&evm_invoke_contract_params)?,
             tipset.key().into(),
         ))?),
         RpcTest::identity(StateDecodeParams::request((
-            Address::from_str(EVM_ADDRESS).unwrap(),
+            evm_address,
             fil_actor_evm_state::v16::Method::InvokeContractDelegate as u64,
             to_vec(&evm_delegate_call_params)?,
             tipset.key().into(),
         ))?),

---

2196-2206: Note for future coverage: Add GetStorageAt once implemented

Once GetStorageAt is supported, it’d be good to add a matching StateDecodeParams test here for parity with the registry and snapshots.

I can add a placeholder test and snapshot entry when you tackle GetStorageAt. Want me to queue that up?

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 settings in your CodeRabbit configuration.

src/rpc/methods/eth/types.rs (3)

65-65: Prefer hex literal and document CBOR prefix meaning.

Make the intent explicit that 0x81 is a CBOR array-of-length-1 prefix.

-const LENGTH_BUF_GET_STORAGE_AT_PARAMS: u8 = 129;
+/// CBOR major type 4 (array) with length 1: 0x80 + 1 = 0x81
+const LENGTH_BUF_GET_STORAGE_AT_PARAMS: u8 = 0x81;

---

83-87: Avoid Vec::insert(0, ...) to prevent O(n) shifting.

Prepending by insertion at index 0 reallocates/moves the entire buffer. Build the vector with capacity and extend instead.

-    pub fn serialize_params(&self) -> anyhow::Result<Vec<u8>> {
-        let mut encoded = fvm_ipld_encoding::to_vec(&RawBytes::new(self.0.to_vec()))?;
-        encoded.insert(0, LENGTH_BUF_GET_STORAGE_AT_PARAMS);
-        Ok(encoded)
-    }
+    pub fn serialize_params(&self) -> anyhow::Result<Vec<u8>> {
+        let encoded = fvm_ipld_encoding::to_vec(&RawBytes::new(self.0.to_vec()))?;
+        let mut with_prefix = Vec::with_capacity(encoded.len() + 1);
+        with_prefix.push(LENGTH_BUF_GET_STORAGE_AT_PARAMS);
+        with_prefix.extend_from_slice(&encoded);
+        Ok(with_prefix)
+    }

---

89-96: Add a negative test for prefix mismatch and a round-trip test for deserialize_params.

Good addition. To harden it, add tests that:

• Round-trip via serialize_params -> deserialize_params.
• Fail with a clear error when the CBOR array-length prefix is not 0x81.

Example tests (place alongside existing tests in this module):

#[test]
fn get_storage_at_params_roundtrip() {
    let original = GetStorageAtParams::new(vec![0xde, 0xad, 0xbe, 0xef]).unwrap();
    let ser = original.serialize_params().unwrap();
    let de = GetStorageAtParams::deserialize_params(&ser).unwrap();
    assert_eq!(original.0, de.0);
}

#[test]
fn get_storage_at_params_bad_prefix() {
    let good = GetStorageAtParams::new(vec![0x0a]).unwrap().serialize_params().unwrap();
    let mut bad = good.clone();
    bad[0] = 0x80; // wrong CBOR array-length (0 not 1)
    let err = GetStorageAtParams::deserialize_params(&bad).unwrap_err();
    assert!(err.to_string().contains("expected CBOR array of length 1"));
}

src/lotus_json/actor_states/methods/evm_actor_params.rs (2)

131-134: Clarify size constraint for StorageKey in JSON schema/docs.

GetStorageAtParams ultimately requires a 32-byte key (left-padded if shorter). Consider documenting this on the JSON field to set expectations for clients.

For example, add a doc comment:
// Must be at most 32 bytes; shorter values are left-padded with zeros to 32 bytes.

---

155-157: Avoid panic on oversized storage_key during JSON -> params conversion.

GetStorageAtParams::new(..) errors on >32 bytes; unwrap() will panic, potentially taking down the request handler on bad input. Since HasLotusJson::from_lotus_json is infallible, defensively clamp to the last 32 bytes to avoid panics while matching the “right-aligned” behavior.

-    fn from_lotus_json(lotus_json: Self::LotusJson) -> Self {
-        GetStorageAtParams::new(lotus_json.storage_key).unwrap()
-    }
+    fn from_lotus_json(lotus_json: Self::LotusJson) -> Self {
+        // Clamp to last 32 bytes to avoid panic on oversized input;
+        // shorter inputs are left-padded by `GetStorageAtParams::new`.
+        let key = if lotus_json.storage_key.len() > 32 {
+            lotus_json.storage_key[lotus_json.storage_key.len() - 32..].to_vec()
+        } else {
+            lotus_json.storage_key
+        };
+        // `new` only fails if input > 32 bytes, which we prevent above.
+        GetStorageAtParams::new(key).expect("validated length <= 32")
+    }

If strict rejection is desired instead of clamping, the alternative is to validate earlier in the request decoding path and return a structured error (but HasLotusJson’s signature doesn’t allow that here).

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.

src/tool/subcommands/api_cmd/api_compare_tests.rs (2)

1900-1904: Add a quick CBOR equivalence check for GetStorageAt to guard against divergence

GetStorageAtParams in v16 is a tuple with a single U256 field. You’re using a bespoke serialize_params() that constructs the CBOR array header and encodes a RawBytes payload. It’s fine, but worth asserting that it equals the canonical to_vec encoding of the actor’s GetStorageAtParams to avoid subtle regressions if either side changes.

Apply this diff within the function to add a sanity assertion:

@@
-    let evm_get_storage_at_params = GetStorageAtParams::new(vec![0xa])?;
+    let evm_get_storage_at_params = GetStorageAtParams::new(vec![0xa])?;
+    // Sanity: our bespoke encoder matches the actor's canonical CBOR for v16
+    {
+        use fil_actor_evm_state::evm_shared::v16::uints::U256;
+        let mut key = [0u8; 32];
+        key[31] = 0x0a; // same storage position as above
+        let canonical = to_vec(&fil_actor_evm_state::v16::GetStorageAtParams {
+            storage_key: U256::from_big_endian(&key),
+        })?;
+        assert_eq!(
+            evm_get_storage_at_params.serialize_params()?,
+            canonical,
+            "GetStorageAt params CBOR mismatch"
+        );
+    }

Citations: v16 GetStorageAtParams { storage_key: U256 }. (docs.rs)

---

1904-1944: Nit: avoid repeated parsing of EVM address

You repeatedly call Address::from_str(EVM_ADDRESS).unwrap(). Parse once and reuse; it reduces noise and removes the unwrap() in test assembly code.

Apply this refactor inside evm_actor_state_decode_params_tests:

@@
-    let tests = vec![
+    let evm_addr = Address::from_str(EVM_ADDRESS)?;
+    let tests = vec![
         RpcTest::identity(StateDecodeParams::request((
-            Address::from_str(EVM_ADDRESS).unwrap(),
+            evm_addr,
             fil_actor_evm_state::v16::Method::Constructor as u64,
             to_vec(&evm_constructor_params)?,
             tipset.key().into(),
         ))?),
         RpcTest::identity(StateDecodeParams::request((
-            Address::from_str(EVM_ADDRESS).unwrap(),
+            evm_addr,
             fil_actor_evm_state::v16::Method::Resurrect as u64,
             to_vec(&evm_constructor_params)?,
             tipset.key().into(),
         ))?),
         RpcTest::identity(StateDecodeParams::request((
-            Address::from_str(EVM_ADDRESS).unwrap(),
+            evm_addr,
             fil_actor_evm_state::v16::Method::GetBytecode as u64,
             vec![],
             tipset.key().into(),
         ))?),
         RpcTest::identity(StateDecodeParams::request((
-            Address::from_str(EVM_ADDRESS).unwrap(),
+            evm_addr,
             fil_actor_evm_state::v16::Method::GetBytecodeHash as u64,
             vec![],
             tipset.key().into(),
         ))?),
         RpcTest::identity(StateDecodeParams::request((
-            Address::from_str(EVM_ADDRESS).unwrap(),
+            evm_addr,
             fil_actor_evm_state::v16::Method::InvokeContract as u64,
             to_vec(&evm_invoke_contract_params)?,
             tipset.key().into(),
         ))?),
         RpcTest::identity(StateDecodeParams::request((
-            Address::from_str(EVM_ADDRESS).unwrap(),
+            evm_addr,
             fil_actor_evm_state::v16::Method::InvokeContractDelegate as u64,
             to_vec(&evm_delegate_call_params)?,
             tipset.key().into(),
         ))?),
         RpcTest::identity(StateDecodeParams::request((
-            Address::from_str(EVM_ADDRESS).unwrap(),
+            evm_addr,
             fil_actor_evm_state::v16::Method::GetStorageAt as u64,
             evm_get_storage_at_params.serialize_params()?,
             tipset.key().into(),
         ))?),
     ];

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.

src/rpc/methods/eth/types.rs (1)

89-97: Import anyhow::Context; current .context(..) calls won’t compile

.context(..) is used here (and elsewhere in this file) but anyhow::Context isn’t in scope. Bring the trait into scope to fix the compile error. This was flagged earlier and still applies.

Apply this diff (outside the selected range) to the imports:

- use anyhow::ensure;
+ use anyhow::{ensure, Context};

src/rpc/methods/eth/types.rs (2)

65-65: Clarify magic number: prefer 0x81 and a short doc comment

129 is 0x81 (CBOR array length = 1). Encoding this as hex with a brief comment makes intent obvious.

-const LENGTH_BUF_GET_STORAGE_AT_PARAMS: u8 = 129;
+/// 0x81 == CBOR major type 4 (array) with length = 1
+const LENGTH_BUF_GET_STORAGE_AT_PARAMS: u8 = 0x81;

---

89-97: Make the prefix-mismatch error actionable

Including the actual prefix helps diagnose malformed inputs quickly.

-        ensure!(
-            prefix == LENGTH_BUF_GET_STORAGE_AT_PARAMS,
-            "expected CBOR array of length 1"
-        );
+        ensure!(
+            prefix == LENGTH_BUF_GET_STORAGE_AT_PARAMS,
+            "expected CBOR array of length 1 (0x81), got 0x{:02x}",
+            prefix
+        );

If useful, I can also add unit tests covering:

• EOF (empty slice) -> “unexpected EOF”
• Wrong prefix -> the updated error with the observed byte
• Valid payload -> roundtrip via serialize_params/deserialize_params
  Want me to push those tests?

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.

src/tool/subcommands/api_cmd/api_compare_tests.rs (3)

1883-1899: Param scaffolding for EVM methods looks sensible.

• ConstructorParams zero-creator and dummy initcode are fine for decode tests.
• InvokeContractParams and DelegateCallParams shapes look correct; using a default CID and zero TokenAmount is acceptable here since we only validate decoding.

If you want to reduce churn in future edits, consider hoisting common dummy values (e.g., input bytes, initcode) into small consts right above the function to de-duplicate magic literals.

---

1902-1945: DRY: parse the EVM address once and reuse it.

You repeatedly call Address::from_str(EVM_ADDRESS).unwrap() for each test case. Parse once to a local evm_addr and reuse to avoid repeated fallible parsing and improve readability.

Apply this diff inside the function:

@@
-    let tests = vec![
-        RpcTest::identity(StateDecodeParams::request((
-            Address::from_str(EVM_ADDRESS).unwrap(),
+    let evm_addr = Address::from_str(EVM_ADDRESS).expect("valid EVM actor address");
+    let tests = vec![
+        RpcTest::identity(StateDecodeParams::request((
+            evm_addr,
             fil_actor_evm_state::v16::Method::Constructor as u64,
             to_vec(&evm_constructor_params)?,
             tipset.key().into(),
         ))?),
         RpcTest::identity(StateDecodeParams::request((
-            Address::from_str(EVM_ADDRESS).unwrap(),
+            evm_addr,
             fil_actor_evm_state::v16::Method::Resurrect as u64,
             to_vec(&evm_constructor_params)?,
             tipset.key().into(),
         ))?),
         RpcTest::identity(StateDecodeParams::request((
-            Address::from_str(EVM_ADDRESS).unwrap(),
+            evm_addr,
             fil_actor_evm_state::v16::Method::GetBytecode as u64,
             vec![],
             tipset.key().into(),
         ))?),
         RpcTest::identity(StateDecodeParams::request((
-            Address::from_str(EVM_ADDRESS).unwrap(),
+            evm_addr,
             fil_actor_evm_state::v16::Method::GetBytecodeHash as u64,
             vec![],
             tipset.key().into(),
         ))?),
         RpcTest::identity(StateDecodeParams::request((
-            Address::from_str(EVM_ADDRESS).unwrap(),
+            evm_addr,
             fil_actor_evm_state::v16::Method::InvokeContract as u64,
             to_vec(&evm_invoke_contract_params)?,
             tipset.key().into(),
         ))?),
         RpcTest::identity(StateDecodeParams::request((
-            Address::from_str(EVM_ADDRESS).unwrap(),
+            evm_addr,
             fil_actor_evm_state::v16::Method::InvokeContractDelegate as u64,
             to_vec(&evm_delegate_call_params)?,
             tipset.key().into(),
         ))?),
         RpcTest::identity(StateDecodeParams::request((
-            Address::from_str(EVM_ADDRESS).unwrap(),
+            evm_addr,
             fil_actor_evm_state::v16::Method::GetStorageAt as u64,
             evm_get_storage_at_params.serialize_params()?,
             tipset.key().into(),
         ))?),
     ];

---

1883-1949: Optional: make tests resilient across protocol versions.

Given EVM methods are only available from v10 onward, you could optionally gate this block based on StateNetworkVersion from tipset and skip when < v10. Not strictly necessary for Calibnet snapshots, but it avoids accidental failures when running against older snapshots.

I can draft a small helper that fetches the network version for tipset and returns Vec<RpcTest> empty if < v10. Want me to add it?

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.