From 98815203e21f8968ab620b6a5190fbb99f9c7f46 Mon Sep 17 00:00:00 2001
From: Ivan Litteri <67517699+ilitteri@users.noreply.github.com>
Date: Wed, 27 Nov 2024 15:35:05 -0300
Subject: [PATCH] fix(levm): `smod` non-compliance (#1305)

Resolves #1291
---
 .../vm/levm/src/opcode_handlers/arithmetic.rs | 53 ++++++++++---------
 crates/vm/levm/tests/edge_case_tests.rs       | 12 +++++
 crates/vm/levm/tests/tests.rs                 |  7 ++-
 3 files changed, 45 insertions(+), 27 deletions(-)

diff --git a/crates/vm/levm/src/opcode_handlers/arithmetic.rs b/crates/vm/levm/src/opcode_handlers/arithmetic.rs
index 5df5177df..0b05f8ba2 100644
--- a/crates/vm/levm/src/opcode_handlers/arithmetic.rs
+++ b/crates/vm/levm/src/opcode_handlers/arithmetic.rs
@@ -134,28 +134,32 @@ impl VM {
     ) -> Result<OpcodeSuccess, VMError> {
         self.increase_consumed_gas(current_call_frame, gas_cost::SMOD)?;
 
-        let dividend = current_call_frame.stack.pop()?;
-        let divisor = current_call_frame.stack.pop()?;
-        if divisor.is_zero() {
+        let unchecked_dividend = current_call_frame.stack.pop()?;
+        let unchecked_divisor = current_call_frame.stack.pop()?;
+
+        if unchecked_divisor.is_zero() {
             current_call_frame.stack.push(U256::zero())?;
-        } else {
-            let normalized_dividend = abs(dividend);
-            let normalized_divisor = abs(divisor);
-
-            let mut remainder =
-                normalized_dividend
-                    .checked_rem(normalized_divisor)
-                    .ok_or(VMError::Internal(
-                        InternalError::ArithmeticOperationDividedByZero,
-                    ))?; // Cannot be zero bc if above;
-
-            // The remainder should have the same sign as the dividend
-            if is_negative(dividend) {
-                remainder = negate(remainder);
+            return Ok(OpcodeSuccess::Continue);
+        }
+
+        let divisor = abs(unchecked_divisor);
+        let dividend = abs(unchecked_dividend);
+
+        let unchecked_remainder = match dividend.checked_rem(divisor) {
+            Some(remainder) => remainder,
+            None => {
+                current_call_frame.stack.push(U256::zero())?;
+                return Ok(OpcodeSuccess::Continue);
             }
+        };
 
-            current_call_frame.stack.push(remainder)?;
-        }
+        let remainder = if is_negative(unchecked_dividend) {
+            negate(unchecked_remainder)
+        } else {
+            unchecked_remainder
+        };
+
+        current_call_frame.stack.push(remainder)?;
 
         Ok(OpcodeSuccess::Continue)
     }
@@ -294,6 +298,11 @@ fn is_negative(value: U256) -> bool {
 }
 
 /// Negates a number in two's complement
+fn negate(value: U256) -> U256 {
+    let (dividend, _overflowed) = (!value).overflowing_add(U256::one());
+    dividend
+}
+
 fn abs(value: U256) -> U256 {
     if is_negative(value) {
         negate(value)
@@ -301,9 +310,3 @@ fn abs(value: U256) -> U256 {
         value
     }
 }
-
-/// Negates a number in two's complement
-fn negate(value: U256) -> U256 {
-    let inverted = !value;
-    inverted.saturating_add(U256::one())
-}
diff --git a/crates/vm/levm/tests/edge_case_tests.rs b/crates/vm/levm/tests/edge_case_tests.rs
index c071bde30..b4a522840 100644
--- a/crates/vm/levm/tests/edge_case_tests.rs
+++ b/crates/vm/levm/tests/edge_case_tests.rs
@@ -225,3 +225,15 @@ fn test_non_compliance_codecopy() {
         &U256::zero()
     );
 }
+
+#[test]
+fn test_non_compliance_smod() {
+    let mut vm =
+        new_vm_with_bytecode(Bytes::copy_from_slice(&[0x60, 1, 0x60, 1, 0x19, 0x07])).unwrap();
+    let mut current_call_frame = vm.call_frames.pop().unwrap();
+    vm.execute(&mut current_call_frame);
+    assert_eq!(
+        current_call_frame.stack.stack.first().unwrap(),
+        &U256::zero()
+    );
+}
diff --git a/crates/vm/levm/tests/tests.rs b/crates/vm/levm/tests/tests.rs
index 06fef1b97..24b81cd64 100644
--- a/crates/vm/levm/tests/tests.rs
+++ b/crates/vm/levm/tests/tests.rs
@@ -176,7 +176,10 @@ fn smod_op() {
     let mut current_call_frame = vm.call_frames.pop().unwrap();
     vm.execute(&mut current_call_frame);
 
-    assert!(vm.current_call_frame_mut().unwrap().stack.pop().unwrap() == U256::from(1));
+    assert_eq!(
+        vm.current_call_frame_mut().unwrap().stack.pop().unwrap(),
+        U256::one()
+    );
 
     // Second Example
     // Example taken from evm.codes
@@ -209,7 +212,7 @@ fn smod_op() {
     )
     .unwrap();
 
-    assert!(vm.current_call_frame_mut().unwrap().stack.pop().unwrap() == c);
+    assert_eq!(vm.current_call_frame_mut().unwrap().stack.pop().unwrap(), c);
 }
 
 #[test]