Skip to content

Add ERC: ERC-721 Guarantee Extension #349

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

Merged
merged 26 commits into from
Apr 7, 2025

Conversation

iunknow588
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.

@eip-review-bot
Copy link
Collaborator

eip-review-bot commented Mar 28, 2024

✅ All reviewers have approved.

Copy link

The commit 178c74f (as a parent of 69e31bf) contains errors.
Please inspect the Run Summary for details.

@github-actions github-actions bot added the w-ci label Mar 28, 2024
@github-actions github-actions bot removed the w-ci label Mar 28, 2024
ERCS/erc-7652.md Outdated
---
eip: 7652
title: ERC-721 Guarantee Extension
description: ERC-721 Guarantor role interface extension
Copy link
Contributor

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 add any information not in your title. Try to expand on the ideas you've introduced in the title. For example, you could explain what an ERC-721 guarantee actually is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your suggestion, the modification is as follows.
description: An extension interface that can guarantee NFT holders the ability to exchange their NFTs for circulating tokens at any time


## Abstract

This specification defines functions outlining a guarantor role for instance of [EIP-721](./eip-721.md). The guarantee interface implements the user-set valuation and guarantee share for a given NFT (token ID), as well as the guarantee rights enjoyed and obligations assumed during subsequent transactions. An implementation enables the user to read or set the current guarantee value for a given NFT (token ID), and also realizes the distribution of guarantee interest and the performance of guarantee obligations. It sends the standardized events when the status changes. This proposal relies on and extends the existing [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 is a decent abstract, but could use a bit of text explaining what a "guarantor role" is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your proposal, the new abstract has added a description of the responsibilities and rights of the guarantor:
Any participant can act as a guarantor to provide a subjective value assessment for an NFT. The guarantor's assessed value represents both the price the guarantor is willing to buy the NFT for and the weight value for sharing the guarantor's commission.


## Motivation

NFT (token ID) commonly face the issue of insufficient market liquidity: the main reason being the lack of transparency in NFT pricing, making it difficult for users to cash out after trading and purchasing NFT (token ID).
Copy link
Contributor

Choose a reason for hiding this comment

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

How is "token ID" related to NFT here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your attention. There is just a slight difference in detail, which is a specific NFT within the set of NFTs.


With the introduction of the guarantor role, different guarantor groups can offer various price guarantees for NFT (token ID), establishing a multi-faceted price evaluation system for NFT (token ID).

After purchasing an NFT (token ID), users can return it to the guarantor at any time at the highest guaranteed price to protect their interests.
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think you need to follow NFT with "token ID" every time you use the initialism.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your suggestion, the new version has removed these redundant expressions.


The keywords “MUST”, “MUST NOT”, “REQUIRED”, “SHALL”, “SHALL NOT”, “SHOULD”, “SHOULD NOT”, “RECOMMENDED”, “MAY”, and “OPTIONAL” in this document are to be interpreted as described in RFC 2119.

Every contract compliant to the `ERC721Guarantee` MUST implement the `IERC721Guarantee` guarantee interface.
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
Every contract compliant to the `ERC721Guarantee` MUST implement the `IERC721Guarantee` guarantee interface.
Every contract compliant with this proposal MUST implement the `IERC721Guarantee` guarantee interface.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your suggestion, the new version has made the relevant revisions to the wording.


Every contract compliant to the `ERC721Guarantee` MUST implement the `IERC721Guarantee` guarantee interface.

The **guarantee extension** is OPTIONAL for EIP-721 contracts.
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
The **guarantee extension** is OPTIONAL for EIP-721 contracts.

You can just omit this because it's always implied. You can't place requirements on implementations of other proposals.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your suggestion, the new version has made the relevant revisions to the wording.

Comment on lines +136 to +142
Key factors influencing the standard:

- Pay attention to ensuring fairness between and within groups when allocating commissions
- Keeping the number of guarantee groups (DAOs)in the interfaces to prevent contract bloat
- The guarantee group is a DAO contract, which MUST implement the `ERC721TokenReceiver` interface
- Simplicity
- Gas Efficiency
Copy link
Contributor

Choose a reason for hiding this comment

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

You should go in-depth on specific choices you made, not give general thoughts on your design.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your suggestion,
These descriptions are issues that should be focuses on in the coding implementation.
The reference implementation will also pay attention to these issues.

Comment on lines +150 to +153
## Reference Implementation

The reference implementation will be provided later.

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
## Reference Implementation
The reference implementation will be provided later.

Just remove this section until you have the reference implementation ready.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your suggestion, the new version has made the relevant revisions to the wording.


## Security Considerations

Needs discussion.
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
Needs discussion.
Needs discussion. <!-- TODO -->

If you use HTML-style comments, the linter will make sure you come back to them before going into the next status.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your suggestion, the new version has corrected this mistake. Very appreciate it.

iunknow588 and others added 10 commits July 28, 2024 22:17
检查错误原因……
错误定位处理
错误故障定位处理
故障定位
故障定位……
故障定位
故障定位
故障定位
故障定位
@SamWilsn
Copy link
Contributor

Did you publish the new version somewhere? I can't see it in the "Files Changed" tab, so I can't review it 😅

Copy link

There has been no activity on this issue for six months. It will be closed in 7 days if there is no new activity. 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.

@eip-review-bot eip-review-bot enabled auto-merge (squash) April 7, 2025 00:07
Copy link
Collaborator

@eip-review-bot eip-review-bot left a comment

Choose a reason for hiding this comment

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

All Reviewers Have Approved; Performing Automatic Merge...

@eip-review-bot eip-review-bot merged commit a3f1588 into ethereum:master Apr 7, 2025
10 checks passed
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.

4 participants