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

✨ add toUint256(int256) to SafeCastLib #511

Closed
0xClandestine opened this issue Jul 24, 2023 · 6 comments
Closed

✨ add toUint256(int256) to SafeCastLib #511

0xClandestine opened this issue Jul 24, 2023 · 6 comments

Comments

@0xClandestine
Copy link

Noticed both are missing:

Thinking we would check that each param is greater than zero:

function toUint256(int256 x) internal pure returns (uint256) {
    if (x < 0) revert FailedConversion();
    return uint256(x);
}
@0xClandestine
Copy link
Author

Wouldn't hurt to add complementWad as well:

function complementWad(uint256 x) internal pure returns (uint256 result) {
    assembly {
        result := mul(lt(x, WAD), sub(WAD, x))
    }
}

@0xClandestine 0xClandestine changed the title ✨ add toUint256(int256) and powWad(uint256, uint256) to SafeCastLib ✨ add toUint256(int256) to SafeCastLib Jul 24, 2023
@0xClandestine
Copy link
Author

powWad(uint256, uint256) could also be added to FixedPointMathLib

@Vectorized
Copy link
Owner

I think let's leave it as powWad(int256 x, int256 y).

@0xClandestine
Copy link
Author

@Vectorized All of this is inspired by Balancer v2 arithmetic:

Complement reference:
https://github.com/balancer/balancer-v2-monorepo/blob/bc3b3fee6e13e01d2efe610ed8118fdb74dfc1f2/pkg/solidity-utils/contracts/math/FixedPoint.sol#L156

Furthermore, for powWad we only need to ensure that the signed integers (params) are greater than zero. Although my main issue is with the return value, which requires me to check that result > 0. So tbh its not a huge issue.

@0xClandestine
Copy link
Author

I think let's leave it as powWad(int256 x, int256 y).

powWad(int256, int256) -> (uint256) could be a happy median

@Vectorized Vectorized mentioned this issue Jul 24, 2023
3 tasks
@Vectorized
Copy link
Owner

Vectorized commented Jul 25, 2023

I think I'll let the people who need powWad and wadComplement copy paste from balancer.

Licensing issue, and high risk of edge cases unaccounted for.

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

No branches or pull requests

2 participants