Closed NoahZinsmeister closed 3 years ago
This is being caused by an invalid UTF-8 sequence in the decoded data for the description
string field, as the error indicates: description: invalid codepoint at offset 2669; unexpected continuation byte
.
You can use the error recovery API to solve this, but I may also be able to tie the error recovery API into the parseLog and parseTransaction API, I'll have to look more into that as it may require a signature change in the response type.
For now, you can do something akin to:
result = iface.decodeEventLog(log.topics[0], log.data, log.topics)
descr = function() {
try {
return result[8];
} catch (error) {
// On error, you may decide a different replacement strategy than the default, "error" strategy
return ethers.utils.toUtf8String(error.error.value), ethers.utils.Utf8ErrorFuncs.replace);
}
})();
If you use this, you will get the following string (which has replaced all the invalid sequences with the UTF-8 replacement character):
"# Upgrade Governance Contract to Compound's Governor Bravo ## Previous Discussion: [Temperature Check](https://gov.uniswap.org/t/temperature-check-upgrade-governance-contract-to-governor-bravo/13610) | [Snapshot](https://snapshot.org/#/uniswap/proposal/QmScNLeajiF2hQh1z9DYqTFKGgrRhHwrHisV4ynmDEQwxH) [Consensus Check](https://gov.uniswap.org/t/consensus-check-upgrade-governance-contract-to-governor-bravo/13707) | [Snapshot](https://snapshot.org/#/uniswap/proposal/QmWbgzHJ8nK2TDaj6LF6BxAMPahy97dMmbbU5kRBw1QkXt) ## TL;DR: Upgrade Uniswap's current governance contract from Governor Alpha to Governor Bravo to improve governance upgradability and protocol safety. [On-Chain Proposal]() ## Summary and Motivation: *Co-written with [Getty Hill](https://twitter.com/getty_hill) (@Getty), [Eddy Lee ](https://twitter.com/yesimeddy) (@elee), [Yuan Han Li ](https://twitter.com/yuan_han_li) (@yuan-han-li), [John Wang ](https://twitter.com/j0hnwang) (@johnwang), and [Ali Khambati ](https://twitter.com/alikhambati1) (@alikhambati)* Governor Alpha, the current governance contract used, was a great start, but in light of Compound's and other protocols upgrade to Governor Bravo, Uniswap should migrate given Bravo's additional safety benefits and upgradability. 1. **Native upgradeability:** Under Governor Alpha, changes to governance parameters require deploying a new contract and completely migrating to a new address. This is particularly damaging to governance participation as it introduces downtime and asynchronicity. Many governance tools and custodians use factory contracts which point to a specific contract address, meaning parties must manually upgrade their infrastructure every time governance parameters are changed under Governor Alpha. This includes tools for creating autonomous proposals like [fish.vote ](https://www.fish.vote/); front-ends such as [Tally ](https://www.withtally.com/), [Sybil](https://sybil.org/#/delegates/uniswap), and [Boardroom ](https://app.boardroom.info/) which aggregate and display governance results for various protocols; and professional custodians which are used by large token holders, delegates, and community members. Enabling a static contract address that has proxy upgradability patterns is paramount for maximizing participation, and maintaining an open and secure governance process. 2. **Voting reason string:** Governor Bravo gives voters the opportunity to add free-form comments (text strings) along with their on-chain votes. Not only does this increase the transparency around the rationale people have behind their votes, but it also facilitates more in-depth and nuanced discussion. 3. **New ��abstain' voting option:** Governor Bravo enables voters to formally abstain rather than forcing them to choose between voting yes/no or not voting at all. This will give voters more flexibility and also increase transparency into delegate behavior. 4. **Proposal numbers won't reset:** Under Governor Alpha, any upgrades to the contract resets the proposal number schema. Notice that [��Proposal 0.4'](https://app.uniswap.org/#/vote/0/4) (which resulted in deployment and migration to a new Governor Alpha contract due to modifying the proposal submission threshold parameter) caused the following on-chain proposal from @HarvardLawBFI to be numbered [��Proposal 1.1' ](https://app.uniswap.org/#/vote/1/1). Under Governor Bravo, this would not be an issue and proposal numbers would be continuous because the contract would be maintained at a single address. 5. **Proposal Cancellation:** Bravo allows user-directed cancellations enabling erroneous proposals to be canceled if need be (rather than forcing people to vote no/abstain). 6. **Review Period:** Governor Bravo allows governance to include a review/analysis period. Currently, Compound uses a 13140 block (~2-day) review period that allows holders to review the proposal. This means that users will have 2-days to prepare for voting (e.g., remove UNI from Aave), unlike Governor Alpha which instantly snapshots users' voting power. A review period substantially improves the accessibility and safety of the governance process. ## Implementation: After speaking with the OpenZeppelin team about their reimplementation of Governor Bravo, we think using Compound's Governor Bravo contract instead makes the most sense at this stage. Although Open Zeppelin's Governor contract takes a simpler approach that can include all Bravo functionality, it does not do so by default. The contract uses similar logic, but the code is new. It has gone through an internal audit and review process but still needs to go through an external audit. For the safety and simplicity of Uniswap, we think using Compound's Governor Bravo contract is the best decision for the time being. Compound's Governor contract has already undergone an [audit](https://blog.openzeppelin.com/compound-governor-bravo-audit/) by OpenZeppelin and has been widely used. We can always change/upgrade to Open Zeppelin Governor contract at a future date if their contract introduces new features/functionality the community is interested in. More details on OpenZeppelin's code can be found here: [Github](https://github.com/OpenZeppelin/openzeppelin-contracts/tree/master/contracts/governance) [Docs](https://docs.openzeppelin.com/contracts/4.x/api/governance) [More info](https://openzeppelin.notion.site/Comparing-Compound-Governor-to-OpenZeppelin-Governor-2-10d0fdcf61ba476fae492b295822ee13) As mentioned in the previous Consensus Check, we have deployed the contract on the Ropsten test net for the community to review our code: [Governor Bravo Delegator](https://ropsten.etherscan.io/address/0x15df15caad12adaa03949014ba5cc49a84803d0f#code) [Governor Bravo Delegate](https://ropsten.etherscan.io/address/0xD8bf60dfC5115F6cB99bb50502346E7b863800f1#code) [Github for contracts](https://github.com/gettty/uniswap-gov) *NB: The `_initiate` function has been slightly modified to take an initial proposal number as an input rather than pulling it from Governor Alpha. This way Uniswap can resume proper proposal numbering.* ## Resource links: 1. [Governor Bravo Development - Protocol Development - Compound Community Forum ](https://www.comp.xyz/t/governor-bravo-development/942) 2. [Understanding Governor Bravo. A review of key changes versus�� | by monetsupply | Tally | Jul, 2021 | Medium ](https://medium.com/tally-blog/understanding-governor-bravo-69b06f1875da) 3. [Compound | Proposal Detail #42 ](https://compound.finance/governance/proposals/42) 4. [Compound | Proposal Detail #43](https://compound.finance/governance/proposals/43)"
You may also want to figure out why non-UTF-8 data is getting into this string, keep in mind that this can be an attack vector in certain circumstances, as using the ignore
or replace
strategies fold errors, which means hashes can be "tricked".
The parser is failing to parse two valid UTF-8 sequences: U+2018 (LEFT SINGLE QUOTATION MARK) and U+2026 (HORIZONTAL ELLIPSIS).
I think these two code points may have been entered in reverse in this proposal, so the parser may be fine. Worth someone else double-checking this. More specifically, the three codepoints making up these characters were entered in reverse order, so that the first one fails with UNEXPECTED_CONTINUE (because it is not a continuation codepoint).
Yeah, it is definitely invalid UTF-8 data.
I have extended the Recoverable API for the parseLog
, parseTransaction
and parseError
methods on Interface
in 5.4.6. You still need to try/catch them when accessing the offending property, but bad properties that are never accessed won't cause any exception.
I'm closing this now. This was an issue with the contract's return data, but the enhanced error recovery has been added to the parse*
variants of the methods, so this issue is simpler to capture.
Let me know if you have any more issues though!
Thanks! :)
This transaction breaks when using
parseLog
. Basic repro pasted below, present in at leastethers@5.4.5
.