-
Notifications
You must be signed in to change notification settings - Fork 23
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
fixed bitlen calc #18
Conversation
// if(msword==1 || msword>>(max_bitlen % 256)==1): | ||
if or( eq(msword, 1), eq(shr(mod(max_bitlen,256),msword),1) ) { | ||
// if(carry==1 || msword>>(max_bitlen % 256)==1): | ||
if or( eq(carry, 1), eq(shr(mod(max_bitlen,256),msword),1) ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That should be fairly simple to illustrate with an example, right ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Say we have two normalized big numbers a = [a2, a1]
and b = [b1]
.
a1 = 0000000000000000000000000000000000000000000000000000000000000001
a2 = 0000000000000000000000000000000000000000000000000000000000000001
b1 = 0000000000000000000000000000000000000000000000000000000000000001
The result would be r = [r2, r1] = [a2, a1 + b1]
.
r1 = 0000000000000000000000000000000000000000000000000000000000000002
r2 = 0000000000000000000000000000000000000000000000000000000000000001
Here you set the most significant word of the result to 1
if an overflow occurs. But in some cases msword
equals 1
even without an overflow, as shown in the example (r2
). This is why I suggest using carry
instead of msword
to determine whether to increase the bit length.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, thanks for your contribution!
would you mind adding a unit test (BigNumbers.t.sol
) implementing this example?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@riordant Done
1e1aee1
to
f382bcf
Compare
@dovgopoly merged :) |
Hi, I noticed a potential bug in the
_add
function when recalculatingbitlen
. Consider a scenario where the most significant word is already 1 and remains unchanged. In this case, the algorithm incorrectly calculates the resultbitlen
.The result
bitlen
is 258 but it's expected to be 257.