Four different auditors were engaged for Liquidations 2.0, aka MIP45. These were:
- methodologies: manual code review, static analysis
- final report
- Trail of Bits
- methodologies: manual code review, property-based fuzzing, static analysis, other programmatic methods
- final report
- methodologies: manual code review
- final report
- methodologies: economic simulations of the new and old liquidation systems
- final report
This post from the Protocol Engineering Team will give an overview and discussion of the findings of each report. Please note that the following summary and the audits themselves are not fool-proof; rather, they attempt to review technical drawbacks and potential limitations with Liquidations 2.0. But they are limited. Thus, all users should heed the risks highlighted in the reports and the risks inherent in all experimental software design and use their independent judgment as to whether they are sufficiently sophisticated or financially capable of engaging with Liquidations 2.0 and the Maker Protocol.
Quantstamp’s final report contains 12 findings. Quantstamp classified the severity of these as: two (2) Medium, four (4) Low, five (5) Informational, and one (1) Undetermined. Two of these issues (QSP-3 and QSP-4) were addressed at the smart contract level; the rest are addressed via documentation (e.g. in MIP45 or code comments). The issues are listed below; for each, a brief summary is given, along with a justification for our chosen response action. More details can be found in the report itself.
This issue refers to the fact that
Clipper.vow() could differ in the deployed system. Similar possibilities in fact exist throughout the Maker protocol; the risk is very low as the components that could be mismatched are set only by governance and rigorous off-chain validation is done on any such action. On the other hand, constraining the configurations could actually be undesirable if the design changes in the future, or if a migration is needed. Thus, we acknowledge the possibility of a mismatch but don’t believe the risk is high enough to warrant addressing.
This issue raises the previously-known possibility that
tip, the parameters that determine the reward given to liquidators, could allow “incentive farming” if set to values that make it profitable to create Vaults for the express purpose of liquidating them. This is documented in MIP45, and it is expected that the Risk team will thoughtfully recommend values of
chip to governance that do not make incentive farming viable. Thus, we acknowledge the issue, but choose not to take action at the smart contract level to mitigate it.
In the version of the code shared with the auditors, if the parameter encoding the liquidation penalty (
chop) were set to zero, it would have caused liquidation attempts to revert. While the risk of this is very low (
chop is set only by governance), we decided to add parameter validation to mitigate this risk, as
chop has an inarguably sensible lower bound of
1 * WAD and should generally, in fact, be set higher to prevent auction grinding attacks.
If the oracle price used to set the initial price of an auction was zero, the auction would have been “stuck” until it reached its configured time-based endpoint. This was fixed by requiring that the starting price of an auction be non-zero.
This issue points out that the
file functions which allow governance or trusted modules to set parameters generally do not validate their arguments. Not doing input validation is actually the default across the MCD contracts, for a few reasons:
- Much more detailed validation of governance actions or trusted modules can, should be, and is done off-chain. Off-chain validation is also more gas-efficient.
- Overly strict input validation is itself a risk as it might limit our options in unforeseen scenarios when it is actually advantageous to set parameters outside of their expected ranges. Core contracts are difficult and risky to update, so we bias low-level components toward greater generality.
- Input validation can always be added easily later in the form of intermediary contracts that limit the access of governance to specific modules.
In rare cases we do add input validation (see QSP-3) if there are strong arguments in favor of doing so (in particular, high severity consequences and straightforward delineation between valid and invalid values). However, for the reasons outlined above, we prefer not to add input validation for the majority of parameters in Liquidations 2.0.
The various parameters that govern auction re-initialization and price evolution are subject to change even while an auction is running. No action was taken for this issue for the following reasons:
- The current auctions also have parameters that could change in-flight, and it hasn’t been a problem.
- Auction parameter updates are expected to be very infrequent.
- The auction interfaces provide features for bidders to protect themselves from some potential consequences of parameter changes, for example the ability to specify the maximum price they are willing to pay for collateral.
- This possibility is clearly documented in MIP45, and integrators, the Risk team, and governance are expected to be aware of this issue and to act accordingly to avoid negative outcomes.
- It is possible to construct governance actions in such a way that running auctions can be checked for prior to making changes.
- The suggested alternative (caching parameters on a per-auction basis) would greatly increase gas costs for auction participants and could have negative consequences of its own (i.e. in some scenarios it may be desirable to update the parameters of running auctions).
We may consider adding the capability for governance to force resets of active auctions in the future to partially mitigate this issue.
We may lock the compiler pragma in the future as makes sense, but currently there is no need to do so from a tooling perspective. This has no effect on the deployed contracts.
This issue notes that the authorization mechanisms in the core contracts do not conform to more widely used ones such as a per-contract
owner role. We prefer MCD’s
wards authorization pattern for its simplicity and flexibility. It is standard within Maker protocol code, well-understood throughout the Maker developer community, and covered in the MCD documentation, so we feel that no additional action is needed to address this issue.
It is possible for the number of authorized callers of a contract to be reduced to zero. This applies to all system contracts and hasn’t presented any practical issue. See also the comments under QSP-6 regarding input validation in governance functions. Note in particular that the ability to revoke all administrative access to a contract may in some cases be desirable.
tip parameter, a fixed DAI reward given to the successful liquidator (or resetter) of an auction, is intended to help defray the costs these actors incur given their uncertain success in bidding. This issue points out that in conditions of extreme congestion, a non-adaptive incentive may cease to adequately encourage desired behaviors. This is a known risk, and must be balanced with concerns around possible incentive farming, which
tip is especially prone to. On-chain incentives are viewed as experimental, particularly given concerns around front-running, and in the short-term we believe it’s best if Risk/governance directly manages setting this parameter. Assuming
tip proves useful, and an optimal algorithm can be settled on, the contracts can be upgraded, or an instant access module can be added to manage the
tip parameter. In particular, it makes sense to wait for EIP-1559 to potentially provide more stable gas pricing before investing significant resources in dynamic reward mechanism design.
This issue highlights that since oracle prices are delayed (to guard against oracle attacks), it’s possible for auctions to start at prices so far above the market price that the auction does not catch up to the market price before needing to be reset, and may eventually settle at a low enough price that bad debt is locked in. This a well-known and much-discussed possibility, covered in MIP45. This possibility must be guarded against choosing risk parameters well.
This issue notes that certain configuration (authorization of the
Dog to call
auth functions of a
Clipper and the need for
cut to be nonzero for nonzero prices to be returned by
StairstepExponentialDecrease) is necessary for correct functioning of Liquidations 2.0 post-deployment. Recommendations were made to authorize the
Dog in the
Clipper constructor and to set
cut to a non-zero value by default. We chose not to do the former because it could, in fact, be unsafe or have unpredictable consequences in some scenarios to blindly authorize a supplied
Dog address. Further, it is a pattern of Maker contracts that constructors only authorize
msg.sender; adhering to this pattern makes the code as a whole easier and safer to reason about and deploy. For the latter, there is not a clear “safe” default, and governance is required to set this parameter in real scenarios anyway. Some points made in the response to QSP-5 regarding input validation apply here as well. A comment was added to the
StairstepExponentialDecrease contract to document the need to set
step, and the importance of setting
cut to a non-zero value was emphasized in MIP45.
The final report from Trail of Bits contains nine (9) findings. Trail of Bits classified the severity of these as: three (3) Medium, three (3) Low, two (2) Informational, and one (1) Undetermined. The issues are listed below; for each, a brief summary is given, along with a justification for our chosen response action. More details can be found in the report itself.
This issue was related to the observation made by QSP-3, although slightly more general (noting that just very low values of
chop are sufficient to block liquidations). The lower bound validation added for
chop (see response to QSP-3) fully mitigates this concern.
Vault ownership is based on Ethereum account address (and collateral type); due to a check in
Clipper.kick, a Vault owned by the zero address cannot be liquidated. This issue does not need to be addressed for the following reasons:
- Only addresses authorized against the
Vatcontract (i.e. governance or a privileged module) could possibly create debt at the zero address. Thus a liquidatable Vault belonging to
0x0is unlikely to arise, and if it does, it will most likely be in a controlled, intentional context.
- If there is an undercollateralized Vault belonging to
0x0due to the edge cases mentioned above, governance can use the
grabAPI to manually liquidate the Vault, or create a special liquidation module calling that API.
- Ordinary users can encumber collateral at the
0x0address, but a) this poses no risk to the system, b) it does not create debt and thus does cannot create and undercollateralized Vault, and c) governance can intervene via
grabto extract the encumbered collateral if needed.
Since the collateral disbursed in a purchase is sometimes adjusted due to the maximum DAI target being reached or dust considerations, it is possible for rounding error from dividing a DAI quantity by the auction’s price to result in zero collateral disbursed despite a nonzero DAI payment. No modifications were made based on this for the initial launch of Liquidations 2.0 for the following reasons:
- It is extremely unlikely that this edge case occurs in practice, in particular, but not only, because DAI quantities have 45 digits of precision whereas prices only have 27.
- Bidders are generally assumed to be doing profit evaluation prior to bidding.
- Bidders can permissionlessly bid through a proxy contract to add a check on collateral received for only a slight increase in gas costs.
We will consider addressing this possibility in future upgrades of the liquidation system if it makes sense to do so.
This issue notes that since anyone can call
Clipper.redo and receive an incentive for starting or restarting an auction, frontrunners are likely to compete to make these calls against more dedicated Keepers. The priority gas auctions that result from these zero-sum dynamics may effectively negate the incentive, with only miners profiting. While it is not bad that miners earn from securing the network, the asymmetric effort between identifying a liquidation or restart opportunity (which requires extra work beyond simply running a node), and frontrunning or otherwise taking advantage of an opportunity found by someone else, could discourage anyone from doing the identification work (as it is strictly less profitable to do so). This could result in a fragile ecosystem with few or no participants proactively triggering liquidations.
This is a well-known and much-discussed concern. After launch, we’ll closely observe what outcomes occur “in the wild” once the auctions are live; it’s possible that the use of the incentives proves unnecessary, or perhaps they’ll be relatively uninteresting to frontrunners due to higher-value opportunities elsewhere. If zero-sum dynamics are observed to negatively impact the Keeper ecosystem, e.g. by discouraging participation as mentioned, stopgap measures for the DAO to consider include using out-of-band mechanisms to compensate Keepers for protecting the protocol (e.g. directly paying select participants for triggering liquidations). Longer-term, we will continue our research into improvements on the current mechanism. Small changes such as adding a delay to reward claimability, or making liquidations and restarts two-step (with a commit-reveal pattern), are already under consideration for a near-future upgrade. Larger game-theoretic changes such as giving Keepers with certain stakes in the system (e.g. governance-locked MKR or large Vault positions of their own) strategic advantages in liquidations are also being discussed, and non-zero-sum liquidation mechanisms like those pioneered by projects such as B.Protocol and KeeperDAO are being actively researched as well.
The full implications of miner-extractable value (MEV) are still being worked out by the Ethereum community. We’re staying up-to-date on all developments, and will be ready to evolve the protocol as necessary to ensure it achieves its design goals. Currently, we believe the greater liquidity available to a Dutch auction mechanism due to composability outweighs the potential negative impacts of MEV for the reliability of the liquidation system.
See discussion under QSP-6, which is an identical observation.
Race conditions between calls to
take can result in bidders purchasing less collateral than they expect, with no lower bound, potentially losing money due to gas costs. It is impossible to fully mitigate this (as even a reverted transaction will consume gas), but similar to ToB-3, bidders can use a permissionless proxy contract to cancel their purchase if the amount of collateral received is lower than what they are willing to accept. We may consider adding a lower limit for collateral purchased as an argument to
take in future upgrades of the liquidation system.
Refer to discussion under QSP-5, which is essentially the same issue. A consistency check was added in the
Dog to ensure
clip addresses are filed for the correct
During development, compiler optimizations were briefly turned on as we wanted to investigate potential gas savings. This issue points out that many Solidity compiler bugs have been found in the optimizer. After assessing the gas savings of compiling with optimizations to be at best a few percent, we decided that compiling with optimizations was not worth the increased risk (given that we haven’t had time to formally verify Liquidations 2.0 prior to launch), and disabled the optimizer for the initial deployment.
This finding highlights a known area of technical debt for the team. We plan to improve the validation in the deployment scripts long-term. It is worth noting that these scripts are only used to fully deploy the system from scratch, e.g. on testchains or fresh mainnet deployments. Enabling MIP45 on mainnet will be done via an executive spell, which does not involve these deployment scripts, and will be extensively validated.
ChainSecurity’s final report contains 10 findings. ChainSecurity classified the severity of these as: four (4) Medium and six (6) Low. The issues are listed below; for each, a brief summary is given, along with a justification for our chosen response action. Their report also included a number of notes–relevant considerations which are not necessarily issues–to which we will also give responses here. More details can be found in the report itself.
This issue pointed out that we forgot to update the comments describing the
bark function after making a change to the code. We fixed the comment, resolving the issue. This might seem minor, but keeping comments and other documentation congruent with the code they describe is actually quite critical–inaccurate documentation could cause confusion and delays, and in the worst case could lead to costly mistakes.
This issue was a bug in an edge case of the auction code: bidding always reduced
ilk.dirt by the DAI actually collected. However, if all collateral from an auction were purchased without covering its target amount of DAI (e.g. because the price has declined too severely), the difference between the collected and target amounts would remain in
ilk.dirt, building up over time. While governance could have acted to correct this, it could potentially have artificially slowed the pace of liquidations in critical moments. This bug has been fixed in the code–when all collateral is removed from an auction, the full remaining DAI target is subtracted from
This was probably the most serious and interesting issue uncovered in the course of the audits. After an Emergency Shutdown, it was possible to corrupt various aspects of the running auctions via reentrancy. This would have been done by calling
take on an active auction, passing instructions to call an external contract that would call the
End.snip function on the same auction passed to
End.snip in turn calls
Clipper.yank, which cancels the auction. The main negative effect was that some remaining auctions would have been unable to fully complete or be canceled, which would translate further into accounting inaccuracies in the global settlement process. This issue was fixed by adding a reentrancy guard to the
There were initially a number of inconsistencies between MIP45 and the code itself. These issues have been resolved by correcting and clarifying the text of MIP45.
Changes made in response to other issues were not initially reflected in MIP45; this has been corrected.
dust value for the relevant
ilk was read from the Vat in
Clipper.take. Since this value is a member of a struct and the Vat does not expose an individual getter function, this required reading all values from the struct, a very gas-inefficient operation. Thus, we added a caching mechanism to the Clipper for
dust, so that the stored value only needs to be (permissionlessly) updated upon a parameter change.
The Clipper initially contained two functions for accessing the value at a particular index of the active auction array. One of these was removed to reduce code size.
This issue noted that when removing the last auction in the active list, more storage operations were performed than necessary, increasing costs. It was suggested that we check for this case and avoid these operations. While this change slightly increases gas costs when any auction besides the last one is removed, the savings in the last-removal case was deemed large enough to justify its implementation.
In certain edge cases of the original code, the liquidation of dusty Vaults (Vaults with total debt less than
Vat.ilk.dust) would have been blocked, even though there was technically room for them. The liquidation logic was re-architected and now dusty Vaults can be liquidated as long as they do not cause
Dirt to exceed
ilk.dirt to exceed
This issue found that a check being done in the code was redundant as it was covered equally well by another check being done. This statement was removed in the process of addressing CS-9.
The external call made from the Clipper carries the risk of allowing funds controlled by the Clipper to be moved in an inappropriate way. For this reason, calls to certain other system contracts are blocked. However, in the future, new functionality could be added to the system that creates a vulnerability if there is a function selector collision. We intend to automate the process of checking functions of new contracts added for such collisions.
ilk.dirt is close to
Dirt is close to
Hole), an attacker could cheaply create many small auctions. This is done by, within a single transaction, repeatedly purchasing a small amount of collateral from an existing auction, and then partially liquidating a Vault. EIP-2929 (implemented in the Berlin hard fork) makes this even cheaper to perform. This could potentially be used for incentive farming, especially if
tip is set too large relative to
dust and gas costs. While incentive farming is a known risk of MIP45, this method is noteworthy as being potentially more cost-effective than others.
A discussion of this possibility has been added to MIP45.
This note observes that there is no synchronization between the time it takes bad debt recorded from liquidations to “mature” into a form that is eligible for cancelation, and the collection of DAI from auctions. This is a known behavior of the system, and well-managed through appropriate tuning of risk parameters such as
Vow.wait and those that control the duration of auctions.
This note observes the fact that transactions aiming to collect the
redo incentive can be front-run, potentially discouraging Keeper participation. It is also possible that certain calls to
take may be front-runnable as well (especially if they rely entirely on DEXes or flash loans for liquidity). A detailed discussion of such concerns is undertaken in the response to ToB-4.
See QSP-2 and CS-Note-2.
This is an observation that non-trivial initialization and authorization procedures are necessary to integrate the new contracts into the system. Such actions are done in executive spells that undergo extremely thorough review and testing, but we hope to improve on this process in the future. In particular, we’d like to start earlier on codifying and testing deployment logic so that it can be covered by audits as well as the core contracts themselves.
Related issues include QSP-1, QSP-12, and ToB-9.
See discussion under QSP-6 (also equivalent to ToB-5).
Since stability fees are not collected after a liquidation, it’s theoretically possible for the proceeds of an auction to fail to offset the fees that would have been collected during the time the auction was running. Not only are very unrealistic values required to achieve this scenario (high fees and long auctions), in most cases the “fees” would actually be bad debt as the worth of the collateral would be less than the debt owed. For practical values of fees (0 to 50%) and practical auction durations (a few hours to days), this is mathematically impossible.
Debt accounting is indeed asynchronous in the Maker protocol; this is by design and allows for great modularity and flexibility in the components of the system, as well as better gas efficiency (e.g. debt cancellation from many auctions can be achieved in a single call to
Vow.heal). While this is sometimes a challenging and counterintuitive concept for newcomers to grasp, the aforementioned benefits more than justify the design.
Gauntlet performed a very different type of audit compared with the other three–economic simulation. Their modeling combined a few different pieces:
- a private, customized Ethereum testnet with a deployment of MCD
- agent bots for various system actors, in particular auction Keepers with different bidding strategies
- price models for ETH and DAI
- a simulated external exchange that the agents could trade on
- a mempool model to allow simulation of stressed network conditions.
This audit tackled two broad tasks: comparing Liquidations 1.2 and 2.0, and examining optimal parameter choices for 2.0. For the former, it was found that: “Liquidations 2.0 system outperforms Liquidations 1.2 in every unambiguous metric of throughput and collateralization risk” (see here). This was the intention of the Liquidations 2.0 design, so this was a satisfying result. For the latter (investigating optimal parameter choices for 2.0), we refer readers to the report for full details, and to the Risk team’s forum post on initial
LINK-A parameters, which used the Gauntlet report to inform parameter choices alongside the Risk team’s complementary simulations and considerations.