Function to cover lost assets

[MathisGD]

Nb: not super convinced about this against supplying on behalf of address zero

[MerlinEgalite]

If the token is an ERC777, one can reenter to decrease the lostAssets value further while transferring only once assets

[MerlinEgalite]

Since the function is public this can be very problematic since we can't rely on the fact that the owner is trusted

[MathisGD]

If the token is an ERC777, one can reenter to decrease the lostAssets value further while transferring only once assets

how can they skip the second token transfer?

[MerlinEgalite]

Doesn't it double count the deposited assets into newLostAssets?

Since lostAssets is reduced by assets and realTotalAssets is increased by assets at the next interaction using _accrueInterest

[MathisGD]

newTotalAssets = realTotalAsset + lostAssets
               = prevRealTotalAssets + assets + prevLostAssets - assets
               = prevRealTotalAssets - prevLostAssets

[MathisGD]

added an assertion in a test for this

[Jean-Grimal]

Looks good tbh.

I don't have anything to see at first sight. It doesn't affect the share/asset conversion price or totalAssets, just reduces lostAssets. It looks safe.

But I agree with you that it could have implications that we don't see now, so it doesn't feel safe to add this feature. Reminds me a lot of the unfamous Euler donate function that ruined their protocol.

Although it's cool to have it, I don't think having a function to cleat bad debt is a priority. I think we shouldn't take this risk

[Rubilmax]

Nb: not super convinced about this against supplying on behalf of address zero

Yeah it seems deposit(assets, address(0)) does the job if we document clearly that lost assets on a vault is actually lostAssets() - convertToAssets(balanceOf(address(0)) instead of simply lostAssets(0). It doesn't look like a burden to me

Maybe we could even rename lostAssets to shortfall or whatever and have a lostAssets getter that calculates this (renaming is useful, the getter is overkill to me)?

[adhusson]

revisiting this, lost assets must take address(0)'s phantom assets into account so lost assets are: nevermind, when giving to address(1) their assets:share ratio is correct.

// the following is wrong
uint donatedAssets = _convertToAssetsWithTotals({
  shares: balanceOf(address(0)), 
  newTotalSupply: totalSupply(), 
  newTotalAssets: totalAssets() - lostAssets()
});

uint actualLostAssets = lostAssets() - donatedAssets