From 2dafae164b66547b43ec9006b4e8be80ae6d56fd Mon Sep 17 00:00:00 2001
From: Mathieu <60658558+enitrat@users.noreply.github.com>
Date: Mon, 8 Jan 2024 18:08:34 +0100
Subject: [PATCH] fix: addmod impl (#865)

<!--- Please provide a general summary of your changes in the title
above -->

<!-- Give an estimate of the time you spent on this PR in terms of work
days.
Did you spend 0.5 days on this PR or rather 2 days?  -->

Time spent on this PR: 0.5d

## Pull request type

<!-- Please try to limit your pull request to one type,
submit multiple pull requests if needed. -->

Please check the type of change your PR introduces:

- [x] Bugfix
- [ ] Feature
- [ ] Code style update (formatting, renaming)
- [ ] Refactoring (no functional changes, no api changes)
- [ ] Build related changes
- [ ] Documentation content changes
- [ ] Other (please describe):

## What is the current behavior?

<!-- Please describe the current behavior that you are modifying,
or link to a relevant issue. -->

Resolves #695 Resolves #691

## What is the new behavior?

<!-- Please describe the behavior or changes that are being added by
this PR. -->

- Fixed the addmod implementation
-
-
---
 blockchain-tests-skip.yml                     |  4 ----
 .../src/PlainOpcodes/PlainOpcodes.sol         |  4 ++++
 .../stop_and_math_operations.cairo            | 22 ++++++++++++++-----
 .../PlainOpcodes/test_plain_opcodes.py        |  4 ++++
 4 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/blockchain-tests-skip.yml b/blockchain-tests-skip.yml
index 76ee39d37..6844d5cf0 100644
--- a/blockchain-tests-skip.yml
+++ b/blockchain-tests-skip.yml
@@ -9,10 +9,6 @@ testname:
     - push0Gas_d0g0v0_Shanghai
     - push0_d1g0v0_Shanghai
   vmArithmeticTest:
-    - addmod_d11g0v0_Shanghai
-    - addmod_d10g0v0_Shanghai
-    - addmod_d9g0v0_Shanghai
-    - addmod_d8g0v0_Shanghai
     - divByZero_d63g0v0_Shanghai
     - divByZero_d64g0v0_Shanghai
     - divByZero_d65g0v0_Shanghai
diff --git a/solidity_contracts/src/PlainOpcodes/PlainOpcodes.sol b/solidity_contracts/src/PlainOpcodes/PlainOpcodes.sol
index 182478065..0e2d19be6 100644
--- a/solidity_contracts/src/PlainOpcodes/PlainOpcodes.sol
+++ b/solidity_contracts/src/PlainOpcodes/PlainOpcodes.sol
@@ -194,6 +194,10 @@ contract PlainOpcodes {
         return mulmod(type(uint256).max, type(uint256).max, type(uint256).max);
     }
 
+    function addmodMax() public pure returns (uint) {
+        return addmod(type(uint256).max, type(uint256).max, type(uint256).max);
+    }
+
     receive() external payable {}
     fallback() external payable {}
 }
diff --git a/src/kakarot/instructions/stop_and_math_operations.cairo b/src/kakarot/instructions/stop_and_math_operations.cairo
index 6cc91cef7..73afe572c 100644
--- a/src/kakarot/instructions/stop_and_math_operations.cairo
+++ b/src/kakarot/instructions/stop_and_math_operations.cairo
@@ -224,12 +224,24 @@ namespace StopAndMathOperations {
         let range_check_ptr = [ap - 2];
         let popped = cast([ap - 1], Uint256*);
 
-        let (sum, _) = uint256_add(popped[0], popped[1]);
-        let (_, remainder) = uint256_unsigned_div_rem(sum, popped[2]);
+        tempvar mod_is_not_zero = popped[2].low + popped[2].high;
+        jmp addmod_not_zero if mod_is_not_zero != 0;
 
         tempvar bitwise_ptr = cast([fp - 7], BitwiseBuiltin*);
         tempvar range_check_ptr = range_check_ptr;
-        tempvar result = Uint256(remainder.low, remainder.high);
+        tempvar result = Uint256(0, 0);
+        jmp end;
+
+        addmod_not_zero:
+        // (a + b) mod n  = (a mod n + b mod n)mod n
+        let (_, a_mod_n) = uint256_unsigned_div_rem(popped[0], popped[2]);
+        let (_, b_mod_n) = uint256_unsigned_div_rem(popped[1], popped[2]);
+        let (sum, carry) = uint256_add(a_mod_n, b_mod_n);
+        let (_, result) = uint256_unsigned_div_rem(sum, popped[2]);
+        tempvar bitwise_ptr = cast([fp - 7], BitwiseBuiltin*);
+        tempvar range_check_ptr = range_check_ptr;
+        tempvar result = result;
+
         jmp end;
 
         MULMOD:
@@ -237,14 +249,14 @@ namespace StopAndMathOperations {
         let popped = cast([ap - 1], Uint256*);
 
         tempvar mod_is_not_zero = popped[2].low + popped[2].high;
-        jmp not_zero if mod_is_not_zero != 0;
+        jmp mulmod_not_zero if mod_is_not_zero != 0;
 
         tempvar bitwise_ptr = cast([fp - 7], BitwiseBuiltin*);
         tempvar range_check_ptr = range_check_ptr;
         tempvar result = Uint256(0, 0);
         jmp end;
 
-        not_zero:
+        mulmod_not_zero:
         let (_, _, result) = uint256_mul_div_mod(popped[0], popped[1], popped[2]);
 
         tempvar bitwise_ptr = cast([fp - 7], BitwiseBuiltin*);
diff --git a/tests/end_to_end/PlainOpcodes/test_plain_opcodes.py b/tests/end_to_end/PlainOpcodes/test_plain_opcodes.py
index b4a48ab7e..a293fb686 100644
--- a/tests/end_to_end/PlainOpcodes/test_plain_opcodes.py
+++ b/tests/end_to_end/PlainOpcodes/test_plain_opcodes.py
@@ -446,3 +446,7 @@ async def test_should_revert_on_fallbacks(
     class TestMulmod:
         async def test_should_return_0(self, plain_opcodes):
             assert 0 == await plain_opcodes.mulmodMax()
+
+    class TestAddmod:
+        async def test_should_return_0(self, plain_opcodes):
+            assert 0 == await plain_opcodes.addmodMax()