diff --git a/.eslintrc b/.eslintrc index 117135b36f9..c15a4d51515 100644 --- a/.eslintrc +++ b/.eslintrc @@ -25,7 +25,7 @@ // Code style "camelcase": ["error", {"properties": "always"}], - "comma-dangle": ["warn", "always-multiline"], + "comma-dangle": ["error", "always-multiline"], "comma-spacing": ["error", {"before": false, "after": true}], "dot-notation": ["error", {"allowKeywords": true, "allowPattern": ""}], "eol-last": ["error", "always"], diff --git a/.github/ISSUE_TEMPLATE.md b/.github/ISSUE_TEMPLATE.md deleted file mode 100644 index cafae1e9e0f..00000000000 --- a/.github/ISSUE_TEMPLATE.md +++ /dev/null @@ -1,34 +0,0 @@ -## 🎉 Description - - - -- [ ] 🐛 This is a bug report. -- [ ] 📈 This is a feature request. - - - -## 💻 Environment - -Next, we need to know what your environment looks like. - -- Which version of OpenZeppelin are you using? -- What network are you deploying to? Ganache? Ropsten? -- How are you deploying your OpenZeppelin-backed contracts? truffle? Remix? Let us know! - -## 📝 Details - -Describe the problem you have been experiencing in more detail. Include as much information as you think is relevant. Keep in mind that transactions can fail for many reasons; context is key here. - -## 🔢 Code To Reproduce Issue [ Good To Have ] - -Please remember that with sample code it's easier to reproduce the bug and it's much faster to fix it. - -``` -insert short code snippets here -``` - - - -## 👍 Other Information - - diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md new file mode 100644 index 00000000000..d932e7632ab --- /dev/null +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -0,0 +1,21 @@ +--- +name: Bug report +about: Report a bug in OpenZeppelin + +--- + + + + + +**💻 Environment** + + + +**📝 Details** + + + +**🔢 Code to reproduce bug** + + diff --git a/.github/ISSUE_TEMPLATE/feature_request.md b/.github/ISSUE_TEMPLATE/feature_request.md new file mode 100644 index 00000000000..404854c009f --- /dev/null +++ b/.github/ISSUE_TEMPLATE/feature_request.md @@ -0,0 +1,14 @@ +--- +name: Feature request +about: Suggest an idea for OpenZeppelin + +--- + +**🧐 Motivation** + + +**📝 Details** + + + + diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 72b331668a3..2d743d0fe1f 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -1,17 +1,20 @@ - + -Fixes # + -# 🚀 Description +Fixes # - + - - -- [ ] 📘 I've reviewed the [OpenZeppelin Contributor Guidelines](../blob/master/CONTRIBUTING.md) -- [ ] ✅ I've added tests where applicable to test my new functionality. -- [ ] 📖 I've made sure that my contracts are well-documented. -- [ ] 🎨 I've run the JS/Solidity linters and fixed any issues (`npm run lint:fix`). + diff --git a/RELEASING.md b/RELEASING.md index 6514b4cf54c..40360afcf93 100644 --- a/RELEASING.md +++ b/RELEASING.md @@ -34,6 +34,12 @@ git push upstream vX.Y.Z-rc.R Draft the release notes in our [GitHub releases](https://github.com/OpenZeppelin/openzeppelin-solidity/releases). Make sure to mark it as a pre-release! Try to be consistent with our previous release notes in the title and format of the text. Release candidates don't need a detailed changelog, but make sure to include a link to GitHub's compare page. +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. +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. + Once the CI run for the new tag is green, publish on npm under the `next` tag. ``` @@ -62,6 +68,12 @@ git push upstream vX.Y.Z Draft the release notes in GitHub releases. Try to be consistent with our previous release notes in the title and format of the text. Make sure to include a detailed changelog. +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. +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. + Once the CI run for the new tag is green, publish on npm. ``` diff --git a/contracts/access/roles/CapperRole.sol b/contracts/access/roles/CapperRole.sol index 756cdd48fc1..a79e8e694c2 100644 --- a/contracts/access/roles/CapperRole.sol +++ b/contracts/access/roles/CapperRole.sol @@ -11,7 +11,7 @@ contract CapperRole { Roles.Role private cappers; constructor() public { - cappers.add(msg.sender); + _addCapper(msg.sender); } modifier onlyCapper() { @@ -24,12 +24,16 @@ contract CapperRole { } function addCapper(address account) public onlyCapper { - cappers.add(account); - emit CapperAdded(account); + _addCapper(account); } function renounceCapper() public { - cappers.remove(msg.sender); + _removeCapper(msg.sender); + } + + function _addCapper(address account) internal { + cappers.add(account); + emit CapperAdded(account); } function _removeCapper(address account) internal { diff --git a/contracts/access/roles/MinterRole.sol b/contracts/access/roles/MinterRole.sol index cc95473c7ed..d908e3004da 100644 --- a/contracts/access/roles/MinterRole.sol +++ b/contracts/access/roles/MinterRole.sol @@ -11,7 +11,7 @@ contract MinterRole { Roles.Role private minters; constructor() public { - minters.add(msg.sender); + _addMinter(msg.sender); } modifier onlyMinter() { @@ -24,12 +24,16 @@ contract MinterRole { } function addMinter(address account) public onlyMinter { - minters.add(account); - emit MinterAdded(account); + _addMinter(account); } function renounceMinter() public { - minters.remove(msg.sender); + _removeMinter(msg.sender); + } + + function _addMinter(address account) internal { + minters.add(account); + emit MinterAdded(account); } function _removeMinter(address account) internal { diff --git a/contracts/access/roles/PauserRole.sol b/contracts/access/roles/PauserRole.sol index 06347e7cfe9..d59839d6bd0 100644 --- a/contracts/access/roles/PauserRole.sol +++ b/contracts/access/roles/PauserRole.sol @@ -11,7 +11,7 @@ contract PauserRole { Roles.Role private pausers; constructor() public { - pausers.add(msg.sender); + _addPauser(msg.sender); } modifier onlyPauser() { @@ -24,12 +24,16 @@ contract PauserRole { } function addPauser(address account) public onlyPauser { - pausers.add(account); - emit PauserAdded(account); + _addPauser(account); } function renouncePauser() public { - pausers.remove(msg.sender); + _removePauser(msg.sender); + } + + function _addPauser(address account) internal { + pausers.add(account); + emit PauserAdded(account); } function _removePauser(address account) internal { diff --git a/contracts/access/roles/SignerRole.sol b/contracts/access/roles/SignerRole.sol index e2729a25573..bbf8fd36fd6 100644 --- a/contracts/access/roles/SignerRole.sol +++ b/contracts/access/roles/SignerRole.sol @@ -11,7 +11,7 @@ contract SignerRole { Roles.Role private signers; constructor() public { - signers.add(msg.sender); + _addSigner(msg.sender); } modifier onlySigner() { @@ -24,12 +24,16 @@ contract SignerRole { } function addSigner(address account) public onlySigner { - signers.add(account); - emit SignerAdded(account); + _addSigner(account); } function renounceSigner() public { - signers.remove(msg.sender); + _removeSigner(msg.sender); + } + + function _addSigner(address account) internal { + signers.add(account); + emit SignerAdded(account); } function _removeSigner(address account) internal { diff --git a/contracts/bounties/BreakInvariantBounty.sol b/contracts/bounties/BreakInvariantBounty.sol index 016d3f2f55d..6dec45a3ba8 100644 --- a/contracts/bounties/BreakInvariantBounty.sol +++ b/contracts/bounties/BreakInvariantBounty.sol @@ -8,24 +8,29 @@ import "../ownership/Ownable.sol"; * @dev This bounty will pay out to a researcher if they break invariant logic of the contract. */ contract BreakInvariantBounty is PullPayment, Ownable { - bool private _claimed; + bool private _claimable; mapping(address => address) private _researchers; event TargetCreated(address createdAddress); + event BountyCanceled(); + + constructor() public { + _claimable = true; + } /** * @dev Fallback function allowing the contract to receive funds, if they haven't already been claimed. */ function() external payable { - require(!_claimed); + require(_claimable); } /** - * @dev Determine if the bounty was claimed. - * @return true if the bounty was claimed, false otherwise. + * @dev Determine if the bounty is claimable. + * @return false if the bounty was claimed, true otherwise. */ - function claimed() public view returns(bool) { - return _claimed; + function claimable() public view returns(bool) { + return _claimable; } /** @@ -45,19 +50,23 @@ contract BreakInvariantBounty is PullPayment, Ownable { * @param target contract */ function claim(Target target) public { + require(_claimable); address researcher = _researchers[target]; require(researcher != address(0)); // Check Target contract invariants require(!target.checkInvariant()); _asyncTransfer(researcher, address(this).balance); - _claimed = true; + _claimable = false; } /** - * @dev Transfers the current balance to the owner and terminates the contract. + * @dev Cancels the bounty and transfers all funds to the owner */ - function destroy() public onlyOwner { - selfdestruct(owner()); + function cancelBounty() public onlyOwner{ + require(_claimable); + _asyncTransfer(owner(), address(this).balance); + _claimable = false; + emit BountyCanceled(); } /** diff --git a/contracts/mocks/ArraysImpl.sol b/contracts/mocks/ArraysImpl.sol new file mode 100644 index 00000000000..8a2b9ec51ae --- /dev/null +++ b/contracts/mocks/ArraysImpl.sol @@ -0,0 +1,18 @@ +pragma solidity ^0.4.24; + +import "../utils/Arrays.sol"; + +contract ArraysImpl { + + using Arrays for uint256[]; + + uint256[] private array; + + constructor(uint256[] _array) public { + array = _array; + } + + function findUpperBound(uint256 _element) external view returns (uint256) { + return array.findUpperBound(_element); + } +} diff --git a/contracts/mocks/SecureInvariantTargetBounty.sol b/contracts/mocks/BreakInvariantBountyMock.sol similarity index 51% rename from contracts/mocks/SecureInvariantTargetBounty.sol rename to contracts/mocks/BreakInvariantBountyMock.sol index 9f2525bf0ef..9343ef01269 100644 --- a/contracts/mocks/SecureInvariantTargetBounty.sol +++ b/contracts/mocks/BreakInvariantBountyMock.sol @@ -5,14 +5,24 @@ pragma solidity ^0.4.24; // solium-disable-next-line max-len import {BreakInvariantBounty, Target} from "../bounties/BreakInvariantBounty.sol"; -contract SecureInvariantTargetMock is Target { - function checkInvariant() public returns(bool) { +contract TargetMock is Target { + bool private exploited; + + function exploitVulnerability() public { + exploited = true; + } + + function checkInvariant() public returns (bool) { + if (exploited) { + return false; + } + return true; } } -contract SecureInvariantTargetBounty is BreakInvariantBounty { +contract BreakInvariantBountyMock is BreakInvariantBounty { function _deployContract() internal returns (address) { - return new SecureInvariantTargetMock(); + return new TargetMock(); } } diff --git a/contracts/mocks/ERC20SnapshotMock.sol b/contracts/mocks/ERC20SnapshotMock.sol new file mode 100644 index 00000000000..51c67a4502e --- /dev/null +++ b/contracts/mocks/ERC20SnapshotMock.sol @@ -0,0 +1,19 @@ +pragma solidity ^0.4.24; + +import "../token/ERC20/ERC20Snapshot.sol"; + + +contract ERC20SnapshotMock is ERC20Snapshot { + + constructor(address initialAccount, uint256 initialBalance) public { + _mint(initialAccount, initialBalance); + } + + function mint(address account, uint256 amount) public { + _mint(account, amount); + } + + function burn(address account, uint256 amount) public { + _burn(account, amount); + } +} diff --git a/contracts/mocks/ERC721FullMock.sol b/contracts/mocks/ERC721FullMock.sol index d555b1f832d..2d85e737220 100644 --- a/contracts/mocks/ERC721FullMock.sol +++ b/contracts/mocks/ERC721FullMock.sol @@ -2,14 +2,17 @@ pragma solidity ^0.4.24; import "../token/ERC721/ERC721Full.sol"; import "../token/ERC721/ERC721Mintable.sol"; +import "../token/ERC721/ERC721MetadataMintable.sol"; import "../token/ERC721/ERC721Burnable.sol"; /** - * @title ERC721Mock + * @title ERC721FullMock * This mock just provides a public mint and burn functions for testing purposes, * and a public setter for metadata URI */ -contract ERC721FullMock is ERC721Full, ERC721Mintable, ERC721Burnable { +contract ERC721FullMock + is ERC721Full, ERC721Mintable, ERC721MetadataMintable, ERC721Burnable { + constructor(string name, string symbol) public ERC721Mintable() ERC721Full(name, symbol) diff --git a/contracts/mocks/ERC721MintableBurnableImpl.sol b/contracts/mocks/ERC721MintableBurnableImpl.sol index 03ff3ac378a..51d3ff4f6bc 100644 --- a/contracts/mocks/ERC721MintableBurnableImpl.sol +++ b/contracts/mocks/ERC721MintableBurnableImpl.sol @@ -2,13 +2,14 @@ pragma solidity ^0.4.24; import "../token/ERC721/ERC721Full.sol"; import "../token/ERC721/ERC721Mintable.sol"; +import "../token/ERC721/ERC721MetadataMintable.sol"; import "../token/ERC721/ERC721Burnable.sol"; /** * @title ERC721MintableBurnableImpl */ contract ERC721MintableBurnableImpl - is ERC721Full, ERC721Mintable, ERC721Burnable { + is ERC721Full, ERC721Mintable, ERC721MetadataMintable, ERC721Burnable { constructor() ERC721Mintable() diff --git a/contracts/mocks/ForceEther.sol b/contracts/mocks/ForceEther.sol deleted file mode 100644 index bca5646f354..00000000000 --- a/contracts/mocks/ForceEther.sol +++ /dev/null @@ -1,15 +0,0 @@ -pragma solidity ^0.4.24; - -// @title Force Ether into a contract. -// @notice even -// if the contract is not payable. -// @notice To use, construct the contract with the target as argument. -// @author Remco Bloemen -contract ForceEther { - - constructor() public payable { } - - function destroyAndSend(address recipient) public { - selfdestruct(recipient); - } -} diff --git a/contracts/mocks/InsecureInvariantTargetBounty.sol b/contracts/mocks/InsecureInvariantTargetBounty.sol deleted file mode 100644 index cac749d6359..00000000000 --- a/contracts/mocks/InsecureInvariantTargetBounty.sol +++ /dev/null @@ -1,18 +0,0 @@ -pragma solidity ^0.4.24; - -// When this line is split, truffle parsing fails. -// See: https://github.com/ethereum/solidity/issues/4871 -// solium-disable-next-line max-len -import {BreakInvariantBounty, Target} from "../bounties/BreakInvariantBounty.sol"; - -contract InsecureInvariantTargetMock is Target { - function checkInvariant() public returns(bool) { - return false; - } -} - -contract InsecureInvariantTargetBounty is BreakInvariantBounty { - function _deployContract() internal returns (address) { - return new InsecureInvariantTargetMock(); - } -} diff --git a/contracts/mocks/MessageHelper.sol b/contracts/mocks/MessageHelper.sol deleted file mode 100644 index 54c08441484..00000000000 --- a/contracts/mocks/MessageHelper.sol +++ /dev/null @@ -1,49 +0,0 @@ -pragma solidity ^0.4.24; - -contract MessageHelper { - - event Show(bytes32 b32, uint256 number, string text); - event Buy(bytes32 b32, uint256 number, string text, uint256 value); - - function showMessage( - bytes32 _message, - uint256 _number, - string _text - ) - public - returns (bool) - { - emit Show(_message, _number, _text); - return true; - } - - function buyMessage( - bytes32 _message, - uint256 _number, - string _text - ) - public - payable - returns (bool) - { - emit Buy( - _message, - _number, - _text, - msg.value); - return true; - } - - function fail() public { - require(false); - } - - function call(address _to, bytes _data) public returns (bool) { - // solium-disable-next-line security/no-low-level-calls - if (_to.call(_data)) - return true; - else - return false; - } - -} diff --git a/contracts/mocks/SignatureBouncerMock.sol b/contracts/mocks/SignatureBouncerMock.sol index ad949403385..85076eae6ea 100644 --- a/contracts/mocks/SignatureBouncerMock.sol +++ b/contracts/mocks/SignatureBouncerMock.sol @@ -63,4 +63,11 @@ contract SignatureBouncerMock is SignatureBouncer, SignerRoleMock { { } + + function tooShortMsgData() + public + onlyValidSignatureAndData("") + view + { + } } diff --git a/contracts/payment/Escrow.sol b/contracts/payment/Escrow.sol index 21f7d6edd4c..7fb118e3a2c 100644 --- a/contracts/payment/Escrow.sol +++ b/contracts/payment/Escrow.sol @@ -39,7 +39,6 @@ contract Escrow is Secondary { */ function withdraw(address payee) public onlyPrimary { uint256 payment = _deposits[payee]; - assert(address(this).balance >= payment); _deposits[payee] = 0; diff --git a/contracts/payment/RefundEscrow.sol b/contracts/payment/RefundEscrow.sol index 3eca6bbcfbb..c3770dbbb88 100644 --- a/contracts/payment/RefundEscrow.sol +++ b/contracts/payment/RefundEscrow.sol @@ -1,7 +1,6 @@ pragma solidity ^0.4.24; import "./ConditionalEscrow.sol"; -import "../ownership/Secondary.sol"; /** * @title RefundEscrow @@ -9,7 +8,7 @@ import "../ownership/Secondary.sol"; * The primary account may close the deposit period, and allow for either withdrawal * by the beneficiary, or refunds to the depositors. */ -contract RefundEscrow is Secondary, ConditionalEscrow { +contract RefundEscrow is ConditionalEscrow { enum State { Active, Refunding, Closed } event Closed(); diff --git a/contracts/payment/SplitPayment.sol b/contracts/payment/SplitPayment.sol index e0e057ae84e..8bfcc7690c3 100644 --- a/contracts/payment/SplitPayment.sol +++ b/contracts/payment/SplitPayment.sol @@ -84,7 +84,6 @@ contract SplitPayment { ); require(payment != 0); - assert(address(this).balance >= payment); _released[account] = _released[account].add(payment); _totalReleased = _totalReleased.add(payment); diff --git a/contracts/token/ERC20/ERC20.sol b/contracts/token/ERC20/ERC20.sol index 8fd420a2909..25b2694a24f 100644 --- a/contracts/token/ERC20/ERC20.sol +++ b/contracts/token/ERC20/ERC20.sol @@ -58,12 +58,7 @@ contract ERC20 is IERC20 { * @param value The amount to be transferred. */ function transfer(address to, uint256 value) public returns (bool) { - require(value <= _balances[msg.sender]); - require(to != address(0)); - - _balances[msg.sender] = _balances[msg.sender].sub(value); - _balances[to] = _balances[to].add(value); - emit Transfer(msg.sender, to, value); + _transfer(msg.sender, to, value); return true; } @@ -98,14 +93,10 @@ contract ERC20 is IERC20 { public returns (bool) { - require(value <= _balances[from]); require(value <= _allowed[from][msg.sender]); - require(to != address(0)); - _balances[from] = _balances[from].sub(value); - _balances[to] = _balances[to].add(value); _allowed[from][msg.sender] = _allowed[from][msg.sender].sub(value); - emit Transfer(from, to, value); + _transfer(from, to, value); return true; } @@ -157,33 +148,48 @@ contract ERC20 is IERC20 { return true; } + /** + * @dev Transfer token for a specified addresses + * @param from The address to transfer from. + * @param to The address to transfer to. + * @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); + _balances[to] = _balances[to].add(value); + emit Transfer(from, to, value); + } + /** * @dev Internal function that mints an amount of the token and assigns it to * an account. This encapsulates the modification of balances such that the * proper events are emitted. * @param account The account that will receive the created tokens. - * @param amount The amount that will be created. + * @param value The amount that will be created. */ - function _mint(address account, uint256 amount) internal { + function _mint(address account, uint256 value) internal { require(account != 0); - _totalSupply = _totalSupply.add(amount); - _balances[account] = _balances[account].add(amount); - emit Transfer(address(0), account, amount); + _totalSupply = _totalSupply.add(value); + _balances[account] = _balances[account].add(value); + emit Transfer(address(0), account, value); } /** * @dev Internal function that burns an amount of the token of a given * account. * @param account The account whose tokens will be burnt. - * @param amount The amount that will be burnt. + * @param value The amount that will be burnt. */ - function _burn(address account, uint256 amount) internal { + function _burn(address account, uint256 value) internal { require(account != 0); - require(amount <= _balances[account]); + require(value <= _balances[account]); - _totalSupply = _totalSupply.sub(amount); - _balances[account] = _balances[account].sub(amount); - emit Transfer(account, address(0), amount); + _totalSupply = _totalSupply.sub(value); + _balances[account] = _balances[account].sub(value); + emit Transfer(account, address(0), value); } /** @@ -191,15 +197,15 @@ contract ERC20 is IERC20 { * account, deducting from the sender's allowance for said account. Uses the * internal burn function. * @param account The account whose tokens will be burnt. - * @param amount The amount that will be burnt. + * @param value The amount that will be burnt. */ - function _burnFrom(address account, uint256 amount) internal { - require(amount <= _allowed[account][msg.sender]); + 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( - amount); - _burn(account, amount); + value); + _burn(account, value); } } diff --git a/contracts/token/ERC20/ERC20Burnable.sol b/contracts/token/ERC20/ERC20Burnable.sol index 014dc7fa8d7..ba4a2ccbed6 100644 --- a/contracts/token/ERC20/ERC20Burnable.sol +++ b/contracts/token/ERC20/ERC20Burnable.sol @@ -24,12 +24,4 @@ contract ERC20Burnable is ERC20 { function burnFrom(address from, uint256 value) public { _burnFrom(from, value); } - - /** - * @dev Overrides ERC20._burn in order for burn and burnFrom to emit - * an additional Burn event. - */ - function _burn(address who, uint256 value) internal { - super._burn(who, value); - } } diff --git a/contracts/token/ERC20/ERC20Capped.sol b/contracts/token/ERC20/ERC20Capped.sol index 0a73159b6ac..9f3ea8bfc04 100644 --- a/contracts/token/ERC20/ERC20Capped.sol +++ b/contracts/token/ERC20/ERC20Capped.sol @@ -27,19 +27,19 @@ contract ERC20Capped is ERC20Mintable { /** * @dev Function to mint tokens * @param to The address that will receive the minted tokens. - * @param amount The amount of tokens to mint. + * @param value The amount of tokens to mint. * @return A boolean that indicates if the operation was successful. */ function mint( address to, - uint256 amount + uint256 value ) public returns (bool) { - require(totalSupply().add(amount) <= _cap); + require(totalSupply().add(value) <= _cap); - return super.mint(to, amount); + return super.mint(to, value); } } diff --git a/contracts/token/ERC20/ERC20Mintable.sol b/contracts/token/ERC20/ERC20Mintable.sol index d6bd9b5eac3..eb01f06e74e 100644 --- a/contracts/token/ERC20/ERC20Mintable.sol +++ b/contracts/token/ERC20/ERC20Mintable.sol @@ -8,53 +8,21 @@ import "../../access/roles/MinterRole.sol"; * @dev ERC20 minting logic */ contract ERC20Mintable is ERC20, MinterRole { - event MintingFinished(); - - bool private _mintingFinished = false; - - modifier onlyBeforeMintingFinished() { - require(!_mintingFinished); - _; - } - - /** - * @return true if the minting is finished. - */ - function mintingFinished() public view returns(bool) { - return _mintingFinished; - } - /** * @dev Function to mint tokens * @param to The address that will receive the minted tokens. - * @param amount The amount of tokens to mint. + * @param value The amount of tokens to mint. * @return A boolean that indicates if the operation was successful. */ function mint( address to, - uint256 amount + uint256 value ) public onlyMinter - onlyBeforeMintingFinished - returns (bool) - { - _mint(to, amount); - return true; - } - - /** - * @dev Function to stop minting new tokens. - * @return True if the operation was successful. - */ - function finishMinting() - public - onlyMinter - onlyBeforeMintingFinished returns (bool) { - _mintingFinished = true; - emit MintingFinished(); + _mint(to, value); return true; } } diff --git a/contracts/token/ERC20/ERC20Snapshot.sol b/contracts/token/ERC20/ERC20Snapshot.sol new file mode 100644 index 00000000000..0e7672cdd8a --- /dev/null +++ b/contracts/token/ERC20/ERC20Snapshot.sol @@ -0,0 +1,131 @@ +pragma solidity ^0.4.24; + +import "./ERC20.sol"; +import "../../utils/Arrays.sol"; +import "../../math/SafeMath.sol"; + + +/** + * @title ERC20Snapshot token + * @dev An ERC20 token which enables taking snapshots of account balances. + * This can be useful to safely implement voting weighed by balance. + */ +contract ERC20Snapshot is ERC20 { + using SafeMath for uint256; + using Arrays for uint256[]; + + // The 0 id represents no snapshot was taken yet. + uint256 private currentSnapshotId; + + mapping (address => uint256[]) private snapshotIds; + mapping (address => uint256[]) private snapshotBalances; + + event Snapshot(uint256 id); + + /** + * @dev Increments current snapshot. Emites Snapshot event. + * @return An uint256 representing current snapshot id. + */ + function snapshot() external returns (uint256) { + currentSnapshotId = currentSnapshotId.add(1); + emit Snapshot(currentSnapshotId); + return currentSnapshotId; + } + + /** + * @dev Returns account balance for specific snapshot. + * @param account address The address to query the balance of. + * @param snapshotId uint256 The snapshot id for which to query the balance of. + * @return An uint256 representing the amount owned by the passed address for specific snapshot. + */ + function balanceOfAt( + address account, + uint256 snapshotId + ) + public + view + returns (uint256) + { + require(snapshotId > 0 && snapshotId <= currentSnapshotId); + + uint256 idx = snapshotIds[account].findUpperBound(snapshotId); + + if (idx == snapshotIds[account].length) { + return balanceOf(account); + } else { + return snapshotBalances[account][idx]; + } + } + + /** + * @dev Transfer token for a specified address. It takes balance snapshot for the sender and recipient account + * before transfer is done. + * @param to The address to transfer to. + * @param value The amount to be transferred. + */ + function transfer(address to, uint256 value) public returns (bool) { + updateSnapshot(msg.sender); + updateSnapshot(to); + return super.transfer(to, value); + } + + /** + * @dev Transfer tokens from one address to another. It takes balance snapshot of both accounts before + * the transfer is done. + * @param from address The address which you want to send tokens from + * @param to address The address which you want to transfer to + * @param value uint256 the amount of tokens to be transferred + */ + function transferFrom( + address from, + address to, + uint256 value + ) + public + returns (bool) + { + updateSnapshot(from); + updateSnapshot(to); + return super.transferFrom(from, to, value); + } + + /** + * @dev Internal function that mints an amount of the token and assigns it to + * an account. This encapsulates the modification of balances such that the + * proper events are emitted. Takes snapshot before tokens are minted. + * @param account The account that will receive the created tokens. + * @param amount The amount that will be created. + */ + function _mint(address account, uint256 amount) internal { + updateSnapshot(account); + super._mint(account, amount); + } + + /** + * @dev Internal function that burns an amount of the token of a given + * account. Takes snapshot before tokens are burned. + * @param account The account whose tokens will be burnt. + * @param amount The amount that will be burnt. + */ + function _burn(address account, uint256 amount) internal { + updateSnapshot(account); + super._burn(account, amount); + } + + function updateSnapshot(address account) private { + if (lastSnapshotId(account) < currentSnapshotId) { + snapshotIds[account].push(currentSnapshotId); + snapshotBalances[account].push(balanceOf(account)); + } + } + + function lastSnapshotId(address account) private view returns (uint256) { + uint256[] storage snapshots = snapshotIds[account]; + if (snapshots.length == 0) { + return 0; + } else { + return snapshots[snapshots.length - 1]; + } + } + +} diff --git a/contracts/token/ERC721/ERC721MetadataMintable.sol b/contracts/token/ERC721/ERC721MetadataMintable.sol new file mode 100644 index 00000000000..95a9f21246e --- /dev/null +++ b/contracts/token/ERC721/ERC721MetadataMintable.sol @@ -0,0 +1,32 @@ +pragma solidity ^0.4.24; + +import "./ERC721Metadata.sol"; +import "../../access/roles/MinterRole.sol"; + + +/** + * @title ERC721MetadataMintable + * @dev ERC721 minting logic with metadata + */ +contract ERC721MetadataMintable is ERC721, ERC721Metadata, MinterRole { + /** + * @dev Function to mint tokens + * @param to The address that will receive the minted tokens. + * @param tokenId The token id to mint. + * @param tokenURI The token URI of the minted token. + * @return A boolean that indicates if the operation was successful. + */ + function mintWithTokenURI( + address to, + uint256 tokenId, + string tokenURI + ) + public + onlyMinter + returns (bool) + { + _mint(to, tokenId); + _setTokenURI(tokenId, tokenURI); + return true; + } +} diff --git a/contracts/token/ERC721/ERC721Mintable.sol b/contracts/token/ERC721/ERC721Mintable.sol index f7cd0e9f94f..74b7b0186f5 100644 --- a/contracts/token/ERC721/ERC721Mintable.sol +++ b/contracts/token/ERC721/ERC721Mintable.sol @@ -1,29 +1,13 @@ pragma solidity ^0.4.24; -import "./ERC721Full.sol"; +import "./ERC721.sol"; import "../../access/roles/MinterRole.sol"; /** * @title ERC721Mintable * @dev ERC721 minting logic */ -contract ERC721Mintable is ERC721Full, MinterRole { - event MintingFinished(); - - bool private _mintingFinished = false; - - modifier onlyBeforeMintingFinished() { - require(!_mintingFinished); - _; - } - - /** - * @return true if the minting is finished. - */ - function mintingFinished() public view returns(bool) { - return _mintingFinished; - } - +contract ERC721Mintable is ERC721, MinterRole { /** * @dev Function to mint tokens * @param to The address that will receive the minted tokens. @@ -36,40 +20,9 @@ contract ERC721Mintable is ERC721Full, MinterRole { ) public onlyMinter - onlyBeforeMintingFinished returns (bool) { _mint(to, tokenId); return true; } - - function mintWithTokenURI( - address to, - uint256 tokenId, - string tokenURI - ) - public - onlyMinter - onlyBeforeMintingFinished - returns (bool) - { - mint(to, tokenId); - _setTokenURI(tokenId, tokenURI); - return true; - } - - /** - * @dev Function to stop minting new tokens. - * @return True if the operation was successful. - */ - function finishMinting() - public - onlyMinter - onlyBeforeMintingFinished - returns (bool) - { - _mintingFinished = true; - emit MintingFinished(); - return true; - } } diff --git a/contracts/utils/Arrays.sol b/contracts/utils/Arrays.sol new file mode 100644 index 00000000000..6882ea99595 --- /dev/null +++ b/contracts/utils/Arrays.sol @@ -0,0 +1,56 @@ +pragma solidity ^0.4.23; + +import "../math/Math.sol"; + + +/** + * @title Arrays + * @dev Utility library of inline array functions + */ +library Arrays { + + /** + * @dev Upper bound search function which is kind of binary search algoritm. It searches sorted + * array to find index of the element value. If element is found then returns it's index otherwise + * it returns index of first element which is grater than searched value. If searched element is + * bigger than any array element function then returns first index after last element (i.e. all + * values inside the array are smaller than the target). Complexity O(log n). + * @param array The array sorted in ascending order. + * @param element The element's value to be find. + * @return The calculated index value. Returns 0 for empty array. + */ + function findUpperBound( + uint256[] storage array, + uint256 element + ) + internal + view + returns (uint256) + { + if (array.length == 0) { + return 0; + } + + uint256 low = 0; + uint256 high = array.length; + + while (low < high) { + uint256 mid = Math.average(low, high); + + // Note that mid will always be strictly less than high (i.e. it will be a valid array index) + // because Math.average rounds down (it does integer division with truncation). + if (array[mid] > element) { + high = mid; + } else { + low = mid + 1; + } + } + + // At this point `low` is the exclusive upper bound. We will return the inclusive upper bound. + if (low > 0 && array[low - 1] == element) { + return low - 1; + } else { + return low; + } + } +} diff --git a/ethpm.json b/ethpm.json index 27a2947263e..bb2674762cc 100644 --- a/ethpm.json +++ b/ethpm.json @@ -1,6 +1,6 @@ { "package_name": "zeppelin", - "version": "1.12.0", + "version": "2.0.0-rc.1", "description": "Secure Smart Contract library for Solidity", "authors": [ "OpenZeppelin Community " diff --git a/package-lock.json b/package-lock.json index 3fa9ab04e48..29034359c34 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "openzeppelin-solidity", - "version": "1.12.0", + "version": "2.0.0-rc.1", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index d3eedb5c278..235e500eedb 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "openzeppelin-solidity", - "version": "1.12.0", + "version": "2.0.0-rc.1", "description": "Secure Smart Contract library for Solidity", "files": [ "build", diff --git a/test/BreakInvariantBounty.test.js b/test/BreakInvariantBounty.test.js index 94a5fa0f7c7..5995f64f599 100644 --- a/test/BreakInvariantBounty.test.js +++ b/test/BreakInvariantBounty.test.js @@ -1,98 +1,130 @@ const { ethGetBalance, ethSendTransaction } = require('./helpers/web3'); +const { ether } = require('./helpers/ether'); +const { sendEther } = require('./helpers/sendTransaction'); +const { balanceDifference } = require('./helpers/balanceDiff'); const expectEvent = require('./helpers/expectEvent'); const { assertRevert } = require('./helpers/assertRevert'); -const SecureInvariantTargetBounty = artifacts.require('SecureInvariantTargetBounty'); -const InsecureInvariantTargetBounty = artifacts.require('InsecureInvariantTargetBounty'); +const BreakInvariantBountyMock = artifacts.require('BreakInvariantBountyMock'); +const TargetMock = artifacts.require('TargetMock'); require('chai') .use(require('chai-bignumber')(web3.BigNumber)) .should(); -const sendReward = async (from, to, value) => ethSendTransaction({ - from, - to, - value, -}); - -const reward = new web3.BigNumber(web3.toWei(1, 'ether')); +const reward = ether(1); -contract('BreakInvariantBounty', function ([_, owner, researcher, nonTarget]) { - context('against secure contract', function () { - beforeEach(async function () { - this.bounty = await SecureInvariantTargetBounty.new({ from: owner }); - }); +contract('BreakInvariantBounty', function ([_, owner, researcher, anyone, nonTarget]) { + beforeEach(async function () { + this.bounty = await BreakInvariantBountyMock.new({ from: owner }); + }); - it('can set reward', async function () { - await sendReward(owner, this.bounty.address, reward); + it('can set reward', async function () { + await sendEther(owner, this.bounty.address, reward); + (await ethGetBalance(this.bounty.address)).should.be.bignumber.equal(reward); + }); - const balance = await ethGetBalance(this.bounty.address); - balance.should.be.bignumber.equal(reward); + context('with reward', function () { + beforeEach(async function () { + await sendEther(owner, this.bounty.address, reward); }); - context('with reward', function () { - beforeEach(async function () { - const result = await this.bounty.createTarget({ from: researcher }); - const event = expectEvent.inLogs(result.logs, 'TargetCreated'); - - this.targetAddress = event.args.createdAddress; - - await sendReward(owner, this.bounty.address, reward); - - const balance = await ethGetBalance(this.bounty.address); - balance.should.be.bignumber.equal(reward); + describe('claim', function () { + it('is initially claimable', async function () { + (await this.bounty.claimable()).should.equal(true); }); - it('cannot claim reward', async function () { - await assertRevert( - this.bounty.claim(this.targetAddress, { from: researcher }), - ); + it('can create claimable target', async function () { + const { logs } = await this.bounty.createTarget({ from: researcher }); + expectEvent.inLogs(logs, 'TargetCreated'); }); - }); - }); - context('against broken contract', function () { - beforeEach(async function () { - this.bounty = await InsecureInvariantTargetBounty.new(); - - const result = await this.bounty.createTarget({ from: researcher }); - const event = expectEvent.inLogs(result.logs, 'TargetCreated'); + context('with target', async function () { + beforeEach(async function () { + const { logs } = await this.bounty.createTarget({ from: researcher }); + const event = expectEvent.inLogs(logs, 'TargetCreated'); + this.target = TargetMock.at(event.args.createdAddress); + }); + + context('before exploiting vulnerability', async function () { + it('reverts when claiming reward', async function () { + await assertRevert(this.bounty.claim(this.target.address, { from: researcher })); + }); + }); + + context('after exploiting vulnerability', async function () { + beforeEach(async function () { + await this.target.exploitVulnerability({ from: researcher }); + }); + + it('sends the reward to the researcher', async function () { + await this.bounty.claim(this.target.address, { from: anyone }); + (await balanceDifference(researcher, () => this.bounty.withdrawPayments(researcher))) + .should.be.bignumber.equal(reward); + (await ethGetBalance(this.bounty.address)).should.be.bignumber.equal(0); + }); + + context('after claiming', async function () { + beforeEach(async function () { + await this.bounty.claim(this.target.address, { from: researcher }); + }); + + it('is not claimable', async function () { + (await this.bounty.claimable()).should.equal(false); + }); + + it('no longer accepts rewards', async function () { + await assertRevert(ethSendTransaction({ from: owner, to: this.bounty.address, value: reward })); + }); + + it('reverts when reclaimed', async function () { + await assertRevert(this.bounty.claim(this.target.address, { from: researcher })); + }); + }); + }); + }); - this.targetAddress = event.args.createdAddress; - await sendReward(owner, this.bounty.address, reward); + context('with non-target', function () { + it('reverts when claiming reward', async function () { + await assertRevert(this.bounty.claim(nonTarget, { from: researcher })); + }); + }); }); - it('can claim reward', async function () { - await this.bounty.claim(this.targetAddress, { from: researcher }); - const claim = await this.bounty.claimed(); - - claim.should.equal(true); - - const researcherPrevBalance = await ethGetBalance(researcher); + describe('cancelBounty', function () { + context('before canceling', function () { + it('is claimable', async function () { + (await this.bounty.claimable()).should.equal(true); + }); + + it('can be canceled by the owner', async function () { + const { logs } = await this.bounty.cancelBounty({ from: owner }); + expectEvent.inLogs(logs, 'BountyCanceled'); + (await balanceDifference(owner, () => this.bounty.withdrawPayments(owner))) + .should.be.bignumber.equal(reward); + }); + + it('reverts when canceled by anyone', async function () { + await assertRevert(this.bounty.cancelBounty({ from: anyone })); + }); + }); - await this.bounty.withdrawPayments(researcher, { gasPrice: 0 }); - const updatedBalance = await ethGetBalance(this.bounty.address); - updatedBalance.should.be.bignumber.equal(0); + context('after canceling', async function () { + beforeEach(async function () { + await this.bounty.cancelBounty({ from: owner }); + }); - const researcherCurrBalance = await ethGetBalance(researcher); - researcherCurrBalance.sub(researcherPrevBalance).should.be.bignumber.equal(reward); - }); + it('is not claimable', async function () { + (await this.bounty.claimable()).should.equal(false); + }); - it('cannot claim reward from non-target', async function () { - await assertRevert( - this.bounty.claim(nonTarget, { from: researcher }) - ); - }); - - context('reward claimed', function () { - beforeEach(async function () { - await this.bounty.claim(this.targetAddress, { from: researcher }); - }); + it('no longer accepts rewards', async function () { + await assertRevert(ethSendTransaction({ from: owner, to: this.bounty.address, value: reward })); + }); - it('should no longer be payable', async function () { - await assertRevert( - sendReward(owner, this.bounty.address, reward) - ); + it('reverts when recanceled', async function () { + await assertRevert(this.bounty.cancelBounty({ from: owner })); + }); }); }); }); diff --git a/test/access/roles/PublicRole.behavior.js b/test/access/roles/PublicRole.behavior.js index 1e489a46886..f1c723b7821 100644 --- a/test/access/roles/PublicRole.behavior.js +++ b/test/access/roles/PublicRole.behavior.js @@ -89,6 +89,11 @@ function shouldBehaveLikePublicRole (authorized, otherAuthorized, [anyone], role (await this.contract[`is${rolename}`](authorized)).should.equal(false); }); + it(`emits a ${rolename}Removed event`, async function () { + const { logs } = await this.contract[`renounce${rolename}`]({ from: authorized }); + expectEvent.inLogs(logs, `${rolename}Removed`, { account: authorized }); + }); + it('doesn\'t revert when renouncing unassigned role', async function () { await this.contract[`renounce${rolename}`]({ from: anyone }); }); diff --git a/test/crowdsale/AllowanceCrowdsale.test.js b/test/crowdsale/AllowanceCrowdsale.test.js index c35707d6d4c..c0ff4cd91ba 100644 --- a/test/crowdsale/AllowanceCrowdsale.test.js +++ b/test/crowdsale/AllowanceCrowdsale.test.js @@ -1,10 +1,11 @@ +const expectEvent = require('../helpers/expectEvent'); const { ether } = require('../helpers/ether'); const { assertRevert } = require('../helpers/assertRevert'); const { ethGetBalance } = require('../helpers/web3'); const BigNumber = web3.BigNumber; -const should = require('chai') +require('chai') .use(require('chai-bignumber')(BigNumber)) .should(); @@ -41,12 +42,12 @@ contract('AllowanceCrowdsale', function ([_, investor, wallet, purchaser, tokenW describe('high-level purchase', function () { it('should log purchase', async function () { const { logs } = await this.crowdsale.sendTransaction({ value: value, from: investor }); - const event = logs.find(e => e.event === 'TokensPurchased'); - should.exist(event); - event.args.purchaser.should.equal(investor); - event.args.beneficiary.should.equal(investor); - event.args.value.should.be.bignumber.equal(value); - event.args.amount.should.be.bignumber.equal(expectedTokenAmount); + expectEvent.inLogs(logs, 'TokensPurchased', { + purchaser: investor, + beneficiary: investor, + value: value, + amount: expectedTokenAmount, + }); }); it('should assign tokens to sender', async function () { diff --git a/test/crowdsale/Crowdsale.test.js b/test/crowdsale/Crowdsale.test.js index 556855796d4..4113f7cb428 100644 --- a/test/crowdsale/Crowdsale.test.js +++ b/test/crowdsale/Crowdsale.test.js @@ -1,10 +1,11 @@ +const expectEvent = require('../helpers/expectEvent'); const { assertRevert } = require('../helpers/assertRevert'); const { ether } = require('../helpers/ether'); const { ethGetBalance } = require('../helpers/web3'); const BigNumber = web3.BigNumber; -const should = require('chai') +require('chai') .use(require('chai-bignumber')(BigNumber)) .should(); @@ -82,12 +83,12 @@ contract('Crowdsale', function ([_, investor, wallet, purchaser]) { describe('high-level purchase', function () { it('should log purchase', async function () { const { logs } = await this.crowdsale.sendTransaction({ value: value, from: investor }); - const event = logs.find(e => e.event === 'TokensPurchased'); - should.exist(event); - event.args.purchaser.should.equal(investor); - event.args.beneficiary.should.equal(investor); - event.args.value.should.be.bignumber.equal(value); - event.args.amount.should.be.bignumber.equal(expectedTokenAmount); + expectEvent.inLogs(logs, 'TokensPurchased', { + purchaser: investor, + beneficiary: investor, + value: value, + amount: expectedTokenAmount, + }); }); it('should assign tokens to sender', async function () { @@ -106,12 +107,12 @@ contract('Crowdsale', function ([_, investor, wallet, purchaser]) { describe('low-level purchase', function () { it('should log purchase', async function () { const { logs } = await this.crowdsale.buyTokens(investor, { value: value, from: purchaser }); - const event = logs.find(e => e.event === 'TokensPurchased'); - should.exist(event); - event.args.purchaser.should.equal(purchaser); - event.args.beneficiary.should.equal(investor); - event.args.value.should.be.bignumber.equal(value); - event.args.amount.should.be.bignumber.equal(expectedTokenAmount); + expectEvent.inLogs(logs, 'TokensPurchased', { + purchaser: purchaser, + beneficiary: investor, + value: value, + amount: expectedTokenAmount, + }); }); it('should assign tokens to beneficiary', async function () { diff --git a/test/crowdsale/FinalizableCrowdsale.test.js b/test/crowdsale/FinalizableCrowdsale.test.js index 187c67a36f8..6eea7f2f25a 100644 --- a/test/crowdsale/FinalizableCrowdsale.test.js +++ b/test/crowdsale/FinalizableCrowdsale.test.js @@ -1,12 +1,12 @@ +const expectEvent = require('../helpers/expectEvent'); const { advanceBlock } = require('../helpers/advanceToBlock'); -const { increaseTimeTo, duration } = require('../helpers/increaseTime'); -const { latestTime } = require('../helpers/latestTime'); +const time = require('../helpers/time'); const { expectThrow } = require('../helpers/expectThrow'); const { EVMRevert } = require('../helpers/EVMRevert'); const BigNumber = web3.BigNumber; -const should = require('chai') +require('chai') .use(require('chai-bignumber')(BigNumber)) .should(); @@ -22,9 +22,9 @@ contract('FinalizableCrowdsale', function ([_, wallet, anyone]) { }); beforeEach(async function () { - this.openingTime = (await latestTime()) + duration.weeks(1); - this.closingTime = this.openingTime + duration.weeks(1); - this.afterClosingTime = this.closingTime + duration.seconds(1); + this.openingTime = (await time.latest()) + time.duration.weeks(1); + this.closingTime = this.openingTime + time.duration.weeks(1); + this.afterClosingTime = this.closingTime + time.duration.seconds(1); this.token = await ERC20.new(); this.crowdsale = await FinalizableCrowdsaleImpl.new( @@ -37,20 +37,19 @@ contract('FinalizableCrowdsale', function ([_, wallet, anyone]) { }); it('can be finalized by anyone after ending', async function () { - await increaseTimeTo(this.afterClosingTime); + await time.increaseTo(this.afterClosingTime); await this.crowdsale.finalize({ from: anyone }); }); it('cannot be finalized twice', async function () { - await increaseTimeTo(this.afterClosingTime); + await time.increaseTo(this.afterClosingTime); await this.crowdsale.finalize({ from: anyone }); await expectThrow(this.crowdsale.finalize({ from: anyone }), EVMRevert); }); it('logs finalized', async function () { - await increaseTimeTo(this.afterClosingTime); + await time.increaseTo(this.afterClosingTime); const { logs } = await this.crowdsale.finalize({ from: anyone }); - const event = logs.find(e => e.event === 'CrowdsaleFinalized'); - should.exist(event); + expectEvent.inLogs(logs, 'CrowdsaleFinalized'); }); }); diff --git a/test/crowdsale/IncreasingPriceCrowdsale.test.js b/test/crowdsale/IncreasingPriceCrowdsale.test.js index 767b5f8e568..2024d77516e 100644 --- a/test/crowdsale/IncreasingPriceCrowdsale.test.js +++ b/test/crowdsale/IncreasingPriceCrowdsale.test.js @@ -1,7 +1,6 @@ const { ether } = require('../helpers/ether'); const { advanceBlock } = require('../helpers/advanceToBlock'); -const { increaseTimeTo, duration } = require('../helpers/increaseTime'); -const { latestTime } = require('../helpers/latestTime'); +const time = require('../helpers/time'); const { assertRevert } = require('../helpers/assertRevert'); const BigNumber = web3.BigNumber; @@ -29,9 +28,9 @@ contract('IncreasingPriceCrowdsale', function ([_, investor, wallet, purchaser]) beforeEach(async function () { await advanceBlock(); - this.startTime = (await latestTime()) + duration.weeks(1); - this.closingTime = this.startTime + duration.weeks(1); - this.afterClosingTime = this.closingTime + duration.seconds(1); + this.startTime = (await time.latest()) + time.duration.weeks(1); + this.closingTime = this.startTime + time.duration.weeks(1); + this.afterClosingTime = this.closingTime + time.duration.seconds(1); this.token = await SimpleToken.new(); }); @@ -61,43 +60,43 @@ contract('IncreasingPriceCrowdsale', function ([_, investor, wallet, purchaser]) }); it('at start', async function () { - await increaseTimeTo(this.startTime); + await time.increaseTo(this.startTime); await this.crowdsale.buyTokens(investor, { value, from: purchaser }); (await this.token.balanceOf(investor)).should.be.bignumber.equal(value.mul(initialRate)); }); it('at time 150', async function () { - await increaseTimeTo(this.startTime + 150); + await time.increaseTo(this.startTime + 150); await this.crowdsale.buyTokens(investor, { value, from: purchaser }); (await this.token.balanceOf(investor)).should.be.bignumber.equal(value.mul(rateAtTime150)); }); it('at time 300', async function () { - await increaseTimeTo(this.startTime + 300); + await time.increaseTo(this.startTime + 300); await this.crowdsale.buyTokens(investor, { value, from: purchaser }); (await this.token.balanceOf(investor)).should.be.bignumber.equal(value.mul(rateAtTime300)); }); it('at time 1500', async function () { - await increaseTimeTo(this.startTime + 1500); + await time.increaseTo(this.startTime + 1500); await this.crowdsale.buyTokens(investor, { value, from: purchaser }); (await this.token.balanceOf(investor)).should.be.bignumber.equal(value.mul(rateAtTime1500)); }); it('at time 30', async function () { - await increaseTimeTo(this.startTime + 30); + await time.increaseTo(this.startTime + 30); await this.crowdsale.buyTokens(investor, { value, from: purchaser }); (await this.token.balanceOf(investor)).should.be.bignumber.equal(value.mul(rateAtTime30)); }); it('at time 150000', async function () { - await increaseTimeTo(this.startTime + 150000); + await time.increaseTo(this.startTime + 150000); await this.crowdsale.buyTokens(investor, { value, from: purchaser }); (await this.token.balanceOf(investor)).should.be.bignumber.equal(value.mul(rateAtTime150000)); }); it('at time 450000', async function () { - await increaseTimeTo(this.startTime + 450000); + await time.increaseTo(this.startTime + 450000); await this.crowdsale.buyTokens(investor, { value, from: purchaser }); (await this.token.balanceOf(investor)).should.be.bignumber.equal(value.mul(rateAtTime450000)); }); diff --git a/test/crowdsale/MintedCrowdsale.behavior.js b/test/crowdsale/MintedCrowdsale.behavior.js index f5d61328c3a..ab8dc8259cc 100644 --- a/test/crowdsale/MintedCrowdsale.behavior.js +++ b/test/crowdsale/MintedCrowdsale.behavior.js @@ -1,8 +1,9 @@ +const expectEvent = require('../helpers/expectEvent'); const { ethGetBalance } = require('../helpers/web3'); const BigNumber = web3.BigNumber; -const should = require('chai') +require('chai') .use(require('chai-bignumber')(BigNumber)) .should(); @@ -20,12 +21,12 @@ function shouldBehaveLikeMintedCrowdsale ([_, investor, wallet, purchaser], rate describe('high-level purchase', function () { it('should log purchase', async function () { const { logs } = await this.crowdsale.sendTransaction({ value: value, from: investor }); - const event = logs.find(e => e.event === 'TokensPurchased'); - should.exist(event); - event.args.purchaser.should.equal(investor); - event.args.beneficiary.should.equal(investor); - event.args.value.should.be.bignumber.equal(value); - event.args.amount.should.be.bignumber.equal(expectedTokenAmount); + expectEvent.inLogs(logs, 'TokensPurchased', { + purchaser: investor, + beneficiary: investor, + value: value, + amount: expectedTokenAmount, + }); }); it('should assign tokens to sender', async function () { diff --git a/test/crowdsale/PostDeliveryCrowdsale.test.js b/test/crowdsale/PostDeliveryCrowdsale.test.js index 94ebd808b15..866e4da15b4 100644 --- a/test/crowdsale/PostDeliveryCrowdsale.test.js +++ b/test/crowdsale/PostDeliveryCrowdsale.test.js @@ -1,6 +1,5 @@ const { advanceBlock } = require('../helpers/advanceToBlock'); -const { increaseTimeTo, duration } = require('../helpers/increaseTime'); -const { latestTime } = require('../helpers/latestTime'); +const time = require('../helpers/time'); const { expectThrow } = require('../helpers/expectThrow'); const { EVMRevert } = require('../helpers/EVMRevert'); const { ether } = require('../helpers/ether'); @@ -24,9 +23,9 @@ contract('PostDeliveryCrowdsale', function ([_, investor, wallet, purchaser]) { }); beforeEach(async function () { - this.openingTime = (await latestTime()) + duration.weeks(1); - this.closingTime = this.openingTime + duration.weeks(1); - this.afterClosingTime = this.closingTime + duration.seconds(1); + this.openingTime = (await time.latest()) + time.duration.weeks(1); + this.closingTime = this.openingTime + time.duration.weeks(1); + this.afterClosingTime = this.closingTime + time.duration.seconds(1); this.token = await SimpleToken.new(); this.crowdsale = await PostDeliveryCrowdsaleImpl.new( this.openingTime, this.closingTime, rate, wallet, this.token.address @@ -36,7 +35,7 @@ contract('PostDeliveryCrowdsale', function ([_, investor, wallet, purchaser]) { context('after opening time', function () { beforeEach(async function () { - await increaseTimeTo(this.openingTime); + await time.increaseTo(this.openingTime); }); context('with bought tokens', function () { @@ -57,7 +56,7 @@ contract('PostDeliveryCrowdsale', function ([_, investor, wallet, purchaser]) { context('after closing time', function () { beforeEach(async function () { - await increaseTimeTo(this.afterClosingTime); + await time.increaseTo(this.afterClosingTime); }); it('allows beneficiaries to withdraw tokens', async function () { diff --git a/test/crowdsale/RefundableCrowdsale.test.js b/test/crowdsale/RefundableCrowdsale.test.js index 749ec5cac59..77e0bfebb93 100644 --- a/test/crowdsale/RefundableCrowdsale.test.js +++ b/test/crowdsale/RefundableCrowdsale.test.js @@ -1,7 +1,6 @@ const { ether } = require('../helpers/ether'); const { advanceBlock } = require('../helpers/advanceToBlock'); -const { increaseTimeTo, duration } = require('../helpers/increaseTime'); -const { latestTime } = require('../helpers/latestTime'); +const time = require('../helpers/time'); const { expectThrow } = require('../helpers/expectThrow'); const { EVMRevert } = require('../helpers/EVMRevert'); const { ethGetBalance } = require('../helpers/web3'); @@ -27,9 +26,9 @@ contract('RefundableCrowdsale', function ([_, wallet, investor, purchaser, anyon }); beforeEach(async function () { - this.openingTime = (await latestTime()) + duration.weeks(1); - this.closingTime = this.openingTime + duration.weeks(1); - this.afterClosingTime = this.closingTime + duration.seconds(1); + this.openingTime = (await time.latest()) + time.duration.weeks(1); + this.closingTime = this.openingTime + time.duration.weeks(1); + this.afterClosingTime = this.closingTime + time.duration.seconds(1); this.preWalletBalance = await ethGetBalance(wallet); this.token = await SimpleToken.new(); @@ -61,7 +60,7 @@ contract('RefundableCrowdsale', function ([_, wallet, investor, purchaser, anyon context('after opening time', function () { beforeEach(async function () { - await increaseTimeTo(this.openingTime); + await time.increaseTo(this.openingTime); }); it('denies refunds', async function () { @@ -75,7 +74,7 @@ contract('RefundableCrowdsale', function ([_, wallet, investor, purchaser, anyon context('after closing time and finalization', function () { beforeEach(async function () { - await increaseTimeTo(this.afterClosingTime); + await time.increaseTo(this.afterClosingTime); await this.crowdsale.finalize({ from: anyone }); }); @@ -95,7 +94,7 @@ contract('RefundableCrowdsale', function ([_, wallet, investor, purchaser, anyon context('after closing time and finalization', function () { beforeEach(async function () { - await increaseTimeTo(this.afterClosingTime); + await time.increaseTo(this.afterClosingTime); await this.crowdsale.finalize({ from: anyone }); }); diff --git a/test/crowdsale/TimedCrowdsale.test.js b/test/crowdsale/TimedCrowdsale.test.js index df3e01c817f..cfbc3d3cf26 100644 --- a/test/crowdsale/TimedCrowdsale.test.js +++ b/test/crowdsale/TimedCrowdsale.test.js @@ -1,7 +1,6 @@ const { ether } = require('../helpers/ether'); const { advanceBlock } = require('../helpers/advanceToBlock'); -const { increaseTimeTo, duration } = require('../helpers/increaseTime'); -const { latestTime } = require('../helpers/latestTime'); +const time = require('../helpers/time'); const { expectThrow } = require('../helpers/expectThrow'); const { EVMRevert } = require('../helpers/EVMRevert'); @@ -25,21 +24,21 @@ contract('TimedCrowdsale', function ([_, investor, wallet, purchaser]) { }); beforeEach(async function () { - this.openingTime = (await latestTime()) + duration.weeks(1); - this.closingTime = this.openingTime + duration.weeks(1); - this.afterClosingTime = this.closingTime + duration.seconds(1); + this.openingTime = (await time.latest()) + time.duration.weeks(1); + this.closingTime = this.openingTime + time.duration.weeks(1); + this.afterClosingTime = this.closingTime + time.duration.seconds(1); this.token = await SimpleToken.new(); }); it('rejects an opening time in the past', async function () { await expectThrow(TimedCrowdsaleImpl.new( - (await latestTime()) - duration.days(1), this.closingTime, rate, wallet, this.token.address + (await time.latest()) - time.duration.days(1), this.closingTime, rate, wallet, this.token.address ), EVMRevert); }); it('rejects a closing time before the opening time', async function () { await expectThrow(TimedCrowdsaleImpl.new( - this.openingTime, this.openingTime - duration.seconds(1), rate, wallet, this.token.address + this.openingTime, this.openingTime - time.duration.seconds(1), rate, wallet, this.token.address ), EVMRevert); }); @@ -53,7 +52,7 @@ contract('TimedCrowdsale', function ([_, investor, wallet, purchaser]) { it('should be ended only after end', async function () { (await this.crowdsale.hasClosed()).should.equal(false); - await increaseTimeTo(this.afterClosingTime); + await time.increaseTo(this.afterClosingTime); (await this.crowdsale.isOpen()).should.equal(false); (await this.crowdsale.hasClosed()).should.equal(true); }); @@ -66,14 +65,14 @@ contract('TimedCrowdsale', function ([_, investor, wallet, purchaser]) { }); it('should accept payments after start', async function () { - await increaseTimeTo(this.openingTime); + await time.increaseTo(this.openingTime); (await this.crowdsale.isOpen()).should.equal(true); await this.crowdsale.send(value); await this.crowdsale.buyTokens(investor, { value: value, from: purchaser }); }); it('should reject payments after end', async function () { - await increaseTimeTo(this.afterClosingTime); + await time.increaseTo(this.afterClosingTime); await expectThrow(this.crowdsale.send(value), EVMRevert); await expectThrow(this.crowdsale.buyTokens(investor, { value: value, from: purchaser }), EVMRevert); }); diff --git a/test/drafts/SignatureBouncer.test.js b/test/drafts/SignatureBouncer.test.js index b320ca98c32..c234687fc31 100644 --- a/test/drafts/SignatureBouncer.test.js +++ b/test/drafts/SignatureBouncer.test.js @@ -128,6 +128,12 @@ contract('SignatureBouncer', function ([_, signer, otherSigner, anyone, authoriz ) ); }); + + it('does not allow msg.data shorter than SIGNATURE_SIZE', async function () { + await assertRevert( + this.sigBouncer.tooShortMsgData({ from: authorizedUser }) + ); + }); }); }); diff --git a/test/examples/SampleCrowdsale.test.js b/test/examples/SampleCrowdsale.test.js index e4714353536..303ddf70400 100644 --- a/test/examples/SampleCrowdsale.test.js +++ b/test/examples/SampleCrowdsale.test.js @@ -1,7 +1,6 @@ const { ether } = require('../helpers/ether'); const { advanceBlock } = require('../helpers/advanceToBlock'); -const { increaseTimeTo, duration } = require('../helpers/increaseTime'); -const { latestTime } = require('../helpers/latestTime'); +const time = require('../helpers/time'); const { expectThrow } = require('../helpers/expectThrow'); const { EVMRevert } = require('../helpers/EVMRevert'); const { assertRevert } = require('../helpers/assertRevert'); @@ -27,9 +26,9 @@ contract('SampleCrowdsale', function ([_, deployer, owner, wallet, investor]) { }); beforeEach(async function () { - this.openingTime = (await latestTime()) + duration.weeks(1); - this.closingTime = this.openingTime + duration.weeks(1); - this.afterClosingTime = this.closingTime + duration.seconds(1); + this.openingTime = (await time.latest()) + time.duration.weeks(1); + this.closingTime = this.openingTime + time.duration.weeks(1); + this.afterClosingTime = this.closingTime + time.duration.seconds(1); this.token = await SampleCrowdsaleToken.new({ from: deployer }); this.crowdsale = await SampleCrowdsale.new( @@ -68,7 +67,7 @@ contract('SampleCrowdsale', function ([_, deployer, owner, wallet, investor]) { const investmentAmount = ether(1); const expectedTokenAmount = RATE.mul(investmentAmount); - await increaseTimeTo(this.openingTime); + await time.increaseTo(this.openingTime); await this.crowdsale.buyTokens(investor, { value: investmentAmount, from: investor }); (await this.token.balanceOf(investor)).should.be.bignumber.equal(expectedTokenAmount); @@ -76,23 +75,23 @@ contract('SampleCrowdsale', function ([_, deployer, owner, wallet, investor]) { }); it('should reject payments after end', async function () { - await increaseTimeTo(this.afterClosingTime); + await time.increaseTo(this.afterClosingTime); await expectThrow(this.crowdsale.send(ether(1)), EVMRevert); await expectThrow(this.crowdsale.buyTokens(investor, { value: ether(1), from: investor }), EVMRevert); }); it('should reject payments over cap', async function () { - await increaseTimeTo(this.openingTime); + await time.increaseTo(this.openingTime); await this.crowdsale.send(CAP); await expectThrow(this.crowdsale.send(1), EVMRevert); }); it('should allow finalization and transfer funds to wallet if the goal is reached', async function () { - await increaseTimeTo(this.openingTime); + await time.increaseTo(this.openingTime); await this.crowdsale.send(GOAL); const beforeFinalization = await ethGetBalance(wallet); - await increaseTimeTo(this.afterClosingTime); + await time.increaseTo(this.afterClosingTime); await this.crowdsale.finalize({ from: owner }); const afterFinalization = await ethGetBalance(wallet); @@ -102,9 +101,9 @@ contract('SampleCrowdsale', function ([_, deployer, owner, wallet, investor]) { it('should allow refunds if the goal is not reached', async function () { const balanceBeforeInvestment = await ethGetBalance(investor); - await increaseTimeTo(this.openingTime); + await time.increaseTo(this.openingTime); await this.crowdsale.sendTransaction({ value: ether(1), from: investor, gasPrice: 0 }); - await increaseTimeTo(this.afterClosingTime); + await time.increaseTo(this.afterClosingTime); await this.crowdsale.finalize({ from: owner }); await this.crowdsale.claimRefund(investor, { gasPrice: 0 }); diff --git a/test/examples/SimpleToken.test.js b/test/examples/SimpleToken.test.js index a28cf4641ab..f59a8200017 100644 --- a/test/examples/SimpleToken.test.js +++ b/test/examples/SimpleToken.test.js @@ -8,34 +8,32 @@ require('chai') .should(); contract('SimpleToken', function ([_, creator]) { - let token; - const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000'; beforeEach(async function () { - token = await SimpleToken.new({ from: creator }); + this.token = await SimpleToken.new({ from: creator }); }); it('has a name', async function () { - (await token.name()).should.equal('SimpleToken'); + (await this.token.name()).should.equal('SimpleToken'); }); it('has a symbol', async function () { - (await token.symbol()).should.equal('SIM'); + (await this.token.symbol()).should.equal('SIM'); }); it('has 18 decimals', async function () { - (await token.decimals()).should.be.bignumber.equal(18); + (await this.token.decimals()).should.be.bignumber.equal(18); }); it('assigns the initial total supply to the creator', async function () { - const totalSupply = await token.totalSupply(); - const creatorBalance = await token.balanceOf(creator); + const totalSupply = await this.token.totalSupply(); + const creatorBalance = await this.token.balanceOf(creator); creatorBalance.should.be.bignumber.equal(totalSupply); - const receipt = await web3.eth.getTransactionReceipt(token.transactionHash); - const logs = decodeLogs(receipt.logs, SimpleToken, token.address); + const receipt = await web3.eth.getTransactionReceipt(this.token.transactionHash); + const logs = decodeLogs(receipt.logs, SimpleToken, this.token.address); logs.length.should.equal(1); logs[0].event.should.equal('Transfer'); logs[0].args.from.valueOf().should.equal(ZERO_ADDRESS); diff --git a/test/helpers/balanceDiff.js b/test/helpers/balanceDiff.js new file mode 100644 index 00000000000..8f88ba40045 --- /dev/null +++ b/test/helpers/balanceDiff.js @@ -0,0 +1,10 @@ +async function balanceDifference (account, promise) { + const balanceBefore = web3.eth.getBalance(account); + await promise(); + const balanceAfter = web3.eth.getBalance(account); + return balanceAfter.minus(balanceBefore); +} + +module.exports = { + balanceDifference, +}; diff --git a/test/helpers/expectEvent.js b/test/helpers/expectEvent.js index 37fa1083eb3..b857b6636bf 100644 --- a/test/helpers/expectEvent.js +++ b/test/helpers/expectEvent.js @@ -1,24 +1,18 @@ -const should = require('chai').should(); +const BigNumber = web3.BigNumber; +const should = require('chai') + .use(require('chai-bignumber')(BigNumber)) + .should(); function inLogs (logs, eventName, eventArgs = {}) { const event = logs.find(function (e) { if (e.event === eventName) { - let matches = true; - for (const [k, v] of Object.entries(eventArgs)) { - if (e.args[k] !== v) { - matches = false; - } - } - - if (matches) { - return true; + contains(e.args, k, v); } + return true; } }); - should.exist(event); - return event; } @@ -27,6 +21,20 @@ async function inTransaction (tx, eventName, eventArgs = {}) { return inLogs(logs, eventName, eventArgs); } +function contains (args, key, value) { + if (isBigNumber(args[key])) { + args[key].should.be.bignumber.equal(value); + } else { + args[key].should.be.equal(value); + } +} + +function isBigNumber (object) { + return object.isBigNumber || + object instanceof BigNumber || + (object.constructor && object.constructor.name === 'BigNumber'); +} + module.exports = { inLogs, inTransaction, diff --git a/test/helpers/latestTime.js b/test/helpers/latestTime.js deleted file mode 100644 index fee9521d677..00000000000 --- a/test/helpers/latestTime.js +++ /dev/null @@ -1,11 +0,0 @@ -const { ethGetBlock } = require('./web3'); - -// Returns the time of the last mined block in seconds -async function latestTime () { - const block = await ethGetBlock('latest'); - return block.timestamp; -} - -module.exports = { - latestTime, -}; diff --git a/test/helpers/sendTransaction.js b/test/helpers/sendTransaction.js index e49376b4488..9d4f3068ff4 100644 --- a/test/helpers/sendTransaction.js +++ b/test/helpers/sendTransaction.js @@ -15,7 +15,16 @@ function sendTransaction (target, name, argsTypes, argsValues, opts) { return target.sendTransaction(Object.assign({ data: encodedData }, opts)); } +function sendEther (from, to, value) { + web3.eth.sendTransaction({ + from: from, + to: to, + value: value, + gasPrice: 0, + }); +} module.exports = { findMethod, sendTransaction, + sendEther, }; diff --git a/test/helpers/increaseTime.js b/test/helpers/time.js similarity index 78% rename from test/helpers/increaseTime.js rename to test/helpers/time.js index b22ba6b04b3..b7f3c000105 100644 --- a/test/helpers/increaseTime.js +++ b/test/helpers/time.js @@ -1,7 +1,13 @@ -const { latestTime } = require('./latestTime'); +const { ethGetBlock } = require('./web3'); + +// Returns the time of the last mined block in seconds +async function latest () { + const block = await ethGetBlock('latest'); + return block.timestamp; +} // Increases ganache time by the passed duration in seconds -function increaseTime (duration) { +function increase (duration) { const id = Date.now(); return new Promise((resolve, reject) => { @@ -31,12 +37,12 @@ function increaseTime (duration) { * * @param target time in seconds */ -async function increaseTimeTo (target) { - const now = (await latestTime()); +async function increaseTo (target) { + const now = (await latest()); if (target < now) throw Error(`Cannot increase current time(${now}) to a moment in the past(${target})`); const diff = target - now; - return increaseTime(diff); + return increase(diff); } const duration = { @@ -49,7 +55,8 @@ const duration = { }; module.exports = { - increaseTime, - increaseTimeTo, + latest, + increase, + increaseTo, duration, }; diff --git a/test/library/ECDSA.test.js b/test/library/ECDSA.test.js index 7fe7056538a..f8d333a5f05 100644 --- a/test/library/ECDSA.test.js +++ b/test/library/ECDSA.test.js @@ -11,56 +11,117 @@ const WRONG_MESSAGE = web3.sha3('Nope'); contract('ECDSA', function ([_, anyone]) { beforeEach(async function () { - this.mock = await ECDSAMock.new(); + this.ecdsa = await ECDSAMock.new(); }); - it('recover v0', async function () { - // Signature generated outside ganache with method web3.eth.sign(signer, message) - const signer = '0x2cc1166f6212628a0deef2b33befb2187d35b86c'; - // eslint-disable-next-line max-len - const signature = '0x5d99b6f7f6d1f73d1a26497f2b1c89b24c0993913f86e9a2d02cd69887d9c94f3c880358579d811b21dd1b7fd9bb01c1d81d10e69f0384e675c32b39643be89200'; - (await this.mock.recover(TEST_MESSAGE, signature)).should.equal(signer); - }); + context('recover with valid signature', function () { + context('with v0 signature', function () { + // Signature generated outside ganache with method web3.eth.sign(signer, message) + const signer = '0x2cc1166f6212628a0deef2b33befb2187d35b86c'; + // eslint-disable-next-line max-len + const signatureWithoutVersion = '0x5d99b6f7f6d1f73d1a26497f2b1c89b24c0993913f86e9a2d02cd69887d9c94f3c880358579d811b21dd1b7fd9bb01c1d81d10e69f0384e675c32b39643be892'; - it('recover v1', async function () { - // Signature generated outside ganache with method web3.eth.sign(signer, message) - const signer = '0x1e318623ab09fe6de3c9b8672098464aeda9100e'; - // eslint-disable-next-line max-len - const signature = '0x331fe75a821c982f9127538858900d87d3ec1f9f737338ad67cad133fa48feff48e6fa0c18abc62e42820f05943e47af3e9fbe306ce74d64094bdf1691ee53e001'; - (await this.mock.recover(TEST_MESSAGE, signature)).should.equal(signer); - }); + context('with 00 as version value', function () { + it('works', async function () { + const version = '00'; + const signature = signatureWithoutVersion + version; + (await this.ecdsa.recover(TEST_MESSAGE, signature)).should.equal(signer); + }); + }); - it('recover using web3.eth.sign()', async function () { - // Create the signature - const signature = signMessage(anyone, TEST_MESSAGE); + context('with 27 as version value', function () { + it('works', async function () { + const version = '1b'; // 27 = 1b. + const signature = signatureWithoutVersion + version; + (await this.ecdsa.recover(TEST_MESSAGE, signature)).should.equal(signer); + }); + }); - // Recover the signer address from the generated message and signature. - (await this.mock.recover( - toEthSignedMessageHash(TEST_MESSAGE), - signature - )).should.equal(anyone); - }); + context('with wrong version', function () { + it('returns 0', async function () { + // The last two hex digits are the signature version. + // The only valid values are 0, 1, 27 and 28. + const version = '02'; + const signature = signatureWithoutVersion + version; + (await this.ecdsa.recover(TEST_MESSAGE, signature)).should.equal( + '0x0000000000000000000000000000000000000000'); + }); + }); + }); - it('recover using web3.eth.sign() should return wrong signer', async function () { - // Create the signature - const signature = signMessage(anyone, TEST_MESSAGE); + context('with v1 signature', function () { + const signer = '0x1e318623ab09fe6de3c9b8672098464aeda9100e'; + // eslint-disable-next-line max-len + const signatureWithoutVersion = '0x331fe75a821c982f9127538858900d87d3ec1f9f737338ad67cad133fa48feff48e6fa0c18abc62e42820f05943e47af3e9fbe306ce74d64094bdf1691ee53e0'; - // Recover the signer address from the generated message and wrong signature. - (await this.mock.recover(WRONG_MESSAGE, signature)).should.not.equal(anyone); + context('with 01 as version value', function () { + it('works', async function () { + const version = '01'; + const signature = signatureWithoutVersion + version; + (await this.ecdsa.recover(TEST_MESSAGE, signature)).should.equal(signer); + }); + }); + + context('with 28 signature', function () { + it('works', async function () { + const version = '1c'; // 28 = 1c. + const signature = signatureWithoutVersion + version; + (await this.ecdsa.recover(TEST_MESSAGE, signature)).should.equal(signer); + }); + }); + + context('with wrong version', function () { + it('returns 0', async function () { + // The last two hex digits are the signature version. + // The only valid values are 0, 1, 27 and 28. + const version = '02'; + const signature = signatureWithoutVersion + version; + (await this.ecdsa.recover(TEST_MESSAGE, signature)).should.equal( + '0x0000000000000000000000000000000000000000'); + }); + }); + }); + + context('using web3.eth.sign', function () { + context('with correct signature', function () { + it('returns signer address', async function () { + // Create the signature + const signature = signMessage(anyone, TEST_MESSAGE); + + // Recover the signer address from the generated message and signature. + (await this.ecdsa.recover( + toEthSignedMessageHash(TEST_MESSAGE), + signature + )).should.equal(anyone); + }); + }); + + context('with wrong signature', function () { + it('does not return signer address', async function () { + // Create the signature + const signature = signMessage(anyone, TEST_MESSAGE); + + // Recover the signer address from the generated message and wrong signature. + (await this.ecdsa.recover(WRONG_MESSAGE, signature)).should.not.equal(anyone); + }); + }); + }); }); - // @TODO - remove `skip` once we upgrade to solc^0.5 - it.skip('recover should revert when a small hash is sent', async function () { - // Create the signature - const signature = signMessage(anyone, TEST_MESSAGE); - await expectThrow( - this.mock.recover(TEST_MESSAGE.substring(2), signature) - ); + context('with small hash', function () { + // @TODO - remove `skip` once we upgrade to solc^0.5 + it.skip('reverts', async function () { + // Create the signature + const signature = signMessage(anyone, TEST_MESSAGE); + await expectThrow( + this.ecdsa.recover(TEST_MESSAGE.substring(2), signature) + ); + }); }); context('toEthSignedMessage', function () { it('should prefix hashes correctly', async function () { - (await this.mock.toEthSignedMessageHash(TEST_MESSAGE)).should.equal(toEthSignedMessageHash(TEST_MESSAGE)); + (await this.ecdsa.toEthSignedMessageHash(TEST_MESSAGE)).should.equal(toEthSignedMessageHash(TEST_MESSAGE)); }); }); }); diff --git a/test/library/MerkleProof.test.js b/test/library/MerkleProof.test.js index 6674880b742..b5d8fbe07e6 100644 --- a/test/library/MerkleProof.test.js +++ b/test/library/MerkleProof.test.js @@ -7,10 +7,8 @@ require('chai') .should(); contract('MerkleProof', function () { - let merkleProof; - beforeEach(async function () { - merkleProof = await MerkleProofWrapper.new(); + this.merkleProof = await MerkleProofWrapper.new(); }); describe('verify', function () { @@ -24,7 +22,7 @@ contract('MerkleProof', function () { const leaf = bufferToHex(sha3(elements[0])); - (await merkleProof.verify(proof, root, leaf)).should.equal(true); + (await this.merkleProof.verify(proof, root, leaf)).should.equal(true); }); it('should return false for an invalid Merkle proof', async function () { @@ -40,7 +38,7 @@ contract('MerkleProof', function () { const badProof = badMerkleTree.getHexProof(badElements[0]); - (await merkleProof.verify(badProof, correctRoot, correctLeaf)).should.equal(false); + (await this.merkleProof.verify(badProof, correctRoot, correctLeaf)).should.equal(false); }); it('should return false for a Merkle proof of invalid length', async function () { @@ -54,7 +52,7 @@ contract('MerkleProof', function () { const leaf = bufferToHex(sha3(elements[0])); - (await merkleProof.verify(badProof, root, leaf)).should.equal(false); + (await this.merkleProof.verify(badProof, root, leaf)).should.equal(false); }); }); }); diff --git a/test/payment/Escrow.behavior.js b/test/payment/Escrow.behavior.js index 761aecd890d..57f1fe5d0ac 100644 --- a/test/payment/Escrow.behavior.js +++ b/test/payment/Escrow.behavior.js @@ -31,10 +31,11 @@ function shouldBehaveLikeEscrow (primary, [payee1, payee2]) { }); it('emits a deposited event', async function () { - const receipt = await this.escrow.deposit(payee1, { from: primary, value: amount }); - - const event = expectEvent.inLogs(receipt.logs, 'Deposited', { payee: payee1 }); - event.args.weiAmount.should.be.bignumber.equal(amount); + const { logs } = await this.escrow.deposit(payee1, { from: primary, value: amount }); + expectEvent.inLogs(logs, 'Deposited', { + payee: payee1, + weiAmount: amount, + }); }); it('can add multiple deposits on a single account', async function () { @@ -83,10 +84,11 @@ function shouldBehaveLikeEscrow (primary, [payee1, payee2]) { it('emits a withdrawn event', async function () { await this.escrow.deposit(payee1, { from: primary, value: amount }); - const receipt = await this.escrow.withdraw(payee1, { from: primary }); - - const event = expectEvent.inLogs(receipt.logs, 'Withdrawn', { payee: payee1 }); - event.args.weiAmount.should.be.bignumber.equal(amount); + const { logs } = await this.escrow.withdraw(payee1, { from: primary }); + expectEvent.inLogs(logs, 'Withdrawn', { + payee: payee1, + weiAmount: amount, + }); }); }); }); diff --git a/test/payment/RefundEscrow.test.js b/test/payment/RefundEscrow.test.js index e412816c405..c499f4009dd 100644 --- a/test/payment/RefundEscrow.test.js +++ b/test/payment/RefundEscrow.test.js @@ -53,9 +53,8 @@ contract('RefundEscrow', function ([_, primary, beneficiary, refundee1, refundee it('only the primary account can enter closed state', async function () { await expectThrow(this.escrow.close({ from: beneficiary }), EVMRevert); - const receipt = await this.escrow.close({ from: primary }); - - expectEvent.inLogs(receipt.logs, 'Closed'); + const { logs } = await this.escrow.close({ from: primary }); + expectEvent.inLogs(logs, 'Closed'); }); context('closed state', function () { @@ -93,9 +92,8 @@ contract('RefundEscrow', function ([_, primary, beneficiary, refundee1, refundee it('only the primary account can enter refund state', async function () { await expectThrow(this.escrow.enableRefunds({ from: beneficiary }), EVMRevert); - const receipt = await this.escrow.enableRefunds({ from: primary }); - - expectEvent.inLogs(receipt.logs, 'RefundsEnabled'); + const { logs } = await this.escrow.enableRefunds({ from: primary }); + expectEvent.inLogs(logs, 'RefundsEnabled'); }); context('refund state', function () { diff --git a/test/payment/SplitPayment.test.js b/test/payment/SplitPayment.test.js index 05cc0b70100..cf9ef0e0184 100644 --- a/test/payment/SplitPayment.test.js +++ b/test/payment/SplitPayment.test.js @@ -1,4 +1,5 @@ -const { ethGetBalance, ethSendTransaction } = require('../helpers/web3'); +const { ethGetBalance } = require('../helpers/web3'); +const { sendEther } = require('./../helpers/sendTransaction'); const BigNumber = web3.BigNumber; @@ -58,7 +59,7 @@ contract('SplitPayment', function ([_, owner, payee1, payee2, payee3, nonpayee1, }); it('should accept payments', async function () { - await ethSendTransaction({ from: owner, to: this.contract.address, value: amount }); + await sendEther(owner, this.contract.address, amount); (await ethGetBalance(this.contract.address)).should.be.bignumber.equal(amount); }); @@ -76,12 +77,12 @@ contract('SplitPayment', function ([_, owner, payee1, payee2, payee3, nonpayee1, }); it('should throw if non-payee want to claim', async function () { - await ethSendTransaction({ from: payer1, to: this.contract.address, value: amount }); + await sendEther(payer1, this.contract.address, amount); await expectThrow(this.contract.release(nonpayee1), EVMRevert); }); it('should distribute funds to payees', async function () { - await ethSendTransaction({ from: payer1, to: this.contract.address, value: amount }); + await sendEther(payer1, this.contract.address, amount); // receive funds const initBalance = await ethGetBalance(this.contract.address); diff --git a/test/token/ERC20/ERC20.test.js b/test/token/ERC20/ERC20.test.js index 3662a41a795..e991c2ae6d1 100644 --- a/test/token/ERC20/ERC20.test.js +++ b/test/token/ERC20/ERC20.test.js @@ -62,12 +62,11 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { it('emits a transfer event', async function () { const { logs } = await this.token.transfer(to, amount, { from: owner }); - const event = expectEvent.inLogs(logs, 'Transfer', { + expectEvent.inLogs(logs, 'Transfer', { from: owner, to: to, + value: amount, }); - - event.args.value.should.be.bignumber.equal(amount); }); }); }); @@ -91,11 +90,11 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { it('emits an approval event', async function () { const { logs } = await this.token.approve(spender, amount, { from: owner }); - logs.length.should.equal(1); - logs[0].event.should.equal('Approval'); - logs[0].args.owner.should.equal(owner); - logs[0].args.spender.should.equal(spender); - logs[0].args.value.should.be.bignumber.equal(amount); + expectEvent.inLogs(logs, 'Approval', { + owner: owner, + spender: spender, + value: amount, + }); }); describe('when there was no approved amount before', function () { @@ -125,11 +124,11 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { it('emits an approval event', async function () { const { logs } = await this.token.approve(spender, amount, { from: owner }); - logs.length.should.equal(1); - logs[0].event.should.equal('Approval'); - logs[0].args.owner.should.equal(owner); - logs[0].args.spender.should.equal(spender); - logs[0].args.value.should.be.bignumber.equal(amount); + expectEvent.inLogs(logs, 'Approval', { + owner: owner, + spender: spender, + value: amount, + }); }); describe('when there was no approved amount before', function () { @@ -195,11 +194,11 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { it('emits a transfer event', async function () { const { logs } = await this.token.transferFrom(owner, to, amount, { from: spender }); - logs.length.should.equal(1); - logs[0].event.should.equal('Transfer'); - logs[0].args.from.should.equal(owner); - logs[0].args.to.should.equal(to); - logs[0].args.value.should.be.bignumber.equal(amount); + expectEvent.inLogs(logs, 'Transfer', { + from: owner, + to: to, + value: amount, + }); }); }); @@ -270,11 +269,11 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { it('emits an approval event', async function () { const { logs } = await this.token.decreaseAllowance(spender, approvedAmount, { from: owner }); - logs.length.should.equal(1); - logs[0].event.should.equal('Approval'); - logs[0].args.owner.should.equal(owner); - logs[0].args.spender.should.equal(spender); - logs[0].args.value.should.be.bignumber.equal(0); + expectEvent.inLogs(logs, 'Approval', { + owner: owner, + spender: spender, + value: 0, + }); }); it('decreases the spender allowance subtracting the requested amount', async function () { @@ -327,11 +326,11 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { it('emits an approval event', async function () { const { logs } = await this.token.increaseAllowance(spender, amount, { from: owner }); - logs.length.should.equal(1); - logs[0].event.should.equal('Approval'); - logs[0].args.owner.should.equal(owner); - logs[0].args.spender.should.equal(spender); - logs[0].args.value.should.be.bignumber.equal(amount); + expectEvent.inLogs(logs, 'Approval', { + owner: owner, + spender: spender, + value: amount, + }); }); describe('when there was no approved amount before', function () { @@ -361,11 +360,11 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { it('emits an approval event', async function () { const { logs } = await this.token.increaseAllowance(spender, amount, { from: owner }); - logs.length.should.equal(1); - logs[0].event.should.equal('Approval'); - logs[0].args.owner.should.equal(owner); - logs[0].args.spender.should.equal(spender); - logs[0].args.value.should.be.bignumber.equal(amount); + expectEvent.inLogs(logs, 'Approval', { + owner: owner, + spender: spender, + value: amount, + }); }); describe('when there was no approved amount before', function () { diff --git a/test/token/ERC20/ERC20Detailed.test.js b/test/token/ERC20/ERC20Detailed.test.js index 71b22a7b9a8..7965b7e9dc5 100644 --- a/test/token/ERC20/ERC20Detailed.test.js +++ b/test/token/ERC20/ERC20Detailed.test.js @@ -7,25 +7,23 @@ require('chai') const ERC20DetailedMock = artifacts.require('ERC20DetailedMock'); contract('ERC20Detailed', function () { - let detailedERC20 = null; - const _name = 'My Detailed ERC20'; const _symbol = 'MDT'; const _decimals = 18; beforeEach(async function () { - detailedERC20 = await ERC20DetailedMock.new(_name, _symbol, _decimals); + this.detailedERC20 = await ERC20DetailedMock.new(_name, _symbol, _decimals); }); it('has a name', async function () { - (await detailedERC20.name()).should.be.equal(_name); + (await this.detailedERC20.name()).should.be.equal(_name); }); it('has a symbol', async function () { - (await detailedERC20.symbol()).should.be.equal(_symbol); + (await this.detailedERC20.symbol()).should.be.equal(_symbol); }); it('has an amount of decimals', async function () { - (await detailedERC20.decimals()).should.be.bignumber.equal(_decimals); + (await this.detailedERC20.decimals()).should.be.bignumber.equal(_decimals); }); }); diff --git a/test/token/ERC20/ERC20Pausable.test.js b/test/token/ERC20/ERC20Pausable.test.js index fa733a3f61e..a524e2442ce 100644 --- a/test/token/ERC20/ERC20Pausable.test.js +++ b/test/token/ERC20/ERC20Pausable.test.js @@ -1,3 +1,4 @@ +const expectEvent = require('../../helpers/expectEvent'); const { assertRevert } = require('../../helpers/assertRevert'); const ERC20PausableMock = artifacts.require('ERC20PausableMock'); @@ -30,8 +31,7 @@ contract('ERC20Pausable', function ([_, pauser, otherPauser, recipient, anotherA it('emits a Pause event', async function () { const { logs } = await this.token.pause({ from }); - logs.length.should.equal(1); - logs[0].event.should.equal('Paused'); + expectEvent.inLogs(logs, 'Paused'); }); }); @@ -72,8 +72,7 @@ contract('ERC20Pausable', function ([_, pauser, otherPauser, recipient, anotherA it('emits an Unpause event', async function () { const { logs } = await this.token.unpause({ from }); - logs.length.should.equal(1); - logs[0].event.should.equal('Unpaused'); + expectEvent.inLogs(logs, 'Unpaused'); }); }); diff --git a/test/token/ERC20/ERC20Snapshot.test.js b/test/token/ERC20/ERC20Snapshot.test.js new file mode 100644 index 00000000000..65534723791 --- /dev/null +++ b/test/token/ERC20/ERC20Snapshot.test.js @@ -0,0 +1,136 @@ +const ERC20Snapshot = artifacts.require('ERC20SnapshotMock'); +const { expectThrow } = require('../../helpers/expectThrow'); +const { EVMRevert } = require('../../helpers/EVMRevert'); +const BigNumber = web3.BigNumber; + +require('chai') + .use(require('chai-bignumber')(BigNumber)) + .should(); + +contract('ERC20Snapshot', function ([_, owner, recipient, spender]) { + beforeEach(async function () { + this.token = await ERC20Snapshot.new(owner, 100); + }); + + context('there is no snapshots yet', function () { + describe('balanceOfAt', function () { + it('rejected for snapshotId parameter equals to 0', async function () { + await expectThrow( + this.token.balanceOfAt(owner, 0), + EVMRevert, + ); + }); + + it('rejected for snapshotId parameter greather than currentSnapshotId', async function () { + await expectThrow( + this.token.balanceOfAt(owner, 1), + EVMRevert, + ); + }); + }); + + describe('after transfer', function () { + beforeEach(async function () { + await this.token.transfer(recipient, 10, { from: owner }); + }); + + it('balanceOf returns correct value', async function () { + (await this.token.balanceOf(owner)).should.be.bignumber.equal(90); + (await this.token.balanceOf(recipient)).should.be.bignumber.equal(10); + }); + }); + }); + + context('snapshots exist', function () { + beforeEach(async function () { + await this.token.snapshot(); + }); + + describe('accounts do not have snapshots yet', function () { + it('snapshot value of the owner account equals to his balance', async function () { + const balanceOfAt = (await this.token.balanceOfAt(owner, 1)); + const balanceOf = (await this.token.balanceOf(owner)); + balanceOfAt.should.be.bignumber.equal(balanceOf); + }); + + it('snapshot value of the recipient account equals to balance', async function () { + const balanceOfAt = (await this.token.balanceOfAt(recipient, 1)); + const balanceOf = (await this.token.balanceOf(recipient)); + balanceOfAt.should.be.bignumber.equal(balanceOf); + }); + }); + + describe('accounts have already one snapshot', function () { + beforeEach(async function () { + await this.token.transfer(recipient, 10, { from: owner }); + }); + + it('snapshot keeps previous balance', async function () { + (await this.token.balanceOfAt(owner, 1)).should.be.bignumber.equal(100); + (await this.token.balanceOfAt(recipient, 1)).should.be.bignumber.equal(0); + }); + + it('account has correct balance', async function () { + (await this.token.balanceOf(owner)).should.be.bignumber.equal(90); + (await this.token.balanceOf(recipient)).should.be.bignumber.equal(10); + }); + }); + + describe('snapshot keeps previous balance even for multiple transfers', async function () { + beforeEach(async function () { + await this.token.transfer(recipient, 10, { from: owner }); + await this.token.transfer(recipient, 10, { from: owner }); + }); + + it('snapshot has previous balance', async function () { + (await this.token.balanceOfAt(owner, 1)).should.be.bignumber.equal(100); + (await this.token.balanceOfAt(recipient, 1)).should.be.bignumber.equal(0); + }); + + it('account has correct balance', async function () { + (await this.token.balanceOf(owner)).should.be.bignumber.equal(80); + (await this.token.balanceOf(recipient)).should.be.bignumber.equal(20); + }); + }); + + describe('snapshot keeps correct values for transfer from action', async function () { + beforeEach(async function () { + await this.token.approve(spender, 20, { from: owner }); + await this.token.transferFrom(owner, recipient, 20, { from: spender }); + }); + + it('spender and recipient snapshot is stored', async function () { + (await this.token.balanceOfAt(owner, 1)).should.be.bignumber.equal(100); + (await this.token.balanceOfAt(recipient, 1)).should.be.bignumber.equal(0); + }); + }); + + describe('new tokens are minted', function () { + beforeEach(async function () { + await this.token.mint(owner, 50); + }); + + it('snapshot keeps balance before mint', async function () { + (await this.token.balanceOfAt(owner, 1)).should.be.bignumber.equal(100); + }); + + it('current balance is greater after mint', async function () { + (await this.token.balanceOf(owner)).should.be.bignumber.equal(150); + }); + }); + + describe('chunk tokens are burned', function () { + beforeEach(async function () { + await this.token.burn(owner, 30); + }); + + it('snapshot keeps balance before burn', async function () { + (await this.token.balanceOfAt(owner, 1)).should.be.bignumber.equal(100); + }); + + it('crrent balance is lower after burn', async function () { + (await this.token.balanceOf(owner)).should.be.bignumber.equal(70); + }); + }); + }); +}); diff --git a/test/token/ERC20/TokenTimelock.test.js b/test/token/ERC20/TokenTimelock.test.js index c10b1b82137..04441fd1ebc 100644 --- a/test/token/ERC20/TokenTimelock.test.js +++ b/test/token/ERC20/TokenTimelock.test.js @@ -1,5 +1,4 @@ -const { latestTime } = require('../../helpers/latestTime'); -const { increaseTimeTo, duration } = require('../../helpers/increaseTime'); +const time = require('../../helpers/time'); const { expectThrow } = require('../../helpers/expectThrow'); const BigNumber = web3.BigNumber; @@ -20,7 +19,7 @@ contract('TokenTimelock', function ([_, minter, beneficiary]) { }); it('rejects a release time in the past', async function () { - const pastReleaseTime = (await latestTime()) - duration.years(1); + const pastReleaseTime = (await time.latest()) - time.duration.years(1); await expectThrow( TokenTimelock.new(this.token.address, beneficiary, pastReleaseTime) ); @@ -28,7 +27,7 @@ contract('TokenTimelock', function ([_, minter, beneficiary]) { context('once deployed', function () { beforeEach(async function () { - this.releaseTime = (await latestTime()) + duration.years(1); + this.releaseTime = (await time.latest()) + time.duration.years(1); this.timelock = await TokenTimelock.new(this.token.address, beneficiary, this.releaseTime); await this.token.mint(this.timelock.address, amount, { from: minter }); }); @@ -44,24 +43,24 @@ contract('TokenTimelock', function ([_, minter, beneficiary]) { }); it('cannot be released just before time limit', async function () { - await increaseTimeTo(this.releaseTime - duration.seconds(3)); + await time.increaseTo(this.releaseTime - time.duration.seconds(3)); await expectThrow(this.timelock.release()); }); it('can be released just after limit', async function () { - await increaseTimeTo(this.releaseTime + duration.seconds(1)); + await time.increaseTo(this.releaseTime + time.duration.seconds(1)); await this.timelock.release(); (await this.token.balanceOf(beneficiary)).should.be.bignumber.equal(amount); }); it('can be released after time limit', async function () { - await increaseTimeTo(this.releaseTime + duration.years(1)); + await time.increaseTo(this.releaseTime + time.duration.years(1)); await this.timelock.release(); (await this.token.balanceOf(beneficiary)).should.be.bignumber.equal(amount); }); it('cannot be released twice', async function () { - await increaseTimeTo(this.releaseTime + duration.years(1)); + await time.increaseTo(this.releaseTime + time.duration.years(1)); await this.timelock.release(); await expectThrow(this.timelock.release()); (await this.token.balanceOf(beneficiary)).should.be.bignumber.equal(amount); diff --git a/test/token/ERC20/TokenVesting.test.js b/test/token/ERC20/TokenVesting.test.js index 1fe9326f5fa..7c4fbcc8e36 100644 --- a/test/token/ERC20/TokenVesting.test.js +++ b/test/token/ERC20/TokenVesting.test.js @@ -1,7 +1,6 @@ const { expectThrow } = require('../../helpers/expectThrow'); const { EVMRevert } = require('../../helpers/EVMRevert'); -const { latestTime } = require('../../helpers/latestTime'); -const { increaseTimeTo, duration } = require('../../helpers/increaseTime'); +const time = require('../../helpers/time'); const { ethGetBlock } = require('../../helpers/web3'); const BigNumber = web3.BigNumber; @@ -18,9 +17,10 @@ contract('TokenVesting', function ([_, owner, beneficiary]) { const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000'; beforeEach(async function () { - this.start = (await latestTime()) + duration.minutes(1); // +1 minute so it starts after contract instantiation - this.cliffDuration = duration.years(1); - this.duration = duration.years(2); + // +1 minute so it starts after contract instantiation + this.start = (await time.latest()) + time.duration.minutes(1); + this.cliffDuration = time.duration.years(1); + this.duration = time.duration.years(2); }); it('rejects a duration shorter than the cliff', async function () { @@ -65,12 +65,12 @@ contract('TokenVesting', function ([_, owner, beneficiary]) { }); it('can be released after cliff', async function () { - await increaseTimeTo(this.start + this.cliffDuration + duration.weeks(1)); + await time.increaseTo(this.start + this.cliffDuration + time.duration.weeks(1)); await this.vesting.release(this.token.address); }); it('should release proper amount after cliff', async function () { - await increaseTimeTo(this.start + this.cliffDuration); + await time.increaseTo(this.start + this.cliffDuration); const { receipt } = await this.vesting.release(this.token.address); const block = await ethGetBlock(receipt.blockNumber); @@ -87,7 +87,7 @@ contract('TokenVesting', function ([_, owner, beneficiary]) { for (let i = 1; i <= checkpoints; i++) { const now = this.start + this.cliffDuration + i * (vestingPeriod / checkpoints); - await increaseTimeTo(now); + await time.increaseTo(now); await this.vesting.release(this.token.address); const expectedVesting = amount.mul(now - this.start).div(this.duration).floor(); @@ -97,7 +97,7 @@ contract('TokenVesting', function ([_, owner, beneficiary]) { }); it('should have released all after end', async function () { - await increaseTimeTo(this.start + this.duration); + await time.increaseTo(this.start + this.duration); await this.vesting.release(this.token.address); (await this.token.balanceOf(beneficiary)).should.bignumber.equal(amount); (await this.vesting.released(this.token.address)).should.bignumber.equal(amount); @@ -120,7 +120,7 @@ contract('TokenVesting', function ([_, owner, beneficiary]) { }); it('should return the non-vested tokens when revoked by owner', async function () { - await increaseTimeTo(this.start + this.cliffDuration + duration.weeks(12)); + await time.increaseTo(this.start + this.cliffDuration + time.duration.weeks(12)); const vested = await this.vesting.vestedAmount(this.token.address); @@ -130,7 +130,7 @@ contract('TokenVesting', function ([_, owner, beneficiary]) { }); it('should keep the vested tokens when revoked by owner', async function () { - await increaseTimeTo(this.start + this.cliffDuration + duration.weeks(12)); + await time.increaseTo(this.start + this.cliffDuration + time.duration.weeks(12)); const vestedPre = await this.vesting.vestedAmount(this.token.address); @@ -142,7 +142,7 @@ contract('TokenVesting', function ([_, owner, beneficiary]) { }); it('should fail to be revoked a second time', async function () { - await increaseTimeTo(this.start + this.cliffDuration + duration.weeks(12)); + await time.increaseTo(this.start + this.cliffDuration + time.duration.weeks(12)); await this.vesting.vestedAmount(this.token.address); diff --git a/test/token/ERC20/behaviors/ERC20Burnable.behavior.js b/test/token/ERC20/behaviors/ERC20Burnable.behavior.js index cf0190046f3..c52594d1876 100644 --- a/test/token/ERC20/behaviors/ERC20Burnable.behavior.js +++ b/test/token/ERC20/behaviors/ERC20Burnable.behavior.js @@ -29,10 +29,11 @@ function shouldBehaveLikeERC20Burnable (owner, initialBalance, [burner]) { }); it('emits a transfer event', async function () { - const event = expectEvent.inLogs(this.logs, 'Transfer'); - event.args.from.should.equal(owner); - event.args.to.should.equal(ZERO_ADDRESS); - event.args.value.should.be.bignumber.equal(amount); + expectEvent.inLogs(this.logs, 'Transfer', { + from: owner, + to: ZERO_ADDRESS, + value: amount, + }); }); } }); @@ -74,10 +75,11 @@ function shouldBehaveLikeERC20Burnable (owner, initialBalance, [burner]) { }); it('emits a transfer event', async function () { - const event = expectEvent.inLogs(this.logs, 'Transfer'); - event.args.from.should.equal(owner); - event.args.to.should.equal(ZERO_ADDRESS); - event.args.value.should.be.bignumber.equal(amount); + expectEvent.inLogs(this.logs, 'Transfer', { + from: owner, + to: ZERO_ADDRESS, + value: amount, + }); }); } }); diff --git a/test/token/ERC20/behaviors/ERC20Mintable.behavior.js b/test/token/ERC20/behaviors/ERC20Mintable.behavior.js index 5046fc62e50..dbda9eeedbc 100644 --- a/test/token/ERC20/behaviors/ERC20Mintable.behavior.js +++ b/test/token/ERC20/behaviors/ERC20Mintable.behavior.js @@ -11,136 +11,44 @@ function shouldBehaveLikeERC20Mintable (minter, [anyone]) { const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000'; describe('as a mintable token', function () { - describe('mintingFinished', function () { - context('when token minting is not finished', function () { - it('returns false', async function () { - (await this.token.mintingFinished()).should.equal(false); - }); - }); - - context('when token minting is finished', function () { - beforeEach(async function () { - await this.token.finishMinting({ from: minter }); - }); - - it('returns true', async function () { - (await this.token.mintingFinished()).should.equal(true); - }); - }); - }); + describe('mint', function () { + const amount = 100; - describe('finishMinting', function () { context('when the sender has minting permission', function () { const from = minter; - context('when token minting was not finished', function () { - it('finishes token minting', async function () { - await this.token.finishMinting({ from }); - - (await this.token.mintingFinished()).should.equal(true); - }); - - it('emits a mint finished event', async function () { - const { logs } = await this.token.finishMinting({ from }); - - await expectEvent.inLogs(logs, 'MintingFinished'); - }); - }); - - context('when token minting was already finished', function () { - beforeEach(async function () { - await this.token.finishMinting({ from }); - }); - - it('reverts', async function () { - await assertRevert(this.token.finishMinting({ from })); - }); + context('for a zero amount', function () { + shouldMint(0); }); - }); - - context('when the sender doesn\'t have minting permission', function () { - const from = anyone; - context('when token minting was not finished', function () { - it('reverts', async function () { - await assertRevert(this.token.finishMinting({ from })); - }); + context('for a non-zero amount', function () { + shouldMint(amount); }); - context('when token minting was already finished', function () { + function shouldMint (amount) { beforeEach(async function () { - await this.token.finishMinting({ from: minter }); - }); - - it('reverts', async function () { - await assertRevert(this.token.finishMinting({ from })); - }); - }); - }); - }); - - describe('mint', function () { - const amount = 100; - - context('when the sender has minting permission', function () { - const from = minter; - - context('when token minting is not finished', function () { - context('for a zero amount', function () { - shouldMint(0); + ({ logs: this.logs } = await this.token.mint(anyone, amount, { from })); }); - context('for a non-zero amount', function () { - shouldMint(amount); + it('mints the requested amount', async function () { + (await this.token.balanceOf(anyone)).should.be.bignumber.equal(amount); }); - function shouldMint (amount) { - beforeEach(async function () { - ({ logs: this.logs } = await this.token.mint(anyone, amount, { from })); - }); - - it('mints the requested amount', async function () { - (await this.token.balanceOf(anyone)).should.be.bignumber.equal(amount); - }); - - it('emits a mint and a transfer event', async function () { - const transferEvent = expectEvent.inLogs(this.logs, 'Transfer', { - from: ZERO_ADDRESS, - to: anyone, - }); - transferEvent.args.value.should.be.bignumber.equal(amount); + it('emits a mint and a transfer event', async function () { + expectEvent.inLogs(this.logs, 'Transfer', { + from: ZERO_ADDRESS, + to: anyone, + value: amount, }); - } - }); - - context('when token minting is finished', function () { - beforeEach(async function () { - await this.token.finishMinting({ from: minter }); }); - - it('reverts', async function () { - await assertRevert(this.token.mint(anyone, amount, { from })); - }); - }); + } }); context('when the sender doesn\'t have minting permission', function () { const from = anyone; - context('when token minting is not finished', function () { - it('reverts', async function () { - await assertRevert(this.token.mint(anyone, amount, { from })); - }); - }); - - context('when token minting is already finished', function () { - beforeEach(async function () { - await this.token.finishMinting({ from: minter }); - }); - - it('reverts', async function () { - await assertRevert(this.token.mint(anyone, amount, { from })); - }); + it('reverts', async function () { + await assertRevert(this.token.mint(anyone, amount, { from })); }); }); }); diff --git a/test/token/ERC721/ERC721.behavior.js b/test/token/ERC721/ERC721.behavior.js index ecdc3352f3b..ce56747def3 100644 --- a/test/token/ERC721/ERC721.behavior.js +++ b/test/token/ERC721/ERC721.behavior.js @@ -1,3 +1,4 @@ +const expectEvent = require('../../helpers/expectEvent'); const { shouldSupportInterfaces } = require('../../introspection/SupportsInterface.behavior'); const { assertRevert } = require('../../helpers/assertRevert'); const { decodeLogs } = require('../../helpers/decodeLogs'); @@ -88,19 +89,19 @@ function shouldBehaveLikeERC721 ( if (approved) { it('emit only a transfer event', async function () { - logs.length.should.be.equal(1); - logs[0].event.should.be.equal('Transfer'); - logs[0].args.from.should.be.equal(owner); - logs[0].args.to.should.be.equal(this.toWhom); - logs[0].args.tokenId.should.be.bignumber.equal(tokenId); + expectEvent.inLogs(logs, 'Transfer', { + from: owner, + to: this.toWhom, + tokenId: tokenId, + }); }); } else { it('emits only a transfer event', async function () { - logs.length.should.be.equal(1); - logs[0].event.should.be.equal('Transfer'); - logs[0].args.from.should.be.equal(owner); - logs[0].args.to.should.be.equal(this.toWhom); - logs[0].args.tokenId.should.be.bignumber.equal(tokenId); + expectEvent.inLogs(logs, 'Transfer', { + from: owner, + to: this.toWhom, + tokenId: tokenId, + }); }); } @@ -161,11 +162,11 @@ function shouldBehaveLikeERC721 ( }); it('emits only a transfer event', async function () { - logs.length.should.be.equal(1); - logs[0].event.should.be.equal('Transfer'); - logs[0].args.from.should.be.equal(owner); - logs[0].args.to.should.be.equal(owner); - logs[0].args.tokenId.should.be.bignumber.equal(tokenId); + expectEvent.inLogs(logs, 'Transfer', { + from: owner, + to: owner, + tokenId: tokenId, + }); }); it('keeps the owner balance', async function () { @@ -337,11 +338,11 @@ function shouldBehaveLikeERC721 ( const itEmitsApprovalEvent = function (address) { it('emits an approval event', async function () { - logs.length.should.be.equal(1); - logs[0].event.should.be.equal('Approval'); - logs[0].args.owner.should.be.equal(owner); - logs[0].args.approved.should.be.equal(address); - logs[0].args.tokenId.should.be.bignumber.equal(tokenId); + expectEvent.inLogs(logs, 'Approval', { + owner: owner, + approved: address, + tokenId: tokenId, + }); }); }; @@ -447,11 +448,11 @@ function shouldBehaveLikeERC721 ( it('emits an approval event', async function () { const { logs } = await this.token.setApprovalForAll(operator, true, { from: owner }); - logs.length.should.be.equal(1); - logs[0].event.should.be.equal('ApprovalForAll'); - logs[0].args.owner.should.be.equal(owner); - logs[0].args.operator.should.be.equal(operator); - logs[0].args.approved.should.equal(true); + expectEvent.inLogs(logs, 'ApprovalForAll', { + owner: owner, + operator: operator, + approved: true, + }); }); }); @@ -469,11 +470,11 @@ function shouldBehaveLikeERC721 ( it('emits an approval event', async function () { const { logs } = await this.token.setApprovalForAll(operator, true, { from: owner }); - logs.length.should.be.equal(1); - logs[0].event.should.be.equal('ApprovalForAll'); - logs[0].args.owner.should.be.equal(owner); - logs[0].args.operator.should.be.equal(operator); - logs[0].args.approved.should.equal(true); + expectEvent.inLogs(logs, 'ApprovalForAll', { + owner: owner, + operator: operator, + approved: true, + }); }); it('can unset the operator approval', async function () { @@ -497,11 +498,11 @@ function shouldBehaveLikeERC721 ( it('emits an approval event', async function () { const { logs } = await this.token.setApprovalForAll(operator, true, { from: owner }); - logs.length.should.be.equal(1); - logs[0].event.should.be.equal('ApprovalForAll'); - logs[0].args.owner.should.be.equal(owner); - logs[0].args.operator.should.be.equal(operator); - logs[0].args.approved.should.equal(true); + expectEvent.inLogs(logs, 'ApprovalForAll', { + owner: owner, + operator: operator, + approved: true, + }); }); }); }); diff --git a/test/token/ERC721/ERC721Full.test.js b/test/token/ERC721/ERC721Full.test.js index f4666d0d9b0..30e4547c82a 100644 --- a/test/token/ERC721/ERC721Full.test.js +++ b/test/token/ERC721/ERC721Full.test.js @@ -1,6 +1,5 @@ const { assertRevert } = require('../../helpers/assertRevert'); const { shouldBehaveLikeERC721 } = require('./ERC721.behavior'); -const { shouldBehaveLikeMintAndBurnERC721 } = require('./ERC721MintBurn.behavior'); const { shouldSupportInterfaces } = require('../../introspection/SupportsInterface.behavior'); const BigNumber = web3.BigNumber; @@ -221,7 +220,6 @@ contract('ERC721Full', function ([ }); shouldBehaveLikeERC721(creator, minter, accounts); - shouldBehaveLikeMintAndBurnERC721(creator, minter, accounts); shouldSupportInterfaces([ 'ERC165', diff --git a/test/token/ERC721/ERC721Holder.test.js b/test/token/ERC721/ERC721Holder.test.js new file mode 100644 index 00000000000..9f6eb856a13 --- /dev/null +++ b/test/token/ERC721/ERC721Holder.test.js @@ -0,0 +1,19 @@ +const ERC721Holder = artifacts.require('ERC721Holder.sol'); +const ERC721Mintable = artifacts.require('ERC721MintableBurnableImpl.sol'); + +require('chai') + .should(); + +contract('ERC721Holder', function ([creator]) { + it('receives an ERC721 token', async function () { + const token = await ERC721Mintable.new({ from: creator }); + const tokenId = 1; + await token.mint(creator, tokenId, { from: creator }); + + const receiver = await ERC721Holder.new(); + await token.approve(receiver.address, tokenId, { from: creator }); + await token.safeTransferFrom(creator, receiver.address, tokenId); + + (await token.ownerOf(tokenId)).should.be.equal(receiver.address); + }); +}); diff --git a/test/token/ERC721/ERC721MintBurn.behavior.js b/test/token/ERC721/ERC721MintBurn.behavior.js index cadfe87cc38..bcf7e9ed303 100644 --- a/test/token/ERC721/ERC721MintBurn.behavior.js +++ b/test/token/ERC721/ERC721MintBurn.behavior.js @@ -42,23 +42,23 @@ function shouldBehaveLikeMintAndBurnERC721 ( }); it('emits a transfer and minted event', async function () { - await expectEvent.inLogs(logs, 'Transfer', { + expectEvent.inLogs(logs, 'Transfer', { from: ZERO_ADDRESS, to: newOwner, + tokenId: thirdTokenId, }); - logs[0].args.tokenId.should.be.bignumber.equal(thirdTokenId); }); }); describe('when the given owner address is the zero address', function () { it('reverts', async function () { - await assertRevert(this.token.mint(ZERO_ADDRESS, thirdTokenId)); + await assertRevert(this.token.mint(ZERO_ADDRESS, thirdTokenId, { from: minter })); }); }); describe('when the given token ID was already tracked by this contract', function () { it('reverts', async function () { - await assertRevert(this.token.mint(owner, firstTokenId)); + await assertRevert(this.token.mint(owner, firstTokenId, { from: minter })); }); }); }); @@ -87,11 +87,11 @@ function shouldBehaveLikeMintAndBurnERC721 ( }); it('emits a burn event', async function () { - logs.length.should.be.equal(1); - logs[0].event.should.be.equal('Transfer'); - logs[0].args.from.should.be.equal(owner); - logs[0].args.to.should.be.equal(ZERO_ADDRESS); - logs[0].args.tokenId.should.be.bignumber.equal(tokenId); + expectEvent.inLogs(logs, 'Transfer', { + from: owner, + to: ZERO_ADDRESS, + tokenId: tokenId, + }); }); }); @@ -117,35 +117,6 @@ function shouldBehaveLikeMintAndBurnERC721 ( }); }); }); - - describe('finishMinting', function () { - it('allows the minter to finish minting', async function () { - const { logs } = await this.token.finishMinting({ from: minter }); - expectEvent.inLogs(logs, 'MintingFinished'); - }); - }); - - context('mintingFinished', function () { - beforeEach(async function () { - await this.token.finishMinting({ from: minter }); - }); - - describe('mint', function () { - it('reverts', async function () { - await assertRevert( - this.token.mint(owner, thirdTokenId, { from: minter }) - ); - }); - }); - - describe('mintWithTokenURI', function () { - it('reverts', async function () { - await assertRevert( - this.token.mintWithTokenURI(owner, thirdTokenId, MOCK_URI, { from: minter }) - ); - }); - }); - }); }); } diff --git a/test/Address.test.js b/test/utils/Address.test.js similarity index 100% rename from test/Address.test.js rename to test/utils/Address.test.js diff --git a/test/utils/Arrays.test.js b/test/utils/Arrays.test.js new file mode 100644 index 00000000000..4bfc9eabacc --- /dev/null +++ b/test/utils/Arrays.test.js @@ -0,0 +1,87 @@ +const ArraysImpl = artifacts.require('ArraysImpl'); + +const BigNumber = web3.BigNumber; + +require('chai') + .use(require('chai-bignumber')(BigNumber)) + .should(); + +contract('Arrays', function () { + context('Even number of elements', function () { + const EVEN_ELEMENTS_ARRAY = [11, 12, 13, 14, 15, 16, 17, 18, 19, 20]; + + beforeEach(async function () { + this.arrays = await ArraysImpl.new(EVEN_ELEMENTS_ARRAY); + }); + + it('should return correct index for the basic case', async function () { + (await this.arrays.findUpperBound(16)).should.be.bignumber.equal(5); + }); + + it('should return 0 for the first element', async function () { + (await this.arrays.findUpperBound(11)).should.be.bignumber.equal(0); + }); + + it('should return index of the last element', async function () { + (await this.arrays.findUpperBound(20)).should.be.bignumber.equal(9); + }); + + it('should return first index after last element if searched value is over the upper boundary', async function () { + (await this.arrays.findUpperBound(32)).should.be.bignumber.equal(10); + }); + + it('should return 0 for the element under the lower boundary', async function () { + (await this.arrays.findUpperBound(2)).should.be.bignumber.equal(0); + }); + }); + + context('Odd number of elements', function () { + const ODD_ELEMENTS_ARRAY = [11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21]; + + beforeEach(async function () { + this.arrays = await ArraysImpl.new(ODD_ELEMENTS_ARRAY); + }); + + it('should return correct index for the basic case', async function () { + (await this.arrays.findUpperBound(16)).should.be.bignumber.equal(5); + }); + + it('should return 0 for the first element', async function () { + (await this.arrays.findUpperBound(11)).should.be.bignumber.equal(0); + }); + + it('should return index of the last element', async function () { + (await this.arrays.findUpperBound(21)).should.be.bignumber.equal(10); + }); + + it('should return first index after last element if searched value is over the upper boundary', async function () { + (await this.arrays.findUpperBound(32)).should.be.bignumber.equal(11); + }); + + it('should return 0 for the element under the lower boundary', async function () { + (await this.arrays.findUpperBound(2)).should.be.bignumber.equal(0); + }); + }); + + context('Array with gap', function () { + const WITH_GAP_ARRAY = [11, 12, 13, 14, 15, 20, 21, 22, 23, 24]; + + beforeEach(async function () { + this.arrays = await ArraysImpl.new(WITH_GAP_ARRAY); + }); + + it('should return index of first element in next filled range', async function () { + (await this.arrays.findUpperBound(17)).should.be.bignumber.equal(5); + }); + }); + + context('Empty array', function () { + beforeEach(async function () { + this.arrays = await ArraysImpl.new([]); + }); + + it('should always return 0 for empty array', async function () { + (await this.arrays.findUpperBound(10)).should.be.bignumber.equal(0); + }); + }); +}); diff --git a/test/ReentrancyGuard.test.js b/test/utils/ReentrancyGuard.test.js similarity index 63% rename from test/ReentrancyGuard.test.js rename to test/utils/ReentrancyGuard.test.js index 7086f8d94f7..3952b2fcbc0 100644 --- a/test/ReentrancyGuard.test.js +++ b/test/utils/ReentrancyGuard.test.js @@ -1,4 +1,4 @@ -const { expectThrow } = require('./helpers/expectThrow'); +const { expectThrow } = require('../helpers/expectThrow'); const ReentrancyMock = artifacts.require('ReentrancyMock'); const ReentrancyAttack = artifacts.require('ReentrancyAttack'); @@ -9,17 +9,14 @@ require('chai') .should(); contract('ReentrancyGuard', function () { - let reentrancyMock; - beforeEach(async function () { - reentrancyMock = await ReentrancyMock.new(); - const initialCounter = await reentrancyMock.counter(); - initialCounter.should.be.bignumber.equal(0); + this.reentrancyMock = await ReentrancyMock.new(); + (await this.reentrancyMock.counter()).should.be.bignumber.equal(0); }); it('should not allow remote callback', async function () { const attacker = await ReentrancyAttack.new(); - await expectThrow(reentrancyMock.countAndCall(attacker.address)); + await expectThrow(this.reentrancyMock.countAndCall(attacker.address)); }); // The following are more side-effects than intended behavior: @@ -27,10 +24,10 @@ contract('ReentrancyGuard', function () { // in the side-effects. it('should not allow local recursion', async function () { - await expectThrow(reentrancyMock.countLocalRecursive(10)); + await expectThrow(this.reentrancyMock.countLocalRecursive(10)); }); it('should not allow indirect local recursion', async function () { - await expectThrow(reentrancyMock.countThisRecursive(10)); + await expectThrow(this.reentrancyMock.countThisRecursive(10)); }); });