Improve ERC4626 fees example (#4476)

Co-authored-by: Francisco <[email protected]> Co-authored-by: Hadrien Croubois <[email protected]> (cherry picked from commit f631d8a)
OpenZeppelin · Sep 21, 2023 · 1014ac2 · 1014ac2
1 parent 0ed435b
commit 1014ac2
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 36 deletions.
diff --git a/contracts/mocks/docs/ERC4626Fees.sol b/contracts/mocks/docs/ERC4626Fees.sol
@@ -4,36 +4,41 @@ pragma solidity ^0.8.0;
 
 import "../../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;
 
-    /** @dev See {IERC4626-previewDeposit}. */
+    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, _entryFeeBasePoint());
+        uint256 fee = _feeOnTotal(assets, _entryFeeBasisPoints());
         return super.previewDeposit(assets - fee);
     }
 
-    /** @dev See {IERC4626-previewMint}. */
+    /// @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, _entryFeeBasePoint());
+        return assets + _feeOnRaw(assets, _entryFeeBasisPoints());
     }
 
-    /** @dev See {IERC4626-previewWithdraw}. */
+    /// @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, _exitFeeBasePoint());
+        uint256 fee = _feeOnRaw(assets, _exitFeeBasisPoints());
         return super.previewWithdraw(assets + fee);
     }
 
-    /** @dev See {IERC4626-previewRedeem}. */
+    /// @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, _exitFeeBasePoint());
+        return assets - _feeOnTotal(assets, _exitFeeBasisPoints());
     }
 
-    /** @dev See {IERC4626-_deposit}. */
+    /// @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, _entryFeeBasePoint());
+        uint256 fee = _feeOnTotal(assets, _entryFeeBasisPoints());
         address recipient = _entryFeeRecipient();
 
         super._deposit(caller, receiver, assets, shares);
@@ -43,15 +48,15 @@ abstract contract ERC4626Fees is ERC4626 {
         }
     }
 
-    /** @dev See {IERC4626-_deposit}. */
+    /// @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, _exitFeeBasePoint());
+        uint256 fee = _feeOnRaw(assets, _exitFeeBasisPoints());
         address recipient = _exitFeeRecipient();
 
         super._withdraw(caller, receiver, owner, assets, shares);
@@ -61,27 +66,35 @@ abstract contract ERC4626Fees is ERC4626 {
         }
     }
 
-    function _entryFeeBasePoint() internal view virtual returns (uint256) {
-        return 0;
+    // === Fee configuration === ///
+
+    function _entryFeeBasisPoints() internal view virtual returns (uint256) {
+        return 0; // replace with e.g. 1_000 for 1%
     }
 
-    function _entryFeeRecipient() internal view virtual returns (address) {
-        return address(0);
+    function _exitFeeBasisPoints() internal view virtual returns (uint256) {
+        return 0; // replace with e.g. 1_000 for 1%
     }
 
-    function _exitFeeBasePoint() internal view virtual returns (uint256) {
-        return 0;
+    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);
+        return address(0); // replace with e.g. a treasury address
     }
 
-    function _feeOnRaw(uint256 assets, uint256 feeBasePoint) private pure returns (uint256) {
-        return assets.mulDiv(feeBasePoint, 1e5, Math.Rounding.Up);
+    // === 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);
     }
 
-    function _feeOnTotal(uint256 assets, uint256 feeBasePoint) private pure returns (uint256) {
-        return assets.mulDiv(feeBasePoint, feeBasePoint + 1e5, 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);
     }
 }
diff --git a/contracts/mocks/token/ERC4646FeesMock.sol b/contracts/mocks/token/ERC4646FeesMock.sol
@@ -5,33 +5,33 @@ pragma solidity ^0.8.0;
 import "../docs/ERC4626Fees.sol";
 
 abstract contract ERC4626FeesMock is ERC4626Fees {
-    uint256 private immutable _entryFeeBasePointValue;
+    uint256 private immutable _entryFeeBasisPointValue;
     address private immutable _entryFeeRecipientValue;
-    uint256 private immutable _exitFeeBasePointValue;
+    uint256 private immutable _exitFeeBasisPointValue;
     address private immutable _exitFeeRecipientValue;
 
     constructor(
-        uint256 entryFeeBasePoint,
+        uint256 entryFeeBasisPoints,
         address entryFeeRecipient,
-        uint256 exitFeeBasePoint,
+        uint256 exitFeeBasisPoints,
         address exitFeeRecipient
     ) {
-        _entryFeeBasePointValue = entryFeeBasePoint;
+        _entryFeeBasisPointValue = entryFeeBasisPoints;
         _entryFeeRecipientValue = entryFeeRecipient;
-        _exitFeeBasePointValue = exitFeeBasePoint;
+        _exitFeeBasisPointValue = exitFeeBasisPoints;
         _exitFeeRecipientValue = exitFeeRecipient;
     }
 
-    function _entryFeeBasePoint() internal view virtual override returns (uint256) {
-        return _entryFeeBasePointValue;
+    function _entryFeeBasisPoints() internal view virtual override returns (uint256) {
+        return _entryFeeBasisPointValue;
     }
 
     function _entryFeeRecipient() internal view virtual override returns (address) {
         return _entryFeeRecipientValue;
     }
 
-    function _exitFeeBasePoint() internal view virtual override returns (uint256) {
-        return _exitFeeBasePointValue;
+    function _exitFeeBasisPoints() internal view virtual override returns (uint256) {
+        return _exitFeeBasisPointValue;
     }
 
     function _exitFeeRecipient() internal view virtual override returns (address) {

diff --git a/test/token/ERC20/extensions/ERC4626.test.js b/test/token/ERC20/extensions/ERC4626.test.js
@@ -689,9 +689,9 @@ contract('ERC4626', function (accounts) {
   }
 
   describe('ERC4626Fees', function () {
-    const feeBasePoint = web3.utils.toBN(5e3);
+    const feeBasisPoints = web3.utils.toBN(5e3);
     const amountWithoutFees = web3.utils.toBN(10000);
-    const fees = amountWithoutFees.mul(feeBasePoint).divn(1e5);
+    const fees = amountWithoutFees.mul(feeBasisPoints).divn(1e4);
     const amountWithFees = amountWithoutFees.add(fees);
 
     describe('input fees', function () {
@@ -701,7 +701,7 @@ contract('ERC4626', function (accounts) {
           name + ' Vault',
           symbol + 'V',
           this.token.address,
-          feeBasePoint,
+          feeBasisPoints,
           other,
           0,
           constants.ZERO_ADDRESS,