public-awesome / cw-nfts

Examples and helpers to build NFT contracts on CosmWasm
Apache License 2.0
185 stars 181 forks source link

Contract level metadata #75

Closed JakeHartnell closed 1 year ago

JakeHartnell commented 2 years ago

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.

JakeHartnell commented 2 years ago

@shanev / @yubrew, how do you feel about these changes? Is there anything you would like to see different?

larry0x commented 1 year ago

SG-721 also has collection-level metadata but it’s implemented differently. I think @shanev should take a look at this and see if we can avoid the conflicts

JakeHartnell commented 1 year ago

SG-721 also has collection-level metadata but it’s implemented differently. I think @shanev should take a look at this and see if we can avoid the conflicts

Yes, would be good to get his thoughts. @shanev care to weigh in?

sg721 uses collection_info, so could make a custom metadata extension with all the same info. https://github.com/public-awesome/launchpad/blob/main/packages/sg721/src/lib.rs#L81

Happy to help on the migration side there. Frontend changes would be minor as well, just would need to continue to check the collection_info field for legacy sg721 contracts that aren't migrated. Queries and everything else would be the same.

JakeHartnell commented 1 year ago

Thanks for this update!

I'm a bit concerned about the additional complexity this is adding with execute and query extensions. Can't those be added be added simply by adding new execute and query functions in a new contract with a wrapper struct? Check out Extending the refactored sg721-base. So instead of extending this contract to support all uses cases, have new contracts compose this and add to it. Thoughts?

A noob getting into CW NFTs might be a bit overwhelmed with all the options. Ideally cw721-base should be a simple starting point.

Valid concern! A couple of thoughts here:

  1. Consumers of cw721-base like sg721-base can certainly make a nicer development experience on top of this. I think that's a great opportunity for sg721. Really love the work @larry0x did there. Perhaps he has some thoughts on this PR?
  2. We already had Execute extensions, but lacked query and collection metadata extensions. Query extensions were merged in with this PR: https://github.com/CosmWasm/cw-nfts/pull/42
  3. Query extensions can remove a lot of repetative code and complexity around extending the base query types, so no need to impl From<QueryMsg> for Cw721QueryMsg like here: https://github.com/public-awesome/launchpad/blob/main/contracts/sg721-base/src/msg.rs#L50
  4. Traits are a little confusing for new rust programmers, but something folks will have to learn. To address this, we can and should make better dev-ex wrappers around this contract like sg721-base. Completely open to better patterns (maybe @larry0x has some ideas?), but I do think this PR makes things a little better by making everything extensible in the same way and giving human readable names to all the traits.

The thing I like about Query and Metadata extensions is that they allow for a single file NFT contract that customizes all aspects.

Here's an example that has custom execute, collection metadata, token metadata, and query extensions... all in 118 lines of code.

use cosmwasm_schema::cw_serde;
use cosmwasm_std::{CustomMsg, Empty};
pub use cw721_base::{
    ContractError, Cw721Contract, ExecuteMsg, InstantiateMsg, MintMsg, MinterResponse, QueryMsg,
};

// Version info for migration
const CONTRACT_NAME: &str = "crates.io:cw721-example";
const CONTRACT_VERSION: &str = env!("CARGO_PKG_VERSION");

// MintExt allows for adding custom on-chain metadata to your NFTs
#[cw_serde]
pub struct MintExt {
    creator: String,
}
impl CustomMsg for MintExt {}

// Define custom contract metadata, the ContractInfo query will return with the info set here
#[cw_serde]
pub struct CollectionMetadataExt {
    pub creator: String,
}
impl CustomMsg for CollectionMetadataExt {}

// Define a custom query ext
#[cw_serde]
pub enum QueryExt {
    AdditionalQuery {},
}
impl CustomMsg for QueryExt {}

// Define a custom query response
#[cw_serde]
pub struct AdditionalQueryResponse {
    message: String,
}

