Skip to content

Commit

Permalink
Merge #4209: NFT Module
Browse files Browse the repository at this point in the history
* in sync with @okwme/cosmos-nft

* remove tmp tx

* structuring and minor changes

* supply and client files

* adding cli client

* complete cli/tx and rest.go

* cleanup and restructuring

* restructure rest folder

* minor updates on clients

* update querier

* encoding for clients and other changes

* genesis, invariants, and keeper updates

* update types

* make golangcibot happy

* renamed and removed bank keeper

* remove handlers for editmetadata, mint, burn, buy

* nft interface

* minor cleanup

* sort collections and nfts

* balance and find

* nft query and tx

* touch ups

* uint in place of int

Signed-off-by: Karoly Albert Szabo <[email protected]>

* little fixes:
- fix error to err to avoid collision
- error handling

Signed-off-by: Karoly Albert Szabo <[email protected]>

* module generalization changes

* fixes

* query with data

* minor updates and TODOs

* fix CLI tx

* golang bot fixes

* handlers and txs done

* update module generalization

* Added very basic tests which for some reason do not work

* fix test

Signed-off-by: Karoly Albert Szabo <[email protected]>

* fixed test, now we should fix implementation, seems to fail

Signed-off-by: Karoly Albert Szabo <[email protected]>

* fix test, create new struct instead of changing the old one

Signed-off-by: Karoly Albert Szabo <[email protected]>

* fix handler with new logic

Signed-off-by: Karoly Albert Szabo <[email protected]>

* let's make it compile

Signed-off-by: Karoly Albert Szabo <[email protected]>

* single failing test example, need to be fixed and extended

Signed-off-by: Karoly Albert Szabo <[email protected]>

* single failing test example, need to be fixed and extended

Signed-off-by: Karoly Albert Szabo <[email protected]>

* reverting work, still problems unmarshalling inside iterator from test

Signed-off-by: Karoly Albert Szabo <[email protected]>

* Setter in nft.go should return NFT instead of BaseNFT

Signed-off-by: Karoly Albert Szabo <[email protected]>

* remove TODOS

Signed-off-by: Karoly Albert Szabo <[email protected]>

* comment out broken tests, we want at least a green mark here

Signed-off-by: Karoly Albert Szabo <[email protected]>

* little fixes

Signed-off-by: Karoly Albert Szabo <[email protected]>

* hopefully no conflict

* minor changes for tests

* change nft id to string, refactors

* messy pause

* Changes Balances to OWners add all necessary functions, updated Keeper with UpdateNFT as as well as MintNFT and made sure they all update Owners

* pause dev to merge sdk master

* go.mod changes

* getting closer still need module.go

* builds!!!

* fix lint begin handler tests

* stableish

* re-order nft attributes, add back mint and burn msgs and handlers

* add errors to minting the same NFT and burning an NFT that doesnt exist

* first querier test

* add simulations for nft msgs

* handler tests check tags now (fixed a bug!)

* update simulation

* generic handler

* need to check if it compiles on another machine

* fix weird interface error

* add back cli

* wtfff

* codec error fixed, logs removed. still returning empty arrays of IDs

* Take empty input as yes answer

Closes: #4564

* Add pending log entry

* merged in master

* marshall errors

* build commands

* working!!!

* linting errors

* remove unused func

* pause

* fix burn error

* fix burn error

* tests for querier

* typo

* tests for NFT types

* module spec standard

* tests for Collection and Collections types

* merge w Fede

* tests for Owner Type

* added genesis tests and beefed up keeper, querier, handler & types tests

* linting errors deadcode

* DONT COVER test_common.go

* add msg type tests

* Update x/nft/internal/keeper/key.go

Co-Authored-By: Federico Kunze <[email protected]>

* Update x/nft/genesis.go

Co-Authored-By: Federico Kunze <[email protected]>

* Update x/nft/client/cli/query.go

Co-Authored-By: Federico Kunze <[email protected]>

* Apply suggestions from code review

* typo

