From aa4c9feabd25a9ffb42d0bf782ceac59aab24017 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Wed, 8 May 2019 13:13:19 -0300 Subject: [PATCH] Add ERC20 compatibility to ERC777. (#1735) * Add ERC20 compatibility. * Reusing ERC20 tests for ERC777. * Improve documentation. * Add changelog entry. * Improved ERC20 behavior tests. * Add revert reasons to ERC777. * ERC20 methods allow sending tokens to contracts with no interface. * Register ERC20 interface. * Add comment about avoidLockingTokens. * Improve revert reason string. * Make ERC777 implement IERC20. * Fix test revert string. * Remove unnecesary require. * Add private _transfer. * Update contracts/drafts/ERC777/ERC777.sol Co-Authored-By: nventuro * Update private helper names. --- CHANGELOG.md | 2 +- contracts/drafts/ERC777/ERC777.sol | 172 ++++++++++++++--- contracts/token/ERC20/ERC20.sol | 18 +- test/drafts/ERC777/ERC777.behavior.js | 29 ++- test/drafts/ERC777/ERC777.test.js | 59 ++++-- test/token/ERC20/ERC20.behavior.js | 268 ++++++++++++++++++++++++++ test/token/ERC20/ERC20.test.js | 265 +------------------------ 7 files changed, 499 insertions(+), 314 deletions(-) create mode 100644 test/token/ERC20/ERC20.behavior.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 61e9ac41b0f..2eb69798846 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ ### New features: * `ERC1820`: added support for interacting with the [ERC1820](https://eips.ethereum.org/EIPS/eip-1820) registry contract (`IERC1820Registry`), as well as base contracts that can be registered as implementers there. ([#1677](https://github.com/OpenZeppelin/openzeppelin-solidity/pull/1677)) - * `ERC777`: initial support for the [ERC777 token](https://eips.ethereum.org/EIPS/eip-777), which has multiple improvements over `ERC20` such as built-in burning, a more straightforward permission system, and optional sender and receiver hooks on transfer (mandatory for contracts!). ([#1684](https://github.com/OpenZeppelin/openzeppelin-solidity/pull/1684)) + * `ERC777`: support for the [ERC777 token](https://eips.ethereum.org/EIPS/eip-777), which has multiple improvements over `ERC20` (but is backwards compatible with it) such as built-in burning, a more straightforward permission system, and optional sender and receiver hooks on transfer (mandatory for contracts!). ([#1684](https://github.com/OpenZeppelin/openzeppelin-solidity/pull/1684)) * All contracts now have revert reason strings, which give insight into error conditions, and help debug failing transactions. ([#1704](https://github.com/OpenZeppelin/openzeppelin-solidity/pull/1704)) ### Improvements: diff --git a/contracts/drafts/ERC777/ERC777.sol b/contracts/drafts/ERC777/ERC777.sol index ab2e53dae5e..1810aa2d98f 100644 --- a/contracts/drafts/ERC777/ERC777.sol +++ b/contracts/drafts/ERC777/ERC777.sol @@ -3,6 +3,7 @@ pragma solidity ^0.5.0; import "./IERC777.sol"; import "./IERC777Recipient.sol"; import "./IERC777Sender.sol"; +import "../../token/ERC20/IERC20.sol"; import "../../math/SafeMath.sol"; import "../../utils/Address.sol"; import "../IERC1820Registry.sol"; @@ -11,20 +12,19 @@ import "../IERC1820Registry.sol"; * @title ERC777 token implementation, with granularity harcoded to 1. * @author etsvigun , Bertrand Masius */ -contract ERC777 is IERC777 { +contract ERC777 is IERC777, IERC20 { using SafeMath for uint256; using Address for address; IERC1820Registry private _erc1820 = IERC1820Registry(0x1820a4B7618BdE71Dce8cdc73aAB6C95905faD24); - string private _name; - - string private _symbol; - mapping(address => uint256) private _balances; uint256 private _totalSupply; + string private _name; + string private _symbol; + bytes32 constant private TOKENS_SENDER_INTERFACE_HASH = keccak256("ERC777TokensSender"); bytes32 constant private TOKENS_RECIPIENT_INTERFACE_HASH = keccak256("ERC777TokensRecipient"); @@ -38,6 +38,9 @@ contract ERC777 is IERC777 { mapping(address => mapping(address => bool)) private _operators; mapping(address => mapping(address => bool)) private _revokedDefaultOperators; + // ERC20-allowances + mapping (address => mapping (address => uint256)) private _allowances; + constructor( string memory name, string memory symbol, @@ -51,8 +54,9 @@ contract ERC777 is IERC777 { _defaultOperators[_defaultOperatorsArray[i]] = true; } - // register interface + // register interfaces _erc1820.setInterfaceImplementer(address(this), keccak256("ERC777Token"), address(this)); + _erc1820.setInterfaceImplementer(address(this), keccak256("ERC20Token"), address(this)); } /** @@ -62,7 +66,7 @@ contract ERC777 is IERC777 { * @param data bytes information attached to the send, and intended for the recipient (to) */ function send(address to, uint256 amount, bytes calldata data) external { - _send(msg.sender, msg.sender, to, amount, data, ""); + _sendRequiringReceptionAck(msg.sender, msg.sender, to, amount, data, ""); } /** @@ -82,8 +86,36 @@ contract ERC777 is IERC777 { ) external { - require(isOperatorFor(msg.sender, from)); - _send(msg.sender, from, to, amount, data, operatorData); + require(isOperatorFor(msg.sender, from), "ERC777: caller is not an operator for holder"); + _sendRequiringReceptionAck(msg.sender, from, to, amount, data, operatorData); + } + + /** + * @dev Transfer token to a specified address. + * Required for ERC20 compatiblity. Note that transferring tokens this way may result in locked tokens (i.e. tokens + * can be sent to a contract that does not implement the ERC777TokensRecipient interface). + * @param to The address to transfer to. + * @param value The amount to be transferred. + */ + function transfer(address to, uint256 value) external returns (bool) { + _transfer(msg.sender, msg.sender, to, value); + return true; + } + + /** + * @dev Transfer tokens from one address to another. + * Note that while this function emits an Approval event, this is not required as per the specification, + * and other compliant implementations may not emit the event. + * Required for ERC20 compatiblity. Note that transferring tokens this way may result in locked tokens (i.e. tokens + * can be sent to a contract that does not implement the ERC777TokensRecipient interface). + * @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) external returns (bool) { + _transfer(msg.sender, from, to, value); + _approve(from, msg.sender, _allowances[from][msg.sender].sub(value)); + return true; } /** @@ -103,7 +135,7 @@ contract ERC777 is IERC777 { * @param operatorData bytes extra information provided by the operator (if any) */ function operatorBurn(address from, uint256 amount, bytes calldata data, bytes calldata operatorData) external { - require(isOperatorFor(msg.sender, from)); + require(isOperatorFor(msg.sender, from), "ERC777: caller is not an operator for holder"); _burn(msg.sender, from, amount, data, operatorData); } @@ -112,7 +144,7 @@ contract ERC777 is IERC777 { * @param operator address to be authorized as operator */ function authorizeOperator(address operator) external { - require(msg.sender != operator); + require(msg.sender != operator, "ERC777: authorizing self as operator"); if (_defaultOperators[operator]) { delete _revokedDefaultOperators[msg.sender][operator]; @@ -128,7 +160,7 @@ contract ERC777 is IERC777 { * @param operator address to revoke operator rights from */ function revokeOperator(address operator) external { - require(operator != msg.sender); + require(operator != msg.sender, "ERC777: revoking self as operator"); if (_defaultOperators[operator]) { _revokedDefaultOperators[msg.sender][operator] = true; @@ -140,17 +172,18 @@ contract ERC777 is IERC777 { } /** - * @return the name of the token. - */ - function name() public view returns (string memory) { - return _name; - } - - /** - * @return the symbol of the token. + * @dev Approve the passed address to spend the specified amount of tokens on behalf of msg.sender. + * Beware that changing an allowance with this method brings the risk that someone may use both the old + * and the new allowance by unfortunate transaction ordering. One possible solution to mitigate this + * race condition is to first reduce the spender's allowance to 0 and set the desired value afterwards: + * https://github.com/ethereum/EIPs/issues/20#issuecomment-263524729 + * Required for ERC20 compatilibity. + * @param spender The address which will spend the funds. + * @param value The amount of tokens to be spent. */ - function symbol() public view returns (string memory) { - return _symbol; + function approve(address spender, uint256 value) external returns (bool) { + _approve(msg.sender, spender, value); + return true; } /** @@ -169,6 +202,27 @@ contract ERC777 is IERC777 { return _balances[tokenHolder]; } + /** + * @return the name of the token. + */ + function name() public view returns (string memory) { + return _name; + } + + /** + * @return the symbol of the token. + */ + function symbol() public view returns (string memory) { + return _symbol; + } + + /** + * @return the number of decimals of the token. + */ + function decimals() public pure returns (uint8) { + return 18; // The spec requires that decimals be 18 + } + /** * @dev Gets the token's granularity, * i.e. the smallest number of tokens (in the basic unit) @@ -203,6 +257,17 @@ contract ERC777 is IERC777 { _operators[tokenHolder][operator]; } + /** + * @dev Function to check the amount of tokens that an owner allowed to a spender. + * Required for ERC20 compatibility. + * @param owner address The address which owns the funds. + * @param spender address The address which will spend the funds. + * @return A uint256 specifying the amount of tokens still available for the spender. + */ + function allowance(address owner, address spender) public view returns (uint256) { + return _allowances[owner][spender]; + } + /** * @dev Mint tokens. Does not check authorization of operator * @dev the caller may ckeck that operator is authorized before calling @@ -221,15 +286,42 @@ contract ERC777 is IERC777 { ) internal { - require(to != address(0)); + require(to != address(0), "ERC777: mint to the zero address"); // Update state variables _totalSupply = _totalSupply.add(amount); _balances[to] = _balances[to].add(amount); - _callTokensReceived(operator, address(0), to, amount, userData, operatorData); + _callTokensReceived(operator, address(0), to, amount, userData, operatorData, true); emit Minted(operator, to, amount, userData, operatorData); + emit Transfer(address(0), to, amount); + } + + function _transfer(address operator, address from, address to, uint256 amount) private { + _sendAllowingNoReceptionAck(operator, from, to, amount, "", ""); + } + + function _sendRequiringReceptionAck( + address operator, + address from, + address to, + uint256 amount, + bytes memory userData, + bytes memory operatorData + ) private { + _send(operator, from, to, amount, userData, operatorData, true); + } + + function _sendAllowingNoReceptionAck( + address operator, + address from, + address to, + uint256 amount, + bytes memory userData, + bytes memory operatorData + ) private { + _send(operator, from, to, amount, userData, operatorData, false); } /** @@ -240,6 +332,7 @@ contract ERC777 is IERC777 { * @param amount uint256 amount of tokens to transfer * @param userData bytes extra information provided by the token holder (if any) * @param operatorData bytes extra information provided by the operator (if any) + * @param requireReceptionAck if true, contract recipients are required to implement ERC777TokensRecipient */ function _send( address operator, @@ -247,12 +340,13 @@ contract ERC777 is IERC777 { address to, uint256 amount, bytes memory userData, - bytes memory operatorData + bytes memory operatorData, + bool requireReceptionAck ) private { - require(from != address(0)); - require(to != address(0)); + require(from != address(0), "ERC777: transfer from the zero address"); + require(to != address(0), "ERC777: transfer to the zero address"); _callTokensToSend(operator, from, to, amount, userData, operatorData); @@ -260,9 +354,10 @@ contract ERC777 is IERC777 { _balances[from] = _balances[from].sub(amount); _balances[to] = _balances[to].add(amount); - _callTokensReceived(operator, from, to, amount, userData, operatorData); + _callTokensReceived(operator, from, to, amount, userData, operatorData, requireReceptionAck); emit Sent(operator, from, to, amount, userData, operatorData); + emit Transfer(from, to, amount); } /** @@ -282,7 +377,7 @@ contract ERC777 is IERC777 { ) private { - require(from != address(0)); + require(from != address(0), "ERC777: burn from the zero address"); _callTokensToSend(operator, from, address(0), amount, data, operatorData); @@ -291,6 +386,17 @@ contract ERC777 is IERC777 { _balances[from] = _balances[from].sub(amount); emit Burned(operator, from, amount, data, operatorData); + emit Transfer(from, address(0), amount); + } + + function _approve(address owner, address spender, uint256 value) private { + // TODO: restore this require statement if this function becomes internal, or is called at a new callsite. It is + // currently unnecessary. + //require(owner != address(0), "ERC777: approve from the zero address"); + require(spender != address(0), "ERC777: approve to the zero address"); + + _allowances[owner][spender] = value; + emit Approval(owner, spender, value); } /** @@ -327,6 +433,7 @@ contract ERC777 is IERC777 { * @param amount uint256 amount of tokens to transfer * @param userData bytes extra information provided by the token holder (if any) * @param operatorData bytes extra information provided by the operator (if any) + * @param requireReceptionAck if true, contract recipients are required to implement ERC777TokensRecipient */ function _callTokensReceived( address operator, @@ -334,15 +441,16 @@ contract ERC777 is IERC777 { address to, uint256 amount, bytes memory userData, - bytes memory operatorData + bytes memory operatorData, + bool requireReceptionAck ) private { address implementer = _erc1820.getInterfaceImplementer(to, TOKENS_RECIPIENT_INTERFACE_HASH); if (implementer != address(0)) { IERC777Recipient(implementer).tokensReceived(operator, from, to, amount, userData, operatorData); - } else { - require(!to.isContract()); + } else if (requireReceptionAck) { + require(!to.isContract(), "ERC777: token recipient contract has no implementer for ERC777TokensRecipient"); } } } diff --git a/contracts/token/ERC20/ERC20.sol b/contracts/token/ERC20/ERC20.sol index 63c87d2f294..30039d274bd 100644 --- a/contracts/token/ERC20/ERC20.sol +++ b/contracts/token/ERC20/ERC20.sol @@ -20,7 +20,7 @@ contract ERC20 is IERC20 { mapping (address => uint256) private _balances; - mapping (address => mapping (address => uint256)) private _allowed; + mapping (address => mapping (address => uint256)) private _allowances; uint256 private _totalSupply; @@ -47,7 +47,7 @@ contract ERC20 is IERC20 { * @return A uint256 specifying the amount of tokens still available for the spender. */ function allowance(address owner, address spender) public view returns (uint256) { - return _allowed[owner][spender]; + return _allowances[owner][spender]; } /** @@ -84,13 +84,13 @@ contract ERC20 is IERC20 { */ function transferFrom(address from, address to, uint256 value) public returns (bool) { _transfer(from, to, value); - _approve(from, msg.sender, _allowed[from][msg.sender].sub(value)); + _approve(from, msg.sender, _allowances[from][msg.sender].sub(value)); return true; } /** * @dev Increase the amount of tokens that an owner allowed to a spender. - * approve should be called when _allowed[msg.sender][spender] == 0. To increment + * approve should be called when _allowances[msg.sender][spender] == 0. To increment * allowed value is better to use this function to avoid 2 calls (and wait until * the first transaction is mined) * From MonolithDAO Token.sol @@ -99,13 +99,13 @@ contract ERC20 is IERC20 { * @param addedValue The amount of tokens to increase the allowance by. */ function increaseAllowance(address spender, uint256 addedValue) public returns (bool) { - _approve(msg.sender, spender, _allowed[msg.sender][spender].add(addedValue)); + _approve(msg.sender, spender, _allowances[msg.sender][spender].add(addedValue)); return true; } /** * @dev Decrease the amount of tokens that an owner allowed to a spender. - * approve should be called when _allowed[msg.sender][spender] == 0. To decrement + * approve should be called when _allowances[msg.sender][spender] == 0. To decrement * allowed value is better to use this function to avoid 2 calls (and wait until * the first transaction is mined) * From MonolithDAO Token.sol @@ -114,7 +114,7 @@ contract ERC20 is IERC20 { * @param subtractedValue The amount of tokens to decrease the allowance by. */ function decreaseAllowance(address spender, uint256 subtractedValue) public returns (bool) { - _approve(msg.sender, spender, _allowed[msg.sender][spender].sub(subtractedValue)); + _approve(msg.sender, spender, _allowances[msg.sender][spender].sub(subtractedValue)); return true; } @@ -171,7 +171,7 @@ contract ERC20 is IERC20 { require(owner != address(0), "ERC20: approve from the zero address"); require(spender != address(0), "ERC20: approve to the zero address"); - _allowed[owner][spender] = value; + _allowances[owner][spender] = value; emit Approval(owner, spender, value); } @@ -185,6 +185,6 @@ contract ERC20 is IERC20 { */ function _burnFrom(address account, uint256 value) internal { _burn(account, value); - _approve(account, msg.sender, _allowed[account][msg.sender].sub(value)); + _approve(account, msg.sender, _allowances[account][msg.sender].sub(value)); } } diff --git a/test/drafts/ERC777/ERC777.behavior.js b/test/drafts/ERC777/ERC777.behavior.js index a2b16073e37..2a8db2acaf0 100644 --- a/test/drafts/ERC777/ERC777.behavior.js +++ b/test/drafts/ERC777/ERC777.behavior.js @@ -184,8 +184,9 @@ function shouldSendTokens (from, operator, to, amount, data, operatorData) { const initialFromBalance = await this.token.balanceOf(from); const initialToBalance = await this.token.balanceOf(to); + let logs; if (!operatorCall) { - const { logs } = await this.token.send(to, amount, data, { from }); + ({ logs } = await this.token.send(to, amount, data, { from })); expectEvent.inLogs(logs, 'Sent', { operator: from, from, @@ -195,7 +196,7 @@ function shouldSendTokens (from, operator, to, amount, data, operatorData) { operatorData: null, }); } else { - const { logs } = await this.token.operatorSend(from, to, amount, data, operatorData, { from: operator }); + ({ logs } = await this.token.operatorSend(from, to, amount, data, operatorData, { from: operator })); expectEvent.inLogs(logs, 'Sent', { operator, from, @@ -206,6 +207,12 @@ function shouldSendTokens (from, operator, to, amount, data, operatorData) { }); } + expectEvent.inLogs(logs, 'Transfer', { + from, + to, + value: amount, + }); + const finalTotalSupply = await this.token.totalSupply(); const finalFromBalance = await this.token.balanceOf(from); const finalToBalance = await this.token.balanceOf(to); @@ -231,8 +238,9 @@ function shouldBurnTokens (from, operator, amount, data, operatorData) { const initialTotalSupply = await this.token.totalSupply(); const initialFromBalance = await this.token.balanceOf(from); + let logs; if (!operatorCall) { - const { logs } = await this.token.burn(amount, data, { from }); + ({ logs } = await this.token.burn(amount, data, { from })); expectEvent.inLogs(logs, 'Burned', { operator: from, from, @@ -241,7 +249,7 @@ function shouldBurnTokens (from, operator, amount, data, operatorData) { operatorData: null, }); } else { - const { logs } = await this.token.operatorBurn(from, amount, data, operatorData, { from: operator }); + ({ logs } = await this.token.operatorBurn(from, amount, data, operatorData, { from: operator })); expectEvent.inLogs(logs, 'Burned', { operator, from, @@ -251,6 +259,12 @@ function shouldBurnTokens (from, operator, amount, data, operatorData) { }); } + expectEvent.inLogs(logs, 'Transfer', { + from, + to: ZERO_ADDRESS, + value: amount, + }); + const finalTotalSupply = await this.token.totalSupply(); const finalFromBalance = await this.token.balanceOf(from); @@ -274,6 +288,7 @@ function shouldInternalMintTokens (operator, to, amount, data, operatorData) { const initialToBalance = await this.token.balanceOf(to); const { logs } = await this.token.mintInternal(operator, to, amount, data, operatorData); + expectEvent.inLogs(logs, 'Minted', { operator, to, @@ -282,6 +297,12 @@ function shouldInternalMintTokens (operator, to, amount, data, operatorData) { operatorData, }); + expectEvent.inLogs(logs, 'Transfer', { + from: ZERO_ADDRESS, + to, + value: amount, + }); + const finalTotalSupply = await this.token.totalSupply(); const finalToBalance = await this.token.balanceOf(to); diff --git a/test/drafts/ERC777/ERC777.test.js b/test/drafts/ERC777/ERC777.test.js index 8a12539b8f6..8d758f6f981 100644 --- a/test/drafts/ERC777/ERC777.test.js +++ b/test/drafts/ERC777/ERC777.test.js @@ -9,6 +9,10 @@ const { shouldBehaveLikeERC777SendBurnWithSendHook, } = require('./ERC777.behavior'); +const { + shouldBehaveLikeERC20, +} = require('../../token/ERC20/ERC20.behavior'); + const ERC777 = artifacts.require('ERC777Mock'); const ERC777SenderRecipientMock = artifacts.require('ERC777SenderRecipientMock'); @@ -32,6 +36,8 @@ contract('ERC777', function ([ this.token = await ERC777.new(holder, initialSupply, name, symbol, defaultOperators); }); + shouldBehaveLikeERC20('ERC777', initialSupply, holder, anyone, defaultOperatorA); + it.skip('does not emit AuthorizedOperator events for default operators', async function () { expectEvent.not.inConstructor(this.token, 'AuthorizedOperator'); // This helper needs to be implemented }); @@ -45,7 +51,7 @@ contract('ERC777', function ([ (await this.token.symbol()).should.equal(symbol); }); - it('has a granularity of 1', async function () { + it('returns a granularity of 1', async function () { (await this.token.granularity()).should.be.bignumber.equal('1'); }); @@ -59,14 +65,23 @@ contract('ERC777', function ([ } }); - it('returns thte total supply', async function () { + it('returns the total supply', async function () { (await this.token.totalSupply()).should.be.bignumber.equal(initialSupply); }); - it('is registered in the registry', async function () { + it('returns 18 when decimals is called', async function () { + (await this.token.decimals()).should.be.bignumber.equal('18'); + }); + + it('the ERC777Token interface is registered in the registry', async function () { (await this.erc1820.getInterfaceImplementer(this.token.address, web3.utils.soliditySha3('ERC777Token'))) .should.equal(this.token.address); }); + + it('the ERC20Token interface is registered in the registry', async function () { + (await this.erc1820.getInterfaceImplementer(this.token.address, web3.utils.soliditySha3('ERC20Token'))) + .should.equal(this.token.address); + }); }); describe('balanceOf', function () { @@ -144,11 +159,15 @@ contract('ERC777', function ([ }); it('reverts when self-authorizing', async function () { - await shouldFail.reverting(this.token.authorizeOperator(holder, { from: holder })); + await shouldFail.reverting.withMessage( + this.token.authorizeOperator(holder, { from: holder }), 'ERC777: authorizing self as operator' + ); }); it('reverts when self-revoking', async function () { - await shouldFail.reverting(this.token.revokeOperator(holder, { from: holder })); + await shouldFail.reverting.withMessage( + this.token.revokeOperator(holder, { from: holder }), 'ERC777: revoking self as operator' + ); }); it('non-operators can be revoked', async function () { @@ -209,7 +228,10 @@ contract('ERC777', function ([ }); it('cannot be revoked for themselves', async function () { - await shouldFail.reverting(this.token.revokeOperator(defaultOperatorA, { from: defaultOperatorA })); + await shouldFail.reverting.withMessage( + this.token.revokeOperator(defaultOperatorA, { from: defaultOperatorA }), + 'ERC777: revoking self as operator' + ); }); context('with revoked default operator', function () { @@ -259,20 +281,35 @@ contract('ERC777', function ([ }); it('send reverts', async function () { - await shouldFail.reverting(this.token.send(this.recipient, amount, data)); + await shouldFail.reverting.withMessage( + this.token.send(this.recipient, amount, data, { from: holder }), + 'ERC777: token recipient contract has no implementer for ERC777TokensRecipient', + ); }); it('operatorSend reverts', async function () { - await shouldFail.reverting( - this.token.operatorSend(this.sender, this.recipient, amount, data, operatorData, { from: operator }) + await shouldFail.reverting.withMessage( + this.token.operatorSend(this.sender, this.recipient, amount, data, operatorData, { from: operator }), + 'ERC777: token recipient contract has no implementer for ERC777TokensRecipient', ); }); it('mint (internal) reverts', async function () { - await shouldFail.reverting( - this.token.mintInternal(operator, this.recipient, amount, data, operatorData) + await shouldFail.reverting.withMessage( + this.token.mintInternal(operator, this.recipient, amount, data, operatorData), + 'ERC777: token recipient contract has no implementer for ERC777TokensRecipient', ); }); + + it('(ERC20) transfer succeeds', async function () { + await this.token.transfer(this.recipient, amount, { from: holder }); + }); + + it('(ERC20) transferFrom succeeds', async function () { + const approved = anyone; + await this.token.approve(approved, amount, { from: this.sender }); + await this.token.transferFrom(this.sender, this.recipient, amount, { from: approved }); + }); }); }); diff --git a/test/token/ERC20/ERC20.behavior.js b/test/token/ERC20/ERC20.behavior.js new file mode 100644 index 00000000000..2536df005ec --- /dev/null +++ b/test/token/ERC20/ERC20.behavior.js @@ -0,0 +1,268 @@ +const { BN, constants, expectEvent, shouldFail } = require('openzeppelin-test-helpers'); +const { ZERO_ADDRESS } = constants; + +function shouldBehaveLikeERC20 (errorPrefix, initialSupply, initialHolder, recipient, anotherAccount) { + describe('total supply', function () { + it('returns the total amount of tokens', async function () { + (await this.token.totalSupply()).should.be.bignumber.equal(initialSupply); + }); + }); + + describe('balanceOf', function () { + describe('when the requested account has no tokens', function () { + it('returns zero', async function () { + (await this.token.balanceOf(anotherAccount)).should.be.bignumber.equal('0'); + }); + }); + + describe('when the requested account has some tokens', function () { + it('returns the total amount of tokens', async function () { + (await this.token.balanceOf(initialHolder)).should.be.bignumber.equal(initialSupply); + }); + }); + }); + + describe('transfer', function () { + describe('when the recipient is not the zero address', function () { + const to = recipient; + + describe('when the sender does not have enough balance', function () { + const amount = initialSupply.addn(1); + + it('reverts', async function () { + await shouldFail.reverting.withMessage(this.token.transfer(to, amount, { from: initialHolder }), + 'SafeMath: subtraction overflow' + ); + }); + }); + + describe('when the sender has enough balance', function () { + const amount = initialSupply; + + it('transfers the requested amount', async function () { + await this.token.transfer(to, amount, { from: initialHolder }); + + (await this.token.balanceOf(initialHolder)).should.be.bignumber.equal('0'); + + (await this.token.balanceOf(to)).should.be.bignumber.equal(amount); + }); + + it('emits a transfer event', async function () { + const { logs } = await this.token.transfer(to, amount, { from: initialHolder }); + + expectEvent.inLogs(logs, 'Transfer', { + from: initialHolder, + to: to, + value: amount, + }); + }); + }); + }); + + describe('when the recipient is the zero address', function () { + const to = ZERO_ADDRESS; + + it('reverts', async function () { + await shouldFail.reverting.withMessage(this.token.transfer(to, initialSupply, { from: initialHolder }), + `${errorPrefix}: transfer to the zero address` + ); + }); + }); + }); + + describe('transfer from', function () { + const spender = recipient; + + describe('when the recipient is not the zero address', function () { + const to = anotherAccount; + + describe('when the spender has enough approved balance', function () { + beforeEach(async function () { + await this.token.approve(spender, initialSupply, { from: initialHolder }); + }); + + describe('when the initial holder has enough balance', function () { + const amount = initialSupply; + + it('transfers the requested amount', async function () { + await this.token.transferFrom(initialHolder, to, amount, { from: spender }); + + (await this.token.balanceOf(initialHolder)).should.be.bignumber.equal('0'); + + (await this.token.balanceOf(to)).should.be.bignumber.equal(amount); + }); + + it('decreases the spender allowance', async function () { + await this.token.transferFrom(initialHolder, to, amount, { from: spender }); + + (await this.token.allowance(initialHolder, spender)).should.be.bignumber.equal('0'); + }); + + it('emits a transfer event', async function () { + const { logs } = await this.token.transferFrom(initialHolder, to, amount, { from: spender }); + + expectEvent.inLogs(logs, 'Transfer', { + from: initialHolder, + to: to, + value: amount, + }); + }); + + it('emits an approval event', async function () { + const { logs } = await this.token.transferFrom(initialHolder, to, amount, { from: spender }); + + expectEvent.inLogs(logs, 'Approval', { + owner: initialHolder, + spender: spender, + value: await this.token.allowance(initialHolder, spender), + }); + }); + }); + + describe('when the initial holder does not have enough balance', function () { + const amount = initialSupply.addn(1); + + it('reverts', async function () { + await shouldFail.reverting.withMessage(this.token.transferFrom( + initialHolder, to, amount, { from: spender }), 'SafeMath: subtraction overflow' + ); + }); + }); + }); + + describe('when the spender does not have enough approved balance', function () { + beforeEach(async function () { + await this.token.approve(spender, initialSupply.subn(1), { from: initialHolder }); + }); + + describe('when the initial holder has enough balance', function () { + const amount = initialSupply; + + it('reverts', async function () { + await shouldFail.reverting.withMessage(this.token.transferFrom( + initialHolder, to, amount, { from: spender }), 'SafeMath: subtraction overflow' + ); + }); + }); + + describe('when the initial holder does not have enough balance', function () { + const amount = initialSupply.addn(1); + + it('reverts', async function () { + await shouldFail.reverting.withMessage(this.token.transferFrom( + initialHolder, to, amount, { from: spender }), 'SafeMath: subtraction overflow' + ); + }); + }); + }); + }); + + describe('when the recipient is the zero address', function () { + const amount = initialSupply; + const to = ZERO_ADDRESS; + + beforeEach(async function () { + await this.token.approve(spender, amount, { from: initialHolder }); + }); + + it('reverts', async function () { + await shouldFail.reverting.withMessage(this.token.transferFrom( + initialHolder, to, amount, { from: spender }), `${errorPrefix}: transfer to the zero address` + ); + }); + }); + }); + + describe('approve', function () { + shouldBehaveLikeERC20Approve(errorPrefix, initialHolder, recipient, initialSupply, + function (owner, spender, amount) { + return this.token.approve(spender, amount, { from: owner }); + } + ); + }); +} + +function shouldBehaveLikeERC20Approve (errorPrefix, owner, spender, supply, approve) { + describe('when the spender is not the zero address', function () { + describe('when the sender has enough balance', function () { + const amount = supply; + + it('emits an approval event', async function () { + const { logs } = await approve.call(this, owner, spender, amount); + + expectEvent.inLogs(logs, 'Approval', { + owner: owner, + spender: spender, + value: amount, + }); + }); + + describe('when there was no approved amount before', function () { + it('approves the requested amount', async function () { + await approve.call(this, owner, spender, amount); + + (await this.token.allowance(owner, spender)).should.be.bignumber.equal(amount); + }); + }); + + describe('when the spender had an approved amount', function () { + beforeEach(async function () { + await approve.call(this, owner, spender, new BN(1)); + }); + + it('approves the requested amount and replaces the previous one', async function () { + await approve.call(this, owner, spender, amount); + + (await this.token.allowance(owner, spender)).should.be.bignumber.equal(amount); + }); + }); + }); + + describe('when the sender does not have enough balance', function () { + const amount = supply.addn(1); + + it('emits an approval event', async function () { + const { logs } = await approve.call(this, owner, spender, amount); + + expectEvent.inLogs(logs, 'Approval', { + owner: owner, + spender: spender, + value: amount, + }); + }); + + describe('when there was no approved amount before', function () { + it('approves the requested amount', async function () { + await approve.call(this, owner, spender, amount); + + (await this.token.allowance(owner, spender)).should.be.bignumber.equal(amount); + }); + }); + + describe('when the spender had an approved amount', function () { + beforeEach(async function () { + await approve.call(this, owner, spender, new BN(1)); + }); + + it('approves the requested amount and replaces the previous one', async function () { + await approve.call(this, owner, spender, amount); + + (await this.token.allowance(owner, spender)).should.be.bignumber.equal(amount); + }); + }); + }); + }); + + describe('when the spender is the zero address', function () { + it('reverts', async function () { + await shouldFail.reverting.withMessage(approve.call(this, owner, ZERO_ADDRESS, supply), + `${errorPrefix}: approve to the zero address` + ); + }); + }); +} + +module.exports = { + shouldBehaveLikeERC20, + shouldBehaveLikeERC20Approve, +}; diff --git a/test/token/ERC20/ERC20.test.js b/test/token/ERC20/ERC20.test.js index ba506d51967..be772655908 100644 --- a/test/token/ERC20/ERC20.test.js +++ b/test/token/ERC20/ERC20.test.js @@ -1,184 +1,21 @@ const { BN, constants, expectEvent, shouldFail } = require('openzeppelin-test-helpers'); const { ZERO_ADDRESS } = constants; +const { + shouldBehaveLikeERC20, + shouldBehaveLikeERC20Approve, +} = require('./ERC20.behavior'); + const ERC20Mock = artifacts.require('ERC20Mock'); contract('ERC20', function ([_, initialHolder, recipient, anotherAccount]) { const initialSupply = new BN(100); + beforeEach(async function () { this.token = await ERC20Mock.new(initialHolder, initialSupply); }); - describe('total supply', function () { - it('returns the total amount of tokens', async function () { - (await this.token.totalSupply()).should.be.bignumber.equal(initialSupply); - }); - }); - - describe('balanceOf', function () { - describe('when the requested account has no tokens', function () { - it('returns zero', async function () { - (await this.token.balanceOf(anotherAccount)).should.be.bignumber.equal('0'); - }); - }); - - describe('when the requested account has some tokens', function () { - it('returns the total amount of tokens', async function () { - (await this.token.balanceOf(initialHolder)).should.be.bignumber.equal(initialSupply); - }); - }); - }); - - describe('transfer', function () { - describe('when the recipient is not the zero address', function () { - const to = recipient; - - describe('when the sender does not have enough balance', function () { - const amount = initialSupply.addn(1); - - it('reverts', async function () { - await shouldFail.reverting.withMessage(this.token.transfer(to, amount, { from: initialHolder }), - 'SafeMath: subtraction overflow' - ); - }); - }); - - describe('when the sender has enough balance', function () { - const amount = initialSupply; - - it('transfers the requested amount', async function () { - await this.token.transfer(to, amount, { from: initialHolder }); - - (await this.token.balanceOf(initialHolder)).should.be.bignumber.equal('0'); - - (await this.token.balanceOf(to)).should.be.bignumber.equal(amount); - }); - - it('emits a transfer event', async function () { - const { logs } = await this.token.transfer(to, amount, { from: initialHolder }); - - expectEvent.inLogs(logs, 'Transfer', { - from: initialHolder, - to: to, - value: amount, - }); - }); - }); - }); - - describe('when the recipient is the zero address', function () { - const to = ZERO_ADDRESS; - - it('reverts', async function () { - await shouldFail.reverting.withMessage(this.token.transfer(to, initialSupply, { from: initialHolder }), - 'ERC20: transfer to the zero address' - ); - }); - }); - }); - - describe('transfer from', function () { - const spender = recipient; - - describe('when the recipient is not the zero address', function () { - const to = anotherAccount; - - describe('when the spender has enough approved balance', function () { - beforeEach(async function () { - await this.token.approve(spender, initialSupply, { from: initialHolder }); - }); - - describe('when the initial holder has enough balance', function () { - const amount = initialSupply; - - it('transfers the requested amount', async function () { - await this.token.transferFrom(initialHolder, to, amount, { from: spender }); - - (await this.token.balanceOf(initialHolder)).should.be.bignumber.equal('0'); - - (await this.token.balanceOf(to)).should.be.bignumber.equal(amount); - }); - - it('decreases the spender allowance', async function () { - await this.token.transferFrom(initialHolder, to, amount, { from: spender }); - - (await this.token.allowance(initialHolder, spender)).should.be.bignumber.equal('0'); - }); - - it('emits a transfer event', async function () { - const { logs } = await this.token.transferFrom(initialHolder, to, amount, { from: spender }); - - expectEvent.inLogs(logs, 'Transfer', { - from: initialHolder, - to: to, - value: amount, - }); - }); - - it('emits an approval event', async function () { - const { logs } = await this.token.transferFrom(initialHolder, to, amount, { from: spender }); - - expectEvent.inLogs(logs, 'Approval', { - owner: initialHolder, - spender: spender, - value: await this.token.allowance(initialHolder, spender), - }); - }); - }); - - describe('when the initial holder does not have enough balance', function () { - const amount = initialSupply.addn(1); - - it('reverts', async function () { - await shouldFail.reverting.withMessage(this.token.transferFrom( - initialHolder, to, amount, { from: spender }), 'SafeMath: subtraction overflow' - ); - }); - }); - }); - - describe('when the spender does not have enough approved balance', function () { - beforeEach(async function () { - await this.token.approve(spender, initialSupply.subn(1), { from: initialHolder }); - }); - - describe('when the initial holder has enough balance', function () { - const amount = initialSupply; - - it('reverts', async function () { - await shouldFail.reverting.withMessage(this.token.transferFrom( - initialHolder, to, amount, { from: spender }), 'SafeMath: subtraction overflow' - ); - }); - }); - - describe('when the initial holder does not have enough balance', function () { - const amount = initialSupply.addn(1); - - it('reverts', async function () { - await shouldFail.reverting.withMessage(this.token.transferFrom( - initialHolder, to, amount, { from: spender }), 'SafeMath: subtraction overflow' - ); - }); - }); - }); - }); - - describe('when the recipient is the zero address', function () { - const amount = initialSupply; - const to = ZERO_ADDRESS; - - beforeEach(async function () { - await this.token.approve(spender, amount, { from: initialHolder }); - }); - - it('reverts', async function () { - await shouldFail.reverting.withMessage(this.token.transferFrom( - initialHolder, to, amount, { from: spender }), 'ERC20: transfer to the zero address' - ); - }); - }); - }); + shouldBehaveLikeERC20('ERC20', initialSupply, initialHolder, recipient, anotherAccount); describe('decrease allowance', function () { describe('when the spender is not the zero address', function () { @@ -493,14 +330,8 @@ contract('ERC20', function ([_, initialHolder, recipient, anotherAccount]) { }); }); - describe('approve', function () { - testApprove(initialHolder, recipient, initialSupply, function (owner, spender, amount) { - return this.token.approve(spender, amount, { from: owner }); - }); - }); - describe('_approve', function () { - testApprove(initialHolder, recipient, initialSupply, function (owner, spender, amount) { + shouldBehaveLikeERC20Approve('ERC20', initialHolder, recipient, initialSupply, function (owner, spender, amount) { return this.token.approveInternal(owner, spender, amount); }); @@ -512,84 +343,4 @@ contract('ERC20', function ([_, initialHolder, recipient, anotherAccount]) { }); }); }); - - function testApprove (owner, spender, supply, approve) { - describe('when the spender is not the zero address', function () { - describe('when the sender has enough balance', function () { - const amount = supply; - - it('emits an approval event', async function () { - const { logs } = await approve.call(this, owner, spender, amount); - - expectEvent.inLogs(logs, 'Approval', { - owner: owner, - spender: spender, - value: amount, - }); - }); - - describe('when there was no approved amount before', function () { - it('approves the requested amount', async function () { - await approve.call(this, owner, spender, amount); - - (await this.token.allowance(owner, spender)).should.be.bignumber.equal(amount); - }); - }); - - describe('when the spender had an approved amount', function () { - beforeEach(async function () { - await approve.call(this, owner, spender, new BN(1)); - }); - - it('approves the requested amount and replaces the previous one', async function () { - await approve.call(this, owner, spender, amount); - - (await this.token.allowance(owner, spender)).should.be.bignumber.equal(amount); - }); - }); - }); - - describe('when the sender does not have enough balance', function () { - const amount = supply.addn(1); - - it('emits an approval event', async function () { - const { logs } = await approve.call(this, owner, spender, amount); - - expectEvent.inLogs(logs, 'Approval', { - owner: owner, - spender: spender, - value: amount, - }); - }); - - describe('when there was no approved amount before', function () { - it('approves the requested amount', async function () { - await approve.call(this, owner, spender, amount); - - (await this.token.allowance(owner, spender)).should.be.bignumber.equal(amount); - }); - }); - - describe('when the spender had an approved amount', function () { - beforeEach(async function () { - await approve.call(this, owner, spender, new BN(1)); - }); - - it('approves the requested amount and replaces the previous one', async function () { - await approve.call(this, owner, spender, amount); - - (await this.token.allowance(owner, spender)).should.be.bignumber.equal(amount); - }); - }); - }); - }); - - describe('when the spender is the zero address', function () { - it('reverts', async function () { - await shouldFail.reverting.withMessage(approve.call(this, owner, ZERO_ADDRESS, supply), - 'ERC20: approve to the zero address' - ); - }); - }); - } });