Move to cw721 package #161
Conversation
…CollectionInfo, ContractInfoResponse -> CollectionInfoResponse
…ration and keep legacy data for backward migration
- 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
taitruong
force-pushed
the
move_to_cw721_package
branch
from
d27219b
to
9e06b0b
Compare
|
|
||
| // impl CustomMsg for Cw2981QueryMsg {} | ||
|
|
||
| impl From<QueryMsg> for Cw721QueryMsg<Extension> { |
There was a problem hiding this comment.
Is a goal of this PR to enable functionality of cw2981? We are adding all the messages here.
| Ok(self | ||
| .base_contract | ||
| .revoke(deps, env, info, spender, token_id)?) | ||
| } | ||
|
|
||
| fn approve_all( | ||
| pub fn transfer_nft_include_nft_expired( |
There was a problem hiding this comment.
Why are we changing the name here? What's the difference between approve_all and transfer nft include nft expired?
| OwnerOf { | ||
| token_id: String, | ||
| /// unset or false will filter out expired approvals, you must set to true to see them | ||
| include_expired: Option<bool>, | ||
| /// unset or false will filter out expired nfts, you must set to true to see them | ||
| include_invalid: Option<bool>, |
There was a problem hiding this comment.
What is difference between "invalid" and "expired" ?
| } | ||
|
|
||
| pub fn approval( | ||
| pub fn query_approval_include_expired_nft( |
There was a problem hiding this comment.
How come we add "query" in the name? Before they just say "approval" so I'm not sure if naming convention necessitates that we put "query" in the name or not?
| } | ||
|
|
||
| // --- helpers --- | ||
| pub fn is_nft_expired(&self, deps: Deps, env: &Env, token_id: &str) -> StdResult<bool> { |
There was a problem hiding this comment.
It seems like this "expired nft" is the bulk of the PR. What exactly is an "expired nft?"
|
Can you better explain why we have this giant app centered around cw721 expiration? Shouldn't "expiration" kind of just be an option handed to a regular cw721 rather than a giant app? |
This PR only moves code from contracts to cw721 package - as a preparation for PR #156 (
Major release v0.19: add CollectionInfo, RoyaltyInfo, updatable NFTs for creator, etc.).@jhernandezb, @JakeHartnell, all checks passed - except lint checker fails with
use of deprecated variant ...I have marked deprecated usages with #[allow(deprecated)], but linter still complaints.
Did the same in launchpad PR, but here it is all fine:
public-awesome/launchpad#668