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 EIP-5334: EIP-721 User And Expires And Level Extension #5334

Merged
merged 15 commits into from
Aug 28, 2022

Conversation

yan253319066
Copy link
Contributor

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.

@eth-bot eth-bot enabled auto-merge (squash) July 25, 2022 06:54
@eth-bot
Copy link
Collaborator

eth-bot commented Jul 25, 2022

A critical exception has occurred:
Message: pr 5334 is already merged; quitting
(cc @alita-moore, @mryalamanchi)

@github-actions
Copy link

The commit de24604 (as a parent of 87c68c5) contains errors. Please inspect the Run Summary for details.

auto-merge was automatically disabled July 25, 2022 07:08

Head branch was pushed to by a user without write access

@yan253319066
Copy link
Contributor Author

@github-actions
Copy link

The commit 25f5ebb (as a parent of 3447ad6) contains errors. Please inspect the Run Summary for details.

yanning added 2 commits July 25, 2022 15:15
@yan253319066
Copy link
Contributor Author

Updated 5327 to 5334

@github-actions
Copy link

The commit 7f2238f (as a parent of 143fc8c) contains errors. Please inspect the Run Summary for details.

@github-actions
Copy link

The commit 71995f4 (as a parent of 2eb63d5) contains errors. Please inspect the Run Summary for details.

@yan253319066 yan253319066 changed the title EIP5327: ERC-721 User And Expires And Level Extension Add ERC-721 User And Expires And Level Extension Jul 25, 2022
@eth-bot eth-bot enabled auto-merge (squash) July 25, 2022 07:21
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.

Reference implementation should only contain the minimum code possible to exemplify an implementation of this standard. It doesn't have to be complete or even compile out of the box and it shouldn't include an entire build system, tooling, nor any extraneous features. I recommend deleting everything in assets/eip-5334 except the ERC5334.sol file.


For test cases, this should be something that is portable to any environment a future reader may be using and not tightly coupled with a particular language/test tooling. Usually this means a JSON file or something that has function, inputs, expected_outputs, expected_exceptions or something. You may want additional columns for expected post-conditions as well.

The readme also shouldn't be included, nor should the .gitignore.

EIPS/eip-5334.md Outdated Show resolved Hide resolved
EIPS/eip-5334.md Outdated Show resolved Hide resolved
auto-merge was automatically disabled July 26, 2022 02:00

Head branch was pushed to by a user without write access

@yan253319066
Copy link
Contributor Author

Thank you for your counsel.

@github-actions
Copy link

The commit 8001859 (as a parent of 2cbaf15) contains errors. Please inspect the Run Summary for details.

@github-actions
Copy link

The commit d9c25a7 (as a parent of 2466c55) contains errors. Please inspect the Run Summary for details.

@github-actions
Copy link

The commit 6ae9c42 (as a parent of d90e80f) contains errors. Please inspect the Run Summary for details.

yanning added 2 commits July 26, 2022 10:17
@github-actions
Copy link

The commit 4d5c541 (as a parent of c0f16a8) contains errors. Please inspect the Run Summary for details.

@yan253319066
Copy link
Contributor Author

@lightclient, @axic, @SamWilsn, @Pandapip1
Thanks

EIPS/eip-5334.md Outdated Show resolved Hide resolved
EIPS/eip-5334.md Outdated Show resolved Hide resolved
EIPS/eip-5334.md Outdated Show resolved Hide resolved
yanning added 2 commits July 27, 2022 10:13
@github-actions
Copy link

The commit 8c5e27a (as a parent of 13d8655) contains errors. Please inspect the Run Summary for details.

@github-actions
Copy link

The commit 9405585 (as a parent of f4f0350) contains errors. Please inspect the Run Summary for details.

@yan253319066
Copy link
Contributor Author

EIPS/eip-5334.md Outdated Show resolved Hide resolved
EIPS/eip-5334.md Outdated Show resolved Hide resolved
EIPS/eip-5334.md Outdated Show resolved Hide resolved
EIPS/eip-5334.md Outdated Show resolved Hide resolved
EIPS/eip-5334.md Outdated Show resolved Hide resolved
@eth-bot eth-bot dismissed a stale review via 280874e August 10, 2022 17:12
Copy link
Contributor

@0xanders 0xanders left a comment

Choose a reason for hiding this comment

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

suggest add setUserLevel() and userLevel() as an extension of EIP-4907

/// @notice Get the user level of an NFT
/// @dev The zero value indicates that there is no user
/// @param tokenId The NFT to get the user level for
/// @return The user level for this NFT
function userLevel(uint256 tokenId) external view returns(uint8);

/// @notice Set the user level of an NFT
/// @param level  User level of the NFT
function SetUserLevel(uint256 tokenId, uint8 level) external view returns(uint256);

EIPS/eip-5334.md Outdated Show resolved Hide resolved
@0xanders
Copy link
Contributor

@lightclient, @SamWilsn, @Pandapip1, please compare this EIP with EIP-4907, there is a lot of the same content.

So I suggest change this EIP as an extension of EIP-4907

@Pandapip1
Copy link
Member

Please make the required changes. They can be found in the files tab

Co-authored-by: Micah Zoltu <micah@zoltu.net>
Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>
@Pandapip1 Pandapip1 requested a review from eth-bot as a code owner August 28, 2022 17:53
@github-actions github-actions bot added c-new Creates a brand new proposal s-draft This EIP is a Draft t-erc labels Aug 28, 2022
@Pandapip1 Pandapip1 changed the title Add ERC-721 User And Expires And Level Extension Add EIP-5334: EIP-721 User And Expires And Level Extension Aug 28, 2022
Copy link
Member

@Pandapip1 Pandapip1 left a comment

Choose a reason for hiding this comment

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

@yan253319066 - if you don't like any of these changes, please feel free to revert them by submitting a PR.

@eth-bot eth-bot enabled auto-merge (squash) August 28, 2022 17:58
@eth-bot eth-bot merged commit 17e25e4 into ethereum:master Aug 28, 2022
nachomazzara pushed a commit to nachomazzara/EIPs that referenced this pull request Jan 13, 2023
…5334)

* EIP5327: ERC-721 User And Expires And Level Extension

* yan

* yan

* yan

* yan

* Delete unnecessary files

* yan

* yan

* yan

* yan

* yan

* yan

* yan

* Apply a few fixes

Co-authored-by: Micah Zoltu <micah@zoltu.net>
Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>

* Move Rationale to Motivation

Co-authored-by: yanning <ynsz@163.com>
Co-authored-by: Pandapip1 <45835846+Pandapip1@users.noreply.github.com>
Co-authored-by: Micah Zoltu <micah@zoltu.net>
Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c-new Creates a brand new proposal s-draft This EIP is a Draft t-erc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants