feat: Starknet inspector by netrome · Pull Request #2084 · near/mpc

netrome · 2026-02-13T14:20:18Z

closes #2054

netrome · 2026-02-13T14:22:05Z

crates/contract-interface/src/types/foreign_chain.rs

+pub enum StarknetFinality {
+    AcceptedOnL2,
+    AcceptedOnL1,
+}


Starknet has a few more statuses we could support (see https://docs.starknet.io/learn/protocol/transactions#transaction-statuses), but I think this is a sensible start.

netrome · 2026-02-13T18:15:04Z

crates/foreign-chain-inspector/src/starknet/inspector.rs

+where
+    Client: ClientT + Send,


These trait bounds are redundant, and should be removed. But this mirrors how it's done in the other extractors so I prefer keeping it consistent for now.

netrome · 2026-02-13T18:23:37Z

crates/primitives/src/hash.rs

    }

+    #[test]
+    fn test_as_hex_with_prefix() {


While I'd normally push for the new naming convention here (e.g. something like as_hex_with_prefix__should_result_in_prefixed_hex), this would stand out a bit too much in this test module - and this test is trivial so I chose to keep it this way for now.

netrome · 2026-02-13T21:04:54Z

crates/node/src/config/foreign_chains/starknet.rs

This config update is relatively stand-alone and could be broken out, but it felt nice to add it since it enables this to be verified end to end.

netrome · 2026-02-13T21:08:51Z

crates/node/src/providers/verify_foreign_tx/sign.rs

+                    })
+                    .collect::<anyhow::Result<_>>()?
+            }
+            dtos::ForeignChainRpcRequest::Starknet(request) => {


Similar to the config change, this wraps up the feature end to end so it's nice to add but is separate from the inspector.

claude · 2026-02-13T21:11:31Z

Code Review

Found several critical issues that need fixing before merge:

1. ❌ Unsafe Wildcard Pattern Matching (Production Safety)

Location: crates/node/src/providers/verify_foreign_tx/sign.rs:185-188

let finality = match request.finality {
    dtos::StarknetFinality::AcceptedOnL2 => StarknetFinality::AcceptedOnL2,
    dtos::StarknetFinality::AcceptedOnL1 => StarknetFinality::AcceptedOnL1,
    _ => bail\!("unknown starknet finality level"),  // ❌ Dangerous wildcard
};

Problem: StarknetFinality is marked #[non_exhaustive]. Using a wildcard pattern means new variants added to the contract interface will compile but fail at runtime. This is dangerous during rolling updates in multi-cluster deployments.

Fix: Remove the wildcard and match exhaustively:

let finality = match request.finality {
    dtos::StarknetFinality::AcceptedOnL2 => StarknetFinality::AcceptedOnL2,
    dtos::StarknetFinality::AcceptedOnL1 => StarknetFinality::AcceptedOnL1,
};

The compiler will force you to update this code when new variants are added.

2. ❌ Same Issue with Extractor Matching

Location: crates/node/src/providers/verify_foreign_tx/sign.rs:190-197

.map(|extractor| match extractor {
    dtos::StarknetExtractor::BlockHash => Ok(StarknetExtractor::BlockHash),
    _ => bail\!("unknown extractor found"),  // ❌ Same problem
})

StarknetExtractor is also #[non_exhaustive]. Remove the wildcard pattern.

3. ⚠️ Unnecessary Clone in Hot Path (Performance)

Location: crates/node/src/providers/verify_foreign_tx/sign.rs:176

let rpc_url = starknet_provider_config.rpc_url.clone();

This clones the entire URL string unnecessarily. Use the rpc_url() method instead which returns Cow<'_, str>:

let http_client = foreign_chain_inspector::build_http_client(
    starknet_provider_config.rpc_url().into_owned(),
    starknet_provider_config.auth.clone().try_into()?,
)?;

4. ℹ️ Missing Execution Status Differentiation

Location: crates/foreign-chain-inspector/src/starknet/inspector.rs:48-50

The current check treats Reverted and Rejected execution statuses the same way:

if rpc_response.execution_status \!= StarknetExecutionStatus::Succeeded {
    return Err(ForeignChainInspectionError::TransactionFailed);
}

Consider adding logging to distinguish these cases for better debugging (Rejected = pre-execution failure, Reverted = execution failure).

Priority: Items #1 and #2 are critical and must be fixed before merge to prevent runtime failures during version upgrades.

⚠️

netrome · 2026-02-13T21:23:51Z

Sorry Claude, but this wasn't your best review. 1. and 2. are needed since the enums are non-exhaustive as you already noted. Also the existing bitcoin logic follows the same pattern. 3. is wrong since this is a string and 4. isn't really important right now.

DSharifi · 2026-02-14T16:54:44Z

crates/foreign-chain-rpc-interfaces/src/starknet.rs

+fn deserialize_starknet_felt_hash<'de, D>(
+    deserializer: D,
+) -> Result<TransportStarknetBlockHash, D::Error>
+where
+    D: Deserializer<'de>,
+{
+    let hash = String::deserialize(deserializer)?;
+    let stripped = hash.strip_prefix("0x").unwrap_or(&hash);
+
+    if stripped.len() > 64 {
+        return Err(serde::de::Error::custom(format!(
+            "felt hex string too long: {hash}"
+        )));
+    }
+
+    let padded = format!("{stripped:0>64}");
+    padded.parse().map_err(|error| {
+        serde::de::Error::custom(format!("invalid starknet felt hash {hash}: {error}"))
+    })
+}


