Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Specify inflation note scenarios for ERC4626 #5247

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Specify inflation note scenarios for ERC4626 #5247

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions contracts/token/ERC20/extensions/ERC4626.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@ import {Math} from "../../../utils/math/Math.sol";
* The `_decimalsOffset()` corresponds to an offset in the decimal representation between the underlying asset's decimals
* and the vault decimals. This offset also determines the rate of virtual shares to virtual assets in the vault, which
* itself determines the initial exchange rate. While not fully preventing the attack, analysis shows that the default
* offset (0) makes it non-profitable even if an attacker is able to capture value from multiple user deposits, as a result
* of the value being captured by the virtual shares (out of the attacker's donation) matching the attacker's expected gains.
* With a larger offset, the attack becomes orders of magnitude more expensive than it is profitable. More details about the
* underlying math can be found xref:erc4626.adoc#inflation-attack[here].
* offset (0) makes it non-profitable in most (but not all) practical scenarios, including when capturing value from
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there are cases where the attack is profitable, we should list them or expand in the erc4626 guide. The thing is that I'm convinced it's more expensive to pay for the computation in comparison to what can be done (also considering the EVM execution limits themselves)

* multiple user deposits, as a result of the value being captured by the virtual shares (out of the attacker's donation)
* matching the attacker's expected gains. With a larger offset, the attack becomes orders of magnitude more expensive
* than it is profitable. More details about the underlying math can be found xref:erc4626.adoc#inflation-attack[here].
*
* The drawback of this approach is that the virtual shares do capture (a very small) part of the value being accrued
* to the vault. Also, if the vault experiences losses, the users try to exit the vault, the virtual shares and assets
Expand Down
Loading