Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
…lidity into feature/token-recover

* 'master' of https://github.com/vittominacori/zeppelin-solidity: (98 commits)
  Renamed roles private variables to adhere to code style. (OpenZeppelin#1507)
  Remove extraneous quantity check, fixes OpenZeppelin#1454 (OpenZeppelin#1455)
  Remove redundant require statements (OpenZeppelin#1409)
  Add the step to delete the build dir to the RELEASE notes (OpenZeppelin#1467)
  add an address typecast to this per issue OpenZeppelin#1457 (OpenZeppelin#1471)
  add improvement in simpletoken example OpenZeppelin#1458 (OpenZeppelin#1473)
  SafeMath Test Coverage Improved (OpenZeppelin#1477)
  The beneficiary parameter of claimRefund is replaced with refundee (OpenZeppelin#1481)
  fix ERC20.sol#L174 and ERC20.sol#L187 should be casted to an address type. (OpenZeppelin#1470)
  Fix/add comment erc721 burnable OpenZeppelin#1464 (OpenZeppelin#1469)
  Release v2.0.0
  Release candidate v2.0.0-rc.4
  Improved some ERC721 internal shenanigans (OpenZeppelin#1450)
  Add warning about trading tokens before refundable crowdsale goal is met (OpenZeppelin#1452)
  Crowdsale.buyTokens is now nonReentrant. (OpenZeppelin#1438)
  InitialRate must be strictly larger than finalRate. (OpenZeppelin#1441)
  Fixed how allowance crowdsale checks remaining tokens. (OpenZeppelin#1449)
  Deleted unnecessary import. (OpenZeppelin#1437)
  Made SampleCrowdsale a bit clearer. (OpenZeppelin#1448)
  Now setting the finalized flag before doing finalization to prevent possbile reentrancy issues. (OpenZeppelin#1447)
  ...
  • Loading branch information
vittominacori committed Nov 21, 2018
2 parents a6ea985 + a833c4b commit fd957c4
Show file tree
Hide file tree
Showing 15 changed files with 67 additions and 41 deletions.
4 changes: 2 additions & 2 deletions RELEASING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down Expand Up @@ -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.

Expand Down
8 changes: 4 additions & 4 deletions contracts/access/roles/CapperRole.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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 {
Expand All @@ -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);
}
}
8 changes: 4 additions & 4 deletions contracts/access/roles/MinterRole.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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 {
Expand All @@ -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);
}
}
8 changes: 4 additions & 4 deletions contracts/access/roles/PauserRole.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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 {
Expand All @@ -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);
}
}
8 changes: 4 additions & 4 deletions contracts/access/roles/SignerRole.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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 {
Expand All @@ -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);
}
}
6 changes: 3 additions & 3 deletions contracts/crowdsale/distribution/RefundableCrowdsale.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion contracts/drafts/TokenVesting.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
11 changes: 4 additions & 7 deletions contracts/examples/SimpleToken.sol
Original file line number Diff line number Diff line change
@@ -1,25 +1,22 @@
pragma solidity ^0.4.24;

import "../token/ERC20/ERC20.sol";
import "../token/ERC20/ERC20Detailed.sol";

/**
* @title SimpleToken
* @dev Very simple ERC20 Token example, where all tokens are pre-assigned to the creator.
* 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);
}

Expand Down
11 changes: 3 additions & 8 deletions contracts/token/ERC20/ERC20.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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(
Expand Down
9 changes: 9 additions & 0 deletions contracts/token/ERC721/ERC721Burnable.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
4 changes: 4 additions & 0 deletions contracts/token/ERC721/ERC721Enumerable.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion ethpm.json
Original file line number Diff line number Diff line change
@@ -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 <maintainers@openzeppelin.org>"
Expand Down
2 changes: 1 addition & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -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",
Expand Down
23 changes: 22 additions & 1 deletion test/math/SafeMath.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down

0 comments on commit fd957c4

Please sign in to comment.