Fix bug in opcode_assertions + add test (#653)

lambdaclass · Dec 21, 2022 · 42dadd1 · 42dadd1
1 parent a2b139d
commit 42dadd1
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 13 deletions.
diff --git a/src/vm/errors/vm_errors.rs b/src/vm/errors/vm_errors.rs
@@ -28,8 +28,8 @@ pub enum VirtualMachineError {
     UnconstrainedResJumpRel,
     #[error("Res.UNCONSTRAINED cannot be used with Opcode.ASSERT_EQ")]
     UnconstrainedResAssertEq,
-    #[error("ASSERT_EQ instruction failed; {0} != {1}")]
-    DiffAssertValues(BigInt, BigInt),
+    #[error("ASSERT_EQ instruction failed; {0:?} != {1:?}")]
+    DiffAssertValues(MaybeRelocatable, MaybeRelocatable),
     #[error("Call failed to write return-pc (inconsistent op0): {0:?} != {1:?}. Did you forget to increment ap?")]
     CantWriteReturnPc(MaybeRelocatable, MaybeRelocatable),
     #[error("Call failed to write return-fp (inconsistent dst): {0:?} != {1:?}. Did you forget to increment ap?")]

diff --git a/src/vm/vm_core.rs b/src/vm/vm_core.rs
@@ -412,15 +412,11 @@ impl VirtualMachine {
                 match &operands.res {
                     None => return Err(VirtualMachineError::UnconstrainedResAssertEq),
                     Some(res) => {
-                        if let (MaybeRelocatable::Int(res_num), MaybeRelocatable::Int(dst_num)) =
-                            (res, &operands.dst)
-                        {
-                            if res_num != dst_num {
-                                return Err(VirtualMachineError::DiffAssertValues(
-                                    dst_num.clone(),
-                                    res_num.clone(),
-                                ));
-                            };
+                        if res != &operands.dst {
+                            return Err(VirtualMachineError::DiffAssertValues(
+                                operands.dst.clone(),
+                                res.clone(),
+                            ));
                         };
                     }
                 };
@@ -2509,8 +2505,43 @@ mod tests {
         assert_eq!(
             vm.opcode_assertions(&instruction, &operands),
             Err(VirtualMachineError::DiffAssertValues(
-                bigint!(9),
-                bigint!(8)
+                MaybeRelocatable::from(bigint!(9)),
+                MaybeRelocatable::from(bigint!(8))
+            ))
+        );
+    }
+
+    #[test]
+    fn opcode_assertions_instruction_failed_relocatables() {
+        let instruction = Instruction {
+            off0: 1,
+            off1: 2,
+            off2: 3,
+            imm: None,
+            dst_register: Register::FP,
+            op0_register: Register::AP,
+            op1_addr: Op1Addr::AP,
+            res: Res::Add,
+            pc_update: PcUpdate::Regular,
+            ap_update: ApUpdate::Regular,
+            fp_update: FpUpdate::APPlus2,
+            opcode: Opcode::AssertEq,
+        };
+
+        let operands = Operands {
+            dst: MaybeRelocatable::from((1, 1)),
+            res: Some(MaybeRelocatable::from((1, 2))),
+            op0: MaybeRelocatable::Int(bigint!(9)),
+            op1: MaybeRelocatable::Int(bigint!(10)),
+        };
+
+        let vm = vm!();
+
+        assert_eq!(
+            vm.opcode_assertions(&instruction, &operands),
+            Err(VirtualMachineError::DiffAssertValues(
+                MaybeRelocatable::from((1, 1)),
+                MaybeRelocatable::from((1, 2))
             ))
         );
     }