Skip to content
New issue

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

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

Already on GitHub? # to your account

LIP-6: burning the limited amount per single run [WIP] #389

Merged
merged 5 commits into from
Jan 24, 2022
Merged

Conversation

TheDZhon
Copy link
Contributor

Make front-running exploits on processLidoOracleReport unprofitable by burning the limited amount only per single report.

Make front-running exploits on `processLidoOracleReport` unprofitable by
burning the limited amount only per single report.
@TheDZhon TheDZhon self-assigned this Jan 18, 2022
@TheDZhon TheDZhon requested review from ujenjt and arwer13 January 18, 2022 21:28
@TheDZhon TheDZhon changed the title LIP-6: audit [WIP] LIP-6: limit the amount to burn per single run [WIP] Jan 18, 2022
@TheDZhon TheDZhon changed the title LIP-6: limit the amount to burn per single run [WIP] LIP-6: burning the limited amount per single run [WIP] Jan 18, 2022
Copy link
Member

@ujenjt ujenjt left a comment

Choose a reason for hiding this comment

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

Looks good 👌 Leave a comment about ability to set burn limit via constructor.

Comment on lines 85 to 86
uint256 private maxBurnAmountPerRunBasePoints = 4; // 0.04% by default for the biggest `stETH:ETH` curve pool

Copy link
Member

Choose a reason for hiding this comment

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

Please consider to set the variable via constructor. It will save one extra call for setting via setter on deploy. 🙌

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll do it, an easy one 👍

Introduce new constructor param.
Add tests.
Copy link
Contributor

@arwer13 arwer13 left a comment

Choose a reason for hiding this comment

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

👍 I've left just a couple of cosmetic remarks.

contracts/0.8.9/SelfOwnedStETHBurner.sol Outdated Show resolved Hide resolved
contracts/0.8.9/SelfOwnedStETHBurner.sol Outdated Show resolved Hide resolved
The token standard has the following direct note:
> Callers MUST handle false from returns (bool success). Callers MUST NOT assume that false is never returned!
Remove reduntant requiremenets.
Remove a revert reason from tests.
@TheDZhon TheDZhon linked an issue Jan 23, 2022 that may be closed by this pull request
@TheDZhon TheDZhon merged commit 3d6a3f5 into develop Jan 24, 2022
@TheDZhon TheDZhon deleted the audit/LIP-6 branch May 19, 2022 05:53
RayLvv pushed a commit to okx-dao/lido-dao that referenced this pull request Dec 12, 2022
LIP-6: burning the limited amount per single run [WIP]
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LIP-6: implement the audit-related improvements and cleanups
3 participants