Expose cw20 logo and marketing info API on dso-token #38
Conversation
| pub logo: Option<Logo>, | ||
| } | ||
|
|
||
| impl From<InstantiateMarketingInfo> for cw20_base::msg::InstantiateMarketingInfo { |
There was a problem hiding this comment.
You use a different type as the root type doesn't have Debug, Clone, PartialEq, right?
After that is merged and released, you can just use the cw20_base type completely, right?
There was a problem hiding this comment.
Yes, that is the plan.
contracts/dso-token/src/contract.rs
Outdated
| verify_sender_on_whitelist(&deps, &info.sender)?; | ||
| } | ||
|
|
||
| verify_sender_on_whitelist(&deps, &info.sender)?; |
There was a problem hiding this comment.
No need to check whitelist here.
We need the whitelist to move tokens, but it is not required if they just change the logo of the description.
It is likely they will be on the whitelist, but we can remove these two checks.
contracts/dso-token/src/contract.rs
Outdated
| @@ -164,6 +161,7 @@ pub fn execute( | |||
| ) | |||
| } | |||
| ExecuteMsg::UploadLogo(logo) => { | |||
| verify_sender_on_whitelist(&deps, &info.sender)?; | |||
| @@ -202,11 +200,15 @@ pub fn query(deps: Deps, _env: Env, msg: QueryMsg) -> StdResult<Binary> { | |||
| QueryMsg::AllAccounts { start_after, limit } => { | |||
| to_binary(&query_all_accounts(deps, start_after, limit)?) | |||
| } | |||
| QueryMsg::MarketingInfo {} => to_binary(&query_marketing_info(deps)?), | |||
| @@ -436,4 +442,163 @@ mod tests { | |||
| .unwrap(); | |||
| assert!(!is_whitelisted.whitelisted); | |||
| } | |||
|
|
|||
| #[test] | |||
| fn update_marketing() { | |||
There was a problem hiding this comment.
Nice to cover with tests.
There was a problem hiding this comment.
As we don't want to check for whitelist are those test needed at all? cw20 covers it pretty well.
There was a problem hiding this comment.
yes, you can drop those tests.
contracts/dso-token/src/contract.rs
Outdated
| } | ||
| ); | ||
|
|
||
| let err = router |
There was a problem hiding this comment.
No need for this unauthorised check.
marketing can always adjust the logos regardless of whitelist
contracts/dso-token/src/contract.rs
Outdated
| ) | ||
| .unwrap_err(); | ||
|
|
||
| assert_eq!(err, "Unauthorized".to_owned()); |
There was a problem hiding this comment.
Same here... no need for unauthorised here
| @@ -30,7 +32,7 @@ pub fn instantiate( | |||
| decimals: msg.decimals, | |||
| initial_balances: msg.initial_balances, | |||
| mint: msg.mint, | |||
| marketing: None, | |||
| marketing: msg.marketing.map(Into::into), | |||
| pub project: Option<String>, | ||
| pub description: Option<String>, | ||
| pub marketing: Option<String>, | ||
| pub logo: Option<Logo>, |
There was a problem hiding this comment.
No strong feelings, but shouldn't at least one of these (project?) be mandatory?
There was a problem hiding this comment.
It is not in original cw20 and I think it is nothing which would make it required. It is basically only for display, and if all are None, the whole structure would not be stored (which is handled in cw20).
| @@ -3,7 +3,9 @@ use cosmwasm_std::{ | |||
| }; | |||
| use cw2::set_contract_version; | |||
| use cw20_base::allowances::query_allowance; | |||
| use cw20_base::contract::{query_balance, query_minter, query_token_info}; | |||
| use cw20_base::contract::{ | |||
| query_balance, query_download_logo, query_marketing_info, query_minter, query_token_info, | |||
There was a problem hiding this comment.
In passing: query_logo or download_logo instead of query_download_logo?
There was a problem hiding this comment.
It is unified convention in cw20_base - all functions called inside query (so partially taking care about handling query message) are in form of query_msg_enum_variant. I actually would strip this all query (possibly made a query as a submodule, but as it is cross-project change, I would leave it for now - especially that this would impact couple contracts already using cw20_base.
Co-authored-by: Mauro Lacy <maurolacy@users.noreply.github.com>
Closes #27