Seems like the starknet response schema is the same as EVM/Ethereum by prefixing with 0x. You can reuse the ethereum_types::H256 crate to avoid these manual serialization implementations.

Updated in 4c13d4a - unfortunately starknet allows stripped-prefix hashes so we still have some custom deserialization logic. But it's cleaner still.

Another alternative would be to drag in some starknet-specific dependency for this but that feels overkill for just hex deserialization.

DSharifi

Mostly looks good.

But I want to discuss if it makes sense to implement the manual (de)serialization of hex strings with 0x prefix.

They (starknet) are reusing a lot of the same format from EVM from their official spec, such as this 0x prefixing and the name of the methods https://github.com/starkware-libs/starknet-specs/blob/master/starknet_vs_ethereum_node_apis.md

In that case, I think we should reuse the ethereum_types crates we already depend on from parity which provides all this (de)serialization logic built in.

gilcu3

Thank you!

Requesting changes because we have a problem with the manual test:

        FAIL [   0.105s] foreign-chain-inspector::starknet_rpc_manual inspector_extracts_block_hash_against_live_rpc_provider
  stdout ───

    running 1 test
    test inspector_extracts_block_hash_against_live_rpc_provider ... FAILED

    failures:

    failures:
        inspector_extracts_block_hash_against_live_rpc_provider

    test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.10s

  stderr ───

    thread 'inspector_extracts_block_hash_against_live_rpc_provider' panicked at crates/foreign-chain-inspector/tests/starknet_rpc_manual.rs:26:10:
    latest block should be fetched: ParseError(Error("invalid length 63, expected a (both 0x-prefixed or not) hex string or byte array containing 32 bytes", line: 1, column: 107))
    note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

gilcu3 · 2026-02-16T07:06:16Z

crates/foreign-chain-inspector/src/starknet.rs

@@ -0,0 +1,17 @@
+use mpc_primitives::hash::Hash32;


I really have a hard time figuring when a type should come from one crate or another. In this case I naively expected this to come from the contract-interface crate, but it comes from mpc-primitives instead. Do we have a well-defined boundary between these two crates? I am all in for merging them if not

Yeah I think this isn't completely obvious for some cases, here's how I'd do it:

Complex types used in the contract interface -> contract-interface
Primitive types used for things unrelated to the interface -> mpc-primitives
Primitive types only used for the interface -> contract-interface

