crates/starknet-devnet-types/src/rpc/transaction_receipt.rs (3)

24-30: Harden deserialization: deny unknown fields on DeployAccountTransactionReceipt.

For parity with CommonTransactionReceipt and to catch future wire-shape drift during tests, add serde(deny_unknown_fields) (gated by testing) here as well.

-#[derive(Debug, Clone, Serialize)]
-#[cfg_attr(feature = "testing", derive(serde::Deserialize))]
+#[derive(Debug, Clone, Serialize)]
+#[cfg_attr(feature = "testing", derive(serde::Deserialize), serde(deny_unknown_fields))]
 pub struct DeployAccountTransactionReceipt {

---

14-22: Optional: consider making the enum deserialization explicit in tests.

If you keep #[serde(untagged)], you can still avoid ambiguity in tests by providing a custom Deserialize (only under testing) that dispatches by the flattened "type" field:

// This snippet is additional code (not a diff) and should replace the derived Deserialize.
// Guard under `#[cfg(feature = "testing")]`.

#[cfg(feature = "testing")]
impl<'de> serde::Deserialize<'de> for TransactionReceipt {
    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
    where
        D: serde::Deserializer<'de>,
    {
        use serde::de::Error as _;
        let v = serde_json::Value::deserialize(deserializer)?;
        let ty = v.get("type").and_then(|t| t.as_str())
            .ok_or_else(|| D::Error::custom("missing `type` field on receipt"))?;
        match ty {
            "DEPLOY_ACCOUNT" => {
                Ok(TransactionReceipt::DeployAccount(
                    serde_json::from_value(v).map_err(D::Error::custom)?
                ))
            }
            "DEPLOY" => {
                Ok(TransactionReceipt::Deploy(
                    serde_json::from_value(v).map_err(D::Error::custom)?
                ))
            }
            "INVOKE" => {
                Ok(TransactionReceipt::Common(
                    serde_json::from_value(v).map_err(D::Error::custom)?
                ))
            }
            "L1_HANDLER" => {
                Ok(TransactionReceipt::L1Handler(
                    serde_json::from_value(v).map_err(D::Error::custom)?
                ))
            }
            _ => Err(D::Error::custom(format!("unknown receipt type `{ty}`"))),
        }
    }
}

Also applies to: 73-89

---

18-22: Receipt update is correct; untagged-deserialize risk is low but consider a safer impl

A quick search found no direct calls to serde_json::from_*::<TransactionReceipt> in the repo, so this enum isn’t currently deserialized anywhere (tests/benches/examples won’t hit the ambiguity). Nevertheless, because both DeployAccountTransactionReceipt and DeployTransactionReceipt flatten CommonTransactionReceipt (including contract_address), the #[serde(untagged)] enum remains order-dependent and could mis-deserialize if used in the future.

• Location:

• crates/starknet-devnet-types/src/rpc/transaction_receipt.rs (the TransactionReceipt enum with #[serde(untagged)])

• Optional refactor: under the testing feature, drop the derived Deserialize and add a custom impl keyed on the "type" field, e.g.:

// Keep Serialize (for emitting), but only derive Deserialize when needed.
#[derive(Serialize)]
#[cfg_attr(feature = "testing", derive(Deserialize))]
#[serde(untagged)]
pub enum TransactionReceipt { /* … */ }

// Alternatively, implement Deserialize manually under `testing`:
#[cfg(feature = "testing")]
impl<'de> Deserialize<'de> for TransactionReceipt {
    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
    where
        D: Deserializer<'de>,
    {
        let val = serde_json::Value::deserialize(deserializer)?;
        let ty = val.get("type").and_then(|v| v.as_str())
            .ok_or_else(|| D::Error::custom("missing `type` field in receipt"))?;
        match ty {
            "DEPLOY_ACCOUNT" => DeployAccountTransactionReceipt::deserialize(val)
                .map(TransactionReceipt::DeployAccount)
                .map_err(D::Error::custom),
            "DEPLOY" => DeployTransactionReceipt::deserialize(val)
                .map(TransactionReceipt::Deploy)
                .map_err(D::Error::custom),
            "L1_HANDLER" => L1HandlerTransactionReceipt::deserialize(val)
                .map(TransactionReceipt::L1Handler)
                .map_err(D::Error::custom),
            "COMMON" => CommonTransactionReceipt::deserialize(val)
                .map(TransactionReceipt::Common)
                .map_err(D::Error::custom),
            other => Err(D::Error::custom(format!("unknown receipt type `{}`", other))),
        }
    }
}

Feel free to drop the Deserialize derive entirely if it’s only ever used in tests.

tests/integration/test_subscription_to_new_tx_receipts.rs (1)

95-105: Minor: avoid moving sockets unnecessarily in the loop.

The pattern for (mut ws, subscription_id) in [(explicit_ws, ...), (implicit_ws, ...)] moves both sockets, which is fine here but slightly obscures ownership. Two explicit blocks (or a small helper) can be clearer when extending this test.

tests/integration/get_transaction_receipt_by_hash.rs (2)

98-122: Event assertions: prefer matching by identity over positional indices for robustness.

The test currently asserts events[0] is the UDC ContractDeployed and events[1] is the STRK Transfer. While this is true today, future fee-model or event-order changes could reorder events and produce brittle failures.

Suggest locating events by (from_address, key) pairs and then asserting their payloads.

-            assert_eq!(receipt.events.len(), 2);
-            let deployment_event = &receipt.events[0];
-            assert_eq!(deployment_event.from_address, UDC_LEGACY_CONTRACT_ADDRESS);
-            assert_eq!(
-                deployment_event.keys,
-                vec![get_selector_from_name("ContractDeployed").unwrap()]
-            );
-            assert_eq!(deployment_event.data[0], expected_contract_address);
-            assert_eq!(deployment_event.data[1], account_address);
-
-            let fee_charge_event = &receipt.events[1];
-            assert_eq!(fee_charge_event.from_address, STRK_ERC20_CONTRACT_ADDRESS);
-            assert_eq!(fee_charge_event.keys, vec![get_selector_from_name("Transfer").unwrap()]);
-            assert_eq!(fee_charge_event.data[0], account_address);
+            // UDC ContractDeployed
+            let udc_key = get_selector_from_name("ContractDeployed").unwrap();
+            let deployment_event = receipt.events.iter().find(|e|
+                e.from_address == UDC_LEGACY_CONTRACT_ADDRESS && e.keys == vec![udc_key]
+            ).expect("missing UDC ContractDeployed event");
+            assert_eq!(deployment_event.data[0], expected_contract_address);
+            assert_eq!(deployment_event.data[1], account_address);
+
+            // STRK fee transfer
+            let transfer_key = get_selector_from_name("Transfer").unwrap();
+            let fee_charge_event = receipt.events.iter().find(|e|
+                e.from_address == STRK_ERC20_CONTRACT_ADDRESS && e.keys == vec![transfer_key]
+            ).expect("missing STRK Transfer event");
+            assert_eq!(fee_charge_event.data[0], account_address);

---

219-221: Make gas-underfunding deterministic without hardcoding.

Use the estimated l2_gas_consumed - 1 rather than a magic number to reduce flakiness across versions/backends.

-        // .l2_gas(u64::try_from(fee.l2_gas_consumed).unwrap() - 1) // subtract to induce failure
-        .l2_gas(1156800 - 1) // TODO: determined experimentally as the actual value used minus 1
+        .l2_gas(u64::try_from(fee.l2_gas_consumed).unwrap() - 1) // subtract to induce failure

Configuration used: Path: .coderabbit.yaml

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.