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

Add ENS support for addressing ERC721 NFTs #2381

Closed
wants to merge 23 commits into from

Conversation

OisinKyne
Copy link

@OisinKyne OisinKyne commented Nov 15, 2019


eip: 2381
title: ENS support for EIP-721 TokenID
author: Oisin Kyne (@OisinKyne)
discussions-to: #2381
status: Draft
type: Standards Track
category: ERC
created: 2019-11-15
requires: 137, 165, 721

Simple Summary

This EIP makes EIP-721 Non-Fungible Tokens addressable by the Ethereum Name Service (EIP-137).

Abstract

A single deployed EIP-721 contract can contain multiple non-fungible tokens. Tokens are addressed by an unsigned 256bit integer, tokenID. This EIP describes an extra getter and setter, tokenID() and setTokenID(), that are included in an ENS resolver contract, to allow for ENS names to resolve to a specific non fungible token within a given EIP-721 contract.

This allows ENS domains like bugcat.cryptokitties.eth and dragon.cryptokitties.eth to resolve to both the CryptoKitties contract address and the tokenID of the non-fungible kitty within it by leveraging EIP-721's tokenURI method with the token ID retrieved from ENS.

Motivation

It makes sense to resolve an ENS name to just a contract address for fungible tokens such as EIP-20, as each token in the contract is indistinguishable from another. However, for non-fungible tokens; pointing to the contract address alone is not enough, as tokens within the contract are meant to be unique and distinguishable. Non-fungibles might have different valuations, different artwork, might grant the holder different rights or rewards etc.

Giving NFTs names makes them more real to the non Ethereum enthusiast that doesn't understand what a contract address or tokenID is. A domain name in the existing web 2.0 world is already widely understood to be a finite resource, and as such, would help convey the scarcity that is a non-Fungible token to new users.

This change might also to an extent, decentralise access to non-fungibles on Ethereum. Currently, the mapping for a human readable name to an NFT is kept off chain, typically on the platform of the NFT issuer. However, if the community moved towards naming NFTs on chain, using ENS, this would allow any client that supports ENS resolution to resolve an ENS name to an NFT, without querying the issuer's off-chain metadata.

Finally, this EIP aims to address fraud in the NFT space. Many famous NFTs are being saved and reuploaded by malicious scammers with the intention of defrauding investors into purchasing rip offs of a real NFT. With ENS NFTs, a digital artist can use their well-known ENS name to confer legitimacy to the canonical NFT representation of their artwork to distinguish it from the imposters.

Specification

The ENS Resolver Profile specified by this EIP adds two functions to a deployed resolver contract:

function tokenID(bytes32 node) public view returns(uint256);
function setTokenID(bytes32 node, uint256 token);

You can evaluate whether a given ENS resolver supports this EIP by using the EIP-165 standard supportsInterface(bytes4).

The interface identifier for this interface is 0x4b23de55.

Rationale

The design of this smart contract is a modified replica of the existing contenthash resolver profile.

The decision to use Contract Address and tokenID as a pair to uniquely identify EIP-721 NFTs is lifted directly from the EIP-721 Rationale quoted here:

Every NFT is identified by a unique uint256 ID inside the EIP-721 smart contract. This identifying number SHALL NOT change for the life of the contract. The pair (contract address, uint256 tokenId) will then be a globally unique and fully-qualified identifier for a specific asset on an Ethereum chain.

Backwards Compatibility

No backwards compatibility issues arise. Users wanting to address NFTs will have to deploy new ENS resolvers themselves, or use the provided PublicResolver below.

Implementation

The following code serves as a sample implementation of this tokenId resolver:

pragma solidity ^0.5.8;

import "../ResolverBase.sol";

contract TokenIDResolver is ResolverBase {
    bytes4 constant private TOKENID_INTERFACE_ID = 0x4b23de55;

    event TokenIDChanged(bytes32 indexed node, uint256 tokenID);

    mapping(bytes32=>uint256) _tokenIDs;

    /**
     * Returns the tokenID associated with an ENS node.
     * @param node The ENS node to query.
     * @return The associated tokenID.
     */
    function tokenID(bytes32 node) public view returns(uint256) {
        return _tokenIDs[node];
    }

    /**
     * Sets the tokenID associated with an ENS node.
     * May only be called by those authorised for this node in the ENS registry.
     * @param node The node to update.
     * @param token The tokenID to set
     */
    function setTokenID(bytes32 node, uint256 token) public authorised(node) {
        emit TokenIDChanged(node, token);
        _tokenIDs[node] = token;
    }

    function supportsInterface(bytes4 interfaceID) public pure returns(bool) {
        return interfaceID == TOKENID_INTERFACE_ID || super.supportsInterface(interfaceID);
    }
}

A fork of the ens/resolvers repo with the change is available, here

The ENS public resolver contract, with this new added profile, has been deployed and verified on etherscan on the following chains:

To test this, I have set this new resolver contract on mainnet to resolve devcon5.oisin.eth to my Devcon Ticket Non-Fugible.

The address it resolves to is:
0x22cc8b3666e926bcbf58cb726143b2b044c80a0c, which is the contract address of all 91 tokens issued.
And the tokenID() function returns:
10798952828109286844408842969080375883371044426718767566816061252817119618319 when you supply it with a node of 0x0532eb949a568331eccfc0b70427e8aa96bcbfcc747607711bd04275323a1b49 (which is the namehash of devcon5.oisin.eth), which is my Non Fungible within that contract, hopefully illustrating this EIPs usefulness. :)

How to check if an ENS name implements this EIP?

