openzeppelin-contracts icon indicating copy to clipboard operation
openzeppelin-contracts copied to clipboard

Add `signum` Function to `SignedMath`

Open pcaversaccio opened this issue 1 year ago • 7 comments

I was wondering, whether OZ should offer a proper signum (sign might be too confusing due to cryptographic functions) function that returns the indication (-1, 0, or 1) of the sign of a 32-byte signed integer. Thoughts?

Example Implementation

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;

library SignedMath {
    /**
     * @dev Returns the indication of the sign of a 32-byte signed integer.
     * @notice The function returns `-1` if `x < 0`, `0` if `x == 0`, and `1` if `x > 0`.
     * For more details on finding the sign of a signed integer, please refer to:
     * https://graphics.stanford.edu/~seander/bithacks.html#CopyIntegerSign.
     */
    function signum(int256 a) internal pure returns (int256 b) {
        /// @solidity memory-safe-assembly
        assembly {
            b := sub(sgt(a, 0), slt(a, 0))
        }
    }
}

pcaversaccio avatar Dec 05 '23 08:12 pcaversaccio

nice

Tigersame avatar Dec 05 '23 16:12 Tigersame

@pcaversaccio What are the use cases for that function? Why return a int256 and not an Enum ?

Amxx avatar Dec 13 '23 10:12 Amxx

@pcaversaccio What are the use cases for that function? Why return a int256 and not an Enum ?

I remember some Curve code that ensures that a partial derivative must be negative and thereafter multiplies it by the sign (-1) to make it again a positive number since they needed the abs value. About the int256 vs. Enum: good question, first, I thought to follow the conventions from other programming languages / frameworks that use the -1, 0, 1 (not all define 0 as a separate return) (see e.g. numpy.sign). Second, an Enum is an uint8 under the hood and if you're interested in the sign, you will work on the int domain usually. So it naturally translates into the same type domain.

pcaversaccio avatar Dec 13 '23 10:12 pcaversaccio

I remember some Curve code that ensures that a partial derivative must be negative and thereafter multiplies it by the sign (-1) to make it again a positive number since they needed the abs value.

I'm not sure signum really helps with that. You can check that the value is negative by directly comparing it 0 ... and if that passes you negate it.

I don't have any stong opinion against that proposal, but I also want to make sure we don't add code would end up unused, and that would affect the discoverability of the other code.

Amxx avatar Dec 13 '23 16:12 Amxx

I can't find this helper in PRBMath. I would expect people to need that for the actual math that PRBMath addresses before they need that for the "basic" math we provide.

Amxx avatar Dec 13 '23 16:12 Amxx

I can't find this helper in PRBMath. I would expect people to need that for the actual math that PRBMath addresses before they need that for the "basic" math we provide.

That makes perfect sense to me, that's why I opened an issue for discussion first (instead of directly doing a PR). Let's see if it's added to PRBMath first.

pcaversaccio avatar Dec 13 '23 16:12 pcaversaccio

I remember some Curve code that ensures that a partial derivative must be negative and thereafter multiplies it by the sign (-1) to make it again a positive number since they needed the abs value.

Generally, the signum function should not be used to get an absolute positive or negative value, the abs function exists for the purpose.

The signum may be needed in the case your equation depends only on a sign of the variable, not the variable itself. The only signum usecase I remember is sin(θ) = sign(cos(θ − π/2)) * sqrt(1 - cos(θ)^2), here it is used to not deal with an unknown sign during equation transformations.

SteMak avatar Jan 05 '24 19:01 SteMak