-
-
Notifications
You must be signed in to change notification settings - Fork 74
Adding MetaMask Liquid Staking Integration Tests and Adapter #138
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Infinite Allowance Overflow Bug
The _ensureAllowance
function attempts to set an infinite allowance for the withdrawalQueue
using stETH.safeIncreaseAllowance(address(withdrawalQueue), type(uint256).max)
. However, safeIncreaseAllowance
adds the specified amount to the existing allowance. If the current allowance is already greater than zero, this operation will cause an arithmetic overflow and revert, as currentAllowance + type(uint256).max
exceeds type(uint256).max
. The intended behavior was likely to set the allowance to type(uint256).max
, which should be achieved using safeApprove
.
src/helpers/LiquidStakingAdapter.sol#L175-L181
delegation-framework/src/helpers/LiquidStakingAdapter.sol
Lines 175 to 181 in 1db0f3a
/// @param _amount Amount needed for the operation | |
function _ensureAllowance(uint256 _amount) private { | |
uint256 allowance_ = stETH.allowance(address(this), address(withdrawalQueue)); | |
if (allowance_ < _amount) { | |
stETH.safeIncreaseAllowance(address(withdrawalQueue), type(uint256).max); | |
} | |
} |
Bug: Permit Value Mismatch Causes Transfer Failures
In the requestWithdrawalsWithPermit
function, the permit
call uses _permit.value
while the subsequent safeTransferFrom
uses totalAmount_
(calculated as the sum of _amounts
). If _permit.value
does not exactly match totalAmount_
, the transfer will either fail due to insufficient allowance or grant excessive allowance.
src/helpers/LiquidStakingAdapter.sol#L115-L135
delegation-framework/src/helpers/LiquidStakingAdapter.sol
Lines 115 to 135 in 1db0f3a
/// @return requestIds_ Array of withdrawal request IDs | |
function requestWithdrawalsWithPermit( | |
uint256[] memory _amounts, | |
IWithdrawalQueue.PermitInput memory _permit | |
) | |
external | |
returns (uint256[] memory requestIds_) | |
{ | |
uint256 totalAmount_ = _calculateTotalAmount(_amounts); | |
// Use permit to approve stETH transfer | |
IERC20Permit(address(stETH)).permit( | |
msg.sender, address(this), _permit.value, _permit.deadline, _permit.v, _permit.r, _permit.s | |
); | |
// Transfer stETH from sender to this contract | |
stETH.safeTransferFrom(msg.sender, address(this), totalAmount_); | |
// Execute common withdrawal logic | |
requestIds_ = _requestWithdrawals(_amounts, totalAmount_, msg.sender); | |
} |
BugBot free trial expires on July 22, 2025
You have used $0.00 of your $50.00 spend limit so far. Manage your spend limit in the Cursor dashboard.
Was this report helpful? Give feedback by reacting with 👍 or 👎
This PR needs an RPC URL secret on github to work |
What?
Why?