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

Non-Fungible Token with usage rights #4512

Closed
wants to merge 21 commits into from
Closed

Conversation

KKimos
Copy link

@KKimos KKimos commented Dec 1, 2021

This standard adds a new right of Non-Fungible Token that the right to use. Through this standard, you can achieve :

Separation of the right to use and ownership of Non-Fungible Token
Non-secured lease Non-Fungible Token
You can continue to use it after you mortgage the Non-Fungible Token
Metaverse sharing economy
It is precisely because of the separation of ownership and use right that the utilization rate of assets can be greater. You must distinguish between the rights of the user and the owner.

Different from 2615, we only added a right to use, so that we can get all the functions of 2615, and reduce many unnecessary operations in 2615. The explanation is as follows:

The owner should not use some of the ownership rights when mortgage NFT, such as transfer, AXS-like breeding or weapon upgrade, because it is likely to change the status of the original NFT, so when the user mortgages the NFT, the ownership must be mortgaged in the contract middle.
In the case of mortgage or transaction, the owner has the right to continue to use it before returning the ransom or selling it
This is the result of my implementation, you can refer to https://github.com/KKimos/ERCX

@eth-bot
Copy link
Collaborator

eth-bot commented Dec 1, 2021

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


(fail) eip-4512.md

  • File with name EIPS/eip-4512.md is new and new files must be reviewed
  • This PR requires review from one of [@lightclient, @axic]

(pass) assets/eip-4512/LICENSE

  • file assets/eip-4512/LICENSE is associated with EIP 4512; because there are also changes being made to EIPS/eip-4512.md all changes to corresponding assets are also allowed

(pass) assets/eip-4512/README.md

  • file assets/eip-4512/README.md is associated with EIP 4512; because there are also changes being made to EIPS/eip-4512.md all changes to corresponding assets are also allowed

(pass) assets/eip-4512/contracts/ERCX.sol

  • file assets/eip-4512/contracts/ERCX.sol is associated with EIP 4512; because there are also changes being made to EIPS/eip-4512.md all changes to corresponding assets are also allowed

(pass) assets/eip-4512/contracts/extensions/ERCXEnumerable.sol

  • file assets/eip-4512/contracts/extensions/ERCXEnumerable.sol is associated with EIP 4512; because there are also changes being made to EIPS/eip-4512.md all changes to corresponding assets are also allowed

(pass) assets/eip-4512/contracts/interface/IERCX.sol

  • file assets/eip-4512/contracts/interface/IERCX.sol is associated with EIP 4512; because there are also changes being made to EIPS/eip-4512.md all changes to corresponding assets are also allowed

(pass) assets/eip-4512/contracts/interface/IERCXEnumerable.sol

  • file assets/eip-4512/contracts/interface/IERCXEnumerable.sol is associated with EIP 4512; because there are also changes being made to EIPS/eip-4512.md all changes to corresponding assets are also allowed

@alita-moore
Copy link
Contributor

A critical exception has occurred (cc @alita-moore):
Message: can not read a block mapping entry; a multiline key may not be an implicit key at line 3, column 7:
author: kkimos [email protected]
^

Will address later

@KKimos
Copy link
Author

KKimos commented Dec 1, 2021

I have been trying but I don't know what went wrong

title: "Non-Fungible Token with usage rights"
status: Draft
type: Standards Track
author: KKimos <[email protected]>
Copy link
Contributor

Choose a reason for hiding this comment

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

There must be at least one author with a GitHub account.

Suggested change
author: KKimos <[email protected]>
author: KKimos (@KKimos)
Suggested change
author: KKimos <[email protected]>
author: KKimos (@KKimos), KKimos <[email protected]>

Either of the above is acceptable.

Comment on lines 11 to 14
## Simple Summary

Separation of Non-Fungible Token ownership and usage rights , Truly in line with real life.

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
## Simple Summary
Separation of Non-Fungible Token ownership and usage rights , Truly in line with real life.

