-
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
#151 make minter optional #152
Conversation
You can also see from adjusted tests - it looks more convenient and natural. |
@@ -136,7 +136,7 @@ mod tests { | |||
let init_msg = InstantiateMsg { | |||
name: "SpaceShips".to_string(), | |||
symbol: "SPACE".to_string(), | |||
minter: CREATOR.to_string(), | |||
minter: None, |
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.
If you scroll up, in line 135, you can see CREATOR is the sender
Reason, why I came to this conclusion is this here: https://github.com/public-awesome/launchpad/blob/main/contracts/collections/sg721-base/src/contract.rs#L53-L58 // cw721 instantiation
let info = CW721ContractInfoResponse {
name: msg.name,
symbol: msg.symbol,
};
self.parent.contract_info.save(deps.storage, &info)?;
cw_ownable::initialize_owner(deps.storage, deps.api, Some(&msg.minter))?; All this logic can completely be removed, by calling pub struct InstantiateMsg {
pub name: String,
pub symbol: String,
pub minter: String,
pub collection_info: CollectionInfo<RoyaltyInfoResponse>,
} |
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.
I'm fine with this, curious what others thinks, but agree it would be slightly nicer.
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.
Good change :)
PR for #151
Rather prefer having
minter: Option<String>
:https://github.com/CosmWasm/cw-nfts/blob/177a993dfb5a1a3164be1baf274f43b1ca53da53/contracts/cw721-base/src/msg.rs#L8-L18
In case of none,
info.sender
should be minter. Makes it easier for other contracts (e.g. minter) re-using this msg.In most cases creator of contract is also minter. Rn creator (=sender) instantiates contract AND need to provides its own address in
minter
prop. You can see this in unit tests:Before: duplicate, since creator is in
info.sender
andminter.prop
After: no need to provider
minter
prop, since creator is sender:Instantiation is like this then:
like before, in case caller should NOT be minter/owner (edge case):
for creator (standard, 99.99% case):
@shanev @JakeHartnell