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.

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

Fix: too high simulated share rate #730

Conversation

TheDZhon
Copy link
Contributor

@TheDZhon TheDZhon commented Apr 8, 2023

Allow the simulated share rate to be either higher or lower within the defined bounds.
A brief explanation was added to the code (see the key extracts below).
Test cases to illustrate both possible scenarios are provided with the current PR.

Resolves the issue with the missed AccountingOracle reports on Görli. The weakness has been discovered internally and hasn't been reported by external teams yet.

Previously existing test cases with submits between ref_slot and submit data block didn't trigger revert because of insufficient TVL change.


        // the simulated share rate can be either higher or lower than the actual one
        // in case of new user-submitted ether & minted `stETH` between the oracle reference slot
        // and the actual report delivery slot
        //
        // lower it can be for a negative token rebase (token rebase >= one off CL balance decrease)
        // higher it can be for a positive token rebase (token rebase <= max positive token rebase)
        //
        // user-submitted ether & minted `stETH` don't exceed the current staking rate limit
        // (see Lido.getCurrentStakeLimit())
        //
        // thus, the `simulatedShareRateDeviationBPLimit` (L) should be set as follows:
        // L = 2 * SRL * max(CLD, MPR),
        // where:
        // - CLD is one-off CL balance decrease (as BP),
        // - MPR is max positive token rebase (as BP),
        // - SRL is staking rate limit normalized by TVL (`maxStakeLimit / totalPooledEther`)
        //   totalPooledEther should be chosen as a reasonable lower bound of the protocol TVL
        //
        uint256 simulatedShareDiff = Math256.absDiff(actualShareRate, _simulatedShareRate);
        uint256 simulatedShareDeviation = (MAX_BASIS_POINTS * simulatedShareDiff) / actualShareRate;

        if (simulatedShareDeviation > _limitsList.simulatedShareRateDeviationBPLimit) {
            revert IncorrectSimulatedShareRate(_simulatedShareRate, actualShareRate);
        }

@TheDZhon TheDZhon requested a review from skozin April 8, 2023 18:39
@TheDZhon TheDZhon changed the base branch from feature/shapella-upgrade to fix/shapella-upgrade-from-rc1-to-rc2 April 10, 2023 08:23
@TheDZhon TheDZhon mentioned this pull request Apr 10, 2023
16 tasks
@TheDZhon TheDZhon merged commit ffa1fa0 into fix/shapella-upgrade-from-rc1-to-rc2 Apr 11, 2023
@TheDZhon TheDZhon deleted the fix/too-high-simulated-share-rate branch April 11, 2023 12:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants