Increase ERC4626 virtual assets to decrease `mint` slippage due to conversion rate rounding

[Rubilmax]

🧐 Motivation

By default, the conversion rate of an ERC4626 vault is 1e(_decimalsOffset()) shares are worth 1 assets.

Let's assume that this conversion rate grows to 1e(_decimalsOffset()) shares are worth 1.05 assets (in real numbers via, for example, accrued interest. If the vault's totalSupply is redeemed, the exchange rate would get rounded up to: 1e(_decimalsOffset()) shares are worth 2 assets.

This rounding can lead to high slippage when minting/redeeming shares of the vault. In fact, the worst-case is: you expect to mint x shares against x / 1e(_decimalsOffset()) assets, but you actually deposit 2 * x / 1e(_decimalsOffset()) assets (ie 100% slippage).

📝 Details

Such a manipulation of the conversion rate requires the totalSupply to be in the hands of a single player, which is a strong assumption.

But it is interesting to see that by increasing the order of magnitude of virtual assets (currently set at 1), you can decrease the order of magnitude of the slippage. And it is linear: 10 virtual assets lead to a maximum 10% slippage upon minting.

So it could be interesting to either:

1. be able to customize the virtual assets used (currently set at 1)
2. increase the virtual assets to 1e4 to limit the slippage to an arbitrary value of 1 bps

[ernestognw]

Hey @Rubilmax, thanks for reporting this

Your analysis sounds about right, but I'm not sure that increasing the default virtual assets is the right choice. The reason is that this issue happens only at very low values (dififcult to get there if there's at least 1 legit holder with a minimal amount). Let me elaborate:

If I understand correctly, the issue is that a majority holder (perhaps a single holder) can manipulate the rate by abusing that both the withdraw() and redeem() functions will round divisions favoring the vault, inflating the rate of assets per share.

