diff --git a/contracts/token/ERC20/ERC20.sol b/contracts/token/ERC20/ERC20.sol index 0201cc0b849..ad2b9aa91ea 100644 --- a/contracts/token/ERC20/ERC20.sol +++ b/contracts/token/ERC20/ERC20.sol @@ -78,6 +78,8 @@ contract ERC20 is IERC20 { * @param _value The amount of tokens to be spent. */ function approve(address _spender, uint256 _value) public returns (bool) { + require(_spender != address(0)); + allowed_[msg.sender][_spender] = _value; emit Approval(msg.sender, _spender, _value); return true; @@ -117,13 +119,15 @@ contract ERC20 is IERC20 { * @param _spender The address which will spend the funds. * @param _addedValue The amount of tokens to increase the allowance by. */ - function increaseApproval( + function increaseAllowance( address _spender, uint256 _addedValue ) public returns (bool) { + require(_spender != address(0)); + allowed_[msg.sender][_spender] = ( allowed_[msg.sender][_spender].add(_addedValue)); emit Approval(msg.sender, _spender, allowed_[msg.sender][_spender]); @@ -139,19 +143,17 @@ contract ERC20 is IERC20 { * @param _spender The address which will spend the funds. * @param _subtractedValue The amount of tokens to decrease the allowance by. */ - function decreaseApproval( + function decreaseAllowance( address _spender, uint256 _subtractedValue ) public returns (bool) { - uint256 oldValue = allowed_[msg.sender][_spender]; - if (_subtractedValue >= oldValue) { - allowed_[msg.sender][_spender] = 0; - } else { - allowed_[msg.sender][_spender] = oldValue.sub(_subtractedValue); - } + require(_spender != address(0)); + + allowed_[msg.sender][_spender] = ( + allowed_[msg.sender][_spender].sub(_subtractedValue)); emit Approval(msg.sender, _spender, allowed_[msg.sender][_spender]); return true; } diff --git a/contracts/token/ERC20/ERC20Pausable.sol b/contracts/token/ERC20/ERC20Pausable.sol index d631fe1d960..4bacd0cae2c 100644 --- a/contracts/token/ERC20/ERC20Pausable.sol +++ b/contracts/token/ERC20/ERC20Pausable.sol @@ -44,7 +44,7 @@ contract ERC20Pausable is ERC20, Pausable { return super.approve(_spender, _value); } - function increaseApproval( + function increaseAllowance( address _spender, uint _addedValue ) @@ -52,10 +52,10 @@ contract ERC20Pausable is ERC20, Pausable { whenNotPaused returns (bool success) { - return super.increaseApproval(_spender, _addedValue); + return super.increaseAllowance(_spender, _addedValue); } - function decreaseApproval( + function decreaseAllowance( address _spender, uint _subtractedValue ) @@ -63,6 +63,6 @@ contract ERC20Pausable is ERC20, Pausable { whenNotPaused returns (bool success) { - return super.decreaseApproval(_spender, _subtractedValue); + return super.decreaseAllowance(_spender, _subtractedValue); } } diff --git a/test/token/ERC20/ERC20.test.js b/test/token/ERC20/ERC20.test.js index a5ca274c79f..f9568b87e1c 100644 --- a/test/token/ERC20/ERC20.test.js +++ b/test/token/ERC20/ERC20.test.js @@ -158,20 +158,8 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { const amount = 100; const spender = ZERO_ADDRESS; - it('approves the requested amount', async function () { - await this.token.approve(spender, amount, { from: owner }); - - (await this.token.allowance(owner, spender)).should.be.bignumber.equal(amount); - }); - - 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); + it('reverts', async function () { + await assertRevert(this.token.approve(spender, amount, { from: owner })); }); }); }); @@ -261,28 +249,14 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { }); }); - describe('decrease approval', function () { + describe('decrease allowance', function () { describe('when the spender is not the zero address', function () { const spender = recipient; - describe('when the sender has enough balance', function () { - const amount = 100; - - it('emits an approval event', async function () { - const { logs } = await this.token.decreaseApproval(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(0); - }); - + function shouldDecreaseApproval (amount) { describe('when there was no approved amount before', function () { - it('keeps the allowance to zero', async function () { - await this.token.decreaseApproval(spender, amount, { from: owner }); - - (await this.token.allowance(owner, spender)).should.be.bignumber.equal(0); + it('reverts', async function () { + await assertRevert(this.token.decreaseAllowance(spender, amount, { from: owner })); }); }); @@ -290,59 +264,46 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { const approvedAmount = amount; beforeEach(async function () { - await this.token.approve(spender, approvedAmount, { from: owner }); + ({ logs: this.logs } = await this.token.approve(spender, approvedAmount, { from: owner })); + }); + + 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); }); it('decreases the spender allowance subtracting the requested amount', async function () { - await this.token.decreaseApproval(spender, approvedAmount - 5, { from: owner }); + await this.token.decreaseAllowance(spender, approvedAmount - 1, { from: owner }); - (await this.token.allowance(owner, spender)).should.be.bignumber.equal(5); + (await this.token.allowance(owner, spender)).should.be.bignumber.equal(1); }); it('sets the allowance to zero when all allowance is removed', async function () { - await this.token.decreaseApproval(spender, approvedAmount, { from: owner }); + await this.token.decreaseAllowance(spender, approvedAmount, { from: owner }); (await this.token.allowance(owner, spender)).should.be.bignumber.equal(0); }); - it('sets the allowance to zero when more than the full allowance is removed', async function () { - await this.token.decreaseApproval(spender, approvedAmount + 5, { from: owner }); - (await this.token.allowance(owner, spender)).should.be.bignumber.equal(0); + it('reverts when more than the full allowance is removed', async function () { + await assertRevert(this.token.decreaseAllowance(spender, approvedAmount + 1, { from: owner })); }); }); + } + + describe('when the sender has enough balance', function () { + const amount = 100; + + shouldDecreaseApproval(amount); }); describe('when the sender does not have enough balance', function () { const amount = 101; - it('emits an approval event', async function () { - const { logs } = await this.token.decreaseApproval(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(0); - }); - - describe('when there was no approved amount before', function () { - it('keeps the allowance to zero', async function () { - await this.token.decreaseApproval(spender, amount, { from: owner }); - - (await this.token.allowance(owner, spender)).should.be.bignumber.equal(0); - }); - }); - - describe('when the spender had an approved amount', function () { - beforeEach(async function () { - await this.token.approve(spender, amount + 1, { from: owner }); - }); - - it('decreases the spender allowance subtracting the requested amount', async function () { - await this.token.decreaseApproval(spender, amount, { from: owner }); - - (await this.token.allowance(owner, spender)).should.be.bignumber.equal(1); - }); - }); + shouldDecreaseApproval(amount); }); }); @@ -350,25 +311,13 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { const amount = 100; const spender = ZERO_ADDRESS; - it('decreases the requested amount', async function () { - await this.token.decreaseApproval(spender, amount, { from: owner }); - - (await this.token.allowance(owner, spender)).should.be.bignumber.equal(0); - }); - - it('emits an approval event', async function () { - const { logs } = await this.token.decreaseApproval(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(0); + it('reverts', async function () { + await assertRevert(this.token.decreaseAllowance(spender, amount, { from: owner })); }); }); }); - describe('increase approval', function () { + describe('increase allowance', function () { const amount = 100; describe('when the spender is not the zero address', function () { @@ -376,7 +325,7 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { describe('when the sender has enough balance', function () { it('emits an approval event', async function () { - const { logs } = await this.token.increaseApproval(spender, amount, { from: owner }); + const { logs } = await this.token.increaseAllowance(spender, amount, { from: owner }); logs.length.should.equal(1); logs[0].event.should.equal('Approval'); @@ -387,7 +336,7 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { describe('when there was no approved amount before', function () { it('approves the requested amount', async function () { - await this.token.increaseApproval(spender, amount, { from: owner }); + await this.token.increaseAllowance(spender, amount, { from: owner }); (await this.token.allowance(owner, spender)).should.be.bignumber.equal(amount); }); @@ -399,7 +348,7 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { }); it('increases the spender allowance adding the requested amount', async function () { - await this.token.increaseApproval(spender, amount, { from: owner }); + await this.token.increaseAllowance(spender, amount, { from: owner }); (await this.token.allowance(owner, spender)).should.be.bignumber.equal(amount + 1); }); @@ -410,7 +359,7 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { const amount = 101; it('emits an approval event', async function () { - const { logs } = await this.token.increaseApproval(spender, amount, { from: owner }); + const { logs } = await this.token.increaseAllowance(spender, amount, { from: owner }); logs.length.should.equal(1); logs[0].event.should.equal('Approval'); @@ -421,7 +370,7 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { describe('when there was no approved amount before', function () { it('approves the requested amount', async function () { - await this.token.increaseApproval(spender, amount, { from: owner }); + await this.token.increaseAllowance(spender, amount, { from: owner }); (await this.token.allowance(owner, spender)).should.be.bignumber.equal(amount); }); @@ -433,7 +382,7 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { }); it('increases the spender allowance adding the requested amount', async function () { - await this.token.increaseApproval(spender, amount, { from: owner }); + await this.token.increaseAllowance(spender, amount, { from: owner }); (await this.token.allowance(owner, spender)).should.be.bignumber.equal(amount + 1); }); @@ -444,20 +393,8 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { describe('when the spender is the zero address', function () { const spender = ZERO_ADDRESS; - it('approves the requested amount', async function () { - await this.token.increaseApproval(spender, amount, { from: owner }); - - (await this.token.allowance(owner, spender)).should.be.bignumber.equal(amount); - }); - - it('emits an approval event', async function () { - const { logs } = await this.token.increaseApproval(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); + it('reverts', async function () { + await assertRevert(this.token.increaseAllowance(spender, amount, { from: owner })); }); }); }); diff --git a/test/token/ERC20/ERC20Pausable.test.js b/test/token/ERC20/ERC20Pausable.test.js index 59eb5ce3877..568b7f147cb 100644 --- a/test/token/ERC20/ERC20Pausable.test.js +++ b/test/token/ERC20/ERC20Pausable.test.js @@ -196,7 +196,7 @@ contract('ERC20Pausable', function ([_, pauser, otherPauser, recipient, anotherA }); it('allows to decrease approval when unpaused', async function () { - await this.token.decreaseApproval(anotherAccount, 40, { from: pauser }); + await this.token.decreaseAllowance(anotherAccount, 40, { from: pauser }); (await this.token.allowance(pauser, anotherAccount)).should.be.bignumber.equal(60); }); @@ -205,7 +205,7 @@ contract('ERC20Pausable', function ([_, pauser, otherPauser, recipient, anotherA await this.token.pause({ from: pauser }); await this.token.unpause({ from: pauser }); - await this.token.decreaseApproval(anotherAccount, 40, { from: pauser }); + await this.token.decreaseAllowance(anotherAccount, 40, { from: pauser }); (await this.token.allowance(pauser, anotherAccount)).should.be.bignumber.equal(60); }); @@ -213,7 +213,7 @@ contract('ERC20Pausable', function ([_, pauser, otherPauser, recipient, anotherA it('reverts when trying to transfer when paused', async function () { await this.token.pause({ from: pauser }); - await assertRevert(this.token.decreaseApproval(anotherAccount, 40, { from: pauser })); + await assertRevert(this.token.decreaseAllowance(anotherAccount, 40, { from: pauser })); }); }); @@ -223,7 +223,7 @@ contract('ERC20Pausable', function ([_, pauser, otherPauser, recipient, anotherA }); it('allows to increase approval when unpaused', async function () { - await this.token.increaseApproval(anotherAccount, 40, { from: pauser }); + await this.token.increaseAllowance(anotherAccount, 40, { from: pauser }); (await this.token.allowance(pauser, anotherAccount)).should.be.bignumber.equal(140); }); @@ -232,7 +232,7 @@ contract('ERC20Pausable', function ([_, pauser, otherPauser, recipient, anotherA await this.token.pause({ from: pauser }); await this.token.unpause({ from: pauser }); - await this.token.increaseApproval(anotherAccount, 40, { from: pauser }); + await this.token.increaseAllowance(anotherAccount, 40, { from: pauser }); (await this.token.allowance(pauser, anotherAccount)).should.be.bignumber.equal(140); }); @@ -240,7 +240,7 @@ contract('ERC20Pausable', function ([_, pauser, otherPauser, recipient, anotherA it('reverts when trying to increase approval when paused', async function () { await this.token.pause({ from: pauser }); - await assertRevert(this.token.increaseApproval(anotherAccount, 40, { from: pauser })); + await assertRevert(this.token.increaseAllowance(anotherAccount, 40, { from: pauser })); }); }); });