Signed SafeMath (#1559)

* signed safe math

* fix lint errors

* refactor overflow checks and add descriptions

* remove incorrect description

* add test for reversed arguments in multiplication test

* fix power operator

* improve multiplication test descriptions

* Update SafeMath.test.js

* add feature to changelog
This commit is contained in:
Francisco Giordano
2018-12-18 15:54:44 -03:00
committed by GitHub
parent fad30c3d2c
commit d17ae0b806
5 changed files with 325 additions and 121 deletions

View File

@ -9,6 +9,7 @@
* `ERC20`: `transferFrom` and `_burnFrom ` now emit `Approval` events, to represent the token's state comprehensively through events. ([#1524](https://github.com/OpenZeppelin/openzeppelin-solidity/pull/1524))
* `ERC721`: added `_burn(uint256 tokenId)`, replacing the similar deprecated function (see below). ([#1550](https://github.com/OpenZeppelin/openzeppelin-solidity/pull/1550))
* `ERC721`: added `_tokensOfOwner(address owner)`, allowing to internally retrieve the array of an account's owned tokens. ([#1522](https://github.com/OpenZeppelin/openzeppelin-solidity/pull/1522))
* `SafeMath`: added overflow-safe operations for signed integers (`int256`). ([#1559](https://github.com/OpenZeppelin/openzeppelin-solidity/pull/1559))
### Improvements:
* The compiler version required by `Array` was behind the rest of the libray so it was updated to `v0.4.24`. ([#1553](https://github.com/OpenZeppelin/openzeppelin-solidity/pull/1553))

View File

@ -5,8 +5,10 @@ pragma solidity ^0.4.24;
* @dev Math operations with safety checks that revert on error
*/
library SafeMath {
int256 constant private INT256_MIN = -2**255;
/**
* @dev Multiplies two numbers, reverts on overflow.
* @dev Multiplies two unsigned integers, reverts on overflow.
*/
function mul(uint256 a, uint256 b) internal pure returns (uint256) {
// Gas optimization: this is cheaper than requiring 'a' not being zero, but the
@ -23,7 +25,26 @@ library SafeMath {
}
/**
* @dev Integer division of two numbers truncating the quotient, reverts on division by zero.
* @dev Multiplies two signed integers, reverts on overflow.
*/
function mul(int256 a, int256 b) internal pure returns (int256) {
// 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-solidity/pull/522
if (a == 0) {
return 0;
}
require(!(a == -1 && b == INT256_MIN)); // This is the only case of overflow not detected by the check below
int256 c = a * b;
require(c / a == b);
return c;
}
/**
* @dev Integer division of two unsigned integers truncating the quotient, reverts on division by zero.
*/
function div(uint256 a, uint256 b) internal pure returns (uint256) {
// Solidity only automatically asserts when dividing by 0
@ -35,7 +56,19 @@ library SafeMath {
}
/**
* @dev Subtracts two numbers, reverts on overflow (i.e. if subtrahend is greater than minuend).
* @dev Integer division of two signed integers truncating the quotient, reverts on division by zero.
*/
function div(int256 a, int256 b) internal pure returns (int256) {
require(b != 0); // Solidity only automatically asserts when dividing by 0
require(!(b == -1 && a == INT256_MIN)); // This is the only case of overflow
int256 c = a / b;
return c;
}
/**
* @dev Subtracts two unsigned integers, reverts on overflow (i.e. if subtrahend is greater than minuend).
*/
function sub(uint256 a, uint256 b) internal pure returns (uint256) {
require(b <= a);
@ -45,7 +78,17 @@ library SafeMath {
}
/**
* @dev Adds two numbers, reverts on overflow.
* @dev Subtracts two signed integers, reverts on overflow.
*/
function sub(int256 a, int256 b) internal pure returns (int256) {
int256 c = a - b;
require((b >= 0 && c <= a) || (b < 0 && c > a));
return c;
}
/**
* @dev Adds two unsigned integers, reverts on overflow.
*/
function add(uint256 a, uint256 b) internal pure returns (uint256) {
uint256 c = a + b;
@ -55,7 +98,17 @@ library SafeMath {
}
/**
* @dev Divides two numbers and returns the remainder (unsigned integer modulo),
* @dev Adds two signed integers, reverts on overflow.
*/
function add(int256 a, int256 b) internal pure returns (int256) {
int256 c = a + b;
require((b >= 0 && c >= a) || (b < 0 && c < a));
return c;
}
/**
* @dev Divides two unsigned integers and returns the remainder (unsigned integer modulo),
* reverts when dividing by zero.
*/
function mod(uint256 a, uint256 b) internal pure returns (uint256) {

View File

@ -1,25 +1,43 @@
pragma solidity ^0.4.24;
import "../math/SafeMath.sol";
contract SafeMathMock {
function mul(uint256 a, uint256 b) public pure returns (uint256) {
function mulUints(uint256 a, uint256 b) public pure returns (uint256) {
return SafeMath.mul(a, b);
}
function div(uint256 a, uint256 b) public pure returns (uint256) {
function mulInts(int256 a, int256 b) public pure returns (int256) {
return SafeMath.mul(a, b);
}
function divUints(uint256 a, uint256 b) public pure returns (uint256) {
return SafeMath.div(a, b);
}
function sub(uint256 a, uint256 b) public pure returns (uint256) {
function divInts(int256 a, int256 b) public pure returns (int256) {
return SafeMath.div(a, b);
}
function subUints(uint256 a, uint256 b) public pure returns (uint256) {
return SafeMath.sub(a, b);
}
function add(uint256 a, uint256 b) public pure returns (uint256) {
function subInts(int256 a, int256 b) public pure returns (int256) {
return SafeMath.sub(a, b);
}
function addUints(uint256 a, uint256 b) public pure returns (uint256) {
return SafeMath.add(a, b);
}
function mod(uint256 a, uint256 b) public pure returns (uint256) {
function addInts(int256 a, int256 b) public pure returns (int256) {
return SafeMath.add(a, b);
}
function modUints(uint256 a, uint256 b) public pure returns (uint256) {
return SafeMath.mod(a, b);
}
}

View File

@ -3,4 +3,6 @@ const BigNumber = web3.BigNumber;
module.exports = {
ZERO_ADDRESS: '0x0000000000000000000000000000000000000000',
MAX_UINT256: new BigNumber(2).pow(256).minus(1),
MAX_INT256: new BigNumber(2).pow(255).minus(1),
MIN_INT256: new BigNumber(2).pow(255).times(-1),
};

View File

@ -1,5 +1,5 @@
const shouldFail = require('../helpers/shouldFail');
const { MAX_UINT256 } = require('../helpers/constants');
const { MAX_UINT256, MIN_INT256, MAX_INT256 } = require('../helpers/constants');
const SafeMathMock = artifacts.require('SafeMathMock');
@ -10,134 +10,264 @@ contract('SafeMath', function () {
this.safeMath = await SafeMathMock.new();
});
describe('add', function () {
it('adds correctly', async function () {
const a = new BigNumber(5678);
const b = new BigNumber(1234);
describe('unsigned', function () {
describe('add', function () {
it('adds correctly', async function () {
const a = new BigNumber(5678);
const b = new BigNumber(1234);
(await this.safeMath.add(a, b)).should.be.bignumber.equal(a.plus(b));
});
it('throws a revert error on addition overflow', async function () {
const a = MAX_UINT256;
const b = new BigNumber(1);
await shouldFail.reverting(this.safeMath.add(a, b));
});
});
describe('sub', function () {
it('subtracts correctly', async function () {
const a = new BigNumber(5678);
const b = new BigNumber(1234);
(await this.safeMath.sub(a, b)).should.be.bignumber.equal(a.minus(b));
});
it('throws a revert error if subtraction result would be negative', async function () {
const a = new BigNumber(1234);
const b = new BigNumber(5678);
await shouldFail.reverting(this.safeMath.sub(a, b));
});
});
describe('mul', function () {
it('multiplies correctly', async function () {
const a = new BigNumber(1234);
const b = new BigNumber(5678);
(await this.safeMath.mul(a, b)).should.be.bignumber.equal(a.times(b));
});
it('handles a zero product correctly (first number as zero)', async function () {
const a = new BigNumber(0);
const b = new BigNumber(5678);
(await this.safeMath.mul(a, b)).should.be.bignumber.equal(a.times(b));
});
it('handles a zero product correctly (second number as zero)', async function () {
const a = new BigNumber(5678);
const b = new BigNumber(0);
(await this.safeMath.mul(a, b)).should.be.bignumber.equal(a.times(b));
});
it('throws a revert error on multiplication overflow', async function () {
const a = MAX_UINT256;
const b = new BigNumber(2);
await shouldFail.reverting(this.safeMath.mul(a, b));
});
});
describe('div', function () {
it('divides correctly', async function () {
const a = new BigNumber(5678);
const b = new BigNumber(5678);
(await this.safeMath.div(a, b)).should.be.bignumber.equal(a.div(b));
});
it('divides zero correctly', async function () {
const a = new BigNumber(0);
const b = new BigNumber(5678);
(await this.safeMath.div(a, b)).should.be.bignumber.equal(0);
});
it('returns complete number result on non-even division', async function () {
const a = new BigNumber(7000);
const b = new BigNumber(5678);
(await this.safeMath.div(a, b)).should.be.bignumber.equal(1);
});
it('throws a revert error on zero division', async function () {
const a = new BigNumber(5678);
const b = new BigNumber(0);
await shouldFail.reverting(this.safeMath.div(a, b));
});
});
describe('mod', function () {
describe('modulos correctly', async function () {
it('when the dividend is smaller than the divisor', async function () {
const a = new BigNumber(284);
const b = new BigNumber(5678);
(await this.safeMath.mod(a, b)).should.be.bignumber.equal(a.mod(b));
(await this.safeMath.addUints(a, b)).should.be.bignumber.equal(a.plus(b));
});
it('when the dividend is equal to the divisor', async function () {
it('reverts on addition overflow', async function () {
const a = MAX_UINT256;
const b = new BigNumber(1);
await shouldFail.reverting(this.safeMath.addUints(a, b));
});
});
describe('sub', function () {
it('subtracts correctly', async function () {
const a = new BigNumber(5678);
const b = new BigNumber(1234);
(await this.safeMath.subUints(a, b)).should.be.bignumber.equal(a.minus(b));
});
it('reverts if subtraction result would be negative', async function () {
const a = new BigNumber(1234);
const b = new BigNumber(5678);
await shouldFail.reverting(this.safeMath.subUints(a, b));
});
});
describe('mul', function () {
it('multiplies correctly', async function () {
const a = new BigNumber(1234);
const b = new BigNumber(5678);
(await this.safeMath.mulUints(a, b)).should.be.bignumber.equal(a.times(b));
});
it('handles a zero product correctly (first number as zero)', async function () {
const a = new BigNumber(0);
const b = new BigNumber(5678);
(await this.safeMath.mulUints(a, b)).should.be.bignumber.equal(a.times(b));
});
it('handles a zero product correctly (second number as zero)', async function () {
const a = new BigNumber(5678);
const b = new BigNumber(0);
(await this.safeMath.mulUints(a, b)).should.be.bignumber.equal(a.times(b));
});
it('reverts on multiplication overflow', async function () {
const a = MAX_UINT256;
const b = new BigNumber(2);
await shouldFail.reverting(this.safeMath.mulUints(a, b));
});
});
describe('div', function () {
it('divides correctly', async function () {
const a = new BigNumber(5678);
const b = new BigNumber(5678);
(await this.safeMath.mod(a, b)).should.be.bignumber.equal(a.mod(b));
(await this.safeMath.divUints(a, b)).should.be.bignumber.equal(a.div(b));
});
it('when the dividend is larger than the divisor', async function () {
it('divides zero correctly', async function () {
const a = new BigNumber(0);
const b = new BigNumber(5678);
(await this.safeMath.divUints(a, b)).should.be.bignumber.equal(0);
});
it('returns complete number result on non-even division', async function () {
const a = new BigNumber(7000);
const b = new BigNumber(5678);
(await this.safeMath.mod(a, b)).should.be.bignumber.equal(a.mod(b));
(await this.safeMath.divUints(a, b)).should.be.bignumber.equal(1);
});
it('when the dividend is a multiple of the divisor', async function () {
const a = new BigNumber(17034); // 17034 == 5678 * 3
const b = new BigNumber(5678);
it('reverts on zero division', async function () {
const a = new BigNumber(5678);
const b = new BigNumber(0);
(await this.safeMath.mod(a, b)).should.be.bignumber.equal(a.mod(b));
await shouldFail.reverting(this.safeMath.divUints(a, b));
});
});
it('reverts with a 0 divisor', async function () {
const a = new BigNumber(5678);
const b = new BigNumber(0);
describe('mod', function () {
describe('modulos correctly', async function () {
it('when the dividend is smaller than the divisor', async function () {
const a = new BigNumber(284);
const b = new BigNumber(5678);
await shouldFail.reverting(this.safeMath.mod(a, b));
(await this.safeMath.modUints(a, b)).should.be.bignumber.equal(a.mod(b));
});
it('when the dividend is equal to the divisor', async function () {
const a = new BigNumber(5678);
const b = new BigNumber(5678);
(await this.safeMath.modUints(a, b)).should.be.bignumber.equal(a.mod(b));
});
it('when the dividend is larger than the divisor', async function () {
const a = new BigNumber(7000);
const b = new BigNumber(5678);
(await this.safeMath.modUints(a, b)).should.be.bignumber.equal(a.mod(b));
});
it('when the dividend is a multiple of the divisor', async function () {
const a = new BigNumber(17034); // 17034 == 5678 * 3
const b = new BigNumber(5678);
(await this.safeMath.modUints(a, b)).should.be.bignumber.equal(a.mod(b));
});
});
it('reverts with a 0 divisor', async function () {
const a = new BigNumber(5678);
const b = new BigNumber(0);
await shouldFail.reverting(this.safeMath.modUints(a, b));
});
});
});
describe('signed', function () {
describe('add', function () {
it('adds correctly if it does not overflow and the result is positve', async function () {
const a = new BigNumber(1234);
const b = new BigNumber(5678);
(await this.safeMath.addUints(a, b)).should.be.bignumber.equal(a.plus(b));
});
it('adds correctly if it does not overflow and the result is negative', async function () {
const a = MAX_INT256;
const b = MIN_INT256;
const result = await this.safeMath.addInts(a, b);
result.should.be.bignumber.equal(a.plus(b));
});
it('reverts on positive addition overflow', async function () {
const a = MAX_INT256;
const b = new BigNumber(1);
await shouldFail.reverting(this.safeMath.addInts(a, b));
});
it('reverts on negative addition overflow', async function () {
const a = MIN_INT256;
const b = new BigNumber(-1);
await shouldFail.reverting(this.safeMath.addInts(a, b));
});
});
describe('sub', function () {
it('subtracts correctly if it does not overflow and the result is positive', async function () {
const a = new BigNumber(5678);
const b = new BigNumber(1234);
const result = await this.safeMath.subInts(a, b);
result.should.be.bignumber.equal(a.minus(b));
});
it('subtracts correctly if it does not overflow and the result is negative', async function () {
const a = new BigNumber(1234);
const b = new BigNumber(5678);
const result = await this.safeMath.subInts(a, b);
result.should.be.bignumber.equal(a.minus(b));
});
it('reverts on positive subtraction overflow', async function () {
const a = MAX_INT256;
const b = new BigNumber(-1);
await shouldFail.reverting(this.safeMath.subInts(a, b));
});
it('reverts on negative subtraction overflow', async function () {
const a = MIN_INT256;
const b = new BigNumber(1);
await shouldFail.reverting(this.safeMath.subInts(a, b));
});
});
describe('mul', function () {
it('multiplies correctly', async function () {
const a = new BigNumber(5678);
const b = new BigNumber(-1234);
const result = await this.safeMath.mulInts(a, b);
result.should.be.bignumber.equal(a.times(b));
});
it('handles a zero product correctly', async function () {
const a = new BigNumber(0);
const b = new BigNumber(5678);
const result = await this.safeMath.mulInts(a, b);
result.should.be.bignumber.equal(a.times(b));
});
it('reverts on multiplication overflow, positive operands', async function () {
const a = MAX_INT256;
const b = new BigNumber(2);
await shouldFail.reverting(this.safeMath.mulInts(a, b));
});
it('reverts when minimum integer is multiplied by -1', async function () {
const a = MIN_INT256;
const b = new BigNumber(-1);
await shouldFail.reverting(this.safeMath.mulInts(a, b));
});
it('reverts when -1 is multiplied by minimum integer', async function () {
const a = new BigNumber(-1);
const b = MIN_INT256;
await shouldFail.reverting(this.safeMath.mulInts(a, b));
});
});
describe('div', function () {
it('divides correctly', async function () {
const a = new BigNumber(-5678);
const b = new BigNumber(5678);
const result = await this.safeMath.divInts(a, b);
result.should.be.bignumber.equal(a.div(b));
});
it('reverts on zero division', async function () {
const a = new BigNumber(-5678);
const b = new BigNumber(0);
await shouldFail.reverting(this.safeMath.divInts(a, b));
});
it('reverts on overflow, negative second', async function () {
const a = new BigNumber(MIN_INT256);
const b = new BigNumber(-1);
await shouldFail.reverting(this.safeMath.divInts(a, b));
});
});
});
});