Draft
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
+95
to
+120
| function redeem(uint256 _stETHAmount, address _ethRecipient) external onlyRole(REDEEMER_ROLE) whenResumed { | ||
| if (_stETHAmount == 0) revert ZeroAmount(); | ||
| if (_ethRecipient == address(0)) revert ZeroRecipient(); | ||
| if (LIDO.isStopped()) revert LidoStopped(); | ||
| if (WITHDRAWAL_QUEUE.isBunkerModeActive()) revert BunkerMode(); | ||
| if (WITHDRAWAL_QUEUE.isPaused()) revert WQPaused(); | ||
|
|
||
| uint256 sharesAmount = LIDO.getSharesByPooledEth(_stETHAmount); | ||
| uint256 etherAmount = LIDO.getPooledEthByShares(sharesAmount); | ||
|
|
||
| uint256 redeemedBefore = STORE.getValue(REDEEMED_ETHER_SLOT); | ||
| uint256 available = _reserveBalance - redeemedBefore; | ||
| if (etherAmount > available) { | ||
| revert InsufficientReserve(etherAmount, available); | ||
| } | ||
|
|
||
| LIDO.transferSharesFrom(msg.sender, address(this), sharesAmount); | ||
| BURNER.requestBurnShares(address(this), sharesAmount); | ||
|
|
||
| STORE.set(REDEEMED_ETHER_SLOT, uint104(redeemedBefore + etherAmount)); | ||
|
|
||
| (bool success,) = _ethRecipient.call{value: etherAmount}(""); | ||
| if (!success) revert ETHTransferFailed(_ethRecipient, etherAmount); | ||
|
|
||
| emit Redeemed(msg.sender, _ethRecipient, _stETHAmount, sharesAmount, etherAmount); | ||
| } |
|
Hardhat Unit Tests Coverage SummaryDetailsDiff against masterResults for commit: 8f8b960 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
rkolpakov
force-pushed
the
feat/redeems-reserve-over-sr-3.0
branch
from
9e8757c to
eb28e1a
Compare
rkolpakov
force-pushed
the
feat/redeems-reserve-over-sr-3.0
branch
from
eb28e1a to
7f95abd
Compare
Comment on lines
+95
to
+100
| function initialize(address _admin) external initializer { | ||
| if (_admin == address(0)) revert AdminCannotBeZero(); | ||
| __AccessControlEnumerable_init(); | ||
| LIDO.approve(address(BURNER), type(uint256).max); | ||
| _grantRole(DEFAULT_ADMIN_ROLE, _admin); | ||
| } |
Comment on lines
+216
to
+223
| function recoverStETHShares(address _recipient) external onlyRole(RECOVER_ROLE) { | ||
| if (_recipient == address(0)) revert ZeroRecipient(); | ||
| uint256 shares = LIDO.sharesOf(address(this)); | ||
| if (shares > 0) { | ||
| emit StETHSharesRecovered(msg.sender, shares, _recipient); | ||
| LIDO.transferShares(_recipient, shares); | ||
| } | ||
| } |
rkolpakov
force-pushed
the
feat/redeems-reserve-over-sr-3.0
branch
from
7f95abd to
5151aa6
Compare
Psirex
reviewed
Apr 20, 2026
| */ | ||
| struct BufferedEtherAllocation { | ||
| uint256 total; | ||
| uint256 redeemsReserve; |
Contributor
There was a problem hiding this comment.
Consider adding it as last property of the struct
| // --- 1. Buffer round-trip: withdraw unredeemed ETH, reconcile bufferedEther --- | ||
| address buffer = locator.redeemsBuffer(); | ||
| if (buffer != address(0)) { | ||
| IRedeemsBuffer(buffer).withdrawUnredeemed(_redeemedEther); |
Contributor
There was a problem hiding this comment.
Nitpick: the naming is a bit misleading here, since withdrawUnredeemed() is called with redeemedEther
| contract RedeemsBuffer is PausableUntil, AccessControlEnumerableUpgradeable { | ||
| using SafeERC20 for IERC20; | ||
| using SafeCast for uint256; | ||
| bytes32 public constant PAUSE_ROLE = keccak256("RedeemsBuffer.PauseRole"); |
Contributor
There was a problem hiding this comment.
I’d recommend keeping the previous role naming convention unless there is a specific need to change it. Having the constant name match the keccak256 input that produces the role hash is a convenient convention and worth preserving if possible
| * @author Lido | ||
| * @notice Generic key-value store that auto-snapshots uint104 values at oracle frame boundaries | ||
| */ | ||
| contract RefSlotStore is IRefSlotStore, AccessControlEnumerable { |
Contributor
There was a problem hiding this comment.
Consider making this contract expose only the beacon for the current oracle reporting phase. It may be better to move away from the refSlot terminology and use naming aligned with oracle frames instead — something like AccountingFrameClock.getCurrentFrame(). A shared global storage contract that can be used by different callers, where one caller may overwrite values written by another, feels a bit scary
| _pre.totalPooledEther - _pre.externalEther, // we need to change the base as shareRate is now calculated on | ||
| _pre.totalShares - _pre.externalShares, // internal ether and shares, but inside it's still total | ||
| _pre.totalPooledEther - _pre.externalEther - _pre.redeemedEther, // we need to change the base as shareRate is now calculated on | ||
| _pre.totalShares - _pre.externalShares - update.redeemedShares, // internal ether and shares, but inside it's still total |
Contributor
There was a problem hiding this comment.
Could this line underflow in the case of a negative report and deferred redeemed shares from the previous report?
Comment on lines
+251
to
+259
| function recoverEther(address _recipient) external onlyRole(RECOVER_ROLE) { | ||
| if (_recipient == address(0)) revert ZeroRecipient(); | ||
| uint256 amount = address(this).balance + _redeemedEther.value - _reserveBalance; | ||
| if (amount > 0) { | ||
| emit EtherRecovered(msg.sender, amount, _recipient); | ||
| (bool success,) = _recipient.call{value: amount}(""); | ||
| if (!success) revert ETHTransferFailed(_recipient, amount); | ||
| } | ||
| } |
Comment on lines
+283
to
+286
| function _currentRefSlot() private view returns (uint256) { | ||
| (uint256 refSlot,) = HASH_CONSENSUS.getCurrentFrame(); | ||
| return refSlot; | ||
| } |
rkolpakov
force-pushed
the
feat/redeems-reserve-over-sr-3.0
branch
4 times, most recently
from
45ef24a to
ba3ee47
Compare
rkolpakov
force-pushed
the
feat/redeems-reserve-over-sr-3.0
branch
from
ba3ee47 to
8f8b960
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.