public-awesome / cw-nfts

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

Prevent repetitive write of cw2 version info during instantiation #110

Closed larry0x closed 1 year ago

larry0x commented 1 year ago

Background

To understand what the problem is, let's first talk about how to extend/inherit/customize the base cw721 contract. Using Rust's "composition" pattern, the way to do this is:

#[derive(Default)]
struct<'a> MyCustomCw721Contract<'a> {
    // for simplicity, I use Empty for all extensions in this example
    pub base: Cw721Contract<'a, Empty, Empty, Empty, Empty>,

    // my custom fields here...
}

(this is what we do with stargaze's sg721-base, btw!)

Then, during instantiation, I simply invoke the base contract's instantiate method:

#[entry_point]
fn instantiate(
    deps: DepsMut,
    env: Env,
    info: MessageInfo,
    msg: InstantiateMsg,
) -> StdResult<Response> {
    // set my custom contract version
    cw2::set_contract_version("crates.io:my-custom-nft", "1.0.0")?;

    let tract = MyCustomCw721Contract::default();

    // run the base contract's instantiate logic
    // !! WARNING: the base contract OVERWRITES my version info here !!
    tract.base.instantiate(deps, env, info, msg)?;

    // my custom instantiate logics...
}

The problem

cw721-base initializes the cw2 version info inside the Cw721Contract::instantiate method. This means, by running tract.base.instantiate, the base contract overwrites my custom cw2 info!

To avoid this, we have to run cw2::set_contract_version AFTER tract.base.instantiate. This means the cw2 storage slot is written TWICE during instantiation, once by the base contract, once by my custom contract.

This problem is present in cw721-metadata-onchain for example, see the comment on L54:

https://github.com/CosmWasm/cw-nfts/blob/a9e4ca67a0d13f56bd38cfcdd69cdaa2548f5de9/contracts/cw721-metadata-onchain/src/lib.rs#L53-L57

The solution

The initialization of cw2 version should NOT be in Cw721Contract::instantiate, but in cw721_base::entry::instantiate. This way, by running Cw721Contract::instantiate the base contract does NOT overwrite the custom cw2 info.

larry0x commented 1 year ago

@JakeHartnell @0xekez @shanev plz review 🙏