refactor: lower virtual shares

[pakim249CAL]

Fixes #282

[Jean-Grimal]

The virtual shares we choose will limit the max amounts (whether we pack storage or not).
So I think we should see this problem the other way around and we should determine the max amount we aim for (or rather the smallest max amount we accept).

[MerlinEgalite]

The virtual shares we choose will limit the max amounts (whether we pack storage or not). So I think we should see this problem the other way around and we should determine the max amount we aim for (or rather the smallest max amount we accept).

Agree. Can you give us the computation to do to get the nb of virtual shares given a max amount please?

[Jean-Grimal]

The computation is quite easy actually, the overflow is more likely to happen on totalBorrowShares and totalSupplyShares which can't be over type(uint128).max = 3.4e38
So we have maxAmount * virtualShares = 3.4e38
Therefore, let virtualShares = 1eX, and let assume that assets don't have more than 18 decimals ,
maxAmount = 3.4e(38 - 18 - X) = 3.4e(20 - X), so with virtualShares = 1e6 : maxAmount = 3.4e14

Is 1e6 virtual shares enough to ensure accuracy?
Let's take the asset with the most expensive wei: WBTC (taken at $30k, 8 decimals).
Let's make 2 assumptions:
The first is that it rises to $1e6 (to consider the worst case scenario), in which case a wei is worth $1e-2.
The second is that accrued interests (which reduce the share/asset ratio, initially at 1e6) cannot bring it down to less than 1e4.
So, in the worst-case scenario, a share of the most expensive asset is worth $1e-7,
So rounding in favor of the protocol on shares only costs the user $1e-7.

[MerlinEgalite]

Maybe we should add a comment.

[QGarchery]

The computation is quite easy actually, the overflow is more likely to happen on totalBorrowShares and totalSupplyShares which can't be over type(uint128).max = 3.4e38
So we have maxAmount * virtualShares = 3.4e38
Therefore, let virtualShares = 1eX, and let assume that assets don't have more than 18 decimals ,
maxAmount = 3.4e(38 - 18 - X) = 3.4e(20 - X), so with virtualShares = 1e6 : maxAmount = 3.4e14

I agree that in the end we don't want values to go over type(uint128).max, for the reasons mentioned in #269 (comment).

But when doing the following computation
https://github.com/morpho-labs/morpho-blue/blob/e5e84d67192bcb29b7174411fea127e1dd97fe71/src/libraries/SharesMathLib.sol#L21-L23
we can temporarily go over type(uint128).max in the multiplication without overflow, and then go back under type(uint128).max after the division.

So it seems like the condition for overflow is more like assets * (virtualShares + totalShares) < 2**256. Notice that the number of totalShares dominates the virtualShares in the general case (because the virtualShares correspond to only one asset) so totalShares should really be taken into account. As a first approximation, we can omit the virtualShares in the above inequality and take totalShares = totalAssets * virtualShares. Because assets <= totalAssets, the condition virtualShares * totalAssets ** 2 < 2**256 should be enough.
This gives us the following table, assuming 1e18 decimals

virtualShares	1e3	1e6	1e9	1e12	1e15	1e18
(max) totalAssets / 1e18	1e19	3e17	1e16	3e14	1e13	3e11

Is 1e6 virtual shares enough to ensure accuracy?

Notice also that we round the assets as well as the shares. This means that we can't be more precise than 1 WEI of asset. To ensure that precision of 1 WEI of asset, it is enough to have more shares than asset.

Given that last point (and #283 (comment)) and the table above, the decision to take 1e6 virtual shares is ok with me

[MerlinEgalite]

Maybe @QGarchery you could push a little comment to explain very briefly the tradeoff we're doing here please?

[QGarchery]

Maybe @QGarchery you could push a little comment to explain very briefly the tradeoff we're doing here please?

Added in #361