-
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
Add EIP: ERC-721 Holding Time Tracking #6806
Add EIP: ERC-721 Holding Time Tracking #6806
Conversation
✅ All reviewers have approved. |
|
||
An implementation of this EIP will be provided upon acceptance of the proposal. | ||
|
||
## Security Considerations |
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 content of this section seems boilerplate generated by ChatGPT since it appear to be unrelated to the actual technicality of providing holding time
EIPS/eip-6806.md
Outdated
|
||
interface IERC721HoldingTime { | ||
function getHoldingInfo(uint256 tokenId) external view returns (address holder, uint256 holdingTime); | ||
function setWhitelistedContract(address whitelistContract, bool ignoreReset) external; |
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.
Do you want to enable an event to be emitted when contract is being allowlisted?
Also, can you change the setWhitelistedContract
to more specific to ERC721HoldingTime since this method name is not very obvious related to holding time
Does it have to be a "contract" or it could be a general address of EOA/Contract?
The commit 55c0fa5 (as a parent of 11b7e3e) contains errors. |
|
EIPS/eip-6806.md
Outdated
|
||
The `setHoldingTimeWhitelistedAddress` function introduces flexibility in determining which transfers should affect holding time calculations. By allowing the contract owner or authorized accounts to whitelist specific addresses, transfers involving these addresses will not reset the holding time. This is particularly important for NFT-based financial transactions or special cases where holding time should not be affected by transfers. | ||
|
||
Together, these functions enhance the utility of the ERC-721 standard, enabling developers to create more sophisticated applications and experiences based on NFT holding time and offering the ability to selectively ignore holding time resets during transfers to or from whitelisted addresses. |
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.
This looks more like motivation. The rationale section should describe technical decisions made within the EIP itself, not justify the EIP as a whole.
The addition of the `getHoldingInfo` function to an extension of the ERC-721 standard enables developers to implement NFT-based applications that require holding time information. This extension maintains compatibility with existing ERC-721 implementations while offering additional functionality for new use cases. | ||
|
||
The `getHoldingInfo` function provides a straightforward method for retrieving the holding time and holder address of an NFT. By using seconds as the unit of time for holding duration, it ensures precision and compatibility with other time-based functions in smart contracts. | ||
|
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.
How about something like this, to explain the return values of getHoldingInfo
?
`getHoldingInfo` returns both `holder` and `holdingTime` so that some token owners (as decided by the implementation) can be ignored for the purposes of calculating holding time. For example, a contract may take ownership of an NFT as collateral for a loan. Such a loan contract could be ignored, so the real owner's holding time increases properly. |
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.
Ok, it looks good
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.
All Reviewers Have Approved; Performing Automatic Merge...
Is the argument |
An extension of ERC-721 that adds an interface to track and describe the holding time of a Non-Fungible Token (NFT) by an account.