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

docs: ADR-043 Base NFT Module #9329

Merged
merged 44 commits into from
Jul 12, 2021
Merged
Changes from 7 commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
8701e97
Add ADR-043 BaseNFT Module
hackmd-deploy May 8, 2021
3876262
Merge branch 'master' into master
shahankhatch May 10, 2021
6c923d3
Adding specifications to NFT fields and clarity on usage.
shahankhatch May 10, 2021
bdb6005
Merge branch 'shahan/adr-043-nft'
chengwenxi May 14, 2021
af68935
resolve #9284 comments
chengwenxi May 17, 2021
be60788
Merge branch 'master' into hulatown/adr-043-nft
chengwenxi May 17, 2021
1759155
Merge branch 'master' into hulatown/adr-043-nft
chengwenxi May 17, 2021
96f9e11
fix typo
chengwenxi May 18, 2021
09c17a6
fix description
chengwenxi May 18, 2021
13cc5a2
rename NFT to BaseNFT
chengwenxi May 18, 2021
435c219
more description
chengwenxi May 25, 2021
1571a59
fix typo
chengwenxi May 25, 2021
1f3ec2a
Adding clarity to ADR 043 nft data field (#9388)
shahankhatch May 26, 2021
b5a36b3
Update docs/architecture/adr-043-nft-module.md
fedekunze May 27, 2021
835b581
Merge branch 'master' into hulatown/adr-043-nft
chengwenxi May 28, 2021
e39f059
apply comments
chengwenxi May 31, 2021
e8e6980
Merge branch 'master' into hulatown/adr-043-nft
fedekunze May 31, 2021
db8bd84
Merge branch 'master' into hulatown/adr-043-nft
mergify[bot] Jun 3, 2021
cc289b6
Merge branch 'master' into hulatown/adr-043-nft
mergify[bot] Jun 3, 2021
3f7c8bb
Merge branch 'master' into hulatown/adr-043-nft
mergify[bot] Jun 3, 2021
1098e27
Merge branch 'master' into hulatown/adr-043-nft
mergify[bot] Jun 3, 2021
55ebf53
Merge branch 'master' into hulatown/adr-043-nft
mergify[bot] Jun 4, 2021
9b4f4cf
Merge branch 'master' into hulatown/adr-043-nft
mergify[bot] Jun 4, 2021
0849698
Merge branch 'master' into hulatown/adr-043-nft
mergify[bot] Jun 4, 2021
ccb2807
Merge branch 'master' into hulatown/adr-043-nft
mergify[bot] Jun 7, 2021
278507b
Merge branch 'master' into hulatown/adr-043-nft
mergify[bot] Jun 7, 2021
8004004
Merge branch 'master' into hulatown/adr-043-nft
mergify[bot] Jun 7, 2021
51df217
apply comments
chengwenxi Jun 10, 2021
5e2f4c3
Merge branch 'master' into hulatown/adr-043-nft
Jun 10, 2021
f1e81fa
Merge branch 'master' into hulatown/adr-043-nft
fedekunze Jun 14, 2021
899e2a2
store nft in bank module
chengwenxi Jun 15, 2021
b641f4c
refactor nft to base nft
chengwenxi Jun 18, 2021
9d60dfd
fix note
chengwenxi Jun 18, 2021
0dbfbdb
rename denom as type -- first batch
Jun 18, 2021
c8642f4
rename denom as type -- second batch
Jun 18, 2021
5152044
minor fix
chengwenxi Jun 18, 2021
0092c53
incorporate feedbacks from 6/18 call
Jun 21, 2021
843f79c
incorporate Billy's feedback
Jul 1, 2021
c98ce3a
fix code formatting
Jul 1, 2021
013e9ca
incorporate feedbacks from aaron, shaun, billy...
Jul 2, 2021
8891fa2
Merge branch 'master' into hulatown/adr-043-nft
chengwenxi Jul 3, 2021
836580a
Merge branch 'master' into hulatown/adr-043-nft
aaronc Jul 6, 2021
afc6680
Merge branch 'master' into hulatown/adr-043-nft
okwme Jul 9, 2021
121dff1
Merge branch 'master' into hulatown/adr-043-nft
okwme Jul 12, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
150 changes: 150 additions & 0 deletions docs/architecture/adr-043-nft-module.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
# ADR 43: NFT Module

## Changelog

- 05.05.2021: Initial Draft

## Status

DRAFT

## Abstract

This ADR defines a generic NFT module of `x/nft` which supports NFTs as a `proto.Any` and contains `NFT` as the default implementation.

## Context
``
fedekunze marked this conversation as resolved.
Show resolved Hide resolved
NFTs are more digital assets than only crypto arts, which is very helpful for accruing value to ATOM. As a result, Cosmos Hub should implement NFT functions and enable a unified mechanism for storing and sending the ownership representative of NFTs ss discussed in https://github.com/cosmos/cosmos-sdk/discussions/9065.
chengwenxi marked this conversation as resolved.
Show resolved Hide resolved

As was discussed in [#9065](https://github.com/cosmos/cosmos-sdk/discussions/9065), several potential solutions can be considered:
- irismod/nft and modules/incubator/nft
- CW721
- DID NFTs
- interNFT

Since NFTs functions/use cases are tightly connected with their logic, it is almost impossible to support all the NFTs' use cases in one Cosmos SDK module by defining and implementing different transaction types.

Considering generic usage and compatibility of interchain protocols including IBC and Gravity Bridge, it is preferred to have a very simple NFT module design which only stores NFTs by id and owner.

This design idea can enable composability that application-specific functions should be managed by other modules on Cosmos Hub or on other Zones by importing the NFT module.

For example, NFTs can be grouped into collections in a collectibles module to define the behaviors such as minting, burning, transferring, etc.

haifengxi marked this conversation as resolved.
Show resolved Hide resolved
The current design is based on the work done by [IRISnet team](https://github.com/irisnet/irismod/tree/master/modules/nft) and an older implementation in the [Cosmos repository](https://github.com/cosmos/modules/tree/master/incubator/nft).


## Decision

We will create a module `x/nft` which only stores NFTs by id and owner.

The interface for the `x/nft` module:

```go
// NFT is an interface used to store NFTs at a given id.
type NFT interface {
GetId() string // define a simple identifier for the NFT
GetOwner() sdk.AccAddress // the current owner's address
GetData() string // specialized metadata to accompany the nft
haifengxi marked this conversation as resolved.
Show resolved Hide resolved
Validate() error
}
haifengxi marked this conversation as resolved.
Show resolved Hide resolved
```

The NFT conforms to the following specifications:
* The Id is an immutable field used as a unique identifier. NFT identifiers don't currently have a naming convention but
can be used in association with existing Hub attributes, e.g., defining an NFT's identifier as an immutable Hub address allows its integration into existing Hub account management modules.
We envision that identifiers can accommodate mint and transfer actions.
* Owner: This mutable field represents the current account owner (on the Hub) of the NFT, i.e., the account that will have authorization
to perform various activities in the future. This can be a user, a module account, or potentially a future NFT module that adds capabilities.
* Data: This mutable field allows for attaching pertinent metadata to the NFT, e.g., to record special parameter change upon transferring or criteria being met.
The data field is used to maintain special information, such as name and URI. Currently, there is no convention for the data placed into this field,
however, best practices recommend defining clear specifications that apply to each specific NFT type.

We will also create `NFT` as the default implementation of the `NFT` interface:
```proto
message NFT {
chengwenxi marked this conversation as resolved.
Show resolved Hide resolved
option (gogoproto.equal) = true;

string id = 1;
string owner = 2;
string data = 3;
}
```

Queries functions for `NFT` is:
chengwenxi marked this conversation as resolved.
Show resolved Hide resolved
```proto
service Query {
fedekunze marked this conversation as resolved.
Show resolved Hide resolved

// NFT queries NFT details based on id.
rpc NFT(QueryNFTRequest) returns (QueryNFTResponse) {
option (google.api.http).get = "/cosmos/nft/v1beta1/nfts/{id}";
}

// NFTs queries all proposals based on the optional onwer.
rpc NFTs(QueryNFTsRequest) returns (QueryNFTsResponse) {
option (google.api.http).get = "/cosmos/nft/v1beta1/nfts";
}
}

// QueryNFTRequest is the request type for the Query/NFT RPC method
message QueryNFTRequest {
string id = 1;
}

// QueryNFTResponse is the response type for the Query/NFT RPC method
message QueryNFTResponse {
google.protobuf.Any nft = 1 [(cosmos_proto.accepts_interface) = "NFT", (gogoproto.customname) = "NFT"];
chengwenxi marked this conversation as resolved.
Show resolved Hide resolved
}

// QueryNFTsRequest is the request type for the Query/NFTs RPC method
message QueryNFTsRequest {
string owner = 1;
cosmos.base.query.v1beta1.PageResponse pagination = 2;
}

// QueryNFTsResponse is the response type for the Query/NFTs RPC method
message QueryNFTsResponse {
repeated google.protobuf.Any nfts = 1 [(cosmos_proto.accepts_interface) = "NFT", (gogoproto.customname) = "NFTs"];
cosmos.base.query.v1beta1.PageResponse pagination = 2;
}
```



## Consequences
haifengxi marked this conversation as resolved.
Show resolved Hide resolved

### Backward Compatibility

No backward incompatibilities.

### Forward Compatibility

This specification conforms to the ERC-721 smart contract specification for NFT identifiers. Note that ERC-721 defines uniqueness based on (contract address, uint256 tokenId), and we conform to this implicitly
because a single module is currently aimed to track NFT identifiers. Note: use of the (mutable) data field to determine uniqueness is not safe.

### Positive

- NFT identifiers available on Cosmos Hub
- Ability to build different NFT modules for the Cosmos Hub, e.g., ERC-721.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
- NFT identifiers available on Cosmos Hub
- Ability to build different NFT modules for the Cosmos Hub, e.g., ERC-721.
- NFT identifiers available for Cosmos Ecosystem
- Common NFT module Interface for the Cosmos Ecosystem, e.g., ERC-721.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is an SDK ADR, so I think we should more focus on the Cosmos ecosystem here.

- NFT module which supports interoperability with IBC and other cross-chain infrastructures like Gravity Bridge
haifengxi marked this conversation as resolved.
Show resolved Hide resolved

### Negative

- Currently, no methods are defined for this module except to store and retrieve data.
Copy link
Collaborator

Choose a reason for hiding this comment

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

In ### Msg Service section we define methods.


### Neutral

- Other functions need more modules. For example, a custody module is needed for NFT trading function, a collectible module is needed for defining NFT properties

## Further Discussions

For other kinds of applications on the Hub, more app-specific modules can be developed in the future:
haifengxi marked this conversation as resolved.
Show resolved Hide resolved
- `x/nft/collectibles`: grouping NFTs into collections and defining properties of NFTs such as minting, burning and transferring, etc.
- `x/nft/custody`: custody of NFTs to support trading functionality
- `x/nft/marketplace`: selling and buying NFTs using sdk.Coins
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are those extensions or new modules? How this will work? Will there be a new services in the nft module?


Other networks in the Cosmos ecosystem could design and implement their own NFT modules for specific NFT applications and use cases.

## References

- Initial discussion: https://github.com/cosmos/cosmos-sdk/discussions/9065
- x/nft: initialize module: https://github.com/cosmos/cosmos-sdk/pull/9174