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: felt_to_bytes_little is underconstrained #1300

Closed
ClementWalter opened this issue Jul 24, 2024 · 2 comments · Fixed by #1317
Closed

bug: felt_to_bytes_little is underconstrained #1300

ClementWalter opened this issue Jul 24, 2024 · 2 comments · Fixed by #1317
Assignees
Labels

Comments

@ClementWalter
Copy link
Member

ClementWalter commented Jul 24, 2024

Bug Report

Kakarot version

The individual [output]``s are not constrained to be modulo base`.

let byte = [output];

@obatirou
Copy link
Collaborator

obatirou commented Jul 30, 2024

POC

Change the hint from

%{
   memory[ids.output] = res = (int(ids.value) % PRIME) % ids.base
   assert res < ids.bound, f'split_int(): Limb {res} is out of range.'
%}
%{
   memory[ids.output] = res = (int(ids.value) % PRIME)
%}

Fix

As the input value can be PRIME - 1, it is not possible to use unsigned_div_rem as the quotient and reminder are range checked to 2**128

I see 2 solutions:

  • transform the value passed to Uint256 and manipulate Uint256 by using uint256_unsigned_div_rem
  • reconstruct the initial input felt value from the outputs with a loop ensuring the decomposition through the hint was genuine

The first one might be more expensive due to the necessary range check implicit param + uint256 manipulation hence we might go with the second one.

@obatirou
Copy link
Collaborator

obatirou commented Jul 30, 2024

A third solution was suggested by Zellic by adding 2 constraints:

  • require [output] to be less than the base
  • require the length of the output to be the minimum required to represent the input

The second part can be calculated thank to split_felt(value) and use bytes_used_128 accordingly on this representation.
This solution seems a good compromise avoiding manipulating Uint256 throughout the decomposition or avoiding overflow check that would need to be implemented for the loop solution.

matthieuauger pushed a commit to matthieuauger/kakarot that referenced this issue Nov 9, 2024
* feat: statically-fetched-env

* fix: statically-fetched-env
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants