From 21baf44cbd73e24763afe2f9c1996eb378e08e0f Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Wed, 12 Jul 2023 13:53:54 -0300 Subject: [PATCH 1/6] Rename rounding modes and complete with fourth --- contracts/mocks/docs/ERC4626Fees.sol | 4 +- contracts/token/ERC20/extensions/ERC4626.sol | 14 +- contracts/utils/math/Math.sol | 21 +- test/helpers/enums.js | 2 +- test/utils/math/Math.t.sol | 16 +- test/utils/math/Math.test.js | 216 +++++++++---------- 6 files changed, 139 insertions(+), 134 deletions(-) diff --git a/contracts/mocks/docs/ERC4626Fees.sol b/contracts/mocks/docs/ERC4626Fees.sol index c82279a605c..49c1095ab47 100644 --- a/contracts/mocks/docs/ERC4626Fees.sol +++ b/contracts/mocks/docs/ERC4626Fees.sol @@ -81,10 +81,10 @@ abstract contract ERC4626Fees is ERC4626 { } function _feeOnRaw(uint256 assets, uint256 feeBasePoint) private pure returns (uint256) { - return assets.mulDiv(feeBasePoint, 1e5, Math.Rounding.Up); + return assets.mulDiv(feeBasePoint, 1e5, Math.Rounding.Ceil); } function _feeOnTotal(uint256 assets, uint256 feeBasePoint) private pure returns (uint256) { - return assets.mulDiv(feeBasePoint, feeBasePoint + 1e5, Math.Rounding.Up); + return assets.mulDiv(feeBasePoint, feeBasePoint + 1e5, Math.Rounding.Ceil); } } diff --git a/contracts/token/ERC20/extensions/ERC4626.sol b/contracts/token/ERC20/extensions/ERC4626.sol index 11f1cd59306..ad3b5a170d2 100644 --- a/contracts/token/ERC20/extensions/ERC4626.sol +++ b/contracts/token/ERC20/extensions/ERC4626.sol @@ -119,12 +119,12 @@ abstract contract ERC4626 is ERC20, IERC4626 { /** @dev See {IERC4626-convertToShares}. */ function convertToShares(uint256 assets) public view virtual returns (uint256) { - return _convertToShares(assets, Math.Rounding.Down); + return _convertToShares(assets, Math.Rounding.Floor); } /** @dev See {IERC4626-convertToAssets}. */ function convertToAssets(uint256 shares) public view virtual returns (uint256) { - return _convertToAssets(shares, Math.Rounding.Down); + return _convertToAssets(shares, Math.Rounding.Floor); } /** @dev See {IERC4626-maxDeposit}. */ @@ -139,7 +139,7 @@ abstract contract ERC4626 is ERC20, IERC4626 { /** @dev See {IERC4626-maxWithdraw}. */ function maxWithdraw(address owner) public view virtual returns (uint256) { - return _convertToAssets(balanceOf(owner), Math.Rounding.Down); + return _convertToAssets(balanceOf(owner), Math.Rounding.Floor); } /** @dev See {IERC4626-maxRedeem}. */ @@ -149,22 +149,22 @@ abstract contract ERC4626 is ERC20, IERC4626 { /** @dev See {IERC4626-previewDeposit}. */ function previewDeposit(uint256 assets) public view virtual returns (uint256) { - return _convertToShares(assets, Math.Rounding.Down); + return _convertToShares(assets, Math.Rounding.Floor); } /** @dev See {IERC4626-previewMint}. */ function previewMint(uint256 shares) public view virtual returns (uint256) { - return _convertToAssets(shares, Math.Rounding.Up); + return _convertToAssets(shares, Math.Rounding.Ceil); } /** @dev See {IERC4626-previewWithdraw}. */ function previewWithdraw(uint256 assets) public view virtual returns (uint256) { - return _convertToShares(assets, Math.Rounding.Up); + return _convertToShares(assets, Math.Rounding.Ceil); } /** @dev See {IERC4626-previewRedeem}. */ function previewRedeem(uint256 shares) public view virtual returns (uint256) { - return _convertToAssets(shares, Math.Rounding.Down); + return _convertToAssets(shares, Math.Rounding.Floor); } /** @dev See {IERC4626-deposit}. */ diff --git a/contracts/utils/math/Math.sol b/contracts/utils/math/Math.sol index f55b69afc12..52fe81891d2 100644 --- a/contracts/utils/math/Math.sol +++ b/contracts/utils/math/Math.sol @@ -13,9 +13,10 @@ library Math { error MathOverflowedMulDiv(); enum Rounding { - Down, // Toward negative infinity - Up, // Toward infinity - Zero // Toward zero + Floor, // Toward negative infinity + Ceil, // Toward positive infinity + Trunc, // Toward zero + Expand // Away from zero } /** @@ -206,7 +207,7 @@ library Math { */ function mulDiv(uint256 x, uint256 y, uint256 denominator, Rounding rounding) internal pure returns (uint256) { uint256 result = mulDiv(x, y, denominator); - if (rounding == Rounding.Up && mulmod(x, y, denominator) > 0) { + if (unsignedRoundsUp(rounding) && mulmod(x, y, denominator) > 0) { result += 1; } return result; @@ -256,7 +257,7 @@ library Math { function sqrt(uint256 a, Rounding rounding) internal pure returns (uint256) { unchecked { uint256 result = sqrt(a); - return result + (rounding == Rounding.Up && result * result < a ? 1 : 0); + return result + (unsignedRoundsUp(rounding) && result * result < a ? 1 : 0); } } @@ -309,7 +310,7 @@ library Math { function log2(uint256 value, Rounding rounding) internal pure returns (uint256) { unchecked { uint256 result = log2(value); - return result + (rounding == Rounding.Up && 1 << result < value ? 1 : 0); + return result + (unsignedRoundsUp(rounding) && 1 << result < value ? 1 : 0); } } @@ -358,7 +359,7 @@ library Math { function log10(uint256 value, Rounding rounding) internal pure returns (uint256) { unchecked { uint256 result = log10(value); - return result + (rounding == Rounding.Up && 10 ** result < value ? 1 : 0); + return result + (unsignedRoundsUp(rounding) && 10 ** result < value ? 1 : 0); } } @@ -401,7 +402,11 @@ library Math { function log256(uint256 value, Rounding rounding) internal pure returns (uint256) { unchecked { uint256 result = log256(value); - return result + (rounding == Rounding.Up && 1 << (result << 3) < value ? 1 : 0); + return result + (unsignedRoundsUp(rounding) && 1 << (result << 3) < value ? 1 : 0); } } + + function unsignedRoundsUp(Rounding rounding) internal pure returns (bool) { + return uint8(rounding) % 2 == 1; + } } diff --git a/test/helpers/enums.js b/test/helpers/enums.js index 75746e08744..6280e0f319b 100644 --- a/test/helpers/enums.js +++ b/test/helpers/enums.js @@ -6,6 +6,6 @@ module.exports = { Enum, ProposalState: Enum('Pending', 'Active', 'Canceled', 'Defeated', 'Succeeded', 'Queued', 'Expired', 'Executed'), VoteType: Enum('Against', 'For', 'Abstain'), - Rounding: Enum('Down', 'Up', 'Zero'), + Rounding: Enum('Floor', 'Ceil', 'Trunc', 'Expand'), OperationState: Enum('Unset', 'Waiting', 'Ready', 'Done'), }; diff --git a/test/utils/math/Math.t.sol b/test/utils/math/Math.t.sol index 5a6be476f3e..d6b0c5d0349 100644 --- a/test/utils/math/Math.t.sol +++ b/test/utils/math/Math.t.sol @@ -31,12 +31,12 @@ contract MathTest is Test { // square of result is bigger than input if (_squareBigger(result, input)) { - assertTrue(rounding == Math.Rounding.Up); + assertTrue(Math.unsignedRoundsUp(rounding)); assertTrue(_squareSmaller(result - 1, input)); } // square of result is smaller than input else if (_squareSmaller(result, input)) { - assertFalse(rounding == Math.Rounding.Up); + assertFalse(Math.unsignedRoundsUp(rounding)); assertTrue(_squareBigger(result + 1, input)); } // input is perfect square @@ -63,10 +63,10 @@ contract MathTest is Test { if (input == 0) { assertEq(result, 0); } else if (_powerOf2Bigger(result, input)) { - assertTrue(rounding == Math.Rounding.Up); + assertTrue(Math.unsignedRoundsUp(rounding)); assertTrue(_powerOf2Smaller(result - 1, input)); } else if (_powerOf2Smaller(result, input)) { - assertFalse(rounding == Math.Rounding.Up); + assertFalse(Math.unsignedRoundsUp(rounding)); assertTrue(_powerOf2Bigger(result + 1, input)); } else { assertEq(2 ** result, input); @@ -90,10 +90,10 @@ contract MathTest is Test { if (input == 0) { assertEq(result, 0); } else if (_powerOf10Bigger(result, input)) { - assertTrue(rounding == Math.Rounding.Up); + assertTrue(Math.unsignedRoundsUp(rounding)); assertTrue(_powerOf10Smaller(result - 1, input)); } else if (_powerOf10Smaller(result, input)) { - assertFalse(rounding == Math.Rounding.Up); + assertFalse(Math.unsignedRoundsUp(rounding)); assertTrue(_powerOf10Bigger(result + 1, input)); } else { assertEq(10 ** result, input); @@ -117,10 +117,10 @@ contract MathTest is Test { if (input == 0) { assertEq(result, 0); } else if (_powerOf256Bigger(result, input)) { - assertTrue(rounding == Math.Rounding.Up); + assertTrue(Math.unsignedRoundsUp(rounding)); assertTrue(_powerOf256Smaller(result - 1, input)); } else if (_powerOf256Smaller(result, input)) { - assertFalse(rounding == Math.Rounding.Up); + assertFalse(Math.unsignedRoundsUp(rounding)); assertTrue(_powerOf256Bigger(result + 1, input)); } else { assertEq(256 ** result, input); diff --git a/test/utils/math/Math.test.js b/test/utils/math/Math.test.js index afd822b1788..93b98f673c2 100644 --- a/test/utils/math/Math.test.js +++ b/test/utils/math/Math.test.js @@ -244,37 +244,37 @@ contract('Math', function () { describe('muldiv', function () { it('divide by 0', async function () { - await expectRevert.unspecified(this.math.$mulDiv(1, 1, 0, Rounding.Down)); + await expectRevert.unspecified(this.math.$mulDiv(1, 1, 0, Rounding.Floor)); }); it('reverts with result higher than 2 ^ 256', async function () { - await expectRevertCustomError(this.math.$mulDiv(5, MAX_UINT256, 2, Rounding.Down), 'MathOverflowedMulDiv', []); + await expectRevertCustomError(this.math.$mulDiv(5, MAX_UINT256, 2, Rounding.Floor), 'MathOverflowedMulDiv', []); }); describe('does round down', async function () { it('small values', async function () { - expect(await this.math.$mulDiv('3', '4', '5', Rounding.Down)).to.be.bignumber.equal('2'); - expect(await this.math.$mulDiv('3', '5', '5', Rounding.Down)).to.be.bignumber.equal('3'); + expect(await this.math.$mulDiv('3', '4', '5', Rounding.Floor)).to.be.bignumber.equal('2'); + expect(await this.math.$mulDiv('3', '5', '5', Rounding.Floor)).to.be.bignumber.equal('3'); }); it('large values', async function () { expect( - await this.math.$mulDiv(new BN('42'), MAX_UINT256_SUB1, MAX_UINT256, Rounding.Down), + await this.math.$mulDiv(new BN('42'), MAX_UINT256_SUB1, MAX_UINT256, Rounding.Floor), ).to.be.bignumber.equal(new BN('41')); - expect(await this.math.$mulDiv(new BN('17'), MAX_UINT256, MAX_UINT256, Rounding.Down)).to.be.bignumber.equal( + expect(await this.math.$mulDiv(new BN('17'), MAX_UINT256, MAX_UINT256, Rounding.Floor)).to.be.bignumber.equal( new BN('17'), ); expect( - await this.math.$mulDiv(MAX_UINT256_SUB1, MAX_UINT256_SUB1, MAX_UINT256, Rounding.Down), + await this.math.$mulDiv(MAX_UINT256_SUB1, MAX_UINT256_SUB1, MAX_UINT256, Rounding.Floor), ).to.be.bignumber.equal(MAX_UINT256_SUB2); expect( - await this.math.$mulDiv(MAX_UINT256, MAX_UINT256_SUB1, MAX_UINT256, Rounding.Down), + await this.math.$mulDiv(MAX_UINT256, MAX_UINT256_SUB1, MAX_UINT256, Rounding.Floor), ).to.be.bignumber.equal(MAX_UINT256_SUB1); - expect(await this.math.$mulDiv(MAX_UINT256, MAX_UINT256, MAX_UINT256, Rounding.Down)).to.be.bignumber.equal( + expect(await this.math.$mulDiv(MAX_UINT256, MAX_UINT256, MAX_UINT256, Rounding.Floor)).to.be.bignumber.equal( MAX_UINT256, ); }); @@ -282,28 +282,28 @@ contract('Math', function () { describe('does round up', async function () { it('small values', async function () { - expect(await this.math.$mulDiv('3', '4', '5', Rounding.Up)).to.be.bignumber.equal('3'); - expect(await this.math.$mulDiv('3', '5', '5', Rounding.Up)).to.be.bignumber.equal('3'); + expect(await this.math.$mulDiv('3', '4', '5', Rounding.Ceil)).to.be.bignumber.equal('3'); + expect(await this.math.$mulDiv('3', '5', '5', Rounding.Ceil)).to.be.bignumber.equal('3'); }); it('large values', async function () { - expect(await this.math.$mulDiv(new BN('42'), MAX_UINT256_SUB1, MAX_UINT256, Rounding.Up)).to.be.bignumber.equal( + expect(await this.math.$mulDiv(new BN('42'), MAX_UINT256_SUB1, MAX_UINT256, Rounding.Ceil)).to.be.bignumber.equal( new BN('42'), ); - expect(await this.math.$mulDiv(new BN('17'), MAX_UINT256, MAX_UINT256, Rounding.Up)).to.be.bignumber.equal( + expect(await this.math.$mulDiv(new BN('17'), MAX_UINT256, MAX_UINT256, Rounding.Ceil)).to.be.bignumber.equal( new BN('17'), ); expect( - await this.math.$mulDiv(MAX_UINT256_SUB1, MAX_UINT256_SUB1, MAX_UINT256, Rounding.Up), + await this.math.$mulDiv(MAX_UINT256_SUB1, MAX_UINT256_SUB1, MAX_UINT256, Rounding.Ceil), ).to.be.bignumber.equal(MAX_UINT256_SUB1); - expect(await this.math.$mulDiv(MAX_UINT256, MAX_UINT256_SUB1, MAX_UINT256, Rounding.Up)).to.be.bignumber.equal( + expect(await this.math.$mulDiv(MAX_UINT256, MAX_UINT256_SUB1, MAX_UINT256, Rounding.Ceil)).to.be.bignumber.equal( MAX_UINT256_SUB1, ); - expect(await this.math.$mulDiv(MAX_UINT256, MAX_UINT256, MAX_UINT256, Rounding.Up)).to.be.bignumber.equal( + expect(await this.math.$mulDiv(MAX_UINT256, MAX_UINT256, MAX_UINT256, Rounding.Ceil)).to.be.bignumber.equal( MAX_UINT256, ); }); @@ -312,35 +312,35 @@ contract('Math', function () { describe('sqrt', function () { it('rounds down', async function () { - expect(await this.math.$sqrt('0', Rounding.Down)).to.be.bignumber.equal('0'); - expect(await this.math.$sqrt('1', Rounding.Down)).to.be.bignumber.equal('1'); - expect(await this.math.$sqrt('2', Rounding.Down)).to.be.bignumber.equal('1'); - expect(await this.math.$sqrt('3', Rounding.Down)).to.be.bignumber.equal('1'); - expect(await this.math.$sqrt('4', Rounding.Down)).to.be.bignumber.equal('2'); - expect(await this.math.$sqrt('144', Rounding.Down)).to.be.bignumber.equal('12'); - expect(await this.math.$sqrt('999999', Rounding.Down)).to.be.bignumber.equal('999'); - expect(await this.math.$sqrt('1000000', Rounding.Down)).to.be.bignumber.equal('1000'); - expect(await this.math.$sqrt('1000001', Rounding.Down)).to.be.bignumber.equal('1000'); - expect(await this.math.$sqrt('1002000', Rounding.Down)).to.be.bignumber.equal('1000'); - expect(await this.math.$sqrt('1002001', Rounding.Down)).to.be.bignumber.equal('1001'); - expect(await this.math.$sqrt(MAX_UINT256, Rounding.Down)).to.be.bignumber.equal( + expect(await this.math.$sqrt('0', Rounding.Floor)).to.be.bignumber.equal('0'); + expect(await this.math.$sqrt('1', Rounding.Floor)).to.be.bignumber.equal('1'); + expect(await this.math.$sqrt('2', Rounding.Floor)).to.be.bignumber.equal('1'); + expect(await this.math.$sqrt('3', Rounding.Floor)).to.be.bignumber.equal('1'); + expect(await this.math.$sqrt('4', Rounding.Floor)).to.be.bignumber.equal('2'); + expect(await this.math.$sqrt('144', Rounding.Floor)).to.be.bignumber.equal('12'); + expect(await this.math.$sqrt('999999', Rounding.Floor)).to.be.bignumber.equal('999'); + expect(await this.math.$sqrt('1000000', Rounding.Floor)).to.be.bignumber.equal('1000'); + expect(await this.math.$sqrt('1000001', Rounding.Floor)).to.be.bignumber.equal('1000'); + expect(await this.math.$sqrt('1002000', Rounding.Floor)).to.be.bignumber.equal('1000'); + expect(await this.math.$sqrt('1002001', Rounding.Floor)).to.be.bignumber.equal('1001'); + expect(await this.math.$sqrt(MAX_UINT256, Rounding.Floor)).to.be.bignumber.equal( '340282366920938463463374607431768211455', ); }); it('rounds up', async function () { - expect(await this.math.$sqrt('0', Rounding.Up)).to.be.bignumber.equal('0'); - expect(await this.math.$sqrt('1', Rounding.Up)).to.be.bignumber.equal('1'); - expect(await this.math.$sqrt('2', Rounding.Up)).to.be.bignumber.equal('2'); - expect(await this.math.$sqrt('3', Rounding.Up)).to.be.bignumber.equal('2'); - expect(await this.math.$sqrt('4', Rounding.Up)).to.be.bignumber.equal('2'); - expect(await this.math.$sqrt('144', Rounding.Up)).to.be.bignumber.equal('12'); - expect(await this.math.$sqrt('999999', Rounding.Up)).to.be.bignumber.equal('1000'); - expect(await this.math.$sqrt('1000000', Rounding.Up)).to.be.bignumber.equal('1000'); - expect(await this.math.$sqrt('1000001', Rounding.Up)).to.be.bignumber.equal('1001'); - expect(await this.math.$sqrt('1002000', Rounding.Up)).to.be.bignumber.equal('1001'); - expect(await this.math.$sqrt('1002001', Rounding.Up)).to.be.bignumber.equal('1001'); - expect(await this.math.$sqrt(MAX_UINT256, Rounding.Up)).to.be.bignumber.equal( + expect(await this.math.$sqrt('0', Rounding.Ceil)).to.be.bignumber.equal('0'); + expect(await this.math.$sqrt('1', Rounding.Ceil)).to.be.bignumber.equal('1'); + expect(await this.math.$sqrt('2', Rounding.Ceil)).to.be.bignumber.equal('2'); + expect(await this.math.$sqrt('3', Rounding.Ceil)).to.be.bignumber.equal('2'); + expect(await this.math.$sqrt('4', Rounding.Ceil)).to.be.bignumber.equal('2'); + expect(await this.math.$sqrt('144', Rounding.Ceil)).to.be.bignumber.equal('12'); + expect(await this.math.$sqrt('999999', Rounding.Ceil)).to.be.bignumber.equal('1000'); + expect(await this.math.$sqrt('1000000', Rounding.Ceil)).to.be.bignumber.equal('1000'); + expect(await this.math.$sqrt('1000001', Rounding.Ceil)).to.be.bignumber.equal('1001'); + expect(await this.math.$sqrt('1002000', Rounding.Ceil)).to.be.bignumber.equal('1001'); + expect(await this.math.$sqrt('1002001', Rounding.Ceil)).to.be.bignumber.equal('1001'); + expect(await this.math.$sqrt(MAX_UINT256, Rounding.Ceil)).to.be.bignumber.equal( '340282366920938463463374607431768211456', ); }); @@ -350,96 +350,96 @@ contract('Math', function () { describe('log2', function () { it('rounds down', async function () { // For some reason calling .$log2() directly fails - expect(await this.math.methods['$log2(uint256,uint8)']('0', Rounding.Down)).to.be.bignumber.equal('0'); - expect(await this.math.methods['$log2(uint256,uint8)']('1', Rounding.Down)).to.be.bignumber.equal('0'); - expect(await this.math.methods['$log2(uint256,uint8)']('2', Rounding.Down)).to.be.bignumber.equal('1'); - expect(await this.math.methods['$log2(uint256,uint8)']('3', Rounding.Down)).to.be.bignumber.equal('1'); - expect(await this.math.methods['$log2(uint256,uint8)']('4', Rounding.Down)).to.be.bignumber.equal('2'); - expect(await this.math.methods['$log2(uint256,uint8)']('5', Rounding.Down)).to.be.bignumber.equal('2'); - expect(await this.math.methods['$log2(uint256,uint8)']('6', Rounding.Down)).to.be.bignumber.equal('2'); - expect(await this.math.methods['$log2(uint256,uint8)']('7', Rounding.Down)).to.be.bignumber.equal('2'); - expect(await this.math.methods['$log2(uint256,uint8)']('8', Rounding.Down)).to.be.bignumber.equal('3'); - expect(await this.math.methods['$log2(uint256,uint8)']('9', Rounding.Down)).to.be.bignumber.equal('3'); - expect(await this.math.methods['$log2(uint256,uint8)'](MAX_UINT256, Rounding.Down)).to.be.bignumber.equal( + expect(await this.math.methods['$log2(uint256,uint8)']('0', Rounding.Floor)).to.be.bignumber.equal('0'); + expect(await this.math.methods['$log2(uint256,uint8)']('1', Rounding.Floor)).to.be.bignumber.equal('0'); + expect(await this.math.methods['$log2(uint256,uint8)']('2', Rounding.Floor)).to.be.bignumber.equal('1'); + expect(await this.math.methods['$log2(uint256,uint8)']('3', Rounding.Floor)).to.be.bignumber.equal('1'); + expect(await this.math.methods['$log2(uint256,uint8)']('4', Rounding.Floor)).to.be.bignumber.equal('2'); + expect(await this.math.methods['$log2(uint256,uint8)']('5', Rounding.Floor)).to.be.bignumber.equal('2'); + expect(await this.math.methods['$log2(uint256,uint8)']('6', Rounding.Floor)).to.be.bignumber.equal('2'); + expect(await this.math.methods['$log2(uint256,uint8)']('7', Rounding.Floor)).to.be.bignumber.equal('2'); + expect(await this.math.methods['$log2(uint256,uint8)']('8', Rounding.Floor)).to.be.bignumber.equal('3'); + expect(await this.math.methods['$log2(uint256,uint8)']('9', Rounding.Floor)).to.be.bignumber.equal('3'); + expect(await this.math.methods['$log2(uint256,uint8)'](MAX_UINT256, Rounding.Floor)).to.be.bignumber.equal( '255', ); }); it('rounds up', async function () { // For some reason calling .$log2() directly fails - expect(await this.math.methods['$log2(uint256,uint8)']('0', Rounding.Up)).to.be.bignumber.equal('0'); - expect(await this.math.methods['$log2(uint256,uint8)']('1', Rounding.Up)).to.be.bignumber.equal('0'); - expect(await this.math.methods['$log2(uint256,uint8)']('2', Rounding.Up)).to.be.bignumber.equal('1'); - expect(await this.math.methods['$log2(uint256,uint8)']('3', Rounding.Up)).to.be.bignumber.equal('2'); - expect(await this.math.methods['$log2(uint256,uint8)']('4', Rounding.Up)).to.be.bignumber.equal('2'); - expect(await this.math.methods['$log2(uint256,uint8)']('5', Rounding.Up)).to.be.bignumber.equal('3'); - expect(await this.math.methods['$log2(uint256,uint8)']('6', Rounding.Up)).to.be.bignumber.equal('3'); - expect(await this.math.methods['$log2(uint256,uint8)']('7', Rounding.Up)).to.be.bignumber.equal('3'); - expect(await this.math.methods['$log2(uint256,uint8)']('8', Rounding.Up)).to.be.bignumber.equal('3'); - expect(await this.math.methods['$log2(uint256,uint8)']('9', Rounding.Up)).to.be.bignumber.equal('4'); - expect(await this.math.methods['$log2(uint256,uint8)'](MAX_UINT256, Rounding.Up)).to.be.bignumber.equal('256'); + expect(await this.math.methods['$log2(uint256,uint8)']('0', Rounding.Ceil)).to.be.bignumber.equal('0'); + expect(await this.math.methods['$log2(uint256,uint8)']('1', Rounding.Ceil)).to.be.bignumber.equal('0'); + expect(await this.math.methods['$log2(uint256,uint8)']('2', Rounding.Ceil)).to.be.bignumber.equal('1'); + expect(await this.math.methods['$log2(uint256,uint8)']('3', Rounding.Ceil)).to.be.bignumber.equal('2'); + expect(await this.math.methods['$log2(uint256,uint8)']('4', Rounding.Ceil)).to.be.bignumber.equal('2'); + expect(await this.math.methods['$log2(uint256,uint8)']('5', Rounding.Ceil)).to.be.bignumber.equal('3'); + expect(await this.math.methods['$log2(uint256,uint8)']('6', Rounding.Ceil)).to.be.bignumber.equal('3'); + expect(await this.math.methods['$log2(uint256,uint8)']('7', Rounding.Ceil)).to.be.bignumber.equal('3'); + expect(await this.math.methods['$log2(uint256,uint8)']('8', Rounding.Ceil)).to.be.bignumber.equal('3'); + expect(await this.math.methods['$log2(uint256,uint8)']('9', Rounding.Ceil)).to.be.bignumber.equal('4'); + expect(await this.math.methods['$log2(uint256,uint8)'](MAX_UINT256, Rounding.Ceil)).to.be.bignumber.equal('256'); }); }); describe('log10', function () { it('rounds down', async function () { - expect(await this.math.$log10('0', Rounding.Down)).to.be.bignumber.equal('0'); - expect(await this.math.$log10('1', Rounding.Down)).to.be.bignumber.equal('0'); - expect(await this.math.$log10('2', Rounding.Down)).to.be.bignumber.equal('0'); - expect(await this.math.$log10('9', Rounding.Down)).to.be.bignumber.equal('0'); - expect(await this.math.$log10('10', Rounding.Down)).to.be.bignumber.equal('1'); - expect(await this.math.$log10('11', Rounding.Down)).to.be.bignumber.equal('1'); - expect(await this.math.$log10('99', Rounding.Down)).to.be.bignumber.equal('1'); - expect(await this.math.$log10('100', Rounding.Down)).to.be.bignumber.equal('2'); - expect(await this.math.$log10('101', Rounding.Down)).to.be.bignumber.equal('2'); - expect(await this.math.$log10('999', Rounding.Down)).to.be.bignumber.equal('2'); - expect(await this.math.$log10('1000', Rounding.Down)).to.be.bignumber.equal('3'); - expect(await this.math.$log10('1001', Rounding.Down)).to.be.bignumber.equal('3'); - expect(await this.math.$log10(MAX_UINT256, Rounding.Down)).to.be.bignumber.equal('77'); + expect(await this.math.$log10('0', Rounding.Floor)).to.be.bignumber.equal('0'); + expect(await this.math.$log10('1', Rounding.Floor)).to.be.bignumber.equal('0'); + expect(await this.math.$log10('2', Rounding.Floor)).to.be.bignumber.equal('0'); + expect(await this.math.$log10('9', Rounding.Floor)).to.be.bignumber.equal('0'); + expect(await this.math.$log10('10', Rounding.Floor)).to.be.bignumber.equal('1'); + expect(await this.math.$log10('11', Rounding.Floor)).to.be.bignumber.equal('1'); + expect(await this.math.$log10('99', Rounding.Floor)).to.be.bignumber.equal('1'); + expect(await this.math.$log10('100', Rounding.Floor)).to.be.bignumber.equal('2'); + expect(await this.math.$log10('101', Rounding.Floor)).to.be.bignumber.equal('2'); + expect(await this.math.$log10('999', Rounding.Floor)).to.be.bignumber.equal('2'); + expect(await this.math.$log10('1000', Rounding.Floor)).to.be.bignumber.equal('3'); + expect(await this.math.$log10('1001', Rounding.Floor)).to.be.bignumber.equal('3'); + expect(await this.math.$log10(MAX_UINT256, Rounding.Floor)).to.be.bignumber.equal('77'); }); it('rounds up', async function () { - expect(await this.math.$log10('0', Rounding.Up)).to.be.bignumber.equal('0'); - expect(await this.math.$log10('1', Rounding.Up)).to.be.bignumber.equal('0'); - expect(await this.math.$log10('2', Rounding.Up)).to.be.bignumber.equal('1'); - expect(await this.math.$log10('9', Rounding.Up)).to.be.bignumber.equal('1'); - expect(await this.math.$log10('10', Rounding.Up)).to.be.bignumber.equal('1'); - expect(await this.math.$log10('11', Rounding.Up)).to.be.bignumber.equal('2'); - expect(await this.math.$log10('99', Rounding.Up)).to.be.bignumber.equal('2'); - expect(await this.math.$log10('100', Rounding.Up)).to.be.bignumber.equal('2'); - expect(await this.math.$log10('101', Rounding.Up)).to.be.bignumber.equal('3'); - expect(await this.math.$log10('999', Rounding.Up)).to.be.bignumber.equal('3'); - expect(await this.math.$log10('1000', Rounding.Up)).to.be.bignumber.equal('3'); - expect(await this.math.$log10('1001', Rounding.Up)).to.be.bignumber.equal('4'); - expect(await this.math.$log10(MAX_UINT256, Rounding.Up)).to.be.bignumber.equal('78'); + expect(await this.math.$log10('0', Rounding.Ceil)).to.be.bignumber.equal('0'); + expect(await this.math.$log10('1', Rounding.Ceil)).to.be.bignumber.equal('0'); + expect(await this.math.$log10('2', Rounding.Ceil)).to.be.bignumber.equal('1'); + expect(await this.math.$log10('9', Rounding.Ceil)).to.be.bignumber.equal('1'); + expect(await this.math.$log10('10', Rounding.Ceil)).to.be.bignumber.equal('1'); + expect(await this.math.$log10('11', Rounding.Ceil)).to.be.bignumber.equal('2'); + expect(await this.math.$log10('99', Rounding.Ceil)).to.be.bignumber.equal('2'); + expect(await this.math.$log10('100', Rounding.Ceil)).to.be.bignumber.equal('2'); + expect(await this.math.$log10('101', Rounding.Ceil)).to.be.bignumber.equal('3'); + expect(await this.math.$log10('999', Rounding.Ceil)).to.be.bignumber.equal('3'); + expect(await this.math.$log10('1000', Rounding.Ceil)).to.be.bignumber.equal('3'); + expect(await this.math.$log10('1001', Rounding.Ceil)).to.be.bignumber.equal('4'); + expect(await this.math.$log10(MAX_UINT256, Rounding.Ceil)).to.be.bignumber.equal('78'); }); }); describe('log256', function () { it('rounds down', async function () { - expect(await this.math.$log256('0', Rounding.Down)).to.be.bignumber.equal('0'); - expect(await this.math.$log256('1', Rounding.Down)).to.be.bignumber.equal('0'); - expect(await this.math.$log256('2', Rounding.Down)).to.be.bignumber.equal('0'); - expect(await this.math.$log256('255', Rounding.Down)).to.be.bignumber.equal('0'); - expect(await this.math.$log256('256', Rounding.Down)).to.be.bignumber.equal('1'); - expect(await this.math.$log256('257', Rounding.Down)).to.be.bignumber.equal('1'); - expect(await this.math.$log256('65535', Rounding.Down)).to.be.bignumber.equal('1'); - expect(await this.math.$log256('65536', Rounding.Down)).to.be.bignumber.equal('2'); - expect(await this.math.$log256('65537', Rounding.Down)).to.be.bignumber.equal('2'); - expect(await this.math.$log256(MAX_UINT256, Rounding.Down)).to.be.bignumber.equal('31'); + expect(await this.math.$log256('0', Rounding.Floor)).to.be.bignumber.equal('0'); + expect(await this.math.$log256('1', Rounding.Floor)).to.be.bignumber.equal('0'); + expect(await this.math.$log256('2', Rounding.Floor)).to.be.bignumber.equal('0'); + expect(await this.math.$log256('255', Rounding.Floor)).to.be.bignumber.equal('0'); + expect(await this.math.$log256('256', Rounding.Floor)).to.be.bignumber.equal('1'); + expect(await this.math.$log256('257', Rounding.Floor)).to.be.bignumber.equal('1'); + expect(await this.math.$log256('65535', Rounding.Floor)).to.be.bignumber.equal('1'); + expect(await this.math.$log256('65536', Rounding.Floor)).to.be.bignumber.equal('2'); + expect(await this.math.$log256('65537', Rounding.Floor)).to.be.bignumber.equal('2'); + expect(await this.math.$log256(MAX_UINT256, Rounding.Floor)).to.be.bignumber.equal('31'); }); it('rounds up', async function () { - expect(await this.math.$log256('0', Rounding.Up)).to.be.bignumber.equal('0'); - expect(await this.math.$log256('1', Rounding.Up)).to.be.bignumber.equal('0'); - expect(await this.math.$log256('2', Rounding.Up)).to.be.bignumber.equal('1'); - expect(await this.math.$log256('255', Rounding.Up)).to.be.bignumber.equal('1'); - expect(await this.math.$log256('256', Rounding.Up)).to.be.bignumber.equal('1'); - expect(await this.math.$log256('257', Rounding.Up)).to.be.bignumber.equal('2'); - expect(await this.math.$log256('65535', Rounding.Up)).to.be.bignumber.equal('2'); - expect(await this.math.$log256('65536', Rounding.Up)).to.be.bignumber.equal('2'); - expect(await this.math.$log256('65537', Rounding.Up)).to.be.bignumber.equal('3'); - expect(await this.math.$log256(MAX_UINT256, Rounding.Up)).to.be.bignumber.equal('32'); + expect(await this.math.$log256('0', Rounding.Ceil)).to.be.bignumber.equal('0'); + expect(await this.math.$log256('1', Rounding.Ceil)).to.be.bignumber.equal('0'); + expect(await this.math.$log256('2', Rounding.Ceil)).to.be.bignumber.equal('1'); + expect(await this.math.$log256('255', Rounding.Ceil)).to.be.bignumber.equal('1'); + expect(await this.math.$log256('256', Rounding.Ceil)).to.be.bignumber.equal('1'); + expect(await this.math.$log256('257', Rounding.Ceil)).to.be.bignumber.equal('2'); + expect(await this.math.$log256('65535', Rounding.Ceil)).to.be.bignumber.equal('2'); + expect(await this.math.$log256('65536', Rounding.Ceil)).to.be.bignumber.equal('2'); + expect(await this.math.$log256('65537', Rounding.Ceil)).to.be.bignumber.equal('3'); + expect(await this.math.$log256(MAX_UINT256, Rounding.Ceil)).to.be.bignumber.equal('32'); }); }); }); From e902116bd1e0bd92dc4a3fc7c31a4e97e1af18e8 Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Wed, 12 Jul 2023 13:57:05 -0300 Subject: [PATCH 2/6] add changeset --- .changeset/wild-rockets-rush.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/wild-rockets-rush.md diff --git a/.changeset/wild-rockets-rush.md b/.changeset/wild-rockets-rush.md new file mode 100644 index 00000000000..7fc6f598d29 --- /dev/null +++ b/.changeset/wild-rockets-rush.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': major +--- + +`Math`: Renamed members of `Rounding` enum, and added a new rounding mode for "away from zero". From b7b8de348917f30b0cd1215a37a80498e506b8da Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Wed, 12 Jul 2023 17:17:14 -0300 Subject: [PATCH 3/6] improve test coverage --- test/utils/math/Math.test.js | 309 +++++++++++++++++++---------------- 1 file changed, 167 insertions(+), 142 deletions(-) diff --git a/test/utils/math/Math.test.js b/test/utils/math/Math.test.js index 93b98f673c2..8f497bae243 100644 --- a/test/utils/math/Math.test.js +++ b/test/utils/math/Math.test.js @@ -6,6 +6,9 @@ const { expectRevertCustomError } = require('../../helpers/customError.js'); const Math = artifacts.require('$Math'); +const RoundingDown = [Rounding.Floor, Rounding.Trunc]; +const RoundingUp = [Rounding.Ceil, Rounding.Expand]; + function expectStruct(value, expected) { for (const key in expected) { if (BN.isBN(value[key])) { @@ -253,193 +256,215 @@ contract('Math', function () { describe('does round down', async function () { it('small values', async function () { - expect(await this.math.$mulDiv('3', '4', '5', Rounding.Floor)).to.be.bignumber.equal('2'); - expect(await this.math.$mulDiv('3', '5', '5', Rounding.Floor)).to.be.bignumber.equal('3'); + for (const rounding of RoundingDown) { + expect(await this.math.$mulDiv('3', '4', '5', rounding)).to.be.bignumber.equal('2'); + expect(await this.math.$mulDiv('3', '5', '5', rounding)).to.be.bignumber.equal('3'); + } }); it('large values', async function () { - expect( - await this.math.$mulDiv(new BN('42'), MAX_UINT256_SUB1, MAX_UINT256, Rounding.Floor), - ).to.be.bignumber.equal(new BN('41')); - - expect(await this.math.$mulDiv(new BN('17'), MAX_UINT256, MAX_UINT256, Rounding.Floor)).to.be.bignumber.equal( - new BN('17'), - ); - - expect( - await this.math.$mulDiv(MAX_UINT256_SUB1, MAX_UINT256_SUB1, MAX_UINT256, Rounding.Floor), - ).to.be.bignumber.equal(MAX_UINT256_SUB2); - - expect( - await this.math.$mulDiv(MAX_UINT256, MAX_UINT256_SUB1, MAX_UINT256, Rounding.Floor), - ).to.be.bignumber.equal(MAX_UINT256_SUB1); - - expect(await this.math.$mulDiv(MAX_UINT256, MAX_UINT256, MAX_UINT256, Rounding.Floor)).to.be.bignumber.equal( - MAX_UINT256, - ); + for (const rounding of RoundingDown) { + expect( + await this.math.$mulDiv(new BN('42'), MAX_UINT256_SUB1, MAX_UINT256, rounding), + ).to.be.bignumber.equal(new BN('41')); + + expect(await this.math.$mulDiv(new BN('17'), MAX_UINT256, MAX_UINT256, rounding)).to.be.bignumber.equal( + new BN('17'), + ); + + expect( + await this.math.$mulDiv(MAX_UINT256_SUB1, MAX_UINT256_SUB1, MAX_UINT256, rounding), + ).to.be.bignumber.equal(MAX_UINT256_SUB2); + + expect( + await this.math.$mulDiv(MAX_UINT256, MAX_UINT256_SUB1, MAX_UINT256, rounding), + ).to.be.bignumber.equal(MAX_UINT256_SUB1); + + expect(await this.math.$mulDiv(MAX_UINT256, MAX_UINT256, MAX_UINT256, rounding)).to.be.bignumber.equal( + MAX_UINT256, + ); + } }); }); describe('does round up', async function () { it('small values', async function () { - expect(await this.math.$mulDiv('3', '4', '5', Rounding.Ceil)).to.be.bignumber.equal('3'); - expect(await this.math.$mulDiv('3', '5', '5', Rounding.Ceil)).to.be.bignumber.equal('3'); + for (const rounding of RoundingUp) { + expect(await this.math.$mulDiv('3', '4', '5', rounding)).to.be.bignumber.equal('3'); + expect(await this.math.$mulDiv('3', '5', '5', rounding)).to.be.bignumber.equal('3'); + } }); it('large values', async function () { - expect(await this.math.$mulDiv(new BN('42'), MAX_UINT256_SUB1, MAX_UINT256, Rounding.Ceil)).to.be.bignumber.equal( - new BN('42'), - ); - - expect(await this.math.$mulDiv(new BN('17'), MAX_UINT256, MAX_UINT256, Rounding.Ceil)).to.be.bignumber.equal( - new BN('17'), - ); - - expect( - await this.math.$mulDiv(MAX_UINT256_SUB1, MAX_UINT256_SUB1, MAX_UINT256, Rounding.Ceil), - ).to.be.bignumber.equal(MAX_UINT256_SUB1); - - expect(await this.math.$mulDiv(MAX_UINT256, MAX_UINT256_SUB1, MAX_UINT256, Rounding.Ceil)).to.be.bignumber.equal( - MAX_UINT256_SUB1, - ); - - expect(await this.math.$mulDiv(MAX_UINT256, MAX_UINT256, MAX_UINT256, Rounding.Ceil)).to.be.bignumber.equal( - MAX_UINT256, - ); + for (const rounding of RoundingUp) { + expect(await this.math.$mulDiv(new BN('42'), MAX_UINT256_SUB1, MAX_UINT256, rounding)).to.be.bignumber.equal( + new BN('42'), + ); + + expect(await this.math.$mulDiv(new BN('17'), MAX_UINT256, MAX_UINT256, rounding)).to.be.bignumber.equal( + new BN('17'), + ); + + expect( + await this.math.$mulDiv(MAX_UINT256_SUB1, MAX_UINT256_SUB1, MAX_UINT256, rounding), + ).to.be.bignumber.equal(MAX_UINT256_SUB1); + + expect(await this.math.$mulDiv(MAX_UINT256, MAX_UINT256_SUB1, MAX_UINT256, rounding)).to.be.bignumber.equal( + MAX_UINT256_SUB1, + ); + + expect(await this.math.$mulDiv(MAX_UINT256, MAX_UINT256, MAX_UINT256, rounding)).to.be.bignumber.equal( + MAX_UINT256, + ); + } }); }); }); describe('sqrt', function () { it('rounds down', async function () { - expect(await this.math.$sqrt('0', Rounding.Floor)).to.be.bignumber.equal('0'); - expect(await this.math.$sqrt('1', Rounding.Floor)).to.be.bignumber.equal('1'); - expect(await this.math.$sqrt('2', Rounding.Floor)).to.be.bignumber.equal('1'); - expect(await this.math.$sqrt('3', Rounding.Floor)).to.be.bignumber.equal('1'); - expect(await this.math.$sqrt('4', Rounding.Floor)).to.be.bignumber.equal('2'); - expect(await this.math.$sqrt('144', Rounding.Floor)).to.be.bignumber.equal('12'); - expect(await this.math.$sqrt('999999', Rounding.Floor)).to.be.bignumber.equal('999'); - expect(await this.math.$sqrt('1000000', Rounding.Floor)).to.be.bignumber.equal('1000'); - expect(await this.math.$sqrt('1000001', Rounding.Floor)).to.be.bignumber.equal('1000'); - expect(await this.math.$sqrt('1002000', Rounding.Floor)).to.be.bignumber.equal('1000'); - expect(await this.math.$sqrt('1002001', Rounding.Floor)).to.be.bignumber.equal('1001'); - expect(await this.math.$sqrt(MAX_UINT256, Rounding.Floor)).to.be.bignumber.equal( - '340282366920938463463374607431768211455', - ); + for (const rounding of RoundingDown) { + expect(await this.math.$sqrt('0', rounding)).to.be.bignumber.equal('0'); + expect(await this.math.$sqrt('1', rounding)).to.be.bignumber.equal('1'); + expect(await this.math.$sqrt('2', rounding)).to.be.bignumber.equal('1'); + expect(await this.math.$sqrt('3', rounding)).to.be.bignumber.equal('1'); + expect(await this.math.$sqrt('4', rounding)).to.be.bignumber.equal('2'); + expect(await this.math.$sqrt('144', rounding)).to.be.bignumber.equal('12'); + expect(await this.math.$sqrt('999999', rounding)).to.be.bignumber.equal('999'); + expect(await this.math.$sqrt('1000000', rounding)).to.be.bignumber.equal('1000'); + expect(await this.math.$sqrt('1000001', rounding)).to.be.bignumber.equal('1000'); + expect(await this.math.$sqrt('1002000', rounding)).to.be.bignumber.equal('1000'); + expect(await this.math.$sqrt('1002001', rounding)).to.be.bignumber.equal('1001'); + expect(await this.math.$sqrt(MAX_UINT256, rounding)).to.be.bignumber.equal( + '340282366920938463463374607431768211455', + ); + } }); it('rounds up', async function () { - expect(await this.math.$sqrt('0', Rounding.Ceil)).to.be.bignumber.equal('0'); - expect(await this.math.$sqrt('1', Rounding.Ceil)).to.be.bignumber.equal('1'); - expect(await this.math.$sqrt('2', Rounding.Ceil)).to.be.bignumber.equal('2'); - expect(await this.math.$sqrt('3', Rounding.Ceil)).to.be.bignumber.equal('2'); - expect(await this.math.$sqrt('4', Rounding.Ceil)).to.be.bignumber.equal('2'); - expect(await this.math.$sqrt('144', Rounding.Ceil)).to.be.bignumber.equal('12'); - expect(await this.math.$sqrt('999999', Rounding.Ceil)).to.be.bignumber.equal('1000'); - expect(await this.math.$sqrt('1000000', Rounding.Ceil)).to.be.bignumber.equal('1000'); - expect(await this.math.$sqrt('1000001', Rounding.Ceil)).to.be.bignumber.equal('1001'); - expect(await this.math.$sqrt('1002000', Rounding.Ceil)).to.be.bignumber.equal('1001'); - expect(await this.math.$sqrt('1002001', Rounding.Ceil)).to.be.bignumber.equal('1001'); - expect(await this.math.$sqrt(MAX_UINT256, Rounding.Ceil)).to.be.bignumber.equal( - '340282366920938463463374607431768211456', - ); + for (const rounding of RoundingUp) { + expect(await this.math.$sqrt('0', rounding)).to.be.bignumber.equal('0'); + expect(await this.math.$sqrt('1', rounding)).to.be.bignumber.equal('1'); + expect(await this.math.$sqrt('2', rounding)).to.be.bignumber.equal('2'); + expect(await this.math.$sqrt('3', rounding)).to.be.bignumber.equal('2'); + expect(await this.math.$sqrt('4', rounding)).to.be.bignumber.equal('2'); + expect(await this.math.$sqrt('144', rounding)).to.be.bignumber.equal('12'); + expect(await this.math.$sqrt('999999', rounding)).to.be.bignumber.equal('1000'); + expect(await this.math.$sqrt('1000000', rounding)).to.be.bignumber.equal('1000'); + expect(await this.math.$sqrt('1000001', rounding)).to.be.bignumber.equal('1001'); + expect(await this.math.$sqrt('1002000', rounding)).to.be.bignumber.equal('1001'); + expect(await this.math.$sqrt('1002001', rounding)).to.be.bignumber.equal('1001'); + expect(await this.math.$sqrt(MAX_UINT256, rounding)).to.be.bignumber.equal( + '340282366920938463463374607431768211456', + ); + } }); }); describe('log', function () { describe('log2', function () { it('rounds down', async function () { - // For some reason calling .$log2() directly fails - expect(await this.math.methods['$log2(uint256,uint8)']('0', Rounding.Floor)).to.be.bignumber.equal('0'); - expect(await this.math.methods['$log2(uint256,uint8)']('1', Rounding.Floor)).to.be.bignumber.equal('0'); - expect(await this.math.methods['$log2(uint256,uint8)']('2', Rounding.Floor)).to.be.bignumber.equal('1'); - expect(await this.math.methods['$log2(uint256,uint8)']('3', Rounding.Floor)).to.be.bignumber.equal('1'); - expect(await this.math.methods['$log2(uint256,uint8)']('4', Rounding.Floor)).to.be.bignumber.equal('2'); - expect(await this.math.methods['$log2(uint256,uint8)']('5', Rounding.Floor)).to.be.bignumber.equal('2'); - expect(await this.math.methods['$log2(uint256,uint8)']('6', Rounding.Floor)).to.be.bignumber.equal('2'); - expect(await this.math.methods['$log2(uint256,uint8)']('7', Rounding.Floor)).to.be.bignumber.equal('2'); - expect(await this.math.methods['$log2(uint256,uint8)']('8', Rounding.Floor)).to.be.bignumber.equal('3'); - expect(await this.math.methods['$log2(uint256,uint8)']('9', Rounding.Floor)).to.be.bignumber.equal('3'); - expect(await this.math.methods['$log2(uint256,uint8)'](MAX_UINT256, Rounding.Floor)).to.be.bignumber.equal( - '255', - ); + for (const rounding of RoundingDown) { + expect(await this.math.methods['$log2(uint256,uint8)']('0', rounding)).to.be.bignumber.equal('0'); + expect(await this.math.methods['$log2(uint256,uint8)']('1', rounding)).to.be.bignumber.equal('0'); + expect(await this.math.methods['$log2(uint256,uint8)']('2', rounding)).to.be.bignumber.equal('1'); + expect(await this.math.methods['$log2(uint256,uint8)']('3', rounding)).to.be.bignumber.equal('1'); + expect(await this.math.methods['$log2(uint256,uint8)']('4', rounding)).to.be.bignumber.equal('2'); + expect(await this.math.methods['$log2(uint256,uint8)']('5', rounding)).to.be.bignumber.equal('2'); + expect(await this.math.methods['$log2(uint256,uint8)']('6', rounding)).to.be.bignumber.equal('2'); + expect(await this.math.methods['$log2(uint256,uint8)']('7', rounding)).to.be.bignumber.equal('2'); + expect(await this.math.methods['$log2(uint256,uint8)']('8', rounding)).to.be.bignumber.equal('3'); + expect(await this.math.methods['$log2(uint256,uint8)']('9', rounding)).to.be.bignumber.equal('3'); + expect(await this.math.methods['$log2(uint256,uint8)'](MAX_UINT256, rounding)).to.be.bignumber.equal( + '255', + ); + } }); it('rounds up', async function () { - // For some reason calling .$log2() directly fails - expect(await this.math.methods['$log2(uint256,uint8)']('0', Rounding.Ceil)).to.be.bignumber.equal('0'); - expect(await this.math.methods['$log2(uint256,uint8)']('1', Rounding.Ceil)).to.be.bignumber.equal('0'); - expect(await this.math.methods['$log2(uint256,uint8)']('2', Rounding.Ceil)).to.be.bignumber.equal('1'); - expect(await this.math.methods['$log2(uint256,uint8)']('3', Rounding.Ceil)).to.be.bignumber.equal('2'); - expect(await this.math.methods['$log2(uint256,uint8)']('4', Rounding.Ceil)).to.be.bignumber.equal('2'); - expect(await this.math.methods['$log2(uint256,uint8)']('5', Rounding.Ceil)).to.be.bignumber.equal('3'); - expect(await this.math.methods['$log2(uint256,uint8)']('6', Rounding.Ceil)).to.be.bignumber.equal('3'); - expect(await this.math.methods['$log2(uint256,uint8)']('7', Rounding.Ceil)).to.be.bignumber.equal('3'); - expect(await this.math.methods['$log2(uint256,uint8)']('8', Rounding.Ceil)).to.be.bignumber.equal('3'); - expect(await this.math.methods['$log2(uint256,uint8)']('9', Rounding.Ceil)).to.be.bignumber.equal('4'); - expect(await this.math.methods['$log2(uint256,uint8)'](MAX_UINT256, Rounding.Ceil)).to.be.bignumber.equal('256'); + for (const rounding of RoundingUp) { + expect(await this.math.methods['$log2(uint256,uint8)']('0', rounding)).to.be.bignumber.equal('0'); + expect(await this.math.methods['$log2(uint256,uint8)']('1', rounding)).to.be.bignumber.equal('0'); + expect(await this.math.methods['$log2(uint256,uint8)']('2', rounding)).to.be.bignumber.equal('1'); + expect(await this.math.methods['$log2(uint256,uint8)']('3', rounding)).to.be.bignumber.equal('2'); + expect(await this.math.methods['$log2(uint256,uint8)']('4', rounding)).to.be.bignumber.equal('2'); + expect(await this.math.methods['$log2(uint256,uint8)']('5', rounding)).to.be.bignumber.equal('3'); + expect(await this.math.methods['$log2(uint256,uint8)']('6', rounding)).to.be.bignumber.equal('3'); + expect(await this.math.methods['$log2(uint256,uint8)']('7', rounding)).to.be.bignumber.equal('3'); + expect(await this.math.methods['$log2(uint256,uint8)']('8', rounding)).to.be.bignumber.equal('3'); + expect(await this.math.methods['$log2(uint256,uint8)']('9', rounding)).to.be.bignumber.equal('4'); + expect(await this.math.methods['$log2(uint256,uint8)'](MAX_UINT256, rounding)).to.be.bignumber.equal('256'); + } }); }); describe('log10', function () { it('rounds down', async function () { - expect(await this.math.$log10('0', Rounding.Floor)).to.be.bignumber.equal('0'); - expect(await this.math.$log10('1', Rounding.Floor)).to.be.bignumber.equal('0'); - expect(await this.math.$log10('2', Rounding.Floor)).to.be.bignumber.equal('0'); - expect(await this.math.$log10('9', Rounding.Floor)).to.be.bignumber.equal('0'); - expect(await this.math.$log10('10', Rounding.Floor)).to.be.bignumber.equal('1'); - expect(await this.math.$log10('11', Rounding.Floor)).to.be.bignumber.equal('1'); - expect(await this.math.$log10('99', Rounding.Floor)).to.be.bignumber.equal('1'); - expect(await this.math.$log10('100', Rounding.Floor)).to.be.bignumber.equal('2'); - expect(await this.math.$log10('101', Rounding.Floor)).to.be.bignumber.equal('2'); - expect(await this.math.$log10('999', Rounding.Floor)).to.be.bignumber.equal('2'); - expect(await this.math.$log10('1000', Rounding.Floor)).to.be.bignumber.equal('3'); - expect(await this.math.$log10('1001', Rounding.Floor)).to.be.bignumber.equal('3'); - expect(await this.math.$log10(MAX_UINT256, Rounding.Floor)).to.be.bignumber.equal('77'); + for (const rounding of RoundingDown) { + expect(await this.math.$log10('0', rounding)).to.be.bignumber.equal('0'); + expect(await this.math.$log10('1', rounding)).to.be.bignumber.equal('0'); + expect(await this.math.$log10('2', rounding)).to.be.bignumber.equal('0'); + expect(await this.math.$log10('9', rounding)).to.be.bignumber.equal('0'); + expect(await this.math.$log10('10', rounding)).to.be.bignumber.equal('1'); + expect(await this.math.$log10('11', rounding)).to.be.bignumber.equal('1'); + expect(await this.math.$log10('99', rounding)).to.be.bignumber.equal('1'); + expect(await this.math.$log10('100', rounding)).to.be.bignumber.equal('2'); + expect(await this.math.$log10('101', rounding)).to.be.bignumber.equal('2'); + expect(await this.math.$log10('999', rounding)).to.be.bignumber.equal('2'); + expect(await this.math.$log10('1000', rounding)).to.be.bignumber.equal('3'); + expect(await this.math.$log10('1001', rounding)).to.be.bignumber.equal('3'); + expect(await this.math.$log10(MAX_UINT256, rounding)).to.be.bignumber.equal('77'); + } }); it('rounds up', async function () { - expect(await this.math.$log10('0', Rounding.Ceil)).to.be.bignumber.equal('0'); - expect(await this.math.$log10('1', Rounding.Ceil)).to.be.bignumber.equal('0'); - expect(await this.math.$log10('2', Rounding.Ceil)).to.be.bignumber.equal('1'); - expect(await this.math.$log10('9', Rounding.Ceil)).to.be.bignumber.equal('1'); - expect(await this.math.$log10('10', Rounding.Ceil)).to.be.bignumber.equal('1'); - expect(await this.math.$log10('11', Rounding.Ceil)).to.be.bignumber.equal('2'); - expect(await this.math.$log10('99', Rounding.Ceil)).to.be.bignumber.equal('2'); - expect(await this.math.$log10('100', Rounding.Ceil)).to.be.bignumber.equal('2'); - expect(await this.math.$log10('101', Rounding.Ceil)).to.be.bignumber.equal('3'); - expect(await this.math.$log10('999', Rounding.Ceil)).to.be.bignumber.equal('3'); - expect(await this.math.$log10('1000', Rounding.Ceil)).to.be.bignumber.equal('3'); - expect(await this.math.$log10('1001', Rounding.Ceil)).to.be.bignumber.equal('4'); - expect(await this.math.$log10(MAX_UINT256, Rounding.Ceil)).to.be.bignumber.equal('78'); + for (const rounding of RoundingUp) { + expect(await this.math.$log10('0', rounding)).to.be.bignumber.equal('0'); + expect(await this.math.$log10('1', rounding)).to.be.bignumber.equal('0'); + expect(await this.math.$log10('2', rounding)).to.be.bignumber.equal('1'); + expect(await this.math.$log10('9', rounding)).to.be.bignumber.equal('1'); + expect(await this.math.$log10('10', rounding)).to.be.bignumber.equal('1'); + expect(await this.math.$log10('11', rounding)).to.be.bignumber.equal('2'); + expect(await this.math.$log10('99', rounding)).to.be.bignumber.equal('2'); + expect(await this.math.$log10('100', rounding)).to.be.bignumber.equal('2'); + expect(await this.math.$log10('101', rounding)).to.be.bignumber.equal('3'); + expect(await this.math.$log10('999', rounding)).to.be.bignumber.equal('3'); + expect(await this.math.$log10('1000', rounding)).to.be.bignumber.equal('3'); + expect(await this.math.$log10('1001', rounding)).to.be.bignumber.equal('4'); + expect(await this.math.$log10(MAX_UINT256, rounding)).to.be.bignumber.equal('78'); + } }); }); describe('log256', function () { it('rounds down', async function () { - expect(await this.math.$log256('0', Rounding.Floor)).to.be.bignumber.equal('0'); - expect(await this.math.$log256('1', Rounding.Floor)).to.be.bignumber.equal('0'); - expect(await this.math.$log256('2', Rounding.Floor)).to.be.bignumber.equal('0'); - expect(await this.math.$log256('255', Rounding.Floor)).to.be.bignumber.equal('0'); - expect(await this.math.$log256('256', Rounding.Floor)).to.be.bignumber.equal('1'); - expect(await this.math.$log256('257', Rounding.Floor)).to.be.bignumber.equal('1'); - expect(await this.math.$log256('65535', Rounding.Floor)).to.be.bignumber.equal('1'); - expect(await this.math.$log256('65536', Rounding.Floor)).to.be.bignumber.equal('2'); - expect(await this.math.$log256('65537', Rounding.Floor)).to.be.bignumber.equal('2'); - expect(await this.math.$log256(MAX_UINT256, Rounding.Floor)).to.be.bignumber.equal('31'); + for (const rounding of RoundingDown) { + expect(await this.math.$log256('0', rounding)).to.be.bignumber.equal('0'); + expect(await this.math.$log256('1', rounding)).to.be.bignumber.equal('0'); + expect(await this.math.$log256('2', rounding)).to.be.bignumber.equal('0'); + expect(await this.math.$log256('255', rounding)).to.be.bignumber.equal('0'); + expect(await this.math.$log256('256', rounding)).to.be.bignumber.equal('1'); + expect(await this.math.$log256('257', rounding)).to.be.bignumber.equal('1'); + expect(await this.math.$log256('65535', rounding)).to.be.bignumber.equal('1'); + expect(await this.math.$log256('65536', rounding)).to.be.bignumber.equal('2'); + expect(await this.math.$log256('65537', rounding)).to.be.bignumber.equal('2'); + expect(await this.math.$log256(MAX_UINT256, rounding)).to.be.bignumber.equal('31'); + } }); it('rounds up', async function () { - expect(await this.math.$log256('0', Rounding.Ceil)).to.be.bignumber.equal('0'); - expect(await this.math.$log256('1', Rounding.Ceil)).to.be.bignumber.equal('0'); - expect(await this.math.$log256('2', Rounding.Ceil)).to.be.bignumber.equal('1'); - expect(await this.math.$log256('255', Rounding.Ceil)).to.be.bignumber.equal('1'); - expect(await this.math.$log256('256', Rounding.Ceil)).to.be.bignumber.equal('1'); - expect(await this.math.$log256('257', Rounding.Ceil)).to.be.bignumber.equal('2'); - expect(await this.math.$log256('65535', Rounding.Ceil)).to.be.bignumber.equal('2'); - expect(await this.math.$log256('65536', Rounding.Ceil)).to.be.bignumber.equal('2'); - expect(await this.math.$log256('65537', Rounding.Ceil)).to.be.bignumber.equal('3'); - expect(await this.math.$log256(MAX_UINT256, Rounding.Ceil)).to.be.bignumber.equal('32'); + for (const rounding of RoundingUp) { + expect(await this.math.$log256('0', rounding)).to.be.bignumber.equal('0'); + expect(await this.math.$log256('1', rounding)).to.be.bignumber.equal('0'); + expect(await this.math.$log256('2', rounding)).to.be.bignumber.equal('1'); + expect(await this.math.$log256('255', rounding)).to.be.bignumber.equal('1'); + expect(await this.math.$log256('256', rounding)).to.be.bignumber.equal('1'); + expect(await this.math.$log256('257', rounding)).to.be.bignumber.equal('2'); + expect(await this.math.$log256('65535', rounding)).to.be.bignumber.equal('2'); + expect(await this.math.$log256('65536', rounding)).to.be.bignumber.equal('2'); + expect(await this.math.$log256('65537', rounding)).to.be.bignumber.equal('3'); + expect(await this.math.$log256(MAX_UINT256, rounding)).to.be.bignumber.equal('32'); + } }); }); }); From 801ca6173c4cd68e7b1d013efd309f6e712de5dc Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Wed, 12 Jul 2023 17:28:13 -0300 Subject: [PATCH 4/6] lint --- test/utils/math/Math.test.js | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/test/utils/math/Math.test.js b/test/utils/math/Math.test.js index 8f497bae243..7d4a58c81b1 100644 --- a/test/utils/math/Math.test.js +++ b/test/utils/math/Math.test.js @@ -264,9 +264,9 @@ contract('Math', function () { it('large values', async function () { for (const rounding of RoundingDown) { - expect( - await this.math.$mulDiv(new BN('42'), MAX_UINT256_SUB1, MAX_UINT256, rounding), - ).to.be.bignumber.equal(new BN('41')); + expect(await this.math.$mulDiv(new BN('42'), MAX_UINT256_SUB1, MAX_UINT256, rounding)).to.be.bignumber.equal( + new BN('41'), + ); expect(await this.math.$mulDiv(new BN('17'), MAX_UINT256, MAX_UINT256, rounding)).to.be.bignumber.equal( new BN('17'), @@ -276,9 +276,9 @@ contract('Math', function () { await this.math.$mulDiv(MAX_UINT256_SUB1, MAX_UINT256_SUB1, MAX_UINT256, rounding), ).to.be.bignumber.equal(MAX_UINT256_SUB2); - expect( - await this.math.$mulDiv(MAX_UINT256, MAX_UINT256_SUB1, MAX_UINT256, rounding), - ).to.be.bignumber.equal(MAX_UINT256_SUB1); + expect(await this.math.$mulDiv(MAX_UINT256, MAX_UINT256_SUB1, MAX_UINT256, rounding)).to.be.bignumber.equal( + MAX_UINT256_SUB1, + ); expect(await this.math.$mulDiv(MAX_UINT256, MAX_UINT256, MAX_UINT256, rounding)).to.be.bignumber.equal( MAX_UINT256, @@ -375,9 +375,7 @@ contract('Math', function () { expect(await this.math.methods['$log2(uint256,uint8)']('7', rounding)).to.be.bignumber.equal('2'); expect(await this.math.methods['$log2(uint256,uint8)']('8', rounding)).to.be.bignumber.equal('3'); expect(await this.math.methods['$log2(uint256,uint8)']('9', rounding)).to.be.bignumber.equal('3'); - expect(await this.math.methods['$log2(uint256,uint8)'](MAX_UINT256, rounding)).to.be.bignumber.equal( - '255', - ); + expect(await this.math.methods['$log2(uint256,uint8)'](MAX_UINT256, rounding)).to.be.bignumber.equal('255'); } }); From 938bce01203ca381df7fbbcb11c647fe14bb1536 Mon Sep 17 00:00:00 2001 From: ernestognw Date: Thu, 13 Jul 2023 12:39:51 -0600 Subject: [PATCH 5/6] Change rounding descriptions --- contracts/utils/Arrays.sol | 2 +- contracts/utils/math/Math.sol | 13 +++++++------ docs/modules/ROOT/pages/erc4626.adoc | 2 +- test/token/ERC20/extensions/ERC4626.test.js | 6 +++--- 4 files changed, 12 insertions(+), 11 deletions(-) diff --git a/contracts/utils/Arrays.sol b/contracts/utils/Arrays.sol index 7dbc86fdd52..0e82e54e52a 100644 --- a/contracts/utils/Arrays.sol +++ b/contracts/utils/Arrays.sol @@ -33,7 +33,7 @@ library Arrays { uint256 mid = Math.average(low, high); // Note that mid will always be strictly less than high (i.e. it will be a valid array index) - // because Math.average rounds down (it does integer division with truncation). + // because Math.average rounds towards zero (it does integer division with truncation). if (unsafeAccess(array, mid).value > element) { high = mid; } else { diff --git a/contracts/utils/math/Math.sol b/contracts/utils/math/Math.sol index 52fe81891d2..1f8dd34fe0f 100644 --- a/contracts/utils/math/Math.sol +++ b/contracts/utils/math/Math.sol @@ -101,8 +101,8 @@ library Math { /** * @dev Returns the ceiling of the division of two numbers. * - * This differs from standard division with `/` in that it rounds up instead - * of rounding down. + * This differs from standard division with `/` in that it rounds towards infinity instead + * of rounding towards zero. */ function ceilDiv(uint256 a, uint256 b) internal pure returns (uint256) { if (b == 0) { @@ -214,7 +214,8 @@ library Math { } /** - * @dev Returns the square root of a number. If the number is not a perfect square, the value is rounded down. + * @dev Returns the square root of a number. If the number is not a perfect square, the value is rounded + * towards zero. * * Inspired by Henry S. Warren, Jr.'s "Hacker's Delight" (Chapter 11). */ @@ -262,7 +263,7 @@ library Math { } /** - * @dev Return the log in base 2, rounded down, of a positive value. + * @dev Return the log in base 2 of a positive value rounded towards zero. * Returns 0 if given 0. */ function log2(uint256 value) internal pure returns (uint256) { @@ -315,7 +316,7 @@ library Math { } /** - * @dev Return the log in base 10, rounded down, of a positive value. + * @dev Return the log in base 10 of a positive value rounded towards zero. * Returns 0 if given 0. */ function log10(uint256 value) internal pure returns (uint256) { @@ -364,7 +365,7 @@ library Math { } /** - * @dev Return the log in base 256, rounded down, of a positive value. + * @dev Return the log in base 256 of a positive value rounded towards zero. * Returns 0 if given 0. * * Adding one to the result gives the number of pairs of hex symbols needed to represent `value` as a hex string. diff --git a/docs/modules/ROOT/pages/erc4626.adoc b/docs/modules/ROOT/pages/erc4626.adoc index c8adce73672..43ec3ebb939 100644 --- a/docs/modules/ROOT/pages/erc4626.adoc +++ b/docs/modules/ROOT/pages/erc4626.adoc @@ -29,7 +29,7 @@ image::erc4626-rate-loglogext.png[More exchange rates in logarithmic scale] === The attack -When depositing tokens, the number of shares a user gets is rounded down. This rounding takes away value from the user in favor or the vault (i.e. in favor of all the current share holders). This rounding is often negligible because of the amount at stake. If you deposit 1e9 shares worth of tokens, the rounding will have you lose at most 0.0000001% of your deposit. However if you deposit 10 shares worth of tokens, you could lose 10% of your deposit. Even worse, if you deposit <1 share worth of tokens, then you get 0 shares, and you basically made a donation. +When depositing tokens, the number of shares a user gets is rounded towards zero. This rounding takes away value from the user in favor or the vault (i.e. in favor of all the current share holders). This rounding is often negligible because of the amount at stake. If you deposit 1e9 shares worth of tokens, the rounding will have you lose at most 0.0000001% of your deposit. However if you deposit 10 shares worth of tokens, you could lose 10% of your deposit. Even worse, if you deposit <1 share worth of tokens, then you get 0 shares, and you basically made a donation. For a given amount of assets, the more shares you receive the safer you are. If you want to limit your losses to at most 1%, you need to receive at least 100 shares. diff --git a/test/token/ERC20/extensions/ERC4626.test.js b/test/token/ERC20/extensions/ERC4626.test.js index 9dfc4c3b818..aead1d50a4c 100644 --- a/test/token/ERC20/extensions/ERC4626.test.js +++ b/test/token/ERC20/extensions/ERC4626.test.js @@ -965,8 +965,8 @@ contract('ERC4626', function (accounts) { } // 5. Bob mints 2000 shares (costs 3001 assets) - // NOTE: Bob's assets spent got rounded up - // NOTE: Alices's vault assets got rounded up + // NOTE: Bob's assets spent got rounded towards infinity + // NOTE: Alices's vault assets got rounded towards infinity { const { tx } = await this.vault.mint(2000, user2, { from: user2 }); await expectEvent.inTransaction(tx, this.token, 'Transfer', { @@ -1056,7 +1056,7 @@ contract('ERC4626', function (accounts) { } // 9. Alice withdraws 3643 assets (2000 shares) - // NOTE: Bob's assets have been rounded back up + // NOTE: Bob's assets have been rounded back towards infinity { const { tx } = await this.vault.withdraw(3643, user1, user1, { from: user1 }); await expectEvent.inTransaction(tx, this.vault, 'Transfer', { From 12e6dad043e99026b98575647f7874ae60446c12 Mon Sep 17 00:00:00 2001 From: Francisco Date: Thu, 13 Jul 2023 17:47:52 -0300 Subject: [PATCH 6/6] Update contracts/utils/math/Math.sol --- contracts/utils/math/Math.sol | 3 +++ 1 file changed, 3 insertions(+) diff --git a/contracts/utils/math/Math.sol b/contracts/utils/math/Math.sol index 1f8dd34fe0f..e3d7f799d41 100644 --- a/contracts/utils/math/Math.sol +++ b/contracts/utils/math/Math.sol @@ -407,6 +407,9 @@ library Math { } } + /** + * @dev Returns whether a provided rounding mode is considered rounding up for unsigned integers. + */ function unsignedRoundsUp(Rounding rounding) internal pure returns (bool) { return uint8(rounding) % 2 == 1; }