[AHB-01M] Discrepant Behaviour of Paybacks

[vm06007]

AHB-01M: Discrepant Behaviour of Paybacks

Type	Severity	Location
Logical Fault		AaveHub.sol:L469-L474, L482-L486, L526-L533, L551-L555

Description:

An AaveHub::paybackExactAmountETH invocation will properly handle "overpays" by refunding any superfluous amount transmitted in the call to the caller, however, an AaveHub::paybackExactAmount invocation will simply fail due to a nested WiseLowLevelHelper::_decreasePositionMappingValue call.

Example:

/**
 * @dev Allows to payback ERC20 token for
 * any postion. Takes _paybackAmount as argument.
 */
function paybackExactAmount(
    uint256 _nftId,
    address _underlyingAsset,
    uint256 _paybackAmount
)
    external
    syncPool(_underlyingAsset)
    returns (uint256)
{
    _checkPositionLocked(
        _nftId
    );

    address aaveToken = aaveTokenAddress[
        _underlyingAsset
    ];

    _safeTransferFrom(
        _underlyingAsset,
        msg.sender,
        address(this),
        _paybackAmount
    );

    uint256 actualAmountDeposit = _wrapAaveReturnValueDeposit(
        _underlyingAsset,
        _paybackAmount,
        address(this)
    );

    uint256 borrowSharesReduction = WISE_LENDING.paybackExactAmount(
        _nftId,
        aaveToken,
        actualAmountDeposit
    );

    emit IsPaybackAave(
        _nftId,
        block.timestamp
    );

    return borrowSharesReduction;
}

/**
 * @dev Allows to payback ETH token for
 * any postion. Takes token amount as argument.
 */
function paybackExactAmountETH(
    uint256 _nftId
)
    external
    payable
    syncPool(WETH_ADDRESS)
    returns (uint256)
{
    _checkPositionLocked(
        _nftId
    );

    address aaveWrappedETH = aaveTokenAddress[
        WETH_ADDRESS
    ];

    uint256 userBorrowShares = WISE_LENDING.getPositionBorrowShares(
        _nftId,
        aaveWrappedETH
    );

    uint256 maxPaybackAmount = WISE_LENDING.paybackAmount(
        aaveWrappedETH,
        userBorrowShares
    );

    (
        uint256 paybackAmount,
        uint256 ethRefundAmount

    ) = _getInfoPayback(
        msg.value,
        maxPaybackAmount
    );

    _wrapETH(
        paybackAmount
    );

    uint256 actualAmountDeposit = _wrapAaveReturnValueDeposit(
        WETH_ADDRESS,
        paybackAmount,
        address(this)
    );

    uint256 borrowSharesReduction = WISE_LENDING.paybackExactAmount(
        _nftId,
        aaveWrappedETH,
        actualAmountDeposit
    );

    if (ethRefundAmount > 0) {
        payable(msg.sender).transfer(
            ethRefundAmount
        );
    }

    emit IsPaybackAave(
        _nftId,
        block.timestamp
    );

    return borrowSharesReduction;
}

Recommendation:

We advise the code to either properly handle overpays in both functions or to allow the code to fail in both cases, ensuring uniformity in the codebase.

[vm06007]

Team disagrees on this suggestion. It is absoltely okay to have slightly different payback flow, because ETH amount is payable it cannot be represented with share similar to paybackWithShares where this amount can be converted and charged through safeTranferFrom.

In this case when working with Native currency (ETH) and Tokens (ERC20) the approach is slightly different so that we can invoke transfer with slightly higher amount to cover functionality similar to paybackWithShares to close the position fully and expect user to have slight refund. While with ERC20 user can use function paybackExactShares when trying to close the position completely or paybackExactAmount when closing position partially.

No changes intended here as it makes no sense or benefit to be this strict for no reason and limit user to only single possible flow. Not to complex code there is no reason to introduce refund in the paybackExactAmount, user can rely on paybackExactShares when closing position fully.