// Define a custom execute extension. Allows for creating new contract methods
#[cw_serde]
pub enum ExecuteExt {
    AdditionalExecute {},
}
impl CustomMsg for ExecuteExt {}

// Put it all together!
pub type CustomCw721<'a> =
    Cw721Contract<'a, MintExt, Empty, CollectionMetadataExt, ExecuteExt, QueryExt>;

#[cfg(not(feature = "library"))]
pub mod entry {
    use super::*;

    use cosmwasm_std::{entry_point, to_binary};
    use cosmwasm_std::{Binary, Deps, DepsMut, Env, MessageInfo, Response, StdResult};
    use cw2::set_contract_version;

    #[entry_point]
    pub fn instantiate(
        mut deps: DepsMut,
        env: Env,
        info: MessageInfo,
        msg: InstantiateMsg<CollectionMetadataExt>,
    ) -> Result<Response, ContractError> {
        // Call the instantiate on our base cw721 with our custom extensions
        let res = CustomCw721::default().instantiate(deps.branch(), env, info, msg)?;
        // Explicitly set contract name and version, otherwise set to cw721-base info
        set_contract_version(deps.storage, CONTRACT_NAME, CONTRACT_VERSION)
            .map_err(ContractError::Std)?;
        Ok(res)
    }

    #[entry_point]
    pub fn execute(
        deps: DepsMut,
        env: Env,
        info: MessageInfo,
        msg: ExecuteMsg<MintExt, ExecuteExt>,
    ) -> Result<Response, ContractError> {
        match msg {
            // Match extension messages
            ExecuteMsg::Extension { msg } => match msg {
                // Map them to their message handlers
                ExecuteExt::AdditionalExecute {} => execute_custom(deps, env, info),
            },
            // Handle other messages with the cw721-base default
            _ => CustomCw721::default().execute(deps, env, info, msg),
        }
    }

    #[entry_point]
    pub fn query(deps: Deps, env: Env, msg: QueryMsg<QueryExt>) -> StdResult<Binary> {
        match msg {
            // Match extension messages
            QueryMsg::Extension { msg } => match msg {
                // Map them to their message handlers
                QueryExt::AdditionalQuery {} => to_binary(&custom_query(deps, env)?),
            },
            // Handle other queries with the cw721-base default
            _ => CustomCw721::default().query(deps, env, msg),
        }
    }

    // Custom execute handler
    pub fn execute_custom(
        deps: DepsMut,
        env: Env,
        info: MessageInfo,
    ) -> Result<Response, ContractError> {
        Ok(Response::default())
    }

    // Custom query handler
    pub fn custom_query(deps: Deps, env: Env) -> StdResult<AdditionalQueryResponse> {
        Ok(AdditionalQueryResponse {
            message: String::from("meow"),
        })
    }
}
JakeHartnell commented 1 year ago

maybe on chain collection info is required by the current ics721. some creators will want to use a collection uri anyway and then relay it to other chains even with a broken collection uri. if there is no optional standard for collection uri, then the number of user generated deviations can easily create downstream problems.

+1

first: cw721-base, does the base mean basic like the 101 version to get up and running quickly? or does it mean base like the foundation of a great empire with heavy infrastructure?

cw721-base (as it is currently written) is meant to a base for other NFT smart contracts like sg721.

To make things easier, we could make a cw721-basic contract as a 101. We could also make a video on writing custom NFT contracts. Personally, I think extension does make things easier and not harder.

Is there room to improve? Of course, but we shouldn't let our quest for perfection be the enemy of the good.

i think it should mean basic and err on the side of too simplistic so non-devs can get productive quickly.

Non-devs can use things like Stargaze Studio or other tools. If people want to work with smart contracts, they have to learn things... I don't think there is a way to get around that.

JakeHartnell commented 1 year ago

Closing in favor of this approach: https://github.com/CosmWasm/cw-nfts/issues/90