search

ethereum / EIPs

The Ethereum Improvement Proposal repository
https://eips.ethereum.org/
Creative Commons Zero v1.0 Universal
12.82k stars 5.24k forks source link

EIP 821: Distinguishable Assets Registry (Contract for NFTs) #821

Closed eordano closed 6 years ago

eordano commented 6 years ago 
EIP: <not assigned>
Title: Distinguishable Assets Registry
Author: Esteban Ordano <esteban@decentraland.org>
Type: Standard Track
Category: ERC
Status: Draft
Created: 2018-01-05
Updated: 2018-02-21

Summary

A Distinguishable Assets Registry (DAR for short) is a contract that tracks ownership of, and information about a set of assets that are distinguishable from each other, and are commonly refferred to as "NFTs", for "Non Fungible Tokens".

See https://github.com/decentraland/erc821 for a reference implementation.

See the "Revisions" section for a history of this ERC.

Abstract

Tracking the ownership of physical or digital distinguishable items on a blockchain has a broad range of applications, from virtual collectibles to physical art pieces. This proposal aims at standardizing a way to reference distinguishable assets along with the foreseeable required operations and interfaces for effective management of those assets on a blockchain.

Introduction

The number of virtual collectibles tracked on the Ethereum blockchain is rapidly growing, creating a demand for a more robust standard for distinguishable digital assets. This proposal suggests improvements to the vocabulary used to refer to such assets, and attempts to provide a solid and future-proof reference implementation of the basic functionality needed. This EIP also proposes better naming conventions and vocabulary for the different components of the NFT economy: the assets, the NFTs (representations of those assets on the blockchain), the DARs (the contracts registering such assets), distinguishing ownership from holding addresses, and more.

See also: ERC #721, ERC #20, ERC #223, and ERC #777.

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.

Specification

A non-fungible token (NFT) is a distinguishable asset that has a unique representation as a register in a smart contract. This proposal specifies such contracts, referenced as Distinguishable Asset Registries, or DARs. NFTs MAY also be reffered to as "deeds".

DARs can be identified by the blockchain in which they were deployed, and the 160-bit address of the contract instance. NFTs are identified by an ID, a 256 bit number, which MAY correspond to some cryptographic hash of the non-fungible's natural key.

NFTs SHOULD be referenced by a URI that follows this schema:

nft://<chain's common name>/<DAR's address>/<NFT's ID>

The NFT's ID SHOULD have a hint that helps to decode it. For instance, if the encoding of the number is in hexadecimal, the NFT’s ID SHOULD start with 0x. The DAR's address SHOULD follow the checksum by casing as proposed on #55.

Some common names for Ethereum blockchains are:

Some examples of NFT URIs follow:

Every NFT MUST have a owner address associated with it. NFTs associated with the null address are assumed non-existent or destroyed.

An owner MAY assign one or multiple operator addresses. These addresses will be able to transfer any asset of the owner.

DARs MUST trigger Transfer events every time a NFT's owner changes. This might happen under three circumstances:

Transfer events SHALL NOT simultaneously have a zero value in both the from and to fields (this means, the same Transfer event can't both create and destroy a token).

Associated Metadata

Metadata associated with each asset is out of scope for this standard.

DAR global methods

totalSupply():uint256

Return the total amount of assets under this DAR. This method SHALL NOT throw.

supportsInterface(bytes8 interfaceId):bool

This method returns true if the interfaceId is a supported interface (165, corresponding to 0x01ffc9a7, or 821, corresponding to 0x959d7abb). This method SHALL NOT throw.

NFT getter methods

exists(uint256 assetId):bool

This method returns a boolean value, true if the asset identified with the given assetId exists under this DAR. This method SHALL NOT throw.

ownerOf(uint256 assetId):address

This method returns the address of the owner of the NFT. This method SHALL NOT throw. If the assetId does not exist, the return value MUST be the null address.

Owner-centric getter methods

balanceOf(address owner):uint256

This method returns the amount of NFTs held by the owner address under this DAR. This method MUST not throw.

assetByIndex(address owner, uint256 index):uint256

This method returns the ID of the indexth NFT held by the owner address under this DAR, when all the IDs of the NFTs held by such address are stored as an array.

This method MUST throw if assetCount(owner) >= index. This method MUST throw if index >= 2^128.

The DAR MAY change the order assigned to any NFT held by a particular address.

This method is expected to be used by other contracts to iterate through an owner's assets. Contracts implementing such iterations SHOULD be aware of race conditions that might occur, if this iteration happens over multiple transactions.

assetsOf(address owner):uint256[]

This method returns an array of IDs of the NFTs held by owner. This method SHALL NOT throw.

Operator getters

isAuthorizedBy(address operator, address owner):bool

This method returns true if owner has called the method authorize with parameters (operator, true) and has not called authorize(operator, false) afterwards. This method returns false otherwise.

This method MUST return true if operator == owner.

This method SHALL NOT throw.

isApprovedFor(address operator, uint256 assetId):bool

This method returns true if owner has called the method approve with parameters (operator, assetId) and has not called it with a different operator value afterwards. This method returns false otherwise.

This method MUST return true if operator == owner.

This method SHALL NOT throw.

Transfers

transfer(address to, uint256 assetId, bytes userData, bytes operatorData)

Transfers holding of the NFT referenced by assetId from ownerOf(assetId) to the address to.

to SHALL NOT be the zero address. If to is the zero address, the call MUST throw.

to SHALL NOT be ownerOf(assetId). If this condition is met, the call MUST throw.

isAuthorizedBy(msg.sender, ownerOf(assetId)) MUST return true as a precondition.

This means that the msg.sender MUST be ownerOf(assetId) or an authorized operator.

If the NFT referenced by assetId does not exist, then the call MUST throw.

If there was any single authorized operator (see the approve() method below), this authorization MUST be cleared.

If the call doesn't throw, it triggers the event Transfer with the following parameters:

If to is a contract's address, this call MUST verify that the contract can receive the tokens. In order to do so, the method MUST do a #165 check for support to handle tokens to the target contract. The implementation for the receiver is as follows:

interface IAssetHolder {
  function onAssetReceived(
    /* address _assetRegistry == msg.sender */
    uint256 _assetId,
    address _previousOwner,
    address _currentOwner,
    bytes   _userData
  ) public;
}

If the supportsInterface method call fails, the transfer must be reversed and this call MUST throw. Otherwise, the method onAssetReceived MUST be invoked with the corresponding information, after the asset has been transferred.

transfer(address to, uint256 assetId, bytes userData)

Shorthand method that MUST be equivalent to calling transfer(to, assetId, userData, EMPTY_BYTES).

transfer(address to, uint256 assetId)

Shorthand method that MUST be equivalent to calling transfer(to, assetId, EMPTY_BYTES, EMPTY_BYTES).

transferFrom(address from, address to, uint256 assetId, bytes userData, bytes operatorData)

Transfers holding of the NFT referenced by assetId from ownerOf(assetId) to the address to, iff from == ownerOf(assetId).

After checking that the asset is owned by the from address, this method MUST behave exactly as calling transfer(to, assetId, EMPTY_BYTES, EMPTY_BYTES).

transferFrom(address from, address to, uint256 assetId, bytes userData)

Shorthand method that MUST be equivalent to calling transferFrom(to, assetId, userData, EMPTY_BYTES).

transferFrom(address from, address to, uint256 assetId)

Shorthand method that MUST be equivalent to calling transferFrom(to, assetId, EMPTY_BYTES, EMPTY_BYTES).

Authorization

authorize(address operator, bool authorized)

If authorized is true, allows operator to transfer any NFT held by msg.sender.

This method MUST throw if operator is the zero address. This method MUST throw if authorized is true and operator is already authorized by the sender. This method MUST throw if authorized is false and operator is unauthorized.

This method MUST throw if msg.sender == operator.

This method MUST trigger an AuthorizeOperator event if it doesn't throw.

approve(address operator, uint256 assetId)

Allow operator to transfer an asset without delegating full access to all assets.

Only the owner of an asset can call this method. Otherwise, the call MUST fail.

Only one address can receive approval at the same time. Clearing approval for a transfer can be done by setting the operator value to the zero address.

This method MUST trigger an Approve event if it doesn't throw.

Events

interface AssetRegistryEvents {
  event Transfer(
    address indexed from,
    address indexed to,
    uint256 indexed assetId,
    address operator,
    bytes userData,
    bytes operatorData
  );
  event AuthorizeOperator(
    address indexed operator,
    address indexed holder,
    bool authorized
  );
  event Approve(
    address indexed owner,
    address indexed operator,
    uint256 indexed assetId
  );
}

Interfaces

interface IAssetRegistry {
  function totalSupply() public view returns (uint256);

  function exists(uint256 assetId) public view returns (bool);
  function ownerOf(uint256 assetId) public view returns (address);

  function balanceOf(address holder) public view returns (uint256);

  function assetByIndex(address holder, uint256 index) public view returns (uint256);
  function assetsOf(address holder) external view returns (uint256[]);

  function transfer(address to, uint256 assetId) public;
  function transfer(address to, uint256 assetId, bytes userData) public;
  function transfer(address to, uint256 assetId, bytes userData, bytes operatorData) public;

  function transferFrom(address from, address to, uint256 assetId) public;
  function transferFrom(address from, address to, uint256 assetId, bytes userData) public;
  function transferFrom(address from, address to, uint256 assetId, bytes userData, bytes operatorData) public;

  function approveAll(address operator, bool authorized) public;
  function approve(address operator, uint256 assetId) public;

  function isAuthorizedBy(address operator, address assetOwner) public view returns (bool);
  function isApprovedFor(address operator, uint256 assetId) public view returns (bool);
  function approvedFor(uint256 assetId) public view returns (address);
}

Implementation

https://github.com/decentraland/erc821

Revisions

Copyright

Copyright and related rights waived via CC0.

ImAllInNow commented 6 years ago

As far as I understand it from the implementation, assetByIndex will constantly change (slightly) as users trade their assets around (since the last item in their asset array is moved around). Is there any real need for the assetByIndex function then since that ordering is not very useful? An indexOfAsset(assetId) function seems more necessary as the main way UI is going to display your assets is to use something like the following:

var assetIds = assetregistry.assetsOf(user);
var sortedAssets = new Array(assetIds.length);
for (var i = 0; i < assetIds.length; ++i) {
    sortedAssets[assetregistry.indexOf(assetIds[i])] = assetId;
}

Edit: It may be that assetByIndex should be implemented to return the asset such that it's actual id in the _indexOfAsset map == index (which it doesn't in your current implementation). It could be that assetByIndex needs to be implemented more like below:

function assetByIndex(address holder, uint256 index) external view returns (uint256) {
    uint size = _assetsOf[holder].length;
    require(index < size);
    for (uint i = 0; i < size; i++) {
        uint256 asset = _assetOf[holder][i];
        if (_indexOfAsset[asset] == index) {
           return asset;
        }
    }
    assert(1 == 0);
}
ImAllInNow commented 6 years ago

I don't think create() (and possibly update()) should necessarily be part of a standard ERC821 token. They seem optional, and for many assets, their creation probably won't be a publicly accessible function and updating might not be relevant for that contract.

Edit: Thinking about it more, I think there is a potential to front-run the create() function too. If a malicious user sees a create call being made pending, they could create that same asset for themselves first. It would make managing these assets difficult to have to try and ignore maliciously added assetIds.

eordano commented 6 years ago

Thanks @ImAllInNow!

1) Regarding assetByIndex. The idea behind this is to have that accessed from a smart contract. Use case: access from another contract for migrating all assets in batch mode to a new owner. allAssets might consume too much gas, or kill the evm stack

2) You're right about create. Front-running makes it a no-go. I'll keep them in the standard implementation as internal.

eordano commented 6 years ago

Regarding update and the metadata: is there really a case where no errata would be published? For my quick checklist:

The rule of the owner being the one authorized to make the changes is policy. You're right, it should not be part of the standard (same as before, I'll keep the mechanism as internal so the implementor can decide on the policy)

eordano commented 6 years ago

Last note on assetsOf: there's a problem of concurrency, it would be nice to have a function that atomically returns all assetIds and all the associated data (otherwise if the order changes between fetches the UI will display erroneous information).

I'll add a note that UIs should list the items in sorted order, or keep their own information based on the transfer logs or user preferences.

ImAllInNow commented 6 years ago

@eordano You're right about update. I was getting it confused with updating the assetId rather than the data associated with the assetId.

abcoathup commented 6 years ago

For those coming straight to ERC821 @eordano has written a distinguishable asset manifesto: https://blog.decentraland.org/the-non-fungibles-revolution-of-2018-304270525b05

abcoathup commented 6 years ago

My original thinking for (unique) assets was given existing ERC20 support in wallets was to create assets that are ERC20 compatible, rather than get wallets to support (unique) assets but if we can get enough support for a unique assets standard for wallets to implement, then all the better.

abcoathup commented 6 years ago

Cost For me reducing the cost of creation and send are hugely important. So want to think about how the standard impacts the implementation and hence the cost.

ERC821 meta data (Similar to my comments on ERC721) For assets to be displayed in wallets we ideally want a name, logo and possibly a symbol representing the assets in ERC21 registry. MetaMask uses name and logo with optional symbol in their metadata. https://github.com/MetaMask/eth-contract-metadata Ideally the logo would be non-optional, though there will likely be use cases where assets won’t want to be displayed in wallets

CUD CUD access may depend on the implementation. E.g.: Create may only be called by the contract owner. Update may only be called by the contract owner or the asset owner depending on what is in data. Destroy may only be called by the asset owner.

Send Does the standard need two send functions?

eordano commented 6 years ago

Thanks for the comments, @abcoathup! Please note that I've updated the body of the proposal to include as much detail as I thought necessary

1) Cost: I can run some benchmarks tomorrow and give you some estimate

2) Should we add a logo? Should there be some kind of specification, perhaps following iOS or Android's standard for logo sizes?

3) CUD: Updated to reflect your thoughts. Note:

Our implementation tries to follow the UNIX principle of providing mechanisms, not policy. That's why it contains functions to create, update, and destroy tokens, but they are marked as internal.

4) Does it save a significant amount of gas? I think not, except perhaps for calls between contracts?

abcoathup commented 6 years ago

@eordano rather than description for the DAR, I was thinking this should be data (metaData) in the same format as the data for the NFT and include a logo.

Suggest reaching out to wallets and dApp browsers (e.g. the likes of MetaMask, Toshi, Status.im and Cipher Browser) for input on logo specifications.

abcoathup commented 6 years ago

Is there a need for both holderOf and 'safeHolderOf' when there is an exists?

Should all functions that accept an assetId require that it exists so there is consistent behavior?

eordano commented 6 years ago

safeHolderOf is a cheaper way of calling exists and holderOf.

Discussion comes from #721 throwing on ownerOf if the asset doesn't exist. I think this prevented exists from being calculated.

My take on this is that both are valid use cases... that you might want to save some gas by just calling safeHolderOf, but that there might be some useful way of checking holderOf and calculating exists yourself. For example, when used as an external function and you want to do only one roundtrip.

The other functions that accept an assetId are not relevantly modified by this behavior, except maybe on the case of assetData, but it might be expected that safeHolderOf would be called first, or the logic implemented "client"-side.

AnthonyAkentiev commented 6 years ago

Thx for the great idea. I think that you have to describe how #821 is different/better from the #721.

As long as #721 was added before, it is your goal to clearly describe why you decided to add new non-fungible token standard. Thx again.

eordano commented 6 years ago

I'll expand later, but for the time being:

1) If I have 100 NFTs, calling approve 100 times with the same to value (a decentralized exchange probably) is very inefficient, specially given that it's usually used that way (see Etherdelta, 0x) 2) Lack of exists function in #721 -- There's no way to do a "try-catch" for ownerOf from a contract. 3) Lack of transferAndCall functionality -- More than 4 million USD have been lost according to https://github.com/Dexaran/ERC223-token-standard. It's critical that we have a way to not loose tokens for sending them to a contract that doesn't handle it. 4) Possibility to log more information in the Transfer event, following #777

dete commented 6 years ago

Lots of good thinking has gone into this. Decentraland brings a perspective to NFTs that we don't have at CryptoKitties. Thanks for bringing this to the community, @eordano!

I'm confused as to why you think this needs to be an whole new standard, tho. I don't know about anyone else, but I don't consider ERC-721 as finalized, and I'm not sure it's appropriate to have a whole new standard that is largely the same.

I appreciate that I've been a bad steward of ERC-721 for the last few weeks, and I can see how you might be frustrated by the lack of forward progress, but I would rather we find a way to move forward with one standard, rather than two. I mean, it's kind of the point of standards! 😅

eordano commented 6 years ago

Hey Dieter, I thought this was a better way to express the wholeness of the changes I was proposing (as you see, they are quite a lot).

The three things were I think we've got completely different approaches are:

dete commented 6 years ago

Using authorizeOperator instead of individually approve-ing every asset

I like this! I haven't had a chance to integrate it into the 721 draft, however.

The need for transfer to prevent tokens from getting lost to contracts that can't handle them

Unfortunately, we do disagree on this. I think that preventing user errors is a porcelain problem, not a plumbing problem. It's not the job of the filesystem to decide if the file you've named .jpg is actually compliant with the JPEG standard. (That's admittedly an exaggerated analogy, but I don't think Ethereum will ever see mass adoption if we continue to assume that users will be directly interacting with smart contracts!)

Upgradability through #820

EIP-820 is obviously very new, and looks pretty smart. So far as I can tell, it doesn't dictate anything about how an interface is structured, though, so it should be possible to use it for any standard past or future (even good ol' ERC-20).

