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 ERC: Named NFT #320

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Add ERC: Named NFT #320

wants to merge 12 commits into from

Conversation

xinbenlv
Copy link
Collaborator

When opening a pull request to submit a new EIP, please use the suggested template: https://github.com/ethereum/EIPs/blob/master/eip-template.md

We have a GitHub bot that automatically merges some PRs. It will merge yours immediately if certain criteria are met:

  • The PR edits only existing draft PRs.
  • The build passes.
  • Your GitHub username or email address is listed in the 'author' header of all affected PRs, inside .
  • If matching on email address, the email address is the one publicly listed on your GitHub profile.

@eip-review-bot
Copy link
Collaborator

eip-review-bot commented Mar 13, 2024

File ERCS/erc-7653.md

Requires 1 more reviewers from @axic, @g11tech, @SamWilsn

@eip-review-bot eip-review-bot changed the title Create draft-erc-named-erc721 Add ERC: Named NFT Mar 13, 2024
ERCS/erc-7653.md Outdated Show resolved Hide resolved
xinbenlv and others added 2 commits March 15, 2024 08:02
Co-authored-by: Andrew B Coathup <28278242+abcoathup@users.noreply.github.com>
---
eip: 7653
title: Named NFT
description: An extension to ERC-721 based on ERC-7632.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Your description doesn't really say anything about what's in this proposal. It only says where to look for more information.

It'd be nice to see at a glance what's actually in this document.


## Abstract

An extension to `ERC-721` based on `ERC-7632` to enable named `ERC-721`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
An extension to `ERC-721` based on `ERC-7632` to enable named `ERC-721`
An extension to [ERC-721](./eip-721.md) based on [ERC-7632](./eip-7632.md) to enable named ERC-721.

Don't use backticks to bypass the linter.


## Abstract

An extension to `ERC-721` based on `ERC-7632` to enable named `ERC-721`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Your abstract is certainly terse, but it doesn't contain enough information about what the proposal actually does. What is a named NFT? How do the names work?


The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", "SHOULD", "SHOULD NOT", "RECOMMENDED", "NOT RECOMMENDED", "MAY", and "OPTIONAL" in this document are to be interpreted as described in RFC 2119 and RFC 8174.

1. A compliant contract MUST implement `ERC-721` and following interface
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
1. A compliant contract MUST implement `ERC-721` and following interface
1. A compliant contract MUST implement `ERC721` and following interface

}
```

2. A compliant contract MUST implement the `ERC-165`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
2. A compliant contract MUST implement the `ERC-165`
2. A compliant contract MUST implement the `ERC165` interface


## Rationale

Needs discussion
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Needs discussion
Needs discussion <!-- TODO -->


## Backwards Compatibility

This ERC is designed to be backward compatible with `ERC-721`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
This ERC is designed to be backward compatible with `ERC-721`
This ERC is designed to be backward compatible with ERC-721.


## Security Considerations

Needs discussion.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Needs discussion.
Needs discussion. <!-- TODO -->

@ethereum ethereum deleted a comment from lucentlabz Jun 3, 2024
Copy link

There has been no activity on this pull request for 2 weeks. It will be closed after 3 months of inactivity. If you would like to move this PR 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.

Copy link

@lucentlabz lucentlabz left a comment

Choose a reason for hiding this comment

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

Close this for fraud

@github-actions github-actions bot removed the w-stale label Aug 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants