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

Return overflowing value when multiplying for UInt256 and Int256 #2389

Closed
Tracked by #1972
quocle108 opened this issue Mar 16, 2023 · 5 comments · Fixed by #2510
Closed
Tracked by #1972

Return overflowing value when multiplying for UInt256 and Int256 #2389

quocle108 opened this issue Mar 16, 2023 · 5 comments · Fixed by #2510

Comments

@quocle108
Copy link

Issue to be solved

There are some cases in which we need to get the overflowing value instead of the maximum value when the overflow happens. For example this case mulDiv(uint256 x, uint256 y, uint256 denominator).

In order to multiply a and b and get the result in 512 bits, we need to get the overflowing value when it happens.

Suggested Solution

No response

@j1010001 j1010001 added the Good First Issue Good for newcomers label Mar 23, 2023
@turbolent turbolent removed their assignment Mar 23, 2023
@j1010001 j1010001 added the P-Low label Mar 23, 2023
@turbolent
Copy link
Member

@turbolent turbolent changed the title Return overflowing value when for UInt256 and Int256 Return overflowing value when multiplying for UInt256 and Int256 May 2, 2023
@darkdrag00nv2
Copy link
Contributor

@turbolent If I understand correctly, we want to implement Word128 and Word256?

The title of this issue is a little confusing. We wouldn't want to change the behavior of UInt256 and Int256 I presume. They should still raise runtime errors on overflow. Please correct me if I am wrong.

@turbolent
Copy link
Member

turbolent commented May 3, 2023

@darkdrag00nv2 Good question and good idea!

I initially thought it might make sense to add a "modular multiplication" operation ((x * y) % k; like mulmod in Solidity).

Though maybe there is no need for arbitrary k here, in which case, like you pointed out, Word* types already wrap around, so we would just need to add Word256 (and maybe Word128 while at it / for consistency).

Integer values can be converted between each other, so UInt256 would then also be convertible to Word256.

@quocle108 Would that be sufficient?

@turbolent turbolent reopened this May 3, 2023
@darkdrag00nv2
Copy link
Contributor

Thanks @turbolent

I think we can implement Word128 and Word256 for starters. We can add more functions further if it is insufficient for @quocle108.

@turbolent
Copy link
Member

@darkdrag00nv2 Sounds good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants