New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Major release v0.19: add CollectionInfo
, RoyaltyInfo
, updatable NFTs for creator, etc.
#156
base: main
Are you sure you want to change the base?
Conversation
…CollectionInfo, ContractInfoResponse -> CollectionInfoResponse
…ration and keep legacy data for backward migration
afaa196
to
a2b5c12
Compare
5fe10c0
to
89da1ab
Compare
- new query GetMinterOwnership and GetCreatorOwnership, deprecated Ownership - new execute UpdateMinterOwnership and UpdateCreatorOwnership, deprecate UpdateOwnership - dedicated stores for minter and creator, where creator usess by default cw_ownable singleton! - new migrate msg allowing to reset creator and minter - cleanup migration and split legacy part to dedicated functions - also make sure using decicated CREATOR and MINTER stores and NOT use cw_ownable::...
- move logic from cw721-base to cw721 - merge cw721-base and cw721-metadata-onchain into one, distinction is: `extension: T` where T for base contract is `type DefaultMetadataExtension = Option<Metadata>` - all logic now in default implementation for traits Cw721Execute and Cw721Query
0494810
to
895f462
Compare
packages/cw721/src/query.rs
Outdated
#[allow(deprecated)] | ||
Cw721QueryMsg::ContractInfo {} => { | ||
to_json_binary(&self.query_collection_info(deps, env)?) | ||
} | ||
Cw721QueryMsg::GetCollectionInfo {} => { | ||
to_json_binary(&self.query_collection_info(deps, env)?) | ||
} | ||
Cw721QueryMsg::NftInfo { token_id } => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also ContractInfo
is deprecated and is replaced by GetCollectionInfo
. In next release all deprecated msgs should be removed. Here it is kept for backward compatibility, so existing frontends wont break!
packages/cw721/src/msg.rs
Outdated
#[deprecated(since = "0.19.0", note = "Please use GetMinterOwnership instead")] | ||
#[returns(Ownership<Addr>)] | ||
Ownership {}, | ||
|
||
#[returns(Ownership<Addr>)] | ||
GetMinterOwnership {}, | ||
|
||
#[returns(Ownership<Addr>)] | ||
GetCreatorOwnership {}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ownership -> GetMinterOwnership
new GetCreatorOwnership
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comments on these queries would be helpful. Good to note what they are in plain English and that minter
will soon be deprecated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
packages/cw721/src/msg.rs
Outdated
#[deprecated(since = "0.19.0", note = "Please use GetMinterOwnership instead")] | ||
#[returns(MinterResponse)] | ||
Minter {}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also for backward compatibility: Minter -> GetMinterOwnership
pub enum Cw721MigrateMsg { | ||
WithUpdate { | ||
minter: Option<String>, | ||
creator: Option<String>, | ||
}, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important: new migrate msg allowing to reset creator and minter!
packages/cw721/src/traits.rs
Outdated
pub trait Cw721Execute<T, C> | ||
where | ||
T: Serialize + DeserializeOwned + Clone, | ||
C: CustomMsg, | ||
{ | ||
type Err: ToString; | ||
|
||
fn transfer_nft( | ||
&self, | ||
deps: DepsMut, | ||
env: Env, | ||
info: MessageInfo, | ||
recipient: String, | ||
token_id: String, | ||
) -> Result<Response<C>, Self::Err>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before it e.g. Cw721Execute
was rather an interface with no logic. Also note confusing generics naming like in Cw721Execute<T, C>
and ...
pub const CREATOR: OwnershipStore = OwnershipStore::new(OWNERSHIP_KEY); | ||
/// - minter is stored in the contract storage using cw_ownable::OwnershipStore (same as for OWNERSHIP but with different key) | ||
pub const MINTER: OwnershipStore = OwnershipStore::new("collection_minter"); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CREATOR
and MINTER
. Please note that singleton ownership was used before for minter, but now used for creator! new minter ownership has a new key collection_minter
. This is also covered in migration (as shown later below).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay so to be clear, we now have an optional second owner which is creator. The max number of owners in the contract would now be 2 and not more than that right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay so to be clear, we now have an optional second owner which is creator. The max number of owners in the contract would now be 2 and not more than that right?
Correct. They are both optional: creator and minter. Naming convention before was very confusing - started with the introduction of cw-ownable
. There it was minter = owner = ownership.
Imo, using terms creator
and minter
is much more clear. Ownership appears here only in terms of minter ownership and creator ownership. Both uses for 2-step for transfer as defined by cw-ownable
:
- Owner transfers to new owner
- New owner accepts, on acceptance minter/creator ownership is changed
/// Migrates only in case ownership is not present | ||
/// !!! Important note here: !!! | ||
/// - creator owns the contract and can update collection info | ||
/// - minter can mint new tokens | ||
/// | ||
/// Before v0.19.0 there were confusing naming conventions: | ||
/// - v0.17.0: minter was replaced by cw_ownable, as a result minter is owner | ||
/// - v0.16.0 and below: minter was stored in dedicated `minter` store (so NOT using cw_ownable at all) | ||
pub fn migrate_legacy_minter_and_creator( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
migrate_legacy_minter_and_creator: migrate legacy minter and set new creator...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@humanalgorithm here is the most important part, allowing to migrate any older version to the latest
pub fn migrate<T, C, E, Q>(deps: DepsMut) -> Result<Response<C>, ContractError> | ||
where | ||
T: Serialize + DeserializeOwned + Clone, | ||
Q: CustomMsg, | ||
E: CustomMsg, | ||
{ | ||
// remove old minter info | ||
let tract16 = v16::Cw721Contract::<T, C, E, Q>::default(); | ||
let minter = tract16.minter.load(deps.storage)?; | ||
tract16.minter.remove(deps.storage); | ||
|
||
// save new ownership info | ||
let ownership = cw_ownable::initialize_owner(deps.storage, deps.api, Some(minter.as_str()))?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also this old migrato logic of minter to ownership is error prone. Like it is e.g. possible to migrate from v0.16 to v0.18. Then minter is transferred to new addr. Keep in mind in old minter store there is still old addr. So migrating back to v0.16 and to v0.18 again, would reset minter addy. Better to always check whether new minter ownship is set - if so no migration needed.
This is an edge case, but still not good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We needed this for migration to v.01.7. I think I might have been the one that "wrote" it but I was just copying migration logic from cw base contract.
CollectionInfo
, including RoyaltyInfo
and creator (along with existing minter).
ccf4784
to
11f3231
Compare
Major refactoring for upcoming v0.19 release.
TL;DR: The cw721 v0.19 release sees a major refactor, introducing distinctions between Creator (manages collection metadata like royalties) and Minter (controls minting), and adds a new UpdateMetadata utility for onchain metadata. We've integrated a new CollectionMedata structure within the cw721 package, moving all core logic for easier access and implementation across contracts, thus streamlining contract architecture. This update simplifies the ecosystem by removing the need for separate cw721-metadata-onchain and cw2981-royalty contracts and clearly defining roles and functionalities.
Distinction between
Creator
andMinter
In previous release
cw721
only knows minter (aka owner). Here we addedCreator
. Creator controls collection metadata (like royalties) whereas minter controls minting.NEW utility:
UpdateNftInfo
andUpdateCollectionMetadata
msgBy adding a new
CollectionMetadataExtension
store as an optional extension forCollectionMetadata
(name and symbol), there is also logic introduced here for updating collection metadata.Same logic is also applied now for updating onchain NFT metadata (after mint).
The creator is now able to do both: updating collection and onchain NFT metadata!
NEW
CollectionMedata
incw721
packageFor making it available to all existing contracts, I had to move all core logic into
cw721
package. As a result, outcome is the following:Cw721Execute
andCw721Query
traitsAnother upgrade is adding
creator
along to existingminter
address. In previous release due to the introduction ofcw-ownable
there has been a renaming ofminter
toownership
- which is confusing. With upcoming v0.19 release it looks like this:ownership
in v0.17-18)ownership
(deprecated)start_trading_time
which belongs to minterStateFactory
all stores are in
state.rs
(pls note newCw721Config
which will be used for contracts):Essentially all boilerplate code (default implementations in
Cw721Execute
andCw721Query
traits) are now incw721
package.As a result contracts are very slim. Please note, implementations by
Cw721Config
, along withCw721Execute
andCw721Query
traits are opionated - though contracts may be implement differently by overriding default implementations.Here is how new
cw721-base
looks like by usingcw721
package:Define struct for contract interaction:
Then implement execute and query traits and using default implementations:
And finally in lib.rs:
That's it!
cw721-metadata-onchain
andcw2981-royalty
contracts removedAnother positive side effect of this refactoring is that there's no need for
cw721-metadata-onchain
andcw2981-royalty
contracts anymore. This is covered incw721-base
. Design hasnt changed and onchain metadata is stored inNftInfo<TNftMetadataExtension>
. Before this PR, funny thing is that incw721-base
it was defined as:TMetadataExtension = Option<Empty>
, allowing to NOT store onchain data, but it doesnt make sense havingNone
andSome(Empty)
.In
cw721-metadata-onchain
contract it was defined as:TMetadataExtension = Option<Metadata>
, here for onchain contract it was allowed to either provide onchain data or event not!In this PR it defines a
pub type DefaultOptionNftMetadataExtension = Option<NftMetadata>
which is used incw721-base
. onchain contract is obsolete and is removed.