Skip to content

Latest commit

 

History

History
107 lines (74 loc) · 3.88 KB

113.md

File metadata and controls

107 lines (74 loc) · 3.88 KB

Furry Berry Armadillo

Medium

Share Dilution Due to Reward Processing Order

Summary

Processing rewards before share calculation causes share dilution as users receive fewer shares when withdrawing or depositing assets after rewards are processed.

deposit

withdraw

Root Cause

In AutoCompoundingPodLp.sol, reward processing occurs before share calculation in both deposit and withdraw functions:

 function withdraw(uint256 _assets, address _receiver, address _owner) external override returns (uint256 _shares) {
        _processRewardsToPodLp(0, block.timestamp);        <@
        _shares = _convertToShares(_assets, Math.Rounding.Ceil);
        _withdraw(_assets, _shares, _msgSender(), _owner, _receiver);
    }
function deposit(uint256 _assets, address _receiver) external override returns (uint256 _shares) {
        _processRewardsToPodLp(0, block.timestamp);                <@
        _shares = _convertToShares(_assets, Math.Rounding.Floor);
        _deposit(_assets, _shares, _receiver);
    }

Internal Pre-conditions

No response

External Pre-conditions

No response

Attack Path

No response

Impact

Users suffer share dilution during withdrawal and deposit, receiving fewer shares for their assets due to reward processing occurring before share calculation.

PoC

Add this test in AutoCompoundingPodLpTest.t.sol

 function testWithdrawShareCalculationIssue() public {
        uint256 initialDeposit = 1000 * 1e18;
        deal(address(mockStakingPoolToken), address(this), initialDeposit);
        mockStakingPoolToken.approve(address(autoCompoundingPodLp), initialDeposit);
        autoCompoundingPodLp.deposit(initialDeposit, address(this));

        address[] memory rewardTokens = new address[](2);
        rewardTokens[0] = address(rewardToken1);
        rewardTokens[1] = address(rewardToken2);
        mockTokenRewards.setProcessedRewardTokens(rewardTokens);

        uint256 rewardAmount = 1000 * 1e18;
        uint256 lpAmountOut = 1000 * 1e18; // 1:1 conversion for clarity
        mockDexAdapter.setSwapV3SingleReturn(lpAmountOut);
        deal(autoCompoundingPodLp.pod().PAIRED_LP_TOKEN(), address(autoCompoundingPodLp), lpAmountOut * 3);
        mockIndexUtils.setAddLPAndStakeReturn(lpAmountOut);

        rewardToken1.mint(address(autoCompoundingPodLp), rewardAmount);

        emit log_named_uint("Initial total assets", autoCompoundingPodLp.totalAssets());
        emit log_named_uint("Initial total supply", autoCompoundingPodLp.totalSupply());

        uint256 withdrawAmount = 500 * 1e18;
        uint256 expectedShares = autoCompoundingPodLp.convertToShares(withdrawAmount);
        emit log_named_uint("Expected shares (pre-rewards)", expectedShares);

        uint256 actualShares = autoCompoundingPodLp.withdraw(withdrawAmount, address(this), address(this));

        emit log_named_uint("Final total assets", autoCompoundingPodLp.totalAssets());
        emit log_named_uint("Final total supply", autoCompoundingPodLp.totalSupply());
        emit log_named_uint("Actual shares burned", actualShares);

        assertLt(actualShares, expectedShares, "Share calculation should be affected by reward processing");
    }
[PASS] testWithdrawShareCalculationIssue() (gas: 1574426)
Logs:
  Initial total assets: 1000000000000000000000
  Initial total supply: 1000000000000000000000
  Expected shares (pre-rewards): 500000000000000000000
  Final total assets: 1500000000000000000000
  Final total supply: 750000000000000000000
  Actual shares burned: 250000000000000000000

Mitigation

The most straightforward solution would be to:

  • Calculate shares before processing rewards.