Proposal: Upgrading the Flopper

Introduction

After the Multi Collateral Dai (MCD) contracts were deployed to the Ethereum mainnet on November 15th, a minor bug was discovered in the debt auction contract, also known as the Flopper. The discovered bug will only affect the system if debt auctions are active during Emergency Shutdown, in which case it prevents their cancellation. It was felt that this issue did not constitute an impediment to activation of the MCD system for a few reasons :

  1. The issue with MKR authorization and debt auctions discussed in the MKR Authority Transfer post was a more serious concern that also did not block activation.
  2. Governance parameters were already in the process of being chosen to make debt auctions unlikely until the MKR authorization issue could be resolved, making it very unlikely that this bug would be encountered (since it further requires Emergency Shutdown).
  3. In a worst-case scenario, the bug would not block Emergency Shutdown from occurring but would change its execution behavior.

However, now that the system is active, it is important to patch the bug promptly. The patch will require a new Flopper contract to be deployed and voted into the system via the governance process.

The Bug

The specific issue comes from the yank(uint id) function of the Flopper. This function is supposed to refund the last bidder on the auction specified by id during Emergency Shutdown since new bids (via dent ) and auction finalization (via deal ) are disabled after Shutdown. In the version with the bug, the Vat’s move function is used to attempt to transfer (internal) Dai from the Flopper to the last bidder (line 153). The issue with this is that during the bidding process, the Flopper never receives any Dai! The first bid goes directly to the Vow’s balance, and subsequent bids just refund the previous bidder. Thus, if this function were to be called in a real-world scenario, it would revert due to an underflow check in move .

Note: For this section, refer to this version of the Flopper, which is a “pre-fix”, so that the bug can still be seen. Also, recall that a debt (or “flop”) auction prints MKR to cover unbacked Dai—for a fixed amount of Dai, bidders compete to see who will accept the least quantity of MKR in return. Thus, bidders pay in Dai, and the winner receives MKR, with all but the final bidder receiving a refund.

Impact of the Bug

Overall, the discovered bug has no effect on the system during normal operation. There is also no effect if no active (i.e. bid-on) debt auctions exist when Emergency Shutdown is triggered. The direct impact of this bug will only occur if Emergency Shutdown is triggered, resulting in the active debt auctions of the system becoming unable to be canceled.

This leads to two indirect impacts:

  • The last Auction Keeper to bid on an outstanding debt auction loses its Dai bid, receiving nothing in return as Shutdown freezes both the ability to bid on and the finalizing of debt auctions (as auction cancellation is intended to refund bids in this situation).
  • Since the un-refunded bids are absorbed into the system surplus, the total debt is decreased, and thus the quantity of each collateral type awarded per Dai is increased (at the expense of the debt auction Keepers).

The Patch

Multiple technical solutions were considered. They were judged based on the following criteria:

  • Minimizing changes to the debt auction mechanism—many of our partners have tested and integrated with this mechanism, and changing it could be disruptive or accidentally introduce other bugs.
  • Replacing as few contracts as possible—without a compelling need to change more than one contract, this minimized technical complexity and risk.
  • Harmony with the design and intent of the rest of the system.

Explanation of the Patch

The chosen solution is to change the move call to a suck call in yank . With the appropriate choice of parameters, this credits Dai to the last bidder as a refund while the Vow’s sin balance (i.e. unbacked debt) is increased to offset the refund. This solution does require the Flopper to be authorized to call protected functions of the Vat ( suck requires authorization). However, it allows us to keep the auction mechanism as close to the version that has been tested on testnets leading up to launch.

The commit that applies the fix to the main MCD repository can be found here.

Steps to Integrate the New Flopper

  1. Deploy the fixed Flopper contract to mainnet.
  2. The deployer uses the Flopper’s rely function to grant calling permissions to the Vow and Governance Pause Proxy; the deployer then calls deny to deauthorize itself.
  3. The new Flopper is given permission to mint MKR via the MkrAuthority (this will be done as part of the MKR Authority Transfer)
  4. An executive vote must be held to elect a spell that will call the Vat’s rely method to give the new Flopper contract permission to call suck and will call file on the Vow to set the new flopper address value.

Looking Back

How did this bug make it into the mainnet deployment? A few factors can be identified:

  1. Formal Verification (FV) is not a perfect solution. In this case, it did not catch this bug because the function-level specs were written in such a way that it mirrored the bug in the source code. A higher level multi-transaction FV that would catch such a bug even with function-level errors is still a work in progress.
  2. This bug exists only when two rare scenarios take place at the same time.
  3. It is extremely rare in software development to have a completely bug-free system. This is why the MakerDAO system was designed to be modular and upgradeable and why we will be working with Governance to roll out initiatives to secure and improve the system in the future.

We will also be introducing our MIPs program in the near future which will introduce a simple, standard approach for Maker stakeholders and our community members to propose new improvements, standard specifications, or process changes for the Maker Protocol.

Next Steps

  • Executive vote for Flopper patch.
  • In terms of the timing of this patch, it is required that it is done when the Flopper is enabled.
  • The Maker Foundation will inform integration partners that there are no backward-compatible issues.
5 Likes