Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: MULMOD failing on modulus 0 #694

Closed
Tracked by #28
greged93 opened this issue Aug 24, 2023 · 0 comments · Fixed by #864
Closed
Tracked by #28

bug: MULMOD failing on modulus 0 #694

greged93 opened this issue Aug 24, 2023 · 0 comments · Fixed by #864
Assignees

Comments

@greged93
Copy link
Contributor

Bug Report

Kakarot version: ee6458a

Current behavior:
Any call to the MULMOD opcode with a modulus value of 0 will fail, due to the fact that we are calling the uint256_mul_div_mod which doesn't accept 0 as a div value.

// Stack input:
// 0 - a: number.
// 1 - b: number.
// 1 - c: modulos.
let stack = ctx.stack;
let (stack, popped) = Stack.pop_n(self=stack, n=3);
let a = popped[0];
let b = popped[1];
let c = popped[2];
// Compute the mul mod
let (_, _, rem) = uint256_mul_div_mod(a, b, c);

Expected behavior:
Given 0 as the modulus value, the returned value should always be 0, whatever the value for first and second stack value (a and b in the above snippet).

Steps to reproduce:

RUN_SPECIFIC_TESTS=mulmod_d12g0v0_Shanghai cargo test -p ef-testing --features ef-tests -- --nocapture

all the below tests will fail due to the modulus value:
https://github.com/ethereum/tests/blob/develop/src/GeneralStateTestsFiller/VMTests/vmArithmeticTest/mulmodFiller.yml#L178-L222

Failing output should be:

thread 'blockchain_tests::vm_tests' panicked at 'attempt to divide by zero', ~/.cargo/registry/src/index.crates.io-6f17d22bba15001f/num-bigint-0.4.3/src/biguint/division.rs:168:9
[!] Case ~/code/rust/ef-tests/crates/ef-testing/ethereum-tests/BlockchainTests/GeneralStateTests/VMTests/vmArithmeticTest/mulmod.json failed (description: mulmod): Test failed: error sending request for url (http://[::1]:52181/): connection closed before message completed
@github-project-automation github-project-automation bot moved this to 🆕 Backlog in Kakarot on Starknet Aug 24, 2023
@Eikix Eikix added this to the Official Ethereum Conformance milestone Aug 29, 2023
@ClementWalter ClementWalter moved this from 🆕 Backlog to 🔖 Ready in Kakarot on Starknet Dec 14, 2023
@ClementWalter ClementWalter self-assigned this Jan 8, 2024
ClementWalter added a commit that referenced this issue Jan 8, 2024
Time spent on this PR: 0.2

## Pull request type

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?

Resolves #694 

## What is the new behavior?

Passing
@github-project-automation github-project-automation bot moved this from 🔖 Ready to ✅ Done in Kakarot on Starknet Jan 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants