Repay/Withdraw max

[MerlinEgalite]

Fixes #40

[MathisGD]

#123 (comment)

[QGarchery]

It would also be nice to have a concrete example of a rounding error that is prevented by this PR.

In general, I think that we should first make sure that we know where all the rounding errors can come from before doing patches like this, and try to find a solution that applies globally

[MerlinEgalite]

It would also be nice to have a concrete example of a rounding error that is prevented by this PR.

In general, I think that we should first make sure that we know where all the rounding errors can come from before doing patches like this, and try to find a solution that applies globally

I don't understand your comment. The PR as it is, is not a patch of rounding but it allows to repay everything by passing type(uint256).max

[QGarchery]

I don't understand your comment. The PR as it is, is not a patch of rounding but it allows to repay everything by passing type(uint256).max

You're right. I was thinking of the issue highlighted by @Jean-Grimal and how this PR introduces another type of rounding error (going from shares to amounts in the withdraw function)

[MerlinEgalite]

I'm ok to put this PR on hold until we're clear about roundings, wdty?

[Rubilmax]

I'm ok to put this PR on hold until we're clear about roundings, wdty?

The rounding issue raised in this PR is actually introduced in this PR, and I believe I have a clear view over it, as described here: #123 (comment).
I am in favor of a change similar to what @Jean-Grimal suggested. I can take care of it.

[MerlinEgalite]

Yes you can do a PR on top if you want.

[julien-devatom]

The disadvantage of using a maxuint256 as exact value is that it complexity the implementation with a permit 2 into a Metamorpho vault where the amount is maximized by maxUint160.
You have to add a specific logic, such as

if(amount == type(uint160).max) 
    MORPHO.supply(market, type(uint256).max, address(this));

[Rubilmax]

The disadvantage of using a maxuint256 as exact value is that it complexity the implementation with a permit 2 into a Metamorpho vault where the amount is maximized by maxUint160.
You have to add a specific logic, such as

if(amount == type(uint160).max) 
    MORPHO.supply(market, type(uint256).max, address(this));

Why would the approval amount and the repay amount be equal? Can't we just approve type(uint160).max via Permit2 and repay type(uint256).max?

[MerlinEgalite]

The disadvantage of using a maxuint256 as exact value is that it complexity the implementation with a permit 2 into a Metamorpho vault where the amount is maximized by maxUint160. You have to add a specific logic, such as

if(amount == type(uint160).max) 
    MORPHO.supply(market, type(uint256).max, address(this));

I'm not even sure we can use permit2 into ERC4626 vaults because you can't pass the signature if you follow the standard.

But this is a very good point though (and a bit annoying for the bulker).

[julien-devatom]

We can, this is a workaround. My point is that if your contract build onto Blue has amount limitation, it locks the max repay/withdraw. Permit2 is just an example.

For example, on ma3 optimizer, the amount defined in repayWithPermit cannot be higher than maxUint128, since the amount is the same one as the permit2 signature one.

My first take would be just to acknowledge that this is a blue specification.
My second one would be to use min(amount, balance).

Btw for the Bulker, this is not blocking since the transferFrom2 and repay are handled separately.

[Rubilmax]

Indeed, it forces integrators to only use type(uint256).max to repay/withdraw all.
If we use the min, an integrator may expect to repay/withdraw x but would end up repaying/withdrawing less than x.

In any case, I don't think it's a big deal, so I'm ok with any solution, though I prefer the type(uint256).max option.

Reminder: this was also discussed in the corresponding issue.

[MerlinEgalite]

Oh so let's integrate this

[MathisGD]

Replaced by #194, now that we have the equivalent even with shares