Julien's review

[julienmartinlevrai]

Emergency Shutdown behavior

Flash mints should be disabled after the End.thaw() phase, because they would allow creating Dai that's unaccounted for. This additional Dai could be used to purchase collateral in the system, thus leaving legitimate Dai holders without the possibility to End.cash() their Dai.

Furthermore, in a ES situation, Dai would probably trade lower in markets than its redemption price, which would create an arbitrage opportunity for a flash borrower to redeem all the collateral in the system (up to line, you say? Keep reading).

Effectiveness of the line parameter

line will prevent a caller from borrowing too much Dai in a single transaction; but they could still include many transactions in the same block, each one of them borrowing up to line amount of Dai.

A throughput parameter that time-limits the total amount of Dai lent to all users would probably be more effective.

Existential risk for other DeFi projects

As one of the cornerstones of the DeFi ecosystem, Maker has a certain degree of responsibility when creating tools that could pose an existential threat to other DeFi projects. The risks of the inclusion of this module should be published well in advance in order to give project maintainers enough time to make sure they're not vulnerable.

Also, there should be a way to stop this module immediately in case it is being used to attack some other project. Ideally, project maintainers themselves should be able to stop this module in a similar fashion as the ESM, perhaps by sending to it a governance-defined amount of Dai, which can be later sent back to its owners by governance.

Questioning the need for the Vat Dai flashloan

AFAICT, the only advantage of the Vat Dai flashloan functionality is to provide gas savings. So I think it only makes sense if there is an actual market demand for such a functionality.

Otherwise, we are just breaking the EIP-3156 standard, providing confusing functionality, increasing the attack surface, adding to the maintainability burden, creating more complex test cases, and in general increasing the mental overload in an already complex and potentially risky module, just to save gas for a use case that may not be that popular.

Simulation tests

Finally, I think a module like this deserves simulation tests created with an adversarial mindset towards popular DeFi projects, as well as to test the Emergency Shutdown scenario.

[hexonaut]

So I don't believe this to be an issue. Yes you are correct that basically any number of DAI can be brought into existence even beyond the end assumption of a fixed amount after end.thaw(). This Dai can then be routed through end.cash() as you say, but once the user has all the collateral they then need to find a way to re-acquire all the Dai they minted. In the case where they take all the collateral they would need to find a way to re-purchase all the Dai in the market which is effectively impossible in 1 tx. Even with smaller flash mints this process effectively is trading collateral for other user's dai which is really what the end process is doing anyways.

I want to say though this kind of thinking is very important for finding edge cases. I appreciate the effort.

[kmbarry1]

I would agree that it certainly doesn't make sense to charge a fee post-thaw, although it doesn't hurt the system--it just has no benefit as the surplus can't be distributed to DAI holders. Sam is right that this doesn't affect the redeemability of DAI. Allowing flash loans post-shutdown might even make the redemption process more efficient, as smaller holders can provide DEX liquidity strategically (e.g. using Uni v3) to get a better price and directly receive the asset they desire versus having to claim and sell N different collaterals directly from the protocol. Although existing DEX flash loan functionality works just as well.