Merge pull request #565 from eqlabs/krisztian/class-hash-references-fix

fix(class_hash): fix up named tuple types for references

crates/pathfinder/src/state/class_hash.rs

@@ -117,39 +117,56 @@ fn compute_class_hash0(mut contract_definition: json::ContractDefinition<'_>) ->
             Ok(())
         })?;
 
-    /// Insert's a space ": " -> " : " for every tuple value in `cairo_type`. This is required by StarkNet 0.10 for
-    /// some obscure backwards compatibility reason for older contracts.
-    fn insert_space(object: &mut serde_json::Map<String, serde_json::Value>) {
+    fn add_extra_space_to_cairo_named_tuples(value: &mut serde_json::Value) {
+        match value {
+            serde_json::Value::Array(v) => walk_array(v),
+            serde_json::Value::Object(m) => walk_map(m),
+            _ => {}
+        }
+    }
+
+    fn walk_array(array: &mut [serde_json::Value]) {
+        for v in array.iter_mut() {
+            add_extra_space_to_cairo_named_tuples(v);
+        }
+    }
+
+    fn walk_map(object: &mut serde_json::Map<String, serde_json::Value>) {
         for (k, v) in object.iter_mut() {
-            if k == "cairo_type" {
-                // Check that the value is a tuple.
-                if let Some(tuple_inner) = v
-                    .as_str()
-                    .and_then(|v| v.strip_prefix('(').and_then(|v| v.strip_suffix(')')))
-                {
-                    let new_val = format!(
-                        "({})",
-                        // FIXME: Replace this crude hack with something more intelligent.
-                        //        It is required because if we receive an already correct ` : `, we will still
-                        //        "repair" it to `  : ` which we then fix at the end.
-                        tuple_inner.replace(": ", " : ").replace("  :", " :")
-                    );
-                    *v = serde_json::Value::String(new_val);
+            match v {
+                serde_json::Value::String(s) => {
+                    let new_value = add_extra_space_to_named_tuple_type_definition(k, s);
+                    if new_value.as_ref() != s {
+                        *v = serde_json::Value::String(new_value.into());
+                    }
                 }
-            } else if let Some(inner_object) = v.as_object_mut() {
-                // `cairo_type` occurs at multiple levels in the object, so we must explore deeper.
-                insert_space(inner_object);
+                _ => add_extra_space_to_cairo_named_tuples(v),
             }
         }
     }
 
+    fn add_extra_space_to_named_tuple_type_definition<'a>(
+        key: &str,
+        value: &'a str,
+    ) -> std::borrow::Cow<'a, str> {
+        use std::borrow::Cow::*;
+        match key {
+            "cairo_type" | "value" => Owned(add_extra_space_before_colon(value)),
+            _ => Borrowed(value),
+        }
+    }
+
+    fn add_extra_space_before_colon(v: &str) -> String {
+        // This is required because if we receive an already correct ` : `, we will still
+        // "repair" it to `  : ` which we then fix at the end.
+        v.replace(": ", " : ").replace("  :", " :")
+    }
+
     // Handle a backwards compatibility hack which is required if compiler_version is not present.
     // See `insert_space` for more details.
     if contract_definition.program.compiler_version.is_none() {
-        let identifiers = contract_definition.program.identifiers.as_object_mut();
-        if let Some(identifiers) = identifiers {
-            insert_space(identifiers);
-        }
+        add_extra_space_to_cairo_named_tuples(&mut contract_definition.program.identifiers);
+        add_extra_space_to_cairo_named_tuples(&mut contract_definition.program.reference_manager);
     }
 
     let truncated_keccak = {
@@ -524,6 +541,25 @@ mod json {
 
             assert_eq!(hash.0, expected);
         }
+
+        #[tokio::test]
+        async fn cairo_0_10_part_3() {
+            let expected = crate::starkhash!(
+                "066af14b94491ba4e2aea1117acf0a3155c53d92fdfd9c1f1dcac90dc2d30157"
+            );
+
+            // Contract who's class contains `compiler_version` property as well as `cairo_type` with tuple values.
+            // These tuple values require a space to be injected in order to achieve the correct hash.
+            let resp = reqwest::get("https://alpha4.starknet.io/feeder_gateway/get_full_contract?blockNumber=latest&contractAddress=0x0424e799d610433168a31aab44c0d3e38b45d97387b45de80089f56c184fa315")
+                .await
+                .unwrap();
+
+            let payload = resp.text().await.expect("response wasn't a string");
+
+            let hash = super::super::compute_class_hash(payload.as_bytes()).unwrap();
+
+            assert_eq!(hash.0, expected);
+        }
     }
 
     #[cfg(test)]

py/src/compute_class_hash.py

@@ -1,8 +1,8 @@
 # reads stdin for a contract_definition json blob, writes a class hash to stdout
 # example: python py/src/compute_class_hash.py < class_definition.json
 
-from starkware.starknet.business_logic.state_objects import ContractDefinitionFact
-from starkware.starknet.services.api.contract_definition import ContractDefinition
+from starkware.starknet.business_logic.state.objects import ContractClassFact
+from starkware.starknet.services.api.contract_class import ContractClass
 from starkware.cairo.lang.vm.crypto import pedersen_hash
 
 import sys
@@ -20,7 +20,7 @@ def main():
     sys.stdin.reconfigure(encoding="utf-8")
     contents = sys.stdin.read()
 
-    cdf = ContractDefinitionFact(ContractDefinition.loads(contents))
+    cdf = ContractClassFact(ContractClass.loads(contents))
 
     print(cdf._hash(pedersen_hash).hex())
     sys.exit(0)