Skip to content
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

Add a logo for the Issued tokens #27

Closed
alexia-ki opened this issue Jul 26, 2021 · 9 comments · Fixed by #38
Closed

Add a logo for the Issued tokens #27

alexia-ki opened this issue Jul 26, 2021 · 9 comments · Fixed by #38
Assignees
Milestone

Comments

@alexia-ki
Copy link

alexia-ki commented Jul 26, 2021

@ethanfrey @ks-victor As was discussed before, we need a way to support the SVG logo file of the digital assets issued.

Design is here
https://www.figma.com/file/8PU2KrQ9J3lnzcFysQ3pc7/desktop-light?node-id=2522%3A1568

@ethanfrey
Copy link
Contributor

This is nice, and we discussed storing an SVG with size limit (5 KB?) on chain and serving it to any client. Thank you Alexia for making a tracking issue.

How urgent/important is this?
Also, do we want this on the non-whitelisted token as well as the whitelisted token?

@alexia-ki
Copy link
Author

@ethanfrey from design point of view user can add any token/pair that he hold to the whitelist. We need to support logo file for new issued token and any other token also. And yes, in the whitelist user needs to see a short name of token with logo

@alexia-ki
Copy link
Author

In my opinion it is important, because now we are expecting to test the interface with users to find hard for understanding places. Usually more pictures means less explanation text. @ethanfrey

@ethanfrey
Copy link
Contributor

What timeframe are you expecting to start user tests? Would like to make API changes.

I ask as we have a 0.0.x branch for changes to the contracts that work with the current tgrade-internal-1 testnet, but the main development is targeting the upcoming wasmd 0.18 release (testnet likely mid-August). Giving a timeframe when this is useful / needed lets me plan if this needs to be "back ported" to the currently deployed contracts or can work with our current development flow.

@ethanfrey
Copy link
Contributor

Technical definition - for dso-token:

  • Add logo arguments to InstantiateMsg
pub struct InstantiateMsg {
  // ...
  pub metadata: Option<Metadata>
}

pub struct Metadata {
  // this must be a SVG file, maximum 5KB. 
  pub logo: Option<String>,
  // address of an account who can modify the metadata
  pub maintainer: Option<String>,
}

We should validate maintainer is a valid Addr. We should also do minimal validation of the logo file to see it matches the spec: https://en.wikipedia.org/wiki/Scalable_Vector_Graphics. One suggestion to ensure it starts with <?xml and contains <svg followed by </svg> somewhere in the code

  • Add query to get this data QueryMsg::Metadata{} returning:
pub struct MetadataResponse {
  pub metadata: Option<Metadata>
}
  • Add method to update the logo (only callable by maintainer if set): ExecuteMsg::SetLogo{logo: Option<String>}

@alexia-ki
Copy link
Author

@ethanfrey As I know, we are expecting to start testing after @ks-victor is back. Means not earlier than mid of August

@ethanfrey
Copy link
Contributor

Please do not start this task yet.
I will make a clear definition tonight.

@ethanfrey
Copy link
Contributor

Moving the bulk of this work to cosmwasm-plus

I'd be happy for a review of CosmWasm/cw-plus#370 where I define the new API. Once that is approved, we will add this logo and other metadata to cw20-base (default permission less token) in CosmWasm/cw-plus#371

Once those are complete, we can close this issue by exposing those new logo/marketing apis in the dso-token contract (just passing through to the cw20-base implementation)

@ethanfrey ethanfrey added this to the v0.1.0 milestone Aug 3, 2021
@ethanfrey
Copy link
Contributor

With CosmWasm/cw-plus#375 merged and cosmwasm-plus v0.8.0-rc2 cut, you just need to upgrade dependencies (#31 and #32) and integrate this into dso-token. No whitelisting is needed on the marketing account that can set the information, so these can just pass through to the root cw20-base implementation.

Should be an easy win.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants