From 2a56e50ece318e2a3112e00e7914233019bc5e82 Mon Sep 17 00:00:00 2001 From: William Entriken Date: Sun, 25 Feb 2018 23:27:38 -0500 Subject: [PATCH 1/2] Allow to query the approved contracts --- EIPS/eip-721.md | 43 ++++++++++++++++++++++++++----------------- 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/EIPS/eip-721.md b/EIPS/eip-721.md index c59a5b1f..2b336290 100644 --- a/EIPS/eip-721.md +++ b/EIPS/eip-721.md @@ -39,33 +39,30 @@ Differences between this standard and EIP-20 are examined below. The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", "SHOULD", "SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL" in this document are to be interpreted as described in [RFC 2119](https://www.ietf.org/rfc/rfc2119.txt). -The **baseline specification** is REQUIRED for all ERC-721 implementations (see "caveats", below). +**Every ERC-721 compliant contract must implement the `ERC721` and [`ERC165`](https://github.com/ethereum/EIPs/blob/master/EIPS/eip-165.md) interfaces** (subject to "caveats" below): ```solidity pragma solidity ^0.4.20; -import "./ERC165.sol"; - /// @title Required part of ERC-721 Deed Standard /// @dev See https://github.com/ethereum/EIPs/blob/master/EIPS/eip-721.md -/// Note: the ERC-165 identifier for this interface is 0xb3a99827 +/// Note: the ERC-165 identifier for this interface is 0xTODO_FILL_IN interface ERC721 /* is ERC165 */ { /// @dev This emits when ownership of any deed changes by any mechanism. /// This event emits when deeds are created (`from` == 0) and destroyed /// (`to` == 0). Exception: during contract creation, any number of deeds /// may be created and assigned without emitting Transfer. At the time of - /// any transfer, the "approved deed controller" is implicitly reset to the - /// zero address. + /// any transfer, the approved address for that deed (if any) is reset to none. event Transfer(address indexed _from, address indexed _to, uint256 _deedId); - /// @dev This emits when the "approved deed controller" for a deed is - /// changed or reaffirmed. The zero address indicates there is no approved - /// deed controller. When a Transfer event emits, this also indicates the - /// approved deed controller (if any) is reset to none. + /// @dev This emits when the approved address for a single is changed or + /// reaffirmed. The zero address indicates there is no approved address. + /// When a Transfer event emits, this also indicates that the approved + /// address for that deed (if any) is reset to none. event Approval(address indexed _owner, address indexed _approved, uint256 _deedId); - /// @dev This emits when a third party ("operator") is enabled or disable for - /// an owner. The operator may manage all deeds of the owner. + /// @dev This emits when an operator is enabled or disable for an owner. + /// The operator may manage all deeds of the owner. event ApprovalForAll(address indexed _owner, address indexed _operator, bool _approved); /// @notice Count all deeds assigned to an owner @@ -104,8 +101,8 @@ interface ERC721 /* is ERC165 */ { /// @param _deedId The deed to transfer function transferFrom(address _from, address _to, uint256 _deedId) external payable; - /// @notice Set or reaffirm the "approved deed controller" for a deed - /// @dev The zero address indicates there is no approved deed controller. + /// @notice Set or reaffirm the approved address for a deed + /// @dev The zero address indicates there is no approved address. /// @dev Throws unless `msg.sender` is the current deed owner, or the /// "delegate operator" of the current deed owner. /// @param _approved The new approved deed controller @@ -117,11 +114,23 @@ interface ERC721 /* is ERC165 */ { /// @dev Emits the ApprovalForAll event /// @param _operator Address to add to the set of authorized operators. /// @param _approved True if the operators is approved, false to revoke approval - function setApprovalForAll(address _operateor, boolean _approved) payable; + function setApprovalForAll(address _operateor, boolean _approved) external; - // CONFORMANCE TO ERC-165 ////////////////////////////////////////////////// + /// @notice Get the approved address for a single deed + /// @dev Throws if `_deedId` is not a valid deed + /// @param _deedId The deed to find the approved address for + /// @return The approved address for this deed, or the zero address if there is none + function getApproved(uint256 _deedId) returns (address); - /// @notice Query if this implements an interface + /// @notice Query if an address is an authorized operator for another address + /// @param _owner The address that owns the deeds + /// @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) returns (bool); +} + +interface ERC165 { + /// @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. From 74dadccc858545aa89edaf6ec1cb5857cd261083 Mon Sep 17 00:00:00 2001 From: William Entriken Date: Mon, 26 Feb 2018 00:40:30 -0500 Subject: [PATCH 2/2] Add transfer security, give everything else a once-over --- EIPS/eip-721.md | 111 +++++++++++++++++++++++++++++++----------------- 1 file changed, 73 insertions(+), 38 deletions(-) diff --git a/EIPS/eip-721.md b/EIPS/eip-721.md index 2b336290..e0ee4405 100644 --- a/EIPS/eip-721.md +++ b/EIPS/eip-721.md @@ -79,32 +79,49 @@ interface ERC721 /* is ERC165 */ { /// @return The address of the owner of the deed function ownerOf(uint256 _deedId) external view returns (address _owner); - /// @notice Transfers the ownership of a deed -- warning the caller is - /// responsible to confirm that the sender is capable of receiving deeds - /// otherwise the deed may become inaccessible! - /// @dev Throws unless `msg.sender` is the current deed owner, the "delegate - /// operator" of the current deed owner, or the "approved deed controller". - /// Throws if `_to` currently owns the deed. Throws if `_to` is the zero - /// address. + /// @notice Transfer ownership of a deed -- THE CALLER IS RESPONSIBLE + /// TO CONFIRM THAT `_to` IS CAPABLE OF RECEIVING DEEDS OR ELSE + /// THEY MAY BE PERMANENTLY LOST + /// @dev Throws unless `msg.sender` is the current deed owner, an authorized + /// operator, or the approved address for this deed. Throws if `_from` is + /// not the current owner of the deed. Throws if `_to` is the zero address. + /// Throws if `_deedId` is not a valid deed. + /// @param _from The new owner for the deed /// @param _to The new owner for the deed /// @param _deedId The deed to transfer - function transfer(address _to, uint256 _deedId) external payable; + function unsafeTransfer(address _from, address _to, uint256 _deedId) external payable; /// @notice Transfers the ownership of a given deed from one address to /// another address - /// @dev Throws unless `msg.sender` is the current deed owner, the "delegate - /// operator" of the current deed owner, or the "approved deed controller". - /// Throws if `_to` currently owns the deed. Throws if `_to` is the zero - /// address. Throws if the deed is not currently owned by _from. + /// @dev Throws unless `msg.sender` is the current deed owner, an authorized + /// operator, or the approved address for this deed. Throws if `_from` is + /// not the current owner of the deed. Throws if `_to` is the zero address. + /// Throws if `_deedId` is not a valid deed. When transfer is complete, + /// this function also calls `onNFTReceived` on `_to` and throws if the return + /// value is not `keccak256("ERC721_ONNFTRECEIVED")`. /// @param _from The current owner for the deed /// @param _to The new owner for the deed /// @param _deedId The deed to transfer - function transferFrom(address _from, address _to, uint256 _deedId) external payable; + /// @param data Additional data with no specified format, sent in call to `_to` + function transferFrom(address _from, address _to, uint256 _deedId, bytes[] data) external payable; + + /// @notice Transfers the ownership of a given deed from one address to + /// another address + /// @dev Throws unless `msg.sender` is the current deed owner, an authorized + /// operator, or the approved address for this deed. Throws if `_from` is + /// not the current owner of the deed. Throws if `_to` is the zero address. + /// Throws if `_deedId` is not a valid deed. When transfer is complete, + /// this function also calls `onNFTReceived` on `_to` and throws if the return + /// value is not `keccak256("ERC721_ONNFTRECEIVED")`. + /// @param _from The current owner for the deed + /// @param _to The new owner for the deed + /// @param _deedId The deed to transfer + function transferFrom(address _from, address _to, uint256 _deedId) external payable; /// @notice Set or reaffirm the approved address for a deed /// @dev The zero address indicates there is no approved address. - /// @dev Throws unless `msg.sender` is the current deed owner, or the - /// "delegate operator" of the current deed owner. + /// @dev Throws unless `msg.sender` is the current deed owner, or an authorized + /// operator of the current deed owner. /// @param _approved The new approved deed controller /// @param _deedId The deed to approve function approve(address _approved, uint256 _deedId) external payable; @@ -114,7 +131,7 @@ interface ERC721 /* is ERC165 */ { /// @dev Emits the ApprovalForAll event /// @param _operator Address to add to the set of authorized operators. /// @param _approved True if the operators is approved, false to revoke approval - function setApprovalForAll(address _operateor, boolean _approved) external; + function setApprovalForAll(address _operator, bool _approved) external; /// @notice Get the approved address for a single deed /// @dev Throws if `_deedId` is not a valid deed @@ -140,7 +157,25 @@ interface ERC165 { } ``` -The **metadata extension** is OPTIONAL for ERC-721 implementations (see "caveats", below). This allows your contract to be interrogated for its name and for details about the *things*. +A wallet/broker/auction application MUST implement the **wallet interface** if it will accept safe transfers. + +```solidity +interface ERC721TokenReceiver { + /// @notice Handle the receipt of a token + /// @dev The ERC721 smart contract calls this function on the recipient + /// after a `transfer`. This function MAY throw to revert and reject the + /// transfer. This function MUST use 50,000 gas or less. Return of other + /// than the magic value MUST result in the transaction being reverted. + /// Note: the contract address is always the message sender. + /// @param _from The sending address + /// @param _tokenId The deed identifier which was sent + /// @param data Additional data with no specified format + /// @return Always returns `keccak256("ERC721_ONNFTRECEIVED")`, unless throwing + function onNFTReceived(address _from, uint256 _tokenId, bytes data) external returns(bytes4); +} +``` + +The **metadata extension** is OPTIONAL for ERC-721 smart contracts (see "caveats", below). This allows your contract to be interrogated for its name and for details about the *things*. ```solidity /// @title Optional metadata extension to ERC-721 Deed Standard @@ -184,7 +219,7 @@ This is the "ERC721 Metadata JSON Schema" referenced above. Learn more about [JS } ``` -The **enumeration extension** is OPTIONAL for ERC-721 implementations (see "caveats", below). This allows your contract to publish the the full list of deeds and make them discoverable. +The **enumeration extension** is OPTIONAL for ERC-721 smart contracts (see "caveats", below). This allows your contract to publish the the full list of deeds and make them discoverable. ```solidity /// @title Optional enumeration extension to ERC-721 Deed Standard @@ -263,41 +298,39 @@ The choice of uint256 allows a wide variety of applications because UUIDs and sh **Transfer mechanism** -ERC-721 standardizes a single transfer function `transfer` and two levels of indirection: +ERC-721 standardizes a safe transfer function `transferFrom` (overloaded with and without a `bytes` parameter) and an unsafe function `unsafeTransfer`. Transfers may be initiated by: -- The owner can transfer a deed to any address (except the zero address). -- The "approved deed controller" (if any) can transfer just like the owner. -- The "delegate operator" can transfer just like the owner and can assign the "approved deed controller". +- The owner of a deed +- The approved address of a deed +- An authorized operator of the current owner of a deed -This provides a powerful set of tools for broker or auction applications to quickly use a *large* number of deeds. +Additionally, an authorized operator may set the approved address for a deed. This provides a powerful set of tools for wallet, broker or auction applications to quickly use a *large* number of deeds. -The `transfer` documentation only specifies conditions when the transaction MUST throw. Your implementation MAY also throw on calls `transfer`, `approve` and `delegate` in other situations. This allows implementations to achieve interesting results: +The transfer and accept functions documentation only specify conditions when the transaction MUST throw. Your implementation MAY also throw in other situations. This allows implementations to achieve interesting results: - **Disallow transfers if the contract is paused** — prior art, [Crypto Kitties](https://github.com/axiomzen/cryptokitties-bounty/blob/master/contracts/KittyOwnership.sol#L79) - **Blacklist certain address from receiving deeds** — prior art, [Crypto Kitties, (lines 565, 566)](https://etherscan.io/address/0x06012c8cf97bead5deae237070f9587f8e7a266d#code). -- **Require every transaction to use the the approve-transfer workflow** — require `transfer` parameter `_to` to equal `msg.sender` -- **Charge a fee to both parties of a transaction** — require payment when calling `approve` with a non-zero `_approved` if it was previously the zero address, refund payment if calling `approve` with the zero address if it was previously a non-zero address, require payment when calling `transfer`, require `transfer` parameter `_to` to equal `msg.sender`, require `transfer` parameter `_to` to be the approved deed controller -- **Read only deed registry** — always throw from `transfer`, `approve` and `delegate` +- **Disallow unsafe transfers** — `unsafeTransfer` throws unless `_to` equals `msg.sender` or `countOf(_to)` is non-zero (because such cases are safe) +- **Charge a fee to both parties of a transaction** — require payment when calling `approve` with a non-zero `_approved` if it was previously the zero address, refund payment if calling `approve` with the zero address if it was previously a non-zero address, require payment when calling `transfer`, require `transfer` parameter `_to` to equal `msg.sender`, require `transfer` parameter `_to` to be the approved address for the deed +- **Read only deed registry** — always throw from `unsafeTransfer`, `transferFrom`, `approve` and `setApprovalForAll` -Failed transactions will throw, a best practice identified in [ERC-233](https://github.com/ethereum/EIPs/issues/223) , [ERC-677](https://github.com/ethereum/EIPs/issues/677), [ERC-827](https://github.com/ethereum/EIPs/issues/827) and [OpenZeppelin](https://github.com/OpenZeppelin/zeppelin-solidity/blob/master/contracts/token/ERC20/SafeERC20.sol). [ERC-20](https://github.com/ethereum/EIPs/blob/master/EIPS/eip-20-token-standard.md) defined an `allowance` feature, this caused a problem when was called and then later modified to a different amount, as [disucssed on OpenZeppelin](https://github.com/OpenZeppelin/zeppelin-solidity/issues/438). In this deed standard, there is no allowance because every deed is unique, the quantity is none or one. Therefore we receive the benefits of ERC-20's original design without problems that have been later discovered. +Failed transactions will throw, a best practice identified in [ERC-233](https://github.com/ethereum/EIPs/issues/223) , [ERC-677](https://github.com/ethereum/EIPs/issues/677), [ERC-827](https://github.com/ethereum/EIPs/issues/827) and [OpenZeppelin](https://github.com/OpenZeppelin/zeppelin-solidity/blob/master/contracts/token/ERC20/SafeERC20.sol). [ERC-20](https://github.com/ethereum/EIPs/blob/master/EIPS/eip-20-token-standard.md) defined an `allowance` feature, this caused a problem when called and then later modified to a different amount, as [disucssed on OpenZeppelin](https://github.com/OpenZeppelin/zeppelin-solidity/issues/438). In ERC-721, there is no allowance because every deed is unique, the quantity is none or one. Therefore we receive the benefits of ERC-20's original design without problems that have been later discovered. -Use of the `transfer` function where the caller is the owner of the deed is controversial as discussed in [ERC-233](https://github.com/ethereum/EIPs/issues/223) and [ERC-677](https://github.com/ethereum/EIPs/issues/677). But we take the position that misuse of the `transfer` function is an implementation mistake not a standards mistake. - -Creating of new deeds and destruction of deeds is not included in the specification. Your contract may implement these by other means. Please see the `event` documentation for your responsibilities when creating or destroying deeds. +Creating of new deeds ("minting") and destruction of deeds ("burning") is not included in the specification. Your contract may implement these by other means. Please see the `event` documentation for your responsibilities when creating or destroying deeds. *Alternatives considered: only allow two-step ERC-20 style transaction, require that `transfer` never throw, require all functions to return a boolean indicating the success of the operation.* **ERC-165 interface** -This specification includes a function `supportsInterface` which is standardized in [ERC-165](https://github.com/ethereum/EIPs/pull/881). So any contract MAY query your contract to see if it complies with ERC-721 and the extensions. +We chose Standard Interface Detection [ERC-165](https://github.com/ethereum/EIPs/blob/master/EIPS/eip-165.md) to expose the interfaces that a ERC-721 smart contract supports. -A future EIP may create a global registry of interfaces for contracts. We strongly support such an EIP and it would allow your contract to to implement ERC721Enumerable, ERC721Metadata, or other interfaces by delegating to a separate contract. +A future EIP may create a global registry of interfaces for contracts. We strongly support such an EIP and it would allow your contract to to implement `ERC721Enumerable`, `ERC721Metadata`, or other interfaces by delegating to a separate contract. **Gas and complexity** (regarding the enumeration extension) This specification contemplates implementations that manage a few and *arbitrarily large* numbers of deeds. If your application is able to grow then [avoid using for/while loops in your code](https://github.com/axiomzen/cryptokitties-bounty/issues/4). These indicate your contract may be unable to scale and gas costs will rise over time without bound. -[We have deployed](https://github.com/fulldecent/erc721-example) a contract to test net which instantiates and tracks 340282366920938463463374607431768211456 different deeds (2^128). That's enough to assign every IPV6 address to an Ethereum account owner, or to track ownership of nanobots a few micron in size and in aggregate totalling half the size of Earth. And you can query it from the blockchain. And every function takes less gas than [querying the ENS](https://ens.domains/). +[We have deployed](https://github.com/fulldecent/erc721-example) a contract to test net which instantiates and tracks 340282366920938463463374607431768211456 different deeds (2^128). That's enough to assign every IPV6 address to an Ethereum account owner, or to track ownership of nanobots a few micron in size and in aggregate totalling half the size of Earth. You can query it from the blockchain. And every function takes less gas than [querying the ENS](https://ens.domains/). This illustration makes clear: the Deed Standard scales. @@ -311,13 +344,13 @@ It may be interesting to consider a use case where deeds are not enumerable or d **Metadata choices** (metadata extension) -We have required `name` and `symbol` functions in the metadata extension. Every token EIP and draft we have reviewed ([ERC-20](https://github.com/ethereum/EIPs/blob/master/EIPS/eip-20-token-standard.md), [ERC-233](https://github.com/ethereum/EIPs/issues/223) , [ERC-677](https://github.com/ethereum/EIPs/issues/677), [ERC-827](https://github.com/ethereum/EIPs/issues/827)) included these functions. Deeds **are not** tokens, but let's adopt existing conventions when it makes sense! +We have required `name` and `symbol` functions in the metadata extension. Every token EIP and draft we reviewed ([ERC-20](https://github.com/ethereum/EIPs/blob/master/EIPS/eip-20-token-standard.md), [ERC-233](https://github.com/ethereum/EIPs/issues/223) , [ERC-677](https://github.com/ethereum/EIPs/issues/677), [ERC-777](https://github.com/ethereum/EIPs/issues/777), [ERC-827](https://github.com/ethereum/EIPs/issues/827)) included these functions. We remind implementation authors that the empty string is a valid response to `name` and `symbol` if you protest to the usage of this mechanism. We also remind everyone that the official contract for tracking 0xProject tokens (`ZRX`) is [0xe41d2489571d322189246dafa5ebde1f4699f498](https://etherscan.io/address/0xe41d2489571d322189246dafa5ebde1f4699f498). Another contract that advertises a name of `0xProject` and symbol `ZRX` is not the well-known (canonical) contract. How a client may determine which token and deed contracts are well-known is outside the scope of this standard. -A mechanism is provided to associate deeds with URIs. We expect that many implementations will take advantage of this to provide metadata for each deed. The image size recommendation is taken from Instagram, they probably know much about image usability. The URI MAY be mutable (i.e. it changes from time to time). We considered a deed representing ownership of a real-world asset, in this case metadata about such an asset may naturally change. +A mechanism is provided to associate deeds with URIs. We expect that many implementations will take advantage of this to provide metadata for each deed. The image size recommendation is taken from Instagram, they probably know much about image usability. The URI MAY be mutable (i.e. it changes from time to time). We considered a deed representing ownership of a house, in this case metadata about the house (image, occupants, etc.) may naturally change. Metadata is returned as a string value. Currently this is only usable as calling from `web3`, not from other contracts. This is acceptable because we have not considered a use case where an on-blockchain application would query such information. @@ -325,7 +358,7 @@ Metadata is returned as a string value. Currently this is only usable as calling **Community consensus** -A significant amount of discussion occurred on [the original ERC-721](ERC-721 issue), additionally we held a live meeting [on Gitter](https://gitter.im/ethereum/ERCs?at=5a62259b5ade18be3998eec4) that had good representation and [was](https://www.reddit.com/r/ethereum/comments/7r2ena/friday_119_live_discussion_on_erc_nonfungible/) [well](https://gitter.im/ethereum/EIPs?at=5a5f823fb48e8c3566f0a5e7) [advertised](https://github.com/ethereum/eips/issues/721#issuecomment-358369377) in relevant communities. Thank you to the participants: +A significant amount of discussion occurred on [the original ERC-721](ERC-721 issue), additionally we held a live meeting [on Gitter](https://gitter.im/ethereum/ERCs?at=5a62259b5ade18be3998eec4) that had good representation and [was](https://www.reddit.com/r/ethereum/comments/7r2ena/friday_119_live_discussion_on_erc_nonfungible/) [well](https://gitter.im/ethereum/EIPs?at=5a5f823fb48e8c3566f0a5e7) [advertised](https://github.com/ethereum/eips/issues/721#issuecomment-358369377) in relevant communities. Thank you to the participants: - [@ImAllInNow](https://github.com/imallinnow) Rob from DEC Gaming / Presenting Michigan Ethereum Meetup Feb 7 - [@Arachnid](https://github.com/arachnid) Nick Johnson @@ -338,7 +371,7 @@ We have been very inclusive in this process and invite anyone with questions or ## Backwards Compatibility -We have adopted `balanceOf`, `totalSupply`, `name` and `symbol` semantics from the [ERC-20](https://github.com/ethereum/EIPs/blob/master/EIPS/eip-20.md) specification. An implementation may also include a function `decimals` that returns `uint8 0` if its intention is to be more compatible with ERC-20 while supporting this standard. However, we find it contrived to require all ERC-721 implementations to support the `decimals` function. +We have adopted `balanceOf`, `totalSupply`, `name` and `symbol` semantics from the [ERC-20](https://github.com/ethereum/EIPs/blob/master/EIPS/eip-20.md) specification. An implementation may also include a function `decimals` that returns `uint8(0)` if its goal is to be more compatible with ERC-20 while supporting this standard. However, we find it contrived to require all ERC-721 implementations to support the `decimals` function. Example deeds implementations as of February 2018: @@ -348,6 +381,8 @@ Example deeds implementations as of February 2018: Note: "Limited edition, collectible tokens" like [Curio Cards](https://mycuriocards.com/) and [Rare Pepe](https://rarepepewallet.com/) are *not* deeds. They're actually a collection of individual fungible tokens, each of which is tracked by its own smart contract with its own total supply (which may be `1` in extreme cases). +The `onNFTReceived` function specifically works around old deployed contracts which may inadvertently return 1 (`true`) in certain circumstances even if they don't implement a function. By returning, and checking for, a magic value we are able to distinguish actual affirmative responses versus these `true`s. + ## Test Cases **TO DO**