Skip to content
This repository was archived by the owner on Jul 14, 2025. It is now read-only.
Open
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
1 change: 1 addition & 0 deletions .solhintignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Migrations.sol
314 changes: 314 additions & 0 deletions contracts/eip721/EIP721.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,314 @@
pragma solidity ^0.4.24;
import "./EIP721Interface.sol";
import "./EIP721MetadataInterface.sol";
import "./EIP721EnumerableInterface.sol";
import "./EIP721TokenReceiverInterface.sol";

/*
This is a full implementation of all ERC721's features.
Influenced by OpenZeppelin's implementation with some stylistic changes.
https://github.com/OpenZeppelin/zeppelin-solidity/tree/master/contracts/token/ERC721
*/

contract EIP721 is EIP721Interface, EIP721MetadataInterface, EIP721EnumerableInterface, ERC165Interface {
string public name;
string public symbol;

// all tokens
uint256[] internal allTokens;
// mapping of token IDs to its index in all Tokens array.
mapping(uint256 => uint256) internal allTokensIndex;
// Array of tokens owned by a specific owner
mapping(address => uint256[]) internal ownedTokens;
// Mapping from token ID to owner
mapping(uint256 => address) internal ownerOfToken;
// Mapping of the token ID to where it is in the owner's array.
mapping(uint256 => uint256) internal ownedTokensIndex;

// Mapping of a token to a specifically approved owner.
mapping(uint256 => address) internal approvedOwnerOfToken;

// An operator is allowed to manage all assets of another owner.
mapping(address => mapping (address => bool)) internal operators;

mapping(uint256 => string) internal tokenURIs;

bytes4 internal constant ERC721_BASE_INTERFACE_SIGNATURE = 0x80ac58cd;
bytes4 internal constant ERC721_METADATA_INTERFACE_SIGNATURE = 0x5b5e139f;
bytes4 internal constant ERC721_ENUMERABLE_INTERFACE_SIGNATURE = 0x780e9d63;
bytes4 internal constant ONERC721RECEIVED_FUNCTION_SIGNATURE = 0x150b7a02;

/* Modifiers */
modifier tokenExists(uint256 _tokenId) {
require(ownerOfToken[_tokenId] != 0);
_;
}

// checks: is the owner of the token == msg.sender?
// OR has the owner of the token granted msg.sender access to operate?
modifier allowedToOperate(uint256 _tokenId) {
require(checkIfAllowedToOperate(_tokenId));
_;
}

modifier allowedToTransfer(address _from, address _to, uint256 _tokenId) {
require(checkIfAllowedToOperate(_tokenId) || approvedOwnerOfToken[_tokenId] == msg.sender);
require(ownerOfToken[_tokenId] == _from);
require(_to != 0); //not allowed to burn in transfer method
_;
}

/// @notice Transfer ownership of an NFT -- THE CALLER IS RESPONSIBLE
/// TO CONFIRM THAT `_to` IS CAPABLE OF RECEIVING NFTS OR ELSE
/// THEY MAY BE PERMANENTLY LOST
/// @dev Throws unless `msg.sender` is the current owner, an authorized
/// operator, or the approved address for this NFT. Throws if `_from` is
/// not the current owner. Throws if `_to` is the zero address. Throws if
/// `_tokenId` is not a valid NFT.
/// @param _from The current owner of the NFT
/// @param _to The new owner
/// @param _tokenId The NFT to transfer
function transferFrom(address _from, address _to, uint256 _tokenId) external payable
tokenExists(_tokenId)
allowedToTransfer(_from, _to, _tokenId) {
//transfer token
settleTransfer(_from, _to, _tokenId);
}

/// @notice Transfers the ownership of an NFT from one address to another address
/// @dev Throws unless `msg.sender` is the current owner, an authorized
/// operator, or the approved address for this NFT. Throws if `_from` is
/// not the current owner. Throws if `_to` is the zero address. Throws if
/// `_tokenId` is not a valid NFT. When transfer is complete, this function
/// checks if `_to` is a smart contract (code size > 0). If so, it calls
/// `onERC721Received` on `_to` and throws if the return value is not
/// `bytes4(keccak256("onERC721Received(address,address,uint256,bytes)"))`.
/// @param _from The current owner of the NFT
/// @param _to The new owner
/// @param _tokenId The NFT to transfer
/// @param data Additional data with no specified format, sent in call to `_to`
function safeTransferFrom(address _from, address _to, uint256 _tokenId, bytes data) external payable
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to be payable?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As stated in the PR, I wondered about this too.

EIP721 by default sets transfer functions to add payable to transfer & approve functions. This does not seem the most secure to me. Should we perhaps think of forcing it to not have this by default?

I'm okay with following the standard and keeping payable.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh, sorry. Started the review on the plane, skipped the PR notes. Will review before I dig in again tomorrow.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The standard is actually quite flexible on this item.

So payable is unnecessary to stick with the standard.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding naming: I believe data can be _data (or the other parameters can have no underscore), since the function ID only uses the types of the parameters, not their Solidity names.

tokenExists(_tokenId)
allowedToTransfer(_from, _to, _tokenId) {
settleTransfer(_from, _to, _tokenId);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's a big deal, but since you have a possible revert later in this function, I think settleTransfer() would be best called at the end.

This would be in keeping with the "Checks, effects, interactions" pattern.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that's the case here since if you call the callback before settling the transfer the callback would have no different state to handle

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only problem is that it's a call, which shouldn't that be usually after internal state handling? either/or, not a big problem, but wondering what is the best practice here.


// check if a smart contract
uint256 size;
assembly { size := extcodesize(_to) } // solhint-disable-line no-inline-assembly
if (size > 0) {
// call on onERC721Received.
require(EIP721TokenReceiverInterface(_to).onERC721Received(msg.sender, _from, _tokenId, data) == ONERC721RECEIVED_FUNCTION_SIGNATURE);
}
}

/// @notice Transfers the ownership of an NFT from one address to another address
/// @dev This works identically to the other function with an extra data parameter,
/// except this function just sets data to ""
/// @param _from The current owner of the NFT
/// @param _to The new owner
/// @param _tokenId The NFT to transfer
function safeTransferFrom(address _from, address _to, uint256 _tokenId) external payable
tokenExists(_tokenId)
allowedToTransfer(_from, _to, _tokenId) {
settleTransfer(_from, _to, _tokenId);

// check if a smart contract
uint256 size;
assembly { size := extcodesize(_to) } // solhint-disable-line no-inline-assembly
if (size > 0) {
// call on onERC721Received.
require(EIP721TokenReceiverInterface(_to).onERC721Received(msg.sender, _from, _tokenId, "") == ONERC721RECEIVED_FUNCTION_SIGNATURE);
}
}

/// @notice Change or reaffirm the approved address for an NFT.
/// @dev The zero address indicates there is no approved address.
/// Throws unless `msg.sender` is the current NFT owner, or an authorized
/// operator of the current owner.
/// @param _approved The new approved NFT controller
/// @param _tokenId The NFT to approve
function approve(address _approved, uint256 _tokenId) external payable
tokenExists(_tokenId)
allowedToOperate(_tokenId) {
address owner = ownerOfToken[_tokenId];
internalApprove(owner, _approved, _tokenId);
}

/// @notice Enable or disable approval for a third party ("operator") to manage
/// all of `msg.sender`'s assets.
/// @dev Emits the ApprovalForAll event. The contract MUST allow
/// multiple operators per owner.
/// @param _operator Address to add to the set of authorized operators.
/// @param _approved True if the operator is approved, false to revoke approval
function setApprovalForAll(address _operator, bool _approved) external {
require(_operator != msg.sender); // can't make oneself an operator
operators[msg.sender][_operator] = _approved;
emit ApprovalForAll(msg.sender, _operator, _approved);
}

/* public View Functions */
/// @notice Count NFTs tracked by this contract
/// @return A count of valid NFTs tracked by this contract, where each one of
/// them has an assigned and queryable owner not equal to the zero address
function totalSupply() external view returns (uint256) {
return allTokens.length;
}

/// @notice Find the owner of an NFT
/// @param _tokenId The identifier for an NFT
/// @dev NFTs assigned to zero address are considered invalid, and queries
/// about them do throw.
/// @return The address of the owner of the NFT
function ownerOf(uint256 _tokenId) external view
tokenExists(_tokenId) returns (address) {
return ownerOfToken[_tokenId];
}

/// @notice Enumerate valid NFTs
/// @dev Throws if `_index` >= `totalSupply()`.
/// @param _index A counter less than `totalSupply()`
/// @return The token identifier for the `_index`th NFT,
/// (sort order not specified)
function tokenByIndex(uint256 _index) external view returns (uint256) {
require(_index < allTokens.length);
return allTokens[_index];
}

/// @notice Enumerate NFTs assigned to an owner
/// @dev Throws if `_index` >= `balanceOf(_owner)` or if
/// `_owner` is the zero address, representing invalid NFTs.
/// @param _owner An address where we are interested in NFTs owned by them
/// @param _index A counter less than `balanceOf(_owner)`
/// @return The token identifier for the `_index`th NFT assigned to `_owner`,
/// (sort order not specified)
function tokenOfOwnerByIndex(address _owner, uint256 _index) external view
tokenExists(_tokenId) returns (uint256 _tokenId) {
require(_index < ownedTokens[_owner].length);
return ownedTokens[_owner][_index];
}

/// @notice Count all NFTs assigned to an owner
/// @dev NFTs assigned to the zero address are considered invalid, and this
/// function throws for queries about the zero address.
/// @param _owner An address for whom to query the balance
/// @return The number of NFTs owned by `_owner`, possibly zero
function balanceOf(address _owner) external view returns (uint256) {
require(_owner != 0);
return ownedTokens[_owner].length;
}

/// @notice Get the approved address for a single NFT
/// @dev Throws if `_tokenId` is not a valid NFT
/// @param _tokenId The NFT to find the approved address for
// todo: The approved address for this NFT, or the zero address if there is none
function getApproved(uint256 _tokenId) external view
tokenExists(_tokenId) returns (address) {
return approvedOwnerOfToken[_tokenId];
}

/// @notice Query if an address is an authorized operator for another address
/// @param _owner The address that owns the NFTs
/// @param _operator The address that acts on behalf of the owner
/// @return True if `_operator` is an approved operator for `_owner`, false otherwise
function isApprovedForAll(address _owner, address _operator) external view returns (bool) {
return operators[_owner][_operator];
}

/// @notice A distinct Uniform Resource Identifier (URI) for a given asset.
/// @dev Throws if `_tokenId` is not a valid NFT. URIs are defined in RFC
/// 3986. The URI may point to a JSON file that conforms to the "ERC721
/// Metadata JSON Schema".
function tokenURI(uint256 _tokenId) external view returns (string) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't find a setter to actually populate the URI mapping?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. It's not in the standard.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The standard also says this function is optional. I would remove it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Part of the goal for me was to implement all functionality in one contract only (subjective choice for readability). Removing this means the metadata interface is only partly implemented, which will also break the interface signature.

It's a subjective choice though.

return tokenURIs[_tokenId];
}

/// @notice Query if a contract implements an interface
/// @param interfaceID The interface identifier, as specified in ERC-165
/// @dev Interface identification is specified in ERC-165. This function
/// uses less than 30,000 gas.
/// @return `true` if the contract implements `interfaceID` and
/// `interfaceID` is not 0xffffffff, `false` otherwise
function supportsInterface(bytes4 interfaceID) external view returns (bool) {

if (interfaceID == ERC721_BASE_INTERFACE_SIGNATURE ||
interfaceID == ERC721_METADATA_INTERFACE_SIGNATURE ||
interfaceID == ERC721_ENUMERABLE_INTERFACE_SIGNATURE) {
return true;
} else { return false; }
}

/* -- Internal Functions -- */
function checkIfAllowedToOperate(uint256 _tokenId) internal view returns (bool) {
return ownerOfToken[_tokenId] == msg.sender || operators[ownerOfToken[_tokenId]][msg.sender];
}

function internalApprove(address _owner, address _approved, uint256 _tokenId) internal {
require(_approved != _owner); //can't approve to owner to itself

// Note: the following code is equivalent to: require(getApproved(_tokenId) != 0) || _approved != 0);
// However: I found the following easier to read & understand.
if (approvedOwnerOfToken[_tokenId] == 0 && _approved == 0) {
revert(); // add reason for revert?
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a comment such as "Revert on NOOP" for readability.

} else {
approvedOwnerOfToken[_tokenId] = _approved;
emit Approval(_owner, _approved, _tokenId);
}
}

function settleTransfer(address _from, address _to, uint256 _tokenId) internal {
//clear pending approvals if there are any
if (approvedOwnerOfToken[_tokenId] != 0) {
internalApprove(_from, 0, _tokenId);
}

removeToken(_from, _tokenId);
addToken(_to, _tokenId);

emit Transfer(_from, _to, _tokenId);
}

function addToken(address _to, uint256 _tokenId) internal {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_to should probably be _owner.

// add new token to all tokens
allTokens.push(_tokenId);
// add new token to index of all tokens.
allTokensIndex[_tokenId] = allTokens.length-1;

// set token to be owned by address _to
ownerOfToken[_tokenId] = _to;
// add that token to an array keeping track of tokens owned by that address
ownedTokens[_to].push(_tokenId);
// add newly pushed to index.
ownedTokensIndex[_tokenId] = ownedTokens[_to].length-1;
}

function removeToken(address _from, uint256 _tokenId) internal {

// remove token from allTokens array.
uint256 allIndex = allTokensIndex[_tokenId];
uint256 allTokensLength = allTokens.length;
//1) Put last tokenID into index of tokenID to be removed.
allTokens[allIndex] = allTokens[allTokensLength - 1];
//2) Take last tokenID that has been moved to the removed token & update its new index
allTokensIndex[allTokens[allTokensLength-1]] = allIndex;
//3) delete last item (since it's now a duplicate)
delete allTokens[allTokensLength-1];
//4) reduce length of array
allTokens.length -= 1;

// remove token from owner array.
// get the index of where this token is in the owner's array
uint256 ownerIndex = ownedTokensIndex[_tokenId];
uint256 ownerLength = ownedTokens[_from].length;
/* Remove Token From Index */
//1) Put last tokenID into index of token to be removed.
ownedTokens[_from][ownerIndex] = ownedTokens[_from][ownerLength-1];
//2) Take last item that has been moved to the removed token & update its index
ownedTokensIndex[ownedTokens[_from][ownerLength-1]] = ownerIndex;
//3) delete last item (since it's now a duplicate)
delete ownedTokens[_from][ownerLength-1];
//4) reduce length of array
ownedTokens[_from].length -= 1;

delete ownerOfToken[_tokenId];
}
}
28 changes: 28 additions & 0 deletions contracts/eip721/EIP721EnumerableInterface.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
pragma solidity ^0.4.24;


/// @title ERC-721 Non-Fungible Token Standard, optional enumeration extension
/// @dev See https://github.com/ethereum/EIPs/blob/master/EIPS/eip-721.md
/// Note: the ERC-165 identifier for this interface is 0x780e9d63
interface EIP721EnumerableInterface {
/// @notice Count NFTs tracked by this contract
/// @return A count of valid NFTs tracked by this contract, where each one of
/// them has an assigned and queryable owner not equal to the zero address
function totalSupply() external view returns (uint256);

/// @notice Enumerate valid NFTs
/// @dev Throws if `_index` >= `totalSupply()`.
/// @param _index A counter less than `totalSupply()`
/// @return The token identifier for the `_index`th NFT,
/// (sort order not specified)
function tokenByIndex(uint256 _index) external view returns (uint256);

/// @notice Enumerate NFTs assigned to an owner
/// @dev Throws if `_index` >= `balanceOf(_owner)` or if
/// `_owner` is the zero address, representing invalid NFTs.
/// @param _owner An address where we are interested in NFTs owned by them
/// @param _index A counter less than `balanceOf(_owner)`
/// @return The token identifier for the `_index`th NFT assigned to `_owner`,
/// (sort order not specified)
function tokenOfOwnerByIndex(address _owner, uint256 _index) external view returns (uint256 _tokenId);
}
Loading