From 2102a94e2540f2237715475966ebae7fc37079cc Mon Sep 17 00:00:00 2001 From: rotcivegaf Date: Thu, 20 May 2021 21:55:31 +0200 Subject: [PATCH 01/12] add contract and tests --- contracts/mocks/SignedMathMock.sol | 23 +++++ contracts/utils/math/SignedMath.sol | 42 ++++++++ test/utils/math/SignedMath.test.js | 144 ++++++++++++++++++++++++++++ 3 files changed, 209 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/contracts/mocks/SignedMathMock.sol b/contracts/mocks/SignedMathMock.sol new file mode 100644 index 00000000000..df398424b44 --- /dev/null +++ b/contracts/mocks/SignedMathMock.sol @@ -0,0 +1,23 @@ +// 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); + } + + function ceilDiv(int256 a, int256 b) public pure returns (int256) { + return SignedMath.ceilDiv(a, b); + } +} diff --git a/contracts/utils/math/SignedMath.sol b/contracts/utils/math/SignedMath.sol new file mode 100644 index 00000000000..2180e702be0 --- /dev/null +++ b/contracts/utils/math/SignedMath.sol @@ -0,0 +1,42 @@ +// 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. The result is rounded + * towards zero. + */ + function average(int256 a, int256 b) internal pure returns (int256) { + return (a + b) / 2; + } + + /** + * @dev Returns the ceiling of the division of two signed numbers. + * + * This differs from standard division with `/` in that it rounds up instead + * of rounding down. + */ + function ceilDiv(int256 a, int256 b) internal pure returns (int256) { + int256 z = a / b; + int8 r = int8(a % b == 0 ? 0 : 1); + return z < 0 ? z - r : z + r; + } +} \ No newline at end of file diff --git a/test/utils/math/SignedMath.test.js b/test/utils/math/SignedMath.test.js new file mode 100644 index 00000000000..17c468dc75f --- /dev/null +++ b/test/utils/math/SignedMath.test.js @@ -0,0 +1,144 @@ +const { BN, constants } = require('@openzeppelin/test-helpers'); +const { expect } = require('chai'); +const { 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 two odd numbers', async function () { + const a = new BN('57417'); + const b = new BN('95431'); + expect(await this.math.average(a, b)).to.be.bignumber.equal(bnAverage(a, b)); + }); + + it('is correctly calculated with two even numbers', async function () { + const a = new BN('42304'); + const b = new BN('84346'); + expect(await this.math.average(a, b)).to.be.bignumber.equal(bnAverage(a, b)); + }); + + it('is correctly calculated with one even number and one odd number', async function () { + const a = new BN('57417'); + const b = new BN('84346'); + expect(await this.math.average(a, b)).to.be.bignumber.equal(bnAverage(a, b)); + }); + + it('is correctly calculated with two odd signed numbers', async function () { + const a = new BN('-57417'); + const b = new BN('-95431'); + expect(await this.math.average(a, b)).to.be.bignumber.equal(bnAverage(a, b)); + }); + + it('is correctly calculated with two even signed numbers', async function () { + const a = new BN('-42304'); + const b = new BN('-84346'); + expect(await this.math.average(a, b)).to.be.bignumber.equal(bnAverage(a, b)); + }); + + it('is correctly calculated with one even signed number and one odd signed number', async function () { + const a = new BN('-57417'); + const b = new BN('-84346'); + expect(await this.math.average(a, b)).to.be.bignumber.equal(bnAverage(a, b)); + }); + + it('is correctly calculated with one odd signed number and one odd number', async function () { + const a = new BN('-57417'); + const b = new BN('95431'); + expect(await this.math.average(a, b)).to.be.bignumber.equal(bnAverage(a, b)); + }); + + it('is correctly calculated with one even signed number and one even number', async function () { + const a = new BN('-42304'); + const b = new BN('84346'); + expect(await this.math.average(a, b)).to.be.bignumber.equal(bnAverage(a, b)); + }); + + it('is correctly calculated with one even signed number and one odd number', async function () { + const a = new BN('-57417'); + const b = new BN('84346'); + + expect(await this.math.average(a, b)).to.be.bignumber.equal(bnAverage(a, b)); + }); + }); + + describe('ceilDiv', function () { + it('does not round up on exact division, unsigned numbers', async function () { + const a = new BN('10'); + const b = new BN('5'); + expect(await this.math.ceilDiv(a, b)).to.be.bignumber.equal('2'); + }); + + it('does not round up on exact division, signed numbers', async function () { + const a = new BN('-10'); + const b = new BN('-5'); + expect(await this.math.ceilDiv(a, b)).to.be.bignumber.equal('2'); + }); + + it('does not round up on exact division', async function () { + const a = new BN('-10'); + const b = new BN('5'); + expect(await this.math.ceilDiv(a, b)).to.be.bignumber.equal('-2'); + }); + + it('rounds up on division with remainders, unsigned numbers', async function () { + const a = new BN('42'); + const b = new BN('13'); + expect(await this.math.ceilDiv(a, b)).to.be.bignumber.equal('4'); + }); + + it('rounds up on division with remainders signed numbers', async function () { + const a = new BN('-42'); + const b = new BN('-13'); + expect(await this.math.ceilDiv(a, b)).to.be.bignumber.equal('4'); + }); + + it('rounds up on division with remainders', async function () { + const a = new BN('-42'); + const b = new BN('13'); + expect(await this.math.ceilDiv(a, b)).to.be.bignumber.equal('-4'); + }); + + it('does not overflow', async function () { + const b = new BN('2'); + const result = new BN('1').shln(254); + expect(await this.math.ceilDiv(MAX_INT256, b)).to.be.bignumber.equal(result); + }); + + it('correctly computes max int256 divided by 1', async function () { + const b = new BN('1'); + expect(await this.math.ceilDiv(MAX_INT256, b)).to.be.bignumber.equal(MAX_INT256); + }); + }); +}); From d9546eb1137e5874ec3aa1eb5943b2b97db8f2d5 Mon Sep 17 00:00:00 2001 From: rotcivegaf Date: Fri, 9 Jul 2021 13:17:33 +0200 Subject: [PATCH 02/12] avoid implicit cast --- contracts/utils/math/SignedMath.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/utils/math/SignedMath.sol b/contracts/utils/math/SignedMath.sol index 2180e702be0..dc32c2b10cb 100644 --- a/contracts/utils/math/SignedMath.sol +++ b/contracts/utils/math/SignedMath.sol @@ -36,7 +36,7 @@ library SignedMath { */ function ceilDiv(int256 a, int256 b) internal pure returns (int256) { int256 z = a / b; - int8 r = int8(a % b == 0 ? 0 : 1); + int256 r = a % b == 0 ? int256(0) : int256(1); return z < 0 ? z - r : z + r; } } \ No newline at end of file From 105f24d0b13c8d554e3f74367544068cea890779 Mon Sep 17 00:00:00 2001 From: rotcivegaf Date: Sat, 10 Jul 2021 01:59:30 +0200 Subject: [PATCH 03/12] add test cases --- test/utils/math/SignedMath.test.js | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/test/utils/math/SignedMath.test.js b/test/utils/math/SignedMath.test.js index 17c468dc75f..3a7278cf818 100644 --- a/test/utils/math/SignedMath.test.js +++ b/test/utils/math/SignedMath.test.js @@ -85,12 +85,31 @@ contract('SignedMath', function (accounts) { expect(await this.math.average(a, b)).to.be.bignumber.equal(bnAverage(a, b)); }); - it('is correctly calculated with one even signed number and one odd number', async function () { + it('is correctly calculated with one even signed number and one odd number and even its greater than odd', async function () { + const a = new BN('-84346'); + const b = new BN('57417'); + + expect(await this.math.average(a, b)).to.be.bignumber.equal(bnAverage(a, b)); + }); + + it('is correctly calculated with one even signed number and one odd number and even its less than odd', async function () { const a = new BN('-57417'); const b = new BN('84346'); expect(await this.math.average(a, b)).to.be.bignumber.equal(bnAverage(a, b)); }); + + it('is correctly calculated with two max int256 numbers', async function () { + const a = MAX_INT256; + + expect(await this.math.average(a, a)).to.be.bignumber.equal(bnAverage(a, a)); + }); + + it('is correctly calculated with two min int256 numbers', async function () { + const a = MAX_INT256; + + expect(await this.math.average(a, a)).to.be.bignumber.equal(bnAverage(a, a)); + }); }); describe('ceilDiv', function () { From 0fb603c885b62a068877075b7a39c8accf527416 Mon Sep 17 00:00:00 2001 From: rotcivegaf Date: Sat, 10 Jul 2021 02:33:32 +0200 Subject: [PATCH 04/12] fix test names --- test/utils/math/SignedMath.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/utils/math/SignedMath.test.js b/test/utils/math/SignedMath.test.js index 3a7278cf818..9d48275c6b3 100644 --- a/test/utils/math/SignedMath.test.js +++ b/test/utils/math/SignedMath.test.js @@ -85,14 +85,14 @@ contract('SignedMath', function (accounts) { expect(await this.math.average(a, b)).to.be.bignumber.equal(bnAverage(a, b)); }); - it('is correctly calculated with one even signed number and one odd number and even its greater than odd', async function () { + it('is correctly calculated with one even signed number and one odd number and even its greater than odd, both in module', async function () { const a = new BN('-84346'); const b = new BN('57417'); expect(await this.math.average(a, b)).to.be.bignumber.equal(bnAverage(a, b)); }); - it('is correctly calculated with one even signed number and one odd number and even its less than odd', async function () { + it('is correctly calculated with one even signed number and one odd number and even its less than odd, both in module', async function () { const a = new BN('-57417'); const b = new BN('84346'); From 995bccaf4f152961b9a7ecb150b154181138b166 Mon Sep 17 00:00:00 2001 From: rotcivegaf Date: Sat, 10 Jul 2021 03:49:55 +0200 Subject: [PATCH 05/12] modify avarage and add tests --- contracts/utils/math/SignedMath.sol | 8 ++++- test/utils/math/SignedMath.test.js | 53 ++++++++++++++++++++++++++--- 2 files changed, 55 insertions(+), 6 deletions(-) diff --git a/contracts/utils/math/SignedMath.sol b/contracts/utils/math/SignedMath.sol index dc32c2b10cb..5d8410c6101 100644 --- a/contracts/utils/math/SignedMath.sol +++ b/contracts/utils/math/SignedMath.sol @@ -25,7 +25,13 @@ library SignedMath { * towards zero. */ function average(int256 a, int256 b) internal pure returns (int256) { - return (a + b) / 2; + if ((b < 0 && a > 0 && -b < a) || (a < 0 && b > 0 && -a < b)) + return (a / 2) + (b / 2) + (((a % 2 + b % 2) - 1) / 2); + + if ((b < 0 && a > 0 && -b > a) || (a < 0 && b > 0 && -a > b)) + return (a / 2) + (b / 2) + (((a % 2 + b % 2) + 1) / 2); + + return (a / 2) + (b / 2) + ((a % 2 + b % 2) / 2); } /** diff --git a/test/utils/math/SignedMath.test.js b/test/utils/math/SignedMath.test.js index 9d48275c6b3..78f4c3fb377 100644 --- a/test/utils/math/SignedMath.test.js +++ b/test/utils/math/SignedMath.test.js @@ -85,16 +85,59 @@ contract('SignedMath', function (accounts) { expect(await this.math.average(a, b)).to.be.bignumber.equal(bnAverage(a, b)); }); + it('is correctly calculated with one even number and one odd signed number and even its greater than odd, both in module', async function () { + const a = new BN('2'); + const b = new BN('-1'); + + expect(await this.math.average(a, b)).to.be.bignumber.equal(bnAverage(a, b)); + }); + + it('is correctly calculated with one odd number and one even signed number and odd its greater than even, both in module', async function () { + const a = new BN('3'); + const b = new BN('-2'); + + expect(await this.math.average(a, b)).to.be.bignumber.equal(bnAverage(a, b)); + }); + + + it('is correctly calculated with one even number and one odd signed number and odd its greater than even, both in module', async function () { + const a = new BN('2'); + const b = new BN('-3'); + + expect(await this.math.average(a, b)).to.be.bignumber.equal(bnAverage(a, b)); + }); + + it('is correctly calculated with one odd number and one even signed number and even its greater than odd, both in module', async function () { + const a = new BN('3'); + const b = new BN('-4'); + + expect(await this.math.average(a, b)).to.be.bignumber.equal(bnAverage(a, b)); + }); + it('is correctly calculated with one even signed number and one odd number and even its greater than odd, both in module', async function () { - const a = new BN('-84346'); - const b = new BN('57417'); + const a = new BN('-2'); + const b = new BN('1'); expect(await this.math.average(a, b)).to.be.bignumber.equal(bnAverage(a, b)); }); - it('is correctly calculated with one even signed number and one odd number and even its less than odd, both in module', async function () { - const a = new BN('-57417'); - const b = new BN('84346'); + it('is correctly calculated with one odd signed number and one even number and odd its greater than even, both in module', async function () { + const a = new BN('-3'); + const b = new BN('2'); + + expect(await this.math.average(a, b)).to.be.bignumber.equal(bnAverage(a, b)); + }); + + it('is correctly calculated with one even signed number and one odd number and odd its greater than even, both in module', async function () { + const a = new BN('-2'); + const b = new BN('3'); + + expect(await this.math.average(a, b)).to.be.bignumber.equal(bnAverage(a, b)); + }); + + it('is correctly calculated with one odd signed number and one even number and even its greater than odd, both in module', async function () { + const a = new BN('-3'); + const b = new BN('4'); expect(await this.math.average(a, b)).to.be.bignumber.equal(bnAverage(a, b)); }); From 234cfb8afa2b1b42f5405c1f69f189acaa884bd0 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 6 Jan 2022 18:13:11 +0100 Subject: [PATCH 06/12] improve signed average formula --- contracts/utils/math/SignedMath.sol | 16 ++++++++-------- test/utils/math/SignedMath.test.js | 9 ++++++++- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/contracts/utils/math/SignedMath.sol b/contracts/utils/math/SignedMath.sol index 5d8410c6101..1b7988de380 100644 --- a/contracts/utils/math/SignedMath.sol +++ b/contracts/utils/math/SignedMath.sol @@ -25,13 +25,13 @@ library SignedMath { * towards zero. */ function average(int256 a, int256 b) internal pure returns (int256) { - if ((b < 0 && a > 0 && -b < a) || (a < 0 && b > 0 && -a < b)) - return (a / 2) + (b / 2) + (((a % 2 + b % 2) - 1) / 2); - - if ((b < 0 && a > 0 && -b > a) || (a < 0 && b > 0 && -a > b)) - return (a / 2) + (b / 2) + (((a % 2 + b % 2) + 1) / 2); - - return (a / 2) + (b / 2) + ((a % 2 + b % 2) / 2); + // (a + b) / 2 can overflow, and the unsigned formula doesn't simply translate to signed integers + int256 base = (a & b) + (a ^ b) / 2; + if ((a < 0 && b < 0) || ((a < 0 || b < 0) && (a + b > 0))) { + return base + (a ^ b) % 2; + } else { + return base; + } } /** @@ -45,4 +45,4 @@ library SignedMath { int256 r = a % b == 0 ? int256(0) : int256(1); return z < 0 ? z - r : z + r; } -} \ No newline at end of file +} diff --git a/test/utils/math/SignedMath.test.js b/test/utils/math/SignedMath.test.js index 78f4c3fb377..e53b0bbdca5 100644 --- a/test/utils/math/SignedMath.test.js +++ b/test/utils/math/SignedMath.test.js @@ -85,6 +85,7 @@ contract('SignedMath', function (accounts) { expect(await this.math.average(a, b)).to.be.bignumber.equal(bnAverage(a, b)); }); + // eslint-disable-next-line max-len it('is correctly calculated with one even number and one odd signed number and even its greater than odd, both in module', async function () { const a = new BN('2'); const b = new BN('-1'); @@ -92,6 +93,7 @@ contract('SignedMath', function (accounts) { expect(await this.math.average(a, b)).to.be.bignumber.equal(bnAverage(a, b)); }); + // eslint-disable-next-line max-len it('is correctly calculated with one odd number and one even signed number and odd its greater than even, both in module', async function () { const a = new BN('3'); const b = new BN('-2'); @@ -99,7 +101,7 @@ contract('SignedMath', function (accounts) { expect(await this.math.average(a, b)).to.be.bignumber.equal(bnAverage(a, b)); }); - + // eslint-disable-next-line max-len it('is correctly calculated with one even number and one odd signed number and odd its greater than even, both in module', async function () { const a = new BN('2'); const b = new BN('-3'); @@ -107,6 +109,7 @@ contract('SignedMath', function (accounts) { expect(await this.math.average(a, b)).to.be.bignumber.equal(bnAverage(a, b)); }); + // eslint-disable-next-line max-len it('is correctly calculated with one odd number and one even signed number and even its greater than odd, both in module', async function () { const a = new BN('3'); const b = new BN('-4'); @@ -114,6 +117,7 @@ contract('SignedMath', function (accounts) { expect(await this.math.average(a, b)).to.be.bignumber.equal(bnAverage(a, b)); }); + // eslint-disable-next-line max-len it('is correctly calculated with one even signed number and one odd number and even its greater than odd, both in module', async function () { const a = new BN('-2'); const b = new BN('1'); @@ -121,6 +125,7 @@ contract('SignedMath', function (accounts) { expect(await this.math.average(a, b)).to.be.bignumber.equal(bnAverage(a, b)); }); + // eslint-disable-next-line max-len it('is correctly calculated with one odd signed number and one even number and odd its greater than even, both in module', async function () { const a = new BN('-3'); const b = new BN('2'); @@ -128,6 +133,7 @@ contract('SignedMath', function (accounts) { expect(await this.math.average(a, b)).to.be.bignumber.equal(bnAverage(a, b)); }); + // eslint-disable-next-line max-len it('is correctly calculated with one even signed number and one odd number and odd its greater than even, both in module', async function () { const a = new BN('-2'); const b = new BN('3'); @@ -135,6 +141,7 @@ contract('SignedMath', function (accounts) { expect(await this.math.average(a, b)).to.be.bignumber.equal(bnAverage(a, b)); }); + // eslint-disable-next-line max-len it('is correctly calculated with one odd signed number and one even number and even its greater than odd, both in module', async function () { const a = new BN('-3'); const b = new BN('4'); From 7f5424513c85784d91f8cdf10e8e870d560d6f44 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 6 Jan 2022 18:29:37 +0100 Subject: [PATCH 07/12] fix lint --- contracts/utils/math/SignedMath.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/utils/math/SignedMath.sol b/contracts/utils/math/SignedMath.sol index 1b7988de380..7ba7237db26 100644 --- a/contracts/utils/math/SignedMath.sol +++ b/contracts/utils/math/SignedMath.sol @@ -28,7 +28,7 @@ library SignedMath { // (a + b) / 2 can overflow, and the unsigned formula doesn't simply translate to signed integers int256 base = (a & b) + (a ^ b) / 2; if ((a < 0 && b < 0) || ((a < 0 || b < 0) && (a + b > 0))) { - return base + (a ^ b) % 2; + return base + ((a ^ b) % 2); } else { return base; } From c2dbd67031344a9899f10935d5cde2efd1a15ba2 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Fri, 7 Jan 2022 15:06:03 +0100 Subject: [PATCH 08/12] better average formula --- contracts/utils/math/SignedMath.sol | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/contracts/utils/math/SignedMath.sol b/contracts/utils/math/SignedMath.sol index 7ba7237db26..b5e03dec787 100644 --- a/contracts/utils/math/SignedMath.sol +++ b/contracts/utils/math/SignedMath.sol @@ -26,12 +26,8 @@ library SignedMath { */ function average(int256 a, int256 b) internal pure returns (int256) { // (a + b) / 2 can overflow, and the unsigned formula doesn't simply translate to signed integers - int256 base = (a & b) + (a ^ b) / 2; - if ((a < 0 && b < 0) || ((a < 0 || b < 0) && (a + b > 0))) { - return base + ((a ^ b) % 2); - } else { - return base; - } + int256 x = (a & b) + ((a ^ b) >> 1); + return x + (int256(uint256(x) >> 255) & (a ^ b)); } /** From f3f5f4621e941c31034cf6414bb3fe367baffcb4 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Fri, 7 Jan 2022 15:13:51 +0100 Subject: [PATCH 09/12] refactor signed average testing --- test/utils/math/SignedMath.test.js | 159 +++++++---------------------- 1 file changed, 36 insertions(+), 123 deletions(-) diff --git a/test/utils/math/SignedMath.test.js b/test/utils/math/SignedMath.test.js index e53b0bbdca5..593a017171d 100644 --- a/test/utils/math/SignedMath.test.js +++ b/test/utils/math/SignedMath.test.js @@ -1,6 +1,6 @@ const { BN, constants } = require('@openzeppelin/test-helpers'); const { expect } = require('chai'); -const { MAX_INT256 } = constants; +const { MIN_INT256, MAX_INT256 } = constants; const SignedMathMock = artifacts.require('SignedMathMock'); @@ -37,128 +37,41 @@ contract('SignedMath', function (accounts) { return a.add(b).divn(2); } - it('is correctly calculated with two odd numbers', async function () { - const a = new BN('57417'); - const b = new BN('95431'); - expect(await this.math.average(a, b)).to.be.bignumber.equal(bnAverage(a, b)); - }); - - it('is correctly calculated with two even numbers', async function () { - const a = new BN('42304'); - const b = new BN('84346'); - expect(await this.math.average(a, b)).to.be.bignumber.equal(bnAverage(a, b)); - }); - - it('is correctly calculated with one even number and one odd number', async function () { - const a = new BN('57417'); - const b = new BN('84346'); - expect(await this.math.average(a, b)).to.be.bignumber.equal(bnAverage(a, b)); - }); - - it('is correctly calculated with two odd signed numbers', async function () { - const a = new BN('-57417'); - const b = new BN('-95431'); - expect(await this.math.average(a, b)).to.be.bignumber.equal(bnAverage(a, b)); - }); - - it('is correctly calculated with two even signed numbers', async function () { - const a = new BN('-42304'); - const b = new BN('-84346'); - expect(await this.math.average(a, b)).to.be.bignumber.equal(bnAverage(a, b)); - }); - - it('is correctly calculated with one even signed number and one odd signed number', async function () { - const a = new BN('-57417'); - const b = new BN('-84346'); - expect(await this.math.average(a, b)).to.be.bignumber.equal(bnAverage(a, b)); - }); - - it('is correctly calculated with one odd signed number and one odd number', async function () { - const a = new BN('-57417'); - const b = new BN('95431'); - expect(await this.math.average(a, b)).to.be.bignumber.equal(bnAverage(a, b)); - }); - - it('is correctly calculated with one even signed number and one even number', async function () { - const a = new BN('-42304'); - const b = new BN('84346'); - expect(await this.math.average(a, b)).to.be.bignumber.equal(bnAverage(a, b)); - }); - - // eslint-disable-next-line max-len - it('is correctly calculated with one even number and one odd signed number and even its greater than odd, both in module', async function () { - const a = new BN('2'); - const b = new BN('-1'); - - expect(await this.math.average(a, b)).to.be.bignumber.equal(bnAverage(a, b)); - }); - - // eslint-disable-next-line max-len - it('is correctly calculated with one odd number and one even signed number and odd its greater than even, both in module', async function () { - const a = new BN('3'); - const b = new BN('-2'); - - expect(await this.math.average(a, b)).to.be.bignumber.equal(bnAverage(a, b)); - }); - - // eslint-disable-next-line max-len - it('is correctly calculated with one even number and one odd signed number and odd its greater than even, both in module', async function () { - const a = new BN('2'); - const b = new BN('-3'); - - expect(await this.math.average(a, b)).to.be.bignumber.equal(bnAverage(a, b)); - }); - - // eslint-disable-next-line max-len - it('is correctly calculated with one odd number and one even signed number and even its greater than odd, both in module', async function () { - const a = new BN('3'); - const b = new BN('-4'); - - expect(await this.math.average(a, b)).to.be.bignumber.equal(bnAverage(a, b)); - }); - - // eslint-disable-next-line max-len - it('is correctly calculated with one even signed number and one odd number and even its greater than odd, both in module', async function () { - const a = new BN('-2'); - const b = new BN('1'); - - expect(await this.math.average(a, b)).to.be.bignumber.equal(bnAverage(a, b)); - }); - - // eslint-disable-next-line max-len - it('is correctly calculated with one odd signed number and one even number and odd its greater than even, both in module', async function () { - const a = new BN('-3'); - const b = new BN('2'); - - expect(await this.math.average(a, b)).to.be.bignumber.equal(bnAverage(a, b)); - }); - - // eslint-disable-next-line max-len - it('is correctly calculated with one even signed number and one odd number and odd its greater than even, both in module', async function () { - const a = new BN('-2'); - const b = new BN('3'); - - expect(await this.math.average(a, b)).to.be.bignumber.equal(bnAverage(a, b)); - }); - - // eslint-disable-next-line max-len - it('is correctly calculated with one odd signed number and one even number and even its greater than odd, both in module', async function () { - const a = new BN('-3'); - const b = new BN('4'); - - expect(await this.math.average(a, b)).to.be.bignumber.equal(bnAverage(a, b)); - }); - - it('is correctly calculated with two max int256 numbers', async function () { - const a = MAX_INT256; - - expect(await this.math.average(a, a)).to.be.bignumber.equal(bnAverage(a, a)); - }); - - it('is correctly calculated with two min int256 numbers', async function () { - const a = MAX_INT256; - - expect(await this.math.average(a, a)).to.be.bignumber.equal(bnAverage(a, a)); + 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})`); + } + } }); }); From 39ea6c889f1570517151024b3099744f3664f875 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Fri, 7 Jan 2022 15:18:50 +0100 Subject: [PATCH 10/12] add doc and changelog entry --- CHANGELOG.md | 1 + contracts/utils/README.adoc | 2 ++ 2 files changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index be54f9160b5..007ff34e28b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ * `ERC2771Context`: use immutable storage to store the forwarder address, no longer an issue since Solidity >=0.8.8 allows reading immutable variables in the constructor. ([#2917](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2917)) * `Base64`: add a library to parse bytes into base64 strings using `encode(bytes memory)` function, and provide examples to show how to use to build URL-safe `tokenURIs` ([#2884](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/#2884)) * `ERC20`: reduce allowance before triggering transfer. + * `SignedMath`: a new signed version of the Math library with `max`, `min`, `average`and `ceilDiv`. ([#2686](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2686)) ## 4.4.1 (2021-12-14) diff --git a/contracts/utils/README.adoc b/contracts/utils/README.adoc index 4f19f1ac224..83e30e78cff 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}} From 4299249bead819b50f863f1d35aee5b811b5d1cd Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Fri, 7 Jan 2022 15:26:56 +0100 Subject: [PATCH 11/12] Update contracts/utils/math/SignedMath.sol Co-authored-by: Francisco Giordano --- contracts/utils/math/SignedMath.sol | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/contracts/utils/math/SignedMath.sol b/contracts/utils/math/SignedMath.sol index b5e03dec787..7b80850c619 100644 --- a/contracts/utils/math/SignedMath.sol +++ b/contracts/utils/math/SignedMath.sol @@ -21,11 +21,11 @@ library SignedMath { } /** - * @dev Returns the average of two signed numbers. The result is rounded - * towards zero. + * @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) { - // (a + b) / 2 can overflow, and the unsigned formula doesn't simply translate to signed integers + // Formula from the book "Hacker's Delight" int256 x = (a & b) + ((a ^ b) >> 1); return x + (int256(uint256(x) >> 255) & (a ^ b)); } From b1082cb397d8aaa13179ecb91042a4c7f377b75b Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Wed, 12 Jan 2022 14:51:49 -0300 Subject: [PATCH 12/12] remove ceilDiv --- CHANGELOG.md | 2 +- contracts/mocks/SignedMathMock.sol | 4 --- contracts/utils/math/SignedMath.sol | 12 ------- test/utils/math/SignedMath.test.js | 49 ----------------------------- 4 files changed, 1 insertion(+), 66 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e50b530b71..4b785e45017 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +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`, `average`and `ceilDiv`. ([#2686](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2686)) + * `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 index df398424b44..8d82e137e0c 100644 --- a/contracts/mocks/SignedMathMock.sol +++ b/contracts/mocks/SignedMathMock.sol @@ -16,8 +16,4 @@ contract SignedMathMock { function average(int256 a, int256 b) public pure returns (int256) { return SignedMath.average(a, b); } - - function ceilDiv(int256 a, int256 b) public pure returns (int256) { - return SignedMath.ceilDiv(a, b); - } } diff --git a/contracts/utils/math/SignedMath.sol b/contracts/utils/math/SignedMath.sol index 7b80850c619..96160910444 100644 --- a/contracts/utils/math/SignedMath.sol +++ b/contracts/utils/math/SignedMath.sol @@ -29,16 +29,4 @@ library SignedMath { int256 x = (a & b) + ((a ^ b) >> 1); return x + (int256(uint256(x) >> 255) & (a ^ b)); } - - /** - * @dev Returns the ceiling of the division of two signed numbers. - * - * This differs from standard division with `/` in that it rounds up instead - * of rounding down. - */ - function ceilDiv(int256 a, int256 b) internal pure returns (int256) { - int256 z = a / b; - int256 r = a % b == 0 ? int256(0) : int256(1); - return z < 0 ? z - r : z + r; - } } diff --git a/test/utils/math/SignedMath.test.js b/test/utils/math/SignedMath.test.js index 593a017171d..0aef556dfbe 100644 --- a/test/utils/math/SignedMath.test.js +++ b/test/utils/math/SignedMath.test.js @@ -74,53 +74,4 @@ contract('SignedMath', function (accounts) { } }); }); - - describe('ceilDiv', function () { - it('does not round up on exact division, unsigned numbers', async function () { - const a = new BN('10'); - const b = new BN('5'); - expect(await this.math.ceilDiv(a, b)).to.be.bignumber.equal('2'); - }); - - it('does not round up on exact division, signed numbers', async function () { - const a = new BN('-10'); - const b = new BN('-5'); - expect(await this.math.ceilDiv(a, b)).to.be.bignumber.equal('2'); - }); - - it('does not round up on exact division', async function () { - const a = new BN('-10'); - const b = new BN('5'); - expect(await this.math.ceilDiv(a, b)).to.be.bignumber.equal('-2'); - }); - - it('rounds up on division with remainders, unsigned numbers', async function () { - const a = new BN('42'); - const b = new BN('13'); - expect(await this.math.ceilDiv(a, b)).to.be.bignumber.equal('4'); - }); - - it('rounds up on division with remainders signed numbers', async function () { - const a = new BN('-42'); - const b = new BN('-13'); - expect(await this.math.ceilDiv(a, b)).to.be.bignumber.equal('4'); - }); - - it('rounds up on division with remainders', async function () { - const a = new BN('-42'); - const b = new BN('13'); - expect(await this.math.ceilDiv(a, b)).to.be.bignumber.equal('-4'); - }); - - it('does not overflow', async function () { - const b = new BN('2'); - const result = new BN('1').shln(254); - expect(await this.math.ceilDiv(MAX_INT256, b)).to.be.bignumber.equal(result); - }); - - it('correctly computes max int256 divided by 1', async function () { - const b = new BN('1'); - expect(await this.math.ceilDiv(MAX_INT256, b)).to.be.bignumber.equal(MAX_INT256); - }); - }); });