I think the last one is the most interesting case, because the two above are fairly obvious. There are crates that may need some primitives that don't rely on the contract interface and those primitives should obviously live in mpc-primitives. And anything that's not a primitive should not live there.

For the last case, both would work so I have no strong opinion but here it's better to maintain locality if the primitive isn't expected to be used anywhere else.

Thanks for the detailed explanation. What is a "primitive" type in this context? Aren't we using it as a synonym of simply "mostly non-interface type"? What is the difference between the crate node-types and mpc-primitives ? I guess that node-types is only about the node, but then why would it be outside of the node?

netrome · 2026-02-16T07:20:18Z

Thank you!

Requesting changes because we have a problem with the manual test:

        FAIL [   0.105s] foreign-chain-inspector::starknet_rpc_manual inspector_extracts_block_hash_against_live_rpc_provider
  stdout ───

    running 1 test
    test inspector_extracts_block_hash_against_live_rpc_provider ... FAILED

    failures:

    failures:
        inspector_extracts_block_hash_against_live_rpc_provider

    test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.10s

  stderr ───

    thread 'inspector_extracts_block_hash_against_live_rpc_provider' panicked at crates/foreign-chain-inspector/tests/starknet_rpc_manual.rs:26:10:
    latest block should be fetched: ParseError(Error("invalid length 63, expected a (both 0x-prefixed or not) hex string or byte array containing 32 bytes", line: 1, column: 107))
    note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Thanks for the observation. This passed before, so it has to do with the hash type change probably. I'll fix it.

netrome linked an issue Feb 13, 2026 that may be closed by this pull request

foreign chain inspector for Starknet #2054

Closed

netrome commented Feb 13, 2026

View reviewed changes

netrome force-pushed the 2054-foreign-chain-inspector-for-starknet branch from f7d42a3 to c26c10d Compare February 13, 2026 14:47

netrome commented Feb 13, 2026

View reviewed changes

netrome marked this pull request as ready for review February 13, 2026 21:09

netrome force-pushed the 2054-foreign-chain-inspector-for-starknet branch from c22269d to 9898996 Compare February 13, 2026 21:10

netrome self-assigned this Feb 13, 2026

netrome requested review from DSharifi and gilcu3 February 13, 2026 21:25

DSharifi reviewed Feb 14, 2026

View reviewed changes

DSharifi requested changes Feb 14, 2026

View reviewed changes

netrome force-pushed the 2054-foreign-chain-inspector-for-starknet branch from 9106a16 to 4c13d4a Compare February 14, 2026 21:54

netrome requested a review from DSharifi February 14, 2026 22:07

gilcu3 requested changes Feb 16, 2026

View reviewed changes

feat: Starknet inspector

504af02

netrome force-pushed the 2054-foreign-chain-inspector-for-starknet branch from a5f59aa to 504af02 Compare February 16, 2026 08:07

fix: Bump vote update gas

d2b3c81

gilcu3 approved these changes Feb 16, 2026

View reviewed changes

DSharifi approved these changes Feb 16, 2026

View reviewed changes

netrome added this pull request to the merge queue Feb 16, 2026

Merged via the queue into main with commit 026de4a Feb 16, 2026
9 checks passed

netrome deleted the 2054-foreign-chain-inspector-for-starknet branch February 16, 2026 12:56

Conversation

netrome commented Feb 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

claude bot commented Feb 13, 2026

Code Review

1. ❌ Unsafe Wildcard Pattern Matching (Production Safety)

2. ❌ Same Issue with Extractor Matching

3. ⚠️ Unnecessary Clone in Hot Path (Performance)

4. ℹ️ Missing Execution Status Differentiation

Uh oh!

netrome commented Feb 13, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DSharifi left a comment

Choose a reason for hiding this comment

Uh oh!

gilcu3 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

netrome commented Feb 16, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

netrome commented Feb 13, 2026 •

edited

Loading