To make sure everything is as expected, one should follow this procedure:

  • Given an ENS name, first look up its resolver contract from the ENS registry by calling resolver(bytes32 node) on the registry contract.
  • Given an ENS name and its resolver contract address, look up its addr field, which returns an address if one is set.
  • Check if this address is an EIP-721 contract, by using EIP-165's supportsInterface method. (EIP-721 interface ID is 0x80ac58cd).
  • If this address is an EIP-721 contract, now you should check whether this name is pointing at the entire NFT contract, or whether it is pointing at a specific NFT within it.
    • First, check if the resolver contract supports EIP-2381, by using EIP-165's supportsInterface method. (EIP-2381 interface ID is 0x4b23de55).
    • If the resolver contract does not support EIP-2381, you can safely assume this name does not address a specific NFT within the EIP-721 contract.
    • If the resolver contract does support EIP-2381, you should call the function: tokenID(bytes32 node) on the resolver contract.
    • If the tokenID function returns 0, you can assume that this name is not addressing a specific NFT. (0 is not a valid tokenID in the EIP-721 standard).
    • If the tokenID function returns a non-zero value, this name is addressing a specific NFT within the contract.
  • If a tokenID is set on the resolver contract, it is advised that you verify that this tokenID exists within the EIP-721 contract, and hasn't been burned or never been minted, for example.

Security Considerations

There is no centralised party involved in this EIP, all contracts are verified on etherscan and all contracts can be redeployed trustlessly if desired.

The main security consideration to note with this EIP is the potential for fraud.

The person that controls the ENS name may not be the same person that issues or owns the NFT it points at. This may not be apparent to non-technical users. This could potentially cause confusion by convincing a user to purchase the ENS name (which is itself an NFT) rather than the NFT addressed by the ENS name.

Similarly, the ENS name owner could update the name to point at a new NFT token at any point in time. This EIP does not propose a form of reverse-resolution to allow for a canonical ENS name for an NFT and therefore should not be used as a permanent record of a particular NFT and instead should just be used as a UX or informational improvement to an NFT.

Copyright

Copyright and related rights waived via CC0.

@wighawag
Copy link
Contributor

That's a great idea.
It would cool to have a reverse registrar for it too so the owner of the token can give it a canonical name

@OisinKyne
Copy link
Author

@wighawag

That's a great idea.
It would cool to have a reverse registrar for it too so the owner of the token can give it a canonical name

I hadn't thought of that.

I think making the current owner of the token the person able to set the canonical name for it makes a lot of sense, but I'm not sure it exactly aligns with how the ownership of reverse addresses are managed in ENS, so I'd prefer to leave it out of the scope of this EIP and consider it for another one.

I've spent some time looking at the spec for reverse resolvers as they are implemented at the moment. As I understand it, both contract accounts and externally owned accounts have the ability to claim the name they are sending an ethereum call from. Specifically on the reserved tld <address>.addr.reverse. However, they can transfer the ownership of this ENS name to any address. In the documentation they suggest this is so as a contract owner, you can manage the reverse resolution of your contracts address from an external account, rather than building the reverse name administration logic into your otherwise unrelated contract.

So the closest way to treat NFT reverse resolutions to address reverse resolutions would be to do something like extend the reverse resolution spec to allow namehash(<tokenid>.tokenID.<address>.addr.reverse) as a valid reverse resolution name.

The question is, who should be able to claim the ENS name <tokenID>.tokenID.<address>.addr.reverse, the contract itself, or the token owner?

I support the idea of extending the reverse resolution spec to allow to differentiate tokens within a contract (especially if we namespace it by using the .tokenID. intermediary so many different types of reverse resolution stay possible), but I wouldn't make the ownership of the name strictly the token holder, I would allow the contract address to claim the names of it's child tokens, and implement it's own logic on how reverse resolution rights for canonical names are distributed or controlled, by current token holder getting to set it or otherwise.

@wighawag
Copy link
Contributor

Ah, yes indeed, it is not that simple.
But that complexity seems to be required because as the ENS documentation mention it allow the contract owner to manage the names without having to go through a call to the contract first. For token holder this would not be necessary : the resolver would simply restrict change based on the current token holder. So by transfering your token to someone else, you let the next token holder to change the canonical name. There would be no claim step just setting names

Regarding letting contract intervene, I am not yet sure if it is a good idea but if we do, we need to make it that by default a token holder is allowed to set its tokenId name. This is because we want such feature to be available for token contract that are already deployed. So maybe the reverse-resolver could check first with the token contract if it is able take control over the resolving process and if not let token holder to decide the name for their token.
In any case, as you mention, let's move that discussion in another EIP. It is actually more specific too since it would require at least a way to get ownership in a specific way (ERC721 ownerOf)

@mcdee
Copy link
Contributor

mcdee commented Nov 16, 2019

Would it perhaps be better to separate the existing address functionality from this feature, for example by altering the calls to:

function tokenID(bytes32 node, address token) public view returns(uint256);
function setTokenID(bytes32 node, address token, uint256 tokenID);

where address token is the address of the relevant ERC-721 token contract? Then the TokenIDResolver can store the full token information by itself and not rely on the AddrResolver profile being present.

You may also want to consider the cases where a single ENS name can resolve to multiple NFTs, with the tokens either in different ERC-721 token contracts or the same ERC-721 token contract.

@OisinKyne
Copy link
Author

Would it perhaps be better to separate the existing address functionality from this feature

I'm not really sure I would add an extra address to this field that could diverge from the main address pointed to by this name, I'd be afraid it might cause confusion.

What I would probably do is use this TokenID EIP in parallel with EIP1844, which is a resolver profile where you pass in an interface ID, and the resolver gives you back an address of a contract that implements that standard. So somebody could ask the ENS resolver, 'Give me a contract that implements ERC721', and the interface resolver would return an address, and then the caller could also ask if the resolver contract implemented this EIP to ask for a specific token within that contract. Returning a tokenID from that interfaceResolver wouldn't work however as a tokenID is uint256 not address.

https://github.com/ensdomains/resolvers/blob/master/contracts/profiles/InterfaceResolver.sol#L6

You may also want to consider the cases where a single ENS name can resolve to multiple NFTs, with the tokens either in different ERC-721 token contracts or the same ERC-721 token contract.

This may be on me, but can one ENS name point to multiple addresses at the same time? It could point to one default address, and another ERC721 address via the interfaceImplementer resolver described above, but in that case the address pointed to by interfaceImplementer would be the one to assume the tokenID relates to, due to the nature of that EIP.

The closest I could find to this was EIP2375, which uses a CNAME like resolver to have multiple names point at one address, but I'm not sure of a case where one name points to multiple addresses. (Unless they are all put into the text resolver under different keys.)

@mcdee
Copy link
Contributor

mcdee commented Nov 16, 2019

I'm not really sure I would add an extra address to this field that could diverge from the main address pointed to by this name, I'd be afraid it might cause confusion.

