Description

While the vault is in progress, the helper function calculateVariableFeeEarningsShare is used to calculate the current amount owed to the sender as a variable-side depositor:

function calculateVariableFeeEarningsShare() internal returns (uint256) {
  (uint256 currentState, uint256 feeEarningsShare) = calculateVariableWithdrawState(
    feeEarnings + withdrawnFeeEarnings,
    variableToWithdrawnFees[msg.sender]
  );

  variableToWithdrawnFees[msg.sender] = currentState;
  withdrawnFeeEarnings += feeEarningsShare;
  feeEarnings -= feeEarningsShare;

  return feeEarningsShare;
}

However, after the vault is finalized, the calculation is done without this helper function but with similar logic:

function vaultEndedWithdraw(uint256 side) internal {
  // [...]

  uint256 feeShareAmount = 0;
  uint256 totalFees = withdrawnFeeEarnings + feeEarnings;

  if (totalFees > 0) {
    (uint256 currentState, uint256 feesShare) = calculateVariableWithdrawState(
      totalFees,
      variableToWithdrawnFees[msg.sender]
    );
    feeShareAmount = feesShare;
    variableToWithdrawnFees[msg.sender] = currentState;
  }

Note that this logic is identical to the logic in the helper function, except that withdrawnFeeEarnings is no longer updated.

Impact

The public view function for withdrawnFeeEarnings will stop tracking the amount of withdrawn fee earnings after the vault is finalized.

There is no other impact because everywhere it is internally read, withdrawnFeeEarnings is added to feeEarnings, and this oversight does not affect the correctness of the sum of the two.

Recommendations

We recommend using the helper function calculateVariableFeeEarningsShare in vaultEndedWithdraw, instead of repeating the logic, to increase code maintainability and ensure that withdrawnFeeEarnings is always correct when read by users and third-party contracts.

Remediation

This issue has been acknowledged by Saffron, and a fix was implemented in commit fddfa3b1↗.