From 9e1c934ffdbae90c81fe313a1bc9a32df5adb7b7 Mon Sep 17 00:00:00 2001 From: Matt Condon Date: Tue, 10 Apr 2018 20:31:29 -0300 Subject: [PATCH] Various fixes and formatting chores (#885) * fix: clean up solium linting errors * fix: make various contracts natspec compliant * fix: this.balance deprecated; convert to address(this).balance * fix: contract.call deprecated and switch to gasleft() * fix: ignore empty block rule project-wide * fix: add ignore cases for the rest of the linting warnings --- .soliumrc.json | 1 + contracts/Bounty.sol | 2 +- contracts/DayLimit.sol | 1 + contracts/ECRecovery.sol | 1 + contracts/LimitBalance.sol | 2 +- contracts/ReentrancyGuard.sol | 8 +- contracts/crowdsale/Crowdsale.sol | 7 +- .../distribution/utils/RefundVault.sol | 2 +- .../price/IncreasingPriceCrowdsale.sol | 1 + .../crowdsale/validation/TimedCrowdsale.sol | 3 + contracts/examples/SimpleSavingsWallet.sol | 4 +- contracts/mocks/ERC223TokenMock.sol | 1 + contracts/mocks/ERC721ReceiverMock.sol | 16 +- contracts/mocks/MessageHelper.sol | 1 + contracts/mocks/ReentrancyAttack.sol | 1 + contracts/mocks/ReentrancyMock.sol | 3 +- contracts/ownership/HasNoEther.sol | 3 +- contracts/ownership/Heritable.sol | 2 + contracts/payment/PullPayment.sol | 2 +- contracts/payment/SplitPayment.sol | 4 +- contracts/token/ERC20/TokenTimelock.sol | 2 + contracts/token/ERC20/TokenVesting.sol | 2 + contracts/token/ERC721/ERC721BasicToken.sol | 174 +++++++++--------- contracts/token/ERC721/ERC721Token.sol | 96 +++++----- contracts/token/ERC827/ERC827.sol | 16 +- contracts/token/ERC827/ERC827Token.sol | 101 +++++----- 26 files changed, 248 insertions(+), 208 deletions(-) diff --git a/.soliumrc.json b/.soliumrc.json index 681b33f27..12ebcfed8 100644 --- a/.soliumrc.json +++ b/.soliumrc.json @@ -3,6 +3,7 @@ "plugins": ["security"], "rules": { "quotes": ["error", "double"], + "no-empty-blocks": "off", "indentation": ["error", 2], "arg-overflow": ["warning", 3], "security/enforce-explicit-visibility": ["error"], diff --git a/contracts/Bounty.sol b/contracts/Bounty.sol index f1fe4071b..2f9879d96 100644 --- a/contracts/Bounty.sol +++ b/contracts/Bounty.sol @@ -43,7 +43,7 @@ contract Bounty is PullPayment, Destructible { require(researcher != 0); // Check Target contract invariants require(!target.checkInvariant()); - asyncSend(researcher, this.balance); + asyncSend(researcher, address(this).balance); claimed = true; } diff --git a/contracts/DayLimit.sol b/contracts/DayLimit.sol index 7a2e51300..0bfa9b6d0 100644 --- a/contracts/DayLimit.sol +++ b/contracts/DayLimit.sol @@ -61,6 +61,7 @@ contract DayLimit { * @return uint256 of today's index. */ function today() private view returns (uint256) { + // solium-disable-next-line security/no-block-members return block.timestamp / 1 days; } diff --git a/contracts/ECRecovery.sol b/contracts/ECRecovery.sol index 41d8703ea..0c82a50d9 100644 --- a/contracts/ECRecovery.sol +++ b/contracts/ECRecovery.sol @@ -47,6 +47,7 @@ library ECRecovery { if (v != 27 && v != 28) { return (address(0)); } else { + // solium-disable-next-line arg-overflow return ecrecover(hash, v, r, s); } } diff --git a/contracts/LimitBalance.sol b/contracts/LimitBalance.sol index a86877191..e3f28cac1 100644 --- a/contracts/LimitBalance.sol +++ b/contracts/LimitBalance.sol @@ -23,7 +23,7 @@ contract LimitBalance { * @dev Checks if limit was reached. Case true, it throws. */ modifier limitedPayable() { - require(this.balance <= limit); + require(address(this).balance <= limit); _; } diff --git a/contracts/ReentrancyGuard.sol b/contracts/ReentrancyGuard.sol index c0f3a04c0..1b55ceebc 100644 --- a/contracts/ReentrancyGuard.sol +++ b/contracts/ReentrancyGuard.sol @@ -12,7 +12,7 @@ contract ReentrancyGuard { /** * @dev We use a single lock for the whole contract. */ - bool private reentrancy_lock = false; + bool private reentrancyLock = false; /** * @dev Prevents a contract from calling itself, directly or indirectly. @@ -23,10 +23,10 @@ contract ReentrancyGuard { * wrapper marked as `nonReentrant`. */ modifier nonReentrant() { - require(!reentrancy_lock); - reentrancy_lock = true; + require(!reentrancyLock); + reentrancyLock = true; _; - reentrancy_lock = false; + reentrancyLock = false; } } diff --git a/contracts/crowdsale/Crowdsale.sol b/contracts/crowdsale/Crowdsale.sol index 4ba4f30bc..cbb59eabc 100644 --- a/contracts/crowdsale/Crowdsale.sol +++ b/contracts/crowdsale/Crowdsale.sol @@ -82,7 +82,12 @@ contract Crowdsale { weiRaised = weiRaised.add(weiAmount); _processPurchase(_beneficiary, tokens); - emit TokenPurchase(msg.sender, _beneficiary, weiAmount, tokens); + emit TokenPurchase( + msg.sender, + _beneficiary, + weiAmount, + tokens + ); _updatePurchasingState(_beneficiary, weiAmount); diff --git a/contracts/crowdsale/distribution/utils/RefundVault.sol b/contracts/crowdsale/distribution/utils/RefundVault.sol index 957417fdf..17a69b08c 100644 --- a/contracts/crowdsale/distribution/utils/RefundVault.sol +++ b/contracts/crowdsale/distribution/utils/RefundVault.sol @@ -44,7 +44,7 @@ contract RefundVault is Ownable { require(state == State.Active); state = State.Closed; emit Closed(); - wallet.transfer(this.balance); + wallet.transfer(address(this).balance); } function enableRefunds() onlyOwner public { diff --git a/contracts/crowdsale/price/IncreasingPriceCrowdsale.sol b/contracts/crowdsale/price/IncreasingPriceCrowdsale.sol index d0ae65ae2..c9f773bf4 100644 --- a/contracts/crowdsale/price/IncreasingPriceCrowdsale.sol +++ b/contracts/crowdsale/price/IncreasingPriceCrowdsale.sol @@ -34,6 +34,7 @@ contract IncreasingPriceCrowdsale is TimedCrowdsale { * @return The number of tokens a buyer gets per wei at a given time */ function getCurrentRate() public view returns (uint256) { + // solium-disable-next-line security/no-block-members uint256 elapsedTime = block.timestamp.sub(openingTime); uint256 timeRange = closingTime.sub(openingTime); uint256 rateRange = initialRate.sub(finalRate); diff --git a/contracts/crowdsale/validation/TimedCrowdsale.sol b/contracts/crowdsale/validation/TimedCrowdsale.sol index 8a4ef1ce4..9977990a7 100644 --- a/contracts/crowdsale/validation/TimedCrowdsale.sol +++ b/contracts/crowdsale/validation/TimedCrowdsale.sol @@ -18,6 +18,7 @@ contract TimedCrowdsale is Crowdsale { * @dev Reverts if not in crowdsale time range. */ modifier onlyWhileOpen { + // solium-disable-next-line security/no-block-members require(block.timestamp >= openingTime && block.timestamp <= closingTime); _; } @@ -28,6 +29,7 @@ contract TimedCrowdsale is Crowdsale { * @param _closingTime Crowdsale closing time */ function TimedCrowdsale(uint256 _openingTime, uint256 _closingTime) public { + // solium-disable-next-line security/no-block-members require(_openingTime >= block.timestamp); require(_closingTime >= _openingTime); @@ -40,6 +42,7 @@ contract TimedCrowdsale is Crowdsale { * @return Whether crowdsale period has elapsed */ function hasClosed() public view returns (bool) { + // solium-disable-next-line security/no-block-members return block.timestamp > closingTime; } diff --git a/contracts/examples/SimpleSavingsWallet.sol b/contracts/examples/SimpleSavingsWallet.sol index 98fdbf784..ea02acec2 100644 --- a/contracts/examples/SimpleSavingsWallet.sol +++ b/contracts/examples/SimpleSavingsWallet.sol @@ -25,7 +25,7 @@ contract SimpleSavingsWallet is Heritable { * @dev wallet can receive funds. */ function () public payable { - emit Received(msg.sender, msg.value, this.balance); + emit Received(msg.sender, msg.value, address(this).balance); } /** @@ -35,6 +35,6 @@ contract SimpleSavingsWallet is Heritable { require(payee != 0 && payee != address(this)); require(amount > 0); payee.transfer(amount); - emit Sent(payee, amount, this.balance); + emit Sent(payee, amount, address(this).balance); } } diff --git a/contracts/mocks/ERC223TokenMock.sol b/contracts/mocks/ERC223TokenMock.sol index d7d0aefed..fbe47fcc8 100644 --- a/contracts/mocks/ERC223TokenMock.sol +++ b/contracts/mocks/ERC223TokenMock.sol @@ -21,6 +21,7 @@ contract ERC223TokenMock is BasicToken { { transfer(_to, _value); bool isContract = false; + // solium-disable-next-line security/no-inline-assembly assembly { isContract := not(iszero(extcodesize(_to))) } diff --git a/contracts/mocks/ERC721ReceiverMock.sol b/contracts/mocks/ERC721ReceiverMock.sol index 1a968647e..ec9f02c11 100644 --- a/contracts/mocks/ERC721ReceiverMock.sol +++ b/contracts/mocks/ERC721ReceiverMock.sol @@ -14,9 +14,21 @@ contract ERC721ReceiverMock is ERC721Receiver { reverts = _reverts; } - function onERC721Received(address _address, uint256 _tokenId, bytes _data) public returns(bytes4) { + function onERC721Received( + address _address, + uint256 _tokenId, + bytes _data + ) + public + returns(bytes4) + { require(!reverts); - emit Received(_address, _tokenId, _data, msg.gas); + emit Received( + _address, + _tokenId, + _data, + gasleft() // msg.gas was deprecated in solidityv0.4.21 + ); return retval; } } diff --git a/contracts/mocks/MessageHelper.sol b/contracts/mocks/MessageHelper.sol index 7cd1ea291..b44b6496a 100644 --- a/contracts/mocks/MessageHelper.sol +++ b/contracts/mocks/MessageHelper.sol @@ -15,6 +15,7 @@ contract MessageHelper { } function call(address to, bytes data) public returns (bool) { + // solium-disable-next-line security/no-low-level-calls if (to.call(data)) return true; else diff --git a/contracts/mocks/ReentrancyAttack.sol b/contracts/mocks/ReentrancyAttack.sol index a66f5fca3..5caebb1d6 100644 --- a/contracts/mocks/ReentrancyAttack.sol +++ b/contracts/mocks/ReentrancyAttack.sol @@ -4,6 +4,7 @@ pragma solidity ^0.4.21; contract ReentrancyAttack { function callSender(bytes4 data) public { + // solium-disable-next-line security/no-low-level-calls require(msg.sender.call(data)); } diff --git a/contracts/mocks/ReentrancyMock.sol b/contracts/mocks/ReentrancyMock.sol index 856a13755..72f4a284f 100644 --- a/contracts/mocks/ReentrancyMock.sol +++ b/contracts/mocks/ReentrancyMock.sol @@ -27,7 +27,8 @@ contract ReentrancyMock is ReentrancyGuard { bytes4 func = bytes4(keccak256("countThisRecursive(uint256)")); if (n > 0) { count(); - bool result = this.call(func, n - 1); + // solium-disable-next-line security/no-low-level-calls + bool result = address(this).call(func, n - 1); require(result == true); } } diff --git a/contracts/ownership/HasNoEther.sol b/contracts/ownership/HasNoEther.sol index beec3611d..6cce9858c 100644 --- a/contracts/ownership/HasNoEther.sol +++ b/contracts/ownership/HasNoEther.sol @@ -36,6 +36,7 @@ contract HasNoEther is Ownable { * @dev Transfer all Ether held by the contract to the owner. */ function reclaimEther() external onlyOwner { - assert(owner.send(this.balance)); + // solium-disable-next-line security/no-send + assert(owner.send(address(this).balance)); } } diff --git a/contracts/ownership/Heritable.sol b/contracts/ownership/Heritable.sol index 2a69e215c..3b592a0a5 100644 --- a/contracts/ownership/Heritable.sol +++ b/contracts/ownership/Heritable.sol @@ -81,6 +81,7 @@ contract Heritable is Ownable { function proclaimDeath() public onlyHeir { require(ownerLives()); emit OwnerProclaimedDead(owner, heir_, timeOfDeath_); + // solium-disable-next-line security/no-block-members timeOfDeath_ = block.timestamp; } @@ -97,6 +98,7 @@ contract Heritable is Ownable { */ function claimHeirOwnership() public onlyHeir { require(!ownerLives()); + // solium-disable-next-line security/no-block-members require(block.timestamp >= timeOfDeath_ + heartbeatTimeout_); emit OwnershipTransferred(owner, heir_); emit HeirOwnershipClaimed(owner, heir_); diff --git a/contracts/payment/PullPayment.sol b/contracts/payment/PullPayment.sol index 1e5806ea7..030e971b3 100644 --- a/contracts/payment/PullPayment.sol +++ b/contracts/payment/PullPayment.sol @@ -23,7 +23,7 @@ contract PullPayment { uint256 payment = payments[payee]; require(payment != 0); - require(this.balance >= payment); + require(address(this).balance >= payment); totalPayments = totalPayments.sub(payment); payments[payee] = 0; diff --git a/contracts/payment/SplitPayment.sol b/contracts/payment/SplitPayment.sol index 8bb3a70f1..019eba694 100644 --- a/contracts/payment/SplitPayment.sol +++ b/contracts/payment/SplitPayment.sol @@ -42,11 +42,11 @@ contract SplitPayment { require(shares[payee] > 0); - uint256 totalReceived = this.balance.add(totalReleased); + uint256 totalReceived = address(this).balance.add(totalReleased); uint256 payment = totalReceived.mul(shares[payee]).div(totalShares).sub(released[payee]); require(payment != 0); - require(this.balance >= payment); + require(address(this).balance >= payment); released[payee] = released[payee].add(payment); totalReleased = totalReleased.add(payment); diff --git a/contracts/token/ERC20/TokenTimelock.sol b/contracts/token/ERC20/TokenTimelock.sol index 990eef2e3..a14d904b5 100644 --- a/contracts/token/ERC20/TokenTimelock.sol +++ b/contracts/token/ERC20/TokenTimelock.sol @@ -21,6 +21,7 @@ contract TokenTimelock { uint256 public releaseTime; function TokenTimelock(ERC20Basic _token, address _beneficiary, uint256 _releaseTime) public { + // solium-disable-next-line security/no-block-members require(_releaseTime > block.timestamp); token = _token; beneficiary = _beneficiary; @@ -31,6 +32,7 @@ contract TokenTimelock { * @notice Transfers tokens held by timelock to beneficiary. */ function release() public { + // solium-disable-next-line security/no-block-members require(block.timestamp >= releaseTime); uint256 amount = token.balanceOf(this); diff --git a/contracts/token/ERC20/TokenVesting.sol b/contracts/token/ERC20/TokenVesting.sol index 7f5b4fb9b..8e902eac7 100644 --- a/contracts/token/ERC20/TokenVesting.sol +++ b/contracts/token/ERC20/TokenVesting.sol @@ -1,3 +1,5 @@ +/* solium-disable security/no-block-members */ + pragma solidity ^0.4.21; import "./ERC20Basic.sol"; diff --git a/contracts/token/ERC721/ERC721BasicToken.sol b/contracts/token/ERC721/ERC721BasicToken.sol index d806ec865..b7bcc8297 100644 --- a/contracts/token/ERC721/ERC721BasicToken.sol +++ b/contracts/token/ERC721/ERC721BasicToken.sol @@ -31,38 +31,38 @@ contract ERC721BasicToken is ERC721Basic { mapping (address => mapping (address => bool)) internal operatorApprovals; /** - * @dev Guarantees msg.sender is owner of the given token - * @param _tokenId uint256 ID of the token to validate its ownership belongs to msg.sender - */ + * @dev Guarantees msg.sender is owner of the given token + * @param _tokenId uint256 ID of the token to validate its ownership belongs to msg.sender + */ modifier onlyOwnerOf(uint256 _tokenId) { require(ownerOf(_tokenId) == msg.sender); _; } /** - * @dev Checks msg.sender can transfer a token, by being owner, approved, or operator - * @param _tokenId uint256 ID of the token to validate - */ + * @dev Checks msg.sender can transfer a token, by being owner, approved, or operator + * @param _tokenId uint256 ID of the token to validate + */ modifier canTransfer(uint256 _tokenId) { require(isApprovedOrOwner(msg.sender, _tokenId)); _; } /** - * @dev Gets the balance of the specified address - * @param _owner address to query the balance of - * @return uint256 representing the amount owned by the passed address - */ + * @dev Gets the balance of the specified address + * @param _owner address to query the balance of + * @return uint256 representing the amount owned by the passed address + */ function balanceOf(address _owner) public view returns (uint256) { require(_owner != address(0)); return ownedTokensCount[_owner]; } /** - * @dev Gets the owner of the specified token ID - * @param _tokenId uint256 ID of the token to query the owner of - * @return owner address currently marked as the owner of the given token ID - */ + * @dev Gets the owner of the specified token ID + * @param _tokenId uint256 ID of the token to query the owner of + * @return owner address currently marked as the owner of the given token ID + */ function ownerOf(uint256 _tokenId) public view returns (address) { address owner = tokenOwner[_tokenId]; require(owner != address(0)); @@ -70,23 +70,23 @@ contract ERC721BasicToken is ERC721Basic { } /** - * @dev Returns whether the specified token exists - * @param _tokenId uint256 ID of the token to query the existance of - * @return whether the token exists - */ + * @dev Returns whether the specified token exists + * @param _tokenId uint256 ID of the token to query the existance of + * @return whether the token exists + */ function exists(uint256 _tokenId) public view returns (bool) { address owner = tokenOwner[_tokenId]; return owner != address(0); } /** - * @dev Approves another address to transfer the given token ID - * @dev The zero address indicates there is no approved address. - * @dev There can only be one approved address per token at a given time. - * @dev Can only be called by the token owner or an approved operator. - * @param _to address to be approved for the given token ID - * @param _tokenId uint256 ID of the token to be approved - */ + * @dev Approves another address to transfer the given token ID + * @dev The zero address indicates there is no approved address. + * @dev There can only be one approved address per token at a given time. + * @dev Can only be called by the token owner or an approved operator. + * @param _to address to be approved for the given token ID + * @param _tokenId uint256 ID of the token to be approved + */ function approve(address _to, uint256 _tokenId) public { address owner = ownerOf(_tokenId); require(_to != owner); @@ -108,11 +108,11 @@ contract ERC721BasicToken is ERC721Basic { } /** - * @dev Sets or unsets the approval of a given operator - * @dev An operator is allowed to transfer all tokens of the sender on their behalf - * @param _to operator address to set the approval - * @param _approved representing the status of the approval to be set - */ + * @dev Sets or unsets the approval of a given operator + * @dev An operator is allowed to transfer all tokens of the sender on their behalf + * @param _to operator address to set the approval + * @param _approved representing the status of the approval to be set + */ function setApprovalForAll(address _to, bool _approved) public { require(_to != msg.sender); operatorApprovals[msg.sender][_to] = _approved; @@ -130,12 +130,12 @@ contract ERC721BasicToken is ERC721Basic { } /** - * @dev Transfers the ownership of a given token ID to another address - * @dev Usage of this method is discouraged, use `safeTransferFrom` whenever possible - * @dev Requires the msg sender to be the owner, approved, or operator - * @param _from current owner of the token - * @param _to address to receive the ownership of the given token ID - * @param _tokenId uint256 ID of the token to be transferred + * @dev Transfers the ownership of a given token ID to another address + * @dev Usage of this method is discouraged, use `safeTransferFrom` whenever possible + * @dev Requires the msg sender to be the owner, approved, or operator + * @param _from current owner of the token + * @param _to address to receive the ownership of the given token ID + * @param _tokenId uint256 ID of the token to be transferred */ function transferFrom(address _from, address _to, uint256 _tokenId) public canTransfer(_tokenId) { require(_from != address(0)); @@ -149,15 +149,15 @@ contract ERC721BasicToken is ERC721Basic { } /** - * @dev Safely transfers the ownership of a given token ID to another address - * @dev If the target address is a contract, it must implement `onERC721Received`, - * which is called upon a safe transfer, and return the magic value - * `bytes4(keccak256("onERC721Received(address,uint256,bytes)"))`; otherwise, - * the transfer is reverted. - * @dev Requires the msg sender to be the owner, approved, or operator - * @param _from current owner of the token - * @param _to address to receive the ownership of the given token ID - * @param _tokenId uint256 ID of the token to be transferred + * @dev Safely transfers the ownership of a given token ID to another address + * @dev If the target address is a contract, it must implement `onERC721Received`, + * which is called upon a safe transfer, and return the magic value + * `bytes4(keccak256("onERC721Received(address,uint256,bytes)"))`; otherwise, + * the transfer is reverted. + * @dev Requires the msg sender to be the owner, approved, or operator + * @param _from current owner of the token + * @param _to address to receive the ownership of the given token ID + * @param _tokenId uint256 ID of the token to be transferred */ function safeTransferFrom( address _from, @@ -167,21 +167,22 @@ contract ERC721BasicToken is ERC721Basic { public canTransfer(_tokenId) { + // solium-disable-next-line arg-overflow safeTransferFrom(_from, _to, _tokenId, ""); } /** - * @dev Safely transfers the ownership of a given token ID to another address - * @dev If the target address is a contract, it must implement `onERC721Received`, - * which is called upon a safe transfer, and return the magic value - * `bytes4(keccak256("onERC721Received(address,uint256,bytes)"))`; otherwise, - * the transfer is reverted. - * @dev Requires the msg sender to be the owner, approved, or operator - * @param _from current owner of the token - * @param _to address to receive the ownership of the given token ID - * @param _tokenId uint256 ID of the token to be transferred - * @param _data bytes data to send along with a safe transfer check - */ + * @dev Safely transfers the ownership of a given token ID to another address + * @dev If the target address is a contract, it must implement `onERC721Received`, + * which is called upon a safe transfer, and return the magic value + * `bytes4(keccak256("onERC721Received(address,uint256,bytes)"))`; otherwise, + * the transfer is reverted. + * @dev Requires the msg sender to be the owner, approved, or operator + * @param _from current owner of the token + * @param _to address to receive the ownership of the given token ID + * @param _tokenId uint256 ID of the token to be transferred + * @param _data bytes data to send along with a safe transfer check + */ function safeTransferFrom( address _from, address _to, @@ -192,6 +193,7 @@ contract ERC721BasicToken is ERC721Basic { canTransfer(_tokenId) { transferFrom(_from, _to, _tokenId); + // solium-disable-next-line arg-overflow require(checkAndCallSafeTransfer(_from, _to, _tokenId, _data)); } @@ -208,11 +210,11 @@ contract ERC721BasicToken is ERC721Basic { } /** - * @dev Internal function to mint a new token - * @dev Reverts if the given token ID already exists - * @param _to The address that will own the minted token - * @param _tokenId uint256 ID of the token to be minted by the msg.sender - */ + * @dev Internal function to mint a new token + * @dev Reverts if the given token ID already exists + * @param _to The address that will own the minted token + * @param _tokenId uint256 ID of the token to be minted by the msg.sender + */ function _mint(address _to, uint256 _tokenId) internal { require(_to != address(0)); addTokenTo(_to, _tokenId); @@ -220,10 +222,10 @@ contract ERC721BasicToken is ERC721Basic { } /** - * @dev Internal function to burn a specific token - * @dev Reverts if the token does not exist - * @param _tokenId uint256 ID of the token being burned by the msg.sender - */ + * @dev Internal function to burn a specific token + * @dev Reverts if the token does not exist + * @param _tokenId uint256 ID of the token being burned by the msg.sender + */ function _burn(address _owner, uint256 _tokenId) internal { clearApproval(_owner, _tokenId); removeTokenFrom(_owner, _tokenId); @@ -231,11 +233,11 @@ contract ERC721BasicToken is ERC721Basic { } /** - * @dev Internal function to clear current approval of a given token ID - * @dev Reverts if the given address is not indeed the owner of the token - * @param _owner owner of the token - * @param _tokenId uint256 ID of the token to be transferred - */ + * @dev Internal function to clear current approval of a given token ID + * @dev Reverts if the given address is not indeed the owner of the token + * @param _owner owner of the token + * @param _tokenId uint256 ID of the token to be transferred + */ function clearApproval(address _owner, uint256 _tokenId) internal { require(ownerOf(_tokenId) == _owner); if (tokenApprovals[_tokenId] != address(0)) { @@ -245,10 +247,10 @@ contract ERC721BasicToken is ERC721Basic { } /** - * @dev Internal function to add a token ID to the list of a given address - * @param _to address representing the new owner of the given token ID - * @param _tokenId uint256 ID of the token to be added to the tokens list of the given address - */ + * @dev Internal function to add a token ID to the list of a given address + * @param _to address representing the new owner of the given token ID + * @param _tokenId uint256 ID of the token to be added to the tokens list of the given address + */ function addTokenTo(address _to, uint256 _tokenId) internal { require(tokenOwner[_tokenId] == address(0)); tokenOwner[_tokenId] = _to; @@ -256,10 +258,10 @@ contract ERC721BasicToken is ERC721Basic { } /** - * @dev Internal function to remove a token ID from the list of a given address - * @param _from address representing the previous owner of the given token ID - * @param _tokenId uint256 ID of the token to be removed from the tokens list of the given address - */ + * @dev Internal function to remove a token ID from the list of a given address + * @param _from address representing the previous owner of the given token ID + * @param _tokenId uint256 ID of the token to be removed from the tokens list of the given address + */ function removeTokenFrom(address _from, uint256 _tokenId) internal { require(ownerOf(_tokenId) == _from); ownedTokensCount[_from] = ownedTokensCount[_from].sub(1); @@ -267,14 +269,14 @@ contract ERC721BasicToken is ERC721Basic { } /** - * @dev Internal function to invoke `onERC721Received` on a target address - * @dev The call is not executed if the target address is not a contract - * @param _from address representing the previous owner of the given token ID - * @param _to target address that will receive the tokens - * @param _tokenId uint256 ID of the token to be transferred - * @param _data bytes optional data to send along with the call - * @return whether the call correctly returned the expected magic value - */ + * @dev Internal function to invoke `onERC721Received` on a target address + * @dev The call is not executed if the target address is not a contract + * @param _from address representing the previous owner of the given token ID + * @param _to target address that will receive the tokens + * @param _tokenId uint256 ID of the token to be transferred + * @param _data bytes optional data to send along with the call + * @return whether the call correctly returned the expected magic value + */ function checkAndCallSafeTransfer( address _from, address _to, diff --git a/contracts/token/ERC721/ERC721Token.sol b/contracts/token/ERC721/ERC721Token.sol index ed0939635..7e43ec7e9 100644 --- a/contracts/token/ERC721/ERC721Token.sol +++ b/contracts/token/ERC721/ERC721Token.sol @@ -33,85 +33,85 @@ contract ERC721Token is ERC721, ERC721BasicToken { mapping(uint256 => string) internal tokenURIs; /** - * @dev Constructor function - */ + * @dev Constructor function + */ function ERC721Token(string _name, string _symbol) public { name_ = _name; symbol_ = _symbol; } /** - * @dev Gets the token name - * @return string representing the token name - */ + * @dev Gets the token name + * @return string representing the token name + */ function name() public view returns (string) { return name_; } /** - * @dev Gets the token symbol - * @return string representing the token symbol - */ + * @dev Gets the token symbol + * @return string representing the token symbol + */ function symbol() public view returns (string) { return symbol_; } /** - * @dev Returns an URI for a given token ID - * @dev Throws if the token ID does not exist. May return an empty string. - * @param _tokenId uint256 ID of the token to query - */ + * @dev Returns an URI for a given token ID + * @dev Throws if the token ID does not exist. May return an empty string. + * @param _tokenId uint256 ID of the token to query + */ function tokenURI(uint256 _tokenId) public view returns (string) { require(exists(_tokenId)); return tokenURIs[_tokenId]; } /** - * @dev Gets the token ID at a given index of the tokens list of the requested owner - * @param _owner address owning the tokens list to be accessed - * @param _index uint256 representing the index to be accessed of the requested tokens list - * @return uint256 token ID at the given index of the tokens list owned by the requested address - */ + * @dev Gets the token ID at a given index of the tokens list of the requested owner + * @param _owner address owning the tokens list to be accessed + * @param _index uint256 representing the index to be accessed of the requested tokens list + * @return uint256 token ID at the given index of the tokens list owned by the requested address + */ function tokenOfOwnerByIndex(address _owner, uint256 _index) public view returns (uint256) { require(_index < balanceOf(_owner)); return ownedTokens[_owner][_index]; } /** - * @dev Gets the total amount of tokens stored by the contract - * @return uint256 representing the total amount of tokens - */ + * @dev Gets the total amount of tokens stored by the contract + * @return uint256 representing the total amount of tokens + */ function totalSupply() public view returns (uint256) { return allTokens.length; } /** - * @dev Gets the token ID at a given index of all the tokens in this contract - * @dev Reverts if the index is greater or equal to the total number of tokens - * @param _index uint256 representing the index to be accessed of the tokens list - * @return uint256 token ID at the given index of the tokens list - */ + * @dev Gets the token ID at a given index of all the tokens in this contract + * @dev Reverts if the index is greater or equal to the total number of tokens + * @param _index uint256 representing the index to be accessed of the tokens list + * @return uint256 token ID at the given index of the tokens list + */ function tokenByIndex(uint256 _index) public view returns (uint256) { require(_index < totalSupply()); return allTokens[_index]; } /** - * @dev Internal function to set the token URI for a given token - * @dev Reverts if the token ID does not exist - * @param _tokenId uint256 ID of the token to set its URI - * @param _uri string URI to assign - */ + * @dev Internal function to set the token URI for a given token + * @dev Reverts if the token ID does not exist + * @param _tokenId uint256 ID of the token to set its URI + * @param _uri string URI to assign + */ function _setTokenURI(uint256 _tokenId, string _uri) internal { require(exists(_tokenId)); tokenURIs[_tokenId] = _uri; } /** - * @dev Internal function to add a token ID to the list of a given address - * @param _to address representing the new owner of the given token ID - * @param _tokenId uint256 ID of the token to be added to the tokens list of the given address - */ + * @dev Internal function to add a token ID to the list of a given address + * @param _to address representing the new owner of the given token ID + * @param _tokenId uint256 ID of the token to be added to the tokens list of the given address + */ function addTokenTo(address _to, uint256 _tokenId) internal { super.addTokenTo(_to, _tokenId); uint256 length = ownedTokens[_to].length; @@ -120,10 +120,10 @@ contract ERC721Token is ERC721, ERC721BasicToken { } /** - * @dev Internal function to remove a token ID from the list of a given address - * @param _from address representing the previous owner of the given token ID - * @param _tokenId uint256 ID of the token to be removed from the tokens list of the given address - */ + * @dev Internal function to remove a token ID from the list of a given address + * @param _from address representing the previous owner of the given token ID + * @param _tokenId uint256 ID of the token to be removed from the tokens list of the given address + */ function removeTokenFrom(address _from, uint256 _tokenId) internal { super.removeTokenFrom(_from, _tokenId); @@ -143,11 +143,11 @@ contract ERC721Token is ERC721, ERC721BasicToken { } /** - * @dev Internal function to mint a new token - * @dev Reverts if the given token ID already exists - * @param _to address the beneficiary that will own the minted token - * @param _tokenId uint256 ID of the token to be minted by the msg.sender - */ + * @dev Internal function to mint a new token + * @dev Reverts if the given token ID already exists + * @param _to address the beneficiary that will own the minted token + * @param _tokenId uint256 ID of the token to be minted by the msg.sender + */ function _mint(address _to, uint256 _tokenId) internal { super._mint(_to, _tokenId); @@ -156,11 +156,11 @@ contract ERC721Token is ERC721, ERC721BasicToken { } /** - * @dev Internal function to burn a specific token - * @dev Reverts if the token does not exist - * @param _owner owner of the token to burn - * @param _tokenId uint256 ID of the token being burned by the msg.sender - */ + * @dev Internal function to burn a specific token + * @dev Reverts if the token does not exist + * @param _owner owner of the token to burn + * @param _tokenId uint256 ID of the token being burned by the msg.sender + */ function _burn(address _owner, uint256 _tokenId) internal { super._burn(_owner, _tokenId); diff --git a/contracts/token/ERC827/ERC827.sol b/contracts/token/ERC827/ERC827.sol index 81485cce8..a0ee18036 100644 --- a/contracts/token/ERC827/ERC827.sol +++ b/contracts/token/ERC827/ERC827.sol @@ -5,16 +5,15 @@ import "../ERC20/ERC20.sol"; /** - @title ERC827 interface, an extension of ERC20 token standard - - Interface of a ERC827 token, following the ERC20 standard with extra - methods to transfer value and data and execute calls in transfers and - approvals. + * @title ERC827 interface, an extension of ERC20 token standard + * + * @dev Interface of a ERC827 token, following the ERC20 standard with extra + * @dev methods to transfer value and data and execute calls in transfers and + * @dev approvals. */ contract ERC827 is ERC20 { - - function approve( address _spender, uint256 _value, bytes _data ) public returns (bool); - function transfer( address _to, uint256 _value, bytes _data ) public returns (bool); + function approve(address _spender, uint256 _value, bytes _data) public returns (bool); + function transfer(address _to, uint256 _value, bytes _data) public returns (bool); function transferFrom( address _from, address _to, @@ -23,5 +22,4 @@ contract ERC827 is ERC20 { ) public returns (bool); - } diff --git a/contracts/token/ERC827/ERC827Token.sol b/contracts/token/ERC827/ERC827Token.sol index 2c69a07bf..28dbfceea 100644 --- a/contracts/token/ERC827/ERC827Token.sol +++ b/contracts/token/ERC827/ERC827Token.sol @@ -1,3 +1,5 @@ +/* solium-disable security/no-low-level-calls */ + pragma solidity ^0.4.21; import "./ERC827.sol"; @@ -5,31 +7,32 @@ import "../ERC20/StandardToken.sol"; /** - @title ERC827, an extension of ERC20 token standard - - Implementation the ERC827, following the ERC20 standard with extra - methods to transfer value and data and execute calls in transfers and - approvals. - Uses OpenZeppelin StandardToken. + * @title ERC827, an extension of ERC20 token standard + * + * @dev Implementation the ERC827, following the ERC20 standard with extra + * @dev methods to transfer value and data and execute calls in transfers and + * @dev approvals. + * + * @dev Uses OpenZeppelin StandardToken. */ contract ERC827Token is ERC827, StandardToken { /** - @dev Addition to ERC20 token methods. It allows to - approve the transfer of value and execute a call with the sent data. - - 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 - - @param _spender The address that will spend the funds. - @param _value The amount of tokens to be spent. - @param _data ABI-encoded contract call to call `_to` address. - - @return true if the call function was executed successfully + * @dev Addition to ERC20 token methods. It allows to + * @dev approve the transfer of value and execute a call with the sent data. + * + * @dev Beware that changing an allowance with this method brings the risk that + * @dev someone may use both the old and the new allowance by unfortunate + * @dev transaction ordering. One possible solution to mitigate this race condition + * @dev is to first reduce the spender's allowance to 0 and set the desired value + * @dev afterwards: + * @dev https://github.com/ethereum/EIPs/issues/20#issuecomment-263524729 + * + * @param _spender The address that will spend the funds. + * @param _value The amount of tokens to be spent. + * @param _data ABI-encoded contract call to call `_to` address. + * + * @return true if the call function was executed successfully */ function approve(address _spender, uint256 _value, bytes _data) public returns (bool) { require(_spender != address(this)); @@ -42,14 +45,14 @@ contract ERC827Token is ERC827, StandardToken { } /** - @dev Addition to ERC20 token methods. Transfer tokens to a specified - address and execute a call with the sent data on the same transaction - - @param _to address The address which you want to transfer to - @param _value uint256 the amout of tokens to be transfered - @param _data ABI-encoded contract call to call `_to` address. - - @return true if the call function was executed successfully + * @dev Addition to ERC20 token methods. Transfer tokens to a specified + * @dev address and execute a call with the sent data on the same transaction + * + * @param _to address The address which you want to transfer to + * @param _value uint256 the amout of tokens to be transfered + * @param _data ABI-encoded contract call to call `_to` address. + * + * @return true if the call function was executed successfully */ function transfer(address _to, uint256 _value, bytes _data) public returns (bool) { require(_to != address(this)); @@ -61,15 +64,15 @@ contract ERC827Token is ERC827, StandardToken { } /** - @dev Addition to ERC20 token methods. Transfer tokens from one address to - another and make a contract call on the same transaction - - @param _from The address which you want to send tokens from - @param _to The address which you want to transfer to - @param _value The amout of tokens to be transferred - @param _data ABI-encoded contract call to call `_to` address. - - @return true if the call function was executed successfully + * @dev Addition to ERC20 token methods. Transfer tokens from one address to + * @dev another and make a contract call on the same transaction + * + * @param _from The address which you want to send tokens from + * @param _to The address which you want to transfer to + * @param _value The amout of tokens to be transferred + * @param _data ABI-encoded contract call to call `_to` address. + * + * @return true if the call function was executed successfully */ function transferFrom( address _from, @@ -89,12 +92,13 @@ contract ERC827Token is ERC827, StandardToken { /** * @dev Addition to StandardToken methods. Increase the amount of tokens that - * an owner allowed to a spender and execute a call with the sent data. + * @dev an owner allowed to a spender and execute a call with the sent data. + * + * @dev approve should be called when allowed[_spender] == 0. To increment + * @dev allowed value is better to use this function to avoid 2 calls (and wait until + * @dev the first transaction is mined) + * @dev From MonolithDAO Token.sol * - * approve should be called when allowed[_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 * @param _spender The address which will spend the funds. * @param _addedValue The amount of tokens to increase the allowance by. * @param _data ABI-encoded contract call to call `_spender` address. @@ -111,12 +115,13 @@ contract ERC827Token is ERC827, StandardToken { /** * @dev Addition to StandardToken methods. Decrease the amount of tokens that - * an owner allowed to a spender and execute a call with the sent data. + * @dev an owner allowed to a spender and execute a call with the sent data. + * + * @dev approve should be called when allowed[_spender] == 0. To decrement + * @dev allowed value is better to use this function to avoid 2 calls (and wait until + * @dev the first transaction is mined) + * @dev From MonolithDAO Token.sol * - * approve should be called when allowed[_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 * @param _spender The address which will spend the funds. * @param _subtractedValue The amount of tokens to decrease the allowance by. * @param _data ABI-encoded contract call to call `_spender` address.