Description

During the withdrawal process, the vault may withdraw assets from strategies (see section ref↗ for more details). When withdrawing from a strategy, the strategy will call the function _handleRewardsOnWithdraw to accrue rewards, which may increase the amount of underlying assets held by the strategy, i.e. result in a change to the return value of the function totalAssets.

function _withdraw(address caller_, address receiver_, address owner_, uint256 assets_, uint256 shares_)
    internal
    virtual
    override(ERC4626Upgradeable)
    onlyVault
{
    // [...]
    _handleRewardsOnWithdraw();
    // [...]
}

Impact

Since the reward is accrued after the assets_ and shares_ have been calculated, if the accrued reward contains the underlying asset, it is not considered when calculating the share price for this withdrawal. As a result, the share price used when calculating assets_ and shares_ may be lower than the share price after rewards have accrued, which could result in users receiving fewer assets.

Recommendations

Consider harvesting rewards from the strategies before calculating the amount of assets to withdraw or the amount of shares to burn.

Remediation

Blueprint Finance provided the following response to this finding:

We acknowledge this issue, but do not plan to fix it for the following reasons:

1. Adding _handleRewardsOnWithdraw() in the withdraw/redeem before calculating assets will require another call and that would increase gas costs.

2. We currently have no use case where rewards are compounded as the same token during _handleRewardsOnWithdraw().

3. A bot already calls vault.harvestRewards() every 30 minutes, effectively handling reward accruals. we can solve this by increasing the bot frequency as well.

4. It will be too much restructuring to implement this change in both strategy and vault codebase.

Given the limited benefit and added complexity, we prefer to leave this as-is.