Move abs(int256) from Math to SafeMath (#3110)

Co-authored-by: Francisco Giordano <frangio.1@gmail.com>
(cherry picked from commit 3eb2d43b0610758c6b85bf4b8ae160db3679c34d)
Signed-off-by: Hadrien Croubois <hadrien.croubois@gmail.com>

CHANGELOG.md

@@ -4,7 +4,6 @@
 
  * `ERC2891`: add implementation of the royalty standard, and the respective extensions for `ERC721` and `ERC1155`. ([#3012](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3012))
  * `GovernorTimelockControl`: improve the `state()` function to have it reflect cases where a proposal has been canceled directly on the timelock. ([#2977](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2977))
- * `Math`: add a `abs(int256)` method that returns the unsigned absolute value of a signed value. ([#2984](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2984))
  * Preset contracts are now deprecated in favor of [Contracts Wizard](https://wizard.openzeppelin.com). ([#2986](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2986))
  * `Governor`: add a relay function to help recover assets sent to a governor that is not its own executor (e.g. when using a timelock). ([#2926](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2926))
  * `GovernorPreventLateQuorum`: add new module to ensure a minimum voting duration is available after the quorum is reached. ([#2973](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2973))
@@ -17,6 +16,7 @@
  * `ERC20`: do not update allowance on `transferFrom` when allowance is `type(uint256).max`. ([#3085](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/#3085))
  * `ERC777`: do not update allowance on `transferFrom` when allowance is `type(uint256).max`. ([#3085](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/#3085))
  * `SignedMath`: a new signed version of the Math library with `max`, `min`,  and `average`. ([#2686](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2686))
+ * `SignedMath`: add a `abs(int256)` method that returns the unsigned absolute value of a signed value. ([#2984](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2984))
  * `ERC1967Upgrade`: Refactor the secure upgrade to use `ERC1822` instead of the previous rollback mechanism. This reduces code complexity and attack surface with similar security guarantees. ([#3021](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3021))
  * `UUPSUpgradeable`: Add `ERC1822` compliance to support the updated secure upgrade mechanism. ([#3021](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3021))
  * Some more functions have been made virtual to customize them via overrides. In many cases this will not imply that other functions in the contract will automatically adapt to the overridden definitions. People who wish to override should consult the source code to understand the impact and if they need to override any additional functions to achieve the desired behavior.

contracts/mocks/MathMock.sol

@@ -20,8 +20,4 @@ contract MathMock {
     function ceilDiv(uint256 a, uint256 b) public pure returns (uint256) {
         return Math.ceilDiv(a, b);
     }
-
-    function abs(int256 n) public pure returns (uint256) {
-        return Math.abs(n);
-    }
 }

contracts/mocks/SignedMathMock.sol

@@ -16,4 +16,8 @@ contract SignedMathMock {
     function average(int256 a, int256 b) public pure returns (int256) {
         return SignedMath.average(a, b);
     }
+
+    function abs(int256 n) public pure returns (uint256) {
+        return SignedMath.abs(n);
+    }
 }

contracts/utils/math/Math.sol

@@ -40,14 +40,4 @@ library Math {
         // (a + b - 1) / b can overflow on addition, so we distribute.
         return a / b + (a % b == 0 ? 0 : 1);
     }
-
-    /**
-     * @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);
-        }
-    }
 }

contracts/utils/math/SignedMath.sol

@@ -30,4 +30,14 @@ library SignedMath {
         int256 x = (a & b) + ((a ^ b) >> 1);
         return x + (int256(uint256(x) >> 255) & (a ^ b));
     }
+
+    /**
+     * @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);
+        }
+    }
 }

test/utils/math/Math.test.js

@@ -1,6 +1,6 @@
 const { BN, constants } = require('@openzeppelin/test-helpers');
 const { expect } = require('chai');
-const { MAX_UINT256, MAX_INT256, MIN_INT256 } = constants;
+const { MAX_UINT256 } = constants;
 
 const MathMock = artifacts.require('MathMock');
 
@@ -85,20 +85,4 @@ contract('Math', function (accounts) {
       expect(await this.math.ceilDiv(MAX_UINT256, b)).to.be.bignumber.equal(MAX_UINT256);
     });
   });
-
-  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());
-      });
-    }
-  });
 });

test/utils/math/SignedMath.test.js

@@ -74,4 +74,20 @@ contract('SignedMath', function (accounts) {
       }
     });
   });
+
+  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());
+      });
+    }
+  });
 });