Conversation
SIERRA_UPDATE_NO_CHANGE_TAG=Simulation change only.
PR SummaryLow Risk Overview Adds simulation tests for a small product and for Reviewed by Cursor Bugbot for commit 6997545. 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 3 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
u128_guarantee_mulsimulation to correctly compute the high and low 128-bit limbs of a 128×128-bit multiplication. The division was previously usingBigInt::one().pow(128)(which equals1, not2^128) instead ofBigInt::one() << 128, causing incorrect limb splitting. Variable names are also updated fromlimb0/limb1tohigh/lowto match their semantic meaning, and the output order is corrected to[high, low, U128MulGuarantee]. Test cases are added to verify both the no-overflow case and the case where the product overflows into the high limb.Type of change
Please check one:
Why is this change needed?
BigInt::one().pow(128)evaluates to1(one raised to the power of 128), not2^128. This meant thediv_remwas dividing by1, producing a remainder of0and a quotient equal to the full product — making the high/low limb split completely wrong for any non-trivial multiplication.What was the behavior or documentation before?
The simulation of
u128_guarantee_mulproduced incorrect high and low limb values due to the division being performed against1instead of2^128.What is the behavior or documentation after?
The simulation correctly splits the 256-bit product of two
u128values into a high 128-bit limb and a low 128-bit limb using a bitshift (<< 128) for the divisor, and returns them in the order[high, low, U128MulGuarantee].Related issue or discussion (if any)
N/A
Additional context
U128MulGuaranteeis also registered in the bijective type mapping intest_utils.rsto support the new simulation tests.