This section is deprecated in favor of a description header field.

Comment on lines 154 to 159
## Reference Implementation

[Github Reposotory](https://github.com/KKimos/ERCX).



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
[Github Reposotory](https://github.com/KKimos/ERCX).

External links should not be used. Either inline the reference implementation, add it to ../assets/eip-4512/ or just remove this section entirely (it is optional).

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for your reply. I have made changes but need to review the new ones. Do I need to submit them at once? Still waiting for review?

@MicahZoltu
Copy link
Contributor

It looks like CI is passing, so you just have to wait for a review from lightclient or axic. They usually get to it within a week, but sometimes it is faster or slower.

EIPS/eip-4512.md Outdated Show resolved Hide resolved
EIPS/eip-4512.md Outdated Show resolved Hide resolved
EIPS/eip-4512.md Outdated Show resolved Hide resolved
EIPS/eip-4512.md Outdated Show resolved Hide resolved
EIPS/eip-4512.md Outdated Show resolved Hide resolved
EIPS/eip-4512.md Outdated Show resolved Hide resolved
EIPS/eip-4512.md Outdated Show resolved Hide resolved
EIPS/eip-4512.md Outdated Show resolved Hide resolved
@wschwab
Copy link
Contributor

wschwab commented Dec 5, 2021

There is a similar ERC also in PR right now: EIP-4400. While there are certainly differences between the two standards, I was wondering if it might be worth your time to try uniting these into one standard as opposed to two very similar ones.

Copy link
Contributor

@wschwab wschwab left a comment

Choose a reason for hiding this comment

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

Hi! I mentioned in the PR thread that this EIP seems very similar imho to EIP-4400, and I would recommend collaborating. In the meantime, I've left some comments that I hope are helpful.

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

Different from EIP-2615, we only added a right to use, so that we can get all the functions of EIP-2615, and reduce many unnecessary operations in EIP-2615. The explanation is as follows:

- The owner should not use some of the ownership rights when mortgage NFT, such as transfer, AXS-like breeding or weapon upgrade, because it is likely to change the status of the original NFT, so when the user mortgages the NFT, the ownership must be mortgaged in the contract middle.
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you clarify what you mean by this? I suspect you're discussing a case where the owner (Alice) of an NFT is mortgaging the NFT via another party (Bob). Who is the owner now? Is Alice now theuser and Bob the owner? What is the "contract middle"?

I think is also laying down some guidelines for what a user can and cannot do, but I wasn't able to understand.

Different from EIP-2615, we only added a right to use, so that we can get all the functions of EIP-2615, and reduce many unnecessary operations in EIP-2615. The explanation is as follows:

- The owner should not use some of the ownership rights when mortgage NFT, such as transfer, AXS-like breeding or weapon upgrade, because it is likely to change the status of the original NFT, so when the user mortgages the NFT, the ownership must be mortgaged in the contract middle.
- In the case of mortgage or transaction, the owner has the right to continue to use it before returning the ransom or selling it
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 see the concept of a ransom elsewhere in the EIP. Maybe it would be beneficial if you outlined the flow of mortgaging and then paying off the mortgage of an NFT using this EIP?

Comment on lines +78 to +82
* @notice Safely transfers `tokenId` tokenUser from `from` to `to`
* @dev If the caller is not `from`, it must be approved to move this tokenUser by {approve} or {setApprovalForAll} or {approveUser}.
* @param from the address token user transfer from
* @param to the address token user transfer to
* @param tokenId the NFT token's Id
Copy link
Contributor

Choose a reason for hiding this comment

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

imho the natspec should include the bytes4 for being an ERC4512Receiver, see, for example ERC721Receiver in EIP-721

```

### ERC-4512 Receiver

Copy link
Contributor

Choose a reason for hiding this comment

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

as above, imho the interface for ERC4512Receiver should be included here for reference

Comment on lines +207 to +229
#### 2. ERC-721

Fully compatible with ERC-721

```solidity
interface ERC721{
event Transfer(address indexed _from, address indexed _to, uint256 indexed _tokenId);
event Approval(address indexed _owner, address indexed _approved, uint256 indexed _tokenId);
event ApprovalForAll(address indexed _owner, address indexed _operator, bool _approved);
function balanceOf(address _owner) external view returns (uint256);
function ownerOf(uint256 _tokenId) external view returns (address);
function safeTransferFrom(address _from, address _to, uint256 _tokenId, bytes data) external payable;
function safeTransferFrom(address _from, address _to, uint256 _tokenId) external payable;
function transferFrom(address _from, address _to, uint256 _tokenId) external payable;
function approve(address _approved, uint256 _tokenId) external payable;
function setApprovalForAll(address _operator, bool _approved) external;
function getApproved(uint256 _tokenId) external view returns (address);
function isApprovedForAll(address _owner, address _operator) external view returns (bool);
}
interface ERC165 {
function supportsInterface(bytes4 interfaceID) external view returns (bool);
}
```
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
#### 2. ERC-721
Fully compatible with ERC-721
```solidity
interface ERC721{
event Transfer(address indexed _from, address indexed _to, uint256 indexed _tokenId);
event Approval(address indexed _owner, address indexed _approved, uint256 indexed _tokenId);
event ApprovalForAll(address indexed _owner, address indexed _operator, bool _approved);
function balanceOf(address _owner) external view returns (uint256);
function ownerOf(uint256 _tokenId) external view returns (address);
function safeTransferFrom(address _from, address _to, uint256 _tokenId, bytes data) external payable;
function safeTransferFrom(address _from, address _to, uint256 _tokenId) external payable;
function transferFrom(address _from, address _to, uint256 _tokenId) external payable;
function approve(address _approved, uint256 _tokenId) external payable;
function setApprovalForAll(address _operator, bool _approved) external;
function getApproved(uint256 _tokenId) external view returns (address);
function isApprovedForAll(address _owner, address _operator) external view returns (bool);
}
interface ERC165 {
function supportsInterface(bytes4 interfaceID) external view returns (bool);
}
```

imho this is unnecessary

Comment on lines +235 to +246
#### 1 . Mortgage

When mortgage NFT, you only need to mortgage the right to use into the mortgage contract, leaving the right to use.The advantage of this is that you can still use your own NFT before delivery.

#### 2 . Rent

Lease does not require any collateral. First, you mortgage the NFT ownership into the contract, and the tenantry leases the right to use.

This has the following benefits :

- No need to mortgage assets
- Don't worry about the risk of default
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar to above, I'm personally not fully understanding the intended path to leverage this EIP for NFT mortgages or rentals. The Rationale section is more for design decisions, so I don't know if this would be the right place to put it, but I think that at least for myself, if there would be more in the Motivation explaining how this EIP is intended to be leveraged, it would help me understand here.

This would also be a good section for discussing why the authors opted to create a more simplified EIP-2612, and any other alternatives that were considered but ultimately discarded in favour of the current EIP.


## Backward compatibility

This protocol is fully backward compatible with ERC721.Refer to above Specification.
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
This protocol is fully backward compatible with ERC721.Refer to above Specification.
This protocol is fully backward compatible with ERC721. Refer to the Specification section above for more details.



## Test Cases

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 see any tests - are there tests somewhere to run?

---
eip: 4512
title: Non-Fungible Token with usage rights
description: Separation of Non-Fungible Token ownership and usage rights , Truly in line with real life.
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
description: Separation of Non-Fungible Token ownership and usage rights , Truly in line with real life.
description: ERC721 NFTs with a separate user role

@KKimos
Copy link
Author

KKimos commented Dec 5, 2021

fully

I tried to contact the author of EIP-4400 and request to merge with him

@lightclient
Copy link
Member

I tried to contact the author of EIP-4400 and request to merge with him

Any progress on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants