Proof-Of-Humanity / proof-of-humanity-web

The Proof Of Humanity web application.
https://app.proofofhumanity.id
MIT License
81 stars 40 forks source link

fix: name display issues fixed #556

Open bilinkis opened 1 year ago

bilinkis commented 1 year ago

Added a check to display the submission's name in case there are issues with the evidence file

mizu-eth commented 1 year ago

This should not be merged IMO. It's an important requirement that each user's names be present in their JSON file, and if it's not present (i.e. the file is invalid), the UI should not try to hide that fact. As long as the registration UI is not bugged, I don't see what purpose this serves. As to why I think this is important: consistency for a start, but also, having the name in the JSON makes it possible to parse the proof of humanity registry using EVM events (which can be retrieved through the getLogs API call) and IPFS only. Although the name is stored on chain as call data, is isn't as easily accessible through the ethereum API and might not be available depending on an ethereum node's configuration. The subgraph is a point of centralisation in practice and its use should not be a requirement.

mizu-eth commented 1 year ago

This PR would also make the PoH registry harder to parse for third parties since they would then have to account for the exception introduced herein. And it's also pretty weird that an exception is being made for just the name field and not any other field, which could also be missing or mangled due to a bug in the registration UI.

By the way, there is a challenge in progress regarding this issue which I am involved in, but whether this PR is merged or not won't have any effect on the current challenge since the jury is instructed to take into account the state of the world at the time that the case starts. So this isn't about winning my case but genuine concern.