Refactor InstantiateMsg for better ics721 support

[JakeHartnell]

Going over ics721 with a few folks and came to the realization there are few things we should change about cw721-base.

Namely, ics721 has no support for the name and symbol fields we currently instantiate the contract with... when you send an NFT over IBC, that information will be lost. Moreover, ics721 does support a classUri which represents collection level metadata. Unfortunately, we have do not have support for this.

We currently have:

#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)]
pub struct InstantiateMsg {
    /// Name of the NFT contract
    pub name: String,
    /// Symbol of the NFT contract
    pub symbol: String,

    /// The minter is the only one who can create new NFTs.
    /// This is designed for a base NFT that is controlled by an external program
    /// or contract. You will likely replace this with custom logic in custom NFTs
    pub minter: String,
}

I propose we kill name and symbol in favor of collection_uri and an extension. Like so:

#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)]
pub struct InstantiateMsg<T> {
    /// URI for Collection Metadata: https://docs.opensea.io/docs/contract-level-metadata
    pub collection_uri: Option<String>,
    /// Extension
    pub extension: T,

    /// The minter is the only one who can create new NFTs.
    /// This is designed for a base NFT that is controlled by an external program
    /// or contract. You will likely replace this with custom logic in custom NFTs
    pub minter: String,
}

This will allow for better support for ics721 and frankly better collection metadata.

The ics721 spec: https://github.com/cosmos/ibc/tree/master/spec/app/ics-721-nft-transfer

[0xekez]

Would also propose that we add a Burn method to the cw721 spec.

[JakeHartnell]

Hmmmm, name and symbol are included in the ERC721 spec... maybe we should keep them?

/// @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);
}

[shanev]

An alternative is a way to support the various ways collection-level metadata is stored.

pub struct InstantiateMsg<T> {
    pub metadata: T,

    pub minter: String,
}

And provide some options:

pub struct ERC721Metadata {
    pub name: String,
    pub symbol: String,
}

pub struct CollectionInfo {
    pub uri: String

This allows flexibility in supporting on-chain collection-level metadata too. I don't see any harm in keeping name, and symbol though. Removing them may cause issues for some chains that already use them.

[JakeHartnell]

An alternative is a way to support the various ways collection-level metadata is stored.

pub struct InstantiateMsg<T> {
    pub metadata: T,

    pub minter: String,
}

And provide some options:

pub struct ERC721Metadata {
    pub name: String,
    pub symbol: String,
}

pub struct CollectionInfo {
    pub uri: String

This allows flexibility in supporting on-chain collection-level metadata too. I don't see any harm in keeping name, and symbol though. Removing them may cause issues for some chains that already use them.

Nice! Yeah, I'm also going back against removing name and symbol.

Would be down with an extensible metadata trait... kind of like it better that way. However, given the role of classUri in the ics721 do you think we should make something like collection_uri part of the default?

We use that info in Stargaze after all, would be nice to standardize it. 😂

Like metadata better than extension. Could certainly include name and symbol in the default examples and make sure metadata is returned from the current contract info queries...

#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)]
pub struct InstantiateMsg<T> {
    /// URI for Collection Metadata: https://docs.opensea.io/docs/contract-level-metadata
    pub collection_uri: Option<String>,
    
    /// Metadata extension
    pub metadata: T,

    /// The minter is the only one who can create new NFTs.
    /// This is designed for a base NFT that is controlled by an external program
    /// or contract. You will likely replace this with custom logic in custom NFTs
    pub minter: String,
}

Maybe something like collection_uri could be part of the metadata extension, I'm just worried about the impact on UX if there are lots of NFTs without contract level metadata.