NoBey/openzeppelin-contracts
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix balance and allowance checks in _burn and _burnFrom (#1218)

Browse Source 
* fix off by one error in _burn and _burnFrom

* add tests for off-by-one error

* change amount used
This commit is contained in:
Francisco Giordano
2018-08-17 19:18:05 -03:00
committed by GitHub
parent 20cf885430
commit 7618b91d9c
2 changed files with 63 additions and 55 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
contracts/token/ERC20/StandardToken.sol
View File

@ -178,7 +178,7 @@ contract StandardToken is ERC20 {
*/
function _burn(address _account, uint256 _amount) internal {
require(_account != 0);
require(balances[_account] > _amount);
require(_amount <= balances[_account]);
totalSupply_ = totalSupply_.sub(_amount);
balances[_account] = balances[_account].sub(_amount);
@ -193,7 +193,7 @@ contract StandardToken is ERC20 {
* @param _amount The amount that will be burnt.
*/
function _burnFrom(address _account, uint256 _amount) internal {
require(allowed[_account][msg.sender] > _amount);
require(_amount <= 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.

114
test/token/ERC20/StandardToken.test.js
View File

@ -498,10 +498,9 @@ contract('StandardToken', function ([_, owner, recipient, anotherAccount]) {
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));
await assertRevert(this.token.burn(ZERO_ADDRESS, 1));
});
describe('for a non null account', function () {
@ -509,38 +508,42 @@ contract('StandardToken', function ([_, owner, recipient, anotherAccount]) {
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,
const describeBurn = function (description, amount) {
describe(description, function () {
beforeEach('burning', async function () {
const { logs } = await this.token.burn(owner, amount);
this.logs = logs;
});
event.args.value.should.be.bignumber.equal(amount);
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);
});
});
});
};
describeBurn('for entire balance', initialSupply);
describeBurn('for less amount than balance', initialSupply.sub(1));
});
});
describe('_burnFrom', function () {
const initialSupply = new BigNumber(100);
const allowance = new BigNumber(70);
const amount = new BigNumber(50);
const spender = anotherAccount;
@ -549,7 +552,7 @@ contract('StandardToken', function ([_, owner, recipient, anotherAccount]) {
});
it('rejects a null account', async function () {
await assertRevert(this.token.burnFrom(ZERO_ADDRESS, amount));
await assertRevert(this.token.burnFrom(ZERO_ADDRESS, 1));
});
describe('for a non null account', function () {
@ -561,36 +564,41 @@ contract('StandardToken', function ([_, owner, recipient, anotherAccount]) {
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,
const describeBurnFrom = function (description, amount) {
describe(description, function () {
beforeEach('burning', async function () {
const { logs } = await this.token.burnFrom(owner, amount, { from: spender });
this.logs = logs;
});
event.args.value.should.be.bignumber.equal(amount);
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);
});
});
});
};
describeBurnFrom('for entire allowance', allowance);
describeBurnFrom('for less amount than allowance', allowance.sub(1));
});
});
});