* cleanup events

* split events

* more cleanup

* remove restrictions from default handlers

* not sure where these go mod changes came from

* sim generated changes

* make format

* add mint and burn sims

* move NFT interface to nft/exported

* make format

* NFT spec

* Updates

* more updates

* update specs readme

* fix sims

* rest additions

* rest additions

* fix invariant

* minimal nft without name, description or image

* sim

* fix sim

* fix sim

* fix Update methods

* nothing

* simplify update and remove

Signed-off-by: Karoly Albert Szabo <[email protected]>

* remove test on memory location

Signed-off-by: Karoly Albert Szabo <[email protected]>

* TEST to get logs, need to be removed

Signed-off-by: Karoly Albert Szabo <[email protected]>

* fix simulator editMetadata Msg type

* owner not found start with empty collection

Signed-off-by: Karoly Albert Szabo <[email protected]>

* artifacts on errors in case of failure, else, no artifacts

Signed-off-by: Karoly Albert Szabo <[email protected]>

* add more invariant checks to handler_tests

* never forget to overwrite

* merge and update spec

* colins feedback

* code coverage test

* code coverage test

* code coverage test

* spelling

* clean up client

* testing code coverage

* testing code coverage

* testing code coverage

* testing code coverage

* testing code coverage

* Update docs/spec/nft/README.md

Co-Authored-By: frog power 4000 <[email protected]>

* Apply suggestions from code review

Co-Authored-By: frog power 4000 <[email protected]>

* minor changes

* integration tests and fixes

* minor golangCI fixes

* Update simapp/app.go

Co-Authored-By: Bot from GolangCI <[email protected]>
  • Loading branch information
2 people authored and rigelrozanski committed Aug 26, 2019
1 parent ee99b8e commit eeb847c
Show file tree
Hide file tree
Showing 62 changed files with 5,617 additions and 24 deletions.
29 changes: 29 additions & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -87,27 +87,56 @@ jobs:
- make:
target: test_sim_import_export
description: "Test application import/export simulation"
- run:
command: |
mkdir -p /tmp/errors
cp /tmp/**/app-simulation-seed* /tmp/errors/
when: on_fail
- store_artifacts:
path: /tmp/errors

test_sim_after_import:
executor: golang
steps:
- make:
target: test_sim_after_import
description: "Test simulation after import"
- run:
command: |
mkdir -p /tmp/errors
cp /tmp/**/app-simulation-seed* /tmp/errors/
when: on_fail
- store_artifacts:
path: /tmp/errors

test_sim_multi_seed_long:
executor: golang
steps:
- make:
target: test_sim_multi_seed_long
description: "Test multi-seed simulation (long)"
- run:
command: |
mkdir -p /tmp/errors
cp /tmp/**/app-simulation-seed* /tmp/errors/
when: on_fail
- store_artifacts:
path: /tmp/errors

test_sim_multi_seed_short:
executor: golang
steps:
- make:
target: test_sim_multi_seed_short
description: "Test multi-seed simulation (short)"
- run:
command: |
mkdir -p /tmp/errors
cp /tmp/**/app-simulation-seed* /tmp/errors/
when: on_fail

- store_artifacts:
path: /tmp/errors

test_cover:
executor: golang
Expand Down
1 change: 1 addition & 0 deletions .codecov.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,4 @@ ignore:
- "docs"
- "*.md"
- "*.rst"
- "x/**/test_common.go"
3 changes: 2 additions & 1 deletion crypto/keys/output_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@ package keys
import (
"testing"

sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/stretchr/testify/require"
"github.com/tendermint/tendermint/crypto"
"github.com/tendermint/tendermint/crypto/multisig"
"github.com/tendermint/tendermint/crypto/secp256k1"

sdk "github.com/cosmos/cosmos-sdk/types"
)

func TestBech32KeysOutput(t *testing.T) {
Expand Down
5 changes: 1 addition & 4 deletions docs/spec/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,7 @@ block.
- [Mint](./mint) - Staking token provision creation.
- [Params](./params) - Globally available parameter store.
- [Supply](./supply) - Total supply of the chain.

## Interchain standards

- [ICS30](./_ics/ics-030-signed-messages.md) - Signed messages standard.
- [NFT](./nft) - Non-fungible tokens.

For details on the underlying blockchain and p2p protocols, see
the [Tendermint specification](https://github.com/tendermint/tendermint/tree/master/docs/spec).
36 changes: 18 additions & 18 deletions docs/spec/SPEC-SPEC.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Specification of Specifications

This file intends to outline the common structure for specifications within
this directory.
this directory.

## Tense

Expand All @@ -15,43 +15,43 @@ be considered preferable. In certain instances, due to the complex nature of
the functionality being described pseudo-code may the most suitable form of
specification. In these cases use of pseudo-code is permissible, but should be
presented in a concise manner, ideally restricted to only the complex
element as a part of a larger description.
element as a part of a larger description.

## Common Layout

The following generalized file structure should be used to breakdown
specifications for modules. With the exception of README.md, `XX` at the
beginning of the file name should be replaced with a number to indicate
document flow (ex. read `01_state.md` before `02_state_transitions.md`). The
following list is nonbinding and all files are optional.

- `README.md` - overview of the module
- `XX_concepts.md` - describe specialized concepts and definitions used throughout the spec
- `XX_state.md` - specify and describe structures expected to marshalled into the store, and their keys
- `XX_state_transitions.md` - standard state transition operations triggered by hooks, messages, etc.
- `XX_messages.md` - specify message structure(s) and expected state machine behaviour(s)
- `XX_begin_block.md` - specify any begin-block operations
- `XX_end_block.md` - specify any end-block operations
- `XX_hooks.md` - describe available hooks to be called by/from this module
- `XX_tags.md` - list and describe event tags used
- `XX_params.md` - list all module parameters, their types (in JSON) and examples
- `XX_future_improvements.md` - describe future improvements of this module
- `XX_appendix.md` - supplementary details referenced elsewhere within the spec
following list is nonbinding and all files are optional.

- `README.md` - overview of the module
- `XX_concepts.md` - describe specialized concepts and definitions used throughout the spec
- `XX_state.md` - specify and describe structures expected to marshalled into the store, and their keys
- `XX_state_transitions.md` - standard state transition operations triggered by hooks, messages, etc.
- `XX_messages.md` - specify message structure(s) and expected state machine behaviour(s)
- `XX_begin_block.md` - specify any begin-block operations
- `XX_end_block.md` - specify any end-block operations
- `XX_hooks.md` - describe available hooks to be called by/from this module
- `XX_events.md` - list and describe event tags used
- `XX_params.md` - list all module parameters, their types (in JSON) and examples
- `XX_future_improvements.md` - describe future improvements of this module
- `XX_appendix.md` - supplementary details referenced elsewhere within the spec

### Notation for key-value mapping

Within `state.md` the following notation `->` should be used to describe key to
value mapping:

```
key -> value
key -> value
```

to represent byte concatenation the `|` may be used. In addition, encoding
type may be specified, for example:

```
0x00 | addressBytes | address2Bytes -> amino(value_object)
0x00 | addressBytes | address2Bytes -> amino(value_object)
```

Additionally, index mappings may be specified by mapping to the `nil` value, for example:
Expand Down
50 changes: 50 additions & 0 deletions docs/spec/nft/01_concepts.md
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"`
}
20 changes: 20 additions & 0 deletions docs/spec/nft/02_state.md
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`.
`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)`
86 changes: 86 additions & 0 deletions docs/spec/nft/03_messages.md
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
}
```
48 changes: 48 additions & 0 deletions docs/spec/nft/04_events.md
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} |
5 changes: 5 additions & 0 deletions docs/spec/nft/05_future_improvements.md
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).
7 changes: 7 additions & 0 deletions docs/spec/nft/06_appendix.md
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)
Loading

0 comments on commit eeb847c

Please sign in to comment.