From 3eb2d43b0610758c6b85bf4b8ae160db3679c34d Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Fri, 14 Jan 2022 23:27:04 +0100 Subject: [PATCH] Move abs(int256) from Math to SafeMath (#3110) Co-authored-by: Francisco Giordano --- CHANGELOG.md | 2 +- contracts/mocks/MathMock.sol | 4 ---- contracts/mocks/SignedMathMock.sol | 4 ++++ contracts/utils/math/Math.sol | 10 ---------- contracts/utils/math/SignedMath.sol | 10 ++++++++++ test/utils/math/Math.test.js | 18 +----------------- test/utils/math/SignedMath.test.js | 16 ++++++++++++++++ 7 files changed, 32 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 724af7ce2..4a70ffd7a 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/contracts/mocks/MathMock.sol b/contracts/mocks/MathMock.sol index f6c11acb4..c651b6bb1 100644 --- a/contracts/mocks/MathMock.sol +++ b/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); - } } diff --git a/contracts/mocks/SignedMathMock.sol b/contracts/mocks/SignedMathMock.sol index 8d82e137e..5a0b27096 100644 --- a/contracts/mocks/SignedMathMock.sol +++ b/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); + } } diff --git a/contracts/utils/math/Math.sol b/contracts/utils/math/Math.sol index c25134321..03d521845 100644 --- a/contracts/utils/math/Math.sol +++ b/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); - } - } } diff --git a/contracts/utils/math/SignedMath.sol b/contracts/utils/math/SignedMath.sol index 961609104..2c47aa0bf 100644 --- a/contracts/utils/math/SignedMath.sol +++ b/contracts/utils/math/SignedMath.sol @@ -29,4 +29,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); + } + } } diff --git a/test/utils/math/Math.test.js b/test/utils/math/Math.test.js index eff64b720..7e194dec7 100644 --- a/test/utils/math/Math.test.js +++ b/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()); - }); - } - }); }); diff --git a/test/utils/math/SignedMath.test.js b/test/utils/math/SignedMath.test.js index 0aef556df..8e9826f02 100644 --- a/test/utils/math/SignedMath.test.js +++ b/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()); + }); + } + }); });