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

Jump to bottom

Fixes related to handleOracleReport and shares burning #636

Merged
merged 6 commits into from
Feb 20, 2023
Merged

Fixes related to handleOracleReport and shares burning #636

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b18a1e7
doc: handleOracleReport with eth_call JSON-RPC API
TheDZhon Feb 18, 2023
8c16b5d
doc: provide an explainer about burn limits
TheDZhon Feb 19, 2023
616f434
chore: revertsWithCustomError for hashconsensus
TheDZhon Feb 19, 2023
f016373
refactor: simulated share rate naming everywhere
TheDZhon Feb 19, 2023
f11eb9a
chore: DONT_FINALIZE_WITHDRAWALS comment
TheDZhon Feb 19, 2023
55c2b09
chore: improve doc for 'checkSimulatedShareRate'
TheDZhon Feb 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
32 changes: 18 additions & 14 deletions contracts/0.4.24/Lido.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,8 @@ contract Lido is Versioned, StETHPermit, AragonApp {

uint256 private constant DEPOSIT_SIZE = 32 ether;
uint256 public constant TOTAL_BASIS_POINTS = 10000;
/// @dev special value for the last finalizable withdrawal request id
/// @dev special value to not finalize withdrawal requests
/// see the `_lastFinalizableRequestId` arg for `handleOracleReport()`
uint256 private constant DONT_FINALIZE_WITHDRAWALS = 0;

/// @dev storage slot position for the Lido protocol contracts locator
Expand Down Expand Up @@ -564,10 +565,9 @@ contract Lido is Versioned, StETHPermit, AragonApp {
* @param _lastFinalizableRequestId right boundary of requestId range if equals 0, no requests should be finalized
* @param _simulatedShareRate share rate that was simulated by oracle when the report data created (1e27 precision)
*
* NB: `_simulatedShareRate` should be calculated by the Oracle daemon
* invoking the method with static call and passing `_lastFinalizableRequestId` == `_simulatedShareRate` == 0
* plugging the returned values to the following formula:
* `_simulatedShareRate = (postTotalPooledEther * 1e27) / postTotalShares`
* NB: `_simulatedShareRate` should be calculated off-chain by calling the method with `eth_call` JSON-RPC API
* while passing `_lastFinalizableRequestId` == `_simulatedShareRate` == 0, and plugging the returned values
* to the following formula: `_simulatedShareRate = (postTotalPooledEther * 1e27) / postTotalShares`
*
* @return postTotalPooledEther amount of ether in the protocol after report
* @return postTotalShares amount of shares in the protocol after report
Expand Down Expand Up @@ -1210,7 +1210,7 @@ contract Lido is Versioned, StETHPermit, AragonApp {
* (i.e., postpone the extra rewards to be applied during the next rounds)
* 5. Invoke finalization of the withdrawal requests
* 6. Distribute protocol fee (treasury & node operators)
* 7. Burn excess shares (withdrawn stETH at least)
* 7. Burn excess shares within the allowed limit (can postpone some shares to be burnt later)
* 8. Complete token rebase by informing observers (emit an event and call the external receivers if any)
* 9. Sanity check for the provided simulated share rate
*/
Expand Down Expand Up @@ -1295,8 +1295,9 @@ contract Lido is Versioned, StETHPermit, AragonApp {
);

// Step 7.
// Burn excess shares (withdrawn stETH at least)
uint256 burntWithdrawalQueueShares = _burnSharesLimited(
// Burn excess shares within the allowed limit (can postpone some shares to be burnt later)
// Return actually burnt shares of the current report's finalized withdrawal requests to use in sanity checks
uint256 burntCurrentWithdrawalShares = _burnSharesLimited(
IBurner(_contracts.burner),
_contracts.withdrawalQueue,
reportContext.sharesToBurnFromWithdrawalQueue,
Expand All @@ -1320,7 +1321,7 @@ contract Lido is Versioned, StETHPermit, AragonApp {
postTotalPooledEther,
postTotalShares,
reportContext.etherToLockOnWithdrawalQueue,
burntWithdrawalQueueShares,
burntCurrentWithdrawalShares,
_reportedData.simulatedShareRate
);
}
Expand Down Expand Up @@ -1384,17 +1385,20 @@ contract Lido is Versioned, StETHPermit, AragonApp {
/*
* @dev Perform burning of `stETH` shares via the dedicated `Burner` contract.
*
* NB: some of the burning amount can be postponed for the next reports
* if positive token rebase smoothened.
* NB: some of the burning amount can be postponed for the next reports if positive token rebase smoothened.
* It's possible that underlying shares of the current oracle report's finalized withdrawals won't be burnt
* completely in a single oracle report round due to the provided `_sharesToBurnLimit` limit
*
* @return burnt shares from withdrawals queue (when some requests finalized)
* @return shares actually burnt for the current oracle report's finalized withdrawals
* these shares are assigned to be burnt most recently, so the amount can be calculated deducting
* `postponedSharesToBurn` shares (if any) after the burn commitment & execution
*/
function _burnSharesLimited(
IBurner _burner,
address _withdrawalQueue,
uint256 _sharesToBurnFromWithdrawalQueue,
uint256 _sharesToBurnLimit
) internal returns (uint256 burntWithdrawalsShares) {
) internal returns (uint256 burntCurrentWithdrawalShares) {
if (_sharesToBurnFromWithdrawalQueue > 0) {
_burner.requestBurnShares(_withdrawalQueue, _sharesToBurnFromWithdrawalQueue);
}
Expand All @@ -1410,7 +1414,7 @@ contract Lido is Versioned, StETHPermit, AragonApp {
(uint256 coverShares, uint256 nonCoverShares) = _burner.getSharesRequestedToBurn();
uint256 postponedSharesToBurn = coverShares.add(nonCoverShares);

burntWithdrawalsShares =
burntCurrentWithdrawalShares =
postponedSharesToBurn < _sharesToBurnFromWithdrawalQueue ?
_sharesToBurnFromWithdrawalQueue - postponedSharesToBurn : 0;
}
Expand Down
56 changes: 28 additions & 28 deletions contracts/0.8.9/sanity_checks/OracleReportSanityChecker.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,10 @@ struct LimitsList {
/// @dev Represented in the Basis Points (100% == 10_000)
uint256 annualBalanceIncreaseBPLimit;

/// @notice The max deviation of stETH.totalPooledEther() / stETH.totalShares() ratio since
/// the previous oracle report
/// @notice The max deviation of the provided `simulatedShareRate`
/// and the actual one within the currently processing oracle report
/// @dev Represented in the Basis Points (100% == 10_000)
uint256 shareRateDeviationBPLimit;
uint256 simulatedShareRateDeviationBPLimit;

/// @notice The max number of exit requests allowed in report to ValidatorsExitBusOracle
uint256 maxValidatorExitRequestsPerReport;
Expand All @@ -73,7 +73,7 @@ struct LimitsListPacked {
uint16 churnValidatorsPerDayLimit;
uint16 oneOffCLBalanceDecreaseBPLimit;
uint16 annualBalanceIncreaseBPLimit;
uint16 shareRateDeviationBPLimit;
uint16 simulatedShareRateDeviationBPLimit;
uint16 maxValidatorExitRequestsPerReport;
uint16 maxAccountingExtraDataListItemsCount;
uint16 maxNodeOperatorsPerExtraDataItemCount;
Expand Down Expand Up @@ -242,15 +242,15 @@ contract OracleReportSanityChecker is AccessControlEnumerable {
_updateLimits(limitsList);
}

/// @notice Sets the new value for the shareRateDeviationBPLimit
/// @param _shareRateDeviationBPLimit new shareRateDeviationBPLimit value
function setShareRateDeviationBPLimit(uint256 _shareRateDeviationBPLimit)
/// @notice Sets the new value for the simulatedShareRateDeviationBPLimit
/// @param _simulatedShareRateDeviationBPLimit new simulatedShareRateDeviationBPLimit value
function setSimulatedShareRateDeviationBPLimit(uint256 _simulatedShareRateDeviationBPLimit)
external
onlyRole(SHARE_RATE_DEVIATION_LIMIT_MANAGER_ROLE)
{
_checkLimitValue(_shareRateDeviationBPLimit, MAX_BASIS_POINTS);
_checkLimitValue(_simulatedShareRateDeviationBPLimit, MAX_BASIS_POINTS);
LimitsList memory limitsList = _limits.unpack();
limitsList.shareRateDeviationBPLimit = _shareRateDeviationBPLimit;
limitsList.simulatedShareRateDeviationBPLimit = _simulatedShareRateDeviationBPLimit;
_updateLimits(limitsList);
}

Expand Down Expand Up @@ -472,9 +472,9 @@ contract OracleReportSanityChecker is AccessControlEnumerable {
/// @notice Applies sanity checks to the simulated share rate for withdrawal requests finalization
/// @param _postTotalPooledEther total pooled ether after report applied
/// @param _postTotalShares total shares after report applied
/// @param _etherLockedOnWithdrawalQueue ether to lock on withdrawal queue
/// @param _sharesBurntFromWithdrawalQueue shares assigned to burn from withdrawal queue
/// @param _simulatedShareRate share rate provided with the oracle report (simulated via static call)
/// @param _etherLockedOnWithdrawalQueue ether locked on withdrawal queue for the current oracle report
/// @param _sharesBurntFromWithdrawalQueue shares burnt from withdrawal queue for the current oracle report
/// @param _simulatedShareRate share rate provided with the oracle report (simulated via off-chain "eth_call")
function checkSimulatedShareRate(
uint256 _postTotalPooledEther,
uint256 _postTotalShares,
Expand All @@ -485,9 +485,9 @@ contract OracleReportSanityChecker is AccessControlEnumerable {
LimitsList memory limitsList = _limits.unpack();

// Pretending that withdrawals were not processed
// virtually return locked ether back to postTotalPooledEther
// virtually return burnt shares back to postTotalShares
_checkFinalizationShareRate(
// virtually return locked ether back to `_postTotalPooledEther`
// virtually return burnt just finalized withdrawals shares back to `_postTotalShares`
_checkSimulatedShareRate(
limitsList,
_postTotalPooledEther + _etherLockedOnWithdrawalQueue,
_postTotalShares + _sharesBurntFromWithdrawalQueue,
Expand Down Expand Up @@ -580,7 +580,7 @@ contract OracleReportSanityChecker is AccessControlEnumerable {
revert IncorrectRequestFinalization(requestTimestampToFinalizeUpTo);
}

function _checkFinalizationShareRate(
function _checkSimulatedShareRate(
LimitsList memory _limitsList,
uint256 _noWithdrawalsPostTotalPooledEther,
uint256 _noWithdrawalsPostTotalShares,
Expand All @@ -592,16 +592,16 @@ contract OracleReportSanityChecker is AccessControlEnumerable {

if (actualShareRate == 0) {
// can't finalize anything if the actual share rate is zero
revert IncorrectFinalizationShareRate(MAX_BASIS_POINTS);
revert IncorrectSimulatedShareRate(MAX_BASIS_POINTS);
}

uint256 finalizationShareDiff = Math256.abs(
uint256 simulatedShareDiff = Math256.abs(
SafeCast.toInt256(_simulatedShareRate) - SafeCast.toInt256(actualShareRate)
);
uint256 finalizationShareDeviation = (MAX_BASIS_POINTS * finalizationShareDiff) / actualShareRate;
uint256 simulatedShareDeviation = (MAX_BASIS_POINTS * simulatedShareDiff) / actualShareRate;

if (finalizationShareDeviation > _limitsList.shareRateDeviationBPLimit) {
revert IncorrectFinalizationShareRate(finalizationShareDeviation);
if (simulatedShareDeviation > _limitsList.simulatedShareRateDeviationBPLimit) {
revert IncorrectSimulatedShareRate(simulatedShareDeviation);
}
}

Expand All @@ -625,9 +625,9 @@ contract OracleReportSanityChecker is AccessControlEnumerable {
_checkLimitValue(_newLimitsList.annualBalanceIncreaseBPLimit, type(uint16).max);
emit AnnualBalanceIncreaseBPLimitSet(_newLimitsList.annualBalanceIncreaseBPLimit);
}
if (_oldLimitsList.shareRateDeviationBPLimit != _newLimitsList.shareRateDeviationBPLimit) {
_checkLimitValue(_newLimitsList.shareRateDeviationBPLimit, type(uint16).max);
emit ShareRateDeviationBPLimitSet(_newLimitsList.shareRateDeviationBPLimit);
if (_oldLimitsList.simulatedShareRateDeviationBPLimit != _newLimitsList.simulatedShareRateDeviationBPLimit) {
_checkLimitValue(_newLimitsList.simulatedShareRateDeviationBPLimit, type(uint16).max);
emit SimulatedShareRateDeviationBPLimitSet(_newLimitsList.simulatedShareRateDeviationBPLimit);
}
if (_oldLimitsList.maxValidatorExitRequestsPerReport != _newLimitsList.maxValidatorExitRequestsPerReport) {
_checkLimitValue(_newLimitsList.maxValidatorExitRequestsPerReport, type(uint16).max);
Expand Down Expand Up @@ -661,7 +661,7 @@ contract OracleReportSanityChecker is AccessControlEnumerable {
event ChurnValidatorsPerDayLimitSet(uint256 churnValidatorsPerDayLimit);
event OneOffCLBalanceDecreaseBPLimitSet(uint256 oneOffCLBalanceDecreaseBPLimit);
event AnnualBalanceIncreaseBPLimitSet(uint256 annualBalanceIncreaseBPLimit);
event ShareRateDeviationBPLimitSet(uint256 shareRateDeviationBPLimit);
event SimulatedShareRateDeviationBPLimitSet(uint256 simulatedShareRateDeviationBPLimit);
event MaxPositiveTokenRebaseSet(uint256 maxPositiveTokenRebase);
event MaxValidatorExitRequestsPerReportSet(uint256 maxValidatorExitRequestsPerReport);
event MaxAccountingExtraDataListItemsCountSet(uint256 maxAccountingExtraDataListItemsCount);
Expand All @@ -677,7 +677,7 @@ contract OracleReportSanityChecker is AccessControlEnumerable {
error IncorrectNumberOfExitRequestsPerReport(uint256 maxRequestsCount);
error IncorrectExitedValidators(uint256 churnLimit);
error IncorrectRequestFinalization(uint256 requestCreationBlock);
error IncorrectFinalizationShareRate(uint256 finalizationShareDeviation);
error IncorrectSimulatedShareRate(uint256 simulatedShareDeviation);
error MaxAccountingExtraDataItemsCountExceeded(uint256 maxItemsCount, uint256 receivedItemsCount);
error ExitedValidatorsLimitExceeded(uint256 limitPerDay, uint256 exitedPerDay);
error TooManyNodeOpsPerExtraDataItem(uint256 itemIndex, uint256 nodeOpsCount);
Expand All @@ -688,7 +688,7 @@ library LimitsListPacker {
res.churnValidatorsPerDayLimit = SafeCast.toUint16(_limitsList.churnValidatorsPerDayLimit);
res.oneOffCLBalanceDecreaseBPLimit = _toBasisPoints(_limitsList.oneOffCLBalanceDecreaseBPLimit);
res.annualBalanceIncreaseBPLimit = _toBasisPoints(_limitsList.annualBalanceIncreaseBPLimit);
res.shareRateDeviationBPLimit = _toBasisPoints(_limitsList.shareRateDeviationBPLimit);
res.simulatedShareRateDeviationBPLimit = _toBasisPoints(_limitsList.simulatedShareRateDeviationBPLimit);
res.requestTimestampMargin = SafeCast.toUint64(_limitsList.requestTimestampMargin);
res.maxPositiveTokenRebase = SafeCast.toUint64(_limitsList.maxPositiveTokenRebase);
res.maxValidatorExitRequestsPerReport = SafeCast.toUint16(_limitsList.maxValidatorExitRequestsPerReport);
Expand All @@ -707,7 +707,7 @@ library LimitsListUnpacker {
res.churnValidatorsPerDayLimit = _limitsList.churnValidatorsPerDayLimit;
res.oneOffCLBalanceDecreaseBPLimit = _limitsList.oneOffCLBalanceDecreaseBPLimit;
res.annualBalanceIncreaseBPLimit = _limitsList.annualBalanceIncreaseBPLimit;
res.shareRateDeviationBPLimit = _limitsList.shareRateDeviationBPLimit;
res.simulatedShareRateDeviationBPLimit = _limitsList.simulatedShareRateDeviationBPLimit;
res.requestTimestampMargin = _limitsList.requestTimestampMargin;
res.maxPositiveTokenRebase = _limitsList.maxPositiveTokenRebase;
res.maxValidatorExitRequestsPerReport = _limitsList.maxValidatorExitRequestsPerReport;
Expand Down
2 changes: 1 addition & 1 deletion lib/abi/OracleReportSanityChecker.json
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion test/0.4.24/lido-handle-oracle-report.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const ORACLE_REPORT_LIMITS_BOILERPLATE = {
churnValidatorsPerDayLimit: 255,
oneOffCLBalanceDecreaseBPLimit: 100,
annualBalanceIncreaseBPLimit: 10000,
shareRateDeviationBPLimit: 10000,
simulatedShareRateDeviationBPLimit: 10000,
maxValidatorExitRequestsPerReport: 10000,
maxAccountingExtraDataListItemsCount: 10000,
maxNodeOperatorsPerExtraDataItemCount: 10000,
Expand Down
22 changes: 11 additions & 11 deletions test/0.8.9/oracle-report-sanity-checker.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ contract('OracleReportSanityChecker', ([deployer, admin, withdrawalVault, elRewa
churnValidatorsPerDayLimit: 55,
oneOffCLBalanceDecreaseBPLimit: 5_00, // 5%
annualBalanceIncreaseBPLimit: 10_00, // 10%
shareRateDeviationBPLimit: 2_50, // 2.5%
simulatedShareRateDeviationBPLimit: 2_50, // 2.5%
maxValidatorExitRequestsPerReport: 2000,
maxAccountingExtraDataListItemsCount: 15,
maxNodeOperatorsPerExtraDataItemCount: 16,
Expand Down Expand Up @@ -82,7 +82,7 @@ contract('OracleReportSanityChecker', ([deployer, admin, withdrawalVault, elRewa
churnValidatorsPerDayLimit: 50,
oneOffCLBalanceDecreaseBPLimit: 10_00,
annualBalanceIncreaseBPLimit: 15_00,
shareRateDeviationBPLimit: 1_50, // 1.5%
simulatedShareRateDeviationBPLimit: 1_50, // 1.5%
maxValidatorExitRequestsPerReport: 3000,
maxAccountingExtraDataListItemsCount: 15 + 1,
maxNodeOperatorsPerExtraDataItemCount: 16 + 1,
Expand All @@ -93,7 +93,7 @@ contract('OracleReportSanityChecker', ([deployer, admin, withdrawalVault, elRewa
assert.notEquals(limitsBefore.churnValidatorsPerDayLimit, newLimitsList.churnValidatorsPerDayLimit)
assert.notEquals(limitsBefore.oneOffCLBalanceDecreaseBPLimit, newLimitsList.oneOffCLBalanceDecreaseBPLimit)
assert.notEquals(limitsBefore.annualBalanceIncreaseBPLimit, newLimitsList.annualBalanceIncreaseBPLimit)
assert.notEquals(limitsBefore.shareRateDeviationBPLimit, newLimitsList.shareRateDeviationBPLimit)
assert.notEquals(limitsBefore.simulatedShareRateDeviationBPLimit, newLimitsList.simulatedShareRateDeviationBPLimit)
assert.notEquals(limitsBefore.maxValidatorExitRequestsPerReport, newLimitsList.maxValidatorExitRequestsPerReport)
assert.notEquals(limitsBefore.maxAccountingExtraDataListItemsCount, newLimitsList.maxAccountingExtraDataListItemsCount)
assert.notEquals(limitsBefore.maxNodeOperatorsPerExtraDataItemCount, newLimitsList.maxNodeOperatorsPerExtraDataItemCount)
Expand All @@ -108,7 +108,7 @@ contract('OracleReportSanityChecker', ([deployer, admin, withdrawalVault, elRewa
assert.equals(limitsAfter.churnValidatorsPerDayLimit, newLimitsList.churnValidatorsPerDayLimit)
assert.equals(limitsAfter.oneOffCLBalanceDecreaseBPLimit, newLimitsList.oneOffCLBalanceDecreaseBPLimit)
assert.equals(limitsAfter.annualBalanceIncreaseBPLimit, newLimitsList.annualBalanceIncreaseBPLimit)
assert.equals(limitsAfter.shareRateDeviationBPLimit, newLimitsList.shareRateDeviationBPLimit)
assert.equals(limitsAfter.simulatedShareRateDeviationBPLimit, newLimitsList.simulatedShareRateDeviationBPLimit)
assert.equals(limitsAfter.maxValidatorExitRequestsPerReport, newLimitsList.maxValidatorExitRequestsPerReport)
assert.equals(limitsAfter.maxAccountingExtraDataListItemsCount, newLimitsList.maxAccountingExtraDataListItemsCount)
assert.equals(limitsAfter.maxNodeOperatorsPerExtraDataItemCount, newLimitsList.maxNodeOperatorsPerExtraDataItemCount)
Expand Down Expand Up @@ -260,22 +260,22 @@ contract('OracleReportSanityChecker', ([deployer, admin, withdrawalVault, elRewa
simulatedShareRate: (BigInt(2) * 10n ** 27n).toString()
}

it('reverts with error IncorrectFinalizationShareRate() when reported and onchain share rate differs', async () => {
const finalizationShareRate = BigInt(ETH(2.10)) * 10n ** 9n
it('reverts with error IncorrectSimulatedShareRate() when reported and onchain share rate differs', async () => {
const simulatedShareRate = BigInt(ETH(2.10)) * 10n ** 9n
const actualShareRate = BigInt(2) * 10n ** 27n
const deviation = (100_00n * (finalizationShareRate - actualShareRate)) / actualShareRate
const deviation = (100_00n * (simulatedShareRate - actualShareRate)) / actualShareRate
await assert.revertsWithCustomError(
oracleReportSanityChecker.checkSimulatedShareRate(
...Object.values({
...correctSimulatedShareRate,
simulatedShareRate: finalizationShareRate.toString()
simulatedShareRate: simulatedShareRate.toString()
})
),
`IncorrectFinalizationShareRate(${deviation.toString()})`
`IncorrectSimulatedShareRate(${deviation.toString()})`
)
})

it('reverts with error IncorrectFinalizationShareRate() when actual share rate is zero', async () => {
it('reverts with error IncorrectSimulatedShareRate() when actual share rate is zero', async () => {
const deviation = 100_00n
await assert.revertsWithCustomError(
oracleReportSanityChecker.checkSimulatedShareRate(
Expand All @@ -285,7 +285,7 @@ contract('OracleReportSanityChecker', ([deployer, admin, withdrawalVault, elRewa
postTotalPooledEther: ETH(0)
})
),
`IncorrectFinalizationShareRate(${deviation.toString()})`
`IncorrectSimulatedShareRate(${deviation.toString()})`
)
})

Expand Down