-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
Creates EIP-5114: Soulbound Token #5114
Conversation
All tests passed; auto-merging...(pass) eip-5114.md
|
// the tokenUri **MUST NOT** point at mutable/censorable content (e.g., https://) | ||
// data from this takes precedent over data returned by `collectionUri` | ||
// any external links referenced by the content at `tokenUri` also **MUST** follow all of the above rules | ||
function tokenUri(uint256 tokenId) external view returns (string tokenUri); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't using tokenURI
here be better than tokenUri
as then the bytes4 signature of the function address in Solidity is equal to e.g. that of EIP-721?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I considered it, but I don't think 721 compatibility matters much because the ownerOf
signature is different and owner lookups require custom code, so this token won't natively work with existing ERC-721 tooling.
Perhaps there are specific tools that don't need to do owner lookups but do need to URI lookups that would benefit from consistency across methods?
Personally I would like to see a compelling reason to break the superior (IMO) naming convention of capitalizing only the first letter of acronyms (it avoids situations like XMLHTTPRPC
with XmlHttpRpc
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO naming convention arguments should be out of scope of this EIP. But a separate EIP could make normative statements on function capitalization conventions.
I was mainly arguing for consistency with EIP-721 but it's not a hard objection from my side.
Could an EIP-5114 Soulbound token be bound to another EIP-5114 token? Are hierarchies permitted? What about loops? |
What is your take on EIP-165 and do you plan to use it here? |
Co-authored-by: Tim Daubenschütz <[email protected]>
I am a big fan of allowing hierarchies. In fact, I wonder if this EIP should be modified such that it allows for binding to not only ERC-721 tokens but also to ERC-5114 tokens so you can have hierarchies? In theory loops are possible I suppose, but it would given the strict rules about immutability, the whole loop would need to be created at once. At first glance, this doesn't seem to be a problem worth resolving, but it may be good to mention it in the security considerations section that tooling should look out for it and protect itself from getting caught in an infinite loop/recursion. |
I think it is a good idea. Would love a PR that adds support for it as I'm not super knowledgeable about how to implement it. If no one submits a PR I'll probably take some time at some point after this is merged as a Draft to figure out how to add it. |
I have added a note about loops to security considerations. I also changed it to just require binding to an NFT, with EIP-721 being merely an example. The only real requirement is that the thing being bound to has an |
to send a PR on EIP-165, would I send it to this origin/branch: MicahZoltu-patch-1? |
The simplest option is to just click on a line or drag-select a handful of lines and leave a comment with a suggestion block like:
That will put the suggested change inline in the PR that can then be one-click committed. Another option is to do as you suggested and submit a PR against my branch. A final option is to wait until this is merged as a Draft, and then submit a PR to modify the EIP once it actually exists. |
EIPS/eip-5114.md
Outdated
// the tokenUri **MUST** be immutable and content addressable (e.g., ipfs://) | ||
// the tokenUri **MUST NOT** point at mutable/censorable content (e.g., https://) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as much as I appreciate the sentiment, it seems out-of-scope to me to specify the nature of the storage, as it is outside the Ethereum network
the requirement for immutability is also questionable to me - I could see a use for a URI that can deliberately be changed or change itself over time
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we don't constrain the storage, then the tokens functionally become mutable. By asserting immutability throughout, we enable applications/users to build caching systems that they cannot built if we only encourage the behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Immutable data doesn't exist. From what I understand, only content-integrity assuring identifiers exist that can indicate mutation. Replication can be used to fallback on non-mutated backups. IMO, e.g. asking to "content-address" resources in tokenURI
would fit better.
"Censor"able is for sure the wrong term as it can mean all sorts of non technical stuff.
Anyways, I agree with @wschwab. For interoperability e.g. between wallets, whether data is mutable or not is out of scope.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not following your claim here that immutable data doesn't exist. If you ask IPFS for something by content hash, you will get either that exact thing, or you will get nothing (if it can't be found on the network). There is no situation where you would get a different result from someone else or at a different point in time.
Regarding censorable content: I tend to agree that censorable content is fuzzy here, and I wish we could nail down something a bit more concrete. What I want to avoid is people linking to content that a provider can remove at a future point in time. The goal with these badges is that they are permanent, they aren't something that comes and goes over time. If you link out to a traditional https
resource, the host of that resource/owner of the domain can remove it. Similarly, the domain may be seized and changed or taken away. Something like IPFS on the other hand allows anyone to ensure the content survives, you don't have to rely on a single operator.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"immutable data"/content-addressable criteria:
I agree with you that some networks of computers can create the illusion that some data is immutable. But on the basis of a principle, the data isn't immutable. E.g. on IPFS/Ethereum/Bitcoin when you run a full node, there's nothing stopping you from going into the chain data directory and switching a few bits. You can always mutate the data. It's likely though that the system maintaining the data will notice and e.g. notably fail or repair itself. That's what I meant by "Replication can be used to fallback on non-mutated backups (in a distributed system)."
How I'd describe it is that by requesting data using content-addressing, a user can guarantee to themselves that they'll either receive the exact data corresponding to the content-address, or nothing at all (e.g. when content-address and data don't align).
I think "content-addressing" is a better term than e.g. "immutable". I guess another usable term could be "tamper-proof" or "tamper-resistant."
data availability guarantees/non-censorable:
I'd strongly opt for e.g. "highly-available data" rather than "non-censorable." From my perspective, censorship is about withholding information in the interest of a few. Data availability, on the other hand, is a term from the blockchain research world that describes the concept of retaining and serving data within e.g. a p2p network. E.g. all EVM storage has high data availability, and anything stored in the Bitcoin blockchain too.
For IPFS, actually, it doesn't have data availability built-in. To pin an IPFS content hash, one has to personally deploy a dedicated node that continues to make the content hash available within the distributed hash table. IMO, if you wanted the ultimate data availability for Soulbound Badges, it'd probably be best to store the contents of tokenUri
on-chain as e.g. a struct. I'm not kidding. It's because if you outsource delivering extremely high-quality data availability to other networks not aligned with Ethereum (e.g. IPFS), it may be that one day they just go down/stop working while Ethereum still works.
Now talking about storing the badge's metadata on-chain, I think it's obligatory I bring up #735
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets move this conversation over to the discussions-to link as I'm hoping this will get merged as a draft soon and I don't want this conversation to get lost.
type: Standards Track | ||
category: ERC | ||
created: 2022-05-30 | ||
requires: 721 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this requirement necessary? I believe you can bind EIP5114 to any address?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The EIP references EIP-721, so it requires it (not all editors agree on this policy).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, in abstract it says that EIP5114 requires it to be bound to another NFT, and gives example of EIP721. In section "Backwards Compatibility" Its stated that EIP721 HAS to be used as a soul. I think the EIP should be more specific if EIP721 as a soul is mandatory, or is some other token or EIP also acceptable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any EIP that matches the interface ownerOf(uint256 id)
I think is the actual requirement. EIP-721 is an example of that, as is EIP-5114.
EIPS/eip-5114.md
Outdated
This is a new tokene type and is not meant to be backward compatible with any existing tokens other than using ERC-721 as souls. | ||
|
||
## Security Considerations | ||
Users of tokens that claim to implement this EIP must be diligent in verifying they actually do. A token author can create a token that, upon initial probing of the API surface, may appear to follow the rulse when in reality it doesn't. For example, the contract could allow transfers via some mechanism and simply not utilize them initially. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use one space after each period.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
Co-authored-by: KillariDev <[email protected]>
Co-authored-by: KillariDev <[email protected]>
Co-authored-by: lightclient <[email protected]>
Co-authored-by: William Schwab <[email protected]>
Per request by @lightclient
Co-authored-by: KillariDev <[email protected]>
|
||
## Abstract | ||
A soulbound token is a token that is bound to another Non-Fungible Token (NFT; e.g., a EIP-721 token) when it is minted, and cannot be transferred/moved after that. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure why this EIP needs to exist. Are there many use cases for nontransferable tokens? Why should implementers prefer this EIP over, say, EIP-4973?
Seems like this EIP needs a Motivation section ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see what you did there. I may consider adding a motivation after this has merged as a draft, but as it isn't required I would like to move forward without it for now.
Co-authored-by: Sam Wilson <[email protected]>
Some suggestions and opinions:
btw. We are also exploring some scenarios about soulbound token, which can be used as a reference, but our scenario is a further Paired bounding scenario, which creates a new identity for the address and stores the relationship between the paired address at the same time. It is an extend of the standard erc721, and its characteristics are mainly:
Behind the token, we store the relationship between two tokens, and the bounding relationship can be queried through the contract interface. we write a contract at: https://github.com/marryinweb3/ERC721-520 we can talk about soulbound tokens more at here |
@Yu-Tou These are good questions/comments, but they should really be made over at this EIP's discussions-to link (https://ethereum-magicians.org/t/eip-5114-soulbound-badges/9417/17). Would you mind posting the same thing over there so it doesn't get lost when this PR is merged? |
ok ,thanks |
* Creates EIP-XXXX: Soulbound Token * Updates EIP number * Update eip-5114.md * Update EIPS/eip-5114.md Co-authored-by: Tim Daubenschütz <[email protected]> * Update eip-5114.md * Update EIPS/eip-5114.md Co-authored-by: KillariDev <[email protected]> * Update EIPS/eip-5114.md Co-authored-by: KillariDev <[email protected]> * Update EIPS/eip-5114.md Co-authored-by: lightclient <[email protected]> * Update EIPS/eip-5114.md Co-authored-by: William Schwab <[email protected]> * fixes double space after sentences Per request by @lightclient * Apply suggestions from code review Co-authored-by: KillariDev <[email protected]> * Apply suggestions from code review Co-authored-by: Sam Wilson <[email protected]> Co-authored-by: Tim Daubenschütz <[email protected]> Co-authored-by: KillariDev <[email protected]> Co-authored-by: lightclient <[email protected]> Co-authored-by: William Schwab <[email protected]> Co-authored-by: Sam Wilson <[email protected]>
No description provided.