Add Function Math.abs

[barakman]

🧐 Motivation
A function which returns the absolute (and obviously unsigned) value of a signed value.

📝 Details

Here is the full work (as I am not authorized to post a Pull Request on your repository):

File Math.sol:

    /**
     * @dev Returns the absolute unsigned value of a signed value.
     */
    function abs(int256 n) internal pure returns (uint256) {
        unchecked {
            // must be unchecked in order to support `n = type(int256).min`
            return uint256(n >= 0 ? n : -n);
        }
    }

File MathMock.sol:

    function abs(int256 n) public pure returns (uint256) {
        return Math.abs(n);
    }

File Math.test.js:

const { MAX_UINT256, MAX_INT256, MIN_INT256 } = constants;
...
  describe('abs', function () {
    for (const n of [MIN_INT256, MIN_INT256.addn(1), new BN('-1'), new BN('0'), new BN('1'), MAX_INT256.subn(1), MAX_INT256]) {
      it(`correctly computes the absolute value of ${n}`, async function () {
        expect(await this.math.abs(n)).to.be.bignumber.equal(n.abs());
      });
    }
  });
...

[Amxx]

Hello @barakman and thank you for this request.

I'm not puzzled about what the return type should be. On the one end returning int256 would allow doing something like int256 y = -x.abs(); but on the other hand it would causes overflow when doing type(int256).min.abs()

For the record, I'm not sure you implementation would work with type(int256).min, this would have to be tested, or documented.

[barakman]

@Amxx:

1. It doesn't make sense to return an int256 for a value which is always non-negative.
2. It is not even possible, because abs(-2**255) == 2**255, which cannot be represented with int256.
3. Everything is tested, including abs(type(int256).min); please have a look at the (JS) code that I posted.

Thanks

[Amxx]

While it is true that abs will always return a positive value, I'm curious about actual use cases, and the fact that it might be used as part or signed arithmetic. I fear this might be seen as implicit casting and be confusing.

Maybe that is just me.

[barakman]

@Amxx: IMHO, function abs should not be used for signed arithmetic by its very definition.

With regards to the specific example that you've given:

int256 y = -x.abs();

The correct thing to do would be:

using SafeCast for uint256;
int256 y = -x.abs().toInt256();

Which will revert for x == type(int256).min at the call to toInt256, but work correctly for all other values.

TBH, for this specific example, I wouldn't even use abs to begin with, but rather, implement something like:

function neg(int256 n) internal pure returns (int256) {
    return n <= 0 ? n : -n;
}

And you can overload that function with:

function neg(uint256 n) internal pure returns (int256) {
    return -n.toInt256();
}

The first function will never revert, the second function will revert for x > type(int256).max.

[Amxx]

Interresting,

maybe, there is a case for adding both abs() and neg() into the Math library

[frangio]

@barakman You should be able to fork the repository and open a pull request. Are you not authorized to do this?

[barakman]

@frangio : That I haven't tried (not since v2.0.0 anyway). I'll give it a try...

[barakman]

@frangio: Done (PR #2984).

Note that I had a mistake in a comment in my original post:

// must be unchecked in order to support `n = type(int256).max`

Which I now fixed to:

// must be unchecked in order to support `n = type(int256).min`

[heldersepu]

This should be closed PR was merged