starknet_transaction_prover: add OpenRPC spec and spec validation by avi-starkware · Pull Request #12879 · starkware-libs/sequencer

avi-starkware · 2026-02-26T09:20:30Z

Note

Low Risk
Primarily adds documentation artifacts and test-only schema validation; production server behavior is unchanged aside from stricter CI coverage that could surface mismatches.

Overview
Adds a formal OpenRPC document (resources/proving_api_openrpc.json) describing the starknet_specVersion and starknet_proveTransaction methods, their schemas, and standardized error definitions.

Introduces a test-only spec validation suite (server/rpc_spec_test.rs) using jsonschema to assert request param compatibility (named + positional), response shapes from the mock RPC, and that error codes/messages in server/errors.rs match the spec (with a completeness guard).

^{Written by Cursor Bugbot for commit f0eb540. This will update automatically on new commits. Configure here.}

reviewable-StarkWare · 2026-02-26T09:20:38Z

This change is

github-actions · 2026-02-26T09:20:56Z

Artifacts upload workflows:

avivg-starkware · 2026-03-03T13:02:05Z

crates/starknet_os_runner/src/server/rpc_spec_test.rs line 175 at r3 (raw file):

#[test]
fn prove_transaction_result_matches_schema() {

How about verifying that prove_transaction matches the starknet_proveTransaction specs definition (i.e., that the params match)?

This validates the result schema.
and I see there are tests validating the params types (' BlockIdandRpcTransaction`) independently .

Code quote:

prove_transaction_result_matches_schema()

avivg-starkware · 2026-03-03T13:08:56Z

FYI @Yoni-Starkware @noaov1

To @AvivYossef-starkware's suggestion, this PR potentially replaces MockProvingRpc.
I added a comment below of a test I think is needed. If this case is addressed, I'll revert #12909.

AvivYossef-starkware

@AvivYossef-starkware reviewed 4 files and all commit messages, and made 8 comments.
Reviewable status: 4 of 5 files reviewed, 6 unresolved discussions (waiting on avi-starkware, noaov1, and Yoni-Starkware).

a discussion (no related file):

Previously, avivg-starkware wrote…

FYI @Yoni-Starkware @noaov1

To @AvivYossef-starkware's suggestion, this PR potentially replaces MockProvingRpc.
I added a comment below of a test I think is needed. If this case is addressed, I'll revert #12909.

@avi-starkware, plz try to make it more readable and add comments

crates/starknet_os_runner/resources/proving_api_openrpc.json line 1 at r3 (raw file):

How did you create this file?

crates/starknet_os_runner/src/server.rs line 8 at r3 (raw file):

#[cfg(test)]
#[path = "server/rpc_spec_test.rs"]

?

Code quote:

[path = "server/rpc_spec_test.rs"]

crates/starknet_os_runner/src/server/rpc_spec_test.rs line 27 at r3 (raw file):

fn load_spec() -> Value {
    serde_json::from_str(OPENRPC_SPEC).expect("OpenRPC spec must be valid JSON")
}

Why don't you use you load_respurce_file method?

Code quote:

/// Embedded OpenRPC specification document.
const OPENRPC_SPEC: &str = include_str!("../../resources/proving_api_openrpc.json");

fn load_spec() -> Value {
    serde_json::from_str(OPENRPC_SPEC).expect("OpenRPC spec must be valid JSON")
}

crates/starknet_os_runner/src/server/rpc_spec_test.rs line 27 at r3 (raw file):

fn load_spec() -> Value {
    serde_json::from_str(OPENRPC_SPEC).expect("OpenRPC spec must be valid JSON")
}

use LaztLock to load it on

Code quote:

fn load_spec() -> Value {
    serde_json::from_str(OPENRPC_SPEC).expect("OpenRPC spec must be valid JSON")
}

crates/starknet_os_runner/src/server/rpc_spec_test.rs line 78 at r3 (raw file):

        obj.insert("required".to_string(), required.into());
    }
}

