ERC4626Fees base point calculation

Hi team,

I am working on a Vault contract that inherits from the 'ERC4626Fees.sol' example code posted here:

The resulting token fee after base point calculation seems to be too low by a factor of 10.

My understanding is that a standard base point calculation would look like this:
base points / 10,000 = percent in decimal form

However the example code has this, for example:

function _feeOnRaw(uint256 assets, uint256 feeBasePoint) private pure returns (uint256) {
        return assets.mulDiv(feeBasePoint, 1e5, Math.Rounding.Up);
    }

1e5 = 100,000

Should it be written as '1e4' instead of '1e5'?

function _feeOnRaw(uint256 assets, uint256 feeBasePoint) private pure returns (uint256) {
        return assets.mulDiv(feeBasePoint, 1e4, Math.Rounding.Up);
    }

ex:
1 base point / 1e4 (10,000) = 0.0001

Nice work on the ERC4626 and ERC4626Fees btw, they seem to be working great and are very helpful.

Thanks,
Jonathan

Yes, apologies, this was an error that we fixed in https://github.com/OpenZeppelin/openzeppelin-contracts/pull/4476 and https://github.com/OpenZeppelin/openzeppelin-contracts/pull/4487 a couple of months ago that we missed pushing to the documentation site.

I've just pushed them and the code should be correct now on https://docs.openzeppelin.com/contracts/4.x/erc4626#fees

1 Like

Awesome, looks good to me. Thanks!

One follow up question on this -

I am testing this code now and it works well, however I am seeing a small discrepancy between the number of vault shares returned to the depositor following a deposit and the value returned by the 'previewDeposit' function.

Example:

  • basis points initialized to 100 (1%)
  • depositor deposits 100 of underlying token
  • depositor receives 99.0099 'vTokens' in return
  • 'previewDeposit' function returns 99 (exactly) though

Accoording to the EIP spec, 'convertToShares' should be rounded down towards 0 so I believe that the depositor should receive 99 vTokens instead of 99.0099 - which would match what 'previewDeposit' is returning.

Since the ERC4626Fees contract is calling 'return super.previewDeposit(assets - fee)' - that function in the parent contract should be rounding down, so I'm not sure how the 99.0099 vTokens is still getting minted.

Thanks,
Jonathan

Note that when the EIP talks about rounding it refers to values that are not representable with the decimals of the vault or token. If the vault has 18 decimals, and when you say "100 tokens" you mean "100e18 token units", "99.0099 shares" is representable as "99.0099e18 share units" so it doesn't indicate an error in rounding.

I ran a test and didn't observe any issues. The previewed amount is exactly equivalent to the minted amount on deposit. Note that the number of shares that are minted is not 99, because when you deposit 100 tokens this is an amount that already includes a fee. Please see the distinction between _feeOnRaw and _feeOnTotal.

The code for my test was:

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import "forge-std/Test.sol";

import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import "@openzeppelin/contracts/token/ERC20/extensions/ERC4626.sol";

