NFT Module #4209
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,3 +24,4 @@ ignore: | |
| - "docs" | ||
| - "*.md" | ||
| - "*.rst" | ||
| - "x/**/test_common.go" | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| # Concepts | ||
|
|
||
| ## NFT | ||
|
|
||
| The `NFT` Interface inherits the BaseNFT struct and includes getter functions for the asset data. It also includes a Stringer function in order to print the struct. The interface may change if metadata is moved to it’s own module as it might no longer be necessary for the flexibility of an interface. | ||
|
|
||
| ```go | ||
| // NFT non fungible token interface | ||
| type NFT interface { | ||
| GetID() string // unique identifier of the NFT | ||
| GetOwner() sdk.AccAddress // gets owner account of the NFT | ||
| SetOwner(address sdk.AccAddress) // gets owner account of the NFT | ||
| GetTokenURI() string // metadata field: URI to retrieve the of chain metadata of the NFT | ||
| EditMetadata(tokenURI string) // edit metadata of the NFT | ||
| String() string // string representation of the NFT object | ||
| } | ||
| ``` | ||
|
|
||
| ## Collections | ||
|
|
||
| A Collection is used to organized sets of NFTs. It contains the denomination of the NFT instead of storing it within each NFT. This saves storage space by removing redundancy. | ||
|
|
||
| ```go | ||
| // Collection of non fungible tokens | ||
| type Collection struct { | ||
| Denom string `json:"denom,omitempty"` // name of the collection; not exported to clients | ||
| NFTs []*NFT `json:"nfts"` // NFTs that belongs to a collection | ||
| } | ||
| ``` | ||
|
|
||
| ## Owner | ||
|
|
||
| An Owner is a struct that includes information about all NFTs owned by a single account. It would be possible to retrieve this information by looping through all Collections but that process could become computationally prohibitive so a more efficient retrieval system is to store redundant information limited to the token ID by owner. | ||
|
|
||
| ```go | ||
| // Owner of non fungible tokens | ||
| type Owner struct { | ||
| Address sdk.AccAddress `json:"address"` | ||
| IDCollections IDCollections `json:"IDCollections"` | ||
| } | ||
| ``` | ||
|
|
||
| An `IDCollection` is similar to a `Collection` except instead of containing NFTs it only contains an array of `NFT` IDs. This saves storage by avoiding redundancy. | ||
|
|
||
| ```go | ||
| // IDCollection of non fungible tokens | ||
| type IDCollection struct { | ||
| Denom string `json:"denom"` | ||
| IDs []string `json:"IDs"` | ||
| } | ||
|
There was a problem hiding this comment. Yo. Okay so I think that the NFT design should be restructured to eliminate the:
We should instead by applying the use of secondary indexes similarly to how the staking module stores references to the same validators by: operator-address, consensus-address, power, etc. (see example https://github.com/cosmos/cosmos-sdk/blob/master/x/staking/keeper/validator.go#L70) This way we should be able to easily create functions on the NFTKeeper to get the NFTs efficiently without requiring these extra object. For example I'd suggest the keeper look like: type NFTKeeper interface {
GetNFTByID(string) NFT
GetCollectionByDenom(string) []*NFT
GetCollectionsByOwner(sdk.AccAddress) []Collection
}Or something along those lines. There was a problem hiding this comment. Opened the issue #4955 |
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| # State | ||
|
|
||
| ## Collections | ||
|
|
||
| As all NFTs belong to a specific `Collection`, they are kept on store in an array | ||
| within each `Collection`. Every time an NFT that belongs to a collection is updated, | ||
| it needs to be updated on the corresponding NFT array on the corresponding `Collection`. | ||
|
There was a problem hiding this comment. whoa - are you saying NFTs are stored in two places? that sounds like asking for trouble. We should most certainly only store pointer objects in the collection to avoid having to update both. (See my suggested code change to the spec). There was a problem hiding this comment. Also referenced in https://github.com/cosmos/cosmos-sdk/issues/4955 |
||
| `denomHash` is used as part of the key to limit the length of the `denomBytes` which is | ||
| a hash of `denomBytes` made from the tendermint [tmhash library](https://github.com/tendermint/tendermint/tree/master/crypto/tmhash). | ||
|
|
||
| - Collections: `0x00 | denomHash -> amino(Collection)` | ||
| - denomHash: `tmhash(denomBytes)` | ||
|
|
||
| ## Owners | ||
|
|
||
| The ownership of an NFT is set initially when an NFT is minted and needs to be | ||
| updated every time there's a transfer or when an NFT is burned. | ||
|
|
||
| - Owners: `0x01 | addressBytes | denomHash -> amino(Owner)` | ||
| - denomHash: `tmhash(denomBytes)` | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,86 @@ | ||
| # Messages | ||
|
|
||
| ## MsgTransferNFT | ||
|
|
||
| This is the most commonly expected MsgType to be supported across chains. While each application specific blockchain will have very different adoption of the `MsgMintNFT`, `MsgBurnNFT` and `MsgEditNFTMetadata` it should be expected that most chains support the ability to transfer ownership of the NFT asset. The exception to this would be non-transferable NFTs that might be attached to reputation or some asset which should not be transferable. It still makes sense for this to be represented as an NFT because there are common queriers which will remain relevant to the NFT type even if non-transferable. This Message will fail if the NFT does not exist. By default it will not fail if the transfer is executed by someone beside the owner. **It is highly recommended that a custom handler is made to restrict use of this Message type to prevent unintended use.** | ||
|
|
||
| | **Field** | **Type** | **Description** | | ||
| |:----------|:-----------------|:--------------------------------------------------------------------------------------------------------------| | ||
| | Sender | `sdk.AccAddress` | The account address of the user sending the NFT. By default it is __not__ required that the sender is also the owner of the NFT. | | ||
| | Recipient | `sdk.AccAddress` | The account address who will receive the NFT as a result of the transfer transaction. | | ||
| | Denom | `string` | The denomination of the NFT, necessary as multiple denominations are able to be represented on each chain. | | ||
| | ID | `string` | The unique ID of the NFT being transferred | | ||
|
|
||
| ```go | ||
| // MsgTransferNFT defines a TransferNFT message | ||
| type MsgTransferNFT struct { | ||
| Sender sdk.AccAddress | ||
| Recipient sdk.AccAddress | ||
| Denom string | ||
| ID string | ||
| } | ||
| ``` | ||
|
|
||
| ## MsgEditNFTMetadata | ||
|
|
||
| This message type allows the `TokenURI` to be updated. By default anyone can execute this Message type. **It is highly recommended that a custom handler is made to restrict use of this Message type to prevent unintended use.** | ||
|
|
||
| | **Field** | **Type** | **Description** | | ||
| |:------------|:-----------------|:-----------------------------------------------------------------------------------------------------------| | ||
| | Sender | `sdk.AccAddress` | The creator of the message | | ||
| | ID | `string` | The unique ID of the NFT being edited | | ||
| | Denom | `string` | The denomination of the NFT, necessary as multiple denominations are able to be represented on each chain. | | ||
| | TokenURI | `string` | The URI pointing to a JSON object that contains subsequent metadata information off-chain | | ||
|
|
||
| ```go | ||
| // MsgEditNFTMetadata edits an NFT's metadata | ||
| type MsgEditNFTMetadata struct { | ||
| Sender sdk.AccAddress | ||
| ID string | ||
| Denom string | ||
| TokenURI string | ||
| } | ||
| ``` | ||
|
|
||
| ## MsgMintNFT | ||
|
|
||
| This message type is used for minting new tokens. If a new `NFT` is minted under a new `Denom`, a new `Collection` will also be created, otherwise the `NFT` is added to the existing `Collection`. If a new `NFT` is minted by a new account, a new `Owner` is created, otherwise the `NFT` `ID` is added to the existing `Owner`'s `IDCollection`. By default anyone can execute this Message type. **It is highly recommended that a custom handler is made to restrict use of this Message type to prevent unintended use.** | ||
|
|
||
| | **Field** | **Type** | **Description** | | ||
| |:------------|:-----------------|:-----------------------------------------------------------------------------------------| | ||
| | Sender | `sdk.AccAddress` | The sender of the Message | | ||
| | Recipient | `sdk.AccAddress` | The recipiet of the new NFT | | ||
| | ID | `string` | The unique ID of the NFT being minted | | ||
| | Denom | `string` | The denomination of the NFT. | | ||
| | TokenURI | `string` | The URI pointing to a JSON object that contains subsequent metadata information off-chain | | ||
|
|
||
| ```go | ||
| // MsgMintNFT defines a MintNFT message | ||
| type MsgMintNFT struct { | ||
| Sender sdk.AccAddress | ||
| Recipient sdk.AccAddress | ||
| ID string | ||
| Denom string | ||
| TokenURI string | ||
| } | ||
| ``` | ||
|
|
||
| ### MsgBurnNFT | ||
|
|
||
| This message type is used for burning tokens which destroys and deletes them. By default anyone can execute this Message type. **It is highly recommended that a custom handler is made to restrict use of this Message type to prevent unintended use.** | ||
|
|
||
|
|
||
| | **Field** | **Type** | **Description** | | ||
| |:----------|:-----------------|:---------------------------------------------------| | ||
| | Sender | `sdk.AccAddress` | The account address of the user burning the token. | | ||
| | ID | `string` | The ID of the Token. | | ||
| | Denom | `string` | The Denom of the Token. | | ||
|
|
||
| ```go | ||
| // MsgBurnNFT defines a BurnNFT message | ||
| type MsgBurnNFT struct { | ||
| Sender sdk.AccAddress | ||
| ID string | ||
| Denom string | ||
| } | ||
| ``` |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| # Events | ||
|
|
||
| The nft module emits the following events: | ||
|
|
||
| ## Handlers | ||
|
|
||
| ### MsgTransferNFT | ||
|
|
||
| | Type | Attribute Key | Attribute Value | | ||
| |--------------|---------------|--------------------| | ||
| | transfer_nft | denom | {nftDenom} | | ||
| | transfer_nft | nft-id | {nftID} | | ||
| | transfer_nft | recipient | {recipientAddress} | | ||
| | message | module | nft | | ||
| | message | action | transfer_nft | | ||
| | message | sender | {senderAddress} | | ||
|
|
||
| ### MsgEditNFTMetadata | ||
|
|
||
| | Type | Attribute Key | Attribute Value | | ||
| |-------------------|---------------|-------------------| | ||
| | edit_nft_metadata | denom | {nftDenom} | | ||
| | edit_nft_metadata | nft-id | {nftID} | | ||
| | message | module | nft | | ||
| | message | action | edit_nft_metadata | | ||
| | message | sender | {senderAddress} | | ||
| | message | token-uri | {tokenURI} | | ||
|
|
||
| ### MsgMintNFT | ||
|
|
||
| | Type | Attribute Key | Attribute Value | | ||
| |----------|---------------|-----------------| | ||
| | mint_nft | denom | {nftDenom} | | ||
| | mint_nft | nft-id | {nftID} | | ||
| | message | module | nft | | ||
| | message | action | mint_nft | | ||
| | message | sender | {senderAddress} | | ||
| | message | token-uri | {tokenURI} | | ||
|
|
||
| ### MsgBurnNFTs | ||
|
|
||
| | Type | Attribute Key | Attribute Value | | ||
| |----------|---------------|-----------------| | ||
| | burn_nft | denom | {nftDenom} | | ||
| | burn_nft | nft-id | {nftID} | | ||
| | message | module | nft | | ||
| | message | action | burn_nft | | ||
| | message | sender | {senderAddress} | |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| # Future Improvements | ||
|
|
||
| There's interesting work that could be done about moving metadata into its own module. This could act as one of the `tokenURI` endpoints if a chain chooses to offer storage as a solution. Furthermore on-chain metadata can be trusted to a higher degree and might be used in secondary actions like price evaluation. Moving metadata to it's own module could be useful for the Bank Module as well. It would be able to describe attributes like decimal places and information regarding vesting schedules. It would be needed to have a level of introspection to describe the content without actually delivering the content for client libraries to interact with it. Using schema.org as a common location to settle metadata schema structure would be a good and impartial place to do so. | ||
|
|
||
| Inter-Blockchain Communication will need to develop its own Message types that allow NFTs to be transferred across chains. Making sure that spec is able to support the NFTs created by this module should be easy. What might be more complicated is a transfer that includes optional metadata so that a receiving chain has the option of parsing and storing it instead of making IBC queries when that data needs to be accessed (assuming that information stays up to date). |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| # Appendix | ||
|
|
||
| * Cosmos SDK: [PR #4209](https://github.com/cosmos/cosmos-sdk/pull/4209) | ||
| * Cosmos SDK: [Issue #4046](https://github.com/cosmos/cosmos-sdk/issues/4046) | ||
| * Interchain Standards: [ICS #17](https://github.com/cosmos/ics/issues/30) | ||
| * Binance: [BEP #7](https://github.com/binance-chain/BEPs/pull/7) | ||
| * Ethereum: [EIP #721](https://github.com/ethereum/EIPs/blob/master/EIPS/eip-721.md) |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
EDIT:
we should most likely eliminate these structs if they are expected to be stored anywhere and simply replace them with indexes on the NFT object see my next comment
couple things, a) in the above owner struct the field is of typeIDCollectionshowever you only define theIDCollection(singular) struct below, we should probably also include the line:just for clarity.b) type Collection should probably expose a functionGetIDCollections() IDCollectionsand similarly NFT should expose a functionGetIDCollection() IDCollection. These functions are worth mentioning maybe? add this into the NFT type interface at maybe?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.
Opened the issue https://github.com/cosmos/cosmos-sdk/issues/4955