diff --git a/contracts/examples/SimpleToken.sol b/contracts/examples/SimpleToken.sol index 835396f2ef2..73df121dd8f 100644 --- a/contracts/examples/SimpleToken.sol +++ b/contracts/examples/SimpleToken.sol @@ -22,9 +22,7 @@ contract SimpleToken is StandardToken { * @dev Constructor that gives msg.sender all of existing tokens. */ constructor() public { - totalSupply_ = INITIAL_SUPPLY; - balances[msg.sender] = INITIAL_SUPPLY; - emit Transfer(address(0), msg.sender, INITIAL_SUPPLY); + _mint(msg.sender, INITIAL_SUPPLY); } } diff --git a/contracts/mocks/BurnableTokenMock.sol b/contracts/mocks/BurnableTokenMock.sol index f7bdd59d9da..a148f17f47e 100644 --- a/contracts/mocks/BurnableTokenMock.sol +++ b/contracts/mocks/BurnableTokenMock.sol @@ -6,8 +6,7 @@ import "../token/ERC20/BurnableToken.sol"; contract BurnableTokenMock is BurnableToken { constructor(address _initialAccount, uint256 _initialBalance) public { - balances[_initialAccount] = _initialBalance; - totalSupply_ = _initialBalance; + _mint(_initialAccount, _initialBalance); } } diff --git a/contracts/mocks/ERC223TokenMock.sol b/contracts/mocks/ERC223TokenMock.sol index 410348cc044..2f92bc5f1f4 100644 --- a/contracts/mocks/ERC223TokenMock.sol +++ b/contracts/mocks/ERC223TokenMock.sol @@ -11,8 +11,7 @@ contract ERC223ContractInterface { contract ERC223TokenMock is StandardToken { constructor(address _initialAccount, uint256 _initialBalance) public { - balances[_initialAccount] = _initialBalance; - totalSupply_ = _initialBalance; + _mint(_initialAccount, _initialBalance); } // ERC223 compatible transfer function (except the name) diff --git a/contracts/mocks/PausableTokenMock.sol b/contracts/mocks/PausableTokenMock.sol index f1a1d53ddb4..24ef281bade 100644 --- a/contracts/mocks/PausableTokenMock.sol +++ b/contracts/mocks/PausableTokenMock.sol @@ -7,7 +7,7 @@ import "../token/ERC20/PausableToken.sol"; contract PausableTokenMock is PausableToken { constructor(address _initialAccount, uint _initialBalance) public { - balances[_initialAccount] = _initialBalance; + _mint(_initialAccount, _initialBalance); } } diff --git a/contracts/mocks/StandardTokenMock.sol b/contracts/mocks/StandardTokenMock.sol index 6a7dd5aa163..e4420c470bc 100644 --- a/contracts/mocks/StandardTokenMock.sol +++ b/contracts/mocks/StandardTokenMock.sol @@ -7,8 +7,19 @@ import "../token/ERC20/StandardToken.sol"; contract StandardTokenMock is StandardToken { constructor(address _initialAccount, uint256 _initialBalance) public { - balances[_initialAccount] = _initialBalance; - totalSupply_ = _initialBalance; + _mint(_initialAccount, _initialBalance); + } + + function mint(address _account, uint256 _amount) public { + _mint(_account, _amount); + } + + function burn(address _account, uint256 _amount) public { + _burn(_account, _amount); + } + + function burnFrom(address _account, uint256 _amount) public { + _burnFrom(_account, _amount); } } diff --git a/contracts/token/ERC20/BurnableToken.sol b/contracts/token/ERC20/BurnableToken.sol index ad49ff11dbc..7b1c7c9be95 100644 --- a/contracts/token/ERC20/BurnableToken.sol +++ b/contracts/token/ERC20/BurnableToken.sol @@ -25,21 +25,15 @@ contract BurnableToken is StandardToken { * @param _value uint256 The amount of token to be burned */ function burnFrom(address _from, uint256 _value) public { - require(_value <= allowed[_from][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[_from][msg.sender] = allowed[_from][msg.sender].sub(_value); - _burn(_from, _value); + _burnFrom(_from, _value); } + /** + * @dev Overrides StandardToken._burn in order for burn and burnFrom to emit + * an additional Burn event. + */ function _burn(address _who, uint256 _value) internal { - require(_value <= balances[_who]); - // no need to require value <= totalSupply, since that would imply the - // sender's balance is greater than the totalSupply, which *should* be an assertion failure - - balances[_who] = balances[_who].sub(_value); - totalSupply_ = totalSupply_.sub(_value); + super._burn(_who, _value); emit Burn(_who, _value); - emit Transfer(_who, address(0), _value); } -} \ No newline at end of file +} diff --git a/contracts/token/ERC20/CappedToken.sol b/contracts/token/ERC20/CappedToken.sol index 497b6fdc418..1af8bdcb647 100644 --- a/contracts/token/ERC20/CappedToken.sol +++ b/contracts/token/ERC20/CappedToken.sol @@ -29,7 +29,7 @@ contract CappedToken is MintableToken { public returns (bool) { - require(totalSupply_.add(_amount) <= cap); + require(totalSupply().add(_amount) <= cap); return super.mint(_to, _amount); } diff --git a/contracts/token/ERC20/MintableToken.sol b/contracts/token/ERC20/MintableToken.sol index 1821058ff94..ba1ca68336f 100644 --- a/contracts/token/ERC20/MintableToken.sol +++ b/contracts/token/ERC20/MintableToken.sol @@ -41,10 +41,8 @@ contract MintableToken is StandardToken, Ownable { canMint returns (bool) { - totalSupply_ = totalSupply_.add(_amount); - balances[_to] = balances[_to].add(_amount); + _mint(_to, _amount); emit Mint(_to, _amount); - emit Transfer(address(0), _to, _amount); return true; } diff --git a/contracts/token/ERC20/StandardToken.sol b/contracts/token/ERC20/StandardToken.sol index 88b215473d4..52e125a2771 100644 --- a/contracts/token/ERC20/StandardToken.sol +++ b/contracts/token/ERC20/StandardToken.sol @@ -8,17 +8,17 @@ import "../../math/SafeMath.sol"; * @title Standard ERC20 token * * @dev Implementation of the basic standard token. - * https://github.com/ethereum/EIPs/issues/20 + * https://github.com/ethereum/EIPs/blob/master/EIPS/eip-20.md * Based on code by FirstBlood: https://github.com/Firstbloodio/token/blob/master/smart_contract/FirstBloodToken.sol */ contract StandardToken is ERC20 { using SafeMath for uint256; - mapping(address => uint256) balances; + mapping (address => uint256) private balances; - mapping (address => mapping (address => uint256)) internal allowed; + mapping (address => mapping (address => uint256)) private allowed; - uint256 totalSupply_; + uint256 private totalSupply_; /** * @dev Total number of tokens in existence @@ -156,4 +156,48 @@ contract StandardToken is ERC20 { return true; } + /** + * @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. + */ + function _mint(address _account, uint256 _amount) internal { + require(_account != 0); + totalSupply_ = totalSupply_.add(_amount); + balances[_account] = balances[_account].add(_amount); + emit Transfer(address(0), _account, _amount); + } + + /** + * @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. + */ + function _burn(address _account, uint256 _amount) internal { + require(_account != 0); + require(balances[_account] > _amount); + + totalSupply_ = totalSupply_.sub(_amount); + balances[_account] = balances[_account].sub(_amount); + emit Transfer(_account, address(0), _amount); + } + + /** + * @dev Internal function that burns an amount of the token of a given + * 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. + */ + function _burnFrom(address _account, uint256 _amount) internal { + require(allowed[_account][msg.sender] > _amount); + + // 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); + } } diff --git a/test/token/ERC20/CappedToken.behavior.js b/test/token/ERC20/CappedToken.behavior.js index 85e3380d1e0..46c541c597c 100644 --- a/test/token/ERC20/CappedToken.behavior.js +++ b/test/token/ERC20/CappedToken.behavior.js @@ -1,4 +1,5 @@ const { expectThrow } = require('../../helpers/expectThrow'); +const expectEvent = require('../../helpers/expectEvent'); const BigNumber = web3.BigNumber; @@ -15,8 +16,8 @@ function shouldBehaveLikeCappedToken (minter, [anyone], cap) { }); it('should mint when amount is less than cap', async function () { - const result = await this.token.mint(anyone, cap.sub(1), { from }); - result.logs[0].event.should.equal('Mint'); + const { logs } = await this.token.mint(anyone, cap.sub(1), { from }); + expectEvent.inLogs(logs, 'Mint'); }); it('should fail to mint if the ammount exceeds the cap', async function () { diff --git a/test/token/ERC20/MintableToken.behavior.js b/test/token/ERC20/MintableToken.behavior.js index 7d34e2719b2..c037d92ff62 100644 --- a/test/token/ERC20/MintableToken.behavior.js +++ b/test/token/ERC20/MintableToken.behavior.js @@ -1,4 +1,5 @@ const { assertRevert } = require('../../helpers/assertRevert'); +const expectEvent = require('../../helpers/expectEvent'); const BigNumber = web3.BigNumber; @@ -7,6 +8,8 @@ require('chai') .should(); function shouldBehaveLikeMintableToken (owner, minter, [anyone]) { + const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000'; + describe('as a basic mintable token', function () { describe('after token creation', function () { it('sender should be token owner', async function () { @@ -104,11 +107,16 @@ function shouldBehaveLikeMintableToken (owner, minter, [anyone]) { it('emits a mint and a transfer event', async function () { const { logs } = await this.token.mint(owner, amount, { from }); - logs.length.should.eq(2); - logs[0].event.should.eq('Mint'); - logs[0].args.to.should.eq(owner); - logs[0].args.amount.should.be.bignumber.equal(amount); - logs[1].event.should.eq('Transfer'); + const mintEvent = expectEvent.inLogs(logs, 'Mint', { + to: owner, + }); + mintEvent.args.amount.should.be.bignumber.equal(amount); + + const transferEvent = expectEvent.inLogs(logs, 'Transfer', { + from: ZERO_ADDRESS, + to: owner, + }); + transferEvent.args.value.should.be.bignumber.equal(amount); }); }); diff --git a/test/token/ERC20/StandardToken.test.js b/test/token/ERC20/StandardToken.test.js index e1f35750200..9487d107a35 100644 --- a/test/token/ERC20/StandardToken.test.js +++ b/test/token/ERC20/StandardToken.test.js @@ -1,4 +1,6 @@ const { assertRevert } = require('../../helpers/assertRevert'); +const expectEvent = require('../../helpers/expectEvent'); + const StandardToken = artifacts.require('StandardTokenMock'); const BigNumber = web3.BigNumber; @@ -68,11 +70,12 @@ contract('StandardToken', function ([_, owner, recipient, anotherAccount]) { it('emits a transfer event', async function () { const { logs } = await this.token.transfer(to, amount, { from: owner }); - logs.length.should.eq(1); - logs[0].event.should.eq('Transfer'); - logs[0].args.from.should.eq(owner); - logs[0].args.to.should.eq(to); - logs[0].args.value.should.be.bignumber.equal(amount); + const event = expectEvent.inLogs(logs, 'Transfer', { + from: owner, + to: to, + }); + + event.args.value.should.be.bignumber.equal(amount); }); }); }); @@ -486,4 +489,136 @@ contract('StandardToken', function ([_, owner, recipient, anotherAccount]) { }); }); }); + + describe('_mint', function () { + const initialSupply = new BigNumber(100); + const amount = new BigNumber(50); + + it('rejects a null account', async function () { + await assertRevert(this.token.mint(ZERO_ADDRESS, amount)); + }); + + describe('for a non null account', function () { + beforeEach('minting', async function () { + const { logs } = await this.token.mint(recipient, amount); + this.logs = logs; + }); + + it('increments totalSupply', async function () { + const expectedSupply = initialSupply.plus(amount); + (await this.token.totalSupply()).should.be.bignumber.equal(expectedSupply); + }); + + it('increments recipient balance', async function () { + (await this.token.balanceOf(recipient)).should.be.bignumber.equal(amount); + }); + + it('emits Transfer event', async function () { + const event = expectEvent.inLogs(this.logs, 'Transfer', { + from: ZERO_ADDRESS, + to: recipient, + }); + + event.args.value.should.be.bignumber.equal(amount); + }); + }); + }); + + describe('_burn', function () { + const initialSupply = new BigNumber(100); + const amount = new BigNumber(50); + + it('rejects a null account', async function () { + await assertRevert(this.token.burn(ZERO_ADDRESS, amount)); + }); + + describe('for a non null account', function () { + it('rejects burning more than balance', async function () { + await assertRevert(this.token.burn(owner, initialSupply.plus(1))); + }); + + describe('for less amount than balance', function () { + beforeEach('burning', async function () { + const { logs } = await this.token.burn(owner, amount); + this.logs = logs; + }); + + it('decrements totalSupply', async function () { + const expectedSupply = initialSupply.minus(amount); + (await this.token.totalSupply()).should.be.bignumber.equal(expectedSupply); + }); + + it('decrements owner balance', async function () { + const expectedBalance = initialSupply.minus(amount); + (await this.token.balanceOf(owner)).should.be.bignumber.equal(expectedBalance); + }); + + it('emits Transfer event', async function () { + const event = expectEvent.inLogs(this.logs, 'Transfer', { + from: owner, + to: ZERO_ADDRESS, + }); + + event.args.value.should.be.bignumber.equal(amount); + }); + }); + }); + }); + + describe('_burnFrom', function () { + const initialSupply = new BigNumber(100); + const allowance = new BigNumber(70); + const amount = new BigNumber(50); + + const spender = anotherAccount; + + beforeEach('approving', async function () { + await this.token.approve(spender, allowance, { from: owner }); + }); + + it('rejects a null account', async function () { + await assertRevert(this.token.burnFrom(ZERO_ADDRESS, amount)); + }); + + describe('for a non null account', function () { + it('rejects burning more than allowance', async function () { + await assertRevert(this.token.burnFrom(owner, allowance.plus(1))); + }); + + it('rejects burning more than balance', async function () { + await assertRevert(this.token.burnFrom(owner, initialSupply.plus(1))); + }); + + describe('for less amount than allowance', function () { + beforeEach('burning', async function () { + const { logs } = await this.token.burnFrom(owner, amount, { from: spender }); + this.logs = logs; + }); + + it('decrements totalSupply', async function () { + const expectedSupply = initialSupply.minus(amount); + (await this.token.totalSupply()).should.be.bignumber.equal(expectedSupply); + }); + + it('decrements owner balance', async function () { + const expectedBalance = initialSupply.minus(amount); + (await this.token.balanceOf(owner)).should.be.bignumber.equal(expectedBalance); + }); + + it('decrements spender allowance', async function () { + const expectedAllowance = allowance.minus(amount); + (await this.token.allowance(owner, spender)).should.be.bignumber.equal(expectedAllowance); + }); + + it('emits Transfer event', async function () { + const event = expectEvent.inLogs(this.logs, 'Transfer', { + from: owner, + to: ZERO_ADDRESS, + }); + + event.args.value.should.be.bignumber.equal(amount); + }); + }); + }); + }); });