Minor_ret_decoding_refactor

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 #### Upcoming Changes
 
+* refactor: Limit ret opcode decodeing to Cairo0's standards. [#1925](https://github.com/lambdaclass/cairo-vm/pull/1925)
+
 #### [2.0.0-rc4] - 2025-01-23
 
 * feat: implement `kzg` data availability hints [#1887](https://github.com/lambdaclass/cairo-vm/pull/1887)

vm/src/vm/decoding/decoder.rs

@@ -82,11 +82,11 @@ pub fn decode_instruction(encoded_instr: u64) -> Result<Instruction, VirtualMach
         _ => return Err(VirtualMachineError::InvalidPcUpdate(pc_update_num)),
     };
 
-    let res = match res_logic_num {
-        0 if matches!(pc_update, PcUpdate::Jnz) => Res::Unconstrained,
-        0 => Res::Op1,
-        1 => Res::Add,
-        2 => Res::Mul,
+    let res = match (res_logic_num, pc_update == PcUpdate::Jnz) {
+        (0, true) => Res::Unconstrained,
+        (0, false) => Res::Op1,
+        (1, false) => Res::Add,
+        (2, false) => Res::Mul,
         _ => return Err(VirtualMachineError::InvalidRes(res_logic_num)),
     };
 
@@ -98,17 +98,38 @@ pub fn decode_instruction(encoded_instr: u64) -> Result<Instruction, VirtualMach
         _ => return Err(VirtualMachineError::InvalidOpcode(opcode_num)),
     };
 
-    let ap_update = match ap_update_num {
-        0 if matches!(opcode, Opcode::Call) => ApUpdate::Add2,
-        0 => ApUpdate::Regular,
-        1 => ApUpdate::Add,
-        2 => ApUpdate::Add1,
+    let ap_update = match (ap_update_num, opcode == Opcode::Call) {
+        (0, true) => ApUpdate::Add2,
+        (0, false) => ApUpdate::Regular,
+        (1, false) => ApUpdate::Add,
+        (2, false) => ApUpdate::Add1,
         _ => return Err(VirtualMachineError::InvalidApUpdate(ap_update_num)),
     };
 
     let fp_update = match opcode {
-        Opcode::Call => FpUpdate::APPlus2,
-        Opcode::Ret => FpUpdate::Dst,
+        Opcode::Call => {
+            if off0 != 0
+                || off1 != 1
+                || ap_update != ApUpdate::Add2
+                || dst_register != Register::AP
+                || op0_register != Register::AP
+            {
+                return Err(VirtualMachineError::InvalidOpcode(opcode_num));
+            };
+            FpUpdate::APPlus2
+        }
+        Opcode::Ret => {
+            if off0 != -2
+                || off2 != -1
+                || dst_register != Register::FP
+                || op1_addr != Op1Addr::FP
+                || res != Res::Op1
+                || pc_update != PcUpdate::Jump
+            {
+                return Err(VirtualMachineError::InvalidOpcode(opcode_num));
+            };
+            FpUpdate::Dst
+        }
         _ => FpUpdate::Regular,
     };
 

vm/src/vm/runners/cairo_runner.rs

@@ -154,7 +154,7 @@ pub struct CairoRunner {
     run_ended: bool,
     segments_finalized: bool,
     execution_public_memory: Option<Vec<usize>>,
-    runner_mode: RunnerMode,
+    pub(crate) runner_mode: RunnerMode,
     pub relocated_memory: Vec<Option<Felt252>>,
     pub exec_scopes: ExecutionScopes,
     pub relocated_trace: Option<Vec<RelocatedTraceEntry>>,

vm/src/vm/security.rs

@@ -4,9 +4,10 @@ use num_traits::ToPrimitive;
 
 use super::{
     errors::{runner_errors::RunnerError, vm_errors::VirtualMachineError},
-    runners::cairo_runner::CairoRunner,
+    runners::cairo_runner::{CairoRunner, RunnerMode},
 };
 use crate::types::relocatable::MaybeRelocatable;
+use crate::Felt252;
 
 /// Verify that the completed run in a runner is safe to be relocated and be
 /// used by other Cairo programs.
@@ -79,6 +80,46 @@ pub fn verify_secure_runner(
         builtin.run_security_checks(&runner.vm)?;
     }
 
+    // Validate ret FP.
+    let initial_fp = runner.get_initial_fp().ok_or_else(|| {
+        VirtualMachineError::Other(anyhow::anyhow!(
+            "Failed to retrieve the initial_fp: it is None. \
+                     The initial_fp field should be initialized after running the entry point."
+        ))
+    })?;
+    let ret_fp_addr = (initial_fp - 2).map_err(VirtualMachineError::Math)?;
+    let ret_fp = runner.vm.get_maybe(&ret_fp_addr).ok_or_else(|| {
+        VirtualMachineError::Other(anyhow::anyhow!(
+            "Ret FP address is not in memory: {ret_fp_addr}"
+        ))
+    })?;
+    let final_fp = runner.vm.get_fp();
+    if final_fp.segment_index != 1 {
+        return Err(VirtualMachineError::Other(anyhow::anyhow!(
+            "Final FP segment index is not 1: {final_fp}"
+        )));
+    }
+    match ret_fp {
+        MaybeRelocatable::RelocatableValue(value) => {
+            if runner.runner_mode == RunnerMode::ProofModeCanonical && value != final_fp {
+                return Err(VirtualMachineError::Other(anyhow::anyhow!(
+                    "Return FP is not equal to final FP: ret_f={ret_fp}, final_fp={final_fp}"
+                )));
+            }
+            if runner.runner_mode == RunnerMode::ExecutionMode && value.offset != final_fp.offset {
+                return Err(VirtualMachineError::Other(anyhow::anyhow!(
+                    "Return FP offset is not equal to final FP offset: ret_f={ret_fp}, final_fp={final_fp}"
+                )));
+            }
+        }
+        MaybeRelocatable::Int(value) => {
+            if Felt252::from(final_fp.offset) != value {
+                return Err(VirtualMachineError::Other(anyhow::anyhow!(
+                    "Return FP offset is not equal to final FP offset: ret_fp={ret_fp}, final_fp={final_fp}"
+                )));
+            }
+        }
+    }
     Ok(())
 }