Skip to content

Latest commit

 

History

History
59 lines (45 loc) · 4.15 KB

091.md

File metadata and controls

59 lines (45 loc) · 4.15 KB

Orbiting Sangria Porpoise

Medium

Users may end up paying higher than required fees during opening & closing of leveraged strategies

Summary

NumaLeverageVaultSwap::swap() calculates buy/sell fees based on the estimated input amount rather than the actually used amount during token swaps. This leads to users paying higher fees than necessary when the actual amount needed for the swap is less than the initial estimate.

Description

When a user initiates a leverage strategy through CNumaToken::leverageStrategy(), the protocol first estimates the amount of tokens needed for the swap (borrowAmount) to ensure getting the desired output amount (_borrowAmount) after accounting for slippage. This estimate calculated by NumaLeverageVaultSwap::getAmountIn() is intentionally higher than likely needed as a safety margin. This and other factors give rise to the following situation:

  1. The protocol estimates required input (e.g., 31 rETH) to get desired output (e.g., 30 NUMA worth):
uint borrowAmount = strat.getAmountIn(_borrowAmount, false);
  1. This full estimated amount is temporarily borrowed to repay the vault:
borrowInternalNoTransfer(borrowAmount, msg.sender);
  1. The swap is executed:
(uint collateralReceived, uint unUsedInput) = strat.swap(
    borrowAmount,
    _borrowAmount,
    false
);

  1. strat.swap internally called vault.buy() which called buyNoMax(). Fee was calculated based on the output numaAmount. This value numaAmount after deduction of fees was returned by the function, which got stored in the variable collateralReceived above.

  2. CNumaToken::leverageStrategy() then continues and checks if collateralReceived > _borrowAmount and returns excess amount to the borrower if true:

        //refund if more collateral is received than needed
        if (collateralReceived > _borrowAmount) {
            // send back the surplus
            SafeERC20.safeTransfer(
                IERC20(underlyingCollateral),
                msg.sender,
                collateralReceived - _borrowAmount
            );
        }

As can be seen, while the excess collateralReceived itself is properly returned, the excess fees charged is not. User had to pay the fees even on the collateralReceived - _borrowAmount amount.

Impact

Users pay higher fees than expected.

Additional Impacted Area

The same issue also crops up when closing a leveraged strategy:

Mitigation

Implement a fee refund mechanism that returns the excess fee after the if (collateralReceived > _borrowAmount) check.