/// @dev ERC4626 vault with entry/exit fees expressed in https://en.wikipedia.org/wiki/Basis_point[basis point (bp)].
abstract contract ERC4626Fees is ERC4626 {
    using Math for uint256;

    uint256 private constant _BASIS_POINT_SCALE = 1e4;

    // === Overrides ===

    /// @dev Preview taking an entry fee on deposit. See {IERC4626-previewDeposit}.
    function previewDeposit(uint256 assets) public view virtual override returns (uint256) {
        uint256 fee = _feeOnTotal(assets, _entryFeeBasisPoints());
        console.log("previewDeposit", assets, fee);
        return super.previewDeposit(assets - fee);
    }

    /// @dev Preview adding an entry fee on mint. See {IERC4626-previewMint}.
    function previewMint(uint256 shares) public view virtual override returns (uint256) {
        uint256 assets = super.previewMint(shares);
        return assets + _feeOnRaw(assets, _entryFeeBasisPoints());
    }

    /// @dev Preview adding an exit fee on withdraw. See {IERC4626-previewWithdraw}.
    function previewWithdraw(uint256 assets) public view virtual override returns (uint256) {
        uint256 fee = _feeOnRaw(assets, _exitFeeBasisPoints());
        return super.previewWithdraw(assets + fee);
    }

    /// @dev Preview taking an exit fee on redeem. See {IERC4626-previewRedeem}.
    function previewRedeem(uint256 shares) public view virtual override returns (uint256) {
        uint256 assets = super.previewRedeem(shares);
        return assets - _feeOnTotal(assets, _exitFeeBasisPoints());
    }

    /// @dev Send entry fee to {_entryFeeRecipient}. See {IERC4626-_deposit}.
    function _deposit(address caller, address receiver, uint256 assets, uint256 shares) internal virtual override {
        uint256 fee = _feeOnTotal(assets, _entryFeeBasisPoints());
        address recipient = _entryFeeRecipient();

        super._deposit(caller, receiver, assets, shares);

        if (fee > 0 && recipient != address(this)) {
            SafeERC20.safeTransfer(IERC20(asset()), recipient, fee);
        }
    }

    /// @dev Send exit fee to {_exitFeeRecipient}. See {IERC4626-_deposit}.
    function _withdraw(
        address caller,
        address receiver,
        address owner,
        uint256 assets,
        uint256 shares
    ) internal virtual override {
        uint256 fee = _feeOnRaw(assets, _exitFeeBasisPoints());
        address recipient = _exitFeeRecipient();

        super._withdraw(caller, receiver, owner, assets, shares);

        if (fee > 0 && recipient != address(this)) {
            SafeERC20.safeTransfer(IERC20(asset()), recipient, fee);
        }
    }

    // === Fee configuration ===

    function _entryFeeBasisPoints() internal view virtual returns (uint256) {
        return 0; // replace with e.g. 100 for 1%
    }

    function _exitFeeBasisPoints() internal view virtual returns (uint256) {
        return 0; // replace with e.g. 100 for 1%
    }

    function _entryFeeRecipient() internal view virtual returns (address) {
        return address(0); // replace with e.g. a treasury address
    }

    function _exitFeeRecipient() internal view virtual returns (address) {
        return address(0); // replace with e.g. a treasury address
    }

    // === Fee operations ===

    /// @dev Calculates the fees that should be added to an amount `assets` that does not already include fees.
    /// Used in {IERC4626-mint} and {IERC4626-withdraw} operations.
    function _feeOnRaw(uint256 assets, uint256 feeBasisPoints) private pure returns (uint256) {
        return assets.mulDiv(feeBasisPoints, _BASIS_POINT_SCALE, Math.Rounding.Up);
    }

    /// @dev Calculates the fee part of an amount `assets` that already includes fees.
    /// Used in {IERC4626-deposit} and {IERC4626-redeem} operations.
    function _feeOnTotal(uint256 assets, uint256 feeBasisPoints) private pure returns (uint256) {
        return assets.mulDiv(feeBasisPoints, feeBasisPoints + _BASIS_POINT_SCALE, Math.Rounding.Up);
    }
}

contract MyToken is ERC20 {
    constructor() ERC20("MyToken", "MTK") {
        _mint(msg.sender, 100e18);
    }
}

contract MyVault is ERC4626Fees {
    address internal deployer = msg.sender;

    constructor(MyToken t) ERC20("MyToken", "MTK") ERC4626(t) {
    }

    function _entryFeeBasisPoints() internal pure override returns (uint256) {
        return 100;
    }

    function _entryFeeRecipient() internal view override returns (address) {
        return deployer;
    }
}


contract Test1 is Test {
    function test() public {
        MyToken t = new MyToken();
        MyVault v = new MyVault(t);
        t.approve(address(v), type(uint256).max);

        uint256 preview = v.previewDeposit(100e18);
        v.deposit(100e18, address(this));
        uint256 balance = v.balanceOf(address(this));
        assertEq(preview, balance);
    }
}