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(governance): special case bravo parsing #2247

Merged
merged 6 commits into from
Aug 31, 2021
Merged

Conversation

zzmp
Copy link
Contributor

@zzmp zzmp commented Aug 27, 2021

Replace invalid UTF-8 characters in proposal descriptions with the replacement character to avoid crashes on future proposals with invalid UTF-8.

This is motivated by the Bravo proposal, which (1) has invalid characters, as U-2018 and U-2026 codepoints are reversed; and (2) omits newlines, preventing correct markdown parsing. This PR fixes the proposal programmatically instead of using a static constant (see e1f37b0) so that the data still originates on-chain, using guidance from ethers-io/ethers.js#1894.

@vercel
Copy link

vercel bot commented Aug 27, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/uniswap/uniswap-interface/6RyXkMFB5eMhvrwjasgbx756yYm6
✅ Preview: https://uniswap-interface-git-bravo-parse-uniswap.vercel.app

@zzmp
Copy link
Contributor Author

zzmp commented Aug 27, 2021

I think that U+2018 and U+2026 codepoints were entered in reverse, and that that's why the parser fails. It seems to have failed parsing in https://www.withtally.com/governance/uniswap/proposal/8 as well (looking at the original description), and been updated for the displayed description.

Do we know if/how this may have happened?

Updates parseLog to delay parsing nested properties, so that they will only fail when accessed.
Copy link
Contributor

@NoahZinsmeister NoahZinsmeister left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks great, thanks @zzmp :)

@NoahZinsmeister NoahZinsmeister merged commit 9e8d08e into main Aug 31, 2021
@NoahZinsmeister NoahZinsmeister deleted the bravo-parse branch August 31, 2021 14:47
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