diff --git a/contracts/mocks/SafeERC20Helper.sol b/contracts/mocks/SafeERC20Helper.sol index 9c0af1312..838a2512c 100644 --- a/contracts/mocks/SafeERC20Helper.sol +++ b/contracts/mocks/SafeERC20Helper.sol @@ -3,24 +3,38 @@ pragma solidity ^0.4.24; import "../token/ERC20/IERC20.sol"; import "../token/ERC20/SafeERC20.sol"; -contract ERC20FailingMock { - uint256 private _allowance; +contract ERC20FailingMock is IERC20 { + function totalSupply() public view returns (uint256) { + return 0; + } - function transfer(address, uint256) public returns (bool) { - return false; - } + function transfer(address, uint256) public returns (bool) { + return false; + } - function transferFrom(address, address, uint256) public returns (bool) { - return false; - } + function transferFrom(address, address, uint256) public returns (bool) { + return false; + } - function approve(address, uint256) public returns (bool) { - return false; - } + function approve(address, uint256) public returns (bool) { + return false; + } - function allowance(address, address) public view returns (uint256) { - return 0; - } + function increaseAllowance(address, uint256) public returns (bool){ + return false; + } + + function decreaseAllowance(address, uint256) public returns (bool){ + return false; + } + + function balanceOf(address) public view returns (uint256) { + return 0; + } + + function allowance(address, address) public view returns (uint256) { + return 0; + } } contract ERC20SucceedingMock { @@ -38,9 +52,17 @@ contract ERC20SucceedingMock { return true; } - function setAllowance(uint256 allowance_) public { - _allowance = allowance_; - } + function increaseAllowance(address, uint256) public returns (bool){ + return true; + } + + function decreaseAllowance(address, uint256) public returns (bool){ + return true; + } + + function balanceOf(address) public view returns (uint256) { + return 0; + } function allowance(address, address) public view returns (uint256) { return _allowance; @@ -98,15 +120,31 @@ contract SafeERC20Helper { _succeeding.safeIncreaseAllowance(address(0), amount); } - function doSucceedingDecreaseAllowance(uint256 amount) public { - _succeeding.safeDecreaseAllowance(address(0), amount); - } + function doFailingIncreaseAllowance() public { + _failing.safeIncreaseAllowance(address(0), 0); + } + + function doFailingDecreaseAllowance() public { + _failing.safeDecreaseAllowance(address(0), 0); + } + + function doSucceedingTransfer() public { + _succeeding.safeTransfer(address(0), 0); + } function setAllowance(uint256 allowance_) public { ERC20SucceedingMock(_succeeding).setAllowance(allowance_); } - function allowance() public view returns (uint256) { - return _succeeding.allowance(address(0), address(0)); - } + function doSucceedingApprove() public { + _succeeding.safeApprove(address(0), 0); + } + + function doSucceedingIncreaseAllowance() public { + _succeeding.safeIncreaseAllowance(address(0), 0); + } + + function doSucceedingDecreaseAllowance() public { + _succeeding.safeDecreaseAllowance(address(0), 0); + } } diff --git a/contracts/token/ERC20/IERC20.sol b/contracts/token/ERC20/IERC20.sol index 7c44280f0..96c689463 100644 --- a/contracts/token/ERC20/IERC20.sol +++ b/contracts/token/ERC20/IERC20.sol @@ -17,7 +17,17 @@ interface IERC20 { function transferFrom(address from, address to, uint256 value) external returns (bool); - event Transfer(address indexed from, address indexed to, uint256 value); + function increaseAllowance(address spender, uint256 addedValue) + external returns (bool); + + function decreaseAllowance(address spender, uint256 subtractedValue) + external returns (bool); + + event Transfer( + address indexed from, + address indexed to, + uint256 value + ); event Approval(address indexed owner, address indexed spender, uint256 value); } diff --git a/contracts/token/ERC20/SafeERC20.sol b/contracts/token/ERC20/SafeERC20.sol index adb217e0b..473c2c431 100644 --- a/contracts/token/ERC20/SafeERC20.sol +++ b/contracts/token/ERC20/SafeERC20.sol @@ -16,25 +16,34 @@ library SafeERC20 { require(token.transfer(to, value)); } - function safeTransferFrom(IERC20 token, address from, address to, uint256 value) internal { - require(token.transferFrom(from, to, value)); - } + function safeApprove( + IERC20 token, + address spender, + uint256 value + ) + internal + { + require((value == 0) || (token.allowance(msg.sender, spender) == 0)); + require(token.approve(spender, value)); + } - function safeApprove(IERC20 token, address spender, uint256 value) internal { - // safeApprove should only be called when setting an initial allowance, - // or when resetting it to zero. To increase and decrease it, use - // 'safeIncreaseAllowance' and 'safeDecreaseAllowance' - require((value == 0) || (token.allowance(msg.sender, spender) == 0)); - require(token.approve(spender, value)); - } + function safeIncreaseAllowance( + IERC20 token, + address spender, + uint256 addedValue + ) + internal + { + require(token.increaseAllowance(spender, addedValue)); + } - function safeIncreaseAllowance(IERC20 token, address spender, uint256 value) internal { - uint256 newAllowance = token.allowance(address(this), spender).add(value); - require(token.approve(spender, newAllowance)); - } - - function safeDecreaseAllowance(IERC20 token, address spender, uint256 value) internal { - uint256 newAllowance = token.allowance(address(this), spender).sub(value); - require(token.approve(spender, newAllowance)); - } + function safeDecreaseAllowance( + IERC20 token, + address spender, + uint256 subtractedValue + ) + internal + { + require(token.decreaseAllowance(spender, subtractedValue)); + } } diff --git a/test/crowdsale/AllowanceCrowdsale.test.js b/test/crowdsale/AllowanceCrowdsale.test.js index 048e4d9fa..4a53c5f97 100644 --- a/test/crowdsale/AllowanceCrowdsale.test.js +++ b/test/crowdsale/AllowanceCrowdsale.test.js @@ -42,7 +42,7 @@ contract('AllowanceCrowdsale', function ([_, investor, wallet, purchaser, tokenW purchaser: investor, beneficiary: investor, value: value, - amount: expectedTokenAmount + amount: expectedTokenAmount, }); }); diff --git a/test/crowdsale/Crowdsale.test.js b/test/crowdsale/Crowdsale.test.js index 53721fcf3..dbd33f1aa 100644 --- a/test/crowdsale/Crowdsale.test.js +++ b/test/crowdsale/Crowdsale.test.js @@ -83,7 +83,7 @@ contract('Crowdsale', function ([_, investor, wallet, purchaser]) { purchaser: investor, beneficiary: investor, value: value, - amount: expectedTokenAmount + amount: expectedTokenAmount, }); }); @@ -106,7 +106,7 @@ contract('Crowdsale', function ([_, investor, wallet, purchaser]) { purchaser: purchaser, beneficiary: investor, value: value, - amount: expectedTokenAmount + amount: expectedTokenAmount, }); }); diff --git a/test/crowdsale/MintedCrowdsale.behavior.js b/test/crowdsale/MintedCrowdsale.behavior.js index 8fee5b35b..ec41e4948 100644 --- a/test/crowdsale/MintedCrowdsale.behavior.js +++ b/test/crowdsale/MintedCrowdsale.behavior.js @@ -21,7 +21,7 @@ function shouldBehaveLikeMintedCrowdsale ([_, investor, wallet, purchaser], rate purchaser: investor, beneficiary: investor, value: value, - amount: expectedTokenAmount + amount: expectedTokenAmount, }); }); diff --git a/test/payment/escrow/Escrow.behavior.js b/test/payment/escrow/Escrow.behavior.js index 08f032472..80bde2f5c 100644 --- a/test/payment/escrow/Escrow.behavior.js +++ b/test/payment/escrow/Escrow.behavior.js @@ -31,7 +31,7 @@ function shouldBehaveLikeEscrow (primary, [payee1, payee2]) { const { logs } = await this.escrow.deposit(payee1, { from: primary, value: amount }); expectEvent.inLogs(logs, 'Deposited', { payee: payee1, - weiAmount: amount + weiAmount: amount, }); }); @@ -80,7 +80,7 @@ function shouldBehaveLikeEscrow (primary, [payee1, payee2]) { const { logs } = await this.escrow.withdraw(payee1, { from: primary }); expectEvent.inLogs(logs, 'Withdrawn', { payee: payee1, - weiAmount: amount + weiAmount: amount, }); }); }); diff --git a/test/token/ERC20/ERC20.test.js b/test/token/ERC20/ERC20.test.js index 3ddd1362a..16d2cab75 100644 --- a/test/token/ERC20/ERC20.test.js +++ b/test/token/ERC20/ERC20.test.js @@ -60,7 +60,7 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { expectEvent.inLogs(logs, 'Transfer', { from: owner, to: to, - value: amount + value: amount, }); }); }); @@ -88,7 +88,7 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { expectEvent.inLogs(logs, 'Approval', { owner: owner, spender: spender, - value: amount + value: amount, }); }); @@ -122,7 +122,7 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { expectEvent.inLogs(logs, 'Approval', { owner: owner, spender: spender, - value: amount + value: amount, }); }); @@ -192,7 +192,7 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { expectEvent.inLogs(logs, 'Transfer', { from: owner, to: to, - value: amount + value: amount, }); }); @@ -277,7 +277,7 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { expectEvent.inLogs(logs, 'Approval', { owner: owner, spender: spender, - value: 0 + value: 0, }); }); @@ -334,7 +334,7 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { expectEvent.inLogs(logs, 'Approval', { owner: owner, spender: spender, - value: amount + value: amount, }); }); @@ -368,7 +368,7 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { expectEvent.inLogs(logs, 'Approval', { owner: owner, spender: spender, - value: amount + value: amount, }); }); diff --git a/test/token/ERC20/SafeERC20.test.js b/test/token/ERC20/SafeERC20.test.js index 4be44cf38..b1308a5b7 100644 --- a/test/token/ERC20/SafeERC20.test.js +++ b/test/token/ERC20/SafeERC20.test.js @@ -22,9 +22,17 @@ contract('SafeERC20', function () { await shouldFail.reverting(this.helper.doFailingApprove()); }); - it('reverts on increaseAllowance', async function () { - await shouldFail.reverting(this.helper.doFailingIncreaseAllowance()); - }); + it('should throw on failed increaseAllowance', async function () { + await shouldFail.reverting(this.helper.doFailingIncreaseAllowance()); + }); + + it('should throw on failed decreaseAllowance', async function () { + await shouldFail.reverting(this.helper.doFailingDecreaseAllowance()); + }); + + it('should not throw on succeeding transfer', async function () { + await this.helper.doSucceedingTransfer(); + }); it('reverts on decreaseAllowance', async function () { await shouldFail.reverting(this.helper.doFailingDecreaseAllowance()); @@ -90,4 +98,12 @@ contract('SafeERC20', function () { }); }); }); + + it('should not throw on succeeding increaseAllowance', async function () { + await this.helper.doSucceedingIncreaseAllowance(); + }); + + it('should not throw on succeeding decreaseAllowance', async function () { + await this.helper.doSucceedingDecreaseAllowance(); + }); }); diff --git a/test/token/ERC20/behaviors/ERC20Burnable.behavior.js b/test/token/ERC20/behaviors/ERC20Burnable.behavior.js index 26f2ee741..6d1c7195b 100644 --- a/test/token/ERC20/behaviors/ERC20Burnable.behavior.js +++ b/test/token/ERC20/behaviors/ERC20Burnable.behavior.js @@ -28,7 +28,7 @@ function shouldBehaveLikeERC20Burnable (owner, initialBalance, [burner]) { expectEvent.inLogs(this.logs, 'Transfer', { from: owner, to: ZERO_ADDRESS, - value: amount + value: amount, }); }); } @@ -74,7 +74,7 @@ function shouldBehaveLikeERC20Burnable (owner, initialBalance, [burner]) { expectEvent.inLogs(this.logs, 'Transfer', { from: owner, to: ZERO_ADDRESS, - value: amount + value: amount, }); }); } diff --git a/test/token/ERC20/behaviors/ERC20Mintable.behavior.js b/test/token/ERC20/behaviors/ERC20Mintable.behavior.js index 933487071..987b67752 100644 --- a/test/token/ERC20/behaviors/ERC20Mintable.behavior.js +++ b/test/token/ERC20/behaviors/ERC20Mintable.behavior.js @@ -33,7 +33,7 @@ function shouldBehaveLikeERC20Mintable (minter, [anyone]) { expectEvent.inLogs(this.logs, 'Transfer', { from: ZERO_ADDRESS, to: anyone, - value: amount + value: amount, }); }); } diff --git a/test/token/ERC721/ERC721.behavior.js b/test/token/ERC721/ERC721.behavior.js index 7850464b2..b7600872c 100644 --- a/test/token/ERC721/ERC721.behavior.js +++ b/test/token/ERC721/ERC721.behavior.js @@ -87,7 +87,7 @@ function shouldBehaveLikeERC721 ( expectEvent.inLogs(logs, 'Transfer', { from: owner, to: this.toWhom, - tokenId: tokenId + tokenId: tokenId, }); }); } else { @@ -95,7 +95,7 @@ function shouldBehaveLikeERC721 ( expectEvent.inLogs(logs, 'Transfer', { from: owner, to: this.toWhom, - tokenId: tokenId + tokenId: tokenId, }); }); } @@ -160,7 +160,7 @@ function shouldBehaveLikeERC721 ( expectEvent.inLogs(logs, 'Transfer', { from: owner, to: owner, - tokenId: tokenId + tokenId: tokenId, }); }); @@ -336,7 +336,7 @@ function shouldBehaveLikeERC721 ( expectEvent.inLogs(logs, 'Approval', { owner: owner, approved: address, - tokenId: tokenId + tokenId: tokenId, }); }); }; @@ -446,7 +446,7 @@ function shouldBehaveLikeERC721 ( expectEvent.inLogs(logs, 'ApprovalForAll', { owner: owner, operator: operator, - approved: true + approved: true, }); }); }); @@ -468,7 +468,7 @@ function shouldBehaveLikeERC721 ( expectEvent.inLogs(logs, 'ApprovalForAll', { owner: owner, operator: operator, - approved: true + approved: true, }); }); @@ -496,7 +496,7 @@ function shouldBehaveLikeERC721 ( expectEvent.inLogs(logs, 'ApprovalForAll', { owner: owner, operator: operator, - approved: true + approved: true, }); }); }); diff --git a/test/token/ERC721/ERC721MintBurn.behavior.js b/test/token/ERC721/ERC721MintBurn.behavior.js index dc3f496c7..97c33ae36 100644 --- a/test/token/ERC721/ERC721MintBurn.behavior.js +++ b/test/token/ERC721/ERC721MintBurn.behavior.js @@ -41,7 +41,7 @@ function shouldBehaveLikeMintAndBurnERC721 ( expectEvent.inLogs(logs, 'Transfer', { from: ZERO_ADDRESS, to: newOwner, - tokenId: thirdTokenId + tokenId: thirdTokenId, }); }); }); @@ -86,7 +86,7 @@ function shouldBehaveLikeMintAndBurnERC721 ( expectEvent.inLogs(logs, 'Transfer', { from: owner, to: ZERO_ADDRESS, - tokenId: tokenId + tokenId: tokenId, }); }); });