-
Notifications
You must be signed in to change notification settings - Fork 181
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
Conversation
5fe10c0
to
89da1ab
Compare
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
packages/cw721/src/msg.rs
Outdated
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 ...
packages/cw721/src/state.rs
Outdated
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
packages/cw721/src/execute.rs
Outdated
/// 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).
contracts/cw721-base/src/error.rs
Outdated
pub enum ContractError { | ||
#[error(transparent)] | ||
Std(#[from] StdError), | ||
|
||
#[error(transparent)] | ||
Ownership(#[from] OwnershipError), | ||
|
||
#[error(transparent)] | ||
Version(#[from] cw2::VersionError), | ||
|
||
#[error("token_id already claimed")] | ||
Claimed {}, | ||
|
||
#[error("Cannot set approval that is already expired")] | ||
Expired {}, | ||
|
||
#[error("Approval not found for: {spender}")] | ||
ApprovalNotFound { spender: String }, | ||
|
||
#[error("No withdraw address set")] | ||
NoWithdrawAddress {}, | ||
} |
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.
Moved to cw721 package for better re-use
contracts/cw721-base/src/msg.rs
Outdated
use schemars::JsonSchema; | ||
|
||
#[cw_serde] | ||
pub struct InstantiateMsg { |
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.
All msgs and response structs moved from cw721-base
to cw721
for better re-use
packages/cw721/README.md
Outdated
} | ||
|
||
#[derive(Serialize, Deserialize, Clone, PartialEq, JsonSchema, Debug)] | ||
pub struct Metadata { |
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.
Should we have some kind of flexible field here if we are putting all these attributes? I wonder if we would ever need more than what's listed here. Or is attributes flexible enough?
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.
It is flexible enough. There is Metadata.attributes
which is a list of Trait
s:
#[derive(Serialize, Deserialize, Clone, PartialEq, JsonSchema, Debug)]
pub struct Trait {
pub display_type: Option<String>,
pub trait_type: String,
pub value: 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.
So it follows 100% ERC721 spec, using metadata and traits.
What is "T" in this struct naming? pub struct Cw721Config< |
It is a general convention for generics starting generally with "T". Has nothing to do with my name :P Cheers, Mr T |
contracts/cw721-base/src/query.rs
Outdated
TMetadataExtension, | ||
TCustomResponseMessage, | ||
TExtensionExecuteMsg, | ||
TCollectionInfoExtension, |
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.
Much nicer with real trait names, been wanting this for a while. ❤️
/// Deprecated: use GetCollectionInfo instead! Will be removed in next release! | ||
ContractInfo {}, |
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.
@JakeHartnell, added docs for all deprecated query msgs and noting they'll be removed in next release.
packages/cw721/src/msg.rs
Outdated
/// The creator is the only one eligible to update `CollectionInfo`. | ||
UpdateCollectionInfo { | ||
collection_info: CollectionInfoMsg<TCollectionInfoExtensionMsg>, | ||
}, |
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.
@JakeHartnell also note docs here
fn proper_minter_and_creator_initialization() { | ||
// case 1: sender is used in case minter and creator is not set | ||
{ | ||
let mut deps = mock_dependencies(); | ||
|
||
let info_minter_and_creator = mock_info("minter_and_creator", &[]); | ||
Cw721Contract::< | ||
DefaultOptionMetadataExtension, | ||
MetadataMsg, | ||
DefaultOptionCollectionInfoExtension, | ||
CollectionInfoExtensionMsg<RoyaltyInfo>, | ||
Empty, | ||
>::default() | ||
.instantiate( | ||
deps.as_mut(), | ||
mock_env(), | ||
info_minter_and_creator.clone(), | ||
Cw721InstantiateMsg { | ||
name: "collection_name".into(), | ||
symbol: "collection_symbol".into(), | ||
collection_info_extension: None, | ||
creator: None, | ||
minter: None, | ||
withdraw_address: None, | ||
}, | ||
"contract_name", | ||
"contract_version", | ||
) | ||
.unwrap(); | ||
|
||
let minter = MINTER.item.load(deps.as_ref().storage).unwrap().owner; | ||
assert_eq!(minter, Some(info_minter_and_creator.sender.clone())); | ||
let creator = CREATOR.item.load(deps.as_ref().storage).unwrap().owner; | ||
assert_eq!(creator, Some(info_minter_and_creator.sender)); | ||
} | ||
// case 2: minter and creator are set | ||
{ |
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.
unit test for optional creator and minter on init
packages/cw721/src/execute.rs
Outdated
let collection_info = CollectionInfo { | ||
name: msg.name, | ||
symbol: msg.symbol, | ||
extension: msg.collection_info_extension, | ||
updated_at: env.block.time, | ||
}; | ||
collection_info.extension.validate()?; |
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.
... collection info validation during instantiation and ...
packages/cw721/src/execute.rs
Outdated
if let Some(symbol) = msg.symbol { | ||
collection_info.symbol = symbol; | ||
} | ||
collection_info.extension = collection_info.extension.update(&msg.extension)?; |
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.
... update collection info during execution ...
packages/cw721/src/state.rs
Outdated
impl Update<CollectionInfoExtensionMsg<RoyaltyInfo>> for CollectionInfoExtension<RoyaltyInfo> { | ||
fn update( | ||
&self, | ||
msg: &CollectionInfoExtensionMsg<RoyaltyInfo>, | ||
) -> Result<Self, crate::error::Cw721ContractError> { | ||
let mut extension = self.clone(); | ||
// validate royalty before updating | ||
if let Some(royalty_info) = &extension.royalty_info { | ||
royalty_info.validate(msg.royalty_info.clone())?; | ||
} | ||
extension.description = msg.description.clone().unwrap_or(self.description.clone()); | ||
extension.image = msg.image.clone().unwrap_or(self.image.clone()); | ||
extension.external_link = msg.external_link.clone().or(self.external_link.clone()); | ||
extension.explicit_content = msg.explicit_content.or(self.explicit_content); | ||
extension.start_trading_time = msg.start_trading_time.or(self.start_trading_time); | ||
extension.royalty_info = msg.royalty_info.clone().or(self.royalty_info.clone()); | ||
extension.validate()?; | ||
|
||
Ok(extension) | ||
} | ||
} |
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.
... and in implementation for Update trait it does a validation. This way it is always guaranteed there is no invalid data. Custom contracts dont need to do this extra check anymore.
packages/cw721/src/state.rs
Outdated
impl Update<NftInfo<Metadata>> for NftInfo<Metadata> { | ||
fn update(&self, msg: &NftInfo<Metadata>) -> Result<Self, crate::error::Cw721ContractError> { | ||
msg.validate()?; | ||
Ok(msg.clone()) | ||
} | ||
} | ||
|
||
impl<TMetadataExtension> Validate for NftInfo<TMetadataExtension> | ||
where | ||
TMetadataExtension: Validate, | ||
{ |
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.
Same applies for NftInfo and ...
packages/cw721/src/state.rs
Outdated
impl Validate for Metadata { | ||
fn validate(&self) -> Result<(), Cw721ContractError> { | ||
// check URLs | ||
if let Some(image) = &self.image { | ||
Url::parse(image)?; | ||
} | ||
if let Some(url) = &self.external_url { | ||
Url::parse(url)?; | ||
} | ||
if let Some(animation_url) = &self.animation_url { | ||
Url::parse(animation_url)?; | ||
} | ||
if let Some(youtube_url) = &self.youtube_url { | ||
Url::parse(youtube_url)?; | ||
} | ||
// Strings must not be empty | ||
if let Some(image_data) = &self.image_data { | ||
if image_data.is_empty() { | ||
return Err(Cw721ContractError::MetadataImageDataEmpty {}); | ||
} | ||
} | ||
if let Some(desc) = &self.description { | ||
if desc.is_empty() { | ||
return Err(Cw721ContractError::MetadataDescriptionEmpty {}); | ||
} | ||
} | ||
if let Some(name) = &self.name { | ||
if name.is_empty() { | ||
return Err(Cw721ContractError::MetadataNameEmpty {}); | ||
} | ||
} | ||
if let Some(background_color) = &self.background_color { | ||
if background_color.is_empty() { | ||
return Err(Cw721ContractError::MetadataBackgroundColorEmpty {}); | ||
} | ||
} | ||
// check traits | ||
if let Some(attributes) = &self.attributes { | ||
for attribute in attributes { | ||
if attribute.trait_type.is_empty() { | ||
return Err(Cw721ContractError::TraitTypeEmpty {}); | ||
} | ||
if attribute.value.is_empty() { | ||
return Err(Cw721ContractError::TraitValueEmpty {}); | ||
} | ||
if let Some(display_type) = &attribute.display_type { | ||
if display_type.is_empty() { | ||
return Err(Cw721ContractError::TraitDisplayTypeEmpty {}); | ||
} | ||
} | ||
} | ||
} | ||
Ok(()) | ||
} | ||
} | ||
|
||
impl Update<MetadataMsg> for Metadata { |
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.
... and Metadata.
fn proper_collection_info_initialization() { | ||
// case 1: extension set with proper data | ||
{ | ||
let mut deps = mock_dependencies(); |
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.
testing update and validate for collection info during instantiation
#[test] | ||
fn proper_collection_info_update() { | ||
// case 1: update with proper data | ||
{ | ||
// initialize contract | ||
let mut deps = mock_dependencies(); | ||
let env = mock_env(); | ||
let info = mock_info(CREATOR_ADDR, &[]); | ||
let instantiated_extension = Some(CollectionInfoExtension { |
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.
testing update and validate for collection info during execution
#[test] | ||
fn test_migrate() { | ||
let mut deps = mock_dependencies(); | ||
|
||
let env = mock_env(); | ||
use cw721_base_016 as v16; | ||
v16::entry::instantiate( |
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.
migration test and especially how minter, creator, collection info looks like BEFORE and AFTER migration
#[test] | ||
fn mint_with_metadata() { | ||
// case 1: mint with valid metadata | ||
{ | ||
let mut deps = mock_dependencies(); | ||
let contract = setup_contract(deps.as_mut()); | ||
|
||
let token_id = "1".to_string(); | ||
let token_uri = "ipfs://foo.bar".to_string(); | ||
let mint_msg = Cw721ExecuteMsg::Mint { | ||
token_id: token_id.clone(), | ||
owner: String::from("medusa"), | ||
token_uri: Some(token_uri.clone()), | ||
extension: Some(Metadata { | ||
image: Some("ipfs://foo.bar/image.png".to_string()), | ||
image_data: Some("image data".to_string()), | ||
external_url: Some("https://github.com".to_string()), | ||
description: Some("description".to_string()), | ||
name: Some("name".to_string()), | ||
attributes: Some(vec![Trait { | ||
trait_type: "trait_type".to_string(), | ||
value: "value".to_string(), | ||
display_type: Some("display_type".to_string()), | ||
}]), | ||
background_color: Some("background_color".to_string()), | ||
animation_url: Some("ssl://animation_url".to_string()), | ||
youtube_url: Some("file://youtube_url".to_string()), | ||
}), | ||
}; | ||
|
||
let info_minter = mock_info(MINTER_ADDR, &[]); | ||
let env = mock_env(); | ||
contract | ||
.execute(deps.as_mut(), env.clone(), info_minter, mint_msg) | ||
.unwrap(); | ||
} |
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.
happy path testing on contract level, where new traits Update
and Validate
are taken into account. So minting now checks new features and make sure e.g. URLs are correct, Strings are not empty - in case Some(string) is provided.
// case 2: mint with invalid metadata | ||
{ | ||
let mut deps = mock_dependencies(); | ||
let contract = setup_contract(deps.as_mut()); | ||
|
||
let token_id = "1".to_string(); | ||
let token_uri = "ipfs://foo.bar".to_string(); | ||
let info_minter = mock_info(MINTER_ADDR, &[]); | ||
let env = mock_env(); | ||
|
||
let valid_metadata = Metadata { | ||
image: Some("ipfs://foo.bar/image.png".to_string()), | ||
image_data: Some("image data".to_string()), | ||
external_url: Some("https://github.com".to_string()), | ||
description: Some("description".to_string()), | ||
name: Some("name".to_string()), | ||
attributes: Some(vec![Trait { | ||
trait_type: "trait_type".to_string(), | ||
value: "value".to_string(), | ||
display_type: Some("display_type".to_string()), | ||
}]), | ||
background_color: Some("background_color".to_string()), | ||
animation_url: Some("ssl://animation_url".to_string()), | ||
youtube_url: Some("file://youtube_url".to_string()), | ||
}; | ||
|
||
// invalid image | ||
let mut metadata = valid_metadata.clone(); | ||
metadata.image = Some("invalid".to_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.
deep happy path tests down to onchain metadata level! Like it checks whether metadata.image
has an invalid URL!
CollectionInfo
, including RoyaltyInfo
and creator (along with existing minter).CollectionMetadata
, including RoyaltyInfo
and creator (along with existing minter).
packages/cw721/src/state.rs
Outdated
pub struct CollectionMetadata<TCollectionMetadataExtension> { | ||
pub name: String, | ||
pub symbol: String, | ||
pub extension: TCollectionMetadataExtension, | ||
pub updated_at: Timestamp, | ||
} |
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.
This is the new CollectionInfo which replaces...
packages/cw721/src/query.rs
Outdated
pub struct ContractInfoResponse { | ||
pub name: String, | ||
pub symbol: 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.
... ContractInfoResponse.
packages/cw721/src/query.rs
Outdated
pub trait Cw721Query< | ||
// Metadata defined in NftInfo. | ||
TNftMetadataExtension, | ||
// Extension defined in CollectionMetadata. | ||
TCollectionMetadataExtension, | ||
> where | ||
TNftMetadataExtension: Cw721State, | ||
TCollectionMetadataExtension: Cw721State, | ||
{ | ||
fn query( | ||
&self, | ||
deps: Deps, | ||
env: &Env, | ||
msg: Cw721QueryMsg<TNftMetadataExtension, TCollectionMetadataExtension>, | ||
) -> StdResult<Binary> { | ||
match msg { | ||
#[allow(deprecated)] | ||
Cw721QueryMsg::Minter {} => to_json_binary(&self.query_minter(deps.storage)?), | ||
#[allow(deprecated)] | ||
Cw721QueryMsg::ContractInfo {} => { | ||
to_json_binary(&self.query_collection_metadata(deps, env)?) | ||
} | ||
Cw721QueryMsg::GetCollectionMetadata {} => { | ||
to_json_binary(&self.query_collection_metadata(deps, env)?) | ||
} |
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.
All logic from cw721-base
move to trait in default implementations. Here with query
entry point, used by contracts e.g. in lib.rs
packages/cw721/src/execute.rs
Outdated
pub trait Cw721Execute< | ||
// Metadata defined in NftInfo (used for mint). | ||
TNftMetadataExtension, | ||
// Message passed for updating metadata. | ||
TNftMetadataExtensionMsg, | ||
// Extension defined in CollectionMetadata. | ||
TCollectionMetadataExtension, | ||
// Message passed for updating collection metadata extension. | ||
TCollectionMetadataExtensionMsg, | ||
// Defines for `CosmosMsg::Custom<T>` in response. Barely used, so `Empty` can be used. | ||
TCustomResponseMsg, | ||
> where | ||
TNftMetadataExtension: Cw721State, | ||
TNftMetadataExtensionMsg: Cw721CustomMsg + StateFactory<TNftMetadataExtension>, | ||
TCollectionMetadataExtension: Cw721State, | ||
TCollectionMetadataExtensionMsg: Cw721CustomMsg + StateFactory<TCollectionMetadataExtension>, | ||
TCustomResponseMsg: CustomMsg, | ||
{ | ||
fn instantiate( | ||
&self, | ||
deps: DepsMut, | ||
env: &Env, | ||
info: &MessageInfo, | ||
msg: Cw721InstantiateMsg<TCollectionMetadataExtensionMsg>, | ||
contract_name: &str, | ||
contract_version: &str, | ||
) -> Result<Response<TCustomResponseMsg>, Cw721ContractError> { | ||
cw2::set_contract_version(deps.storage, contract_name, contract_version)?; | ||
let config = Cw721Config::< | ||
TNftMetadataExtension, | ||
TNftMetadataExtensionMsg, | ||
TCollectionMetadataExtension, | ||
TCollectionMetadataExtensionMsg, | ||
TCustomResponseMsg, | ||
>::default(); |
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.
Cw721Execute
trait with entry points for instantiate, execute, query, and migrate. Also in both both traits specific calls define, like in...
packages/cw721/src/execute.rs
Outdated
fn transfer_nft( | ||
&self, | ||
deps: DepsMut, | ||
env: &Env, | ||
info: &MessageInfo, | ||
recipient: String, | ||
token_id: String, | ||
) -> Result<Response<TCustomResponseMsg>, Cw721ContractError> { | ||
_transfer_nft::<TNftMetadataExtension>(deps, env, info, &recipient, &token_id)?; | ||
|
||
Ok(Response::new() | ||
.add_attribute("action", "transfer_nft") | ||
.add_attribute("sender", info.sender.to_string()) | ||
.add_attribute("recipient", recipient) | ||
.add_attribute("token_id", 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.
... transfer_nft
default implementation.
packages/cw721/src/execute.rs
Outdated
let collection_metadata = msg.create(deps.as_ref(), env, info, Some(¤t))?; | ||
config | ||
.collection_metadata | ||
.save(deps.storage, &collection_metadata)?; |
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.
... where in update_collection_metadata
where on msg, a single line creates a new, updated state:
let collection_metadata = msg.create(deps.as_ref(), env, info, Some(¤t))?;
And create()
in StateFactory
does a deep creation and validation: ...
packages/cw721/src/state.rs
Outdated
if self.name.is_some() && self.name.clone().unwrap().is_empty() { | ||
return Err(Cw721ContractError::CollectionNameEmpty {}); | ||
} | ||
if self.symbol.is_some() && self.symbol.clone().unwrap().is_empty() { | ||
return Err(Cw721ContractError::CollectionSymbolEmpty {}); | ||
} | ||
Ok(()) |
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.
... here in validate()
in makes sure that name
and symbol
in CollectionMetadata
is not empty.
packages/cw721/src/state.rs
Outdated
fn create( | ||
&self, | ||
deps: Deps, | ||
env: &cosmwasm_std::Env, | ||
info: &cosmwasm_std::MessageInfo, | ||
current: Option<&CollectionMetadata<TCollectionMetadataExtension>>, | ||
) -> Result<CollectionMetadata<TCollectionMetadataExtension>, Cw721ContractError> { | ||
self.validate(deps, env, info, current)?; | ||
match current { | ||
// Some: update existing metadata | ||
Some(current) => { | ||
let mut updated = current.clone(); | ||
if let Some(name) = &self.name { | ||
updated.name = name.clone(); | ||
} |
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.
... then in create
of StateFactory, it:
- validates msg first,
- uses current state and
- update it with msg data
... then it goes one level deeper, ...
packages/cw721/src/state.rs
Outdated
let current_extension = current.extension.clone(); | ||
let updated_extension = | ||
self.extension | ||
.create(deps, env, info, Some(¤t_extension))?; | ||
updated.extension = updated_extension; | ||
Ok(updated) |
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.
... and creates CollectionMedataExtension
, including validation of optional props like description, royalties, start trading time, etc.
packages/cw721/src/msg.rs
Outdated
fn validate( | ||
&self, | ||
deps: Deps, | ||
_env: &Env, | ||
info: &MessageInfo, | ||
_current: Option<&CollectionMetadataExtension<RoyaltyInfo>>, | ||
) -> Result<(), Cw721ContractError> { | ||
// start trading time can only be updated by minter | ||
let minter_initialized = MINTER.item.may_load(deps.storage)?; | ||
if self.start_trading_time.is_some() | ||
&& minter_initialized.is_some() | ||
&& MINTER.assert_owner(deps.storage, &info.sender).is_err() | ||
{ | ||
return Err(Cw721ContractError::NotMinter {}); | ||
} | ||
// all other props collection metadata extension can only be updated by the creator | ||
let creator_initialized = CREATOR.item.may_load(deps.storage)?; | ||
if (self.description.is_some() | ||
|| self.image.is_some() | ||
|| self.external_link.is_some() | ||
|| self.explicit_content.is_some() | ||
|| self.royalty_info.is_some()) | ||
&& creator_initialized.is_some() | ||
&& CREATOR.assert_owner(deps.storage, &info.sender).is_err() | ||
{ | ||
return Err(Cw721ContractError::NotCollectionCreator {}); | ||
} | ||
// check description length, must not be empty and max 512 chars | ||
if let Some(description) = &self.description { | ||
if description.is_empty() { | ||
return Err(Cw721ContractError::CollectionDescriptionEmpty {}); | ||
} | ||
if description.len() > MAX_COLLECTION_DESCRIPTION_LENGTH as usize { | ||
return Err(Cw721ContractError::CollectionDescriptionTooLong { | ||
max_length: MAX_COLLECTION_DESCRIPTION_LENGTH, | ||
}); | ||
} | ||
} |
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.
Pls also note that StateFactory
can also do complex validation. Like here in CollectionMetadataExtension
:
start_trading_time
can only be updated by Minter- all other probs belongs to and can only be updated by Creator 😎
In short: all state-based logic can be handled in StateFactory.
…rationContract for ease-of-use
…a-onchein, cleanup
df8ebc8
to
a996184
Compare
Major refactoring for upcoming v0.19 release.
TL;DR: v0.19 release is a major refactoring, introducing distinctions between Creator (manages collection metadata like royalties) and Minter (controls minting), and extends
UpdateNftInfo
with extension for onchain metadata. We've also added a newCollectionInfo
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
is only aware of minter (aka owner/minter ownership). Here we addedCreator
. Creator controls collection info (like royalties) whereas minter controls minting.NEW utility:
UpdateNftInfo
andUpdateCollectionInfo
msgBy adding a new
CollectionInfoExtension
store as an optional extension forCollectionInfo
(name and symbol), there is also logic introduced here for updating collection metadata.The creator is now able to do both: updating collection and NFT Info!
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.