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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Math.sqrt #3149

Closed
Dentrax opened this issue Jan 29, 2022 · 3 comments 路 Fixed by #3242
Closed

Add Math.sqrt #3149

Dentrax opened this issue Jan 29, 2022 · 3 comments 路 Fixed by #3242

Comments

@Dentrax
Copy link

Dentrax commented Jan 29, 2022

馃 Motivation
It would be nice to have. Currently, we have to write our "custom" function that we can find the logic on the internet. But I'm not so sure whether those are "safe" to use.

馃摑 Details
Something like:

https://github.com/Uniswap/v2-core/blob/4dd59067c76dea4a0e8e4bfdda41877a6b16dedc/contracts/libraries/Math.sol#L11

math.Sqrt(16)
function sqrt(uint x) constant returns (uint y) {
    uint z = (x + 1) / 2;
    y = x;
    while (z < y) {
        y = z;
        z = (x / z + z) / 2;
    }

In OpenZeppelin way:

   function sqrt(uint y) internal pure returns (uint z) {
        if (y > 3) {
            z = y;
            uint x = (y.div(2)).add(1);
            while (x < z) {
                z = x;
                x = ((y.div(x)).add(x)).div(2);
            }
        } else if (y != 0) {
            z = 1;
        }
    }
@frangio
Copy link
Contributor

frangio commented Jan 29, 2022

Thanks for the request @Dentrax!

The reason we don't have this function currently is that it's not constant time. But I think we should take a closer look to see if there is real risk of running out of gas in the worst case, and what is the "average" cost.

It would be interesting to see smart contracts in production that are using sqrt to see if they took any special precautions and what particular implementations they used.

@Amxx
Copy link
Collaborator

Amxx commented Jan 30, 2022

@frangio UniswapV2 pairs use sqrt.

@frangio
Copy link
Contributor

frangio commented Jan 31, 2022

O(1) implementation of pow in Balancer, can be used as sqrt with y = 0.5:

https://github.com/balancer-labs/balancer-v2-monorepo/blob/master/pkg/solidity-utils/contracts/math/LogExpMath.sol#L89

@frangio frangio changed the title Consider add Sqrt() in SafeMath Add Math.sqrt Feb 1, 2022
@frangio frangio mentioned this issue Mar 22, 2022
3 tasks
@Amxx Amxx closed this as completed in #3242 Jun 7, 2022
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

Successfully merging a pull request may close this issue.

3 participants