Conversation
SIERRA_UPDATE_NO_CHANGE_TAG=Sim change only.
|
This change is |
PR SummaryLow Risk Overview On the overflow branch (branch 1), the simulator now splits the value with Adds branch tests for a small felt ( Reviewed by Cursor Bugbot for commit c795932. Bugbot is set up for automated code reviews on this repo. Configure here. |
TomerStarkware
left a comment
There was a problem hiding this comment.
@TomerStarkware reviewed 2 files and all commit messages, and made 1 comment.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on eytan-starkware).
Summary
Fixes the simulation of
u128s_from_felt252to correctly return both the high and low 128-bit components when the inputFelt252value does not fit in au128. Previously, the failure branch returned only theRangeCheckvalue with no decomposition. Now, when the value exceedsu128::MAX, it is split viadiv_remby2^128into a high part and a low part, both returned asUint128values alongsideRangeCheck.Type of change
Please check one:
Why is this change needed?
The simulation of
u128s_from_felt252was incomplete: when the felt252 value was too large to fit in au128, the failure branch only returnedRangeCheckwithout providing the high and low decomposedu128values. This made the simulation diverge from the actual semantics of the libfunc, which decomposes the value into(high, low)parts.What was the behavior or documentation before?
When
u128s_from_felt252was simulated with a value that does not fit in au128, the failure branch returnedvec![CoreValue::RangeCheck]with no decomposition of the input value.What is the behavior or documentation after?
The failure branch now computes
(high, low) = value / 2^128and returnsvec![CoreValue::RangeCheck, CoreValue::Uint128(high), CoreValue::Uint128(low)], matching the expected libfunc semantics. Tests covering both the success case (e.g.,5) and the failure case (e.g.,3 * 2^128 + 7) are added.Related issue or discussion (if any)
N/A
Additional context
N/A