Hi,

Here is an attempt to add support for custom queries and custom executes to cw-nft (https://github.com/CosmWasm/cw-nfts/issues/31).

I was trying to extend cw-nft but It seems difficult to extend cw721-base without including it in your project.

This is just an attempt, and I am not a rust daily developper. So if my diff is silly or useless feel free to ignore it.

Implements the changes described here: https://github.com/CosmWasm/cw-nfts/issues/2

cw721_base -- removes name, description, and image, only top level field is token_uri and extension. cw721_metadata_onchain -- example of Extension that moves metadata on-chain, supports name, description, image, and more

Fixes #44

This is a starting point to discuss how to fix the issue that contracts wholesale inheriting the base instantiate message have the incorrect contract version info.

This could be a problem when migrating, for example.

This PR proposes to add an optional field that takes a struct of values from a child contract. If set to None it will just use the base contract version info.

Appreciate this might need some refining and/or further discussion.

A minimal example of the EIP-2981 spec implemented in CW-land. i.e. #7

It's a very light wrapper/extension for CW721 compliant contracts that adds two additional query messages:

• RoyaltyInfo to be called when selling an NFT, to find out what royalties are owed
• CheckRoyalties to be called on the contract, to see if it implements CW-2981

This adds royalty information at token mint time.

The main thing are the queries that are called when the marketplace sells an NFT. Different contracts might implement them differently, e.g. to do more complex logic than the straight percentage here, but the returned queries should conform to the shape here.

Aside - could move those (and the extension handlers) to a package, if it makes them more reusable.

Continuing a discussion from Discord: https://discord.com/channels/737637324434833438/892092030870749214/892537033208254465

What do we want for Token IDs?

Currently, they are strings. @shanev brought up the point that "now with the existence of Uint256Key, there might be a better case for using it just like ERC-721". He also added:

Because can run into iteration issues in the store if they can be arbitrary strings no?

Personally, I've come across this recently with the all_tokens query when there are different token ids like "1" and "uuid-43123d-eaGg3Xz".

@ethanfrey brought up some good questions:

Having them all the same length may make enumeration nicer (alphabetical), but do you really want them encoded as integers ("183836284794036363790202743525168039373")? or rather hex encoded bytes? or enforce this is ascii and right pad with spaces?

So hence this ticket.

Uint256Key seems to make sense for me, but curious what others think. CC @ben2x4, @alwin-peng, @the-frey.

I try to implement what's in #81. Remaining issues:

• In cw2981-royalites, QueryMsg<Cw2981QueryMsg> is used. As in cw721-base generates an empty response for Extension, there is no response generated for Cw2981QueryMsg. I haven't found a solution for this.
• In cw721-non-transferable, most of QueryMsg enums are redefined. I want a more robust solution than copy lines from cw721-base.

Adds support for an optional collection_uri for use in ics721, as well as extensible on chain collection level metadata.

Also added a migrate entrypoint, messages, and tests.

Closes #68.

Making use of cw-plus storage functionality skip for all queries which allow paging.

This is a breaking change in smart contracts as functions that query now require an extra Option<u32> However for frontend it is non-breaking as request is an optional attribute.

This does not change any of the current implementation, merely exposes an alternative way of pagination as opposed to the start_after field. We can now page by page index.

Changes

• Versioning

  • Bump Rust edition to 2021
  • Bump cosmwasm-std to v1.1
  • Bump cw-plus packages to v0.14
  • Bump all contracts and packages in this repo to v0.14 to be consistent with cw-plus

• CI

  • Bump Rust version used in CI to v1.60.0 (was 1.58.1)
  • Bump cosmwasm-vm used in CI to v1.1.0 (was 1.0.0-beta8)

• Dependencies

  • Order all dependencies alphabetically
  • Removed a few unused dependencies
  • Moved a few dependencies that are only used in tests to dev-dependencies

• Schema

  • Replace the derive statement with the new cw_serde decorator, and regenerate schema accordingly:

- #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)]
- #[serde(rename_all = "snake_case")]
+ #[cw_serde]

• Imports
  • in cw721-base/src/helpers.rs it imports the wrong PhantomData:

- use serde::__private::PhantomData;
+ use std::markers::PhantomData;

I assume this is because someone used editor auto-completion for imports...

Notes

• The most powerful feature provided by cw v1.1, the query response derivation, does not work with cw721-base because it doesn't support query msgs that contain generics. I suggest removing QueryMsg::Extension.
• For types that contain generics, it is necessary to rename them when generating schemas. See this example. That is, you want query_msg.json instead of query_msg_for__cw2891_query_msg.json. The latter won't be picked up by the ts-codegen so is kinda useless.

The current implementation of CW721 is not directly compatible with ERC721 with metadata. The proposal is to remove some fields and add a token_uri field for a metadata extension.

Collection-wide fields for CW721:

• name (name of the NFT collection)
• symbol (symbol of NFT collection)

For each TokenId expect it to point to one field token_uri, like ERC721

So mint would look like this:

#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)]
pub struct MintMsg {
    /// Unique ID of the NFT
    pub token_id: String,
    /// The owner of the newly minter NFT
    pub owner: String,
    /// A distinct Uniform Resource Identifier (URI) for a given asset.
    pub token_uri: String,
}

We expect the token_uri to point to a JSON file that conforms to the CW721 JSON schema (a direct copy of the ERC721 JSON schema):

{
    "title": "Asset Metadata",
    "type": "object",
    "properties": {
        "name": {
            "type": "string",
            "description": "Identifies the asset to which this NFT represents"
        },
        "description": {
            "type": "string",
            "description": "Describes the asset to which this NFT represents"
        },
        "image": {
            "type": "string",
            "description": "A URI pointing to a resource with mime type image/* representing the asset to which this NFT represents. Consider making any images at a width between 320 and 1080 pixels and aspect ratio between 1.91:1 and 4:5 inclusive."
        }
    }
}

This where the fields name, description, and image go. Individual NFT creators can then add new JSON fields to extend their NFT:

For example adding an attributes field to support traits:

{
    "name": "name-of-my-asset",
    "description": "description-of-my-asset",
    "image": "image-url",
    "attributes": [
        {
          "trait_type": "Name of some trait", 
          "value": "Value of some trait"
        }, 
    ]
}

In general, extensions to the CW721 JSON schema should follow the opensea metadata standard.

With cross chain initiatives and NFT bridges coming, we want to be able to bridge Ethereum NFTs to chains running CosmWasm, and the best way to enable this is to ensure there is a simple mapping between ERC721 and CW721. I would be happy to make this change (as well as a similar change to CW1155), if the devs think this is reasonable.

Let me preface this issue by saying I'm relatively new to Rust and cosmwasm so this could be "user error."

I was able to mint an NFT on my local development with cw721-metadata-onchain but I'm having issues with cw721-fixed-price. Yes, all the tests pass, and I'm able to store it, but when I try to instantiate it I get:

Error: rpc error: code = InvalidArgument desc = failed to execute message; message index: 0: dispatch: submessages: Error parsing into type cw721_fixed_price::msg::InstantiateMsg: missing field `owner`: instantiate wasm contract failed: invalid request

I have tried including owner in the json object (and I get errors correctly when not adding unit_price, so I know the json is parsing correctly) but that didn't work. I have a feeling it's something with the submessages, maybe incorrect type handling between Cw721InstantiateMsg and InstantiateMsg. But really I have no idea. Any help would be appreciated. Thanks in advance.

Also, in case versioning matters, I set up my local environment for Juno via the instructions on their site here: https://docs.junonetwork.io/smart-contracts-and-junod-development/junod-local-dev-setup#run-juno

Quick question what extension means and is suppose to be: the purpose of cw721 extension is solely for metadata?

Check here: https://github.com/CosmWasm/cw-nfts/blob/main/packages/cw721/src/query.rs#L120

But reading the ERC721 spec there are 2 types of extensions: metadata and enumeration metadata: https://eips.ethereum.org/EIPS/eip-721

Check here: https://github.com/ethereum/EIPs/blob/master/EIPS/eip-721.md

The metadata extension is OPTIONAL for ERC-721 smart contracts (see "caveats", below). This allows your smart contract to be interrogated for its name and for details about the assets which your NFTs represent.

/// @title ERC-721 Non-Fungible Token Standard, optional metadata extension
/// @dev See https://eips.ethereum.org/EIPS/eip-721
///  Note: the ERC-165 identifier for this interface is 0x5b5e139f.
interface ERC721Metadata /* is ERC721 */ {
    /// @notice A descriptive name for a collection of NFTs in this contract
    function name() external view returns (string _name);

    /// @notice An abbreviated name for NFTs in this contract
    function symbol() external view returns (string _symbol);

    /// @notice A distinct Uniform Resource Identifier (URI) for a given asset.
    /// @dev Throws if `_tokenId` is not a valid NFT. URIs are defined in RFC
    ///  3986. The URI may point to a JSON file that conforms to the "ERC721
    ///  Metadata JSON Schema".
    function tokenURI(uint256 _tokenId) external view returns (string);
}

The enumeration extension is OPTIONAL for ERC-721 smart contracts (see "caveats", below). This allows your contract to publish its full list of NFTs and make them discoverable.

/// @title ERC-721 Non-Fungible Token Standard, optional enumeration extension
/// @dev See https://eips.ethereum.org/EIPS/eip-721
///  Note: the ERC-165 identifier for this interface is 0x780e9d63.
interface ERC721Enumerable /* is ERC721 */ {
    /// @notice Count NFTs tracked by this contract
    /// @return A count of valid NFTs tracked by this contract, where each one of
    ///  them has an assigned and queryable owner not equal to the zero address
    function totalSupply() external view returns (uint256);

    /// @notice Enumerate valid NFTs
    /// @dev Throws if `_index` >= `totalSupply()`.
    /// @param _index A counter less than `totalSupply()`
    /// @return The token identifier for the `_index`th NFT,
    ///  (sort order not specified)
    function tokenByIndex(uint256 _index) external view returns (uint256);

    /// @notice Enumerate NFTs assigned to an owner
    /// @dev Throws if `_index` >= `balanceOf(_owner)` or if
    ///  `_owner` is the zero address, representing invalid NFTs.
    /// @param _owner An address where we are interested in NFTs owned by them
    /// @param _index A counter less than `balanceOf(_owner)`
    /// @return The token identifier for the `_index`th NFT assigned to `_owner`,
    ///   (sort order not specified)
    function tokenOfOwnerByIndex(address _owner, uint256 _index) external view returns (uint256);
}

Current:

enum ExecuteMsg<T> {
    Mint(MintMsg<T>),
}

struct MintMsg<T> {
    pub token_id: String,
    pub owner: String,
    pub token_uri: Option<String>,
    pub extension: T,
}

New:

enum ExecuteMsg {
    Mint {
        token_id: String,
        owner: String,
        token_uri: Option<String>,
        extension: T,
    },
}

This is backward compatible because ExecuteMsg serializes to the same JSON.

Cw4973 - Account-bound Tokens (ABT)

Cw4973 is implemented based on the description of EIP-4973. Some requirements in the description will be changed for suitable with CosmWasm characteristics.

In the contract, I also implement the signature based on ADR 036 and signature verification based on @cosmjs/amino.

Specification

Cw4973 is extended from Cw721-base with some modifications in execution section:

• All the metadata structures are unchanged.
• All the query functions and query messages are unchanged.
• All the execute functions and execute messages are removed.
• We add 3 following execute functions and execute messages to the contract:
  • Give
  • Take
  • Unequip

The problem

cw721::ExecuteMsg defines a minimum set of execute methods that all CW-721 variants are expected to implement. However, projects often want to extend it by adding their own methods. For example, cw721-base adds "mint" and "burn" methods. The process for adding these methods is rather cumbersome, for example:

// cw721-base/src/msg.rs
pub enum Cw721ExecuteMsg {
    // below are all copy-pasted from cw721::ExecuteMsg
    TransferNft { .. },
    SendNft { .. },
    Approve { .. },
    Revoke { .. },
    ApproveAll { .. },

    // only the following are new methods added by cw721-base
    RevokeAll { .. },
    Burn { .. }
}

Imo, there are two drawbacks with this approach:

• A lot of copy-pasting is needed (very verbose)
• It is not enforced that the execute message implements all variants required by CW721 specs (What if, say, TransferNft is omitted by mistake? Preferably we want an error to be thrown at compile time to alert the developer that they forgot something.)

Proposed solution

@0xekez showed me it is possible to "autofill" an enum with some pre-defined variants by a macro, which is an approach already used by DAO DAO: https://github.com/DA0-DA0/dao-contracts/blob/main/packages/cwd-macros/src/lib.rs

I propose we create a #[cw721_execute] macro that if used the following way:

use cw721::cw721_execute;

#[cw721_execute]
enum ExecuteMsg {
    Foo,
    Bar,
}

Will transform the enum to:

#[cw_serde]
enum ExecuteMsg {
    TransferNft { .. },
    SendNft { .. },
    Approve { .. },
    Revoke { .. },
    ApproveAll { .. },
    Foo,
    Bar,
}

This prevents the developer to have to copy all the CW721 enum variants over, and enforces all variants are included. Additionally, #[cw_serde] is automatically applied.

In the cw721 crate, the execute message can simply be defined as:

#[cw721_execute]
pub enum ExecuteMsg {}

Further suggestion

It'd be even better if the following traits can be automatically implemented as well:

impl From<cw721::ExecuteMsg> for ExecuteMsg {
    fn from(msg: cw721::ExecuteMsg) -> Self {
        match msg {
            cw721::ExecuteMsg::TransferNft { .. } => Self::TransferNft { .. },
            // ...
        }
    }
}

impl TryFrom<ExecuteMsg> for cw721::ExecuteMsg {
    type Error = String; // or other error type we prefer
    fn try_from(msg: ExecuteMsg) -> Result<Self, Self::Error> {
        match msg {
            ExecuteMsg::TransferNft { .. } => Ok(cw721::ExecuteMsg::TransferNft { .. }),
            // ...
            ExecuteMsg::Foo => Err("`cw721::ExecuteMsg` does not have variant `Foo`".to_string()),
            ExecuteMsg::Bar => Err("`cw721::ExecuteMsg` does not have variant `Bar`".to_string()),
        }
    }
}

This makes it much easier to implement execute entry point:

#[entry_point]
pub fn execute(
    deps: DepsMut,
    env: Env,
    info: MessageInfo,
    msg: ExecuteMsg,
) -> Result<Response, ContractError> {
    match msg {
        // For custom execute methods, dispatch to the appropriate
        // handler functions
        ExecuteMsg::Foo => execute_foo(deps, env, info),
        ExecuteMsg::Bar => execute_bar(deps, env, info),

        // For generic cw721 execute methods,
        // instead of writing a bunch of matching statements,
        // simply cast `ExecuteMsg` into the parent message type
        // and dispatch to the parent contract.
        msg => {
            let parent = Cw721Contract::<Empty, Empty>::default();
            parent.execute(deps, env, info, msg.try_into()?)
        },
    }
}

Notes

• If adopted, this change will deprecate cw721::ExecuteMsg::Extension method as it is no longer needed.

For operators added using ApproveAll, the only way to see them is using AllOperators query. However, that query is paginated which makes it hard to use from another contract. I suggest that we add a query type to directly check the existence of a <owner, operator> pair so operators can check immediately if their approval statuses.