With multicoin (https://eips.ethereum.org/EIPS/eip-2304) there isn't so much an idea of a "main address" as there is an address for each coin type. So a single ENS domain can have multiple addresses, separated out by the type of address (Bitcoin, Ethereum, etc.).

However, in general the rationale behind having a single ENS address is more for it being a well-known and trusted recipient of funds. In that situation it makes sense for there to be a single address, whereas there are multiple NFTs. The proposal for having a single token per domain, and having multiple tokens by having multiple subdomains, is relatively heavyweight in terms of transactions (one to create the subdomain, one to set the subdomain's resolver, one to set the token). It would be cheaper, and potentially cleaner, to have multiple tokens per domain. For example, you may have an NFT that gives you access to an event, another that gives you a proof of attendance at a conference, etc. that each have their purpose and you would like to have linked to the same domain.

Perhaps a cleaner way would be to include some sort of purpose identifier. This would expand the functions (again) to:

function tokenID(bytes32 node, bytes32 purpose, address token) public view returns(uint256);
function setTokenID(bytes32 node, bytes32 purpose, address token, uint256 tokenID);

Where purpose would be keccak256("Devcon V attendance") or the hash of a similar string that is well-known by whomever wants to look up the presence of the token (you could use the special purpose value of 0x00 for the default token for the domain if required).

@OisinKyne
Copy link
Author

@mcdee I like that.

I guess I wanted to have a single NFT to be the highlight of a specific domain, which is still achievable with what you propose by having a default 0x0...0 purpose field, while still not capping the amount of NFTs attachable to a given name. I talk a bit more about this near the end of this overly long comment.

I spent a bit of time thinking about how this EIP might relate to a multi-chain ENS future, and whether multi chain support differed from multiple token support on a single chain. In the end I think the variety of implementations mean multichain support for ERC721 tokens doesn't really make sense for this EIP in my opinion, so you're welcome to skip past the next few paragraphs to the purpose question if you're skimming these comments.

function tokenID(bytes32 node, bytes32 purpose, address token) public view returns(uint256);
function setTokenID(bytes32 node, bytes32 purpose, address token, uint256 tokenID);

Why not?

function tokenID(bytes32 node, bytes32 purpose, uint cointType, bytes tokenAddress) public view returns(uint256);
function setTokenID(bytes32 node, bytes32 purpose, uint coinType, bytes tokenAddress, uint256 tokenID);

I suppose I would ask myself, what other chains also support tokens on top of them, and similarly address NFTs with a uint256, Ethereum-derived chains, maybe, but then they don't need the added complexity of having to parse addresses as bytes to support the encoding of other chain addresses.

But:

function tokenID(bytes32 node, bytes32 purpose, uint cointType, address token) public view returns(uint256);
function setTokenID(bytes32 node, bytes32 purpose, uint coinType, address token, uint256 tokenID);

Would be a different implementation to the addr overload in Multi-Chain Support, so that might not be a good idea. I also wonder what part chainID might play in this if you wanted to have NFTs addressable across Ethereum chains like:

function tokenID(bytes32 node, bytes32 purpose, uint chainID, address token) public view returns(uint256);
function setTokenID(bytes32 node, bytes32 purpose, uint chainID, address token, uint256 tokenID);

This raised a question for me with respect to multichain ENS naming; If you're an Ethereum-like chain, adopting ENS naming, does the main ethereum network become the canonical location for looking up a name, or do you look up the ENS registry on your own deployed chain?

In my opinion I think I'm answering my own question about whether this should support multiple tokens, single chain, or single/multiple tokens, multiple chains in favour of the multiple tokens, one chain model. Multiple chains doesn't make so much sense due to the wide variety of token/lack of token concepts on other chains coupled with other implementations of non fungible tokens. The only case I see where it would make sense would be other Ethereum chains. And I think you can just deploy this contract on a different Ethereum chain no problem, unless that is non-Ethereum names use the mainnet Ethereum ENS registry as the authoritative place for names on their chain, I don't actually know if that's the case or not, I would have thought that would delegitimise their chain.

Back to the topic of the purpose field:

From a gas, complexity saving measure, I wonder is there any way to infer purpose from the token's address? Maintaining a semi authoritative list of 'Purposes' for reverse resolution of these values would be difficult, also if I understand the discussion around the multichain debate, they would not be enumerable/discoverable by being in a mapping.

I wonder, would a token's recorded purpose and the contract address it lives in ever need to change/diverge? Would it be possible to infer a purpose for a non fungible token by reverse resolving the token contracts name into an ENS name, or other ENS data?

Maybe we could go with your first suggestion of just adding address, without purpose, and if you wanted to explicitly single any one of the NFTs out as the primary focus of this domain,, that could be indicated by the resolver user by using Interface Discovery to return that NFT's particular ERC721 token address if asked for one using it's interface ID. If the NFT was the only thing you wanted this name to point to you could set addr too for good measure.

So that would end up looking like:

function tokenID(bytes32 node, address token) public view returns(uint256);
function setTokenID(bytes32 node, address token, uint256 tokenID);

What are your thoughts on that?

@OisinKyne
Copy link
Author

@mcdee

I've been thinking about this some more, if we used:

function tokenID(bytes32 node, address token) public view returns(uint256);
function setTokenID(bytes32 node, address token, uint256 tokenID);

And we modified the above resolver storage to be a mapping like:

mapping(bytes32=>mapping(uint256=>address)) _tokenIDs;

This would map name to tokenID, to address, there is a chance of collision between addressing two tokens with the same tokenID but from different contracts (plenty of tokens count up from index 0). The other option I see of:

mapping(bytes32=>mapping(address=>uint256)) _tokenIDs;

Would map name to address to tokenID, but with this setup, you could only store one token per contract address, also not ideal.

Is there a better data structure I can be using than nested mappings for this? I could use an array, but that comes with complexity around gas limits limiting how many tokens you can assign to one domain, and array traversal issues in general.

But also, in favour of the array over a mapping, if you don't know what tokens a domain holds, there isn't a good way to list all values of a mapping, If our mapping is bytes32 node -> [{address, uint256},{}] you could loop through them listing all of them.

@OisinKyne
Copy link
Author

@mcdee

I've still been trying to figure out a good way to blend the want for multiple tokens to be addressable by one ENS name with the challenges of making an efficient data structure for storing them.

I was wondering, if we go with the mapping approach (ens -> address -> tokenID or ens -> tokenID -> address), and as such, the individual tokens are not enumerable (no easy way to GET all), it makes it scaling this easier as a lookup is O(1) rather than linear, and you don't risk running out of gas. The tradeoff however is not being able to address two tokens from the same contract, or alternatively, not being able to address two tokens with the same tokenID. I think with this collision problem, coupled with the fact that this structure wouldn't allow tokens to be discoverable should rule this approach out. I also think, this behaviour is equivalent to pointing an ENS name at a wallet, and then ENS browsers inferring what tokens a wallet holds by asking well known NFT contracts whether this address has a balance.

The second approach of using an array, although making tokens enumerable, has major limitations due to array traversal issues and gas costs associated with traversing an array. I could see this approach being worth investigating, but I don't actually know how much people would use this feature.

Do you feel strongly about having multiple NFTs addressable by a single ENS name in a manner other than having an ENS name that points at an address that holds a variety of NFTs? I get that they could have different use cases, but I wonder is the use case it solves for so niche that it might take away from the adoption of the simple case (1 name, 1 NFT) that I am hoping to solve with this EIP due to the complexity of traversing arrays or guessing contract addresses that might have a balance?

@mcdee
Copy link
Contributor

mcdee commented Nov 22, 2019

Do you feel strongly about having multiple NFTs addressable by a single ENS name in a manner other than having an ENS name that points at an address that holds a variety of NFTs? I get that they could have different use cases, but I wonder is the use case it solves for so niche that it might take away from the adoption of the simple case (1 name, 1 NFT) that I am hoping to solve with this EIP due to the complexity of traversing arrays or guessing contract addresses that might have a balance?

I do think it would be a significant advantage, but equally it doesn't come without downsides. Mapping would be the best way to proceed; I don't have a problem with inability to iterate over tokens, although it could be done by having an array of token IDs at the end of the (user=>token) mapping.

Ultimately it's your call as to if you believe the additional work (and gas) is worth the benefit and complexity.

@axic axic added the type: ERC label Nov 22, 2019
@OisinKyne
Copy link
Author

@mcdee

I appreciate your input on this, but I think the mapping approach of (ens node -> contract address -> tokenID) won't really work for the use case I am working on that made me write this EIP, as I would specifically need to be able to name multiple tokens within the same contract, so a mapping based on address could only hold one token from any given contract. Alternatively, a mapping based on tokenID (ens node -> tokenID -> address) could have weird behaviour and collisions due to overlapping tokenIDs in distinct contracts, so I wouldn't like to go down that route either.

I am building a product to make it easier for (non blockchain savvy) customers to create NFTs. I wanted to avoid having to deploy a new ERC721 contract per token or batch of tokens, in favour of simply deploying one ERC721 contract per customer, and every NFT they mint goes into the same contract.

So instead of a create opcode every time you wanted a new token, we would have one create when a user registers for the first time, and every time they want a new NFT, my backend just does a call to mint() instead of a create, which would save a significant amount of gas (it costs approx 2.05mill for create vs 70k for a mint), and this design decision would also save me creating a new off chain table to record the 1 to N mappings of users to their various deployed ERC721 contracts, because every user has just 1 contract and its address can go in the one table.

The question was raised to me, 'how do you differentiate between different tokens in a single contract?' The business suggested that the tokens should all go in separate contracts depending on a customers self described category, but I am of the opinion that the only segregation of tokens we need to do is at the human level. Code and smart contracts don't care whether a contract has 10000 or 10 tokens within it, it's the users that want to see a sensible segregation of tokens. Also these tokens are non-fungibles, and aren't meant to be indistinguishable, we are also working on making ERC20s easily issuable also.

As a result, I came up with this EIP, of allowing you to single out and identify a token via an ENS name. So if you want to draw attention to a given token specifically, you can assign it a name in ENS. How I see this being used in practice is, customers will just point their top level ENS name for their company/shop at the token contract, but if they want to go out of their way to highlight a specific token, they can create an ENS subdomain especially for said token.

Customers that want to highlight twenty tokens all with the same name suggest to me that those 20 tokens are fungible in reality, and as such could be in an ERC20 contract with 0 decimal places instead. This can be done under the hood, and the customer won't need to know the differences down to the contract level. They'll just want an ENS subdomain name that returns a 20 token group.

So if it's cool with you, I might leave this EIP as unchanged then. I really appreciate all of the extra thought you have made me put into this proposal though!

@mcdee
Copy link
Contributor

mcdee commented Nov 27, 2019

So if it's cool with you, I might leave this EIP as unchanged then.

Of course; they were just suggestions.

EIPS/eip-2381.md Outdated Show resolved Hide resolved
EIPS/eip-2381.md Outdated Show resolved Hide resolved
@github-actions
Copy link

github-actions bot commented Sep 8, 2020

There has been no activity on this pull request for two months. It will be closed in a week if no further activity occurs. If you would like to move this EIP forward, please respond to any outstanding feedback or add a comment indicating that you have addressed all required feedback and are ready for a review.

@github-actions github-actions bot added the stale label Sep 8, 2020
EIPS/eip-2381.md Outdated Show resolved Hide resolved
EIPS/eip-2381.md Outdated Show resolved Hide resolved
EIPS/eip-2381.md Outdated Show resolved Hide resolved
EIPS/eip-2381.md Outdated Show resolved Hide resolved
EIPS/eip-2381.md Outdated Show resolved Hide resolved
- The contract address pointed at by the resolver of an ENS name implements ERC721 (Interface ID of: `0x80ac58cd`)
- One specific edge case to be aware of. In the ERC721 spec, a `tokenID` of 0 is a valid identifier of a token. If an ENS name uses this new resolver contract, but doesn't set a `tokenID` value, this field will default to 0, which implicitly is a valid `tokenID`. If an ERC721 contract owner wants to address their contract with an ENS name, but does not want to address any specific token within it, they should use a resolver contract without this EIP included, to avoid the case where an ENS resolver infers that this name is to point to `tokenID` 0 within the contract, which may or may not exist. The alternative for this edge case would be to make this EIP require that `tokenID` be non-zero to be valid, but this would make specific tokens with `tokenID` of 0 not addressable by this EIP, which would not be ideal, considering they are compliant tokens in the 721 spec.

## Copyright
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
## Copyright
## Security Considerations
TBD
## Copyright

Security Considerations is a required section.

Copy link
Author

Choose a reason for hiding this comment

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

I wrote a section for Security Considerations, it could probably be better though. Advice welcome. https://github.com/OisinKyne/EIPs/blob/master/EIPS/eip-2381.md#security-considerations

Copy link
Contributor

Choose a reason for hiding this comment

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

A security considerations section could just read "there are no known security issues associated with this EIP", it doesn't have to be complicated. :) In this case, I think you have some good info in here though that is worth mentioning.

EIPS/eip-2381.md Outdated Show resolved Hide resolved
EIPS/eip-2381.md Outdated Show resolved Hide resolved
EIPS/eip-2381.md Outdated Show resolved Hide resolved
EIPS/eip-2381.md Outdated Show resolved Hide resolved
@OisinKyne
Copy link
Author

Thanks for the comments @MicahZoltu and @axic. I'm currently building a UI to demo this EIP and I will make sure to address these before I refresh it.

@github-actions github-actions bot removed the stale label Sep 9, 2020
Copy link
Contributor

@MicahZoltu MicahZoltu left a comment

Choose a reason for hiding this comment

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

Going to leave this unmerged for the moment in case you want to apply some of the feedback I left before merging to draft. Mention me if you want me to merge now, and then you can address the feedback (optionally) in PRs against this after it is a draft.

EIPS/eip-2381.md Outdated Show resolved Hide resolved
EIPS/eip-2381.md Outdated Show resolved Hide resolved
EIPS/eip-2381.md Outdated Show resolved Hide resolved
EIPS/eip-2381.md Outdated Show resolved Hide resolved
EIPS/eip-2381.md Outdated Show resolved Hide resolved
EIPS/eip-2381.md Outdated Show resolved Hide resolved
@OisinKyne
Copy link
Author

Cheers thanks for the feedback @MicahZoltu let's leave it unmerged for now and I will address these and look to get it merged in a week or two. :)

@OisinKyne
Copy link
Author

Hi @MicahZoltu

If you are happy with how things are looking, I am game to merge it as a draft now and we can clean up any outstanding issues via PRs. :)

Copy link
Contributor

@MicahZoltu MicahZoltu left a comment

Choose a reason for hiding this comment

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

Only required change for draft is the discussions-to link. The rest you can address at a later phase.

eip: 2381
title: ENS support for EIP-721 TokenID
author: Oisin Kyne (@OisinKyne)
discussions-to: https://github.com/ethereum/EIPs/issues/2381
Copy link
Contributor

Choose a reason for hiding this comment

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

discussions-to must not be the PR itself. https://ethereum-magicians.org/ is an ideal target for a discussions-to link, but a separate GitHub issue would be appropriate as well.

Comment on lines +100 to +114
A fork of the [ens/resolvers](https://github.com/ensdomains/resolvers) repo with the change is available, [here](https://github.com/OisinKyne/resolvers/blob/master/contracts/profiles/TokenIDResolver.sol)

The ENS public resolver contract, with this new added profile, has been deployed and verified on etherscan on the following chains:

- Rinkeby Testnet: [0x9d5dd30b5d77665f0c2f082cccc077d349ba1afc](https://rinkeby.etherscan.io/address/0x9d5dd30b5d77665f0c2f082cccc077d349ba1afc)
- Ropsten Testnet: [0xf39f73b0c748d284dcea3f0da8bbdefa7a789c6b](https://ropsten.etherscan.io/address/0xf39f73b0c748d284dcea3f0da8bbdefa7a789c6b)
- Goerli Testnet: [0x2618f1ed8590cb750489ce5de0d1c05d8375bbdf](https://goerli.etherscan.io/address/0x2618f1ed8590cb750489ce5de0d1c05d8375bbdf)
- Mainnet: [0xb2eef9d0235a339179a7e177e818439dcca9d76e](https://etherscan.io/address/0xb2eef9d0235a339179a7e177e818439dcca9d76e)

To test this, I have set [this](https://etherscan.io/address/0x888ab947cb7135dc25d4936e9a49b4e2bcdea467) new resolver contract on mainnet to resolve [`devcon5.oisin.eth`](https://etherscan.io/enslookup?q=devcon5.oisin.eth) to my Devcon Ticket Non-Fugible.

The address it resolves to is:
[0x22cc8b3666e926bcbf58cb726143b2b044c80a0c](https://etherscan.io/token/0x22cc8b3666e926bcbf58cb726143b2b044c80a0c), which is the [contract address](https://etherscan.io/token/0x22cc8b3666e926bcbf58cb726143b2b044c80a0c) of all 91 tokens issued.
And the `tokenID()` function returns:
[10798952828109286844408842969080375883371044426718767566816061252817119618319](https://etherscan.io/token/0x22cc8b3666e926bcbf58cb726143b2b044c80a0c?a=10798952828109286844408842969080375883371044426718767566816061252817119618319) when you supply it with a node of `0x0532eb949a568331eccfc0b70427e8aa96bcbfcc747607711bd04275323a1b49` (which is the namehash of `devcon5.oisin.eth`), which is _my_ Non Fungible within that contract, hopefully illustrating this EIPs usefulness. :)
Copy link
Contributor

Choose a reason for hiding this comment

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

EIPs should avoid external links whenever possible, especially to places that don't have strong guarantees of the link not moving or eventually disappearing. My recommendation is just to delete this whole section as it doesn't add a lot of value to the specification itself. It would be great content for a blog, readme, docs site, etc. though.


To make sure everything is as expected, one should follow this procedure:

- Given an ENS name, first look up its resolver contract from the [ENS registry](https://docs.ens.domains/contract-api-reference/ens) by calling `resolver(bytes32 node)` on the registry contract.
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment as above, try to avoid external links whenever possible. In this case, we can simply remove the link:

Suggested change
- Given an ENS name, first look up its resolver contract from the [ENS registry](https://docs.ens.domains/contract-api-reference/ens) by calling `resolver(bytes32 node)` on the registry contract.
- Given an ENS name, first look up its resolver contract from the ENS registry by calling `resolver(bytes32 node)` on the registry contract.

- The contract address pointed at by the resolver of an ENS name implements ERC721 (Interface ID of: `0x80ac58cd`)
- One specific edge case to be aware of. In the ERC721 spec, a `tokenID` of 0 is a valid identifier of a token. If an ENS name uses this new resolver contract, but doesn't set a `tokenID` value, this field will default to 0, which implicitly is a valid `tokenID`. If an ERC721 contract owner wants to address their contract with an ENS name, but does not want to address any specific token within it, they should use a resolver contract without this EIP included, to avoid the case where an ENS resolver infers that this name is to point to `tokenID` 0 within the contract, which may or may not exist. The alternative for this edge case would be to make this EIP require that `tokenID` be non-zero to be valid, but this would make specific tokens with `tokenID` of 0 not addressable by this EIP, which would not be ideal, considering they are compliant tokens in the 721 spec.

## Copyright
Copy link
Contributor

Choose a reason for hiding this comment

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

A security considerations section could just read "there are no known security issues associated with this EIP", it doesn't have to be complicated. :) In this case, I think you have some good info in here though that is worth mentioning.

@makoto
Copy link

makoto commented Nov 23, 2020

Hi, @OisinKyne Thank you for pointing to this EIP. Didn't realise this proposal existed for almost a year!

I really like the motivation and intention behind the proposal but it may make things confusing as I initially thought "Oh this is a way to turn subdomain as NFT", but it's actually not. Then I thought "Oh this is to turn ENS resolver as NFT, as opposed to registrant to be NFT which is how we did at .eth name", but that's not true either as the resolver itself is not transferrable.

It may especially become confusing if I use this proposal to matoken.eth as the name itself is already NFT (and the proposal was intended to be used on subdomain, but doesn't stop it to be used on second level domain either).

Could you explain a bit more in detail about how to use this to address fraud in the NFT space?
If you are the fan of https://twitter.com/An0nym0usNobod who is known for anonymousnobody.eth
Do you expect all the NFT buyers to visit https://app.ens.domains/name/anonymousnobody.eth/subdomains , and check the list of subdomains and their associated NFT detail? If so, what feature would you wish app.ens.domains to implement to make the experience nicer?

Also, are there any ways for NFT platforms (such as OpenSea, KnownOrigin, etc) to integrate with this EIP so that it becomes more discoverable?

Lastly, do you have some ideas on how to combat the fraud of using this system itself, as you mentioned The person that controls the ENS name may not be the same person that issues or owns the NFT it points at. at the security concern?
It would be sad if this proposal gets abused for fraud when it was intended to combat the fraud in NFT space to begin with.

@OisinKyne
Copy link
Author

Hi @makoto

Thanks for the kind words and your feedback makes sense.

The problem I wanted to solve at the start was "Sweet I've got a devcon ticket NFT, I want to address it with ENS", however the best I could do was to point at the full ERC721 contract address, I couldn't highlight a specific NFT within the contract, which I think is important, as NFTs are meant to be non-fungible, so having a naming system that can distinguish between them was important in my mind.

the resolver itself is not transferrable

Yeah the resolver is just a fork of the ENS resolver contract, found here. I will open a PR back into the main repo if this EIP is successful and adopted.

Do you expect all the NFT buyers to visit https://app.ens.domains/name/anonymousnobody.eth/subdomains , and check the list of subdomains and their associated NFT detail?

That does sound like a very involved process. The person I had in mind when talking about fraud is RAC. He has issued two NFTs on superrare and is launching another on Niftygateway soon, it is reasonable to assume he will issue more NFTs on a number of NFT auction sites. However, because he does not control the issuing platform's ERC721 contracts, he is limited on what he can do to demonstrate that the NFT is legitimate and has his 'blessing' so to speak (and isn't someone pirating his work). What RAC does control is rac.eth, so he can selectively say, this NFT is ed1.rac.eth, this one on another platform is ed2.rac.eth, and he can become known for naming all of his official NFTs, no matter who he chooses to issue with. I think of it like a signature to prove legitimacy. Anyone can create an NFT with stolen RAC artwork, but they cannot get him to sign them by addressing them with his ENS name. Similarly, the NFT can be bought on superrare, and auctioned on opensea and it will still retain his ENS subdomain addressing it, no scanning through Twitter to see if he posted the exact contract address for the NFT.

If so, what feature would you wish app.ens.domains to implement to make the experience nicer?

If this standard gets adopted, maybe ens.domains can query a resolver's supportsInterface(0x4b23de55) and then checks if resolver.tokenID !== 0 and if both of these are true it could show tokenID in the list alongside addr, txt, contenthash and more. If it wanted to go further it could use ERC721 metadata to show the image of the NFT if we are getting really optimistic.

Also, are there any ways for NFT platforms (such as OpenSea, KnownOrigin, etc) to integrate with this EIP so that it becomes more discoverable?

I am planning to put together an outreach email for as many NFT platforms as I can get contact details for, I just started with the ENS core devs first :) I am hoping they will be open to the idea. One of the first things I will need to address however is matching this EIP with an accompanying one for ERC1155 because it has a lot of adoption in the NFT space too.

Integration is pretty much:

  • Get ENS namehash
  • ens.resolver(namehash)
  • resolver.addr(namehash)
  • addr.supportsInterface(ERC721)
  • resolver.supportsInterface(ERC2381)
  • resolver.tokenID(namehash)
  • Display NFT matching addr+tokenID

It would be sad if this proposal gets abused for fraud when it was intended to combat the fraud in NFT space to begin with.

I totally understand that, it's why I don't want to launch the front end until I have reverse resolution of the name and NFT owner's addresses working. I believe people will get to know famous NFT issuer's ENS handles, and that these people won't rug pull their users by hot swapping ENS names onto dodgy NFTs. I do believe a user is more likely to notice a fake ENS name than a fake contract address. e.g. 0x22cc8b3666e926bcbf58cb726143b2b044c80a0c and 0x22cc8beef6e926bcbeefb726143beef4c80a0c might be too similar for a user just checking the beginning and end matches, but ed1.rac.eth versus ed1.r_a_c.eth should set off alarm bells as it does with regular phishing attacks.

@makoto
Copy link

makoto commented Nov 24, 2020

Anyone can create an NFT with stolen RAC artwork, but they cannot get him to sign them by addressing them with his ENS name. Similarly, the NFT can be bought on superrare, and auctioned on opensea and it will still retain his ENS subdomain addressing it, no scanning through Twitter to see if he posted the exact contract address for the NFT.

Yeah, the interoperability is very powerful. Also we can add NFT gallery feature on app.ens.domains or anyone can create something better as all the data is accessible via TheGraph.

If this standard gets adopted, maybe ens.domains can query a resolver's supportsInterface(0x4b23de55) and then checks if resolver.tokenID !== 0 and if both of these are true it could show tokenID in the list alongside addr, txt, contenthash and more.

That can be done easily.

If it wanted to go further it could use ERC721 metadata to show the image of the NFT if we are getting really optimistic.

I like the idea a lot, especially the fact that you can render the art image.

This could be a good potential use case for ENS on Layer2, as our current idea is to migrate ENS per domain by adding Layer2 support at the resolver level. It will be very expensive (on gas) for NFT artist to spend gas on 4 transactions (create subdomain, set resolver, set eth address, then set tokenId record) for every single item he has issued (especially for NFT which allows multiple editions).

One thought about the interface.
@mcdee suggested to change setTokenID so that it does not rely on setAddress. In my case, I suggest we could extend setTokenID to have contract address at parent domain and if tokenID is null, you can treat it as wildcard.

function setTokenID(bytes32 node, address token, uint256 tokenID);

This could be useful, if for example POAP could set this at poap.xyz so that 34074.poap.xyz to represent https://app.poap.xyz/token/34074 . It does not look nice but it may do the work of prevent fraud.

@Arachnid also mentioned the possibility of adding wildcard at ENS level so this could be redundant but I don't know if we have EIP for that yet.

I don't think your current approach of relying on setAddress to store ERC721 contract address can express this intention.

I am in favour of NOT using setAddress to store ERC721 contract address. In addition to requiring users to spend extra transaction, it prevents users from using the ETH address for other purpose (eg: donation ETH address).

====

Lastly I was initially a bit skeptical of this idea as you can always use text record as a substitution. However, this will promote ERC721 as a first class citizen and could give ENS adoption among NFT artists, so I do support the idea!

@OisinKyne
Copy link
Author

Also we can add NFT gallery feature on app.ens.domains
That can be done easily.
I like the idea a lot, especially the fact that you can render the art image.

Thanks for the positive response to all this, I would love to get some of these features into app.ens.domains.

this will promote ERC721 as a first class citizen and could give ENS adoption among NFT artists, so I do support the idea!

My biggest concern about getting feedback on this EIP was that people would say 'just put it in a txt record and be done with it', but your framing of first class NFT support is a great way of looking at it.

This could be a good potential use case for ENS on Layer2

I definitely agree, I have been trying to keep up with the developments in that space.

I am in favour of NOT using setAddress to store ERC721 contract address. In addition to requiring users to spend extra transaction, it prevents users from using the ETH address for other purpose (eg: donation ETH address).

I understand the need for minimising transactions, but I think having a second address at the exact same nodehash could lead to unintended bugs for people implementing. (E.g. addr is the default, what if they don't know to pull a different address from the tokenID function and end up displaying Donation Address + Token ID by mistake). However I still think with that exact overload you could shorten the flow and I would be happy to add it to the spec, I think I just would implement it slightly differently. Here's how I imagine it:

# Slowest flow
ens.setSubnodeOwner(node, label, owner)
ens.setResolver(node, resolverAddress)
resolver.setAddr(node, nftAddress)
resolver.setTokenId(node, tokenID)
# Shortened flow
ens.setSubnodeRecord(node, label, owner, resolverAddress) # Handles the set resolver step for us
resolver.setTokenId(node, nftAddress, tokenID) # Sets both addr and tokenID

So this would shorten the flow, but I would still save nftAddress into the AddrResolver rather than in the tokenIDResolver contract storage to keep things simple and backwards compatible. But;

I suggest we could extend setTokenID to have contract address at parent domain and if tokenID is null, you can treat it as wildcard

I would love to skip setting addresses and rely on some sort of wildcard fallback, where calling subnode.addr returns parentnode.addr if none is specified at the subnode level, is there anywhere I can look to see the wildcard implementations in progress? I think that would allow us to skip the setAddr tx entirely, and would make the tokenID overload a nice to have but unlikely to be needed often (only when the parent node is not the NFT contract address).

So, to summarise:

  • I like the proposed override of setTokenId and would be happy to add it in the manner I outlined to reduce the number of txs required.
  • I don't think having two potential contract addresses at a single nodehash is ideal from an integration perspective
  • I strongly support a wildcard/inheritance/fallback approach where if no address is specified at the subdomain level, it recurses upwards until addr gets a response, but I assume this is outside of the scope of this EIP and is instead best suited to a wildcard EIP.

How does that sound to you @makoto?

@Arachnid
Copy link
Contributor

I know it's a little late in the day for this, but an alternative idea: What if we define a syntax for NFT names of the form id@contract? So if the name coolnft.eth points to the NFT contract address, [email protected] is NFT number 1, and so forth. This wouldn't actually require any resolver changes, just client support, and it would allow automatically naming all NFTs with only a single transaction.

We could also define alternate encodings for the NFT ID part - perhaps a version with a higher radix to reduce the length of NFT identifiers.

@OisinKyne
Copy link
Author

@Arachnid it would make for a simple implementation on the ENS side, but I think it would sacrifice a lot of fine-grained control over what tokens you address and what you do not.

For example, if I'm an artist and use oisin.eth as my ENS name pointing to my EOA, and I mint an NFT on opensea, I don't want to address every NFT on opensea as a subdomain of my ENS name, I only want to address specifically my one. For example, if I tell people "Hey everyone, my NFT on opensea is [email protected]", all of a sudden every NFT on opensea's contract is addressable as <id>@opensea.oisin.eth, which I definitely do not want.

I'm trying to not bind ENS tightly with every NFT in a contract address, because the ENS owner often won't have much if any control over the contract address because they will typically belong to the NFT issuance platform rather than the artist.

@github-actions
Copy link

There has been no activity on this pull request for two months. It will be closed in a week if no further activity occurs. If you would like to move this EIP forward, please respond to any outstanding feedback or add a comment indicating that you have addressed all required feedback and are ready for a review.

@github-actions github-actions bot added the stale label Jan 24, 2021
@Arachnid
Copy link
Contributor

Arachnid commented Jan 25, 2021 via email

@github-actions github-actions bot removed the stale label Jan 25, 2021
@OisinKyne
Copy link
Author

Thanks for the bump to keep this MR alive @Arachnid.

For an update with regards to where this EIP is at; I had forgotten that solidity can now return multiple types from a function call rather than a struct of sorts. So moving the NFT's address from the addr field to an EIP2381 specific resolver field is feasible and does not need duplicate sets and calls like I had anticipated and as a result I am okay with making that change. However this EIP has fallen off of my priority list by quite a way and I do not have the motivation to start this in the foreseeable future.

If that changes I will push an update to this MR to reflect the proposal.

@lightclient
Copy link
Member

@OisinKyne I am going to close this as you don't plan to pursue in the foreseeable future. Please reopen if that changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants