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

fix: name display issues fixed #556

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bilinkis
Copy link
Contributor

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

@mizu-eth
Copy link

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
Copy link

mizu-eth commented Nov 19, 2022

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.

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 this pull request may close these issues.

2 participants