lets make it more readable
consider something like
( just make the test

struct SpecError {
        code: i32,
        message: String,
        expects_data: bool,
    }

    impl SpecError {
        fn from_spec(value: &Value) -> Self {
            Self {
                code: value["code"].as_i64().unwrap() as i32,
                message: value["message"].as_str().unwrap().to_string(),
                expects_data: value.get("data").is_some(),
            }
        }

        fn assert_matches(&self, error: &ErrorObjectOwned) {
            assert_eq!(error.code(), self.code);
            assert_eq!(error.message(), self.message);
            assert_eq!(error.data().is_some(), self.expects_data);
        }
    }

    #[rstest]
    #[case("BLOCK_NOT_FOUND",           error::block_not_found())]
    #[case("ACCOUNT_VALIDATION_FAILED", error::validation_failure("test".to_string()))]
    #[case("UNSUPPORTED_TX_VERSION",    error::unsupported_tx_version("v99".to_string()))]
    #[case("SERVICE_BUSY",              error::service_busy(2))]
    fn error_responses_match_spec(
        #[case] spec_key: &str,
        #[case] actual: ErrorObjectOwned,
    ) {
        let spec = load_spec();
        SpecError::from_spec(&spec["components"]["errors"][spec_key])
            .assert_matches(&actual);
    }

Code quote:

/// Transforms spec error definitions into JSON Schemas that can validate error responses.
///
/// Adapted from `apollo_rpc::test_utils::fix_errors`.
fn fix_errors(spec: &mut Value) {
    let Some(errors) = spec
        .as_object_mut()
        .and_then(|obj| obj.get_mut("components"))
        .and_then(|components| components.as_object_mut())
        .and_then(|components| components.get_mut("errors"))
        .and_then(|errors| errors.as_object_mut())
    else {
        return;
    };
    for value in errors.values_mut() {
        let obj = value.as_object_mut().unwrap();
        let Some(code) = obj.get("code").cloned() else {
            continue;
        };
        let Some(message) = obj.get("message").cloned() else {
            continue;
        };
        let has_data = obj.contains_key("data");
        obj.clear();
        let mut properties = serde_json::Map::from_iter([
            (
                "code".to_string(),
                serde_json::Map::from_iter([
                    ("type".to_string(), "integer".into()),
                    ("enum".to_string(), vec![code].into()),
                ])
                .into(),
            ),
            (
                "message".to_string(),
                serde_json::Map::from_iter([
                    ("type".to_string(), "string".into()),
                    ("enum".to_string(), vec![message].into()),
                ])
                .into(),
            ),
        ]);
        let mut required: Vec<Value> = vec!["code".into(), "message".into()];
        if has_data {
            properties.insert("data".to_string(), serde_json::Map::from_iter([]).into());
            required.push("data".into());
        }
        obj.insert("properties".to_string(), properties.into());
        obj.insert("required".to_string(), required.into());
    }
}

crates/starknet_os_runner/src/server/rpc_spec_test.rs line 269 at r3 (raw file):

         {}\nErrors: {:?}",
        serde_json::to_string_pretty(&serialized).unwrap(),
        tx_schema.validate(&serialized).err().map(|e| e.collect::<Vec<_>>()),

Share something like it with all the tests

 fn assert_matches_schema(schema: &JSONSchema, value: &Value, label: &str) {
      assert!(
          schema.is_valid(value),
          "{label} does not match spec schema.\nSerialized: {}\nErrors: {:?}",
          serde_json::to_string_pretty(value).unwrap(),
          schema.validate(value).err().map(|e| e.collect::<Vec<_>>()),
      );
  }

Code quote:

    assert!(
        tx_schema.is_valid(&serialized),
        "Serialized RpcTransaction does not match RPC_TRANSACTION schema.\nSerialized: \
         {}\nErrors: {:?}",
        serde_json::to_string_pretty(&serialized).unwrap(),
        tx_schema.validate(&serialized).err().map(|e| e.collect::<Vec<_>>()),

crates/starknet_os_runner/src/server/rpc_spec_test.rs line 292 at r3 (raw file):

            block_id_schema.validate(&serialized).err().map(|e| e.collect::<Vec<_>>()),
        );
    }

Suggestion:

  #[rstest]
  #[case("hash", BlockId::Hash(BlockHash(felt!("0x123"))))]
  #[case("number", BlockId::Number(BlockNumber(42)))]
  #[case("latest", BlockId::Latest)]
  fn serialized_block_id_matches_schema(#[case] label: &str, #[case] block_id: BlockId) {
      let schema = compile_component_schema("BLOCK_ID");
      assert_matches_schema(&schema, &serde_json::to_value(block_id).unwrap(), label);
  }

avi-starkware

@avi-starkware made 5 comments.
Reviewable status: 1 of 5 files reviewed, 6 unresolved discussions (waiting on avivg-starkware, AvivYossef-starkware, noaov1, and Yoni-Starkware).

crates/starknet_os_runner/src/server/rpc_spec_test.rs line 27 at r3 (raw file):

Previously, AvivYossef-starkware wrote…

Why don't you use you load_respurce_file method?

I used read_json_file instead.

crates/starknet_os_runner/src/server/rpc_spec_test.rs line 27 at r3 (raw file):

Previously, AvivYossef-starkware wrote…

use LaztLock to load it on

Done.

crates/starknet_os_runner/src/server/rpc_spec_test.rs line 78 at r3 (raw file):

Previously, AvivYossef-starkware wrote…

lets make it more readable
consider something like
( just make the test

struct SpecError {
        code: i32,
        message: String,
        expects_data: bool,
    }

    impl SpecError {
        fn from_spec(value: &Value) -> Self {
            Self {
                code: value["code"].as_i64().unwrap() as i32,
                message: value["message"].as_str().unwrap().to_string(),
                expects_data: value.get("data").is_some(),
            }
        }

        fn assert_matches(&self, error: &ErrorObjectOwned) {
            assert_eq!(error.code(), self.code);
            assert_eq!(error.message(), self.message);
            assert_eq!(error.data().is_some(), self.expects_data);
        }
    }

    #[rstest]
    #[case("BLOCK_NOT_FOUND",           error::block_not_found())]
    #[case("ACCOUNT_VALIDATION_FAILED", error::validation_failure("test".to_string()))]
    #[case("UNSUPPORTED_TX_VERSION",    error::unsupported_tx_version("v99".to_string()))]
    #[case("SERVICE_BUSY",              error::service_busy(2))]
    fn error_responses_match_spec(
        #[case] spec_key: &str,
        #[case] actual: ErrorObjectOwned,
    ) {
        let spec = load_spec();
        SpecError::from_spec(&spec["components"]["errors"][spec_key])
            .assert_matches(&actual);
    }

Done.

crates/starknet_os_runner/src/server/rpc_spec_test.rs line 175 at r3 (raw file):

Previously, avivg-starkware wrote…

How about verifying that prove_transaction matches the starknet_proveTransaction specs definition (i.e., that the params match)?

This validates the result schema.
and I see there are tests validating the params types (' BlockIdandRpcTransaction`) independently .

Done.

I added a test that actually tries to send a request (with stucture based on the to the JSON-RPC module and makes sure it doesn't get an `invalid params

crates/starknet_os_runner/src/server/rpc_spec_test.rs line 292 at r3 (raw file):

            block_id_schema.validate(&serialized).err().map(|e| e.collect::<Vec<_>>()),
        );
    }

Done.

avi-starkware

@avi-starkware made 1 comment.
Reviewable status: 1 of 5 files reviewed, 6 unresolved discussions (waiting on avivg-starkware, AvivYossef-starkware, noaov1, and Yoni-Starkware).

crates/starknet_os_runner/src/server/rpc_spec_test.rs line 269 at r3 (raw file):

Previously, AvivYossef-starkware wrote…

Share something like it with all the tests

 fn assert_matches_schema(schema: &JSONSchema, value: &Value, label: &str) {
      assert!(
          schema.is_valid(value),
          "{label} does not match spec schema.\nSerialized: {}\nErrors: {:?}",
          serde_json::to_string_pretty(value).unwrap(),
          schema.validate(value).err().map(|e| e.collect::<Vec<_>>()),
      );
  }

Done.

AvivYossef-starkware

@AvivYossef-starkware reviewed 4 files and all commit messages, made 5 comments, and resolved 5 discussions.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on avi-starkware, avivg-starkware, noaov1, and Yoni-Starkware).

crates/starknet_os_runner/src/server/rpc_spec_test.rs line 76 at r4 (raw file):

    let rpc_impl = ProvingRpcServerImpl::new(RpcVirtualSnosProver::new(&config), 2);
    rpc_impl.into_rpc()
}

And better to be a fixture

Suggestion:

fn rpc_module() -> RpcModule<ProvingRpcServerImpl> {
    let config =
        ProverConfig { rpc_node_url: "http://localhost:1".to_string(), ..Default::default() };
    let rpc_impl = ProvingRpcServerImpl::new(RpcVirtualSnosProver::new(&config), 2);
    rpc_impl.into_rpc()
}

crates/starknet_os_runner/src/server/rpc_spec_test.rs line 177 at r4 (raw file):

            assert_ne!(
                code,
                -32602,

plz define it as a const and document it
(Maybe you can import it from JSON-RPCSE? )

Code quote:

-32602

crates/starknet_os_runner/src/server/rpc_spec_test.rs line 202 at r4 (raw file):

        (
            PROVE_TRANSACTION_METHOD,
            serde_json::to_value(sample_prove_transaction_result()).unwrap(),

It won't catch an issue if someone changes the return type of starknet_prove_transaction ( but it may be unlikely? )

Code quote:

serde_json::to_value(sample_prove_transaction_result()).unwrap()

crates/starknet_os_runner/src/server/rpc_spec_test.rs line 223 at r4 (raw file):

fn error_responses_match_spec(#[case] spec_key: &str, #[case] actual: ErrorObjectOwned) {
    SpecError::from_spec(&SPEC["components"]["errors"][spec_key]).assert_matches(&actual);
}

What if we add more errors?

Code quote:

#[rstest]
#[case("BLOCK_NOT_FOUND", error::block_not_found())]
#[case("ACCOUNT_VALIDATION_FAILED", error::validation_failure("test".to_string()))]
#[case("UNSUPPORTED_TX_VERSION", error::unsupported_tx_version("v99".to_string()))]
#[case("SERVICE_BUSY", error::service_busy(2))]
fn error_responses_match_spec(#[case] spec_key: &str, #[case] actual: ErrorObjectOwned) {
    SpecError::from_spec(&SPEC["components"]["errors"][spec_key]).assert_matches(&actual);
}

crates/starknet_os_runner/src/server/rpc_spec_test.rs line 231 at r4 (raw file):

fn serialized_block_id_matches_schema(#[case] label: &str, #[case] block_id: BlockId) {
    let schema = compile_schema_for_ref(&SPEC, &format!("components/schemas/BLOCK_ID"));
    assert_matches_schema(&schema, &serde_json::to_value(block_id).unwrap(), label);

Why do you specifically test BlockId?

Code quote:

#[rstest]
#[case("hash", BlockId::Hash(BlockHash(felt!("0x123"))))]
#[case("number", BlockId::Number(BlockNumber(42)))]
#[case("latest", BlockId::Latest)]
fn serialized_block_id_matches_schema(#[case] label: &str, #[case] block_id: BlockId) {
    let schema = compile_schema_for_ref(&SPEC, &format!("components/schemas/BLOCK_ID"));
    assert_matches_schema(&schema, &serde_json::to_value(block_id).unwrap(), label);

avivg-starkware

@avivg-starkware made 1 comment.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on avi-starkware, noaov1, and Yoni-Starkware).

crates/starknet_os_runner/src/server/rpc_spec_test.rs line 185 at r5 (raw file):

        }
    }
}

The test mixes object construction with the test logic, making it hard to tell what's being tested. I'd suggest extracting a build_method_test_cases helper that creates and validates the test data, so the test body is just as follows.

Also, we currently only test named params ("params": {…}). We should test positional params ("params": […]) too.

I sketched out the helpers below as a general direction.

Suggestion (i):

/// Validates that the spec's parameter definitions match the actual RPC implementation.
///
/// For each method, sends both a named-param and a positional-param JSON-RPC request and asserts
/// no "invalid params" error.
#[tokio::test]
async fn spec_request_params_match_rpc_module() {
    let module = build_test_rpc_module();
    let test_cases = build_method_test_cases(&module);

    for (method_name, method_index, sample_values) in &test_cases {
        assert_rpc_accepts_spec_param_names(&module, method_name, *method_index, sample_values)
            .await;
        assert_rpc_accepts_spec_param_indices(&module, method_name, sample_values).await;
    }
}

Code snippet (ii):

/// Builds test cases with resolved spec data for each RPC method.
/// Also asserts that the test cases cover every method in the spec and module (completeness guard).
fn build_method_test_cases(
    module: &RpcModule<ProvingRpcServerImpl>,
) -> Vec<(&'static str, usize, Vec<Value>)> {
    let sample_values: Vec<(&str, Vec<Value>)> = vec![
        (SPEC_VERSION_METHOD, vec![]),
        (
            PROVE_TRANSACTION_METHOD,
            vec![
                serde_json::to_value(BlockId::Latest).unwrap(),
                serde_json::to_value(rpc_invoke_tx(invoke_tx_args!())).unwrap(),
            ],
        ),
    ];

    // Completeness guard: test cases must cover all spec and module methods.
    let methods = SPEC["methods"].as_array().unwrap();
    let tested: HashSet<&str> = sample_values.iter().map(|(name, _)| *name).collect();
    let spec_methods: HashSet<&str> = methods.iter().map(|m| m["name"].as_str().unwrap()).collect();
    let module_methods: HashSet<&str> = module.method_names().collect();
    assert_eq!(tested, spec_methods, "Test cases don't cover all spec methods");
    assert_eq!(tested, module_methods, "Test cases don't cover all module methods");

    // Resolve spec method index and validate sample values against spec schemas.
    sample_values
        .into_iter()
        .map(|(name, values)| {
            let method_index =
                methods.iter().position(|m| m["name"].as_str().unwrap() == name).unwrap();
            let spec_params = methods[method_index]["params"].as_array().unwrap();
            assert_eq!(
                spec_params.len(),
                values.len(),
                "Sample value count for {name} doesn't match spec param count"
            );
            for (param_index, (spec_param, sample_value)) in
                spec_params.iter().zip(&values).enumerate()
            {
                let param_name = spec_param["name"].as_str().unwrap();
                let schema = compile_schema_for_ref(
                    &SPEC,
                    &format!("methods/{method_index}/params/{param_index}/schema"),
                );
                assert_matches_schema(
                    &schema,
                    sample_value,
                    &format!("Parameter '{param_name}' of {name}"),
                );
            }
            (name, method_index, values)
        })
        .collect()
}

/// Sends a JSON-RPC request and asserts the module does not reject it with -32602 ("invalid
/// params"). `params_str` is the raw JSON fragment for the `"params"` field (may be empty).
async fn assert_rpc_does_not_reject_params(
    module: &RpcModule<ProvingRpcServerImpl>,
    method_name: &str,
    params_str: &str,
) {
    let req = format!(r#"{{"jsonrpc":"2.0","id":"1","method":"{method_name}"{params_str}}}"#);

    let (resp, _) = module.raw_json_request(&req, 1).await.unwrap();
    let json_resp: Value = serde_json::from_str(resp.get()).unwrap();

    if let Some(error) = json_resp.get("error") {
        let code = error["code"].as_i64().unwrap();
        assert_ne!(
            code,
            -32602,
            "Method {method_name}: RPC module rejected request with 'invalid params', meaning the \
             spec's parameter names or types don't match the implementation.\nRequest: \
             {req}\nError: {}",
            serde_json::to_string_pretty(error).unwrap(),
        );
    }
}

/// Sends a named-param JSON-RPC request (e.g. `"params": {"block_id": ...}`) and asserts
/// the module accepts it.
async fn assert_rpc_accepts_spec_param_names(
    module: &RpcModule<ProvingRpcServerImpl>,
    method_name: &str,
    method_index: usize,
    sample_values: &[Value],
) {
    let spec_params = SPEC["methods"][method_index]["params"].as_array().unwrap();
    let params_obj: serde_json::Map<String, Value> = spec_params
        .iter()
        .zip(sample_values)
        .map(|(spec_param, value)| {
            (spec_param["name"].as_str().unwrap().to_string(), value.clone())
        })
        .collect();
    let params_str = if params_obj.is_empty() {
        String::new()
    } else {
        format!(r#", "params": {}"#, serde_json::to_string(&params_obj).unwrap())
    };
    assert_rpc_does_not_reject_params(module, method_name, &params_str).await;
}

/// Sends a positional-param JSON-RPC request (e.g. `"params": [...]`) and asserts the module
/// accepts it.
async fn assert_rpc_accepts_spec_param_indices(
    module: &RpcModule<ProvingRpcServerImpl>,
    method_name: &str,
    sample_values: &[Value],
) {
    let params_str = if sample_values.is_empty() {
        String::new()
    } else {
        format!(r#", "params": {}"#, serde_json::to_string(&sample_values).unwrap())
    };
    assert_rpc_does_not_reject_params(module, method_name, &params_str).await;
}

avi-starkware

@avi-starkware made 6 comments.
Reviewable status: 1 of 6 files reviewed, 7 unresolved discussions (waiting on avivg-starkware, AvivYossef-starkware, noaov1, and Yoni-Starkware).

crates/starknet_os_runner/src/server.rs line 8 at r3 (raw file):

Previously, AvivYossef-starkware wrote…

?

Removed

crates/starknet_os_runner/src/server/rpc_spec_test.rs line 76 at r4 (raw file):

Previously, AvivYossef-starkware wrote…

And better to be a fixture

Done.

crates/starknet_os_runner/src/server/rpc_spec_test.rs line 177 at r4 (raw file):

Previously, AvivYossef-starkware wrote…

plz define it as a const and document it
(Maybe you can import it from JSON-RPCSE? )

Done.

crates/starknet_os_runner/src/server/rpc_spec_test.rs line 223 at r4 (raw file):

Previously, AvivYossef-starkware wrote…

What if we add more errors?

I added a check that goes over all errors in the spec.
To make sure we go over all errors that exits we will have to make them into an enum, which is a bigger change better to move to do in a separate PR.

crates/starknet_os_runner/src/server/rpc_spec_test.rs line 231 at r4 (raw file):

Previously, AvivYossef-starkware wrote…

Why do you specifically test BlockId?

I just thought it would be easy to cover all three cases for this schema, but it is probably overkill...

Removed.

crates/starknet_os_runner/src/server/rpc_spec_test.rs line 185 at r5 (raw file):

Previously, avivg-starkware wrote…

The test mixes object construction with the test logic, making it hard to tell what's being tested. I'd suggest extracting a build_method_test_cases helper that creates and validates the test data, so the test body is just as follows.

Also, we currently only test named params ("params": {…}). We should test positional params ("params": […]) too.

I sketched out the helpers below as a general direction.

Done.

AvivYossef-starkware

@AvivYossef-starkware reviewed 5 files and all commit messages, made 4 comments, and resolved 3 discussions.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on avi-starkware, avivg-starkware, noaov1, and Yoni-Starkware).

crates/starknet_os_runner/src/server/rpc_spec_test.rs line 223 at r4 (raw file):

Previously, avi-starkware (Avi Cohen) wrote…

I added a check that goes over all errors in the spec.
To make sure we go over all errors that exits we will have to make them into an enum, which is a bigger change better to move to do in a separate PR.

Can you add TODO?

crates/starknet_os_runner/src/server/rpc_spec_test.rs line 96 at r6 (raw file):

    module: &RpcModule<ProvingRpcServerImpl>,
) -> Vec<(&'static str, usize, Vec<Value>)> {
    let sample_values: Vec<(&str, Vec<Value>)> = vec![

consider defining a struct

Code quote:

(&str, Vec<Value>)

crates/starknet_os_runner/src/server/rpc_spec_test.rs line 141 at r6 (raw file):

                );
            }
            (name, method_index, values)

Define a struct

Code quote:

 (name, method_index, values)

crates/starknet_os_runner/src/server/rpc_spec_test.rs line 183 at r6 (raw file):

        .iter()
        .zip(sample_values)
        .map(|(spec_param, value)| {

Suggestion:

sample_value

avivg-starkware

@avivg-starkware made 1 comment and resolved 2 discussions.
Reviewable status: 3 of 6 files reviewed, 1 unresolved discussion (waiting on avi-starkware, AvivYossef-starkware, noaov1, and Yoni-Starkware).

crates/starknet_os_runner/src/server/rpc_spec_test.rs line 202 at r4 (raw file):

Previously, AvivYossef-starkware wrote…

It won't catch an issue if someone changes the return type of starknet_prove_transaction ( but it may be unlikely? )

Why unlikely?
I think testing the response is needed; in the SDK, we will test the response object against the specs. If the response object in the specs does not match the RPC, then the SDK's test will be irrelevant.

avi-starkware · 2026-03-08T09:07:40Z

crates/starknet_os_runner/src/server/rpc_spec_test.rs line 202 at r4 (raw file):

Previously, avivg-starkware wrote…

Why unlikely?
I think testing the response is needed; in the SDK, we will test the response object against the specs. If the response object in the specs does not match the RPC, then the SDK's test will be irrelevant.

I changed it to use a mock rpc to make sure the response types match the spec

avi-starkware · 2026-03-08T10:13:35Z

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

🤖 Generated with Claude Code

avivg-starkware

see comments re readabilty

@avivg-starkware partially reviewed 6 files and all commit messages, made 7 comments, and resolved 1 discussion.
Reviewable status: 6 of 9 files reviewed, 6 unresolved discussions (waiting on avi-starkware, AvivYossef-starkware, noaov1, and Yoni-Starkware).

crates/starknet_transaction_prover/src/server/rpc_spec_test.rs line 32 at r15 (raw file):

        ProverConfig { rpc_node_url: "http://localhost:1".to_string(), ..Default::default() };
    let rpc_impl = ProvingRpcServerImpl::new(RpcVirtualSnosProver::new(&config), 2);
    rpc_impl.into_rpc()

Suggestion (i):

    let config =
        ProverConfig { rpc_node_url: DUMMY_RPC_NODE_URL.to_string(), ..Default::default() };
    let rpc_impl = ProvingRpcServerImpl::new(
        RpcVirtualSnosProver::new(&config),
        TEST_MAX_CONCURRENT_REQUESTS,
    );

Code snippet (ii):

/// Dummy URL for test fixture.
const DUMMY_RPC_NODE_URL: &str = "http://localhost:1";
/// Max concurrent requests for the test fixture.
const TEST_MAX_CONCURRENT_REQUESTS: usize = 2;

crates/starknet_transaction_prover/src/server/rpc_spec_test.rs line 77 at r15 (raw file):

            code: i32::try_from(value["code"].as_i64().unwrap()).unwrap(),
            message: value["message"].as_str().unwrap().to_string(),
            expects_data: value.get("data").is_some(),

Do we not care what IS the data? just that it is there?

Code quote:

expects_data: value.get("data").is_some(),

crates/starknet_transaction_prover/src/server/rpc_spec_test.rs line 153 at r15 (raw file):

    let req = format!(r#"{{"jsonrpc":"2.0","id":"1","method":"{method_name}"{params_str}}}"#);

    let (resp, _) = module.raw_json_request(&req, 1).await.unwrap();

I think this is more readable (the funcs are used below as well )

Suggestion (i):

    let req = format_rpc_request(method_name, params_str);

    let (resp, _) = module.raw_json_request(&req, RPC_RESPONSE_BUFFER_SIZE).await.unwrap();

Code snippet (ii):

/// Builds a positional-param JSON fragment (e.g. `,"params":[...]`).
/// Returns an empty string when `sample_values` is empty.
fn format_params(sample_values: &[Value]) -> String {
    if sample_values.is_empty() {
        String::new()
    } else {
        format!(r#","params":{}"#, serde_json::to_string(sample_values).unwrap())
    }
}

/// Builds a JSON-RPC request string.
fn format_rpc_request(method_name: &str, params_fragment: &str) -> String {
    format!(r#"{{"jsonrpc":"2.0","id":"1","method":"{method_name}"{params_fragment}}}"#)
}

crates/starknet_transaction_prover/src/server/rpc_spec_test.rs line 205 at r15 (raw file):

        format!(r#", "params": {}"#, serde_json::to_string(&sample_values).unwrap())
    };
    assert_rpc_does_not_reject_params(module, method_name, &params_str).await;

I find this more readable

Suggestion:

    assert_rpc_does_not_reject_params(
        module,
        method_name,
        &format_params(sample_values),
    )
    .await;

crates/starknet_transaction_prover/src/server/rpc_spec_test.rs line 236 at r15 (raw file):

        let request =
            format!(r#"{{"jsonrpc":"2.0","id":"1","method":"{method_name}"{params_str}}}"#);
        let (resp, _) = mock_rpc_module.raw_json_request(&request, 1).await.unwrap();

Suggestion:

        let request =
            format_rpc_request(method_name, &format_params(sample_params));
        let (resp, _) =
            mock_rpc_module.raw_json_request(&request, RPC_RESPONSE_BUFFER_SIZE).await.unwrap();

crates/starknet_transaction_prover/src/server/rpc_spec_test.rs line 262 at r15 (raw file):

    );

    let (response, _) = rpc_module.raw_json_request(&request, 1).await.unwrap();

Suggestion:

    let sample_values = vec![
        serde_json::to_value(BlockId::Pending).unwrap(),
        serde_json::to_value(rpc_invoke_tx(invoke_tx_args!())).unwrap(),
    ];
    let request = format_rpc_request(
        PROVE_TRANSACTION_METHOD,
        &format_params(&sample_values),
    );

    let (response, _) =
        rpc_module.raw_json_request(&request, RPC_RESPONSE_BUFFER_SIZE).await.unwrap()

avi-starkware

@avi-starkware made 6 comments and resolved 5 discussions.
Reviewable status: 4 of 9 files reviewed, 1 unresolved discussion (waiting on avivg-starkware, AvivYossef-starkware, noaov1, and Yoni-Starkware).

crates/starknet_transaction_prover/src/server/rpc_spec_test.rs line 77 at r15 (raw file):

Previously, avivg-starkware wrote…

Do we not care what IS the data? just that it is there?

The spec only defines data as a string type (content varies at runtime), so presence-checking is the right level for spec conformance

crates/starknet_transaction_prover/src/server/rpc_spec_test.rs line 153 at r15 (raw file):

Previously, avivg-starkware wrote…

I think this is more readable (the funcs are used below as well )

Done.

crates/starknet_transaction_prover/src/server/rpc_spec_test.rs line 205 at r15 (raw file):

Previously, avivg-starkware wrote…

I find this more readable

Done.

crates/starknet_transaction_prover/src/server/rpc_spec_test.rs line 32 at r15 (raw file):

        ProverConfig { rpc_node_url: "http://localhost:1".to_string(), ..Default::default() };
    let rpc_impl = ProvingRpcServerImpl::new(RpcVirtualSnosProver::new(&config), 2);
    rpc_impl.into_rpc()

Done.

crates/starknet_transaction_prover/src/server/rpc_spec_test.rs line 236 at r15 (raw file):

        let request =
            format!(r#"{{"jsonrpc":"2.0","id":"1","method":"{method_name}"{params_str}}}"#);
        let (resp, _) = mock_rpc_module.raw_json_request(&request, 1).await.unwrap();

Done.

crates/starknet_transaction_prover/src/server/rpc_spec_test.rs line 262 at r15 (raw file):

    );

    let (response, _) = rpc_module.raw_json_request(&request, 1).await.unwrap();

Done.

avivg-starkware

@avivg-starkware partially reviewed 1 file and resolved 1 discussion.
Reviewable status: 5 of 9 files reviewed, all discussions resolved (waiting on AvivYossef-starkware, noaov1, and Yoni-Starkware).

avi-starkware

@avi-starkware partially reviewed 9 files and all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on noaov1 and Yoni-Starkware).

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

avi-starkware

@avi-starkware partially reviewed 2 files and all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on noaov1 and Yoni-Starkware).

avi-starkware force-pushed the avi/privacy/add-rpc-spec branch 3 times, most recently from 02086f9 to 857c8f8 Compare February 26, 2026 11:56

avi-starkware requested review from AvivYossef-starkware, Yoni-Starkware and noaov1 February 26, 2026 12:03

avi-starkware force-pushed the avi/privacy/add-rpc-spec branch from 857c8f8 to 5af226f Compare February 28, 2026 11:03

cursor Bot reviewed Feb 28, 2026

View reviewed changes

Comment thread crates/starknet_transaction_prover/resources/proving_api_openrpc.json

avi-starkware force-pushed the avi/privacy/add-rpc-spec branch 2 times, most recently from 35c6087 to 657f805 Compare March 1, 2026 09:19

AvivYossef-starkware requested changes Mar 4, 2026

View reviewed changes

avi-starkware force-pushed the avi/privacy/add-rpc-spec branch 2 times, most recently from 163b684 to c87dff8 Compare March 4, 2026 10:33

avi-starkware commented Mar 4, 2026

View reviewed changes

avi-starkware force-pushed the avi/privacy/add-rpc-spec branch from c87dff8 to 24a1976 Compare March 4, 2026 12:40

AvivYossef-starkware requested changes Mar 4, 2026

View reviewed changes

avivg-starkware suggested changes Mar 4, 2026

View reviewed changes

avi-starkware force-pushed the avi/privacy/add-rpc-spec branch from 24a1976 to 003d2aa Compare March 5, 2026 10:47

cursor Bot reviewed Mar 5, 2026

View reviewed changes

Comment thread crates/starknet_transaction_prover/src/server/rpc_spec_test.rs

avi-starkware commented Mar 5, 2026

View reviewed changes

AvivYossef-starkware requested changes Mar 5, 2026

View reviewed changes

avi-starkware force-pushed the avi/privacy/add-rpc-spec branch 2 times, most recently from a1d844f to 7d6de3e Compare March 5, 2026 11:59

cursor Bot reviewed Mar 5, 2026

View reviewed changes

Comment thread crates/starknet_os_runner/src/server/rpc_spec_test.rs Outdated

avi-starkware force-pushed the avi/privacy/add-rpc-spec branch from 7d6de3e to 94bd95d Compare March 5, 2026 12:31

avi-starkware force-pushed the avi/privacy/add-rpc-spec branch from 8c34eab to 03f8343 Compare March 5, 2026 17:24

avivg-starkware suggested changes Mar 5, 2026

View reviewed changes

avi-starkware force-pushed the avi/privacy/add-rpc-spec branch 2 times, most recently from bc1795d to 32803ba Compare March 8, 2026 08:52

avi-starkware changed the title ~~starknet_os_runner: add OpenRPC spec and spec validation~~ starknet_transaction_prover: add OpenRPC spec and spec validation Mar 8, 2026

avi-starkware force-pushed the avi/privacy/add-rpc-spec branch from 32803ba to 68a7893 Compare March 8, 2026 10:22

avivg-starkware approved these changes Mar 8, 2026

View reviewed changes

avi-starkware force-pushed the avi/privacy/add-rpc-spec branch 2 times, most recently from 62d5ab7 to 0e99bb9 Compare March 8, 2026 17:33

avi-starkware commented Mar 8, 2026

View reviewed changes

avivg-starkware reviewed Mar 8, 2026

View reviewed changes

avi-starkware force-pushed the avi/privacy/add-rpc-spec branch 2 times, most recently from 134af25 to f6c4b85 Compare March 10, 2026 07:15

avi-starkware commented Mar 10, 2026

View reviewed changes

avi-starkware enabled auto-merge March 10, 2026 07:22

avi-starkware disabled auto-merge March 10, 2026 07:22

avi-starkware enabled auto-merge March 10, 2026 07:22

avi-starkware disabled auto-merge March 10, 2026 07:58

avi-starkware force-pushed the avi/privacy/add-rpc-spec branch from f6c4b85 to 278b168 Compare March 10, 2026 08:07

cursor Bot reviewed Mar 10, 2026

View reviewed changes

Comment thread crates/starknet_transaction_prover/resources/proving_api_openrpc.json

starknet_transaction_prover: add OpenRPC spec and spec validation

f0eb540

avi-starkware force-pushed the avi/privacy/add-rpc-spec branch from 278b168 to f0eb540 Compare March 10, 2026 08:15

avi-starkware commented Mar 10, 2026

View reviewed changes

avi-starkware enabled auto-merge March 10, 2026 08:27

avi-starkware added this pull request to the merge queue Mar 10, 2026

Merged via the queue into main-v0.14.2 with commit c6c1c8b Mar 10, 2026
36 of 38 checks passed

github-actions Bot locked and limited conversation to collaborators Mar 11, 2026

Conversation

avi-starkware commented Feb 26, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

reviewable-StarkWare commented Feb 26, 2026

Uh oh!

github-actions Bot commented Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

avivg-starkware commented Mar 3, 2026

Uh oh!

avivg-starkware commented Mar 3, 2026

Uh oh!

AvivYossef-starkware left a comment

Choose a reason for hiding this comment

Uh oh!

avi-starkware left a comment

Choose a reason for hiding this comment

Uh oh!

avi-starkware left a comment

Choose a reason for hiding this comment

Uh oh!

AvivYossef-starkware left a comment

Choose a reason for hiding this comment

Uh oh!

avivg-starkware left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

avi-starkware left a comment

Choose a reason for hiding this comment

Uh oh!

AvivYossef-starkware left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

avivg-starkware left a comment

Choose a reason for hiding this comment

Uh oh!

avi-starkware commented Mar 8, 2026

Uh oh!

avi-starkware commented Mar 8, 2026

Code review

Uh oh!

avivg-starkware left a comment

Choose a reason for hiding this comment

Uh oh!

avi-starkware left a comment

Choose a reason for hiding this comment

Uh oh!

avivg-starkware left a comment

Choose a reason for hiding this comment

Uh oh!

avi-starkware left a comment

Choose a reason for hiding this comment

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

avi-starkware left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

avi-starkware commented Feb 26, 2026 •

edited by cursor Bot

Loading

github-actions Bot commented Feb 26, 2026 •

edited

Loading