Repay/Withdraw max

[MerlinEgalite]

In the past is used to allows a user to pass type(uint256).max and compute the max. On the current code base it's not possible. Should we?

[MathisGD]

The problem is that withdrawer/repayer are often not sync (computing their amount off-chain), and some blocks can pass between their submission and the tx being included. Thus it's convenient for them to have a way to make sure that they withdraw/repay everything.

In see two ways of solving this inside of the protocol:

• take the max if the amount is max (the idea you are mentionning)
• or take a number of share as argument of repay / withdraw

Nb: this is not an issue for user computing their amount synchronously (ie through a smart contract)

[MerlinEgalite]

or take a number of share as argument of repay / withdraw

Do you suggest shifting from amount to shares or duplicating functions?

[MathisGD]

shifting, I really hate duplicates

[QGarchery]

I like the shares idea, but then I think it completely removes the ability of a user to interact directly with the protocol.
We could provide a layer on top to easily maniputate positions, it could contain:

• a solution to this, where the user can pass an amount and it would do the accounting of the shares
• an ETH Gateway
• an id management, where you can provide just the id of the market
• ...

[MerlinEgalite]

For now I'm more inclined to do the first option as it's a one liner in the end and quite simple. The other options are adding a lot of complexity for something that the protocol can easily do (I know this is an argument quite common)

[MathisGD]

I think it completely removes the ability of a user to interact directly with the protocol

I don't think so, cTokens are not rebasing but they are "unusable direclty" right ?

[QGarchery]

I don't think so, cTokens are not rebasing but they are "unusable direclty" right ?

I'm talking about the lending pool, not the token. In Compound, you pass the amount in underlying that you want to supply. If you had to pass the shares, yes I think Compound would get significantly harder to use directly. As a user, I care about the assets I will transfer and I don't care about the shares (this is the internal accounting of the protocol)

[makcandrov]

I think being able to withdraw/repay the maximum amount is a must. We can trigger this logic only if amount == type(uint256).max

[Rubilmax]

Using shares on withdraw/repay is what I suggested here:

https://github.com/morpho-labs/blue/blob/63e8da9592869a8935a4dadba3b73063db19302f/src/Morpho.sol#L103-L104

There's a middle ground where you also expose redeem/redeemBorrow functions that behave like withdraw/repay but expect shares as input (see EIP4626)
Shares can always get queried anyway, so I disagree with the fact that it complexifies the protocol

I am also ok to enable withdraw/repay max, only when input is type(uint256).max though. The reasoning is the same as reverting when input is 0: I perfer reverting anytime the input is unexpected (regarding the values stored in state: 0 or any amount above the user's balance), except for conventional inputs (such as type(uint256).max)

References in my PoC

https://github.com/morpho-labs/blue/blob/63e8da9592869a8935a4dadba3b73063db19302f/src/libraries/MarketStateLib.sol#L47-L76

https://github.com/morpho-labs/blue/blob/63e8da9592869a8935a4dadba3b73063db19302f/src/libraries/MarketStateLib.sol#L89-L125

[MerlinEgalite]

I think I'm for having the capacity to put type(uint256).max and returning the amount indeed deposited or withdrawn

[MerlinEgalite]

@julien-devatom do you have thoughts on this?

[julien-devatom]

Using shares is cleaner and also more understandable: you know what you are withdrawing. Input is not "random" where, by using max, you don't really know how much you're withdrawing. more predictable, and you provide what the user is requesting.

[julien-devatom]

For the UX of the protocol, this is obviously harder, but you can easily have an exchange rate and shift from one to another

[MerlinEgalite]

Should we expose those getters?

[QGarchery]

Even if you use shares you have to deal with some rounding errors and now it's the underlying amount that will be "random". It's annoying if you want to approve precisely, but more importantly it's less obvious what you will actually receive/give (as underlying amount, which is what matters for the user)

[MathisGD]

Should we expose those getters?

#3

[MathisGD]

I would be cool to know what "share" of withdraw / repay are withdrawing/repaying max

[MerlinEgalite]

I would be cool to know what "share" of withdraw / repay are withdrawing/repaying max

A user can already check which share she has no?

[Rubilmax]

Even if you use shares you have to deal with some rounding errors and now it's the underlying amount that will be "random". It's annoying if you want to approve precisely, but more importantly it's less obvious what you will actually receive/give (as underlying amount, which is what matters for the user)

I want to emphasize this, as we've seen twice with the bulker that it's very useful to know how much exactly the bulker will receive (resp. transfer) upon withdrawing (resp. repaying), to avoid leaving dust.
In the case of a withdrawal, using Math.min is a workaround.
In the case of a repay, we fully rely on the bulker to have the appropriate amount of tokens, which is most of the time calculated upfront, offchain...

For other general usecases, we have the possibility to return the amount actually withdrawn (resp. repaid).

On a side note, I believe that EIP4626 already discussed this. It is the reason I suggest to stick to it. Here's the full discussion lol.

[MerlinEgalite]

It is the reason I suggest to stick to it

You mean exposing the 2 entry points (shares and underlying), right?

Quite a long discussion 😅

[Rubilmax]

You mean exposing the 2 entry points (shares and underlying), right?

Well yes, but it's the default choice of following what others engineered before us. It seems everything is questioned lately, so I'm not sure this habit has wind in its sails... But it's my suggestion.
It also includes exposing some way of switching from shares to assets. It seems exposing shares and totalSupply + totalShares of a market is enough to calculate it on & off-chain.

[Rubilmax]

Actually I read the discussion and they do talk about shares vs assets starting from here.

Out of topic: There are also discussions about naming here.

Actually, after reading a lot of it:

• at first I thought it was a good idea to guide our thoughts based on what's done in EIPs
• but EIPs are designed in a very abstract manner, to answer most of needs. Therefore, it may be sub-optimal for specific usecases

Here are my thoughts on the topic of this issue after further reasoning:

Issues with using assets

• assets can accrue right upon interaction, which makes it difficult to repay all. We solve this using a conventional input type(uint256).max or Math.min(assets, sharesOf(user))
• Any offchain integrator won't be able to precisely calculate the amount of assets repaid (resp. withdrawn) because of asynchronicity (non-atomicity)

Issues with using shares

• Requires to query the amount of shares possessed. Can be queried off-chain.
• Any offchain integrator won't be able to precisely calculate the amount of assets repaid (resp. withdrawn) because of asynchronicity (non-atomicity)

So it seems using shares avoid having to think of edge cases. So I'd finally suggest to only expose repay & withdraw functions using shares and returning assets. Thanks to the return value, onchain integrators are able to have the amount actually repaid (resp. withdrawn), or even know the amount upfront because their interaction is atomical.

[MerlinEgalite]

using shares and returning assets.

I think this could be very misleading and lead to errors.

Because in both cases we would have the issue below, it seems that they will need to infinite approve blue and pass either type(uint256).max (for an underlying-based mechanism) or the current number of shares (for a share-based mechanism) adding 1 added CALL to blue. It's not obvious to me that the share-based mechanism.

Any offchain integrator won't be able to precisely calculate the amount of assets repaid (resp. withdrawn) because of asynchronicity (non-atomicity)

[Rubilmax]

I think this could be very misleading and lead to errors.

This is what EIP4626 does, and it seems to not be misleading since the Ethereum community accepted the proposal. If manipulating both amounts is misleading then anything can be misleading at this point. Naming is key.

I didn't understand the second part of your comment.

[MerlinEgalite]

This is what EIP4626 does, and it seems to not be misleading since the Ethereum community accepted the proposal

True. But they accepted proposals that led to many hacks as well (ERC777). So I would be cautious about that type of argument.

I didn't understand the second part of your comment.

Take this example. Let's say your an integrator that wants to repay all of his debt.

Underlying-based mechanism:

1. Infinite approve blue to spend your tokens.
2. Trigger repay(type(uint256).max) returns (repaid) (require adding a line in the contract to handle this).

VS

Share-based mechanism:

1. Infinite approve blue to spend your tokens.
2. Query your shares amount.
3. Trigger repay(shares) returns (repaid)

It seems that 2. is a required and added step for the share-based mechanism.

[Rubilmax]

That's correct, though you can query it offchain and pass it as parameter. I have added this to my comment above.

I may lean towards using assets now (lol), because I infer that most usecases are repayAll/withdrawAll. Needs to be backed by data.

Using shares would be appropriate to perform relative interactions such as repay(1/4)/withdraw(1/4). Perhaps is it overkill because no one requires such a level of precision. A usecase would typically be: price has moved 1%, I am leveraged 5 times, repay 5% (to keep same LTV).

Is it overkill to expose both?

[MerlinEgalite]

I think this is a bit overkill yes haha

[MerlinEgalite]

To sum up: do we agree on the following?

• Use underlying amount as parameter
• Allow repay/withdraw max with type(uint256).max

[Rubilmax]

I stumbled upon something related when working on rounding directions: upon withdrawal, we round up when calculating the shares associated with an input amount.

This number of shares can thus be 1 wei higher than the user's shares, resulting in an underflow when writing to storage.
Meaning that a user who wants to repay x may revert because of this. One way to fix this could be to just Math.min(shares, user shares), but then we wouldn't revert for too large withdrawals.

So the only thing that I can think of to be able to revert on unexpected withdrawals as well as not revert for amounts associated with shares 1 wei off would be:

if(amount == type(uint256).max) shares = userShares;
else {
    require(shares <= userShares + 1);
    shares = Math.min(shares, userShares);
}

Which seems to be a bit complicated for such a behavior vs a simple Math.min that doesn't revert on unexpected withdrawal inputs

[MerlinEgalite]

Maybe it puts into question the fact of rounding up on withdraw?

[MathisGD]

Summary of our work/discussions:

Objective: allowing user, that are potentially asynchronous (EOAs) to withdraw / repay max

Solution 1: do nothing

onchain integrators would do something like that (see #183):

_BLUE.accrueInterests(market, id);

uint256 shares = _BLUE.supplyShare(id, msg.sender);
uint256 totalSupply = _BLUE.totalSupply(id);
uint256 totalSupplyShares = _BLUE.totalSupplyShares(id);

uint256 amount = shares.mulDivDown(totalSupply, totalSupplyShares);

_BLUE.withdraw(market, amount, msg.sender);

and offchain users would have to authorize a smart-contract to do that for them

gas cost: 81283 (see #183)

NB: this should be absolutely coupled with putting accrueInterests public
NB: the gas cost doesn't account for the authorization (for offchain users)$
NB: this solution is not really still in the discussions

Solution 2: type(uint256).max

on/offchain integrator would do something like that (see #184):

_BLUE.withdraw(market, type(uint256).max, msg.sender);

+: really simple to repay/withdraw max
-: code slightly more complicated, and costly (for everyone)

gas cost: 78420 (see #184)

Solution 3: shares as input

on/offchain integrators would do something like that (see #194):

blue.withdraw(market, blue.supplyShare(id, address(this)), address(this), receiver);

gas cost: unknown

+: simpler and cheaper code, more natural to withdraw/repay max, no issues with type(uint256), fixes #177, fixes #155
-: less natural to people who don't understand anything of the logic, discrepancy between supply/borrow - withdraw/repay interfaces

[Jean-Grimal]

Solution 2: type(uint256).max

I'm strongly in favor of this solution. It allows users to withdraw/repay the exact amount they want without caring about the shares or having to compute anything on their own. And I don't think it makes the code less readable.

I also think discrepancy between supply/borrow - withdraw/repay introduced by Solution 3 would be misleading, and therefore a problem.

[Rubilmax]

And I'm strongly in favor of a solution using shares as input and exposing corresponding return values, because Blue is low-level primitive where exact inputs are more important than simple inputs for a user-facing friendly API