Add workflow to compare vm outputs for factorial on all layouts (#1653)

* Show only layout builtins in air private input * Add test * Add changelog entry * Fix wasm import * Run for extra step in proof mode * Add recursive layout * Add script * Add workflow * Add workflow * Add workflow * Update file permissions * Fix * Fix script * Fix workflow * Fix workflow * Fix typo * Fix workflow * Fix workflow * Fix workflow * Fix workflow * Clean files after script
lambdaclass · Mar 7, 2024 · 1fc537c · 1fc537c
1 parent 7566aa5
commit 1fc537c
Show file tree

Hide file tree

Showing 8 changed files with 205 additions and 64 deletions.
diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml
@@ -613,3 +613,42 @@ jobs:
         cairo-compile cairo_programs/array_sum.cairo --no_debug_info --output cairo_programs/array_sum.json
         cd examples/wasm-demo
         wasm-pack build --target=web
+        
+  compare-factorial-outputs-all-layouts:
+    name: Compare factorial outputs for all layouts
+    needs: [ build-programs, build-release ]
+    runs-on: ubuntu-22.04
+    steps:
+    - name: Checkout
+      uses: actions/checkout@v3
+
+    - name: Python3 Build
+      uses: actions/setup-python@v4
+      with:
+        python-version: '3.9'
+        cache: 'pip'
+
+    - name: Install cairo-lang and deps
+      run: pip install -r requirements.txt
+
+    - name: Fetch release binary
+      uses: actions/cache/restore@v3
+      with:
+        key: cli-bin-rel-${{ github.sha }}
+        path: target/release/cairo-vm-cli
+        fail-on-cache-miss: true
+
+    - uses: actions/download-artifact@master
+      with:
+        name: proof_programs
+        path: cairo_programs/proof_programs/
+
+    - name: Fetch programs
+      uses: actions/cache/restore@v3
+      with:
+        path: ${{ env.CAIRO_PROGRAMS_PATH }}
+        key: cairo_proof_programs-cache-${{ hashFiles('cairo_programs/**/*.cairo', 'examples/wasm-demo/src/array_sum.cairo') }}
+        fail-on-cache-miss: true
+
+    - name: Run script
+      run: ./vm/src/tests/compare_factorial_outputs_all_layouts.sh
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,8 @@
 
 #### Upcoming Changes
 
+* feat: Show only layout builtins in air private input [#1651](https://github.com/lambdaclass/cairo-vm/pull/1651)
+
 * feat: Sort builtin segment info upon serialization for Cairo PIE [#1654](https://github.com/lambdaclass/cairo-vm/pull/1654)
 
 * feat: Fix output serialization for cairo 1 [#1645](https://github.com/lambdaclass/cairo-vm/pull/1645)

diff --git a/cairo-vm-cli/src/main.rs b/cairo-vm-cli/src/main.rs
@@ -61,6 +61,7 @@ fn validate_layout(value: &str) -> Result<String, String> {
         "plain"
         | "small"
         | "dex"
+        | "recursive"
         | "starknet"
         | "starknet_with_keccak"
         | "recursive_large_output"

diff --git a/cairo1-run/src/cairo_run.rs b/cairo1-run/src/cairo_run.rs
@@ -195,6 +195,10 @@ pub fn cairo_run_program(
 
     // Run it until the end / infinite loop in proof_mode
     runner.run_until_pc(end, &mut vm, &mut hint_processor)?;
+    if cairo_run_config.proof_mode {
+        runner.run_for_steps(1, &mut vm, &mut hint_processor)?;
+    }
+
     if cairo_run_config.proof_mode || cairo_run_config.append_return_values {
         // As we will be inserting the return values into the output segment after running the main program (right before the infinite loop) the computed size for the output builtin will be 0
         // We need to manually set the segment size for the output builtin's segment so memory hole counting doesn't fail due to having a higher accessed address count than the segment's size

diff --git a/vm/src/air_private_input.rs b/vm/src/air_private_input.rs
@@ -17,13 +17,20 @@ use crate::Felt252;
 pub struct AirPrivateInputSerializable {
     trace_path: String,
     memory_path: String,
-    pedersen: Vec<PrivateInput>,
-    range_check: Vec<PrivateInput>,
-    ecdsa: Vec<PrivateInput>,
-    bitwise: Vec<PrivateInput>,
-    ec_op: Vec<PrivateInput>,
-    keccak: Vec<PrivateInput>,
-    poseidon: Vec<PrivateInput>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pedersen: Option<Vec<PrivateInput>>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    range_check: Option<Vec<PrivateInput>>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    ecdsa: Option<Vec<PrivateInput>>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    bitwise: Option<Vec<PrivateInput>>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    ec_op: Option<Vec<PrivateInput>>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    keccak: Option<Vec<PrivateInput>>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    poseidon: Option<Vec<PrivateInput>>,
 }
 
 // Contains only builtin public inputs, useful for library users
@@ -108,44 +115,34 @@ impl AirPrivateInput {
         AirPrivateInputSerializable {
             trace_path,
             memory_path,
-            pedersen: self.0.get(HASH_BUILTIN_NAME).cloned().unwrap_or_default(),
-            range_check: self
-                .0
-                .get(RANGE_CHECK_BUILTIN_NAME)
-                .cloned()
-                .unwrap_or_default(),
-            ecdsa: self
-                .0
-                .get(SIGNATURE_BUILTIN_NAME)
-                .cloned()
-                .unwrap_or_default(),
-            bitwise: self
-                .0
-                .get(BITWISE_BUILTIN_NAME)
-                .cloned()
-                .unwrap_or_default(),
-            ec_op: self.0.get(EC_OP_BUILTIN_NAME).cloned().unwrap_or_default(),
-            keccak: self.0.get(KECCAK_BUILTIN_NAME).cloned().unwrap_or_default(),
-            poseidon: self
-                .0
-                .get(POSEIDON_BUILTIN_NAME)
-                .cloned()
-                .unwrap_or_default(),
+            pedersen: self.0.get(HASH_BUILTIN_NAME).cloned(),
+            range_check: self.0.get(RANGE_CHECK_BUILTIN_NAME).cloned(),
+            ecdsa: self.0.get(SIGNATURE_BUILTIN_NAME).cloned(),
+            bitwise: self.0.get(BITWISE_BUILTIN_NAME).cloned(),
+            ec_op: self.0.get(EC_OP_BUILTIN_NAME).cloned(),
+            keccak: self.0.get(KECCAK_BUILTIN_NAME).cloned(),
+            poseidon: self.0.get(POSEIDON_BUILTIN_NAME).cloned(),
         }
     }
 }
 
 impl From<AirPrivateInputSerializable> for AirPrivateInput {
     fn from(private_input: AirPrivateInputSerializable) -> Self {
-        Self(HashMap::from([
-            (HASH_BUILTIN_NAME, private_input.pedersen),
-            (RANGE_CHECK_BUILTIN_NAME, private_input.range_check),
-            (SIGNATURE_BUILTIN_NAME, private_input.ecdsa),
-            (BITWISE_BUILTIN_NAME, private_input.bitwise),
-            (EC_OP_BUILTIN_NAME, private_input.ec_op),
-            (KECCAK_BUILTIN_NAME, private_input.keccak),
-            (POSEIDON_BUILTIN_NAME, private_input.poseidon),
-        ]))
+        let mut inputs = HashMap::new();
+        let mut insert_input = |input_name, input| {
+            if let Some(input) = input {
+                inputs.insert(input_name, input);
+            }
+        };
+        insert_input(HASH_BUILTIN_NAME, private_input.pedersen);
+        insert_input(RANGE_CHECK_BUILTIN_NAME, private_input.range_check);
+        insert_input(SIGNATURE_BUILTIN_NAME, private_input.ecdsa);
+        insert_input(BITWISE_BUILTIN_NAME, private_input.bitwise);
+        insert_input(EC_OP_BUILTIN_NAME, private_input.ec_op);
+        insert_input(KECCAK_BUILTIN_NAME, private_input.keccak);
+        insert_input(POSEIDON_BUILTIN_NAME, private_input.poseidon);
+
+        Self(inputs)
     }
 }
 
@@ -168,44 +165,47 @@ mod tests {
         assert_matches::assert_matches,
     };
 
+    #[cfg(any(target_arch = "wasm32", no_std, not(feature = "std")))]
+    use crate::alloc::string::ToString;
+
     #[cfg(feature = "std")]
     #[test]
     fn test_from_serializable() {
         let serializable_private_input = AirPrivateInputSerializable {
             trace_path: "trace.bin".to_string(),
             memory_path: "memory.bin".to_string(),
-            pedersen: vec![PrivateInput::Pair(PrivateInputPair {
+            pedersen: Some(vec![PrivateInput::Pair(PrivateInputPair {
                 index: 0,
                 x: Felt252::from(100),
                 y: Felt252::from(200),
-            })],
-            range_check: vec![PrivateInput::Value(PrivateInputValue {
+            })]),
+            range_check: Some(vec![PrivateInput::Value(PrivateInputValue {
                 index: 10000,
                 value: Felt252::from(8000),
-            })],
-            ecdsa: vec![PrivateInput::Signature(PrivateInputSignature {
+            })]),
+            ecdsa: Some(vec![PrivateInput::Signature(PrivateInputSignature {
                 index: 0,
                 pubkey: Felt252::from(123),
                 msg: Felt252::from(456),
                 signature_input: SignatureInput {
                     r: Felt252::from(654),
                     w: Felt252::from(321),
                 },
-            })],
-            bitwise: vec![PrivateInput::Pair(PrivateInputPair {
+            })]),
+            bitwise: Some(vec![PrivateInput::Pair(PrivateInputPair {
                 index: 4,
                 x: Felt252::from(7),
                 y: Felt252::from(8),
-            })],
-            ec_op: vec![PrivateInput::EcOp(PrivateInputEcOp {
+            })]),
+            ec_op: Some(vec![PrivateInput::EcOp(PrivateInputEcOp {
                 index: 1,
                 p_x: Felt252::from(10),
                 p_y: Felt252::from(10),
                 m: Felt252::from(100),
                 q_x: Felt252::from(11),
                 q_y: Felt252::from(14),
-            })],
-            keccak: vec![PrivateInput::KeccakState(PrivateInputKeccakState {
+            })]),
+            keccak: Some(vec![PrivateInput::KeccakState(PrivateInputKeccakState {
                 index: 0,
                 input_s0: Felt252::from(0),
                 input_s1: Felt252::from(1),
@@ -215,23 +215,47 @@ mod tests {
                 input_s5: Felt252::from(5),
                 input_s6: Felt252::from(6),
                 input_s7: Felt252::from(7),
-            })],
-            poseidon: vec![PrivateInput::PoseidonState(PrivateInputPoseidonState {
-                index: 42,
-                input_s0: Felt252::from(1),
-                input_s1: Felt252::from(2),
-                input_s2: Felt252::from(3),
-            })],
+            })]),
+            poseidon: Some(vec![PrivateInput::PoseidonState(
+                PrivateInputPoseidonState {
+                    index: 42,
+                    input_s0: Felt252::from(1),
+                    input_s1: Felt252::from(2),
+                    input_s2: Felt252::from(3),
+                },
+            )]),
         };
 
         let private_input = AirPrivateInput::from(serializable_private_input.clone());
 
-        assert_matches!(private_input.0.get(HASH_BUILTIN_NAME), Some(data) if *data == serializable_private_input.pedersen);
-        assert_matches!(private_input.0.get(RANGE_CHECK_BUILTIN_NAME), Some(data) if *data == serializable_private_input.range_check);
-        assert_matches!(private_input.0.get(SIGNATURE_BUILTIN_NAME), Some(data) if *data == serializable_private_input.ecdsa);
-        assert_matches!(private_input.0.get(BITWISE_BUILTIN_NAME), Some(data) if *data == serializable_private_input.bitwise);
-        assert_matches!(private_input.0.get(EC_OP_BUILTIN_NAME), Some(data) if *data == serializable_private_input.ec_op);
-        assert_matches!(private_input.0.get(KECCAK_BUILTIN_NAME), Some(data) if *data == serializable_private_input.keccak);
-        assert_matches!(private_input.0.get(POSEIDON_BUILTIN_NAME), Some(data) if *data == serializable_private_input.poseidon);
+        assert_matches!(private_input.0.get(HASH_BUILTIN_NAME), data if data == serializable_private_input.pedersen.as_ref());
+        assert_matches!(private_input.0.get(RANGE_CHECK_BUILTIN_NAME), data if data == serializable_private_input.range_check.as_ref());
+        assert_matches!(private_input.0.get(SIGNATURE_BUILTIN_NAME), data if data == serializable_private_input.ecdsa.as_ref());
+        assert_matches!(private_input.0.get(BITWISE_BUILTIN_NAME), data if data == serializable_private_input.bitwise.as_ref());
+        assert_matches!(private_input.0.get(EC_OP_BUILTIN_NAME), data if data == serializable_private_input.ec_op.as_ref());
+        assert_matches!(private_input.0.get(KECCAK_BUILTIN_NAME), data if data == serializable_private_input.keccak.as_ref());
+        assert_matches!(private_input.0.get(POSEIDON_BUILTIN_NAME), data if data == serializable_private_input.poseidon.as_ref());
+    }
+
+    #[test]
+    fn serialize_air_private_input_small_layout_only_builtins() {
+        let config = crate::cairo_run::CairoRunConfig {
+            proof_mode: true,
+            relocate_mem: true,
+            trace_enabled: true,
+            layout: "small",
+            ..Default::default()
+        };
+        let (runner, vm) = crate::cairo_run::cairo_run(include_bytes!("../../cairo_programs/proof_programs/fibonacci.json"), &config, &mut crate::hint_processor::builtin_hint_processor::builtin_hint_processor_definition::BuiltinHintProcessor::new_empty()).unwrap();
+        let public_input = runner.get_air_private_input(&vm);
+        let serialized_public_input =
+            public_input.to_serializable("/dev/null".to_string(), "/dev/null".to_string());
+        assert!(serialized_public_input.pedersen.is_some());
+        assert!(serialized_public_input.range_check.is_some());
+        assert!(serialized_public_input.ecdsa.is_some());
+        assert!(serialized_public_input.bitwise.is_none());
+        assert!(serialized_public_input.ec_op.is_none());
+        assert!(serialized_public_input.keccak.is_none());
+        assert!(serialized_public_input.poseidon.is_none());
     }
 }
diff --git a/vm/src/cairo_run.rs b/vm/src/cairo_run.rs
@@ -88,6 +88,10 @@ pub fn cairo_run_program(
     cairo_runner
         .run_until_pc(end, &mut vm, hint_executor)
         .map_err(|err| VmException::from_vm_error(&cairo_runner, &vm, err))?;
+
+    if cairo_run_config.proof_mode {
+        cairo_runner.run_for_steps(1, &mut vm, hint_executor)?;
+    }
     cairo_runner.end_run(
         cairo_run_config.disable_trace_padding,
         false,

diff --git a/vm/src/tests/compare_factorial_outputs_all_layouts.sh b/vm/src/tests/compare_factorial_outputs_all_layouts.sh
@@ -0,0 +1,66 @@
+#!/usr/bin/env sh
+
+factorial_compiled="cairo_programs/proof_programs/factorial.json"
+passed_tests=0
+failed_tests=0
+exit_code=0
+
+for layout in "plain" "small" "dex" "recursive" "starknet" "starknet_with_keccak" "recursive_large_output" "all_solidity" "starknet_with_keccak"; do
+    # Run cairo_vm
+    echo "Running cairo-vm with layout $layout"
+    cargo run -p cairo-vm-cli --release -- --layout $layout --proof_mode $factorial_compiled --trace_file factorial_rs.trace --memory_file factorial_rs.memory --air_public_input factorial_rs.air_public_input --air_private_input factorial_rs.air_private_input
+    # Run cairo_lang
+    echo "Running cairo_lang with layout $layout"
+    cairo-run --layout $layout --proof_mode  --program $factorial_compiled --trace_file factorial_py.trace --memory_file factorial_py.memory --air_public_input factorial_py.air_public_input --air_private_input factorial_py.air_private_input
+    # Compare trace
+    echo "Running trace comparison for layout $layout"
+    if ! diff -q factorial_rs.trace factorial_py.trace; then
+        echo "Trace differs for layout $layout"
+        exit_code=1
+        failed_tests=$((failed_tests + 1))
+    else
+        passed_tests=$((passed_tests + 1))
+    fi
+    # Compare memory
+    echo "Running memory comparison for layout $layout"
+    if ! ./vm/src/tests/memory_comparator.py factorial_rs.memory factorial_py.memory; then
+        echo "Memory differs for layout $layout"
+        exit_code=1
+        failed_tests=$((failed_tests + 1))
+    else
+        passed_tests=$((passed_tests + 1))
+    fi
+    # Compare air public input
+    echo "Running air public input  comparison for layout $layout"
+    if ! ./vm/src/tests/air_public_input_comparator.py factorial_rs.air_public_input factorial_py.air_public_input; then
+        echo "Air public input differs for layout $layout"
+        exit_code=1
+        failed_tests=$((failed_tests + 1))
+    else
+        passed_tests=$((passed_tests + 1))
+    fi
+    # Compare air private input
+    echo "Running air private input  comparison for layout $layout"
+    if ! ./vm/src/tests/air_private_input_comparator.py factorial_rs.air_private_input factorial_py.air_private_input; then
+        echo "Air private input differs for layout $layout"
+        exit_code=1
+        failed_tests=$((failed_tests + 1))
+    else
+        passed_tests=$((passed_tests + 1))
+    fi
+    # Clean files generated by the script
+    echo "Cleaning files"
+    rm factorial_rs.*
+    rm factorial_py.*
+done
+
+if test $failed_tests != 0; then
+    echo "Comparisons: $failed_tests failed, $passed_tests passed, $((failed_tests + passed_tests)) total"
+elif test $passed_tests = 0; then
+    echo "No tests ran!"
+    exit_code=2
+else
+    echo "All $passed_tests tests passed; no discrepancies found"
+fi
+
+exit "${exit_code}"
diff --git a/vm/src/vm/runners/cairo_runner.rs b/vm/src/vm/runners/cairo_runner.rs
@@ -179,6 +179,7 @@ impl CairoRunner {
             "plain" => CairoLayout::plain_instance(),
             "small" => CairoLayout::small_instance(),
             "dex" => CairoLayout::dex_instance(),
+            "recursive" => CairoLayout::recursive_instance(),
             "starknet" => CairoLayout::starknet_instance(),
             "starknet_with_keccak" => CairoLayout::starknet_with_keccak_instance(),
             "recursive_large_output" => CairoLayout::recursive_large_output_instance(),