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 initial draft for Name-bound tokens #5107

Closed
wants to merge 5 commits into from

Conversation

TimDaub
Copy link
Contributor

@TimDaub TimDaub commented May 24, 2022

Comment on lines +47 to +50
/// @dev This emits when ownership between ENS nodes of any NBT changes by any mechanism.
/// This event emits when NBTs are created (`from` == 0x0) and destroyed
/// (`to` == 0x0).
event Transfer(bytes32 indexed _from, bytes32 indexed _to, uint256 indexed _id);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not have a mint and burn event instead? Seems like that would be more clear than Transfer(0, ...) and Transfer(..., 0, ...)

Copy link
Member

Choose a reason for hiding this comment

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

That is the "industry standard" with all tokens 😬


We have adopted the [`EIP-165`](./eip-165.md) and `ERC721Metadata` functions purposefully to create a high degree of backward compatibility with [`EIP-721`](./eip-721.md). We have deliberately used [`EIP-721`](./eip-721.md) terminology such as `function ownerOf(...)` and `event Transfer` to minimize the effort of familiarization for `EIP-XXXX` implementers already familiar with, e.g., [`EIP-20`](./eip-20.md) or [`EIP-721`](./eip-721.md).

## Copyright
Copy link
Contributor

Choose a reason for hiding this comment

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

Security Considerations section is required.

EIPS/eip-xxxx.md Outdated Show resolved Hide resolved

## Backwards Compatibility

We have adopted the [`EIP-165`](./eip-165.md) and `ERC721Metadata` functions purposefully to create a high degree of backward compatibility with [`EIP-721`](./eip-721.md). We have deliberately used [`EIP-721`](./eip-721.md) terminology such as `function ownerOf(...)` and `event Transfer` to minimize the effort of familiarization for `EIP-XXXX` implementers already familiar with, e.g., [`EIP-20`](./eip-20.md) or [`EIP-721`](./eip-721.md).
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be in the rationale, rather than the backwards compatibility section.

Co-authored-by: Micah Zoltu <micah@zoltu.net>
@qingfeng
Copy link

Is this an attempt at SBT tokens?

/// @title Name-bound tokens
/// @dev See https://eips.ethereum.org/EIPS/eip-XXXX
/// Note: the ERC-165 identifier for this interface is 0x6352211e.
interface IERCXXXX /* is ERC165, ERC721Metadata */ {
Copy link
Member

Choose a reason for hiding this comment

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

This interface does not seem to define anything really. Is this still work in progress?

All of this can be accomplished by 721 or 1155. What you may want to specify is a getter function signaling whether a token is "soulbound" or not, but enforcement of that via transfer is still up to the particular token.

Copy link
Contributor

Choose a reason for hiding this comment

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

I disagree on this I think. The specification for a soulbound token defines ownerOf and it defines that the result of ownerOf is immutable across temporally disconnected calls. This means that if you call ownerOf and get back XXX, you can be confident that if you call ownerOf again at any point in the future you would get the same result.

Copy link
Contributor Author

@TimDaub TimDaub May 30, 2022

Choose a reason for hiding this comment

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

@axic would it help if along with the interface definition, we checked in a reference implementation to this PR?

Copy link
Member

Choose a reason for hiding this comment

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

This is an interface. The only thing what can be prescribed is what the semantics should be, but it cannot be enforced via an interface description. Reference implementations are out of scope.

Copy link
Member

@lightclient lightclient left a comment

Choose a reason for hiding this comment

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

I think you should consider working together with @MicahZoltu since you both are pursuing a "soulbound" token standard. (see #5114)

EIPS/eip-xxxx.md Outdated Show resolved Hide resolved
EIPS/eip-xxxx.md Outdated Show resolved Hide resolved
@TimDaub
Copy link
Contributor Author

TimDaub commented May 31, 2022

I think you should consider working together with @MicahZoltu since you both are pursuing a "soulbound" token standard. (see #5114)

If you look on Ethereum Magicians and the PRS here on GitHub you'll see that @MicahZoltu and I are indeed closely interating albeit having varying opinions. I think their Soulbound Badges are cool and @MicahZoltu was eager to contribute a lot to 4973.

But this proposal is specifically about binding tokens to an ENS hashnode and so it is quite different from what's suggested elsewhere.

@Pandapip1
Copy link
Member

@TimDaub Please push an empty commit so that the new CI can run.

@eth-bot
Copy link
Collaborator

eth-bot commented Jul 16, 2022

Hi! I'm a bot, and I wanted to automerge your PR, but couldn't because of the following issue(s):


(fail) EIPS/eip-xxxx.md

classification
ambiguous
  • 'EIPS/eip-xxxx.md' must be in eip-###.md format; this error will be overwritten upon relevant editor approval

@@ -0,0 +1,86 @@
---
eip: 5107
Copy link
Member

Choose a reason for hiding this comment

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

Please change the file name.

Co-authored-by: lightclient <14004106+lightclient@users.noreply.github.com>
@TimDaub
Copy link
Contributor Author

TimDaub commented Jul 26, 2022

Is this an attempt at SBT tokens?

yes

@TimDaub
Copy link
Contributor Author

TimDaub commented Jul 26, 2022

I'm closing this PR, but since it's licensed as CC0, anyone can repropose it and merge it.

@TimDaub TimDaub closed this Jul 26, 2022
@github-actions
Copy link

The commit 57d60d3 (as a parent of e799739) contains errors. Please inspect the Run Summary for details.

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.

None yet

7 participants