[DUX] Post Mortem: Poll Vote Calculation Discrepancy

Summary

An issue was found with regard to how the Governance Portal calculates poll voting weight. We discovered 21 polls which have outcomes that were changed, all of which are “greenlight” polls. Other polls had MKR weight balances that were changed, but it did not affect the vote outcome. While researching this issue, we uncovered other errors, detailed below. The DUX team has taken multiple steps to mitigate the issue and prevent similar ones from occurring in the future.

Background

On August 8th 2021, the TokenFlow team contacted the DUX team about a discrepancy in how their MCD Voting Tracker tool (https://beta.mcdgov.info/) was calculating poll votes and how it is done in the Governance Portal (https://vote.makerdao.com/). The Governance Portal uses a custom database and blockchain scraper called the gov-polling-db (https://github.com/makerdao/gov-polling-db) to listen for events on the blockchain and record them to a database. This data is parsed and then used to display poll data in the UI.

As we researched the cause of this discrepancy, we uncovered three erroneous calculations being used in the governance polling database to generate the polling results displayed in the Governance Portal.

  1. If a user has locked MKR directly into the Chief contract by any address which is either a “hot” or “cold” wallet linked to a vote proxy, that wallet’s Chief balance would not be counted towards their voting weight.
  2. The query used by the Governance Portal would double count MKR in an address’ wallet if it was the “hot” or “cold” wallet of a vote proxy that was linked to itself (where the “hot” and “cold” wallets are the same address).
  3. When showing poll results of historical polls, the voting weight of users who created a vote proxy after the poll ended were not included. Fortunately, this bug only affected how the UI displayed historical voting data, it did not affect the outcome of any polls.

We thank the TokenFlow team for flagging the discrepancy and participating in the discussion that ensued, which helped us discover these issues. See the appendix below for a spreadsheet analysis of the polls affected by these issues.

Detailed Analysis

1. ‘Chief balance issue’

This issue only affects polls which ended after the Chief v1.2 deployment on November 25, 2020. When the polling system was first introduced, a user’s hot and cold wallet Chief balances were counted towards their poll voting weight. When the new Chief contract (v1.2) was deployed, the Maker Foundation team managing the Governance Portal took the opportunity to optimize the calculation query for polling results. At that time they decided to discontinue including the hot and cold wallet balances—in part because the V2 Governance Portal UI does not allow users to deposit into chief directly if they have a vote proxy, and in part to improve the efficiency of the query. However, the Governance Portal UI does allow users to deposit directly into chief v1.2 if they have a v1.1 vote proxy, and the polling database accounts for both v1.1 and v1.2 vote proxies. This fact was overlooked by the foundation team when they made the change in the calculation logic.

Of all the different poll types, only greenlight polls have changed outcomes. The following is breakdown of how the issue affected each poll type.

Greenlight Polls:

  • 68 total polls
  • 21 outcomes changed

Because greenlight polls are calculated with a score based on “yay” and “nay” votes, any changes to the poll breakdown cause a change to the outcome. We are working with the Gov Alpha team to move forward with these changes.

For a more detailed breakdown on which greenlight polls were changed, see the spreadsheet in the appendix below.

Inclusion Polls:

  • 30 Total polls
  • 0 polls with majority outcome change

Some inclusion polls had changes to the vote tally, but no change to the majority outcome, including the 3K MKR minimum threshold.

Ratification Polls:

  • 17 Total polls
  • 0 polls with majority outcome change

Like the inclusion polls, some ratification polls had changes to the vote tally, but no change to the majority outcome, including the 10K MKR minimum threshold.

Weekly Polls:

  • 119 Total polls
  • 0 polls with majority outcome change

Once again, these polls had changes to the vote tally, but no change to the majority outcome.

Ranked-Choice Polls:

  • 0 polls with outcome change

See point #3 below for more information on ranked choice polls.

This issue was fixed by updating our query to check for balances in chief of either the hot or cold wallet address of the vote proxy user.

2. Double-Counting Bug

If the “hot” and “cold” wallet addresses of a vote proxy were the same, their wallet weight would be double-counted. Fortunately, none of the polls have a changed outcome as a result of fixing this bug.

Breakdown:

  • 380 total polls investigated
  • 0 polls with an outcome change

The fix for this bug was to add a check in our query making sure that the hot and cold wallets are not the same before calculating the wallet amount.

3. Historical Display Issue

This issue did not affect any polling outcomes, it only affected what was displayed to a user when viewing the poll detail page after a poll ended. Two ranked-choice polls would have displayed an incorrect winning option prior to fixing this issue. The fix that was applied is to supply a block id to the query that retrieves this data equal to the block id when the poll ended. This check ensures that the correct vote proxy is used for display.

Bug Discoverability

The consequences of the code changes we’ve outlined in the first section were overlooked by the foundation team working on the Governance Portal at the time and were not caught by their QA process.

In addition, the fact that these issues were never flagged by users of the Governance Portal implies that the verifiability of Maker’s polling process needs improvement.

  1. The Governance Portal UI did not allow for easy verification of the user’s voting power being accurately registered, which can be improved by adding more data visualization in the UI.
  2. The governance polling database repository was not made open-source. This is an important step toward enhancing transparency and improving the verifiability of the data presented in the UI.
  3. This incident also serves as a reminder that Maker’s governance polling process is largely an off-chain governance process—even though the votes leverage on-chain data (eg. MKR token balances of users), the calculations and follow-up to the polls all occur off-chain.

Mitigation Steps: Short-term

  • All of the aforementioned issues with the poll voting weight calculation have been fixed. The DUX team has conferred with Gov Alpha and confirmed that we want to re-introduce including hot and cold chief balances into the calculation for polling votes.
  • We have generated comparisons of the data which present the outcomes of polls affected by the bug and provided this to Gov Alpha, as well as included them here in the appendix. Gov Alpha will follow up on how to move forward with the greenlight polls of which the outcomes were changed due to the calculation issues.
  • We will open-source the Gov Polling Database repository—the aforementioned custom database and blockchain scraper used by the Governance Portal. In service to one of our three governing principles, transparency, we feel that this data should be open for anyone to view and have access to. Users will now be able to locally sync and run the same database and ETL tool that we use in production.
  • We have documented how gov-polling-db is used by the Governance Portal to calculate poll results and added a new README to the repo itself.
  • In order to improve the verifiability of the governance process, the next release of the Governance Portal will include a more detailed visual representation of the vote breakdown on the poll detail pages. This will make it easier for voters to verify that their MKR weight is being counted correctly by displaying each address that voted and their MKR weight.

Mitigation Steps: Long-term

As this incident has taught us, the development and maintenance of reliable tooling for Maker governance requires focused attention and a thorough QA process. As part of the handover from the Foundation, the DUX team has inherited several systems and repositories from the former JavaScript Product team—a team that was supporting a whole range of products across the Maker ecosystem. Because the DUX team has defined Maker governance UX as our sole business domain (as described in our MIP) we believe our core unit is afforded a sharper focus on the related systems which will hopefully prevent similar mistakes from being made in the future.

In addition to the above, we will holistically review our entire QA process and incorporate tests related to business logic, since this particular issue would not have been detected by our current process should we have pushed the query change that caused the described issues. We will share our findings and improvements soon.

Lastly, we will explore how to further improve the verifiability of data in Maker’s governance tooling with the goal of enabling anyone to confirm the data presented in the UI is correct. This could include providing relevant external links to 3rd party sites like Etherscan and EthTx, as well as exploring more nuanced ways to present complex calculations like voting weight. We will prioritize related work in our upcoming public roadmap.

Appendix

  1. Poll voting weight calculation bug: Impact analysis Spreadsheet

    https://docs.google.com/spreadsheets/d/e/2PACX-1vQ8DD6mqojQXvUDDm69oX1NMPXellhC733xqmv09iSFwE9ZIejoEfSr9ftX7FEauDvLmi3F71pSvoC_/pubhtml

    Contains five sheets:

    1. All polls post-chief1.2
      1. Every poll affected by the chief bug, their old & new weights, and if their outcome changed.
    2. Affected ranked choice polls
      1. Identifies the two ranked choice polls affected by the historical display issue.
    3. Greenlight Polls changed post-Chief v1.2
      1. Shows how each of the affected greenlight polls were changed.
    4. Affected polls pre-Chief1.2
      1. Presents the polls affected by the double counting bug pre-chief 1.2.
    5. All Polls Titled
      1. A reference for every poll researched.
  2. TokenFlow Analysis of Polling Weight
    https://forum.makerdao.com/t/polling-results-algorithm/10177

  3. How to Calculate Polling Results
    https://dux.makerdao.network/how-to-calculate-polling-results

13 Likes

Couple of notes from us here.

The greenlight scores within the collateral prioritization sheet will be corrected shortly. This may alter the prioritization of a number of collateral types. The Core Units will continue to take into account this prioritization order as much as possible in their efforts going forwards. It should be noted though, that recently the prioritization has been heavily skewed towards providing as much DAI supply as possible - so this may not result in immediate changes.

We’re lucky that there were no significant changes to more critical poll types, such as MIP Ratification or Inclusion polls. The results would have been harder to rectify if that was the case.

We remain confident in DUX to develop and improve the voting portal in the future - though we will push for the use of before-after regression tests in the event of future modifications to the portal’s business logic.

We would also like to pursue the development of the result-counting reference implementation created by TokenFlow such that it can be used to contrast and confirm the correctness of future changes.

Thanks to both DUX and TokenFlow for their efforts over the past weeks to analyze and resolve the discovered issues.

9 Likes

Good job by the team digging through all the details and coming up with this thorough review and taking action to prevent similar mistakes in the future. And thanks to the TokenFlow team to bringing the issue to light.

@SES-Core-Unit, including myself, provided some feedback to the team before the post-mortem was published, and we support the analysis and conclusions. There are however two elements that I want to highlight in particular:

1/ Smaller, specialized core units are the way to go

Looking at this issue and others that happened in the past, it is clear that it is the right choice to have Core Units with a narrow focus, more than was typically the case for the teams in the Foundation. While more was done with fewer resources, the thinly spread resources also caused quality issues like the one we’re witnessing here.

The DUX Core Unit focusing specifically on the Governance UX, and the Sidestream Core Unit (currently in the SES incubator) focusing specifically on Auctions services and UIs are two examples that should positively influence the quality and correctness of the MakerDAO tooling that we’re building.

Do one thing and do it well, this should the working model that should yield particularly good results in a decentralized organization.

2/ Transparency and verifiability are key

The issue that was detected illustrates once again that we need to prioritize openness and transparency, but also that transparency alone is not enough.

It needs to be practical and easy to verify data that is presented in the user interfaces that we build. I hope that it will become a standard practice and expected UX pattern to always link to the original blockchain data that readily confirms the information in the UI.

Verifiability reduces the level of trust that the user needs to put in the UI implementation. This is maybe the most challenging issue with decentralized UIs.

14 Likes

These have been updated with the corrected values.

6 Likes