ERC4626 inflation attack mitigation #3979
Conversation
… inflation attacks
🦋 Changeset detectedLatest commit: e708b4d The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Co-authored-by: Francisco <[email protected]>
Co-authored-by: Francisco <[email protected]>
Co-authored-by: Francisco <[email protected]>
|
You're welcome 😉 |
|
@boringcrypto Oh, not sure if this was your point but we should probably credit YieldBox in our docs. It took us a lot of time to be convinced but that was definitely how we got to this solution (#3706 (comment)). Do you know if you were the first to propose this approach? |
|
|
||
| const { tx } = await this.vault.deposit(parseToken(1), recipient, { from: holder }); | ||
|
|
||
| await expectEvent.inTransaction(tx, this.token, 'Transfer', { |
There was a problem hiding this comment.
Maybe I missed them somehow, but the tests do not include any event emission tests for Deposit and Withdraw. I would say those are the most important ones to test in this context.
There was a problem hiding this comment.
Addressed here: #4072
(you should work with us 😄 )
There was a problem hiding this comment.
highly appreciate your comment and am happy it improves the overall code quality - I can't commit anything full-time currently and will keep monitoring the OZ contracts as a side project for now (but never say never 😄)
Fixes #3706
Fixes LIB-599
Fixes LIB-653
Use virtual shares and assets to mitigate inflation attacks. The vault adds 1:N virtual asset/shares to enforce an initial exchange rate. This solves the issue of unhealty/broken vaults. It also limits the effectiveness of inflation attacks
The N value is
10**offset, withoffsetbeing the decimal (precision) difference between the vault and the underlying token.Technically, the
offsetcorrespond the difference (in orders of magnitude) between the funds required by the attackers and the funds being deposited by the user.Note that this is a breaking change: the presence of virtual shares means the vault is likely to "capture" some value, and 1 unit (wei) to underlying asset might be unrecoverable. The test file shows that on a typical lifecyle
Analysis of the offset effect on the inflation attack
PR Checklist
npx changeset add)