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

x/Staking: make validator info have an explicit link to profile pictures #9988

Closed
4 tasks
ValarDragon opened this issue Aug 23, 2021 · 7 comments · Fixed by #21315
Closed
4 tasks

x/Staking: make validator info have an explicit link to profile pictures #9988

ValarDragon opened this issue Aug 23, 2021 · 7 comments · Fixed by #21315

Comments

@ValarDragon
Copy link
Contributor

Problem

Historically, validator info was obtained indirectly, via validators having a link to a keybase in their identity field, and then explorers & wallets going to keybase to find their profile picture and displaying that.

This was making a bet on keybase becoming big, which ended up not panning out. (Even at launch, IIRC, getting cosmos validator profile pictures ended up being a large load on keybase servers)

Now explorers like mintscan maintain their own repository for storing profile pictures, that has no authentication to ensure it maps to what the on-chain validator wanted.

Proposal

We should make validator info field have an explicit field for profile_picture, which links to a png or jpg image of some max size, that explorers / wallets / etc. can all use in a unified manner.

We shouldn't be having images done indirectly via keybase imo, because the scale of cosmos >> the scale of keybase, and clearly this has failed at getting sufficient adoption.


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@alexanderbez
Copy link
Contributor

I'm not sure if I'd so far as to say that Keybase failed. I'd also be seriously shocked that explorers or clients actually overwhelm their servers.

That being said, I do think it's a bit weird that identity implicitly implies a reliance on Keybase. I don't really feel comfortable about adding profile_picture. Instead, what do you think about modifying the existing identity field to conform to a (TBD) format. E.g. <type>:<URI>.

e.g.

  • png:https://gravatar.com/avatar.png
  • keybase:B20C...

type can be completely open or we can limit to a supported set (png, jpg, keybase, etc..)

@ValarDragon
Copy link
Contributor Author

The explorer / client overhead was AFAIU an issue in like the first month of launch, though I could be totally wrong!

Hrm, why is profie_picture weird though. Basically everywhere people have public identities have profile pictures. I feel like it simplifies supporting profile pictures, since otherwise we have one of the following two situations:

  1. folks would need to implement getting profile pictures from many different providers
  2. or we make identity a list of <type>:<URI> objects, with one of them commonly being png:... / JPG:...

@alexanderbez
Copy link
Contributor

It just seems too limited. Like having <type>:<URI> seems way more flexible to me. We can support direct links, Keybase, Gravatar, etc...

@ValarDragon
Copy link
Contributor Author

Wait but keybase rn is specifically a profile, not just a link to the image.

I think identity and profile_picture are distinct roles. (identity could be a twitter profile, versus profile picture is just the png everyone PRs to every single explorer / wallet)

@tac0turtle
Copy link
Member

Adding a link to a profile picture makes sense. Having something resolve to only the png, jpeg or other format seems fine. From a front end perspective it could be weird with handling different sizes. TBH id prefer to move away from key base to something like the twitter api as an identity. Most people don't use key base and have to set one up for running validators.

@facundomedica facundomedica self-assigned this May 13, 2022
@tac0turtle
Copy link
Member

what about adding a metadata field that can be extended if needed on the fly instead of needing to update a struct each time.

@PikachuEXE
Copy link
Contributor

I imagine there would be more than one "identity" too
Though there can be some rules/checks for known types to prevent accidental invalid values, I think unknown types should be allowed too
And it's data reader's responsibility & right to choose what types to support

e.g.
identity: twitter:@Something,stargaze_name:plottablegoldentrain,icns:who.cosmos (Not sure what would be a good separator though. ,/;/|?

I am not so sure if profile picture should be fit into identity though. Current implementation seems to be just a side effect due to keybase profiles having one profile picture. Probably not every type of "profile" (known or unknown) support having a profile picture. But I am fine with just adding types for profile picture(ssss) if it's supported (by data readers).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 🥳 Done
Development

Successfully merging a pull request may close this issue.

7 participants