public-awesome / cw-nfts

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

Migrate uses upgrades/v0_17 #132

Closed taitruong closed 3 months ago

taitruong commented 1 year ago

current version is v0.18 but it still uses this: https://github.com/CosmWasm/cw-nfts/blob/177a993dfb5a1a3164be1baf274f43b1ca53da53/contracts/cw721-base/src/upgrades/v0_17.rs#L23-L24

...
    Ok(Response::new()
        .add_attribute("action", "migrate")
        .add_attribute("from_version", "0.16.0")
        .add_attribute("to_version", "0.17.0")
        .add_attribute("old_minter", minter)
        .add_attributes(ownership.into_attributes()))

So response attributes are not correct. There are const like EXPECTED_FROM_VERSION and CONTRACT_VERSION it could use for from_version and to_version

taitruong commented 1 year ago

This migration does affect not only v0.16 - but also older versions. Migration logic assumes current contract is version v0.16 (and lower): https://github.com/CosmWasm/cw-nfts/blob/177a993dfb5a1a3164be1baf274f43b1ca53da53/contracts/cw721-base/src/upgrades/v0_17.rs#L14-L19

Instead it may query for ownership and if it doesn't exist, it could do a migration, like this:

// cw721 v0.17 and higher holds ownership in the contract
let ownership: StdResult<Ownership<Addr>> = deps
    .querier
    .query_wasm_smart(sender, &cw721_base::msg::QueryMsg::Ownership::<Addr> {});
if ownership.is_err() {
    // cw721 v0.16 and lower holds minter
    let minter_response: cw721_base_016::msg::MinterResponse = deps
        .querier
        .query_wasm_smart(sender, &cw721_base_016::QueryMsg::Minter::<Empty> {})?;
    // migration logic here...
}
taitruong commented 3 months ago

Fixed in #161