kmbarry1 review comments

[kmbarry1]

1. Potentially limited usefulness when DAI supply is near the global debt ceiling.

If a flash loan is taken that causes debt to exceed Line, this means that the receiver contract can't trigger any action, either directly or indirectly, that results in drawing DAI. This potentially somewhat limits the usefulness of the flash lending feature. A workaround would be to increase the global debt ceiling for the duration of the external call by the amount of the flashloan, although this would increase gas costs.

2. Consider adding a DAI reclamation function.

This contract seems especially likely to be accidentally sent either ERC-20 or internal DAI (e.g. by an incorrect implementation of receiver logic). It may be prudent to add functions that would transfer such funds to the Vow (from where governance can further redistribute them if desired). I don't feel strongly about this, however.

3. Argument for accounting safety

The Vow doesn't know about sin balances assigned to other contracts, and similarly ignores Vat.debt and Vat.vice, so it seems like there shouldn't be any unexpected behaviors there.

4. Argument for DSR safety

The requirement to drip before depositing into the Pot prevents extracting value from the system in single-block operations.

[gbalabasquer]

1. another reason to evaluate the possibility to set Line really high.

2. vat.dai can be recovered from governance without an extra function, but it is true that erc20 DAI will be stuck. IMO there is a solution for this that also makes the user experience more efficient.
   I had originally proposed the vat.move happens in an async function so it is gas cheaper for the users. We might have two extra functions, one that just does the vat.move and other that does a daiJoin of the balance first.

3. Right, that is why the PR for changing the flash loan context to the vow shouldn't go.

4. Right.

[kmbarry1]

1. Agree the issue goes away if Line is set quite high, although then you lose any protection it might otherwise provide.
2. Ah yes, I like the async profit sweeping to save gas, so long as we're sure it will be called regularly (seems like the sort of thing MKR holders would have to run a Keeper for). To recover internal DAI, do you propose having this module hope the pause proxy or something? Governance cannot directly confiscate DAI if such permission is not granted.

Another thought: a very large Vault could potentially use this feature to evade stability fees. Basically, they would need to frontrun any call that would trigger drip on their ilk of interest with a call of their own that takes a flashloan, repays their debt, drips, and then redraws their debt to repay the loan (plus paying the fee). I think in practice this will not be profitable, as the flash loan fee will cost more than their stability fee, and there will be gas costs too. But wanted to explicitly write the thought down--good argument for why we should never set the fee to zero for flash loans.