Bug in 2020/08/07 Executive Spell Contract

Unfortunately, a minor bug in the 2020/08/07 spell contract resulted in the WBTC-A debt ceiling being reduced to approxmiately zero DAI instead of being increased to 80 million DAI which was the original intent.

@Mariano_Conti’s description of the issue can be found here.


No funds are at risk, this bug just prevents further WBTC minting (which was already not possible due to the protocol already having hit the previous ceiling of 40M.)

This thread should be used for community discussion, and I will ask the Smart Contracts Domain team to post more details here when they are able to do so.

Another executive spell will be deployed shortly which will allow MKR Holders to restore the WBTC-A debt ceiling.

11 Likes

This is a really good, harmless lesson.

MakerDAO would benefit from having an independent team validate that an executive spell performs it’s intended function.

This need will only become greater as the Maker Foundation dissolves and executive spell creators could be malicious.

6 Likes

I completely agree. I wrote a bit about this idea a fair while ago now (though I imagined something more general than just for executive spells). I don’t want to derail the thread too heavily but the topic is here if anyone wants to read up. Validation Teams: A proposal for a new variety of team

4 Likes

Hey everyone. We’ll do a longer post-mortem on Monday, but what happened is:

The Maker Protocol operates with 3 different units:

  • wad for gem balances (18 decimals, what ERC20s use)
  • ray for units like stability fees (27 decimals)
  • rad for very precise numbers (45 units, or 18+27)

Numbers that accumulate stability fees tend to be RAD since we want the highest precision available. Debt ceilings (line) are in rad because the underlying dai debt is in rad as well.

In today’s spell, the WBTC-A debt ceiling was changed to:

80 * MILLION * WAD

instead of

80 * MILLION * RAD

We have several tests, but in this case, the test had it’s unit modified as well. We will be revising and adding more tests so this does not happen again!

A new executive will be deployed that correctly fixes the WBTC-A debt ceiling.

WBTC was already at 100% utilization, so this doesn’t change things much. But it is a wake up call for us to be even more diligent when writing, testing and deploying executive spells.

Any questions, please let me know!

17 Likes

Frequent manual adjustment of DC and SF is destined to increase the risk of human error. Early realization of intelligent adjustment is the only way to reduce the risk of human error.

2 Likes

Right now, vote.makerdao.com says execution of this proposal will happen on Aug 10, 2020, 24:52 UTC 24:52. I believe that’s a bug. What’s the correct execution time?

This is not related to this specific spell.

I will assume this is correct: “Passed on Aug 9, 2020, 12:52 UTC”

According to https://catflip.co/ delay parameter is currently set to 12h => Aug 10, 00:52 UTC

As per (official rocket) chat, frontend people are already aware of the defect.
I can’t find it chat link atm (user SamM pointed it out), but it’s chrome(chromium?) bug. Firefox would display the time correctly.

If the exact same code was run on a testnet there would be an opportunity to see exactly what it does (assuming it does something immediate). I thought this was something that MakerDao was already doing for executive spells.

Why does Daistats show the increase to 608 million max if the system reduced the max for WBTC to 0? Just pulling data from someplace else or manual entry?

Global debt ceiling is another risk param, that is usually the sum of all individual debt ceilings (but doesn’t necessarily have to be).

In this case, the bug was only for the individual WBTC-A debt ceiling which was set to ~0, but the global debt ceiling was correctly increased by 80,000,000 to 608m.

3 Likes

The Maker Foundation Smart Contracts team does it, but unfortunately we’ve transferred the error to the tests as well. We will have to work on this process to try to avoid this happening again.
However your statement actually makes sense, Is someone in the MakerDAO checking the spells?

1 Like

But did the spell execute on a testnet? It’s just that nobody noticed debt ceiling being near 0?

We use a tool from dapptools called hevm with which we can fork the mainnet status and apply the spell before even deploying it (and also after).
The problem was that we transferred the error to the expected result in the function that evaluates the status after the spell is applied.
This shows that we definitely need to do some improvements to our tests like adding ranges to the expected results to avoid units mistakes happen again.

9 Likes

While this error was relatively innocent, if we had a unit error on something like the duty parameter, the result would be disastrous. I second increasing the sanity checks on the spells going forward as well as having an independent audit team.

2 Likes

This topic was automatically closed 7 days after the last reply. New replies are no longer allowed.