From 3458c1e8541ce0a0cd935828c9db8f9cbca988a0 Mon Sep 17 00:00:00 2001 From: rotcivegaf Date: Wed, 12 Jan 2022 16:08:59 -0300 Subject: [PATCH] Add SignedMath with math utilities for signed integers (#2686) * add contract and tests * avoid implicit cast * add test cases * fix test names * modify avarage and add tests * improve signed average formula * fix lint * better average formula * refactor signed average testing * add doc and changelog entry * Update contracts/utils/math/SignedMath.sol Co-authored-by: Francisco Giordano * remove ceilDiv Co-authored-by: Hadrien Croubois Co-authored-by: Francisco Giordano --- CHANGELOG.md | 1 + contracts/mocks/SignedMathMock.sol | 19 +++++++ contracts/utils/README.adoc | 2 + contracts/utils/math/SignedMath.sol | 32 ++++++++++++ test/utils/math/SignedMath.test.js | 77 +++++++++++++++++++++++++++++ 5 files changed, 131 insertions(+) create mode 100644 contracts/mocks/SignedMathMock.sol create mode 100644 contracts/utils/math/SignedMath.sol create mode 100644 test/utils/math/SignedMath.test.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 53a765f44..a96813b38 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ * `ERC20`: reduce allowance before triggering transfer. ([#3056](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/#3056)) * `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)) ### Breaking change diff --git a/contracts/mocks/SignedMathMock.sol b/contracts/mocks/SignedMathMock.sol new file mode 100644 index 000000000..8d82e137e --- /dev/null +++ b/contracts/mocks/SignedMathMock.sol @@ -0,0 +1,19 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.0; + +import "../utils/math/SignedMath.sol"; + +contract SignedMathMock { + function max(int256 a, int256 b) public pure returns (int256) { + return SignedMath.max(a, b); + } + + function min(int256 a, int256 b) public pure returns (int256) { + return SignedMath.min(a, b); + } + + function average(int256 a, int256 b) public pure returns (int256) { + return SignedMath.average(a, b); + } +} diff --git a/contracts/utils/README.adoc b/contracts/utils/README.adoc index 4f19f1ac2..83e30e78c 100644 --- a/contracts/utils/README.adoc +++ b/contracts/utils/README.adoc @@ -27,6 +27,8 @@ Finally, {Create2} contains all necessary utilities to safely use the https://bl {{Math}} +{{SignedMath}} + {{SafeCast}} {{SafeMath}} diff --git a/contracts/utils/math/SignedMath.sol b/contracts/utils/math/SignedMath.sol new file mode 100644 index 000000000..961609104 --- /dev/null +++ b/contracts/utils/math/SignedMath.sol @@ -0,0 +1,32 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.0; + +/** + * @dev Standard signed math utilities missing in the Solidity language. + */ +library SignedMath { + /** + * @dev Returns the largest of two signed numbers. + */ + function max(int256 a, int256 b) internal pure returns (int256) { + return a >= b ? a : b; + } + + /** + * @dev Returns the smallest of two signed numbers. + */ + function min(int256 a, int256 b) internal pure returns (int256) { + return a < b ? a : b; + } + + /** + * @dev Returns the average of two signed numbers without overflow. + * The result is rounded towards zero. + */ + function average(int256 a, int256 b) internal pure returns (int256) { + // Formula from the book "Hacker's Delight" + int256 x = (a & b) + ((a ^ b) >> 1); + return x + (int256(uint256(x) >> 255) & (a ^ b)); + } +} diff --git a/test/utils/math/SignedMath.test.js b/test/utils/math/SignedMath.test.js new file mode 100644 index 000000000..0aef556df --- /dev/null +++ b/test/utils/math/SignedMath.test.js @@ -0,0 +1,77 @@ +const { BN, constants } = require('@openzeppelin/test-helpers'); +const { expect } = require('chai'); +const { MIN_INT256, MAX_INT256 } = constants; + +const SignedMathMock = artifacts.require('SignedMathMock'); + +contract('SignedMath', function (accounts) { + const min = new BN('-1234'); + const max = new BN('5678'); + + beforeEach(async function () { + this.math = await SignedMathMock.new(); + }); + + describe('max', function () { + it('is correctly detected in first argument position', async function () { + expect(await this.math.max(max, min)).to.be.bignumber.equal(max); + }); + + it('is correctly detected in second argument position', async function () { + expect(await this.math.max(min, max)).to.be.bignumber.equal(max); + }); + }); + + describe('min', function () { + it('is correctly detected in first argument position', async function () { + expect(await this.math.min(min, max)).to.be.bignumber.equal(min); + }); + + it('is correctly detected in second argument position', async function () { + expect(await this.math.min(max, min)).to.be.bignumber.equal(min); + }); + }); + + describe('average', function () { + function bnAverage (a, b) { + return a.add(b).divn(2); + } + + it('is correctly calculated with various input', async function () { + const valuesX = [ + new BN('0'), + new BN('3'), + new BN('-3'), + new BN('4'), + new BN('-4'), + new BN('57417'), + new BN('-57417'), + new BN('42304'), + new BN('-42304'), + MIN_INT256, + MAX_INT256, + ]; + + const valuesY = [ + new BN('0'), + new BN('5'), + new BN('-5'), + new BN('2'), + new BN('-2'), + new BN('57417'), + new BN('-57417'), + new BN('42304'), + new BN('-42304'), + MIN_INT256, + MAX_INT256, + ]; + + for (const x of valuesX) { + for (const y of valuesY) { + expect(await this.math.average(x, y)) + .to.be.bignumber.equal(bnAverage(x, y), `Bad result for average(${x}, ${y})`); + } + } + }); + }); +});