Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error handling in ERC20 and ERC721 #1702

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions contracts/token/ERC20/ERC20.sol
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ contract ERC20 is IERC20 {
* @param value The amount to be transferred.
*/
function _transfer(address from, address to, uint256 value) internal {
require(to != address(0));
require(to != address(0), "transfer recipient is zero");

_balances[from] = _balances[from].sub(value);
_balances[to] = _balances[to].add(value);
Expand All @@ -140,7 +140,7 @@ contract ERC20 is IERC20 {
* @param value The amount that will be created.
*/
function _mint(address account, uint256 value) internal {
require(account != address(0));
require(account != address(0), "mint recipient is zero");

_totalSupply = _totalSupply.add(value);
_balances[account] = _balances[account].add(value);
Expand All @@ -154,7 +154,7 @@ contract ERC20 is IERC20 {
* @param value The amount that will be burnt.
*/
function _burn(address account, uint256 value) internal {
require(account != address(0));
require(account != address(0), "burn recipient is zero");

_totalSupply = _totalSupply.sub(value);
_balances[account] = _balances[account].sub(value);
Expand All @@ -168,8 +168,8 @@ contract ERC20 is IERC20 {
* @param value The number of tokens that can be spent.
*/
function _approve(address owner, address spender, uint256 value) internal {
require(spender != address(0));
require(owner != address(0));
require(spender != address(0), "spender address can't be zero");
require(owner != address(0), "owner address can't be zero");

_allowed[owner][spender] = value;
emit Approval(owner, spender, value);
Expand Down
27 changes: 15 additions & 12 deletions contracts/token/ERC721/ERC721.sol
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ contract ERC721 is ERC165, IERC721 {
* @return uint256 representing the amount owned by the passed address
*/
function balanceOf(address owner) public view returns (uint256) {
require(owner != address(0));
require(owner != address(0), "balance query for zero address");
return _ownedTokensCount[owner].current();
}

Expand All @@ -68,7 +68,7 @@ contract ERC721 is ERC165, IERC721 {
*/
function ownerOf(uint256 tokenId) public view returns (address) {
address owner = _tokenOwner[tokenId];
require(owner != address(0));
require(owner != address(0), "owner doesn't exist");
return owner;
}

Expand All @@ -82,8 +82,8 @@ contract ERC721 is ERC165, IERC721 {
*/
function approve(address to, uint256 tokenId) public {
address owner = ownerOf(tokenId);
require(to != owner);
require(msg.sender == owner || isApprovedForAll(owner, msg.sender));
require(to != owner, "transfer recipient must not be current owner");
require(msg.sender == owner || isApprovedForAll(owner, msg.sender), "sender is not owner nor is approved");

_tokenApprovals[tokenId] = to;
emit Approval(owner, to, tokenId);
Expand All @@ -96,7 +96,7 @@ contract ERC721 is ERC165, IERC721 {
* @return address currently approved for the given token ID
*/
function getApproved(uint256 tokenId) public view returns (address) {
require(_exists(tokenId));
require(_exists(tokenId), "tokenId to aprove doesn't exists");
return _tokenApprovals[tokenId];
}

Expand All @@ -107,7 +107,7 @@ contract ERC721 is ERC165, IERC721 {
* @param approved representing the status of the approval to be set
*/
function setApprovalForAll(address to, bool approved) public {
require(to != msg.sender);
require(to != msg.sender, "sender can't be receipent");
_operatorApprovals[msg.sender][to] = approved;
emit ApprovalForAll(msg.sender, to, approved);
}
Expand All @@ -131,7 +131,7 @@ contract ERC721 is ERC165, IERC721 {
* @param tokenId uint256 ID of the token to be transferred
*/
function transferFrom(address from, address to, uint256 tokenId) public {
require(_isApprovedOrOwner(msg.sender, tokenId));
require(_isApprovedOrOwner(msg.sender, tokenId), "sender is not owner nor is approved");

_transferFrom(from, to, tokenId);
}
Expand Down Expand Up @@ -186,6 +186,7 @@ contract ERC721 is ERC165, IERC721 {
* is an operator of the owner, or is the owner of the token
*/
function _isApprovedOrOwner(address spender, uint256 tokenId) internal view returns (bool) {
require(_exists(tokenId), "tokenId doesn't exist");
address owner = ownerOf(tokenId);
return (spender == owner || getApproved(tokenId) == spender || isApprovedForAll(owner, spender));
}
Expand All @@ -197,8 +198,8 @@ contract ERC721 is ERC165, IERC721 {
* @param tokenId uint256 ID of the token to be minted
*/
function _mint(address to, uint256 tokenId) internal {
require(to != address(0));
require(!_exists(tokenId));
require(to != address(0), "mint recipient is zero");
require(!_exists(tokenId), "tokenId to mint already exist");

_tokenOwner[tokenId] = to;
_ownedTokensCount[to].increment();
Expand All @@ -214,7 +215,8 @@ contract ERC721 is ERC165, IERC721 {
* @param tokenId uint256 ID of the token being burned
*/
function _burn(address owner, uint256 tokenId) internal {
require(ownerOf(tokenId) == owner);
require(_exists(tokenId), "tokenId to burn doesn't exist");
require(ownerOf(tokenId) == owner, "can only performed by owner");

_clearApproval(tokenId);

Expand All @@ -241,8 +243,9 @@ contract ERC721 is ERC165, IERC721 {
* @param tokenId uint256 ID of the token to be transferred
*/
function _transferFrom(address from, address to, uint256 tokenId) internal {
require(ownerOf(tokenId) == from);
require(to != address(0));
require(_exists(tokenId), "tokenId to transfer doesn't exist");
require(ownerOf(tokenId) == from, "can only performed by owner");
require(to != address(0), "transfer recipient is zero");

_clearApproval(tokenId);

Expand Down
2 changes: 1 addition & 1 deletion contracts/token/ERC721/ERC721Burnable.sol
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ contract ERC721Burnable is ERC721 {
* @param tokenId uint256 id of the ERC721 token to be burned.
*/
function burn(uint256 tokenId) public {
require(_isApprovedOrOwner(msg.sender, tokenId));
require(_isApprovedOrOwner(msg.sender, tokenId), "sender is not owner nor is approved");
_burn(tokenId);
}
}