Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename rounding modes and complete with fourth #4455

Merged
merged 6 commits into from
Jul 13, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/wild-rockets-rush.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'openzeppelin-solidity': major
---

`Math`: Renamed members of `Rounding` enum, and added a new rounding mode for "away from zero".
4 changes: 2 additions & 2 deletions contracts/mocks/docs/ERC4626Fees.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
14 changes: 7 additions & 7 deletions contracts/token/ERC20/extensions/ERC4626.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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}. */
Expand All @@ -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}. */
Expand All @@ -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}. */
Expand Down
21 changes: 13 additions & 8 deletions contracts/utils/math/Math.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

/**
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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) {
frangio marked this conversation as resolved.
Show resolved Hide resolved
return uint8(rounding) % 2 == 1;
}
}
2 changes: 1 addition & 1 deletion test/helpers/enums.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
};
16 changes: 8 additions & 8 deletions test/utils/math/Math.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down