Liquid Staking
Stakelink
DeFiHardhatOracle
50,000 USDC
View results
Previous Next
Submission Details
Severity: high
Invalid

`StakingPool::canDeposit()` function does not account for unused deposits

0xsecuri

Summary

The canDeposit() function in the StakingPool contract is intended to return the total available deposit room, accounting for unused deposits sitting in the pool, as indicated by the inline documentation. However, the current implementation does not account for these unused deposits, leading to potentially inefficient capital utilization. This discrepancy between the intended functionality and the actual implementation can result in fewer tokens being deposited into the pool than expected, affecting the protocol's efficiency.

Vulnerability Details

The inline documentation suggests that canDeposit() should account for unused deposits, as seen below:

/**
* @notice Returns the total available deposit room for this pool
@> * @dev accounts for unused deposits sitting in pool
* @return available deposit room
*/
function canDeposit() external view returns (uint256) {
uint256 max = getMaxDeposits();
​
if (max <= totalStaked) {
return 0;
} else {
return max - totalStaked;
}
}

However, the current implementation only calculates the available deposit room based on the totalStaked tokens and does not consider any unused deposits that are sitting in the pool outside of a strategy. This unused deposit amount is represented by the getUnusedDeposits() function, which is used to retrieve the amount of unused deposits:

/**
* @notice Returns the amount of unused asset tokens sitting in this pool outside a strategy
* @dev these tokens earn no yield and will be deposited ASAP on the next call to _depositLiquidity
* @return amount of unused tokens
*/
function getUnusedDeposits() external view returns (uint256) {
return token.balanceOf(address(this));
}

As we can see from the docs of the getUnusedDeposits function it is mentioned that the tokens sitting in the pool outside a strategy will be deposited ASAP on the next call to _depositLiquidity (which happens when PriorityPool::deposit() is triggered, provided there is available space in the StakingPool).

By ignoring unused deposits, the protocol's deposit logic will fail to deposit the optimal amount, leading to capital inefficiency.

The issue becomes more severe in the PriorityPool contract, where the canDeposit() function is used to determine how much can be deposited into the StakingPool:

PriorityPool.sol#L622-L630

function _deposit(
address _account,
uint256 _amount,
bool _shouldQueue,
bytes[] memory _data
) internal {
...
if (toDeposit != 0) {
@> uint256 canDeposit = stakingPool.canDeposit();
if (canDeposit != 0) {
@> uint256 toDepositIntoPool = toDeposit <= canDeposit ? toDeposit : canDeposit;
@> stakingPool.deposit(_account, toDepositIntoPool, _data);
toDeposit -= toDepositIntoPool;
}
...
}

Due to the current incorrect logic of canDeposit(), fewer tokens will be deposited into the pool than expected, leading to suboptimal utilization of capital.

Impact

The protocol is not functioning as expected, leading to capital inefficiencies. Users and external protocols relying on canDeposit() to account for unused deposits may be misled about the protocol's actual behavior, potentially causing confusion and financial losses.

Tools Used

Manual code review

Recommendations

Modify the canDeposit() function to account for unused deposits in the calculation of the available deposit room.

Updates

Lead Judging Commences

inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Appeal created

0xsecuri Submitter
about 1 year ago
inallhonesty Lead Judge
12 months ago
inallhonesty Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.