The scenario you present is one in which after the manipulation, the rate assets/share rate gets to 100% slippage. However, this only happens if the amount of shares is extremely low (will be 1 after redeeming the totalSupply()). I made an interactive visualization in the past that I think will help us to understand the scenario a bit better (note it's not using 18 decimals for simplicity as most tokens do): https://www.desmos.com/calculator/abafa0l3ss

When there's only 1 share with no virtual offset, the amount loss on deposits is 100% slippage as you mention (purple line). In the example, the loss is of 2100 / 2 = 1050:

As expected, the deposit loss gets reduced to 19 and 20 after increasing the virtual offset:

Virtual Offset = 1 | (190 units captured by the vault)

Virtual Offset = 2 | (20 units captured by the vault)

This suggests that a higher virtual offset should be set to reduce this deposit "loss" captured by the vault. However, the catch is in the following:

Such a manipulation of the conversion rate requires the totalSupply to be in the hands of a single player, which is a strong assumption.

Remember that the virtual offset is there to increase the cost of manipulating the rate when the vault is empty. However, any deposit adds offset. For example, if at least one user has a balance of 1e-16 shares (of an 18 decimals vault with no offset), the single entity manipulating the price will need to fight against a 1e2 offset.

A single legit holder has 1e-16 shares | (175 units captured by the vault)

A single legit holder has 1e-15 shares | (20 units captured by the vault)

My conclusion is that on one hand, the virtual offset should be used to avoid manipulating when the vault is empty, but the assets captured after deployment introduce a regular offset that makes it more difficult to manipulate the rate. And on the other, adding a default offset (let's say 1e4) won't provide a lot of value and it's breaking the assumption that most tokens have 18 decimals.

Although the ecosystem should be ready to handle tokens that are not 18 decimals, we don't feel we should decide the default decimals used by the ERC4626 and the value is currently customizable via override before deployment.

Happy to hear your comments!

[Rubilmax]

It seems we're on the same page regarding the behavior of the vault and virtual assets/shares: independent legit holders will prevent the vault's conversion rate from being rounded up ; in other words: the assumption that the whole supply is in the hands of a single holder is too strong and you choose not to address this issue

Right?

Just out of curiosity though, I don't understand this statement:

And on the other, adding a default offset (let's say 1e4) won't provide a lot of value and it's breaking the assumption that most tokens have 18 decimals.

Why does it not provide a lot of value? Why is it breaking the assumption that most tokens have 18 decimals?

Perhaps have I been unclear when talking about virtual shares and assets:

• when I'm talking about "virtual shares", it corresponds to your 10 ** offset
• but when I'm talking about virtual assets, I am referring to the constant 1 added to the totalAssets() in the following conversion utilities:

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/552cffde563e83043a6c3a35012b626a25eba775/contracts/token/ERC20/extensions/ERC4626.sol#L226-L238

So I'm not suggesting to hardcode an offset of 4 or make it not customizable, but rather make the other side be customizable too (or hardcoded high enough with regard to this issue)

[ernestognw]

Yeah, seems like our understanding is the same. I'd add that we chose not to address it because its impact depends on 1) the users impacted and 2) the amount of assets stolen. In my opinion, the more users the vault has, the more difficult it becomes to manipulate the rate lowering the potential impact to just a couple of wei (or hundreds of wei in the worst case).

Note it's tricky to play using examples with low values (e.g. 1, 2, or 10) because it makes it seem like the potential losses are in the order of "1, 2, or 3 tokens", but in reality, most of these rounding errors are 1, 2, or 3 wei (at least for 18 decimal tokens).

Why does it not provide a lot of value? Why is it breaking the assumption that most tokens have 18 decimals?

Yeah, good call. I rushed into saying it doesn't provide a lot of value, but my reasoning is that the success of this attack depends on an attacker being able to manipulate the slippage in a way that's profitable (enough).

Even if a single holder exists, there are no other users to steal from. And even if there were users in the system, the value captured by the vault is really low since the vault tends to a fair share/asset ratio. Not to mention that it's distributed among all the users and not just the majority holder.

I consider adding a default virtual (shares) offset doesn't add a lot of value because it's aiming to prevent a hypothetical attack with not enough incentives to be performed and very limited impact.

Although having 18 decimals may not be a big deal, I don't think adding a default virtual (shares) offset is more valuable than leaving it with 18 decimals as most tokens do by default.

I'm interested in hearing how this attack vector is harmful in any sense, though, perhaps I'm missing something 😄

• when I'm talking about "virtual shares", it corresponds to your 10 ** offset
• but when I'm talking about virtual assets, I am referring to the constant 1 added to the totalAssets() in the following conversion utilities:

My bad, I didn't notice you were referring to assets and not shares (!). I have a couple of questions:

• You mentioned that "10 virtual assets lead to a maximum 10% slippage upon minting.". Can you elaborate on that?
• Don't you think avoiding such slippage is possible by using deposit(...) with the exact assets instead of mint(...)?

My understanding is that adding an asset offset will make the minting loss even higher, this is because the deposit rate (i.e. shares minted for assets) would equal to assets * totalSupply() + 10 ** _decimalsOffset / totalAssets + 10 ** _assetsOffset. By increasing the assetsOffset you increase the divisor, creating a higher slippage when using the deposit(...) function, so it doesn't fully prevent the slippage, but it's shifting it to the other "minting" function.

[Rubilmax]

I'm interested in hearing how this attack vector is harmful in any sense, though, perhaps I'm missing something

Clearly we're on the same page on this: it was an illustration of a non-trivial rounding that can lead to potential threat ; but in this case requires a lot of very strong assumptions

I think the goal would now be more to:

1. discuss about this conversion rate rounding when the vault is emptied to make sure we're not missing something anywhere else
2. to discuss about the inconvenient of being able to customize the virtual assets

Let me answer your questions now:

• Don't you think avoiding such slippage is possible by using deposit(...) with the exact assets instead of mint(...)?

100%! I think the disclosure is that there can be significant slippage using mint with the current virtual assets

My understanding is that adding an asset offset will make the minting loss even higher, this is because the deposit rate (i.e. shares minted for assets) would equal to assets * totalSupply() + 10 ** _decimalsOffset / totalAssets + 10 ** _assetsOffset. By increasing the assetsOffset you increase the divisor, creating a higher slippage when using the deposit(...) function, so it doesn't fully prevent the slippage, but it's shifting it to the other "minting" function.

This is correct. There's a catch though: increasing the assets offset also increases the precision on real assets earned by each individual virtual share. This is very related to:

• You mentioned that "10 virtual assets lead to a maximum 10% slippage upon minting.". Can you elaborate on that?

Example when the shares of a vault is concentrated to a single malicious shareholder (or one that can be manipulated / flashloaned):

• parameters: 1 virtual asset, 1 virtual share ; let's suppose that 1 asset was accrued over the totalAssets of 20 (so exchange rate is: 1.05 asset for 1 share). Then if the vault is emptied, exchange rate is rounded up to 2 assets for 1 share: if a mint is sandwiched by a vault emptying + deposit after, the minter can endure a slippage of 90.5% to the upside: they expected an exchange rate of 1.05 but experienced an exchange rate of 2 (worst-case: ~100% when exchange rate is 1.0...0001 for 1 share).
• parameters: 10 virtual assets, 10 virtual shares ; let's suppose that 1 asset was accrued over the totalAssets of 20 (so exchange rate is: 1.05 asset for 10 shares). Then if the vault is emptied, exchange rate is rounded up to 11 assets for 10 shares: if a mint is sandwiched by a vault emptying + deposit after, the minter can endure a slippage of 4.8% to the upside: they expected an exchange rate of 1.05 but experienced an exchange rate of 1.1 (worst-case: ~10% when exchange rate is 1.0...0001 for 10 shares).

So there would also need to be a constraint on the assetsOffset: something like assetsOffset <= decimalsOffset