ImAllInNow commented 6 years ago

I agree with @dete that moving towards one NFT standard is preferable to multiple standards especially when this concept is new and has not been specifically supported by wallets/exchanges yet (that I know of).

Definitely some good improvements suggested in the interface as described here. I like the use of an authorized operator and the idea of storing an index mapping to allow for a users assets to be stored as an array for easy retrieval through MetaMask without timing out when there are large numbers of assets (I recently noticed I can't retrieve all CryptoKitties for a user anymore :( ).

I think deciding to prevent user transfer errors can be left up to the contract itself. It is a slight extra cost to make this check so if a contract wants to add to the gas cost of transfers to protect their users, that's great, but I don't think it necessarily needs to be mandated in the standard.

simondlr commented 6 years ago

I think that preventing user errors is a porcelain problem, not a plumbing problem.

I think regardless whether the UI should prevent the user from doing this (I think it should), it's still valuable to have another contract be notified with only requiring 1 transaction (from a plumbing perspective).

macalinao commented 6 years ago

Have you considered implementing something for lienholders?

Let's say you have a plot of LAND that you are using as collateral for a loan, e.g. a mortgage. What if there was a way to allow the lender contract to transfer ownership of a plot of LAND without the land owner being able to revoke this access?

shrugs commented 6 years ago

I expect lienholders (new word for me!) to be a strong usecase for NFTs, especially as we move to a tokenized future. Think loans collateralized by fine art (or, hell, cryptokitties). Same with, say, propy houses.

It'd be possible to create that functionality with a "wrapper" contract (like an escrow), but that's not entirely ideal.

Could either be an optional extension to this EIP (nice) or another EIP (long process).

hyperfekt commented 6 years ago

I suggest that every ERC821 asset registry must register the "IAssetRegistry" interface with ERC820, that way contracts can automatically discern if a given NFT registry conforms to this standard.
An example of how that would be used: https://github.com/hyperfekt/ethereum-auctions/blob/master/NFTAuction.sol#L29

AusIV commented 6 years ago

What is the operatorData argument of transfer() for? The description of "the operatorData argument" is a self-referential description, and says nothing about what data goes there.

I'm kind of assuming it's just a blob of information that people can use when they need to communicate something not defined by the spec, but I feel like it needs more clarification.

AusIV commented 6 years ago

I feel like there's still room for an approve() call.

Calling recipient.onAssetReceived() eliminates part of the need for approve() in that you an send an asset and invoke the recipient in the same call, rather than needing to call approve(), invoke a separate contract and have it execute transferFrom().

Having authorizeOperator() addresses another part of the need for approve(), for the scenario where there may be a considerable period of time between authorization and transfer (such as waiting for an order to be filled on an exchange). But I feel authorizing another account to transfer all of your registered assets, much like authorizing a contract to transfer an unlimited number of your ERC20 tokens, is a bit of an anti-pattern. It saves gas, which is obviously not insignificant, but it also means that if the security of authorized operator is compromised all assets are subject to transfer, not just the ones you intended that operator to transfer on your behalf.

In light of the numerous security compromises we've already seen (the DAO hack, Parity Multisig Wallets Rounds 1 and 2) building a standard on the assumption that contracts will always work as intended seems short sighted. An approve() method would allow people to be more conservative, only risking assets they anticipate transferring in the event that the operator contract gets compromised. That would come at a higher gas cost for the authorizer, but if they are confident in the contract they are authorizing, they would still have the option to call authorizeOperator().

eordano commented 6 years ago

@AusIV the operatorData is there for protocols like 0x where you'd like to log some information about why the transaction was successful, for better auditability. For example, you could have the market making order be the operatorData, and the userData would be whatever arguments you want to pass on to the contract receiving the asset.

I think you're right about allowing approve as well as approveAll. I was thinking that one could just have a "filter" contract that can take care of this granularity, to save the complexity of implementing that to the DAR, but now that I think of it again, I'm not convinced. What do you think?

eordano commented 6 years ago

@hyperfekt I think the correct way to go about this is that the DAR should register the ERC821 interface (an empty interface, just using it for the name) as a way of signaling that it implements the standard... what do you think? In any case, I added the isERC821() method because of feedback from potential users that they want to have an easier way to check for compatibility.

eordano commented 6 years ago

I just updated the post with the following changes:

Historical versions can be seen at: https://github.com/decentraland/erc821/blob/master/ERC821.md

hyperfekt commented 6 years ago

@eordano What makes you propose an empty interface? I might be confused what you mean by this - shouldn't every registry announcing ERC821 support offer the full interface?

One thing that I'm a bit concerned with is the balanceOf alias - this could lead users who are not aware of the non-fungible nature to accidentally transfer random NFTs with id <= assetCount instead of that number of tokens, could it not?

hyperfekt commented 6 years ago

Also I fear that allowing to detect support via a method seriously hampers upgradeability in the spirit of ERC #820, where an old NFT registry can have the new methods added. This of course only works if it's default or required to only use the ERC821 methods with the implementer ERC820 returns (maybe this condition should be part of ERC820?). Please tell me if I've misunderstood how ERC820 upgradeability is supposed to work. /cc @jbaylina

AusIV commented 6 years ago

What are you thinking in terms of a filter contract? Would that be something the user could specify that would be called to determine whether a given operator was allowed to move a given asset? That might work, but I'd hope to see some kind of pattern established for what it would look like. One thing to keep in mind is that people implementing DARs are expected to have some level of competency in solidity, (or perhaps other EVM languages) while people merely holding assets on them very likely don't. I would avoid putting onus on the asset holders to manage other contracts.

The other factor is that there will be template DARs that will likely already implement a lot of the complexity of handling approvals, and so long as those templates are well implemented I think most DARs will implement it correctly. With filters and operator contracts, on the other hand, it's likely that each one will be unique, making it more likely that some will be implemented badly and compromise the assets of users that depend on them.

eordano commented 6 years ago

@hyperfekt People implementing DARs would be organizations creating new kinds of digital assets. The DARs don't have to register themselves as IAssetHolder (in fact, they probably shouldn't -- that's why 223 initially started, token contracts usually are not meant to hold tokens). But they might choose to register the "erc821" interface and point the resolution to the same contract.

eordano commented 6 years ago

@AusIV The fact of the matter is that the approve function is used with ~infinite values in practice. We built an application with non-infinite approve values and it was a pain to handle that. Wallets would be the implementators of the filtering functionality --supposedly experts as well.

To summarize, on one side:

On the other:

Three more comments against single approves:

hyperfekt commented 6 years ago

@eordano I think you might have misunderstood me. I was saying registries should register themselves as implementing "IAssetRegistry", not "IAssetHolder".

I'd like to support the argument for 'filter contracts'. Take a look at #777 where a perfectly analogous discussion is happening right now. This is a very important feature that would greatly increase flexibility, reduce the trust necessary in other contracts and simplify the development of many applications. It would also allow people who need it to implement single item approve on top.

eordano commented 6 years ago

Oh I see @hyperfekt sorry for the confusion! Thanks for the correction. I'll take a look at the discussion!

AusIV commented 6 years ago

I'm not opposed to allowing unlimited operators, but I'd like to see an option to just authorize specific items. I think the answer of "hold different assets in different accounts" may be limiting, depending on the nature of the DAR. It's not hard to imagine asset types where holding assets in the same account might offer some advantage over holding them in separate ones.

Some ERC20 tokens have adopted the convention that if you set an allowance of MAX_UINT256, they won't subtract out transferFroms, making the allowance literally infinite, not just effectively unlimited since it's very unlikely that an account would ever transfer more than MAX_UINT256. I would advocate making this pattern official for ERC821. If operators have to be approved on a per-asset basis, we could have a reserved assetId (I would propose MAX_UINT256) which could grant the operator access to all assets in the DAR.

This would add a bit of complexity in that the DAR would have to track approvals on a per-asset basis, but it wouldn't require completely different structures for tracking individual asset approvals and complete account approvals, and users who prefer to save gas by authorizing the operator for all assets in one go could do so.

Assuming approvals are stored in a mapping (address => mapping (address => mapping (uint => boolean))) approvals, checking an approval for operator trying to move asset on behalf of owner could be done with:

bool allowed = approvals[owner][operator][MAX_UINT256] || approvals[owner][operator][asset]

I feel this pattern would allow people to be more conservative with their authorizations without adding too much complexity to DARs, while still allowing people who want to save gas to do so by approving the operator for all of their assets.

tenthirtyone commented 6 years ago

This may work better as a separate EIP but I wanted to bring this up because I don't see it anywhere else.

I want to use NFTs to facilitate DAO voting. In executing a proposal the DAO will call balanceOf on the Shareholder ERC-20 to tally the votes. The way balanceOf was implemented in the original 721 spec was a disadvantage. Instead of counting each NFT as 1 vote, it would be better to count them based on its weight or value within the DAR.

For my specific use case, we make an NFT representing a donation. To make the donation you purchase the NFT. The NFT is effectively a certificate/receipt. I want to weigh each of the NFTs according to their price and then have that weight count as votes in a DAO.

If balanceOf instead returned the total weight or value of a holders NFTs then the DAR would be a drop-in replacement for, or sit beside in an array, ERC-20 contracts in DAO voting.

In my case, we use an ICO to raise funds for charity and let the token holders vote in the DAO to choose charities. In time, the number of NFTs increase from the donations that are made the number of votes coming from the DAR surpass the number of votes from the ERC-20 contract.

This allows us to 'bootstrap' the organization with the original ICO donors, but then the users who are ultimately served by the DAO inherit the DAO.

For now, I've been tracking this attribute as 'voteWeight' and maintaining it in the _transfer function. It's a simple change this way, but then you have to add extra logic to the DAO to account for the separate vote source logic. Effectively duplicate code because of naming convention.

eordano commented 6 years ago

Hey @AusIV I just updated the text to add approve as in ERC721. As a result, I think that except for requiring contracts to implement onAssetReceived to receive tokens, or functions like takeOwnership or transferFrom (that can be emulated with transfer), any ERC821 can be treated like an ERC721.

cc: @dete @hyperfekt @Shrugs

eordano commented 6 years ago

@tenthirtyone I just updated the text to make it more ERC-20 look-alike. In particular, I added balanceOf. Let me know of any other questions. I think you probably need another weight contract, as the use case would assume that each NFT has some value (for our LAND that has some sense, but dealing with valuation changes is something that most likely won't apply to all DARs)

eordano commented 6 years ago

@AusIV with regards to multiple authorizations, the solution you proposed adds a lot of complexity and I'd leave it for a filtering contract. Simple ERC-721-like authorization to "transferFrom" once (as it's used in the wild) looks more adequate to me for simplicity.

AusIV commented 6 years ago

@eordano - Can you please elaborate on filtering contracts, or provide references on the topic? It's not clear to me how a filtering contract can restrict operator access after a user has granted an operator unlimited access to their NFTs.

eordano commented 6 years ago

@AusIV no longer necessary as I added an approve, but I meant something like authorizing a contract like this one to be an operator:

contract Filter is Ownable {
  address actualOperator;
  ERC821 dar;
  mapping(address => bool) authorized;
  function transfer(address to, uint asset) {
    require(authorized[asset]);
    require(msg.sender == actualOperator);
    dar.transfer(to, asset);
  }
  function authorize(uint asset, bool _authorized) onlyOwner {
    authorized[asset] = _authorized;
  }
}

where you are the owner and you have authorized this contract to be an operator. Yes, it can move any token, but the code is simple enough to be considered pretty safe to be allowed operation of all your assets.

dekz commented 6 years ago

0x is considering organising a working group to discuss and push forward the adoption of an NFT standard. I understand that it is fairly short notice, are there any interested parties who are also attending ETHDenver who would like to join such a discussion?

If there is enough interest we can carve out a time during ETHDenver and invite those who are interested or have contributed to the various proposals.

Please contact me at jacob@0xproject.com if you're interested. If this meeting eventuates an agenda and meeting notes will be made available.

macalinao commented 6 years ago

Interested @Jacob. Sent an email On Mon, Feb 12, 2018 at 16:23 Jacob Evans notifications@github.com wrote:

0x is considering organising a working group to discuss and push forward the adoption of an NFT standard. I understand that it is fairly short notice, are there any interested parties who are also attending ETHDenver who would like to join such a discussion?

If there is enough interest we can carve out a time during ETHDenver and invite those who are interested or have contributed to the various proposals.

Please contact me at jacob@0xproject.com if you're interested. If this meeting eventuates an agenda and meeting notes will be made available.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/ethereum/EIPs/issues/821#issuecomment-365083496, or mute the thread https://github.com/notifications/unsubscribe-auth/AAYfb_GJ9rec8_OT0phr3dI23NeFX-Mfks5tULnbgaJpZM4RUdR9 .

inhumantsar commented 6 years ago

The only issue I have with this PR is the naming. Distinguishable Asset Registry isn't terrible, but I think it makes sense to drop NFT here and now.

I could say to people, "This DAR tracks NFTs and their owners," but then the next words out of my mouth are always going to be "distinguishable asset registry and non-fungible token". If I don't lose their attention right then and there, I'm going to have to spend the next minute or two explaining wtf I'm talking about.

But if I could say, "This asset registry contract tracks assets and their owners," they stand a reasonable chance of following the conversation without having to feel stupid. Instead of tuning out or asking me to explain what those words mean, they're going to ask me what I'm doing with it that's so interesting.

While Asset is definitely better than NFT, it's not specific enough. That's why the spec calls it a distinguishable asset registry. This is one area where I feel like #841 really shines. Deed nails the readability problem. It's short enough already to resist shortening by default, it pluralizes well, it's speakable, and it's implicitly non-fungible by definition.

Non-fungible token takes 18 characters and 1-2 explanations (1 to define fungible, 1 to explain what an NFT does) to do what deed does in just four letters and zero explanation. Namely, they both accurately describe a record of ownership of some unique asset.

Examples using "NFT":

Examples using "Deed":

NFT might be a household term in our circles right now and we might all be crypto- or finance-savvy devs, but the next wave of adopters won't be. I think we'd do the community a favor by making it easier to bring them (and their investors) on board. Deed Registry and Deeds do that better than Distinguishable Asset Registry and Non-fungible Tokens can.

If #821 moved to friendlier naming, I'm not sure there would be any gain in adopting #841 over this.

eordano commented 6 years ago

@inhumantsar the reality is that NFT is sticky. Deed doesn't have that catchy-ness.

I came up with "DAR" to avoid the issue of having to differentiate between "NFT" and "the NFT contract" (something that happens a lot with ERC20s). It actually stuck with some other people I have discussed this standard with.

inhumantsar commented 6 years ago

Organizational inertia is a terrible excuse to maintain any practice though. If everyone just stuck to whatever came first, we wouldn't be discussing this vs. #841.

I'll agree that "Deed" isn't the sexiest word for engineers to use but engineers aren't the only people we need this to catch on with. We can use a term other than Deed, but intentionally moving toward jargon and opaque acronyms is anti-noob, which is anti-adoption.

To drive mass-market adoption, it will help immensely to name it with enthusiastic laypeople in mind. They're the ones who are going to be talking to the media and their friends about the things we build. If they have to spend 5-10 minutes explaining every other acronym that falls out of their face, fewer people are going to listen.

edit: For the record, I prefer Asset over Deed, but "Distinguishable" launches it into jargon land.

eordano commented 6 years ago

@inhumantsar This EIP works at the API level, so I guess it's just for developers. I think deed is less specific. You can approach the whole community to adopt using "deed" at the UX level, and even write a proposal for that, but I feel confident in ruling "enduser-friendly naming" out of scope. Not so much with other naming considerations that I suggested, like using holder instead of owner (as in the actual "owner" living outside of the blockchain and holder is the address that supposedly represents him) or operator to refer to someone who has been approved to move assets on behalf of the holder address.

inhumantsar commented 6 years ago

I guess it's just for developers

I disagree. If you take a look at the press that CryptoKitties got, you can clearly see that the high-level naming we choose, like the title of the spec, is not just for developers. In fact, it's probably the only thing from the spec that the general population is going to be exposed to.

holder and owner, by contrast, will only ever be exposed to people writing contracts or code that interacts with them.

[edit] quoted the wrong sentence, also words

moskalyk commented 6 years ago

Non-Divisibility & Multiple Ownership I believe is core to serving a Non-Fungible Token.

Whether this is incorporated in this standard, or, possibly the #874 conversation pertaining to Weighted NFTs.

Use Case: The use case where I am encountering the need, is for a National Land Registry whereby a NFT / Title has the ability to contain many owners, and the future potential of using ownership in equity.

The problem of Non-divisibility can be seen to be a limitation within the Dharma Protocol whitepaper, we see this as a limitation as well.

Current State: Right now this can be circumvented with a proxy contract

Non-Fungibility and scarcity tend to align pretty nicely for those assets of higher value. I strongly believe that the future of these tokens, will more and more require a proxy contract to facilitate Non-Divisibility & Multiple Ownership if the current state of is implemented.

Therefore, to reduce gas with less transactions / calls, increase adoption, and increase security with standardization / best practices for transferring value / delegating trust in a standardized way, I think we should discuss for #821.

@eordano, @tenthirtyone Thoughts?

I'll Note: If anyone has encountered the best way to handle the governance of N number of owners in a decentralized way please point me in the direction. Transferring ownership with multiple owners requires coordination, which I believe to be a custom function depending on the NFT token. Considerations are: voting on chain with some level of quorum consensus (but becomes expensive), or possibly voting / agreeing off chain with state channels, with on-chain settlement (only a research area now).