diff --git a/RELEASING.md b/RELEASING.md index 40360afcf93..7ef7f098f64 100644 --- a/RELEASING.md +++ b/RELEASING.md @@ -36,7 +36,7 @@ Draft the release notes in our [GitHub releases](https://github.com/OpenZeppelin Before publishing on npm you need to generate the build artifacts. This is not done automatically at the moment because of a bug in Truffle. Since some of the contracts should not be included in the package, this is a _hairy_ process that you need to do with care. -1. Delete the `contracts/mocks` and `contracts/examples` directories. +1. Delete the `contracts/mocks`, `contracts/examples` and `build` directories. 2. Run `truffle compile`. (Note that the Truffle process may never exit and you will have to interrupt it.) 3. Recover the directories using `git checkout`. It doesn't matter if you do this now or later. @@ -70,7 +70,7 @@ Draft the release notes in GitHub releases. Try to be consistent with our previo Before publishing on npm you need to generate the build artifacts. This is not done automatically at the moment because of a bug in Truffle. Since some of the contracts should not be included in the package, this is a _hairy_ process that you need to do with care. -1. Delete the `contracts/mocks` and `contracts/examples` directories. +1. Delete the `contracts/mocks`, `contracts/examples` and `build` directories. 2. Run `truffle compile`. (Note that the Truffle process may never exit and you will have to interrupt it.) 3. Recover the directories using `git checkout`. It doesn't matter if you do this now or later. diff --git a/contracts/access/roles/CapperRole.sol b/contracts/access/roles/CapperRole.sol index d6b2ba0662c..25c2d7c7349 100644 --- a/contracts/access/roles/CapperRole.sol +++ b/contracts/access/roles/CapperRole.sol @@ -8,7 +8,7 @@ contract CapperRole { event CapperAdded(address indexed account); event CapperRemoved(address indexed account); - Roles.Role private cappers; + Roles.Role private _cappers; constructor() internal { _addCapper(msg.sender); @@ -20,7 +20,7 @@ contract CapperRole { } function isCapper(address account) public view returns (bool) { - return cappers.has(account); + return _cappers.has(account); } function addCapper(address account) public onlyCapper { @@ -32,12 +32,12 @@ contract CapperRole { } function _addCapper(address account) internal { - cappers.add(account); + _cappers.add(account); emit CapperAdded(account); } function _removeCapper(address account) internal { - cappers.remove(account); + _cappers.remove(account); emit CapperRemoved(account); } } diff --git a/contracts/access/roles/MinterRole.sol b/contracts/access/roles/MinterRole.sol index 57f4a5a80ca..450f696526b 100644 --- a/contracts/access/roles/MinterRole.sol +++ b/contracts/access/roles/MinterRole.sol @@ -8,7 +8,7 @@ contract MinterRole { event MinterAdded(address indexed account); event MinterRemoved(address indexed account); - Roles.Role private minters; + Roles.Role private _minters; constructor() internal { _addMinter(msg.sender); @@ -20,7 +20,7 @@ contract MinterRole { } function isMinter(address account) public view returns (bool) { - return minters.has(account); + return _minters.has(account); } function addMinter(address account) public onlyMinter { @@ -32,12 +32,12 @@ contract MinterRole { } function _addMinter(address account) internal { - minters.add(account); + _minters.add(account); emit MinterAdded(account); } function _removeMinter(address account) internal { - minters.remove(account); + _minters.remove(account); emit MinterRemoved(account); } } diff --git a/contracts/access/roles/PauserRole.sol b/contracts/access/roles/PauserRole.sol index 58bb6a3138c..698bb8769c2 100644 --- a/contracts/access/roles/PauserRole.sol +++ b/contracts/access/roles/PauserRole.sol @@ -8,7 +8,7 @@ contract PauserRole { event PauserAdded(address indexed account); event PauserRemoved(address indexed account); - Roles.Role private pausers; + Roles.Role private _pausers; constructor() internal { _addPauser(msg.sender); @@ -20,7 +20,7 @@ contract PauserRole { } function isPauser(address account) public view returns (bool) { - return pausers.has(account); + return _pausers.has(account); } function addPauser(address account) public onlyPauser { @@ -32,12 +32,12 @@ contract PauserRole { } function _addPauser(address account) internal { - pausers.add(account); + _pausers.add(account); emit PauserAdded(account); } function _removePauser(address account) internal { - pausers.remove(account); + _pausers.remove(account); emit PauserRemoved(account); } } diff --git a/contracts/access/roles/SignerRole.sol b/contracts/access/roles/SignerRole.sol index 6a2bc602d14..8d76b831f8a 100644 --- a/contracts/access/roles/SignerRole.sol +++ b/contracts/access/roles/SignerRole.sol @@ -8,7 +8,7 @@ contract SignerRole { event SignerAdded(address indexed account); event SignerRemoved(address indexed account); - Roles.Role private signers; + Roles.Role private _signers; constructor() internal { _addSigner(msg.sender); @@ -20,7 +20,7 @@ contract SignerRole { } function isSigner(address account) public view returns (bool) { - return signers.has(account); + return _signers.has(account); } function addSigner(address account) public onlySigner { @@ -32,12 +32,12 @@ contract SignerRole { } function _addSigner(address account) internal { - signers.add(account); + _signers.add(account); emit SignerAdded(account); } function _removeSigner(address account) internal { - signers.remove(account); + _signers.remove(account); emit SignerRemoved(account); } } diff --git a/contracts/crowdsale/distribution/RefundableCrowdsale.sol b/contracts/crowdsale/distribution/RefundableCrowdsale.sol index 9b814cd7dc3..da02baee6f5 100644 --- a/contracts/crowdsale/distribution/RefundableCrowdsale.sol +++ b/contracts/crowdsale/distribution/RefundableCrowdsale.sol @@ -50,13 +50,13 @@ contract RefundableCrowdsale is FinalizableCrowdsale { /** * @dev Investors can claim refunds here if crowdsale is unsuccessful - * @param beneficiary Whose refund will be claimed. + * @param refundee Whose refund will be claimed. */ - function claimRefund(address beneficiary) public { + function claimRefund(address refundee) public { require(finalized()); require(!goalReached()); - _escrow.withdraw(beneficiary); + _escrow.withdraw(refundee); } /** diff --git a/contracts/drafts/TokenVesting.sol b/contracts/drafts/TokenVesting.sol index 2f5fd2fdc4a..3fa49f79faf 100644 --- a/contracts/drafts/TokenVesting.sol +++ b/contracts/drafts/TokenVesting.sol @@ -161,7 +161,7 @@ contract TokenVesting is Ownable { * @param token ERC20 token which is being vested */ function _vestedAmount(IERC20 token) private view returns (uint256) { - uint256 currentBalance = token.balanceOf(this); + uint256 currentBalance = token.balanceOf(address(this)); uint256 totalBalance = currentBalance.add(_released[token]); if (block.timestamp < _cliff) { diff --git a/contracts/examples/SimpleToken.sol b/contracts/examples/SimpleToken.sol index 8c63fdd984a..04b016a73e2 100644 --- a/contracts/examples/SimpleToken.sol +++ b/contracts/examples/SimpleToken.sol @@ -1,6 +1,7 @@ pragma solidity ^0.4.24; import "../token/ERC20/ERC20.sol"; +import "../token/ERC20/ERC20Detailed.sol"; /** * @title SimpleToken @@ -8,18 +9,14 @@ import "../token/ERC20/ERC20.sol"; * Note they can later distribute these tokens as they wish using `transfer` and other * `ERC20` functions. */ -contract SimpleToken is ERC20 { +contract SimpleToken is ERC20, ERC20Detailed { - string public constant name = "SimpleToken"; - string public constant symbol = "SIM"; - uint8 public constant decimals = 18; - - uint256 public constant INITIAL_SUPPLY = 10000 * (10 ** uint256(decimals)); + uint256 public constant INITIAL_SUPPLY = 10000 * (10 ** uint256(decimals())); /** * @dev Constructor that gives msg.sender all of existing tokens. */ - constructor() public { + constructor() public ERC20Detailed("SimpleToken", "SIM", 18) { _mint(msg.sender, INITIAL_SUPPLY); } diff --git a/contracts/token/ERC20/ERC20.sol b/contracts/token/ERC20/ERC20.sol index 25b2694a24f..5c550539f99 100644 --- a/contracts/token/ERC20/ERC20.sol +++ b/contracts/token/ERC20/ERC20.sol @@ -93,8 +93,6 @@ contract ERC20 is IERC20 { public returns (bool) { - require(value <= _allowed[from][msg.sender]); - _allowed[from][msg.sender] = _allowed[from][msg.sender].sub(value); _transfer(from, to, value); return true; @@ -155,7 +153,6 @@ contract ERC20 is IERC20 { * @param value The amount to be transferred. */ function _transfer(address from, address to, uint256 value) internal { - require(value <= _balances[from]); require(to != address(0)); _balances[from] = _balances[from].sub(value); @@ -171,7 +168,8 @@ contract ERC20 is IERC20 { * @param value The amount that will be created. */ function _mint(address account, uint256 value) internal { - require(account != 0); + require(account != address(0)); + _totalSupply = _totalSupply.add(value); _balances[account] = _balances[account].add(value); emit Transfer(address(0), account, value); @@ -184,8 +182,7 @@ contract ERC20 is IERC20 { * @param value The amount that will be burnt. */ function _burn(address account, uint256 value) internal { - require(account != 0); - require(value <= _balances[account]); + require(account != address(0)); _totalSupply = _totalSupply.sub(value); _balances[account] = _balances[account].sub(value); @@ -200,8 +197,6 @@ contract ERC20 is IERC20 { * @param value The amount that will be burnt. */ function _burnFrom(address account, uint256 value) internal { - require(value <= _allowed[account][msg.sender]); - // Should https://github.com/OpenZeppelin/zeppelin-solidity/issues/707 be accepted, // this function needs to emit an event with the updated approval. _allowed[account][msg.sender] = _allowed[account][msg.sender].sub( diff --git a/contracts/token/ERC721/ERC721Burnable.sol b/contracts/token/ERC721/ERC721Burnable.sol index 3fc0277e5cc..71f3bcdd18f 100644 --- a/contracts/token/ERC721/ERC721Burnable.sol +++ b/contracts/token/ERC721/ERC721Burnable.sol @@ -2,7 +2,16 @@ pragma solidity ^0.4.24; import "./ERC721.sol"; +/** + * @title ERC721 Burnable Token + * @dev ERC721 Token that can be irreversibly burned (destroyed). + */ contract ERC721Burnable is ERC721 { + + /** + * @dev Burns a specific ERC721 token. + * @param tokenId uint256 id of the ERC721 token to be burned. + */ function burn(uint256 tokenId) public { diff --git a/contracts/token/ERC721/ERC721Enumerable.sol b/contracts/token/ERC721/ERC721Enumerable.sol index c8551472070..234307755e6 100644 --- a/contracts/token/ERC721/ERC721Enumerable.sol +++ b/contracts/token/ERC721/ERC721Enumerable.sol @@ -4,6 +4,10 @@ import "./IERC721Enumerable.sol"; import "./ERC721.sol"; import "../../introspection/ERC165.sol"; +/** + * @title ERC-721 Non-Fungible Token with optional enumeration extension logic + * @dev See https://github.com/ethereum/EIPs/blob/master/EIPS/eip-721.md + */ contract ERC721Enumerable is ERC165, ERC721, IERC721Enumerable { // Mapping from owner to list of owned token IDs mapping(address => uint256[]) private _ownedTokens; diff --git a/ethpm.json b/ethpm.json index bb2674762cc..a60b4a60336 100644 --- a/ethpm.json +++ b/ethpm.json @@ -1,6 +1,6 @@ { "package_name": "zeppelin", - "version": "2.0.0-rc.1", + "version": "2.0.0", "description": "Secure Smart Contract library for Solidity", "authors": [ "OpenZeppelin Community " diff --git a/package-lock.json b/package-lock.json index 29034359c34..9e2fe893b73 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "openzeppelin-solidity", - "version": "2.0.0-rc.1", + "version": "2.0.0", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index 235e500eedb..f97f1050aca 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "openzeppelin-solidity", - "version": "2.0.0-rc.1", + "version": "2.0.0", "description": "Secure Smart Contract library for Solidity", "files": [ "build", diff --git a/test/math/SafeMath.test.js b/test/math/SafeMath.test.js index dcbb4fbb689..47e8559efc7 100644 --- a/test/math/SafeMath.test.js +++ b/test/math/SafeMath.test.js @@ -53,13 +53,20 @@ contract('SafeMath', function () { (await this.safeMath.mul(a, b)).should.be.bignumber.equal(a.times(b)); }); - it('handles a zero product correctly', async function () { + 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); @@ -76,6 +83,20 @@ contract('SafeMath', function () { (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);