-
Notifications
You must be signed in to change notification settings - Fork 11.7k
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
Support Saturation Arithmetic Operations #5029
base: master
Are you sure you want to change the base?
Changes from 5 commits
b51a340
47c77d9
0109a89
fa7b02e
07bcce5
d513400
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'openzeppelin-solidity': minor | ||
--- | ||
|
||
`Math`, `SignedMath`: Add saturating arithmetic operations, such as `saturatingAdd`, `saturatingSub` and `saturatingMul`. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,14 +17,36 @@ library Math { | |
Expand // Away from zero | ||
} | ||
|
||
/** | ||
* @dev Unsigned saturating addition, bounds to `2 ** 256 - 1` instead of overflowing. | ||
*/ | ||
function saturatingAdd(uint256 a, uint256 b) internal pure returns (uint256) { | ||
unchecked { | ||
a = a + b; | ||
b = 0 - SafeCast.toUint(a < b); | ||
return a | b; | ||
} | ||
} | ||
|
||
/** | ||
* @dev Returns the addition of two unsigned integers, with an success flag (no overflow). | ||
*/ | ||
function tryAdd(uint256 a, uint256 b) internal pure returns (bool success, uint256 result) { | ||
unchecked { | ||
uint256 c = a + b; | ||
if (c < a) return (false, 0); | ||
return (true, c); | ||
success = c >= a; | ||
// equivalent to: c >= a ? c : 0 | ||
result = SafeCast.toUint(success) * c; | ||
} | ||
} | ||
|
||
/** | ||
* @dev Unsigned saturating subtraction, bounds to zero instead of overflowing. | ||
*/ | ||
function saturatingSub(uint256 a, uint256 b) internal pure returns (uint256) { | ||
unchecked { | ||
// equivalent to: a >= b ? a - b : 0 | ||
return (a - b) * SafeCast.toUint(a > b); | ||
} | ||
} | ||
|
||
|
@@ -33,8 +55,25 @@ library Math { | |
*/ | ||
function trySub(uint256 a, uint256 b) internal pure returns (bool success, uint256 result) { | ||
unchecked { | ||
if (b > a) return (false, 0); | ||
return (true, a - b); | ||
success = a >= b; | ||
// equivalent to: success ? c : 0 | ||
result = SafeCast.toUint(success) * (a - b); | ||
} | ||
} | ||
|
||
/** | ||
* @dev Unsigned saturating multiplication, bounds to `2 ** 256 - 1` instead of overflowing. | ||
*/ | ||
function saturatingMul(uint256 a, uint256 b) internal pure returns (uint256) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. use of assembly may be efficient, but readability is not great. I'd be more confortable with that function being (bool success, uint256 result) = tryMul(a, b);
return ternary(success, result, type(uint256).max); There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I had to use assembly because There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Btw I opened an issue in the solidity compiler, division should not revert in unchecked blocks: |
||
unchecked { | ||
uint256 c = a * b; | ||
bool success; | ||
assembly { | ||
// Only true when the multiplication doesn't overflow | ||
// (c / a == b) || (a == 0) | ||
success := or(eq(div(c, a), b), iszero(a)) | ||
} | ||
return c | (SafeCast.toUint(success) - 1); | ||
} | ||
} | ||
|
||
|
@@ -43,13 +82,14 @@ library Math { | |
*/ | ||
function tryMul(uint256 a, uint256 b) internal pure returns (bool success, uint256 result) { | ||
unchecked { | ||
// Gas optimization: this is cheaper than requiring 'a' not being zero, but the | ||
// benefit is lost if 'b' is also tested. | ||
// See: https://github.com/OpenZeppelin/openzeppelin-contracts/pull/522 | ||
if (a == 0) return (true, 0); | ||
uint256 c = a * b; | ||
if (c / a != b) return (false, 0); | ||
return (true, c); | ||
assembly { | ||
// Only true when the multiplication doesn't overflow | ||
// (c / a == b) || (a == 0) | ||
success := or(eq(div(c, a), b), iszero(a)) | ||
} | ||
// equivalent to: success ? c : 0 | ||
result = SafeCast.toUint(success) * c; | ||
} | ||
} | ||
|
||
|
@@ -58,8 +98,11 @@ library Math { | |
*/ | ||
function tryDiv(uint256 a, uint256 b) internal pure returns (bool success, uint256 result) { | ||
unchecked { | ||
if (b == 0) return (false, 0); | ||
return (true, a / b); | ||
success = b > 0; | ||
assembly { | ||
// In EVM any value divided by zero is zero. | ||
result := div(a, b) | ||
} | ||
} | ||
} | ||
|
||
|
@@ -68,8 +111,11 @@ library Math { | |
*/ | ||
function tryMod(uint256 a, uint256 b) internal pure returns (bool success, uint256 result) { | ||
unchecked { | ||
if (b == 0) return (false, 0); | ||
return (true, a % b); | ||
success = b > 0; | ||
assembly { | ||
// In EVM a value modulus zero is equal to zero. | ||
result := mod(a, b) | ||
} | ||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -65,4 +65,74 @@ library SignedMath { | |
return uint256((n + mask) ^ mask); | ||
} | ||
} | ||
|
||
/** | ||
* @dev Signed saturating addition, computes `a + b` saturating at the numeric bounds instead of overflowing. | ||
*/ | ||
function saturatingAdd(int256 a, int256 b) internal pure returns (int256) { | ||
unchecked { | ||
int256 c = a + b; | ||
// Rationale: | ||
// - overflow is only possible when both `a` and `b` are positive | ||
// - underflow is only possible when both `a` and `b` are negative | ||
// | ||
// Lemma: | ||
// (i) - if `a > (a + b)` is true, then `b` MUST be negative, otherwise overflow happened. | ||
// (ii) - if `a > (a + b)` is false, then `b` MUST be non-negative, otherwise underflow happened. | ||
// | ||
// So the following statement will be true only if an overflow or underflow happened: | ||
// statement: a > (a + b) == (b >= 0) | ||
// | ||
// We can use the sign of `b` to distinguish between overflow and underflow, as demonstrated below: | ||
// | a > (a + b) | b >= 0 | | ||
// | true | true | Lemma (i) thus Overflow | ||
// | false | false | Lemma (ii) thus Underflow | ||
// | true | false | Ok | ||
// | false | true | Ok | ||
bool sign = b >= 0; | ||
bool overflow = a > c == sign; | ||
|
||
// Efficient branchless method to retrieve the boundary limit: | ||
// (1 << 255) == type(int256).min | ||
// (1 << 255) - 1 == type(int256).max | ||
uint256 limit = (SafeCast.toUint(overflow) << 255) - SafeCast.toUint(sign); | ||
|
||
return ternary(overflow, int256(limit), c); | ||
Comment on lines
+92
to
+100
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For this one, this is the assembly implementation, it uses
|
||
} | ||
} | ||
|
||
/** | ||
* @dev Signed saturating subtraction, computes `a - b` saturating at the numeric bounds instead of overflowing. | ||
*/ | ||
function saturatingSub(int256 a, int256 b) internal pure returns (int256) { | ||
unchecked { | ||
int256 c = a - b; | ||
// Rationale: | ||
// - overflow is only possible when `a` is zero or positive and `b` is negative | ||
// - underflow is only possible when `a` is negative and `b` is positive | ||
// | ||
// Lemma: | ||
// (i) - if `a >= (a - b)` is true, then `b` MUST be non-negative, otherwise overflow happened. | ||
// (ii) - if `a >= (a - b)` is false, then `b` MUST be negative, otherwise underflow happened. | ||
// | ||
// So the following statement will be true only if an overflow or underflow happened: | ||
// statement: a >= (a - b) == (b < 0) | ||
// | ||
// We can use the sign of `b` to distinguish between overflow and underflow, as demonstrated below: | ||
// | a >= (a - b) | b < 0 | | ||
// | true | true | Lemma (i) thus Overflow | ||
// | false | false | Lemma (ii) thus Underflow | ||
// | true | false | Ok | ||
// | false | true | Ok | ||
bool sign = b < 0; | ||
bool overflow = a >= c == sign; | ||
|
||
// Efficient branchless method to retrieve the boundary limit: | ||
// (1 << 255) == type(int256).min | ||
// (1 << 255) - 1 == type(int256).max | ||
uint256 limit = (SafeCast.toUint(overflow) << 255) - SafeCast.toUint(sign); | ||
|
||
return ternary(overflow, int256(limit), c); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could also do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's actually a good point, because the ternary operator looks like:
Whoever if
a
orb
is zero, it can be simplified to:So it works like an
and
operator, I really wish the compiler could do this kind of optimization for us, I was wondering if creating a new operator would be confusing, like:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Otherwise I can refactor it for use the
ternary
at a little gas cost but increased code readability.